Skip to content

Commit ce6d4f1

Browse files
committed
Address code review feedback on mTLS refactoring
1 parent 7522a19 commit ce6d4f1

3 files changed

Lines changed: 26 additions & 3 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ def should_use_client_cert():
139139
bool value will be returned
140140
If GOOGLE_API_USE_CLIENT_CERTIFICATE is unset, the value will be inferred by
141141
reading a file pointed at by GOOGLE_API_CERTIFICATE_CONFIG or
142-
CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH, and verifying it
142+
CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH, or the default path
143+
like ~/.config/gcloud/certificate_config.json, and verifying it
143144
contains a "workload" section. If so, the function will return True,
144145
otherwise False.
145146

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -588,10 +588,16 @@ def test_success_with_override(self):
588588
returned_path = _mtls_helper._get_cert_config_path(config_path)
589589
assert returned_path == config_path
590590

591-
def test_override_does_not_exist(self):
591+
@mock.patch("google.auth.transport._mtls_helper._LOGGER.debug")
592+
def test_override_does_not_exist(self, mock_debug):
592593
config_path = "fake/file/path"
593594
returned_path = _mtls_helper._get_cert_config_path(config_path)
594595
assert returned_path is None
596+
mock_debug.assert_called_once_with(
597+
"Certificate configuration file explicitly specified via %s at %s does not exist",
598+
"function argument",
599+
"fake/file/path",
600+
)
595601

596602
@mock.patch.dict(
597603
os.environ,
@@ -878,7 +884,6 @@ def test_config_file_invalid_json_type(self, mock_exists, mock_file):
878884
@mock.patch.dict(os.environ, {}, clear=True)
879885
@mock.patch("os.path.exists", autospec=True)
880886
def test_no_env_vars_set(self, mock_exists, mock_open):
881-
_mtls_helper._has_logged_mtls_warning = False
882887
mock_exists.return_value = False
883888
mock_open.side_effect = FileNotFoundError()
884889
assert _mtls_helper.check_use_client_cert() is False

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,23 @@ def side_effect(path):
5151
mock_check.assert_any_call(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
5252

5353

54+
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
55+
@mock.patch("google.auth.transport._mtls_helper._check_config_path")
56+
def test_has_default_client_cert_source_without_context_aware(
57+
mock_check, mock_get_cert
58+
):
59+
"""
60+
Tests that if include_context_aware is False, it skips checking CONTEXT_AWARE_METADATA_PATH.
61+
"""
62+
mock_get_cert.return_value = None
63+
64+
result = mtls.has_default_client_cert_source(include_context_aware=False)
65+
66+
assert result is False
67+
mock_get_cert.assert_called_once_with(include_context_aware=False)
68+
mock_check.assert_not_called()
69+
70+
5471
@mock.patch("google.auth.transport._mtls_helper._check_config_path")
5572
@mock.patch("google.auth.transport._mtls_helper._get_cert_config_path")
5673
def test_has_default_client_cert_source_falls_back(mock_get_cert, mock_check):

0 commit comments

Comments
 (0)