Skip to content

Commit 3aa269b

Browse files
committed
fix(auth): centralize cert discovery logic and steps
- Centralize mTLS configuration file path discovery by replacing disparate checks in `check_use_client_cert` and `has_default_client_cert_source` with a unified `_get_cert_config_path` helper. - Ensure X.509 Workload Identity Federation (WIF) takes precedence over SecureConnect by checking `_get_cert_config_path` before `CONTEXT_AWARE_METADATA_PATH` in `has_default_client_cert_source`. - Remove temporary Cloud Run certificate and key path overrides from `_get_workload_cert_and_key_paths`. - Properly honor `CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH` in availability checks, fixing an existing mismatch where auto-enablement would evaluate to true but the downstream source checker would miss the path. - Allow mTLS auto-enablement to properly trigger off the default `gcloud` configuration file (`~/.config/gcloud/certificate_config.json`), resolving previous contradictory behavior. - Ensure strict environment variable precedence: invalid paths supplied via environment variables now correctly result in failures rather than silently falling back to default disk locations. - Update tests to reflect the new centralized resolution and precedence logic.
1 parent 4253fab commit 3aa269b

4 files changed

Lines changed: 49 additions & 182 deletions

File tree

packages/google-auth/google/auth/transport/_mtls_helper.py

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -61,20 +61,6 @@
6161
b"-----BEGIN PASSPHRASE-----(.+)-----END PASSPHRASE-----", re.DOTALL
6262
)
6363

64-
# Temporary patch to accomodate incorrect cert config in Cloud Run prod environment.
65-
_WELL_KNOWN_CLOUD_RUN_CERT_PATH = (
66-
"/var/run/secrets/workload-spiffe-credentials/certificates.pem"
67-
)
68-
_WELL_KNOWN_CLOUD_RUN_KEY_PATH = (
69-
"/var/run/secrets/workload-spiffe-credentials/private_key.pem"
70-
)
71-
_INCORRECT_CLOUD_RUN_CERT_PATH = (
72-
"/var/lib/volumes/certificate/workload-certificates/certificates.pem"
73-
)
74-
_INCORRECT_CLOUD_RUN_KEY_PATH = (
75-
"/var/lib/volumes/certificate/workload-certificates/private_key.pem"
76-
)
77-
7864

7965
class _MemfdCreationError(OSError):
8066
"""Raised when Linux in-memory virtual file creation (memfd) fails."""
@@ -489,25 +475,6 @@ def _get_workload_cert_and_key_paths(config_path, include_context_aware=True):
489475
cert_path = workload["cert_path"]
490476
key_path = workload["key_path"]
491477

492-
# == BEGIN Temporary Cloud Run PATCH ==
493-
# See https://github.com/googleapis/google-auth-library-python/issues/1881
494-
if (cert_path == _INCORRECT_CLOUD_RUN_CERT_PATH) and (
495-
key_path == _INCORRECT_CLOUD_RUN_KEY_PATH
496-
):
497-
if not path.exists(cert_path) and not path.exists(key_path):
498-
_LOGGER.debug(
499-
"Applying Cloud Run certificate path patch. "
500-
"Configured paths not found: %s, %s. "
501-
"Using well-known paths: %s, %s",
502-
cert_path,
503-
key_path,
504-
_WELL_KNOWN_CLOUD_RUN_CERT_PATH,
505-
_WELL_KNOWN_CLOUD_RUN_KEY_PATH,
506-
)
507-
cert_path = _WELL_KNOWN_CLOUD_RUN_CERT_PATH
508-
key_path = _WELL_KNOWN_CLOUD_RUN_KEY_PATH
509-
# == END Temporary Cloud Run PATCH ==
510-
511478
return cert_path, key_path
512479

513480

@@ -768,10 +735,8 @@ def check_use_client_cert():
768735

769736
# Auto-enablement checks (when GOOGLE_API_USE_CLIENT_CERTIFICATE is not set)
770737

771-
# Check if the value of GOOGLE_API_CERTIFICATE_CONFIG is set.
772-
cert_path = getenv(environment_vars.GOOGLE_API_CERTIFICATE_CONFIG) or getenv(
773-
environment_vars.CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH
774-
)
738+
# Check if a workload config file exists.
739+
cert_path = _get_cert_config_path(include_context_aware=True)
775740

