Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 212 additions & 12 deletions packages/google-auth/google/auth/transport/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,23 @@
import http.client as http_client
import logging
import numbers
import threading
import time
from typing import Optional
from urllib import parse as urllib_parse

try:
import requests
import requests.adapters
import requests.exceptions
from requests.packages.urllib3.util.ssl_ import ( # type: ignore
create_urllib3_context,
)
import urllib3.exceptions
except ImportError as caught_exc: # pragma: NO COVER
raise ImportError(
"The requests library is not installed from please install the requests package to use the requests transport."
) from caught_exc
import requests.adapters # pylint: disable=ungrouped-imports
import requests.exceptions # pylint: disable=ungrouped-imports
from requests.packages.urllib3.util.ssl_ import ( # type: ignore
create_urllib3_context,
) # pylint: disable=ungrouped-imports

from google.auth import _helpers
from google.auth import exceptions
Expand Down Expand Up @@ -144,6 +147,17 @@ def __init__(self, session: Optional[requests.Session] = None) -> None:

self.session = session

# The adapter this Request mounted. Stays None if the session already had
# an mTLS adapter when it was passed in.
self._mtls_adapter = None
self._cached_cert = None

# Prefixes on self.session where _mtls_adapter is mounted, so a rotation
# can remount the new adapter on all of them.
self._mounted_mtls_prefixes = set()

self._mtls_lock = threading.RLock()

def __del__(self):
try:
if hasattr(self, "session") and self.session is not None:
Expand All @@ -154,14 +168,168 @@ def __del__(self):
# TypeError.
pass

def _configure_mtls_if_needed(
self, url, force_reconfigure=False, client_cert_callback=None
):
"""Lazily mounts a mutual TLS adapter onto the session for mTLS endpoints.

Args:
url (str): The target request URL.
force_reconfigure (bool): If True, rebuilds the mTLS adapter and remounts
all tracked URL prefixes even if already configured.
client_cert_callback (Optional[Callable[[], Tuple[bytes, bytes]]]): Optional
callback returning (cert_bytes, key_bytes) in PEM format.
"""
if not _mtls_helper.is_mtls_endpoint(url):
return
if not _mtls_helper.check_use_client_cert():
return

# Mount on the specific origin rather than globally on "https://" so client
# certificates are not sent to other hosts sharing this session. The trailing
# slash prevents requests' startswith() matching from matching lookalike domains.
parsed = urllib_parse.urlparse(url)
prefix = f"{parsed.scheme}://{parsed.netloc}/"

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.

Do we need to worry about case-sensitivity here?


# If the session has an mTLS adapter mounted for this URL that was not created
# by this Request (e.g. via AuthorizedSession or a custom caller mount), leave
# it untouched. Checking `session_adapter is not self._mtls_adapter` ensures
# `force_reconfigure=True` can still replace our own `_mtls_adapter`.
session_adapter = self.session.get_adapter(url if parsed.path else prefix)
if (
getattr(session_adapter, "_is_mtls", False)

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.

When AuthorizedSession.configure_mtls_channel() or AuthorizedHttp configures mTLS on the session or pool, it sets _is_mtls = True but leaves self._mtls_adapter, self._mtls_http, and self._cached_cert as None on the underlying Request object. During token refresh, _configure_mtls_if_needed and _get_http_for_url return early, used_cert evaluates to None, and 401 rotation never runs. Initialize self._cached_cert when _is_mtls is already set on the adapter or pool, and check getattr(adapter, "_is_mtls", False) when computing used_cert.

and session_adapter is not self._mtls_adapter
):
return

if not force_reconfigure and prefix in self._mounted_mtls_prefixes:
return

with self._mtls_lock:
# Re-check in case another thread mounted this prefix while waiting on the lock.
if not force_reconfigure and prefix in self._mounted_mtls_prefixes:
return

has_cert, cert, key = _mtls_helper.get_client_cert_and_key(

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.

get_client_cert_and_key() runs before the if force_reconfigure or self._mtls_adapter is None check at line 219. When _mtls_adapter already exists and a request hits a new mTLS host prefix, the else branch at line 252 reuses _mtls_adapter and discards the newly fetched certificate and key, wasting a SecureConnect subprocess fork. Moving the get_client_cert_and_key() call inside the if branch avoids the unused fetch.

client_cert_callback
)
if not has_cert:
return

