From 3be66f72df7aec6fc5b507e7de34bdd4a1b78542 Mon Sep 17 00:00:00 2001 From: Ho Ngoc Hai Date: Mon, 20 Apr 2026 08:31:26 +0700 Subject: [PATCH] feat(listings): rate limit feature-listing via @nestjs/throttler (TEC-2930) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wire ThrottlerModule to a Redis-backed storage (shared across API instances) using @nest-lab/throttler-storage-redis. - Add FeatureListingThrottlerGuard that tracks per-user when JWT is present, falling back to the real client IP behind the reverse proxy — keeps per-user and per-IP buckets independent. - Apply @Throttle({ default: { limit: 10, ttl: 60_000 } }) + the guard to POST /listings/:id/feature and document 429 in Swagger. - Integration test (feature-listing-throttle.integration.spec.ts) verifies: 10 reqs pass / 11th returns 429 with Retry-After, separate IPs keep their own quotas, and the tracker key logic. Co-Authored-By: Paperclip --- apps/api/package.json | 1 + apps/api/src/app.module.ts | 18 ++ ...ature-listing-throttle.integration.spec.ts | 199 ++++++++++++++++++ .../src/modules/listings/listings.module.ts | 4 + .../controllers/listings.controller.ts | 7 +- .../guards/feature-listing-throttler.guard.ts | 35 +++ .../modules/shared/infrastructure/index.ts | 1 + pnpm-lock.yaml | 21 ++ 8 files changed, 284 insertions(+), 2 deletions(-) create mode 100644 apps/api/src/modules/listings/__tests__/feature-listing-throttle.integration.spec.ts create mode 100644 apps/api/src/modules/shared/infrastructure/guards/feature-listing-throttler.guard.ts diff --git a/apps/api/package.json b/apps/api/package.json index 4432161..b644ef4 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -17,6 +17,7 @@ "@aws-sdk/client-s3": "^3.1026.0", "@aws-sdk/s3-request-presigner": "^3.1026.0", "@goodgo/mcp-servers": "workspace:*", + "@nest-lab/throttler-storage-redis": "^1.2.0", "@nestjs/bullmq": "^11.0.4", "@nestjs/common": "^11.0.0", "@nestjs/config": "^4.0.4", diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts index 7864cb6..97bcabc 100644 --- a/apps/api/src/app.module.ts +++ b/apps/api/src/app.module.ts @@ -1,3 +1,4 @@ +import { ThrottlerStorageRedisService } from '@nest-lab/throttler-storage-redis'; import { BullModule } from '@nestjs/bullmq'; import { type MiddlewareConsumer, Module, type NestModule, RequestMethod } from '@nestjs/common'; import { APP_FILTER, APP_GUARD, APP_INTERCEPTOR } from '@nestjs/core'; @@ -70,6 +71,8 @@ import { AppController } from './app.controller'; // ── Rate Limiting ── // Default: 60 requests per 60 seconds per IP // Override per-route with @Throttle() decorator + // Storage: Redis-backed sliding window so limits are shared across + // every API instance (required for TEC-2930 feature-listing throttling). ThrottlerModule.forRoot({ throttlers: [ { @@ -88,6 +91,21 @@ import { AppController } from './app.controller'; limit: process.env['NODE_ENV'] === 'test' || process.env['NODE_ENV'] === 'development' ? 10_000 : 20, }, ], + storage: new ThrottlerStorageRedisService({ + host: process.env['REDIS_HOST'] ?? 'localhost', + port: Number(process.env['REDIS_PORT'] ?? 6379), + password: process.env['REDIS_PASSWORD'] ?? undefined, + // Single retry per command + bounded reconnect backoff so a + // transient Redis blip cannot stall the request path. Behaviour + // matches RedisService for consistency. + maxRetriesPerRequest: 1, + enableReadyCheck: false, + lazyConnect: true, + retryStrategy(times: number): number { + return Math.min(times * 1000, 5000); + }, + keyPrefix: 'throttler:', + }), }), ], controllers: [AppController], diff --git a/apps/api/src/modules/listings/__tests__/feature-listing-throttle.integration.spec.ts b/apps/api/src/modules/listings/__tests__/feature-listing-throttle.integration.spec.ts new file mode 100644 index 0000000..d5007b1 --- /dev/null +++ b/apps/api/src/modules/listings/__tests__/feature-listing-throttle.integration.spec.ts @@ -0,0 +1,199 @@ +import { ThrottlerStorageRedisService } from '@nest-lab/throttler-storage-redis'; +import { Controller, type INestApplication, Post, UseGuards } from '@nestjs/common'; +import { Test, type TestingModule } from '@nestjs/testing'; +import { Throttle, ThrottlerModule } from '@nestjs/throttler'; +import Redis from 'ioredis'; +import request from 'supertest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import { FeatureListingThrottlerGuard } from '../../shared/infrastructure/guards/feature-listing-throttler.guard'; + +/** + * TEC-2930 — feature-listing throttle integration. + * + * Spins up a minimal Nest app whose only route mirrors the production + * `POST /listings/:id/feature` decoration: + * + * @UseGuards(FeatureListingThrottlerGuard) + * @Throttle({ default: { limit: 10, ttl: 60_000 } }) + * + * Backed by the same Redis-backed throttler storage as production. We do + * not boot the full ListingsModule — that would require Postgres, JWT, the + * payments stack, etc. The contract under test is purely "does the + * endpoint emit 429 after the configured limit, and are per-user and + * per-IP buckets kept separate?". + */ + +const REDIS_HOST = process.env['REDIS_HOST'] ?? 'localhost'; +const REDIS_PORT = Number(process.env['REDIS_PORT'] ?? 16379); +const REDIS_PASSWORD = process.env['REDIS_PASSWORD'] ?? 'goodgo-redis-dev'; + +@Controller('listings') +class StubListingsController { + @Post(':id/feature') + @Throttle({ default: { limit: 10, ttl: 60_000 } }) + @UseGuards(FeatureListingThrottlerGuard) + async featureListing(): Promise<{ ok: true }> { + return { ok: true }; + } +} + +async function flushTestKeys(redis: Redis): Promise { + const keys = await redis.keys('throttle-test:throttler:*'); + if (keys.length > 0) { + await redis.del(...keys); + } +} + +describe('Feature listing throttler (TEC-2930)', () => { + let app: INestApplication; + let redis: Redis; + let storage: ThrottlerStorageRedisService; + let redisAvailable = false; + + beforeAll(async () => { + redis = new Redis({ + host: REDIS_HOST, + port: REDIS_PORT, + password: REDIS_PASSWORD, + lazyConnect: true, + maxRetriesPerRequest: 1, + }); + + try { + await redis.connect(); + await redis.ping(); + redisAvailable = true; + } catch { + redisAvailable = false; + } + + if (!redisAvailable) { + // Skip the suite gracefully — we'll guard each test below. + return; + } + + storage = new ThrottlerStorageRedisService({ + host: REDIS_HOST, + port: REDIS_PORT, + password: REDIS_PASSWORD, + keyPrefix: 'throttle-test:throttler:', + maxRetriesPerRequest: 1, + }); + + const moduleRef: TestingModule = await Test.createTestingModule({ + imports: [ + ThrottlerModule.forRoot({ + throttlers: [{ name: 'default', ttl: 60_000, limit: 10 }], + storage, + }), + ], + controllers: [StubListingsController], + }).compile(); + + app = moduleRef.createNestApplication(); + await app.init(); + }); + + afterAll(async () => { + if (app) { + await app.close(); + } + if (storage?.redis && 'quit' in storage.redis) { + await (storage.redis as Redis).quit(); + } + if (redis.status !== 'end') { + await redis.quit(); + } + }); + + beforeEach(async () => { + if (redisAvailable) { + await flushTestKeys(redis); + } + }); + + it('allows the first 10 requests then returns 429', async () => { + if (!redisAvailable) { + console.warn('Skipping: Redis not reachable at', `${REDIS_HOST}:${REDIS_PORT}`); + return; + } + + const target = '/listings/abc-123/feature'; + + for (let i = 0; i < 10; i += 1) { + const res = await request(app.getHttpServer()) + .post(target) + .set('X-Forwarded-For', '203.0.113.10') + .send({}); + expect(res.status, `request ${i + 1} should pass`).toBe(201); + } + + const blocked = await request(app.getHttpServer()) + .post(target) + .set('X-Forwarded-For', '203.0.113.10') + .send({}); + expect(blocked.status).toBe(429); + // @nestjs/throttler exposes Retry-After when the limit is exceeded. + expect(blocked.headers['retry-after']).toBeDefined(); + }); + + it('keeps per-IP buckets independent', async () => { + if (!redisAvailable) return; + + const target = '/listings/abc-123/feature'; + + for (let i = 0; i < 10; i += 1) { + await request(app.getHttpServer()) + .post(target) + .set('X-Forwarded-For', '203.0.113.20') + .send({}) + .expect(201); + } + + // The first IP is now exhausted... + await request(app.getHttpServer()) + .post(target) + .set('X-Forwarded-For', '203.0.113.20') + .send({}) + .expect(429); + + // ...but a different IP still has its full quota. + await request(app.getHttpServer()) + .post(target) + .set('X-Forwarded-For', '198.51.100.42') + .send({}) + .expect(201); + }); + + it('separates buckets by tracker key (user vs ip)', async () => { + if (!redisAvailable) return; + + // Verify the guard's key derivation directly. Hitting the HTTP layer + // would require wiring JwtAuthGuard, which is out of scope for this + // unit-of-behaviour. We test the tracker contract instead. + const guard = new FeatureListingThrottlerGuard( + { throttlers: [{ name: 'default', ttl: 60_000, limit: 10 }] }, + storage, + // @ts-expect-error reflector is not used by getTracker + { getAllAndOverride: () => undefined }, + ); + + const trackerForUser = await (guard as unknown as { + getTracker: (req: unknown) => Promise; + }).getTracker({ + headers: {}, + ip: '127.0.0.1', + user: { sub: 'user-123', role: 'USER' }, + }); + expect(trackerForUser).toBe('user:user-123'); + + const trackerForAnon = await (guard as unknown as { + getTracker: (req: unknown) => Promise; + }).getTracker({ + headers: { 'x-forwarded-for': '203.0.113.55, 10.0.0.1' }, + ip: '127.0.0.1', + user: null, + }); + expect(trackerForAnon).toBe('ip:203.0.113.55'); + }); +}); diff --git a/apps/api/src/modules/listings/listings.module.ts b/apps/api/src/modules/listings/listings.module.ts index 3b61938..15d9209 100644 --- a/apps/api/src/modules/listings/listings.module.ts +++ b/apps/api/src/modules/listings/listings.module.ts @@ -1,6 +1,7 @@ import { Module } from '@nestjs/common'; import { CqrsModule } from '@nestjs/cqrs'; import { MulterModule } from '@nestjs/platform-express'; +import { FeatureListingThrottlerGuard } from '@modules/shared'; import { AdminFeatureListingHandler } from './application/commands/admin-feature-listing/admin-feature-listing.handler'; import { CreateListingHandler } from './application/commands/create-listing/create-listing.handler'; import { DeleteListingHandler } from './application/commands/delete-listing/delete-listing.handler'; @@ -79,6 +80,9 @@ const EventHandlers = [ // Cron FeaturedListingExpiryCronService, + + // Guards (per-route) + FeatureListingThrottlerGuard, ], exports: [LISTING_REPOSITORY, PROPERTY_REPOSITORY], }) diff --git a/apps/api/src/modules/listings/presentation/controllers/listings.controller.ts b/apps/api/src/modules/listings/presentation/controllers/listings.controller.ts index a0448e7..5d87876 100644 --- a/apps/api/src/modules/listings/presentation/controllers/listings.controller.ts +++ b/apps/api/src/modules/listings/presentation/controllers/listings.controller.ts @@ -25,10 +25,11 @@ import { ApiQuery, ApiParam, } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import type { Response } from 'express'; import * as QRCode from 'qrcode'; import { type JwtPayload, CurrentUser, Roles, JwtAuthGuard, RolesGuard } from '@modules/auth'; -import { NotFoundException, EndpointRateLimit, EndpointRateLimitGuard, FileValidationPipe, type UploadedFile as ValidatedFile } from '@modules/shared'; +import { NotFoundException, EndpointRateLimit, EndpointRateLimitGuard, FeatureListingThrottlerGuard, FileValidationPipe, type UploadedFile as ValidatedFile } from '@modules/shared'; import { RequireQuota, QuotaGuard } from '@modules/subscriptions'; import { CreateListingCommand } from '../../application/commands/create-listing/create-listing.command'; import type { CreateListingResult } from '../../application/commands/create-listing/create-listing.handler'; @@ -363,7 +364,9 @@ export class ListingsController { @ApiResponse({ status: 400, description: 'Invalid package or listing not ACTIVE' }) @ApiResponse({ status: 401, description: 'Unauthorized' }) @ApiResponse({ status: 403, description: 'Not the seller or assigned agent' }) - @UseGuards(JwtAuthGuard) + @ApiResponse({ status: 429, description: 'Too many requests — feature-listing rate limit exceeded' }) + @UseGuards(JwtAuthGuard, FeatureListingThrottlerGuard) + @Throttle({ default: { limit: 10, ttl: 60_000 } }) @Post(':id/feature') async featureListing( @Param('id') id: string, diff --git a/apps/api/src/modules/shared/infrastructure/guards/feature-listing-throttler.guard.ts b/apps/api/src/modules/shared/infrastructure/guards/feature-listing-throttler.guard.ts new file mode 100644 index 0000000..784a6e4 --- /dev/null +++ b/apps/api/src/modules/shared/infrastructure/guards/feature-listing-throttler.guard.ts @@ -0,0 +1,35 @@ +import { Injectable } from '@nestjs/common'; +import { ThrottlerGuard } from '@nestjs/throttler'; +import { type Request } from 'express'; + +interface AuthenticatedRequest extends Request { + user?: { sub?: string; role?: string }; +} + +/** + * Throttler guard for the `feature-listing` endpoint. + * + * Extends the default ThrottlerGuard so rate limits are enforced via the + * globally-configured Redis-backed storage (see AppModule ThrottlerModule + * wiring). The tracker splits traffic per authenticated user when a JWT + * payload is present, otherwise falls back to the real client IP behind + * the reverse proxy. This produces independent per-user and per-IP buckets + * as required by TEC-2930. + */ +@Injectable() +export class FeatureListingThrottlerGuard extends ThrottlerGuard { + protected override getTracker(req: AuthenticatedRequest): Promise { + const userId = req.user?.sub; + if (userId) { + return Promise.resolve(`user:${userId}`); + } + + const forwarded = req.headers['x-forwarded-for']; + const ip = + typeof forwarded === 'string' + ? (forwarded.split(',')[0]?.trim() ?? req.ip ?? '127.0.0.1') + : (req.ip ?? '127.0.0.1'); + + return Promise.resolve(`ip:${ip}`); + } +} diff --git a/apps/api/src/modules/shared/infrastructure/index.ts b/apps/api/src/modules/shared/infrastructure/index.ts index 0a5f47d..2eba008 100644 --- a/apps/api/src/modules/shared/infrastructure/index.ts +++ b/apps/api/src/modules/shared/infrastructure/index.ts @@ -22,6 +22,7 @@ export { SanitizeInputMiddleware } from './middleware/sanitize-input.middleware' export { CsrfMiddleware } from './middleware/csrf.middleware'; export { maskPii } from './pii-masker'; export { ThrottlerBehindProxyGuard } from './guards/throttler-behind-proxy.guard'; +export { FeatureListingThrottlerGuard } from './guards/feature-listing-throttler.guard'; export { UserRateLimitGuard, DEFAULT_ROLE_LIMITS, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e0d7446..f00cf64 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -87,6 +87,9 @@ importers: '@goodgo/mcp-servers': specifier: workspace:* version: link:../../libs/mcp-servers + '@nest-lab/throttler-storage-redis': + specifier: ^1.2.0 + version: 1.2.0(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(@nestjs/throttler@6.5.0(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(reflect-metadata@0.2.2))(ioredis@5.10.1)(reflect-metadata@0.2.2) '@nestjs/bullmq': specifier: ^11.0.4 version: 11.0.4(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(bullmq@5.74.1) @@ -1513,6 +1516,15 @@ packages: '@napi-rs/wasm-runtime@0.2.12': resolution: {integrity: sha512-ZVWUcfwY4E/yPitQJl481FjFo3K22D6qF0DuFH6Y/nbnE11GY5uguDxZMGXPQ8WQ0128MXQD7TnfHyK4oWoIJQ==} + '@nest-lab/throttler-storage-redis@1.2.0': + resolution: {integrity: sha512-tMkUyo68NCKTR+zILk+EC35SMYBtDPZY2mCj7ZaCietWGVTnuP4zwq9ERYfvU6kJv6h8teNZrC6MJCmY6/dljw==} + peerDependencies: + '@nestjs/common': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + '@nestjs/core': ^7.0.0 || ^8.0.0 || ^9.0.0 || ^10.0.0 || ^11.0.0 + '@nestjs/throttler': '>=6.0.0' + ioredis: '>=5.0.0' + reflect-metadata: ^0.2.1 + '@nestjs/bull-shared@11.0.4': resolution: {integrity: sha512-VBJcDHSAzxQnpcDfA0kt9MTGUD1XZzfByV70su0W0eDCQ9aqIEBlzWRW21tv9FG9dIut22ysgDidshdjlnczLw==} peerDependencies: @@ -8663,6 +8675,15 @@ snapshots: '@tybys/wasm-util': 0.10.1 optional: true + '@nest-lab/throttler-storage-redis@1.2.0(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(@nestjs/throttler@6.5.0(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(reflect-metadata@0.2.2))(ioredis@5.10.1)(reflect-metadata@0.2.2)': + dependencies: + '@nestjs/common': 11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/core': 11.1.18(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@11.1.18)(@nestjs/websockets@11.1.19)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/throttler': 6.5.0(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)(reflect-metadata@0.2.2) + ioredis: 5.10.1 + reflect-metadata: 0.2.2 + tslib: 2.8.1 + '@nestjs/bull-shared@11.0.4(@nestjs/common@11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@11.1.18)': dependencies: '@nestjs/common': 11.1.18(class-transformer@0.5.1)(class-validator@0.15.1)(reflect-metadata@0.2.2)(rxjs@7.8.2)