fix(auth): add lazy mTLS configuration and certificate rotation to requests and urllib3 Request transports

[nbayati]

Token refreshes and service account impersonation flows that call mtls endpoints directly through google.auth.transport.requests.Request or google.auth.transport.urllib3.Request previously failed because only AuthorizedSession and AuthorizedHttp configured client certificates.

• Lazily configure _MutualTlsAdapter per host prefix in requests.Request and a dedicated mTLS PoolManager in urllib3.Request when an .mtls. URL is requested, while deferring if the underlying session or pool already has mTLS configured.

• Preserve existing adapter and pool configuration (max_retries, pool_connections, pool_maxsize, pool_block, timeout, headers, num_pools) when creating mTLS adapters and pool managers.

• Reconfigure the mTLS adapter or pool manager and retry once on 401 Unauthorized if the client certificate on disk has rotated.

• Retry once on ClosedPoolError if a concurrent thread reconfigured the mTLS adapter or pool manager and closed the previous pool while a request was in flight.

related bug: b/555142182

[gemini-code-assist]

Code Review

This pull request implements thread-safe, lazy mutual TLS (mTLS) adapter configuration and automatic client certificate rotation/retry mechanisms on 401 Unauthorized responses or closed connection pools for both requests and urllib3 transports. Feedback on these changes highlights three key issues: a typo referencing the non-existent DEFAULT_POOLBLOCK instead of DEFAULT_POOL_BLOCK in requests.adapters, a mismatch in URL matching for get_adapter when URLs lack a trailing slash (which disables certificate rotation), and a compatibility issue with urllib3 v2.x where maxsize should be checked alongside _maxsize.

[gemini-code-assist]

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,
	)

[gemini-code-assist]

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.

[gemini-code-assist]

In urllib3 v2.x, the RecentlyUsedContainer class uses the public attribute maxsize instead of the private _maxsize attribute used in urllib3 v1.x. To ensure compatibility across both versions of urllib3, check for both attributes.

Suggested change

	if hasattr(getattr(self.http, "pools", None), "_maxsize"):
	kwargs["num_pools"] = self.http.pools._maxsize
	pools = getattr(self.http, "pools", None)
	if pools is not None:
	if hasattr(pools, "maxsize"):
	kwargs["num_pools"] = pools.maxsize
	elif hasattr(pools, "_maxsize"):
	kwargs["num_pools"] = pools._maxsize

[lsirac]

If another thread calls close() while a request is running, self._mtls_http and self._cached_cert become None. self._cached_cert != used_cert then evaluates None != used_cert as True, and the retry at line 319 or 331 crashes with AttributeError when calling self._mtls_http.request(). Check that self._cached_cert and self._mtls_http are not None in _should_retry_closed_pool and _handle_mtls_unauthorized_response.

[lsirac]

old_mtls_http.clear() only empties the RecentlyUsedContainer dictionary and never closes the underlying HTTPConnectionPool instances because dispose_func defaults to None. Active sockets stay open, ClosedPoolError is never raised, and _should_retry_closed_pool becomes dead code. Set http.pools.dispose_func = lambda pool: pool.close() when creating the pool manager, or iterate over old_mtls_http.pools.values() and close each pool directly.

[lsirac]

_get_http_for_url copies only retries, maxsize, block, and timeout, dropping custom CA settings like ca_certs and ca_cert_dir in favor of certifi.where(). Also, if self.http is a urllib3.ProxyManager, _make_mutual_tls_http creates a standard PoolManager and bypasses the proxy. Propagate ca_certs and ca_cert_dir to ctx.load_verify_locations, and instantiate ProxyManager when self.http is a proxy manager.

[lsirac]

In addition to dropping ProxyManager and custom CA settings, _get_http_for_url completely bypasses a caller-injected custom PoolManager or mock transport on self.http, causing requests to .mtls.googleapis.com to open live network connections to oauth2.mtls.googleapis.com:443 while the injected mock receives zero calls.

[lsirac]

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.

[lsirac]

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.

[lsirac]

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.

[lsirac]

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.

[lsirac]

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.

[lsirac]

http_client_pool = self._get_http_for_url(url) binds the pool to a local variable and releases self._mtls_lock before line 307 checks http_client_pool is self._mtls_http. If another thread rotates the certificate in between, self._mtls_http points to the new pool while http_client_pool still holds the old pool, setting used_cert to None and skipping 401 retry when the request fails on the old pool. Return http_client_pool and used_cert together from _get_http_for_url while holding the lock.

[lsirac]

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.

[lsirac]

The caller's timeout parameter no longer bounds Request.__call__. _run_cert_provider_command runs subprocess.Popen(...).communicate() without a timeout while _mtls_lock is held, so a hung SecureConnect helper blocks the caller and every other thread waiting on _mtls_lock indefinitely. Should the certificate provider command receive a timeout derived from the request timeout?

[lsirac]

check_use_client_cert() runs before the cached fast path at line 177. When GOOGLE_API_USE_CLIENT_CERTIFICATE is unset, every request to an mTLS endpoint calls os.path.exists(), open(), and json.load() on certificate_config.json even after _mtls_http is already built and cached. There is also no negative caching when get_client_cert_and_key() returns (None, None), so hosts without a client cert repeat full discovery under _mtls_lock on every call. The same ordering appears at requests.py line 185.

[lsirac]

Certificate rotation failure is logged at debug level while rotation success at line 243 is logged at info level. Operators monitoring production logs at INFO or WARNING will see successful rotations but miss failures that cause repeated 401 responses. For comparison, AuthorizedHttp.urlopen at line 643 logs mTLS reconfiguration failures with _LOGGER.error and re-raises.

[lsirac]

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.

[lsirac]

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__.

[lsirac]

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.

[lsirac]

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.

[lsirac]

_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.

[lsirac]

In addition to dropping ProxyManager and custom CA settings, _get_http_for_url completely bypasses a caller-injected custom PoolManager or mock transport on self.http, causing requests to .mtls.googleapis.com to open live network connections to oauth2.mtls.googleapis.com:443 while the injected mock receives zero calls.

[macastelaz]

I think you are already on top of this, but just flagging it here for completeness as well that we'll still want comprehensive unit test coverage for these changes.

[macastelaz]

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.

[macastelaz]

Do we need to worry about case-sensitivity here?