if force_reconfigure or self._mtls_adapter is None:
# Copy necessary configuration from the session's current adapter so
# existing adapter settings carry over to the mTLS adapter.
kwargs = {}
if session_adapter is not None:
kwargs["max_retries"] = getattr(session_adapter, "max_retries", 0)
kwargs["pool_connections"] = getattr(
session_adapter,
"_pool_connections",
requests.adapters.DEFAULT_POOLSIZE,
)
kwargs["pool_maxsize"] = getattr(
session_adapter,
"_pool_maxsize",
requests.adapters.DEFAULT_POOLSIZE,
)
kwargs["pool_block"] = getattr(
session_adapter,
"_pool_block",
requests.adapters.DEFAULT_POOLBLOCK,
)
Comment on lines +235 to +239

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.

high

The attribute DEFAULT_POOLBLOCK does not exist in requests.adapters. The correct attribute name is DEFAULT_POOL_BLOCK. Referencing DEFAULT_POOLBLOCK will raise an AttributeError if the fallback path is ever hit.

Suggested change
kwargs["pool_block"] = getattr(
session_adapter,
"_pool_block",
requests.adapters.DEFAULT_POOLBLOCK,
)
kwargs["pool_block"] = getattr(
session_adapter,
"_pool_block",
requests.adapters.DEFAULT_POOL_BLOCK,
)

old_mtls_adapter = self._mtls_adapter
self._mtls_adapter = _MutualTlsAdapter(cert, key, **kwargs)
self._cached_cert = cert
self._mounted_mtls_prefixes.add(prefix)

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.

Adding prefix to self._mounted_mtls_prefixes before calling self.session.mount creates a race condition. Another thread checking prefix in self._mounted_mtls_prefixes at line 205 outside the lock can return early and send traffic on the default non-mTLS adapter before the mount finishes. Move self._mounted_mtls_prefixes.add(prefix) after self.session.mount.


# Mount the new adapter across all tracked .mtls. prefixes and close

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.

The comment states that old_mtls_adapter.close() clears idle pooled connections while allowing in-flight requests to complete, and lines 304 and 368 add retry logic for ClosedPoolError. However, HTTPAdapter.close() calls PoolManager.clear(), which uses RecentlyUsedContainer with dispose_func=None in both urllib3 1.26 and 2.7. Because clear() removes pool references from the container without calling pool.close(), ClosedPoolError is never raised on either urllib3 version and the retry handler at lines 304 and 368 is unreachable.

# any replaced adapter. HTTPAdapter.close() clears idle pooled connections
# without interrupting in-flight requests or streams.
for tracked_prefix in self._mounted_mtls_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.

The rotation remount loop for p in self._mounted_mtls_prefixes mounts self._mtls_adapter without checking whether the current adapter on self.session still belongs to this Request. Unlike the initial mount guard at lines 199 through 203, a rotation triggered by one endpoint will overwrite a custom adapter that the caller mounted on another prefix after initialization.

self.session.mount(tracked_prefix, self._mtls_adapter)

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.

self.session.mount() mutates and reorders self.session.adapters in place. Concurrent threads calling self.session.get_adapter(url) iterate over self.session.adapters without holding self._mtls_lock, which raises RuntimeError: OrderedDict mutated during iteration. Copy self.session.adapters, update and sort the copy, and assign it back to self.session.adapters atomically.

if old_mtls_adapter is not None:
old_mtls_adapter.close()
else:
self.session.mount(prefix, self._mtls_adapter)
self._mounted_mtls_prefixes.add(prefix)

def _handle_mtls_unauthorized_response(self, url, used_cert):
"""Handles a 401 Unauthorized response from an mTLS endpoint.

Checks whether the client certificate on disk has rotated since
``used_cert`` was cached, and reconfigures the mTLS adapter if so.

Args:
url (str): The target request URL that returned 401.
used_cert (bytes): The client certificate bytes used for the
failed request.

Returns:
bool: True if the mTLS adapter was reconfigured (by this thread or a
concurrent thread) and the request should be retried.
"""
with self._mtls_lock:
if self._cached_cert != used_cert:
return True

