chore: Update based on reviewer comments, updated helpers so that the sync helpers can be reused

Signed-off-by: Radhika Agrawal <agrawalradhika@google.com>

Author: agrawalradhika-cell

google/auth/aio/transport/mtls.py

@@ -18,32 +18,14 @@
 
 import asyncio
 import logging
-from os import getenv, path
 
 from google.auth import exceptions
 import google.auth.transport._mtls_helper
+import google.auth.transport.mtls
 
-CERTIFICATE_CONFIGURATION_DEFAULT_PATH = "~/.config/gcloud/certificate_config.json"
 _LOGGER = logging.getLogger(__name__)
 
 
-def _check_config_path(config_path):
-    """Checks for config file path. If it exists, returns the absolute path with user expansion;
-    otherwise returns None.
-
-    Args:
-        config_path (str): The config file path for certificate_config.json for example
-
-    Returns:
-        str: absolute path if exists and None otherwise.
-    """
-    config_path = path.expanduser(config_path)
-    if not path.exists(config_path):
-        _LOGGER.debug("%s is not found.", config_path)
-        return None
-    return config_path
-
-
 async def _run_in_executor(func, *args):
     """Run a blocking function in an executor to avoid blocking the event loop.
 
@@ -58,20 +40,6 @@ async def _run_in_executor(func, *args):
         return await loop.run_in_executor(None, func, *args)
 
 
-def has_default_client_cert_source():
-    """Check if default client SSL credentials exists on the device.
-
-    Returns:
-        bool: indicating if the default client cert source exists.
-    """
-    if _check_config_path(CERTIFICATE_CONFIGURATION_DEFAULT_PATH) is not None:
-        return True
-    cert_config_path = getenv("GOOGLE_API_CERTIFICATE_CONFIG")
-    if cert_config_path and _check_config_path(cert_config_path) is not None:
-        return True
-    return False
-
-
 def default_client_cert_source():
     """Get a callback which returns the default client SSL credentials.
 
@@ -83,7 +51,9 @@ def default_client_cert_source():
         google.auth.exceptions.DefaultClientCertSourceError: If the default
             client SSL credentials don't exist or are malformed.
     """
-    if not has_default_client_cert_source():
+    if not google.auth.transport.mtls.has_default_client_cert_source(
+        include_context_aware=False
+    ):
         raise exceptions.MutualTLSChannelError(
             "Default client cert source doesn't exist"
         )
@@ -126,6 +96,7 @@ async def get_client_ssl_credentials(
     cert, key = await _run_in_executor(
         google.auth.transport._mtls_helper._get_workload_cert_and_key,
         certificate_config_path,
+        False,
     )
 
     if cert and key:
@@ -155,13 +126,11 @@ async def get_client_cert_and_key(client_cert_callback=None):
             the cert and key.
     """
     if client_cert_callback:
+        result = client_cert_callback()
         try:
-            # If it's awaitable, this works.
-            cert, key = await client_cert_callback()
+            cert, key = await result
         except TypeError:
-            # If it's not awaitable (e.g., a tuple), result is already the data.
-            cert, key = client_cert_callback()
-
+            cert, key = result
         return True, cert, key
 
     has_cert, cert, key, _ = await get_client_ssl_credentials()

google/auth/transport/_mtls_helper.py

@@ -25,6 +25,8 @@
 from google.auth import exceptions
 
 CONTEXT_AWARE_METADATA_PATH = "~/.secureConnect/context_aware_metadata.json"
