Skip to content

ToolAuthHandler._get_existing_credential refreshes OAuth2 credentials in memory but doesn't persist themΒ #5329

Closed

Description

@doughayden

πŸ”΄ Required Information

Describe the Bug:

ToolAuthHandler._get_existing_credential() calls OAuth2CredentialRefresher.refresh() to refresh expired OAuth2 credentials, but the refreshed credential is only updated in memory β€” it's never written back to the credential store. As a result, the next tool invocation reads the stale pre-refresh credential from state, attempts another refresh using the now-rotated refresh_token, fails (because the provider already invalidated the old refresh_token), and triggers a full OAuth re-authorization flow.

The class already has a _store_credential() method that does exactly what's needed β€” it's just not called after a successful refresh.

Likely cause of the oversight:

OAuth2CredentialRefresher.refresh() calls update_credential_with_tokens(), which mutates the oauth2 sub-object of the passed AuthCredential in place (auth_credential.oauth2.access_token = tokens.get("access_token"), etc.). A reader could reasonably assume that because the refresh mutates in place, the credential store already reflects the new tokens. That assumption holds for a store that holds object references directly β€” but not for the actual implementations.

ToolContextCredentialStore.store_credential() serializes via auth_credential.model_dump(exclude_none=True) and writes the resulting dict to tool_context.state. get_credential() reconstructs a fresh Pydantic model from that dict on each call. The in-memory object returned by get_credential() is disconnected from the stored dict β€” mutations to the object don't propagate back. So the next invocation deserializes the original pre-refresh dict and gets the stale tokens.

The fix is to explicitly re-serialize by calling _store_credential() after the refresh.

Steps to Reproduce:

  1. Configure an OpenAPIToolset with OAuth2 authorizationCode flow, using a provider that rotates refresh_tokens on refresh (Salesforce, many OIDC providers).
  2. Use a persistent session backing store (DatabaseSessionService or similar).
  3. Complete the initial OAuth authorization flow.
  4. Expire the access_token (naturally or by setting expires_at=<past> in session state).
  5. Invoke a tool β€” refresh fires, returns a new access_token and rotated refresh_token. The tool executes successfully.
  6. Send another message that triggers a tool call.
  7. Observe: a full OAuth redirect is triggered, even though the refresh just succeeded moments earlier.

Expected Behavior:

After a successful refresh, the new access_token and refresh_token should be persisted to the credential store. The next tool invocation should use the refreshed credential β€” no re-authorization should be required.

Observed Behavior:

The first post-expiry invocation works correctly (in-memory refresh succeeded). The second invocation fails because the credential in state still has the stale pre-refresh tokens, and attempting to refresh with a rotated refresh_token causes the provider to reject the request. The framework then calls _request_credential() and triggers a full OAuth redirect.

Environment Details:

  • ADK Version: google-adk 1.28.0
  • Desktop OS: macOS (reproduced), Linux (expected to affect all platforms)
  • Python Version: Python 3.13

Model Information:

  • Are you using LiteLLM: No
  • Which model is being used: gemini-2.5-flash (this is in the auth path, not the model path)

🟑 Optional Information

Additional Context:

Root cause in _get_existing_credential in google/adk/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.py:

async def _get_existing_credential(
    self,
) -> Optional[AuthCredential]:
    """Checks for and returns an existing, exchanged credential."""
    if self.credential_store:
        existing_credential = self.credential_store.get_credential(
            self.auth_scheme, self.auth_credential
        )
        if existing_credential:
            if existing_credential.oauth2:
                refresher = OAuth2CredentialRefresher()
                if await refresher.is_refresh_needed(existing_credential):
                    existing_credential = await refresher.refresh(
                        existing_credential, self.auth_scheme
                    )
                    # ← refreshed credential is never written back to store
            return existing_credential
    return None

Proposed fix β€” add one line after the refresh call:

async def _get_existing_credential(
    self,
) -> Optional[AuthCredential]:
    if self.credential_store:
        existing_credential = self.credential_store.get_credential(
            self.auth_scheme, self.auth_credential
        )
        if existing_credential:
            if existing_credential.oauth2:
                refresher = OAuth2CredentialRefresher()
                if await refresher.is_refresh_needed(existing_credential):
                    existing_credential = await refresher.refresh(
                        existing_credential, self.auth_scheme
                    )
                    self._store_credential(existing_credential)  # ← persist refreshed credential
            return existing_credential
    return None

Impact: Any ADK user with an OAuth provider that rotates refresh_tokens (standard security practice) will see a full re-authorization prompt on the message after every access_token expiry. This is additionally misleading because the first post-expiry invocation works correctly (the in-memory refresh succeeded), so the failure mode shows up one step removed from the root cause.

