From e35e6f55af027803b40aab587793eb4400f334d9 Mon Sep 17 00:00:00 2001 From: Zainab Wahab Date: Sat, 17 Oct 2026 22:55:46 +0100 Subject: [PATCH] feat(audit): AuditEvent table and write path for existing state transitions - AuditEvent model + migration: actor user/role, entity type/id, action, from/to state, request id, minimal validated metadata, createdAt. Append-only: no update/delete path in code, and DB triggers reject UPDATE/DELETE (row) and TRUNCATE (statement). No FKs, on purpose. - New audit module. record(tx, actor, event) refuses the root client, so an event can only be written inside the transaction of the change it describes. Metadata is limited to flat scalars, bounded, and keys that could carry PHI or secrets are refused. - X-Request-Id middleware (keeps a sane inbound id, otherwise mints a UUID); controllers pass auditActor(req). - Wired into appointment booking (inside bookSlot's existing transaction, after the slot claim), doctor verification (previous status read FOR UPDATE; a no-op records nothing), and payment fund/release/refund (status writes conditional on the observed state; a racing second action gets 409 and records nothing). - GET /admin/audit-events: admin-only, entity/actor/date filters, (createdAt, id) keyset cursor, limit capped at 200. - Escrow SDK test seam so the payment flows are tested without testnet. Phase 3/4 transitions are explicitly left to wire when they're built. Closes #43 Co-Authored-By: Claude Opus 5.5 (1M context) --- ARCHITECTURE.md | 33 +- ARCHITECTURE_ESSENTIALS.md | 2 +- ROADMAP.md | 14 +- .../migration.sql | 50 ++ backend/prisma/schema.prisma | 38 ++ backend/src/app.ts | 6 + backend/src/middleware/request-id.ts | 38 ++ .../modules/appointments/controllers/index.ts | 3 +- .../modules/appointments/services/index.ts | 24 +- .../src/modules/audit/controllers/index.ts | 9 + .../src/modules/audit/repositories/index.ts | 57 ++ backend/src/modules/audit/routes/index.ts | 9 + backend/src/modules/audit/services/index.ts | 152 ++++++ backend/src/modules/audit/validation.ts | 24 + backend/src/modules/auth/controllers/index.ts | 5 +- .../src/modules/doctors/controllers/index.ts | 3 +- .../src/modules/doctors/repositories/index.ts | 25 +- backend/src/modules/doctors/services/index.ts | 30 +- .../src/modules/payments/controllers/index.ts | 7 +- .../modules/payments/repositories/index.ts | 31 +- .../payments/services/escrow-sdk-loader.ts | 9 + .../src/modules/payments/services/index.ts | 60 ++- backend/tests/integration/audit.test.mts | 486 ++++++++++++++++++ .../testnet-integration/payments.test.mts | 15 +- backend/tests/unit/audit.test.ts | 110 ++++ docs/api/README.md | 32 +- 26 files changed, 1232 insertions(+), 40 deletions(-) create mode 100644 backend/prisma/migrations/20261017212335_add_audit_event/migration.sql create mode 100644 backend/src/middleware/request-id.ts create mode 100644 backend/src/modules/audit/controllers/index.ts create mode 100644 backend/src/modules/audit/repositories/index.ts create mode 100644 backend/src/modules/audit/routes/index.ts create mode 100644 backend/src/modules/audit/services/index.ts create mode 100644 backend/src/modules/audit/validation.ts create mode 100644 backend/tests/integration/audit.test.mts create mode 100644 backend/tests/unit/audit.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8fb9ba9..0b693ba 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -53,6 +53,7 @@ Each module under `backend/src/modules//` owns its own `routes/`, `control - `lab-orders` — lab order issuance and versioning. - `pharmacy-orders` — fulfillment-side record for prescriptions at a specific pharmacy (kept distinct from `prescriptions` itself — see data model note in §4.7). - `notifications` — outbound notification dispatch, provider-agnostic. +- `audit` — the append-only `AuditEvent` trail (Phase 8, issue #43). Owns one write helper, `auditService.record(tx, actor, event)`, that every module calls **inside the transaction of the state change it describes** (it refuses the root client), and the admin-only `GET /admin/audit-events`. Controllers build the actor with `auditActor(req)` (`src/middleware/request-id.ts`: user id, role, `X-Request-Id`). Wired today: appointment booking, doctor verification, payment fund/release/refund. **Phase 3/4 transitions (referrals, prescriptions, lab orders, fulfillment) must call `record()` in the same transaction when they're built** — that wiring is not done here. ## 4. Data models @@ -197,6 +198,36 @@ LoginAttempt (issue #35) `User` fields added in Phase 8 live in §4.1; the `email_verified_at` / `EmailVerificationToken` pair (issue #34) is in §4.1 as well. +### 4.10 Audit trail (Phase 8, issue #43) +``` +AuditEvent + id, actor_user_id (nullable — null = system), actor_role (nullable Role) + entity_type ('Appointment' | 'DoctorProfile' | 'PaymentIntent' so far), entity_id + action (dotted, e.g. 'appointment.booked'), from_state, to_state (nullable) + request_id (X-Request-Id of the causing request), metadata (jsonb, nullable) + created_at + # APPEND-ONLY: no update/delete path in code, and BEFORE UPDATE/DELETE + (row) + BEFORE TRUNCATE (statement) triggers reject mutation in the + database. A future retention policy must drop/recreate them in its own + reviewed migration. + # No foreign keys on purpose: an ON DELETE action would have to rewrite + audit rows (forbidden), and RESTRICT would let the trail block account + lifecycle (e.g. issue #41's purge). Ids are recorded as facts. + # Written only by audit.record() inside the transaction of the change it + describes — the event exists iff the change committed. + # metadata: <= 16 flat scalar keys, strings <= 200 chars; keys that look + like personal/secret data (name, email, phone, dob, notes, intake, + password, secret, token, key, body, payload, …) are refused. Never PHI. + # created_at is stamped by Prisma on the app server per row (ms + precision); same-millisecond rows are ordered by id. + # indexes: (entity_type, entity_id, created_at), (actor_user_id, + created_at), (created_at, id) — the last backs the admin cursor. +``` +Transitions recorded (action: from → to): +- `appointment.booked`: `null → scheduled` (metadata: slot id, doctor profile id, origin) +- `doctor.verification_status_changed`: previous → new verification status (read under `FOR UPDATE`; a no-op set records nothing) +- `payment_intent.funded` `created → escrowed`, `payment_intent.released` `escrowed → released`, `payment_intent.refunded` `escrowed → refunded` (metadata: order type, amount, escrow id, tx hash, contract id — public on-chain references only). The status write is now conditional on the observed state, so a racing second action gets `409` and records nothing. + ## 5. Key flows (sequence-level, not code) ### 5.1 Booking (concurrency-safe) @@ -230,7 +261,7 @@ LoginAttempt (issue #35) 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). 3. No schema support yet for a doctor working across multiple facilities/locations (PRD edge case 6) — `AvailabilitySlot` currently assumes one calendar per doctor. -4. No audit-log table — every FR around auditability (PRD §7) currently relies on `created_at/updated_at` timestamps only, which is not sufficient for "who changed what." A dedicated `AuditEvent` table is likely needed before this ships to real patients. +4. No audit-log table — every FR around auditability (PRD §7) currently relies on `created_at/updated_at` timestamps only, which is not sufficient for "who changed what." A dedicated `AuditEvent` table is likely needed before this ships to real patients. **Foundation resolved 2026-10-17 (issue #43):** `AuditEvent` (§4.10) + `audit.record()` in the same transaction as the change, append-only at the DB level, wired into booking, doctor verification and payment fund/release/refund. **Still open:** Phase 3/4 transitions (referrals, prescriptions, lab orders, fulfillment) must be wired as they're built; the admin login-throttle clear (`/auth/login-attempts/clear`) still only logs to the console; the app connects as the database owner, which could disable the triggers — enforcing append-only against the owner needs a separate least-privilege DB role (deployment work, issue #22). 5. CSRF on the cookie-authenticated routes — the refresh cookie has to be `SameSite=None; Secure` (the Pages frontend is a different site — see `docs/adr/0003`), which means any site the patient visits can make their browser send it to `/auth/refresh` and `/auth/logout`. **Resolved 2026-10-15 (issue #37):** a per-session CSRF token is bound into the refresh token and required in the `X-CSRF-Token` header, the `Origin` header is validated against an explicit allowlist, and CORS is now a per-environment allowlist (`CORS_ORIGINS`) instead of one configured string. The double-submit cookie pattern — the usual answer — is *not* usable here: a cookie set by this API is a third-party cookie to `docforum.github.io`, so the frontend's JavaScript can never read it; the token travels in the login/refresh response body instead. A request with no `Origin` (curl, server-to-server) is allowed through the Origin check but still needs the token. See `backend/src/middleware/csrf.ts`, and `docs/api/README.md` for what the client must send. Consequence: refresh cookies issued before this change carry no `csrf` claim and are rejected, i.e. existing sessions need one re-login. ### 6.3 What's overengineered (architecture-level, for v1)? diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index a5393e8..19dacaa 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -40,7 +40,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc - No routing rule yet for a referral with no specific target doctor (which specialist gets it?). - No expiry/TTL job for held slots or pending referrals — currently just an enum value with no worker. - Polymorphic `FulfillmentRecord.order_id` is a sketch, not final — may need splitting into two tables. -- No `AuditEvent` table yet, despite PRD requiring auditability — needed before real patient data. +- `AuditEvent` foundation exists (Phase 8, issue #43 — ARCHITECTURE.md §4.10): append-only (DB triggers), written by `auditService.record(tx, …)` **inside the same transaction** as the change. Wired for booking, doctor verification and payment fund/release/refund only — **Phase 3/4 transitions must call it when they're built.** Never put PHI in `metadata`. - No email/SMS provider integrated yet (Phase 6, issue #17) — email verification (issue #34) therefore can't deliver its link on a hosted deployment; `backend/scripts/mark-email-verified.mjs` is the interim operator path. - One doctor = one calendar assumed; multi-facility doctors not modeled. diff --git a/ROADMAP.md b/ROADMAP.md index 501c0f5..f67d309 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,6 +1,6 @@ # ROADMAP.md — DocForum -Last updated: 2026-09-26 +Last updated: 2026-10-17 Status: Phase 0, Phase 1, and Phase 5.5 (Payments, custodial v1 — see `docs/adr/0004-custodial-payments-v1.md`) built and tested. Phases 2-6 and 8-9 scoped into GitHub issues (#1-#22, #21 closed); Phase 7 is @@ -113,6 +113,7 @@ for status and issue links. - [x] Login brute-force protection: per-account and per-IP throttling with backoff. Postgres-backed buckets (`LoginAttempt`), a single conditional upsert so concurrent failures are counted atomically, exponential backoff with `Retry-After`, a bounded temporary lock, an admin clear endpoint, a prune interval in `src/server.ts`, and a `TRUST_PROXY` setting so the IP isn't Render's proxy address. Tracked as [issue #35](https://github.com/DocForum/docforum-core/issues/35) (High, 200 pts) — **Done**, merged as PR #138. - [ ] CSRF protection for the cookie-authenticated refresh/logout endpoints, plus an explicit per-environment CORS allowlist. A per-session CSRF token bound into the refresh token (not a double-submit cookie — a cookie set by this API is third-party to `docforum.github.io`, so the web client can't read it), a required `X-CSRF-Token` header on `/auth/refresh` + `/auth/logout`, an `Origin` allowlist check, and `CORS_ORIGINS` replacing the single configured origin. Tracked as [issue #37](https://github.com/DocForum/docforum-core/issues/37) (High, 200 pts) — **in progress**, branch `feat/csrf-protection-for-cookie-authenticated-refresh`. - [ ] 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**. +- [ ] `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, doctor verification and payment fund/release/refund. Referrals, prescriptions, lab orders and fulfillment (Phases 3–5) must call `record()` when they land; the login-throttle admin clear still logs to console only. Tracked as [issue #43](https://github.com/DocForum/docforum-core/issues/43) (High, 200 pts) — **in review**, branch `feat/43-auditevent-foundation`. - [ ] 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`. - [ ] Privacy access-control audit: confirm intake/consultation notes are visible only per PRD §7 (patient, treating doctor(s) in-thread, fulfilling facility for that specific order only) — write tests, not just code review. Still blocked — meaningfully auditable only once Phase 3/4/5 access paths exist. Not yet scoped into an issue. @@ -380,3 +381,14 @@ for status and issue links. tests assert a forged cross-origin request is rejected while the legitimate cross-site client still succeeds. +- 2026-10-17 — Issue #43 (AuditEvent foundation) opened as a PR: the + `AuditEvent` table (append-only, enforced by DB triggers; no FKs on + purpose), an `audit` module whose `record()` refuses to run outside a + transaction, an `X-Request-Id` middleware, and `GET /admin/audit-events` + with a keyset cursor. Wired into booking (inside `bookSlot`'s existing + transaction, after the slot claim — the concurrency guarantee is + unchanged), doctor verification (previous status read `FOR UPDATE`) and + payment fund/release/refund (status writes now conditional on the + observed state, so a racing second action gets 409 instead of silently + overwriting). Rollback coupling proven with a real injected DB failure, + not a mock. Phase 3/4 wiring explicitly left pending. diff --git a/backend/prisma/migrations/20261017212335_add_audit_event/migration.sql b/backend/prisma/migrations/20261017212335_add_audit_event/migration.sql new file mode 100644 index 0000000..555f003 --- /dev/null +++ b/backend/prisma/migrations/20261017212335_add_audit_event/migration.sql @@ -0,0 +1,50 @@ +-- CreateTable +CREATE TABLE "AuditEvent" ( + "id" TEXT NOT NULL, + "actorUserId" TEXT, + "actorRole" "Role", + "entityType" TEXT NOT NULL, + "entityId" TEXT NOT NULL, + "action" TEXT NOT NULL, + "fromState" TEXT, + "toState" TEXT, + "requestId" TEXT, + "metadata" JSONB, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "AuditEvent_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE INDEX "AuditEvent_entityType_entityId_createdAt_idx" ON "AuditEvent"("entityType", "entityId", "createdAt"); + +-- CreateIndex +CREATE INDEX "AuditEvent_actorUserId_createdAt_idx" ON "AuditEvent"("actorUserId", "createdAt"); + +-- CreateIndex +CREATE INDEX "AuditEvent_createdAt_id_idx" ON "AuditEvent"("createdAt", "id"); + + +-- Append-only enforcement (Phase 8, issue #43). The application has no +-- update/delete path for AuditEvent; these triggers make the database refuse +-- one too, for every role that doesn't disable triggers. Row-level for +-- UPDATE/DELETE, statement-level for TRUNCATE (which bypasses row triggers). +-- +-- A future, reviewed retention policy that must remove old events has to do +-- so in its own migration that explicitly drops/recreates these triggers — +-- deliberately loud, never an incidental side effect of application code. +CREATE FUNCTION "audit_event_reject_mutation"() RETURNS trigger +LANGUAGE plpgsql AS $$ +BEGIN + RAISE EXCEPTION 'AuditEvent is append-only: % is not allowed', TG_OP + USING ERRCODE = 'insufficient_privilege'; +END; +$$; + +CREATE TRIGGER "AuditEvent_no_update_delete" + BEFORE UPDATE OR DELETE ON "AuditEvent" + FOR EACH ROW EXECUTE FUNCTION "audit_event_reject_mutation"(); + +CREATE TRIGGER "AuditEvent_no_truncate" + BEFORE TRUNCATE ON "AuditEvent" + FOR EACH STATEMENT EXECUTE FUNCTION "audit_event_reject_mutation"(); diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 5ceed14..72392e5 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -454,3 +454,41 @@ model WalletLink { createdAt DateTime @default(now()) updatedAt DateTime @updatedAt } + +// --- Audit trail (Phase 8, issue #43) ------------------------------------- +// One row per state transition on a record that matters for clinical, +// legal or financial accountability (PRD §7: "who, when"). Written by the +// audit module's record(), always inside the same transaction as the +// change it describes — if the change rolls back, so does its event. +// +// APPEND-ONLY. There is no update or delete path in code, and a database +// trigger (see the add_audit_event migration) rejects UPDATE, DELETE and +// TRUNCATE outright. Deliberately no foreign keys: an FK's ON DELETE action +// would have to rewrite or remove audit rows (which the trigger forbids), +// and RESTRICT would let the audit trail block account lifecycle. Ids are +// recorded as facts about the past. +// +// No PHI, ever: `metadata` is a small flat map of ids/codes validated by +// record() — never names, contact details, DOB, intake text, or copies of +// request/response bodies. +model AuditEvent { + id String @id @default(uuid()) + // Null only for system-initiated transitions (none exist yet). + actorUserId String? + actorRole Role? + // e.g. 'Appointment', 'DoctorProfile', 'PaymentIntent'. + entityType String + entityId String + // e.g. 'appointment.booked', 'payment_intent.released'. + action String + fromState String? + toState String? + // X-Request-Id of the HTTP request that caused it (src/middleware/request-id.ts). + requestId String? + metadata Json? + createdAt DateTime @default(now()) + + @@index([entityType, entityId, createdAt]) + @@index([actorUserId, createdAt]) + @@index([createdAt, id]) +} diff --git a/backend/src/app.ts b/backend/src/app.ts index c9cd9be..399d79e 100644 --- a/backend/src/app.ts +++ b/backend/src/app.ts @@ -8,6 +8,8 @@ import { doctorsRouter } from './modules/doctors/routes'; import { availabilityRouter } from './modules/availability/routes'; import { appointmentsRouter } from './modules/appointments/routes'; import { paymentsRouter } from './modules/payments/routes'; +import { auditRouter } from './modules/audit/routes'; +import { requestId } from './middleware/request-id'; export function createApp() { const app = express(); @@ -20,6 +22,9 @@ export function createApp() { // Explicit allowlist per environment (Phase 8, issue #37) — see // config.corsOrigins. `credentials: true` is required for the refresh // cookie and is exactly why the allowlist can't be a wildcard. + // Phase 8, issue #43: first, so every response (errors included) carries + // an X-Request-Id and audit events can be tied back to it. + app.use(requestId); app.use(cors({ origin: config.corsOrigins, credentials: true })); app.use(express.json()); app.use(cookieParser()); @@ -31,6 +36,7 @@ export function createApp() { app.use('/availability', availabilityRouter); app.use('/appointments', appointmentsRouter); app.use('/payments', paymentsRouter); + app.use('/admin', auditRouter); // Must be registered last — Express only routes errors here once every // other handler has had a chance to run. diff --git a/backend/src/middleware/request-id.ts b/backend/src/middleware/request-id.ts new file mode 100644 index 0000000..c66c615 --- /dev/null +++ b/backend/src/middleware/request-id.ts @@ -0,0 +1,38 @@ +// Request ids (Phase 8, issue #43). Every request gets one, so an audit +// event can be tied back to the HTTP request (and its logs) that caused it. +// An inbound `X-Request-Id` (e.g. from Render's proxy or a client) is kept +// when it's a sane token; otherwise a fresh UUID is minted. Echoed back in +// the response header either way. +import type { NextFunction, Request, Response } from 'express'; +import { randomUUID } from 'node:crypto'; +import type { AuditActor } from '../modules/audit/services'; + +declare global { + // eslint-disable-next-line @typescript-eslint/no-namespace + namespace Express { + interface Request { + requestId?: string; + } + } +} + +export const REQUEST_ID_HEADER = 'x-request-id'; + +// Bounded charset and length: the value is stored and logged, so it must +// not be a vehicle for arbitrary client data. +const ACCEPTABLE_REQUEST_ID = /^[A-Za-z0-9._:-]{8,128}$/; + +export function requestId(req: Request, res: Response, next: NextFunction) { + const inbound = req.get(REQUEST_ID_HEADER); + const id = inbound && ACCEPTABLE_REQUEST_ID.test(inbound) ? inbound : randomUUID(); + req.requestId = id; + res.set('X-Request-Id', id); + next(); +} + +/** The audit actor for an authenticated request: who, in which role, under + * which request id. Controllers pass this into the service whose state + * change is being audited. */ +export function auditActor(req: Request): AuditActor { + return { userId: req.user?.id ?? null, role: req.user?.role ?? null, requestId: req.requestId ?? null }; +} diff --git a/backend/src/modules/appointments/controllers/index.ts b/backend/src/modules/appointments/controllers/index.ts index fa2a00e..f07f9f7 100644 --- a/backend/src/modules/appointments/controllers/index.ts +++ b/backend/src/modules/appointments/controllers/index.ts @@ -2,11 +2,12 @@ import type { Request, Response } from 'express'; import * as appointmentsService from '../services'; import { bookSlotSchema } from '../validation'; import { HttpError } from '../../../http-error'; +import { auditActor } from '../../../middleware/request-id'; export async function book(req: Request, res: Response) { if (!req.user) throw new HttpError(401, 'Authentication required.'); const { availabilitySlotId } = bookSlotSchema.parse(req.body); - const appointment = await appointmentsService.bookSlot(req.user.id, availabilitySlotId); + const appointment = await appointmentsService.bookSlot(auditActor(req), availabilitySlotId); res.status(201).json(appointment); } diff --git a/backend/src/modules/appointments/services/index.ts b/backend/src/modules/appointments/services/index.ts index f8e50f1..7bf1626 100644 --- a/backend/src/modules/appointments/services/index.ts +++ b/backend/src/modules/appointments/services/index.ts @@ -3,6 +3,7 @@ import { HttpError } from '../../../http-error'; import * as appointmentsRepo from '../repositories'; import * as availabilityService from '../../availability/services'; import * as patientsService from '../../patients/services'; +import * as auditService from '../../audit/services'; /** * Books a slot for the calling patient. This is the correctness-critical @@ -19,9 +20,15 @@ import * as patientsService from '../../patients/services'; * * See backend/tests/integration/booking.test.ts for the concurrency proof * this is built to satisfy. + * + * Phase 8, issue #43: the booking's AuditEvent is written as a third step + * of the same transaction, after the Appointment row exists — so an event + * exists if and only if the booking committed. A caller that loses the + * race fails at step 1 and never reaches it. */ -export async function bookSlot(userId: string, availabilitySlotId: string) { - const patientProfile = await patientsService.getMyPatientProfile(userId); +export async function bookSlot(actor: auditService.AuditActor, availabilitySlotId: string) { + if (!actor.userId) throw new HttpError(401, 'Authentication required.'); + const patientProfile = await patientsService.getMyPatientProfile(actor.userId); return prisma.$transaction(async (tx) => { const claimed = await availabilityService.claimOpenSlot(tx, availabilitySlotId); @@ -36,11 +43,22 @@ export async function bookSlot(userId: string, availabilitySlotId: string) { throw new HttpError(404, 'Slot not found.'); } - return appointmentsRepo.createAppointment(tx, { + const appointment = await appointmentsRepo.createAppointment(tx, { patientProfileId: patientProfile.id, doctorProfileId: slot.doctorProfileId, availabilitySlotId, }); + + await auditService.record(tx, actor, { + entityType: 'Appointment', + entityId: appointment.id, + action: 'appointment.booked', + fromState: null, + toState: appointment.status, + metadata: { availabilitySlotId, doctorProfileId: slot.doctorProfileId, origin: appointment.origin }, + }); + + return appointment; }); } diff --git a/backend/src/modules/audit/controllers/index.ts b/backend/src/modules/audit/controllers/index.ts new file mode 100644 index 0000000..b5a5d5d --- /dev/null +++ b/backend/src/modules/audit/controllers/index.ts @@ -0,0 +1,9 @@ +import type { Request, Response } from 'express'; +import * as auditService from '../services'; +import { listAuditEventsQuerySchema } from '../validation'; + +export async function listAuditEvents(req: Request, res: Response) { + const query = listAuditEventsQuerySchema.parse(req.query); + const page = await auditService.listAuditEvents(query); + res.status(200).json(page); +} diff --git a/backend/src/modules/audit/repositories/index.ts b/backend/src/modules/audit/repositories/index.ts new file mode 100644 index 0000000..18a3c43 --- /dev/null +++ b/backend/src/modules/audit/repositories/index.ts @@ -0,0 +1,57 @@ +// audit repositories — AuditEvent data access. INSERT and SELECT only: this +// table is append-only (issue #43), enforced again by a database trigger. +// There is intentionally no update or delete function here; don't add one. +import { prisma } from '../../../db/prisma-client'; +import type { Prisma, Role } from '@prisma/client'; + +export interface AuditEventRow { + actorUserId: string | null; + actorRole: Role | null; + entityType: string; + entityId: string; + action: string; + fromState: string | null; + toState: string | null; + requestId: string | null; + metadata: Prisma.InputJsonValue | undefined; +} + +/** Must be given the transaction client of the change being audited. */ +export function insertAuditEvent(tx: Prisma.TransactionClient, row: AuditEventRow) { + return tx.auditEvent.create({ data: row }); +} + +export interface AuditEventQuery { + entityType?: string; + entityId?: string; + actorUserId?: string; + from?: Date; + to?: Date; + /** Keyset position: return events strictly older than this one. */ + before?: { createdAt: Date; id: string }; + take: number; +} + +/** Newest first, ordered by (createdAt DESC, id DESC) — a total order, so a + * keyset cursor on that pair never skips or repeats a row. */ +export function findAuditEvents(query: AuditEventQuery) { + const and: Prisma.AuditEventWhereInput[] = []; + if (query.entityType) and.push({ entityType: query.entityType }); + if (query.entityId) and.push({ entityId: query.entityId }); + if (query.actorUserId) and.push({ actorUserId: query.actorUserId }); + if (query.from) and.push({ createdAt: { gte: query.from } }); + if (query.to) and.push({ createdAt: { lte: query.to } }); + if (query.before) { + and.push({ + OR: [ + { createdAt: { lt: query.before.createdAt } }, + { createdAt: query.before.createdAt, id: { lt: query.before.id } }, + ], + }); + } + return prisma.auditEvent.findMany({ + where: { AND: and }, + orderBy: [{ createdAt: 'desc' }, { id: 'desc' }], + take: query.take, + }); +} diff --git a/backend/src/modules/audit/routes/index.ts b/backend/src/modules/audit/routes/index.ts new file mode 100644 index 0000000..a8f74aa --- /dev/null +++ b/backend/src/modules/audit/routes/index.ts @@ -0,0 +1,9 @@ +import { Router } from 'express'; +import { authenticate, requireRole } from '../../../middleware/auth'; +import * as auditController from '../controllers'; + +export const auditRouter = Router(); + +// Phase 8, issue #43 — admin-only, read-only. There is no write route: events +// are only ever created by record() inside the transaction they describe. +auditRouter.get('/audit-events', authenticate, requireRole('admin'), auditController.listAuditEvents); diff --git a/backend/src/modules/audit/services/index.ts b/backend/src/modules/audit/services/index.ts new file mode 100644 index 0000000..914f028 --- /dev/null +++ b/backend/src/modules/audit/services/index.ts @@ -0,0 +1,152 @@ +// audit services (Phase 8, issue #43) — the write helper every module calls +// when it changes the state of an accountable record, plus the admin query. +// Other modules import from here only, never from audit/repositories. +// +// The contract of record(): +// - it runs INSIDE the transaction of the change it describes (it refuses +// the root client), so an event exists if and only if its change +// committed; +// - it carries only what's needed to attribute the transition — actor, +// entity, action, from/to state, request id — plus a small, flat, +// validated metadata map of ids/codes. Never PHI, never request bodies. +import type { Prisma, Role } from '@prisma/client'; +import { HttpError } from '../../../http-error'; +import * as auditRepo from '../repositories'; + +/** Who caused a transition. Controllers build it from the request with + * `auditActor(req)` (src/middleware/request-id.ts). `userId`/`role` are null + * only for system-initiated changes. */ +export interface AuditActor { + userId: string | null; + role: Role | null; + requestId: string | null; +} + +export const SYSTEM_ACTOR: AuditActor = { userId: null, role: null, requestId: null }; + +export type AuditEntityType = 'Appointment' | 'DoctorProfile' | 'PaymentIntent'; + +export type AuditMetadataValue = string | number | boolean | null; +export type AuditMetadata = Record; + +export interface AuditEventInput { + entityType: AuditEntityType; + entityId: string; + action: string; + fromState: string | null; + toState: string | null; + metadata?: AuditMetadata; +} + +const ACTION = /^[a-z][a-z_]*(\.[a-z][a-z_]*)+$/; +const METADATA_KEY = /^[a-zA-Z][a-zA-Z0-9_]{0,63}$/; +const MAX_METADATA_KEYS = 16; +const MAX_METADATA_STRING = 200; +/** Defence in depth against a caller passing personal or secret data: any + * key that *looks* like it could carry it is refused outright. Callers + * should only ever pass ids and codes. */ +const FORBIDDEN_METADATA_KEY = + /(name|email|phone|dob|birth|address|note|intake|symptom|diagnos|password|secret|token|key|credential|body|payload)/i; + +export class AuditMetadataError extends Error { + constructor(message: string) { + super(message); + this.name = 'AuditMetadataError'; + } +} + +/** Validates metadata: flat scalars only, bounded, no suspicious keys. + * Exported for tests. Throws AuditMetadataError — a programming error, so + * it aborts the surrounding transaction rather than being swallowed. */ +export function validateAuditMetadata(metadata: AuditMetadata | undefined): AuditMetadata | undefined { + if (metadata === undefined) return undefined; + if (metadata === null || typeof metadata !== 'object' || Array.isArray(metadata)) { + throw new AuditMetadataError('Audit metadata must be a flat object.'); + } + const entries = Object.entries(metadata); + if (entries.length > MAX_METADATA_KEYS) throw new AuditMetadataError('Audit metadata has too many keys.'); + for (const [key, value] of entries) { + if (!METADATA_KEY.test(key)) throw new AuditMetadataError(`Invalid audit metadata key: ${key}`); + if (FORBIDDEN_METADATA_KEY.test(key) && !/Id$/.test(key)) { + throw new AuditMetadataError(`Audit metadata key not allowed (could carry personal or secret data): ${key}`); + } + const ok = + value === null || + typeof value === 'boolean' || + (typeof value === 'number' && Number.isFinite(value)) || + (typeof value === 'string' && value.length <= MAX_METADATA_STRING); + if (!ok) throw new AuditMetadataError(`Audit metadata value for ${key} must be a short scalar.`); + } + return metadata; +} + +/** + * Records one state transition, inside the caller's transaction. Passing + * the root Prisma client (which would commit the event on its own, before + * — or without — the change) is refused at runtime. + */ +export async function record(tx: Prisma.TransactionClient, actor: AuditActor, event: AuditEventInput): Promise { + if (typeof (tx as { $transaction?: unknown }).$transaction === 'function') { + throw new Error('audit.record() must run inside the transaction of the change it describes.'); + } + if (!ACTION.test(event.action)) throw new Error(`Invalid audit action: ${event.action}`); + const metadata = validateAuditMetadata(event.metadata); + await auditRepo.insertAuditEvent(tx, { + actorUserId: actor.userId, + actorRole: actor.role, + entityType: event.entityType, + entityId: event.entityId, + action: event.action, + fromState: event.fromState, + toState: event.toState, + requestId: actor.requestId, + metadata, + }); +} + +// --- Admin query ----------------------------------------------------------- + +export interface AuditEventFilters { + entityType?: string; + entityId?: string; + actorUserId?: string; + from?: Date; + to?: Date; + cursor?: string; + limit: number; +} + +/** Opaque cursor: base64url of "|" of the last item. */ +export function encodeAuditCursor(event: { createdAt: Date; id: string }): string { + return Buffer.from(`${event.createdAt.toISOString()}|${event.id}`, 'utf8').toString('base64url'); +} + +export function decodeAuditCursor(cursor: string): { createdAt: Date; id: string } { + const raw = Buffer.from(cursor, 'base64url').toString('utf8'); + const separator = raw.indexOf('|'); + const createdAt = new Date(raw.slice(0, separator)); + const id = raw.slice(separator + 1); + if (separator < 1 || Number.isNaN(createdAt.getTime()) || !id) { + throw new HttpError(400, 'Invalid cursor.', 'INVALID_CURSOR'); + } + return { createdAt, id }; +} + +export async function listAuditEvents(filters: AuditEventFilters) { + if (filters.from && filters.to && filters.from.getTime() > filters.to.getTime()) { + throw new HttpError(400, '`from` must not be after `to`.'); + } + const rows = await auditRepo.findAuditEvents({ + entityType: filters.entityType, + entityId: filters.entityId, + actorUserId: filters.actorUserId, + from: filters.from, + to: filters.to, + before: filters.cursor ? decodeAuditCursor(filters.cursor) : undefined, + // One extra row tells us whether there's a next page without a count. + take: filters.limit + 1, + }); + const items = rows.slice(0, filters.limit); + const last = items[items.length - 1]; + return { items, nextCursor: rows.length > filters.limit && last ? encodeAuditCursor(last) : null }; +} diff --git a/backend/src/modules/audit/validation.ts b/backend/src/modules/audit/validation.ts new file mode 100644 index 0000000..561ca85 --- /dev/null +++ b/backend/src/modules/audit/validation.ts @@ -0,0 +1,24 @@ +import { z } from 'zod'; + +const isoDate = z + .string() + .refine((value) => !Number.isNaN(new Date(value).getTime()) && /^\d{4}-\d{2}-\d{2}/.test(value), { + message: 'must be an ISO-8601 date or date-time', + }) + .transform((value) => new Date(value)); + +// `GET /admin/audit-events` (Phase 8, issue #43). Strict: an unknown query +// parameter is a client bug worth surfacing, not something to ignore. +export const listAuditEventsQuerySchema = z + .object({ + entityType: z.string().min(1).max(64).optional(), + entityId: z.string().min(1).max(128).optional(), + actorUserId: z.string().min(1).max(128).optional(), + from: isoDate.optional(), + to: isoDate.optional(), + cursor: z.string().min(1).max(512).optional(), + // Bounded on purpose: no unpaged dump of the audit trail. + limit: z.coerce.number().int().min(1).max(200).default(50), + }) + .strict() + .refine((q) => !q.entityId || q.entityType, { message: 'entityId requires entityType' }); diff --git a/backend/src/modules/auth/controllers/index.ts b/backend/src/modules/auth/controllers/index.ts index f5625b7..1fa46dd 100644 --- a/backend/src/modules/auth/controllers/index.ts +++ b/backend/src/modules/auth/controllers/index.ts @@ -59,8 +59,9 @@ export async function login(req: Request, res: Response) { } /** Admin-only: clear a login lock/throttle bucket for an account or an IP. - * Writes an audit line to the server log — there is no AuditEvent table yet - * (ARCHITECTURE_ESSENTIALS.md known gaps), so this is the interim record. */ + * Writes an audit line to the server log. Not wired into AuditEvent yet + * (issue #43 covered booking, doctor verification and payments only — see + * ARCHITECTURE.md §6.2.4), so this is the interim record. */ export async function clearLoginAttempts(req: Request, res: Response) { const { email, ip } = clearLoginAttemptsSchema.parse(req.body); const cleared = await authService.clearLoginAttempts({ email, ip }); diff --git a/backend/src/modules/doctors/controllers/index.ts b/backend/src/modules/doctors/controllers/index.ts index dc3afe1..dddef28 100644 --- a/backend/src/modules/doctors/controllers/index.ts +++ b/backend/src/modules/doctors/controllers/index.ts @@ -3,6 +3,7 @@ import * as doctorsService from '../services'; import { setVerificationStatusSchema } from '../validation'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; +import { auditActor } from '../../../middleware/request-id'; export async function getMe(req: Request, res: Response) { if (!req.user) throw new HttpError(401, 'Authentication required.'); @@ -13,6 +14,6 @@ export async function getMe(req: Request, res: Response) { export async function setVerification(req: Request, res: Response) { const { status } = setVerificationStatusSchema.parse(req.body); const doctorProfileId = getParam(req, 'doctorProfileId'); - const profile = await doctorsService.setVerificationStatus(doctorProfileId, status); + const profile = await doctorsService.setVerificationStatus(auditActor(req), doctorProfileId, status); res.status(200).json(profile); } diff --git a/backend/src/modules/doctors/repositories/index.ts b/backend/src/modules/doctors/repositories/index.ts index b8d18f5..a2b3c54 100644 --- a/backend/src/modules/doctors/repositories/index.ts +++ b/backend/src/modules/doctors/repositories/index.ts @@ -1,14 +1,29 @@ import { prisma } from '../../../db/prisma-client'; -import type { VerificationStatus } from '@prisma/client'; +import type { Prisma, VerificationStatus } from '@prisma/client'; + +type DbClient = typeof prisma | Prisma.TransactionClient; export function findDoctorProfileByUserId(userId: string) { return prisma.doctorProfile.findUnique({ where: { userId } }); } -export function findDoctorProfileById(id: string) { - return prisma.doctorProfile.findUnique({ where: { id } }); +export function findDoctorProfileById(id: string, db: DbClient = prisma) { + return db.doctorProfile.findUnique({ where: { id } }); +} + +/** Reads the current verification status with a row lock, inside a + * transaction (issue #43): two admins changing it at once are serialised, + * so each audit event records the state that was really replaced. */ +export async function lockVerificationStatus( + tx: Prisma.TransactionClient, + id: string, +): Promise { + const rows = await tx.$queryRaw<{ verificationStatus: VerificationStatus }[]>` + SELECT "verificationStatus" FROM "DoctorProfile" WHERE "id" = ${id} FOR UPDATE + `; + return rows[0]?.verificationStatus ?? null; } -export function updateVerificationStatus(id: string, status: VerificationStatus) { - return prisma.doctorProfile.update({ where: { id }, data: { verificationStatus: status } }); +export function updateVerificationStatus(db: DbClient, id: string, status: VerificationStatus) { + return db.doctorProfile.update({ where: { id }, data: { verificationStatus: status } }); } diff --git a/backend/src/modules/doctors/services/index.ts b/backend/src/modules/doctors/services/index.ts index 90d63c4..8fde8aa 100644 --- a/backend/src/modules/doctors/services/index.ts +++ b/backend/src/modules/doctors/services/index.ts @@ -1,4 +1,6 @@ import { HttpError } from '../../../http-error'; +import { prisma } from '../../../db/prisma-client'; +import * as auditService from '../../audit/services'; import * as doctorsRepo from '../repositories'; import type { VerificationStatus } from '@prisma/client'; @@ -29,8 +31,30 @@ export async function assertOwnsDoctorProfile(userId: string, doctorProfileId: s * workflow is manual/admin-gated in v1"). No automated verification * pipeline exists or is planned for v1; this is intentionally just a * status flip an admin performs after some out-of-band check. + * + * Phase 8, issue #43: the flip and its AuditEvent (who, from, to) commit + * together. Setting the status it already has is not a transition — it + * returns the profile unchanged and records nothing. */ -export async function setVerificationStatus(doctorProfileId: string, status: VerificationStatus) { - await getDoctorProfileById(doctorProfileId); // 404s if missing - return doctorsRepo.updateVerificationStatus(doctorProfileId, status); +export async function setVerificationStatus( + actor: auditService.AuditActor, + doctorProfileId: string, + status: VerificationStatus, +) { + return prisma.$transaction(async (tx) => { + const current = await doctorsRepo.lockVerificationStatus(tx, doctorProfileId); + if (current === null) throw new HttpError(404, 'Doctor profile not found.'); + if (current === status) { + return (await doctorsRepo.findDoctorProfileById(doctorProfileId, tx))!; + } + const profile = await doctorsRepo.updateVerificationStatus(tx, doctorProfileId, status); + await auditService.record(tx, actor, { + entityType: 'DoctorProfile', + entityId: doctorProfileId, + action: 'doctor.verification_status_changed', + fromState: current, + toState: status, + }); + return profile; + }); } diff --git a/backend/src/modules/payments/controllers/index.ts b/backend/src/modules/payments/controllers/index.ts index ebe6a91..14e1a7f 100644 --- a/backend/src/modules/payments/controllers/index.ts +++ b/backend/src/modules/payments/controllers/index.ts @@ -3,6 +3,7 @@ import * as paymentsService from '../services'; import { linkWalletSchema, createPaymentIntentSchema } from '../validation'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; +import { auditActor } from '../../../middleware/request-id'; // BigInt doesn't serialize via JSON.stringify by default — every response // here that carries one converts to a string explicitly. @@ -57,18 +58,18 @@ export async function getIntent(req: Request, res: Response) { export async function fundIntent(req: Request, res: Response) { const id = getParam(req, 'id'); - const intent = await paymentsService.fundPaymentIntent(id); + const intent = await paymentsService.fundPaymentIntent(auditActor(req), id); res.status(200).json(serializeIntent(intent)); } export async function releaseIntent(req: Request, res: Response) { const id = getParam(req, 'id'); - const intent = await paymentsService.releasePaymentIntent(id); + const intent = await paymentsService.releasePaymentIntent(auditActor(req), id); res.status(200).json(serializeIntent(intent)); } export async function refundIntent(req: Request, res: Response) { const id = getParam(req, 'id'); - const intent = await paymentsService.refundPaymentIntent(id); + const intent = await paymentsService.refundPaymentIntent(auditActor(req), id); res.status(200).json(serializeIntent(intent)); } diff --git a/backend/src/modules/payments/repositories/index.ts b/backend/src/modules/payments/repositories/index.ts index 7958616..1e560f4 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -2,7 +2,7 @@ // Never store PHI here. Never call docforum-escrow directly from here — // go through services/. import { prisma } from '../../../db/prisma-client'; -import type { PaymentIntentStatus } from '@prisma/client'; +import type { PaymentIntentStatus, Prisma } from '@prisma/client'; /** Reads FacilityProfile directly — a Phase 1 core identity model, not * something the (still-placeholder) facilities module exclusively owns. */ @@ -36,13 +36,32 @@ export function findPaymentIntentById(id: string) { return prisma.paymentIntent.findUnique({ where: { id } }); } -export function markEscrowed(id: string, input: { escrowId: bigint; stellarTxHash: string; sorobanContractId: string }) { - return prisma.paymentIntent.update({ - where: { id }, +/** + * Status writes are conditional on the state the caller observed (issue + * #43): `created -> escrowed`, `escrowed -> released|refunded`. They run in + * the caller's transaction alongside the AuditEvent, and return null if the + * row was no longer in the expected state (a concurrent action won), so an + * audit event can never claim a transition that didn't happen. The + * check-then-act gap before the on-chain call is issue #24's to close. + */ +export async function markEscrowed( + tx: Prisma.TransactionClient, + id: string, + input: { escrowId: bigint; stellarTxHash: string; sorobanContractId: string }, +) { + const result = await tx.paymentIntent.updateMany({ + where: { id, status: 'created' }, data: { status: 'escrowed', ...input }, }); + return result.count === 1 ? tx.paymentIntent.findUniqueOrThrow({ where: { id } }) : null; } -export function markStatus(id: string, status: PaymentIntentStatus) { - return prisma.paymentIntent.update({ where: { id }, data: { status } }); +export async function markStatus( + tx: Prisma.TransactionClient, + id: string, + from: PaymentIntentStatus, + to: PaymentIntentStatus, +) { + const result = await tx.paymentIntent.updateMany({ where: { id, status: from }, data: { status: to } }); + return result.count === 1 ? tx.paymentIntent.findUniqueOrThrow({ where: { id } }) : null; } diff --git a/backend/src/modules/payments/services/escrow-sdk-loader.ts b/backend/src/modules/payments/services/escrow-sdk-loader.ts index 98eddce..06ab6c3 100644 --- a/backend/src/modules/payments/services/escrow-sdk-loader.ts +++ b/backend/src/modules/payments/services/escrow-sdk-loader.ts @@ -12,3 +12,12 @@ export function loadEscrowSdk(): Promise { modulePromise ??= import('@docforum/escrow-sdk'); return modulePromise; } + +/** Test seam (issue #43): integration tests that exercise the DB side of + * fund/release/refund substitute a fake SDK instead of calling testnet. + * `undefined` restores the real one. Never called by application code. */ +export function setEscrowSdkForTests(sdk: EscrowSdkModule | undefined): void { + modulePromise = sdk ? Promise.resolve(sdk) : undefined; +} + +export type { EscrowSdkModule }; diff --git a/backend/src/modules/payments/services/index.ts b/backend/src/modules/payments/services/index.ts index c36cc05..29e15ea 100644 --- a/backend/src/modules/payments/services/index.ts +++ b/backend/src/modules/payments/services/index.ts @@ -13,6 +13,14 @@ import { HttpError } from '../../../http-error'; import { config } from '../../../config'; import * as paymentsRepo from '../repositories'; import { loadEscrowSdk } from './escrow-sdk-loader'; +import { prisma } from '../../../db/prisma-client'; +import * as auditService from '../../audit/services'; +import type { PaymentIntent, PaymentIntentStatus, Prisma } from '@prisma/client'; + +// Test seam re-exported through the service surface (same convention as the +// auth module's setEmailSender) — see escrow-sdk-loader.ts. +export { setEscrowSdkForTests } from './escrow-sdk-loader'; +export type { EscrowSdkModule } from './escrow-sdk-loader'; async function requirePaymentsConfig() { const { platformPayerSecret, platformReleaserSecret, escrowContractId, escrowTokenId } = config.payments; @@ -69,8 +77,42 @@ export async function getPaymentIntent(id: string) { return intent; } +/** Commits a status change and its AuditEvent together (issue #43). If the + * row moved on concurrently, nothing is written and the caller gets 409. */ +async function commitTransition( + actor: auditService.AuditActor, + intent: PaymentIntent, + action: string, + to: PaymentIntentStatus, + write: (tx: Prisma.TransactionClient) => Promise, +): Promise { + return prisma.$transaction(async (tx) => { + const updated = await write(tx); + if (!updated) { + throw new HttpError(409, `PaymentIntent changed concurrently — it is no longer '${intent.status}'.`); + } + await auditService.record(tx, actor, { + entityType: 'PaymentIntent', + entityId: intent.id, + action, + fromState: intent.status, + toState: to, + // Public, non-identifying references only: the escrow is on a public + // ledger anyway. No patient data, no secrets. + metadata: { + orderType: updated.orderType, + amountStroops: updated.amountStroops.toString(), + escrowId: updated.escrowId?.toString() ?? null, + stellarTxHash: updated.stellarTxHash, + sorobanContractId: updated.sorobanContractId, + }, + }); + return updated; + }); +} + /** Funds the escrow from the platform's own payer identity. The intent must be `created` and its facility must have a linked wallet (the on-chain payee). */ -export async function fundPaymentIntent(id: string) { +export async function fundPaymentIntent(actor: auditService.AuditActor, id: string) { const intent = await getPaymentIntent(id); if (intent.status !== 'created') { throw new HttpError(409, `PaymentIntent is '${intent.status}', not 'created' — cannot fund.`); @@ -94,11 +136,13 @@ export async function fundPaymentIntent(id: string) { releaser: platformReleaser.publicKey(), }); - return paymentsRepo.markEscrowed(id, { escrowId, stellarTxHash: txHash, sorobanContractId: escrowContractId }); + return commitTransition(actor, intent, 'payment_intent.funded', 'escrowed', (tx) => + paymentsRepo.markEscrowed(tx, id, { escrowId, stellarTxHash: txHash, sorobanContractId: escrowContractId }), + ); } /** Releases the escrow to the facility's linked wallet. v1: admin-triggered (see ADR 0004 "Consequences") — no automatic FulfillmentRecord trigger yet, since that model doesn't exist. */ -export async function releasePaymentIntent(id: string) { +export async function releasePaymentIntent(actor: auditService.AuditActor, id: string) { const intent = await getPaymentIntent(id); if (intent.status !== 'escrowed' || intent.escrowId === null) { throw new HttpError(409, `PaymentIntent is '${intent.status}', not 'escrowed' — cannot release.`); @@ -109,11 +153,13 @@ export async function releasePaymentIntent(id: string) { await client.release({ escrowId: intent.escrowId, caller: platformReleaser }); - return paymentsRepo.markStatus(id, 'released'); + return commitTransition(actor, intent, 'payment_intent.released', 'released', (tx) => + paymentsRepo.markStatus(tx, id, 'escrowed', 'released'), + ); } /** Refunds the escrow back toward the platform payer. v1: admin-triggered, same reasoning as release. */ -export async function refundPaymentIntent(id: string) { +export async function refundPaymentIntent(actor: auditService.AuditActor, id: string) { const intent = await getPaymentIntent(id); if (intent.status !== 'escrowed' || intent.escrowId === null) { throw new HttpError(409, `PaymentIntent is '${intent.status}', not 'escrowed' — cannot refund.`); @@ -124,5 +170,7 @@ export async function refundPaymentIntent(id: string) { await client.refund({ escrowId: intent.escrowId, caller: platformReleaser }); - return paymentsRepo.markStatus(id, 'refunded'); + return commitTransition(actor, intent, 'payment_intent.refunded', 'refunded', (tx) => + paymentsRepo.markStatus(tx, id, 'escrowed', 'refunded'), + ); } diff --git a/backend/tests/integration/audit.test.mts b/backend/tests/integration/audit.test.mts new file mode 100644 index 0000000..159f39d --- /dev/null +++ b/backend/tests/integration/audit.test.mts @@ -0,0 +1,486 @@ +// Integration tests for the AuditEvent foundation (Phase 8, issue #43), +// against the real transitions — not the audit service in isolation: +// booking, doctor verification and payment fund/release/refund each emit an +// event with the right actor/entity/action/states/request id; the event +// commits with its change and rolls back with it (failure injected by a real +// database trigger, not a mock); a transition that fails or loses a race +// emits nothing; metadata carries no PHI or secrets; the table is +// append-only at the database level; and GET /admin/audit-events filters +// and pages deterministically. Self-contained, same embedded-Postgres +// pattern as booking.test.mts. Own port (5441). The escrow SDK is faked — +// on-chain behaviour is the testnet suite's job. +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 } from 'vitest'; +import request from 'supertest'; +import jwt from 'jsonwebtoken'; +import EmbeddedPostgres from 'embedded-postgres'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5441; +const runId = Date.now(); +const PAYER_SECRET = `SAUDITPAYERSENTINEL${runId}`; +const RELEASER_SECRET = `SAUDITRELEASERSENTINEL${runId}`; +const TX_HASH = 'c'.repeat(64); + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-audit-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; + +const emailedTokens = new Map(); +const sdkBehaviour = { failRelease: false, nextEscrowId: 77n }; + +describe('AuditEvent foundation', () => { + let app: App; + let prisma: Prisma; + let audit: typeof import('../../src/modules/audit/services/index.js'); + let admin: { id: string; token: string }; + let doctor: { userId: string; profileId: string; token: string }; + let patients: { userId: string; token: string; email: string }[]; + let facilityId: string; + let slots: string[]; + + async function signup(role: 'patient' | 'doctor', label: string) { + const email = `${label}-${runId}@example.com`; + const res = await request(app).post('/auth/signup').send({ role, email, password: 'longenough123', fullName: `${label} Fullname` }); + expect(res.status).toBe(200); + expect((await request(app).post('/auth/verify-email').send({ token: emailedTokens.get(email) })).status).toBe(200); + return { userId: res.body.user.id as string, token: res.body.accessToken as string, email }; + } + + /** A real DB-level failure in the audit insert, scoped to one entity type: + * proves the state change rolls back with its event. */ + async function failAuditInsertsFor(entityType: string) { + await prisma.$executeRawUnsafe(` + CREATE OR REPLACE FUNCTION test_fail_audit_insert() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN + IF NEW."entityType" = '${entityType}' THEN RAISE EXCEPTION 'injected audit failure'; END IF; + RETURN NEW; + END; $$`); + await prisma.$executeRawUnsafe( + `CREATE TRIGGER test_fail_audit_insert BEFORE INSERT ON "AuditEvent" FOR EACH ROW EXECUTE FUNCTION test_fail_audit_insert()`, + ); + return async () => { + await prisma.$executeRawUnsafe(`DROP TRIGGER test_fail_audit_insert ON "AuditEvent"`); + }; + } + + const adminAuth = () => ({ Authorization: `Bearer ${admin.token}` }); + const listEvents = (query: Record) => + request(app).get('/admin/audit-events').query(query).set(adminAuth()); + + async function createIntent() { + return prisma.paymentIntent.create({ + data: { orderType: 'lab_order', orderId: `order-${Math.random()}`, patientProfileId: (await prisma.patientProfile.findFirstOrThrow()).id, facilityId, amountStroops: 5000n }, + }); + } + + 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.PLATFORM_PAYER_SECRET = PAYER_SECRET; + process.env.PLATFORM_RELEASER_SECRET = RELEASER_SECRET; + process.env.ESCROW_CONTRACT_ID = 'CTESTCONTRACT'; + process.env.ESCROW_TOKEN_ID = 'CTESTTOKEN'; + + 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'); + const paymentsServices = await import('../../src/modules/payments/services/index.js'); + audit = await import('../../src/modules/audit/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])); + }); + + class FakeEscrowClient { + async createEscrow() { + const escrowId = sdkBehaviour.nextEscrowId; + sdkBehaviour.nextEscrowId += 1n; + return { escrowId, txHash: TX_HASH }; + } + async release() { + if (sdkBehaviour.failRelease) throw new Error('simulated on-chain failure'); + } + async refund() {} + } + paymentsServices.setEscrowSdkForTests({ + Keypair: { fromSecret: () => ({ publicKey: () => 'GPLATFORMRELEASERPUBLICKEY' }) }, + EscrowClient: FakeEscrowClient, + } as unknown as import('../../src/modules/payments/services/index.js').EscrowSdkModule); + + const adminUser = await prisma.user.create({ + data: { email: `admin-${runId}@example.com`, passwordHash: 'x', role: 'admin', emailVerifiedAt: new Date() }, + }); + admin = { id: adminUser.id, token: jwt.sign({ sub: adminUser.id, role: 'admin' }, 'test-access-secret', { expiresIn: '15m' }) }; + + const doctorUser = await signup('doctor', 'audit-doctor'); + doctor = { ...doctorUser, profileId: (await prisma.doctorProfile.findUniqueOrThrow({ where: { userId: doctorUser.userId } })).id }; + patients = await Promise.all(Array.from({ length: 5 }, (_, i) => signup('patient', `audit-patient-${i}`))); + + const tomorrow = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString().slice(0, 10); + await request(app) + .post(`/availability/doctors/${doctor.profileId}/slots/generate`) + .set('Authorization', `Bearer ${doctor.token}`) + .send({ date: tomorrow, startHour: 9, endHour: 12, slotLengthMinutes: 30 }); + slots = ((await request(app).get(`/availability/doctors/${doctor.profileId}/slots`)).body as { id: string }[]).map((s) => s.id); + + 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: 'Audit Lab', type: 'lab' } })).id; + await prisma.walletLink.create({ data: { facilityId, stellarPublicKey: `G${'A'.repeat(55)}` } }); + }, 180_000); + + afterAll(async () => { + const authServices = await import('../../src/modules/auth/services/index.js'); + const paymentsServices = await import('../../src/modules/payments/services/index.js'); + authServices.resetEmailSender(); + paymentsServices.setEscrowSdkForTests(undefined); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + describe('appointment booking', () => { + it('emits one event with actor, entity, states and the request id', async () => { + const before = new Date(); + const res = await request(app) + .post('/appointments') + .set('Authorization', `Bearer ${patients[0]!.token}`) + .set('X-Request-Id', `booking-req-${runId}`) + .send({ availabilitySlotId: slots[0] }); + expect(res.status).toBe(201); + expect(res.headers['x-request-id']).toBe(`booking-req-${runId}`); + + const events = await prisma.auditEvent.findMany({ where: { entityType: 'Appointment', entityId: res.body.id } }); + expect(events).toHaveLength(1); + const [event] = events; + expect(event).toMatchObject({ + actorUserId: patients[0]!.userId, + actorRole: 'patient', + action: 'appointment.booked', + fromState: null, + toState: 'scheduled', + requestId: `booking-req-${runId}`, + metadata: { availabilitySlotId: slots[0], doctorProfileId: doctor.profileId, origin: 'direct_booking' }, + }); + // The DB timestamp comes from the transaction; allow for clock skew + // between the test process and Postgres of a few seconds. + expect(Math.abs(event!.createdAt.getTime() - before.getTime())).toBeLessThan(5_000); + const serialized = JSON.stringify(event); + expect(serialized).not.toContain(patients[0]!.email); + expect(serialized).not.toContain('Fullname'); + }); + + it('mints a request id when none (or a malformed one) is sent', async () => { + const res = await request(app) + .post('/appointments') + .set('Authorization', `Bearer ${patients[1]!.token}`) + .set('X-Request-Id', 'bad id!') + .send({ availabilitySlotId: slots[1] }); + expect(res.status).toBe(201); + const event = await prisma.auditEvent.findFirstOrThrow({ where: { entityId: res.body.id } }); + expect(event.requestId).toMatch(/^[0-9a-f-]{36}$/); + expect(res.headers['x-request-id']).toBe(event.requestId); + }); + + it('records exactly one event when concurrent bookings race — losers record nothing', async () => { + const slot = slots[2]!; + const responses = await Promise.all( + patients.map((p) => request(app).post('/appointments').set('Authorization', `Bearer ${p.token}`).send({ availabilitySlotId: slot })), + ); + expect(responses.filter((r) => r.status === 201)).toHaveLength(1); + expect(responses.filter((r) => r.status === 409)).toHaveLength(patients.length - 1); + const appointment = await prisma.appointment.findUniqueOrThrow({ where: { availabilitySlotId: slot } }); + expect(await prisma.auditEvent.count({ where: { action: 'appointment.booked', metadata: { path: ['availabilitySlotId'], equals: slot } } })).toBe(1); + expect(await prisma.auditEvent.count({ where: { entityId: appointment.id } })).toBe(1); + }); + + it('rolls the booking back when its audit write fails — no orphan event, no orphan booking', async () => { + const slot = slots[3]!; + const restore = await failAuditInsertsFor('Appointment'); + try { + const res = await request(app).post('/appointments').set('Authorization', `Bearer ${patients[1]!.token}`).send({ availabilitySlotId: slot }); + expect(res.status).toBe(500); + } finally { + await restore(); + } + expect((await prisma.availabilitySlot.findUniqueOrThrow({ where: { id: slot } })).status).toBe('open'); + expect(await prisma.appointment.count({ where: { availabilitySlotId: slot } })).toBe(0); + // And with the failure removed, the same slot books normally. + const retry = await request(app).post('/appointments').set('Authorization', `Bearer ${patients[1]!.token}`).send({ availabilitySlotId: slot }); + expect(retry.status).toBe(201); + expect(await prisma.auditEvent.count({ where: { entityId: retry.body.id } })).toBe(1); + }); + }); + + describe('doctor verification', () => { + const patch = (status: string, requestIdHeader?: string) => { + const req = request(app).patch(`/doctors/${doctor.profileId}/verification`).set(adminAuth()); + if (requestIdHeader) req.set('X-Request-Id', requestIdHeader); + return req.send({ status }); + }; + + it('records who changed it and the real previous and new states', async () => { + expect((await patch('verified', `verify-req-${runId}`)).status).toBe(200); + expect((await patch('rejected')).status).toBe(200); + const events = await prisma.auditEvent.findMany({ + where: { entityType: 'DoctorProfile', entityId: doctor.profileId }, + orderBy: [{ createdAt: 'asc' }, { id: 'asc' }], + }); + expect(events.map((e) => [e.fromState, e.toState])).toEqual([ + ['pending', 'verified'], + ['verified', 'rejected'], + ]); + expect(events[0]).toMatchObject({ + actorUserId: admin.id, + actorRole: 'admin', + action: 'doctor.verification_status_changed', + requestId: `verify-req-${runId}`, + metadata: null, + }); + }); + + it('records nothing when the status is unchanged', async () => { + const before = await prisma.auditEvent.count({ where: { entityId: doctor.profileId } }); + expect((await patch('rejected')).status).toBe(200); + expect(await prisma.auditEvent.count({ where: { entityId: doctor.profileId } })).toBe(before); + }); + + it('rolls the status change back when its audit write fails', async () => { + const restore = await failAuditInsertsFor('DoctorProfile'); + try { + expect((await patch('verified')).status).toBe(500); + } finally { + await restore(); + } + expect((await prisma.doctorProfile.findUniqueOrThrow({ where: { id: doctor.profileId } })).verificationStatus).toBe('rejected'); + }); + }); + + describe('payment fund / release / refund', () => { + const post = (intentId: string, action: 'fund' | 'release' | 'refund') => + request(app).post(`/payments/intents/${intentId}/${action}`).set(adminAuth()).set('X-Request-Id', `pay-${action}-${runId}`); + + it('records created -> escrowed -> released with on-chain references and no secrets', async () => { + const intent = await createIntent(); + expect((await post(intent.id, 'fund')).status).toBe(200); + expect((await post(intent.id, 'release')).status).toBe(200); + const events = await prisma.auditEvent.findMany({ where: { entityId: intent.id }, orderBy: [{ createdAt: 'asc' }, { id: 'asc' }] }); + expect(events.map((e) => [e.action, e.fromState, e.toState])).toEqual([ + ['payment_intent.funded', 'created', 'escrowed'], + ['payment_intent.released', 'escrowed', 'released'], + ]); + expect(events[0]).toMatchObject({ + actorUserId: admin.id, + actorRole: 'admin', + requestId: `pay-fund-${runId}`, + metadata: { orderType: 'lab_order', amountStroops: '5000', stellarTxHash: TX_HASH, sorobanContractId: 'CTESTCONTRACT' }, + }); + expect((events[0]!.metadata as Record).escrowId).toMatch(/^\d+$/); + const serialized = JSON.stringify(events); + for (const secret of [PAYER_SECRET, RELEASER_SECRET, 'GPLATFORMRELEASERPUBLICKEY']) expect(serialized).not.toContain(secret); + }); + + it('records escrowed -> refunded', async () => { + const intent = await createIntent(); + await post(intent.id, 'fund'); + expect((await post(intent.id, 'refund')).status).toBe(200); + const refund = await prisma.auditEvent.findFirstOrThrow({ where: { entityId: intent.id, action: 'payment_intent.refunded' } }); + expect([refund.fromState, refund.toState]).toEqual(['escrowed', 'refunded']); + }); + + it('records nothing when the operation fails — no invented transition', async () => { + const intent = await createIntent(); + await post(intent.id, 'fund'); + sdkBehaviour.failRelease = true; + try { + expect((await post(intent.id, 'release')).status).toBe(500); + } finally { + sdkBehaviour.failRelease = false; + } + expect((await prisma.paymentIntent.findUniqueOrThrow({ where: { id: intent.id } })).status).toBe('escrowed'); + expect(await prisma.auditEvent.count({ where: { entityId: intent.id, action: 'payment_intent.released' } })).toBe(0); + // An invalid transition is refused before anything happens, too. + expect((await post(intent.id, 'fund')).status).toBe(409); + expect(await prisma.auditEvent.count({ where: { entityId: intent.id } })).toBe(1); + }); + + it('records one release when two race for the same intent', async () => { + const intent = await createIntent(); + await post(intent.id, 'fund'); + const [a, b] = await Promise.all([post(intent.id, 'release'), post(intent.id, 'refund')]); + expect([a.status, b.status].sort()).toEqual([200, 409]); + const finalStatus = (await prisma.paymentIntent.findUniqueOrThrow({ where: { id: intent.id } })).status; + const terminal = await prisma.auditEvent.findMany({ where: { entityId: intent.id, action: { in: ['payment_intent.released', 'payment_intent.refunded'] } } }); + expect(terminal).toHaveLength(1); + expect(terminal[0]!.toState).toBe(finalStatus); + }); + + it('rolls the status change back when its audit write fails', async () => { + const intent = await createIntent(); + const restore = await failAuditInsertsFor('PaymentIntent'); + try { + expect((await post(intent.id, 'fund')).status).toBe(500); + } finally { + await restore(); + } + expect((await prisma.paymentIntent.findUniqueOrThrow({ where: { id: intent.id } })).status).toBe('created'); + expect(await prisma.auditEvent.count({ where: { entityId: intent.id } })).toBe(0); + }); + }); + + describe('append-only', () => { + it('has no update or delete path in the audit module', async () => { + const repo = await import('../../src/modules/audit/repositories/index.js'); + expect(Object.keys(repo).sort()).toEqual(['findAuditEvents', 'insertAuditEvent']); + }); + + it('rejects UPDATE, DELETE and TRUNCATE at the database level', async () => { + const event = await prisma.auditEvent.findFirstOrThrow(); + const count = await prisma.auditEvent.count(); + await expect(prisma.auditEvent.update({ where: { id: event.id }, data: { action: 'tampered.action' } })).rejects.toThrow(/append-only/); + await expect(prisma.auditEvent.updateMany({ data: { toState: 'x' } })).rejects.toThrow(/append-only/); + await expect(prisma.auditEvent.delete({ where: { id: event.id } })).rejects.toThrow(/append-only/); + await expect(prisma.auditEvent.deleteMany()).rejects.toThrow(/append-only/); + await expect(prisma.$executeRawUnsafe('TRUNCATE "AuditEvent"')).rejects.toThrow(/append-only/); + expect(await prisma.auditEvent.count()).toBe(count); + expect(await prisma.auditEvent.findUniqueOrThrow({ where: { id: event.id } })).toEqual(event); + }); + + it('refuses to record outside a transaction', async () => { + await expect( + audit.record(prisma as never, audit.SYSTEM_ACTOR, { entityType: 'Appointment', entityId: 'x', action: 'appointment.booked', fromState: null, toState: 'scheduled' }), + ).rejects.toThrow(/inside the transaction/); + }); + }); + + describe('GET /admin/audit-events', () => { + it('is admin-only', async () => { + expect((await request(app).get('/admin/audit-events')).status).toBe(401); + expect((await request(app).get('/admin/audit-events').set('Authorization', `Bearer ${patients[0]!.token}`)).status).toBe(403); + expect((await request(app).get('/admin/audit-events').set('Authorization', `Bearer ${doctor.token}`)).status).toBe(403); + expect((await request(app).get('/admin/audit-events').set(adminAuth())).status).toBe(200); + }); + + it('filters by entity, actor and date range', async () => { + const appointments = await listEvents({ entityType: 'Appointment', limit: 200 }); + expect(appointments.status).toBe(200); + expect(appointments.body.items.length).toBeGreaterThan(0); + expect(appointments.body.items.every((e: { entityType: string }) => e.entityType === 'Appointment')).toBe(true); + + const one = await listEvents({ entityType: 'DoctorProfile', entityId: doctor.profileId }); + expect(one.body.items.map((e: { toState: string }) => e.toState)).toEqual(['rejected', 'verified']); + + const byAdmin = await listEvents({ actorUserId: admin.id, limit: 200 }); + expect(byAdmin.body.items.length).toBeGreaterThan(0); + expect(byAdmin.body.items.every((e: { actorUserId: string }) => e.actorUserId === admin.id)).toBe(true); + const byPatient = await listEvents({ actorUserId: patients[0]!.userId }); + expect(byPatient.body.items.every((e: { actorUserId: string }) => e.actorUserId === patients[0]!.userId)).toBe(true); + + const all = (await listEvents({ limit: 200 })).body.items as { createdAt: string }[]; + const pivot = all[Math.floor(all.length / 2)]!.createdAt; + const after = (await listEvents({ from: pivot, limit: 200 })).body.items as { createdAt: string }[]; + const before = (await listEvents({ to: pivot, limit: 200 })).body.items as { createdAt: string }[]; + expect(after.every((e) => e.createdAt >= pivot)).toBe(true); + expect(before.every((e) => e.createdAt <= pivot)).toBe(true); + expect(new Set([...after, ...before].map((e) => JSON.stringify(e))).size).toBe(all.length); + expect((await listEvents({ from: '2999-01-01' })).body).toEqual({ items: [], nextCursor: null }); + }); + + it('validates the query', async () => { + const invalid: Record[] = [ + { limit: 0 }, + { limit: 500 }, + { from: 'not-a-date' }, + { unknown: 'x' }, + { entityId: 'a' }, + { cursor: 'garbage' }, + { from: '2026-10-17', to: '2026-10-01' }, + ]; + for (const query of invalid) { + const res = await listEvents(query); + expect(res.status, JSON.stringify(query)).toBe(400); + } + }); + + it('pages with a stable cursor: every event once, in a total order, even with identical timestamps', async () => { + // 23 events with one identical createdAt (Prisma stamps createdAt per + // row on the client, so real same-millisecond writes collide like + // this) — the cursor must tie-break on id. Inserted directly: INSERT + // is the one operation the append-only trigger allows. + const entityId = `bulk-${runId}`; + const sameInstant = new Date('2026-01-01T00:00:00.000Z'); + await prisma.auditEvent.createMany({ + data: Array.from({ length: 23 }, (_, i) => ({ + entityType: 'PaymentIntent', + entityId, + action: 'payment_intent.funded', + fromState: 'created', + toState: 'escrowed', + metadata: { seq: i }, + createdAt: sameInstant, + })), + }); + const rows = await prisma.auditEvent.findMany({ where: { entityId } }); + expect(new Set(rows.map((r) => r.createdAt.getTime())).size).toBe(1); + + const collect = async () => { + const seen: { id: string; createdAt: string }[] = []; + let cursor: string | null = null; + let pages = 0; + do { + const res = await listEvents({ entityType: 'PaymentIntent', entityId, limit: 5, ...(cursor ? { cursor } : {}) }); + expect(res.status).toBe(200); + expect(res.body.items.length).toBeLessThanOrEqual(5); + seen.push(...res.body.items); + cursor = res.body.nextCursor; + pages += 1; + // A newer event arriving mid-scan must not disturb the remaining pages. + if (pages === 2) { + await prisma.$transaction((tx) => audit.record(tx, audit.SYSTEM_ACTOR, { entityType: 'PaymentIntent', entityId, action: 'payment_intent.released', fromState: 'escrowed', toState: 'released' })); + } + } while (cursor); + return { seen, pages }; + }; + + const { seen, pages } = await collect(); + expect(pages).toBe(5); + expect(seen).toHaveLength(23); + expect(new Set(seen.map((e) => e.id)).size).toBe(23); + const expected = rows.map((r) => r.id).sort().reverse(); + expect(seen.map((e) => e.id)).toEqual(expected); + + // Deterministic: a second full scan sees the same 23 (plus the newer one, first). + const again = await collect(); + expect(again.seen.slice(1).map((e) => e.id)).toEqual(expected); + expect(again.seen[0]!.id).not.toBe(expected[0]); + }); + }); + + it('keeps the existing booking guarantee: every booked slot has exactly one appointment', async () => { + const booked = await prisma.availabilitySlot.findMany({ where: { doctorProfileId: doctor.profileId, status: 'booked' } }); + for (const slot of booked) expect(await prisma.appointment.count({ where: { availabilitySlotId: slot.id } })).toBe(1); + }); +}); diff --git a/backend/tests/testnet-integration/payments.test.mts b/backend/tests/testnet-integration/payments.test.mts index 0a1c1a8..3ae83ae 100644 --- a/backend/tests/testnet-integration/payments.test.mts +++ b/backend/tests/testnet-integration/payments.test.mts @@ -43,6 +43,11 @@ const pg = new EmbeddedPostgres({ }); const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_payments_test`; +// Phase 8, issue #43: fund/release/refund now take the audit actor. This +// suite calls the services directly (no HTTP request), so it acts as a +// system actor. +const TEST_ACTOR = { userId: null, role: null, requestId: `testnet-${runId}` }; + async function fundedKeypair(): Promise { const kp = Keypair.random(); const res = await fetch(`https://friendbot.stellar.org?addr=${encodeURIComponent(kp.publicKey())}`); @@ -113,12 +118,12 @@ describe('payments module against live testnet', () => { }); expect(intent.status).toBe('created'); - const funded = await paymentsService.fundPaymentIntent(intent.id); + const funded = await paymentsService.fundPaymentIntent(TEST_ACTOR, intent.id); expect(funded.status).toBe('escrowed'); expect(funded.escrowId).not.toBeNull(); expect(funded.stellarTxHash).toBeTruthy(); - const released = await paymentsService.releasePaymentIntent(intent.id); + const released = await paymentsService.releasePaymentIntent(TEST_ACTOR, intent.id); expect(released.status).toBe('released'); }, 60_000); @@ -131,8 +136,8 @@ describe('payments module against live testnet', () => { amountStroops: 500_000n, }); - await paymentsService.fundPaymentIntent(intent.id); - const refunded = await paymentsService.refundPaymentIntent(intent.id); + await paymentsService.fundPaymentIntent(TEST_ACTOR, intent.id); + const refunded = await paymentsService.refundPaymentIntent(TEST_ACTOR, intent.id); expect(refunded.status).toBe('refunded'); }, 60_000); @@ -152,6 +157,6 @@ describe('payments module against live testnet', () => { amountStroops: 500_000n, }); - await expect(paymentsService.fundPaymentIntent(intent.id)).rejects.toThrow(/no linked wallet/); + await expect(paymentsService.fundPaymentIntent(TEST_ACTOR, intent.id)).rejects.toThrow(/no linked wallet/); }, 30_000); }); diff --git a/backend/tests/unit/audit.test.ts b/backend/tests/unit/audit.test.ts new file mode 100644 index 0000000..e3e05c7 --- /dev/null +++ b/backend/tests/unit/audit.test.ts @@ -0,0 +1,110 @@ +// AuditEvent foundation (Phase 8, issue #43): the pure parts — metadata +// validation (no PHI/secrets), the keyset cursor, query validation, the +// transaction-only guard on record(), and the request-id middleware. +// Database-backed behaviour is in tests/integration/audit.test.mts. +import { describe, expect, it } from 'vitest'; +import type { Prisma } from '@prisma/client'; +import { + AuditMetadataError, + SYSTEM_ACTOR, + decodeAuditCursor, + encodeAuditCursor, + record, + validateAuditMetadata, +} from '../../src/modules/audit/services'; +import { listAuditEventsQuerySchema } from '../../src/modules/audit/validation'; +import { requestId } from '../../src/middleware/request-id'; + +describe('validateAuditMetadata', () => { + it('accepts a small flat map of ids and codes', () => { + const metadata = { availabilitySlotId: 's1', doctorProfileId: 'd1', origin: 'direct_booking', amountStroops: '100', escrowId: null, retried: false, attempt: 2 }; + expect(validateAuditMetadata(metadata)).toEqual(metadata); + expect(validateAuditMetadata(undefined)).toBeUndefined(); + }); + + it.each(['email', 'fullName', 'patientName', 'phone', 'dateOfBirth', 'dob', 'notes', 'intakeText', 'symptoms', 'password', 'secret', 'releaserSecret', 'refreshToken', 'apiKey', 'requestBody', 'payload'])( + 'refuses a key that could carry personal or secret data: %s', + (key) => { + expect(() => validateAuditMetadata({ [key]: 'x' })).toThrow(AuditMetadataError); + }, + ); + + it('refuses nested objects, arrays, long strings, non-finite numbers and too many keys', () => { + expect(() => validateAuditMetadata({ nested: { a: 1 } } as never)).toThrow(AuditMetadataError); + expect(() => validateAuditMetadata({ list: [1, 2] } as never)).toThrow(AuditMetadataError); + expect(() => validateAuditMetadata({ reason: 'x'.repeat(201) })).toThrow(AuditMetadataError); + expect(() => validateAuditMetadata({ n: Number.NaN })).toThrow(AuditMetadataError); + expect(() => validateAuditMetadata(Object.fromEntries(Array.from({ length: 17 }, (_, i) => [`k${i}`, i])))).toThrow(AuditMetadataError); + expect(() => validateAuditMetadata({ 'bad key': 1 })).toThrow(AuditMetadataError); + }); +}); + +describe('record()', () => { + it('refuses the root client — it must run inside the transaction of the change', async () => { + const rootLike = { $transaction: () => undefined, auditEvent: { create: () => undefined } } as unknown as Prisma.TransactionClient; + await expect( + record(rootLike, SYSTEM_ACTOR, { entityType: 'Appointment', entityId: 'a', action: 'appointment.booked', fromState: null, toState: 'scheduled' }), + ).rejects.toThrow(/inside the transaction/); + }); + + it('refuses a malformed action name before writing anything', async () => { + let wrote = false; + const tx = { auditEvent: { create: async () => { wrote = true; } } } as unknown as Prisma.TransactionClient; + await expect( + record(tx, SYSTEM_ACTOR, { entityType: 'Appointment', entityId: 'a', action: 'Booked!', fromState: null, toState: null }), + ).rejects.toThrow(/Invalid audit action/); + expect(wrote).toBe(false); + }); +}); + +describe('audit cursor', () => { + it('round-trips (createdAt, id) and rejects garbage', () => { + const position = { createdAt: new Date('2026-10-17T10:00:00.123Z'), id: 'abc-123' }; + expect(decodeAuditCursor(encodeAuditCursor(position))).toEqual(position); + for (const bad of ['', 'not-base64-!!', Buffer.from('nodate|id').toString('base64url'), Buffer.from('2026-10-17T10:00:00Z|').toString('base64url')]) { + expect(() => decodeAuditCursor(bad)).toThrow(/Invalid cursor/); + } + }); +}); + +describe('listAuditEventsQuerySchema', () => { + it('defaults the limit and coerces it from the query string', () => { + expect(listAuditEventsQuerySchema.parse({}).limit).toBe(50); + expect(listAuditEventsQuerySchema.parse({ limit: '10' }).limit).toBe(10); + }); + + it('bounds the page size', () => { + expect(listAuditEventsQuerySchema.safeParse({ limit: '0' }).success).toBe(false); + expect(listAuditEventsQuerySchema.safeParse({ limit: '201' }).success).toBe(false); + }); + + it('rejects unknown parameters, bad dates and entityId without entityType', () => { + expect(listAuditEventsQuerySchema.safeParse({ patientEmail: 'x' }).success).toBe(false); + expect(listAuditEventsQuerySchema.safeParse({ from: 'yesterday' }).success).toBe(false); + expect(listAuditEventsQuerySchema.safeParse({ entityId: 'a1' }).success).toBe(false); + expect(listAuditEventsQuerySchema.parse({ from: '2026-10-01', to: '2026-10-17T12:00:00Z' }).to).toEqual(new Date('2026-10-17T12:00:00Z')); + }); +}); + +describe('requestId middleware', () => { + function run(inbound?: string) { + const headers: Record = {}; + const req = { get: (name: string) => (name.toLowerCase() === 'x-request-id' ? inbound : undefined) } as never as { requestId?: string; get(n: string): string | undefined }; + const res = { set: (name: string, value: string) => { headers[name] = value; } }; + let called = false; + requestId(req as never, res as never, () => { called = true; }); + return { id: req.requestId!, header: headers['X-Request-Id'], called }; + } + + it('keeps a well-formed inbound id and echoes it', () => { + expect(run('render-abc12345')).toEqual({ id: 'render-abc12345', header: 'render-abc12345', called: true }); + }); + + it('replaces a missing, short, oversized or unsafe id with a fresh UUID', () => { + for (const inbound of [undefined, 'short', 'x'.repeat(129), 'has spaces in it', '']) { + const { id, header } = run(inbound); + expect(id).toMatch(/^[0-9a-f-]{36}$/); + expect(header).toBe(id); + } + }); +}); diff --git a/docs/api/README.md b/docs/api/README.md index ee29323..51399a5 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -112,8 +112,8 @@ account also pays bcrypt's comparison cost, so timing doesn't leak either). user is stuck behind: body `{ email?, ip? }` (at least one), returns `{ cleared }`. Clear **both** when a lock was taken out via the account's IP, or the IP bucket will keep refusing them. It's logged to the server console -— there is no `AuditEvent` table yet (known gap), so that's the interim -record. +— it isn't wired into `AuditEvent` yet (issue #43 covered booking, doctor +verification and payments), so that's the interim record. Under the hood this uses Express's `trust proxy` setting via the `TRUST_PROXY` env var: `1` behind Render's single proxy hop, `false` (the @@ -151,6 +151,34 @@ drift apart. See `docs/adr/0003` for why production needs `SameSite=None`. | `POST /appointments` | patient | `{ availabilitySlotId }` | **The concurrency-critical endpoint** — see `ARCHITECTURE.md` §5.1 and `backend/tests/integration/booking.test.mts`. `409` if the slot is no longer open. | | `GET /appointments/mine` | patient | — | Caller's own appointments, most recent first. | +## Audit trail (`/admin/audit-events`) — `backend/src/modules/audit` + +Phase 8, issue #43. Every response carries an `X-Request-Id` header: a +well-formed inbound `X-Request-Id` (8–128 chars of `[A-Za-z0-9._:-]`) is +kept, anything else is replaced with a UUID. Audit events store it, so an +event can be traced to the request (and logs) that caused it. + +| Method & path | Auth | Notes | +|---|---|---| +| `GET /admin/audit-events` | admin | Newest first. Query: `entityType`, `entityId` (requires `entityType`), `actorUserId`, `from` / `to` (ISO-8601 date or date-time, inclusive), `limit` (1–200, default 50), `cursor`. Unknown parameters are `400`. Returns `{ items, nextCursor }`; pass `nextCursor` back as `cursor` for the next page (`null` = last page). | + +Item shape: `{ id, actorUserId, actorRole, entityType, entityId, action, +fromState, toState, requestId, metadata, createdAt }`. `metadata` is a small +flat map of ids/codes (never personal data). Ordering is +`(createdAt DESC, id DESC)` and the cursor is a keyset position on that +pair, so paging never skips or repeats an event; events written after a scan +started appear at the front of a new scan, not mid-way through the old one. +`400 INVALID_CURSOR` for a malformed cursor; `400` if `from` is after `to`. + +Recorded today: `appointment.booked`, `doctor.verification_status_changed`, +`payment_intent.{funded,released,refunded}` — see ARCHITECTURE.md §4.10. +There is no write or delete endpoint, and the table rejects UPDATE/DELETE +in the database. + +Payment actions (`POST /payments/intents/:id/{fund,release,refund}`) now +return `409` when the intent changed state concurrently between the check +and the write (previously the second write silently overwrote the first). + ## Not yet implemented Everything else in `ARCHITECTURE.md` §3's module list (`patients`,