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.
This commit is contained in:
parent
d236196476
commit
cedd623466
189
apps/web/src/actions/auth.test.ts
Normal file
189
apps/web/src/actions/auth.test.ts
Normal file
@ -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<string, string>): 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");
|
||||||
|
});
|
||||||
|
});
|
||||||
116
apps/web/src/actions/auth.ts
Normal file
116
apps/web/src/actions/auth.ts
Normal file
@ -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<string> {
|
||||||
|
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<LoginResult> {
|
||||||
|
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<void> {
|
||||||
|
const jar = await cookies();
|
||||||
|
jar.delete(COOKIE_NAME);
|
||||||
|
redirect("/login");
|
||||||
|
}
|
||||||
Loading…
x
Reference in New Issue
Block a user