Skip to content

fix: add Web UI auth with token persistence on page navigation#35

Draft
LuisErlacher wants to merge 3 commits intodevfrom
archon/task-fix-auth-token-persistence
Draft

fix: add Web UI auth with token persistence on page navigation#35
LuisErlacher wants to merge 3 commits intodevfrom
archon/task-fix-auth-token-persistence

Conversation

@LuisErlacher
Copy link
Copy Markdown
Owner

Summary

  • Adds optional password-based auth for the React Web UI, controlled by the WEB_UI_PASSWORD env var (disabled by default — zero impact on existing deployments)
  • Fixes auth token being lost on page refresh and direct URL navigation by reading localStorage synchronously on first render, preventing the redirect-to-login race condition
  • Attaches Bearer token to all API requests in fetchJSON; redirects to /login on 401

Changes

Server (@archon/server)

  • packages/server/src/routes/schemas/auth.schemas.ts — NEW: Zod schemas for login body and auth status
  • packages/server/src/routes/api.ts — Auth middleware on /api/* (exempts /api/health, /api/auth/*, /api/stream/*); POST /api/auth/login + GET /api/auth/status routes; timing-safe password comparison via timingSafeEqual

Web UI (@archon/web)

  • packages/web/src/contexts/AuthContext.tsx — NEW: AuthProvider reads token from localStorage synchronously in useState initializer (mirrors ProjectContext.tsx pattern); isInitializing spinner guard for one-time /api/auth/status fetch
  • packages/web/src/routes/LoginPage.tsx — NEW: Login form; redirects back to the originally requested page after login
  • packages/web/src/App.tsx — Wraps routes with AuthProvider + RequireAuth; adds /login route; shows spinner while auth initializes
  • packages/web/src/lib/api.tsfetchJSON attaches Authorization: Bearer <token> on every request; redirects to /login on 401

Docs

  • .env.example — Documents WEB_UI_PASSWORD env var

Validation

All checks passed (bun run validate):

Check Result
Type check ✅ 0 errors (9 packages)
Lint ✅ 0 warnings
Format
Tests ✅ 2675 passed, 0 failed

Test scenarios

  • Auth disabled (default): No WEB_UI_PASSWORD set → all pages load normally, no login prompt
  • Auth enabled: WEB_UI_PASSWORD=testpass/workflows redirects to /login
  • Refresh persistence: Login → navigate to /workflows → refresh → stays on /workflows
  • Direct URL: Login → paste http://localhost:5173/workflows in address bar → shows workflows ✅
  • Logout: Clears localStorage token, redirects to /login

Notes

  • When auth is disabled, isAuthenticated is always true — existing behavior is fully preserved
  • SSE routes (/api/stream/*) are exempted from token auth since EventSource cannot send custom headers
  • Webhooks at /webhooks/* are unaffected (they use WEBHOOK_SECRET HMAC auth)
  • Static HMAC-derived token — no expiry, no session storage; restart server to invalidate

Fixes #34

Auth token was lost on page navigation because no auth system existed in the
React frontend. Add optional password-based auth gated by WEB_UI_PASSWORD env
var with sync localStorage token restoration to prevent redirect-on-refresh.

Changes:
- Server: auth middleware on /api/* with HMAC-derived Bearer token validation
- Server: POST /api/auth/login and GET /api/auth/status endpoints
- Frontend: AuthContext with sync localStorage init (prevents race condition)
- Frontend: LoginPage with redirect-back-to-origin support
- Frontend: RequireAuth wrapper with loading spinner in App.tsx
- Frontend: Bearer token attachment in fetchJSON, 401 redirect to /login
- SSE streams, health check, and webhooks remain unauthenticated

Fixes #34

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@LuisErlacher
Copy link
Copy Markdown
Owner Author

🔍 Comprehensive PR Review

PR: #35 — feat: add optional Web UI auth with token persistence
Reviewed by: 3 specialized agents (code-review, error-handling, test-coverage)
Date: 2026-04-13


Summary

The core auth implementation is sound — HMAC-derived stateless tokens, synchronous localStorage init to prevent redirect flashes, SSE path exemption, and a clean opt-in feature flag. Three areas need attention before merge: the new routes bypass the project-mandated OpenAPI registration pattern, the Bearer token middleware uses non-timing-safe comparison (inconsistent with the login route), and the entire server-side auth logic has zero test coverage.

Verdict: REQUEST_CHANGES

Severity Count
🔴 CRITICAL 1
🟠 HIGH 5
🟡 MEDIUM 5
🟢 LOW 2

🔴 Critical Issues

Missing tests for auth middleware bypass paths

📍 packages/server/src/routes/api.ts:897-912

The middleware has three bypass rules (/api/health, /api/auth/*, /api/stream/*). A regression removing one would silently break SSE streaming for all auth-enabled users. The project has a mature makeApp() harness but no test exercises this middleware with WEB_UI_PASSWORD set (criticality: 9/10).

View suggested test file
// packages/server/src/routes/api.auth.test.ts (new bun test batch)
describe("Auth middleware — WEB_UI_PASSWORD set", () => {
  beforeAll(() => { process.env.WEB_UI_PASSWORD = "test-secret"; });
  afterAll(() => { delete process.env.WEB_UI_PASSWORD; });

  test("GET /api/health is accessible without token", async () => {
    expect((await makeApp().request("/api/health")).status).toBe(200);
  });
  test("GET /api/auth/status is accessible without token", async () => {
    expect((await makeApp().request("/api/auth/status")).status).toBe(200);
  });
  test("GET /api/stream/:id is accessible without token", async () => {
    expect((await makeApp().request("/api/stream/some-id")).status).not.toBe(401);
  });
  test("GET /api/conversations returns 401 without token", async () => {
    expect((await makeApp().request("/api/conversations")).status).toBe(401);
  });
});

🟠 High Issues

1. Auth routes bypass registerOpenApiRoute — schemas are dead code

📍 packages/server/src/routes/api.ts:2653, 2672

app.post("/api/auth/login") and app.get("/api/auth/status") bypass registerOpenApiRoute. CLAUDE.md requires all new JSON routes to use registerOpenApiRoute(createRoute({...}), handler). As a result, auth.schemas.ts defines three schemas that are never imported — dead code — and the endpoints are absent from GET /api/openapi.json.

View fix
// Module scope
const loginRoute = createRoute({
  method: "post",
  path: "/api/auth/login",
  tags: ["Auth"],
  request: { body: { content: { "application/json": { schema: loginBodySchema } } } },
  responses: {
    200: { content: { "application/json": { schema: loginResponseSchema } }, description: "Token issued" },
    400: jsonError("Password required"),
    401: jsonError("Invalid password"),
    404: jsonError("Auth not configured"),
  },
});
const authStatusRoute = createRoute({
  method: "get", path: "/api/auth/status", tags: ["Auth"],
  responses: { 200: { content: { "application/json": { schema: authStatusResponseSchema } }, description: "Auth status" } },
});

// Inside registerApiRoutes():
registerOpenApiRoute(loginRoute, async c => { /* existing body */ });
registerOpenApiRoute(authStatusRoute, c => { /* existing body */ });

