modelcontextprotocol / modelcontextprotocol/python-sdk
OAuth client: authorization URL is built with a second `?` when the advertised `authorization_endpoint` already carries a query (RFC 6749 §3.1)
まだ誰も着手していません。
- 主要言語
- Python
- スター
- 24.3k
- フォーク
- 4k
- 平均マージ
- 1日 1時間
- マージ済み PR(30日)
- 31
説明
Initial Checks
- I confirm that I'm using the newest release of my line (verified on 2.2.0 and 1.30.0, and on
main) - I confirm that I searched for my issue in the issues before opening this one (searched for "authorization_endpoint query", "authorization_url urlencode", "second ?")
Release line
v2 (and v1 — same code)
Description
OAuthClientProvider._perform_authorization builds the browser redirect as
authorization_url = f"{auth_endpoint}?{urlencode(auth_params)}" # src/mcp/client/auth/oauth2.py:427 on main
auth_endpoint comes straight from the server's RFC 8414 metadata (authorization_endpoint). RFC 6749 §3.1 says that URI "MAY include an application/x-www-form-urlencoded formatted query component, which MUST be retained when adding additional query parameters". When it does carry one, the f-string produces a second ?:
advertised: https://auth.example.com/authorize?tenant=acme
sent: https://auth.example.com/authorize?tenant=acme?response_type=code&client_id=…&redirect_uri=…&state=…&code_challenge=…
The authorization server then receives tenant = "acme?response_type=code" and no response_type at all — a hard failure at the consent page, on every authorization, for every server whose endpoint carries a query. Servers do advertise such endpoints: a tenant/policy selector (Azure AD B2C's ?p=<policy> is the well-known one), or — how we hit it — an environment/tier tag on a multi-tenant consent app (Nevermined advertises https://nevermined.app/oauth/authorize?network=sandbox|live because one consent app fronts two authorization servers). The TypeScript SDK is unaffected: client/auth.js builds the URL with new URL(endpoint) + searchParams.set(...), which retains the existing query.
Example Code
Minimal reproduction of the URL construction (no server needed):
from urllib.parse import urlencode
auth_endpoint = "https://auth.example.com/authorize?tenant=acme" # from RFC 8414 metadata
auth_params = {"response_type": "code", "client_id": "c", "state": "s"}
print(f"{auth_endpoint}?{urlencode(auth_params)}")
# https://auth.example.com/authorize?tenant=acme?response_type=code&client_id=c&state=s
# ^ second '?' — the server sees tenant="acme?response_type=code"
Expected (RFC 6749 §3.1):
https://auth.example.com/authorize?tenant=acme&response_type=code&client_id=c&state=s
Proposed fix — merge onto the existing query instead of concatenating:
from urllib.parse import parse_qsl, urlencode, urlsplit, urlunsplit
def build_authorization_url(authorization_endpoint: str, params: dict[str, str]) -> str:
parts = urlsplit(authorization_endpoint)
query = parse_qsl(parts.query, keep_blank_values=True) + list(params.items())
return urlunsplit(parts._replace(query=urlencode(query)))
I have this change ready on a branch — https://github.com/r-marques/python-sdk/tree/fix/authorization-url-retains-endpoint-query — as a small PR (helper + two unit tests + one flow test that drives _perform_authorization with a query-bearing authorization_endpoint; uv run pytest tests/client/test_auth.py → 163 passed / 1 xfailed, ruff + pyright clean) and would be glad to open it if you'd like to take an outside PR for this — happy to defer to a maintainer fix otherwise.
Disclosure: drafted with AI assistance (Claude Code); the behaviour was verified by hand against the 1.30.0 and 2.2.0 wheels and main, and I can explain every line of the proposed change.
Python & MCP Python SDK
Python 3.14.7
mcp 2.2.0 (also reproduced on 1.30.0; the line is unchanged on main @ oauth2.py:427)
コントリビューションガイド
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
調査の方向性
src/mcp/client/auth/oauth2.py:427 から始めて、_perform_authorization がどのように URL を構築するかを確認します。tests/client/test_auth.py を読み、その pytest コマンドを実行し、すでに query を持つ authorization_endpoint をカバーしてください。既存の query が保持され、認可パラメーターが正しく追加され、ruff と pyright も引き続きクリーンであれば完了です。
索引モデルが issue の本文から書いたものです。
評価
- 技術スタック
- python
- 領域
- authentication
- issue の種類
- バグ
- 難易度
- 2/5
- 見積もり時間
- 1〜3時間
- 活発さ
- 活発
- 明瞭さ
- 明確に書かれている
- 初心者へのやさしさ
- 35/100