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.

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

feat: implements the OAuth token exchange spec based on rfc8693 #598

Merged
merged 5 commits into from
Sep 10, 2020
Merged

feat: implements the OAuth token exchange spec based on rfc8693 #598

merged 5 commits into from
Sep 10, 2020

Conversation

bojeil-google
Copy link
Contributor

Implements an internal utility for exchanging OAuth tokens using the rfc/8693 spec.

@google-cla
Copy link

google-cla bot commented Sep 3, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Sep 3, 2020
@google-cla
Copy link

google-cla bot commented Sep 3, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

subject_token_type,
resource=None,
audience=None,
scope=None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be renamed to scopes for consistency with other methods in this library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The spec defines this as scope (the request parameter) but since that is the space separated string, and this is an array which will be joined into a string, I have renamed it to scopes. Note that the base credential class will align with the rest of the library in using scopes.

actor_token (Optional[str]): The optional OAuth 2.0 token exchange actor token.
actor_token_type (Optional[str]): The optional OAuth 2.0 token exchange actor token type.
additional_options (Optional[Mapping[str, str]]): The optional additional
non-standard GCP specific options.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above

Suggested change
non-standard GCP specific options.
non-standard Google specific options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

The implementation will support various types of client authentication as
allowed in the spec.

A deviation on the spec will be for additional GCP specific options that
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless these options are only available with Cloud APIs, I think it's better to be more general (Google instead of GCP).

Suggested change
A deviation on the spec will be for additional GCP specific options that
A deviation on the spec will be for additional Google specific options that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@google-cla
Copy link

google-cla bot commented Sep 10, 2020

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@busunkim96 busunkim96 added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 10, 2020
@busunkim96 busunkim96 merged commit 1b45700 into googleapis:byoid Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants