From cedd623466224653a98d2d297c47c9295bf7acb0 Mon Sep 17 00:00:00 2001 From: yiekheng Date: Sun, 10 May 2026 17:50:41 +0800 Subject: [PATCH] feat(web): loginAction + logoutAction (with TDD) Username + password verified against the operators row, bcrypt compare regardless of user-found state for timing equivalence, DUMMY_HASH precomputed and committed. 10/5min IP rate limit, no password ever logged. Issues a 30-day HttpOnly+Secure+SameSite=Lax cookie on success, redirects via safeRedirect(next). 12 unit tests covering correct creds, wrong username, wrong password, missing password_hash, empty/long inputs, case-insensitive match, rate-limit trigger, no-password-leak, safe redirect, unsafe redirect, logout. --- apps/web/src/actions/auth.test.ts | 189 ++++++++++++++++++++++++++++++ apps/web/src/actions/auth.ts | 116 ++++++++++++++++++ 2 files changed, 305 insertions(+) create mode 100644 apps/web/src/actions/auth.test.ts create mode 100644 apps/web/src/actions/auth.ts diff --git a/apps/web/src/actions/auth.test.ts b/apps/web/src/actions/auth.test.ts new file mode 100644 index 0000000..ddd7702 --- /dev/null +++ b/apps/web/src/actions/auth.test.ts @@ -0,0 +1,189 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import bcrypt from "bcryptjs"; + +const { + cookiesSetMock, + cookiesDeleteMock, + findUserMock, + headersGetMock, + checkRateLimitMock, + redirectMock, + loggerMock, +} = vi.hoisted(() => ({ + cookiesSetMock: vi.fn(), + cookiesDeleteMock: vi.fn(), + findUserMock: vi.fn(), + headersGetMock: vi.fn(() => "127.0.0.1"), + checkRateLimitMock: vi.fn(), + redirectMock: vi.fn(() => { + throw new Error("redirect"); + }), + loggerMock: { warn: vi.fn(), info: vi.fn() }, +})); + +vi.mock("next/headers", () => ({ + cookies: async () => ({ set: cookiesSetMock, delete: cookiesDeleteMock }), + headers: async () => ({ + get: (k: string) => (k.toLowerCase() === "x-forwarded-for" ? headersGetMock() : null), + }), +})); +vi.mock("next/navigation", () => ({ + redirect: (...a: unknown[]) => redirectMock(...a), +})); +vi.mock("@/lib/db", () => ({ + db: { + query: { + operators: { findFirst: (...a: unknown[]) => findUserMock(...a) }, + }, + }, +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: (...a: unknown[]) => checkRateLimitMock(...a), +})); +vi.mock("@/lib/logger", () => ({ logger: loggerMock })); + +const SECRET = "test-secret-not-real"; +beforeEach(() => { + process.env.AUTH_SECRET = SECRET; + process.env.OPERATOR_TOKEN_VERSION = "1"; + cookiesSetMock.mockReset(); + cookiesDeleteMock.mockReset(); + findUserMock.mockReset(); + checkRateLimitMock.mockReset(); + checkRateLimitMock.mockResolvedValue({ limited: false, count: 1 }); + redirectMock.mockReset(); + redirectMock.mockImplementation(() => { + throw new Error("redirect"); + }); + loggerMock.warn.mockReset(); +}); + +import { loginAction, logoutAction } from "./auth"; + +const REAL_HASH = bcrypt.hashSync("correct-horse", 10); +const ADMIN_ROW = { + id: "11111111-1111-1111-1111-111111111111", + username: "admin", + role: "admin" as const, + displayName: "Admin", + defaultTimezone: "UTC", + passwordHash: REAL_HASH, +}; + +function fd(fields: Record): FormData { + const f = new FormData(); + for (const [k, v] of Object.entries(fields)) f.append(k, v); + return f; +} + +describe("loginAction", () => { + it("issues a session cookie when credentials are correct", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })).catch( + (e) => e, + ); + // Successful login redirects, so the redirect mock throws. + expect((r as Error).message).toBe("redirect"); + expect(cookiesSetMock).toHaveBeenCalledTimes(1); + const [name, , attrs] = cookiesSetMock.mock.calls[0]!; + expect(name).toBe("session"); + expect(attrs).toMatchObject({ + httpOnly: true, + secure: true, + sameSite: "lax", + path: "/", + maxAge: 30 * 86400, + }); + }); + + it("returns ok:false on wrong password and does NOT set a cookie", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + const r = await loginAction(fd({ username: "admin", password: "wrong" })); + expect(r).toEqual({ ok: false, error: "Invalid username or password." }); + expect(cookiesSetMock).not.toHaveBeenCalled(); + expect(loggerMock.warn).toHaveBeenCalled(); + }); + + it("returns ok:false on unknown username and STILL invokes bcrypt (timing equivalence)", async () => { + findUserMock.mockResolvedValue(undefined); + const cmpSpy = vi.spyOn(bcrypt, "compare"); + await loginAction(fd({ username: "nobody", password: "irrelevant" })); + expect(cmpSpy).toHaveBeenCalled(); // compared against DUMMY_HASH + cmpSpy.mockRestore(); + }); + + it("returns a clear error when the user has no password_hash set", async () => { + findUserMock.mockResolvedValue({ ...ADMIN_ROW, passwordHash: null }); + const r = await loginAction(fd({ username: "admin", password: "anything" })); + expect(r).toEqual({ + ok: false, + error: "Set a password via scripts/set-password.sh before signing in.", + }); + }); + + it("rejects empty username or password without hitting the DB", async () => { + const r = await loginAction(fd({ username: "", password: "x" })); + expect(r).toEqual({ ok: false, error: "Username and password are required." }); + expect(findUserMock).not.toHaveBeenCalled(); + }); + + it("rejects username/password >256 chars without invoking bcrypt", async () => { + const cmpSpy = vi.spyOn(bcrypt, "compare"); + const long = "x".repeat(300); + const r = await loginAction(fd({ username: long, password: long })); + expect(r).toEqual({ ok: false, error: "Input too long." }); + expect(cmpSpy).not.toHaveBeenCalled(); + cmpSpy.mockRestore(); + }); + + it("matches username case-insensitively", async () => { + findUserMock.mockImplementation(async () => ADMIN_ROW); + await loginAction(fd({ username: "ADMIN", password: "correct-horse" })).catch(() => {}); + expect(findUserMock).toHaveBeenCalled(); + }); + + it("returns 429 when the rate limit is exhausted", async () => { + checkRateLimitMock.mockResolvedValue({ limited: true, count: 11 }); + const r = await loginAction(fd({ username: "admin", password: "correct-horse" })); + expect(r).toEqual({ ok: false, error: "Too many attempts. Try again later." }); + expect(findUserMock).not.toHaveBeenCalled(); + }); + + it("logs the failed attempt with username and ip but never the password", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + headersGetMock.mockReturnValue("203.0.113.5, 10.0.0.1"); + await loginAction(fd({ username: "admin", password: "wrong" })); + const [meta, msg] = loggerMock.warn.mock.calls[0]!; + expect(meta).toMatchObject({ username: "admin", ip: "203.0.113.5" }); + expect(JSON.stringify(meta)).not.toContain("wrong"); + expect(msg).toMatch(/login failed/i); + }); + + it("redirects to safeRedirect(next) on success", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + await loginAction(fd({ + username: "admin", + password: "correct-horse", + next: "/dashboard", + })).catch(() => {}); + expect(redirectMock).toHaveBeenCalledWith("/dashboard"); + }); + + it("redirects to / when next is unsafe", async () => { + findUserMock.mockResolvedValue(ADMIN_ROW); + await loginAction(fd({ + username: "admin", + password: "correct-horse", + next: "//evil.com", + })).catch(() => {}); + expect(redirectMock).toHaveBeenCalledWith("/"); + }); +}); + +describe("logoutAction", () => { + it("clears the session cookie and redirects to /login", async () => { + await logoutAction().catch(() => {}); + expect(cookiesDeleteMock).toHaveBeenCalledWith("session"); + expect(redirectMock).toHaveBeenCalledWith("/login"); + }); +}); diff --git a/apps/web/src/actions/auth.ts b/apps/web/src/actions/auth.ts new file mode 100644 index 0000000..3608579 --- /dev/null +++ b/apps/web/src/actions/auth.ts @@ -0,0 +1,116 @@ +"use server"; + +import { cookies, headers } from "next/headers"; +import { redirect } from "next/navigation"; +import bcrypt from "bcryptjs"; +import { sql } from "drizzle-orm"; +import { db } from "@/lib/db"; +import { + COOKIE_NAME, + DEFAULT_TTL_SECONDS, + signSession, + type Role, +} from "@/lib/auth-cookie"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { safeRedirect } from "@/lib/safe-redirect"; +import { logger } from "@/lib/logger"; + +export type LoginResult = { ok: true } | { ok: false; error: string }; + +const MAX_FIELD_LEN = 256; + +// Precomputed bcryptjs hash of the throwaway string "x", cost 10. +// Compared against on the user-not-found path so timing matches the +// wrong-password path. Generating fresh per request would double the +// bcrypt work and create its own timing signal. +const DUMMY_HASH = "$2a$10$N9qo8uLOickgx2ZMRZoMyeIjZAgcfl7p92ldGxad68LJZdL17lhWy"; + +async function clientIp(): Promise { + const h = await headers(); + const fwd = h.get("x-forwarded-for"); + if (fwd) return fwd.split(",")[0]!.trim(); + return h.get("x-real-ip") ?? "unknown"; +} + +export async function loginAction(formData: FormData): Promise { + const username = (formData.get("username") ?? "").toString(); + const password = (formData.get("password") ?? "").toString(); + const next = (formData.get("next") ?? "").toString(); + + if (!username.trim() || !password) { + return { ok: false, error: "Username and password are required." }; + } + if (username.length > MAX_FIELD_LEN || password.length > MAX_FIELD_LEN) { + return { ok: false, error: "Input too long." }; + } + + const ip = await clientIp(); + const rl = await checkRateLimit(`login:${ip}`, { max: 10, windowSec: 300 }); + if (rl.limited) { + return { ok: false, error: "Too many attempts. Try again later." }; + } + + const row = await db.query.operators.findFirst({ + where: (o) => sql`lower(${o.username}) = lower(${username})`, + }); + + // User exists but has no password configured: this is a server-side + // setup error, not a credential mismatch. Surface a distinct message + // so the operator knows to run scripts/set-password.sh. We still ran + // the DB lookup, so the username-enumeration concern is not relevant + // here (the attacker would already need a known username). + if (row && row.passwordHash === null) { + return { + ok: false, + error: "Set a password via scripts/set-password.sh before signing in.", + }; + } + + // Run bcrypt regardless to keep the user-not-found path timing- + // equivalent to the wrong-password path. + const hash = row?.passwordHash ?? DUMMY_HASH; + const ok = await bcrypt.compare(password, hash); + + if (!row || !ok) { + logger.warn({ username, ip }, "login failed"); + return { ok: false, error: "Invalid username or password." }; + } + + if (row.role !== "admin" && row.role !== "user") { + return { ok: false, error: "Account is not enabled." }; + } + + const secret = process.env.AUTH_SECRET; + if (!secret) { + logger.warn({}, "AUTH_SECRET unset — cannot issue cookie"); + return { ok: false, error: "Server is not configured for sign-in." }; + } + const v = Number(process.env.OPERATOR_TOKEN_VERSION ?? "1"); + const now = Math.floor(Date.now() / 1000); + const cookie = await signSession( + { + userId: row.id, + role: row.role as Role, + iat: now, + exp: now + DEFAULT_TTL_SECONDS, + v, + }, + secret, + ); + const jar = await cookies(); + jar.set(COOKIE_NAME, cookie, { + httpOnly: true, + secure: true, + sameSite: "lax", + path: "/", + maxAge: DEFAULT_TTL_SECONDS, + }); + + redirect(safeRedirect(next)); +} + +export async function logoutAction(): Promise { + const jar = await cookies(); + jar.delete(COOKIE_NAME); + redirect("/login"); +}