From 90189661ddaa025bd9420477cc99a7e309a21f82 Mon Sep 17 00:00:00 2001 From: LGTM-ButItsBroken Date: Fri, 16 Oct 2026 22:36:55 +0100 Subject: [PATCH] feat(auth): Prometheus metrics, query optimisation, Redis caching, per-client rate limits --- docs/api/auth-caching.md | 7 ++ docs/api/auth-db-optimization.md | 7 ++ docs/api/auth-metrics.md | 10 +++ docs/api/auth-rate-limiting.md | 14 ++++ src/auth/auth-cache.service.ts | 44 ++++++++++++ src/auth/auth-metrics.ts | 32 +++++++++ src/auth/auth.controller.ts | 15 ++++ src/auth/auth.module.ts | 4 ++ src/auth/auth.service.ts | 42 ++++++----- .../guards/auth-route-rate-limit.guard.ts | 72 +++++++++++++++++++ .../interceptors/auth-metrics.interceptor.ts | 26 +++++++ src/auth/rate-limit.service.ts | 14 ++++ 12 files changed, 268 insertions(+), 19 deletions(-) create mode 100644 docs/api/auth-caching.md create mode 100644 docs/api/auth-db-optimization.md create mode 100644 docs/api/auth-metrics.md create mode 100644 docs/api/auth-rate-limiting.md create mode 100644 src/auth/auth-cache.service.ts create mode 100644 src/auth/auth-metrics.ts create mode 100644 src/auth/guards/auth-route-rate-limit.guard.ts create mode 100644 src/auth/interceptors/auth-metrics.interceptor.ts diff --git a/docs/api/auth-caching.md b/docs/api/auth-caching.md new file mode 100644 index 00000000..a7cfa8c7 --- /dev/null +++ b/docs/api/auth-caching.md @@ -0,0 +1,7 @@ +# Auth API Redis caching + +`GET /auth/me/dashboard`-style dashboard data (`AuthService.getDashboard`) is cached per user for 30 seconds through `AuthCacheService`, which uses the shared `CacheService` (Redis when configured). + +- Key: `auth:dashboard:`; TTL 30s, so data can be up to 30 seconds stale. +- Read-through only: cache errors, or no cache being available, fall back to computing the response, so caching can never fail a request. +- Token, session and API-key checks are deliberately **not** cached, because revocation has to take effect immediately. diff --git a/docs/api/auth-db-optimization.md b/docs/api/auth-db-optimization.md new file mode 100644 index 00000000..aa92a5b7 --- /dev/null +++ b/docs/api/auth-db-optimization.md @@ -0,0 +1,7 @@ +# Auth API database query optimisation + +Changes in `AuthService`: + +- **Refresh-token reuse detection** counted tokens in a family by fetching every row just to log the number; it now uses `count()`. +- **Logout everywhere** fetched all of a user's blacklisted refresh tokens only for `.length`; it now uses `count()`. +- **Dashboard** ran a query for the user's ten latest properties whose result was never used (removed), and fetched up to three API key rows to report their number. It now uses `apiKey.count()`, so `apiKeysCount` is the real number of keys rather than being capped at 3. diff --git a/docs/api/auth-metrics.md b/docs/api/auth-metrics.md new file mode 100644 index 00000000..27e394f8 --- /dev/null +++ b/docs/api/auth-metrics.md @@ -0,0 +1,10 @@ +# Auth API Prometheus metrics + +Exposed on the existing `/metrics` endpoint: + +| Metric | Type | Labels | +| --- | --- | --- | +| `auth_requests_total` | counter | `operation` (controller handler name), `outcome` (`success`, `client_error`, `server_error`) | +| `auth_request_duration_seconds` | histogram | `operation` | + +`AuthMetricsInterceptor` records both for every `AuthController` handler. Labels are bounded (a fixed set of handler names and three outcomes), so cardinality does not grow with user input. Example alert: rate of `auth_requests_total{outcome="server_error"}` above a threshold. diff --git a/docs/api/auth-rate-limiting.md b/docs/api/auth-rate-limiting.md new file mode 100644 index 00000000..9d520881 --- /dev/null +++ b/docs/api/auth-rate-limiting.md @@ -0,0 +1,14 @@ +# Auth API rate limiting + +`AuthRouteRateLimitGuard` + `@AuthRouteLimit({ max, windowMs })` limit sensitive unauthenticated routes per client address (hashed before storage). Each client has its own counter per route, so one abusive client cannot lock out everyone else. + +| Route | Limit | +| --- | --- | +| `POST /auth/register` | 5 per hour | +| `POST /auth/refresh` | 30 per 15 min | +| `POST /auth/password-reset/request` | 5 per hour | +| `POST /auth/password-reset/reset` | 10 per hour | +| `POST /auth/verify-email` | 10 per 15 min | +| `POST /auth/email/resend` | 3 per hour | + +Over the limit the response is `429` with a `Retry-After` header. Login keeps its existing account/IP lockout (`LoginRateLimitService`). Note: the `ENDPOINT_RATE_LIMITS` table is a single bucket shared by all clients, so it was not used here. diff --git a/src/auth/auth-cache.service.ts b/src/auth/auth-cache.service.ts new file mode 100644 index 00000000..8d367bac --- /dev/null +++ b/src/auth/auth-cache.service.ts @@ -0,0 +1,44 @@ +import { Injectable, Optional } from '@nestjs/common'; +import { CacheService } from '../cache/cache.service'; +import { CacheOperationStatus } from '../cache/cache.types'; + +export const AUTH_DASHBOARD_TTL_SECONDS = 30; + +/** + * Read-through Redis cache for expensive, read-only Auth API responses. + * Cache failures never fail a request: on any cache error, or when no cache is + * available, the loader runs as if caching were off. + */ +@Injectable() +export class AuthCacheService { + constructor(@Optional() private readonly cache?: CacheService) {} + + dashboardKey(userId: string): string { + return `auth:dashboard:${userId}`; + } + + async remember(key: string, ttlSeconds: number, loader: () => Promise): Promise { + if (!this.cache) return loader(); + try { + const hit = await this.cache.get(key); + if (hit.status === CacheOperationStatus.SUCCESS && hit.data !== undefined) return hit.data as T; + } catch { + return loader(); + } + const value = await loader(); + try { + await this.cache.set(key, value, ttlSeconds); + } catch { + /* a failed write only means the next call recomputes */ + } + return value; + } + + async invalidateDashboard(userId: string): Promise { + try { + await this.cache?.del(this.dashboardKey(userId)); + } catch { + /* the entry expires on its own within the TTL */ + } + } +} diff --git a/src/auth/auth-metrics.ts b/src/auth/auth-metrics.ts new file mode 100644 index 00000000..72a085e5 --- /dev/null +++ b/src/auth/auth-metrics.ts @@ -0,0 +1,32 @@ +import { Counter, Histogram, register } from 'prom-client'; + +/** + * Prometheus metrics for the Auth API. Labels are bounded: `operation` is a + * controller handler name and `outcome` is one of three values, so cardinality + * cannot grow with user input. Metrics are looked up before being created so + * hot reloads and test suites do not register them twice. + */ +export type AuthOutcome = 'success' | 'client_error' | 'server_error'; + +export const authRequestsTotal: Counter<'operation' | 'outcome'> = + (register.getSingleMetric('auth_requests_total') as Counter<'operation' | 'outcome'>) ?? + new Counter({ + name: 'auth_requests_total', + help: 'Auth API requests by handler and outcome', + labelNames: ['operation', 'outcome'], + }); + +export const authRequestDurationSeconds: Histogram<'operation'> = + (register.getSingleMetric('auth_request_duration_seconds') as Histogram<'operation'>) ?? + new Histogram({ + name: 'auth_request_duration_seconds', + help: 'Auth API handler latency in seconds', + labelNames: ['operation'], + buckets: [0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5], + }); + +export function outcomeForStatus(status: number): AuthOutcome { + if (status >= 500) return 'server_error'; + if (status >= 400) return 'client_error'; + return 'success'; +} diff --git a/src/auth/auth.controller.ts b/src/auth/auth.controller.ts index 39700d72..3831ed08 100644 --- a/src/auth/auth.controller.ts +++ b/src/auth/auth.controller.ts @@ -30,15 +30,20 @@ import { GoogleProfile } from './strategies/google.strategy'; import { UserRole } from '../types/prisma.types'; import { Request } from 'express'; import { VerifyEmailDto } from '../users/dto/email-change.dto'; +import { AuthMetricsInterceptor } from './interceptors/auth-metrics.interceptor'; +import { AuthRouteLimit, AuthRouteRateLimitGuard } from './guards/auth-route-rate-limit.guard'; @ApiTags('Authentication') @Controller('auth') +@UseInterceptors(AuthMetricsInterceptor) export class AuthController { constructor( private readonly authService: AuthService, private readonly apiKeyAnalyticsService: ApiKeyAnalyticsService, ) {} + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 5, windowMs: 60 * 60 * 1000 }) @Post('register') register(@Body() registerDto: RegisterDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -64,6 +69,8 @@ export class AuthController { return this.authService.login(loginDto, ipAddress, userAgent); } + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 30, windowMs: 15 * 60 * 1000 }) @Post('refresh') refresh(@Body() refreshTokenDto: RefreshTokenDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -223,11 +230,15 @@ export class AuthController { return this.apiKeyAnalyticsService.getUsageAnalytics(id, user.sub); } + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 5, windowMs: 60 * 60 * 1000 }) @Post('password-reset/request') requestPasswordReset(@Body() requestPasswordResetDto: RequestPasswordResetDto) { return this.authService.requestPasswordReset(requestPasswordResetDto); } + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 10, windowMs: 60 * 60 * 1000 }) @Post('password-reset/reset') resetPassword(@Body() resetPasswordDto: ResetPasswordDto) { return this.authService.resetPassword(resetPasswordDto); @@ -246,6 +257,8 @@ export class AuthController { return this.authService.getLoginStatus(data.email); } + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 10, windowMs: 15 * 60 * 1000 }) @Post('verify-email') verifyEmail(@Body() verifyEmailDto: VerifyEmailDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -253,6 +266,8 @@ export class AuthController { return this.authService.verifyInitialEmail(verifyEmailDto.token, ipAddress, userAgent); } + @UseGuards(AuthRouteRateLimitGuard) + @AuthRouteLimit({ max: 3, windowMs: 60 * 60 * 1000 }) @Post('email/resend') resendVerification(@Body() data: { email: string }, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; diff --git a/src/auth/auth.module.ts b/src/auth/auth.module.ts index af78f09e..a2d04118 100644 --- a/src/auth/auth.module.ts +++ b/src/auth/auth.module.ts @@ -20,12 +20,16 @@ import { RateLimitGuard } from './guards/rate-limit.guard'; import { RateLimitHeadersInterceptor } from './interceptors/rate-limit-headers.interceptor'; import { RateLimitAdminController } from './controllers/rate-limit-admin.controller'; import { FraudModule } from '../fraud/fraud.module'; +import { AuthCacheService } from './auth-cache.service'; +import { AuthRouteRateLimitGuard } from './guards/auth-route-rate-limit.guard'; @Module({ imports: [PrismaModule, UsersModule, SessionsModule, EmailModule, FraudModule, PassportModule], controllers: [AuthController, RateLimitAdminController], providers: [ AuthService, + AuthCacheService, + AuthRouteRateLimitGuard, ApiKeyAnalyticsService, LoginRateLimitService, RateLimitService, diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 4078c663..da2bbd57 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -7,6 +7,7 @@ import { ServiceUnavailableException, UnauthorizedException, } from '@nestjs/common'; +import { AUTH_DASHBOARD_TTL_SECONDS, AuthCacheService } from './auth-cache.service'; import { ConfigService } from '@nestjs/config'; import { Prisma, BlacklistedToken, Document, ApiKey, PasswordHistory } from '@prisma/client'; import { randomUUID } from 'crypto'; @@ -114,6 +115,7 @@ export class AuthService { private readonly fraudService: FraudService, @Optional() private readonly apiKeyAnalyticsService?: ApiKeyAnalyticsService, @Optional() private readonly twoFactorService?: TwoFactorService, + @Optional() private readonly authCache?: AuthCacheService, ) { const jwtSecret = this.configService.get('JWT_SECRET'); if (!jwtSecret || jwtSecret.length < MIN_JWT_SECRET_LENGTH) { @@ -586,16 +588,16 @@ export class AuthService { // Invalidate entire token family if it exists if (blacklistedToken.tokenFamily) { - const familyTokens = await this.prisma.blacklistedToken.findMany({ + // Only the count is logged, so count in the database instead of fetching rows. + const familyTokenCount = await this.prisma.blacklistedToken.count({ where: { tokenFamily: blacklistedToken.tokenFamily, expiresAt: { gt: now }, // Only active tokens }, - select: { jti: true }, }); this.logger.warn( - `Invalidating ${familyTokens.length} tokens in family ${blacklistedToken.tokenFamily} due to reuse detection`, + `Invalidating ${familyTokenCount} tokens in family ${blacklistedToken.tokenFamily} due to reuse detection`, ); // All tokens in this family are already blacklisted, but we log the event @@ -694,7 +696,7 @@ export class AuthService { await this.sessionsService.revokeAllSessions(user.sub); // Find all blacklisted refresh tokens for this user that are still active - const blacklistedRefreshTokens = await this.prisma.blacklistedToken.findMany({ + const blacklistedRefreshTokenCount = await this.prisma.blacklistedToken.count({ where: { userId: user.sub, tokenType: 'REFRESH', @@ -705,13 +707,13 @@ export class AuthService { }); this.logger.log( - `User ${user.sub} (${this.hashEmail(user.email)}) logged out from all devices at ${logoutTime.toISOString()}. Total active blacklisted refresh tokens: ${blacklistedRefreshTokens.length}`, + `User ${user.sub} (${this.hashEmail(user.email)}) logged out from all devices at ${logoutTime.toISOString()}. Total active blacklisted refresh tokens: ${blacklistedRefreshTokenCount}`, ); return { message: 'Logged out from all devices successfully', logoutTime: logoutTime.toISOString(), - blacklistedTokensCount: blacklistedRefreshTokens.length, + blacklistedTokensCount: blacklistedRefreshTokenCount, clientAction: { clearStorage: true, clearCookies: true, @@ -732,7 +734,18 @@ export class AuthService { return sanitizeUser(foundUser); } + /** + * Dashboard summary, cached for 30 seconds per user (Redis via CacheService). + * Data may be up to 30s stale; any cache failure falls back to computing it. + */ async getDashboard(user: AuthUserPayload) { + if (!this.authCache) return this.loadDashboard(user); + return this.authCache.remember(this.authCache.dashboardKey(user.sub), AUTH_DASHBOARD_TTL_SECONDS, () => + this.loadDashboard(user), + ); + } + + private async loadDashboard(user: AuthUserPayload) { const foundUser = await this.prisma.user.findUnique({ where: { id: user.sub }, }); @@ -741,14 +754,9 @@ export class AuthService { throw new NotFoundException('User not found'); } - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const [properties, buyerTransactions, sellerTransactions, documents, apiKeys] = + // The recent-properties list was fetched here but never used, so it is no longer queried. + const [buyerTransactions, sellerTransactions, documents, apiKeysCount] = await Promise.all([ - this.prisma.property.findMany({ - where: { ownerId: user.sub }, - orderBy: { createdAt: 'desc' }, - take: 10, - }), this.prisma.transaction.findMany({ where: { buyerId: user.sub }, orderBy: { createdAt: 'desc' }, @@ -800,11 +808,7 @@ export class AuthService { orderBy: { createdAt: 'desc' }, take: 5, }), - this.prisma.apiKey.findMany({ - where: { userId: user.sub }, - orderBy: { createdAt: 'desc' }, - take: 3, - }), + this.prisma.apiKey.count({ where: { userId: user.sub } }), ]); const [ @@ -869,7 +873,7 @@ export class AuthService { totalSales, completedPurchases, completedSales, - apiKeysCount: apiKeys.length, + apiKeysCount, }, recentActivity, recommendations: recommendationProperties.map((p: PropertyWithOwnerName) => ({ diff --git a/src/auth/guards/auth-route-rate-limit.guard.ts b/src/auth/guards/auth-route-rate-limit.guard.ts new file mode 100644 index 00000000..35609624 --- /dev/null +++ b/src/auth/guards/auth-route-rate-limit.guard.ts @@ -0,0 +1,72 @@ +import { + CanActivate, + ExecutionContext, + HttpException, + HttpStatus, + Injectable, + SetMetadata, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { RateLimitService } from '../rate-limit.service'; +import { createSha256 } from '../security.utils'; + +export const AUTH_ROUTE_LIMIT_KEY = 'auth-route-limit'; + +export interface AuthRouteLimitOptions { + /** Requests allowed per client per window. */ + max: number; + windowMs: number; +} + +/** Limit one unauthenticated Auth route per client address. */ +export const AuthRouteLimit = (options: AuthRouteLimitOptions) => + SetMetadata(AUTH_ROUTE_LIMIT_KEY, options); + +/** + * Per-client rate limit for sensitive, unauthenticated Auth routes + * (registration, token refresh, password reset, email verification). + * + * Each client address gets its own counter per route, so one abusive client + * cannot lock everyone else out. The address is hashed before it reaches the + * key store. Over the limit the response is `429` with `Retry-After`. + */ +@Injectable() +export class AuthRouteRateLimitGuard implements CanActivate { + constructor( + private readonly reflector: Reflector, + private readonly rateLimitService: RateLimitService, + ) {} + + async canActivate(context: ExecutionContext): Promise { + const options = this.reflector.get( + AUTH_ROUTE_LIMIT_KEY, + context.getHandler(), + ); + if (!options) return true; + + const request = context.switchToHttp().getRequest(); + const response = context.switchToHttp().getResponse(); + const route = `${request.method} ${request.route?.path ?? request.url}`; + const client = createSha256(String(request.ip ?? request.socket?.remoteAddress ?? 'unknown')); + + const status = await this.rateLimitService.checkScopedRateLimit( + `auth-route:${route}`, + client, + options.max, + options.windowMs, + ); + + if (status.isExceeded) { + if (status.retryAfter !== undefined) response.setHeader('Retry-After', String(status.retryAfter)); + throw new HttpException( + { + statusCode: HttpStatus.TOO_MANY_REQUESTS, + message: 'Too many requests. Please try again later.', + retryAfter: status.retryAfter, + }, + HttpStatus.TOO_MANY_REQUESTS, + ); + } + return true; + } +} diff --git a/src/auth/interceptors/auth-metrics.interceptor.ts b/src/auth/interceptors/auth-metrics.interceptor.ts new file mode 100644 index 00000000..795b3839 --- /dev/null +++ b/src/auth/interceptors/auth-metrics.interceptor.ts @@ -0,0 +1,26 @@ +import { CallHandler, ExecutionContext, Injectable, NestInterceptor } from '@nestjs/common'; +import { Observable } from 'rxjs'; +import { tap } from 'rxjs/operators'; +import { authRequestDurationSeconds, authRequestsTotal, outcomeForStatus } from '../auth-metrics'; + +/** Records `auth_requests_total` and `auth_request_duration_seconds` for each handler call. */ +@Injectable() +export class AuthMetricsInterceptor implements NestInterceptor { + intercept(context: ExecutionContext, next: CallHandler): Observable { + const operation = context.getHandler().name; + const end = authRequestDurationSeconds.startTimer({ operation }); + + const record = (status: number): void => { + end(); + authRequestsTotal.inc({ operation, outcome: outcomeForStatus(status) }); + }; + + return next.handle().pipe( + tap({ + next: () => record(context.switchToHttp().getResponse().statusCode ?? 200), + error: (err: { status?: number; getStatus?: () => number }) => + record(typeof err?.getStatus === 'function' ? err.getStatus() : (err?.status ?? 500)), + }), + ); + } +} diff --git a/src/auth/rate-limit.service.ts b/src/auth/rate-limit.service.ts index 885f18a7..4348b5e8 100644 --- a/src/auth/rate-limit.service.ts +++ b/src/auth/rate-limit.service.ts @@ -84,6 +84,20 @@ export class RateLimitService { /** * Generic rate limit check */ + /** + * Fixed-window limit for an arbitrary scope and identifier (for example + * `auth-route:POST /auth/register` per client). Unlike the endpoint limit, + * which is one bucket shared by everyone, this counts each identifier separately. + */ + async checkScopedRateLimit( + scope: string, + identifier: string, + max: number, + windowMs: number, + ): Promise { + return this.checkRateLimit(`rate-limit:scope:${scope}:${identifier}`, max, windowMs); + } + private async checkRateLimit( key: string, limit: number,