Skip to content

Improve OAuth2 Token Refresh Implementation #290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Shifat7 opened this issue Apr 19, 2025 · 2 comments
Open

Improve OAuth2 Token Refresh Implementation #290

Shifat7 opened this issue Apr 19, 2025 · 2 comments
Assignees
Labels
tools Issues related to tools
Milestone

Comments

@Shifat7
Copy link

Shifat7 commented Apr 19, 2025

Problem

The current OAuth2 token refresh implementation has several limitations:

  1. Token refresh logic doesn't properly handle OpenID Connect configurations where tokenUrl is accessed as an object property instead of a dictionary key
  2. Test coverage for token refresh scenarios is insufficient and doesn't accurately mock OAuth2 session behaviour

Proposed Changes

Implementation Updates

  • Fixed token URL access in OAuth2 exchanger to properly handle both OpenID Connect and OAuth2 configurations:
    # Before
    token_url = auth_scheme.openIdConnect.tokenUrl or auth_scheme.oauth2.tokenUrl
    
    # After
    token_url = (auth_scheme.openIdConnect.get("tokenUrl") if auth_scheme.openIdConnect else None) or auth_scheme.oauth2.tokenUrl

Test Improvements

  • Added comprehensive test coverage for token refresh scenarios:
    • Successful token refresh
    • Missing token URL handling
    • Network request failures
    • Fallback to existing token when refresh fails
  • Improved mock setup to accurately reflect real OAuth2 session behavior
  • Added parameterized tests for both GOOGLE_AI and VERTEX configurations
@Shifat7
Copy link
Author

Shifat7 commented Apr 19, 2025

Opened the PR for this change

@Shifat7
Copy link
Author

Shifat7 commented Apr 21, 2025

@hangfei would love to get your feedback on this thanks!

@hangfei hangfei assigned hangfei and seanzhou1023 and unassigned hangfei Apr 28, 2025
@hangfei hangfei added the tools Issues related to tools label Apr 28, 2025
@seanzhou1023 seanzhou1023 added this to the FixIt Week milestone May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues related to tools
Projects
None yet
Development

No branches or pull requests

3 participants