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 91ddaf1a9..4eb6ecc31 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 @@ -12,6 +12,7 @@ import { RESET_LINK_TTL_MS, } from "./forgot-password.service"; import { maskOtpTarget } from "./mask-target.util"; +import { isDomesticPhone } from "../otp/otp.service"; /** The account a staff-triggered reset would land on. */ export interface CustomerResetTarget { @@ -19,6 +20,12 @@ export interface CustomerResetTarget { name: string; email: string | null; phone: string | null; + /** + * Whether the SMS gateway (domestic-only) can reach `phone`. `null` when + * there is no phone. The backoffice uses this to disable the SMS channel for + * foreign numbers instead of sending a link that will never arrive. + */ + phoneIsDomestic: boolean | null; } export interface SentResetLink { @@ -58,6 +65,9 @@ export class CustomerResetService { name: `${profile.firstName} ${profile.lastName}`.trim(), email: user.email ?? null, phone: user.phoneNumber ?? null, + phoneIsDomestic: user.phoneNumber + ? isDomesticPhone(user.phoneNumber) + : null, }; } @@ -80,6 +90,17 @@ export class CustomerResetService { const target = this.forgotPasswordService.targetFor(user, channel); if (!target) return null; + // A foreign number is unreachable by the domestic-only SMS gateway — treat + // it like a missing phone rather than reporting "link sent" for a message + // that will never arrive. The backoffice disables the channel up front via + // `phoneIsDomestic`; this guards direct API calls. + if (channel === "phone" && target.phone && !isDomesticPhone(target.phone)) { + this.logger.warn( + `Staff reset via SMS refused for user ${userId} — non-domestic phone`, + ); + 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. 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 0494b48b6..7b7f97f12 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 @@ -1,4 +1,4 @@ -import { OtpService, normalizeOtpTarget } from './otp.service'; +import { OtpService, isDomesticPhone, normalizeOtpTarget } from './otp.service'; describe('normalizeOtpTarget', () => { it('canonicalises Ethiopian forms to one E.164 key', () => { @@ -28,6 +28,18 @@ describe('normalizeOtpTarget', () => { }); }); +describe('isDomesticPhone', () => { + it.each(['+251986680099', '0986680099', '0712345678', '251986680099'])( + 'accepts Ethiopian mobile form %s', + (phone) => expect(isDomesticPhone(phone)).toBe(true), + ); + + it.each(['+14155550123', '+447911123456', '+2519866', '12345'])( + 'rejects non-domestic or malformed %s', + (phone) => expect(isDomesticPhone(phone)).toBe(false), + ); +}); + interface FakeRow { id: string; phone?: string; @@ -184,6 +196,26 @@ describe('OtpService — dual-channel send', () => { expect(email.sendEmail).not.toHaveBeenCalled(); }); + it('skips SMS for a foreign number when email is available', async () => { + const { service, sms, email, rows } = makeService(); + await service.sendOtp({ phone: '+14155550123', email: 'user@example.com' }); + + // The gateway is domestic-only — email is the delivery route, but the + // foreign phone stays on the row so verify still matches either channel. + expect(sms.sendSms).not.toHaveBeenCalled(); + expect(email.sendEmail).toHaveBeenCalledTimes(1); + await expect( + service.verifyOtpForAction({ phone: '+14155550123' }, rows()[0]!.otp), + ).resolves.toEqual({ success: true }); + }); + + it('still attempts SMS for a foreign number when it is the only channel', async () => { + const { service, sms } = makeService(); + await service.sendOtp({ phone: '+14155550123' }); + + expect(sms.sendSms).toHaveBeenCalledTimes(1); + }); + it('still succeeds when one transport throws', async () => { const { service, rows } = makeService({ sms: async () => { 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 e19323067..283a7f77d 100644 --- a/apps/edr-freight-api/src/modules/otp/otp.service.ts +++ b/apps/edr-freight-api/src/modules/otp/otp.service.ts @@ -46,6 +46,16 @@ function normalizePhone(rawPhone: string): string { return digits.length >= 11 ? `+${digits}` : raw; } +/** + * Whether a phone is an Ethiopian mobile the SMS gateway can actually reach — + * the carrier integration is domestic-only, so a send to anything else is + * queued and silently lost. Callers use this to fall back to email instead of + * pretending an SMS is on its way. + */ +export function isDomesticPhone(rawPhone: string): boolean { + return /^\+251[79]\d{8}$/.test(normalizePhone(rawPhone)); +} + /** * Canonicalise every channel present on the target. Each field is normalised * independently — a dual-channel target must end up with both halves in their @@ -143,6 +153,20 @@ export class OtpService { // /otp/verify routes (a NestJS ThrottlerGuard / @Throttle) — none exists // in the codebase yet. + // A foreign number is unreachable by the domestic-only SMS gateway; when + // email is also on the target, go email-only rather than queueing an SMS + // that will never arrive. With no email the SMS attempt stays — it is the + // only route there is. + const smsPhone = + target.phone && (!target.email || isDomesticPhone(target.phone)) + ? target.phone + : null; + if (target.phone && !smsPhone) { + this.logger.warn( + `otp.dispatch.sms-skipped target=${label} — non-domestic phone, delivering via email only`, + ); + } + // 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 @@ -150,7 +174,7 @@ export class OtpService { const outcomes = ( await Promise.all([ target.email ? this.dispatchEmail(target.email, otp) : null, - target.phone ? this.dispatchSms(target.phone, otp) : null, + smsPhone ? this.dispatchSms(smsPhone, otp) : null, ]) ).filter((outcome): outcome is DispatchOutcome => outcome !== null); 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 2175d4d7b..f0e9b266a 100644 --- a/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx +++ b/apps/edr-freight-web/backoffice/src/components/customers/ResetPasswordAction.tsx @@ -61,8 +61,11 @@ export default function ResetPasswordAction({ if (!allowed) return null; + // SMS is domestic-only: a foreign number counts as unavailable, same as a + // missing one, so staff can't send a link that will never arrive. + const phoneUsable = !!target?.phone && target.phoneIsDomestic !== false; const channelMissing = - !!target && (channel === "email" ? !target.email : !target.phone); + !!target && (channel === "email" ? !target.email : !phoneUsable); return ( <> @@ -106,9 +109,13 @@ export default function ResetPasswordAction({