fix(auth): keep subject when copying impersonated credentials - #18429
Open
Om-singhaI wants to merge 1 commit into
Open
Om-singhaI wants to merge 1 commit into
Om-singhaI wants to merge 1 commit into
Conversation
`_make_copy` rebuilt the credential without `subject`, so `with_scopes`, `with_quota_project` and the deprecated `with_trust_boundary` returned a copy that no longer did domain wide delegation. The copy's refresh calls `:generateAccessToken` and mints a token for the service account instead of the end user, and it rebuilds the Regional Access Boundary lookup URL that googleapis#17763 skips while a subject is set.
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the _make_copy method in impersonated_credentials.py to preserve the subject attribute when copying credentials. It also adds corresponding unit tests to ensure that with_quota_project and with_scopes correctly preserve the subject and maintain expected behaviors, such as domain-wide delegation. There are no review comments to address.
This file contains hidden or 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
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.
Fixes #18428 🦕
_make_copyingoogle/auth/impersonated_credentials.pyrebuilds the credential throughself.__class__(...)and forwards every constructor argument exceptsubject. The domain wide delegation flow landed in 34ee3fe on the old repo, after that helper already existed, and the helper was never updated. The same field ingoogle/oauth2/service_account.pyis forwarded by its own_make_copy.So
with_scopes(),with_quota_project()and the deprecatedwith_trust_boundary()return a credential that has quietly stopped doing domain wide delegation. It refreshes through:generateAccessToken, so the token belongs to the service account rather than the user, and it rebuilds the Regional Access Boundary lookup URL that #17763 skips while a subject is set.Anyone whose setup works today because the copy drops the subject would see a change, but that means relying on the copy authenticating as the service account.
What changed:
_make_copypassessubject=self._subject, next to the arguments it already passes.tests/test_impersonated_credentials.py: the subject surviveswith_quota_projectand the RAB lookup URL staysNone, the subject surviveswith_scopes, and a scoped copy still refreshes through:signJwt.Testing, from
packages/google-authon macOS 26.6.2 with Python 3.13.15 andpip install -e ".[requests]":python -m pytest tests/test_impersonated_credentials.py -q: 62 passed, against 58 on main.python -m pytest tests/test_credentials.py -q: 25 passed.python -m pytest tests/test__default.py -q: 96 passed.google/auth/impersonated_credentials.pyto main and keeping the new tests: all four new cases fail, two onassert None == 'user@example.com'and two with aRefreshErrorbecause the copy posts to:generateAccessToken.ruff check --select I --line-length=88andruff format --check --line-length=88pass on both files, andflake8 google/auth/impersonated_credentials.py tests/test_impersonated_credentials.pyis clean. My ruff was 0.15.17, not the pinned 0.14.14._make_copy, on the path the new assertions read, so it's covered.