From db48155a7d2e08e2e120dc478bda622f3fadfc93 Mon Sep 17 00:00:00 2001 From: Karl Ludwig Weise Date: Thu, 2 Jul 2026 18:32:01 +0200 Subject: [PATCH] Throttle now applies to all tenants across a server --- src/lib/server/db/central-schema.ts | 2 - .../__tests__/appointment-service.test.ts | 7 +--- .../services/__tests__/user-service.test.ts | 38 +++++++++++++++++- .../server/services/appointment-service.ts | 4 +- src/lib/server/services/challenge-throttle.ts | 40 +++---------------- .../bootstrap-challenge/+server.ts | 6 +-- .../appointments/bootstrap-verify/+server.ts | 6 +-- .../[id]/appointments/challenge/+server.ts | 2 +- .../appointments/verify-challenge/+server.ts | 8 +--- src/server-hooks/rateLimitHandle.ts | 30 ++++++-------- 10 files changed, 64 insertions(+), 79 deletions(-) diff --git a/src/lib/server/db/central-schema.ts b/src/lib/server/db/central-schema.ts index c62ccb8..896b037 100644 --- a/src/lib/server/db/central-schema.ts +++ b/src/lib/server/db/central-schema.ts @@ -234,8 +234,6 @@ export const userInvite = pgTable( export const challengeThrottle = pgTable("challenge_throttle", { /** Primary key - identifier (email hash for PIN challenges, email for passkey challenges) */ id: text("id").primaryKey(), - /** Tenant ID for scoping throttles to single tenants to restrict global lock-out. Might be null for global throttles on administrative accounts */ - tenantId: uuid("tenant_id").references(() => tenant.id, { onDelete: "cascade" }), /** Number of failed attempts */ failedAttempts: integer("failed_attempts").default(0).notNull(), /** When the throttle was last updated */ diff --git a/src/lib/server/services/__tests__/appointment-service.test.ts b/src/lib/server/services/__tests__/appointment-service.test.ts index 19f8596..b4c1def 100644 --- a/src/lib/server/services/__tests__/appointment-service.test.ts +++ b/src/lib/server/services/__tests__/appointment-service.test.ts @@ -852,11 +852,7 @@ describe("AppointmentService", () => { ); expect(challengeStore.consume).toHaveBeenCalledWith("challenge-123", "tenant-123"); - expect(challengeThrottleService.clearThrottle).toHaveBeenCalledWith( - "email-hash-123", - "pin", - "tenant-123", - ); + expect(challengeThrottleService.clearThrottle).toHaveBeenCalledWith("email-hash-123", "pin"); }); it("should throw NotFoundError when challenge is not found", async () => { @@ -925,7 +921,6 @@ describe("AppointmentService", () => { expect(challengeThrottleService.recordFailedAttempt).toHaveBeenCalledWith( "email-hash-123", "pin", - "tenant-123", ); }); diff --git a/src/lib/server/services/__tests__/user-service.test.ts b/src/lib/server/services/__tests__/user-service.test.ts index 7518dbe..6ec81ed 100644 --- a/src/lib/server/services/__tests__/user-service.test.ts +++ b/src/lib/server/services/__tests__/user-service.test.ts @@ -6,12 +6,36 @@ import { NotFoundError, ValidationError } from "../../utils/errors"; vi.mock("../../db", () => ({ centralDb: { insert: vi.fn(), - select: vi.fn(), + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(), + })), + })), update: vi.fn(), delete: vi.fn(), transaction: vi.fn(), limit: vi.fn(), }, + db: { + insert: vi.fn(), + select: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + limit: vi.fn(), + })), + })), + })), + update: vi.fn(() => ({ + from: vi.fn(() => ({ + where: vi.fn(() => ({ + set: vi.fn(), + })), + })), + })), + delete: vi.fn(), + transaction: vi.fn(), + limit: vi.fn(), + }, })); // Mock the email service @@ -41,17 +65,19 @@ import { UserService } from "../user-service"; describe("UserService", () => { let mockCentralDb: any; + let mockDb: any; let mockUuidv7: any; let mockAddMinutes: any; let mockSendConfirmationEmail: any; let mockTenantAdminService: any; beforeEach(async () => { - vi.clearAllMocks(); + vi.resetAllMocks(); // Get mocked modules const dbModule = await vi.importMock("../../db"); mockCentralDb = dbModule.centralDb; + mockDb = dbModule.db; const uuidModule = await vi.importMock("uuidv7"); mockUuidv7 = uuidModule.uuidv7; @@ -226,6 +252,14 @@ describe("UserService", () => { execute: vi.fn().mockResolvedValue({ count: 1 }), }; + const mockInviteUpdateBuilder: any = { + set: vi.fn().mockReturnThis(), + where: vi.fn().mockReturnThis(), + returning: vi.fn().mockResolvedValue([{ id: "invite-123", used: true }]), + }; + mockInviteUpdateBuilder.then = (resolve: any) => resolve([{ id: "invite-123", used: true }]); + mockDb.update.mockReturnValue(mockInviteUpdateBuilder); + // First call for user lookup, second call for tenant admin count, third for total count mockCentralDb.select .mockReturnValueOnce(mockUserInviteBuilder) diff --git a/src/lib/server/services/appointment-service.ts b/src/lib/server/services/appointment-service.ts index 1b85cf5..872d467 100644 --- a/src/lib/server/services/appointment-service.ts +++ b/src/lib/server/services/appointment-service.ts @@ -1097,13 +1097,13 @@ export class AppointmentService { }); // Record failed attempt for throttling - await challengeThrottleService.recordFailedAttempt(emailHash, "pin", this.tenantId); + await challengeThrottleService.recordFailedAttempt(emailHash, "pin"); throw new ValidationError("Invalid challenge response"); } // Clear throttle on successful verification - await challengeThrottleService.clearThrottle(emailHash, "pin", this.tenantId); + await challengeThrottleService.clearThrottle(emailHash, "pin"); const db = await this.getDb(); diff --git a/src/lib/server/services/challenge-throttle.ts b/src/lib/server/services/challenge-throttle.ts index 130bd7f..275e6f5 100644 --- a/src/lib/server/services/challenge-throttle.ts +++ b/src/lib/server/services/challenge-throttle.ts @@ -53,23 +53,14 @@ class ChallengeThrottleService { * @param type - Type of challenge (pin or passkey) * @param tenantId - Tenant ID for scoping the throttle check */ - async checkThrottle( - identifier: string, - type: ThrottleType, - tenantId?: string, - ): Promise { + async checkThrottle(identifier: string, type: ThrottleType): Promise { const now = new Date(); // Get throttle record from central DB const records = await centralDb .select() .from(challengeThrottle) - .where( - and( - eq(challengeThrottle.id, identifier), - tenantId ? eq(challengeThrottle.tenantId, tenantId) : undefined, - ), - ) + .where(and(eq(challengeThrottle.id, identifier))) .limit(1); if (records.length === 0) { @@ -82,14 +73,7 @@ class ChallengeThrottleService { // Check if throttle has expired if (now > record.resetAt) { // Throttle expired, clean up and allow - await centralDb - .delete(challengeThrottle) - .where( - and( - eq(challengeThrottle.id, identifier), - tenantId ? eq(challengeThrottle.tenantId, tenantId) : undefined, - ), - ); + await centralDb.delete(challengeThrottle).where(and(eq(challengeThrottle.id, identifier))); return { allowed: true, retryAfterMs: 0, failedAttempts: 0 }; } @@ -123,11 +107,7 @@ class ChallengeThrottleService { * @param type - Type of challenge (pin or passkey) * @param tenantId - Tenant ID for scoping the throttle record */ - async recordFailedAttempt( - identifier: string, - type: ThrottleType, - tenantId?: string, - ): Promise { + async recordFailedAttempt(identifier: string, type: ThrottleType): Promise { const now = new Date(); const resetAt = new Date(now.getTime() + THROTTLE_RESET_DURATION_MS); @@ -138,7 +118,6 @@ class ChallengeThrottleService { failedAttempts: 1, lastAttemptAt: now, resetAt, - tenantId: tenantId, }) .onConflictDoUpdate({ target: challengeThrottle.id, @@ -160,15 +139,8 @@ class ChallengeThrottleService { * @param type - Type of challenge (pin or passkey) * @param tenantId - Tenant ID for scoping the throttle clearance */ - async clearThrottle(identifier: string, type: ThrottleType, tenantId?: string): Promise { - await centralDb - .delete(challengeThrottle) - .where( - and( - eq(challengeThrottle.id, identifier), - tenantId ? eq(challengeThrottle.tenantId, tenantId) : undefined, - ), - ); + async clearThrottle(identifier: string, type: ThrottleType): Promise { + await centralDb.delete(challengeThrottle).where(and(eq(challengeThrottle.id, identifier))); logger.debug(`Cleared ${type} challenge throttle`, { identifier: identifier.slice(0, 8), diff --git a/src/routes/api/tenants/[id]/appointments/bootstrap-challenge/+server.ts b/src/routes/api/tenants/[id]/appointments/bootstrap-challenge/+server.ts index 7891ce9..898dc60 100644 --- a/src/routes/api/tenants/[id]/appointments/bootstrap-challenge/+server.ts +++ b/src/routes/api/tenants/[id]/appointments/bootstrap-challenge/+server.ts @@ -107,11 +107,7 @@ export const POST: RequestHandler = async ({ request, params }) => { emailHash: body.emailHash, }); - const throttleResult = await challengeThrottleService.checkThrottle( - binding, - "passkey", - tenantId, - ); + const throttleResult = await challengeThrottleService.checkThrottle(binding, "passkey"); if (!throttleResult.allowed) { return json( { diff --git a/src/routes/api/tenants/[id]/appointments/bootstrap-verify/+server.ts b/src/routes/api/tenants/[id]/appointments/bootstrap-verify/+server.ts index 6e5285b..1c237f1 100644 --- a/src/routes/api/tenants/[id]/appointments/bootstrap-verify/+server.ts +++ b/src/routes/api/tenants/[id]/appointments/bootstrap-verify/+server.ts @@ -124,7 +124,7 @@ export const POST: RequestHandler = async ({ request, params }) => { } if (storedChallenge.emailHash !== binding) { - await challengeThrottleService.recordFailedAttempt(binding, "passkey", tenantId); + await challengeThrottleService.recordFailedAttempt(binding, "passkey"); throw new ValidationError("Invalid bootstrap challenge binding"); } @@ -136,11 +136,11 @@ export const POST: RequestHandler = async ({ request, params }) => { }); if (!matchesPowDifficulty(digest, NEW_CLIENT_BOOTSTRAP_DIFFICULTY)) { - await challengeThrottleService.recordFailedAttempt(binding, "passkey", tenantId); + await challengeThrottleService.recordFailedAttempt(binding, "passkey"); throw new ValidationError("Invalid bootstrap proof of work"); } - await challengeThrottleService.clearThrottle(binding, "passkey", tenantId); + await challengeThrottleService.clearThrottle(binding, "passkey"); const bookingAccessToken = await generateNewClientBootstrapToken({ tenantId, diff --git a/src/routes/api/tenants/[id]/appointments/challenge/+server.ts b/src/routes/api/tenants/[id]/appointments/challenge/+server.ts index 134b196..7ec1eab 100644 --- a/src/routes/api/tenants/[id]/appointments/challenge/+server.ts +++ b/src/routes/api/tenants/[id]/appointments/challenge/+server.ts @@ -139,7 +139,7 @@ export const POST: RequestHandler = async ({ request, params }) => { const { emailHash } = requestSchema.parse(body); // Check throttling - const throttleResult = await challengeThrottleService.checkThrottle(emailHash, "pin", tenantId); + const throttleResult = await challengeThrottleService.checkThrottle(emailHash, "pin"); if (!throttleResult.allowed) { logger.warn("PIN challenge throttled", { diff --git a/src/routes/api/tenants/[id]/appointments/verify-challenge/+server.ts b/src/routes/api/tenants/[id]/appointments/verify-challenge/+server.ts index e141130..9e42657 100644 --- a/src/routes/api/tenants/[id]/appointments/verify-challenge/+server.ts +++ b/src/routes/api/tenants/[id]/appointments/verify-challenge/+server.ts @@ -169,11 +169,7 @@ export const POST: RequestHandler = async ({ request, params }) => { }); // Record failed attempt for throttling - await challengeThrottleService.recordFailedAttempt( - storedChallenge.emailHash, - "pin", - tenantId, - ); + await challengeThrottleService.recordFailedAttempt(storedChallenge.emailHash, "pin"); throw new ValidationError("Invalid challenge response"); } @@ -213,7 +209,7 @@ export const POST: RequestHandler = async ({ request, params }) => { }); // Clear throttle on successful verification - await challengeThrottleService.clearThrottle(storedChallenge.emailHash, "pin", tenantId); + await challengeThrottleService.clearThrottle(storedChallenge.emailHash, "pin"); const response: ChallengeVerificationResponse = { valid: true, diff --git a/src/server-hooks/rateLimitHandle.ts b/src/server-hooks/rateLimitHandle.ts index c18cb3b..976a701 100644 --- a/src/server-hooks/rateLimitHandle.ts +++ b/src/server-hooks/rateLimitHandle.ts @@ -1,4 +1,4 @@ -import { type Handle } from "@sveltejs/kit"; +import { type Handle, type RequestEvent } from "@sveltejs/kit"; /** In-memory store for rate limiting records per client IP */ const rateLimitStore = new Map(); @@ -12,25 +12,20 @@ export const RATE_LIMIT_MAX_REQUESTS = 20; * Extracts the client IP address from request headers * * Checks headers in order of preference: - * 1. x-forwarded-for (takes first IP from comma-separated list) - * 2. x-real-ip - * 3. Returns 'unknown' if no IP found + * 1. Returns IP + * 2. Returns 'unknown' if no IP found * - * @param {Request} request - The incoming HTTP request + * @param {RequestEvent} event - The incoming HTTP request event * @returns {string} The client IP address or 'unknown' */ -function getClientIP(request: Request): string { - const forwarded = request.headers.get("x-forwarded-for"); - if (forwarded) { - return forwarded.split(",")[0].trim(); +function getClientIP(event: RequestEvent): string { + try { + return event.getClientAddress(); + } catch { + // Thrown by adapter-node if behind a proxy without XFF_DEPTH configured, + // or if the platform can't determine an address. + return "unknown"; } - - const realIP = request.headers.get("x-real-ip"); - if (realIP) { - return realIP; - } - - return "unknown"; } /** @@ -83,8 +78,7 @@ setInterval(() => { * @returns {Promise} The response with applied headers and rate limiting */ export const rateLimitHandle: Handle = async ({ event, resolve }) => { - const { request } = event; - const clientIP = getClientIP(request); + const clientIP = getClientIP(event); if (isRateLimited(clientIP)) { return new Response("Too Many Requests", {