Skip to content

fix(auth): only trigger mTLS certificate rotation on mTLS endpoints - #17928

Open
attharva-24 wants to merge 3 commits into
googleapis:mainfrom
attharva-24:fix/requests-mtls-url-check
Open

fix(auth): only trigger mTLS certificate rotation on mTLS endpoints#17928
attharva-24 wants to merge 3 commits into
googleapis:mainfrom
attharva-24:fix/requests-mtls-url-check

Conversation

@attharva-24

Copy link
Copy Markdown
Contributor

Description

In AuthorizedSession.request within the requests transport, a 401 Unauthorized response triggered the mTLS client certificate rotation check regardless of whether the endpoint was actually an mTLS URL.

For sessions configured with mTLS support, calling standard, non-mTLS endpoints (like standard googleapis.com URLs) would trigger unnecessary certificate parsing, disk reads, and adapter reconfiguration upon token expiration.

This PR adds a URL verification check (matching the behavior in urllib3.py) to ensure certificate rotation is only triggered on requests sent to designated mTLS endpoints (mtls.googleapis.com or mtls.sandbox.googleapis.com).

Tests

  • Added test_cert_rotation_skipped_on_non_mtls_url in tests/transport/test_requests.py to verify that rotation is skipped on non-mTLS URLs.
  • Updated existing mTLS rotation tests to target actual mTLS URLs so that certificate rotation is correctly triggered and tested.

@attharva-24
attharva-24 requested review from a team as code owners July 28, 2026 20:34

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restricts mTLS certificate rotation on 401 Unauthorized responses to only occur when the request URL matches specific mTLS prefixes (such as mtls.googleapis.com or mtls.sandbox.googleapis.com), and updates the test suite to verify this behavior. The feedback suggests optimizing the URL prefix check by using a generator expression inside the any() function to allow short-circuiting instead of evaluating a full list comprehension.

if response.status_code == http_client.UNAUTHORIZED:
if self.is_mtls:
MTLS_URL_PREFIXES = ["mtls.googleapis.com", "mtls.sandbox.googleapis.com"]
use_mtls = self.is_mtls and any([prefix in url for prefix in MTLS_URL_PREFIXES])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a list comprehension inside any() creates a temporary list in memory and evaluates all elements, defeating the short-circuiting behavior of any(). Using a generator expression instead allows any() to short-circuit as soon as a match is found, which is more efficient.

Suggested change
use_mtls = self.is_mtls and any([prefix in url for prefix in MTLS_URL_PREFIXES])
use_mtls = self.is_mtls and any(prefix in url for prefix in MTLS_URL_PREFIXES)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a fair optimization to use

@parthea parthea assigned attharva-24 and unassigned macastelaz Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants