Skip to content

authorizationHandler drops the OAuth state parameter on error redirects #2773

Description

@Ssavan99

authorizationHandler omits state from the error redirect when authorization
parameter validation fails, so a client that validates state on the callback —
the standard CSRF check from RFC 6749 §4.1.2.1 — cannot accept or correlate the
error response. The client sees a state mismatch instead of the actual OAuth
error, and the real cause (invalid_request) never reaches the user.

RFC 6749 §4.1.2.1 requires state on the error response whenever the request
carried one:

state: REQUIRED if the "state" parameter was present in the client
authorization request. The exact value received from the client.

The success path echoes state correctly; only the error path drops it.

Reproduction

import express from 'express';
import { authorizationHandler } from '@modelcontextprotocol/sdk/server/auth/handlers/authorize.js';

const CLIENT = { client_id: 'test-client', redirect_uris: ['https://client.example.com/cb'] };
const provider = {
    clientsStore: { getClient: async id => (id === CLIENT.client_id ? CLIENT : undefined) },
    async authorize(client, params, res) {
        const u = new URL(params.redirectUri);
        u.searchParams.set('code', 'authcode');
        if (params.state) u.searchParams.set('state', params.state);
        res.redirect(302, u.href);
    }
};

const app = express();
app.use('/authorize', authorizationHandler({ provider, rateLimit: false }));
const server = app.listen(3000);

// state is echoed on success:
//   /authorize?client_id=test-client&redirect_uri=...&state=xyz789
//             &response_type=code&code_challenge=abc&code_challenge_method=S256
//   -> 302 https://client.example.com/cb?code=authcode&state=xyz789
//
// state is dropped as soon as any other parameter fails validation:
//   /authorize?client_id=test-client&redirect_uri=...&state=xyz789&response_type=code
//   -> 302 https://client.example.com/cb?error=invalid_request&error_description=...
//      (no state)

Any Phase-2 validation failure reproduces it: missing code_challenge,
code_challenge_method=plain, a non-URL resource.

Cause

In packages/server-legacy/src/auth/handlers/authorize.ts, state is read from
the parse result after the parse is checked:

let state;
try {
    const parseResult = RequestAuthorizationParamsSchema.safeParse(...);
    if (!parseResult.success) {
        throw new InvalidRequestError(parseResult.error.message);   // throws here
    }
    const { scope, code_challenge, resource } = parseResult.data;
    state = parseResult.data.state;                                 // never runs

The catch then calls createErrorRedirect(redirect_uri, error, state, issuer)
with state still undefined, and createErrorRedirect omits the parameter.

Expected

The error redirect carries state=xyz789, matching the success path and RFC 6749
§4.1.2.1.

Why this matters

I run an MCP server on this SDK in production — HTTP transport behind a public
URL, mcpAuthRouter with an OAuth read scope, remote clients — and found this
while reading through that auth path rather than from an incident. The impact is
still concrete: a client whose authorization request is malformed gets back a
callback it has to reject on CSRF grounds, so the operator sees a state-mismatch
error from the client and no trace of the real cause. It turns a one-line
parameter mistake into a blind debugging session, on the error path, which is
exactly where the diagnostic is supposed to work.

Environment

  • @modelcontextprotocol/sdk 1.29.0, and reproduced on main @ 5119ee7
  • Node 22.13.0, express 5.2.1

I have a fix with regression tests and can open a PR.

Disclosure: found and patched with AI assistance (Claude Code). I have reviewed
the diagnosis, the fix, and the tests, and can speak to any of it.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions