mirror of
https://github.com/Tria-plc/edr-platform.git
synced 2026-08-28 14:50:57 +00:00
fix issue
This commit is contained in:
@@ -19,6 +19,9 @@ function toTarget(phone?: string, email?: string): OtpTarget {
|
||||
throw new BadRequestException("phone or email is required");
|
||||
}
|
||||
|
||||
// TODO: these public routes need per-target + per-IP rate limiting (a NestJS
|
||||
// ThrottlerGuard / @Throttle on /otp/send and /otp/verify). No Throttler is
|
||||
// wired into the app yet; add @nestjs/throttler and apply it here.
|
||||
@Controller("otp")
|
||||
@Public()
|
||||
export class OtpController {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// otp.service.ts
|
||||
|
||||
import { BadRequestException, Injectable, Logger } from "@nestjs/common";
|
||||
import { randomInt } from "node:crypto";
|
||||
|
||||
import { OtpRepository } from "./otp.repository";
|
||||
|
||||
@@ -25,7 +26,9 @@ export class OtpService {
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
generateOtp(): string {
|
||||
return Math.floor(100000 + Math.random() * 900000).toString();
|
||||
// Cryptographically secure 6-digit code (100000–999999). Math.random() is a
|
||||
// non-CSPRNG and must never be used to mint a security token.
|
||||
return randomInt(100000, 1000000).toString();
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -50,8 +53,13 @@ export class OtpService {
|
||||
await this.otpRepository.createOtp(target, otp);
|
||||
}
|
||||
|
||||
// A freshly issued code gets a fresh guess budget.
|
||||
this.actionAttempts.delete(this.targetKey(target));
|
||||
// NOTE: do NOT reset the brute-force attempt counter on send. Clearing it
|
||||
// here let an attacker wipe the per-target guess budget just by calling
|
||||
// /otp/send between guesses. The counter is cleared only when the code is
|
||||
// consumed/expired during verification.
|
||||
// TODO: add per-target + per-IP rate limiting on the public /otp/send and
|
||||
// /otp/verify routes (a NestJS ThrottlerGuard / @Throttle) — none exists
|
||||
// in the codebase yet.
|
||||
|
||||
if (target.email) {
|
||||
// send email (queued to RabbitMQ via the shared Email service)
|
||||
@@ -94,6 +102,7 @@ export class OtpService {
|
||||
async verifyOtp(target: OtpTarget, otp: string) {
|
||||
// find the channel's row
|
||||
const otpData = await this.otpRepository.findByTarget(target);
|
||||
const key = this.targetKey(target);
|
||||
|
||||
// not found
|
||||
if (!otpData) {
|
||||
@@ -102,13 +111,35 @@ export class OtpService {
|
||||
);
|
||||
}
|
||||
|
||||
// invalid otp
|
||||
// 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();
|
||||
if (ageMs > this.ACTION_OTP_TTL_MS) {
|
||||
await this.otpRepository.deleteOtp(otpData);
|
||||
this.actionAttempts.delete(key);
|
||||
throw new BadRequestException(
|
||||
"Verification code has expired. Request a new one.",
|
||||
);
|
||||
}
|
||||
|
||||
// invalid otp — per-target attempt cap so a 6-digit code can't be
|
||||
// brute-forced within its TTL; the code is burned once the budget is spent.
|
||||
if (otpData.otp !== otp) {
|
||||
const attempts = (this.actionAttempts.get(key) ?? 0) + 1;
|
||||
if (attempts >= this.MAX_ACTION_ATTEMPTS) {
|
||||
await this.otpRepository.deleteOtp(otpData);
|
||||
this.actionAttempts.delete(key);
|
||||
throw new BadRequestException(
|
||||
"Too many incorrect attempts. Request a new code.",
|
||||
);
|
||||
}
|
||||
this.actionAttempts.set(key, attempts);
|
||||
throw new BadRequestException("Invalid OTP");
|
||||
}
|
||||
|
||||
// mark verified
|
||||
await this.otpRepository.markVerified(otpData);
|
||||
// single-use: consume the code on success so it can't be replayed.
|
||||
await this.otpRepository.deleteOtp(otpData);
|
||||
this.actionAttempts.delete(key);
|
||||
|
||||
return {
|
||||
success: true,
|
||||
|
||||
Reference in New Issue
Block a user