From 17d89b73f1563f72f535860920f76362978c4c59 Mon Sep 17 00:00:00 2001 From: hayor1 <140841763+hayor1@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:10:16 +0100 Subject: [PATCH] fix(kyc): configure submission rate limits --- server/.env.example | 9 +++ .../db/migrations/016_kyc_ip_rate_limit.sql | 15 ++++ server/src/middleware/rateLimitKyc.test.ts | 73 +++++++++++++++++++ server/src/middleware/rateLimitKyc.ts | 42 +++++++---- 4 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 server/src/db/migrations/016_kyc_ip_rate_limit.sql create mode 100644 server/src/middleware/rateLimitKyc.test.ts diff --git a/server/.env.example b/server/.env.example index 8bc1131..e0fb351 100644 --- a/server/.env.example +++ b/server/.env.example @@ -176,3 +176,12 @@ OTEL_SERVICE_NAME=airflex-server # [OPTIONAL] Set to "false" to disable telemetry instrumentation (e.g. in test runs). Defaults to true. OTEL_ENABLED=true + +# --------------------------------------------------------------------------- +# KYC submission rate limit (issue #485) +# --------------------------------------------------------------------------- +# [OPTIONAL] Per-user submissions in the rolling window. Defaults to 3. +KYC_MAX_SUBMISSIONS=3 + +# [OPTIONAL] Rolling window length in minutes. Defaults to 60. +KYC_WINDOW_MINUTES=60 diff --git a/server/src/db/migrations/016_kyc_ip_rate_limit.sql b/server/src/db/migrations/016_kyc_ip_rate_limit.sql new file mode 100644 index 0000000..754e8bb --- /dev/null +++ b/server/src/db/migrations/016_kyc_ip_rate_limit.sql @@ -0,0 +1,15 @@ +-- Allow KYC rate-limit attempts to fall back to the request IP when there is +-- no authenticated user identity. Existing authenticated rows remain intact. +ALTER TABLE kyc_submission_attempts + ALTER COLUMN user_id DROP NOT NULL; + +ALTER TABLE kyc_submission_attempts + ADD COLUMN IF NOT EXISTS ip_address INET; + +ALTER TABLE kyc_submission_attempts + ADD CONSTRAINT kyc_submission_attempts_has_identity + CHECK (user_id IS NOT NULL OR ip_address IS NOT NULL); + +CREATE INDEX IF NOT EXISTS idx_kyc_submission_attempts_ip_time + ON kyc_submission_attempts (ip_address, requested_at DESC) + WHERE user_id IS NULL; diff --git a/server/src/middleware/rateLimitKyc.test.ts b/server/src/middleware/rateLimitKyc.test.ts new file mode 100644 index 0000000..8b39b6d --- /dev/null +++ b/server/src/middleware/rateLimitKyc.test.ts @@ -0,0 +1,73 @@ +import type { Request, Response, NextFunction } from "express"; +import { pool } from "../db/pool"; +import { KYC_MAX_SUBMISSIONS, rateLimitKyc } from "./rateLimitKyc"; + +jest.mock("../db/pool", () => ({ + pool: { connect: jest.fn() }, +})); + +describe("rateLimitKyc", () => { + beforeEach(() => jest.clearAllMocks()); + + it("rejects an over-limit submission with 429 and Retry-After", async () => { + const oldest = new Date(Date.now() - 1_000).toISOString(); + const query = jest.fn(async (statement: string) => { + if (statement.includes("SELECT COUNT(*)")) { + return { rows: [{ attempts: String(KYC_MAX_SUBMISSIONS + 1), oldest }] }; + } + return { rows: [], rowCount: 1 }; + }); + const release = jest.fn(); + (pool.connect as jest.Mock).mockResolvedValue({ query, release }); + + const req = { + ip: "203.0.113.10", + user: { sub: "00000000-0000-4000-8000-000000000123" }, + } as unknown as Request; + const json = jest.fn(); + const status = jest.fn().mockReturnValue({ json }); + const res = { + setHeader: jest.fn(), + status, + } as unknown as Response; + const next = jest.fn() as NextFunction; + + await rateLimitKyc(req, res, next); + + expect(status).toHaveBeenCalledWith(429); + expect(json).toHaveBeenCalledWith(expect.objectContaining({ error: expect.stringContaining("Too many KYC submissions") })); + expect(res.setHeader).toHaveBeenCalledWith("X-RateLimit-Limit", String(KYC_MAX_SUBMISSIONS)); + expect(res.setHeader).toHaveBeenCalledWith("Retry-After", expect.any(String)); + expect(next).not.toHaveBeenCalled(); + expect(release).toHaveBeenCalledTimes(1); + }); + + it("uses the request IP when no authenticated user ID is available", async () => { + const query = jest.fn(async (statement: string) => { + if (statement.includes("SELECT COUNT(*)")) { + return { rows: [{ attempts: "1", oldest: new Date().toISOString() }] }; + } + return { rows: [], rowCount: 1 }; + }); + (pool.connect as jest.Mock).mockResolvedValue({ query, release: jest.fn() }); + + const req = { + ip: "203.0.113.20", + socket: { remoteAddress: "203.0.113.20" }, + } as unknown as Request; + const res = { + setHeader: jest.fn(), + status: jest.fn().mockReturnThis(), + json: jest.fn(), + } as unknown as Response; + const next = jest.fn() as NextFunction; + + await rateLimitKyc(req, res, next); + + expect(query).toHaveBeenCalledWith( + expect.stringContaining("INSERT INTO kyc_submission_attempts"), + [null, "203.0.113.20"] + ); + expect(next).toHaveBeenCalledTimes(1); + }); +}); diff --git a/server/src/middleware/rateLimitKyc.ts b/server/src/middleware/rateLimitKyc.ts index a5e6317..ebbcc70 100644 --- a/server/src/middleware/rateLimitKyc.ts +++ b/server/src/middleware/rateLimitKyc.ts @@ -15,16 +15,21 @@ import { AuthenticatedRequest } from "./authenticate"; * DB-backed (not in-memory) means the limit holds across restarts and * multiple server instances. * - * Keyed on the authenticated user's ID (not phone/IP) since this endpoint is - * already behind `authenticate` — the caller's identity is exactly what - * should be rate limited. + * Keyed on the authenticated user's ID, with the request IP as a fallback + * when this middleware is reused before authentication. */ /** KYC submissions allowed per user per window. */ -export const KYC_MAX_SUBMISSIONS = 3; +function positiveInteger(value: string | undefined, fallback: number): number { + if (!value?.trim()) return fallback; + const parsed = Number(value); + return Number.isSafeInteger(parsed) && parsed > 0 ? parsed : fallback; +} + +export const KYC_MAX_SUBMISSIONS = positiveInteger(process.env["KYC_MAX_SUBMISSIONS"], 3); /** Window length in minutes. */ -export const KYC_WINDOW_MINUTES = 60; +export const KYC_WINDOW_MINUTES = positiveInteger(process.env["KYC_WINDOW_MINUTES"], 60); export interface KycRateLimitState { allowed: boolean; @@ -40,24 +45,32 @@ export interface KycRateLimitState { * is about to be rejected — otherwise a caller past the limit could keep * retrying for free and the window would never fill. */ -export async function checkKycRateLimit(userId: string): Promise { +export async function checkKycRateLimit( + userId: string | null, + ipAddress: string | null = null +): Promise { + if (!userId && !ipAddress) { + throw new Error("A user ID or IP address is required for KYC rate limiting."); + } const client = await pool.connect(); try { await client.query("BEGIN"); await client.query( - `INSERT INTO kyc_submission_attempts (user_id, requested_at) VALUES ($1, NOW())`, - [userId] + `INSERT INTO kyc_submission_attempts (user_id, ip_address, requested_at) + VALUES ($1::uuid, $2::inet, NOW())`, + [userId, userId ? null : ipAddress] ); const { rows } = await client.query<{ attempts: string; oldest: string | null }>( `SELECT COUNT(*)::text AS attempts, MIN(requested_at)::text AS oldest FROM kyc_submission_attempts - WHERE user_id = $1 - AND requested_at > NOW() - ($2 || ' minutes')::interval`, - [userId, String(KYC_WINDOW_MINUTES)] + WHERE (($1::uuid IS NOT NULL AND user_id = $1::uuid) + OR ($1::uuid IS NULL AND ip_address = $2::inet)) + AND requested_at > NOW() - ($3 || ' minutes')::interval`, + [userId, ipAddress, String(KYC_WINDOW_MINUTES)] ); await client.query("COMMIT"); @@ -102,16 +115,15 @@ export async function rateLimitKyc( next: NextFunction ): Promise { const userId = (req as AuthenticatedRequest).user?.sub; + const ipAddress = req.ip || req.socket.remoteAddress || null; - if (!userId) { - // authenticate should already have rejected this request; nothing to - // rate-limit without an identity. + if (!userId && !ipAddress) { next(); return; } try { - const state = await checkKycRateLimit(userId); + const state = await checkKycRateLimit(userId ?? null, ipAddress); res.setHeader("X-RateLimit-Limit", String(KYC_MAX_SUBMISSIONS)); res.setHeader("X-RateLimit-Remaining", String(state.remaining));