diff --git a/optional-mcps/asana/manifest.yaml b/optional-mcps/asana/manifest.yaml index 511701a16d..3d050f29a9 100644 --- a/optional-mcps/asana/manifest.yaml +++ b/optional-mcps/asana/manifest.yaml @@ -52,6 +52,8 @@ post_install: | Its Client ID / Client secret are the ASANA_CLIENT_ID / ASANA_CLIENT_SECRET values prompted above (stored in the profile's .env, never in config.yaml). - Then run `hermes mcp login asana`, approve access in the browser, and + Then run `hermes mcp login asana` (or click Authorize in the dashboard / + Desktop: the callback still arrives at http://localhost:27890/callback, so + the browser must run on the same machine as Hermes), approve access, and restart (or `/reload-mcp`) the Hermes session or gateway that should expose the Asana tools. diff --git a/tests/tools/test_mcp_dashboard_oauth.py b/tests/tools/test_mcp_dashboard_oauth.py index fce5edec17..fca56ab12f 100644 --- a/tests/tools/test_mcp_dashboard_oauth.py +++ b/tests/tools/test_mcp_dashboard_oauth.py @@ -166,11 +166,53 @@ def test_failed_reauth_rollback_preserves_newer_oauth_state(tmp_path, monkeypatc monkeypatch.setenv("HERMES_HOME", str(tmp_path)) storage = HermesTokenStorage("reports") storage._tokens_path().parent.mkdir(parents=True) - storage._tokens_path().write_text("OLD") + storage._tokens_path().write_text("OLD", encoding="utf-8") backup = storage.snapshot() storage.remove() - storage._tokens_path().write_text("FRESH") + storage._tokens_path().write_text("FRESH", encoding="utf-8") storage.restore(backup, only_if_absent=True) - assert storage._tokens_path().read_text() == "FRESH" + assert storage._tokens_path().read_text(encoding="utf-8") == "FRESH" + + +def test_preregistered_pinned_redirect_port_keeps_loopback_listener_under_dashboard_flow(tmp_path, monkeypatch): + """A no-DCR entry (``client_id`` + ``redirect_port``, e.g. the shipped Asana manifest) registered + ``http://localhost:/callback`` with the vendor, which matches redirect URLs exactly. The + dashboard/Desktop Authorize button must therefore keep that loopback URI and listener — the + dashboard flow only publishes the authorization URL — instead of forcing its own callback URL.""" + import socket + import urllib.request + + 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, + force_interactive_oauth, + ) + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + flow = DashboardOAuthFlow(flow_id="flow-5", server_name="asana", profile=None, hermes_home=str(tmp_path), + redirect_uri="https://agent.example/mcp/oauth/callback/flow-5") + cfg = {"client_id": "pre-registered", "client_secret": "s", "redirect_host": "localhost", "redirect_port": port} + with dashboard_oauth_flow(flow), force_interactive_oauth(): + assert _configure_callback_port(cfg, HermesTokenStorage("asana")) == port + assert str(_build_client_metadata(cfg).redirect_uris[0]) == f"http://localhost:{port}/callback" + asyncio.run(_make_redirect_handler(port)("https://idp.example/authorize?state=state-5")) + assert flow.authorization_url == "https://idp.example/authorize?state=state-5" # dashboard shows the URL + + async def _authorize(): + waiter = asyncio.ensure_future(_make_callback_waiter(port, timeout=10)()) + await asyncio.sleep(0.3) # listener bound + await asyncio.to_thread( + lambda: urllib.request.urlopen(f"http://127.0.0.1:{port}/callback?code=code-5&state=state-5", timeout=5).read()) + return await waiter + + result = asyncio.run(_authorize()) + assert (result.code, result.state) == ("code-5", "state-5") diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index c5b3883242..fbb45edc55 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -845,8 +845,9 @@ def _make_callback_waiter(port: int, cimd_url: str | None = None, timeout: float """ async def _wait(): dashboard_flow = get_dashboard_oauth_flow() - if dashboard_flow is not None: - # Dashboard flow speaks the legacy tuple; normalize to one shape. + if dashboard_flow is not None and not port: + # Dashboard flow speaks the legacy tuple; normalize to one shape. A pinned loopback port + # (pre-registered client) listens locally instead; the dashboard only shows the URL. return _authorization_code_result(*await dashboard_flow.wait_for_callback()) # The SDK entered the authorization-code flow, so any cached token is unusable. Reject BEFORE # binding: binding would block for the full timeout and collide with the TIME_WAIT port on retry. @@ -864,8 +865,9 @@ def _make_callback_waiter(port: int, cimd_url: str | None = None, timeout: float handler_cls, result = _make_callback_handler() server = _start_callback_server(port, handler_cls) threading.Thread(target=server.handle_request, daemon=True).start() - # Paste fallback races the HTTP listener; whichever fills result first wins. - if _is_interactive(): + # Paste fallback races the HTTP listener; whichever fills result first wins (no stdin reader + # under a dashboard flow — the gateway's stdin is not the user's). + if _is_interactive() and dashboard_flow is None: print( "\n Or paste the redirect URL here (or the ``?code=...&state=...`` portion) and press Enter. " "Type ``skip`` + Enter to continue without this server:", @@ -1005,12 +1007,16 @@ def _configure_callback_port(cfg: dict, storage: "HermesTokenStorage | None" = N """ global _oauth_port dashboard_flow = get_dashboard_oauth_flow() - if dashboard_flow is not None: + # A pre-registered client with a pinned ``redirect_port`` (no DCR; the vendor matches the registered + # loopback URI exactly) keeps its loopback listener even under the dashboard/Desktop flow: the + # dashboard's own callback URL was never registered with the vendor, so that flow could not complete. + pinned_loopback = bool(cfg.get("client_id") and cfg.get("redirect_port")) + if dashboard_flow is not None and not pinned_loopback: cfg["_resolved_port"] = 0 cfg["redirect_uri"] = cfg.get("redirect_uri") or dashboard_flow.redirect_uri return 0 cached_uri, cached_port = _cached_redirect(storage) - if cached_uri and not cfg.get("redirect_uri"): + if cached_uri and not cfg.get("redirect_uri") and not pinned_loopback: cfg["redirect_uri"] = cached_uri cfg["_resolved_port"] = 0 return 0 diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index 928e7c2985..25c3d63f96 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -203,7 +203,13 @@ mcp_servers: Read the entry's `post_install` notes for the exact app type and redirect URL to register, then run `hermes mcp login ` and restart (or -`/reload-mcp`) the session or gateway that should expose the tools. +`/reload-mcp`) the session or gateway that should expose the tools. The +dashboard / Desktop **Authorize** button works too: because the client is +pre-registered with a pinned `redirect_port`, Hermes keeps the registered +loopback callback (`http://localhost:27890/callback`) instead of the +dashboard's own callback URL — so the browser you approve in must run on the +same machine as the Hermes process. For a remote host, use `hermes mcp login` +over SSH port-forwarding. ### Updating tool selection later