From 846195b759ecc4273bae63640fa2f5323391f134 Mon Sep 17 00:00:00 2001 From: demilade18-git Date: Fri, 16 Oct 2026 22:40:06 +0100 Subject: [PATCH] feat(auth): OpenAPI docs, input sanitization, password refactor, retry with backoff --- docs/api/auth-input-sanitization.md | 9 ++ docs/api/auth-openapi.md | 5 + docs/api/auth-refactor.md | 9 ++ docs/api/auth-retry.md | 10 ++ src/auth/auth.controller.ts | 37 ++++++- src/auth/auth.service.ts | 163 ++++++++++++++-------------- src/auth/dto/auth.dto.ts | 6 + src/auth/retry.util.ts | 57 ++++++++++ src/auth/sanitize.util.ts | 31 ++++++ 9 files changed, 244 insertions(+), 83 deletions(-) create mode 100644 docs/api/auth-input-sanitization.md create mode 100644 docs/api/auth-openapi.md create mode 100644 docs/api/auth-refactor.md create mode 100644 docs/api/auth-retry.md create mode 100644 src/auth/retry.util.ts create mode 100644 src/auth/sanitize.util.ts diff --git a/docs/api/auth-input-sanitization.md b/docs/api/auth-input-sanitization.md new file mode 100644 index 00000000..6c71d7cb --- /dev/null +++ b/docs/api/auth-input-sanitization.md @@ -0,0 +1,9 @@ +# Auth API input sanitization + +`RegisterDto` and `LoginDto` normalise input through class-transformer (the global `ValidationPipe` already runs with `transform: true`): + +- `email`: Unicode NFKC normalisation, control characters removed, surrounding whitespace trimmed. The case is **not** changed, so existing accounts keep matching. +- `firstName` / `lastName`: additionally strip HTML tags and collapse whitespace. +- Passwords and tokens are never altered. + +Sanitising happens before validation, so `" user@example.com "` is accepted as `user@example.com` and a name consisting only of tags becomes empty and fails `@IsNotEmpty`. Helpers live in `src/auth/sanitize.util.ts`. diff --git a/docs/api/auth-openapi.md b/docs/api/auth-openapi.md new file mode 100644 index 00000000..c82aee5d --- /dev/null +++ b/docs/api/auth-openapi.md @@ -0,0 +1,5 @@ +# Auth API OpenAPI documentation + +The main `AuthController` operations now carry `@ApiOperation` summaries and `@ApiResponse` entries (success plus the documented 400/401/404/429 cases) so they appear described in Swagger: register, login, refresh, logout, logout-all, me, change-password, password-reset request/reset and verify-email. + +Open the Swagger UI exposed by the app and look under the **Authentication** tag. Protected operations show the `access-token` bearer scheme through the existing `@ApiBearerAuth` decorators. diff --git a/docs/api/auth-refactor.md b/docs/api/auth-refactor.md new file mode 100644 index 00000000..7e1f31fd --- /dev/null +++ b/docs/api/auth-refactor.md @@ -0,0 +1,9 @@ +# Auth API refactor: password handling + +`changePassword` and `resetPassword` duplicated three blocks. They now share private helpers in `AuthService`: + +- `assertPasswordMeetsPolicy` - complexity check, 400 listing the broken rules, +- `assertPasswordNotReused` - compares against recent hashes, 400 if reused, +- `recordPasswordHistory` - stores the new hash and trims history beyond the retention limit. + +Behaviour, error messages and the transaction boundaries are unchanged; each method is roughly 25 lines shorter and the reuse and retention rules now live in one place. diff --git a/docs/api/auth-retry.md b/docs/api/auth-retry.md new file mode 100644 index 00000000..a773cfc2 --- /dev/null +++ b/docs/api/auth-retry.md @@ -0,0 +1,10 @@ +# Auth API retry with backoff + +`retryWithBackoff` (`src/auth/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 or `isRetryable` says stop. + +Used for: + +- the reCAPTCHA verification request: up to 3 attempts, retried on network errors, `429` and `5xx` (other responses are final). If it still fails the existing behaviour applies (verification fails closed). +- sending the password reset email: up to 3 attempts, so a transient mail-provider error does not silently lose the email. Persistent failure still propagates as before. + +Login and token operations are not retried: they are not idempotent from the caller's side and failures there are authoritative. diff --git a/src/auth/auth.controller.ts b/src/auth/auth.controller.ts index 39700d72..05a62937 100644 --- a/src/auth/auth.controller.ts +++ b/src/auth/auth.controller.ts @@ -1,5 +1,5 @@ import { Body, Controller, Delete, Get, Param, Patch, Post, Req, UseGuards } from '@nestjs/common'; -import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { ApiBearerAuth, ApiOperation, ApiResponse, ApiTags } from '@nestjs/swagger'; import { AuthService } from './auth.service'; import { ApiKeyAnalyticsService } from './api-key-analytics.service'; import { @@ -39,6 +39,10 @@ export class AuthController { private readonly apiKeyAnalyticsService: ApiKeyAnalyticsService, ) {} + @ApiOperation({ summary: 'Register a new account' }) + @ApiResponse({ status: 201, description: 'Account created; verification email sent' }) + @ApiResponse({ status: 400, description: 'Validation failed or email already registered' }) + @ApiResponse({ status: 429, description: 'Too many registration attempts' }) @Post('register') register(@Body() registerDto: RegisterDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -57,6 +61,11 @@ export class AuthController { return this.authService.googleOAuthLogin(req.user); } + @ApiOperation({ summary: 'Log in with email and password' }) + @ApiResponse({ status: 201, description: 'Access and refresh tokens (or a 2FA challenge)' }) + @ApiResponse({ status: 400, description: 'Validation failed' }) + @ApiResponse({ status: 401, description: 'Invalid credentials or locked account' }) + @ApiResponse({ status: 429, description: 'Too many attempts' }) @Post('login') login(@Body() loginDto: LoginDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -64,6 +73,9 @@ export class AuthController { return this.authService.login(loginDto, ipAddress, userAgent); } + @ApiOperation({ summary: 'Exchange a refresh token for a new token pair' }) + @ApiResponse({ status: 201, description: 'New access and refresh tokens' }) + @ApiResponse({ status: 401, description: 'Refresh token invalid, expired or reused' }) @Post('refresh') refresh(@Body() refreshTokenDto: RefreshTokenDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; @@ -73,6 +85,9 @@ export class AuthController { @ApiBearerAuth('access-token') @UseGuards(JwtAuthGuard) + @ApiOperation({ summary: 'Log out the current session' }) + @ApiResponse({ status: 201, description: 'Tokens revoked' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) @Post('logout') logout( @CurrentUser() user: AuthUserPayload, @@ -84,6 +99,9 @@ export class AuthController { @ApiBearerAuth('access-token') @UseGuards(JwtAuthGuard) + @ApiOperation({ summary: 'Log out of every device' }) + @ApiResponse({ status: 201, description: 'All sessions revoked' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) @Post('logout-all') logoutAllDevices(@CurrentUser() user: AuthUserPayload, @Req() request: { accessToken?: string }) { return this.authService.logoutAllDevices(user, request.accessToken); @@ -91,6 +109,10 @@ export class AuthController { @ApiBearerAuth('access-token') @UseGuards(JwtAuthGuard) + @ApiOperation({ summary: 'Get the authenticated user's profile' }) + @ApiResponse({ status: 200, description: 'Sanitised user profile' }) + @ApiResponse({ status: 401, description: 'Not authenticated' }) + @ApiResponse({ status: 404, description: 'User not found' }) @Get('me') me(@CurrentUser() user: AuthUserPayload) { return this.authService.me(user); @@ -98,6 +120,10 @@ export class AuthController { @ApiBearerAuth('access-token') @UseGuards(JwtAuthGuard, FreshTwoFactorGuard) + @ApiOperation({ summary: 'Change the account password' }) + @ApiResponse({ status: 201, description: 'Password updated; other sessions revoked' }) + @ApiResponse({ status: 400, description: 'Weak or recently used password' }) + @ApiResponse({ status: 401, description: 'Current password incorrect' }) @Post('change-password') changePassword( @CurrentUser() user: AuthUserPayload, @@ -223,11 +249,17 @@ export class AuthController { return this.apiKeyAnalyticsService.getUsageAnalytics(id, user.sub); } + @ApiOperation({ summary: 'Request a password reset email' }) + @ApiResponse({ status: 201, description: 'Request accepted (response does not reveal whether the email exists)' }) + @ApiResponse({ status: 400, description: 'Validation failed' }) @Post('password-reset/request') requestPasswordReset(@Body() requestPasswordResetDto: RequestPasswordResetDto) { return this.authService.requestPasswordReset(requestPasswordResetDto); } + @ApiOperation({ summary: 'Reset the password with an emailed token' }) + @ApiResponse({ status: 201, description: 'Password reset' }) + @ApiResponse({ status: 400, description: 'Invalid, used or expired token, or weak password' }) @Post('password-reset/reset') resetPassword(@Body() resetPasswordDto: ResetPasswordDto) { return this.authService.resetPassword(resetPasswordDto); @@ -246,6 +278,9 @@ export class AuthController { return this.authService.getLoginStatus(data.email); } + @ApiOperation({ summary: 'Verify an email address with the emailed token' }) + @ApiResponse({ status: 201, description: 'Email verified and tokens issued' }) + @ApiResponse({ status: 400, description: 'Invalid or expired token' }) @Post('verify-email') verifyEmail(@Body() verifyEmailDto: VerifyEmailDto, @Req() request: Request) { const ipAddress = request.ip || request.socket.remoteAddress; diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index 4078c663..9526ee59 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -7,8 +7,9 @@ import { ServiceUnavailableException, UnauthorizedException, } from '@nestjs/common'; +import { TransientError, retryWithBackoff } from './retry.util'; import { ConfigService } from '@nestjs/config'; -import { Prisma, BlacklistedToken, Document, ApiKey, PasswordHistory } from '@prisma/client'; +import { Prisma, BlacklistedToken, Document, ApiKey } from '@prisma/client'; import { randomUUID } from 'crypto'; import * as jwt from 'jsonwebtoken'; import { PrismaService } from '../database/prisma.service'; @@ -890,6 +891,51 @@ export class AuthService { }; } + /** Throws a 400 listing every complexity rule the password breaks. */ + private assertPasswordMeetsPolicy(newPassword: string): void { + const passwordErrors = validatePassword(newPassword, this.configService); + if (passwordErrors.length > 0) { + throw new BadRequestException( + `Password does not meet complexity requirements: ${passwordErrors.join('; ')}`, + ); + } + } + + /** Throws a 400 if `newPassword` matches any of the recent password hashes. */ + private async assertPasswordNotReused( + newPassword: string, + recentHashes: string[], + historyLimit: number, + ): Promise { + const reused = await Promise.all(recentHashes.map((hash) => comparePassword(newPassword, hash))); + if (reused.some(Boolean)) { + throw new BadRequestException( + `Password reuse is not allowed for the last ${historyLimit} passwords`, + ); + } + } + + /** Stores the new hash and deletes history entries beyond the retention limit. */ + private async recordPasswordHistory( + tx: Prisma.TransactionClient, + userId: string, + passwordHash: string, + historyLimit: number, + ): Promise { + await tx.passwordHistory.create({ data: { userId, passwordHash } }); + + const expired = await tx.passwordHistory.findMany({ + where: { userId }, + orderBy: { createdAt: 'desc' }, + skip: historyLimit, + }); + if (expired.length > 0) { + await tx.passwordHistory.deleteMany({ + where: { id: { in: expired.map((entry: { id: string }) => entry.id) } }, + }); + } + } + async changePassword(user: AuthUserPayload, data: ChangePasswordDto) { const passwordHistoryLimit = this.getPasswordHistoryLimit(); const existingUser = await this.prisma.user.findUnique({ @@ -913,27 +959,15 @@ export class AuthService { throw new UnauthorizedException('Current password is incorrect'); } - const passwordErrors = validatePassword(data.newPassword, this.configService); - if (passwordErrors.length > 0) { - throw new BadRequestException( - `Password does not meet complexity requirements: ${passwordErrors.join('; ')}`, - ); - } - - const passwordReused = await Promise.all( + this.assertPasswordMeetsPolicy(data.newPassword); + await this.assertPasswordNotReused( + data.newPassword, existingUser.passwordHistory .slice(0, passwordHistoryLimit) - .map((entry: { passwordHash: string }) => - comparePassword(data.newPassword, entry.passwordHash), - ), + .map((entry: { passwordHash: string }) => entry.passwordHash), + passwordHistoryLimit, ); - if (passwordReused.some(Boolean)) { - throw new BadRequestException( - `Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`, - ); - } - const newPasswordHash = await hashPassword(data.newPassword, this.bcryptRounds); await this.prisma.$transaction(async (tx: Prisma.TransactionClient) => { @@ -944,28 +978,7 @@ export class AuthService { }, }); - await tx.passwordHistory.create({ - data: { - userId: existingUser.id, - passwordHash: newPasswordHash, - }, - }); - - const historyEntries = await tx.passwordHistory.findMany({ - where: { userId: existingUser.id }, - orderBy: { createdAt: 'desc' }, - skip: passwordHistoryLimit, - }); - - if (historyEntries.length > 0) { - await tx.passwordHistory.deleteMany({ - where: { - id: { - in: historyEntries.map((entry: { id: string }) => entry.id), - }, - }, - }); - } + await this.recordPasswordHistory(tx, existingUser.id, newPasswordHash, passwordHistoryLimit); }); // Audit log password change (#886) @@ -1672,7 +1685,12 @@ export class AuthService { }); // Send reset email with raw token (not the hash) - await this.emailService.sendPasswordResetEmail(user.email, resetToken); + // Retried with backoff: a transient mail-provider failure should not lose the reset email. + await retryWithBackoff(() => this.emailService.sendPasswordResetEmail(user.email, resetToken), { + attempts: 3, + baseDelayMs: 300, + maxDelayMs: 2_000, + }); } async resetPassword(data: ResetPasswordDto): Promise { @@ -1700,12 +1718,7 @@ export class AuthService { const passwordHistoryLimit = this.getPasswordHistoryLimit(); - const passwordErrors = validatePassword(data.newPassword, this.configService); - if (passwordErrors.length > 0) { - throw new BadRequestException( - `Password does not meet complexity requirements: ${passwordErrors.join('; ')}`, - ); - } + this.assertPasswordMeetsPolicy(data.newPassword); // Check if new password was used recently const recentPasswords = await this.prisma.passwordHistory.findMany({ @@ -1713,15 +1726,11 @@ export class AuthService { orderBy: { createdAt: 'desc' }, take: passwordHistoryLimit, }); - - const reuseResults = await Promise.all( - recentPasswords.map((entry) => comparePassword(data.newPassword, entry.passwordHash)), + await this.assertPasswordNotReused( + data.newPassword, + recentPasswords.map((entry) => entry.passwordHash), + passwordHistoryLimit, ); - if (reuseResults.some(Boolean)) { - throw new BadRequestException( - `Password reuse is not allowed for the last ${passwordHistoryLimit} passwords`, - ); - } const newPasswordHash = await hashPassword(data.newPassword, this.bcryptRounds); @@ -1737,27 +1746,7 @@ export class AuthService { data: { usedAt: new Date() }, }); - await tx.passwordHistory.create({ - data: { - userId: resetToken.userId, - passwordHash: newPasswordHash, - }, - }); - - // Clean up old password history entries - const historyEntries = await tx.passwordHistory.findMany({ - where: { userId: resetToken.userId }, - orderBy: { createdAt: 'desc' }, - skip: passwordHistoryLimit, - }); - - if (historyEntries.length > 0) { - await tx.passwordHistory.deleteMany({ - where: { - id: { in: historyEntries.map((entry: PasswordHistory) => entry.id) }, - }, - }); - } + await this.recordPasswordHistory(tx, resetToken.userId, newPasswordHash, passwordHistoryLimit); }); await this.sessionsService.revokeAllSessions(resetToken.userId); @@ -1827,13 +1816,23 @@ export class AuthService { } try { - const response = await fetch('https://www.google.com/recaptcha/api/siteverify', { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', + // Retried with backoff on network errors, 429 and 5xx; other responses are final. + const response = await retryWithBackoff( + async () => { + const res = await fetch('https://www.google.com/recaptcha/api/siteverify', { + method: 'POST', + headers: { + 'Content-Type': 'application/x-www-form-urlencoded', + }, + body: `secret=${secret}&response=${token}`, + }); + if (res.status === 429 || res.status >= 500) { + throw new TransientError(`reCAPTCHA responded ${res.status}`); + } + return res; }, - body: `secret=${secret}&response=${token}`, - }); + { attempts: 3, baseDelayMs: 200, maxDelayMs: 1_500 }, + ); const data = (await response.json()) as RecaptchaVerifyResponse; diff --git a/src/auth/dto/auth.dto.ts b/src/auth/dto/auth.dto.ts index 84c2875c..920e68d4 100644 --- a/src/auth/dto/auth.dto.ts +++ b/src/auth/dto/auth.dto.ts @@ -10,8 +10,11 @@ import { MinLength, Matches, } from 'class-validator'; +import { Transform } from 'class-transformer'; +import { CleanText, TrimIdentifier } from '../sanitize.util'; export class RegisterDto { + @Transform(TrimIdentifier) @IsEmail() email: string; @@ -23,10 +26,12 @@ export class RegisterDto { }) password: string; + @Transform(CleanText) @IsString() @IsNotEmpty() firstName: string; + @Transform(CleanText) @IsString() @IsNotEmpty() lastName: string; @@ -37,6 +42,7 @@ export class RegisterDto { } export class LoginDto { + @Transform(TrimIdentifier) @IsEmail() email: string; diff --git a/src/auth/retry.util.ts b/src/auth/retry.util.ts new file mode 100644 index 00000000..e06ba83d --- /dev/null +++ b/src/auth/retry.util.ts @@ -0,0 +1,57 @@ +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 (e.g. for client errors). 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; +} + +/** Marks a failure as safe to retry (e.g. a 5xx or 429 from an upstream service). */ +export class TransientError extends Error { + constructor(message: string) { + super(message); + this.name = 'TransientError'; + } +} + +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/auth/sanitize.util.ts b/src/auth/sanitize.util.ts new file mode 100644 index 00000000..5f3b49db --- /dev/null +++ b/src/auth/sanitize.util.ts @@ -0,0 +1,31 @@ +import { TransformFnParams } from 'class-transformer'; + +// Matches C0/C1 control characters (including NUL) except tab/newline, which +// collapse into spaces below. +// 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 + * intentionally never sanitized: altering them would change the secret. + */ +export const TrimIdentifier = ({ value }: TransformFnParams): unknown => sanitizeIdentifier(value); +export const CleanText = ({ value }: TransformFnParams): unknown => sanitizeText(value);