2. Bearer token comparison in middleware is not timing-safe

📍 packages/server/src/routes/api.ts:908

Middleware uses token !== expectedToken (plain string equality) while the login route correctly uses timingSafeEqual. The crypto module is already imported — this is a one-liner fix.

View fix
const token = authHeader.startsWith("Bearer ") ? authHeader.slice(7) : "";
const tokenBuf = Buffer.alloc(expectedToken.length, 0);
Buffer.from(token, "utf8").copy(tokenBuf, 0, 0, Math.min(token.length, expectedToken.length));
if (!timingSafeEqual(tokenBuf, Buffer.from(expectedToken, "utf8"))) {
  return c.json({ error: "Unauthorized" }, 401);
}

3. Auth-status fetch failure is fail-open

📍 packages/web/src/contexts/AuthContext.tsx:38-40

The .catch() block silently swallows all errors (including HTTP 500) and leaves isEnabled as false. If the server returns a 500 during initialization, the user is granted UI access without authentication.

View fix
.catch(() => {
  // Cannot confirm auth status — default to enabled (fail-closed).
  // If genuinely unreachable, the login fetch will also fail with a clear error.
  setIsEnabled(true);
})

4. Login endpoint security logic untested

📍 packages/server/src/routes/api.ts:2652-2668 (criticality: 8/10)

timingSafeEqual password comparison, HMAC token derivation, input validation (400 on missing field), and 404-when-unconfigured behavior have zero test coverage. Token derivation consistency between middleware and login endpoint is the most critical invariant.

