diff --git a/backend/docs/RBAC.md b/backend/docs/RBAC.md deleted file mode 100644 index f50deec..0000000 --- a/backend/docs/RBAC.md +++ /dev/null @@ -1,60 +0,0 @@ -# Role-Based Access Control - -The backend uses a route decorator plus guard for role-based access control. - -## Supported roles - -- `USER` -- `MODERATOR` -- `ADMIN` - -Canonical enum: `backend/src/users/enums/userRole.enum.ts` - -## Hierarchy - -- `ADMIN` inherits `MODERATOR` and `USER` permissions -- `MODERATOR` inherits `USER` permissions -- `USER` only has `USER` permissions - -## Basic usage - -```ts -@Roles(userRole.ADMIN) -@Post() -createPuzzle() {} -``` - -This returns `403 Forbidden` with: - -```txt -Access denied. Required role: ADMIN -``` - -## Multiple roles (OR logic) - -```ts -@Roles(userRole.ADMIN, userRole.MODERATOR) -@Get() -findAllUsers() {} -``` - -Any listed role is enough. - -## Ownership-aware access - -```ts -@Roles({ roles: [userRole.ADMIN], ownership: { param: 'id' } }) -@Patch(':id') -updateUser() {} -``` - -This allows either: - -- an `ADMIN` -- the authenticated user whose `userId` matches `req.params.id` - -## Notes - -- RBAC runs after authentication middleware and expects `request.user.userRole` -- Missing role information in the auth context is treated as a server error -- Denied access attempts are logged for audit/security review diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 1c09e73..756f436 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -27,7 +27,6 @@ import { ApiVersionService, } from './common/versioning'; import { DocsController } from './docs/docs.controller'; -import { RolesGuard } from './roles/roles.guard'; // const ENV = process.env.NODE_ENV; // console.log('NODE_ENV:', process.env.NODE_ENV); @@ -110,7 +109,7 @@ import { RolesGuard } from './roles/roles.guard'; HealthModule, ], controllers: [AppController, DocsController], - providers: [AppService, ApiVersionService, RolesGuard], + providers: [AppService, ApiVersionService], }) export class AppModule implements NestModule { /** diff --git a/backend/src/auth/interfaces/activeInterface.ts b/backend/src/auth/interfaces/activeInterface.ts index 8371ece..ccb040a 100644 --- a/backend/src/auth/interfaces/activeInterface.ts +++ b/backend/src/auth/interfaces/activeInterface.ts @@ -1,5 +1,3 @@ -import { userRole } from '../../users/enums/userRole.enum'; - /**Active user data interface */ export interface ActiveUserData { /**sub of type number */ @@ -7,7 +5,4 @@ export interface ActiveUserData { /**email of type string */ email?: string; - - /**authenticated user role */ - userRole?: userRole; } diff --git a/backend/src/auth/middleware/jwt-auth.middleware.ts b/backend/src/auth/middleware/jwt-auth.middleware.ts index 292d6ed..39de808 100644 --- a/backend/src/auth/middleware/jwt-auth.middleware.ts +++ b/backend/src/auth/middleware/jwt-auth.middleware.ts @@ -7,7 +7,6 @@ import { } from '@nestjs/common'; import { Request, Response, NextFunction } from 'express'; import * as jwt from 'jsonwebtoken'; -import { userRole } from '../../users/enums/userRole.enum'; /** * Interface for the Redis client to support token blacklisting. @@ -41,7 +40,7 @@ export interface JwtAuthMiddlewareOptions { export interface DecodedUserPayload { userId: string; email: string; - userRole: userRole; + userRole: string; [key: string]: any; } @@ -125,7 +124,7 @@ export class JwtAuthMiddleware implements NestMiddleware { const userPayload: DecodedUserPayload = { userId, email: decoded.email, - userRole: (decoded.userRole || decoded.role || userRole.USER) as userRole, + userRole: decoded.userRole || decoded.role, }; if (!userPayload.userId || !userPayload.email) { diff --git a/backend/src/progress/controllers/progress.controller.ts b/backend/src/progress/controllers/progress.controller.ts index e068bdf..199f084 100644 --- a/backend/src/progress/controllers/progress.controller.ts +++ b/backend/src/progress/controllers/progress.controller.ts @@ -22,8 +22,6 @@ import { CategoryStatsDto } from '../dtos/category-stats.dto'; import { OverallStatsDto } from '../dtos/overall-stats.dto'; import { ActiveUser } from '../../auth/decorators/activeUser.decorator'; import { ActiveUserData } from '../../auth/interfaces/activeInterface'; -import { Roles } from '../../roles/roles.decorator'; -import { userRole } from '../../users/enums/userRole.enum'; @Controller('progress') @ApiTags('Progress') @@ -37,7 +35,6 @@ export class ProgressController { ) {} @Get() - @Roles(userRole.USER) @ApiOperation({ summary: 'Get paginated progress history', description: @@ -65,7 +62,6 @@ export class ProgressController { } @Get('stats') - @Roles(userRole.USER) @ApiOperation({ summary: 'Get overall user statistics', description: @@ -85,7 +81,6 @@ export class ProgressController { } @Get('category/:id') - @Roles(userRole.USER) @ApiOperation({ summary: 'Get category-specific statistics', description: diff --git a/backend/src/puzzles/controllers/puzzles-v1.controller.ts b/backend/src/puzzles/controllers/puzzles-v1.controller.ts index 6bc5d51..e5529c2 100644 --- a/backend/src/puzzles/controllers/puzzles-v1.controller.ts +++ b/backend/src/puzzles/controllers/puzzles-v1.controller.ts @@ -10,8 +10,6 @@ import { PuzzlesService } from '../providers/puzzles.service'; import { CreatePuzzleDto } from '../dtos/create-puzzle.dto'; import { Puzzle } from '../entities/puzzle.entity'; import { PuzzleQueryDto } from '../dtos/puzzle-query.dto'; -import { Roles } from '../../roles/roles.decorator'; -import { userRole } from '../../users/enums/userRole.enum'; @Controller('puzzles') @Version('1') @@ -30,7 +28,6 @@ export class PuzzlesV1Controller { constructor(private readonly puzzlesService: PuzzlesService) {} @Post() - @Roles(userRole.ADMIN) @ApiOperation({ summary: 'Create a new puzzle (v1 contract)' }) @ApiResponse({ status: 201, diff --git a/backend/src/puzzles/controllers/puzzles-v2.controller.ts b/backend/src/puzzles/controllers/puzzles-v2.controller.ts index cbe9131..556c944 100644 --- a/backend/src/puzzles/controllers/puzzles-v2.controller.ts +++ b/backend/src/puzzles/controllers/puzzles-v2.controller.ts @@ -21,8 +21,6 @@ import { PuzzlesService } from '../providers/puzzles.service'; import { CreatePuzzleDto } from '../dtos/create-puzzle.dto'; import { Puzzle } from '../entities/puzzle.entity'; import { PuzzleDifficulty } from '../enums/puzzle-difficulty.enum'; -import { Roles } from '../../roles/roles.decorator'; -import { userRole } from '../../users/enums/userRole.enum'; class PuzzleV2QueryDto { @IsOptional() @@ -102,7 +100,6 @@ export class PuzzlesV2Controller { constructor(private readonly puzzlesService: PuzzlesService) {} @Post() - @Roles(userRole.ADMIN) @ApiOperation({ summary: 'Create a new puzzle (v2 contract)' }) @ApiResponse({ status: 201, diff --git a/backend/src/roles/roles.decorator.ts b/backend/src/roles/roles.decorator.ts index 670b0e4..786eaab 100644 --- a/backend/src/roles/roles.decorator.ts +++ b/backend/src/roles/roles.decorator.ts @@ -1,45 +1,5 @@ -import { applyDecorators, SetMetadata, UseGuards } from '@nestjs/common'; -import { ApiBearerAuth, ApiForbiddenResponse } from '@nestjs/swagger'; +import { SetMetadata } from '@nestjs/common'; import { userRole } from '../users/enums/userRole.enum'; -import { RolesGuard } from './roles.guard'; export const ROLES_KEY = 'roles'; - -export interface OwnershipRequirement { - param: string; - userIdField?: string; -} - -export interface RolesOptions { - roles: userRole[]; - ownership?: OwnershipRequirement; -} - -export function Roles(...roles: userRole[]): MethodDecorator & ClassDecorator; -export function Roles( - options: RolesOptions, -): MethodDecorator & ClassDecorator; -export function Roles( - ...rolesOrOptions: [RolesOptions] | userRole[] -): MethodDecorator & ClassDecorator { - const options = - typeof rolesOrOptions[0] === 'object' && !Array.isArray(rolesOrOptions[0]) - ? (rolesOrOptions[0] as RolesOptions) - : ({ - roles: rolesOrOptions as userRole[], - } satisfies RolesOptions); - - const readableRoles = options.roles.map((role) => role.toUpperCase()).join(' or '); - const forbiddenMessage = options.ownership - ? `Access denied. Required role: ${readableRoles} or ownership of this resource` - : `Access denied. Required role: ${readableRoles}`; - - return applyDecorators( - SetMetadata(ROLES_KEY, options), - UseGuards(RolesGuard), - ApiBearerAuth(), - ApiForbiddenResponse({ - description: forbiddenMessage, - }), - ); -} +export const Roles = (...roles: userRole[]) => SetMetadata(ROLES_KEY, roles); diff --git a/backend/src/roles/roles.guard.spec.ts b/backend/src/roles/roles.guard.spec.ts deleted file mode 100644 index 3eff9fc..0000000 --- a/backend/src/roles/roles.guard.spec.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { - ExecutionContext, - ForbiddenException, - InternalServerErrorException, -} from '@nestjs/common'; -import { Reflector } from '@nestjs/core'; -import { userRole } from '../users/enums/userRole.enum'; -import { RolesGuard } from './roles.guard'; -import { RolesOptions } from './roles.decorator'; - -describe('RolesGuard', () => { - let reflector: Reflector & { - getAllAndOverride: jest.Mock; - }; - - let guard: RolesGuard; - - beforeEach(() => { - jest.clearAllMocks(); - reflector = { - getAllAndOverride: jest.fn(), - } as unknown as Reflector & { - getAllAndOverride: jest.Mock; - }; - guard = new RolesGuard(reflector); - }); - - it('allows admins through user routes via hierarchy', () => { - mockRoles({ roles: [userRole.USER] }); - const context = createContext({ - user: { userId: '1', userRole: userRole.ADMIN }, - }); - - expect(guard.canActivate(context)).toBe(true); - }); - - it('allows access when any required role matches', () => { - mockRoles({ roles: [userRole.ADMIN, userRole.MODERATOR] }); - const context = createContext({ - user: { userId: '1', userRole: userRole.MODERATOR }, - }); - - expect(guard.canActivate(context)).toBe(true); - }); - - it('allows ownership-based access', () => { - mockRoles({ - roles: [userRole.ADMIN], - ownership: { param: 'id' }, - }); - const context = createContext({ - user: { userId: '42', userRole: userRole.USER }, - params: { id: '42' }, - }); - - expect(guard.canActivate(context)).toBe(true); - }); - - it('throws 403 with a clear message when access is denied', () => { - mockRoles({ roles: [userRole.ADMIN] }); - const context = createContext({ - user: { userId: '9', userRole: userRole.USER }, - }); - - expect(() => guard.canActivate(context)).toThrow( - new ForbiddenException('Access denied. Required role: ADMIN'), - ); - }); - - it('throws 500 when the role is missing from auth context', () => { - mockRoles({ roles: [userRole.ADMIN] }); - const context = createContext({ - user: { userId: '9' }, - }); - - expect(() => guard.canActivate(context)).toThrow( - InternalServerErrorException, - ); - }); -}); - -function mockRoles(options: RolesOptions) { - reflector.getAllAndOverride.mockReturnValue(options); -} - -function createContext(request: Record): ExecutionContext { - return { - getClass: jest.fn(), - getHandler: jest.fn(), - switchToHttp: () => ({ - getRequest: () => ({ - method: 'GET', - url: '/users/42', - originalUrl: '/users/42', - params: {}, - ...request, - }), - }), - } as unknown as ExecutionContext; -} diff --git a/backend/src/roles/roles.guard.ts b/backend/src/roles/roles.guard.ts index 2a5af36..bc10c82 100644 --- a/backend/src/roles/roles.guard.ts +++ b/backend/src/roles/roles.guard.ts @@ -1,116 +1,38 @@ import { CanActivate, ExecutionContext, - ForbiddenException, Injectable, - InternalServerErrorException, - Logger, + ForbiddenException, } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; -import { Request } from 'express'; +import { ROLES_KEY } from './roles.decorator'; import { userRole } from '../users/enums/userRole.enum'; -import { OwnershipRequirement, ROLES_KEY, RolesOptions } from './roles.decorator'; - -type AuthenticatedRequestUser = { - userId?: string; - sub?: string; - email?: string; - userRole?: userRole; - role?: userRole; - [key: string]: unknown; -}; - -type AuthenticatedRequest = Request & { - user?: AuthenticatedRequestUser; -}; - -const ROLE_HIERARCHY: Record = { - [userRole.ADMIN]: [userRole.ADMIN, userRole.MODERATOR, userRole.USER], - [userRole.MODERATOR]: [userRole.MODERATOR, userRole.USER], - [userRole.USER]: [userRole.USER], - [userRole.GUEST]: [userRole.GUEST], -}; @Injectable() export class RolesGuard implements CanActivate { - private readonly logger = new Logger(RolesGuard.name); - - constructor(private readonly reflector: Reflector) {} + constructor(private reflector: Reflector) {} canActivate(context: ExecutionContext): boolean { - const options = this.reflector.getAllAndOverride(ROLES_KEY, [ - context.getHandler(), - context.getClass(), - ]); - - if (!options || options.roles.length === 0) { - return true; - } - - const request = context.switchToHttp().getRequest(); - const user = request.user; - - if (!user) { - throw new ForbiddenException('Access denied. Authentication context is missing.'); - } - - const effectiveRole = user.userRole ?? user.role; - - if (!effectiveRole) { - this.logger.error( - `Authenticated user is missing a role on ${request.method} ${request.originalUrl ?? request.url}`, - ); - throw new InternalServerErrorException( - 'User role is missing from the authentication context.', - ); - } - - if (this.hasRequiredRole(effectiveRole, options.roles)) { - return true; - } - - if (this.isOwner(request, user, options.ownership)) { - return true; - } - - const requiredRoles = options.roles.map((role) => role.toUpperCase()).join(' or '); - const message = options.ownership - ? `Access denied. Required role: ${requiredRoles} or ownership of this resource` - : `Access denied. Required role: ${requiredRoles}`; - - this.logger.warn( - JSON.stringify({ - event: 'rbac_denied', - method: request.method, - path: request.originalUrl ?? request.url, - userId: user.userId ?? user.sub ?? null, - userRole: effectiveRole, - requiredRoles: options.roles, - ownershipParam: options.ownership?.param ?? null, - }), + const requiredRoles = this.reflector.getAllAndOverride( + ROLES_KEY, + [context.getHandler(), context.getClass()], ); - throw new ForbiddenException(message); - } - - private hasRequiredRole(currentRole: userRole, requiredRoles: userRole[]): boolean { - const allowedRoles = ROLE_HIERARCHY[currentRole] ?? [currentRole]; + if (!requiredRoles) return true; - return requiredRoles.some((requiredRole) => allowedRoles.includes(requiredRole)); - } + const request = context + .switchToHttp() + .getRequest<{ user?: { role?: userRole } }>(); + const user = request.user; - private isOwner( - request: AuthenticatedRequest, - user: AuthenticatedRequestUser, - ownership?: OwnershipRequirement, - ): boolean { - if (!ownership) { - return false; + if ( + !user || + user.role === undefined || + !requiredRoles.includes(user.role) + ) { + throw new ForbiddenException('Forbidden: Insufficient role'); } - const userId = user[ownership.userIdField ?? 'userId'] ?? user.userId ?? user.sub; - const resourceOwner = request.params?.[ownership.param]; - - return !!userId && !!resourceOwner && String(userId) === String(resourceOwner); + return true; } } diff --git a/backend/src/users/controllers/users.controller.ts b/backend/src/users/controllers/users.controller.ts index f9a8f1d..3972b52 100644 --- a/backend/src/users/controllers/users.controller.ts +++ b/backend/src/users/controllers/users.controller.ts @@ -7,7 +7,6 @@ import { Param, Body, Query, - UseGuards, } from '@nestjs/common'; import { UsersService } from '../providers/users.service'; import { XpLevelService } from '../providers/xp-level.service'; @@ -16,13 +15,9 @@ import { paginationQueryDto } from '../../common/pagination/paginationQueryDto'; import { EditUserDto } from '../dtos/editUserDto.dto'; import { CreateUserDto } from '../dtos/createUserDto'; import { User } from '../user.entity'; -import { RolesGuard } from '../../roles/roles.guard'; -import { Roles } from '../../roles/roles.decorator'; -import { userRole } from '../enums/userRole.enum'; @Controller('users') @ApiTags('users') -@UseGuards(RolesGuard) export class UsersController { constructor( private readonly usersService: UsersService, @@ -30,7 +25,6 @@ export class UsersController { ) {} @Delete(':id') - @Roles(userRole.ADMIN) @ApiOperation({ summary: 'Delete user by ID' }) @ApiResponse({ status: 200, description: 'User successfully deleted' }) @ApiResponse({ status: 404, description: 'User not found' }) @@ -57,13 +51,11 @@ export class UsersController { } @Get() - @Roles(userRole.ADMIN, userRole.MODERATOR) findAll(@Query() dto: paginationQueryDto) { return this.usersService.findAllUsers(dto); } @Get(':id') - @Roles({ roles: [userRole.ADMIN, userRole.MODERATOR], ownership: { param: 'id' } }) findOne(@Param('id') id: string) { return id; } @@ -81,7 +73,6 @@ export class UsersController { } @Patch(':id') - @Roles({ roles: [userRole.ADMIN], ownership: { param: 'id' } }) @ApiOperation({ summary: 'Update user by ID' }) @ApiResponse({ status: 200, description: 'user successfully updated' }) @ApiResponse({ status: 404, description: 'User not found' }) diff --git a/backend/src/users/enums/userRole.enum.ts b/backend/src/users/enums/userRole.enum.ts index 568bd2f..98ce5b1 100644 --- a/backend/src/users/enums/userRole.enum.ts +++ b/backend/src/users/enums/userRole.enum.ts @@ -1,6 +1,5 @@ export enum userRole { ADMIN = 'admin', - MODERATOR = 'moderator', USER = 'user', GUEST = 'guest', }