-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(auth): implement regional access boundary support for standalone JWT and async service accounts #17025
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
Merged
+1,769
−174
Merged
feat(auth): implement regional access boundary support for standalone JWT and async service accounts #17025
Changes from all commits
Commits
Show all changes
34 commits
Select commit
Hold shift + click to select a range
b9b8316
feat: add async support for RAB to ServiceAccountCredentials and impl…
nbayati a5b9adf
Add unit tests for the async RAB implementation.
nbayati 09b9f41
fix async unit tests
nbayati 59dbb2c
Update unit tests to accept both mtls and standard allowedLocations e…
nbayati be55639
test: verify iam endpoint constant resolution in mTLS environments
nbayati d38da42
refactor: introduce _after_refresh hook in Credentials base class to …
nbayati 44f1110
add __setstate__ to the base RAB class for backward compatibility
nbayati aeb426c
Implement RAB support for jwt credentials
nbayati 6e0c00f
fix lint errors
nbayati 6a7bb27
fix: preserve refresh manager type when copying RAB manager
nbayati 2682925
refactor(auth): optimize RAB manager copy logic to only share boundar…
nbayati 140ee7e
fix(auth): enhance client lookup robustness with defensive checks and…
nbayati ad5baca
refactor(auth): centralize async RAB lifecycle via _after_refresh bas…
nbayati 529355a
feat: add pickling support for _AsyncRegionalAccessBoundaryRefreshMan…
nbayati fbe32fe
revert changes to the _token_endpoint_request_no_throw to keep PR foc…
nbayati 6676fd1
fix(auth): align async client with AIO transport spec and add unit tests
nbayati 832ae7b
test(auth): assert closed session safety in async RAB refresh and fix…
nbayati 106e72b
docs(auth): clarify async RAB transport requirements in docstrings
nbayati 7e65ee7
feat(auth): support async blocking RAB lookups and add support to asy…
nbayati 9475f9e
update unit tests
nbayati ef80089
test(auth): assert RAB blocking flag and data copying on subclass cre…
nbayati bb117de
fix(auth): allow status-based retries on non-JSON RAB lookup failures
nbayati 69d8300
fix: catch generic exceptions during async body streaming
nbayati 4c9796f
refactor(auth): move RAB endpoints to dynamic utils to support runtim…
nbayati 3177328
fix(auth): prevent sync-on-async crash in RAB manager blocking refresh
nbayati 56542c4
fix(auth): skip compute engine RAB lookup for non-SA based identities
nbayati fb10a3f
fix(auth): add async before_request to _jwt_async.OnDemandCredentials…
nbayati 1ef1f1f
refactor(auth): move RAB helper imports to top level
nbayati 8087ff0
fix(auth): retry RAB lookup on timeout and transport errors in async …
nbayati 1a5004e
fix(auth): propagate blocking RAB config to impersonated credentials
nbayati 05c9942
fix(auth): restrict metadata email check to standard email format
nbayati 64a6723
fix lint errors
nbayati 9917119
test(auth): mock sleep in client retry tests to speed up execution
nbayati 0ec659e
style: fix lint issues
nbayati File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
_jwt_async.OnDemandCredentialsinherits fromjwt.OnDemandCredentialsfirst. Becausejwt.OnDemandCredentialsimplementsbefore_requestsynchronously, Python's MRO resolvesbefore_requestto the synchronous parent, shadowing the async base.When called via the experimental async HTTP transport (
_aiohttp_requests.py), which has no initialization type-checking and explicitly awaitsbefore_request(...), the synchronous method executes, returnsNone, and the event loop attempts toawait None, raising:TypeError: object NoneType can't be used in 'await' expressionThere 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.
Thanks for catching this. I've resolved this by adding an async def before_request override to the OnDemandCredentials class that delegates to the parent class, and updated the corresponding async test cases to properly await the call.
As a side note, I have OnDemandCredentials on my radar as a class that might need to support RAB in the future, but I have decided to keep it out of scope for this PR.
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.
Ok sounds good. I think we have a few open items to follow up on IIRC. Where are they being tracked?
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'm tracking the additional credential types in the RAB google sheet tracker for a follow up release.
For the items that weren't RAB specific, I have made a github issue: