Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix scopes joining #7706

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Fix scopes joining #7706

merged 1 commit into from
Apr 30, 2021

Conversation

lukasz-walkiewicz
Copy link
Member

Multiple OAuth2 scopes should be space-separated

@cla-bot cla-bot bot added the cla-signed label Apr 22, 2021
@kokosing kokosing requested a review from s2lomon April 23, 2021 10:11
s2lomon
s2lomon approved these changes Apr 23, 2021
View reviewed changes
Copy link
Member

@s2lomon s2lomon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, just as the @kokosing said, a test would be welcomed.

@lukasz-walkiewicz
Copy link
Member Author

I've added a test.

kokosing
kokosing approved these changes Apr 28, 2021
View reviewed changes
Copy link
Member

@kokosing kokosing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for test. Verified locally and it works as expected.

...ino-main/src/test/java/io/trino/server/security/oauth2/TestDynamicCallbackOAuth2Service.java Outdated Show resolved Hide resolved
...ino-main/src/test/java/io/trino/server/security/oauth2/TestDynamicCallbackOAuth2Service.java Outdated Show resolved Hide resolved
core/trino-main/src/main/java/io/trino/server/security/oauth2/ScribeJavaOAuth2Client.java Outdated Show resolved Hide resolved
@kokosing
Copy link
Member 

2021-04-28T12:51:09.8013701Z java.lang.RuntimeException: Error reading JWK keys from https://localhost:49169/.well-known/jwks.json
2021-04-28T12:51:09.8015630Z 	at io.trino.server.security.jwt.JwkService.fetchKeys(JwkService.java:140)
2021-04-28T12:51:09.8017798Z 	at io.trino.server.security.jwt.JwkService.refreshKeys(JwkService.java:128)
2021-04-28T12:51:09.8019563Z 	at io.trino.server.security.jwt.JwkService.lambda$start$0(JwkService.java:86)
2021-04-28T12:51:09.8021189Z 	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
2021-04-28T12:51:09.8022752Z 	at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
2021-04-28T12:51:09.8024873Z 	at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305)
2021-04-28T12:51:09.8027210Z 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
2021-04-28T12:51:09.8029137Z 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
2021-04-28T12:51:09.8030479Z 	at java.base/java.lang.Thread.run(Thread.java:829)
2021-04-28T12:51:09.8032201Z Caused by: java.util.concurrent.RejectedExecutionException: AuthorizationPreservingHttpClient@29cf6c03{STOPPED} is stopped
2021-04-28T12:51:09.8036314Z 	at org.eclipse.jetty.client.HttpDestination.send(HttpDestination.java:298)
2021-04-28T12:51:09.8038141Z 	at org.eclipse.jetty.client.HttpDestination.send(HttpDestination.java:263)
2021-04-28T12:51:09.8039501Z 	at org.eclipse.jetty.client.HttpClient.send(HttpClient.java:600)
2021-04-28T12:51:09.8040830Z 	at org.eclipse.jetty.client.HttpRequest.sendAsync(HttpRequest.java:778)
2021-04-28T12:51:09.8042202Z 	at org.eclipse.jetty.client.HttpRequest.send(HttpRequest.java:765)
2021-04-28T12:51:09.8044323Z 	at io.airlift.http.client.jetty.JettyHttpClient.execute(JettyHttpClient.java:507)
2021-04-28T12:51:09.8046251Z 	at io.trino.server.security.jwt.JwkService.fetchKeys(JwkService.java:137)
2021-04-28T12:51:09.8047617Z 	... 8 more

Should I be concerned? Not sure what tests produced that.

@kokosing
Copy link
Member 

2021-04-28T13:11:37.4889284Z 2021-04-28T08:11:37.488-0500	INFO	pool-36-thread-2	io.airlift.bootstrap.LifeCycleManager	Life cycle stopped
2021-04-28T13:11:56.8063182Z ##[error]The operation was canceled.

It looks like testng stopped by JVM didn't quit.

@kokosing
Copy link
Member

CI hit: #7783

@lukasz-walkiewicz
Copy link
Member Author

CI hit: #7805

@lukasz-walkiewicz
Fix scopes joining
c0a1f10 
Multiple OAuth2 scopes should be space-separated
@kokosing kokosing merged commit d69b6c9 into trinodb:master Apr 30, 2021
@kokosing kokosing mentioned this pull request Apr 30, 2021
Closed
9 tasks
@martint martint added this to the 356 milestone Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s2lomon s2lomon s2lomon approved these changes

@kokosing kokosing kokosing approved these changes

Assignees
No one assigned
Labels
cla-signed
Milestone
356
Development

Successfully merging this pull request may close these issues.
4 participants
@lukasz-walkiewicz @kokosing @s2lomon @martint