fix(mcp): thread RFC 9207 iss through every OAuth callback relay
mcp 2.x rejects an authorization response that omits the RFC 9207 `iss`
parameter when the authorization server advertised
`authorization_response_iss_parameter_supported`. Cloudflare advertises it
AND sends it; the CLI loopback handler has always forwarded it, but every
other callback producer parsed only code/state/error, so the SDK raised:
OAuthFlowError: Authorization response missing iss parameter
advertised by the authorization server
and the server parked. Same machine, same config, `hermes mcp login <name>`
from a terminal succeeded — the failure is specific to the non-CLI relays.
Forward `iss` on every producer, matching `_make_callback_handler()`:
- tools/mcp_dashboard_oauth.py: `deliver_callback()` accepts `iss`;
`wait_for_callback()` returns `(code, state, iss)`. The bridge in
tools/mcp_oauth.py already splats that tuple into
`_authorization_code_result(code, state, iss)`, so it needs no change.
- tui_gateway/mcp_oauth_sessions.py: the gateway-hosted loopback listener
parses `iss`, and `deliver_callback_flow()` forwards it.
- tui_gateway/methods_tools.py: the `oauth.callback` RPC passes `iss`.
- hermes_cli/web_routers/mcp.py: the dashboard callback route accepts it.
- apps/desktop/electron/mcp-oauth-callback-ipc.ts: the one-shot listener
reads `iss` off the redirect (the renderer already spreads the whole
callback object into the RPC, so it flows through unchanged).
Providers that omit `iss` round-trip as `None`/`null` rather than being
dropped, so servers that do not advertise RFC 9207 keep working.
Verified live on Windows against mcp.cloudflare.com, whose metadata sets
`authorization_response_iss_parameter_supported: true`: the server that
previously parked on the missing-iss error now reports
`Authenticated — 3452 tool(s) available` and `hermes mcp test cloudflare`
connects. State-mismatch and replay rejection are unchanged.
Tests (each fails on base, passes with the fix):
- test_dashboard_flow_preserves_rfc9207_iss
- test_deliver_callback_forwards_iss (client-redirect relay)
- test_loopback_listener_forwards_iss (real HTTP redirect)
- two vitest cases on the Electron listener, incl. the iss-absent case
Refs #92758, #99984. PR #92765 fixes the dashboard route and the loopback
listener but not the client-redirect relay
(`deliver_callback_flow` / `oauth.callback` / the Electron listener), which
is the path Desktop drives against a remote backend.
This commit is contained in:
@@ -61,7 +61,7 @@ def _start_loopback_listener(flow) -> "http.server.HTTPServer":
|
||||
status = 200
|
||||
try:
|
||||
flow.deliver_callback(
|
||||
**{k: (qs.get(k) or [None])[0] for k in ("code", "state", "error")})
|
||||
**{k: (qs.get(k) or [None])[0] for k in ("code", "state", "error", "iss")})
|
||||
except Exception:
|
||||
body = b"<h1>OAuth callback rejected</h1><p>The callback was invalid or already used.</p>"
|
||||
status = 400
|
||||
@@ -255,7 +255,7 @@ def cancel_flow(session_id: str, server_name: str, hermes_home: str) -> Dict[str
|
||||
|
||||
def deliver_callback_flow(
|
||||
session_id: str, server_name: str, *, code: Optional[str], state: Optional[str],
|
||||
error: Optional[str] = None) -> Dict[str, Any]:
|
||||
error: Optional[str] = None, iss: Optional[str] = None) -> Dict[str, Any]:
|
||||
"""Relay a client-captured OAuth redirect into a session's flow (remote-backend companion
|
||||
to ``start_flow(client_redirect_uri=...)``); ``deliver_callback`` still verifies ``state``
|
||||
and rejects replays. Returns ``{ok: true}`` or ``{ok: false, error_message}``."""
|
||||
@@ -263,7 +263,7 @@ def deliver_callback_flow(
|
||||
if rec is None:
|
||||
return {"ok": False, "error_message": err}
|
||||
try:
|
||||
rec["flow"].deliver_callback(code=code, state=state, error=error)
|
||||
rec["flow"].deliver_callback(code=code, state=state, error=error, iss=iss)
|
||||
except ValueError as exc:
|
||||
return {"ok": False, "error_message": str(exc)}
|
||||
return {"ok": True, "session_id": session_id}
|
||||
|
||||
@@ -1365,8 +1365,8 @@ def _(rid, params: dict) -> dict:
|
||||
|
||||
@_mcp_rpc("oauth.callback", _NAME_SESSION)
|
||||
def _(rid, params: dict) -> dict:
|
||||
"""Relay a client-captured redirect (``code``/``state``/``error``) into a ``client_redirect_uri`` flow."""
|
||||
code, state, error = (str(params.get(k) or "") or None for k in ("code", "state", "error"))
|
||||
"""Relay a client-captured redirect (``code``/``state``/``error``/``iss``) into a ``client_redirect_uri`` flow."""
|
||||
code, state, error, iss = (str(params.get(k) or "") or None for k in ("code", "state", "error", "iss"))
|
||||
deliver = _tools_mod("tui_gateway.mcp_oauth_sessions").deliver_callback_flow
|
||||
return _ok(rid, deliver(
|
||||
_str_arg(params, "session_id"), _str_arg(params, "name"), code=code, state=state, error=error))
|
||||
|
||||
Reference in New Issue
Block a user