Save existing refresh_token in store if no new refresh_token is returned#483
Merged
pcarleton merged 1 commit intomodelcontextprotocol:mainfrom May 21, 2025
Conversation
As described in https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-12#name-refresh-token-response, server MAY return a new refresh_token. In case it doesn't, current implementation discard initial refresh token if it was still valid.
Contributor
Author
|
Hi @ihrpr, sorry for the ping 🙇♂️ |
pcarleton
approved these changes
May 21, 2025
Member
pcarleton
left a comment
pcarleton
left a comment
There was a problem hiding this comment.
LGTM thanks for this.
I was wondering if we could do this at the callsite instead of in the function itself, but I think that's messier. One concern would be if the refresh token was consumed, and we're keeping around a stale one, but I think stale refresh token handling more generally should handle this.
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
3 participants
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.
As described in OAuth 2.1 specs on refresh token response, server MAY return a new refresh_token. In case it doesn't, current implementation discards initially obtained refresh_token even if it is still valid.
Motivation and Context
Ensure refresh_token can be used more than once to generate a new access_token
How Has This Been Tested?
Added a test case to
auth.test.tsto simulate the case where norefresh_tokenis returned in response from/tokenrequest withrefresh_tokengrant.Breaking Changes
None
Types of changes
Checklist
Additional context