From 78b127e2978a40bda232f039598fd8bb03342cbe Mon Sep 17 00:00:00 2001 From: Tarrii Date: Fri, 16 Oct 2026 22:38:07 +0100 Subject: [PATCH] feat(users): Prometheus metrics, query optimisation, Redis caching, per-caller rate limits --- docs/api/users-caching.md | 7 ++ docs/api/users-db-optimization.md | 4 ++ docs/api/users-metrics.md | 10 +++ docs/api/users-rate-limiting.md | 12 ++++ src/auth/rate-limit.service.ts | 14 ++++ .../guards/users-route-rate-limit.guard.ts | 69 +++++++++++++++++++ .../interceptors/users-metrics.interceptor.ts | 26 +++++++ src/users/users-cache.service.ts | 44 ++++++++++++ src/users/users-metrics.ts | 32 +++++++++ src/users/users.controller.ts | 16 +++-- src/users/users.module.ts | 4 ++ src/users/users.service.ts | 26 +++++-- 12 files changed, 254 insertions(+), 10 deletions(-) create mode 100644 docs/api/users-caching.md create mode 100644 docs/api/users-db-optimization.md create mode 100644 docs/api/users-metrics.md create mode 100644 docs/api/users-rate-limiting.md create mode 100644 src/users/guards/users-route-rate-limit.guard.ts create mode 100644 src/users/interceptors/users-metrics.interceptor.ts create mode 100644 src/users/users-cache.service.ts create mode 100644 src/users/users-metrics.ts diff --git a/docs/api/users-caching.md b/docs/api/users-caching.md new file mode 100644 index 00000000..e7628510 --- /dev/null +++ b/docs/api/users-caching.md @@ -0,0 +1,7 @@ +# Users API Redis caching + +`UsersService.getUserStatistics` (`GET /users/me/statistics`) is cached per user for 60 seconds through `UsersCacheService`, which uses the shared `CacheService` (Redis when configured). + +- Key: `users:statistics:`; TTL 60s, so counts can be up to a minute stale. +- Read-through only: cache errors, or no cache being available, fall back to the database, so caching can never fail a request. +- Profile, role and permission data is deliberately **not** cached, because changes to it have to be visible immediately. diff --git a/docs/api/users-db-optimization.md b/docs/api/users-db-optimization.md new file mode 100644 index 00000000..3371d384 --- /dev/null +++ b/docs/api/users-db-optimization.md @@ -0,0 +1,4 @@ +# Users API database query optimisation + +- **`GET /users/me/statistics`** loaded the user together with every property and every buyer/seller transaction row just to report their lengths. It now selects two columns plus Prisma `_count` aggregates, so the database counts and no rows are transferred. +- **`GET /users/me/login-history`** returned the entire history, which grows without bound. It now returns the newest 100 entries (same ordering and fields). diff --git a/docs/api/users-metrics.md b/docs/api/users-metrics.md new file mode 100644 index 00000000..26f3af48 --- /dev/null +++ b/docs/api/users-metrics.md @@ -0,0 +1,10 @@ +# Users API Prometheus metrics + +Exposed on the existing `/metrics` endpoint: + +| Metric | Type | Labels | +| --- | --- | --- | +| `users_requests_total` | counter | `operation` (controller handler name), `outcome` (`success`, `client_error`, `server_error`) | +| `users_request_duration_seconds` | histogram | `operation` | + +`UsersMetricsInterceptor` records both for every `UsersController` handler. Labels are bounded (a fixed set of handler names and three outcomes), so cardinality does not grow with user input. diff --git a/docs/api/users-rate-limiting.md b/docs/api/users-rate-limiting.md new file mode 100644 index 00000000..82e7d91a --- /dev/null +++ b/docs/api/users-rate-limiting.md @@ -0,0 +1,12 @@ +# Users API rate limiting + +`UsersRouteRateLimitGuard` + `@UsersRouteLimit({ max, windowMs })` limit costly or abusable routes per caller (the authenticated user id, falling back to the client address). Each route has its own counter. + +| Route | Limit | +| --- | --- | +| `POST /users` (admin) | 10 per hour | +| `GET /users/search` (admin) | 60 per minute | +| `GET /users/me/statistics` | 30 per minute | +| `GET /users/me/login-history` | 30 per minute | + +Over the limit the response is `429` with a `Retry-After` header. The guard runs after the JWT guard so the caller is known. Existing in-controller limits (data export download, reactivation) are unchanged. 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, diff --git a/src/users/guards/users-route-rate-limit.guard.ts b/src/users/guards/users-route-rate-limit.guard.ts new file mode 100644 index 00000000..33a63ad9 --- /dev/null +++ b/src/users/guards/users-route-rate-limit.guard.ts @@ -0,0 +1,69 @@ +import { + CanActivate, + ExecutionContext, + HttpException, + HttpStatus, + Injectable, + SetMetadata, +} from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { RateLimitService } from '../../auth/rate-limit.service'; + +export const USERS_ROUTE_LIMIT_KEY = 'users-route-limit'; + +export interface UsersRouteLimitOptions { + /** Requests allowed per caller per window. */ + max: number; + windowMs: number; +} + +/** Limit one Users route per authenticated caller. Apply after the JWT guard. */ +export const UsersRouteLimit = (options: UsersRouteLimitOptions) => + SetMetadata(USERS_ROUTE_LIMIT_KEY, options); + +/** + * Per-caller rate limit for costly or abusable Users routes. The caller is the + * authenticated user id (falling back to the client address), and each route has + * its own counter, so heavy use of one route does not consume another's budget. + * Over the limit the response is `429` with `Retry-After`. + */ +@Injectable() +export class UsersRouteRateLimitGuard implements CanActivate { + constructor( + private readonly reflector: Reflector, + private readonly rateLimitService: RateLimitService, + ) {} + + async canActivate(context: ExecutionContext): Promise { + const options = this.reflector.get( + USERS_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 caller = String(request.authUser?.sub ?? request.user?.sub ?? request.ip ?? 'unknown'); + + const status = await this.rateLimitService.checkScopedRateLimit( + `users-route:${route}`, + caller, + 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/users/interceptors/users-metrics.interceptor.ts b/src/users/interceptors/users-metrics.interceptor.ts new file mode 100644 index 00000000..27b0e0cd --- /dev/null +++ b/src/users/interceptors/users-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 { usersOutcomeForStatus, usersRequestDurationSeconds, usersRequestsTotal } from '../users-metrics'; + +/** Records `users_requests_total` and `users_request_duration_seconds` for each handler call. */ +@Injectable() +export class UsersMetricsInterceptor implements NestInterceptor { + intercept(context: ExecutionContext, next: CallHandler): Observable { + const operation = context.getHandler().name; + const end = usersRequestDurationSeconds.startTimer({ operation }); + + const record = (status: number): void => { + end(); + usersRequestsTotal.inc({ operation, outcome: usersOutcomeForStatus(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/users/users-cache.service.ts b/src/users/users-cache.service.ts new file mode 100644 index 00000000..e6b686e2 --- /dev/null +++ b/src/users/users-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 USER_STATISTICS_TTL_SECONDS = 60; + +/** + * Read-through Redis cache for expensive, read-only Users 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 UsersCacheService { + constructor(@Optional() private readonly cache?: CacheService) {} + + statisticsKey(userId: string): string { + return `users:statistics:${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 invalidateStatistics(userId: string): Promise { + try { + await this.cache?.del(this.statisticsKey(userId)); + } catch { + /* the entry expires on its own within the TTL */ + } + } +} diff --git a/src/users/users-metrics.ts b/src/users/users-metrics.ts new file mode 100644 index 00000000..acbce9b1 --- /dev/null +++ b/src/users/users-metrics.ts @@ -0,0 +1,32 @@ +import { Counter, Histogram, register } from 'prom-client'; + +/** + * Prometheus metrics for the Users API. Labels are bounded: `operation` is a + * controller handler name and `outcome` 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 UsersOutcome = 'success' | 'client_error' | 'server_error'; + +export const usersRequestsTotal: Counter<'operation' | 'outcome'> = + (register.getSingleMetric('users_requests_total') as Counter<'operation' | 'outcome'>) ?? + new Counter({ + name: 'users_requests_total', + help: 'Users API requests by handler and outcome', + labelNames: ['operation', 'outcome'], + }); + +export const usersRequestDurationSeconds: Histogram<'operation'> = + (register.getSingleMetric('users_request_duration_seconds') as Histogram<'operation'>) ?? + new Histogram({ + name: 'users_request_duration_seconds', + help: 'Users API handler latency in seconds', + labelNames: ['operation'], + buckets: [0.01, 0.05, 0.1, 0.25, 0.5, 1, 2.5, 5], + }); + +export function usersOutcomeForStatus(status: number): UsersOutcome { + if (status >= 500) return 'server_error'; + if (status >= 400) return 'client_error'; + return 'success'; +} diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index ac8beabd..3554eeeb 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -16,6 +16,7 @@ import { Res, StreamableFile, UseGuards, + UseInterceptors, } from '@nestjs/common'; import { Response } from 'express'; import * as crypto from 'crypto'; @@ -30,6 +31,8 @@ import { UserRole } from '../types/prisma.types'; import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; import { UsersService } from './users.service'; import { ActivityLogService } from './activity-log.service'; +import { UsersMetricsInterceptor } from './interceptors/users-metrics.interceptor'; +import { UsersRouteLimit, UsersRouteRateLimitGuard } from './guards/users-route-rate-limit.guard'; import { CreateUserDto, SearchUsersDto, @@ -50,6 +53,7 @@ const REACTIVATE_WINDOW_MS = 60 * 60 * 1000; @ApiTags('Users') @ApiBearerAuth('access-token') +@UseInterceptors(UsersMetricsInterceptor) @Controller('users') export class UsersController { private readonly downloadRateLimitMap = new Map(); @@ -122,7 +126,8 @@ export class UsersController { // ─── Admin Endpoints ───────────────────────────────────────────── - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(JwtAuthGuard, RolesGuard, UsersRouteRateLimitGuard) + @UsersRouteLimit({ max: 10, windowMs: 60 * 60 * 1000 }) @Roles(UserRole.ADMIN) @Post() create(@Body() createUserDto: CreateUserDto) { @@ -136,14 +141,16 @@ export class UsersController { return this.usersService.findAll(); } - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(JwtAuthGuard, RolesGuard, UsersRouteRateLimitGuard) + @UsersRouteLimit({ max: 60, windowMs: 60 * 1000 }) @Roles(UserRole.ADMIN) @Get('search') search(@Query() query: SearchUsersDto) { return this.usersService.search(query); } - @UseGuards(JwtAuthGuard) + @UseGuards(JwtAuthGuard, UsersRouteRateLimitGuard) + @UsersRouteLimit({ max: 30, windowMs: 60 * 1000 }) @Get('me/statistics') getStatistics(@CurrentUser() user: AuthUserPayload) { return this.usersService.getUserStatistics(user.sub); @@ -387,7 +394,8 @@ export class UsersController { return this.usersService.getMyReferrals(user.sub); } - @UseGuards(JwtAuthGuard) + @UseGuards(JwtAuthGuard, UsersRouteRateLimitGuard) + @UsersRouteLimit({ max: 30, windowMs: 60 * 1000 }) @Get('me/login-history') getLoginHistory(@CurrentUser() user: AuthUserPayload) { return this.usersService.getLoginHistory(user.sub); diff --git a/src/users/users.module.ts b/src/users/users.module.ts index d8ad1055..319f4b99 100644 --- a/src/users/users.module.ts +++ b/src/users/users.module.ts @@ -18,6 +18,8 @@ import { DataExportService } from './data-export.service'; import { I18nModule } from '../i18n/i18n.module'; import { UserImportController } from './user-import.controller'; import { UserImportService } from './user-import.service'; +import { UsersCacheService } from './users-cache.service'; +import { UsersRouteRateLimitGuard } from './guards/users-route-rate-limit.guard'; @Module({ imports: [PrismaModule, SessionsModule, I18nModule], @@ -31,6 +33,8 @@ import { UserImportService } from './user-import.service'; ], providers: [ UsersService, + UsersCacheService, + UsersRouteRateLimitGuard, UserPreferencesService, ActivityLogService, EmailVerificationService, diff --git a/src/users/users.service.ts b/src/users/users.service.ts index d41d7f1d..2380bd0c 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -2,9 +2,11 @@ import { Injectable, Logger, OnModuleInit, + Optional, NotFoundException, BadRequestException, } from '@nestjs/common'; +import { USER_STATISTICS_TTL_SECONDS, UsersCacheService } from './users-cache.service'; import * as crypto from 'crypto'; import * as fs from 'fs'; import * as path from 'path'; @@ -32,6 +34,7 @@ export class UsersService implements OnModuleInit { constructor( private readonly prisma: PrismaService, private readonly sessionsService: SessionsService, + @Optional() private readonly usersCache?: UsersCacheService, ) {} async getProfile(userId: string): Promise { @@ -209,13 +212,22 @@ export class UsersService implements OnModuleInit { }); } + /** Cached for 60 seconds per user; statistics may be that stale. Cache failures fall back to the database. */ async getUserStatistics(userId: string) { + if (!this.usersCache) return this.loadUserStatistics(userId); + return this.usersCache.remember(this.usersCache.statisticsKey(userId), USER_STATISTICS_TTL_SECONDS, () => + this.loadUserStatistics(userId), + ); + } + + private async loadUserStatistics(userId: string) { + // Count in the database instead of loading every property and transaction row. const user = await this.prisma.user.findUnique({ where: { id: userId }, - include: { - properties: true, - buyerTransactions: true, - sellerTransactions: true, + select: { + createdAt: true, + lastActivityAt: true, + _count: { select: { properties: true, buyerTransactions: true, sellerTransactions: true } }, }, }); @@ -223,8 +235,8 @@ export class UsersService implements OnModuleInit { throw new NotFoundException('User not found'); } - const propertiesCount = user.properties.length; - const transactionsCount = user.buyerTransactions.length + user.sellerTransactions.length; + const propertiesCount = user._count.properties; + const transactionsCount = user._count.buyerTransactions + user._count.sellerTransactions; const now = new Date(); const createdAt = new Date(user.createdAt); @@ -885,6 +897,8 @@ export class UsersService implements OnModuleInit { orderBy: { timestamp: 'desc', }, + // Newest 100 entries: the history grows without bound, and one response should not. + take: 100, select: { timestamp: true, ipAddress: true,