Skip to content

Commit

Permalink
fix: prevent user from deleting their own roles
Browse files Browse the repository at this point in the history
  • Loading branch information
Emnaghz committed Sep 18, 2024
1 parent 549b946 commit cb64234
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 16 deletions.
54 changes: 46 additions & 8 deletions api/src/user/controllers/role.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -42,7 +43,6 @@ describe('RoleController', () => {
let roleService: RoleService;
let permissionService: PermissionService;
let userService: UserService;
let notFoundId: string;
let roleAdmin: Role;
let rolePublic: Role;

Expand Down Expand Up @@ -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', () => {
Expand All @@ -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);
Expand Down
29 changes: 23 additions & 6 deletions api/src/user/controllers/role.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -34,13 +37,15 @@ 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')
export class RoleController extends BaseController<Role, RoleStub> {
constructor(
private readonly roleService: RoleService,
private readonly logger: LoggerService,
private readonly userService: UserService,
) {
super(roleService);
}
Expand Down Expand Up @@ -142,12 +147,24 @@ export class RoleController extends BaseController<Role, RoleStub> {
@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;
}
}
4 changes: 2 additions & 2 deletions frontend/src/components/roles/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit cb64234

Please sign in to comment.