fix(mcp): re-register the OAuth client when its redirect URI no longer matches - #2620
Conversation
🦋 Changeset detectedLatest commit: 8cef67f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98d5b81926
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| provider.setRedirectUrl(new URL(callbackServer.redirectUri)); | ||
| await provider.ready; | ||
| await provider.invalidateStaleRegistration(callbackServer.redirectUri); |
There was a problem hiding this comment.
Dropping the stored DCR record before auth() runs removes the old clientInformation that the SDK uses for the refresh-token branch. On servers that bind refresh tokens to the original client_id, refreshAuthorization() will now fail with invalid_grant/invalid_client; the SDK re-throws those OAuth errors instead of falling through to the browser flow, so a user who still had a usable refresh token can lose reauth entirely. The same pattern is applied in the v1 service.
Useful? React with 👍 / 👎.
…r matches The callback listener binds a random port per flow, while DCR registration records the redirect URI of the flow that created it — so every interactive authorization after the first was rejected with "Invalid redirect URI", an error rendered only in the user's browser while the client waited for a callback that never came. Detect the mismatch before invoking auth() and drop the stale registration so the flow re-registers with the current callback URI (v1 + v2). Resolve MoonshotAI#2606
98d5b81 to
8cef67f
Compare
2 participants
Related Issue
Resolve #2606
Problem
See the linked issue and its root-cause comment:
startCallbackServer()binds a random free port per flow (oauth/callback-server.ts,listen(0)), while dynamic client registration records the first flow's redirect URI. Every subsequent interactive authorization therefore presents a callback URI the registration does not cover, and the authorization endpoint rejects it with "Invalid redirect URI" — deterministically, on the second interactive flow and every one after. The rejection renders only in the user's browser; this client just waits for a callback that never comes. The only recovery was manually deletingcredentials/mcp/<key>-client.json.Verified live in both directions: a second interactive authorization against a stored registration failed with exactly this error; after clearing the registration the same flow succeeded first try.
What changed
McpOAuthClientProvider(v1mcp/oauth/provider.ts, v2mcpCore/oauth/provider.ts) gainsinvalidateStaleRegistration(redirectUri): when a stored registration exists and itsredirect_urisdo not cover the given URI, the client registration is dropped (tokens and discovery state untouched) andtrueis returned.beginAuthorization(v1/v2oauth/service.ts) calls it right after binding the callback listener, before invokingauth()— soauth()finds no client information and re-registers with the current callback URI. Flows whose registration still matches are untouched.Verification
test/mcp/oauth-store.test.ts, v2test/mcpCore/oauth/store.test.ts): mismatched registration dropped withtrue; matching registration kept; no-op without a stored registration. Suites pass (16 v1 / 12 v2).tsc --noEmitandoxlintclean for both packages.@moonshot-ai/kimi-code: patch) — the failure is user-visible (re-authorization goes from always-failing to working).