Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,46 @@ Bundle format (schemaVersion 1): `{ format: "docforum.patient-export",
schemaVersion, exportId, generatedAt, sections: { <name>: { version, items } } }`
— see §3.1 for where sections come from.

### 4.12 Idempotency keys (Phase 8, issue #48)
```
IdempotencyKey
id, user_id, key, method, route, request_hash
response_status (nullable int), response_body (jsonb, nullable),
response_headers (jsonb, nullable — `Location` only, never Set-Cookie)
created_at, expires_at
# unique (user_id, key) — this constraint IS the concurrency mechanism, the
same way §5.1's conditional slot claim is. Whichever request wins the
INSERT owns the attempt; every other one reads the row and either replays
the stored response, is told one is in flight, or is refused for reusing
the key on a different request. There is no in-memory lock anywhere, so
this behaves identically on one instance and on ten.
# Scoped per user on purpose: an Idempotency-Key is a guessable string, so
it must never be usable as a bearer credential for another account's
stored body. Every read filters on user_id.
# route is the route *pattern* ('/payments/intents/:id/fund'), request_hash
is SHA-256 over method + concrete path + canonical JSON body. The concrete
path is in the hash so one key aimed at two different :ids is a mismatch,
not a replay of the first record's response.
# response_* are NULL while the attempt is in flight. That gap is what a
concurrent duplicate sees, and what it waits on.
# NOT an AuditEvent: a 5xx deletes the row outright (a server error is not
a result — the client retries with the same key and it runs again), and an
attempt abandoned by a crashed process is taken over once it is older than
IDEMPOTENCY_IN_PROGRESS_TIMEOUT_SECONDS. A takeover is only ever by an
identical request, so a stale row can't be re-pointed at a new operation.
# Retained for IDEMPOTENCY_RECORD_TTL_HOURS (default 24) and pruned by an
interval in src/server.ts. Reads filter on expires_at first, so a missed
prune costs storage, not correctness.
# Holds a copy of a response the requesting user already received. Not
patient data today, and deliberately not exported or listed — see §6.2.6
for the condition that would change that.
```
Opted into by middleware (`src/middleware/idempotency.ts`), per route, not
globally: `POST /appointments`, `POST /payments/intents`, and
`POST /payments/intents/:id/{fund,release,refund}`. The header is optional
(a request without one passes through untouched, so existing clients keep
working), but a client that retries should always send one.

## 5. Key flows (sequence-level, not code)

### 5.1 Booking (concurrency-safe)
Expand Down Expand Up @@ -327,6 +367,7 @@ schemaVersion, exportId, generatedAt, sections: { <name>: { version, items } } }
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. **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. **Added 2026-10-19 (issue #48):** an `IdempotencyKey` row caches a *copy of a response body* for 24 hours (§4.12). Today every opted-in route returns non-PHI data (an appointment id and status, a payment intent), so this is transport bookkeeping and not a patient-data surface — it is not part of the patient-export bundle (§3.1) and no endpoint lists it. **The condition that would change that:** if a route returning clinical free text (consultation notes, intake payloads, lab results) ever opts in, each row becomes a temporary PHI copy subject to right-of-access and retention, and the export registry in §3.1 has to gain a section for it before that route is allowed to opt in. Until then, adding a route to the opted-in list is a deliberate decision, not a default.

### 6.3 What's overengineered (architecture-level, for v1)?
1. Polymorphic fulfillment design (§4.7) may itself be premature abstraction — two small, boring, separate tables might beat one clever polymorphic one at this stage. Flagged as a design smell to revisit, not shipped as-is.
Expand All @@ -338,6 +379,7 @@ schemaVersion, exportId, generatedAt, sections: { <name>: { version, items } } }

