feat: [aiohttp] Add mTLS reconfiguration logic when certificate mismatch for existing credentials & Agent Identity workloads - #18224
feat: [aiohttp] Add mTLS reconfiguration logic when certificate mismatch for existing credentials & Agent Identity workloads #18224agrawalradhika-cell wants to merge 19 commits into
Conversation
feat: Add retry for cert rotation handling
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces client certificate rotation handling for asynchronous authorized sessions when encountering an unauthorized response under mTLS. The review feedback highlights a violation of the repository style guide regarding exception contract compliance, suggesting that the certificate parameter check should be wrapped in a try-except block to gracefully fall back to the original response rather than crashing. Additionally, the feedback recommends updating the corresponding unit tests to assert this resilient fallback behavior.
agrawalradhika-cell
changed the title
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Handle exceptions during mTLS reconfiguration with warnings instead of errors.
…logs Updated test logic to assert response instead of expecting an error.
…sync executor Refactor unauthorized response handling to use async executor for MTLS parameter checks.
chore: Reset mTLS init task upon client certificate change
Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>
…eck after 401 check chore: Refactor mTLS channel reconfiguration logic for adding mTLS check after 401 check
Implement mTLS rotation lock to prevent race conditions during certificate reconfiguration.
chore: Change warning to error log for mTLS channel reconfiguration failure.
chore: Refactor mTLS handling for unauthorized responses
Remove unnecessary continue statement after mTLS configuration.
Refactor tests for certificate rotation and error handling in AsyncAuthorizedSession. Update test names for clarity and ensure proper logging of errors.
Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>
Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>
Handle RefreshError during credential refresh to prevent unhandled exceptions.
Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>
| url, | ||
| data=data, | ||
| headers=headers, | ||
| max_allowed_time=max_allowed_time, |
There was a problem hiding this comment.
max_allowed_time=max_allowed_time resets the timer on each retry instead of limiting total runtime.
| or hasattr(data, "read") | ||
| ) | ||
| if getattr(self, "is_mtls", False) and any( | ||
| prefix in url for prefix in MTLS_URL_PREFIXES |
There was a problem hiding this comment.
prefix in url searches the entire URL, so paths or query parameters can trigger certificate rotation on standard hosts. Parse and normalize urlsplit(url).hostname, then match only exact hostnames or subdomains of documented mTLS endpoints.
| else: | ||
| response.close() | ||
| try: | ||
| await self._credentials.refresh(self._auth_request) |
There was a problem hiding this comment.
Closing the 401 response before credentials refresh returns an unreadable response if RefreshError occurs. Also, StaticCredentials and AnonymousCredentials raise uncaught InvalidOperation errors during refresh. Close the response only after refresh succeeds. Return the open original response if refresh fails or is unsupported.
| ): | ||
| self._mtls_init_task = None | ||
| await self.configure_mtls_channel( | ||
| lambda: (call_cert_bytes, call_key_bytes) |
There was a problem hiding this comment.
Initial setup can use client_cert_callback, but check_parameters_for_unauthorized_response always checks application default credentials. This mismatch can replace custom certificates or skip rotation. Save the initial certificate source, and reuse it for all fingerprint checks and reconfiguration.
| and self._mtls_init_task.done() | ||
| ): | ||
| self._mtls_init_task = None | ||
| await self.configure_mtls_channel( |
There was a problem hiding this comment.
configure_mtls_channel closes the active ClientSession, which aborts in-flight concurrent requests. Keep old sessions open until their requests finish, or close them in AsyncAuthorizedSession.close(). Add a concurrency regression test for this case.
3 participants
feat: [aiohttp] Add mTLS reconfiguration logic when certificate mismatch for existing credentials & Agent Identity workloads
Fixes #18227 #18227 🦕