chore: filter out safari phone as foreign

This commit is contained in:
Nathnael
2026-07-22 07:18:07 +00:00
parent 82cb5508cc
commit a5ab6fe5c1
2 changed files with 97 additions and 94 deletions

View File

@@ -1,41 +1,42 @@
import { OtpService, isDomesticPhone, normalizeOtpTarget } from './otp.service';
import { OtpService, isDomesticPhone, normalizeOtpTarget } from "./otp.service";
describe('normalizeOtpTarget', () => {
it('canonicalises Ethiopian forms to one E.164 key', () => {
const forms = ['+251986680099', '251986680099', '0986680099', '+251 98 668 0099'];
describe("normalizeOtpTarget", () => {
it("canonicalises Ethiopian forms to one E.164 key", () => {
const forms = [
"+251986680099",
"251986680099",
"0986680099",
"+251 98 668 0099",
];
const keys = forms.map((phone) => normalizeOtpTarget({ phone }).phone);
expect(new Set(keys)).toEqual(new Set(['+251986680099']));
expect(new Set(keys)).toEqual(new Set(["+251986680099"]));
});
it('maps local 07… mobile to +2517…', () => {
expect(normalizeOtpTarget({ phone: '0712345678' }).phone).toBe('+251712345678');
});
it('canonicalises email case and surrounding whitespace to one key', () => {
const forms = ['a@b.com', 'A@B.com', ' a@B.COM ', 'A@b.COM'];
it("canonicalises email case and surrounding whitespace to one key", () => {
const forms = ["a@b.com", "A@B.com", " a@B.COM ", "A@b.COM"];
const keys = forms.map((email) => normalizeOtpTarget({ email }).email);
expect(new Set(keys)).toEqual(new Set(['a@b.com']));
expect(new Set(keys)).toEqual(new Set(["a@b.com"]));
});
it('keeps an already-normalised email stable (idempotent)', () => {
const once = normalizeOtpTarget({ email: ' User@Example.COM ' }).email!;
it("keeps an already-normalised email stable (idempotent)", () => {
const once = normalizeOtpTarget({ email: " User@Example.COM " }).email!;
expect(normalizeOtpTarget({ email: once }).email).toBe(once);
});
it('keeps an already-normalised number stable (idempotent)', () => {
const once = normalizeOtpTarget({ phone: '0986680099' }).phone!;
it("keeps an already-normalised number stable (idempotent)", () => {
const once = normalizeOtpTarget({ phone: "0986680099" }).phone!;
expect(normalizeOtpTarget({ phone: once }).phone).toBe(once);
});
});
describe('isDomesticPhone', () => {
it.each(['+251986680099', '0986680099', '0712345678', '251986680099'])(
'accepts Ethiopian mobile form %s',
describe("isDomesticPhone", () => {
it.each(["+251986680099", "0986680099", "251986680099"])(
"accepts Ethiopian mobile form %s",
(phone) => expect(isDomesticPhone(phone)).toBe(true),
);
it.each(['+14155550123', '+447911123456', '+2519866', '12345'])(
'rejects non-domestic or malformed %s',
it.each(["+14155550123", "+447911123456", "0712345678", "+2519866", "12345"])(
"rejects non-domestic or malformed %s",
(phone) => expect(isDomesticPhone(phone)).toBe(false),
);
});
@@ -63,7 +64,8 @@ function makeService(
let nextId = 1;
const matches = (row: FakeRow, t: { phone?: string; email?: string }) =>
(!!t.email && row.email === t.email) || (!!t.phone && row.phone === t.phone);
(!!t.email && row.email === t.email) ||
(!!t.phone && row.phone === t.phone);
const repo = {
findByTarget: jest.fn(
@@ -101,30 +103,30 @@ function makeService(
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 () => {
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' });
await service.sendOtp({ phone: "+251986680099" });
await expect(
service.verifyOtpForAction({ phone: '0986680099' }, rows()[0]!.otp),
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 () => {
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 ' });
await service.sendOtp({ email: " User@Example.COM " });
await expect(
service.verifyOtpForAction({ email: 'user@example.com' }, rows()[0]!.otp),
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' };
describe("OtpService — dual-channel send", () => {
const both = { phone: "0986680099", email: "User@Example.COM" };
it('sends ONE code to both transports', async () => {
it("sends ONE code to both transports", async () => {
const { service, sms, email, rows } = makeService();
await service.sendOtp(both);
@@ -134,92 +136,95 @@ describe('OtpService — dual-channel send', () => {
// Same secret on both messages — the user types whichever arrives first.
expect(sms.sendSms).toHaveBeenCalledWith(
expect.objectContaining({
to: '+251986680099',
to: "+251986680099",
message: expect.stringContaining(otp),
}),
);
expect(email.sendEmail).toHaveBeenCalledWith(
expect.objectContaining({
to: 'user@example.com',
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',
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);
["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 });
});
await expect(
service.verifyOtpForAction(target, rows()[0]!.otp),
).resolves.toEqual({ success: true });
},
);
it('consuming the code via one channel kills the other', async () => {
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);
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),
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 () => {
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({ phone: "0986680099" });
await service.sendOtp(both);
expect(rows()).toHaveLength(1);
expect(rows()[0]).toMatchObject({ email: 'user@example.com' });
expect(rows()[0]).toMatchObject({ email: "user@example.com" });
});
it('degrades to one channel when the account has only one contact', async () => {
it("degrades to one channel when the account has only one contact", async () => {
const { service, sms, email } = makeService();
await service.sendOtp({ phone: '0986680099' });
await service.sendOtp({ phone: "0986680099" });
expect(sms.sendSms).toHaveBeenCalledTimes(1);
expect(email.sendEmail).not.toHaveBeenCalled();
});
it('skips SMS for a foreign number when email is available', async () => {
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' });
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),
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 () => {
it("still attempts SMS for a foreign number when it is the only channel", async () => {
const { service, sms } = makeService();
await service.sendOtp({ phone: '+14155550123' });
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({
sms: async () => {
throw new Error('broker down');
throw new Error("broker down");
},
});
@@ -229,24 +234,24 @@ describe('OtpService — dual-channel send', () => {
});
// The code is live and verifiable on the channel that worked.
await expect(
service.verifyOtpForAction({ email: 'user@example.com' }, rows()[0]!.otp),
service.verifyOtpForAction({ email: "user@example.com" }, rows()[0]!.otp),
).resolves.toEqual({ success: true });
});
it('fails the request when every transport throws', async () => {
it("fails the request when every transport throws", async () => {
const { service } = makeService({
sms: async () => {
throw new Error('broker down');
throw new Error("broker down");
},
email: async () => {
throw new Error('broker down');
throw new Error("broker down");
},
});
await expect(service.sendOtp(both)).rejects.toThrow('Failed to send OTP');
await expect(service.sendOtp(both)).rejects.toThrow("Failed to send OTP");
});
it('shares one brute-force budget across both channels', async () => {
it("shares one brute-force budget across both channels", async () => {
const { service, rows } = makeService();
await service.sendOtp(both);
const otp = rows()[0]!.otp;
@@ -254,17 +259,17 @@ describe('OtpService — dual-channel send', () => {
// 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' },
{ 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(target, "000000"),
).rejects.toThrow("Invalid verification code");
}
await expect(
service.verifyOtpForAction({ email: 'user@example.com' }, '000000'),
service.verifyOtpForAction({ email: "user@example.com" }, "000000"),
).rejects.toThrow(/Too many incorrect attempts/);
// Burned: even the correct code no longer works.

View File

@@ -36,9 +36,9 @@ function channelsOf(target: OtpTarget): Array<"email" | "sms"> {
*/
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+/, '');
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
@@ -53,7 +53,7 @@ function normalizePhone(rawPhone: string): string {
* pretending an SMS is on its way.
*/
export function isDomesticPhone(rawPhone: string): boolean {
return /^\+251[79]\d{8}$/.test(normalizePhone(rawPhone));
return /^\+2519\d{8}$/.test(normalizePhone(rawPhone));
}
/**
@@ -180,10 +180,8 @@ export class OtpService {
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}` : ""
`otp.dispatch channel=${outcome.channel} target=${label} queued=${outcome.queued
} latencyMs=${Date.now() - startedAt}${outcome.error ? ` error=${outcome.error}` : ""
}`,
);
}
@@ -207,8 +205,7 @@ export class OtpService {
// 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 channels=${channels.join("+")} target=${label} rabbitmqEnabled=${
process.env.RABBITMQ_ENABLED ?? "unset"
`otp.dispatch.dropped channels=${channels.join("+")} target=${label} rabbitmqEnabled=${process.env.RABBITMQ_ENABLED ?? "unset"
} — no transport reported hand-off; no code will arrive for this send`,
);
}
@@ -233,8 +230,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 channels=${channels.join("+")} target=${label} latencyMs=${
Date.now() - startedAt
`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,
);
@@ -294,9 +290,7 @@ export class OtpService {
* address while printing the credential next to it would buy nothing.
*/
private targetLabel(target: OtpTarget): string {
return (
[target.email, target.phone].filter(Boolean).join("+") || "unknown"
);
return [target.email, target.phone].filter(Boolean).join("+") || "unknown";
}
/**
@@ -312,9 +306,8 @@ export class OtpService {
) {
const line = `otp.verify channels=${channelsOf(target).join(
"+",
)} target=${this.targetLabel(target)} mode=${mode} result=${result}${
detail ? ` ${detail}` : ""
}`;
)} target=${this.targetLabel(target)} mode=${mode} result=${result}${detail ? ` ${detail}` : ""
}`;
if (result === "ok") this.logger.log(line);
else this.logger.warn(line);
}
@@ -463,7 +456,12 @@ export class OtpService {
await this.otpRepository.deleteOtp(otpData);
this.actionAttempts.delete(key);
this.logVerify(target, "action", "expired", `ageMs=${ageMs} ttlMs=${ttlMs}`);
this.logVerify(
target,
"action",
"expired",
`ageMs=${ageMs} ttlMs=${ttlMs}`,
);
throw new BadRequestException(
"Verification code has expired. Request a new one.",
);