From 3a9391d9c8201b98894f935fcaa9f3e3d2f636e6 Mon Sep 17 00:00:00 2001 From: Schalli Date: Thu, 10 Sep 2026 10:35:43 +0200 Subject: [PATCH] feat(quick-260910-das): Steuerungsschicht binden, Selbstloesch-Riegel schliessen - user.controller.ts: alle sieben Zugriffe binden. ADMIN-Zweig der Benutzerliste laeuft ueber forTenant() mit weiterhin bestehender Mandantenbedingung im where; SUPER_ADMIN-Zweig ueber die neue UserService.findAllForPlatformAdmin(). Die drei Wege ueber die Kennung loesen den Zielbenutzer rollenabhaengig ueber resolveTargetUser() auf (ADMIN gebunden an eigenen Mandanten, SUPER_ADMIN uebergreifend); der Schreibzugriff bei update/delete bindet an den Mandanten des Zielbenutzers, nicht des Aufrufers, damit die uebergreifende Verwaltung durch die oberste Rolle erhalten bleibt - Selbstloesch-Riegel (Befund H) repariert: verglich bisher gegen currentUser.sub, ein Feld, das der Sitzungsnachweis nicht traegt -- der Riegel griff nie. Jetzt gegen currentUser.id. Verhaltensaenderung: ein Administrator kann sein eigenes Konto nun nicht mehr loeschen - Alle fuenf Selbstbedienungszugriffe (Bild hochladen/loeschen/ ausliefern, Akzentfarbe) binden an die Mandantenkennung aus dem Sitzungsnachweis - user.controller.spec.ts (neu): Zwei-Klienten-Nachweis fuer die vorher testlose Steuerungsschicht, 8 Testfaelle, Falsifizierungsnachweis fuer eine gebundene Stelle sowie Rot-vor-Reparatur-Nachweis fuer den Selbstloesch-Riegel (siehe SUMMARY) - docs/mandantentrennung-zugriffsklassifikation.md: alle vier handgepflegten Stellen nachgezogen (Uebersichtszeile 8/14, Summenzeile 118/124, Klassen-Verteilung 63 Paare, Hintergrunddienst-Abschnitt auf fuenf Faelle inkl. admin-seed.service.ts als erster beidseitig korrekter Fall) sowie zwei Klassenkorrekturen (user.service.ts/user und admin-seed.service.ts/user je auf "beides") - docs/mandantentrennung-etappe2-fehlerrichtung.md: Nachtrag zum user-Abschnitt mit den tatsaechlich umgesetzten Pfaden, der geschlossenen Luecke und den Falsifizierungsnachweisen - 810 Tests gruen (8 neue in user.controller.spec.ts), Typpruefung sauber, Wegwerf-Werkzeug meldet weiterhin alle 53 Pruefungen bestanden Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01AMASaSxv5QMY7RncqZriRR --- apps/api/src/user/user.controller.spec.ts | 280 ++++++++++++++++++ apps/api/src/user/user.controller.ts | 108 ++++--- ...andantentrennung-etappe2-fehlerrichtung.md | 66 +++++ ...andantentrennung-zugriffsklassifikation.md | 68 +++-- 4 files changed, 468 insertions(+), 54 deletions(-) create mode 100644 apps/api/src/user/user.controller.spec.ts diff --git a/apps/api/src/user/user.controller.spec.ts b/apps/api/src/user/user.controller.spec.ts new file mode 100644 index 0000000..c709c1a --- /dev/null +++ b/apps/api/src/user/user.controller.spec.ts @@ -0,0 +1,280 @@ +import { ForbiddenException, NotFoundException } from '@nestjs/common'; +import { Role } from '@prisma/client'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { UserController } from './user.controller'; + +/** + * UserController — Zwei-Klienten-Nachweis (260910-das, Aufgabe 3, Befund + * C, dritte Form). + * + * Diese Steuerungsschicht hatte bisher KEINE Testdatei: sie kann auf gar + * keinen Fehler rot werden, obwohl in ihr sieben der siebzehn Zugriffe des + * Bereichs UND die gesamte Rollenlogik liegen, die entscheidet, wer wessen + * Benutzer sehen darf. `UserService` wird hier als Attrappe gestellt — sein + * Bindungsverhalten ist bereits in Aufgabe 2 geprueft (`user.service.spec.ts`). + * Der direkte Prisma-Zugriff dieses Controllers (findAll-ADMIN-Zweig, die + * vier Selbstbedienungswege) bekommt denselben Zwei-Klienten-Nachweis wie + * in `groups.service.spec.ts`. + */ +vi.mock('../prisma/prisma-tenant.extension', () => ({ + forTenant: vi.fn((prisma: any, tenantId: string) => prisma.__makeBoundClient(tenantId)), +})); + +vi.mock('fs', () => ({ + mkdirSync: vi.fn(), + writeFileSync: vi.fn(), + existsSync: vi.fn(() => true), + unlinkSync: vi.fn(), + readFileSync: vi.fn(() => Buffer.from('fake-image-bytes')), +})); + +function makeFakePrisma() { + const users = new Map(); + const boundCallLog: { tenantId: string; model: string; method: string }[] = []; + + function projectSelect(row: any, select: any) { + const projected: any = {}; + for (const key of Object.keys(select)) { + if (select[key]) projected[key] = row[key]; + } + return projected; + } + + function scopedFindMany(tenantId: string, args: any) { + let rows = Array.from(users.values()).filter((u) => u.tenantId === tenantId); + rows = [...rows].sort((a, b) => a.username.localeCompare(b.username)); + return args?.select ? rows.map((r) => projectSelect(r, args.select)) : rows; + } + + function scopedFindUnique(tenantId: string, args: any) { + const row = users.get(args.where.id); + if (!row || row.tenantId !== tenantId) return null; + return args.select ? projectSelect(row, args.select) : row; + } + + function scopedUpdate(tenantId: string, args: any) { + const row = users.get(args.where.id); + if (!row || row.tenantId !== tenantId) { + const err: any = new Error('Record to update not found'); + err.code = 'P2025'; + throw err; + } + const updated = { ...row, ...args.data }; + users.set(args.where.id, updated); + return updated; + } + + const fake: any = { + __seedUser(user: any) { + users.set(user.id, user); + }, + __boundCallLog: boundCallLog, + __makeBoundClient(tenantId: string) { + return { + __isBoundClient: true, + __tenantId: tenantId, + user: { + findMany: async (args: any) => { + boundCallLog.push({ tenantId, model: 'user', method: 'findMany' }); + return scopedFindMany(tenantId, args); + }, + findUnique: async (args: any) => { + boundCallLog.push({ tenantId, model: 'user', method: 'findUnique' }); + return scopedFindUnique(tenantId, args); + }, + update: async (args: any) => { + boundCallLog.push({ tenantId, model: 'user', method: 'update' }); + return scopedUpdate(tenantId, args); + }, + }, + }; + }, + }; + + return fake; +} + +function expectBoundCall(prisma: any, tenantId: string, model: string, method: string) { + const found = prisma.__boundCallLog.some( + (c: any) => c.tenantId === tenantId && c.model === model && c.method === method, + ); + expect( + found, + `erwarteter gebundener Aufruf ${model}.${method}(tenant=${tenantId}) fehlt im Protokoll: ${JSON.stringify(prisma.__boundCallLog)}`, + ).toBe(true); +} + +function makeUserServiceMock() { + return { + findById: vi.fn(), + findByIdForPlatformAdmin: vi.fn(), + findAllForPlatformAdmin: vi.fn(), + findByUsername: vi.fn(), + create: vi.fn(), + update: vi.fn(), + deactivate: vi.fn(), + delete: vi.fn(), + }; +} + +describe('UserController', () => { + let prisma: any; + let userService: any; + let controller: UserController; + + beforeEach(() => { + prisma = makeFakePrisma(); + userService = makeUserServiceMock(); + controller = new UserController(userService as any, prisma); + }); + + describe('findAll', () => { + it('Test 1: die Benutzerliste eines Mandanten-Administrators steht gebunden im Protokoll, trägt die Mandantenkennung aus dem Sitzungsnachweis, und liefert keine Benutzer eines zweiten Mandanten', async () => { + prisma.__seedUser({ + id: 'u-a', + username: 'alice', + tenantId: 't1', + email: 'alice@x.invalid', + displayName: null, + role: 'USER', + isActive: true, + createdAt: new Date(), + lastLoginAt: null, + }); + prisma.__seedUser({ + id: 'u-b', + username: 'bob', + tenantId: 't2', + email: 'bob@x.invalid', + displayName: null, + role: 'USER', + isActive: true, + createdAt: new Date(), + lastLoginAt: null, + }); + + const result = await controller.findAll({ role: Role.ADMIN, tenantId: 't1', id: 'admin1' }); + + expect(result.map((u: any) => u.username)).toEqual(['alice']); + expectBoundCall(prisma, 't1', 'user', 'findMany'); + }); + + it('Test 2: die Benutzerliste des Plattform-Administrators geht über die neue, übergreifende Methode des Dienstes und liefert weiterhin die Benutzer aller Mandanten in der bisherigen Sortierung — die Rollenverzweigung bleibt erhalten', async () => { + const expected = [ + { id: 'u-a', username: 'alice', tenantId: 't1' }, + { id: 'u-b', username: 'bob', tenantId: 't2' }, + ]; + userService.findAllForPlatformAdmin.mockResolvedValue(expected); + + const result = await controller.findAll({ + role: Role.SUPER_ADMIN, + tenantId: 't1', + id: 'super1', + }); + + expect(result).toBe(expected); + expect(userService.findAllForPlatformAdmin).toHaveBeenCalledTimes(1); + expect(prisma.__boundCallLog).toHaveLength(0); + }); + }); + + describe('Zielbenutzer-Auflösung (findOne/update/remove)', () => { + it('Test 3: die drei Wege über die Kennung lösen den Zielbenutzer rollenabhängig auf — für einen Mandanten-Administrator gebunden an dessen eigenen Mandanten, für den Plattform-Administrator über die übergreifende Methode', async () => { + const targetUser = { id: 'u-x', username: 'x', tenantId: 't1' }; + + userService.findById.mockResolvedValue(targetUser); + await controller.findOne('u-x', { role: Role.ADMIN, tenantId: 't1', id: 'admin1' }); + expect(userService.findById).toHaveBeenCalledWith('t1', 'u-x'); + + userService.findByIdForPlatformAdmin.mockResolvedValue(targetUser); + await controller.findOne('u-x', { role: Role.SUPER_ADMIN, tenantId: 't2', id: 'super1' }); + expect(userService.findByIdForPlatformAdmin).toHaveBeenCalledWith('u-x'); + }); + + it('Test 4: ein Mandanten-Administrator, der einen Benutzer eines fremden Mandanten über dessen Kennung anspricht, bekommt weiterhin eine Ablehnung — die gebundene Auflösung liefert bereits null, die Ablehnung ist eine Nicht-gefunden-Ausnahme, keine Verbotene-Ausnahme (die vorgeschaltete, ausdrückliche Mandantenprüfung bleibt trotzdem als zweite Schicht bestehen)', async () => { + userService.findById.mockResolvedValue(null); + + await expect( + controller.findOne('u-foreign', { role: Role.ADMIN, tenantId: 't1', id: 'admin1' }), + ).rejects.toBeInstanceOf(NotFoundException); + }); + }); + + describe('create', () => { + it('Test 5: ein Mandanten-Administrator kann weiterhin keine oberste Rolle vergeben, und die Anlage eines Benutzers landet weiterhin im Mandanten des Aufrufers, wenn dieser nicht die oberste Rolle trägt', async () => { + const currentUser = { role: Role.ADMIN, tenantId: 't1', id: 'admin1' }; + + await expect( + controller.create( + { username: 'x', email: 'x@x.invalid', password: '12345678', role: Role.SUPER_ADMIN } as any, + currentUser, + ), + ).rejects.toBeInstanceOf(ForbiddenException); + + userService.create.mockResolvedValue({ id: 'u-new', tenantId: 't1' }); + await controller.create( + { username: 'y', email: 'y@y.invalid', password: '12345678', tenantId: 't9' } as any, + currentUser, + ); + expect(userService.create).toHaveBeenCalledWith( + expect.objectContaining({ tenantId: 't1' }), + ); + }); + }); + + describe('remove — Selbstlöschriegel (Befund H)', () => { + it('Test 6: der Riegel gegen das Löschen des eigenen Kontos greift', async () => { + const currentUser = { role: Role.ADMIN, tenantId: 't1', id: 'admin1' }; + userService.findById.mockResolvedValue({ id: 'admin1', tenantId: 't1' }); + + await expect(controller.remove('admin1', currentUser)).rejects.toBeInstanceOf( + ForbiddenException, + ); + expect(userService.delete).not.toHaveBeenCalled(); + }); + }); + + describe('Selbstbedienungswege (Befund G)', () => { + const currentUser = { role: Role.USER, tenantId: 't1', id: 'me' }; + + beforeEach(() => { + prisma.__seedUser({ + id: 'me', + username: 'me', + tenantId: 't1', + avatarPath: 'user-files/avatars/me.png', + }); + }); + + it('Test 7: alle fünf Zugriffe der vier Selbstbedienungswege stehen gebunden im Protokoll, mit der Mandantenkennung aus dem Sitzungsnachweis', async () => { + await controller.uploadAvatar({ buffer: Buffer.from('x'), mimetype: 'image/png' }, currentUser); + await controller.deleteAvatar(currentUser); + await controller.updateAccentColor({ color: '#ff00aa' }, currentUser); + + prisma.__seedUser({ + id: 'me', + username: 'me', + tenantId: 't1', + avatarPath: 'user-files/avatars/me.png', + }); + const res = { setHeader: vi.fn(), send: vi.fn() }; + await controller.getAvatar(currentUser, res as any); + + const ownUserCalls = prisma.__boundCallLog.filter( + (c: any) => c.tenantId === 't1' && c.model === 'user', + ); + // uploadAvatar (1 update) + deleteAvatar (1 findUnique + 1 update) + + // updateAccentColor (1 update) + getAvatar (1 findUnique) = 5. + expect(ownUserCalls.length).toBe(5); + }); + + it('Test 8: der Weg, der ein Bild ausliefert, liefert für einen Benutzer ohne hinterlegtes Bild weiterhin die vorhandene Nicht-gefunden-Ausnahme und ändert sein Verhalten nicht', async () => { + prisma.__seedUser({ id: 'me', username: 'me', tenantId: 't1', avatarPath: null }); + const res = { setHeader: vi.fn(), send: vi.fn() }; + + await expect(controller.getAvatar(currentUser, res as any)).rejects.toBeInstanceOf( + NotFoundException, + ); + }); + }); +}); diff --git a/apps/api/src/user/user.controller.ts b/apps/api/src/user/user.controller.ts index 59c527f..96d62b5 100644 --- a/apps/api/src/user/user.controller.ts +++ b/apps/api/src/user/user.controller.ts @@ -22,6 +22,7 @@ import { Response } from 'express'; import { CurrentUser } from '../auth/decorators/current-user.decorator'; import { Roles } from '../auth/decorators/roles.decorator'; import { RolesGuard } from '../auth/guards/roles.guard'; +import { forTenant } from '../prisma/prisma-tenant.extension'; import { PrismaService } from '../prisma/prisma.service'; import { CreateUserDto } from './dto/create-user.dto'; import { UpdateUserDto } from './dto/update-user.dto'; @@ -40,6 +41,13 @@ function resolveAvatarsDir(): string { return path.resolve(__dirname, '..', '..', '..', '..', 'user-files', 'avatars'); } +/** + * Bindung an forTenant() (WINDOWS #20 Etappe 2, 260910-das, Aufgabe 3): alle + * sieben Zugriffe dieses Controllers laufen entweder direkt ueber einen + * gebundenen Klienten (`tenantPrisma`, Konvention aus `ldap`, `groups`, + * `dkv`, `auth`, `user.service.ts`) oder ueber die uebergreifenden Methoden + * von `UserService`, deren Rumpf je Mandant gebunden ist. + */ @Controller('users') @UseGuards(RolesGuard) export class UserController { @@ -48,6 +56,22 @@ export class UserController { private readonly prisma: PrismaService, ) {} + /** + * Loest den Zielbenutzer rollenabhaengig auf (260910-das, Aufgabe 3): ein + * Mandanten-Administrator sieht nur den eigenen Mandanten (gebunden ueber + * `UserService.findById`), die oberste Rolle (SUPER_ADMIN) behaelt die + * uebergreifende Sicht ueber `UserService.findByIdForPlatformAdmin()` -- + * diese Verzweigung ist die Stelle, an der dieser Bereich die gewollte + * uebergreifende Sicht von der mandantengebundenen unterscheidet, und sie + * darf nicht eingeebnet werden. + */ + private async resolveTargetUser(currentUser: any, id: string) { + if (currentUser.role === Role.SUPER_ADMIN) { + return this.userService.findByIdForPlatformAdmin(id); + } + return this.userService.findById(currentUser.tenantId, id); + } + /** * GET /users * ADMIN sees own-tenant users only. SUPER_ADMIN sees all users. @@ -57,24 +81,19 @@ export class UserController { @Roles(Role.ADMIN, Role.SUPER_ADMIN) async findAll(@CurrentUser() currentUser: any) { if (currentUser.role === Role.SUPER_ADMIN) { - return this.prisma.user.findMany({ - select: { - id: true, - username: true, - email: true, - displayName: true, - role: true, - isActive: true, - tenantId: true, - createdAt: true, - lastLoginAt: true, - }, - orderBy: { username: 'asc' }, - }); + // Plattform-Administratorsicht (Befund F): die bestehende, gewollte + // Funktion der obersten Rolle bleibt erhalten, laeuft aber ueber die + // Schleife-je-Mandant-gebunden aus UserService.findAllForPlatformAdmin() + // statt ueber ein ungebundenes findMany(). + return this.userService.findAllForPlatformAdmin(); } - // ADMIN: filter by own tenant - return this.prisma.user.findMany({ + // ADMIN: gebunden an den eigenen Mandanten. Die vorhandene + // Mandantenbedingung im where BLEIBT erhalten -- nicht entfernen mit + // dem Argument, das mache jetzt die Datenbank; dieselbe Regel, die die + // Bereiche `tenders` und `dkv` aufgestellt haben. + const tenantPrisma = forTenant(this.prisma, currentUser.tenantId) as any; + return tenantPrisma.user.findMany({ where: { tenantId: currentUser.tenantId }, select: { id: true, @@ -97,13 +116,7 @@ export class UserController { @Get(':id') @Roles(Role.ADMIN, Role.SUPER_ADMIN) async findOne(@Param('id') id: string, @CurrentUser() currentUser: any) { - // 260910-das, Aufgabe 2: UserService.findById() bekommt einen - // Pflicht-Mandanten (siehe user.service.ts). Nur die Signatur wird hier - // nachgezogen, damit die Typpruefung sauber bleibt -- die Rollenlogik - // (insbesondere die uebergreifende SUPER_ADMIN-Sicht ueber - // findByIdForPlatformAdmin()) wird erst in Aufgabe 3 vollstaendig - // verdrahtet. - const user = await this.userService.findById(currentUser.tenantId, id); + const user = await this.resolveTargetUser(currentUser, id); if (!user) { throw new NotFoundException('User not found'); } @@ -162,8 +175,7 @@ export class UserController { @Body() dto: UpdateUserDto, @CurrentUser() currentUser: any, ) { - // 260910-das, Aufgabe 2: siehe Kommentar in findOne() oben. - const user = await this.userService.findById(currentUser.tenantId, id); + const user = await this.resolveTargetUser(currentUser, id); if (!user) { throw new NotFoundException('User not found'); } @@ -181,7 +193,12 @@ export class UserController { throw new ForbiddenException('Cannot assign SUPER_ADMIN role'); } - const updated = await this.userService.update(currentUser.tenantId, id, { + // Der Schreibzugriff bindet an den Mandanten des ZIELBENUTZERS, wie er + // aus der vorangegangenen Aufloesung hervorgeht — NICHT an den des + // Aufrufers (260910-das, Aufgabe 3). Nur so bleibt die uebergreifende + // Verwaltung durch die oberste Rolle erhalten und ist der + // Schreibzugriff trotzdem gebunden. + const updated = await this.userService.update(user.tenantId, id, { username: dto.username, email: dto.email, password: dto.password, @@ -201,14 +218,21 @@ export class UserController { @Delete(':id') @Roles(Role.ADMIN, Role.SUPER_ADMIN) async remove(@Param('id') id: string, @CurrentUser() currentUser: any) { - // 260910-das, Aufgabe 2: siehe Kommentar in findOne() oben. - const user = await this.userService.findById(currentUser.tenantId, id); + const user = await this.resolveTargetUser(currentUser, id); if (!user) { throw new NotFoundException('User not found'); } - // Cannot delete self - if (user.id === currentUser.sub) { + // Cannot delete self (260910-das, Befund H): dieser Vergleich verglich + // bisher gegen `currentUser.sub` — ein Feld, das der Sitzungsnachweis + // GAR NICHT traegt (JwtStrategy.validate() liefert exakt { id, + // username, role, tenantId }). Der Riegel hat deshalb NIE gegriffen: + // ein Administrator konnte sich selbst loeschen und seinen Mandanten + // ohne Verwaltung zuruecklassen. Die Reparatur ist eine + // Verhaltensaenderung: ein Administrator kann sein eigenes Konto nun + // nicht mehr loeschen — das ist die urspruengliche, im Code bereits + // formulierte Absicht. + if (user.id === currentUser.id) { throw new ForbiddenException('Cannot delete your own account'); } @@ -220,13 +244,21 @@ export class UserController { throw new ForbiddenException('Cannot delete users from other tenants'); } - await this.userService.delete(currentUser.tenantId, id); + // Gebunden an den Mandanten des ZIELBENUTZERS, derselbe Grund wie bei + // update() oben. + await this.userService.delete(user.tenantId, id); return { message: 'User deleted' }; } // ─── Self-service avatar endpoints (all authenticated roles) ─────────────── // No @Roles() → RolesGuard.canActivate() returns true when requiredRoles is // empty (see guards/roles.guard.ts). Global JwtAuthGuard still enforces auth. + // + // Alle fuenf Zugriffe binden vollstaendig an die Mandantenkennung aus dem + // Sitzungsnachweis (260910-das, Befund G, Aufgabe 3): der angemeldete + // Benutzer liegt per Definition im Mandanten seiner eigenen Sitzung, die + // Bindung aendert an Pfaden, Dateityp-Pruefung, Groessenbegrenzung und dem + // Aufraeumen alter Bilddateien nichts. /** * POST /users/me/avatar @@ -272,7 +304,8 @@ export class UserController { // Persist relative path (relative to monorepo root) const relativePath = path.join('user-files', 'avatars', filename); - await this.prisma.user.update({ + const tenantPrisma = forTenant(this.prisma, currentUser.tenantId) as any; + await tenantPrisma.user.update({ where: { id: currentUser.id }, data: { avatarPath: relativePath }, }); @@ -286,7 +319,8 @@ export class UserController { */ @Delete('me/avatar') async deleteAvatar(@CurrentUser() currentUser: any) { - const user = await this.prisma.user.findUnique({ + const tenantPrisma = forTenant(this.prisma, currentUser.tenantId) as any; + const user = await tenantPrisma.user.findUnique({ where: { id: currentUser.id }, select: { avatarPath: true }, }); @@ -297,7 +331,7 @@ export class UserController { if (fs.existsSync(absolutePath)) { fs.unlinkSync(absolutePath); } - await this.prisma.user.update({ + await tenantPrisma.user.update({ where: { id: currentUser.id }, data: { avatarPath: null }, }); @@ -320,7 +354,8 @@ export class UserController { throw new BadRequestException('Invalid color format. Use hex (#rrggbb).'); } - await this.prisma.user.update({ + const tenantPrisma = forTenant(this.prisma, currentUser.tenantId) as any; + await tenantPrisma.user.update({ where: { id: currentUser.id }, data: { accentColor: body.color ?? null }, }); @@ -338,7 +373,8 @@ export class UserController { @CurrentUser() currentUser: any, @Res() res: Response, ) { - const user = await this.prisma.user.findUnique({ + const tenantPrisma = forTenant(this.prisma, currentUser.tenantId) as any; + const user = await tenantPrisma.user.findUnique({ where: { id: currentUser.id }, select: { avatarPath: true }, }); diff --git a/docs/mandantentrennung-etappe2-fehlerrichtung.md b/docs/mandantentrennung-etappe2-fehlerrichtung.md index 04e2e34..06c1af7 100644 --- a/docs/mandantentrennung-etappe2-fehlerrichtung.md +++ b/docs/mandantentrennung-etappe2-fehlerrichtung.md @@ -1182,6 +1182,72 @@ entscheidet sie nicht. Er bindet dienst-intern, wie `ldap`, `groups`, Jeweils mit der Feststellung, dass sie geprüft und bewusst gelassen sind — nicht übersehen. +**Nachtrag (260910-das, Aufgabe 3).** Wie in den vorherigen Durchläufen wird +der Text oben NICHT umgeschrieben — er beschreibt korrekt den Stand zum +Zeitpunkt der Messung (Aufgabe 1); dieser Nachtrag hält fest, was Aufgabe 2/3 +tatsächlich umgesetzt haben. + +*Tatsächlich umgesetzte Pfade gegen die angekündigten gehalten:* alle in (u2) +genannten Pfade sind wie beschrieben umgestellt. `UserService.findById`, +`create`, `update`, `deactivate`, `delete` laufen über `forTenant()`; +`create`/`update` übersetzen die plattformweite Eindeutigkeitsverletzung in +eine deutsche Konfliktmeldung, die weder Halter noch Mandant nennt. +`AdminSeedService.seedAdmin()` bindet die Erstanlage des Administrators an +den unmittelbar zuvor angelegten Mandanten und entschärft die Startsperre +(P2002 wird wie „Administrator existiert bereits" behandelt, jeder andere +Fehler bricht weiterhin ab). `UserController` bindet alle sieben eigenen +Zugriffe (ADMIN-Zweig der Benutzerliste, alle fünf Selbstbedienungswege) und +löst die drei Wege über die Kennung rollenabhängig auf. `findByUsername` +bleibt wie angekündigt ungebunden, mit richtiggestelltem Kopfkommentar. + +*Die geschlossene Lücke im Selbstlösch-Riegel (Befund H):* der Vergleich +`user.id === currentUser.sub` griff nie, weil der Sitzungsnachweis kein Feld +`sub` trägt (`JwtStrategy.validate()` liefert exakt `{ id, username, role, +tenantId }`). Der Nachweis kommt aus der Reihenfolge selbst, nicht aus einer +Behauptung: `user.controller.spec.ts`, Test 6, wurde zuerst gegen die alte +Fassung ausgeführt — die Zeile `if (user.id === currentUser.sub)` wurde +probeweise wiederhergestellt, der Testlauf zeigte den erwarteten roten Test +(„promise resolved … instead of rejecting"), danach wurde auf +`currentUser.id` repariert und derselbe Testlauf grün. Die Wirkung ist eine +Verhaltensänderung: ein Administrator kann sein eigenes Konto seither nicht +mehr löschen — das ist die ursprüngliche, im Code bereits formulierte +Absicht, nicht neu erfunden. + +*Die gewählte Form der Plattform-Administratorsicht, samt Beleg aus der +Messung:* wie in (u3)/Befund F angekündigt, laufen +`UserService.findAllForPlatformAdmin()` und `findByIdForPlatformAdmin()` als +Schleife über alle Mandanten (`this.prisma.tenant.findMany`, ungebunden, weil +`Tenant` keinen Zeilenschutz trägt) mit je EINEM gebundenen Lesezugriff im +Rumpf — dieselbe Form wie `AdminSeedService.ensureDefaultGroupsForAllTenants()`. +Der Beleg, dass diese Form die heutige Sicht erhält statt sie zu mindern, +kommt aus Aufgabe 1: `user-fan-out-je-mandant-gebunden-liefert-alle-zeilen` +maß die Vereinigung der je-Mandant gebundenen `SELECT`s gegen eine über die +Wartungsrolle (mit `BYPASSRLS`) gemessene Gesamtmenge — beide Mengen waren +identisch (`["alice","bob","carol","dave"]`). In `user.service.spec.ts` +bestätigen Test 6 und Test 7 dasselbe am Code: je Mandant genau EIN +Protokolleintrag, die Sortierung nach Benutzername bleibt über die +zusammengeführten Teilmengen hinweg korrekt, und eine Kennungsauflösung für +einen Benutzer eines fremden Mandanten gelingt nachweislich über einen +gebundenen Lesezugriff. + +*Falsifizierungsnachweise (Aufgabe 2 und 3), je einmal durchgeführt und +zurückgenommen:* in Aufgabe 2 wurde `UserService.findById` probeweise auf +den ungebundenen Klienten zurückgebaut — genau `user.service.spec.ts`, Test +4, wurde rot, mit der Meldung „erwarteter gebundener Aufruf +user.findUnique(tenant=t1) fehlt im Protokoll: []"; der Rückbau wurde +zurückgenommen, derselbe Testlauf danach wieder grün. In derselben Aufgabe +wurde zusätzlich `AdminSeedService.seedAdmin()`s gebundener +`tenantPrisma.user.create`-Aufruf probeweise auf den ungebundenen Basisclient +zurückgebaut — sechs Tests wurden rot (u. a. Test 9–12), alle mit der +Meldung „tenantPrisma.user.create is not a function", weil der ungebundene +Basisclient in der Testattrappe keine `create`-Methode auf `user` trägt; der +Rückbau wurde zurückgenommen, alle zehn Tests danach wieder grün. In Aufgabe +3 wurde `UserController.uploadAvatar`s gebundener Schreibzugriff probeweise +auf den ungebundenen Basisclient zurückgebaut — genau +`user.controller.spec.ts`, Test 7, wurde rot, mit der Meldung „Cannot read +properties of undefined (reading 'update')"; der Rückbau wurde +zurückgenommen, derselbe Testlauf danach wieder grün. + ## Verweis Die Bestandsaufnahme, welche Fundstelle den hier beschriebenen Übergang diff --git a/docs/mandantentrennung-zugriffsklassifikation.md b/docs/mandantentrennung-zugriffsklassifikation.md index 3a0fa01..94ad679 100644 --- a/docs/mandantentrennung-zugriffsklassifikation.md +++ b/docs/mandantentrennung-zugriffsklassifikation.md @@ -100,7 +100,7 @@ autoritative Quelle. | groups | 0 | 31 | **war 37/0** — Aufgabe 2/3 (260909-jts) haben `groups.service.ts` (12 Methoden) und `module-grants.service.ts` (5 Methoden) vollständig auf `forTenant()`/`withTenantTransaction()` umgestellt. Die neun zusätzlichen, über `tx` gebundenen Zugriffe innerhalb der drei Transaktionen zählt dieses einfache Muster nicht mit (siehe Methodenhinweis oben) | | ldap | 4 | 26 | **war 21/0** — Aufgabe 2/3 (260909-ipc) haben `ldap-config.service.ts` (5 Methoden) und `ldap.service.ts` (6 Methoden, 11 Abfragen) auf `forTenant()` umgestellt. Die 4 verbleibenden ungebundenen Treffer sind bewusst: `getAllActiveConfigs`/`onApplicationBootstrap` (Befund B) und `resolveEmailForWrite` (Befund A, T-IPC-04) | | dkv | 1 | 22 | **war 21/0** — Aufgabe 2/3 (260909-mir) haben `dkv.service.ts` vollständig auf `forTenant()` umgestellt: Konfigurationspfade (`loadConfig`, `getConfigForApi`, `saveConfig`, `testConnection`), Historie, Fahrzeugstammdaten und der neue Besitzriegel vor dem Ausfuhrdatei-Download. Gebunden sind es 22 statt 21, weil der Riegel einen zusätzlichen Lesezugriff auf `dkvInvoiceHistory` einführt (T-MIR-03). Der eine verbleibende ungebundene Treffer ist der benannte Planer-Startpfad `loadAnyActiveConfigForScheduler()` (Befund D, WINDOWS #21) — bewusst, mit dreifacher Markierung | -| user | 17 | 0 | unverändert | +| user | 8 | 14 | **war 17/0** — Aufgabe 2/3 (260910-das) haben `user.service.ts` (`findById`/`create`/`update`/`deactivate`/`delete` sowie die zwei neuen Plattform-Administratorsicht-Methoden), `admin-seed.service.ts` (Erstanlage des Administrators) und `user.controller.ts` (Benutzerliste des ADMIN-Zweigs, alle drei Kennungswege ueber die Dienstmethoden, alle fuenf Selbstbedienungszugriffe) auf `forTenant()` umgestellt. Die 8 verbleibenden ungebundenen Rohtreffer sind bewusst: `findByUsername` in `user.service.ts` (plattformweit eindeutiger Schluessel, derselbe Fall wie `resolveEmailForWrite` im Bereich `ldap`), die Erstanlage-Pruefung und beide Zugriffe auf `tenant` in `admin-seed.service.ts`, sowie der neue Schleifentreiber `this.prisma.tenant.findMany` der beiden Plattform-Administratorsicht-Methoden in `user.service.ts` (`Tenant` traegt keinen Zeilenschutz) | | module-registry | 17 | 0 | unverändert | | dashboard | 13 | 0 | unverändert | | auth | 8 | 5 | unverändert gegenüber dem in Etappe 1 (260909-eor) gemessenen Stand | @@ -108,9 +108,9 @@ autoritative Quelle. | tenant | 8 | 0 | unverändert | | favorites | 7 | 0 | unverändert | | settings | 4 | 0 | unverändert | -| **Summe** | **127** | **110** | Ungebunden: war 147 nach 260909-laa, Delta = die 20 in Aufgabe 2/3 (260909-mir) umgestellten `dkv`-Rohtreffer. Gebunden: war 88, jetzt zusätzlich 22 in `dkv` (20 umgestellte plus 2 neue Zugriffe des Besitzriegels). Quergemessen beim Abschluss von 260909-mir: ein roher `grep` über `apps/api/src` zählt 126 statt 127 ungebundene Treffer — die Differenz stammt aus einer geringfügig anderen Ausschlussregel für Testdateien, nicht aus einer offenen Fundstelle. Diese Übersicht ist eine Buchführungshilfe; **autoritativ ist die Fundstellentabelle unten**, die `rls-access-inventory.spec.ts` bei jedem Lauf gegen den Quelltext prüft | +| **Summe** | **118** | **124** | Ungebunden: war 127 nach 260909-mir, Delta = die 9 in Aufgabe 2/3 (260910-das) gesunkenen `user`-Rohtreffer (17→8). Gebunden: war 110, jetzt zusätzlich 14 in `user`. Diese Übersicht ist eine Buchführungshilfe; **autoritativ ist die Fundstellentabelle unten**, die `rls-access-inventory.spec.ts` bei jedem Lauf gegen den Quelltext prüft | -## Klassen-Verteilung (nach (Datei, Modell)-Fundstellen, 62 Paare) +## Klassen-Verteilung (nach (Datei, Modell)-Fundstellen, 63 Paare) Stand 260909-jts (Aufgabe 3): 61 Paare aus dem vorherigen Durchlauf (260909-ipc) plus ein bisher vollstaendig unsichtbares Paar @@ -131,21 +131,37 @@ hinzugekommen oder verschwunden — nur EINE Klasse hat sich verschoben: `listForUser`/`createPlatform`/`remove` bleiben bewusst uebergreifend), der exakte Praezedenzfall aus `ldapConfig` in 260909-ipc. +**Stand 260910-das (Aufgabe 3):** 63 Paare — 62 aus dem vorherigen +Durchlauf plus EIN neues Paar (`user.service.ts`/`tenant`, Klasse +`keine-mandantengebundene-tabelle`, Stand `ungebunden`): der Schleifentreiber +der neu eingefuehrten Plattform-Administratorsicht (Befund F/N, Aufgabe 1/2). +Zusaetzlich ZWEI Klassenkorrekturen, jede mit eigener Begruendung an der +Fundstelle in der Bestandsaufnahme oben: `user.service.ts`/`user` wechselt +von `muss-mandantengebunden` auf `beides` — wortgleich derselbe +Praezedenzfall wie `ldap.service.ts`/`user` in 260909-ipc +(`resolveEmailForWrite`, hier `findByUsername`); `admin-seed.service.ts`/`user` +wechselt von `bewusst-uebergreifend` auf `beides`, weil die bisherige +Begruendung nachweislich falsch war (Befund J: der Mandant ist bei der +Erstanlage des Administrators bereits bekannt, nicht strukturell fehlend). +Alle drei Zahlen sind der Ausgabe von `rls-access-inventory.spec.ts` +entnommen, nicht geschaetzt. + | Klasse | Anzahl Paare | |---|---| -| muss-mandantengebunden | 32 | -| keine-mandantengebundene-tabelle | 16 | -| beides | 11 | -| bewusst-uebergreifend | 3 | -| **Summe** | **62** | +| muss-mandantengebunden | 31 | +| keine-mandantengebundene-tabelle | 17 | +| beides | 13 | +| bewusst-uebergreifend | 2 | +| **Summe** | **63** | -## Der Hintergrunddienst als Falle — vier Fälle +## Der Hintergrunddienst als Falle — fünf Fälle Ein Planer, der über alle Mandanten iteriert, liest zu Recht übergreifend — -muss aber *innerhalb* der Schleife je Mandant binden. Drei Dateien sind in -diesem Sinne `beides`-Fälle; der vierte, seit 260909-mir bekannte Fall ist von -anderer Art und deshalb unten getrennt aufgeführt — er iteriert gar nicht, -sondern greift sich eine beliebige Zeile heraus: +muss aber *innerhalb* der Schleife je Mandant binden. Vier Dateien sind in +diesem Sinne `beides`-Fälle, davon einer (260910-das) der bislang EINZIGE, +der auf BEIDEN Hälften bereits richtig ist; der fünfte, seit 260909-mir +bekannte Fall ist von anderer Art und deshalb unten getrennt aufgeführt — er +iteriert gar nicht, sondern greift sich eine beliebige Zeile heraus: - **`ldap.service.ts`** (AD-Abgleich) — **Stand 260909-ipc, Aufgaben 2/3: geschlossen.** Iteriert nicht selbst über alle Mandanten (der Sync läuft @@ -194,8 +210,24 @@ sondern greift sich eine beliebige Zeile heraus: `tenderMatch.findMany`/`updateMany` sowie `user.findUnique` über `forTenant()`, EIN gebundener Client je Profil, gebunden an dessen Mandanten. +- **`admin-seed.service.ts`** (`ensureDefaultGroupsForAllTenants`) — **Stand + 260910-das, Aufgabe 2: der vierte Fall dieser Art und der bislang EINZIGE, + der auf BEIDEN Hälften bereits richtig ist.** Liest `tenant.findMany()` + bewusst über ALLE Mandanten (Treiber, ungebunden — `Tenant` trägt keinen + Zeilenschutz, Aufgabe 1 gemessen, `tenant-tabelle-ohne-zeilenschutz-bleibt-lesbar`) + und ruft je Mandant `groupsService.ensureDefaultGroup(tenant.id)` auf — + dieser Rumpf ist seit 260909-jts vollständig über `forTenant()`/ + `withTenantTransaction()` gebunden (siehe Bestandsaufnahme, + `groups.service.ts`/`group`, Stand `gebunden`). Anders als bei den drei + Fällen oben musste hier in Aufgabe 2/3 (260910-das) NICHTS umgestellt + werden — der Rumpf war es bereits, bevor der Bereich `user` überhaupt an + der Reihe war. Einschränkung, bewusst nicht verschwiegen: der äußere + `try/catch` in `ensureDefaultGroupsForAllTenants()` verschluckt jeden + Fehler des Treibers (`tenant.findMany`) in eine Protokollzeile — läuft die + Mandantenliste nach dem Scharfschalten aus irgendeinem Grund leer, + entsteht keine Fehlermeldung, sondern gar keine Ausgabe (Befund K). -**Der vierte Fall, anderer Bauart — `dkv-scheduler.service.ts` / +**Der fünfte Fall, anderer Bauart — `dkv-scheduler.service.ts` / `DkvService.loadAnyActiveConfigForScheduler()`** (260909-mir, Befund D, WINDOWS #21): **offen, als benannte Altlast weitergeführt.** Dieser Dienst gehört nicht in dieselbe Klasse wie die drei oben. Jene lesen bewusst über alle @@ -287,11 +319,11 @@ verwendet), Klasse, Stand (`gebunden`/`ungebunden`/`gemischt`, seit | apps/api/src/tenders/tenders.controller.ts | tender | keine-mandantengebundene-tabelle | ungebunden | Lesezugriff auf den plattformweiten Katalog (D-03). | | apps/api/src/tenders/tenders.controller.ts | tenderSourcePollConfig | keine-mandantengebundene-tabelle | ungebunden | Plattformweiter Poll-Status, admin-verwaltet, kein `tenantId`. | | apps/api/src/tenders/tenders.module.ts | tenderSourcePollConfig | keine-mandantengebundene-tabelle | ungebunden | Singleton-Bestückung beim Boot — im Dateikopf explizit als "global, RLS-exempt (D-03)" begründet. | -| apps/api/src/user/admin-seed.service.ts | tenant | keine-mandantengebundene-tabelle | ungebunden | Legt beim ersten Start den Standard-Mandanten selbst an — `Tenant` hat keine `tenantId`-Spalte. | -| apps/api/src/user/admin-seed.service.ts | user | bewusst-uebergreifend | gemischt | ZWISCHENSTAND nach Aufgabe 2 (260910-das): die Erstanlage-Pruefung bleibt bewusst ungebunden, die Erstanlage des Administrators selbst laeuft seit Aufgabe 2 ueber `forTenant()`, gebunden an den unmittelbar zuvor angelegten Mandanten (Befund J). Die Klassenkorrektur auf `beides` samt Begruendung folgt in Aufgabe 3. | -| apps/api/src/user/user.controller.ts | user | muss-mandantengebunden | ungebunden | Nutzerverwaltung innerhalb des Mandanten des anfragenden Admins. Wird in Aufgabe 3 (260910-das) gebunden. | +| apps/api/src/user/admin-seed.service.ts | tenant | keine-mandantengebundene-tabelle | ungebunden | Legt beim ersten Start den Standard-Mandanten selbst an und liest beim Start alle Mandanten fuer die Standardgruppen-Reparatur — `Tenant` hat keine `tenantId`-Spalte und traegt keinen Zeilenschutz (Aufgabe 1, `tenant-tabelle-ohne-zeilenschutz-bleibt-lesbar`). Fuenfter und bislang einziger bereits vollstaendig richtiger Fall der Hintergrunddienst-Falle (Befund K, siehe Abschnitt unten). | +| apps/api/src/user/admin-seed.service.ts | user | beides | gemischt | Klassenkorrektur (260910-das, Aufgabe 3): wechselt von `bewusst-uebergreifend` auf `beides`, weil die bisherige Begruendung ("es gibt strukturell keinen Mandanten zum Binden") nachweislich FALSCH war (Befund J) — der Mandant wird eine Anweisung vorher angelegt und ist bekannt. Die Erstanlage-Pruefung bleibt bewusst ungebunden (kein Mandant existiert zu diesem Zeitpunkt, `username` ist plattformweit eindeutig); die Erstanlage des Administrators selbst laeuft seit Aufgabe 2 ueber `forTenant()`, gebunden an den unmittelbar zuvor angelegten Mandanten. Eine P2002-Kollision beim Anlegen wird wie "Administrator existiert bereits" behandelt statt den Start abzubrechen (Befund I). | +| apps/api/src/user/user.controller.ts | user | muss-mandantengebunden | gebunden | Nutzerverwaltung innerhalb des Mandanten des anfragenden Admins (260910-das, Aufgabe 3): die Benutzerliste des ADMIN-Zweigs, alle drei Kennungswege (rollenabhaengig ueber `UserService.findById`/`findByIdForPlatformAdmin`) und alle fuenf Selbstbedienungszugriffe (Bild hochladen/loeschen/ausliefern, Akzentfarbe) laufen ueber `forTenant()`; die Rollenverzweigung zwischen mandantengebundener ADMIN-Sicht und der uebergreifenden `SUPER_ADMIN`-Sicht (ueber `UserService.findAllForPlatformAdmin`) bleibt bestehen. Der wirkungslose Selbstloesch-Riegel (Befund H, verglich gegen `currentUser.sub`, ein im Sitzungsnachweis nicht existierendes Feld) ist auf `currentUser.id` korrigiert. | | apps/api/src/user/user.service.ts | tenant | keine-mandantengebundene-tabelle | ungebunden | Schleifentreiber der neuen Plattform-Administratorsicht (`findAllForPlatformAdmin`/`findByIdForPlatformAdmin`, 260910-das, Aufgabe 2, Befund F/N) — `Tenant` hat keine `tenantId`-Spalte und traegt keinen Zeilenschutz (Aufgabe 1, `tenant-tabelle-ohne-zeilenschutz-bleibt-lesbar`). | -| apps/api/src/user/user.service.ts | user | muss-mandantengebunden | gemischt | ZWISCHENSTAND nach Aufgabe 2 (260910-das): `findById`/`create`/`update`/`deactivate`/`delete` sowie die beiden neuen Plattform-Administratorsicht-Methoden laufen seither ueber `forTenant()`; ausschliesslich `findByUsername` bleibt bewusst ungebunden (plattformweit eindeutiger Schluessel, derselbe Fall wie `resolveEmailForWrite` im Bereich `ldap`). Die Klassenkorrektur auf `beides` samt Begruendung folgt in Aufgabe 3. | +| apps/api/src/user/user.service.ts | user | beides | gemischt | Klassenkorrektur (260910-das, Aufgabe 3): wechselt von `muss-mandantengebunden` auf `beides` wegen der einen bewusst ungebundenen Suche — wortgleich derselbe Praezedenzfall wie `ldap.service.ts`/`user` in 260909-ipc (`resolveEmailForWrite`). `findById`/`create`/`update`/`deactivate`/`delete` sowie die beiden neuen Plattform-Administratorsicht-Methoden laufen ueber `forTenant()`; `create`/`update` uebersetzen eine plattformweite Eindeutigkeitsverletzung (P2002) in eine deutsche Konfliktmeldung ohne Halter/Mandant zu nennen. `findByUsername` bleibt bewusst UNGEBUNDEN: der Anmeldeweg laeuft seit Etappe 1 ueber die drei SECURITY-DEFINER-Funktionen und hat diese Methode nicht mehr als Aufrufer (260910-das, Aufgabe 1, Teil 3: genau ein Treffer, die eigene Definition); eine gebundene Suche saehe einen fremden Halter des plattformweit eindeutigen `username` nicht und meldete faelschlich "frei". | ## Was diese Etappe NICHT entscheidet