From 3297486e313ef652b4b26c58d3d1d31354e9a6c2 Mon Sep 17 00:00:00 2001 From: forehalo Date: Thu, 25 Apr 2024 09:45:30 +0000 Subject: [PATCH] fix(server): skip throttle for currentUser (#6700) --- .../backend/server/src/core/auth/resolver.ts | 9 +--- .../backend/server/src/fundamentals/index.ts | 2 +- .../src/fundamentals/throttler/decorators.ts | 7 +-- .../src/fundamentals/throttler/index.ts | 4 +- .../server/tests/nestjs/throttler.spec.ts | 44 +++++++++++++++---- 5 files changed, 45 insertions(+), 21 deletions(-) diff --git a/packages/backend/server/src/core/auth/resolver.ts b/packages/backend/server/src/core/auth/resolver.ts index b347ca1f28..baa32a7dec 100644 --- a/packages/backend/server/src/core/auth/resolver.ts +++ b/packages/backend/server/src/core/auth/resolver.ts @@ -12,7 +12,7 @@ import { } from '@nestjs/graphql'; import type { Request, Response } from 'express'; -import { Config, Throttle } from '../../fundamentals'; +import { Config, SkipThrottle, Throttle } from '../../fundamentals'; import { UserService } from '../user'; import { UserType } from '../user/types'; import { validators } from '../utils/validators'; @@ -33,12 +33,6 @@ export class ClientTokenType { sessionToken?: string; } -/** - * Auth resolver - * Token rate limit: 20 req/m - * Sign up/in rate limit: 10 req/m - * Other rate limit: 5 req/m - */ @Throttle('strict') @Resolver(() => UserType) export class AuthResolver { @@ -49,6 +43,7 @@ export class AuthResolver { private readonly token: TokenService ) {} + @SkipThrottle() @Public() @Query(() => UserType, { name: 'currentUser', diff --git a/packages/backend/server/src/fundamentals/index.ts b/packages/backend/server/src/fundamentals/index.ts index 5abf98febb..5060d35432 100644 --- a/packages/backend/server/src/fundamentals/index.ts +++ b/packages/backend/server/src/fundamentals/index.ts @@ -27,7 +27,7 @@ export { export type { PrismaTransaction } from './prisma'; export * from './storage'; export { type StorageProvider, StorageProviderFactory } from './storage'; -export { CloudThrottlerGuard, Throttle } from './throttler'; +export { CloudThrottlerGuard, SkipThrottle, Throttle } from './throttler'; export { getRequestFromHost, getRequestResponseFromContext, diff --git a/packages/backend/server/src/fundamentals/throttler/decorators.ts b/packages/backend/server/src/fundamentals/throttler/decorators.ts index 1baa7d9dd0..742a32d729 100644 --- a/packages/backend/server/src/fundamentals/throttler/decorators.ts +++ b/packages/backend/server/src/fundamentals/throttler/decorators.ts @@ -1,7 +1,7 @@ import { applyDecorators, SetMetadata } from '@nestjs/common'; import { SkipThrottle, Throttle as RawThrottle } from '@nestjs/throttler'; -export type Throttlers = 'default' | 'strict'; +export type Throttlers = 'default' | 'strict' | 'authenticated'; export const THROTTLER_PROTECTED = 'affine_throttler:protected'; /** @@ -10,8 +10,9 @@ export const THROTTLER_PROTECTED = 'affine_throttler:protected'; * If a Controller or Query do not protected behind a Throttler, * it will never be rate limited. * - * - Ease: 120 calls within 60 seconds - * - Strict: 10 calls within 60 seconds + * - default: 120 calls within 60 seconds + * - strict: 10 calls within 60 seconds + * - authenticated: no rate limit for authenticated users, apply [default] throttler for unauthenticated users * * @example * diff --git a/packages/backend/server/src/fundamentals/throttler/index.ts b/packages/backend/server/src/fundamentals/throttler/index.ts index a75490f093..f15f408c12 100644 --- a/packages/backend/server/src/fundamentals/throttler/index.ts +++ b/packages/backend/server/src/fundamentals/throttler/index.ts @@ -166,10 +166,12 @@ export class CloudThrottlerGuard extends ThrottlerGuard { } getSpecifiedThrottler(context: ExecutionContext) { - return this.reflector.getAllAndOverride( + const throttler = this.reflector.getAllAndOverride( THROTTLER_PROTECTED, [context.getHandler(), context.getClass()] ); + + return throttler === 'authenticated' ? undefined : throttler; } } diff --git a/packages/backend/server/tests/nestjs/throttler.spec.ts b/packages/backend/server/tests/nestjs/throttler.spec.ts index 77921961f5..2c113b1fcc 100644 --- a/packages/backend/server/tests/nestjs/throttler.spec.ts +++ b/packages/backend/server/tests/nestjs/throttler.spec.ts @@ -48,6 +48,13 @@ class ThrottledController { return 'default3'; } + @Public() + @Get('/authenticated') + @Throttle('authenticated') + none() { + return 'none'; + } + @Throttle('strict') @Get('/strict') strict() { @@ -156,7 +163,6 @@ test('should use default throttler for unauthenticated user when not specified', t.is(headers.limit, '120'); t.is(headers.remaining, '119'); - t.regex(headers.reset, /59|60/); }); test('should skip throttler for unauthenticated user when specified', async t => { @@ -192,7 +198,6 @@ test('should use specified throttler for unauthenticated user', async t => { t.is(headers.limit, '20'); t.is(headers.remaining, '19'); - t.regex(headers.reset, /59|60/); }); // ==== authenticated user visits ==== @@ -223,7 +228,6 @@ test('should use default throttler for authenticated user when not specified', a t.is(headers.limit, '120'); t.is(headers.remaining, '119'); - t.regex(headers.reset, /59|60/); }); test('should use same throttler for multiple routes', async t => { @@ -238,7 +242,6 @@ test('should use same throttler for multiple routes', async t => { t.is(headers.limit, '120'); t.is(headers.remaining, '119'); - t.regex(headers.reset, /59|60/); res = await request(app.getHttpServer()) .get('/throttled/default2') @@ -263,7 +266,6 @@ test('should use different throttler if specified', async t => { t.is(headers.limit, '120'); t.is(headers.remaining, '119'); - t.regex(headers.reset, /59|60/); res = await request(app.getHttpServer()) .get('/throttled/default3') @@ -274,7 +276,34 @@ test('should use different throttler if specified', async t => { t.is(headers.limit, '10'); t.is(headers.remaining, '9'); - t.regex(headers.reset, /59|60/); +}); + +test('should skip throttler for authenticated if `authenticated` throttler used', async t => { + const { app, cookie } = t.context; + + const res = await request(app.getHttpServer()) + .get('/throttled/authenticated') + .set('Cookie', cookie) + .expect(200); + + const headers = rateLimitHeaders(res); + + t.is(headers.limit, undefined!); + t.is(headers.remaining, undefined!); + t.is(headers.reset, undefined!); +}); + +test('should apply `default` throttler for authenticated user if `authenticated` throttler used', async t => { + const { app } = t.context; + + const res = await request(app.getHttpServer()) + .get('/throttled/authenticated') + .expect(200); + + const headers = rateLimitHeaders(res); + + t.is(headers.limit, '120'); + t.is(headers.remaining, '119'); }); test('should skip throttler for authenticated user when specified', async t => { @@ -304,7 +333,6 @@ test('should use specified throttler for authenticated user', async t => { t.is(headers.limit, '20'); t.is(headers.remaining, '19'); - t.regex(headers.reset, /59|60/); }); test('should separate anonymous and authenticated user throttlers', async t => { @@ -323,9 +351,7 @@ test('should separate anonymous and authenticated user throttlers', async t => { t.is(authenticatedResHeaders.limit, '120'); t.is(authenticatedResHeaders.remaining, '119'); - t.regex(authenticatedResHeaders.reset, /59|60/); t.is(unauthenticatedResHeaders.limit, '120'); t.is(unauthenticatedResHeaders.remaining, '119'); - t.regex(unauthenticatedResHeaders.reset, /59|60/); });