diff --git a/src/api/auth/login.ts b/src/api/auth/login.ts index d0d26cd0..19e25bd0 100644 --- a/src/api/auth/login.ts +++ b/src/api/auth/login.ts @@ -4,10 +4,37 @@ import type { Context } from 'hono'; import { setCookie } from 'hono/cookie'; import { createSession, getUserByEmail } from '../../db/repositories/usersRepository.js'; import { SESSION_COOKIE_NAME } from './cookie.js'; +import { checkRateLimit, recordSuccessfulLogin } from './rateLimiter.js'; const SESSION_EXPIRY_DAYS = 30; +/** + * Extract the client IP from a Hono context. + * Checks x-forwarded-for first (for reverse-proxy deployments), then falls + * back to the raw remote address. + */ +function getClientIp(c: Context): string { + const forwarded = c.req.header('x-forwarded-for'); + if (forwarded) { + // x-forwarded-for may contain a comma-separated list; take the first value + return forwarded.split(',')[0].trim(); + } + // Hono exposes the raw Request; in Node.js the remote address isn't directly + // available on Request, so fall back to a sentinel value that still works for + // rate limiting purposes in environments that don't set x-forwarded-for. + return 'unknown'; +} + export async function loginHandler(c: Context) { + const ip = getClientIp(c); + + // Rate-limit check (before parsing credentials to avoid wasted work) + const rateCheck = checkRateLimit(ip); + if (rateCheck.limited) { + c.header('Retry-After', String(rateCheck.retryAfterSeconds)); + return c.json({ error: 'Too many login attempts. Please try again later.' }, 429); + } + const body = await c.req.json<{ email?: string; password?: string }>(); if (!body.email || !body.password) { return c.json({ error: 'Email and password are required' }, 400); @@ -23,6 +50,9 @@ export async function loginHandler(c: Context) { return c.json({ error: 'Invalid credentials' }, 401); } + // Successful login — reset the rate-limit counter for this IP + recordSuccessfulLogin(ip); + const token = randomBytes(64).toString('hex'); const expiresAt = new Date(Date.now() + SESSION_EXPIRY_DAYS * 24 * 60 * 60 * 1000); diff --git a/src/api/auth/rateLimiter.ts b/src/api/auth/rateLimiter.ts new file mode 100644 index 00000000..e09eb35c --- /dev/null +++ b/src/api/auth/rateLimiter.ts @@ -0,0 +1,108 @@ +/** + * In-memory sliding-window rate limiter for the login endpoint. + * + * Tracks login attempts per IP address. After MAX_ATTEMPTS within the + * WINDOW_MS window, subsequent requests are rejected with a 429 response + * that includes a Retry-After header. + * + * Only failed login attempts are counted. A successful login resets the + * counter for the IP so it is not counted against the rate limit. + * + * A cleanup interval runs every CLEANUP_INTERVAL_MS to evict expired entries + * and prevent unbounded memory growth. + */ + +export const MAX_ATTEMPTS = 10; +export const WINDOW_MS = 60_000; // 1 minute + +const CLEANUP_INTERVAL_MS = 5 * 60_000; // 5 minutes + +interface RateLimitEntry { + count: number; + resetAt: number; +} + +// Exported for testing +export const rateLimitStore = new Map(); + +let cleanupTimer: ReturnType | null = null; + +/** + * Start the periodic cleanup interval (idempotent). + * Called lazily on first use so tests can control timing. + */ +function ensureCleanupStarted(): void { + if (cleanupTimer !== null) return; + cleanupTimer = setInterval(() => { + const now = Date.now(); + for (const [ip, entry] of rateLimitStore) { + if (now >= entry.resetAt) { + rateLimitStore.delete(ip); + } + } + }, CLEANUP_INTERVAL_MS); + // Allow Node.js process to exit even if the interval is running + if (cleanupTimer.unref) { + cleanupTimer.unref(); + } +} + +/** + * Reset the cleanup timer — used in tests only. + */ +export function _resetForTesting(): void { + if (cleanupTimer !== null) { + clearInterval(cleanupTimer); + cleanupTimer = null; + } + rateLimitStore.clear(); +} + +/** + * Run the cleanup sweep — used in tests only. + */ +export function _runCleanup(): void { + const now = Date.now(); + for (const [ip, entry] of rateLimitStore) { + if (now >= entry.resetAt) { + rateLimitStore.delete(ip); + } + } +} + +/** + * Check whether the IP has exceeded the rate limit. + * + * @returns `{ limited: false }` when within the limit, or + * `{ limited: true, retryAfterSeconds: number }` when over the limit. + */ +export function checkRateLimit( + ip: string, +): { limited: false } | { limited: true; retryAfterSeconds: number } { + ensureCleanupStarted(); + + const now = Date.now(); + const entry = rateLimitStore.get(ip); + + if (!entry || now >= entry.resetAt) { + // No entry or window has expired — first attempt in a new window + rateLimitStore.set(ip, { count: 1, resetAt: now + WINDOW_MS }); + return { limited: false }; + } + + if (entry.count >= MAX_ATTEMPTS) { + const retryAfterSeconds = Math.ceil((entry.resetAt - now) / 1000); + return { limited: true, retryAfterSeconds }; + } + + entry.count += 1; + return { limited: false }; +} + +/** + * Record a successful login for the IP — resets the counter so successful + * logins are not counted against the rate limit. + */ +export function recordSuccessfulLogin(ip: string): void { + rateLimitStore.delete(ip); +} diff --git a/tests/unit/api/auth/login.test.ts b/tests/unit/api/auth/login.test.ts index 7544a400..8313bb91 100644 --- a/tests/unit/api/auth/login.test.ts +++ b/tests/unit/api/auth/login.test.ts @@ -1,9 +1,11 @@ import { Hono } from 'hono'; -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const mockGetUserByEmail = vi.fn(); const mockCreateSession = vi.fn(); const mockBcryptCompare = vi.fn(); +const mockCheckRateLimit = vi.fn(); +const mockRecordSuccessfulLogin = vi.fn(); vi.mock('../../../../src/db/repositories/usersRepository.js', () => ({ getUserByEmail: (...args: unknown[]) => mockGetUserByEmail(...args), @@ -16,6 +18,11 @@ vi.mock('bcrypt', () => ({ }, })); +vi.mock('../../../../src/api/auth/rateLimiter.js', () => ({ + checkRateLimit: (...args: unknown[]) => mockCheckRateLimit(...args), + recordSuccessfulLogin: (...args: unknown[]) => mockRecordSuccessfulLogin(...args), +})); + import { SESSION_COOKIE_NAME } from '../../../../src/api/auth/cookie.js'; import { loginHandler } from '../../../../src/api/auth/login.js'; @@ -25,10 +32,10 @@ function createTestApp() { return app; } -function postLogin(app: Hono, body: Record) { +function postLogin(app: Hono, body: Record, headers?: Record) { return app.request('/api/auth/login', { method: 'POST', - headers: { 'Content-Type': 'application/json' }, + headers: { 'Content-Type': 'application/json', ...headers }, body: JSON.stringify(body), }); } @@ -43,6 +50,16 @@ const mockUser = { }; describe('loginHandler', () => { + beforeEach(() => { + // Default: not rate-limited + mockCheckRateLimit.mockReturnValue({ limited: false }); + mockRecordSuccessfulLogin.mockReturnValue(undefined); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + it('returns 400 when email is missing', async () => { const app = createTestApp(); const res = await postLogin(app, { password: 'pass' }); @@ -123,4 +140,84 @@ describe('loginHandler', () => { const expectedExpiry = Date.now() + thirtyDaysMs; expect(Math.abs(expiresAt.getTime() - expectedExpiry)).toBeLessThan(5000); }); + + describe('rate limiting', () => { + it('returns 429 when the rate limit is exceeded', async () => { + mockCheckRateLimit.mockReturnValue({ limited: true, retryAfterSeconds: 45 }); + const app = createTestApp(); + + const res = await postLogin(app, { email: 'test@example.com', password: 'pass' }); + + expect(res.status).toBe(429); + const body = await res.json(); + expect(body.error).toMatch(/too many/i); + }); + + it('includes Retry-After header when rate-limited', async () => { + mockCheckRateLimit.mockReturnValue({ limited: true, retryAfterSeconds: 30 }); + const app = createTestApp(); + + const res = await postLogin(app, { email: 'test@example.com', password: 'pass' }); + + expect(res.headers.get('Retry-After')).toBe('30'); + }); + + it('does not call getUserByEmail when rate-limited', async () => { + mockCheckRateLimit.mockReturnValue({ limited: true, retryAfterSeconds: 10 }); + const app = createTestApp(); + + await postLogin(app, { email: 'test@example.com', password: 'pass' }); + + expect(mockGetUserByEmail).not.toHaveBeenCalled(); + }); + + it('calls checkRateLimit with the IP from x-forwarded-for header', async () => { + const app = createTestApp(); + + await postLogin( + app, + { email: 'test@example.com', password: 'pass' }, + { 'x-forwarded-for': '203.0.113.42' }, + ); + + expect(mockCheckRateLimit).toHaveBeenCalledWith('203.0.113.42'); + }); + + it('uses the first IP when x-forwarded-for is a comma-separated list', async () => { + const app = createTestApp(); + + await postLogin( + app, + { email: 'test@example.com', password: 'pass' }, + { 'x-forwarded-for': '203.0.113.42, 10.0.0.1, 192.168.1.1' }, + ); + + expect(mockCheckRateLimit).toHaveBeenCalledWith('203.0.113.42'); + }); + + it('calls recordSuccessfulLogin on successful authentication', async () => { + mockGetUserByEmail.mockResolvedValue(mockUser); + mockBcryptCompare.mockResolvedValue(true); + mockCreateSession.mockResolvedValue('session-id'); + const app = createTestApp(); + + await postLogin( + app, + { email: 'test@example.com', password: 'correct' }, + { 'x-forwarded-for': '203.0.113.42' }, + ); + + expect(mockRecordSuccessfulLogin).toHaveBeenCalledWith('203.0.113.42'); + }); + + it('does not call recordSuccessfulLogin on failed authentication', async () => { + mockGetUserByEmail.mockResolvedValue(mockUser); + mockBcryptCompare.mockResolvedValue(false); + const app = createTestApp(); + + await postLogin(app, { email: 'test@example.com', password: 'wrong' }); + + expect(mockRecordSuccessfulLogin).not.toHaveBeenCalled(); + }); + }); }); diff --git a/tests/unit/api/auth/rateLimiter.test.ts b/tests/unit/api/auth/rateLimiter.test.ts new file mode 100644 index 00000000..9fbc8731 --- /dev/null +++ b/tests/unit/api/auth/rateLimiter.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + MAX_ATTEMPTS, + WINDOW_MS, + _resetForTesting, + _runCleanup, + checkRateLimit, + rateLimitStore, + recordSuccessfulLogin, +} from '../../../../src/api/auth/rateLimiter.js'; + +describe('rateLimiter', () => { + beforeEach(() => { + _resetForTesting(); + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + _resetForTesting(); + }); + + describe('checkRateLimit — under the limit', () => { + it('allows the first attempt from a new IP', () => { + const result = checkRateLimit('1.2.3.4'); + expect(result).toEqual({ limited: false }); + }); + + it('allows attempts up to MAX_ATTEMPTS without blocking', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + const result = checkRateLimit('1.2.3.4'); + expect(result).toEqual({ limited: false }); + } + }); + + it('tracks attempt counts per IP independently', () => { + for (let i = 0; i < MAX_ATTEMPTS - 1; i++) { + checkRateLimit('1.1.1.1'); + } + // Different IP should still be under limit + const result = checkRateLimit('2.2.2.2'); + expect(result).toEqual({ limited: false }); + }); + }); + + describe('checkRateLimit — at and over the limit', () => { + it('blocks the (MAX_ATTEMPTS + 1)th attempt from the same IP', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.2.3.4'); + } + const result = checkRateLimit('1.2.3.4'); + expect(result).toMatchObject({ limited: true }); + }); + + it('returns retryAfterSeconds close to the window length on first block', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.2.3.4'); + } + const result = checkRateLimit('1.2.3.4'); + expect(result.limited).toBe(true); + if (result.limited) { + // retryAfterSeconds should be <= ceil(WINDOW_MS / 1000) + expect(result.retryAfterSeconds).toBeGreaterThan(0); + expect(result.retryAfterSeconds).toBeLessThanOrEqual(WINDOW_MS / 1000); + } + }); + + it('continues blocking subsequent requests after the limit is reached', () => { + for (let i = 0; i < MAX_ATTEMPTS + 5; i++) { + checkRateLimit('1.2.3.4'); + } + const result = checkRateLimit('1.2.3.4'); + expect(result).toMatchObject({ limited: true }); + }); + }); + + describe('checkRateLimit — window reset', () => { + it('allows requests again after the window expires', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.2.3.4'); + } + // Advance past the window + vi.advanceTimersByTime(WINDOW_MS + 1); + + const result = checkRateLimit('1.2.3.4'); + expect(result).toEqual({ limited: false }); + }); + + it('resets the count when the window expires', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.2.3.4'); + } + vi.advanceTimersByTime(WINDOW_MS + 1); + + // Should be able to make MAX_ATTEMPTS new attempts in the fresh window + for (let i = 0; i < MAX_ATTEMPTS; i++) { + const result = checkRateLimit('1.2.3.4'); + expect(result).toEqual({ limited: false }); + } + }); + }); + + describe('recordSuccessfulLogin', () => { + it('resets the rate-limit counter so subsequent attempts are allowed', () => { + // Exhaust the limit + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.2.3.4'); + } + expect(checkRateLimit('1.2.3.4')).toMatchObject({ limited: true }); + + // Successful login clears the entry + recordSuccessfulLogin('1.2.3.4'); + + // Now the same IP should be allowed again + const result = checkRateLimit('1.2.3.4'); + expect(result).toEqual({ limited: false }); + }); + + it('does not affect other IPs when called', () => { + for (let i = 0; i < MAX_ATTEMPTS; i++) { + checkRateLimit('1.1.1.1'); + checkRateLimit('2.2.2.2'); + } + + recordSuccessfulLogin('1.1.1.1'); + + // IP 1 should be cleared + expect(checkRateLimit('1.1.1.1')).toEqual({ limited: false }); + // IP 2 should still be blocked + expect(checkRateLimit('2.2.2.2')).toMatchObject({ limited: true }); + }); + + it('is a no-op for an IP that has no entry', () => { + expect(() => recordSuccessfulLogin('9.9.9.9')).not.toThrow(); + }); + }); + + describe('cleanup — memory leak prevention', () => { + it('removes expired entries when cleanup runs', () => { + checkRateLimit('1.2.3.4'); + expect(rateLimitStore.size).toBe(1); + + // Advance past the window so the entry is expired + vi.advanceTimersByTime(WINDOW_MS + 1); + _runCleanup(); + + expect(rateLimitStore.size).toBe(0); + }); + + it('does not remove entries that are still within their window', () => { + checkRateLimit('1.2.3.4'); + // Advance but NOT past the window + vi.advanceTimersByTime(WINDOW_MS - 1000); + _runCleanup(); + + expect(rateLimitStore.size).toBe(1); + }); + + it('only removes expired entries, leaving active ones intact', () => { + vi.setSystemTime(0); + checkRateLimit('old-ip'); + + // Advance past the first window, then create a second entry + vi.advanceTimersByTime(WINDOW_MS + 1); + checkRateLimit('new-ip'); + + _runCleanup(); + + expect(rateLimitStore.has('old-ip')).toBe(false); + expect(rateLimitStore.has('new-ip')).toBe(true); + }); + }); +});