mirror of
https://github.com/open-reception/appointment-booking-software.git
synced 2026-08-17 21:25:52 +02:00
Throttle now applies to all tenants across a server
This commit is contained in:
@@ -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 */
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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();
|
||||
|
||||
|
||||
@@ -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<ThrottleResult> {
|
||||
async checkThrottle(identifier: string, type: ThrottleType): Promise<ThrottleResult> {
|
||||
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<void> {
|
||||
async recordFailedAttempt(identifier: string, type: ThrottleType): Promise<void> {
|
||||
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<void> {
|
||||
await centralDb
|
||||
.delete(challengeThrottle)
|
||||
.where(
|
||||
and(
|
||||
eq(challengeThrottle.id, identifier),
|
||||
tenantId ? eq(challengeThrottle.tenantId, tenantId) : undefined,
|
||||
),
|
||||
);
|
||||
async clearThrottle(identifier: string, type: ThrottleType): Promise<void> {
|
||||
await centralDb.delete(challengeThrottle).where(and(eq(challengeThrottle.id, identifier)));
|
||||
|
||||
logger.debug(`Cleared ${type} challenge throttle`, {
|
||||
identifier: identifier.slice(0, 8),
|
||||
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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", {
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, { count: number; resetTime: number }>();
|
||||
@@ -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<Response>} 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", {
|
||||
|
||||
Reference in New Issue
Block a user