fix(mcp): dashboard/Desktop Authorize honours a pre-registered client's pinned loopback redirect
A no-DCR entry (client_id + redirect_port, e.g. the Asana manifest) has http://localhost:<port>/callback registered with the vendor, which matches redirect URLs exactly; the dashboard flow overrode it with its own callback URL, so the in-app Authorize button could never complete for such entries. The pinned loopback listener now wins (over the dashboard URL and any cached registration URI); the dashboard flow only publishes the authorization URL, and the stdin paste reader stays off under a dashboard flow. Docs/post_install say the approving browser must run on the Hermes machine.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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:<port>/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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 <name>` 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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user