-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fix substring matches in WWW-Authenticate parsing #3041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Whning0513
wants to merge
7
commits into
modelcontextprotocol:main
Choose a base branch
from
Whning0513:fix-www-auth-substring-match-3009
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+191
−9
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
5de906e
Fix substring matches in WWW-Authenticate parsing
Whning0513 8a05814
Avoid matching auth params inside quoted values
Whning0513 d725212
Handle Bearer auth params in multi-challenge headers
Whning0513 3e99ffe
Handle more Bearer auth-param edge cases
Whning0513 d4cfbc5
Address WWW-Authenticate parser review feedback
Whning0513 b5df93c
Merge branch 'main' into fix-www-auth-substring-match-3009
Whning0513 397896e
Use current HTTP client name in auth tests
Whning0513 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,88 @@ | |||||
| 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 | ||||||
| escaped = False | ||||||
|
|
||||||
| for char in header_value: | ||||||
| if escaped: | ||||||
| escaped = False | ||||||
| elif char == "\\" and in_quotes: | ||||||
| escaped = True | ||||||
| elif 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: | ||||||
| if collecting: | ||||||
| break | ||||||
| collecting = True | ||||||
| auth_params = [remainder.strip()] | ||||||
| continue | ||||||
|
|
||||||
| if collecting: | ||||||
| if separator and "=" not in scheme and not remainder.lstrip().startswith("="): | ||||||
| break | ||||||
| auth_params.append(segment) | ||||||
|
|
||||||
| if not auth_params: | ||||||
| return None | ||||||
| return ", ".join(part for part in auth_params if part) | ||||||
|
|
||||||
|
|
||||||
| _AUTH_PARAM_PATTERN = re.compile( | ||||||
| r"(?:^|,\s*)(?P<name>[A-Za-z][A-Za-z0-9_-]*)\s*=\s*" | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: Generic auth-param lookup no longer recognizes valid parameter names such as Prompt for AI agents
Suggested change
|
||||||
| r'(?:"(?P<quoted>(?:\\.|[^"\\])*)"|(?P<unquoted>[^,\s]+))' | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def _extract_auth_param(auth_params: str, field_name: str) -> str | None: | ||||||
| """Extract an auth-param from a comma-delimited parameter string.""" | ||||||
| for match in _AUTH_PARAM_PATTERN.finditer(auth_params): | ||||||
| if match.group("name") != field_name: | ||||||
| continue | ||||||
| quoted = match.group("quoted") | ||||||
| value = quoted if quoted is not None else match.group("unquoted") | ||||||
| if not value: | ||||||
| return None | ||||||
| return re.sub(r"\\(.)", r"\1", value) if quoted is not None else value | ||||||
| return None | ||||||
|
|
||||||
|
|
||||||
| def _extract_field_from_bearer_challenge(response: Response, field_name: str) -> str | None: | ||||||
| """Extract an auth-param from the first Bearer challenge.""" | ||||||
| www_auth_header = response.headers.get("WWW-Authenticate") | ||||||
| if not www_auth_header: | ||||||
| return None | ||||||
|
|
||||||
| auth_params = _extract_bearer_auth_params(www_auth_header) | ||||||
| return _extract_auth_param(auth_params, field_name) if auth_params is not None else None | ||||||
|
|
||||||
|
|
||||||
| def extract_field_from_www_auth(response: Response, field_name: str) -> str | None: | ||||||
| """Extract field from WWW-Authenticate header. | ||||||
|
|
||||||
|
|
@@ -26,13 +108,11 @@ 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) | ||||||
|
|
||||||
| if match: | ||||||
| # Return quoted value if present, otherwise unquoted value | ||||||
| return match.group(1) or match.group(2) | ||||||
| for segment in _split_www_authenticate_segments(www_auth_header): | ||||||
| scheme, separator, remainder = segment.partition(" ") | ||||||
| auth_params = remainder.strip() if separator and "=" not in scheme else segment | ||||||
| if value := _extract_auth_param(auth_params, field_name): | ||||||
| return value | ||||||
|
|
||||||
| return None | ||||||
|
|
||||||
|
|
@@ -43,7 +123,7 @@ def extract_scope_from_www_auth(response: Response) -> str | None: | |||||
| Returns: | ||||||
| Scope string if found in WWW-Authenticate header, None otherwise | ||||||
| """ | ||||||
| return extract_field_from_www_auth(response, "scope") | ||||||
| return _extract_field_from_bearer_challenge(response, "scope") | ||||||
|
|
||||||
|
|
||||||
| def extract_resource_metadata_from_www_auth(response: Response) -> str | None: | ||||||
|
|
@@ -55,7 +135,7 @@ def extract_resource_metadata_from_www_auth(response: Response) -> str | None: | |||||
| if not response or response.status_code != 401: | ||||||
| return None # pragma: no cover | ||||||
|
|
||||||
| return extract_field_from_www_auth(response, "resource_metadata") | ||||||
| return _extract_field_from_bearer_challenge(response, "resource_metadata") | ||||||
|
|
||||||
|
|
||||||
| def build_protected_resource_metadata_discovery_urls(www_auth_url: str | None, server_url: str) -> list[str]: | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.