modelcontextprotocol / modelcontextprotocol/python-sdk
OAuth client sends client_id in token body under client_secret_basic (strict servers reject as multiple auth methods)
まだ誰も着手していません。
- 主要言語
- Python
- スター
- 24.3k
- フォーク
- 4k
- 平均マージ
- 1日 1時間
- マージ済み PR(30日)
- 31
説明
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:
ClientAuthenticator.authenticate_request(src/mcp/server/auth/middleware/client_auth.py) requiresclient_idin the body.TokenHandler's request models (AuthorizationCodeRequest/RefreshTokenRequest,src/mcp/server/auth/handlers/token.py) declareclient_id: stras required — a missing value returns a Pydantic400 invalid_request: client_id: Field required.- 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
mainand running the client + interaction OAuth suites (green). The only residual failures were #1847's own error-message test updates, which appear stranded by atests/server/fastmcp→tests/server/mcpserverdirectory rename onmain(i.e. #1847 looks like it needs a rebase) — unrelated to the client change.
Questions for maintainers:
- How would you like the client change sequenced against #1847 — folded into #1847, or as a linked follow-up PR that merges after it?
- Which branch(es) —
main(v2), and/or av1.xbackport? 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.
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
src/mcp/client/auth/oauth2.py の OAuthClientProvider.prepare_token_auth と test_basic_auth_token_exchange から始めます。src/mcp/server/auth/middleware/client_auth.py の ClientAuthenticator.authenticate_request、src/mcp/server/auth/handlers/token.py のリクエストモデル、および issue #1847 を読みます。クライアント側の変更と対応するサーバー側の変更によって OAuth インタラクションテストに合格すれば完了です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- python
- 領域
- api, authentication
- issue の種類
- バグ
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 活発さ
- 静か
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 48/100