From 64ca334ca2d32f4555c769c9672d1ef595d91ddd Mon Sep 17 00:00:00 2001 From: Richard Fontein <32132657+rifont@users.noreply.github.com> Date: Wed, 20 Dec 2023 09:17:03 +0000 Subject: [PATCH] feat(api): Return api service level in rate limit headers (#4976) * feat(shared): Add api rate limit configuration types and cosntants * feat(api): Add get-api-rate-limit-configuration use-case * refactor(api): Move get-api-rate-limit use case to rate-limiting module per PR feedback * fix(api): Add module import for get-api-rate-limit use case * fix(api): Remove redundant imports * feat(application-generic): Expose SADD and EVAL redis operations on cache service * feat(api): Add evaluate-api-rate-limit use cases * chore: Run pnpm install to resolve merge conflicts on lockfile * fix(api): Add better logging for rate limit evaluation error * test(api): Add tests for evaluate-api-rate-limit use-case * docs(api): Fix rate limit mock comment * docs(api): Add further clarification to rate limit evaluation mock * docs(api): Further mock redis eval clarification * fix(shared): Adjust typing of IApiRateLimitConfiguration to allow arbitrary configuration types * fix(shared): Rename refillInterval to windowDuration to align with rate limiting RFC nomenclature * feat(api): Expose refillRate, windowDuration, and burstLimit from evaluate-api-rate-limit use case * fix(api): Remove redundant ICacheService import * feat(api): Extract evaluate-api-rate-limit use-case typings * feat(api): Add ApiRateLimitGuard and related ThrottleCategory decorator * chore(api): Add @nestjs/throttler library * fix(api): Use enum for rate limit header keys * feat(api): Add runtime feature toggle and fix dependency injection on rate limit guard * feat(api): Add @nestjs/throttler and rate limit guard to RateLimitingModule * fix(api): Add rate limiting imports * feat(api): Extract default reflector metadata for rate limit guard * fix(application-generic): Use correct method params for eval, add mock client tests * chore(application-generic): Tidy up cache typings * feat(api): Return algorithm in rate limit execution * feat(api): Add support for bulk operation cost in rate limit execution * feat(api): Add variable cost rate limiting algorithm * feat(shared): Add default bulk cost for api rate limiting * feat(shared): Add bulk cost typing for api rate limiting * refactor(shared): Move rate limit flag from system-critical to feature-flags * refactor(application-generic): Convert rate limit flag from system-critical to feature-flags * feat(application-generic): Create custom provider for rate limit feature flag * chore(api, shared, application-generic): Rename api rate limiting feature flag for consistency * fix(app-generic): Rename api rate limiting file * feat(api): Add variable request type handling to throttler * feat(api): Add support use-cases for rate limit evaluation * chore(api): Revert accidental changes max-rate-limit use case * feat(api): Add bulk support to evaluate rate limit use case * chore(api, dal, shared): Rename API Rate Limiting enums and typings for consistency * fix(shared): Export rate limit types from index * fix(api): Add get-max-rate-limit command back * fix(api): Update rate limit use case imports * fix(app-gen): Fix mock cache SADD implementation to match return val of set operations * fix(api): Fix import for get max rate limit use-case test * fix(api): Remove unused refillRate result * fix(api): Use correct cost enum for evaluate rate limit use-case * fix(api): Tidy up throttler guard * feat(api): Modify token bucket algorithm to allow for variable cost * chore(api): Tidy up algo logic * chore(api): Fix comments on algo * feat(api): Add local caching back to rate limit algo * chore(api): Refactor token bucket algorithm into separate use-case * feat(api): Add modified token bucket rate limiter * fix(api): Make cache client adapter static and add tests * fix(api): Fix use-case tests * fix(api, shared): Use snake_case enum values for rate limit configuration * fix(api): Fix evaluate rate limit import * fix(api): Use enum value * fix(api): Fix bad enum reference * fix(api): Fix test describe naming * test(api): Add throttler guard test * test(api): Add more tests * feat(api): Add rate limit configuration environment variables * test(api): Fix burst limit calculation * fix(api): Make the createLimiter method functional * fix(api): Invalidate max rate limit cache entities when loading max limit config module * feat(app-gen): Add key builder for service config and tidy up base keygen locations * fix(api): Fix get-max-ratelimit usecase CachedEntity builder to use correct environment ID on * fix(api): Specify default limits on max-rate-limit class attribute * feat(api): Add environment and organization repository methods for api rate limits * test(api): Add test rate limit controller * feat(api): Add rate limit guard to app module * feat(api): Add strong typing to environment variables * feat(api): Convert rate limit guard to interceptor to provide auth context * test(api): Add rate limit guard tests * fix(api): Fix env vars in tests * fix(api): Consolidate algorithm into use-case * test(api): Rename rate limit test spec to e2e * fix(api): remove redundant e2e gitkeep * test(api): Add tests for variable-cost token bucket rate limiter * fix(api): Revert addition of variable-cost token bucket * fix(api): Address PR review comments * fix(api): Remove e2e tests for rate limiting temporarily * fix(api): Remove unused test case and import * fix(api): Revert package.json change * Revert "fix(api): Remove unused test case and import" This reverts commit b8ce7a4b7de36038c87220519a20860caedbac79. * Revert "fix(api): Remove e2e tests for rate limiting temporarily" This reverts commit 5e91dc3c296a33b4b1e8ff5061124cb31a6b17df. * fix(api): Remove failing throttler tests and modify expected reset * test(api): Add throttled request test * fix(api): Add cspell definitions for rate limiting * fix(api): Use rate limiter before idempotency interceptor * fix(api): Add comment on nestjs throttler config * test(api): update test * test(api): Add tolerance for throttled count * test(api): Fix tolerance for upstash * fix(api): Typo * fix(dal): Fix updateApiRateLimits return value * fix(api): Auto-generate name prefix * fix(api): Use invalidate by key instead of query * fix(api): Remove redundant import * fix(api): Fix cache invalidation test * feat(api): Apply rate limit category and cost decorators to api controllers and methods * fix(api): Fix typo * fix(api): Add separate before statements for unit and e2e tests * test(api): Use regex for variable policy header values * fix(api): Toggle launch darkly off to allow test to define FF state * fix(api): Fix launch darkly toggle off * feat(api): Add eslint rule to block @nestjs swagger ApiResponse decorator imports * feat(api): Add types and constants for common responses and headers * feat(api): Add swagger overrides for response decorators * feat(api): Add new openapi spec endpoint, add more api info * feat(api): Apply ApiCommonHeaders to all relevant controllers * fix(api): Remove unused import and update cspell * fix(api): Remove redundant satisfies * feat(api): Add header type generics * test(api): Add tests for http header enum types * fix(api): Fix spellcheck error * fix(api): Use compound words in spellcheck * fix(api): Increase error tolerance on rate limiting to reduce test flakiness * fix(api): Fix import * fix(api): Use non-ambient type for header enum type check function declaration * fix(api): Update swagger endpoint * fix(api): Rename swagger file in github action * refactor(infra): Rename Swagger to OpenAPI * refactor(infra): Rename again * refactor(infra): Rename gh action from swagger to openapi * docs(api): Add comment on swagger decorators * docs(api): Improve description * docs(api): Use informative description * docs(api): Formalise documentation link * docs(api): Fix description for api key scheme * refactor(api): Move swagger setup to separate module * fix(api): Remove duplication decorator * feat(api): Add descriptions and external reference documentation for each API tag * chore(infra): Use Spectral for API validation and style linting * feat(api): Add Spectral API linting * fix(api): Enable PORT override for package script * fix(api): Fix inconsistent endpoint param and override faulty endpoint * feat(api): Add passport apikey strategy * fix(api): Remove redundant check from roles guard * fix(api): Remove redundant authentication check from root env guard * fix(api): Remove redundant authentication check from session decorator * fix(api): Use user payload in throttler and idempotency interceptors * fix(api): Remove redundant logger assign in jwt strategy * fix(api): Remove redundant logger assign in trigger use-case * fix(app-gen): Update auth service to return expected auth validation user entities * fix(api): Update throttler guard to read from req.user * fix(app-gen): Remove log * fix(api): Add headerapikey to cspell * fix(api): Remove redundant log * revert(api): Auth guard authscheme checl * fix(api): Auth scheme resolution * Update apps/api/src/app/shared/framework/idempotency.interceptor.ts * feat(api): Return api service level in rate limit headers * fix(api): Remove custom from enum * fix(api): Revert accidental change * fix(api): Add catch for apiheader strategy * fix(api): Typesafe user handling in auth guard * feat(app-gen): Add custom user attributes to all logs * fix(api): Return false on error * revert(app-gen): Revert changes to trigger use-case logger assign * chore(api, app-gen, shared): Create reusable enums for auth scheme and auth strategy * chore(api, app-gen): Rename JwtAuthGuard to UserAuthGuard * fix(infra): Fix cspell * fix(api): Fix getApiKeyUser signature to match cache build signature * fix(api): Handle undefined user * test(api): Add auth guard tests * fix(api): More reuse of http header enum * fix(api): Header keys import * Revert "fix(api): Revert accidental change" This reverts commit 48a68055116e763cd66328290c48f6e8b23a6f95. * Revert "fix(api): Remove custom from enum" This reverts commit 81ef5ac665bfd16882d38f686b7f8bac54dd81da. * revert rate limit change * Revert "revert rate limit change" This reverts commit c50321b481befc41026cfee48f5dce5a1584edfb. * Revert "Revert "fix(api): Remove custom from enum"" This reverts commit 8762ba2388e97996a761b463440a1ec4936865d7. * Revert "Revert "fix(api): Revert accidental change"" This reverts commit 139e860b064897bd43a3e82b661813db1eceb4d2. * feat(api, app-gen): Add instrumentation for critical paths * chore(app-gen): Fix rxjs type annotation on auth guard * feat(api): Instrument throttler guard canActivate * fix(api): Remove unecessary type annotation * Update package.json --- .../guards/throttler.guard.e2e.ts | 4 + .../rate-limiting/guards/throttler.guard.ts | 35 +++++- .../evaluate-api-rate-limit.spec.ts | 6 +- .../evaluate-api-rate-limit.types.ts | 4 + .../evaluate-api-rate-limit.usecase.ts | 3 +- .../get-api-rate-limit-maximum.dto.ts | 8 ++ .../get-api-rate-limit-maximum.spec.ts | 102 +++++++++++++----- .../get-api-rate-limit-maximum.usecase.ts | 17 +-- ...te-limit-service-maximum-config.usecase.ts | 3 +- .../framework/idempotency.interceptor.ts | 3 +- packages/application-generic/package.json | 3 +- .../application-generic/src/logging/index.ts | 1 + .../src/services/auth/user.auth.guard.ts | 9 ++ .../src/services/feature-flags.service.ts | 2 + pnpm-lock.yaml | 3 + 15 files changed, 162 insertions(+), 41 deletions(-) create mode 100644 apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.dto.ts diff --git a/apps/api/src/app/rate-limiting/guards/throttler.guard.e2e.ts b/apps/api/src/app/rate-limiting/guards/throttler.guard.e2e.ts index 343e34072a1..2ae7ad114dd 100644 --- a/apps/api/src/app/rate-limiting/guards/throttler.guard.e2e.ts +++ b/apps/api/src/app/rate-limiting/guards/throttler.guard.e2e.ts @@ -89,6 +89,10 @@ describe('API Rate Limiting', () => { { name: 'comment', expectedRegex: `comment="[a-zA-Z ]*"` }, { name: 'category', expectedRegex: `category="(${Object.values(ApiRateLimitCategoryEnum).join('|')})"` }, { name: 'cost', expectedRegex: `cost="(${Object.values(ApiRateLimitCostEnum).join('|')})"` }, + { + name: 'serviceLevel', + expectedRegex: `serviceLevel="[a-zA-Z]*"`, + }, ]; testParams.forEach(({ name, expectedRegex }) => { diff --git a/apps/api/src/app/rate-limiting/guards/throttler.guard.ts b/apps/api/src/app/rate-limiting/guards/throttler.guard.ts index a4b595fea20..672e9d67553 100644 --- a/apps/api/src/app/rate-limiting/guards/throttler.guard.ts +++ b/apps/api/src/app/rate-limiting/guards/throttler.guard.ts @@ -8,9 +8,10 @@ import { ThrottlerStorage, } from '@nestjs/throttler'; import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common'; +import { Observable } from 'rxjs'; import { EvaluateApiRateLimit, EvaluateApiRateLimitCommand } from '../usecases/evaluate-api-rate-limit'; import { Reflector } from '@nestjs/core'; -import { FeatureFlagCommand, GetIsApiRateLimitingEnabled } from '@novu/application-generic'; +import { FeatureFlagCommand, GetIsApiRateLimitingEnabled, Instrument } from '@novu/application-generic'; import { ApiRateLimitCategoryEnum, ApiRateLimitCostEnum, ApiAuthSchemeEnum, IJwtPayload } from '@novu/shared'; import { ThrottlerCost, ThrottlerCategory } from './throttler.decorator'; import { HttpRequestHeaderKeysEnum, HttpResponseHeaderKeysEnum } from '../../shared/framework/types'; @@ -51,6 +52,11 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter } } + @Instrument() + canActivate(context: ExecutionContext): Promise { + return super.canActivate(context); + } + protected async shouldSkip(context: ExecutionContext): Promise { const isAllowedAuthScheme = this.isAllowedAuthScheme(context); if (!isAllowedAuthScheme) { @@ -88,7 +94,7 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter // Return early if the current user agent should be ignored. if (Array.isArray(ignoreUserAgents)) { for (const pattern of ignoreUserAgents) { - if (pattern.test(req.headers[HttpRequestHeaderKeysEnum.AUTHORIZATION.toLowerCase()])) { + if (pattern.test(req.headers[HttpRequestHeaderKeysEnum.USER_AGENT.toLowerCase()])) { return true; } } @@ -103,7 +109,7 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter const { organizationId, environmentId } = this.getReqUser(context); - const { success, limit, remaining, reset, windowDuration, burstLimit, algorithm } = + const { success, limit, remaining, reset, windowDuration, burstLimit, algorithm, apiServiceLevel } = await this.evaluateApiRateLimit.execute( EvaluateApiRateLimitCommand.create({ organizationId, @@ -120,8 +126,25 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter res.header(HttpResponseHeaderKeysEnum.RATELIMIT_RESET, secondsToReset); res.header( HttpResponseHeaderKeysEnum.RATELIMIT_POLICY, - this.createPolicyHeader(limit, windowDuration, burstLimit, algorithm, apiRateLimitCategory, apiRateLimitCost) + this.createPolicyHeader( + limit, + windowDuration, + burstLimit, + algorithm, + apiRateLimitCategory, + apiRateLimitCost, + apiServiceLevel + ) ); + res.rateLimitPolicy = { + limit, + windowDuration, + burstLimit, + algorithm, + apiRateLimitCategory, + apiRateLimitCost, + apiServiceLevel, + }; if (success) { return true; @@ -137,7 +160,8 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter burstLimit: number, algorithm: string, apiRateLimitCategory: ApiRateLimitCategoryEnum, - apiRateLimitCost: ApiRateLimitCostEnum + apiRateLimitCost: ApiRateLimitCostEnum, + apiServiceLevel: string ): string { const policyMap = { w: windowDuration, @@ -145,6 +169,7 @@ export class ApiRateLimitInterceptor extends ThrottlerGuard implements NestInter comment: `"${algorithm}"`, category: `"${apiRateLimitCategory}"`, cost: `"${apiRateLimitCost}"`, + serviceLevel: `"${apiServiceLevel}"`, }; const policy = Object.entries(policyMap).reduce((acc, [key, value]) => { return `${acc};${key}=${value}`; diff --git a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.spec.ts b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.spec.ts index 0bd3b75f0af..2aa6d1539dc 100644 --- a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.spec.ts +++ b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.spec.ts @@ -5,6 +5,7 @@ import { ApiRateLimitAlgorithmEnum, ApiRateLimitCategoryEnum, ApiRateLimitCostEnum, + ApiServiceLevelEnum, IApiRateLimitAlgorithm, IApiRateLimitCost, } from '@novu/shared'; @@ -22,6 +23,7 @@ const mockApiRateLimitAlgorithm: IApiRateLimitAlgorithm = { [ApiRateLimitAlgorithmEnum.WINDOW_DURATION]: 2, }; const mockApiRateLimitCost = ApiRateLimitCostEnum.SINGLE; +const mockApiServiceLevel = ApiServiceLevelEnum.FREE; const mockCost = 1; const mockApiRateLimitCostConfig: Partial = { [mockApiRateLimitCost]: mockCost, @@ -59,7 +61,9 @@ describe('EvaluateApiRateLimit', async () => { getApiRateLimitCostConfig = moduleRef.get(GetApiRateLimitCostConfig); evaluateTokenBucketRateLimit = moduleRef.get(EvaluateTokenBucketRateLimit); - getApiRateLimitMaximumStub = sinon.stub(getApiRateLimitMaximum, 'execute').resolves(mockMaxLimit); + getApiRateLimitMaximumStub = sinon + .stub(getApiRateLimitMaximum, 'execute') + .resolves([mockMaxLimit, mockApiServiceLevel]); getApiRateLimitAlgorithmConfigStub = sinon .stub(getApiRateLimitAlgorithmConfig, 'default') .value(mockApiRateLimitAlgorithm); diff --git a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.types.ts b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.types.ts index 66107393cad..8fee719fd16 100644 --- a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.types.ts +++ b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.types.ts @@ -35,4 +35,8 @@ export type EvaluateApiRateLimitResponseDto = { * The cost of the request. */ cost: number; + /** + * The API service level used to evaluate the request. + */ + apiServiceLevel: string; }; diff --git a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.usecase.ts b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.usecase.ts index 2a870ba3ae6..92671651905 100644 --- a/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.usecase.ts +++ b/apps/api/src/app/rate-limiting/usecases/evaluate-api-rate-limit/evaluate-api-rate-limit.usecase.ts @@ -20,7 +20,7 @@ export class EvaluateApiRateLimit { @InstrumentUsecase() async execute(command: EvaluateApiRateLimitCommand): Promise { - const maxLimitPerSecond = await this.getApiRateLimitMaximum.execute( + const [maxLimitPerSecond, apiServiceLevel] = await this.getApiRateLimitMaximum.execute( GetApiRateLimitMaximumCommand.create({ apiRateLimitCategory: command.apiRateLimitCategory, environmentId: command.environmentId, @@ -60,6 +60,7 @@ export class EvaluateApiRateLimit { refillRate, algorithm: this.evaluateTokenBucketRateLimit.algorithm, cost, + apiServiceLevel, }; } diff --git a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.dto.ts b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.dto.ts new file mode 100644 index 00000000000..08e734d8888 --- /dev/null +++ b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.dto.ts @@ -0,0 +1,8 @@ +import { ApiServiceLevelEnum } from '@novu/shared'; + +export const CUSTOM_API_SERVICE_LEVEL = 'custom'; + +export type ApiServiceLevel = ApiServiceLevelEnum | typeof CUSTOM_API_SERVICE_LEVEL; + +// Array type to keep the cached entity as small as possible for more performant caching +export type GetApiRateLimitMaximumDto = [apiRateLimitMaximum: number, apiServiceLevel: ApiServiceLevel]; diff --git a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.spec.ts b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.spec.ts index c2fb580dbdd..21b8fee5a11 100644 --- a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.spec.ts +++ b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.spec.ts @@ -9,6 +9,7 @@ import { GetApiRateLimitMaximum, GetApiRateLimitMaximumCommand } from './index'; import { SharedModule } from '../../../shared/shared.module'; import { GetApiRateLimitServiceMaximumConfig } from '../get-api-rate-limit-service-maximum-config'; import { RateLimitingModule } from '../../rate-limiting.module'; +import { CUSTOM_API_SERVICE_LEVEL } from './get-api-rate-limit-maximum.dto'; const mockDefaultApiRateLimits = { [ApiServiceLevelEnum.FREE]: { @@ -91,7 +92,7 @@ describe('GetApiRateLimitMaximum', async () => { }); it('should return api rate limit for the category set on environment', async () => { - const rateLimit = await useCase.execute( + const [rateLimit] = await useCase.execute( GetApiRateLimitMaximumCommand.create({ organizationId: session.organization._id, environmentId: session.environment._id, @@ -101,6 +102,18 @@ describe('GetApiRateLimitMaximum', async () => { expect(rateLimit).to.equal(mockGlobalLimit); }); + + it('should return api service level of CUSTOM', async () => { + const [, apiServiceLevel] = await useCase.execute( + GetApiRateLimitMaximumCommand.create({ + organizationId: session.organization._id, + environmentId: session.environment._id, + apiRateLimitCategory: mockApiRateLimitCategory, + }) + ); + + expect(apiServiceLevel).to.equal(CUSTOM_API_SERVICE_LEVEL); + }); }); describe('Environment DOES NOT have rate limits specified', () => { @@ -112,39 +125,78 @@ describe('GetApiRateLimitMaximum', async () => { }); }); - it('should return default api rate limit for the organizations apiServiceLevel when apiServiceLevel IS set on organization', async () => { + describe('Organization DOES have api service level specified', () => { const mockApiServiceLevel = ApiServiceLevelEnum.FREE; - findOneOrganizationStub.resolves({ - apiServiceLevel: mockApiServiceLevel, + + beforeEach(() => { + findOneOrganizationStub.resolves({ + apiServiceLevel: mockApiServiceLevel, + }); }); - const defaultApiRateLimit = mockDefaultApiRateLimits[mockApiServiceLevel][mockApiRateLimitCategory]; - const rateLimit = await useCase.execute( - GetApiRateLimitMaximumCommand.create({ - organizationId: session.organization._id, - environmentId: session.environment._id, - apiRateLimitCategory: mockApiRateLimitCategory, - }) - ); + it('should return default api rate limit for the organizations apiServiceLevel when apiServiceLevel IS set on organization', async () => { + const defaultApiRateLimit = mockDefaultApiRateLimits[mockApiServiceLevel][mockApiRateLimitCategory]; + + const [rateLimit] = await useCase.execute( + GetApiRateLimitMaximumCommand.create({ + organizationId: session.organization._id, + environmentId: session.environment._id, + apiRateLimitCategory: mockApiRateLimitCategory, + }) + ); + + expect(rateLimit).to.equal(defaultApiRateLimit); + }); + + it('should return the api service level set on organization when apiServiceLevel IS set on organization', async () => { + const mockApiServiceLevel = ApiServiceLevelEnum.FREE; + + const [, apiServiceLevel] = await useCase.execute( + GetApiRateLimitMaximumCommand.create({ + organizationId: session.organization._id, + environmentId: session.environment._id, + apiRateLimitCategory: mockApiRateLimitCategory, + }) + ); - expect(rateLimit).to.equal(defaultApiRateLimit); + expect(apiServiceLevel).to.equal(mockApiServiceLevel); + }); }); - it('should return default api rate limit for the UNLIMITED serice level when apiServiceLevel IS NOT set on organization', async () => { - findOneOrganizationStub.resolves({ - apiServiceLevel: undefined, + describe('Organization DOES NOT have api service level specified', () => { + beforeEach(() => { + findOneOrganizationStub.resolves({ + apiServiceLevel: undefined, + }); }); - const defaultApiRateLimit = mockDefaultApiRateLimits[ApiServiceLevelEnum.UNLIMITED][mockApiRateLimitCategory]; - const rateLimit = await useCase.execute( - GetApiRateLimitMaximumCommand.create({ - organizationId: session.organization._id, - environmentId: session.environment._id, - apiRateLimitCategory: mockApiRateLimitCategory, - }) - ); + it('should return default api rate limit for the UNLIMITED service level when apiServiceLevel IS NOT set on organization', async () => { + const defaultApiRateLimit = mockDefaultApiRateLimits[ApiServiceLevelEnum.UNLIMITED][mockApiRateLimitCategory]; - expect(rateLimit).to.equal(defaultApiRateLimit); + const [rateLimit] = await useCase.execute( + GetApiRateLimitMaximumCommand.create({ + organizationId: session.organization._id, + environmentId: session.environment._id, + apiRateLimitCategory: mockApiRateLimitCategory, + }) + ); + + expect(rateLimit).to.equal(defaultApiRateLimit); + }); + + it('should return the default api service level of UNLIMITED when apiServiceLevel IS NOT set on organization', async () => { + const defaultApiServiceLevel = ApiServiceLevelEnum.UNLIMITED; + + const [, apiServiceLevel] = await useCase.execute( + GetApiRateLimitMaximumCommand.create({ + organizationId: session.organization._id, + environmentId: session.environment._id, + apiRateLimitCategory: mockApiRateLimitCategory, + }) + ); + + expect(apiServiceLevel).to.equal(defaultApiServiceLevel); + }); }); it('should throw an error when the organization is not found', async () => { diff --git a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.usecase.ts b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.usecase.ts index 2d1fa007154..a907666194c 100644 --- a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.usecase.ts +++ b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-maximum/get-api-rate-limit-maximum.usecase.ts @@ -1,9 +1,10 @@ import { Injectable, InternalServerErrorException, Logger } from '@nestjs/common'; import { EnvironmentRepository, OrganizationRepository } from '@novu/dal'; -import { buildMaximumApiRateLimitKey, CachedEntity } from '@novu/application-generic'; +import { buildMaximumApiRateLimitKey, CachedEntity, InstrumentUsecase } from '@novu/application-generic'; import { ApiRateLimitCategoryEnum, ApiServiceLevelEnum, IApiRateLimitMaximum } from '@novu/shared'; import { GetApiRateLimitMaximumCommand } from './get-api-rate-limit-maximum.command'; import { GetApiRateLimitServiceMaximumConfig } from '../get-api-rate-limit-service-maximum-config'; +import { ApiServiceLevel, CUSTOM_API_SERVICE_LEVEL, GetApiRateLimitMaximumDto } from './get-api-rate-limit-maximum.dto'; const LOG_CONTEXT = 'GetApiRateLimit'; @@ -15,7 +16,8 @@ export class GetApiRateLimitMaximum { private getDefaultApiRateLimits: GetApiRateLimitServiceMaximumConfig ) {} - async execute(command: GetApiRateLimitMaximumCommand): Promise { + @InstrumentUsecase() + async execute(command: GetApiRateLimitMaximumCommand): Promise { return await this.getApiRateLimit({ apiRateLimitCategory: command.apiRateLimitCategory, _environmentId: command.environmentId, @@ -38,7 +40,7 @@ export class GetApiRateLimitMaximum { apiRateLimitCategory: ApiRateLimitCategoryEnum; _environmentId: string; _organizationId: string; - }): Promise { + }): Promise { const environment = await this.environmentRepository.findOne({ _id: _environmentId }); if (!environment) { @@ -48,7 +50,9 @@ export class GetApiRateLimitMaximum { } let apiRateLimits: IApiRateLimitMaximum; + let apiServiceLevel: ApiServiceLevel; if (environment.apiRateLimits) { + apiServiceLevel = CUSTOM_API_SERVICE_LEVEL; apiRateLimits = environment.apiRateLimits; } else { const organization = await this.organizationRepository.findOne({ _id: _organizationId }); @@ -60,15 +64,16 @@ export class GetApiRateLimitMaximum { } if (organization.apiServiceLevel) { - apiRateLimits = this.getDefaultApiRateLimits.default[organization.apiServiceLevel]; + apiServiceLevel = organization.apiServiceLevel; } else { // TODO: NV-3067 - Remove this once all organizations have a service level - apiRateLimits = this.getDefaultApiRateLimits.default[ApiServiceLevelEnum.UNLIMITED]; + apiServiceLevel = ApiServiceLevelEnum.UNLIMITED; } + apiRateLimits = this.getDefaultApiRateLimits.default[apiServiceLevel]; } const apiRateLimit = apiRateLimits[apiRateLimitCategory]; - return apiRateLimit; + return [apiRateLimit, apiServiceLevel]; } } diff --git a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-service-maximum-config/get-api-rate-limit-service-maximum-config.usecase.ts b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-service-maximum-config/get-api-rate-limit-service-maximum-config.usecase.ts index 9ea495c3074..df219183fe0 100644 --- a/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-service-maximum-config/get-api-rate-limit-service-maximum-config.usecase.ts +++ b/apps/api/src/app/rate-limiting/usecases/get-api-rate-limit-service-maximum-config/get-api-rate-limit-service-maximum-config.usecase.ts @@ -1,4 +1,4 @@ -import { Injectable, OnModuleInit } from '@nestjs/common'; +import { Injectable, Logger, OnModuleInit } from '@nestjs/common'; import { ApiRateLimitCategoryEnum, ApiRateLimitServiceMaximumEnvVarFormat, @@ -37,6 +37,7 @@ export class GetApiRateLimitServiceMaximumConfig implements OnModuleInit { const newHash = this.getConfigHash(newDefault); if (previousHash !== newHash) { + Logger.log(`Updating API Rate Limit Maximum config cache`, GetApiRateLimitServiceMaximumConfig.name); await this.cacheService.set(cacheKey, newHash); this.invalidateCache.invalidateByKey({ diff --git a/apps/api/src/app/shared/framework/idempotency.interceptor.ts b/apps/api/src/app/shared/framework/idempotency.interceptor.ts index f85ba9f9c8b..d622305d867 100644 --- a/apps/api/src/app/shared/framework/idempotency.interceptor.ts +++ b/apps/api/src/app/shared/framework/idempotency.interceptor.ts @@ -11,7 +11,7 @@ import { BadRequestException, ConflictException, } from '@nestjs/common'; -import { CacheService } from '@novu/application-generic'; +import { CacheService, Instrument } from '@novu/application-generic'; import { Observable, of, throwError } from 'rxjs'; import { catchError, map } from 'rxjs/operators'; import { createHash } from 'crypto'; @@ -34,6 +34,7 @@ enum ReqStatusEnum { export class IdempotencyInterceptor implements NestInterceptor { constructor(private readonly cacheService: CacheService) {} + @Instrument() async intercept(context: ExecutionContext, next: CallHandler): Promise> { const request = context.switchToHttp().getRequest(); const idempotencyKey = this.getIdempotencyKey(context); diff --git a/packages/application-generic/package.json b/packages/application-generic/package.json index a67c4c5291b..8e983d13d90 100644 --- a/packages/application-generic/package.json +++ b/packages/application-generic/package.json @@ -133,6 +133,7 @@ "redlock": "4.2.0", "reflect-metadata": "^0.1.13", "rrule": "^2.7.2", + "rxjs": "7.8.1", "@nestjs/passport": "^10.0.1", "jsonwebtoken": "9.0.0", "i18next": "^23.7.6" @@ -191,4 +192,4 @@ "**/*.spec.js" ] } -} \ No newline at end of file +} diff --git a/packages/application-generic/src/logging/index.ts b/packages/application-generic/src/logging/index.ts index 51231b548d6..a5d43a5d55e 100644 --- a/packages/application-generic/src/logging/index.ts +++ b/packages/application-generic/src/logging/index.ts @@ -131,6 +131,7 @@ export function createNestLoggingModuleOptions( organizationId: req?.user?.organizationId || null, }, authScheme: req?.authScheme, + rateLimitPolicy: res?.rateLimitPolicy, }), }, }; diff --git a/packages/application-generic/src/services/auth/user.auth.guard.ts b/packages/application-generic/src/services/auth/user.auth.guard.ts index fac43035e78..306db0b36bf 100644 --- a/packages/application-generic/src/services/auth/user.auth.guard.ts +++ b/packages/application-generic/src/services/auth/user.auth.guard.ts @@ -10,6 +10,8 @@ import { IJwtPayload, PassportStrategyEnum, } from '@novu/shared'; +import { Instrument } from '../../instrumentation'; +import { Observable } from 'rxjs'; type SentryUser = { id: string; @@ -29,6 +31,13 @@ export class UserAuthGuard extends AuthGuard([ super(); } + @Instrument() + canActivate( + context: ExecutionContext + ): boolean | Promise | Observable { + return super.canActivate(context); + } + getAuthenticateOptions(context: ExecutionContext): IAuthModuleOptions { const request = context.switchToHttp().getRequest(); const authorizationHeader = request.headers.authorization; diff --git a/packages/application-generic/src/services/feature-flags.service.ts b/packages/application-generic/src/services/feature-flags.service.ts index fe4efd0176d..a4a461c2ba0 100644 --- a/packages/application-generic/src/services/feature-flags.service.ts +++ b/packages/application-generic/src/services/feature-flags.service.ts @@ -1,4 +1,5 @@ import { Injectable, Logger } from '@nestjs/common'; +import { Instrument } from '../instrumentation'; import { LaunchDarklyService } from './launch-darkly.service'; @@ -112,6 +113,7 @@ export class FeatureFlagsService { * - Default value with the value provided by Novu's environment variable if set * - Default value with the value provided by the hardcoded fallback */ + @Instrument() public async get( key: FeatureFlagsKeysEnum, defaultValue: T, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 69aad93cb1b..249a47df2b9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -2717,6 +2717,9 @@ importers: rrule: specifier: ^2.7.2 version: 2.7.2 + rxjs: + specifier: 7.8.1 + version: 7.8.1 optionalDependencies: '@taskforcesh/bullmq-pro': specifier: 5.1.14