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)

Ouverte
#3,505 2 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

v1 v2
Langage dominant
Python
Étoiles
24.3k
Forks
4k
Merge moyen
1 j 1 h
PR mergées (30 j)
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)

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par src/mcp/client/auth/oauth2.py:427 et examinez comment _perform_authorization construit son URL. Lisez tests/client/test_auth.py, exécutez sa commande pytest et couvrez le cas d’un authorization_endpoint qui contient déjà une query ; c’est terminé lorsque la query existante est conservée et que les paramètres d’autorisation sont ajoutés correctement, tout en gardant ruff et pyright propres.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
python
Domaine
authentication
Type d'issue
Bug
Difficulté
2/5
Temps estimé
1-3 heures
Activité
Active
Clarté
Clairement spécifiée
Accessibilité débutants
35/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.