From 4fd85b6fdfbcadaa447e56302fb154ef76e7e392 Mon Sep 17 00:00:00 2001 From: Oyinkans0la12 <147610565+Oyinkans0la12@users.noreply.github.com> Date: Thu, 15 Oct 2026 18:01:10 +0000 Subject: [PATCH 1/4] feat(auth): password reset flow with single-use, hashed, expiring tokens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds POST /auth/password-reset/request (always 202, no account-existence signal, rate-limited per email and per IP) and POST /auth/password-reset/confirm. Only a SHA-256 hash of the token is stored, with an expiry and a used_at marker; a second request supersedes the first in the same transaction, so at most one token is ever live. A successful reset rotates the password and bumps a new User.tokenVersion, which is carried as `tv` in every access/refresh token and checked at refresh — every outstanding refresh token for that user dies immediately. That is the interim revocation mechanism the issue named for the case where #18 has not landed. Link delivery goes through the notifications module's provider-agnostic NotificationChannel (console adapter by default), so no provider decision (#17) was needed to ship this. Docs updated in the same change: ARCHITECTURE.md §4.1/§4.9/§6.1.4, ARCHITECTURE_ESSENTIALS.md, docs/api/README.md, ROADMAP.md. Refs #33 --- ARCHITECTURE.md | 24 +- ARCHITECTURE_ESSENTIALS.md | 3 + ROADMAP.md | 16 +- .../migration.sql | 24 ++ backend/prisma/schema.prisma | 36 ++- backend/src/config/index.ts | 15 ++ backend/src/modules/auth/controllers/index.ts | 29 ++- .../src/modules/auth/repositories/index.ts | 61 ++++- backend/src/modules/auth/routes/index.ts | 5 + backend/src/modules/auth/services/index.ts | 27 ++- .../modules/auth/services/password-reset.ts | 110 +++++++++ .../src/modules/auth/services/rate-limit.ts | 48 ++++ backend/src/modules/auth/services/tokens.ts | 6 + backend/src/modules/auth/validation.ts | 12 + .../notifications/repositories/index.ts | 11 +- .../modules/notifications/services/index.ts | 71 +++++- .../tests/integration/password-reset.test.mts | 229 ++++++++++++++++++ backend/tests/unit/password-reset.test.ts | 66 +++++ docs/api/README.md | 18 +- 19 files changed, 790 insertions(+), 21 deletions(-) create mode 100644 backend/prisma/migrations/20261015175723_add_password_reset_tokens/migration.sql create mode 100644 backend/src/modules/auth/services/password-reset.ts create mode 100644 backend/src/modules/auth/services/rate-limit.ts create mode 100644 backend/tests/integration/password-reset.test.mts create mode 100644 backend/tests/unit/password-reset.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index c98333db..6e2595f1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -64,6 +64,7 @@ Canonical source of truth: `backend/prisma/schema.prisma`. This section is the h ``` User id, email, phone, password_hash, role (enum: patient|doctor|facility|admin) + token_version (int, default 0 — see 4.9) created_at, updated_at PatientProfile (1:1 User) @@ -167,6 +168,27 @@ Notification id, user_id, type, payload (jsonb), read_at (nullable), created_at ``` +### 4.9 Auth hardening (Phase 8) +``` +PasswordResetToken (issue #33) + id, user_id (FK -> User, cascade) + token_hash (unique — SHA-256 hex of the raw token; the raw token is + never stored, logged, or returned by the API) + expires_at, used_at (nullable), created_at +``` +Design notes: `used_at` covers both "already used" and "superseded by a +newer request" — requesting a second reset marks the first token used in the +same transaction that creates the new row, so only one token per user is ever +live. Consuming a token also sets the new `password_hash` and increments +`User.token_version` (see §4.1) in one transaction: every refresh token issued +before the reset carries the old `tv` and is rejected at `POST /auth/refresh`. +`token_version` is the interim kill switch issue #33 asked for while the full +revocation list (§6.1.4, issue #18) doesn't exist — it is a per-user counter, +not a per-session store, so it can't revoke one device and leave others. +On success, delivery goes through `notifications/services` — a +provider-agnostic `NotificationChannel` interface with a console adapter, so +no provider decision (issue #17) is needed for this to work. + ## 5. Key flows (sequence-level, not code) ### 5.1 Booking (concurrency-safe) @@ -193,7 +215,7 @@ Notification 1. Naive slot booking without the DB-level conditional update in §5.1 — the most likely first bug filed against this repo. **Resolved 2026-09-14:** implemented exactly as designed in §5.1 — see `backend/src/modules/availability/repositories/index.ts` (`claimOpenSlot`) and `backend/src/modules/appointments/services/index.ts` (`bookSlot`, the transaction wrapping it). Integration-tested with real concurrency in `backend/tests/integration/booking.test.mts`. 2. `Referral.target_doctor_id` left nullable with no fallback assignment logic — referrals to "a specialty" with no specific doctor need a real routing rule (round robin? load-based?) or they'll pile up unassigned. **Not designed yet — flag as open architecture gap.** 3. Polymorphic `FulfillmentRecord.order_id` (§4.7) — polymorphic FKs are a known footgun for referential integrity in Postgres. Needs either two separate tables (`PrescriptionFulfillment`, `LabOrderFulfillment`) or a check-constraint + application-level guard. **Current schema sketch is a simplification that should be revisited before implementation**, not treated as final. -4. JWT refresh rotation without a revocation list — a stolen refresh token has no clean kill switch until this is added. +4. JWT refresh rotation without a revocation list — a stolen refresh token has no clean kill switch until this is added. **Partially addressed 2026-10-15 (issue #33):** a per-user `token_version` claim is now checked at refresh time, so a password reset kills every outstanding refresh token for that user. Still open: per-session revocation (issue #18) and the fact that an access token stays valid for its remaining TTL (≤15 min) after a bump — there is no per-request revocation check. ### 6.2 What edge cases are we missing (architecture-level)? 1. What happens to `AvailabilitySlot.status = 'held'` if a booking transaction fails/times out mid-flight — is there a TTL to release the hold? (Needed or slots leak into permanent limbo.) diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index 257f27bd..9dae9156 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -26,6 +26,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc `Consultation` → `Prescription`(+`PrescriptionItem`) / `LabOrder`(+`LabResult`) `FulfillmentRecord` (facility-owned) references an order but doesn't mutate it. `FacilityProfile` → `WalletLink` (payout address) ; `PatientProfile`/`FacilityProfile` → `PaymentIntent` (opaque `orderType`/`orderId` — see docs/adr/0004) +`User` → `PasswordResetToken` (SHA-256 hash only, single-use, expiring — Phase 8 #33). `User.token_version` is embedded in every access/refresh token as `tv` and checked at refresh; bumping it (password reset) kills all outstanding refresh tokens for that user. ## Payments (Phase 5.5 — see docs/adr/0004-custodial-payments-v1.md) - **Custodial v1**: `docforum-core` holds its own Stellar payer + releaser identities (`PLATFORM_PAYER_SECRET`/`PLATFORM_RELEASER_SECRET` env vars). Patients never hold or sign with a wallet — the only reason this is possible without violating `docforum-web`'s "no direct Stellar calls" hard rule. @@ -39,6 +40,8 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc - No expiry/TTL job for held slots or pending referrals — currently just an enum value with no worker. - Polymorphic `FulfillmentRecord.order_id` is a sketch, not final — may need splitting into two tables. - No `AuditEvent` table yet, despite PRD requiring auditability — needed before real patient data. +- Only a per-user token version, no per-session revocation store (issue #18). Password reset can kill every session for a user but not one device; and a bumped-away access token still works for the rest of its 15-min TTL. +- The password-reset rate limiter is in-process memory (auth/services/rate-limit.ts) — correct for a single instance, wrong across replicas (needs a shared store). - One doctor = one calendar assumed; multi-facility doctors not modeled. ## Overengineering guardrails (things NOT to build yet) diff --git a/ROADMAP.md b/ROADMAP.md index bf51c734..beaee55f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -107,7 +107,8 @@ for status and issue links. - [ ] ~~Shared: auth screens (signup/login per role).~~ → already built, `docforum-web` Phase W1. ## Phase 8 — Hardening (safety, audit, security) -**Status: Not started. Three of five items scoped into issues #18–#20 (independent of Phases 3–7); the other two remain blocked.** +**Status: In progress. Three of five items scoped into issues #18–#20 (independent of Phases 3–7); three more opened since (#33 password reset, #36 TOTP 2FA, #38 session management) and all three now landed — see notes below. `AuditEvent` and the privacy audit remain blocked on Phases 3/4/5.** +- [x] Password reset flow with single-use, hashed, expiring tokens — `POST /auth/password-reset/request` (always 202, rate-limited per email/per IP) and `POST /auth/password-reset/confirm`. New `PasswordResetToken` table (SHA-256 hash only, `used_at`, `expires_at` — ARCHITECTURE.md §4.9); a second request supersedes the first in the same transaction. Consuming a token rotates the password and bumps `User.token_version`, which kills every outstanding refresh token for that user — the interim revocation mechanism issue #33 specified while #18 is unbuilt. Link delivery goes through a provider-agnostic `NotificationChannel` (console adapter), so it did not depend on #17. Closes [issue #33](https://github.com/DocForum/docforum-core/issues/33). Integration-tested against a real Postgres: unknown-vs-known email response parity, expiry, reuse, supersession, and post-reset refresh rejection. - [ ] `AuditEvent` table + write path for referral/prescription/lab-order state transitions (ARCHITECTURE.md §6.2.4 — flagged as needed before real patient data). Still blocked — needs the Phase 3/4 models (referral, prescription, lab order) to exist first. Not yet scoped into an issue. - [ ] JWT refresh revocation list (ARCHITECTURE.md §6.1.4). Tracked as [issue #18](https://github.com/DocForum/docforum-core/issues/18) (Medium, 150 pts) — unblocked now, operates on Phase 1's auth module. - [ ] Held-slot TTL/release job (ARCHITECTURE.md §6.2.1). Tracked as [issue #19](https://github.com/DocForum/docforum-core/issues/19) (Medium, 150 pts) — unblocked now, operates on Phase 1's `AvailabilitySlot`. @@ -341,3 +342,16 @@ for status and issue links. yet Accepted. Added to the docs site. - 2026-10-09 — Opened issue #32: upgrade to escrow SDK 0.3.0 and the current testnet contract. +- 2026-10-15 — Landed Phase 8's password reset flow (issue #33): + `POST /auth/password-reset/request` (always `202`, rate-limited per + email and per IP, no account-existence signal) and + `POST /auth/password-reset/confirm`. New `PasswordResetToken` table + stores only a SHA-256 hash with `expires_at`/`used_at`; a second + request supersedes the first inside one transaction. Reset bumps a new + `User.token_version`, embedded in every token as `tv` and checked at + refresh — outstanding refresh tokens die immediately, which is the + interim kill switch #33 asked for while #18's revocation list is + unbuilt. Delivery goes through a provider-agnostic + `NotificationChannel` (console adapter), so #17 stayed unblocking. + ARCHITECTURE.md §4.1/§4.9 and ARCHITECTURE_ESSENTIALS.md updated; + the §6.1.4 revocation gap is now marked partially resolved. 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 e0239b77..570f5f79 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -103,13 +103,43 @@ model User { createdAt DateTime @default(now()) updatedAt DateTime @updatedAt - patientProfile PatientProfile? - doctorProfile DoctorProfile? - facilityProfile FacilityProfile? + // 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. This is + // the interim kill switch issue #33 explicitly asked for while the full + // revocation list (#18) doesn't exist yet — see ARCHITECTURE.md §6.1.4. + tokenVersion Int @default(0) + + patientProfile PatientProfile? + doctorProfile DoctorProfile? + facilityProfile FacilityProfile? + passwordResetTokens PasswordResetToken[] @@index([role]) } +// 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 PatientProfile { id String @id @default(uuid()) userId String @unique diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index 3339a26f..9a0bd23f 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -21,6 +21,21 @@ export const config = { refreshTtl: '30d', }, corsOrigin: process.env.CORS_ORIGIN ?? 'http://localhost:5173', + // 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), + }, // Deliberately NOT required at startup via requireEnv() — unrelated // parts of the app (auth, availability, etc.) shouldn't fail to boot // just because payments isn't configured. The payments module itself diff --git a/backend/src/modules/auth/controllers/index.ts b/backend/src/modules/auth/controllers/index.ts index ea1c07b2..e5bad00f 100644 --- a/backend/src/modules/auth/controllers/index.ts +++ b/backend/src/modules/auth/controllers/index.ts @@ -1,6 +1,11 @@ import type { Request, Response } from 'express'; import * as authService from '../services'; -import { loginSchema, signupSchema } from '../validation'; +import { + loginSchema, + passwordResetConfirmSchema, + passwordResetRequestSchema, + signupSchema, +} from '../validation'; import { HttpError } from '../../../http-error'; const REFRESH_COOKIE = 'docforum_refresh'; @@ -62,3 +67,25 @@ export function logout(_req: Request, res: Response) { res.clearCookie(REFRESH_COOKIE, REFRESH_COOKIE_OPTIONS); 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, REFRESH_COOKIE_OPTIONS); + res.status(200).json({ message: 'Password updated. Please sign in with your new password.' }); +} diff --git a/backend/src/modules/auth/repositories/index.ts b/backend/src/modules/auth/repositories/index.ts index c59338dd..098b21f9 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'; @@ -47,3 +47,58 @@ export function createSelfServeUser(input: CreateSelfServeUserInput) { return user; }); } + +/** + * 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 c048acf9..ef1037e5 100644 --- a/backend/src/modules/auth/routes/index.ts +++ b/backend/src/modules/auth/routes/index.ts @@ -7,3 +7,8 @@ authRouter.post('/signup', authController.signup); authRouter.post('/login', authController.login); authRouter.post('/refresh', authController.refresh); authRouter.post('/logout', 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); diff --git a/backend/src/modules/auth/services/index.ts b/backend/src/modules/auth/services/index.ts index eaf3b5e5..519cd3f7 100644 --- a/backend/src/modules/auth/services/index.ts +++ b/backend/src/modules/auth/services/index.ts @@ -13,6 +13,17 @@ import type { Role } from '@prisma/client'; 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 interface AuthenticatedUser { id: string; email: string; @@ -37,8 +48,11 @@ function toPublicUser(user: { return { id: user.id, email: user.email, role: user.role, createdAt: user.createdAt, updatedAt: user.updatedAt }; } -function issueSession(user: { id: string; role: Role }): { accessToken: string; refreshToken: string } { - const payload = { sub: user.id, role: user.role }; +function issueSession(user: { id: string; role: Role; tokenVersion: number }): { + accessToken: string; + refreshToken: string; +} { + const payload = { sub: user.id, role: user.role, tv: user.tokenVersion }; return { accessToken: signAccessToken(payload), refreshToken: signRefreshToken(payload) }; } @@ -104,6 +118,15 @@ export async function refresh(refreshToken: string): Promise { const user = await findUserById(payload.sub); if (!user) throw new HttpError(401, 'Invalid or expired refresh token.'); + // `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.'); + } + const tokens = issueSession(user); return { user: toPublicUser(user), ...tokens }; } 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 ba342c74..c85a0816 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'; @@ -5,6 +9,8 @@ import type { Role } from '@prisma/client'; export interface AccessTokenPayload { sub: string; // user id role: Role; + /** User.tokenVersion at issue time. Compared at refresh time. */ + tv?: number; } export function signAccessToken(payload: AccessTokenPayload): string { diff --git a/backend/src/modules/auth/validation.ts b/backend/src/modules/auth/validation.ts index 1e8a0489..893fcc11 100644 --- a/backend/src/modules/auth/validation.ts +++ b/backend/src/modules/auth/validation.ts @@ -11,3 +11,15 @@ export const loginSchema = z.object({ email: z.string().email(), password: z.string().min(1), }); + +// Password reset (Phase 8 #33). +export const passwordResetRequestSchema = z.object({ + email: z.string().email(), +}); + +export const passwordResetConfirmSchema = z.object({ + token: z.string().min(1), + // Same minimum as signup — a reset must not be able to set a weaker + // password than the account could have been created with. + 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..25e95c4d --- /dev/null +++ b/backend/tests/integration/password-reset.test.mts @@ -0,0 +1,229 @@ +// 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 5436 +// (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 = 5436; +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 5436, 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:5436/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; + expect(cookie).toBeTruthy(); + + expect((await request(app).post('/auth/refresh').set('Cookie', cookie)).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); + 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 aa21154f..edc964bd 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -16,14 +16,22 @@ spec yet — TBD, tracked informally, not blocking Phase 1. | `POST /auth/login` | none | `{ email, password }` | Same response shape as signup. | | `POST /auth/refresh` | refresh cookie | — | Reads the httpOnly cookie (not the body). Returns a fresh `{ user, accessToken }` and rotates the cookie. | | `POST /auth/logout` | none | — | Clears the refresh cookie. `204 No Content`. | +| `POST /auth/password-reset/request` | none | `{ email }` | **Always `202`**, same body for a known and an unknown email — no account-existence signal. Sends a single-use link through the `notifications` module (console adapter today, provider TBD per issue #17). Rate-limited per email (3/h) and per IP (10/h) → `429`. | +| `POST /auth/password-reset/confirm` | none | `{ token, newPassword }` | `200` on success. The token is single-use, expires after 30 min, and is stored only as a SHA-256 hash. `400` with the **same generic message** for unknown, expired, reused, and superseded tokens. Success invalidates every outstanding refresh token for that user (per-user `tokenVersion` bump) and clears the refresh cookie. | -Access tokens: `Authorization: Bearer `, 15-minute TTL, no -revocation list yet (see `ARCHITECTURE_ESSENTIALS.md` known gaps — -JWT refresh rotation without a revocation list). Refresh tokens: httpOnly -cookie, 30-day TTL, `sameSite: 'lax'`/`secure: false` — **local-dev-only -choice**, see the comment above `setRefreshCookie` in +Access tokens: `Authorization: Bearer `, 15-minute TTL. Refresh +tokens: httpOnly cookie, 30-day TTL, `sameSite: 'lax'`/`secure: false` — +**local-dev-only choice**, see the comment above `setRefreshCookie` in `backend/src/modules/auth/controllers/index.ts` for what production needs. +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 | From d9e7528e67235e0429ba3ec6e9a92be31864feb7 Mon Sep 17 00:00:00 2001 From: Oyinkans0la12 <147610565+Oyinkans0la12@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:42:10 +0000 Subject: [PATCH 2/4] fix(auth): restore password reset model and wiring dropped in merge The main merge resolution lost the PasswordResetToken model, config block, repository functions, route/validation wiring and the tokenVersion refresh check, leaving the branch unable to generate the Prisma client. Restore them, and send the CSRF token on refresh in the integration test now that /auth/refresh is CSRF-protected. --- backend/prisma/schema.prisma | 21 +++++++ backend/src/config/index.ts | 15 +++++ backend/src/modules/auth/controllers/index.ts | 4 +- .../src/modules/auth/repositories/index.ts | 55 +++++++++++++++++++ backend/src/modules/auth/routes/index.ts | 5 ++ backend/src/modules/auth/services/index.ts | 25 ++++++++- backend/src/modules/auth/services/tokens.ts | 2 + backend/src/modules/auth/validation.ts | 11 ++++ .../tests/integration/password-reset.test.mts | 11 +++- 9 files changed, 144 insertions(+), 5 deletions(-) diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index a42b7e80..f82f9183 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -162,6 +162,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 fb38515a..3f294276 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, @@ -151,6 +153,6 @@ export async function confirmPasswordReset(req: Request, res: Response) { // 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, REFRESH_COOKIE_OPTIONS); + res.clearCookie(REFRESH_COOKIE, config.cookie); res.status(200).json({ message: 'Password updated. Please sign in with your new password.' }); } diff --git a/backend/src/modules/auth/repositories/index.ts b/backend/src/modules/auth/repositories/index.ts index e4074994..4bbb7f75 100644 --- a/backend/src/modules/auth/repositories/index.ts +++ b/backend/src/modules/auth/repositories/index.ts @@ -184,3 +184,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 12064bc2..9bcaedc1 100644 --- a/backend/src/modules/auth/routes/index.ts +++ b/backend/src/modules/auth/routes/index.ts @@ -27,3 +27,8 @@ authRouter.post('/login-attempts/clear', authenticate, requireRole('admin'), aut // /logout. See middleware/csrf.ts for the choice and its justification. 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); diff --git a/backend/src/modules/auth/services/index.ts b/backend/src/modules/auth/services/index.ts index 455c87b9..fecb1537 100644 --- a/backend/src/modules/auth/services/index.ts +++ b/backend/src/modules/auth/services/index.ts @@ -36,6 +36,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'; + // Re-exported for the same reason: tests (and, at Phase 6, the provider // integration) replace the outbound email sender through this module's // service surface, not by importing the dispatcher file directly. @@ -88,12 +99,12 @@ function toPublicUser(user: { }; } -function issueSession(user: { id: string; role: Role }): { +function issueSession(user: { id: string; role: Role; tokenVersion?: number }): { 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(); @@ -370,6 +381,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/tokens.ts b/backend/src/modules/auth/services/tokens.ts index 9f09e9fb..60a73673 100644 --- a/backend/src/modules/auth/services/tokens.ts +++ b/backend/src/modules/auth/services/tokens.ts @@ -13,6 +13,8 @@ export interface AccessTokenPayload { * compare against `User.sessionsInvalidatedAt` (Phase 8, issue #39) — * tokens minted before a deactivation must stop working immediately. */ iat?: number; + /** 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/tests/integration/password-reset.test.mts b/backend/tests/integration/password-reset.test.mts index 25e95c4d..e43c8cf5 100644 --- a/backend/tests/integration/password-reset.test.mts +++ b/backend/tests/integration/password-reset.test.mts @@ -204,15 +204,22 @@ describe('password reset', () => { 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)).status).toBe(200); + 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); + const afterReset = await request(app) + .post('/auth/refresh') + .set('Cookie', cookie) + .set('X-CSRF-Token', csrfToken); expect(afterReset.status).toBe(401); }); From a5a00b7582637449b821f3d0f7899b2eb5e4ea0b Mon Sep 17 00:00:00 2001 From: Oyinkans0la12 <147610565+Oyinkans0la12@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:43:48 +0000 Subject: [PATCH 3/4] fix(auth): restore password-reset wiring dropped in the sessions merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge with main kept the schema model but lost the User.tokenVersion column, the `tv` claim on tokens, the two password-reset controller handlers and routes, and the password-reset service re-exports — so `prisma generate` produced a client without `tokenVersion`, typecheck failed on unused imports and the missing `tv` field, and `resetPasswordResetRateLimits` was unreachable from the test's import surface. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- backend/prisma/schema.prisma | 7 ++++++ backend/src/modules/auth/controllers/index.ts | 22 +++++++++++++++++++ backend/src/modules/auth/routes/index.ts | 5 +++++ backend/src/modules/auth/services/index.ts | 17 +++++++++++--- backend/src/modules/auth/services/tokens.ts | 2 ++ 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index d5a222c0..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. diff --git a/backend/src/modules/auth/controllers/index.ts b/backend/src/modules/auth/controllers/index.ts index 0aa19c90..71cc68ce 100644 --- a/backend/src/modules/auth/controllers/index.ts +++ b/backend/src/modules/auth/controllers/index.ts @@ -147,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/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 93378a1a..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 }); diff --git a/backend/src/modules/auth/services/tokens.ts b/backend/src/modules/auth/services/tokens.ts index 14110ebc..a85d02d4 100644 --- a/backend/src/modules/auth/services/tokens.ts +++ b/backend/src/modules/auth/services/tokens.ts @@ -17,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; } /** From f6315d5b104c4e3695cacd63587909c08d6f84a4 Mon Sep 17 00:00:00 2001 From: Oyinkans0la12 <147610565+Oyinkans0la12@users.noreply.github.com> Date: Tue, 20 Oct 2026 14:55:17 +0000 Subject: [PATCH 4/4] fix(test): move password-reset integration suite off port 5436 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5436 is login-throttle.test.mts's port. vitest runs integration files in parallel and each boots its own embedded Postgres, so the two files raced for the same port and password-reset's beforeAll hook timed out in CI. 🤖 Generated with Codebuff Co-Authored-By: Codebuff --- backend/tests/integration/password-reset.test.mts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/tests/integration/password-reset.test.mts b/backend/tests/integration/password-reset.test.mts index e43c8cf5..1e9b3c98 100644 --- a/backend/tests/integration/password-reset.test.mts +++ b/backend/tests/integration/password-reset.test.mts @@ -4,7 +4,7 @@ // 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 5436 +// 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'; @@ -17,7 +17,7 @@ import request from 'supertest'; import EmbeddedPostgres from 'embedded-postgres'; const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); -const PORT = 5436; +const PORT = 5449; const runId = Date.now(); // Must be set before anything from src/ is imported: config/index.ts reads @@ -27,9 +27,9 @@ 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 5436, so it has to say so before +// (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:5436/docforum_test'; +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({