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/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 0c3848dd..0a02dc90 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -127,7 +127,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(UserInjectionGuardPipe) createUserDto: CreateUserDto) { @@ -141,14 +142,16 @@ export class UsersController { return this.usersService.findAll(query); } - @UseGuards(JwtAuthGuard, RolesGuard) + @UseGuards(JwtAuthGuard, RolesGuard, UsersRouteRateLimitGuard) + @UsersRouteLimit({ max: 60, windowMs: 60 * 1000 }) @Roles(UserRole.ADMIN) @Get('search') search(@Query(UserInjectionGuardPipe) 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); @@ -392,7 +395,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.service.ts b/src/users/users.service.ts index 40eec418..31e9ff66 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -2,6 +2,7 @@ import { Injectable, Logger, OnModuleInit, + Optional, NotFoundException, BadRequestException, } from '@nestjs/common'; @@ -33,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 { @@ -210,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 } }, }, }); @@ -224,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); @@ -904,6 +915,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,