From 0549a88d57467a052e06693623db65cc293d5343 Mon Sep 17 00:00:00 2001 From: Nathnael Date: Mon, 20 Jul 2026 12:10:49 +0000 Subject: [PATCH] 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 {