Skip to content

Comments

fix (auth): refresh token expiry#680

Merged
alexhancock merged 1 commit intomodelcontextprotocol:mainfrom
ArcadeAI:fix/refresh-expiry
Feb 24, 2026
Merged

fix (auth): refresh token expiry#680
alexhancock merged 1 commit intomodelcontextprotocol:mainfrom
ArcadeAI:fix/refresh-expiry

Conversation

@wdawson
Copy link
Contributor

@wdawson wdawson commented Feb 23, 2026

OAuth token expiration was not being handled correctly.

Motivation and Context

Two bugs currently exist:

  1. Token expiry is never detected: expires_in() from the oauth2 crate returns the original duration from the token response (e.g., 3600s), not the time remaining. So the check if expires_in <= Duration::from_secs(0) is always false
  2. Refresh failures don't trigger re-auth: If refresh_token() fails (e.g., refresh token revoked), the error propagates as TokenRefreshFailed instead of AuthorizationRequired, so it's more difficult for the client to know when to re-prompt.

How Has This Been Tested?

  • Tested locally against an Arcade.dev MCP Gateway server
  • Added unit tests for the new behavior

Breaking Changes

Technically breaking, but should provide better behavior:

  • The AuthorizationRequired error now propagates instead of TokenRefreshFailed to provide a clear, consistent contract for clients

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Also contains the fix from @glicht in #678 (thanks!). That should merge first for proper attribution for that fix.

AI Disclosure: AI assisted with implementation and test authoring. All changes were reviewed and guided by hand.

@wdawson wdawson requested a review from a team as a code owner February 23, 2026 22:47
@github-actions github-actions bot added T-core Core library changes T-transport Transport layer changes labels Feb 23, 2026
@glicht
Copy link
Contributor

glicht commented Feb 24, 2026

I also saw this behavior with the expiry. I wasn't fully sure if it was by design or a bug. My way to get around this was to implement at the credential store's load function to decrement the expire_in to reflect the actual number of seconds left for expiration (or zero once expired).

Copy link
Contributor

@alexhancock alexhancock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good find, and thanks for preserving all the interaction with the credential store

@alexhancock alexhancock merged commit 83808d3 into modelcontextprotocol:main Feb 24, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants