From 6420c72e89e2462c19adde17bbb25e31d787b8c0 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 20 Jul 2026 11:20:50 +0000 Subject: [PATCH 1/6] feat(auth): implement staff-triggered password-reset links --- apps/edr-freight-api/.env.example | 4 + apps/edr-freight-api/src/config/app.config.ts | 8 + .../modules/auth/customer-reset.controller.ts | 37 +++- .../modules/auth/customer-reset.service.ts | 144 ++++++++++-- .../modules/auth/dto/forgot-password.dto.ts | 18 +- .../auth/forgot-password.controller.ts | 19 +- .../modules/auth/forgot-password.service.ts | 105 ++++++++- .../src/modules/auth/freight-auth.module.ts | 3 + .../customers/ResetPasswordAction.tsx | 123 +++++++---- .../backoffice/src/constants/QUERY_KEYS.ts | 2 + .../backoffice/src/constants/URLS.ts | 2 + .../backoffice/src/services/api.ts | 8 + .../src/services/customers.service.ts | 17 +- .../backoffice/src/types/customer.ts | 18 +- apps/edr-freight-web/portal/src/App.tsx | 6 + .../src/components/errors/ApiErrorModal.tsx | 1 + .../portal/src/constants/URLS.ts | 1 + .../pages/accounts/ResetPasswordLinkPage.tsx | 208 ++++++++++++++++++ .../portal/src/services/api.ts | 7 + .../portal/src/services/auth.service.ts | 15 ++ apps/edr-freight-web/portal/src/types/auth.ts | 15 ++ 21 files changed, 688 insertions(+), 73 deletions(-) create mode 100644 apps/edr-freight-web/portal/src/pages/accounts/ResetPasswordLinkPage.tsx diff --git a/apps/edr-freight-api/.env.example b/apps/edr-freight-api/.env.example index 6fdaab48b..d80ce75c6 100644 --- a/apps/edr-freight-api/.env.example +++ b/apps/edr-freight-api/.env.example @@ -22,6 +22,10 @@ TELEBIRR_PRIVATE_KEY= TELEBIRR_PUBLIC_KEY= TELEBIRR_INSECURE_TLS=false +# Public origin of the freight customer portal. Password-reset links sent to +# customers are built against this — it must be browser-reachable. +FREIGHT_PORTAL_URL=http://localhost:5173 + # Portal pages the payment provider redirects the browser to after payment. # Point these at the freight portal's public payment result routes. PAYMENT_RETURN_URL=http://localhost:5173/payment/success diff --git a/apps/edr-freight-api/src/config/app.config.ts b/apps/edr-freight-api/src/config/app.config.ts index e493cc393..050b07145 100644 --- a/apps/edr-freight-api/src/config/app.config.ts +++ b/apps/edr-freight-api/src/config/app.config.ts @@ -9,6 +9,14 @@ export default registerAs("app", () => ({ env: process.env.NODE_ENV ?? "development", port: parseInt(process.env.PORT ?? "3001", 10), apiPrefix: "api", + /** + * Public origin of the freight customer portal. Password-reset links mailed + * or SMS'd to customers are built against this, so it must be the address the + * customer's browser can actually reach — not an internal service name. + */ + portalBaseUrl: ( + process.env.FREIGHT_PORTAL_URL ?? "http://localhost:5173" + ).replace(/\/+$/, ""), trainScheduling: { maxTrainWeightTons: numberFromEnv("TRAIN_SCHEDULING_MAX_WEIGHT_TONS", 3500), maxTrainLengthMeters: numberFromEnv("TRAIN_SCHEDULING_MAX_LENGTH_METERS", 760), diff --git a/apps/edr-freight-api/src/modules/auth/customer-reset.controller.ts b/apps/edr-freight-api/src/modules/auth/customer-reset.controller.ts index 2bf9f82fd..52a900fe8 100644 --- a/apps/edr-freight-api/src/modules/auth/customer-reset.controller.ts +++ b/apps/edr-freight-api/src/modules/auth/customer-reset.controller.ts @@ -1,6 +1,7 @@ import { Body, Controller, + Get, NotFoundException, Param, ParseUUIDPipe, @@ -11,11 +12,14 @@ import { ApiBearerAuth, ApiOperation, ApiTags } from "@nestjs/swagger"; import { BookingStaff } from "../../common/booking-guards"; import { FREIGHT_PERMS } from "../../seed/freight-permissions.registry"; import { BackofficeResetPasswordDto } from "./dto/forgot-password.dto"; -import { CustomerResetService } from "./customer-reset.service"; +import { + CustomerResetService, + CustomerResetTarget, +} from "./customer-reset.service"; /** - * Staff-triggered password reset. The customer receives the code and sets their - * own password — staff never see or handle a credential. + * Staff-triggered password reset. The customer receives a single-use link and + * sets their own password — staff never see or handle a credential. */ @ApiTags("backoffice") @Controller("backoffice/customers") @@ -23,26 +27,45 @@ import { CustomerResetService } from "./customer-reset.service"; export class CustomerResetController { constructor(private readonly customerResetService: CustomerResetService) {} + @Get(":companyId/reset-target") + @BookingStaff(FREIGHT_PERMS.customers.resetPassword) + @ApiOperation({ + summary: "The primary contact's IAM account a reset link would be sent to", + }) + async resetTarget( + @Param("companyId", ParseUUIDPipe) companyId: string, + ): Promise { + const target = await this.customerResetService.getResetTarget(companyId); + + if (!target) { + throw new NotFoundException( + "This customer has no active primary-contact account to reset", + ); + } + + return target; + } + @Post(":companyId/reset-password") @BookingStaff(FREIGHT_PERMS.customers.resetPassword) @ApiOperation({ - summary: "Send a password-reset code to a customer's primary contact", + summary: "Send a password-reset link to a customer's primary contact", }) async resetPassword( @Param("companyId", ParseUUIDPipe) companyId: string, @Body() dto: BackofficeResetPasswordDto, ) { - const maskedTarget = await this.customerResetService.sendResetToCustomer( + const sent = await this.customerResetService.sendResetLinkToCustomer( companyId, dto.channel, ); - if (!maskedTarget) { + if (!sent) { throw new NotFoundException( `No active primary contact with ${dto.channel === "email" ? "an email address" : "a phone number"} for this customer`, ); } - return { channel: dto.channel, maskedTarget }; + return sent; } } diff --git a/apps/edr-freight-api/src/modules/auth/customer-reset.service.ts b/apps/edr-freight-api/src/modules/auth/customer-reset.service.ts index 4c8f67599..91ddaf1a9 100644 --- a/apps/edr-freight-api/src/modules/auth/customer-reset.service.ts +++ b/apps/edr-freight-api/src/modules/auth/customer-reset.service.ts @@ -1,10 +1,31 @@ import { Injectable, Logger } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; import { InjectRepository } from "@nestjs/typeorm"; import { Repository } from "typeorm"; import { ExternalProfile } from "../companies/entities/external-profile.entity"; +import { EmailClientService } from "../notifications/email-client.service"; +import { SmsClientService } from "../notifications/sms-client.service"; import { ResetChannel } from "./dto/forgot-password.dto"; -import { ForgotPasswordService } from "./forgot-password.service"; +import { + ForgotPasswordService, + RESET_LINK_TTL_MS, +} from "./forgot-password.service"; +import { maskOtpTarget } from "./mask-target.util"; + +/** The account a staff-triggered reset would land on. */ +export interface CustomerResetTarget { + userId: string; + name: string; + email: string | null; + phone: string | null; +} + +export interface SentResetLink { + channel: ResetChannel; + maskedTarget: string; + expiresAt: string; +} @Injectable() export class CustomerResetService { @@ -14,19 +35,110 @@ export class CustomerResetService { @InjectRepository(ExternalProfile) private readonly externalProfileRepository: Repository, private readonly forgotPasswordService: ForgotPasswordService, + private readonly emailClient: EmailClientService, + private readonly smsClient: SmsClientService, + private readonly config: ConfigService, ) {} /** - * Send a reset code to the company's primary contact. Returns the masked - * destination, or null when there is no eligible account for that channel. + * The IAM account a reset would actually reach. The backoffice shows these + * values rather than `company.email` / `company.phone`: the company row holds + * business contact detail, while the link is delivered to the primary + * contact's own login credentials — the two drift apart routinely, and showing + * the wrong one has staff telling customers to check an inbox nothing was sent + * to. + */ + async getResetTarget(companyId: string): Promise { + const resolved = await this.resolvePrimaryContactUser(companyId); + if (!resolved) return null; + + const { profile, user, userId } = resolved; + return { + userId, + name: `${profile.firstName} ${profile.lastName}`.trim(), + email: user.email ?? null, + phone: user.phoneNumber ?? null, + }; + } + + /** + * Mint a password-reset link and send it to the company's primary contact. + * Returns the masked destination, or null when there is no eligible account + * for that channel. * * Unlike the public flow this reports failure honestly — the caller is an * authenticated staff member, so there is nothing to enumerate. */ - async sendResetToCustomer( + async sendResetLinkToCustomer( companyId: string, channel: ResetChannel, - ): Promise { + ): Promise { + const resolved = await this.resolvePrimaryContactUser(companyId); + if (!resolved) return null; + + const { user, userId } = resolved; + const target = this.forgotPasswordService.targetFor(user, channel); + if (!target) return null; + + // Mint first, send second: a failed send leaves an unused ticket that simply + // expires, whereas sending a link before the ticket exists would hand the + // customer a URL that is dead on arrival. + const ticket = await this.forgotPasswordService.mintResetTicket( + userId, + RESET_LINK_TTL_MS, + ); + const link = this.buildResetLink(ticket.userId, ticket.verificationCode); + const expiresAt = new Date(Date.now() + RESET_LINK_TTL_MS); + + const { queued } = target.email + ? await this.emailClient.sendEmail({ + to: target.email, + subject: "Reset your EDR Freight password", + text: + "A password reset was started for your EDR Freight account.\n\n" + + `Open this link to choose a new password:\n${link}\n\n` + + "The link expires in 24 hours and can only be used once. If you did " + + "not expect this, ignore this message — your password stays unchanged.", + }) + : await this.smsClient.sendSms({ + to: target.phone as string, + message: `Reset your EDR Freight password: ${link} (expires in 24 hours, single use)`, + }); + + this.logger.log( + `Staff-triggered ${channel} reset link sent to user ${userId} (company ${companyId}) queued=${queued}`, + ); + + if (!queued) { + // The ticket is committed and the backoffice is about to say "link sent", + // but nothing left this process — with RABBITMQ_ENABLED=false both clients + // are no-ops. Without this line the only symptom is a customer who never + // receives anything, indistinguishable from carrier loss. + this.logger.error( + `reset-link.dispatch.dropped channel=${channel} user=${userId} rabbitmqEnabled=${ + process.env.RABBITMQ_ENABLED ?? "unset" + } — transport reported no hand-off; no link will arrive`, + ); + // SECURITY: logs a live password-reset credential in cleartext. Same + // deliberate tradeoff the OTP service makes — this is the only way to + // complete a reset on an environment with no broker. Only reached when + // delivery already failed. + this.logger.warn(`Undelivered reset link for user ${userId}: ${link}`); + } + + return { + channel, + maskedTarget: maskOtpTarget(target), + expiresAt: expiresAt.toISOString(), + }; + } + + /** + * The company's primary contact, gated on the same active-account rule the + * public flow uses — so a suspended customer cannot be reactivated by a + * staff-triggered reset (IAM's `set-password` flips `isActive` back on). + */ + private async resolvePrimaryContactUser(companyId: string) { const profile = await this.externalProfileRepository.findOne({ where: { companyId, isPrimaryContact: true }, }); @@ -36,24 +148,28 @@ export class CustomerResetService { return null; } - // Resolve through the same active-account gate the public flow uses, so a - // suspended customer cannot be reactivated by a staff-triggered reset. const user = await this.forgotPasswordService.resolveActiveUserById( profile.userId, ); - if (!user) { + if (!user?.id) { this.logger.warn( `Primary contact ${profile.userId} of company ${companyId} is not an active account`, ); return null; } - const target = await this.forgotPasswordService.requestReset(user, channel); - if (!target) return null; + return { profile, user, userId: user.id }; + } - this.logger.log( - `Staff-triggered ${channel} reset sent to user ${user.id} (company ${companyId})`, - ); - return this.forgotPasswordService.maskTarget(target); + /** + * The portal route that trades the token for a set-password form. Params are + * URL-encoded because the token is base64url — safe as-is, but the encoding + * keeps this correct if the token format ever changes. + */ + private buildResetLink(userId: string, token: string): string { + const base = this.config.get("app.portalBaseUrl"); + return `${base}/reset-password?uid=${encodeURIComponent( + userId, + )}&token=${encodeURIComponent(token)}`; } } diff --git a/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts b/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts index be2f9bdac..c0bb0c151 100644 --- a/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts +++ b/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts @@ -1,5 +1,5 @@ import { ApiProperty } from "@nestjs/swagger"; -import { IsEnum, IsNotEmpty, IsString } from "class-validator"; +import { IsEnum, IsNotEmpty, IsString, IsUUID } from "class-validator"; /** The channel the reset code is delivered over. */ export enum ResetChannel { @@ -33,3 +33,19 @@ export class BackofficeResetPasswordDto { @IsEnum(ResetChannel) channel!: ResetChannel; } + +/** + * The two halves of a reset link's query string. Together they stand in for the + * identifier + OTP pair of the typed flow: the token proves possession of the + * inbox/handset the link was delivered to. + */ +export class ResolveResetLinkDto { + @ApiProperty({ description: "IAM user id from the reset link's `uid` param" }) + @IsUUID() + userId!: string; + + @ApiProperty({ description: "Opaque token from the reset link's `token` param" }) + @IsString() + @IsNotEmpty() + token!: string; +} diff --git a/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts b/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts index da49982d2..6fef56e0f 100644 --- a/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts +++ b/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts @@ -5,8 +5,13 @@ import { Public } from "@edr/api-common"; import { ForgotPasswordRequestDto, ForgotPasswordVerifyDto, + ResolveResetLinkDto, } from "./dto/forgot-password.dto"; -import { ForgotPasswordService, ResetTicket } from "./forgot-password.service"; +import { + ForgotPasswordService, + ResetLinkAccount, + ResetTicket, +} from "./forgot-password.service"; /** * Freight-owned reset flow. IAM ships a `forgot-password` route, but it only @@ -66,4 +71,16 @@ export class ForgotPasswordController { dto.otp, ); } + + @Post("forgot-password/resolve-link") + @ApiOperation({ + summary: "Validate a staff-issued reset link and return its set-password ticket", + description: + "Takes the link's uid/token pair. The returned { userId, identifier, verificationCode } " + + "is the body for PATCH /api/auth/set-password, so the customer never types an identifier. " + + "A bad or expired link is rejected here rather than after the password is typed.", + }) + resolveLink(@Body() dto: ResolveResetLinkDto): Promise { + return this.forgotPasswordService.resolveResetLink(dto.userId, dto.token); + } } diff --git a/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts b/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts index b357c2cfb..8a470c864 100644 --- a/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts +++ b/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts @@ -4,7 +4,7 @@ import { BadRequestException, Injectable, Logger } from "@nestjs/common"; import { InjectDataSource, InjectRepository } from "@nestjs/typeorm"; import { DataSource, Repository } from "typeorm"; -import { hashPassword } from "@tria-plc/api-common/utils/argon"; +import { hashPassword, verifyPassword } from "@tria-plc/api-common/utils/argon"; import { EOtpType } from "@tria-plc/iamapi-common/enums/otp.enum"; import { User } from "@tria-plc/iamapi-common/entities/iam/user/user.entity"; import { UserVerification } from "@tria-plc/iamapi-common/entities/iam/user/user-verification.entity"; @@ -22,11 +22,32 @@ const RESET_TICKET_TTL_MS = 10 * 60 * 1000; /** How long the emailed/SMS'd OTP stays valid before it must be re-requested. */ const RESET_OTP_TTL_MS = 10 * 60 * 1000; +/** + * A staff-triggered reset link lives longer than a typed OTP: the customer may + * only see the SMS/email hours after the call that prompted it. + */ +export const RESET_LINK_TTL_MS = 24 * 60 * 60 * 1000; + +/** IAM refuses a ticket once its row hits this many failed attempts. */ +const MAX_TICKET_ATTEMPTS = 5; + export interface ResetTicket { userId: string; verificationCode: string; } +/** + * What a valid reset link resolves to. `identifier` is the value IAM's + * `set-password` matches the user on (it accepts email / username / phone), so + * the portal can spend the ticket without the customer typing anything. + */ +export interface ResetLinkAccount { + userId: string; + identifier: string; + maskedIdentifier: string; + verificationCode: string; +} + @Injectable() export class ForgotPasswordService { private readonly logger = new Logger(ForgotPasswordService.name); @@ -82,13 +103,23 @@ export class ForgotPasswordService { } /** The address the code goes to, taken from the account — never from input. */ - private targetFor(user: User, channel: ResetChannel): OtpTarget | null { + targetFor(user: User, channel: ResetChannel): OtpTarget | null { if (channel === ResetChannel.Email) { return user.email ? { email: user.email } : null; } return user.phoneNumber ? { phone: user.phoneNumber } : null; } + /** + * The value IAM's `set-password` will match this account on. It looks the user + * up by email OR username OR phoneNumber (and lowercases whatever it is + * given), so prefer email, then phone, and fall back to username last — + * a mixed-case username would not survive that lowercasing. + */ + private identifierFor(user: User): string | null { + return user.email ?? user.phoneNumber ?? user.username ?? null; + } + /** * Send a reset code to the account's own email/phone. Returns the target so * authenticated (backoffice) callers can echo a masked version; unauthenticated @@ -134,9 +165,18 @@ export class ForgotPasswordService { await this.otpService.verifyOtpForAction(target, otp, RESET_OTP_TTL_MS); + return await this.mintResetTicket(user.id, RESET_TICKET_TTL_MS); + } + + /** + * Mint a single-use IAM reset ticket. Shared by the OTP flow (where the code + * is the proof of possession) and the staff-triggered link flow (where the + * ticket travels in the link and delivery to the account's own inbox/handset + * is the proof). + */ + async mintResetTicket(userId: string, ttlMs: number): Promise { const code = randomBytes(24).toString("base64url"); const verificationCode = await hashPassword(code); - const userId = user.id; await this.dataSource.transaction(async (manager) => { const repo = manager.getRepository(UserVerification); @@ -147,7 +187,7 @@ export class ForgotPasswordService { userId, otpType: EOtpType.RESET_PASSWORD, verificationCode, - expiresAt: new Date(Date.now() + RESET_TICKET_TTL_MS), + expiresAt: new Date(Date.now() + ttlMs), isUsed: false, attemptCount: 0, }); @@ -157,6 +197,63 @@ export class ForgotPasswordService { return { userId, verificationCode: code }; } + /** + * Validate a reset link and hand back everything the portal needs to spend it + * on IAM's `PATCH /api/auth/set-password`. + * + * The checks mirror IAM's own — newest row, unused, unexpired, attempts left, + * argon match — so a link that resolves here is one IAM will honour. Doing + * them up front is what lets the page say "this link has expired" before the + * customer types a password rather than after. + * + * Every rejection is the same message: a link is a bearer credential, and the + * holder of a bad one learns nothing about why it failed or whether the user + * id exists. + */ + async resolveResetLink( + userId: string, + token: string, + ): Promise { + const invalid = new BadRequestException( + "This password-reset link is invalid or has expired. Request a new one.", + ); + + const user = await this.resolveActiveUserById(userId); + const identifier = user && this.identifierFor(user); + if (!user || !identifier) throw invalid; + + const verification = await this.dataSource + .getRepository(UserVerification) + .findOne({ + where: { userId, otpType: EOtpType.RESET_PASSWORD }, + order: { createdAt: "DESC" }, + }); + + // `expiresAt` / `attemptCount` are optional on IAM's entity but always + // written by `mintResetTicket`. A row missing either is malformed, so treat + // it as expired rather than letting it through unchecked. + if ( + !verification || + verification.isUsed || + !verification.expiresAt || + verification.expiresAt < new Date() || + (verification.attemptCount ?? 0) >= MAX_TICKET_ATTEMPTS || + !(await verifyPassword(token, verification.verificationCode)) + ) { + this.logger.warn(`Reset link rejected for user ${userId}`); + throw invalid; + } + + return { + userId, + identifier, + maskedIdentifier: maskOtpTarget( + identifier.includes("@") ? { email: identifier } : { phone: identifier }, + ), + verificationCode: token, + }; + } + /** `+251911234567` -> `+251•••••4567`; `ab@x.com` -> `a•@x.com`. */ maskTarget(target: OtpTarget): string { return maskOtpTarget(target); diff --git a/apps/edr-freight-api/src/modules/auth/freight-auth.module.ts b/apps/edr-freight-api/src/modules/auth/freight-auth.module.ts index 1a375d86f..10dbd0b37 100644 --- a/apps/edr-freight-api/src/modules/auth/freight-auth.module.ts +++ b/apps/edr-freight-api/src/modules/auth/freight-auth.module.ts @@ -7,6 +7,7 @@ import { User } from '@tria-plc/iamapi-common/entities/iam/user/user.entity'; import { UserVerification } from '@tria-plc/iamapi-common/entities/iam/user/user-verification.entity'; import { ExternalProfile } from '../companies/entities/external-profile.entity'; +import { NotificationsModule } from '../notifications/notifications.module'; import { OtpModule } from '../otp/otp.module'; import { AccountController } from './account.controller'; import { AccountService } from './account.service'; @@ -29,6 +30,8 @@ import { FreightMeService } from './freight-me.service'; Employee, ]), OtpModule, + // Reset links go out over email/SMS directly, not through the OTP service. + NotificationsModule, ], controllers: [ FreightMeController, diff --git a/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx b/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx index bc505a21f..2175d4d7b 100644 --- a/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx +++ b/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx @@ -1,5 +1,5 @@ -import { Button, Modal, Radio, Stack, Text } from "@mantine/core"; -import { useMutation } from "@tanstack/react-query"; +import { Alert, Button, Loader, Modal, Radio, Stack, Text } from "@mantine/core"; +import { useMutation, useQuery } from "@tanstack/react-query"; import { KeyRound } from "lucide-react"; import { useState } from "react"; @@ -10,32 +10,48 @@ import { api } from "@/services/api"; import type { Company, ResetChannel } from "@/types/customer"; export interface ResetPasswordActionProps { - company: Pick; + company: Pick; } /** - * Staff-triggered password reset. Sends a one-time code to the customer's - * primary contact; the customer picks their own new password. No credential is - * ever shown to or handled by staff. + * Staff-triggered password reset. Sends a single-use link to the customer's + * primary contact; the customer opens it and picks their own new password. No + * credential is ever shown to or handled by staff. */ -export default function ResetPasswordAction({ company }: ResetPasswordActionProps) { +export default function ResetPasswordAction({ + company, +}: ResetPasswordActionProps) { const { user } = useAuth(); const { toast } = useToast(); const [opened, setOpened] = useState(false); const [channel, setChannel] = useState("phone"); + const allowed = hasPermission(user, FREIGHT_PERMS.customers.resetPassword); + + // The destination is the primary contact's IAM account, not the company + // record — those are different fields and routinely hold different values, so + // showing `company.phone` here would tell staff the wrong number. Only fetched + // once the modal is open. + const targetQuery = useQuery( + api.customers.resetTarget.queryOptions({ + input: { companyId: company.id }, + enabled: allowed && opened, + }), + ); + const target = targetQuery.data; + const { mutate, isPending } = useMutation( api.customers.resetPassword.mutationOptions({ onSuccess: (result) => { setOpened(false); toast({ - title: "Reset code sent", - description: `The customer can now reset their password using the code sent to ${result.maskedTarget}.`, + title: "Reset link sent", + description: `The customer can set a new password using the link sent to ${result.maskedTarget}. It expires in 24 hours.`, }); }, onError: (error) => { toast({ - title: "Could not send reset code", + title: "Could not send reset link", description: error.message, variant: "destructive", }); @@ -43,7 +59,10 @@ export default function ResetPasswordAction({ company }: ResetPasswordActionProp }), ); - if (!hasPermission(user, FREIGHT_PERMS.customers.resetPassword)) return null; + if (!allowed) return null; + + const channelMissing = + !!target && (channel === "email" ? !target.email : !target.phone); return ( <> @@ -58,46 +77,66 @@ export default function ResetPasswordAction({ company }: ResetPasswordActionProp setOpened(false)} - title="Send a password-reset code" + title="Send a password-reset link" centered > - We'll send a one-time code to this customer's primary contact. - They choose their own new password — you will not see it. + We'll send a single-use link to this customer's primary + contact. They choose their own new password — you will not see it. + The link expires in 24 hours. - setChannel(v as ResetChannel)} - label="Send the code via" - > - - - + {targetQuery.isLoading ? ( + + - + ) : targetQuery.isError ? ( + + {targetQuery.error.message} + + ) : target ? ( + <> + setChannel(v as ResetChannel)} + label={`Send the link to ${target.name || "the primary contact"} via`} + > + + + + + - - The code goes to the primary contact's own email or phone, which - may differ from the company contact details shown above. - + + These are the primary contact's own login details, which may + differ from the company contact details on the profile. + - + + + ) : null} diff --git a/apps/edr-freight-web/backoffice/src/constants/QUERY_KEYS.ts b/apps/edr-freight-web/backoffice/src/constants/QUERY_KEYS.ts index 160e0a61d..2f65e110e 100644 --- a/apps/edr-freight-web/backoffice/src/constants/QUERY_KEYS.ts +++ b/apps/edr-freight-web/backoffice/src/constants/QUERY_KEYS.ts @@ -38,6 +38,8 @@ export const QUERY_KEYS = { documents: (id: string) => ["customers", "detail", id, "documents"] as const, payments: (id: string) => ["customers", "detail", id, "payments"] as const, + resetTarget: (id: string) => + ["customers", "detail", id, "reset-target"] as const, changeRequests: (id: string) => ["customers", "detail", id, "change-requests"] as const, }, diff --git a/apps/edr-freight-web/backoffice/src/constants/URLS.ts b/apps/edr-freight-web/backoffice/src/constants/URLS.ts index c1de9839e..2c68e4ba9 100644 --- a/apps/edr-freight-web/backoffice/src/constants/URLS.ts +++ b/apps/edr-freight-web/backoffice/src/constants/URLS.ts @@ -88,6 +88,8 @@ export const URL_CONSTANTS = { `/payments/by-company/${id}/customer-view`, RESET_PASSWORD: (companyId: string) => `/backoffice/customers/${companyId}/reset-password`, + RESET_TARGET: (companyId: string) => + `/backoffice/customers/${companyId}/reset-target`, }, BILLING: { diff --git a/apps/edr-freight-web/backoffice/src/services/api.ts b/apps/edr-freight-web/backoffice/src/services/api.ts index 6c6245ee6..8f05be897 100644 --- a/apps/edr-freight-web/backoffice/src/services/api.ts +++ b/apps/edr-freight-web/backoffice/src/services/api.ts @@ -10,6 +10,7 @@ import type { CustomerBooking, CustomerDocument, CustomerPayment, + CustomerResetTarget, PaginatedCompanies, ProfileStatus, ResetChannel, @@ -2581,6 +2582,13 @@ export const api = { ({ id }) => QUERY_KEYS.CUSTOMERS.payments(id), ), + resetTarget: endpoint<{ companyId: string }, CustomerResetTarget>( + "customers", + "resetTarget", + ({ companyId }) => customersService.resetTarget(companyId), + ({ companyId }) => QUERY_KEYS.CUSTOMERS.resetTarget(companyId), + ), + resetPassword: endpoint< { companyId: string; channel: ResetChannel }, ResetPasswordResult diff --git a/apps/edr-freight-web/backoffice/src/services/customers.service.ts b/apps/edr-freight-web/backoffice/src/services/customers.service.ts index c9a649946..ae41d705f 100644 --- a/apps/edr-freight-web/backoffice/src/services/customers.service.ts +++ b/apps/edr-freight-web/backoffice/src/services/customers.service.ts @@ -9,6 +9,7 @@ import type { CustomerBooking, CustomerDocument, CustomerPayment, + CustomerResetTarget, PaginatedCompanies, ProfileStatus, ResetChannel, @@ -89,8 +90,20 @@ export const customersService = { }, /** - * Send a password-reset code to the company's primary contact. Staff never - * receive a credential — the customer sets their own password from the code. + * The IAM account a reset link would go to. Read before offering the action + * so staff see the credentials the link actually reaches, not the company's + * business contact details. + */ + resetTarget(companyId: string): Promise { + return apiClient + .get(URL_CONSTANTS.COMPANIES.RESET_TARGET(companyId)) + .then((r) => r.data); + }, + + /** + * Send a password-reset link to the company's primary contact. Staff never + * receive a credential — the customer opens the link and sets their own + * password. */ resetPassword( companyId: string, diff --git a/apps/edr-freight-web/backoffice/src/types/customer.ts b/apps/edr-freight-web/backoffice/src/types/customer.ts index a66d82a28..95bc4264e 100644 --- a/apps/edr-freight-web/backoffice/src/types/customer.ts +++ b/apps/edr-freight-web/backoffice/src/types/customer.ts @@ -110,13 +110,27 @@ export interface CompanyChangeRequest { updatedAt: string; } -/** The channel a customer's password-reset code is delivered over. */ +/** The channel a customer's password-reset link is delivered over. */ export type ResetChannel = "email" | "phone"; export interface ResetPasswordResult { channel: ResetChannel; - /** Where the code went, e.g. `+251•••4821` — safe to show to staff. */ + /** Where the link went, e.g. `+251•••4821` — safe to show to staff. */ maskedTarget: string; + /** ISO timestamp after which the link stops working. */ + expiresAt: string; +} + +/** + * The IAM account a reset link would reach — the company's primary contact. + * Distinct from `Company.email` / `Company.phone`, which are business contact + * details and routinely differ from the credentials the customer logs in with. + */ +export interface CustomerResetTarget { + userId: string; + name: string; + email: string | null; + phone: string | null; } /** Mirrors backend `Company` (+ its `companyProfiles`). */ diff --git a/apps/edr-freight-web/portal/src/App.tsx b/apps/edr-freight-web/portal/src/App.tsx index 7c5b5e5b5..8bd7cc74f 100644 --- a/apps/edr-freight-web/portal/src/App.tsx +++ b/apps/edr-freight-web/portal/src/App.tsx @@ -35,6 +35,7 @@ import MyPortalPage from "./pages/MyPortalPage"; import MySignaturePage from "./pages/MySignaturePage"; import SettingsPage from "./pages/SettingsPage"; import ForgotPasswordPage from "./pages/accounts/ForgotPasswordPage"; +import ResetPasswordLinkPage from "./pages/accounts/ResetPasswordLinkPage"; import LoginPage from "./pages/accounts/LoginPage"; import SetPasswordPage from "./pages/accounts/SetPasswordPage"; import SignupPage from "./pages/accounts/SignupPage"; @@ -265,6 +266,11 @@ const App = () => { } /> + {/* Staff-issued reset links land here. Deliberately outside + RedirectIfAuthed: a customer with a stale session still needs the link + to work, and the token — not the session — is what authorises it. */} + } /> + {/* Signup-flow pages; reached while a session already exists */} } /> } /> diff --git a/apps/edr-freight-web/portal/src/components/errors/ApiErrorModal.tsx b/apps/edr-freight-web/portal/src/components/errors/ApiErrorModal.tsx index 236f33730..467c88043 100644 --- a/apps/edr-freight-web/portal/src/components/errors/ApiErrorModal.tsx +++ b/apps/edr-freight-web/portal/src/components/errors/ApiErrorModal.tsx @@ -31,6 +31,7 @@ const EXCLUDED_PATH_PATTERNS = [ /^\/forgot-password/, /^\/otp/, /^\/set-password/, + /^\/reset-password/, /warehouse/i, /first-mile/i, /last-mile/i, diff --git a/apps/edr-freight-web/portal/src/constants/URLS.ts b/apps/edr-freight-web/portal/src/constants/URLS.ts index c920e20d8..090502a3a 100644 --- a/apps/edr-freight-web/portal/src/constants/URLS.ts +++ b/apps/edr-freight-web/portal/src/constants/URLS.ts @@ -8,6 +8,7 @@ export const URL_CONSTANTS = { CHANGE_PASSWORD: "/api/auth/change-password", FORGOT_PASSWORD_REQUEST: "/api/auth/forgot-password/request", FORGOT_PASSWORD_VERIFY: "/api/auth/forgot-password/verify", + FORGOT_PASSWORD_RESOLVE_LINK: "/api/auth/forgot-password/resolve-link", }, USERS: { diff --git a/apps/edr-freight-web/portal/src/pages/accounts/ResetPasswordLinkPage.tsx b/apps/edr-freight-web/portal/src/pages/accounts/ResetPasswordLinkPage.tsx new file mode 100644 index 000000000..4776287ae --- /dev/null +++ b/apps/edr-freight-web/portal/src/pages/accounts/ResetPasswordLinkPage.tsx @@ -0,0 +1,208 @@ +import { type FormEvent, useEffect, useState } from "react"; +import { Alert, Button, Loader, PasswordInput, Stack } from "@mantine/core"; +import { AlertCircle, KeyRound } from "lucide-react"; +import { Link, useNavigate, useSearchParams } from "react-router-dom"; + +import AuthShell from "@/components/auth/AuthShell"; +import PasswordChecklist from "@/components/auth/PasswordChecklist"; +import { api } from "@/services/api"; +import type { ResetLinkAccount } from "@/types/auth"; +import { meetsAllRequirements } from "@/utils/passwordSchema"; +import { extractApiError } from "@/utils/result"; + +/** + * Lands the password-reset link a staff member sends from the backoffice + * (`/reset-password?uid=…&token=…`). + * + * The link itself is the proof of possession — it was delivered to the address + * on the account — so there is no code to type. The token is validated before + * the form appears, which is what lets an expired link say so up front instead + * of after a password has been chosen. + */ +export default function ResetPasswordLinkPage() { + const navigate = useNavigate(); + const [params] = useSearchParams(); + const userId = params.get("uid") ?? ""; + const token = params.get("token") ?? ""; + + const [account, setAccount] = useState(null); + const [checking, setChecking] = useState(true); + const [linkError, setLinkError] = useState(null); + + const [password, setPassword] = useState(""); + const [confirmPassword, setConfirmPassword] = useState(""); + const [submitting, setSubmitting] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + if (!userId || !token) { + setLinkError( + "This password-reset link is incomplete. Open the full link from your email or SMS.", + ); + setChecking(false); + return; + } + + let cancelled = false; + api.auth.resolveResetLink + .call({ userId, token }) + .then((resolved) => { + if (!cancelled) setAccount(resolved); + }) + .catch((err) => { + if (!cancelled) setLinkError(extractApiError(err).message); + }) + .finally(() => { + if (!cancelled) setChecking(false); + }); + + return () => { + cancelled = true; + }; + }, [userId, token]); + + const handleSubmit = async (event: FormEvent) => { + event.preventDefault(); + setError(null); + + if (!account) return; + if (password !== confirmPassword) { + setError("Passwords do not match."); + return; + } + + setSubmitting(true); + try { + await api.auth.resetPassword.call({ + userId: account.userId, + // Resolved server-side from the token — the account holder never types + // an identifier, so there is nothing here to get wrong. + email: account.identifier, + verificationCode: account.verificationCode, + newPassword: password, + confirmPassword, + }); + navigate("/login", { replace: true, state: { passwordReset: true } }); + } catch (err) { + setError(extractApiError(err).message); + } finally { + setSubmitting(false); + } + }; + + return ( + +
+
+ + + +
+ +
+

