| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,41 @@ | ||
| """Utilities for OAuth 2.0 Resource Indicators (RFC 8707) and PKCE (RFC 7636).""" | ||
|
|
||
| import time | ||
| from urllib.parse import urlparse, urlsplit, urlunsplit | ||
| from urllib.parse import urlsplit, urlunsplit | ||
|
|
||
| from pydantic import AnyUrl, HttpUrl | ||
|
|
||
| # WHATWG URL treats these percent-encoded spellings as dot-segments too. | ||
| _SINGLE_DOT_SEGMENTS = {".", "%2e"} | ||
| _DOUBLE_DOT_SEGMENTS = {"..", ".%2e", "%2e.", "%2e%2e"} | ||
|
|
||
|
|
||
| def _remove_dot_segments(path: str) -> str: | ||
| """Resolve "." and ".." segments in a URL path (RFC 3986 section 5.2.4).""" | ||
| segments = path.split("/") | ||
| output: list[str] = [] | ||
| for index, segment in enumerate(segments): | ||
| is_last = index == len(segments) - 1 | ||
| kind = segment.lower() | ||
| if kind in _DOUBLE_DOT_SEGMENTS: | ||
| if len(output) > 1: | ||
| output.pop() | ||
| if is_last: | ||
| output.append("") | ||
| elif kind in _SINGLE_DOT_SEGMENTS: | ||
| if is_last: | ||
| output.append("") | ||
| else: | ||
| output.append(segment) | ||
| return "/".join(output) | ||
|
|
||
|
|
||
| def resource_url_from_server_url(url: str | HttpUrl | AnyUrl) -> str: | ||
| """Convert server URL to canonical resource URL per RFC 8707. | ||
|
|
||
| RFC 8707 section 2 states that resource URIs "MUST NOT include a fragment component". | ||
| Returns absolute URI with lowercase scheme/host for canonical form. | ||
| Returns absolute URI with lowercase scheme/host and dot-segments resolved, so the | ||
| resource identifies the same location an HTTP client would actually request. | ||
|
|
||
| Args: | ||
| url: Server URL to convert | ||
| Expand All | @@ -23,7 +48,14 @@ | |
|
|
||
| # Parse the URL and remove fragment, create canonical form | ||
| parsed = urlsplit(url_str) | ||
| canonical = urlunsplit(parsed._replace(scheme=parsed.scheme.lower(), netloc=parsed.netloc.lower(), fragment="")) | ||
| canonical = urlunsplit( | ||
| parsed._replace( | ||
| scheme=parsed.scheme.lower(), | ||
| netloc=parsed.netloc.lower(), | ||
| path=_remove_dot_segments(parsed.path), | ||
|
Check notice on line 55 in src/mcp/shared/auth_utils.py
|
||
| fragment="", | ||
| ) | ||
| ) | ||
|
|
||
| return canonical | ||
|
|
||
| Expand All | @@ -34,7 +66,8 @@ | |
| A requested resource matches if it has the same scheme, domain, port, | ||
| and its path starts with the configured resource's path. This allows | ||
| hierarchical matching where a token for a parent resource can be used | ||
| for child resources. | ||
| for child resources. Dot-segments in either path are resolved before | ||
| comparing. | ||
|
|
||
| Args: | ||
| requested_resource: The resource URL being requested | ||
| Expand All | @@ -44,17 +77,17 @@ | |
| True if the requested resource matches the configured resource | ||
| """ | ||
| # Parse both URLs | ||
| requested = urlparse(requested_resource) | ||
| configured = urlparse(configured_resource) | ||
| requested = urlsplit(requested_resource) | ||
| configured = urlsplit(configured_resource) | ||
|
|
||
| # Compare scheme, host, and port (origin) | ||
| if requested.scheme.lower() != configured.scheme.lower() or requested.netloc.lower() != configured.netloc.lower(): | ||
|
Check notice on line 84 in src/mcp/shared/auth_utils.py
|
||
|
Comment thread
Copy link
Copy Markdown
Contributor
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. Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟣 Pre-existing: default-port normalization asymmetry — the PRM resource side is WHATWG-normalized by pydantic (explicit :443/:80 is stripped, e.g. AnyHttpUrl("https://host:443/mcp") serializes as https://host/mcp), but resource_url_from_server_url() (netloc kept verbatim at src/mcp/shared/auth_utils.py:54) and the netloc equality check in check_resource_allowed() (line 84) keep the explicit default port from the raw server_url string. The PR's stated goal is making both operands of check_resource_allowed agree on normalization (it fixed dot-segments for exactly this reason, per the committed property test asserting agreement with pydantic's WHATWG parser), but the default-port half of WHATWG normalization is still missing, so "host:443" != "host" fails the origin… Extended reasoning...A user configures OAuthClientProvider(server_url="https://api.example.com:443/mcp") — a spelling httpx treats as identical to the portless URL. The server's protected-resource metadata advertises resource "https://api.example.com/mcp" (or even "https://api.example.com:443/mcp" — pydantic strips the port when the client parses it either way, so str(prm.resource) is always portless). In _validate_resource_match (src/mcp/client/auth/oauth2.py:576-578), default_resource is "https://api.example.com:443/mcp" while prm_resource is "https://api.example.com/mcp"; the netloc comparison at auth_utils.py:84 returns False and the OAuth flow aborts with OAuthFlowError: Protected resource https://api.example.com/mcp does not match expected https://api.example.com:443/mcp, even though both strings name the same location. The same asymmetry makes get_resource_url() (oauth2.py:210) never adopt the PRM resource for such configs. Fix in one place: strip the scheme's default port (or otherwise WHATWG-normalize the netloc) when deriving/comparing, mirroring what was just done for dot-s Verification: pre_existing — The asymmetry is real: check_resource_allowed compares netloc strings verbatim (src/mcp/shared/auth_utils.py:84 requested.netloc.lower() != configured.netloc.lower()) and resource_url_from_server_url keeps the netloc as written (line 54), while the PRM side is resource: AnyHttpUrl (src/mcp/shared/auth.py:243), which pydantic v2 serializes with scheme-default ports stripped. The
Sorry, something went wrong.
claude[bot] reacted with thumbs up emoji
claude[bot] reacted with thumbs down emoji
All reactions
|
||
| return False | ||
|
|
||
| # Normalize trailing slashes before comparison so that | ||
| # "/foo" and "/foo/" are treated as equivalent. | ||
| requested_path = requested.path | ||
| configured_path = configured.path | ||
| requested_path = _remove_dot_segments(requested.path) | ||
| configured_path = _remove_dot_segments(configured.path) | ||
| if not requested_path.endswith("/"): | ||
| requested_path += "/" | ||
| if not configured_path.endswith("/"): | ||
| Expand Down | ||
| Back | FazBrowse Home | New Git URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality🟣 Pre-existing, left behind by this partial fix: the resource URL is now derived with dot-segments floored at the path root, but build_protected_resource_metadata_discovery_urls (src/mcp/client/auth/utils.py:90) still embeds the raw unresolved path into '/.well-known/oauth-protected-resource{path}' via urljoin, whose RFC 3986 resolution has no floor at the well-known prefix — a '..' that the new _remove_dot_segments correctly discards at root instead consumes the 'oauth-protected-resource' segment, so PRM discovery queries the wrong well-known URL for exactly the dot-segmented server_urls this PR sets out to handle (same pattern at utils.py:158 for AS metadata).
Extended reasoning...A user configures OAuthClientProvider with server_url='https://host/a/../../b/mcp'. After this PR, resource_url_from_server_url correctly derives 'https://host/b/mcp' (the second '..' is floored at root by _remove_dot_segments). But path-based PRM discovery builds urljoin('https://host', '/.well-known/oauth-protected-resource/a/../../b/mcp'); CPython's urljoin pops segments with a bare resolved_path.pop(), so the second '..' removes 'oauth-protected-resource' and the client fetches 'https://host/.well-known/b/mcp' — never the RFC 9728 location 'https://host/.well-known/oauth-protected-resource/b/mcp'. On a multi-tenant server that only serves path-based PRM, discovery 404s, falls back to the root-based well-known, and _validate_resource_match then raises OAuthFlowError (or the client silently adopts the broader root PRM resource), even though the derived resource identifier is now correct. Fix: resolve dot-segments in server_url's path (e.g. reuse _remove_dot_segments / resource_url_from_server_url) before constructing the well-known discovery URLs.
Verification: pre_existing — the mechanism is real, but the base branch fails identically by the same route through untouched code. The diff (git diff 0cee624..HEAD) touches only src/mcp/shared/auth_utils.py and two test files; src/mcp/client/auth/utils.py is unchanged. At src/mcp/client/auth/utils.py:89-91, path-based PRM discovery embeds the raw, unresolved server path: `path_based_url = urljoin(base_url, f"
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.