fix(auth): centralize cert discovery logic and steps by nbayati · Pull Request #17696 · googleapis/google-cloud-python

nbayati · 2026-07-11T00:56:42Z

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.
fixes Auth(mtls): _has_logged_mtls_warning masks distinct mTLS configuration errors #17697

- 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.

gemini-code-assist

Code Review

This pull request removes the temporary Cloud Run certificate path patch and its associated tests. It also refactors and simplifies certificate configuration path resolution in check_use_client_cert and has_default_client_cert_source by utilizing the helper function _get_cert_config_path, updating the corresponding unit tests to match. I have no feedback to provide as there are no review comments.

…g level

…SON structure

parthea · 2026-07-13T16:14:25Z

/gemini review

gemini-code-assist

Code Review

This pull request refactors mTLS configuration path resolution by centralizing path retrieval in _get_cert_config_path and removing the temporary Cloud Run patch. It also changes mTLS auto-enablement failure logging from warnings to debug messages. The review feedback highlights a potential TypeError crash if the loaded certificate configuration JSON is not a dictionary, and suggests validating the JSON type before accessing its keys.

gemini-code-assist · 2026-07-13T16:17:09Z

        try:
            with open(cert_path, "r") as f:
                content = json.load(f)
        except (FileNotFoundError, OSError, json.JSONDecodeError) as e:


If the certificate configuration file contains a non-dictionary JSON value (such as null, 123, or true), json.load(f) will return a non-dict type. Checking "cert_configs" in content will raise a TypeError and crash the application.

Furthermore, to prevent security vulnerabilities or confusing downstream authorization errors, we must not silently fall back to standard TLS when mTLS configuration or certificate loading fails. Instead, we should fail fast by raising an appropriate exception.

try: with open(cert_path, "r") as f: content = json.load(f) if not isinstance(content, dict): raise ValueError("Certificate configuration file is not a JSON object.") except (FileNotFoundError, OSError, json.JSONDecodeError) as e:

References

Do not silently fall back to standard TLS if mutual TLS (mTLS) configuration or certificate loading fails. Silently falling back can introduce security vulnerabilities or cause confusing downstream authorization errors; instead, fail fast by raising an appropriate exception.

The first point is incorrect. The code already explicitly performs an isinstance(content, dict) check before interacting with the JSON payload, so no TypeError can occur.

Regarding the suggestion to raise an exception: check_use_client_cert() has historically fallen back to standard TLS by returning False if auto-enablement cannot proceed. Changing this behavior to raise an exception would be a breaking change for downstream users and violates backwards compatibility. We will stick with the current graceful fallback behavior.

andyrzhao

LGTM overall - thanks for working on this Negar!

lsirac · 2026-07-16T16:09:25Z

-    def test_no_env_vars_set(self):
+    @mock.patch("os.path.exists", autospec=True)
+    def test_no_env_vars_set(self, mock_exists, mock_open):
+        _mtls_helper._has_logged_mtls_warning = False


_has_logged_mtls_warning from _mtls_helper.py was deleted, but this still sets _mtls_helper._has_logged_mtls_warning = False

Good catch! I removed this leftover line since we no longer use that flag for warnings.

lsirac · 2026-07-16T16:11:27Z

    bool value will be returned
    If GOOGLE_API_USE_CLIENT_CERTIFICATE is unset, the value will be inferred by
-    reading a file pointed at by GOOGLE_API_CERTIFICATE_CONFIG, and verifying it
+    reading a file pointed at by GOOGLE_API_CERTIFICATE_CONFIG or


The docstring for check_use_client_cert() in _mtls_helper.py lists three paths: GOOGLE_API_CERTIFICATE_CONFIG, CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH, and ~/.config/gcloud/certificate_config.json. The updated docstring for should_use_client_cert() in mtls.py omits the default ~/.config/gcloud/certificate_config.json path.

lsirac · 2026-07-16T16:11:52Z

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.

lsirac

Gemini Review:

1. Hardcoded Unix path breaks default `gcloud` discovery on Windows

_mtls_helper.py — Line 26

