From 87e2edcbdecfce962db54ad2e596f86b0e749b63 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Fri, 7 Aug 2026 07:40:50 +0000 Subject: [PATCH] fix(auth): stop class guards shadowing route permission keys Nest runs class and method guards together, so a class-level view key ANDs with every action key below it. Staff granted only an action were denied before their key was ever checked: OCC could not fulfil wagon transfers, dispatchers could not create a yard, and track staff could not assign first/last-mile vehicles. Reads now carry the view key themselves, and the warehouses baseline lists every key its routes use. --- .../src/modules/first-mile/first-mile.controller.ts | 6 +++++- .../src/modules/last-mile/last-mile.controller.ts | 8 +++++++- .../wagons/wagon-transfer-requests.controller.ts | 7 ++++++- .../src/modules/warehouses/warehouses.controller.ts | 10 ++++++++-- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/apps/edr-freight-api/src/modules/first-mile/first-mile.controller.ts b/apps/edr-freight-api/src/modules/first-mile/first-mile.controller.ts index 952f924d6..e6610a59d 100644 --- a/apps/edr-freight-api/src/modules/first-mile/first-mile.controller.ts +++ b/apps/edr-freight-api/src/modules/first-mile/first-mile.controller.ts @@ -28,7 +28,9 @@ import { FirstMileInvoiceService } from './first-mile-invoice.service'; @ApiTags('first-mile') @ApiBearerAuth() @Controller('first-mile') -@BookingStaff(FREIGHT_PERMS.firstMile.view) +// No class-level key: Nest stacks class and method guards, so a class-level +// `view` would AND with every action key below and lock out staff granted only +// an action (e.g. assign_vehicles). Each route carries its own key instead. export class FirstMileController { constructor( private readonly firstMileService: FirstMileService, @@ -36,6 +38,7 @@ export class FirstMileController { ) { } @Get() + @BookingStaff(FREIGHT_PERMS.firstMile.view) @ApiOperation({ summary: 'List first-mile legs' }) findAll( @Query('status') status?: string, @@ -58,6 +61,7 @@ export class FirstMileController { } @Get(':id') + @BookingStaff(FREIGHT_PERMS.firstMile.view) @ApiOperation({ summary: 'Get a first-mile leg by ID' }) findOne(@Param('id', ParseUUIDPipe) id: string) { return this.firstMileService.findById(id); diff --git a/apps/edr-freight-api/src/modules/last-mile/last-mile.controller.ts b/apps/edr-freight-api/src/modules/last-mile/last-mile.controller.ts index d7e2ba1ff..7a2765fd8 100644 --- a/apps/edr-freight-api/src/modules/last-mile/last-mile.controller.ts +++ b/apps/edr-freight-api/src/modules/last-mile/last-mile.controller.ts @@ -34,7 +34,9 @@ import { LastMileInvoiceService } from './last-mile-invoice.service'; @ApiTags('last-mile') @ApiBearerAuth() @Controller('last-mile') -@BookingStaff(FREIGHT_PERMS.lastMile.view) +// No class-level key: Nest stacks class and method guards, so a class-level +// `view` would AND with every action key below and lock out staff granted only +// an action (e.g. assign_vehicles). Each route carries its own key instead. export class LastMileController { constructor( private readonly lastMileService: LastMileService, @@ -42,6 +44,7 @@ export class LastMileController { ) {} @Get() + @BookingStaff(FREIGHT_PERMS.lastMile.view) @ApiOperation({ summary: 'List last-mile legs' }) findAll( @Query('status') status?: string, @@ -64,18 +67,21 @@ export class LastMileController { } @Get(':id') + @BookingStaff(FREIGHT_PERMS.lastMile.view) @ApiOperation({ summary: 'Get a last-mile leg by ID' }) findOne(@Param('id', ParseUUIDPipe) id: string) { return this.lastMileService.findById(id); } @Get('booking/:bookingId/arrival-trucks') + @BookingStaff(FREIGHT_PERMS.lastMile.view) @ApiOperation({ summary: "Assigned EDR last-mile trucks for a booking (arrival/exit weighing prefill)" }) arrivalTrucks(@Param('bookingId', ParseUUIDPipe) bookingId: string) { return this.lastMileService.arrivalTrucksForBooking(bookingId); } @Get('booking/:bookingId/remaining-tons') + @BookingStaff(FREIGHT_PERMS.lastMile.view) @ApiOperation({ summary: 'Bulk drawdown: tonnage still to be hauled (total − departed trucks)' }) remainingTons(@Param('bookingId', ParseUUIDPipe) bookingId: string) { return this.lastMileService.remainingTonsForBooking(bookingId); diff --git a/apps/edr-freight-api/src/modules/wagons/wagon-transfer-requests.controller.ts b/apps/edr-freight-api/src/modules/wagons/wagon-transfer-requests.controller.ts index 9e69c3bf6..5525132a3 100644 --- a/apps/edr-freight-api/src/modules/wagons/wagon-transfer-requests.controller.ts +++ b/apps/edr-freight-api/src/modules/wagons/wagon-transfer-requests.controller.ts @@ -41,7 +41,9 @@ const toInt = (value?: string): number | undefined => { */ @ApiTags('wagon-transfer-requests') @Controller('wagon-transfer-requests') -@WagonTransferView() +// No class-level key: Nest stacks class and method guards, so a class-level +// `transfer_view` would AND with every action key below and lock out the OCC +// staff granted only `transfer_fulfill`. Reads carry the view key themselves. export class WagonTransferRequestsController { constructor(private readonly service: WagonTransferRequestsService) {} @@ -56,6 +58,7 @@ export class WagonTransferRequestsController { } @Get() + @WagonTransferView() @ApiOperation({ summary: 'Transfer desk list — paginated, filterable by status (comma-separated), yards and wagon type', @@ -84,6 +87,7 @@ export class WagonTransferRequestsController { // matches in declaration order, so `/history` would otherwise be captured by // the `:id` param route (and rejected by ParseUUIDPipe). @Get('history') + @WagonTransferView() @ApiQuery({ name: 'page', required: false }) @ApiQuery({ name: 'pageSize', required: false }) @ApiOperation({ @@ -129,6 +133,7 @@ export class WagonTransferRequestsController { } @Get(':id') + @WagonTransferView() @ApiOperation({ summary: 'Get one transfer request' }) findOne(@Param('id', ParseUUIDPipe) id: string) { return this.service.findById(id); diff --git a/apps/edr-freight-api/src/modules/warehouses/warehouses.controller.ts b/apps/edr-freight-api/src/modules/warehouses/warehouses.controller.ts index 3ee381a8c..f3d894dfc 100644 --- a/apps/edr-freight-api/src/modules/warehouses/warehouses.controller.ts +++ b/apps/edr-freight-api/src/modules/warehouses/warehouses.controller.ts @@ -14,13 +14,19 @@ import { WarehousesService } from './warehouses.service'; @ApiTags('warehouses') @ApiBearerAuth() // Baseline read: warehouse reference data is consumed by inventory/dashboard -// flows too, so any of the three view permissions grants reads. Writes stack -// their specific create/update permission per route on top. +// flows too, so any of the view permissions grants reads. Writes stack their +// specific create/update permission per route on top — which means every key +// used by a route below must also appear here, or the class guard denies +// before the route's own key is ever consulted (Nest ANDs the two). @Controller('warehouses') @BookingStaff([ FREIGHT_PERMS.warehouses.view, FREIGHT_PERMS.warehouseInventory.view, FREIGHT_PERMS.warehouseDashboard.view, + FREIGHT_PERMS.warehouses.create, + FREIGHT_PERMS.warehouses.update, + FREIGHT_PERMS.warehouseYards.view, + FREIGHT_PERMS.warehouseYards.create, ]) export class WarehousesController { constructor(