From 91a9aca0d8785060d0470ba8df27a0e07baf6c4c Mon Sep 17 00:00:00 2001 From: Florian Wilde Date: Sun, 23 Aug 2026 23:20:26 +0200 Subject: [PATCH] Verweise auf fremde Akten an einer Stelle pruefen Wenn ein Formular einen Fremdschluessel mitschickt, kommt der aus einer Auswahlliste im Browser. Der Server darf sich darauf nicht verlassen - eine veraenderte Anfrage kann jede beliebige Id enthalten. Alle 22 Dateien unter src/server/actions/ durchgesehen. Das Ergebnis ist besser als befuerchtet: Bis auf eine Stelle war jeder Verweis geprueft. buchung.vorgangId ging ungeprueft durch. Konto, Beleg und Vertrag wurden geprueft, der Vorgang nicht. Die Pruefung liegt jetzt in lib/auth/verweise.ts statt verstreut in den Aktionen. Die acht erlaubten Ziele sind eine ausgeschriebene Aufzaehlung und kein dynamischer Tabellenzugriff: So sieht der Typpruefer jede Abfrage, und welche Tabellen erreichbar sind, ist eine Entscheidung und keine Nebenwirkung. Mehrere Verweise laufen nebeneinander - bei vier Verweisen eine Wartezeit statt vier. Umgestellt: die vier Einzelpruefungen der Buchung, die drei des Vermoegenspostens, die eigene Hilfsfunktion fuer den Ansprechpartner. Nicht angefasst wurden die Stellen, die ihre eigene, passendere Form haben - etwa das Kontaktprotokoll, das beide Verweise parallel laedt und feldgenaue Meldungen zurueckgibt. Zum Test: Der erste Entwurf rief die Server Action per fetch nach. Der waere auch ohne jede Pruefung gruen gewesen, weil Next einen Aufruf ohne gueltige Action-Id ohnehin ablehnt - er haette nichts bewiesen. Der jetzige schiebt dem echten Auswahlfeld einen Eintrag mit fremder Id unter und schickt das Formular normal ab. Gegenprobe mit ausgebauter Pruefung: faellt um, wie er soll. Refs #23 --- e2e/fremdakte.spec.ts | 116 ++++++++++++++++++++++++++++++ src/lib/auth/verweise.ts | 121 ++++++++++++++++++++++++++++++++ src/server/actions/finanzen.ts | 69 +++++------------- src/server/actions/vorgaenge.ts | 46 ++++-------- 4 files changed, 269 insertions(+), 83 deletions(-) create mode 100644 e2e/fremdakte.spec.ts create mode 100644 src/lib/auth/verweise.ts diff --git a/e2e/fremdakte.spec.ts b/e2e/fremdakte.spec.ts new file mode 100644 index 0000000..a7ce52a --- /dev/null +++ b/e2e/fremdakte.spec.ts @@ -0,0 +1,116 @@ +import { expect, test } from '@playwright/test' +import { PrismaClient } from '@prisma/client' +import { anmelden } from './hilfen' + +/** + * Verweise über Aktengrenzen hinweg — gegen die echte Anwendung. + * + * Die Auswahllisten im Browser zeigen nur die eigene Akte. Darauf darf + * sich der Server nicht verlassen: Wer das Formular verändert, schickt + * jede beliebige Id. + * + * Genau das wird hier nachgestellt — nicht als nachgebauter Aufruf, + * sondern indem dem echten Auswahlfeld ein Eintrag mit einer fremden Id + * untergeschoben und das Formular ganz normal abgeschickt wird. Nur so + * beweist der Test etwas: Er liefe rot, wenn die Prüfung fehlte. + * + * Solange nur eine betreute Person angelegt ist, wäre der Fehler + * folgenlos. Er bliebe es nicht — ein fremder Name stünde im Briefkopf. + */ + +const KENNUNG = 'Prüfhausen Fremdakte' + +async function mitDb(arbeit: (db: PrismaClient) => Promise): Promise { + const db = new PrismaClient() + try { + return await arbeit(db) + } finally { + await db.$disconnect() + } +} + +async function aufraeumen(): Promise { + await mitDb(async (db) => { + await db.betreute.deleteMany({ where: { nachname: { contains: 'Fremdakte' } } }) + }) +} + +test.describe('Verweise auf eine fremde Akte', () => { + test.beforeAll(async () => { + await aufraeumen() + }) + + test.afterAll(async () => { + await aufraeumen() + }) + + test('weist einen Kontakt aus einer anderen Akte ab', async ({ page }) => { + // Zweite Akte mit einem Kontakt darin — der angemeldete Benutzer hat + // auf sie keinen Zugriff. + const fremderKontakt = await mitDb(async (db) => { + const fremde = await db.betreute.create({ + data: { + vorname: 'Otto', + nachname: `${KENNUNG} Fremdakte`, + geburtsdatum: new Date(Date.UTC(1940, 0, 1)), + }, + }) + const kontakt = await db.kontakt.create({ + data: { + betreuteId: fremde.id, + name: `${KENNUNG} Fremder Sachbearbeiter`, + rolle: 'SACHBEARBEITER_BEZIRK', + }, + }) + return kontakt.id + }) + + await anmelden(page) + // Irgendeinen Vorgang der eigenen Akte öffnen. + const vorgangId = await mitDb(async (db) => { + const vorgang = await db.vorgang.findFirst({ + where: { geloeschtAm: null, betreute: { vorname: 'Erika' } }, + select: { id: true }, + }) + return vorgang?.id ?? '' + }) + test.skip(vorgangId === '', 'Kein Vorgang in der eigenen Akte.') + + await page.goto(`/vorgaenge/${vorgangId}`) + const auswahl = page.locator('select[name="ansprechpartnerId"]') + await auswahl.waitFor() + + // Dem echten Auswahlfeld die fremde Id unterschieben und ganz normal + // abschicken — so, wie es ein verändertes Formular täte. + await auswahl.evaluate((feld, id) => { + const eintrag = document.createElement('option') + eintrag.value = id + eintrag.textContent = 'untergeschoben' + ;(feld as HTMLSelectElement).append(eintrag) + ;(feld as HTMLSelectElement).value = id + }, fremderKontakt) + + await page.getByRole('button', { name: 'Speichern' }).click() + + // Der Server lehnt ab, mit einer Meldung, die den Grund nennt. + // Grosszuegige Frist: Im Entwicklungsmodus wird die Server Action beim + // ersten Aufruf uebersetzt, und das dauert laenger als die Vorgabe. + await expect(page.getByText(/gehört nicht zu dieser betreuten Person/).first()).toBeVisible({ + timeout: 45_000, + }) + + // Und vor allem: Die Verknüpfung ist nicht entstanden. + const verknuepft = await mitDb((db) => + db.vorgang.count({ where: { ansprechpartnerId: fremderKontakt } }), + ) + expect(verknuepft).toBe(0) + }) + + test('die Auswahlliste zeigt den fremden Kontakt gar nicht erst', async ({ page }) => { + await anmelden(page) + await page.goto('/vorgaenge') + + const inhalt = await page.locator('body').innerText() + expect(inhalt).not.toContain('Fremder Sachbearbeiter') + }) +}) diff --git a/src/lib/auth/verweise.ts b/src/lib/auth/verweise.ts new file mode 100644 index 0000000..1233706 --- /dev/null +++ b/src/lib/auth/verweise.ts @@ -0,0 +1,121 @@ +import 'server-only' +import { db, nichtGeloescht } from '@/lib/db' + +/** + * Gehoert ein verknuepfter Datensatz zu dieser Akte? + * + * Wenn ein Formular einen Fremdschluessel mitschickt - Konto, Vertrag, + * Immobilie, Dokument, Vorgang, Kontakt -, dann kommt der aus einer + * Auswahlliste im Browser. Der Server darf sich darauf nicht verlassen: + * Eine veraenderte Anfrage kann jede beliebige Id enthalten. + * + * Ohne die Pruefung entstuende eine Verknuepfung ueber Aktengrenzen + * hinweg. Was daran gefaehrlich ist, sieht man erst an den Folgen: Der + * Rueckkaufswert der einen Akte haengt am Vertrag der anderen, ein + * fremder Name steht im Briefkopf, ein fremder Beschluss in der + * Genehmigungskette. + * + * Solange nur eine betreute Person angelegt ist, ist all das folgenlos. + * Die Pruefung gehoert trotzdem hin - sie kostet eine Abfrage, und die + * zweite betreute Person kommt frueher, als man denkt. + * + * Bewusst KEINE Ausnahme geworfen: Formularaktionen geben Fehler zurueck, + * damit die Eingaben des Benutzers nicht verlorengehen. + */ + +/** Die Tabellen, auf die ein Formular verweisen darf. */ +export type Verweisziel = + | 'konto' + | 'vertrag' + | 'immobilie' + | 'dokument' + | 'vorgang' + | 'kontakt' + | 'vermoegensposten' + | 'buchung' + +type Sucher = (id: string, betreuteId: string) => Promise<{ id: string } | null> + +/** + * Je Ziel eine Abfrage, ausgeschrieben statt dynamisch zusammengesetzt. + * + * Das ist ein paar Zeilen laenger als ein Zugriff ueber den Tabellennamen - + * dafuer ist die Liste der erlaubten Ziele eine bewusste Aufzaehlung und + * keine Nebenwirkung, und der Typpruefer sieht jede Abfrage. + */ +const SUCHER: Record = { + konto: (id, betreuteId) => + db.konto.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + vertrag: (id, betreuteId) => + db.vertrag.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + immobilie: (id, betreuteId) => + db.immobilie.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + dokument: (id, betreuteId) => + db.dokument.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + vorgang: (id, betreuteId) => + db.vorgang.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + kontakt: (id, betreuteId) => + db.kontakt.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), + vermoegensposten: (id, betreuteId) => + db.vermoegensposten.findFirst({ + where: { id, betreuteId, ...nichtGeloescht }, + select: { id: true }, + }), + buchung: (id, betreuteId) => + db.buchung.findFirst({ where: { id, betreuteId, ...nichtGeloescht }, select: { id: true } }), +} + +const BESCHRIFTUNG: Record = { + konto: 'Dieses Konto', + vertrag: 'Dieser Vertrag', + immobilie: 'Diese Immobilie', + dokument: 'Dieses Dokument', + vorgang: 'Dieser Vorgang', + kontakt: 'Dieser Kontakt', + vermoegensposten: 'Dieser Vermögensposten', + buchung: 'Diese Buchung', +} + +/** + * Prueft einen einzelnen Verweis. + * + * Ein leerer Verweis ist in Ordnung - "keine Verknuepfung" ist ein + * gueltiger Zustand und keine Verletzung. + * + * @returns null, wenn alles stimmt; sonst die fertige Fehlermeldung. + */ +export async function verweisPruefen( + ziel: Verweisziel, + id: string | null | undefined, + betreuteId: string, +): Promise { + if (!id) return null + + const treffer = await SUCHER[ziel](id, betreuteId) + + return treffer ? null : `${BESCHRIFTUNG[ziel]} gehört nicht zu dieser betreuten Person.` +} + +/** + * Prueft mehrere Verweise auf einmal und meldet den ersten Verstoss. + * + * Die Abfragen laufen nebeneinander: Bei vier Verweisen ist das eine + * Wartezeit statt vier. + * + * @example + * const fehler = await verweisePruefen(betreuteId, { + * konto: werte.kontoId, + * vertrag: werte.vertragId, + * }) + * if (fehler) return { fehler } + */ +export async function verweisePruefen( + betreuteId: string, + verweise: Partial>, +): Promise { + const eintraege = Object.entries(verweise) as [Verweisziel, string | null | undefined][] + const ergebnisse = await Promise.all( + eintraege.map(([ziel, id]) => verweisPruefen(ziel, id, betreuteId)), + ) + return ergebnisse.find((meldung) => meldung !== null) ?? null +} diff --git a/src/server/actions/finanzen.ts b/src/server/actions/finanzen.ts index 9ff113b..4291d00 100644 --- a/src/server/actions/finanzen.ts +++ b/src/server/actions/finanzen.ts @@ -4,6 +4,7 @@ import { revalidatePath } from 'next/cache' import { redirect } from 'next/navigation' import { Prisma } from '@prisma/client' import { db, nichtGeloescht } from '@/lib/db' +import { verweisePruefen } from '@/lib/auth/verweise' import { erfordereZugang, ZugriffVerweigert } from '@/lib/auth/sitzung' import { aenderungErmitteln, protokollieren } from '@/lib/audit' import { sealen } from '@/lib/crypto/feld' @@ -325,29 +326,15 @@ export async function buchungSpeichern( } const werte = gepruft.data - if (werte.kontoId) { - const konto = await db.konto.findFirst({ - where: { id: werte.kontoId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!konto) return { fehler: 'Dieses Konto gehört nicht zu dieser betreuten Person.' } - } - - // Wie beim Konto: Der Vertrag muss zu dieser Akte gehoeren. - if (werte.vertragId) { - const vertrag = await db.vertrag.findFirst({ - where: { id: werte.vertragId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!vertrag) return { fehler: 'Dieser Vertrag gehört nicht zu dieser betreuten Person.' } - } - if (werte.belegId) { - const beleg = await db.dokument.findFirst({ - where: { id: werte.belegId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!beleg) return { fehler: 'Dieses Dokument gehört nicht zu dieser betreuten Person.' } - } + // Alle vier Verweise auf einmal - der Vorgang war bisher der einzige, + // der ungeprueft durchging. + const verweisfehler = await verweisePruefen(betreuteId, { + konto: werte.kontoId, + vertrag: werte.vertragId, + dokument: werte.belegId, + vorgang: werte.vorgangId, + }) + if (verweisfehler) return { fehler: verweisfehler } const felder = { kontoId: werte.kontoId, @@ -635,34 +622,14 @@ export async function vermoegenspostenSpeichern( } const werte = gepruft.data - if (werte.kontoId) { - const konto = await db.konto.findFirst({ - where: { id: werte.kontoId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!konto) return { fehler: 'Dieses Konto gehört nicht zu dieser betreuten Person.' } - } - - // Dieselbe Pruefung fuer die beiden anderen Verweise. Sie fehlte: - // An der Verknuepfung zum Vertrag haengt dessen Rueckkaufswert, und - // die darf nicht in eine fremde Akte zeigen. - if (werte.vertragId) { - const vertrag = await db.vertrag.findFirst({ - where: { id: werte.vertragId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!vertrag) return { fehler: 'Dieser Vertrag gehört nicht zu dieser betreuten Person.' } - } - - if (werte.immobilieId) { - const immobilie = await db.immobilie.findFirst({ - where: { id: werte.immobilieId, betreuteId, ...nichtGeloescht }, - select: { id: true }, - }) - if (!immobilie) { - return { fehler: 'Diese Immobilie gehört nicht zu dieser betreuten Person.' } - } - } + // An der Verknuepfung zum Vertrag haengt dessen Rueckkaufswert - sie + // darf nicht in eine fremde Akte zeigen. + const verweisfehler = await verweisePruefen(betreuteId, { + konto: werte.kontoId, + vertrag: werte.vertragId, + immobilie: werte.immobilieId, + }) + if (verweisfehler) return { fehler: verweisfehler } const felder = { bezeichnung: werte.bezeichnung, diff --git a/src/server/actions/vorgaenge.ts b/src/server/actions/vorgaenge.ts index bb337a7..6c95527 100644 --- a/src/server/actions/vorgaenge.ts +++ b/src/server/actions/vorgaenge.ts @@ -6,6 +6,7 @@ import type { Prisma } from '@prisma/client' import { db, nichtGeloescht } from '@/lib/db' import { erfordereZugang } from '@/lib/auth/sitzung' import { aenderungErmitteln, protokollieren, type Aenderung } from '@/lib/audit' +import { verweisePruefen } from '@/lib/auth/verweise' import { oeffnen, sealen } from '@/lib/crypto/feld' import { AufgabeAbhakenSchema, @@ -69,34 +70,6 @@ async function kontaktnameZu(kontaktId: string | null): Promise { return [kontakt.name, kontakt.organisation].filter(Boolean).join(', ') } -/** - * Gehoert der Ansprechpartner zu dieser Akte? - * - * Ohne die Pruefung koennte ein veraendertes Formular einen Kontakt aus - * einer fremden Akte an den Vorgang haengen - und sein Name stuende - * anschliessend im Briefkopf. - */ -async function ansprechpartnerPruefen( - betreuteId: string, - ansprechpartnerId: string | null, -): Promise<{ ok: true } | { ok: false; zustand: Formularzustand }> { - if (!ansprechpartnerId) return { ok: true } - - const kontakt = await db.kontakt.findFirst({ - where: { id: ansprechpartnerId, betreuteId, geloeschtAm: null }, - select: { id: true }, - }) - if (kontakt) return { ok: true } - - return { - ok: false, - zustand: { - fehler: 'Diesen Kontakt gibt es nicht (mehr).', - felder: { ansprechpartnerId: 'Bitte einen Kontakt aus dem Verzeichnis wählen.' }, - }, - } -} - export async function vorgangAnlegen( _vorher: Formularzustand, formular: FormData, @@ -119,8 +92,13 @@ export async function vorgangAnlegen( const eingabe = gelesen.data - const geprueft = await ansprechpartnerPruefen(betreuteId, eingabe.ansprechpartnerId) - if (!geprueft.ok) return geprueft.zustand + // Sein Name stuende sonst im Briefkopf - siehe lib/auth/verweise.ts. + const verweisfehler = await verweisePruefen(betreuteId, { + kontakt: eingabe.ansprechpartnerId, + }) + if (verweisfehler) { + return { fehler: verweisfehler, felder: { ansprechpartnerId: verweisfehler } } + } const jetzt = new Date() @@ -250,8 +228,12 @@ export async function vorgangAendern( const eingabe = gelesen.data - const geprueft = await ansprechpartnerPruefen(vorgang.betreuteId, eingabe.ansprechpartnerId) - if (!geprueft.ok) return geprueft.zustand + const verweisfehler = await verweisePruefen(vorgang.betreuteId, { + kontakt: eingabe.ansprechpartnerId, + }) + if (verweisfehler) { + return { fehler: verweisfehler, felder: { ansprechpartnerId: verweisfehler } } + } const vorherOffen = oeffnen('Vorgang', vorgang)