R190-01: Rollentausch zerstoerte, wo er ablehnte

Befund der Pentesterin. updateUser machte deleteMany + createMany ohne Transaktion und ohne skipDuplicates. Eine doppelte roleId ([4,4]) oder eine erfundene ([99999]) kam durch die Teilmengenregel - sie bringt keine Rechte mit, faellt dort also nicht auf - und schlug erst beim Schreiben fehl, nach dem Loeschen. Antwort HTTP 400, Konto danach ohne jede Rolle. Die Antwort log ueber sich selbst: "400" liest sich als abgelehnt, nichts passiert. Zerstoert wurde trotzdem. Damit war die Letzter-Admin-Sperre umgangen, ohne sie anzugreifen: Sie prueft die Absicht (die Admin-Rolle steht im Body, also "bleibt Admin"), der Schreibvorgang scheiterte danach. Aussperrung, Rueckweg nur per CLI. Auch versehentlich ausloesbar durch doppelte IDs aus dem Frontend. Es war meine eigene Haertung, die ich nicht mitgenommen habe: updateRole hatte Dedup und skipDuplicates seit Etappe 1, das Geschwister updateUser/createUser nicht. Genau das Muster, das in derselben Runde dreimal aufgeraeumt wurde - eine Absicherung, die nur in einer von zwei Kopien ankommt. normalisiereRollenIds() prueft jetzt vor jeder Entscheidung und jedem Schreibvorgang gegen die Datenbank, dedupliziert, und der Tausch laeuft in einer Transaktion. Die Letzter-Admin-Sperre arbeitet damit auf einer Liste, die auch einloesbar ist. Nebenbefund beim Nachstellen: Die rohe Prisma-Fehlermeldung ging wortwoertlich an den Client, inklusive Dateipfad des Servers. Wird jetzt protokolliert statt ausgeliefert. Nachgestellt: [22,22] -> 200 mit erhaltener Rolle; [99999] und [22,99999] -> 400 mit Rollen unveraendert; [-1] -> 400; [22,23] -> 200 korrekt gesetzt. createUser ebenso, abgelehnte Anlagen hinterlassen kein Konto. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-09 01:14:53 +02:00
parent 9f879759d8
commit b2963be482
4 changed files with 135 additions and 12 deletions
@@ -7,7 +7,7 @@ import { AUDIT_OPS_ROLLE } from '../services/user.service.js';
 import { ApiResponse, AuthRequest } from '../types/index.js';
 import { emit as emitSecurityEvent, contextFromRequest } from '../services/securityMonitor.service.js';
 import { pickUserCreate, pickUserUpdate, pickRoleUpdate, isValidEmail, sanitizePhoneField } from '../utils/sanitize.js';
-import { RechteEskalationError, RollenSperrError } from '../services/rechte.service.js';
+import { RechteEskalationError, RollenSperrError, UngueltigeEingabeError } from '../services/rechte.service.js';
 import { validatePasswordComplexity, STAFF_MIN_PASSWORD_LENGTH } from '../utils/passwordGenerator.js';

 // Users
@@ -650,6 +650,22 @@ function antworteAufRollenFehler(res: Response, error: unknown, fallback: string
    res.status(403).json({ success: false, error: error.message } as ApiResponse);
    return;
  }
