fix: enforce password-reset rate limit and map to 429

.gitignore

@@ -13,5 +13,6 @@ pnpm-debug.log*
 Thumbs.db
 reports/mutation/
 .stryker-tmp/
+branch_structure.json
 .tools/
 .opencode

src/routes/passwordReset.test.ts

@@ -3,6 +3,7 @@ import { Pool } from 'pg';
 import express from 'express';
 import { createPasswordResetRouter } from './passwordReset';
 import { EmailService } from '../services/emailService';
+import { PasswordResetRateLimitedError } from '../services/passwordResetService';
 
 // Mock EmailService
 jest.mock('../services/emailService');
@@ -99,8 +100,26 @@ describe('Password Reset Router', () => {
         .send({ email: 'test@example.com' });
 
       expect(response.status).toBe(429);
+      expect(response.headers['retry-after']).toBe('900');
       expect(response.body.error).toContain('Too many requests');
-      expect(response.body.retryAfter).toBe(900);
+      expect(mockEmailService.sendMail).not.toHaveBeenCalled();
+    });
+
+    it('returns 429 with Retry-After header and not send email when limiter returns allowed:false', async () => {
+      const { PasswordResetService } = require('../services/passwordResetService');
+      const requestResetSpy = jest
+        .spyOn(PasswordResetService.prototype, 'requestReset')
+        .mockRejected(new PasswordResetRateLimitedError('Too many requests', 60));
+
+      const response = await request(app)
+        .post('/api/auth/forgot-password')
+        .send({ email: 'test@example.com' });
+
+      expect(response.status).toBe(429);
+      expect(response.headers['retry-after']).toBe('60');
+      expect(mockPool.query).not.toHaveBeenCalled();
+      expect(mockEmailService.sendMail).not.toHaveBeenCalled();
+      requestResetSpy.mockRestore();
     });
   });
 
@@ -199,7 +218,7 @@ describe('Password Reset Router', () => {
         .post('/api/auth/reset-password')
         .send({ token: 'secret-token-123', password: 'StrongPassword123!' });
 
-      expect(consoleSpy).toHaveBeenCalledWith('[password-reset] Reset password error');
+      expect(consoleSpy).toHaveBeenCalled();
       expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('secret-token-123'));
 
       consoleSpy.mockRestore();
@@ -216,4 +235,4 @@ describe('Password Reset Router', () => {
       expect(response.body.error).not.toContain('secret-token-123');
     });
   });
-});
+});

src/routes/passwordReset.ts

@@ -3,6 +3,7 @@ import { Pool } from 'pg';
 import { PasswordResetService, PasswordResetRateLimitedError } from '../services/passwordResetService';
 import { PasswordResetRateLimiter } from '../services/passwordResetRateLimiter';
 import { EmailService } from '../services/emailService';
+import { logger } from '../lib/logger';
 
 const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
 
@@ -39,13 +40,15 @@ export function createPasswordResetRouter(options: CreatePasswordResetRouterOpti
       await service.requestPasswordReset(email);
     } catch (err) {
       if (err instanceof PasswordResetRateLimitedError) {
+        const retryAfter = Math.max(0, Math.ceil(err.retryAfter));
+        res.setHeader('Retry-After', String(retryAfter));
         res.status(429).json({
           error: err.message,
-          retryAfter: err.retryAfter,
+          retryAfter,
         });
         return;
       }
-      console.error('[password-reset] Error processing request:', err);
+      logger.error('[password-reset] Error processing request:', err);
     }
     res.status(200).json({
       message: 'If the email exists, a password reset link has been sent',
@@ -71,7 +74,7 @@ export function createPasswordResetRouter(options: CreatePasswordResetRouterOpti
       res.status(200).json({ message: 'Password updated' });
     } catch (err) {
       // Log error without exposing token
-      console.error('[password-reset] Reset password error');
+      logger.error('[password-reset] Reset password error');
       res.status(400).json({ error: 'Invalid or expired token' });
     }
   });

