diff --git a/tests/tools/test_mcp_oauth_issuer_origin.py b/tests/tools/test_mcp_oauth_issuer_origin.py new file mode 100644 index 0000000000..d1d5e6fd7d --- /dev/null +++ b/tests/tools/test_mcp_oauth_issuer_origin.py @@ -0,0 +1,113 @@ +"""A path-scoped authorization server whose RFC 8414 metadata names its origin (#116233). + +Strava's MCP connector advertises ``authorization_servers: ["https://www.strava.com/mcp-issuer"]`` and +serves ``/.well-known/oauth-authorization-server/mcp-issuer`` with ``issuer: "https://www.strava.com"``. +The SDK's exact-string issuer check (RFC 8414 §3.3) rejected that document and discovery never completed. +Hermes accepts exactly this shape — the document fetched from the well-known URL derived from the advertised +identifier, naming that identifier's origin — through the real provider flow; every other mismatch is still +rejected. +""" +from __future__ import annotations + +import json +from urllib.parse import parse_qs, urlsplit + +import pytest + +pytest.importorskip("mcp.client.auth.oauth2") + +RESOURCE = "https://res.example/mcp" +AS_ORIGIN = "https://as.example" +ADVERTISED = f"{AS_ORIGIN}/mcp-issuer" +PATH_DOC = "/.well-known/oauth-authorization-server/mcp-issuer" + + +def _asm(issuer): + return {"issuer": issuer, "authorization_endpoint": f"{AS_ORIGIN}/authorize", "token_endpoint": f"{AS_ORIGIN}/token", + "registration_endpoint": f"{AS_ORIGIN}/register", "response_types_supported": ["code"], + "code_challenge_methods_supported": ["S256"], "authorization_response_iss_parameter_supported": True} + + +class _StandIn: + """Resource + authorization server behind one httpx MockTransport; ``issuer_doc`` maps ASM path -> document.""" + + def __init__(self, httpx, issuer_doc): + self.httpx, self.issuer_doc, self.hits = httpx, issuer_doc, [] + + def __call__(self, request): + url = str(request.url) + path = urlsplit(url).path + self.hits.append((request.method, url)) + j = lambda status, body, **h: self.httpx.Response(status, json=body, headers=h, request=request) # noqa: E731 + if url == RESOURCE: + if request.headers.get("Authorization") == "Bearer AT-1": + return j(200, {"ok": True}) + return j(401, {}, **{"WWW-Authenticate": 'Bearer resource_metadata="https://res.example/.well-known/oauth-protected-resource"'}) + if url == "https://res.example/.well-known/oauth-protected-resource": + return j(200, {"resource": RESOURCE, "authorization_servers": [ADVERTISED]}) + if url.startswith(AS_ORIGIN) and path in self.issuer_doc: + return j(200, self.issuer_doc[path]) + if url == f"{AS_ORIGIN}/register": + return j(201, {"client_id": "dcr-1", "redirect_uris": ["http://127.0.0.1:1/cb"], "token_endpoint_auth_method": "none", + "grant_types": ["authorization_code", "refresh_token"], "response_types": ["code"]}) + if url == f"{AS_ORIGIN}/token": + return j(200, {"access_token": "AT-1", "token_type": "Bearer", "expires_in": 3600, "refresh_token": "RT-1"}) + return j(404, {}) + + +async def _run_flow(tmp_path, monkeypatch, issuer_doc): + from mcp.shared.auth import OAuthClientMetadata + from pydantic import AnyUrl + + from tools.mcp_oauth import HermesTokenStorage, _authorization_code_result + from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests + from tools.mcp_tool import sdk_httpx + + httpx = sdk_httpx() + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + reset_manager_for_tests() + seen = {} + + async def redirect(url): + seen["authorize_url"] = url + seen["state"] = parse_qs(urlsplit(url).query)["state"][0] + + async def callback(): + return _authorization_code_result("code-1", seen["state"], iss=AS_ORIGIN) + + storage = HermesTokenStorage("srv") + provider = _HERMES_PROVIDER_CLS( + server_name="srv", server_url=RESOURCE, storage=storage, + client_metadata=OAuthClientMetadata(redirect_uris=[AnyUrl("http://127.0.0.1:1/cb")], client_name="Hermes Agent"), + redirect_handler=redirect, callback_handler=callback) + standin = _StandIn(httpx, issuer_doc) + async with httpx.AsyncClient(auth=provider, transport=httpx.MockTransport(standin)) as client: + response = await client.get(RESOURCE) + return response, standin, seen, provider + + +@pytest.mark.asyncio +async def test_origin_issued_document_of_path_scoped_server_completes_the_flow(tmp_path, monkeypatch): + response, standin, seen, provider = await _run_flow(tmp_path, monkeypatch, {PATH_DOC: _asm(AS_ORIGIN)}) + + assert response.status_code == 200 + assert seen["authorize_url"].startswith(f"{AS_ORIGIN}/authorize?") + assert ("POST", f"{AS_ORIGIN}/register") in standin.hits and ("POST", f"{AS_ORIGIN}/token") in standin.hits + assert str(provider.context.oauth_metadata.issuer).rstrip("/") == AS_ORIGIN + # SEP-2352 binding stays on the advertised identifier, so the next 401 reuses this client instead of re-registering. + assert json.loads((tmp_path / "mcp-tokens" / "srv.client.json").read_text())["issuer"] == ADVERTISED + assert json.loads((tmp_path / "mcp-tokens" / "srv.json").read_text())["access_token"] == "AT-1" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("issuer_doc", [ + pytest.param({PATH_DOC: _asm("https://other.example")}, id="different-origin"), + pytest.param({"/.well-known/oauth-authorization-server": _asm(AS_ORIGIN)}, id="root-document-only"), + pytest.param({PATH_DOC: _asm(f"{AS_ORIGIN}/other-path")}, id="different-path"), +]) +async def test_other_issuer_shapes_are_still_rejected(tmp_path, monkeypatch, issuer_doc): + from mcp.client.auth.exceptions import OAuthFlowError, OAuthRegistrationError + + with pytest.raises((OAuthFlowError, OAuthRegistrationError)): + await _run_flow(tmp_path, monkeypatch, issuer_doc) + assert not (tmp_path / "mcp-tokens" / "srv.json").exists() diff --git a/tools/mcp_oauth_device.py b/tools/mcp_oauth_device.py index 6a27b9584c..4b5e59df57 100644 --- a/tools/mcp_oauth_device.py +++ b/tools/mcp_oauth_device.py @@ -63,6 +63,8 @@ async def _device_metadata(client, server_url, auth_server_url): """Issuer-bound device metadata of one authorization server; raises when it is unusable.""" from mcp.client.auth.utils import build_oauth_authorization_server_metadata_discovery_urls, validate_metadata_issuer + from tools.mcp_oauth_provider import metadata_issued_by_origin + for url in build_oauth_authorization_server_metadata_discovery_urls(auth_server_url, server_url): response = await client.get(url) if response.status_code == 404: @@ -71,7 +73,7 @@ async def _device_metadata(client, server_url, auth_server_url): if not data.get("device_authorization_endpoint"): raise RuntimeError("Server does not advertise device authorization; use --flow browser if supported") metadata = DeviceOAuthMetadata.model_validate(data) - if auth_server_url: + if auth_server_url and not metadata_issued_by_origin(metadata, auth_server_url, response): validate_metadata_issuer(metadata, auth_server_url) grants = metadata.grant_types_supported if grants is not None and DEVICE_GRANT not in grants: @@ -110,7 +112,11 @@ async def _register(client, provider, cfg): raise RuntimeError("Server has no registration endpoint; configure oauth.client_id (and client_secret if required)") response = await client.post(str(endpoint), json=metadata) data = _payload(response, "Client registration") - data["issuer"] = str(context.oauth_metadata.issuer) + # SEP-2352: bind the credentials to the identifier the SDK's runtime flow compares them against — the + # advertised authorization server when the resource advertised one, else the metadata issuer (its Step 4 + # rule). For a path-scoped server whose document names its origin (#116233) the two differ, and a + # binding to the document issuer would make the next 401 discard this client and its tokens. + data["issuer"] = str(context.auth_server_url or context.oauth_metadata.issuer) context.client_info = OAuthClientInformationFull.model_validate(data) provider._coerce_client_secret_post() try: diff --git a/tools/mcp_oauth_provider.py b/tools/mcp_oauth_provider.py index 6a35edc7c9..6b9c4de6d3 100644 --- a/tools/mcp_oauth_provider.py +++ b/tools/mcp_oauth_provider.py @@ -122,6 +122,31 @@ class HermesProviderMixin: self.context.callback_handler = _fill_iss + async def _hermes_accept_origin_issued_metadata(self, response): + """Accept a path-scoped authorization server's metadata document whose ``issuer`` is the origin + it lives under (see ``metadata_issued_by_origin``); the SDK's exact-string check (RFC 8414 §3.3) + would reject it and park the connection on an issuer mismatch (Strava, #116233). + + The SDK validates inside its Step 2 loop right after reading the response, so the document is + installed on the context here and the SDK is handed an empty 204: ``handle_auth_metadata_response`` + reads that as "stop trying", leaving the installed document in place. ``auth_server_url`` is left + untouched, so the SEP-2352 credential binding still uses the advertised identifier (stable across + runs), while the RFC 9207 ``iss`` check and Hermes' refresh-token binding use the document's issuer. + Every other response goes back to the SDK unchanged, including its issuer check.""" + from mcp.shared.auth import OAuthMetadata + from pydantic import ValidationError + try: + metadata = OAuthMetadata.model_validate_json(await response.aread()) + except ValidationError: + return response + if not metadata_issued_by_origin(metadata, self.context.auth_server_url, response): + return response + self._hermes_logger.info( + "MCP OAuth: accepting authorization-server metadata from %s whose issuer %s is the origin of the " + "advertised server %s", response.url, metadata.issuer, self.context.auth_server_url) + self.context.oauth_metadata = metadata + return type(response)(204, request=response.request) + def _prepare_token_request(self, request): """Stamp the configured User-Agent onto a token/refresh request.""" ua = getattr(self, "_hermes_token_user_agent", None) # tests build via __new__ @@ -205,6 +230,8 @@ class HermesProviderMixin: failure = _asm_discovery_failure(sent) if failure: discovery_failures.append(failure) + elif getattr(sent, "status_code", None) == 200: + sent = await self._hermes_accept_origin_issued_metadata(sent) finally: await self._hermes_release_refresh_fence() @@ -441,6 +468,35 @@ def _metadata_issuer(context: Any) -> str | None: return (str(issuer).rstrip("/") or None) if issuer else None +def metadata_issued_by_origin(metadata: Any, auth_server_url: str | None, response: Any) -> bool: + """Whether *metadata* may stand in for the exact-issuer match of RFC 8414 §3.3 because it is the + document of the path-scoped authorization server *auth_server_url* and names that server's origin. + + The issuer check stops a party controlling a path or a sibling host from making the client accept + endpoints of a different authorization server (RFC 8414 §3.3, RFC 9728 §3.3). This narrow shape keeps + that boundary: *response* must be the document fetched directly (no redirect) from the RFC 8414 §3.1 + well-known URL DERIVED from the advertised identifier, ``/.well-known/oauth-authorization-server + `` — a location only the origin's operator controls — and its ``issuer`` must be exactly that + origin, i.e. the advertised server is ``issuer + path``. ``response.url`` is the URL the body was + actually read from (the final request after any followed redirect), so a redirected document never + matches. Whoever can publish that document already + controls the origin's well-known tree, so accepting it grants a path-controlling attacker nothing. + Strava's MCP connector publishes exactly this pair (#116233). Anything else (another origin, a + different path, the root or OIDC fallback documents, a redirect target) still goes through the + exact-string check.""" + from urllib.parse import urlsplit + if not auth_server_url: + return False + parts = urlsplit(auth_server_url) + path = parts.path.rstrip("/") + if (not path or ".." in path.split("/") or parts.username is not None or parts.query or parts.fragment + or response.status_code != 200): + return False + origin = f"{parts.scheme}://{parts.netloc}" + derived = f"{origin}/.well-known/oauth-authorization-server{path}" + return str(response.url) == derived and str(metadata.issuer).rstrip("/") == origin + + def bind_issuer_from_context(context: Any) -> None: """Record the discovered issuer so the next ``storage.set_tokens`` (exchange or refresh) carries it. No-op when metadata is not discovered yet or storage is not Hermes'.""" diff --git a/website/docs/reference/mcp-config-reference.md b/website/docs/reference/mcp-config-reference.md index 71828ec63d..5cdbe3ca51 100644 --- a/website/docs/reference/mcp-config-reference.md +++ b/website/docs/reference/mcp-config-reference.md @@ -355,7 +355,9 @@ on denial or expiry. No browser is launched and no callback listener is needed. When the server's protected-resource metadata lists several authorization servers, device login scans them in order and uses the first one whose metadata issuer matches its advertised URL and that offers the `device_code` grant (a browser-only server listed first is skipped); -issuer validation is never relaxed. +the only accepted difference is the path-scoped shape described under "OAuth-authenticated HTTP servers" +in the MCP feature guide (a server advertised as `https://host/path` whose document at +`/.well-known/oauth-authorization-server/path` names `https://host`). Set `oauth.flow: device` on the server to make `hermes mcp login` and `hermes mcp reauth` (including `reauth --all`) use device authorization. `login --flow browser` overrides that diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index 3031dd3fd6..09258517fd 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -349,6 +349,8 @@ Refresh tokens are bound to the authorization server that granted them: Hermes r The redirect back from the authorization server is checked against RFC 9207: when the server's metadata advertises `authorization_response_iss_parameter_supported`, a redirect without a matching `iss` is rejected. Figma's authorization server (`https://api.figma.com`) advertises that support and then omits `iss`; Hermes fills the missing value from the discovered issuer for that one issuer and logs a warning, so `hermes mcp login figma` completes. A present-but-different `iss` is still rejected, and no other server gets the exemption. +The authorization server's metadata document must name the server the resource advertised (RFC 8414 §3.3); a document for a different server is rejected before any registration or login. One shape is accepted without an exact match: a server advertised with a path (`https://host/path`) whose document, fetched from `https://host/.well-known/oauth-authorization-server/path`, names the origin `https://host` as its issuer — Strava's MCP connector publishes exactly that pair. Only the origin's operator controls that well-known location, so the document is treated as the advertised server's own; a document naming another origin or another path, or one reached only through a redirect or a fallback location, still fails with `Authorization server metadata issuer mismatch`. + **Remote / headless hosts.** When Hermes runs on a different machine than your browser, the loopback callback can't reach your laptop. Ways to complete the flow: - **Hermes Desktop (automatic):** when you run the OAuth sign-in from the Desktop app's MCP setup UI against a remote backend, Desktop hosts the callback listener on *your* machine and relays the authorization back to the gateway automatically — no tunnel, paste, or proxy needed. Requires both the Desktop app and the backend to be up to date.