Skip to content

CORSMiddleware on /register and /token forwards any non-preflight OPTIONS request straight to the body-reading handler #3652

Description

@JordanAfolabiLaw

Affected versions: mcp 1.30.0 (confirmed); still present in the latest release, mcp 2.3.0, same code at src/mcp/server/auth/routes.py:119-120 (/token) and :132-136 (/register).

Where

  • routes.py:57-63: _cors() wraps the handler in starlette.middleware.cors.CORSMiddleware with allow_methods=["POST","OPTIONS"].
  • routes.py:118-123 and :130-136: both routes list OPTIONS and are wrapped by _cors.
  • starlette/middleware/cors.py:86 (starlette 1.7.0): CORSMiddleware.__call__ only treats the request as a preflight when origin is not None AND method == "OPTIONS" AND "access-control-request-method" in headers; any other OPTIONS (including one with no CORS headers at all) falls through to simple_response, which calls the wrapped handler.

Repro (standalone, mcp + starlette + httpx only)

import anyio
from pydantic import AnyHttpUrl
from starlette.applications import Starlette
from mcp.server.auth.routes import create_auth_routes
from mcp.server.auth.settings import ClientRegistrationOptions
from httpx import ASGITransport, AsyncClient

class DummyProvider:
    def __init__(self): self.clients = {}
    async def register_client(self, client_info): self.clients[client_info.client_id] = client_info
    async def get_client(self, client_id): return self.clients.get(client_id)
    async def authorize(self, *a, **k): ...
    async def load_authorization_code(self, *a, **k): ...
    async def exchange_authorization_code(self, *a, **k): ...
    async def load_refresh_token(self, *a, **k): ...
    async def exchange_refresh_token(self, *a, **k): ...
    async def revoke_token(self, *a, **k): ...

async def main():
    provider = DummyProvider()
    routes = create_auth_routes(provider, issuer_url=AnyHttpUrl("https://example.test"), client_registration_options=ClientRegistrationOptions(enabled=True))
    app = Starlette(routes=routes)
    transport = ASGITransport(app=app, raise_app_exceptions=False)
    async with AsyncClient(transport=transport, base_url="https://example.test") as client:
        r1 = await client.options("/register")
        print("OPTIONS /register, no body, no CORS headers ->", r1.status_code)
        r2 = await client.request("OPTIONS", "/register", content=b'{"redirect_uris": ["https://client.example/cb"], "client_name": "demo"}', headers={"Content-Type": "application/json"})
        print("OPTIONS /register, JSON body, no CORS headers ->", r2.status_code, r2.text[:200])
        print("Clients stored:", list(provider.clients.keys()))

anyio.run(main)

Expected vs actual

  • Expected: a non-preflight OPTIONS is rejected (405) or answered empty, never reaching the handler.
  • Actual: the empty-body OPTIONS reaches RegistrationHandler.handle, which calls request.json() on an empty body and raises an uncaught JSONDecodeError (unhandled 500), and the JSON-body OPTIONS is parsed, assigned a client_id/client_secret, stored via provider.register_client, and answered 201 — a registration created over what looked like a CORS preflight, bypassing rate limiting.

Suggested fix

Check origin is not None and "access-control-request-method" in headers before dispatch, in create_auth_routes/cors_middleware, answering a non-preflight OPTIONS with 405 + Allow header; no Starlette change required.

