diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4fc40f8..1ee76d8 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -91,6 +91,23 @@ immediately by a new request). Sections run one at a time and the bundle is flushed in bounded chunks to `PatientExportChunk` (§4.11), so memory is bounded by one page of rows plus one chunk regardless of history length. +### 3.1 Account deletion and retention holds (Phase 8, issue #41) + +`DELETE /patients/me` is orchestrated by `patients` and anonymises in one +transaction: the `User` row through `auth`'s service (`anonymizeAccount`, +which takes the caller's transaction client) and the `PatientProfile` +through `patients`' own repository. Nothing is cascaded; see +`docs/adr/0010` for the per-model retention table. + +The scheduled purge (`src/jobs/account-purge.ts`) may only hard-delete a +*tombstone*: an anonymised patient that no module still holds records for. +"Holds records" is answered by each owning module through its own service, +registered in `src/retention-holds.ts` (wiring, outside `modules/`, because +`patients` can't import modules that already import it). The RESTRICT +foreign keys are the backstop. **A new module with patient-linked records +must do both:** reference `PatientProfile` with `onDelete: Restrict`, and add +a hold. + ## 4. Data models Canonical source of truth: `backend/prisma/schema.prisma`. This section is the human-readable mirror — if they drift, the schema file wins and this doc should be corrected in the same PR. @@ -111,6 +128,10 @@ User Accept-Language at signup, unsupported tags fall back to `en`) locale (string, default `en` — Phase 9 issue #133; supported language selected from Accept-Language at signup, unsupported tags fall back to `en`) + deleted_at (nullable — Phase 8 issue #41, docs/adr/0010; set once, + irreversibly, when the account is deleted. The row is anonymised in + place: email -> deleted+@deleted.invalid, phone null, password + unusable, email_verified_at null. Login and refresh refuse it.) created_at, updated_at EmailVerificationToken (Phase 8 issue #34) @@ -122,7 +143,11 @@ EmailVerificationToken (Phase 8 issue #34) guarantee, so two concurrent redemptions can't both win. PatientProfile (1:1 User) - id, user_id, full_name, date_of_birth, guardian_user_id (nullable — see PRD edge case: minors) + id, user_id (FK -> User, ON DELETE RESTRICT since issue #41 — was CASCADE) + full_name, date_of_birth, guardian_user_id (nullable — see PRD edge case: minors) + anonymized_at (nullable — issue #41; set with full_name -> 'Deleted patient', + date_of_birth/guardian_user_id -> null. The row stays so Appointment and + PaymentIntent keep their FK; both of those reference it ON DELETE RESTRICT.) DoctorProfile (1:1 User) id, user_id, full_name, verification_status (enum: pending|verified|rejected) @@ -465,6 +490,8 @@ surfaces as `deleted_user`. 4. JWT refresh rotation without a revocation list — a stolen refresh token has no clean kill switch until this is added. 5. **Discovered 2026-10-15 while building email verification (Phase 8, issue #34):** the email/SMS provider is still undecided (Phase 6, issue #17), so verification links can't actually be delivered in a hosted environment yet — `backend/src/modules/auth/services/email-dispatcher.ts` is a no-op seam until a provider is integrated. Until then `backend/scripts/mark-email-verified.mjs` is the documented out-of-band path for an operator with DB access. Not a reason to skip the guard: the guard is what makes the flow safe once delivery exists, and shipping it locked-off would just mean shipping it untested. +6. **Discovered 2026-10-17 (issue #41):** after `DELETE /patients/me`, the caller's access token (stateless JWT, 15-minute TTL) still passes `authenticate` until it expires — there's no revocation list (item 4 above, issue #18). It can't reach anything: every patient route resolves the caller through `getMyPatientProfile`, which refuses an anonymised profile; booking/payments refuse it via `requireVerifiedEmail` (verification is cleared); refresh refuses `deleted_at`. Closing the window itself is #18's job. Also: Postgres reports `ON DELETE RESTRICT` as SQLSTATE 23001, which Prisma surfaces as an *unknown* request error, not `P2003` — `patients/services` `isForeignKeyRefusal` handles both; `middleware/error-handler.ts` currently maps neither to a 4xx. + ### 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.) 2. Referral `expired` status exists in the enum but no expiry job is designed yet — orphaned referrals (PRD 8.1.3 sibling problem). **Partly addressed 2026-10-17 (Phase 3, issue #67):** repeated no-shows now drive a referral to `expired` (`NO_SHOW_EXPIRY_THRESHOLD = 2`, with `referring_doctor_notified_at` set as the seam Phase 6's notification layer will read). What is still missing is **time-based** expiry — a pending referral nobody ever accepts, or one whose target doctor never offers a slot, has no TTL and no worker. Tracked as issue #8; per §6.3.2 it should ship as a simple interval/cron, not a queue. diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index 36475c3..ce21c38 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -8,7 +8,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc - Auth: JWT access + refresh, roles = `patient | doctor | facility | admin`. The refresh cookie is `SameSite=None` cross-site, so `/auth/refresh` + `/auth/logout` require an `X-CSRF-Token` header (a per-session token bound into the refresh JWT) and an allowlisted `Origin` — see `backend/src/middleware/csrf.ts`. CORS is an explicit per-environment allowlist (`CORS_ORIGINS`). - Frontend: React + TypeScript + Vite, React Query + Zustand. - Video/voice, SMS/email, file storage: **third-party, behind an interface** — never build custom. -- Jobs: keep it simple (cron/interval) until a real job exists; don't pre-install a queue. Existing interval jobs: login-throttle prune (`src/server.ts`) and patient-export worker/cleanup (`src/jobs/patient-exports.ts`), both started only from `server.ts`. +- Jobs: keep it simple (cron/interval) until a real job exists; don't pre-install a queue. Existing interval jobs: login-throttle prune (`src/server.ts`), patient-export worker/cleanup (`src/jobs/patient-exports.ts`) and deleted-account tombstone purge (`src/jobs/account-purge.ts`), all started only from `server.ts`. - New module holding patient data? Register a patient-export section (ARCHITECTURE.md §3.1) — right of access must stay complete. ## Hard rules (violating these = request changes on the PR) @@ -30,6 +30,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc `User` → `AccessLog` (Phase 8, issue #45 — PHI read-access log: who viewed which clinical record, when; fire-and-forget writes, fail-open). `LoginAttempt` (Phase 8, issue #35 — Postgres-backed login throttle buckets, one per normalised email and per client IP; counters move only via a conditional upsert). `PatientProfile` → `PatientExport` → `PatientExportChunk` (Phase 8, issue #40 — right-of-access export jobs + their chunked JSON bundle; each module contributes a section via its own service, registered in `src/export-sections.ts` — see ARCHITECTURE.md §3.1). +Account deletion (Phase 8, issue #41 — docs/adr/0010): anonymise in place (`User.deletedAt`, `PatientProfile.anonymizedAt`), never cascade. `PatientProfile.userId` is ON DELETE RESTRICT; new patient-linked clinical tables must be RESTRICT too **and** register a retention hold in `src/retention-holds.ts`. `FacilityProfile` → `WalletLink` (payout address) ; `Prescription` or `LabOrder` → exactly one typed `PaymentIntent` FK (issue #85) ## Payments (Phase 5.5 — see docs/adr/0004-custodial-payments-v1.md) diff --git a/ROADMAP.md b/ROADMAP.md index 3da29b0..4d01573 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -126,6 +126,7 @@ roadmap. See each phase section below for status and issue links. - [ ] Admin account lifecycle: secure bootstrap CLI, admin invites, and account deactivation. Tracked as [issue #39](https://github.com/DocForum/docforum-core/issues/39) (High, 200 pts) — **in progress**. - [ ] Patient data export (right of access) as a complete, machine-readable bundle. `POST /patients/me/export` + `GET /patients/me/export/:id` (+ `/download`), owner-only; an exporter registry where each module contributes its own section through its service (ARCHITECTURE.md §3.1 — `account`, `profile`, `appointments`, `payments` today); versioned JSON with `generatedAt`; generated by an interval worker in `src/jobs/patient-exports.ts` and stored as bounded chunks in Postgres (`PatientExport`/`PatientExportChunk`, §4.11 — no object-storage provider exists yet, see §6.3.4); expiry + cleanup after `PATIENT_EXPORT_TTL_HOURS`. Tracked as [issue #40](https://github.com/DocForum/docforum-core/issues/40) (High, 200 pts) — **in review**, branch `feat/40-patient-data-export`. Later modules (referrals, prescriptions, lab orders) must register their own section when they land; a verified guardian exporting a dependent's data is left to issue #42's model. - [ ] `AuditEvent` table and write path — **foundation done, Phase 3/4 wiring pending.** Append-only `AuditEvent` (DB triggers reject UPDATE/DELETE/TRUNCATE), `auditService.record()` that must run inside the state change's own transaction, request ids on every response, and `GET /admin/audit-events` (entity/actor/date filters, keyset cursor). Wired into appointment booking (both the direct and the referral-origin path), doctor verification, payment fund/release/refund, the patient's referral accept/decline and the referral admin actions (`referral.reassign` / `referral.cancel`, Phase 3 issue #68 — recorded as `entityType: 'Referral'` rows in this one table, not a second referral-specific table). Still unaudited: the referral's *creation* and the cascade-driven `completed`/`expired` transitions (no actor to attribute them to), the remaining Phase 4 order transitions, and the login-throttle admin clear (console-only). Tracked as [issue #43](https://github.com/DocForum/docforum-core/issues/43) (High, 200 pts) — **in review**, branch `feat/43-auditevent-foundation`. +- [ ] Account deletion with clinical-record retention: `DELETE /patients/me` (password re-confirmation, throttled) anonymises the identity in place and keeps appointments + payment intents intact; `PatientProfile.userId` FK CASCADE → RESTRICT (migration); interval purge of record-free tombstones (`src/jobs/account-purge.ts`, holds in `src/retention-holds.ts`). Decisions in `docs/adr/0010` — the **legal retention period for clinical/order/payment records stays an open decision** (kept indefinitely until set). Tracked as [issue #41](https://github.com/DocForum/docforum-core/issues/41) (High, 200 pts) — **in review**, branch `feat/41-account-deletion-retention`. Follow-up when #40 also merges: delete a deleted patient's `PatientExport` rows inside the anonymisation transaction (ADR 0010 table). - [ ] 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`. - [x] Access-control test suite: who may read intake, consultation and payment data. Integration tests (`tests/integration/access-control.test.mts`) establishing the test harness and access-control patterns for the three PHI data categories; intake/consultation test cases are scaffolded as placeholders until Phase 2/3 routes exist, payment-intent access cases exercise the live admin-gated routes. Tracked as [issue #44](https://github.com/DocForum/docforum-core/issues/44) — **Done**. diff --git a/backend/.env.example b/backend/.env.example index 8de44ab..b01b062 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -44,6 +44,11 @@ PATIENT_EXPORT_CLEANUP_INTERVAL_SECONDS=300 PATIENT_EXPORT_STALE_AFTER_SECONDS=900 PATIENT_EXPORT_MAX_ATTEMPTS=3 +# Account deletion purge (Phase 8, issue #41, docs/adr/0010). Only record-free +# tombstones are purged; the grace period is operational, not legal. +ACCOUNT_PURGE_GRACE_HOURS=24 +ACCOUNT_PURGE_INTERVAL_SECONDS=21600 + # Payments (Phase 5.5 — custodial v1, see docs/adr/0004-custodial-payments-v1.md) # Testnet only (docforum-escrow hard rule 4) — real secret keys, never commit # a filled-in .env. Generate + fund your own with: diff --git a/backend/prisma/migrations/20261021120000_account_deletion_retention/migration.sql b/backend/prisma/migrations/20261021120000_account_deletion_retention/migration.sql new file mode 100644 index 0000000..9817bfb --- /dev/null +++ b/backend/prisma/migrations/20261021120000_account_deletion_retention/migration.sql @@ -0,0 +1,12 @@ +-- DropForeignKey +ALTER TABLE "PatientProfile" DROP CONSTRAINT "PatientProfile_userId_fkey"; + +-- AlterTable +ALTER TABLE "PatientProfile" ADD COLUMN "anonymizedAt" TIMESTAMP(3); + +-- AlterTable +ALTER TABLE "User" ADD COLUMN "deletedAt" TIMESTAMP(3); + +-- AddForeignKey +ALTER TABLE "PatientProfile" ADD CONSTRAINT "PatientProfile_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE RESTRICT ON UPDATE CASCADE; + diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index f19852e..ea0e5b3 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -134,6 +134,13 @@ model User { // 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) + // Phase 8 (issue #41, docs/adr/0010): set once, irreversibly, when the + // account is deleted. The row is anonymised in place (email tombstoned, + // phone cleared, password unusable) rather than deleted, because clinical + // and payment records still reference its patient profile. Refresh is + // refused once set. Distinct from admin deactivation (#39), which is + // reversible. + deletedAt DateTime? createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -270,10 +277,19 @@ model PatientProfile { // resolved here. Don't build on top of this field assuming it means // anything enforced yet. guardianUserId String? + // Phase 8 (issue #41, docs/adr/0010): set when the owner deleted their + // account. fullName/dateOfBirth/guardianUserId are anonymised at the same + // moment; the row itself stays so appointments and payments keep their + // foreign key. getMyPatientProfile refuses an anonymised profile. + anonymizedAt DateTime? createdAt DateTime @default(now()) updatedAt DateTime @updatedAt - user User @relation(fields: [userId], references: [id], onDelete: Cascade) + // RESTRICT, not CASCADE (issue #41): a patient's User row is never + // hard-deleted while this profile exists — deletion is anonymisation + // (ADR 0010), and an accidental user delete must fail loudly rather than + // take the clinical identity anchor with it. + user User @relation(fields: [userId], references: [id], onDelete: Restrict) appointments Appointment[] paymentIntents PaymentIntent[] prescriptions Prescription[] @@ -470,7 +486,9 @@ model Appointment { createdAt DateTime @default(now()) updatedAt DateTime @updatedAt - patientProfile PatientProfile @relation(fields: [patientProfileId], references: [id]) + // Explicit RESTRICT (was the Prisma default — same SQL): an appointment is + // a clinical record and must outlive any account deletion (ADR 0010). + patientProfile PatientProfile @relation(fields: [patientProfileId], references: [id], onDelete: Restrict) doctorProfile DoctorProfile @relation(fields: [doctorProfileId], references: [id]) availabilitySlot AvailabilitySlot? @relation(fields: [availabilitySlotId], references: [id]) referral Referral? @relation("ReferralOriginatedAppointments", fields: [referralId], references: [id]) @@ -762,7 +780,9 @@ model PaymentIntent { labOrder LabOrder? @relation(fields: [labOrderId], references: [id]) patientProfileId String - patientProfile PatientProfile @relation(fields: [patientProfileId], references: [id]) + // Explicit RESTRICT (was the Prisma default — same SQL): a payment record + // must outlive any account deletion (ADR 0010). + patientProfile PatientProfile @relation(fields: [patientProfileId], references: [id], onDelete: Restrict) facilityId String facility FacilityProfile @relation(fields: [facilityId], references: [id]) diff --git a/backend/src/app.ts b/backend/src/app.ts index 97926f4..fd9a1a3 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -17,12 +17,15 @@ import { auditRouter, adminAuditRouter, patientAuditRouter } from './modules/aud import { escrowEventsRouter } from './modules/payments/routes/escrow-events'; import { requestId } from './middleware/request-id'; import { referralsRouter } from './modules/referrals/routes'; +import { registerRetentionHolds } from './retention-holds'; export function createApp() { const app = express(); // Phase 8, issue #40: idempotent, so building the app twice is harmless. registerPatientExportSections(); + // Phase 8, issue #41: idempotent, so building the app twice is harmless. + registerRetentionHolds(); // Phase 8, issue #35: login throttling keys on the client IP, so the // proxy chain has to be resolved deliberately rather than guessed. See diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index a44ad84..7731c4f 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -137,6 +137,15 @@ export const config = { staleAfterSeconds: Number(process.env.PATIENT_EXPORT_STALE_AFTER_SECONDS ?? 15 * 60), maxAttempts: Number(process.env.PATIENT_EXPORT_MAX_ATTEMPTS ?? 3), }, + + // Account deletion purge (Phase 8, issue #41, docs/adr/0010). Only + // tombstones with no retained records are ever purged; the grace period is + // operational (lets requests in flight under a still-valid access token + // finish), not a legal retention period — that one is an open decision. + accountPurge: { + graceHours: Number(process.env.ACCOUNT_PURGE_GRACE_HOURS ?? 24), + intervalSeconds: Number(process.env.ACCOUNT_PURGE_INTERVAL_SECONDS ?? 6 * 60 * 60), + }, // Explicit, per-environment CORS allowlist (Phase 8, issue #37). A single // configured origin string was one typo away from "reflect whatever origin // asked"; an allowlist makes every permitted frontend a deliberate entry. diff --git a/backend/src/jobs/README.md b/backend/src/jobs/README.md index f75c7e0..b6eff9c 100644 --- a/backend/src/jobs/README.md +++ b/backend/src/jobs/README.md @@ -17,6 +17,7 @@ safe. | Login-throttle prune (issue #35) | `src/server.ts` | Deletes stale `LoginAttempt` buckets. | | Patient-export worker (issue #40) | `patient-exports.ts` | Generates pending exports one at a time; woken immediately by a new request, plus every `PATIENT_EXPORT_WORKER_INTERVAL_SECONDS`. Requeues claims older than `PATIENT_EXPORT_STALE_AFTER_SECONDS` (up to `PATIENT_EXPORT_MAX_ATTEMPTS`). | | Patient-export cleanup (issue #40) | `patient-exports.ts` | Every `PATIENT_EXPORT_CLEANUP_INTERVAL_SECONDS`: expires `ready` exports past `expiresAt` and deletes their chunks, plus any chunk that can never be served. | +| Deleted-account purge (issue #41, ADR 0010) | `account-purge.ts` | Every `ACCOUNT_PURGE_INTERVAL_SECONDS`: hard-deletes anonymised patients that no module holds records for (`src/retention-holds.ts`), once `ACCOUNT_PURGE_GRACE_HOURS` has passed. Never deletes clinical, order or payment rows. Logs counts and error types only. | Job logs carry error *types* only, never messages — a driver error can echo the patient data it was handling. diff --git a/backend/src/jobs/account-purge.ts b/backend/src/jobs/account-purge.ts new file mode 100644 index 0000000..8847d07 --- /dev/null +++ b/backend/src/jobs/account-purge.ts @@ -0,0 +1,33 @@ +// Deleted-account tombstone purge (Phase 8, issue #41, docs/adr/0010). A +// plain interval, per ARCHITECTURE_ESSENTIALS.md ("keep jobs simple until a +// real job exists — don't pre-install a queue"); move it onto a shared job +// runner if one is ever introduced. Started from src/server.ts only, so test +// suites that build the app never start a timer. +import { config } from '../config'; +import { purgeDeletedPatientAccounts } from '../modules/patients/services'; + +export function startAccountPurgeJob(): () => void { + let running = false; + const run = () => { + if (running) return; + running = true; + void purgeDeletedPatientAccounts() + .then(({ purged, held, failed }) => { + if (purged > 0 || failed > 0) { + // Counts only — never ids or personal fields. + // eslint-disable-next-line no-console + console.log(`[jobs] account purge: ${purged} purged, ${held} retained (records), ${failed} failed`); + } + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.error(`[jobs] account purge failed: ${err instanceof Error ? err.name : typeof err}`); + }) + .finally(() => { + running = false; + }); + }; + const timer = setInterval(run, config.accountPurge.intervalSeconds * 1000); + timer.unref(); + return () => clearInterval(timer); +} diff --git a/backend/src/modules/appointments/repositories/index.ts b/backend/src/modules/appointments/repositories/index.ts index fd282d6..7f7316a 100644 --- a/backend/src/modules/appointments/repositories/index.ts +++ b/backend/src/modules/appointments/repositories/index.ts @@ -82,3 +82,8 @@ export function updateStatusIfScheduled(db: DbClient, id: string, status: Appoin export function clearAvailabilitySlotLink(db: DbClient, id: string) { return db.appointment.updateMany({ where: { id }, data: { availabilitySlotId: null } }); } + +/** Existence check for the account-deletion retention hold (issue #41). */ +export async function patientHasAppointments(patientProfileId: string): Promise { + return (await prisma.appointment.findFirst({ where: { patientProfileId }, select: { id: true } })) !== null; +} diff --git a/backend/src/modules/appointments/services/index.ts b/backend/src/modules/appointments/services/index.ts index ae5c701..b2536a1 100644 --- a/backend/src/modules/appointments/services/index.ts +++ b/backend/src/modules/appointments/services/index.ts @@ -306,3 +306,9 @@ export async function forceCancelAppointment(db: Prisma.TransactionClient, appoi } return appointmentsRepo.findAppointmentById(db, appointmentId); } + +/** Retention hold (issue #41, docs/adr/0010): appointments are clinical + * records, so a patient who has any is never purged after deletion. */ +export function hasAppointmentsForPatient(patientProfileId: string): Promise { + return appointmentsRepo.patientHasAppointments(patientProfileId); +} diff --git a/backend/src/modules/audit/services/index.ts b/backend/src/modules/audit/services/index.ts index b3c7785..0d0eca6 100644 --- a/backend/src/modules/audit/services/index.ts +++ b/backend/src/modules/audit/services/index.ts @@ -298,3 +298,13 @@ export async function pruneOldAccessLogs(): Promise { const cutoff = new Date(Date.now() - ACCESS_LOG_RETENTION_DAYS * 24 * 60 * 60 * 1000); return auditRepo.pruneAccessLogs(cutoff); } + +/** Retention hold (issue #41, docs/adr/0010): AccessLog.patientProfileId + * cascades, so purging a profile would erase the record of who viewed it. + * The purge waits until the access log's own retention prune has removed + * those rows (ACCESS_LOG_RETENTION_DAYS). */ +export function hasAccessLogsForPatient(patientProfileId: string): Promise { + // Reuses the existing read rather than adding a repository export: this + // repository's surface is pinned (tests/integration/audit.test.mts). + return auditRepo.findAccessForPatient({ patientProfileId, take: 1 }).then((rows) => rows.length > 0); +} diff --git a/backend/src/modules/auth/repositories/index.ts b/backend/src/modules/auth/repositories/index.ts index 3305128..d977028 100644 --- a/backend/src/modules/auth/repositories/index.ts +++ b/backend/src/modules/auth/repositories/index.ts @@ -3,14 +3,14 @@ // 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'; +import type { Prisma, Role } from '@prisma/client'; export function findUserByEmail(email: string) { return prisma.user.findUnique({ where: { email } }); } -export function findUserById(id: string) { - return prisma.user.findUnique({ where: { id } }); +export function findUserById(id: string, db: typeof prisma | Prisma.TransactionClient = prisma) { + return db.user.findUnique({ where: { id } }); } // --- Refresh sessions (Phase 8 #38) ------------------------------------- @@ -287,3 +287,45 @@ export function consumePasswordResetToken(input: { return true; }); } + +// --- Account deletion (Phase 8, issue #41, docs/adr/0010) ---------------- +// Called inside the patients module's deletion transaction, so every write +// here takes the caller's transaction client. + +type DbClient = typeof prisma | Prisma.TransactionClient; + +/** Anonymises the User row in place. Conditional on `deletedAt IS NULL`, so + * of two concurrent deletions exactly one does the work; returns whether + * this call did. */ +export async function anonymizeUser( + db: DbClient, + userId: string, + data: { tombstoneEmail: string; unusablePasswordHash: string; deletedAt: Date }, +): Promise { + const result = await db.user.updateMany({ + where: { id: userId, deletedAt: null }, + data: { + email: data.tombstoneEmail, + phone: null, + passwordHash: data.unusablePasswordHash, + emailVerifiedAt: null, + deletedAt: data.deletedAt, + }, + }); + return result.count === 1; +} + +export function deleteVerificationTokensForUser(db: DbClient, userId: string) { + return db.emailVerificationToken.deleteMany({ where: { userId } }).then((result) => result.count); +} + +export function deleteLoginAttemptBucket(db: DbClient, scope: 'email' | 'ip', key: string) { + return db.loginAttempt.deleteMany({ where: { scope, key } }).then((result) => result.count); +} + +/** Hard-deletes an already-anonymised User (tombstone purge). Conditional + * on `deletedAt` being set, so a live account can never be removed here. */ +export async function deleteAnonymizedUser(db: DbClient, userId: string): Promise { + const result = await db.user.deleteMany({ where: { id: userId, deletedAt: { not: null } } }); + return result.count === 1; +} diff --git a/backend/src/modules/auth/services/index.ts b/backend/src/modules/auth/services/index.ts index 22c5dc3..e06d496 100644 --- a/backend/src/modules/auth/services/index.ts +++ b/backend/src/modules/auth/services/index.ts @@ -4,8 +4,12 @@ import { config } from '../../../config'; import { HttpError } from '../../../http-error'; import { + anonymizeUser, consumeVerificationToken, createSelfServeUser, + deleteAnonymizedUser, + deleteLoginAttemptBucket, + deleteVerificationTokensForUser, findLatestVerificationToken, findUserByEmail, findUserById, @@ -28,10 +32,11 @@ import { } from './login-throttle'; import { csrfTokensMatch, generateCsrfToken } from '../../../middleware/csrf'; import { redeemAdminInvite } from '../../users/services'; -import type { Role } from '@prisma/client'; import type { PatientExportSection } from '../../patients/services'; import type { Locale } from '../../../i18n'; import { formatMessage, resolveLocale } from '../../../i18n'; +import { randomBytes } from 'node:crypto'; +import type { Prisma, Role } from '@prisma/client'; // Re-exported so middleware/auth.ts (shared infra, not a peer module) can // verify tokens through this module's service surface rather than @@ -260,7 +265,10 @@ export async function login(input: LoginInput, context: SessionContext = {}): Pr // Same message whether the email doesn't exist or the password is wrong — // and the same *cost*, hence the dummy comparison for an absent account. const valid = await verifyPassword(input.password, user?.passwordHash ?? ABSENT_ACCOUNT_HASH); - if (!user || !valid) { + // `deletedAt` (issue #41): a deleted account's password is already + // unusable and its address is a tombstone, so this is defence in depth — + // and it answers exactly like a wrong password. + if (!user || !valid || user.deletedAt) { const afterFailure = await registerFailedLogin(throttleKeys); if (afterFailure.blocked) throw new LoginThrottledError(afterFailure.retryAfterSeconds); throw new HttpError(401, 'Invalid email or password.'); @@ -406,7 +414,8 @@ export async function refresh(refreshToken: string, csrfToken?: string | null): } const user = await findUserById(payload.sub); - if (!user) throw new HttpError(401, 'Invalid or expired refresh token.'); + // A deleted account (issue #41) can't refresh its way back in. + if (!user || user.deletedAt) throw new HttpError(401, 'Invalid or expired refresh token.'); // Deactivated accounts can't refresh out of their deactivation, and a // session revoked by deactivation/reactivation can't come back to life @@ -503,3 +512,80 @@ export const accountExportSection: PatientExportSection = { ]; }, }; +// --- Account deletion (Phase 8, issue #41, docs/adr/0010) ---------------- +// The patients module orchestrates `DELETE /patients/me`; this module owns +// everything about the login identity it touches. + +/** + * Re-confirms the caller's password for a destructive action. Shares the + * login throttle's per-account bucket: a locked account is refused before + * the hash is compared, and a wrong password counts as a failed login — so + * a stolen access token can't be used to brute-force the password here. + */ +export async function confirmPassword(userId: string, password: string, ip?: string | null): Promise { + const user = await findUserById(userId); + if (!user || user.deletedAt) throw new HttpError(401, 'Authentication required.'); + + const throttleKeys = { emailKey: emailThrottleKey(user.email), ipKey: normalizeClientIp(ip) }; + const gate = await checkLoginThrottle(throttleKeys); + if (gate.blocked) { + const afterFailure = await registerFailedLogin(throttleKeys); + throw new LoginThrottledError(Math.max(gate.retryAfterSeconds, afterFailure.retryAfterSeconds)); + } + + if (!(await verifyPassword(password, user.passwordHash))) { + const afterFailure = await registerFailedLogin(throttleKeys); + if (afterFailure.blocked) throw new LoginThrottledError(afterFailure.retryAfterSeconds); + throw new HttpError(403, 'Password confirmation failed.', 'PASSWORD_CONFIRMATION_FAILED'); + } +} + +export async function isAccountDeleted(userId: string): Promise { + const user = await findUserById(userId); + return Boolean(user?.deletedAt); +} + +/** The address a deleted account is left with: unique (derived from the id), + * stable across retries, and undeliverable (`.invalid` is reserved, RFC + * 2606). It frees the original address for a new signup. */ +export function tombstoneEmail(userId: string): string { + return `deleted+${userId}@deleted.invalid`; +} + +/** A real bcrypt hash of 32 random bytes nobody ever sees — so the deleted + * account's login can never succeed, and the login path still pays a + * normal bcrypt comparison. Computed outside the deletion transaction + * (bcrypt is deliberately slow). */ +export function createUnusablePasswordHash(): Promise { + return hashPassword(randomBytes(32).toString('base64url')); +} + +/** + * Anonymises the login identity, inside the caller's transaction: email to + * its tombstone, phone cleared, password unusable, verification cleared, + * `deletedAt` set; verification tokens and the old address's login-throttle + * bucket deleted. Returns false (and changes nothing) if the account was + * already deleted — repeat and concurrent deletions are no-ops. + */ +export async function anonymizeAccount( + tx: Prisma.TransactionClient, + userId: string, + input: { unusablePasswordHash: string; deletedAt: Date }, +): Promise { + const user = await findUserById(userId, tx); + if (!user || user.deletedAt) return false; + const anonymized = await anonymizeUser(tx, userId, { + tombstoneEmail: tombstoneEmail(userId), + unusablePasswordHash: input.unusablePasswordHash, + deletedAt: input.deletedAt, + }); + if (!anonymized) return false; + await deleteVerificationTokensForUser(tx, userId); + await deleteLoginAttemptBucket(tx, 'email', emailThrottleKey(user.email)); + return true; +} + +/** Tombstone purge: removes an already-anonymised User row. */ +export function purgeAnonymizedAccount(tx: Prisma.TransactionClient, userId: string): Promise { + return deleteAnonymizedUser(tx, userId); +} diff --git a/backend/src/modules/orders/repositories/index.ts b/backend/src/modules/orders/repositories/index.ts index 1422a51..205b9b2 100644 --- a/backend/src/modules/orders/repositories/index.ts +++ b/backend/src/modules/orders/repositories/index.ts @@ -42,4 +42,12 @@ export function findLabOrderExportPage(patientProfileId: string, cursor: string where: { patientProfileId }, orderBy: [{ id: 'asc' }], take, ...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}), include: { result: true }, }); -} \ No newline at end of file +} + +export async function patientHasOrders(patientProfileId: string): Promise { + const [prescription, labOrder] = await Promise.all([ + prisma.prescription.findFirst({ where: { patientProfileId }, select: { id: true } }), + prisma.labOrder.findFirst({ where: { patientProfileId }, select: { id: true } }), + ]); + return prescription !== null || labOrder !== null; +} diff --git a/backend/src/modules/orders/services/index.ts b/backend/src/modules/orders/services/index.ts index bb27b96..de8e1ce 100644 --- a/backend/src/modules/orders/services/index.ts +++ b/backend/src/modules/orders/services/index.ts @@ -155,4 +155,11 @@ export const labOrdersExportSection: PatientExportSection = { })), ); }, -}; \ No newline at end of file +}; + +/** Retention hold (issue #41, docs/adr/0010): prescriptions and lab orders + * are append-only clinical orders, so a patient who has any is never purged + * after deletion. */ +export function hasOrdersForPatient(patientProfileId: string): Promise { + return ordersRepo.patientHasOrders(patientProfileId); +} diff --git a/backend/src/modules/patients/controllers/index.ts b/backend/src/modules/patients/controllers/index.ts index e529de7..76f6610 100644 --- a/backend/src/modules/patients/controllers/index.ts +++ b/backend/src/modules/patients/controllers/index.ts @@ -3,6 +3,8 @@ import { once } from 'node:events'; import * as patientsService from '../services'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; +import { deleteAccountSchema } from '../validation'; +import { LoginThrottledError } from '../../auth/services'; // Phase 8, issue #40 — right of access. The subject is always `req.user`; // no route here accepts a patient id. @@ -49,3 +51,18 @@ export async function downloadExport(req: Request, res: Response) { res.destroy(err instanceof Error ? err : new Error('EXPORT_STREAM_FAILED')); } } + +/** Phase 8, issue #41 — account deletion (docs/adr/0010). 204 whether this + * call anonymised the account or it was already deleted (idempotent). */ +export async function deleteMe(req: Request, res: Response) { + if (!req.user) throw new HttpError(401, 'Authentication required.'); + const { password } = deleteAccountSchema.parse(req.body ?? {}); + try { + await patientsService.deleteMyAccount(req.user.id, password, req.ip); + } catch (err) { + // Password re-confirmation shares the login throttle (issue #35). + if (err instanceof LoginThrottledError) res.set('Retry-After', String(err.retryAfterSeconds)); + throw err; + } + res.status(204).send(); +} diff --git a/backend/src/modules/patients/repositories/index.ts b/backend/src/modules/patients/repositories/index.ts index f1afefe..e6b01a4 100644 --- a/backend/src/modules/patients/repositories/index.ts +++ b/backend/src/modules/patients/repositories/index.ts @@ -1,5 +1,7 @@ import { prisma } from '../../../db/prisma-client'; -import type { PatientExportStatus } from '@prisma/client'; +import type { PatientExportStatus, Prisma } from '@prisma/client'; + +type DbClient = typeof prisma | Prisma.TransactionClient; export function findPatientProfileByUserId(userId: string) { return prisma.patientProfile.findUnique({ where: { userId } }); @@ -99,6 +101,31 @@ export async function markExportReady( return result.count === 1; } +// --- Account deletion (Phase 8, issue #41, docs/adr/0010) ---------------- + +/** What a deleted patient's profile shows from now on (e.g. in a doctor's + * appointment list). */ +export const ANONYMIZED_PATIENT_NAME = 'Deleted patient'; + +/** Anonymises the profile in place, conditional on it not already being + * anonymised. The row (and its id) stays: appointments and payments keep + * their foreign key to it. */ +export async function anonymizeProfile(db: DbClient, profileId: string, anonymizedAt: Date): Promise { + const result = await db.patientProfile.updateMany({ + where: { id: profileId, anonymizedAt: null }, + data: { fullName: ANONYMIZED_PATIENT_NAME, dateOfBirth: null, guardianUserId: null, anonymizedAt }, + }); + return result.count === 1; +} + +/** Account deletion (issue #41, docs/adr/0010): exports are derived copies + * of the patient's data, not records of care, so they go at anonymisation. + * Chunks cascade. An export still being generated loses its row here, so its + * final conditional `ready` update matches nothing and it never publishes. */ +export function deleteExportsForPatient(db: DbClient, patientProfileId: string) { + return db.patientExport.deleteMany({ where: { patientProfileId } }).then((result) => result.count); +} + /** processing -> failed, only for the attempt that is still current. */ export async function markExportFailed( exportId: string, @@ -166,3 +193,23 @@ export function deleteUnservableExportChunks(): Promise { export function findPatientProfileById(id: string) { return prisma.patientProfile.findUnique({ where: { id } }); } +/** One keyset page of anonymised profiles old enough to be purge + * candidates. */ +export function findAnonymizedProfilesPage(anonymizedBefore: Date, cursor: string | undefined, take: number) { + return prisma.patientProfile.findMany({ + where: { anonymizedAt: { lte: anonymizedBefore } }, + select: { id: true, userId: true }, + orderBy: { id: 'asc' }, + take, + ...(cursor ? { cursor: { id: cursor }, skip: 1 } : {}), + }); +} + +/** Hard-deletes an anonymised profile. Conditional on `anonymizedAt` being + * set, so a live profile can never be removed here; and every clinical or + * payment table references the profile with ON DELETE RESTRICT, so a row + * that still has records makes this throw instead of cascading. */ +export async function deleteAnonymizedProfile(db: DbClient, profileId: string): Promise { + const result = await db.patientProfile.deleteMany({ where: { id: profileId, anonymizedAt: { not: null } } }); + return result.count === 1; +} diff --git a/backend/src/modules/patients/routes/index.ts b/backend/src/modules/patients/routes/index.ts index b4eaa65..7d350c4 100644 --- a/backend/src/modules/patients/routes/index.ts +++ b/backend/src/modules/patients/routes/index.ts @@ -15,3 +15,8 @@ patientsRouter.get( requireRole('patient'), patientsController.downloadExport, ); +// Account deletion (Phase 8, issue #41, docs/adr/0010). Self-only — the +// account is the authenticated caller, never a path or body parameter. +// Deliberately not behind requireVerifiedEmail: an unverified account must +// still be deletable. +patientsRouter.delete('/me', authenticate, requireRole('patient'), patientsController.deleteMe); diff --git a/backend/src/modules/patients/services/index.ts b/backend/src/modules/patients/services/index.ts index b700797..21758a9 100644 --- a/backend/src/modules/patients/services/index.ts +++ b/backend/src/modules/patients/services/index.ts @@ -1,9 +1,12 @@ -import { HttpError } from '../../../http-error'; +import { Prisma } from '@prisma/client'; +import { prisma } from '../../../db/prisma-client'; import { config } from '../../../config'; +import { HttpError } from '../../../http-error'; import * as patientsRepo from '../repositories'; import { ChunkWriter, EXPORT_SCHEMA_VERSION, ExportSectionError, writeExportBundle } from './export-bundle'; import { listExportSections, type PatientExportSection } from './export-registry'; import type { PatientExport, PatientExportStatus } from '@prisma/client'; +import * as authService from '../../auth/services'; // The registry is part of this module's service surface: other modules // import `PatientExportSection`/`paginate` from here to build their section, @@ -19,7 +22,11 @@ export { EXPORT_FORMAT, EXPORT_SCHEMA_VERSION } from './export-bundle'; export async function getMyPatientProfile(userId: string) { const profile = await patientsRepo.findPatientProfileByUserId(userId); - if (!profile) throw new HttpError(404, 'No patient profile for this account.'); + // An anonymised profile (issue #41) belongs to a deleted account: every + // patient route resolves the caller through here, so a still-valid access + // token stops reaching booking, listing, etc. the moment the account is + // deleted. + if (!profile || profile.anonymizedAt) throw new HttpError(404, 'No patient profile for this account.'); return profile; } @@ -285,3 +292,136 @@ export async function getPatientProfileById(id: string) { if (!profile) throw new HttpError(404, 'Patient profile not found.'); return profile; } + +// --- Account deletion (Phase 8, issue #41, docs/adr/0010) ---------------- + +/** + * `DELETE /patients/me`. Anonymises the caller's identity (auth module: + * User row; this module: PatientProfile) in one transaction and keeps every + * clinical, order and payment record, per ADR 0010's retention table. + * + * Idempotent: an already-deleted account returns `alreadyDeleted: true` + * without checking the password (it no longer has a usable one) and without + * touching anything; two concurrent requests anonymise exactly once. + */ +export async function deleteMyAccount( + userId: string, + password: string, + ip?: string | null, +): Promise<{ alreadyDeleted: boolean }> { + if (await authService.isAccountDeleted(userId)) return { alreadyDeleted: true }; + const profile = await getMyPatientProfile(userId); + + try { + await authService.confirmPassword(userId, password, ip); + } catch (err) { + // A concurrent deletion may have replaced the password between our two + // checks; that request already did the work, so this one is a no-op. + if (await authService.isAccountDeleted(userId)) return { alreadyDeleted: true }; + throw err; + } + + const unusablePasswordHash = await authService.createUnusablePasswordHash(); + const deletedAt = new Date(); + const anonymized = await prisma.$transaction(async (tx) => { + if (!(await authService.anonymizeAccount(tx, userId, { unusablePasswordHash, deletedAt }))) return false; + await patientsRepo.anonymizeProfile(tx, profile.id, deletedAt); + await patientsRepo.deleteExportsForPatient(tx, profile.id); + return true; + }); + return { alreadyDeleted: !anonymized }; +} + +/** A module that keeps records about a patient which must survive account + * deletion (ADR 0010). The purge never removes a profile any hold reports + * true for. Registered in src/retention-holds.ts. */ +export interface RetentionHold { + name: string; + holds(patientProfileId: string): Promise; +} + +const retentionHolds = new Map(); + +export function registerRetentionHold(hold: RetentionHold): void { + const existing = retentionHolds.get(hold.name); + if (existing && existing !== hold) throw new Error(`Retention hold "${hold.name}" is already registered.`); + retentionHolds.set(hold.name, hold); +} + +export function listRetentionHolds(): RetentionHold[] { + return [...retentionHolds.values()]; +} + +/** + * True when a delete was refused by a foreign key that still references the + * row. Postgres reports `ON DELETE RESTRICT` as SQLSTATE 23001 + * (restrict_violation), which Prisma surfaces as an *unknown* request error + * rather than P2003 (P2003 is 23503, what NO ACTION raises) — so both shapes + * are checked. + */ +export function isForeignKeyRefusal(err: unknown): boolean { + if (err instanceof Prisma.PrismaClientKnownRequestError) return err.code === 'P2003'; + if (err instanceof Prisma.PrismaClientUnknownRequestError) { + return /23001|violates RESTRICT setting of foreign key constraint/.test(err.message); + } + return false; +} + +export interface PurgeResult { + purged: number; + held: number; + failed: number; +} + +/** + * Scheduled purge (ADR 0010): hard-deletes tombstones only — anonymised + * patients with no retained records, once the grace period has passed. + * Clinical, order and payment records are never deleted (their retention + * period is an open decision). Race-safe: holds are checked first and the + * RESTRICT foreign keys backstop the delete, so a record created in between + * makes that one delete fail and the account is simply kept. Re-running is a + * no-op for everything already purged. + */ +export async function purgeDeletedPatientAccounts(now = new Date()): Promise { + const anonymizedBefore = new Date(now.getTime() - config.accountPurge.graceHours * 60 * 60 * 1000); + const holds = listRetentionHolds(); + if (holds.length === 0) { + // Fail closed: with no holds registered (a wiring bug) everything would + // look purgeable. The RESTRICT FKs would still stop it, but don't try. + throw new Error('No retention holds registered — refusing to purge.'); + } + + const result: PurgeResult = { purged: 0, held: 0, failed: 0 }; + let cursor: string | undefined; + for (;;) { + const page = await patientsRepo.findAnonymizedProfilesPage(anonymizedBefore, cursor, 100); + if (page.length === 0) break; + cursor = page[page.length - 1]!.id; + + for (const profile of page) { + const held = await Promise.all(holds.map((hold) => hold.holds(profile.id))); + if (held.some(Boolean)) { + result.held += 1; + continue; + } + try { + await prisma.$transaction(async (tx) => { + if (!(await patientsRepo.deleteAnonymizedProfile(tx, profile.id))) return; + await authService.purgeAnonymizedAccount(tx, profile.userId); + }); + result.purged += 1; + } catch (err) { + if (isForeignKeyRefusal(err)) { + // A RESTRICT FK refused: a record exists that no hold reported yet. + result.held += 1; + } else { + result.failed += 1; + // eslint-disable-next-line no-console + console.error(`[jobs] account purge failed for one profile: ${err instanceof Error ? err.name : typeof err}`); + } + } + } + if (page.length < 100) break; + } + return result; +} diff --git a/backend/src/modules/patients/validation.ts b/backend/src/modules/patients/validation.ts new file mode 100644 index 0000000..c0d77f0 --- /dev/null +++ b/backend/src/modules/patients/validation.ts @@ -0,0 +1,6 @@ +import { z } from 'zod'; + +// Phase 8, issue #41 — `DELETE /patients/me` re-confirms the password. +export const deleteAccountSchema = z.object({ + password: z.string().min(1, 'password is required to delete your account.'), +}); diff --git a/backend/src/modules/payments/repositories/index.ts b/backend/src/modules/payments/repositories/index.ts index 118e2cf..e543db9 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -121,3 +121,8 @@ export async function markStatus( const result = await tx.paymentIntent.updateMany({ where: { id, status: from }, data: { status: to } }); return result.count === 1 ? tx.paymentIntent.findUniqueOrThrow({ where: { id } }) : null; } + +/** Existence check for the account-deletion retention hold (issue #41). */ +export async function patientHasPaymentIntents(patientProfileId: string): Promise { + return (await prisma.paymentIntent.findFirst({ where: { patientProfileId }, select: { id: true } })) !== null; +} diff --git a/backend/src/modules/payments/services/index.ts b/backend/src/modules/payments/services/index.ts index 047039c..2bd29c0 100644 --- a/backend/src/modules/payments/services/index.ts +++ b/backend/src/modules/payments/services/index.ts @@ -355,3 +355,8 @@ export const paymentsExportSection: PatientExportSection = { ); }, }; +/** Retention hold (issue #41, docs/adr/0010): payment records are kept, so + * a patient who has any is never purged after deletion. */ +export function hasPaymentIntentsForPatient(patientProfileId: string): Promise { + return paymentsRepo.patientHasPaymentIntents(patientProfileId); +} diff --git a/backend/src/modules/referrals/repositories/index.ts b/backend/src/modules/referrals/repositories/index.ts index 2b487d4..819754f 100644 --- a/backend/src/modules/referrals/repositories/index.ts +++ b/backend/src/modules/referrals/repositories/index.ts @@ -203,3 +203,7 @@ export function listAdminQueue(filters: { orderBy: { createdAt: 'asc' }, }); } + +export async function patientHasReferrals(patientProfileId: string): Promise { + return (await prisma.referral.findFirst({ where: { patientProfileId }, select: { id: true } })) !== null; +} diff --git a/backend/src/modules/referrals/services/index.ts b/backend/src/modules/referrals/services/index.ts index 00413e3..b449f34 100644 --- a/backend/src/modules/referrals/services/index.ts +++ b/backend/src/modules/referrals/services/index.ts @@ -656,3 +656,10 @@ export async function listAuditEvents(referralId: string) { }); return page.items; } + +/** Retention hold (issue #41, docs/adr/0010): a referral carries an + * immutable clinical snapshot, so a patient who has any is never purged + * after deletion. */ +export function hasReferralsForPatient(patientProfileId: string): Promise { + return referralsRepo.patientHasReferrals(patientProfileId); +} diff --git a/backend/src/retention-holds.ts b/backend/src/retention-holds.ts new file mode 100644 index 0000000..69c797c --- /dev/null +++ b/backend/src/retention-holds.ts @@ -0,0 +1,29 @@ +// Composition root for account-deletion retention holds (Phase 8, issue +// #41, docs/adr/0010). Each module that keeps records about a patient which +// must survive account deletion answers "does this patient have any?" +// through its own service; the patients module's purge never removes a +// profile any hold says yes for. Lives outside modules/ because it's +// wiring: the patients module can't import modules that already import it. +// +// New module with patient-linked records (intake, consultations, referrals, +// prescriptions, lab orders)? Reference PatientProfile with +// `onDelete: Restrict` AND add a hold here. The FK is the backstop; the hold +// is what keeps the purge from even trying. +import { hasAppointmentsForPatient } from './modules/appointments/services'; +import { hasAccessLogsForPatient } from './modules/audit/services'; +import { hasOrdersForPatient } from './modules/orders/services'; +import { hasPaymentIntentsForPatient } from './modules/payments/services'; +import { hasReferralsForPatient } from './modules/referrals/services'; +import { registerRetentionHold, type RetentionHold } from './modules/patients/services'; + +export const RETENTION_HOLDS: RetentionHold[] = [ + { name: 'appointments', holds: hasAppointmentsForPatient }, + { name: 'payments', holds: hasPaymentIntentsForPatient }, + { name: 'orders', holds: hasOrdersForPatient }, + { name: 'referrals', holds: hasReferralsForPatient }, + { name: 'access_logs', holds: hasAccessLogsForPatient }, +]; + +export function registerRetentionHolds(): void { + for (const hold of RETENTION_HOLDS) registerRetentionHold(hold); +} diff --git a/backend/src/server.ts b/backend/src/server.ts index fa1c957..7475c67 100644 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -6,7 +6,7 @@ import { startPatientExportJobs } from './jobs/patient-exports'; import { startEscrowEventPoller } from './jobs/escrow-events'; import { validatePlatformSigners } from './modules/payments/services/platform-signers'; import { verifyPaymentsNetwork } from './modules/payments/services'; - +import { startAccountPurgeJob } from './jobs/account-purge'; const app = createApp(); @@ -63,3 +63,5 @@ pruneTimer.unref(); // Patient data export worker + expiry cleanup (Phase 8, issue #40). startPatientExportJobs(); startEscrowEventPoller(); +// Deleted-account tombstone purge (Phase 8, issue #41, docs/adr/0010). +startAccountPurgeJob(); diff --git a/backend/tests/integration/account-deletion.test.mts b/backend/tests/integration/account-deletion.test.mts new file mode 100644 index 0000000..6512d86 --- /dev/null +++ b/backend/tests/integration/account-deletion.test.mts @@ -0,0 +1,446 @@ +// Integration tests for account deletion with clinical-record retention +// (Phase 8, issue #41, docs/adr/0010), against a populated database: +// password re-confirmation, PII anonymised, login/refresh deactivated, the +// original address reusable, appointments + payments surviving byte-for-byte, +// the RESTRICT foreign keys, idempotency (repeat and concurrent), and the +// tombstone purge touching eligible rows only. Self-contained, same +// embedded-Postgres pattern as booking.test.mts. Own port (5453). +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it, onTestFinished } from 'vitest'; +import request from 'supertest'; +import bcrypt from 'bcryptjs'; +import EmbeddedPostgres from 'embedded-postgres'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5453; +const runId = Date.now(); +const PASSWORD = 'longenough123'; + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-deletion-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`; + +type App = ReturnType; +type Prisma = typeof import('../../src/db/prisma-client.js').prisma; +type PatientsService = typeof import('../../src/modules/patients/services/index.js'); + +interface Patient { + email: string; + userId: string; + profileId: string; + token: string; + csrfToken: string; + refreshCookie: string; +} + +const emailedTokens = new Map(); + +describe('account deletion with clinical-record retention', () => { + let app: App; + let prisma: Prisma; + let patients: PatientsService; + let doctorToken: string; + let doctorProfileId: string; + let facilityId: string; + let alice: Patient; // has an appointment + payments + let bob: Patient; // unrelated, must be untouched + let carol: Patient; // no records, unverified + let dave: Patient; // no records, deleted recently + let erin: Patient; // used for the throttle test + + async function signup(role: 'patient' | 'doctor', label: string, verify = true) { + const email = `${label}-${runId}@example.com`; + const res = await request(app).post('/auth/signup').send({ role, email, password: PASSWORD, fullName: `${label} Fullname` }); + expect(res.status).toBe(200); + if (verify) { + const verified = await request(app).post('/auth/verify-email').send({ token: emailedTokens.get(email) }); + expect(verified.status).toBe(200); + } + const cookie = (res.headers['set-cookie'] as unknown as string[]).find((c) => c.startsWith('docforum_refresh='))!; + return { + email, + userId: res.body.user.id as string, + token: res.body.accessToken as string, + csrfToken: res.body.csrfToken as string, + refreshCookie: cookie.split(';')[0]!, + }; + } + + async function signupPatient(label: string, verify = true): Promise { + const user = await signup('patient', label, verify); + const profile = await prisma.patientProfile.findUniqueOrThrow({ where: { userId: user.userId } }); + return { ...user, profileId: profile.id }; + } + + function deleteAccount(patient: Patient, body: object | undefined) { + const req = request(app).delete('/patients/me').set('Authorization', `Bearer ${patient.token}`); + return body === undefined ? req : req.send(body); + } + + async function snapshotRecords(profileId: string) { + return { + appointments: await prisma.appointment.findMany({ where: { patientProfileId: profileId }, orderBy: { id: 'asc' } }), + payments: await prisma.paymentIntent.findMany({ where: { patientProfileId: profileId }, orderBy: { id: 'asc' } }), + }; + } + + async function snapshotAccount(p: Patient) { + return { + user: await prisma.user.findUnique({ where: { id: p.userId } }), + profile: await prisma.patientProfile.findUnique({ where: { id: p.profileId } }), + ...(await snapshotRecords(p.profileId)), + }; + } + + 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', + }); + + process.env.DATABASE_URL = databaseUrl; + process.env.ACCOUNT_PURGE_GRACE_HOURS = '24'; + + const appModule = await import('../../src/app.js'); + const dbModule = await import('../../src/db/prisma-client.js'); + const authServices = await import('../../src/modules/auth/services/index.js'); + patients = await import('../../src/modules/patients/services/index.js'); + app = appModule.createApp(); + prisma = dbModule.prisma; + + authServices.setEmailSender(async (message) => { + const match = /[?&]token=([^&\s]+)/.exec(message.text); + if (match) emailedTokens.set(message.to, decodeURIComponent(match[1])); + }); + + const doctor = await signup('doctor', 'deletion-doctor'); + doctorToken = doctor.token; + doctorProfileId = (await prisma.doctorProfile.findUniqueOrThrow({ where: { userId: doctor.userId } })).id; + + alice = await signupPatient('alice'); + bob = await signupPatient('bob'); + carol = await signupPatient('carol', false); + dave = await signupPatient('dave'); + erin = await signupPatient('erin'); + + // Populate the fields that must be anonymised, beyond what signup sets. + for (const p of [alice, bob]) { + await prisma.user.update({ where: { id: p.userId }, data: { phone: '+2348000000000' } }); + await prisma.patientProfile.update({ + where: { id: p.profileId }, + data: { dateOfBirth: new Date('1990-05-01'), guardianUserId: doctor.userId }, + }); + } + + // Real bookings through the API for Alice and Bob. + const tomorrow = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString().slice(0, 10); + await request(app) + .post(`/availability/doctors/${doctorProfileId}/slots/generate`) + .set('Authorization', `Bearer ${doctorToken}`) + .send({ date: tomorrow, startHour: 9, endHour: 11, slotLengthMinutes: 30 }); + const slots = (await request(app).get(`/availability/doctors/${doctorProfileId}/slots`)).body as { id: string }[]; + for (const [i, p] of [alice, bob].entries()) { + const booked = await request(app) + .post('/appointments') + .set('Authorization', `Bearer ${p.token}`) + .send({ availabilitySlotId: slots[i]!.id }); + expect(booked.status).toBe(201); + } + + const facilityUser = await prisma.user.create({ data: { email: `lab-${runId}@example.com`, passwordHash: 'x', role: 'facility' } }); + facilityId = (await prisma.facilityProfile.create({ data: { userId: facilityUser.id, name: 'Lab', type: 'lab' } })).id; + for (const p of [alice, bob]) { + // Main replaced PaymentIntent's polymorphic `orderType`/`orderId` with + // real `Prescription`/`LabOrder` FKs under a "exactly one order" CHECK + // (issue #85), so build the chain consultation -> lab order -> intent + // on the appointment this test already booked (no extra appointments: + // the counts below are part of what is asserted). + const appointment = await prisma.appointment.findFirstOrThrow({ where: { patientProfileId: p.profileId } }); + const consultation = await prisma.consultation.create({ data: { appointmentId: appointment.id } }); + const labOrder = await prisma.labOrder.create({ + data: { + consultationId: consultation.id, + patientProfileId: p.profileId, + orderingDoctorId: doctorProfileId, + facilityProfileId: facilityId, + amountStroops: 1000n, + testType: 'Test panel', + }, + }); + await prisma.paymentIntent.create({ + data: { + labOrderId: labOrder.id, + patientProfileId: p.profileId, + facilityId, + amountStroops: 1000n, + status: 'escrowed', + escrowId: 7n, + stellarTxHash: 'f'.repeat(64), + }, + }); + } + + // One failed login for Alice, so a throttle bucket keyed by her address exists. + const failed = await request(app).post('/auth/login').send({ email: alice.email, password: 'wrong-password' }); + expect(failed.status).toBe(401); + }, 180_000); + + afterAll(async () => { + const authServices = await import('../../src/modules/auth/services/index.js'); + authServices.resetEmailSender(); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + it('rejects a missing or wrong password and non-patients, changing nothing', async () => { + const before = await snapshotAccount(alice); + expect((await deleteAccount(alice, undefined)).status).toBe(400); + expect((await deleteAccount(alice, { password: '' })).status).toBe(400); + const wrong = await deleteAccount(alice, { password: 'not-my-password' }); + expect(wrong.status).toBe(403); + expect(wrong.body).toEqual({ message: 'Password confirmation failed.', code: 'PASSWORD_CONFIRMATION_FAILED' }); + + const asDoctor = await request(app).delete('/patients/me').set('Authorization', `Bearer ${doctorToken}`).send({ password: PASSWORD }); + expect(asDoctor.status).toBe(403); + expect((await request(app).delete('/patients/me').send({ password: PASSWORD })).status).toBe(401); + + const after = await snapshotAccount(alice); + expect({ ...after, user: { ...after.user, updatedAt: null } }).toEqual({ ...before, user: { ...before.user, updatedAt: null } }); + }); + + it('counts wrong confirmations against the login throttle', async () => { + // Every request in this file comes from the same loopback IP, so the + // per-IP bucket is shared; isolate this test to the per-account bucket. + const clearIpBuckets = async () => { + await prisma.loginAttempt.deleteMany({ where: { scope: 'ip' } }); + }; + await clearIpBuckets(); + onTestFinished(clearIpBuckets); + const statuses: number[] = []; + for (let i = 0; i < 4; i += 1) statuses.push((await deleteAccount(erin, { password: 'guess' })).status); + expect(statuses.slice(0, 2)).toEqual([403, 403]); + const throttled = await deleteAccount(erin, { password: PASSWORD }); + expect(throttled.status).toBe(429); + expect(Number(throttled.headers['retry-after'])).toBeGreaterThan(0); + // Still not deleted. + expect((await prisma.user.findUniqueOrThrow({ where: { id: erin.userId } })).deletedAt).toBeNull(); + }); + + it('anonymises identity and keeps appointments and payments byte-for-byte', async () => { + const recordsBefore = await snapshotRecords(alice.profileId); + expect(recordsBefore.appointments).toHaveLength(1); + expect(recordsBefore.payments).toHaveLength(1); + const oldHash = (await prisma.user.findUniqueOrThrow({ where: { id: alice.userId } })).passwordHash; + expect(await prisma.emailVerificationToken.count({ where: { userId: alice.userId } })).toBeGreaterThan(0); + expect(await prisma.loginAttempt.count({ where: { scope: 'email', key: alice.email.toLowerCase() } })).toBe(1); + + const res = await deleteAccount(alice, { password: PASSWORD }); + expect(res.status).toBe(204); + + const user = await prisma.user.findUniqueOrThrow({ where: { id: alice.userId } }); + expect(user.email).toBe(`deleted+${alice.userId}@deleted.invalid`); + expect(user.phone).toBeNull(); + expect(user.emailVerifiedAt).toBeNull(); + expect(user.deletedAt).toBeInstanceOf(Date); + expect(user.passwordHash).not.toBe(oldHash); + expect(await bcrypt.compare(PASSWORD, user.passwordHash)).toBe(false); + + const profile = await prisma.patientProfile.findUniqueOrThrow({ where: { id: alice.profileId } }); + expect(profile).toMatchObject({ fullName: 'Deleted patient', dateOfBirth: null, guardianUserId: null }); + expect(profile.anonymizedAt?.getTime()).toBe(user.deletedAt!.getTime()); + + // No personal field survives anywhere on the two rows. + const rows = JSON.stringify({ user, profile }); + for (const pii of [alice.email, 'alice Fullname', '+2348000000000', '1990-05-01']) expect(rows).not.toContain(pii); + + expect(await prisma.emailVerificationToken.count({ where: { userId: alice.userId } })).toBe(0); + expect(await prisma.loginAttempt.count({ where: { scope: 'email', key: alice.email.toLowerCase() } })).toBe(0); + + // Clinical and payment records: identical, still linked to the profile. + expect(await snapshotRecords(alice.profileId)).toEqual(recordsBefore); + // The slot link is nullable since main made cancellations release it + // (Phase 3); this appointment was never cancelled, so it still points at one. + const slotId = recordsBefore.appointments[0]!.availabilitySlotId; + expect(slotId).not.toBeNull(); + const slot = await prisma.availabilitySlot.findUniqueOrThrow({ where: { id: slotId! } }); + expect(slot.status).toBe('booked'); + }); + + it('deactivates login, refresh and every patient route for the old session', async () => { + const login = await request(app).post('/auth/login').send({ email: alice.email, password: PASSWORD }); + expect(login.status).toBe(401); + const tombstoneLogin = await request(app) + .post('/auth/login') + .send({ email: `deleted+${alice.userId}@deleted.invalid`, password: PASSWORD }); + expect(tombstoneLogin.status).toBe(401); + + const refresh = await request(app) + .post('/auth/refresh') + .set('Cookie', alice.refreshCookie) + .set('X-CSRF-Token', alice.csrfToken); + expect(refresh.status).toBe(401); + + // The access token is a stateless JWT still inside its TTL (no revocation + // list yet, issue #18) — but nothing reachable with it. + const mine = await request(app).get('/appointments/mine').set('Authorization', `Bearer ${alice.token}`); + expect(mine.status).toBe(404); + const slots = (await request(app).get(`/availability/doctors/${doctorProfileId}/slots`)).body as { id: string }[]; + const book = await request(app) + .post('/appointments') + .set('Authorization', `Bearer ${alice.token}`) + .send({ availabilitySlotId: slots[0]!.id }); + expect(book.status).toBe(403); + expect(book.body.code).toBe('EMAIL_NOT_VERIFIED'); + }); + + it('frees the original address for a brand-new, empty signup', async () => { + const res = await request(app).post('/auth/signup').send({ role: 'patient', email: alice.email, password: 'another-password', fullName: 'New Alice' }); + expect(res.status).toBe(200); + expect(res.body.user.id).not.toBe(alice.userId); + const profile = await prisma.patientProfile.findUniqueOrThrow({ where: { userId: res.body.user.id } }); + expect(profile.id).not.toBe(alice.profileId); + expect(await prisma.appointment.count({ where: { patientProfileId: profile.id } })).toBe(0); + // The new account can log in; the old one still can't be reached. + expect((await request(app).post('/auth/login').send({ email: alice.email, password: 'another-password' })).status).toBe(200); + }); + + it('is idempotent: a repeat deletion is a no-op, concurrent ones anonymise once', async () => { + const before = await snapshotAccount(alice); + for (const password of [PASSWORD, 'anything']) { + expect((await deleteAccount(alice, { password })).status).toBe(204); + } + expect(await snapshotAccount(alice)).toEqual(before); + + const [first, second] = await Promise.all([deleteAccount(dave, { password: PASSWORD }), deleteAccount(dave, { password: PASSWORD })]); + expect([first.status, second.status]).toEqual([204, 204]); + const user = await prisma.user.findUniqueOrThrow({ where: { id: dave.userId } }); + expect(user.deletedAt).not.toBeNull(); + expect(user.email).toBe(`deleted+${dave.userId}@deleted.invalid`); + }); + + it('lets an unverified account delete itself', async () => { + expect((await deleteAccount(carol, { password: PASSWORD })).status).toBe(204); + expect((await prisma.patientProfile.findUniqueOrThrow({ where: { id: carol.profileId } })).anonymizedAt).not.toBeNull(); + }); + + it('enforces RESTRICT: a deleted patient with records cannot be hard-deleted by accident', async () => { + // Each refusal is checked with the same classifier the purge uses as its + // race backstop, against the error Postgres actually raises. + const refusal = (promise: Promise) => + promise.then( + () => 'deleted', + (err) => (patients.isForeignKeyRefusal(err) ? 'refused' : `other: ${String(err)}`), + ); + expect(await refusal(prisma.user.delete({ where: { id: alice.userId } }))).toBe('refused'); + expect(await refusal(prisma.patientProfile.delete({ where: { id: alice.profileId } }))).toBe('refused'); + // Even a patient with no records: the User row can't go while its profile exists. + expect(await refusal(prisma.user.delete({ where: { id: carol.userId } }))).toBe('refused'); + // And the purge's own delete path is refused for a profile with records. + expect( + await refusal(prisma.$transaction((tx) => tx.patientProfile.deleteMany({ where: { id: alice.profileId } }))), + ).toBe('refused'); + expect(await prisma.user.count({ where: { id: { in: [alice.userId, carol.userId] } } })).toBe(2); + }); + + it('purges only record-free tombstones past the grace period, and is safe to re-run', async () => { + const bobBefore = await snapshotAccount(bob); + const aliceRecords = await snapshotRecords(alice.profileId); + // Carol deleted long ago; Dave just now (inside the grace period). + const longAgo = new Date(Date.now() - 48 * 60 * 60 * 1000); + await prisma.patientProfile.update({ where: { id: carol.profileId }, data: { anonymizedAt: longAgo } }); + await prisma.patientProfile.update({ where: { id: alice.profileId }, data: { anonymizedAt: longAgo } }); + + const result = await patients.purgeDeletedPatientAccounts(); + expect(result).toEqual({ purged: 1, held: 1, failed: 0 }); + + expect(await prisma.user.findUnique({ where: { id: carol.userId } })).toBeNull(); + expect(await prisma.patientProfile.findUnique({ where: { id: carol.profileId } })).toBeNull(); + // Dave: still inside the grace period. + expect(await prisma.patientProfile.findUnique({ where: { id: dave.profileId } })).not.toBeNull(); + // Alice: has records, kept with them. + expect(await prisma.patientProfile.findUnique({ where: { id: alice.profileId } })).not.toBeNull(); + expect(await snapshotRecords(alice.profileId)).toEqual(aliceRecords); + // Bob and live accounts: untouched. + expect(await snapshotAccount(bob)).toEqual(bobBefore); + expect(await prisma.user.count({ where: { deletedAt: null, role: 'patient' } })).toBeGreaterThanOrEqual(3); + + // Re-running purges nothing new; moving the clock past Dave's grace purges him. + expect(await patients.purgeDeletedPatientAccounts()).toEqual({ purged: 0, held: 1, failed: 0 }); + const later = new Date(Date.now() + 25 * 60 * 60 * 1000); + expect(await patients.purgeDeletedPatientAccounts(later)).toEqual({ purged: 1, held: 1, failed: 0 }); + expect(await prisma.user.findUnique({ where: { id: dave.userId } })).toBeNull(); + }); + + it('deletes exports at anonymisation, and holds a tombstone for referrals and access logs too', async () => { + expect(patients.listRetentionHolds().map((hold) => hold.name)).toEqual( + expect.arrayContaining(['appointments', 'payments', 'orders', 'referrals', 'access_logs']), + ); + // Alice's lab order alone is enough for the orders hold. + const ordersHold = patients.listRetentionHolds().find((hold) => hold.name === 'orders')!; + expect(await ordersHold.holds(alice.profileId)).toBe(true); + + // A fresh patient: erin's address is locked by the throttle test. + const frank = await signupPatient('frank'); + // An export (a derived copy, ADR 0010) goes with the anonymisation. + const exportRow = await prisma.patientExport.create({ + data: { patientProfileId: frank.profileId, chunks: { create: { attempt: 0, seq: 0, data: '{}' } } }, + }); + expect((await deleteAccount(frank, { password: PASSWORD })).status).toBe(204); + expect(await prisma.patientExport.count({ where: { patientProfileId: frank.profileId } })).toBe(0); + expect(await prisma.patientExportChunk.count({ where: { exportId: exportRow.id } })).toBe(0); + + // A referral (RESTRICT) and an access-log row (CASCADE) each keep the + // tombstone; the access log must not be erased by the purge. + const consultation = await prisma.consultation.findFirstOrThrow(); + const referral = await prisma.referral.create({ + data: { + patientProfileId: frank.profileId, + sourceConsultationId: consultation.id, + referringDoctorId: doctorProfileId, + targetDoctorId: doctorProfileId, + snapshotNotes: 'snapshot', + }, + }); + const accessLog = await prisma.accessLog.create({ + data: { patientProfileId: frank.profileId, entityType: 'PatientProfile', entityId: frank.profileId }, + }); + const later = new Date(Date.now() + 25 * 60 * 60 * 1000); + + expect((await patients.purgeDeletedPatientAccounts(later)).purged).toBe(0); + expect(await prisma.patientProfile.findUnique({ where: { id: frank.profileId } })).not.toBeNull(); + + await prisma.referral.delete({ where: { id: referral.id } }); + expect((await patients.purgeDeletedPatientAccounts(later)).purged).toBe(0); + expect(await prisma.accessLog.findUnique({ where: { id: accessLog.id } })).not.toBeNull(); + + // Once the access log's own retention prune has removed it, the tombstone goes. + await prisma.accessLog.delete({ where: { id: accessLog.id } }); + expect((await patients.purgeDeletedPatientAccounts(later)).purged).toBe(1); + expect(await prisma.user.findUnique({ where: { id: frank.userId } })).toBeNull(); + }); + + it('leaves unrelated patients completely unchanged', async () => { + const user = await prisma.user.findUniqueOrThrow({ where: { id: bob.userId } }); + expect(user).toMatchObject({ email: bob.email, phone: '+2348000000000', deletedAt: null }); + expect(user.emailVerifiedAt).not.toBeNull(); + const profile = await prisma.patientProfile.findUniqueOrThrow({ where: { id: bob.profileId } }); + expect(profile).toMatchObject({ fullName: 'bob Fullname', anonymizedAt: null }); + expect((await request(app).post('/auth/login').send({ email: bob.email, password: PASSWORD })).status).toBe(200); + expect((await request(app).get('/appointments/mine').set('Authorization', `Bearer ${bob.token}`)).body).toHaveLength(1); + }); +}); diff --git a/backend/tests/unit/account-deletion.test.ts b/backend/tests/unit/account-deletion.test.ts new file mode 100644 index 0000000..d1d164a --- /dev/null +++ b/backend/tests/unit/account-deletion.test.ts @@ -0,0 +1,49 @@ +// Account deletion (Phase 8, issue #41, docs/adr/0010): the pure helpers. +// Database behaviour is in tests/integration/account-deletion.test.mts. +import { describe, expect, it } from 'vitest'; +import { Prisma } from '@prisma/client'; +import { tombstoneEmail } from '../../src/modules/auth/services'; +import { isForeignKeyRefusal } from '../../src/modules/patients/services'; +import { signupSchema } from '../../src/modules/auth/validation'; + +describe('tombstoneEmail', () => { + it('is stable per user, unique across users, and on the reserved .invalid TLD', () => { + const a = tombstoneEmail('11111111-1111-1111-1111-111111111111'); + expect(a).toBe(tombstoneEmail('11111111-1111-1111-1111-111111111111')); + expect(a).not.toBe(tombstoneEmail('22222222-2222-2222-2222-222222222222')); + expect(a.endsWith('@deleted.invalid')).toBe(true); + }); + + it('never collides with an address someone could sign up with in practice', () => { + // It's syntactically an email (so the column's shape is unchanged) but + // on a TLD that can't be registered or receive mail. + const parsed = signupSchema.safeParse({ + role: 'patient', + email: tombstoneEmail('11111111-1111-1111-1111-111111111111'), + password: 'longenough', + fullName: 'x', + }); + expect(parsed.success).toBe(true); + }); +}); + +describe('isForeignKeyRefusal', () => { + it('recognises Postgres RESTRICT (23001), which Prisma reports as an unknown error', () => { + const err = new Prisma.PrismaClientUnknownRequestError( + 'PostgresError { code: "23001", message: "update or delete on table \\"User\\" violates RESTRICT setting of foreign key constraint" }', + { clientVersion: 'test' }, + ); + expect(isForeignKeyRefusal(err)).toBe(true); + }); + + it('recognises P2003 (NO ACTION / 23503)', () => { + const err = new Prisma.PrismaClientKnownRequestError('fk', { code: 'P2003', clientVersion: 'test' }); + expect(isForeignKeyRefusal(err)).toBe(true); + }); + + it('does not swallow unrelated errors', () => { + expect(isForeignKeyRefusal(new Prisma.PrismaClientKnownRequestError('dup', { code: 'P2002', clientVersion: 'test' }))).toBe(false); + expect(isForeignKeyRefusal(new Prisma.PrismaClientUnknownRequestError('deadlock detected', { clientVersion: 'test' }))).toBe(false); + expect(isForeignKeyRefusal(new Error('boom'))).toBe(false); + }); +}); diff --git a/changelog.d/41-account-deletion-retention.md b/changelog.d/41-account-deletion-retention.md new file mode 100644 index 0000000..8997387 --- /dev/null +++ b/changelog.d/41-account-deletion-retention.md @@ -0,0 +1,21 @@ +Phase 8: account deletion with clinical-record retention +([issue #41](https://github.com/DocForum/docforum-core/issues/41)), with ADR +0010's per-model retention table. `DELETE /patients/me` anonymises in place +rather than cascading: `User.deletedAt` + `PatientProfile.anonymizedAt`, +tombstoned email (frees the address for a new signup), unusable password, +cleared verification; appointments and payment intents are untouched and keep +their FK. The migration flips `PatientProfile.userId` from CASCADE to +RESTRICT (the only FK changed — inventory in the ADR). An interval purge +removes only tombstones no module holds records for. Found while testing: +Postgres reports RESTRICT as SQLSTATE 23001, which Prisma surfaces as an +unknown error rather than P2003 — recorded in ARCHITECTURE.md §6.1.6. The +legal retention period for clinical records is explicitly left open (an open +decision in the ADR), not invented. + +Rebased onto `main` after #40 (patient export), #45 (access log) and the +Phase 3/4 referral and order models landed: the purge now also holds back for +prescriptions, lab orders, referrals and access-log rows (each through its +own module's service), and a patient's exports are deleted inside the +anonymisation transaction, as the ADR's wiring note required of whichever of +#40/#41 merged second. ADR renumbered 0007 → 0010 (0007 is now the backups +ADR). diff --git a/docs/adr/0010-account-deletion-and-clinical-record-retention.md b/docs/adr/0010-account-deletion-and-clinical-record-retention.md new file mode 100644 index 0000000..540469a --- /dev/null +++ b/docs/adr/0010-account-deletion-and-clinical-record-retention.md @@ -0,0 +1,159 @@ +# ADR 0010: Patient account deletion anonymises identity and retains clinical, order and payment records + +Status: Proposed (issue #41). Becomes Accepted when the implementing PR merges. + +## Context + +A patient must be able to delete their account (PRD §5: the architecture +must not preclude NDPR/GDPR, and erasure is the request after access, #40). +But the records that hang off a patient are not only the patient's: + +- an `Appointment` is part of a doctor's calendar and clinical history; +- a `PaymentIntent` is a financial record whose on-chain escrow + (`stellarTxHash`, `escrowId`) is immutable and must stay reconcilable; +- PRD §7 makes orders append-only "for legal and clinical safety", and + requires every state transition on clinical records to stay attributable. + +Before this ADR, the only deletion semantics in the schema were the foreign +keys' `ON DELETE` actions. Inventory, taken from the committed migrations +(not assumed): + +| FK | ON DELETE (before) | Effect of deleting a patient's `User` row | +|---|---|---| +| `PatientProfile.userId → User` | **CASCADE** | Deletes the profile, the clinical identity anchor, whenever nothing RESTRICTs it, and fails with a raw FK error when something does | +| `EmailVerificationToken.userId → User` | CASCADE | Tokens go with the user (fine) | +| `Appointment.patientProfileId → PatientProfile` | RESTRICT (Prisma default) | Blocks the cascade above, so the delete errors out | +| `PaymentIntent.patientProfileId → PatientProfile` | RESTRICT (Prisma default) | Same | +| `DoctorProfile.userId → User`, `FacilityProfile.userId → User` | CASCADE | Not patient data. Their clinical/payment references (`Appointment.doctorProfileId`, `PaymentIntent.facilityId`) are already RESTRICT, so a cascade there can fail but can't destroy history | +| `AvailabilitySlot → DoctorProfile`, `DoctorSpecialty → *`, `FacilityCapability → FacilityProfile`, `WalletLink → FacilityProfile` | CASCADE | Operational rows of doctors/facilities. A booked slot is protected by `Appointment.availabilitySlotId` (RESTRICT) | +| `Appointment.availabilitySlotId → AvailabilitySlot`, `Appointment.doctorProfileId → DoctorProfile`, `PaymentIntent.facilityId → FacilityProfile` | RESTRICT | Already protective | + +Models that reference `PatientProfile` and landed on `main` after this +inventory was first taken (re-checked when rebasing this PR): + +| FK | ON DELETE | Treatment | +|---|---|---| +| `Referral.patientProfileId → PatientProfile` | RESTRICT (Prisma default) | Clinical record — retention hold `referrals` | +| `Prescription.patientProfileId`, `LabOrder.patientProfileId → PatientProfile` | RESTRICT (Prisma default) | Append-only orders — retention hold `orders` | +| `AccessLog.patientProfileId → PatientProfile` (issue #45) | CASCADE | Audit of who viewed the record — retention hold `access_logs` (below) | +| `PatientExport.patientProfileId → PatientProfile` (issue #40) | CASCADE | Derived copy — deleted at anonymisation | + +`IntakeForm`, `Consultation`, `LabResult` and `FulfillmentRecord` reach the +patient through an appointment or an order rather than their own patient +foreign key, so the holds above already cover them. Anything that later +gains a direct patient FK is covered by the rule in the retention table. + +No repository document establishes a **legal retention period** for +clinical, order or payment records after an account is deleted (PRD §5 puts +the full compliance programme out of scope). This ADR does not invent one. +That decision was confirmed with the maintainer before implementation (see +"Open decision" below). + +## Decision + +**Deleting a patient account anonymises the identity in place. It never +deletes the `User` or `PatientProfile` row while any clinical, order or +payment record references them.** Clinical, order and payment records are +kept intact and keep their foreign keys, so a doctor still sees "this +appointment happened", an auditor can still reconcile a payment against +its escrow, and nothing in them is rewritten. They just no longer lead to +a person. + +### Retention table + +| Model / data | At `DELETE /patients/me` | Retained for | Rationale | +|---|---|---|---| +| `User.email` | **Anonymised** to `deleted+@deleted.invalid` | until purged (below) | Frees the address: the unique index no longer blocks a future signup with it. `.invalid` is reserved (RFC 2606), so it can never deliver mail. Derived from the id, so it's unique and identical on every retry. | +| `User.phone` | **Deleted** (set null) | — | PII, no retention purpose. | +| `User.passwordHash` | **Replaced** with a bcrypt hash of 32 random bytes nobody knows | — | Login is impossible even if someone learned the tombstone address. Still a well-formed hash, so the login path's timing stays uniform. | +| `User.emailVerifiedAt` | **Cleared** | — | The address is gone. This also makes the existing `requireVerifiedEmail` guard refuse booking/payment mutations immediately, for an access token that's still inside its 15-minute TTL. | +| `User.deletedAt` (new) | **Set** | permanent | Irreversible marker. Refresh is refused once it's set, it makes repeat requests a no-op, and it's the purge clock. Kept distinct from admin deactivation (#39 / PR #141), which is reversible. | +| `User.role`, `User.id`, `createdAt` | Retained | while the row exists | Not identifying on their own. The id is what clinical rows ultimately join to. | +| `PatientProfile.fullName` | **Anonymised** to `Deleted patient` | — | What a doctor's appointment list shows from now on. | +| `PatientProfile.dateOfBirth` | **Deleted** (null) | — | PII. | +| `PatientProfile.guardianUserId` | **Deleted** (null) | — | Points at another person. Unenforced field; #42 replaces it. | +| `PatientProfile.anonymizedAt` (new) | **Set** | permanent | Owned by the patients module. Every patient route resolves the caller's profile through `getMyPatientProfile`, which refuses an anonymised profile, so booking, listing and repeat deletion stop working immediately. | +| `PatientProfile.id` | Retained | while any record references it | The link that keeps appointments/payments attached without any PII. | +| `EmailVerificationToken` (all of the user's) | **Deleted** | — | Bookkeeping tied to the old address. | +| `LoginAttempt` bucket keyed by the old normalised email | **Deleted** | — | It *is* the email address. IP-keyed buckets aren't tied to the account and keep their existing 24 h prune. | +| `Appointment` | **Retained unchanged** | **Indefinitely. Legal period is an open decision** | Clinical record, and part of the doctor's history. FK stays RESTRICT. | +| `PaymentIntent` | **Retained unchanged** | **Indefinitely. Legal period is an open decision** | Financial record; the escrow on-chain is immutable regardless. FK stays RESTRICT. | +| `Referral`, `Prescription`(+items), `LabOrder`(+`LabResult`) | **Retained unchanged** | **Indefinitely. Open decision** | Hard rules 1/3: orders and referral snapshots are immutable. Patient FK is RESTRICT; retention holds `referrals` and `orders`. | +| Future clinical/order rows (`IntakeForm`, `Consultation`, `FulfillmentRecord`, if they gain a direct patient FK) | **Retained unchanged** | **Indefinitely. Open decision** | **Rule for whoever builds them:** their patient FK must be `onDelete: Restrict`, and they must be added to the purge's retention holds (`src/retention-holds.ts`). | +| `AccessLog` rows about the patient (issue #45) | **Retained unchanged** | `ACCESS_LOG_RETENTION_DAYS`, then pruned by its own job | The audit trail of who read the record. Its FK cascades, so the purge holds (`access_logs`) until the access-log prune has removed them, instead of erasing them with the profile. | +| `PatientExport` (+chunks) (issue #40) | **Deleted at anonymisation**, in the same transaction | — | Derived copies, not records of care. An export mid-generation loses its row, so it can never publish. | +| Notifications / other copies of patient data (future) | **Deleted at anonymisation** | — | Derived copies, not records of care. | + +### What it does to related records + +Nothing is cascaded. Appointments and payment intents keep pointing at the +same `PatientProfile.id`. Doctors see `Deleted patient` in place of a name. +The escrow and its tx hash stay reconcilable. Free-text clinical content +written in the future (intake notes, consultation notes) is **not** +rewritten. It's a clinical record, and editing it would break the +append-only rules. If such text names the patient, that's residual personal +data in the clinical record, which retention law typically permits, and it's +listed under open decisions. + +### Schema change + +`PatientProfile.userId → User` becomes **`ON DELETE RESTRICT`**. Hard-deleting +a patient's `User` row is no longer a way anything can happen. Anonymisation +is the path, and an accidental `DELETE FROM "User"` fails loudly instead of +taking the clinical identity anchor with it. +`Appointment.patientProfileId` and `PaymentIntent.patientProfileId` are +already RESTRICT; they're now written as explicit `onDelete: Restrict` in +`schema.prisma` so the intent can't be lost to a default changing. Doctor +and facility cascades are deliberately not changed: no deletion flow exists +for those roles, their clinical/payment references are already RESTRICT, +and changing them is a separate decision for whoever designs those flows. + +### Scheduled purge + +An interval job (`src/jobs/account-purge.ts`; plain interval, per +ARCHITECTURE_ESSENTIALS) hard-deletes **tombstones only**: an anonymised +patient whose profile has no retained records (no appointment, payment +intent, prescription, lab order, referral or access-log row, checked through +each module's service via `src/retention-holds.ts`), +once `ACCOUNT_PURGE_GRACE_HOURS` (default 24) has passed since deletion. +The grace period is operational, not legal. It lets requests already in +flight under a still-valid access token finish before the row disappears. + +- It never deletes clinical/order/payment rows. With the retention period + open, nothing it could delete is a record of care. +- It is race-safe. Holds are checked first, and the delete itself is + backstopped by the RESTRICT foreign keys, so a record that appeared + between the check and the delete makes the delete fail. That account is + skipped, never cascaded. +- It is idempotent. It works only on rows where `anonymizedAt`/`deletedAt` + are already set, and re-running it finds nothing new. +- Logs carry counts and error types only, never ids paired with personal + fields, and never error messages. + +When a legal retention period is decided, the purge is where it goes: a +per-category age check before a record type stops counting as a hold. That +is a follow-up, not something this ADR pre-builds. + +## Open decision (not resolved here) + +- **How long clinical, order and payment records are kept after deletion**, + per jurisdiction (e.g. NDPR in Nigeria; medical-record retention rules). + Until it's decided they're kept indefinitely, which is the + safe direction for clinical safety and the reversible one: a record kept + too long can still be deleted later, a record deleted too early can't be + recovered. Confirmed with the maintainer for #41. +- Residual personal data inside clinical free text (above). + +## Consequences + +- `DELETE /patients/me` requires the current password. A failed confirmation + counts against the account's login-throttle bucket, so a stolen access + token can't be used to brute-force the password through this route. +- Access tokens are stateless JWTs with a 15-minute TTL, and there's no + revocation list (issue #18). After deletion the token still passes + `authenticate` until it expires, but every patient route refuses it + (anonymised profile), booking/payments refuse it (no verified email), and + refresh refuses the session (`deletedAt`). Nothing personal is reachable + through it. +- Anonymisation is irreversible. There's no undo and no "restore my + account". The old address can sign up again as a brand-new, empty account. diff --git a/docs/api/README.md b/docs/api/README.md index 1fcc463..0616027 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -355,6 +355,39 @@ the cleanup job deletes the stored bundle. Other knobs: `PATIENT_EXPORT_PAGE_SIZ `PATIENT_EXPORT_MAX_ATTEMPTS` — see `backend/src/config/index.ts` and `backend/src/jobs/README.md`. +### Account deletion (Phase 8, issue #41) + +| Method & path | Auth | Body | Notes | +|---|---|---|---| +| `DELETE /patients/me` | patient | `{ password }` | **Deletes the caller's account** (Phase 8, issue #41 — `docs/adr/0010`). `204 No Content`. Irreversible. | + +What deletion does: the login identity and profile are **anonymised in +place** — email becomes `deleted+@deleted.invalid` (so the original +address is free to sign up again, as a new empty account), phone/DOB are +cleared, the name becomes `Deleted patient`, the password becomes unusable, +verification tokens and the address's login-throttle bucket are deleted. +**Appointments and payment records are kept unchanged** (clinical and +financial records; see the ADR's retention table), still linked to the +anonymised profile. + +Errors: `400` without a `password`; `403 PASSWORD_CONFIRMATION_FAILED` for a +wrong one; `403` for non-patient roles. Wrong confirmations count against the +account's login throttle (issue #35), so repeated guesses get `429` with +`Retry-After`. Not gated on email verification — an unverified account can +delete itself. + +Idempotent: a repeat request from the same (still unexpired) access token +returns `204` and changes nothing; concurrent requests anonymise once. + +After deletion: login and `/auth/refresh` return `401`. The access token is a +stateless JWT and stays syntactically valid until its 15-minute TTL ends (no +revocation list yet, issue #18), but every patient route returns `404` for +it and booking/payment routes return `403 EMAIL_NOT_VERIFIED`. Clients should +discard their tokens on `204`. + +A scheduled purge later hard-deletes deleted accounts that have **no** +retained records, `ACCOUNT_PURGE_GRACE_HOURS` (default 24) after deletion. + ## Not yet implemented `patients`, `facilities`, `prescriptions`, `lab-orders`,