Issue: CERTIFICATE_CONFIGURATION_DEFAULT_PATH hardcodes "~/.config/gcloud/certificate_config.json" as a POSIX string literal. On Windows, the gcloud CLI defaults to storing configuration in %APPDATA%\gcloud (C:\Users\<username>\AppData\Roaming\gcloud\), not ~/.config/gcloud/. As a result, auto-enablement on Windows will fail to discover the default gcloud certificate config.
Fix: Construct the default path dynamically based on OS or check %APPDATA%\gcloud\certificate_config.json when running on Windows so default gcloud certificate discovery works across OSes.

2. `_get_cert_config_path` ignores `CLOUDSDK_CONFIG`

_mtls_helper.py — Lines 440–450

Issue: _get_cert_config_path checks GOOGLE_API_CERTIFICATE_CONFIG and CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH, but ignores CLOUDSDK_CONFIG (the standard environment variable used to specify a custom gcloud configuration directory). When a user sets CLOUDSDK_CONFIG=/custom/path/gcloud, gcloud writes certificate_config.json inside $CLOUDSDK_CONFIG/, but _get_cert_config_path falls back straight to ~/.config/gcloud/ and fails to discover the file.
Fix: Check os.environ.get("CLOUDSDK_CONFIG") in _get_cert_config_path before falling back to CERTIFICATE_CONFIGURATION_DEFAULT_PATH, resolving os.path.join(cloudsdk_config, "certificate_config.json") if present.

3. _mtls_helper.py (Line 418) — Docstring nitpick

The docstring for _get_cert_config_path omits the include_context_aware: bool parameter description. Adding it to the Args: section would keep documentation complete.
Release / Behavior Note for check_use_client_cert

Because check_use_client_cert() now delegates to _get_cert_config_path(include_context_aware=True), it will inspect ~/.config/gcloud/certificate_config.json by default when environment variables are unset. If that file contains a "workload" section, mTLS will auto-enable on developer machines. (This is an intended feature change, but good to highlight in release notes).

nbayati requested review from a team as code owners July 11, 2026 00:56

nbayati requested a review from andyrzhao July 11, 2026 00:58

gemini-code-assist Bot reviewed Jul 11, 2026

View reviewed changes

nbayati added 2 commits July 10, 2026 18:12

test(mtls): fix mTLS auto-enablement test failure in test_grpc.py

5533e0c

minor fixes and clean up

f95ebd0

nbayati force-pushed the centralize_cert_path branch from d91693a to f95ebd0 Compare July 11, 2026 23:32

nbayati added 4 commits July 11, 2026 19:23

chore: fix mtls explicit cert config path existence check

8d40f20

update docstring and unit tests

05b0d64

fix(auth): revert mTLS auto-enablement configuration warnings to debu…

9497cb4

…g level

test(auth): add test coverage for mTLS auto-enablement with invalid J…

bb69aa9

…SON structure

parthea reviewed Jul 13, 2026

View reviewed changes

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

parthea assigned nbayati Jul 13, 2026

gemini-code-assist Bot reviewed Jul 13, 2026

View reviewed changes

andyrzhao approved these changes Jul 13, 2026

View reviewed changes

Comment thread packages/google-auth/google/auth/transport/_mtls_helper.py Outdated

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

parthea mentioned this pull request Jul 13, 2026

Cloud Run Cert-Misconfiguration Temporary Patch (tracking bug) #15137

Closed

fix: add source tracking for explicit mtls cert path

7522a19

macastelaz approved these changes Jul 16, 2026

View reviewed changes

lsirac requested changes Jul 16, 2026

View reviewed changes

Address code review feedback on mTLS refactoring

ce6d4f1

nbayati requested a review from lsirac July 17, 2026 15:31

lsirac approved these changes Jul 17, 2026

View reviewed changes

lsirac requested changes Jul 17, 2026

View reviewed changes

address comments

835cc37

lsirac approved these changes Jul 18, 2026

View reviewed changes

Uh oh!

Conversation

nbayati commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

parthea commented Jul 13, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

andyrzhao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lsirac Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

nbayati Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

lsirac left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

1. Hardcoded Unix path breaks default gcloud discovery on Windows

2. _get_cert_config_path ignores CLOUDSDK_CONFIG

3. _mtls_helper.py (Line 418) — Docstring nitpick

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nbayati commented Jul 11, 2026 •

edited

Loading

lsirac left a comment •

edited

Loading

1. Hardcoded Unix path breaks default `gcloud` discovery on Windows

2. `_get_cert_config_path` ignores `CLOUDSDK_CONFIG`