MCP OAuth connect: fix missing state param (CL-6371) - #115
Merged
Merged
Conversation
Cover the authorize URL carrying a CSRF-binding state param, a full connect where the provider echoes it back, and a provider that omits state we sent -- rejected as a CSRF failure via the consumer envelope, never a raw provider error string.
The MCP SDK's auth() only appends ?state= to the authorize URL when the OAuthClientProvider implements an optional state() method; ours never did, so every MCP OAuth connect (PostHog and every other preset, all sharing this one route) sent authorize requests with no state at all. PostHog's authorization server rejects those with "Missing state parameter." The route already minted a nonce and sealed it into the connect cookie, but never plumbed it into the SDK provider or checked it back on the callback -- CSRF protection that looked wired but did nothing. Fix: mint the nonce before calling auth() on /start, hand it to the provider via session.state so the SDK sends it as state=, and on /callback require the provider's returned state to match exactly. Missing or mismatched state now redirects with a `state_mismatch` error code through the same outcome/code envelope every other failure here uses, never the raw provider message.
4 tasks
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
1 participant
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
auth()only appendsstate=to the authorize URL when theOAuthClientProviderimplements an optionalstate()method, and ours never did -- so every MCP OAuth connect (all presets share this one route) sent authorize requests with nostateat all.nonceand sealed it into the connect cookie, but never plumbed it into the SDK provider or validated it back on the callback -- CSRF protection that looked wired but did nothing.auth()on/start, hand it to the provider viasession.stateso the SDK sends it asstate=, and on/callbackrequire the provider's returnedstateto match exactly. Missing or mismatched state now redirects with astate_mismatchcode through the existing outcome/code envelope, never the raw provider error text (CL-6360 idiom)./.well-known/oauth-protected-resource/mcp,oauth.posthog.com/.well-known/oauth-authorization-server): standard authorization-code + PKCE (S256) flow, no credentialed requests made.Test plan
bun testinpackages/connections(188 tests, 0 fail)statestateback completes the connectstatewe sent is rejected withcode=state_mismatch, never the raw error texttsc --noEmitandeslintclean on touched files