From 290bacdf0d7ca1e59adda4008f00b3ea2f232a30 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 09:40:37 +0000 Subject: [PATCH] feat(patients): account deletion with clinical-record retention: anonymise identity, keep the audit trail ADR 0007 decides, per model, what is deleted, anonymised or retained. The FK inventory was taken from the committed migrations. The legal retention period for clinical, order and payment records is left as an explicit open decision (kept indefinitely). - DELETE /patients/me with password re-confirmation, sharing the login throttle. It anonymises in place in one transaction: email tombstoned to deleted+@deleted.invalid (frees the address), phone/DOB/guardian cleared, name -> "Deleted patient", unusable password, verification cleared, User.deletedAt + PatientProfile.anonymizedAt set, verification tokens and the email throttle bucket deleted. Idempotent and safe under concurrency. - Login and refresh refuse deleted accounts; every patient route refuses an anonymised profile. - Appointments and payment intents are kept unchanged. - Migration: PatientProfile.userId FK CASCADE -> RESTRICT (the only FK changed). Appointment/PaymentIntent RESTRICT is now explicit in the schema; no SQL change there. - Interval purge of record-free tombstones after a grace period. Retention holds come from each module's service (src/retention-holds.ts), with the RESTRICT FKs as the race backstop. Closes #41 Co-Authored-By: Claude Opus 5.5 (1M context) --- ARCHITECTURE.md | 29 +- ARCHITECTURE_ESSENTIALS.md | 3 +- ROADMAP.md | 1 + backend/.env.example | 5 + .../migration.sql | 12 + backend/prisma/schema.prisma | 26 +- backend/src/app.ts | 3 + backend/src/config/index.ts | 9 + backend/src/jobs/README.md | 1 + backend/src/jobs/account-purge.ts | 33 ++ .../appointments/repositories/index.ts | 5 + .../modules/appointments/services/index.ts | 6 + backend/src/modules/audit/services/index.ts | 10 + .../src/modules/auth/repositories/index.ts | 48 +- backend/src/modules/auth/services/index.ts | 92 +++- .../src/modules/orders/repositories/index.ts | 10 +- backend/src/modules/orders/services/index.ts | 9 +- .../src/modules/patients/controllers/index.ts | 17 + .../modules/patients/repositories/index.ts | 49 +- backend/src/modules/patients/routes/index.ts | 5 + .../src/modules/patients/services/index.ts | 144 +++++- backend/src/modules/patients/validation.ts | 6 + .../modules/payments/repositories/index.ts | 5 + .../src/modules/payments/services/index.ts | 5 + .../modules/referrals/repositories/index.ts | 4 + .../src/modules/referrals/services/index.ts | 7 + backend/src/retention-holds.ts | 29 ++ backend/src/server.ts | 4 +- .../integration/account-deletion.test.mts | 446 ++++++++++++++++++ backend/tests/unit/account-deletion.test.ts | 49 ++ changelog.d/41-account-deletion-retention.md | 21 + ...-deletion-and-clinical-record-retention.md | 159 +++++++ docs/api/README.md | 33 ++ 33 files changed, 1268 insertions(+), 17 deletions(-) create mode 100644 backend/prisma/migrations/20261021120000_account_deletion_retention/migration.sql create mode 100644 backend/src/jobs/account-purge.ts create mode 100644 backend/src/modules/patients/validation.ts create mode 100644 backend/src/retention-holds.ts create mode 100644 backend/tests/integration/account-deletion.test.mts create mode 100644 backend/tests/unit/account-deletion.test.ts create mode 100644 changelog.d/41-account-deletion-retention.md create mode 100644 docs/adr/0010-account-deletion-and-clinical-record-retention.md 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 faa6814..66f13e6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -123,6 +123,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 7696748..d118d27 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 f059127..0dfa68f 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -132,6 +132,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 affe04d..06548ff 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -72,3 +72,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 66d7154..64468d2 100644 --- a/backend/src/modules/payments/services/index.ts +++ b/backend/src/modules/payments/services/index.ts @@ -263,3 +263,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 ce32fa2..5285795 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -331,6 +331,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`,