- **Testing**: unit tests per module (`backend/tests/unit`), integration tests for booking/referral/order flows specifically because those are the correctness-critical paths (`backend/tests/integration`).
- **Security — login throttling (Phase 8, issue #35)**: `POST /auth/login` is throttled per normalised email *and* per client IP, in Postgres (`LoginAttempt`, §4.9): exponential backoff with `Retry-After` from `LOGIN_THROTTLE_BACKOFF_AFTER` failures, a temporary lock at `LOGIN_THROTTLE_LOCK_AFTER`, and a cleanup interval in `src/server.ts`. Two things this depends on operationally: (1) `TRUST_PROXY` must be set correctly in production — Render terminates TLS in front of exactly one hop, so `TRUST_PROXY=1`; leaving it unset makes every request look like Render's proxy (one shared bucket for everyone), and `true` lets a client spoof `X-Forwarded-For` for a fresh bucket per request; (2) the throttle deliberately does not reveal whether an account exists — locked, backoff and unknown-account responses are identical, and an absent account still pays the bcrypt comparison cost so response timing doesn't leak either.
- **Idempotent retries (Phase 8, issue #48)**: `POST /appointments`, `POST /payments/intents` and `POST /payments/intents/:id/{fund,release,refund}` honour an `Idempotency-Key` header (`src/middleware/idempotency.ts`). A repeat of the same key with the same request body inside 24 h replays the stored status and body instead of running the mutation again; the same key with a different body is `422`; a second request arriving while the first is still running is `409 IN_PROGRESS` with a `Retry-After`, because ownership is decided by the unique constraint on `(userId, key)` rather than by an in-memory lock that would be correct on one instance and broken on two. Keys are namespaced per user, and 5xx is never cached so a client can retry a failed call with the same key. See §4.12 for the model.
- **Migrations**: Prisma migrations are the only way schema changes happen — no manual DB edits, ever.
- **Environments**: local (Docker Compose), staging, production — staging/production infra choice is an open item (see ROADMAP).

Expand Down
2 changes: 2 additions & 0 deletions ARCHITECTURE_ESSENTIALS.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc
`User` → `EmailVerificationToken` (Phase 8, issue #34 — hashed/expiring, single-use; `User.emailVerifiedAt` gates booking + payment mutations, never login/browsing).
`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).
`User` → `IdempotencyKey` (Phase 8, issue #48 — `Idempotency-Key` replay protection for booking + payment mutations; the unique `(userId, key)` index *is* the lock, so there is no in-memory lock; middleware in `src/middleware/idempotency.ts`, per-route opt-in, see §4.12).
`FacilityProfile` → `WalletLink` (payout address) ; `PatientProfile`/`FacilityProfile` → `PaymentIntent` (opaque `orderType`/`orderId` — see docs/adr/0004)

## Payments (Phase 5.5 — see docs/adr/0004-custodial-payments-v1.md)
Expand All @@ -42,6 +43,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.
- `IdempotencyKey.response_body` (Phase 8, issue #48 — ARCHITECTURE.md §4.12) is a temporary copy of a response, kept 24 h. Non-PHI today; **a route that returns clinical free text must not opt in** until the export registry (ARCHITECTURE.md §3.1) can account for it.
- `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.
Expand Down
26 changes: 24 additions & 2 deletions ROADMAP.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# ROADMAP.md — DocForum

Last updated: 2026-10-17
Last updated: 2026-10-19
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
Expand Down Expand Up @@ -107,14 +107,15 @@ for status and issue links.
- [ ] ~~Shared: auth screens (signup/login per role).~~ → already built, `docforum-web` Phase W1.

## Phase 8 — Hardening (safety, audit, security)
**Status: In progress. In addition to the original five items (#18–#20 scoped, two still blocked), four auth-hardening items are scoped as #34, #35, #37 and #39 — #34 and #35 are merged; #37 and #39 are open PRs. An access-control test suite (#44) is built and tested. Each is independent of the others and of Phases 3–7, and each carries its own schema/doc updates.**
**Status: In progress. In addition to the original five items (#18–#20 scoped, two still blocked), four auth-hardening items are scoped as #34, #35, #37 and #39 — #34 and #35 are merged; #37 and #39 are open PRs. An access-control test suite (#44) is built and tested, and `Idempotency-Key` replay protection (#48) is in review. Each is independent of the others and of Phases 3–7, and each carries its own schema/doc updates.**

- [x] Email verification on signup, with restricted access for unverified accounts. `User.emailVerifiedAt` + `EmailVerificationToken` (hashed, expiring, single-use), `POST /auth/verify-email`, `POST /auth/verify-email/resend`, and a `requireVerifiedEmail` guard on booking + payment mutations (never on login/browsing). Migration backfills existing rows as verified. Tracked as [issue #34](https://github.com/DocForum/docforum-core/issues/34) (High, 200 pts) — **Done**, merged as PR #137. Found a real dependency while building it: the email *provider* is still an open decision (Phase 6, issue #17), so links can't be delivered on a hosted deployment yet — see `backend/src/modules/auth/services/email-dispatcher.ts` and ARCHITECTURE.md §6.1.5.
- [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**.
- [ ] 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, 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`.
- [ ] Idempotency-Key support for booking and payment mutations. `POST /appointments`, `POST /payments/intents` and `POST /payments/intents/:id/{fund,release,refund}` honour an `Idempotency-Key` header: a repeat within 24 h replays the stored response instead of running the mutation again, the same key with a different body/route/target is `422`, and a second request arriving while the first is still running is `409 IN_PROGRESS` + `Retry-After`. Concurrency is decided by a unique `(userId, key)` constraint on a new `IdempotencyKey` table (ARCHITECTURE.md §4.12) — deliberately not an in-memory lock, which would be correct on one instance and silently broken on two. Keys are namespaced per user (one account can never replay or read another's stored body), 5xx is never cached so a client can retry the same key, and a crashed attempt is taken over by an identical retry after `IDEMPOTENCY_IN_PROGRESS_TIMEOUT_SECONDS`. Tracked as [issue #48](https://github.com/DocForum/docforum-core/issues/48) (High, 200 pts) — **in review**, branch `feat/idempotency-key-support-for-booking-and-payment`.
- [ ] 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**.
Expand Down Expand Up @@ -405,3 +406,24 @@ for status and issue links.
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.
- 2026-10-19 — Issue #48 (`Idempotency-Key` for booking and payment
mutations) opened as a PR: an `IdempotencyKey` table whose unique
`(userId, key)` index *is* the concurrency mechanism — the winner of the
INSERT runs the handler, everyone else reads the row and replays, waits or
is refused. Deliberately not an in-memory lock (correct on one instance,
silently broken on two, and emptied by the restart a retry storm follows),
which makes this the §5.1 booking guarantee applied one layer up. Opted in
per route on `POST /appointments`, `POST /payments/intents` and
`POST /payments/intents/:id/{fund,release,refund}`; the header stays
optional so no existing client breaks. A retry within 24 h replays the
stored response (proven for payments by counting escrow-SDK calls: a retried
fund makes exactly one chain call), the same key with a different body,
route or target `:id` is `422`, and a request that collides with one still
in flight is `409 IN_PROGRESS` + `Retry-After` — proven both deterministically
(handler held open by a gate) and under a genuine 8-way concurrent burst.
Keys are scoped per user so one account can never replay or read another's
stored body; 5xx is never cached so a client can retry the same key and have
it actually run; an attempt left unfinished by a crashed process is taken over
by an *identical* retry only. Recorded as a new hard question in
ARCHITECTURE.md §6.2.6: a route returning clinical free text must not opt in
until the export registry can account for the cached copy.
Loading
Loading