From 330704166780dbd23a674a5ef4381fc64e87a90d Mon Sep 17 00:00:00 2001 From: Mekjah Date: Sat, 16 May 2026 16:25:54 +0100 Subject: [PATCH] fix(backend): implement password reset service with rate limiting and abuse prevention --- package-lock.json | 13 ++ package.json | 1 + src/app.ts | 9 + .../010_create_password_reset_tokens.sql | 2 +- src/lib/errors.ts | 14 ++ .../passwordResetRateLimiter.test.ts | 56 ++++++ src/middleware/passwordResetRateLimiter.ts | 23 +++ src/routes/passwordReset.test.ts | 93 ++++++++++ src/routes/passwordReset.ts | 110 ++++++------ src/services/passwordResetService.test.ts | 141 +++++++++++++++ src/services/passwordResetService.ts | 162 ++++++++++-------- 11 files changed, 497 insertions(+), 127 deletions(-) create mode 100644 src/middleware/passwordResetRateLimiter.test.ts create mode 100644 src/middleware/passwordResetRateLimiter.ts create mode 100644 src/routes/passwordReset.test.ts create mode 100644 src/services/passwordResetService.test.ts diff --git a/package-lock.json b/package-lock.json index 5e242b07..d925b9e7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,6 +15,7 @@ "cors": "^2.8.5", "dotenv": "^16.4.5", "express": "^4.22.1", + "express-rate-limit": "^6.11.2", "jsonwebtoken": "^9.0.3", "morgan": "^1.10.0", "pg": "^8.19.0", @@ -4141,6 +4142,18 @@ "url": "https://opencollective.com/express" } }, + "node_modules/express-rate-limit": { + "version": "6.11.2", + "resolved": "https://registry.npmjs.org/express-rate-limit/-/express-rate-limit-6.11.2.tgz", + "integrity": "sha512-a7uwwfNTh1U60ssiIkuLFWHt4hAC5yxlLGU2VP0X4YNlyEDZAqF4tK3GD3NSitVBrCQmQ0++0uOyFOgC2y4DDw==", + "license": "MIT", + "engines": { + "node": ">= 14" + }, + "peerDependencies": { + "express": "^4 || ^5" + } + }, "node_modules/express/node_modules/debug": { "version": "2.6.9", "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", diff --git a/package.json b/package.json index 04d73b18..f246517b 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "cors": "^2.8.5", "dotenv": "^16.4.5", "express": "^4.22.1", + "express-rate-limit": "^6.11.2", "jsonwebtoken": "^9.0.3", "morgan": "^1.10.0", "pg": "^8.19.0", diff --git a/src/app.ts b/src/app.ts index 59c694f1..7cb681d7 100644 --- a/src/app.ts +++ b/src/app.ts @@ -9,6 +9,8 @@ import { SessionRepository } from './db/repositories/sessionRepository'; import { createLogoutRouter } from './auth/logout/logoutRoute'; import { createChangePasswordRouter } from './auth/changePassword/changePasswordRoute'; import { createLoginRouter } from './auth/login/loginRoute'; +import { createPasswordResetRouter } from './routes/passwordReset'; +import { emailService } from './services/emailService'; import { createHealthRouter } from './routes/health'; import { dbHealth } from './db/client'; import { createOfferingSyncRouter } from './routes/offeringSync'; @@ -16,7 +18,13 @@ import { UserRepository } from './db/repositories/userRepository'; import { JwtIssuer, UserRole, UserRepository as IUserRepository, SessionRepository as ISessionRepository } from './auth/login/types'; import { LoginService } from './auth/login/loginService'; import { issueToken } from './lib/jwt'; +import { Logger } from './lib/logger'; import { MetricsCollector } from './lib/metrics'; +import { RefreshTokenRepositoryAdapter } from './auth/refresh/repositoryAdapter'; +import { JwtTokenServiceAdapter } from './auth/refresh/tokenServiceAdapter'; +import { RefreshService } from './auth/refresh/refreshService'; +import { createRefreshRouter } from './auth/refresh/refreshRoute'; +import { errorHandler } from './middleware/errorHandler'; // Adapter to convert database User to login service UserRecord class UserRepositoryAdapter implements IUserRepository { @@ -117,6 +125,7 @@ export function createApp() { app.use(createRefreshRouter({ refreshService })); app.use(createLogoutRouter({ requireAuth, sessionRepository })); app.use(createChangePasswordRouter({ requireAuth, db: pool })); + app.use(createPasswordResetRouter(pool, { emailSender: emailService.sendMail.bind(emailService), appUrl: process.env.APP_URL })); app.use('/api/v1/health', createHealthRouter(pool, dbHealth, metrics)); // Offering sync routes diff --git a/src/db/migrations/010_create_password_reset_tokens.sql b/src/db/migrations/010_create_password_reset_tokens.sql index 91b75770..ec7a6782 100644 --- a/src/db/migrations/010_create_password_reset_tokens.sql +++ b/src/db/migrations/010_create_password_reset_tokens.sql @@ -6,7 +6,7 @@ CREATE TABLE IF NOT EXISTS password_reset_tokens ( user_id UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, token_hash TEXT NOT NULL UNIQUE, expires_at TIMESTAMPTZ NOT NULL, - used_at TIMESTAMPTZ, + used BOOLEAN NOT NULL DEFAULT FALSE, created_at TIMESTAMPTZ NOT NULL DEFAULT NOW() ); diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 8583c279..425539d7 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -157,3 +157,17 @@ export class BadRequestError extends AppError { this.name = 'BadRequestError'; } } + +export class PasswordResetTokenInvalidError extends AppError { + constructor(message: string = 'Invalid or expired password reset token') { + super(ErrorCode.BAD_REQUEST, 400, message); + this.name = 'PasswordResetTokenInvalidError'; + } +} + +export class PasswordResetRateLimitError extends AppError { + constructor(message: string = 'Too many password reset requests') { + super(ErrorCode.TOO_MANY_REQUESTS, 429, message); + this.name = 'PasswordResetRateLimitError'; + } +} diff --git a/src/middleware/passwordResetRateLimiter.test.ts b/src/middleware/passwordResetRateLimiter.test.ts new file mode 100644 index 00000000..5fb13d8b --- /dev/null +++ b/src/middleware/passwordResetRateLimiter.test.ts @@ -0,0 +1,56 @@ +import { Request, Response, NextFunction } from 'express'; +import { createPasswordResetRateLimiter } from './passwordResetRateLimiter'; +import { PasswordResetRateLimitError } from '../lib/errors'; +import { logger } from '../lib/logger'; + +describe('Password reset rate limiter middleware', () => { + const makeReq = (): Request => ({ + ip: '127.0.0.1', + socket: { remoteAddress: '127.0.0.1' }, + headers: {}, + get: jest.fn().mockReturnValue(undefined), + app: { get: jest.fn().mockReturnValue(false) }, + } as unknown as Request); + + const makeRes = (): Response => ({ + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + setHeader: jest.fn(), + getHeader: jest.fn(), + } as unknown as Response); + + beforeEach(() => { + jest.spyOn(logger, 'warn').mockImplementation(() => {}); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('calls next for requests under the limit', async () => { + const res = makeRes(); + const next: NextFunction = jest.fn(); + const limiter = createPasswordResetRateLimiter(); + + await limiter(makeReq(), res, next); + + expect(next).toHaveBeenCalledTimes(1); + }); + + it('calls next with PasswordResetRateLimitError when the limit is exceeded', async () => { + const res = makeRes(); + const next: NextFunction = jest.fn(); + const limiter = createPasswordResetRateLimiter(); + + for (let i = 0; i < 5; i++) { + await limiter(makeReq(), res, next); + } + + await limiter(makeReq(), res, next); + + expect(next).toHaveBeenLastCalledWith(expect.any(PasswordResetRateLimitError)); + expect(logger.warn).toHaveBeenCalledWith('Password reset rate limit exceeded', { + ip: '127.0.0.1', + }); + }); +}); diff --git a/src/middleware/passwordResetRateLimiter.ts b/src/middleware/passwordResetRateLimiter.ts new file mode 100644 index 00000000..0116c7c9 --- /dev/null +++ b/src/middleware/passwordResetRateLimiter.ts @@ -0,0 +1,23 @@ +import rateLimit from 'express-rate-limit'; +import { Request, Response, NextFunction } from 'express'; +import { PasswordResetRateLimitError } from '../lib/errors'; +import { logger } from '../lib/logger'; + +const WINDOW_MS = 15 * 60 * 1000; +const MAX_REQUESTS = 5; + +export function createPasswordResetRateLimiter() { + return rateLimit({ + windowMs: WINDOW_MS, + max: MAX_REQUESTS, + standardHeaders: true, + legacyHeaders: false, + handler: (req: Request, _res: Response, next: NextFunction) => { + const ip = req.ip || req.socket?.remoteAddress || 'unknown'; + logger.warn('Password reset rate limit exceeded', { ip }); + next(new PasswordResetRateLimitError()); + }, + }); +} + +export const passwordResetRateLimiter = createPasswordResetRateLimiter(); diff --git a/src/routes/passwordReset.test.ts b/src/routes/passwordReset.test.ts new file mode 100644 index 00000000..4e7d56a2 --- /dev/null +++ b/src/routes/passwordReset.test.ts @@ -0,0 +1,93 @@ +import express, { ErrorRequestHandler, NextFunction, Request, Response } from 'express'; +import request from 'supertest'; +import { createPasswordResetRouter } from './passwordReset'; +import { errorHandler } from '../middleware/errorHandler'; + +describe('Password reset routes', () => { + let app: express.Express; + let mockPool: any; + let emailSender: jest.Mock, [string, string, string]>; + let mockClient: any; + + beforeEach(() => { + mockClient = { + query: jest.fn(async () => ({ rows: [] })), + release: jest.fn(), + }; + + mockPool = { + query: jest.fn(), + connect: jest.fn(async () => mockClient), + }; + + emailSender = jest.fn, [string, string, string]>(async () => {}); + + app = express(); + app.use(express.json()); + app.use(createPasswordResetRouter(mockPool, { emailSender, appUrl: 'http://localhost' })); + app.use(((err, req, res, next) => errorHandler(err, req, res, next)) as ErrorRequestHandler); + }); + + it('returns 200 for an unknown email without exposing user existence', async () => { + mockPool.query.mockResolvedValueOnce({ rows: [] }); + + const response = await request(app) + .post('/api/auth/forgot-password') + .send({ email: 'unknown@example.com' }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + message: 'If the email exists, a password reset link has been sent', + }); + expect(emailSender).not.toHaveBeenCalled(); + }); + + it('returns 200 for invalid email format on request password reset', async () => { + const response = await request(app) + .post('/api/auth/forgot-password') + .send({ email: 'invalid-email' }); + + expect(response.status).toBe(200); + expect(response.body).toEqual({ + message: 'If the email exists, a password reset link has been sent', + }); + }); + + it('rate limits repeated password reset requests from the same IP', async () => { + mockPool.query.mockResolvedValue({ rows: [] }); + + for (let i = 0; i < 5; i++) { + await request(app) + .post('/api/auth/forgot-password') + .send({ email: `test${i}@example.com` }); + } + + const response = await request(app) + .post('/api/auth/forgot-password') + .send({ email: 'test6@example.com' }); + + expect(response.status).toBe(429); + expect(response.body).toEqual(expect.objectContaining({ + code: 'TOO_MANY_REQUESTS', + })); + }); + + it('returns 400 for reset-password with unknown token', async () => { + mockPool.connect.mockResolvedValue(mockClient); + mockClient.query.mockImplementation(async (sql: string) => { + if (sql === 'BEGIN' || sql === 'ROLLBACK') { + return { rows: [] }; + } + return { rows: [] }; + }); + + const response = await request(app) + .post('/api/auth/reset-password') + .send({ token: 'f'.repeat(64), password: 'StrongPassword123!' }); + + expect(response.status).toBe(400); + expect(response.body).toEqual(expect.objectContaining({ + code: 'BAD_REQUEST', + })); + }); +}); diff --git a/src/routes/passwordReset.ts b/src/routes/passwordReset.ts index 9d688e6e..03bad729 100644 --- a/src/routes/passwordReset.ts +++ b/src/routes/passwordReset.ts @@ -1,68 +1,68 @@ -import { Router, Request, Response } from 'express'; +import { Router, Request, Response, NextFunction } from 'express'; import { Pool } from 'pg'; -import { PasswordResetService, PasswordResetRateLimitedError } from '../services/passwordResetService'; -import { PasswordResetRateLimiter } from '../services/passwordResetRateLimiter'; +import { z } from 'zod'; +import { PasswordResetService, EmailSender } from '../services/passwordResetService'; +import { createPasswordResetRateLimiter } from '../middleware/passwordResetRateLimiter'; +import { Errors } from '../lib/errors'; -const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; -const MIN_PASSWORD_LENGTH = 8; +const forgotPasswordSchema = z.object({ + email: z.string().email(), +}); -export function createPasswordResetRouter(db: Pool): Router { +const resetPasswordSchema = z.object({ + token: z.string().length(64).regex(/^[0-9a-fA-F]{64}$/), + password: z.string().min(12), +}); + +const NEUTRAL_FORGOT_RESPONSE = { + message: 'If the email exists, a password reset link has been sent', +}; + +export interface PasswordResetRouterOptions { + emailSender?: EmailSender; + appUrl?: string; +} + +export function createPasswordResetRouter(db: Pool, opts?: PasswordResetRouterOptions): Router { const router = Router(); - const rateLimiter = new PasswordResetRateLimiter(db, { - maxRequests: 3, - windowMinutes: 60, - blockMinutes: 15, - }); const service = new PasswordResetService(db, { - emailSender: async (to, subject, body) => { - console.log(`[email] to=${to} subject="${subject}" body="${body}"`); - }, - rateLimiter, + emailSender: opts?.emailSender, + appUrl: opts?.appUrl, }); - router.post('/api/auth/forgot-password', async (req: Request, res: Response) => { - const { email } = req.body ?? {}; - if (typeof email !== 'string' || !EMAIL_RE.test(email)) { - res.status(200).json({ - message: 'If the email exists, a password reset link has been sent', - }); - return; - } - try { - await service.requestPasswordReset(email); - } catch (err) { - if (err instanceof PasswordResetRateLimitedError) { - res.status(429).json({ - error: err.message, - retryAfter: err.retryAfter, - }); - return; + router.post( + '/api/auth/forgot-password', + createPasswordResetRateLimiter(), + async (req: Request, res: Response, next: NextFunction) => { + try { + const { email } = forgotPasswordSchema.parse(req.body); + await service.requestPasswordReset(email); + return res.status(200).json(NEUTRAL_FORGOT_RESPONSE); + } catch (error) { + if (error instanceof z.ZodError) { + return res.status(200).json(NEUTRAL_FORGOT_RESPONSE); + } + return next(error); } - console.error('[password-reset] Error processing request:', err); - } - res.status(200).json({ - message: 'If the email exists, a password reset link has been sent', - }); - }); + }, + ); - router.post('/api/auth/reset-password', async (req: Request, res: Response) => { - const { token, password } = req.body ?? {}; - if (typeof token !== 'string' || typeof password !== 'string' || password.length < MIN_PASSWORD_LENGTH) { - res.status(400).json({ error: 'Invalid token or password' }); - return; - } - try { - const ok = await service.resetPassword(token, password); - if (!ok) { - res.status(400).json({ error: 'Invalid or expired token' }); - return; + router.post( + '/api/auth/reset-password', + createPasswordResetRateLimiter(), + async (req: Request, res: Response, next: NextFunction) => { + try { + const { token, password } = resetPasswordSchema.parse(req.body); + await service.resetPassword(token, password); + return res.status(200).json({ message: 'Password has been reset' }); + } catch (error) { + if (error instanceof z.ZodError) { + return next(Errors.validationError('Invalid password reset payload', error.issues)); + } + return next(error); } - res.status(200).json({ message: 'Password updated' }); - } catch (err) { - console.error('[password-reset] Reset password error:', err); - res.status(400).json({ error: 'Invalid or expired token' }); - } - }); + }, + ); return router; } diff --git a/src/services/passwordResetService.test.ts b/src/services/passwordResetService.test.ts new file mode 100644 index 00000000..36291e6b --- /dev/null +++ b/src/services/passwordResetService.test.ts @@ -0,0 +1,141 @@ +import { PasswordResetService } from './passwordResetService'; +import { PasswordResetTokenInvalidError } from '../lib/errors'; + +describe('PasswordResetService', () => { + let mockPool: any; + let mockClient: any; + let emailSender: jest.Mock, [string, string, string]>; + let service: PasswordResetService; + + beforeEach(() => { + emailSender = jest.fn(async () => {}); + mockClient = { + query: jest.fn(async () => ({ rows: [] })), + release: jest.fn(), + }; + mockPool = { + connect: jest.fn(async () => mockClient), + query: jest.fn(), + }; + service = new PasswordResetService(mockPool, { + emailSender, + appUrl: 'http://localhost', + }); + }); + + it('returns silently for unknown email and does not send mail', async () => { + mockPool.query.mockResolvedValueOnce({ rows: [] }); + + await expect(service.requestPasswordReset('unknown@example.com')).resolves.toBeUndefined(); + expect(emailSender).not.toHaveBeenCalled(); + expect(mockPool.query).toHaveBeenCalledWith( + expect.stringContaining('SELECT id, email FROM users'), + ['unknown@example.com'], + ); + }); + + it('invalidates old tokens and inserts a new token before sending email', async () => { + mockPool.query.mockResolvedValueOnce({ rows: [{ id: 'user-1', email: 'user@example.com' }] }); + + const sent = await service.requestPasswordReset('user@example.com'); + + expect(mockPool.connect).toHaveBeenCalled(); + expect(mockClient.query).toHaveBeenNthCalledWith(1, 'BEGIN'); + expect(mockClient.query).toHaveBeenCalledWith( + expect.stringContaining('UPDATE password_reset_tokens SET used = TRUE'), + ['user-1'], + ); + expect(mockClient.query).toHaveBeenCalledWith( + expect.stringContaining('INSERT INTO password_reset_tokens'), + expect.any(Array), + ); + expect(mockClient.query).toHaveBeenLastCalledWith('COMMIT'); + expect(emailSender).toHaveBeenCalledTimes(1); + expect(emailSender).toHaveBeenCalledWith( + 'user@example.com', + 'Password reset request', + expect.stringMatching(/Use this secure token to reset your password: [0-9a-f]{64}/), + ); + expect(sent).toBeUndefined(); + }); + + it('throws PasswordResetTokenInvalidError when token is unknown', async () => { + mockPool.connect.mockResolvedValue(mockClient); + mockClient.query.mockImplementation(async (query: string) => { + if (query === 'BEGIN' || query === 'ROLLBACK' || query === 'COMMIT') { + return { rows: [] }; + } + return { rows: [] }; + }); + + await expect(service.resetPassword('f'.repeat(64), 'StrongPass123!')).rejects.toBeInstanceOf( + PasswordResetTokenInvalidError, + ); + expect(mockClient.query).toHaveBeenCalledWith(expect.stringContaining('BEGIN')); + expect(mockClient.query).toHaveBeenCalledWith( + expect.stringContaining('SELECT id, user_id, expires_at, used'), + [expect.any(String)], + ); + }); + + it('throws PasswordResetTokenInvalidError when token was already used', async () => { + const pastDate = new Date(Date.now() + 60_000); + mockPool.connect.mockResolvedValue(mockClient); + mockClient.query.mockImplementation(async (query: string) => { + if (query === 'BEGIN' || query === 'ROLLBACK') { + return { rows: [] }; + } + if (query.includes('SELECT id, user_id, expires_at, used')) { + return { rows: [{ id: 'token-1', user_id: 'user-1', expires_at: pastDate, used: true }] }; + } + return { rows: [] }; + }); + + await expect(service.resetPassword('f'.repeat(64), 'StrongPass123!')).rejects.toBeInstanceOf( + PasswordResetTokenInvalidError, + ); + }); + + it('throws PasswordResetTokenInvalidError when token is expired', async () => { + const expired = new Date(Date.now() - 1000); + mockPool.connect.mockResolvedValue(mockClient); + mockClient.query.mockImplementation(async (query: string) => { + if (query === 'BEGIN' || query === 'ROLLBACK') { + return { rows: [] }; + } + if (query.includes('SELECT id, user_id, expires_at, used')) { + return { rows: [{ id: 'token-1', user_id: 'user-1', expires_at: expired, used: false }] }; + } + return { rows: [] }; + }); + + await expect(service.resetPassword('f'.repeat(64), 'StrongPass123!')).rejects.toBeInstanceOf( + PasswordResetTokenInvalidError, + ); + }); + + it('updates password and marks token used when reset token is valid', async () => { + const futureDate = new Date(Date.now() + 60_000); + mockPool.connect.mockResolvedValue(mockClient); + mockClient.query.mockImplementation(async (query: string) => { + if (query === 'BEGIN' || query === 'COMMIT' || query === 'ROLLBACK') { + return { rows: [] }; + } + if (query.includes('SELECT id, user_id, expires_at, used')) { + return { rows: [{ id: 'token-1', user_id: 'user-1', expires_at: futureDate, used: false }] }; + } + return { rows: [] }; + }); + + await expect(service.resetPassword('f'.repeat(64), 'StrongPass123!')).resolves.toBeUndefined(); + expect(mockClient.query).toHaveBeenCalledWith( + expect.stringContaining('UPDATE password_reset_tokens SET used = TRUE'), + ['token-1'], + ); + expect(mockClient.query).toHaveBeenCalledWith( + expect.stringContaining('UPDATE users SET password_hash = $1 WHERE id = $2'), + expect.any(Array), + ); + expect(mockClient.query).toHaveBeenLastCalledWith('COMMIT'); + }); +}); diff --git a/src/services/passwordResetService.ts b/src/services/passwordResetService.ts index e434381e..212f57a2 100644 --- a/src/services/passwordResetService.ts +++ b/src/services/passwordResetService.ts @@ -1,6 +1,8 @@ import { Pool, PoolClient } from 'pg'; import { randomBytes, createHash } from 'node:crypto'; -import { PasswordResetRateLimiter, RateLimitResult } from './passwordResetRateLimiter'; +import { hashPassword as hashUserPassword } from '../utils/password'; +import { logger } from '../lib/logger'; +import { PasswordResetTokenInvalidError, Errors } from '../lib/errors'; export type EmailSender = (to: string, subject: string, body: string) => Promise; @@ -8,53 +10,27 @@ export interface PasswordResetServiceOptions { emailSender?: EmailSender; tokenTtlMinutes?: number; appUrl?: string; - rateLimiter?: PasswordResetRateLimiter; -} - -export class PasswordResetRateLimitedError extends Error { - constructor( - message: string, - public readonly retryAfter: number, - public readonly rateLimitResult: RateLimitResult - ) { - super(message); - this.name = 'PasswordResetRateLimitedError'; - } } export class PasswordResetService { - private emailSender: EmailSender; - private tokenTtlMinutes: number; - private appUrl: string; - private rateLimiter?: PasswordResetRateLimiter; + private readonly emailSender: EmailSender; + private readonly tokenTtlMinutes: number; + private readonly appUrl: string; constructor(private readonly db: Pool, opts?: PasswordResetServiceOptions) { - this.emailSender = opts?.emailSender ?? (async (_to, _subject, _body) => {}); - this.tokenTtlMinutes = opts?.tokenTtlMinutes ?? 60; + this.emailSender = opts?.emailSender ?? (async () => {}); + this.tokenTtlMinutes = opts?.tokenTtlMinutes ?? 15; this.appUrl = opts?.appUrl ?? process.env.APP_URL ?? 'http://localhost:3000'; - this.rateLimiter = opts?.rateLimiter; } async requestPasswordReset(emailRaw: string): Promise { const email = emailRaw.trim().toLowerCase(); - - if (this.rateLimiter) { - try { - const rateLimitResult = await this.rateLimiter.checkRateLimit(email); - if (!rateLimitResult.allowed) { - throw new PasswordResetRateLimitedError( - 'Too many password reset requests. Please try again later.', - rateLimitResult.retryAfter ?? this.rateLimiter['blockMinutes'] * 60, - rateLimitResult - ); - } - } catch (err) { - console.error('[password-reset] Rate limiter error, failing open:', err); - } - } - const user = await this.findUserByEmail(email); + if (!user) { + logger.info('Password reset request for unknown user', { + event: 'password_reset_unknown_email', + }); return; } @@ -62,28 +38,47 @@ export class PasswordResetService { const tokenHash = this.hashToken(token); const expiresAt = new Date(Date.now() + this.tokenTtlMinutes * 60_000); - await this.db.query( - `INSERT INTO password_reset_tokens (user_id, token_hash, expires_at) VALUES ($1, $2, $3)`, - [user.id, tokenHash, expiresAt], - ); + const client = await this.db.connect(); + try { + await client.query('BEGIN'); + await client.query( + `UPDATE password_reset_tokens SET used = TRUE WHERE user_id = $1 AND used = FALSE`, + [user.id], + ); + await client.query( + `INSERT INTO password_reset_tokens (user_id, token_hash, expires_at) VALUES ($1, $2, $3)`, + [user.id, tokenHash, expiresAt], + ); + await client.query('COMMIT'); + } catch (error) { + await client.query('ROLLBACK'); + logger.error('Failed to create password reset token', { error }); + throw Errors.internal('Unable to process password reset request'); + } finally { + client.release(); + } + + logger.info('Password reset token issued', { + userId: user.id, + expiresAt: expiresAt.toISOString(), + }); - const resetLink = `${this.appUrl}/reset-password?token=${encodeURIComponent(token)}`; await this.emailSender( user.email, - 'Password Reset', - `Use this link to reset your password: ${resetLink}`, + 'Password reset request', + `Use this secure token to reset your password: ${token}`, ); } - async resetPassword(tokenRaw: string, newPassword: string): Promise { - const tokenHash = this.hashToken(tokenRaw); - let client: PoolClient | null = null; + async resetPassword(tokenRaw: string, newPassword: string): Promise { + const tokenHash = this.hashToken(tokenRaw.trim()); + const client = await this.db.connect(); + try { - client = await this.db.connect(); await client.query('BEGIN'); const { rows } = await client.query( - `SELECT id, user_id, expires_at, used_at + `SELECT id, user_id, expires_at, used FROM password_reset_tokens WHERE token_hash = $1 FOR UPDATE`, @@ -92,38 +87,63 @@ export class PasswordResetService { if (rows.length === 0) { await client.query('ROLLBACK'); - return false; + logger.warn('Password reset attempt failed: invalid token', { + event: 'password_reset_invalid_token', + }); + throw new PasswordResetTokenInvalidError(); } - const row = rows[0] as { id: string; user_id: string; expires_at: Date; used_at: Date | null }; - if (row.used_at || new Date(row.expires_at) < new Date()) { + const row = rows[0] as { + id: string; + user_id: string; + expires_at: Date; + used: boolean; + }; + + if (row.used) { await client.query('ROLLBACK'); - return false; + logger.warn('Password reset attempt failed: token already used', { + event: 'password_reset_used_token', + userId: row.user_id, + }); + throw new PasswordResetTokenInvalidError(); } - const passwordHash = this.hashPassword(newPassword); + if (new Date(row.expires_at) < new Date()) { + await client.query('ROLLBACK'); + logger.warn('Password reset attempt failed: token expired', { + event: 'password_reset_expired_token', + userId: row.user_id, + }); + throw new PasswordResetTokenInvalidError(); + } await client.query( - `UPDATE users SET password_hash = $1 WHERE id = $2`, - [passwordHash, row.user_id], + `UPDATE password_reset_tokens SET used = TRUE WHERE id = $1`, + [row.id], ); + const passwordHash = await hashUserPassword(newPassword); await client.query( - `UPDATE password_reset_tokens SET used_at = NOW() WHERE id = $1`, - [row.id], + `UPDATE users SET password_hash = $1 WHERE id = $2`, + [passwordHash, row.user_id], ); - await client.query('COMMIT'); - return true; - } catch { - if (client) { - try { - await client.query('ROLLBACK'); - } catch {} + + logger.info('Password reset token used', { + userId: row.user_id, + tokenId: row.id, + }); + } catch (error) { + await client.query('ROLLBACK'); + if (error instanceof PasswordResetTokenInvalidError) { + throw error; } - throw new Error('Password reset failed'); + + logger.error('Password reset failed', { error }); + throw Errors.internal('Unable to reset password'); } finally { - client?.release(); + client.release(); } } @@ -132,7 +152,11 @@ export class PasswordResetService { `SELECT id, email FROM users WHERE LOWER(email) = $1 LIMIT 1`, [email], ); - if (rows.length === 0) return null; + + if (rows.length === 0) { + return null; + } + return { id: rows[0].id, email: rows[0].email }; } @@ -143,9 +167,5 @@ export class PasswordResetService { private hashToken(token: string): string { return createHash('sha256').update(token).digest('hex'); } - - private hashPassword(plaintext: string): string { - return createHash('sha256').update(plaintext).digest('hex'); - } }