fix(auth): never sign anyone in through OIDC without an allowlist

changelog.d/fixes/oidc-empty-allowlist-fail-closed.md

@@ -0,0 +1 @@
+- **fix(auth):** the OIDC callback no longer signs anyone in when `oidcAllowedSubjects` is empty, and the settings route rejects any update that would leave OIDC enabled with an empty allowlist, not only the one that enables it

docs/architecture/AUTHZ_GUIDE.md

@@ -64,10 +64,11 @@ supplemented:
 - `GET /api/auth/oidc/callback` validates `state`, exchanges the authorization
   code, and verifies the ID token's signature via the issuer's JWKS
   (`jose`'s `createRemoteJWKSet`, cached per JWKS URI) with `issuer`/`audience`
-  checks. An optional `oidcAllowedSubjects` allowlist matches the token's
-  `sub` claim or its `email` claim — the email claim is only honored when
-  `email_verified === true`, so an unverified email at the IdP can never pass
-  the gate.
+  checks. The `oidcAllowedSubjects` allowlist is required: it matches the
+  token's `sub` claim or its `email` claim — the email claim is only honored
+  when `email_verified === true`, so an unverified email at the IdP can never
+  pass the gate — and with no entries the callback refuses every login
+  (`not_configured`).
 - On success it mints the **exact same** 30-day `auth_token` JWT the password
   login issues (`src/app/api/auth/login/route.ts`), so the rest of the
   dashboard session pipeline (auto-refresh, cookie flags) is unchanged —

docs/openapi.yaml

@@ -5903,9 +5903,9 @@ paths:
       summary: Complete OIDC login for the dashboard admin gate
       description: |
         Validates the `state` cookie, exchanges the authorization `code` for tokens,
-        verifies the ID token against the issuer's JWKS (audience = client id), and —
-        if `oidcAllowedSubjects` is configured — checks the token's `sub`/`email` against
-        that allowlist. On success it mints the same 30-day `auth_token` dashboard-session
+        verifies the ID token against the issuer's JWKS (audience = client id), and
+        checks the token's `sub`/`email` against `oidcAllowedSubjects`, which must hold at
+        least one entry (an empty list ends in `not_configured`). On success it mints the same 30-day `auth_token` dashboard-session
         JWT used by password login and redirects to `/dashboard`.
       parameters:
         - name: code

src/app/api/auth/oidc/callback/route.ts

@@ -85,7 +85,13 @@ export async function GET(request: Request) {
       ? settings.oidcRedirectPath
       : "/api/auth/oidc/callback";
 
-  if (!enabled || !rawIssuer || !clientId || !clientSecret) {
+  // Without an allowlist every account at the identity provider would be let in as the
+  // dashboard admin, so an empty list counts as not configured.
+  const allowed = Array.isArray(settings.oidcAllowedSubjects)
+    ? settings.oidcAllowedSubjects.filter((v: unknown) => typeof v === "string" && v.trim() !== "")
+    : [];
+
+  if (!enabled || !rawIssuer || !clientId || !clientSecret || allowed.length === 0) {
     return NextResponse.redirect(new URL("/login?oidc_error=not_configured", originEarly));
   }
 
@@ -190,23 +196,18 @@ export async function GET(request: Request) {
       audience: clientId,
     });
 
-    // Optional subject / email whitelist
-    const allowed = Array.isArray(settings.oidcAllowedSubjects) ? settings.oidcAllowedSubjects : [];
-    if (allowed.length > 0) {
-      const sub = typeof payload.sub === "string" ? payload.sub : "";
-      const emailVerified = (payload as Record<string, unknown>).email_verified === true;
-      const email =
-        emailVerified && typeof (payload as Record<string, unknown>).email === "string"
-          ? ((payload as Record<string, unknown>).email as string).toLowerCase()
-          : "";
-      const ok = allowed.some((v: unknown) => {
-        if (typeof v !== "string") return false;
-        if (v === sub) return true;
-        return email !== "" && v.toLowerCase() === email;
-      });
-      if (!ok) {
-        return NextResponse.redirect(new URL("/login?oidc_error=subject_not_allowed", originEarly));
-      }
+    const sub = typeof payload.sub === "string" ? payload.sub : "";
+    const emailVerified = (payload as Record<string, unknown>).email_verified === true;
+    const email =
+      emailVerified && typeof (payload as Record<string, unknown>).email === "string"
+        ? ((payload as Record<string, unknown>).email as string).toLowerCase()
+        : "";
+    const ok = allowed.some((v: string) => {
+      if (v === sub) return true;
+      return email !== "" && v.toLowerCase() === email;
+    });
+    if (!ok) {
+      return NextResponse.redirect(new URL("/login?oidc_error=subject_not_allowed", originEarly));
     }
   } catch {
     return NextResponse.redirect(new URL("/login?oidc_error=id_token_invalid", originEarly));

src/app/api/settings/route.ts

@@ -323,13 +323,14 @@ export async function PATCH(request: Request) {
       }) as typeof body.modelLockout;
     }
 
-    if (body.oidcEnabled === true) {
+    if (body.oidcEnabled !== undefined || body.oidcAllowedSubjects !== undefined) {
       const current = await getSettings();
+      const enabled = body.oidcEnabled ?? current.oidcEnabled === true;
       const subjects = Array.isArray(body.oidcAllowedSubjects)
         ? (body.oidcAllowedSubjects as unknown[])
         : ((current.oidcAllowedSubjects as unknown[] | undefined) ?? []);
       const hasAtLeastOne = subjects.some((s) => typeof s === "string" && s.trim().length > 0);
-      if (!hasAtLeastOne) {
+      if (enabled && !hasAtLeastOne) {
         emitSettingsFailureAudit(request, actor, "OIDC_ALLOWED_SUBJECTS_REQUIRED", attemptedKeys);
         return NextResponse.json(
           {

tests/unit/oidc-callback.test.ts

@@ -75,7 +75,7 @@ async function setupFullOidcSettings() {
     oidcClientId: "client-oidc-test",
     oidcClientSecret: "secret-oidc-test",
     oidcRedirectPath: "/api/auth/oidc/callback",
-    oidcAllowedSubjects: [],
+    oidcAllowedSubjects: ["user-123"],
   });
 }
 
@@ -518,7 +518,7 @@ test("OIDC callback handles issuer with trailing slash in settings and token (#1
     oidcClientId: "client-oidc-authentik",
     oidcClientSecret: "secret-oidc-authentik",
     oidcRedirectPath: "/api/auth/oidc/callback",
-    oidcAllowedSubjects: [],
+    oidcAllowedSubjects: ["authentik-user-1"],
   });
 
   const { idToken, jwks } = await createSignedIdToken({
@@ -583,7 +583,7 @@ test("OIDC callback handles issuer mismatch on trailing slash between settings a
     oidcClientId: "client-oidc-authentik-mismatch",
     oidcClientSecret: "secret-oidc-authentik-mismatch",
     oidcRedirectPath: "/api/auth/oidc/callback",
-    oidcAllowedSubjects: [],
+    oidcAllowedSubjects: ["authentik-user-2"],
   });
 
   // Token signed with trailing slash (common with Authentik discovery)
@@ -634,3 +634,35 @@ test("OIDC callback handles issuer mismatch on trailing slash between settings a
     globalThis.fetch = originalFetch;
   }
 });
+
+for (const [label, subjects] of [
+  ["an empty allowlist", []],
+  ["an allowlist of blank entries", ["", "   "]],
+] as const) {
+  test(`OIDC callback refuses to sign anyone in with ${label} (not_configured)`, async () => {
+    await setupFullOidcSettings();
+    await localDb.updateSettings({ oidcAllowedSubjects: [...subjects] });
+
+    const testState = "state-empty-allowlist";
+    capturedCookies["oidc_state"] = { value: testState };
+
+    const originalFetch = globalThis.fetch;
+    let outboundCalls = 0;
+    globalThis.fetch = (async () => {
+      outboundCalls += 1;
+      throw new Error("the identity provider must not be contacted");
+    }) as typeof fetch;
+    try {
+      const response = await callbackRoute.GET(
+        new Request(`http://localhost/api/auth/oidc/callback?code=foo&state=${testState}`)
+      );
+
+      assert.equal(response.status, 307);
+      assert.ok((response.headers.get("location") || "").includes("oidc_error=not_configured"));
+      assert.equal(outboundCalls, 0);
+      assert.equal(capturedCookies["auth_token"], undefined);
+    } finally {
+      globalThis.fetch = originalFetch;
+    }
+  });
+}

tests/unit/oidc-settings-allowlist-guard.test.ts

@@ -0,0 +1,87 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { makeManagementSessionRequest } from "../helpers/managementSession.ts";
+
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-oidc-settings-guard-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const core = await import("../../src/lib/db/core.ts");
+const settingsDb = await import("../../src/lib/db/settings.ts");
+const settingsRoute = await import("../../src/app/api/settings/route.ts");
+
+test.beforeEach(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+});
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });
+});
+
+async function patch(body: Record<string, unknown>) {
+  const response = await settingsRoute.PATCH(
+    await makeManagementSessionRequest("http://localhost/api/settings", {
+      method: "PATCH",
+      body,
+    })
+  );
+  return { status: response.status, body: (await response.json()) as any };
+}
+
+async function seedEnabled(subjects: string[]) {
+  await settingsDb.updateSettings({
+    oidcEnabled: true,
+    oidcIssuer: "https://idp.test",
+    oidcClientId: "client",
+    oidcClientSecret: "secret",
+    oidcAllowedSubjects: subjects,
+  });
+}
+
+test("emptying the allowlist while OIDC stays enabled is rejected", async () => {
+  await seedEnabled(["admin@example.com"]);
+
+  const result = await patch({ oidcAllowedSubjects: [] });
+
+  assert.equal(result.status, 400);
+  assert.equal(result.body.error.code, "OIDC_ALLOWED_SUBJECTS_REQUIRED");
+  assert.deepEqual((await settingsDb.getSettings()).oidcAllowedSubjects, ["admin@example.com"]);
+});
+
+test("an allowlist of blank entries is rejected while OIDC is enabled", async () => {
+  await seedEnabled(["admin@example.com"]);
+
+  const result = await patch({ oidcAllowedSubjects: ["", "  "] });
+
+  assert.equal(result.status, 400);
+  assert.equal(result.body.error.code, "OIDC_ALLOWED_SUBJECTS_REQUIRED");
+});
+
+test("enabling OIDC without any allowed subject is rejected", async () => {
+  const result = await patch({ oidcEnabled: true });
+
+  assert.equal(result.status, 400);
+  assert.equal(result.body.error.code, "OIDC_ALLOWED_SUBJECTS_REQUIRED");
+});
+
+test("replacing the allowlist with another non-empty one is accepted", async () => {
+  await seedEnabled(["admin@example.com"]);
+
+  const result = await patch({ oidcAllowedSubjects: ["other@example.com"] });
+
+  assert.equal(result.status, 200);
+  assert.deepEqual((await settingsDb.getSettings()).oidcAllowedSubjects, ["other@example.com"]);
+});
+
+test("the allowlist can be emptied when OIDC is off", async () => {
+  await settingsDb.updateSettings({ oidcEnabled: false, oidcAllowedSubjects: ["a@example.com"] });
+
+  const result = await patch({ oidcAllowedSubjects: [] });
+
+  assert.equal(result.status, 200);
+});