-
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
Open
nbayati
wants to merge
19
commits into
googleapis:main
Choose a base branch
from
nbayati:rab-support-async-sa-jwt
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat(auth): implement regional access boundary support for standalone JWT and async service accounts #17025
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
b33d818
feat: add async support for RAB to ServiceAccountCredentials and impl…
nbayati 80529c6
Add unit tests for the async RAB implementation.
nbayati 73617c5
fix async unit tests
nbayati 9e43ecc
Update unit tests to accept both mtls and standard allowedLocations e…
nbayati 4d5787b
test: verify iam endpoint constant resolution in mTLS environments
nbayati e3f8e90
refactor: introduce _after_refresh hook in Credentials base class to …
nbayati 95af8e5
add __setstate__ to the base RAB class for backward compatibility
nbayati 1b4270b
Implement RAB support for jwt credentials
nbayati 0478330
fix lint errors
nbayati 9650ac7
fix: preserve refresh manager type when copying RAB manager
nbayati 5b6e715
refactor(auth): optimize RAB manager copy logic to only share boundar…
nbayati 2729e70
fix(auth): enhance client lookup robustness with defensive checks and…
nbayati e1ee432
refactor(auth): centralize async RAB lifecycle via _after_refresh bas…
nbayati 39fbe10
feat: add pickling support for _AsyncRegionalAccessBoundaryRefreshMan…
nbayati d20d7ba
revert changes to the _token_endpoint_request_no_throw to keep PR foc…
nbayati c1b946e
fix(auth): align async client with AIO transport spec and add unit tests
nbayati 75b3231
test(auth): assert closed session safety in async RAB refresh and fix…
nbayati a952527
docs(auth): clarify async RAB transport requirements in docstrings
nbayati be67ab6
feat(auth): support async blocking RAB lookups and add support to asy…
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.
Unlike the synchronous refresh manager which safely deepcopies the transport (
copied_request = copy.deepcopy(request)), the async manager passes the exact samerequestinstance directly into the background coroutine task. Becausestart_refreshis invoked insidebefore_request, the main application coroutine immediately proceeds to make its actual service API HTTP call using the exact samerequesttransport while the background task is concurrently using it, risking HTTP state corruption or interleaved headers.Additionally, spawning
asyncio.create_task(_worker())without tracking cancellation hooks upon client session closure can potentially cause dangling tasks that raiseRuntimeError: Session is closedwhen executing against closed client sessions.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.
Both concerns are valid, but safe under the hood:
Deepcopying the Transport is impossible and not needed in async
The async transport object (e.g., aiohttp_requests.Request) contains an active aiohttp.ClientSession with open TCP sockets. Attempting to copy.deepcopy(request) will instantly raise a TypeError: cannot pickle 'ClientSession' object and crash the application at runtime. Unlike synchronous transports running on separate OS threads, async HTTP clients (like aiohttp or httpx) are natively designed to handle concurrent requests sharing the same connection session. All request-specific states (headers, payloads) are stored in localized coroutine call stack frames, preventing any HTTP state corruption or interleaving.
Session closure and dangling tasks are handled safely
The background worker is a single-shot asyncio task that executes exactly one lookup request and then immediately terminates and gets garbage-collected.
If the user's application closes the underlying client session while a background task is still running, the resulting RuntimeError: Session is closed is safely caught by the worker's generic except Exception as e: block. It logs a warning, fails open cleanly, and does not raise an unhandled exception or crash the application.
I think no code changes are required here, as the current design is fully protected.