-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add Google client initialisation helper method to generate ID tokens #78
Merged
deadlycoconuts
merged 15 commits into
caraml-dev:main
from
deadlycoconuts:add_common_google_auth_methods
Mar 28, 2023
Merged
Add Google client initialisation helper method to generate ID tokens #78
deadlycoconuts
merged 15 commits into
caraml-dev:main
from
deadlycoconuts:add_common_google_auth_methods
Mar 28, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deadlycoconuts
changed the title
Add Google Client initialisation helper method for various types of credentials
Add Google client initialisation helper method to generate ID tokens
Mar 27, 2023
deadlycoconuts
force-pushed
the
add_common_google_auth_methods
branch
from
March 27, 2023 11:33
ed62d53
to
1cc4b28
Compare
deadlycoconuts
commented
Mar 27, 2023
krithika369
approved these changes
Mar 28, 2023
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.
@deadlycoconuts, thanks for the detailed PR description and the neat solution. Left some small comments. The rest LGTM!
krithika369
reviewed
Mar 28, 2023
Thanks a lot for the comments! I'll be merging this now! 🚀 |
deadlycoconuts
added a commit
to caraml-dev/merlin
that referenced
this pull request
Mar 30, 2023
**What this PR does / why we need it**: With the introduction of a helper method in MLP (caraml-dev/mlp#78) to abstract away the creation of a Google client to append ID tokens to the headers of outgoing requests, that is authenticated with service or user accounts, this PR imports the said helper method and uses it to initialise its Google client. This ensures that all outgoing requests from the Turing API server contain ID tokens in their headers. **Which issue(s) this PR fixes**: Fixes # **Does this PR introduce a user-facing change?**: ```release-note NONE ``` **Checklist** - [ ] Added unit test, integration, and/or e2e tests - [ ] Tested locally - [ ] Updated documentation - [ ] Update Swagger spec if the PR introduce API changes - [ ] Regenerated Golang and Python client if the PR introduce API changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Context
With the move to use OIDC identity (ID) tokens instead of OAuth2.0 access tokens to authenticate all users of CaraML (including end-users and more importantly in the context of this PR, other CaraML components), there is a need to update the way in which each individual CaraML component authenticates itself and the token which gets injected into the header of every single outgoing request from them.
As of present each outgoing request from a CaraML component has an OAuth2.0 access token appended to its header, and this process is handled by an HTTP client generated by one of the helper methods from the Google client library, not unlike the one below:
However, this helper method does not generate an HTTP client that will append ID tokens to outgoing requests; a separate helper method from a different Google client library is needed to do that:
To complicate things further, the Google client libraries does not (easily) allow ID tokens to be generated when a service account is not used for authentication, and instead defaults to generating access tokens. Hence, a separate method to generate the HTTP client is needed for scenarios when the other forms of credentials are used for authentication, such as most notably, user accounts.
This PR thus introduces a helper method that handles not only the HTTP client creation for authentication with a service account, but also for other types of credentials. The latter is done by manually initialising the token source of the credentials, and then manually extracting the ID token from them before encapsulating them in a regular HTTP client struct - this process is highly inspired by what was suggested as a hack by unhappy users here to retrieve the ID token manually when not using service accounts, as well as the Google client library's way of wrapping a
TokenSource
struct into an HTTP client.This helper method will subsequently be imported and used by the various CaraML components to initialise an authenticated Google client, which will ensure that all outgoing requests from them would contain ID tokens, regardless of the type of credentials used.