diff --git a/backend/prisma/migrations/20261015175723_add_password_reset_tokens/migration.sql b/backend/prisma/migrations/20261015175723_add_password_reset_tokens/migration.sql new file mode 100644 index 00000000..77f2c836 --- /dev/null +++ b/backend/prisma/migrations/20261015175723_add_password_reset_tokens/migration.sql @@ -0,0 +1,24 @@ +-- AlterTable +ALTER TABLE "User" ADD COLUMN "tokenVersion" INTEGER NOT NULL DEFAULT 0; + +-- CreateTable +CREATE TABLE "PasswordResetToken" ( + "id" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "tokenHash" TEXT NOT NULL, + "expiresAt" TIMESTAMP(3) NOT NULL, + "usedAt" TIMESTAMP(3), + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "PasswordResetToken_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "PasswordResetToken_tokenHash_key" ON "PasswordResetToken"("tokenHash"); + +-- CreateIndex +CREATE INDEX "PasswordResetToken_userId_idx" ON "PasswordResetToken"("userId"); + +-- AddForeignKey +ALTER TABLE "PasswordResetToken" ADD CONSTRAINT "PasswordResetToken_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 2335c33b..94fba03f 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -124,6 +124,12 @@ model User { // refresh-token revocation list (issue #18) — this is the narrow "revoke // everything for one account" case that deactivation needs. sessionsInvalidatedAt DateTime? + // Per-user refresh-token version. Embedded in every access/refresh token + // as `tv` and compared against this column at refresh time. Bumping it + // (password reset, Phase 8 #33) invalidates every outstanding refresh + // token for the user, since none of them carry the new value — the + // interim kill switch issue #33 asked for while #18 doesn't exist yet. + tokenVersion Int @default(0) createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -133,6 +139,7 @@ model User { refreshSessions RefreshSession[] emailVerificationTokens EmailVerificationToken[] + passwordResetTokens PasswordResetToken[] adminInvitesSent AdminInvite[] // Phase 8 (issue #45): PHI read-access log rows this user wrote (as viewer). // Opposite of AccessLog.accessedByUser. @@ -158,6 +165,27 @@ model EmailVerificationToken { @@index([userId]) } +// Password reset tokens (Phase 8 #33). Only the SHA-256 hash of the +// token is ever stored — the raw token exists exactly once, in the +// notification that carries the reset link, and is never logged or +// persisted. Single-use (`usedAt`) and expiring (`expiresAt`). +model PasswordResetToken { + id String @id @default(uuid()) + userId String + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + // SHA-256 hex of the raw token. Unique so lookup-by-hash is a single + // indexed read, and a duplicate hash (astronomically unlikely) is a + // constraint violation rather than a silent overwrite. + tokenHash String @unique + expiresAt DateTime + // Non-null once used or superseded by a newer request — either way the + // token is dead. Requesting a second reset marks the first one used. + usedAt DateTime? + createdAt DateTime @default(now()) + + @@index([userId]) +} + model AdminInvite { id String @id @default(uuid()) email String diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index 911967f8..f059127b 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -43,6 +43,21 @@ export const config = { accessTtl: '15m', refreshTtl: '30d', }, + // Base URL of docforum-web, used to build the link in outbound + // notifications (password reset today). Not a provider decision — see + // issue #17 — just the URL the recipient is sent to. + appBaseUrl: process.env.APP_BASE_URL ?? 'http://localhost:5173', + passwordReset: { + // Raw tokens are single-use and short-lived; only their SHA-256 hash + // is stored (see PasswordResetToken in schema.prisma). + tokenTtlMinutes: Number(process.env.PASSWORD_RESET_TOKEN_TTL_MINUTES ?? 30), + // Rate limits on POST /auth/password-reset/request, so the endpoint + // can't be used to spam an inbox. Applied per email AND per IP — + // either limit being hit returns 429. See auth/services/rate-limit.ts + // for the (in-memory, single-process) caveat. + requestsPerEmailPerHour: Number(process.env.PASSWORD_RESET_REQUESTS_PER_EMAIL_PER_HOUR ?? 3), + requestsPerIpPerHour: Number(process.env.PASSWORD_RESET_REQUESTS_PER_IP_PER_HOUR ?? 10), + }, // Phase 8 hardening knobs (issues #34/#35/#37). Env-configurable so a // preview deployment can be tightened without a code change, and so // tests can set short windows instead of waiting real minutes. diff --git a/backend/src/modules/auth/controllers/index.ts b/backend/src/modules/auth/controllers/index.ts index 4bb6134e..71cc68ce 100644 --- a/backend/src/modules/auth/controllers/index.ts +++ b/backend/src/modules/auth/controllers/index.ts @@ -3,6 +3,8 @@ import * as authService from '../services'; import { clearLoginAttemptsSchema, loginSchema, + passwordResetConfirmSchema, + passwordResetRequestSchema, signupSchema, signupWithInviteSchema, verifyEmailSchema, @@ -145,6 +147,28 @@ export function logout(req: Request, res: Response) { res.status(204).send(); } +// Both reset endpoints answer identically for known and unknown emails — +// same status, same body, no branch on the result of the lookup. Rate +// limiting is the one intentional exception (429), and it fires before any +// account lookup, so it can't leak existence either. +export async function requestPasswordReset(req: Request, res: Response) { + const { email } = passwordResetRequestSchema.parse(req.body); + await authService.requestPasswordReset({ email, ip: req.ip ?? 'unknown' }); + res.status(202).json({ + message: 'If an account exists for that email, a password reset link has been sent.', + }); +} + +export async function confirmPasswordReset(req: Request, res: Response) { + const input = passwordResetConfirmSchema.parse(req.body); + await authService.confirmPasswordReset(input); + // The reset already killed every refresh token for this user + // (tokenVersion bump) — clear the caller's cookie too so a browser that + // still holds the old one isn't left sending a dead token. + res.clearCookie(REFRESH_COOKIE, config.cookie); + res.status(200).json({ message: 'Password updated. Please sign in with your new password.' }); +} + // --- Session management (Phase 8 #38) ----------------------------------- function refreshCookieValue(req: Request): string | undefined { diff --git a/backend/src/modules/auth/repositories/index.ts b/backend/src/modules/auth/repositories/index.ts index 697f47b8..98dba01d 100644 --- a/backend/src/modules/auth/repositories/index.ts +++ b/backend/src/modules/auth/repositories/index.ts @@ -1,7 +1,7 @@ // Auth module's repository — the only place that touches the `User` / -// `PatientProfile` / `DoctorProfile` tables for auth purposes. Other -// modules must go through this module's services, never import this file -// directly (AGENTS.md hard rule 4). +// `PatientProfile` / `DoctorProfile` / `PasswordResetToken` tables for auth +// purposes. Other modules must go through this module's services, never +// import this file directly (AGENTS.md hard rule 4). import { prisma } from '../../../db/prisma-client'; import type { Role } from '@prisma/client'; @@ -230,3 +230,58 @@ export function clearLoginAttempts(buckets: { scope: string; key: string }[]) { export function pruneStaleLoginAttempts(before: Date) { return prisma.loginAttempt.deleteMany({ where: { lastFailedAt: { lt: before } } }).then((result) => result.count); } + +/** + * Stores a new single-use reset token for a user, superseding any of that + * user's outstanding ones in the same transaction — "requesting a second + * reset invalidates the first token" is a DB-level guarantee here, not an + * application-level hope. + */ +export function createPasswordResetToken(input: { + userId: string; + tokenHash: string; + expiresAt: Date; +}) { + return prisma.$transaction(async (tx) => { + await tx.passwordResetToken.updateMany({ + where: { userId: input.userId, usedAt: null }, + data: { usedAt: new Date() }, + }); + return tx.passwordResetToken.create({ + data: { userId: input.userId, tokenHash: input.tokenHash, expiresAt: input.expiresAt }, + }); + }); +} + +/** Lookup by token hash (unique index) — never by the raw token, which is + * never stored. */ +export function findPasswordResetTokenByHash(tokenHash: string) { + return prisma.passwordResetToken.findUnique({ where: { tokenHash } }); +} + +/** + * Consumes a reset token and applies the new password in one transaction: + * marks the token used, sets the new password hash, and increments + * `User.tokenVersion` so every previously issued refresh token for this + * user stops working (issue #33; the full revocation list is #18). + */ +export function consumePasswordResetToken(input: { + tokenId: string; + userId: string; + passwordHash: string; +}) { + return prisma.$transaction(async (tx) => { + const claimed = await tx.passwordResetToken.updateMany({ + where: { id: input.tokenId, usedAt: null }, + data: { usedAt: new Date() }, + }); + // Two concurrent confirms of the same token: only one claims the row. + if (claimed.count !== 1) return false; + + await tx.user.update({ + where: { id: input.userId }, + data: { passwordHash: input.passwordHash, tokenVersion: { increment: 1 } }, + }); + return true; + }); +} diff --git a/backend/src/modules/auth/routes/index.ts b/backend/src/modules/auth/routes/index.ts index 4170386c..20ee8ccd 100644 --- a/backend/src/modules/auth/routes/index.ts +++ b/backend/src/modules/auth/routes/index.ts @@ -28,6 +28,11 @@ authRouter.post('/login-attempts/clear', authenticate, requireRole('admin'), aut authRouter.post('/refresh', requireAllowedOrigin, authController.refresh); authRouter.post('/logout', requireAllowedOrigin, authController.logout); +// Password reset (Phase 8 #33) — both unauthenticated; possession of the +// emailed single-use token is the credential for the confirm step. +authRouter.post('/password-reset/request', authController.requestPasswordReset); +authRouter.post('/password-reset/confirm', authController.confirmPasswordReset); + // Session management (Phase 8 #38). Note the ordering: `DELETE /sessions` // (revoke all others) is registered before `DELETE /sessions/:id` — Express // matches in registration order, and `/sessions` must not be read as an id. diff --git a/backend/src/modules/auth/services/index.ts b/backend/src/modules/auth/services/index.ts index f6dd758c..aee73e6b 100644 --- a/backend/src/modules/auth/services/index.ts +++ b/backend/src/modules/auth/services/index.ts @@ -37,6 +37,17 @@ import type { PatientExportSection } from '../../patients/services'; export { verifyAccessToken } from './tokens'; export type { AccessTokenPayload } from './tokens'; +// Password reset (Phase 8 #33) lives in its own file; re-exported here so +// controllers keep calling a single service surface. +export { + confirmPasswordReset, + hashResetToken, + requestPasswordReset, + resetPasswordResetRateLimits, + type ConfirmPasswordResetInput, + type RequestPasswordResetInput, +} from './password-reset'; + export { assertSessionUsable, listSessions, @@ -105,10 +116,10 @@ function toPublicUser(user: { } function issueTokensForSession( - user: { id: string; role: Role }, + user: { id: string; role: Role; tokenVersion?: number }, sessionId: string, ): { accessToken: string; refreshToken: string; csrfToken: string } { - const payload = { sub: user.id, role: user.role }; + const payload = { sub: user.id, role: user.role, tv: user.tokenVersion ?? 0 }; // A fresh CSRF token per session, and per rotation — bound into the // refresh token so verifying it needs no server-side state (issue #37). const csrfToken = generateCsrfToken(); @@ -124,7 +135,7 @@ function issueTokensForSession( /** Starts a new session (one per login) and issues its first token pair. */ async function issueNewSession( - user: { id: string; role: Role }, + user: { id: string; role: Role; tokenVersion?: number }, context: SessionContext, ): Promise<{ accessToken: string; refreshToken: string; csrfToken: string; sessionId: string }> { const session = await startSession({ userId: user.id, ...context }); @@ -397,6 +408,16 @@ export async function refresh(refreshToken: string, csrfToken?: string | null): // session revoked by deactivation/reactivation can't come back to life // through a refresh cookie either (Phase 8, issue #39). if (user.deactivatedAt) throw new HttpError(403, 'This account has been deactivated.'); + + // `tv` check: a password reset (or anything else that bumps + // `User.tokenVersion`) kills every refresh token issued before it. Tokens + // predating this field carry no `tv` and read as version 0, which is + // exactly right — they stay valid only while the user has never had a + // version bump. + if ((payload.tv ?? 0) !== user.tokenVersion) { + throw new HttpError(401, 'Invalid or expired refresh token.'); + } + if (isSessionRevoked(user.sessionsInvalidatedAt, payload.iat)) { throw new HttpError(401, 'Your session has been revoked. Please sign in again.'); } diff --git a/backend/src/modules/auth/services/password-reset.ts b/backend/src/modules/auth/services/password-reset.ts new file mode 100644 index 00000000..271ceabd --- /dev/null +++ b/backend/src/modules/auth/services/password-reset.ts @@ -0,0 +1,110 @@ +// Password reset flow (issue #33). Two endpoints, deliberately paired: +// - request: always behaves the same whether or not the email exists. +// - confirm: single-use, expiring, hashed token → new password + every +// outstanding refresh token invalidated. +// +// Security notes enforced here, not just in the controller: +// - The raw token only ever exists in memory (and in the outbound +// notification). The DB stores its SHA-256 hash. +// - Unknown/expired/reused/superseded tokens all produce the *same* +// generic error, and the hash comparison is constant-time. +// - Rate-limited per email and per IP before any account lookup. +import { createHash, randomBytes, timingSafeEqual } from 'node:crypto'; +import { config } from '../../../config'; +import { HttpError } from '../../../http-error'; +import { sendPasswordResetLink } from '../../notifications/services'; +import { hashPassword } from './password'; +import { + consumePasswordResetToken, + createPasswordResetToken, + findPasswordResetTokenByHash, + findUserByEmail, +} from '../repositories'; +import { createRateLimiter } from './rate-limit'; + +const HOUR_MS = 60 * 60 * 1000; + +/** Deliberately 400, not 404/410 — the client must not be able to tell + * which of "no such token", "expired", "already used" or "superseded by a + * newer request" happened. */ +const GENERIC_INVALID_TOKEN_MESSAGE = + 'This password reset link is invalid or has expired. Request a new one.'; + +const emailLimiter = createRateLimiter(config.passwordReset.requestsPerEmailPerHour, HOUR_MS); +const ipLimiter = createRateLimiter(config.passwordReset.requestsPerIpPerHour, HOUR_MS); + +export function hashResetToken(rawToken: string): string { + return createHash('sha256').update(rawToken).digest('hex'); +} + +/** Constant-time comparison of two hex-encoded hashes. */ +function hashesEqual(a: string, b: string): boolean { + const bufA = Buffer.from(a, 'utf8'); + const bufB = Buffer.from(b, 'utf8'); + if (bufA.length !== bufB.length) return false; + return timingSafeEqual(bufA, bufB); +} + +export interface RequestPasswordResetInput { + email: string; + /** Coarse source address for the per-IP limit. Never persisted. */ + ip: string; +} + +/** + * Issues a reset token if the email belongs to an account. Always resolves + * — the caller responds 202 either way, so response *and* status are + * identical for known and unknown addresses. + */ +export async function requestPasswordReset(input: RequestPasswordResetInput): Promise { + if (!emailLimiter.consume(input.email.toLowerCase()) || !ipLimiter.consume(input.ip)) { + throw new HttpError(429, 'Too many password reset requests. Please try again later.'); + } + + const user = await findUserByEmail(input.email); + if (!user) { + // Do the same shape of work for an unknown address so timing doesn't + // leak existence: hash a throwaway token, then drop it. + hashResetToken(randomBytes(32).toString('base64url')); + return; + } + + const rawToken = randomBytes(32).toString('base64url'); + const expiresAt = new Date(Date.now() + config.passwordReset.tokenTtlMinutes * 60_000); + + await createPasswordResetToken({ userId: user.id, tokenHash: hashResetToken(rawToken), expiresAt }); + await sendPasswordResetLink({ email: user.email, token: rawToken, expiresAt }); +} + +export interface ConfirmPasswordResetInput { + token: string; + newPassword: string; +} + +/** Applies a new password for a valid, unexpired, unused token. */ +export async function confirmPasswordReset(input: ConfirmPasswordResetInput): Promise { + const presentedHash = hashResetToken(input.token); + const record = await findPasswordResetTokenByHash(presentedHash); + + if (!record || !hashesEqual(record.tokenHash, presentedHash)) { + throw new HttpError(400, GENERIC_INVALID_TOKEN_MESSAGE); + } + if (record.usedAt) throw new HttpError(400, GENERIC_INVALID_TOKEN_MESSAGE); + if (record.expiresAt.getTime() <= Date.now()) throw new HttpError(400, GENERIC_INVALID_TOKEN_MESSAGE); + + const passwordHash = await hashPassword(input.newPassword); + const consumed = await consumePasswordResetToken({ + tokenId: record.id, + userId: record.userId, + passwordHash, + }); + // Lost the race to a concurrent confirm of the same token. + if (!consumed) throw new HttpError(400, GENERIC_INVALID_TOKEN_MESSAGE); +} + +/** Test seam only — the limiters are module-level singletons so the limits + * actually hold across requests. */ +export function resetPasswordResetRateLimits(): void { + emailLimiter.reset(); + ipLimiter.reset(); +} diff --git a/backend/src/modules/auth/services/rate-limit.ts b/backend/src/modules/auth/services/rate-limit.ts new file mode 100644 index 00000000..ba257e93 --- /dev/null +++ b/backend/src/modules/auth/services/rate-limit.ts @@ -0,0 +1,48 @@ +// Fixed-window in-memory rate limiter (auth module — used by the password +// reset request endpoint today). +// +// Caveat, stated rather than hidden: this is per-process state, so it does +// not survive a restart and does not hold across multiple backend +// instances. That's adequate for the current single-instance preview +// deploy (docs/adr/0003) and for making the endpoint un-spammable in a way +// that's testable; a horizontally-scaled deployment needs a shared store +// (Redis) — flagged in ARCHITECTURE_ESSENTIALS.md known gaps, not silently +// assumed away. +export interface RateLimiter { + /** Records an attempt for `key`. Returns true if it's allowed, + * false if the caller is over the limit for the current window. */ + consume(key: string, now?: number): boolean; + reset(): void; +} + +const DEFAULT_MAX_TRACKED_KEYS = 10_000; + +export function createRateLimiter(limit: number, windowMs: number): RateLimiter { + const buckets = new Map(); + + return { + consume(key: string, now = Date.now()): boolean { + // Opportunistic prune so a long-running process can't grow the map + // without bound from one-off IPs/emails. + if (buckets.size > DEFAULT_MAX_TRACKED_KEYS) { + for (const [k, bucket] of buckets) { + if (bucket.resetAt <= now) buckets.delete(k); + } + } + + const bucket = buckets.get(key); + if (!bucket || bucket.resetAt <= now) { + buckets.set(key, { count: 1, resetAt: now + windowMs }); + return true; + } + + if (bucket.count >= limit) return false; + bucket.count += 1; + return true; + }, + + reset() { + buckets.clear(); + }, + }; +} diff --git a/backend/src/modules/auth/services/tokens.ts b/backend/src/modules/auth/services/tokens.ts index 923171d2..a85d02d4 100644 --- a/backend/src/modules/auth/services/tokens.ts +++ b/backend/src/modules/auth/services/tokens.ts @@ -1,3 +1,7 @@ +// Token signing/verification. Every token carries `tv` (the user's +// refresh-token version at issue time) so bumping `User.tokenVersion` +// invalidates all outstanding tokens without a per-request DB read — the +// check happens where the token is actually exchanged (refresh). import jwt from 'jsonwebtoken'; import { config } from '../../../config'; import type { Role } from '@prisma/client'; @@ -13,6 +17,8 @@ export interface AccessTokenPayload { * the access token stays stateless so protected requests don't need a * session lookup. */ sid?: string; + /** User.tokenVersion at issue time. Compared at refresh time (Phase 8 #33). */ + tv?: number; } /** diff --git a/backend/src/modules/auth/validation.ts b/backend/src/modules/auth/validation.ts index e9924542..4406603a 100644 --- a/backend/src/modules/auth/validation.ts +++ b/backend/src/modules/auth/validation.ts @@ -36,3 +36,14 @@ export const clearLoginAttemptsSchema = z .refine((value) => Boolean(value.email || value.ip), { message: 'Provide an email or an ip to clear.', }); + +// Phase 8, issue #33 — password reset. The confirm schema repeats signup's +// minimum password length so a reset can't weaken the account. +export const passwordResetRequestSchema = z.object({ + email: z.string().email(), +}); + +export const passwordResetConfirmSchema = z.object({ + token: z.string().min(1), + newPassword: z.string().min(8), +}); diff --git a/backend/src/modules/notifications/repositories/index.ts b/backend/src/modules/notifications/repositories/index.ts index b3405b64..30c17f2a 100644 --- a/backend/src/modules/notifications/repositories/index.ts +++ b/backend/src/modules/notifications/repositories/index.ts @@ -1,3 +1,10 @@ -// notifications repositories — placeholder. Data access only. Not to be imported -// by other modules directly — go through services/ instead. +// notifications repositories — placeholder. Data access only. Not to be +// imported by other modules directly — go through services/ instead. +// +// No persistence yet on purpose: the `Notification` model in schema.prisma +// is still the Phase 0 placeholder (no userId column, ARCHITECTURE.md §4.8), +// and the in-app notification feed is Phase 6 — which is blocked on the +// trigger points in Phases 3/4 (ROADMAP.md). Outbound *delivery* (including +// the password reset link, #33) does not need that model, so it goes +// through services/ without a DB row rather than inventing one here. export {}; diff --git a/backend/src/modules/notifications/services/index.ts b/backend/src/modules/notifications/services/index.ts index cc950672..719d1018 100644 --- a/backend/src/modules/notifications/services/index.ts +++ b/backend/src/modules/notifications/services/index.ts @@ -1,3 +1,68 @@ -// notifications services — placeholder. Other modules must only import from here, -// never from repositories/ directly. See AGENTS.md "Non-negotiable rules". -export {}; +// notifications services — the only exported surface other modules may +// call. Other modules must only import from here, never from +// repositories/ directly. See AGENTS.md "Non-negotiable rules". +// +// Provider-agnostic by design (ARCHITECTURE.md §1.5 / §2, PRD OQ / issue +// #17): outbound delivery goes through a `NotificationChannel` interface, +// and the default implementation is a console adapter. Picking a real +// email/SMS provider is issue #17 and deliberately does NOT block the +// features that need to send a message — only the adapter changes. +import { config } from '../../../config'; + +export interface NotificationMessage { + /** Destination address — an email for now (the only channel interface we need). */ + to: string; + subject: string; + body: string; +} + +export interface NotificationChannel { + send(message: NotificationMessage): Promise; +} + +/** Dev/default adapter. Writes the message to stdout instead of sending + * it anywhere — the only way to read a reset link in local dev until a + * real provider is wired in (#17). Never logs a password, only the + * single-use link (whose token is useless once used/expired). */ +export const consoleNotificationChannel: NotificationChannel = { + async send(message: NotificationMessage) { + // eslint-disable-next-line no-console + console.log(`[notifications] → ${message.to}\n${message.subject}\n${message.body}`); + }, +}; + +let channel: NotificationChannel = consoleNotificationChannel; + +/** Swap the delivery adapter (real provider in prod, a spy in tests). + * Returns the previous channel so callers can restore it. */ +export function setNotificationChannel(next: NotificationChannel): NotificationChannel { + const previous = channel; + channel = next; + return previous; +} + +export async function send(message: NotificationMessage): Promise { + await channel.send(message); +} + +/** Single-use, expiring reset link. The raw token is passed through to + * the channel and never stored — only its hash is (auth module's + * repository). */ +export async function sendPasswordResetLink(input: { + email: string; + token: string; + expiresAt: Date; +}): Promise { + const link = `${config.appBaseUrl}/reset-password?token=${encodeURIComponent(input.token)}`; + const minutes = Math.round((input.expiresAt.getTime() - Date.now()) / 60_000); + await send({ + to: input.email, + subject: 'Reset your DocForum password', + body: [ + 'A password reset was requested for this account.', + `Open this single-use link within ${minutes} minutes to choose a new password:`, + link, + 'If you did not request this, you can ignore this message — your password is unchanged.', + ].join('\n'), + }); +} diff --git a/backend/tests/integration/password-reset.test.mts b/backend/tests/integration/password-reset.test.mts new file mode 100644 index 00000000..1e9b3c98 --- /dev/null +++ b/backend/tests/integration/password-reset.test.mts @@ -0,0 +1,236 @@ +// Password reset (Phase 8 #33) — integration, against a real Postgres, +// because the properties that matter here are DB-level ones: only the hash +// is stored, a token is single-use, a second request supersedes the first, +// and old refresh tokens die after a reset. A mocked repository would not +// prove any of that. +// +// Self-contained like booking.test.mts: throwaway Postgres on port 5449 +// (distinct port per integration file — vitest runs files in parallel). +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import EmbeddedPostgres from 'embedded-postgres'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5449; +const runId = Date.now(); + +// Must be set before anything from src/ is imported: config/index.ts reads +// process.env at module load. A high per-IP limit keeps the per-email limit +// (the one under test) reachable from a single test-runner IP. +process.env.PASSWORD_RESET_REQUESTS_PER_IP_PER_HOUR = '1000'; +process.env.PASSWORD_RESET_REQUESTS_PER_EMAIL_PER_HOUR = '3'; +process.env.PASSWORD_RESET_TOKEN_TTL_MINUTES = '30'; +// vitest.config.mts points DATABASE_URL at booking.test.mts's cluster +// (5433); this file runs its own on 5449, so it has to say so before +// src/db/prisma-client.ts constructs the client at import time. +process.env.DATABASE_URL = 'postgresql://docforum:docforum@localhost:5449/docforum_test'; + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-reset-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); + +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`; + +const GENERIC_INVALID = 'This password reset link is invalid or has expired. Request a new one.'; + +type App = ReturnType; + +describe('password reset', () => { + let app: App; + let prisma: typeof import('../../src/db/prisma-client.js').prisma; + let setNotificationChannel: typeof import('../../src/modules/notifications/services/index.js').setNotificationChannel; + let resetRateLimits: typeof import('../../src/modules/auth/services/index.js').resetPasswordResetRateLimits; + let sent: { to: string; subject: string; body: string }[]; + + const email = `reset-${runId}@example.com`; + const originalPassword = 'original-password-1'; + const newPassword = 'brand-new-password-2'; + + /** The most recent reset link dispatched through the notifications + * interface — this is the "console/dev adapter" seam, replaced by a spy. */ + function lastToken(): string { + const last = sent.at(-1); + if (!last) throw new Error('no notification was sent'); + const match = /token=([^\s]+)/.exec(last.body); + if (!match) throw new Error('no token in notification body'); + return decodeURIComponent(match[1]); + } + + function requestReset(targetEmail: string) { + return request(app).post('/auth/password-reset/request').send({ email: targetEmail }); + } + + function confirmReset(token: string, password: string) { + return request(app).post('/auth/password-reset/confirm').send({ token, newPassword: password }); + } + + beforeAll(async () => { + await pg.initialise(); + await pg.start(); + await pg.createDatabase('docforum_test'); + + execFileSync('npx', ['prisma', 'migrate', 'deploy'], { + cwd: BACKEND_ROOT, + env: { ...process.env, DATABASE_URL: databaseUrl }, + stdio: 'pipe', + }); + + const [appModule, dbModule, notificationsModule, authModule] = await Promise.all([ + import('../../src/app.js'), + import('../../src/db/prisma-client.js'), + import('../../src/modules/notifications/services/index.js'), + import('../../src/modules/auth/services/index.js'), + ]); + app = appModule.createApp(); + prisma = dbModule.prisma; + setNotificationChannel = notificationsModule.setNotificationChannel; + resetRateLimits = authModule.resetPasswordResetRateLimits; + + sent = []; + setNotificationChannel({ + async send(message) { + sent.push(message); + }, + }); + + const signup = await request(app) + .post('/auth/signup') + .send({ role: 'patient', email, password: originalPassword, fullName: 'Reset Tester' }); + expect(signup.status).toBe(200); + }); + + afterAll(async () => { + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + // The rate limiters are module-level singletons (so the limit genuinely + // holds across requests) — reset them so one test's requests can't make + // the next test's first request a 429. The limit itself is exercised in + // its own test below. + beforeEach(() => { + resetRateLimits(); + }); + + it('answers identically for an unknown and a known email (no account-existence leak)', async () => { + const before = sent.length; + const unknown = await requestReset(`nobody-${runId}@example.com`); + const known = await requestReset(email); + + expect(unknown.status).toBe(202); + expect(known.status).toBe(202); + expect(known.body).toEqual(unknown.body); + // No account, no notification — and nothing in the response says so. + expect(sent.length).toBe(before + 1); + expect(sent.at(-1)?.to).toBe(email); + }); + + it('stores only the SHA-256 hash of the token, never the raw token', async () => { + expect((await requestReset(email)).status).toBe(202); + const rawToken = lastToken(); + + const rows = await prisma.passwordResetToken.findMany({ where: { userId: (await prisma.user.findUniqueOrThrow({ where: { email } })).id } }); + expect(rows.length).toBeGreaterThan(0); + + const expectedHash = createHash('sha256').update(rawToken).digest('hex'); + expect(rows.some((row) => row.tokenHash === expectedHash)).toBe(true); + expect(rows.every((row) => row.tokenHash !== rawToken)).toBe(true); + }); + + it('resets the password and rejects reusing the same token', async () => { + expect((await requestReset(email)).status).toBe(202); + const rawToken = lastToken(); + + const first = await confirmReset(rawToken, newPassword); + expect(first.status).toBe(200); + + const reuse = await confirmReset(rawToken, 'yet-another-password-3'); + expect(reuse.status).toBe(400); + expect(reuse.body.message).toBe(GENERIC_INVALID); + + // The new password is live, the old one is not. + expect((await request(app).post('/auth/login').send({ email, password: newPassword })).status).toBe(200); + expect((await request(app).post('/auth/login').send({ email, password: originalPassword })).status).toBe(401); + }); + + it('invalidates the previous token when a second reset is requested', async () => { + expect((await requestReset(email)).status).toBe(202); + const superseded = lastToken(); + expect((await requestReset(email)).status).toBe(202); + const current = lastToken(); + + const stale = await confirmReset(superseded, 'stale-token-password-1'); + expect(stale.status).toBe(400); + expect(stale.body.message).toBe(GENERIC_INVALID); + + expect((await confirmReset(current, newPassword)).status).toBe(200); + }); + + it('rejects an expired token with the same generic error', async () => { + const user = await prisma.user.findUniqueOrThrow({ where: { email } }); + const rawToken = `expired-${runId}`; + await prisma.passwordResetToken.create({ + data: { + userId: user.id, + tokenHash: createHash('sha256').update(rawToken).digest('hex'), + expiresAt: new Date(Date.now() - 60_000), + }, + }); + + const res = await confirmReset(rawToken, 'expired-token-password-1'); + expect(res.status).toBe(400); + expect(res.body.message).toBe(GENERIC_INVALID); + }); + + it('rejects a token that was never issued, with the same generic error', async () => { + const res = await confirmReset('definitely-not-a-real-token', 'whatever-password-1'); + expect(res.status).toBe(400); + expect(res.body.message).toBe(GENERIC_INVALID); + }); + + it('stops accepting refresh tokens issued before the reset', async () => { + const login = await request(app).post('/auth/login').send({ email, password: newPassword }); + expect(login.status).toBe(200); + const cookie = login.headers['set-cookie']?.[0] as unknown as string; + const csrfToken = login.body.csrfToken as string; + expect(cookie).toBeTruthy(); + expect(csrfToken).toBeTruthy(); + + expect( + (await request(app).post('/auth/refresh').set('Cookie', cookie).set('X-CSRF-Token', csrfToken)).status, + ).toBe(200); + + expect((await requestReset(email)).status).toBe(202); + expect((await confirmReset(lastToken(), newPassword)).status).toBe(200); + + // Same cookie, same token — now revoked by the tokenVersion bump. + const afterReset = await request(app) + .post('/auth/refresh') + .set('Cookie', cookie) + .set('X-CSRF-Token', csrfToken); + expect(afterReset.status).toBe(401); + }); + + it('rate-limits requests per email', async () => { + // 3 is the configured per-email limit (PASSWORD_RESET_REQUESTS_PER_EMAIL_PER_HOUR). + const limitedEmail = `limited-${runId}@example.com`; + expect((await requestReset(limitedEmail)).status).toBe(202); + expect((await requestReset(limitedEmail)).status).toBe(202); + expect((await requestReset(limitedEmail)).status).toBe(202); + + const tooMany = await requestReset(limitedEmail); + expect(tooMany.status).toBe(429); + }); +}); diff --git a/backend/tests/unit/password-reset.test.ts b/backend/tests/unit/password-reset.test.ts new file mode 100644 index 00000000..5a5b7747 --- /dev/null +++ b/backend/tests/unit/password-reset.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from 'vitest'; +import { createRateLimiter } from '../../src/modules/auth/services/rate-limit'; +import { hashResetToken } from '../../src/modules/auth/services/password-reset'; +import { passwordResetConfirmSchema, passwordResetRequestSchema } from '../../src/modules/auth/validation'; + +describe('passwordResetRequestSchema', () => { + it('accepts a valid email', () => { + expect(passwordResetRequestSchema.safeParse({ email: 'patient@example.com' }).success).toBe(true); + }); + + it('rejects a malformed email', () => { + expect(passwordResetRequestSchema.safeParse({ email: 'not-an-email' }).success).toBe(false); + }); +}); + +describe('passwordResetConfirmSchema', () => { + it('accepts a token with a long-enough password', () => { + expect(passwordResetConfirmSchema.safeParse({ token: 'abc123', newPassword: 'longenough123' }).success).toBe(true); + }); + + it('rejects a reset that would set a weaker password than signup allows', () => { + expect(passwordResetConfirmSchema.safeParse({ token: 'abc123', newPassword: 'short' }).success).toBe(false); + }); + + it('rejects an empty token', () => { + expect(passwordResetConfirmSchema.safeParse({ token: '', newPassword: 'longenough123' }).success).toBe(false); + }); +}); + +describe('hashResetToken', () => { + it('is deterministic and never returns the raw token', () => { + const raw = 'raw-token-value'; + const hash = hashResetToken(raw); + expect(hash).toBe(hashResetToken(raw)); + expect(hash).not.toContain(raw); + // SHA-256 hex. + expect(hash).toHaveLength(64); + }); + + it('gives different hashes for different tokens', () => { + expect(hashResetToken('token-a')).not.toBe(hashResetToken('token-b')); + }); +}); + +describe('createRateLimiter', () => { + it('allows up to the limit within the window, then blocks', () => { + const limiter = createRateLimiter(3, 60_000); + expect([1, 2, 3].map(() => limiter.consume('email:a'))).toEqual([true, true, true]); + expect(limiter.consume('email:a')).toBe(false); + }); + + it('tracks keys independently', () => { + const limiter = createRateLimiter(1, 60_000); + expect(limiter.consume('email:a')).toBe(true); + expect(limiter.consume('email:b')).toBe(true); + expect(limiter.consume('email:a')).toBe(false); + }); + + it('resets once the window has elapsed', () => { + const limiter = createRateLimiter(1, 1_000); + const start = 1_000_000; + expect(limiter.consume('ip:1', start)).toBe(true); + expect(limiter.consume('ip:1', start + 500)).toBe(false); + expect(limiter.consume('ip:1', start + 1_001)).toBe(true); + }); +}); diff --git a/docs/api/README.md b/docs/api/README.md index 4df31f59..4d592207 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -138,6 +138,14 @@ cookie, 30-day TTL, `SameSite=None; Secure` in production and (`backend/src/config/index.ts`) so the refresh and CSRF cookies can never drift apart. See `docs/adr/0003` for why production needs `SameSite=None`. +Every token carries a `tv` claim (the user's `tokenVersion` at issue time). +Bumping `User.tokenVersion` — which password reset does — invalidates every +outstanding refresh token for that user at the next refresh. This is the +interim kill switch for issue #33; the fuller revocation list (per-session +revocation, #18) is still open, so a bumped-away **access** token keeps +working until its 15-minute TTL elapses. Refresh tokens are checked +against `tokenVersion` on every `POST /auth/refresh`. + ## Doctors (`/doctors`) — `backend/src/modules/doctors` | Method & path | Auth | Notes |