diff --git a/apps/desktop/electron/mcp-oauth-callback-ipc.test.ts b/apps/desktop/electron/mcp-oauth-callback-ipc.test.ts index 5ac2f1619c..8878f93687 100644 --- a/apps/desktop/electron/mcp-oauth-callback-ipc.test.ts +++ b/apps/desktop/electron/mcp-oauth-callback-ipc.test.ts @@ -59,6 +59,37 @@ test('listen binds a loopback listener and wait resolves with the redirect param await assert.rejects(fetch(`${redirectUri}?code=again&state=st-1`)) }) +test('wait relays the RFC 9207 iss parameter from the redirect', async () => { + // mcp 2.x rejects an authorization response omitting `iss` when the server + // advertised `authorization_response_iss_parameter_supported` (Cloudflare, + // Resend), so the listener must not drop it. + const { id, redirectUri } = (await invoke('hermes:mcp-oauth:listen')) as { id: string; redirectUri: string } + + const waitPromise = invoke('hermes:mcp-oauth:wait', id, 5000) as Promise<{ + code: null | string + iss: null | string + state: null | string + }> + + await fetch(`${redirectUri}?code=abc123&state=st-1&iss=${encodeURIComponent('https://mcp.cloudflare.com')}`) + + const result = await waitPromise + + assert.equal(result.code, 'abc123') + assert.equal(result.iss, 'https://mcp.cloudflare.com') +}) + +test('a redirect without iss reports it as null rather than undefined', async () => { + // Providers that do not advertise RFC 9207 keep working unchanged. + const { id, redirectUri } = (await invoke('hermes:mcp-oauth:listen')) as { id: string; redirectUri: string } + + const waitPromise = invoke('hermes:mcp-oauth:wait', id, 5000) as Promise<{ iss: null | string }> + + await fetch(`${redirectUri}?code=abc123&state=st-1`) + + assert.equal((await waitPromise).iss, null) +}) + test('non-callback noise (favicon) does not settle the listener', async () => { const { id, redirectUri } = (await invoke('hermes:mcp-oauth:listen')) as { id: string; redirectUri: string } const origin = redirectUri.replace(/\/callback$/, '') diff --git a/apps/desktop/electron/mcp-oauth-callback-ipc.ts b/apps/desktop/electron/mcp-oauth-callback-ipc.ts index 7e064eeb9b..e42b0fc27a 100644 --- a/apps/desktop/electron/mcp-oauth-callback-ipc.ts +++ b/apps/desktop/electron/mcp-oauth-callback-ipc.ts @@ -41,6 +41,10 @@ const DONE_HTML = interface CallbackResult { code: null | string error: null | string + // RFC 9207 issuer identifier. Cloudflare and Resend advertise + // `authorization_response_iss_parameter_supported`, and mcp 2.x rejects an + // authorization response that omits it — relay it rather than dropping it. + iss: null | string state: null | string } @@ -83,7 +87,7 @@ function dispose(id: string) { } if (!entry.settled) { - settle(id, { code: null, error: 'cancelled', state: null }) + settle(id, { code: null, error: 'cancelled', iss: null, state: null }) } pending.delete(id) @@ -112,6 +116,7 @@ export function registerMcpOauthCallbackIpc() { let code: null | string = null let state: null | string = null let error: null | string = null + let iss: null | string = null try { const parsed = new URL(url, 'http://127.0.0.1') @@ -119,11 +124,12 @@ export function registerMcpOauthCallbackIpc() { code = parsed.searchParams.get('code') state = parsed.searchParams.get('state') error = parsed.searchParams.get('error') + iss = parsed.searchParams.get('iss') } catch { error = 'unparseable callback URL' } - settle(id, { code, error, state }) + settle(id, { code, error, iss, state }) }) await new Promise((resolve, reject) => { @@ -143,7 +149,7 @@ export function registerMcpOauthCallbackIpc() { const entry = pending.get(String(id || '')) if (!entry) { - return { code: null, error: 'listener not found', state: null } + return { code: null, error: 'listener not found', iss: null, state: null } } if (entry.result) { @@ -158,7 +164,7 @@ export function registerMcpOauthCallbackIpc() { const result = await new Promise(resolve => { const timer = setTimeout(() => { - settle(String(id), { code: null, error: 'timeout waiting for OAuth callback', state: null }) + settle(String(id), { code: null, error: 'timeout waiting for OAuth callback', iss: null, state: null }) }, timeout) entry.waiters.push(value => { diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 6e4b48f732..bc55bd9108 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -368,13 +368,13 @@ declare global { /** One-shot loopback callback listener for MCP OAuth against remote * backends (electron/mcp-oauth-callback-ipc.ts): bind on THIS machine, * pass redirectUri as client_redirect_uri to mcp.servers.oauth.start, - * await the provider redirect, relay code/state via oauth.callback. */ + * await the provider redirect, relay code/state/iss via oauth.callback. */ mcpOauth?: { listen: () => Promise<{ id: string; redirectUri: string }> wait: ( id: string, timeoutMs?: number - ) => Promise<{ code: null | string; error: null | string; state: null | string }> + ) => Promise<{ code: null | string; error: null | string; iss: null | string; state: null | string }> cancel: (id: string) => Promise } openPreviewInBrowser?: (url: string) => Promise diff --git a/hermes_cli/web_routers/mcp.py b/hermes_cli/web_routers/mcp.py index 8537496b0c..fa17cba2fb 100644 --- a/hermes_cli/web_routers/mcp.py +++ b/hermes_cli/web_routers/mcp.py @@ -307,6 +307,7 @@ async def mcp_oauth_callback( code: Optional[str] = None, state: Optional[str] = None, error: Optional[str] = None, + iss: Optional[str] = None, ): _gc_mcp_oauth_flows() with _mcp_oauth_flows_lock: @@ -322,7 +323,7 @@ async def mcp_oauth_callback( if flow is None: return HTMLResponse("

OAuth flow expired

Return to Hermes and try again.

", status_code=404) try: - flow.deliver_callback(code=code, state=state, error=error) + flow.deliver_callback(code=code, state=state, error=error, iss=iss) except ValueError as exc: return HTMLResponse( "

OAuth callback rejected

The callback was invalid or already used.

", diff --git a/tests/tools/test_mcp_dashboard_oauth.py b/tests/tools/test_mcp_dashboard_oauth.py index 198a2f0977..bf781e50f6 100644 --- a/tests/tools/test_mcp_dashboard_oauth.py +++ b/tests/tools/test_mcp_dashboard_oauth.py @@ -27,7 +27,25 @@ def test_dashboard_flow_exposes_authorization_url_and_accepts_callback(): } flow.deliver_callback(code="code-1", state="s1", error=None) - assert asyncio.run(flow.wait_for_callback()) == ("code-1", "s1") + assert asyncio.run(flow.wait_for_callback()) == ("code-1", "s1", None) + + +def test_dashboard_flow_preserves_rfc9207_iss(): + """RFC 9207 ``iss`` survives the callback bridge: mcp 2.x rejects an authorization response + that omits it when the authorization server advertised support (Cloudflare, Resend).""" + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-iss", + server_name="cloudflare", + profile=None, + hermes_home="/tmp/hermes-test", + redirect_uri="https://agent.example/mcp/oauth/callback/flow-iss", + ) + asyncio.run(flow.publish_authorization_url("https://idp.example/authorize?state=s1")) + + flow.deliver_callback(code="code-1", state="s1", error=None, iss="https://mcp.cloudflare.com") + assert asyncio.run(flow.wait_for_callback()) == ("code-1", "s1", "https://mcp.cloudflare.com") def test_dashboard_flow_accepts_only_one_concurrent_callback(): diff --git a/tests/tui_gateway/test_mcp_oauth_client_callback.py b/tests/tui_gateway/test_mcp_oauth_client_callback.py index e89a2eb245..6ca187ec10 100644 --- a/tests/tui_gateway/test_mcp_oauth_client_callback.py +++ b/tests/tui_gateway/test_mcp_oauth_client_callback.py @@ -186,7 +186,36 @@ def test_deliver_callback_accepts_matching_state(): flow = _make_session() out = deliver_callback_flow("sess-relay-1", "hosp", code="abc", state="s3cr3tstate") assert out == {"ok": True, "session_id": "sess-relay-1"} - assert flow._callback == ("abc", "s3cr3tstate") + assert flow._callback == ("abc", "s3cr3tstate", None) + + +def test_deliver_callback_forwards_iss(): + """The client-redirect relay carries RFC 9207 ``iss`` into the flow. Desktop drives this path + against a remote backend, and mcp 2.x rejects a response missing ``iss`` when the authorization + server advertised ``authorization_response_iss_parameter_supported``.""" + flow = _make_session() + out = deliver_callback_flow( + "sess-relay-1", "hosp", code="abc", state="s3cr3tstate", iss="https://as.example.com" + ) + assert out["ok"] is True + assert flow._callback == ("abc", "s3cr3tstate", "https://as.example.com") + + +def test_loopback_listener_forwards_iss(): + """The gateway-hosted loopback listener parses ``iss`` off the redirect rather than dropping it.""" + import urllib.request + + flow = _make_session(session_id="sess-relay-loop", server="loopy", state="loopstate") + httpd = mcp_oauth_sessions._start_loopback_listener(flow) + try: + port = httpd.server_address[1] + urllib.request.urlopen( + f"http://127.0.0.1:{port}/callback?code=abc&state=loopstate&iss=https%3A%2F%2Fas.example.com", + timeout=5, + ).read() + finally: + httpd.shutdown() + assert flow._callback == ("abc", "loopstate", "https://as.example.com") def test_deliver_callback_rejects_state_mismatch(): diff --git a/tools/mcp_dashboard_oauth.py b/tools/mcp_dashboard_oauth.py index c03551db74..ffcac3a8cc 100644 --- a/tools/mcp_dashboard_oauth.py +++ b/tools/mcp_dashboard_oauth.py @@ -43,7 +43,7 @@ class DashboardOAuthFlow: error: str | None = None tools: list[dict] = field(default_factory=list) expected_state: str | None = field(default=None, init=False) - _callback: tuple[str, str | None] | None = field(default=None, init=False, repr=False) + _callback: tuple[str, str | None, str | None] | None = field(default=None, init=False, repr=False) _callback_error: str | None = field(default=None, init=False, repr=False) _authorization_ready: threading.Event = _event_field() _callback_ready: threading.Event = _event_field() @@ -70,8 +70,15 @@ class DashboardOAuthFlow: raise RuntimeError(self.error or "MCP OAuth flow ended before authorization") return self.authorization_url - def deliver_callback(self, *, code: str | None, state: str | None, error: str | None) -> None: - """Hand the browser redirect to the waiting flow; ``state`` must match exactly.""" + def deliver_callback( + self, *, code: str | None, state: str | None, error: str | None, iss: str | None = None + ) -> None: + """Hand the browser redirect to the waiting flow; ``state`` must match exactly. + + ``iss`` (RFC 9207) is carried through: mcp 2.x rejects an authorization response that omits + it when the server advertised ``authorization_response_iss_parameter_supported``, which + Cloudflare and Resend both do. Dropping it breaks login against those providers. + """ with self._lock: if self._callback_ready.is_set(): raise ValueError("OAuth callback already received") @@ -80,12 +87,12 @@ class DashboardOAuthFlow: if error: self._callback_error = error elif code: - self._callback = (code, state) + self._callback = (code, state, iss) else: self._callback_error = "OAuth callback did not include code or error" self._callback_ready.set() - async def wait_for_callback(self, timeout: float = 300.0) -> tuple[str, str | None]: + async def wait_for_callback(self, timeout: float = 300.0) -> tuple[str, str | None, str | None]: if not await asyncio.to_thread(self._callback_ready.wait, timeout): raise TimeoutError("Timed out waiting for MCP OAuth callback") if self._callback_error: diff --git a/tui_gateway/mcp_oauth_sessions.py b/tui_gateway/mcp_oauth_sessions.py index fc74662fab..c09e55d09f 100644 --- a/tui_gateway/mcp_oauth_sessions.py +++ b/tui_gateway/mcp_oauth_sessions.py @@ -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"

OAuth callback rejected

The callback was invalid or already used.

" 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} diff --git a/tui_gateway/methods_tools.py b/tui_gateway/methods_tools.py index ce052d64eb..044598c46f 100644 --- a/tui_gateway/methods_tools.py +++ b/tui_gateway/methods_tools.py @@ -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))