Skip to content

fix(client/auth): use stored refresh_token on 401 instead of full re-authorization#2875

Open
beraterkanelcelik wants to merge 1 commit into
modelcontextprotocol:mainfrom
beraterkanelcelik:fix-oauth-refresh-main
Open

fix(client/auth): use stored refresh_token on 401 instead of full re-authorization#2875
beraterkanelcelik wants to merge 1 commit into
modelcontextprotocol:mainfrom
beraterkanelcelik:fix-oauth-refresh-main

Conversation

@beraterkanelcelik

Copy link
Copy Markdown

Problem

When an OAuthClientProvider is reconstructed and loads previously stored tokens
via TokenStorage, a stored refresh_token is never used — every access-token
expiry forces a full interactive re-authorization.

Root cause, in async_auth_flow (src/mcp/client/auth/oauth2.py):

  • _initialize() loads current_tokens from storage but does not restore
    token_expiry_time (it stays None). OAuthToken.expires_in is relative, so a
    reloaded provider has no way to know the token's absolute expiry.
  • With token_expiry_time is None, is_token_valid() returns True for the
    loaded token regardless of its real state, so the proactive-refresh branch
    if not is_token_valid() and can_refresh_token() is skipped.
  • The server then rejects the stale access token with 401, but the 401 branch
    proceeds directly to a full authorization-code grant. It never attempts a
    refresh_token grant — even when a valid refresh_token and client_info are
    present in storage.

This is especially visible for clients that build a fresh provider per request and
rely on persisted tokens: they re-authorize interactively on every access-token
expiry instead of refreshing silently.

Fix

In the 401 branch of async_auth_flow, after metadata discovery and before
client registration / authorization, attempt a refresh_token grant when
can_refresh_token() is true. The discovery already performed in that branch
resolves the token endpoint, so the stored refresh_token can be used directly.
If the refresh fails, the flow falls back to the existing full re-authorization
path.

Tests

Adds to tests/client/test_auth.py:

  • test_initialize_does_not_restore_token_expiry — documents the precondition
    (is_token_valid() is True after _initialize() loads a stored token,
    because expiry isn't restored).
  • test_reloaded_provider_uses_refresh_token_on_401 — drives the full
    reload → 401 → refresh path and asserts a refresh_token grant is issued (not a
    new authorization) and the original request is retried with the refreshed token.
  • test_reloaded_provider_falls_back_to_reauth_when_refresh_fails — asserts the
    flow falls back to full re-authorization when the refresh grant fails.

All existing auth tests continue to pass.

…auth

A reloaded OAuthClientProvider never restores token_expiry_time, so
is_token_valid() reports a stored (possibly expired) access token as valid and
the proactive-refresh branch in async_auth_flow is skipped. When the server then
returns 401, the flow goes straight to a full authorization-code grant — even
though a valid refresh_token and client_info are present in storage.

Clients that construct a fresh provider per request (loading tokens from
persistent storage) therefore force an interactive re-authorization on every
access-token expiry instead of refreshing silently.

Attempt a refresh_token grant in the 401 branch, after metadata discovery and
before client registration / authorization. Discovery in that branch already
yields the token endpoint, so the stored refresh_token is used directly; only if
the refresh fails does the flow fall back to full re-authorization.

Tests: add reload -> 401 -> refresh coverage to tests/client/test_auth.py for
both the success path and the fallback-to-reauth path when refresh fails.

Reported-by: Berat Elcelik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant