Fix token_expiry_time upon context initialization for stored tokens.
#1784
+109
−0
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.
Partial fix for #1318.
Motivation and Context
When storing OAuth tokens in a persistent storage, the OAuthClientProvider context would never refresh tokens, because it would consider tokens valid (by
is_token_valid()) and proceed with the expiredaccess_tokenuntil it meets a 401 response from the MCP server, forcing users to start a new OAuth flow.The proposed change is small and only solve part of the problem. It's mainly inspired by the discussion from #1318 and the approach taken in the FastMCP library: https://github.com/jlowin/fastmcp/blob/main/src/fastmcp/client/auth/oauth.py
This fix works in the case where
token.expires_inreturned by theTokenStorageis well calculated (which is not the case if we just simply store theOAuthTokenas is, cf. below), and when the token endpoint is the defaultMCP_SERVER_URL/token.This PR could also be considered a draft, as it raises other questions about how
client_metadata(in case where token endpoint is not obvious) andtoken.expires_atvalues should be stored.How Has This Been Tested?
Here's a sample script to test the proposed change (tested with Notion MCP and Linear MCP). It uses a StoredToken class to hold an extra expires_at value, to return a correct token.expires_in value in
get_tokens(cf. jlowin/fastmcp@f73b7b5)Steps:
Breaking Changes
No breaking changes.
Types of changes
Checklist
Additional context
The added tests complement the existing
test_token_validity_checkas it assumes thetoken_expiry_timegets set at initialization.