try:
(
call_cert_bytes,
call_key_bytes,
cached_fp,
current_fp,
) = _mtls_helper.check_parameters_for_unauthorized_response(

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.

_handle_mtls_unauthorized_response calls _mtls_helper.check_parameters_for_unauthorized_response(self._cached_cert) on every 401 response from an mTLS endpoint. That helper invokes call_client_cert_callback(), which forks the SecureConnect cert-provider subprocess (10 to 92 ms) while holding _mtls_lock, even when the certificate has not changed and the 401 was caused by an expired or invalid token. The same behavior occurs in urllib3.py at line 237.

self._cached_cert
)
if cached_fp == current_fp:
return False

_LOGGER.info(
"Client certificate has changed, reconfiguring mTLS adapter."
)
self._configure_mtls_if_needed(
url,
force_reconfigure=True,
client_cert_callback=lambda: (call_cert_bytes, call_key_bytes),

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.

During 401 rotation, check_parameters_for_unauthorized_response calls call_client_cert_callback(), which generates an encrypted private key and passphrase for SecureConnect but discards the passphrase into _. Passing that encrypted key to ctx.load_cert_chain with password=None causes OpenSSL to prompt on /dev/tty and hang headless processes, or fail with OSError. Propagate the passphrase from call_client_cert_callback or call _mtls_helper.get_client_cert_and_key() directly when reconfiguring.

)
except Exception as exc:
_LOGGER.debug(
"Failed to reconfigure mTLS adapter on 401 response: %s",
exc,
)
return False

return self._cached_cert != used_cert

def _should_retry_closed_pool(self, caught_exc, used_cert):
"""Checks whether a ConnectionError should be retried on the new mTLS adapter.

Args:
caught_exc (requests.exceptions.ConnectionError): The exception
raised during the request.
used_cert (Optional[bytes]): The client certificate bytes used for
the failed request.

Returns:
bool: True if the error wraps a ClosedPoolError and a concurrent
thread reconfigured the mTLS adapter with a new certificate
while this request was in flight.
"""
return (
used_cert is not None
and self._cached_cert != used_cert
and bool(caught_exc.args)
and isinstance(caught_exc.args[0], urllib3.exceptions.ClosedPoolError)
)

