From fb37ed037cb77255f46d65b14815061f98e0639d Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 4 Nov 2025 14:30:00 -0800 Subject: [PATCH 1/5] feat: Add public wrapper for use_client_Cert function Signed-off-by: Radhika Agrawal --- google/auth/transport/_mtls_helper.py | 34 +++++++++++++-------------- google/auth/transport/grpc.py | 6 ++--- google/auth/transport/mtls.py | 19 +++++++++++++++ google/auth/transport/requests.py | 2 +- google/auth/transport/urllib3.py | 2 +- tests/transport/test__mtls_helper.py | 14 +++++------ tests/transport/test_mtls.py | 11 +++++++++ 7 files changed, 59 insertions(+), 29 deletions(-) diff --git a/google/auth/transport/_mtls_helper.py b/google/auth/transport/_mtls_helper.py index 5ad105a52..bf50e6b5e 100644 --- a/google/auth/transport/_mtls_helper.py +++ b/google/auth/transport/_mtls_helper.py @@ -408,28 +408,27 @@ def client_cert_callback(): def check_use_client_cert(): - """Returns the value of the GOOGLE_API_USE_CLIENT_CERTIFICATE variable, - or an inferred value('true' or 'false') if unset. + """Returns boolean for whether the client certificate should be used for mTLS. - This value is meant to be interpreted as a "true" or "false" value - representing whether the client certificate should be used, but could be any - arbitrary string. - - If GOOGLE_API_USE_CLIENT_CERTIFICATE is unset, the value value will be - inferred by reading a file pointed at by GOOGLE_API_CERTIFICATE_CONFIG, and - verifying it contains a "workload" section. If so, the function will return - "true", otherwise "false". + This value is meant to be interpreted as a boolean representing whether + the client certificate should be used. 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 contains a "workload" + section. If so, the function will return True, otherwise False. Returns: - str: The value of GOOGLE_API_USE_CLIENT_CERTIFICATE, or an inferred value - ("true" or "false") if unset. This string should contain a value, but may - be an any arbitrary string read from the user's set - GOOGLE_API_USE_CLIENT_CERTIFICATE. + bool: Whether the client certificate should be used for mTLS connection. """ use_client_cert = getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE") # Check if the value of GOOGLE_API_USE_CLIENT_CERTIFICATE is set. if use_client_cert: - return use_client_cert.lower() + if use_client_cert.lower() == "true": + return True + # Check if GOOGLE_API_USE_CLIENT_CERTIFICATE is set to false explicitly. + # Invalid values for GOOGLE_API_USE_CLIENT_CERTIFICATE are not handled here. + # That will be handled by the code calling this function. + elif use_client_cert.lower() == "false": + return False else: # Check if the value of GOOGLE_API_CERTIFICATE_CONFIG is set. cert_path = getenv("GOOGLE_API_CERTIFICATE_CONFIG") @@ -439,7 +438,7 @@ def check_use_client_cert(): content = json.load(f) # verify json has workload key content["cert_configs"]["workload"] - return "true" + return True except ( FileNotFoundError, OSError, @@ -448,4 +447,5 @@ def check_use_client_cert(): json.JSONDecodeError, ) as e: _LOGGER.debug("error decoding certificate: %s", e) - return "false" + return False + diff --git a/google/auth/transport/grpc.py b/google/auth/transport/grpc.py index 747c1dcb2..c4cadfd52 100644 --- a/google/auth/transport/grpc.py +++ b/google/auth/transport/grpc.py @@ -255,13 +255,13 @@ def my_client_cert_callback(): # If SSL credentials are not explicitly set, try client_cert_callback and ADC. if not ssl_credentials: use_client_cert = _mtls_helper.check_use_client_cert() - if use_client_cert == "true" and client_cert_callback: + if use_client_cert and client_cert_callback: # Use the callback if provided. cert, key = client_cert_callback() ssl_credentials = grpc.ssl_channel_credentials( certificate_chain=cert, private_key=key ) - elif use_client_cert == "true": + elif use_client_cert: # Use application default SSL credentials. adc_ssl_credentils = SslCredentials() ssl_credentials = adc_ssl_credentils.ssl_credentials @@ -292,7 +292,7 @@ class SslCredentials: def __init__(self): use_client_cert = _mtls_helper.check_use_client_cert() - if use_client_cert != "true": + if use_client_cert: self._is_mtls = False else: # Load client SSL credentials. diff --git a/google/auth/transport/mtls.py b/google/auth/transport/mtls.py index e7a7304f6..fce1f3836 100644 --- a/google/auth/transport/mtls.py +++ b/google/auth/transport/mtls.py @@ -110,3 +110,22 @@ def callback(): return cert_path, key_path, passphrase_bytes return callback + +def should_use_client_cert(): + """Returns boolean for whether the client certificate should be used for mTLS. + + This is a wrapper around _mtls_helper.check_use_client_cert(). + The value is meant to be interpreted as a boolean representing whether + the client certificate should be used. 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 contains a "workload" + section. If so, the function will return True, otherwise False. Also, note + that if GOOGLE_API_USE_CLIENT_CERTIFICATE is set but is not 'true' or 'false' + (case-insensitive), this check is inconclusive, that case should be handled + by the caller. + + Returns: + bool: indicating whether the client certificate should be used for mTLS. + """ + return _mtls_helper.check_use_client_cert() + diff --git a/google/auth/transport/requests.py b/google/auth/transport/requests.py index 9e1f15751..7a45d5655 100644 --- a/google/auth/transport/requests.py +++ b/google/auth/transport/requests.py @@ -443,7 +443,7 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. """ use_client_cert = google.auth.transport._mtls_helper.check_use_client_cert() - if use_client_cert != "true": + if use_client_cert: self._is_mtls = False return try: diff --git a/google/auth/transport/urllib3.py b/google/auth/transport/urllib3.py index 01be1dd05..915891e4e 100644 --- a/google/auth/transport/urllib3.py +++ b/google/auth/transport/urllib3.py @@ -334,7 +334,7 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. """ use_client_cert = transport._mtls_helper.check_use_client_cert() - if use_client_cert != "true": + if use_client_cert: return False try: import OpenSSL diff --git a/tests/transport/test__mtls_helper.py b/tests/transport/test__mtls_helper.py index c4959c1bc..5b8e31aba 100644 --- a/tests/transport/test__mtls_helper.py +++ b/tests/transport/test__mtls_helper.py @@ -643,7 +643,7 @@ def test_crypto_error(self): def test_check_use_client_cert(self, monkeypatch): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true") use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "true" + assert use_client_cert def test_check_use_client_cert_for_workload_with_config_file(self, monkeypatch): config_data = { @@ -663,19 +663,19 @@ def test_check_use_client_cert_for_workload_with_config_file(self, monkeypatch): mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "true" + assert use_client_cert def test_check_use_client_cert_false(self, monkeypatch): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false") use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "false" + assert not use_client_cert def test_check_use_client_cert_for_workload_with_config_file_not_found( self, monkeypatch ): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "") use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "false" + assert not use_client_cert def test_check_use_client_cert_for_workload_with_config_file_not_json( self, monkeypatch @@ -688,7 +688,7 @@ def test_check_use_client_cert_for_workload_with_config_file_not_json( mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "false" + assert not use_client_cert def test_check_use_client_cert_for_workload_with_config_file_no_workload( self, monkeypatch @@ -702,11 +702,11 @@ def test_check_use_client_cert_for_workload_with_config_file_no_workload( mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "false" + assert not use_client_cert def test_check_use_client_cert_when_file_does_not_exist(self, monkeypatch): config_filename = "mock_certificate_config.json" monkeypatch.setenv("GOOGLE_API_CERTIFICATE_CONFIG", config_filename) monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "") use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert == "false" + assert not use_client_cert diff --git a/tests/transport/test_mtls.py b/tests/transport/test_mtls.py index ea549ae14..1178c09f6 100644 --- a/tests/transport/test_mtls.py +++ b/tests/transport/test_mtls.py @@ -94,3 +94,14 @@ def test_default_client_encrypted_cert_source( callback = mtls.default_client_encrypted_cert_source("cert_path", "key_path") with pytest.raises(exceptions.MutualTLSChannelError): callback() + +@mock.patch( + "google.auth.transport._mtls_helper.check_use_client_cert", autospec=True +) +def test_should_use_client_cert(check_use_client_cert): + check_use_client_cert.return_value = mock.Mock() + assert mtls.should_use_client_cert() + + check_use_client_cert.return_value = False + assert not mtls.should_use_client_cert() + From 39bff9b8e74cf280622924dba9452c6eaed3d33f Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 4 Nov 2025 14:31:14 -0800 Subject: [PATCH 2/5] fix: update lint fixes Signed-off-by: Radhika Agrawal --- google/auth/transport/_mtls_helper.py | 1 - google/auth/transport/mtls.py | 2 +- tests/transport/test_mtls.py | 6 ++---- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/google/auth/transport/_mtls_helper.py b/google/auth/transport/_mtls_helper.py index bf50e6b5e..d400421b7 100644 --- a/google/auth/transport/_mtls_helper.py +++ b/google/auth/transport/_mtls_helper.py @@ -448,4 +448,3 @@ def check_use_client_cert(): ) as e: _LOGGER.debug("error decoding certificate: %s", e) return False - diff --git a/google/auth/transport/mtls.py b/google/auth/transport/mtls.py index fce1f3836..98164d47f 100644 --- a/google/auth/transport/mtls.py +++ b/google/auth/transport/mtls.py @@ -111,6 +111,7 @@ def callback(): return callback + def should_use_client_cert(): """Returns boolean for whether the client certificate should be used for mTLS. @@ -128,4 +129,3 @@ def should_use_client_cert(): bool: indicating whether the client certificate should be used for mTLS. """ return _mtls_helper.check_use_client_cert() - diff --git a/tests/transport/test_mtls.py b/tests/transport/test_mtls.py index 1178c09f6..ef3053958 100644 --- a/tests/transport/test_mtls.py +++ b/tests/transport/test_mtls.py @@ -95,13 +95,11 @@ def test_default_client_encrypted_cert_source( with pytest.raises(exceptions.MutualTLSChannelError): callback() -@mock.patch( - "google.auth.transport._mtls_helper.check_use_client_cert", autospec=True -) + +@mock.patch("google.auth.transport._mtls_helper.check_use_client_cert", autospec=True) def test_should_use_client_cert(check_use_client_cert): check_use_client_cert.return_value = mock.Mock() assert mtls.should_use_client_cert() check_use_client_cert.return_value = False assert not mtls.should_use_client_cert() - From ff1de0a3b6efebf4e3f83117fd5be41b565456fe Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 4 Nov 2025 16:01:32 -0800 Subject: [PATCH 3/5] fix: Fix docstrings and minor nits Signed-off-by: Radhika Agrawal --- google/auth/transport/_mtls_helper.py | 16 +++++++--------- google/auth/transport/grpc.py | 2 +- google/auth/transport/mtls.py | 14 ++++++-------- google/auth/transport/requests.py | 2 +- google/auth/transport/urllib3.py | 2 +- tests/transport/test__mtls_helper.py | 14 +++++++------- 6 files changed, 23 insertions(+), 27 deletions(-) diff --git a/google/auth/transport/_mtls_helper.py b/google/auth/transport/_mtls_helper.py index d400421b7..df331c314 100644 --- a/google/auth/transport/_mtls_helper.py +++ b/google/auth/transport/_mtls_helper.py @@ -410,11 +410,12 @@ def client_cert_callback(): def check_use_client_cert(): """Returns boolean for whether the client certificate should be used for mTLS. - This value is meant to be interpreted as a boolean representing whether - the client certificate should be used. 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 contains a "workload" - section. If so, the function will return True, otherwise False. + If GOOGLE_API_USE_CLIENT_CERTIFICATE is set to true or false, a corresponding + 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 contains a "workload" section. If so, the function will return True, + otherwise False. Returns: bool: Whether the client certificate should be used for mTLS connection. @@ -424,10 +425,7 @@ def check_use_client_cert(): if use_client_cert: if use_client_cert.lower() == "true": return True - # Check if GOOGLE_API_USE_CLIENT_CERTIFICATE is set to false explicitly. - # Invalid values for GOOGLE_API_USE_CLIENT_CERTIFICATE are not handled here. - # That will be handled by the code calling this function. - elif use_client_cert.lower() == "false": + else: return False else: # Check if the value of GOOGLE_API_CERTIFICATE_CONFIG is set. diff --git a/google/auth/transport/grpc.py b/google/auth/transport/grpc.py index c4cadfd52..d9185e7aa 100644 --- a/google/auth/transport/grpc.py +++ b/google/auth/transport/grpc.py @@ -292,7 +292,7 @@ class SslCredentials: def __init__(self): use_client_cert = _mtls_helper.check_use_client_cert() - if use_client_cert: + if not use_client_cert: self._is_mtls = False else: # Load client SSL credentials. diff --git a/google/auth/transport/mtls.py b/google/auth/transport/mtls.py index 98164d47f..75e43687f 100644 --- a/google/auth/transport/mtls.py +++ b/google/auth/transport/mtls.py @@ -116,14 +116,12 @@ def should_use_client_cert(): """Returns boolean for whether the client certificate should be used for mTLS. This is a wrapper around _mtls_helper.check_use_client_cert(). - The value is meant to be interpreted as a boolean representing whether - the client certificate should be used. 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 contains a "workload" - section. If so, the function will return True, otherwise False. Also, note - that if GOOGLE_API_USE_CLIENT_CERTIFICATE is set but is not 'true' or 'false' - (case-insensitive), this check is inconclusive, that case should be handled - by the caller. + If GOOGLE_API_USE_CLIENT_CERTIFICATE is set to true or false, a corresponding + 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 + contains a "workload" section. If so, the function will return True, + otherwise False. Returns: bool: indicating whether the client certificate should be used for mTLS. diff --git a/google/auth/transport/requests.py b/google/auth/transport/requests.py index 7a45d5655..d1ff8f368 100644 --- a/google/auth/transport/requests.py +++ b/google/auth/transport/requests.py @@ -443,7 +443,7 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. """ use_client_cert = google.auth.transport._mtls_helper.check_use_client_cert() - if use_client_cert: + if not use_client_cert: self._is_mtls = False return try: diff --git a/google/auth/transport/urllib3.py b/google/auth/transport/urllib3.py index 915891e4e..353cb8e08 100644 --- a/google/auth/transport/urllib3.py +++ b/google/auth/transport/urllib3.py @@ -334,7 +334,7 @@ def configure_mtls_channel(self, client_cert_callback=None): creation failed for any reason. """ use_client_cert = transport._mtls_helper.check_use_client_cert() - if use_client_cert: + if not use_client_cert: return False try: import OpenSSL diff --git a/tests/transport/test__mtls_helper.py b/tests/transport/test__mtls_helper.py index 5b8e31aba..a93d0e9e7 100644 --- a/tests/transport/test__mtls_helper.py +++ b/tests/transport/test__mtls_helper.py @@ -643,7 +643,7 @@ def test_crypto_error(self): def test_check_use_client_cert(self, monkeypatch): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true") use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert + assert use_client_cert is True def test_check_use_client_cert_for_workload_with_config_file(self, monkeypatch): config_data = { @@ -663,19 +663,19 @@ def test_check_use_client_cert_for_workload_with_config_file(self, monkeypatch): mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert use_client_cert + assert use_client_cert is True def test_check_use_client_cert_false(self, monkeypatch): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false") use_client_cert = _mtls_helper.check_use_client_cert() - assert not use_client_cert + assert use_client_cert is False def test_check_use_client_cert_for_workload_with_config_file_not_found( self, monkeypatch ): monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "") use_client_cert = _mtls_helper.check_use_client_cert() - assert not use_client_cert + assert use_client_cert is False def test_check_use_client_cert_for_workload_with_config_file_not_json( self, monkeypatch @@ -688,7 +688,7 @@ def test_check_use_client_cert_for_workload_with_config_file_not_json( mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert not use_client_cert + assert use_client_cert is False def test_check_use_client_cert_for_workload_with_config_file_no_workload( self, monkeypatch @@ -702,11 +702,11 @@ def test_check_use_client_cert_for_workload_with_config_file_no_workload( mock_file_handle = mock.mock_open(read_data=config_file_content) with mock.patch("builtins.open", mock_file_handle): use_client_cert = _mtls_helper.check_use_client_cert() - assert not use_client_cert + assert use_client_cert is False def test_check_use_client_cert_when_file_does_not_exist(self, monkeypatch): config_filename = "mock_certificate_config.json" monkeypatch.setenv("GOOGLE_API_CERTIFICATE_CONFIG", config_filename) monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "") use_client_cert = _mtls_helper.check_use_client_cert() - assert not use_client_cert + assert use_client_cert is False From 8fa9fb6d84a31167c2e76535b72cbec5cff9d9c4 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 4 Nov 2025 16:08:09 -0800 Subject: [PATCH 4/5] fix: check update in _mtls_helper Signed-off-by: Radhika Agrawal --- google/auth/transport/_mtls_helper.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/google/auth/transport/_mtls_helper.py b/google/auth/transport/_mtls_helper.py index df331c314..026280228 100644 --- a/google/auth/transport/_mtls_helper.py +++ b/google/auth/transport/_mtls_helper.py @@ -423,10 +423,7 @@ def check_use_client_cert(): use_client_cert = getenv("GOOGLE_API_USE_CLIENT_CERTIFICATE") # Check if the value of GOOGLE_API_USE_CLIENT_CERTIFICATE is set. if use_client_cert: - if use_client_cert.lower() == "true": - return True - else: - return False + return use_client_cert.lower() == "true" else: # Check if the value of GOOGLE_API_CERTIFICATE_CONFIG is set. cert_path = getenv("GOOGLE_API_CERTIFICATE_CONFIG") From cf6df546fd2ef077076a02daf9e9ec9162a76a77 Mon Sep 17 00:00:00 2001 From: Radhika Agrawal Date: Tue, 4 Nov 2025 16:29:43 -0800 Subject: [PATCH 5/5] chore: Add unit test for specific case and improve docstring Signed-off-by: Radhika Agrawal --- google/auth/transport/_mtls_helper.py | 3 ++- tests/transport/test__mtls_helper.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/google/auth/transport/_mtls_helper.py b/google/auth/transport/_mtls_helper.py index 026280228..7740f2fe8 100644 --- a/google/auth/transport/_mtls_helper.py +++ b/google/auth/transport/_mtls_helper.py @@ -411,7 +411,8 @@ def check_use_client_cert(): """Returns boolean for whether the client certificate should be used for mTLS. If GOOGLE_API_USE_CLIENT_CERTIFICATE is set to true or false, a corresponding - bool value will be returned. + bool value will be returned. If the value is set to an unexpected string, it + will default to False. 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 contains a "workload" section. If so, the function will return True, diff --git a/tests/transport/test__mtls_helper.py b/tests/transport/test__mtls_helper.py index a93d0e9e7..01d5e3a40 100644 --- a/tests/transport/test__mtls_helper.py +++ b/tests/transport/test__mtls_helper.py @@ -670,6 +670,11 @@ def test_check_use_client_cert_false(self, monkeypatch): use_client_cert = _mtls_helper.check_use_client_cert() assert use_client_cert is False + def test_check_use_client_cert_unsupported_value(self, monkeypatch): + monkeypatch.setenv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "dummy") + use_client_cert = _mtls_helper.check_use_client_cert() + assert use_client_cert is False + def test_check_use_client_cert_for_workload_with_config_file_not_found( self, monkeypatch ):