modelcontextprotocol / modelcontextprotocol/python-sdk

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

Đang mở
#3,138 6 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

auth bug needs confirmation P1 v1 v2
Ngôn ngữ chính
Python
Star
24.3k
Fork
4k
Merge trung bình
1 ngày 1 giờ
Pull request đã merge (30 ngày)
31

Mô tả

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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Hướng nghiên cứu

Bắt đầu với OAuthClientProvider.prepare_token_auth trong src/mcp/client/auth/oauth2.py và test_basic_auth_token_exchange. Đọc ClientAuthenticator.authenticate_request trong src/mcp/server/auth/middleware/client_auth.py, các model request trong src/mcp/server/auth/handlers/token.py và issue #1847; hoàn tất khi các thay đổi tương ứng ở client và server giúp các bài kiểm thử tương tác OAuth vượt qua.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
python
Lĩnh vực
api, authentication
Loại issue
Lỗi
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.