Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 55 additions & 6 deletions src/mcp/client/auth/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,52 @@
from mcp.shared.inbound import MCP_PROTOCOL_VERSION_HEADER


def _split_www_authenticate_segments(header_value: str) -> list[str]:
"""Split a WWW-Authenticate header on top-level commas."""
segments: list[str] = []
current: list[str] = []
in_quotes = False

for char in header_value:
if char == '"':
in_quotes = not in_quotes
if char == "," and not in_quotes:
segment = "".join(current).strip()
if segment:
segments.append(segment)
current = []
continue
current.append(char)

tail = "".join(current).strip()
if tail:
segments.append(tail)
return segments


def _extract_bearer_auth_params(www_auth_header: str) -> str | None:
"""Return the auth-param portion of the first Bearer challenge."""
segments = _split_www_authenticate_segments(www_auth_header)
collecting = False
auth_params: list[str] = []

for segment in segments:
scheme, separator, remainder = segment.partition(" ")
if scheme.lower() == "bearer" and separator:
collecting = True
auth_params = [remainder.strip()]
continue
Comment on lines +50 to +53

@cubic-dev-ai cubic-dev-ai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When a WWW-Authenticate header contains more than one Bearer challenge, the parser currently keeps overwriting the collected auth params and ends up using the last Bearer challenge. That can make scope/resource metadata extraction come from a different challenge than intended. The overwrite happens because every Bearer ... segment reinitializes auth_params; stopping at the first Bearer challenge (or only initializing once) would keep behavior consistent with the function contract.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/auth/utils.py, line 50:

<comment>When a `WWW-Authenticate` header contains more than one Bearer challenge, the parser currently keeps overwriting the collected auth params and ends up using the last Bearer challenge. That can make scope/resource metadata extraction come from a different challenge than intended. The overwrite happens because every `Bearer ...` segment reinitializes `auth_params`; stopping at the first Bearer challenge (or only initializing once) would keep behavior consistent with the function contract.</comment>

<file context>
@@ -16,6 +16,52 @@
+
+    for segment in segments:
+        scheme, separator, remainder = segment.partition(" ")
+        if scheme.lower() == "bearer" and separator:
+            collecting = True
+            auth_params = [remainder.strip()]
</file context>
Suggested change
if scheme.lower() == "bearer" and separator:
collecting = True
auth_params = [remainder.strip()]
continue
if scheme.lower() == "bearer" and separator:
if collecting:
break
collecting = True
auth_params = [remainder.strip()]
continue
Fix with cubic


if collecting:
if separator and "=" not in scheme:
break
Comment on lines +56 to +57

@cubic-dev-ai cubic-dev-ai Bot Jul 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Bearer params after a valid auth-param that uses optional whitespace before = can be skipped, so fields later in the same challenge may return None. The break condition is classifying scope = "..."-style segments as a new challenge; consider excluding segments where the remainder starts with =.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/mcp/client/auth/utils.py, line 56:

<comment>Bearer params after a valid `auth-param` that uses optional whitespace before `=` can be skipped, so fields later in the same challenge may return `None`. The break condition is classifying `scope = "..."`-style segments as a new challenge; consider excluding segments where the remainder starts with `=`.</comment>

<file context>
@@ -16,6 +16,52 @@
+            continue
+
+        if collecting:
+            if separator and "=" not in scheme:
+                break
+            auth_params.append(segment)
</file context>
Suggested change
if separator and "=" not in scheme:
break
if separator and "=" not in scheme and not remainder.lstrip().startswith("="):
break
Fix with cubic

auth_params.append(segment)

if not auth_params:
return None
return ", ".join(part for part in auth_params if part)


def extract_field_from_www_auth(response: Response, field_name: str) -> str | None:
"""Extract field from WWW-Authenticate header.

Expand All @@ -26,13 +72,16 @@ def extract_field_from_www_auth(response: Response, field_name: str) -> str | No
if not www_auth_header:
return None

# Pattern matches: field_name="value" or field_name=value (unquoted)
pattern = rf'{field_name}=(?:"([^"]+)"|([^\s,]+))'
match = re.search(pattern, www_auth_header)
auth_params = _extract_bearer_auth_params(www_auth_header)
if auth_params is None:
return None

if match:
# Return quoted value if present, otherwise unquoted value
return match.group(1) or match.group(2)
# Match comma-delimited auth-params while respecting quoted values.
pattern = re.compile(r'(?:^|,\s*)(?P<name>[A-Za-z][A-Za-z0-9_-]*)=(?:"(?P<quoted>[^"]+)"|(?P<unquoted>[^,\s]+))')
for match in pattern.finditer(auth_params):
if match.group("name") == field_name:
# Return quoted value if present, otherwise unquoted value
return match.group("quoted") or match.group("unquoted")

return None

Expand Down
17 changes: 17 additions & 0 deletions tests/client/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -2016,6 +2016,17 @@ class TestWWWAuthenticate:
"resource_metadata",
"https://api.example.com/auth/metadata?version=1",
),
('Bearer error_scope="decoy", scope="read write"', "scope", "read write"),
(
'Bearer error_description="missing scope=write permission", scope="read write"',
"scope",
"read write",
),
(
'Basic realm="legacy", Bearer scope="read write", error="insufficient_scope"',
"scope",
"read write",
),
],
)
def test_extract_field_from_www_auth_valid_cases(
Expand Down Expand Up @@ -2047,6 +2058,12 @@ def test_extract_field_from_www_auth_valid_cases(
# Header without requested field
('Bearer realm="api", error="insufficient_scope"', "scope", "no scope parameter"),
('Bearer realm="api", scope="read write"', "resource_metadata", "no resource_metadata parameter"),
('Bearer custom_scope="leaked"', "scope", "field name appears only as a substring"),
(
'Bearer x_resource_metadata="https://decoy.example.com"',
"resource_metadata",
"field name appears only as a substring",
),
# Malformed field (empty value)
("Bearer scope=", "scope", "malformed scope parameter"),
("Bearer resource_metadata=", "resource_metadata", "malformed resource_metadata parameter"),
Expand Down
Loading