Merge pull request #941 from Tria-plc/freight/feat/fixes-v1

Freight/feat/fixes v1

apps/edr-freight-api/src/modules/bookings/bookings.controller.ts

@@ -252,8 +252,11 @@ export class BookingsController {
     return this.bookingsService.findAll(filter, companyId);
   }
 
+  // Powers the customer-detail bookings tab, so `customers:view` reaches it too
+  // — otherwise a staffer granted only the customer permission gets a page whose
+  // tabs 403 individually.
   @Get("by-company/:companyId/customer-view")
-  @BookingView()
+  @BookingStaff([FREIGHT_PERMS.customers.view, FREIGHT_PERMS.bookings.view])
   @ApiOperation({
     summary: "List bookings for a company (customer-view shape, backoffice)",
   })

apps/edr-freight-api/src/modules/companies/companies.controller.ts

@@ -11,13 +11,22 @@ import {
   HttpCode,
   HttpStatus,
   UseInterceptors,
+  UseGuards,
   UploadedFiles,
   BadRequestException,
+  NotFoundException,
 } from "@nestjs/common";
 import { AnyFilesInterceptor } from "@nestjs/platform-express";
 import { ApiOperation, ApiTags, ApiConsumes } from "@nestjs/swagger";
 import { CurrentUser } from "@edr/api-common";
-import { FreightAdmin } from "../../common/booking-guards";
+import type { TCurrentUser } from "@tria-plc/api-common/modules/auth/types/current-user.type";
+import { JwtGuard } from "@tria-plc/api-common/modules/auth/services/jwt.guard";
+import { BookingStaff } from "../../common/booking-guards";
+import {
+  assertFreightPermission,
+  hasFreightPermission,
+} from "../../common/freight-permission.util";
+import { FREIGHT_PERMS } from "../../seed/freight-permissions.registry";
 import { FilesService } from "../files/files.service";
 import { CompaniesService } from "./companies.service";
 import { CreateCompanyDto } from "./dto/create-company.dto";
@@ -59,6 +68,23 @@ interface CurrentIamUser {
   phoneNumber?: string;
 }
 
+/**
+ * Which permission a status write needs. Approving/reactivating is a different
+ * authority from suspending, but both arrive on the same route with the target
+ * in the BODY — a route-level guard can't tell them apart, so the handlers
+ * assert against this map instead.
+ *
+ * Keyed by string so it serves both `CompanyStatus` and `ProfileStatus`
+ * (a superset: it adds `rejected`).
+ */
+const STATUS_PERM: Record<string, string> = {
+  active: FREIGHT_PERMS.customers.verify,
+  pending: FREIGHT_PERMS.customers.verify,
+  rejected: FREIGHT_PERMS.customers.verify,
+  suspended: FREIGHT_PERMS.customers.deactivate,
+  blacklisted: FREIGHT_PERMS.customers.deactivate,
+};
+
 @ApiTags("Companies")
 @Controller("companies")
 export class CompaniesController {
@@ -410,7 +436,7 @@ export class CompaniesController {
 
   // Used by backoffice
   @Post()
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.create)
   @ApiOperation({
     summary:
       "Create a new company (customer, freight_forwarder, dj_freight_forwarder, transporter)",
@@ -421,12 +447,14 @@ export class CompaniesController {
   }
 
   @Get("stats")
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "Company counts by status (KPI strip)" })
   async getStats(): Promise<CompanyStatsResponseDto> {
     return this.companiesService.getCompanyStats();
   }
 
   @Get()
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "List companies (paginated, filterable)" })
   async findAll(
     @Query() query: ListCompaniesQueryDto,
@@ -436,6 +464,7 @@ export class CompaniesController {
   }
 
   @Get(":id")
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "Get company by ID" })
   async findById(
     @Param("id", ParseUUIDPipe) id: string,
@@ -446,30 +475,77 @@ export class CompaniesController {
     return dto;
   }
 
+  /**
+   * Edits fields AND carries `status`, so it spans two authorities. The route
+   * guard is one-of (a status-only caller must get in); the asserts below are
+   * what actually authorize: touching `status` needs the permission
+   * {@link STATUS_PERM} maps it to, touching anything else needs
+   * `customers:update`. Both checks are required — without the second, a
+   * caller holding only `customers:deactivate` could rename the company.
+   */
   @Patch(":id")
-  @FreightAdmin()
+  @BookingStaff([
+    FREIGHT_PERMS.customers.update,
+    FREIGHT_PERMS.customers.verify,
+    FREIGHT_PERMS.customers.deactivate,
+  ])
   @ApiOperation({ summary: "Update a company" })
   async update(
     @Param("id", ParseUUIDPipe) id: string,
     @Body() dto: UpdateCompanyDto,
+    @CurrentUser() user: TCurrentUser,
   ): Promise<ResponseCompanyDto> {
+    const { status, ...fields } = dto;
+    if (status) assertFreightPermission(user, STATUS_PERM[status]);
+    if (Object.keys(fields).length > 0) {
+      assertFreightPermission(user, FREIGHT_PERMS.customers.update);
+    }
     const company = await this.companiesService.updateCompany(id, dto);
     return new ResponseCompanyDto(company);
   }
 
   @Delete(":id")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.deactivate)
   @ApiOperation({ summary: "Soft-delete a company" })
   @HttpCode(HttpStatus.NO_CONTENT)
   async remove(@Param("id", ParseUUIDPipe) id: string): Promise<void> {
     await this.companiesService.deleteCompany(id);
   }
 
