modelcontextprotocol / modelcontextprotocol/python-sdk

OAuth client sends client_id in token body under client_secret_basic (strict servers reject as multiple auth methods)

Offen
#3,138 6 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen

Dieses Issue hat noch niemand übernommen.

auth bug needs confirmation P1 v1 v2
Vorherrschende Sprache
Python
Sterne
24.3k
Forks
4k
Ø Merge
1 T. 1 Std.
Gemergte PRs (30 T.)
31

Beschreibung

Initial Checks
  • I confirm that I'm using the latest version of MCP Python SDK
  • I confirm that I searched for my issue before opening this issue
Description

Under token_endpoint_auth_method = client_secret_basic, the OAuth client sends the client credentials in the HTTP Authorization: Basic header and also leaves client_id in the token-exchange POST body. Strict token endpoints reject this as two authentication methods in a single request.

OAuthClientProvider.prepare_token_auth (src/mcp/client/auth/oauth2.py) strips only client_secret from the body:

headers["Authorization"] = f"Basic {encoded_credentials}"
# Don't include client_secret in body for basic auth
data = {k: v for k, v in data.items() if k != "client_secret"}   # client_id remains in the body

Expected: with Basic auth, the request presents exactly one authentication method — the Authorization header — with no client credentials or identifier duplicated in the body.

Actual: client_id stays in the body.

Against Notion's MCP server (https://mcp.notion.com/mcp), where dynamic client registration returns token_endpoint_auth_method = client_secret_basic, the token exchange fails with:

Token exchange failed (400): "Client must not use multiple authentication methods"

There is also a secondary, misleading cascade (not the root cause): the failed first attempt starts a second OAuth flow that reuses the loopback callback port, and stale browser tabs redirect old approvals into it, surfacing OAuthFlowError: State parameter mismatch.

I want to frame this as an interop issue rather than a compliance accusation. RFC 6749 §2.3.1 presents the HTTP Basic header and body credentials as alternatives; sending only the header is unambiguously valid, removes the ambiguity, and loses no information (the client_id is already present, base64-encoded, in the Basic header). Stripping client_id from the body makes the SDK work against strict servers like Notion at no cost to lenient ones.

The one-line fix in the client_secret_basic branch:

data = {k: v for k, v in data.items() if k not in ("client_secret", "client_id")}

Note: the existing test test_basic_auth_token_exchange currently asserts client_id ... in content # client_id still in body — this behaviour was codified in the same commit that introduced Basic auth support (#1334), so the fix inverts that assertion.

Update: this is the CLIENT half — it depends on the server side (#1847)

The one-line client change cannot land alone. Body client_id is load-bearing on this SDK's own server side in three places, so a client that omits it fails against any server built on this SDK:

  1. ClientAuthenticator.authenticate_request (src/mcp/server/auth/middleware/client_auth.py) requires client_id in the body.
  2. TokenHandler's request models (AuthorizationCodeRequest / RefreshTokenRequest, src/mcp/server/auth/handlers/token.py) declare client_id: str as required — a missing value returns a Pydantic 400 invalid_request: client_id: Field required.
  3. The grant-ownership checks compare auth_code.client_id != token_request.client_id (and the refresh equivalent).

The matching server-side change is exactly what #1847 already implements: it makes body client_id optional, re-sources the ownership checks from the authenticated client_info.client_id, and reads client_id from the Basic header. So the two are complementary halves of one fix:

  • Client-only (this issue's change) → the SDK's own server rejects with Missing client_id. Verified.
  • Client change + #1847's server change → the previously-failing Basic-auth token exchange passes end-to-end. Verified locally by grafting #1847's server diff onto main and running the client + interaction OAuth suites (green). The only residual failures were #1847's own error-message test updates, which appear stranded by a tests/server/fastmcptests/server/mcpserver directory rename on main (i.e. #1847 looks like it needs a rebase) — unrelated to the client change.

Questions for maintainers:

  1. How would you like the client change sequenced against #1847 — folded into #1847, or as a linked follow-up PR that merges after it?
  2. Which branch(es) — main (v2), and/or a v1.x backport? The live breakage is on v1 (1.27.x) via Notion.

I'm happy to open the client-side PR (one-line change + updated tests) once there's buy-in and a preferred sequencing.

Example Code
# Any client using this SDK against Notion MCP, where dynamic registration
# returns token_endpoint_auth_method = client_secret_basic. For example via mcp2cli:
#   mcp2cli --mcp https://mcp.notion.com/mcp --oauth --list
#
# 1. Complete the browser authorization.
# 2. The token exchange returns:
#    Token exchange failed (400): "Client must not use multiple authentication methods"
Python & MCP Python SDK
Python 3.10
mcp: reproduced on 1.27.2 and 1.28.1; bug also present on main @ 3a6f299
Server: Notion MCP (https://mcp.notion.com/mcp), token_endpoint_auth_method = client_secret_basic

This report was prepared with AI assistance. I've reviewed the analysis and reproduced the failure myself, and can speak to the change directly.

Beitragsleitfaden

Beitragsleitfaden öffnen

Erste Schritte

  1. Lies das ganze Issue und danach den Beitragsleitfaden des Projekts.
  2. Schreib ins Issue, dass du es übernimmst — das erspart doppelte Arbeit.
  3. Forke das Repository und arbeite in einem Branch.
  4. Öffne einen Pull Request, der die Issue-Nummer nennt.

Rechercherichtung

Beginne mit OAuthClientProvider.prepare_token_auth in src/mcp/client/auth/oauth2.py und test_basic_auth_token_exchange. Lies ClientAuthenticator.authenticate_request in src/mcp/server/auth/middleware/client_auth.py, die Request-Modelle in src/mcp/server/auth/handlers/token.py und Issue #1847; abgeschlossen ist die Aufgabe, wenn die Änderungen am Client und die entsprechenden Änderungen am Server ausreichen, damit die OAuth-Interaktionstests bestehen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python
Bereich
api, authentication
Issue-Typ
Bug
Schwierigkeit
4/5
Geschätzter Aufwand
3-5 Tage
Aktivitätsstatus
Ruhig
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
48/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.