diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-base-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-base-query-runner.service.ts index baf012cd4e..69b5036483 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-base-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-base-query-runner.service.ts @@ -31,6 +31,8 @@ import { WorkspacePreQueryHookPayload } from 'src/engine/api/graphql/workspace-q import { WorkspaceQueryHookService } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service'; import { ApiKeyRoleService } from 'src/engine/core-modules/api-key/api-key-role.service'; import { AuthContext } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { MetricsService } from 'src/engine/core-modules/metrics/metrics.service'; +import { MetricsKeys } from 'src/engine/core-modules/metrics/types/metrics-keys.type'; import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { type PermissionFlagType } from 'src/engine/metadata-modules/permissions/constants/permission-flag-type.constants'; @@ -77,6 +79,8 @@ export abstract class CommonBaseQueryRunnerService< protected readonly throttlerService: ThrottlerService; @Inject() protected readonly twentyConfigService: TwentyConfigService; + @Inject() + protected readonly metricsService: MetricsService; protected abstract readonly operationName: CommonQueryNames; @@ -94,7 +98,7 @@ export abstract class CommonBaseQueryRunnerService< ); } - await this.throttleQueryExecution(authContext.workspace.id); + await this.throttleQueryExecution(authContext); await this.validate(args, queryRunnerContext); @@ -331,35 +335,50 @@ export abstract class CommonBaseQueryRunnerService< }; } - private async throttleQueryExecution(workspaceId: string) { - const shortConfig = { - key: `api:throttler:${workspaceId}-short-limit`, - maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_SHORT_LIMIT'), - timeWindow: this.twentyConfigService.get( - 'API_RATE_LIMITING_SHORT_TTL_IN_MS', - ), - }; + private async throttleQueryExecution(authContext: WorkspaceAuthContext) { + try { + if (!isDefined(authContext.apiKey)) return; - const longConfig = { - key: `api:throttler:${workspaceId}-long-limit`, - maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'), - timeWindow: this.twentyConfigService.get( - 'API_RATE_LIMITING_LONG_TTL_IN_MS', - ), - }; + const workspaceId = authContext.workspace.id; - await this.throttlerService.tokenBucketThrottle( - shortConfig.key, - 1, - shortConfig.maxTokens, - shortConfig.timeWindow, - ); + const shortConfig = { + key: `api:throttler:${workspaceId}-short-limit`, + maxTokens: this.twentyConfigService.get( + 'API_RATE_LIMITING_SHORT_LIMIT', + ), + timeWindow: this.twentyConfigService.get( + 'API_RATE_LIMITING_SHORT_TTL_IN_MS', + ), + }; - await this.throttlerService.tokenBucketThrottle( - longConfig.key, - 1, - longConfig.maxTokens, - longConfig.timeWindow, - ); + const longConfig = { + key: `api:throttler:${workspaceId}-long-limit`, + maxTokens: this.twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'), + timeWindow: this.twentyConfigService.get( + 'API_RATE_LIMITING_LONG_TTL_IN_MS', + ), + }; + + await this.throttlerService.tokenBucketThrottleOrThrow( + shortConfig.key, + 1, + shortConfig.maxTokens, + shortConfig.timeWindow, + ); + + await this.throttlerService.tokenBucketThrottleOrThrow( + longConfig.key, + 1, + longConfig.maxTokens, + longConfig.timeWindow, + ); + } catch (error) { + await this.metricsService.incrementCounter({ + key: MetricsKeys.CommonApiQueryRateLimited, + shouldStoreInCache: false, + }); + + throw error; + } } } diff --git a/packages/twenty-server/src/engine/api/common/core-common-api.module.ts b/packages/twenty-server/src/engine/api/common/core-common-api.module.ts index 23caec262b..76a89f25c2 100644 --- a/packages/twenty-server/src/engine/api/common/core-common-api.module.ts +++ b/packages/twenty-server/src/engine/api/common/core-common-api.module.ts @@ -12,6 +12,7 @@ import { WorkspaceQueryHookModule } from 'src/engine/api/graphql/workspace-query import { WorkspaceQueryRunnerModule } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-runner.module'; import { ApiKeyModule } from 'src/engine/core-modules/api-key/api-key.module'; import { FileModule } from 'src/engine/core-modules/file/file.module'; +import { MetricsModule } from 'src/engine/core-modules/metrics/metrics.module'; import { ThrottlerModule } from 'src/engine/core-modules/throttler/throttler.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { RoleTargetsEntity } from 'src/engine/metadata-modules/role/role-targets.entity'; @@ -35,6 +36,7 @@ import { WorkspacePermissionsCacheModule } from 'src/engine/metadata-modules/wor ViewFilterModule, ViewFilterGroupModule, ThrottlerModule, + MetricsModule, ], providers: [ ProcessNestedRelationsHelper, diff --git a/packages/twenty-server/src/engine/api/graphql/metadata.module-factory.ts b/packages/twenty-server/src/engine/api/graphql/metadata.module-factory.ts index d51c209d85..b2f7c5a3c4 100644 --- a/packages/twenty-server/src/engine/api/graphql/metadata.module-factory.ts +++ b/packages/twenty-server/src/engine/api/graphql/metadata.module-factory.ts @@ -4,7 +4,6 @@ import GraphQLJSON from 'graphql-type-json'; import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface'; import { useCachedMetadata } from 'src/engine/api/graphql/graphql-config/hooks/use-cached-metadata'; -import { useThrottler } from 'src/engine/api/graphql/graphql-config/hooks/use-throttler'; import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module'; import { type CacheStorageService } from 'src/engine/core-modules/cache-storage/services/cache-storage.service'; import { type ExceptionHandlerService } from 'src/engine/core-modules/exception-handler/exception-handler.service'; @@ -31,13 +30,6 @@ export const metadataModuleFactory = async ( }, resolvers: { JSON: GraphQLJSON }, plugins: [ - useThrottler({ - ttl: twentyConfigService.get('API_RATE_LIMITING_LONG_TTL_IN_MS') / 1000, - limit: twentyConfigService.get('API_RATE_LIMITING_LONG_LIMIT'), - identifyFn: (context) => { - return context.req.user?.id ?? context.req.ip ?? 'anonymous'; - }, - }), useGraphQLErrorHandlerHook({ metricsService: metricsService, exceptionHandlerService, diff --git a/packages/twenty-server/src/engine/core-modules/metrics/types/metrics-keys.type.ts b/packages/twenty-server/src/engine/core-modules/metrics/types/metrics-keys.type.ts index f8551a3736..64c097708a 100644 --- a/packages/twenty-server/src/engine/core-modules/metrics/types/metrics-keys.type.ts +++ b/packages/twenty-server/src/engine/core-modules/metrics/types/metrics-keys.type.ts @@ -29,4 +29,5 @@ export enum MetricsKeys { CronJobDeletedWorkspace = 'cron-job/deleted-workspace', JobWebhookCallCompleted = 'job/webhook-call-completed', SignUpSuccess = 'sign-up/success', + CommonApiQueryRateLimited = 'common-api-query/rate-limited', } diff --git a/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.spec.ts b/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.spec.ts index 9edb348c09..e830546285 100644 --- a/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.spec.ts @@ -31,7 +31,7 @@ describe('ThrottlerService', () => { jest.clearAllMocks(); }); - describe('tokenBucketThrottle', () => { + describe('tokenBucketThrottleOrThrow', () => { const key = 'test-throttle-key'; const maxTokens = 100; const timeWindow = 1000; // 1 second @@ -39,7 +39,7 @@ describe('ThrottlerService', () => { it('should allow request when tokens are available (first request)', async () => { cacheStorageService.get.mockResolvedValue(null); - await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow); + await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow); expect(cacheStorageService.get).toHaveBeenCalledWith(key); expect(cacheStorageService.set).toHaveBeenCalledWith( @@ -60,7 +60,7 @@ describe('ThrottlerService', () => { lastRefillAt: now - 100, }); - await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow); + await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow); expect(cacheStorageService.get).toHaveBeenCalledWith(key); expect(cacheStorageService.set).toHaveBeenCalledWith( @@ -82,11 +82,11 @@ describe('ThrottlerService', () => { }); await expect( - service.tokenBucketThrottle(key, 10, maxTokens, timeWindow), + service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow), ).rejects.toThrow(ThrottlerException); await expect( - service.tokenBucketThrottle(key, 10, maxTokens, timeWindow), + service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow), ).rejects.toThrow('Limit reached'); expect(cacheStorageService.set).not.toHaveBeenCalled(); @@ -105,7 +105,7 @@ describe('ThrottlerService', () => { jest.spyOn(Date, 'now').mockReturnValue(now); - await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow); + await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow); expect(cacheStorageService.set).toHaveBeenCalledWith( key, @@ -130,7 +130,7 @@ describe('ThrottlerService', () => { jest.spyOn(Date, 'now').mockReturnValue(now); - await service.tokenBucketThrottle(key, 10, maxTokens, timeWindow); + await service.tokenBucketThrottleOrThrow(key, 10, maxTokens, timeWindow); // Available tokens should be capped at maxTokens (100) expect(cacheStorageService.set).toHaveBeenCalledWith( diff --git a/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.ts b/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.ts index f56885b426..acb948118d 100644 --- a/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.ts +++ b/packages/twenty-server/src/engine/core-modules/throttler/throttler.service.ts @@ -28,7 +28,7 @@ export class ThrottlerService { await this.cacheStorage.set(key, currentCount + 1, ttl); } - async tokenBucketThrottle( + async tokenBucketThrottleOrThrow( key: string, tokensToConsume: number, maxTokens: number, diff --git a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts index 1881fb9fba..2efbbcf83a 100644 --- a/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts +++ b/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts @@ -975,7 +975,7 @@ export class ConfigVariables { type: ConfigVariableType.NUMBER, }) @CastToPositiveNumber() - API_RATE_LIMITING_LONG_LIMIT = 1000; + API_RATE_LIMITING_LONG_LIMIT = 100; @ConfigVariablesMetadata({ group: ConfigVariablesGroup.SSL,