Tria/edr-platform
1
0
Fork 0
mirror of https://github.com/Tria-plc/edr-platform.git synced 2026-08-30 08:48:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: otp double sending

Browse Source
This commit is contained in:
Nathnael
2026-07-20 12:10:49 +00:00
parent aa02700e4c
commit 0549a88d57
17 changed files with 741 additions and 461 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
apps/edr-freight-api/src/modules/otp/otp.controller.ts
View File

@@ -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

134
apps/edr-freight-api/src/modules/otp/otp.repository.ts
View File

@@ -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<OtpVerification>[] {
const where: FindOptionsWhere<OtpVerification>[] =
[];
// ---------------------------------------------------------------------------
// 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
);
}
}
}

231
apps/edr-freight-api/src/modules/otp/otp.service.spec.ts
View File

@@ -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<string, { phone?: string; email?: string; otp: string; updatedAt: Date }>();
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/,
);
});
});

284
apps/edr-freight-api/src/modules/otp/otp.service.ts
View File

@@ -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<DispatchOutcome> {
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<DispatchOutcome> {
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<string, number>();
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) {