Activity

  1. added
    v2Affects the v2 line (2.x on main)
    v1Affects the v1.x maintenance line
    on Oct 7, 2026
  2. v0ropaev commented on Oct 7, 2026

    @v0ropaev

    Reproduced on current main (91941ed) with the repo's own MockOAuthProvider and httpx2, four cases through create_auth_routes with registration and revocation enabled:

    OPTIONS /register, JSON body, no CORS headers -> 201  {"client_id":"ea5b219c-...","client_secret":"...", ...}
    provider.clients after that request                  -> ['ea5b219c-9d22-4673-933f-5be9fa491353']
    OPTIONS /register, Origin + Access-Control-Request-Method -> 200 with the CORS headers (a real preflight, unaffected)
    OPTIONS /revoke, form body                           -> 401 {"error":"unauthorized_client","error_description":"Missing client_id"}
    OPTIONS /token,  form body                           -> 401 {"error":"invalid_client","error_description":"Missing client_id"}
    

    So the routing is as you describe, and /revoke belongs on the list too (routes.py:143 registers it the same way). But of the three, /register is the only one where the OPTIONS has an effect: /token and /revoke run ClientAuthenticator.authenticate_request before touching the body, so they answer 401 regardless of method. On /register there is nothing to authenticate against, so a client really does get minted.

    One thing that matters for whichever fix you pick. tests/server/auth/test_error_handling.py:300 already parametrises this exact shape:

    # The other methods these routes accept reach the same body-reading handlers.
    ("OPTIONS", "/token", _FORM),
    ("OPTIONS", "/revoke", _FORM),
    ("OPTIONS", "/register", "application/json"),

    and asserts 413 for an oversized body. So the ordering is load-bearing. I tried the obvious shape, a small ASGI wrapper that answers 405 for an OPTIONS that got past CORSMiddleware (a genuine preflight never reaches it, since CORS is the outermost wrapper), and placement decides whether those three stay green:

    • _cors(guard(_body_limited(handler))) gives 405 before the body is measured, and those three parametrisations go from 413 to 405.
    • _cors(_body_limited(guard(handler))) keeps them at 413 and still turns a normal-size OPTIONS into 405. With the guard there, tests/server/auth and tests/server/mcpserver/auth are 136 passed, same as on main, and the four cases above become 405 / 200 / 405 / 405.

    Not sending a PR, per CONTRIBUTING. Happy to answer anything about the above if it is useful.

  3. rupak-eng commented on Oct 8, 2026

    @rupak-eng

    Reproduced on current main (91941ed4) with the issue's harness: OPTIONS /register with a JSON body, no CORS headers → 201 and a client registered in the provider; empty-body OPTIONS /register → 400 from the handler; OPTIONS /token → 401 from the token handler. So the write side effect is real, not just a status-code quirk.

    The approach I'd take: short-circuit non-preflight OPTIONS inside the _cors wrapper (the outermost layer), answering 204 empty before the body-limit middleware or the handler ever runs. Preflight detection mirrors CORSMiddleware's own rule (Origin + Access-Control-Request-Method required), so true preflights keep flowing into the middleware unchanged, and POST behaviour is untouched. Sitting outside the body limit also means an over-limit OPTIONS body is never read at all (204 instead of the current 413 — the old 413 was a side effect of the fall-through, not a designed property; the oversized-body protection for POST is unchanged).

    I have this implemented and tested locally (6 new regression tests in tests/server/auth/test_routes.py: empty/body-carrying OPTIONS on /register and /token → 204 with nothing registered; origin-without-preflight-headers → 204; true preflight → 200 with CORS headers; POST /register still mints a client; plus a pin that an over-limit OPTIONS body is answered 204 without being read). Full tests/server/auth/ suite green (100 passed), ruff check/ruff format/pyright clean, and the touched code is fully covered. Happy to open a PR if this is something you'd take from outside.

    AI disclosure: this comment and the local implementation were prepared with AI assistance; I ran the reproduction and tests myself and can explain the change.

  4. KaiyiQuan commented on Oct 8, 2026

    @KaiyiQuan

    Hi, I've prepared a fix on KaiyiQuan:fix/3652-options-405 (PR #3658): _reject_non_preflight_options between the CORS layer and the body reader so plain OPTIONS on /token, /register, /revoke get 405 instead of being routed into the body-reading handler. 80 auth tests pass. Could you assign this issue so the PR can be reviewed? Thanks!

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

    bugSomething isn't workingv1Affects the v1.x maintenance linev2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions