modelcontextprotocol / modelcontextprotocol/python-sdk
OAuth client sends client_id in token body under client_secret_basic (strict servers reject as multiple auth methods)
还没有人认领这个 Issue。
- 主要语言
- Python
- 星标
- 24.3k
- 派生
- 4k
- 平均合并
- 1 天 1 小时
- 30 天内合并 PR
- 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 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 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