diff --git a/.env.local.example b/.env.local.example index 4640a81..255043a 100644 --- a/.env.local.example +++ b/.env.local.example @@ -22,9 +22,11 @@ CACHE_TTL=300 # Rate limit for image proxy (integer >= 1, requests per minute per IP, default: 120) # RATE_LIMIT_RPM=120 -# Trusted Proxies: Comma-separated list of IP addresses (e.g., from Nginx, Traefik, or Vercel). -# If set, rate limiting will only trust X-Forwarded-For headers from these IPs. -# TRUSTED_PROXIES=127.0.0.1,10.0.0.1 +# Number of reverse proxies in front of the app (nginx/Traefik/Caddy = 1, +# Cloudflare in front of nginx = 2). Required for correct rate limiting behind +# a proxy — without it the client IP is read from a spoofable header. +# The app must be reachable ONLY through the proxy (bind to 127.0.0.1). +# TRUSTED_PROXY_HOPS=1 # Secret for signing auth cookies and encrypting asset tokens (string, min 16 chars). # If not provided, IMMICH_API_KEY will be used as a fallback (not recommended). diff --git a/CLAUDE.md b/CLAUDE.md index ad3922c..7133589 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,7 +92,7 @@ Per-subpage and per-album password gating using HMAC tokens in `HttpOnly` cookie ### Rate limiting (`lib/rate-limit.ts`) -In-memory sliding-window rate limiter. Important: **in-memory only** — does not work across multiple Node.js instances. Uses FIFO eviction (not reject-on-full) to prevent DoS via store flooding. Configurable via `RATE_LIMIT_RPM`. Trusted proxy IPs must be set via `TRUSTED_PROXIES` env var to safely trust `X-Forwarded-For` headers. +In-memory sliding-window rate limiter. Important: **in-memory only** — does not work across multiple Node.js instances. Uses FIFO eviction (not reject-on-full) to prevent DoS via store flooding. Configurable via `RATE_LIMIT_RPM`. `TRUSTED_PROXY_HOPS` must be set to the number of reverse proxies in front of the app (nginx = 1); the client IP is then read that many entries from the right of `X-Forwarded-For`, which proxies append to. Without it the IP comes from a spoofable header. Bucket keys are namespaced per endpoint (`image:`, `map:`, `auth:`, …) so limits don't collide. ### Theming system diff --git a/README.md b/README.md index a803575..0086b5b 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,11 @@ Immich Folio acts as a **secure reverse proxy** between your visitors and your p +## Requirements + +- **Immich 3.0 or newer.** Immich 3.0 changed how album assets are retrieved; earlier versions are not supported as of v0.9.0. On an older server, albums render with the correct title but no photos, and the map stays empty — the app logs a warning naming this as the likely cause. +- Node.js 20+ (or just use the Docker image) + ## Quick Start ```bash @@ -91,19 +96,23 @@ IMMICH_API_KEY=your-api-key SITE_TITLE=My Photography # default: "Gallery" SITE_SUBTITLE=A visual journal # default: empty CACHE_TTL=300 # seconds, default: 300 -RATE_LIMIT_RPM=120 # requests/min/IP, default: 120 +RATE_LIMIT_RPM=1500 # requests/min/IP, default: 1500 +AUTH_SECRET=long-random-string # required in production +TRUSTED_PROXY_HOPS=1 # reverse proxies in front, default: 0 ADMIN_PASSWORD=your-secure-password # enables /admin panel ``` +> Behind a reverse proxy, set `TRUSTED_PROXY_HOPS` to the number of proxies in front of the app (nginx/Traefik/Caddy = 1; Cloudflare in front of nginx = 2). Without it the client IP is read from a header the client itself can set, which defeats the brute-force limits on the password endpoints. See [Trusted Proxies](docs/gallery-config.md#trusted-proxies). + ### Immich API Key Permissions Create a dedicated API key in Immich under **Account Settings → API Keys**. Immich Folio only needs **read access** — it never modifies your library. -| Permission | Required | Used for | -|---|---|---| -| `album.read` | ✅ Yes | List and fetch album metadata & photo lists | -| `asset.read` | ✅ Yes | Fetch asset metadata, EXIF data, thumbnails, previews, and originals | -| `asset.view` | ✅ Yes | Stream image/video files (thumbnail, preview, video playback) | +| Permission | Required | Used for | +| ------------ | -------- | -------------------------------------------------------------------- | +| `album.read` | ✅ Yes | List and fetch album metadata & photo lists | +| `asset.read` | ✅ Yes | Fetch asset metadata, EXIF data, thumbnails, previews, and originals | +| `asset.view` | ✅ Yes | Stream image/video files (thumbnail, preview, video playback) | > **No write permissions needed.** `album.create`, `asset.upload`, `asset.delete`, etc. can all be left **off**. diff --git a/app/[...path]/page.tsx b/app/[...path]/page.tsx index c422df1..1792a61 100644 --- a/app/[...path]/page.tsx +++ b/app/[...path]/page.tsx @@ -25,6 +25,7 @@ import { assetExifSummary, assetAspectRatio, } from '@/lib/urls'; +import { encodeAssetId } from '@/lib/tokens'; import { getConfig, type GridConfig } from '@/lib/config'; import { isProtected, isAuthenticated } from '@/lib/auth'; import PasswordGate from '@/components/PasswordGate'; @@ -96,7 +97,9 @@ function toPhotoItems(assets: ImmichAsset[], showExif: boolean): PhotoItem[] { const exif = showExif && a.type === 'IMAGE' ? assetExifSummary(a) : undefined; const isVideo = a.type === 'VIDEO'; return { - id: a.id, + // Opaque token, never the raw Immich UUID — this object is serialized + // into the RSC payload and shipped to the browser. Used only as a React key. + id: encodeAssetId(a.id), type: isVideo ? 'video' : 'image', thumbUrl: imageUrl(a.id, 'preview'), previewUrl: imageUrl(a.id, 'preview'), diff --git a/app/admin/layout.tsx b/app/admin/layout.tsx index 8013802..828941e 100644 --- a/app/admin/layout.tsx +++ b/app/admin/layout.tsx @@ -5,6 +5,16 @@ export const metadata: Metadata = { robots: { index: false, follow: false }, }; +/** + * Required for the CSP nonce. The admin page is a pure client component, so + * Next.js would statically prerender it at build time — and a prerendered HTML + * file cannot carry the per-request nonce that middleware.ts issues. Since the + * CSP uses 'strict-dynamic' (which makes 'self' inert), unnonced script tags + * are blocked and the panel never hydrates. Rendering per request lets Next.js + * stamp the nonce onto the script tags it emits. + */ +export const dynamic = 'force-dynamic'; + export default function AdminLayout({ children }: { children: React.ReactNode }) { return
{children}
; } diff --git a/app/api/admin/albums/[albumId]/assets/route.ts b/app/api/admin/albums/[albumId]/assets/route.ts index aa75913..1d2310f 100644 --- a/app/api/admin/albums/[albumId]/assets/route.ts +++ b/app/api/admin/albums/[albumId]/assets/route.ts @@ -5,6 +5,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { getConfig } from '@/lib/config'; +import { isUuid } from '@/lib/uuid'; interface ImmichAlbumDetail { assets: Array<{ @@ -35,8 +36,14 @@ export async function GET( const { albumId } = await params; + // Without this, `..%2f..%2fusers` resolves inside fetch() to an arbitrary + // Immich endpoint, called with the server's API key. + if (!isUuid(albumId)) { + return NextResponse.json({ error: 'Invalid album ID' }, { status: 400 }); + } + try { - const res = await fetch(`${config.immich.apiUrl}/albums/${albumId}`, { + const res = await fetch(`${config.immich.apiUrl}/albums/${encodeURIComponent(albumId)}`, { headers: { 'x-api-key': config.immich.apiKey, Accept: 'application/json', diff --git a/app/api/admin/auth/route.ts b/app/api/admin/auth/route.ts index cfea489..aa97e44 100644 --- a/app/api/admin/auth/route.ts +++ b/app/api/admin/auth/route.ts @@ -7,9 +7,25 @@ import { COOKIE_NAME, SESSION_DURATION_MS, } from '@/lib/admin/auth'; +import { checkRateLimit, getClientIp } from '@/lib/rate-limit'; + +/** Admin login attempts per minute per IP — the highest-value credential in the app. */ +const ADMIN_AUTH_RPM = 5; /** POST: Login with admin password. */ export async function POST(request: NextRequest) { + const ip = getClientIp(request); + const rl = checkRateLimit(`admin-auth:${ip}`, ADMIN_AUTH_RPM); + if (!rl.success) { + return NextResponse.json( + { error: 'Too many requests' }, + { + status: 429, + headers: { 'Retry-After': String(Math.ceil((rl.resetAt - Date.now()) / 1000)) }, + }, + ); + } + if (!isAdminEnabled()) { return NextResponse.json( { error: 'Admin panel is not enabled. Set ADMIN_PASSWORD in your environment.' }, diff --git a/app/api/admin/thumbnail/[id]/route.ts b/app/api/admin/thumbnail/[id]/route.ts index 0d1842e..0a46b28 100644 --- a/app/api/admin/thumbnail/[id]/route.ts +++ b/app/api/admin/thumbnail/[id]/route.ts @@ -6,11 +6,9 @@ import { NextRequest, NextResponse } from 'next/server'; import { isAdminAuthenticated, isAdminEnabled } from '@/lib/admin/auth'; import { getConfig } from '@/lib/config'; +import { isUuid } from '@/lib/uuid'; -export async function GET( - request: NextRequest, - { params }: { params: Promise<{ id: string }> }, -) { +export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { if (!isAdminEnabled()) { return NextResponse.json({ error: 'Admin not enabled' }, { status: 403 }); } @@ -25,22 +23,17 @@ export async function GET( const { id: assetId } = await params; - // Validate it looks like a UUID - const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - if (!uuidRegex.test(assetId)) { + if (!isUuid(assetId)) { return NextResponse.json({ error: 'Invalid asset ID' }, { status: 400 }); } try { - const res = await fetch( - `${config.immich.apiUrl}/assets/${assetId}/thumbnail?size=thumbnail`, - { - headers: { - 'x-api-key': config.immich.apiKey, - Accept: 'application/octet-stream', - }, + const res = await fetch(`${config.immich.apiUrl}/assets/${assetId}/thumbnail?size=thumbnail`, { + headers: { + 'x-api-key': config.immich.apiKey, + Accept: 'application/octet-stream', }, - ); + }); if (!res.ok) { return NextResponse.json({ error: 'Asset not found' }, { status: 404 }); diff --git a/app/api/image/[id]/route.ts b/app/api/image/[id]/route.ts index 8a8a4d0..f28c4ff 100644 --- a/app/api/image/[id]/route.ts +++ b/app/api/image/[id]/route.ts @@ -27,7 +27,7 @@ function widthToSize(w: number): ImageSize { export async function GET(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { // ── Rate limiting ────────────────────────────────── const ip = getClientIp(request); - const { success, remaining, resetAt } = checkRateLimit(ip, getConfig().rateLimitRpm); + const { success, remaining, resetAt } = checkRateLimit(`image:${ip}`, getConfig().rateLimitRpm); if (!success) { const retryAfter = Math.ceil((resetAt - Date.now()) / 1000); diff --git a/app/api/map/route.ts b/app/api/map/route.ts index 6ba75ba..00d244c 100644 --- a/app/api/map/route.ts +++ b/app/api/map/route.ts @@ -14,10 +14,12 @@ import { checkRateLimit, getClientIp } from '@/lib/rate-limit'; export const dynamic = 'force-dynamic'; +/** Map data is cached client-side, so a low limit is plenty. */ +const MAP_RPM = 120; + export async function GET(request: NextRequest) { - // Rate limit: 120 requests/minute per IP (map data is cached client-side; this guards against abuse) const ip = getClientIp(request); - const rl = checkRateLimit(ip, 120); + const rl = checkRateLimit(`map:${ip}`, MAP_RPM); if (!rl.success) { return NextResponse.json( { error: 'Too many requests' }, @@ -43,28 +45,50 @@ export async function GET(request: NextRequest) { const locations = await getMapData(); - // Filter locations and albums based on auth - const authCache = new Map(); + // Filter locations and albums based on auth. + // An album is visible only if BOTH its subpage gate (if any) and its own + // album-level password gate are satisfied. Standalone albums are NOT + // implicitly public — they can carry their own password. + const subpageAuthCache = new Map(); + const albumAuthCache = new Map(); + + const isSubpageAllowed = (slug: string): boolean => { + const cached = subpageAuthCache.get(slug); + if (cached !== undefined) return cached; + const result = isAuthenticated(slug, getCookie); + subpageAuthCache.set(slug, result); + return result; + }; + + const isAlbumAllowed = (albumId: string): boolean => { + const cached = albumAuthCache.get(albumId); + if (cached !== undefined) return cached; + const result = isAuthenticated(albumId, getCookie, 'album'); + albumAuthCache.set(albumId, result); + return result; + }; + const publicLocations = locations .map((loc) => { - // Only keep albums the user is allowed to see - const allowedAlbums = loc.albums.filter((a) => { - if (!a.subpageSlug) return true; // Standalone albums are public - if (authCache.has(a.subpageSlug)) return authCache.get(a.subpageSlug)!; - const result = isAuthenticated(a.subpageSlug, getCookie); - authCache.set(a.subpageSlug, result); - return result; - }); + const allowedAlbums = loc.albums.filter( + (a) => (!a.subpageSlug || isSubpageAllowed(a.subpageSlug)) && isAlbumAllowed(a.id), + ); if (allowedAlbums.length === 0) return null; + // Aggregate over visible albums only, so neither the marker position, + // the photo count, nor the cover asset reveals a protected album. + const photoCount = allowedAlbums.reduce((sum, a) => sum + a.photoCount, 0); + const latSum = allowedAlbums.reduce((sum, a) => sum + a.latSum, 0); + const lngSum = allowedAlbums.reduce((sum, a) => sum + a.lngSum, 0); + return { city: loc.city, country: loc.country, - lat: loc.lat, - lng: loc.lng, - photoCount: loc.photoCount, // Note: This count might be slightly off if some photos are in hidden albums - coverUrl: imageUrl(loc.coverAssetId, 'thumbnail'), + lat: latSum / photoCount, + lng: lngSum / photoCount, + photoCount, + coverUrl: imageUrl(allowedAlbums[0].coverAssetId, 'thumbnail'), albums: allowedAlbums.map((a) => ({ name: a.name, url: a.subpageSlug ? `/${a.subpageSlug}/${a.slug}` : `/${a.slug}`, diff --git a/app/api/og/route.tsx b/app/api/og/route.tsx index 315b8ea..7d786a5 100644 --- a/app/api/og/route.tsx +++ b/app/api/og/route.tsx @@ -11,11 +11,19 @@ import { NextRequest, NextResponse } from 'next/server'; import { getConfig } from '@/lib/config'; import { checkRateLimit, getClientIp } from '@/lib/rate-limit'; +/** + * OG image rendering runs satori + resvg per request and each unique ?title + * misses every cache tier, so this is far more expensive than an image proxy + * hit. It must not inherit RATE_LIMIT_RPM (default 1500) — that turns the + * endpoint into an unauthenticated CPU amplifier. + */ +const OG_RPM = 30; + export async function GET(request: NextRequest) { const ip = getClientIp(request); - const { theme, rateLimitRpm } = getConfig(); + const { theme } = getConfig(); - const { success } = checkRateLimit(`og:${ip}`, rateLimitRpm); + const { success } = checkRateLimit(`og:${ip}`, OG_RPM); if (!success) { return new NextResponse('Too many requests', { status: 429 }); } diff --git a/docs/gallery-config.md b/docs/gallery-config.md index 73cb7e1..a303bde 100644 --- a/docs/gallery-config.md +++ b/docs/gallery-config.md @@ -208,14 +208,37 @@ To protect against brute-force attacks and resource exhaustion, Immich Folio inc - `RATE_LIMIT_RPM`: Maximum requests per minute per IP (default: `1500` for general API, `120` for map data). ### Trusted Proxies - -If you are running Immich Folio behind a reverse proxy (like Nginx, Traefik, Caddy, or Cloudflare), you should configure trusted proxies to prevent IP spoofing. - -- `TRUSTED_PROXIES`: A comma-separated list of IP addresses of your proxies. - -Example: + +If you run Immich Folio behind a reverse proxy (nginx, Traefik, Caddy, Cloudflare), tell it how many proxies sit in front. Without this, the rate limiter reads a client-supplied header and an attacker can pick their own bucket — defeating brute-force protection on the password endpoints. + +- `TRUSTED_PROXY_HOPS`: Number of reverse proxies in front of the app (default: `0`). + ```bash -TRUSTED_PROXIES=127.0.0.1,172.18.0.1 +# nginx / Traefik / Caddy directly in front of the app +TRUSTED_PROXY_HOPS=1 + +# Cloudflare in front of nginx +TRUSTED_PROXY_HOPS=2 ``` - -When set, the rate limiter will only trust `X-Forwarded-For` and `X-Real-IP` headers if the request directly comes from one of these IPs. If not set, headers are trusted as a best-effort, which is less secure in public-facing environments. + +The client IP is taken that many entries from the **right** of `X-Forwarded-For`. Proxies append the address they actually observed, so everything to the left is client-supplied and ignored — a visitor sending `X-Forwarded-For: 1.1.1.1` cannot influence which bucket they land in. + +> ⚠️ **The app must be reachable only through the proxy.** Bind it to localhost (`127.0.0.1:3000`) or keep it on an internal Docker network. If clients can connect directly they control the entire header, and no setting can recover the real IP. + +Matching nginx config: + +```nginx +location / { + proxy_pass http://127.0.0.1:3000; + proxy_set_header Host $host; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; +} +``` + +`$proxy_add_x_forwarded_for` is the important one: it *appends* the real peer address rather than overwriting the header. + +#### Migrating from `TRUSTED_PROXIES` + +`TRUSTED_PROXIES` has been removed. It matched proxy IPs against the socket peer address, which a self-hosted Next.js server never exposes — so the check never passed. Setting it silently put **every visitor into one shared rate-limit bucket**, letting a single client trip the limit for the entire site. If it is still set, the app assumes `TRUSTED_PROXY_HOPS=1` and logs a warning; replace it with an explicit value. diff --git a/lib/__tests__/admin-auth.test.ts b/lib/__tests__/admin-auth.test.ts new file mode 100644 index 0000000..c49e126 --- /dev/null +++ b/lib/__tests__/admin-auth.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import crypto from 'crypto'; + +// next/headers is only used by isAdminAuthenticated(), which we do not exercise here. +vi.mock('next/headers', () => ({ cookies: async () => ({ get: () => undefined }) })); + +const ADMIN_PASSWORD = 'correct-horse-battery-staple'; +const AUTH_SECRET = 'test-auth-secret-32-chars-long-min'; + +vi.mock('../env', () => ({ + env: { + get AUTH_SECRET() { + return process.env.__TEST_AUTH_SECRET; + }, + get ADMIN_PASSWORD() { + return process.env.__TEST_ADMIN_PASSWORD; + }, + }, +})); + +/** Import fresh so module-level state cannot leak between cases. */ +async function loadAuth() { + vi.resetModules(); + return import('../admin/auth'); +} + +function forgeToken(signingKey: Buffer, payload: object): string { + const data = Buffer.from(JSON.stringify(payload)).toString('base64url'); + const sig = crypto.createHmac('sha256', signingKey).update(data).digest('base64url'); + return `${data}.${sig}`; +} + +beforeEach(() => { + process.env.__TEST_AUTH_SECRET = AUTH_SECRET; + process.env.__TEST_ADMIN_PASSWORD = ADMIN_PASSWORD; +}); + +afterEach(() => { + delete process.env.__TEST_AUTH_SECRET; + delete process.env.__TEST_ADMIN_PASSWORD; + vi.unstubAllEnvs(); +}); + +describe('admin session tokens', () => { + it('accepts a token it just issued', async () => { + const { createAdminToken, verifyAdminToken } = await loadAuth(); + expect(verifyAdminToken(createAdminToken())).toBe(true); + }); + + it('rejects a token forged with the old hardcoded dev fallback secret', async () => { + const { verifyAdminToken } = await loadAuth(); + // The exact key an attacker could derive from the public source before the fix. + const legacyKey = crypto.createHash('sha256').update('admin:dev-fallback-secret').digest(); + const forged = forgeToken(legacyKey, { + role: 'admin', + iat: Date.now(), + exp: Date.now() + 60_000, + }); + expect(verifyAdminToken(forged)).toBe(false); + }); + + it('does not fall back to a guessable secret when AUTH_SECRET is unset in dev', async () => { + delete process.env.__TEST_AUTH_SECRET; + const { verifyAdminToken } = await loadAuth(); + const legacyKey = crypto.createHash('sha256').update('admin:dev-fallback-secret').digest(); + const forged = forgeToken(legacyKey, { + role: 'admin', + iat: Date.now(), + exp: Date.now() + 60_000, + }); + expect(verifyAdminToken(forged)).toBe(false); + }); + + it('throws rather than signing with a guessable secret when AUTH_SECRET is unset in production', async () => { + delete process.env.__TEST_AUTH_SECRET; + vi.stubEnv('NODE_ENV', 'production'); + const { createAdminToken } = await loadAuth(); + expect(() => createAdminToken()).toThrow(/AUTH_SECRET/); + }); + + it('returns false (not throw) for a malformed cookie with a short signature', async () => { + const { verifyAdminToken } = await loadAuth(); + // Pre-fix this hit crypto.timingSafeEqual outside the try block -> RangeError -> HTTP 500. + expect(() => verifyAdminToken('a.b')).not.toThrow(); + expect(verifyAdminToken('a.b')).toBe(false); + expect(verifyAdminToken('not-a-token')).toBe(false); + expect(verifyAdminToken('')).toBe(false); + }); + + it('rejects an expired token', async () => { + const { verifyAdminToken } = await loadAuth(); + const key = crypto + .createHash('sha256') + .update(`admin:${AUTH_SECRET}:${ADMIN_PASSWORD}`) + .digest(); + const expired = forgeToken(key, { role: 'admin', iat: 0, exp: Date.now() - 1000 }); + expect(verifyAdminToken(expired)).toBe(false); + }); + + it('rejects a validly signed token whose role is not admin', async () => { + const { verifyAdminToken } = await loadAuth(); + const key = crypto + .createHash('sha256') + .update(`admin:${AUTH_SECRET}:${ADMIN_PASSWORD}`) + .digest(); + const wrongRole = forgeToken(key, { role: 'viewer', iat: 0, exp: Date.now() + 60_000 }); + expect(verifyAdminToken(wrongRole)).toBe(false); + }); + + it('invalidates existing sessions when ADMIN_PASSWORD is rotated', async () => { + const first = await loadAuth(); + const token = first.createAdminToken(); + expect(first.verifyAdminToken(token)).toBe(true); + + process.env.__TEST_ADMIN_PASSWORD = 'a-brand-new-password'; + const second = await loadAuth(); + expect(second.verifyAdminToken(token)).toBe(false); + }); +}); + +describe('verifyAdminPassword', () => { + it('accepts the configured password and rejects others', async () => { + const { verifyAdminPassword } = await loadAuth(); + expect(verifyAdminPassword(ADMIN_PASSWORD)).toBe(true); + expect(verifyAdminPassword('wrong')).toBe(false); + expect(verifyAdminPassword(`${ADMIN_PASSWORD}x`)).toBe(false); + expect(verifyAdminPassword('')).toBe(false); + }); + + it('rejects everything when no ADMIN_PASSWORD is configured', async () => { + delete process.env.__TEST_ADMIN_PASSWORD; + const { verifyAdminPassword, isAdminEnabled } = await loadAuth(); + expect(isAdminEnabled()).toBe(false); + expect(verifyAdminPassword('')).toBe(false); + expect(verifyAdminPassword('anything')).toBe(false); + }); +}); diff --git a/lib/__tests__/auth.test.ts b/lib/__tests__/auth.test.ts index 839ef64..97963cd 100644 --- a/lib/__tests__/auth.test.ts +++ b/lib/__tests__/auth.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; // Mock config to provide a predictable API key and subpage config vi.mock('@/lib/config', () => ({ @@ -93,3 +93,60 @@ describe('isAuthenticated', () => { expect(isAuthenticated('public', getCookie)).toBe(true); }); }); + +describe('token expiry', () => { + function tokenFor(slug: string, password: string): string { + return authenticate(slug, password)!.split('=')[1].split(';')[0]; + } + + afterEach(() => { + vi.useRealTimers(); + }); + + it('rejects a token once its embedded expiry has passed', () => { + const token = tokenFor('private', 'secret123'); + const getCookie = (name: string) => (name === 'lb_auth_private' ? token : undefined); + expect(isAuthenticated('private', getCookie)).toBe(true); + + // Previously the 24h lifetime lived only in the cookie's Max-Age, so a + // captured token replayed via curl worked forever. + vi.useFakeTimers(); + vi.setSystemTime(Date.now() + 25 * 60 * 60 * 1000); + expect(isAuthenticated('private', getCookie)).toBe(false); + }); + + it('still accepts a token just before it expires', () => { + const token = tokenFor('private', 'secret123'); + const getCookie = (name: string) => (name === 'lb_auth_private' ? token : undefined); + + vi.useFakeTimers(); + vi.setSystemTime(Date.now() + 23 * 60 * 60 * 1000); + expect(isAuthenticated('private', getCookie)).toBe(true); + }); + + it('rejects a token whose expiry was tampered with', () => { + const token = tokenFor('private', 'secret123'); + const [, sig] = token.split('.'); + const farFuture = Date.now() + 365 * 24 * 60 * 60 * 1000; + + // The expiry is covered by the HMAC, so extending it invalidates the token. + const getCookie = (name: string) => + name === 'lb_auth_private' ? `${farFuture}.${sig}` : undefined; + expect(isAuthenticated('private', getCookie)).toBe(false); + }); + + it('rejects malformed and legacy tokens without throwing', () => { + for (const value of ['', '.', 'abc', 'abc.def', `${Date.now() + 1000}.`, 'deadbeef']) { + const getCookie = (name: string) => (name === 'lb_auth_private' ? value : undefined); + expect(() => isAuthenticated('private', getCookie)).not.toThrow(); + expect(isAuthenticated('private', getCookie)).toBe(false); + } + }); + + it('sets a cookie Max-Age consistent with the embedded expiry', () => { + const cookie = authenticate('private', 'secret123')!; + const maxAge = Number(cookie.match(/Max-Age=(\d+)/)![1]); + const exp = Number(cookie.split('=')[1].split(';')[0].split('.')[0]); + expect(Math.abs(exp - (Date.now() + maxAge * 1000))).toBeLessThan(2000); + }); +}); diff --git a/lib/__tests__/immich.test.ts b/lib/__tests__/immich.test.ts index 97a0a56..a59be5e 100644 --- a/lib/__tests__/immich.test.ts +++ b/lib/__tests__/immich.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { immich } from '../immich'; import * as config from '../config'; +import { cache } from '../cache'; // Mock the config by wrapping it in a factory vi.mock('../config', async () => { @@ -28,6 +29,9 @@ describe('ImmichClient', () => { beforeEach(() => { vi.clearAllMocks(); + // getAlbum() memoises into the module-level LRU; without this, later tests + // read a previous test's album and never reach the mocked fetch. + cache.clear(); }); describe('request()', () => { @@ -80,23 +84,105 @@ describe('ImmichClient', () => { expect(album).toBeNull(); }); - it('returns album data and filters trashed assets', async () => { - mockFetch.mockResolvedValueOnce({ + /** + * Immich 3.x returns an empty `assets` array from GET /albums/:id, so + * assets come from a separate POST /search/metadata call. Route the mock by + * URL rather than by call order — the two run concurrently. + */ + function mockAlbumApi(options: { + order?: 'asc' | 'desc'; + pages?: Array<{ items: unknown[]; nextPage: string | null }>; + }) { + const pages = options.pages ?? []; + const json = (body: unknown) => ({ ok: true, headers: { get: () => 'application/json' }, - json: async () => ({ + json: async () => body, + }); + + mockFetch.mockImplementation(async (url: string, init?: { body?: string }) => { + if (url.includes('/search/metadata')) { + const page = JSON.parse(init?.body ?? '{}').page ?? 1; + const p = pages[page - 1] ?? { items: [], nextPage: null }; + return json({ assets: { items: p.items, nextPage: p.nextPage, total: p.items.length } }); + } + // Mirrors the real 3.x response: assetCount set, assets empty. + return json({ id: 'album-1', albumName: 'Name', - assets: [ - { id: 'asset-1', isTrashed: false }, - { id: 'asset-2', isTrashed: true }, - ], - }), + assetCount: 2, + assets: [], + order: options.order ?? 'desc', + }); + }); + } + + const asset = (id: string, date: string, isTrashed = false) => ({ + id, + isTrashed, + fileCreatedAt: date, + }); + + it('loads assets via metadata search and filters trashed ones', async () => { + mockAlbumApi({ + pages: [ + { + items: [ + asset('asset-1', '2024-01-02T00:00:00Z'), + asset('asset-2', '2024-01-03T00:00:00Z', true), + ], + nextPage: null, + }, + ], }); const album = await immich.getAlbum('album-1'); expect(album?.assets).toHaveLength(1); expect(album?.assets[0].id).toBe('asset-1'); }); + + it('requests EXIF data — without it the grid, lightbox and map go blank', async () => { + mockAlbumApi({ pages: [{ items: [], nextPage: null }] }); + await immich.getAlbum('album-1'); + + const search = mockFetch.mock.calls.find((c: unknown[]) => + String(c[0]).includes('/search/metadata'), + ); + expect(search).toBeDefined(); + const body = JSON.parse((search[1] as { body: string }).body); + expect(body.withExif).toBe(true); + expect(body.albumIds).toEqual(['album-1']); + expect(search[1].method).toBe('POST'); + }); + + it('follows pagination until nextPage is null', async () => { + mockAlbumApi({ + pages: [ + { items: [asset('a', '2024-01-01T00:00:00Z')], nextPage: '2' }, + { items: [asset('b', '2024-01-02T00:00:00Z')], nextPage: '3' }, + { items: [asset('c', '2024-01-03T00:00:00Z')], nextPage: null }, + ], + }); + + const album = await immich.getAlbum('album-1'); + expect(album?.assets.map((a) => a.id)).toEqual(['c', 'b', 'a']); // desc by fileCreatedAt + }); + + it('honours the album sort order', async () => { + const items = [ + asset('old', '2024-01-01T00:00:00Z'), + asset('new', '2024-06-01T00:00:00Z'), + asset('mid', '2024-03-01T00:00:00Z'), + ]; + + mockAlbumApi({ order: 'asc', pages: [{ items, nextPage: null }] }); + let album = await immich.getAlbum('album-1'); + expect(album?.assets.map((a) => a.id)).toEqual(['old', 'mid', 'new']); + + cache.clear(); // same album id — otherwise the asc result is reused + mockAlbumApi({ order: 'desc', pages: [{ items, nextPage: null }] }); + album = await immich.getAlbum('album-1'); + expect(album?.assets.map((a) => a.id)).toEqual(['new', 'mid', 'old']); + }); }); }); diff --git a/lib/__tests__/rate-limit.test.ts b/lib/__tests__/rate-limit.test.ts index 554a586..fc4d976 100644 --- a/lib/__tests__/rate-limit.test.ts +++ b/lib/__tests__/rate-limit.test.ts @@ -1,48 +1,87 @@ -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; import { NextRequest } from 'next/server'; + +let mockHops = 0; +vi.mock('@/lib/config', () => ({ + getConfig: () => ({ trustedProxyHops: mockHops }), +})); + import { checkRateLimit, getClientIp } from '@/lib/rate-limit'; -describe('getClientIp', () => { - it('prioritizes request.ip when present', () => { - const req = { - ip: '1.2.3.4', - headers: new Headers({ - 'x-real-ip': '5.6.7.8', - 'x-forwarded-for': '9.10.11.12', - }), - } as unknown as NextRequest; +function req(headers: Record, ip?: string): NextRequest { + return { ...(ip ? { ip } : {}), headers: new Headers(headers) } as unknown as NextRequest; +} - expect(getClientIp(req)).toBe('1.2.3.4'); +describe('getClientIp with no proxy configured (TRUSTED_PROXY_HOPS=0)', () => { + beforeEach(() => { + mockHops = 0; }); - it('falls back to x-real-ip if request.ip is missing', () => { - const req = { - headers: new Headers({ - 'x-real-ip': '5.6.7.8', - 'x-forwarded-for': '9.10.11.12', - }), - } as unknown as NextRequest; - - expect(getClientIp(req)).toBe('5.6.7.8'); + it('prioritizes request.ip when the platform provides it', () => { + expect(req({ 'x-real-ip': '5.6.7.8' }, '1.2.3.4')).toBeTruthy(); + expect(getClientIp(req({ 'x-real-ip': '5.6.7.8' }, '1.2.3.4'))).toBe('1.2.3.4'); }); - it('falls back to x-forwarded-for if request.ip and x-real-ip are missing', () => { - const req = { - headers: new Headers({ - 'x-forwarded-for': '9.10.11.12, 13.14.15.16', - }), - } as unknown as NextRequest; - - // Should take the first IP from x-forwarded-for list - expect(getClientIp(req)).toBe('9.10.11.12'); + it('falls back to headers on a best-effort basis', () => { + expect(getClientIp(req({ 'x-real-ip': '5.6.7.8' }))).toBe('5.6.7.8'); + expect(getClientIp(req({ 'x-forwarded-for': '9.10.11.12, 13.14.15.16' }))).toBe('9.10.11.12'); }); - it('returns unknown if no IP headers are present', () => { - const req = { - headers: new Headers(), - } as unknown as NextRequest; + it('returns unknown if no IP information is available at all', () => { + expect(getClientIp(req({}))).toBe('unknown'); + }); +}); - expect(getClientIp(req)).toBe('unknown'); +describe('getClientIp behind one trusted proxy (TRUSTED_PROXY_HOPS=1, e.g. nginx)', () => { + beforeEach(() => { + mockHops = 1; + }); + + it('takes the rightmost X-Forwarded-For entry, which the proxy appended itself', () => { + // nginx `$proxy_add_x_forwarded_for` appends the real peer address, so the + // last entry is authoritative no matter what the client sent. + expect(getClientIp(req({ 'x-forwarded-for': '203.0.113.9' }))).toBe('203.0.113.9'); + }); + + it('ignores client-supplied entries to the left of the appended one', () => { + // The attacker sends `X-Forwarded-For: 1.1.1.1`; nginx appends their real IP. + const spoofed = req({ 'x-forwarded-for': '1.1.1.1, 203.0.113.9' }); + expect(getClientIp(spoofed)).toBe('203.0.113.9'); + }); + + it('cannot be pinned to an attacker-chosen bucket by a long forged chain', () => { + const forged = req({ 'x-forwarded-for': '1.1.1.1, 2.2.2.2, 3.3.3.3, 203.0.113.9' }); + expect(getClientIp(forged)).toBe('203.0.113.9'); + }); + + it('does not trust a spoofed X-Real-IP over the appended X-Forwarded-For entry', () => { + const spoofed = req({ 'x-real-ip': '1.1.1.1', 'x-forwarded-for': '9.9.9.9, 203.0.113.9' }); + expect(getClientIp(spoofed)).toBe('203.0.113.9'); + }); + + it('accepts X-Real-IP when the proxy sets only that header (nginx overwrites it)', () => { + expect(getClientIp(req({ 'x-real-ip': '203.0.113.9' }))).toBe('203.0.113.9'); + }); + + it('refuses to identify a request that did not traverse the proxy', () => { + // No proxy headers at all, yet hops are configured: the request reached the + // app directly. Fall back to unknown rather than to a spoofable value. + expect(getClientIp(req({}))).toBe('unknown'); + }); +}); + +describe('getClientIp behind two trusted proxies (TRUSTED_PROXY_HOPS=2)', () => { + beforeEach(() => { + mockHops = 2; + }); + + it('skips both proxy hops from the right', () => { + const r = req({ 'x-forwarded-for': '203.0.113.9, 10.0.0.5' }); + expect(getClientIp(r)).toBe('203.0.113.9'); + }); + + it('returns unknown when the chain is shorter than the configured hop count', () => { + expect(getClientIp(req({ 'x-forwarded-for': '10.0.0.5' }))).toBe('unknown'); }); }); diff --git a/lib/admin/auth.ts b/lib/admin/auth.ts index 89a2150..66c3929 100644 --- a/lib/admin/auth.ts +++ b/lib/admin/auth.ts @@ -5,14 +5,20 @@ import crypto from 'crypto'; import { env } from '../env'; +import { resolveAuthSecret } from '../secret'; import { cookies } from 'next/headers'; const COOKIE_NAME = 'folio_admin_session'; const SESSION_DURATION_MS = 24 * 60 * 60 * 1000; // 24 hours function getSigningKey(): Buffer { - const secret = env.AUTH_SECRET || 'dev-fallback-secret'; - return crypto.createHash('sha256').update(`admin:${secret}`).digest(); + // Bind the key to ADMIN_PASSWORD as well, so rotating the password + // immediately invalidates every outstanding session token. + const secret = resolveAuthSecret(); + return crypto + .createHash('sha256') + .update(`admin:${secret}:${env.ADMIN_PASSWORD ?? ''}`) + .digest(); } /** Create a signed session token. */ @@ -37,7 +43,12 @@ export function verifyAdminToken(token: string): boolean { const key = getSigningKey(); const expectedSig = crypto.createHmac('sha256', key).update(data).digest('base64url'); - if (!crypto.timingSafeEqual(Buffer.from(sig), Buffer.from(expectedSig))) { + // timingSafeEqual throws on length mismatch — guard first so a malformed + // cookie yields a clean 401 instead of an unhandled RangeError (500). + const sigBuf = Buffer.from(sig); + const expectedBuf = Buffer.from(expectedSig); + if (sigBuf.length !== expectedBuf.length) return false; + if (!crypto.timingSafeEqual(sigBuf, expectedBuf)) { return false; } diff --git a/lib/auth.ts b/lib/auth.ts index 4f3500b..c49f381 100644 --- a/lib/auth.ts +++ b/lib/auth.ts @@ -48,9 +48,17 @@ function hmac(data: string): string { return crypto.createHmac('sha256', getConfig().authSecret).update(data).digest('hex'); } -function authToken(key: string, passwordSecret: string): string { - // Use the hash/password as part of the HMAC for a secure, unique session token - return hmac(`${key}:${passwordSecret}`); +/** + * Build a session token of the form `.`. + * + * The expiry is part of the signed payload, so it cannot be extended by the + * client. Without it the token was a pure function of (slug, password) and + * therefore valid forever — the 24h lifetime existed only as the cookie's + * Max-Age, which the client enforces and an attacker replaying a captured + * token simply ignores. + */ +function authToken(key: string, passwordSecret: string, expiresAt: number): string { + return `${expiresAt}.${hmac(`${key}:${passwordSecret}:${expiresAt}`)}`; } function cookieName(key: string, type: 'subpage' | 'album'): string { @@ -132,8 +140,8 @@ export function authenticate( if (!isValid) return null; - const token = authToken(key, storedPassword); const maxAge = TOKEN_EXPIRY_HOURS * 60 * 60; + const token = authToken(key, storedPassword, Date.now() + maxAge * 1000); const secure = process.env.NODE_ENV === 'production' ? '; Secure' : ''; return `${cookieName(key, type)}=${token}; HttpOnly; Path=/; Max-Age=${maxAge}; SameSite=Strict${secure}`; @@ -154,10 +162,21 @@ export function isAuthenticated( const cookie = getCookie(cookieName(key, type)); if (!cookie) return false; - const expected = authToken(key, storedPassword); + // Legacy tokens (bare HMAC, no expiry) do not parse here and are rejected; + // the visitor simply re-enters the password. + const sep = cookie.indexOf('.'); + if (sep === -1) return false; + + const expiresAt = Number(cookie.slice(0, sep)); + if (!Number.isFinite(expiresAt) || expiresAt <= Date.now()) return false; + + const expected = authToken(key, storedPassword, expiresAt); // Constant-time comparison to prevent timing attacks try { - return crypto.timingSafeEqual(Buffer.from(cookie, 'hex'), Buffer.from(expected, 'hex')); + const a = Buffer.from(cookie, 'utf8'); + const b = Buffer.from(expected, 'utf8'); + if (a.length !== b.length) return false; + return crypto.timingSafeEqual(a, b); } catch { return false; } diff --git a/lib/config/index.ts b/lib/config/index.ts index 49a280f..0b8d326 100644 --- a/lib/config/index.ts +++ b/lib/config/index.ts @@ -1,5 +1,5 @@ -import crypto from 'crypto'; import { env } from '../env'; +import { resolveAuthSecret } from '../secret'; import { loadYaml, clearYamlCache, validateUuid } from './parser'; import { resolveTheme, VALID_LAYOUTS } from './theme'; import { @@ -17,7 +17,6 @@ export * from './schema'; export * from './theme'; let _config: AppConfig | null = null; -let _fallbackSecret: string | null = null; /** Invalidate the cached config so the next getConfig() call re-reads YAML files. */ export function invalidateConfigCache(): void { @@ -58,22 +57,7 @@ export function getConfig(): AppConfig { const apiUrl = env.IMMICH_API_URL; const apiKey = env.IMMICH_API_KEY; - let { AUTH_SECRET } = env; - if (!AUTH_SECRET) { - if (process.env.NODE_ENV === 'production') { - throw new Error( - 'SECURITY ERROR: AUTH_SECRET is not set in production. Please set a long random string as AUTH_SECRET in your .env.', - ); - } - if (!_fallbackSecret) { - _fallbackSecret = crypto.randomBytes(32).toString('hex'); - } - console.warn( - '\n⚠️ SECURITY WARNING: AUTH_SECRET is not set. Generating a temporary random secret for this session.\n Please set a long random string as AUTH_SECRET in your .env for better security.\n', - ); - AUTH_SECRET = _fallbackSecret; - } - const authSecret = AUTH_SECRET; + const authSecret = resolveAuthSecret(); const gallery = loadYaml('gallery.yaml'); const settings = loadYaml('settings.yaml') || {}; @@ -109,7 +93,7 @@ export function getConfig(): AppConfig { albumHeroImages: {}, cacheTtl: env.CACHE_TTL * 1000, rateLimitRpm: env.RATE_LIMIT_RPM, - trustedProxies: env.TRUSTED_PROXIES, + trustedProxyHops: env.TRUSTED_PROXY_HOPS, needsSetup: true, }; return _config; @@ -293,7 +277,7 @@ export function getConfig(): AppConfig { albumHeroImages, cacheTtl: env.CACHE_TTL * 1000, rateLimitRpm: env.RATE_LIMIT_RPM, - trustedProxies: env.TRUSTED_PROXIES, + trustedProxyHops: env.TRUSTED_PROXY_HOPS, needsSetup: false, }; return _config; diff --git a/lib/config/schema.ts b/lib/config/schema.ts index 8c44d17..f1c2751 100644 --- a/lib/config/schema.ts +++ b/lib/config/schema.ts @@ -100,7 +100,7 @@ export interface AppConfig { albumHeroImages: Record; cacheTtl: number; rateLimitRpm: number; - trustedProxies: string[]; + trustedProxyHops: number; needsSetup?: boolean; } diff --git a/lib/env.ts b/lib/env.ts index 3121dbc..c492219 100644 --- a/lib/env.ts +++ b/lib/env.ts @@ -11,7 +11,7 @@ export interface Env { CACHE_TTL: number; RATE_LIMIT_RPM: number; AUTH_SECRET?: string; - TRUSTED_PROXIES: string[]; + TRUSTED_PROXY_HOPS: number; WEBHOOK_SECRET?: string; ADMIN_PASSWORD?: string; } @@ -39,6 +39,26 @@ function parseEnv(): Env { const rateLimit = rateLimitStr && !isNaN(parseInt(rateLimitStr, 10)) ? parseInt(rateLimitStr, 10) : 1500; + // Number of reverse proxies in front of the app. Determines how far from the + // right of X-Forwarded-For the real client IP sits. nginx/Traefik/Caddy = 1; + // add 1 for each additional layer (e.g. Cloudflare in front of nginx = 2). + const hopsStr = process.env.TRUSTED_PROXY_HOPS; + let trustedProxyHops = hopsStr && !isNaN(parseInt(hopsStr, 10)) ? parseInt(hopsStr, 10) : 0; + + // TRUSTED_PROXIES (removed) matched proxy IPs against the socket peer address, + // which self-hosted Next.js never exposes — so it silently disabled per-client + // rate limiting and lumped every visitor into one shared bucket. Migrate the + // common single-proxy case rather than leaving those deploys broken. + if (trustedProxyHops === 0 && process.env.TRUSTED_PROXIES) { + trustedProxyHops = 1; + console.warn( + '\n⚠️ TRUSTED_PROXIES is deprecated and has no effect. Assuming TRUSTED_PROXY_HOPS=1.\n' + + ' Set TRUSTED_PROXY_HOPS explicitly (nginx/Traefik/Caddy = 1) and remove TRUSTED_PROXIES.\n', + ); + } + + if (trustedProxyHops < 0) trustedProxyHops = 0; + return { IMMICH_API_URL: apiUrl, IMMICH_API_KEY: apiKey as string, @@ -47,10 +67,7 @@ function parseEnv(): Env { CACHE_TTL: Math.max(0, cacheTtl), RATE_LIMIT_RPM: Math.max(1, rateLimit), AUTH_SECRET: process.env.AUTH_SECRET, - TRUSTED_PROXIES: (process.env.TRUSTED_PROXIES || '') - .split(',') - .map((ip) => ip.trim()) - .filter((ip) => ip !== ''), + TRUSTED_PROXY_HOPS: trustedProxyHops, WEBHOOK_SECRET: process.env.WEBHOOK_SECRET || undefined, ADMIN_PASSWORD: process.env.ADMIN_PASSWORD || undefined, }; diff --git a/lib/immich.ts b/lib/immich.ts index 49a6fc0..8cff3a2 100644 --- a/lib/immich.ts +++ b/lib/immich.ts @@ -80,16 +80,19 @@ class ImmichClient { return getConfig(); } - private async request(endpoint: string): Promise { + private async request(endpoint: string, body?: unknown): Promise { if (this.config.needsSetup) return null; const url = `${this.config.immich.apiUrl}${endpoint}`; try { const res = await fetch(url, { + method: body === undefined ? 'GET' : 'POST', headers: { 'x-api-key': this.config.immich.apiKey, Accept: 'application/json', + ...(body === undefined ? {} : { 'Content-Type': 'application/json' }), }, + ...(body === undefined ? {} : { body: JSON.stringify(body) }), }); if (!res.ok) { @@ -320,6 +323,49 @@ class ImmichClient { /** * Get a single album with its assets. */ + /** + * Fetch every asset belonging to an album. + * + * Immich 3.x no longer embeds assets in `GET /albums/:id` — the response + * still carries `assetCount` but `assets` comes back empty, and + * `?withoutAssets=false` does not change that. Metadata search is the + * supported replacement. + * + * `withExif` is required: without it the response omits `exifInfo`, and the + * grid (aspect ratios), the lightbox EXIF panel and the map (GPS) all go + * blank even though the images themselves load. + */ + private async fetchAlbumAssets(albumId: string): Promise { + const PAGE_SIZE = 1000; // Immich rejects size > 1000 with a validation error + const MAX_PAGES = 100; // Backstop against a malformed nextPage looping forever + const assets: ImmichAsset[] = []; + let page = 1; + + for (let i = 0; i < MAX_PAGES; i++) { + const res = await this.request<{ + assets?: { items?: ImmichAsset[]; nextPage?: string | null }; + }>('/search/metadata', { + albumIds: [albumId], + withExif: true, + size: PAGE_SIZE, + page, + }); + + const items = res?.assets?.items; + if (!items?.length) break; + assets.push(...items); + + // nextPage is a string page number, or null on the last page. The + // response's `total` only counts the current page, so it cannot bound + // this loop. + const next = Number(res?.assets?.nextPage); + if (!Number.isFinite(next) || next <= page) break; + page = next; + } + + return assets; + } + async getAlbum(albumId: string): Promise { // Security: only serve configured albums if (!this.config.albums.includes(albumId)) { @@ -337,11 +383,34 @@ class ImmichClient { const promise = (async () => { try { - const album = await this.request(`/albums/${encodeURIComponent(albumId)}`); + // Metadata and assets come from two different endpoints (see + // fetchAlbumAssets); they are independent, so fetch them together. + const [album, assets] = await Promise.all([ + this.request(`/albums/${encodeURIComponent(albumId)}`), + this.fetchAlbumAssets(albumId), + ]); if (!album) return null; + // An album that Immich says has photos but returns none is almost + // always a server older than 3.0, where metadata search behaves + // differently. Say so instead of rendering a silently empty gallery. + if (album.assetCount > 0 && assets.length === 0) { + console.warn( + `[Immich] Album "${album.albumName}" reports ${album.assetCount} assets but the metadata search returned none. ` + + `Immich Folio requires Immich 3.0 or newer.`, + ); + } + // Filter out trashed assets - album.assets = (album.assets || []).filter((a) => !a.isTrashed); + album.assets = assets.filter((a) => !a.isTrashed); + + // The album endpoint used to return assets in the album's configured + // order. Metadata search happens to default to fileCreatedAt desc, but + // that is not contractual — sort explicitly so 'asc' albums are right. + const dir = album.order === 'asc' ? 1 : -1; + album.assets.sort( + (a, b) => dir * (Date.parse(a.fileCreatedAt) - Date.parse(b.fileCreatedAt)), + ); const name = this.config.albumOverrides[album.id] ?? album.albumName; const description = this.config.albumDescriptions[album.id] ?? album.description ?? ''; diff --git a/lib/mapService.ts b/lib/mapService.ts index 0d062c5..3a93303 100644 --- a/lib/mapService.ts +++ b/lib/mapService.ts @@ -2,15 +2,30 @@ import { immich } from './immich'; import { getConfig } from './config'; import { cache } from './cache'; +/** + * One album's contribution to a location marker. + * + * Aggregation is deliberately kept per-album (rather than pre-summed) so the + * API layer can drop albums the viewer is not authorized to see *before* + * computing coords, counts and the cover asset. Summing here would leak + * protected albums into every marker. + */ +export interface MapAlbumEntry { + id: string; + name: string; + slug: string; + subpageSlug?: string; + photoCount: number; + latSum: number; + lngSum: number; + coverAssetId: string; +} + /** A clustered map marker — one per unique city/country. */ export interface MapLocation { city: string; country: string; - lat: number; - lng: number; - photoCount: number; - coverAssetId: string; - albums: { name: string; slug: string; subpageSlug?: string }[]; + albums: MapAlbumEntry[]; } /** @@ -50,60 +65,50 @@ export async function getMapData(): Promise { fullAlbums.push(...chunkResults); } - // Bucket assets by city+country - const buckets = new Map< - string, - { - latSum: number; - lngSum: number; - count: number; - coverAssetId: string; - albumIds: Set; - } - >(); + // Bucket assets by city+country, keeping each album's contribution separate + const buckets = new Map>(); for (const album of fullAlbums) { if (!album) continue; + const meta = albumMeta.get(album.id); + if (!meta) continue; + for (const asset of album.assets) { const exif = asset.exifInfo; if (!exif?.latitude || !exif?.longitude || !exif?.city || !exif?.country) continue; const key = `${exif.city}|${exif.country}`; - let bucket = buckets.get(key); - if (!bucket) { - bucket = { + let byAlbum = buckets.get(key); + if (!byAlbum) { + byAlbum = new Map(); + buckets.set(key, byAlbum); + } + + let entry = byAlbum.get(album.id); + if (!entry) { + entry = { + id: album.id, + name: meta.name, + slug: meta.slug, + subpageSlug: meta.subpageSlug, + photoCount: 0, latSum: 0, lngSum: 0, - count: 0, coverAssetId: asset.id, - albumIds: new Set(), }; - buckets.set(key, bucket); + byAlbum.set(album.id, entry); } - bucket.latSum += exif.latitude; - bucket.lngSum += exif.longitude; - bucket.count++; - bucket.albumIds.add(album.id); + entry.latSum += exif.latitude; + entry.lngSum += exif.longitude; + entry.photoCount++; } } // Convert buckets to MapLocation[] const locations: MapLocation[] = []; - for (const [key, bucket] of buckets) { + for (const [key, byAlbum] of buckets) { const [city, country] = key.split('|'); - const lat = bucket.latSum / bucket.count; - const lng = bucket.lngSum / bucket.count; - locations.push({ - city, - country, - lat, - lng, - photoCount: bucket.count, - coverAssetId: bucket.coverAssetId, - albums: [...bucket.albumIds] - .map((id) => albumMeta.get(id)) - .filter((m): m is NonNullable => !!m), - }); + locations.push({ city, country, albums: [...byAlbum.values()] }); } cache.set(cacheKey, locations, config.cacheTtl); diff --git a/lib/rate-limit.ts b/lib/rate-limit.ts index 33aa971..7560548 100644 --- a/lib/rate-limit.ts +++ b/lib/rate-limit.ts @@ -12,28 +12,59 @@ import { NextRequest } from 'next/server'; import { getConfig } from './config'; +/** Bucket key used when the client cannot be identified with any confidence. */ +const UNIDENTIFIED = 'unknown'; + +/** + * Resolve the client IP to use as a rate-limit bucket key. + * + * Self-hosted Next.js cannot see the socket peer address (`request.ip` is only + * populated by platforms like Vercel), so behind a reverse proxy the client IP + * has to come from a header. Headers are attacker-controlled unless a proxy + * overwrites them, which is what TRUSTED_PROXY_HOPS declares. + * + * With `TRUSTED_PROXY_HOPS=n`, the entry `n` from the *right* of + * X-Forwarded-For is used. Proxies append the address they actually saw + * (nginx: `proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for`), so + * everything to the left is client-supplied and ignored. A client that sends + * `X-Forwarded-For: 1.1.1.1` therefore cannot pick its own bucket. + * + * ⚠️ This holds only while the app is reachable *exclusively* through the + * proxy. Bind the server to localhost — if clients can connect directly they + * control the whole header and no parsing strategy can help. + */ export function getClientIp(request: NextRequest): string { - const config = getConfig(); + const hops = getConfig().trustedProxyHops; + // @ts-expect-error - Next.js 15+ removed request.ip from types but hosting platforms still populate it const directIp = request.ip as string | undefined; + // The real socket peer address, where available, is never spoofable. + if (directIp) return directIp; const xForwardedFor = request.headers.get('x-forwarded-for'); const xRealIp = request.headers.get('x-real-ip'); - // If trusted proxies are configured, only trust headers if the request - // comes from one of those proxies. - if (config.trustedProxies.length > 0) { - if (directIp && config.trustedProxies.includes(directIp)) { - return xRealIp ?? xForwardedFor?.split(',')[0].trim() ?? directIp; + if (hops > 0) { + if (xForwardedFor) { + const chain = xForwardedFor + .split(',') + .map((s) => s.trim()) + .filter(Boolean); + // Chain shorter than the configured hop count means the request did not + // traverse the full proxy chain — do not fall back to a spoofable entry. + return chain[chain.length - hops] ?? UNIDENTIFIED; } - // If we have trusted proxies but the direct IP is unknown or untrusted, - // we return the direct IP (if any) and ignore potentially spoofed headers. - return directIp ?? 'unknown'; + // nginx's `proxy_set_header X-Real-IP $remote_addr` overwrites rather than + // appends, so it is trustworthy — but only for a single hop, since an outer + // proxy would leave an inner proxy's value in place. + if (xRealIp && hops === 1) return xRealIp; + return UNIDENTIFIED; } - // Fallback for self-hosted environments without explicit trusted proxies: - // We have to trust headers as a best-effort, but this is vulnerable to spoofing. - return directIp ?? xRealIp ?? xForwardedFor?.split(',')[0].trim() ?? 'unknown'; + // No proxy declared: headers are best-effort only. This still separates + // honest clients into their own buckets, but a deliberate attacker can spoof + // them. Set TRUSTED_PROXY_HOPS if you run behind a reverse proxy. + return xRealIp ?? xForwardedFor?.split(',')[0].trim() ?? UNIDENTIFIED; } interface RateLimitEntry { diff --git a/lib/secret.ts b/lib/secret.ts new file mode 100644 index 0000000..ada4bf9 --- /dev/null +++ b/lib/secret.ts @@ -0,0 +1,35 @@ +/** + * Single source of truth for AUTH_SECRET resolution. + * + * Fails closed in production: a missing AUTH_SECRET throws rather than falling + * back to a guessable constant. In development an ephemeral per-process secret + * is generated instead, so sessions simply do not survive a restart. + */ + +import crypto from 'crypto'; +import { env } from './env'; + +let _fallbackSecret: string | null = null; +let _warned = false; + +export function resolveAuthSecret(): string { + const secret = env.AUTH_SECRET; + if (secret) return secret; + + if (process.env.NODE_ENV === 'production') { + throw new Error( + 'SECURITY ERROR: AUTH_SECRET is not set in production. Please set a long random string as AUTH_SECRET in your .env.', + ); + } + + if (!_fallbackSecret) { + _fallbackSecret = crypto.randomBytes(32).toString('hex'); + } + if (!_warned) { + console.warn( + '\n⚠️ SECURITY WARNING: AUTH_SECRET is not set. Generating a temporary random secret for this session.\n Please set a long random string as AUTH_SECRET in your .env for better security.\n', + ); + _warned = true; + } + return _fallbackSecret; +} diff --git a/lib/tokens.ts b/lib/tokens.ts index 9305840..646419d 100644 --- a/lib/tokens.ts +++ b/lib/tokens.ts @@ -8,6 +8,7 @@ import crypto from 'crypto'; import { getConfig } from './config'; +import { isUuid } from './uuid'; let _key: Buffer | null = null; @@ -72,8 +73,7 @@ export function decodeAssetId(token: string): string | null { } // Validate it looks like a UUID - const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; - return uuidRegex.test(decrypted) ? decrypted : null; + return isUuid(decrypted) ? decrypted : null; } catch { return null; } diff --git a/lib/uuid.ts b/lib/uuid.ts new file mode 100644 index 0000000..1e905ba --- /dev/null +++ b/lib/uuid.ts @@ -0,0 +1,14 @@ +/** + * Shared UUID validation. + * + * Immich object IDs are interpolated into upstream API URLs, so anything that + * reaches `fetch()` must be shape-checked first: the URL constructor resolves + * `..` segments, which would otherwise turn an album ID into an arbitrary + * Immich endpoint reached with the server's API key. + */ + +const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +export function isUuid(value: string): boolean { + return UUID_REGEX.test(value); +} diff --git a/middleware.ts b/middleware.ts index 1810f36..2c14930 100644 --- a/middleware.ts +++ b/middleware.ts @@ -7,7 +7,11 @@ export function middleware(request: NextRequest) { // Define CSP directives const cspDirectives = [ "default-src 'self'", - `script-src 'self' 'nonce-${nonce}' 'strict-dynamic' 'unsafe-inline'`, // 'unsafe-inline' is ignored by modern browsers if nonce/strict-dynamic is present, but kept for fallback + // No 'unsafe-inline' fallback: CSP3 browsers ignore it next to a nonce, but + // CSP2-only browsers ignore 'strict-dynamic' and would honour it — making + // the fallback strictly worse than having none. There are no inline + //