feat(mcp-oauth): Desktop MCP OAuth now completes against remote backends (client-side callback relay)
The gateway's session-backed MCP OAuth flow (mcp.servers.oauth.start) binds its browser-callback listener on the BACKEND machine's 127.0.0.1. When the Desktop app connects to a remote backend (SSH/Tailscale), the user's browser resolves that loopback to the user's machine, the redirect dies, and every OAuth catalog server (ClickUp, Hospitable, ...) fails in-app with no working path — the exact topology from the 'MCP Recurring erros' support thread. Fix mirrors the Desktop's native gateway login (native-oauth-login.ts): - gateway: mcp.servers.oauth.start accepts client_redirect_uri (loopback-only, RFC 8252-style validation); when supplied no gateway listener is bound and the OAuth redirect_uri pins to the client's listener. - gateway: new mcp.servers.oauth.callback RPC relays the client-captured code/state into the flow; state verification stays in DashboardOAuthFlow.deliver_callback (constant-time compare, replay-safe). - desktop: mcp-oauth-callback-ipc.ts hosts a one-shot 127.0.0.1 listener in the main process (hermes:mcp-oauth:listen/wait/cancel via preload bridge). - desktop: hermes-bots mcp-setup.tsx prefers the client listener for local AND remote backends, falling back to the legacy gateway-listener flow on older gateways (feature-detect via start rejection). - docs: remote-host MCP OAuth section documents the automatic Desktop path. Validation: 19 new gateway tests (validator allowlist, listener skip, relay accept/reject/replay) — sabotage-verified; 5 new desktop tests against a real ephemeral listener; E2E through the real session registry + flow bridge with a stubbed provider probe; tsc electron+renderer builds clean.
This commit is contained in:
@@ -268,6 +268,7 @@ import { loadNativeTokenSet, type NativeTokenStoreIo, persistNativeTokenSet } fr
|
||||
import { serializeJsonBody, setJsonRequestHeaders } from './oauth-net-request'
|
||||
import { LEGACY_OAUTH_PARTITION, resolveOauthPartition } from './oauth-partition'
|
||||
import { createParentStartMarkerResolver, parentWatchdogEnv } from './parent-process-identity'
|
||||
import { registerMcpOauthCallbackIpc } from './mcp-oauth-callback-ipc'
|
||||
import { registerPetOverlayIpc } from './pet-overlay-ipc'
|
||||
import {
|
||||
buildRegistryProfileRoutes,
|
||||
@@ -16841,6 +16842,10 @@ registerFsIpc({
|
||||
// Git-driven features (worktrees, review pane, repo scan) — see git-ipc.ts.
|
||||
registerGitIpc({ resolveGitBinary, resolveGhBinary })
|
||||
|
||||
// Client-side loopback callback for MCP OAuth against remote backends — see
|
||||
// mcp-oauth-callback-ipc.ts.
|
||||
registerMcpOauthCallbackIpc()
|
||||
|
||||
// Embedded terminal PTY host (hermes:terminal:*) — see terminal-ipc.ts.
|
||||
const terminalIpc = registerTerminalIpc({
|
||||
isWindows: IS_WINDOWS,
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
/**
|
||||
* Tests for electron/mcp-oauth-callback-ipc.ts — the client-side one-shot
|
||||
* loopback listener MCP OAuth uses against remote backends. Uses a REAL
|
||||
* ephemeral http listener (it binds 127.0.0.1:0, no fixed ports) with the
|
||||
* electron ipcMain mocked, and drives synthetic browser hits with fetch.
|
||||
*
|
||||
* Run with: vitest run --project electron mcp-oauth-callback-ipc
|
||||
*/
|
||||
|
||||
import assert from 'node:assert/strict'
|
||||
|
||||
import { test, vi } from 'vitest'
|
||||
|
||||
const handlers = new Map<string, (...args: unknown[]) => unknown>()
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
ipcMain: {
|
||||
handle: (channel: string, fn: (...args: unknown[]) => unknown) => {
|
||||
handlers.set(channel, fn)
|
||||
}
|
||||
}
|
||||
}))
|
||||
|
||||
const { registerMcpOauthCallbackIpc } = await import('./mcp-oauth-callback-ipc')
|
||||
|
||||
registerMcpOauthCallbackIpc()
|
||||
|
||||
const invoke = (channel: string, ...args: unknown[]) => {
|
||||
const fn = handlers.get(channel)
|
||||
|
||||
assert.ok(fn, `handler registered for ${channel}`)
|
||||
|
||||
return fn!({}, ...args)
|
||||
}
|
||||
|
||||
test('listen binds a loopback listener and wait resolves with the redirect params', async () => {
|
||||
const { id, redirectUri } = (await invoke('hermes:mcp-oauth:listen')) as { id: string; redirectUri: string }
|
||||
|
||||
assert.match(redirectUri, /^http:\/\/127\.0\.0\.1:\d+\/callback$/)
|
||||
|
||||
const waitPromise = invoke('hermes:mcp-oauth:wait', id, 5000) as Promise<{
|
||||
code: null | string
|
||||
error: null | string
|
||||
state: null | string
|
||||
}>
|
||||
|
||||
const res = await fetch(`${redirectUri}?code=abc123&state=st-1`)
|
||||
|
||||
assert.equal(res.status, 200)
|
||||
assert.match(await res.text(), /return to Hermes/)
|
||||
|
||||
const result = await waitPromise
|
||||
|
||||
assert.equal(result.code, 'abc123')
|
||||
assert.equal(result.state, 'st-1')
|
||||
assert.equal(result.error, null)
|
||||
|
||||
// Listener is one-shot: the port must be closed after the callback.
|
||||
await assert.rejects(fetch(`${redirectUri}?code=again&state=st-1`))
|
||||
})
|
||||
|
||||
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$/, '')
|
||||
|
||||
const res = await fetch(`${origin}/favicon.ico`)
|
||||
|
||||
assert.equal(res.status, 200)
|
||||
|
||||
const waitPromise = invoke('hermes:mcp-oauth:wait', id, 5000) as Promise<{ code: null | string }>
|
||||
|
||||
await fetch(`${redirectUri}?code=late-code&state=s`)
|
||||
|
||||
const result = await waitPromise
|
||||
|
||||
assert.equal(result.code, 'late-code')
|
||||
})
|
||||
|
||||
test('provider error param is forwarded', async () => {
|
||||
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; error: null | string }>
|
||||
|
||||
await fetch(`${redirectUri}?error=access_denied&state=s`)
|
||||
|
||||
const result = await waitPromise
|
||||
|
||||
assert.equal(result.code, null)
|
||||
assert.equal(result.error, 'access_denied')
|
||||
})
|
||||
|
||||
test('cancel tears the listener down and wait reports listener not found afterwards', async () => {
|
||||
const { id, redirectUri } = (await invoke('hermes:mcp-oauth:listen')) as { id: string; redirectUri: string }
|
||||
|
||||
assert.equal(await invoke('hermes:mcp-oauth:cancel', id), true)
|
||||
|
||||
await assert.rejects(fetch(`${redirectUri}?code=x&state=s`))
|
||||
|
||||
const result = (await invoke('hermes:mcp-oauth:wait', id, 100)) as { error: null | string }
|
||||
|
||||
assert.equal(result.error, 'listener not found')
|
||||
})
|
||||
|
||||
test('wait times out when no callback arrives', async () => {
|
||||
const { id } = (await invoke('hermes:mcp-oauth:listen')) as { id: string }
|
||||
|
||||
const result = (await invoke('hermes:mcp-oauth:wait', id, 1000)) as { code: null | string; error: null | string }
|
||||
|
||||
assert.equal(result.code, null)
|
||||
assert.match(String(result.error), /timeout/)
|
||||
})
|
||||
@@ -0,0 +1,181 @@
|
||||
/**
|
||||
* mcp-oauth-callback-ipc.ts
|
||||
*
|
||||
* Client-side loopback callback listener for MCP OAuth against a REMOTE
|
||||
* backend. The gateway's own `mcp.servers.oauth.start` flow binds its
|
||||
* callback listener on the BACKEND machine's 127.0.0.1 — unreachable from
|
||||
* the user's browser when Desktop connects over SSH/Tailscale, so the
|
||||
* provider redirect dies on the user's machine and the flow times out.
|
||||
*
|
||||
* This module gives the renderer the same primitive the native gateway
|
||||
* login uses (native-oauth-login.ts): bind an ephemeral one-shot listener
|
||||
* on the USER'S loopback, hand its URL to the gateway as the OAuth
|
||||
* redirect_uri (`client_redirect_uri` on oauth.start), and resolve with the
|
||||
* redirect's `code`/`state` so the renderer can relay them via
|
||||
* `mcp.servers.oauth.callback`.
|
||||
*
|
||||
* Security posture:
|
||||
* - binds 127.0.0.1 on an ephemeral port; closes on first callback,
|
||||
* cancel, or timeout — no long-lived listener;
|
||||
* - the listener only ever RECEIVES `code`/`state` query params and
|
||||
* forwards them to the renderer; no tokens are exchanged here — the
|
||||
* gateway verifies `state` (constant-time) before redeeming anything;
|
||||
* - the browser sees only a minimal "return to Hermes" page.
|
||||
*/
|
||||
|
||||
import http from 'node:http'
|
||||
import type { AddressInfo } from 'node:net'
|
||||
|
||||
import { ipcMain } from 'electron'
|
||||
|
||||
const DEFAULT_WAIT_TIMEOUT_MS = 5 * 60 * 1000
|
||||
const MAX_PENDING_LISTENERS = 8
|
||||
|
||||
const DONE_HTML =
|
||||
'<!doctype html><meta charset="utf-8"><title>Authorization received</title>' +
|
||||
'<body style="font:15px system-ui;margin:3rem;text-align:center">' +
|
||||
'<h2>✓ Authorization received</h2>' +
|
||||
'<p>You can close this window and return to Hermes.</p>' +
|
||||
'<script>setTimeout(()=>window.close(),800)</script>'
|
||||
|
||||
interface CallbackResult {
|
||||
code: null | string
|
||||
error: null | string
|
||||
state: null | string
|
||||
}
|
||||
|
||||
interface PendingListener {
|
||||
result: CallbackResult | null
|
||||
server: http.Server
|
||||
settled: boolean
|
||||
waiters: Array<(result: CallbackResult) => void>
|
||||
}
|
||||
|
||||
const pending = new Map<string, PendingListener>()
|
||||
let nextId = 1
|
||||
|
||||
function settle(id: string, result: CallbackResult) {
|
||||
const entry = pending.get(id)
|
||||
|
||||
if (!entry || entry.settled) {
|
||||
return
|
||||
}
|
||||
|
||||
entry.settled = true
|
||||
entry.result = result
|
||||
|
||||
try {
|
||||
entry.server.close()
|
||||
} catch {
|
||||
// already closed
|
||||
}
|
||||
|
||||
for (const waiter of entry.waiters.splice(0)) {
|
||||
waiter(result)
|
||||
}
|
||||
}
|
||||
|
||||
function dispose(id: string) {
|
||||
const entry = pending.get(id)
|
||||
|
||||
if (!entry) {
|
||||
return
|
||||
}
|
||||
|
||||
if (!entry.settled) {
|
||||
settle(id, { code: null, error: 'cancelled', state: null })
|
||||
}
|
||||
|
||||
pending.delete(id)
|
||||
}
|
||||
|
||||
export function registerMcpOauthCallbackIpc() {
|
||||
// Bind a one-shot loopback listener; resolves { id, redirectUri }.
|
||||
ipcMain.handle('hermes:mcp-oauth:listen', async () => {
|
||||
if (pending.size >= MAX_PENDING_LISTENERS) {
|
||||
throw new Error('Too many MCP OAuth listeners are already pending')
|
||||
}
|
||||
|
||||
const id = String(nextId++)
|
||||
|
||||
const server = http.createServer((req, res) => {
|
||||
res.writeHead(200, { 'content-type': 'text/html; charset=utf-8' })
|
||||
res.end(DONE_HTML)
|
||||
|
||||
const url = req.url || '/'
|
||||
|
||||
// Ignore favicon and other noise — wait for the ?code= / ?error= hit.
|
||||
if (!/[?&](code|error)=/.test(url)) {
|
||||
return
|
||||
}
|
||||
|
||||
let code: null | string = null
|
||||
let state: null | string = null
|
||||
let error: null | string = null
|
||||
|
||||
try {
|
||||
const parsed = new URL(url, 'http://127.0.0.1')
|
||||
|
||||
code = parsed.searchParams.get('code')
|
||||
state = parsed.searchParams.get('state')
|
||||
error = parsed.searchParams.get('error')
|
||||
} catch {
|
||||
error = 'unparseable callback URL'
|
||||
}
|
||||
|
||||
settle(id, { code, error, state })
|
||||
})
|
||||
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
server.once('error', reject)
|
||||
server.listen(0, '127.0.0.1', () => resolve())
|
||||
})
|
||||
|
||||
const port = (server.address() as AddressInfo).port
|
||||
|
||||
pending.set(id, { result: null, server, settled: false, waiters: [] })
|
||||
|
||||
return { id, redirectUri: `http://127.0.0.1:${port}/callback` }
|
||||
})
|
||||
|
||||
// Resolve when the redirect arrives (or timeout). Safe to call once per id.
|
||||
ipcMain.handle('hermes:mcp-oauth:wait', async (_event, id, timeoutMs) => {
|
||||
const entry = pending.get(String(id || ''))
|
||||
|
||||
if (!entry) {
|
||||
return { code: null, error: 'listener not found', state: null }
|
||||
}
|
||||
|
||||
if (entry.result) {
|
||||
const result = entry.result
|
||||
|
||||
pending.delete(String(id))
|
||||
|
||||
return result
|
||||
}
|
||||
|
||||
const timeout = Math.min(Math.max(Number(timeoutMs) || DEFAULT_WAIT_TIMEOUT_MS, 1000), 15 * 60 * 1000)
|
||||
|
||||
const result = await new Promise<CallbackResult>(resolve => {
|
||||
const timer = setTimeout(() => {
|
||||
settle(String(id), { code: null, error: 'timeout waiting for OAuth callback', state: null })
|
||||
}, timeout)
|
||||
|
||||
entry.waiters.push(value => {
|
||||
clearTimeout(timer)
|
||||
resolve(value)
|
||||
})
|
||||
})
|
||||
|
||||
pending.delete(String(id))
|
||||
|
||||
return result
|
||||
})
|
||||
|
||||
// Tear a listener down without waiting (user cancelled, flow errored).
|
||||
ipcMain.handle('hermes:mcp-oauth:cancel', (_event, id) => {
|
||||
dispose(String(id || ''))
|
||||
|
||||
return true
|
||||
})
|
||||
}
|
||||
@@ -270,6 +270,14 @@ contextBridge.exposeInMainWorld('hermesDesktop', {
|
||||
setDisableF12: blocked => ipcRenderer.send('hermes:devtools:disable-f12', blocked),
|
||||
setPreviewShortcutActive: active => ipcRenderer.send('hermes:previewShortcutActive', Boolean(active)),
|
||||
openExternal: url => ipcRenderer.invoke('hermes:openExternal', url),
|
||||
mcpOauth: {
|
||||
// One-shot loopback listener for MCP OAuth against remote backends: bind
|
||||
// on this machine, hand redirectUri to mcp.servers.oauth.start, then wait
|
||||
// for the provider redirect and relay code/state via oauth.callback.
|
||||
listen: () => ipcRenderer.invoke('hermes:mcp-oauth:listen'),
|
||||
wait: (id, timeoutMs) => ipcRenderer.invoke('hermes:mcp-oauth:wait', id, timeoutMs),
|
||||
cancel: id => ipcRenderer.invoke('hermes:mcp-oauth:cancel', id)
|
||||
},
|
||||
openPreviewInBrowser: url => ipcRenderer.invoke('hermes:openPreviewInBrowser', url),
|
||||
reachPreviewUrl: url => ipcRenderer.invoke('hermes:preview:reach', url),
|
||||
setActiveConnectionRoute: route => ipcRenderer.send('hermes:connection:active-route', route),
|
||||
|
||||
Vendored
+9
@@ -300,6 +300,15 @@ declare global {
|
||||
setDisableF12?: (blocked: boolean) => void
|
||||
setPreviewShortcutActive?: (active: boolean) => void
|
||||
openExternal: (url: string) => Promise<void>
|
||||
/** 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. */
|
||||
mcpOauth?: {
|
||||
listen: () => Promise<{ id: string; redirectUri: string }>
|
||||
wait: (id: string, timeoutMs?: number) => Promise<{ code: null | string; error: null | string; state: null | string }>
|
||||
cancel: (id: string) => Promise<boolean>
|
||||
}
|
||||
openPreviewInBrowser?: (url: string) => Promise<void>
|
||||
fetchLinkTitle: (url: string) => Promise<string>
|
||||
/** A site's icon as a data URL, or '' when it has none we can read.
|
||||
|
||||
@@ -281,22 +281,100 @@ export function McpSetupButton({ profile, entry, onDone, ensureProfile }: McpSet
|
||||
}
|
||||
}
|
||||
|
||||
const start = await mcpRpc('mcp.servers.oauth.start', {
|
||||
// Client-side callback listener (electron/mcp-oauth-callback-ipc.ts): the
|
||||
// browser always runs on THIS machine, so hosting the OAuth redirect here
|
||||
// works for local AND remote backends alike. Against a remote backend it
|
||||
// is the only working flow — the gateway's own 127.0.0.1 listener is on
|
||||
// the backend host, unreachable from this machine's browser. Falls back
|
||||
// to the legacy gateway-listener flow when the bridge or the gateway-side
|
||||
// callback RPC is unavailable (older builds).
|
||||
const mcpOauthBridge =
|
||||
typeof window !== 'undefined' && window.hermesDesktop && window.hermesDesktop.mcpOauth
|
||||
? window.hermesDesktop.mcpOauth
|
||||
: null
|
||||
|
||||
let listener: { id: string; redirectUri: string } | null = null
|
||||
|
||||
if (mcpOauthBridge) {
|
||||
try {
|
||||
listener = await mcpOauthBridge.listen()
|
||||
} catch {
|
||||
listener = null
|
||||
}
|
||||
}
|
||||
|
||||
let start = await mcpRpc('mcp.servers.oauth.start', {
|
||||
profile,
|
||||
name: entry.name
|
||||
name: entry.name,
|
||||
...(listener ? { client_redirect_uri: listener.redirectUri } : {})
|
||||
})
|
||||
|
||||
// Older gateway rejecting the loopback URI shape (or a stale build that
|
||||
// validates differently): retry once on the legacy gateway-listener path.
|
||||
if (!start.ok && listener) {
|
||||
try {
|
||||
await mcpOauthBridge!.cancel(listener.id)
|
||||
} catch {
|
||||
/* listener teardown is best-effort */
|
||||
}
|
||||
|
||||
listener = null
|
||||
start = await mcpRpc('mcp.servers.oauth.start', {
|
||||
profile,
|
||||
name: entry.name
|
||||
})
|
||||
}
|
||||
|
||||
const payload = start.result && (start.result.result || start.result)
|
||||
const authUrl = payload && (payload.auth_url || payload.verification_url)
|
||||
const sessionId = payload && payload.session_id
|
||||
|
||||
if (!start.ok || !authUrl || !sessionId) {
|
||||
if (listener) {
|
||||
try {
|
||||
await mcpOauthBridge!.cancel(listener.id)
|
||||
} catch {
|
||||
/* listener teardown is best-effort */
|
||||
}
|
||||
}
|
||||
|
||||
setPhase('error')
|
||||
setMessage(start.error || 'Could not start OAuth')
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// With a client listener bound: await the provider redirect here and relay
|
||||
// code/state to the gateway. Runs concurrently with the status poll below;
|
||||
// errors surface through the poll (the gateway marks the flow failed).
|
||||
if (listener) {
|
||||
const listenerId = listener.id
|
||||
|
||||
void (async () => {
|
||||
const cb = await mcpOauthBridge!.wait(listenerId)
|
||||
|
||||
if (cb.error === 'cancelled') {
|
||||
return
|
||||
}
|
||||
|
||||
const relay = await mcpRpc('mcp.servers.oauth.callback', {
|
||||
profile,
|
||||
name: entry.name,
|
||||
session_id: sessionId,
|
||||
code: cb.code || undefined,
|
||||
state: cb.state || undefined,
|
||||
error: cb.error || undefined
|
||||
})
|
||||
|
||||
const rp = relay.result && (relay.result.result || relay.result)
|
||||
|
||||
if (!relay.ok || (rp && rp.ok === false)) {
|
||||
setPhase('error')
|
||||
setMessage((rp && rp.error_message) || relay.error || 'OAuth callback relay failed')
|
||||
}
|
||||
})()
|
||||
}
|
||||
|
||||
// Open the auth URL in the native browser, same as provider OAuth.
|
||||
// TODO(bot-mode-types): the plugin SDK's `host` has no `openExternal`, so this
|
||||
// branch is dead and the window bridge / window.open fallbacks are the only
|
||||
|
||||
@@ -0,0 +1,231 @@
|
||||
"""Tests for the client-side callback (remote-backend) variant of the
|
||||
session-backed MCP OAuth flow (tui_gateway/mcp_oauth_sessions.py).
|
||||
|
||||
Covers the three seams added for remote Desktop backends:
|
||||
- _validate_client_redirect_uri: loopback-only allowlist for the
|
||||
client-supplied redirect URI (rejects public hosts/schemes so a gateway
|
||||
never pins an attacker-controlled redirect into a DCR registration);
|
||||
- start_flow(client_redirect_uri=...): no gateway-side listener is bound and
|
||||
the flow's redirect_uri is pinned to the client's listener;
|
||||
- deliver_callback_flow: relays a client-captured code/state into the flow
|
||||
with the SAME state verification as the loopback path (wrong state
|
||||
rejected, replay rejected, unknown session rejected).
|
||||
"""
|
||||
|
||||
import threading
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.mcp_dashboard_oauth import DashboardOAuthFlow
|
||||
from tui_gateway import mcp_oauth_sessions
|
||||
from tui_gateway.mcp_oauth_sessions import (
|
||||
_validate_client_redirect_uri,
|
||||
deliver_callback_flow,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _validate_client_redirect_uri
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"uri,expected",
|
||||
[
|
||||
("http://127.0.0.1:8412/callback", "http://127.0.0.1:8412/callback"),
|
||||
("http://localhost:60000/callback", "http://localhost:60000/callback"),
|
||||
# Path defaulting
|
||||
("http://127.0.0.1:9999", "http://127.0.0.1:9999/callback"),
|
||||
# Surrounding whitespace tolerated
|
||||
(" http://127.0.0.1:8412/callback ", "http://127.0.0.1:8412/callback"),
|
||||
],
|
||||
)
|
||||
def test_validate_accepts_loopback_http(uri, expected):
|
||||
assert _validate_client_redirect_uri(uri) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"uri",
|
||||
[
|
||||
"https://127.0.0.1:8412/callback", # https is not a native loopback
|
||||
"http://evil.example.com:8412/callback", # public host
|
||||
"http://192.168.1.10:8412/callback", # LAN host
|
||||
"http://127.0.0.1/callback", # no port
|
||||
"http://user:pass@127.0.0.1:8412/callback", # credentials
|
||||
"javascript:alert(1)",
|
||||
"",
|
||||
"not a url",
|
||||
],
|
||||
)
|
||||
def test_validate_rejects_non_loopback(uri):
|
||||
with pytest.raises(ValueError):
|
||||
_validate_client_redirect_uri(uri)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# start_flow with client_redirect_uri: no gateway listener, URI pinned
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _fake_worker_publishes_url(monkeypatch, state="teststate123"):
|
||||
"""Replace the OAuth worker with a stub that publishes an authorize URL
|
||||
carrying *state* and then waits for the callback like the real worker's
|
||||
SDK does."""
|
||||
|
||||
def worker(session_id, hermes_home, server_name, cfg, reconnect_live):
|
||||
rec = mcp_oauth_sessions._sessions.get(session_id)
|
||||
flow = rec["flow"]
|
||||
import asyncio
|
||||
|
||||
asyncio.run(
|
||||
flow.publish_authorization_url(
|
||||
f"https://as.example.com/authorize?client_id=x&state={state}"
|
||||
)
|
||||
)
|
||||
# Wait for the callback (delivered by the test), then approve.
|
||||
try:
|
||||
asyncio.run(flow.wait_for_callback(timeout=5))
|
||||
flow.mark_approved()
|
||||
except Exception as exc: # pragma: no cover - failure surface
|
||||
flow.mark_error(str(exc))
|
||||
finally:
|
||||
flow.mark_worker_done()
|
||||
|
||||
monkeypatch.setattr(mcp_oauth_sessions, "_worker", worker)
|
||||
|
||||
|
||||
def test_start_flow_client_redirect_skips_gateway_listener(monkeypatch):
|
||||
_fake_worker_publishes_url(monkeypatch)
|
||||
|
||||
bound = []
|
||||
real_listener = mcp_oauth_sessions._start_loopback_listener
|
||||
monkeypatch.setattr(
|
||||
mcp_oauth_sessions,
|
||||
"_start_loopback_listener",
|
||||
lambda flow: bound.append(flow) or real_listener(flow),
|
||||
)
|
||||
|
||||
result = mcp_oauth_sessions.start_flow(
|
||||
"/tmp/hermes-test-home",
|
||||
"clicky",
|
||||
{"url": "https://mcp.example.com/mcp", "auth": "oauth"},
|
||||
client_redirect_uri="http://127.0.0.1:8412/callback",
|
||||
)
|
||||
|
||||
assert result["session_id"]
|
||||
assert result["auth_url"].startswith("https://as.example.com/authorize")
|
||||
assert bound == [], "gateway listener must NOT be bound with a client redirect"
|
||||
|
||||
rec = mcp_oauth_sessions._sessions[result["session_id"]]
|
||||
assert rec["httpd"] is None
|
||||
assert rec["flow"].redirect_uri == "http://127.0.0.1:8412/callback"
|
||||
|
||||
# Cleanup: deliver the callback so the stub worker thread exits.
|
||||
deliver_callback_flow(
|
||||
result["session_id"], "clicky", code="authcode", state="teststate123"
|
||||
)
|
||||
rec["flow"]._worker_done.wait(5)
|
||||
|
||||
|
||||
def test_start_flow_rejects_bad_client_redirect(monkeypatch):
|
||||
_fake_worker_publishes_url(monkeypatch)
|
||||
with pytest.raises(ValueError):
|
||||
mcp_oauth_sessions.start_flow(
|
||||
"/tmp/hermes-test-home",
|
||||
"clicky2",
|
||||
{"url": "https://mcp.example.com/mcp", "auth": "oauth"},
|
||||
client_redirect_uri="https://evil.example.com/callback",
|
||||
)
|
||||
# No session must be left behind by a rejected start.
|
||||
assert all(
|
||||
r["server_name"] != "clicky2" for r in mcp_oauth_sessions._sessions.values()
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# deliver_callback_flow: relay accept/reject semantics
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_session(session_id="sess-relay-1", server="hosp", state="s3cr3tstate"):
|
||||
flow = DashboardOAuthFlow(
|
||||
flow_id=session_id,
|
||||
server_name=server,
|
||||
profile=None,
|
||||
hermes_home="/tmp/hermes-test-home",
|
||||
redirect_uri="http://127.0.0.1:9000/callback",
|
||||
)
|
||||
# Pin the expected state the way publish_authorization_url does.
|
||||
import asyncio
|
||||
|
||||
asyncio.run(
|
||||
flow.publish_authorization_url(
|
||||
f"https://as.example.com/authorize?state={state}"
|
||||
)
|
||||
)
|
||||
rec = {
|
||||
"session_id": session_id,
|
||||
"server_name": server,
|
||||
"hermes_home": "/tmp/hermes-test-home",
|
||||
"flow": flow,
|
||||
"httpd": None,
|
||||
"created_at": __import__("time").time(),
|
||||
}
|
||||
with mcp_oauth_sessions._sessions_lock:
|
||||
mcp_oauth_sessions._sessions[session_id] = rec
|
||||
return flow
|
||||
|
||||
|
||||
def teardown_function(_fn):
|
||||
with mcp_oauth_sessions._sessions_lock:
|
||||
mcp_oauth_sessions._sessions.clear()
|
||||
|
||||
|
||||
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")
|
||||
|
||||
|
||||
def test_deliver_callback_rejects_state_mismatch():
|
||||
_make_session()
|
||||
out = deliver_callback_flow("sess-relay-1", "hosp", code="abc", state="WRONG")
|
||||
assert out["ok"] is False
|
||||
assert "state" in out["error_message"].lower()
|
||||
|
||||
|
||||
def test_deliver_callback_rejects_replay():
|
||||
_make_session()
|
||||
first = deliver_callback_flow(
|
||||
"sess-relay-1", "hosp", code="abc", state="s3cr3tstate"
|
||||
)
|
||||
assert first["ok"] is True
|
||||
second = deliver_callback_flow(
|
||||
"sess-relay-1", "hosp", code="abc", state="s3cr3tstate"
|
||||
)
|
||||
assert second["ok"] is False
|
||||
|
||||
|
||||
def test_deliver_callback_unknown_session_and_name_mismatch():
|
||||
_make_session()
|
||||
assert deliver_callback_flow("nope", "hosp", code="a", state="s")["ok"] is False
|
||||
out = deliver_callback_flow("sess-relay-1", "other-server", code="a", state="s")
|
||||
assert out["ok"] is False
|
||||
assert "mismatch" in out["error_message"]
|
||||
|
||||
|
||||
def test_deliver_callback_propagates_provider_error():
|
||||
flow = _make_session()
|
||||
out = deliver_callback_flow(
|
||||
"sess-relay-1", "hosp", code=None, state="s3cr3tstate", error="access_denied"
|
||||
)
|
||||
assert out["ok"] is True # accepted; the flow records the provider error
|
||||
|
||||
async def _check():
|
||||
with pytest.raises(RuntimeError, match="access_denied"):
|
||||
await flow.wait_for_callback(timeout=1)
|
||||
|
||||
import asyncio
|
||||
|
||||
asyncio.run(_check())
|
||||
@@ -27,6 +27,18 @@ Client contract (what the desktop plugin does):
|
||||
2. open ``auth_url`` in the native browser (``openExternal``)
|
||||
3. poll ``mcp.servers.oauth.poll(profile, name, session_id)`` until
|
||||
``status == "approved"`` (tokens persisted) or ``"error"``.
|
||||
|
||||
Remote-backend variant (client-side callback): when the desktop app runs on a
|
||||
DIFFERENT machine than the gateway (SSH/Tailscale remote backend), the
|
||||
gateway-side ``127.0.0.1`` listener is unreachable from the user's browser —
|
||||
the redirect lands on the user's machine where nothing is listening, and the
|
||||
flow times out. For that topology the client binds its OWN loopback listener
|
||||
(same pattern as the desktop's native gateway login), passes its
|
||||
``redirect_uri`` to ``start`` (``client_redirect_uri``), and relays the
|
||||
provider redirect back via ``deliver_callback_flow`` /
|
||||
``mcp.servers.oauth.callback``. State verification stays server-side in
|
||||
``DashboardOAuthFlow.deliver_callback`` — a relayed code with the wrong
|
||||
``state`` is rejected exactly like a forged loopback hit.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -75,6 +87,31 @@ def _shutdown_listener(rec: Dict[str, Any]) -> None:
|
||||
rec["httpd"] = None
|
||||
|
||||
|
||||
def _validate_client_redirect_uri(uri: str) -> str:
|
||||
"""Validate a client-supplied loopback redirect URI.
|
||||
|
||||
Only plain-http loopback URLs are accepted (``http://127.0.0.1:<port>/...``
|
||||
or ``http://localhost:<port>/...``), mirroring RFC 8252 native-app rules —
|
||||
the client hosts a one-shot listener on ITS machine, so anything else
|
||||
(public hosts, https proxies, schemes) is rejected to keep the gateway from
|
||||
pinning an attacker-controlled redirect into a DCR registration.
|
||||
"""
|
||||
parsed = urlparse(str(uri or "").strip())
|
||||
host = (parsed.hostname or "").lower()
|
||||
if (
|
||||
parsed.scheme != "http"
|
||||
or host not in ("127.0.0.1", "localhost", "::1")
|
||||
or not parsed.port
|
||||
or parsed.username is not None
|
||||
or parsed.password is not None
|
||||
):
|
||||
raise ValueError(
|
||||
"client_redirect_uri must be a loopback http URL like "
|
||||
"http://127.0.0.1:<port>/callback"
|
||||
)
|
||||
return f"http://{'[' + host + ']' if ':' in host else host}:{parsed.port}{parsed.path or '/callback'}"
|
||||
|
||||
|
||||
def _start_loopback_listener(flow) -> "http.server.HTTPServer":
|
||||
"""Bind a loopback callback listener that feeds the flow's deliver_callback.
|
||||
|
||||
@@ -217,6 +254,7 @@ def start_flow(
|
||||
*,
|
||||
reconnect_live: bool = False,
|
||||
url_timeout: float = 30.0,
|
||||
client_redirect_uri: Optional[str] = None,
|
||||
) -> Dict[str, Any]:
|
||||
"""Begin an MCP OAuth flow and return ``{session_id, auth_url, flow}``.
|
||||
|
||||
@@ -224,9 +262,18 @@ def start_flow(
|
||||
OAuth-capable). ``hermes_home`` is the already-resolved profile home dir
|
||||
string. Blocks up to ``url_timeout`` for the worker to publish the browser
|
||||
authorization URL, then returns it.
|
||||
|
||||
``client_redirect_uri`` (remote-backend variant): a loopback callback URL
|
||||
the CLIENT hosts on its own machine. When set (and valid), no gateway-side
|
||||
listener is bound — the OAuth ``redirect_uri`` is pinned to the client's
|
||||
listener, and the client relays the redirect's ``code``/``state`` back via
|
||||
``deliver_callback_flow``. Invalid values raise ``ValueError``.
|
||||
"""
|
||||
from tools.mcp_dashboard_oauth import DashboardOAuthFlow
|
||||
|
||||
if client_redirect_uri is not None:
|
||||
client_redirect_uri = _validate_client_redirect_uri(client_redirect_uri)
|
||||
|
||||
_gc_sessions()
|
||||
|
||||
with _sessions_lock:
|
||||
@@ -254,9 +301,17 @@ def start_flow(
|
||||
redirect_uri="", # set below once the loopback port is known
|
||||
reconnect_live=reconnect_live,
|
||||
)
|
||||
httpd = _start_loopback_listener(flow)
|
||||
port = httpd.server_address[1]
|
||||
flow.redirect_uri = f"http://127.0.0.1:{port}/callback"
|
||||
if client_redirect_uri:
|
||||
# Remote-backend variant: the CLIENT hosts the callback listener on its
|
||||
# own machine and relays the code via deliver_callback_flow(). No
|
||||
# gateway-side listener is bound — a 127.0.0.1 port here would be
|
||||
# unreachable from the user's browser anyway.
|
||||
httpd = None
|
||||
flow.redirect_uri = client_redirect_uri
|
||||
else:
|
||||
httpd = _start_loopback_listener(flow)
|
||||
port = httpd.server_address[1]
|
||||
flow.redirect_uri = f"http://127.0.0.1:{port}/callback"
|
||||
|
||||
rec = {
|
||||
"session_id": session_id,
|
||||
@@ -337,3 +392,38 @@ def poll_flow(session_id: str, server_name: str) -> Dict[str, Any]:
|
||||
if status == "approved":
|
||||
out["tools"] = list(getattr(flow, "tools", []) or [])
|
||||
return out
|
||||
|
||||
|
||||
def deliver_callback_flow(
|
||||
session_id: str,
|
||||
server_name: str,
|
||||
*,
|
||||
code: Optional[str],
|
||||
state: Optional[str],
|
||||
error: 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=...)``: the
|
||||
desktop app's loopback listener caught the provider redirect on the USER'S
|
||||
machine and forwards ``code``/``state`` (or ``error``) here. Security
|
||||
properties are unchanged from the gateway-listener path — the underlying
|
||||
``DashboardOAuthFlow.deliver_callback`` verifies ``state`` against the
|
||||
pinned authorization request (constant-time compare) and rejects replays,
|
||||
so a forged or replayed relay fails identically to a forged loopback hit.
|
||||
|
||||
Returns ``{ok: true}`` on acceptance or ``{ok: false, error_message}``.
|
||||
"""
|
||||
with _sessions_lock:
|
||||
rec = _sessions.get(session_id)
|
||||
if rec is None:
|
||||
return {"ok": False, "error_message": "OAuth session not found or expired"}
|
||||
if rec["server_name"] != server_name:
|
||||
return {"ok": False, "error_message": "server name mismatch for session"}
|
||||
|
||||
flow = rec["flow"]
|
||||
try:
|
||||
flow.deliver_callback(code=code, state=state, error=error)
|
||||
except ValueError as exc:
|
||||
return {"ok": False, "error_message": str(exc)}
|
||||
return {"ok": True, "session_id": session_id}
|
||||
|
||||
@@ -2322,7 +2322,8 @@ def _(rid, params: dict) -> dict:
|
||||
def _(rid, params: dict) -> dict:
|
||||
"""Begin a session-backed OAuth flow for an MCP server in a profile.
|
||||
|
||||
Params: optional ``profile``, ``name`` (required). Result:
|
||||
Params: optional ``profile``, ``name`` (required), optional
|
||||
``client_redirect_uri``. Result:
|
||||
``{ok: true, session_id, auth_url, flow: "pkce"}``.
|
||||
|
||||
The client (desktop) opens ``auth_url`` in the native browser
|
||||
@@ -2334,12 +2335,21 @@ def _(rid, params: dict) -> dict:
|
||||
(``_probe_single_server`` under ``force_interactive_oauth``), and a loopback
|
||||
listener captures the browser redirect — no FastAPI request object needed.
|
||||
|
||||
``client_redirect_uri`` (remote backends): a loopback URL the CLIENT hosts
|
||||
on its own machine (``http://127.0.0.1:<port>/callback``). When supplied,
|
||||
the gateway binds NO listener — the provider redirects to the client's
|
||||
listener and the client relays the code via ``mcp.servers.oauth.callback``.
|
||||
This is the only flow that works when the desktop app and the gateway run
|
||||
on different machines (SSH/Tailscale remote backend), where the gateway's
|
||||
own 127.0.0.1 listener is unreachable from the user's browser.
|
||||
|
||||
Runs on the RPC thread pool (see _LONG_HANDLERS): start blocks briefly for
|
||||
the authorization URL to be published.
|
||||
"""
|
||||
name = str(params.get("name") or "").strip()
|
||||
if not name:
|
||||
return _err(rid, 4063, "name required")
|
||||
client_redirect_uri = str(params.get("client_redirect_uri") or "").strip() or None
|
||||
token, err = _mcp_resolve_profile(rid, params)
|
||||
if err:
|
||||
return err
|
||||
@@ -2363,7 +2373,9 @@ def _(rid, params: dict) -> dict:
|
||||
cfg["auth"] = "oauth"
|
||||
|
||||
hermes_home = str(get_hermes_home().expanduser().resolve(strict=False))
|
||||
result = mcp_oauth_sessions.start_flow(hermes_home, name, cfg)
|
||||
result = mcp_oauth_sessions.start_flow(
|
||||
hermes_home, name, cfg, client_redirect_uri=client_redirect_uri
|
||||
)
|
||||
return _ok(
|
||||
rid,
|
||||
{
|
||||
@@ -2373,6 +2385,8 @@ def _(rid, params: dict) -> dict:
|
||||
"flow": result["flow"],
|
||||
},
|
||||
)
|
||||
except ValueError as e:
|
||||
return _err(rid, 4001, str(e))
|
||||
except Exception as e:
|
||||
return _err(rid, 5024, str(e))
|
||||
finally:
|
||||
@@ -2412,6 +2426,44 @@ def _(rid, params: dict) -> dict:
|
||||
_mcp_reset_profile(token)
|
||||
|
||||
|
||||
@method("mcp.servers.oauth.callback")
|
||||
def _(rid, params: dict) -> dict:
|
||||
"""Relay a client-captured OAuth redirect into a running MCP OAuth flow.
|
||||
|
||||
Remote-backend companion to ``mcp.servers.oauth.start`` with
|
||||
``client_redirect_uri``: the desktop app's local loopback listener caught
|
||||
the provider redirect on the user's machine and forwards its query params
|
||||
here. Params: optional ``profile``, ``name`` (required), ``session_id``
|
||||
(required), ``code``, ``state``, ``error``. Result: ``{ok: true}`` once the
|
||||
callback is accepted (state verified inside the flow bridge), or
|
||||
``{ok: false, error_message}`` on mismatch/expiry.
|
||||
"""
|
||||
name = str(params.get("name") or "").strip()
|
||||
if not name:
|
||||
return _err(rid, 4063, "name required")
|
||||
session_id = str(params.get("session_id") or "").strip()
|
||||
if not session_id:
|
||||
return _err(rid, 4063, "session_id required")
|
||||
token, err = _mcp_resolve_profile(rid, params)
|
||||
if err:
|
||||
return err
|
||||
try:
|
||||
from tui_gateway import mcp_oauth_sessions
|
||||
|
||||
result = mcp_oauth_sessions.deliver_callback_flow(
|
||||
session_id,
|
||||
name,
|
||||
code=str(params.get("code") or "") or None,
|
||||
state=str(params.get("state") or "") or None,
|
||||
error=str(params.get("error") or "") or None,
|
||||
)
|
||||
return _ok(rid, result)
|
||||
except Exception as e:
|
||||
return _err(rid, 5024, str(e))
|
||||
finally:
|
||||
_mcp_reset_profile(token)
|
||||
|
||||
|
||||
@method("skills.reload")
|
||||
def _(rid, params: dict) -> dict:
|
||||
try:
|
||||
|
||||
@@ -271,8 +271,9 @@ mcp_servers:
|
||||
|
||||
On first connect, Hermes prints an authorize URL, opens your browser when possible, and waits for the OAuth callback on a local loopback port. Tokens are cached at `~/.hermes/mcp-tokens/<server>.json` with 0o600 perms; subsequent runs reuse them silently until refresh fails.
|
||||
|
||||
**Remote / headless hosts.** When Hermes runs on a different machine than your browser, the loopback callback can't reach your laptop. Two ways to complete the flow:
|
||||
**Remote / headless hosts.** When Hermes runs on a different machine than your browser, the loopback callback can't reach your laptop. Ways to complete the flow:
|
||||
|
||||
- **Hermes Desktop (automatic):** when you run the OAuth sign-in from the Desktop app's MCP setup UI against a remote backend, Desktop hosts the callback listener on *your* machine and relays the authorization back to the gateway automatically — no tunnel, paste, or proxy needed. Requires both the Desktop app and the backend to be up to date.
|
||||
- **Paste-back (no setup):** on an interactive terminal Hermes prints "Or paste the redirect URL here…" alongside the authorize URL. Open the URL in your browser, approve, copy the full URL the browser ends up on (the redirect will show a connection error — that's expected), paste it at the prompt. Bare `?code=…&state=…` query strings work too.
|
||||
- **SSH port forward:** `ssh -N -L <port>:127.0.0.1:<port> user@host` in a separate terminal, then let the redirect flow normally.
|
||||
- **Proxied callback (`redirect_uri`):** when a public HTTPS endpoint forwards to the host (e.g. a Tailscale Funnel or reverse proxy pointed at the callback port), set `oauth.redirect_uri` and the browser redirect reaches Hermes on its own — no tunnel or paste needed:
|
||||
|
||||
Reference in New Issue
Block a user