From 847f4e9493c1c6fb55d63c7728d176c781592f0a Mon Sep 17 00:00:00 2001 From: Julian Appel Date: Mon, 28 Sep 2026 22:42:03 +0200 Subject: [PATCH] Harden user management integration --- src/app/config/layout.tsx | 27 +++---- src/app/config/page.tsx | 4 +- src/db/repositories/user.repository.ts | 84 ++++++++++++++------- src/db/schema/users.ts | 3 +- src/domain/ports/user.store.ts | 40 ++++++++++ src/domain/services/user.service.ts | 64 ++++++++-------- src/frontend/components/Sidebar.tsx | 24 +++--- src/frontend/components/cable-sizing-api.ts | 9 ++- src/frontend/utils/api.ts | 5 ++ src/frontend/utils/auth-api.ts | 11 ++- src/frontend/utils/config-access.ts | 25 ++++++ src/server/auth/auth.middleware.ts | 23 ++++-- src/server/auth/password.ts | 44 +++++++++-- src/server/composition/auth.ts | 7 +- src/shared/constants/request-security.ts | 2 + tests/authentication.test.ts | 67 ++++++++++++++-- tests/config-access.test.ts | 26 +++++++ tests/write-request-headers.test.ts | 40 ++++++++++ 18 files changed, 394 insertions(+), 111 deletions(-) create mode 100644 src/domain/ports/user.store.ts create mode 100644 src/frontend/utils/config-access.ts create mode 100644 src/shared/constants/request-security.ts create mode 100644 tests/config-access.test.ts create mode 100644 tests/write-request-headers.test.ts diff --git a/src/app/config/layout.tsx b/src/app/config/layout.tsx index 1356d78..72d9b6d 100644 --- a/src/app/config/layout.tsx +++ b/src/app/config/layout.tsx @@ -4,26 +4,27 @@ import Link from "next/link"; import { usePathname, useRouter } from "next/navigation"; import { useEffect } from "react"; import { useAuth } from "../../frontend/components/AuthProvider"; - -const CONFIG_TABS = [ - { href: "/config/benutzer", label: "Benutzer" }, - { href: "/config/konto", label: "Konto" }, -] as const; +import { + getConfigRedirect, + getConfigTabs, +} from "../../frontend/utils/config-access"; export default function ConfigLayout({ children }: { children: React.ReactNode }) { const pathname = usePathname(); const router = useRouter(); const { status, isAdmin } = useAuth(); + const tabs = getConfigTabs(isAdmin); + const redirectTarget = getConfigRedirect(pathname, isAdmin); useEffect(() => { - // LoginGate already guarantees a signed-in user with a set password by - // the time this layout renders; only the role still needs checking. - if (status === "signed-in" && !isAdmin) { - router.replace("/"); - } - }, [status, isAdmin, router]); + if (status !== "signed-in") return; + if (redirectTarget) router.replace(redirectTarget); + }, [status, redirectTarget, router]); - if (status !== "signed-in" || !isAdmin) { + if ( + status !== "signed-in" || + redirectTarget !== null + ) { return null; } @@ -34,7 +35,7 @@ export default function ConfigLayout({ children }: { children: React.ReactNode } - {isAdmin && ( - - )} +
{user && !isCollapsed && (
diff --git a/src/frontend/components/cable-sizing-api.ts b/src/frontend/components/cable-sizing-api.ts index 28530d2..73582b5 100644 --- a/src/frontend/components/cable-sizing-api.ts +++ b/src/frontend/components/cable-sizing-api.ts @@ -6,6 +6,10 @@ import type { CableSizingInput, CableSizingResult, } from "../../cable-sizing/domain/cable-sizing-calculation"; +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../../shared/constants/request-security"; export interface CalculateCableSizingResponse { result: CableSizingResult; @@ -17,7 +21,10 @@ export async function calculateCableSizing( ): Promise { const response = await fetch("/api/cable-sizing/calculate", { method: "POST", - headers: { "Content-Type": "application/json" }, + headers: { + "Content-Type": "application/json", + [WRITE_REQUEST_HEADER_NAME]: WRITE_REQUEST_HEADER_VALUE, + }, body: JSON.stringify(input), cache: "no-store", }); diff --git a/src/frontend/utils/api.ts b/src/frontend/utils/api.ts index 8f5a878..5256195 100644 --- a/src/frontend/utils/api.ts +++ b/src/frontend/utils/api.ts @@ -1,3 +1,7 @@ +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../../shared/constants/request-security"; import type { BuildingDto, CircuitListDto, @@ -122,6 +126,7 @@ async function request(url: string, init?: RequestInit): Promise { ...init, headers: { "Content-Type": "application/json", + [WRITE_REQUEST_HEADER_NAME]: WRITE_REQUEST_HEADER_VALUE, ...init?.headers, }, cache: "no-store", diff --git a/src/frontend/utils/auth-api.ts b/src/frontend/utils/auth-api.ts index f7057f9..a6f12b3 100644 --- a/src/frontend/utils/auth-api.ts +++ b/src/frontend/utils/auth-api.ts @@ -1,3 +1,8 @@ +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../../shared/constants/request-security"; + // Small, self-contained fetch helper for the auth/user endpoints - kept // separate from the large domain api.ts so this feature doesn't have to // touch that file's shared `request()` internals. @@ -39,7 +44,11 @@ export class AuthApiError extends Error { async function request(url: string, init?: RequestInit): Promise { const response = await fetch(url, { ...init, - headers: { "Content-Type": "application/json", ...init?.headers }, + headers: { + "Content-Type": "application/json", + [WRITE_REQUEST_HEADER_NAME]: WRITE_REQUEST_HEADER_VALUE, + ...init?.headers, + }, cache: "no-store", }); if (!response.ok) { diff --git a/src/frontend/utils/config-access.ts b/src/frontend/utils/config-access.ts new file mode 100644 index 0000000..86dd2a2 --- /dev/null +++ b/src/frontend/utils/config-access.ts @@ -0,0 +1,25 @@ +export interface ConfigNavigationEntry { + href: string; + label: string; +} + +const ACCOUNT_ENTRY = { href: "/config/konto", label: "Konto" } as const; +const USERS_ENTRY = { href: "/config/benutzer", label: "Benutzer" } as const; + +export function getConfigTabs(isAdmin: boolean): ConfigNavigationEntry[] { + return isAdmin ? [USERS_ENTRY, ACCOUNT_ENTRY] : [ACCOUNT_ENTRY]; +} + +export function getConfigEntry(isAdmin: boolean): ConfigNavigationEntry { + return isAdmin ? { href: USERS_ENTRY.href, label: "Config" } : ACCOUNT_ENTRY; +} + +export function getConfigRedirect(pathname: string | null, isAdmin: boolean): string | null { + if (pathname === "/config") { + return isAdmin ? USERS_ENTRY.href : ACCOUNT_ENTRY.href; + } + if (!isAdmin && pathname?.startsWith(USERS_ENTRY.href)) { + return ACCOUNT_ENTRY.href; + } + return null; +} diff --git a/src/server/auth/auth.middleware.ts b/src/server/auth/auth.middleware.ts index ce65bfb..bafae03 100644 --- a/src/server/auth/auth.middleware.ts +++ b/src/server/auth/auth.middleware.ts @@ -2,8 +2,12 @@ import type { NextFunction, Request, RequestHandler, Response } from "express"; import { SESSION_COOKIE, type AuthConfig } from "./auth-config.js"; import { readCookie } from "./cookie.js"; import type { SessionStore } from "./sessions.js"; -import type { UserRepository } from "../../db/repositories/user.repository.js"; +import type { UserStore } from "../../domain/ports/user.store.js"; import type { UserRole } from "../../shared/constants/user-roles.js"; +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../../shared/constants/request-security.js"; export interface AuthUser { id: string; @@ -30,7 +34,7 @@ export function sessionIdFromRequest(req: { export async function identityFromRequest( req: { headers: { cookie?: string | undefined } }, sessions: SessionStore, - users: UserRepository, + users: Pick, slideExpiry = true ): Promise { const session = sessions.get(sessionIdFromRequest(req), slideExpiry); @@ -74,10 +78,11 @@ const SAFE_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); * CSRF defense for cookie-authenticated writes. * * SameSite=Lax alone is not enough here: every other service on the same - * host (other ports of the same IP) counts as "same-site" and would get the - * cookie attached. Browsers send Sec-Fetch-Site on every request, so a write - * is only accepted from the app's own origin. Clients without the header - * (curl, scripts) carry no ambient browser cookie and are not a CSRF vector. + * host (other ports of the same IP) counts as "same-site" and may receive the + * cookie. Fetch Metadata is also not guaranteed on plain HTTP in the LAN. + * Every write therefore needs an application header that HTML forms cannot + * set; a cross-origin fetch would require a CORS preflight, which the API does + * not permit. Sec-Fetch-Site remains an additional browser-side check. */ export function requireSameOriginForWrites(): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { @@ -85,6 +90,10 @@ export function requireSameOriginForWrites(): RequestHandler { next(); return; } + if (req.get(WRITE_REQUEST_HEADER_NAME) !== WRITE_REQUEST_HEADER_VALUE) { + res.status(403).json({ error: "Schreibzugriff ohne gültigen Anwendungsnachweis abgelehnt." }); + return; + } const fetchSite = req.headers["sec-fetch-site"]; if (fetchSite === undefined || fetchSite === "same-origin" || fetchSite === "none") { next(); @@ -98,7 +107,7 @@ export function requireSameOriginForWrites(): RequestHandler { export function createAuthGuard( authConfig: AuthConfig, sessions: SessionStore, - users: UserRepository, + users: Pick, openPaths: readonly string[] = ["/auth/login", "/auth/status", "/auth/logout"] ): RequestHandler { return (req: Request, res: Response, next: NextFunction) => { diff --git a/src/server/auth/password.ts b/src/server/auth/password.ts index 96814e1..97132af 100644 --- a/src/server/auth/password.ts +++ b/src/server/auth/password.ts @@ -4,6 +4,12 @@ const SCRYPT_N = 16384; const SCRYPT_R = 8; const SCRYPT_P = 1; const KEY_LENGTH = 64; +const SALT_LENGTH = 16; +const MAX_SCRYPT_MEMORY = 64 * 1024 * 1024; +const MIN_SCRYPT_N = 1 << 14; +const MAX_SCRYPT_N = 1 << 16; +const MAX_SCRYPT_R = 16; +const MAX_SCRYPT_P = 8; // A hand-rolled wrapper instead of util.promisify(scrypt): promisify only // resolves to scrypt's 3-argument overload, so the 4-argument form used for @@ -30,8 +36,13 @@ function scryptAsync( // Format: scrypt$N$r$p$saltHex$hashHex - the parameters travel with the // hash so they can be tuned later without invalidating existing rows. export async function hashPassword(password: string): Promise { - const salt = randomBytes(16); - const derivedKey = await scryptAsync(password, salt, KEY_LENGTH); + const salt = randomBytes(SALT_LENGTH); + const derivedKey = await scryptAsync(password, salt, KEY_LENGTH, { + N: SCRYPT_N, + r: SCRYPT_R, + p: SCRYPT_P, + maxmem: MAX_SCRYPT_MEMORY, + }); return [ "scrypt", SCRYPT_N, @@ -52,12 +63,33 @@ export async function verifyPassword( const n = Number(nRaw); const r = Number(rRaw); const p = Number(pRaw); - if (!Number.isFinite(n) || !Number.isFinite(r) || !Number.isFinite(p)) { + if ( + !Number.isSafeInteger(n) || + n < MIN_SCRYPT_N || + n > MAX_SCRYPT_N || + (n & (n - 1)) !== 0 || + !Number.isSafeInteger(r) || + r < 1 || + r > MAX_SCRYPT_R || + !Number.isSafeInteger(p) || + p < 1 || + p > MAX_SCRYPT_P || + !/^[0-9a-f]{32}$/i.test(saltHex) || + !/^[0-9a-f]{128}$/i.test(hashHex) + ) { return false; } const salt = Buffer.from(saltHex, "hex"); const expected = Buffer.from(hashHex, "hex"); - const derivedKey = await scryptAsync(password, salt, expected.length, { N: n, r, p }); - if (derivedKey.length !== expected.length) return false; - return timingSafeEqual(derivedKey, expected); + try { + const derivedKey = await scryptAsync(password, salt, expected.length, { + N: n, + r, + p, + maxmem: MAX_SCRYPT_MEMORY, + }); + return timingSafeEqual(derivedKey, expected); + } catch { + return false; + } } diff --git a/src/server/composition/auth.ts b/src/server/composition/auth.ts index 1b5ce09..07b822b 100644 --- a/src/server/composition/auth.ts +++ b/src/server/composition/auth.ts @@ -3,9 +3,14 @@ import { LoginThrottle } from "../auth/login-throttle.js"; import { SessionStore } from "../auth/sessions.js"; import { UserService } from "../../domain/services/user.service.js"; import { userRepository } from "./application-repositories.js"; +import { hashPassword, verifyPassword } from "../auth/password.js"; export const authConfig = readAuthConfig(); export const sessionStore = new SessionStore(authConfig.sessionTtlMs, authConfig.sessionMaxAgeMs); sessionStore.startSweeping(); export const loginThrottle = new LoginThrottle(); -export const userService = new UserService(userRepository, authConfig); +export const userService = new UserService( + userRepository, + { bootstrap: authConfig.bootstrap }, + { hash: hashPassword, verify: verifyPassword } +); diff --git a/src/shared/constants/request-security.ts b/src/shared/constants/request-security.ts new file mode 100644 index 0000000..f89ce10 --- /dev/null +++ b/src/shared/constants/request-security.ts @@ -0,0 +1,2 @@ +export const WRITE_REQUEST_HEADER_NAME = "X-Leistungsbilanz-Request"; +export const WRITE_REQUEST_HEADER_VALUE = "1"; diff --git a/tests/authentication.test.ts b/tests/authentication.test.ts index e634716..e5c431d 100644 --- a/tests/authentication.test.ts +++ b/tests/authentication.test.ts @@ -21,6 +21,10 @@ import { requireSameOriginForWrites } from "../src/server/auth/auth.middleware.j import type { AuthConfig } from "../src/server/auth/auth-config.js"; import { createUserSchema } from "../src/shared/validation/user.schemas.js"; import { loginSchema } from "../src/shared/validation/auth.schemas.js"; +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../src/shared/constants/request-security.js"; const HOUR = 60 * 60 * 1000; @@ -34,7 +38,14 @@ function createUserService() { cookieSecure: false, bootstrap: { username: "admin", password: "bootstrap-password-123" }, }; - return { repository, service: new UserService(repository, config) }; + return { + repository, + service: new UserService( + repository, + { bootstrap: config.bootstrap }, + { hash: hashPassword, verify: verifyPassword } + ), + }; } describe("password hashing", () => { @@ -52,6 +63,17 @@ describe("password hashing", () => { it("rejects malformed stored hashes instead of throwing", async () => { assert.equal(await verifyPassword("x", "not-a-hash"), false); assert.equal(await verifyPassword("x", "scrypt$a$b$c$00$00"), false); + assert.equal( + await verifyPassword( + "x", + `scrypt$1073741824$8$1$${"00".repeat(16)}$${"00".repeat(64)}` + ), + false + ); + assert.equal( + await verifyPassword("x", `scrypt$16384$8$1$00$${"00".repeat(64)}`), + false + ); }); }); @@ -135,12 +157,20 @@ describe("cookie parsing", () => { }); describe("same-origin guard for writes", () => { - function run(method: string, fetchSite?: string) { + function run(method: string, fetchSite?: string, includeApplicationHeader = true) { let status: number | undefined; let passed = false; const req = { method, - headers: fetchSite ? { "sec-fetch-site": fetchSite } : {}, + headers: { + ...(fetchSite ? { "sec-fetch-site": fetchSite } : {}), + ...(includeApplicationHeader + ? { [WRITE_REQUEST_HEADER_NAME.toLowerCase()]: WRITE_REQUEST_HEADER_VALUE } + : {}), + }, + get(name: string) { + return this.headers[name.toLowerCase() as keyof typeof this.headers]; + }, } as unknown as Request; const res = { status(code: number) { @@ -158,12 +188,13 @@ describe("same-origin guard for writes", () => { } it("lets reads and same-origin writes through", () => { - assert.equal(run("GET", "cross-site").passed, true); + assert.equal(run("GET", "cross-site", false).passed, true); assert.equal(run("POST", "same-origin").passed, true); - assert.equal(run("POST").passed, true, "non-browser clients carry no ambient cookie"); + assert.equal(run("POST").passed, true, "scripts can send the explicit application header"); }); - it("rejects writes from other sites and other ports of the same host", () => { + it("rejects writes without the application header or from another origin", () => { + assert.deepEqual(run("POST", undefined, false), { passed: false, status: 403 }); assert.deepEqual(run("POST", "cross-site"), { passed: false, status: 403 }); assert.deepEqual(run("PATCH", "same-site"), { passed: false, status: 403 }); assert.deepEqual(run("DELETE", "same-site"), { passed: false, status: 403 }); @@ -239,6 +270,30 @@ describe("user service", () => { await assert.rejects(service.updateRoleAndActive(second.id, { active: false }), LastAdminError); }); + it("preserves one active admin across concurrent updates", async () => { + const { repository, service } = createUserService(); + await service.ensureBootstrapAdmin(); + const second = await service.createUser({ + username: "zweiter-admin", + name: "Zweiter Admin", + role: "admin", + initialPassword: "start-passwort", + }); + assert.ok(second); + const [first] = (await repository.list()).filter((user) => user.username === "admin"); + + const results = await Promise.allSettled([ + service.updateRoleAndActive(first.id, { role: "user" }), + service.updateRoleAndActive(second.id, { role: "user" }), + ]); + assert.equal(results.filter((result) => result.status === "fulfilled").length, 1); + assert.equal(results.filter((result) => result.status === "rejected").length, 1); + const activeAdmins = (await repository.list()).filter( + (user) => user.role === "admin" && user.active + ); + assert.equal(activeAdmins.length, 1); + }); + it("requires the current password and a different new one", async () => { const { service } = createUserService(); const user = await service.createUser({ diff --git a/tests/config-access.test.ts b/tests/config-access.test.ts new file mode 100644 index 0000000..3fdc83d --- /dev/null +++ b/tests/config-access.test.ts @@ -0,0 +1,26 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + getConfigEntry, + getConfigRedirect, + getConfigTabs, +} from "../src/frontend/utils/config-access.js"; + +describe("config access", () => { + it("offers account settings to every signed-in user", () => { + assert.deepEqual(getConfigEntry(false), { href: "/config/konto", label: "Konto" }); + assert.deepEqual(getConfigTabs(false), [{ href: "/config/konto", label: "Konto" }]); + assert.equal(getConfigRedirect("/config", false), "/config/konto"); + assert.equal(getConfigRedirect("/config/konto", false), null); + }); + + it("keeps user administration admin-only", () => { + assert.equal(getConfigRedirect("/config/benutzer", false), "/config/konto"); + assert.deepEqual(getConfigTabs(true), [ + { href: "/config/benutzer", label: "Benutzer" }, + { href: "/config/konto", label: "Konto" }, + ]); + assert.deepEqual(getConfigEntry(true), { href: "/config/benutzer", label: "Config" }); + assert.equal(getConfigRedirect("/config/benutzer", true), null); + }); +}); diff --git a/tests/write-request-headers.test.ts b/tests/write-request-headers.test.ts new file mode 100644 index 0000000..c78a277 --- /dev/null +++ b/tests/write-request-headers.test.ts @@ -0,0 +1,40 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { authApi } from "../src/frontend/utils/auth-api.js"; +import { createProject } from "../src/frontend/utils/api.js"; +import { calculateCableSizing } from "../src/frontend/components/cable-sizing-api.js"; +import type { CableSizingInput } from "../src/cable-sizing/domain/cable-sizing-calculation.js"; +import { + WRITE_REQUEST_HEADER_NAME, + WRITE_REQUEST_HEADER_VALUE, +} from "../src/shared/constants/request-security.js"; + +describe("write request clients", () => { + it("send the application header from every frontend API client", async () => { + const originalFetch = globalThis.fetch; + const requests: RequestInit[] = []; + globalThis.fetch = async (_input, init) => { + requests.push(init ?? {}); + return new Response(JSON.stringify({}), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }; + + try { + await authApi.login("admin", "start-passwort"); + await createProject("Testprojekt"); + await calculateCableSizing({} as CableSizingInput); + } finally { + globalThis.fetch = originalFetch; + } + + assert.equal(requests.length, 3); + for (const request of requests) { + assert.equal( + new Headers(request.headers).get(WRITE_REQUEST_HEADER_NAME), + WRITE_REQUEST_HEADER_VALUE + ); + } + }); +});