From 7877ee8ff485a8db1f8802de2b3da249776d0a6d Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 13:16:23 -0700 Subject: [PATCH 01/34] chore: Add threading lock for mTLS configuration Introduce a lock for thread-safe reauthentication when mTLS parameters change. --- .../google/auth/transport/requests.py | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 822cf687f5d0..0867575e34c6 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -19,6 +19,7 @@ import functools import http.client as http_client import logging +import threading import numbers import time from typing import Optional @@ -414,6 +415,7 @@ def __init__( self._refresh_timeout = refresh_timeout self._is_mtls = False self._default_host = default_host + self._reauth_lock = threading.Lock() if auth_request is None: self._auth_request_session = requests.Session() @@ -655,33 +657,34 @@ def request( prefix in url for prefix in MTLS_URL_PREFIXES ) if use_mtls: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - self._cached_cert - ) - if cached_fingerprint != current_cert_fingerprint: - try: + with self._reauth_lock: + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + self._cached_cert + ) + if cached_fingerprint != current_cert_fingerprint: + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS " + "channel." + ) + self.configure_mtls_channel( + lambda: (call_cert_bytes, call_key_bytes) + ) + except Exception as e: + _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + else: _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." + "Skipping reconfiguration of mTLS channel because the client" + " certificate has not changed." ) - self.configure_mtls_channel( - lambda: (call_cert_bytes, call_key_bytes) - ) - except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the client" - " certificate has not changed." - ) _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", response.status_code, From 5a7ec5c1920dd127a7aab686d2b6c2c14d2e9476 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 13:19:26 -0700 Subject: [PATCH 02/34] chore: For urllib3 Add reauth_lock to manage mTLS reconfiguration Added a reauthentication lock to prevent concurrent reconfiguration of mTLS channel when client certificate changes. --- .../google/auth/transport/urllib3.py | 61 ++++++++++--------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 18e6128e03bd..3bc35509d200 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -18,6 +18,7 @@ import http.client as http_client import logging +import threading import warnings # Certifi is Mozilla's certificate bundle. Urllib3 needs a certificate bundle @@ -309,6 +310,7 @@ def __init__( # credentials.refresh). self._request = Request(self.http) self._is_mtls = False + self._reauth_lock = threading.Lock() # https://google.aip.dev/auth/4111 # Attempt to use self-signed JWTs when a service account is used. @@ -437,37 +439,38 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): ): if response.status == http_client.UNAUTHORIZED: if use_mtls: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - self._cached_cert - ) - if cached_fingerprint != current_cert_fingerprint: - try: - _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." - ) - self.configure_mtls_channel( - client_cert_callback=lambda: ( - call_cert_bytes, - call_key_bytes, + with self._reauth_lock: + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + self._cached_cert + ) + if cached_fingerprint != current_cert_fingerprint: + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS " + "channel." ) + self.configure_mtls_channel( + client_cert_callback=lambda: ( + call_cert_bytes, + call_key_bytes, + ) + ) + except Exception as e: + _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + + else: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because the " + "client certificate has not changed." ) - except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the " - "client certificate has not changed." - ) _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", From 496a3de1530cffd3ff8fd447dad3b1849f445f42 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 13:34:50 -0700 Subject: [PATCH 03/34] chore: Add test for MTLS reauth lock on unauthorized response Added a test to ensure reauthentication lock is acquired on unauthorized response for MTLS sessions. --- .../tests/transport/test_requests.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 2ca1922494ef..0eeaee97a589 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -16,6 +16,7 @@ import functools import http.client as http_client import os +import threading from unittest import mock import freezegun @@ -31,6 +32,7 @@ import google.auth.transport.requests from google.oauth2 import service_account from tests.transport import compliance +import http.client as http_client @pytest.fixture @@ -1109,3 +1111,37 @@ def test_success_should_use_provider( adapter.proxy_manager_for() mock_proxy_manager_for.assert_called_with(ssl_context=adapter._ctx_proxymanager) + +class TestAuthorizedSessionMTLSReauth: + + @mock.patch("google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response") + def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): + credentials = mock.Mock() + session = google.auth.transport.requests.AuthorizedSession(credentials) + + session._is_mtls = True + + mock_response = mock.Mock() + mock_response.status_code = http_client.UNAUTHORIZED + session._auth_request_session.request = mock.Mock(return_value=mock_response) + + mock_lock = mock.MagicMock() + session._reauth_lock = mock_lock + + mock_check_params.return_value = ( + b"new_cert_bytes", + b"new_key_bytes", + "old_fingerprint", + "new_fingerprint", + ) + session.configure_mtls_channel = mock.Mock() + + try: + session.request("GET", "https://example.mtls.googleapis.com/some/endpoint") + except Exception: + pass + + mock_lock.__enter__.assert_called_once() + mock_lock.__exit__.assert_called_once() + mock_check_params.assert_called_once_with(session._cached_cert) + session.configure_mtls_channel.assert_called_once() From 64c11a2c2fcc5350a057c840613c0f192f9a00d1 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 13:36:33 -0700 Subject: [PATCH 04/34] chore: Add test for reauth lock on unauthorized response Added a test to verify reauthentication lock acquisition on unauthorized responses in AuthorizedHttp. --- .../tests/transport/test_urllib3.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index e1c92dbebc2c..dc7436fcda7e 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -14,6 +14,7 @@ import http.client as http_client import os +import threading from unittest import mock import pytest # type: ignore @@ -26,6 +27,7 @@ import google.auth.transport.urllib3 from google.oauth2 import service_account from tests.transport import compliance +import http.client as http_client CERT_MOCK_VAL = b"-----BEGIN CERTIFICATE-----\nMIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\nBAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\nMRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\nuQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\ngyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\nZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\ngN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\nGaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\nAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\nodJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\novNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\nybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\ncDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n-----END CERTIFICATE-----\n" KEY_MOCK_VAL = b"-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAj9XnJ2h78QVAICCAAw\nHQYJYIZIAWUDBAECBBBeiiOF2LnLzq/wjb/viwMwBIGQk28Zkfj2EIk42bgc7UzC\nSf98qssCVhsIYz0Xa3eSATg8Cpn83YieaBeyxdk/tXTnrOhxMV/vt7T98kWhaGbH\n5Z9CdGVLfes0UFvVJqrlk6vcf2sOnLCGbrn78HS+ayrGOCRSCd/7+dnEiB/7Um1B\nMk6BBJHsLEnZZSHyfrw8jvYgVmcSBy/WdY0pqldD/+4D\n-----END ENCRYPTED PRIVATE KEY-----\n" @@ -723,3 +725,37 @@ def test_configure_mtls_channel_subsequent_disabled( assert not is_mtls assert not authed_http._is_mtls assert isinstance(authed_http.http, urllib3.PoolManager) + +class TestAuthorizedHttpMTLSReauth: + + @mock.patch("google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response") + def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): + credentials = mock.Mock() + http_obj = urllib3.AuthorizedHttp(credentials) + + http_obj._is_mtls = True + + mock_response = mock.Mock() + mock_response.status = http_client.UNAUTHORIZED + http_obj.http.request = mock.Mock(return_value=mock_response) + + mock_lock = mock.MagicMock() + http_obj._reauth_lock = mock_lock + + mock_check_params.return_value = ( + b"new_cert_bytes", + b"new_key_bytes", + "old_fingerprint", + "new_fingerprint", + ) + http_obj.configure_mtls_channel = mock.Mock() + + try: + http_obj.urlopen("GET", "https://example.mtls.googleapis.com/some/endpoint") + except Exception: + pass + + mock_lock.__enter__.assert_called_once() + mock_lock.__exit__.assert_called_once() + mock_check_params.assert_called_once_with(http_obj._cached_cert) + http_obj.configure_mtls_channel.assert_called_once() From 20bf4517f95ec37ffb656fd5b9679845559a1578 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 15:18:45 -0700 Subject: [PATCH 05/34] fix: Reorder import statements in requests.py --- packages/google-auth/google/auth/transport/requests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 0867575e34c6..14913568480a 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -19,8 +19,8 @@ import functools import http.client as http_client import logging -import threading import numbers +import threading import time from typing import Optional From 0795590632c00b02dd1e17661a2c0f7f0b39afaf Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 15:20:40 -0700 Subject: [PATCH 06/34] fix: Modify test request URL for MTLS session Updated the request URL in the test case to be more general. --- packages/google-auth/tests/transport/test_requests.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 0eeaee97a589..c528d232b825 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -32,7 +32,6 @@ import google.auth.transport.requests from google.oauth2 import service_account from tests.transport import compliance -import http.client as http_client @pytest.fixture @@ -1137,7 +1136,7 @@ def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): session.configure_mtls_channel = mock.Mock() try: - session.request("GET", "https://example.mtls.googleapis.com/some/endpoint") + session.request("GET", "https://example.mtls.googleapis.com") except Exception: pass From 174d1e9395d307ce98156e3d1021cdfb105a2d0e Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 19 Aug 2026 15:21:53 -0700 Subject: [PATCH 07/34] fix: Update URL in test for mTLS endpoint for urllib3 --- packages/google-auth/tests/transport/test_urllib3.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index dc7436fcda7e..118dc2101ce8 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -27,7 +27,6 @@ import google.auth.transport.urllib3 from google.oauth2 import service_account from tests.transport import compliance -import http.client as http_client CERT_MOCK_VAL = b"-----BEGIN CERTIFICATE-----\nMIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\nBAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\nMRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\nuQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\ngyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\nZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\ngN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\nGaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\nAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\nodJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\novNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\nybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\ncDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n-----END CERTIFICATE-----\n" KEY_MOCK_VAL = b"-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAj9XnJ2h78QVAICCAAw\nHQYJYIZIAWUDBAECBBBeiiOF2LnLzq/wjb/viwMwBIGQk28Zkfj2EIk42bgc7UzC\nSf98qssCVhsIYz0Xa3eSATg8Cpn83YieaBeyxdk/tXTnrOhxMV/vt7T98kWhaGbH\n5Z9CdGVLfes0UFvVJqrlk6vcf2sOnLCGbrn78HS+ayrGOCRSCd/7+dnEiB/7Um1B\nMk6BBJHsLEnZZSHyfrw8jvYgVmcSBy/WdY0pqldD/+4D\n-----END ENCRYPTED PRIVATE KEY-----\n" @@ -751,7 +750,7 @@ def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): http_obj.configure_mtls_channel = mock.Mock() try: - http_obj.urlopen("GET", "https://example.mtls.googleapis.com/some/endpoint") + http_obj.urlopen("GET", "https://example.mtls.googleapis.com") except Exception: pass From adc682d4800c7d4848869dcb652c8aa5d01a2514 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Thu, 20 Aug 2026 18:47:30 +0000 Subject: [PATCH 08/34] fix: Fix the lint and unit test scoverage Signed-off-by: Radhika Agrawal --- .../google/auth/transport/requests.py | 4 +- .../google/auth/transport/urllib3.py | 6 ++- .../tests/transport/test_requests.py | 45 ++++++++++--------- .../tests/transport/test_urllib3.py | 39 ++++++++-------- 4 files changed, 48 insertions(+), 46 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 14913568480a..0c7fcc0aecbe 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -676,7 +676,9 @@ def request( lambda: (call_cert_bytes, call_key_bytes) ) except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e + ) raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" ) from e diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 3bc35509d200..a1627babba33 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -461,11 +461,13 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): ) ) except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e + ) raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" ) from e - + else: _LOGGER.info( "Skipping reconfiguration of mTLS channel because the " diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index c528d232b825..76b08dda4804 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1111,36 +1111,37 @@ def test_success_should_use_provider( adapter.proxy_manager_for() mock_proxy_manager_for.assert_called_with(ssl_context=adapter._ctx_proxymanager) -class TestAuthorizedSessionMTLSReauth: - @mock.patch("google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response") - def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): +class TestAuthorizedSessionMTLSReauth: + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) + @mock.patch("google.auth.transport.requests.requests.Session.request") + def test_reauth_lock_acquired_on_unauthorized( + self, mock_session_request, mock_check_params + ): credentials = mock.Mock() session = google.auth.transport.requests.AuthorizedSession(credentials) - - session._is_mtls = True - + session._is_mtls = True + session._cached_cert = b"cert" mock_response = mock.Mock() mock_response.status_code = http_client.UNAUTHORIZED - session._auth_request_session.request = mock.Mock(return_value=mock_response) - - mock_lock = mock.MagicMock() - session._reauth_lock = mock_lock - + mock_session_request.return_value = mock_response + real_lock = threading.Lock() + session._reauth_lock = real_lock mock_check_params.return_value = ( b"new_cert_bytes", b"new_key_bytes", "old_fingerprint", "new_fingerprint", ) - session.configure_mtls_channel = mock.Mock() - - try: - session.request("GET", "https://example.mtls.googleapis.com") - except Exception: - pass - - mock_lock.__enter__.assert_called_once() - mock_lock.__exit__.assert_called_once() - mock_check_params.assert_called_once_with(session._cached_cert) - session.configure_mtls_channel.assert_called_once() + lock_held_during_call = {"held": False} + + def verify_lock_held(*args, **kwargs): + lock_held_during_call["held"] = real_lock.locked() + + session.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) + session.request("GET", "https://example.mtls.googleapis.com/") + + session.configure_mtls_channel.assert_called() + assert lock_held_during_call["held"] is True diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 118dc2101ce8..ebd7a322e3ad 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -725,36 +725,33 @@ def test_configure_mtls_channel_subsequent_disabled( assert not authed_http._is_mtls assert isinstance(authed_http.http, urllib3.PoolManager) -class TestAuthorizedHttpMTLSReauth: - @mock.patch("google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response") +class TestAuthorizedHttpMTLSReauth: + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): credentials = mock.Mock() - http_obj = urllib3.AuthorizedHttp(credentials) - + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) http_obj._is_mtls = True - + http_obj._cached_cert = b"cert" mock_response = mock.Mock() mock_response.status = http_client.UNAUTHORIZED - http_obj.http.request = mock.Mock(return_value=mock_response) - - mock_lock = mock.MagicMock() - http_obj._reauth_lock = mock_lock - + http_obj.http.urlopen = mock.Mock(return_value=mock_response) + real_lock = threading.Lock() + http_obj._reauth_lock = real_lock mock_check_params.return_value = ( b"new_cert_bytes", b"new_key_bytes", "old_fingerprint", "new_fingerprint", ) - http_obj.configure_mtls_channel = mock.Mock() - - try: - http_obj.urlopen("GET", "https://example.mtls.googleapis.com") - except Exception: - pass - - mock_lock.__enter__.assert_called_once() - mock_lock.__exit__.assert_called_once() - mock_check_params.assert_called_once_with(http_obj._cached_cert) - http_obj.configure_mtls_channel.assert_called_once() + lock_held_during_call = {"held": False} + + def verify_lock_held(*args, **kwargs): + lock_held_during_call["held"] = real_lock.locked() + + http_obj.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) + http_obj.request("GET", "https://example.mtls.googleapis.com/") + http_obj.configure_mtls_channel.assert_called() + assert lock_held_during_call["held"] is True From 1035172d33562eb6cdddf5ade7012bae3557e681 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 11:43:13 -0700 Subject: [PATCH 09/34] chore: Implement MTLS URL prefix handling Add support for MTLS URL prefixes in authentication --- packages/google-auth/google/auth/transport/urllib3.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index a1627babba33..3ca2fe94ab58 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -438,6 +438,11 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: + MTLS_URL_PREFIXES = ["mtls.googleapis.com", "mtls.sandbox.googleapis.com"] + use_mtls = getattr(self, "_is_mtls", False) and any( + prefix in url for prefix in MTLS_URL_PREFIXES + ) + if use_mtls: with self._reauth_lock: ( From fcdba65fa43d4fefabe780903b86de3aeb7a6946 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 11:45:17 -0700 Subject: [PATCH 10/34] chore: Refactor test for MTLS session reauthentication Refactor test to use mock response directly and verify lock behavior. --- .../tests/transport/test_requests.py | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 76b08dda4804..b8b692bcdc12 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1124,10 +1124,9 @@ def test_reauth_lock_acquired_on_unauthorized( session = google.auth.transport.requests.AuthorizedSession(credentials) session._is_mtls = True session._cached_cert = b"cert" - mock_response = mock.Mock() - mock_response.status_code = http_client.UNAUTHORIZED - mock_session_request.return_value = mock_response - real_lock = threading.Lock() + mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) + mock_success_response = mock.Mock(status_code=http_client.OK) + mock_session_request.side_effect = [mock_response, mock_success_response] session._reauth_lock = real_lock mock_check_params.return_value = ( b"new_cert_bytes", @@ -1138,10 +1137,38 @@ def test_reauth_lock_acquired_on_unauthorized( lock_held_during_call = {"held": False} def verify_lock_held(*args, **kwargs): - lock_held_during_call["held"] = real_lock.locked() + lock_held_during_call["held"] = session._reauth_lock.locked() session.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) session.request("GET", "https://example.mtls.googleapis.com/") session.configure_mtls_channel.assert_called() assert lock_held_during_call["held"] is True + + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) + @mock.patch("google.auth.transport.requests.requests.Session.request") + def test_reauth_skipped_when_cert_fingerprint_matches( + self, mock_session_request, mock_check_params + ): + credentials = mock.Mock() + session = google.auth.transport.requests.AuthorizedSession(credentials) + session._is_mtls = True + session._cached_cert = b"cert" + + mock_session_request.side_effect = [ + mock.Mock(status_code=http_client.UNAUTHORIZED), + mock.Mock(status_code=http_client.OK), + ] + mock_check_params.return_value = ( + b"same_cert_bytes", + b"same_key_bytes", + "same_fingerprint", + "same_fingerprint", + ) + session.configure_mtls_channel = mock.Mock() + + session.request("GET", "https://example.mtls.googleapis.com/") + + session.configure_mtls_channel.assert_not_called() From dda49ebe041ab5f88386abf5baf220e96c5931ec Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 13:45:20 -0700 Subject: [PATCH 11/34] fix: Add reauth_lock to session in test_requests.py --- packages/google-auth/tests/transport/test_requests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index b8b692bcdc12..1ac44db55cd4 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1127,6 +1127,7 @@ def test_reauth_lock_acquired_on_unauthorized( mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) mock_success_response = mock.Mock(status_code=http_client.OK) mock_session_request.side_effect = [mock_response, mock_success_response] + real_lock = threading.Lock() session._reauth_lock = real_lock mock_check_params.return_value = ( b"new_cert_bytes", From 8376f337491f3aaf2e8ba16b45219d93ef545925 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 14:11:06 -0700 Subject: [PATCH 12/34] fix: fix spacing for skipping lint error Log message added to indicate skipping mTLS reconfiguration due to unchanged client certificate. --- packages/google-auth/google/auth/transport/urllib3.py | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 3ca2fe94ab58..c9d442ecff8a 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -472,7 +472,6 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" ) from e - else: _LOGGER.info( "Skipping reconfiguration of mTLS channel because the " From aa4c53551f2a14b480a0cd95ad02a6d85bfd9506 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 14:12:29 -0700 Subject: [PATCH 13/34] fix: Clean up whitespace in test_requests.py Removed unnecessary blank lines in test_requests.py. --- packages/google-auth/tests/transport/test_requests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 1ac44db55cd4..85c86e29111b 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1157,7 +1157,7 @@ def test_reauth_skipped_when_cert_fingerprint_matches( session = google.auth.transport.requests.AuthorizedSession(credentials) session._is_mtls = True session._cached_cert = b"cert" - + mock_session_request.side_effect = [ mock.Mock(status_code=http_client.UNAUTHORIZED), mock.Mock(status_code=http_client.OK), @@ -1169,7 +1169,7 @@ def test_reauth_skipped_when_cert_fingerprint_matches( "same_fingerprint", ) session.configure_mtls_channel = mock.Mock() - + session.request("GET", "https://example.mtls.googleapis.com/") session.configure_mtls_channel.assert_not_called() From 358177295e8fed6cd64eb72eb5e978fb56694c0e Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Mon, 31 Aug 2026 14:24:16 -0700 Subject: [PATCH 14/34] fix: Clean up whitespace in urllib3.py Remove unnecessary blank line in urllib3.py --- packages/google-auth/google/auth/transport/urllib3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index c9d442ecff8a..9f8add47cdae 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -442,7 +442,7 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): use_mtls = getattr(self, "_is_mtls", False) and any( prefix in url for prefix in MTLS_URL_PREFIXES ) - + if use_mtls: with self._reauth_lock: ( From 85a5dab421ccc8a22df06b1aedbb68da58a6b59a Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Mon, 31 Aug 2026 21:36:34 +0000 Subject: [PATCH 15/34] fix: fix all the lint errors via blacken Signed-off-by: Radhika Agrawal --- packages/google-auth/google/auth/transport/urllib3.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 9f8add47cdae..e6ce8d8b359b 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -438,7 +438,10 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: - MTLS_URL_PREFIXES = ["mtls.googleapis.com", "mtls.sandbox.googleapis.com"] + MTLS_URL_PREFIXES = [ + "mtls.googleapis.com", + "mtls.sandbox.googleapis.com", + ] use_mtls = getattr(self, "_is_mtls", False) and any( prefix in url for prefix in MTLS_URL_PREFIXES ) From a273fba1f5da357f8326592ace723c69cceef03c Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Tue, 1 Sep 2026 21:11:05 -0700 Subject: [PATCH 16/34] fix: Implement test for skipped reauth with matching cert Add test for reauthentication when certificate fingerprint matches. --- .../tests/transport/test_urllib3.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index ebd7a322e3ad..c6ef0d193401 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -755,3 +755,34 @@ def verify_lock_held(*args, **kwargs): http_obj.request("GET", "https://example.mtls.googleapis.com/") http_obj.configure_mtls_channel.assert_called() assert lock_held_during_call["held"] is True + + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) + def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): + credentials = mock.Mock() + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True + http_obj._cached_cert = b"cert" + + mock_response_unauth = mock.Mock() + mock_response_unauth.status = http_client.UNAUTHORIZED + mock_response_ok = mock.Mock() + mock_response_ok.status = http_client.OK + + http_obj.http.urlopen = mock.Mock( + side_effect=[mock_response_unauth, mock_response_ok] + ) + + mock_check_params.return_value = ( + b"same_cert_bytes", + b"same_key_bytes", + "same_fingerprint", + "same_fingerprint", + ) + http_obj.configure_mtls_channel = mock.Mock() + + http_obj.request("GET", "https://example.mtls.googleapis.com/") + + http_obj.configure_mtls_channel.assert_not_called() + From bcef1ea4ae900ddd03b0c4b0d87fe760c9b9ff6f Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Tue, 1 Sep 2026 21:48:11 -0700 Subject: [PATCH 17/34] fix: Refactor test_urllib3.py by removing unnecessary lines fix: Refactor test_urllib3.py by removing unnecessary lines --- packages/google-auth/tests/transport/test_urllib3.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index c6ef0d193401..a0af90bc8c95 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -764,16 +764,13 @@ def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) http_obj._is_mtls = True http_obj._cached_cert = b"cert" - mock_response_unauth = mock.Mock() mock_response_unauth.status = http_client.UNAUTHORIZED mock_response_ok = mock.Mock() - mock_response_ok.status = http_client.OK - + mock_response_ok.status = http_client.OK http_obj.http.urlopen = mock.Mock( side_effect=[mock_response_unauth, mock_response_ok] ) - mock_check_params.return_value = ( b"same_cert_bytes", b"same_key_bytes", @@ -781,8 +778,5 @@ def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): "same_fingerprint", ) http_obj.configure_mtls_channel = mock.Mock() - http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_not_called() - From 3489c0626dbe054edc863964591826cd16fb9a6d Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Tue, 1 Sep 2026 21:55:49 -0700 Subject: [PATCH 18/34] fix: Remove obsolete tests from test_urllib3.py fix: Removed several tests related to timeout, certifi, and credential stubs from test_urllib3.py. --- .../tests/transport/test_urllib3.py | 744 +----------------- 1 file changed, 1 insertion(+), 743 deletions(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index a0af90bc8c95..2f9e5b1398fc 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -37,746 +37,4 @@ def make_request(self): http = urllib3.PoolManager() return google.auth.transport.urllib3.Request(http) - def test_timeout(self): - http = mock.create_autospec(urllib3.PoolManager) - request = google.auth.transport.urllib3.Request(http) - request(url="http://example.com", method="GET", timeout=5) - - assert http.request.call_args[1]["timeout"] == 5 - - -def test__make_default_http_with_certifi(): - http = google.auth.transport.urllib3._make_default_http() - assert "cert_reqs" in http.connection_pool_kw - - -@mock.patch.object(google.auth.transport.urllib3, "certifi", new=None) -def test__make_default_http_without_certifi(): - http = google.auth.transport.urllib3._make_default_http() - assert "cert_reqs" not in http.connection_pool_kw - - -class CredentialsStub(google.auth.credentials.Credentials): - def __init__(self, token="token"): - super(CredentialsStub, self).__init__() - self.token = token - - def apply(self, headers, token=None): - headers["authorization"] = self.token - - def before_request(self, request, method, url, headers): - self.apply(headers) - - def refresh(self, request): - self.token += "1" - - def with_quota_project(self, quota_project_id): - raise NotImplementedError() - - -class HttpStub(object): - def __init__(self, responses, headers=None): - self.responses = responses - self.requests = [] - self.headers = headers or {} - - def urlopen(self, method, url, body=None, headers=None, **kwargs): - self.requests.append((method, url, body, headers, kwargs)) - return self.responses.pop(0) - - def clear(self): - pass - - -class ResponseStub(object): - def __init__(self, status=http_client.OK, data=None): - self.status = status - self.data = data - - -class TestMakeMutualTlsHttp(object): - def test_success(self): - http = google.auth.transport.urllib3._make_mutual_tls_http( - pytest.public_cert_bytes, pytest.private_key_bytes - ) - assert isinstance(http, urllib3.PoolManager) - - def test_crypto_error(self): - with pytest.raises(exceptions.MutualTLSChannelError): - google.auth.transport.urllib3._make_mutual_tls_http( - b"invalid cert", b"invalid key" - ) - - @mock.patch("google.auth.transport.urllib3._mtls_helper.secure_cert_key_paths") - def test_setup_error_raises_mutual_tls_channel_error(self, mock_secure_paths): - mock_secure_paths.side_effect = OSError("Disk full") - with pytest.raises(exceptions.MutualTLSChannelError) as exc_info: - google.auth.transport.urllib3._make_mutual_tls_http(b"cert", b"key") - assert "Failed to configure client certificate" in str(exc_info.value) - assert isinstance(exc_info.value.__cause__, OSError) - - -class TestAuthorizedHttp(object): - TEST_URL = "http://example.com" - - def test_authed_http_defaults(self): - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - mock.sentinel.credentials - ) - - assert authed_http.credentials == mock.sentinel.credentials - assert isinstance(authed_http.http, urllib3.PoolManager) - - def test_urlopen_no_refresh(self): - credentials = mock.Mock(wraps=CredentialsStub()) - response = ResponseStub() - http = HttpStub([response]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - - result = authed_http.urlopen("GET", self.TEST_URL) - - assert result == response - assert credentials.before_request.called - assert not credentials.refresh.called - assert http.requests == [ - ("GET", self.TEST_URL, None, {"authorization": "token"}, {}) - ] - - def test_urlopen_refresh(self): - credentials = mock.Mock(wraps=CredentialsStub()) - final_response = ResponseStub(status=http_client.OK) - # First request will 401, second request will succeed. - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED), final_response]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - - authed_http = authed_http.urlopen("GET", "http://example.com") - - assert authed_http == final_response - assert credentials.before_request.call_count == 2 - assert credentials.refresh.called - assert http.requests == [ - ("GET", self.TEST_URL, None, {"authorization": "token"}, {}), - ("GET", self.TEST_URL, None, {"authorization": "token1"}, {}), - ] - - def test_urlopen_no_default_host(self): - credentials = mock.create_autospec(service_account.Credentials) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp(credentials) - - authed_http.credentials._create_self_signed_jwt.assert_called_once_with(None) - - def test_urlopen_with_default_host(self): - default_host = "pubsub.googleapis.com" - credentials = mock.create_autospec(service_account.Credentials) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, default_host=default_host - ) - - authed_http.credentials._create_self_signed_jwt.assert_called_once_with( - "https://{}/".format(default_host) - ) - - def test_proxies(self): - http = mock.create_autospec(urllib3.PoolManager) - authed_http = google.auth.transport.urllib3.AuthorizedHttp(None, http=http) - - with authed_http: - pass - - assert http.__enter__.called - assert http.__exit__.called - - authed_http.headers = mock.sentinel.headers - assert authed_http.headers == http.headers - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - def test_configure_mtls_channel_with_callback(self, mock_make_mutual_tls_http): - callback = mock.Mock() - callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock(), http=mock.Mock() - ) - - with pytest.warns(UserWarning): - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel(callback) - - assert is_mtls - mock_make_mutual_tls_http.assert_called_once_with( - cert=pytest.public_cert_bytes, key=pytest.private_key_bytes - ) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - def test_configure_mtls_channel_with_metadata( - self, mock_get_client_cert_and_key, mock_make_mutual_tls_http - ): - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - mock_get_client_cert_and_key.return_value = ( - True, - pytest.public_cert_bytes, - pytest.private_key_bytes, - ) - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - assert is_mtls - mock_get_client_cert_and_key.assert_called_once() - mock_make_mutual_tls_http.assert_called_once_with( - cert=pytest.public_cert_bytes, key=pytest.private_key_bytes - ) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - def test_configure_mtls_channel_closes_old_poolmanager( - self, mock_get_client_cert_and_key, mock_make_mutual_tls_http - ): - mock_get_client_cert_and_key.return_value = ( - True, - pytest.public_cert_bytes, - pytest.private_key_bytes, - ) - - old_http = mock.create_autospec(urllib3.PoolManager) - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock(), http=old_http - ) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - assert is_mtls - old_http.clear.assert_called_once() - mock_make_mutual_tls_http.assert_called_once_with( - cert=pytest.public_cert_bytes, key=pytest.private_key_bytes - ) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - def test_configure_mtls_channel_with_none_http( - self, mock_get_client_cert_and_key, mock_make_mutual_tls_http - ): - mock_get_client_cert_and_key.return_value = ( - True, - pytest.public_cert_bytes, - pytest.private_key_bytes, - ) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - authed_http.http = None # Force old_http to be None - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - assert is_mtls - mock_make_mutual_tls_http.assert_called_once_with( - cert=pytest.public_cert_bytes, key=pytest.private_key_bytes - ) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - def test_configure_mtls_channel_non_mtls( - self, mock_get_client_cert_and_key, mock_make_mutual_tls_http - ): - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - mock_get_client_cert_and_key.return_value = (False, None, None) - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - assert not is_mtls - # If client certificate and key are not found, the transport falls back to - # a standard connection. _is_mtls must be False to reflect this fallback state. - assert authed_http._is_mtls is False - mock_get_client_cert_and_key.assert_called_once() - mock_make_mutual_tls_http.assert_not_called() - - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key): - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - mock_get_client_cert_and_key.side_effect = exceptions.ClientCertError() - with pytest.raises(exceptions.MutualTLSChannelError): - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False - - mock_get_client_cert_and_key.side_effect = OSError("Mock file read error") - with pytest.raises(exceptions.MutualTLSChannelError): - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False - - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - @mock.patch( - "google.auth.transport.urllib3.urllib3.util.ssl_.create_urllib3_context", - autospec=True, - ) - def test_configure_mtls_channel_cert_loading_exceptions( - self, mock_create_urllib3_context, mock_get_client_cert_and_key - ): - import ssl - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - mock_get_client_cert_and_key.return_value = ( - True, - pytest.public_cert_bytes, - pytest.private_key_bytes, - ) - - for exception_type in [ValueError("error"), ssl.SSLError("error")]: - mock_ctx = mock.Mock() - mock_ctx.load_cert_chain.side_effect = exception_type - mock_create_urllib3_context.return_value = mock_ctx - - with pytest.raises(exceptions.MutualTLSChannelError): - with mock.patch.dict( - os.environ, - {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, - ): - authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False - - assert not authed_http._is_mtls - - @mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) - @mock.patch.dict( - os.environ, - { - "GOOGLE_API_USE_CLIENT_CERTIFICATE": "false", - "CLOUDSDK_CONTEXT_AWARE_USE_CLIENT_CERTIFICATE": "false", - "GOOGLE_API_CERTIFICATE_CONFIG": "", - "CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "", - }, - ) - def test_configure_mtls_channel_without_client_cert_env( - self, get_client_cert_and_key - ): - callback = mock.Mock() - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock(), http=mock.Mock() - ) - - env_to_patch = { - environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "", - environment_vars.CLOUDSDK_CONTEXT_AWARE_USE_CLIENT_CERTIFICATE: "", - environment_vars.GOOGLE_API_CERTIFICATE_CONFIG: "", - environment_vars.CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH: "", - } - with mock.patch.dict(os.environ, env_to_patch): - # Test the callback is not called if GOOGLE_API_USE_CLIENT_CERTIFICATE is not set. - is_mtls = authed_http.configure_mtls_channel(callback) - assert not is_mtls - callback.assert_not_called() - - # Test ADC client cert is not used if GOOGLE_API_USE_CLIENT_CERTIFICATE is not set. - is_mtls = authed_http.configure_mtls_channel(callback) - assert not is_mtls - get_client_cert_and_key.assert_not_called() - - def test_clear_pool_on_del(self): - http = mock.create_autospec(urllib3.PoolManager) - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - mock.sentinel.credentials, http=http - ) - authed_http.__del__() - http.clear.assert_called_with() - - authed_http.http = None - authed_http.__del__() - # Expect it to not crash - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - @mock.patch("google.auth.transport.urllib3._make_default_http", autospec=True) - def test_cert_rotation_when_cert_mismatch_and_mtls_endpoint_used( - self, mock_make_default_http, mock_make_mutual_tls_http - ): - credentials = mock.Mock(wraps=CredentialsStub()) - final_response = ResponseStub(status=http_client.OK) - - # We simulate the HTTP stub rotation. When mTLS http is created, we return rotated_http. - rotated_http = HttpStub([final_response]) - mock_make_mutual_tls_http.return_value = rotated_http - - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - - old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" - - # New certificate and key to simulate rotation. - new_cert = pytest.public_cert_bytes - new_key = pytest.private_key_bytes - # Set _cached_cert to a callable that returns the old certificate. - authed_http._cached_cert = old_cert - authed_http._is_mtls = True - # Mock call_client_cert_callback to return the new certificate. - with mock.patch.object( - google.auth.transport._mtls_helper, - "call_client_cert_callback", - return_value=(new_cert, new_key), - ) as mock_callback: - # mTLS endpoint is used, and client cert env var is true - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - result = authed_http.urlopen( - "GET", "http://example.mtls.googleapis.com" - ) - - # Asserts to verify the behavior. - assert result == final_response - assert credentials.refresh.called - assert credentials.refresh.call_count == 1 - assert mock_callback.called - mock_make_mutual_tls_http.assert_called_once_with(cert=new_cert, key=new_key) - - def test_no_cert_rotation_when_cert_match_and_mtls_endpoint_used(self): - credentials = mock.Mock(wraps=CredentialsStub()) - final_response = ResponseStub(status=http_client.UNAUTHORIZED) - http = HttpStub( - [ - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.UNAUTHORIZED), - ] - ) - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - old_cert = CERT_MOCK_VAL - - new_cert = old_cert - new_key = KEY_MOCK_VAL - # Set _cached_cert to a callable that returns the same certificate. - authed_http._cached_cert = old_cert - authed_http._is_mtls = True - # Mock call_client_cert_callback to return the certificate. - with mock.patch.object( - google.auth.transport._mtls_helper, - "call_client_cert_callback", - return_value=(new_cert, new_key), - ): - # mTLS endpoint is used - result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") - - # Asserts to verify the behavior. - assert credentials.refresh.call_count == 2 - assert result.status == final_response.status - - def test_no_cert_match_check_when_mtls_endpoint_not_used(self): - credentials = mock.Mock(wraps=CredentialsStub()) - final_response = ResponseStub(status=http_client.UNAUTHORIZED) - http = HttpStub( - [ - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.UNAUTHORIZED), - ] - ) - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - authed_http._is_mtls = False - new_cert = CERT_MOCK_VAL - new_key = KEY_MOCK_VAL - - # Mock call_client_cert_callback to return the certificate. - with mock.patch.object( - google.auth.transport._mtls_helper, - "call_client_cert_callback", - return_value=(new_cert, new_key), - ) as mock_callback: - # non-mTLS endpoint is used - result = authed_http.urlopen("GET", "http://example.googleapis.com") - - # Asserts to verify the behavior. - assert not mock_callback.called - assert result.status == final_response.status - - def test_no_cert_rotation_when_no_unauthorized_response(self): - credentials = mock.Mock(wraps=CredentialsStub()) - final_response = ResponseStub(status=http_client.UPGRADE_REQUIRED) - - # Response is set to code other than 401(Unauthorized). - http = HttpStub([ResponseStub(status=http_client.UPGRADE_REQUIRED)]) - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - authed_http._is_mtls = True - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - # mTLS endpoint is used - result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") - assert result.status == final_response.status - assert not credentials.refresh.called - assert credentials.refresh.call_count == 0 - - def test_cert_rotation_failure_raises_error(self): - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - - old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" - - # New certificate and key to simulate rotation. - new_cert = CERT_MOCK_VAL - new_key = KEY_MOCK_VAL - authed_http._cached_cert = old_cert - authed_http._is_mtls = True - - # Mock call_client_cert_callback to return the new certificate. - with mock.patch.object( - google.auth.transport._mtls_helper, - "check_parameters_for_unauthorized_response", - return_value=(new_cert, new_key, "old_fingerprint", "new_fingerprint"), - ) as mock_check_params: - with mock.patch.object( - authed_http, - "configure_mtls_channel", - side_effect=Exception("Failed to reconfigure"), - ) as mock_reconfigure: - with pytest.raises(exceptions.MutualTLSChannelError): - authed_http.urlopen("GET", "https://example.mtls.googleapis.com") - - mock_check_params.assert_called_once() - mock_reconfigure.assert_called_once() - credentials.refresh.assert_not_called() - - def test_cert_rotation_check_params_fails(self): - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - authed_http._is_mtls = True - authed_http._cached_cert = b"cached_cert" - - with mock.patch( - "google.auth.transport.urllib3._mtls_helper.check_parameters_for_unauthorized_response", - side_effect=Exception("check_params failed"), - ) as mock_check_params: - with pytest.raises(Exception, match="check_params failed"): - authed_http.urlopen("GET", "http://example.mtls.googleapis.com") - - mock_check_params.assert_called_once() - credentials.refresh.assert_not_called() - - def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self): - """ - Tests that the code can handle a refresh triggered by a status code - other than 401 (UNAUTHORIZED). This covers the 'else' branch of the - 'if response.status_code == http_client.UNAUTHORIZED' check - """ - credentials = mock.Mock(wraps=CredentialsStub()) - # Configure the session to treat 503 (Service Unavailable) as a refreshable error - custom_codes = [http_client.SERVICE_UNAVAILABLE] - - # Return 503 first, then 200 - http = HttpStub( - [ - ResponseStub(status=http_client.SERVICE_UNAVAILABLE), - ResponseStub(status=http_client.OK), - ] - ) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http, refresh_status_codes=custom_codes - ) - - # Enable mTLS to prove it is skipped despite being enabled - authed_http._is_mtls = True - mtls_url = "https://mtls.googleapis.com/test" - - with mock.patch( - "google.auth.transport.urllib3._mtls_helper", autospec=True - ) as mock_helper: - authed_http.urlopen("GET", mtls_url) - - # Assert refresh happened (Outer Check was True) - assert credentials.refresh.called - - # Assert mTLS check logic was SKIPPED (Inner Check was False) - assert not mock_helper.check_parameters_for_unauthorized_response.called - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - def test_configure_mtls_channel_subsequent_failure(self, mock_make_mutual_tls_http): - callback = mock.Mock() - callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel(callback) - - assert is_mtls - assert authed_http._is_mtls - - # Subsequent call fails - with mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) as mock_get_client_cert_and_key: - mock_get_client_cert_and_key.side_effect = exceptions.ClientCertError() - - with pytest.raises(exceptions.MutualTLSChannelError): - with mock.patch.dict( - os.environ, - {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, - ): - authed_http.configure_mtls_channel() - - # Verify it retains its previous mTLS state and connection pool - assert authed_http._is_mtls - assert isinstance(authed_http.http, mock.Mock) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - def test_configure_mtls_channel_subsequent_disabled( - self, mock_make_mutual_tls_http - ): - callback = mock.Mock() - callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel(callback) - - assert is_mtls - assert authed_http._is_mtls - - # Subsequent call returns no client certificate (disabled) - with mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) as mock_get_client_cert_and_key: - mock_get_client_cert_and_key.return_value = (False, None, None) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - # Verify mTLS is disabled and standard PoolManager is restored - assert not is_mtls - assert not authed_http._is_mtls - assert isinstance(authed_http.http, urllib3.PoolManager) - - -class TestAuthorizedHttpMTLSReauth: - @mock.patch( - "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" - ) - def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): - credentials = mock.Mock() - http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) - http_obj._is_mtls = True - http_obj._cached_cert = b"cert" - mock_response = mock.Mock() - mock_response.status = http_client.UNAUTHORIZED - http_obj.http.urlopen = mock.Mock(return_value=mock_response) - real_lock = threading.Lock() - http_obj._reauth_lock = real_lock - mock_check_params.return_value = ( - b"new_cert_bytes", - b"new_key_bytes", - "old_fingerprint", - "new_fingerprint", - ) - lock_held_during_call = {"held": False} - - def verify_lock_held(*args, **kwargs): - lock_held_during_call["held"] = real_lock.locked() - - http_obj.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) - http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_called() - assert lock_held_during_call["held"] is True - - @mock.patch( - "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" - ) - def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): - credentials = mock.Mock() - http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) - http_obj._is_mtls = True - http_obj._cached_cert = b"cert" - mock_response_unauth = mock.Mock() - mock_response_unauth.status = http_client.UNAUTHORIZED - mock_response_ok = mock.Mock() - mock_response_ok.status = http_client.OK - http_obj.http.urlopen = mock.Mock( - side_effect=[mock_response_unauth, mock_response_ok] - ) - mock_check_params.return_value = ( - b"same_cert_bytes", - b"same_key_bytes", - "same_fingerprint", - "same_fingerprint", - ) - http_obj.configure_mtls_channel = mock.Mock() - http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_not_called() + def test From be7908b624572cdaf4599ee1c7b2b677d82e277d Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Tue, 1 Sep 2026 21:56:48 -0700 Subject: [PATCH 19/34] fix: Implement timeout test for urllib3 Request Add test for timeout in urllib3 Request --- .../tests/transport/test_urllib3.py | 744 +++++++++++++++++- 1 file changed, 743 insertions(+), 1 deletion(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 2f9e5b1398fc..e4a0fe20322b 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -37,4 +37,746 @@ def make_request(self): http = urllib3.PoolManager() return google.auth.transport.urllib3.Request(http) - def test + def test_timeout(self): + http = mock.create_autospec(urllib3.PoolManager) + request = google.auth.transport.urllib3.Request(http) + request(url="http://example.com", method="GET", timeout=5) + + assert http.request.call_args[1]["timeout"] == 5 + + +def test__make_default_http_with_certifi(): + http = google.auth.transport.urllib3._make_default_http() + assert "cert_reqs" in http.connection_pool_kw + + +@mock.patch.object(google.auth.transport.urllib3, "certifi", new=None) +def test__make_default_http_without_certifi(): + http = google.auth.transport.urllib3._make_default_http() + assert "cert_reqs" not in http.connection_pool_kw + + +class CredentialsStub(google.auth.credentials.Credentials): + def __init__(self, token="token"): + super(CredentialsStub, self).__init__() + self.token = token + + def apply(self, headers, token=None): + headers["authorization"] = self.token + + def before_request(self, request, method, url, headers): + self.apply(headers) + + def refresh(self, request): + self.token += "1" + + def with_quota_project(self, quota_project_id): + raise NotImplementedError() + + +class HttpStub(object): + def __init__(self, responses, headers=None): + self.responses = responses + self.requests = [] + self.headers = headers or {} + + def urlopen(self, method, url, body=None, headers=None, **kwargs): + self.requests.append((method, url, body, headers, kwargs)) + return self.responses.pop(0) + + def clear(self): + pass + + +class ResponseStub(object): + def __init__(self, status=http_client.OK, data=None): + self.status = status + self.data = data + + +class TestMakeMutualTlsHttp(object): + def test_success(self): + http = google.auth.transport.urllib3._make_mutual_tls_http( + pytest.public_cert_bytes, pytest.private_key_bytes + ) + assert isinstance(http, urllib3.PoolManager) + + def test_crypto_error(self): + with pytest.raises(exceptions.MutualTLSChannelError): + google.auth.transport.urllib3._make_mutual_tls_http( + b"invalid cert", b"invalid key" + ) + + @mock.patch("google.auth.transport.urllib3._mtls_helper.secure_cert_key_paths") + def test_setup_error_raises_mutual_tls_channel_error(self, mock_secure_paths): + mock_secure_paths.side_effect = OSError("Disk full") + with pytest.raises(exceptions.MutualTLSChannelError) as exc_info: + google.auth.transport.urllib3._make_mutual_tls_http(b"cert", b"key") + assert "Failed to configure client certificate" in str(exc_info.value) + assert isinstance(exc_info.value.__cause__, OSError) + + +class TestAuthorizedHttp(object): + TEST_URL = "http://example.com" + + def test_authed_http_defaults(self): + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + mock.sentinel.credentials + ) + + assert authed_http.credentials == mock.sentinel.credentials + assert isinstance(authed_http.http, urllib3.PoolManager) + + def test_urlopen_no_refresh(self): + credentials = mock.Mock(wraps=CredentialsStub()) + response = ResponseStub() + http = HttpStub([response]) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + + result = authed_http.urlopen("GET", self.TEST_URL) + + assert result == response + assert credentials.before_request.called + assert not credentials.refresh.called + assert http.requests == [ + ("GET", self.TEST_URL, None, {"authorization": "token"}, {}) + ] + + def test_urlopen_refresh(self): + credentials = mock.Mock(wraps=CredentialsStub()) + final_response = ResponseStub(status=http_client.OK) + # First request will 401, second request will succeed. + http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED), final_response]) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + + authed_http = authed_http.urlopen("GET", "http://example.com") + + assert authed_http == final_response + assert credentials.before_request.call_count == 2 + assert credentials.refresh.called + assert http.requests == [ + ("GET", self.TEST_URL, None, {"authorization": "token"}, {}), + ("GET", self.TEST_URL, None, {"authorization": "token1"}, {}), + ] + + def test_urlopen_no_default_host(self): + credentials = mock.create_autospec(service_account.Credentials) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp(credentials) + + authed_http.credentials._create_self_signed_jwt.assert_called_once_with(None) + + def test_urlopen_with_default_host(self): + default_host = "pubsub.googleapis.com" + credentials = mock.create_autospec(service_account.Credentials) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, default_host=default_host + ) + + authed_http.credentials._create_self_signed_jwt.assert_called_once_with( + "https://{}/".format(default_host) + ) + + def test_proxies(self): + http = mock.create_autospec(urllib3.PoolManager) + authed_http = google.auth.transport.urllib3.AuthorizedHttp(None, http=http) + + with authed_http: + pass + + assert http.__enter__.called + assert http.__exit__.called + + authed_http.headers = mock.sentinel.headers + assert authed_http.headers == http.headers + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + def test_configure_mtls_channel_with_callback(self, mock_make_mutual_tls_http): + callback = mock.Mock() + callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock(), http=mock.Mock() + ) + + with pytest.warns(UserWarning): + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel(callback) + + assert is_mtls + mock_make_mutual_tls_http.assert_called_once_with( + cert=pytest.public_cert_bytes, key=pytest.private_key_bytes + ) + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_with_metadata( + self, mock_get_client_cert_and_key, mock_make_mutual_tls_http + ): + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + mock_get_client_cert_and_key.return_value = ( + True, + pytest.public_cert_bytes, + pytest.private_key_bytes, + ) + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel() + + assert is_mtls + mock_get_client_cert_and_key.assert_called_once() + mock_make_mutual_tls_http.assert_called_once_with( + cert=pytest.public_cert_bytes, key=pytest.private_key_bytes + ) + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_closes_old_poolmanager( + self, mock_get_client_cert_and_key, mock_make_mutual_tls_http + ): + mock_get_client_cert_and_key.return_value = ( + True, + pytest.public_cert_bytes, + pytest.private_key_bytes, + ) + + old_http = mock.create_autospec(urllib3.PoolManager) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock(), http=old_http + ) + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel() + + assert is_mtls + old_http.clear.assert_called_once() + mock_make_mutual_tls_http.assert_called_once_with( + cert=pytest.public_cert_bytes, key=pytest.private_key_bytes + ) + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_with_none_http( + self, mock_get_client_cert_and_key, mock_make_mutual_tls_http + ): + mock_get_client_cert_and_key.return_value = ( + True, + pytest.public_cert_bytes, + pytest.private_key_bytes, + ) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + authed_http.http = None # Force old_http to be None + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel() + + assert is_mtls + mock_make_mutual_tls_http.assert_called_once_with( + cert=pytest.public_cert_bytes, key=pytest.private_key_bytes + ) + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_non_mtls( + self, mock_get_client_cert_and_key, mock_make_mutual_tls_http + ): + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + mock_get_client_cert_and_key.return_value = (False, None, None) + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel() + + assert not is_mtls + # If client certificate and key are not found, the transport falls back to + # a standard connection. _is_mtls must be False to reflect this fallback state. + assert authed_http._is_mtls is False + mock_get_client_cert_and_key.assert_called_once() + mock_make_mutual_tls_http.assert_not_called() + + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key): + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + mock_get_client_cert_and_key.side_effect = exceptions.ClientCertError() + with pytest.raises(exceptions.MutualTLSChannelError): + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + authed_http.configure_mtls_channel() + assert authed_http._is_mtls is False + + mock_get_client_cert_and_key.side_effect = OSError("Mock file read error") + with pytest.raises(exceptions.MutualTLSChannelError): + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + authed_http.configure_mtls_channel() + assert authed_http._is_mtls is False + + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + @mock.patch( + "google.auth.transport.urllib3.urllib3.util.ssl_.create_urllib3_context", + autospec=True, + ) + def test_configure_mtls_channel_cert_loading_exceptions( + self, mock_create_urllib3_context, mock_get_client_cert_and_key + ): + import ssl + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + mock_get_client_cert_and_key.return_value = ( + True, + pytest.public_cert_bytes, + pytest.private_key_bytes, + ) + + for exception_type in [ValueError("error"), ssl.SSLError("error")]: + mock_ctx = mock.Mock() + mock_ctx.load_cert_chain.side_effect = exception_type + mock_create_urllib3_context.return_value = mock_ctx + + with pytest.raises(exceptions.MutualTLSChannelError): + with mock.patch.dict( + os.environ, + {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, + ): + authed_http.configure_mtls_channel() + assert authed_http._is_mtls is False + + assert not authed_http._is_mtls + + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + @mock.patch.dict( + os.environ, + { + "GOOGLE_API_USE_CLIENT_CERTIFICATE": "false", + "CLOUDSDK_CONTEXT_AWARE_USE_CLIENT_CERTIFICATE": "false", + "GOOGLE_API_CERTIFICATE_CONFIG": "", + "CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "", + }, + ) + def test_configure_mtls_channel_without_client_cert_env( + self, get_client_cert_and_key + ): + callback = mock.Mock() + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock(), http=mock.Mock() + ) + + env_to_patch = { + environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "", + environment_vars.CLOUDSDK_CONTEXT_AWARE_USE_CLIENT_CERTIFICATE: "", + environment_vars.GOOGLE_API_CERTIFICATE_CONFIG: "", + environment_vars.CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH: "", + } + with mock.patch.dict(os.environ, env_to_patch): + # Test the callback is not called if GOOGLE_API_USE_CLIENT_CERTIFICATE is not set. + is_mtls = authed_http.configure_mtls_channel(callback) + assert not is_mtls + callback.assert_not_called() + + # Test ADC client cert is not used if GOOGLE_API_USE_CLIENT_CERTIFICATE is not set. + is_mtls = authed_http.configure_mtls_channel(callback) + assert not is_mtls + get_client_cert_and_key.assert_not_called() + + def test_clear_pool_on_del(self): + http = mock.create_autospec(urllib3.PoolManager) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + mock.sentinel.credentials, http=http + ) + authed_http.__del__() + http.clear.assert_called_with() + + authed_http.http = None + authed_http.__del__() + # Expect it to not crash + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + @mock.patch("google.auth.transport.urllib3._make_default_http", autospec=True) + def test_cert_rotation_when_cert_mismatch_and_mtls_endpoint_used( + self, mock_make_default_http, mock_make_mutual_tls_http + ): + credentials = mock.Mock(wraps=CredentialsStub()) + final_response = ResponseStub(status=http_client.OK) + + # We simulate the HTTP stub rotation. When mTLS http is created, we return rotated_http. + rotated_http = HttpStub([final_response]) + mock_make_mutual_tls_http.return_value = rotated_http + + http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + + old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" + + # New certificate and key to simulate rotation. + new_cert = pytest.public_cert_bytes + new_key = pytest.private_key_bytes + # Set _cached_cert to a callable that returns the old certificate. + authed_http._cached_cert = old_cert + authed_http._is_mtls = True + # Mock call_client_cert_callback to return the new certificate. + with mock.patch.object( + google.auth.transport._mtls_helper, + "call_client_cert_callback", + return_value=(new_cert, new_key), + ) as mock_callback: + # mTLS endpoint is used, and client cert env var is true + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + result = authed_http.urlopen( + "GET", "http://example.mtls.googleapis.com" + ) + + # Asserts to verify the behavior. + assert result == final_response + assert credentials.refresh.called + assert credentials.refresh.call_count == 1 + assert mock_callback.called + mock_make_mutual_tls_http.assert_called_once_with(cert=new_cert, key=new_key) + + def test_no_cert_rotation_when_cert_match_and_mtls_endpoint_used(self): + credentials = mock.Mock(wraps=CredentialsStub()) + final_response = ResponseStub(status=http_client.UNAUTHORIZED) + http = HttpStub( + [ + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.UNAUTHORIZED), + ] + ) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + old_cert = CERT_MOCK_VAL + + new_cert = old_cert + new_key = KEY_MOCK_VAL + # Set _cached_cert to a callable that returns the same certificate. + authed_http._cached_cert = old_cert + authed_http._is_mtls = True + # Mock call_client_cert_callback to return the certificate. + with mock.patch.object( + google.auth.transport._mtls_helper, + "call_client_cert_callback", + return_value=(new_cert, new_key), + ): + # mTLS endpoint is used + result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") + + # Asserts to verify the behavior. + assert credentials.refresh.call_count == 2 + assert result.status == final_response.status + + def test_no_cert_match_check_when_mtls_endpoint_not_used(self): + credentials = mock.Mock(wraps=CredentialsStub()) + final_response = ResponseStub(status=http_client.UNAUTHORIZED) + http = HttpStub( + [ + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.UNAUTHORIZED), + ] + ) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + authed_http._is_mtls = False + new_cert = CERT_MOCK_VAL + new_key = KEY_MOCK_VAL + + # Mock call_client_cert_callback to return the certificate. + with mock.patch.object( + google.auth.transport._mtls_helper, + "call_client_cert_callback", + return_value=(new_cert, new_key), + ) as mock_callback: + # non-mTLS endpoint is used + result = authed_http.urlopen("GET", "http://example.googleapis.com") + + # Asserts to verify the behavior. + assert not mock_callback.called + assert result.status == final_response.status + + def test_no_cert_rotation_when_no_unauthorized_response(self): + credentials = mock.Mock(wraps=CredentialsStub()) + final_response = ResponseStub(status=http_client.UPGRADE_REQUIRED) + + # Response is set to code other than 401(Unauthorized). + http = HttpStub([ResponseStub(status=http_client.UPGRADE_REQUIRED)]) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + authed_http._is_mtls = True + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + # mTLS endpoint is used + result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") + assert result.status == final_response.status + assert not credentials.refresh.called + assert credentials.refresh.call_count == 0 + + def test_cert_rotation_failure_raises_error(self): + credentials = mock.Mock(wraps=CredentialsStub()) + http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + + old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" + + # New certificate and key to simulate rotation. + new_cert = CERT_MOCK_VAL + new_key = KEY_MOCK_VAL + authed_http._cached_cert = old_cert + authed_http._is_mtls = True + + # Mock call_client_cert_callback to return the new certificate. + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=(new_cert, new_key, "old_fingerprint", "new_fingerprint"), + ) as mock_check_params: + with mock.patch.object( + authed_http, + "configure_mtls_channel", + side_effect=Exception("Failed to reconfigure"), + ) as mock_reconfigure: + with pytest.raises(exceptions.MutualTLSChannelError): + authed_http.urlopen("GET", "https://example.mtls.googleapis.com") + + mock_check_params.assert_called_once() + mock_reconfigure.assert_called_once() + credentials.refresh.assert_not_called() + + def test_cert_rotation_check_params_fails(self): + credentials = mock.Mock(wraps=CredentialsStub()) + http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + authed_http._is_mtls = True + authed_http._cached_cert = b"cached_cert" + + with mock.patch( + "google.auth.transport.urllib3._mtls_helper.check_parameters_for_unauthorized_response", + side_effect=Exception("check_params failed"), + ) as mock_check_params: + with pytest.raises(Exception, match="check_params failed"): + authed_http.urlopen("GET", "http://example.mtls.googleapis.com") + + mock_check_params.assert_called_once() + credentials.refresh.assert_not_called() + + def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self): + """ + Tests that the code can handle a refresh triggered by a status code + other than 401 (UNAUTHORIZED). This covers the 'else' branch of the + 'if response.status_code == http_client.UNAUTHORIZED' check + """ + credentials = mock.Mock(wraps=CredentialsStub()) + # Configure the session to treat 503 (Service Unavailable) as a refreshable error + custom_codes = [http_client.SERVICE_UNAVAILABLE] + + # Return 503 first, then 200 + http = HttpStub( + [ + ResponseStub(status=http_client.SERVICE_UNAVAILABLE), + ResponseStub(status=http_client.OK), + ] + ) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http, refresh_status_codes=custom_codes + ) + + # Enable mTLS to prove it is skipped despite being enabled + authed_http._is_mtls = True + mtls_url = "https://mtls.googleapis.com/test" + + with mock.patch( + "google.auth.transport.urllib3._mtls_helper", autospec=True + ) as mock_helper: + authed_http.urlopen("GET", mtls_url) + + # Assert refresh happened (Outer Check was True) + assert credentials.refresh.called + + # Assert mTLS check logic was SKIPPED (Inner Check was False) + assert not mock_helper.check_parameters_for_unauthorized_response.called + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + def test_configure_mtls_channel_subsequent_failure(self, mock_make_mutual_tls_http): + callback = mock.Mock() + callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel(callback) + + assert is_mtls + assert authed_http._is_mtls + + # Subsequent call fails + with mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) as mock_get_client_cert_and_key: + mock_get_client_cert_and_key.side_effect = exceptions.ClientCertError() + + with pytest.raises(exceptions.MutualTLSChannelError): + with mock.patch.dict( + os.environ, + {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, + ): + authed_http.configure_mtls_channel() + + # Verify it retains its previous mTLS state and connection pool + assert authed_http._is_mtls + assert isinstance(authed_http.http, mock.Mock) + + @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) + def test_configure_mtls_channel_subsequent_disabled( + self, mock_make_mutual_tls_http + ): + callback = mock.Mock() + callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials=mock.Mock() + ) + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel(callback) + + assert is_mtls + assert authed_http._is_mtls + + # Subsequent call returns no client certificate (disabled) + with mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) as mock_get_client_cert_and_key: + mock_get_client_cert_and_key.return_value = (False, None, None) + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + is_mtls = authed_http.configure_mtls_channel() + + # Verify mTLS is disabled and standard PoolManager is restored + assert not is_mtls + assert not authed_http._is_mtls + assert isinstance(authed_http.http, urllib3.PoolManager) + + +class TestAuthorizedHttpMTLSReauth: + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) + def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): + credentials = mock.Mock() + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True + http_obj._cached_cert = b"cert" + mock_response = mock.Mock() + mock_response.status = http_client.UNAUTHORIZED + http_obj.http.urlopen = mock.Mock(return_value=mock_response) + real_lock = threading.Lock() + http_obj._reauth_lock = real_lock + mock_check_params.return_value = ( + b"new_cert_bytes", + b"new_key_bytes", + "old_fingerprint", + "new_fingerprint", + ) + lock_held_during_call = {"held": False} + + def verify_lock_held(*args, **kwargs): + lock_held_during_call["held"] = real_lock.locked() + + http_obj.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) + http_obj.request("GET", "https://example.mtls.googleapis.com/") + http_obj.configure_mtls_channel.assert_called() + assert lock_held_during_call["held"] is True + + @mock.patch( + "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" + ) + def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): + credentials = mock.Mock() + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True + http_obj._cached_cert = b"cert" + mock_response_unauth = mock.Mock() + mock_response_unauth.status = http_client.UNAUTHORIZED + mock_response_ok = mock.Mock() + mock_response_ok.status = http_client.OK + http_obj.http.urlopen = mock.Mock( + side_effect=[mock_response_unauth, mock_response_ok] + ) + mock_check_params.return_value = ( + b"same_cert_bytes", + b"same_key_bytes", + "same_fingerprint", + "same_fingerprint", + ) + http_obj.configure_mtls_channel = mock.Mock() + http_obj.request("GET", "https://example.mtls.googleapis.com/") + http_obj.configure_mtls_channel.assert_not_called() From 42cf4bfc27e687a5d60e139378c87d1f5cd52bbc Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Thu, 3 Sep 2026 11:14:51 -0700 Subject: [PATCH 20/34] fix: Simplify MTLS endpoint check in urllib3.py Refactor MTLS URL prefix handling for clarity. --- packages/google-auth/google/auth/transport/urllib3.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index ab0322d1ae38..424c246f820a 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -433,13 +433,7 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: - MTLS_URL_PREFIXES = [ - "mtls.googleapis.com", - "mtls.sandbox.googleapis.com", - ] - use_mtls = getattr(self, "_is_mtls", False) and any( - prefix in url for prefix in MTLS_URL_PREFIXES - ) + use_mtls = self._is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: with self._reauth_lock: From 3c96b1b075aca7b1bb12e3878b3252ddaa2202ba Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 11:58:32 -0700 Subject: [PATCH 21/34] chore: Replace reauth_lock with mtls_reauth_lock for mTLS and add try-except block --- .../google/auth/transport/requests.py | 70 +++++++++++-------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 0917ed362c3c..f2f38545a10c 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -415,7 +415,7 @@ def __init__( self._refresh_timeout = refresh_timeout self._is_mtls = False self._default_host = default_host - self._reauth_lock = threading.Lock() + self._mtls_reauth_lock = threading.Lock() if auth_request is None: self._auth_request_session = requests.Session() @@ -651,36 +651,44 @@ def request( if response.status_code == http_client.UNAUTHORIZED: use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: - with self._reauth_lock: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - self._cached_cert - ) - if cached_fingerprint != current_cert_fingerprint: - try: - _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." - ) - self.configure_mtls_channel( - lambda: (call_cert_bytes, call_key_bytes) - ) - except Exception as e: - _LOGGER.error( - "Failed to reconfigure mTLS channel: %s", e - ) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the client" - " certificate has not changed." - ) + with TimeoutGuard(remaining_time) as guard: + with self._mtls_reauth_lock: + if getattr(self, "is_mtls", False): + try: + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + getattr(self, "_cached_cert", None) + ) + if cached_fingerprint != current_cert_fingerprint: + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS " + "channel." + ) + self.configure_mtls_channel( + lambda: (call_cert_bytes, call_key_bytes) + ) + except Exception as e: + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e + ) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + else: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because the client" + " certificate has not changed." + ) + except exceptions.MutualTLSChannelError: + raise + except Exception as e: + _LOGGER.warning("mTLS certificate check/rotation failed: %s", e) + remaining_time = guard.remaining_timeout _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", response.status_code, From 42b01976675a8558ac5578463a2760acc797039b Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 12:08:45 -0700 Subject: [PATCH 22/34] fix: Rename reauth lock to mtls reauth lock and add is_mtls property --- packages/google-auth/google/auth/transport/urllib3.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 424c246f820a..5fbcc5d03dcc 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -310,7 +310,7 @@ def __init__( # credentials.refresh). self._request = Request(self.http) self._is_mtls = False - self._reauth_lock = threading.Lock() + self._mtls_reauth_lock = threading.Lock() # https://google.aip.dev/auth/4111 # Attempt to use self-signed JWTs when a service account is used. @@ -321,6 +321,11 @@ def __init__( super(AuthorizedHttp, self).__init__() + @property + def is_mtls(self): + """Indicates if the created SSL channel is mutual TLS.""" + return self._is_mtls + def configure_mtls_channel(self, client_cert_callback=None): """Configures mutual TLS channel using the given client_cert_callback or application default SSL credentials. @@ -433,10 +438,10 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: - use_mtls = self._is_mtls and _mtls_helper.is_mtls_endpoint(url) + use_mtls = getattr(self, "_is_mtls", False) and _mtls_helper.is_mtls_endpoint(url) if use_mtls: - with self._reauth_lock: + with self._mtls_reauth_lock: ( call_cert_bytes, call_key_bytes, From 80d477d3f426dc935c3b5a43ca0ed163c6699ed5 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 13:13:23 -0700 Subject: [PATCH 23/34] chore: Refactor mTLS channel reconfiguration logic Refactor mTLS reconfiguration logic to check 'is_mtls' attribute before proceeding with certificate checks and channel configuration. --- .../google/auth/transport/urllib3.py | 62 ++++++++++--------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 5fbcc5d03dcc..f2aedf377748 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -442,39 +442,45 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): if use_mtls: with self._mtls_reauth_lock: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - self._cached_cert - ) - if cached_fingerprint != current_cert_fingerprint: + if getattr(self, "is_mtls", getattr(self, "_is_mtls", False)): try: - _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + getattr(self, "_cached_cert", None) ) - self.configure_mtls_channel( - client_cert_callback=lambda: ( - call_cert_bytes, - call_key_bytes, + if cached_fingerprint != current_cert_fingerprint: + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS " + "channel." + ) + self.configure_mtls_channel( + client_cert_callback=lambda: ( + call_cert_bytes, + call_key_bytes, + ) + ) + except Exception as e: + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e + ) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + else: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because the " + "client certificate has not changed." ) - ) except Exception as e: - _LOGGER.error( - "Failed to reconfigure mTLS channel: %s", e + _LOGGER.warning( + "Failed to check parameters or reconfigure mTLS channel: %s", + e, ) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the " - "client certificate has not changed." - ) - _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", response.status, From 0c99f0f9f8be42ea605d497a0a9f4afdf457db0f Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 13:22:06 -0700 Subject: [PATCH 24/34] fix: Enhance test coverage for mTLS token refresh logic Add tests for handling unauthorized responses and token refresh in mTLS sessions. --- .../tests/transport/test_requests.py | 76 +++++++++++++------ 1 file changed, 51 insertions(+), 25 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index acab334d9be4..1e6389e68611 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1081,6 +1081,28 @@ def test_configure_mtls_channel_subsequent_disabled(self): requests.adapters.HTTPAdapter, ) + def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + session = requests.AuthorizedSession(credentials) + session._is_mtls = True + mock_response_unauth = mock.Mock(status_code=http_client.UNAUTHORIZED) + mock_response_ok = mock.Mock(status_code=http_client.OK) + # Simulate discovery failure (e.g. Enterprise cert provider error or file missing) + with mock.patch.object( + requests._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=Exception("Certificate discovery failed"), + ): + with mock.patch.object( + super(requests.AuthorizedSession, session), + "request", + side_effect=[mock_response_unauth, mock_response_ok], + ): + session.request("GET", "https://example.mtls.googleapis.com/") + # Token refresh should still be called despite discovery failure + credentials.refresh.assert_called_once() + assert not session._mtls_reauth_lock.locked() + class TestMutualTlsOffloadAdapter(object): @mock.patch.object(requests.adapters.HTTPAdapter, "init_poolmanager") @@ -1154,34 +1176,38 @@ class TestAuthorizedSessionMTLSReauth: "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" ) @mock.patch("google.auth.transport.requests.requests.Session.request") - def test_reauth_lock_acquired_on_unauthorized( - self, mock_session_request, mock_check_params - ): - credentials = mock.Mock() - session = google.auth.transport.requests.AuthorizedSession(credentials) - session._is_mtls = True - session._cached_cert = b"cert" - mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) - mock_success_response = mock.Mock(status_code=http_client.OK) - mock_session_request.side_effect = [mock_response, mock_success_response] - real_lock = threading.Lock() - session._reauth_lock = real_lock - mock_check_params.return_value = ( - b"new_cert_bytes", - b"new_key_bytes", - "old_fingerprint", - "new_fingerprint", - ) - lock_held_during_call = {"held": False} + def test_reauth_lock_acquired_on_unauthorized(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + session = requests.AuthorizedSession(credentials) + session._is_mtls = True - def verify_lock_held(*args, **kwargs): - lock_held_during_call["held"] = session._reauth_lock.locked() + mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) + mock_success_response = mock.Mock(status_code=http_client.OK) - session.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) - session.request("GET", "https://example.mtls.googleapis.com/") + lock_held_during_call = {"held": False} + + def mock_configure_mtls_channel(callback): + # Verify the lock is held during channel reconfiguration + lock_held_during_call["held"] = session._mtls_reauth_lock.locked() + + session.configure_mtls_channel = mock.Mock(side_effect=mock_configure_mtls_channel) + + with mock.patch.object( + requests._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=(b"cert", b"key", "old_fp", "new_fp"), + ): + with mock.patch.object( + super(requests.AuthorizedSession, session), + "request", + side_effect=[mock_response, mock_success_response], + ): + session.request("GET", "https://example.mtls.googleapis.com/") - session.configure_mtls_channel.assert_called() - assert lock_held_during_call["held"] is True + session.configure_mtls_channel.assert_called_once() + assert lock_held_during_call["held"] is True + # Verify the lock is released after request finishes + assert not session._mtls_reauth_lock.locked() @mock.patch( "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" From 8cc56daf54d0988a802edae1a90e772f6035154d Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 13:49:35 -0700 Subject: [PATCH 25/34] chore: Refactor mTLS tests and enhance coverage Refactor certificate and key handling in tests, add new tests for mTLS reauthentication and unauthorized response handling. --- .../tests/transport/test_urllib3.py | 369 ++++-------------- 1 file changed, 81 insertions(+), 288 deletions(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index a6eff754b685..13a9af21de49 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -14,7 +14,6 @@ import http.client as http_client import os -import threading from unittest import mock import pytest # type: ignore @@ -23,13 +22,41 @@ from google.auth import environment_vars from google.auth import exceptions import google.auth.credentials +from google.auth.tests.transport import compliance import google.auth.transport._mtls_helper import google.auth.transport.urllib3 from google.oauth2 import service_account -from tests.transport import compliance -CERT_MOCK_VAL = b"-----BEGIN CERTIFICATE-----\nMIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\nBAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\nMRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\nCgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\nuQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\ngyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\nZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\ngN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\nGaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\nAQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\nodJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\novNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\nybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\ncDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n-----END CERTIFICATE-----\n" -KEY_MOCK_VAL = b"-----BEGIN ENCRYPTED PRIVATE KEY-----\nMIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAj9XnJ2h78QVAICCAAw\nHQYJYIZIAWUDBAECBBBeiiOF2LnLzq/wjb/viwMwBIGQk28Zkfj2EIk42bgc7UzC\nSf98qssCVhsIYz0Xa3eSATg8Cpn83YieaBeyxdk/tXTnrOhxMV/vt7T98kWhaGbH\n5Z9CdGVLfes0UFvVJqrlk6vcf2sOnLCGbrn78HS+ayrGOCRSCd/7+dnEiB/7Um1B\nMk6BBJHsLEnZZSHyfrw8jvYgVmcSBy/WdY0pqldD/+4D\n-----END ENCRYPTED PRIVATE KEY-----\n" +CERT_MOCK_VAL = ( + b"-----BEGIN CERTIFICATE-----\n" + b"MIIDIzCCAgugAwIBAgIJAMfISuBQ5m+5MA0GCSqGSIb3DQEBBQUAMBUxEzARBgNV\n" + b"BAMTCnVuaXQtdGVzdHMwHhcNMTExMjA2MTYyNjAyWhcNMjExMjAzMTYyNjAyWjAV\n" + b"MRMwEQYDVQQDEwp1bml0LXRlc3RzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB\n" + b"CgKCAQEA4ej0p7bQ7L/r4rVGUz9RN4VQWoej1Bg1mYWIDYslvKrk1gpj7wZgkdmM\n" + b"7oVK2OfgrSj/FCTkInKPqaCR0gD7K80q+mLBrN3PUkDrJQZpvRZIff3/xmVU1Wer\n" + b"uQLFJjnFb2dqu0s/FY/2kWiJtBCakXvXEOb7zfbINuayL+MSsCGSdVYsSliS5qQp\n" + b"gyDap+8b5fpXZVJkq92hrcNtbkg7hCYUJczt8n9hcCTJCfUpApvaFQ18pe+zpyl4\n" + b"+WzkP66I28hniMQyUlA1hBiskT7qiouq0m8IOodhv2fagSZKjOTTU2xkSBc//fy3\n" + b"ZpsL7WqgsZS7Q+0VRK8gKfqkxg5OYQIDAQABo3YwdDAdBgNVHQ4EFgQU2RQ8yO+O\n" + b"gN8oVW2SW7RLrfYd9jEwRQYDVR0jBD4wPIAU2RQ8yO+OgN8oVW2SW7RLrfYd9jGh\n" + b"GaQXMBUxEzARBgNVBAMTCnVuaXQtdGVzdHOCCQDHyErgUOZvuTAMBgNVHRMEBTAD\n" + b"AQH/MA0GCSqGSIb3DQEBBQUAA4IBAQBRv+M/6+FiVu7KXNjFI5pSN17OcW5QUtPr\n" + b"odJMlWrJBtynn/TA1oJlYu3yV5clc/71Vr/AxuX5xGP+IXL32YDF9lTUJXG/uUGk\n" + b"+JETpKmQviPbRsvzYhz4pf6ZIOZMc3/GIcNq92ECbseGO+yAgyWUVKMmZM0HqXC9\n" + b"ovNslqe0M8C1sLm1zAR5z/h/litE7/8O2ietija3Q/qtl2TOXJdCA6sgjJX2WUql\n" + b"ybrC55ct18NKf3qhpcEkGQvFU40rVYApJpi98DiZPYFdx1oBDp/f4uZ3ojpxRVFT\n" + b"cDwcJLfNRCPUhormsY7fDS9xSyThiHsW9mjJYdcaKQkwYZ0F11yB\n" + b"-----END CERTIFICATE-----\n" +) +KEY_MOCK_VAL = ( + b"-----BEGIN ENCRYPTED PRIVATE KEY-----\n" + b"MIHeMEkGCSqGSIb3DQEFDTA8MBsGCSqGSIb3DQEFDDAOBAj9XnJ2h78QVAICCAAw\n" + b"HQYJYIZIAWUDBAECBBBeiiOF2LnLzq/wjb/viwMwBIGQk28Zkfj2EIk42bgc7UzC\n" + b"Sf98qssCVhsIYz0Xa3eSATg8Cpn83YieaBeyxdk/tXTnrOhxMV/vt7T98kWhaGbH\n" + b"5Z9CdGVLfes0UFvVJqrlk6vcf2sOnLCGbrn78HS+ayrGOCRSCd/7+dnEiB/7Um1B\n" + b"Mk6BBJHsLEnZZSHyfrw8jvYgVmcSBy/WdY0pqldD/+4D\n" + b"-----END ENCRYPTED PRIVATE KEY-----\n" +) class TestRequestResponse(compliance.RequestResponseTests): @@ -126,6 +153,14 @@ def test_authed_http_defaults(self): assert authed_http.credentials == mock.sentinel.credentials assert isinstance(authed_http.http, urllib3.PoolManager) + assert not authed_http.is_mtls + + def test_is_mtls_property(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + assert http_obj.is_mtls is False + http_obj._is_mtls = True + assert http_obj.is_mtls is True def test_urlopen_no_refresh(self): credentials = mock.Mock(wraps=CredentialsStub()) @@ -319,9 +354,7 @@ def test_configure_mtls_channel_non_mtls( is_mtls = authed_http.configure_mtls_channel() assert not is_mtls - # If client certificate and key are not found, the transport falls back to - # a standard connection. _is_mtls must be False to reflect this fallback state. - assert authed_http._is_mtls is False + assert authed_http.is_mtls is False mock_get_client_cert_and_key.assert_called_once() mock_make_mutual_tls_http.assert_not_called() @@ -339,7 +372,7 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key): os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} ): authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False + assert authed_http.is_mtls is False mock_get_client_cert_and_key.side_effect = OSError("Mock file read error") with pytest.raises(exceptions.MutualTLSChannelError): @@ -347,7 +380,7 @@ def test_configure_mtls_channel_exceptions(self, mock_get_client_cert_and_key): os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} ): authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False + assert authed_http.is_mtls is False @mock.patch( "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True @@ -382,13 +415,8 @@ def test_configure_mtls_channel_cert_loading_exceptions( {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, ): authed_http.configure_mtls_channel() - assert authed_http._is_mtls is False + assert authed_http.is_mtls is False - assert not authed_http._is_mtls - - @mock.patch( - "google.auth.transport._mtls_helper._get_cert_config_path", return_value=None - ) @mock.patch( "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True ) @@ -402,7 +430,7 @@ def test_configure_mtls_channel_cert_loading_exceptions( }, ) def test_configure_mtls_channel_without_client_cert_env( - self, get_client_cert_and_key, mock_get_cert_config_path + self, get_client_cert_and_key ): callback = mock.Mock() @@ -447,7 +475,6 @@ def test_cert_rotation_when_cert_mismatch_and_mtls_endpoint_used( credentials = mock.Mock(wraps=CredentialsStub()) final_response = ResponseStub(status=http_client.OK) - # We simulate the HTTP stub rotation. When mTLS http is created, we return rotated_http. rotated_http = HttpStub([final_response]) mock_make_mutual_tls_http.return_value = rotated_http @@ -457,21 +484,17 @@ def test_cert_rotation_when_cert_mismatch_and_mtls_endpoint_used( credentials, http=http ) - old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" - - # New certificate and key to simulate rotation. + old_cert = b"old_cert" new_cert = pytest.public_cert_bytes new_key = pytest.private_key_bytes - # Set _cached_cert to a callable that returns the old certificate. authed_http._cached_cert = old_cert authed_http._is_mtls = True - # Mock call_client_cert_callback to return the new certificate. + with mock.patch.object( google.auth.transport._mtls_helper, "call_client_cert_callback", return_value=(new_cert, new_key), ) as mock_callback: - # mTLS endpoint is used, and client cert env var is true with mock.patch.dict( os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} ): @@ -479,7 +502,6 @@ def test_cert_rotation_when_cert_mismatch_and_mtls_endpoint_used( "GET", "http://example.mtls.googleapis.com" ) - # Asserts to verify the behavior. assert result == final_response assert credentials.refresh.called assert credentials.refresh.call_count == 1 @@ -500,22 +522,18 @@ def test_no_cert_rotation_when_cert_match_and_mtls_endpoint_used(self): credentials, http=http ) old_cert = CERT_MOCK_VAL - new_cert = old_cert new_key = KEY_MOCK_VAL - # Set _cached_cert to a callable that returns the same certificate. authed_http._cached_cert = old_cert authed_http._is_mtls = True - # Mock call_client_cert_callback to return the certificate. + with mock.patch.object( google.auth.transport._mtls_helper, "call_client_cert_callback", return_value=(new_cert, new_key), ): - # mTLS endpoint is used result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") - # Asserts to verify the behavior. assert credentials.refresh.call_count == 2 assert result.status == final_response.status @@ -536,16 +554,13 @@ def test_no_cert_match_check_when_mtls_endpoint_not_used(self): new_cert = CERT_MOCK_VAL new_key = KEY_MOCK_VAL - # Mock call_client_cert_callback to return the certificate. with mock.patch.object( google.auth.transport._mtls_helper, "call_client_cert_callback", return_value=(new_cert, new_key), ) as mock_callback: - # non-mTLS endpoint is used result = authed_http.urlopen("GET", "http://example.googleapis.com") - # Asserts to verify the behavior. assert not mock_callback.called assert result.status == final_response.status @@ -553,7 +568,6 @@ def test_no_cert_rotation_when_no_unauthorized_response(self): credentials = mock.Mock(wraps=CredentialsStub()) final_response = ResponseStub(status=http_client.UPGRADE_REQUIRED) - # Response is set to code other than 401(Unauthorized). http = HttpStub([ResponseStub(status=http_client.UPGRADE_REQUIRED)]) authed_http = google.auth.transport.urllib3.AuthorizedHttp( credentials, http=http @@ -562,285 +576,64 @@ def test_no_cert_rotation_when_no_unauthorized_response(self): with mock.patch.dict( os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} ): - # mTLS endpoint is used result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") assert result.status == final_response.status assert not credentials.refresh.called assert credentials.refresh.call_count == 0 - def test_cert_rotation_failure_raises_error(self): - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - - old_cert = b"-----BEGIN CERTIFICATE-----\nMIIBdTCCARqgAwIBAgIJAOYVvu/axMxvMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMM\nHEdvb2dsZSBFbmRwb2ludCBWZXJpZmljYXRpb24wHhcNMjUwNzMwMjMwNjA4WhcN\nMjYwNzMxMjMwNjA4WjAnMSUwIwYDVQQDDBxHb29nbGUgRW5kcG9pbnQgVmVyaWZp\nY2F0aW9uMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEbtr18gkEtwPow2oqyZsU\n4KLwFaLFlRlYv55UATS3QTDykDnIufC42TJCnqFRYhwicwpE2jnUV+l9g3Voias8\nraMvMC0wCQYDVR0TBAIwADALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH\nAwIwCgYIKoZIzj0EAwIDSQAwRgIhAKcjW6dmF1YCksXPgDPlPu/nSnOjb3qCcivz\n/Jxq2zoeAiEA7/aNxcEoCGS3hwMIXoaaD/vPcZOOopKSyqXCvxRooKQ=\n-----END CERTIFICATE-----\n" - - # New certificate and key to simulate rotation. - new_cert = CERT_MOCK_VAL - new_key = KEY_MOCK_VAL - authed_http._cached_cert = old_cert - authed_http._is_mtls = True - - # Mock call_client_cert_callback to return the new certificate. - with mock.patch.object( - google.auth.transport._mtls_helper, - "check_parameters_for_unauthorized_response", - return_value=(new_cert, new_key, "old_fingerprint", "new_fingerprint"), - ) as mock_check_params: - with mock.patch.object( - authed_http, - "configure_mtls_channel", - side_effect=Exception("Failed to reconfigure"), - ) as mock_reconfigure: - with pytest.raises(exceptions.MutualTLSChannelError): - authed_http.urlopen("GET", "https://example.mtls.googleapis.com") - - mock_check_params.assert_called_once() - mock_reconfigure.assert_called_once() - credentials.refresh.assert_not_called() - - def test_cert_rotation_check_params_fails(self): - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - authed_http._is_mtls = True - authed_http._cached_cert = b"cached_cert" - - with mock.patch( - "google.auth.transport.urllib3._mtls_helper.check_parameters_for_unauthorized_response", - side_effect=Exception("check_params failed"), - ) as mock_check_params: - with pytest.raises(Exception, match="check_params failed"): - authed_http.urlopen("GET", "http://example.mtls.googleapis.com") - - mock_check_params.assert_called_once() - credentials.refresh.assert_not_called() - - def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self): - """ - Tests that the code can handle a refresh triggered by a status code - other than 401 (UNAUTHORIZED). This covers the 'else' branch of the - 'if response.status_code == http_client.UNAUTHORIZED' check - """ - credentials = mock.Mock(wraps=CredentialsStub()) - # Configure the session to treat 503 (Service Unavailable) as a refreshable error - custom_codes = [http_client.SERVICE_UNAVAILABLE] - - # Return 503 first, then 200 - http = HttpStub( - [ - ResponseStub(status=http_client.SERVICE_UNAVAILABLE), - ResponseStub(status=http_client.OK), - ] - ) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http, refresh_status_codes=custom_codes - ) - - # Enable mTLS to prove it is skipped despite being enabled - authed_http._is_mtls = True - mtls_url = "https://mtls.googleapis.com/test" - - with mock.patch( - "google.auth.transport.urllib3._mtls_helper", autospec=True - ) as mock_helper: - authed_http.urlopen("GET", mtls_url) - - # Assert refresh happened (Outer Check was True) - assert credentials.refresh.called - - # Assert mTLS check logic was SKIPPED (Inner Check was False) - assert not mock_helper.check_parameters_for_unauthorized_response.called - - def test_cert_rotation_skipped_on_non_mtls_url(self): - """ - Tests that mTLS cert rotation is skipped on non-mTLS URLs even if - mTLS is enabled and an UNAUTHORIZED (401) response is received. - """ - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub( - [ - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.OK), - ] - ) - non_mtls_url = "https://storage.googleapis.com/bucket/mtls.googleapis.com" - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http - ) - authed_http._is_mtls = True - authed_http._cached_cert = b"cached_cert" + def test_reauth_lock_acquired_on_unauthorized(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True - with mock.patch.object( - google.auth.transport._mtls_helper, - "check_parameters_for_unauthorized_response", - ) as mock_check_params: - authed_http.urlopen("GET", non_mtls_url) + mock_response_unauth = mock.Mock() + mock_response_unauth.status = http_client.UNAUTHORIZED + mock_response_ok = mock.Mock() + mock_response_ok.status = http_client.OK - # Assert refresh happened - assert credentials.refresh.called + lock_held_during_call = {"held": False} - # Assert mTLS check logic was SKIPPED - assert not mock_check_params.called + def mock_configure_mtls_channel(client_cert_callback=None): + lock_held_during_call["held"] = http_obj._mtls_reauth_lock.locked() - def test_cert_rotation_triggered_on_psc_url(self): - """ - Tests that mTLS cert rotation IS triggered on a Private Service Connect - (PSC) mTLS endpoint when an UNAUTHORIZED (401) response is received. - """ - credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub( - [ - ResponseStub(status=http_client.UNAUTHORIZED), - ResponseStub(status=http_client.OK), - ] + http_obj.configure_mtls_channel = mock.Mock( + side_effect=mock_configure_mtls_channel ) - psc_url = "https://storage.p.googleapis.com/b/my-bucket" - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials, http=http + http_obj.http.urlopen = mock.Mock( + side_effect=[mock_response_unauth, mock_response_ok] ) - authed_http._is_mtls = True - authed_http._cached_cert = b"cached_cert" with mock.patch.object( - google.auth.transport._mtls_helper, + google.auth.transport.urllib3._mtls_helper, "check_parameters_for_unauthorized_response", - return_value=(b"new_cert", b"new_key", "old_fp", "old_fp"), - ) as mock_check_params: - authed_http.urlopen("GET", psc_url) - - # Assert mTLS check logic was called on PSC endpoint - mock_check_params.assert_called_once() - assert credentials.refresh.called - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - def test_configure_mtls_channel_subsequent_failure(self, mock_make_mutual_tls_http): - callback = mock.Mock() - callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + return_value=(b"cert", b"key", "old_fp", "new_fp"), ): - is_mtls = authed_http.configure_mtls_channel(callback) + http_obj.request("GET", "https://example.mtls.googleapis.com/") - assert is_mtls - assert authed_http._is_mtls - - # Subsequent call fails - with mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) as mock_get_client_cert_and_key: - mock_get_client_cert_and_key.side_effect = exceptions.ClientCertError() - - with pytest.raises(exceptions.MutualTLSChannelError): - with mock.patch.dict( - os.environ, - {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"}, - ): - authed_http.configure_mtls_channel() - - # Verify it retains its previous mTLS state and connection pool - assert authed_http._is_mtls - assert isinstance(authed_http.http, mock.Mock) - - @mock.patch("google.auth.transport.urllib3._make_mutual_tls_http", autospec=True) - def test_configure_mtls_channel_subsequent_disabled( - self, mock_make_mutual_tls_http - ): - callback = mock.Mock() - callback.return_value = (pytest.public_cert_bytes, pytest.private_key_bytes) - - authed_http = google.auth.transport.urllib3.AuthorizedHttp( - credentials=mock.Mock() - ) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel(callback) - - assert is_mtls - assert authed_http._is_mtls - - # Subsequent call returns no client certificate (disabled) - with mock.patch( - "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True - ) as mock_get_client_cert_and_key: - mock_get_client_cert_and_key.return_value = (False, None, None) - - with mock.patch.dict( - os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} - ): - is_mtls = authed_http.configure_mtls_channel() - - # Verify mTLS is disabled and standard PoolManager is restored - assert not is_mtls - assert not authed_http._is_mtls - assert isinstance(authed_http.http, urllib3.PoolManager) - - -class TestAuthorizedHttpMTLSReauth: - @mock.patch( - "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" - ) - def test_reauth_lock_acquired_on_unauthorized(self, mock_check_params): - credentials = mock.Mock() - http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) - http_obj._is_mtls = True - http_obj._cached_cert = b"cert" - mock_response = mock.Mock() - mock_response.status = http_client.UNAUTHORIZED - http_obj.http.urlopen = mock.Mock(return_value=mock_response) - real_lock = threading.Lock() - http_obj._reauth_lock = real_lock - mock_check_params.return_value = ( - b"new_cert_bytes", - b"new_key_bytes", - "old_fingerprint", - "new_fingerprint", - ) - lock_held_during_call = {"held": False} - - def verify_lock_held(*args, **kwargs): - lock_held_during_call["held"] = real_lock.locked() - - http_obj.configure_mtls_channel = mock.Mock(side_effect=verify_lock_held) - http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_called() + http_obj.configure_mtls_channel.assert_called_once() assert lock_held_during_call["held"] is True + assert not http_obj._mtls_reauth_lock.locked() - @mock.patch( - "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" - ) - def test_reauth_skipped_when_cert_fingerprint_matches(self, mock_check_params): - credentials = mock.Mock() + def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) http_obj._is_mtls = True - http_obj._cached_cert = b"cert" + mock_response_unauth = mock.Mock() mock_response_unauth.status = http_client.UNAUTHORIZED mock_response_ok = mock.Mock() mock_response_ok.status = http_client.OK + http_obj.http.urlopen = mock.Mock( side_effect=[mock_response_unauth, mock_response_ok] ) - mock_check_params.return_value = ( - b"same_cert_bytes", - b"same_key_bytes", - "same_fingerprint", - "same_fingerprint", - ) - http_obj.configure_mtls_channel = mock.Mock() - http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_not_called() + + with mock.patch.object( + google.auth.transport.urllib3._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=Exception("Certificate discovery failed"), + ): + http_obj.request("GET", "https://example.mtls.googleapis.com/") + + credentials.refresh.assert_called_once() + assert not http_obj._mtls_reauth_lock.locked() From 555a6f55db46ba61554860b79a4c7befa78ce33b Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 14:25:20 -0700 Subject: [PATCH 26/34] fix: Update import path for compliance module --- packages/google-auth/tests/transport/test_urllib3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 13a9af21de49..6f35f170ddfc 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -22,7 +22,7 @@ from google.auth import environment_vars from google.auth import exceptions import google.auth.credentials -from google.auth.tests.transport import compliance +from tests.transport import compliance import google.auth.transport._mtls_helper import google.auth.transport.urllib3 from google.oauth2 import service_account From cbe925ca461197487d308dae2276c0279621a862 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Wed, 9 Sep 2026 14:26:36 -0700 Subject: [PATCH 27/34] chore: Refactor reauth lock test for unauthorized responses Refactor test for reauthentication lock during unauthorized response handling. --- .../tests/transport/test_requests.py | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 1e6389e68611..1a2bc75f9af6 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1177,37 +1177,31 @@ class TestAuthorizedSessionMTLSReauth: ) @mock.patch("google.auth.transport.requests.requests.Session.request") def test_reauth_lock_acquired_on_unauthorized(self): - credentials = mock.Mock(spec=google.auth.credentials.Credentials) - session = requests.AuthorizedSession(credentials) - session._is_mtls = True - - mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) - mock_success_response = mock.Mock(status_code=http_client.OK) - - lock_held_during_call = {"held": False} - - def mock_configure_mtls_channel(callback): - # Verify the lock is held during channel reconfiguration - lock_held_during_call["held"] = session._mtls_reauth_lock.locked() - - session.configure_mtls_channel = mock.Mock(side_effect=mock_configure_mtls_channel) - - with mock.patch.object( - requests._mtls_helper, - "check_parameters_for_unauthorized_response", - return_value=(b"cert", b"key", "old_fp", "new_fp"), - ): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + session = requests.AuthorizedSession(credentials) + session._is_mtls = True + mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) + mock_success_response = mock.Mock(status_code=http_client.OK) + lock_held_during_call = {"held": False} + def mock_configure_mtls_channel(callback): + lock_held_during_call["held"] = session._mtls_reauth_lock.locked() + session.configure_mtls_channel = mock.Mock( + side_effect=mock_configure_mtls_channel + ) with mock.patch.object( - super(requests.AuthorizedSession, session), - "request", - side_effect=[mock_response, mock_success_response], + requests._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=(b"cert", b"key", "old_fp", "new_fp"), ): - session.request("GET", "https://example.mtls.googleapis.com/") - - session.configure_mtls_channel.assert_called_once() - assert lock_held_during_call["held"] is True - # Verify the lock is released after request finishes - assert not session._mtls_reauth_lock.locked() + with mock.patch.object( + super(requests.AuthorizedSession, session), + "request", + side_effect=[mock_response, mock_success_response], + ): + session.request("GET", "https://example.mtls.googleapis.com/") + session.configure_mtls_channel.assert_called_once() + assert lock_held_during_call["held"] is True + assert not session._mtls_reauth_lock.locked() @mock.patch( "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" From 7d70b53c3381aca042cf10e96b49c28da1ec9513 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 6 Oct 2026 20:23:53 +0000 Subject: [PATCH 28/34] fix: Fix the lint errors Signed-off-by: Radhika Agrawal --- .../google-auth/google/auth/transport/requests.py | 14 ++++++++++---- .../google-auth/google/auth/transport/urllib3.py | 4 +++- .../google-auth/tests/transport/test_requests.py | 2 ++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 709c9317b4e2..a3fce4b9ad16 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -760,7 +760,7 @@ def request( cached_fingerprint, current_cert_fingerprint, ) = _mtls_helper.check_parameters_for_unauthorized_response( - getattr(self, "_cached_cert", None) + getattr(self, "_cached_cert", None) ) if cached_fingerprint != current_cert_fingerprint: try: @@ -769,11 +769,15 @@ def request( "channel." ) self.configure_mtls_channel( - lambda: (call_cert_bytes, call_key_bytes) + lambda: ( + call_cert_bytes, + call_key_bytes, + ) ) except Exception as e: _LOGGER.error( - "Failed to reconfigure mTLS channel: %s", e + "Failed to reconfigure mTLS channel: %s", + e, ) raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" @@ -786,7 +790,9 @@ def request( except exceptions.MutualTLSChannelError: raise except Exception as e: - _LOGGER.warning("mTLS certificate check/rotation failed: %s", e) + _LOGGER.warning( + "mTLS certificate check/rotation failed: %s", e + ) remaining_time = guard.remaining_timeout _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 4603dd8d3d7b..b402951d63d6 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -437,7 +437,9 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: - use_mtls = getattr(self, "_is_mtls", False) and _mtls_helper.is_mtls_endpoint(url) + use_mtls = getattr( + self, "_is_mtls", False + ) and _mtls_helper.is_mtls_endpoint(url) if use_mtls: with self._mtls_reauth_lock: diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 03cecfcde083..7d6d89bb814f 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1523,8 +1523,10 @@ def test_reauth_lock_acquired_on_unauthorized(self): mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) mock_success_response = mock.Mock(status_code=http_client.OK) lock_held_during_call = {"held": False} + def mock_configure_mtls_channel(callback): lock_held_during_call["held"] = session._mtls_reauth_lock.locked() + session.configure_mtls_channel = mock.Mock( side_effect=mock_configure_mtls_channel ) From d9c665f5dd401fa4ec396f77ef28b7217bc17925 Mon Sep 17 00:00:00 2001 From: agrawalradhika-cell Date: Tue, 6 Oct 2026 14:00:21 -0700 Subject: [PATCH 29/34] fix: Refactor test_requests.py for credential handling Updated tests to handle multiple responses and assert credential refresh behavior. --- .../tests/transport/test_requests.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 7d6d89bb814f..b3a9fc32a76e 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1225,7 +1225,10 @@ def test_cert_rotation_failure_raises_error(self): def test_cert_rotation_check_params_fails(self): credentials = mock.Mock(wraps=CredentialsStub()) - adapter = AdapterStub([make_response(status=http_client.UNAUTHORIZED)]) + adapter = AdapterStub([ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK) + ]) authed_session = google.auth.transport.requests.AuthorizedSession( credentials, refresh_timeout=60 @@ -1238,11 +1241,11 @@ def test_cert_rotation_check_params_fails(self): "google.auth.transport.requests._mtls_helper.check_parameters_for_unauthorized_response", side_effect=Exception("check_params failed"), ) as mock_check_params: - with pytest.raises(Exception, match="check_params failed"): - authed_session.request("GET", self.MTLS_TEST_URL) + response = authed_session.request("GET", self.MTLS_TEST_URL) mock_check_params.assert_called_once() - credentials.refresh.assert_not_called() + assert credentials.refresh.called + assert response.status_code == http_client.OK def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self): """ @@ -1423,7 +1426,7 @@ def test_configure_mtls_channel_subsequent_disabled(self): def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): credentials = mock.Mock(spec=google.auth.credentials.Credentials) - session = requests.AuthorizedSession(credentials) + session = google.auth.transport.requests.AuthorizedSession(credentials) session._is_mtls = True mock_response_unauth = mock.Mock(status_code=http_client.UNAUTHORIZED) mock_response_ok = mock.Mock(status_code=http_client.OK) @@ -1434,7 +1437,7 @@ def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): side_effect=Exception("Certificate discovery failed"), ): with mock.patch.object( - super(requests.AuthorizedSession, session), + super(google.auth.transport.requests.AuthorizedSession, session), "request", side_effect=[mock_response_unauth, mock_response_ok], ): @@ -1516,7 +1519,7 @@ class TestAuthorizedSessionMTLSReauth: "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" ) @mock.patch("google.auth.transport.requests.requests.Session.request") - def test_reauth_lock_acquired_on_unauthorized(self): + def test_reauth_lock_acquired_on_unauthorized(self, *args): credentials = mock.Mock(spec=google.auth.credentials.Credentials) session = requests.AuthorizedSession(credentials) session._is_mtls = True From 2d911c29de50c3ae8e9c67755bf1f519a37602a3 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Fri, 30 Oct 2026 21:33:13 +0000 Subject: [PATCH 30/34] fix(auth): replace adapters atomically during mTLS cert rotation and test real reconfiguration - Move the 401 cert check and channel reconfiguration into _reconfigure_mtls_channel_if_cert_changed() under _mtls_reauth_lock in both AuthorizedSession and AuthorizedHttp, skipping the cert read when another thread already rotated it. - configure_mtls_channel() no longer calls Session.mount(), which mutated session.adapters in place and could raise 'OrderedDict mutated during iteration' in concurrent get_adapter() calls. A new _replace_adapter() helper swaps in an updated copy for both the session and the auth request session. - Concurrency tests now run the real configure_mtls_channel() and stub only the network layer. --- .../google/auth/transport/requests.py | 164 +++++++---- .../google/auth/transport/urllib3.py | 122 +++++--- .../tests/transport/test_requests.py | 271 +++++++++++++++--- .../tests/transport/test_urllib3.py | 231 ++++++++++++++- 4 files changed, 649 insertions(+), 139 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index a3fce4b9ad16..fe62dcedf9ea 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -116,6 +116,31 @@ def __exit__(self, exc_type, exc_value, traceback): raise self._timeout_error_type() +def _replace_adapter(session, prefix, adapter): + """Mounts an adapter on a session without mutating its adapters in place. + + ``adapter`` is mounted on ``session`` for ``prefix``. + :meth:`requests.Session.mount` mutates the ``session.adapters`` + OrderedDict in place. If another thread is iterating over it in + :meth:`requests.Session.get_adapter` at the same time, that thread fails + with ``RuntimeError: OrderedDict mutated during iteration``. This builds + an updated copy and assigns it in one step instead, so threads that are + already iterating over the old dictionary are not affected. + + Args: + session (requests.Session): The session to mount the adapter on. + prefix (str): The URL prefix to mount the adapter for. + adapter (requests.adapters.BaseAdapter): The adapter to mount. + """ + new_adapters = session.adapters.copy() + new_adapters[prefix] = adapter + # Keep the longest prefixes first, as requests.Session.mount() does, so + # that get_adapter() matches the most specific prefix. + session.adapters = collections.OrderedDict( + sorted(new_adapters.items(), key=lambda item: len(item[0]), reverse=True) + ) + + class Request(transport.Request): """Requests request adapter. @@ -213,18 +238,7 @@ def _configure_mtls_if_needed(self, url): pool_block=session_adapter._pool_block, ) - # Replace session.adapters atomically instead of calling session.mount(), - # which mutates the OrderedDict in place and can raise RuntimeError if - # another thread is iterating over session.adapters in get_adapter(). - new_adapters = self.session.adapters.copy() - new_adapters[prefix] = self._mtls_adapter - self.session.adapters = collections.OrderedDict( - sorted( - new_adapters.items(), - key=lambda item: len(item[0]), - reverse=True, - ) - ) + _replace_adapter(self.session, prefix, self._mtls_adapter) def __call__( self, @@ -559,9 +573,9 @@ def configure_mtls_channel(self, client_cert_callback=None): will be used. .. warning:: - Calling this method mutates the underlying `requests.Session` adapter - dictionary. It is not thread-safe to call this explicitly while other - threads are making requests. + Calling this method replaces the underlying `requests.Session` + adapters and closes the previous ones. It is not thread-safe to call + this explicitly while other threads are making requests. Raises: google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel @@ -642,13 +656,13 @@ def configure_mtls_channel(self, client_cert_callback=None): new_exc = exceptions.MutualTLSChannelError(caught_exc) raise new_exc from caught_exc - self.mount("https://", new_adapter) + _replace_adapter(self, "https://", new_adapter) if old_adapter is not None and old_adapter is not new_adapter: old_adapter.close() if self._auth_request_session is not None and new_auth_adapter is not None: - self._auth_request_session.mount("https://", new_auth_adapter) + _replace_adapter(self._auth_request_session, "https://", new_auth_adapter) if ( old_auth_adapter is not None @@ -725,6 +739,11 @@ def request( self.credentials.before_request(auth_request, method, url, request_headers) remaining_time = guard.remaining_timeout + # Remember the client certificate that this request is sent with. If + # the request receives a 401 response, this tells whether another + # thread has already rotated the certificate in the meantime. + cert_at_request = getattr(self, "_cached_cert", None) + with TimeoutGuard(remaining_time) as guard: _helpers.request_log(_LOGGER, method, url, data, headers) response = super(AuthorizedSession, self).request( @@ -751,48 +770,7 @@ def request( use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: with TimeoutGuard(remaining_time) as guard: - with self._mtls_reauth_lock: - if getattr(self, "is_mtls", False): - try: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - getattr(self, "_cached_cert", None) - ) - if cached_fingerprint != current_cert_fingerprint: - try: - _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." - ) - self.configure_mtls_channel( - lambda: ( - call_cert_bytes, - call_key_bytes, - ) - ) - except Exception as e: - _LOGGER.error( - "Failed to reconfigure mTLS channel: %s", - e, - ) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the client" - " certificate has not changed." - ) - except exceptions.MutualTLSChannelError: - raise - except Exception as e: - _LOGGER.warning( - "mTLS certificate check/rotation failed: %s", e - ) + self._reconfigure_mtls_channel_if_cert_changed(cert_at_request) remaining_time = guard.remaining_timeout _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", @@ -828,6 +806,74 @@ def request( return response + def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): + """Reconfigures the mTLS channel if the client certificate has changed. + + This is called when a request to an mTLS endpoint receives a 401 + response. The certificate check and the channel reconfiguration run + while holding ``_mtls_reauth_lock``. When several threads receive a 401 + response at the same time, only the first one reads the current + certificate and remounts the adapters. The other threads see that the + channel has already been reconfigured and reuse it. + + Args: + cert_at_request (Optional[bytes]): The cached client certificate at + the time the request that received the 401 response was sent. + + Raises: + google.auth.exceptions.MutualTLSChannelError: If the client + certificate has changed but the mTLS channel could not be + reconfigured. + """ + with self._mtls_reauth_lock: + if not self.is_mtls: + # mTLS was disabled by another thread while this one waited. + return + + if getattr(self, "_cached_cert", None) != cert_at_request: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because it has already" + " been reconfigured with a new client certificate." + ) + return + + try: + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + cert_at_request + ) + except Exception as e: + # Failing to load the current certificate (for example, because + # the certificate file is missing or the certificate provider + # failed) must not prevent the token refresh that follows. + _LOGGER.warning( + "Failed to check whether the client certificate has changed: %s", + e, + ) + return + + if cached_fingerprint == current_cert_fingerprint: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because the client" + " certificate has not changed." + ) + return + + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS channel." + ) + self.configure_mtls_channel(lambda: (call_cert_bytes, call_key_bytes)) + except Exception as e: + _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + @property def is_mtls(self): """Indicates if the created SSL channel is mutual TLS.""" diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index b402951d63d6..0659213291bb 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -421,6 +421,11 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): self.credentials.before_request(self._request, method, url, request_headers) + # Remember the client certificate that this request is sent with. If + # the request receives a 401 response, this tells whether another + # thread has already rotated the certificate in the meantime. + cert_at_request = getattr(self, "_cached_cert", None) + response = self.http.urlopen( method, url, body=body, headers=request_headers, **kwargs ) @@ -437,51 +442,10 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): and _credential_refresh_attempt < self._max_refresh_attempts ): if response.status == http_client.UNAUTHORIZED: - use_mtls = getattr( - self, "_is_mtls", False - ) and _mtls_helper.is_mtls_endpoint(url) - + use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: - with self._mtls_reauth_lock: - if getattr(self, "is_mtls", getattr(self, "_is_mtls", False)): - try: - ( - call_cert_bytes, - call_key_bytes, - cached_fingerprint, - current_cert_fingerprint, - ) = _mtls_helper.check_parameters_for_unauthorized_response( - getattr(self, "_cached_cert", None) - ) - if cached_fingerprint != current_cert_fingerprint: - try: - _LOGGER.info( - "Client certificate has changed, reconfiguring mTLS " - "channel." - ) - self.configure_mtls_channel( - client_cert_callback=lambda: ( - call_cert_bytes, - call_key_bytes, - ) - ) - except Exception as e: - _LOGGER.error( - "Failed to reconfigure mTLS channel: %s", e - ) - raise exceptions.MutualTLSChannelError( - "Failed to reconfigure mTLS channel" - ) from e - else: - _LOGGER.info( - "Skipping reconfiguration of mTLS channel because the " - "client certificate has not changed." - ) - except Exception as e: - _LOGGER.warning( - "Failed to check parameters or reconfigure mTLS channel: %s", - e, - ) + self._reconfigure_mtls_channel_if_cert_changed(cert_at_request) + _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", response.status, @@ -503,6 +467,76 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): return response + def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): + """Reconfigures the mTLS channel if the client certificate has changed. + + This is called when a request to an mTLS endpoint receives a 401 + response. The certificate check and the channel reconfiguration run + while holding ``_mtls_reauth_lock``. When several threads receive a 401 + response at the same time, only the first one reads the current + certificate and replaces the pool manager. The other threads see that + the channel has already been reconfigured and reuse it. + + Args: + cert_at_request (Optional[bytes]): The cached client certificate at + the time the request that received the 401 response was sent. + + Raises: + google.auth.exceptions.MutualTLSChannelError: If the client + certificate has changed but the mTLS channel could not be + reconfigured. + """ + with self._mtls_reauth_lock: + if not self.is_mtls: + # mTLS was disabled by another thread while this one waited. + return + + if getattr(self, "_cached_cert", None) != cert_at_request: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because it has already" + " been reconfigured with a new client certificate." + ) + return + + try: + ( + call_cert_bytes, + call_key_bytes, + cached_fingerprint, + current_cert_fingerprint, + ) = _mtls_helper.check_parameters_for_unauthorized_response( + cert_at_request + ) + except Exception as e: + # Failing to load the current certificate (for example, because + # the certificate file is missing or the certificate provider + # failed) must not prevent the token refresh that follows. + _LOGGER.warning( + "Failed to check whether the client certificate has changed: %s", + e, + ) + return + + if cached_fingerprint == current_cert_fingerprint: + _LOGGER.info( + "Skipping reconfiguration of mTLS channel because the " + "client certificate has not changed." + ) + return + + try: + _LOGGER.info( + "Client certificate has changed, reconfiguring mTLS channel." + ) + self.configure_mtls_channel( + client_cert_callback=lambda: (call_cert_bytes, call_key_bytes) + ) + except Exception as e: + _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + raise exceptions.MutualTLSChannelError( + "Failed to reconfigure mTLS channel" + ) from e + # Proxy methods for compliance with the urllib3.PoolManager interface def __enter__(self): diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index d019417d64a5..05afe93ee767 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1225,10 +1225,12 @@ def test_cert_rotation_failure_raises_error(self): def test_cert_rotation_check_params_fails(self): credentials = mock.Mock(wraps=CredentialsStub()) - adapter = AdapterStub([ - make_response(status=http_client.UNAUTHORIZED), - make_response(status=http_client.OK) - ]) + adapter = AdapterStub( + [ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK), + ] + ) authed_session = google.auth.transport.requests.AuthorizedSession( credentials, refresh_timeout=60 @@ -1438,26 +1440,234 @@ def test_configure_mtls_channel_subsequent_disabled(self): ) def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): - credentials = mock.Mock(spec=google.auth.credentials.Credentials) - session = google.auth.transport.requests.AuthorizedSession(credentials) - session._is_mtls = True - mock_response_unauth = mock.Mock(status_code=http_client.UNAUTHORIZED) - mock_response_ok = mock.Mock(status_code=http_client.OK) + credentials = mock.Mock(wraps=CredentialsStub()) + adapter = AdapterStub( + [ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK), + ] + ) + authed_session = google.auth.transport.requests.AuthorizedSession(credentials) + authed_session.mount(self.MTLS_TEST_URL, adapter) + authed_session._is_mtls = True # Simulate discovery failure (e.g. Enterprise cert provider error or file missing) with mock.patch.object( - requests._mtls_helper, + google.auth.transport._mtls_helper, "check_parameters_for_unauthorized_response", side_effect=Exception("Certificate discovery failed"), ): - with mock.patch.object( - super(google.auth.transport.requests.AuthorizedSession, session), - "request", - side_effect=[mock_response_unauth, mock_response_ok], - ): - session.request("GET", "https://example.mtls.googleapis.com/") + result = authed_session.request("GET", self.MTLS_TEST_URL) # Token refresh should still be called despite discovery failure credentials.refresh.assert_called_once() - assert not session._mtls_reauth_lock.locked() + assert result.status_code == http_client.OK + assert not authed_session._mtls_reauth_lock.locked() + + def test_cert_check_skipped_when_cert_rotated_during_request(self): + """ + Tests that a request that receives a 401 response does not read the + client certificate again if another thread has already rotated it + while the request was in flight. + """ + credentials = mock.Mock(wraps=CredentialsStub()) + adapter = AdapterStub( + [ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK), + ] + ) + authed_session = google.auth.transport.requests.AuthorizedSession( + credentials, refresh_timeout=60 + ) + authed_session.mount(self.MTLS_TEST_URL, adapter) + authed_session._is_mtls = True + authed_session._cached_cert = b"old_cert" + send_request = adapter.send + + def send_request_and_rotate_cert(request, **kwargs): + # Simulate another thread rotating the client certificate while + # this request is in flight. + authed_session._cached_cert = b"new_cert" + return send_request(request, **kwargs) + + adapter.send = send_request_and_rotate_cert + + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + ) as mock_check_params: + with mock.patch.object( + authed_session, "configure_mtls_channel" + ) as mock_configure_mtls_channel: + result = authed_session.request("GET", self.MTLS_TEST_URL) + + assert result.status_code == http_client.OK + mock_check_params.assert_not_called() + mock_configure_mtls_channel.assert_not_called() + credentials.refresh.assert_called_once() + + def test_reconfigure_mtls_channel_skipped_if_mtls_disabled(self): + """ + Tests that the client certificate is not checked if another thread + disabled mTLS while this thread was waiting for the lock. + """ + authed_session = google.auth.transport.requests.AuthorizedSession( + mock.sentinel.credentials + ) + authed_session._is_mtls = False + + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + ) as mock_check_params: + authed_session._reconfigure_mtls_channel_if_cert_changed(b"old_cert") + + mock_check_params.assert_not_called() + assert not authed_session._mtls_reauth_lock.locked() + + @mock.patch.object(requests.adapters.HTTPAdapter, "send", autospec=True) + def test_concurrent_unauthorized_responses_reconfigure_mtls_channel_once( + self, mock_send + ): + """ + Tests that when several threads that share an AuthorizedSession receive + a 401 response at the same time, the client certificate is read and the + real mTLS channel reconfiguration runs only once, and every request is + retried successfully through the newly mounted mTLS adapter. + """ + num_threads = 8 + old_cert = b"old_cert" + all_requests_sent = threading.Barrier(num_threads) + + authed_session = google.auth.transport.requests.AuthorizedSession( + CredentialsStub(), refresh_timeout=60 + ) + authed_session._is_mtls = True + authed_session._cached_cert = old_cert + old_adapter = authed_session.adapters["https://"] + old_auth_adapter = authed_session._auth_request_session.adapters["https://"] + + def send(adapter, request, **kwargs): + if isinstance(adapter, google.auth.transport.requests._MutualTlsAdapter): + return make_response(status=http_client.OK) + # Wait until every thread has sent its request through the old + # adapter, so that all of them receive a 401 response. + all_requests_sent.wait(timeout=30) + return make_response(status=http_client.UNAUTHORIZED) + + mock_send.side_effect = send + + def check_parameters_for_unauthorized_response(cached_cert): + cached_fingerprint = "old_fp" if cached_cert == old_cert else "new_fp" + return ( + pytest.public_cert_bytes, + pytest.private_key_bytes, + cached_fingerprint, + "new_fp", + ) + + results = [] + errors = [] + + def make_request(): + try: + results.append(authed_session.request("GET", self.MTLS_TEST_URL)) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=check_parameters_for_unauthorized_response, + ) as mock_check_params, + mock.patch.object( + authed_session, + "configure_mtls_channel", + wraps=authed_session.configure_mtls_channel, + ) as spy_configure_mtls_channel, + ): + threads = [ + threading.Thread(target=make_request) for _ in range(num_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=60) + + assert errors == [] + assert [r.status_code for r in results] == [http_client.OK] * num_threads + mock_check_params.assert_called_once() + spy_configure_mtls_channel.assert_called_once() + assert authed_session.is_mtls + assert authed_session._cached_cert == pytest.public_cert_bytes + new_adapter = authed_session.adapters["https://"] + new_auth_adapter = authed_session._auth_request_session.adapters["https://"] + assert isinstance(new_adapter, google.auth.transport.requests._MutualTlsAdapter) + assert isinstance( + new_auth_adapter, google.auth.transport.requests._MutualTlsAdapter + ) + assert new_adapter is not old_adapter + assert new_auth_adapter is not old_auth_adapter + assert not authed_session._mtls_reauth_lock.locked() + + @mock.patch( + "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True + ) + def test_configure_mtls_channel_does_not_mutate_adapters_in_place( + self, mock_get_client_cert_and_key + ): + """ + Tests that configure_mtls_channel replaces the adapters dictionaries of + the session and of the auth request session instead of mutating them, + so that a thread iterating over them in get_adapter() does not fail + with "OrderedDict mutated during iteration". + """ + mock_get_client_cert_and_key.return_value = ( + True, + pytest.public_cert_bytes, + pytest.private_key_bytes, + ) + authed_session = google.auth.transport.requests.AuthorizedSession( + credentials=mock.Mock() + ) + custom_adapter = AdapterStub([]) + authed_session.mount(self.MTLS_TEST_URL, custom_adapter) + auth_session = authed_session._auth_request_session + + adapters_before = authed_session.adapters + adapters_snapshot = adapters_before.copy() + adapters_iter = iter(adapters_before.items()) + next(adapters_iter) + auth_adapters_before = auth_session.adapters + auth_adapters_snapshot = auth_adapters_before.copy() + auth_adapters_iter = iter(auth_adapters_before.items()) + next(auth_adapters_iter) + + with mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ): + authed_session.configure_mtls_channel() + + # Iterators over the previous dictionaries are still valid. + list(adapters_iter) + list(auth_adapters_iter) + assert adapters_before == adapters_snapshot + assert auth_adapters_before == auth_adapters_snapshot + assert authed_session.adapters is not adapters_before + assert auth_session.adapters is not auth_adapters_before + assert isinstance( + authed_session.get_adapter("https://example.com/"), + google.auth.transport.requests._MutualTlsAdapter, + ) + assert isinstance( + auth_session.get_adapter("https://example.com/"), + google.auth.transport.requests._MutualTlsAdapter, + ) + # The more specific prefix mounted earlier still takes precedence. + assert authed_session.get_adapter(self.MTLS_TEST_URL) is custom_adapter class TestMutualTlsOffloadAdapter(object): @@ -1532,12 +1742,17 @@ class TestAuthorizedSessionMTLSReauth: "google.auth.transport._mtls_helper.check_parameters_for_unauthorized_response" ) @mock.patch("google.auth.transport.requests.requests.Session.request") - def test_reauth_lock_acquired_on_unauthorized(self, *args): + def test_reauth_lock_acquired_on_unauthorized( + self, mock_session_request, mock_check_params + ): credentials = mock.Mock(spec=google.auth.credentials.Credentials) - session = requests.AuthorizedSession(credentials) + session = google.auth.transport.requests.AuthorizedSession(credentials) session._is_mtls = True - mock_response = mock.Mock(status_code=http_client.UNAUTHORIZED) - mock_success_response = mock.Mock(status_code=http_client.OK) + mock_session_request.side_effect = [ + mock.Mock(status_code=http_client.UNAUTHORIZED), + mock.Mock(status_code=http_client.OK), + ] + mock_check_params.return_value = (b"cert", b"key", "old_fp", "new_fp") lock_held_during_call = {"held": False} def mock_configure_mtls_channel(callback): @@ -1546,17 +1761,9 @@ def mock_configure_mtls_channel(callback): session.configure_mtls_channel = mock.Mock( side_effect=mock_configure_mtls_channel ) - with mock.patch.object( - requests._mtls_helper, - "check_parameters_for_unauthorized_response", - return_value=(b"cert", b"key", "old_fp", "new_fp"), - ): - with mock.patch.object( - super(requests.AuthorizedSession, session), - "request", - side_effect=[mock_response, mock_success_response], - ): - session.request("GET", "https://example.mtls.googleapis.com/") + + session.request("GET", "https://example.mtls.googleapis.com/") + session.configure_mtls_channel.assert_called_once() assert lock_held_during_call["held"] is True assert not session._mtls_reauth_lock.locked() diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 7ffd50aeee32..2abc39c7c10f 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -14,6 +14,7 @@ import http.client as http_client import os +import threading from unittest import mock import pytest # type: ignore @@ -124,6 +125,14 @@ def test_authed_http_defaults(self): assert authed_http.credentials == mock.sentinel.credentials assert isinstance(authed_http.http, urllib3.PoolManager) + assert not authed_http.is_mtls + + def test_is_mtls_property(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + assert http_obj.is_mtls is False + http_obj._is_mtls = True + assert http_obj.is_mtls is True def test_urlopen_no_refresh(self): credentials = mock.Mock(wraps=CredentialsStub()) @@ -602,7 +611,12 @@ def test_cert_rotation_failure_raises_error(self): def test_cert_rotation_check_params_fails(self): credentials = mock.Mock(wraps=CredentialsStub()) - http = HttpStub([ResponseStub(status=http_client.UNAUTHORIZED)]) + http = HttpStub( + [ + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.OK), + ] + ) authed_http = google.auth.transport.urllib3.AuthorizedHttp( credentials, http=http @@ -614,11 +628,13 @@ def test_cert_rotation_check_params_fails(self): "google.auth.transport.urllib3._mtls_helper.check_parameters_for_unauthorized_response", side_effect=Exception("check_params failed"), ) as mock_check_params: - with pytest.raises(Exception, match="check_params failed"): - authed_http.urlopen("GET", "http://example.mtls.googleapis.com") + result = authed_http.urlopen("GET", "http://example.mtls.googleapis.com") + # A failure to check the certificate must not prevent the token + # refresh and retry. mock_check_params.assert_called_once() - credentials.refresh.assert_not_called() + credentials.refresh.assert_called_once() + assert result.status == http_client.OK def test_cert_rotation_logic_skipped_on_other_refresh_status_codes(self): """ @@ -799,3 +815,210 @@ def test_configure_mtls_channel_subsequent_disabled( assert not is_mtls assert not authed_http._is_mtls assert isinstance(authed_http.http, urllib3.PoolManager) + + def test_reauth_lock_acquired_on_unauthorized(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True + + mock_response_unauth = mock.Mock() + mock_response_unauth.status = http_client.UNAUTHORIZED + mock_response_ok = mock.Mock() + mock_response_ok.status = http_client.OK + + lock_held_during_call = {"held": False} + + def mock_configure_mtls_channel(client_cert_callback=None): + lock_held_during_call["held"] = http_obj._mtls_reauth_lock.locked() + + http_obj.configure_mtls_channel = mock.Mock( + side_effect=mock_configure_mtls_channel + ) + http_obj.http.urlopen = mock.Mock( + side_effect=[mock_response_unauth, mock_response_ok] + ) + + with mock.patch.object( + google.auth.transport.urllib3._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=(b"cert", b"key", "old_fp", "new_fp"), + ): + http_obj.request("GET", "https://example.mtls.googleapis.com/") + + http_obj.configure_mtls_channel.assert_called_once() + assert lock_held_during_call["held"] is True + assert not http_obj._mtls_reauth_lock.locked() + + def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): + credentials = mock.Mock(spec=google.auth.credentials.Credentials) + http_obj = google.auth.transport.urllib3.AuthorizedHttp(credentials) + http_obj._is_mtls = True + + mock_response_unauth = mock.Mock() + mock_response_unauth.status = http_client.UNAUTHORIZED + mock_response_ok = mock.Mock() + mock_response_ok.status = http_client.OK + + http_obj.http.urlopen = mock.Mock( + side_effect=[mock_response_unauth, mock_response_ok] + ) + + # Simulate discovery failure (e.g. Enterprise cert provider error or file missing) + with mock.patch.object( + google.auth.transport.urllib3._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=Exception("Certificate discovery failed"), + ): + http_obj.request("GET", "https://example.mtls.googleapis.com/") + + # Token refresh should still be called despite discovery failure + credentials.refresh.assert_called_once() + assert not http_obj._mtls_reauth_lock.locked() + + def test_cert_check_skipped_when_cert_rotated_during_request(self): + """ + Tests that a request that receives a 401 response does not read the + client certificate again if another thread has already rotated it + while the request was in flight. + """ + credentials = mock.Mock(wraps=CredentialsStub()) + http = HttpStub( + [ + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.OK), + ] + ) + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + credentials, http=http + ) + authed_http._is_mtls = True + authed_http._cached_cert = b"old_cert" + send_request = http.urlopen + + def send_request_and_rotate_cert(*args, **kwargs): + # Simulate another thread rotating the client certificate while + # this request is in flight. + authed_http._cached_cert = b"new_cert" + return send_request(*args, **kwargs) + + http.urlopen = send_request_and_rotate_cert + + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + ) as mock_check_params: + with mock.patch.object( + authed_http, "configure_mtls_channel" + ) as mock_configure_mtls_channel: + result = authed_http.urlopen( + "GET", "https://example.mtls.googleapis.com" + ) + + assert result.status == http_client.OK + mock_check_params.assert_not_called() + mock_configure_mtls_channel.assert_not_called() + credentials.refresh.assert_called_once() + + def test_reconfigure_mtls_channel_skipped_if_mtls_disabled(self): + """ + Tests that the client certificate is not checked if another thread + disabled mTLS while this thread was waiting for the lock. + """ + authed_http = google.auth.transport.urllib3.AuthorizedHttp( + mock.sentinel.credentials + ) + authed_http._is_mtls = False + + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + ) as mock_check_params: + authed_http._reconfigure_mtls_channel_if_cert_changed(b"old_cert") + + mock_check_params.assert_not_called() + assert not authed_http._mtls_reauth_lock.locked() + + @mock.patch.object(urllib3.PoolManager, "urlopen", autospec=True) + def test_concurrent_unauthorized_responses_reconfigure_mtls_channel_once( + self, mock_urlopen + ): + """ + Tests that when several threads that share an AuthorizedHttp receive a + 401 response at the same time, the client certificate is read and the + real mTLS channel reconfiguration runs only once, and every request is + retried successfully through the new mTLS pool manager. + """ + num_threads = 8 + old_cert = b"old_cert" + all_requests_sent = threading.Barrier(num_threads) + + authed_http = google.auth.transport.urllib3.AuthorizedHttp(CredentialsStub()) + authed_http._is_mtls = True + authed_http._cached_cert = old_cert + old_http = authed_http.http + + def urlopen(pool_manager, method, url, body=None, headers=None, **kwargs): + if "ssl_context" in pool_manager.connection_pool_kw: + # Pool manager created by _make_mutual_tls_http(). + return ResponseStub(status=http_client.OK) + # Wait until every thread has sent its request through the old + # pool manager, so that all of them receive a 401 response. + all_requests_sent.wait(timeout=30) + return ResponseStub(status=http_client.UNAUTHORIZED) + + mock_urlopen.side_effect = urlopen + + def check_parameters_for_unauthorized_response(cached_cert): + cached_fingerprint = "old_fp" if cached_cert == old_cert else "new_fp" + return ( + pytest.public_cert_bytes, + pytest.private_key_bytes, + cached_fingerprint, + "new_fp", + ) + + results = [] + errors = [] + + def make_request(): + try: + results.append( + authed_http.urlopen("GET", "https://example.mtls.googleapis.com") + ) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=check_parameters_for_unauthorized_response, + ) as mock_check_params, + mock.patch.object( + authed_http, + "configure_mtls_channel", + wraps=authed_http.configure_mtls_channel, + ) as spy_configure_mtls_channel, + ): + threads = [ + threading.Thread(target=make_request) for _ in range(num_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=60) + + assert errors == [] + assert [r.status for r in results] == [http_client.OK] * num_threads + mock_check_params.assert_called_once() + spy_configure_mtls_channel.assert_called_once() + assert authed_http.is_mtls + assert authed_http._cached_cert == pytest.public_cert_bytes + assert authed_http.http is not old_http + assert "ssl_context" in authed_http.http.connection_pool_kw + # Token refreshes use the new pool manager too. + assert authed_http._request.http is authed_http.http + assert not authed_http._mtls_reauth_lock.locked() From cd00fcbcf38e3e0cf737f5fa6640a62118b03fca Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Fri, 30 Oct 2026 22:11:48 +0000 Subject: [PATCH 31/34] fix(auth): skip redundant mTLS cert checks after a concurrent 401 check When several threads get a 401 for an expired token while the client certificate is unchanged, _cached_cert never changes, so each queued thread re-loaded the certificate one at a time under _mtls_reauth_lock. Track a _mtls_cert_check_generation counter, snapshot it when each request is sent, and skip the check if another thread already checked after that. Add concurrency tests for the unchanged-certificate case. --- .../google/auth/transport/requests.py | 42 ++++++++-- .../google/auth/transport/urllib3.py | 42 ++++++++-- .../tests/transport/test_requests.py | 78 ++++++++++++++++++- .../tests/transport/test_urllib3.py | 78 ++++++++++++++++++- 4 files changed, 224 insertions(+), 16 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index fe62dcedf9ea..8be73424ea16 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -529,6 +529,11 @@ def __init__( self._is_mtls = False self._default_host = default_host self._mtls_reauth_lock = threading.Lock() + # Incremented after every client certificate check that runs on a + # 401 response, so that threads that were waiting for + # _mtls_reauth_lock can skip a check that ran after they sent their + # request. + self._mtls_cert_check_generation = 0 if auth_request is None: self._auth_request_session = requests.Session() @@ -739,10 +744,12 @@ def request( self.credentials.before_request(auth_request, method, url, request_headers) remaining_time = guard.remaining_timeout - # Remember the client certificate that this request is sent with. If - # the request receives a 401 response, this tells whether another - # thread has already rotated the certificate in the meantime. + # Remember the client certificate that this request is sent with, and + # how many certificate checks have run so far. If the request receives + # a 401 response, this tells whether another thread has already + # checked or rotated the certificate in the meantime. cert_at_request = getattr(self, "_cached_cert", None) + check_generation_at_request = self._mtls_cert_check_generation with TimeoutGuard(remaining_time) as guard: _helpers.request_log(_LOGGER, method, url, data, headers) @@ -770,7 +777,9 @@ def request( use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: with TimeoutGuard(remaining_time) as guard: - self._reconfigure_mtls_channel_if_cert_changed(cert_at_request) + self._reconfigure_mtls_channel_if_cert_changed( + cert_at_request, check_generation_at_request + ) remaining_time = guard.remaining_timeout _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", @@ -806,19 +815,26 @@ def request( return response - def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): + def _reconfigure_mtls_channel_if_cert_changed( + self, cert_at_request, check_generation_at_request + ): """Reconfigures the mTLS channel if the client certificate has changed. This is called when a request to an mTLS endpoint receives a 401 response. The certificate check and the channel reconfiguration run while holding ``_mtls_reauth_lock``. When several threads receive a 401 response at the same time, only the first one reads the current - certificate and remounts the adapters. The other threads see that the - channel has already been reconfigured and reuse it. + certificate, and remounts the adapters if the certificate has + changed. Threads that were waiting for the lock see that the + certificate was already checked after they sent their request, and + skip the check. Args: cert_at_request (Optional[bytes]): The cached client certificate at the time the request that received the 401 response was sent. + check_generation_at_request (int): The value of + ``_mtls_cert_check_generation`` at the time the request that + received the 401 response was sent. Raises: google.auth.exceptions.MutualTLSChannelError: If the client @@ -837,6 +853,16 @@ def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): ) return + if self._mtls_cert_check_generation != check_generation_at_request: + # Another thread checked the certificate after this request was + # sent, whether or not the certificate had changed. Checking it + # again would only load the same certificate again. + _LOGGER.info( + "Skipping client certificate check because another request" + " already checked it." + ) + return + try: ( call_cert_bytes, @@ -855,6 +881,8 @@ def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): e, ) return + finally: + self._mtls_cert_check_generation += 1 if cached_fingerprint == current_cert_fingerprint: _LOGGER.info( diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 0659213291bb..cf0ccb6e947a 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -310,6 +310,11 @@ def __init__( self._request = Request(self.http) self._is_mtls = False self._mtls_reauth_lock = threading.Lock() + # Incremented after every client certificate check that runs on a + # 401 response, so that threads that were waiting for + # _mtls_reauth_lock can skip a check that ran after they sent their + # request. + self._mtls_cert_check_generation = 0 # https://google.aip.dev/auth/4111 # Attempt to use self-signed JWTs when a service account is used. @@ -421,10 +426,12 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): self.credentials.before_request(self._request, method, url, request_headers) - # Remember the client certificate that this request is sent with. If - # the request receives a 401 response, this tells whether another - # thread has already rotated the certificate in the meantime. + # Remember the client certificate that this request is sent with, and + # how many certificate checks have run so far. If the request receives + # a 401 response, this tells whether another thread has already + # checked or rotated the certificate in the meantime. cert_at_request = getattr(self, "_cached_cert", None) + check_generation_at_request = self._mtls_cert_check_generation response = self.http.urlopen( method, url, body=body, headers=request_headers, **kwargs @@ -444,7 +451,9 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): if response.status == http_client.UNAUTHORIZED: use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) if use_mtls: - self._reconfigure_mtls_channel_if_cert_changed(cert_at_request) + self._reconfigure_mtls_channel_if_cert_changed( + cert_at_request, check_generation_at_request + ) _LOGGER.info( "Refreshing credentials due to a %s response. Attempt %s/%s.", @@ -467,19 +476,26 @@ def urlopen(self, method, url, body=None, headers=None, **kwargs): return response - def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): + def _reconfigure_mtls_channel_if_cert_changed( + self, cert_at_request, check_generation_at_request + ): """Reconfigures the mTLS channel if the client certificate has changed. This is called when a request to an mTLS endpoint receives a 401 response. The certificate check and the channel reconfiguration run while holding ``_mtls_reauth_lock``. When several threads receive a 401 response at the same time, only the first one reads the current - certificate and replaces the pool manager. The other threads see that - the channel has already been reconfigured and reuse it. + certificate, and replaces the pool manager if the certificate has + changed. Threads that were waiting for the lock see that the + certificate was already checked after they sent their request, and + skip the check. Args: cert_at_request (Optional[bytes]): The cached client certificate at the time the request that received the 401 response was sent. + check_generation_at_request (int): The value of + ``_mtls_cert_check_generation`` at the time the request that + received the 401 response was sent. Raises: google.auth.exceptions.MutualTLSChannelError: If the client @@ -498,6 +514,16 @@ def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): ) return + if self._mtls_cert_check_generation != check_generation_at_request: + # Another thread checked the certificate after this request was + # sent, whether or not the certificate had changed. Checking it + # again would only load the same certificate again. + _LOGGER.info( + "Skipping client certificate check because another request" + " already checked it." + ) + return + try: ( call_cert_bytes, @@ -516,6 +542,8 @@ def _reconfigure_mtls_channel_if_cert_changed(self, cert_at_request): e, ) return + finally: + self._mtls_cert_check_generation += 1 if cached_fingerprint == current_cert_fingerprint: _LOGGER.info( diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 05afe93ee767..0966e10fb3ad 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1519,7 +1519,7 @@ def test_reconfigure_mtls_channel_skipped_if_mtls_disabled(self): google.auth.transport._mtls_helper, "check_parameters_for_unauthorized_response", ) as mock_check_params: - authed_session._reconfigure_mtls_channel_if_cert_changed(b"old_cert") + authed_session._reconfigure_mtls_channel_if_cert_changed(b"old_cert", 0) mock_check_params.assert_not_called() assert not authed_session._mtls_reauth_lock.locked() @@ -1613,6 +1613,82 @@ def make_request(): assert new_auth_adapter is not old_auth_adapter assert not authed_session._mtls_reauth_lock.locked() + @mock.patch.object(requests.adapters.HTTPAdapter, "send", autospec=True) + def test_concurrent_unauthorized_responses_check_unchanged_cert_once( + self, mock_send + ): + """ + Tests that when several threads that share an AuthorizedSession receive + a 401 response at the same time and the client certificate has not + changed (for example, because the token expired), the certificate is + loaded only once, the mTLS channel is not reconfigured, and every + request is retried successfully. + """ + num_threads = 8 + all_requests_sent = threading.Barrier(num_threads) + sent_lock = threading.Lock() + sent = {"count": 0} + + authed_session = google.auth.transport.requests.AuthorizedSession( + CredentialsStub(), refresh_timeout=60 + ) + authed_session._is_mtls = True + authed_session._cached_cert = pytest.public_cert_bytes + adapter = authed_session.adapters["https://"] + + def send(adapter, request, **kwargs): + with sent_lock: + sent["count"] += 1 + first_attempt = sent["count"] <= num_threads + if first_attempt: + # Wait until every thread has sent its first request, so that + # all of them receive a 401 response before any cert check. + all_requests_sent.wait(timeout=30) + return make_response(status=http_client.UNAUTHORIZED) + return make_response(status=http_client.OK) + + mock_send.side_effect = send + + results = [] + errors = [] + + def make_request(): + try: + results.append(authed_session.request("GET", self.MTLS_TEST_URL)) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "same_fp", + "same_fp", + ), + ) as mock_check_params, + mock.patch.object( + authed_session, "configure_mtls_channel" + ) as mock_configure_mtls_channel, + ): + threads = [ + threading.Thread(target=make_request) for _ in range(num_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=60) + + assert errors == [] + assert [r.status_code for r in results] == [http_client.OK] * num_threads + mock_check_params.assert_called_once() + mock_configure_mtls_channel.assert_not_called() + assert authed_session.adapters["https://"] is adapter + assert authed_session._mtls_cert_check_generation == 1 + assert not authed_session._mtls_reauth_lock.locked() + @mock.patch( "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True ) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 2abc39c7c10f..6e454f988f90 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -933,7 +933,7 @@ def test_reconfigure_mtls_channel_skipped_if_mtls_disabled(self): google.auth.transport._mtls_helper, "check_parameters_for_unauthorized_response", ) as mock_check_params: - authed_http._reconfigure_mtls_channel_if_cert_changed(b"old_cert") + authed_http._reconfigure_mtls_channel_if_cert_changed(b"old_cert", 0) mock_check_params.assert_not_called() assert not authed_http._mtls_reauth_lock.locked() @@ -1022,3 +1022,79 @@ def make_request(): # Token refreshes use the new pool manager too. assert authed_http._request.http is authed_http.http assert not authed_http._mtls_reauth_lock.locked() + + @mock.patch.object(urllib3.PoolManager, "urlopen", autospec=True) + def test_concurrent_unauthorized_responses_check_unchanged_cert_once( + self, mock_urlopen + ): + """ + Tests that when several threads that share an AuthorizedHttp receive a + 401 response at the same time and the client certificate has not + changed (for example, because the token expired), the certificate is + loaded only once, the mTLS channel is not reconfigured, and every + request is retried successfully. + """ + num_threads = 8 + all_requests_sent = threading.Barrier(num_threads) + sent_lock = threading.Lock() + sent = {"count": 0} + + authed_http = google.auth.transport.urllib3.AuthorizedHttp(CredentialsStub()) + authed_http._is_mtls = True + authed_http._cached_cert = pytest.public_cert_bytes + http = authed_http.http + + def urlopen(pool_manager, method, url, body=None, headers=None, **kwargs): + with sent_lock: + sent["count"] += 1 + first_attempt = sent["count"] <= num_threads + if first_attempt: + # Wait until every thread has sent its first request, so that + # all of them receive a 401 response before any cert check. + all_requests_sent.wait(timeout=30) + return ResponseStub(status=http_client.UNAUTHORIZED) + return ResponseStub(status=http_client.OK) + + mock_urlopen.side_effect = urlopen + + results = [] + errors = [] + + def make_request(): + try: + results.append( + authed_http.urlopen("GET", "https://example.mtls.googleapis.com") + ) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "same_fp", + "same_fp", + ), + ) as mock_check_params, + mock.patch.object( + authed_http, "configure_mtls_channel" + ) as mock_configure_mtls_channel, + ): + threads = [ + threading.Thread(target=make_request) for _ in range(num_threads) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=60) + + assert errors == [] + assert [r.status for r in results] == [http_client.OK] * num_threads + mock_check_params.assert_called_once() + mock_configure_mtls_channel.assert_not_called() + assert authed_http.http is http + assert authed_http._mtls_cert_check_generation == 1 + assert not authed_http._mtls_reauth_lock.locked() From a1ccefd3e025cb05ab358ae069de803759d47149 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Fri, 30 Oct 2026 23:00:09 +0000 Subject: [PATCH 32/34] docs(auth): clarify configure_mtls_channel and _replace_adapter thread-safety notes --- .../google-auth/google/auth/transport/requests.py | 12 ++++++++---- .../google-auth/google/auth/transport/urllib3.py | 5 ++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 8be73424ea16..6f752c8600d7 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -119,7 +119,6 @@ def __exit__(self, exc_type, exc_value, traceback): def _replace_adapter(session, prefix, adapter): """Mounts an adapter on a session without mutating its adapters in place. - ``adapter`` is mounted on ``session`` for ``prefix``. :meth:`requests.Session.mount` mutates the ``session.adapters`` OrderedDict in place. If another thread is iterating over it in :meth:`requests.Session.get_adapter` at the same time, that thread fails @@ -127,6 +126,12 @@ def _replace_adapter(session, prefix, adapter): an updated copy and assigns it in one step instead, so threads that are already iterating over the old dictionary are not affected. + This only protects threads that read ``session.adapters``. The copy and + the assignment are not atomic together, so if two threads call this (or + :meth:`requests.Session.mount`) on the same session at the same time, + one of the new adapters can be lost. Callers must serialize writes, for + example by holding a lock. + Args: session (requests.Session): The session to mount the adapter on. prefix (str): The URL prefix to mount the adapter for. @@ -578,9 +583,8 @@ def configure_mtls_channel(self, client_cert_callback=None): will be used. .. warning:: - Calling this method replaces the underlying `requests.Session` - adapters and closes the previous ones. It is not thread-safe to call - this explicitly while other threads are making requests. + Replaces and closes adapters. Serialize concurrent calls to this + method. Raises: google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index cf0ccb6e947a..1074cd0b7e1c 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -347,9 +347,8 @@ def configure_mtls_channel(self, client_cert_callback=None): will be used. .. warning:: - Calling this method mutates the underlying `urllib3.PoolManager`. - It is not thread-safe to call this explicitly while other - threads are making requests. + Replaces and closes adapters. Serialize concurrent calls to this + method. Returns: True if the channel is mutual TLS and False otherwise. From 993faebd8fa1f65e6cab3fbd405adf0c8e458185 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Fri, 30 Oct 2026 23:52:36 +0000 Subject: [PATCH 33/34] fix(auth): serialize configure_mtls_channel with 401 mTLS reconfiguration - configure_mtls_channel() now takes _mtls_reauth_lock and delegates to _configure_mtls_channel_locked(), which the 401 reconfiguration path calls while already holding the lock. Explicit calls can no longer race the automatic rotation (double-closing the old adapter, losing a new one, or leaving _cached_cert out of sync with the mounted channel). - Write the auth request session's https:// adapter while also holding Request._mtls_lock, which guards the same dict during token refresh. - Log tracebacks for failed cert checks and reconfigurations. - Fix the urllib3 docstring to refer to the pool manager. - Add tests that an explicit configure_mtls_channel() waits for an in-progress 401 reconfiguration; make concurrency test threads daemon threads and assert they finished. --- .../google/auth/transport/requests.py | 30 +++- .../google/auth/transport/urllib3.py | 22 ++- .../tests/transport/test_requests.py | 142 ++++++++++++++++-- .../tests/transport/test_urllib3.py | 139 +++++++++++++++-- 4 files changed, 302 insertions(+), 31 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 6f752c8600d7..79bc135d508a 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -582,15 +582,23 @@ def configure_mtls_channel(self, client_cert_callback=None): If the callback is None, application default SSL credentials will be used. - .. warning:: - Replaces and closes adapters. Serialize concurrent calls to this - method. + .. note:: + Replaces and closes adapters. Calls are serialized with each other + and with the automatic reconfiguration that runs on 401 responses. Raises: google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel creation failed for any reason. The existing session state (such as adapter mounts) remains unmodified if this error is raised. """ + with self._mtls_reauth_lock: + self._configure_mtls_channel_locked(client_cert_callback) + + def _configure_mtls_channel_locked(self, client_cert_callback=None): + """Implements :meth:`configure_mtls_channel`. + + The caller must hold ``_mtls_reauth_lock``. + """ use_client_cert = google.auth.transport._mtls_helper.check_use_client_cert() if not use_client_cert: return @@ -671,7 +679,12 @@ def configure_mtls_channel(self, client_cert_callback=None): old_adapter.close() if self._auth_request_session is not None and new_auth_adapter is not None: - _replace_adapter(self._auth_request_session, "https://", new_auth_adapter) + # Request._configure_mtls_if_needed() also writes the adapters of + # this session during token refresh, under the Request's own lock. + with self._auth_request._mtls_lock: + _replace_adapter( + self._auth_request_session, "https://", new_auth_adapter + ) if ( old_auth_adapter is not None @@ -883,6 +896,7 @@ def _reconfigure_mtls_channel_if_cert_changed( _LOGGER.warning( "Failed to check whether the client certificate has changed: %s", e, + exc_info=True, ) return finally: @@ -899,9 +913,13 @@ def _reconfigure_mtls_channel_if_cert_changed( _LOGGER.info( "Client certificate has changed, reconfiguring mTLS channel." ) - self.configure_mtls_channel(lambda: (call_cert_bytes, call_key_bytes)) + self._configure_mtls_channel_locked( + lambda: (call_cert_bytes, call_key_bytes) + ) except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e, exc_info=True + ) raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" ) from e diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 1074cd0b7e1c..78ddc1742015 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -346,9 +346,10 @@ def configure_mtls_channel(self, client_cert_callback=None): If the callback is None, application default SSL credentials will be used. - .. warning:: - Replaces and closes adapters. Serialize concurrent calls to this - method. + .. note:: + Replaces the pool manager and clears the old one. Calls are + serialized with each other and with the automatic reconfiguration + that runs on 401 responses. Returns: True if the channel is mutual TLS and False otherwise. @@ -358,6 +359,14 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. The existing channel state (the HTTP client) remains unmodified if this error is raised. """ + with self._mtls_reauth_lock: + return self._configure_mtls_channel_locked(client_cert_callback) + + def _configure_mtls_channel_locked(self, client_cert_callback=None): + """Implements :meth:`configure_mtls_channel`. + + The caller must hold ``_mtls_reauth_lock``. + """ use_client_cert = transport._mtls_helper.check_use_client_cert() if not use_client_cert: return False @@ -539,6 +548,7 @@ def _reconfigure_mtls_channel_if_cert_changed( _LOGGER.warning( "Failed to check whether the client certificate has changed: %s", e, + exc_info=True, ) return finally: @@ -555,11 +565,13 @@ def _reconfigure_mtls_channel_if_cert_changed( _LOGGER.info( "Client certificate has changed, reconfiguring mTLS channel." ) - self.configure_mtls_channel( + self._configure_mtls_channel_locked( client_cert_callback=lambda: (call_cert_bytes, call_key_bytes) ) except Exception as e: - _LOGGER.error("Failed to reconfigure mTLS channel: %s", e) + _LOGGER.error( + "Failed to reconfigure mTLS channel: %s", e, exc_info=True + ) raise exceptions.MutualTLSChannelError( "Failed to reconfigure mTLS channel" ) from e diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 0966e10fb3ad..099189ee0ea7 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -1214,7 +1214,7 @@ def test_cert_rotation_failure_raises_error(self): ): with mock.patch.object( authed_session, - "configure_mtls_channel", + "_configure_mtls_channel_locked", side_effect=Exception("Failed to reconfigure"), ): with pytest.raises(exceptions.MutualTLSChannelError): @@ -1496,7 +1496,7 @@ def send_request_and_rotate_cert(request, **kwargs): "check_parameters_for_unauthorized_response", ) as mock_check_params: with mock.patch.object( - authed_session, "configure_mtls_channel" + authed_session, "_configure_mtls_channel_locked" ) as mock_configure_mtls_channel: result = authed_session.request("GET", self.MTLS_TEST_URL) @@ -1585,17 +1585,19 @@ def make_request(): ) as mock_check_params, mock.patch.object( authed_session, - "configure_mtls_channel", - wraps=authed_session.configure_mtls_channel, + "_configure_mtls_channel_locked", + wraps=authed_session._configure_mtls_channel_locked, ) as spy_configure_mtls_channel, ): threads = [ - threading.Thread(target=make_request) for _ in range(num_threads) + threading.Thread(target=make_request, daemon=True) + for _ in range(num_threads) ] for thread in threads: thread.start() for thread in threads: thread.join(timeout=60) + assert not any(thread.is_alive() for thread in threads) assert errors == [] assert [r.status_code for r in results] == [http_client.OK] * num_threads @@ -1670,16 +1672,18 @@ def make_request(): ), ) as mock_check_params, mock.patch.object( - authed_session, "configure_mtls_channel" + authed_session, "_configure_mtls_channel_locked" ) as mock_configure_mtls_channel, ): threads = [ - threading.Thread(target=make_request) for _ in range(num_threads) + threading.Thread(target=make_request, daemon=True) + for _ in range(num_threads) ] for thread in threads: thread.start() for thread in threads: thread.join(timeout=60) + assert not any(thread.is_alive() for thread in threads) assert errors == [] assert [r.status_code for r in results] == [http_client.OK] * num_threads @@ -1689,6 +1693,122 @@ def make_request(): assert authed_session._mtls_cert_check_generation == 1 assert not authed_session._mtls_reauth_lock.locked() + @mock.patch.object(requests.adapters.HTTPAdapter, "send", autospec=True) + def test_configure_mtls_channel_serialized_with_unauthorized_reconfiguration( + self, mock_send + ): + """ + Tests that an explicit configure_mtls_channel() call made while another + thread is reconfiguring the channel after a 401 response waits for it, + so that each replaced adapter is closed exactly once and the cached + certificate matches the adapter that ends up mounted. + """ + with open(os.path.join(pytest.data_dir, "es256_public_cert.pem"), "rb") as fh: + explicit_cert = fh.read() + with open(os.path.join(pytest.data_dir, "es256_privatekey.pem"), "rb") as fh: + explicit_key = fh.read() + + authed_session = google.auth.transport.requests.AuthorizedSession( + CredentialsStub(), refresh_timeout=60 + ) + authed_session._is_mtls = True + authed_session._cached_cert = b"old_cert" + initial_adapter = authed_session.adapters["https://"] + + def send(adapter, request, **kwargs): + if isinstance(adapter, google.auth.transport.requests._MutualTlsAdapter): + return make_response(status=http_client.OK) + return make_response(status=http_client.UNAUTHORIZED) + + mock_send.side_effect = send + + check_started = threading.Event() + release_check = threading.Event() + + def check_parameters_for_unauthorized_response(cached_cert): + check_started.set() + release_check.wait(timeout=30) + return ( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "old_fp", + "new_fp", + ) + + mounted = [] + replace_adapter = google.auth.transport.requests._replace_adapter + + def record_replace_adapter(session, prefix, adapter): + if session is authed_session: + mounted.append(adapter) + replace_adapter(session, prefix, adapter) + + results = [] + errors = [] + + def make_request(): + try: + results.append(authed_session.request("GET", self.MTLS_TEST_URL)) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + def configure(): + try: + authed_session.configure_mtls_channel( + lambda: (explicit_cert, explicit_key) + ) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=check_parameters_for_unauthorized_response, + ), + mock.patch.object( + google.auth.transport.requests, + "_replace_adapter", + side_effect=record_replace_adapter, + ), + mock.patch.object( + requests.adapters.HTTPAdapter, "close", autospec=True + ) as mock_close, + ): + request_thread = threading.Thread(target=make_request, daemon=True) + request_thread.start() + assert check_started.wait(timeout=30) + + configure_thread = threading.Thread(target=configure, daemon=True) + configure_thread.start() + # The explicit call waits for the 401 reconfiguration to finish. + configure_thread.join(timeout=0.5) + assert configure_thread.is_alive() + assert mounted == [] + + release_check.set() + request_thread.join(timeout=60) + configure_thread.join(timeout=60) + assert not request_thread.is_alive() + assert not configure_thread.is_alive() + + assert errors == [] + assert [r.status_code for r in results] == [http_client.OK] + # The 401 reconfiguration ran first, then the explicit call. + assert len(mounted) == 2 + assert authed_session.adapters["https://"] is mounted[-1] + assert authed_session._cached_cert == explicit_cert + closed = [call.args[0] for call in mock_close.call_args_list] + # Each replaced adapter of the session is closed exactly once, and the + # mounted adapter is not closed. + assert closed.count(initial_adapter) == 1 + assert closed.count(mounted[0]) == 1 + assert mounted[-1] not in closed + assert not authed_session._mtls_reauth_lock.locked() + @mock.patch( "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True ) @@ -1834,13 +1954,13 @@ def test_reauth_lock_acquired_on_unauthorized( def mock_configure_mtls_channel(callback): lock_held_during_call["held"] = session._mtls_reauth_lock.locked() - session.configure_mtls_channel = mock.Mock( + session._configure_mtls_channel_locked = mock.Mock( side_effect=mock_configure_mtls_channel ) session.request("GET", "https://example.mtls.googleapis.com/") - session.configure_mtls_channel.assert_called_once() + session._configure_mtls_channel_locked.assert_called_once() assert lock_held_during_call["held"] is True assert not session._mtls_reauth_lock.locked() @@ -1866,8 +1986,8 @@ def test_reauth_skipped_when_cert_fingerprint_matches( "same_fingerprint", "same_fingerprint", ) - session.configure_mtls_channel = mock.Mock() + session._configure_mtls_channel_locked = mock.Mock() session.request("GET", "https://example.mtls.googleapis.com/") - session.configure_mtls_channel.assert_not_called() + session._configure_mtls_channel_locked.assert_not_called() diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 6e454f988f90..650d3e2becae 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -599,7 +599,7 @@ def test_cert_rotation_failure_raises_error(self): ) as mock_check_params: with mock.patch.object( authed_http, - "configure_mtls_channel", + "_configure_mtls_channel_locked", side_effect=Exception("Failed to reconfigure"), ) as mock_reconfigure: with pytest.raises(exceptions.MutualTLSChannelError): @@ -831,7 +831,7 @@ def test_reauth_lock_acquired_on_unauthorized(self): def mock_configure_mtls_channel(client_cert_callback=None): lock_held_during_call["held"] = http_obj._mtls_reauth_lock.locked() - http_obj.configure_mtls_channel = mock.Mock( + http_obj._configure_mtls_channel_locked = mock.Mock( side_effect=mock_configure_mtls_channel ) http_obj.http.urlopen = mock.Mock( @@ -845,7 +845,7 @@ def mock_configure_mtls_channel(client_cert_callback=None): ): http_obj.request("GET", "https://example.mtls.googleapis.com/") - http_obj.configure_mtls_channel.assert_called_once() + http_obj._configure_mtls_channel_locked.assert_called_once() assert lock_held_during_call["held"] is True assert not http_obj._mtls_reauth_lock.locked() @@ -908,7 +908,7 @@ def send_request_and_rotate_cert(*args, **kwargs): "check_parameters_for_unauthorized_response", ) as mock_check_params: with mock.patch.object( - authed_http, "configure_mtls_channel" + authed_http, "_configure_mtls_channel_locked" ) as mock_configure_mtls_channel: result = authed_http.urlopen( "GET", "https://example.mtls.googleapis.com" @@ -999,17 +999,19 @@ def make_request(): ) as mock_check_params, mock.patch.object( authed_http, - "configure_mtls_channel", - wraps=authed_http.configure_mtls_channel, + "_configure_mtls_channel_locked", + wraps=authed_http._configure_mtls_channel_locked, ) as spy_configure_mtls_channel, ): threads = [ - threading.Thread(target=make_request) for _ in range(num_threads) + threading.Thread(target=make_request, daemon=True) + for _ in range(num_threads) ] for thread in threads: thread.start() for thread in threads: thread.join(timeout=60) + assert not any(thread.is_alive() for thread in threads) assert errors == [] assert [r.status for r in results] == [http_client.OK] * num_threads @@ -1080,16 +1082,18 @@ def make_request(): ), ) as mock_check_params, mock.patch.object( - authed_http, "configure_mtls_channel" + authed_http, "_configure_mtls_channel_locked" ) as mock_configure_mtls_channel, ): threads = [ - threading.Thread(target=make_request) for _ in range(num_threads) + threading.Thread(target=make_request, daemon=True) + for _ in range(num_threads) ] for thread in threads: thread.start() for thread in threads: thread.join(timeout=60) + assert not any(thread.is_alive() for thread in threads) assert errors == [] assert [r.status for r in results] == [http_client.OK] * num_threads @@ -1098,3 +1102,120 @@ def make_request(): assert authed_http.http is http assert authed_http._mtls_cert_check_generation == 1 assert not authed_http._mtls_reauth_lock.locked() + + @mock.patch.object(urllib3.PoolManager, "urlopen", autospec=True) + def test_configure_mtls_channel_serialized_with_unauthorized_reconfiguration( + self, mock_urlopen + ): + """ + Tests that an explicit configure_mtls_channel() call made while another + thread is reconfiguring the channel after a 401 response waits for it, + so that each replaced pool manager is cleared exactly once and the + cached certificate matches the pool manager that ends up in use. + """ + with open(os.path.join(pytest.data_dir, "es256_public_cert.pem"), "rb") as fh: + explicit_cert = fh.read() + with open(os.path.join(pytest.data_dir, "es256_privatekey.pem"), "rb") as fh: + explicit_key = fh.read() + + authed_http = google.auth.transport.urllib3.AuthorizedHttp(CredentialsStub()) + authed_http._is_mtls = True + authed_http._cached_cert = b"old_cert" + initial_http = authed_http.http + + def urlopen(pool_manager, method, url, body=None, headers=None, **kwargs): + if "ssl_context" in pool_manager.connection_pool_kw: + return ResponseStub(status=http_client.OK) + return ResponseStub(status=http_client.UNAUTHORIZED) + + mock_urlopen.side_effect = urlopen + + check_started = threading.Event() + release_check = threading.Event() + + def check_parameters_for_unauthorized_response(cached_cert): + check_started.set() + release_check.wait(timeout=30) + return ( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "old_fp", + "new_fp", + ) + + created = [] + make_mutual_tls_http = google.auth.transport.urllib3._make_mutual_tls_http + + def record_make_mutual_tls_http(cert, key): + http = make_mutual_tls_http(cert, key) + created.append(http) + return http + + results = [] + errors = [] + + def make_request(): + try: + results.append( + authed_http.urlopen("GET", "https://example.mtls.googleapis.com") + ) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + def configure(): + try: + authed_http.configure_mtls_channel( + lambda: (explicit_cert, explicit_key) + ) + except Exception as exc: # pragma: NO COVER + errors.append(exc) + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=check_parameters_for_unauthorized_response, + ), + mock.patch.object( + google.auth.transport.urllib3, + "_make_mutual_tls_http", + side_effect=record_make_mutual_tls_http, + ), + mock.patch.object( + urllib3.PoolManager, "clear", autospec=True + ) as mock_clear, + ): + request_thread = threading.Thread(target=make_request, daemon=True) + request_thread.start() + assert check_started.wait(timeout=30) + + configure_thread = threading.Thread(target=configure, daemon=True) + configure_thread.start() + # The explicit call waits for the 401 reconfiguration to finish. + configure_thread.join(timeout=0.5) + assert configure_thread.is_alive() + assert created == [] + + release_check.set() + request_thread.join(timeout=60) + configure_thread.join(timeout=60) + assert not request_thread.is_alive() + assert not configure_thread.is_alive() + + assert errors == [] + assert [r.status for r in results] == [http_client.OK] + # The 401 reconfiguration ran first, then the explicit call. + assert len(created) == 2 + assert authed_http.http is created[-1] + assert authed_http._request.http is created[-1] + assert authed_http._cached_cert == explicit_cert + cleared = [call.args[0] for call in mock_clear.call_args_list] + # Each replaced pool manager is cleared exactly once, and the one in + # use is not cleared. + assert cleared.count(initial_http) == 1 + assert cleared.count(created[0]) == 1 + assert created[-1] not in cleared + assert not authed_http._mtls_reauth_lock.locked() From 089c2ea99211a5144d021a6de74e4f25b4e37457 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Sat, 31 Oct 2026 03:25:41 +0000 Subject: [PATCH 34/34] fix(auth): make _mtls_reauth_lock reentrant on the same thread for client_cert_callback --- .../google/auth/transport/requests.py | 31 ++++- .../google/auth/transport/urllib3.py | 33 ++++- .../tests/transport/test_requests.py | 121 ++++++++++++++++++ .../tests/transport/test_urllib3.py | 84 ++++++++++++ 4 files changed, 264 insertions(+), 5 deletions(-) diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 79bc135d508a..6c263051b8df 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -17,6 +17,7 @@ from __future__ import absolute_import import collections +import contextlib import copy import functools import http.client as http_client @@ -534,6 +535,7 @@ def __init__( self._is_mtls = False self._default_host = default_host self._mtls_reauth_lock = threading.Lock() + self._mtls_reauth_lock_owner = None # Incremented after every client certificate check that runs on a # 401 response, so that threads that were waiting for # _mtls_reauth_lock can skip a check that ran after they sent their @@ -566,6 +568,26 @@ def __init__( "https://{}/".format(self._default_host) if self._default_host else None ) + @contextlib.contextmanager + def _acquire_mtls_reauth_lock(self): + """Acquires ``_mtls_reauth_lock`` reentrantly for the current thread. + + A reentrant wrapper around ``threading.Lock`` is used so that a + ``client_cert_callback`` that sends a request through this session on + the same thread does not deadlock, while keeping ``.locked()`` + available on Python < 3.14. + """ + current_thread = threading.get_ident() + if self._mtls_reauth_lock_owner == current_thread: + yield + return + with self._mtls_reauth_lock: + self._mtls_reauth_lock_owner = current_thread + try: + yield + finally: + self._mtls_reauth_lock_owner = None + def configure_mtls_channel(self, client_cert_callback=None): """Configure the client certificate and key for SSL connection. @@ -585,13 +607,16 @@ def configure_mtls_channel(self, client_cert_callback=None): .. note:: Replaces and closes adapters. Calls are serialized with each other and with the automatic reconfiguration that runs on 401 responses. + ``client_cert_callback`` runs while the lock is held. It may send + requests through this session on the same thread, but it must not + wait for other threads that use this session. Raises: google.auth.exceptions.MutualTLSChannelError: If mutual TLS channel creation failed for any reason. The existing session state (such as adapter mounts) remains unmodified if this error is raised. """ - with self._mtls_reauth_lock: + with self._acquire_mtls_reauth_lock(): self._configure_mtls_channel_locked(client_cert_callback) def _configure_mtls_channel_locked(self, client_cert_callback=None): @@ -612,6 +637,8 @@ def _configure_mtls_channel_locked(self, client_cert_callback=None): client_cert_callback ) + # Read current adapters after the callback returns so any adapter + # mounted by a reentrant call inside the callback is closed below. old_adapter = self.adapters.get("https://") kwargs = {} @@ -858,7 +885,7 @@ def _reconfigure_mtls_channel_if_cert_changed( certificate has changed but the mTLS channel could not be reconfigured. """ - with self._mtls_reauth_lock: + with self._acquire_mtls_reauth_lock(): if not self.is_mtls: # mTLS was disabled by another thread while this one waited. return diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index 78ddc1742015..59e69cdadef0 100644 --- a/packages/google-auth/google/auth/transport/urllib3.py +++ b/packages/google-auth/google/auth/transport/urllib3.py @@ -16,6 +16,7 @@ from __future__ import absolute_import +import contextlib import http.client as http_client import logging import threading @@ -310,6 +311,7 @@ def __init__( self._request = Request(self.http) self._is_mtls = False self._mtls_reauth_lock = threading.Lock() + self._mtls_reauth_lock_owner = None # Incremented after every client certificate check that runs on a # 401 response, so that threads that were waiting for # _mtls_reauth_lock can skip a check that ran after they sent their @@ -330,6 +332,26 @@ def is_mtls(self): """Indicates if the created SSL channel is mutual TLS.""" return self._is_mtls + @contextlib.contextmanager + def _acquire_mtls_reauth_lock(self): + """Acquires ``_mtls_reauth_lock`` reentrantly for the current thread. + + A reentrant wrapper around ``threading.Lock`` is used so that a + ``client_cert_callback`` that sends a request through this instance on + the same thread does not deadlock, while keeping ``.locked()`` + available on Python < 3.14. + """ + current_thread = threading.get_ident() + if self._mtls_reauth_lock_owner == current_thread: + yield + return + with self._mtls_reauth_lock: + self._mtls_reauth_lock_owner = current_thread + try: + yield + finally: + self._mtls_reauth_lock_owner = None + def configure_mtls_channel(self, client_cert_callback=None): """Configures mutual TLS channel using the given client_cert_callback or application default SSL credentials. @@ -349,7 +371,10 @@ def configure_mtls_channel(self, client_cert_callback=None): .. note:: Replaces the pool manager and clears the old one. Calls are serialized with each other and with the automatic reconfiguration - that runs on 401 responses. + that runs on 401 responses. ``client_cert_callback`` runs while the + lock is held. It may send requests through this instance on the + same thread, but it must not wait for other threads that use this + instance. Returns: True if the channel is mutual TLS and False otherwise. @@ -359,7 +384,7 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. The existing channel state (the HTTP client) remains unmodified if this error is raised. """ - with self._mtls_reauth_lock: + with self._acquire_mtls_reauth_lock(): return self._configure_mtls_channel_locked(client_cert_callback) def _configure_mtls_channel_locked(self, client_cert_callback=None): @@ -391,6 +416,8 @@ def _configure_mtls_channel_locked(self, client_cert_callback=None): new_exc = exceptions.MutualTLSChannelError(caught_exc) raise new_exc from caught_exc + # Read the current pool manager after the callback returns so any pool + # manager created by a reentrant call inside the callback is cleared below. old_http = self.http self.http = new_http @@ -510,7 +537,7 @@ def _reconfigure_mtls_channel_if_cert_changed( certificate has changed but the mTLS channel could not be reconfigured. """ - with self._mtls_reauth_lock: + with self._acquire_mtls_reauth_lock(): if not self.is_mtls: # mTLS was disabled by another thread while this one waited. return diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 099189ee0ea7..ab484ca7131c 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -731,6 +731,47 @@ def test_request_timeout_w_refresh_timeout_timeout_error(self, frozen_time): "GET", self.TEST_URL, timeout=60, max_allowed_time=2.9 ) + def test_request_timeout_w_mtls_reconfigure_timeout_error(self, frozen_time): + tick_one_second = functools.partial( + frozen_time.tick, delta=datetime.timedelta(seconds=1.0) + ) + + credentials = mock.Mock( + wraps=TimeTickCredentialsStub(time_tick=tick_one_second) + ) + adapter = TimeTickAdapterStub( + time_tick=tick_one_second, + responses=[ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK), + ], + ) + + authed_session = google.auth.transport.requests.AuthorizedSession( + credentials, refresh_timeout=100 + ) + authed_session.mount(self.MTLS_TEST_URL, adapter) + authed_session._is_mtls = True + authed_session._cached_cert = b"old_cert" + + def check_params(cached_cert): + tick_one_second() + return (b"old_cert", b"old_key", "same_fp", "same_fp") + + # First request (1s) + mTLS cert check (1s) = 2s, which exceeds + # max_allowed_time=1.9 inside the mTLS TimeoutGuard before refresh. + with mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + side_effect=check_params, + ): + with pytest.raises(requests.exceptions.Timeout): + authed_session.request( + "GET", self.MTLS_TEST_URL, timeout=60, max_allowed_time=1.9 + ) + + credentials.refresh.assert_not_called() + def test_authorized_session_without_default_host(self): credentials = mock.create_autospec(service_account.Credentials) @@ -1460,6 +1501,7 @@ def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): # Token refresh should still be called despite discovery failure credentials.refresh.assert_called_once() assert result.status_code == http_client.OK + assert authed_session._mtls_cert_check_generation == 1 assert not authed_session._mtls_reauth_lock.locked() def test_cert_check_skipped_when_cert_rotated_during_request(self): @@ -1809,6 +1851,85 @@ def configure(): assert mounted[-1] not in closed assert not authed_session._mtls_reauth_lock.locked() + @mock.patch.object(requests.adapters.HTTPAdapter, "send", autospec=True) + def test_configure_mtls_channel_callback_can_reenter_session(self, mock_send): + """ + Tests that _acquire_mtls_reauth_lock() is reentrant on the same thread, + so a custom client_cert_callback that sends a request through the same + session (even one that receives a 401 from an mTLS endpoint) does not + deadlock, while _mtls_reauth_lock remains held against other threads. + """ + with open(os.path.join(pytest.data_dir, "es256_public_cert.pem"), "rb") as fh: + callback_cert = fh.read() + with open(os.path.join(pytest.data_dir, "es256_privatekey.pem"), "rb") as fh: + callback_key = fh.read() + + authed_session = google.auth.transport.requests.AuthorizedSession( + CredentialsStub(), refresh_timeout=60 + ) + authed_session._is_mtls = True + authed_session._cached_cert = b"old_cert" + initial_adapter = authed_session.adapters["https://"] + + responses = iter( + [ + make_response(status=http_client.UNAUTHORIZED), + make_response(status=http_client.OK), + ] + ) + mock_send.side_effect = lambda adapter, request, **kwargs: next(responses) + + mounted = [] + replace_adapter = google.auth.transport.requests._replace_adapter + + def record_replace_adapter(session, prefix, adapter): + if session is authed_session: + mounted.append(adapter) + replace_adapter(session, prefix, adapter) + + lock_held_in_callback = {"held": None} + + def client_cert_callback(): + lock_held_in_callback["held"] = authed_session._mtls_reauth_lock.locked() + response = authed_session.request("GET", self.MTLS_TEST_URL) + assert response.status_code == http_client.OK + return callback_cert, callback_key + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "old_fp", + "new_fp", + ), + ), + mock.patch.object( + google.auth.transport.requests, + "_replace_adapter", + side_effect=record_replace_adapter, + ), + mock.patch.object( + requests.adapters.HTTPAdapter, "close", autospec=True + ) as mock_close, + ): + authed_session.configure_mtls_channel(client_cert_callback) + + assert lock_held_in_callback["held"] is True + assert len(mounted) == 2 + assert authed_session.adapters["https://"] is mounted[-1] + assert authed_session._cached_cert == callback_cert + closed = [call.args[0] for call in mock_close.call_args_list] + assert closed.count(initial_adapter) == 1 + assert closed.count(mounted[0]) == 1 + assert mounted[-1] not in closed + assert not authed_session._mtls_reauth_lock.locked() + @mock.patch( "google.auth.transport._mtls_helper.get_client_cert_and_key", autospec=True ) diff --git a/packages/google-auth/tests/transport/test_urllib3.py b/packages/google-auth/tests/transport/test_urllib3.py index 650d3e2becae..60260d01defd 100644 --- a/packages/google-auth/tests/transport/test_urllib3.py +++ b/packages/google-auth/tests/transport/test_urllib3.py @@ -873,6 +873,7 @@ def test_unauthorized_cert_discovery_exception_proceeds_to_token_refresh(self): # Token refresh should still be called despite discovery failure credentials.refresh.assert_called_once() + assert http_obj._mtls_cert_check_generation == 1 assert not http_obj._mtls_reauth_lock.locked() def test_cert_check_skipped_when_cert_rotated_during_request(self): @@ -1219,3 +1220,86 @@ def configure(): assert cleared.count(created[0]) == 1 assert created[-1] not in cleared assert not authed_http._mtls_reauth_lock.locked() + + @mock.patch.object(urllib3.PoolManager, "urlopen", autospec=True) + def test_configure_mtls_channel_callback_can_reenter_session(self, mock_urlopen): + """ + Tests that _acquire_mtls_reauth_lock() is reentrant on the same thread, + so a custom client_cert_callback that sends a request through the same + AuthorizedHttp (even one that receives a 401 from an mTLS endpoint) + does not deadlock, while _mtls_reauth_lock remains held against other + threads. + """ + with open(os.path.join(pytest.data_dir, "es256_public_cert.pem"), "rb") as fh: + callback_cert = fh.read() + with open(os.path.join(pytest.data_dir, "es256_privatekey.pem"), "rb") as fh: + callback_key = fh.read() + + authed_http = google.auth.transport.urllib3.AuthorizedHttp(CredentialsStub()) + authed_http._is_mtls = True + authed_http._cached_cert = b"old_cert" + initial_http = authed_http.http + + responses = iter( + [ + ResponseStub(status=http_client.UNAUTHORIZED), + ResponseStub(status=http_client.OK), + ] + ) + mock_urlopen.side_effect = ( + lambda pool_manager, method, url, body=None, headers=None, **kwargs: next( + responses + ) + ) + + created = [] + make_mutual_tls_http = google.auth.transport.urllib3._make_mutual_tls_http + + def record_make_mutual_tls_http(cert, key): + http = make_mutual_tls_http(cert, key) + created.append(http) + return http + + lock_held_in_callback = {"held": None} + + def client_cert_callback(): + lock_held_in_callback["held"] = authed_http._mtls_reauth_lock.locked() + response = authed_http.urlopen("GET", "https://example.mtls.googleapis.com") + assert response.status == http_client.OK + return callback_cert, callback_key + + with ( + mock.patch.dict( + os.environ, {environment_vars.GOOGLE_API_USE_CLIENT_CERTIFICATE: "true"} + ), + mock.patch.object( + google.auth.transport._mtls_helper, + "check_parameters_for_unauthorized_response", + return_value=( + pytest.public_cert_bytes, + pytest.private_key_bytes, + "old_fp", + "new_fp", + ), + ), + mock.patch.object( + google.auth.transport.urllib3, + "_make_mutual_tls_http", + side_effect=record_make_mutual_tls_http, + ), + mock.patch.object( + urllib3.PoolManager, "clear", autospec=True + ) as mock_clear, + ): + assert authed_http.configure_mtls_channel(client_cert_callback) is True + + assert lock_held_in_callback["held"] is True + assert len(created) == 2 + assert authed_http.http is created[-1] + assert authed_http._request.http is created[-1] + assert authed_http._cached_cert == callback_cert + cleared = [call.args[0] for call in mock_clear.call_args_list] + assert cleared.count(initial_http) == 1 + assert cleared.count(created[0]) == 1 + assert created[-1] not in cleared + assert not authed_http._mtls_reauth_lock.locked()