From 6420c72e89e2462c19adde17bbb25e31d787b8c0 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 20 Jul 2026 11:20:50 +0000 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] 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/5] 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 {