From f91cb17b0ed6e64443745c2106a7d721a218eb0e Mon Sep 17 00:00:00 2001 From: JunghwanNA <70629228+shaun0927@users.noreply.github.com> Date: Sat, 18 Apr 2026 02:20:00 +0900 Subject: [PATCH] Restore token-based auth helpers for Secret and Parameter Manager Both integration helpers advertised an auth_token constructor path, but the implementation instantiated google.auth.credentials.Credentials directly. That base class is abstract, so callers hit a TypeError before any request could be made. This patch switches both helpers to a concrete OAuth2 Credentials object and adds regression coverage that exercises the real constructor path. The SecretManager helper also now rejects service_account_json + auth_token up front so its behavior matches its own documented contract. Constraint: Keep the change narrow to the validated auth helper regressions Rejected: Leave the existing mocked constructor tests in place | they masked the real runtime failure Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep token-path tests using the real credentials class so abstract-constructor regressions do not reappear Tested: .venv/bin/pytest tests/unittests/integrations/secret_manager/test_secret_client.py tests/unittests/integrations/parameter_manager/test_parameter_client.py -q Not-tested: Full repo test suite; lint tooling was not installed in the local venv --- .../parameter_manager/parameter_client.py | 13 +---- .../secret_manager/secret_client.py | 19 ++++---- .../test_parameter_client.py | 32 +++++-------- .../secret_manager/test_secret_client.py | 48 ++++++++++--------- 4 files changed, 48 insertions(+), 64 deletions(-) diff --git a/src/google/adk/integrations/parameter_manager/parameter_client.py b/src/google/adk/integrations/parameter_manager/parameter_client.py index 2f0f12322e..51ab3a68f4 100644 --- a/src/google/adk/integrations/parameter_manager/parameter_client.py +++ b/src/google/adk/integrations/parameter_manager/parameter_client.py @@ -19,10 +19,9 @@ from typing import Optional from google.api_core.gapic_v1 import client_info -import google.auth from google.auth import default as default_service_credential -import google.auth.transport.requests from google.cloud import parametermanager_v1 +from google.oauth2 import credentials as user_credentials from google.oauth2 import service_account from ... import version @@ -81,15 +80,7 @@ def __init__( except json.JSONDecodeError as e: raise ValueError(f"Invalid service account JSON: {e}") from e elif auth_token: - credentials = google.auth.credentials.Credentials( - token=auth_token, - refresh_token=None, - token_uri=None, - client_id=None, - client_secret=None, - ) - request = google.auth.transport.requests.Request() - credentials.refresh(request) + credentials = user_credentials.Credentials(token=auth_token) else: try: credentials, _ = default_service_credential( diff --git a/src/google/adk/integrations/secret_manager/secret_client.py b/src/google/adk/integrations/secret_manager/secret_client.py index df06743565..4d29f2ed93 100644 --- a/src/google/adk/integrations/secret_manager/secret_client.py +++ b/src/google/adk/integrations/secret_manager/secret_client.py @@ -19,10 +19,9 @@ from typing import Optional from google.api_core.gapic_v1 import client_info -import google.auth from google.auth import default as default_service_credential -import google.auth.transport.requests from google.cloud import secretmanager +from google.oauth2 import credentials as user_credentials from google.oauth2 import service_account from ... import version @@ -65,6 +64,12 @@ def __init__( is not valid JSON. google.auth.exceptions.GoogleAuthError: If authentication fails. """ + if service_account_json and auth_token: + raise ValueError( + "Must provide either 'service_account_json' or 'auth_token', not" + " both." + ) + if service_account_json: try: credentials = service_account.Credentials.from_service_account_info( @@ -73,15 +78,7 @@ def __init__( except json.JSONDecodeError as e: raise ValueError(f"Invalid service account JSON: {e}") from e elif auth_token: - credentials = google.auth.credentials.Credentials( - token=auth_token, - refresh_token=None, - token_uri=None, - client_id=None, - client_secret=None, - ) - request = google.auth.transport.requests.Request() - credentials.refresh(request) + credentials = user_credentials.Credentials(token=auth_token) else: try: credentials, _ = default_service_credential( diff --git a/tests/unittests/integrations/parameter_manager/test_parameter_client.py b/tests/unittests/integrations/parameter_manager/test_parameter_client.py index 1fe07bfd24..ce61fc6bb4 100644 --- a/tests/unittests/integrations/parameter_manager/test_parameter_client.py +++ b/tests/unittests/integrations/parameter_manager/test_parameter_client.py @@ -21,6 +21,7 @@ from google.adk.integrations.parameter_manager.parameter_client import ParameterManagerClient from google.adk.integrations.parameter_manager.parameter_client import USER_AGENT from google.api_core.gapic_v1 import client_info +from google.oauth2.credentials import Credentials import pytest @@ -92,28 +93,19 @@ def test_init_with_service_account_json( @patch("google.cloud.parametermanager_v1.ParameterManagerClient") def test_init_with_auth_token(self, mock_pm_client_class): """Test initialization with auth token.""" - # Setup auth_token = "test-token" - mock_credentials = MagicMock() - with ( - patch("google.auth.credentials.Credentials") as mock_credentials_class, - patch("google.auth.transport.requests.Request") as mock_request, - ): - mock_credentials_class.return_value = mock_credentials - - # Execute - client = ParameterManagerClient(auth_token=auth_token) - - # Verify - mock_credentials.refresh.assert_called_once() - mock_pm_client_class.assert_called_once() - call_kwargs = mock_pm_client_class.call_args.kwargs - assert call_kwargs["credentials"] == mock_credentials - assert call_kwargs["client_options"] is None - assert call_kwargs["client_info"].user_agent == USER_AGENT - assert client._credentials == mock_credentials - assert client._client == mock_pm_client_class.return_value + client = ParameterManagerClient(auth_token=auth_token) + + mock_pm_client_class.assert_called_once() + call_kwargs = mock_pm_client_class.call_args.kwargs + assert isinstance(call_kwargs["credentials"], Credentials) + assert call_kwargs["credentials"].token == auth_token + assert call_kwargs["client_options"] is None + assert call_kwargs["client_info"].user_agent == USER_AGENT + assert isinstance(client._credentials, Credentials) + assert client._credentials.token == auth_token + assert client._client == mock_pm_client_class.return_value @patch("google.cloud.parametermanager_v1.ParameterManagerClient") @patch( diff --git a/tests/unittests/integrations/secret_manager/test_secret_client.py b/tests/unittests/integrations/secret_manager/test_secret_client.py index e463bd95bc..297aa5bfd7 100644 --- a/tests/unittests/integrations/secret_manager/test_secret_client.py +++ b/tests/unittests/integrations/secret_manager/test_secret_client.py @@ -21,6 +21,7 @@ from google.adk.integrations.secret_manager.secret_client import SecretManagerClient from google.adk.integrations.secret_manager.secret_client import USER_AGENT from google.api_core.gapic_v1 import client_info +from google.oauth2.credentials import Credentials import pytest import google @@ -94,30 +95,19 @@ def test_init_with_service_account_json( @patch("google.cloud.secretmanager.SecretManagerServiceClient") def test_init_with_auth_token(self, mock_secret_manager_client): """Test initialization with auth token.""" - # Setup auth_token = "test-token" - mock_credentials = MagicMock() - # Mock the entire credentials creation process - with ( - patch("google.auth.credentials.Credentials") as mock_credentials_class, - patch("google.auth.transport.requests.Request") as mock_request, - ): - # Configure the mock to return our mock_credentials when instantiated - mock_credentials_class.return_value = mock_credentials - - # Execute - client = SecretManagerClient(auth_token=auth_token) - - # Verify - mock_credentials.refresh.assert_called_once() - mock_secret_manager_client.assert_called_once() - call_kwargs = mock_secret_manager_client.call_args.kwargs - assert call_kwargs["credentials"] == mock_credentials - assert call_kwargs["client_options"] is None - assert call_kwargs["client_info"].user_agent == USER_AGENT - assert client._credentials == mock_credentials - assert client._client == mock_secret_manager_client.return_value + client = SecretManagerClient(auth_token=auth_token) + + mock_secret_manager_client.assert_called_once() + call_kwargs = mock_secret_manager_client.call_args.kwargs + assert isinstance(call_kwargs["credentials"], Credentials) + assert call_kwargs["credentials"].token == auth_token + assert call_kwargs["client_options"] is None + assert call_kwargs["client_info"].user_agent == USER_AGENT + assert isinstance(client._credentials, Credentials) + assert client._credentials.token == auth_token + assert client._client == mock_secret_manager_client.return_value @patch("google.cloud.secretmanager.SecretManagerServiceClient") @patch( @@ -170,6 +160,20 @@ def test_init_with_invalid_service_account_json(self): with pytest.raises(ValueError, match="Invalid service account JSON"): SecretManagerClient(service_account_json="invalid-json") + def test_init_with_both_service_account_json_and_auth_token(self): + """Test initialization rejects conflicting credential inputs.""" + with pytest.raises( + ValueError, + match=( + "Must provide either 'service_account_json' or 'auth_token', not" + " both." + ), + ): + SecretManagerClient( + service_account_json=json.dumps({"type": "service_account"}), + auth_token="test-token", + ) + @patch("google.cloud.secretmanager.SecretManagerServiceClient") @patch( "google.adk.integrations.secret_manager.secret_client.default_service_credential"