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)

Open
#3,505 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

v1 v2
Dominant language
Python
Stars
24.3k
Forks
4k
Avg merge
1d 1h
Merged PRs (30d)
31

Description

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)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at src/mcp/client/auth/oauth2.py:427 and inspect how _perform_authorization constructs its URL. Read tests/client/test_auth.py, run its pytest command, and cover an authorization_endpoint that already has a query; done means the existing query is retained and the authorization parameters are added correctly, with ruff and pyright still clean.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
authentication
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.