adk-python icon indicating copy to clipboard operation
adk-python copied to clipboard

fix: oauth refresh not triggered on token expiry

Open MarlzRana opened this issue 1 week ago • 3 comments

Link to Issue or Description of Change

Link to an existing issue:

  • Closes: https://github.com/google/adk-python/issues/3761

Problem: The method below does not return the credential and whether the credential was exchanged. It returns the credential and whether an exchanger was found. Two different things, with the latter always true if one was able to exchange to get a credential initially, as an exchanger existed for that credential type. https://github.com/google/adk-python/blob/0094eea3cadf5fe2e960cc558e467dd2131de1b7/src/google/adk/auth/credential_manager.py#L208-L225

Which means in the aforementioned (modal) case, that in the below code was_exchanged is always true, and we never hit the code to refresh the credential if needed - which means credential refresh never happens.

https://github.com/google/adk-python/blob/0094eea3cadf5fe2e960cc558e467dd2131de1b7/src/google/adk/auth/credential_manager.py#L161-L167 Solution: Let's get the credential exchangers to indicate whether they performed token exchange and pass that back correctly in _exchange_credential in the credential manager, so that was_exchanged is correctly set to False (and not always True) when we already have a token in the credential, and hence, we do check if the credential needs refreshing, and refresh if necessary (instead of always skipping over this logic when was_exchanged was always True prior to this PR).

Testing Plan

Unit Tests:

  • [x] I have added or updated unit tests for my change.
  • [x] All unit tests pass locally.

Manual End-to-End (E2E) Tests:

Tested this locally with the ADK web and a long-running session that surpassed the duration of the token validity, and observed token refresh occur, and no longer obtained a 401.

Checklist

  • [x] I have read the 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.
  • [x] Any dependent changes have been merged and published in downstream modules.

MarlzRana avatar Nov 30 '25 19:11 MarlzRana

Summary of Changes

Hello @MarlzRana, I'm Gemini Code Assist[^1]! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where OAuth token refreshes were not being correctly triggered upon token expiry. The solution involves a significant refactoring of the credential exchange mechanism to provide more explicit feedback on whether a token exchange actually occurred. By modifying the exchange methods to return a boolean flag alongside the credential, the system can now accurately determine if a refresh was successful or necessary, leading to more robust and reliable authentication flows.

Highlights

  • Enhanced OAuth Token Exchange Logic: The core change involves modifying the exchange methods within the authentication system to return a tuple containing both the AuthCredential and a boolean flag indicating whether a token exchange (or refresh) actually occurred. This provides clearer state management for credential updates.
  • Improved Credential Management: The CredentialManager now explicitly checks this boolean flag to determine if a credential was successfully exchanged, which is crucial for correctly handling token expiry and refresh scenarios, ensuring that an OAuth refresh is properly triggered when needed.
  • Updated OAuth2 Exchanger Behavior: The OAuth2CredentialExchanger now consistently returns (credential, False) in cases where an exchange is not needed (e.g., token already valid, authlib not available, session creation fails, fetch_token fails, or unsupported grant type) and (credential, True) upon successful exchange.
  • Refactored Unit Tests: Corresponding unit tests for OAuth2CredentialExchanger have been updated to assert the new boolean return value and remove unnecessary @pytest.mark.asyncio decorators, leveraging pytest-asyncio's auto-detection for async test functions.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with :thumbsup: and :thumbsdown: on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

[^1]: Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

gemini-code-assist[bot] avatar Nov 30 '25 19:11 gemini-code-assist[bot]

Response from ADK Triaging Agent

Hello @MarlzRana, thank you for creating this PR!

Could you please fill out the Testing Plan section in your PR description? This information will help reviewers to review your PR more efficiently. Thanks!

adk-bot avatar Nov 30 '25 19:11 adk-bot

Response from ADK Triaging Agent

Hello @MarlzRana, thank you for creating this PR!

Could you please fill out the Testing Plan section in your PR description? This information will help reviewers to review your PR more efficiently. Thanks!

adk-bot avatar Nov 30 '25 20:11 adk-bot