From 6bf8218fea72b53ca04da34821082ad4caaa67d2 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 16 Aug 2026 21:03:07 +0200 Subject: [PATCH] fix(auth): harden persistent sessions --- backend/src/auth/session-store.ts | 235 +++++++++++-- backend/test/auth-session-store.test.ts | 323 +++++++++++++++++- backend/test/fixtures/oidc-state-consumer.mts | 19 ++ 3 files changed, 537 insertions(+), 40 deletions(-) create mode 100644 backend/test/fixtures/oidc-state-consumer.mts diff --git a/backend/src/auth/session-store.ts b/backend/src/auth/session-store.ts index 0d9c706e..76339052 100644 --- a/backend/src/auth/session-store.ts +++ b/backend/src/auth/session-store.ts @@ -7,6 +7,7 @@ import { fstatSync, fsyncSync, lstatSync, + linkSync, mkdirSync, openSync, readSync, @@ -25,6 +26,7 @@ import type { AuthSessionRecord, OidcStateRecord, Permission, Role } from "./typ const TOKEN_BYTES = 32; const TOKEN_PATTERN = /^[A-Za-z0-9_-]{43}$/; const DIGEST_FILENAME_PATTERN = /^[a-f0-9]{64}\.json$/; +const CLAIM_FILENAME_PATTERN = /^[a-f0-9]{64}\.claim$/; const PRIVATE_DIRECTORY_MODE = 0o700; const PRIVATE_FILE_MODE = 0o600; const MAX_SESSION_RECORD_BYTES = 16 * 1024; @@ -97,6 +99,7 @@ export interface AuthSessionStore { interface FileIdentity { dev: number; ino: number; + uid: number; size: number; mtimeMs: number; } @@ -104,6 +107,7 @@ interface FileIdentity { interface DirectoryIdentity { dev: number; ino: number; + uid: number; mode?: number; } @@ -203,12 +207,12 @@ const oidcStateInputSchema = z.strictObject({ }); function sameFileIdentity(left: FileIdentity, right: FileIdentity): boolean { - return left.dev === right.dev && left.ino === right.ino && left.size === right.size + return left.dev === right.dev && left.ino === right.ino && left.uid === right.uid && left.size === right.size && left.mtimeMs === right.mtimeMs; } function sameDirectoryIdentity(left: DirectoryIdentity, right: DirectoryIdentity): boolean { - return left.dev === right.dev && left.ino === right.ino && left.mode === right.mode; + return left.dev === right.dev && left.ino === right.ino && left.uid === right.uid && left.mode === right.mode; } function isNotFound(error: unknown): boolean { @@ -229,8 +233,13 @@ function digestFilename(rawValue: string): string { return `${createHash("sha256").update(rawValue).digest("hex")}.json`; } -function assertFilename(filename: string): void { +function claimFilename(filename: string): string { if (!DIGEST_FILENAME_PATTERN.test(filename)) throw invalid(); + return filename.slice(0, -".json".length) + ".claim"; +} + +function assertFilename(filename: string): void { + if (!DIGEST_FILENAME_PATTERN.test(filename) && !CLAIM_FILENAME_PATTERN.test(filename)) throw invalid(); } function filePath(directory: string, filename: string): string { @@ -240,22 +249,31 @@ function filePath(directory: string, filename: string): string { return path; } -function fileIdentity(info: Stats): FileIdentity { - if (!info.isFile() || info.isSymbolicLink() || info.nlink !== 1 - || (info.mode & 0o7777) !== PRIVATE_FILE_MODE || info.size < 0) { +function ownerId(): number { + const getEffectiveUserId = process.geteuid; + if (typeof getEffectiveUserId !== "function") throw invalid(); + const euid = getEffectiveUserId(); + if (!Number.isSafeInteger(euid) || euid < 0) throw invalid(); + return euid; +} + +function fileIdentity(info: Stats, expectedLinks = 1): FileIdentity { + if (!info.isFile() || info.isSymbolicLink() || info.nlink !== expectedLinks + || info.uid !== ownerId() || (info.mode & 0o7777) !== PRIVATE_FILE_MODE || info.size < 0) { throw invalid(); } - return { dev: info.dev, ino: info.ino, size: info.size, mtimeMs: info.mtimeMs }; + return { dev: info.dev, ino: info.ino, uid: info.uid, size: info.size, mtimeMs: info.mtimeMs }; } function directoryIdentity(path: string): DirectoryIdentity { const info = lstatSync(path) as Stats; if (!info.isDirectory() || info.isSymbolicLink() || realpathSync(path) !== path) throw invalid(); - if (process.platform !== "win32" && (info.mode & 0o7777) !== PRIVATE_DIRECTORY_MODE) throw invalid(); + if (info.uid !== ownerId() || (info.mode & 0o7777) !== PRIVATE_DIRECTORY_MODE) throw invalid(); return { dev: info.dev, ino: info.ino, - ...(process.platform === "win32" ? {} : { mode: info.mode & 0o7777 }), + uid: info.uid, + mode: info.mode & 0o7777, }; } @@ -282,6 +300,11 @@ function privateDirectory(path: string): void { } function storageDirectories(root: string): StorageDirectories { + // Node's chmod is not a Windows DACL boundary. The existing Go operator store has a + // CreateFile security-descriptor path, but no equivalent safe Node primitive is available. + // Refuse before probing or creating the configured root rather than publishing browser state + // with inherited ACLs. + if (process.platform === "win32") throw invalid(); if (typeof root !== "string" || root.length === 0 || root.includes("\0") || !isAbsolute(root) || normalize(root) !== root) throw invalid(); privateDirectory(root); @@ -328,6 +351,7 @@ function readTrusted( filename: string, maximumBytes: number, parse: (source: string) => T, + expectedLinks = 1, ): TrustedFile | undefined { const path = filePath(directory, filename); let directoryDescriptor: number | undefined; @@ -335,7 +359,7 @@ function readTrusted( try { const beforeDirectory = directoryIdentity(directory); const beforePath = lstatSync(path) as Stats; - const before = fileIdentity(beforePath); + const before = fileIdentity(beforePath, expectedLinks); if (before.size > maximumBytes) throw invalid(); directoryDescriptor = openDirectory(directory); @@ -344,7 +368,7 @@ function readTrusted( : directoryIdentityFromDescriptor(directoryDescriptor); if (!sameDirectoryIdentity(beforeDirectory, openedDirectory)) throw invalid(); descriptor = openSync(path, constants.O_RDONLY | (constants.O_NOFOLLOW ?? 0) | (constants.O_NONBLOCK ?? 0)); - const opened = fileIdentity(fstatSync(descriptor) as Stats); + const opened = fileIdentity(fstatSync(descriptor) as Stats, expectedLinks); if (!sameFileIdentity(before, opened) || opened.size > maximumBytes) throw invalid(); const contents = Buffer.allocUnsafe(maximumBytes + 1); @@ -356,8 +380,8 @@ function readTrusted( } if (offset > maximumBytes) throw invalid(); - const after = fileIdentity(fstatSync(descriptor) as Stats); - const afterPath = fileIdentity(lstatSync(path) as Stats); + const after = fileIdentity(fstatSync(descriptor) as Stats, expectedLinks); + const afterPath = fileIdentity(lstatSync(path) as Stats, expectedLinks); const afterDirectory = directoryIdentity(directory); const afterOpenedDirectory = directoryDescriptor === undefined ? afterDirectory @@ -384,11 +408,12 @@ function readTrusted( function directoryIdentityFromDescriptor(descriptor: number): DirectoryIdentity { const info = fstatSync(descriptor) as Stats; if (!info.isDirectory() || info.isSymbolicLink()) throw invalid(); - if (process.platform !== "win32" && (info.mode & 0o7777) !== PRIVATE_DIRECTORY_MODE) throw invalid(); + if (info.uid !== ownerId() || (info.mode & 0o7777) !== PRIVATE_DIRECTORY_MODE) throw invalid(); return { dev: info.dev, ino: info.ino, - ...(process.platform === "win32" ? {} : { mode: info.mode & 0o7777 }), + uid: info.uid, + mode: info.mode & 0o7777, }; } @@ -459,11 +484,23 @@ function replaceTrusted( } } -function removeTrusted(directory: string, filename: string, expected?: FileIdentity): boolean { +function removeTrusted( + directory: string, + filename: string, + expected?: FileIdentity, + expectedLinks = 1, +): boolean { const path = filePath(directory, filename); try { - const current = fileIdentity(lstatSync(path) as Stats); + const beforeDirectory = directoryIdentity(directory); + const current = fileIdentity(lstatSync(path) as Stats, expectedLinks); if (expected && !sameFileIdentity(expected, current)) throw invalid(); + // Revalidate both names immediately before unlink. On POSIX unlink never follows a final + // symlink, and this closes the observable replacement window before that operation. + const finalDirectory = directoryIdentity(directory); + const final = fileIdentity(lstatSync(path) as Stats, expectedLinks); + if (!sameDirectoryIdentity(beforeDirectory, finalDirectory) || !sameFileIdentity(current, final) + || (expected !== undefined && !sameFileIdentity(expected, final))) throw invalid(); unlinkSync(path); syncDirectory(directory); return true; @@ -489,6 +526,99 @@ function parseOidcStateRecord(source: string): OidcStateRecord { } } +interface OidcStateClaim { + state: TrustedFile; + claimIdentity: FileIdentity; +} + +function inspectOidcStateClaim( + directory: string, + filename: string, +): OidcStateClaim | "orphan" | undefined { + const claimedFilename = claimFilename(filename); + let claimInfo: Stats; + try { + claimInfo = lstatSync(filePath(directory, claimedFilename)) as Stats; + } catch (error) { + if (isNotFound(error)) return undefined; + throw invalid(); + } + let sourceInfo: Stats; + try { + sourceInfo = lstatSync(filePath(directory, filename)) as Stats; + } catch (error) { + if (!isNotFound(error)) throw invalid(); + // A consumer may have just unlinked the source and not yet removed its claim. Do not + // remove that orphan here: doing so could make the winning consumer fail closed after it + // has read the state. prune() removes abandoned orphan claims after the state lifetime. + fileIdentity(claimInfo, 1); + return "orphan"; + } + const sourceIdentity = fileIdentity(sourceInfo, 2); + const claimIdentity = fileIdentity(claimInfo, 2); + if (!sameFileIdentity(sourceIdentity, claimIdentity)) throw invalid(); + const state = readTrusted(directory, filename, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord, 2); + if (!state || !sameFileIdentity(sourceIdentity, state.identity)) throw invalid(); + return { state, claimIdentity }; +} + +function oidcClaimExists(directory: string, filename: string): boolean { + const claimedFilename = claimFilename(filename); + try { + const info = lstatSync(filePath(directory, claimedFilename)) as Stats; + if (info.nlink !== 1 && info.nlink !== 2) throw invalid(); + fileIdentity(info, info.nlink); + return true; + } catch (error) { + if (isNotFound(error)) return false; + throw invalid(); + } +} + +/** + * Claim a state by creating a hard link with a deterministic digest-only name. link(2) / NTFS + * CreateHardLink fails if another process has already installed that name, unlike rename which + * can overwrite the previous claimant. A crash leaves the pair unavailable until expiry/prune. + */ +function claimOidcState(directory: string, filename: string): OidcStateClaim | undefined { + // A competing process may be partway through consumption. It has already won and must be + // the only process allowed to deserialize the record; this process simply treats it as used. + if (oidcClaimExists(directory, filename)) return undefined; + + const statePath = filePath(directory, filename); + const claimedFilename = claimFilename(filename); + const claimedPath = filePath(directory, claimedFilename); + let before: FileIdentity; + try { + before = fileIdentity(lstatSync(statePath) as Stats); + } catch (error) { + if (isNotFound(error)) return undefined; + throw invalid(); + } + try { + linkSync(statePath, claimedPath); + } catch (error) { + if (isNotFound(error)) return undefined; + if ((error as NodeJS.ErrnoException | undefined)?.code === "EEXIST") return undefined; + throw invalid(); + } + try { + const sourceIdentity = fileIdentity(lstatSync(statePath) as Stats, 2); + const claimIdentity = fileIdentity(lstatSync(claimedPath) as Stats, 2); + if (!sameFileIdentity(before, sourceIdentity) || !sameFileIdentity(sourceIdentity, claimIdentity)) throw invalid(); + const state = readTrusted(directory, filename, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord, 2); + if (!state || !sameFileIdentity(state.identity, sourceIdentity)) throw invalid(); + return { state, claimIdentity }; + } catch { + throw invalid(); + } +} + +function removeClaimedOidcState(directory: string, filename: string, claim: OidcStateClaim): void { + if (!removeTrusted(directory, filename, claim.state.identity, 2)) throw invalid(); + if (!removeTrusted(directory, claimFilename(filename), claim.claimIdentity)) throw invalid(); +} + function serialize(record: AuthSessionRecord | OidcStateRecord, maximumBytes: number): Buffer { const contents = Buffer.from(`${JSON.stringify(record)}\n`, "utf8"); if (contents.length > maximumBytes) throw invalid(); @@ -530,7 +660,9 @@ function validLocalUser(user: LocalSessionUser | undefined, record: AuthSessionR } async function recordIsCurrent(record: AuthSessionRecord, validity: AuthSessionValidity | undefined): Promise { - if (!validity) return true; + // A root-only store remains useful for creation/diagnostics, but is intentionally incapable + // of authenticating a principal. Task 8 must supply config and local-registry dependencies. + if (!validity) return false; const revision = await validity.currentAuthConfigRevision(); if (typeof revision !== "string" || revision !== record.authConfigRevision) return false; if (record.method !== "local") return true; @@ -704,10 +836,16 @@ export function createFileAuthSessionStore(root: string, validity?: AuthSessionV const filename = digestFilename(state); return withLock(lockKey(root, "oidc", filename), async () => { const directories = storageDirectories(root); - const trusted = readTrusted(directories.oidc, filename, MAX_OIDC_STATE_RECORD_BYTES, parseOidcStateRecord); - if (!trusted) return undefined; - removeTrusted(directories.oidc, filename, trusted.identity); - return oidcStateExpired(trusted.value, nowMs) ? undefined : trusted.value; + const claim = claimOidcState(directories.oidc, filename); + // An installed claim belongs to another process/store instance. Only the process which + // created the hard link is allowed to receive the record. + if (!claim) return undefined; + if (oidcStateExpired(claim.state.value, nowMs)) { + removeClaimedOidcState(directories.oidc, filename, claim); + return undefined; + } + removeClaimedOidcState(directories.oidc, filename, claim); + return claim.state.value; }); } @@ -745,13 +883,52 @@ export function createFileAuthSessionStore(root: string, validity?: AuthSessionV parseSessionRecord, (record, timestamp) => sessionExpired(record as AuthSessionRecord, timestamp), ); - await pruneDirectory( - directories.oidc, - "oidc", - MAX_OIDC_STATE_RECORD_BYTES, - parseOidcStateRecord, - (record, timestamp) => oidcStateExpired(record as OidcStateRecord, timestamp), - ); + let oidcEntries: string[]; + try { + oidcEntries = readdirSync(directories.oidc); + } catch { + throw invalid(); + } + const stateFilenames = new Set(oidcEntries.filter((entry) => DIGEST_FILENAME_PATTERN.test(entry))); + for (const filename of stateFilenames) { + await withLock(lockKey(root, "oidc", filename), async () => { + const claim = inspectOidcStateClaim(directories.oidc, filename); + if (claim === "orphan") return; + if (claim) { + if (oidcStateExpired(claim.state.value, nowMs)) { + removeClaimedOidcState(directories.oidc, filename, claim); + removed += 1; + } + return; + } + const trusted = readTrusted( + directories.oidc, + filename, + MAX_OIDC_STATE_RECORD_BYTES, + parseOidcStateRecord, + ); + if (trusted && oidcStateExpired(trusted.value, nowMs) + && removeTrusted(directories.oidc, filename, trusted.identity)) { + removed += 1; + } + }); + } + for (const claimedFilename of oidcEntries) { + if (!CLAIM_FILENAME_PATTERN.test(claimedFilename)) continue; + const filename = `${claimedFilename.slice(0, -".claim".length)}.json`; + if (stateFilenames.has(filename)) continue; + await withLock(lockKey(root, "oidc", filename), async () => { + const claim = inspectOidcStateClaim(directories.oidc, filename); + if (claim !== "orphan") return; + const claimIdentity = fileIdentity(lstatSync(filePath(directories.oidc, claimedFilename)) as Stats); + // A hard link retains the source mtime. It is therefore a conservative, bounded + // expiry marker even if a process crashed between the two unlink operations. + if (nowMs >= claimIdentity.mtimeMs + OIDC_STATE_TTL_MS + && removeTrusted(directories.oidc, claimedFilename, claimIdentity)) { + removed += 1; + } + }); + } return removed; } diff --git a/backend/test/auth-session-store.test.ts b/backend/test/auth-session-store.test.ts index 525c7e34..fa659b56 100644 --- a/backend/test/auth-session-store.test.ts +++ b/backend/test/auth-session-store.test.ts @@ -1,20 +1,58 @@ import { createHash } from "node:crypto"; +import { fork } from "node:child_process"; import { chmodSync, existsSync, linkSync, lstatSync, + mkdirSync, mkdtempSync, readFileSync, realpathSync, renameSync, rmSync, symlinkSync, + unlinkSync, writeFileSync, } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; + +const fsHooks = vi.hoisted(() => ({ + afterRead: undefined as undefined | (() => void), + afterWrite: undefined as undefined | (() => void), + afterLstat: undefined as undefined | ((path: string) => boolean), + transformLstat: undefined as undefined | ((path: string, info: import("node:fs").Stats) => import("node:fs").Stats), +})); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readSync: (...args: Parameters) => { + const result = actual.readSync(...args); + const callback = fsHooks.afterRead; + fsHooks.afterRead = undefined; + callback?.(); + return result; + }, + writeSync: (...args: Parameters) => { + const result = actual.writeSync(...args); + const callback = fsHooks.afterWrite; + fsHooks.afterWrite = undefined; + callback?.(); + return result; + }, + lstatSync: (...args: Parameters) => { + const original = actual.lstatSync(...args); + const result = fsHooks.transformLstat?.(String(args[0]), original) ?? original; + const callback = fsHooks.afterLstat; + if (callback?.(String(args[0]))) fsHooks.afterLstat = undefined; + return result; + }, + }; +}); import { createFileAuthSessionStore, deriveCsrfToken, @@ -25,8 +63,13 @@ import { const roots: string[] = []; const base = new Date("2030-01-02T03:04:05.000Z"); const revision = "a".repeat(64); +const validLocalUser = { enabled: true, authRevision: 7, roles: ["admin"] as const }; afterEach(() => { + fsHooks.afterRead = undefined; + fsHooks.afterWrite = undefined; + fsHooks.afterLstat = undefined; + fsHooks.transformLstat = undefined; for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); @@ -41,6 +84,17 @@ function digestPath(rootPath: string, directory: "sessions" | "oidc", rawValue: return join(rootPath, directory, `${createHash("sha256").update(rawValue).digest("hex")}.json`); } +function claimPath(rootPath: string, rawState: string): string { + return join(rootPath, "oidc", `${createHash("sha256").update(rawState).digest("hex")}.claim`); +} + +function validStore(storageRoot: string): AuthSessionStore { + return createFileAuthSessionStore(storageRoot, { + currentAuthConfigRevision: () => revision, + findLocalUser: async () => validLocalUser, + }); +} + async function create( store: AuthSessionStore, overrides: Partial = {}, @@ -69,10 +123,72 @@ async function expectStoreInvalid(operation: Promise): Promise { await expect(operation).rejects.toThrow("auth_session_store_invalid"); } +async function isolatedOidcConsumer(storageRoot: string, state: string): Promise<{ + start(): void; + result: Promise; +}> { + const child = fork(new URL("./fixtures/oidc-state-consumer.mts", import.meta.url), [], { + cwd: process.cwd(), + execArgv: ["--import", "tsx"], + env: { + ...process.env, + THT_TEST_SESSION_ROOT: storageRoot, + THT_TEST_OIDC_STATE: state, + }, + silent: true, + }); + const ready = new Promise((resolve, reject) => { + child.once("message", (message) => { + if (message === "ready") resolve(); + else reject(new Error("OIDC consumer did not become ready")); + }); + child.once("error", reject); + child.once("exit", (code) => { + if (code !== null && code !== 0) reject(new Error("OIDC consumer exited before ready")); + }); + }); + const result = new Promise((resolve, reject) => { + child.on("message", (message) => { + if (message && typeof message === "object" && "consumed" in message) { + const outcome = message as { consumed: unknown; failed?: unknown }; + if (outcome.failed === true) reject(new Error("OIDC consumer failed")); + else resolve(outcome.consumed === true); + } + }); + child.on("error", reject); + child.on("exit", (code) => { + if (code !== 0) reject(new Error("OIDC consumer exited without a result")); + }); + }); + await ready; + return { start: () => child.send("consume"), result }; +} + describe("file-backed auth session store", () => { - test("creates 256-bit opaque tokens, digest-only files, and derived CSRF values", async () => { + test("fails closed and revokes a session when constructed without validity dependencies", async () => { const storageRoot = root(); const store = createFileAuthSessionStore(storageRoot); + const created = await create(store); + + await expect(store.resolve(created.token)).resolves.toBeUndefined(); + expect(existsSync(digestPath(storageRoot, "sessions", created.token))).toBe(false); + }); + + test("revokes a session when a validity dependency throws", async () => { + const storageRoot = root(); + const store = createFileAuthSessionStore(storageRoot, { + currentAuthConfigRevision: () => { throw new Error("dependency unavailable"); }, + findLocalUser: async () => validLocalUser, + }); + const created = await create(store); + + await expectStoreInvalid(store.resolve(created.token)); + expect(existsSync(digestPath(storageRoot, "sessions", created.token))).toBe(false); + }); + + test("creates 256-bit opaque tokens, digest-only files, and derived CSRF values", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); const first = await create(store); const second = await create(store); const path = digestPath(storageRoot, "sessions", first.token); @@ -99,9 +215,9 @@ describe("file-backed auth session store", () => { test("survives a backend restart and respects idle and absolute expiry", async () => { const storageRoot = root(); - const firstStore = createFileAuthSessionStore(storageRoot); + const firstStore = validStore(storageRoot); const created = await create(firstStore); - const restartedStore = createFileAuthSessionStore(storageRoot); + const restartedStore = validStore(storageRoot); await expect(restartedStore.resolve(created.token, new Date(base.getTime() + 9 * 60_000))) .resolves.toMatchObject({ subject: created.record.subject, remembered: true }); @@ -116,7 +232,7 @@ describe("file-backed auth session store", () => { test("touches at most once per five minutes and never extends absolute expiry", async () => { const storageRoot = root(); - const store = createFileAuthSessionStore(storageRoot); + const store = validStore(storageRoot); const created = await create(store); const path = digestPath(storageRoot, "sessions", created.token); const before = readFileSync(path, "utf8"); @@ -134,7 +250,7 @@ describe("file-backed auth session store", () => { test("revokes sessions and prunes expired session and OIDC-state records", async () => { const storageRoot = root(); - const store = createFileAuthSessionStore(storageRoot); + const store = validStore(storageRoot); const revoked = await create(store); const expired = await create(store, { idleTtlMs: 60_000, absoluteTtlMs: 60_000 }); const oidc = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base); @@ -150,7 +266,7 @@ describe("file-backed auth session store", () => { test("creates bounded OIDC state records that expire and are single-use", async () => { const storageRoot = root(); - const store = createFileAuthSessionStore(storageRoot); + const store = validStore(storageRoot); const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), @@ -169,9 +285,64 @@ describe("file-backed auth session store", () => { .resolves.toBeUndefined(); }); + test("fails closed when an OIDC state already has an atomic filesystem claim", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base); + const statePath = digestPath(storageRoot, "oidc", created.state); + linkSync(statePath, claimPath(storageRoot, created.state)); + + await expect(store.consumeOidcState(created.state)).resolves.toBeUndefined(); + expect(existsSync(statePath)).toBe(true); + }); + + test("prunes an expired OIDC state abandoned after an atomic claim", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base); + const statePath = digestPath(storageRoot, "oidc", created.state); + const stateClaimPath = claimPath(storageRoot, created.state); + linkSync(statePath, stateClaimPath); + + await expect(store.prune(new Date(base.getTime() + 10 * 60_000))).resolves.toBe(1); + expect(existsSync(statePath)).toBe(false); + expect(existsSync(stateClaimPath)).toBe(false); + }); + + test("retains an in-flight orphan claim but removes it after the bounded recovery window", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base); + const statePath = digestPath(storageRoot, "oidc", created.state); + const stateClaimPath = claimPath(storageRoot, created.state); + linkSync(statePath, stateClaimPath); + unlinkSync(statePath); + + await expect(store.consumeOidcState(created.state)).resolves.toBeUndefined(); + expect(existsSync(stateClaimPath)).toBe(true); + await expect(store.prune(new Date("2031-01-02T03:04:05.000Z"))).resolves.toBe(1); + expect(existsSync(stateClaimPath)).toBe(false); + }); + + test("allows exactly one separate Node isolate to consume an OIDC state", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await store.createOidcState({ nonce: "n".repeat(43), codeVerifier: "v".repeat(43), returnTo: "/" }, base); + const [first, second] = await Promise.all([ + isolatedOidcConsumer(storageRoot, created.state), + isolatedOidcConsumer(storageRoot, created.state), + ]); + + first.start(); + second.start(); + const outcomes = await Promise.all([first.result, second.result]); + expect(outcomes.filter(Boolean)).toHaveLength(1); + expect(existsSync(digestPath(storageRoot, "oidc", created.state))).toBe(false); + }); + test.skipIf(process.platform === "win32")("refuses symlinked and hard-linked session records", async () => { const storageRoot = root(); - const store = createFileAuthSessionStore(storageRoot); + const store = validStore(storageRoot); const symlinked = await create(store); const symlinkPath = digestPath(storageRoot, "sessions", symlinked.token); const target = `${symlinkPath}.target`; @@ -187,7 +358,7 @@ describe("file-backed auth session store", () => { test("refuses malformed and oversized session records without disclosing their contents", async () => { const storageRoot = root(); - const store = createFileAuthSessionStore(storageRoot); + const store = validStore(storageRoot); const malformed = await create(store); const malformedPath = digestPath(storageRoot, "sessions", malformed.token); writeFileSync(malformedPath, "{}", { encoding: "utf8", mode: 0o600 }); @@ -201,6 +372,136 @@ describe("file-backed auth session store", () => { await expectStoreInvalid(store.resolve(oversized.token)); }); + test.skipIf(process.platform === "win32")("refuses unsafe existing roots and storage subdirectories", async () => { + const unsafeRoot = root(); + chmodSync(unsafeRoot, 0o755); + await expectStoreInvalid(create(validStore(unsafeRoot))); + + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + chmodSync(join(storageRoot, "sessions"), 0o755); + await expectStoreInvalid(store.resolve(created.token)); + + const outer = root(); + const realRoot = join(outer, "real-auth"); + mkdirSync(realRoot, { mode: 0o700 }); + chmodSync(realRoot, 0o700); + const linkedRoot = join(outer, "linked-auth"); + symlinkSync(realRoot, linkedRoot); + await expectStoreInvalid(create(validStore(linkedRoot))); + }); + + test.skipIf(process.platform === "win32")("refuses storage owned by a different identity", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const sessions = join(storageRoot, "sessions"); + fsHooks.transformLstat = (observed, info) => { + if (observed !== sessions) return info; + const foreign = Object.create(info) as import("node:fs").Stats; + Object.defineProperty(foreign, "uid", { value: info.uid + 1 }); + return foreign; + }; + + await expectStoreInvalid(store.resolve(created.token)); + }); + + test.skipIf(process.platform === "win32")("refuses directory replacement during a session read", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const sessions = join(storageRoot, "sessions"); + const replacement = join(storageRoot, "sessions-replacement"); + fsHooks.afterRead = () => { + renameSync(sessions, replacement); + symlinkSync(replacement, sessions); + }; + + await expectStoreInvalid(store.resolve(created.token)); + }); + + test("refuses file replacement during a touch", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const path = digestPath(storageRoot, "sessions", created.token); + const replacement = `${path}.replacement`; + fsHooks.afterWrite = () => { + writeFileSync(replacement, "{}", { encoding: "utf8", mode: 0o600 }); + chmodSync(replacement, 0o600); + renameSync(replacement, path); + }; + + await expectStoreInvalid(store.touch(created.token, new Date(base.getTime() + 5 * 60_000))); + }); + + test.skipIf(process.platform === "win32")("refuses directory replacement during a touch", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const path = digestPath(storageRoot, "sessions", created.token); + const sessions = join(storageRoot, "sessions"); + const replacement = join(storageRoot, "sessions-replacement"); + fsHooks.afterLstat = (observed) => { + if (observed !== path) return false; + renameSync(sessions, replacement); + symlinkSync(replacement, sessions); + return true; + }; + + await expectStoreInvalid(store.touch(created.token, new Date(base.getTime() + 5 * 60_000))); + }); + + test("refuses file replacement during revoke", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const path = digestPath(storageRoot, "sessions", created.token); + const replacement = `${path}.replacement`; + writeFileSync(replacement, "{}", { encoding: "utf8", mode: 0o600 }); + chmodSync(replacement, 0o600); + fsHooks.afterLstat = (observed) => { + if (observed !== path) return false; + renameSync(replacement, path); + return true; + }; + + await expectStoreInvalid(store.revoke(created.token)); + expect(readFileSync(path, "utf8")).toBe("{}"); + }); + + test.skipIf(process.platform === "win32")("refuses directory replacement during revoke", async () => { + const storageRoot = root(); + const store = validStore(storageRoot); + const created = await create(store); + const path = digestPath(storageRoot, "sessions", created.token); + const sessions = join(storageRoot, "sessions"); + const replacement = join(storageRoot, "sessions-replacement"); + fsHooks.afterLstat = (observed) => { + if (observed !== path) return false; + renameSync(sessions, replacement); + symlinkSync(replacement, sessions); + return true; + }; + + await expectStoreInvalid(store.revoke(created.token)); + }); + + test("rejects native Windows storage before any state write", async () => { + const storageRoot = root(); + const store = validStore(join(storageRoot, "windows-auth-state")); + const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + if (!originalPlatform) throw new Error("platform descriptor unavailable"); + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + try { + await expectStoreInvalid(create(store)); + expect(existsSync(join(storageRoot, "windows-auth-state"))).toBe(false); + } finally { + Object.defineProperty(process, "platform", originalPlatform); + } + }); + test("revokes on config, local-user, revision, enabled, or role mismatch before returning", async () => { const storageRoot = root(); let currentRevision = revision; @@ -235,8 +536,8 @@ describe("file-backed auth session store", () => { test("serializes concurrent resolve and revoke without resurrecting a record", async () => { const storageRoot = root(); - const firstStore = createFileAuthSessionStore(storageRoot); - const secondStore = createFileAuthSessionStore(storageRoot); + const firstStore = validStore(storageRoot); + const secondStore = validStore(storageRoot); const created = await create(firstStore); await Promise.all([ diff --git a/backend/test/fixtures/oidc-state-consumer.mts b/backend/test/fixtures/oidc-state-consumer.mts new file mode 100644 index 00000000..a196a0c8 --- /dev/null +++ b/backend/test/fixtures/oidc-state-consumer.mts @@ -0,0 +1,19 @@ +import { createFileAuthSessionStore } from "../../src/auth/session-store.js"; + +const root = process.env.THT_TEST_SESSION_ROOT; +const state = process.env.THT_TEST_OIDC_STATE; + +if (!root || !state || !process.send) process.exit(2); + +process.send("ready"); +process.once("message", async (message) => { + if (message !== "consume") process.exit(3); + try { + const record = await createFileAuthSessionStore(root).consumeOidcState(state); + process.send?.({ consumed: record !== undefined }); + process.exit(0); + } catch { + process.send?.({ consumed: false, failed: true }); + process.exit(1); + } +});