From 3e24b16f566045399012bc1185fe0cdb6e1a1be9 Mon Sep 17 00:00:00 2001 From: Shannon Sands Date: Thu, 9 Jul 2026 14:11:11 +1000 Subject: [PATCH] fix(dashboard): support mobile OAuth login --- hermes_cli/dashboard_auth/middleware.py | 6 +- hermes_cli/dashboard_auth/routes.py | 8 ++ hermes_cli/web_server.py | 50 +++++++- .../test_dashboard_auth_password_login.py | 18 +++ tests/hermes_cli/test_web_oauth_dispatch.py | 50 ++++++++ web/src/components/OAuthLoginModal.tsx | 78 +++++++----- web/src/i18n/en.ts | 7 +- web/src/i18n/types.ts | 2 + web/src/lib/api.test.ts | 82 +++++++++++-- web/src/lib/api.ts | 68 +++-------- web/src/lib/clipboard.test.ts | 114 ++++++++++++++++++ web/src/lib/clipboard.ts | 56 +++++++++ 12 files changed, 446 insertions(+), 93 deletions(-) create mode 100644 web/src/lib/clipboard.test.ts create mode 100644 web/src/lib/clipboard.ts diff --git a/hermes_cli/dashboard_auth/middleware.py b/hermes_cli/dashboard_auth/middleware.py index 2c5f5b4f7b9..362ed729d65 100644 --- a/hermes_cli/dashboard_auth/middleware.py +++ b/hermes_cli/dashboard_auth/middleware.py @@ -150,6 +150,8 @@ def _auto_sso_response(request: Request) -> Response | None: * exactly ONE interactive provider is registered — with two or more we can't pick for the user, so the ``/login`` chooser must render; with zero there's nothing to redirect to; + * that provider is OAuth-style, not a password form provider. Password + providers must render ``/login`` so the user can enter credentials; * the one-shot loop-guard marker is ABSENT. Its presence means we already bounced to the portal once and came back still unauthenticated (no portal session) — auto-redirecting again would @@ -185,6 +187,9 @@ def _auto_sso_response(request: Request) -> Response | None: from hermes_cli.dashboard_auth.prefix import prefix_from_request provider = providers[0] + if getattr(provider, "supports_password", False): + return None + prefix = prefix_from_request(request) next_param = _safe_next_target(request) from urllib.parse import quote @@ -458,4 +463,3 @@ def _attempt_refresh(request: Request, *, refresh_token): if new_session is not None: return new_session, provider.name return None - diff --git a/hermes_cli/dashboard_auth/routes.py b/hermes_cli/dashboard_auth/routes.py index 9e80f2583c5..ee595be7d82 100644 --- a/hermes_cli/dashboard_auth/routes.py +++ b/hermes_cli/dashboard_auth/routes.py @@ -192,6 +192,14 @@ async def auth_login(request: Request, provider: str, next: str = ""): status_code=404, detail=f"Provider does not support interactive login: {provider!r}", ) + if getattr(p, "supports_password", False): + from urllib.parse import quote + + safe_next = _validate_post_login_target(next) + login_url = f"{_prefix(request)}/login" + if safe_next: + login_url = f"{login_url}?next={quote(safe_next, safe='')}" + return RedirectResponse(url=login_url, status_code=302) try: ls = p.start_login(redirect_uri=_redirect_uri(request)) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index db6b6a1ec8d..fa270747528 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -9087,6 +9087,54 @@ def _xai_device_poller(session_id: str) -> None: sess["error_message"] = str(e) +def _http_response_error_detail(resp: Any) -> str: + """Best-effort extraction of a short provider error detail.""" + try: + payload = resp.json() + except Exception: + payload = None + if isinstance(payload, dict): + error = payload.get("error") + if isinstance(error, dict): + parts = [ + str(error.get(key, "")).strip() + for key in ("message", "error_description", "code", "type") + if str(error.get(key, "")).strip() + ] + if parts: + return ": ".join(parts) + if isinstance(error, str) and error.strip(): + return error.strip() + for key in ("detail", "message", "error_description"): + value = payload.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + text = str(getattr(resp, "text", "") or "").strip() + return text[:500] + + +def _codex_device_code_start_error(resp: Any) -> str: + """Dashboard-facing OpenAI Codex device-code start failure.""" + status = getattr(resp, "status_code", "unknown") + detail = _http_response_error_detail(resp) + lower = detail.lower() + if "device" in lower and ("authori" in lower or "enable" in lower): + message = ( + "OpenAI rejected the device-code login request. Your OpenAI " + "account may need device-code authorization enabled before Hermes " + "can start this dashboard login. Enable device-code authorization " + "in OpenAI, then return here and click Login again." + ) + else: + message = ( + "OpenAI rejected the device-code login request. Please try Login " + "again from the dashboard after checking your OpenAI account settings." + ) + if detail: + return f"{message} (HTTP {status}: {detail})" + return f"{message} (HTTP {status})" + + def _codex_full_login_worker(session_id: str) -> None: """Run the complete OpenAI Codex device-code flow. @@ -9119,7 +9167,7 @@ def _codex_full_login_worker(session_id: str) -> None: headers={"Content-Type": "application/json"}, ) if resp.status_code != 200: - raise RuntimeError(f"deviceauth/usercode returned {resp.status_code}") + raise RuntimeError(_codex_device_code_start_error(resp)) device_data = resp.json() user_code = device_data.get("user_code", "") device_auth_id = device_data.get("device_auth_id", "") diff --git a/tests/hermes_cli/test_dashboard_auth_password_login.py b/tests/hermes_cli/test_dashboard_auth_password_login.py index b9508c34e49..1fa59480115 100644 --- a/tests/hermes_cli/test_dashboard_auth_password_login.py +++ b/tests/hermes_cli/test_dashboard_auth_password_login.py @@ -198,6 +198,24 @@ class TestProviderListFlag: prov = {p["name"]: p for p in resp.json()["providers"]} assert prov["testpw"]["supports_password"] is True + def test_password_provider_html_redirects_to_login_form(self, gated_app): + resp = gated_app.get("/", follow_redirects=False) + assert resp.status_code == 302 + assert resp.headers["location"] == "/login?next=%2F" + + login = gated_app.get(resp.headers["location"]) + assert login.status_code == 200 + assert '
(null); const [pkceCode, setPkceCode] = useState(""); const [errorMsg, setErrorMsg] = useState(null); + const [copyStatus, setCopyStatus] = useState<"idle" | "copied" | "failed">( + "idle", + ); const [secondsLeft, setSecondsLeft] = useState(null); const isMounted = useRef(true); const pollTimer = useRef(null); + const copyResetTimer = useRef(null); const { t } = useI18n(); // Initiate flow on mount @@ -59,6 +63,8 @@ export function OAuthLoginModal({ provider, onClose, onSuccess }: Props) { return () => { isMounted.current = false; if (pollTimer.current !== null) window.clearInterval(pollTimer.current); + if (copyResetTimer.current !== null) + window.clearTimeout(copyResetTimer.current); }; // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -162,6 +168,24 @@ export function OAuthLoginModal({ provider, onClose, onSuccess }: Props) { return `${m}:${String(r).padStart(2, "0")}`; }; + const handleCopyDeviceCode = async (code: string) => { + if (copyResetTimer.current !== null) { + window.clearTimeout(copyResetTimer.current); + copyResetTimer.current = null; + } + const copied = await copyTextToClipboard(code); + if (!isMounted.current) return; + setCopyStatus(copied ? "copied" : "failed"); + copyResetTimer.current = window.setTimeout(() => { + if (isMounted.current) setCopyStatus("idle"); + copyResetTimer.current = null; + }, 2000); + }; + + const deviceCode = start?.flow === "device_code" ? start.user_code : ""; + const verificationUrl = + start?.flow === "device_code" ? start.verification_url : ""; + return (
- { - ( - start as Extract< - OAuthStartResponse, - { flow: "device_code" } - > - ).user_code - } + {deviceCode} - - ).user_code +
+ {copyStatus === "failed" && ( +

+ {t.oauth.copyFailed ?? + "Could not copy automatically. Select the code and copy it manually."} +

+ )} - ).verification_url - } + href={verificationUrl} target="_blank" rel="noopener noreferrer" className="text-xs text-muted-foreground hover:text-foreground inline-flex items-center gap-1" diff --git a/web/src/i18n/en.ts b/web/src/i18n/en.ts index e2ba0cc0369..a8be9310835 100644 --- a/web/src/i18n/en.ts +++ b/web/src/i18n/en.ts @@ -501,16 +501,19 @@ export const en: Translations = { oauth: { title: "Provider Logins (OAuth)", providerLogins: "Provider Logins (OAuth)", - description: "{connected} of {total} OAuth providers connected. Login flows currently run via the CLI; click Copy command and paste into a terminal to set up.", + description: + "{connected} of {total} OAuth providers connected. Use Login for dashboard-supported flows; CLI commands remain available for external or fallback setup.", connected: "Connected", expired: "Expired", - notConnected: "Not connected. Run {command} in a terminal.", + notConnected: "Not connected. Use Login when available, or run {command} in a terminal.", runInTerminal: "in a terminal.", noProviders: "No OAuth-capable providers detected.", login: "Login", disconnect: "Disconnect", managedExternally: "Managed externally", copied: "Copied ✓", + copyCode: "Copy code", + copyFailed: "Could not copy automatically. Select the code and copy it manually.", cli: "Copy", copyCliCommand: "Copy CLI command (for external / fallback)", connect: "Connect", diff --git a/web/src/i18n/types.ts b/web/src/i18n/types.ts index a93f921cadf..9898891dc04 100644 --- a/web/src/i18n/types.ts +++ b/web/src/i18n/types.ts @@ -530,6 +530,8 @@ export interface Translations { disconnect: string; managedExternally: string; copied: string; + copyCode?: string; + copyFailed?: string; cli: string; copyCliCommand: string; connect: string; diff --git a/web/src/lib/api.test.ts b/web/src/lib/api.test.ts index 4da9234ac5d..4d63d51a01a 100644 --- a/web/src/lib/api.test.ts +++ b/web/src/lib/api.test.ts @@ -2,21 +2,28 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { api } from "./api"; +const SESSION_HEADER = "X-Hermes-Session-Token"; + afterEach(() => { vi.restoreAllMocks(); vi.unstubAllGlobals(); }); +function jsonFetchMock(body: unknown = { ok: true }) { + return vi.fn( + async () => + new Response(JSON.stringify(body), { + headers: { "Content-Type": "application/json" }, + status: 200, + }), + ); +} + describe("api.getModelOptions", () => { it("requests a live model refresh when asked", async () => { vi.stubGlobal("window", {}); - const fetchMock = vi.fn(async () => - new Response(JSON.stringify({ providers: [] }), { - headers: { "Content-Type": "application/json" }, - status: 200, - }), - ); + const fetchMock = jsonFetchMock({ providers: [] }); vi.stubGlobal("fetch", fetchMock); await api.getModelOptions({ refresh: true }); @@ -30,12 +37,7 @@ describe("api.getModelOptions", () => { it("keeps explicit profile scoping when refreshing", async () => { vi.stubGlobal("window", {}); - const fetchMock = vi.fn(async () => - new Response(JSON.stringify({ providers: [] }), { - headers: { "Content-Type": "application/json" }, - status: 200, - }), - ); + const fetchMock = jsonFetchMock({ providers: [] }); vi.stubGlobal("fetch", fetchMock); await api.getModelOptions({ profile: "default", refresh: true }); @@ -46,3 +48,59 @@ describe("api.getModelOptions", () => { ); }); }); + +describe("api OAuth helpers", () => { + it("starts OAuth login in gated mode without requiring an injected session token", async () => { + vi.stubGlobal("window", { __HERMES_AUTH_REQUIRED__: true }); + const fetchMock = jsonFetchMock({ + flow: "device_code", + session_id: "oauth-session", + }); + vi.stubGlobal("fetch", fetchMock); + + await api.startOAuthLogin("openai-codex"); + + expect(fetchMock).toHaveBeenCalledWith( + "/api/providers/oauth/openai-codex/start", + expect.objectContaining({ + body: "{}", + credentials: "include", + method: "POST", + }), + ); + const headers = fetchMock.mock.calls[0][1]?.headers as Headers; + expect(headers.get("Content-Type")).toBe("application/json"); + expect(headers.has(SESSION_HEADER)).toBe(false); + }); + + it("still sends the injected session token for OAuth login in loopback mode", async () => { + vi.stubGlobal("window", { __HERMES_SESSION_TOKEN__: "loopback-token" }); + const fetchMock = jsonFetchMock({ + flow: "device_code", + session_id: "oauth-session", + }); + vi.stubGlobal("fetch", fetchMock); + + await api.startOAuthLogin("openai-codex"); + + const headers = fetchMock.mock.calls[0][1]?.headers as Headers; + expect(headers.get(SESSION_HEADER)).toBe("loopback-token"); + }); + + it("runs provider auth mutations in gated mode via cookie auth", async () => { + vi.stubGlobal("window", { __HERMES_AUTH_REQUIRED__: true }); + const fetchMock = jsonFetchMock({ ok: true }); + vi.stubGlobal("fetch", fetchMock); + + await api.disconnectOAuthProvider("anthropic"); + await api.submitOAuthCode("anthropic", "oauth-session", "code-123"); + await api.cancelOAuthSession("oauth-session"); + await api.revealEnvVar("OPENAI_API_KEY"); + + for (const call of fetchMock.mock.calls) { + const init = call[1] as RequestInit; + expect(init.credentials).toBe("include"); + expect((init.headers as Headers).has(SESSION_HEADER)).toBe(false); + } + }); +}); diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index e6320812982..457eb10bb93 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -34,7 +34,6 @@ declare global { __HERMES_AUTH_REQUIRED__?: boolean; } } -let _sessionToken: string | null = null; const SESSION_HEADER = "X-Hermes-Session-Token"; function setSessionHeader(headers: Headers, token: string): void { @@ -197,16 +196,6 @@ function pluginPath(name: string): string { return name.split("/").map(encodeURIComponent).join("/"); } -async function getSessionToken(): Promise { - if (_sessionToken) return _sessionToken; - const injected = window.__HERMES_SESSION_TOKEN__; - if (injected) { - _sessionToken = injected; - return _sessionToken; - } - throw new Error("Session token not available — page must be served by the Hermes dashboard server"); -} - /** * Fetch a single-use ticket for a WebSocket upgrade in gated mode. * @@ -553,17 +542,12 @@ export const api = { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ key }), }), - revealEnvVar: async (key: string) => { - const token = await getSessionToken(); - return fetchJSON<{ key: string; value: string }>("/api/env/reveal", { + revealEnvVar: (key: string) => + fetchJSON<{ key: string; value: string }>("/api/env/reveal", { method: "POST", - headers: { - "Content-Type": "application/json", - [SESSION_HEADER]: token, - }, + headers: { "Content-Type": "application/json" }, body: JSON.stringify({ key }), - }); - }, + }), // Cron jobs getCronJobs: (profile = "all") => @@ -788,58 +772,42 @@ export const api = { // OAuth provider management getOAuthProviders: () => fetchJSON("/api/providers/oauth"), - disconnectOAuthProvider: async (providerId: string) => { - const token = await getSessionToken(); - return fetchJSON<{ ok: boolean; provider: string }>( + disconnectOAuthProvider: (providerId: string) => + fetchJSON<{ ok: boolean; provider: string }>( `/api/providers/oauth/${encodeURIComponent(providerId)}`, { method: "DELETE", - headers: { [SESSION_HEADER]: token }, }, - ); - }, - startOAuthLogin: async (providerId: string) => { - const token = await getSessionToken(); - return fetchJSON( + ), + startOAuthLogin: (providerId: string) => + fetchJSON( `/api/providers/oauth/${encodeURIComponent(providerId)}/start`, { method: "POST", - headers: { - "Content-Type": "application/json", - [SESSION_HEADER]: token, - }, + headers: { "Content-Type": "application/json" }, body: "{}", }, - ); - }, - submitOAuthCode: async (providerId: string, sessionId: string, code: string) => { - const token = await getSessionToken(); - return fetchJSON( + ), + submitOAuthCode: (providerId: string, sessionId: string, code: string) => + fetchJSON( `/api/providers/oauth/${encodeURIComponent(providerId)}/submit`, { method: "POST", - headers: { - "Content-Type": "application/json", - [SESSION_HEADER]: token, - }, + headers: { "Content-Type": "application/json" }, body: JSON.stringify({ session_id: sessionId, code }), }, - ); - }, + ), pollOAuthSession: (providerId: string, sessionId: string) => fetchJSON( `/api/providers/oauth/${encodeURIComponent(providerId)}/poll/${encodeURIComponent(sessionId)}`, ), - cancelOAuthSession: async (sessionId: string) => { - const token = await getSessionToken(); - return fetchJSON<{ ok: boolean }>( + cancelOAuthSession: (sessionId: string) => + fetchJSON<{ ok: boolean }>( `/api/providers/oauth/sessions/${encodeURIComponent(sessionId)}`, { method: "DELETE", - headers: { [SESSION_HEADER]: token }, }, - ); - }, + ), // Messaging platforms (gateway channels) getMessagingPlatforms: () => diff --git a/web/src/lib/clipboard.test.ts b/web/src/lib/clipboard.test.ts new file mode 100644 index 00000000000..8f6fd1fba21 --- /dev/null +++ b/web/src/lib/clipboard.test.ts @@ -0,0 +1,114 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { copyTextToClipboard } from "./clipboard"; + +const originalNavigator = globalThis.navigator; +const originalDocument = globalThis.document; +const originalWindow = globalThis.window; + +function setGlobal( + key: K, + value: (typeof globalThis)[K] | undefined, +) { + Object.defineProperty(globalThis, key, { + configurable: true, + value, + }); +} + +afterEach(() => { + setGlobal("navigator", originalNavigator); + setGlobal("document", originalDocument); + setGlobal("window", originalWindow); + vi.restoreAllMocks(); +}); + +describe("copyTextToClipboard", () => { + it("uses navigator.clipboard when available", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setGlobal( + "navigator", + { clipboard: { writeText } } as unknown as Navigator, + ); + setGlobal("document", undefined); + + await expect(copyTextToClipboard("CODEX-1234")).resolves.toBe(true); + expect(writeText).toHaveBeenCalledWith("CODEX-1234"); + }); + + it("falls back to selection copy when Clipboard API fails", async () => { + const writeText = vi.fn().mockRejectedValue(new Error("not allowed")); + const appendChild = vi.fn(); + const removeChild = vi.fn(); + const execCommand = vi.fn().mockReturnValue(true); + const textarea = { + focus: vi.fn(), + select: vi.fn(), + setAttribute: vi.fn(), + setSelectionRange: vi.fn(), + style: {}, + value: "", + } as unknown as HTMLTextAreaElement; + + setGlobal( + "navigator", + { clipboard: { writeText } } as unknown as Navigator, + ); + setGlobal("document", { + body: { appendChild, removeChild }, + createElement: vi.fn(() => textarea), + execCommand, + getSelection: vi.fn(() => null), + } as unknown as Document); + + await expect(copyTextToClipboard("CODEX-1234")).resolves.toBe(true); + + expect(writeText).toHaveBeenCalledWith("CODEX-1234"); + expect(textarea.value).toBe("CODEX-1234"); + expect(appendChild).toHaveBeenCalledWith(textarea); + expect(textarea.select).toHaveBeenCalled(); + expect(execCommand).toHaveBeenCalledWith("copy"); + expect(removeChild).toHaveBeenCalledWith(textarea); + }); + + it("uses selection copy directly in insecure browser contexts", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + const appendChild = vi.fn(); + const removeChild = vi.fn(); + const execCommand = vi.fn().mockReturnValue(true); + const textarea = { + focus: vi.fn(), + select: vi.fn(), + setAttribute: vi.fn(), + setSelectionRange: vi.fn(), + style: {}, + value: "", + } as unknown as HTMLTextAreaElement; + + setGlobal( + "navigator", + { clipboard: { writeText } } as unknown as Navigator, + ); + setGlobal( + "window", + { isSecureContext: false } as unknown as Window & typeof globalThis, + ); + setGlobal("document", { + body: { appendChild, removeChild }, + createElement: vi.fn(() => textarea), + execCommand, + getSelection: vi.fn(() => null), + } as unknown as Document); + + await expect(copyTextToClipboard("CODEX-1234")).resolves.toBe(true); + + expect(writeText).not.toHaveBeenCalled(); + expect(execCommand).toHaveBeenCalledWith("copy"); + }); + + it("returns false when no copy mechanism is available", async () => { + setGlobal("navigator", {} as Navigator); + setGlobal("document", undefined); + + await expect(copyTextToClipboard("CODEX-1234")).resolves.toBe(false); + }); +}); diff --git a/web/src/lib/clipboard.ts b/web/src/lib/clipboard.ts new file mode 100644 index 00000000000..749168e8e5d --- /dev/null +++ b/web/src/lib/clipboard.ts @@ -0,0 +1,56 @@ +export async function copyTextToClipboard(text: string): Promise { + const clipboard = + typeof navigator === "undefined" ? undefined : navigator.clipboard; + const secureContext = + typeof window === "undefined" ? true : window.isSecureContext; + if (secureContext && clipboard?.writeText) { + try { + await clipboard.writeText(text); + return true; + } catch { + // Fall through to the selection-based copy path below. + } + } + + if (typeof document === "undefined") { + return false; + } + + const textarea = document.createElement("textarea"); + textarea.value = text; + textarea.setAttribute("readonly", ""); + textarea.style.position = "fixed"; + textarea.style.top = "-1000px"; + textarea.style.left = "-1000px"; + textarea.style.opacity = "0"; + document.body.appendChild(textarea); + + const selection = document.getSelection(); + const ranges: Range[] = []; + if (selection) { + for (let i = 0; i < selection.rangeCount; i += 1) { + ranges.push(selection.getRangeAt(i)); + } + } + + textarea.focus(); + textarea.select(); + textarea.setSelectionRange(0, textarea.value.length); + + let copied = false; + try { + copied = document.execCommand("copy"); + } catch { + copied = false; + } finally { + document.body.removeChild(textarea); + if (selection) { + selection.removeAllRanges(); + for (const range of ranges) { + selection.addRange(range); + } + } + } + + return copied; +}