def __call__(
self,
url,
method="GET",
body=None,
headers=None,
timeout=_DEFAULT_TIMEOUT,
**kwargs
**kwargs,
):
"""Make an HTTP request using requests.

Expand All @@ -184,18 +352,49 @@ def __call__(
google.auth.exceptions.TransportError: If any exception occurred.
"""
try:
_helpers.request_log(_LOGGER, method, url, body, headers)
response = self.session.request(
method, url, data=body, headers=headers, timeout=timeout, **kwargs
self._configure_mtls_if_needed(url)

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.

self._configure_mtls_if_needed(url) can raise ClientCertError, MutualTLSChannelError, or a raw OSError/FileNotFoundError. The same issue appears at urllib3.py line 303. The only handler on this try block catches requests.exceptions.RequestException, so those exceptions escape untranslated even though the docstring promises TransportError. Because ClientCertError and MutualTLSChannelError are sibling subclasses of GoogleAuthError rather than subclasses of TransportError, google.api_core.retry.retry_base.if_transient_error does not match them and default retry stops covering token refresh. This triggers when GOOGLE_API_USE_CLIENT_CERTIFICATE is unset and certificate_config.json points to a missing or unreadable cert_path. On main that request returns 200, whereas this branch raises FileNotFoundError. Catching (exceptions.GoogleAuthError, OSError) and raising exceptions.TransportError from it will preserve the documented contract.

# Snapshot the active cert before the network call in case another
# thread reconfigures mTLS mid-flight.
used_cert = (
self._cached_cert
if self.session.get_adapter(url) is self._mtls_adapter

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.

Passing a bytes URL such as b"https://oauth2.googleapis.com/token" to Request.__call__ now raises TypeError: can't concat str to bytes inside self.session.get_adapter(url). Previously requests.Session.request normalized bytes URLs via builtin_str(url) before calling get_adapter. Because get_adapter(url) runs unconditionally on every call, this breaks bytes URLs even when mTLS is disabled.

else None
)
Comment on lines +358 to 362

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.

high

When url has no trailing slash (e.g., https://pubsub.mtls.googleapis.com), self.session.get_adapter(url) will return the default https:// adapter instead of the mounted _mtls_adapter (which is mounted on the prefix with a trailing slash). However, requests itself will prepare the URL and append a trailing slash, meaning the request will actually use the mTLS adapter. This mismatch causes used_cert to be evaluated as None, which prevents certificate rotation and 401 retries from working for URLs without a trailing slash. Normalize the URL using the same logic as _configure_mtls_if_needed before calling get_adapter. Additionally, ensure robust type handling when parsing the URL (which may be a urllib3.util.Url object, bytes, or string) by checking for a .url attribute, safely decoding bytes to UTF-8, and falling back to string conversion.

            url_str = url.url if hasattr(url, "url") else url
            if isinstance(url_str, bytes):
                url_str = url_str.decode("utf-8")
            else:
                url_str = str(url_str)
            parsed = urllib_parse.urlparse(url_str)
            adapter_url = url_str if parsed.path else f"{parsed.scheme}://{parsed.netloc}/"
            used_cert = (
                self._cached_cert
                if self.session.get_adapter(adapter_url) is self._mtls_adapter
                else None
            )
References
  1. When parsing or validating URLs that may be passed as urllib3.util.Url objects, bytes, or strings, ensure robust type handling by checking for a .url attribute, safely decoding bytes to UTF-8, and falling back to string conversion to prevent TypeErrors during parsing.

_helpers.request_log(_LOGGER, method, url, body, headers)
try:
response = self.session.request(
method, url, data=body, headers=headers, timeout=timeout, **kwargs
)
except requests.exceptions.ConnectionError as caught_exc:
if not self._should_retry_closed_pool(caught_exc, used_cert):
raise
used_cert = self._cached_cert
_helpers.request_log(_LOGGER, method, url, body, headers)
response = self.session.request(
method, url, data=body, headers=headers, timeout=timeout, **kwargs
)
_helpers.response_log(_LOGGER, response)

if (
response.status_code == http_client.UNAUTHORIZED
and used_cert is not None
and _mtls_helper.is_mtls_endpoint(url)
and self._handle_mtls_unauthorized_response(url, used_cert)

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.

nit/opt: I find the current structure here to sort of "bury" the side-effect that this has which on the surface here presents as just another condition to be met before taking some action (but it internally does things too - e.g. updates the pool manager) - I wonder if there may be small tweaks we could make that would make this side effect more apparent.

):
_helpers.request_log(_LOGGER, method, url, body, headers)
response = self.session.request(

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.

Retrying on a 401 response overwrites response without closing it, which leaks the connection when stream=True or preload_content=False. Also, if body is a generator or file stream, the first request consumes it and the retry sends an empty body. Call response.close() in requests.py and response.release_conn() in urllib3.py before retrying, and skip the retry if body is an unrewindable stream.

method, url, data=body, headers=headers, timeout=timeout, **kwargs
)
_helpers.response_log(_LOGGER, response)

return _Response(response)
except requests.exceptions.RequestException as caught_exc:
new_exc = exceptions.TransportError(caught_exc)
raise new_exc from caught_exc


class _MutualTlsAdapter(requests.adapters.HTTPAdapter):
_is_mtls = True

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.

Placing _is_mtls = True above the docstring in _MutualTlsAdapter and _MutualTlsOffloadAdapter at line 464 turns the string literal into an unassigned expression statement and sets __doc__ to None on both classes. Moving _is_mtls = True below the docstring restores __doc__.

"""
A TransportAdapter that enables mutual TLS.

Expand Down Expand Up @@ -262,6 +461,7 @@ def proxy_manager_for(self, *args, **kwargs):


class _MutualTlsOffloadAdapter(requests.adapters.HTTPAdapter):
_is_mtls = True
"""
A TransportAdapter that enables mutual TLS and offloads the client side
signing operation to the signing library.
Expand Down Expand Up @@ -570,7 +770,7 @@ def request(
headers=None,
max_allowed_time=None,
timeout=_DEFAULT_TIMEOUT,
**kwargs
**kwargs,
):
"""Implementation of Requests' request.

Expand Down Expand Up @@ -632,7 +832,7 @@ def request(
data=data,
headers=request_headers,
timeout=timeout,
**kwargs
**kwargs,
)
remaining_time = guard.remaining_timeout

Expand Down Expand Up @@ -705,7 +905,7 @@ def request(
max_allowed_time=remaining_time,
timeout=timeout,
_credential_refresh_attempt=_credential_refresh_attempt + 1,
**kwargs
**kwargs,
)

return response
Expand Down
Loading
Loading