From e0c1814cf16b0a825a03373588939d08602f2a9c Mon Sep 17 00:00:00 2001 From: JP Date: Sat, 8 Aug 2026 20:20:39 +0000 Subject: [PATCH] Put images behind the session and make routes private by default The /minio-images rewrite proxied straight onto the MinIO bucket with no authentication, and the bucket carries an anonymous read policy. Verified from the public internet: GET /minio-images?list-type=2 returned a full ListBucketResult naming all 33 objects, and any key then fetched 200. Every menu scan, drink photo and bar photo was enumerable and downloadable by anyone. Replaced with a route handler that requires a session and streams via the existing getImage(). The URL shape is unchanged, so stored imageUrls, uploadImage/getImageUrl and every keep working. Nothing uses next/image and it must stay that way here: the optimizer fetches server-side without the session cookie. Route protection was an allowlist that had to be updated by hand for each new page, and had already been missed for /bar, /bartender and /recommend, which rendered to logged-out visitors. /recipes was in the middleware matcher but not the authorized() list, so it fell through too. Inverted both to a denylist so new pages are private by default. /api stays out of the matcher because authorized() answers with an HTML redirect and API clients need the JSON 401 those routes already return. Also fixes a cross-user write: POST /api/recipes took sourceDrinkId from the client with no ownership check, and the drink detail page loaded its recipes relation unfiltered, so one user could attach an arbitrary recipe to another user's drink where it rendered permanently and the owner could not delete it. Co-Authored-By: Claude Opus 5 (1M context) --- next.config.mjs | 35 +++------------- src/app/(app)/drinks/[id]/page.tsx | 3 ++ src/app/api/recipes/route.ts | 13 ++++++ src/app/minio-images/[...key]/route.ts | 56 ++++++++++++++++++++++++++ src/lib/auth.ts | 28 ++++++++----- src/middleware.ts | 16 ++++---- 6 files changed, 103 insertions(+), 48 deletions(-) create mode 100644 src/app/minio-images/[...key]/route.ts diff --git a/next.config.mjs b/next.config.mjs index 3c59148..897acf9 100644 --- a/next.config.mjs +++ b/next.config.mjs @@ -1,33 +1,10 @@ /** @type {import('next').NextConfig} */ const nextConfig = { output: "standalone", - images: { - remotePatterns: [ - { - protocol: "http", - hostname: "localhost", - port: "9000", - pathname: "/drink-images/**", - }, - { - protocol: "https", - hostname: "*.amazonaws.com", - pathname: "/**", - }, - ], - }, - async rewrites() { - // Proxy image requests to MinIO so URLs work from any device - const minioHost = process.env.MINIO_ENDPOINT || "localhost"; - const minioPort = process.env.MINIO_PORT || "9000"; - const minioBucket = process.env.MINIO_BUCKET || "drink-images"; - return [ - { - source: "/minio-images/:path*", - destination: `http://${minioHost}:${minioPort}/${minioBucket}/:path*`, - }, - ]; - }, + // No `images.remotePatterns` and no rewrite to MinIO: images are served by the + // session-gated route at src/app/minio-images/[...key]/route.ts. The rewrite this + // replaced was an unauthenticated proxy onto the bucket. Nothing uses next/image, + // and it must stay that way - the optimizer fetches without the session cookie. async headers() { return [ { @@ -69,13 +46,11 @@ const nextConfig = { // only ever receive as a URL at sign-in. [ "img-src 'self' data: blob:", - "http://localhost:9000", - "https://*.amazonaws.com", "https://lh3.googleusercontent.com", "https://avatars.githubusercontent.com", ].join(" "), "font-src 'self'", - "connect-src 'self' http://localhost:9000", + "connect-src 'self'", "frame-ancestors 'none'", "base-uri 'self'", "form-action 'self'", diff --git a/src/app/(app)/drinks/[id]/page.tsx b/src/app/(app)/drinks/[id]/page.tsx index 97cf889..7aae1aa 100644 --- a/src/app/(app)/drinks/[id]/page.tsx +++ b/src/app/(app)/drinks/[id]/page.tsx @@ -48,6 +48,9 @@ export default async function DrinkDetailPage({ orderBy: { createdAt: "desc" }, }, recipes: { + // Defence in depth: the POST route now verifies drink ownership, but this + // relation would otherwise render any user's recipe on this drink's page. + where: { userId: session.user.id }, orderBy: { createdAt: "desc" }, }, }, diff --git a/src/app/api/recipes/route.ts b/src/app/api/recipes/route.ts index 22cd8b7..42b4d79 100644 --- a/src/app/api/recipes/route.ts +++ b/src/app/api/recipes/route.ts @@ -72,6 +72,19 @@ export async function POST(request: Request) { ) } + // sourceDrinkId comes from the client and is a foreign key onto Drink. Without + // this check a user could attach a recipe to someone else's drink, where it + // would render on their drink page and they could not delete it. + if (parsed.data.sourceDrinkId) { + const ownsDrink = await prisma.drink.findFirst({ + where: { id: parsed.data.sourceDrinkId, userId: session.user.id }, + select: { id: true }, + }) + if (!ownsDrink) { + return NextResponse.json({ error: "Drink not found" }, { status: 404 }) + } + } + const recipe = await prisma.recipe.create({ data: { userId: session.user.id, diff --git a/src/app/minio-images/[...key]/route.ts b/src/app/minio-images/[...key]/route.ts new file mode 100644 index 0000000..3b8d2b7 --- /dev/null +++ b/src/app/minio-images/[...key]/route.ts @@ -0,0 +1,56 @@ +import { auth } from "@/lib/auth" +import { getImage } from "@/lib/s3" + +/** + * Serves images out of object storage behind the session. + * + * This replaces a Next.js rewrite that proxied straight onto the MinIO bucket with + * no authentication. Combined with the bucket's anonymous read policy that made + * every image in the system enumerable and downloadable from the public internet - + * `GET /minio-images?list-type=2` returned a full ListBucketResult. + * + * The URL shape is deliberately unchanged, so everything that already stores or + * renders `/minio-images/...` keeps working: uploadImage/getImageUrl in lib/s3.ts, + * imageUrlSchema in lib/validators.ts, and every in the app. + * + * Note the app renders images with plain , not next/image. That matters: the + * image optimizer fetches server-side without the session cookie, so switching to + * next/image would 401 every image here. + */ +export const dynamic = "force-dynamic" + +export async function GET( + _request: Request, + { params }: { params: { key: string[] } } +) { + const session = await auth() + if (!session?.user?.id) { + return new Response(null, { status: 401 }) + } + + const key = params.key.join("/") + if (key.includes("..")) { + return new Response(null, { status: 400 }) + } + + try { + const object = await getImage(key) + if (!object.Body) return new Response(null, { status: 404 }) + + return new Response(object.Body as unknown as ReadableStream, { + headers: { + "Content-Type": object.ContentType ?? "application/octet-stream", + ...(object.ContentLength + ? { "Content-Length": String(object.ContentLength) } + : {}), + // Keys embed a random UUID, so an object never changes under a given key. + // `private` keeps it out of any shared cache now that it is session-gated. + "Cache-Control": "private, max-age=31536000, immutable", + }, + }) + } catch { + // Missing key, or storage unavailable - do not distinguish, to avoid + // confirming whether a given key exists. + return new Response(null, { status: 404 }) + } +} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 42a8d31..59aee2b 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -6,6 +6,13 @@ import { PrismaAdapter } from "@auth/prisma-adapter" import { prisma } from "@/lib/prisma" import { rateLimit } from "@/lib/rate-limit" +/** + * The only routes reachable without a session. Matched exactly or as a path + * prefix, so `/share` also covers `/share/`. Everything else is private - + * see the `authorized` callback and the denylist matcher in src/middleware.ts. + */ +const PUBLIC_ROUTES = ["/login", "/register", "/share"] + const providers = [ Google({ clientId: process.env.GOOGLE_CLIENT_ID, @@ -66,19 +73,18 @@ export const { handlers, signIn, signOut, auth } = NextAuth({ }, authorized({ auth, request: { nextUrl } }) { const isLoggedIn = !!auth?.user - const isOnApp = nextUrl.pathname.startsWith("/dashboard") || - nextUrl.pathname.startsWith("/scan") || - nextUrl.pathname.startsWith("/drinks") || - nextUrl.pathname.startsWith("/rate") || - nextUrl.pathname.startsWith("/settings") || - nextUrl.pathname.startsWith("/wishlist") + const { pathname } = nextUrl - if (isOnApp) { - if (isLoggedIn) return true - return false - } + // Everything is private unless named here. Paired with the denylist matcher + // in middleware.ts, so a new page is protected by default rather than public + // until somebody remembers to add it. + const isPublic = PUBLIC_ROUTES.some( + (p) => pathname === p || pathname.startsWith(`${p}/`) + ) - if (isLoggedIn && (nextUrl.pathname === "/login" || nextUrl.pathname === "/register")) { + if (!isPublic && !isLoggedIn) return false + + if (isLoggedIn && (pathname === "/login" || pathname === "/register")) { return Response.redirect(new URL("/dashboard", nextUrl)) } diff --git a/src/middleware.ts b/src/middleware.ts index 51db14c..a0345bc 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -1,13 +1,15 @@ export { auth as middleware } from "@/lib/auth" export const config = { + // A denylist, not an allowlist. The previous per-route allowlist meant every new + // page was public until someone remembered to add it here - /bar, /bartender and + // /recommend all shipped unprotected that way. Everything is matched now except + // static assets, and `authorized` in lib/auth.ts names the few public routes. + // + // /api is deliberately excluded: `authorized` answers with an HTML redirect to + // /login, whereas API clients need a JSON 401 - which every route already returns + // from its own auth() check. matcher: [ - "/dashboard/:path*", - "/scan/:path*", - "/drinks/:path*", - "/rate/:path*", - "/settings/:path*", - "/wishlist/:path*", - "/recipes/:path*", + "/((?!api|_next/static|_next/image|favicon.ico|manifest.json).*)", ], }