From 44abb24ed346532d520be30eac15f4f0d8055dd5 Mon Sep 17 00:00:00 2001 From: northersubair Date: Tue, 25 Aug 2026 17:25:27 +0100 Subject: [PATCH] feat: configurable brute-force lockout, email sanitization, localized risk flags, sanitized error responses --- backend/src/auth/brute-force.guard.spec.ts | 84 ++++++++++++++ backend/src/auth/brute-force.guard.ts | 11 +- .../src/auth/dto/register-auth.dto.spec.ts | 41 +++++++ backend/src/auth/dto/register-auth.dto.ts | 3 + .../filters/http-exception.filter.spec.ts | 106 ++++++++++++++++++ .../common/filters/http-exception.filter.ts | 52 +++++++-- .../risk-assessment.controller.ts | 6 +- .../risk-assessment.service.spec.ts | 30 +++++ .../risk-assessment.service.ts | 50 ++++++++- 9 files changed, 364 insertions(+), 19 deletions(-) create mode 100644 backend/src/auth/brute-force.guard.spec.ts create mode 100644 backend/src/auth/dto/register-auth.dto.spec.ts create mode 100644 backend/src/common/filters/http-exception.filter.spec.ts create mode 100644 backend/src/risk-assessment/risk-assessment.service.spec.ts diff --git a/backend/src/auth/brute-force.guard.spec.ts b/backend/src/auth/brute-force.guard.spec.ts new file mode 100644 index 00000000..8a91599d --- /dev/null +++ b/backend/src/auth/brute-force.guard.spec.ts @@ -0,0 +1,84 @@ +import { ConfigService } from '@nestjs/config'; +import { BruteForceGuard } from './brute-force.guard'; + +describe('BruteForceGuard', () => { + it('should be defined', () => { + const guard = new BruteForceGuard({ + get: () => '30', + } as unknown as ConfigService); + expect(guard).toBeDefined(); + }); + + it('should lock account after 5 failed attempts', () => { + const guard = new BruteForceGuard({ + get: () => '15', + } as unknown as ConfigService); + const email = 'target@example.com'; + + for (let i = 0; i < 5; i++) { + guard.recordFailedLogin(email); + } + + const mockCtx = { + switchToHttp: () => ({ + getRequest: () => ({ body: { email } }), + }), + }; + + expect(() => guard.canActivate(mockCtx as any)).toThrow( + 'Account locked due to multiple failed login attempts', + ); + }); + + it('should use configurable lock time from env', () => { + const guard = new BruteForceGuard({ + get: (key: string, defaultVal: string) => + key === 'BRUTE_FORCE_LOCK_MINUTES' ? '30' : defaultVal, + } as unknown as ConfigService); + + const email = 'test@example.com'; + guard.recordFailedLogin(email); + guard.recordFailedLogin(email); + guard.recordFailedLogin(email); + guard.recordFailedLogin(email); + guard.recordFailedLogin(email); + + const record = (guard as any).failedAttempts.get(email); + const lockDurationMs = record.lockUntil - Date.now(); + expect(lockDurationMs).toBeGreaterThan(29 * 60 * 1000); + expect(lockDurationMs).toBeLessThanOrEqual(30 * 60 * 1000); + }); + + it('should reset attempts', () => { + const guard = new BruteForceGuard({ + get: () => '15', + } as unknown as ConfigService); + const email = 'reset@example.com'; + + guard.recordFailedLogin(email); + guard.recordFailedLogin(email); + guard.resetAttempts(email); + + const mockCtx = { + switchToHttp: () => ({ + getRequest: () => ({ body: { email } }), + }), + }; + + expect(() => guard.canActivate(mockCtx as any)).not.toThrow(); + }); + + it('should allow access when no email in body', () => { + const guard = new BruteForceGuard({ + get: () => '15', + } as unknown as ConfigService); + + const mockCtx = { + switchToHttp: () => ({ + getRequest: () => ({ body: {} }), + }), + }; + + expect(guard.canActivate(mockCtx as any)).toBe(true); + }); +}); diff --git a/backend/src/auth/brute-force.guard.ts b/backend/src/auth/brute-force.guard.ts index 461b4632..b00b146a 100644 --- a/backend/src/auth/brute-force.guard.ts +++ b/backend/src/auth/brute-force.guard.ts @@ -4,6 +4,7 @@ import { ExecutionContext, UnauthorizedException, } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; @Injectable() export class BruteForceGuard implements CanActivate { @@ -12,7 +13,15 @@ export class BruteForceGuard implements CanActivate { { count: number; lockUntil?: Date } >(); private readonly MAX_ATTEMPTS = 5; - private readonly LOCK_TIME_MS = 15 * 60 * 1000; // 15 mins + private readonly LOCK_TIME_MS: number; + + constructor(private readonly configService: ConfigService) { + const lockMinutes = parseInt( + this.configService.get('BRUTE_FORCE_LOCK_MINUTES', '15'), + 10, + ); + this.LOCK_TIME_MS = lockMinutes * 60 * 1000; + } canActivate(context: ExecutionContext): boolean { const req = context.switchToHttp().getRequest(); diff --git a/backend/src/auth/dto/register-auth.dto.spec.ts b/backend/src/auth/dto/register-auth.dto.spec.ts new file mode 100644 index 00000000..df063424 --- /dev/null +++ b/backend/src/auth/dto/register-auth.dto.spec.ts @@ -0,0 +1,41 @@ +import { validate } from 'class-validator'; +import { plainToInstance } from 'class-transformer'; +import { RegisterAuthDto } from './register-auth.dto'; + +describe('RegisterAuthDto', () => { + it('should trim and lowercase email', async () => { + const dto = plainToInstance(RegisterAuthDto, { + email: ' Test@EXAMPLE.COM ', + password: 'password123', + fullName: ' John Doe ', + }); + const errors = await validate(dto); + expect(errors.length).toBe(0); + expect(dto.email).toBe('test@example.com'); + expect(dto.fullName).toBe('John Doe'); + }); + + it('should reject invalid email', async () => { + const dto = plainToInstance(RegisterAuthDto, { + email: 'not-an-email', + password: 'password123', + fullName: 'John Doe', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const emailErrors = errors.filter((e) => e.property === 'email'); + expect(emailErrors.length).toBeGreaterThan(0); + }); + + it('should reject short password', async () => { + const dto = plainToInstance(RegisterAuthDto, { + email: 'test@example.com', + password: '12345', + fullName: 'John Doe', + }); + const errors = await validate(dto); + expect(errors.length).toBeGreaterThan(0); + const passwordErrors = errors.filter((e) => e.property === 'password'); + expect(passwordErrors.length).toBeGreaterThan(0); + }); +}); diff --git a/backend/src/auth/dto/register-auth.dto.ts b/backend/src/auth/dto/register-auth.dto.ts index 84c5ad00..cb8fee98 100644 --- a/backend/src/auth/dto/register-auth.dto.ts +++ b/backend/src/auth/dto/register-auth.dto.ts @@ -1,7 +1,9 @@ import { IsEmail, IsNotEmpty, MinLength } from 'class-validator'; +import { Transform } from 'class-transformer'; export class RegisterAuthDto { @IsEmail() + @Transform(({ value }) => value?.trim().toLowerCase()) email: string; @IsNotEmpty() @@ -9,5 +11,6 @@ export class RegisterAuthDto { password: string; @IsNotEmpty() + @Transform(({ value }) => value?.trim()) fullName: string; } diff --git a/backend/src/common/filters/http-exception.filter.spec.ts b/backend/src/common/filters/http-exception.filter.spec.ts new file mode 100644 index 00000000..86567897 --- /dev/null +++ b/backend/src/common/filters/http-exception.filter.spec.ts @@ -0,0 +1,106 @@ +import { HttpExceptionFilter } from './http-exception.filter'; +import { HttpException, HttpStatus } from '@nestjs/common'; + +describe('HttpExceptionFilter', () => { + const mockResponse = () => { + const res: any = {}; + res.status = jest.fn().mockReturnValue(res); + res.json = jest.fn().mockReturnValue(res); + return res; + }; + + const mockRequest = (overrides = {}) => + ({ + method: 'POST', + url: '/auth/login', + requestId: 'req-123', + ...overrides, + }) as any; + + const mockHost = (req: any, res: any) => + ({ + switchToHttp: () => ({ + getRequest: () => req, + getResponse: () => res, + }), + }) as any; + + it('should pass through non-sensitive error messages', () => { + const filter = new HttpExceptionFilter(true); + const res = mockResponse(); + const req = mockRequest(); + const exception = new HttpException( + { message: 'Email already registered' }, + HttpStatus.CONFLICT, + ); + + filter.catch(exception, mockHost(req, res)); + + expect(res.status).toHaveBeenCalledWith(409); + const body = res.json.mock.calls[0][0]; + expect(body.message).toBe('Email already registered'); + }); + + it('should sanitize sensitive error messages in production', () => { + const filter = new HttpExceptionFilter(true); + const res = mockResponse(); + const req = mockRequest(); + const exception = new HttpException( + { message: 'ER_DUP_ENTRY: Duplicate entry for key' }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + + filter.catch(exception, mockHost(req, res)); + + const body = res.json.mock.calls[0][0]; + expect(body.message).toBe( + 'An unexpected error occurred. Please try again later.', + ); + }); + + it('should NOT sanitize in non-production mode', () => { + const filter = new HttpExceptionFilter(false); + const res = mockResponse(); + const req = mockRequest(); + const exception = new HttpException( + { message: 'ER_DUP_ENTRY: Duplicate entry' }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + + filter.catch(exception, mockHost(req, res)); + + const body = res.json.mock.calls[0][0]; + expect(body.message).toBe('ER_DUP_ENTRY: Duplicate entry'); + }); + + it('should handle generic Error instances', () => { + const filter = new HttpExceptionFilter(false); + const res = mockResponse(); + const req = mockRequest(); + const exception = new Error('Something went wrong'); + + filter.catch(exception, mockHost(req, res)); + + expect(res.status).toHaveBeenCalledWith(500); + const body = res.json.mock.calls[0][0]; + expect(body.statusCode).toBe(500); + expect(body.stack).toBeDefined(); + }); + + it('should sanitize database connection errors in production', () => { + const filter = new HttpExceptionFilter(true); + const res = mockResponse(); + const req = mockRequest(); + const exception = new HttpException( + { message: 'connect ECONNREFUSED 127.0.0.1:5432' }, + HttpStatus.INTERNAL_SERVER_ERROR, + ); + + filter.catch(exception, mockHost(req, res)); + + const body = res.json.mock.calls[0][0]; + expect(body.message).toBe( + 'An unexpected error occurred. Please try again later.', + ); + }); +}); diff --git a/backend/src/common/filters/http-exception.filter.ts b/backend/src/common/filters/http-exception.filter.ts index 427c0957..84da0d4e 100644 --- a/backend/src/common/filters/http-exception.filter.ts +++ b/backend/src/common/filters/http-exception.filter.ts @@ -8,6 +8,24 @@ import { } from '@nestjs/common'; import { Response, Request } from 'express'; +const SENSITIVE_ERROR_PATTERNS = [ + /password/i, + /secret/i, + /token/i, + /jwt/i, + /hash/i, + /bcrypt/i, + /e\.?r\.?r\.?o\.?r\.?/i, + /ENOENT/i, + /EACCES/i, + /connect.*refused/i, + /database/i, + /sql/i, + /query.*failed/i, + /constraint/i, + /duplicate/i, +]; + @Catch() export class HttpExceptionFilter implements ExceptionFilter { private readonly logger = new Logger(HttpExceptionFilter.name); @@ -26,21 +44,16 @@ export class HttpExceptionFilter implements ExceptionFilter { const errorResponse = isHttp ? exception.getResponse() - : { message: (exception as Error)?.message }; // fallback message + : { message: 'An unexpected error occurred' }; const { message, error } = this.normalizeResponse(errorResponse, exception); - const requestId = (request as any).requestId || request.headers['x-request-id'] || 'req-id'; const errorCode = (errorResponse as any)?.errorCode || error || `ERR_${status}`; - const requestId = request.requestId || 'unknown'; - const errorCode = (errorResponse as any)?.errorCode || error || `ERR_${status}`; - - - const payload = { + const payload: Record = { statusCode: status, errorCode, message, @@ -50,7 +63,10 @@ export class HttpExceptionFilter implements ExceptionFilter { path: request.url, }; - this.logger.error(`${status} ${request.method} ${request.url} -> ${message}`, (exception as Error)?.stack); + this.logger.error( + `${status} ${request.method} ${request.url} -> ${message}`, + (exception as Error)?.stack, + ); if (!this.isProduction && exception instanceof Error) { Object.assign(payload, { stack: exception.stack }); @@ -67,24 +83,36 @@ export class HttpExceptionFilter implements ExceptionFilter { let error = HttpStatus.INTERNAL_SERVER_ERROR.toString(); if (typeof response === 'string') { - message = response; + message = this.sanitizeMessage(response); } else if (response && typeof response === 'object') { const body = response as Record; if (body.message) { - message = Array.isArray(body.message) + const raw = Array.isArray(body.message) ? body.message.join(', ') : body.message; + message = this.sanitizeMessage(raw); } else if (exception instanceof Error && exception.message) { - message = exception.message; + message = this.sanitizeMessage(exception.message); } if (body.error) { error = body.error; } } else if (exception instanceof Error) { - message = exception.message; + message = this.sanitizeMessage(exception.message); } return { message, error }; } + + private sanitizeMessage(raw: string): string { + if (!this.isProduction) return raw; + + for (const pattern of SENSITIVE_ERROR_PATTERNS) { + if (pattern.test(raw)) { + return 'An unexpected error occurred. Please try again later.'; + } + } + return raw; + } } diff --git a/backend/src/risk-assessment/risk-assessment.controller.ts b/backend/src/risk-assessment/risk-assessment.controller.ts index 7461ef8d..bd4b6838 100644 --- a/backend/src/risk-assessment/risk-assessment.controller.ts +++ b/backend/src/risk-assessment/risk-assessment.controller.ts @@ -1,4 +1,4 @@ -import { Controller, Get, Param, UseGuards } from '@nestjs/common'; +import { Controller, Get, Param, Query, UseGuards } from '@nestjs/common'; import { RiskAssessmentService } from './risk-assessment.service'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard'; @@ -9,7 +9,7 @@ export class RiskAssessmentController { @Get(':id/risk') @UseGuards(JwtAuthGuard) - async getRisk(@Param('id') id: string) { - return this.riskService.assessDocument(id); + async getRisk(@Param('id') id: string, @Query('lang') lang?: string) { + return this.riskService.assessDocument(id, lang); } } diff --git a/backend/src/risk-assessment/risk-assessment.service.spec.ts b/backend/src/risk-assessment/risk-assessment.service.spec.ts new file mode 100644 index 00000000..5f562d56 --- /dev/null +++ b/backend/src/risk-assessment/risk-assessment.service.spec.ts @@ -0,0 +1,30 @@ +import { + RiskFlag, + RISK_FLAG_DESCRIPTIONS, +} from './risk-assessment.service'; + +describe('Risk flag localization', () => { + it('should have descriptions for all risk flags', () => { + const flags = Object.values(RiskFlag); + for (const flag of flags) { + expect(RISK_FLAG_DESCRIPTIONS[flag]).toBeDefined(); + expect(RISK_FLAG_DESCRIPTIONS[flag]['en']).toBeDefined(); + expect(RISK_FLAG_DESCRIPTIONS[flag]['fr']).toBeDefined(); + expect(RISK_FLAG_DESCRIPTIONS[flag]['es']).toBeDefined(); + } + }); + + it('should have non-empty descriptions', () => { + for (const [flag, descriptions] of Object.entries(RISK_FLAG_DESCRIPTIONS)) { + for (const [lang, desc] of Object.entries(descriptions)) { + expect(desc.trim().length).toBeGreaterThan(0); + } + } + }); + + it('should return English description for unknown locale', () => { + const desc = RISK_FLAG_DESCRIPTIONS[RiskFlag.MISSING_PARCEL_ID]; + expect(desc['xx']).toBeUndefined(); + expect(desc['en']).toContain('parcel'); + }); +}); diff --git a/backend/src/risk-assessment/risk-assessment.service.ts b/backend/src/risk-assessment/risk-assessment.service.ts index e4c4852f..652cb232 100644 --- a/backend/src/risk-assessment/risk-assessment.service.ts +++ b/backend/src/risk-assessment/risk-assessment.service.ts @@ -15,9 +15,47 @@ export enum RiskFlag { UNKNOWN_ISSUER = 'UNKNOWN_ISSUER', } +export const RISK_FLAG_DESCRIPTIONS: Record> = { + [RiskFlag.MISSING_PARCEL_ID]: { + en: 'No parcel identifier (APN/PIN) was found in the document.', + fr: "Aucun identifiant de parcelle (APN/PIN) n'a été trouvé dans le document.", + es: 'No se encontró ningún identificador de parcela (APN/PIN) en el documento.', + }, + [RiskFlag.OVERLAPPING_CLAIM]: { + en: 'This document covers land already claimed by another filing.', + fr: 'Ce document couvre un terrain déjà revendiqué par une autre déclaration.', + es: 'Este documento cubre tierras ya reclamadas por otra declaración.', + }, + [RiskFlag.FORGED_SIGNATURE_INDICATOR]: { + en: 'Heuristic analysis suggests the signature may have been forged.', + fr: "L'analyse heuristique suggère que la signature a pu être falsifiée.", + es: 'El análisis heurístico sugiere que la firma puede haber sido falsificada.', + }, + [RiskFlag.EXPIRED_DOCUMENT]: { + en: 'The document has passed its stated expiry or validity date.', + fr: 'Le document a dépassé sa date d\'expiration ou de validité.', + es: 'El documento ha pasado su fecha de vencimiento o validez.', + }, + [RiskFlag.INCOMPLETE_OWNERSHIP_CHAIN]: { + en: 'The ownership chain is incomplete or the title is too short.', + fr: 'La chaîne de propriété est incomplète ou le titre est trop court.', + es: 'La cadena de propiedad está incompleta o el título es demasiado corto.', + }, + [RiskFlag.UNKNOWN_ISSUER]: { + en: 'The issuing authority could not be verified against known registries.', + fr: "L'autorité émettrice n'a pas pu être vérifiée auprès des registres connus.", + es: 'No se pudo verificar la autoridad emisora contra los registros conocidos.', + }, +}; + +export interface RiskFlagResult { + flag: RiskFlag; + description: string; +} + export interface RiskResult { score: number; - flags: RiskFlag[]; + flags: RiskFlagResult[]; contentAnalysisPossible: boolean; } @@ -59,7 +97,7 @@ export class RiskAssessmentService { : DEFAULT_KNOWN_ISSUERS; } - async assessDocument(documentId: string): Promise { + async assessDocument(documentId: string, lang?: string): Promise { const document = await this.documentsService.findById(documentId); if (!document) { throw new NotFoundException('Document not found'); @@ -70,9 +108,15 @@ export class RiskAssessmentService { await this.documentsService.updateRisk(documentId, score, flags); + const locale = lang || 'en'; + const flagResults: RiskFlagResult[] = flags.map((flag) => ({ + flag, + description: RISK_FLAG_DESCRIPTIONS[flag]?.[locale] || RISK_FLAG_DESCRIPTIONS[flag]?.['en'] || flag, + })); + return { score, - flags, + flags: flagResults, contentAnalysisPossible: document.mimeType === 'application/pdf', }; }