+  if (error instanceof UngueltigeEingabeError) {
+    res.status(400).json({ success: false, error: error.message } as ApiResponse);
+    return;
+  }
+
+  // Datenbankfehler NICHT durchreichen. Prisma haengt den vollstaendigen
+  // Aufruf samt Dateipfad an die Meldung - das ging bisher wortwoertlich an
+  // den Client. Nach aussen die allgemeine Auskunft, die Einzelheiten ins
+  // Serverprotokoll.
+  const name = error instanceof Error ? error.name : '';
+  if (name.startsWith('Prisma')) {
+    console.error(`[${fallback}]`, error);
+    res.status(400).json({ success: false, error: fallback } as ApiResponse);
+    return;
+  }
+
  res.status(400).json({
    success: false,
    error: error instanceof Error ? error.message : fallback,
@@ -42,6 +42,18 @@ export class RechteEskalationError extends Error {
  }
 }

+/**
+ * Die Eingabe nennt etwas, das es nicht gibt - eine unbekannte Rechte- oder
+ * Rollen-ID. Eigene Klasse, damit daraus ein 400 wird und nicht ein 500 aus
+ * einem Fremdschluesselfehler tief in Prisma.
+ */
+export class UngueltigeEingabeError extends Error {
+  constructor(nachricht: string) {
+    super(nachricht);
+    this.name = 'UngueltigeEingabeError';
+  }
+}
+
 /** Der Vorgang zielte auf eine Systemrolle, die von der Anwendung gepflegt wird. */
 export class RollenSperrError extends Error {
  constructor(nachricht: string) {
@@ -103,7 +115,7 @@ export async function rechteVonPermissionIds(ids: number[]): Promise<Set<string>
  if (gefunden.length !== new Set(ids).size) {
    const bekannt = new Set(gefunden.map((p) => p.id));
    const unbekannt = [...new Set(ids)].filter((id) => !bekannt.has(id));
-    throw new Error(`Unbekannte Rechte-ID: ${unbekannt.join(', ')}`);
+    throw new UngueltigeEingabeError(`Unbekannte Rechte-ID: ${unbekannt.join(', ')}`);
  }
  for (const p of gefunden) rechte.add(`${p.resource}:${p.action}`);
  return rechte;
@@ -175,3 +187,40 @@ export function rechteDerHaken(haken: {
  if (haken.hasAuditOpsAccess === true) rechte.push(...rechteDerSystemrolle(ROLLE_AUDIT_BETRIEB));
  return [...new Set(rechte)];
 }
+
+/**
+ * Prueft Rollen-IDs und gibt sie doppelt-frei zurueck.
+ *
+ * Muss VOR jedem Schreibvorgang laufen. Ohne das kam eine Dublette
+ * (`[4,4]`) oder eine erfundene ID erst beim Schreiben zum Vorschein - und
+ * beim Rollentausch geschah das NACH dem Loeschen der alten Zuordnungen.
+ * Ergebnis war ein HTTP 400, das wie "Eingabe abgelehnt, nichts passiert"
+ * aussah, waehrend das Konto in Wahrheit ohne jede Rolle dastand.
+ *
+ * Besonders bitter am letzten Admin: Die Sperre prueft die ABSICHT (die
+ * Admin-Rolle steht ja in der Anfrage) und liess den Vorgang durch - der
+ * Schreibvorgang scheiterte danach. Aussperrung, obwohl die Sperre gegriffen
+ * zu haben schien (Pentest R190-01).
+ *
+ * `rechteVonRollen` fing das nicht ab: Eine unbekannte ID findet einfach
+ * keine Rolle, bringt also keine Rechte mit und faellt durch die
+ * Teilmengenregel nicht auf. Das war richtig fuer die Rechtefrage und
+ * falsch als Eingabepruefung - zwei verschiedene Aufgaben.
+ */
+export async function normalisiereRollenIds(roleIds: number[]): Promise<number[]> {
+  const eindeutig = [...new Set(roleIds)];
+  if (eindeutig.length === 0) return [];
+  if (!eindeutig.every((id) => Number.isInteger(id) && id >= 1)) {
+    throw new UngueltigeEingabeError('roleIds darf nur positive ganze Zahlen enthalten');
+  }
+  const gefunden = await prisma.role.findMany({
+    where: { id: { in: eindeutig } },
+    select: { id: true },
+  });
+  if (gefunden.length !== eindeutig.length) {
+    const bekannt = new Set(gefunden.map((r) => r.id));
+    const unbekannt = eindeutig.filter((id) => !bekannt.has(id));
+    throw new UngueltigeEingabeError(`Unbekannte Rollen-ID: ${unbekannt.join(', ')}`);
+  }
+  return eindeutig;
+}
@@ -7,6 +7,7 @@ import {
  rechteVonRollen,
  rechteDerHaken,
  effektiveRechte,
+  normalisiereRollenIds,
  meldeTraegerAb,
  RollenSperrError,
 } from './rechte.service.js';
@@ -171,8 +172,13 @@ export async function createUser(data: {
  // Was das neue Konto koennen wird - aus den Rollen UND aus den Haken.
  // `handelnderId` ist Pflichtparameter, damit kein Aufrufer die Pruefung
  // vergessen kann; der Compiler erzwingt sie.
+  // Erst pruefen, dann rechnen: Eine unbekannte oder doppelte Rollen-ID
+  // faellt der Teilmengenregel nicht auf (sie bringt keine Rechte mit) und
+  // schluege erst beim Schreiben fehl.
+  const rollenIds = await normalisiereRollenIds(data.roleIds);
+
  await pruefeTeilmenge(handelnderId, [
-    ...(await rechteVonRollen(data.roleIds)),
+    ...(await rechteVonRollen(rollenIds)),
    ...rechteDerHaken(data),
  ]);

@@ -189,7 +195,7 @@ export async function createUser(data: {
      telegramUsername: data.telegramUsername || null,
      signalNumber: data.signalNumber || null,
      roles: {
-        create: data.roleIds.map((roleId) => ({ roleId })),
+        create: rollenIds.map((roleId) => ({ roleId })),
      },
    },
    select: {
@@ -245,6 +251,13 @@ export async function updateUser(
 ) {
  const { roleIds, password, hasDeveloperAccess, hasGdprAccess, hasAuditOpsAccess, ...userData } = data;

+  // Rollen-IDs pruefen, bevor irgendetwas geschrieben oder entschieden wird.
+  // Die Letzter-Admin-Sperre weiter unten arbeitet mit dieser Liste - mit
+  // einer unbekannten ID darin haette sie eine Absicht bestaetigt, die der
+  // Schreibvorgang danach nicht einloesen konnte (Pentest R190-01).
+  const gepruefteRollenIds =
+    roleIds !== undefined ? await normalisiereRollenIds(roleIds) : undefined;
+
  // Teilmengenregel auf dem ZUWACHS, nicht auf dem Endzustand.
  //
  // Wer nur den Nachnamen eines hoeher privilegierten Kollegen korrigiert,
@@ -255,7 +268,7 @@ export async function updateUser(
    const bisher = await effektiveRechte(id);
    const zuwachs = new Set<string>();
    if (roleIds !== undefined) {
-      for (const r of await rechteVonRollen(roleIds)) {
+      for (const r of await rechteVonRollen(gepruefteRollenIds!)) {
        if (!bisher.has(r)) zuwachs.add(r);
      }
    }
@@ -299,7 +312,7 @@ export async function updateUser(
      let willStillBeAdmin = false;
      if (rolesAreBeingChanged) {
        const newRoles = await prisma.role.findMany({
-          where: { id: { in: roleIds } },
+          where: { id: { in: gepruefteRollenIds! } },
          include: {
            permissions: {
              include: { permission: true },
@@ -382,12 +395,21 @@ export async function updateUser(
    },
  });

-  // Update roles if provided
-  if (roleIds) {
-    await prisma.userRole.deleteMany({ where: { userId: id } });
-    await prisma.userRole.createMany({
-      data: roleIds.map((roleId) => ({ userId: id, roleId })),
-    });
+  // Rollentausch in EINER Transaktion.
+  //
+  // Vorher standen deleteMany und createMany nackt nebeneinander: Scheiterte
+  // das Anlegen, war das Loeschen schon passiert und das Konto hatte gar
+  // keine Rolle mehr - bei einer Antwort, die wie "abgelehnt, nichts
+  // geschehen" aussah. `updateRole` hatte diese Haertung laengst; sie war
+  // nur nicht zum Geschwister mitgewandert (Pentest R190-01).
+  if (gepruefteRollenIds !== undefined) {
+    await prisma.$transaction([
+      prisma.userRole.deleteMany({ where: { userId: id } }),
+      prisma.userRole.createMany({
+        data: gepruefteRollenIds.map((roleId) => ({ userId: id, roleId })),
+        skipDuplicates: true,
+      }),
+    ]);
  }

  // Handle developer access
@@ -98,6 +98,42 @@ isolierte Instanz (keine Multi-Tenancy im Code), Provisioning + Abrechnung
 ## ✅ Erledigt


+
+- [x] **🧨 R190-01: Rollentausch zerstörte, wo er ablehnte** (2026-09-09)
+  - Befund der Pentesterin: `updateUser` machte `deleteMany` + `createMany`
+    **ohne Transaktion und ohne `skipDuplicates`**. Eine doppelte roleId
+    (`[4,4]`) oder eine erfundene (`[99999]`) kam durch die Teilmengenregel –
+    sie bringt ja keine Rechte mit – und schlug erst beim Schreiben fehl,
+    **nach** dem Löschen. Antwort: HTTP 400. Konto danach: **keine Rolle**.
+  - Die Antwort log über sich selbst: „400" liest sich als *abgelehnt, nichts
+    passiert*. Zerstört wurde trotzdem.
+  - **Die Letzter-Admin-Sperre wurde damit umgangen**, ohne sie anzugreifen:
+    Sie prüft die *Absicht* (die Admin-Rolle steht im Body → „bleibt Admin"),
+    der Schreibvorgang scheiterte danach. Aussperrung, Rückweg nur per CLI.
+    Auch versehentlich auslösbar durch doppelte IDs aus dem Frontend.
+  - **Es war meine Härtung, die ich nicht mitgenommen habe.** `updateRole`
+    hatte Dedup und `skipDuplicates` seit Etappe 1 – das Geschwister
+    `updateUser`/`createUser` nicht. Genau das Muster, das ich in derselben
+    Runde dreimal angeprangert habe: eine Absicherung, die nur in einer von
+    zwei Kopien ankommt.
+  - Fix: `normalisiereRollenIds()` prüft **vor** jeder Entscheidung und jedem
+    Schreibvorgang gegen die Datenbank (unbekannte ID → 400 mit Klartext),
+    dedupliziert, und der Tausch läuft in `$transaction` mit
+    `skipDuplicates`. Die Letzter-Admin-Sperre arbeitet damit auf einer Liste,
+    die auch einlösbar ist.
+  - **Nebenbefund beim Nachstellen**: Die rohe Prisma-Fehlermeldung ging
+    wortwörtlich an den Client – inklusive Dateipfad des Servers. Wird jetzt
+    protokolliert statt ausgeliefert.
+  - Nachgestellt und gegengeprüft: `[22,22]` → 200, Rolle erhalten;
+    `[99999]` und `[22,99999]` → 400, Rollen **unverändert**; `[-1]` → 400;
+    `[22,23]` → 200 korrekt gesetzt. `createUser` ebenso, abgelehnte Anlagen
+    hinterlassen kein Konto.
+  - Bestätigt dicht gemeldet: Restore/Factory-Reset ohne `roles:manage` → 403,
+    roleIds+Haken gleichzeitig → 403 ohne Teil-Write, alle vier Vergabewege →
+    403, isSystem-Sperre inkl. `" aDmIn "` → 403.
+  - Dateien: `src/services/rechte.service.ts`, `src/services/user.service.ts`,
+    `src/controllers/user.controller.ts`
+
 - [x] **🔑 Rechtemodell Etappe 1: Katalog begradigt, Selbst-Erhöhung geschlossen** (2026-09-04)
  - Vorarbeit für die Rollen-Oberfläche (Etappe 2). Eine Checkbox-Liste über
    einem Katalog, der nicht stimmt, wäre schlimmer als gar keine.