diff --git a/packages/google-auth/google/auth/transport/requests.py b/packages/google-auth/google/auth/transport/requests.py index 3f1ec345765c..a3fce4b9ad16 100644 --- a/packages/google-auth/google/auth/transport/requests.py +++ b/packages/google-auth/google/auth/transport/requests.py @@ -514,6 +514,7 @@ def __init__( self._refresh_timeout = refresh_timeout self._is_mtls = False self._default_host = default_host + self._mtls_reauth_lock = threading.Lock() if auth_request is None: self._auth_request_session = requests.Session() @@ -749,33 +750,50 @@ def request( if response.status_code == http_client.UNAUTHORIZED: use_mtls = self.is_mtls and _mtls_helper.is_mtls_endpoint(url) 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( - 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, diff --git a/packages/google-auth/google/auth/transport/urllib3.py b/packages/google-auth/google/auth/transport/urllib3.py index b4b4ebe1daac..b402951d63d6 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 @@ -308,6 +309,7 @@ def __init__( # credentials.refresh). self._request = Request(self.http) self._is_mtls = False + self._mtls_reauth_lock = threading.Lock() # https://google.aip.dev/auth/4111 # Attempt to use self-signed JWTs when a service account is used. @@ -318,6 +320,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. @@ -430,40 +437,51 @@ 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: - ( - 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: ( + 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, ) - ) - 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, diff --git a/packages/google-auth/tests/transport/test_requests.py b/packages/google-auth/tests/transport/test_requests.py index 5ca8788ca42b..d019417d64a5 100644 --- a/packages/google-auth/tests/transport/test_requests.py +++ b/packages/google-auth/tests/transport/test_requests.py @@ -20,6 +20,7 @@ import os import pickle import ssl +import threading from unittest import mock import freezegun @@ -1224,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 @@ -1237,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): """ @@ -1433,6 +1437,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 = 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) + # 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(google.auth.transport.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") @@ -1499,3 +1525,66 @@ 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" + ) + @mock.patch("google.auth.transport.requests.requests.Session.request") + 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 + 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( + 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_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" + ) + @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()