ffa1843951
Merge https://github.com/google/adk-python/pull/5692
### Link to Issue or Description of Change
**1. Link to an existing issue (if applicable):**
- Closes: #5691
This change adds `auth_credential.oauth2.redirect_uri = None` to the OAuth2 strip block at the three call sites where credential hashing happens. `redirect_uri` is deployment configuration (which callback URL the auth server should redirect to), not part of the credential identity, so it should be excluded from the hash just as access_token, refresh_token, expires_at, and the other transient OAuth2 fields already are. Without this change, a credential minted under one deployment URL cannot be retrieved when the deployment moves to another.
### Testing Plan
**Unit Tests:**
- [x] I have added or updated unit tests for my change.
- [x] All unit tests pass locally.
Three new tests, one per affected method. Each constructs two `AuthCredential` instances that differ only in `redirect_uri` and asserts the computed key is identical:
- `tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py::test_credential_key_is_stable_across_redirect_uri`
- `tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py::test_legacy_credential_key_is_stable_across_redirect_uri`
- `tests/unittests/auth/test_auth_config.py::test_credential_key_is_stable_across_redirect_uri`
```
$ pytest tests/unittests/tools/openapi_tool/openapi_spec_parser/test_tool_auth_handler.py tests/unittests/auth/test_auth_config.py
======================== 14 passed, 8 warnings in 2.90s ========================
$ pytest tests/unittests/
================ 5740 passed, 2340 warnings in 86.64s (0:01:26) ================
```
**Manual End-to-End (E2E) Tests:**
A self-contained Runner-based reproduction is at https://github.com/doughayden/adk-issue-examples/tree/main/05-redirect_uri_in_credential_hash. The agent definition (`agent.py`) wires up an `OpenAPIToolset` against a local OAuth2 test server, configured with one redirect_uri value. `main.py` constructs an `InMemoryRunner`, seeds a real (non-expired) OAuth2 credential into session state under a different redirect_uri's hash, and runs the agent. The `--apply-fix` flag monkey-patches the proposed fix to demonstrate the resolution end-to-end.
Without the fix:
```
🌤️ WeatherAssistant Agent — redirect_uri-in-hash repro
============================================================
Proposed fix applied: False
STORED_REDIRECT_URI: http://localhost:8080/callback
CURRENT_REDIRECT_URI: http://localhost:8081/callback
Hash keys produced by ToolContextCredentialStore.get_credential_key:
STORED → oauth2_55f666541ad22e39_oauth2_8ba0457897522d9d_existing_exchanged_credential
CURRENT → oauth2_55f666541ad22e39_oauth2_ae16199243c358df_existing_exchanged_credential
❌ Keys differ — credentials minted under STORED are not retrievable.
👤 User: What's the weather in San Francisco?
🌤️ Weather Assistant event stream:
[function_call] get_weather by WeatherAssistant
[auth_event] adk_request_credential by WeatherAssistant
[function_response] get_weather by WeatherAssistant
[text] WeatherAssistant: 'It seems I need your authorization to access weather data. Could you please g...'
Event counts:
function_calls: 1
auth_events: 1
function_responses: 1
text_events: 1
✅ Bug reproduced: agent emitted 1 adk_request_credential event(s) despite a valid seeded credential being present in state.
```
With the fix:
```
🌤️ WeatherAssistant Agent — redirect_uri-in-hash repro
============================================================
Proposed fix applied: True
STORED_REDIRECT_URI: http://localhost:8080/callback
CURRENT_REDIRECT_URI: http://localhost:8081/callback
Hash keys produced by ToolContextCredentialStore.get_credential_key:
STORED → oauth2_55f666541ad22e39_oauth2_c2ad46dffd26cd87_existing_exchanged_credential
CURRENT → oauth2_55f666541ad22e39_oauth2_c2ad46dffd26cd87_existing_exchanged_credential
✅ Keys match — fix is taking effect at the hash level.
👤 User: What's the weather in San Francisco?
🌤️ Weather Assistant event stream:
[function_call] get_weather by WeatherAssistant
[function_response] get_weather by WeatherAssistant
[text] WeatherAssistant: 'The weather in San Francisco is Clear with a temperature of 30 degrees Celsiu...'
Event counts:
function_calls: 1
auth_events: 0
function_responses: 1
text_events: 1
✅ Fix verified: tool call succeeded against the seeded credential without an adk_request_credential prompt.
```
### Checklist
- [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] New and existing unit tests pass locally with my changes.
- [x] I have manually tested my changes end-to-end.
- [ ] Any dependent changes have been merged and published in downstream modules.
### Additional context
**Scope:**
The same strip block exists at three call sites and has the same gap at all three. This PR patches all three. Patching only the tool-level pair (`tool_auth_handler.py`) and leaving the framework-level path (`auth_tool.py:AuthConfig.get_credential_key`) would leave the bug reachable for any consumer that does not work around #5327 with `get_auth_config = lambda: None`. Patching only `AuthConfig.get_credential_key` and leaving the tool-level pair would leave the bug reachable on the standard tool-level credential lookup path.
**Upgrade note:**
The credential_key shape changes with this fix: `redirect_uri` is no longer included in the hash. OAuth credentials cached in existing session state under the pre-fix key shape become unreachable under the new key. Users should expect a one-time re-auth prompt on the first run after upgrading. Subsequent runs use the new key normally.
**Related:**
- #5327 (preemptive toolset auth)
- #5328 (refresh request scope)
- #5329 (refreshed credential persistence, fixed in 218ea76e)
- #5637 (tool-level auth termination)
Co-authored-by: George Weale <gweale@google.com>
COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/5692 from doughayden:fix/credential-key-strip-redirect-uri 3be2a85c75ebca865467499de8bf29af3e735c31
PiperOrigin-RevId: 941217122
217 lines
6.9 KiB
Python
217 lines
6.9 KiB
Python
# Copyright 2026 Google LLC
|
|
#
|
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
|
# you may not use this file except in compliance with the License.
|
|
# You may obtain a copy of the License at
|
|
#
|
|
# http://www.apache.org/licenses/LICENSE-2.0
|
|
#
|
|
# Unless required by applicable law or agreed to in writing, software
|
|
# distributed under the License is distributed on an "AS IS" BASIS,
|
|
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
# See the License for the specific language governing permissions and
|
|
# limitations under the License.
|
|
|
|
import os
|
|
from pathlib import Path
|
|
import subprocess
|
|
import sys
|
|
|
|
from fastapi.openapi.models import OAuth2
|
|
from fastapi.openapi.models import OAuthFlowAuthorizationCode
|
|
from fastapi.openapi.models import OAuthFlows
|
|
from google.adk.auth.auth_credential import AuthCredential
|
|
from google.adk.auth.auth_credential import AuthCredentialTypes
|
|
from google.adk.auth.auth_credential import OAuth2Auth
|
|
from google.adk.auth.auth_schemes import CustomAuthScheme
|
|
from google.adk.auth.auth_tool import AuthConfig
|
|
import pytest
|
|
|
|
|
|
class TestAuthConfig:
|
|
"""Tests for the AuthConfig method."""
|
|
|
|
|
|
@pytest.fixture
|
|
def oauth2_auth_scheme():
|
|
"""Create an OAuth2 auth scheme for testing."""
|
|
# Create the OAuthFlows object first
|
|
flows = OAuthFlows(
|
|
authorizationCode=OAuthFlowAuthorizationCode(
|
|
authorizationUrl="https://example.com/oauth2/authorize",
|
|
tokenUrl="https://example.com/oauth2/token",
|
|
scopes={"read": "Read access", "write": "Write access"},
|
|
)
|
|
)
|
|
|
|
# Then create the OAuth2 object with the flows
|
|
return OAuth2(flows=flows)
|
|
|
|
|
|
@pytest.fixture
|
|
def oauth2_credentials():
|
|
"""Create OAuth2 credentials for testing."""
|
|
return AuthCredential(
|
|
auth_type=AuthCredentialTypes.OAUTH2,
|
|
oauth2=OAuth2Auth(
|
|
client_id="mock_client_id",
|
|
client_secret="mock_client_secret",
|
|
redirect_uri="https://example.com/callback",
|
|
),
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def auth_config(oauth2_auth_scheme, oauth2_credentials):
|
|
"""Create an AuthConfig for testing."""
|
|
# Create a copy of the credentials for the exchanged_auth_credential
|
|
exchanged_credential = oauth2_credentials.model_copy(deep=True)
|
|
|
|
return AuthConfig(
|
|
auth_scheme=oauth2_auth_scheme,
|
|
raw_auth_credential=oauth2_credentials,
|
|
exchanged_auth_credential=exchanged_credential,
|
|
)
|
|
|
|
|
|
@pytest.fixture
|
|
def auth_config_with_key(oauth2_auth_scheme, oauth2_credentials):
|
|
"""Create an AuthConfig for testing."""
|
|
|
|
return AuthConfig(
|
|
auth_scheme=oauth2_auth_scheme,
|
|
raw_auth_credential=oauth2_credentials,
|
|
credential_key="test_key",
|
|
)
|
|
|
|
|
|
def test_custom_credential_key(auth_config_with_key):
|
|
"""Test using custom credential key."""
|
|
|
|
key = auth_config_with_key.credential_key
|
|
assert key == "test_key"
|
|
|
|
|
|
def test_credential_key(auth_config):
|
|
"""Test generating a unique credential key."""
|
|
|
|
key = auth_config.credential_key
|
|
assert key.startswith("adk_oauth2_")
|
|
assert "_oauth2_" in key
|
|
|
|
|
|
def test_get_credential_key_with_extras(auth_config):
|
|
"""Test generating a key when model_extra exists."""
|
|
# Add model_extra to test cleanup
|
|
|
|
original_key = auth_config.credential_key
|
|
key = auth_config.credential_key
|
|
|
|
auth_config.auth_scheme.model_extra["extra_field"] = "value"
|
|
auth_config.raw_auth_credential.model_extra["extra_field"] = "value"
|
|
|
|
assert original_key == key
|
|
assert "extra_field" in auth_config.auth_scheme.model_extra
|
|
assert "extra_field" in auth_config.raw_auth_credential.model_extra
|
|
|
|
|
|
def test_credential_key_is_stable_across_python_hash_seed():
|
|
"""Test AuthConfig key generation does not depend on PYTHONHASHSEED."""
|
|
repo_root = Path(__file__).resolve().parents[3]
|
|
pythonpath = str(repo_root / "src")
|
|
code = "\n".join([
|
|
"from fastapi.openapi.models import OAuth2",
|
|
"from fastapi.openapi.models import OAuthFlowAuthorizationCode",
|
|
"from fastapi.openapi.models import OAuthFlows",
|
|
"from google.adk.auth.auth_credential import AuthCredential",
|
|
"from google.adk.auth.auth_credential import AuthCredentialTypes",
|
|
"from google.adk.auth.auth_credential import OAuth2Auth",
|
|
"from google.adk.auth.auth_tool import AuthConfig",
|
|
"",
|
|
"auth_scheme = OAuth2(",
|
|
" flows=OAuthFlows(",
|
|
" authorizationCode=OAuthFlowAuthorizationCode(",
|
|
" authorizationUrl='https://example.com/oauth2/authorize',",
|
|
" tokenUrl='https://example.com/oauth2/token',",
|
|
" scopes={'read': 'Read access'},",
|
|
" )",
|
|
" )",
|
|
")",
|
|
"auth_cred = AuthCredential(",
|
|
" auth_type=AuthCredentialTypes.OAUTH2,",
|
|
" oauth2=OAuth2Auth(",
|
|
" client_id='mock_client_id',",
|
|
" client_secret='mock_client_secret',",
|
|
" ),",
|
|
")",
|
|
"print(AuthConfig(",
|
|
" auth_scheme=auth_scheme,",
|
|
" raw_auth_credential=auth_cred,",
|
|
").credential_key)",
|
|
])
|
|
|
|
def _run_with_seed(seed: str) -> str:
|
|
env = os.environ.copy()
|
|
env["PYTHONHASHSEED"] = seed
|
|
env["PYTHONPATH"] = os.pathsep.join(
|
|
[pythonpath, env.get("PYTHONPATH", "")]
|
|
).strip(os.pathsep)
|
|
return subprocess.check_output(
|
|
[sys.executable, "-c", code],
|
|
env=env,
|
|
text=True,
|
|
).strip()
|
|
|
|
assert _run_with_seed("0") == _run_with_seed("1")
|
|
|
|
|
|
def test_credential_key_with_custom_auth_scheme():
|
|
"""Test generating a credential key when the auth scheme is a CustomAuthScheme (type_ is a string)."""
|
|
custom_scheme = CustomAuthScheme.model_validate({"type": "mock_custom_type"})
|
|
|
|
custom_config = AuthConfig(
|
|
auth_scheme=custom_scheme,
|
|
)
|
|
|
|
key = custom_config.credential_key
|
|
assert key.startswith("adk_mock_custom_type_")
|
|
assert len(key) > len("adk_mock_custom_type_")
|
|
|
|
|
|
def test_credential_key_is_stable_across_redirect_uri(oauth2_auth_scheme):
|
|
"""AuthConfig.credential_key should be invariant under redirect_uri changes.
|
|
|
|
redirect_uri is deployment configuration (which callback URL the auth
|
|
server should redirect to), not part of the credential identity. Two
|
|
AuthConfig instances built from credentials that share the same client_id,
|
|
client_secret, and scopes but differ only in redirect_uri should produce
|
|
the same credential_key.
|
|
"""
|
|
credential_local = AuthCredential(
|
|
auth_type=AuthCredentialTypes.OAUTH2,
|
|
oauth2=OAuth2Auth(
|
|
client_id="client",
|
|
client_secret="secret",
|
|
redirect_uri="http://localhost:8001/oauth2callback",
|
|
),
|
|
)
|
|
credential_deployed = AuthCredential(
|
|
auth_type=AuthCredentialTypes.OAUTH2,
|
|
oauth2=OAuth2Auth(
|
|
client_id="client",
|
|
client_secret="secret",
|
|
redirect_uri="https://deployed.example.com/oauth2callback",
|
|
),
|
|
)
|
|
|
|
config_local = AuthConfig(
|
|
auth_scheme=oauth2_auth_scheme,
|
|
raw_auth_credential=credential_local,
|
|
)
|
|
config_deployed = AuthConfig(
|
|
auth_scheme=oauth2_auth_scheme,
|
|
raw_auth_credential=credential_deployed,
|
|
)
|
|
|
|
assert config_local.credential_key == config_deployed.credential_key
|