fix(auth): keep the OAuth2 client secret out of session state

`SessionStateCredentialService` wrote the exchanged credential to session
state verbatim, so the agent's `client_secret` reached anything that can read
a session. It is now stripped before storage and put back from the auth config
in the store's own `load_credential`, which token refresh needs. The end user's
own tokens are still stored in clear text; that is what this store is for, and
its docstring now says so.

Only new writes are affected. A session already persisted by a durable backend
such as `DatabaseSessionService` or `SqliteSessionService` still holds the
secret in clear text, so rotate any OAuth2 client secret an agent has used with
one.

Co-authored-by: Kathy Wu <wukathy@google.com>
PiperOrigin-RevId: 987142295

Author: wukath

src/google/adk/auth/auth_handler.py

@@ -24,6 +24,8 @@
 from .auth_schemes import OpenIdConnectWithConfig
 from .auth_tool import AuthConfig
 from .exchanger.oauth2_credential_exchanger import OAuth2CredentialExchanger
+from .oauth2_credential_util import _credential_without_client_secret
+from .oauth2_credential_util import _with_configured_client
 
 if TYPE_CHECKING:
   from ..sessions.state import State
@@ -48,18 +50,6 @@ def _normalize_oauth_scopes(
   return list(scopes)
 
 
-def _credential_without_client_secret(
-    credential: AuthCredential | None,
-) -> AuthCredential | None:
-  """Returns a copy of credential with the OAuth2 client secret removed."""
-  if credential is None:
-    return None
-  redacted = credential.model_copy(deep=True)
-  if redacted.oauth2 is not None:
-    redacted.oauth2.client_secret = None
-  return redacted
-
-
 def _without_client_secret(auth_config: AuthConfig) -> AuthConfig:
   """Returns a copy of auth_config with OAuth2 client secrets removed.
 
@@ -103,8 +93,9 @@ async def parse_and_store_auth_response(self, state: State) -> None:
 
     temp_credential_key = "temp:" + credential_key
 
-    self.auth_config.exchanged_auth_credential = self._with_configured_client(
-        self.auth_config.exchanged_auth_credential
+    self.auth_config.exchanged_auth_credential = _with_configured_client(
+        credential=self.auth_config.exchanged_auth_credential,
+        raw_credential=self.auth_config.raw_auth_credential,
     )
     credential = self.auth_config.exchanged_auth_credential
     if self._is_exchangeable(credential):
@@ -117,28 +108,6 @@ def _validate(self) -> None:
     if not self.auth_config.auth_scheme:
       raise ValueError("auth_scheme is empty.")
 
-  def _with_configured_client(
-      self, credential: AuthCredential | None
-  ) -> AuthCredential | None:
-    """Returns credential with the configured OAuth2 client identity restored.
-
-    The credential comes back from the client, which must not be able to
-    choose which OAuth2 client the token is exchanged for. The original is
-    left untouched, so the copy held in session state keeps no secret.
-    """
-    raw_credential = self.auth_config.raw_auth_credential
-    if (
-        credential is None
-        or credential.oauth2 is None
-        or raw_credential is None
-        or raw_credential.oauth2 is None
-    ):
-      return credential
-    restored = credential.model_copy(deep=True)
-    restored.oauth2.client_id = raw_credential.oauth2.client_id
-    restored.oauth2.client_secret = raw_credential.oauth2.client_secret
-    return restored
-
   def _is_exchangeable(self, credential: AuthCredential | None) -> bool:
     """Returns whether credential still needs, and can do, a token exchange."""
     if not isinstance(
@@ -197,7 +166,10 @@ def get_auth_response(self, state: State) -> AuthCredential | None:
       return None
 
     key, credential = stored
-    credential = self._with_configured_client(credential)
+    credential = _with_configured_client(
+        credential=credential,
+        raw_credential=self.auth_config.raw_auth_credential,
+    )
     if not self._is_exchangeable(credential):
       return credential
 

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

@@ -14,6 +14,7 @@
 
 from __future__ import annotations
 
+import logging
 from typing import Optional
 
 from typing_extensions import override
@@ -22,14 +23,24 @@
 from ...utils.feature_decorator import experimental
 from ..auth_credential import AuthCredential
 from ..auth_tool import AuthConfig
+from ..oauth2_credential_util import _credential_without_client_secret
+from ..oauth2_credential_util import _with_configured_client_secret
 from .base_credential_service import BaseCredentialService
 
+logger = logging.getLogger("google_adk." + __name__)
+
 
 @experimental
 class SessionStateCredentialService(BaseCredentialService):
   """Class for implementation of credential service using session state as the
   store.
+
   Note: store credential in session may not be secure, use at your own risk.
+  Session state is returned verbatim by the session-read endpoints, so whatever
+  is kept here is readable by anything that can reach them. The agent's OAuth2
+  client secret is stripped before storage and put back from the auth config on
+  load; the end user's own access and refresh tokens are what this store exists
+  to hold and are kept in clear text.
   """
 
   @override
@@ -54,7 +65,30 @@ async def load_credential(
         Optional[AuthCredential]: the credential saved in the store.
 
     """
-    return callback_context.state.get(auth_config.credential_key)
+    stored = callback_context.state.get(auth_config.credential_key)
+    # A session service that persists state as JSON (DatabaseSessionService,
+    # SqliteSessionService) hands back what `save_credential` wrote as a plain
+    # dict, not as the model it was written as.
+    if isinstance(stored, dict):
+      stored = AuthCredential.model_validate(stored)
+    elif stored is not None and not isinstance(stored, AuthCredential):
+      # Something other than `save_credential` put this here. `AuthHandler`
+      # accepts a bare token string under the same key, but this store's
+      # contract is a credential or nothing.
+      logger.warning(
+          "Ignoring %s stored under the credential key: this store only reads"
+          " credentials written by save_credential.",
+          type(stored).__name__,
+      )
+      return None
+
+    # `save_credential` kept no client secret, so put the configured one back
+    # here rather than in any one caller: a credential without it cannot
+    # exchange or refresh.
+    return _with_configured_client_secret(
+        credential=stored,
+        raw_credential=auth_config.raw_auth_credential,
+    )
 
   @override
   async def save_credential(
@@ -78,6 +112,10 @@ async def save_credential(
         None
     """
 
+    # The client secret belongs to the agent's OAuth2 client, not to the end
+    # user whose session this is, and session state is readable by anything
+    # that can read the session. `load_credential` re-attaches it from the
+    # auth config, so it is not needed here.
     callback_context.state[auth_config.credential_key] = (
-        auth_config.exchanged_auth_credential
+        _credential_without_client_secret(auth_config.exchanged_auth_credential)
     )

src/google/adk/auth/oauth2_credential_util.py

@@ -36,6 +36,74 @@
 _TOKEN_REQUEST_TIMEOUT_SECONDS = 10
 
 
+def _credential_without_client_secret(
+    credential: Optional[AuthCredential],
+) -> Optional[AuthCredential]:
+  """Returns a copy of credential with the OAuth2 client secret removed.
+
+  The client secret identifies the agent's OAuth2 client, not the end user, so
+  it must not travel to the client or reach any store the client can read. Call
+  sites that still need it for a token request re-attach it from the tool's own
+  configuration, so dropping it here costs nothing.
+  """
+  if credential is None:
+    return None
+  redacted = credential.model_copy(deep=True)
+  if redacted.oauth2 is not None:
+    redacted.oauth2.client_secret = None
+  return redacted
+
+
+def _with_configured_client_secret(
+    *,
+    credential: Optional[AuthCredential],
+    raw_credential: Optional[AuthCredential],
+) -> Optional[AuthCredential]:
+  """Returns credential with the configured OAuth2 client secret restored.
+
+  The inverse of `_credential_without_client_secret`: a credential read back
+  from a store that holds no secret needs one again before a token exchange or
+  refresh. Only the secret is put back, so the rest of the stored credential
+  round trips untouched.
+  """
+  if (
+      raw_credential is None
+      or raw_credential.oauth2 is None
+      or credential is None
+      or credential.oauth2 is None
+  ):
+    return credential
+  restored = credential.model_copy(deep=True)
+  if restored.oauth2 is not None:
+    restored.oauth2.client_secret = raw_credential.oauth2.client_secret
+  return restored
+
+
+def _with_configured_client(
+    *,
+    credential: Optional[AuthCredential],
+    raw_credential: Optional[AuthCredential],
+) -> Optional[AuthCredential]:
+  """Returns credential with the whole configured OAuth2 client restored.
+
+  For credentials that came back through the client, which must not be able to
+  pick which OAuth2 client its token is exchanged for, so the client id is
+  pinned to the tool's own configuration along with the secret.
+  """
+  restored = _with_configured_client_secret(
+      credential=credential, raw_credential=raw_credential
+  )
+  if (
+      raw_credential is None
+      or raw_credential.oauth2 is None
+      or restored is None
+      or restored.oauth2 is None
+  ):
+    return restored
+  restored.oauth2.client_id = raw_credential.oauth2.client_id
+  return restored
+
+
 @experimental
 def create_oauth2_session(
     auth_scheme: AuthScheme,