From e54ce15426ab21133b08f2045d266665655b7fc3 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 16 Aug 2026 17:35:40 +0200 Subject: [PATCH] fix(auth): fail closed configuration compatibility --- backend/src/app.ts | 3 ++ backend/src/auth/auth.ts | 3 +- backend/src/auth/config.ts | 33 +++++++++++++++------- backend/src/config.ts | 5 ++++ backend/test/app-auth-mode.test.ts | 27 ++++++++++++++++++ backend/test/auth-config.test.ts | 45 +++++++++++++++++++++++++++++- backend/test/config.test.ts | 17 +++++++++++ 7 files changed, 120 insertions(+), 13 deletions(-) create mode 100644 backend/test/app-auth-mode.test.ts diff --git a/backend/src/app.ts b/backend/src/app.ts index 628632c3..1b92762f 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -137,6 +137,9 @@ export function buildApp(config: AppConfig, deps?: BuildAppDeps): FastifyInstanc const piManagement = deps?.piManagement ?? createPiManagement(config, { listModels }); const maintenanceBarrier = deps?.maintenanceBarrier ?? new MaintenanceBarrier(config.maintenanceFile); + if (config.authMode === "local" || config.authMode === "oidc") { + throw new Error("configured authentication mode is not implemented"); + } const authenticate = authPreHandler(config.authMode); app.addHook("preHandler", async (req, reply) => { // Process readiness is intentionally unauthenticated for local container/proxy probes. diff --git a/backend/src/auth/auth.ts b/backend/src/auth/auth.ts index b8dbcfc3..8379e8ba 100644 --- a/backend/src/auth/auth.ts +++ b/backend/src/auth/auth.ts @@ -1,12 +1,11 @@ import type { FastifyRequest, FastifyReply } from "fastify"; import { localPrincipal, type PrincipalContext, upstreamPrincipal } from "./principal.js"; -import type { AuthMode } from "./types.js"; declare module "fastify" { interface FastifyRequest { principal?: PrincipalContext } } -export function authPreHandler(mode: AuthMode) { +export function authPreHandler(mode: "none" | "mock" | "upstream") { return async (req: FastifyRequest, reply: FastifyReply) => { if (mode === "none") { req.principal = localPrincipal(); diff --git a/backend/src/auth/config.ts b/backend/src/auth/config.ts index 82e51796..32acc4c3 100644 --- a/backend/src/auth/config.ts +++ b/backend/src/auth/config.ts @@ -59,7 +59,9 @@ const oidcSchema = z.strictObject({ authorization: z.strictObject({ groupRoles: groupRolesSchema }), }); -function readBoundedConfig(path: string): string { +interface FileIdentity { dev: number; ino: number; size: number; mtimeMs: number } + +function readBoundedConfig(path: string): { source: string; identity: FileIdentity } { let fd: number | undefined; try { fd = openSync(path, constants.O_RDONLY | constants.O_NOFOLLOW); @@ -68,7 +70,10 @@ function readBoundedConfig(path: string): string { const buffer = Buffer.allocUnsafe(MAX_AUTH_CONFIG_BYTES + 1); const bytesRead = readSync(fd, buffer, 0, buffer.length, 0); if (bytesRead > MAX_AUTH_CONFIG_BYTES) throw invalid(); - return new TextDecoder("utf-8", { fatal: true }).decode(buffer.subarray(0, bytesRead)); + return { + source: new TextDecoder("utf-8", { fatal: true }).decode(buffer.subarray(0, bytesRead)), + identity: { dev: info.dev, ino: info.ino, size: info.size, mtimeMs: info.mtimeMs }, + }; } catch { throw invalid(); } finally { @@ -105,7 +110,7 @@ function canonicalize(value: unknown): unknown { if (Array.isArray(value)) return value.map(canonicalize); if (value && typeof value === "object") { return Object.fromEntries(Object.entries(value as Record) - .sort(([left], [right]) => left.localeCompare(right)) + .sort(([left], [right]) => left < right ? -1 : left > right ? 1 : 0) .map(([key, nested]) => [key, canonicalize(nested)])); } return value; @@ -139,13 +144,16 @@ function parseAuthenticationConfig(source: string): AuthenticationConfig { } catch { throw invalid(); } } -export function loadAuthenticationConfig(path: string): LoadedAuthConfig { +function loadAuthenticationConfigWithIdentity(path: string): { loaded: LoadedAuthConfig; identity: FileIdentity } { if (typeof path !== "string" || path.length === 0 || path.trim() !== path || path.includes("\0")) throw invalid(); - const value = parseAuthenticationConfig(readBoundedConfig(path)); - return { value, revision: canonicalRevision(value), sourcePath: path }; + const read = readBoundedConfig(path); + const value = parseAuthenticationConfig(read.source); + return { loaded: { value, revision: canonicalRevision(value), sourcePath: path }, identity: read.identity }; } -interface FileIdentity { dev: number; ino: number; size: number; mtimeMs: number } +export function loadAuthenticationConfig(path: string): LoadedAuthConfig { + return loadAuthenticationConfigWithIdentity(path).loaded; +} function fileIdentity(path: string): FileIdentity { try { @@ -164,9 +172,14 @@ export function createAuthenticationConfigProvider(path: string): Authentication return { current(): LoadedAuthConfig { const before = fileIdentity(path); if (cached && sameIdentity(cached.identity, before)) return cached.loaded; - const loaded = loadAuthenticationConfig(path); - cached = { identity: fileIdentity(path), loaded }; - return loaded; + for (let attempt = 0; attempt < 2; attempt += 1) { + const { loaded, identity } = loadAuthenticationConfigWithIdentity(path); + if (sameIdentity(identity, fileIdentity(path))) { + cached = { identity, loaded }; + return loaded; + } + } + throw invalid(); } }; } diff --git a/backend/src/config.ts b/backend/src/config.ts index 88da45ce..6b6b9587 100644 --- a/backend/src/config.ts +++ b/backend/src/config.ts @@ -190,6 +190,11 @@ export function loadConfig(env: Record): AppConfig { if (!(["none", "mock", "upstream"] as const).includes(requestedMode as "none" | "mock" | "upstream")) { throw new Error(`unsupported AUTH_MODE=${requestedMode}; use none, mock, or upstream`); } + const nodeEnvironment = env.NODE_ENV ?? process.env.NODE_ENV; + if ((requestedMode === "none" || requestedMode === "mock") + && nodeEnvironment !== "development" && nodeEnvironment !== "test") { + throw new Error("production requires auth.yaml or AUTH_MODE=upstream"); + } authMode = requestedMode as "none" | "mock" | "upstream"; } const publicExposure = env.THOTH_PUBLIC_EXPOSURE === "true"; diff --git a/backend/test/app-auth-mode.test.ts b/backend/test/app-auth-mode.test.ts new file mode 100644 index 00000000..8a51d872 --- /dev/null +++ b/backend/test/app-auth-mode.test.ts @@ -0,0 +1,27 @@ +import { expect, test } from "vitest"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { stringify } from "yaml"; +import { buildApp } from "../src/app.js"; +import { loadConfig } from "../src/config.js"; + +test("configured OIDC fails app startup until an OIDC handler is installed", () => { + const directory = mkdtempSync(join(tmpdir(), "thothii-app-oidc-mode-")); + const file = join(directory, "auth.yaml"); + writeFileSync(file, stringify({ + version: 1, mode: "oidc", publicUrl: "https://thothii.example.org", + oidc: { + issuer: "https://authentik.example.org/application/o/thothii/", clientId: "thothii", + clientSecretRef: "THT_OIDC_CLIENT_SECRET", scopes: ["openid"], groupsClaim: "groups", + }, + groupCatalog: { driver: "authentik", baseUrl: "https://authentik.example.org", apiTokenRef: "THT_AUTHENTIK_API_TOKEN" }, + authorization: { groupRoles: { "TOT Users": ["user"], "TOT Admin": ["admin"] } }, + }), "utf8"); + try { + expect(() => buildApp(loadConfig({ THT_AUTH_CONFIG_FILE: file }))) + .toThrow("configured authentication mode is not implemented"); + } finally { + rmSync(directory, { recursive: true, force: true }); + } +}); diff --git a/backend/test/auth-config.test.ts b/backend/test/auth-config.test.ts index 1beb6db6..707c4dd4 100644 --- a/backend/test/auth-config.test.ts +++ b/backend/test/auth-config.test.ts @@ -1,5 +1,6 @@ -import { afterEach, expect, test } from "vitest"; +import { afterEach, expect, test, vi } from "vitest"; import { mkdtempSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { createHash } from "node:crypto"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { stringify } from "yaml"; @@ -9,6 +10,22 @@ import { rolesToPermissions, } from "../src/auth/config.js"; +const readHook = vi.hoisted(() => ({ callback: undefined as undefined | (() => void) })); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readSync: (...args: any[]) => { + const result = (actual.readSync as any)(...args); + const callback = readHook.callback; + readHook.callback = undefined; + callback?.(); + return result; + }, + }; +}); + const directories: string[] = []; afterEach(() => { @@ -157,6 +174,22 @@ test("canonical group map order produces one stable revision", () => { expect(loadAuthenticationConfig(first).revision).toBe(loadAuthenticationConfig(reordered).revision); }); +test("canonical revisions use code-unit ordering for non-ASCII group names", () => { + const loaded = loadAuthenticationConfig(writeFixture(oidcConfig({ + authorization: { groupRoles: { "Ångström users": ["user"], "Zebra admins": ["admin"] } }, + }))); + const canonicalize = (value: unknown): unknown => Array.isArray(value) + ? value.map(canonicalize) + : value && typeof value === "object" + ? Object.fromEntries(Object.entries(value as Record) + .sort(([left], [right]) => left < right ? -1 : left > right ? 1 : 0) + .map(([key, nested]) => [key, canonicalize(nested)])) + : value; + const expected = createHash("sha256").update(JSON.stringify(canonicalize(loaded.value))).digest("hex"); + + expect(loaded.revision).toBe(expected); +}); + test("provider reloads after an atomic configuration replacement", () => { const file = writeFixture(localConfig()); const provider = createAuthenticationConfigProvider(file); @@ -170,6 +203,16 @@ test("provider reloads after an atomic configuration replacement", () => { expect(reloaded.value.publicUrl).toBe("http://127.0.0.1:9999"); }); +test("provider retries when replacement occurs between its read and cache identity check", () => { + const file = writeFixture(localConfig()); + const replacement = `${file}.replacement`; + writeFileSync(replacement, stringify(localConfig({ publicUrl: "http://127.0.0.1:9999" })), "utf8"); + const provider = createAuthenticationConfigProvider(file); + readHook.callback = () => renameSync(replacement, file); + + expect(provider.current().value.publicUrl).toBe("http://127.0.0.1:9999"); +}); + test("rejects input larger than one MiB", () => { const file = writeFixture(`${"#".repeat(1024 * 1024)}\n`); expect(() => loadAuthenticationConfig(file)).toThrow("authentication configuration is invalid"); diff --git a/backend/test/config.test.ts b/backend/test/config.test.ts index f15d6469..086902bf 100644 --- a/backend/test/config.test.ts +++ b/backend/test/config.test.ts @@ -71,6 +71,23 @@ test("loadConfig keeps local development defaults", () => { expect(loadConfig({}).dataRoot).toBeUndefined(); }); +test("loadConfig allows none and mock only outside production when auth.yaml is absent", () => { + const originalNodeEnvironment = process.env.NODE_ENV; + delete process.env.NODE_ENV; + try { + expect(() => loadConfig({ AUTH_MODE: "none" })).toThrow("production requires auth.yaml or AUTH_MODE=upstream"); + } finally { + if (originalNodeEnvironment === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = originalNodeEnvironment; + } + expect(loadConfig({ NODE_ENV: "test", AUTH_MODE: "mock" }).authMode).toBe("mock"); + expect(loadConfig({ NODE_ENV: "development", AUTH_MODE: "none" }).authMode).toBe("none"); + expect(loadConfig({ NODE_ENV: "production", AUTH_MODE: "upstream" }).authMode).toBe("upstream"); + expect(() => loadConfig({ NODE_ENV: "production" })).toThrow("production requires auth.yaml or AUTH_MODE=upstream"); + expect(() => loadConfig({ NODE_ENV: "production", AUTH_MODE: "mock" })) + .toThrow("production requires auth.yaml or AUTH_MODE=upstream"); +}); + test("loadConfig makes an existing auth.yaml authoritative and rejects AUTH_MODE split-brain", () => { const { directory, file } = authFile(oidcAuthConfig()); try {