Repository navigation
OAuth client: authorization URL is built with a second ? when the advertised authorization_endpoint already carries a query (RFC 6749 §3.1) #3505
Description
Activity
- addedv2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)v1Affects the v1.x maintenance lineAffects the v1.x maintenance line
on Sep 15, 2026 Confirmed on main. The f-string at oauth2.py:427 blindly appends ? without checking if auth_endpoint already has a query component.
Fix is to use urlsplit/parse_qsl/urlunsplit to merge the params. TypeScript SDK already does this correctly via new URL(endpoint) + searchParams.set(...).
I'd be glad to open a PR if you're taking outside contributions for this — otherwise happy to defer.
Confirmed on current
main(6affe5c):src/mcp/client/auth/oauth2.py:427still doesauthorization_url = f"{auth_endpoint}?{urlencode(auth_params)}"
so a query-bearing
authorization_endpoint(which RFC 6749 §3.1 requires the client to retain) gets a second?, and the AS parses e.g.tenant=acme?response_type=code. Reproduced by driving_perform_authorization_code_grantwithauthorization_endpoint = https://auth.example.com/authorize?tenant=acme.Fix plan:
- Module-level helper in
oauth2.py:
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)))
-
Line 427 becomes
build_authorization_url(auth_endpoint, auth_params). Plain endpoints render byte-identically to today; query-bearing ones get the new params merged with&, existing params first. -
Tests in
tests/client/test_auth.py: unit tests for the helper (plain endpoint unchanged, existing query retained, blank values kept), plus a flow test driving_perform_authorization_code_grantwith a query-bearing endpoint that asserts the captured redirect retainstenant=acmeand carriesresponse_type=codebehind a single?.
Same root cause as #2776 (with the stalled #2779) — happy for this to be deduped either way; the patch is identical. If you'd take an outside PR for it, please assign and I'll open it against
mainright away (av1.xbackport too if wanted).Affiliation: thyn-ai. Drafted with AI assistance; verified against
mainby hand, and I can explain every line of the change.- Module-level helper in
We hit this in production against Salesforce's authorization server, whose discovered
authorization_endpointcarries?prompt=select_account— the resulting double-?URL breaks the flow, and we currently work around it with a subclass override. I have a minimal fix ready against currentmain(a_build_authorization_urlhelper that merges the flow's params into the endpoint's existing query per RFC 6749 §3.1, plus a regression test) — happy to have a maintainer assign this so the PR stays open. Noting #2779 takes the same approach but predates the httpx2 rename and no longer applies cleanly.🤖 Generated with Claude Code
Same issue here with Datadog subdomain query
When using Datadog Custom Domain OAuth , the expected subdomain should be passed via
?subdomain=<SUBDOMAIN>In
v1implementation, the bug comes frommcp.client.auth.utils:build_protected_resource_metadata_discovery_urlsfunction that the Priority 2 only preserves the Path and remove the queries like below:def build_protected_resource_metadata_discovery_urls(www_auth_url: str | None, server_url: str) -> list[str]: ... # Priority 2: Path-based well-known URI (if server has a path component) if parsed.path and parsed.path != "/": - path_based_url = urljoin(base_url, f"/.well-known/oauth-protected-resource{parsed.path}") + path_based_url = urlunparse(parsed._replace(path=f"/.well-known/oauth-protected-resource{parsed.path}")) urls.append(path_based_url) ...Kludex commented
on Oct 10, 2026 MemberMore actionsBoth reports identify the OAuth authorization URL appending a second
?when the advertised endpoint already has query parameters. This is tracked in #2776, so I’m closing this as a duplicate. AI-assisted triage; I reviewed both reports.
Initial Checks
main)Release line
v2 (and v1 — same code)
Description
OAuthClientProvider._perform_authorizationbuilds the browser redirect asauth_endpointcomes 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?:The authorization server then receives
tenant = "acme?response_type=code"and noresponse_typeat 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 advertiseshttps://nevermined.app/oauth/authorize?network=sandbox|livebecause one consent app fronts two authorization servers). The TypeScript SDK is unaffected:client/auth.jsbuilds the URL withnew URL(endpoint)+searchParams.set(...), which retains the existing query.Example Code
Minimal reproduction of the URL construction (no server needed):
Expected (RFC 6749 §3.1):
Proposed fix — merge onto the existing query instead of concatenating:
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_authorizationwith a query-bearingauthorization_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