From 9e9c039f2488abf56efa9f8e0837410f2ecb191a Mon Sep 17 00:00:00 2001 From: samecho Date: Wed, 14 Oct 2026 20:31:32 -0400 Subject: [PATCH 1/2] fix(openapi): exchange client credentials without user consent Use the existing async OAuth exchanger for client-credentials-only schemes before the interactive authorization path. Reuse unexpired session tokens and obtain a new client token when they expire. Keep client secrets out of the session token cache and preserve authorization-code consent. Fixes #3753 --- .../openapi_spec_parser/tool_auth_handler.py | 61 +++++++ .../test_tool_auth_handler.py | 172 ++++++++++++++++++ 2 files changed, 233 insertions(+) diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py index a6b489787d2..790a79c16db 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py @@ -23,6 +23,7 @@ from typing import NamedTuple from typing import Optional +from fastapi.openapi.models import OAuth2 from pydantic import BaseModel from ....auth.auth_credential import AuthCredential @@ -31,6 +32,8 @@ from ....auth.auth_schemes import AuthSchemeType from ....auth.auth_tool import _stable_model_digest from ....auth.auth_tool import AuthConfig +from ....auth.exchanger.base_credential_exchanger import CredentialExchangeError +from ....auth.exchanger.oauth2_credential_exchanger import OAuth2CredentialExchanger from ....auth.refresher.oauth2_credential_refresher import OAuth2CredentialRefresher from ...tool_context import ToolContext from ..auth.credential_exchangers.auto_auth_credential_exchanger import AutoAuthCredentialExchanger @@ -591,6 +594,9 @@ async def handle_unauthorized_error(self) -> UnauthorizedRecoveryState: requested but this credential has spent its budget; "failed" if no recovery was possible. """ + if self._is_client_credentials_only(): + return "failed" + # Budget bookkeeping needs the store to name the credential and the context # to read and write the counts. Without both there is nowhere to record an # attempt, so the budgets are skipped rather than silently reset to zero on @@ -727,6 +733,54 @@ def _external_exchange_required(self, credential) -> bool: and not credential.oauth2.access_token ) + def _is_client_credentials_only(self) -> bool: + if not isinstance(self.auth_scheme, OAuth2): + return False + flows = self.auth_scheme.flows + return bool( + flows.clientCredentials + and not (flows.authorizationCode or flows.implicit or flows.password) + ) + + async def _prepare_client_credentials( + self, raw_credential: AuthCredential + ) -> AuthPreparationResult: + credential = ( + self.credential_store.get_credential( + self.auth_scheme, self.auth_credential + ) + if self.credential_store + else None + ) or raw_credential.model_copy(deep=True) + if not ( + credential + and credential.oauth2 + and credential.oauth2.access_token + and not await OAuth2CredentialRefresher().is_refresh_needed(credential) + ): + credential = raw_credential.model_copy(deep=True) + assert credential.oauth2 is not None + credential.oauth2.access_token = None + credential.oauth2.expires_at = None + credential.oauth2.expires_in = None + result = await OAuth2CredentialExchanger().exchange( + credential, self.auth_scheme + ) + credential = result.credential + if not (credential.oauth2 and credential.oauth2.access_token): + raise CredentialExchangeError( + "Client credentials exchange did not return an access token." + ) + # The token cache is session state, which must not hold the client secret. + credential.oauth2.client_secret = None + self._store_credential(credential) + + return AuthPreparationResult( + state="done", + auth_scheme=self.auth_scheme, + auth_credential=self._exchange_credential(credential), + ) + async def prepare_auth_credentials( self, ) -> AuthPreparationResult: @@ -736,6 +790,13 @@ async def prepare_auth_credentials( if not self.auth_scheme: return AuthPreparationResult(state="done") + if ( + self._is_client_credentials_only() + and self.auth_credential + and self.auth_credential.oauth2 + ): + return await self._prepare_client_credentials(self.auth_credential) + # Check for existing credential. existing_credential = await self._get_existing_credential() diff --git a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py index 59a32a3450c..79a48e941ab 100644 --- a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py +++ b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py @@ -14,13 +14,20 @@ import asyncio from contextlib import asynccontextmanager +import json +import threading import time from typing import Optional from unittest.mock import AsyncMock from unittest.mock import MagicMock from unittest.mock import patch +from urllib.parse import parse_qs from authlib.common.errors import AuthlibBaseError +from fastapi.openapi.models import OAuth2 +from fastapi.openapi.models import OAuthFlowAuthorizationCode +from fastapi.openapi.models import OAuthFlowClientCredentials +from fastapi.openapi.models import OAuthFlows from google.adk.agents.invocation_context import InvocationContext from google.adk.agents.llm_agent import LlmAgent from google.adk.auth.auth_credential import AuthCredential @@ -29,6 +36,7 @@ from google.adk.auth.auth_credential import HttpCredentials from google.adk.auth.auth_credential import OAuth2Auth from google.adk.auth.auth_schemes import AuthScheme +from google.adk.auth.exchanger.base_credential_exchanger import CredentialExchangeError from google.adk.auth.refresher.oauth2_credential_refresher import OAuth2CredentialRefresher from google.adk.events.event import Event from google.adk.sessions.in_memory_session_service import InMemorySessionService @@ -44,6 +52,7 @@ from google.adk.tools.tool_context import ToolContext from pydantic import BaseModel import pytest +from requests import Response # Helper function to create a mock ToolContext @@ -59,6 +68,169 @@ def create_mock_tool_context(): ) +@pytest.fixture +def client_credentials_case(): + context = create_mock_tool_context() + scheme = OAuth2( + flows=OAuthFlows( + clientCredentials=OAuthFlowClientCredentials( + tokenUrl='https://token.example.test/token', + scopes={'read': 'Read'}, + ) + ) + ) + credential = AuthCredential( + auth_type=AuthCredentialTypes.OAUTH2, + oauth2=OAuth2Auth(client_id='client', client_secret='test-client-secret'), + ) + return ( + ToolAuthHandler.from_tool_context(context, scheme, credential), + context, + credential, + ) + + +@pytest.fixture +def token_requests(monkeypatch): + calls = [] + + def send(session, request, **kwargs): + calls.append((request, threading.get_ident())) + response = Response() + response.status_code = 200 + response.headers['Content-Type'] = 'application/json' + response._content = json.dumps({ + 'access_token': f'token-{len(calls)}', + 'token_type': 'Bearer', + 'expires_in': 3600, + }).encode() + response.request = request + return response + + monkeypatch.setattr('requests.sessions.Session.send', send) + return calls + + +async def test_client_credentials_exchange_without_user_authorization( + client_credentials_case, token_requests +): + """Client credentials get a bearer token without asking for user consent.""" + handler, context, configured = client_credentials_case + + result = await handler.prepare_auth_credentials() + + assert result.state == 'done' + assert result.auth_credential.http.credentials.token == 'token-1' + assert not context.actions.requested_auth_configs + request, thread_id = token_requests[0] + assert request.method == 'POST' + assert parse_qs(request.body)['grant_type'] == ['client_credentials'] + assert thread_id != threading.get_ident() + assert configured.oauth2.access_token is None + assert configured.oauth2.client_secret == 'test-client-secret' + assert 'test-client-secret' not in json.dumps(context.state.to_dict()) + + +async def test_client_credentials_reuse_cached_token( + client_credentials_case, token_requests +): + """A second preparation reuses the session's unexpired token.""" + handler, context, _ = client_credentials_case + first = await handler.prepare_auth_credentials() + second = await handler.prepare_auth_credentials() + + assert first.auth_credential == second.auth_credential + assert len(token_requests) == 1 + assert not context.actions.requested_auth_configs + + +async def test_expired_client_credentials_token_is_exchanged_again( + client_credentials_case, token_requests, monkeypatch +): + """Expired client tokens are renewed with the client grant, not a login URL.""" + handler, context, _ = client_credentials_case + await handler.prepare_auth_credentials() + later = time.time() + 7200 + monkeypatch.setattr(time, 'time', lambda: later) + + result = await handler.prepare_auth_credentials() + + assert result.auth_credential.http.credentials.token == 'token-2' + assert len(token_requests) == 2 + assert all( + parse_qs(r.body)['grant_type'] == ['client_credentials'] + for r, _ in token_requests + ) + assert not context.actions.requested_auth_configs + + +@pytest.mark.parametrize('expired', [False, True]) +async def test_configured_client_token_respects_expiry( + client_credentials_case, token_requests, expired +): + """A configured token is used while valid and replaced after expiry.""" + handler, _, _ = client_credentials_case + handler.auth_credential.oauth2.access_token = 'configured' + handler.auth_credential.oauth2.expires_at = ( + 1 if expired else time.time() + 3600 + ) + + result = await handler.prepare_auth_credentials() + + assert result.auth_credential.http.credentials.token == ( + 'token-1' if expired else 'configured' + ) + assert len(token_requests) == (1 if expired else 0) + + +async def test_failed_client_exchange_does_not_request_user_consent( + client_credentials_case, monkeypatch +): + """A failed server exchange is surfaced without falling back to a browser.""" + handler, context, _ = client_credentials_case + + def unavailable(*args, **kwargs): + raise ConnectionError('Token endpoint unavailable') + + monkeypatch.setattr('requests.sessions.Session.send', unavailable) + with pytest.raises( + CredentialExchangeError, match='did not return an access token' + ): + await handler.prepare_auth_credentials() + + assert not context.actions.requested_auth_configs + assert not context.state.to_dict() + + +async def test_client_credentials_401_does_not_request_user_consent( + client_credentials_case, +): + """A rejected client token cannot be repaired with interactive consent.""" + handler, context, _ = client_credentials_case + + assert await handler.handle_unauthorized_error() == 'failed' + assert not context.actions.requested_auth_configs + + +@pytest.mark.parametrize('include_client_grant', [False, True]) +async def test_authorization_code_flow_still_requests_consent( + client_credentials_case, token_requests, include_client_grant +): + """Authorization-code schemes keep their interactive flow, including mixed schemes.""" + handler, context, _ = client_credentials_case + handler.auth_scheme.flows.authorizationCode = OAuthFlowAuthorizationCode( + authorizationUrl='https://auth.example.test/authorize', + tokenUrl='https://token.example.test/token', + scopes={'read': 'Read'}, + ) + if not include_client_grant: + handler.auth_scheme.flows.clientCredentials = None + + assert (await handler.prepare_auth_credentials()).state == 'pending' + assert context.actions.requested_auth_configs + assert not token_requests + + # Test cases for OpenID Connect class MockOpenIdConnectCredentialExchanger(OAuth2CredentialExchanger): From 8d9f57d2af8641da78d5b1689739676f35da3f69 Mon Sep 17 00:00:00 2001 From: samecho Date: Wed, 14 Oct 2026 20:39:51 -0400 Subject: [PATCH 2/2] fix(openapi): discard rejected client tokens before later calls --- .../openapi_tool/openapi_spec_parser/tool_auth_handler.py | 1 + .../openapi_spec_parser/test_tool_auth_handler.py | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py b/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py index 790a79c16db..b8c0a20c4b1 100644 --- a/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py +++ b/src/google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py @@ -595,6 +595,7 @@ async def handle_unauthorized_error(self) -> UnauthorizedRecoveryState: recovery was possible. """ if self._is_client_credentials_only(): + self._evict_credentials() return "failed" # Budget bookkeeping needs the store to name the credential and the context diff --git a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py index 79a48e941ab..9b8f0a252e3 100644 --- a/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py +++ b/tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py @@ -203,12 +203,15 @@ def unavailable(*args, **kwargs): async def test_client_credentials_401_does_not_request_user_consent( - client_credentials_case, + client_credentials_case, token_requests ): - """A rejected client token cannot be repaired with interactive consent.""" + """A rejected token is discarded so a later call can obtain a new one.""" handler, context, _ = client_credentials_case + await handler.prepare_auth_credentials() assert await handler.handle_unauthorized_error() == 'failed' + result = await handler.prepare_auth_credentials() + assert result.auth_credential.http.credentials.token == 'token-2' assert not context.actions.requested_auth_configs