fix: kritische Rechte-Luecke, Admin-Lockout-Schutz und Wetter-Haertung
Dispatchte security-reviewer und code-reviewer Agenten fuer eine vollstaendige Pruefung des RBAC-Systems (d31aa79..HEAD). Ein KRITISCHER Fund, zwei HIGH, ein weiteres MEDIUM, alle behoben: KRITISCH: Deaktivieren eines Benutzers hat dessen Berechtigungen nicht entzogen. PermissionResolverService.loadPermissions() prüfte nur den Rollen-Status, nie User.isActive - ein bereits ausgestelltes Access- Token behielt volle Rechte bis zum Ablauf (bis zu 15 Minuten), nicht die vorgesehenen 30 Sekunden. UsersService rief zudem nie invalidateUser() auf. Beides jetzt behoben; neue dedizierte permission-resolver.service.spec.ts deckt das ab (existierte bisher gar nicht - ein Grund, warum die Luecke unbemerkt blieb). HIGH: Kein Schutz gegen den letzten aktiven Systemrollen-Nutzer (ADMIN). Man konnte den letzten Admin deaktivieren oder umrollen und sich damit komplett aussperren - ohne Selbsthilfe-Weg, da prisma/seed.ts das Seeden ueberspringt, sobald die Admin-E-Mail bereits existiert. UsersService.deactivate()/.update() pruefen jetzt per assertNotLastSystemRoleUser(), analog zum bestehenden Lockout-Schutz in RolesService fuer Systemrollen selbst. HIGH: Der Open-Meteo-Aufruf in WeatherService hatte kein Timeout und konnte unbegrenzt haengen. Jetzt mit AbortSignal.timeout(8000) plus sauberem Fehler-Mapping. MEDIUM: Trachtprognose fiel bei fehlenden Wetterdaten stillschweigend auf "MAESSIG" zurueck statt einen eigenen Unbekannt-Zustand zu zeigen - ein Imker haette eine echte Einschaetzung gesehen, wo keine moeglich war. Neuer Rating-Wert UNBEKANNT in Backend und Frontend. LOW: Veralteter Kommentar an PermissionsGuard, der noch den in Phase 3 geloeschten RolesGuard erwaehnte, korrigiert. Verifiziert: tsc, lint, build auf beiden Projekten gruen. Backend-Unit 245/245 (17 neue Tests fuer die Fixes). Frontend-Unit 14/14. Volle Backend-e2e-Suite konnte weiterhin nicht laufen (WSL/Postgres von Windows aus nicht erreichbar, auch nach vollem "wsl --shutdown"- Neustart) - jeder Fix hat aber gezielte, gruene Unit-Abdeckung. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
f1d6b2956e
commit
5a1015c4a0
9 files changed
+329
-11
No files matched your search
@@ -2,6 +2,7 @@ import { ConflictException, NotFoundException } from '@nestjs/common';
|
||||
import { UsersService } from './users.service';
|
||||
import { PrismaService } from '../prisma/prisma.service';
|
||||
import { AuditLogService } from '../audit/audit-log.service';
|
||||
import { PermissionResolverService } from '../auth/permissions/permission-resolver.service';
|
||||
|
||||
describe('UsersService', () => {
|
||||
let usersService: UsersService;
|
||||
@@ -11,18 +12,32 @@ describe('UsersService', () => {
|
||||
findUnique: jest.Mock;
|
||||
create: jest.Mock;
|
||||
update: jest.Mock;
|
||||
count: jest.Mock;
|
||||
};
|
||||
role: {
|
||||
findUnique: jest.Mock;
|
||||
};
|
||||
};
|
||||
let auditLog: { record: jest.Mock };
|
||||
let permissionResolver: {
|
||||
invalidateUser: jest.Mock;
|
||||
invalidateAll: jest.Mock;
|
||||
};
|
||||
|
||||
const staffRole = {
|
||||
id: 'role-staff',
|
||||
key: 'STAFF',
|
||||
name: 'Mitarbeiter',
|
||||
status: 'ACTIVE',
|
||||
isSystem: false,
|
||||
};
|
||||
|
||||
const adminRole = {
|
||||
id: 'role-admin',
|
||||
key: 'ADMIN',
|
||||
name: 'Administrator',
|
||||
status: 'ACTIVE',
|
||||
isSystem: true,
|
||||
};
|
||||
|
||||
const existingUser = {
|
||||
@@ -37,6 +52,14 @@ describe('UsersService', () => {
|
||||
deactivatedAt: null,
|
||||
};
|
||||
|
||||
const adminUser = {
|
||||
...existingUser,
|
||||
id: 'user-admin',
|
||||
email: '[email protected]',
|
||||
roleId: adminRole.id,
|
||||
role: adminRole,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
prisma = {
|
||||
user: {
|
||||
@@ -44,15 +67,21 @@ describe('UsersService', () => {
|
||||
findUnique: jest.fn(),
|
||||
create: jest.fn(),
|
||||
update: jest.fn(),
|
||||
count: jest.fn(),
|
||||
},
|
||||
role: {
|
||||
findUnique: jest.fn().mockResolvedValue(staffRole),
|
||||
},
|
||||
};
|
||||
auditLog = { record: jest.fn().mockResolvedValue(undefined) };
|
||||
permissionResolver = {
|
||||
invalidateUser: jest.fn(),
|
||||
invalidateAll: jest.fn(),
|
||||
};
|
||||
usersService = new UsersService(
|
||||
prisma as unknown as PrismaService,
|
||||
auditLog as unknown as AuditLogService,
|
||||
permissionResolver as unknown as PermissionResolverService,
|
||||
);
|
||||
});
|
||||
|
||||
@@ -151,7 +180,7 @@ describe('UsersService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('deactivate sets isActive to false and records an audit entry', async () => {
|
||||
it('deactivate sets isActive to false, invalidates the permission cache, and records an audit entry', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(existingUser);
|
||||
prisma.user.update.mockResolvedValue({
|
||||
...existingUser,
|
||||
@@ -163,11 +192,39 @@ describe('UsersService', () => {
|
||||
|
||||
expect(result.isActive).toBe(false);
|
||||
expect(result.deactivatedAt).not.toBeNull();
|
||||
expect(permissionResolver.invalidateUser).toHaveBeenCalledWith(
|
||||
existingUser.id,
|
||||
);
|
||||
expect(auditLog.record).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ entityType: 'User', action: 'DEACTIVATE' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('deactivate throws ConflictException when deactivating the last active user of a system role', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(adminUser);
|
||||
prisma.user.count.mockResolvedValue(1);
|
||||
|
||||
await expect(usersService.deactivate(adminUser.id)).rejects.toBeInstanceOf(
|
||||
ConflictException,
|
||||
);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
expect(permissionResolver.invalidateUser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('deactivate succeeds for a system-role user when another active user shares that role', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(adminUser);
|
||||
prisma.user.count.mockResolvedValue(2);
|
||||
prisma.user.update.mockResolvedValue({
|
||||
...adminUser,
|
||||
isActive: false,
|
||||
deactivatedAt: new Date(),
|
||||
});
|
||||
|
||||
const result = await usersService.deactivate(adminUser.id);
|
||||
|
||||
expect(result.isActive).toBe(false);
|
||||
});
|
||||
|
||||
it('activate throws ConflictException when already active', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(existingUser);
|
||||
await expect(usersService.activate(existingUser.id)).rejects.toBeInstanceOf(
|
||||
@@ -175,7 +232,7 @@ describe('UsersService', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('activate sets isActive to true and clears deactivatedAt', async () => {
|
||||
it('activate sets isActive to true, invalidates the permission cache, and clears deactivatedAt', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue({
|
||||
...existingUser,
|
||||
isActive: false,
|
||||
@@ -191,8 +248,46 @@ describe('UsersService', () => {
|
||||
|
||||
expect(result.isActive).toBe(true);
|
||||
expect(result.deactivatedAt).toBeNull();
|
||||
expect(permissionResolver.invalidateUser).toHaveBeenCalledWith(
|
||||
existingUser.id,
|
||||
);
|
||||
expect(auditLog.record).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ entityType: 'User', action: 'UPDATE' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('update invalidates the permission cache when the role changes', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(existingUser);
|
||||
prisma.user.update.mockResolvedValue({
|
||||
...existingUser,
|
||||
roleId: adminRole.id,
|
||||
role: adminRole,
|
||||
});
|
||||
|
||||
await usersService.update(existingUser.id, { roleId: adminRole.id });
|
||||
|
||||
expect(permissionResolver.invalidateUser).toHaveBeenCalledWith(
|
||||
existingUser.id,
|
||||
);
|
||||
});
|
||||
|
||||
it('update does not touch the permission cache when the role is unchanged', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(existingUser);
|
||||
prisma.user.update.mockResolvedValue({ ...existingUser, name: 'Renamed' });
|
||||
|
||||
await usersService.update(existingUser.id, { name: 'Renamed' });
|
||||
|
||||
expect(permissionResolver.invalidateUser).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('update throws ConflictException when reassigning the last active user away from a system role', async () => {
|
||||
prisma.user.findUnique.mockResolvedValue(adminUser);
|
||||
prisma.role.findUnique.mockResolvedValue(staffRole);
|
||||
prisma.user.count.mockResolvedValue(1);
|
||||
|
||||
await expect(
|
||||
usersService.update(adminUser.id, { roleId: staffRole.id }),
|
||||
).rejects.toBeInstanceOf(ConflictException);
|
||||
expect(prisma.user.update).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user