776741
if cert_path:
777742
try:

packages/google-auth/google/auth/transport/mtls.py

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
from google.auth import exceptions
2525
from google.auth.transport import _mtls_helper
2626

27-
2827
_LOGGER = logging.getLogger(__name__)
2928

3029

@@ -45,24 +44,17 @@ def has_default_client_cert_source(include_context_aware=True):
4544
bool: indicating if the default client cert source exists.
4645
"""
4746
if (
48-
include_context_aware
49-
and _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
47+
_mtls_helper._get_cert_config_path(include_context_aware=include_context_aware)
5048
is not None
5149
):
5250
return True
5351
if (
54-
_mtls_helper._check_config_path(
55-
_mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH
56-
)
52+
include_context_aware
53+
and _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
5754
is not None
5855
):
5956
return True
60-
cert_config_path = getenv("GOOGLE_API_CERTIFICATE_CONFIG")
61-
if (
62-
cert_config_path
63-
and _mtls_helper._check_config_path(cert_config_path) is not None
64-
):
65-
return True
57+
6658
return False
6759

6860

packages/google-auth/tests/transport/test__mtls_helper.py

Lines changed: 18 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -341,93 +341,6 @@ def test_success_with_certificate_config(
341341
assert key == pytest.private_key_bytes
342342
assert passphrase is None
343343

344-
@mock.patch(
345-
"google.auth.transport._mtls_helper._read_cert_and_key_files", autospec=True
346-
)
347-
@mock.patch(
348-
"google.auth.transport._mtls_helper._get_cert_config_path", autospec=True
349-
)
350-
@mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True)
351-
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
352-
def test_success_with_certificate_config_cloud_run_patch(
353-
self,
354-
mock_check_config_path,
355-
mock_load_json_file,
356-
mock_get_cert_config_path,
357-
mock_read_cert_and_key_files,
358-
):
359-
cert_config_path = "/path/to/config"
360-
mock_check_config_path.return_value = cert_config_path
361-
mock_load_json_file.return_value = {
362-
"cert_configs": {
363-
"workload": {
364-
"cert_path": _mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
365-
"key_path": _mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
366-
}
367-
}
368-
}
369-
mock_get_cert_config_path.return_value = cert_config_path
370-
mock_read_cert_and_key_files.return_value = (
371-
pytest.public_cert_bytes,
372-
pytest.private_key_bytes,
373-
)
374-
375-
has_cert, cert, key, passphrase = _mtls_helper.get_client_ssl_credentials()
376-
assert has_cert
377-
assert cert == pytest.public_cert_bytes
378-
assert key == pytest.private_key_bytes
379-
assert passphrase is None
380-
381-
mock_read_cert_and_key_files.assert_called_once_with(
382-
_mtls_helper._WELL_KNOWN_CLOUD_RUN_CERT_PATH,
383-
_mtls_helper._WELL_KNOWN_CLOUD_RUN_KEY_PATH,
384-
)
385-
386-
@mock.patch("os.path.exists", autospec=True)
387-
@mock.patch(
388-
"google.auth.transport._mtls_helper._read_cert_and_key_files", autospec=True
389-
)
390-
@mock.patch(
391-
"google.auth.transport._mtls_helper._get_cert_config_path", autospec=True
392-
)
393-
@mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True)
394-
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
395-
def test_success_with_certificate_config_cloud_run_patch_skipped_if_cert_exists(
396-
self,
397-
mock_check_config_path,
398-
mock_load_json_file,
399-
mock_get_cert_config_path,
400-
mock_read_cert_and_key_files,
401-
mock_os_path_exists,
402-
):
403-
cert_config_path = "/path/to/config"
404-
mock_check_config_path.return_value = cert_config_path
405-
mock_os_path_exists.return_value = True
406-
mock_load_json_file.return_value = {
407-
"cert_configs": {
408-
"workload": {
409-
"cert_path": _mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
410-
"key_path": _mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
411-
}
412-
}
413-
}
414-
mock_get_cert_config_path.return_value = cert_config_path
415-
mock_read_cert_and_key_files.return_value = (
416-
pytest.public_cert_bytes,
417-
pytest.private_key_bytes,
418-
)
419-
420-
has_cert, cert, key, passphrase = _mtls_helper.get_client_ssl_credentials()
421-
assert has_cert
422-
assert cert == pytest.public_cert_bytes
423-
assert key == pytest.private_key_bytes
424-
assert passphrase is None
425-
426-
mock_read_cert_and_key_files.assert_called_once_with(
427-
_mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
428-
_mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
429-
)
430-
431344
@mock.patch(
432345
"google.auth.transport._mtls_helper._get_workload_cert_and_key", autospec=True
433346
)
@@ -848,7 +761,9 @@ def test_env_var_explicit_garbage(self):
848761
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
849762
},
850763
)
851-
def test_config_file_success(self, mock_file):
764+
@mock.patch("os.path.exists", autospec=True)
765+
def test_config_file_success(self, mock_exists, mock_file):
766+
mock_exists.return_value = True
852767
# We manually apply mock_open here so we can keep autospec=True on the decorator
853768
mock_file.side_effect = mock.mock_open(
854769
read_data='{"cert_configs": {"workload": "exists"}}'
@@ -865,7 +780,9 @@ def test_config_file_success(self, mock_file):
865780
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
866781
},
867782
)
868-
def test_config_file_missing_keys(self, mock_file):
783+
@mock.patch("os.path.exists", autospec=True)
784+
def test_config_file_missing_keys(self, mock_exists, mock_file):
785+
mock_exists.return_value = True
869786
mock_file.side_effect = mock.mock_open(read_data='{"cert_configs": {}}')
870787
assert _mtls_helper.check_use_client_cert() is False
871788

@@ -879,7 +796,9 @@ def test_config_file_missing_keys(self, mock_file):
879796
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
880797
},
881798
)
882-
def test_config_file_bad_json(self, mock_file):
799+
@mock.patch("os.path.exists", autospec=True)
800+
def test_config_file_bad_json(self, mock_exists, mock_file):
801+
mock_exists.return_value = True
883802
mock_file.side_effect = mock.mock_open(read_data="{bad_json")
884803
assert _mtls_helper.check_use_client_cert() is False
885804

@@ -893,12 +812,16 @@ def test_config_file_bad_json(self, mock_file):
893812
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
894813
},
895814
)
896-
def test_config_file_not_found(self, mock_file):
815+
@mock.patch("os.path.exists", autospec=True)
816+
def test_config_file_not_found(self, mock_exists, mock_file):
817+
mock_exists.return_value = True
897818
mock_file.side_effect = FileNotFoundError
898819
assert _mtls_helper.check_use_client_cert() is False
899820

900821
@mock.patch.dict(os.environ, {}, clear=True)
901-
def test_no_env_vars_set(self):
822+
@mock.patch("os.path.exists", autospec=True)
823+
def test_no_env_vars_set(self, mock_exists):
824+
mock_exists.return_value = False
902825
assert _mtls_helper.check_use_client_cert() is False
903826

904827
def test_use_client_cert_precedence(self):
@@ -941,7 +864,9 @@ def test_use_client_cert_fallback(self):
941864
assert _mtls_helper.check_use_client_cert() is False
942865

943866
@mock.patch("builtins.open", autospec=True)
944-
def test_check_use_client_cert_config_fallback(self, mock_file):
867+
@mock.patch("os.path.exists", autospec=True)
868+
def test_check_use_client_cert_config_fallback(self, mock_exists, mock_file):
869+
mock_exists.return_value = True
945870
# Test fallback for config file when determining if client cert should be used
946871
cloudsdk_path = "/path/to/cloudsdk/config"
947872

packages/google-auth/tests/transport/test_mtls.py

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,18 @@
2323
from google.auth.transport import mtls
2424

2525

26+
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
2627
@mock.patch("google.auth.transport._mtls_helper._check_config_path")
27-
def test_has_default_client_cert_source_with_context_aware_metadata(mock_check):
28+
def test_has_default_client_cert_source_with_context_aware_metadata(
29+
mock_check, mock_get_cert
30+
):
2831
"""
2932
Directly tests the logic: if CONTEXT_AWARE_METADATA_PATH is found, return True.
3033
"""
3134

32-
# Setup: Return a path only for the Context Aware Metadata Path
35+
# Setup: _get_cert_config_path returns None, so it falls back to context aware metadata
36+
mock_get_cert.return_value = None
37+
3338
def side_effect(path):
3439
if path == _mtls_helper.CONTEXT_AWARE_METADATA_PATH:
3540
return "/path/to/context_aware_metadata.json"
@@ -42,71 +47,51 @@ def side_effect(path):
4247

4348
# Assert
4449
assert result is True
50+
mock_get_cert.assert_called_once_with(include_context_aware=True)
4551
mock_check.assert_any_call(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
4652
assert side_effect("non-matching-path") is None
4753

4854

4955
@mock.patch("google.auth.transport._mtls_helper._check_config_path")
50-
def test_has_default_client_cert_source_falls_back(mock_check):
56+
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
57+
def test_has_default_client_cert_source_falls_back(mock_get_cert, mock_check):
5158
"""
52-
Tests that it skips CONTEXT_AWARE_METADATA_PATH if None, and checks the next path.
59+
Tests that it checks X.509 WIF first, and if found, returns True without checking context aware metadata.
5360
"""
5461

55-
# Setup: First path is None, second path is valid
56-
def side_effect(path):
57-
if path == _mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH:
58-
return "/path/to/default_cert.json"
59-
return None
60-
61-
mock_check.side_effect = side_effect
62+
# Setup: First path is valid
63+
mock_get_cert.return_value = "/path/to/default_cert.json"
6264

6365
# Execute
6466
result = mtls.has_default_client_cert_source(True)
6567

6668
# Assert
6769
assert result is True
6870
# Verify the sequence of calls
69-
expected_calls = [
70-
mock.call(_mtls_helper.CONTEXT_AWARE_METADATA_PATH),
71-
mock.call(_mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH),
72-
]
73-
mock_check.assert_has_calls(expected_calls)
71+
mock_get_cert.assert_called_once_with(include_context_aware=True)
72+
mock_check.assert_not_called()
7473

7574

76-
@mock.patch("google.auth.transport.mtls.getenv", autospec=True)
75+
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path", autospec=True)
7776
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
78-
def test_has_default_client_cert_source_env_var_success(check_config_path, mock_getenv):
79-
# 1. Mock getenv to return our test path
80-
mock_getenv.side_effect = lambda var: (
81-
"path/to/cert.json" if var == "GOOGLE_API_CERTIFICATE_CONFIG" else None
82-
)
83-
84-
# 2. Mock _check_config_path side effect
85-
def side_effect(path):
86-
# Return None for legacy paths to ensure we reach the env var logic
87-
if path == "path/to/cert.json":
88-
return "/absolute/path/to/cert.json"
89-
return None
90-
91-
check_config_path.side_effect = side_effect
77+
def test_has_default_client_cert_source_env_var_success(
78+
check_config_path, get_cert_config_path
79+
):
80+
check_config_path.return_value = None
81+
get_cert_config_path.return_value = "/absolute/path/to/cert.json"
9282

93-
# 3. This should now return True
9483
assert mtls.has_default_client_cert_source(True)
9584

96-
# 4. Verify the env var path was checked
97-
check_config_path.assert_called_with("path/to/cert.json")
85+
get_cert_config_path.assert_called_with(include_context_aware=True)
9886

9987

100-
@mock.patch("google.auth.transport.mtls.getenv", autospec=True)
88+
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path", autospec=True)
10189
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
10290
def test_has_default_client_cert_source_env_var_invalid_config_path(
103-
check_config_path, mock_getenv
91+
check_config_path, get_cert_config_path
10492
):
105-
# Set the env var but make the check fail
106-
mock_getenv.side_effect = lambda var: (
107-
"invalid/path" if var == "GOOGLE_API_CERTIFICATE_CONFIG" else None
108-
)
10993
check_config_path.return_value = None
94+
get_cert_config_path.return_value = None
11095

11196
assert not mtls.has_default_client_cert_source(True)
11297

0 commit comments

Comments
 (0)