diff --git a/docs/api/users-input-sanitization.md b/docs/api/users-input-sanitization.md new file mode 100644 index 00000000..9a9118df --- /dev/null +++ b/docs/api/users-input-sanitization.md @@ -0,0 +1,9 @@ +# Users API input sanitization + +`CreateUserDto`, `UpdateUserDto` and the admin `SearchUsersDto` normalise input through class-transformer (the global `ValidationPipe` already runs with `transform: true`): + +- `email` (and the search `email` filter): Unicode NFKC normalisation, control characters removed, surrounding whitespace trimmed. The case is **not** changed, so existing accounts keep matching. +- `firstName`, `lastName`, search `q` and `name`: additionally strip HTML tags and collapse whitespace. +- Passwords are never altered. + +Sanitising happens before validation, so a name made only of tags becomes empty. Helpers live in `src/users/sanitize.util.ts`. diff --git a/docs/api/users-openapi.md b/docs/api/users-openapi.md new file mode 100644 index 00000000..f4796210 --- /dev/null +++ b/docs/api/users-openapi.md @@ -0,0 +1,5 @@ +# Users API OpenAPI documentation + +The main `UsersController` operations now carry `@ApiOperation` summaries and `@ApiResponse` entries (success plus the documented 400/401/403/404 cases): create, list, search, get by id, update, `me/statistics`, `me/profile` (get and update) and `me/login-history`. + +Open the Swagger UI exposed by the app and look under the **Users** tag. Protected operations show the `access-token` bearer scheme through the existing `@ApiBearerAuth` decorator. diff --git a/docs/api/users-refactor.md b/docs/api/users-refactor.md new file mode 100644 index 00000000..56257e04 --- /dev/null +++ b/docs/api/users-refactor.md @@ -0,0 +1,5 @@ +# Users API refactor: user search + +`UsersService.search` mixed query building with paging and was over 100 lines. The filter construction moved to `buildSearchWhere`, which uses one `contains` helper instead of repeating the case-insensitive match object eleven times. + +Behaviour is unchanged, including the existing quirk that a single-word `name` replaces any `q` match (a multi-word name uses `AND` of first and last name instead). `search` itself now only paginates and shapes the response. diff --git a/docs/api/users-retry.md b/docs/api/users-retry.md new file mode 100644 index 00000000..02f7471d --- /dev/null +++ b/docs/api/users-retry.md @@ -0,0 +1,5 @@ +# Users API retry with backoff + +`retryWithBackoff` (`src/users/retry.util.ts`) retries a failing call with exponential backoff (doubling from the base delay, capped, with up to 50% jitter) and rethrows the last error when attempts run out. + +Used for the email-change verification emails (initial send and resend): up to 3 attempts, so a transient mail-provider error does not lose the email. If every attempt fails the existing behaviour applies: the error is logged and the request still succeeds, as before. diff --git a/src/users/dto/user.dto.ts b/src/users/dto/user.dto.ts index d5105bcc..c07bda17 100644 --- a/src/users/dto/user.dto.ts +++ b/src/users/dto/user.dto.ts @@ -10,7 +10,8 @@ import { Min, Max, } from 'class-validator'; -import { Type } from 'class-transformer'; +import { Transform, Type } from 'class-transformer'; +import { CleanText, TrimIdentifier } from '../sanitize.util'; import { InputType, Field } from '@nestjs/graphql'; export class UpdatePreferencesDto { @@ -35,6 +36,7 @@ export class UpdatePreferencesDto { } export class CreateUserDto { + @Transform(TrimIdentifier) @IsEmail() email: string; @@ -42,9 +44,11 @@ export class CreateUserDto { @MinLength(8) password: string; + @Transform(CleanText) @IsString() firstName: string; + @Transform(CleanText) @IsString() lastName: string; @@ -79,16 +83,19 @@ export class CreateUserDto { @InputType() export class UpdateUserDto { @Field({ nullable: true }) + @Transform(TrimIdentifier) @IsOptional() @IsEmail() email?: string; @Field({ nullable: true }) + @Transform(CleanText) @IsOptional() @IsString() firstName?: string; @Field({ nullable: true }) + @Transform(CleanText) @IsOptional() @IsString() lastName?: string; @@ -114,14 +121,17 @@ export class UpdateUserDto { timezone?: string; } export class SearchUsersDto { + @Transform(CleanText) @IsOptional() @IsString() q?: string; + @Transform(TrimIdentifier) @IsOptional() @IsString() email?: string; + @Transform(CleanText) @IsOptional() @IsString() name?: string; diff --git a/src/users/email-verification.service.ts b/src/users/email-verification.service.ts index 6be8662c..79d4566d 100644 --- a/src/users/email-verification.service.ts +++ b/src/users/email-verification.service.ts @@ -1,4 +1,5 @@ import { Injectable, BadRequestException, NotFoundException, Logger } from '@nestjs/common'; +import { retryWithBackoff } from './retry.util'; import { PrismaService } from '../database/prisma.service'; import { ChangeEmailDto } from './dto/email-change.dto'; import { randomBytes } from 'crypto'; @@ -54,15 +55,16 @@ export class EmailVerificationService { }); // Send verification email with token - await this.emailService - .sendEmail({ + await retryWithBackoff(() => + this.emailService.sendEmail({ to: data.newEmail, subject: 'Verify your email - PropChain', template: 'email-verification', context: { token }, userId: userId, emailType: 'email_verification', - }) + }), + ) .catch((err) => { // Fail quietly but log this.logger.error('Failed to queue verification email:', err?.message || err); @@ -103,15 +105,16 @@ export class EmailVerificationService { }); // Send verification email - await this.emailService - .sendEmail({ + await retryWithBackoff(() => + this.emailService.sendEmail({ to: user.pendingEmail, subject: 'Verify your email - PropChain', template: 'email-verification', context: { token }, userId: userId, emailType: 'email_verification', - }) + }), + ) .catch((err) => { this.logger.error('Failed to queue verification email:', err?.message || err); }); diff --git a/src/users/retry.util.ts b/src/users/retry.util.ts new file mode 100644 index 00000000..013e122c --- /dev/null +++ b/src/users/retry.util.ts @@ -0,0 +1,49 @@ +export interface RetryOptions { + /** Total attempts including the first. Default 3. */ + attempts?: number; + /** Delay before the first retry in ms; doubles each retry. Default 200. */ + baseDelayMs?: number; + /** Ceiling on any single delay in ms. Default 2000. */ + maxDelayMs?: number; + /** Return false to stop retrying immediately. Default: retry everything. */ + isRetryable?: (error: unknown) => boolean; + /** Injectable for tests. */ + sleep?: (ms: number) => Promise; + /** Injectable for tests; returns a value in [0, 1). */ + random?: () => number; +} + +const defaultSleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +/** Delay before retry number `retry` (1-based): exponential, capped, with up to 50% jitter. */ +export function backoffDelayMs(retry: number, baseDelayMs: number, maxDelayMs: number, random: () => number): number { + const exponential = Math.min(maxDelayMs, baseDelayMs * 2 ** (retry - 1)); + return Math.round(exponential * (0.5 + random() * 0.5)); +} + +/** + * Run `fn`, retrying failures with exponential backoff and jitter. The last + * error is rethrown once attempts run out or `isRetryable` says to stop. + */ +export async function retryWithBackoff(fn: () => Promise, options: RetryOptions = {}): Promise { + const { + attempts = 3, + baseDelayMs = 200, + maxDelayMs = 2_000, + isRetryable = () => true, + sleep = defaultSleep, + random = Math.random, + } = options; + + let lastError: unknown; + for (let attempt = 1; attempt <= Math.max(1, attempts); attempt++) { + try { + return await fn(); + } catch (error) { + lastError = error; + if (attempt >= attempts || !isRetryable(error)) break; + await sleep(backoffDelayMs(attempt, baseDelayMs, maxDelayMs, random)); + } + } + throw lastError; +} diff --git a/src/users/sanitize.util.ts b/src/users/sanitize.util.ts new file mode 100644 index 00000000..0d17ebe1 --- /dev/null +++ b/src/users/sanitize.util.ts @@ -0,0 +1,28 @@ +import { TransformFnParams } from 'class-transformer'; + +// Matches C0/C1 control characters (including NUL) except tab/newline, which +// collapse into spaces for free text. +// eslint-disable-next-line no-control-regex +const CONTROL_CHARS = /[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F-\u009F]/g; +const HTML_TAGS = /<[^>]*>/g; + +/** Trim and strip control characters. Safe for identifiers such as email addresses. */ +export function sanitizeIdentifier(value: unknown): unknown { + if (typeof value !== 'string') return value; + return value.normalize('NFKC').replace(CONTROL_CHARS, '').trim(); +} + +/** Free text such as names: also removes HTML tags and collapses whitespace. */ +export function sanitizeText(value: unknown): unknown { + if (typeof value !== 'string') return value; + return value + .normalize('NFKC') + .replace(CONTROL_CHARS, '') + .replace(HTML_TAGS, '') + .replace(/\s+/g, ' ') + .trim(); +} + +/** class-transformer callbacks for DTO fields. Passwords and tokens are never sanitized. */ +export const TrimIdentifier = ({ value }: TransformFnParams): unknown => sanitizeIdentifier(value); +export const CleanText = ({ value }: TransformFnParams): unknown => sanitizeText(value); diff --git a/src/users/users.controller.ts b/src/users/users.controller.ts index 0a02dc90..b3e75ff7 100644 --- a/src/users/users.controller.ts +++ b/src/users/users.controller.ts @@ -28,7 +28,7 @@ import { Roles } from '../auth/decorators/roles.decorator'; import { CurrentUser } from '../auth/decorators/current-user.decorator'; import { AuthUserPayload } from '../auth/types/auth-user.type'; import { UserRole } from '../types/prisma.types'; -import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { ApiBearerAuth, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { UsersService } from './users.service'; import { ActivityLogService } from './activity-log.service'; import { UsersTraceInterceptor } from './interceptors/users-trace.interceptor'; @@ -130,6 +130,11 @@ export class UsersController { @UseGuards(JwtAuthGuard, RolesGuard, UsersRouteRateLimitGuard) @UsersRouteLimit({ max: 10, windowMs: 60 * 60 * 1000 }) @Roles(UserRole.ADMIN) + @ApiOperation({ summary: 'Create a user (admin)' }) + @ApiResponse({ status: 201, description: 'User created' }) + @ApiResponse({ status: 400, description: 'Validation failed' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 403, description: 'Admin role required' }) @Post() create(@Body(UserInjectionGuardPipe) createUserDto: CreateUserDto) { return this.usersService.create(createUserDto); @@ -137,6 +142,10 @@ export class UsersController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) + @ApiOperation({ summary: 'List active users (admin)' }) + @ApiResponse({ status: 200, description: 'Array of users, or { data, meta } when page/limit are given' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 403, description: 'Admin role required' }) @Get() findAll(@Query() query: UsersPaginationQueryDto) { return this.usersService.findAll(query); @@ -145,6 +154,11 @@ export class UsersController { @UseGuards(JwtAuthGuard, RolesGuard, UsersRouteRateLimitGuard) @UsersRouteLimit({ max: 60, windowMs: 60 * 1000 }) @Roles(UserRole.ADMIN) + @ApiOperation({ summary: 'Search users (admin)' }) + @ApiResponse({ status: 200, description: '{ data, meta } page of matching users' }) + @ApiResponse({ status: 400, description: 'Invalid query' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 403, description: 'Admin role required' }) @Get('search') search(@Query(UserInjectionGuardPipe) query: SearchUsersDto) { return this.usersService.search(query); @@ -159,6 +173,11 @@ export class UsersController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) + @ApiOperation({ summary: 'Get a user by id (admin)' }) + @ApiResponse({ status: 200, description: 'The user' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 403, description: 'Admin role required' }) + @ApiResponse({ status: 404, description: 'User not found' }) @Get(':id') findOne(@Param('id') id: string) { return this.usersService.findOne(id); @@ -166,6 +185,12 @@ export class UsersController { @UseGuards(JwtAuthGuard, RolesGuard) @Roles(UserRole.ADMIN) + @ApiOperation({ summary: 'Update a user (admin)' }) + @ApiResponse({ status: 200, description: 'The updated user' }) + @ApiResponse({ status: 400, description: 'Validation failed' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 403, description: 'Admin role required' }) + @ApiResponse({ status: 404, description: 'User not found' }) @Put(':id') update(@Param('id') id: string, @Body(UserInjectionGuardPipe) updateUserDto: UpdateUserDto) { return this.usersService.update(id, updateUserDto); @@ -195,12 +220,20 @@ export class UsersController { // ─── Profile Management (#306) ─────────────────────────────────── @UseGuards(JwtAuthGuard) + @ApiOperation({ summary: 'Get the caller's profile' }) + @ApiResponse({ status: 200, description: 'Profile with statistics' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 404, description: 'User not found' }) @Get('me/profile') getProfile(@CurrentUser() user: AuthUserPayload) { return this.usersService.getProfile(user.sub); } @UseGuards(JwtAuthGuard) + @ApiOperation({ summary: 'Update the caller's profile' }) + @ApiResponse({ status: 200, description: 'The updated profile' }) + @ApiResponse({ status: 400, description: 'Validation failed' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) @Put('me/profile') updateProfile(@CurrentUser() user: AuthUserPayload, @Body(UserInjectionGuardPipe) updateProfileDto: UpdateProfileDto) { return this.usersService.updateProfile(user.sub, updateProfileDto); diff --git a/src/users/users.service.ts b/src/users/users.service.ts index 31e9ff66..0622722c 100644 --- a/src/users/users.service.ts +++ b/src/users/users.service.ts @@ -700,80 +700,46 @@ export class UsersService implements OnModuleInit { }); } - async search(filters: SearchUsersDto) { - const { q, email, name, page = 1, limit = 10 } = filters; - const skip = (page - 1) * limit; - - const where: Prisma.UserWhereInput = { - isDeactivated: false, - }; + /** + * Builds the Prisma filter for the admin user search: active users only, with + * a free-text match over email and names (`q`), a partial email match, and a + * name match ("First Last" matches both parts, a single word matches either). + */ + private buildSearchWhere(filters: { q?: string; email?: string; name?: string }): Prisma.UserWhereInput { + const { q, email, name } = filters; + const contains = (value: string) => ({ contains: value, mode: 'insensitive' as const }); + const where: Prisma.UserWhereInput = { isDeactivated: false }; if (q) { - where.OR = [ - { - email: { - contains: q, - mode: 'insensitive', - }, - }, - { - firstName: { - contains: q, - mode: 'insensitive', - }, - }, - { - lastName: { - contains: q, - mode: 'insensitive', - }, - }, - ]; + where.OR = [{ email: contains(q) }, { firstName: contains(q) }, { lastName: contains(q) }]; } if (email) { - where.email = { - contains: email, - mode: 'insensitive', - }; + where.email = contains(email); } if (name) { const nameParts = name.split(' '); - if (nameParts.length > 1) { where.AND = [ - { - firstName: { - contains: nameParts[0], - mode: 'insensitive', - }, - }, - { - lastName: { - contains: nameParts[nameParts.length - 1], - mode: 'insensitive', - }, - }, + { firstName: contains(nameParts[0]) }, + { lastName: contains(nameParts[nameParts.length - 1]) }, ]; } else { - where.OR = [ - { - firstName: { - contains: name, - mode: 'insensitive', - }, - }, - { - lastName: { - contains: name, - mode: 'insensitive', - }, - }, - ]; + // As before, a single-word name replaces any `q` match. + where.OR = [{ firstName: contains(name) }, { lastName: contains(name) }]; } } + return where; + } + + async search(filters: SearchUsersDto) { + const { q, email, name, page = 1, limit = 10 } = filters; + const skip = (page - 1) * limit; + + const where = this.buildSearchWhere({ q, email, name }); + const [users, total] = await Promise.all([ this.prisma.user.findMany({ where,