Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs(samples): added auth samples and tests #927
docs(samples): added auth samples and tests #927
Changes from all commits
3bd604d
bd1bc55
ece1c56
0c26c33
45449b2
7ef36df
b2f367e
01b57ad
b9dee60
3ed6be0
dd39fd4
a46ccc4
5eb6506
cc6a5a5
7c68634
1baeebe
9fc3a7a
b8c5f3f
680cfdd
1944e5d
a943290
d7d6257
cf11754
b910be5
e23fe35
ada91fb
8573bb3
591d0e6
82d7350
54013b5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This should use a try/catch and include the warning about making sure to close: disingenuous: https://googlecloudplatform.github.io/samples-style-guide/#clients
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.
I am not sure of this as many of the storage samples look like they don't follow the auto close try-catch..
https://github.com/googleapis/java-storage/blob/main/samples/snippets/src/main/java/com/example/storage/bucket/ListBuckets.java
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.
I've also separated out the delegation flow from the single service account flow. It doesn't make sense to create an ID token from a delegation chain, because there's no authz included for ID tokens. So let's just stick to a single privilege-bearing (current term, subject to change) service account (no delegation chain).
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.
Reopening because you are still including delegate chain. There should be just one privilege-bearing service account and one caller, which can be represented by either a service account or a user account.
https://cloud.google.com/iam/docs/create-short-lived-credentials-direct
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 delegates parameter is mandatory in the "Impersonated Credentials" API, unlike the other one we used from IAM library.
Hence, I am unable to remove it. Added clarifying comments stating that it's optional. wdyt?
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.
Could we just say
// delegates: Unused for this call
or something like that? I'm concerned that the subject of chaining is very complex, and that folks will spend a lot of time trying to parse and understand what identities need to have what permission on what-- only to get to the bottom and find out it's unused. Why put them through that?