diff --git a/hermes_cli/dashboard_auth/middleware.py b/hermes_cli/dashboard_auth/middleware.py index caa1b3a6e5d..5c029cac62d 100644 --- a/hermes_cli/dashboard_auth/middleware.py +++ b/hermes_cli/dashboard_auth/middleware.py @@ -53,6 +53,7 @@ _GATE_PUBLIC_PREFIXES: tuple[str, ...] = ( "/auth/logout", "/login", "/api/auth/providers", + "/api/mcp/oauth/callback/", "/assets/", "/favicon.ico", "/ds-assets/", diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index a50f615d60a..0b9d8d9f884 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -587,7 +587,8 @@ async def auth_middleware(request: Request, call_next): if getattr(request.app.state, "auth_required", False): return await call_next(request) path = request.url.path - if path.startswith("/api/") and path not in _PUBLIC_API_PATHS: + is_mcp_oauth_callback = path.startswith("/api/mcp/oauth/callback/") + if path.startswith("/api/") and path not in _PUBLIC_API_PATHS and not is_mcp_oauth_callback: if not _has_valid_session_token(request) and not _has_valid_query_token(request, path): return JSONResponse( status_code=401, @@ -11325,98 +11326,73 @@ async def test_mcp_server(name: str, profile: Optional[str] = None): } -@app.post("/api/mcp/servers/{name}/auth") -async def auth_mcp_server(name: str, profile: Optional[str] = None): - """Run the OAuth flow for an HTTP MCP server (opens the system browser). +_MCP_DASHBOARD_OAUTH_TTL = 15 * 60 +_mcp_oauth_flows: dict[str, "DashboardOAuthFlow"] = {} - Mirrors ``hermes mcp login``: wipe cached OAuth state so the probe forces - a fresh browser flow, connect, then verify a token actually landed on disk - (some providers serve tools/list unauthenticated — see - ``_reauth_oauth_server``). Blocks until the browser flow completes, so it - runs in a worker thread. ``auth: oauth`` is persisted only on success. - """ + +def _gc_mcp_oauth_flows() -> None: + cutoff = time.time() - _MCP_DASHBOARD_OAUTH_TTL + stale = [ + flow_id + for flow_id, flow in _mcp_oauth_flows.items() + if getattr(flow, "created_at", 0) < cutoff + ] + for flow_id in stale: + _mcp_oauth_flows.pop(flow_id, None) + + +def _mcp_oauth_callback_url(request: Request, flow_id: str) -> str: + """Build the externally reachable callback URL for a dashboard flow.""" + from urllib.parse import urlparse, urlunparse + + from hermes_cli.dashboard_auth.prefix import prefix_from_request, resolve_public_url + + suffix = f"/api/mcp/oauth/callback/{flow_id}" + public_url = resolve_public_url() + if public_url: + return f"{public_url}{suffix}" + base = urlparse(str(request.base_url)) + prefix = prefix_from_request(request) + return urlunparse(base._replace(path=f"{prefix}{suffix}", params="", query="", fragment="")) + + +def _run_dashboard_mcp_oauth(flow, cfg: dict) -> None: + """Run the normal MCP probe with dashboard redirect/callback handlers.""" from hermes_cli.mcp_config import ( - _get_mcp_servers, _oauth_tokens_present, _probe_single_server, _save_mcp_server, ) - - with _profile_scope(profile): - servers = _get_mcp_servers() - if name not in servers: - raise HTTPException(status_code=404, detail=f"Server '{name}' not found") - - cfg = dict(servers[name]) - if not cfg.get("url"): - raise HTTPException( - status_code=400, - detail="stdio servers authenticate via env keys, not OAuth", - ) - # A server carrying `headers` uses API-key/bearer auth; a 401 there is a bad - # key, not an OAuth prompt. Refuse rather than rewrite it to `auth: oauth` - # and corrupt a working header-auth config. (Explicit `auth: oauth` is fine.) - if cfg.get("headers") and cfg.get("auth") != "oauth": - raise HTTPException( - status_code=400, - detail="This server uses header/API-key auth, not OAuth — check its key.", - ) - cfg["auth"] = "oauth" - - def _run(): + try: + from tools.mcp_dashboard_oauth import dashboard_oauth_flow from tools.mcp_oauth import HermesTokenStorage, force_interactive_oauth + from tools.mcp_oauth_manager import get_manager - # Home-only scope, not _profile_scope: this blocks on the browser flow - # for up to minutes; holding the shared skills lock that whole time - # would freeze every other endpoint. Config writes here (_save_mcp_server) - # resolve HERMES_HOME via the contextvar override, which is all they need. - with _config_profile_scope(profile), force_interactive_oauth(): - storage = HermesTokenStorage(name) - # Snapshot before clearing: a re-auth wipes cached state to force a - # fresh consent, but if the flow fails we must NOT leave the user - # worse off than before — restore the working token on any failure. + with ( + _config_profile_scope(flow.profile), + force_interactive_oauth(), + dashboard_oauth_flow(flow), + ): + storage = HermesTokenStorage(flow.server_name) backup = storage.snapshot() try: - from tools.mcp_oauth_manager import get_manager - - get_manager().remove(name) - except Exception: - pass # No cached state to clear — fine. - try: - # The default 30s connect timeout would kill the flow while the - # user is still on the consent screen — give the browser - # round-trip the full callback window (300s in mcp_oauth) plus - # headroom so the connect wrapper can't pre-empt it. Honor a - # larger configured connect_timeout when the user set one. - try: - _cfg_timeout = float(cfg.get("connect_timeout", 0)) - except (TypeError, ValueError): - _cfg_timeout = 0.0 + get_manager().remove(flow.server_name) tools = _probe_single_server( - name, cfg, connect_timeout=max(_cfg_timeout, 315) + flow.server_name, + cfg, + connect_timeout=max(float(cfg.get("connect_timeout", 0) or 0), 315), ) + if not _oauth_tokens_present(flow.server_name): + raise RuntimeError( + "The server responded, but no OAuth token was obtained — " + "this provider may require a manually-registered OAuth client." + ) + _save_mcp_server(flow.server_name, cfg) + flow.tools = [{"name": t, "description": d} for t, d in tools] + flow.mark_approved() except Exception: storage.restore(backup) raise - if not _oauth_tokens_present(name): - storage.restore(backup) - return { - "ok": False, - "error": ( - "The server responded, but no OAuth token was obtained — " - "this provider may require a manually-registered OAuth " - "client (see `hermes mcp login`)." - ), - "tools": [], - } - _save_mcp_server(name, cfg) - return { - "ok": True, - "tools": [{"name": t, "description": d} for t, d in tools], - } - - try: - return await asyncio.to_thread(_run) except Exception as exc: msg = str(exc) # Providers that gate RFC 7591 registration to pre-approved clients @@ -11426,13 +11402,95 @@ async def auth_mcp_server(name: str, profile: Optional[str] = None): lowered = msg.lower() if "403" in msg and ("regist" in lowered or "forbidden" in lowered): msg = ( - f"'{name}' only allows pre-approved OAuth clients — it rejected " + f"'{flow.server_name}' only allows pre-approved OAuth clients — it rejected " "client registration (403), so no browser flow can start. " "Options: add a pre-registered client to this server's entry " "(oauth: {client_id: ..., client_secret: ...}), or use the " "provider's stdio / API-key server instead." ) - return {"ok": False, "error": msg, "tools": []} + flow.mark_error(msg) + finally: + # Dashboard auth builds a provider with a public callback URI and bridge + # handlers. Evict that one-shot provider after completion; persisted + # tokens/client registration remain for the normal runtime rebuild. + try: + from tools.mcp_oauth_manager import get_manager + + get_manager().evict(flow.server_name) + except Exception: + pass + + +@app.post("/api/mcp/servers/{name}/auth") +async def auth_mcp_server(name: str, request: Request, profile: Optional[str] = None): + """Start MCP OAuth and hand the authorization URL to the dashboard browser.""" + from hermes_cli.mcp_config import _get_mcp_servers + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + _require_token(request) + _gc_mcp_oauth_flows() + with _profile_scope(profile): + servers = _get_mcp_servers() + if name not in servers: + raise HTTPException(status_code=404, detail=f"Server '{name}' not found") + cfg = dict(servers[name]) + if not cfg.get("url"): + raise HTTPException(status_code=400, detail="stdio servers authenticate via env keys, not OAuth") + if cfg.get("headers") and cfg.get("auth") != "oauth": + raise HTTPException(status_code=400, detail="This server uses header/API-key auth, not OAuth") + cfg["auth"] = "oauth" + + flow_id = secrets.token_urlsafe(24) + flow = DashboardOAuthFlow( + flow_id=flow_id, + server_name=name, + profile=profile, + redirect_uri=_mcp_oauth_callback_url(request, flow_id), + ) + _mcp_oauth_flows[flow_id] = flow + threading.Thread( + target=_run_dashboard_mcp_oauth, + args=(flow, cfg), + daemon=True, + name=f"mcp-oauth-{name}", + ).start() + try: + await flow.wait_for_authorization_url(timeout=30) + except Exception as exc: + flow.mark_error(str(exc)) + return flow.snapshot() + + +@app.get("/api/mcp/oauth/flows/{flow_id}") +async def mcp_oauth_flow_status(flow_id: str, request: Request): + _require_token(request) + _gc_mcp_oauth_flows() + flow = _mcp_oauth_flows.get(flow_id) + if flow is None: + raise HTTPException(status_code=404, detail="OAuth flow not found or expired") + snapshot = flow.snapshot() + snapshot["tools"] = flow.tools + return snapshot + + +@app.get("/api/mcp/oauth/callback/{flow_id}") +async def mcp_oauth_callback( + flow_id: str, + code: Optional[str] = None, + state: Optional[str] = None, + error: Optional[str] = None, +): + _gc_mcp_oauth_flows() + flow = _mcp_oauth_flows.get(flow_id) + 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) + except ValueError as exc: + return HTMLResponse("

OAuth callback rejected

The callback was already used.

", status_code=409) + if error: + return HTMLResponse("

Authorization failed

Return to Hermes for details.

", status_code=400) + return HTMLResponse("

Authorization received

You can close this tab and return to Hermes.

") class MCPEnabledToggle(BaseModel): diff --git a/tests/hermes_cli/test_mcp_dashboard_oauth.py b/tests/hermes_cli/test_mcp_dashboard_oauth.py new file mode 100644 index 00000000000..b569fd70b88 --- /dev/null +++ b/tests/hermes_cli/test_mcp_dashboard_oauth.py @@ -0,0 +1,120 @@ +"""Dashboard HTTP contract for hosted MCP OAuth.""" + +from unittest.mock import patch + +import pytest + + +def _client(): + from starlette.testclient import TestClient + + from hermes_cli.web_server import app, _SESSION_HEADER_NAME, _SESSION_TOKEN + + client = TestClient(app) + client.headers[_SESSION_HEADER_NAME] = _SESSION_TOKEN + return client + + +@pytest.fixture(autouse=True) +def _clear_flows(): + from hermes_cli import web_server + + web_server._mcp_oauth_flows.clear() + yield + web_server._mcp_oauth_flows.clear() + + +def test_hosted_auth_start_returns_public_authorization_url(monkeypatch): + from hermes_cli import web_server + + client = _client() + client.post( + "/api/mcp/servers", + json={"name": "reports", "url": "https://mcp.example/mcp", "auth": "oauth"}, + ) + + def fake_worker(flow, cfg): + import asyncio + + asyncio.run(flow.publish_authorization_url("https://idp.example/authorize?state=s1")) + + monkeypatch.setattr(web_server, "_run_dashboard_mcp_oauth", fake_worker) + with patch( + "hermes_cli.dashboard_auth.prefix.resolve_public_url", + return_value="https://agent.example", + ): + response = client.post("/api/mcp/servers/reports/auth") + + assert response.status_code == 200 + body = response.json() + assert body["status"] == "authorization_required" + assert body["authorization_url"] == "https://idp.example/authorize?state=s1" + flow = web_server._mcp_oauth_flows[body["flow_id"]] + assert flow.redirect_uri == f"https://agent.example/api/mcp/oauth/callback/{body['flow_id']}" + + +def test_hosted_callback_is_public_and_delivers_code(): + from hermes_cli import web_server + from hermes_cli.dashboard_auth.public_paths import PUBLIC_API_PATHS + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-public", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/api/mcp/oauth/callback/flow-public", + ) + web_server._mcp_oauth_flows[flow.flow_id] = flow + + assert "/api/mcp/oauth/callback" not in PUBLIC_API_PATHS + response = _client().get( + "/api/mcp/oauth/callback/flow-public?code=abc&state=expected" + ) + assert response.status_code == 200 + assert flow._callback == ("abc", "expected") + + +def test_hosted_callback_bypasses_gated_cookie_auth(monkeypatch): + from starlette.testclient import TestClient + + from hermes_cli import web_server + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-gated", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/api/mcp/oauth/callback/flow-gated", + ) + web_server._mcp_oauth_flows[flow.flow_id] = flow + monkeypatch.setattr(web_server.app.state, "auth_required", True, raising=False) + + response = TestClient(web_server.app).get( + "/api/mcp/oauth/callback/flow-gated?code=abc&state=expected" + ) + + assert response.status_code == 200 + assert flow._callback == ("abc", "expected") + + +def test_flow_status_does_not_expose_authorization_code(): + from hermes_cli import web_server + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-status", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/api/mcp/oauth/callback/flow-status", + ) + flow.authorization_url = "https://idp.example/authorize" + flow.status = "approved" + flow._callback = ("secret-code", "secret-state") + web_server._mcp_oauth_flows[flow.flow_id] = flow + + response = _client().get("/api/mcp/oauth/flows/flow-status") + assert response.status_code == 200 + body = response.json() + assert body["status"] == "approved" + assert "secret-code" not in response.text + assert "secret-state" not in response.text diff --git a/tests/tools/test_mcp_dashboard_oauth.py b/tests/tools/test_mcp_dashboard_oauth.py new file mode 100644 index 00000000000..35976d4e965 --- /dev/null +++ b/tests/tools/test_mcp_dashboard_oauth.py @@ -0,0 +1,131 @@ +"""Hosted-dashboard bridge for MCP OAuth browser callbacks.""" + +import asyncio + +import pytest + + +def test_dashboard_flow_exposes_authorization_url_and_accepts_callback(): + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-1", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/mcp/oauth/callback/flow-1", + ) + + asyncio.run(flow.publish_authorization_url("https://idp.example/authorize?state=s1")) + assert flow.snapshot() == { + "flow_id": "flow-1", + "server_name": "reports", + "status": "authorization_required", + "authorization_url": "https://idp.example/authorize?state=s1", + "error": None, + } + + flow.deliver_callback(code="code-1", state="s1", error=None) + assert asyncio.run(flow.wait_for_callback()) == ("code-1", "s1") + + +def test_dashboard_flow_rejects_second_callback(): + from tools.mcp_dashboard_oauth import DashboardOAuthFlow + + flow = DashboardOAuthFlow( + flow_id="flow-2", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/mcp/oauth/callback/flow-2", + ) + flow.deliver_callback(code="first", state="state", error=None) + with pytest.raises(ValueError, match="already received"): + flow.deliver_callback(code="second", state="state", error=None) + + +def test_dashboard_context_overrides_redirect_and_handlers(): + from tools.mcp_dashboard_oauth import ( + DashboardOAuthFlow, + dashboard_oauth_flow, + get_dashboard_oauth_flow, + ) + + flow = DashboardOAuthFlow( + flow_id="flow-3", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/mcp/oauth/callback/flow-3", + ) + assert get_dashboard_oauth_flow() is None + with dashboard_oauth_flow(flow): + assert get_dashboard_oauth_flow() is flow + assert get_dashboard_oauth_flow() is None + + +def test_mcp_oauth_helpers_use_dashboard_flow_without_loopback_port(): + from tools.mcp_dashboard_oauth import DashboardOAuthFlow, dashboard_oauth_flow + from tools.mcp_oauth import ( + HermesTokenStorage, + _build_client_metadata, + _configure_callback_port, + _make_callback_waiter, + _make_redirect_handler, + ) + + flow = DashboardOAuthFlow( + flow_id="flow-4", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/mcp/oauth/callback/flow-4", + ) + cfg = {} + with dashboard_oauth_flow(flow): + assert _configure_callback_port(cfg, HermesTokenStorage("reports")) == 0 + metadata = _build_client_metadata(cfg) + assert str(metadata.redirect_uris[0]) == flow.redirect_uri + + asyncio.run(_make_redirect_handler(0)("https://idp.example/authorize")) + flow.deliver_callback(code="code-4", state="state-4", error=None) + assert asyncio.run(_make_callback_waiter(0)()) == ("code-4", "state-4") + + assert flow.authorization_url == "https://idp.example/authorize" + + +def test_manager_build_allows_dashboard_flow_without_tty(tmp_path, monkeypatch): + from tools.mcp_dashboard_oauth import DashboardOAuthFlow, dashboard_oauth_flow + from tools.mcp_oauth_manager import MCPOAuthManager + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setattr("tools.mcp_oauth.sys.stdin.isatty", lambda: False) + flow = DashboardOAuthFlow( + flow_id="flow-5", + server_name="reports", + profile=None, + redirect_uri="https://agent.example/api/mcp/oauth/callback/flow-5", + ) + with dashboard_oauth_flow(flow): + provider = MCPOAuthManager().get_or_build_provider( + "reports", "https://mcp.example/mcp", {} + ) + assert provider is not None + assert str(provider.context.client_metadata.redirect_uris[0]) == flow.redirect_uri + + +def test_manager_evict_preserves_persisted_oauth_state(tmp_path, monkeypatch): + from tools.mcp_oauth import HermesTokenStorage + from tools.mcp_oauth_manager import MCPOAuthManager, _ProviderEntry + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + storage = HermesTokenStorage("reports") + storage._tokens_path().parent.mkdir(parents=True) + storage._tokens_path().write_text( + '{"access_token":"a","token_type":"Bearer"}' + ) + manager = MCPOAuthManager() + manager._entries["reports"] = _ProviderEntry( + server_url="https://mcp.example/mcp", oauth_config={} + ) + + manager.evict("reports") + + assert "reports" not in manager._entries + assert storage._tokens_path().exists() diff --git a/tools/mcp_dashboard_oauth.py b/tools/mcp_dashboard_oauth.py new file mode 100644 index 00000000000..d436066544c --- /dev/null +++ b/tools/mcp_dashboard_oauth.py @@ -0,0 +1,110 @@ +"""Dashboard-mediated callback bridge for MCP OAuth. + +The MCP SDK remains responsible for discovery, DCR, PKCE, state validation and +token exchange. This module only moves the two human/browser callbacks from a +loopback listener into the already-authenticated dashboard session. +""" + +from __future__ import annotations + +import asyncio +import contextvars +import threading +import time +from contextlib import contextmanager +from dataclasses import dataclass, field +from typing import Iterator + + +@dataclass +class DashboardOAuthFlow: + flow_id: str + server_name: str + profile: str | None + redirect_uri: str + created_at: float = field(default_factory=time.time) + status: str = "starting" + authorization_url: str | None = None + error: str | None = None + tools: list[dict] = field(default_factory=list) + _callback: tuple[str, 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 = field(default_factory=threading.Event, init=False, repr=False) + _callback_ready: threading.Event = field(default_factory=threading.Event, init=False, repr=False) + + async def publish_authorization_url(self, url: str) -> None: + self.authorization_url = url + self.status = "authorization_required" + self._authorization_ready.set() + + async def wait_for_authorization_url(self, timeout: float = 30.0) -> str: + ready = await asyncio.to_thread(self._authorization_ready.wait, timeout) + if not ready: + raise TimeoutError("Timed out waiting for MCP authorization URL") + if not self.authorization_url: + 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: + if self._callback_ready.is_set(): + raise ValueError("OAuth callback already received") + if error: + self._callback_error = error + elif code: + self._callback = (code, state) + 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]: + ready = await asyncio.to_thread(self._callback_ready.wait, timeout) + if not ready: + raise TimeoutError("Timed out waiting for MCP OAuth callback") + if self._callback_error: + raise RuntimeError(f"OAuth authorization failed: {self._callback_error}") + if self._callback is None: + raise RuntimeError("OAuth callback did not include an authorization code") + return self._callback + + def mark_approved(self) -> None: + self.status = "approved" + self.error = None + + def mark_error(self, error: str) -> None: + self.status = "error" + self.error = error + self._authorization_ready.set() + self._callback_ready.set() + + def snapshot(self) -> dict: + return { + "flow_id": self.flow_id, + "server_name": self.server_name, + "status": self.status, + "authorization_url": self.authorization_url, + "error": self.error, + } + + +_current_dashboard_flow: contextvars.ContextVar[DashboardOAuthFlow | None] = ( + contextvars.ContextVar("mcp_dashboard_oauth_flow", default=None) +) + + +@contextmanager +def dashboard_oauth_flow(flow: DashboardOAuthFlow) -> Iterator[None]: + token = _current_dashboard_flow.set(flow) + try: + yield + finally: + _current_dashboard_flow.reset(token) + + +def get_dashboard_oauth_flow() -> DashboardOAuthFlow | None: + return _current_dashboard_flow.get() diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index 7959852319f..e50fcbc9c2f 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -630,6 +630,13 @@ def _make_redirect_handler(port: int, redirect_uri: str | None = None): Opens the browser automatically when possible; always prints the URL as a fallback for headless/SSH/gateway environments. """ + from tools.mcp_dashboard_oauth import get_dashboard_oauth_flow + + dashboard_flow = get_dashboard_oauth_flow() + if dashboard_flow is not None: + await dashboard_flow.publish_authorization_url(authorization_url) + return + # Fail fast at the authorization boundary in non-interactive contexts # (systemd gateway, cron, background MCP discovery). A cached-but-unusable # token (expired/revoked, refresh rejected) makes the SDK fall through to @@ -743,6 +750,12 @@ def _make_callback_waiter(port: int): """ async def _wait() -> tuple[str, str | None]: + from tools.mcp_dashboard_oauth import get_dashboard_oauth_flow + + dashboard_flow = get_dashboard_oauth_flow() + if dashboard_flow is not None: + return await dashboard_flow.wait_for_callback() + # Reject before binding the callback listener in non-interactive # contexts. Reaching here means the SDK entered the authorization-code # flow (a valid or refreshable token would never call the callback @@ -972,6 +985,13 @@ def _configure_callback_port( consolidation PR. """ global _oauth_port + from tools.mcp_dashboard_oauth import get_dashboard_oauth_flow + + dashboard_flow = get_dashboard_oauth_flow() + if dashboard_flow is not None: + cfg["_resolved_port"] = 0 + cfg["redirect_uri"] = dashboard_flow.redirect_uri + return 0 requested = int(cfg.get("redirect_port", 0)) # Precedence: explicit config port → cached client-registration port → # fresh ephemeral port. The cached port keeps re-auth consistent with the diff --git a/tools/mcp_oauth_manager.py b/tools/mcp_oauth_manager.py index f4a37683ff6..087a1af91a9 100644 --- a/tools/mcp_oauth_manager.py +++ b/tools/mcp_oauth_manager.py @@ -532,7 +532,13 @@ class MCPOAuthManager: cfg = dict(entry.oauth_config or {}) storage = HermesTokenStorage(server_name) - if not _is_interactive() and not storage.has_cached_tokens(): + from tools.mcp_dashboard_oauth import get_dashboard_oauth_flow + + if ( + get_dashboard_oauth_flow() is None + and not _is_interactive() + and not storage.has_cached_tokens() + ): raise OAuthNonInteractiveError( "MCP OAuth for " f"'{server_name}': non-interactive environment and no " @@ -576,6 +582,11 @@ class MCPOAuthManager: server_name, ) + def evict(self, server_name: str) -> None: + """Drop only the in-process provider, preserving persisted OAuth state.""" + with self._entries_lock: + self._entries.pop(server_name, None) + # -- Disk watch ---------------------------------------------------------- async def invalidate_if_disk_changed(self, server_name: str) -> bool: diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py index 4438a715718..b0c1b976bcf 100644 --- a/tools/mcp_tool.py +++ b/tools/mcp_tool.py @@ -3775,6 +3775,27 @@ def _wrap_with_home_override(coro: "Coroutine") -> "Coroutine": return _scoped() +def _wrap_with_dashboard_oauth_flow(coro): + """Propagate a dashboard OAuth flow onto the dedicated MCP loop task.""" + try: + from tools.mcp_dashboard_oauth import ( + dashboard_oauth_flow, + get_dashboard_oauth_flow, + ) + + flow = get_dashboard_oauth_flow() + except Exception: + return coro + if flow is None: + return coro + + async def _scoped(): + with dashboard_oauth_flow(flow): + return await coro + + return _scoped() + + def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): """Schedule a coroutine on the MCP event loop and block until done. @@ -3809,6 +3830,7 @@ def _run_on_mcp_loop(coro_or_factory, timeout: float = 30): # task's own context (task-local — concurrent calls carrying different # scopes don't interfere). No-op when no override is active. coro = _wrap_with_home_override(coro) + coro = _wrap_with_dashboard_oauth_flow(coro) future = safe_schedule_threadsafe( coro, loop, diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 2691b577d22..114b2c89576 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -996,10 +996,14 @@ export const api = { body: JSON.stringify(body), }), authMcpServer: (name: string) => - fetchJSON( + fetchJSON( `/api/mcp/servers/${encodeURIComponent(name)}/auth`, { method: "POST" }, ), + getMcpOAuthFlow: (flowId: string) => + fetchJSON( + `/api/mcp/oauth/flows/${encodeURIComponent(flowId)}`, + ), removeMcpServer: (name: string) => fetchJSON<{ ok: boolean }>(`/api/mcp/servers/${encodeURIComponent(name)}`, { method: "DELETE", @@ -1465,6 +1469,15 @@ export interface McpTestResult { tools: Array<{ name: string; description: string }>; } +export interface McpOAuthFlow { + flow_id: string; + server_name: string; + status: "starting" | "authorization_required" | "approved" | "error"; + authorization_url: string | null; + error: string | null; + tools?: Array<{ name: string; description: string }>; +} + export interface MessagingPlatformEnvVar { key: string; required: boolean; diff --git a/web/src/lib/mcp-dashboard-oauth.test.ts b/web/src/lib/mcp-dashboard-oauth.test.ts new file mode 100644 index 00000000000..7de079e0c04 --- /dev/null +++ b/web/src/lib/mcp-dashboard-oauth.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it, vi } from "vitest"; +import { completeMcpDashboardOAuth } from "./mcp-dashboard-oauth"; + +describe("completeMcpDashboardOAuth", () => { + it("opens the authorization URL in the dashboard browser and polls to approval", async () => { + const authWindow = { location: { href: "" }, opener: {} } as unknown as Window; + const open = vi.fn().mockReturnValue(authWindow); + const start = vi.fn().mockResolvedValue({ + flow_id: "flow-1", + server_name: "reports", + status: "authorization_required", + authorization_url: "https://idp.example/authorize", + error: null, + }); + const status = vi + .fn() + .mockResolvedValueOnce({ + flow_id: "flow-1", + server_name: "reports", + status: "authorization_required", + authorization_url: "https://idp.example/authorize", + error: null, + tools: [], + }) + .mockResolvedValueOnce({ + flow_id: "flow-1", + server_name: "reports", + status: "approved", + authorization_url: "https://idp.example/authorize", + error: null, + tools: [{ name: "list_reports", description: "List reports" }], + }); + + const result = await completeMcpDashboardOAuth({ + serverName: "reports", + start, + status, + open, + sleep: async () => {}, + }); + + expect(open).toHaveBeenCalledWith( + "about:blank", + "_blank", + ); + expect(authWindow.opener).toBeNull(); + expect(authWindow.location.href).toBe("https://idp.example/authorize"); + expect(status).toHaveBeenCalledTimes(2); + expect(result.status).toBe("approved"); + }); + + it("surfaces a terminal OAuth error", async () => { + const close = vi.fn(); + await expect( + completeMcpDashboardOAuth({ + serverName: "reports", + start: async () => ({ + flow_id: "flow-2", + server_name: "reports", + status: "error", + authorization_url: null, + error: "registration denied", + }), + status: vi.fn(), + open: vi.fn().mockReturnValue({ location: { href: "" }, close }), + sleep: async () => {}, + }), + ).rejects.toThrow("registration denied"); + expect(close).toHaveBeenCalledOnce(); + }); + + it("fails before starting when the browser blocks the popup", async () => { + const start = vi.fn(); + await expect( + completeMcpDashboardOAuth({ + serverName: "reports", + start, + status: vi.fn(), + open: vi.fn().mockReturnValue(null), + }), + ).rejects.toThrow("popup was blocked"); + expect(start).not.toHaveBeenCalled(); + }); +}); diff --git a/web/src/lib/mcp-dashboard-oauth.ts b/web/src/lib/mcp-dashboard-oauth.ts new file mode 100644 index 00000000000..62bbfb8975e --- /dev/null +++ b/web/src/lib/mcp-dashboard-oauth.ts @@ -0,0 +1,51 @@ +import type { McpOAuthFlow } from "./api"; + +type CompleteOptions = { + serverName: string; + start: (name: string) => Promise; + status: (flowId: string) => Promise; + open: (url?: string | URL, target?: string, features?: string) => unknown; + sleep?: (milliseconds: number) => Promise; +}; + +const defaultSleep = (milliseconds: number) => + new Promise((resolve) => window.setTimeout(resolve, milliseconds)); + +export async function completeMcpDashboardOAuth({ + serverName, + start, + status, + open, + sleep = defaultSleep, +}: CompleteOptions): Promise { + // Open synchronously from the click handler, before the first await. Browsers + // otherwise classify the later OAuth popup as unsolicited and block it. + const authWindow = open("about:blank", "_blank") as Window | null; + if (!authWindow) { + throw new Error("OAuth popup was blocked — allow popups for this dashboard and retry"); + } + authWindow.opener = null; + let started: McpOAuthFlow; + try { + started = await start(serverName); + if (started.status === "error") { + throw new Error(started.error || "OAuth failed to start"); + } + if (!started.authorization_url) { + throw new Error("OAuth server did not provide an authorization URL"); + } + authWindow.location.href = started.authorization_url; + } catch (error) { + authWindow.close(); + throw error; + } + + for (;;) { + const current = await status(started.flow_id); + if (current.status === "approved") return current; + if (current.status === "error") { + throw new Error(current.error || "OAuth authorization failed"); + } + await sleep(1000); + } +} diff --git a/web/src/pages/McpPage.tsx b/web/src/pages/McpPage.tsx index 1939c1f5756..454411bf14e 100644 --- a/web/src/pages/McpPage.tsx +++ b/web/src/pages/McpPage.tsx @@ -27,6 +27,7 @@ import { buildMcpServerCreate, type McpTransport, } from "@/lib/mcp-server-create"; +import { completeMcpDashboardOAuth } from "@/lib/mcp-dashboard-oauth"; function isHttpUrl(value: string): boolean { return /^https?:\/\//i.test(value.trim()); @@ -183,13 +184,17 @@ export default function McpPage() { const handleAuthenticate = async (server: McpServer) => { setAuthenticating(server.name); try { - const result = await api.authMcpServer(server.name); - setTestResults((prev) => ({ ...prev, [server.name]: result })); - if (result.ok) { - showToast(`${server.name}: OAuth authentication complete`, "success"); - } else { - showToast(`${server.name}: ${result.error ?? "OAuth failed"}`, "error"); - } + const result = await completeMcpDashboardOAuth({ + serverName: server.name, + start: api.authMcpServer, + status: api.getMcpOAuthFlow, + open: window.open.bind(window), + }); + setTestResults((prev) => ({ + ...prev, + [server.name]: { ok: true, tools: result.tools ?? [] }, + })); + showToast(`${server.name}: OAuth authentication complete`, "success"); } catch (e) { showToast(`OAuth error: ${e}`, "error"); } finally {