diff --git a/prisma/schema.prisma b/prisma/schema.prisma index eada22b..6c4b927 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -279,6 +279,11 @@ model MenuScan { model MenuItem { id String @id @default(cuid()) + // Denormalised from MenuScan. Ownership was only ever transitive via scanId, which + // worked because nothing queries MenuItem directly - but left any future direct + // query an IDOR with nothing to stop it. Nullable only for rows created before + // this column existed; every write sets it. + userId String? scanId String name String type DrinkType @@ -297,6 +302,7 @@ model MenuItem { matchedDrink Drink? @relation(fields: [matchedDrinkId], references: [id], onDelete: SetNull) @@index([scanId]) + @@index([userId]) } // ─── Wishlist / Try Later ──────────────────────────────────────── diff --git a/src/app/api/admin/users/[id]/route.ts b/src/app/api/admin/users/[id]/route.ts index f03b942..954a689 100644 --- a/src/app/api/admin/users/[id]/route.ts +++ b/src/app/api/admin/users/[id]/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server" import { z } from "zod" import { requireOwner } from "@/lib/authz" import { prisma } from "@/lib/prisma" -import { deleteImage } from "@/lib/s3" +import { deleteImagesByUrl } from "@/lib/images" import { revokeGatewayKey } from "@/lib/ai/switchboard-keys" const patchSchema = z.object({ @@ -91,23 +91,19 @@ export async function DELETE( // Best effort after the fact: never let storage or the gateway block a deletion // the user asked for, and never leave the account half-deleted. - const keyPrefix = "/minio-images/" - const objectKeys = [...drinks, ...barItems, ...scans] - .map((r) => r.imageUrl) - .filter((u): u is string => !!u && u.startsWith(keyPrefix)) - .map((u) => u.slice(keyPrefix.length)) + const urls = [...drinks, ...barItems, ...scans].map((r) => r.imageUrl) + const gatewayKeyIds = keys + .map((k) => k.gatewayKeyId) + .filter((id): id is string => !!id) - await Promise.allSettled([ - ...objectKeys.map((k) => deleteImage(k)), - ...keys - .map((k) => k.gatewayKeyId) - .filter((id): id is string => !!id) - .map((id) => revokeGatewayKey(id)), + const [imagesDeleted] = await Promise.all([ + deleteImagesByUrl(urls), + Promise.allSettled(gatewayKeyIds.map((id) => revokeGatewayKey(id))), ]) return NextResponse.json({ success: true, - imagesDeleted: objectKeys.length, - gatewayKeysRevoked: keys.filter((k) => k.gatewayKeyId).length, + imagesDeleted, + gatewayKeysRevoked: gatewayKeyIds.length, }) } diff --git a/src/app/api/auth/register/route.ts b/src/app/api/auth/register/route.ts index 3c343c2..10aed23 100644 --- a/src/app/api/auth/register/route.ts +++ b/src/app/api/auth/register/route.ts @@ -26,9 +26,13 @@ const registerSchema = z.object({ export async function POST(request: Request) { try { - // Rate limit: 5 registration attempts per IP per minute + // Rate limit: 5 registration attempts per IP per minute. + // Take the LAST hop, not the first: everything before it is client-supplied and + // can be forged to get a fresh bucket per request. The last entry is the one our + // own reverse proxy appended. const forwarded = request.headers.get("x-forwarded-for") - const ip = forwarded?.split(",")[0]?.trim() ?? "unknown" + const hops = forwarded?.split(",").map((h) => h.trim()).filter(Boolean) ?? [] + const ip = hops.length > 0 ? hops[hops.length - 1] : "unknown" const rl = rateLimit(`register:${ip}`, 5, 60 * 1000) if (!rl.success) { return NextResponse.json( diff --git a/src/app/api/bar/[id]/route.ts b/src/app/api/bar/[id]/route.ts index 1109eed..70d2ed1 100644 --- a/src/app/api/bar/[id]/route.ts +++ b/src/app/api/bar/[id]/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server" import { auth } from "@/lib/auth" import { prisma } from "@/lib/prisma" +import { deleteImagesByUrl } from "@/lib/images" import { barItemUpdateSchema } from "@/lib/validators" export async function PUT( @@ -65,7 +66,7 @@ export async function DELETE( // Check ownership const existing = await prisma.barItem.findUnique({ where: { id: params.id }, - select: { userId: true }, + select: { userId: true, imageUrl: true }, }) if (!existing) { @@ -80,6 +81,9 @@ export async function DELETE( where: { id: params.id }, }) + // After the row is gone, so a storage hiccup cannot block the delete. + void deleteImagesByUrl([existing.imageUrl]) + return NextResponse.json({ success: true }) } catch (error) { console.error("DELETE /api/bar/[id] error:", error) diff --git a/src/app/api/bar/barcode-lookup/route.ts b/src/app/api/bar/barcode-lookup/route.ts index 3ca999e..c337234 100644 --- a/src/app/api/bar/barcode-lookup/route.ts +++ b/src/app/api/bar/barcode-lookup/route.ts @@ -29,7 +29,7 @@ function mapOffCategoryToDrinkType(tags: string[]): string | null { return null } -async function lookupOpenFoodFacts(barcode: string) { +async function lookupOpenFoodFacts(barcode: string, userId: string) { try { const res = await fetch( `https://world.openfoodfacts.org/api/v2/product/${barcode}.json`, @@ -48,8 +48,9 @@ async function lookupOpenFoodFacts(barcode: string) { // item with no image beats one with an image that cannot render. const remoteImage = product.image_url || product.image_front_url || product.image_front_small_url || null + // Namespaced by user so the image route can enforce ownership by key prefix. const imageUrl = remoteImage - ? await mirrorExternalImage(remoteImage, `bar/${barcode}`) + ? await mirrorExternalImage(remoteImage, `${userId}/bar/${barcode}`) : null // Extract ABV from alcohol_100g nutrient or nutriments @@ -153,7 +154,7 @@ export async function POST(request: Request) { } // Try Open Food Facts first - const offResult = await lookupOpenFoodFacts(barcode) + const offResult = await lookupOpenFoodFacts(barcode, session.user.id) if (offResult) { return NextResponse.json({ ...offResult, barcode, source: "openfoodfacts" }) } diff --git a/src/app/api/drinks/[id]/route.ts b/src/app/api/drinks/[id]/route.ts index d4ea4d3..d3e3c21 100644 --- a/src/app/api/drinks/[id]/route.ts +++ b/src/app/api/drinks/[id]/route.ts @@ -1,6 +1,7 @@ import { NextRequest, NextResponse } from "next/server" import { auth } from "@/lib/auth" import { prisma } from "@/lib/prisma" +import { deleteImagesByUrl } from "@/lib/images" import { drinkUpdateSchema } from "@/lib/validators" export async function GET( @@ -113,7 +114,7 @@ export async function DELETE( // Check ownership const existing = await prisma.drink.findUnique({ where: { id: params.id }, - select: { userId: true }, + select: { userId: true, imageUrl: true }, }) if (!existing) { @@ -128,6 +129,9 @@ export async function DELETE( where: { id: params.id }, }) + // After the row is gone, so a storage hiccup cannot block the delete. + void deleteImagesByUrl([existing.imageUrl]) + return NextResponse.json({ success: true }) } catch (error) { console.error("DELETE /api/drinks/[id] error:", error) diff --git a/src/app/api/scan/[id]/route.ts b/src/app/api/scan/[id]/route.ts index 93ea9a1..e1865a7 100644 --- a/src/app/api/scan/[id]/route.ts +++ b/src/app/api/scan/[id]/route.ts @@ -1,6 +1,7 @@ import { NextResponse } from "next/server" import { auth } from "@/lib/auth" import { prisma } from "@/lib/prisma" +import { deleteImagesByUrl } from "@/lib/images" export async function GET( request: Request, @@ -61,5 +62,8 @@ export async function DELETE( await prisma.menuScan.delete({ where: { id: params.id } }) + // After the row is gone, so a storage hiccup cannot block the delete. + void deleteImagesByUrl([scan.imageUrl]) + return NextResponse.json({ success: true }) } diff --git a/src/app/api/scan/route.ts b/src/app/api/scan/route.ts index 34b6efd..5ba37a6 100644 --- a/src/app/api/scan/route.ts +++ b/src/app/api/scan/route.ts @@ -4,6 +4,11 @@ import { prisma } from "@/lib/prisma" import { uploadImage } from "@/lib/s3" import { rateLimit } from "@/lib/rate-limit" import { randomUUID } from "crypto" +import { + reapStuckScans, + tryAcquireScanSlot, + releaseScanSlot, +} from "@/lib/scan-health" export async function GET(request: Request) { const session = await auth() @@ -11,6 +16,10 @@ export async function GET(request: Request) { return NextResponse.json({ error: "Unauthorized" }, { status: 401 }) } + // Lazy housekeeping: a deploy abandons in-flight scans and there is no queue to + // recover them, so mark the stale ones failed before listing. + await reapStuckScans(session.user.id) + const { searchParams } = new URL(request.url) const page = parseInt(searchParams.get("page") || "1") const limit = parseInt(searchParams.get("limit") || "20") @@ -82,10 +91,26 @@ export async function POST(request: Request) { }, }) + if (!tryAcquireScanSlot()) { + // Fail fast rather than piling vision calls onto a busy process. The row is + // already created, so the user sees why instead of an endless spinner. + await prisma.menuScan.update({ + where: { id: scan.id }, + data: { + status: "FAILED", + errorMessage: "Too many scans are being processed right now. Please try again in a moment.", + }, + }) + return NextResponse.json( + { error: "Busy processing other scans. Please try again in a moment." }, + { status: 503 } + ) + } + // Kick off async processing - don't await - processMenuScan(scan.id, buffer, file.type, session.user.id).catch( - (error) => console.error("Scan processing error:", error) - ) + processMenuScan(scan.id, buffer, file.type, session.user.id) + .catch((error) => console.error("Scan processing error:", error)) + .finally(releaseScanSlot) return NextResponse.json(scan, { status: 201 }) } catch (error) { @@ -138,6 +163,7 @@ async function processMenuScan( return { scanId, + userId, name: item.name, type: item.type, subType: item.subType, diff --git a/src/app/api/settings/account/route.ts b/src/app/api/settings/account/route.ts new file mode 100644 index 0000000..54d510e --- /dev/null +++ b/src/app/api/settings/account/route.ts @@ -0,0 +1,62 @@ +import { NextResponse } from "next/server" +import { requireUser } from "@/lib/authz" +import { prisma } from "@/lib/prisma" +import { deleteImagesByUrl } from "@/lib/images" +import { revokeGatewayKey } from "@/lib/ai/switchboard-keys" + +/** + * Self-service account deletion. + * + * Once the app holds other people's drink history - including Rating.location, which + * is a record of where they were - being able to delete it themselves is the minimum, + * rather than having to ask the owner. + * + * The owner cannot delete themselves this way: it would strand the deployment with + * no one able to issue invites or manage members. + */ +export async function DELETE() { + const session = await requireUser() + if (session instanceof NextResponse) return session + + const userId = session.user.id + + const user = await prisma.user.findUnique({ + where: { id: userId }, + select: { role: true }, + }) + if (!user) return NextResponse.json({ error: "Not found" }, { status: 404 }) + if (user.role === "OWNER") { + return NextResponse.json( + { + error: + "The owner account cannot be deleted here. Transfer ownership first.", + }, + { status: 400 } + ) + } + + // Collected before the cascade makes them unreachable. + const [drinks, barItems, scans, keys] = await Promise.all([ + prisma.drink.findMany({ where: { userId }, select: { imageUrl: true } }), + prisma.barItem.findMany({ where: { userId }, select: { imageUrl: true } }), + prisma.menuScan.findMany({ where: { userId }, select: { imageUrl: true } }), + prisma.userApiKey.findMany({ where: { userId }, select: { gatewayKeyId: true } }), + ]) + + await prisma.user.delete({ where: { id: userId } }) + + // Best effort: storage or the gateway being unavailable must not leave the + // account half-deleted. + const gatewayKeyIds = keys + .map((k) => k.gatewayKeyId) + .filter((id): id is string => !!id) + + await Promise.all([ + deleteImagesByUrl([...drinks, ...barItems, ...scans].map((r) => r.imageUrl)), + Promise.allSettled(gatewayKeyIds.map((id) => revokeGatewayKey(id))), + ]) + + // The jwt callback returns null once the user is gone, so the session ends on the + // next request without any extra step here. + return NextResponse.json({ success: true }) +} diff --git a/src/app/minio-images/[...key]/route.ts b/src/app/minio-images/[...key]/route.ts index fe39a50..ff1c3e9 100644 --- a/src/app/minio-images/[...key]/route.ts +++ b/src/app/minio-images/[...key]/route.ts @@ -19,6 +19,18 @@ import { getImage } from "@/lib/s3" */ export const dynamic = "force-dynamic" +/** + * Object keys are namespaced by the user who created them, in two shapes: + * /... uploads, and mirrored product images under /bar/ + * scans//... menu scan photos + * + * Anything else is unreachable by design - if a new writer appears that does not + * namespace its keys, its images 404 rather than becoming readable by everyone. + */ +function canReadKey(key: string, userId: string): boolean { + return key.startsWith(`${userId}/`) || key.startsWith(`scans/${userId}/`) +} + export async function GET( _request: Request, { params }: { params: { key: string[] } } @@ -33,6 +45,11 @@ export async function GET( return new Response(null, { status: 400 }) } + // 404 rather than 403: do not confirm that someone else's key exists. + if (!canReadKey(key, session.user.id)) { + return new Response(null, { status: 404 }) + } + try { const object = await getImage(key) if (!object.Body) return new Response(null, { status: 404 }) diff --git a/src/components/settings/delete-account-card.tsx b/src/components/settings/delete-account-card.tsx new file mode 100644 index 0000000..be63f37 --- /dev/null +++ b/src/components/settings/delete-account-card.tsx @@ -0,0 +1,84 @@ +"use client" + +import { useState } from "react" +import { signOut } from "next-auth/react" +import { Loader2, TriangleAlert } from "lucide-react" +import { Button } from "@/components/ui/button" +import { Input } from "@/components/ui/input" +import { Label } from "@/components/ui/label" +import { + Card, + CardContent, + CardDescription, + CardHeader, + CardTitle, +} from "@/components/ui/card" + +const CONFIRM_WORD = "delete" + +export function DeleteAccountCard() { + const [confirm, setConfirm] = useState("") + const [error, setError] = useState("") + const [loading, setLoading] = useState(false) + + async function handleDelete() { + setError("") + setLoading(true) + try { + const res = await fetch("/api/settings/account", { method: "DELETE" }) + if (!res.ok) { + const body = await res.json().catch(() => ({})) + setError(body.error || "Could not delete the account") + setLoading(false) + return + } + await signOut({ callbackUrl: "/login" }) + } catch { + setError("Something went wrong. Please try again.") + setLoading(false) + } + } + + return ( + + + + + Delete account + + + Permanently removes your account and everything in it - drinks, ratings, + bar, photos and scans. This cannot be undone. Export a backup first if you + want to keep a copy. + + + + {error && ( +
+ {error} +
+ )} +
+ + setConfirm(e.target.value)} + placeholder={CONFIRM_WORD} + disabled={loading} + /> +
+ + + + ) +} diff --git a/src/components/settings/settings-client.tsx b/src/components/settings/settings-client.tsx index 209d2cb..b1ed110 100644 --- a/src/components/settings/settings-client.tsx +++ b/src/components/settings/settings-client.tsx @@ -11,6 +11,7 @@ import { Badge } from "@/components/ui/badge" import { Separator } from "@/components/ui/separator" import { Key, Trash2, Check, Loader2, Shield, Sliders } from "lucide-react" import { BackupRestore } from "@/components/settings/backup-restore" +import { DeleteAccountCard } from "@/components/settings/delete-account-card" interface ApiKeyInfo { id: string @@ -141,6 +142,8 @@ export function SettingsClient({ isOwner }: { isOwner: boolean }) { {/* Backup & Restore Section */} + + {!isOwner && } ) diff --git a/src/lib/backup.ts b/src/lib/backup.ts index 542dfd2..c1c7290 100644 --- a/src/lib/backup.ts +++ b/src/lib/backup.ts @@ -1,5 +1,6 @@ import { objectsToCsv } from "@/lib/csv" import { prisma } from "@/lib/prisma" +import { imageUrlSchema } from "@/lib/validators" import type { Drink, Rating, @@ -466,11 +467,42 @@ type DrinkType = "BEER" | "WINE" | "COCKTAIL" | "SPIRIT" | "OTHER" type BarItemCategory = "SPIRITS" | "LIQUEURS" | "MIXERS" | "BITTERS" | "GARNISHES" | "TOOLS" type BarItemQuantity = "FULL" | "HALF" | "LOW" | "EMPTY" +/** + * Upper bound on what one restore may write. + * + * The restore is correctly scoped to the caller, but nothing bounded its size: a + * crafted file could create an unlimited number of rows in a single transaction + * against shared Postgres. Self-harm with one user; a denial of service once other + * people share the database. + */ +const MAX_RESTORE_ROWS = 20000 + +/** + * imageUrl comes straight from an uploaded CSV and never passed through the schema + * the API enforces, so a crafted file could put an arbitrary string in the column. + */ +function safeImageUrl(value: string | null | undefined): string | null { + if (!value) return null + return imageUrlSchema.safeParse(value).success ? value : null +} + export async function executeRestore( userId: string, data: ParsedBackupData, mode: RestoreMode ): Promise { + const totalRows = + data.drinks.length + + data.ratings.length + + data.wishlistItems.length + + data.sharedLists.length + + data.barItems.length + if (totalRows > MAX_RESTORE_ROWS) { + throw new Error( + `That backup has ${totalRows} rows, more than the ${MAX_RESTORE_ROWS} allowed in one restore.` + ) + } + return await prisma.$transaction( async (tx) => { const summary: RestoreSummary = { @@ -553,7 +585,7 @@ export async function executeRestore( region: drink.region ?? null, abv: drink.abv ?? null, description: drink.description ?? null, - imageUrl: drink.imageUrl ?? null, + imageUrl: safeImageUrl(drink.imageUrl), }, }) drinkIdMap.set(drink._originalId, created.id) @@ -577,7 +609,7 @@ export async function executeRestore( region: drink.region ?? null, abv: drink.abv ?? null, description: drink.description ?? null, - imageUrl: drink.imageUrl ?? null, + imageUrl: safeImageUrl(drink.imageUrl), }, }) summary.drinks.updated++ @@ -595,7 +627,7 @@ export async function executeRestore( region: drink.region ?? null, abv: drink.abv ?? null, description: drink.description ?? null, - imageUrl: drink.imageUrl ?? null, + imageUrl: safeImageUrl(drink.imageUrl), }, }) drinkIdMap.set(drink._originalId, created.id) diff --git a/src/lib/images.ts b/src/lib/images.ts new file mode 100644 index 0000000..cc65998 --- /dev/null +++ b/src/lib/images.ts @@ -0,0 +1,34 @@ +import { deleteImage } from "@/lib/s3" + +const PROXY_PREFIX = "/minio-images/" + +/** Storage key behind one of our image URLs, or null if it is not one of ours. */ +export function imageKeyFromUrl(url: string | null | undefined): string | null { + if (!url || !url.startsWith(PROXY_PREFIX)) return null + const key = url.slice(PROXY_PREFIX.length) + return key.length > 0 && !key.includes("..") ? key : null +} + +/** + * Delete the objects behind these image URLs, ignoring anything that is not ours. + * + * Always best effort: storage being unavailable must never fail a delete the user + * asked for, and must never leave the row behind either. Call after the database + * work, and do not await the result for correctness. + */ +export async function deleteImagesByUrl( + urls: (string | null | undefined)[] +): Promise { + const keys = urls + .map(imageKeyFromUrl) + .filter((k): k is string => k !== null) + + if (keys.length === 0) return 0 + + const results = await Promise.allSettled(keys.map((k) => deleteImage(k))) + const failed = results.filter((r) => r.status === "rejected").length + if (failed > 0) { + console.warn(`[images] ${failed}/${keys.length} object deletions failed`) + } + return keys.length - failed +} diff --git a/src/lib/scan-health.ts b/src/lib/scan-health.ts new file mode 100644 index 0000000..8a5d0ad --- /dev/null +++ b/src/lib/scan-health.ts @@ -0,0 +1,60 @@ +import { prisma } from "@/lib/prisma" + +/** + * Menu scans are processed detached from the request, so a restart - which every + * deploy causes - abandons anything in flight and leaves the row PROCESSING + * forever, with a spinner that never resolves. + * + * There is no job queue to recover them, so instead they are reaped lazily whenever + * a user looks at their scans. Cheap, needs no scheduler, and the worst case is a + * stuck scan showing as failed a little later than it truly failed. + */ +const STUCK_AFTER_MS = 15 * 60 * 1000 + +export async function reapStuckScans(userId: string): Promise { + try { + const { count } = await prisma.menuScan.updateMany({ + where: { + userId, + status: "PROCESSING", + updatedAt: { lt: new Date(Date.now() - STUCK_AFTER_MS) }, + }, + data: { + status: "FAILED", + errorMessage: "Processing was interrupted. Please try scanning again.", + }, + }) + if (count > 0) { + console.warn(`[scan] reaped ${count} stuck scan(s) for ${userId}`) + } + return count + } catch (error) { + // Never let housekeeping break a page load. + console.warn("[scan] reap failed:", error) + return 0 + } +} + +/** + * Caps concurrent menu extractions in this process. + * + * Each one is a vision call with a 4 minute timeout that costs real money, and the + * work is detached from the request so nothing else bounds it. With one user this + * never mattered; with several it is the difference between a queue and a stampede. + */ +const MAX_CONCURRENT_SCANS = 2 +let inFlight = 0 + +export function tryAcquireScanSlot(): boolean { + if (inFlight >= MAX_CONCURRENT_SCANS) return false + inFlight += 1 + return true +} + +export function releaseScanSlot(): void { + inFlight = Math.max(0, inFlight - 1) +} + +export function scansInFlight(): number { + return inFlight +}