From 628d35bfcdbf592bf54839f9f1d485213dc1f4dd Mon Sep 17 00:00:00 2001 From: Matthias Hochmeister Date: Tue, 9 Jun 2026 10:42:37 +0200 Subject: [PATCH] fix(admin): serverseitige authTyp-Pruefung beim Reset + Merge-PK-Kollision resetUserPassword: laedt das Konto in der Transaktion und bricht bei authTyp !== "local" ab (kein Hash, kein user.reset-Audit). Damit wird die dokumentierte Invariante "nur lokale Wehr-Benutzer zuruecksetzen" auch serverseitig erzwungen, nicht nur im UI. resetBrigadeUserPassword faengt den Fehler als { ok: false, error } ab. mergeMerkmal: loest PK-Kollisionen in vehicle_template_merkmale auf, indem proposed-Zeilen geloescht werden, wenn das Ziel-Merkmal in derselben Vorlage bereits existiert (zusammengesetzter PK template_id, merkmal_id). Das gesamte Umhaengen ist zudem in try/catch gekapselt und liefert bei Fehlern eine klare { ok: false }-Meldung - analog zu promoteMerkmal. Neue Unit-Tests (db/tx gemockt, kein Postgres noetig) decken beide Pfade ab. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../_actions/__tests__/proposals.test.ts | 144 ++++++++++++++++++ src/app/(admin)/_actions/brigades.ts | 16 +- src/app/(admin)/_actions/proposals.ts | 79 +++++++--- src/lib/admin/__tests__/provisioning.test.ts | 88 +++++++++++ src/lib/admin/provisioning.ts | 7 + 5 files changed, 311 insertions(+), 23 deletions(-) create mode 100644 src/app/(admin)/_actions/__tests__/proposals.test.ts create mode 100644 src/lib/admin/__tests__/provisioning.test.ts diff --git a/src/app/(admin)/_actions/__tests__/proposals.test.ts b/src/app/(admin)/_actions/__tests__/proposals.test.ts new file mode 100644 index 0000000..989f205 --- /dev/null +++ b/src/app/(admin)/_actions/__tests__/proposals.test.ts @@ -0,0 +1,144 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// --- Mocks --------------------------------------------------------------- + +const PROPOSED = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"; +const ZIEL = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"; + +// Geteilter, veraenderbarer Zustand. vi.hoisted laeuft vor den (gehoisteten) +// vi.mock-Factories, sodass diese den State sicher referenzieren koennen. +const state = vi.hoisted(() => ({ + // merkmale-Zeilen, die der Top-Level db.select() liefert. + merkmaleRows: [] as Array<{ id: string; typ: string; status: string }>, + // Reihenfolge der tx.select()-Ergebnisse fuer vehicle_template_merkmale: + // [0] = proposed-Templates, [1] = ziel-Templates. + vtmSelectQueue: [] as Array>, + ops: [] as { type: string; table: string; vals?: unknown }[], +})); + +function tableName(arg: unknown): string { + return (arg as { __name?: string })?.__name ?? "unknown"; +} + +vi.mock("@/db/schema", () => ({ + merkmale: { __name: "merkmale" }, + merkmalValues: { __name: "merkmal_values" }, + vehicleTemplateMerkmale: { __name: "vehicle_template_merkmale" }, +})); + +function makeTx() { + return { + select: () => ({ + from: () => ({ + where: () => Promise.resolve(state.vtmSelectQueue.shift() ?? []), + }), + }), + update: (table: unknown) => ({ + set: (vals: Record) => ({ + where: () => { + state.ops.push({ type: "update", table: tableName(table), vals }); + return Promise.resolve(undefined); + }, + }), + }), + delete: (table: unknown) => ({ + where: () => { + state.ops.push({ type: "delete", table: tableName(table) }); + return Promise.resolve(undefined); + }, + }), + }; +} + +vi.mock("@/db", () => ({ + db: { + select: () => ({ + from: () => Promise.resolve(state.merkmaleRows), + }), + transaction: (cb: (tx: ReturnType) => Promise) => + cb(makeTx()), + }, +})); + +vi.mock("@/lib/auth/guards", () => ({ + requirePlatformAdmin: () => + Promise.resolve({ user: { id: "actor-1" } }), +})); + +vi.mock("@/lib/audit", () => ({ + writeAudit: () => Promise.resolve(), +})); + +vi.mock("next/cache", () => ({ + revalidatePath: () => undefined, +})); + +// drizzle-orm Helfer (eq/and/inArray) muessen echte Aufrufe ueberstehen. +vi.mock("drizzle-orm", () => ({ + eq: (...a: unknown[]) => ({ op: "eq", a }), + and: (...a: unknown[]) => ({ op: "and", a }), + inArray: (...a: unknown[]) => ({ op: "inArray", a }), +})); + +// ------------------------------------------------------------------------- + +import { mergeMerkmal } from "@/app/(admin)/_actions/proposals"; + +describe("mergeMerkmal", () => { + beforeEach(() => { + state.ops.length = 0; + state.vtmSelectQueue = []; + state.merkmaleRows = [ + { id: PROPOSED, typ: "number", status: "proposed" }, + { id: ZIEL, typ: "number", status: "active" }, + ]; + }); + + it("lehnt unterschiedliche Typen ab", async () => { + state.merkmaleRows = [ + { id: PROPOSED, typ: "boolean", status: "proposed" }, + { id: ZIEL, typ: "number", status: "active" }, + ]; + const res = await mergeMerkmal({ proposedId: PROPOSED, zielId: ZIEL }); + expect(res.ok).toBe(false); + }); + + it("haengt ohne Kollision alle vtm-Zeilen um (kein Delete der proposed-vtm)", async () => { + // proposed in Template T1, Ziel in keinem -> keine Kollision. + state.vtmSelectQueue = [[{ templateId: "T1" }], []]; + const res = await mergeMerkmal({ proposedId: PROPOSED, zielId: ZIEL }); + expect(res.ok).toBe(true); + // Es darf kein Kollisions-Delete auf vtm geben, nur das finale + // merkmale-Delete. + const vtmDeletes = state.ops.filter( + (o) => o.type === "delete" && o.table === "vehicle_template_merkmale", + ); + expect(vtmDeletes).toHaveLength(0); + const vtmUpdates = state.ops.filter( + (o) => o.type === "update" && o.table === "vehicle_template_merkmale", + ); + expect(vtmUpdates).toHaveLength(1); + expect( + state.ops.some((o) => o.type === "delete" && o.table === "merkmale"), + ).toBe(true); + }); + + it("loescht kollidierende proposed-vtm-Zeilen vor dem Umhaengen", async () => { + // proposed und Ziel teilen sich Template T1 -> Kollision auf PK. + state.vtmSelectQueue = [[{ templateId: "T1" }], [{ templateId: "T1" }]]; + const res = await mergeMerkmal({ proposedId: PROPOSED, zielId: ZIEL }); + expect(res.ok).toBe(true); + const vtmDeletes = state.ops.filter( + (o) => o.type === "delete" && o.table === "vehicle_template_merkmale", + ); + expect(vtmDeletes).toHaveLength(1); + // Delete des Kollisions-Eintrags vor dem Umhaengen. + const deleteIdx = state.ops.findIndex( + (o) => o.type === "delete" && o.table === "vehicle_template_merkmale", + ); + const updateIdx = state.ops.findIndex( + (o) => o.type === "update" && o.table === "vehicle_template_merkmale", + ); + expect(deleteIdx).toBeLessThan(updateIdx); + }); +}); diff --git a/src/app/(admin)/_actions/brigades.ts b/src/app/(admin)/_actions/brigades.ts index f277767..461c123 100644 --- a/src/app/(admin)/_actions/brigades.ts +++ b/src/app/(admin)/_actions/brigades.ts @@ -50,7 +50,17 @@ export async function resetBrigadeUserPassword( const s = await requirePlatformAdmin(); const p = userResetSchema.safeParse(input); if (!p.success) return { ok: false, error: "Ungültige ID." }; - const { tempPassword } = await resetUserPassword(p.data.userId, s.user.id); - revalidatePath("/admin/wehren"); - return { ok: true, tempPassword }; + try { + const { tempPassword } = await resetUserPassword(p.data.userId, s.user.id); + revalidatePath("/admin/wehren"); + return { ok: true, tempPassword }; + } catch (e) { + return { + ok: false, + error: + e instanceof Error + ? e.message + : "Passwort konnte nicht zurückgesetzt werden.", + }; + } } diff --git a/src/app/(admin)/_actions/proposals.ts b/src/app/(admin)/_actions/proposals.ts index 03d8932..f62512a 100644 --- a/src/app/(admin)/_actions/proposals.ts +++ b/src/app/(admin)/_actions/proposals.ts @@ -1,6 +1,6 @@ "use server"; -import { eq } from "drizzle-orm"; +import { and, eq, inArray } from "drizzle-orm"; import { revalidatePath } from "next/cache"; import { db } from "@/db"; import { merkmale, merkmalValues, vehicleTemplateMerkmale } from "@/db/schema"; @@ -80,25 +80,64 @@ export async function mergeMerkmal(input: { }; } - await db.transaction(async (tx) => { - await tx - .update(merkmalValues) - .set({ merkmalId: ziel.data }) - .where(eq(merkmalValues.merkmalId, proposed.data)); - await tx - .update(vehicleTemplateMerkmale) - .set({ merkmalId: ziel.data }) - .where(eq(vehicleTemplateMerkmale.merkmalId, proposed.data)); - await tx.delete(merkmale).where(eq(merkmale.id, proposed.data)); - await writeAudit( - s.user.id, - "merkmal.merge", - "merkmal", - ziel.data, - { merged: proposed.data }, - tx, - ); - }); + try { + await db.transaction(async (tx) => { + // vehicle_template_merkmale hat den zusammengesetzten PK + // (template_id, merkmal_id). Hat eine Vorlage bereits sowohl das + // vorgeschlagene als auch das Ziel-Merkmal, würde ein pauschales + // Umhängen den PK verletzen. Solche kollidierenden Proposed-Zeilen + // werden daher gelöscht statt umgehängt. + const proposedVtm = await tx + .select({ templateId: vehicleTemplateMerkmale.templateId }) + .from(vehicleTemplateMerkmale) + .where(eq(vehicleTemplateMerkmale.merkmalId, proposed.data)); + const zielVtm = await tx + .select({ templateId: vehicleTemplateMerkmale.templateId }) + .from(vehicleTemplateMerkmale) + .where(eq(vehicleTemplateMerkmale.merkmalId, ziel.data)); + const zielTemplateIds = new Set(zielVtm.map((r) => r.templateId)); + const collidingTemplateIds = proposedVtm + .map((r) => r.templateId) + .filter((id) => zielTemplateIds.has(id)); + + if (collidingTemplateIds.length > 0) { + await tx + .delete(vehicleTemplateMerkmale) + .where( + and( + eq(vehicleTemplateMerkmale.merkmalId, proposed.data), + inArray( + vehicleTemplateMerkmale.templateId, + collidingTemplateIds, + ), + ), + ); + } + + await tx + .update(merkmalValues) + .set({ merkmalId: ziel.data }) + .where(eq(merkmalValues.merkmalId, proposed.data)); + await tx + .update(vehicleTemplateMerkmale) + .set({ merkmalId: ziel.data }) + .where(eq(vehicleTemplateMerkmale.merkmalId, proposed.data)); + await tx.delete(merkmale).where(eq(merkmale.id, proposed.data)); + await writeAudit( + s.user.id, + "merkmal.merge", + "merkmal", + ziel.data, + { merged: proposed.data }, + tx, + ); + }); + } catch { + return { + ok: false, + error: "Zusammenführen fehlgeschlagen. Bitte erneut versuchen.", + }; + } revalidatePath("/admin/merkmale/proposals"); return { ok: true }; diff --git a/src/lib/admin/__tests__/provisioning.test.ts b/src/lib/admin/__tests__/provisioning.test.ts new file mode 100644 index 0000000..34ec72f --- /dev/null +++ b/src/lib/admin/__tests__/provisioning.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +// --- Mocks --------------------------------------------------------------- + +const auditCalls: unknown[][] = []; +const updateSetCalls: Record[] = []; + +// Steuert, welchen authTyp das geladene Konto hat (oder kein Treffer). +let selectResult: Array<{ authTyp: "local" | "authentik" }> = []; + +function makeTx() { + return { + select: () => ({ + from: () => ({ + where: () => Promise.resolve(selectResult), + }), + }), + update: () => ({ + set: (vals: Record) => { + updateSetCalls.push(vals); + return { where: () => Promise.resolve(undefined) }; + }, + }), + }; +} + +vi.mock("@/db", () => ({ + db: { + transaction: (cb: (tx: ReturnType) => Promise) => + cb(makeTx()), + }, +})); + +vi.mock("@/lib/auth/password", () => ({ + hashPassword: (_pw: string) => Promise.resolve("HASHED"), +})); + +vi.mock("@/lib/audit", () => ({ + writeAudit: (...args: unknown[]) => { + auditCalls.push(args); + return Promise.resolve(); + }, +})); + +vi.mock("@/lib/geo/nominatim", () => ({ + geocodeAddress: () => Promise.resolve({ status: "fail" }), +})); + +// ------------------------------------------------------------------------- + +import { resetUserPassword } from "@/lib/admin/provisioning"; + +const USER = "11111111-1111-1111-1111-111111111111"; +const ACTOR = "22222222-2222-2222-2222-222222222222"; + +describe("resetUserPassword", () => { + beforeEach(() => { + auditCalls.length = 0; + updateSetCalls.length = 0; + selectResult = []; + }); + + it("setzt das Passwort fuer ein lokales Konto zurueck und schreibt Audit", async () => { + selectResult = [{ authTyp: "local" }]; + const res = await resetUserPassword(USER, ACTOR); + expect(typeof res.tempPassword).toBe("string"); + expect(res.tempPassword.length).toBeGreaterThan(0); + expect(updateSetCalls).toEqual([{ passwortHash: "HASHED" }]); + expect(auditCalls).toHaveLength(1); + expect(auditCalls[0]?.[1]).toBe("user.reset"); + }); + + it("bricht fuer Authentik-Konten ab: kein Hash, kein Audit", async () => { + selectResult = [{ authTyp: "authentik" }]; + await expect(resetUserPassword(USER, ACTOR)).rejects.toThrow( + /lokale Konten/i, + ); + expect(updateSetCalls).toHaveLength(0); + expect(auditCalls).toHaveLength(0); + }); + + it("bricht ab, wenn das Konto nicht existiert", async () => { + selectResult = []; + await expect(resetUserPassword(USER, ACTOR)).rejects.toThrow(); + expect(updateSetCalls).toHaveLength(0); + expect(auditCalls).toHaveLength(0); + }); +}); diff --git a/src/lib/admin/provisioning.ts b/src/lib/admin/provisioning.ts index bef4a55..09ea300 100644 --- a/src/lib/admin/provisioning.ts +++ b/src/lib/admin/provisioning.ts @@ -116,6 +116,13 @@ export async function resetUserPassword( const temp = generateTempPassword(); const hash = await hashPassword(temp); await db.transaction(async (tx) => { + const [u] = await tx + .select({ authTyp: users.authTyp }) + .from(users) + .where(eq(users.id, userId)); + if (!u || u.authTyp !== "local") { + throw new Error("Nur lokale Konten können zurückgesetzt werden."); + } await tx .update(users) .set({ passwortHash: hash })