From e0c5a3dfc7d0c98946d4b6b13f68a9cefbdd5496 Mon Sep 17 00:00:00 2001 From: Adewale Iyanuoluwa Isaac Date: Wed, 29 Jul 2026 11:33:00 +0100 Subject: [PATCH 1/2] fix(rbac): secure RolesController with admin authorization guards and DTOs --- src/rbac/roles/dto/create-role.dto.ts | 14 +++++ src/rbac/roles/dto/update-role.dto.ts | 14 +++++ src/rbac/roles/roles.controller.ts | 75 ++++++++++++++++++++++----- 3 files changed, 90 insertions(+), 13 deletions(-) create mode 100644 src/rbac/roles/dto/create-role.dto.ts create mode 100644 src/rbac/roles/dto/update-role.dto.ts diff --git a/src/rbac/roles/dto/create-role.dto.ts b/src/rbac/roles/dto/create-role.dto.ts new file mode 100644 index 00000000..70242b33 --- /dev/null +++ b/src/rbac/roles/dto/create-role.dto.ts @@ -0,0 +1,14 @@ +import { IsString, IsOptional, IsUUID } from 'class-validator'; + +export class CreateRoleDto { + @IsString() + name: string; + + @IsString() + @IsOptional() + description?: string; + + @IsUUID(undefined, { each: true }) + @IsOptional() + permissionIds?: string[]; +} diff --git a/src/rbac/roles/dto/update-role.dto.ts b/src/rbac/roles/dto/update-role.dto.ts new file mode 100644 index 00000000..a9dbb846 --- /dev/null +++ b/src/rbac/roles/dto/update-role.dto.ts @@ -0,0 +1,14 @@ +import { IsString, IsOptional, IsUUID } from 'class-validator'; + +export class UpdateRoleDto { + @IsString() + name: string; + + @IsString() + @IsOptional() + description?: string; + + @IsUUID(undefined, { each: true }) + @IsOptional() + permissionIds?: string[]; +} diff --git a/src/rbac/roles/roles.controller.ts b/src/rbac/roles/roles.controller.ts index 6ad4ef6f..4d05ae88 100644 --- a/src/rbac/roles/roles.controller.ts +++ b/src/rbac/roles/roles.controller.ts @@ -1,18 +1,52 @@ -import { Controller, Get, Post, Body, Param, Put, Delete } from '@nestjs/common'; +import { + Controller, + Get, + Post, + Body, + Param, + Put, + Delete, + UseGuards, + Req, +} from '@nestjs/common'; +import { Request } from 'express'; +import { ApiBearerAuth } from '@nestjs/swagger'; import { RolesService } from './roles.service'; import { Role } from '../entities/role.entity'; +import { JwtAuthGuard } from '../../auth/guards/jwt-auth.guard'; +import { RolesGuard } from '../../auth/guards/roles.guard'; +import { Roles } from '../../auth/decorators/roles.decorator'; +import { CreateRoleDto } from './dto/create-role.dto'; +import { UpdateRoleDto } from './dto/update-role.dto'; +@ApiBearerAuth() +@UseGuards(JwtAuthGuard, RolesGuard) +@Roles('admin') @Controller('roles') export class RolesController { constructor(private readonly rolesService: RolesService) {} + private extractContext(req: Request) { + const user: any = req.user || {}; + return { + actorId: user.id || user.sub, + actorEmail: user.email, + ipAddress: req.ip, + userAgent: req.headers['user-agent'], + }; + } + @Post() async create( - @Body('name') name: string, - @Body('description') description?: string, - @Body('permissionIds') permissionIds?: string[], + @Body() createRoleDto: CreateRoleDto, + @Req() req: Request, ): Promise { - return this.rolesService.createRole(name, description, permissionIds); + return this.rolesService.createRole( + createRoleDto.name, + createRoleDto.description, + createRoleDto.permissionIds, + this.extractContext(req), + ); } @Get() @@ -28,31 +62,46 @@ export class RolesController { @Put(':id') async update( @Param('id') id: string, - @Body('name') name: string, - @Body('description') description?: string, - @Body('permissionIds') permissionIds?: string[], + @Body() updateRoleDto: UpdateRoleDto, + @Req() req: Request, ): Promise { - return this.rolesService.updateRole(id, name, description, permissionIds); + return this.rolesService.updateRole( + id, + updateRoleDto.name, + updateRoleDto.description, + updateRoleDto.permissionIds, + this.extractContext(req), + ); } @Delete(':id') - async remove(@Param('id') id: string): Promise { - return this.rolesService.deleteRole(id); + async remove(@Param('id') id: string, @Req() req: Request): Promise { + return this.rolesService.deleteRole(id, this.extractContext(req)); } @Post(':roleId/permissions/:permissionId') async addPermission( @Param('roleId') roleId: string, @Param('permissionId') permissionId: string, + @Req() req: Request, ): Promise { - return this.rolesService.addPermissionToRole(roleId, permissionId); + return this.rolesService.addPermissionToRole( + roleId, + permissionId, + this.extractContext(req), + ); } @Delete(':roleId/permissions/:permissionId') async removePermission( @Param('roleId') roleId: string, @Param('permissionId') permissionId: string, + @Req() req: Request, ): Promise { - return this.rolesService.removePermissionFromRole(roleId, permissionId); + return this.rolesService.removePermissionFromRole( + roleId, + permissionId, + this.extractContext(req), + ); } } From ec49a42b9e7fd01c657615d4ea2555b7544ac4a0 Mon Sep 17 00:00:00 2001 From: Adewale Iyanuoluwa Isaac Date: Wed, 29 Jul 2026 11:33:08 +0100 Subject: [PATCH 2/2] test(rbac): add e2e test for RolesController authorization gap --- test/security/roles-rbac.e2e-spec.ts | 72 ++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 test/security/roles-rbac.e2e-spec.ts diff --git a/test/security/roles-rbac.e2e-spec.ts b/test/security/roles-rbac.e2e-spec.ts new file mode 100644 index 00000000..691a668c --- /dev/null +++ b/test/security/roles-rbac.e2e-spec.ts @@ -0,0 +1,72 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { INestApplication, ExecutionContext } from '@nestjs/common'; +import request from 'supertest'; +import { AppModule } from '../../src/app.module'; +import { JwtAuthGuard } from '../../src/auth/guards/jwt-auth.guard'; +import { RolesGuard } from '../../src/auth/guards/roles.guard'; + +describe('RolesController RBAC Security (e2e)', () => { + let app: INestApplication; + + const mockJwtAuthGuard = { + canActivate: (context: ExecutionContext) => { + const req = context.switchToHttp().getRequest(); + const auth = req.headers.authorization; + if (!auth) return false; + if (auth === 'Bearer admin-token') { + req.user = { id: 'admin-1', email: 'admin@test.com', roles: ['admin'] }; + return true; + } + if (auth === 'Bearer user-token') { + req.user = { id: 'user-1', email: 'user@test.com', roles: ['user'] }; + return true; + } + return false; + }, + }; + + const mockRolesGuard = { + canActivate: (context: ExecutionContext) => { + const req = context.switchToHttp().getRequest(); + const user = req.user; + if (!user) return false; + if (user.roles?.includes('admin')) return true; + return false; + }, + }; + + beforeAll(async () => { + const moduleFixture: TestingModule = await Test.createTestingModule({ + imports: [AppModule], + }) + .overrideGuard(JwtAuthGuard) + .useValue(mockJwtAuthGuard) + .overrideGuard(RolesGuard) + .useValue(mockRolesGuard) + .compile(); + + app = moduleFixture.createNestApplication(); + await app.init(); + }); + + afterAll(async () => { + if (app) { + await app.close(); + } + }); + + it('should return 401 Unauthorized for unauthenticated POST /roles', async () => { + return request(app.getHttpServer()) + .post('/roles') + .send({ name: 'hacker-role', description: 'Malicious role' }) + .expect(401); + }); + + it('should return 403 Forbidden for authenticated non-admin POST /roles', async () => { + return request(app.getHttpServer()) + .post('/roles') + .set('Authorization', 'Bearer user-token') + .send({ name: 'hacker-role', description: 'Malicious role' }) + .expect(403); + }); +});