From b73e3ebb9eaad1d19c04925e8a7c25292ec8b60f Mon Sep 17 00:00:00 2001 From: Rhea Rafferty Date: Wed, 19 Aug 2026 09:02:39 +0000 Subject: [PATCH 1/3] fix(admin): renew the browser session in place instead of a rotting admin credential (task #6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin console live-re-checked the Raft role by storing the user's Raft access token (raft_access_token_ciphertext) on the session and calling /api/oauth/userinfo each request. That credential rots independently of the 14-day Hands session: Raft's token lives ~1h and issues no refresh_token, and a SIGNED_URL_SECRET rotation makes the ciphertext undecryptable. Result: the outer session stays valid (app pages work) while /admin 401s forever — "logged in but can't enter admin" — and the SPA collapsed six distinct 401 codes into one misleading "sign in again". Fix (Yaoheng's refined option 3 / Argus's silent re-auth; confirmed against the Raft OAuth contract — no refresh_token, no service-credential role endpoint, so D0/D2 are out): - worker/src/middleware/hands_admin.ts: return SESSION_REAUTH_REQUIRED for the browser-recoverable failures (ciphertext won't decrypt = key rotated; Raft rejects the token = expired or member removed). The NULL-ciphertext branch stays ADMIN_RELOGIN_REQUIRED so agent/API sessions (for which NULL is legitimate) are never told to re-auth. - admin/src/lib/api.ts: on SESSION_REAUTH_REQUIRED the shared request layer does one full-page Login-with-Raft renewal (/api/auth/login?return=) that atomically replaces the whole browser session. Single-flight + 30s loop guard: if it comes back still failing (Raft not logged in, or user removed), fall through to the manual page instead of redirect-looping. - admin/src/pages/HandsAdmin.tsx: distinguish ADMIN_AUTH_UNAVAILABLE (server config — contact ops, no login) from the recoverable re-login fallback. One login layer, no {live session + dead credential} half-state, live per-request role revocation preserved. Honest residual: Raft's 1h token + no refresh means active admin use hits a seamless auto-renewal ~hourly; gap-free would need a Raft-side service-role endpoint or longer token (cross-Raft ask). Tests: worker/test/hands_admin_access.test.ts +4 (agent-safe NULL; decrypt-fail and Raft-reject -> SESSION_REAUTH_REQUIRED; config -> ADMIN_AUTH_UNAVAILABLE); existing owner/admin->200 + member/viewer->403 cover renew + immediate revocation. Worker tsc + admin tsc + admin vite build clean; full worker suite green (the one sqlite3-ENOENT failure is env-only, green in CI). Co-Authored-By: Claude Opus 4.8 Signed-off-by: Rhea Rafferty --- admin/src/lib/api.ts | 36 ++++++++++++++++++++++ admin/src/pages/HandsAdmin.tsx | 11 ++++++- worker/src/middleware/hands_admin.ts | 14 +++++++-- worker/test/hands_admin_access.test.ts | 41 ++++++++++++++++++++++---- 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/admin/src/lib/api.ts b/admin/src/lib/api.ts index 6f20075a..7e9fc045 100644 --- a/admin/src/lib/api.ts +++ b/admin/src/lib/api.ts @@ -471,6 +471,33 @@ export interface Invite { invite_url?: string; } +// When the Worker reports SESSION_REAUTH_REQUIRED, the browser session's Raft +// re-check credential rotted or expired (the ~1h Raft access token, or a key +// change). Renew the whole browser session with one full-page Login-with-Raft: +// if the user's Raft session is still alive this returns without interaction and +// replaces the Hands session atomically; if it isn't, a real login is shown. +const REAUTH_AT_KEY = "hands:admin-reauth-at"; +const REAUTH_MIN_INTERVAL_MS = 30_000; + +function triggerSessionReauth(): Promise | null { + // Loop guard / single-flight: if we renewed moments ago and are still being told + // to, the renewal did not fix it (user removed from the server, or Raft not logged + // in so the redirect bounced straight back). Fall through to the normal error so + // the manual "Sign in again" page shows instead of redirect-looping. + try { + const last = Number(window.sessionStorage.getItem(REAUTH_AT_KEY) || 0); + if (Date.now() - last < REAUTH_MIN_INTERVAL_MS) return null; + window.sessionStorage.setItem(REAUTH_AT_KEY, String(Date.now())); + } catch { + return null; // no sessionStorage → don't risk a loop; show the manual page + } + clearAuthToken(); + const back = window.location.pathname + window.location.search; + window.location.href = `/api/auth/login?return=${encodeURIComponent(back)}`; + // Navigation is underway; never resolve so callers don't flash an error first. + return new Promise(() => {}); +} + async function request( path: string, init: RequestInit & { admin?: boolean } = {}, @@ -492,6 +519,15 @@ async function request( // non-JSON body; leave as text } if (!res.ok) { + if ( + res.status === 401 && + typeof body === "object" && + body && + (body as any).code === "SESSION_REAUTH_REQUIRED" + ) { + const reauth = triggerSessionReauth(); + if (reauth) return reauth as unknown as Promise; + } const msg = typeof body === "object" && body && "error" in body ? String((body as any).error) diff --git a/admin/src/pages/HandsAdmin.tsx b/admin/src/pages/HandsAdmin.tsx index ec9fbfab..b028b066 100644 --- a/admin/src/pages/HandsAdmin.tsx +++ b/admin/src/pages/HandsAdmin.tsx @@ -26,7 +26,16 @@ export function HandsAdmin() { const overview = useQuery({ queryKey: ["hands-admin", "overview"], queryFn: getHandsAdminOverview, retry: false }); if (overview.isPending) return
Loading observability…
; if (overview.error instanceof ApiError && overview.error.status === 401) { - return

Sign in again

A fresh Raft session is required for live administrator verification.

Continue with Raft
; + const code = (overview.error.body as { code?: string } | null)?.code; + if (code === "ADMIN_AUTH_UNAVAILABLE") { + // Server-side configuration problem (missing secret / Raft origin). Re-login + // cannot fix it — do not send the operator into a login loop. + return

Admin verification unavailable

Live administrator verification is temporarily unavailable because of a server configuration issue. This is not a login problem — please contact an operator.

; + } + // SESSION_REAUTH_REQUIRED is normally renewed in the API layer before it reaches + // here; this manual page is the fallback when the loop guard tripped (Raft not + // logged in, or the user is no longer a member of the server). + return

Sign in again

Your administrator session needs a fresh Raft sign-in.

Continue with Raft
; } if (overview.error instanceof ApiError && overview.error.status === 403) { return

Administrator access required

This page is limited to administrators of an approved Raft server.

; diff --git a/worker/src/middleware/hands_admin.ts b/worker/src/middleware/hands_admin.ts index 761604a6..82a98806 100644 --- a/worker/src/middleware/hands_admin.ts +++ b/worker/src/middleware/hands_admin.ts @@ -43,12 +43,22 @@ export const requireHandsAdmin: MiddlewareHandler try { accessToken = await decryptAdminRaftToken(secret, session.raft_access_token_ciphertext); } catch { - return deny(c, 401, "ADMIN_RELOGIN_REQUIRED"); + // A non-NULL ciphertext that won't decrypt is a browser session whose Raft + // credential rotted (e.g. the encryption key changed). It is recoverable by + // renewing the browser session (a fresh Login-with-Raft re-encrypts with the + // current key) — signal that, not the generic relogin. Agent/API sessions hit + // the NULL-ciphertext branch above and are never told to re-auth. + return deny(c, 401, "SESSION_REAUTH_REQUIRED"); } const response = await fetch(new URL("/api/oauth/userinfo", c.env.RAFT_API_ORIGIN), { headers: { authorization: `Bearer ${accessToken}` }, }); - if (!response.ok) return deny(c, 401, "AUTH_EXPIRED"); + // Raft rejects an expired token (the ~1h access token) — and also a user removed + // from the server. Both are recoverable by a browser session renewal: a valid user + // gets a fresh token and continues; a removed user gets a fresh token whose userinfo + // still fails, so the frontend's loop guard falls back to the manual page (still + // denied — role revocation stays immediate). + if (!response.ok) return deny(c, 401, "SESSION_REAUTH_REQUIRED"); const live = await response.json(); const allowedServers = (c.env.HANDS_ADMIN_ALLOWED_SERVER_IDS || "") diff --git a/worker/test/hands_admin_access.test.ts b/worker/test/hands_admin_access.test.ts index bb20af06..08532630 100644 --- a/worker/test/hands_admin_access.test.ts +++ b/worker/test/hands_admin_access.test.ts @@ -17,18 +17,29 @@ function database(ciphertext: string | null) { } as unknown as D1Database; } -async function request(role: string, serverId = "server-allowed", ciphertext?: string | null) { +async function request( + role: string, + serverId = "server-allowed", + ciphertext?: string | null, + opts: { userinfoOk?: boolean; secret?: string | null } = {}, +) { const secret = "test-secret"; const encrypted = ciphertext === undefined ? await encryptAdminRaftToken(secret, "raft-token") : ciphertext; - vi.stubGlobal("fetch", vi.fn(async () => new Response(JSON.stringify({ - sub: account.provider_subject, server_id: serverId, server_role: role, - }), { headers: { "content-type": "application/json" } }))); + const userinfoOk = opts.userinfoOk !== false; + vi.stubGlobal("fetch", vi.fn(async () => new Response( + userinfoOk + ? JSON.stringify({ sub: account.provider_subject, server_id: serverId, server_role: role }) + : "unauthorized", + { status: userinfoOk ? 200 : 401, headers: { "content-type": "application/json" } }, + ))); const app = new Hono(); app.use("*", async (c, next) => { c.set("admin_account", account); await next(); }); app.use("/admin/*", requireHandsAdmin); app.get("/admin/overview", (c) => c.json({ ok: true })); const env = { - DB: database(encrypted), SIGNED_URL_SECRET: secret, RAFT_API_ORIGIN: "https://api.raft.build", + DB: database(encrypted), + SIGNED_URL_SECRET: opts.secret === undefined ? secret : (opts.secret ?? undefined), + RAFT_API_ORIGIN: "https://api.raft.build", HANDS_ADMIN_ALLOWED_SERVER_IDS: "server-allowed", } as unknown as Env; return app.request("https://app.hands.build/admin/overview", { headers: { authorization: "Bearer hands-session" } }, env); @@ -40,6 +51,26 @@ describe("requireHandsAdmin", () => { it.each(["member", "viewer"])("denies live server role %s before the handler", async (role) => expect((await request(role)).status).toBe(403)); it("denies another Raft server", async () => expect((await request("admin", "other-server")).status).toBe(403)); it("requires a fresh login for sessions without a live Raft credential", async () => expect((await request("admin", "server-allowed", null)).status).toBe(401)); + it("does NOT tell a no-credential session to re-auth (agent-safe: NULL ciphertext is legitimate for agent sessions)", async () => { + const res = await request("admin", "server-allowed", null); + expect(await res.json()).toEqual({ error: "unauthorized", code: "ADMIN_RELOGIN_REQUIRED" }); + }); + it("renews the browser session when the stored credential can't be decrypted (key rotated)", async () => { + const wrongKey = await encryptAdminRaftToken("rotated-secret", "raft-token"); + const res = await request("admin", "server-allowed", wrongKey); + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); + }); + it("renews the browser session when Raft rejects the token (expired, or member removed)", async () => { + const res = await request("admin", "server-allowed", undefined, { userinfoOk: false }); + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); + }); + it("reports config-unavailable (not a re-login) when the admin secret is missing", async () => { + const res = await request("admin", "server-allowed", "irrelevant-ciphertext", { secret: null }); + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ error: "unauthorized", code: "ADMIN_AUTH_UNAVAILABLE" }); + }); it("does not confuse app admin role with Raft server admin role", async () => { // The stored account is an org/app admin, but live Raft says only member. const response = await request("member"); From 526254460968d468c0fa98b8253e28acf35d34f4 Mon Sep 17 00:00:00 2001 From: Rhea Rafferty Date: Wed, 19 Aug 2026 09:16:23 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(admin):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20human+NULL=20renews,=20frontend=20reauth=20tests,=20honest?= =?UTF-8?q?=20wording=20(task=20#6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Yaoheng review of b73e3ebb — 2 blockers: 1. Human sessions with a NULL raft_access_token_ciphertext (legacy / captured before the credential existed) were left on ADMIN_RELOGIN_REQUIRED, which does NOT trigger the new renewal — likely artin's current lock-out shape, so the fix would not have reached him. Branch hands_admin.ts by principal_type: human+NULL -> SESSION_REAUTH_REQUIRED (renew); agent+NULL -> ADMIN_RELOGIN_REQUIRED (agents legitimately have none and must never be sent through a browser re-auth). +2 tests, both paths. 2. The frontend reauth behavior had no tests. Add a jsdom test (admin/src/lib/apiReauth.dom.test.ts, +5) locking the four gates: redirect only on SESSION_REAUTH_REQUIRED, carrying the return path; single-flight / 30s loop guard (a second within the window errors to the manual page — no redirect loop); no redirect on ADMIN_AUTH_UNAVAILABLE (config) / 403 / the legacy ADMIN_RELOGIN_REQUIRED. Non-blocker: corrected the api.ts comment — the renewal is a client-side switch to a new session (localStorage token), NOT a server-side atomic replace; the old D1 row is not revoked, it just expires. Scope note (Argus): blocker 1 closes the CURRENT lock-out (human+NULL) but NOT the separate, still-unexplained "re-login also fails" — a fresh login always writes a ciphertext (auth.ts:577), so it never hits the NULL branch. That second box needs the empirical gate 5 (a real browser fresh-login -> /admin 200 by a test admin, not artin); prime candidate is the #460 browser-login proof cookie (400 "Missing or invalid browser login proof"): 10-min state TTL / browser cookie policy / prod cookie domain. Verified: worker hands_admin_access 12/12 + tsc 0; admin suite 38/38 + tsc 0 + vite build. Co-Authored-By: Claude Opus 4.8 Signed-off-by: Rhea Rafferty --- admin/src/lib/api.ts | 8 ++- admin/src/lib/apiReauth.dom.test.ts | 90 ++++++++++++++++++++++++++ worker/src/middleware/hands_admin.ts | 13 +++- worker/test/hands_admin_access.test.ts | 17 +++-- 4 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 admin/src/lib/apiReauth.dom.test.ts diff --git a/admin/src/lib/api.ts b/admin/src/lib/api.ts index 7e9fc045..60338791 100644 --- a/admin/src/lib/api.ts +++ b/admin/src/lib/api.ts @@ -473,9 +473,11 @@ export interface Invite { // When the Worker reports SESSION_REAUTH_REQUIRED, the browser session's Raft // re-check credential rotted or expired (the ~1h Raft access token, or a key -// change). Renew the whole browser session with one full-page Login-with-Raft: -// if the user's Raft session is still alive this returns without interaction and -// replaces the Hands session atomically; if it isn't, a real login is shown. +// change). Renew it with one full-page Login-with-Raft: if the user's Raft session +// is still alive this returns without interaction and the browser switches to the +// new Hands session (the callback stores a fresh token); if it isn't, a real login +// is shown. This is a client-side switch to a new session — it does not revoke the +// old D1 session server-side (that row simply expires). const REAUTH_AT_KEY = "hands:admin-reauth-at"; const REAUTH_MIN_INTERVAL_MS = 30_000; diff --git a/admin/src/lib/apiReauth.dom.test.ts b/admin/src/lib/apiReauth.dom.test.ts new file mode 100644 index 00000000..a788b382 --- /dev/null +++ b/admin/src/lib/apiReauth.dom.test.ts @@ -0,0 +1,90 @@ +// @vitest-environment jsdom +// +// The shared request layer renews the browser session on SESSION_REAUTH_REQUIRED +// (the admin console's "one layer, no half-session" fix). These tests lock the four +// gates: it redirects only on that code, carries the return path, is single-flight / +// loop-guarded, and never redirects on config errors or 403. +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +let hrefLog: string[] = []; + +function setLocation(pathname: string, search = "") { + Object.defineProperty(window, "location", { + configurable: true, + value: { + pathname, + search, + hash: "", + get href() { + return hrefLog[hrefLog.length - 1] ?? ""; + }, + set href(v: string) { + hrefLog.push(v); + }, + }, + }); +} + +function mockJson(status: number, body: unknown) { + vi.stubGlobal( + "fetch", + vi.fn(async () => new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } })), + ); +} + +// Fresh module per test so the module-load consumeAuthTokenFromUrl() and closures reset. +async function loadApi() { + vi.resetModules(); + return import("./api"); +} + +beforeEach(() => { + hrefLog = []; + setLocation("/admin", "?x=1"); + window.sessionStorage.clear(); + window.localStorage.clear(); +}); +afterEach(() => vi.unstubAllGlobals()); + +describe("SESSION_REAUTH_REQUIRED renewal (shared request layer)", () => { + it("redirects to Login with Raft carrying the current path as return", async () => { + const api = await loadApi(); + mockJson(401, { error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); + // The renewal returns a never-resolving promise; assert the side effect instead. + void api.getHandsAdminOverview(); + await vi.waitFor(() => expect(hrefLog.length).toBe(1)); + expect(hrefLog[0]).toBe(`/api/auth/login?return=${encodeURIComponent("/admin?x=1")}`); + }); + + it("is single-flight / loop-guarded: a second SESSION_REAUTH_REQUIRED within the window does not redirect again, it errors", async () => { + const api = await loadApi(); + mockJson(401, { error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); + void api.getHandsAdminOverview(); + await vi.waitFor(() => expect(hrefLog.length).toBe(1)); + // Second call, still SESSION_REAUTH_REQUIRED: the 30s guard blocks a re-redirect, + // so it surfaces the error (→ the manual "Sign in again" fallback) instead of looping. + await expect(api.getHandsAdminOverview()).rejects.toMatchObject({ status: 401 }); + expect(hrefLog.length).toBe(1); + }); + + it("does NOT redirect on ADMIN_AUTH_UNAVAILABLE (server config, not a login problem)", async () => { + const api = await loadApi(); + mockJson(401, { error: "unauthorized", code: "ADMIN_AUTH_UNAVAILABLE" }); + await expect(api.getHandsAdminOverview()).rejects.toMatchObject({ status: 401 }); + expect(hrefLog.length).toBe(0); + }); + + it("does NOT redirect on 403 (not an administrator)", async () => { + const api = await loadApi(); + mockJson(403, { error: "forbidden", code: "HANDS_ADMIN_REQUIRED" }); + await expect(api.getHandsAdminOverview()).rejects.toMatchObject({ status: 403 }); + expect(hrefLog.length).toBe(0); + }); + + it("does NOT redirect on the legacy ADMIN_RELOGIN_REQUIRED (agent/no-credential — never browser-renew)", async () => { + const api = await loadApi(); + mockJson(401, { error: "unauthorized", code: "ADMIN_RELOGIN_REQUIRED" }); + await expect(api.getHandsAdminOverview()).rejects.toMatchObject({ status: 401 }); + expect(hrefLog.length).toBe(0); + }); +}); diff --git a/worker/src/middleware/hands_admin.ts b/worker/src/middleware/hands_admin.ts index 82a98806..211e357f 100644 --- a/worker/src/middleware/hands_admin.ts +++ b/worker/src/middleware/hands_admin.ts @@ -34,7 +34,18 @@ export const requireHandsAdmin: MiddlewareHandler LIMIT 1`, ).bind(await sha256Hex(sessionToken), account.id, Date.now()) .first<{ raft_access_token_ciphertext: string | null }>(); - if (!session?.raft_access_token_ciphertext) return deny(c, 401, "ADMIN_RELOGIN_REQUIRED"); + if (!session?.raft_access_token_ciphertext) { + // A human browser session with no stored Raft credential (a legacy session, or + // one from before the credential was captured) is recoverable by renewing the + // browser session — a fresh Login-with-Raft stores one. An agent/CLI session + // legitimately never has this credential and must NOT be sent through a browser + // re-auth; it just cannot use the admin console. + return deny( + c, + 401, + account.principal_type === "agent" ? "ADMIN_RELOGIN_REQUIRED" : "SESSION_REAUTH_REQUIRED", + ); + } const secret = c.env.SIGNED_URL_SECRET || c.env.RAFT_CLIENT_SECRET; if (!secret || !c.env.RAFT_API_ORIGIN) return deny(c, 401, "ADMIN_AUTH_UNAVAILABLE"); diff --git a/worker/test/hands_admin_access.test.ts b/worker/test/hands_admin_access.test.ts index 08532630..b077864e 100644 --- a/worker/test/hands_admin_access.test.ts +++ b/worker/test/hands_admin_access.test.ts @@ -21,7 +21,7 @@ async function request( role: string, serverId = "server-allowed", ciphertext?: string | null, - opts: { userinfoOk?: boolean; secret?: string | null } = {}, + opts: { userinfoOk?: boolean; secret?: string | null; principalType?: "human" | "agent" } = {}, ) { const secret = "test-secret"; const encrypted = ciphertext === undefined ? await encryptAdminRaftToken(secret, "raft-token") : ciphertext; @@ -33,7 +33,10 @@ async function request( { status: userinfoOk ? 200 : 401, headers: { "content-type": "application/json" } }, ))); const app = new Hono(); - app.use("*", async (c, next) => { c.set("admin_account", account); await next(); }); + app.use("*", async (c, next) => { + c.set("admin_account", { ...account, principal_type: opts.principalType ?? account.principal_type }); + await next(); + }); app.use("/admin/*", requireHandsAdmin); app.get("/admin/overview", (c) => c.json({ ok: true })); const env = { @@ -51,8 +54,14 @@ describe("requireHandsAdmin", () => { it.each(["member", "viewer"])("denies live server role %s before the handler", async (role) => expect((await request(role)).status).toBe(403)); it("denies another Raft server", async () => expect((await request("admin", "other-server")).status).toBe(403)); it("requires a fresh login for sessions without a live Raft credential", async () => expect((await request("admin", "server-allowed", null)).status).toBe(401)); - it("does NOT tell a no-credential session to re-auth (agent-safe: NULL ciphertext is legitimate for agent sessions)", async () => { - const res = await request("admin", "server-allowed", null); + it("renews a HUMAN browser session with no stored credential (legacy/missing-ciphertext — the current lock-out shape)", async () => { + const res = await request("admin", "server-allowed", null, { principalType: "human" }); + expect(res.status).toBe(401); + expect(await res.json()).toEqual({ error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); + }); + it("does NOT tell an AGENT/CLI session to re-auth (NULL ciphertext is legitimate for agents — never browser-renew)", async () => { + const res = await request("admin", "server-allowed", null, { principalType: "agent" }); + expect(res.status).toBe(401); expect(await res.json()).toEqual({ error: "unauthorized", code: "ADMIN_RELOGIN_REQUIRED" }); }); it("renews the browser session when the stored credential can't be decrypted (key rotated)", async () => { From 1b65677b85a7889f4cf57b30f518621c22873342 Mon Sep 17 00:00:00 2001 From: Rhea Rafferty Date: Wed, 19 Aug 2026 09:34:05 +0000 Subject: [PATCH 3/3] fix(admin): branch Raft userinfo non-2xx by status, not blanket re-auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit requireHandsAdmin mapped every non-2xx from Raft /api/oauth/userinfo to SESSION_REAUTH_REQUIRED, collapsing three distinct world states into one reading and dragging normal users into repeated logins on a Raft outage or a permission denial. Branch by status instead: 401 -> SESSION_REAUTH_REQUIRED (token invalid/expired/removed; recoverable by browser session renewal) 403 -> HANDS_ADMIN_REQUIRED (permission decision, not login) 429/5xx/other -> ADMIN_VERIFICATION_UNAVAILABLE (503; transient, retry — re-login would not help and worsens rate-limiting) Widen deny() to 503 ("unavailable"). Worker tests cover the 403 and the 429/500/502/503 branches; the frontend jsdom lock adds 503 to the set of codes that must NOT trigger a login redirect. Addresses review blocker 3 (hands_admin.ts:72). Signed-off-by: Rhea Rafferty Co-Authored-By: Claude Opus 4.8 --- admin/src/lib/apiReauth.dom.test.ts | 7 +++++++ worker/src/middleware/hands_admin.ts | 26 ++++++++++++++++++-------- worker/test/hands_admin_access.test.ts | 19 +++++++++++++++---- 3 files changed, 40 insertions(+), 12 deletions(-) diff --git a/admin/src/lib/apiReauth.dom.test.ts b/admin/src/lib/apiReauth.dom.test.ts index a788b382..1507ac90 100644 --- a/admin/src/lib/apiReauth.dom.test.ts +++ b/admin/src/lib/apiReauth.dom.test.ts @@ -81,6 +81,13 @@ describe("SESSION_REAUTH_REQUIRED renewal (shared request layer)", () => { expect(hrefLog.length).toBe(0); }); + it("does NOT redirect on 503 ADMIN_VERIFICATION_UNAVAILABLE (transient Raft outage — retry, not re-login)", async () => { + const api = await loadApi(); + mockJson(503, { error: "unavailable", code: "ADMIN_VERIFICATION_UNAVAILABLE" }); + await expect(api.getHandsAdminOverview()).rejects.toMatchObject({ status: 503 }); + expect(hrefLog.length).toBe(0); + }); + it("does NOT redirect on the legacy ADMIN_RELOGIN_REQUIRED (agent/no-credential — never browser-renew)", async () => { const api = await loadApi(); mockJson(401, { error: "unauthorized", code: "ADMIN_RELOGIN_REQUIRED" }); diff --git a/worker/src/middleware/hands_admin.ts b/worker/src/middleware/hands_admin.ts index 211e357f..4e170de2 100644 --- a/worker/src/middleware/hands_admin.ts +++ b/worker/src/middleware/hands_admin.ts @@ -14,8 +14,9 @@ type LiveRaftUser = { server_role?: string; }; -function deny(c: Context, status: 401 | 403, code: string) { - return c.json({ error: status === 401 ? "unauthorized" : "forbidden", code }, status); +function deny(c: Context, status: 401 | 403 | 503, code: string) { + const error = status === 401 ? "unauthorized" : status === 403 ? "forbidden" : "unavailable"; + return c.json({ error, code }, status); } export const requireHandsAdmin: MiddlewareHandler = async (c, next) => { @@ -64,12 +65,21 @@ export const requireHandsAdmin: MiddlewareHandler const response = await fetch(new URL("/api/oauth/userinfo", c.env.RAFT_API_ORIGIN), { headers: { authorization: `Bearer ${accessToken}` }, }); - // Raft rejects an expired token (the ~1h access token) — and also a user removed - // from the server. Both are recoverable by a browser session renewal: a valid user - // gets a fresh token and continues; a removed user gets a fresh token whose userinfo - // still fails, so the frontend's loop guard falls back to the manual page (still - // denied — role revocation stays immediate). - if (!response.ok) return deny(c, 401, "SESSION_REAUTH_REQUIRED"); + // Distinguish WHY Raft refused — three different world states must not collapse into + // one reading, or transient outages and permission denials drag normal users into + // repeated logins: + // 401 — token invalid/expired (or the user was removed): recoverable by a browser + // session renewal (a valid user continues; a removed user's renewed token + // still fails userinfo, so the frontend loop guard falls back to the manual + // page — still denied, role revocation stays immediate). + // 403 — a permission decision: not a login problem, do not renew. + // 429 / 5xx / anything else — a transient Raft outage: re-login would not help and + // retrying worsens rate-limiting, so surface "temporarily unavailable". + if (!response.ok) { + if (response.status === 401) return deny(c, 401, "SESSION_REAUTH_REQUIRED"); + if (response.status === 403) return deny(c, 403, "HANDS_ADMIN_REQUIRED"); + return deny(c, 503, "ADMIN_VERIFICATION_UNAVAILABLE"); + } const live = await response.json(); const allowedServers = (c.env.HANDS_ADMIN_ALLOWED_SERVER_IDS || "") diff --git a/worker/test/hands_admin_access.test.ts b/worker/test/hands_admin_access.test.ts index b077864e..f2b2da10 100644 --- a/worker/test/hands_admin_access.test.ts +++ b/worker/test/hands_admin_access.test.ts @@ -21,16 +21,17 @@ async function request( role: string, serverId = "server-allowed", ciphertext?: string | null, - opts: { userinfoOk?: boolean; secret?: string | null; principalType?: "human" | "agent" } = {}, + opts: { userinfoOk?: boolean; userinfoStatus?: number; secret?: string | null; principalType?: "human" | "agent" } = {}, ) { const secret = "test-secret"; const encrypted = ciphertext === undefined ? await encryptAdminRaftToken(secret, "raft-token") : ciphertext; - const userinfoOk = opts.userinfoOk !== false; + const userinfoStatus = opts.userinfoStatus ?? (opts.userinfoOk === false ? 401 : 200); + const userinfoOk = userinfoStatus >= 200 && userinfoStatus < 300; vi.stubGlobal("fetch", vi.fn(async () => new Response( userinfoOk ? JSON.stringify({ sub: account.provider_subject, server_id: serverId, server_role: role }) - : "unauthorized", - { status: userinfoOk ? 200 : 401, headers: { "content-type": "application/json" } }, + : "error", + { status: userinfoStatus, headers: { "content-type": "application/json" } }, ))); const app = new Hono(); app.use("*", async (c, next) => { @@ -75,6 +76,16 @@ describe("requireHandsAdmin", () => { expect(res.status).toBe(401); expect(await res.json()).toEqual({ error: "unauthorized", code: "SESSION_REAUTH_REQUIRED" }); }); + it("does NOT renew on a Raft 403 (permission decision, not a login problem) — maps to admin-required", async () => { + const res = await request("admin", "server-allowed", undefined, { userinfoStatus: 403 }); + expect(res.status).toBe(403); + expect(await res.json()).toEqual({ error: "forbidden", code: "HANDS_ADMIN_REQUIRED" }); + }); + it.each([429, 500, 502, 503])("does NOT renew on a transient Raft %i — surfaces verification-unavailable, not re-login", async (status) => { + const res = await request("admin", "server-allowed", undefined, { userinfoStatus: status }); + expect(res.status).toBe(503); + expect(await res.json()).toEqual({ error: "unavailable", code: "ADMIN_VERIFICATION_UNAVAILABLE" }); + }); it("reports config-unavailable (not a re-login) when the admin secret is missing", async () => { const res = await request("admin", "server-allowed", "irrelevant-ciphertext", { secret: null }); expect(res.status).toBe(401);