Skip to content

Commit 7219d8c

Browse files
committed
fix(client): preserve multi-value existing query params in auth URL
Address review feedback: dict(parse_qsl(...)) collapsed duplicate query keys on the server-advertised authorization_endpoint to the last value. Build the merged query from an ordered pair list instead, dropping only keys the flow overrides, so existing multi-value params (e.g. scope=a&scope=b) survive. Adds a regression test.
1 parent 79259a6 commit 7219d8c

2 files changed

Lines changed: 21 additions & 3 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,18 @@ def _build_authorization_url(auth_endpoint: str, auth_params: Mapping[str, str |
6767
Naively appending ``?<params>`` would produce an invalid URL with two ``?``
6868
separators, so the existing query is parsed and merged with ``auth_params``.
6969
Flow-generated params take precedence on key conflicts; ``None`` values are
70-
dropped rather than serialized as the literal string ``"None"``.
70+
dropped rather than serialized as the literal string ``"None"``. Existing
71+
multi-value query params (e.g. ``?scope=a&scope=b``) are preserved rather
72+
than collapsed, except for keys that the flow overrides.
7173
"""
7274
parsed = urlparse(auth_endpoint)
73-
merged_params = dict(parse_qsl(parsed.query, keep_blank_values=True))
74-
merged_params.update({key: value for key, value in auth_params.items() if value is not None})
75+
flow_params = {key: value for key, value in auth_params.items() if value is not None}
76+
# Keep existing endpoint params (including duplicate keys) except those the
77+
# flow overrides, then append the authoritative flow params.
78+
existing = [
79+
(key, value) for key, value in parse_qsl(parsed.query, keep_blank_values=True) if key not in flow_params
80+
]
81+
merged_params = existing + list(flow_params.items())
7582
return urlunparse(parsed._replace(query=urlencode(merged_params)))
7683

7784

tests/client/test_auth.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3213,6 +3213,17 @@ def test_build_authorization_url_flow_params_win_on_conflict(self):
32133213
params = parse_qs(urlparse(url).query)
32143214
assert params["response_type"] == ["code"]
32153215

3216+
def test_build_authorization_url_preserves_multi_value_existing_query(self):
3217+
"""Duplicate keys on the endpoint query are preserved, not collapsed."""
3218+
url = _build_authorization_url(
3219+
"https://auth.example.com/authorize?scope=a&scope=b",
3220+
{"response_type": "code"},
3221+
)
3222+
params = parse_qs(urlparse(url).query)
3223+
assert params["scope"] == ["a", "b"]
3224+
assert params["response_type"] == ["code"]
3225+
assert url.count("?") == 1
3226+
32163227
@pytest.mark.anyio
32173228
async def test_perform_authorization_preserves_endpoint_query(self, oauth_provider: OAuthClientProvider):
32183229
"""End-to-end: redirect URL stays valid when the endpoint has a query string."""

0 commit comments

Comments
 (0)