From ebbceadfee2c9b629cf44dacf3b21865a82992ce Mon Sep 17 00:00:00 2001 From: affy-exe Date: Fri, 16 Oct 2026 14:23:45 +0000 Subject: [PATCH 1/2] feat(payments): SEP-10 Stellar Web Authentication server: challenge and verify endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds generic SEP-10 infrastructure (ADR 0005 step 4, issue #77): before a patient or facility wallet can be trusted as payer or payee, the backend must prove the caller controls that Stellar address. - GET /payments/sep10/challenge?account=Gโ€ฆ โ€” authenticated + verified email; full strkey checksum validation; builds a server-signed challenge via @stellar/stellar-sdk WebAuth helpers (Manage Data nonce, 5-minute time bounds, home domain + web_auth_domain) and records the issuance binding (userId, account, nonce, expiresAt). - POST /payments/sep10/verify โ€” walks the SEP-10 server verification list (structure, sequence 0, server source, time bounds, operation shape, server signature), then client signatures: multisig weight thresholds loaded from Horizon via verifyChallengeTxThreshold, or the spec's master-key rule for unfunded accounts via verifyChallengeTxSigners. Bound to the authenticated user and account; nonce consumed with ONE conditional UPDATE so concurrent replays can't both succeed. - New Sep10Challenge model (unique nonce, userId, account, expiresAt, consumedAt, createdAt, index on expiresAt) + migration. - Dedicated SEP10_SIGNING_SECRET config, separate from payer/releaser keys, validated at startup (names vars, never secrets), documented in .env.example. - Horizon access sits behind an injectable seam, so 15 unit + 18 integration tests cover happy path, replay, concurrency, expiry, wrong signer, multisig thresholds, unfunded accounts, user/account binding and tampering with zero network. Docs: ARCHITECTURE.md ยง4.10, ARCHITECTURE_ESSENTIALS.md data-model map, ROADMAP.md Phase 5.5 + changelog, docs/api/README.md endpoint reference. Closes #77 ๐Ÿค– Generated with Codebuff Co-Authored-By: Codebuff --- ARCHITECTURE.md | 30 ++ ARCHITECTURE_ESSENTIALS.md | 2 + ROADMAP.md | 24 +- backend/.env.example | 22 + backend/package-lock.json | 21 +- backend/package.json | 1 + .../migration.sql | 22 + backend/prisma/schema.prisma | 35 ++ backend/src/config/index.ts | 33 ++ .../src/modules/payments/controllers/index.ts | 3 + .../src/modules/payments/controllers/sep10.ts | 22 + .../modules/payments/repositories/index.ts | 26 + backend/src/modules/payments/routes/index.ts | 21 + .../src/modules/payments/services/index.ts | 13 + .../src/modules/payments/services/sep10.ts | 384 ++++++++++++++ .../payments/services/stellar-sdk-loader.ts | 14 + backend/src/modules/payments/validation.ts | 17 + backend/src/server.ts | 19 +- backend/tests/integration/sep10.test.mts | 502 ++++++++++++++++++ backend/tests/unit/sep10.test.mts | 311 +++++++++++ backend/vitest.config.mts | 6 + docs/api/README.md | 71 ++- 22 files changed, 1570 insertions(+), 29 deletions(-) create mode 100644 backend/prisma/migrations/20261016130509_add_sep10_challenges/migration.sql create mode 100644 backend/src/modules/payments/controllers/sep10.ts create mode 100644 backend/src/modules/payments/services/sep10.ts create mode 100644 backend/src/modules/payments/services/stellar-sdk-loader.ts create mode 100644 backend/tests/integration/sep10.test.mts create mode 100644 backend/tests/unit/sep10.test.mts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8fb9ba9..2883ab5 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -197,6 +197,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 SEP-10 wallet-ownership proofs (Phase 5.5, issue #77) +``` +Sep10Challenge + id, user_id (FK -> User), account (the Gโ€ฆ account the challenge was + issued for), nonce (unique โ€” the 48-random-bytes base64 value in the + challenge's " auth" Manage Data op), expires_at, + consumed_at (nullable), created_at + # One row per issued SEP-10 challenge (GET /payments/sep10/challenge); + redeemed exactly once by POST /payments/sep10/verify. Consumption is + a single conditional UPDATE (consumed_at IS NULL AND expires_at > + now), so concurrent replays can't both succeed โ€” the same DB-level + shape as ยง5.1's booking guarantee and EmailVerificationToken + redemption (ยง4.1). + # Deliberately no XDR/signature/PHI columns: the challenge is + re-derived from the submitted XDR at verify time; this ledger only + proves "we issued this nonce to this user for this account, once, + within its window". Bound to the issuing user at issue time, so a + signed challenge is never redeemable by another user or for another + account. + # Backs ADR 0005 step 4 (prove a patient/facility controls a Stellar + address before it is trusted as payer/payee). The SEP-10 server + signing key is its own identity (SEP10_SIGNING_SECRET), never the + payer/releaser keys. Owns no new endpoint semantics beyond + /payments/sep10/* โ€” see docs/api/README.md. +``` +(`PaymentIntent` / `WalletLink` from Phase 5.5 are documented in +`docs/adr/0004-custodial-payments-v1.md` rather than this section; the +SEP-10 ledger is the first payments model added directly here because it +is a user-identity-adjacent table with a FK to `User`.) + ## 5. Key flows (sequence-level, not code) ### 5.1 Booking (concurrency-safe) diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index a5393e8..f234540 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -28,12 +28,14 @@ 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). `FacilityProfile` โ†’ `WalletLink` (payout address) ; `PatientProfile`/`FacilityProfile` โ†’ `PaymentIntent` (opaque `orderType`/`orderId` โ€” see docs/adr/0004) +`User` โ†’ `Sep10Challenge` (SEP-10 wallet-ownership proofs, issue #77 โ€” single-use nonce binding a signed challenge to the issuing user + Stellar account; consumed atomically) ## Payments (Phase 5.5 โ€” see docs/adr/0004-custodial-payments-v1.md) - **Custodial v1**: `docforum-core` holds its own Stellar payer + releaser identities (`PLATFORM_PAYER_SECRET`/`PLATFORM_RELEASER_SECRET` env vars). Patients never hold or sign with a wallet โ€” the only reason this is possible without violating `docforum-web`'s "no direct Stellar calls" hard rule. - `WalletLink` is scoped to the **facility** (the on-chain payee), not the patient. - `PaymentIntent.orderType`/`orderId` are opaque, unvalidated โ€” `Prescription`/`LabOrder` aren't real records yet (Phase 4). Don't build on top of these assuming they're FK-validated. - `release`/`refund` are admin-triggered HTTP actions in v1 (`POST /payments/intents/:id/{release,refund}`) โ€” there's no `FulfillmentRecord.status == fulfilled` to trigger off of yet. Don't treat this as the final design. +- **SEP-10 wallet-ownership proof** (issue #77): `GET /payments/sep10/challenge` + `POST /payments/sep10/verify` (see docs/api/README.md) with a dedicated `SEP10_SIGNING_SECRET` (never the payer/releaser keys). The `Sep10Challenge` nonce ledger is single-use via one conditional UPDATE โ€” no app-level check-then-write. This is generic infrastructure for ADR 0005's wallet linking; it issues no session, DocForum's own JWT still authenticates the caller. - The actual Stellar contract calls go through `@docforum/escrow-sdk` (`backend/src/modules/payments/services/escrow-sdk-loader.ts` โ€” dynamic `import()`, the SDK is ESM-only and this backend is CommonJS). Never reimplement contract-calling logic in this repo. ## Known unresolved design gaps (do not silently "fix" without flagging in PR description) diff --git a/ROADMAP.md b/ROADMAP.md index 501c0f5..6ad2cb3 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -87,6 +87,7 @@ for status and issue links. - [ ] Idempotent escrow funding + on-chain reconcile job (`fundPaymentIntent` is check-then-act โ€” docforum-escrow threat model ยง5). Tracked as [issue #24](https://github.com/DocForum/docforum-core/issues/24) (High, 200 pts). - [ ] ADR: releaser key custody โ€” multisig, KMS, or per-period rotation (threat model T1/T2). Tracked as [issue #25](https://github.com/DocForum/docforum-core/issues/25) (Medium, 150 pts). - [ ] Checksum-validate facility wallet keys, not just regex. Tracked as [issue #26](https://github.com/DocForum/docforum-core/issues/26) (Trivial, 100 pts). +- [x] SEP-10 Stellar Web Authentication server (ADR 0005 step 4): `GET /payments/sep10/challenge` + `POST /payments/sep10/verify`, full SEP-10 server verification list (incl. multisig thresholds from Horizon and the unfunded-account master-key rule), a single-use `Sep10Challenge` nonce ledger consumed atomically, and a dedicated `SEP10_SIGNING_SECRET` separate from the payer/releaser keys. Generic infrastructure the non-custodial wallet linking builds on โ€” the endpoints themselves issue no session and change no custodial behaviour. Tracked as [issue #77](https://github.com/DocForum/docforum-core/issues/77) (High, 200 pts) โ€” **Done**. - [ ] Non-custodial patient payments (v2): patients fund escrows from their own Stellar wallet, custodial kept as fallback. **Proposed** in `docs/adr/0005-non-custodial-patient-payments.md`; implementation issues to be opened once it's Accepted. - [ ] Upgrade to `@docforum/escrow-sdk` 0.3.0 and the current testnet contract (`refund_after`, balance-delta check, `getEscrow`) โ€” step 1 of ADR 0005's plan, but worth doing regardless. Tracked as [issue #32](https://github.com/DocForum/docforum-core/issues/32) (Medium, 150 pts); depends on `docforum-escrow` publishing `sdk-v0.3.0`. - [ ] Doc reconciliation: PRD ยง8.1.6 (facility fulfillment integrity) โ€” not marked resolved; this phase proves the payment *mechanism* works, not the fulfillment-integrity question, which still needs real `FulfillmentRecord`/facility-matching (Phase 5) to actually address. @@ -344,11 +345,24 @@ for status and issue links. sponsorship. Custodial v1 stays as a fallback. Status: Proposed, not yet Accepted. Added to the docs site. - 2026-10-09 โ€” Opened issue #32: upgrade to escrow SDK 0.3.0 and the - current testnet contract. -- 2026-10-15 โ€” Started Phase 8's auth-hardening work, scoped as issues - #34 (email verification), #35 (login brute-force throttling), #37 (CSRF - for the cookie-authenticated endpoints + per-environment CORS allowlist) - and #39 (admin account lifecycle). All four are independent of Phases + current testnet contract.- 2026-10-16 โ€” Implemented issue #77 (SEP-10 Stellar Web Authentication server, Phase 5.5 / ADR 0005 step 4): `GET + /payments/sep10/challenge` builds a server-signed challenge (SDK + helper โ€” Manage Data nonce, 5-minute time bounds, home domain + + web_auth_domain) and records the issuance binding; `POST + /payments/sep10/verify` walks the full SEP-10 server verification + list (structure, time bounds, server signature, client signatures + with multisig weight thresholds loaded from Horizon, and the + spec's master-key rule for unfunded accounts), then consumes the + nonce with ONE conditional UPDATE so concurrent replays can't both + succeed โ€” same DB-level pattern as booking. New `Sep10Challenge` + model (unique nonce, userId, account, expiry) with a migration; a + dedicated `SEP10_SIGNING_SECRET` (never the payer/releaser keys, + validated at startup, never logged). Horizon is behind an injectable + seam so the 15 unit + 18 integration tests run with zero network. + Docs updated: ARCHITECTURE ยง4.10, essentials data-model map, + docs/api/README.md (endpoints + errors + unfunded behaviour). +- 2026-10-15 โ€” Started Phase 8's auth-hardening work, scoped as issues #34 (email verification), #35 (login brute-force throttling), #37 (CSRF + for the cookie-authenticated endpoints + per-environment CORS allowlist) and #39 (admin account lifecycle). All four are independent of Phases 3โ€“7 and of each other. #34 merged as PR #137: `User.emailVerifiedAt` + a hashed/expiring/single-use `EmailVerificationToken` table (backfilled as verified for existing rows), `POST /auth/verify-email`, diff --git a/backend/.env.example b/backend/.env.example index 7846963..2e034af 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -43,3 +43,25 @@ PLATFORM_RELEASER_SECRET= ESCROW_CONTRACT_ID=CAXESEEAJILFFCN3PH2HHNMWAAHLEB64UTJHZ6IP7C5BBICT3S3VODSM # Native XLM SAC on testnet (stellar contract id asset --asset native --network testnet) ESCROW_TOKEN_ID=CDLZFC3SYJYDZT7K67VZ75HPJVIEUVNIXF47ZG2FB2RMQQVU2HHGCYSC + +# SEP-10 Stellar Web Authentication (issue #77) โ€” GET/POST /payments/sep10/*. +# The SEP10_SIGNING_SECRET is a DEDICATED identity, separate from +# PLATFORM_PAYER_SECRET/PLATFORM_RELEASER_SECRET โ€” it only ever signs web-auth +# challenges, so it can be rotated without touching money-moving keys. All four +# SEP10_SIGNING_SECRET/SEP10_HOME_DOMAIN/SEP10_WEB_AUTH_DOMAIN are required at +# server startup (validateSep10Config) โ€” the server refuses to boot without +# them. Secrets are never logged. +# Generate a keypair with: node -e "const {Keypair}=require('@stellar/stellar-sdk');console.log(Keypair.random().secret())" +SEP10_SIGNING_SECRET= +# Home domain of the service requiring authentication (the first Manage Data +# op key is " auth"). +SEP10_HOME_DOMAIN=docforum.github.io +# Domain of THIS server that issues the challenges (the "web_auth_domain" +# Manage Data op value) โ€” the API host, not the frontend. +SEP10_WEB_AUTH_DOMAIN=docforum-core.onrender.com +# Optional โ€” default to testnet, matching this repo's testnet-only payments +# constraint (docforum-escrow hard rule 4): +# SEP10_NETWORK_PASSPHRASE='Test SDF Network ; September 2015' +# SEP10_HORIZON_URL=https://horizon-testnet.stellar.org +# Challenge time bounds in seconds (default 300): +# SEP10_CHALLENGE_TIMEOUT_SECONDS=300 diff --git a/backend/package-lock.json b/backend/package-lock.json index c9c6bca..c073dc9 100644 --- a/backend/package-lock.json +++ b/backend/package-lock.json @@ -10,6 +10,7 @@ "dependencies": { "@docforum/escrow-sdk": "https://github.com/DocForum/docforum-escrow/releases/download/sdk-v0.2.0/docforum-escrow-sdk-0.2.0.tgz", "@prisma/client": "^6.19.3", + "@stellar/stellar-sdk": "^16.0.1", "bcryptjs": "^3.0.3", "cookie-parser": "^1.4.7", "cors": "^2.8.6", @@ -1387,7 +1388,6 @@ "cpu": [ "ppc64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1404,7 +1404,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1421,7 +1420,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1438,7 +1436,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1455,7 +1452,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1472,7 +1468,6 @@ "cpu": [ "arm" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1489,7 +1484,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1506,7 +1500,6 @@ "cpu": [ "loong64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1523,7 +1516,6 @@ "cpu": [ "mips64el" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1540,7 +1532,6 @@ "cpu": [ "ppc64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1557,7 +1548,6 @@ "cpu": [ "riscv64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1574,7 +1564,6 @@ "cpu": [ "s390x" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1591,7 +1580,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1608,7 +1596,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1625,7 +1612,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1642,7 +1628,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1659,7 +1644,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1676,7 +1660,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1693,7 +1676,6 @@ "cpu": [ "arm64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ @@ -1710,7 +1692,6 @@ "cpu": [ "x64" ], - "dev": true, "license": "Apache-2.0", "optional": true, "os": [ diff --git a/backend/package.json b/backend/package.json index ebe5a93..a9483bb 100644 --- a/backend/package.json +++ b/backend/package.json @@ -18,6 +18,7 @@ "dependencies": { "@docforum/escrow-sdk": "https://github.com/DocForum/docforum-escrow/releases/download/sdk-v0.2.0/docforum-escrow-sdk-0.2.0.tgz", "@prisma/client": "^6.19.3", + "@stellar/stellar-sdk": "^16.0.1", "bcryptjs": "^3.0.3", "cookie-parser": "^1.4.7", "cors": "^2.8.6", diff --git a/backend/prisma/migrations/20261016130509_add_sep10_challenges/migration.sql b/backend/prisma/migrations/20261016130509_add_sep10_challenges/migration.sql new file mode 100644 index 0000000..b222c39 --- /dev/null +++ b/backend/prisma/migrations/20261016130509_add_sep10_challenges/migration.sql @@ -0,0 +1,22 @@ +-- CreateTable +CREATE TABLE "Sep10Challenge" ( + "id" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "account" TEXT NOT NULL, + "nonce" TEXT NOT NULL, + "expiresAt" TIMESTAMP(3) NOT NULL, + "consumedAt" TIMESTAMP(3), + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "Sep10Challenge_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "Sep10Challenge_nonce_key" ON "Sep10Challenge"("nonce"); + +-- CreateIndex +CREATE INDEX "Sep10Challenge_expiresAt_idx" ON "Sep10Challenge"("expiresAt"); + +-- AddForeignKey +ALTER TABLE "Sep10Challenge" ADD CONSTRAINT "Sep10Challenge_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 5ceed14..8564d28 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -114,6 +114,7 @@ model User { facilityProfile FacilityProfile? emailVerificationTokens EmailVerificationToken[] + sep10Challenges Sep10Challenge[] @@index([role]) } @@ -440,6 +441,40 @@ model PaymentIntent { @@index([status]) } +// SEP-10 (Stellar Web Authentication) challenge issuance ledger โ€” +// one row per issued challenge, created by GET /payments/sep10/challenge +// and redeemed exactly once by POST /payments/sep10/verify (issue #77, +// ADR 0005 step 4: proving the caller controls a Stellar address before +// it is ever trusted as payer or payee). +// +// The nonce is the random 64-byte value embedded in the challenge +// transaction's `" auth"` Manage Data operation. Storing it +// (unique) is what makes a challenge single-use: verify consumes the row +// with ONE conditional UPDATE (`consumedAt IS NULL AND expiresAt > now`), +// so two concurrent replays can't both win โ€” the same DB-level pattern +// as booking's conditional slot update (ARCHITECTURE.md ยง5.1). +// +// Deliberately no other fields: no XDR, no signature, no PHI. The +// challenge itself is re-derivable from the submitted XDR at verify +// time; this table only proves "we issued this nonce to this user for +// this account, it hasn't been used, and it hasn't expired." +model Sep10Challenge { + id String @id @default(uuid()) + userId String + account String // the G... (or M...) account the challenge was issued for + nonce String @unique // hex/utf8 nonce from the challenge's Manage Data op + expiresAt DateTime // matches the challenge's timebounds maxTime (+ record TTL) + consumedAt DateTime? + createdAt DateTime @default(now()) + + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + + // Pruning and the "is this row still redeemable" check both scan by + // expiry โ€” same reason LoginAttempt indexes its window. Lookups at + // verify time go through the unique `nonce` index. + @@index([expiresAt]) +} + model WalletLink { id String @id @default(uuid()) // Scoped to the facility being paid, not the patient โ€” v1 is a diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index 1e54027..706bb64 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -118,4 +118,37 @@ export const config = { escrowContractId: process.env.ESCROW_CONTRACT_ID, escrowTokenId: process.env.ESCROW_TOKEN_ID, }, + // SEP-10 Stellar Web Authentication (issue #77, ADR 0005 step 4). + // + // The SEP-10 signing key is deliberately its own identity โ€” never + // PLATFORM_PAYER_SECRET or PLATFORM_RELEASER_SECRET. It exists only to + // sign/verify web-auth challenges, so if it ever leaks the blast radius + // is "someone can mint challenge transactions we would accept", not + // "someone can move platform funds". Rotating it doesn't touch the + // money-moving keys and vice versa. + // + // Unlike the payer/releaser keys above, this block IS validated at + // process startup (server.ts calls validateSep10Config()): issue #77 + // explicitly asks for startup validation, and a typo'd/missing web-auth + // key would otherwise only surface on the first challenge request. The + // money keys stay lazily validated per ADR 0004 โ€” booting the app for + // auth/booking work must not require payments to be configured. + // + // Secrets are never logged: startup validation reports the ENV VAR NAME + // only, and no code path prints the secret value. + sep10: { + signingSecret: process.env.SEP10_SIGNING_SECRET, + homeDomain: process.env.SEP10_HOME_DOMAIN, + webAuthDomain: process.env.SEP10_WEB_AUTH_DOMAIN, + // Testnet defaults, matching this repo's testnet-only payments + // constraint (docforum-escrow hard rule 4). Overridable per + // environment when a deployment points elsewhere. + networkPassphrase: process.env.SEP10_NETWORK_PASSPHRASE ?? 'Test SDF Network ; September 2015', + horizonUrl: process.env.SEP10_HORIZON_URL ?? 'https://horizon-testnet.stellar.org', + // Challenge time bounds. SEP-10's listed verification step only + // requires *bounded* timebounds; 300s is @stellar/stellar-sdk's own + // default and what issue #77 asks for. (SEP-10's prose recommends + // 900s โ€” 300s is the stricter choice and is what we build and test.) + challengeTimeoutSeconds: Number(process.env.SEP10_CHALLENGE_TIMEOUT_SECONDS ?? 300), + }, } as const; diff --git a/backend/src/modules/payments/controllers/index.ts b/backend/src/modules/payments/controllers/index.ts index ebe6a91..fa4dba9 100644 --- a/backend/src/modules/payments/controllers/index.ts +++ b/backend/src/modules/payments/controllers/index.ts @@ -72,3 +72,6 @@ export async function refundIntent(req: Request, res: Response) { const intent = await paymentsService.refundPaymentIntent(id); res.status(200).json(serializeIntent(intent)); } + +// SEP-10 Stellar Web Authentication (issue #77) โ€” see ./sep10.ts. +export { sep10Challenge, sep10Verify } from './sep10'; diff --git a/backend/src/modules/payments/controllers/sep10.ts b/backend/src/modules/payments/controllers/sep10.ts new file mode 100644 index 0000000..4bf8813 --- /dev/null +++ b/backend/src/modules/payments/controllers/sep10.ts @@ -0,0 +1,22 @@ +// SEP-10 controllers (issue #77) โ€” thin HTTP layer over services/sep10. +// Route-level `authenticate`/`requireVerifiedEmail` middleware runs first +// (see routes/index.ts); these handlers only parse input, call the +// service, and shape the response. +import type { Request, Response } from 'express'; +import * as paymentsService from '../services'; +import { sep10ChallengeQuerySchema, sep10VerifySchema } from '../validation'; +import { HttpError } from '../../../http-error'; + +export async function sep10Challenge(req: Request, res: Response) { + if (!req.user) throw new HttpError(401, 'Authentication required.'); + const { account } = sep10ChallengeQuerySchema.parse(req.query); + const challenge = await paymentsService.createSep10Challenge(req.user.id, account); + res.status(200).json(challenge); +} + +export async function sep10Verify(req: Request, res: Response) { + if (!req.user) throw new HttpError(401, 'Authentication required.'); + const { transaction } = sep10VerifySchema.parse(req.body); + const result = await paymentsService.verifySep10Challenge(req.user.id, transaction); + res.status(200).json({ ...result, userId: req.user.id }); +} diff --git a/backend/src/modules/payments/repositories/index.ts b/backend/src/modules/payments/repositories/index.ts index 7958616..7fed3d4 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -46,3 +46,29 @@ export function markEscrowed(id: string, input: { escrowId: bigint; stellarTxHas export function markStatus(id: string, status: PaymentIntentStatus) { return prisma.paymentIntent.update({ where: { id }, data: { status } }); } + +// --- SEP-10 (Stellar Web Authentication) โ€” issue #77 ---------------------- +// One row per issued challenge. The nonce comes from the challenge +// transaction's first Manage Data op; its uniqueness makes a challenge +// single-use at the DB level. + +export function createSep10Challenge(input: { userId: string; account: string; nonce: string; expiresAt: Date }) { + return prisma.sep10Challenge.create({ data: input }); +} + +export function findSep10ChallengeByNonce(nonce: string) { + return prisma.sep10Challenge.findUnique({ where: { nonce } }); +} + +/** Atomically consumes a challenge โ€” ONE conditional UPDATE, no + * read-then-write. Only a row that is still unconsumed AND unexpired + * moves to `consumedAt`; the returned `count` tells the caller whether + * THIS request won the race. Two concurrent replays of the same signed + * challenge can never both see `count === 1`. Same DB-level pattern as + * booking's conditional slot update (ARCHITECTURE.md ยง5.1). */ +export function consumeSep10Challenge(nonce: string, consumedAt: Date) { + return prisma.sep10Challenge.updateMany({ + where: { nonce, consumedAt: null, expiresAt: { gt: consumedAt } }, + data: { consumedAt }, + }); +} diff --git a/backend/src/modules/payments/routes/index.ts b/backend/src/modules/payments/routes/index.ts index 7f80503..01131aa 100644 --- a/backend/src/modules/payments/routes/index.ts +++ b/backend/src/modules/payments/routes/index.ts @@ -46,3 +46,24 @@ paymentsRouter.post( requireVerifiedEmail, paymentsController.refundIntent, ); + +// SEP-10 Stellar Web Authentication (issue #77, ADR 0005 step 4): prove +// the caller controls a Stellar address before it is ever trusted as a +// payer or payee. Any authenticated role may link a wallet (patient +// wallets become the payer in v2; facility payout wallets replace the +// type-the-address form), so no role gate here โ€” but a VERIFIED address +// is required, same as every other payments mutation in this module +// (issue #34): a wallet link gated only by a possibly-unverified email +// would be a bypass of the guard POST /payments/wallet-link already has. +paymentsRouter.get( + '/sep10/challenge', + authenticate, + requireVerifiedEmail, + paymentsController.sep10Challenge, +); +paymentsRouter.post( + '/sep10/verify', + authenticate, + requireVerifiedEmail, + paymentsController.sep10Verify, +); diff --git a/backend/src/modules/payments/services/index.ts b/backend/src/modules/payments/services/index.ts index c36cc05..e90c751 100644 --- a/backend/src/modules/payments/services/index.ts +++ b/backend/src/modules/payments/services/index.ts @@ -14,6 +14,19 @@ import { config } from '../../../config'; import * as paymentsRepo from '../repositories'; import { loadEscrowSdk } from './escrow-sdk-loader'; +// SEP-10 Stellar Web Authentication (issue #77). Re-exported through +// this module's service surface (not imported file-by-file from outside) +// per the AGENTS.md module-boundary rule โ€” same pattern as the auth +// module's token/throttle re-exports. +export { + createSep10Challenge, + verifySep10Challenge, + resetSep10AccountFetcher, + setSep10AccountFetcher, + validateSep10Config, +} from './sep10'; +export type { Sep10AccountFetcher, Sep10AccountInfo, Sep10ChallengeResponse, Sep10Signer, Sep10VerifyResult } from './sep10'; + async function requirePaymentsConfig() { const { platformPayerSecret, platformReleaserSecret, escrowContractId, escrowTokenId } = config.payments; const missing = Object.entries({ platformPayerSecret, platformReleaserSecret, escrowContractId, escrowTokenId }) diff --git a/backend/src/modules/payments/services/sep10.ts b/backend/src/modules/payments/services/sep10.ts new file mode 100644 index 0000000..4379b3b --- /dev/null +++ b/backend/src/modules/payments/services/sep10.ts @@ -0,0 +1,384 @@ +// SEP-10 Stellar Web Authentication server (issue #77, ADR 0005 step 4). +// +// Before a patient or facility wallet can be trusted as payer or payee, +// the backend must prove the caller controls that Stellar address. SEP-10 +// is the standard flow: the server issues a challenge transaction, the +// user's wallet signs it server-side (the backend builds and submits +// everything; the browser only relays XDR โ€” docforum-web hard rule 1), +// and the server verifies the signature. This file is generic +// infrastructure; wallet-link storage (patient/facility scoped links) is +// future work on top of it. +// +// What verifies what โ€” the SEP-10 server verification list, in order: +// 1. base64 XDR deserializes as a Transaction โ†’ readChallengeTx +// 2. sequence number == 0 โ†’ readChallengeTx +// 3. tx source == server signing account โ†’ readChallengeTx +// 4. bounded, unexpired timebounds โ†’ readChallengeTx (SDK +// allows 5 min of clock-drift grace; our DB record enforces the issued +// timeout strictly, so the two layers together are tighter than either) +// 5. >= 1 operation; first op is Manage Data with the client account as +// source; key is " auth"; value is a 48-byte-base64 +// nonce; subsequent ops are Manage Data sourced by the server with +// web_auth_domain == ours โ†’ readChallengeTx +// 6. server signature present and valid โ†’ readChallengeTx +// (steps 1-6 = WebAuth.readChallengeTx) +// 7. client signatures: for a FUNDED account, signer weights loaded from +// Horizon must meet the account's medium threshold +// (verifyChallengeTxThreshold); for an UNFUNDED account (per spec โ€” +// see below) the master key must sign +// (verifyChallengeTxSigners). +// Plus our own additions on top of the spec list: the challenge's nonce +// must exist in OUR issuance ledger, still unconsumed and unexpired +// (single-use, consumed atomically), and must be bound to the +// authenticated caller and the exact account in the challenge. +import { HttpError } from '../../../http-error'; +import { config } from '../../../config'; +import * as paymentsRepo from '../repositories'; +import { loadStellarSdk } from './stellar-sdk-loader'; +import type { Keypair, Transaction } from '@stellar/stellar-sdk' with { 'resolution-mode': 'import' }; + +// --- Configuration ------------------------------------------------------- + +export interface Sep10Config { + serverKeypair: Keypair; + serverAccountId: string; + homeDomain: string; + webAuthDomain: string; + networkPassphrase: string; + horizonUrl: string; + timeoutSeconds: number; +} + +/** Human-readable configuration problems, naming ENV VAR NAMES only โ€” + * never the secret value. Used by both the startup check and the lazy + * per-request check. */ +export async function sep10ConfigProblems(): Promise { + const problems: string[] = []; + if (!config.sep10.signingSecret) problems.push('SEP10_SIGNING_SECRET'); + if (!config.sep10.homeDomain) problems.push('SEP10_HOME_DOMAIN'); + if (!config.sep10.webAuthDomain) problems.push('SEP10_WEB_AUTH_DOMAIN'); + if (!Number.isInteger(config.sep10.challengeTimeoutSeconds) || config.sep10.challengeTimeoutSeconds <= 0) { + problems.push('SEP10_CHALLENGE_TIMEOUT_SECONDS (must be a positive integer)'); + } + if (config.sep10.signingSecret) { + const { Keypair } = await loadStellarSdk(); + try { + Keypair.fromSecret(config.sep10.signingSecret); + } catch { + // The parse error itself would echo the bad secret โ€” report the var + // name, never the value. + problems.push('SEP10_SIGNING_SECRET (not a valid Stellar secret key)'); + } + } + return problems; +} + +/** Startup validation (issue #77: "validate at startup"). Called from + * src/server.ts before the listener starts, so a deployment with a + * missing/typo'd web-auth key fails to boot instead of 500ing on the + * first challenge request. Deliberately NOT called at module import: + * test suites build the app without importing server.ts. */ +export async function validateSep10Config(): Promise { + const problems = await sep10ConfigProblems(); + if (problems.length > 0) { + throw new Error(`SEP-10 configuration invalid โ€” missing or invalid env var(s): ${problems.join(', ')}.`); + } +} + +/** Lazy per-request check for paths that need the config (mirrors + * requirePaymentsConfig in the sibling payments services). Throws a 500 + * HttpError, not a boot failure โ€” this is the second line of defence for + * environments that don't run server.ts. */ +async function requireSep10Config(): Promise { + const problems = await sep10ConfigProblems(); + if (problems.length > 0) { + throw new HttpError(500, `SEP-10 is not configured โ€” missing or invalid env var(s): ${problems.join(', ')}.`); + } + const { Keypair } = await loadStellarSdk(); + const serverKeypair = Keypair.fromSecret(config.sep10.signingSecret!); + return { + serverKeypair, + serverAccountId: serverKeypair.publicKey(), + homeDomain: config.sep10.homeDomain!, + webAuthDomain: config.sep10.webAuthDomain!, + networkPassphrase: config.sep10.networkPassphrase, + horizonUrl: config.sep10.horizonUrl, + timeoutSeconds: config.sep10.challengeTimeoutSeconds, + }; +} + +// --- Horizon seam (tests inject a fake; no network in the test suite) ---- + +export interface Sep10Signer { + key: string; + weight: number; + type: string; +} + +export interface Sep10AccountInfo { + accountId: string; + thresholds: { low_threshold: number; med_threshold: number; high_threshold: number }; + signers: Sep10Signer[]; +} + +/** Returns null when the account does not exist on the network yet + * (Horizon 404 โ€” the SEP-10 "unfunded" case). Throws on any other + * Horizon failure. */ +export type Sep10AccountFetcher = (accountId: string) => Promise; + +let accountFetcherOverride: Sep10AccountFetcher | undefined; + +/** Test seam, same pattern as auth's setEmailSender: unit/integration + * tests replace the outbound Horizon call through this module's service + * surface instead of mocking a network client. Never called with an + * override in production code. */ +export function setSep10AccountFetcher(fetcher: Sep10AccountFetcher): void { + accountFetcherOverride = fetcher; +} + +export function resetSep10AccountFetcher(): void { + accountFetcherOverride = undefined; +} + +function isHorizonNotFound(err: unknown): boolean { + if (typeof err !== 'object' || err === null) return false; + const candidate = err as { name?: string; status?: number; response?: { status?: number } }; + return candidate.name === 'NotFoundError' || candidate.status === 404 || candidate.response?.status === 404; +} + +async function defaultAccountFetcher(accountId: string): Promise { + const { Horizon } = await loadStellarSdk(); + const horizon = new Horizon.Server(config.sep10.horizonUrl); + try { + const account = await horizon.accounts().accountId(accountId).call(); + return { + accountId: account.id, + thresholds: { + low_threshold: account.thresholds.low_threshold, + med_threshold: account.thresholds.med_threshold, + high_threshold: account.thresholds.high_threshold, + }, + signers: account.signers.map((signer) => ({ key: signer.key, weight: signer.weight, type: signer.type })), + }; + } catch (err) { + if (isHorizonNotFound(err)) return null; + throw err; + } +} + +async function fetchAccountInfo(accountId: string): Promise { + try { + return await (accountFetcherOverride ?? defaultAccountFetcher)(accountId); + } catch (err) { + if (err instanceof HttpError) throw err; + if (isHorizonNotFound(err)) return null; + // Don't forward Horizon/axios internals (URLs, request config) to the + // client โ€” the fact that we couldn't load signers is all they get. + throw new HttpError(502, 'Could not load the account from Horizon to verify its signers.', 'SEP10_HORIZON_UNAVAILABLE'); + } +} + +// --- Challenge building -------------------------------------------------- + +/** The nonce is the base64 string in the first Manage Data op's value + * (" auth", source = client account): 48 random bytes + * base64-encoded to 64 chars, per SEP-10. That exact string is what gets + * stored as Sep10Challenge.nonce โ€” the single-use key. */ +function challengeNonce(tx: Transaction): string { + const [firstOp] = tx.operations; + if (!firstOp || firstOp.type !== 'manageData') { + throw new HttpError(500, 'Built challenge is malformed (first operation is not Manage Data).'); + } + // Parsed Manage Data values arrive as a string for valid UTF-8 (which + // a base64 nonce always is) and as a Buffer otherwise; normalise to + // the exact string form SEP-10 specifies. + const value = firstOp.value; + return Buffer.isBuffer(value) ? value.toString('utf8') : String(value); +} + +/** Parses a submitted challenge XDR just enough to pull the nonce out. + * Assumes the caller already ran WebAuth.readChallengeTx (structure, + * timebounds and server signature are its job, not this helper's). */ +function signedChallengeNonce(tx: Transaction): string { + return challengeNonce(tx); +} + +export interface Sep10ChallengeResponse { + transaction: string; + expires_in: number; + network_passphrase: string; +} + +/** GET /payments/sep10/challenge โ€” builds a server-signed SEP-10 + * challenge for `account` and records the issuance binding + * (userId, account, nonce, expiresAt) so a verified signature can only + * ever be redeemed by that user, for that account, once. */ +export async function createSep10Challenge(userId: string, account: string): Promise { + const { StrKey, WebAuth } = await loadStellarSdk(); + + // Full strkey validation (base32 + CRC16 checksum), not just the Gโ€ฆ + // shape โ€” a checksum-invalid key would otherwise be discovered only at + // wallet-signing time. + if (!StrKey.isValidEd25519PublicKey(account)) { + throw new HttpError(400, 'account must be a valid Stellar public key (Gโ€ฆ).', 'SEP10_INVALID_ACCOUNT'); + } + + const cfg = await requireSep10Config(); + + let transaction: string; + try { + // Server-signed, Manage Data nonce, bounded timebounds, home domain + // and web_auth_domain โ€” all handled by the SDK helper. + transaction = WebAuth.buildChallengeTx( + cfg.serverKeypair, + account, + cfg.homeDomain, + cfg.timeoutSeconds, + cfg.networkPassphrase, + cfg.webAuthDomain, + ); + } catch (err) { + // No secret can appear here (the keypair was already validated), but + // keep the message generic anyway. + throw new HttpError(500, `Could not build the SEP-10 challenge: ${err instanceof Error ? err.message : 'unknown error'}`); + } + + // Read our own challenge back through the same validation path a client + // submission takes โ€” if it doesn't round-trip, we'd be handing out a + // challenge we would later reject. + const read = WebAuth.readChallengeTx(transaction, cfg.serverAccountId, cfg.networkPassphrase, cfg.homeDomain, cfg.webAuthDomain); + const nonce = challengeNonce(read.tx); + const maxTimeSeconds = Number(read.tx.timeBounds?.maxTime ?? NaN); + if (!Number.isFinite(maxTimeSeconds)) { + throw new HttpError(500, 'Built challenge has no time bounds.'); + } + const expiresAt = new Date(maxTimeSeconds * 1000); + + await paymentsRepo.createSep10Challenge({ userId, account, nonce, expiresAt }); + + return { + transaction, + expires_in: cfg.timeoutSeconds, + network_passphrase: cfg.networkPassphrase, + }; +} + +// --- Challenge verification --------------------------------------------- + +export interface Sep10VerifyResult { + account: string; + signers: string[]; +} + +/** POST /payments/sep10/verify โ€” the full SEP-10 server verification + * list plus DocForum's own single-use/binding rules. On success the + * caller has proven control of `account` (fundamentally: the same proof + * of control the Stellar network itself would honour for this + * transaction), and the challenge is consumed. + * + * UNFUNDED ACCOUNTS (SEP-10 spec): an account that does not exist on the + * network has no signers and no thresholds on record, so weights cannot + * be checked. The spec's rule for that case is that the challenge must be + * signed by the account's MASTER key, and by exactly one other signature + * (the server's) on the envelope โ€” verifyChallengeTxSigners enforces + * both. We detect the unfunded case by a Horizon 404; any other Horizon + * error is a 502, never silently treated as unfunded (that would downgrade + * multisig accounts to master-key-only verification). + * + * This endpoint does NOT mint a DocForum session JWT: DocForum's own + * auth (Bearer access token) already authenticates the caller โ€” SEP-10's + * job here is proving wallet control, not issuing a session. */ +export async function verifySep10Challenge(userId: string, challengeTx: string): Promise { + const cfg = await requireSep10Config(); + const { WebAuth } = await loadStellarSdk(); + + // Steps 1-6 of the SEP-10 list: deserialize, sequence == 0, source == + // server account, bounded/unexpired timebounds, operation shape + // (first op Manage Data sourced by the client with " auth" + // key and a valid nonce, later ops Manage Data sourced by the server, + // web_auth_domain == ours), and the server signature. + let read: { tx: Transaction; clientAccountID: string }; + try { + read = WebAuth.readChallengeTx( + challengeTx, + cfg.serverAccountId, + cfg.networkPassphrase, + cfg.homeDomain, + cfg.webAuthDomain, + ); + } catch (err) { + throw new HttpError(400, `Invalid challenge: ${err instanceof Error ? err.message : 'could not parse transaction'}`, 'SEP10_INVALID_CHALLENGE'); + } + + const nonce = signedChallengeNonce(read.tx); + + // Our own ledger: was this challenge issued by us, is it still good, + // and is it THIS caller's, for THIS account? Checked before the Horizon + // round trip so a mismatched submission can't make us fetch signers. + const record = await paymentsRepo.findSep10ChallengeByNonce(nonce); + if (!record) { + throw new HttpError(401, 'Challenge was not issued by this server.', 'SEP10_CHALLENGE_UNKNOWN'); + } + if (record.userId !== userId || record.account !== read.clientAccountID) { + // A signed challenge is never transferable: not to another user, and + // not for another account than the one the challenge was issued for. + throw new HttpError(403, 'This challenge was issued to a different user or account.', 'SEP10_CHALLENGE_NOT_FOR_CALLER'); + } + if (record.consumedAt) { + throw new HttpError(401, 'This challenge has already been used.', 'SEP10_CHALLENGE_ALREADY_USED'); + } + // Strict check against OUR clock, without the SDK's 5-minute drift + // grace โ€” a challenge is good for exactly the window we issued it for. + if (record.expiresAt.getTime() <= Date.now()) { + throw new HttpError(401, 'This challenge has expired.', 'SEP10_CHALLENGE_EXPIRED'); + } + + // Steps 7/8 of the SEP-10 list: client signatures. + const accountInfo = await fetchAccountInfo(read.clientAccountID); + let matchedSigners: string[]; + try { + if (accountInfo === null) { + // Unfunded account โ€” no on-chain signers/thresholds exist; the + // master key must sign (documented in the API docs as well). + matchedSigners = WebAuth.verifyChallengeTxSigners( + challengeTx, + cfg.serverAccountId, + cfg.networkPassphrase, + [read.clientAccountID], + cfg.homeDomain, + cfg.webAuthDomain, + ); + } else { + // Multisig: load signers + thresholds from Horizon and enforce the + // account's MEDIUM threshold โ€” SEP-10's guidance is medium for + // authority aligned with the ability to move funds, which is + // exactly what a verified wallet link gates. The server's own + // signature contributes no client weight (enforced by the SDK). + matchedSigners = WebAuth.verifyChallengeTxThreshold( + challengeTx, + cfg.serverAccountId, + cfg.networkPassphrase, + accountInfo.thresholds.med_threshold, + accountInfo.signers, + cfg.homeDomain, + cfg.webAuthDomain, + ); + } + } catch (err) { + // A structurally sound challenge whose client signature is missing, + // unknown, or below threshold. + throw new HttpError(401, `Challenge signature verification failed: ${err instanceof Error ? err.message : 'unknown error'}`, 'SEP10_INVALID_SIGNATURE'); + } + + // Step 9 โ€” single-use, consumed atomically and LAST: a bad signature + // never burns a legitimately issued nonce, and two concurrent replays + // of the same signed challenge race here โ€” exactly one conditional + // UPDATE wins (`WHERE consumedAt IS NULL AND expiresAt > now`). + const consumedAt = new Date(); + const { count } = await paymentsRepo.consumeSep10Challenge(nonce, consumedAt); + if (count !== 1) { + throw new HttpError(401, 'This challenge has already been used.', 'SEP10_CHALLENGE_ALREADY_USED'); + } + + return { account: read.clientAccountID, signers: matchedSigners }; +} diff --git a/backend/src/modules/payments/services/stellar-sdk-loader.ts b/backend/src/modules/payments/services/stellar-sdk-loader.ts new file mode 100644 index 0000000..50d10a8 --- /dev/null +++ b/backend/src/modules/payments/services/stellar-sdk-loader.ts @@ -0,0 +1,14 @@ +// @stellar/stellar-sdk (v16) ships both CJS and ESM builds, but its +// package `types` resolve as ECMAScript-module declarations โ€” a static +// import in this CommonJS backend trips TS1479/TS1541 under `module: +// node16`. A dynamic `import()` is required instead, exactly like the +// existing escrow-sdk-loader.ts (ESM-only @docforum/escrow-sdk) and the +// embedded-postgres interop. One cached module promise per process. +type StellarSdkModule = typeof import('@stellar/stellar-sdk', { with: { 'resolution-mode': 'import' } }); + +let modulePromise: Promise | undefined; + +export function loadStellarSdk(): Promise { + modulePromise ??= import('@stellar/stellar-sdk'); + return modulePromise; +} diff --git a/backend/src/modules/payments/validation.ts b/backend/src/modules/payments/validation.ts index 3513c8d..4644be4 100644 --- a/backend/src/modules/payments/validation.ts +++ b/backend/src/modules/payments/validation.ts @@ -4,6 +4,8 @@ export const linkWalletSchema = z.object({ // Stellar public keys are 56 chars, start with 'G'. Not a full // checksum validation (that's what create_escrow/the network itself // will reject) โ€” just a sanity bound against obvious garbage input. + // (The SEP-10 challenge endpoint DOES checksum-validate via + // StrKey.isValidEd25519PublicKey โ€” see services/sep10.ts.) stellarPublicKey: z .string() .regex(/^G[A-Z2-7]{55}$/, 'stellarPublicKey must be a valid Stellar public key (starts with G, 56 chars).'), @@ -25,3 +27,18 @@ export const createPaymentIntentSchema = z.object({ .regex(/^[1-9][0-9]*$/, 'amountStroops must be a positive integer string.') .transform((value) => BigInt(value)), }); + +// SEP-10 (issue #77). Shape-only gate: the checksum-accurate strkey +// validation (and the 400) happens in services/sep10.ts via +// StrKey.isValidEd25519PublicKey, so the service layer is safe for +// non-HTTP callers too. +export const sep10ChallengeQuerySchema = z.object({ + account: z + .string() + .regex(/^G[A-Z2-7]{55}$/, 'account must be a valid Stellar public key (starts with G, 56 chars).'), +}); + +export const sep10VerifySchema = z.object({ + // base64 TransactionEnvelope XDR of the wallet-signed challenge. + transaction: z.string().min(1), +}); diff --git a/backend/src/server.ts b/backend/src/server.ts index 52cd7e2..f470c82 100644 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -1,12 +1,27 @@ import { createApp } from './app'; import { config } from './config'; import { pruneLoginAttempts } from './modules/auth/services'; +import { validateSep10Config } from './modules/payments/services'; const app = createApp(); -app.listen(config.port, () => { +// Fail fast on a missing/invalid SEP-10 web-auth configuration (issue +// #77): a typo'd signing secret should stop boot, not surface as a 500 +// on the first challenge request. Reports env var NAMES only โ€” never a +// secret value. Placed here (not at config import) so test suites, which +// only build the app, aren't forced to boot with payments configured. +async function start(): Promise { + await validateSep10Config(); + app.listen(config.port, () => { + // eslint-disable-next-line no-console + console.log(`docforum-core backend listening on :${config.port}`); + }); +} + +start().catch((err) => { // eslint-disable-next-line no-console - console.log(`docforum-core backend listening on :${config.port}`); + console.error(err instanceof Error ? err.message : err); + process.exit(1); }); // Login-throttle cleanup (Phase 8, issue #35). A plain interval, per diff --git a/backend/tests/integration/sep10.test.mts b/backend/tests/integration/sep10.test.mts new file mode 100644 index 0000000..441c890 --- /dev/null +++ b/backend/tests/integration/sep10.test.mts @@ -0,0 +1,502 @@ +// Integration tests for the SEP-10 Stellar Web Authentication server +// (issue #77) โ€” real embedded Postgres (nonce ledger, atomic consume), +// real HTTP through the Express app, and a FAKE Horizon (no network: the +// account/signer lookup is injected via setSep10AccountFetcher, same +// service-surface seam pattern as auth's setEmailSender). +// +// What lives here (DB- or HTTP-level): the happy path, replay +// (sequential and genuinely concurrent), expiry (both the challenge's +// time bounds and an expired nonce record), multisig thresholds with +// differing weights, unfunded accounts (master-key rule), user/account +// binding, unknown nonces, and the endpoint-level auth/validation cases. +// The structural verification list (wrong source, bad sequence, wrong +// domains, extra ops, wrong server signature) is unit-tested in +// tests/unit/sep10.test.mts against the same service functions. +// +// Self-contained embedded Postgres, own port (5438) so it runs alongside +// the other integration files (5433 booking, 5435/5436/5437 others). +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, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import EmbeddedPostgres from 'embedded-postgres'; +import { Keypair, TransactionBuilder, WebAuth } from '@stellar/stellar-sdk'; +import type { Transaction } from '@stellar/stellar-sdk'; +import type { Sep10AccountInfo } from '../../src/modules/payments/services/sep10.js'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5438; +const runId = Date.now(); +const PASSWORD = 'longenough123'; + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-sep10-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`; + +type App = ReturnType; +type Prisma = typeof import('../../src/db/prisma-client.js').prisma; +type Sep10Services = typeof import('../../src/modules/payments/services/index.js'); + +describe('SEP-10 Stellar Web Authentication server', () => { + let app: App; + let prisma: Prisma; + let sep10: Sep10Services; + let serverKp: Keypair; + let homeDomain: string; + let webAuthDomain: string; + let networkPassphrase: string; + + // Fake Horizon state: per-account overrides; anything not overridden is + // treated as a funded account whose master key is the only signer + // (weight 1, med threshold 1) โ€” the plain single-signer case. + const accountOverrides = new Map(); + function fundedSingleSigner(accountId: string) { + return { + accountId, + thresholds: { low_threshold: 0, med_threshold: 1, high_threshold: 1 }, + signers: [{ key: accountId, weight: 1, type: 'ed25519_public_key' }], + }; + } + + 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; + + const appModule = await import('../../src/app.js'); + const dbModule = await import('../../src/db/prisma-client.js'); + const servicesModule = await import('../../src/modules/payments/services/index.js'); + const configModule = await import('../../src/config/index.js'); + app = appModule.createApp(); + prisma = dbModule.prisma; + sep10 = servicesModule; + + serverKp = Keypair.fromSecret(configModule.config.sep10.signingSecret!); + homeDomain = configModule.config.sep10.homeDomain!; + webAuthDomain = configModule.config.sep10.webAuthDomain!; + networkPassphrase = configModule.config.sep10.networkPassphrase; + + sep10.setSep10AccountFetcher((accountId) => + Promise.resolve(accountOverrides.has(accountId) ? accountOverrides.get(accountId)! : fundedSingleSigner(accountId)), + ); + }); + + afterAll(async () => { + sep10?.resetSep10AccountFetcher(); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + afterEach(() => { + accountOverrides.clear(); + }); + + // --- helpers ----------------------------------------------------------- + + async function signupVerified(label: string): Promise<{ userId: string; accessToken: string }> { + const email = `sep10-${label}-${runId}@example.com`; + const res = await request(app) + .post('/auth/signup') + .send({ role: 'patient', email, password: PASSWORD, fullName: `SEP10 ${label}` }); + expect(res.status).toBe(200); + const userId = res.body.user.id as string; + // The route requires a verified address (issue #34 guard) โ€” mark it + // verified directly, same as the other payments integration suites. + await prisma.user.update({ where: { id: userId }, data: { emailVerifiedAt: new Date() } }); + return { userId, accessToken: res.body.accessToken as string }; + } + + function getChallenge(accessToken: string, account: string) { + return request(app).get('/payments/sep10/challenge').set('Authorization', `Bearer ${accessToken}`).query({ account }); + } + + function postVerify(accessToken: string, transaction: string) { + return request(app) + .post('/payments/sep10/verify') + .set('Authorization', `Bearer ${accessToken}`) + .send({ transaction }); + } + + function signChallenge(challengeXdr: string, ...keypairs: Keypair[]): string { + const tx = TransactionBuilder.fromXDR(challengeXdr, networkPassphrase) as Transaction; + for (const kp of keypairs) tx.sign(kp); + return tx.toEnvelope().toXDR('base64').toString(); + } + + function challengeNonce(challengeXdr: string): string { + const read = WebAuth.readChallengeTx(challengeXdr, serverKp.publicKey(), networkPassphrase, homeDomain, webAuthDomain); + const [firstOp] = read.tx.operations; + if (!firstOp || firstOp.type !== 'manageData') throw new Error('challenge has no leading Manage Data operation'); + return String(firstOp.value); + } + + /** Issues a challenge over HTTP for `account` and signs it with the + * given keypairs, returning both the raw challenge and the signed XDR. */ + async function issueAndSign(accessToken: string, account: string, ...keypairs: Keypair[]) { + const res = await getChallenge(accessToken, account); + expect(res.status).toBe(200); + return { challenge: res.body.transaction as string, signed: signChallenge(res.body.transaction, ...keypairs) }; + } + + // --- challenge endpoint ------------------------------------------------ + + it('issues a well-formed challenge bound to the caller and the requested account', async () => { + const { accessToken, userId } = await signupVerified('shape'); + const clientKp = Keypair.random(); + + const res = await getChallenge(accessToken, clientKp.publicKey()); + expect(res.status).toBe(200); + expect(res.body.expires_in).toBe(300); + expect(res.body.network_passphrase).toBe(networkPassphrase); + + // Structure per SEP-10: server-signed, sequence 0, bounded ~5-min + // timebounds, Manage Data nonce sourced by the client, web_auth_domain. + const read = WebAuth.readChallengeTx( + res.body.transaction, + serverKp.publicKey(), + networkPassphrase, + homeDomain, + webAuthDomain, + ); + expect(read.clientAccountID).toBe(clientKp.publicKey()); + const tx = read.tx; + expect(Number(tx.sequence)).toBe(0); + expect(Number(tx.timeBounds!.maxTime) - Number(tx.timeBounds!.minTime)).toBe(300); + + // The issuance ledger recorded the binding. + const row = await prisma.sep10Challenge.findUnique({ where: { nonce: challengeNonce(res.body.transaction) } }); + expect(row).not.toBeNull(); + expect(row!.userId).toBe(userId); + expect(row!.account).toBe(clientKp.publicKey()); + expect(row!.consumedAt).toBeNull(); + expect(row!.expiresAt.getTime()).toBe(Number(tx.timeBounds!.maxTime) * 1000); + }); + + it('rejects an invalid account parameter with 400 (shape and checksum)', async () => { + const { accessToken } = await signupVerified('bad-account'); + const rowsBefore = await prisma.sep10Challenge.count(); + const tooShort = await getChallenge(accessToken, 'GNOTAKEY'); + expect(tooShort.status).toBe(400); + expect(tooShort.body.message).toMatch(/valid Stellar public key/); + + // Right shape, corrupted checksum. + const valid = Keypair.random().publicKey(); + const corrupted = valid.slice(0, -1) + (valid.endsWith('A') ? 'B' : 'A'); + const badChecksum = await getChallenge(accessToken, corrupted); + expect(badChecksum.status).toBe(400); + expect(badChecksum.body.message).toMatch(/valid Stellar public key/); + + const missing = await request(app).get('/payments/sep10/challenge').set('Authorization', `Bearer ${accessToken}`); + expect(missing.status).toBe(400); + + // Nothing was recorded for any of them. + expect(await prisma.sep10Challenge.count()).toBe(rowsBefore); + }); + + it('rejects unauthenticated requests to both endpoints', async () => { + const noAuthGet = await request(app).get('/payments/sep10/challenge').query({ account: Keypair.random().publicKey() }); + expect(noAuthGet.status).toBe(401); + + const noAuthPost = await request(app).post('/payments/sep10/verify').send({ transaction: 'x' }); + expect(noAuthPost.status).toBe(401); + }); + + it('rejects an unauthenticated-email account with 403 EMAIL_NOT_VERIFIED', async () => { + const email = `sep10-unverified-${runId}@example.com`; + const signup = await request(app) + .post('/auth/signup') + .send({ role: 'patient', email, password: PASSWORD, fullName: 'SEP10 Unverified' }); + expect(signup.status).toBe(200); + const token = signup.body.accessToken as string; + + const res = await getChallenge(token, Keypair.random().publicKey()); + expect(res.status).toBe(403); + expect(res.body.code).toBe('EMAIL_NOT_VERIFIED'); + }); + + // --- happy path + single use ------------------------------------------- + + it('verifies a signed challenge end-to-end and consumes the nonce', async () => { + const { accessToken, userId } = await signupVerified('happy'); + const clientKp = Keypair.random(); + + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp); + + const verify = await postVerify(accessToken, signed); + expect(verify.status).toBe(200); + expect(verify.body).toEqual({ account: clientKp.publicKey(), signers: [clientKp.publicKey()], userId }); + + const row = await prisma.sep10Challenge.findUnique({ where: { nonce: challengeNonce(signed) } }); + expect(row!.consumedAt).not.toBeNull(); + }); + + it('rejects a replayed challenge', async () => { + const { accessToken } = await signupVerified('replay'); + const clientKp = Keypair.random(); + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp); + + expect((await postVerify(accessToken, signed)).status).toBe(200); + + const replay = await postVerify(accessToken, signed); + expect(replay.status).toBe(401); + expect(replay.body.code).toBe('SEP10_CHALLENGE_ALREADY_USED'); + }); + + it('lets exactly ONE of several concurrent verifies succeed', async () => { + const { accessToken } = await signupVerified('concurrent'); + const clientKp = Keypair.random(); + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp); + + // Genuinely concurrent: all in flight before any response is read. + // The nonce is consumed by a single conditional UPDATE, so exactly one + // request can win โ€” the same DB-level pattern as double-booking. + const results = await Promise.all(Array.from({ length: 8 }, () => postVerify(accessToken, signed))); + + const succeeded = results.filter((r) => r.status === 200); + const rejected = results.filter((r) => r.status === 401); + expect(succeeded).toHaveLength(1); + expect(rejected).toHaveLength(7); + for (const r of rejected) expect(r.body.code).toBe('SEP10_CHALLENGE_ALREADY_USED'); + }); + + // --- expiry ----------------------------------------------------------- + + it('rejects a challenge whose time bounds have expired', async () => { + const { accessToken } = await signupVerified('expired-bounds'); + const clientKp = Keypair.random(); + + // Build (with the real server key) a challenge that expired long ago โ€” + // beyond the SDK's 5-minute clock-drift grace window. + const { Account, BASE_FEE, Operation, TransactionBuilder: TB } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + const expired = new TB(new Account(serverKp.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase, + timebounds: { minTime: now - 1800, maxTime: now - 1200 }, + }) + .addOperation( + Operation.manageData({ name: `${homeDomain} auth`, value: 'a'.repeat(64), source: clientKp.publicKey() }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: webAuthDomain, source: serverKp.publicKey() })) + .build(); + expired.sign(serverKp); + + const res = await postVerify(accessToken, signChallenge(expired.toEnvelope().toXDR('base64'), clientKp)); + expect(res.status).toBe(400); + expect(res.body.code).toBe('SEP10_INVALID_CHALLENGE'); + expect(res.body.message).toMatch(/expired/); + }); + + it('rejects a challenge whose nonce record has expired', async () => { + const { accessToken } = await signupVerified('expired-record'); + const clientKp = Keypair.random(); + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp); + + // The tx time bounds still allow the SDK's drift grace, but OUR + // ledger row is past its issued expiry โ€” the stricter check wins. + await prisma.sep10Challenge.update({ + where: { nonce: challengeNonce(signed) }, + data: { expiresAt: new Date(Date.now() - 60_000) }, + }); + + const res = await postVerify(accessToken, signed); + expect(res.status).toBe(401); + expect(res.body.code).toBe('SEP10_CHALLENGE_EXPIRED'); + }); + + // --- signatures: wrong signer, multisig, unfunded ---------------------- + + it('rejects a challenge signed by a key that is not a signer', async () => { + const { accessToken } = await signupVerified('wrong-signer'); + const clientKp = Keypair.random(); + const attackerKp = Keypair.random(); + + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), attackerKp); + + const res = await postVerify(accessToken, signed); + expect(res.status).toBe(401); + expect(res.body.code).toBe('SEP10_INVALID_SIGNATURE'); + // The nonce was NOT burned by a failed verification. + const row = await prisma.sep10Challenge.findUnique({ where: { nonce: challengeNonce(signed) } }); + expect(row!.consumedAt).toBeNull(); + }); + + it('rejects a challenge carrying an extra unrecognized signature', async () => { + const { accessToken } = await signupVerified('extra-sig'); + const clientKp = Keypair.random(); + + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp, Keypair.random()); + + const res = await postVerify(accessToken, signed); + expect(res.status).toBe(401); + expect(res.body.code).toBe('SEP10_INVALID_SIGNATURE'); + }); + + it('accepts a multisig account when signer weights meet the medium threshold', async () => { + const { accessToken } = await signupVerified('multisig-ok'); + const clientKp = Keypair.random(); + const signer1 = Keypair.random(); + const signer2 = Keypair.random(); + // threshold 3; weights 1 + 2 โ€” only BOTH signatures together pass. + accountOverrides.set(clientKp.publicKey(), { + accountId: clientKp.publicKey(), + thresholds: { low_threshold: 0, med_threshold: 3, high_threshold: 3 }, + signers: [ + { key: clientKp.publicKey(), weight: 1, type: 'ed25519_public_key' }, + { key: signer1.publicKey(), weight: 1, type: 'ed25519_public_key' }, + { key: signer2.publicKey(), weight: 2, type: 'ed25519_public_key' }, + ], + }); + + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), signer1, signer2); + + const res = await postVerify(accessToken, signed); + expect(res.status).toBe(200); + expect(new Set(res.body.signers)).toEqual(new Set([signer1.publicKey(), signer2.publicKey()])); + }); + + it('rejects a multisig challenge below the threshold, including single signers of differing weights', async () => { + const { accessToken } = await signupVerified('multisig-below'); + const clientKp = Keypair.random(); + const signer1 = Keypair.random(); // weight 1 + const signer2 = Keypair.random(); // weight 2 โ€” still below threshold 3 alone + accountOverrides.set(clientKp.publicKey(), { + accountId: clientKp.publicKey(), + thresholds: { low_threshold: 0, med_threshold: 3, high_threshold: 3 }, + signers: [ + { key: clientKp.publicKey(), weight: 1, type: 'ed25519_public_key' }, + { key: signer1.publicKey(), weight: 1, type: 'ed25519_public_key' }, + { key: signer2.publicKey(), weight: 2, type: 'ed25519_public_key' }, + ], + }); + + // The heaviest single signer (weight 2) still misses threshold 3โ€ฆ + const heavyOnly = await issueAndSign(accessToken, clientKp.publicKey(), signer2); + const heavyRes = await postVerify(accessToken, heavyOnly.signed); + expect(heavyRes.status).toBe(401); + expect(heavyRes.body.code).toBe('SEP10_INVALID_SIGNATURE'); + expect(heavyRes.body.message).toMatch(/threshold/); + + // โ€ฆand so does the lightest one. + const lightOnly = await issueAndSign(accessToken, clientKp.publicKey(), signer1); + const lightRes = await postVerify(accessToken, lightOnly.signed); + expect(lightRes.status).toBe(401); + expect(lightRes.body.code).toBe('SEP10_INVALID_SIGNATURE'); + }); + + it('accepts an UNFUNDED account only when the master key signs (SEP-10 spec)', async () => { + const { accessToken } = await signupVerified('unfunded'); + const unfundedKp = Keypair.random(); + accountOverrides.set(unfundedKp.publicKey(), null); // Horizon 404 โ€” not yet created + + // Spec rule: no signers/thresholds exist on-chain yet, so the master + // key of the client account must sign the challenge. + const { signed } = await issueAndSign(accessToken, unfundedKp.publicKey(), unfundedKp); + const ok = await postVerify(accessToken, signed); + expect(ok.status).toBe(200); + expect(ok.body.signers).toEqual([unfundedKp.publicKey()]); + + // A different key standing in for an unfunded account must fail. + const another = await issueAndSign(accessToken, unfundedKp.publicKey(), Keypair.random()); + const bad = await postVerify(accessToken, another.signed); + expect(bad.status).toBe(401); + expect(bad.body.code).toBe('SEP10_INVALID_SIGNATURE'); + }); + + // --- binding and the nonce ledger -------------------------------------- + + it('rejects a challenge issued to user A when user B submits it', async () => { + const a = await signupVerified('user-a'); + const b = await signupVerified('user-b'); + const clientKp = Keypair.random(); + + const { signed } = await issueAndSign(a.accessToken, clientKp.publicKey(), clientKp); + expect((await postVerify(a.accessToken, signed)).status).toBe(200); + + // Even ignoring consumption: B can never redeem A's challenge. Reissue + // a fresh one to demonstrate the binding, not the single-use rule. + const fresh = await issueAndSign(a.accessToken, clientKp.publicKey(), clientKp); + const res = await postVerify(b.accessToken, fresh.signed); + expect(res.status).toBe(403); + expect(res.body.code).toBe('SEP10_CHALLENGE_NOT_FOR_CALLER'); + }); + + it('rejects a challenge whose recorded account does not match the challenge tx', async () => { + const { accessToken } = await signupVerified('account-mismatch'); + const clientKp = Keypair.random(); + const otherKp = Keypair.random(); + + const { signed } = await issueAndSign(accessToken, clientKp.publicKey(), clientKp); + // Simulate a ledger row whose account binding no longer matches the + // transaction's client account (e.g. tampering or a bad migration). + await prisma.sep10Challenge.update({ + where: { nonce: challengeNonce(signed) }, + data: { account: otherKp.publicKey() }, + }); + + const res = await postVerify(accessToken, signed); + expect(res.status).toBe(403); + expect(res.body.code).toBe('SEP10_CHALLENGE_NOT_FOR_CALLER'); + }); + + it('rejects a structurally valid challenge that was never issued by this server', async () => { + const { accessToken } = await signupVerified('unknown-nonce'); + const clientKp = Keypair.random(); + + // Perfectly signed by the real server key โ€” but its nonce is not in + // our issuance ledger (nobody issued it, or the row was pruned). + const forged = WebAuth.buildChallengeTx(serverKp, clientKp.publicKey(), homeDomain, 300, networkPassphrase, webAuthDomain); + + const res = await postVerify(accessToken, signChallenge(forged, clientKp)); + expect(res.status).toBe(401); + expect(res.body.code).toBe('SEP10_CHALLENGE_UNKNOWN'); + }); + + it('rejects a tampered challenge that has extra client-sourced operations', async () => { + const { accessToken } = await signupVerified('tampered'); + const clientKp = Keypair.random(); + + const challengeRes = await getChallenge(accessToken, clientKp.publicKey()); + const challenge = challengeRes.body.transaction as string; + + // Rebuild the challenge with an added client-sourced op (invalid per + // SEP-10: extra ops must be server-sourced Manage Data), re-signed by + // the server key so only the structural rule can catch it. + const { Account, BASE_FEE, Operation, TransactionBuilder: TB } = await import('@stellar/stellar-sdk'); + const original = TransactionBuilder.fromXDR(challenge, networkPassphrase) as Transaction; + const tampered = new TB(new Account(serverKp.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase, + timebounds: { minTime: Number(original.timeBounds!.minTime), maxTime: Number(original.timeBounds!.maxTime) }, + }) + .addOperation( + Operation.manageData({ name: `${homeDomain} auth`, value: 'a'.repeat(64), source: clientKp.publicKey() }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: webAuthDomain, source: serverKp.publicKey() })) + .addOperation(Operation.manageData({ name: 'sneaky', value: 'x', source: clientKp.publicKey() })) + .build(); + tampered.sign(serverKp); + + const res = await postVerify(accessToken, signChallenge(tampered.toEnvelope().toXDR('base64'), clientKp)); + expect(res.status).toBe(400); + expect(res.body.code).toBe('SEP10_INVALID_CHALLENGE'); + }); +}); diff --git a/backend/tests/unit/sep10.test.mts b/backend/tests/unit/sep10.test.mts new file mode 100644 index 0000000..74f8915 --- /dev/null +++ b/backend/tests/unit/sep10.test.mts @@ -0,0 +1,311 @@ +// Unit tests for the SEP-10 server (issue #77) โ€” no database, no network. +// +// Two groups: +// 1. Configuration validation (startup check reports var NAMES, never +// secret values). +// 2. The structural half of the SEP-10 server verification list โ€” the +// cases WebAuth.readChallengeTx rejects (malformed XDR, wrong source, +// bad sequence, bad timebounds, wrong domains, extra operations, +// missing/wrong server signature). These fail BEFORE any DB access, +// so they run without the embedded Postgres the integration suite +// uses. Signature/threshold/binding/replay cases live in +// tests/integration/sep10.test.mts. +import { describe, expect, it, vi } from 'vitest'; +import { Keypair, StrKey, WebAuth } from '@stellar/stellar-sdk'; +import { HttpError } from '../../src/http-error.js'; +import { config } from '../../src/config/index.js'; +import { + createSep10Challenge, + sep10ConfigProblems, + validateSep10Config, + verifySep10Challenge, +} from '../../src/modules/payments/services/sep10.js'; + +const HOME_DOMAIN = config.sep10.homeDomain!; +const WEB_AUTH_DOMAIN = config.sep10.webAuthDomain!; +const NETWORK_PASSPHRASE = config.sep10.networkPassphrase; +const SERVER_KP = Keypair.fromSecret(config.sep10.signingSecret!); +const TIMEOUT = 300; + +async function expectHttpError(promise: Promise, status: number, code: string): Promise { + try { + await promise; + } catch (err) { + expect(err).toBeInstanceOf(HttpError); + const httpErr = err as HttpError; + expect(httpErr.status).toBe(status); + expect(httpErr.code).toBe(code); + return httpErr; + } + throw new Error(`expected the promise to reject with ${status} ${code}`); +} + +describe('SEP-10 config validation (issue #77)', () => { + it('passes with the test suite configuration', async () => { + await expect(validateSep10Config()).resolves.toBeUndefined(); + await expect(sep10ConfigProblems()).resolves.toEqual([]); + }); + + it('names each missing env var, and never echoes a secret value', async () => { + vi.resetModules(); + vi.stubEnv('SEP10_SIGNING_SECRET', ''); + vi.stubEnv('SEP10_HOME_DOMAIN', ''); + vi.stubEnv('SEP10_WEB_AUTH_DOMAIN', ''); + vi.stubEnv('SEP10_CHALLENGE_TIMEOUT_SECONDS', 'not-a-number'); + + const { sep10ConfigProblems: freshProblems } = await import('../../src/modules/payments/services/sep10.js'); + const problems = await freshProblems(); + + expect(problems).toContain('SEP10_SIGNING_SECRET'); + expect(problems).toContain('SEP10_HOME_DOMAIN'); + expect(problems).toContain('SEP10_WEB_AUTH_DOMAIN'); + expect(problems.join(' ')).toContain('SEP10_CHALLENGE_TIMEOUT_SECONDS'); + + vi.unstubAllEnvs(); + vi.resetModules(); + }); + + it('flags a malformed signing secret without printing it', async () => { + vi.resetModules(); + vi.stubEnv('SEP10_SIGNING_SECRET', 'Sdefinitelynotarealsecretkey'); + + const { sep10ConfigProblems: freshProblems } = await import('../../src/modules/payments/services/sep10.js'); + const problems = await freshProblems(); + + expect(problems).toContain('SEP10_SIGNING_SECRET (not a valid Stellar secret key)'); + expect(problems.join(' ')).not.toContain('Sdefinitelynotarealsecretkey'); + + vi.unstubAllEnvs(); + vi.resetModules(); + }); +}); + +describe('SEP-10 challenge account validation', () => { + it('rejects a G-shaped key that fails the strkey checksum', async () => { + // Valid public key with the last character flipped โ€” right shape, + // wrong CRC16 checksum. Must be caught server-side (400), not at + // wallet-signing time. + const valid = Keypair.random().publicKey(); + const corrupted = valid.slice(0, -1) + (valid.endsWith('A') ? 'B' : 'A'); + expect(StrKey.isValidEd25519PublicKey(corrupted)).toBe(false); + + const err = await expectHttpError(createSep10Challenge('user-id-unused', corrupted), 400, 'SEP10_INVALID_ACCOUNT'); + expect(err.message).toContain('valid Stellar public key'); + }); + + it('rejects non-G input (muxed Mโ€ฆ, lowercase, wrong length)', async () => { + for (const bad of ['GSHORT', 'g' + 'A'.repeat(55), Keypair.random().publicKey().toLowerCase(), 'M' + 'A'.repeat(55)]) { + await expectHttpError(createSep10Challenge('user-id-unused', bad), 400, 'SEP10_INVALID_ACCOUNT'); + } + }); +}); + +describe('SEP-10 server verification list โ€” structural checks (unit)', () => { + const clientKp = Keypair.random(); + + it('rejects garbage input that is not XDR', async () => { + await expectHttpError(verifySep10Challenge('user-id-unused', 'not-an-xdr-string'), 400, 'SEP10_INVALID_CHALLENGE'); + }); + + it('rejects a transaction whose source is not the server signing account', async () => { + // Built and signed by an attacker-controlled "server" key: the tx + // source is the attacker's account, so it is not our challenge. + const attackerServerKp = Keypair.random(); + const forged = WebAuth.buildChallengeTx( + attackerServerKp, + clientKp.publicKey(), + HOME_DOMAIN, + TIMEOUT, + NETWORK_PASSPHRASE, + WEB_AUTH_DOMAIN, + ); + const err = await expectHttpError(verifySep10Challenge('user-id-unused', forged), 400, 'SEP10_INVALID_CHALLENGE'); + expect(err.message).toContain('source account'); + }); + + it('rejects a challenge not signed by the server (wrong/missing server signature)', async () => { + // Correct source account, but signed by a key that is NOT the + // configured server key โ€” the server signature check must fail. + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const impostorKp = Keypair.random(); + const now = Math.floor(Date.now() / 1000); + const forged = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now, maxTime: now + TIMEOUT }, + }) + .addOperation( + Operation.manageData({ + name: `${HOME_DOMAIN} auth`, + value: 'a'.repeat(64), + source: clientKp.publicKey(), + }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: WEB_AUTH_DOMAIN, source: SERVER_KP.publicKey() })) + .build(); + forged.sign(impostorKp); + + const err = await expectHttpError( + verifySep10Challenge('user-id-unused', forged.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + expect(err.message).toContain('signed by server'); + }); + + it('rejects a non-zero sequence number', async () => { + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + const forged = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '0'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now, maxTime: now + TIMEOUT }, + }) + .addOperation( + Operation.manageData({ + name: `${HOME_DOMAIN} auth`, + value: 'a'.repeat(64), + source: clientKp.publicKey(), + }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: WEB_AUTH_DOMAIN, source: SERVER_KP.publicKey() })) + .build(); + forged.sign(SERVER_KP); + + const err = await expectHttpError( + verifySep10Challenge('user-id-unused', forged.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + expect(err.message).toContain('sequence number'); + }); + + it('rejects expired time bounds', async () => { + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + // Expired well beyond the SDK's 5-minute clock-drift grace window. + const expired = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now - 1800, maxTime: now - 1200 }, + }) + .addOperation( + Operation.manageData({ + name: `${HOME_DOMAIN} auth`, + value: 'a'.repeat(64), + source: clientKp.publicKey(), + }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: WEB_AUTH_DOMAIN, source: SERVER_KP.publicKey() })) + .build(); + expired.sign(SERVER_KP); + + const err = await expectHttpError( + verifySep10Challenge('user-id-unused', expired.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + expect(err.message).toContain('expired'); + }); + + it('rejects a challenge whose Manage Data key uses a different home domain', async () => { + const forged = WebAuth.buildChallengeTx( + SERVER_KP, + clientKp.publicKey(), + 'evil.example', + TIMEOUT, + NETWORK_PASSPHRASE, + WEB_AUTH_DOMAIN, + ); + const err = await expectHttpError(verifySep10Challenge('user-id-unused', forged), 400, 'SEP10_INVALID_CHALLENGE'); + expect(err.message).toContain('home domain'); + }); + + it('rejects a challenge whose web_auth_domain operation value is wrong', async () => { + const forged = WebAuth.buildChallengeTx( + SERVER_KP, + clientKp.publicKey(), + HOME_DOMAIN, + TIMEOUT, + NETWORK_PASSPHRASE, + 'evil.example', // web_auth_domain op will carry this, not ours + ); + await expectHttpError(verifySep10Challenge('user-id-unused', forged), 400, 'SEP10_INVALID_CHALLENGE'); + }); + + it('rejects extra operations that are not server-sourced Manage Data', async () => { + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + const forged = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now, maxTime: now + TIMEOUT }, + }) + .addOperation( + Operation.manageData({ + name: `${HOME_DOMAIN} auth`, + value: 'a'.repeat(64), + source: clientKp.publicKey(), + }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: WEB_AUTH_DOMAIN, source: SERVER_KP.publicKey() })) + // A client-sourced extra op โ€” a SEP-10 challenge only ever allows + // server-sourced additional operations (plus client_domain). + .addOperation( + Operation.manageData({ name: 'sneaky', value: 'x', source: clientKp.publicKey() }), + ) + .build(); + forged.sign(SERVER_KP); + + await expectHttpError( + verifySep10Challenge('user-id-unused', forged.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + }); + + it('rejects a challenge whose first operation is not Manage Data', async () => { + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + const forged = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now, maxTime: now + TIMEOUT }, + }) + .addOperation(Operation.bumpSequence({ bumpTo: '1', source: clientKp.publicKey() })) + .build(); + forged.sign(SERVER_KP); + + await expectHttpError( + verifySep10Challenge('user-id-unused', forged.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + }); + + it('rejects a challenge with a nonce that is not 48-random-bytes base64', async () => { + const { Account, BASE_FEE, Operation, TransactionBuilder } = await import('@stellar/stellar-sdk'); + const now = Math.floor(Date.now() / 1000); + const forged = new TransactionBuilder(new Account(SERVER_KP.publicKey(), '-1'), { + fee: BASE_FEE, + networkPassphrase: NETWORK_PASSPHRASE, + timebounds: { minTime: now, maxTime: now + TIMEOUT }, + }) + .addOperation( + Operation.manageData({ + name: `${HOME_DOMAIN} auth`, + value: 'too-short', // not 48 bytes once base64-decoded + source: clientKp.publicKey(), + }), + ) + .addOperation(Operation.manageData({ name: 'web_auth_domain', value: WEB_AUTH_DOMAIN, source: SERVER_KP.publicKey() })) + .build(); + forged.sign(SERVER_KP); + + await expectHttpError( + verifySep10Challenge('user-id-unused', forged.toEnvelope().toXDR('base64')), + 400, + 'SEP10_INVALID_CHALLENGE', + ); + }); +}); diff --git a/backend/vitest.config.mts b/backend/vitest.config.mts index c500601..e99c540 100644 --- a/backend/vitest.config.mts +++ b/backend/vitest.config.mts @@ -10,6 +10,12 @@ export default defineConfig({ env: { JWT_ACCESS_SECRET: 'test-access-secret', JWT_REFRESH_SECRET: 'test-refresh-secret', + // SEP-10 (issue #77) โ€” a throwaway keypair used ONLY by the test + // suite; it holds no funds and exists to make the challenge/verify + // code paths deterministic offline. Not a real deployment secret. + SEP10_SIGNING_SECRET: 'SC3Z2YBPD7KGXC73CTBNDRGQOIUSAZUH445QIB2PPKWO7CXULRY75JGJ', + SEP10_HOME_DOMAIN: 'docforum.test', + SEP10_WEB_AUTH_DOMAIN: 'api.docforum.test', // Port 5433, not 5432 โ€” tests/integration/booking.test.mts spins up // its own throwaway Postgres cluster on this port so the suite is // self-contained (no pre-existing DB required). A distinct port diff --git a/docs/api/README.md b/docs/api/README.md index ee29323..0a21c2c 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -151,9 +151,76 @@ 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. | +## Payments (`/payments`) โ€” `backend/src/modules/payments` + +Custodial v1 escrow actions (`POST /payments/wallet-link`, +`GET/POST /payments/intents/...`) exist and are admin/facility-gated โ€” +see `docs/adr/0004-custodial-payments-v1.md` for why. The table below +covers the SEP-10 endpoints added for the non-custodial flow +(`docs/adr/0005`, issue #77). + +### SEP-10 Stellar Web Authentication (issue #77) + +Proves the caller controls a Stellar address before it can ever be +trusted as a payer or payee. **Both endpoints require a verified-email +account** (same guard as every payments mutation โ€” issue #34), and +neither issues a DocForum session: your normal access token +authenticates the *user*; SEP-10 proves control of the *wallet*. + +| Method & path | Auth | Body/query | Notes | +|---|---|---|---| +| `GET /payments/sep10/challenge` | access token (+ verified email) | `?account=Gโ€ฆ` | Returns `200 { transaction, expires_in, network_passphrase }` โ€” the base64 XDR of a server-signed SEP-10 challenge (5-minute time bounds by default, `SEP10_CHALLENGE_TIMEOUT_SECONDS`). Hand `transaction` to the user's wallet for signing (build/sign/submit all stay server-side per `docforum-web`'s hard rule โ€” the browser only relays XDR). `400` if `account` is missing, malformed, or fails the Stellar strkey checksum (`code: 'SEP10_INVALID_ACCOUNT'`). | +| `POST /payments/sep10/verify` | access token (+ verified email) | `{ transaction }` (signed challenge XDR) | Runs the full SEP-10 server verification list plus this server's own single-use/binding rules. Returns `200 { account, signers, userId }` on success, and the nonce is consumed โ€” a challenge is single-use, even concurrently. | + +**What verify checks, in order** (SEP-10's server verification list): + +1. XDR deserializes as a transaction; sequence number is 0; source is + the server's SEP-10 signing account; time bounds are present, bounded + and unexpired. +2. The first operation is Manage Data, sourced by `account`, keyed + ` auth`, with a spec-sized (48-random-bytes base64) + nonce; later operations are server-sourced Manage Data including + `web_auth_domain` equal to this server's configured domain. +3. The server's signature on the challenge is valid. +4. The nonce exists in this server's issuance ledger (`Sep10Challenge`), + is still unconsumed and unexpired, and is bound to **the calling user + and the exact account in the challenge** โ€” a signed challenge is + never redeemable by another user or for another account. +5. Client signatures: for a **funded** account, signer weights loaded + from Horizon must meet the account's **medium threshold**; + for an **unfunded account** (not yet created on the network โ€” no + signers/thresholds exist to weigh), the challenge must be signed by + the account's **master key**, per the SEP-10 spec. A Horizon outage is + a `502`, never silently treated as "unfunded". +6. The nonce is consumed with a single conditional UPDATE โ€” of several + concurrent submissions of the same signed challenge, exactly one + succeeds. + +**Errors from `POST /payments/sep10/verify`** (all also carry a `message`): + +| Status | `code` | Meaning | +|---|---|---| +| 400 | `SEP10_INVALID_CHALLENGE` | Not a valid/well-formed SEP-10 challenge: bad XDR, wrong source, non-zero sequence, expired time bounds, wrong home domain or `web_auth_domain`, extra operations, or a missing/wrong server signature. | +| 401 | `SEP10_INVALID_SIGNATURE` | Well-formed challenge, but the client signature is missing, from a non-signer, or below the account's medium threshold (incl. the unfunded-account master-key rule). A failed verification does NOT burn the nonce. | +| 401 | `SEP10_CHALLENGE_UNKNOWN` | The challenge's nonce was never issued by this server (or its record was pruned). | +| 401 | `SEP10_CHALLENGE_EXPIRED` | The issued nonce record is past its expiry (stricter than the transaction time bounds). | +| 401 | `SEP10_CHALLENGE_ALREADY_USED` | Replay โ€” the nonce is already consumed. | +| 403 | `SEP10_CHALLENGE_NOT_FOR_CALLER` | The challenge was issued to a different user or for a different account. | +| 401 / 403 | โ€” | Standard auth failures: no/invalid access token (`401`), unverified email (`403 EMAIL_NOT_VERIFIED`). | + +**Configuration** (see `.env.example`; the first three are required at +server startup โ€” the server refuses to boot without them, and secrets +are never logged): `SEP10_SIGNING_SECRET` (a dedicated key, separate +from `PLATFORM_PAYER_SECRET`/`PLATFORM_RELEASER_SECRET` โ€” it only signs +web-auth challenges, so it can be rotated without touching money-moving +keys), `SEP10_HOME_DOMAIN`, `SEP10_WEB_AUTH_DOMAIN`, plus optional +`SEP10_NETWORK_PASSPHRASE`, `SEP10_HORIZON_URL` (both default to +testnet) and `SEP10_CHALLENGE_TIMEOUT_SECONDS` (default 300). + ## Not yet implemented Everything else in `ARCHITECTURE.md` ยง3's module list (`patients`, `facilities`, `consultations`, `referrals`, `prescriptions`, `lab-orders`, -`pharmacy-orders`, `payments`, `notifications`) has no routes yet โ€” Phase -2 onward, see `ROADMAP.md`. +`pharmacy-orders`, `notifications`) has no routes yet โ€” Phase +2 onward, see `ROADMAP.md`. (`payments` has routes โ€” see the Payments +section above.) From fda820ba95ffdbbfa5c977250ab41eacfa313297 Mon Sep 17 00:00:00 2001 From: affy-exe Date: Fri, 16 Oct 2026 16:23:07 +0000 Subject: [PATCH 2/2] feat(payments): verified wallet links for patients and facilities (replace typed-in addresses) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the facility-only, typed-in wallet address with SEP-10-verified owner-scoped links (issue #78, ADR 0005 step 4), stacked on the #77 SEP-10 branch. - Schema: WalletLink generalized to ownerType (patient | facility) + ownerId with verificationMethod ('sep10' | 'unverified_legacy'), verifiedAt, replacedAt and a unique activeOwnerKey sentinel โ€” one ACTIVE link per owner at the DB level (Prisma cannot express a partial unique index; the sentinel's unique index can, and survives migrate dev). Re-linking deactivates instead of mutating/deleting. PaymentIntent gains fundedWalletAddress, the payee snapshot written once at funding. - Migration: hand-ordered so it is safe on populated tables; existing facility rows backfill to active unverified_legacy links (proven by a migration test that inserts an old-shape row, applies only this migration, and asserts the backfill + constraint participation). - POST /payments/wallet-link: strict body { transaction } only โ€” a raw address is a 400. Owner resolved from the caller's own profile (403 otherwise); address derived from the verified challenge; link creation and the SEP-10 nonce consumption commit in ONE transaction (verifySep10Challenge now takes a transaction client), so a failed verification creates no link and burns no nonce. - Patient link responses carry ADR 0005's on-chain privacy notice verbatim (unit-tested against the ADR file so it cannot drift). - Funding guard: fundPaymentIntent refuses non-sep10 facility links after WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF (default 2027-01-01, startup-validated, documented in .env.example) with 409 WALLET_LINK_UNVERIFIED, and snapshots fundedWalletAddress so re-linking never rewrites an escrow's payee. - Tests: 8 unit + 12 integration (fake Horizon + fake escrow SDK โ€” no network): happy paths both roles, strict body, failed/replayed/wrong- user verification, DB constraint + concurrent creates, re-link escrow isolation, cutoff refusal/allowance, migration backfill. - Docs: ARCHITECTURE.md ยง4.11, ARCHITECTURE_ESSENTIALS payments section, docs/api/README.md wallet-link reference, ROADMAP Phase 5.5 + changelog. Closes #78 ๐Ÿค– Generated with Codebuff Co-Authored-By: Codebuff --- ARCHITECTURE.md | 40 ++ ARCHITECTURE_ESSENTIALS.md | 3 +- ROADMAP.md | 19 + backend/.env.example | 9 + .../migration.sql | 66 ++ backend/prisma/schema.prisma | 87 ++- backend/src/config/index.ts | 22 + .../src/modules/payments/controllers/index.ts | 21 +- .../modules/payments/repositories/index.ts | 91 ++- backend/src/modules/payments/routes/index.ts | 19 +- .../payments/services/escrow-sdk-loader.ts | 17 + .../src/modules/payments/services/index.ts | 50 +- .../src/modules/payments/services/sep10.ts | 25 +- .../modules/payments/services/wallet-link.ts | 173 ++++++ backend/src/modules/payments/validation.ts | 19 +- backend/src/server.ts | 5 +- .../tests/integration/wallet-link.test.mts | 570 ++++++++++++++++++ .../testnet-integration/payments.test.mts | 16 +- backend/tests/unit/wallet-link.test.mts | 97 +++ docs/api/README.md | 61 +- 20 files changed, 1337 insertions(+), 73 deletions(-) create mode 100644 backend/prisma/migrations/20261016153822_add_verified_wallet_links/migration.sql create mode 100644 backend/src/modules/payments/services/wallet-link.ts create mode 100644 backend/tests/integration/wallet-link.test.mts create mode 100644 backend/tests/unit/wallet-link.test.mts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 2883ab5..36583f3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -227,6 +227,46 @@ Sep10Challenge SEP-10 ledger is the first payments model added directly here because it is a user-identity-adjacent table with a FK to `User`.) +### 4.11 Verified wallet links (Phase 5.5, issue #78) +``` +WalletLink + id, owner_type (enum: patient|facility), owner_id (PatientProfile.id or + FacilityProfile.id โ€” polymorphic, deliberately NO FK relation, same + shape as ยง4.7's FulfillmentRecord; caller ownership is validated in + the payments service before any write) + stellar_address, verification_method (enum: sep10 | unverified_legacy), + verified_at (null iff unverified_legacy), replaced_at (null while + this is the owner's active link), active_owner_key (unique sentinel), + created_at, updated_at + # ONE ACTIVE LINK PER OWNER, DB-level: active_owner_key = + ":" while active, NULL once replaced โ€” the + unique index on it is the constraint (Prisma's schema DSL cannot + express a partial unique index WHERE replaced_at IS NULL, and a + raw-SQL one would be dropped as drift by the next `prisma migrate + dev`). + # Re-linking DEACTIVATES (replaced_at) instead of mutating or + deleting, so history survives. stellar_address and + verification_method are write-once. `sep10` rows are created ONLY + by POST /payments/wallet-link's verification flow (inside the same + transaction that consumes the SEP-10 nonce); there is no seed, + script, admin route or service method that can create one โ€” the + `unverified_legacy` value only ever comes from the + add_verified_wallet_links migration backfill of old typed-in rows. + # Patient link responses carry ADR 0005's on-chain privacy notice + VERBATIM (constant in services/wallet-link.ts, unit-tested against + the ADR file itself so it can't drift into a paraphrase). + +PaymentIntent (base fields documented in ADR 0004 ยง4 context) + funded_wallet_address โ€” NEW (issue #78): snapshot of the payee address + the escrow was actually funded to, written exactly once with the + `escrowed` transition. A later re-link cannot rewrite which address + an already-funded escrow references. +``` +Funding guard: after `WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF` (config, +validated at startup), `fundPaymentIntent` refuses facility links whose +`verification_method` is not `sep10` with `409 WALLET_LINK_UNVERIFIED` โ€” +enforced in the funding service, not the controller. + ## 5. Key flows (sequence-level, not code) ### 5.1 Booking (concurrency-safe) diff --git a/ARCHITECTURE_ESSENTIALS.md b/ARCHITECTURE_ESSENTIALS.md index f234540..3cffea6 100644 --- a/ARCHITECTURE_ESSENTIALS.md +++ b/ARCHITECTURE_ESSENTIALS.md @@ -27,7 +27,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc `FulfillmentRecord` (facility-owned) references an order but doesn't mutate it. `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). -`FacilityProfile` โ†’ `WalletLink` (payout address) ; `PatientProfile`/`FacilityProfile` โ†’ `PaymentIntent` (opaque `orderType`/`orderId` โ€” see docs/adr/0004) +`FacilityProfile`/`PatientProfile` โ†’ `WalletLink` (owner-scoped wallet link, issue #78 โ€” one ACTIVE per owner (DB-enforced sentinel unique), `verificationMethod` `sep10` | `unverified_legacy`, re-link deactivates instead of mutating) ; `PatientProfile`/`FacilityProfile` โ†’ `PaymentIntent` (opaque `orderType`/`orderId` โ€” see docs/adr/0004; snapshots `fundedWalletAddress` at funding) `User` โ†’ `Sep10Challenge` (SEP-10 wallet-ownership proofs, issue #77 โ€” single-use nonce binding a signed challenge to the issuing user + Stellar account; consumed atomically) ## Payments (Phase 5.5 โ€” see docs/adr/0004-custodial-payments-v1.md) @@ -36,6 +36,7 @@ Quick-reference only. Full reasoning lives in `ARCHITECTURE.md`. Full product sc - `PaymentIntent.orderType`/`orderId` are opaque, unvalidated โ€” `Prescription`/`LabOrder` aren't real records yet (Phase 4). Don't build on top of these assuming they're FK-validated. - `release`/`refund` are admin-triggered HTTP actions in v1 (`POST /payments/intents/:id/{release,refund}`) โ€” there's no `FulfillmentRecord.status == fulfilled` to trigger off of yet. Don't treat this as the final design. - **SEP-10 wallet-ownership proof** (issue #77): `GET /payments/sep10/challenge` + `POST /payments/sep10/verify` (see docs/api/README.md) with a dedicated `SEP10_SIGNING_SECRET` (never the payer/releaser keys). The `Sep10Challenge` nonce ledger is single-use via one conditional UPDATE โ€” no app-level check-then-write. This is generic infrastructure for ADR 0005's wallet linking; it issues no session, DocForum's own JWT still authenticates the caller. +- **Verified wallet links** (issue #78): `POST /payments/wallet-link` accepts ONLY a signed SEP-10 challenge โ€” never a raw address; the address is derived from the verified challenge, and link creation + nonce consumption commit in ONE transaction. Patient responses carry ADR 0005's on-chain privacy notice verbatim. Funding refuses `unverified_legacy` facility links after `WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF` (`409 WALLET_LINK_UNVERIFIED`, enforced in the funding service) and snapshots `PaymentIntent.fundedWalletAddress` so re-linking never rewrites an escrow's payee. - The actual Stellar contract calls go through `@docforum/escrow-sdk` (`backend/src/modules/payments/services/escrow-sdk-loader.ts` โ€” dynamic `import()`, the SDK is ESM-only and this backend is CommonJS). Never reimplement contract-calling logic in this repo. ## Known unresolved design gaps (do not silently "fix" without flagging in PR description) diff --git a/ROADMAP.md b/ROADMAP.md index 6ad2cb3..c0a851a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -88,6 +88,7 @@ for status and issue links. - [ ] ADR: releaser key custody โ€” multisig, KMS, or per-period rotation (threat model T1/T2). Tracked as [issue #25](https://github.com/DocForum/docforum-core/issues/25) (Medium, 150 pts). - [ ] Checksum-validate facility wallet keys, not just regex. Tracked as [issue #26](https://github.com/DocForum/docforum-core/issues/26) (Trivial, 100 pts). - [x] SEP-10 Stellar Web Authentication server (ADR 0005 step 4): `GET /payments/sep10/challenge` + `POST /payments/sep10/verify`, full SEP-10 server verification list (incl. multisig thresholds from Horizon and the unfunded-account master-key rule), a single-use `Sep10Challenge` nonce ledger consumed atomically, and a dedicated `SEP10_SIGNING_SECRET` separate from the payer/releaser keys. Generic infrastructure the non-custodial wallet linking builds on โ€” the endpoints themselves issue no session and change no custodial behaviour. Tracked as [issue #77](https://github.com/DocForum/docforum-core/issues/77) (High, 200 pts) โ€” **Done**. +- [x] Verified wallet links for patients and facilities (ADR 0005 step 4, replaces typed-in addresses): generalized owner-scoped `WalletLink` (patient | facility) with `verificationMethod`/`verifiedAt`, one ACTIVE link per owner DB-enforced, re-link deactivates instead of mutating, `POST /payments/wallet-link` accepts only a signed SEP-10 challenge (link + nonce consume in one transaction), patient responses carry ADR 0005's privacy notice verbatim, funding refuses unverified legacy links after a configurable `WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF`, and `PaymentIntent.fundedWalletAddress` snapshots the funded payee. Tracked as [issue #78](https://github.com/DocForum/docforum-core/issues/78) (High, 200 pts) โ€” **Done** (stacked on #77's branch). - [ ] Non-custodial patient payments (v2): patients fund escrows from their own Stellar wallet, custodial kept as fallback. **Proposed** in `docs/adr/0005-non-custodial-patient-payments.md`; implementation issues to be opened once it's Accepted. - [ ] Upgrade to `@docforum/escrow-sdk` 0.3.0 and the current testnet contract (`refund_after`, balance-delta check, `getEscrow`) โ€” step 1 of ADR 0005's plan, but worth doing regardless. Tracked as [issue #32](https://github.com/DocForum/docforum-core/issues/32) (Medium, 150 pts); depends on `docforum-escrow` publishing `sdk-v0.3.0`. - [ ] Doc reconciliation: PRD ยง8.1.6 (facility fulfillment integrity) โ€” not marked resolved; this phase proves the payment *mechanism* works, not the fulfillment-integrity question, which still needs real `FulfillmentRecord`/facility-matching (Phase 5) to actually address. @@ -361,6 +362,24 @@ for status and issue links. seam so the 15 unit + 18 integration tests run with zero network. Docs updated: ARCHITECTURE ยง4.10, essentials data-model map, docs/api/README.md (endpoints + errors + unfunded behaviour). +- 2026-10-16 โ€” Implemented issue #78 (verified wallet links, stacked on + #77's branch): `WalletLink` generalized from facility-only/typed-in to + an owner (patient | facility) with `verificationMethod` / + `verifiedAt` / `replacedAt`, one active link per owner enforced by a + DB unique sentinel (Prisma can't express a partial index โ€” noted in + the schema), and the migration backfills existing facility rows as + `unverified_legacy` (proven by a real migration test that inserts an + old-shape row first). `POST /payments/wallet-link` now takes only a + signed SEP-10 challenge (strict body โ€” a raw address is a 400) and + creates the link inside the same transaction that consumes the nonce; + the only `sep10`-row writer is that flow. Patient link responses + carry ADR 0005's on-chain privacy notice verbatim (unit-tested + against the ADR file). Funding refuses unverified legacy links after + `WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF` (default 2027-01-01, + startup-validated, `409 WALLET_LINK_UNVERIFIED`) and snapshots + `PaymentIntent.fundedWalletAddress` so re-linking can't rewrite an + escrow's payee. 8 unit + 12 integration tests (fake Horizon + fake + escrow SDK โ€” no network). ADR 0005 unchanged (no factual conflict). - 2026-10-15 โ€” Started Phase 8's auth-hardening work, scoped as issues #34 (email verification), #35 (login brute-force throttling), #37 (CSRF for the cookie-authenticated endpoints + per-environment CORS allowlist) and #39 (admin account lifecycle). All four are independent of Phases 3โ€“7 and of each other. #34 merged as PR #137: `User.emailVerifiedAt` + diff --git a/backend/.env.example b/backend/.env.example index 2e034af..05b1eb2 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -65,3 +65,12 @@ SEP10_WEB_AUTH_DOMAIN=docforum-core.onrender.com # SEP10_HORIZON_URL=https://horizon-testnet.stellar.org # Challenge time bounds in seconds (default 300): # SEP10_CHALLENGE_TIMEOUT_SECONDS=300 + +# Verified wallet links (issue #78) โ€” after this instant, escrow funding +# only accepts facility wallet links whose verificationMethod is 'sep10'; +# links backfilled from the old typed-in form ('unverified_legacy') are +# refused with 409 WALLET_LINK_UNVERIFIED. Before the cutoff they still +# work. ISO date (UTC). Validated at startup โ€” a malformed value stops +# boot. Default when unset: 2027-01-01T00:00:00Z (explicit fail-closed +# grace window โ€” set your own per deployment). +WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF=2027-01-01T00:00:00Z diff --git a/backend/prisma/migrations/20261016153822_add_verified_wallet_links/migration.sql b/backend/prisma/migrations/20261016153822_add_verified_wallet_links/migration.sql new file mode 100644 index 0000000..6429b55 --- /dev/null +++ b/backend/prisma/migrations/20261016153822_add_verified_wallet_links/migration.sql @@ -0,0 +1,66 @@ +-- Verified wallet links (issue #78): generalize WalletLink from +-- facility-only/typed-in to an owner (patient | facility) with a +-- verification method, and snapshot the funded payee address on +-- PaymentIntent. +-- +-- Hand-ordered (generated by `prisma migrate diff`, then rewritten): +-- the generated script added NOT NULL columns and dropped `facilityId` +-- before any backfill, which fails on a populated table. This version +-- adds columns nullable โ†’ backfills from `facilityId` โ†’ constrains โ†’ +-- only then drops the old shape, so it is safe to run against existing +-- data (every pre-existing facility row becomes an active +-- `unverified_legacy` link owned by its facility). +-- +-- Reversibility: reversing back to the old shape is practical only for +-- `ownerType = 'facility'` rows (recreate `facilityId` from `ownerId`, +-- restore the FK/unique index); patient-era rows and verified metadata +-- have no old-shape home, so a down migration would drop them. Not +-- written for that reason โ€” restore from backup if reversal is needed. + +-- CreateEnum +CREATE TYPE "WalletLinkOwnerType" AS ENUM ('patient', 'facility'); + +-- CreateEnum +CREATE TYPE "WalletLinkVerificationMethod" AS ENUM ('sep10', 'unverified_legacy'); + +-- AlterTable: PaymentIntent gains the funding-time payee snapshot (null +-- until the next funding; no backfill possible โ€” historical intents +-- never recorded their payee address). +ALTER TABLE "PaymentIntent" ADD COLUMN "fundedWalletAddress" TEXT; + +-- AlterTable: new columns nullable first so rows can be backfilled. +ALTER TABLE "WalletLink" ADD COLUMN "activeOwnerKey" TEXT; +ALTER TABLE "WalletLink" ADD COLUMN "ownerId" TEXT; +ALTER TABLE "WalletLink" ADD COLUMN "ownerType" "WalletLinkOwnerType"; +ALTER TABLE "WalletLink" ADD COLUMN "replacedAt" TIMESTAMP(3); +ALTER TABLE "WalletLink" ADD COLUMN "verificationMethod" "WalletLinkVerificationMethod"; +ALTER TABLE "WalletLink" ADD COLUMN "verifiedAt" TIMESTAMP(3); + +-- Backfill: every existing row was a facility link typed in by hand โ†’ +-- active, owned by its facility, explicitly marked unverified. Their +-- addresses were never SEP-10-checked, so `verifiedAt` stays NULL and +-- funding honours them only until WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF. +UPDATE "WalletLink" +SET "ownerType" = 'facility', + "ownerId" = "facilityId", + "verificationMethod" = 'unverified_legacy', + "activeOwnerKey" = 'facility:' || "facilityId" +WHERE "facilityId" IS NOT NULL; + +-- Now constrain (all rows backfilled above; table empty if fresh). +ALTER TABLE "WalletLink" ALTER COLUMN "ownerType" SET NOT NULL; +ALTER TABLE "WalletLink" ALTER COLUMN "ownerId" SET NOT NULL; +ALTER TABLE "WalletLink" ALTER COLUMN "verificationMethod" SET NOT NULL; + +-- Drop old facility-only shape (FK must go before the column). +ALTER TABLE "WalletLink" DROP CONSTRAINT "WalletLink_facilityId_fkey"; +DROP INDEX "WalletLink_facilityId_key"; +ALTER TABLE "WalletLink" DROP COLUMN "facilityId"; + +-- CreateIndex +-- The one-active-link-per-owner DB guarantee: unique on the sentinel +-- column that is non-NULL exactly while the link is active. +CREATE UNIQUE INDEX "WalletLink_activeOwnerKey_key" ON "WalletLink"("activeOwnerKey"); + +-- CreateIndex +CREATE INDEX "WalletLink_ownerType_ownerId_idx" ON "WalletLink"("ownerType", "ownerId"); diff --git a/backend/prisma/schema.prisma b/backend/prisma/schema.prisma index 8564d28..1229e70 100644 --- a/backend/prisma/schema.prisma +++ b/backend/prisma/schema.prisma @@ -229,7 +229,11 @@ model FacilityProfile { user User @relation(fields: [userId], references: [id], onDelete: Cascade) capabilities FacilityCapability[] paymentIntents PaymentIntent[] - walletLink WalletLink? + // NOTE: no `walletLink` relation anymore โ€” WalletLink is generalized to + // a polymorphic owner (patient | facility) as of issue #78, so it + // carries no FK relation to either profile (same shape as + // FulfillmentRecord's polymorphic order ref, ยง6.1.3). Ownership is + // validated in the payments service, not by the schema. } model FacilityCapability { @@ -429,6 +433,14 @@ model PaymentIntent { // release/refund against the right on-chain escrow. escrowId BigInt? stellarTxHash String? + // Snapshot of the payee address this intent was actually funded to + // (issue #78). Set at funding time from the facility's then-ACTIVE + // wallet link and never changed afterwards โ€” so re-linking a + // different address later cannot rewrite which wallet an already- + // funded escrow paid, and the escrow keeps referencing its link even + // after that link is replaced. Append-only-in-spirit: written exactly + // once, with the funding transition. + fundedWalletAddress String? // Which deployed contract this intent was funded against โ€” this repo // has already had two testnet contract IDs (Phase E1 vs. E2), so this // isn't hypothetical. @@ -475,17 +487,72 @@ model Sep10Challenge { @@index([expiresAt]) } -model WalletLink { - id String @id @default(uuid()) - // Scoped to the facility being paid, not the patient โ€” v1 is a - // custodial model (docforum-core funds escrows from its own platform - // Stellar identity; patients never hold or sign with a wallet here). - // See ADR 0004. - facilityId String @unique - facility FacilityProfile @relation(fields: [facilityId], references: [id], onDelete: Cascade) +// Verified wallet links (issue #78, ADR 0005). Generalized from the +// facility-only, typed-in-address shape: an owner is a PatientProfile or +// a FacilityProfile (polymorphic ownerType+ownerId โ€” deliberately no FK +// relation, same shape as FulfillmentRecord's polymorphic order ref), +// and every link carries HOW it was verified. +// +// A link row is immutable except for `replacedAt`/`activeOwnerKey`: +// re-linking deactivates the old row and inserts a new one, so history +// (including the address escrows were actually funded to โ€” see +// PaymentIntent.fundedWalletAddress) is preserved rather than mutated. +// +// `verificationMethod`: +// - `sep10` โ€” created ONLY by the POST /payments/wallet-link +// verification flow (services/wallet-link.ts), inside the same +// transaction that consumes the SEP-10 nonce. No seed, script, admin +// route or internal service method may create a `sep10` row โ€” the +// constructor for it is private to that flow. +// - `unverified_legacy` โ€” backfilled by the add_verified_wallet_links +// migration for pre-existing facility rows (typed-in addresses). +// Usable for funding only until +// WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF passes. +// +// One ACTIVE link per owner, enforced at the DB level via +// `activeOwnerKey`: it holds ":" while active and +// NULL once replaced. A plain unique constraint on it gives exactly the +// "at most one unreplaced row per owner" guarantee โ€” Prisma's schema DSL +// cannot express a partial unique index (`WHERE replacedAt IS NULL`), +// and a raw-SQL index would be silently dropped by the next +// `prisma migrate dev` as schema drift. Postgres treats NULLs as +// distinct, so any number of replaced rows can coexist. +enum WalletLinkOwnerType { + patient + facility +} - stellarPublicKey String +enum WalletLinkVerificationMethod { + sep10 + unverified_legacy +} + +model WalletLink { + id String @id @default(uuid()) + ownerType WalletLinkOwnerType + // PatientProfile.id or FacilityProfile.id. Polymorphic โ€” no FK on + // purpose (see note above); existence + caller ownership are checked + // in services/wallet-link.ts before any write. + ownerId String + // Gโ€ฆ address. NEVER accepted from a client-supplied field: derived + // server-side from the account the SEP-10 challenge was issued for and + // successfully verified. + stellarPublicKey String + verificationMethod WalletLinkVerificationMethod + // Set iff verificationMethod = 'sep10' (time the challenge passed). + verifiedAt DateTime? + // Set when a newer link superseded this one. Only these two fields + // ever change on a row; stellarPublicKey/verificationMethod are + // write-once. + replacedAt DateTime? + // ":" while this is the owner's active link, NULL + // once replaced. The unique index on this column IS the one-active- + // link-per-owner DB guarantee. + activeOwnerKey String? @unique createdAt DateTime @default(now()) updatedAt DateTime @updatedAt + + // Lookups by owner (active-link read path, deactivation sweep). + @@index([ownerType, ownerId]) } diff --git a/backend/src/config/index.ts b/backend/src/config/index.ts index 706bb64..1fcbf15 100644 --- a/backend/src/config/index.ts +++ b/backend/src/config/index.ts @@ -28,6 +28,17 @@ function parseCorsOrigins(raw: string | undefined, isProduction: boolean): strin .filter((origin) => origin.length > 0); } +/** Issue #78: the date after which funding refuses facility wallet links + * that were never SEP-10 verified. Defaults to 2027-01-01 (UTC) โ€” a + * deliberate fail-closed default: legacy links keep working for a + * defined grace window without any deployment needing to remember to + * set the variable, but "never enforce" is never the default. Returns + * NaN for a malformed value so startup validation can name the var. */ +function parseWalletLinkCutoff(raw: string | undefined): number { + if (raw === undefined || raw.trim() === '') return Date.parse('2027-01-01T00:00:00Z'); + return Date.parse(raw); +} + export const config = { port: Number(process.env.PORT ?? 4000), databaseUrl: requireEnv('DATABASE_URL'), @@ -117,6 +128,17 @@ export const config = { platformReleaserSecret: process.env.PLATFORM_RELEASER_SECRET, escrowContractId: process.env.ESCROW_CONTRACT_ID, escrowTokenId: process.env.ESCROW_TOKEN_ID, + // Issue #78 โ€” after this instant, funding only accepts facility + // wallet links with verificationMethod = 'sep10'; before it, links + // backfilled from the old typed-in form (`unverified_legacy`) still + // work. Parsed leniently here (invalid input yields NaN); startup + // validation (validateWalletLinkConfig, server.ts) rejects a + // malformed value by env var NAME, and the funding service refuses + // to guess if it somehow sees NaN (fail-closed 500, not a silent + // pass). + walletLinkUnverifiedFundingCutoff: parseWalletLinkCutoff( + process.env.WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF, + ), }, // SEP-10 Stellar Web Authentication (issue #77, ADR 0005 step 4). // diff --git a/backend/src/modules/payments/controllers/index.ts b/backend/src/modules/payments/controllers/index.ts index fa4dba9..3c9ccd6 100644 --- a/backend/src/modules/payments/controllers/index.ts +++ b/backend/src/modules/payments/controllers/index.ts @@ -1,6 +1,6 @@ import type { Request, Response } from 'express'; import * as paymentsService from '../services'; -import { linkWalletSchema, createPaymentIntentSchema } from '../validation'; +import { walletLinkSchema, createPaymentIntentSchema } from '../validation'; import { HttpError } from '../../../http-error'; import { getParam } from '../../../request-params'; @@ -29,16 +29,23 @@ function serializeIntent(intent: { export async function linkWallet(req: Request, res: Response) { if (!req.user) throw new HttpError(401, 'Authentication required.'); - const facilityId = await paymentsService.getOwnFacilityProfileId(req.user.id); - const { stellarPublicKey } = linkWalletSchema.parse(req.body); - const link = await paymentsService.linkWallet(facilityId, stellarPublicKey); - res.status(200).json(link); + // strict schema: rejects any raw-address field outright. + const { transaction } = walletLinkSchema.parse(req.body); + const link = await paymentsService.linkWalletViaSep10(req.user.id, req.user.role, transaction); + // Patient links always carry the ADR 0005 on-chain privacy notice, + // verbatim, in the response body โ€” a client cannot render success + // without it (issue #78 requirement 3). Facility links (payout + // addresses) don't have a patient privacy dimension. + res.status(200).json( + link.ownerType === 'patient' + ? { ...link, privacyNotice: paymentsService.PATIENT_WALLET_LINK_PRIVACY_NOTICE } + : link, + ); } export async function getOwnWallet(req: Request, res: Response) { if (!req.user) throw new HttpError(401, 'Authentication required.'); - const facilityId = await paymentsService.getOwnFacilityProfileId(req.user.id); - const link = await paymentsService.getWalletLink(facilityId); + const link = await paymentsService.getOwnWalletLink(req.user.id, req.user.role); if (!link) throw new HttpError(404, 'No wallet linked yet.'); res.status(200).json(link); } diff --git a/backend/src/modules/payments/repositories/index.ts b/backend/src/modules/payments/repositories/index.ts index 7fed3d4..1036676 100644 --- a/backend/src/modules/payments/repositories/index.ts +++ b/backend/src/modules/payments/repositories/index.ts @@ -2,7 +2,13 @@ // 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, WalletLinkOwnerType } from '@prisma/client'; + +// A Prisma client OR an open interactive transaction. Every function +// that participates in the wallet-link <-> SEP-10 nonce transaction +// takes this (defaulting to the shared client) so callers can compose +// atomicity without each function hardcoding `prisma`. +type Db = Prisma.TransactionClient; /** Reads FacilityProfile directly โ€” a Phase 1 core identity model, not * something the (still-placeholder) facilities module exclusively owns. */ @@ -10,16 +16,56 @@ export function findFacilityProfileByUserId(userId: string) { return prisma.facilityProfile.findUnique({ where: { userId } }); } -export function upsertWalletLink(facilityId: string, stellarPublicKey: string) { - return prisma.walletLink.upsert({ - where: { facilityId }, - create: { facilityId, stellarPublicKey }, - update: { stellarPublicKey }, +/** Same reasoning as above, for the (placeholder) patients module. */ +export function findPatientProfileByUserId(userId: string) { + return prisma.patientProfile.findUnique({ where: { userId } }); +} + +// --- Wallet links (issue #78) -------------------------------------------- +// NOTE ON WRITES: `createWalletLink` is the only WalletLink insert in +// the repo, and its sole caller is the SEP-10 verification flow in +// services/wallet-link.ts (inside the transaction that consumes the +// nonce). There is deliberately no upsert/raw-address write path and no +// legacy-creation function โ€” `unverified_legacy` rows can only come from +// the migration backfill. + +export function createWalletLink( + data: { + ownerType: WalletLinkOwnerType; + ownerId: string; + stellarPublicKey: string; + verificationMethod: 'sep10'; + verifiedAt: Date; + activeOwnerKey: string; + }, + db: Db = prisma, +) { + return db.walletLink.create({ data }); +} + +export function findActiveWalletLink(ownerType: WalletLinkOwnerType, ownerId: string, db: Db = prisma) { + return db.walletLink.findFirst({ + where: { ownerType, ownerId, replacedAt: null }, + orderBy: { createdAt: 'desc' }, }); } -export function findWalletLinkByFacilityId(facilityId: string) { - return prisma.walletLink.findUnique({ where: { facilityId } }); +/** Deactivates the owner's current link (if any) โ€” conditional + * `updateMany`, so only an unreplaced row moves. Returns the number of + * rows deactivated (0 = owner had no active link; 1 = normal re-link). + * Paired with the unique `activeOwnerKey`, this is what makes "one + * active link per owner" hold under concurrency: the insert that follows + * can only succeed once the sentinel key is free. */ +export function deactivateActiveWalletLink( + ownerType: WalletLinkOwnerType, + ownerId: string, + now: Date, + db: Db = prisma, +) { + return db.walletLink.updateMany({ + where: { ownerType, ownerId, replacedAt: null }, + data: { replacedAt: now, activeOwnerKey: null }, + }); } export function createPaymentIntent(input: { @@ -36,7 +82,10 @@ export function findPaymentIntentById(id: string) { return prisma.paymentIntent.findUnique({ where: { id } }); } -export function markEscrowed(id: string, input: { escrowId: bigint; stellarTxHash: string; sorobanContractId: string }) { +export function markEscrowed( + id: string, + input: { escrowId: bigint; stellarTxHash: string; sorobanContractId: string; fundedWalletAddress: string }, +) { return prisma.paymentIntent.update({ where: { id }, data: { status: 'escrowed', ...input }, @@ -56,19 +105,29 @@ export function createSep10Challenge(input: { userId: string; account: string; n return prisma.sep10Challenge.create({ data: input }); } -export function findSep10ChallengeByNonce(nonce: string) { - return prisma.sep10Challenge.findUnique({ where: { nonce } }); -} - /** Atomically consumes a challenge โ€” ONE conditional UPDATE, no * read-then-write. Only a row that is still unconsumed AND unexpired * moves to `consumedAt`; the returned `count` tells the caller whether * THIS request won the race. Two concurrent replays of the same signed * challenge can never both see `count === 1`. Same DB-level pattern as - * booking's conditional slot update (ARCHITECTURE.md ยง5.1). */ -export function consumeSep10Challenge(nonce: string, consumedAt: Date) { - return prisma.sep10Challenge.updateMany({ + * booking's conditional slot update (ARCHITECTURE.md ยง5.1). + * Takes an optional transaction client so the wallet-link flow can + * consume the nonce and write the link in ONE transaction (issue #78). */ +export function consumeSep10Challenge(nonce: string, consumedAt: Date, db: Db = prisma) { + return db.sep10Challenge.updateMany({ where: { nonce, consumedAt: null, expiresAt: { gt: consumedAt } }, data: { consumedAt }, }); } + +export function findSep10ChallengeByNonce(nonce: string, db: Db = prisma) { + return db.sep10Challenge.findUnique({ where: { nonce } }); +} + +/** Runs `fn` inside ONE interactive transaction with a generous timeout + * (the wallet-link flow performs a Horizon lookup inside it). Single + * place the service layer opens transactions, so the Prisma client + * stays a repository concern. */ +export function inTransaction(fn: (tx: Prisma.TransactionClient) => Promise): Promise { + return prisma.$transaction(fn, { timeout: 15_000 }); +} diff --git a/backend/src/modules/payments/routes/index.ts b/backend/src/modules/payments/routes/index.ts index 01131aa..9c501c5 100644 --- a/backend/src/modules/payments/routes/index.ts +++ b/backend/src/modules/payments/routes/index.ts @@ -4,15 +4,28 @@ import * as paymentsController from '../controllers'; export const paymentsRouter = Router(); -// Facility manages its own payout wallet. +// Wallet links (issue #78): patients link the wallet they pay FROM +// (ADR 0005 v2 payer), facilities the wallet they are paid TO. Only +// these two roles can own a link โ€” resolved to the caller's OWN profile +// in the service, so linking for an owner you don't control is a 403. +// +// The body is a signed SEP-10 challenge, never an address (see +// validation.ts); still verified-email-gated like every payments +// mutation here (issue #34 โ€” and ADR 0005's replacement for the old +// typed-in endpoint, which was gated the same way). paymentsRouter.post( '/wallet-link', authenticate, - requireRole('facility'), + requireRole('patient', 'facility'), requireVerifiedEmail, paymentsController.linkWallet, ); -paymentsRouter.get('/wallet-link', authenticate, requireRole('facility'), paymentsController.getOwnWallet); +paymentsRouter.get( + '/wallet-link', + authenticate, + requireRole('patient', 'facility'), + paymentsController.getOwnWallet, +); // v1 (custodial, admin-triggered โ€” see docs/adr/0004-custodial-payments-v1.md // "Consequences"): Phase 4/5 (Orders/Fulfillment) don't exist yet, so diff --git a/backend/src/modules/payments/services/escrow-sdk-loader.ts b/backend/src/modules/payments/services/escrow-sdk-loader.ts index 98eddce..7db4b55 100644 --- a/backend/src/modules/payments/services/escrow-sdk-loader.ts +++ b/backend/src/modules/payments/services/escrow-sdk-loader.ts @@ -7,8 +7,25 @@ type EscrowSdkModule = typeof import('@docforum/escrow-sdk', { with: { 'resolution-mode': 'import' } }); let modulePromise: Promise | undefined; +let override: EscrowSdkModule | undefined; export function loadEscrowSdk(): Promise { + if (override) return Promise.resolve(override); modulePromise ??= import('@docforum/escrow-sdk'); return modulePromise; } + +// Test seam (issue #78) โ€” same pattern as auth's setEmailSender and the +// SEP-10 Horizon fetcher override: lets the offline integration suite +// exercise the real funding flow (guard, snapshot, status transition) +// without reaching the escrow contract on testnet. Never set outside +// tests. +export type EscrowSdkForTests = EscrowSdkModule; + +export function setEscrowSdkOverride(sdk: EscrowSdkForTests): void { + override = sdk; +} + +export function resetEscrowSdkOverride(): void { + override = undefined; +} diff --git a/backend/src/modules/payments/services/index.ts b/backend/src/modules/payments/services/index.ts index e90c751..e2f2341 100644 --- a/backend/src/modules/payments/services/index.ts +++ b/backend/src/modules/payments/services/index.ts @@ -13,6 +13,7 @@ import { HttpError } from '../../../http-error'; import { config } from '../../../config'; import * as paymentsRepo from '../repositories'; import { loadEscrowSdk } from './escrow-sdk-loader'; +import { assertWalletLinkFundingAllowed } from './wallet-link'; // SEP-10 Stellar Web Authentication (issue #77). Re-exported through // this module's service surface (not imported file-by-file from outside) @@ -27,6 +28,22 @@ export { } from './sep10'; export type { Sep10AccountFetcher, Sep10AccountInfo, Sep10ChallengeResponse, Sep10Signer, Sep10VerifyResult } from './sep10'; +// Verified wallet links (issue #78) โ€” SEP-10-only link creation, +// owner-scoped reads, the funding-cutoff guard, and the ADR 0005 +// privacy notice constant, all through this module's service surface. +export { + PATIENT_WALLET_LINK_PRIVACY_NOTICE, + assertWalletLinkFundingAllowed, + getOwnWalletLink, + linkWalletViaSep10, + validateWalletLinkConfig, + walletLinkFundingAllowed, +} from './wallet-link'; +export type { WalletLinkOwner } from './wallet-link'; +// Test seam for the escrow SDK (offline funding tests, issue #78). +export { resetEscrowSdkOverride, setEscrowSdkOverride } from './escrow-sdk-loader'; +export type { EscrowSdkForTests } from './escrow-sdk-loader'; + async function requirePaymentsConfig() { const { platformPayerSecret, platformReleaserSecret, escrowContractId, escrowTokenId } = config.payments; const missing = Object.entries({ platformPayerSecret, platformReleaserSecret, escrowContractId, escrowTokenId }) @@ -49,20 +66,6 @@ async function escrowClient(contractId: string) { return new EscrowClient({ contractId }); } -export async function getOwnFacilityProfileId(userId: string): Promise { - const profile = await paymentsRepo.findFacilityProfileByUserId(userId); - if (!profile) throw new HttpError(403, 'No facility profile is associated with this account.'); - return profile.id; -} - -export function linkWallet(facilityId: string, stellarPublicKey: string) { - return paymentsRepo.upsertWalletLink(facilityId, stellarPublicKey); -} - -export function getWalletLink(facilityId: string) { - return paymentsRepo.findWalletLinkByFacilityId(facilityId); -} - export interface CreatePaymentIntentInput { orderType: 'prescription' | 'lab_order'; orderId: string; @@ -89,9 +92,16 @@ export async function fundPaymentIntent(id: string) { throw new HttpError(409, `PaymentIntent is '${intent.status}', not 'created' โ€” cannot fund.`); } - const walletLink = await paymentsRepo.findWalletLinkByFacilityId(intent.facilityId); + // The facility's ACTIVE link (issue #78: one per owner; replaced rows + // are history and must never be picked up here). + const walletLink = await paymentsRepo.findActiveWalletLink('facility', intent.facilityId); if (!walletLink) throw new HttpError(409, 'The facility for this payment has no linked wallet.'); + // Funding guard (issue #78 requirement 4) โ€” in the SERVICE, so scripts, + // future controllers or internal callers all hit it. Unverified legacy + // links fund only before WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF. + assertWalletLinkFundingAllowed(walletLink); + const { platformPayer, platformReleaser, escrowContractId, escrowTokenId } = await requirePaymentsConfig(); const client = await escrowClient(escrowContractId); @@ -107,7 +117,15 @@ export async function fundPaymentIntent(id: string) { releaser: platformReleaser.publicKey(), }); - return paymentsRepo.markEscrowed(id, { escrowId, stellarTxHash: txHash, sorobanContractId: escrowContractId }); + return paymentsRepo.markEscrowed(id, { + escrowId, + stellarTxHash: txHash, + sorobanContractId: escrowContractId, + // Snapshot the payee this escrow was actually funded to (issue #78 + // requirement 5): a later re-link of the facility's wallet must not + // rewrite which address this intent references. + fundedWalletAddress: walletLink.stellarPublicKey, + }); } /** 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. */ diff --git a/backend/src/modules/payments/services/sep10.ts b/backend/src/modules/payments/services/sep10.ts index 4379b3b..b71b956 100644 --- a/backend/src/modules/payments/services/sep10.ts +++ b/backend/src/modules/payments/services/sep10.ts @@ -35,8 +35,15 @@ import { HttpError } from '../../../http-error'; import { config } from '../../../config'; import * as paymentsRepo from '../repositories'; import { loadStellarSdk } from './stellar-sdk-loader'; +import type { Prisma } from '@prisma/client'; import type { Keypair, Transaction } from '@stellar/stellar-sdk' with { 'resolution-mode': 'import' }; +/** An open interactive transaction (or nothing โ€” the repository + * defaults apply). The wallet-link flow (issue #78) passes its + * transaction so challenge verification, nonce consumption and link + * creation commit or roll back together. */ +export type Sep10Db = Prisma.TransactionClient; + // --- Configuration ------------------------------------------------------- export interface Sep10Config { @@ -287,8 +294,18 @@ export interface Sep10VerifyResult { * * This endpoint does NOT mint a DocForum session JWT: DocForum's own * auth (Bearer access token) already authenticates the caller โ€” SEP-10's - * job here is proving wallet control, not issuing a session. */ -export async function verifySep10Challenge(userId: string, challengeTx: string): Promise { + * job here is proving wallet control, not issuing a session. + * + * `db` (issue #78): when supplied, every ledger read AND the nonce + * consumption happen inside the caller's interactive transaction, so a + * caller that writes its own records (e.g. a wallet link) gets all-or- + * nothing: a rejected write rolls the nonce consumption back, and a + * consumed nonce without the caller's write cannot survive. */ +export async function verifySep10Challenge( + userId: string, + challengeTx: string, + db?: Sep10Db, +): Promise { const cfg = await requireSep10Config(); const { WebAuth } = await loadStellarSdk(); @@ -315,7 +332,7 @@ export async function verifySep10Challenge(userId: string, challengeTx: string): // Our own ledger: was this challenge issued by us, is it still good, // and is it THIS caller's, for THIS account? Checked before the Horizon // round trip so a mismatched submission can't make us fetch signers. - const record = await paymentsRepo.findSep10ChallengeByNonce(nonce); + const record = await paymentsRepo.findSep10ChallengeByNonce(nonce, db); if (!record) { throw new HttpError(401, 'Challenge was not issued by this server.', 'SEP10_CHALLENGE_UNKNOWN'); } @@ -375,7 +392,7 @@ export async function verifySep10Challenge(userId: string, challengeTx: string): // of the same signed challenge race here โ€” exactly one conditional // UPDATE wins (`WHERE consumedAt IS NULL AND expiresAt > now`). const consumedAt = new Date(); - const { count } = await paymentsRepo.consumeSep10Challenge(nonce, consumedAt); + const { count } = await paymentsRepo.consumeSep10Challenge(nonce, consumedAt, db); if (count !== 1) { throw new HttpError(401, 'This challenge has already been used.', 'SEP10_CHALLENGE_ALREADY_USED'); } diff --git a/backend/src/modules/payments/services/wallet-link.ts b/backend/src/modules/payments/services/wallet-link.ts new file mode 100644 index 0000000..aa18710 --- /dev/null +++ b/backend/src/modules/payments/services/wallet-link.ts @@ -0,0 +1,173 @@ +// Verified wallet links (issue #78, ADR 0005). Replaces the old +// facility-only, typed-in `POST /payments/wallet-link` body: an address +// is NEVER read from the request. The only way to create a link is to +// submit a signed SEP-10 challenge, and the address is derived from the +// account that challenge was issued for and successfully verified +// against (`verifySep10Challenge` from issue #77). +// +// Invariant (reviewer check): `createWalletLink` (the only WalletLink +// insert in the repositories) is called from exactly one place โ€” the +// transaction below, after `verifySep10Challenge` has resolved without +// throwing. There is no seed, no admin route, no script and no legacy +// write path; `unverified_legacy` rows only ever come from the +// add_verified_wallet_links migration backfill. +import { HttpError } from '../../../http-error'; +import { config } from '../../../config'; +import * as paymentsRepo from '../repositories'; +import { verifySep10Challenge } from './sep10'; +import type { Role, WalletLink, WalletLinkVerificationMethod } from '@prisma/client'; + +/** + * ADR 0005, "Consequences โ†’ Privacy risk" โ€” reproduced VERBATIM (every + * word and punctuation mark identical; only the ADR's line wrapping is + * collapsed to single spaces so the string is usable in a JSON body). + * tests/unit/wallet-link.test.ts asserts this constant still appears + * verbatim in docs/adr/0005-non-custodial-patient-payments.md, so this + * can never drift into a paraphrase silently. + * + * It rides on EVERY patient wallet-link response so a client cannot + * render the link success without somewhere carrying this text โ€” the ADR + * says "The wallet-linking screen must say this plainly." + */ +export const PATIENT_WALLET_LINK_PRIVACY_NOTICE = + '**Privacy risk: health information can leak on-chain.** A payment from a patient\'s wallet to a facility\'s wallet is public forever. If the patient\'s address is ever linked to their identity, the facility it paid (for example a specialised clinic) can reveal health information. `condition_ref` stays an opaque id (escrow threat model T9), but that doesn\'t hide who paid whom. The wallet-linking screen must say this plainly.'; + +export interface WalletLinkOwner { + ownerType: 'patient' | 'facility'; + ownerId: string; +} + +/** Resolves the owner record the caller is allowed to link FOR. Exactly + * one owner per caller: a user's role determines which profile exists + * (both profile tables are 1:1 with User), so linking for an owner you + * don't control is a 403 before any verification work happens. + * + * Multi-user facility authorisation (several staff linking for one + * facility) is NOT modeled โ€” FacilityProfile is 1:1 with User today โ€” + * and is listed as an open design decision in the PR. */ +async function resolveCallerOwner(userId: string, role: Role): Promise { + if (role === 'patient') { + const profile = await paymentsRepo.findPatientProfileByUserId(userId); + if (!profile) throw new HttpError(403, 'No patient profile is associated with this account.'); + return { ownerType: 'patient', ownerId: profile.id }; + } + if (role === 'facility') { + const profile = await paymentsRepo.findFacilityProfileByUserId(userId); + if (!profile) throw new HttpError(403, 'No facility profile is associated with this account.'); + return { ownerType: 'facility', ownerId: profile.id }; + } + throw new HttpError(403, 'Only patient and facility accounts can link a wallet.'); +} + +/** + * POST /payments/wallet-link โ€” verifies the signed SEP-10 challenge AND + * creates (or replaces) the caller's link in ONE transaction, together + * with consuming the challenge nonce: + * + * - verification fails โ†’ the whole transaction rolls back: no link is + * created and the nonce is NOT burned (the user can retry); + * - the link write fails (e.g. a concurrent request won the + * one-active-link race) โ†’ the nonce consumption rolls back too, so a + * lost race costs nothing but a fresh challenge; + * - success โ†’ nonce consumed and exactly one active link, atomically. + * + * The returned `stellarPublicKey` is always the challenge's verified + * account โ€” no code path here consults a client-supplied address. + */ +export async function linkWalletViaSep10( + userId: string, + role: Role, + signedChallengeTx: string, +): Promise { + const owner = await resolveCallerOwner(userId, role); + const now = new Date(); + + try { + return await paymentsRepo.inTransaction(async (tx) => { + // Full SEP-10 server verification, ledger binding (this user) and + // atomic nonce consumption โ€” inside this transaction. + const { account } = await verifySep10Challenge(userId, signedChallengeTx, tx); + + // Re-link deactivates rather than mutates/deletes: the previous + // row keeps its address and history, and the active sentinel key + // frees up for the new row (unique index keeps this race-safe). + await paymentsRepo.deactivateActiveWalletLink(owner.ownerType, owner.ownerId, now, tx); + + return paymentsRepo.createWalletLink( + { + ownerType: owner.ownerType, + ownerId: owner.ownerId, + stellarPublicKey: account, + verificationMethod: 'sep10', + verifiedAt: now, + activeOwnerKey: `${owner.ownerType}:${owner.ownerId}`, + }, + tx, + ); + }); + } catch (err) { + // P2002 on activeOwnerKey = a concurrent request created the link + // first (interactive transaction aborted, nonce restored above). + if (typeof err === 'object' && err !== null && (err as { code?: string }).code === 'P2002') { + throw new HttpError(409, 'Another wallet link for this account was just created. Refresh and try again.', 'WALLET_LINK_CONFLICT'); + } + throw err; + } +} + +/** GET /payments/wallet-link โ€” the caller's own ACTIVE link (replaced + * rows are history, not exposed here). Null when none. */ +export async function getOwnWalletLink(userId: string, role: Role): Promise { + const owner = await resolveCallerOwner(userId, role); + return paymentsRepo.findActiveWalletLink(owner.ownerType, owner.ownerId); +} + +// --- Funding cutoff guard (requirement 4) -------------------------------- + +/** True once a wallet link may be used as an escrow payee/funder: + * verified links always; `unverified_legacy` links only before + * WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF. Exported (and pure in + * `nowMs`/cutoff) so tests can walk the whole matrix without waiting for + * a real date to pass. */ +export function walletLinkFundingAllowed( + verificationMethod: WalletLinkVerificationMethod, + cutoffMs: number, + nowMs: number, +): boolean { + if (verificationMethod === 'sep10') return true; + return nowMs <= cutoffMs; +} + +/** The enforcement point used by the funding service (not a controller + * check): refuses to fund an escrow whose payee is an unverified legacy + * link after the cutoff. Fail-closed on a misparsed cutoff rather than + * silently allowing funding. */ +export function assertWalletLinkFundingAllowed(link: Pick): void { + const cutoffMs = config.payments.walletLinkUnverifiedFundingCutoff; + if (Number.isNaN(cutoffMs)) { + throw new HttpError( + 500, + 'WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF is not a valid date โ€” funding is refused until it is.', + 'PAYMENTS_MISCONFIGURED', + ); + } + if (!walletLinkFundingAllowed(link.verificationMethod, cutoffMs, Date.now())) { + throw new HttpError( + 409, + "This facility's wallet link predates SEP-10 verification and the funding cutoff has passed. Re-link with a SEP-10-verified wallet to fund payments.", + 'WALLET_LINK_UNVERIFIED', + ); + } +} + +/** + * Startup validation (requirement 4: "validated at startup"). Called + * from src/server.ts next to validateSep10Config() โ€” a malformed cutoff + * must stop boot instead of surfacing as a 500 at first funding. + * Names the env var, never any value. + */ +export function validateWalletLinkConfig(): void { + if (Number.isNaN(config.payments.walletLinkUnverifiedFundingCutoff)) { + throw new Error('WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF is not a valid ISO date.'); + } +} diff --git a/backend/src/modules/payments/validation.ts b/backend/src/modules/payments/validation.ts index 4644be4..916b33f 100644 --- a/backend/src/modules/payments/validation.ts +++ b/backend/src/modules/payments/validation.ts @@ -1,14 +1,15 @@ import { z } from 'zod'; -export const linkWalletSchema = z.object({ - // Stellar public keys are 56 chars, start with 'G'. Not a full - // checksum validation (that's what create_escrow/the network itself - // will reject) โ€” just a sanity bound against obvious garbage input. - // (The SEP-10 challenge endpoint DOES checksum-validate via - // StrKey.isValidEd25519PublicKey โ€” see services/sep10.ts.) - stellarPublicKey: z - .string() - .regex(/^G[A-Z2-7]{55}$/, 'stellarPublicKey must be a valid Stellar public key (starts with G, 56 chars).'), +// Issue #78 โ€” the typed-in address body is GONE. strictObject: a +// client that still sends `stellarPublicKey` (or anything else) is a +// loud 400, never a silently-ignored field, so nobody can believe they +// linked an address the server never saw. The only accepted input is +// the signed SEP-10 challenge; the address comes from verifying it. +export const walletLinkSchema = z.strictObject({ + // base64 TransactionEnvelope XDR of the wallet-signed SEP-10 challenge + // (issue #77), issued to the authenticated caller for the account + // being linked. + transaction: z.string().min(1), }); export const createPaymentIntentSchema = z.object({ diff --git a/backend/src/server.ts b/backend/src/server.ts index f470c82..318df4a 100644 --- a/backend/src/server.ts +++ b/backend/src/server.ts @@ -1,7 +1,7 @@ import { createApp } from './app'; import { config } from './config'; import { pruneLoginAttempts } from './modules/auth/services'; -import { validateSep10Config } from './modules/payments/services'; +import { validateSep10Config, validateWalletLinkConfig } from './modules/payments/services'; const app = createApp(); @@ -12,6 +12,9 @@ const app = createApp(); // only build the app, aren't forced to boot with payments configured. async function start(): Promise { await validateSep10Config(); + // Issue #78: a malformed WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF must + // stop boot too โ€” naming the env var, never a value. + validateWalletLinkConfig(); app.listen(config.port, () => { // eslint-disable-next-line no-console console.log(`docforum-core backend listening on :${config.port}`); diff --git a/backend/tests/integration/wallet-link.test.mts b/backend/tests/integration/wallet-link.test.mts new file mode 100644 index 0000000..fd1fa52 --- /dev/null +++ b/backend/tests/integration/wallet-link.test.mts @@ -0,0 +1,570 @@ +// Integration tests for verified wallet links (issue #78) โ€” real embedded +// Postgres (active-link constraint, transactions, migration backfill), +// real HTTP through the Express app, FAKE Horizon (SEP-10 signer lookup) +// and a FAKE escrow SDK (funding path) โ€” zero network. +// +// The cutoff env var is set to a PAST date at module load, so this suite +// runs with the guard ENFORCED: legacy links must be refused, verified +// links allowed. The complementary "legacy allowed before the cutoff" +// side is unit-tested (tests/unit/wallet-link.test.ts) against the +// default grace window. +// +// Own port (5439) so it runs alongside the other integration files +// (5433 booking, 5435/5436/5437 others, 5438 sep10). +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 bcrypt from 'bcryptjs'; +import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import request from 'supertest'; +import EmbeddedPostgres from 'embedded-postgres'; +import { Keypair, TransactionBuilder } from '@stellar/stellar-sdk'; +import type { Transaction } from '@stellar/stellar-sdk'; +import type { Sep10AccountInfo } from '../../src/modules/payments/services/sep10.js'; +import type { EscrowSdkForTests } from '../../src/modules/payments/services/index.js'; + +const BACKEND_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..'); +const PORT = 5439; +const runId = Date.now(); +const PASSWORD = 'longenough123'; + +// --- env BEFORE src/config loads (see login-throttle.test.mts pattern) --- +// Guard enforced NOW: past cutoff => unverified_legacy funding refused. +process.env.WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF = '2026-01-01T00:00:00Z'; +// Money keys only need to parse as real keypairs; the escrow SDK itself +// is faked (setEscrowSdkOverride), so nothing signs or broadcasts. +process.env.PLATFORM_PAYER_SECRET = Keypair.random().secret(); +process.env.PLATFORM_RELEASER_SECRET = Keypair.random().secret(); +process.env.ESCROW_CONTRACT_ID = 'TEST_CONTRACT_ID'; +process.env.ESCROW_TOKEN_ID = 'TEST_TOKEN_ID'; + +const dataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-wallet-link-pg-')); +const pg = new EmbeddedPostgres({ + databaseDir: dataDir, + user: 'docforum', + password: 'docforum', + port: PORT, + persistent: false, +}); +const databaseUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_test`; + +type App = ReturnType; +type Prisma = typeof import('../../src/db/prisma-client.js').prisma; +type Services = typeof import('../../src/modules/payments/services/index.js'); + +describe('verified wallet links (issue #78)', () => { + let app: App; + let prisma: Prisma; + let services: Services; + let networkPassphrase: string; + let patientNotice: string; + + // Fake Horizon (same shape as sep10.test.mts): unfunded = null. + const accountOverrides = new Map(); + const fundedSingleSigner = (accountId: string): Sep10AccountInfo => ({ + accountId, + thresholds: { low_threshold: 0, med_threshold: 1, high_threshold: 1 }, + signers: [{ key: accountId, weight: 1, type: 'ed25519_public_key' }], + }); + + // Fake escrow SDK: records what funding asked for, returns plausible + // values โ€” lets us assert the payee snapshot and re-link isolation + // without touching testnet. + const escrowCalls: { payee?: string }[] = []; + let releaseCalls = 0; + + beforeAll(async () => { + await pg.initialise(); + await pg.start(); + await pg.createDatabase('docforum_test'); + await pg.createDatabase('docforum_migration_test'); + + execFileSync('npx', ['prisma', 'migrate', 'deploy'], { + cwd: BACKEND_ROOT, + env: { ...process.env, DATABASE_URL: databaseUrl }, + stdio: 'pipe', + }); + + process.env.DATABASE_URL = databaseUrl; + + const appModule = await import('../../src/app.js'); + const dbModule = await import('../../src/db/prisma-client.js'); + const servicesModule = await import('../../src/modules/payments/services/index.js'); + const configModule = await import('../../src/config/index.js'); + app = appModule.createApp(); + prisma = dbModule.prisma; + services = servicesModule; + + networkPassphrase = configModule.config.sep10.networkPassphrase; + patientNotice = servicesModule.PATIENT_WALLET_LINK_PRIVACY_NOTICE; + + servicesModule.setSep10AccountFetcher((accountId) => + Promise.resolve(accountOverrides.has(accountId) ? accountOverrides.get(accountId)! : fundedSingleSigner(accountId)), + ); + + servicesModule.setEscrowSdkOverride({ + Keypair, + EscrowClient: class { + async createEscrow(args: { payee: string }) { + escrowCalls.push({ payee: args.payee }); + return { escrowId: BigInt(escrowCalls.length), txHash: `fake-tx-${escrowCalls.length}` }; + } + async release() { + releaseCalls += 1; + return { txHash: 'fake-release' }; + } + async refund() { + return { txHash: 'fake-refund' }; + } + }, + } as unknown as EscrowSdkForTests); + }); + + afterAll(async () => { + services?.resetEscrowSdkOverride(); + services?.resetSep10AccountFetcher(); + await prisma?.$disconnect(); + await pg.stop(); + fs.rmSync(dataDir, { recursive: true, force: true }); + }); + + afterEach(() => { + accountOverrides.clear(); + }); + + // --- helpers ----------------------------------------------------------- + + async function signupPatient(label: string): Promise<{ userId: string; token: string }> { + const email = `wl-patient-${label}-${runId}@example.com`; + const res = await request(app) + .post('/auth/signup') + .send({ role: 'patient', email, password: PASSWORD, fullName: `WL ${label}` }); + expect(res.status).toBe(200); + await prisma.user.update({ where: { id: res.body.user.id }, data: { emailVerifiedAt: new Date() } }); + return { userId: res.body.user.id as string, token: res.body.accessToken as string }; + } + + /** Facility accounts are admin-invited (signup rejects the role), so + * this arranges one directly and logs in through the real login route. */ + async function createFacility(label: string): Promise<{ userId: string; facilityId: string; token: string }> { + const email = `wl-facility-${label}-${runId}@example.com`; + const user = await prisma.user.create({ + data: { + email, + passwordHash: bcrypt.hashSync(PASSWORD, 4), + role: 'facility', + emailVerifiedAt: new Date(), + }, + }); + const profile = await prisma.facilityProfile.create({ + data: { userId: user.id, name: `WL Facility ${label}`, type: 'lab' }, + }); + const login = await request(app).post('/auth/login').send({ email, password: PASSWORD }); + expect(login.status).toBe(200); + return { userId: user.id, facilityId: profile.id, token: login.body.accessToken as string }; + } + + function getChallenge(token: string, account: string) { + return request(app).get('/payments/sep10/challenge').set('Authorization', `Bearer ${token}`).query({ account }); + } + + function postLink(token: string, body: unknown) { + return request(app).post('/payments/wallet-link').set('Authorization', `Bearer ${token}`).send(body as object); + } + + /** Challenge + wallet sign in one step (issue #77 flow). */ + async function signedChallengeFor(token: string, ...signers: Keypair[]): Promise { + const account = signers[0]!.publicKey(); + const res = await getChallenge(token, account); + expect(res.status).toBe(200); + const tx = TransactionBuilder.fromXDR(res.body.transaction, networkPassphrase) as Transaction; + for (const signer of signers) tx.sign(signer); + return tx.toEnvelope().toXDR('base64').toString(); + } + + async function activeLinks(ownerType: 'patient' | 'facility', ownerId: string) { + return prisma.walletLink.findMany({ where: { ownerType, ownerId, replacedAt: null } }); + } + + // --- link creation ----------------------------------------------------- + + it('facility: links a verified wallet end-to-end (address derived from the challenge)', async () => { + const facility = await createFacility('happy'); + const wallet = Keypair.random(); + const signed = await signedChallengeFor(facility.token, wallet); + + const res = await postLink(facility.token, { transaction: signed }); + expect(res.status).toBe(200); + expect(res.body.stellarPublicKey).toBe(wallet.publicKey()); // derived, not supplied + expect(res.body.ownerType).toBe('facility'); + expect(res.body.ownerId).toBe(facility.facilityId); + expect(res.body.verificationMethod).toBe('sep10'); + expect(res.body.verifiedAt).toBeTruthy(); + expect(res.body.replacedAt).toBeNull(); + // Facility payout link carries no patient privacy notice. + expect(res.body.privacyNotice).toBeUndefined(); + + const row = await prisma.walletLink.findFirst({ where: { ownerId: facility.facilityId } }); + expect(row!.activeOwnerKey).toBe(`facility:${facility.facilityId}`); + }); + + it('patient: links a verified wallet and the ADR 0005 privacy notice rides in the response', async () => { + const patient = await signupPatient('happy'); + const wallet = Keypair.random(); + const signed = await signedChallengeFor(patient.token, wallet); + + const res = await postLink(patient.token, { transaction: signed }); + expect(res.status).toBe(200); + expect(res.body.stellarPublicKey).toBe(wallet.publicKey()); + expect(res.body.ownerType).toBe('patient'); + expect(res.body.verificationMethod).toBe('sep10'); + // Requirement 3: verbatim notice, cannot be skipped by the client. + expect(res.body.privacyNotice).toBe(patientNotice); + expect(res.body.privacyNotice).toContain('public forever'); + + const profile = await prisma.patientProfile.findUnique({ where: { userId: patient.userId } }); + const rows = await activeLinks('patient', profile!.id); + expect(rows).toHaveLength(1); + }); + + it('rejects a raw address in the body โ€” the address can only come from the verified challenge', async () => { + const patient = await signupPatient('raw'); + const address = Keypair.random().publicKey(); + const rowsBefore = await prisma.walletLink.count(); + + const withAddress = await postLink(patient.token, { stellarPublicKey: address }); + expect(withAddress.status).toBe(400); + + const both = await postLink(patient.token, { transaction: 'AAAA', stellarPublicKey: address }); + expect(both.status).toBe(400); + + expect(await prisma.walletLink.count()).toBe(rowsBefore); + }); + + it('creates NO link when SEP-10 verification fails, and does not burn the nonce', async () => { + const patient = await signupPatient('failed'); + const accountOwner = Keypair.random(); + const impostor = Keypair.random(); + + // Challenge issued for accountOwner, but signed by someone else. + const res1 = await getChallenge(patient.token, accountOwner.publicKey()); + const tx = TransactionBuilder.fromXDR(res1.body.transaction, networkPassphrase) as Transaction; + tx.sign(impostor); + const signed = tx.toEnvelope().toXDR('base64').toString(); + + const res = await postLink(patient.token, { transaction: signed }); + expect(res.status).toBe(401); + expect(res.body.code).toBe('SEP10_INVALID_SIGNATURE'); + + const profile = await prisma.patientProfile.findUnique({ where: { userId: patient.userId } }); + expect(await activeLinks('patient', profile!.id)).toHaveLength(0); + + // The failed attempt did NOT consume the challenge nonce โ€” the row + // is still unconsumed, so the user hasn't lost anything but a retry. + const challengeRow = await prisma.sep10Challenge.findFirst({ orderBy: { createdAt: 'desc' } }); + expect(challengeRow!.consumedAt).toBeNull(); + }); + + it('rejects a REPLAYED signed challenge (second link attempt) without creating a second link', async () => { + const patient = await signupPatient('replay'); + const wallet = Keypair.random(); + const signed = await signedChallengeFor(patient.token, wallet); + + expect((await postLink(patient.token, { transaction: signed })).status).toBe(200); + const replay = await postLink(patient.token, { transaction: signed }); + expect(replay.status).toBe(401); + expect(replay.body.code).toBe('SEP10_CHALLENGE_ALREADY_USED'); + + const profile = await prisma.patientProfile.findUnique({ where: { userId: patient.userId } }); + expect(await activeLinks('patient', profile!.id)).toHaveLength(1); + }); + + it("rejects a challenge issued to user A when user B submits it (B gets no link)", async () => { + const a = await signupPatient('owner-a'); + const b = await signupPatient('owner-b'); + const wallet = Keypair.random(); + const signed = await signedChallengeFor(a.token, wallet); + + const res = await postLink(b.token, { transaction: signed }); + expect(res.status).toBe(403); + expect(res.body.code).toBe('SEP10_CHALLENGE_NOT_FOR_CALLER'); + + const bProfile = await prisma.patientProfile.findUnique({ where: { userId: b.userId } }); + expect(await activeLinks('patient', bProfile!.id)).toHaveLength(0); + }); + + it('refuses roles that cannot own a wallet link (doctor) with 403', async () => { + const email = `wl-doctor-${runId}@example.com`; + const signup = await request(app) + .post('/auth/signup') + .send({ role: 'doctor', email, password: PASSWORD, fullName: 'WL Doctor' }); + expect(signup.status).toBe(200); + await prisma.user.update({ where: { id: signup.body.user.id }, data: { emailVerifiedAt: new Date() } }); + + const res = await postLink(signup.body.accessToken, { transaction: 'AAAA' }); + expect(res.status).toBe(403); + }); + + it('enforces ONE active link per owner at the DB level, including concurrent creates', async () => { + const facility = await createFacility('concurrent'); + const key = `facility:${facility.facilityId}`; + + // Raw DB constraint check: a second ACTIVE row for the same owner + // violates the unique sentinel even outside the service. + const base = { + ownerType: 'facility' as const, + ownerId: facility.facilityId, + verificationMethod: 'sep10' as const, + verifiedAt: new Date(), + }; + await prisma.walletLink.create({ data: { ...base, stellarPublicKey: Keypair.random().publicKey(), activeOwnerKey: key } }); + await expect( + prisma.walletLink.create({ data: { ...base, stellarPublicKey: Keypair.random().publicKey(), activeOwnerKey: key } }), + ).rejects.toMatchObject({ code: 'P2002' }); + await prisma.walletLink.deleteMany({ where: { ownerId: facility.facilityId } }); + + // CONCURRENT inserts of two active rows for the same owner (no + // pre-existing row): the unique index must let exactly one through, + // whichever order Postgres runs them in. + const rawCreate = () => + prisma.walletLink.create({ data: { ...base, stellarPublicKey: Keypair.random().publicKey(), activeOwnerKey: key } }); + const rawResults = await Promise.allSettled([rawCreate(), rawCreate()]); + expect(rawResults.filter((r) => r.status === 'fulfilled')).toHaveLength(1); + const rejected = rawResults.find((r) => r.status === 'rejected') as PromiseRejectedResult; + expect((rejected.reason as { code?: string }).code).toBe('P2002'); + expect(await activeLinks('facility', facility.facilityId)).toHaveLength(1); + await prisma.walletLink.deleteMany({ where: { ownerId: facility.facilityId } }); + + // Genuinely concurrent HTTP creates: two valid challenges, both in + // flight before either resolves. Exactly one active link must exist + // afterwards. (The two statuses depend on interleaving: a true + // insert-collision yields 200+409; if the second request starts + // after the first committed, it is a plain re-link and both get 200 + // with the first row replaced. Both serializations are legal โ€” the + // invariant is "never two active links".) + const w1 = Keypair.random(); + const w2 = Keypair.random(); + const s1 = await signedChallengeFor(facility.token, w1); + const s2 = await signedChallengeFor(facility.token, w2); + const [r1, r2] = await Promise.all([postLink(facility.token, { transaction: s1 }), postLink(facility.token, { transaction: s2 })]); + + expect([r1.status, r2.status].every((s) => s === 200 || s === 409)).toBe(true); + expect([r1.status, r2.status]).toContain(200); + const active = await activeLinks('facility', facility.facilityId); + expect(active).toHaveLength(1); // THE invariant + expect([w1.publicKey(), w2.publicKey()]).toContain(active[0]!.stellarPublicKey); + }); + + // --- re-link safety + funding ----------------------------------------- + + it('re-link deactivates the old link and leaves escrows funded to the old address untouched', async () => { + const facility = await createFacility('relink'); + const patient = await signupPatient('relink'); + const patientProfile = await prisma.patientProfile.findUniqueOrThrow({ where: { userId: patient.userId } }); + + const addr1 = Keypair.random(); + const addr2 = Keypair.random(); + + // Link v1 (verified), fund an escrow to it. + expect((await postLink(facility.token, { transaction: await signedChallengeFor(facility.token, addr1) })).status).toBe(200); + + const intent = await services.createPaymentIntent({ + orderType: 'lab_order', + orderId: 'relink-order', + patientProfileId: patientProfile.id, + facilityId: facility.facilityId, + amountStroops: 500_000n, + }); + const funded = await services.fundPaymentIntent(intent.id); + expect(funded.status).toBe('escrowed'); + expect(funded.fundedWalletAddress).toBe(addr1.publicKey()); + expect(escrowCalls.at(-1)!.payee).toBe(addr1.publicKey()); + + // Re-link to a different address: old row is REPLACED (not mutated, + // not deleted), new row active. + const relink = await postLink(facility.token, { transaction: await signedChallengeFor(facility.token, addr2) }); + expect(relink.status).toBe(200); + expect(relink.body.stellarPublicKey).toBe(addr2.publicKey()); + + const all = await prisma.walletLink.findMany({ where: { ownerId: facility.facilityId }, orderBy: { createdAt: 'asc' } }); + expect(all).toHaveLength(2); + const [oldLink, newLink] = all; + expect(oldLink!.stellarPublicKey).toBe(addr1.publicKey()); // history preserved + expect(oldLink!.replacedAt).not.toBeNull(); + expect(oldLink!.activeOwnerKey).toBeNull(); + expect(newLink!.stellarPublicKey).toBe(addr2.publicKey()); + expect(newLink!.replacedAt).toBeNull(); + expect(newLink!.activeOwnerKey).toBe(`facility:${facility.facilityId}`); + + // The already-funded escrow still references its own address/link โ€” + // re-linking did not rewrite it โ€” and release works regardless. + const intentAfter = await services.getPaymentIntent(intent.id); + expect(intentAfter.fundedWalletAddress).toBe(addr1.publicKey()); + const released = await services.releasePaymentIntent(intent.id); + expect(released.status).toBe('released'); + expect(releaseCalls).toBeGreaterThan(0); + }); + + it('funding refuses an UNVERIFIED legacy link after the cutoff, and funds verified links after it', async () => { + // The suite's cutoff is in the past, so "after cutoff" is now. + const legacy = await createFacility('legacy'); + const verified = await createFacility('verified-post-cutoff'); + const patient = await signupPatient('funding'); + + // Simulate a migrated legacy link (typed-in address, never verified). + await prisma.walletLink.create({ + data: { + ownerType: 'facility', + ownerId: legacy.facilityId, + stellarPublicKey: Keypair.random().publicKey(), + verificationMethod: 'unverified_legacy', + activeOwnerKey: `facility:${legacy.facilityId}`, + }, + }); + expect(services.walletLinkFundingAllowed('unverified_legacy', Date.parse('2026-01-01T00:00:00Z'), Date.now())).toBe(false); + + const patientProfile = await prisma.patientProfile.findUniqueOrThrow({ where: { userId: patient.userId } }); + const legacyIntent = await services.createPaymentIntent({ + orderType: 'lab_order', + orderId: 'legacy-order', + patientProfileId: patientProfile.id, + facilityId: legacy.facilityId, + amountStroops: 100_000n, + }); + const callsBefore = escrowCalls.length; + await expect(services.fundPaymentIntent(legacyIntent.id)).rejects.toMatchObject({ + status: 409, + code: 'WALLET_LINK_UNVERIFIED', + }); + expect(escrowCalls.length).toBe(callsBefore); // SDK never reached + expect((await services.getPaymentIntent(legacyIntent.id)).status).toBe('created'); // untouched + + // A SEP-10-verified link funds fine after the same cutoff. + const wallet = Keypair.random(); + expect((await postLink(verified.token, { transaction: await signedChallengeFor(verified.token, wallet) })).status).toBe(200); + const verifiedIntent = await services.createPaymentIntent({ + orderType: 'lab_order', + orderId: 'verified-order', + patientProfileId: patientProfile.id, + facilityId: verified.facilityId, + amountStroops: 100_000n, + }); + const funded = await services.fundPaymentIntent(verifiedIntent.id); + expect(funded.status).toBe('escrowed'); + expect(funded.fundedWalletAddress).toBe(wallet.publicKey()); + }); + + it('GET /payments/wallet-link returns the caller\'s active link and 404 when none', async () => { + const fresh = await signupPatient('get-link'); + expect((await request(app).get('/payments/wallet-link').set('Authorization', `Bearer ${fresh.token}`)).status).toBe(404); + + const wallet = Keypair.random(); + await postLink(fresh.token, { transaction: await signedChallengeFor(fresh.token, wallet) }); + const got = await request(app).get('/payments/wallet-link').set('Authorization', `Bearer ${fresh.token}`); + expect(got.status).toBe(200); + expect(got.body.stellarPublicKey).toBe(wallet.publicKey()); + expect(got.body.verificationMethod).toBe('sep10'); + }); + + // --- migration --------------------------------------------------------- + + it('migration: existing facility rows become active unverified_legacy links', async () => { + // Real migration test: bring up a scratch DB at the schema JUST + // BEFORE this migration, insert an old-shape facility link, then + // apply only the new migration and assert the backfill. + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docforum-wallet-mig-')); + const tmpPrisma = path.join(tmpRoot, 'prisma'); + const NEW_MIGRATION = '20261016153822_add_verified_wallet_links'; + try { + fs.cpSync(path.join(BACKEND_ROOT, 'prisma'), tmpPrisma, { recursive: true }); + fs.rmSync(path.join(tmpPrisma, 'migrations', NEW_MIGRATION), { recursive: true, force: true }); + + const migUrl = `postgresql://docforum:docforum@localhost:${PORT}/docforum_migration_test`; + const deploy = (schemaPath: string) => + execFileSync('npx', ['prisma', 'migrate', 'deploy', '--schema', schemaPath], { + cwd: BACKEND_ROOT, + env: { ...process.env, DATABASE_URL: migUrl }, + stdio: 'pipe', + }); + + deploy(path.join(tmpPrisma, 'schema.prisma')); + + // Old-shape row: facility-only, typed-in address. + const { PrismaClient } = await import('@prisma/client'); + const migPrisma = new PrismaClient({ datasourceUrl: migUrl }); + try { + const user = await migPrisma.user.create({ + data: { email: `mig-${runId}@example.com`, passwordHash: 'unused', role: 'facility' }, + }); + const profile = await migPrisma.facilityProfile.create({ + data: { userId: user.id, name: 'Mig Facility', type: 'lab' }, + }); + const oldAddress = Keypair.random().publicKey(); + await migPrisma.$executeRawUnsafe( + 'INSERT INTO "WalletLink" ("id", "facilityId", "stellarPublicKey", "createdAt", "updatedAt") VALUES ($1, $2, $3, now(), now())', + `mig-link-${runId}`, + profile.id, + oldAddress, + ); + + // Apply ONLY the new migration. + fs.cpSync( + path.join(BACKEND_ROOT, 'prisma', 'migrations', NEW_MIGRATION), + path.join(tmpPrisma, 'migrations', NEW_MIGRATION), + { recursive: true }, + ); + deploy(path.join(tmpPrisma, 'schema.prisma')); + + const rows = await migPrisma.walletLink.findMany({ where: { ownerId: profile.id } }); + expect(rows).toHaveLength(1); + const row = rows[0]!; + expect(row.stellarPublicKey).toBe(oldAddress); // address untouched + expect(row.ownerType).toBe('facility'); + expect(row.verificationMethod).toBe('unverified_legacy'); // never SEP-10-checked + expect(row.verifiedAt).toBeNull(); + expect(row.replacedAt).toBeNull(); + expect(row.activeOwnerKey).toBe(`facility:${profile.id}`); // still the active link + + // And the backfilled link participates in the new world: it can + // be replaced, and it can only be replaced once. + await migPrisma.walletLink.create({ + data: { + ownerType: 'facility', + ownerId: profile.id, + stellarPublicKey: Keypair.random().publicKey(), + verificationMethod: 'sep10', + verifiedAt: new Date(), + activeOwnerKey: `facility:${profile.id}`, + }, + }).then( + () => { + throw new Error('second active link should have been rejected'); + }, + (err: { code?: string }) => expect(err.code).toBe('P2002'), + ); + // Deactivate-then-insert is the legal path: + await migPrisma.walletLink.updateMany({ + where: { ownerId: profile.id, replacedAt: null }, + data: { replacedAt: new Date(), activeOwnerKey: null }, + }); + const promoted = await migPrisma.walletLink.create({ + data: { + ownerType: 'facility', + ownerId: profile.id, + stellarPublicKey: Keypair.random().publicKey(), + verificationMethod: 'sep10', + verifiedAt: new Date(), + activeOwnerKey: `facility:${profile.id}`, + }, + }); + expect(promoted.verificationMethod).toBe('sep10'); + } finally { + await migPrisma.$disconnect(); + } + } finally { + fs.rmSync(tmpRoot, { recursive: true, force: true }); + } + }, 60_000); +}); diff --git a/backend/tests/testnet-integration/payments.test.mts b/backend/tests/testnet-integration/payments.test.mts index 0a1c1a8..261ab50 100644 --- a/backend/tests/testnet-integration/payments.test.mts +++ b/backend/tests/testnet-integration/payments.test.mts @@ -95,7 +95,21 @@ describe('payments module against live testnet', () => { patientProfileId = patient.id; const payoutWallet = await fundedKeypair(); - await paymentsService.linkWallet(facilityId, payoutWallet.publicKey()); + // Issue #78 removed the typed-in link path: this live-testnet suite + // arranges its payee directly as an explicitly-unverified legacy + // link (test setup, not a product code path โ€” there is no service + // that can create one anymore). Unverified links still fund until + // WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF, which this suite does not + // set, so the default grace window applies. + await prisma.walletLink.create({ + data: { + ownerType: 'facility', + ownerId: facilityId, + stellarPublicKey: payoutWallet.publicKey(), + verificationMethod: 'unverified_legacy', + activeOwnerKey: `facility:${facilityId}`, + }, + }); }); afterAll(async () => { diff --git a/backend/tests/unit/wallet-link.test.mts b/backend/tests/unit/wallet-link.test.mts new file mode 100644 index 0000000..637ed1e --- /dev/null +++ b/backend/tests/unit/wallet-link.test.mts @@ -0,0 +1,97 @@ +// Unit tests for verified wallet links (issue #78) โ€” no database, no +// network. Complements tests/integration/wallet-link.test.mts, which +// covers the HTTP/DB flows; this file covers the pure guards, the +// request schema, the privacy-notice verbatim guarantee, and startup +// validation of the cutoff env var. +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it, vi } from 'vitest'; +import { config } from '../../src/config/index.js'; +import { walletLinkSchema } from '../../src/modules/payments/validation.js'; +import { + PATIENT_WALLET_LINK_PRIVACY_NOTICE, + assertWalletLinkFundingAllowed, + validateWalletLinkConfig, + walletLinkFundingAllowed, +} from '../../src/modules/payments/services/wallet-link.js'; + +// backend/tests/unit โ†’ repo root (the ADR lives outside backend/). +const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../../..'); + +describe('patient wallet-link privacy notice (ADR 0005, verbatim)', () => { + it('appears word-for-word in docs/adr/0005-non-custodial-patient-payments.md', () => { + const adr = fs.readFileSync(path.join(REPO_ROOT, 'docs/adr/0005-non-custodial-patient-payments.md'), 'utf8'); + // Normalise whitespace on both sides: the constant collapses the + // ADR's line wrapping to single spaces so it is usable in JSON, but + // every word, punctuation mark and markdown tick must match โ€” any + // paraphrase fails this test. + const normalise = (text: string) => text.replace(/\s+/g, ' ').trim(); + expect(normalise(adr)).toContain(normalise(PATIENT_WALLET_LINK_PRIVACY_NOTICE)); + expect(PATIENT_WALLET_LINK_PRIVACY_NOTICE).toContain('public forever'); + expect(PATIENT_WALLET_LINK_PRIVACY_NOTICE).toContain('must say this plainly'); + }); +}); + +describe('funding cutoff guard (requirement 4)', () => { + const cutoff = config.payments.walletLinkUnverifiedFundingCutoff; + + it('the test environment has a valid cutoff', () => { + expect(Number.isNaN(cutoff)).toBe(false); + }); + + it('walletLinkFundingAllowed: legacy links work before the cutoff, never after; verified links always work', () => { + expect(walletLinkFundingAllowed('unverified_legacy', cutoff, cutoff - 1)).toBe(true); // 1ms before + expect(walletLinkFundingAllowed('unverified_legacy', cutoff, cutoff)).toBe(true); // boundary still allowed + expect(walletLinkFundingAllowed('unverified_legacy', cutoff, cutoff + 1)).toBe(false); // just past + expect(walletLinkFundingAllowed('unverified_legacy', cutoff, cutoff + 365 * 24 * 3600_000)).toBe(false); + + expect(walletLinkFundingAllowed('sep10', cutoff, cutoff - 1)).toBe(true); + expect(walletLinkFundingAllowed('sep10', cutoff, cutoff + 1)).toBe(true); + expect(walletLinkFundingAllowed('sep10', cutoff, Date.now())).toBe(true); + }); + + it('assertWalletLinkFundingAllowed passes for both kinds before the cutoff (default grace window)', () => { + // This suite runs with the DEFAULT cutoff (2027-01-01) โ€” well after + // "now", so legacy links are still inside the grace window here; + // the after-cutoff refusal path is integration-tested with an + // explicit past cutoff in tests/integration/wallet-link.test.mts. + expect(() => assertWalletLinkFundingAllowed({ verificationMethod: 'unverified_legacy' })).not.toThrow(); + expect(() => assertWalletLinkFundingAllowed({ verificationMethod: 'sep10' })).not.toThrow(); + }); +}); + +describe('wallet-link request schema (strict โ€” no raw addresses, ever)', () => { + it('accepts exactly { transaction }', () => { + expect(walletLinkSchema.parse({ transaction: 'AAAA' })).toEqual({ transaction: 'AAAA' }); + }); + + it('rejects a typed-in address, with or without a transaction', () => { + const address = 'G' + 'A'.repeat(55); + expect(() => walletLinkSchema.parse({ transaction: 'AAAA', stellarPublicKey: address })).toThrow(); + expect(() => walletLinkSchema.parse({ stellarPublicKey: address })).toThrow(); + expect(() => walletLinkSchema.parse({ account: address })).toThrow(); + expect(() => walletLinkSchema.parse({})).toThrow(); + expect(() => walletLinkSchema.parse({ transaction: '' })).toThrow(); + }); +}); + +describe('cutoff startup validation (requirement 4: validated at startup)', () => { + it('passes with the test suite configuration', () => { + expect(() => validateWalletLinkConfig()).not.toThrow(); + }); + + it('names the env var when the cutoff is malformed', async () => { + vi.resetModules(); + vi.stubEnv('WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF', 'not-a-date'); + + const fresh = await import('../../src/modules/payments/services/wallet-link.js'); + expect(() => fresh.validateWalletLinkConfig()).toThrow(/WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF/); + // Never echoes a value (this one isn't secret, but the rule is the + // rule โ€” same message shape as the other startup validators). + expect(() => fresh.validateWalletLinkConfig()).not.toThrow(/not-a-date/); + + vi.unstubAllEnvs(); + vi.resetModules(); + }); +}); diff --git a/docs/api/README.md b/docs/api/README.md index 0a21c2c..6a41bc0 100644 --- a/docs/api/README.md +++ b/docs/api/README.md @@ -153,11 +153,62 @@ drift apart. See `docs/adr/0003` for why production needs `SameSite=None`. ## Payments (`/payments`) โ€” `backend/src/modules/payments` -Custodial v1 escrow actions (`POST /payments/wallet-link`, -`GET/POST /payments/intents/...`) exist and are admin/facility-gated โ€” -see `docs/adr/0004-custodial-payments-v1.md` for why. The table below -covers the SEP-10 endpoints added for the non-custodial flow -(`docs/adr/0005`, issue #77). +Custodial v1 escrow actions (`GET/POST /payments/intents/...`) are +admin/facility-gated โ€” see `docs/adr/0004-custodial-payments-v1.md` for +why. Wallet linking and SEP-10 (below) are the non-custodial building +blocks from `docs/adr/0005` (issues #77/#78). + +### Wallet links (issue #78) + +Wallet links are **owner-scoped and verified**: a patient links the +wallet they will pay FROM, a facility the wallet they are paid TO. The +endpoint never accepts an address โ€” only a signed SEP-10 challenge โ€” +and the address is derived server-side from the account that challenge +was verified for. + +| Method & path | Auth | Body | Notes | +|---|---|---|---| +| `POST /payments/wallet-link` | access token, role `patient` or `facility`, + verified email | `{ transaction }` (signed SEP-10 challenge XDR) | Creates (or replaces) the caller's OWN active link: `200 { id, ownerType, ownerId, stellarPublicKey, verificationMethod: 'sep10', verifiedAt, replacedAt: null, createdAt, updatedAt }`. Patient responses additionally carry `privacyNotice` (see below). Link creation and the SEP-10 nonce consumption commit in one transaction โ€” a failed verification creates no link and burns no nonce. | +| `GET /payments/wallet-link` | access token, role `patient` or `facility` | โ€” | The caller's active link, or `404 { message: 'No wallet linked yet.' }`. Replaced links are history and are not returned. | + +**Rules:** + +- **One active link per owner**, enforced by a DB unique constraint โ€” + concurrent creates race it (`409 WALLET_LINK_CONFLICT` for the loser) + rather than ever producing two. Re-linking deactivates the previous + link (`replacedAt` set) instead of mutating or deleting it, so history + survives; already-funded escrows keep referencing their own address + (snapshot in `PaymentIntent.fundedWalletAddress`). +- **Patient privacy notice:** every patient link response includes + `privacyNotice` โ€” ADR 0005's on-chain privacy risk text **verbatim** + (word-for-word; a unit test checks it against the ADR file), so a + client cannot render the success screen without carrying it: + > **Privacy risk: health information can leak on-chain.** A payment + > from a patient's wallet to a facility's wallet is public forever. โ€ฆ + (full text in the response body / `services/wallet-link.ts`). + The ADR mandates the *display* but is silent on an acknowledgement + field โ€” none is required (open design decision in the PR). + +**Errors:** + +| Status | `code` | Meaning | +|---|---|---| +| 400 | โ€” | Malformed body, or **any raw-address field** (`stellarPublicKey`, `account`, โ€ฆ) โ€” the schema is strict on purpose. | +| 401 | `SEP10_*` | Challenge invalid/untrusted/replayed/expired โ€” same codes as `POST /payments/sep10/verify`; no link is created and the nonce is not consumed. | +| 403 | `SEP10_CHALLENGE_NOT_FOR_CALLER` | Challenge was issued to a different user/account. | +| 403 | โ€” | Role cannot own a link (doctor/admin), no profile, or email not verified (`EMAIL_NOT_VERIFIED`). | +| 409 | `WALLET_LINK_CONFLICT` | A concurrent request created the owner's link first. | +| 404 | โ€” | `GET` with no active link. | + +**Legacy links & the funding cutoff:** existing facility links +(typed-in before this change) migrated to `verificationMethod: +'unverified_legacy'`, `verifiedAt: null`. Escrow funding keeps accepting +them **before** `WALLET_LINK_UNVERIFIED_FUNDING_CUTOFF` (ISO date, +validated at startup, default `2027-01-01T00:00:00Z`, see +`.env.example`); **after** it, funding a facility whose link is not +`sep10` fails with `409 { code: 'WALLET_LINK_UNVERIFIED' }` (enforced in +the funding service โ€” re-link with a SEP-10-verified wallet to resume). +Verified links fund normally before and after the cutoff. ### SEP-10 Stellar Web Authentication (issue #77)