modelcontextprotocol / modelcontextprotocol/python-sdk

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

オープン
#3,138 コメント 6 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

auth bug needs confirmation P1 v1 v2
主要言語
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:

  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.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. 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

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。