Skip to content

Commit 0d01bf7

Browse files
caohy1988claude
andcommitted
fix(auth): migrate credential storage to secret: scope (Phase 2)
Migrate existing credential writers to use the `secret:` prefix so that OAuth tokens and credentials are never persisted to session storage backends. - Change BIGQUERY_TOKEN_CACHE_KEY to "secret:bigquery_token_cache" - Update SessionStateCredentialService.save_credential and load_credential to prefix credential_key with State.SECRET_PREFIX - Backward-compatible migration: load paths try the secret-prefixed key first, then fall back to the legacy unprefixed key. On fallback hit, the value is copied to the secret: key and the legacy key is set to None so it is cleared from persistent storage on the next state delta flush. - Use key-presence check (not truthiness) so explicit None in the secret-scoped key is respected and does not revive stale legacy credentials. Depends on #5132 (Phase 1: secret: scope infrastructure) Closes #5112 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ce8d2a3 commit 0d01bf7

File tree

4 files changed

+122
-22
lines changed

4 files changed

+122
-22
lines changed

src/google/adk/auth/credential_service/session_state_credential_service.py

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from typing_extensions import override
2020

2121
from ...agents.callback_context import CallbackContext
22+
from ...sessions.state import State
2223
from ...utils.feature_decorator import experimental
2324
from ..auth_credential import AuthCredential
2425
from ..auth_tool import AuthConfig
@@ -54,7 +55,20 @@ async def load_credential(
5455
Optional[AuthCredential]: the credential saved in the store.
5556
5657
"""
57-
return callback_context.state.get(auth_config.credential_key)
58+
secret_key = State.SECRET_PREFIX + auth_config.credential_key
59+
# Use `in` (not truthiness) so an explicit None is respected.
60+
if secret_key in callback_context.state:
61+
return callback_context.state[secret_key]
62+
# Fall back to legacy unprefixed key, then migrate: copy into
63+
# secret: scope and clear the legacy key so it is removed from
64+
# persistent storage on the next state delta flush.
65+
legacy_key = auth_config.credential_key
66+
if legacy_key in callback_context.state:
67+
value = callback_context.state[legacy_key]
68+
callback_context.state[secret_key] = value
69+
callback_context.state[legacy_key] = None
70+
return value
71+
return None
5872

5973
@override
6074
async def save_credential(
@@ -78,6 +92,6 @@ async def save_credential(
7892
None
7993
"""
8094

81-
callback_context.state[auth_config.credential_key] = (
95+
callback_context.state[State.SECRET_PREFIX + auth_config.credential_key] = (
8296
auth_config.exchanged_auth_credential
8397
)

src/google/adk/tools/_google_credentials.py

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -171,11 +171,17 @@ async def get_valid_credentials(
171171
f" {self.credentials_config.external_access_token_key}."
172172
)
173173
# First, try to get credentials from the tool context
174-
creds_json = (
175-
tool_context.state.get(self.credentials_config._token_cache_key, None)
176-
if self.credentials_config._token_cache_key
177-
else None
178-
)
174+
cache_key = self.credentials_config._token_cache_key
175+
creds_json = tool_context.state.get(cache_key, None) if cache_key else None
176+
# Fall back to legacy unprefixed key, then migrate: copy into
177+
# secret: scope and clear the legacy key so it is removed from
178+
# persistent storage on the next state delta flush.
179+
if creds_json is None and cache_key and cache_key.startswith("secret:"):
180+
legacy_key = cache_key[len("secret:") :]
181+
creds_json = tool_context.state.get(legacy_key, None)
182+
if creds_json is not None:
183+
tool_context.state[cache_key] = creds_json
184+
tool_context.state[legacy_key] = None
179185
creds = (
180186
google.oauth2.credentials.Credentials.from_authorized_user_info(
181187
json.loads(creds_json), self.credentials_config.scopes

src/google/adk/tools/bigquery/bigquery_credentials.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
from ...features import FeatureName
1919
from .._google_credentials import BaseGoogleCredentialsConfig
2020

21-
BIGQUERY_TOKEN_CACHE_KEY = "bigquery_token_cache"
21+
BIGQUERY_TOKEN_CACHE_KEY = "secret:bigquery_token_cache"
2222
BIGQUERY_SCOPES = [
2323
"https://www.googleapis.com/auth/bigquery",
2424
"https://www.googleapis.com/auth/dataplex.read-write",

tests/unittests/auth/credential_service/test_session_state_credential_service.py

Lines changed: 94 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from google.adk.auth.auth_credential import OAuth2Auth
2424
from google.adk.auth.auth_tool import AuthConfig
2525
from google.adk.auth.credential_service.session_state_credential_service import SessionStateCredentialService
26+
from google.adk.sessions.state import State
2627
import pytest
2728

2829

@@ -265,10 +266,11 @@ async def test_state_persistence_across_operations(
265266
# Save credential
266267
await credential_service.save_credential(auth_config, callback_context)
267268

268-
# Verify state contains the credential
269-
assert auth_config.credential_key in callback_context.state
269+
# Verify state contains the credential under secret: prefix
270+
secret_key = State.SECRET_PREFIX + auth_config.credential_key
271+
assert secret_key in callback_context.state
270272
assert (
271-
callback_context.state[auth_config.credential_key]
273+
callback_context.state[secret_key]
272274
== auth_config.exchanged_auth_credential
273275
)
274276

@@ -279,9 +281,9 @@ async def test_state_persistence_across_operations(
279281
assert result is not None
280282

281283
# Verify state still contains the credential
282-
assert auth_config.credential_key in callback_context.state
284+
assert secret_key in callback_context.state
283285
assert (
284-
callback_context.state[auth_config.credential_key]
286+
callback_context.state[secret_key]
285287
== auth_config.exchanged_auth_credential
286288
)
287289

@@ -300,7 +302,7 @@ async def test_state_persistence_across_operations(
300302
await credential_service.save_credential(auth_config, callback_context)
301303

302304
# Verify state was updated
303-
assert callback_context.state[auth_config.credential_key] == new_credential
305+
assert callback_context.state[secret_key] == new_credential
304306

305307
@pytest.mark.asyncio
306308
async def test_credential_key_uniqueness(
@@ -344,13 +346,12 @@ async def test_credential_key_uniqueness(
344346
await credential_service.save_credential(auth_config1, callback_context)
345347
await credential_service.save_credential(auth_config2, callback_context)
346348

347-
# Verify both exist in state with different keys
348-
assert "unique_key_1" in callback_context.state
349-
assert "unique_key_2" in callback_context.state
350-
assert (
351-
callback_context.state["unique_key_1"]
352-
!= callback_context.state["unique_key_2"]
353-
)
349+
# Verify both exist in state with secret-prefixed keys
350+
sk1 = State.SECRET_PREFIX + "unique_key_1"
351+
sk2 = State.SECRET_PREFIX + "unique_key_2"
352+
assert sk1 in callback_context.state
353+
assert sk2 in callback_context.state
354+
assert callback_context.state[sk1] != callback_context.state[sk2]
354355

355356
# Load and verify both credentials
356357
result1 = await credential_service.load_credential(
@@ -379,10 +380,89 @@ async def test_direct_state_access(
379380
redirect_uri="https://direct.com/callback",
380381
),
381382
)
382-
callback_context.state[auth_config.credential_key] = test_credential
383+
callback_context.state[State.SECRET_PREFIX + auth_config.credential_key] = (
384+
test_credential
385+
)
383386

384387
# Load using the service
385388
result = await credential_service.load_credential(
386389
auth_config, callback_context
387390
)
388391
assert result == test_credential
392+
393+
@pytest.mark.asyncio
394+
async def test_load_falls_back_to_legacy_unprefixed_key(
395+
self, credential_service, auth_config, callback_context
396+
):
397+
"""Credentials stored under the old unprefixed key are still found."""
398+
legacy_cred = AuthCredential(
399+
auth_type=AuthCredentialTypes.OAUTH2,
400+
oauth2=OAuth2Auth(
401+
client_id="legacy_client",
402+
client_secret="legacy_secret",
403+
),
404+
)
405+
# Simulate a session persisted before the secret: migration
406+
callback_context.state[auth_config.credential_key] = legacy_cred
407+
408+
result = await credential_service.load_credential(
409+
auth_config, callback_context
410+
)
411+
assert result is not None
412+
assert result.oauth2.client_id == "legacy_client"
413+
# Legacy key should be migrated: secret key populated, legacy cleared
414+
secret_key = State.SECRET_PREFIX + auth_config.credential_key
415+
assert secret_key in callback_context.state
416+
assert callback_context.state[secret_key] == legacy_cred
417+
assert callback_context.state[auth_config.credential_key] is None
418+
419+
@pytest.mark.asyncio
420+
async def test_secret_key_takes_precedence_over_legacy(
421+
self, credential_service, auth_config, callback_context
422+
):
423+
"""When both keys exist, the secret-prefixed key wins."""
424+
old_cred = AuthCredential(
425+
auth_type=AuthCredentialTypes.OAUTH2,
426+
oauth2=OAuth2Auth(
427+
client_id="old_client",
428+
client_secret="old_secret",
429+
),
430+
)
431+
new_cred = AuthCredential(
432+
auth_type=AuthCredentialTypes.OAUTH2,
433+
oauth2=OAuth2Auth(
434+
client_id="new_client",
435+
client_secret="new_secret",
436+
),
437+
)
438+
callback_context.state[auth_config.credential_key] = old_cred
439+
callback_context.state[State.SECRET_PREFIX + auth_config.credential_key] = (
440+
new_cred
441+
)
442+
443+
result = await credential_service.load_credential(
444+
auth_config, callback_context
445+
)
446+
assert result.oauth2.client_id == "new_client"
447+
448+
@pytest.mark.asyncio
449+
async def test_explicit_none_secret_key_not_revived_by_legacy(
450+
self, credential_service, auth_config, callback_context
451+
):
452+
"""Explicit None in secret: key must not fall back to legacy key."""
453+
old_cred = AuthCredential(
454+
auth_type=AuthCredentialTypes.OAUTH2,
455+
oauth2=OAuth2Auth(
456+
client_id="stale_client",
457+
client_secret="stale_secret",
458+
),
459+
)
460+
callback_context.state[auth_config.credential_key] = old_cred
461+
callback_context.state[State.SECRET_PREFIX + auth_config.credential_key] = (
462+
None
463+
)
464+
465+
result = await credential_service.load_credential(
466+
auth_config, callback_context
467+
)
468+
assert result is None

0 commit comments

Comments
 (0)