From cb64234ab1c4f62a46bbfa7037d6b00fab360624 Mon Sep 17 00:00:00 2001 From: Emnaghz Date: Wed, 18 Sep 2024 17:29:44 +0100 Subject: [PATCH] fix: prevent user from deleting their own roles --- .../user/controllers/role.controller.spec.ts | 54 ++++++++++++++++--- api/src/user/controllers/role.controller.ts | 29 +++++++--- frontend/src/components/roles/index.tsx | 4 +- 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/api/src/user/controllers/role.controller.spec.ts b/api/src/user/controllers/role.controller.spec.ts index adb758c..fae89e6 100644 --- a/api/src/user/controllers/role.controller.spec.ts +++ b/api/src/user/controllers/role.controller.spec.ts @@ -8,10 +8,11 @@ */ import { CACHE_MANAGER } from '@nestjs/cache-manager'; -import { NotFoundException } from '@nestjs/common'; +import { ForbiddenException, NotFoundException } from '@nestjs/common'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { MongooseModule } from '@nestjs/mongoose'; import { Test, TestingModule } from '@nestjs/testing'; +import { Session as ExpressSession } from 'express-session'; import { AttachmentRepository } from '@/attachment/repositories/attachment.repository'; import { AttachmentModel } from '@/attachment/schemas/attachment.schema'; @@ -42,7 +43,6 @@ describe('RoleController', () => { let roleService: RoleService; let permissionService: PermissionService; let userService: UserService; - let notFoundId: string; let roleAdmin: Role; let rolePublic: Role; @@ -190,17 +190,48 @@ describe('RoleController', () => { }); describe('deleteOne', () => { - it('should delete role by id', async () => { - const result = await roleController.deleteOne(roleAdmin.id); - notFoundId = roleAdmin.id; - expect(result).toEqual({ acknowledged: true, deletedCount: 1 }); + it("should throw ForbiddenException if the role is part of the user's roles", async () => { + const session = { passport: { user: { id: 'user1' } } } as ExpressSession; + const roleId = 'role1'; + + userService.findOneAndPopulate = jest.fn().mockResolvedValue({ + roles: [{ id: roleId }], + }); + + await expect(roleController.deleteOne(roleId, session)).rejects.toThrow( + ForbiddenException, + ); }); - it('should throw a NotFoundException when attempting to delete a role by id', async () => { - await expect(roleController.deleteOne(notFoundId)).rejects.toThrow( + it('should throw NotFoundException if the role is not found', async () => { + const session = { passport: { user: { id: 'user1' } } } as ExpressSession; + const roleId = 'role2'; + + userService.findOneAndPopulate = jest.fn().mockResolvedValue({ + roles: [{ id: 'role1' }], + }); + + roleService.deleteOne = jest.fn().mockResolvedValue({ deletedCount: 0 }); + + await expect(roleController.deleteOne(roleId, session)).rejects.toThrow( NotFoundException, ); }); + + it('should return the result if the role is successfully deleted', async () => { + const session = { passport: { user: { id: 'user1' } } } as ExpressSession; + const roleId = 'role2'; + + userService.findOneAndPopulate = jest.fn().mockResolvedValue({ + roles: [{ id: 'role1' }], + }); + + const deleteResult = { deletedCount: 1 }; + roleService.deleteOne = jest.fn().mockResolvedValue(deleteResult); + + const result = await roleController.deleteOne(roleId, session); + expect(result).toEqual(deleteResult); + }); }); describe('updateOne', () => { @@ -225,6 +256,13 @@ describe('RoleController', () => { }); it('should throw a NotFoundException when attempting to modify a role', async () => { + const notFoundId = 'nonexistentRoleId'; + const roleUpdateDto = { name: 'newRoleName' }; + + roleService.updateOne = jest + .fn() + .mockRejectedValue(new NotFoundException()); + await expect( roleController.updateOne(notFoundId, roleUpdateDto), ).rejects.toThrow(NotFoundException); diff --git a/api/src/user/controllers/role.controller.ts b/api/src/user/controllers/role.controller.ts index 5ae2919..7fe36c3 100644 --- a/api/src/user/controllers/role.controller.ts +++ b/api/src/user/controllers/role.controller.ts @@ -19,8 +19,11 @@ import { Patch, Query, UseInterceptors, + Session, + ForbiddenException, } from '@nestjs/common'; import { CsrfCheck } from '@tekuconcept/nestjs-csrf'; +import { Session as ExpressSession } from 'express-session'; import { TFilterQuery } from 'mongoose'; import { CsrfInterceptor } from '@/interceptors/csrf.interceptor'; @@ -34,6 +37,7 @@ import { SearchFilterPipe } from '@/utils/pipes/search-filter.pipe'; import { RoleCreateDto, RoleUpdateDto } from '../dto/role.dto'; import { Role, RoleStub } from '../schemas/role.schema'; import { RoleService } from '../services/role.service'; +import { UserService } from '../services/user.service'; @UseInterceptors(CsrfInterceptor) @Controller('role') @@ -41,6 +45,7 @@ export class RoleController extends BaseController { constructor( private readonly roleService: RoleService, private readonly logger: LoggerService, + private readonly userService: UserService, ) { super(roleService); } @@ -142,12 +147,24 @@ export class RoleController extends BaseController { @CsrfCheck(true) @Delete(':id') @HttpCode(204) - async deleteOne(@Param('id') id: string) { - const result = await this.roleService.deleteOne(id); - if (result.deletedCount === 0) { - this.logger.warn(`Unable to delete Role by id ${id}`); - throw new NotFoundException(`Role with ID ${id} not found`); + async deleteOne(@Param('id') id: string, @Session() session: ExpressSession) { + const roles = ( + await this.userService.findOneAndPopulate(session.passport?.user?.id, [ + 'roles', + ]) + ).roles.map((role) => role.id); + if (roles.includes(id)) { + throw new ForbiddenException("Your account's role can't be deleted"); + } else { + try { + const result = await this.roleService.deleteOne(id); + if (result.deletedCount === 0) { + throw new NotFoundException(`Role with ID ${id} not found`); + } + return result; + } catch (error) { + throw new NotFoundException(`Role with ID ${id} not found`); + } } - return result; } } diff --git a/frontend/src/components/roles/index.tsx b/frontend/src/components/roles/index.tsx index 380728f..4ae6b6f 100644 --- a/frontend/src/components/roles/index.tsx +++ b/frontend/src/components/roles/index.tsx @@ -57,8 +57,8 @@ export const Roles = () => { }, ); const { mutateAsync: deleteRole } = useDelete(EntityType.ROLE, { - onError: () => { - toast.error(t("message.internal_server_error")); + onError: (error) => { + toast.error(t(error.message || "message.internal_server_error")); }, onSuccess() { deleteDialogCtl.closeDialog();