Skip to content

Commit 8682552

Browse files
committed
feat: implement comprehensive user deletion with authorization and safety checks
- Add authorization logic (users delete own accounts, admins delete any, last admin protection) - Implement transaction-based soft deletion preserving email/role/clerk_id while clearing PII - Add role-specific profile cleanup (student thesis_description, supervisor bio/spots) - Include enhanced logging with deletion statistics - Replace hardcoded constants with configurable limits (User search limit) - Complete rewrite with comprehensive test coverage for all deletion scenarios
1 parent a8d286d commit 8682552

10 files changed

Lines changed: 571 additions & 76 deletions
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { ForbiddenException } from '@nestjs/common';
2+
3+
export class LastAdminDeleteException extends ForbiddenException {
4+
constructor() {
5+
super('Cannot delete the last admin in the system');
6+
this.name = 'LastAdminDeleteException';
7+
}
8+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { ForbiddenException } from '@nestjs/common';
2+
3+
export class UserDeleteForbiddenException extends ForbiddenException {
4+
constructor() {
5+
super('You can only delete your own account');
6+
this.name = 'UserDeleteForbiddenException';
7+
}
8+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { ApiProperty } from '@nestjs/swagger';
2+
3+
export class DeleteUserSuccessDto {
4+
@ApiProperty({
5+
description: 'Indicates if the operation was successful',
6+
example: true,
7+
})
8+
success: boolean;
9+
10+
@ApiProperty({
11+
description: 'Success message with user email',
12+
example: 'User john.doe@fhstp.ac.at has been deleted successfully',
13+
})
14+
message: string;
15+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { Role } from '@prisma/client';
2+
3+
/**
4+
* Type definition for user deletion operation parameters
5+
*
6+
* Provides type safety for the complex user deletion process
7+
* that involves multiple related entities and database operations.
8+
*/
9+
export interface DeleteUserOperation {
10+
userId: string;
11+
userEmail: string;
12+
userRole: Role;
13+
studentId?: string;
14+
supervisorId?: string;
15+
}
16+
17+
/**
18+
* Type definition for user deletion operation results
19+
*
20+
* Contains statistics about what was deleted/modified during
21+
* the user deletion process for logging and audit purposes.
22+
*/
23+
export interface DeleteUserResult {
24+
deletedTagsCount: number;
25+
deletedBlocksCount: number;
26+
profileUpdated: boolean;
27+
}

backend/src/modules/users/users.controller.spec.ts

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ describe('UsersController', () => {
3636

3737
const USER_UUID = '123e4567-e89b-12d3-a456-426614174000';
3838
const USER_UUID_2 = '123e4567-e89b-12d3-a456-426614174001';
39+
const USER_UUID_3 = '123e4567-e89b-12d3-a456-426614174003';
40+
const ADMIN_UUID = '123e4567-e89b-12d3-a456-426614174002';
3941
const TAG_UUID_1 = 'f47ac10b-58cc-4372-a567-0e02b2c3d479';
4042
const TAG_UUID_2 = 'a1b2c3d4-e5f6-7890-1234-567890abcdef';
4143
const CLERK_ID = 'user_2NUj8tGhSFhTLD9sdP0q4P7VoJM';
@@ -56,7 +58,7 @@ describe('UsersController', () => {
5658

5759
const mockAdmin: User = {
5860
...mockUser,
59-
id: 'admin-id',
61+
id: ADMIN_UUID,
6062
email: 'admin@fhstp.ac.at',
6163
role: Role.ADMIN,
6264
};
@@ -235,9 +237,16 @@ describe('UsersController', () => {
235237

236238
describe('deleteUser', () => {
237239
it('should delete a user', async () => {
238-
mockUsersService.deleteUser.mockResolvedValue(mockUser);
239-
await controller.deleteUser(USER_UUID);
240-
expect(mockUsersService.deleteUser).toHaveBeenCalledWith(USER_UUID);
240+
const mockDeleteResult = {
241+
success: true,
242+
message: 'User has been deleted successfully',
243+
};
244+
mockUsersService.deleteUser.mockResolvedValue(mockDeleteResult);
245+
246+
const result = await controller.deleteUser(USER_UUID, mockUser);
247+
248+
expect(result).toEqual(mockDeleteResult);
249+
expect(mockUsersService.deleteUser).toHaveBeenCalledWith(USER_UUID, mockUser);
241250
});
242251
});
243252

@@ -340,12 +349,12 @@ describe('UsersController', () => {
340349

341350
it("should throw UnauthorizedException when a student tries to view another student's blocked supervisors", async () => {
342351
// Arrange
343-
const otherStudentId = 'other-student-id'; // This is different from mockUser.id (USER_UUID)
352+
const otherStudentUserId = USER_UUID_3;
344353
const currentUser = mockUser; // Student with id USER_UUID
345354

346355
try {
347356
// Act
348-
await controller.findBlockedSupervisorsByStudentUserId(otherStudentId, currentUser);
357+
await controller.findBlockedSupervisorsByStudentUserId(otherStudentUserId, currentUser);
349358
// If we get here, fail the test
350359
fail('Expected an UnauthorizedException to be thrown');
351360
} catch (error) {
@@ -399,13 +408,13 @@ describe('UsersController', () => {
399408

400409
it('should throw UnauthorizedException when a student tries to block a supervisor on behalf of another student', async () => {
401410
// Arrange
402-
const otherStudentId = 'other-student-id'; // This is different from mockUser.id (USER_UUID)
411+
const otherStudentUserId = USER_UUID_3;
403412
const dto: CreateUserBlockDto = { blocked_id: USER_UUID_2 };
404413
const currentUser = mockUser; // Student with id USER_UUID
405414

406415
try {
407416
// Act
408-
await controller.createUserBlock(otherStudentId, dto, currentUser);
417+
await controller.createUserBlock(otherStudentUserId, dto, currentUser);
409418
// If we get here, fail the test
410419
fail('Expected an UnauthorizedException to be thrown');
411420
} catch (error) {
@@ -464,13 +473,13 @@ describe('UsersController', () => {
464473

465474
it('should throw UnauthorizedException when a student tries to unblock a supervisor on behalf of another student', async () => {
466475
// Arrange
467-
const otherStudentId = 'other-student-id';
476+
const otherStudentUserId = USER_UUID_3;
468477
const supervisorUserId = USER_UUID_2;
469478
const currentUser = mockUser; // Student
470479

471480
// Act & Assert
472481
await expect(
473-
controller.removeUserBlock(otherStudentId, supervisorUserId, currentUser),
482+
controller.removeUserBlock(otherStudentUserId, supervisorUserId, currentUser),
474483
).rejects.toThrow(UnauthorizedException);
475484
expect(mockUsersService.deleteUserBlock).not.toHaveBeenCalled();
476485
});

backend/src/modules/users/users.controller.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
import { UsersService } from './users.service';
2020
import { CreateUserDto } from './dto/create-user.dto';
2121
import { UpdateUserDto } from './dto/update-user.dto';
22+
import { DeleteUserSuccessDto } from './dto/delete-user-success.dto';
2223
import {
2324
ApiTags,
2425
ApiOperation,
@@ -396,12 +397,11 @@ export class UsersController {
396397
}
397398

398399
@Delete(':id')
399-
@Roles(Role.ADMIN)
400-
@HttpCode(HttpStatus.NO_CONTENT)
400+
@HttpCode(HttpStatus.OK)
401401
@ApiOperation({
402402
summary: 'Delete user (Soft Delete)',
403403
description:
404-
'Soft delete a user. Preserves data but marks user as deleted and they will no longer appear in regular queries.',
404+
'Soft delete a user. Users can delete their own accounts, admins can delete any user. The last admin cannot delete themselves.',
405405
})
406406
@ApiParam({
407407
name: 'id',
@@ -411,11 +411,22 @@ export class UsersController {
411411
required: true,
412412
example: '123e4567-e89b-12d3-a456-426614174000',
413413
})
414-
@ApiResponse({ status: 204, description: 'User has been successfully deleted.' })
414+
@ApiResponse({
415+
status: 200,
416+
description: 'User has been successfully deleted.',
417+
type: DeleteUserSuccessDto,
418+
})
415419
@ApiResponse({ status: 404, description: 'User not found.' })
416420
@ApiResponse({ status: 400, description: 'Bad request - Invalid User ID format.' })
417-
deleteUser(@Param('id', ParseUUIDPipe) id: string): Promise<User> {
418-
return this.usersService.deleteUser(id);
421+
@ApiResponse({
422+
status: 403,
423+
description: 'Forbidden - Not authorized to delete this user or last admin.',
424+
})
425+
deleteUser(
426+
@Param('id', ParseUUIDPipe) id: string,
427+
@CurrentUser() currentUser: User,
428+
): Promise<DeleteUserSuccessDto> {
429+
return this.usersService.deleteUser(id, currentUser);
419430
}
420431

421432
/**

0 commit comments

Comments
 (0)