From 5e245f28afb5234b1f7f01f651f03aef82e5dbe6 Mon Sep 17 00:00:00 2001 From: naftis Date: Wed, 30 Oct 2024 16:15:23 +0200 Subject: [PATCH] fix: ratelimiting using 'username' as key instead of the actual username --- packages/gateway/src/rate-limit.test.ts | 55 ++++++++++++++++++++++++- packages/gateway/src/rate-limit.ts | 9 ++-- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/packages/gateway/src/rate-limit.test.ts b/packages/gateway/src/rate-limit.test.ts index 89aa2dbafe..e91bceff75 100644 --- a/packages/gateway/src/rate-limit.test.ts +++ b/packages/gateway/src/rate-limit.test.ts @@ -14,7 +14,6 @@ import { resolvers as locationRootResolvers } from '@gateway/features/location/r import * as fetchAny from 'jest-fetch-mock' import * as jwt from 'jsonwebtoken' import { readFileSync } from 'fs' -// eslint-disable-next-line import/no-relative-parent-imports import { startContainer, stopContainer, @@ -22,6 +21,7 @@ import { } from './utils/redis-test-utils' import { StartedTestContainer } from 'testcontainers' import { savedAdministrativeLocation } from '@opencrvs/commons/fixtures' +import { createServer } from '@gateway/server' const fetch = fetchAny as any const resolvers = rootResolvers as any @@ -239,4 +239,57 @@ describe('Rate limit', () => { return expect(() => Promise.all(resolverCalls)).not.toThrowError() }) + + it.only('handles multiple users authenticating with different usernames', async () => { + const server = await createServer() + + // okay to go through 10 times + for (let i = 1; i <= 10; i++) { + const res = await server.app.inject({ + method: 'POST', + url: '/auth/authenticate', + payload: { + username: 'test.user', + password: 'test' + } + }) + + expect((res.result as any).statusCode).not.toBe(402) + } + + // should return 402 Forbidden + const res = await server.app.inject({ + method: 'POST', + url: '/auth/authenticate', + payload: { + username: 'test.user', + password: 'test' + } + }) + expect((res.result as any).statusCode).toBe(402) + + // okay to go through 10 times with a different username + for (let i = 1; i <= 10; i++) { + const res = await server.app.inject({ + method: 'POST', + url: '/auth/authenticate', + payload: { + username: 'test.user2', + password: 'test' + } + }) + expect((res.result as any).statusCode).not.toBe(402) + } + + // should return 402 Forbidden + const res2 = await server.app.inject({ + method: 'POST', + url: '/auth/authenticate', + payload: { + username: 'test.user2', + password: 'test' + } + }) + expect((res2.result as any).statusCode).toBe(402) + }) }) diff --git a/packages/gateway/src/rate-limit.ts b/packages/gateway/src/rate-limit.ts index ddce825a08..00267e25cb 100644 --- a/packages/gateway/src/rate-limit.ts +++ b/packages/gateway/src/rate-limit.ts @@ -105,14 +105,17 @@ export const rateLimitedRoute = const key = pathOptionsForKey!.find( (path) => get(payload, path) !== undefined ) + const value = payload[key!] - if (!key) { - throw new Error("Couldn't find a rate limiting key in payload") + if (!value) { + throw new Error( + "Couldn't find the value for a rate limiting key in payload" + ) } return withRateLimit( { - key: `${key}:${route}`, + key: `${value}:${route}`, requestsPerMinute }, fn