From 5073d9342a287ec6d961f53b5178b7ca344ef3f2 Mon Sep 17 00:00:00 2001 From: Grovy311 Date: Mon, 28 Sep 2026 21:30:30 +0200 Subject: [PATCH] Harden authentication and put filter toggle and fields in one row Security: per-username login throttling, constant-time unknown-user check, Sec-Fetch-Site CSRF guard for writes, session rotation on login and password change, absolute session lifetime, sessions ended on reset and deactivation, last-admin protection, input length caps, ASCII usernames, clickjacking headers. Covered by tests/authentication.test.ts. Filter panel: toggle, fields and count share one row to save space. Co-Authored-By: Claude Opus 5.5 --- compose.yaml | 1 + docs/authentication.md | 28 ++- next.config.mjs | 16 ++ src/app/globals.css | 21 +- src/domain/services/user.service.ts | 67 +++++- src/frontend/components/filter-panel.tsx | 47 ++-- src/server/auth/auth-config.ts | 23 +- src/server/auth/auth.middleware.ts | 48 +++- src/server/auth/cookie.ts | 7 +- src/server/auth/login-throttle.ts | 53 +++++ src/server/auth/sessions.ts | 60 ++++- src/server/composition/auth.ts | 5 +- src/server/controllers/auth.controller.ts | 47 ++-- src/server/controllers/user.controller.ts | 32 ++- src/server/index.ts | 8 +- src/shared/validation/auth.schemas.ts | 13 +- src/shared/validation/user.schemas.ts | 23 +- tests/authentication.test.ts | 265 ++++++++++++++++++++++ 18 files changed, 683 insertions(+), 81 deletions(-) create mode 100644 src/server/auth/login-throttle.ts create mode 100644 tests/authentication.test.ts diff --git a/compose.yaml b/compose.yaml index bab8c3d..2a2eecd 100644 --- a/compose.yaml +++ b/compose.yaml @@ -51,6 +51,7 @@ services: AUTH_BOOTSTRAP_USERNAME: "${AUTH_BOOTSTRAP_USERNAME:-}" AUTH_BOOTSTRAP_PASSWORT: "${AUTH_BOOTSTRAP_PASSWORT:-}" AUTH_SESSION_HOURS: "${AUTH_SESSION_HOURS:-12}" + AUTH_SESSION_MAX_DAYS: "${AUTH_SESSION_MAX_DAYS:-7}" COOKIE_SECURE: "${COOKIE_SECURE:-false}" # Nicht auf dem Host veroeffentlicht: die API wird ueber den # Next.js-Rewrite im Web-Dienst erreicht. diff --git a/docs/authentication.md b/docs/authentication.md index 3d197cf..13a9f72 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -33,7 +33,8 @@ Verknüpfung mit einer Mitarbeiter-/Personen-Stammdatentabelle. `username`, `passwordHash` (scrypt, `src/server/auth/password.ts`), `name`, `role`, `active`, `mustChangePassword`. - **Sessions:** In-Memory-Store (`src/server/auth/sessions.ts`), Cookie - `lb_session`, sliding expiry (`AUTH_SESSION_HOURS`, Standard 12h). Ein + `lb_session`, sliding expiry (`AUTH_SESSION_HOURS`, Standard 12h) mit + absoluter Obergrenze (`AUTH_SESSION_MAX_DAYS`, Standard 7). Ein Neustart der API beendet alle Sessions – für einen Single-Container-Dienst ein akzeptabler Kompromiss gegenüber einer persistierten Session-Tabelle. - **Middleware:** `src/server/auth/auth.middleware.ts` schützt `/api/*` @@ -44,6 +45,28 @@ Verknüpfung mit einer Mitarbeiter-/Personen-Stammdatentabelle. (`src/app/layout.tsx`) zeigt Login- bzw. Passwortformular, solange nicht angemeldet bzw. `mustChangePassword` gesetzt ist, sonst die App. +## Schutzmechanismen + +| Angriff | Schutz | +|---|---| +| Passwort-Raten (online) | `LoginThrottle`: 5 Fehlversuche je Anmeldename in 15 min → HTTP 429 bis das Fenster abläuft. Schlüssel ist der Name, nicht die IP (alle Anfragen kommen über den Next-Rewrite mit derselben Quell-IP). | +| Passwort-Raten (offline, DB-Diebstahl) | scrypt (N=16384, r=8, p=1), eigenes Salt je Hash | +| Nutzernamen ausspähen (Timing) | Unbekannte Namen werden gegen einen festen Dummy-Hash geprüft – immer genau ein scrypt-Lauf; einheitliche Fehlermeldung | +| CSRF | SameSite=Lax **plus** `requireSameOriginForWrites`: schreibende Anfragen nur mit `Sec-Fetch-Site: same-origin`. Nötig, weil andere Dienste auf demselben Host (andere Ports) als „same-site“ gelten | +| Session-Fixation | Neue Session-ID bei jedem Login und nach jedem Passwortwechsel | +| Gestohlenes Cookie | httpOnly; gleitende Laufzeit (`AUTH_SESSION_HOURS`, 12 h) **und** harte Obergrenze (`AUTH_SESSION_MAX_DAYS`, 7 Tage) | +| Kompromittiertes Konto | Passwort-Reset und Deaktivieren beenden sofort alle Sessions des Kontos; Passwortwechsel beendet alle anderen | +| Aussperren | Der letzte aktive Admin kann weder herabgestuft noch deaktiviert werden; eigener Zugang ist in der Liste gesperrt | +| Rechteausweitung / Mass Assignment | Rolle kommt aus der DB, nie aus der Anfrage; zod-Schemas mit `.strict()` | +| DoS über riesige Passwörter | Längengrenzen (Name 100, Passwort 200 Zeichen) vor scrypt | +| Look-alike-Namen | Anmeldename nur `a-z 0-9 . _ @ -`, klein geschrieben | +| Clickjacking | `X-Frame-Options: SAMEORIGIN`, `frame-ancestors 'self'` (next.config.mjs) | +| Speicherwachstum | Max. 20 Sessions je Nutzer, periodisches Aufräumen abgelaufener Sessions, Throttle-Tabelle begrenzt | + +**Nicht durch Code lösbar:** Ohne TLS gehen Passwort und Session-Cookie im +Klartext durchs Netz. Für Zugriff außerhalb des LANs HTTPS über einen +Reverse-Proxy vorschalten und `COOKIE_SECURE=true` setzen. + ## Bootstrap-Admin `AUTH_BOOTSTRAP_USERNAME` / `AUTH_BOOTSTRAP_PASSWORT` (Env-Variablen, siehe @@ -56,6 +79,7 @@ in `compose.yaml` selbst. ## Bewusst nicht umgesetzt (Stand jetzt) - Konfigurierbare Rechte-Matrix statt fixer `admin`/`user`-Rollen -- Konten-Sperre nach Fehlversuchen +- Dauerhafte Konten-Sperre nach Fehlversuchen (es gibt nur die zeitlich + begrenzte Drosselung, siehe oben) - E-Mail-/Einmal-Link-Passwortvergabe, OIDC/Entra-Anmeldung - Verknüpfung von Zugängen mit einer Mitarbeiter-Stammdatentabelle diff --git a/next.config.mjs b/next.config.mjs index e0610b9..a6cb01e 100644 --- a/next.config.mjs +++ b/next.config.mjs @@ -11,6 +11,22 @@ const nextConfig = { typescript: { tsconfigPath: "./tsconfig.next.json", }, + poweredByHeader: false, + // Clickjacking: the Config pages hold admin actions, so no foreign page + // may frame the app. Same-origin framing stays allowed. + async headers() { + return [ + { + source: "/:path*", + headers: [ + { key: "X-Frame-Options", value: "SAMEORIGIN" }, + { key: "Content-Security-Policy", value: "frame-ancestors 'self'" }, + { key: "X-Content-Type-Options", value: "nosniff" }, + { key: "Referrer-Policy", value: "same-origin" }, + ], + }, + ]; + }, async rewrites() { return [ { diff --git a/src/app/globals.css b/src/app/globals.css index fec2745..0f1a642 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -452,13 +452,25 @@ a { background: var(--color-bg); } -.filter-panel-header { +.filter-panel-row { display: flex; align-items: center; gap: 10px; } -.filter-panel-header .filter-panel-toggle { +/* Expanded: the fields carry a label above their input, so toggle and + count sit on the input line (bottom) instead of the label line. */ +.filter-panel-row-expanded { + align-items: flex-end; +} + +.filter-panel-row-expanded .filter-panel-toggle, +.filter-panel-row-expanded .filter-panel-count { + margin-bottom: calc((2.1rem - 24px) / 2); +} + +.filter-panel-row .filter-panel-toggle { + flex: 0 0 auto; width: 24px; height: 24px; padding: 0; @@ -468,7 +480,7 @@ a { color: var(--color-ink-soft); } -.filter-panel-header .filter-panel-toggle:hover { +.filter-panel-row .filter-panel-toggle:hover { background: var(--color-surface); } @@ -503,10 +515,11 @@ a { .filter-bar { display: flex; + flex: 1 1 auto; + min-width: 0; flex-wrap: wrap; align-items: flex-end; gap: var(--space-sm) var(--space-md); - margin-top: 0.5rem; } .filter-bar-field, diff --git a/src/domain/services/user.service.ts b/src/domain/services/user.service.ts index 55720c1..0968d02 100644 --- a/src/domain/services/user.service.ts +++ b/src/domain/services/user.service.ts @@ -24,7 +24,29 @@ export class WrongPasswordError extends Error { } } +export class PasswordUnchangedError extends Error { + constructor() { + super("Das neue Passwort muss sich vom bisherigen unterscheiden."); + } +} + +export class LastAdminError extends Error { + constructor() { + super("Es muss mindestens ein aktiver Administrator bleiben."); + } +} + +function isUniqueConstraintError(error: unknown): boolean { + return ( + error instanceof Error && + "code" in error && + (error as { code?: unknown }).code === "SQLITE_CONSTRAINT_UNIQUE" + ); +} + export class UserService { + private dummyHash: Promise | null = null; + constructor( private readonly repository: UserRepository, private readonly authConfig: AuthConfig @@ -38,6 +60,9 @@ export class UserService { * escape hatch instead of a standing backdoor. */ async ensureBootstrapAdmin(): Promise { + // Warm the dummy hash at startup so even the first unknown-user login + // costs one scrypt run, not two. + await this.getDummyHash(); if (!this.authConfig.bootstrap) return; const existingCount = await this.repository.count(); if (existingCount > 0) return; @@ -55,9 +80,10 @@ export class UserService { async authenticate(username: string, password: string) { const user = await this.repository.findByUsername(username); - // Always hash something, even for an unknown user: otherwise the - // response time would leak which usernames exist. - const passwordHash = user?.passwordHash ?? (await hashPassword("placeholder")); + // Unknown users are verified against a fixed dummy hash: exactly one + // scrypt run either way, so response time doesn't reveal which usernames + // exist. + const passwordHash = user?.passwordHash ?? (await this.getDummyHash()); const passwordMatches = await verifyPassword(password, passwordHash); if (!user || !user.active || !passwordMatches) { throw new InvalidCredentialsError(); @@ -74,15 +100,32 @@ export class UserService { const existing = await this.repository.findByUsername(input.username); if (existing) throw new UsernameTakenError(); const passwordHash = await hashPassword(input.initialPassword); - return this.repository.create({ - username: input.username, - name: input.name, - role: input.role, - passwordHash, - }); + try { + return await this.repository.create({ + username: input.username, + name: input.name, + role: input.role, + passwordHash, + }); + } catch (error) { + // Two admins creating the same name at once: the unique index decides. + if (isUniqueConstraintError(error)) throw new UsernameTakenError(); + throw error; + } } async updateRoleAndActive(userId: string, patch: { role?: UserRole; active?: boolean }) { + const removesAdmin = patch.role === "user" || patch.active === false; + if (removesAdmin) { + const target = await this.repository.findById(userId); + const targetIsActiveAdmin = target?.role === "admin" && target.active; + if (targetIsActiveAdmin) { + const activeAdmins = (await this.repository.list()).filter( + (user) => user.role === "admin" && user.active + ); + if (activeAdmins.length <= 1) throw new LastAdminError(); + } + } return this.repository.updateRoleAndActive(userId, patch); } @@ -98,7 +141,13 @@ export class UserService { if (!user) throw new WrongPasswordError(); const currentMatches = await verifyPassword(currentPassword, user.passwordHash); if (!currentMatches) throw new WrongPasswordError(); + if (newPassword === currentPassword) throw new PasswordUnchangedError(); const passwordHash = await hashPassword(newPassword); return this.repository.setPassword(userId, passwordHash, false); } + + private getDummyHash(): Promise { + this.dummyHash ??= hashPassword("dummy-password-for-constant-time-login"); + return this.dummyHash; + } } diff --git a/src/frontend/components/filter-panel.tsx b/src/frontend/components/filter-panel.tsx index afa1fde..9c9923b 100644 --- a/src/frontend/components/filter-panel.tsx +++ b/src/frontend/components/filter-panel.tsx @@ -55,9 +55,15 @@ export function CollapsibleFilterPanel({ writeCollapsed(storageKey, next); } + // One row: toggle, then either the "Filter" label (collapsed) or the filter + // fields themselves (expanded), and the count at the right edge. return (
-
+
- Filter - {isCollapsed && isFilterActive ? ( - Filter aktiv - ) : null} + {isCollapsed ? ( + <> + Filter + {isFilterActive ? Filter aktiv : null} + + ) : ( +
+ {children} + {isFilterActive ? ( + + ) : null} +
+ )} {visibleCount} von {totalCount}
- {isCollapsed ? null : ( -
- {children} - {isFilterActive ? ( - - ) : null} -
- )}
); } diff --git a/src/server/auth/auth-config.ts b/src/server/auth/auth-config.ts index d2157f4..c4c3b44 100644 --- a/src/server/auth/auth-config.ts +++ b/src/server/auth/auth-config.ts @@ -1,9 +1,13 @@ export const SESSION_COOKIE = "lb_session"; const DEFAULT_SESSION_HOURS = 12; +const DEFAULT_SESSION_MAX_DAYS = 7; export interface AuthConfig { + /** Idle timeout; slides forward on activity. */ sessionTtlMs: number; + /** Hard upper bound for a session, regardless of activity. */ + sessionMaxAgeMs: number; cookieSecure: boolean; /** * Only used to create the very first admin when the users table is still @@ -15,11 +19,21 @@ export interface AuthConfig { export class AuthConfigError extends Error {} -export function readAuthConfig(env: NodeJS.ProcessEnv = process.env): AuthConfig { - const hours = Number(env.AUTH_SESSION_HOURS ?? DEFAULT_SESSION_HOURS); - if (!Number.isFinite(hours) || hours <= 0) { - throw new AuthConfigError("AUTH_SESSION_HOURS must be a positive number."); +function positiveNumber(raw: string | undefined, fallback: number, name: string): number { + const value = Number(raw ?? fallback); + if (!Number.isFinite(value) || value <= 0) { + throw new AuthConfigError(`${name} must be a positive number.`); } + return value; +} + +export function readAuthConfig(env: NodeJS.ProcessEnv = process.env): AuthConfig { + const hours = positiveNumber(env.AUTH_SESSION_HOURS, DEFAULT_SESSION_HOURS, "AUTH_SESSION_HOURS"); + const maxDays = positiveNumber( + env.AUTH_SESSION_MAX_DAYS, + DEFAULT_SESSION_MAX_DAYS, + "AUTH_SESSION_MAX_DAYS" + ); const bootstrapUsername = (env.AUTH_BOOTSTRAP_USERNAME ?? "").trim().toLowerCase(); const bootstrapPassword = env.AUTH_BOOTSTRAP_PASSWORT ?? ""; @@ -36,6 +50,7 @@ export function readAuthConfig(env: NodeJS.ProcessEnv = process.env): AuthConfig return { sessionTtlMs: hours * 60 * 60 * 1000, + sessionMaxAgeMs: maxDays * 24 * 60 * 60 * 1000, cookieSecure: (env.COOKIE_SECURE ?? "").trim().toLowerCase() === "true", bootstrap: bootstrapUsername ? { username: bootstrapUsername, password: bootstrapPassword } diff --git a/src/server/auth/auth.middleware.ts b/src/server/auth/auth.middleware.ts index 53a1767..ce65bfb 100644 --- a/src/server/auth/auth.middleware.ts +++ b/src/server/auth/auth.middleware.ts @@ -46,17 +46,55 @@ export async function identityFromRequest( }; } -export function cookieOptions(authConfig: AuthConfig) { +export function cookieOptions(authConfig: AuthConfig, maxAgeMs: number) { return { httpOnly: true, sameSite: "lax" as const, secure: authConfig.cookieSecure, path: "/", - maxAge: authConfig.sessionTtlMs, + maxAge: maxAgeMs, }; } -/** Protects /api. Only the login route and the status check stay reachable without a session. */ +/** Sets the session cookie with the same remaining lifetime the server uses. */ +export function setSessionCookie( + res: Response, + authConfig: AuthConfig, + sessions: SessionStore, + sessionId: string +): void { + const session = sessions.get(sessionId, false); + if (!session) return; + res.cookie(SESSION_COOKIE, sessionId, cookieOptions(authConfig, sessions.remainingMs(session))); +} + +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. + */ +export function requireSameOriginForWrites(): RequestHandler { + return (req: Request, res: Response, next: NextFunction) => { + if (SAFE_METHODS.has(req.method)) { + next(); + return; + } + const fetchSite = req.headers["sec-fetch-site"]; + if (fetchSite === undefined || fetchSite === "same-origin" || fetchSite === "none") { + next(); + return; + } + res.status(403).json({ error: "Anfrage von fremder Herkunft abgelehnt." }); + }; +} + +/** Protects /api. Only login, status and logout stay reachable without a session. */ export function createAuthGuard( authConfig: AuthConfig, sessions: SessionStore, @@ -76,9 +114,7 @@ export function createAuthGuard( } req.authUser = identity; const sessionId = sessionIdFromRequest(req); - if (sessionId) { - res.cookie(SESSION_COOKIE, sessionId, cookieOptions(authConfig)); - } + if (sessionId) setSessionCookie(res, authConfig, sessions, sessionId); next(); }) .catch(next); diff --git a/src/server/auth/cookie.ts b/src/server/auth/cookie.ts index 0204ca3..344ff65 100644 --- a/src/server/auth/cookie.ts +++ b/src/server/auth/cookie.ts @@ -9,7 +9,12 @@ export function readCookie( if (separatorIndex === -1) continue; const key = part.slice(0, separatorIndex).trim(); if (key !== name) continue; - return decodeURIComponent(part.slice(separatorIndex + 1).trim()); + // A malformed value must read as "no session", not throw into a 500. + try { + return decodeURIComponent(part.slice(separatorIndex + 1).trim()); + } catch { + return undefined; + } } return undefined; } diff --git a/src/server/auth/login-throttle.ts b/src/server/auth/login-throttle.ts new file mode 100644 index 0000000..d81719b --- /dev/null +++ b/src/server/auth/login-throttle.ts @@ -0,0 +1,53 @@ +/** + * Slows down online password guessing per username. + * + * Keyed by username, not by client IP: every request reaches the API through + * the Next.js rewrite, so the socket address is always the web container. + * The trade-off is that someone who knows a username can keep that account + * temporarily throttled; the block lifts by itself after the window, so it is + * a nuisance, not a lockout. + */ +export class LoginThrottle { + private readonly failures = new Map(); + + constructor( + private readonly maxFailures = 5, + private readonly windowMs = 15 * 60 * 1000, + private readonly maxTrackedKeys = 10_000, + private readonly now: () => number = Date.now + ) {} + + /** Milliseconds until another attempt is allowed; 0 = allowed now. */ + retryAfterMs(username: string): number { + const recent = this.recentFailures(this.key(username)); + if (recent.length < this.maxFailures) return 0; + return Math.max(0, recent[0] + this.windowMs - this.now()); + } + + recordFailure(username: string): void { + const key = this.key(username); + const recent = this.recentFailures(key); + recent.push(this.now()); + if (!this.failures.has(key) && this.failures.size >= this.maxTrackedKeys) { + // Bound memory under a flood of random usernames: drop the oldest key. + const oldestKey = this.failures.keys().next().value; + if (oldestKey !== undefined) this.failures.delete(oldestKey); + } + this.failures.set(key, recent); + } + + recordSuccess(username: string): void { + this.failures.delete(this.key(username)); + } + + private recentFailures(key: string): number[] { + const cutoff = this.now() - this.windowMs; + const recent = (this.failures.get(key) ?? []).filter((time) => time > cutoff); + if (recent.length === 0) this.failures.delete(key); + return recent; + } + + private key(username: string): string { + return username.trim().toLowerCase(); + } +} diff --git a/src/server/auth/sessions.ts b/src/server/auth/sessions.ts index 501f4c4..9ee1fe0 100644 --- a/src/server/auth/sessions.ts +++ b/src/server/auth/sessions.ts @@ -3,24 +3,50 @@ import { randomUUID } from "node:crypto"; interface Session { id: string; userId: string; + createdAt: number; expiresAt: number; } +const MAX_SESSIONS_PER_USER = 20; +const SWEEP_INTERVAL_MS = 10 * 60 * 1000; + /** * In-memory session store. The API runs as a single container/process, so a * restart clearing every session is an acceptable trade-off for the * simplicity of not needing a persisted session table. + * + * Sessions slide forward on activity (`idleTtlMs`) but never outlive + * `absoluteTtlMs` - otherwise a stolen cookie could be kept alive forever. */ export class SessionStore { private readonly sessions = new Map(); - constructor(private readonly ttlMs: number) {} + constructor( + private readonly idleTtlMs: number, + private readonly absoluteTtlMs: number, + private readonly now: () => number = Date.now + ) {} + + /** Removes expired sessions; unreferenced ones would otherwise pile up. */ + startSweeping(): void { + setInterval(() => this.sweep(), SWEEP_INTERVAL_MS).unref(); + } + + sweep(): void { + const now = this.now(); + for (const [id, session] of this.sessions) { + if (this.isExpired(session, now)) this.sessions.delete(id); + } + } create(userId: string): Session { + this.enforcePerUserLimit(userId); + const now = this.now(); const session: Session = { id: randomUUID(), userId, - expiresAt: Date.now() + this.ttlMs, + createdAt: now, + expiresAt: Math.min(now + this.idleTtlMs, now + this.absoluteTtlMs), }; this.sessions.set(session.id, session); return session; @@ -31,20 +57,28 @@ export class SessionStore { if (!sessionId) return undefined; const session = this.sessions.get(sessionId); if (!session) return undefined; - if (session.expiresAt < Date.now()) { + const now = this.now(); + if (this.isExpired(session, now)) { this.sessions.delete(sessionId); return undefined; } - if (slideExpiry) session.expiresAt = Date.now() + this.ttlMs; + if (slideExpiry) { + session.expiresAt = Math.min(now + this.idleTtlMs, session.createdAt + this.absoluteTtlMs); + } return session; } + /** Remaining lifetime, used as cookie max-age so browser and server agree. */ + remainingMs(session: Session): number { + return Math.max(0, session.expiresAt - this.now()); + } + destroy(sessionId: string | undefined): void { if (!sessionId) return; this.sessions.delete(sessionId); } - /** Ends every other session for a user, e.g. after a password change. */ + /** Ends every session of a user, optionally except one (e.g. the caller's own). */ destroyAllForUser(userId: string, exceptSessionId?: string): void { for (const [id, session] of this.sessions) { if (session.userId === userId && id !== exceptSessionId) { @@ -52,4 +86,20 @@ export class SessionStore { } } } + + private isExpired(session: Session, now: number): boolean { + return session.expiresAt <= now || session.createdAt + this.absoluteTtlMs <= now; + } + + // Oldest sessions go first, so a legitimate user logging in on a new + // device is never refused - only memory growth is bounded. + private enforcePerUserLimit(userId: string): void { + const own = [...this.sessions.values()] + .filter((session) => session.userId === userId) + .sort((a, b) => a.createdAt - b.createdAt); + while (own.length >= MAX_SESSIONS_PER_USER) { + const oldest = own.shift(); + if (oldest) this.sessions.delete(oldest.id); + } + } } diff --git a/src/server/composition/auth.ts b/src/server/composition/auth.ts index 751f324..1b5ce09 100644 --- a/src/server/composition/auth.ts +++ b/src/server/composition/auth.ts @@ -1,8 +1,11 @@ import { readAuthConfig } from "../auth/auth-config.js"; +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"; export const authConfig = readAuthConfig(); -export const sessionStore = new SessionStore(authConfig.sessionTtlMs); +export const sessionStore = new SessionStore(authConfig.sessionTtlMs, authConfig.sessionMaxAgeMs); +sessionStore.startSweeping(); +export const loginThrottle = new LoginThrottle(); export const userService = new UserService(userRepository, authConfig); diff --git a/src/server/controllers/auth.controller.ts b/src/server/controllers/auth.controller.ts index 9dabee5..5eaf699 100644 --- a/src/server/controllers/auth.controller.ts +++ b/src/server/controllers/auth.controller.ts @@ -1,11 +1,16 @@ import type { Request, Response } from "express"; import { changePasswordSchema, loginSchema } from "../../shared/validation/auth.schemas.js"; -import { InvalidCredentialsError, WrongPasswordError } from "../../domain/services/user.service.js"; -import { authConfig, sessionStore, userService } from "../composition/auth.js"; +import { + InvalidCredentialsError, + PasswordUnchangedError, + WrongPasswordError, +} from "../../domain/services/user.service.js"; +import { authConfig, loginThrottle, sessionStore, userService } from "../composition/auth.js"; import { cookieOptions, identityFromRequest, sessionIdFromRequest, + setSessionCookie, } from "../auth/auth.middleware.js"; import { SESSION_COOKIE } from "../auth/auth-config.js"; import { userRepository } from "../composition/application-repositories.js"; @@ -13,9 +18,7 @@ import { userRepository } from "../composition/application-repositories.js"; export async function getAuthStatus(req: Request, res: Response) { const identity = await identityFromRequest(req, sessionStore, userRepository); const sessionId = sessionIdFromRequest(req); - if (identity && sessionId) { - res.cookie(SESSION_COOKIE, sessionId, cookieOptions(authConfig)); - } + if (identity && sessionId) setSessionCookie(res, authConfig, sessionStore, sessionId); res.json({ signedIn: Boolean(identity), mustChangePassword: identity?.mustChangePassword ?? false, @@ -28,13 +31,29 @@ export async function login(req: Request, res: Response) { res.status(401).json({ error: "Anmeldung fehlgeschlagen." }); return; } + const { username, password } = parsed.data; + + const retryAfterMs = loginThrottle.retryAfterMs(username); + if (retryAfterMs > 0) { + res.setHeader("Retry-After", Math.ceil(retryAfterMs / 1000).toString()); + res.status(429).json({ + error: "Zu viele Fehlversuche. Bitte in einigen Minuten erneut versuchen.", + }); + return; + } + try { - const user = await userService.authenticate(parsed.data.username, parsed.data.password); + const user = await userService.authenticate(username, password); + loginThrottle.recordSuccess(username); + // A fresh session id on every login: an id planted before login + // (session fixation) never becomes authenticated. + sessionStore.destroy(sessionIdFromRequest(req)); const session = sessionStore.create(user.id); - res.cookie(SESSION_COOKIE, session.id, cookieOptions(authConfig)); + setSessionCookie(res, authConfig, sessionStore, session.id); res.json({ signedIn: true, mustChangePassword: user.mustChangePassword }); } catch (error) { if (error instanceof InvalidCredentialsError) { + loginThrottle.recordFailure(username); res.status(401).json({ error: error.message }); return; } @@ -59,7 +78,7 @@ export async function changePassword(req: Request, res: Response) { } const parsed = changePasswordSchema.safeParse(req.body); if (!parsed.success) { - res.status(400).json({ error: parsed.error.flatten() }); + res.status(400).json({ error: "Das neue Passwort muss 8 bis 200 Zeichen lang sein." }); return; } try { @@ -69,20 +88,22 @@ export async function changePassword(req: Request, res: Response) { parsed.data.newPassword ); } catch (error) { - if (error instanceof WrongPasswordError) { + if (error instanceof WrongPasswordError || error instanceof PasswordUnchangedError) { res.status(400).json({ error: error.message }); return; } throw error; } - // A password change invalidates every other session of this account; the - // current one stays so the change doesn't log the user out themselves. - sessionStore.destroyAllForUser(identity.id, sessionIdFromRequest(req)); + // Every session of this account ends - including the current one, which is + // replaced by a fresh id so the user stays signed in on this device only. + sessionStore.destroyAllForUser(identity.id); + const session = sessionStore.create(identity.id); + setSessionCookie(res, authConfig, sessionStore, session.id); res.json({ changed: true }); } export function logout(req: Request, res: Response) { sessionStore.destroy(sessionIdFromRequest(req)); - res.clearCookie(SESSION_COOKIE, { ...cookieOptions(authConfig), maxAge: undefined }); + res.clearCookie(SESSION_COOKIE, { ...cookieOptions(authConfig, 0), maxAge: undefined }); res.json({ signedOut: true }); } diff --git a/src/server/controllers/user.controller.ts b/src/server/controllers/user.controller.ts index 0ad3cbc..bdf8d0f 100644 --- a/src/server/controllers/user.controller.ts +++ b/src/server/controllers/user.controller.ts @@ -1,11 +1,12 @@ import type { Request, Response } from "express"; +import type { ZodError } from "zod"; import { createUserSchema, resetPasswordSchema, updateUserSchema, } from "../../shared/validation/user.schemas.js"; -import { UsernameTakenError } from "../../domain/services/user.service.js"; -import { userService } from "../composition/auth.js"; +import { LastAdminError, UsernameTakenError } from "../../domain/services/user.service.js"; +import { sessionStore, userService } from "../composition/auth.js"; import { userRepository } from "../composition/application-repositories.js"; // The password hash never leaves this layer. @@ -29,6 +30,10 @@ function toDto(user: { }; } +function firstIssue(error: ZodError): string { + return error.issues[0]?.message ?? "Ungültige Eingabe."; +} + export async function listUsers(_req: Request, res: Response) { const rows = await userRepository.list(); res.json(rows.map(toDto)); @@ -37,7 +42,7 @@ export async function listUsers(_req: Request, res: Response) { export async function createUser(req: Request, res: Response) { const parsed = createUserSchema.safeParse(req.body); if (!parsed.success) { - res.status(400).json({ error: parsed.error.flatten() }); + res.status(400).json({ error: firstIssue(parsed.error) }); return; } try { @@ -64,7 +69,7 @@ export async function updateUser(req: Request, res: Response) { } const parsed = updateUserSchema.safeParse(req.body); if (!parsed.success) { - res.status(400).json({ error: parsed.error.flatten() }); + res.status(400).json({ error: firstIssue(parsed.error) }); return; } // The signed-in admin can't lock themselves out by deactivating or @@ -78,8 +83,18 @@ export async function updateUser(req: Request, res: Response) { res.status(404).json({ error: "Benutzer nicht gefunden." }); return; } - const updated = await userService.updateRoleAndActive(userId, parsed.data); - res.json(updated ? toDto(updated) : null); + try { + const updated = await userService.updateRoleAndActive(userId, parsed.data); + // Deactivation takes effect immediately, not just at the next login. + if (parsed.data.active === false) sessionStore.destroyAllForUser(userId); + res.json(updated ? toDto(updated) : null); + } catch (error) { + if (error instanceof LastAdminError) { + res.status(400).json({ error: error.message }); + return; + } + throw error; + } } export async function resetUserPassword(req: Request, res: Response) { @@ -90,7 +105,7 @@ export async function resetUserPassword(req: Request, res: Response) { } const parsed = resetPasswordSchema.safeParse(req.body); if (!parsed.success) { - res.status(400).json({ error: parsed.error.flatten() }); + res.status(400).json({ error: firstIssue(parsed.error) }); return; } const existing = await userRepository.findById(userId); @@ -99,5 +114,8 @@ export async function resetUserPassword(req: Request, res: Response) { return; } const updated = await userService.resetPassword(userId, parsed.data.newPassword); + // A reset is usually a response to a suspected compromise: every open + // session of that account ends now. + sessionStore.destroyAllForUser(userId); res.json(updated ? toDto(updated) : null); } diff --git a/src/server/index.ts b/src/server/index.ts index 868de79..9a65ce0 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -7,7 +7,11 @@ import { cableSizingRouter } from "./routes/cable-sizing.routes.js"; import { authRouter } from "./routes/auth.routes.js"; import { userRouter } from "./routes/user.routes.js"; import { errorMiddleware } from "./middleware/error.middleware.js"; -import { createAuthGuard, requirePasswordChanged } from "./auth/auth.middleware.js"; +import { + createAuthGuard, + requirePasswordChanged, + requireSameOriginForWrites, +} from "./auth/auth.middleware.js"; import { authConfig, sessionStore, userService } from "./composition/auth.js"; import { userRepository } from "./composition/application-repositories.js"; import { createLogger, toErrorMeta } from "../shared/logging/logger.js"; @@ -19,6 +23,7 @@ const port = Number(process.env.PORT || 3000); const heartbeatIntervalMs = 5 * 60 * 1000; const shutdownTimeoutMs = 15 * 1000; +app.disable("x-powered-by"); app.use(express.json({ limit: "25mb" })); app.use((req, res, next) => { @@ -59,6 +64,7 @@ app.get("/health", (_req, res) => { // /auth/login, /auth/status and /auth/logout (see createAuthGuard's // openPaths). authRouter is mounted AFTER the guard (not before) so that // /auth/me and /auth/change-password still get req.authUser populated by it. +app.use("/api", requireSameOriginForWrites()); app.use("/api", createAuthGuard(authConfig, sessionStore, userRepository)); app.use("/api", requirePasswordChanged()); diff --git a/src/shared/validation/auth.schemas.ts b/src/shared/validation/auth.schemas.ts index 544b052..7d95eec 100644 --- a/src/shared/validation/auth.schemas.ts +++ b/src/shared/validation/auth.schemas.ts @@ -1,15 +1,20 @@ import { z } from "zod"; +// Upper bounds keep a huge request body from being fed into scrypt. +export const MAX_USERNAME_LENGTH = 100; +export const MAX_PASSWORD_LENGTH = 200; +export const MIN_PASSWORD_LENGTH = 8; + export const loginSchema = z .object({ - username: z.string().trim().min(1), - password: z.string().min(1), + username: z.string().trim().min(1).max(MAX_USERNAME_LENGTH), + password: z.string().min(1).max(MAX_PASSWORD_LENGTH), }) .strict(); export const changePasswordSchema = z .object({ - currentPassword: z.string().min(1), - newPassword: z.string().min(8).max(200), + currentPassword: z.string().min(1).max(MAX_PASSWORD_LENGTH), + newPassword: z.string().min(MIN_PASSWORD_LENGTH).max(MAX_PASSWORD_LENGTH), }) .strict(); diff --git a/src/shared/validation/user.schemas.ts b/src/shared/validation/user.schemas.ts index ceb8961..cec4c5f 100644 --- a/src/shared/validation/user.schemas.ts +++ b/src/shared/validation/user.schemas.ts @@ -1,14 +1,29 @@ import { z } from "zod"; import { USER_ROLES } from "../constants/user-roles.js"; +import { + MAX_PASSWORD_LENGTH, + MAX_USERNAME_LENGTH, + MIN_PASSWORD_LENGTH, +} from "./auth.schemas.js"; -const MIN_PASSWORD_LENGTH = 8; +// ASCII only: rules out look-alike Unicode names ("admіn" with a Cyrillic i) +// and invisible whitespace. E-mail addresses fit this pattern. +const USERNAME_PATTERN = /^[a-z0-9._@-]+$/; export const createUserSchema = z .object({ - username: z.string().trim().min(1).max(200), + username: z + .string() + .trim() + .toLowerCase() + .min(1) + .max(MAX_USERNAME_LENGTH) + .regex(USERNAME_PATTERN, { + message: "Anmeldename: nur a-z, 0-9 sowie . _ @ - erlaubt.", + }), name: z.string().trim().min(1).max(200), role: z.enum(USER_ROLES), - initialPassword: z.string().min(MIN_PASSWORD_LENGTH).max(200), + initialPassword: z.string().min(MIN_PASSWORD_LENGTH).max(MAX_PASSWORD_LENGTH), }) .strict(); @@ -24,7 +39,7 @@ export const updateUserSchema = z export const resetPasswordSchema = z .object({ - newPassword: z.string().min(MIN_PASSWORD_LENGTH).max(200), + newPassword: z.string().min(MIN_PASSWORD_LENGTH).max(MAX_PASSWORD_LENGTH), }) .strict(); diff --git a/tests/authentication.test.ts b/tests/authentication.test.ts new file mode 100644 index 0000000..e634716 --- /dev/null +++ b/tests/authentication.test.ts @@ -0,0 +1,265 @@ +import path from "node:path"; +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import type { Request, Response } from "express"; +import { migrate } from "drizzle-orm/better-sqlite3/migrator"; +import { createDatabaseContext } from "../src/db/database-context.js"; +import { UserRepository } from "../src/db/repositories/user.repository.js"; +import { + InvalidCredentialsError, + LastAdminError, + PasswordUnchangedError, + UserService, + UsernameTakenError, + WrongPasswordError, +} from "../src/domain/services/user.service.js"; +import { hashPassword, verifyPassword } from "../src/server/auth/password.js"; +import { SessionStore } from "../src/server/auth/sessions.js"; +import { LoginThrottle } from "../src/server/auth/login-throttle.js"; +import { readCookie } from "../src/server/auth/cookie.js"; +import { requireSameOriginForWrites } from "../src/server/auth/auth.middleware.js"; +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"; + +const HOUR = 60 * 60 * 1000; + +function createUserService() { + const context = createDatabaseContext(":memory:"); + migrate(context.db, { migrationsFolder: path.resolve("src", "db", "migrations") }); + const repository = new UserRepository(context.db); + const config: AuthConfig = { + sessionTtlMs: 12 * HOUR, + sessionMaxAgeMs: 7 * 24 * HOUR, + cookieSecure: false, + bootstrap: { username: "admin", password: "bootstrap-password-123" }, + }; + return { repository, service: new UserService(repository, config) }; +} + +describe("password hashing", () => { + it("verifies the right password and rejects a wrong one", async () => { + const hash = await hashPassword("richtiges-passwort"); + assert.match(hash, /^scrypt\$16384\$8\$1\$[0-9a-f]{32}\$[0-9a-f]{128}$/); + assert.equal(await verifyPassword("richtiges-passwort", hash), true); + assert.equal(await verifyPassword("falsches-passwort", hash), false); + }); + + it("uses a fresh salt per hash", async () => { + assert.notEqual(await hashPassword("gleich"), await hashPassword("gleich")); + }); + + 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); + }); +}); + +describe("session store", () => { + it("expires idle sessions and slides on activity", () => { + let now = 0; + const store = new SessionStore(1 * HOUR, 24 * HOUR, () => now); + const session = store.create("user-1"); + now = 50 * 60 * 1000; + assert.ok(store.get(session.id)); + now += 50 * 60 * 1000; + assert.ok(store.get(session.id), "activity must have extended the idle window"); + now += 2 * HOUR; + assert.equal(store.get(session.id), undefined); + }); + + it("never outlives the absolute lifetime, even when constantly used", () => { + let now = 0; + const store = new SessionStore(1 * HOUR, 3 * HOUR, () => now); + const session = store.create("user-1"); + for (let step = 0; step < 5; step += 1) { + now += 30 * 60 * 1000; + store.get(session.id); + } + now = 3 * HOUR; + assert.equal(store.get(session.id), undefined); + }); + + it("ends all sessions of a user except an optional one", () => { + const store = new SessionStore(HOUR, 24 * HOUR); + const first = store.create("user-1"); + const second = store.create("user-1"); + const other = store.create("user-2"); + store.destroyAllForUser("user-1", second.id); + assert.equal(store.get(first.id), undefined); + assert.ok(store.get(second.id)); + assert.ok(store.get(other.id)); + }); + + it("bounds the number of sessions per user by dropping the oldest", () => { + let now = 0; + const store = new SessionStore(HOUR, 24 * HOUR, () => now); + const oldest = store.create("user-1"); + for (let index = 0; index < 20; index += 1) { + now += 1; + store.create("user-1"); + } + assert.equal(store.get(oldest.id), undefined); + }); +}); + +describe("login throttle", () => { + it("blocks a username after repeated failures and releases it after the window", () => { + let now = 0; + const throttle = new LoginThrottle(3, 15 * 60 * 1000, 100, () => now); + for (let attempt = 0; attempt < 3; attempt += 1) { + assert.equal(throttle.retryAfterMs("Admin"), 0); + throttle.recordFailure("admin "); + } + assert.ok(throttle.retryAfterMs("ADMIN") > 0, "normalized usernames share a counter"); + assert.equal(throttle.retryAfterMs("someone-else"), 0); + now = 15 * 60 * 1000 + 1; + assert.equal(throttle.retryAfterMs("admin"), 0); + }); + + it("resets the counter after a successful login", () => { + const throttle = new LoginThrottle(2); + throttle.recordFailure("admin"); + throttle.recordSuccess("admin"); + throttle.recordFailure("admin"); + assert.equal(throttle.retryAfterMs("admin"), 0); + }); +}); + +describe("cookie parsing", () => { + it("reads the session cookie and ignores malformed encodings", () => { + assert.equal(readCookie("a=1; lb_session=abc; b=2", "lb_session"), "abc"); + assert.equal(readCookie("lb_session=%E0%A4%A", "lb_session"), undefined); + assert.equal(readCookie(undefined, "lb_session"), undefined); + }); +}); + +describe("same-origin guard for writes", () => { + function run(method: string, fetchSite?: string) { + let status: number | undefined; + let passed = false; + const req = { + method, + headers: fetchSite ? { "sec-fetch-site": fetchSite } : {}, + } as unknown as Request; + const res = { + status(code: number) { + status = code; + return this; + }, + json() { + return this; + }, + } as unknown as Response; + requireSameOriginForWrites()(req, res, () => { + passed = true; + }); + return { passed, status }; + } + + it("lets reads and same-origin writes through", () => { + assert.equal(run("GET", "cross-site").passed, true); + assert.equal(run("POST", "same-origin").passed, true); + assert.equal(run("POST").passed, true, "non-browser clients carry no ambient cookie"); + }); + + it("rejects writes from other sites and other ports of the same host", () => { + 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 }); + }); +}); + +describe("input validation", () => { + it("rejects look-alike and whitespace usernames and normalizes case", () => { + const base = { name: "Test", role: "user", initialPassword: "start-passwort" }; + assert.equal(createUserSchema.safeParse({ ...base, username: "admіn" }).success, false); + assert.equal(createUserSchema.safeParse({ ...base, username: "ad min" }).success, false); + const parsed = createUserSchema.parse({ ...base, username: " Max.Muster@Firma.de " }); + assert.equal(parsed.username, "max.muster@firma.de"); + }); + + it("caps login input length so huge bodies never reach scrypt", () => { + assert.equal( + loginSchema.safeParse({ username: "admin", password: "x".repeat(201) }).success, + false + ); + }); +}); + +describe("user service", () => { + it("creates the bootstrap admin only while the table is empty", async () => { + const { repository, service } = createUserService(); + await service.ensureBootstrapAdmin(); + await service.ensureBootstrapAdmin(); + const users = await repository.list(); + assert.equal(users.length, 1); + assert.equal(users[0].role, "admin"); + assert.equal(users[0].mustChangePassword, true); + }); + + it("rejects unknown users, wrong passwords and deactivated accounts alike", async () => { + const { service } = createUserService(); + const user = await service.createUser({ + username: "max", + name: "Max", + role: "user", + initialPassword: "start-passwort", + }); + assert.ok(user); + await assert.rejects(service.authenticate("niemand", "egal"), InvalidCredentialsError); + await assert.rejects(service.authenticate("max", "falsch"), InvalidCredentialsError); + assert.equal((await service.authenticate("MAX", "start-passwort")).id, user.id); + await service.updateRoleAndActive(user.id, { active: false }); + await assert.rejects(service.authenticate("max", "start-passwort"), InvalidCredentialsError); + }); + + it("refuses duplicate usernames", async () => { + const { service } = createUserService(); + const input = { username: "max", name: "Max", role: "user" as const, initialPassword: "start-passwort" }; + await service.createUser(input); + await assert.rejects(service.createUser(input), UsernameTakenError); + }); + + it("never removes the last active admin", async () => { + const { repository, service } = createUserService(); + await service.ensureBootstrapAdmin(); + const [admin] = await repository.list(); + await assert.rejects(service.updateRoleAndActive(admin.id, { role: "user" }), LastAdminError); + await assert.rejects(service.updateRoleAndActive(admin.id, { active: false }), LastAdminError); + + const second = await service.createUser({ + username: "zweiter", + name: "Zweiter", + role: "admin", + initialPassword: "start-passwort", + }); + assert.ok(second); + await service.updateRoleAndActive(admin.id, { role: "user" }); + await assert.rejects(service.updateRoleAndActive(second.id, { active: false }), LastAdminError); + }); + + it("requires the current password and a different new one", async () => { + const { service } = createUserService(); + const user = await service.createUser({ + username: "max", + name: "Max", + role: "user", + initialPassword: "start-passwort", + }); + assert.ok(user); + await assert.rejects( + service.changeOwnPassword(user.id, "falsch", "neues-passwort"), + WrongPasswordError + ); + await assert.rejects( + service.changeOwnPassword(user.id, "start-passwort", "start-passwort"), + PasswordUnchangedError + ); + const changed = await service.changeOwnPassword(user.id, "start-passwort", "neues-passwort"); + assert.equal(changed?.mustChangePassword, false); + + const reset = await service.resetPassword(user.id, "admin-vergeben"); + assert.equal(reset?.mustChangePassword, true); + }); +});