fix(client/auth): use stored refresh_token on 401 instead of full re-authorization#2875
Open
beraterkanelcelik wants to merge 1 commit into
Open
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When an
OAuthClientProvideris reconstructed and loads previously stored tokensvia
TokenStorage, a storedrefresh_tokenis never used — every access-tokenexpiry forces a full interactive re-authorization.
Root cause, in
async_auth_flow(src/mcp/client/auth/oauth2.py):_initialize()loadscurrent_tokensfrom storage but does not restoretoken_expiry_time(it staysNone).OAuthToken.expires_inis relative, so areloaded provider has no way to know the token's absolute expiry.
token_expiry_time is None,is_token_valid()returnsTruefor theloaded token regardless of its real state, so the proactive-refresh branch
if not is_token_valid() and can_refresh_token()is skipped.401, but the401branchproceeds directly to a full authorization-code grant. It never attempts a
refresh_tokengrant — even when a validrefresh_tokenandclient_infoarepresent 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
401branch ofasync_auth_flow, after metadata discovery and beforeclient registration / authorization, attempt a
refresh_tokengrant whencan_refresh_token()is true. The discovery already performed in that branchresolves the token endpoint, so the stored
refresh_tokencan 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()isTrueafter_initialize()loads a stored token,because expiry isn't restored).
test_reloaded_provider_uses_refresh_token_on_401— drives the fullreload → 401 → refresh path and asserts a
refresh_tokengrant is issued (not anew authorization) and the original request is retried with the refreshed token.
test_reloaded_provider_falls_back_to_reauth_when_refresh_fails— asserts theflow falls back to full re-authorization when the refresh grant fails.
All existing auth tests continue to pass.