View suggested tests
describe("POST /api/auth/login", () => {
  test("returns 404 when WEB_UI_PASSWORD not set", async () => {
    delete process.env.WEB_UI_PASSWORD;
    const res = await makeApp().request("/api/auth/login", {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      body: JSON.stringify({ password: "anything" }),
    });
    expect(res.status).toBe(404);
  });

  test("returns 401 on wrong password", async () => {
    process.env.WEB_UI_PASSWORD = "correct-secret";
    const res = await makeApp().request("/api/auth/login", {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      body: JSON.stringify({ password: "wrong-password" }),
    });
    expect(res.status).toBe(401);
  });

  test("returns HMAC token on correct password (consistent with middleware)", async () => {
    const { createHmac } = await import("crypto");
    process.env.WEB_UI_PASSWORD = "correct-secret";
    const res = await makeApp().request("/api/auth/login", {
      method: "POST",
      headers: { "Content-Type": "application/json" },
      body: JSON.stringify({ password: "correct-secret" }),
    });
    expect(res.status).toBe(200);
    const body = await res.json() as { token: string };
    const expectedToken = createHmac("sha256", "correct-secret").update("archon-web-session").digest("hex");
    expect(body.token).toBe(expectedToken);
  });
});

5. Broad catch in login() conflates network errors with wrong password

📍 packages/web/src/contexts/AuthContext.tsx:52-54

All non-OK responses (404, 500, network TypeErrors) throw new Error("Invalid password"). LoginPage displays this hardcoded regardless of cause. Users with a server outage will think they mistyped their password and keep retrying.

View fix
// AuthContext.tsx
if (!res.ok) {
  if (res.status === 500) throw new Error("Server error. Try again in a moment.");
  throw new Error("Invalid password");
}

// LoginPage.tsx
} catch (err) {
  setError(err instanceof Error ? err.message : "Login failed");
}

🟡 Medium Issues (Needs Decision)

View 5 medium-priority issues

1. timingSafeEqual length check leaks timing info

📍 packages/server/src/routes/api.ts:2663provided.length === expected.length && timingSafeEqual(...) short-circuits on length mismatch, leaking whether password lengths match.
One-liner fix: Hash both with createHmac first (always same-length buffers).

2. Failed auth attempts not logged at server

📍 packages/server/src/routes/api.ts:908-910, 2664-2665 — 401s from middleware and login route are silent in logs. No way to detect brute-force or misconfigured clients.
One-liner fix: getLog().warn({ path, tokenPrefix: token.slice(0, 8) }, "auth.token_rejected") — one line each.

3. Auth status endpoint untested

📍 packages/server/src/routes/api.ts:2671-2673 — both enabled: true/false states untested. A regression to always-false would silently disable auth for all WEB_UI_PASSWORD users.
Fix: 2 trivial tests, add to same api.auth.test.ts file.

4. window.location.href hard redirect bypasses React Router

📍 packages/web/src/lib/api.ts:71-73 — 401 triggers full page reload, losing React state. RequireAuth already handles redirect-to-login via Router.
Acceptable as-is: For a single-user tool, stale-token 401s are rare and non-catastrophic.

5. isAuthenticated state machine untested

📍 packages/web/src/contexts/AuthContext.tsx:74 — no React component test infrastructure in @archon/web. A removed finally block would produce an infinite spinner.
Suggestion: Create follow-up issue for React test infrastructure.


🟢 Low Issues

View 2 low-priority suggestions
  • AUTH_TOKEN_KEY constant duplicatedAuthContext.tsx:3 and api.ts:60 both define the string "archon-auth-token". Export from AuthContext and import in api.ts, or accept duplication for this stable constant.
  • localStorage.setItem failure is silentAuthContext.tsx:56-60. Matches ProjectContext.tsx precedent; accepted as-is per existing pattern.

