feat(core): bind MCP OAuth refresh tokens to their issuing authorization server - #47814
Open
rekram1-node wants to merge 2 commits into
Open
feat(core): bind MCP OAuth refresh tokens to their issuing authorization server#47814rekram1-node wants to merge 2 commits into
rekram1-node wants to merge 2 commits into
Conversation
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.
Summary
Follow-up to #47743. Records the authorization server's
issuerwhen an MCP OAuth credential is obtained, and refuses to present the stored refresh token if a later connection discovers a different issuer for the same MCP server. Mirrors Codex'svalidate_refresh_token_issuer(codex-rs/rmcp-client/src/oauth/issuer_binding.rs).Why
A refresh token must only go back to the server that issued it. With Dynamic Client Registration this was implicitly enforced: the
client_idwas minted by one specific authorization server, so if the MCP server switched auth providers the old id was simply unknown and refresh failed. A CIMDclient_idis a public URL that is valid at any authorization server. If an MCP server's/.well-known/oauth-protected-resourcestarts pointing somewhere else, nothing in the current flow stops us from sending the refresh token to the new (possibly hostile) server. Storing the issuer at login and checking it before refresh closes that.Behavior
McpOAuth.authorizestoresmetadata.issueron the credential from the discovery it already performs.saveDiscoveryStatehook) and passes it totoTokens. If the stored issuer is set and differs,refresh_tokenis omitted from what the SDK sees, so the SDK skips refresh and falls through to re-authorization →UnauthorizedError→needs_auth. The access token is still presented, so an unexpired session keeps working until it lapses. A warning is logged with expected/actual issuer.Changes
packages/core/src/mcp/oauth.ts—issuerFromCredential,issuerontoCredential, issuer-awaretoTokens,onDiscoveryprovider optionpackages/core/src/mcp/index.ts— connect-time provider records the discovered issuer, withholds refresh on mismatch, carries the issuer forward on refreshpackages/core/test/mcp-oauth.test.ts— login stores the issuer; refresh is withheld and no/tokenrequest is made when discovery names a different issuerTest plan
bun test test/mcp-oauth.test.tsinpackages/core(10 pass)bun typecheckinpackages/core