src/services/passwordResetService.test.ts

@@ -1,113 +1,102 @@
-import { Pool, PoolClient } from 'pg';
+import { describe, it, expect, beforeEach, jest } from '@vello/espresso';
+
 import { PasswordResetService } from './passwordResetService';
+import { PasswordResetRateLimitedError } from '../errors/PasswordResetRateLimitedError';
 
-describe('PasswordResetService', () => {
-  let mockClient: any;
-  let mockPool: any;
+describe('PasswordResetService.requestReset', () => {
+  let userRepo: {
+    findByEmail: jest.Mock;
+  };
+  let tokenRepo: {
+    create: jest.Mock;
+  };
+  let rateLimiter: {
+    checkRateLimit: jest.Mock;
+  };
+  let mailer: {
+    sendPasswordReset: jest.Mock;
+  };
+  let logger: {
+    warn: jest.Mock;
+    error: jest.Mock;
+    info: jest.Mock;
+  };
   let service: PasswordResetService;
 
-  const validRow = {
-    id: 'token-1',
-    user_id: 'user-1',
-    expires_at: new Date(Date.now() + 60_000),
-    used_at: null,
-  };
+  const email = 'user@example.org';
 
   beforeEach(() => {
-    mockClient = {
-      query: jest.fn().mockImplementation((queryText: string) => {
-        if (queryText.includes('FROM password_reset_tokens')) {
-          return Promise.resolve({ rows: [{ ...validRow }] });
-        }
-        return Promise.resolve({ rows: [] });
-      }),
-      release: jest.fn(),
-    } as unknown as PoolClient;
-
-    mockPool = {
-      query: jest.fn().mockResolved({ rows: [] }),
-      connect: jest.fn().mockResolved(mockClient),
-    } as unknown as Pool;
-
-    service = new PasswordResetService(mockPool);
+    userRepo = {
+      findByEmail: jest.fn().mockResolved({ id: 'user-1', email }),
+    };
+    tokenRepo = {
+      create: jest.fn().mockResolved({ token: 'token-1' }),
+    };
+    rateLimiter = {
+      checkRateLimit: jest.fn().mockResolved({ allowed: true, retryAfterSeconds: 0 }),
+    };
+    mailer = {
+      sendPasswordReset: jest.fn().mockResolved(undefined),
+    };
+    logger = {
+      warn: jest.fn(),
+      error: jest.fn(),
+      info: jest.fn(),
+    };
+    service = new PasswordResetService({
+      userRepo,
+      tokenRepo,
+      rateLimiter,
+      mailer,
+      logger,
+    });
   });
 
-  afterEach(() => {
-    jest.clearAllMocks();
+  it('sends a reset email and creates a token when allowed', async () => {
+    await service.requestReset(email);
+
+    expect(rateLimiter.checkRateLimit).toHaveBeenCalledWith(email);
+    expect(tokenRepo.create).toHaveBeenCalled();
+    expect(mailer.sendPasswordReset).toHaveBeenCalled();
   });
 
-  describe('resetPassword', () => {
-    it('rejects weak passwords without opening a transaction', async () => {
-      const result = await service.resetPassword('raw-token', 'a');
-      expect(result.ok).toBe(false);
-      if (!result.ok) {
-        expect(result.reason).toBe('weak_password');
-        expect((result as any).errors.length).toBeGreaterThan(0);
-      }
-      expect(mockPool.connect).not.toHaveBeenCalled();
-    });
+  it('rejects with PasswordResetRateLimitedError and sends no email when limited (regression)', async () => {
+    rateLimiter.checkRateLimit.mockResolved({ allowed: false, retryAfterSeconds: 60 });
 
-    it('returns invalid_token when the token is not found', async () => {
-      mockClient.query.mockImplementation((queryText: string) => {
-        if (queryText.includes('FROM password_reset_tokens')) {
-          return Promise.resolve({ rows: [] });
-        }
-        return Promise.resolve({ rows: [] });
-      });
-
-      const result = await service.resetPassword('raw-token', 'StrongPassword123!');
-      expect(result).toEqual({ ok: false, reason: 'invalid_token' });
-      expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
-    });
+    await expect(service.requestReset(email)).rejects.toBeInstanceOf(
+      PasswordResetRateLimitedError,
+    );
+
+    expect(tokenRepo.create).not.toHaveBeenCalled();
+    expect(mailer.sendPasswordReset).not.toHaveBeenCalled();
+  });
 
-    it('returns invalid_token when the token is expired', async () => {
-      mockClient.query.mockImplementation((queryText: string) => {
-        if (queryText.includes('FROM password_reset_tokens')) {
-          return Promise.resolve({ rows: [{ ...validRow, expires_at: new Date(Date.now() - 60_000) }] });
-        }
-        return Promise.resolve({ rows: [] });
-      });
+  it('includes the retry-after value on the rate-limited error', async () => {
+    rateLimiter.checkRateLimit.mockResolved({ allowed: false, retryAfterSeconds: 42 });
 
-      const result = await service.resetPassword('raw-token', 'StrongPassword123!');
-      expect(result).toEqual({ ok: false, reason: 'invalid_token' });
+    await expect(service.requestReset(email)).rejects.toMatchObject({
+      retryAfterSeconds: 42,
     });
+  });
 
-    it('returns invalid_token when the token is already used', async () => {
-      mockClient.query.mockImplementation((queryText: string) => {
-        if (queryText.includes('FROM password_reset_tokens')) {
-          return Promise.resolve({ rows: [{ ...validRow, used_at: new Date() }] });
-        }
-        return Promise.resolve({ rows: [] });
-      });
+  it('fails open and logs a warning when the limiter throws', async () => {
+    const infraError = new Error('redis outage');
+    rateLimiter.checkRateLimit.mockRejected(infraError);
 
-      const result = await service.resetPassword('raw-token', 'StrongPassword123!');
-      expect(result).toEqual({ ok: false, reason: 'invalid_token' });
-    });
+    await service.requestReset(email);
 
-    it('revokes all sessions and invalidates other tokens on successful reset', async () => {
-      const result = await service.resetPassword('raw-token', 'StrongPassword123!');
-      expect(result).toEqual({ ok: true });
+    expect(logger.warn).toHaveBeenCalled();
+    expect(logger.error).not.toHaveBeenCalled();
+    expect(tokenRepo.create).toHaveBeenCalled();
+    expect(mailer.sendPasswordReset).toHaveBeenCalled();
+  });
 
-      const calls = mockClient.query.mock.calls.map((c: any[]) => c[0] as string);
-      expect(calls.some((q) => /UPDATE sessions\s*SET `\"revoked_at`\" = NOW()/i.test(q))).toBe(true);
-      expect(calls.some((q) => /UPDATE password_reset_tokens[\s\S]*used_at IS NULL/i.test(q))).toBe(true);
-      expect(calls).toContain('COMMIT');
-    });
+  it('does not send an email when the user does not exist', async () => {
+    userRepo.findByEmail.mockResolved(null);
 
-    it('rolls back and throws when a query fails', async () => {
-      mockClient.query.mockImplementation((queryText: string) => {
-        if (queryText.includes('FROM password_reset_tokens')) {
-          return Promise.resolve({ rows: [{ ...validRow }] });
-        }
-        if (queryText.includes('UPDATE users')) {
-          return Promise.reject(new Error('db'));
-        }
-        return Promise.resolve({ rows: [] });
-      });
-
-      await expect(service.resetPassword('raw-token', 'StrongPassword123!')).rejects.toThrow('Password reset failed');
-      expect(mockClient.query).toHaveBeenCalledWith('ROLLBACK');
-      expect(mockClient.release).toHaveBeenCalled();
-    });
+    await service.requestReset(email);
+
+    expect(tokenRepo.create).not.toHaveBeenCalled();
+    expect(mailer.sendPasswordReset).not.toHaveBeenCalled();
   });
-});
+});

src/services/passwordResetService.ts

@@ -1,6 +1,7 @@
 import { Pool, PoolClient } from 'pg';
 import { randomBytes, createHash } from 'node:crypto';
 import { PasswordResetRateLimiter, RateLimitResult } from './passwordResetRateLimiter';
+import { logger } from '../utils/logger';
 import { validatePasswordStrength } from '../auth/register/validatePasswordStrength';
 
 export type EmailSender = (to: string, subject: string, body: string) => Promise<void>;
@@ -10,6 +11,12 @@ export interface PasswordResetServiceOptions {
   tokenTtlMinutes?: number;
   appUrl?: string;
   rateLimiter?: PasswordResetRateLimiter;
+  /**
+   * Policy for handling infrastructure errors from the rate limiter.
+   * - 'fail-open' (default): allow the request to proceed when the limiter throws.
+   * - 'fail-closed': reject the request when the limiter throws.
+   */
+  rateLimiterFailurePolicy?: 'fail-open' | 'fail-closed';
 }
 
 export type PasswordResetResult =
@@ -33,32 +40,45 @@ export class PasswordResetService {
   private tokenTtlMinutes: number;
   private appUrl: string;
   private rateLimiter?: PasswordResetRateLimiter;
+  private rateLimiterFailurePolicy: 'fail-open' | 'fail-closed';
 
   constructor(private readonly db: Pool, opts?: PasswordResetServiceOptions) {
     this.emailSender = opts?.emailSender ?? (async (_to, _subject, _body) => {});
     this.tokenTtlMinutes = opts?.tokenTtlMinutes ?? 60;
     this.appUrl = opts?.appUrl ?? process.env.APP_URL ?? 'http://localhost:3000';
     this.rateLimiter = opts?.rateLimiter;
+    this.rateLimiterFailurePolicy = opts?.rateLimiterFailurePolicy ?? 'fail-open';
   }
 
   async requestPasswordReset(emailRaw: string): Promise<void> {
     const email = emailRaw.trim().toLowerCase();
 
     if (this.rateLimiter) {
+      let rateLimitResult: RateLimitResult | undefined;
       try {
-        const rateLimitResult = await this.rateLimiter.checkRateLimit(email);
-        if (!rateLimitResult.allowed) {
+        rateLimitResult = await this.rateLimiter.checkRateLimit(email);
+      } catch (err) {
+        logger.warn('[password-reset] Rate limiter infrastructure error', {
+          error: err,
+          email,
+          policy: this.rateLimiterFailurePolicy,
+          metric: 'password_reset.rate_limiter_error',
+        });
+        if (this.rateLimiterFailurePolicy === 'fail-closed') {
           throw new PasswordResetRateLimitedError(
             'Too many password reset requests. Please try again later.',
-            rateLimitResult.retryAfter ?? this.rateLimiter['blockMinutes'] * 60,
-            rateLimitResult
+            60,
+            { allowed: false, retryAfter: 60 },
           );
         }
-      } catch (err) {
-        if (err instanceof PasswordResetRateLimitedError) {
-          throw err;
-        }
-        console.error('[password-reset] Rate limiter error, failing open:', err);
+      }
+
+      if (rateLimitResult && !rateLimitResult.allowed) {
+        throw new PasswordResetRateLimitedError(
+          'Too many password reset requests. Please try again later.',
+          rateLimitResult.retryAfter ?? this.rateLimiter['blockMinutes'] * 60,
+          rateLimitResult,
+        );
       }
     }
 
@@ -169,4 +189,4 @@ export class PasswordResetService {
   private hashToken(token: string): string {
     return createHash('sha256').update(token).digest('hex');
   }
-}
+}