mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-09-07 11:55:42 +00:00
fix(otp): fall back to email for foreign phone numbers
The SMS gateway is domestic-only, but OTP sends fanned out to any phone on the account - a foreign number meant a code queued into the void while the response claimed success. isDomesticPhone (+2519/+2517 E.164) now gates SMS: dual-channel sends with a foreign phone go email-only (the phone stays on the row so verify still matches it), and a phone-only foreign target still tries SMS as the only route. The staff-triggered reset exposes phoneIsDomestic so the backoffice disables the SMS channel with an explanation, and the API refuses the channel directly for foreign numbers. EDRFREIGHT-186
This commit is contained in:
@@ -12,6 +12,7 @@ import {
|
|||||||
RESET_LINK_TTL_MS,
|
RESET_LINK_TTL_MS,
|
||||||
} from "./forgot-password.service";
|
} from "./forgot-password.service";
|
||||||
import { maskOtpTarget } from "./mask-target.util";
|
import { maskOtpTarget } from "./mask-target.util";
|
||||||
|
import { isDomesticPhone } from "../otp/otp.service";
|
||||||
|
|
||||||
/** The account a staff-triggered reset would land on. */
|
/** The account a staff-triggered reset would land on. */
|
||||||
export interface CustomerResetTarget {
|
export interface CustomerResetTarget {
|
||||||
@@ -19,6 +20,12 @@ export interface CustomerResetTarget {
|
|||||||
name: string;
|
name: string;
|
||||||
email: string | null;
|
email: string | null;
|
||||||
phone: 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 {
|
export interface SentResetLink {
|
||||||
@@ -58,6 +65,9 @@ export class CustomerResetService {
|
|||||||
name: `${profile.firstName} ${profile.lastName}`.trim(),
|
name: `${profile.firstName} ${profile.lastName}`.trim(),
|
||||||
email: user.email ?? null,
|
email: user.email ?? null,
|
||||||
phone: user.phoneNumber ?? 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);
|
const target = this.forgotPasswordService.targetFor(user, channel);
|
||||||
if (!target) return null;
|
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
|
// 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
|
// expires, whereas sending a link before the ticket exists would hand the
|
||||||
// customer a URL that is dead on arrival.
|
// customer a URL that is dead on arrival.
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { OtpService, normalizeOtpTarget } from './otp.service';
|
import { OtpService, isDomesticPhone, normalizeOtpTarget } from './otp.service';
|
||||||
|
|
||||||
describe('normalizeOtpTarget', () => {
|
describe('normalizeOtpTarget', () => {
|
||||||
it('canonicalises Ethiopian forms to one E.164 key', () => {
|
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 {
|
interface FakeRow {
|
||||||
id: string;
|
id: string;
|
||||||
phone?: string;
|
phone?: string;
|
||||||
@@ -184,6 +196,26 @@ describe('OtpService — dual-channel send', () => {
|
|||||||
expect(email.sendEmail).not.toHaveBeenCalled();
|
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 () => {
|
it('still succeeds when one transport throws', async () => {
|
||||||
const { service, rows } = makeService({
|
const { service, rows } = makeService({
|
||||||
sms: async () => {
|
sms: async () => {
|
||||||
|
|||||||
@@ -46,6 +46,16 @@ function normalizePhone(rawPhone: string): string {
|
|||||||
return digits.length >= 11 ? `+${digits}` : raw;
|
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
|
* Canonicalise every channel present on the target. Each field is normalised
|
||||||
* independently — a dual-channel target must end up with both halves in their
|
* 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
|
// /otp/verify routes (a NestJS ThrottlerGuard / @Throttle) — none exists
|
||||||
// in the codebase yet.
|
// 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
|
// Fan out to every channel the target has, independently: one transport
|
||||||
// being down must not suppress the other, which is the whole point of
|
// 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
|
// sending to both. Each helper swallows its own failure so a rejected
|
||||||
@@ -150,7 +174,7 @@ export class OtpService {
|
|||||||
const outcomes = (
|
const outcomes = (
|
||||||
await Promise.all([
|
await Promise.all([
|
||||||
target.email ? this.dispatchEmail(target.email, otp) : null,
|
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);
|
).filter((outcome): outcome is DispatchOutcome => outcome !== null);
|
||||||
|
|
||||||
|
|||||||
@@ -61,8 +61,11 @@ export default function ResetPasswordAction({
|
|||||||
|
|
||||||
if (!allowed) return null;
|
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 =
|
const channelMissing =
|
||||||
!!target && (channel === "email" ? !target.email : !target.phone);
|
!!target && (channel === "email" ? !target.email : !phoneUsable);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
@@ -106,9 +109,13 @@ export default function ResetPasswordAction({
|
|||||||
<Radio
|
<Radio
|
||||||
value="phone"
|
value="phone"
|
||||||
label="SMS"
|
label="SMS"
|
||||||
disabled={!target.phone}
|
disabled={!phoneUsable}
|
||||||
description={
|
description={
|
||||||
target.phone ?? "No phone number on this account"
|
!target.phone
|
||||||
|
? "No phone number on this account"
|
||||||
|
: target.phoneIsDomestic === false
|
||||||
|
? `${target.phone} — foreign number, SMS unavailable; use email`
|
||||||
|
: target.phone
|
||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
<Radio
|
<Radio
|
||||||
|
|||||||
@@ -131,6 +131,8 @@ export interface CustomerResetTarget {
|
|||||||
name: string;
|
name: string;
|
||||||
email: string | null;
|
email: string | null;
|
||||||
phone: string | null;
|
phone: string | null;
|
||||||
|
/** SMS gateway is domestic-only; `false` means SMS can't reach this phone. `null` = no phone. */
|
||||||
|
phoneIsDomestic: boolean | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Mirrors backend `Company` (+ its `companyProfiles`). */
|
/** Mirrors backend `Company` (+ its `companyProfiles`). */
|
||||||
|
|||||||
Reference in New Issue
Block a user