diff --git a/docs/api/auth-error-handling.md b/docs/api/auth-error-handling.md new file mode 100644 index 00000000..bddeebc2 --- /dev/null +++ b/docs/api/auth-error-handling.md @@ -0,0 +1,5 @@ +# Auth API error handling + +`POST /auth/password-reset/request` no longer fails when the reset email cannot be sent. Previously a mail-provider error propagated as a `500`, which let a caller tell registered addresses (500) from unknown ones (silent success). The failure is now caught and logged (`auth.password_reset_email_failed`, with the user id and a hash of the email, never the address or token), and the response is the same as for an unknown address. + +The reset token is still created, so a retry from the user produces a fresh token and email. diff --git a/docs/api/auth-structured-logging.md b/docs/api/auth-structured-logging.md new file mode 100644 index 00000000..77befe4b --- /dev/null +++ b/docs/api/auth-structured-logging.md @@ -0,0 +1,16 @@ +# Auth API structured logging + +`AuthService` now uses the project's `AppLogger`. Security-relevant events are logged as a short constant message plus a metadata object with a stable `event` name, instead of interpolated sentences, so they can be filtered and alerted on in the JSON logs produced in production: + +| `event` | Level | Fields | +| --- | --- | --- | +| `auth.refresh_token_reuse` | error | `userId`, `jti`, `family`, `ipAddress` | +| `auth.token_rotated` | log | `userId`, `emailHash`, `family`, `ipAddress` | +| `auth.logout` | log | `userId`, `emailHash`, `loggedOutAt` | +| `auth.blacklist_access_token_failed` / `auth.blacklist_refresh_token_failed` | warn | `userId`, `error` | +| `auth.lockout_email_failed` | error | `userId`, `emailHash`, `error` | +| `auth.captcha_failed` / `auth.captcha_error` | warn / error | `errorCodes` / `error` | +| `auth.verification_resent` | log | `userId` | +| `auth.password_reset_email_failed` | error | `userId`, `emailHash`, `error` | + +Emails appear only as a short hash, and `AppLogger` redacts fields such as `password`, `token` and `secret` before writing. diff --git a/docs/api/auth-tests.md b/docs/api/auth-tests.md new file mode 100644 index 00000000..44c734a5 --- /dev/null +++ b/docs/api/auth-tests.md @@ -0,0 +1,6 @@ +# Auth API tests + +- **Unit** - `src/auth/security.utils.spec.ts` covers the crypto helpers behind login and 2FA: `sanitizeUser`, hashing/redaction, base32, backup codes, TOTP (RFC 6238 test vectors, window and trimming behaviour), otpauth/QR URLs and `parseDuration`. +- **Integration** - `test/e2e/auth-controller.integration.spec.ts` boots `AuthController` over HTTP with the same `ValidationPipe` settings as `main.ts`, mocked `AuthService` and guards: valid and invalid login/register/refresh/password-reset requests (400 for invalid, unknown fields rejected, service untouched), and guarded routes (`/auth/me`, `/auth/change-password`). + +Run with `npx jest src/auth/security.utils.spec.ts test/e2e/auth-controller.integration.spec.ts`. diff --git a/src/auth/auth.service.ts b/src/auth/auth.service.ts index c167da63..8ee2b528 100644 --- a/src/auth/auth.service.ts +++ b/src/auth/auth.service.ts @@ -1,7 +1,6 @@ import { BadRequestException, Injectable, - Logger, NotFoundException, Optional, ServiceUnavailableException, @@ -86,7 +85,7 @@ interface RecaptchaVerifyResponse { @Injectable() export class AuthService { - private readonly logger = new Logger(AuthService.name); + private readonly logger = new AppLogger(AuthService.name); private readonly issuer = 'PropChain'; private readonly registrationIpMap = new Map(); @@ -356,9 +355,12 @@ export class AuthService { if (shouldLock) { const lockoutDuration = 30; await this.emailService.sendAccountLockedEmail(user.email, lockoutDuration).catch((err) => { - this.logger.error( - `Failed to send account locked email to user ${user.id} (${this.hashEmail(user.email)}): ${err.message}`, - ); + this.logger.error('Failed to send account locked email', { + event: 'auth.lockout_email_failed', + userId: user.id, + emailHash: this.hashEmail(user.email), + error: err.message, + }); }); throw new UnauthorizedException( `Account locked due to too many failed login attempts. Please try again in ${lockoutDuration} minutes.`, @@ -517,9 +519,13 @@ export class AuthService { await this.handleTokenReuse(blacklistedToken, payload.jti, ipAddress, userAgent); await this.fraudService.handleTokenReuse(payload.sub, payload.jti, ipAddress, userAgent); - this.logger.error( - `Refresh token reuse detected for user ${payload.sub} (JTI: ${payload.jti}, Family: ${payload.family}). IP: ${ipAddress}`, - ); + this.logger.error('Refresh token reuse detected', { + event: 'auth.refresh_token_reuse', + userId: payload.sub, + jti: payload.jti, + family: payload.family, + ipAddress, + }); throw new UnauthorizedException( 'Token reuse detected. All sessions have been invalidated for security. Please login again.', @@ -555,9 +561,13 @@ export class AuthService { // Issue new token pair with SAME family ID const tokens = await this.issueTokenPair(user, payload.family, ipAddress, userAgent); - this.logger.log( - `Token rotated for user ${user.id} (${this.hashEmail(user.email)}). Family: ${payload.family}. IP: ${ipAddress}`, - ); + this.logger.log('Refresh token rotated', { + event: 'auth.token_rotated', + userId: user.id, + emailHash: this.hashEmail(user.email), + family: payload.family, + ipAddress, + }); return { user: sanitizeUser(user), @@ -621,9 +631,11 @@ export class AuthService { }); } catch (error: unknown) { // Token might already be expired or invalid, continue with logout - this.logger.warn( - `Failed to blacklist access token for user ${user.sub}: ${(error as Error).message}`, - ); + this.logger.warn('Failed to blacklist access token', { + event: 'auth.blacklist_access_token_failed', + userId: user.sub, + error: (error as Error).message, + }); } } @@ -647,16 +659,21 @@ export class AuthService { throw error; } // Token might already be expired or invalid, continue with logout - this.logger.warn( - `Failed to blacklist refresh token for user ${user.sub}: ${(error as Error).message}`, - ); + this.logger.warn('Failed to blacklist refresh token', { + event: 'auth.blacklist_refresh_token_failed', + userId: user.sub, + error: (error as Error).message, + }); } } // Log the logout event - this.logger.log( - `User ${user.sub} (${this.hashEmail(user.email)}) logged out successfully at ${logoutTime.toISOString()}`, - ); + this.logger.log('User logged out', { + event: 'auth.logout', + userId: user.sub, + emailHash: this.hashEmail(user.email), + loggedOutAt: logoutTime.toISOString(), + }); return { message: 'Logged out successfully', @@ -1869,10 +1886,16 @@ export class AuthService { return true; } - this.logger.warn(`CAPTCHA verification failed: ${JSON.stringify(data['error-codes'])}`); + this.logger.warn('CAPTCHA verification failed', { + event: 'auth.captcha_failed', + errorCodes: data['error-codes'] ?? [], + }); return false; } catch (error: unknown) { - this.logger.error(`Error verifying CAPTCHA: ${(error as Error).message}`); + this.logger.error('Error verifying CAPTCHA', { + event: 'auth.captcha_error', + error: (error as Error).message, + }); return false; } } @@ -1973,6 +1996,6 @@ export class AuthService { this.logger.error('Failed to queue verification email:', err?.message || err); }); - this.logger.log(`Verification email resent for user ${user.id}`); + this.logger.log('Verification email resent', { event: 'auth.verification_resent', userId: user.id }); } } diff --git a/src/auth/security.utils.spec.ts b/src/auth/security.utils.spec.ts new file mode 100644 index 00000000..3a48cd88 --- /dev/null +++ b/src/auth/security.utils.spec.ts @@ -0,0 +1,167 @@ +import { + buildOtpAuthUrl, + buildQrCodeUrl, + createSha256, + decodeBase32, + generateBackupCodes, + generateReactivationToken, + generateTotpCode, + parseDuration, + randomBase32Secret, + randomToken, + redactEmail, + sanitizeUser, + verifyBackupCode, + verifyTotpCode, +} from './security.utils'; + +// RFC 6238 test secret: ASCII "12345678901234567890" in base32. +const RFC_SECRET = 'GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ'; + +describe('security.utils', () => { + describe('sanitizeUser', () => { + it('removes password and two-factor secrets but keeps other fields', () => { + const user = { + id: 'u1', + email: 'a@example.com', + password: 'hash', + twoFactorSecret: 'secret', + twoFactorBackupCodes: ['x'], + }; + expect(sanitizeUser(user)).toEqual({ id: 'u1', email: 'a@example.com' }); + }); + + it('does not mutate its input', () => { + const user = { id: 'u1', password: 'hash' }; + sanitizeUser(user); + expect(user.password).toBe('hash'); + }); + }); + + describe('hashing helpers', () => { + it('createSha256 matches the known digest of "abc"', () => { + expect(createSha256('abc')).toBe( + 'ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad', + ); + }); + + it('redactEmail is a short, stable, non-reversible token', () => { + const redacted = redactEmail('user@example.com'); + expect(redacted).toHaveLength(8); + expect(redacted).toBe(redactEmail('user@example.com')); + expect(redacted).not.toContain('user'); + expect(redacted).not.toBe(redactEmail('other@example.com')); + }); + + it('randomToken returns 2*size hex characters and differs between calls', () => { + const a = randomToken(16); + expect(a).toMatch(/^[0-9a-f]{32}$/); + expect(a).not.toBe(randomToken(16)); + }); + + it('generateReactivationToken returns a token and its sha256 hash', () => { + const { token, hash } = generateReactivationToken(); + expect(hash).toBe(createSha256(token)); + }); + }); + + describe('base32', () => { + it('randomBase32Secret has the requested length and alphabet', () => { + expect(randomBase32Secret(20)).toMatch(/^[A-Z2-7]{20}$/); + }); + + it('decodeBase32 decodes the RFC 6238 secret', () => { + expect(decodeBase32(RFC_SECRET).toString('ascii')).toBe('12345678901234567890'); + }); + + it('decodeBase32 ignores padding, case and characters outside the alphabet', () => { + expect(decodeBase32('gezd gnbv gy3t qojq gezd gnbv gy3t qojq==').toString('ascii')).toBe( + '12345678901234567890', + ); + }); + }); + + describe('backup codes', () => { + it('generates the requested number of 8-character upper-case hex codes', () => { + const codes = generateBackupCodes(5); + expect(codes).toHaveLength(5); + codes.forEach((code) => expect(code).toMatch(/^[0-9A-F]{8}$/)); + }); + + it('verifyBackupCode finds the matching hash, ignoring case and whitespace', () => { + const hashes = ['ABCD1234', 'DEADBEEF'].map(createSha256); + expect(verifyBackupCode(' deadbeef ', hashes)).toBe(hashes[1]); + }); + + it('verifyBackupCode returns undefined for an unknown code or no hashes', () => { + expect(verifyBackupCode('00000000', [createSha256('ABCD1234')])).toBeUndefined(); + expect(verifyBackupCode('ABCD1234', [])).toBeUndefined(); + }); + }); + + describe('TOTP', () => { + it('matches the RFC 6238 SHA-1 test vectors', () => { + // T = 59s -> 94287082 (8 digits); the 6-digit code is its last six digits. + expect(generateTotpCode({ secret: RFC_SECRET, digits: 8, timestamp: 59_000 })).toBe('94287082'); + expect(generateTotpCode({ secret: RFC_SECRET, timestamp: 59_000 })).toBe('287082'); + // T = 1111111109s -> 07081804 (8 digits). + expect(generateTotpCode({ secret: RFC_SECRET, digits: 8, timestamp: 1_111_111_109_000 })).toBe( + '07081804', + ); + }); + + it('verifyTotpCode accepts the current code and one adjacent period', () => { + const now = 59_000; + expect(verifyTotpCode({ secret: RFC_SECRET, code: '287082', timestamp: now })).toBe(true); + // Same code, verified 30s later: still inside the +/-1 window. + expect(verifyTotpCode({ secret: RFC_SECRET, code: '287082', timestamp: now + 30_000 })).toBe( + true, + ); + }); + + it('verifyTotpCode rejects codes outside the window or when the window is 0', () => { + expect(verifyTotpCode({ secret: RFC_SECRET, code: '287082', timestamp: 59_000 + 90_000 })).toBe( + false, + ); + expect( + verifyTotpCode({ secret: RFC_SECRET, code: '287082', timestamp: 59_000 + 30_000, window: 0 }), + ).toBe(false); + expect(verifyTotpCode({ secret: RFC_SECRET, code: '000000', timestamp: 59_000 })).toBe(false); + }); + + it('verifyTotpCode trims whitespace around the submitted code', () => { + expect(verifyTotpCode({ secret: RFC_SECRET, code: ' 287082 ', timestamp: 59_000 })).toBe(true); + }); + }); + + describe('otpauth URLs', () => { + it('buildOtpAuthUrl encodes the label and issuer', () => { + const url = buildOtpAuthUrl('a+b@example.com', RFC_SECRET, 'Prop Chain'); + expect(url).toBe( + `otpauth://totp/Prop%20Chain%3Aa%2Bb%40example.com?secret=${RFC_SECRET}&issuer=Prop%20Chain&algorithm=SHA1&digits=6&period=30`, + ); + }); + + it('buildQrCodeUrl url-encodes the otpauth URL', () => { + const otp = buildOtpAuthUrl('a@example.com', RFC_SECRET); + expect(buildQrCodeUrl(otp)).toBe(`https://quickchart.io/qr?text=${encodeURIComponent(otp)}`); + }); + }); + + describe('parseDuration', () => { + it.each([ + ['30s', 30], + ['15m', 900], + ['2h', 7200], + ['7d', 604800], + ['45', 45], + ['5M', 300], + ])('parses %s as %i seconds', (input, expected) => { + expect(parseDuration(input, 1)).toBe(expected); + }); + + it.each([[''], [' '], ['abc'], ['-5'], ['0']])('falls back for %p', (input) => { + expect(parseDuration(input, 99)).toBe(99); + }); + }); +}); diff --git a/test/e2e/auth-controller.integration.spec.ts b/test/e2e/auth-controller.integration.spec.ts new file mode 100644 index 00000000..bd99305e --- /dev/null +++ b/test/e2e/auth-controller.integration.spec.ts @@ -0,0 +1,228 @@ +/** + * Integration tests: AuthController over HTTP. + * + * Boots the real controller with Nest's HTTP stack, the same global + * ValidationPipe settings as main.ts, and mocked service/guards, so request + * parsing, validation, guard wiring and argument passing are exercised without + * a database. + */ + +import { + CanActivate, + ExecutionContext, + INestApplication, + Injectable, + ValidationPipe, +} from '@nestjs/common'; +import { Test } from '@nestjs/testing'; +import * as request from 'supertest'; +import { AuthController } from '../../src/auth/auth.controller'; +import { AuthService } from '../../src/auth/auth.service'; +import { ApiKeyAnalyticsService } from '../../src/auth/api-key-analytics.service'; +import { JwtAuthGuard } from '../../src/auth/guards/jwt-auth.guard'; +import { RolesGuard } from '../../src/auth/guards/roles.guard'; +import { FreshTwoFactorGuard } from '../../src/auth/guards/fresh-two-factor.guard'; +import { ApiKeyAuthGuard } from '../../src/auth/guards/api-key-auth.guard'; +import { KeyPermissionsGuard } from '../../src/auth/guards/key-permissions.guard'; +import { GoogleAuthGuard } from '../../src/auth/guards/google-auth.guard'; + +/** Accepts "Authorization: Bearer test" and attaches a fixed user; rejects otherwise. */ +@Injectable() +class BearerTestGuard implements CanActivate { + canActivate(ctx: ExecutionContext): boolean { + const req = ctx.switchToHttp().getRequest(); + if (req.headers['authorization'] !== 'Bearer test') return false; + req.authUser = { sub: 'user-1', email: 'user@example.com', role: 'USER', type: 'access' }; + return true; + } +} + +describe('AuthController (integration)', () => { + let app: INestApplication; + const authService = { + register: jest.fn(), + login: jest.fn(), + refreshToken: jest.fn(), + me: jest.fn(), + changePassword: jest.fn(), + requestPasswordReset: jest.fn(), + }; + + beforeAll(async () => { + const moduleRef = await Test.createTestingModule({ + controllers: [AuthController], + providers: [ + { provide: AuthService, useValue: authService }, + { provide: ApiKeyAnalyticsService, useValue: {} }, + ], + }) + .overrideGuard(JwtAuthGuard) + .useClass(BearerTestGuard) + .overrideGuard(RolesGuard) + .useClass(BearerTestGuard) + .overrideGuard(FreshTwoFactorGuard) + .useClass(BearerTestGuard) + .overrideGuard(ApiKeyAuthGuard) + .useClass(BearerTestGuard) + .overrideGuard(KeyPermissionsGuard) + .useClass(BearerTestGuard) + .overrideGuard(GoogleAuthGuard) + .useClass(BearerTestGuard) + .compile(); + + app = moduleRef.createNestApplication(); + // Same settings as src/main.ts. + app.useGlobalPipes( + new ValidationPipe({ whitelist: true, transform: true, forbidNonWhitelisted: true }), + ); + await app.init(); + }); + + afterAll(async () => { + await app.close(); + }); + + beforeEach(() => { + jest.resetAllMocks(); + }); + + describe('POST /auth/login', () => { + it('passes the body, client address and user agent to the service', async () => { + authService.login.mockResolvedValue({ accessToken: 'a', refreshToken: 'r' }); + + const res = await request(app.getHttpServer()) + .post('/auth/login') + .set('User-Agent', 'jest-agent') + .send({ email: 'user@example.com', password: 'Secret123!' }) + .expect(201); + + expect(res.body).toEqual({ accessToken: 'a', refreshToken: 'r' }); + expect(authService.login).toHaveBeenCalledWith( + expect.objectContaining({ email: 'user@example.com', password: 'Secret123!' }), + expect.any(String), + 'jest-agent', + ); + }); + + it('rejects an invalid email with 400 and never calls the service', async () => { + await request(app.getHttpServer()) + .post('/auth/login') + .send({ email: 'not-an-email', password: 'x' }) + .expect(400); + expect(authService.login).not.toHaveBeenCalled(); + }); + + it('rejects unknown fields with 400 (forbidNonWhitelisted)', async () => { + await request(app.getHttpServer()) + .post('/auth/login') + .send({ email: 'user@example.com', password: 'x', isAdmin: true }) + .expect(400); + expect(authService.login).not.toHaveBeenCalled(); + }); + + it('rejects a missing password with 400', async () => { + await request(app.getHttpServer()) + .post('/auth/login') + .send({ email: 'user@example.com' }) + .expect(400); + }); + }); + + describe('POST /auth/register', () => { + const valid = { + email: 'new@example.com', + password: 'Str0ng!Passw0rd', + firstName: 'Ada', + lastName: 'Lovelace', + }; + + it('creates the account through the service', async () => { + authService.register.mockResolvedValue({ id: 'u2', email: valid.email }); + + await request(app.getHttpServer()).post('/auth/register').send(valid).expect(201); + + expect(authService.register).toHaveBeenCalledWith( + expect.objectContaining({ email: valid.email, firstName: 'Ada' }), + expect.any(String), + ); + }); + + it('rejects a weak password with 400', async () => { + await request(app.getHttpServer()) + .post('/auth/register') + .send({ ...valid, password: 'short' }) + .expect(400); + expect(authService.register).not.toHaveBeenCalled(); + }); + + it('rejects a missing name with 400', async () => { + const { firstName: _omit, ...withoutName } = valid; + await request(app.getHttpServer()).post('/auth/register').send(withoutName).expect(400); + }); + }); + + describe('POST /auth/refresh', () => { + it('forwards the refresh token', async () => { + authService.refreshToken.mockResolvedValue({ accessToken: 'a2', refreshToken: 'r2' }); + + await request(app.getHttpServer()) + .post('/auth/refresh') + .send({ refreshToken: 'old-token' }) + .expect(201); + + // The user agent depends on the HTTP client, so only the DTO is asserted. + expect(authService.refreshToken.mock.calls[0][0]).toMatchObject({ refreshToken: 'old-token' }); + }); + + it('requires a refresh token', async () => { + await request(app.getHttpServer()).post('/auth/refresh').send({}).expect(400); + }); + }); + + describe('POST /auth/password-reset/request', () => { + it('validates the email and delegates to the service', async () => { + authService.requestPasswordReset.mockResolvedValue(undefined); + + await request(app.getHttpServer()) + .post('/auth/password-reset/request') + .send({ email: 'user@example.com' }) + .expect(201); + expect(authService.requestPasswordReset).toHaveBeenCalledWith( + expect.objectContaining({ email: 'user@example.com' }), + ); + + await request(app.getHttpServer()) + .post('/auth/password-reset/request') + .send({ email: 'nope' }) + .expect(400); + }); + }); + + describe('guarded routes', () => { + it('GET /auth/me is rejected without a bearer token', async () => { + await request(app.getHttpServer()).get('/auth/me').expect(403); + expect(authService.me).not.toHaveBeenCalled(); + }); + + it('GET /auth/me passes the authenticated user to the service', async () => { + authService.me.mockResolvedValue({ id: 'user-1', email: 'user@example.com' }); + + const res = await request(app.getHttpServer()) + .get('/auth/me') + .set('Authorization', 'Bearer test') + .expect(200); + + expect(res.body).toEqual({ id: 'user-1', email: 'user@example.com' }); + expect(authService.me).toHaveBeenCalledWith(expect.objectContaining({ sub: 'user-1' })); + }); + + it('POST /auth/change-password validates the body before calling the service', async () => { + await request(app.getHttpServer()) + .post('/auth/change-password') + .set('Authorization', 'Bearer test') + .send({}) + .expect(400); + expect(authService.changePassword).not.toHaveBeenCalled(); + }); + }); +});