diff --git a/backend/src/controllers/user.controller.ts b/backend/src/controllers/user.controller.ts index a7203106..c9732fb0 100644 --- a/backend/src/controllers/user.controller.ts +++ b/backend/src/controllers/user.controller.ts @@ -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, diff --git a/backend/src/services/rechte.service.ts b/backend/src/services/rechte.service.ts index 5955f331..affaf337 100644 --- a/backend/src/services/rechte.service.ts +++ b/backend/src/services/rechte.service.ts @@ -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 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 { + 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; +} diff --git a/backend/src/services/user.service.ts b/backend/src/services/user.service.ts index 5d0bd24c..78d72e95 100644 --- a/backend/src/services/user.service.ts +++ b/backend/src/services/user.service.ts @@ -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(); 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 diff --git a/docs/todo.md b/docs/todo.md index 5ce86074..77b56b11 100644 --- a/docs/todo.md +++ b/docs/todo.md @@ -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.