From 47a7786760b27e1f037382fb1459a46addae99cb Mon Sep 17 00:00:00 2001 From: nbayati <99771966+nbayati@users.noreply.github.com> Date: Tue, 3 Mar 2026 21:47:04 -0800 Subject: [PATCH 1/4] fix: gracefully fallback if workload fields are missing from cert config Prevents exceptions during gECC flows or when falling back to SecureConnect by returning None instead of raising ClientCertError when X.509 workload fields are absent. --- .../google/auth/transport/_mtls_helper.py | 28 ++++++------------- .../tests/transport/test__mtls_helper.py | 20 +++++++------ 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/packages/google-auth/google/auth/transport/_mtls_helper.py b/packages/google-auth/google/auth/transport/_mtls_helper.py index 50465d1b766c..e924d52f753c 100644 --- a/packages/google-auth/google/auth/transport/_mtls_helper.py +++ b/packages/google-auth/google/auth/transport/_mtls_helper.py @@ -183,36 +183,24 @@ def _get_workload_cert_and_key_paths(config_path, include_context_aware=True): data = _load_json_file(absolute_path) + # We return None, None if the expected workload fields are not present. + # The certificate config might be present for other types of connections (e.g. gECC), + # and we want to gracefully fallback to testing other mTLS configurations + # like SecureConnect instead of throwing an exception. if "cert_configs" not in data: - raise exceptions.ClientCertError( - 'Certificate config file {} is in an invalid format, a "cert configs" object is expected'.format( - absolute_path - ) - ) + return None, None cert_configs = data["cert_configs"] if "workload" not in cert_configs: - raise exceptions.ClientCertError( - 'Certificate config file {} is in an invalid format, a "workload" cert config is expected'.format( - absolute_path - ) - ) + return None, None workload = cert_configs["workload"] if "cert_path" not in workload: - raise exceptions.ClientCertError( - 'Certificate config file {} is in an invalid format, a "cert_path" is expected in the workload cert config'.format( - absolute_path - ) - ) + return None, None cert_path = workload["cert_path"] if "key_path" not in workload: - raise exceptions.ClientCertError( - 'Certificate config file {} is in an invalid format, a "key_path" is expected in the workload cert config'.format( - absolute_path - ) - ) + return None, None key_path = workload["key_path"] # == BEGIN Temporary Cloud Run PATCH == diff --git a/packages/google-auth/tests/transport/test__mtls_helper.py b/packages/google-auth/tests/transport/test__mtls_helper.py index eeb5e1310d5f..8aba1d6d1316 100644 --- a/packages/google-auth/tests/transport/test__mtls_helper.py +++ b/packages/google-auth/tests/transport/test__mtls_helper.py @@ -566,8 +566,9 @@ def test_no_cert_configs(self, mock_get_cert_config_path, mock_load_json_file): mock_get_cert_config_path.return_value = "/path/to/cert" mock_load_json_file.return_value = {} - with pytest.raises(exceptions.ClientCertError): - _mtls_helper._get_workload_cert_and_key("") + actual_cert, actual_key = _mtls_helper._get_workload_cert_and_key("") + assert actual_cert is None + assert actual_key is None @mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True) @mock.patch( @@ -577,8 +578,9 @@ def test_no_workload(self, mock_get_cert_config_path, mock_load_json_file): mock_get_cert_config_path.return_value = "/path/to/cert" mock_load_json_file.return_value = {"cert_configs": {}} - with pytest.raises(exceptions.ClientCertError): - _mtls_helper._get_workload_cert_and_key("") + actual_cert, actual_key = _mtls_helper._get_workload_cert_and_key("") + assert actual_cert is None + assert actual_key is None @mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True) @mock.patch( @@ -590,8 +592,9 @@ def test_no_cert_file(self, mock_get_cert_config_path, mock_load_json_file): "cert_configs": {"workload": {"key_path": "path/to/key"}} } - with pytest.raises(exceptions.ClientCertError): - _mtls_helper._get_workload_cert_and_key("") + actual_cert, actual_key = _mtls_helper._get_workload_cert_and_key("") + assert actual_cert is None + assert actual_key is None @mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True) @mock.patch( @@ -603,8 +606,9 @@ def test_no_key_file(self, mock_get_cert_config_path, mock_load_json_file): "cert_configs": {"workload": {"cert_path": "path/to/key"}} } - with pytest.raises(exceptions.ClientCertError): - _mtls_helper._get_workload_cert_and_key("") + actual_cert, actual_key = _mtls_helper._get_workload_cert_and_key("") + assert actual_cert is None + assert actual_key is None class TestReadCertAndKeyFile(object): From 532fbb9d468118c6b3189f35dfb89b927067b2d3 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Mar 2026 09:15:53 -0800 Subject: [PATCH 2/4] keep error when cert_configs is missing --- .../google/auth/transport/_mtls_helper.py | 10 +++++++--- .../tests/transport/aio/test_sessions_mtls.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/google-auth/google/auth/transport/_mtls_helper.py b/packages/google-auth/google/auth/transport/_mtls_helper.py index e924d52f753c..49a8101b2854 100644 --- a/packages/google-auth/google/auth/transport/_mtls_helper.py +++ b/packages/google-auth/google/auth/transport/_mtls_helper.py @@ -182,14 +182,18 @@ def _get_workload_cert_and_key_paths(config_path, include_context_aware=True): return None, None data = _load_json_file(absolute_path) + if "cert_configs" not in data: + raise exceptions.ClientCertError( + 'Certificate config file {} is in an invalid format, a "cert configs" object is expected'.format( + absolute_path + ) + ) + cert_configs = data["cert_configs"] # We return None, None if the expected workload fields are not present. # The certificate config might be present for other types of connections (e.g. gECC), # and we want to gracefully fallback to testing other mTLS configurations # like SecureConnect instead of throwing an exception. - if "cert_configs" not in data: - return None, None - cert_configs = data["cert_configs"] if "workload" not in cert_configs: return None, None diff --git a/packages/google-auth/tests/transport/aio/test_sessions_mtls.py b/packages/google-auth/tests/transport/aio/test_sessions_mtls.py index b623eda6caa4..18fdb2e58cf1 100644 --- a/packages/google-auth/tests/transport/aio/test_sessions_mtls.py +++ b/packages/google-auth/tests/transport/aio/test_sessions_mtls.py @@ -98,6 +98,25 @@ async def test_configure_mtls_channel_invalid_format(self): with pytest.raises(exceptions.MutualTLSChannelError): await session.configure_mtls_channel() + @pytest.mark.asyncio + async def test_configure_mtls_channel_invalud_fields(self): + """ + If cert is missing expected keys, it should fail gracefully + """ + with mock.patch.dict( + os.environ, {"GOOGLE_API_USE_CLIENT_CERTIFICATE": "true"} + ), mock.patch("os.path.exists") as mock_exists, mock.patch( + "builtins.open", mock.mock_open(read_data='{"cert_configs": {}}') + ): + mock_exists.return_value = True + mock_creds = mock.AsyncMock(spec=credentials.Credentials) + session = sessions.AsyncAuthorizedSession(mock_creds) + + await session.configure_mtls_channel() + + # If the file couldn't be parsed, it shouldn't error; it just won't use mTLS + assert session._is_mtls is False + @pytest.mark.asyncio async def test_configure_mtls_channel_mock_callback(self): """ From f74386a07eee9b13aa1ab2736ba9e7802d92d2d9 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Mar 2026 09:31:21 -0800 Subject: [PATCH 3/4] updated test --- packages/google-auth/tests/transport/test__mtls_helper.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/google-auth/tests/transport/test__mtls_helper.py b/packages/google-auth/tests/transport/test__mtls_helper.py index 8aba1d6d1316..078df67470d2 100644 --- a/packages/google-auth/tests/transport/test__mtls_helper.py +++ b/packages/google-auth/tests/transport/test__mtls_helper.py @@ -566,9 +566,8 @@ def test_no_cert_configs(self, mock_get_cert_config_path, mock_load_json_file): mock_get_cert_config_path.return_value = "/path/to/cert" mock_load_json_file.return_value = {} - actual_cert, actual_key = _mtls_helper._get_workload_cert_and_key("") - assert actual_cert is None - assert actual_key is None + with pytest.raises(exceptions.ClientCertError): + _mtls_helper._get_workload_cert_and_key("") @mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True) @mock.patch( From a9f70463bfa6f836b122295fffd8fa57364ded40 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Wed, 4 Mar 2026 09:35:04 -0800 Subject: [PATCH 4/4] added back in line break --- packages/google-auth/google/auth/transport/_mtls_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/google-auth/google/auth/transport/_mtls_helper.py b/packages/google-auth/google/auth/transport/_mtls_helper.py index 49a8101b2854..d6450291c7f2 100644 --- a/packages/google-auth/google/auth/transport/_mtls_helper.py +++ b/packages/google-auth/google/auth/transport/_mtls_helper.py @@ -182,6 +182,7 @@ def _get_workload_cert_and_key_paths(config_path, include_context_aware=True): return None, None data = _load_json_file(absolute_path) + if "cert_configs" not in data: raise exceptions.ClientCertError( 'Certificate config file {} is in an invalid format, a "cert configs" object is expected'.format(