+ Choose a new password +

+

+ {checking + ? "Checking your reset link…" + : account + ? `Resetting the password for ${account.maskedIdentifier}.` + : "This link can no longer be used."} +

+
+ + {checking ? ( +
+ +
+ ) : null} + + {!checking && linkError ? ( + + }> + {linkError} + + +

+ Remembered it?{" "} + + Back to sign in + +

+ + ) : null} + + {!checking && account ? ( +
+ +
+ setPassword(event.target.value)} + /> + +
+ + setConfirmPassword(event.target.value)} + /> + + {error ? ( + } + > + {error} + + ) : null} + + + +
+ ) : null} +
+ + ); +} diff --git a/apps/edr-freight-web/portal/src/services/api.ts b/apps/edr-freight-web/portal/src/services/api.ts index 0b9204a9c..ce13fad55 100644 --- a/apps/edr-freight-web/portal/src/services/api.ts +++ b/apps/edr-freight-web/portal/src/services/api.ts @@ -81,7 +81,9 @@ import type { SignupResponse, ForgotPasswordRequestPayload, ForgotPasswordVerifyPayload, + ResetLinkAccount, ResetTicket, + ResolveResetLinkPayload, SendContactOtpPayload, SendContactOtpResponse, UpdateAccountNamePayload, @@ -130,6 +132,11 @@ export const api = { "verifyPasswordResetOtp", authService.verifyPasswordResetOtp, ), + resolveResetLink: endpoint( + "auth", + "resolveResetLink", + authService.resolveResetLink, + ), resetPassword: endpoint( "auth", "resetPassword", diff --git a/apps/edr-freight-web/portal/src/services/auth.service.ts b/apps/edr-freight-web/portal/src/services/auth.service.ts index f72bbfaf9..90ab39826 100644 --- a/apps/edr-freight-web/portal/src/services/auth.service.ts +++ b/apps/edr-freight-web/portal/src/services/auth.service.ts @@ -11,7 +11,9 @@ import type { LoginResponse, OtpPayload, OtpResponse, + ResetLinkAccount, ResetTicket, + ResolveResetLinkPayload, SendContactOtpPayload, SendContactOtpResponse, SetPasswordPayload, @@ -79,6 +81,19 @@ export const authService = { return { userId: res.data.userId, verificationCode: res.data.verificationCode }; }, + /** + * Validate a staff-issued reset link before showing the password form, and + * pick up the ticket it carries. Rejected links (expired, already spent) fail + * here rather than after the customer has typed a new password. + */ + resolveResetLink: async (body: ResolveResetLinkPayload) => { + const res = await client.post( + URL_CONSTANTS.AUTH.FORGOT_PASSWORD_RESOLVE_LINK, + body, + ); + return res.data; + }, + /** * Spend the reset ticket. Distinct from `setPassword` above, which the * authenticated post-signup flow drives through `useAuth` — this one carries diff --git a/apps/edr-freight-web/portal/src/types/auth.ts b/apps/edr-freight-web/portal/src/types/auth.ts index 0dadac608..441afc4ba 100644 --- a/apps/edr-freight-web/portal/src/types/auth.ts +++ b/apps/edr-freight-web/portal/src/types/auth.ts @@ -121,6 +121,21 @@ export interface ResetTicket { verificationCode: string; } +/** The `uid` / `token` pair carried by a staff-issued password-reset link. */ +export interface ResolveResetLinkPayload { + userId: string; + token: string; +} + +/** + * A validated reset link. Carries the identifier IAM matches the account on, so + * the customer never has to type one — plus a masked copy safe to display. + */ +export interface ResetLinkAccount extends ResetTicket { + identifier: string; + maskedIdentifier: string; +} + export interface GenerateVerificationCodePayload { email: string; phoneNumber: string; From 5bf82bf53111a7899a0f12a7b60893b65e675595 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 20 Jul 2026 11:28:18 +0000 Subject: [PATCH 2/6] fix: notify the backoffice of new booking --- .../booking-lifecycle-notifier.service.ts | 13 +++++ .../contract-booking.completion.spec.ts | 1 + .../contract-booking.consolidation.spec.ts | 1 + .../contracts/contract-booking.service.ts | 14 +++++- .../src/pages/accounts/CompanyProfileForm.tsx | 48 ++++++++++++++----- 5 files changed, 64 insertions(+), 13 deletions(-) diff --git a/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts b/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts index 1cef9528c..44c51e4c1 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-lifecycle-notifier.service.ts @@ -253,6 +253,19 @@ export class BookingLifecycleNotifierService { // ── Staff-facing (backoffice inbox) ──────────────────────────────────────── + /** + * A booking was created under a contract. Contract drawdowns never pass + * through submit, so this is the only point at which staff learn the booking + * exists — {@link submittedToStaff} covers the direct-booking flow instead. + */ + createdToStaff(b: Booking): void { + this.inAppStaff( + b, + 'New booking created', + `Booking ${this.ref(b)} was created under a contract and has entered the pipeline.`, + ); + } + /** Customer submitted a booking for review. */ submittedToStaff(b: Booking): void { this.inAppStaff( diff --git a/apps/edr-freight-api/src/modules/contracts/contract-booking.completion.spec.ts b/apps/edr-freight-api/src/modules/contracts/contract-booking.completion.spec.ts index 4f3deb513..7f0aaabea 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-booking.completion.spec.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-booking.completion.spec.ts @@ -27,6 +27,7 @@ describe('ContractBookingService — quantity-cap completion', () => { {} as never, // workflowService {} as never, // invoiceService {} as never, // clearanceFeeService + { createdToStaff: jest.fn() } as never, // bookingNotifier {} as never, // dataSource {} as never, // trainSchedulingService {} as never, // bookingBatchService diff --git a/apps/edr-freight-api/src/modules/contracts/contract-booking.consolidation.spec.ts b/apps/edr-freight-api/src/modules/contracts/contract-booking.consolidation.spec.ts index f28468936..68c02fa7d 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-booking.consolidation.spec.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-booking.consolidation.spec.ts @@ -58,6 +58,7 @@ describe('ContractBookingService — drawdown consolidation gate', () => { {} as never, // workflowService invoiceService as never, {} as never, // clearanceFeeService + { createdToStaff: jest.fn() } as never, // bookingNotifier {} as never, // dataSource {} as never, // trainSchedulingService {} as never, // bookingBatchService diff --git a/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts index fd1f530b8..e230380f0 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-booking.service.ts @@ -18,6 +18,7 @@ import { BookingContainerUnit } from '../bookings/entities/booking-container-uni import { BookingsRepository } from '../bookings/bookings.repository'; import { BookingPricingService } from '../bookings/booking-pricing.service'; import { BookingTransitionService } from '../bookings/booking-transition.service'; +import { BookingLifecycleNotifierService } from '../bookings/booking-lifecycle-notifier.service'; import { ConsolidationService } from '../bookings/consolidation.service'; import { PriceLineItemDto } from '../bookings/dto/generate-price-response.dto'; import { BookingInvoiceService } from '../bookings/booking-invoice.service'; @@ -97,6 +98,7 @@ export class ContractBookingService { private readonly workflowService: ClearanceWorkflowService, private readonly invoiceService: BookingInvoiceService, private readonly clearanceFeeService: ClearanceFeeService, + private readonly bookingNotifier: BookingLifecycleNotifierService, private readonly dataSource: DataSource, @Inject(forwardRef(() => TrainSchedulingService)) private readonly trainSchedulingService: TrainSchedulingService, @@ -352,6 +354,12 @@ export class ContractBookingService { const withContainers = await this.bookingsRepository.findByIdWithFiles( booking.id, ); + + // Tell staff the booking exists. Placed after the zero-price rollback (which + // hard-deletes the row) and before the consolidation gate, so it fires + // exactly once whether the booking parks for a partner or finalizes inline. + this.bookingNotifier.createdToStaff(withContainers ?? booking); + const intendedStatus = generalCustoms || generalSelfClear ? 'AWAITING_DOCUMENTS' @@ -481,6 +489,7 @@ export class ContractBookingService { ); const result = await this.bookingsRepository.findByIdWithFiles(booking.id); + this.bookingNotifier.createdToStaff(result ?? booking); return { booking: result ?? booking, warnings: [] }; } @@ -568,7 +577,10 @@ export class ContractBookingService { await this.clearanceFeeService.issueForBooking(booking, contract); } - return (await this.bookingsRepository.findByIdWithFiles(booking.id)) ?? booking; + const created = + (await this.bookingsRepository.findByIdWithFiles(booking.id)) ?? booking; + this.bookingNotifier.createdToStaff(created); + return created; } /** diff --git a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx index 47c00bef2..bef027c78 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx @@ -9,10 +9,11 @@ import { Stack, Text, TextInput, + Tooltip, } from "@mantine/core"; import { zodResolver } from "@hookform/resolvers/zod"; import { useQuery } from "@tanstack/react-query"; -import { AlertCircle, ArrowLeft, ArrowRight } from "lucide-react"; +import { AlertCircle, ArrowLeft, ArrowRight, Info } from "lucide-react"; import { useEffect, useMemo, useRef, useState } from "react"; import { Controller, useForm } from "react-hook-form"; @@ -293,22 +294,28 @@ export default function CompanyProfileForm({ const [contactSameAsGm, setContactSameAsGm] = useState(false); const [poaSameAsContact, setPoaSameAsContact] = useState(false); - // General Manager source: the eTrade-registered business owner when a TIN - // lookup found one, otherwise the registering user's own account details. + // General Manager source. The company step's email/phone are seeded from + // eTrade (and the account email) but stay editable, so the link reads the + // CURRENT form values rather than the frozen eTrade snapshot — an edit on the + // company step propagates here, the same way "Same as General Manager" tracks + // the general manager's live values. eTrade's owner name has no editable + // field of its own, so it falls back to the registering user's account name. + const companyEmail = watch("companyEmail"); + const companyPhone = watch("companyPhone"); const gmSourceName = etradeOwner?.name ?? user.name?.en ?? ""; - const gmSourcePhone = etradeOwner - ? etradeOwner.phone - : toEthiopianE164(user.phoneNumber); + const gmSourceEmail = companyEmail || user.email || ""; + const gmSourcePhone = + companyPhone || etradeOwner?.phone || toEthiopianE164(user.phoneNumber) || ""; useEffect(() => { if (!gmSameAsOwner) return; setValue("generalManagerName", gmSourceName, { shouldValidate: true }); - setValue("generalManagerEmail", user.email ?? "", { shouldValidate: true }); - setValue("generalManagerPhone", gmSourcePhone ?? "", { + setValue("generalManagerEmail", gmSourceEmail, { shouldValidate: true }); + setValue("generalManagerPhone", gmSourcePhone, { shouldValidate: true, }); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [gmSameAsOwner, gmSourceName, gmSourcePhone, user.email]); + }, [gmSameAsOwner, gmSourceName, gmSourceEmail, gmSourcePhone]); const toggleGmSameAsOwner = (checked: boolean) => { setGmSameAsOwner(checked); @@ -624,7 +631,24 @@ export default function CompanyProfileForm({ {...register("vatNumber")} /> + FAN Number (16 digits) + + + + + } placeholder="1234567890123456" maxLength={16} error={errors.fanNumber?.message} @@ -743,8 +767,8 @@ export default function CompanyProfileForm({ title="Same as business owner" description={ etradeOwner - ? "Reuse the eTrade-registered owner's name and phone (email from your account). Uncheck to enter different details." - : "Reuse your account's name, email and phone. Uncheck to enter different details." + ? "Reuse the eTrade-registered owner's name, plus the company email and phone as you entered them. Uncheck to enter different details." + : "Reuse your account's name and the company email and phone as you entered them. Uncheck to enter different details." } /> Date: Mon, 20 Jul 2026 11:44:07 +0000 Subject: [PATCH 3/6] fix: reuse the locaiton too --- .../src/pages/accounts/CompanyProfileForm.tsx | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx index bef027c78..dd68c7006 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/CompanyProfileForm.tsx @@ -343,13 +343,28 @@ export default function CompanyProfileForm({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [contactSameAsGm, gmName, gmEmail, gmPhone]); + // The contact-person step has no location/address of its own, so the linked + // PoA takes the company's — location as entered, address as composed from the + // company's address fields. Both stay mirrored while the link is checked. + const companyLocation = watch("companyLocation"); + const companyAddress = watch("companyAddress"); + useEffect(() => { if (!poaSameAsContact) return; setValue("poaName", contactName ?? ""); setValue("poaEmail", contactEmail ?? ""); setValue("poaPhone", contactPhone ?? ""); + setValue("poaLocation", companyLocation ?? ""); + setValue("poaAddress", companyAddress ?? ""); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [poaSameAsContact, contactName, contactEmail, contactPhone]); + }, [ + poaSameAsContact, + contactName, + contactEmail, + contactPhone, + companyLocation, + companyAddress, + ]); const toggleContactSameAsGm = (checked: boolean) => { setContactSameAsGm(checked); @@ -367,6 +382,8 @@ export default function CompanyProfileForm({ setValue("poaName", ""); setValue("poaEmail", ""); setValue("poaPhone", ""); + setValue("poaLocation", ""); + setValue("poaAddress", ""); } }; @@ -852,7 +869,7 @@ export default function CompanyProfileForm({ checked={poaSameAsContact} onToggle={togglePoaSameAsContact} title="Same as contact person" - description="Reuse the contact person's name, email and phone. Uncheck to enter different details." + description="Reuse the contact person's name, email and phone, plus the company's location and address. Uncheck to enter different details." /> )} Date: Mon, 20 Jul 2026 11:52:08 +0000 Subject: [PATCH 4/6] fix: disable global errors --- apps/edr-freight-web/backoffice/src/auth/http.ts | 8 ++------ apps/edr-freight-web/portal/src/utils/api.ts | 15 +++++---------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/apps/edr-freight-web/backoffice/src/auth/http.ts b/apps/edr-freight-web/backoffice/src/auth/http.ts index 9fe86ab1e..b31fd836a 100644 --- a/apps/edr-freight-web/backoffice/src/auth/http.ts +++ b/apps/edr-freight-web/backoffice/src/auth/http.ts @@ -1,10 +1,6 @@ import axios from "axios"; import { API_BASE_URL } from "@/constants/apiConfig"; -import { - emitApiError, - extractApiErrorPayload, -} from "@/components/errors/ApiErrorModal"; import { captureApiError } from "@/lib/posthog"; import { AUTH_TOKEN_COOKIE, @@ -120,8 +116,8 @@ api.interceptors.response.use( error.response.status !== 401 && !originalRequest?.suppressErrorModal ) { - const payload = extractApiErrorPayload(error); - if (payload) emitApiError(payload); + // const payload = extractApiErrorPayload(error); + // if (payload) emitApiError(payload); } return Promise.reject(error); } diff --git a/apps/edr-freight-web/portal/src/utils/api.ts b/apps/edr-freight-web/portal/src/utils/api.ts index 69dfe308f..a316d9520 100644 --- a/apps/edr-freight-web/portal/src/utils/api.ts +++ b/apps/edr-freight-web/portal/src/utils/api.ts @@ -2,10 +2,6 @@ import { UseQueryOptions, QueryObserverOptions } from "@tanstack/react-query"; import axios, { AxiosError, InternalAxiosRequestConfig } from "axios"; import { URL_CONSTANTS } from "@/constants/URLS"; import { API_BASE_URL } from "@/constants/apiConfig"; -import { - emitApiError, - extractApiErrorPayload, -} from "@/components/errors/ApiErrorModal"; import { captureApiError } from "@/lib/posthog"; const client = axios.create({ @@ -63,10 +59,9 @@ async function refreshSessionTokens(): Promise { } type TokenPair = { token: string; refreshToken: string }; - const { data } = await client.post & { data?: TokenPair }>( - URL_CONSTANTS.AUTH.REFRESH_TOKEN, - { refreshToken }, - ); + const { data } = await client.post< + Partial & { data?: TokenPair } + >(URL_CONSTANTS.AUTH.REFRESH_TOKEN, { refreshToken }); // The API returns the pair flat ({ success, token, refreshToken }); accept // a { data: { ... } }-wrapped shape too so a transform change can't // silently break refresh again. @@ -118,8 +113,8 @@ client.interceptors.response.use( // Surface the server's actual error message in the global error modal // (401s are handled by the session flow below/redirects, so skip them). if (error.response && error.response.status !== 401) { - const payload = extractApiErrorPayload(error); - if (payload) emitApiError(payload); + // const payload = extractApiErrorPayload(error); + // if (payload) emitApiError(payload); } return Promise.reject(error); } From 0549a88d57467a052e06693623db65cc293d5343 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 20 Jul 2026 12:10:49 +0000 Subject: [PATCH 5/6] feat: otp double sending --- .../modules/auth/dto/forgot-password.dto.ts | 30 +- .../auth/forgot-password.controller.ts | 16 +- .../modules/auth/forgot-password.service.ts | 43 ++- .../src/modules/auth/mask-target.util.ts | 25 +- .../contracts/contract-transition.service.ts | 80 +++-- .../src/modules/otp/otp.controller.ts | 15 +- .../src/modules/otp/otp.repository.ts | 134 ++++++--- .../src/modules/otp/otp.service.spec.ts | 231 +++++++++++--- .../src/modules/otp/otp.service.ts | 284 ++++++++++++------ .../backoffice/src/auth/types.ts | 4 - .../src/components/auth/OtpChannelStep.tsx | 102 +++---- .../src/pages/auth/ForgotPasswordPage.tsx | 28 +- .../src/components/auth/OtpChannelStep.tsx | 102 +++---- .../src/pages/accounts/ForgotPasswordPage.tsx | 28 +- .../portal/src/pages/accounts/SignupPage.tsx | 55 ++-- .../src/pages/contracts/ContractViewPage.tsx | 16 +- apps/edr-freight-web/portal/src/types/auth.ts | 9 +- 17 files changed, 741 insertions(+), 461 deletions(-) diff --git a/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts b/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts index c0bb0c151..34e16f628 100644 --- a/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts +++ b/apps/edr-freight-api/src/modules/auth/dto/forgot-password.dto.ts @@ -1,7 +1,11 @@ -import { ApiProperty } from "@nestjs/swagger"; -import { IsEnum, IsNotEmpty, IsString, IsUUID } from "class-validator"; +import { ApiProperty, ApiPropertyOptional } from "@nestjs/swagger"; +import { IsEnum, IsNotEmpty, IsOptional, IsString, IsUUID } from "class-validator"; -/** The channel the reset code is delivered over. */ +/** + * The channel a reset LINK is delivered over. The OTP flow no longer picks one — + * it sends to every contact on the account — but the staff-triggered link flow + * still delivers over exactly one transport. + */ export enum ResetChannel { Email = "email", Phone = "phone", @@ -16,13 +20,27 @@ export class ForgotPasswordRequestDto { @IsNotEmpty() identifier!: string; - @ApiProperty({ enum: ResetChannel }) + /** + * Accepted and ignored. The code now goes to the account's email AND phone, + * so there is nothing to choose — kept optional so clients still sending it + * (older portal/backoffice builds) are not rejected outright. + * @deprecated + */ + @ApiPropertyOptional({ + enum: ResetChannel, + deprecated: true, + description: "Ignored — the code is sent to every contact on the account.", + }) + @IsOptional() @IsEnum(ResetChannel) - channel!: ResetChannel; + channel?: ResetChannel; } export class ForgotPasswordVerifyDto extends ForgotPasswordRequestDto { - @ApiProperty({ description: "The 6-digit code sent to the chosen channel" }) + @ApiProperty({ + description: + "The 6-digit code sent to the account's email and phone. Either delivery carries the same code.", + }) @IsString() @IsNotEmpty() otp!: string; diff --git a/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts b/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts index 6fef56e0f..448a380c9 100644 --- a/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts +++ b/apps/edr-freight-api/src/modules/auth/forgot-password.controller.ts @@ -29,17 +29,19 @@ export class ForgotPasswordController { @Post("forgot-password/request") @ApiOperation({ - summary: "Send a password-reset code over email or SMS", + summary: "Send a password-reset code to the account's email AND phone", description: - "Always reports success. An unknown, inactive, or channel-less account is " + - "indistinguishable from a real one, so this cannot be used to enumerate accounts.", + "One code, delivered over every contact the account has; either delivery " + + "verifies it. Always reports success — an unknown, inactive, or contactless " + + "account is indistinguishable from a real one, so this cannot be used to " + + "enumerate accounts.", }) async request(@Body() dto: ForgotPasswordRequestDto): Promise<{ success: true }> { const user = await this.forgotPasswordService.resolveActiveUser(dto.identifier); if (user) { try { - await this.forgotPasswordService.requestReset(user, dto.channel); + await this.forgotPasswordService.requestReset(user); } catch (error) { // A delivery failure must not change the response shape either — log it // and let the caller sit on the OTP screen. @@ -65,11 +67,7 @@ export class ForgotPasswordController { "alongside the same identifier and the new password.", }) verify(@Body() dto: ForgotPasswordVerifyDto): Promise { - return this.forgotPasswordService.verifyAndMintTicket( - dto.identifier, - dto.channel, - dto.otp, - ); + return this.forgotPasswordService.verifyAndMintTicket(dto.identifier, dto.otp); } @Post("forgot-password/resolve-link") diff --git a/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts b/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts index 8a470c864..a3dbf1061 100644 --- a/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts +++ b/apps/edr-freight-api/src/modules/auth/forgot-password.service.ts @@ -102,7 +102,11 @@ export class ForgotPasswordService { .orderBy("u.createdAt", "DESC"); } - /** The address the code goes to, taken from the account — never from input. */ + /** + * A single channel of the account, for flows that genuinely deliver over one + * transport (the staff-triggered reset LINK picks email or SMS). Taken from + * the account — never from input. + */ targetFor(user: User, channel: ResetChannel): OtpTarget | null { if (channel === ResetChannel.Email) { return user.email ? { email: user.email } : null; @@ -110,6 +114,19 @@ export class ForgotPasswordService { return user.phoneNumber ? { phone: user.phoneNumber } : null; } + /** + * Every contact the account has. The reset OTP goes to all of them and any one + * verifies it — a customer whose SMS never lands can finish from their inbox + * without restarting the flow on a different channel. An account holding only + * one of the two degrades to that channel; only a contactless account is null. + */ + targetsFor(user: User): OtpTarget | null { + const target: OtpTarget = {}; + if (user.email) target.email = user.email; + if (user.phoneNumber) target.phone = user.phoneNumber; + return target.email || target.phone ? target : null; + } + /** * The value IAM's `set-password` will match this account on. It looks the user * up by email OR username OR phoneNumber (and lowercases whatever it is @@ -121,20 +138,17 @@ export class ForgotPasswordService { } /** - * Send a reset code to the account's own email/phone. Returns the target so - * authenticated (backoffice) callers can echo a masked version; unauthenticated - * callers must discard it. + * Send one reset code to every contact on the account — email AND phone — + * returning the target so authenticated (backoffice) callers can echo a masked + * version; unauthenticated callers must discard it. * * Note: `otp_verifications` keys rows by a unique phone/email, and `sendOtp` - * upserts. A reset request therefore overwrites any pending signup code for - * the same address — last code sent wins. That is the pre-existing behaviour - * between any two flows sharing this table. + * replaces every row the target overlaps. A reset request therefore overwrites + * any pending signup code for the same addresses — last code sent wins. That + * is the pre-existing behaviour between any two flows sharing this table. */ - async requestReset( - user: User, - channel: ResetChannel, - ): Promise { - const target = this.targetFor(user, channel); + async requestReset(user: User): Promise { + const target = this.targetsFor(user); if (!target) return null; await this.otpService.sendOtp(target); @@ -151,11 +165,12 @@ export class ForgotPasswordService { */ async verifyAndMintTicket( identifier: string, - channel: ResetChannel, otp: string, ): Promise { const user = await this.resolveActiveUser(identifier); - const target = user && this.targetFor(user, channel); + // Same set of contacts `requestReset` sent to, so the code resolves whichever + // of the two the customer actually received it on. + const target = user && this.targetsFor(user); if (!user?.id || !target) { // Same shape as a wrong code: a caller probing for accounts learns nothing diff --git a/apps/edr-freight-api/src/modules/auth/mask-target.util.ts b/apps/edr-freight-api/src/modules/auth/mask-target.util.ts index 213a14656..81d49a522 100644 --- a/apps/edr-freight-api/src/modules/auth/mask-target.util.ts +++ b/apps/edr-freight-api/src/modules/auth/mask-target.util.ts @@ -1,16 +1,27 @@ import { OtpTarget } from "../otp/otp.service"; +function maskEmail(email: string): string { + const [local, domain] = email.split("@"); + const head = local.slice(0, 1); + return `${head}${"•".repeat(Math.max(local.length - 1, 1))}@${domain}`; +} + +function maskPhone(phone: string): string { + return `${phone.slice(0, 4)}${"•".repeat(Math.max(phone.length - 8, 1))}${phone.slice(-4)}`; +} + /** * Mask an OTP target for echoing back to the caller: `+251911234567` -> * `+251•••••4567`; `ab@x.com` -> `a•@x.com`. Never return an unmasked target to * a caller who has not yet proven possession of the channel. + * + * A dual-channel target masks both and joins them, so the UI can say exactly + * where the code went ("a•@x.com and +251•••••4567") — a user who only checks + * one of the two otherwise assumes the other never received anything. */ export function maskOtpTarget(target: OtpTarget): string { - if (target.email) { - const [local, domain] = target.email.split("@"); - const head = local.slice(0, 1); - return `${head}${"•".repeat(Math.max(local.length - 1, 1))}@${domain}`; - } - const phone = target.phone ?? ""; - return `${phone.slice(0, 4)}${"•".repeat(Math.max(phone.length - 8, 1))}${phone.slice(-4)}`; + const parts: string[] = []; + if (target.email) parts.push(maskEmail(target.email)); + if (target.phone) parts.push(maskPhone(target.phone)); + return parts.join(" and "); } diff --git a/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts index bba044c85..6acceaaa0 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts @@ -73,6 +73,27 @@ function maskPhone(phone: string): string { return `${'•'.repeat(trimmed.length - 4)}${trimmed.slice(-4)}`; } +/** Email counterpart of {@link maskPhone} (`jane@x.com` → `j•••@x.com`). */ +function maskEmail(email: string): string { + const [local, domain] = email.trim().split('@'); + if (!domain) return email.trim(); + return `${local.slice(0, 1)}${'•'.repeat(Math.max(local.length - 1, 1))}@${domain}`; +} + +/** + * Where the signing code went, for the "we sent a code to …" line in the UI. + * Both contacts are listed when both were used — a signer who only watches their + * handset otherwise has no idea the email carries the same code. + */ +function maskSignerContacts(contacts: { phone?: string; email?: string }): string { + return [ + contacts.email ? maskEmail(contacts.email) : null, + contacts.phone ? maskPhone(contacts.phone) : null, + ] + .filter(Boolean) + .join(' and '); +} + /** Status-machine guard mirroring booking-status.util. */ function assertContractStatus(contract: Contract, allowed: string[]): void { if (!allowed.includes(contract.status)) { @@ -109,34 +130,39 @@ export class ContractTransitionService { ) {} /** - * The phone the signing OTP is sent to and verified against: the signer's own - * IAM account number. + * The contacts the signing OTP is sent to and verified against: the signer's + * own IAM account phone AND email. One code goes to both and either delivery + * verifies it, so a signer whose SMS is delayed can still complete from their + * inbox instead of abandoning a ready contract. * * H12(b): resolved server-side from the authenticated user id, never from the - * request body — a caller-supplied number would let an attacker point the code - * at their own phone. Ownership is already gated separately by + * request body — caller-supplied contacts would let an attacker point the code + * at their own phone or mailbox. Ownership is already gated separately by * {@link ContractsService.assertCustomerCanAccessContract}, so this binds the * signature to the *person* signing rather than to a company landline that may * be shared, stale, or imported from eTrade. */ - private async resolveSignerPhone(signerUserId?: string): Promise { + private async resolveSignerContacts( + signerUserId?: string, + ): Promise<{ phone?: string; email?: string }> { if (!signerUserId) { // Unreachable in practice (the ownership gate rejects a missing user - // first), but never fall back to another number if it ever changes. + // first), but never fall back to another account if it ever changes. throw new BadRequestException('Authentication required to sign'); } - const rows: Array<{ phone_number: string | null }> = + const rows: Array<{ phone_number: string | null; email: string | null }> = await this.dataSource.query( - `SELECT phone_number FROM iam.users WHERE id = $1 AND is_active = true`, + `SELECT phone_number, email FROM iam.users WHERE id = $1 AND is_active = true`, [signerUserId], ); const phone = rows[0]?.phone_number?.trim(); - if (!phone) { + const email = rows[0]?.email?.trim(); + if (!phone && !email) { throw new BadRequestException( - 'Your account has no registered phone number. Add one in Settings → Account before signing.', + 'Your account has no registered phone number or email. Add one in Settings → Account before signing.', ); } - return phone; + return { ...(phone ? { phone } : {}), ...(email ? { email } : {}) }; } /** Customer submits the contract for approval → SUBMITTED; freeze unit rates. */ @@ -839,11 +865,11 @@ export class ContractTransitionService { } /** - * Send the sudo-mode signing OTP to the SIGNER's own registered phone — the - * same number {@link sign} verifies against. The client never picks the number - * (that is the H12(b) trust property): it only asks us to send, and we resolve - * the phone from the authenticated user id. Returns a masked hint so the UI can - * say where the code went without exposing the full number. + * Send the sudo-mode signing OTP to the SIGNER's own registered phone and + * email — the same contacts {@link sign} verifies against. The client never + * picks them (that is the H12(b) trust property): it only asks us to send, and + * we resolve them from the authenticated user id. Returns a masked hint so the + * UI can say where the code went without exposing the full values. */ async sendSigningOtp( contractId: string, @@ -858,9 +884,9 @@ export class ContractTransitionService { ); assertContractStatus(contract, ['CONTRACT_READY']); - const signerPhone = await this.resolveSignerPhone(options.signerUserId); - await this.otpService.sendOtp({ phone: signerPhone }); - return { sentTo: maskPhone(signerPhone) }; + const signerContacts = await this.resolveSignerContacts(options.signerUserId); + await this.otpService.sendOtp(signerContacts); + return { sentTo: maskSignerContacts(signerContacts) }; } /** Customer signs the ready contract → SIGNED_CUSTOMER. */ @@ -887,17 +913,17 @@ export class ContractTransitionService { } // Sudo-mode gate: a fresh, single-use OTP must be verified before the // signature is applied. H12(b): verify against the SIGNER's own registered - // phone, resolved server-side from the authenticated user id — never a - // caller-supplied number, which an attacker could point at their own - // phone. Ownership is already asserted above, so this proves the specific - // person holding the account is present, not merely that someone reached a - // shared company line. Must resolve identically to sendSigningOtp, or send - // and verify would target different numbers. - const signerPhone = await this.resolveSignerPhone(options.signerUserId); + // contacts, resolved server-side from the authenticated user id — never + // caller-supplied ones, which an attacker could point at their own phone + // or mailbox. Ownership is already asserted above, so this proves the + // specific person holding the account is present, not merely that someone + // reached a shared company line. Must resolve identically to + // sendSigningOtp, or send and verify would target different contacts. + const signerContacts = await this.resolveSignerContacts(options.signerUserId); if (!dto.otp) { throw new BadRequestException('OTP verification is required to sign the contract'); } - await this.otpService.verifyOtpForAction({ phone: signerPhone }, dto.otp); + await this.otpService.verifyOtpForAction(signerContacts, dto.otp); await this.applySignature(contract, dto, options); await this.contractsRepository.update(contractId, { status: 'SIGNED_CUSTOMER', diff --git a/apps/edr-freight-api/src/modules/otp/otp.controller.ts b/apps/edr-freight-api/src/modules/otp/otp.controller.ts index 6b0078429..289cbe2d0 100644 --- a/apps/edr-freight-api/src/modules/otp/otp.controller.ts +++ b/apps/edr-freight-api/src/modules/otp/otp.controller.ts @@ -11,12 +11,17 @@ import { import { OtpService, OtpTarget } from "./otp.service"; import { Public } from "@edr/api-common"; -// Exactly one of phone/email must be present per request — the channel the -// code is sent through / checked against. +// At least one of phone/email must be present. When BOTH are given the code is +// sent to both and either one verifies it — the caller no longer picks a single +// channel, it just states every address it knows for the account. function toTarget(phone?: string, email?: string): OtpTarget { - if (email) return { email }; - if (phone) return { phone }; - throw new BadRequestException("phone or email is required"); + const target: OtpTarget = {}; + if (email?.trim()) target.email = email; + if (phone?.trim()) target.phone = phone; + if (!target.email && !target.phone) { + throw new BadRequestException("phone or email is required"); + } + return target; } // TODO: these public routes need per-target + per-IP rate limiting (a NestJS diff --git a/apps/edr-freight-api/src/modules/otp/otp.repository.ts b/apps/edr-freight-api/src/modules/otp/otp.repository.ts index 7abd434d8..a264c5351 100644 --- a/apps/edr-freight-api/src/modules/otp/otp.repository.ts +++ b/apps/edr-freight-api/src/modules/otp/otp.repository.ts @@ -4,10 +4,12 @@ import { Injectable } from "@nestjs/common"; import { InjectRepository } from "@nestjs/typeorm"; -import { Repository } from "typeorm"; +import { FindOptionsWhere, Repository } from "typeorm"; import { OtpVerification } from "./otp.entity"; +type Target = { phone?: string; email?: string }; + @Injectable() export class OtpRepository { constructor( @@ -46,54 +48,112 @@ export class OtpRepository { } // --------------------------------------------------------------------------- - // Find By Target (either channel) + // Find By Target (any named channel) // --------------------------------------------------------------------------- - async findByTarget( - target: { phone?: string; email?: string } - ) { - return target.email - ? this.findByEmail(target.email) - : this.findByPhone(target.phone!); - } + /** + * OR across every channel the target names. A code sent to both phone and + * email lives in ONE row carrying both values, so a verify that quotes either + * one resolves the same row — that is what makes "sent to both, verify with + * either" work. + */ + private whereForTarget( + target: Target + ): FindOptionsWhere[] { + const where: FindOptionsWhere[] = + []; - // --------------------------------------------------------------------------- - // Create OTP - // --------------------------------------------------------------------------- - - async createOtp( - target: { phone?: string; email?: string }, - otp: string - ) { - const entity = - this.repository.create({ - phone: target.phone, + if (target.email) + where.push({ email: target.email, - otp, - verified: false, }); - return this.repository.save( - entity + if (target.phone) + where.push({ + phone: target.phone, + }); + + return where; + } + + async findAllByTarget( + target: Target + ) { + const where = + this.whereForTarget(target); + + if (!where.length) return []; + + // Newest first: a target that somehow overlaps two legacy single-channel + // rows should resolve to the most recently issued code, not an arbitrary one. + return this.repository.find({ + where, + order: { updatedAt: "DESC" }, + }); + } + + async findByTarget( + target: Target + ) { + const [ + newest, + ] = await this.findAllByTarget( + target ); + + return newest ?? null; } // --------------------------------------------------------------------------- - // Update OTP + // Replace OTP (upsert across every channel the target names) // --------------------------------------------------------------------------- - async updateOtp( - otpVerification: OtpVerification, + /** + * Drop every row this target overlaps and write a single fresh one holding + * all its channels. + * + * `phone` and `email` are each UNIQUE, so a dual-channel send can collide with + * up to two pre-existing single-channel rows (say an old signup code on the + * phone and a reset code on the email). Merging into one row instead of + * updating in place is what keeps that from raising a unique violation, and it + * preserves the single-use guarantee: consuming the code deletes one row and + * kills every channel it was sent to at once. + * + * "Last code sent wins" was already the behaviour between any two flows + * sharing this table — this only widens it from one channel to all of them. + */ + async replaceOtp( + target: Target, otp: string - ) { - otpVerification.otp = otp; + ): Promise<{ + record: OtpVerification; + rotated: boolean; + }> { + const existing = + await this.findAllByTarget( + target + ); - otpVerification.verified = - false; + if (existing.length) { + await this.repository.remove( + existing + ); + } - return this.repository.save( - otpVerification - ); + const record = + await this.repository.save( + this.repository.create({ + phone: target.phone, + email: target.email, + otp, + verified: false, + }) + ); + + return { + record, + rotated: existing.length > 0, + }; } // --------------------------------------------------------------------------- @@ -115,8 +175,8 @@ export class OtpRepository { // Delete OTP (single-use consume) // --------------------------------------------------------------------------- - // Hard delete so the unique `phone` row is freed and a fresh code can be - // requested for the same number on the next action. + // Hard delete so the unique `phone`/`email` rows are freed and a fresh code can + // be requested for the same target on the next action. async deleteOtp( otpVerification: OtpVerification ) { @@ -124,4 +184,4 @@ export class OtpRepository { otpVerification ); } -} \ No newline at end of file +} diff --git a/apps/edr-freight-api/src/modules/otp/otp.service.spec.ts b/apps/edr-freight-api/src/modules/otp/otp.service.spec.ts index c9d5c5714..0494b48b6 100644 --- a/apps/edr-freight-api/src/modules/otp/otp.service.spec.ts +++ b/apps/edr-freight-api/src/modules/otp/otp.service.spec.ts @@ -28,55 +28,216 @@ describe('normalizeOtpTarget', () => { }); }); -describe('OtpService — send/verify agree across phone formats', () => { - // In-memory fake keyed by the exact phone string the service stores under, so - // the test proves normalisation makes send and verify collide on one key. - function makeService() { - const rows = new Map(); - const repo = { - findByTarget: jest.fn(async (t: { phone?: string; email?: string }) => - rows.get(t.email ?? t.phone!) ?? null, - ), - updateOtp: jest.fn(async (existing: { otp: string }, otp: string) => { - existing.otp = otp; - }), - createOtp: jest.fn(async (t: { phone?: string; email?: string }, otp: string) => { - rows.set(t.phone ?? t.email!, { ...t, otp, updatedAt: new Date(0) }); - }), - deleteOtp: jest.fn(async (row: { phone?: string; email?: string }) => { - rows.delete(row.phone ?? row.email!); - }), - }; - // Both clients return `{ queued }` — the service reads it to tell a published - // code apart from one the transport silently dropped. - const sms = { sendSms: jest.fn().mockResolvedValue({ queued: true }) }; - const email = { sendEmail: jest.fn().mockResolvedValue({ queued: true }) }; - const service = new OtpService(repo as never, sms as never, email as never); - return { service, rows }; - } +interface FakeRow { + id: string; + phone?: string; + email?: string; + otp: string; + updatedAt: Date; +} +/** + * In-memory stand-in for OtpRepository, mirroring the two properties the service + * depends on: rows are matched by OR across every channel named, and a send + * replaces all overlapping rows with one row carrying every channel. + */ +function makeService( + transports: { + sms?: () => Promise<{ queued: boolean }>; + email?: () => Promise<{ queued: boolean }>; + } = {}, +) { + let rows: FakeRow[] = []; + let nextId = 1; + + const matches = (row: FakeRow, t: { phone?: string; email?: string }) => + (!!t.email && row.email === t.email) || (!!t.phone && row.phone === t.phone); + + const repo = { + findByTarget: jest.fn( + async (t: { phone?: string; email?: string }) => + rows.filter((row) => matches(row, t))[0] ?? null, + ), + replaceOtp: jest.fn( + async (t: { phone?: string; email?: string }, otp: string) => { + const overlapping = rows.filter((row) => matches(row, t)); + rows = rows.filter((row) => !overlapping.includes(row)); + const record: FakeRow = { + id: String(nextId++), + ...t, + otp, + updatedAt: new Date(), + }; + rows.push(record); + return { record, rotated: overlapping.length > 0 }; + }, + ), + deleteOtp: jest.fn(async (row: FakeRow) => { + rows = rows.filter((r) => r !== row); + }), + }; + + // Both clients return `{ queued }` — the service reads it to tell a published + // code apart from one the transport silently dropped. + const sms = { + sendSms: jest.fn(transports.sms ?? (async () => ({ queued: true }))), + }; + const email = { + sendEmail: jest.fn(transports.email ?? (async () => ({ queued: true }))), + }; + const service = new OtpService(repo as never, sms as never, email as never); + return { service, sms, email, rows: () => rows }; +} + +describe('OtpService — send/verify agree across phone formats', () => { it('verifies a code sent to +251… when verify is called with 09…', async () => { const { service, rows } = makeService(); await service.sendOtp({ phone: '+251986680099' }); - const stored = [...rows.values()][0]!.otp; - - // Fresh TTL: stamp updatedAt to now so the action verifier does not expire it. - [...rows.values()][0]!.updatedAt = new Date(); await expect( - service.verifyOtpForAction({ phone: '0986680099' }, stored), + service.verifyOtpForAction({ phone: '0986680099' }, rows()[0]!.otp), ).resolves.toEqual({ success: true }); }); it('verifies a code sent to User@X.com when verify is called with user@x.com', async () => { const { service, rows } = makeService(); await service.sendOtp({ email: ' User@Example.COM ' }); - const stored = [...rows.values()][0]!.otp; - - [...rows.values()][0]!.updatedAt = new Date(); await expect( - service.verifyOtpForAction({ email: 'user@example.com' }, stored), + service.verifyOtpForAction({ email: 'user@example.com' }, rows()[0]!.otp), ).resolves.toEqual({ success: true }); }); }); + +describe('OtpService — dual-channel send', () => { + const both = { phone: '0986680099', email: 'User@Example.COM' }; + + it('sends ONE code to both transports', async () => { + const { service, sms, email, rows } = makeService(); + await service.sendOtp(both); + + const otp = rows()[0]!.otp; + expect(sms.sendSms).toHaveBeenCalledTimes(1); + expect(email.sendEmail).toHaveBeenCalledTimes(1); + // Same secret on both messages — the user types whichever arrives first. + expect(sms.sendSms).toHaveBeenCalledWith( + expect.objectContaining({ + to: '+251986680099', + message: expect.stringContaining(otp), + }), + ); + expect(email.sendEmail).toHaveBeenCalledWith( + expect.objectContaining({ + to: 'user@example.com', + text: expect.stringContaining(otp), + }), + ); + // One row, both channels canonicalised. + expect(rows()).toHaveLength(1); + expect(rows()[0]).toMatchObject({ + phone: '+251986680099', + email: 'user@example.com', + }); + }); + + it.each([ + ['phone alone', { phone: '0986680099' }], + ['email alone', { email: 'user@example.com' }], + ['both', both], + ])('verifies a dual-channel code when quoted back by %s', async (_label, target) => { + const { service, rows } = makeService(); + await service.sendOtp(both); + + await expect( + service.verifyOtpForAction(target, rows()[0]!.otp), + ).resolves.toEqual({ success: true }); + }); + + it('consuming the code via one channel kills the other', async () => { + const { service, rows } = makeService(); + await service.sendOtp(both); + const otp = rows()[0]!.otp; + + await service.verifyOtpForAction({ email: 'user@example.com' }, otp); + + // Single-use is per-code, not per-channel: the phone half must be dead too. + await expect( + service.verifyOtpForAction({ phone: '0986680099' }, otp), + ).rejects.toThrow(/No verification code was requested/); + }); + + it('replaces an overlapping single-channel row instead of colliding with it', async () => { + const { service, rows } = makeService(); + // A pending signup code on the phone only, then a dual-channel send. + await service.sendOtp({ phone: '0986680099' }); + await service.sendOtp(both); + + expect(rows()).toHaveLength(1); + expect(rows()[0]).toMatchObject({ email: 'user@example.com' }); + }); + + it('degrades to one channel when the account has only one contact', async () => { + const { service, sms, email } = makeService(); + await service.sendOtp({ phone: '0986680099' }); + + expect(sms.sendSms).toHaveBeenCalledTimes(1); + expect(email.sendEmail).not.toHaveBeenCalled(); + }); + + it('still succeeds when one transport throws', async () => { + const { service, rows } = makeService({ + sms: async () => { + throw new Error('broker down'); + }, + }); + + await expect(service.sendOtp(both)).resolves.toMatchObject({ + success: true, + delivered: true, + }); + // The code is live and verifiable on the channel that worked. + await expect( + service.verifyOtpForAction({ email: 'user@example.com' }, rows()[0]!.otp), + ).resolves.toEqual({ success: true }); + }); + + it('fails the request when every transport throws', async () => { + const { service } = makeService({ + sms: async () => { + throw new Error('broker down'); + }, + email: async () => { + throw new Error('broker down'); + }, + }); + + await expect(service.sendOtp(both)).rejects.toThrow('Failed to send OTP'); + }); + + it('shares one brute-force budget across both channels', async () => { + const { service, rows } = makeService(); + await service.sendOtp(both); + const otp = rows()[0]!.otp; + + // Alternating channels must not hand the attacker two independent budgets: + // 5 wrong guesses in total burn the code regardless of how they are split. + for (const target of [ + { phone: '0986680099' }, + { email: 'user@example.com' }, + { phone: '0986680099' }, + { email: 'user@example.com' }, + ]) { + await expect(service.verifyOtpForAction(target, '000000')).rejects.toThrow( + 'Invalid verification code', + ); + } + await expect( + service.verifyOtpForAction({ email: 'user@example.com' }, '000000'), + ).rejects.toThrow(/Too many incorrect attempts/); + + // Burned: even the correct code no longer works. + await expect(service.verifyOtpForAction(both, otp)).rejects.toThrow( + /No verification code was requested/, + ); + }); +}); diff --git a/apps/edr-freight-api/src/modules/otp/otp.service.ts b/apps/edr-freight-api/src/modules/otp/otp.service.ts index 5e91251c9..e19323067 100644 --- a/apps/edr-freight-api/src/modules/otp/otp.service.ts +++ b/apps/edr-freight-api/src/modules/otp/otp.service.ts @@ -8,10 +8,24 @@ import { OtpRepository } from "./otp.repository"; import { SmsClientService } from "../notifications/sms-client.service"; import { EmailClientService } from "../notifications/email-client.service"; -// Exactly one of phone/email is set — enforced by the controller before it -// reaches here. +/** + * Where a code goes. At least one of phone/email must be set — enforced by the + * controller and re-checked here. When BOTH are set the same code is sent to + * both and either one can be used to verify it: a user who never receives the + * SMS can still finish from their inbox, and vice versa. Callers that resolve + * contacts from IAM pass whatever the account actually has, so an account with + * only one of the two silently degrades to a single channel. + */ export type OtpTarget = { phone?: string; email?: string }; +/** Which transports a target resolves to, in a stable order for logging. */ +function channelsOf(target: OtpTarget): Array<"email" | "sms"> { + const channels: Array<"email" | "sms"> = []; + if (target.email) channels.push("email"); + if (target.phone) channels.push("sms"); + return channels; +} + /** * Canonicalise a phone to E.164 so the code stored on send and the one looked * up on verify collide regardless of how the number was typed. Without this, @@ -19,30 +33,51 @@ export type OtpTarget = { phone?: string; email?: string }; * a code sent to one is invisible to the others — the send/verify halves must * agree on the exact string. Ethiopian local `09…`/`07…` (10 digits) maps to * `+2519…`/`+2517…`; a bare `251…` gains its `+`; anything already `+…` is kept. - * Email targets pass through untouched. + */ +function normalizePhone(rawPhone: string): string { + const raw = rawPhone.trim(); + const digits = raw.replace(/[^\d+]/g, ''); + if (digits.startsWith('+')) return digits; + const bare = digits.replace(/^0+/, ''); + if (/^251\d{9}$/.test(digits)) return `+${digits}`; + if (/^9\d{8}$|^7\d{8}$/.test(bare)) return `+251${bare}`; + // Unknown shape (foreign number, already-clean intl without +) — prefix + if + // it looks like a full international number, else leave as typed. + return digits.length >= 11 ? `+${digits}` : raw; +} + +/** + * Canonicalise every channel present on the target. Each field is normalised + * independently — a dual-channel target must end up with both halves in their + * canonical form, since verify may arrive naming either one. */ export function normalizeOtpTarget(target: OtpTarget): OtpTarget { - if (target.email) { - // Same contract as the phone branch below: the string stored on send and the - // one looked up on verify must be byte-identical, or the code is invisible to - // the verifier. Addresses reach us from a raw `@Body("email")` with no DTO or - // ValidationPipe, so `User@X.com`, `user@x.com` and a copy-paste with a + const normalized: OtpTarget = {}; + + if (target.email?.trim()) { + // Same contract as the phone branch: the string stored on send and the one + // looked up on verify must be byte-identical, or the code is invisible to + // the verifier. Addresses reach us from a raw `@Body("email")` with no DTO + // or ValidationPipe, so `User@X.com`, `user@x.com` and a copy-paste with a // trailing space are three different keys for one mailbox. Domains are // case-insensitive (RFC 1035); local-parts are formally case-sensitive // (RFC 5321 §2.4) but no mail provider in practice treats them so, and // matching what users expect beats matching the letter of the spec here. - return { email: target.email.trim().toLowerCase() }; + normalized.email = target.email.trim().toLowerCase(); } - if (!target.phone) return target; - const raw = target.phone.trim(); - const digits = raw.replace(/[^\d+]/g, ''); - if (digits.startsWith('+')) return { phone: digits }; - const bare = digits.replace(/^0+/, ''); - if (/^251\d{9}$/.test(digits)) return { phone: `+${digits}` }; - if (/^9\d{8}$|^7\d{8}$/.test(bare)) return { phone: `+251${bare}` }; - // Unknown shape (foreign number, already-clean intl without +) — prefix + if - // it looks like a full international number, else leave as typed. - return { phone: digits.length >= 11 ? `+${digits}` : raw }; + + if (target.phone?.trim()) { + normalized.phone = normalizePhone(target.phone); + } + + return normalized; +} + +/** One transport's hand-off outcome. Never thrown — collected and reported. */ +interface DispatchOutcome { + channel: "email" | "sms"; + queued: boolean; + error?: string; } @Injectable() @@ -69,35 +104,35 @@ export class OtpService { // --------------------------------------------------------------------------- async sendOtp(rawTarget: OtpTarget) { - // Store under the canonical E.164 key so verify (which normalises the same - // way) always finds this row regardless of how either side typed the number. + // Store under the canonical keys so verify (which normalises the same way) + // always finds this row regardless of how either side typed the number. const target = normalizeOtpTarget(rawTarget); - const channel = target.email ? "email" : "sms"; + const channels = channelsOf(target); const label = this.targetLabel(target); const startedAt = Date.now(); + + if (channels.length === 0) { + throw new BadRequestException("phone or email is required"); + } + try { // The verification code is generated server-side — never supplied by the // caller — so the OTP stays a secret known only to the server and the - // recipient of the SMS/email. + // recipient of the SMS/email. ONE code covers every channel: the user + // types whichever message reaches them first. const otp = this.generateOtp(); - // find existing row for this channel - const existing = await this.otpRepository.findByTarget(target); - - // update existing otp - if (existing) { - await this.otpRepository.updateOtp(existing, otp); - } else { - // create new otp - await this.otpRepository.createOtp(target, otp); - } + // Replaces every row this target overlaps with, so a dual-channel send + // leaves exactly one row holding both halves — verify then resolves the + // same row whichever channel it is given. + const { rotated } = await this.otpRepository.replaceOtp(target, otp); // `rotate` means a code already existed for this target and was replaced — // the previous one is now dead. A user holding a slow-to-arrive SMS and // typing its code will fail against the row; this line is how that shows up // in the log rather than as an unexplained "invalid OTP" report. this.logger.log( - `otp.issue channel=${channel} target=${label} action=${existing ? "rotate" : "create"}`, + `otp.issue channels=${channels.join("+")} target=${label} action=${rotated ? "rotate" : "create"}`, ); // NOTE: do NOT reset the brute-force attempt counter on send. Clearing it @@ -108,35 +143,49 @@ export class OtpService { // /otp/verify routes (a NestJS ThrottlerGuard / @Throttle) — none exists // in the codebase yet. + // Fan out to every channel the target has, independently: one transport + // being down must not suppress the other, which is the whole point of + // sending to both. Each helper swallows its own failure so a rejected + // email publish still leaves the SMS delivered (and the code valid). + const outcomes = ( + await Promise.all([ + target.email ? this.dispatchEmail(target.email, otp) : null, + target.phone ? this.dispatchSms(target.phone, otp) : null, + ]) + ).filter((outcome): outcome is DispatchOutcome => outcome !== null); + + for (const outcome of outcomes) { + this.logger.log( + `otp.dispatch channel=${outcome.channel} target=${label} queued=${ + outcome.queued + } latencyMs=${Date.now() - startedAt}${ + outcome.error ? ` error=${outcome.error}` : "" + }`, + ); + } + + // Every channel threw. Nothing can arrive and there is no partial success + // to preserve — fail the request the way a single-channel send always did. + if (outcomes.every((outcome) => outcome.error)) { + throw new Error( + outcomes.map((o) => `${o.channel}: ${o.error}`).join("; "), + ); + } + // Both clients report hand-off, not delivery — capture it rather than - // discarding it, so "queued=false" is distinguishable from a code that was - // published fine and lost downstream at the carrier. - const { queued } = target.email - ? await this.emailClient.sendEmail({ - to: target.email, - subject: "Your EDR Freight verification code", - text: `Your verification code is ${otp}`, - }) - : await this.smsClient.sendSms({ - to: target.phone as string, - message: `Your verification code is ${otp}`, - }); + // discarding it, so "delivered=false" is distinguishable from a code that + // was published fine and lost downstream at the carrier. + const delivered = outcomes.some((outcome) => outcome.queued); - this.logger.log( - `otp.dispatch channel=${channel} target=${label} queued=${queued} latencyMs=${ - Date.now() - startedAt - }`, - ); - - if (!queued) { + if (!delivered) { // The row is committed and we are about to answer "OTP sent successfully", // but nothing left this process. Without this line the only symptom is a // user who never receives a code — indistinguishable from carrier loss, // and the misleading success response makes it look like our side worked. this.logger.error( - `otp.dispatch.dropped channel=${channel} target=${label} rabbitmqEnabled=${ + `otp.dispatch.dropped channels=${channels.join("+")} target=${label} rabbitmqEnabled=${ process.env.RABBITMQ_ENABLED ?? "unset" - } — transport reported no hand-off; no code will arrive for this send`, + } — no transport reported hand-off; no code will arrive for this send`, ); } @@ -146,13 +195,13 @@ export class OtpService { // aggregation is the debugging path for flaky SMS here) — if that tradeoff // is ever revisited, gate on an env flag rather than deleting the line, so // dev keeps its workflow. - this.logger.log(`OTP send for ${target.email ?? target.phone}: ${otp}`); + this.logger.log(`OTP send for ${label}: ${otp}`); return { success: true, // Distinguishes "we published it" from "the transport is a no-op". The // HTTP response shape is unchanged; the controller drops this field. - delivered: queued, + delivered, message: "OTP sent successfully", }; @@ -160,7 +209,7 @@ export class OtpService { // Log the real cause (DB/SMS/email failure) with its stack so a deployed // "Failed to send OTP" 400 is diagnosable from the API logs, not opaque. this.logger.error( - `otp.dispatch.failed channel=${channel} target=${label} latencyMs=${ + `otp.dispatch.failed channels=${channels.join("+")} target=${label} latencyMs=${ Date.now() - startedAt }: ${error instanceof Error ? error.message : String(error)}`, error instanceof Error ? error.stack : undefined, @@ -170,13 +219,60 @@ export class OtpService { } /** - * Correlation key shared by every `otp.*` line for one address, so a send and - * its later verify can be joined with a single grep. The raw target is used + * Publish to one transport, converting a throw into a reported outcome. A + * broker error on one channel must not abort the other — with dual-channel + * sends the user still has a working route to the code. + */ + private async dispatchEmail( + email: string, + otp: string, + ): Promise { + try { + const { queued } = await this.emailClient.sendEmail({ + to: email, + subject: "Your EDR Freight verification code", + text: `Your verification code is ${otp}`, + }); + return { channel: "email", queued }; + } catch (error) { + return { + channel: "email", + queued: false, + error: error instanceof Error ? error.message : String(error), + }; + } + } + + /** SMS half of {@link dispatchEmail}; same swallow-and-report contract. */ + private async dispatchSms( + phone: string, + otp: string, + ): Promise { + try { + const { queued } = await this.smsClient.sendSms({ + to: phone, + message: `Your verification code is ${otp}`, + }); + return { channel: "sms", queued }; + } catch (error) { + return { + channel: "sms", + queued: false, + error: error instanceof Error ? error.message : String(error), + }; + } + } + + /** + * Correlation key shared by every `otp.*` line for one target, so a send and + * its later verify can be joined with a single grep. The raw values are used * because the code itself is already logged in cleartext above — hashing the * address while printing the credential next to it would buy nothing. */ private targetLabel(target: OtpTarget): string { - return target.email ?? target.phone ?? "unknown"; + return ( + [target.email, target.phone].filter(Boolean).join("+") || "unknown" + ); } /** @@ -190,15 +286,37 @@ export class OtpService { result: "ok" | "invalid" | "expired" | "exhausted" | "not_found", detail?: string, ) { - const line = `otp.verify channel=${ - target.email ? "email" : "sms" - } target=${this.targetLabel(target)} mode=${mode} result=${result}${ + const line = `otp.verify channels=${channelsOf(target).join( + "+", + )} target=${this.targetLabel(target)} mode=${mode} result=${result}${ detail ? ` ${detail}` : "" }`; if (result === "ok") this.logger.log(line); else this.logger.warn(line); } + /** + * "No code for this target" phrased for whichever channels were named. A + * dual-channel caller gets a neutral message — naming one channel would be + * misleading when the code went to both. + */ + private notFoundMessage(target: OtpTarget, requested: boolean): string { + const channels = channelsOf(target); + if (channels.length !== 1) { + return requested + ? "No verification code was requested for this account" + : "No verification code found for this account"; + } + if (target.email) { + return requested + ? "No verification code was requested for this email" + : "Email address not found"; + } + return requested + ? "No verification code was requested for this phone" + : "Phone number not found"; + } + // --------------------------------------------------------------------------- // Verify OTP // --------------------------------------------------------------------------- @@ -207,20 +325,24 @@ export class OtpService { // Same canonicalisation as sendOtp so a code stored under +2519… is found // when verify is called with 09… (or any equivalent form). const target = normalizeOtpTarget(rawTarget); - // find the channel's row + // Matches on ANY channel the caller named, so a code sent to both phone and + // email verifies whichever one the user quotes back. const otpData = await this.otpRepository.findByTarget(target); - const key = this.targetKey(target); // not found if (!otpData) { - // No row for this key. Most often a normalisation mismatch or a code that - // was already consumed/burned — not necessarily a caller who never asked. + // No row for this target. Most often a normalisation mismatch or a code + // that was already consumed/burned — not necessarily a caller who never + // asked. this.logVerify(target, "simple", "not_found"); - throw new BadRequestException( - target.email ? "Email address not found" : "Phone number not found", - ); + throw new BadRequestException(this.notFoundMessage(target, false)); } + // Key the attempt budget on the ROW, not on the channels the caller happened + // to name — otherwise guessing alternately by phone and by email would hand + // an attacker two independent budgets against the same code. + const key = otpData.id; + // TTL: reuse the same age window as the hardened action verifier — an old // code can't be verified. const ageMs = Date.now() - new Date(otpData.updatedAt).getTime(); @@ -265,7 +387,8 @@ export class OtpService { throw new BadRequestException("Invalid OTP"); } - // single-use: consume the code on success so it can't be replayed. + // single-use: consume the code on success so it can't be replayed. One row + // covers every channel it was sent to, so this kills all of them at once. await this.otpRepository.deleteOtp(otpData); this.actionAttempts.delete(key); this.logVerify(target, "simple", "ok", `ageMs=${ageMs}`); @@ -273,9 +396,7 @@ export class OtpService { return { success: true, - message: target.email - ? "Email verified successfully" - : "Phone verified successfully", + message: "Verification successful", }; } @@ -297,10 +418,6 @@ export class OtpService { private readonly MAX_ACTION_ATTEMPTS = 5; private readonly actionAttempts = new Map(); - private targetKey(target: OtpTarget): string { - return target.email ? `email:${target.email}` : `phone:${target.phone}`; - } - async verifyOtpForAction( rawTarget: OtpTarget, otp: string, @@ -308,17 +425,14 @@ export class OtpService { ) { const target = normalizeOtpTarget(rawTarget); const otpData = await this.otpRepository.findByTarget(target); - const key = this.targetKey(target); if (!otpData) { this.logVerify(target, "action", "not_found"); - throw new BadRequestException( - target.email - ? "No verification code was requested for this email" - : "No verification code was requested for this phone", - ); + throw new BadRequestException(this.notFoundMessage(target, true)); } + // Row-keyed for the same reason as verifyOtp: one code, one budget. + const key = otpData.id; const ageMs = Date.now() - new Date(otpData.updatedAt).getTime(); if (ageMs > ttlMs) { diff --git a/apps/edr-freight-web/backoffice/src/auth/types.ts b/apps/edr-freight-web/backoffice/src/auth/types.ts index 1c79819d5..2dac243b4 100644 --- a/apps/edr-freight-web/backoffice/src/auth/types.ts +++ b/apps/edr-freight-web/backoffice/src/auth/types.ts @@ -58,13 +58,9 @@ export interface LoginResponse extends Partial { mfaRequired?: boolean; } -/** The channel a password-reset code is delivered over. */ -export type ResetChannel = "email" | "phone"; - export interface ForgotPasswordRequestPayload { /** Email, username, or E.164 phone — whatever the user typed, normalised. */ identifier: string; - channel: ResetChannel; } export interface ForgotPasswordVerifyPayload extends ForgotPasswordRequestPayload { diff --git a/apps/edr-freight-web/backoffice/src/components/auth/OtpChannelStep.tsx b/apps/edr-freight-web/backoffice/src/components/auth/OtpChannelStep.tsx index b61bb2eab..59c57ae13 100644 --- a/apps/edr-freight-web/backoffice/src/components/auth/OtpChannelStep.tsx +++ b/apps/edr-freight-web/backoffice/src/components/auth/OtpChannelStep.tsx @@ -1,70 +1,20 @@ -import { Alert, Button, PinInput, SegmentedControl, Stack, Text } from "@mantine/core"; -import { - AlertCircle, - ArrowLeft, - Mail, - RotateCw, - ShieldCheck, - Smartphone, -} from "lucide-react"; +import { Alert, Button, PinInput, Stack, Text } from "@mantine/core"; +import { AlertCircle, ArrowLeft, RotateCw, ShieldCheck } from "lucide-react"; import { maskEmail, maskPhone } from "@/utils/identifier"; -export type OtpChannel = "phone" | "email"; - export const OTP_LENGTH = 6; -export interface OtpChannelSelectProps { - value: OtpChannel; - onChange: (channel: OtpChannel) => void; - disabled?: boolean; - label?: string; -} - -/** Phone/email toggle deciding where the verification code is sent. */ -export function OtpChannelSelect({ - value, - onChange, - disabled, - label = "Send verification code via", -}: OtpChannelSelectProps) { - return ( -
- - {label} - - onChange(v as OtpChannel)} - data={[ - { - value: "phone", - label: ( - - Phone - - ), - }, - { - value: "email", - label: ( - - Email - - ), - }, - ]} - /> -
- ); -} - export interface OtpChannelStepProps { - channel: OtpChannel; - /** Raw email or phone the code went to; masked before display. */ - target: string; + /** + * Raw contacts the code was sent to; masked before display. The API sends one + * code to every contact on the account, so both are usually set — pass only + * what the client actually knows. Omit both when the client cannot know them + * (the forgot-password flow deliberately never reveals an account's contacts) + * and a generic line is shown instead. + */ + email?: string; + phone?: string; value: string; onChange: (otp: string) => void; onVerify: () => void; @@ -82,11 +32,13 @@ export interface OtpChannelStepProps { /** * The "enter the code we sent you" stage. Shared by signup and the - * forgot-password flow — both send through the same `/api/otp/*` service. + * forgot-password flow — both send through the same `/api/otp/*` service, which + * delivers a single code to the account's email AND phone; whichever message + * arrives first can be typed here. */ export default function OtpChannelStep({ - channel, - target, + email, + phone, value, onChange, onVerify, @@ -100,7 +52,10 @@ export default function OtpChannelStep({ description, submitLabel, }: OtpChannelStepProps) { - const maskedTarget = channel === "email" ? maskEmail(target) : maskPhone(target); + const maskedTargets = [ + email ? maskEmail(email) : null, + phone ? maskPhone(phone) : null, + ].filter(Boolean) as string[]; const busy = sending || verifying; return ( @@ -113,12 +68,23 @@ export default function OtpChannelStep({

- {title ?? `Verify your ${channel === "email" ? "email" : "phone"}`} + {title ?? "Verify it's you"}

We sent a {OTP_LENGTH}-digit code to{" "} - {maskedTarget}.{" "} - {description ?? "Enter it to continue."} + {maskedTargets.length ? ( + maskedTargets.map((target, index) => ( + + {index > 0 ? " and " : null} + {target} + + )) + ) : ( + + the email and phone on your account + + )} + . {description ?? "Enter it to continue."}

diff --git a/apps/edr-freight-web/backoffice/src/pages/auth/ForgotPasswordPage.tsx b/apps/edr-freight-web/backoffice/src/pages/auth/ForgotPasswordPage.tsx index 7a21c0d8b..3adae61b0 100644 --- a/apps/edr-freight-web/backoffice/src/pages/auth/ForgotPasswordPage.tsx +++ b/apps/edr-freight-web/backoffice/src/pages/auth/ForgotPasswordPage.tsx @@ -10,11 +10,7 @@ import { } from "@/auth/api"; import type { ResetTicket } from "@/auth/types"; import AuthShell from "@/components/auth/AuthShell"; -import OtpChannelStep, { - OTP_LENGTH, - OtpChannelSelect, - type OtpChannel, -} from "@/components/auth/OtpChannelStep"; +import OtpChannelStep, { OTP_LENGTH } from "@/components/auth/OtpChannelStep"; import PasswordChecklist from "@/components/auth/PasswordChecklist"; import { useResendCooldown } from "@/hooks/useResendCooldown"; import { normaliseIdentifier } from "@/utils/identifier"; @@ -28,7 +24,6 @@ const ForgotPasswordPage = () => { const [stage, setStage] = useState("identify"); const [identifier, setIdentifier] = useState(""); - const [channel, setChannel] = useState("phone"); const [otpCode, setOtpCode] = useState(""); // The reset ticket lives in memory only — persisting it would leave a // password-change credential sitting in localStorage. @@ -45,7 +40,7 @@ const ForgotPasswordPage = () => { const normalised = normaliseIdentifier(identifier); const sendCode = async () => { - await requestPasswordResetRequest({ identifier: normalised, channel }); + await requestPasswordResetRequest({ identifier: normalised }); setOtpCode(""); resendCooldown.start(); }; @@ -89,7 +84,6 @@ const ForgotPasswordPage = () => { try { const result = await verifyPasswordResetOtpRequest({ identifier: normalised, - channel, otp: otpCode.trim(), }); setTicket(result); @@ -138,13 +132,10 @@ const ForgotPasswordPage = () => { } }; - const identifierLabel = - channel === "email" ? "the email on your account" : "the phone on your account"; - return (
{stage === "identify" ? ( @@ -176,16 +167,9 @@ const ForgotPasswordPage = () => { onChange={(event) => setIdentifier(event.target.value)} /> - -

- The code goes to {identifierLabel}, which may differ from what you - typed above. + The code goes to the email and phone on your account, which may + differ from what you typed above.

{error ? ( @@ -217,8 +201,6 @@ const ForgotPasswordPage = () => { {stage === "otp" ? ( void; - disabled?: boolean; - label?: string; -} - -/** Phone/email toggle deciding where the verification code is sent. */ -export function OtpChannelSelect({ - value, - onChange, - disabled, - label = "Send verification code via", -}: OtpChannelSelectProps) { - return ( -
- - {label} - - onChange(v as OtpChannel)} - data={[ - { - value: "phone", - label: ( - - Phone - - ), - }, - { - value: "email", - label: ( - - Email - - ), - }, - ]} - /> -
- ); -} - export interface OtpChannelStepProps { - channel: OtpChannel; - /** Raw email or phone the code went to; masked before display. */ - target: string; + /** + * Raw contacts the code was sent to; masked before display. The API sends one + * code to every contact on the account, so both are usually set — pass only + * what the client actually knows. Omit both when the client cannot know them + * (the forgot-password flow deliberately never reveals an account's contacts) + * and a generic line is shown instead. + */ + email?: string; + phone?: string; value: string; onChange: (otp: string) => void; onVerify: () => void; @@ -82,11 +32,13 @@ export interface OtpChannelStepProps { /** * The "enter the code we sent you" stage. Shared by signup and the - * forgot-password flow — both send through the same `/api/otp/*` service. + * forgot-password flow — both send through the same `/api/otp/*` service, which + * delivers a single code to the account's email AND phone; whichever message + * arrives first can be typed here. */ export default function OtpChannelStep({ - channel, - target, + email, + phone, value, onChange, onVerify, @@ -100,7 +52,10 @@ export default function OtpChannelStep({ description, submitLabel, }: OtpChannelStepProps) { - const maskedTarget = channel === "email" ? maskEmail(target) : maskPhone(target); + const maskedTargets = [ + email ? maskEmail(email) : null, + phone ? maskPhone(phone) : null, + ].filter(Boolean) as string[]; const busy = sending || verifying; return ( @@ -113,12 +68,23 @@ export default function OtpChannelStep({

- {title ?? `Verify your ${channel === "email" ? "email" : "phone"}`} + {title ?? "Verify it's you"}

We sent a {OTP_LENGTH}-digit code to{" "} - {maskedTarget}.{" "} - {description ?? "Enter it to continue."} + {maskedTargets.length ? ( + maskedTargets.map((target, index) => ( + + {index > 0 ? " and " : null} + {target} + + )) + ) : ( + + the email and phone on your account + + )} + . {description ?? "Enter it to continue."}

diff --git a/apps/edr-freight-web/portal/src/pages/accounts/ForgotPasswordPage.tsx b/apps/edr-freight-web/portal/src/pages/accounts/ForgotPasswordPage.tsx index 58cfeab43..1c41dad72 100644 --- a/apps/edr-freight-web/portal/src/pages/accounts/ForgotPasswordPage.tsx +++ b/apps/edr-freight-web/portal/src/pages/accounts/ForgotPasswordPage.tsx @@ -5,11 +5,7 @@ import { Link, useNavigate } from "react-router-dom"; import { useResendCooldown } from "@/hooks/useResendCooldown"; import AuthShell from "@/components/auth/AuthShell"; -import OtpChannelStep, { - OTP_LENGTH, - OtpChannelSelect, - type OtpChannel, -} from "@/components/auth/OtpChannelStep"; +import OtpChannelStep, { OTP_LENGTH } from "@/components/auth/OtpChannelStep"; import PasswordChecklist from "@/components/auth/PasswordChecklist"; import { api } from "@/services/api"; import type { ResetTicket } from "@/types/auth"; @@ -24,7 +20,6 @@ export default function ForgotPasswordPage() { const [stage, setStage] = useState("identify"); const [identifier, setIdentifier] = useState(""); - const [channel, setChannel] = useState("phone"); const [otpCode, setOtpCode] = useState(""); // The reset ticket lives in memory only — persisting it would leave a // password-change credential sitting in localStorage. @@ -41,7 +36,7 @@ export default function ForgotPasswordPage() { const normalised = normaliseIdentifier(identifier); const sendCode = async () => { - await api.auth.requestPasswordReset.call({ identifier: normalised, channel }); + await api.auth.requestPasswordReset.call({ identifier: normalised }); setOtpCode(""); resendCooldown.start(); }; @@ -85,7 +80,6 @@ export default function ForgotPasswordPage() { try { const result = await api.auth.verifyPasswordResetOtp.call({ identifier: normalised, - channel, otp: otpCode.trim(), }); setTicket(result); @@ -134,13 +128,10 @@ export default function ForgotPasswordPage() { } }; - const identifierLabel = - channel === "email" ? "the email on your account" : "the phone on your account"; - return (
{stage === "identify" ? ( @@ -172,16 +163,9 @@ export default function ForgotPasswordPage() { onChange={(event) => setIdentifier(event.target.value)} /> - -

- The code goes to {identifierLabel}, which may differ from what you - typed above. + The code goes to the email and phone on your account, which may + differ from what you typed above.

{error ? ( @@ -213,8 +197,6 @@ export default function ForgotPasswordPage() { {stage === "otp" ? ( (null); - // Two-stage signup: fill the form, then a mandatory OTP challenge on the - // chosen channel before the account is actually created. The account is only - // created after the code is verified — the OTP is a hard requirement. + // Two-stage signup: fill the form, then a mandatory OTP challenge before the + // account is actually created. The code goes to BOTH the email and phone just + // entered — one code, either delivery verifies it — so there is nothing for + // the user to choose. The account is only created after the code is verified; + // the OTP is a hard requirement. const [stage, setStage] = useState<"form" | "otp">("form"); const [pendingData, setPendingData] = useState(null); - // Which contact method the code was sent to — chosen on the form, locked in - // once the challenge is sent. - const [channel, setChannel] = useState("phone"); - const [otpChannel, setOtpChannel] = useState("phone"); const [sending, setSending] = useState(false); const [verifying, setVerifying] = useState(false); const [otpCode, setOtpCode] = useState(""); @@ -101,8 +95,8 @@ export default function SignupPage() { const passwordValue = watch("password") ?? ""; // Step 1 — form is valid: make sure the email/phone aren't already - // registered, then send a fresh code to the chosen channel and move to - // the OTP challenge. + // registered, then send a fresh code to both of them and move to the OTP + // challenge. const requestOtp = async (data: FormData) => { setError(null); setSending(true); @@ -124,11 +118,8 @@ export default function SignupPage() { return; } - await api.auth.sendOTP.call( - channel === "email" ? { email: data.email } : { phone: data.phone }, - ); + await api.auth.sendOTP.call({ email: data.email, phone: data.phone }); setPendingData(data); - setOtpChannel(channel); setOtpCode(""); setOtpError(null); resendCooldown.start(); @@ -145,11 +136,10 @@ export default function SignupPage() { setOtpError(null); setSending(true); try { - await api.auth.sendOTP.call( - otpChannel === "email" - ? { email: pendingData.email } - : { phone: pendingData.phone }, - ); + await api.auth.sendOTP.call({ + email: pendingData.email, + phone: pendingData.phone, + }); setOtpCode(""); resendCooldown.start(); } catch (err) { @@ -170,9 +160,8 @@ export default function SignupPage() { setVerifying(true); try { await api.auth.verifyOTP.call({ - ...(otpChannel === "email" - ? { email: pendingData.email } - : { phone: pendingData.phone }), + email: pendingData.email, + phone: pendingData.phone, otp: otpCode.trim(), }); const payload: SignupPayload = { @@ -259,12 +248,6 @@ export default function SignupPage() { disabled={sending} /> - -
) : ( (null); @@ -168,8 +169,9 @@ export default function ContractViewPage() { if (!signerName.trim()) return; const image = usingSaved ? savedSignatureImage : signatureData; if (!image) return; - // The server resolves and validates the company phone; if none is on file it - // returns a clear 400 that surfaces via the mutation's onError. + // The server resolves and validates the signer's own contacts; if the + // account has neither phone nor email it returns a clear 400 that surfaces + // via the mutation's onError. setOtpCode(""); sendOtpMutation.mutate(); }; @@ -405,8 +407,8 @@ export default function ContractViewPage() { /> - For security, enter the 6-digit code we sent by SMS to the - contract company's registered number + For security, enter the 6-digit code we sent to your registered + contacts {otpSentTo ? ( <> {" "} diff --git a/apps/edr-freight-web/portal/src/types/auth.ts b/apps/edr-freight-web/portal/src/types/auth.ts index 441afc4ba..5f3319737 100644 --- a/apps/edr-freight-web/portal/src/types/auth.ts +++ b/apps/edr-freight-web/portal/src/types/auth.ts @@ -33,7 +33,10 @@ export interface SignupResponse { } export interface OtpPayload { - /** Exactly one of phone/email — the channel the code is sent through. */ + /** + * At least one of phone/email. Send both and the API delivers one code to + * both, verifiable by quoting either back. + */ phone?: string; email?: string; /** Required on verify; omitted on send (the server generates the code). */ @@ -102,13 +105,9 @@ export interface SetPasswordPayload { verificationCode: string; } -/** The channel a password-reset code is delivered over. */ -export type ResetChannel = "email" | "phone"; - export interface ForgotPasswordRequestPayload { /** Email, username, or E.164 phone — whatever the user typed, normalised. */ identifier: string; - channel: ResetChannel; } export interface ForgotPasswordVerifyPayload extends ForgotPasswordRequestPayload { From b90afadfbaff8e89264e0bd23ef87d28597dfc15 Mon Sep 17 00:00:00 2001 From: Marshal Date: Mon, 20 Jul 2026 12:24:58 +0000 Subject: [PATCH 6/6] contrat nad booking modification --- .../src/common/freight-permission.util.ts | 109 ++++++++- ...000000000-DropBookingApprovalWidenRoles.ts | 43 ++++ ...0000000-CreateContractDocumentRevisions.ts | 40 ++++ .../bookings/booking-next-step.util.ts | 7 +- .../booking-transition.accept.spec.ts | 20 +- .../bookings/booking-transition.service.ts | 183 ++------------- .../modules/bookings/bookings.controller.ts | 43 ---- .../src/modules/bookings/bookings.module.ts | 2 - .../modules/bookings/bookings.repository.ts | 56 ----- .../entities/booking-approval-step.entity.ts | 48 ---- .../bookings/entities/booking.entity.ts | 3 - .../contracts/contract-document-diff.util.ts | 168 ++++++++++++++ .../contract-document-history.service.ts | 61 +++++ .../contracts/contract-transition.service.ts | 214 +++++++++++------ .../modules/contracts/contracts.controller.ts | 40 +++- .../src/modules/contracts/contracts.module.ts | 4 + .../entities/contract-approval-step.entity.ts | 4 +- .../contract-document-revision.entity.ts | 36 +++ .../controllers/approval-rules.controller.ts | 9 + .../dto/create-approval-rule.dto.ts | 15 +- .../entities/approval-rule.entity.ts | 4 +- .../modules/rule-engine/rule-engine.module.ts | 2 - .../rule-engine/rule-engine.service.ts | 81 ------- .../services/approval-rules.service.ts | 17 ++ .../components/bookings/ApprovalStepsCard.tsx | 217 ------------------ .../bookings/BookingActionsMenu.tsx | 24 -- .../bookings/BookingApprovalProgressCell.tsx | 29 --- .../bookings/detail/BookingApprovalCard.tsx | 69 ------ .../bookings/detail/booking-detail.styles.ts | 9 - .../src/components/bookings/detail/index.ts | 1 - .../bookings/useBookingActionDialog.ts | 44 +--- .../contracts/ContractActionsToolbar.tsx | 102 ++++---- .../contracts/ContractApprovalStepsCard.tsx | 57 +---- .../contracts/ContractDocumentEditorModal.tsx | 8 +- .../contracts/ContractPreviewModal.tsx | 78 +++++++ .../contracts/ContractRevisionTimeline.tsx | 134 +++++++++++ .../backoffice/src/constants/URLS.ts | 7 +- .../features/bookings/approval-progress.ts | 79 ------- .../bookings/booking-actions.config.ts | 94 +------- .../features/bookings/mapBookingListRow.ts | 3 +- .../src/hooks/bookings/useBookings.ts | 38 --- .../src/hooks/contracts/useContracts.ts | 28 ++- .../src/hooks/rule-engine/useRuleEngine.ts | 32 ++- .../src/pages/bookings/BookingDetailPage.tsx | 40 ---- .../bookings/BookingRequestDetailPage.tsx | 8 - .../pages/bookings/BookingRequestsPage.tsx | 76 +++--- .../contracts/ContractRequestDetailPage.tsx | 2 + .../ruleEngine/RuleEngineResourcePage.tsx | 27 ++- .../src/pages/ruleEngine/config/resources.ts | 20 +- .../backoffice/src/services/api.ts | 14 -- .../src/services/bookings.service.ts | 18 -- .../src/services/contracts.service.ts | 26 ++- .../services/ruleEngine/ruleEngine.service.ts | 28 +++ .../backoffice/src/types/booking.ts | 18 +- packages/types/src/freight/contracts.ts | 44 +++- 55 files changed, 1210 insertions(+), 1373 deletions(-) create mode 100644 apps/edr-freight-api/src/migrations/2410000000000-DropBookingApprovalWidenRoles.ts create mode 100644 apps/edr-freight-api/src/migrations/2420000000000-CreateContractDocumentRevisions.ts delete mode 100644 apps/edr-freight-api/src/modules/bookings/entities/booking-approval-step.entity.ts create mode 100644 apps/edr-freight-api/src/modules/contracts/contract-document-diff.util.ts create mode 100644 apps/edr-freight-api/src/modules/contracts/contract-document-history.service.ts create mode 100644 apps/edr-freight-api/src/modules/contracts/entities/contract-document-revision.entity.ts delete mode 100644 apps/edr-freight-web/backoffice/src/components/bookings/ApprovalStepsCard.tsx delete mode 100644 apps/edr-freight-web/backoffice/src/components/bookings/BookingApprovalProgressCell.tsx delete mode 100644 apps/edr-freight-web/backoffice/src/components/bookings/detail/BookingApprovalCard.tsx create mode 100644 apps/edr-freight-web/backoffice/src/components/contracts/ContractPreviewModal.tsx create mode 100644 apps/edr-freight-web/backoffice/src/components/contracts/ContractRevisionTimeline.tsx delete mode 100644 apps/edr-freight-web/backoffice/src/features/bookings/approval-progress.ts diff --git a/apps/edr-freight-api/src/common/freight-permission.util.ts b/apps/edr-freight-api/src/common/freight-permission.util.ts index 69596c21d..ced6e3c46 100644 --- a/apps/edr-freight-api/src/common/freight-permission.util.ts +++ b/apps/edr-freight-api/src/common/freight-permission.util.ts @@ -7,16 +7,23 @@ const SUPER_ADMIN_ROLE = 'super_admin'; const ORGANIZATION_ADMIN_ROLE = 'organization_admin'; type PermissionLike = { key?: string }; +type PositionTypeLike = { key?: string }; type MeLikeUser = { roles?: { key?: string }[]; permissions?: PermissionLike[]; employee?: | { - position?: { permissions?: PermissionLike[] }; + position?: { + permissions?: PermissionLike[]; + positionType?: PositionTypeLike | null; + }; delegatedPositions?: { permissions?: PermissionLike[] }[]; } | { - positions?: { permissions?: PermissionLike[] }[]; + positions?: { + permissions?: PermissionLike[]; + positionType?: PositionTypeLike | null; + }[]; }[] | null; }; @@ -90,12 +97,110 @@ export function assertFreightPermission( throw new ForbiddenException(`Missing permission: ${permissionKey}`); } +/** + * The caller's IAM position-type keys (`iam.position_types.key`). A position + * type is the platform's notion of a role — it is what carries permissions via + * `iam.position_type_permissions` — and it is the vocabulary contract approval + * chains are configured in. + * + * Mirrors `collectPermissionKeys`' handling of both JWT shapes: `employee` is + * an object on some tokens and an array on others. + * + * Note delegated positions carry no `positionType` in the token, so a delegate + * is not reachable here — they authorize through the permission arm of + * `assertCanApproveContractStep` instead. + */ +export function collectPositionTypeKeys( + user: MeLikeUser | null | undefined, +): string[] { + const employee = user?.employee; + if (!employee) return []; + + const keys = new Set(); + + if (Array.isArray(employee)) { + for (const emp of employee) { + for (const pos of emp.positions ?? []) { + if (pos.positionType?.key) keys.add(pos.positionType.key); + } + } + return [...keys]; + } + + if (employee.position?.positionType?.key) { + keys.add(employee.position.positionType.key); + } + return [...keys]; +} + +/** + * Legacy chain roles predate position types. Historical `approval_rules` and + * in-flight `contract_approval_steps` rows still carry them, so map each to the + * position types that stand in for it. Without this, an approver holding a + * modern position type could not action an older step. + */ +const LEGACY_ROLE_POSITION_TYPES: Record = { + LINE_STAFF: ['employee', 'teamLeader', 'officeHead', 'recordOfficer'], + DIRECTOR: ['director', 'operation-director'], + CEO: ['chief', 'deputy'], +}; + const APPROVE_ROLE_PERMISSION: Record = { LINE_STAFF: FREIGHT_PERMS.bookings.approveLineStaff, DIRECTOR: FREIGHT_PERMS.bookings.approveDirector, CEO: FREIGHT_PERMS.bookings.approveCeo, }; +const CONTRACT_APPROVE_ROLE_PERMISSION: Record = { + LINE_STAFF: FREIGHT_PERMS.contracts.approveLineStaff, + DIRECTOR: FREIGHT_PERMS.contracts.approveDirector, + CEO: FREIGHT_PERMS.contracts.approveCeo, +}; + +const ANY_CONTRACT_APPROVE_PERMISSION = [ + FREIGHT_PERMS.contracts.approveLineStaff, + FREIGHT_PERMS.contracts.approveDirector, + FREIGHT_PERMS.contracts.approveCeo, +]; + +/** + * May this caller action a contract approval step requiring `requiredRole`? + * + * `requiredRole` is an `iam.position_types.key` for chains configured by an + * admin, or one of the legacy LINE_STAFF/DIRECTOR/CEO strings for older rows. + * A caller passes when any of these hold: + * + * - they are a super/organization admin (blanket bypass); + * - their position type matches the step, directly or via a legacy alias; + * - they hold the approve permission the legacy role maps to; + * - they hold any contract approve permission — this covers delegates (whose + * position type is absent from the token) and staff whose IAM position has + * no position type assigned yet. + */ +export function assertCanApproveContractStep( + user: TCurrentUser | MeLikeUser | null | undefined, + requiredRole: string, +): void { + if (isFreightApprovalAdmin(user)) return; + + const positionTypes = collectPositionTypeKeys(user); + if (positionTypes.includes(requiredRole)) return; + + const aliases = LEGACY_ROLE_POSITION_TYPES[requiredRole] ?? []; + if (aliases.some((alias) => positionTypes.includes(alias))) return; + + const legacyPermission = CONTRACT_APPROVE_ROLE_PERMISSION[requiredRole]; + if (legacyPermission && hasFreightPermission(user, legacyPermission)) return; + + if (ANY_CONTRACT_APPROVE_PERMISSION.some((p) => hasFreightPermission(user, p))) { + return; + } + + throw new ForbiddenException( + `You are not the required approver (${requiredRole}) for this step.`, + ); +} + export function assertCanApproveBookingStep( user: TCurrentUser | MeLikeUser | null | undefined, requiredRole: string, diff --git a/apps/edr-freight-api/src/migrations/2410000000000-DropBookingApprovalWidenRoles.ts b/apps/edr-freight-api/src/migrations/2410000000000-DropBookingApprovalWidenRoles.ts new file mode 100644 index 000000000..1b4674b05 --- /dev/null +++ b/apps/edr-freight-api/src/migrations/2410000000000-DropBookingApprovalWidenRoles.ts @@ -0,0 +1,43 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Bookings no longer run an approval chain — accepting an intake approves the + * booking outright and generates its contract. The approval chain is now a + * contract-only concern, so `freight.approval_rules` is read by contracts alone. + * + * Also widens the role columns: chain steps now reference IAM position-type + * keys (`iam.position_types.key`), and real keys run past the old varchar(30) + * (e.g. '-marketing-manager-/-general-manager' is 38 chars), which would fail + * on insert. + */ +export class DropBookingApprovalWidenRoles2410000000000 + implements MigrationInterface +{ + name = 'DropBookingApprovalWidenRoles2410000000000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `DROP TABLE IF EXISTS freight.booking_approval_step;`, + ); + + for (const [table, column] of [ + ['approval_rules', 'required_role'], + ['approval_rules', 'blocks_role'], + ['contract_approval_steps', 'required_role'], + ['contract_approval_steps', 'blocks_role'], + ] as const) { + await queryRunner.query( + `ALTER TABLE freight.${table} ALTER COLUMN ${column} TYPE varchar(64);`, + ); + } + } + + /** + * No-op: the booking approval chain is retired, so re-creating the table + * would leave dead schema behind. Narrowing the role columns again would + * truncate any position-type key already stored. + */ + public async down(): Promise { + // intentionally empty + } +} diff --git a/apps/edr-freight-api/src/migrations/2420000000000-CreateContractDocumentRevisions.ts b/apps/edr-freight-api/src/migrations/2420000000000-CreateContractDocumentRevisions.ts new file mode 100644 index 000000000..db06b2ba2 --- /dev/null +++ b/apps/edr-freight-api/src/migrations/2420000000000-CreateContractDocumentRevisions.ts @@ -0,0 +1,40 @@ +import { MigrationInterface, QueryRunner } from 'typeorm'; + +/** + * Audit trail for contract document edits. The document stays editable through + * the whole approval chain (each approver may edit on their turn), so the + * contract itself only ever holds the current snapshot — this table records who + * changed which article, and when. + */ +export class CreateContractDocumentRevisions2420000000000 + implements MigrationInterface +{ + name = 'CreateContractDocumentRevisions2420000000000'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query(` + CREATE TABLE IF NOT EXISTS freight.contract_document_revisions ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now(), + deleted_at timestamptz, + contract_id uuid NOT NULL REFERENCES freight.contracts(id) ON DELETE CASCADE, + actor_id uuid, + actor_role varchar(64), + step_id uuid, + summary varchar(255), + changes jsonb NOT NULL DEFAULT '[]'::jsonb + ); + `); + await queryRunner.query(` + CREATE INDEX IF NOT EXISTS idx_contract_document_revisions_contract + ON freight.contract_document_revisions (contract_id, created_at DESC); + `); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `DROP TABLE IF EXISTS freight.contract_document_revisions;`, + ); + } +} diff --git a/apps/edr-freight-api/src/modules/bookings/booking-next-step.util.ts b/apps/edr-freight-api/src/modules/bookings/booking-next-step.util.ts index d93b5b7bd..ddb4ce1e5 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-next-step.util.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-next-step.util.ts @@ -1,4 +1,3 @@ -import { BookingApprovalStep } from './entities/booking-approval-step.entity'; import { Booking } from './entities/booking.entity'; export interface BookingNextStep { @@ -9,7 +8,11 @@ export interface BookingNextStep { export function computeNextStep( booking: Pick, - nextPendingStep?: Pick | null, + /** + * Retained for call-site compatibility — bookings no longer run an approval + * chain, so this is always null. Approvals are a contract-only concern. + */ + nextPendingStep?: { requiredRole: string; stepOrder: number } | null, ): BookingNextStep | null { const { status } = booking; diff --git a/apps/edr-freight-api/src/modules/bookings/booking-transition.accept.spec.ts b/apps/edr-freight-api/src/modules/bookings/booking-transition.accept.spec.ts index 607a0d7a4..068ed53af 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-transition.accept.spec.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-transition.accept.spec.ts @@ -22,14 +22,17 @@ describe('BookingTransitionService — acceptIntake validity window', () => { findById: jest.fn().mockResolvedValue(booking), }; const ruleEngineService = { - instantiateApprovalSteps: jest.fn().mockResolvedValue([]), + assertNoHardBlocks: jest.fn(), + }; + const contractService = { + generateContract: jest.fn().mockResolvedValue({ id: 'b-1' }), }; const service = new BookingTransitionService( bookingsRepository as never, ruleEngineService as never, {} as never, // pricingService - {} as never, // contractService + contractService as never, {} as never, // filesService {} as never, // fileUploadSettingsService {} as never, // bookingBatchService @@ -57,7 +60,7 @@ describe('BookingTransitionService — acceptIntake validity window', () => { dutySlipUploadedToStaff: jest.fn(), } as never, // notifier ); - return { service, bookingsRepository, ruleEngineService }; + return { service, bookingsRepository, ruleEngineService, contractService }; } it('rejects accept when validity days is missing or non-positive', async () => { @@ -81,7 +84,7 @@ describe('BookingTransitionService — acceptIntake validity window', () => { const [id, updates] = bookingsRepository.update.mock.calls[0]; expect(id).toBe('b-1'); expect(updates).toMatchObject({ - status: 'PENDING_APPROVAL', + status: 'APPROVED', approvedByStaffId: 'staff-1', contractValidityDays: 10, }); @@ -96,12 +99,9 @@ describe('BookingTransitionService — acceptIntake validity window', () => { expect((updates.approvedByStaffAt as Date).getTime()).toBe(from.getTime()); }); - it('instantiates the approval chain when accepting', async () => { - const { service, ruleEngineService } = makeService(); + it('approves outright and generates the contract (no approval chain)', async () => { + const { service, contractService } = makeService(); await service.acceptIntake('b-1', 'staff-1', 30); - expect(ruleEngineService.instantiateApprovalSteps).toHaveBeenCalledWith( - 'b-1', - expect.objectContaining({ freightType: 'CONTAINER' }), - ); + expect(contractService.generateContract).toHaveBeenCalledWith('b-1'); }); }); diff --git a/apps/edr-freight-api/src/modules/bookings/booking-transition.service.ts b/apps/edr-freight-api/src/modules/bookings/booking-transition.service.ts index aacc68012..29fec7997 100644 --- a/apps/edr-freight-api/src/modules/bookings/booking-transition.service.ts +++ b/apps/edr-freight-api/src/modules/bookings/booking-transition.service.ts @@ -7,9 +7,7 @@ import { Logger, Optional, } from "@nestjs/common"; -import type { TCurrentUser } from "@tria-plc/api-common/modules/auth/types/current-user.type"; -import { assertCanApproveBookingStep } from '../../common/freight-permission.util'; import { BookingBatchService } from '../train-scheduling/booking-batch.service'; import { eatDay } from '../train-scheduling/batch-window.util'; import { isRoadService } from './road.util'; @@ -248,16 +246,6 @@ export class BookingTransitionService { return fresh; } - /** Auto-create booking approval steps from system rules when none exist yet. */ - private async ensureBookingApprovalSteps(booking: Booking): Promise { - if ((booking.approvalSteps?.length ?? 0) > 0) return; - - await this.ruleEngineService.instantiateApprovalSteps(booking.id, { - freightType: booking.freightType as "CONTAINER" | "BULK", - cargoTypeId: booking.cargoTypeId, - }); - } - async acceptIntake( bookingId: string, actorId: string, @@ -283,21 +271,33 @@ export class BookingTransitionService { const validUntil = new Date(validFrom); validUntil.setDate(validUntil.getDate() + validityDays); - await this.ruleEngineService.instantiateApprovalSteps(bookingId, { - freightType: booking.freightType as "CONTAINER" | "BULK", - cargoTypeId: booking.cargoTypeId, - }); - - const updated = await this.bookingsRepository.update(bookingId, { - status: "PENDING_APPROVAL", + // Bookings no longer run a multi-step approval chain — accepting the intake + // approves the booking outright and generates its contract. (The approval + // chain is a contract-only concern now; see contract-transition.service.) + await this.bookingsRepository.update(bookingId, { + status: "APPROVED", approvedByStaffId: actorId, approvedByStaffAt: validFrom, contractValidityDays: validityDays, contractValidFrom: validFrom, contractValidUntil: validUntil, } as never); - const fresh = await this.bookingsService.findById(updated!.id); + + // Generating the contract is best-effort: the acceptance is already + // committed, so a failure here must not roll it back. The booking stays + // APPROVED and staff can retry generation from the booking page. + try { + await this.contractService.generateContract(bookingId); + } catch (err) { + this.logger.warn( + `Contract generation failed after accepting booking ${bookingId}: ${err}. ` + + `The booking is APPROVED — retry generation from the booking page.`, + ); + } + + const fresh = await this.bookingsService.findById(bookingId); this.notifier.accepted(fresh); + this.notifier.approved(fresh); return fresh; } @@ -324,140 +324,6 @@ export class BookingTransitionService { return fresh; } - async approveStep( - bookingId: string, - stepId: string, - actorId: string, - requiredRole: string, - authUser?: TCurrentUser, - ): Promise { - if (authUser) { - assertCanApproveBookingStep(authUser, requiredRole); - } - - let booking = await this.bookingsService.findById(bookingId); - assertBookingStatus(booking, [ - "PENDING_APPROVAL", - "APPROVED_PENDING_SIGNATURE", - ]); - - if ((booking.approvalSteps?.length ?? 0) === 0) { - await this.ensureBookingApprovalSteps(booking); - booking = await this.bookingsService.findById(bookingId); - } - - const step = await this.bookingsRepository.findApprovalStepById( - bookingId, - stepId, - ); - if (!step || step.status !== "PENDING") { - throw new BadRequestException( - "Approval step not found or already actioned", - ); - } - - const next = - await this.bookingsRepository.findNextPendingApprovalStep(bookingId); - if (!next || next.id !== step.id) { - throw new BadRequestException( - "Approval steps must be completed in order", - ); - } - - if (step.requiredRole !== requiredRole) { - throw new BadRequestException( - `Step requires role ${step.requiredRole}, not ${requiredRole}`, - ); - } - - const blocksRole = step.blocksRole; - if (blocksRole && blocksRole === requiredRole) { - throw new BadRequestException( - `Role ${requiredRole} is blocked for this step`, - ); - } - - await this.bookingsRepository.completeApprovalStep( - step.id, - actorId, - "APPROVED", - ); - - const updates: Record = {}; - const now = new Date(); - - if (requiredRole === "LINE_STAFF") { - updates.status = "APPROVED_PENDING_SIGNATURE"; - updates.approvedByStaffId = actorId; - updates.approvedByStaffAt = now; - } else if (requiredRole === "DIRECTOR") { - updates.signedByDirectorId = actorId; - updates.signedByDirectorAt = now; - } else if (requiredRole === "CEO") { - updates.signedByCeoId = actorId; - updates.signedByCeoAt = now; - } - - const allDone = - await this.bookingsRepository.allApprovalStepsComplete(bookingId); - if (allDone) { - updates.status = "APPROVED"; - } - - if (Object.keys(updates).length > 0) { - await this.bookingsRepository.update(bookingId, updates as never); - } - - if (allDone) { - const generated = await this.contractService.generateContract(bookingId); - const fresh = await this.bookingsService.findById(generated.id); - this.notifier.approved(fresh); - return fresh; - } - - return this.bookingsService.findById(bookingId); - } - - async rejectStep( - bookingId: string, - stepId: string, - actorId: string, - reason: string, - ): Promise { - const booking = await this.bookingsService.findById(bookingId); - assertBookingStatus(booking, [ - "PENDING_APPROVAL", - "APPROVED_PENDING_SIGNATURE", - ]); - - const step = await this.bookingsRepository.findApprovalStepById( - bookingId, - stepId, - ); - if (!step) throw new BadRequestException("Approval step not found"); - - await this.bookingsRepository.completeApprovalStep( - step.id, - actorId, - "REJECTED", - reason, - ); - - await this.bookingsRepository.createReviewNote( - bookingId, - reason, - "REJECTION", - actorId, - ); - - const updated = await this.bookingsRepository.update(bookingId, { - status: "REJECTED", - } as never); - const fresh = await this.bookingsService.findById(updated!.id); - this.notifier.rejected(fresh, reason); - return fresh; - } - async customerSign(bookingId: string): Promise { const booking = await this.bookingsService.findById(bookingId); assertBookingStatus(booking, ["CONTRACT_READY"]); @@ -1298,12 +1164,9 @@ export class BookingTransitionService { } let nextStep: BookingNextStep | null = null; try { - const nextPending = - booking.status === "PENDING_APPROVAL" || - booking.status === "APPROVED_PENDING_SIGNATURE" - ? await this.bookingsRepository.findNextPendingApprovalStep(booking.id) - : null; - nextStep = computeNextStep(booking, nextPending); + // Bookings no longer carry an approval chain, so there is never a pending + // approval step to hint at. + nextStep = computeNextStep(booking, null); } catch (err) { this.logger.warn( `enrichBookingResponse: next-step lookup failed for ${booking.id}: ${(err as Error).message}`, diff --git a/apps/edr-freight-api/src/modules/bookings/bookings.controller.ts b/apps/edr-freight-api/src/modules/bookings/bookings.controller.ts index 56e788b93..bc8c80982 100644 --- a/apps/edr-freight-api/src/modules/bookings/bookings.controller.ts +++ b/apps/edr-freight-api/src/modules/bookings/bookings.controller.ts @@ -52,10 +52,8 @@ import { GeneratePriceResponseDto } from './dto/generate-price-response.dto'; import { SubmitBookingResponseDto } from './dto/submit-booking-response.dto'; import { AcceptIntakeDto, - ApproveStepDto, CancelBookingDto, RejectBookingDto, - RejectStepDto, RequestChangesDto, ReviewDocumentDto, RequestOperationDto, @@ -1023,47 +1021,6 @@ export class BookingsController { return this.transitionService.enrichBookingResponse(booking); } - @Post(":id/approval-steps/:stepId/approve") - @BookingStaff([ - FREIGHT_PERMS.bookings.approveLineStaff, - FREIGHT_PERMS.bookings.approveDirector, - FREIGHT_PERMS.bookings.approveCeo, - ]) - @ApiOperation({ summary: "Approve one approval step in sequence" }) - async approveStep( - @Param("id", ParseUUIDPipe) id: string, - @Param("stepId", ParseUUIDPipe) stepId: string, - @Body() dto: ApproveStepDto, - @CurrentUser() user: TCurrentUser, - ) { - const booking = await this.transitionService.approveStep( - id, - stepId, - resolveAuthUserId(user), - dto.requiredRole, - user, - ); - return this.transitionService.enrichBookingResponse(booking); - } - - @Post(":id/approval-steps/:stepId/reject") - @BookingStaff(FREIGHT_PERMS.bookings.rejectApproval) - @ApiOperation({ summary: "Reject at approval step" }) - async rejectStep( - @Param("id", ParseUUIDPipe) id: string, - @Param("stepId", ParseUUIDPipe) stepId: string, - @Body() dto: RejectStepDto, - @CurrentUser() user: AuthUserPayload, - ) { - const booking = await this.transitionService.rejectStep( - id, - stepId, - resolveAuthUserId(user), - dto.reason, - ); - return this.transitionService.enrichBookingResponse(booking); - } - @Post(":id/contract/generate") @BookingStaff(FREIGHT_PERMS.bookings.generateContract) @ApiOperation({ summary: "Generate contract PDF from template" }) diff --git a/apps/edr-freight-api/src/modules/bookings/bookings.module.ts b/apps/edr-freight-api/src/modules/bookings/bookings.module.ts index 15c5e751d..38d7d2ac6 100644 --- a/apps/edr-freight-api/src/modules/bookings/bookings.module.ts +++ b/apps/edr-freight-api/src/modules/bookings/bookings.module.ts @@ -30,7 +30,6 @@ import { BookingsRepository } from './bookings.repository'; import { ConsolidationService } from './consolidation.service'; import { ContainerValidationService } from './container-validation.service'; import { BookingsService } from './bookings.service'; -import { BookingApprovalStep } from './entities/booking-approval-step.entity'; import { BookingCargoModifier } from './entities/booking-cargo-modifier.entity'; import { BookingDocumentReview } from './entities/booking-document-review.entity'; import { BookingContainer } from './entities/booking-container.entity'; @@ -60,7 +59,6 @@ import { VehiclesModule } from "../vehicles/vehicles.module"; Booking, BookingContainer, BookingCargoModifier, - BookingApprovalStep, BookingDocumentReview, BookingRateSnapshot, BookingReviewNote, diff --git a/apps/edr-freight-api/src/modules/bookings/bookings.repository.ts b/apps/edr-freight-api/src/modules/bookings/bookings.repository.ts index c93ebd9c7..590bd7262 100644 --- a/apps/edr-freight-api/src/modules/bookings/bookings.repository.ts +++ b/apps/edr-freight-api/src/modules/bookings/bookings.repository.ts @@ -9,7 +9,6 @@ import { ContainerType } from '../rule-engine/entities/container-type.entity'; import { Contract } from '../contracts/entities/contract.entity'; import { ContractRateSnapshot } from '../contracts/entities/contract-rate-snapshot.entity'; import { ContractRoute } from '../contracts/entities/contract-route.entity'; -import { BookingApprovalStep } from './entities/booking-approval-step.entity'; import { BookingCargoModifier } from './entities/booking-cargo-modifier.entity'; import { BookingDocumentReview, @@ -114,7 +113,6 @@ export class BookingsRepository extends BaseRepository { .leftJoinAndSelect('booking.originYard', 'oy') .leftJoinAndSelect('booking.destinationYard', 'dy') .leftJoinAndSelect('booking.shippingLine', 'sl') - .leftJoinAndSelect('booking.approvalSteps', 'steps') .leftJoinAndSelect('booking.rateSnapshots', 'snapshots') .leftJoinAndSelect('booking.cargoModifiers', 'modifiers') .leftJoinAndSelect('booking.reviewNotes', 'reviewNotes') @@ -435,58 +433,6 @@ export class BookingsRepository extends BaseRepository { await this.dataSource.getRepository(BookingContainer).delete({ bookingId }); } - /** Lowest-order pending approval step (sequential enforcement). */ - async findNextPendingApprovalStep( - bookingId: string, - ): Promise { - return this.dataSource.getRepository(BookingApprovalStep).findOne({ - where: { bookingId, status: 'PENDING' }, - order: { stepOrder: 'ASC' }, - }); - } - - async findApprovalStepById( - bookingId: string, - stepId: string, - ): Promise { - return this.dataSource.getRepository(BookingApprovalStep).findOne({ - where: { bookingId, id: stepId }, - }); - } - - /** Get pending approval step for a role (must match next in sequence). */ - async findPendingApprovalStep( - bookingId: string, - requiredRole: string, - ): Promise { - const next = await this.findNextPendingApprovalStep(bookingId); - if (!next || next.requiredRole !== requiredRole) return null; - return next; - } - - /** Mark an approval step complete. */ - async completeApprovalStep( - stepId: string, - actorId: string, - status: 'APPROVED' | 'REJECTED', - remarks?: string, - ): Promise { - await this.dataSource.getRepository(BookingApprovalStep).update(stepId, { - status, - actionedByStaffId: actorId, - actionedAt: new Date(), - remarks, - }); - } - - /** Check if all approval steps are approved. */ - async allApprovalStepsComplete(bookingId: string): Promise { - const pending = await this.dataSource.getRepository(BookingApprovalStep).count({ - where: { bookingId, status: 'PENDING' }, - }); - return pending === 0; - } - // ── Clearance document reviews ──────────────────────────────────────────── findDocumentReviews(bookingId: string): Promise { @@ -673,7 +619,6 @@ export class BookingsRepository extends BaseRepository { .leftJoinAndSelect('booking.destinationYard', 'destinationYard') .leftJoinAndSelect('booking.cargoType', 'cargo') .leftJoinAndSelect('booking.serviceType', 'serviceType') - .leftJoinAndSelect('booking.approvalSteps', 'approvalSteps') .where('booking.status IN (:...statuses)', { statuses }); if (options.excludeBulk) { @@ -722,7 +667,6 @@ export class BookingsRepository extends BaseRepository { .leftJoinAndSelect('booking.originYard', 'originYard') .leftJoinAndSelect('booking.destinationYard', 'destinationYard') .leftJoinAndSelect('booking.serviceType', 'serviceType') - .leftJoinAndSelect('booking.approvalSteps', 'approvalSteps') .leftJoinAndSelect('booking.consolidationPartner', 'consolidationPartner') // Contract reference for the list column + search (no entity relation on // Booking → contract, so join the entity by id and select just the diff --git a/apps/edr-freight-api/src/modules/bookings/entities/booking-approval-step.entity.ts b/apps/edr-freight-api/src/modules/bookings/entities/booking-approval-step.entity.ts deleted file mode 100644 index 68018e883..000000000 --- a/apps/edr-freight-api/src/modules/bookings/entities/booking-approval-step.entity.ts +++ /dev/null @@ -1,48 +0,0 @@ -import { BaseEntity } from '@edr/api-common'; -import { Column, Entity, Index, JoinColumn, ManyToOne } from 'typeorm'; -import { ApprovalRule } from '../../rule-engine/entities/approval-rule.entity'; -import { Booking } from './booking.entity'; - -export const APPROVAL_STEP_STATUSES = ['PENDING', 'APPROVED', 'REJECTED', 'SKIPPED'] as const; -export type ApprovalStepStatus = typeof APPROVAL_STEP_STATUSES[number]; - -@Entity({ schema: 'freight', name: 'booking_approval_step' }) -@Index(['bookingId']) -@Index(['status']) -@Index(['bookingId', 'stepOrder']) -export class BookingApprovalStep extends BaseEntity { - @Column({ name: 'booking_id', type: 'uuid' }) - bookingId!: string; - - @ManyToOne(() => Booking, (b) => b.approvalSteps, { onDelete: 'CASCADE' }) - @JoinColumn({ name: 'booking_id' }) - booking?: Booking; - - @Column({ name: 'approval_rule_id', type: 'uuid' }) - approvalRuleId!: string; - - @ManyToOne(() => ApprovalRule) - @JoinColumn({ name: 'approval_rule_id' }) - approvalRule?: ApprovalRule; - - @Column({ name: 'step_order', type: 'smallint' }) - stepOrder!: number; - - @Column({ name: 'required_role', type: 'varchar', length: 30 }) - requiredRole!: string; - - @Column({ name: 'blocks_role', type: 'varchar', length: 30, nullable: true }) - blocksRole?: string | null; - - @Column({ name: 'status', type: 'varchar', length: 20, default: 'PENDING' }) - status!: ApprovalStepStatus; - - @Column({ name: 'actioned_by_staff_id', type: 'uuid', nullable: true }) - actionedByStaffId?: string | null; - - @Column({ name: 'actioned_at', type: 'timestamptz', nullable: true }) - actionedAt?: Date | null; - - @Column({ name: 'remarks', type: 'text', nullable: true }) - remarks?: string | null; -} diff --git a/apps/edr-freight-api/src/modules/bookings/entities/booking.entity.ts b/apps/edr-freight-api/src/modules/bookings/entities/booking.entity.ts index f74cb515e..2aa3ee8c2 100644 --- a/apps/edr-freight-api/src/modules/bookings/entities/booking.entity.ts +++ b/apps/edr-freight-api/src/modules/bookings/entities/booking.entity.ts @@ -10,7 +10,6 @@ import { ShippingLine } from '../../rule-engine/entities/shipping-line.entity'; import { Yard } from '../../rule-engine/entities/yard.entity'; import { Train } from '../../trains/entities/train.entity'; import { FileRecord } from '../../files/entities/file.entity'; -import { BookingApprovalStep } from './booking-approval-step.entity'; import { BookingCargoModifier } from './booking-cargo-modifier.entity'; import { BookingContainer } from './booking-container.entity'; import { BookingContainerAllocation } from './booking-container-allocation.entity'; @@ -557,8 +556,6 @@ export class Booking extends BaseEntity { @OneToMany(() => BookingCargoModifier, (m) => m.booking) cargoModifiers?: BookingCargoModifier[]; - @OneToMany(() => BookingApprovalStep, (s) => s.booking) - approvalSteps?: BookingApprovalStep[]; @OneToMany(() => BookingRateSnapshot, (s) => s.booking) rateSnapshots?: BookingRateSnapshot[]; diff --git a/apps/edr-freight-api/src/modules/contracts/contract-document-diff.util.ts b/apps/edr-freight-api/src/modules/contracts/contract-document-diff.util.ts new file mode 100644 index 000000000..154777aea --- /dev/null +++ b/apps/edr-freight-api/src/modules/contracts/contract-document-diff.util.ts @@ -0,0 +1,168 @@ +import type { + ContractDocumentArticle, + ContractDocumentSnapshot, +} from './entities/contract.entity'; + +/** + * One recorded change between two document snapshots. Granularity is per + * article: a body edit is reported as "the body changed", not as a text diff. + */ +export type ContractDocumentChange = + | { kind: 'ARTICLE_ADDED'; articleId: string; title: string } + | { kind: 'ARTICLE_REMOVED'; articleId: string; title: string } + | { + kind: 'ARTICLE_RENAMED'; + articleId: string; + title: string; + fromTitle: string; + } + | { kind: 'ARTICLE_BODY_CHANGED'; articleId: string; title: string } + | { + kind: 'ARTICLE_REORDERED'; + articleId: string; + title: string; + fromOrder: number; + toOrder: number; + } + | { kind: 'DOCUMENT_TITLE_CHANGED'; title: string; fromTitle: string | null } + | { kind: 'WHEREAS_CHANGED'; added: number; removed: number }; + +type SnapshotLike = Pick< + ContractDocumentSnapshot, + 'documentTitle' | 'whereasClauses' | 'articles' +> | null; + +/** Match on id when present, else on normalized title (editors may omit ids). */ +function articleKey(article: ContractDocumentArticle): string { + return article.id || `title:${article.title.trim().toLowerCase()}`; +} + +function indexArticles( + articles: ContractDocumentArticle[] | undefined, +): Map { + const map = new Map(); + for (const article of articles ?? []) { + map.set(articleKey(article), article); + } + return map; +} + +/** + * Compare two document snapshots and describe what changed, article by article. + * Returns an empty array when the snapshots are equivalent, so callers can skip + * recording a no-op revision. + */ +export function diffSnapshots( + before: SnapshotLike, + after: SnapshotLike, +): ContractDocumentChange[] { + const changes: ContractDocumentChange[] = []; + + const beforeTitle = before?.documentTitle ?? null; + const afterTitle = after?.documentTitle ?? null; + if (beforeTitle !== afterTitle && afterTitle !== null) { + changes.push({ + kind: 'DOCUMENT_TITLE_CHANGED', + title: afterTitle, + fromTitle: beforeTitle, + }); + } + + const beforeWhereas = before?.whereasClauses ?? []; + const afterWhereas = after?.whereasClauses ?? []; + const beforeWhereasSet = new Set(beforeWhereas); + const afterWhereasSet = new Set(afterWhereas); + const whereasAdded = afterWhereas.filter((c) => !beforeWhereasSet.has(c)).length; + const whereasRemoved = beforeWhereas.filter((c) => !afterWhereasSet.has(c)).length; + if (whereasAdded > 0 || whereasRemoved > 0) { + changes.push({ + kind: 'WHEREAS_CHANGED', + added: whereasAdded, + removed: whereasRemoved, + }); + } + + const beforeArticles = indexArticles(before?.articles); + const afterArticles = indexArticles(after?.articles); + + for (const [key, article] of afterArticles) { + const previous = beforeArticles.get(key); + if (!previous) { + changes.push({ + kind: 'ARTICLE_ADDED', + articleId: article.id, + title: article.title, + }); + continue; + } + + if (previous.title !== article.title) { + changes.push({ + kind: 'ARTICLE_RENAMED', + articleId: article.id, + title: article.title, + fromTitle: previous.title, + }); + } + if (previous.body !== article.body) { + changes.push({ + kind: 'ARTICLE_BODY_CHANGED', + articleId: article.id, + title: article.title, + }); + } + if (previous.order !== article.order) { + changes.push({ + kind: 'ARTICLE_REORDERED', + articleId: article.id, + title: article.title, + fromOrder: previous.order, + toOrder: article.order, + }); + } + } + + for (const [key, article] of beforeArticles) { + if (afterArticles.has(key)) continue; + changes.push({ + kind: 'ARTICLE_REMOVED', + articleId: article.id, + title: article.title, + }); + } + + return changes; +} + +/** Short human summary of a change set, e.g. "2 articles edited, 1 article added". */ +export function summarizeChanges(changes: ContractDocumentChange[]): string { + if (changes.length === 0) return 'No changes'; + + const articleVerbs: Record = { + ARTICLE_ADDED: 'added', + ARTICLE_REMOVED: 'removed', + ARTICLE_RENAMED: 'renamed', + ARTICLE_BODY_CHANGED: 'edited', + ARTICLE_REORDERED: 'reordered', + }; + + const counts = new Map(); + const parts: string[] = []; + + for (const change of changes) { + const verb = articleVerbs[change.kind]; + if (verb) { + counts.set(verb, (counts.get(verb) ?? 0) + 1); + } else if (change.kind === 'DOCUMENT_TITLE_CHANGED') { + parts.push('document title changed'); + } else if (change.kind === 'WHEREAS_CHANGED') { + parts.push('recitals changed'); + } + } + + const articleParts = [...counts.entries()].map( + ([verb, count]) => `${count} article${count === 1 ? '' : 's'} ${verb}`, + ); + + return [...articleParts, ...parts].join(', '); +} diff --git a/apps/edr-freight-api/src/modules/contracts/contract-document-history.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-document-history.service.ts new file mode 100644 index 000000000..2808ea6cf --- /dev/null +++ b/apps/edr-freight-api/src/modules/contracts/contract-document-history.service.ts @@ -0,0 +1,61 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { InjectRepository } from '@nestjs/typeorm'; +import { Repository } from 'typeorm'; + +import { diffSnapshots, summarizeChanges } from './contract-document-diff.util'; +import { ContractDocumentRevision } from './entities/contract-document-revision.entity'; +import type { ContractDocumentSnapshot } from './entities/contract.entity'; + +export interface RecordRevisionInput { + contractId: string; + before: ContractDocumentSnapshot | null; + after: ContractDocumentSnapshot | null; + actorId?: string | null; + actorRole?: string | null; + stepId?: string | null; +} + +@Injectable() +export class ContractDocumentHistoryService { + private readonly logger = new Logger(ContractDocumentHistoryService.name); + + constructor( + @InjectRepository(ContractDocumentRevision) + private readonly revisionRepo: Repository, + ) {} + + /** + * Append a revision describing what an edit changed. Best-effort: recording + * history must never break the edit that triggered it, so failures are logged + * and swallowed. A no-op edit records nothing. + */ + async record(input: RecordRevisionInput): Promise { + try { + const changes = diffSnapshots(input.before, input.after); + if (changes.length === 0) return; + + await this.revisionRepo.save( + this.revisionRepo.create({ + contractId: input.contractId, + actorId: input.actorId ?? null, + actorRole: input.actorRole ?? null, + stepId: input.stepId ?? null, + summary: summarizeChanges(changes), + changes, + }), + ); + } catch (err) { + this.logger.error( + `Failed to record document revision for contract ${input.contractId}: ${String(err)}`, + ); + } + } + + /** Revision history for a contract, newest first. */ + list(contractId: string): Promise { + return this.revisionRepo.find({ + where: { contractId }, + order: { createdAt: 'DESC' }, + }); + } +} diff --git a/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts b/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts index bba044c85..2fd1e4cb4 100644 --- a/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts +++ b/apps/edr-freight-api/src/modules/contracts/contract-transition.service.ts @@ -3,6 +3,7 @@ import { ConflictException, Injectable, Logger, + ServiceUnavailableException, } from '@nestjs/common'; import { InjectDataSource } from '@nestjs/typeorm'; import { DataSource } from 'typeorm'; @@ -17,7 +18,8 @@ import { ContractPdfService } from '../../contracts/contract-pdf.service'; import { ContractViewModel } from '../../contracts/contract-view-model.builder'; import { MinioService } from '../minio/minio.service'; import { FileRecord } from '../files/entities/file.entity'; -import { assertCanApproveBookingStep } from '../../common/freight-permission.util'; +import { assertCanApproveContractStep } from '../../common/freight-permission.util'; +import { ContractDocumentHistoryService } from './contract-document-history.service'; import { ApprovalRulesService } from '../rule-engine/services/approval-rules.service'; import { CargoTypesService } from '../rule-engine/services/cargo-types.service'; import { DropdownSettingsService } from '../dropdown-settings/dropdown-settings.service'; @@ -48,8 +50,12 @@ export interface ContractDocumentDraft { articles: ContractDocumentArticle[]; code: string | null; name: string | null; - /** True once the document may no longer be edited/regenerated. */ + /** True when THIS caller may not edit — the inverse of `editableByMe`. */ locked: boolean; + /** Whether the requesting user is the approver whose turn it is to edit. */ + editableByMe: boolean; + /** Role holding editing rights right now, for "locked because…" messaging. */ + nextApproverRole: string | null; generatedAt: Date | null; status: string; } @@ -62,6 +68,28 @@ export interface ContractDocumentDraft { */ const CONTRACT_VALIDITY_PERIODS_CODE = 'contract_validity_periods'; +/** + * Approval chains are configured in IAM position types, so a step's role no + * longer maps onto the contract's fixed approver columns. These sets keep those + * legacy columns populated for the roles that still correspond to one — both the + * original role strings on historical rows and the position types that replaced + * them. Steps outside these sets are recorded only in `contract_approval_steps`, + * which is the source of truth. + */ +const LEGACY_STAFF_ROLES = new Set([ + 'LINE_STAFF', + 'employee', + 'teamLeader', + 'officeHead', + 'recordOfficer', +]); +const LEGACY_DIRECTOR_ROLES = new Set([ + 'DIRECTOR', + 'director', + 'operation-director', +]); +const LEGACY_CEO_ROLES = new Set(['CEO', 'chief', 'deputy']); + /** * Mask a phone for display — keep the last 4 digits, star the rest * (`+251986680099` → `•••••••0099`). Used to tell the customer WHERE the signing @@ -87,6 +115,7 @@ export class ContractTransitionService { private readonly logger = new Logger(ContractTransitionService.name); constructor( + private readonly documentHistory: ContractDocumentHistoryService, private readonly contractsRepository: ContractsRepository, private readonly contractsService: ContractsService, private readonly pricingService: ContractPricingService, @@ -229,18 +258,22 @@ export class ContractTransitionService { */ async getContractDocumentDraft( contractId: string, + user?: TCurrentUser | null, ): Promise { const contract = await this.contractsService.findById(contractId); const snapshot = (contract.documentSnapshot as ContractDocumentSnapshot | null) ?? (await this.resolveDocumentSnapshot(contract)); + const editableByMe = await this.documentIsEditableBy(contract, user); return { documentTitle: snapshot?.documentTitle ?? null, whereasClauses: snapshot?.whereasClauses ?? [], articles: snapshot?.articles ?? [], code: snapshot?.code ?? null, name: snapshot?.name ?? null, - locked: !this.documentIsEditable(contract), + locked: !editableByMe, + editableByMe, + nextApproverRole: await this.nextApproverRole(contract), generatedAt: contract.contractGeneratedAt ?? null, status: contract.status, }; @@ -255,10 +288,12 @@ export class ContractTransitionService { async updateContractDocument( contractId: string, input: ContractDocumentSnapshotInput, + user?: TCurrentUser | null, + actorId?: string, ): Promise { const contract = await this.contractsService.findById(contractId); assertContractStatus(contract, ['PENDING_APPROVAL']); - this.assertDocumentEditable(contract); + await this.assertDocumentEditable(contract, user); const current = (contract.documentSnapshot as ContractDocumentSnapshot | null) ?? @@ -270,9 +305,25 @@ export class ContractTransitionService { whereasClauses: input.whereasClauses ?? current?.whereasClauses ?? [], articles: input.articles ?? current?.articles ?? [], }; + const next = this.normalizeSnapshot(merged); await this.contractsRepository.update(contractId, { - documentSnapshot: this.normalizeSnapshot(merged), + documentSnapshot: next, } as never); + + // Audit the edit after it lands. Recording history must never break the + // edit itself, so the history service swallows its own failures. + const step = await this.contractsRepository.findNextPendingApprovalStep( + contractId, + ); + await this.documentHistory.record({ + contractId, + before: current, + after: next, + actorId: actorId ?? null, + actorRole: step?.requiredRole ?? null, + stepId: step?.id ?? null, + }); + return this.contractsService.findById(contractId); } @@ -334,23 +385,54 @@ export class ContractTransitionService { } /** - * The per-contract document may be edited/regenerated while the contract is at - * the accept stage (SUBMITTED) or in approval with NO approver having acted - * yet. The first approval action freezes it. + * The contract document stays editable for the whole approval chain, but only + * by the approver whose turn it is: whoever can action the next pending step. + * Approving therefore hands editing rights to the next approver in the chain. + * + * Edits never reset approvals already given — earlier approvers stay approved. */ - private documentIsEditable(contract: Contract): boolean { + private async documentIsEditableBy( + contract: Contract, + user?: TCurrentUser | null, + ): Promise { if (contract.status === 'SUBMITTED') return true; if (contract.status !== 'PENDING_APPROVAL') return false; - return !(contract.approvalSteps ?? []).some((s) => s.status !== 'PENDING'); + + const next = await this.contractsRepository.findNextPendingApprovalStep( + contract.id, + ); + if (!next) return false; + if (!user) return false; + + try { + assertCanApproveContractStep(user, next.requiredRole); + return true; + } catch { + return false; + } } - private assertDocumentEditable(contract: Contract): void { - if (!this.documentIsEditable(contract)) { - throw new ConflictException( - 'The contract document is locked — an approver has already acted or the ' + - 'contract has advanced. It can no longer be edited or regenerated.', - ); - } + /** The role that currently holds editing rights, for UI messaging. */ + private async nextApproverRole(contract: Contract): Promise { + if (contract.status !== 'PENDING_APPROVAL') return null; + const next = await this.contractsRepository.findNextPendingApprovalStep( + contract.id, + ); + return next?.requiredRole ?? null; + } + + private async assertDocumentEditable( + contract: Contract, + user?: TCurrentUser | null, + ): Promise { + if (await this.documentIsEditableBy(contract, user)) return; + + const role = await this.nextApproverRole(contract); + throw new ConflictException( + role + ? `The contract document can only be edited by the current approver (${role}).` + : 'The contract document is locked — the contract has advanced beyond approval.', + ); } /** @@ -513,25 +595,11 @@ export class ContractTransitionService { contractId: string, stepId: string, actorId: string, - requiredRole: string, authUser?: TCurrentUser, ): Promise { - if (authUser) { - assertCanApproveBookingStep(authUser, requiredRole); - } - const contract = await this.contractsService.findById(contractId); assertContractStatus(contract, ['PENDING_APPROVAL', 'APPROVED_PENDING_SIGNATURE']); - // Approvers review the generated contract document, so it must exist before - // the first approval can be recorded. Staff generate it (from the frozen, - // optionally-edited snapshot) at the accept stage. - if (contract.status === 'PENDING_APPROVAL' && !contract.contractGeneratedAt) { - throw new BadRequestException( - 'Generate the contract document before it can be approved.', - ); - } - const step = await this.contractsRepository.findApprovalStepById(contractId, stepId); if (!step || step.status !== 'PENDING') { throw new BadRequestException('Approval step not found or already actioned'); @@ -541,31 +609,34 @@ export class ContractTransitionService { if (!next || next.id !== step.id) { throw new BadRequestException('Approval steps must be completed in order'); } - if (step.requiredRole !== requiredRole) { - throw new BadRequestException( - `Step requires role ${step.requiredRole}, not ${requiredRole}`, - ); - } - if (step.blocksRole && step.blocksRole === requiredRole) { - throw new BadRequestException(`Role ${requiredRole} is blocked for this step`); + + // The role is the step's own — never the caller's claim about themselves. + const requiredRole = step.requiredRole; + if (authUser) { + assertCanApproveContractStep(authUser, requiredRole); } await this.contractsRepository.completeApprovalStep(step.id, actorId, 'APPROVED'); // Record who acted on this step, but DO NOT advance the contract status here — - // approving one step (e.g. LINE_STAFF) must not finalize the chain while later - // steps (e.g. DIRECTOR) are still pending. Status only moves to APPROVED once - // every step in the chain is complete; until then the contract stays in - // PENDING_APPROVAL so the next required role can act. + // approving one step must not finalize the chain while later steps are still + // pending. Status only moves to APPROVED once every step in the chain is + // complete; until then the contract stays in PENDING_APPROVAL so the next + // required approver can act. + // + // `contract_approval_steps` is the source of truth for who approved what — a + // chain is an arbitrary sequence of position types and cannot be represented + // by fixed columns. The legacy columns below are still stamped, best-effort, + // for the three roles that map onto them so older readers keep working. const updates: Record = {}; const now = new Date(); - if (requiredRole === 'LINE_STAFF') { + if (LEGACY_STAFF_ROLES.has(requiredRole)) { updates.approvedByStaffId = actorId; updates.approvedByStaffAt = now; - } else if (requiredRole === 'DIRECTOR') { + } else if (LEGACY_DIRECTOR_ROLES.has(requiredRole)) { updates.signedByDirectorId = actorId; updates.signedByDirectorAt = now; - } else if (requiredRole === 'CEO') { + } else if (LEGACY_CEO_ROLES.has(requiredRole)) { updates.signedByCeoId = actorId; updates.signedByCeoAt = now; } @@ -579,14 +650,19 @@ export class ContractTransitionService { const updated = await this.contractsService.findById(contractId); if (allDone) { this.notifier.approved(updated); - // Every step approved → CONTRACT_READY. The document was already generated - // (and reviewed) at the accept stage, so we reuse it rather than - // re-rendering. Best-effort: a hiccup must not roll back the approval. + // Final approval is what produces the contract PDF — until now there was + // only a live preview. The approval steps are already committed, so a + // render failure must not roll them back; surface it instead of swallowing + // it, since an APPROVED contract with no document needs operator action. try { return await this.finalizeApprovedContract(contractId); } catch (err) { - this.logger.warn( - `Finalizing contract after final approval failed for ${updated.reference}: ${err}`, + this.logger.error( + `Contract PDF generation failed after final approval for ${updated.reference}: ${err}`, + ); + throw new ServiceUnavailableException( + 'All approvals were recorded, but generating the contract PDF failed. ' + + 'Retry generation from the contract page.', ); } } @@ -594,24 +670,13 @@ export class ContractTransitionService { } /** - * Staff (re)generate the contract PDF. Two stages: - * - PENDING_APPROVAL: render from the frozen (optionally staff-edited) - * snapshot so approvers review the real document. Status is UNCHANGED, and - * it is blocked once an approver has acted (the document is then locked). - * - APPROVED / APPROVED_PENDING_SIGNATURE (fallback): render and advance to - * CONTRACT_READY. - * PDF rendering (Puppeteer/Chromium) is best-effort and never blocks the - * transition — the document re-renders lazily on view/download. + * Retry path for a contract that finished approval but whose PDF failed to + * render (Chromium unavailable, etc.). The normal flow generates the document + * automatically on the final approval — there is no manual generate step + * before that, only the live preview. */ async generateContract(contractId: string): Promise { const contract = await this.contractsService.findById(contractId); - - if (contract.status === 'PENDING_APPROVAL') { - this.assertDocumentEditable(contract); - await this.renderContractDocument(contract); - return this.contractsService.findById(contractId); - } - assertContractStatus(contract, ['APPROVED', 'APPROVED_PENDING_SIGNATURE']); await this.renderContractDocument(contract); await this.contractsRepository.update(contractId, { @@ -626,11 +691,17 @@ export class ContractTransitionService { * changes status. Rendering is best-effort — a Chromium hiccup defers the file * (it re-renders on view/download) but the timestamp is still stamped. */ - private async renderContractDocument(contract: Contract): Promise { + private async renderContractDocument( + contract: Contract, + options: { strict?: boolean } = {}, + ): Promise { const { view } = await this.documentViewModelBuilder.build(contract.id); try { await this.upsertContractPdf(contract.id, contract.reference, view); } catch (err) { + // Strict callers (final approval) need to know the PDF is missing — it is + // the artifact of the completed chain, not a cache that can refill later. + if (options.strict) throw err; this.logger.warn( `Contract PDF deferred for ${contract.reference}: ${err}. It will render on view/download once Chromium is available.`, ); @@ -642,15 +713,14 @@ export class ContractTransitionService { } /** - * Every approval step landed → CONTRACT_READY. The document was already - * generated (and reviewed) at the accept stage, so reuse it; render now only - * if it was somehow never generated. Never re-renders over an existing file. + * Every approval step landed → generate the contract PDF, then CONTRACT_READY. + * This is the only point at which the document is produced: approvers review a + * live preview, and the final approval is what turns it into a PDF. Renders + * unconditionally so the file reflects every edit made during the chain. */ private async finalizeApprovedContract(contractId: string): Promise { const contract = await this.contractsService.findById(contractId); - if (!contract.contractGeneratedAt) { - await this.renderContractDocument(contract); - } + await this.renderContractDocument(contract, { strict: true }); await this.contractsRepository.update(contractId, { status: 'CONTRACT_READY', } as never); diff --git a/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts b/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts index b62b89e32..ca4814081 100644 --- a/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts +++ b/apps/edr-freight-api/src/modules/contracts/contracts.controller.ts @@ -32,6 +32,7 @@ import { } from '@nestjs/swagger'; import { BookingStaff } from '../../common/booking-guards'; +import { ContractDocumentHistoryService } from './contract-document-history.service'; import { FREIGHT_PERMS } from '../../seed/freight-permissions.registry'; import { assertFreightPermission, @@ -60,7 +61,6 @@ import { ContractListSummaryDto } from './dto/contract-list-summary.dto'; import { AcceptContractDto } from './dto/accept-contract.dto'; import { UpdateContractDocumentDto } from './dto/contract-document.dto'; import { - ApproveStepDto, RejectContractDto, RejectStepDto, RequestChangesDto, @@ -90,6 +90,7 @@ import { @ApiBearerAuth() export class ContractsController { constructor( + private readonly documentHistory: ContractDocumentHistoryService, private readonly contractsService: ContractsService, private readonly pricingService: ContractPricingService, private readonly transitionService: ContractTransitionService, @@ -352,8 +353,22 @@ export class ContractsController { summary: 'Editable contract-document draft (this contract\'s snapshot, or the live template) for the accept/edit dialog', }) - getContractDocumentDraft(@Param('id', ParseUUIDPipe) id: string) { - return this.transitionService.getContractDocumentDraft(id); + getContractDocumentDraft( + @Param('id', ParseUUIDPipe) id: string, + @CurrentUser() user: TCurrentUser, + ) { + // Editability depends on WHO is asking — only the approver whose turn it is + // may edit — so the caller is part of the draft lookup. + return this.transitionService.getContractDocumentDraft(id, user); + } + + @Get(':id/document/revisions') + @BookingStaff(FREIGHT_PERMS.contracts.view) + @ApiOperation({ + summary: 'Audit trail of edits to this contract\'s document (newest first)', + }) + getContractDocumentRevisions(@Param('id', ParseUUIDPipe) id: string) { + return this.documentHistory.list(id); } @Put(':id/document/articles') @@ -365,8 +380,14 @@ export class ContractsController { updateContractDocument( @Param('id', ParseUUIDPipe) id: string, @Body() dto: UpdateContractDocumentDto, + @CurrentUser() user: TCurrentUser, ) { - return this.transitionService.updateContractDocument(id, dto); + return this.transitionService.updateContractDocument( + id, + dto, + user, + resolveAuthUserId(user), + ); } @Post(':id/staff/request-changes') @@ -396,23 +417,20 @@ export class ContractsController { } @Post(':id/approval-steps/:stepId/approve') - @BookingStaff([ - FREIGHT_PERMS.contracts.approveLineStaff, - FREIGHT_PERMS.contracts.approveDirector, - FREIGHT_PERMS.contracts.approveCeo, - ]) + @BookingStaff(FREIGHT_PERMS.contracts.view) @ApiOperation({ summary: 'Approve one approval step in sequence' }) approveStep( @Param('id', ParseUUIDPipe) id: string, @Param('stepId', ParseUUIDPipe) stepId: string, - @Body() dto: ApproveStepDto, @CurrentUser() user: TCurrentUser, ) { + // Whether this caller may approve depends on the step's own required role + // (an IAM position type), so the service resolves the step and authorizes + // against it — the client never declares its own role. return this.transitionService.approveStep( id, stepId, resolveAuthUserId(user), - dto.requiredRole, user, ); } diff --git a/apps/edr-freight-api/src/modules/contracts/contracts.module.ts b/apps/edr-freight-api/src/modules/contracts/contracts.module.ts index 5177b6a39..050417a45 100644 --- a/apps/edr-freight-api/src/modules/contracts/contracts.module.ts +++ b/apps/edr-freight-api/src/modules/contracts/contracts.module.ts @@ -41,6 +41,8 @@ import { ContractRateSnapshot } from './entities/contract-rate-snapshot.entity'; import { ContractSignature } from './entities/contract-signature.entity'; import { ContractApprovalStep } from './entities/contract-approval-step.entity'; import { ContractReviewNote } from './entities/contract-review-note.entity'; +import { ContractDocumentRevision } from './entities/contract-document-revision.entity'; +import { ContractDocumentHistoryService } from './contract-document-history.service'; import { ContractClearanceCycle } from './entities/contract-clearance-cycle.entity'; import { ContractDocumentReview } from './entities/contract-document-review.entity'; import { ClearanceMilestone } from './entities/clearance-milestone.entity'; @@ -64,6 +66,7 @@ import { ContractDocumentViewModelBuilder } from '../../contracts/contract-docum ContractSignature, ContractApprovalStep, ContractReviewNote, + ContractDocumentRevision, ContractClearanceCycle, ContractDocumentReview, ClearanceMilestone, @@ -107,6 +110,7 @@ import { ContractDocumentViewModelBuilder } from '../../contracts/contract-docum ClearanceFeeService, ContractNotifierService, ContractTransitionService, + ContractDocumentHistoryService, ContractClearanceService, ClearanceWorkflowService, BookingClearanceService, diff --git a/apps/edr-freight-api/src/modules/contracts/entities/contract-approval-step.entity.ts b/apps/edr-freight-api/src/modules/contracts/entities/contract-approval-step.entity.ts index 3c8c7fd5f..0a47dd3e1 100644 --- a/apps/edr-freight-api/src/modules/contracts/entities/contract-approval-step.entity.ts +++ b/apps/edr-freight-api/src/modules/contracts/entities/contract-approval-step.entity.ts @@ -26,10 +26,10 @@ export class ContractApprovalStep extends BaseEntity { @Column({ name: 'step_order', type: 'smallint', default: 0 }) stepOrder!: number; - @Column({ name: 'required_role', type: 'varchar', length: 40 }) + @Column({ name: 'required_role', type: 'varchar', length: 64 }) requiredRole!: string; - @Column({ name: 'blocks_role', type: 'varchar', length: 40, nullable: true }) + @Column({ name: 'blocks_role', type: 'varchar', length: 64, nullable: true }) blocksRole?: string | null; @Column({ name: 'status', type: 'varchar', length: 20, default: 'PENDING' }) diff --git a/apps/edr-freight-api/src/modules/contracts/entities/contract-document-revision.entity.ts b/apps/edr-freight-api/src/modules/contracts/entities/contract-document-revision.entity.ts new file mode 100644 index 000000000..bc7e12e3e --- /dev/null +++ b/apps/edr-freight-api/src/modules/contracts/entities/contract-document-revision.entity.ts @@ -0,0 +1,36 @@ +import { BaseEntity } from '@edr/api-common'; +import { Column, Entity, Index, JoinColumn, ManyToOne } from 'typeorm'; +import type { ContractDocumentChange } from '../contract-document-diff.util'; +import { Contract } from './contract.entity'; + +/** + * Append-only audit of contract document edits. The document stays editable + * through the whole approval chain, so this records who changed which article + * and when — the contract itself only ever holds the current snapshot. + */ +@Entity({ schema: 'freight', name: 'contract_document_revisions' }) +@Index(['contractId']) +export class ContractDocumentRevision extends BaseEntity { + @Column({ name: 'contract_id', type: 'uuid' }) + contractId!: string; + + @ManyToOne(() => Contract, { onDelete: 'CASCADE' }) + @JoinColumn({ name: 'contract_id' }) + contract?: Contract; + + @Column({ name: 'actor_id', type: 'uuid', nullable: true }) + actorId?: string | null; + + /** The approval step's required role at the time of the edit. */ + @Column({ name: 'actor_role', type: 'varchar', length: 64, nullable: true }) + actorRole?: string | null; + + @Column({ name: 'step_id', type: 'uuid', nullable: true }) + stepId?: string | null; + + @Column({ name: 'summary', type: 'varchar', length: 255, nullable: true }) + summary?: string | null; + + @Column({ name: 'changes', type: 'jsonb', default: () => `'[]'::jsonb` }) + changes!: ContractDocumentChange[]; +} diff --git a/apps/edr-freight-api/src/modules/rule-engine/controllers/approval-rules.controller.ts b/apps/edr-freight-api/src/modules/rule-engine/controllers/approval-rules.controller.ts index 8d9ed00c9..5d17efe1e 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/controllers/approval-rules.controller.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/controllers/approval-rules.controller.ts @@ -31,6 +31,15 @@ export class ApprovalRulesController { return this.service.findChain(flag === 'true'); } + @Get('position-types') + @RuleEngineView('approval-rules') + @ApiOperation({ + summary: 'IAM position types to choose from when building an approval chain', + }) + listPositionTypes() { + return this.service.listPositionTypes(); + } + @Post('reorder') @RuleEngineManage('approval-rules') @HttpCode(HttpStatus.NO_CONTENT) diff --git a/apps/edr-freight-api/src/modules/rule-engine/dto/create-approval-rule.dto.ts b/apps/edr-freight-api/src/modules/rule-engine/dto/create-approval-rule.dto.ts index 5861b1ad8..196242802 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/dto/create-approval-rule.dto.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/dto/create-approval-rule.dto.ts @@ -1,8 +1,6 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { IsBoolean, IsInt, IsOptional, IsString, IsUUID, MaxLength, Min } from 'class-validator'; -const ROLES = ['LINE_STAFF', 'DIRECTOR', 'CEO'] as const; - export class CreateApprovalRuleDto { @ApiProperty({ description: 'True = Director+CEO chain; False = LineStaff+Director chain' }) @IsBoolean() @@ -19,9 +17,12 @@ export class CreateApprovalRuleDto { @IsUUID('4') insertAfterId?: string; - @ApiProperty({ enum: ROLES, description: 'Role required to action this step' }) + @ApiProperty({ + description: + 'IAM position-type key required to action this step (see GET /approval-rules/position-types)', + }) @IsString() - @MaxLength(30) + @MaxLength(64) requiredRole!: string; @ApiProperty({ description: 'Label shown in UI, e.g. "Review & Approve"', maxLength: 50 }) @@ -29,9 +30,11 @@ export class CreateApprovalRuleDto { @MaxLength(50) actionLabel!: string; - @ApiPropertyOptional({ enum: ROLES, description: 'Role explicitly blocked from actioning this step' }) + @ApiPropertyOptional({ + description: 'IAM position-type key explicitly blocked from actioning this step', + }) @IsOptional() @IsString() - @MaxLength(30) + @MaxLength(64) blocksRole?: string; } diff --git a/apps/edr-freight-api/src/modules/rule-engine/entities/approval-rule.entity.ts b/apps/edr-freight-api/src/modules/rule-engine/entities/approval-rule.entity.ts index 94fb4355d..63448c5f3 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/entities/approval-rule.entity.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/entities/approval-rule.entity.ts @@ -12,12 +12,12 @@ export class ApprovalRule extends BaseEntity { @Column({ name: 'step_order', type: 'smallint' }) stepOrder!: number; - @Column({ name: 'required_role', type: 'varchar', length: 30 }) + @Column({ name: 'required_role', type: 'varchar', length: 64 }) requiredRole!: string; @Column({ name: 'action_label', type: 'varchar', length: 50 }) actionLabel!: string; - @Column({ name: 'blocks_role', type: 'varchar', length: 30, nullable: true }) + @Column({ name: 'blocks_role', type: 'varchar', length: 64, nullable: true }) blocksRole?: string | null; } diff --git a/apps/edr-freight-api/src/modules/rule-engine/rule-engine.module.ts b/apps/edr-freight-api/src/modules/rule-engine/rule-engine.module.ts index 3a342ef62..95b5e381d 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/rule-engine.module.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/rule-engine.module.ts @@ -64,7 +64,6 @@ import { RuleEngineService } from './rule-engine.service'; import { NotificationInboxModule } from '../notification-inbox/notification-inbox.module'; -import { BookingApprovalStep } from '../bookings/entities/booking-approval-step.entity'; import { BookingCargoModifier } from '../bookings/entities/booking-cargo-modifier.entity'; import { BookingContainer } from '../bookings/entities/booking-container.entity'; import { BookingRateSnapshot } from '../bookings/entities/booking-rate-snapshot.entity'; @@ -87,7 +86,6 @@ import { BookingRateSnapshot } from '../bookings/entities/booking-rate-snapshot. ApprovalRule, BookingContainer, BookingCargoModifier, - BookingApprovalStep, BookingRateSnapshot, ]), // Team notifications for the priority-rule approval workflow. diff --git a/apps/edr-freight-api/src/modules/rule-engine/rule-engine.service.ts b/apps/edr-freight-api/src/modules/rule-engine/rule-engine.service.ts index d1715d9fd..9d7aa6ba9 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/rule-engine.service.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/rule-engine.service.ts @@ -1,6 +1,5 @@ import { Inject, Injectable, BadRequestException } from '@nestjs/common'; import { DataSource } from 'typeorm'; -import { BookingApprovalStep } from '../bookings/entities/booking-approval-step.entity'; import { BookingRateSnapshot } from '../bookings/entities/booking-rate-snapshot.entity'; import { Rate, RateTrigger } from './entities/rate.entity'; import { @@ -23,15 +22,10 @@ import { IRatesRepository, RATES_REPOSITORY, } from './interfaces/rates.repository.interface'; -import { - IApprovalRulesRepository, - APPROVAL_RULES_REPOSITORY, -} from './interfaces/approval-rules.repository.interface'; import { IShippingLinesRepository, SHIPPING_LINES_REPOSITORY, } from './interfaces/shipping-lines.repository.interface'; -import { DEFAULT_APPROVAL_RULE_ROWS } from './approval-rules.defaults'; import { GOVERNMENT_PRIORITY_BONUS } from './government-priority.constants'; export interface BookingContainerEvalInput { @@ -124,8 +118,6 @@ export class RuleEngineService { private readonly priorityConfigsRepo: IPriorityConfigsRepository, @Inject(RATES_REPOSITORY) private readonly ratesRepo: IRatesRepository, - @Inject(APPROVAL_RULES_REPOSITORY) - private readonly approvalRulesRepo: IApprovalRulesRepository, @Inject(SHIPPING_LINES_REPOSITORY) private readonly shippingLinesRepo: IShippingLinesRepository, private readonly dataSource: DataSource, @@ -390,79 +382,6 @@ export class RuleEngineService { return violations; } - /** - * Ensure ITMLS default approval chains exist (container + bulk). Idempotent. - */ - async ensureDefaultApprovalRules(): Promise { - for (const flag of [false, true] as const) { - const existing = await this.approvalRulesRepo.findChainForCargo(flag); - if (existing.length > 0) continue; - - const rows = DEFAULT_APPROVAL_RULE_ROWS.filter( - (r) => r.requiresDirectorApproval === flag, - ); - for (const row of rows) { - await this.approvalRulesRepo.create({ - requiresDirectorApproval: row.requiresDirectorApproval, - stepOrder: row.stepOrder, - requiredRole: row.requiredRole, - actionLabel: row.actionLabel, - blocksRole: row.blocksRole, - }); - } - } - } - - /** - * Instantiate booking_approval_step rows from approval_rules by freight type. - */ - async instantiateApprovalSteps( - bookingId: string, - options: { - freightType: 'CONTAINER' | 'BULK'; - cargoTypeId?: string | null; - }, - ): Promise { - await this.ensureDefaultApprovalRules(); - - let requiresDirectorApproval = false; - - if (options.cargoTypeId) { - const cargoType = await this.cargoTypesRepo.findById(options.cargoTypeId); - if (!cargoType) { - throw new BadRequestException(`Cargo type ${options.cargoTypeId} not found`); - } - requiresDirectorApproval = cargoType.requiresDirectorApproval; - } - - const chain = await this.approvalRulesRepo.findChainForCargo( - requiresDirectorApproval, - ); - - if (chain.length === 0) { - throw new BadRequestException( - `Approval chain could not be loaded for requiresDirectorApproval=${requiresDirectorApproval}.`, - ); - } - - const stepRepo = this.dataSource.getRepository(BookingApprovalStep); - const steps: BookingApprovalStep[] = []; - - for (const rule of chain) { - const step = stepRepo.create({ - bookingId, - approvalRuleId: rule.id, - stepOrder: rule.stepOrder, - requiredRole: rule.requiredRole, - blocksRole: rule.blocksRole ?? null, - status: 'PENDING', - }); - steps.push(await stepRepo.save(step)); - } - - return steps; - } - /** * Snapshot only the rates used in a booking's final price. */ diff --git a/apps/edr-freight-api/src/modules/rule-engine/services/approval-rules.service.ts b/apps/edr-freight-api/src/modules/rule-engine/services/approval-rules.service.ts index c22677608..843819dba 100644 --- a/apps/edr-freight-api/src/modules/rule-engine/services/approval-rules.service.ts +++ b/apps/edr-freight-api/src/modules/rule-engine/services/approval-rules.service.ts @@ -1,5 +1,6 @@ import { PaginatedResponse } from '@edr/types'; import { BadRequestException, Inject, Injectable, NotFoundException } from '@nestjs/common'; +import { DataSource } from 'typeorm'; import { CreateApprovalRuleDto } from '../dto/create-approval-rule.dto'; import { ListApprovalRulesQueryDto } from '../dto/list-rule-engine-query.dto'; import { ReorderItemsDto } from '../dto/reorder-items.dto'; @@ -17,6 +18,7 @@ export class ApprovalRulesService { @Inject(APPROVAL_RULES_REPOSITORY) private readonly repository: IApprovalRulesRepository, private readonly displayOrder: DisplayOrderService, + private readonly dataSource: DataSource, ) {} /** List approval rules — standard paginated envelope with server-side search. */ @@ -29,6 +31,21 @@ export class ApprovalRulesService { return this.repository.findChainForCargo(requiresDirectorApproval); } + /** + * IAM position types, for the approval-step role picker. A chain step names + * the position type that must approve it, so this is the vocabulary an admin + * builds chains from. Read straight from the shared `iam` schema — the same + * pattern the freight API already uses for `iam.users`. + */ + async listPositionTypes(): Promise> { + const rows = await this.dataSource.query< + Array<{ key: string; label: string }> + >(`SELECT key, COALESCE(name->>'en', key) AS label + FROM iam.position_types + ORDER BY 2`); + return rows.map((row) => ({ label: row.label, value: row.key })); + } + /** Get an approval rule by ID. */ async findById(id: string): Promise { const entity = await this.repository.findById(id); diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/ApprovalStepsCard.tsx b/apps/edr-freight-web/backoffice/src/components/bookings/ApprovalStepsCard.tsx deleted file mode 100644 index a38e2f0e5..000000000 --- a/apps/edr-freight-web/backoffice/src/components/bookings/ApprovalStepsCard.tsx +++ /dev/null @@ -1,217 +0,0 @@ -import { useMemo, useState } from "react"; -import { Check, ShieldCheck } from "lucide-react"; -import { Stack, Group, Text, Badge, Button, Box } from "@mantine/core"; - -import { BookingConfirmDialog } from "./BookingConfirmDialog"; -import { useAuth } from "@/auth/useAuth"; -import { formatApprovalProgress } from "@/features/bookings/approval-progress"; -import { - buildApproveActionForStep, - canActOnApprovalStep, - getNextPendingApprovalStep, -} from "@/features/bookings/booking-actions.config"; -import type { useBookingMutations } from "@/hooks/bookings/useBookings"; -import type { BookingApprovalStep, BookingDetail } from "@/types/booking"; -import { SectionCard } from "./detail/SectionCard"; - -type Mutations = ReturnType; - -interface ApprovalStepsCardProps { - booking: BookingDetail; - mutations: Mutations; -} - -/** Approval chain with inline approve on the current pending step. */ -export function ApprovalStepsCard({ booking, mutations }: ApprovalStepsCardProps) { - const { user } = useAuth(); - const [confirmOpen, setConfirmOpen] = useState(false); - const [pendingStep, setPendingStep] = useState(null); - - const steps = useMemo( - () => [...(booking.approvalSteps ?? [])].sort((a, b) => a.stepOrder - b.stepOrder), - [booking.approvalSteps], - ); - - const nextPending = getNextPendingApprovalStep(steps); - const summary = formatApprovalProgress(booking.status, steps); - const pendingAction = pendingStep ? buildApproveActionForStep(pendingStep) : null; - - const openApprove = (step: BookingApprovalStep) => { - setPendingStep(step); - setConfirmOpen(true); - }; - - const closeApprove = () => { - setConfirmOpen(false); - setPendingStep(null); - }; - - const runApprove = () => { - if (!pendingStep) return; - mutations.approveStep.mutate( - { stepId: pendingStep.id, requiredRole: pendingStep.requiredRole }, - { onSuccess: () => closeApprove() }, - ); - }; - - const subtitle = - summary.detail || - (nextPending - ? `Next: ${nextPending.requiredRole} · step ${nextPending.stepOrder}` - : steps.length - ? "All steps complete" - : "Accept submission to begin"); - - return ( - <> - - {steps.filter((s) => s.status === "APPROVED").length}/{steps.length} - - } - > - - {subtitle} - - - {steps.length === 0 ? ( - - Use Accept for approval in staff actions to instantiate steps. - - ) : ( - - {steps.map((step) => ( - - ))} - - )} - - - { - if (!open) closeApprove(); - else setConfirmOpen(true); - }} - action={pendingAction} - reference={booking.reference} - inputValue="" - onInputChange={() => {}} - onConfirm={runApprove} - isPending={mutations.approveStep.isPending} - /> - - ); -} - -function StepRow({ - step, - steps, - user, - isNext, - isPending, - onApprove, -}: { - step: BookingApprovalStep; - steps: BookingApprovalStep[]; - user: ReturnType["user"]; - isNext: boolean; - isPending: boolean; - onApprove: (step: BookingApprovalStep) => void; -}) { - const canApprove = canActOnApprovalStep(user, step, steps); - const statusColor = - step.status === "APPROVED" - ? "edr-green" - : step.status === "REJECTED" - ? "red" - : isNext - ? "edr-green" - : "gray"; - - return ( - - - - {step.stepOrder} - - - - {step.requiredRole} - - {step.remarks && ( - - {step.remarks} - - )} - - - - {canApprove && ( - - )} - - {step.status} - - - - ); -} diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/BookingActionsMenu.tsx b/apps/edr-freight-web/backoffice/src/components/bookings/BookingActionsMenu.tsx index 13ac9577d..b2d260e37 100644 --- a/apps/edr-freight-web/backoffice/src/components/bookings/BookingActionsMenu.tsx +++ b/apps/edr-freight-web/backoffice/src/components/bookings/BookingActionsMenu.tsx @@ -6,7 +6,6 @@ import { BookingConfirmDialog } from "./BookingConfirmDialog"; import { useBookingActionDialog } from "./useBookingActionDialog"; import { useAuth } from "@/auth/useAuth"; import { - getNextPendingApprovalStep, isAllocateAction, isClearanceNavAction, isContractNavAction, @@ -37,7 +36,6 @@ export function BookingActionsMenu({ status: row.status, paymentCurrency: row.paymentCurrency, reference: row.reference, - approvalSteps: row.approvalSteps, schedulingStatus: row.schedulingStatus, customsClearingEnabled: row.customsClearingEnabled, }; @@ -192,28 +190,6 @@ function ActionDialog({ }} isPending={flow.mutations.isPending || flow.detailLoading} confirmDisabled={flow.confirmDisabled} - extra={ - flow.detailLoading ? ( - - Loading approval steps… - - ) : pendingAction?.id === "approve" && - !getNextPendingApprovalStep(flow.mergedContext.approvalSteps) ? ( - - No pending approval step. Refresh the page after staff accept, or reject the - booking. - - ) : null - } /> ); } diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/BookingApprovalProgressCell.tsx b/apps/edr-freight-web/backoffice/src/components/bookings/BookingApprovalProgressCell.tsx deleted file mode 100644 index be051371c..000000000 --- a/apps/edr-freight-web/backoffice/src/components/bookings/BookingApprovalProgressCell.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { formatApprovalProgress } from "@/features/bookings/approval-progress"; -import type { BookingListRow } from "@/types/booking"; -import { cn } from "@/lib/utils"; - -interface BookingApprovalProgressCellProps { - row: BookingListRow; -} - -export function BookingApprovalProgressCell({ row }: BookingApprovalProgressCellProps) { - const summary = formatApprovalProgress(row.status, row.approvalSteps); - - return ( -
-

- {summary.label} -

- {summary.detail ? ( -

- {summary.detail} -

- ) : null} -
- ); -} diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/detail/BookingApprovalCard.tsx b/apps/edr-freight-web/backoffice/src/components/bookings/detail/BookingApprovalCard.tsx deleted file mode 100644 index c33631d72..000000000 --- a/apps/edr-freight-web/backoffice/src/components/bookings/detail/BookingApprovalCard.tsx +++ /dev/null @@ -1,69 +0,0 @@ -import { CheckCircle, Clock, XCircle } from "lucide-react"; -import { Group, Text, Badge, Timeline } from "@mantine/core"; - -import { SectionCard } from "./SectionCard"; -import { - approvalStatusColor, - formatDateTime, - type BookingApprovalStepView, -} from "./booking-detail.styles"; - -export interface BookingApprovalCardProps { - steps: BookingApprovalStepView[]; - approvedCount: number; -} - -/** Vertical timeline of the booking's approval chain. */ -export function BookingApprovalCard({ steps, approvedCount }: BookingApprovalCardProps) { - return ( - - {approvedCount} / {steps.length} approved - - } - > - - {steps.map((step) => ( - - ) : step.status === "REJECTED" ? ( - - ) : ( - - ) - } - title={ - - - {step.requiredRole.replace(/_/g, " ")} - - - {step.status} - - - } - > - {step.actionedAt && ( - - {formatDateTime(step.actionedAt)} - - )} - - ))} - - - ); -} diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/detail/booking-detail.styles.ts b/apps/edr-freight-web/backoffice/src/components/bookings/detail/booking-detail.styles.ts index 03fcebaae..d3d3e3bc1 100644 --- a/apps/edr-freight-web/backoffice/src/components/bookings/detail/booking-detail.styles.ts +++ b/apps/edr-freight-web/backoffice/src/components/bookings/detail/booking-detail.styles.ts @@ -102,14 +102,6 @@ export interface BookingContainerView { }; } -export interface BookingApprovalStepView { - id: string; - stepOrder: number; - requiredRole: string; - status: string; - actionedAt?: string | null; -} - export interface BookingReviewNoteView { id: string; note: string; @@ -150,7 +142,6 @@ export interface BookingDetailView { cargoType?: BookingNamedRefView; shippingLine?: BookingNamedRefView; bookingContainers?: BookingContainerView[]; - approvalSteps?: BookingApprovalStepView[]; reviewNotes?: BookingReviewNoteView[]; files?: BookingFileView[]; } diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/detail/index.ts b/apps/edr-freight-web/backoffice/src/components/bookings/detail/index.ts index b948cc5dc..f4a677991 100644 --- a/apps/edr-freight-web/backoffice/src/components/bookings/detail/index.ts +++ b/apps/edr-freight-web/backoffice/src/components/bookings/detail/index.ts @@ -10,7 +10,6 @@ export * from "./BookingLifecycleStepper"; export * from "./BookingRouteCard"; export * from "./BookingContainersCard"; export * from "./BookingContainerUnitsCard"; -export * from "./BookingApprovalCard"; export * from "./BookingReviewNotesCard"; export * from "./BookingPaymentCard"; export * from "./BookingPaymentCountdownCard"; diff --git a/apps/edr-freight-web/backoffice/src/components/bookings/useBookingActionDialog.ts b/apps/edr-freight-web/backoffice/src/components/bookings/useBookingActionDialog.ts index 6db13efc6..be7111745 100644 --- a/apps/edr-freight-web/backoffice/src/components/bookings/useBookingActionDialog.ts +++ b/apps/edr-freight-web/backoffice/src/components/bookings/useBookingActionDialog.ts @@ -2,12 +2,11 @@ import { useCallback, useState } from "react"; import { getBookingActions, - getNextPendingApprovalStep, type BookingActionContext, type BookingActionDef, } from "@/features/bookings/booking-actions.config"; import { useAuth } from "@/auth/useAuth"; -import { useBookingDetail, useBookingMutations } from "@/hooks/bookings/useBookings"; +import { useBookingMutations } from "@/hooks/bookings/useBookings"; /** A contract validity window must be a whole number of days, 1–365. */ function isValidValidityDays(value: string): boolean { @@ -24,22 +23,11 @@ export function useBookingActionDialog( const [selectedFile, setSelectedFile] = useState(null); const [dialogOpen, setDialogOpen] = useState(false); - const needsApprovalSteps = - pendingAction?.id === "approve" || pendingAction?.id === "rejectApproval"; + // Bookings no longer have an approval chain, so the dialog needs nothing + // beyond the list-row context it was handed. + const detailLoading = false; - const needsApprovalContext = - context.status === "PENDING_APPROVAL" || - context.status === "APPROVED_PENDING_SIGNATURE"; - - const { data: detail, isLoading: detailLoading } = useBookingDetail( - needsApprovalSteps || needsApprovalContext ? bookingId : undefined, - ); - - const mergedContext: BookingActionContext = { - ...context, - approvalSteps: detail?.approvalSteps ?? context.approvalSteps, - reference: detail?.reference ?? context.reference, - }; + const mergedContext: BookingActionContext = { ...context }; const { user } = useAuth(); const mutations = useBookingMutations(bookingId); @@ -86,24 +74,6 @@ export function useBookingActionDialog( { onSuccess }, ); break; - case "approve": { - const step = getNextPendingApprovalStep(mergedContext.approvalSteps); - if (!step) return; - mutations.approveStep.mutate( - { stepId: step.id, requiredRole: step.requiredRole }, - { onSuccess }, - ); - break; - } - case "rejectApproval": { - const step = getNextPendingApprovalStep(mergedContext.approvalSteps); - if (!step) return; - mutations.rejectStep.mutate( - { stepId: step.id, reason: inputValue.trim() }, - { onSuccess }, - ); - break; - } case "viewContract": break; case "startTransit": @@ -122,16 +92,12 @@ export function useBookingActionDialog( pendingAction, inputValue, selectedFile, - mergedContext.approvalSteps, mutations, closeDialog, ]); const confirmDisabled = mutations.isPending || - (needsApprovalSteps && detailLoading) || - (pendingAction?.id === "approve" && - !getNextPendingApprovalStep(mergedContext.approvalSteps)) || (pendingAction?.input === "file" && !selectedFile) || (pendingAction?.input === "reason" && !inputValue.trim()) || (pendingAction?.input === "note" && !inputValue.trim()) || diff --git a/apps/edr-freight-web/backoffice/src/components/contracts/ContractActionsToolbar.tsx b/apps/edr-freight-web/backoffice/src/components/contracts/ContractActionsToolbar.tsx index 0aeb624b3..7896da730 100644 --- a/apps/edr-freight-web/backoffice/src/components/contracts/ContractActionsToolbar.tsx +++ b/apps/edr-freight-web/backoffice/src/components/contracts/ContractActionsToolbar.tsx @@ -4,11 +4,10 @@ import { useQuery } from "@tanstack/react-query"; import { Button, Modal, Stack, Text, Textarea } from "@mantine/core"; import { Check, - FileCheck, + Eye, FilePen, FileSignature, MessageSquareWarning, - RefreshCw, ShieldCheck, XCircle, Zap, @@ -16,8 +15,10 @@ import { import type { Freight } from "@edr/types"; import { api } from "@/services/api"; +import { contractsService } from "@/services/contracts.service"; import { SectionCard } from "@/components/bookings/detail/SectionCard"; import { ContractDocumentEditorModal } from "@/components/contracts/ContractDocumentEditorModal"; +import { ContractPreviewModal } from "@/components/contracts/ContractPreviewModal"; import type { useContractMutations } from "@/hooks/contracts/useContracts"; /** Dropdown-settings code holding the admin-configured contract validity days. */ @@ -51,11 +52,21 @@ export function ContractActionsToolbar({ const [editorOpen, setEditorOpen] = useState(false); const [editorMode, setEditorMode] = useState<"accept" | "edit">("accept"); + const [previewOpen, setPreviewOpen] = useState(false); const [changesOpen, setChangesOpen] = useState(false); const [changesNote, setChangesNote] = useState(""); const [rejectOpen, setRejectOpen] = useState(false); const [rejectReason, setRejectReason] = useState(""); + // Whether the document is editable depends on WHO is viewing — only the + // approver whose turn it is may edit — so the server decides, not the client. + const { data: draft } = useQuery({ + queryKey: ["contracts", contract.id, "document-draft"], + queryFn: () => contractsService.getContractDocumentDraft(contract.id), + enabled: contract.status === "PENDING_APPROVAL", + staleTime: 0, + }); + // Admin-configured validity durations (days) for the accept dialog. Staff can // only pick one of these — no free-typing. Read-only setting, fetched once. const { data: validitySetting, isLoading: validityLoading } = useQuery({ @@ -87,18 +98,11 @@ export function ContractActionsToolbar({ } const canAccept = status === "SUBMITTED"; - // While the contract is PENDING_APPROVAL and NO approver has acted yet, staff - // can edit this contract's articles and (re)generate its PDF. The first - // approval action locks the document. - const docLocked = - status !== "PENDING_APPROVAL" || - (contract.approvalSteps ?? []).some((s) => s.status !== "PENDING"); - const canEditGenerate = status === "PENDING_APPROVAL" && !docLocked; - const documentGenerated = Boolean(contract.contractGeneratedAt); - // Legacy fallback: if a contract ever lands on APPROVED without a document - // (older flow), still offer a manual generate that moves it to CONTRACT_READY. - const needsManualGenerate = - status === "APPROVED" && !contract.contractGeneratedAt; + // The document stays editable for the whole approval chain, but only by the + // approver whose turn it is. The server resolves that against the caller's + // position type; the client cannot derive it. + const canEditDocument = Boolean(draft?.editableByMe); + const inApproval = status === "PENDING_APPROVAL"; // Signing now happens on the contract VIEW page (staff must open and read the // generated contract before signing) — no sign button in this toolbar. const canViewContract = @@ -154,56 +158,41 @@ export function ContractActionsToolbar({ )} - {canEditGenerate && ( + {inApproval && ( <> - {documentGenerated - ? "Document generated. Approvers can now review it. You can still edit and regenerate until the first approval." - : "Review the contract document, edit its articles if needed, then generate it so approvers can review."} + {canEditDocument + ? "It is your turn to approve. You can edit the articles before approving — the PDF is generated automatically once the last approver approves." + : draft?.nextApproverRole + ? `Awaiting ${draft.nextApproverRole}. Only the current approver can edit the document.` + : "Awaiting approval."} - + {canEditDocument && ( + + )} )} - {needsManualGenerate && ( - - )} - {canViewContract && ( - - - { setArticles((prev) => { @@ -215,7 +217,9 @@ export function ContractDocumentEditorModal({ icon={locked ? : } > {locked - ? "This document is locked — an approver has already acted, so it can no longer be edited." + ? draft?.nextApproverRole + ? `Only the current approver (${draft.nextApproverRole}) can edit this document right now.` + : "This document can no longer be edited — the contract has advanced beyond approval." : "Edits apply to THIS contract only. The six shared templates are never changed."} diff --git a/apps/edr-freight-web/backoffice/src/components/contracts/ContractPreviewModal.tsx b/apps/edr-freight-web/backoffice/src/components/contracts/ContractPreviewModal.tsx new file mode 100644 index 000000000..c165645b7 --- /dev/null +++ b/apps/edr-freight-web/backoffice/src/components/contracts/ContractPreviewModal.tsx @@ -0,0 +1,78 @@ +import { useQuery } from "@tanstack/react-query"; +import { Alert, Group, Loader, Modal, Text } from "@mantine/core"; +import { Info } from "lucide-react"; + +import { contractsService } from "@/services/contracts.service"; + +interface ContractPreviewModalProps { + opened: boolean; + onClose: () => void; + contractId: string; +} + +/** + * Live preview of the contract document. Renders server-side HTML, not the + * stored PDF — the PDF is only produced once the final approver approves, so + * before that this is the document. Served in an iframe so the contract's own + * styles stay sandboxed away from the app. + */ +export function ContractPreviewModal({ + opened, + onClose, + contractId, +}: ContractPreviewModalProps) { + const { data, isLoading, isError } = useQuery({ + queryKey: ["contracts", contractId, "contract-view"], + queryFn: () => contractsService.getContractView(contractId), + enabled: opened, + // The document changes as approvers edit it, so never serve a stale render. + staleTime: 0, + }); + + return ( + + } + color="blue" + variant="light" + mb="sm" + p="xs" + > + + Draft preview. The PDF is generated automatically once the final + approver approves. + + + + {isLoading ? ( + + + + Rendering document… + + + ) : isError || !data?.html ? ( + + The document could not be rendered. Check that the contract has a + template and try again. + + ) : ( +