-
Notifications
You must be signed in to change notification settings - Fork 312
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: service account is able to use a private token endpoint #784
Conversation
@silvolu @arithmetic1728 PTAL. The corresponding bug is 190841374. |
Please wait for me on this. |
Tagged as |
The fix should be simpler here: when exchanging a JWT for an access token, the value of the 'aud' field must be |
@silvolu The token uri is not hardcoded because (1) google-auth allows users to provide their own token uri (even though it should be always google-auth-library-python/google/oauth2/service_account.py Lines 123 to 127 in 22affe1
(2) if the creds is created from service_account.json file, then google-auth reads token uri from the json file google-auth-library-python/google/oauth2/service_account.py Lines 237 to 240 in 22affe1
|
I'm not saying that the token_uri should be hardcoded, I'm saying that the audience field should be hardcoded. This class is not a generic implementation, the description clearly says "This module implements the JWT Profile for OAuth 2.0 Authorization Grants as defined by In Google's infrastructure, when exchanging a JWT for a token, the audience is always |
Sorry I misunderstood your comment. Yea, this way is much simpler. |
I updated the PR to hard-code the aud which is much simpler. Please take another look. |
looks good to me |
@liuchaoren Yes I believe Tres added that to prevent an early merge. I've removed the label. |
@liuchaoren It looks like some unit tests are failing, could you take a look? To run the unit tests locally, |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
In Private Service Connect, users can use an endpoint which is private to their VPC network. The request is eventually routed to the oauth2.googleapis.com/token so the "aud" in the assertion still should be oauth2.googleapis.com/token.
After this change, service account can send requests to the private endpoint (if configured) and still use the oauth2.googleapis.com/token in the assertion.