Skip to content

fix(auth): centralize cert discovery logic and steps #17696

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nbayati merged 10 commits into from
Jul 18, 2026
Merged

fix(auth): centralize cert discovery logic and steps #17696

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3aa269b
fix(auth): centralize cert discovery logic and steps
nbayati Jul 10, 2026
5533e0c
test(mtls): fix mTLS auto-enablement test failure in test_grpc.py
nbayati Jul 11, 2026
f95ebd0
minor fixes and clean up
nbayati Jul 11, 2026
8d40f20
chore: fix mtls explicit cert config path existence check
nbayati Jul 12, 2026
05b0d64
update docstring and unit tests
nbayati Jul 12, 2026
9497cb4
fix(auth): revert mTLS auto-enablement configuration warnings to debu…
nbayati Jul 12, 2026
bb69aa9
test(auth): add test coverage for mTLS auto-enablement with invalid J…
nbayati Jul 12, 2026
7522a19
fix: add source tracking for explicit mtls cert path
nbayati Jul 16, 2026
ce6d4f1
Address code review feedback on mTLS refactoring
nbayati Jul 17, 2026
835cc37
address comments
nbayati Jul 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 2 additions & 37 deletions packages/google-auth/google/auth/transport/_mtls_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,20 +61,6 @@
b"-----BEGIN PASSPHRASE-----(.+)-----END PASSPHRASE-----", re.DOTALL
)

# Temporary patch to accomodate incorrect cert config in Cloud Run prod environment.
_WELL_KNOWN_CLOUD_RUN_CERT_PATH = (
"/var/run/secrets/workload-spiffe-credentials/certificates.pem"
)
_WELL_KNOWN_CLOUD_RUN_KEY_PATH = (
"/var/run/secrets/workload-spiffe-credentials/private_key.pem"
)
_INCORRECT_CLOUD_RUN_CERT_PATH = (
"/var/lib/volumes/certificate/workload-certificates/certificates.pem"
)
_INCORRECT_CLOUD_RUN_KEY_PATH = (
"/var/lib/volumes/certificate/workload-certificates/private_key.pem"
)


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

# == BEGIN Temporary Cloud Run PATCH ==
# See https://github.com/googleapis/google-auth-library-python/issues/1881
Comment thread
parthea marked this conversation as resolved.
if (cert_path == _INCORRECT_CLOUD_RUN_CERT_PATH) and (
key_path == _INCORRECT_CLOUD_RUN_KEY_PATH
):
if not path.exists(cert_path) and not path.exists(key_path):
_LOGGER.debug(
"Applying Cloud Run certificate path patch. "
"Configured paths not found: %s, %s. "
"Using well-known paths: %s, %s",
cert_path,
key_path,
_WELL_KNOWN_CLOUD_RUN_CERT_PATH,
_WELL_KNOWN_CLOUD_RUN_KEY_PATH,
)
cert_path = _WELL_KNOWN_CLOUD_RUN_CERT_PATH
key_path = _WELL_KNOWN_CLOUD_RUN_KEY_PATH
# == END Temporary Cloud Run PATCH ==

return cert_path, key_path


Expand Down Expand Up @@ -768,10 +735,8 @@ def check_use_client_cert():

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

# Check if the value of GOOGLE_API_CERTIFICATE_CONFIG is set.
cert_path = getenv(environment_vars.GOOGLE_API_CERTIFICATE_CONFIG) or getenv(
environment_vars.CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH
)
# Check if a workload config file exists.
cert_path = _get_cert_config_path(include_context_aware=True)

if cert_path:
try:
Expand Down
16 changes: 4 additions & 12 deletions packages/google-auth/google/auth/transport/mtls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from google.auth import exceptions
from google.auth.transport import _mtls_helper


_LOGGER = logging.getLogger(__name__)


