From aecc16f04f3d6bfc4d38ff5b243d429f33bf753a Mon Sep 17 00:00:00 2001 From: Iain Sproat <68657+iainsproat@users.noreply.github.com> Date: Sun, 23 Feb 2025 13:16:45 +0000 Subject: [PATCH] handle rate limit error when registering users --- packages/server/modules/auth/strategies/local.ts | 4 +++- packages/server/modules/core/rest/ratelimiter.ts | 15 +++++++++++++-- .../server/modules/core/services/ratelimiter.ts | 13 ------------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/server/modules/auth/strategies/local.ts b/packages/server/modules/auth/strategies/local.ts index 8ab18a339..832d346ed 100644 --- a/packages/server/modules/auth/strategies/local.ts +++ b/packages/server/modules/auth/strategies/local.ts @@ -107,7 +107,7 @@ const localStrategyBuilderFactory = const rateLimitResult = await deps.getRateLimitResult('USER_CREATE', source) if (isRateLimitBreached(rateLimitResult)) { addRateLimitHeadersToResponse(res, rateLimitResult) - return next(new RateLimitError(rateLimitResult)) + throw new RateLimitError(rateLimitResult, 'Created too many new users') } } @@ -158,6 +158,8 @@ const localStrategyBuilderFactory = } catch (err) { const e = ensureError(err, 'Unexpected issue occured while registering') switch (e.constructor) { + case RateLimitError: + return res.status(429).send({ err: e.message }) case PasswordTooShortError: case UserInputError: case InviteNotFoundError: diff --git a/packages/server/modules/core/rest/ratelimiter.ts b/packages/server/modules/core/rest/ratelimiter.ts index fcb7848c3..6a70fb6f5 100644 --- a/packages/server/modules/core/rest/ratelimiter.ts +++ b/packages/server/modules/core/rest/ratelimiter.ts @@ -1,8 +1,7 @@ -import type { RequestHandler, Response } from 'express' +import type { Request, RequestHandler, Response } from 'express' import { getActionForPath, getRateLimitResult, - getSourceFromRequest, isRateLimitBreached, RATE_LIMITERS, type RateLimitBreached, @@ -12,6 +11,8 @@ import { isRateLimiterEnabled } from '@/modules/shared/helpers/envHelper' import { getRequestPath } from '@/modules/core/helpers/server' import { RateLimitError } from '@/modules/core/errors/ratelimit' import { ensureError } from '@speckle/shared' +import { getTokenFromRequest } from '@/modules/shared/middleware' +import { getIpFromRequest } from '@/modules/shared/utils/ip' export const createRateLimiterMiddleware = ( rateLimiterMapping: RateLimiterMapping = RATE_LIMITERS @@ -67,3 +68,13 @@ export const addRateLimitHeadersToResponse = ( ) res.setHeader('X-Speckle-Meditation', 'https://http.cat/429') } + +export const getSourceFromRequest = (req: Request): string => { + let source: string | null = + req?.context?.userId || + getTokenFromRequest(req)?.substring(10) || // token ID + getIpFromRequest(req) + + if (!source) source = 'unknown' + return source +} diff --git a/packages/server/modules/core/services/ratelimiter.ts b/packages/server/modules/core/services/ratelimiter.ts index 792ca436f..f4d8f401b 100644 --- a/packages/server/modules/core/services/ratelimiter.ts +++ b/packages/server/modules/core/services/ratelimiter.ts @@ -1,4 +1,3 @@ -import express from 'express' import { getRedisUrl, getIntFromEnv } from '@/modules/shared/helpers/envHelper' import { BurstyRateLimiter, @@ -8,10 +7,8 @@ import { RateLimiterRes } from 'rate-limiter-flexible' import { TIME } from '@speckle/shared' -import { getIpFromRequest } from '@/modules/shared/utils/ip' import { rateLimiterLogger } from '@/logging/logging' import { createRedisClient } from '@/modules/shared/redis/redis' -import { getTokenFromRequest } from '@/modules/shared/middleware' export interface RateLimitResult { isWithinLimits: boolean @@ -271,16 +268,6 @@ export const getActionForPath = (path: string, verb: string): RateLimitAction => return 'ALL_REQUESTS' } -export const getSourceFromRequest = (req: express.Request): string => { - let source: string | null = - req?.context?.userId || - getTokenFromRequest(req)?.substring(10) || // token ID - getIpFromRequest(req) - - if (!source) source = 'unknown' - return source -} - // we need to take the `BurstyRateLimiter` specific type because // its not considered as an RateLimiterAbstract in the rate-limiter-flexible package // This is just a rant comment, but why define the Abstract then if not