-
Notifications
You must be signed in to change notification settings - Fork 33
Use user provided scopes to all OAuth authentication flow. #495
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.
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
Conversation
|
|
||
| List<String> scopes = b.scopes; | ||
| if (scopes == null) { | ||
| scopes = Arrays.asList("offline_access", "clusters", "sql"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we are changing the default scopes to "all".
It will work. But is this a security concern? Users will start getting tokens with more scopes without noticing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Python and Go SDKs use all-apis as default for this flow. I don't know if using more restrictive scopes in Java is intentional or accidental. My guess is the latter given that these values were set in one of the very first SDK commit. Do you have more context? In any case, I think this is something worth better calling out in the changelogs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea. It makes sense to have parity. Lets use all-apis and add it to the Changelog.
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
What changes are proposed in this pull request?
This PR updates OAuth related flows to use the scopes user provided scopes if any, or default to
all-apisotherwize.How is this tested?
Unit and integration tests.