From cecc2bc637c3822af88fce0fc032b4bd04a045ae Mon Sep 17 00:00:00 2001 From: Simon Axelsson Date: Tue, 15 Sep 2026 19:09:53 +0200 Subject: [PATCH] fix(payments): authenticate order detail and deposit checkout --- app/api/orders/[id]/route.ts | 4 +- app/api/payments/[order_id]/deposit/route.ts | 3 + docs/architecture/ADR-001-order-ownership.md | 58 ++++++++++++++++++++ docs/architecture/ARCHITECTURE.md | 37 +++++++------ tests/api/anonymous-routes-auth.test.ts | 16 ++++++ tests/api/stripe-deposit-metadata.test.ts | 3 + 6 files changed, 104 insertions(+), 17 deletions(-) create mode 100644 docs/architecture/ADR-001-order-ownership.md diff --git a/app/api/orders/[id]/route.ts b/app/api/orders/[id]/route.ts index 4daf1c8..f029c03 100644 --- a/app/api/orders/[id]/route.ts +++ b/app/api/orders/[id]/route.ts @@ -3,7 +3,9 @@ import { getOrder, setStatusIfCurrent } from "@/lib/orders-store"; import { requireRouteAuth } from "@/src/auth/require-route-auth"; // GET /api/orders/{id} -export async function GET(_req: NextRequest, ctx: { params: Promise<{ id: string }> }) { +export async function GET(req: NextRequest, ctx: { params: Promise<{ id: string }> }) { + const auth = await requireRouteAuth(req); + if (auth.response) return auth.response; const { id } = await ctx.params; const order = await getOrder(id); if (!order) return NextResponse.json({ error: { code: "not_found", message: "Order saknas" } }, { status: 404 }); diff --git a/app/api/payments/[order_id]/deposit/route.ts b/app/api/payments/[order_id]/deposit/route.ts index 33715ec..2e1e5d8 100644 --- a/app/api/payments/[order_id]/deposit/route.ts +++ b/app/api/payments/[order_id]/deposit/route.ts @@ -1,5 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { getOrder } from "@/lib/orders-store"; +import { requireRouteAuth } from "@/src/auth/require-route-auth"; // POST /api/payments/[order_id]/deposit // Returns { url } for Stripe Checkout. Returns 501 when STRIPE_SECRET_KEY is absent. @@ -7,6 +8,8 @@ export async function POST( request: NextRequest, { params }: { params: Promise<{ order_id: string }> }, ) { + const auth = await requireRouteAuth(request); + if (auth.response) return auth.response; const { order_id } = await params; const secretKey = process.env.STRIPE_SECRET_KEY; diff --git a/docs/architecture/ADR-001-order-ownership.md b/docs/architecture/ADR-001-order-ownership.md new file mode 100644 index 0000000..f41b873 --- /dev/null +++ b/docs/architecture/ADR-001-order-ownership.md @@ -0,0 +1,58 @@ +# ADR-001: Verified tenant ownership for payment orders + +**Status:** Proposed +**Date:** 2026-09-15 +**Deciders:** SIAX 0pt product and platform owners + +## Context + +The existing order configurator creates anonymous drafts, stores no verified +buyer or tenant on them, and later refers to an order by its ID. A valid +Zitadel principal proves who is calling but does not prove that this caller +owns that particular order. An order ID is an identifier, not an authorization +secret. The nullable `orders.org_id` UUID cannot directly hold the verified +tenant ID, which is a string. The repository has no automated SQL migration +runner, and legacy rows may remain unowned. + +The payment flow must remain API-first and work consistently in UI, CLI and +MCP. It must not make anonymous drafts payable by treating a guessed or leaked +ID as ownership. + +## Decision + +Require a verified Zitadel principal before creating a payable order. Persist +`tenant_id` and buyer subject on each new order in a database migration. +Every read, status mutation and Checkout creation must use a tenant-scoped +query predicate; a cross-tenant order returns 404. The Stripe webhook uses +its verified signature plus recorded order ID and payment facts to advance +status, never a browser-supplied tenant claim. Legacy null-owned orders remain +unpayable until an audited, explicit migration or support workflow binds +them to a verified buyer. Keep the anonymous configurator as an estimate +without creating a payable order until sign-in. + +## Options considered + +| Option | Complexity | Scale | Consequence | +|---|---|---|---| +| Authenticate only, lookup by order ID | Low | High | Rejected: any signed-in user can access another order. | +| Bearer claim token for anonymous drafts | Medium | High | Requires capability issuance, hashing, secure browser storage and recovery. Not selected for initial production. | +| Verified tenant + buyer subject at order creation | Medium | High | Chosen: explicit ownership at source, shared API boundary. | + +## Consequences + +- UI must obtain a Zitadel session before order creation and send credentials + to the order and payment APIs; estimates stay anonymous. +- SQL migration must be applied and verified before the scoped code is deployed. +- Existing unowned rows are not silently assigned to a tenant. +- Idempotency keys must be scoped to the verified tenant. +- Admin operations require an explicit, audited role-bound path rather than + bypassing the buyer predicate. + +## Action items + +1. [ ] Build and verify the migration and deploy runner. +2. [ ] Bind create-order to verified tenant and buyer subject; scope idempotency. +3. [ ] Scope GET/PATCH/deposit/release to tenant at the SQL boundary. +4. [ ] Wire UI Zitadel login/session and CLI/MCP bearer auth; keep anonymous estimates. +5. [ ] Test cross-tenant 404, legacy unowned rejection, webhook, and staging + purchase/refund end-to-end before enabling live Stripe. diff --git a/docs/architecture/ARCHITECTURE.md b/docs/architecture/ARCHITECTURE.md index f528720..48cbec2 100644 --- a/docs/architecture/ARCHITECTURE.md +++ b/docs/architecture/ARCHITECTURE.md @@ -138,11 +138,11 @@ Risknivå = SIAX L0–L5-taxonomin, bedömd av mig utifrån vad handlern gör. | Sanktions-/PEP-screening | `GET /api/screen?q=` | L2 | Nej — proxar mot OpenSanctions | | AI-parsning av fritext | `POST /api/ai/parse` | L2 | Nej | | Skapa order | `POST /api/orders` | L2 | Nej | -| **Lista alla ordrar** | `GET /api/orders` | **L3** | **Nej** — ingen org-scoping *(live: 200, `[]`)* | -| Läs/ändra order | `GET,PATCH /api/orders/{id}` | L3 | Nej | -| **Adminmetrik (orderpipeline, belopp)** | `GET /api/admin/metrics` | **L3** | **Nej** *(live-verifierat: 200 anonymt)* | -| Deposition (Stripe checkout) | `POST /api/payments/{order_id}/deposit` | **L4** | Nej | -| **Frigör escrow-slutbetalning** | `POST /api/payments/{order_id}/release` | **L4** | **Nej** — sätter dessutom status `delivered` (§8 S4) | +| **Lista alla ordrar** | `GET /api/orders` | **L3** | Route-auth i source; org-scoping saknas | +| Läs/ändra order | `GET,PATCH /api/orders/{id}` | L3 | Route-auth i source; orderägande saknas | +| **Adminmetrik (orderpipeline, belopp)** | `GET /api/admin/metrics` | **L3** | Route-auth i source; äldre liveobservation ej omverifierad | +| Deposition (Stripe checkout) | `POST /api/payments/{order_id}/deposit` | **L4** | Route-auth i source; orderägande saknas | +| **Frigör escrow-slutbetalning** | `POST /api/payments/{order_id}/release` | **L4** | Route-auth i source; `delivered` först efter betald webhook; orderägande saknas | | Stripe-webhook | `POST /api/webhooks/stripe` | L3 | Signaturkontroll via `STRIPE_WEBHOOK_SECRET` | | Prenumerationer | `GET,POST /api/subscriptions` | L2 | Nej | | Filuppladdning | `GET,POST /api/uploads` | L3 | Nej (`501` utan R2) | @@ -356,6 +356,9 @@ Om/när Postgres provisioneras måste detta lösas separat — jfr estate-blocke också verifierad auth. **Fortfarande öppet före live-läge:** orderägarskap och tenant-bindning vid checkout/acceptance behöver implementeras och verifieras; en verifierad identitet ensam är inte per-order auktorisation. + Enskild order-GET och deposit-checkout kräver nu också route-auth; dessa + två vägar är inte längre anonyma i source. Ägarbindningens beslut och + beroenden dokumenteras i `ADR-001-order-ownership.md`. - `SECURITY.md` finns med rapporteringsväg (`security@siax.io`). ### KÄNDA LUCKOR @@ -387,18 +390,20 @@ alltså en innehavare av ett giltigt token för tenant A skicka `X-Tenant-Id`-konflikt hjälper inte — den kontrollerar bara *headern*, inte *bodyn*. **Tenantisolering är inte implementerad.** -**S3 — Hela det icke-`/api/0pt`-prefixade API:t saknar auth.** -Matchern täcker bara `/api/0pt/:path*`. Ordrar, adminmetrik, betalningar, -prenumerationer, uppladdningar, AI-parsning och DB-grants är oskyddade i koden. -*(live-verifierat: `GET /api/admin/metrics` och `GET /api/orders` svarar 200 -anonymt.)* De är tomma idag enbart för att produktionsinstansen saknar databas. +**S3 — Historiskt anonymt icke-`/api/0pt`-API; delvis åtgärdat i source.** +Matchern täcker bara `/api/0pt/:path*`. Orderkö/adminmetrik, enskild +order-GET, deposit- och release-checkout samt order-PATCH har nu explicita +route-auth-grindar i source. De äldre live-observationerna av anonymt 200 +beskriver tidigare deploy, inte verifierat nuläge. Övriga icke-prefixade +API:er kräver egen inventering, och per-order tenantägande är fortfarande +öppet enligt ADR-001. -**S4 — Anonym anropare kan trigga escrow-slutbetalning och flagga leverans.** -`POST /api/payments/{order_id}/release` kräver ingen identitet. Den skapar en -Stripe-checkout på `order.balance_ore` och kör därefter -`setStatus(order_id, "delivered")` **oavsett om betalningen genomförs**. -Statusövergången sker alltså på begäran, inte på betalning. (Skyddas i praktiken -idag bara av att `getOrder` inte hittar något utan databas.) +**S4 — Historisk escrow-bugg åtgärdad i source; tenantägande återstår.** +`POST /api/payments/{order_id}/release` kräver nu route-auth och skapar +Checkout först efter `accepted`. En skapad session sätter inte `delivered`; +endast en signerad, betald och matchande Stripe-webhook får göra det. +Cross-tenant-skydd av den enskilda ordern saknas fortfarande, så livebetalning +får inte aktiveras innan ADR-001 är genomförd och verifierad. **S5 — CL0UD-policyklienten är fail-open utan produktionsspärr.** `src/clients/cl0ud-client.ts:45` — saknas `CL0UD_URL` returneras alltid diff --git a/tests/api/anonymous-routes-auth.test.ts b/tests/api/anonymous-routes-auth.test.ts index 0ef2fd3..712ea48 100644 --- a/tests/api/anonymous-routes-auth.test.ts +++ b/tests/api/anonymous-routes-auth.test.ts @@ -15,7 +15,9 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { NextRequest } from 'next/server'; import { GET as ordersGET } from '@/app/api/orders/route'; +import { GET as orderGET } from '@/app/api/orders/[id]/route'; import { GET as adminMetricsGET } from '@/app/api/admin/metrics/route'; +import { POST as depositPOST } from '@/app/api/payments/[order_id]/deposit/route'; function req(url: string, headers: Record = {}): NextRequest { return new NextRequest(new URL(url, 'http://localhost'), { headers }); @@ -49,6 +51,20 @@ describe('anonymous-access regression: /api/orders (GET) and /api/admin/metrics expect(res.status).toBe(401); }); + it('GET /api/orders/{id} rejects anonymous and spoofed-tenant reads before loading an order', async () => { + const params = { params: Promise.resolve({ id: 'order-unknown' }) }; + expect((await orderGET(req('/api/orders/order-unknown'), params)).status).toBe(401); + expect((await orderGET(req('/api/orders/order-unknown', { + 'x-0pt-verified-tenant': 'tenant_attacker', + }), params)).status).toBe(401); + }); + + it('deposit checkout rejects anonymous callers before checking Stripe credentials or order id', async () => { + const request = req('/api/payments/order-unknown/deposit'); + const result = await depositPOST(request, { params: Promise.resolve({ order_id: 'order-unknown' }) }); + expect(result.status).toBe(401); + }); + it('REGRESSION: GET /api/admin/metrics 401s with no Authorization header (was: 200 + financial aggregates)', async () => { const res = await adminMetricsGET(req('/api/admin/metrics')); expect(res.status).toBe(401); diff --git a/tests/api/stripe-deposit-metadata.test.ts b/tests/api/stripe-deposit-metadata.test.ts index aa40223..fea1cfd 100644 --- a/tests/api/stripe-deposit-metadata.test.ts +++ b/tests/api/stripe-deposit-metadata.test.ts @@ -4,6 +4,9 @@ import { NextRequest } from "next/server"; vi.mock("@/lib/orders-store", () => ({ getOrder: vi.fn().mockResolvedValue({ order_id: "order-1", deposit_ore: 5000 }), })); +vi.mock("@/src/auth/require-route-auth", () => ({ + requireRouteAuth: vi.fn().mockResolvedValue({ ctx: { tenantId: "tenant-test" } }), +})); import { POST } from "@/app/api/payments/[order_id]/deposit/route";