fix(mcp): accept an origin-issued metadata document for a path-scoped OAuth authorization server
A protected resource may advertise a path-scoped authorization server (`https://www.strava.com/mcp-issuer`) whose RFC 8414 document, served from `/.well-known/oauth-authorization-server/mcp-issuer`, declares the origin (`https://www.strava.com`) as its issuer. The SDK's exact-string check (`validate_metadata_issuer`, RFC 8414 §3.3) rejected that document with "Authorization server metadata issuer mismatch" and the connection parked before registration or login (#116233). `metadata_issued_by_origin` accepts exactly that shape and nothing else: the document must have been read from the well-known URL derived from the advertised identifier (so a redirect target, the root document or an OIDC fallback never qualify) and its issuer must be the advertised identifier's origin. Only the origin's operator controls that location, so a party controlling a path or a sibling host cannot use it to make the client accept another server's endpoints; whoever could would already control the exact-match document too. The browser flow applies it in the mixin's request pump (installs the document, hands the SDK a 204 so its loop stops) without touching `auth_server_url`, so SEP-2352 credential binding keeps the advertised identifier while the RFC 9207 `iss` check and refresh-token issuer binding use the document's issuer. The device flow applies the same rule in its own discovery and now binds registered credentials the way the SDK's Step 4 does, so the runtime flow reuses them instead of discarding them on the next 401. Supersedes #116359: its rule accepted the origin issuer from any discovery URL (root and OIDC fallbacks, redirect targets) and fabricated a 500 response. Co-authored-by: Finn763 <165816600+Finn763@users.noreply.github.com>
This commit is contained in:
113
tests/tools/test_mcp_oauth_issuer_origin.py
Normal file
113
tests/tools/test_mcp_oauth_issuer_origin.py
Normal file
@@ -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()
|
||||
@@ -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:
|
||||
|
||||
@@ -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, ``<origin>/.well-known/oauth-authorization-server
|
||||
<path>`` — 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'."""
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user