From cddf4cf0a3b0ecbcccab6650c5a89e7cc792f3f0 Mon Sep 17 00:00:00 2001 From: Patrick Schiller Date: Thu, 27 Aug 2026 15:10:05 +0200 Subject: [PATCH] fix(api): secure request workflow identities Signed-off-by: Codex --- apps/api/openapi.json | 95 +++++++++++++ .../src/app/employees/employees.controller.ts | 29 ++-- .../src/app/requests/requests.controller.ts | 128 ++++++++++++++---- apps/api/src/app/requests/requests.service.ts | 80 +++++++++-- 4 files changed, 275 insertions(+), 57 deletions(-) diff --git a/apps/api/openapi.json b/apps/api/openapi.json index c4d3e79..700d7a9 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -126,6 +126,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["employees"] }, "post": { @@ -172,6 +177,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["employees"] }, "put": { @@ -1794,6 +1804,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] }, "post": { @@ -1814,6 +1829,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1835,6 +1855,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1856,6 +1881,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1878,6 +1908,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1909,6 +1944,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1940,6 +1980,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -1971,6 +2016,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2002,6 +2052,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2033,6 +2088,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2064,6 +2124,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2095,6 +2160,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2126,6 +2196,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2157,6 +2232,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2188,6 +2268,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2210,6 +2295,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, @@ -2232,6 +2322,11 @@ "description": "" } }, + "security": [ + { + "bearer": [] + } + ], "tags": ["requests"] } }, diff --git a/apps/api/src/app/employees/employees.controller.ts b/apps/api/src/app/employees/employees.controller.ts index ba9b98a..06eed61 100644 --- a/apps/api/src/app/employees/employees.controller.ts +++ b/apps/api/src/app/employees/employees.controller.ts @@ -25,12 +25,16 @@ import { } from './employees.dto'; @ApiTags('employees') +@ApiBearerAuth() +@UseGuards(JwtAuthGuard) @Controller('employees') export class EmployeesController { constructor(private readonly employees: EmployeesService) {} @Get() - list(@Query('includeInactive') includeInactive?: string): Promise { + list( + @Query('includeInactive') includeInactive?: string, + ): Promise { return this.employees.list({ includeInactive: includeInactive === 'true' }); } @@ -40,16 +44,14 @@ export class EmployeesController { } @Post() - @ApiBearerAuth() - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(RolesGuard) @Roles('HRAdmin') create(@Body() dto: CreateEmployeeDto): Promise { return this.employees.create(dto); } @Put(':id') - @ApiBearerAuth() - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(RolesGuard) @Roles('HRAdmin') update( @Param('id', new ParseUUIDPipe()) id: string, @@ -59,8 +61,7 @@ export class EmployeesController { } @Post(':id/password') - @ApiBearerAuth() - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(RolesGuard) @Roles('HRAdmin') @HttpCode(HttpStatus.NO_CONTENT) setPassword( @@ -71,18 +72,20 @@ export class EmployeesController { } @Delete(':id') - @ApiBearerAuth() - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(RolesGuard) @Roles('HRAdmin') - deactivate(@Param('id', new ParseUUIDPipe()) id: string): Promise { + deactivate( + @Param('id', new ParseUUIDPipe()) id: string, + ): Promise { return this.employees.deactivate(id); } @Post(':id/reactivate') - @ApiBearerAuth() - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(RolesGuard) @Roles('HRAdmin') - reactivate(@Param('id', new ParseUUIDPipe()) id: string): Promise { + reactivate( + @Param('id', new ParseUUIDPipe()) id: string, + ): Promise { return this.employees.reactivate(id); } } diff --git a/apps/api/src/app/requests/requests.controller.ts b/apps/api/src/app/requests/requests.controller.ts index 108e1f5..ab41a63 100644 --- a/apps/api/src/app/requests/requests.controller.ts +++ b/apps/api/src/app/requests/requests.controller.ts @@ -1,5 +1,17 @@ -import { Body, Controller, Get, Param, ParseUUIDPipe, Post, Query } from '@nestjs/common'; -import { ApiTags } from '@nestjs/swagger'; +import { + Body, + Controller, + Get, + Param, + ParseUUIDPipe, + Post, + Query, + UseGuards, +} from '@nestjs/common'; +import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { CurrentUser } from '../auth/current-user.decorator'; +import { JwtAuthGuard } from '../auth/jwt-auth.guard'; +import type { JwtUser } from '../auth/jwt.strategy'; import { RequestsService } from './requests.service'; import { BulkApproveDto, @@ -15,12 +27,15 @@ import { } from './requests.dto'; @ApiTags('requests') +@ApiBearerAuth() +@UseGuards(JwtAuthGuard) @Controller('requests') export class RequestsController { constructor(private readonly service: RequestsService) {} @Get() list( + @CurrentUser() user: JwtUser, @Query('employeeId') employeeId?: string, @Query('status') status?: string, @Query('workflowState') workflowState?: string, @@ -28,39 +43,69 @@ export class RequestsController { @Query('currentApproverId') currentApproverId?: string, @Query('substituteId') substituteId?: string, ): Promise { - return this.service.list({ employeeId, status, workflowState, approverId, currentApproverId, substituteId }); + return this.service.list( + { + employeeId, + status, + workflowState, + approverId, + currentApproverId, + substituteId, + }, + user.id, + ); } @Get(':id') - get(@Param('id', new ParseUUIDPipe()) id: string): Promise { - return this.service.getById(id); + get( + @Param('id', new ParseUUIDPipe()) id: string, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.getById(id, user.id); } @Get(':id/events') - events(@Param('id', new ParseUUIDPipe()) id: string): Promise { - return this.service.events(id); + events( + @Param('id', new ParseUUIDPipe()) id: string, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.events(id, user.id); } @Post() - create(@Body() dto: CreateRequestDto): Promise { - return this.service.create(dto); + create( + @Body() dto: CreateRequestDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.create({ ...dto, employeeId: user.id }); } @Post('vacation') - createVacation(@Body() dto: CreateVacationDto): Promise { - return this.service.createVacation(dto); + createVacation( + @Body() dto: CreateVacationDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.createVacation({ ...dto, employeeId: user.id }); } // ----- Generic approve / reject (legacy convenience) ----- @Post(':id/approve') - approve(@Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto): Promise { - return this.service.approve(id, body.actorId, body.note ?? null); + approve( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.approve(id, user.id, body.note ?? null); } @Post(':id/reject') - reject(@Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto): Promise { - return this.service.reject(id, body.actorId, body.note ?? null); + reject( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.reject(id, user.id, body.note ?? null); } // ----- Vacation workflow ----- @@ -69,66 +114,88 @@ export class RequestsController { managerApprove( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: ManagerApproveDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.managerApprove(id, body.actorId, body.note ?? null, !!body.requiresHrConfirmation); + return this.service.managerApprove( + id, + user.id, + body.note ?? null, + !!body.requiresHrConfirmation, + ); } @Post(':id/manager-reject') managerReject( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.managerReject(id, body.actorId, body.note ?? null); + return this.service.managerReject(id, user.id, body.note ?? null); } @Post(':id/hr-confirm') - hrConfirm(@Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto): Promise { - return this.service.hrConfirm(id, body.actorId, body.note ?? null); + hrConfirm( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.hrConfirm(id, user.id, body.note ?? null); } @Post(':id/hr-reject') hrReject( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.hrReject(id, body.actorId, body.note); + return this.service.hrReject(id, user.id, body.note); } @Post(':id/substitute/accept') substituteAccept( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.substituteAccept(id, body.actorId, body.note ?? null); + return this.service.substituteAccept(id, user.id, body.note ?? null); } @Post(':id/substitute/decline') substituteDecline( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.substituteDecline(id, body.actorId, body.note); + return this.service.substituteDecline(id, user.id, body.note); } @Post(':id/return') returnForRevision( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, + @CurrentUser() user: JwtUser, ): Promise { - return this.service.returnForRevision(id, body.actorId, body.note); + return this.service.returnForRevision(id, user.id, body.note); } @Post(':id/cancel') - cancel(@Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto): Promise { - return this.service.cancel(id, body.actorId, body.note ?? null); + cancel( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() body: TransitionDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.cancel(id, user.id, body.note ?? null); } // ----- Bulk ----- @Post('bulk-approve') - bulkApprove(@Body() body: BulkApproveDto): Promise { + bulkApprove( + @Body() body: BulkApproveDto, + @CurrentUser() user: JwtUser, + ): Promise { return this.service.bulkApprove( - body.actorId, + user.id, body.ids, body.note ?? null, !!body.requiresHrConfirmation, @@ -136,7 +203,10 @@ export class RequestsController { } @Post('bulk-reject') - bulkReject(@Body() body: BulkRejectDto): Promise { - return this.service.bulkReject(body.actorId, body.ids, body.note); + bulkReject( + @Body() body: BulkRejectDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.bulkReject(user.id, body.ids, body.note); } } diff --git a/apps/api/src/app/requests/requests.service.ts b/apps/api/src/app/requests/requests.service.ts index fd23ceb..5036c75 100644 --- a/apps/api/src/app/requests/requests.service.ts +++ b/apps/api/src/app/requests/requests.service.ts @@ -53,7 +53,10 @@ export class RequestsService { private readonly schedules: WorkSchedulesService, ) {} - async list(filter: ListRequestsFilter): Promise { + async list( + filter: ListRequestsFilter, + actorId: string, + ): Promise { const where: Prisma.RequestWhereInput = {}; if (filter.employeeId) where.employeeId = filter.employeeId; if (filter.status) @@ -66,19 +69,22 @@ export class RequestsService { where.currentApproverId = filter.currentApproverId; if (filter.substituteId) where.substituteId = filter.substituteId; const rows = await this.prisma.request.findMany({ - where, + where: { AND: [where, await this.visibilityWhere(actorId)] }, orderBy: { createdAt: 'desc' }, take: 200, }); return rows.map(toRequestDto); } - async getById(id: string): Promise { - return toRequestDto(await this.assertRequest(id)); + async getById(id: string, actorId: string): Promise { + const request = await this.assertRequest(id); + await this.assertCanView(request, actorId); + return toRequestDto(request); } - async events(id: string): Promise { - await this.assertRequest(id); + async events(id: string, actorId: string): Promise { + const request = await this.assertRequest(id); + await this.assertCanView(request, actorId); const rows = await this.prisma.requestEvent.findMany({ where: { requestId: id }, orderBy: { occurredAt: 'asc' }, @@ -247,7 +253,7 @@ export class RequestsService { if (requiresTwoStageApproval(request)) { // Off-hours TimeAdjustment: manager approves the off-hours allowance, // then HR finalises the actual time correction. - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); return this.transitionVacation( request, 'manager_approve_with_hr', @@ -255,7 +261,7 @@ export class RequestsService { note, ); } - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); const alreadyApproved = request.workflowState === 'Approved'; const updated = await this.prisma.$transaction(async (tx) => { const updated = await tx.request.update({ @@ -290,7 +296,7 @@ export class RequestsService { if (request.type === 'Vacation') { return this.transitionVacation(request, 'manager_reject', actorId, note); } - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); const updated = await this.prisma.$transaction(async (tx) => { const updated = await tx.request.update({ where: { id: request.id }, @@ -321,7 +327,7 @@ export class RequestsService { requiresHrConfirmation: boolean, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); // Off-hours TimeAdjustments always need HR confirmation, regardless of the // flag set by the manager — the spec calls for a "Sondergenehmigung" first, // then the actual time correction. @@ -339,7 +345,7 @@ export class RequestsService { note: string | null, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); return this.transitionVacation(request, 'manager_reject', actorId, note); } @@ -398,7 +404,7 @@ export class RequestsService { note: string, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); return this.transitionVacation(request, 'manager_return', actorId, note); } @@ -424,7 +430,7 @@ export class RequestsService { try { const request = await this.assertRequest(id); if (request.workflowState === 'PendingManager') { - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); const forced = requiresTwoStageApproval(request); const event: WorkflowEvent = requiresHrConfirmation || forced @@ -489,7 +495,7 @@ export class RequestsService { try { const request = await this.assertRequest(id); if (request.workflowState === 'PendingManager') { - await this.assertApproverRole(actorId); + await this.assertApprover(request, actorId); const updated = await this.transitionVacation( request, 'manager_reject', @@ -543,6 +549,11 @@ export class RequestsService { if (request.employeeId !== actorId) { // HR/Manager may also cancel — check role. const actor = await this.employees.getById(actorId); + if (actor.role === 'Manager' && request.currentApproverId !== actorId) { + throw new ForbiddenException( + 'Managers may only cancel assigned requests', + ); + } if (actor.role !== 'Manager' && actor.role !== 'HRAdmin') { throw new ForbiddenException( 'Only the requester or a Manager/HRAdmin may cancel', @@ -661,13 +672,52 @@ export class RequestsService { return request; } - private async assertApproverRole(actorId: string): Promise { + private async assertApprover( + request: Request, + actorId: string, + ): Promise { const actor = await this.employees.getById(actorId); if (actor.role !== 'Manager' && actor.role !== 'HRAdmin') { throw new ForbiddenException( 'Only Manager or HRAdmin may approve/reject', ); } + if (actor.role === 'Manager' && request.currentApproverId !== actorId) { + throw new ForbiddenException( + 'Managers may only transition assigned requests', + ); + } + } + + private async visibilityWhere( + actorId: string, + ): Promise { + const actor = await this.employees.getById(actorId); + if (actor.role === 'HRAdmin') return {}; + return { + OR: [ + { employeeId: actorId }, + { approverId: actorId }, + { currentApproverId: actorId }, + { substituteId: actorId }, + ], + }; + } + + private async assertCanView( + request: Request, + actorId: string, + ): Promise { + const actor = await this.employees.getById(actorId); + if ( + actor.role !== 'HRAdmin' && + request.employeeId !== actorId && + request.approverId !== actorId && + request.currentApproverId !== actorId && + request.substituteId !== actorId + ) { + throw new ForbiddenException('Not permitted to view this request'); + } } private async assertHrAdminRole(actorId: string): Promise {