Repository navigation
CORSMiddleware on /register and /token forwards any non-preflight OPTIONS request straight to the body-reading handler #3652
Copy link
Copy link
Open
Labels
bugSomething isn't workingSomething isn't workingv1Affects the v1.x maintenance lineAffects the v1.x maintenance linev2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)
Description
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 instarlette.middleware.cors.CORSMiddlewarewithallow_methods=["POST","OPTIONS"].routes.py:118-123and:130-136: both routes listOPTIONSand are wrapped by_cors.starlette/middleware/cors.py:86(starlette 1.7.0):CORSMiddleware.__call__only treats the request as a preflight whenorigin is not NoneANDmethod == "OPTIONS"AND"access-control-request-method" in headers; any otherOPTIONS(including one with no CORS headers at all) falls through tosimple_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
OPTIONSis rejected (405) or answered empty, never reaching the handler. - Actual: the empty-body
OPTIONSreachesRegistrationHandler.handle, which callsrequest.json()on an empty body and raises an uncaughtJSONDecodeError(unhandled 500), and the JSON-bodyOPTIONSis parsed, assigned aclient_id/client_secret, stored viaprovider.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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workingv1Affects the v1.x maintenance lineAffects the v1.x maintenance linev2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)
Activity
v0ropaev commentedon Oct 7, 2026
Reproduced on current main (
91941ed) with the repo's ownMockOAuthProviderandhttpx2, four cases throughcreate_auth_routeswith registration and revocation enabled:So the routing is as you describe, and
/revokebelongs on the list too (routes.py:143registers it the same way). But of the three,/registeris the only one where the OPTIONS has an effect:/tokenand/revokerunClientAuthenticator.authenticate_requestbefore touching the body, so they answer 401 regardless of method. On/registerthere 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:300already parametrises this exact shape: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/authandtests/server/mcpserver/authare 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.
rupak-eng commentedon Oct 8, 2026
Reproduced on current main (
91941ed4) with the issue's harness:OPTIONS /registerwith a JSON body, no CORS headers →201and a client registered in the provider; empty-bodyOPTIONS /register→400from the handler;OPTIONS /token→401from 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
OPTIONSinside the_corswrapper (the outermost layer), answering204empty before the body-limit middleware or the handler ever runs. Preflight detection mirrorsCORSMiddleware's own rule (Origin+Access-Control-Request-Methodrequired), so true preflights keep flowing into the middleware unchanged, andPOSTbehaviour is untouched. Sitting outside the body limit also means an over-limitOPTIONSbody 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 forPOSTis unchanged).I have this implemented and tested locally (6 new regression tests in
tests/server/auth/test_routes.py: empty/body-carryingOPTIONSon/registerand/token→ 204 with nothing registered; origin-without-preflight-headers → 204; true preflight → 200 with CORS headers;POST /registerstill mints a client; plus a pin that an over-limitOPTIONSbody is answered 204 without being read). Fulltests/server/auth/suite green (100 passed),ruff check/ruff format/pyrightclean, 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.
KaiyiQuan commentedon Oct 8, 2026
Hi, I've prepared a fix on
KaiyiQuan:fix/3652-options-405(PR #3658):_reject_non_preflight_optionsbetween the CORS layer and the body reader so plain OPTIONS on/token,/register,/revokeget 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!