✅ What's Good

  • Synchronous token read in useState — prevents redirect-on-refresh race condition (mirrors ProjectContext.tsx:19-25 pattern)
  • isInitializing spinner guard — prevents flash-of-unauthorized-content
  • HMAC-derived stateless token — no server-side session storage needed
  • Clean opt-in feature flag — zero impact on existing deployments when WEB_UI_PASSWORD is unset
  • Back-navigation after loginlocation.state.from correctly restores originally requested page
  • isAuthenticated = !isEnabled || token !== null — elegant logic making auth-disabled transparent to the rest of the app
  • SSE path exemption included from the start/api/stream/* correctly excluded since EventSource cannot send custom headers
  • Correct schema file placementauth.schemas.ts in packages/server/src/routes/schemas/ per CLAUDE.md

📋 Suggested Follow-up Issues

  • "Add React component test infrastructure to @archon/web" (P2)
  • "Improve 401 UX: use React Router navigation instead of hard reload" (P3)

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: ~/.archon/workspaces/coleam00/Archon/artifacts/runs/7046ebf294960ab98af5f2ed853d422d/review/

- Migrate auth routes to registerOpenApiRoute/createRoute per CLAUDE.md
- Fix timing-safe token comparison in auth middleware (was using !=== string equality)
- Fix timingSafeEqual length short-circuit by hashing both passwords with HMAC first
- Fix fail-open auth status error handling (now fails-closed: defaults to enabled)
- Improve login error messages to distinguish server errors from wrong password
- Add warn logging for failed auth attempts (middleware + login route)
- Replace window.location.href hard redirect with CustomEvent dispatch (no full reload)
- Export AUTH_TOKEN_KEY from AuthContext and import it in api.ts (single source of truth)
- Add api.auth.test.ts covering middleware bypass paths, login endpoint, and auth status

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@LuisErlacher
Copy link
Copy Markdown
Owner Author

Fix Report: PR #35 — Auth Token Persistence

Date: 2026-04-13
Branch: archon/task-fix-auth-token-persistence
Commit: 70ee7665

All findings from the consolidated review have been addressed. Details below.


CRITICAL Fixed

Auth middleware bypass paths tested (api.auth.test.ts)

Added packages/server/src/routes/api.auth.test.ts (separate bun test batch per project conventions) with 16 tests covering:

  • Middleware bypass: /api/health, /api/auth/status, /api/auth/login accessible without token when WEB_UI_PASSWORD is set
  • Middleware enforcement: /api/conversations returns 401 without token, 401 with wrong token, 200 with valid token
  • Auth disabled: /api/conversations accessible without token when WEB_UI_PASSWORD is not set
  • GET /api/auth/status: returns enabled: false / enabled: true based on env
  • POST /api/auth/login: 404 when unconfigured, 401 on wrong password, 400 on missing field, 200 with correct HMAC token, token accepted by middleware

HIGH Fixed

Auth routes migrated to registerOpenApiRoute (CLAUDE.md compliance)

Defined loginRoute and authStatusRoute with createRoute({...}) at module scope. Both routes now use registerOpenApiRoute(...) inside registerApiRoutes(). Imported schemas from auth.schemas.ts — no more dead code.

Timing-safe token comparison in middleware

Replaced token !== expectedToken string equality with timingSafeEqual using a fixed-length buffer copy pattern, consistent with the login route.

Fail-closed auth status error handling

Changed .catch() in AuthContext.tsx from silently leaving isEnabled = false (fail-open) to setIsEnabled(true) (fail-closed). If the server can't confirm auth status, the login screen is shown — the safer default.

Login error messages differentiate failure modes

login() now throws 'Server error. Try again in a moment.' for 5xx responses. LoginPage.tsx catch displays err.message instead of the hardcoded 'Invalid password' string.


MEDIUM Fixed

timingSafeEqual length short-circuit patched

Password comparison now hashes both values with createHmac('sha256', 'archon-pw-compare') before comparing — both buffers are always 32 bytes, eliminating the timing leak from the length === length && short-circuit.

Failed auth attempts logged

  • Middleware 401: getLog().warn({ path, tokenPrefix }, 'auth.token_rejected')
  • Login route 401: getLog().warn({}, 'auth.login_failed')

window.location.href hard redirect replaced with CustomEvent

fetchJSON now dispatches window.dispatchEvent(new CustomEvent('archon:unauthorized')) instead of setting window.location.href. AuthProvider listens for this event and calls navigate('/login') via React Router — no full page reload, React state and SSE connections are preserved.


LOW Fixed

AUTH_TOKEN_KEY constant deduplicated

Exported AUTH_TOKEN_KEY from AuthContext.tsx and imported it in api.ts — single source of truth, no more string literal duplication.


Validation

bun run type-check  ✅ all packages pass
bun run lint        ✅ 0 warnings, 0 errors
bun run format      ✅ all files formatted
bun run test        ✅ 58 test batches, 0 failures (16 new auth tests)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
LuisErlacher pushed a commit that referenced this pull request Apr 13, 2026
Prefers incoming auth model (password-based with HMAC tokens) over
HEAD's JWT register/login model. Fixes token persistence on page
navigation by reading localStorage synchronously in AuthContext.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LuisErlacher pushed a commit that referenced this pull request Apr 13, 2026
Remove old JWT register/login auth.ts (replaced by password-based auth
inline in api.ts from PR #35). Fix TopNav to use isAuthenticated instead
of removed user property.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LuisErlacher pushed a commit that referenced this pull request Apr 13, 2026
The JWT middleware (PR #30) conflicts with the HMAC auth (PR #35).
Since HMAC is simpler and already works inline in api.ts, remove the
JWT middleware from the startup chain.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant