fix: Rechteausweitung ueber Rollen-Rechtezuweisung geschlossen

RolesService.create/update uebernahm eine explizit uebergebene
permissionKeys-Liste ungeprueft. Ein Nutzer mit nur roles:update
konnte damit JEDER Rolle - auch der eigenen, ohne users:assign-role -
beliebige weitere Rechte (bis hin zu allen 92 Katalog-Eintraegen)
hinzufuegen, da das Bearbeiten der eigenen Rollen-Rechte nicht ueber
User.roleId laeuft und daher vom bereits gefixten
users:assign-role-Check nicht erfasst wurde.

Neue Regel (Standard-RBAC-Prinzip "keine Rechte ueber das eigene
Rechte-Set hinaus vergeben"): create/update pruefen jetzt, dass der
aufrufende Nutzer jedes zu vergebende Recht selbst bereits besitzt,
bevor die Rolle angelegt/geaendert wird.

roles.service.ts hatte bislang keine Unit-Tests; neue Spec-Datei deckt
den Fix sowie die uebrigen Methoden (create/update/deactivate/
reactivate) ab.

Co-Authored-By: Claude Sonnet 5 <[email protected]>

ApiCore-Backend/src/roles/roles.service.spec.ts

@@ -0,0 +1,288 @@
+import {
+  ConflictException,
+  ForbiddenException,
+  NotFoundException,
+} from '@nestjs/common';
+import { RolesService } from './roles.service';
+import { PrismaService } from '../prisma/prisma.service';
+import { AuditLogService } from '../audit/audit-log.service';
+import { PermissionResolverService } from '../auth/permissions/permission-resolver.service';
+
+describe('RolesService', () => {
+  let service: RolesService;
+  let tx: {
+    role: {
+      update: jest.Mock;
+      findUniqueOrThrow: jest.Mock;
+      create: jest.Mock;
+    };
+    rolePermission: { deleteMany: jest.Mock; createMany: jest.Mock };
+    permission: { findMany: jest.Mock };
+  };
+  let prisma: {
+    role: { findUnique: jest.Mock; findMany: jest.Mock; update: jest.Mock };
+    $transaction: jest.Mock;
+  };
+  let auditLog: { record: jest.Mock };
+  let permissionResolver: {
+    invalidateAll: jest.Mock;
+    getPermissions: jest.Mock;
+  };
+
+  const staffRole = {
+    id: 'role-staff',
+    key: 'STAFF',
+    name: 'Mitarbeiter',
+    description: null,
+    isSystem: false,
+    status: 'ACTIVE',
+    createdAt: new Date('2026-01-01'),
+    updatedAt: new Date('2026-01-01'),
+    deactivatedAt: null as Date | null,
+    permissions: [{ permission: { key: 'apiaries:read' } }],
+    _count: { users: 0 },
+  };
+
+  const adminRole = {
+    ...staffRole,
+    id: 'role-admin',
+    key: 'ADMIN',
+    isSystem: true,
+  };
+
+  beforeEach(() => {
+    tx = {
+      role: {
+        update: jest.fn().mockResolvedValue({}),
+        findUniqueOrThrow: jest.fn(),
+        create: jest.fn(),
+      },
+      rolePermission: {
+        deleteMany: jest.fn().mockResolvedValue({}),
+        createMany: jest.fn().mockResolvedValue({}),
+      },
+      permission: {
+        findMany: jest.fn().mockResolvedValue([{ id: 'perm-1' }]),
+      },
+    };
+    prisma = {
+      role: {
+        findUnique: jest.fn(),
+        findMany: jest.fn(),
+        update: jest.fn(),
+      },
+      $transaction: jest.fn((callback: (txArg: typeof tx) => unknown) =>
+        callback(tx),
+      ),
+    };
+    auditLog = { record: jest.fn().mockResolvedValue(undefined) };
+    permissionResolver = {
+      invalidateAll: jest.fn(),
+      getPermissions: jest.fn().mockResolvedValue(new Set(['apiaries:read'])),
+    };
+    service = new RolesService(
+      prisma as unknown as PrismaService,
+      auditLog as unknown as AuditLogService,
+      permissionResolver as unknown as PermissionResolverService,
+    );
+  });
+
+  describe('findById', () => {
+    it('throws NotFoundException when the role does not exist', async () => {
+      prisma.role.findUnique.mockResolvedValue(null);
+      await expect(service.findById('missing')).rejects.toBeInstanceOf(
+        NotFoundException,
+      );
+    });
+  });
+
+  describe('create', () => {
+    it('throws ConflictException when the key is already in use', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      await expect(
+        service.create(
+          { key: 'STAFF', name: 'x', permissionKeys: [] },
+          'user-1',
+        ),
+      ).rejects.toBeInstanceOf(ConflictException);
+    });
+
+    it('throws ForbiddenException when granting a permission the caller does not hold', async () => {
+      prisma.role.findUnique.mockResolvedValue(null);
+      permissionResolver.getPermissions.mockResolvedValue(
+        new Set(['apiaries:read']),
+      );
+
+      await expect(
+        service.create(
+          {
+            key: 'MANAGER',
+            name: 'Manager',
+            permissionKeys: ['apiaries:read', 'users:assign-role'],
+          },
+          'user-1',
+        ),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+      expect(tx.rolePermission.createMany).not.toHaveBeenCalled();
+    });
+
+    it('creates the role and grants only permissions the caller already holds', async () => {
+      prisma.role.findUnique.mockResolvedValue(null);
+      permissionResolver.getPermissions.mockResolvedValue(
+        new Set(['apiaries:read']),
+      );
+      tx.role.create.mockResolvedValue(staffRole);
+      tx.role.findUniqueOrThrow.mockResolvedValue(staffRole);
+
+      const result = await service.create(
+        {
+          key: 'STAFF',
+          name: 'Mitarbeiter',
+          permissionKeys: ['apiaries:read'],
+        },
+        'user-1',
+      );
+
+      expect(result.key).toBe('STAFF');
+      expect(permissionResolver.invalidateAll).toHaveBeenCalled();
+      expect(auditLog.record).toHaveBeenCalledWith(
+        expect.objectContaining({ entityType: 'Role', action: 'CREATE' }),
+      );
+    });
+
+    it('skips the permission-holding check when there is no acting user', async () => {
+      prisma.role.findUnique.mockResolvedValue(null);
+      tx.role.create.mockResolvedValue(staffRole);
+      tx.role.findUniqueOrThrow.mockResolvedValue(staffRole);
+
+      await expect(
+        service.create({
+          key: 'STAFF',
+          name: 'Mitarbeiter',
+          permissionKeys: ['users:assign-role'],
+        }),
+      ).resolves.toBeDefined();
+      expect(permissionResolver.getPermissions).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('update', () => {
+    it('throws ForbiddenException when the role is a system role', async () => {
+      prisma.role.findUnique.mockResolvedValue(adminRole);
+      await expect(
+        service.update('role-admin', { name: 'x' }, 'user-1'),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+    });
+
+    it('throws ForbiddenException when granting a permission the caller does not hold, even for their own role', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      permissionResolver.getPermissions.mockResolvedValue(
+        new Set(['apiaries:read']),
+      );
+
+      await expect(
+        service.update(
+          'role-staff',
+          {
+            permissionKeys: [
+              'apiaries:read',
+              'roles:update',
+              'users:assign-role',
+            ],
+          },
+          'user-1',
+        ),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+      expect(tx.role.update).not.toHaveBeenCalled();
+    });
+
+    it('updates name/description without triggering the permission check when permissionKeys is absent', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      tx.role.findUniqueOrThrow.mockResolvedValue(staffRole);
+
+      await service.update('role-staff', { name: 'Neuer Name' }, 'user-1');
+
+      expect(permissionResolver.getPermissions).not.toHaveBeenCalled();
+      expect(tx.rolePermission.deleteMany).not.toHaveBeenCalled();
+    });
+
+    it('replaces permissions and invalidates the cache when the caller holds everything granted', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      permissionResolver.getPermissions.mockResolvedValue(
+        new Set(['apiaries:read']),
+      );
+      tx.role.findUniqueOrThrow.mockResolvedValue(staffRole);
+
+      const result = await service.update(
+        'role-staff',
+        { permissionKeys: ['apiaries:read'] },
+        'user-1',
+      );
+
+      expect(result.key).toBe('STAFF');
+      expect(tx.rolePermission.deleteMany).toHaveBeenCalledWith({
+        where: { roleId: 'role-staff' },
+      });
+      expect(permissionResolver.invalidateAll).toHaveBeenCalled();
+    });
+  });
+
+  describe('deactivate', () => {
+    it('throws ForbiddenException for a system role', async () => {
+      prisma.role.findUnique.mockResolvedValue(adminRole);
+      await expect(
+        service.deactivate('role-admin', 'user-1'),
+      ).rejects.toBeInstanceOf(ForbiddenException);
+    });
+
+    it('throws ConflictException when users are still assigned', async () => {
+      prisma.role.findUnique.mockResolvedValue({
+        ...staffRole,
+        _count: { users: 2 },
+      });
+      await expect(
+        service.deactivate('role-staff', 'user-1'),
+      ).rejects.toBeInstanceOf(ConflictException);
+    });
+
+    it('deactivates an unused role', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      prisma.role.update.mockResolvedValue({
+        ...staffRole,
+        status: 'INACTIVE',
+        deactivatedAt: new Date(),
+      });
+
+      const result = await service.deactivate('role-staff', 'user-1');
+
+      expect(result.status).toBe('INACTIVE');
+      expect(permissionResolver.invalidateAll).toHaveBeenCalled();
+    });
+  });
+
+  describe('reactivate', () => {
+    it('throws ConflictException when already active', async () => {
+      prisma.role.findUnique.mockResolvedValue(staffRole);
+      await expect(
+        service.reactivate('role-staff', 'user-1'),
+      ).rejects.toBeInstanceOf(ConflictException);
+    });
+
+    it('reactivates an inactive role', async () => {
+      prisma.role.findUnique.mockResolvedValue({
+        ...staffRole,
+        status: 'INACTIVE',
+        deactivatedAt: new Date(),
+      });
+      prisma.role.update.mockResolvedValue({
+        ...staffRole,
+        status: 'ACTIVE',
+        deactivatedAt: null,
+      });
+
+      const result = await service.reactivate('role-staff', 'user-1');
+
+      expect(result.status).toBe('ACTIVE');
+    });
+  });
+});

ApiCore-Backend/src/roles/roles.service.ts

@@ -81,6 +81,31 @@ export class RolesService {
     return this.toSummary(await this.findById(id));
   }
 
+  /**
+   * Nobody can grant a role a permission they do not hold themselves - a
+   * caller with only roles:update could otherwise turn any role (including
+   * one they belong to) into a shadow-admin by adding every catalog key, no
+   * users:assign-role needed at all since editing a role's own grants
+   * doesn't touch User.roleId. This is the standard "can't escalate beyond
+   * what you have" RBAC rule.
+   */
+  private async assertCanGrantPermissions(
+    permissionKeys: string[],
+    actingUserId?: string,
+  ): Promise<void> {
+    if (!actingUserId) {
+      return;
+    }
+    const callerPermissions =
+      await this.permissionResolver.getPermissions(actingUserId);
+    const missing = permissionKeys.filter((key) => !callerPermissions.has(key));
+    if (missing.length > 0) {
+      throw new ForbiddenException(
+        `Cannot grant permissions you do not hold yourself: ${missing.join(', ')}`,
+      );
+    }
+  }
+
   async create(
     dto: CreateRoleDto,
     actingUserId?: string,
@@ -91,6 +116,7 @@ export class RolesService {
     if (existing) {
       throw new ConflictException('Role key already in use');
     }
+    await this.assertCanGrantPermissions(dto.permissionKeys, actingUserId);
     const role = await this.prisma.$transaction(async (tx) => {
       const created = await tx.role.create({
         data: {
@@ -126,6 +152,9 @@ export class RolesService {
     if (before.isSystem) {
       throw new ForbiddenException('System roles cannot be edited');
     }
+    if (dto.permissionKeys) {
+      await this.assertCanGrantPermissions(dto.permissionKeys, actingUserId);
+    }
     const role = await this.prisma.$transaction(async (tx) => {
       await tx.role.update({
         where: { id },