Expand All @@ -45,24 +44,17 @@ def has_default_client_cert_source(include_context_aware=True):
bool: indicating if the default client cert source exists.
"""
if (
include_context_aware
and _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
_mtls_helper._get_cert_config_path(include_context_aware=include_context_aware)
is not None
):
return True
if (
_mtls_helper._check_config_path(
_mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH
)
include_context_aware
and _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
is not None
):
return True
cert_config_path = getenv("GOOGLE_API_CERTIFICATE_CONFIG")
if (
cert_config_path
and _mtls_helper._check_config_path(cert_config_path) is not None
):
return True

return False


Expand Down
111 changes: 18 additions & 93 deletions packages/google-auth/tests/transport/test__mtls_helper.py
View file
Open in desktop

@lsirac lsirac Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR introduces two untested branches:

  • No test calls has_default_client_cert_source(include_context_aware=False) to verify that setting include_context_aware=False skips checking CONTEXT_AWARE_METADATA_PATH.
  • No test verifies that _get_cert_config_path logs a debug message when an explicitly specified certificate configuration file (is_explicit=True) does not exist.

@nbayati nbayati Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Original file line number Diff line number Diff line change
Expand Up @@ -341,93 +341,6 @@ def test_success_with_certificate_config(
assert key == pytest.private_key_bytes
assert passphrase is None

@mock.patch(
"google.auth.transport._mtls_helper._read_cert_and_key_files", autospec=True
)
@mock.patch(
"google.auth.transport._mtls_helper._get_cert_config_path", autospec=True
)
@mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
def test_success_with_certificate_config_cloud_run_patch(
self,
mock_check_config_path,
mock_load_json_file,
mock_get_cert_config_path,
mock_read_cert_and_key_files,
):
cert_config_path = "/path/to/config"
mock_check_config_path.return_value = cert_config_path
mock_load_json_file.return_value = {
"cert_configs": {
"workload": {
"cert_path": _mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
"key_path": _mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
}
}
}
mock_get_cert_config_path.return_value = cert_config_path
mock_read_cert_and_key_files.return_value = (
pytest.public_cert_bytes,
pytest.private_key_bytes,
)

has_cert, cert, key, passphrase = _mtls_helper.get_client_ssl_credentials()
assert has_cert
assert cert == pytest.public_cert_bytes
assert key == pytest.private_key_bytes
assert passphrase is None

mock_read_cert_and_key_files.assert_called_once_with(
_mtls_helper._WELL_KNOWN_CLOUD_RUN_CERT_PATH,
_mtls_helper._WELL_KNOWN_CLOUD_RUN_KEY_PATH,
)

@mock.patch("os.path.exists", autospec=True)
@mock.patch(
"google.auth.transport._mtls_helper._read_cert_and_key_files", autospec=True
)
@mock.patch(
"google.auth.transport._mtls_helper._get_cert_config_path", autospec=True
)
@mock.patch("google.auth.transport._mtls_helper._load_json_file", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
def test_success_with_certificate_config_cloud_run_patch_skipped_if_cert_exists(
self,
mock_check_config_path,
mock_load_json_file,
mock_get_cert_config_path,
mock_read_cert_and_key_files,
mock_os_path_exists,
):
cert_config_path = "/path/to/config"
mock_check_config_path.return_value = cert_config_path
mock_os_path_exists.return_value = True
mock_load_json_file.return_value = {
"cert_configs": {
"workload": {
"cert_path": _mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
"key_path": _mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
}
}
}
mock_get_cert_config_path.return_value = cert_config_path
mock_read_cert_and_key_files.return_value = (
pytest.public_cert_bytes,
pytest.private_key_bytes,
)

has_cert, cert, key, passphrase = _mtls_helper.get_client_ssl_credentials()
assert has_cert
assert cert == pytest.public_cert_bytes
assert key == pytest.private_key_bytes
assert passphrase is None

mock_read_cert_and_key_files.assert_called_once_with(
_mtls_helper._INCORRECT_CLOUD_RUN_CERT_PATH,
_mtls_helper._INCORRECT_CLOUD_RUN_KEY_PATH,
)

@mock.patch(
"google.auth.transport._mtls_helper._get_workload_cert_and_key", autospec=True
)
Expand Down Expand Up @@ -848,7 +761,9 @@ def test_env_var_explicit_garbage(self):
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
},
)
def test_config_file_success(self, mock_file):
@mock.patch("os.path.exists", autospec=True)
def test_config_file_success(self, mock_exists, mock_file):
mock_exists.return_value = True
# We manually apply mock_open here so we can keep autospec=True on the decorator
mock_file.side_effect = mock.mock_open(
read_data='{"cert_configs": {"workload": "exists"}}'
Expand All @@ -865,7 +780,9 @@ def test_config_file_success(self, mock_file):
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
},
)
def test_config_file_missing_keys(self, mock_file):
@mock.patch("os.path.exists", autospec=True)
def test_config_file_missing_keys(self, mock_exists, mock_file):
mock_exists.return_value = True
mock_file.side_effect = mock.mock_open(read_data='{"cert_configs": {}}')
assert _mtls_helper.check_use_client_cert() is False

Expand All @@ -879,7 +796,9 @@ def test_config_file_missing_keys(self, mock_file):
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
},
)
def test_config_file_bad_json(self, mock_file):
@mock.patch("os.path.exists", autospec=True)
def test_config_file_bad_json(self, mock_exists, mock_file):
mock_exists.return_value = True
mock_file.side_effect = mock.mock_open(read_data="{bad_json")
assert _mtls_helper.check_use_client_cert() is False

Expand All @@ -893,12 +812,16 @@ def test_config_file_bad_json(self, mock_file):
"CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH": "",
},
)
def test_config_file_not_found(self, mock_file):
@mock.patch("os.path.exists", autospec=True)
def test_config_file_not_found(self, mock_exists, mock_file):
mock_exists.return_value = True
mock_file.side_effect = FileNotFoundError
assert _mtls_helper.check_use_client_cert() is False

@mock.patch.dict(os.environ, {}, clear=True)
def test_no_env_vars_set(self):
@mock.patch("os.path.exists", autospec=True)
def test_no_env_vars_set(self, mock_exists):
mock_exists.return_value = False
assert _mtls_helper.check_use_client_cert() is False

def test_use_client_cert_precedence(self):
Expand Down Expand Up @@ -941,7 +864,9 @@ def test_use_client_cert_fallback(self):
assert _mtls_helper.check_use_client_cert() is False

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

Expand Down
65 changes: 25 additions & 40 deletions packages/google-auth/tests/transport/test_mtls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,18 @@
from google.auth.transport import mtls


@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
@mock.patch("google.auth.transport._mtls_helper._check_config_path")
def test_has_default_client_cert_source_with_context_aware_metadata(mock_check):
def test_has_default_client_cert_source_with_context_aware_metadata(
mock_check, mock_get_cert
):
"""
Directly tests the logic: if CONTEXT_AWARE_METADATA_PATH is found, return True.
"""

# Setup: Return a path only for the Context Aware Metadata Path
# Setup: _get_cert_config_path returns None, so it falls back to context aware metadata
mock_get_cert.return_value = None

def side_effect(path):
if path == _mtls_helper.CONTEXT_AWARE_METADATA_PATH:
return "/path/to/context_aware_metadata.json"
Expand All @@ -42,71 +47,51 @@ def side_effect(path):

# Assert
assert result is True
mock_get_cert.assert_called_once_with(include_context_aware=True)
mock_check.assert_any_call(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
assert side_effect("non-matching-path") is None


@mock.patch("google.auth.transport._mtls_helper._check_config_path")
def test_has_default_client_cert_source_falls_back(mock_check):
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
def test_has_default_client_cert_source_falls_back(mock_get_cert, mock_check):
"""
Tests that it skips CONTEXT_AWARE_METADATA_PATH if None, and checks the next path.
Tests that it checks X.509 WIF first, and if found, returns True without checking context aware metadata.
"""

# Setup: First path is None, second path is valid
def side_effect(path):
if path == _mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH:
return "/path/to/default_cert.json"
return None

mock_check.side_effect = side_effect
# Setup: First path is valid
mock_get_cert.return_value = "/path/to/default_cert.json"

# Execute
result = mtls.has_default_client_cert_source(True)

# Assert
assert result is True
# Verify the sequence of calls
expected_calls = [
mock.call(_mtls_helper.CONTEXT_AWARE_METADATA_PATH),
mock.call(_mtls_helper.CERTIFICATE_CONFIGURATION_DEFAULT_PATH),
]
mock_check.assert_has_calls(expected_calls)
mock_get_cert.assert_called_once_with(include_context_aware=True)
mock_check.assert_not_called()


@mock.patch("google.auth.transport.mtls.getenv", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
def test_has_default_client_cert_source_env_var_success(check_config_path, mock_getenv):
# 1. Mock getenv to return our test path
mock_getenv.side_effect = lambda var: (
"path/to/cert.json" if var == "GOOGLE_API_CERTIFICATE_CONFIG" else None
)

# 2. Mock _check_config_path side effect
def side_effect(path):
# Return None for legacy paths to ensure we reach the env var logic
if path == "path/to/cert.json":
return "/absolute/path/to/cert.json"
return None

check_config_path.side_effect = side_effect
def test_has_default_client_cert_source_env_var_success(
check_config_path, get_cert_config_path
):
check_config_path.return_value = None
get_cert_config_path.return_value = "/absolute/path/to/cert.json"

# 3. This should now return True
assert mtls.has_default_client_cert_source(True)

# 4. Verify the env var path was checked
check_config_path.assert_called_with("path/to/cert.json")
get_cert_config_path.assert_called_with(include_context_aware=True)


@mock.patch("google.auth.transport.mtls.getenv", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path", autospec=True)
@mock.patch("google.auth.transport._mtls_helper._check_config_path", autospec=True)
def test_has_default_client_cert_source_env_var_invalid_config_path(
check_config_path, mock_getenv
check_config_path, get_cert_config_path
):
# Set the env var but make the check fail
mock_getenv.side_effect = lambda var: (
"invalid/path" if var == "GOOGLE_API_CERTIFICATE_CONFIG" else None
)
check_config_path.return_value = None
get_cert_config_path.return_value = None

assert not mtls.has_default_client_cert_source(True)

Expand Down
Loading