From 15a0dbe3b15fe7a9d73d0ff21bf828aa4544f4e8 Mon Sep 17 00:00:00 2001 From: Simon Axelsson Date: Tue, 11 Aug 2026 04:06:13 +0200 Subject: [PATCH] fix(security): require auth on GET /api/orders and GET /api/admin/metrics A live compliance audit against 0pt.app confirmed both endpoints returned HTTP 200 with zero authentication -- the full order queue and financial pipeline aggregates (deposit/pipeline totals) were readable by anyone. Root cause: middleware.ts's matcher only covers /api/0pt/:path*, by design ("does NOT touch ... the unrelated payment/order APIs"), so nothing upstream of these two handlers ever verified the caller. Fix: both GET handlers now call the same fail-closed Zitadel verifier /api/0pt/* relies on (resolveAuthContext, src/auth/context.ts) directly, via a new thin wrapper (src/auth/require-route-auth.ts) that maps a rejected decision to the same 401/403 JSON shape middleware.ts already returns. The wrapper re-verifies the bearer token itself rather than trusting any x-0pt-verified-* header on the request, since middleware never runs in front of these routes to overwrite a client-supplied one. POST /api/orders (anonymous draft/quote creation before the deposit-payment step) is intentionally left as-is -- only the read paths that leaked data are gated. Tracked as part of docs/compliance/BACKLOG.md P0-3. Co-Authored-By: Claude Sonnet 5 --- app/api/admin/metrics/route.ts | 12 +++- app/api/orders/route.ts | 14 ++++- src/auth/require-route-auth.ts | 50 +++++++++++++++ tests/api/anonymous-routes-auth.test.ts | 84 +++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 src/auth/require-route-auth.ts create mode 100644 tests/api/anonymous-routes-auth.test.ts diff --git a/app/api/admin/metrics/route.ts b/app/api/admin/metrics/route.ts index 1fd5a4e..5d64deb 100644 --- a/app/api/admin/metrics/route.ts +++ b/app/api/admin/metrics/route.ts @@ -1,8 +1,14 @@ -import { NextResponse } from "next/server"; +import { NextRequest, NextResponse } from "next/server"; import { listOrders } from "@/lib/orders-store"; +import { requireRouteAuth } from "@/src/auth/require-route-auth"; -// GET /api/admin/metrics — order queue aggregates for the admin dashboard. -export async function GET() { +// GET /api/admin/metrics — order queue + financial aggregates for the admin +// dashboard. Outside middleware.ts's matcher (/api/0pt/:path* only) and +// previously unauthenticated entirely, so auth is enforced here directly via +// the same fail-closed Zitadel verifier — see src/auth/require-route-auth.ts. +export async function GET(req: NextRequest) { + const auth = await requireRouteAuth(req); + if (auth.response) return auth.response; const orders = await listOrders(); const byStatus: Record = {}; let depositPaidOre = 0; diff --git a/app/api/orders/route.ts b/app/api/orders/route.ts index a4cf033..1f06010 100644 --- a/app/api/orders/route.ts +++ b/app/api/orders/route.ts @@ -1,7 +1,11 @@ import { NextRequest, NextResponse } from "next/server"; import { createOrder, listOrders, type OrderConfig } from "@/lib/orders-store"; +import { requireRouteAuth } from "@/src/auth/require-route-auth"; // POST /api/orders (docs/04) — create a draft order from a configurator config. +// Intentionally anonymous: a prospect drafts/estimates a job before signing in +// to pay the deposit (see docs/01, the configurator flow). Only reading back +// the order queue (GET, below) is a data-exposure risk. export async function POST(req: NextRequest) { let body: { flow?: string; config?: OrderConfig }; try { @@ -18,8 +22,14 @@ export async function POST(req: NextRequest) { return NextResponse.json({ order_id: order.order_id, status: order.status, deposit_ore: order.deposit_ore, total_ore: order.total_ore }, { status: 201 }); } -// GET /api/orders — list (org scoping added with auth). -export async function GET() { +// GET /api/orders — list all orders (queue + financial aggregates). This is +// outside middleware.ts's matcher (/api/0pt/:path* only), so auth is enforced +// here directly via the same fail-closed Zitadel verifier — see +// src/auth/require-route-auth.ts for why the request is re-verified rather +// than trusting a forwarded header. +export async function GET(req: NextRequest) { + const auth = await requireRouteAuth(req); + if (auth.response) return auth.response; const orders = await listOrders(); return NextResponse.json(orders); } diff --git a/src/auth/require-route-auth.ts b/src/auth/require-route-auth.ts new file mode 100644 index 0000000..d153f86 --- /dev/null +++ b/src/auth/require-route-auth.ts @@ -0,0 +1,50 @@ +/** + * `0pt/auth/require-route-auth.ts` -- thin Next.js wrapper around + * `resolveAuthContext` (src/auth/context.ts) for API route handlers that sit + * OUTSIDE `middleware.ts`'s matcher (`/api/0pt/:path*` only -- see the + * comment at the top of middleware.ts: "does NOT touch ... the unrelated + * payment/order APIs"). + * + * `/api/orders` (GET) and `/api/admin/*` are such routes: nothing upstream of + * the handler verifies the caller today, so the handler must call + * `resolveAuthContext` itself. Critically, it must NOT trust any + * `x-0pt-verified-*` header found on the incoming request as a shortcut -- + * those headers are only trustworthy on routes middleware.ts actually runs in + * front of, where it overwrites them with its own verified values. A client + * hitting one of these unmatched routes directly controls every request + * header, including `x-0pt-verified-tenant`, so trusting it here would be a + * trivial bypass. This wrapper always re-verifies via `resolveAuthContext` + * (real Zitadel JWT verification, fail-closed by default -- see context.ts), + * exactly as middleware.ts does for `/api/0pt/*`. + * + * Mirrors middleware.ts's decision -> HTTP response mapping exactly, so a + * caller sees the identical 401/403 JSON shape whether the check ran in + * middleware or here. + */ +import { NextResponse, type NextRequest } from 'next/server'; +import { resolveAuthContext, type OptRequestContext } from './context'; + +export type RouteAuthResult = { response: NextResponse } | { response?: undefined; ctx: OptRequestContext }; + +/** + * Enforce authentication for a route handler not covered by middleware.ts's + * matcher. Returns `{ response }` (send immediately) when unauthenticated or + * unauthorized, or `{ ctx }` (the verified request context) when the caller + * may proceed. + */ +export async function requireRouteAuth(req: NextRequest): Promise { + const decision = await resolveAuthContext(req.headers); + if (!decision.ok) { + return { + response: NextResponse.json( + { + error: decision.code, + message: decision.message, + ...(decision.details ? { details: decision.details } : {}), + }, + { status: decision.status }, + ), + }; + } + return { ctx: decision.ctx }; +} diff --git a/tests/api/anonymous-routes-auth.test.ts b/tests/api/anonymous-routes-auth.test.ts new file mode 100644 index 0000000..0ef2fd3 --- /dev/null +++ b/tests/api/anonymous-routes-auth.test.ts @@ -0,0 +1,84 @@ +/** + * Regression tests for the anonymous-access finding on `/api/orders` (GET) + * and `/api/admin/metrics` (GET): a live compliance audit confirmed both + * responded HTTP 200 with zero authentication (the order queue + financial + * pipeline aggregates were readable by anyone). Both are outside + * middleware.ts's matcher (`/api/0pt/:path*` only), so each handler now + * calls `requireRouteAuth` (src/auth/require-route-auth.ts -> the same + * fail-closed `resolveAuthContext`/Zitadel verifier `/api/0pt/*` relies on) + * directly. + * + * No DATABASE_URL is set in this test env, so lib/orders-store.ts falls back + * to its in-memory store -- these tests only exercise the auth gate, not + * order persistence. + */ +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 adminMetricsGET } from '@/app/api/admin/metrics/route'; + +function req(url: string, headers: Record = {}): NextRequest { + return new NextRequest(new URL(url, 'http://localhost'), { headers }); +} + +describe('anonymous-access regression: /api/orders (GET) and /api/admin/metrics (GET)', () => { + const saved = { auth: process.env.AUTH_ENFORCE, node: process.env.NODE_ENV }; + + beforeEach(() => { + // Default posture: AUTH_ENFORCE unset => enforced (fail-closed), matching + // production and matching how resolveAuthContext's own tests run. + delete process.env.AUTH_ENFORCE; + }); + + afterEach(() => { + if (saved.auth === undefined) delete process.env.AUTH_ENFORCE; + else process.env.AUTH_ENFORCE = saved.auth; + if (saved.node === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = saved.node; + }); + + it('REGRESSION: GET /api/orders 401s with no Authorization header (was: 200 + full order list)', async () => { + const res = await ordersGET(req('/api/orders')); + expect(res.status).toBe(401); + const body = await res.json(); + expect(body.error).toBe('OPT_UNAUTHENTICATED'); + }); + + it('GET /api/orders 401s on an invalid/unparseable bearer token', async () => { + const res = await ordersGET(req('/api/orders', { authorization: 'Bearer not.a.real.jwt' })); + expect(res.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); + const body = await res.json(); + expect(body.error).toBe('OPT_UNAUTHENTICATED'); + }); + + it('a bare X-Tenant-Id header alone (no bearer token) does not grant access to either route', async () => { + // Guards against a shortcut implementation that trusts a client-supplied + // tenant/verified header instead of re-verifying the token. + const ordersRes = await ordersGET(req('/api/orders', { 'x-tenant-id': 'tenant_attacker' })); + expect(ordersRes.status).toBe(401); + + const metricsRes = await adminMetricsGET(req('/api/admin/metrics', { 'x-tenant-id': 'tenant_attacker' })); + expect(metricsRes.status).toBe(401); + }); + + it('a spoofed x-0pt-verified-tenant header alone does not grant access (never trusted directly)', async () => { + // These routes sit outside middleware.ts's matcher, so nothing strips or + // overwrites this header before the handler sees it -- proving the + // handler re-verifies via the real JWT check rather than trusting it. + const res = await ordersGET(req('/api/orders', { 'x-0pt-verified-tenant': 'tenant_attacker' })); + expect(res.status).toBe(401); + }); + + it('AUTH_ENFORCE=false (local-dev escape hatch) still allows both routes through, unauthenticated', async () => { + process.env.AUTH_ENFORCE = 'false'; + const ordersRes = await ordersGET(req('/api/orders')); + expect(ordersRes.status).toBe(200); + const metricsRes = await adminMetricsGET(req('/api/admin/metrics')); + expect(metricsRes.status).toBe(200); + }); +}); -- 2.54.0