Skip to content

fix(auth): distinguish rejected refresh tokens from transient failures#963

Open
stevenlee-oai wants to merge 1 commit into
modelcontextprotocol:mainfrom
stevenlee-oai:dev/stevenlee/oauth-typed-refresh-rejection
Open

fix(auth): distinguish rejected refresh tokens from transient failures#963
stevenlee-oai wants to merge 1 commit into
modelcontextprotocol:mainfrom
stevenlee-oai:dev/stevenlee/oauth-typed-refresh-rejection

Conversation

@stevenlee-oai

@stevenlee-oai stevenlee-oai commented Jul 7, 2026

Copy link
Copy Markdown

Summary

AuthError::TokenRefreshFailed(String) currently combines definitive OAuth refresh-token rejection with transient request, parse, and provider failures. try_refresh_or_reauth() then maps the whole variant to AuthorizationRequired, so a temporary provider failure can incorrectly prompt users to sign in again.

This change:

  • adds TokenRefreshRejected for OAuth invalid_grant;
  • maps only AuthorizationRequired and TokenRefreshRejected to reauthorization;
  • keeps temporarily_unavailable, transport, parse, and other failures as retryable TokenRefreshFailed;
  • returns AuthorizationRequired directly when no refresh token exists.

Tests

  • cargo test -p rmcp --all-features invalid_grant_refresh_requires_reauthorization
  • cargo test -p rmcp --all-features temporary_refresh_failure_does_not_require_reauthorization
  • cargo test -p rmcp --all-features refresh_token_returns_error_when_no_refresh_token
  • cargo clippy -p rmcp --all-features --lib --tests

The three focused tests pass. Clippy passes with two pre-existing sse_stream::from_byte_stream deprecation warnings.

@stevenlee-oai stevenlee-oai requested a review from a team as a code owner July 7, 2026 16:25
@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Jul 7, 2026

@DaleSeo DaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the contribution, @stevenlee-oai. I have a quick comment and we should address the linting errors to merge this.

/// Get access token from local credential store.
/// If expired, refresh it automatically when a refresh token is available.
/// When the access token has expired and no refresh token is available (or
/// the refresh itself fails), returns [`AuthError::AuthorizationRequired`]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we update this public doc about the new error?

@DaleSeo

DaleSeo commented Jul 8, 2026

Copy link
Copy Markdown
Member

@stevenlee-oai The linting errors will be gone once you rebase!

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.

2 participants