Skip to content

[rmcp] Notify credential stores when a refresh token is rejected - #1285

Merged
DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
mzeng-openai:dev/mzeng/oauth-rejected-refresh-hook-3.2
Sep 28, 2026
Merged

DaleSeo merged 2 commits into
modelcontextprotocol:mainfrom
mzeng-openai:dev/mzeng/oauth-rejected-refresh-hook-3.2

Conversation

@mzeng-openai

@mzeng-openai mzeng-openai commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A provider can reject a refresh token with invalid_grant, but the credential store currently has no callback to retire that token. Later calls can load it again and repeat the failed exchange.

This adds an optional CredentialStore::on_refresh_token_rejected callback at the shared refresh boundary. It receives the credentials used for the exchange while the refresh guard is still held. The default is a no-op, so existing stores keep their behavior. Callback failures are logged without replacing TokenRefreshRejected, so callers still receive the signal to reauthorize. Other refresh failures do not invoke the callback.

Key review points:

  • crates/rmcp/src/transport/auth.rs: the callback must not reacquire the refresh guard or overwrite credentials that have been replaced.

The PR targets the current upstream SDK. The original hook is also available as a 3.2.0 backport at d042aac037d97967470660b8000630b95eea3141 in mzeng-openai/rust-sdk. That backport does not include the follow-up that preserves the reauthorization signal when the callback fails.

Test Plan

  • Before the callback-error follow-up, cargo test -p rmcp --lib --features auth,client,transport-streamable-http-client,reqwest transport::auth::tests:: passed all 199 tests; the original 3.2.0 backport passed all 185 tests.
  • Tests cover the attempted credential snapshot, guard lifetime, callback failure, transient errors, and unchanged default behavior. The follow-up adds a regression proving that get_access_token() returns AuthorizationRequired when the rejection callback fails, with one provider request and the refresh guard released.
  • git diff --check passed for the follow-up. The latest push starts a new CI run; its results are pending.

@github-actions github-actions Bot added T-core Core library changes T-transport Transport layer changes labels Sep 18, 2026
@mzeng-openai
mzeng-openai marked this pull request as ready for review September 18, 2026 19:22
@mzeng-openai
mzeng-openai requested a review from a team as a code owner September 18, 2026 19:22
@mzeng-openai
mzeng-openai force-pushed the dev/mzeng/oauth-rejected-refresh-hook-3.2 branch from 4884f93 to 76ae383 Compare September 18, 2026 21:20
Comment thread crates/rmcp/src/transport/auth.rs Outdated

@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, @mzeng-openai!

@DaleSeo
DaleSeo merged commit ae2f9c9 into modelcontextprotocol:main Sep 28, 2026
24 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.

2 participants