Companion issues (filed together):

Minimal Reproduction Code:

Complete runnable reproduction β€” mirrors the contributing/samples/oauth2_client_credentials layout (agent.py + main.py + oauth2_test_server.py + README.md), adapted to use the authorization_code flow. A --apply-fix CLI flag monkey-patches the proposed fix so the same script demonstrates both the bug and its resolution:

https://github.com/doughayden/adk-issue-examples/tree/2f454e73c2f1885ebe9be61125e02800cb2164b3/03-refresh_not_persisted

Expected output β€” the script sends two weather queries in the same session. Without --apply-fix, the first succeeds but the second triggers a full re-auth because the refreshed credential was never persisted to the store. With --apply-fix, both succeed.

Workaround applied in our project β€” monkey-patch ToolAuthHandler._get_existing_credential:

from google.adk.tools.openapi_tool.openapi_spec_parser import tool_auth_handler
from google.adk.auth.refresher import oauth2_credential_refresher

async def _get_existing_credential_and_persist(self):
    if not self.credential_store:
        return None
    existing = self.credential_store.get_credential(
        self.auth_scheme, self.auth_credential
    )
    if not existing:
        return None
    if existing.oauth2:
        refresher = oauth2_credential_refresher.OAuth2CredentialRefresher()
        if await refresher.is_refresh_needed(existing):
            refreshed = await refresher.refresh(existing, self.auth_scheme)
            self._store_credential(refreshed)
            return refreshed
    return existing

tool_auth_handler.ToolAuthHandler._get_existing_credential = _get_existing_credential_and_persist

How often has this issue occurred?: Always (100%) β€” reproduces on every access_token expiry for any provider that rotates refresh_tokens.

Activity

  1. added
    auth[Component] This issue is related to authorization
    on Apr 14, 2026
  2. added theissue type on Apr 14, 2026
  3. llalitkumarrr commented on Apr 16, 2026

    @llalitkumarrr
    Collaborator

    Hello @doughayden ,

    Thank you for such a detailed report and for proposing a fix. We noticed there is a PR linked to this issue that aligns with your suggestion. If you have time could you please take a look at it and share your feedback?

    Meanwhile our team will review the PR and merge it once it passes all checks and is approved by reviewer.

  4. added
    request clarification[Status] The maintainer need clarification or more information from the author
    on Apr 16, 2026
  5. doughayden commented on Apr 16, 2026

    @doughayden
    ContributorAuthor

    @llalitkumarrr thanks for your reply!

    Yes, PR #5350 matches the proposed fix exactly. self._store_credential(existing_credential) after the refresh in _get_existing_credential, same line, same location.

    The added regression test (test_refreshed_credential_is_persisted_to_store) follows the existing pattern in test_tool_auth_handler.py (mirrors test_openid_connect_existing_oauth2_token_refresh) and exercises the fix behaviorally. It uses a real ToolContextCredentialStore and reads back via get_credential to verify the rotated access_token and refresh_token are persisted, so it would fail without the fix.

    Looks good to merge from my side.

  6. llalitkumarrr commented on Apr 21, 2026

    @llalitkumarrr
    Collaborator

    Hello @doughayden ,

    Thank you for sharing your inputs.

    Hello @rohityan could you please take a look at linked PR?

  7. added
    needs review[Status] The PR/issue is awaiting review from the maintainer
    and removed
    request clarification[Status] The maintainer need clarification or more information from the author
    on Apr 21, 2026
  8. adk-bot commented on May 1, 2026

    @adk-bot
    Collaborator

    This issue has been automatically marked as stale because it has not had recent activity for 7 days after a maintainer requested clarification. It will be closed if no further activity occurs within 7 days.

  9. added
    stale[Status] Issues which have been marked inactive since there is no user response
    request clarification[Status] The maintainer need clarification or more information from the author
    on May 1, 2026
  10. doughayden commented on May 4, 2026

    @doughayden
    ContributorAuthor

    @llalitkumarrr gentle bump to ensure this doesn't go stale and close, ty.

  11. removed
    stale[Status] Issues which have been marked inactive since there is no user response
    on May 5, 2026
  12. removed
    request clarification[Status] The maintainer need clarification or more information from the author
    on May 5, 2026
  13. added a commit that references this issue on May 12, 2026
    218ea76
  14. added 2 commits that reference this issue on Aug 11, 2026
    b488d06
    dc2f8f9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

auth[Component] This issue is related to authorizationneeds review[Status] The PR/issue is awaiting review from the maintainer

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions