Merge pull request #41 from Hexastack/40-issue-prevent-users-from-del…

…eting-their-own-roles

fix: prevent user from deleting their own roles

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 { Request } from 'express';
 
 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,51 @@ 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 req = { user: { roles: ['role1'] } } as unknown as Request;
+      const roleId = 'role1';
+
+      userService.findOne = jest.fn().mockResolvedValue(null);
+
+      await expect(roleController.deleteOne(roleId, req)).rejects.toThrow(
+        ForbiddenException,
+      );
+    });
+
+    it('should throw ForbiddenException if the role is associated with other users', async () => {
+      const req = { user: { roles: ['role2'] } } as unknown as Request;
+      const roleId = 'role1';
+
+      userService.findOne = jest.fn().mockResolvedValue({ id: 'user2' });
+
+      await expect(roleController.deleteOne(roleId, req)).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 req = { user: { roles: ['role2'] } } as unknown as Request;
+      const roleId = 'role1';
+
+      userService.findOne = jest.fn().mockResolvedValue(null);
+      roleService.deleteOne = jest.fn().mockResolvedValue({ deletedCount: 0 });
+
+      await expect(roleController.deleteOne(roleId, req)).rejects.toThrow(
         NotFoundException,
       );
     });
+
+    it('should return the result if the role is successfully deleted', async () => {
+      const req = { user: { roles: ['role2'] } } as unknown as Request;
+      const roleId = 'role1';
+
+      userService.findOne = jest.fn().mockResolvedValue(null);
+      const deleteResult = { deletedCount: 1 };
+      roleService.deleteOne = jest.fn().mockResolvedValue(deleteResult);
+
+      const result = await roleController.deleteOne(roleId, req);
+      expect(result).toEqual(deleteResult);
+    });
   });
 
   describe('updateOne', () => {
@@ -225,6 +259,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);

api/src/user/controllers/role.controller.ts

@@ -19,8 +19,11 @@ import {
   Patch,
   Query,
   UseInterceptors,
+  ForbiddenException,
+  Req,
 } from '@nestjs/common';
 import { CsrfCheck } from '@tekuconcept/nestjs-csrf';
+import { Request } from 'express';
 import { TFilterQuery } from 'mongoose';
 
 import { CsrfInterceptor } from '@/interceptors/csrf.interceptor';
@@ -33,7 +36,9 @@ import { SearchFilterPipe } from '@/utils/pipes/search-filter.pipe';
 
 import { RoleCreateDto, RoleUpdateDto } from '../dto/role.dto';
 import { Role, RoleFull, RolePopulate, RoleStub } from '../schemas/role.schema';
+import { User } from '../schemas/user.schema';
 import { RoleService } from '../services/role.service';
+import { UserService } from '../services/user.service';
 
 @UseInterceptors(CsrfInterceptor)
 @Controller('role')
@@ -46,6 +51,7 @@ export class RoleController extends BaseController<
   constructor(
     private readonly roleService: RoleService,
     private readonly logger: LoggerService,
+    private readonly userService: UserService,
   ) {
     super(roleService);
   }
@@ -147,12 +153,22 @@ 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, @Req() req: Request) {
+    const userRoles = (req.user as User).roles;
+
+    const associatedUser = await this.userService.findOne({
+      roles: { $in: [id] },
+    });
+    if (userRoles.includes(id)) {
+      throw new ForbiddenException("Your account's role can't be deleted");
+    } else if (associatedUser) {
+      throw new ForbiddenException('Role is associated with other users');
+    } else {
+      const result = await this.roleService.deleteOne(id);
+      if (result.deletedCount === 0) {
+        throw new NotFoundException(`Role with ID ${id} not found`);
+      }
+      return result;
     }
-    return result;
   }
 }

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();