From 4602f86b89427e64657ff748f7e2496b1de4970d Mon Sep 17 00:00:00 2001 From: Demian Date: Sat, 11 Apr 2026 01:28:58 -0400 Subject: [PATCH 1/4] feat: rate limit authentication endpoints Register ThrottlerModule globally with configurable default limits (100 req/60s). Apply stricter per-endpoint limits on auth routes via @Throttle decorator: - POST /auth/login: 10 req/60s (AUTH_LOGIN_THROTTLE_LIMIT/TTL) - POST /auth/register: 5 req/60s (AUTH_REGISTER_THROTTLE_LIMIT/TTL) - POST /auth/forgot-password: 5 req/60s (AUTH_FORGOT_THROTTLE_LIMIT/TTL) All limits are env-configurable. ThrottlerGuard applied as global APP_GUARD. 429 responses include Retry-After header via ThrottlerGuard defaults. Closes #95 --- backend/.env.example | 10 ++++++++++ backend/package.json | 1 + backend/src/app.module.ts | 21 ++++++++++++++++++++- backend/src/modules/auth/auth.controller.ts | 20 +++++++++++++++++++- pnpm-lock.yaml | 19 ++++++++++++++++++- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/backend/.env.example b/backend/.env.example index 9e03ac1..42558ff 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -17,6 +17,16 @@ USE_REDIS_CACHE=true PORT=3001 APP_NAME=STATION BACKEND +# Rate Limiting (TTL in milliseconds) +THROTTLE_TTL=60000 +THROTTLE_LIMIT=100 +AUTH_LOGIN_THROTTLE_TTL=60000 +AUTH_LOGIN_THROTTLE_LIMIT=10 +AUTH_REGISTER_THROTTLE_TTL=60000 +AUTH_REGISTER_THROTTLE_LIMIT=5 +AUTH_FORGOT_THROTTLE_TTL=60000 +AUTH_FORGOT_THROTTLE_LIMIT=5 + # UEX Sync Configuration UEX_SYNC_ENABLED=true UEX_CATEGORIES_SYNC_ENABLED=true diff --git a/backend/package.json b/backend/package.json index 76649e9..b06a1d9 100644 --- a/backend/package.json +++ b/backend/package.json @@ -42,6 +42,7 @@ "@nestjs/platform-express": "^10.0.0", "@nestjs/schedule": "^6.0.1", "@nestjs/swagger": "^7.4.2", + "@nestjs/throttler": "^6.5.0", "@nestjs/typeorm": "^10.0.2", "@types/bcrypt": "^6.0.0", "axios": "^1.13.2", diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index 80e507e..3ae1f6f 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -1,8 +1,10 @@ import { Module, DynamicModule } from '@nestjs/common'; +import { APP_GUARD } from '@nestjs/core'; import { TypeOrmModule } from '@nestjs/typeorm'; import { ConfigModule, ConfigService } from '@nestjs/config'; import { CacheModule } from '@nestjs/cache-manager'; import { ScheduleModule } from '@nestjs/schedule'; +import { ThrottlerModule, ThrottlerGuard } from '@nestjs/throttler'; import { redisStore } from 'cache-manager-redis-yet'; import { UsersModule } from './modules/users/users.module'; import { AuthModule } from './modules/auth/auth.module'; @@ -36,6 +38,17 @@ if (!isTest) { ConfigModule.forRoot({ isGlobal: true, }), + ThrottlerModule.forRootAsync({ + imports: [ConfigModule], + inject: [ConfigService], + useFactory: (configService: ConfigService) => [ + { + name: 'default', + ttl: configService.get('THROTTLE_TTL', 60000), + limit: configService.get('THROTTLE_LIMIT', 100), + }, + ], + }), ...conditionalImports, CacheModule.registerAsync({ isGlobal: true, @@ -118,6 +131,12 @@ if (!isTest) { OrgInventoryModule, ], controllers: [AppController], - providers: [AppService], + providers: [ + AppService, + { + provide: APP_GUARD, + useClass: ThrottlerGuard, + }, + ], }) export class AppModule {} diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 080e06a..7eded24 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -6,6 +6,7 @@ import { ApiBody, ApiBearerAuth, } from '@nestjs/swagger'; +import { Throttle } from '@nestjs/throttler'; import { AuthService } from './auth.service'; import { LocalAuthGuard } from './local-auth.guard'; import { JwtAuthGuard } from './jwt-auth.guard'; @@ -35,6 +36,12 @@ export class AuthController { }) @ApiResponse({ status: 200, description: 'Successfully logged in' }) @ApiResponse({ status: 401, description: 'Invalid credentials' }) + @Throttle({ + default: { + ttl: parseInt(process.env['AUTH_LOGIN_THROTTLE_TTL'] ?? '60000'), + limit: parseInt(process.env['AUTH_LOGIN_THROTTLE_LIMIT'] ?? '10'), + }, + }) @UseGuards(LocalAuthGuard) @Post('login') async login(@Request() req: ExpressRequest) { @@ -44,7 +51,12 @@ export class AuthController { @ApiOperation({ summary: 'Register new user' }) @ApiResponse({ status: 201, description: 'User successfully registered' }) @ApiResponse({ status: 400, description: 'Invalid input data' }) - // Registration Route: No guards required + @Throttle({ + default: { + ttl: parseInt(process.env['AUTH_REGISTER_THROTTLE_TTL'] ?? '60000'), + limit: parseInt(process.env['AUTH_REGISTER_THROTTLE_LIMIT'] ?? '5'), + }, + }) @Post('register') async register(@Body() userDto: UserDto) { return this.authService.register(userDto); @@ -80,6 +92,12 @@ export class AuthController { description: 'If an account with that email exists, a password reset link has been sent', }) + @Throttle({ + default: { + ttl: parseInt(process.env['AUTH_FORGOT_THROTTLE_TTL'] ?? '60000'), + limit: parseInt(process.env['AUTH_FORGOT_THROTTLE_LIMIT'] ?? '5'), + }, + }) @Post('forgot-password') async forgotPassword(@Body() forgotPasswordDto: ForgotPasswordDto) { return this.authService.requestPasswordReset(forgotPasswordDto.email); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f6c590f..018d436 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -53,6 +53,9 @@ importers: '@nestjs/swagger': specifier: ^7.4.2 version: 7.4.2(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2) + '@nestjs/throttler': + specifier: ^6.5.0 + version: 6.5.0(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(reflect-metadata@0.2.2) '@nestjs/typeorm': specifier: ^10.0.2 version: 10.0.2(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(reflect-metadata@0.2.2)(rxjs@7.8.2)(typeorm@0.3.27(babel-plugin-macros@3.1.0)(pg@8.16.3)(redis@5.9.0)(reflect-metadata@0.2.2)(ts-node@10.9.2(@types/node@20.19.25)(typescript@5.9.3))) @@ -1067,6 +1070,13 @@ packages: '@nestjs/platform-express': optional: true + '@nestjs/throttler@6.5.0': + resolution: {integrity: sha512-9j0ZRfH0QE1qyrj9JjIRDz5gQLPqq9yVC2nHsrosDVAfI5HHw08/aUAWx9DZLSdQf4HDkmhTTEGLrRFHENvchQ==} + 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 + reflect-metadata: ^0.1.13 || ^0.2.0 + '@nestjs/typeorm@10.0.2': resolution: {integrity: sha512-H738bJyydK4SQkRCTeh1aFBxoO1E9xdL/HaLGThwrqN95os5mEyAtK7BLADOS+vldP4jDZ2VQPLj4epWwRqCeQ==} peerDependencies: @@ -2609,11 +2619,12 @@ packages: glob@10.4.5: resolution: {integrity: sha512-7Bv8RF0k6xjo7d4A/PxYLbUCfb6c+Vpd2/mB2yRDlew7Jb5hEXiCD9ibfO7wpk8i4sevK6DFny9h7EYbM3/sHg==} + deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me hasBin: true glob@7.2.3: resolution: {integrity: sha512-nFR0zLpU2YCaRxwoCJvL6UvCH2JFyFVIvwTLsIf21AuHlMskA1hhTdk+LlYJtOlYt9v6dvszD2BGRqBL+iQK9Q==} - deprecated: Glob versions prior to v9 are no longer supported + deprecated: Old versions of glob are not supported, and contain widely publicized security vulnerabilities, which have been fixed in the current version. Please update. Support for old versions may be purchased (at exorbitant rates) by contacting i@izs.me globals@13.24.0: resolution: {integrity: sha512-AhO5QUcj8llrbG09iWhPU2B204J1xnPeL8kQmVorSsy+Sjj1sk8gIyh6cUocGmH4L0UuhAJy+hJMRA4mgA4mFQ==} @@ -5446,6 +5457,12 @@ snapshots: optionalDependencies: '@nestjs/platform-express': 10.4.20(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20) + '@nestjs/throttler@6.5.0(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(reflect-metadata@0.2.2)': + dependencies: + '@nestjs/common': 10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) + '@nestjs/core': 10.4.20(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/platform-express@10.4.20)(reflect-metadata@0.2.2)(rxjs@7.8.2) + reflect-metadata: 0.2.2 + '@nestjs/typeorm@10.0.2(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(reflect-metadata@0.2.2)(rxjs@7.8.2)(typeorm@0.3.27(babel-plugin-macros@3.1.0)(pg@8.16.3)(redis@5.9.0)(reflect-metadata@0.2.2)(ts-node@10.9.2(@types/node@20.19.25)(typescript@5.9.3)))': dependencies: '@nestjs/common': 10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2) From fe6198cf385e75f933b73c17e7a9bdc1e61ff71c Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 19:15:26 -0400 Subject: [PATCH 2/4] fix: move imports before module-level constants in auth.controller.ts --- backend/src/modules/auth/auth.controller.ts | 24 ++++++++++----------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 2b7224f..6b092ec 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -17,6 +17,18 @@ import { ApiBearerAuth, } from '@nestjs/swagger'; import { Throttle } from '@nestjs/throttler'; +import { AuthService } from './auth.service'; +import { LocalAuthGuard } from './local-auth.guard'; +import { JwtAuthGuard } from './jwt-auth.guard'; +import { RefreshTokenAuthGuard } from './refresh-token-auth.guard'; +import { UserDto } from '../users/dto/user.dto'; +import { User } from '../users/user.entity'; +import { Request as ExpressRequest, Response } from 'express'; +import { + ChangePasswordDto, + ForgotPasswordDto, + ResetPasswordDto, +} from './dto/password-reset.dto'; // Parse throttle config once at module load time. // Number() handles numeric strings and NaN from non-numeric input; the @@ -48,18 +60,6 @@ const FORGOT_LIMIT = toThrottleInt( process.env['AUTH_FORGOT_THROTTLE_LIMIT'], 5, ); -import { AuthService } from './auth.service'; -import { LocalAuthGuard } from './local-auth.guard'; -import { JwtAuthGuard } from './jwt-auth.guard'; -import { RefreshTokenAuthGuard } from './refresh-token-auth.guard'; -import { UserDto } from '../users/dto/user.dto'; -import { User } from '../users/user.entity'; -import { Request as ExpressRequest, Response } from 'express'; -import { - ChangePasswordDto, - ForgotPasswordDto, - ResetPasswordDto, -} from './dto/password-reset.dto'; @ApiTags('auth') @Controller('auth') From c679ad8d89287fb5a05ec0b5145f6c083cf8da2b Mon Sep 17 00:00:00 2001 From: Demian Date: Wed, 15 Apr 2026 20:02:08 -0400 Subject: [PATCH 3/4] fix: address PR #115 review comments (round 2) - getTracker(): use req.ips[0] (Express proxy-aware) instead of reading X-Forwarded-For directly; avoids spoofing when trust proxy is not set - main.ts: move dotenv/config to first import so process.env is populated before any module-level code evaluates it (fixes per-route throttle constants not reading .env values during local development) - e2e test: add Retry-After header assertions (defined, integer, > 0) --- backend/src/common/guards/throttler.guard.ts | 30 ++++++++++---------- backend/src/main.ts | 9 ++++-- backend/test/auth-rate-limit.e2e-spec.ts | 6 ++++ 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/backend/src/common/guards/throttler.guard.ts b/backend/src/common/guards/throttler.guard.ts index ef2d12a..5570d4a 100644 --- a/backend/src/common/guards/throttler.guard.ts +++ b/backend/src/common/guards/throttler.guard.ts @@ -3,25 +3,25 @@ import { ThrottlerGuard } from '@nestjs/throttler'; import { Request } from 'express'; /** - * Extends the default ThrottlerGuard to derive the client identifier from the - * X-Forwarded-For header when the app is deployed behind a reverse proxy or - * load balancer. Falls back to req.ip when the header is absent (direct - * connections or local development). + * Extends the default ThrottlerGuard to derive the client IP from the + * IP address resolved by Express. When the app is deployed behind a trusted + * reverse proxy or load balancer, Express populates `req.ips`; otherwise this + * falls back to `req.ip` for direct connections or local development. * - * Production note: ensure the upstream proxy is trusted so that clients cannot - * spoof the X-Forwarded-For header (e.g. configure `app.set('trust proxy', 1)` - * on the underlying Express adapter, or restrict which IPs may set the header - * at the network level). + * Production note: configure proxy trust correctly on the underlying Express + * adapter (for example, `app.set('trust proxy', 1)`) so forwarded addresses + * are only used when supplied by trusted infrastructure. Reading the raw + * X-Forwarded-For header directly is avoided here because it can be spoofed + * by any client that reaches the app without passing through the proxy. */ @Injectable() export class CustomThrottlerGuard extends ThrottlerGuard { protected async getTracker(req: Record): Promise { - const forwarded = (req as Request).headers['x-forwarded-for']; - const clientIp = Array.isArray(forwarded) - ? forwarded[0]?.trim() - : typeof forwarded === 'string' - ? forwarded.split(',')[0]?.trim() - : undefined; - return clientIp ?? (req as Request).ip ?? 'unknown'; + const request = req as Request; + // req.ips is populated by Express only when trust proxy is configured; + // it contains the full chain of forwarded IPs with spoofed entries stripped. + // Fall back to req.ip (the direct connection address) when not behind a proxy. + const clientIp = request.ips?.length ? request.ips[0] : request.ip; + return clientIp ?? 'unknown'; } } diff --git a/backend/src/main.ts b/backend/src/main.ts index fac16cf..1a0a583 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -1,3 +1,9 @@ +// dotenv/config must be the very first import so that process.env is populated +// before any other module is evaluated. Module-level code (e.g. decorator +// arguments and top-level constants) in NestJS modules runs at require() time, +// which is before bootstrap() — moving the load here ensures .env values are +// visible to those expressions during local development. +import 'dotenv/config'; import 'reflect-metadata'; import { NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; @@ -5,13 +11,10 @@ import { Logger, ValidationPipe } from '@nestjs/common'; import { ConfigService } from '@nestjs/config'; import { SwaggerModule, DocumentBuilder } from '@nestjs/swagger'; import * as figlet from 'figlet'; -import * as dotenv from 'dotenv'; import cookieParser from 'cookie-parser'; import helmet from 'helmet'; import { HttpExceptionFilter } from './common/filters/http-exception.filter'; -dotenv.config(); - async function bootstrap() { const app = await NestFactory.create(AppModule); diff --git a/backend/test/auth-rate-limit.e2e-spec.ts b/backend/test/auth-rate-limit.e2e-spec.ts index 8934902..00b0441 100644 --- a/backend/test/auth-rate-limit.e2e-spec.ts +++ b/backend/test/auth-rate-limit.e2e-spec.ts @@ -62,5 +62,11 @@ describe('Auth - Rate Limiting (e2e)', () => { .send(payload); expect(response.status).toBe(429); + + // Throttler must include a Retry-After header so clients know when to retry. + expect(response.headers['retry-after']).toBeDefined(); + const retryAfter = Number(response.headers['retry-after']); + expect(Number.isInteger(retryAfter)).toBe(true); + expect(retryAfter).toBeGreaterThan(0); }); }); From 2b39a113bb5d8d3c74ebceb9ca1ee4008ec489c9 Mon Sep 17 00:00:00 2001 From: Demian Date: Wed, 15 Apr 2026 22:28:20 -0400 Subject: [PATCH 4/4] test: harden auth-rate-limit e2e spec - Replace \`Number(env) || 10\` with \`toThrottleInt()\` to mirror the exact same parsing/flooring/fallback logic used in auth.controller.ts; prevents a misconfigured env value (e.g. 'Infinity') from hanging the for loop - Read \`AUTH_LOGIN_THROTTLE_TTL_MS\` via the same helper to derive the expected TTL window in seconds - Add upper-bound assertion: \`Retry-After\` must be <= \`ceil(TTL_MS / 1000)\`; a larger value would indicate a unit mismatch or misconfiguration in the throttler setup --- backend/test/auth-rate-limit.e2e-spec.ts | 32 +++++++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/backend/test/auth-rate-limit.e2e-spec.ts b/backend/test/auth-rate-limit.e2e-spec.ts index 00b0441..a4898ab 100644 --- a/backend/test/auth-rate-limit.e2e-spec.ts +++ b/backend/test/auth-rate-limit.e2e-spec.ts @@ -15,13 +15,31 @@ import { seedSystemUser } from './helpers/seed-system-user'; * spins up its own application instance the throttle state is isolated and * will not bleed into other e2e suites. */ + +// Mirror the same parsing logic used by auth.controller.ts so the test stays +// in sync with production behaviour (floats are floored, non-finite values +// fall back to the default, matching toThrottleInt in the controller). +const toThrottleInt = (value: string | undefined, fallback: number): number => { + const n = Number(value); + return Number.isFinite(n) && n > 0 ? Math.floor(n) : fallback; +}; + describe('Auth - Rate Limiting (e2e)', () => { let app: INestApplication; - // LOGIN_LIMIT must match the AUTH_LOGIN_THROTTLE_LIMIT default (10) or the - // env var set in the test environment. We read it here so the assertion - // stays in sync with the controller's actual config. - const loginLimit = Number(process.env['AUTH_LOGIN_THROTTLE_LIMIT']) || 10; + // LOGIN_LIMIT and LOGIN_TTL_MS must match the AUTH_LOGIN_THROTTLE_* defaults + // (10 requests / 60 s) or the env vars set in the test environment. Using + // toThrottleInt() ensures we mirror the exact same rounding/validation as the + // production controller, so a misconfigured env value (e.g. 'Infinity') will + // produce the same safe fallback in both places rather than hanging the loop. + const loginLimit = toThrottleInt( + process.env['AUTH_LOGIN_THROTTLE_LIMIT'], + 10, + ); + const loginTtlMs = toThrottleInt( + process.env['AUTH_LOGIN_THROTTLE_TTL_MS'], + 60_000, + ); beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ @@ -68,5 +86,11 @@ describe('Auth - Rate Limiting (e2e)', () => { const retryAfter = Number(response.headers['retry-after']); expect(Number.isInteger(retryAfter)).toBe(true); expect(retryAfter).toBeGreaterThan(0); + + // Retry-After must not exceed the configured TTL window. A value larger + // than the window indicates a misconfiguration or unit mismatch (e.g. ms + // instead of seconds) in the throttler setup. + const loginTtlSeconds = Math.ceil(loginTtlMs / 1000); + expect(retryAfter).toBeLessThanOrEqual(loginTtlSeconds); }); });