+  /**
+   * Dual-audience: staff read any customer's documents, and the portal reads
+   * its OWN during onboarding (`companiesService.getDocuments`). So the route
+   * is authenticated-only and the split happens here — same shape as
+   * `GET /contracts/:id`. Gating it on a staff permission alone would 403 every
+   * customer on their own documents.
+   *
+   * The staff arm is one-of because two pages consume it: the customer detail
+   * page (`customers:view`) and the contract-request detail page, whose route
+   * is gated on `contracts:view` — a contract reviewer without the customer
+   * permission still needs the applicant's documents.
+   */
   @Get(":companyId/documents")
+  @UseGuards(JwtGuard)
   @ApiOperation({ summary: "List documents uploaded for a company" })
   async listDocuments(
     @Param("companyId", ParseUUIDPipe) companyId: string,
+    @CurrentUser() user: TCurrentUser,
   ) {
+    const isStaff = [
+      FREIGHT_PERMS.customers.view,
+      FREIGHT_PERMS.contracts.view,
+      FREIGHT_PERMS.bookings.view,
+    ].some((p) => hasFreightPermission(user, p));
+
+    if (!isStaff) {
+      const { company } = await this.companiesService.getCompanyInfoByUserId(
+        user.id,
+      );
+      // Hidden as NotFound rather than Forbidden so company ids can't be probed.
+      if (company.id !== companyId) {
+        throw new NotFoundException(`Company ${companyId} not found`);
+      }
+    }
     const files = await this.filesService.findByResource(companyId, "companies");
     return Promise.all(
       files.map(async (f) => ({
@@ -490,7 +566,7 @@ export class CompaniesController {
   }
 
   @Post("documents/:fileId/request-change")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.verify)
   @ApiOperation({
     summary: "Ask the customer to correct one uploaded document",
     description:
@@ -532,14 +608,23 @@ export class CompaniesController {
     return this.companiesService.uploadCompanyDocuments(companyId, files, user.id);
   }
 
+  /**
+   * Approve / reject / suspend / blacklist all arrive here with the target in
+   * the body, so authorization is per-status via {@link STATUS_PERM} rather
+   * than on the route (the guard is only the one-of gate).
+   */
   @Patch("company-profiles/:profileId/status")
-  @FreightAdmin()
+  @BookingStaff([
+    FREIGHT_PERMS.customers.verify,
+    FREIGHT_PERMS.customers.deactivate,
+  ])
   @ApiOperation({ summary: "Update a company profile's approval status" })
   async updateCompanyProfileStatus(
-    @CurrentUser() user: CurrentIamUser,
+    @CurrentUser() user: TCurrentUser,
     @Param("profileId", ParseUUIDPipe) profileId: string,
     @Body() dto: UpdateCompanyProfileStatusDto,
   ): Promise<ResponseCompanyProfileDto> {
+    assertFreightPermission(user, STATUS_PERM[dto.status]);
     const profile = await this.companiesService.setCompanyProfileStatus(
       profileId,
       dto.status,
@@ -550,7 +635,7 @@ export class CompaniesController {
   }
 
   @Get(":companyId/change-requests")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "List a company's profile change requests" })
   async listChangeRequests(
     @Param("companyId", ParseUUIDPipe) companyId: string,
@@ -560,7 +645,7 @@ export class CompaniesController {
   }
 
   @Post("change-requests/:id/approve")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.verify)
   @ApiOperation({
     summary: "Approve a pending profile change request (applies the changes)",
   })
@@ -576,7 +661,7 @@ export class CompaniesController {
   }
 
   @Post("change-requests/:id/reject")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.verify)
   @ApiOperation({
     summary: "Reject a pending profile change request with a note",
   })
@@ -594,7 +679,7 @@ export class CompaniesController {
   }
 
   @Post(":companyId/profiles")
-  @FreightAdmin()
+  @BookingStaff(FREIGHT_PERMS.customers.update)
   @ApiOperation({ summary: "Add a profile (employee) to a company" })
   async createProfile(
     @Param("companyId", ParseUUIDPipe) companyId: string,
@@ -608,6 +693,7 @@ export class CompaniesController {
   }
 
   @Get(":companyId/profiles")
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "List profiles for a company" })
   async listProfiles(
     @Param("companyId", ParseUUIDPipe) companyId: string,
@@ -618,6 +704,7 @@ export class CompaniesController {
   }
 
   @Get("profile/user/:userId")
+  @BookingStaff(FREIGHT_PERMS.customers.view)
   @ApiOperation({ summary: "Get profile by IAM user ID" })
   async findProfileByUser(
     @Param("userId", ParseUUIDPipe) userId: string,

apps/edr-freight-api/src/modules/payment/payment.controller.ts

@@ -16,7 +16,8 @@ import {
 } from "@nestjs/swagger";
 import { Response } from "express";
 import { Public } from "@edr/api-common";
-import { BookingView } from "../../common/booking-guards";
+import { BookingStaff, BookingView } from "../../common/booking-guards";
+import { FREIGHT_PERMS } from "../../seed/freight-permissions.registry";
 import { PaymentService } from "./payment.service";
 import { IntentStatusDto } from "./payments.dto";
 
@@ -25,7 +26,9 @@ import { IntentStatusDto } from "./payments.dto";
 export class PaymentController {
     constructor(private readonly paymentService: PaymentService) { }
 
+    // Customer-detail payments tab — same one-of rule as the bookings tab.
     @Get("by-company/:companyId/customer-view")
+    @BookingStaff([FREIGHT_PERMS.customers.view, FREIGHT_PERMS.payments.view])
     @ApiOperation({ summary: "List payments for a company (customer-view shape, backoffice)" })
     findByCompanyCustomerView(
         @Param("companyId", ParseUUIDPipe) companyId: string,