+
+# Default gcloud config path, to be used with path.expanduser for cross-platform compatibility.
 CERTIFICATE_CONFIGURATION_DEFAULT_PATH = "~/.config/gcloud/certificate_config.json"
 _CERT_PROVIDER_COMMAND = "cert_provider_command"
 _CERT_REGEX = re.compile(
@@ -103,14 +105,18 @@ def _load_json_file(path):
     return json_data
 
 
-def _get_workload_cert_and_key(certificate_config_path=None):
+def _get_workload_cert_and_key(
+    certificate_config_path=None, include_context_aware=True
+):
     """Read the workload identity cert and key files specified in the certificate config provided.
     If no config path is provided, check the environment variable: "GOOGLE_API_CERTIFICATE_CONFIG"
     first, then the well known gcloud location: "~/.config/gcloud/certificate_config.json".
 
     Args:
         certificate_config_path (string): The certificate config path. If no path is provided,
         the environment variable will be checked first, then the well known gcloud location.
+        include_context_aware (bool): If context aware metadata path should be checked for the
+        SecureConnect mTLS configuration.
 
     Returns:
         Tuple[Optional[bytes], Optional[bytes]]: client certificate bytes in PEM format and key
@@ -121,15 +127,17 @@ def _get_workload_cert_and_key(certificate_config_path=None):
         the certificate or key information.
     """
 
-    cert_path, key_path = _get_workload_cert_and_key_paths(certificate_config_path)
+    cert_path, key_path = _get_workload_cert_and_key_paths(
+        certificate_config_path, include_context_aware
+    )
 
     if cert_path is None and key_path is None:
         return None, None
 
     return _read_cert_and_key_files(cert_path, key_path)
 
 
-def _get_cert_config_path(certificate_config_path=None):
+def _get_cert_config_path(certificate_config_path=None, include_context_aware=True):
     """Get the certificate configuration path based on the following order:
 
     1: Explicit override, if set
@@ -141,6 +149,8 @@ def _get_cert_config_path(certificate_config_path=None):
     Args:
         certificate_config_path (string): The certificate config path. If provided, the well known
         location and environment variable will be ignored.
+        include_context_aware (bool): If context aware metadata path should be checked for the
+        SecureConnect mTLS configuration.
 
     Returns:
         The absolute path of the certificate config file, and None if the file does not exist.
@@ -155,7 +165,7 @@ def _get_cert_config_path(certificate_config_path=None):
                 environment_vars.CLOUDSDK_CONTEXT_AWARE_CERTIFICATE_CONFIG_FILE_PATH,
                 None,
             )
-            if env_path is not None and env_path != "":
+            if include_context_aware and env_path is not None and env_path != "":
                 certificate_config_path = env_path
             else:
                 certificate_config_path = CERTIFICATE_CONFIGURATION_DEFAULT_PATH
@@ -166,8 +176,8 @@ def _get_cert_config_path(certificate_config_path=None):
     return certificate_config_path
 
 
-def _get_workload_cert_and_key_paths(config_path):
-    absolute_path = _get_cert_config_path(config_path)
+def _get_workload_cert_and_key_paths(config_path, include_context_aware=True):
+    absolute_path = _get_cert_config_path(config_path, include_context_aware)
     if absolute_path is None:
         return None, None
 

google/auth/transport/mtls.py

@@ -20,14 +20,19 @@
 from google.auth.transport import _mtls_helper
 
 
-def has_default_client_cert_source():
+def has_default_client_cert_source(include_context_aware):
     """Check if default client SSL credentials exists on the device.
 
+    Args:
+       include_context_aware (bool): include_context_aware indicates if context_aware
+       path location will be checked or should it be skipped.
+
     Returns:
         bool: indicating if the default client cert source exists.
     """
     if (
-        _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
+        include_context_aware
+        and _mtls_helper._check_config_path(_mtls_helper.CONTEXT_AWARE_METADATA_PATH)
         is not None
     ):
         return True
@@ -58,7 +63,7 @@ def default_client_cert_source():
         google.auth.exceptions.DefaultClientCertSourceError: If the default
             client SSL credentials don't exist or are malformed.
     """
-    if not has_default_client_cert_source():
+    if not has_default_client_cert_source(include_context_aware=True):
         raise exceptions.MutualTLSChannelError(
             "Default client cert source doesn't exist"
         )
@@ -94,7 +99,7 @@ def default_client_encrypted_cert_source(cert_path, key_path):
         google.auth.exceptions.DefaultClientCertSourceError: If any problem
             occurs when loading or saving the client certificate and key.
     """
-    if not has_default_client_cert_source():
+    if not has_default_client_cert_source(include_context_aware=True):
         raise exceptions.MutualTLSChannelError(
             "Default client encrypted cert source doesn't exist"
         )

tests/transport/test_aio_mtls_helper.py

@@ -24,49 +24,13 @@
 
 
 class TestMTLS:
-    @mock.patch("google.auth.aio.transport.mtls.path.expanduser")
-    @mock.patch("google.auth.aio.transport.mtls.path.exists")
-    def test__check_config_path_exists(self, mock_exists, mock_expand):
-        mock_expand.side_effect = lambda x: x.replace("~", "/home/user")
-        mock_exists.return_value = True
-
-        input_path = "~/config.json"
-        expected_path = "/home/user/config.json"
-        result = mtls._check_config_path(input_path)
-
-        assert result == expected_path
-        mock_exists.assert_called_with(expected_path)
-
-    @mock.patch("google.auth.aio.transport.mtls.path.exists", return_value=False)
-    def test__check_config_path_not_found(self, mock_exists):
-        result = mtls._check_config_path("nonexistent.json")
-        assert result is None
-
-    @mock.patch("google.auth.aio.transport.mtls._check_config_path")
-    @mock.patch("google.auth.aio.transport.mtls.getenv")
-    def test_has_default_client_cert_source_env_var(self, mock_getenv, mock_check):
-        custom_path = "/custom/path.json"
-        mock_check.side_effect = lambda x: custom_path if x == custom_path else None
-        mock_getenv.return_value = custom_path
-
-        assert mtls.has_default_client_cert_source() is True
-
-    @mock.patch("google.auth.aio.transport.mtls._check_config_path")
-    @mock.patch("google.auth.aio.transport.mtls.getenv")
-    def test_has_default_client_cert_source_check_priority(
-        self, mock_getenv, mock_check
-    ):
-        mock_check.return_value = "/default/path.json"
-
-        assert mtls.has_default_client_cert_source() is True
-        mock_getenv.assert_not_called()
-
+    @pytest.mark.asyncio
     @mock.patch(
-        "google.auth.aio.transport.mtls.has_default_client_cert_source",
-        return_value=False,
+        "google.auth.transport.mtls.has_default_client_cert_source", return_value=False
     )
-    def test_default_client_cert_source_none(self, mock_has_default):
-        with pytest.raises(exceptions.MutualTLSChannelError):
+    async def test_default_client_cert_source_not_found(self, mock_has_default):
+        """Tests that a MutualTLSChannelError is raised if no cert source exists."""
+        with pytest.raises(exceptions.MutualTLSChannelError, match="doesn't exist"):
             mtls.default_client_cert_source()
 
     @pytest.mark.asyncio
@@ -75,45 +39,35 @@ def test_default_client_cert_source_none(self, mock_has_default):
         new_callable=mock.AsyncMock,
     )
     @mock.patch(
-        "google.auth.aio.transport.mtls.has_default_client_cert_source",
-        return_value=True,
+        "google.auth.transport.mtls.has_default_client_cert_source", return_value=True
     )
     async def test_default_client_cert_source_success(
         self, mock_has_default, mock_get_cert_key
     ):
+        """Tests the async callback returned by default_client_cert_source."""
         mock_get_cert_key.return_value = (True, CERT_DATA, KEY_DATA)
 
-        # Note: default_client_cert_source is NOT async, but it returns an async callback
+        # default_client_cert_source is a factory that returns an async callback
         callback = mtls.default_client_cert_source()
         assert callable(callback)
 
         cert, key = await callback()
         assert cert == CERT_DATA
         assert key == KEY_DATA
 
-    @pytest.mark.asyncio
-    @mock.patch(
-        "google.auth.aio.transport.mtls.has_default_client_cert_source",
-        return_value=False,
-    )
-    async def test_default_client_cert_source_not_found(self, mock_has_default):
-        with pytest.raises(exceptions.MutualTLSChannelError, match="doesn't exist"):
-            await mtls.default_client_cert_source()
-
     @pytest.mark.asyncio
     @mock.patch(
         "google.auth.aio.transport.mtls.get_client_cert_and_key",
         new_callable=mock.AsyncMock,
     )
     @mock.patch(
-        "google.auth.aio.transport.mtls.has_default_client_cert_source",
-        return_value=True,
+        "google.auth.transport.mtls.has_default_client_cert_source", return_value=True
     )
     async def test_default_client_cert_source_callback_wraps_exception(
         self, mock_has, mock_get
     ):
+        """Tests that the callback wraps underlying errors into MutualTLSChannelError."""
         mock_get.side_effect = ValueError("Format error")
-
         callback = mtls.default_client_cert_source()
 
         with pytest.raises(exceptions.MutualTLSChannelError) as excinfo:
@@ -123,6 +77,7 @@ async def test_default_client_cert_source_callback_wraps_exception(
     @pytest.mark.asyncio
     @mock.patch("google.auth.transport._mtls_helper._get_workload_cert_and_key")
     async def test_get_client_ssl_credentials_success(self, mock_workload):
+        """Tests successful retrieval of workload credentials via the executor."""
         mock_workload.return_value = (CERT_DATA, KEY_DATA)
 
         success, cert, key, passphrase = await mtls.get_client_ssl_credentials()
@@ -135,6 +90,7 @@ async def test_get_client_ssl_credentials_success(self, mock_workload):
     @pytest.mark.asyncio
     @mock.patch("google.auth.aio.transport.mtls.get_client_ssl_credentials")
     async def test_get_client_cert_and_key_no_credentials_found(self, mock_get_ssl):
+        """Tests behavior when no credentials are found at the default location."""
         mock_get_ssl.return_value = (False, None, None, None)
 
         success, cert, key = await mtls.get_client_cert_and_key(None)
@@ -145,7 +101,7 @@ async def test_get_client_cert_and_key_no_credentials_found(self, mock_get_ssl):
 
     @pytest.mark.asyncio
     async def test_get_client_cert_and_key_callback_async(self):
-        # Test with an actual coroutine/AsyncMock to satisfy the 'await' in your code
+        """Tests that an async callback is correctly awaited."""
         callback = mock.AsyncMock(return_value=(CERT_DATA, KEY_DATA))
 
         success, cert, key = await mtls.get_client_cert_and_key(callback)
@@ -157,51 +113,24 @@ async def test_get_client_cert_and_key_callback_async(self):
 
     @pytest.mark.asyncio
     async def test_get_client_cert_and_key_callback_sync(self):
-        # Test the fallback logic: if it's a sync function, the TypeError is caught
+        """Tests that a sync callback is handled via the TypeError fallback."""
         callback = mock.Mock(return_value=(CERT_DATA, KEY_DATA))
 
         success, cert, key = await mtls.get_client_cert_and_key(callback)
 
         assert success is True
         assert cert == CERT_DATA
-        # In your current implementation, this might still show 2 calls if the
-        # first 'await' attempt triggers a call before failing.
-        # To strictly avoid 2 calls, the implementation would need to check inspect.iscoroutinefunction.
-        assert callback.call_count >= 1
-
-    @pytest.mark.asyncio
-    @mock.patch(
-        "google.auth.aio.transport.mtls.get_client_ssl_credentials",
-        new_callable=mock.AsyncMock,
-    )
-    async def test_get_client_cert_and_key_default(self, mock_get_credentials):
-        mock_get_credentials.return_value = (True, CERT_DATA, KEY_DATA, None)
-
-        success, cert, key = await mtls.get_client_cert_and_key(None)
-
-        assert success is True
-        assert cert == CERT_DATA
-        assert key == KEY_DATA
-        mock_get_credentials.assert_called_once()
+        # Note: In the source, the first 'await' will call the function.
+        # When it fails to await, the exception handler uses the result already obtained.
+        assert callback.call_count == 1
 
     @pytest.mark.asyncio
     @mock.patch("google.auth.transport._mtls_helper._get_workload_cert_and_key")
     async def test_get_client_ssl_credentials_error(self, mock_workload):
+        """Tests exception propagation from the workload helper."""
         mock_workload.side_effect = exceptions.ClientCertError(
             "Failed to read metadata"
         )
 
         with pytest.raises(exceptions.ClientCertError, match="Failed to read metadata"):
             await mtls.get_client_ssl_credentials()
-
-    @pytest.mark.asyncio
-    @mock.patch("google.auth.aio.transport.mtls.get_client_ssl_credentials")
-    async def test_get_client_cert_and_key_exception_propagation(self, mock_get_ssl):
-        mock_get_ssl.side_effect = exceptions.ClientCertError(
-            "Underlying credentials failed"
-        )
-
-        with pytest.raises(
-            exceptions.ClientCertError, match="Underlying credentials failed"
-        ):
-            await mtls.get_client_cert_and_key(client_cert_callback=None)