From 26573996281d4679a037051a1ac2054d025566e0 Mon Sep 17 00:00:00 2001 From: Dylan Garvis Date: Sat, 1 Aug 2026 14:42:19 -0400 Subject: [PATCH] feat(api): standardize errors as problem details --- README.md | 2 +- apps/web/package.json | 4 +- apps/web/src/app/api/[...path]/route.test.ts | 18 +++ apps/web/src/app/api/[...path]/route.ts | 20 ++++ apps/web/src/app/api/route.ts | 19 +++ .../src/app/api/velocity/access/route.test.ts | 78 +++++++++++++ apps/web/src/app/api/velocity/access/route.ts | 110 +++++++++++++++--- apps/web/src/lib/database-errors.test.ts | 17 +++ apps/web/src/lib/database-errors.ts | 23 ++++ apps/web/src/lib/problem-response.test.ts | 29 +++++ apps/web/src/lib/problem-response.ts | 16 +++ apps/web/vitest.config.mts | 10 ++ docs/api-errors.md | 43 +++++++ package-lock.json | 3 +- packages/contracts/src/index.ts | 29 +++++ .../contracts/test/problem-details.test.ts | 37 ++++++ 16 files changed, 441 insertions(+), 17 deletions(-) create mode 100644 apps/web/src/app/api/[...path]/route.test.ts create mode 100644 apps/web/src/app/api/[...path]/route.ts create mode 100644 apps/web/src/app/api/route.ts create mode 100644 apps/web/src/app/api/velocity/access/route.test.ts create mode 100644 apps/web/src/lib/database-errors.test.ts create mode 100644 apps/web/src/lib/database-errors.ts create mode 100644 apps/web/src/lib/problem-response.test.ts create mode 100644 apps/web/src/lib/problem-response.ts create mode 100644 apps/web/vitest.config.mts create mode 100644 docs/api-errors.md create mode 100644 packages/contracts/test/problem-details.test.ts diff --git a/README.md b/README.md index 4d199bb..356ddfd 100644 --- a/README.md +++ b/README.md @@ -72,4 +72,4 @@ The token is displayed once and stored only as a SHA-256 hash. - Velocity admission checks are fail closed - ProxyCheck.io geolocation and VPN/proxy/Tor detection with a 48-hour PostgreSQL cache -See [`docs/architecture.md`](docs/architecture.md) for trust boundaries and service responsibilities, and [`docs/security-review.md`](docs/security-review.md) for implemented controls and production requirements. +See [`docs/architecture.md`](docs/architecture.md) for trust boundaries and service responsibilities, [`docs/api-errors.md`](docs/api-errors.md) for the RFC 9457 API error contract, and [`docs/security-review.md`](docs/security-review.md) for implemented controls and production requirements. diff --git a/apps/web/package.json b/apps/web/package.json index 9a8201f..87d10d1 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -6,6 +6,7 @@ "dev": "next dev", "build": "next build", "start": "next start", + "test": "vitest run", "lint": "eslint .", "typecheck": "tsc --noEmit" }, @@ -29,6 +30,7 @@ "eslint": "^9.39.4", "eslint-config-next": "^16.2.1", "tailwindcss": "^4.2.1", - "typescript": "^5.9.3" + "typescript": "^5.9.3", + "vitest": "^4.1.0" } } diff --git a/apps/web/src/app/api/[...path]/route.test.ts b/apps/web/src/app/api/[...path]/route.test.ts new file mode 100644 index 0000000..f127074 --- /dev/null +++ b/apps/web/src/app/api/[...path]/route.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { GET } from "./route"; + +describe("unknown API routes", () => { + it("return an RFC 9457 not-found problem", async () => { + const response = GET(new Request("http://localhost/api/does-not-exist")); + + expect(response.status).toBe(404); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(await response.json()).toEqual({ + type: "urn:error:not-found", + title: "API route not found", + status: 404, + detail: "The requested API route does not exist.", + instance: "/api/does-not-exist", + }); + }); +}); diff --git a/apps/web/src/app/api/[...path]/route.ts b/apps/web/src/app/api/[...path]/route.ts new file mode 100644 index 0000000..043adb9 --- /dev/null +++ b/apps/web/src/app/api/[...path]/route.ts @@ -0,0 +1,20 @@ +import { problemDetails } from "@minecraft-account-manager/contracts"; +import { problemInstance, problemResponse } from "@/lib/problem-response"; + +function notFound(request: Request) { + const instance = problemInstance(request); + return problemResponse(problemDetails( + "urn:error:not-found", + "API route not found", + 404, + "The requested API route does not exist.", + instance, + )); +} + +export const GET = notFound; +export const POST = notFound; +export const PUT = notFound; +export const PATCH = notFound; +export const DELETE = notFound; +export const OPTIONS = notFound; diff --git a/apps/web/src/app/api/route.ts b/apps/web/src/app/api/route.ts new file mode 100644 index 0000000..914a696 --- /dev/null +++ b/apps/web/src/app/api/route.ts @@ -0,0 +1,19 @@ +import { problemDetails } from "@minecraft-account-manager/contracts"; +import { problemInstance, problemResponse } from "@/lib/problem-response"; + +function notFound(request: Request) { + return problemResponse(problemDetails( + "urn:error:not-found", + "API route not found", + 404, + "The requested API route does not exist.", + problemInstance(request), + )); +} + +export const GET = notFound; +export const POST = notFound; +export const PUT = notFound; +export const PATCH = notFound; +export const DELETE = notFound; +export const OPTIONS = notFound; diff --git a/apps/web/src/app/api/velocity/access/route.test.ts b/apps/web/src/app/api/velocity/access/route.test.ts new file mode 100644 index 0000000..b395e9d --- /dev/null +++ b/apps/web/src/app/api/velocity/access/route.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from "vitest"; +import { GET, POST } from "./route"; + +describe("Velocity access API problems", () => { + it("returns RFC 9457 for unsupported methods", async () => { + const response = GET(new Request("http://localhost/api/velocity/access")); + + expect(response.status).toBe(405); + expect(response.headers.get("allow")).toBe("POST"); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(await response.json()).toMatchObject({ + type: "urn:error:method-not-allowed", + title: "Method not allowed", + status: 405, + instance: "/api/velocity/access", + }); + }); + + it("returns RFC 9457 when credentials are missing", async () => { + const response = await POST(new Request("http://localhost/api/velocity/access", { + method: "POST", + body: JSON.stringify({}), + })); + + expect(response.status).toBe(401); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(await response.json()).toEqual({ + type: "urn:error:unauthorized", + title: "Unauthorized", + status: 401, + detail: "A valid Velocity server credential is required.", + instance: "/api/velocity/access", + }); + }); + + it("rejects unsupported media types as Problem Details", async () => { + const response = await POST(new Request("http://localhost/api/velocity/access", { + method: "POST", + headers: { + authorization: "Bearer test-token", + "content-type": "text/plain", + }, + body: "not json", + })); + + expect(response.status).toBe(415); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(await response.json()).toMatchObject({ + type: "urn:error:unsupported-media-type", + title: "Unsupported media type", + status: 415, + instance: "/api/velocity/access", + }); + }); + + it("includes machine-readable validation issues", async () => { + const response = await POST(new Request("http://localhost/api/velocity/access", { + method: "POST", + headers: { + authorization: "Bearer test-token", + "content-type": "application/json", + }, + body: JSON.stringify({ serverId: "velocity-main" }), + })); + const problem = await response.json(); + + expect(response.status).toBe(400); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(problem).toMatchObject({ + type: "urn:error:invalid-velocity-access-request", + title: "Invalid Velocity access request", + status: 400, + instance: "/api/velocity/access", + extensions: { issues: expect.any(Array) }, + }); + expect(problem.extensions.issues.length).toBeGreaterThan(0); + }); +}); diff --git a/apps/web/src/app/api/velocity/access/route.ts b/apps/web/src/app/api/velocity/access/route.ts index 7b211f5..bd4d187 100644 --- a/apps/web/src/app/api/velocity/access/route.ts +++ b/apps/web/src/app/api/velocity/access/route.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { isRequestTimestampFresh, verifyHashedToken } from "@minecraft-account-manager/auth"; -import { velocityAccessRequestSchema } from "@minecraft-account-manager/contracts"; +import { problemDetails, velocityAccessRequestSchema } from "@minecraft-account-manager/contracts"; import { appSettings, events, @@ -12,25 +12,83 @@ import { import { and, eq, isNull, lt, sql } from "drizzle-orm"; import { NextResponse } from "next/server"; import { db } from "@/lib/database"; +import { isUniqueConstraintViolation } from "@/lib/database-errors"; import { getIpIntelligence, toAuditIpData } from "@/lib/ip-intelligence"; +import { problemInstance, problemResponse } from "@/lib/problem-response"; const MAX_CLOCK_SKEW_MS = 45_000; const DEFAULT_DENIAL_MESSAGE = "Please register your Minecraft account before joining."; -export async function POST(request: Request) { +function methodNotAllowed(request: Request) { + const response = problemResponse(problemDetails( + "urn:error:method-not-allowed", + "Method not allowed", + 405, + "This endpoint only accepts POST requests.", + problemInstance(request), + )); + response.headers.set("allow", "POST"); + return response; +} + +export const GET = methodNotAllowed; +export const PUT = methodNotAllowed; +export const PATCH = methodNotAllowed; +export const DELETE = methodNotAllowed; + +async function handleVelocityAccess(request: Request) { + const instance = problemInstance(request); const authorization = request.headers.get("authorization") ?? ""; const token = authorization.startsWith("Bearer ") ? authorization.slice(7).trim() : ""; - if (!token) return NextResponse.json({ allowed: false, message: "Unauthorized" }, { status: 401 }); + if (!token) { + return problemResponse(problemDetails( + "urn:error:unauthorized", + "Unauthorized", + 401, + "A valid Velocity server credential is required.", + instance, + )); + } + + const mediaType = request.headers.get("content-type")?.split(";", 1)[0]?.trim().toLowerCase(); + if (mediaType !== "application/json") { + return problemResponse(problemDetails( + "urn:error:unsupported-media-type", + "Unsupported media type", + 415, + "Velocity access requests must use application/json.", + instance, + )); + } const parsed = velocityAccessRequestSchema.safeParse(await request.json().catch(() => null)); if (!parsed.success) { - return NextResponse.json({ allowed: false, message: "Invalid access request" }, { status: 400 }); + return problemResponse(problemDetails( + "urn:error:invalid-velocity-access-request", + "Invalid Velocity access request", + 400, + "The request body does not match the required Velocity access contract.", + instance, + { + issues: parsed.error.issues.map((issue) => ({ + path: issue.path.join("."), + message: issue.message, + code: issue.code, + })), + }, + )); } const input = parsed.data; const occurredAt = new Date(input.occurredAt); if (!isRequestTimestampFresh(occurredAt, new Date(), MAX_CLOCK_SKEW_MS)) { - return NextResponse.json({ allowed: false, message: "Expired access request" }, { status: 401 }); + return problemResponse(problemDetails( + "urn:error:expired-velocity-access-request", + "Expired Velocity access request", + 401, + "The request timestamp is outside the allowed clock-skew window.", + instance, + )); } const [credential] = await db @@ -40,7 +98,13 @@ export async function POST(request: Request) { .limit(1); if (!credential || !verifyHashedToken(token, credential.secretHash)) { - return NextResponse.json({ allowed: false, message: "Unauthorized" }, { status: 401 }); + return problemResponse(problemDetails( + "urn:error:unauthorized", + "Unauthorized", + 401, + "The Velocity server credential is invalid or revoked.", + instance, + )); } const [settings] = await db.select().from(appSettings).where(eq(appSettings.id, "default")).limit(1); @@ -75,8 +139,7 @@ export async function POST(request: Request) { : { classification: "unknown" as const, provider: null }; const auditIpData = toAuditIpData(intelligence); - try { - const decision = await db.transaction(async (tx) => { + const decision = await db.transaction(async (tx) => { await tx.delete(pluginRequests).where(lt(pluginRequests.expiresAt, new Date())); await tx.insert(pluginRequests).values({ requestId: input.requestId, @@ -203,12 +266,31 @@ export async function POST(request: Request) { return { allowed: true as const, message: "Account approved." }; }); - return NextResponse.json(decision); + return NextResponse.json(decision); +} + +export async function POST(request: Request) { + const instance = problemInstance(request); + try { + return await handleVelocityAccess(request); } catch (error) { - console.error("Velocity access decision failed", error); - return NextResponse.json( - { allowed: false, message: denialMessage }, - { status: 503 }, - ); + if (isUniqueConstraintViolation(error, "plugin_requests_pkey")) { + return problemResponse(problemDetails( + "urn:error:replayed-velocity-access-request", + "Replayed Velocity access request", + 409, + "The request ID has already been processed.", + instance, + )); + } + + console.error("Velocity access request failed"); + return problemResponse(problemDetails( + "urn:error:service-unavailable", + "Service unavailable", + 503, + "The access decision could not be completed.", + instance, + )); } } diff --git a/apps/web/src/lib/database-errors.test.ts b/apps/web/src/lib/database-errors.test.ts new file mode 100644 index 0000000..6149c65 --- /dev/null +++ b/apps/web/src/lib/database-errors.test.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { isUniqueConstraintViolation } from "./database-errors"; + +describe("database error classification", () => { + it("recognizes a nested PostgreSQL unique-constraint violation", () => { + expect(isUniqueConstraintViolation({ + cause: { code: "23505", constraint_name: "plugin_requests_pkey" }, + }, "plugin_requests_pkey")).toBe(true); + }); + + it("does not mistake another unique constraint for a replay", () => { + expect(isUniqueConstraintViolation({ + code: "23505", + constraint_name: "minecraft_accounts_active_uuid_uidx", + }, "plugin_requests_pkey")).toBe(false); + }); +}); diff --git a/apps/web/src/lib/database-errors.ts b/apps/web/src/lib/database-errors.ts new file mode 100644 index 0000000..addca1d --- /dev/null +++ b/apps/web/src/lib/database-errors.ts @@ -0,0 +1,23 @@ +export function isUniqueConstraintViolation( + error: unknown, + expectedConstraint: string, +): boolean { + if (!error || typeof error !== "object") return false; + const databaseError = error as { + code?: unknown; + constraint_name?: unknown; + constraint?: unknown; + message?: unknown; + cause?: unknown; + }; + const constraint = databaseError.constraint_name ?? databaseError.constraint; + if ( + databaseError.code === "23505" && + (constraint === expectedConstraint || String(databaseError.message).includes(expectedConstraint)) + ) { + return true; + } + return databaseError.cause + ? isUniqueConstraintViolation(databaseError.cause, expectedConstraint) + : false; +} diff --git a/apps/web/src/lib/problem-response.test.ts b/apps/web/src/lib/problem-response.test.ts new file mode 100644 index 0000000..8a40c32 --- /dev/null +++ b/apps/web/src/lib/problem-response.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from "vitest"; +import { problemInstance, problemResponse } from "./problem-response"; + +describe("problemResponse", () => { + it("uses the request path and query as the problem instance", () => { + expect(problemInstance(new Request("https://example.com/api/items?cursor=next"))) + .toBe("/api/items?cursor=next"); + }); + + it("returns RFC 9457 JSON with the required media type", async () => { + const response = problemResponse({ + type: "urn:error:unauthorized", + title: "Unauthorized", + status: 401, + detail: "A valid Velocity server credential is required.", + instance: "/api/velocity/access", + }); + + expect(response.status).toBe(401); + expect(response.headers.get("content-type")).toBe("application/problem+json"); + expect(await response.json()).toEqual({ + type: "urn:error:unauthorized", + title: "Unauthorized", + status: 401, + detail: "A valid Velocity server credential is required.", + instance: "/api/velocity/access", + }); + }); +}); diff --git a/apps/web/src/lib/problem-response.ts b/apps/web/src/lib/problem-response.ts new file mode 100644 index 0000000..f5e47c8 --- /dev/null +++ b/apps/web/src/lib/problem-response.ts @@ -0,0 +1,16 @@ +import type { ProblemDetails } from "@minecraft-account-manager/contracts"; + +export function problemInstance(request: Request) { + const url = new URL(request.url); + return `${url.pathname}${url.search}`; +} + +export function problemResponse(problem: ProblemDetails) { + return new Response(JSON.stringify(problem), { + status: problem.status, + headers: { + "cache-control": "no-store", + "content-type": "application/problem+json", + }, + }); +} diff --git a/apps/web/vitest.config.mts b/apps/web/vitest.config.mts new file mode 100644 index 0000000..f2d7b49 --- /dev/null +++ b/apps/web/vitest.config.mts @@ -0,0 +1,10 @@ +import { fileURLToPath } from "node:url"; +import { defineConfig } from "vitest/config"; + +export default defineConfig({ + resolve: { + alias: { + "@": fileURLToPath(new URL("./src", import.meta.url)), + }, + }, +}); diff --git a/docs/api-errors.md b/docs/api-errors.md new file mode 100644 index 0000000..7b0930c --- /dev/null +++ b/docs/api-errors.md @@ -0,0 +1,43 @@ +# API error contract + +Application-owned HTTP APIs use Problem Details per RFC 7807 / RFC 9457, matching `game-ingest-server`. + +Every error response has media type `application/problem+json` and the shape: + +```json +{ + "type": "urn:error:invalid-velocity-access-request", + "title": "Invalid Velocity access request", + "status": 400, + "detail": "The request body does not match the required Velocity access contract.", + "instance": "/api/velocity/access", + "extensions": { + "issues": [ + { + "path": "minecraftUuid", + "message": "Expected a compact Java Edition UUID", + "code": "invalid_format" + } + ] + } +} +``` + +`type`, `title`, and `status` are required. `detail`, `instance`, and `extensions` are included when relevant. + +## Problem catalog + +| Type | Status | Meaning | +| --- | ---: | --- | +| `urn:error:invalid-velocity-access-request` | 400 | Request JSON does not satisfy the shared Velocity contract | +| `urn:error:unauthorized` | 401 | Velocity bearer credential is missing, invalid, or revoked | +| `urn:error:expired-velocity-access-request` | 401 | Request timestamp is outside the accepted clock-skew window | +| `urn:error:not-found` | 404 | Unknown application-owned API route | +| `urn:error:method-not-allowed` | 405 | The endpoint does not support the requested HTTP method | +| `urn:error:replayed-velocity-access-request` | 409 | Request ID was already processed | +| `urn:error:unsupported-media-type` | 415 | The request does not use `application/json` | +| `urn:error:service-unavailable` | 503 | A safe access decision could not be completed | + +A whitelist denial remains an HTTP `200` response containing `{ "allowed": false, "message": "..." }`. That response is a successfully evaluated authorization decision, not an HTTP error. + +Browser server actions continue to use redirects and accessible HTML messages. OAuth protocol responses under `/api/auth/*` are owned by NextAuth and follow that protocol's response behavior. diff --git a/package-lock.json b/package-lock.json index cda6953..c33eccc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -54,7 +54,8 @@ "eslint": "^9.39.4", "eslint-config-next": "^16.2.1", "tailwindcss": "^4.2.1", - "typescript": "^5.9.3" + "typescript": "^5.9.3", + "vitest": "^4.1.0" } }, "node_modules/@alloc/quick-lru": { diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index b68c114..eef0ed1 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -45,3 +45,32 @@ export const velocityAccessResponseSchema = z.discriminatedUnion("allowed", [ ]); export type VelocityAccessResponse = z.infer; + +export const problemDetailsSchema = z.object({ + type: z.string().min(1), + title: z.string().min(1), + status: z.number().int().min(100).max(599), + detail: z.string().min(1).optional(), + instance: z.string().min(1).optional(), + extensions: z.record(z.string(), z.unknown()).optional(), +}); + +export type ProblemDetails = z.infer; + +export function problemDetails( + type: string, + title: string, + status: number, + detail?: string, + instance?: string, + extensions?: Record, +): ProblemDetails { + return { + type, + title, + status, + ...(detail ? { detail } : {}), + ...(instance ? { instance } : {}), + ...(extensions ? { extensions } : {}), + }; +} diff --git a/packages/contracts/test/problem-details.test.ts b/packages/contracts/test/problem-details.test.ts new file mode 100644 index 0000000..bad25c0 --- /dev/null +++ b/packages/contracts/test/problem-details.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { problemDetails, problemDetailsSchema } from "../src/index"; + +describe("RFC 9457 Problem Details", () => { + it("builds the same problem shape as game-ingest-server", () => { + const problem = problemDetails( + "urn:error:invalid-velocity-access-request", + "Invalid Velocity access request", + 400, + "The request body does not match the required contract.", + "/api/velocity/access", + { issues: [{ path: "minecraftUuid", message: "Invalid UUID" }] }, + ); + + expect(problemDetailsSchema.parse(problem)).toEqual({ + type: "urn:error:invalid-velocity-access-request", + title: "Invalid Velocity access request", + status: 400, + detail: "The request body does not match the required contract.", + instance: "/api/velocity/access", + extensions: { issues: [{ path: "minecraftUuid", message: "Invalid UUID" }] }, + }); + }); + + it("omits optional members instead of serializing undefined values", () => { + expect(problemDetails("urn:error:internal", "Internal server error", 500)).toEqual({ + type: "urn:error:internal", + title: "Internal server error", + status: 500, + }); + }); + + it("rejects values outside valid HTTP status codes", () => { + expect(() => problemDetailsSchema.parse({ type: "urn:error:test", title: "Test", status: 42 })) + .toThrow(); + }); +});