From c9db31d108d9cf54eb5b0e0f9f4a72d5b2cd60c2 Mon Sep 17 00:00:00 2001 From: David Gilman Date: Wed, 23 Sep 2026 08:43:10 -0400 Subject: [PATCH] fix(client/auth): tolerate comma-separated scope strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 6749 §3.3 separates scopes with spaces, but real-world authorization servers (e.g. Linear) return comma-separated scope lists in token responses and WWW-Authenticate challenges. Scope membership checks and the SEP-2350 step-up union split on whitespace only, so a comma-separated grant is treated as a single opaque scope: unions stop deduplicating and offline_access handling misfires. Add a parse_scopes helper accepting both separators and use it in union_scopes and the offline_access membership checks. Co-Authored-By: Claude Fable 5 --- src/mcp/client/auth/oauth2.py | 3 ++- src/mcp/client/auth/utils.py | 18 ++++++++++++++---- tests/client/test_auth.py | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/mcp/client/auth/oauth2.py b/src/mcp/client/auth/oauth2.py index 8588208924..0d590dfc39 100644 --- a/src/mcp/client/auth/oauth2.py +++ b/src/mcp/client/auth/oauth2.py @@ -31,6 +31,7 @@ extract_resource_metadata_from_www_auth, extract_scope_from_www_auth, get_client_metadata_scopes, + parse_scopes, handle_auth_metadata_response, handle_protected_resource_response, handle_registration_response, @@ -421,7 +422,7 @@ async def _perform_authorization_code_grant(self) -> tuple[str, str]: # OIDC requires prompt=consent when offline_access is requested # https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess - if "offline_access" in self.context.client_metadata.scope.split(): + if "offline_access" in parse_scopes(self.context.client_metadata.scope): auth_params["prompt"] = "consent" authorization_url = f"{auth_endpoint}?{urlencode(auth_params)}" diff --git a/src/mcp/client/auth/utils.py b/src/mcp/client/auth/utils.py index 0a2ba80aec..e393f7ce9e 100644 --- a/src/mcp/client/auth/utils.py +++ b/src/mcp/client/auth/utils.py @@ -40,6 +40,16 @@ def extract_field_from_www_auth(response: Response, field_name: str) -> str | No return None +# RFC 6749 §3.3 separates scopes with spaces, but some authorization servers return +# comma-separated lists; accept both so a compliant client is not broken by the server's choice. +_SCOPE_SEPARATOR_RE = re.compile(r"[\s,]+") + + +def parse_scopes(scope: str) -> list[str]: + """Split a scope string into individual scopes, accepting space and comma separators.""" + return [item for item in _SCOPE_SEPARATOR_RE.split(scope.strip()) if item] + + def extract_scope_from_www_auth(response: Response) -> str | None: """Extract scope parameter from WWW-Authenticate header as per RFC 6750. @@ -127,7 +137,7 @@ def get_client_metadata_scopes( and "offline_access" in authorization_server_metadata.scopes_supported and client_grant_types is not None and "refresh_token" in client_grant_types - and "offline_access" not in selected_scope.split() + and "offline_access" not in parse_scopes(selected_scope) ): selected_scope = f"{selected_scope} offline_access" @@ -135,7 +145,7 @@ def get_client_metadata_scopes( def union_scopes(previous_scope: str | None, new_scope: str | None) -> str | None: - """Merge two space-delimited scope strings, preserving order and dropping duplicates. + """Merge two scope strings, preserving order and dropping duplicates. SEP-2350: on step-up re-authorization the client requests the union of previously requested scopes and the newly challenged scopes, so escalating one operation does not drop the @@ -147,9 +157,9 @@ def union_scopes(previous_scope: str | None, new_scope: str | None) -> str | Non if not new_scope: return previous_scope - merged = previous_scope.split() + merged = parse_scopes(previous_scope) seen = set(merged) - for scope in new_scope.split(): + for scope in parse_scopes(new_scope): if scope not in seen: merged.append(scope) seen.add(scope) diff --git a/tests/client/test_auth.py b/tests/client/test_auth.py index 18a1566705..3fe78236a7 100644 --- a/tests/client/test_auth.py +++ b/tests/client/test_auth.py @@ -28,6 +28,7 @@ handle_auth_metadata_response, handle_registration_response, is_valid_client_metadata_url, + parse_scopes, should_use_client_metadata_url, union_scopes, validate_authorization_response_iss, @@ -3066,6 +3067,7 @@ def test_validate_metadata_issuer_rejects_mismatch(): pytest.param(None, "mcp:write", "mcp:write", id="no-previous"), pytest.param("mcp:basic", None, "mcp:basic", id="no-new"), pytest.param(None, None, None, id="both-empty"), + pytest.param("read,write", "write, admin", "read write admin", id="comma-separated-normalized"), ], ) def test_union_scopes(previous: str | None, new: str | None, expected: str | None): @@ -3073,6 +3075,21 @@ def test_union_scopes(previous: str | None, new: str | None, expected: str | Non assert union_scopes(previous, new) == expected +@pytest.mark.parametrize( + ("scope", "expected"), + [ + pytest.param("read write", ["read", "write"], id="space-separated"), + pytest.param("read,write", ["read", "write"], id="comma-separated"), + pytest.param("read, write,\tadmin", ["read", "write", "admin"], id="mixed-separators"), + pytest.param(" read ", ["read"], id="surrounding-whitespace"), + pytest.param("", [], id="empty"), + ], +) +def test_parse_scopes(scope: str, expected: list[str]): + """RFC 6749 §3.3 separates scopes with spaces, but some servers use commas; accept both.""" + assert parse_scopes(scope) == expected + + def test_credentials_match_issuer_same_issuer(): info = OAuthClientInformationFull(client_id="c", redirect_uris=[AnyUrl("http://localhost/cb")], issuer="https://as") assert credentials_match_issuer(info, "https://as", None) is True