From 895ba927ccb9211969821da2871ad664d79d8f37 Mon Sep 17 00:00:00 2001 From: Patrick Schiller Date: Thu, 27 Aug 2026 15:17:30 +0200 Subject: [PATCH] fix(api): authenticate request approvals Signed-off-by: Codex --- .../src/api/vacation-workflow.e2e.spec.ts | 51 ++++++++ apps/api/openapi.json | 95 +++++++++++++++ .../src/app/employees/employees.controller.ts | 29 +++-- .../src/app/requests/requests.controller.ts | 114 ++++++++++++++---- apps/api/src/app/requests/requests.service.ts | 32 +++-- 5 files changed, 272 insertions(+), 49 deletions(-) diff --git a/apps/api-e2e/src/api/vacation-workflow.e2e.spec.ts b/apps/api-e2e/src/api/vacation-workflow.e2e.spec.ts index 4701903..0cdcb9a 100644 --- a/apps/api-e2e/src/api/vacation-workflow.e2e.spec.ts +++ b/apps/api-e2e/src/api/vacation-workflow.e2e.spec.ts @@ -74,6 +74,57 @@ describe('Vacation workflow — POST /api/requests/vacation + transitions', () = await ctx.reset(); }); + it('requires authentication for employee and request APIs', async () => { + await ctx.http.get('/api/employees').expect(401); + await ctx.http + .post('/api/requests') + .send({ + employeeId: '00000000-0000-0000-0000-000000000001', + type: 'TimeAdjustment', + from: `${YEAR}-08-03T09:00:00.000Z`, + to: `${YEAR}-08-03T11:00:00.000Z`, + }) + .expect(401); + }); + + it('derives requester and approver identities from the bearer token', async () => { + const cast = await setupOrgChart(ctx); + const outsider = await seedEmployee(ctx.prisma, { + personalNo: '0011', + firstName: 'Mara', + lastName: 'Schulz', + email: 'mara@test.local', + role: 'Manager', + managerId: cast.hannah.id, + }); + const outsiderToken = await login(ctx.http, outsider.email); + + const created = await ctx.http + .post('/api/requests') + .set('Authorization', `Bearer ${cast.anna.token}`) + .send({ + employeeId: cast.erik.id, + type: 'TimeAdjustment', + from: `${YEAR}-08-03T09:00:00.000Z`, + to: `${YEAR}-08-03T11:00:00.000Z`, + }) + .expect(201); + expect(created.body.employeeId).toBe(cast.anna.id); + + await ctx.http + .post(`/api/requests/${created.body.id}/approve`) + .set('Authorization', `Bearer ${outsiderToken}`) + .send({ actorId: cast.hannah.id }) + .expect(403); + + const approved = await ctx.http + .post(`/api/requests/${created.body.id}/approve`) + .set('Authorization', `Bearer ${cast.marc.token}`) + .send({ actorId: cast.hannah.id }) + .expect(201); + expect(approved.body.approverId).toBe(cast.marc.id); + }); + it('happy path without substitute: Submitted → PendingManager → Approved', async () => { const cast = await setupOrgChart(ctx); 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..53e936b 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,6 +27,8 @@ import { } from './requests.dto'; @ApiTags('requests') +@ApiBearerAuth() +@UseGuards(JwtAuthGuard) @Controller('requests') export class RequestsController { constructor(private readonly service: RequestsService) {} @@ -28,7 +42,14 @@ 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, + }); } @Get(':id') @@ -37,30 +58,46 @@ export class RequestsController { } @Get(':id/events') - events(@Param('id', new ParseUUIDPipe()) id: string): Promise { + events( + @Param('id', new ParseUUIDPipe()) id: string, + ): Promise { return this.service.events(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 +106,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 +195,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..c8bcbbf 100644 --- a/apps/api/src/app/requests/requests.service.ts +++ b/apps/api/src/app/requests/requests.service.ts @@ -242,12 +242,13 @@ export class RequestsService { const request = await this.assertRequest(id); if (request.type === 'Vacation') { // Vacation must use the multi-stage workflow. + await this.assertApproverScope(actorId, request); return this.transitionVacation(request, 'manager_approve', actorId, note); } 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.assertApproverScope(actorId, request); return this.transitionVacation( request, 'manager_approve_with_hr', @@ -255,7 +256,7 @@ export class RequestsService { note, ); } - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); const alreadyApproved = request.workflowState === 'Approved'; const updated = await this.prisma.$transaction(async (tx) => { const updated = await tx.request.update({ @@ -288,9 +289,10 @@ export class RequestsService { ): Promise { const request = await this.assertRequest(id); if (request.type === 'Vacation') { + await this.assertApproverScope(actorId, request); return this.transitionVacation(request, 'manager_reject', actorId, note); } - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); const updated = await this.prisma.$transaction(async (tx) => { const updated = await tx.request.update({ where: { id: request.id }, @@ -321,7 +323,7 @@ export class RequestsService { requiresHrConfirmation: boolean, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); // 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 +341,7 @@ export class RequestsService { note: string | null, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); return this.transitionVacation(request, 'manager_reject', actorId, note); } @@ -398,7 +400,7 @@ export class RequestsService { note: string, ): Promise { const request = await this.assertRequest(id); - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); return this.transitionVacation(request, 'manager_return', actorId, note); } @@ -424,7 +426,7 @@ export class RequestsService { try { const request = await this.assertRequest(id); if (request.workflowState === 'PendingManager') { - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); const forced = requiresTwoStageApproval(request); const event: WorkflowEvent = requiresHrConfirmation || forced @@ -489,7 +491,7 @@ export class RequestsService { try { const request = await this.assertRequest(id); if (request.workflowState === 'PendingManager') { - await this.assertApproverRole(actorId); + await this.assertApproverScope(actorId, request); const updated = await this.transitionVacation( request, 'manager_reject', @@ -661,13 +663,23 @@ export class RequestsService { return request; } - private async assertApproverRole(actorId: string): Promise { + private async assertApproverScope( + actorId: string, + request: Request, + ): Promise { const actor = await this.employees.getById(actorId); - if (actor.role !== 'Manager' && actor.role !== 'HRAdmin') { + if (actor.role === 'HRAdmin') return; + if (actor.role !== 'Manager') { throw new ForbiddenException( 'Only Manager or HRAdmin may approve/reject', ); } + const employee = await this.employees.getById(request.employeeId); + if (employee.managerId !== actorId) { + throw new ForbiddenException( + 'Managers may only approve/reject requests from their direct reports', + ); + } } private async assertHrAdminRole(actorId: string): Promise {