From 5c7de96b7fda9ca037a01b93fabe69d1be224893 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Sat, 9 May 2026 15:55:09 +0200 Subject: [PATCH] fix(auth): default COOKIE_SECURE to 'auto' in production + first-install UX (#427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two intertwined bugs reported in #427 by @iSchumi6210: 1. Login silently fails over HTTP. Backend defaulted COOKIE_SECURE to true when NODE_ENV=production. Over plain HTTP the browser drops the Secure cookie → next /auth/session request returns 401 → redirect back to /admin/login → no error shown. picpeak-setup.sh writes NODE_ENV=production but never writes COOKIE_SECURE, so every first-time install without a reverse proxy hits this. 2. Admin password is generated but admins can't find it. The 001_init.js migration writes the generated password to data/ADMIN_CREDENTIALS.txt inside the backend container, but picpeak-setup.sh only copies it out when --reset-admin-password is passed. Default-path users never see it and resort to manual bcrypt updates in psql. Changes: - tokenUtils.js: production default goes from `true` to `'auto'`. On real HTTPS req.secure is true → Secure flag is still emitted (no security regression for reverse-proxy deployments). On plain HTTP req.secure is false → Secure flag omitted → login works. Users who explicitly want the strict HTTPS-only behaviour can still set COOKIE_SECURE=true. - .env.example: rewrite the COOKIE_SECURE block to make the new default obvious and explain when to override (set =true for strict, =false to skip the per-request check, leave unset for the auto behaviour). - picpeak-setup.sh (both Docker and native paths): - Write COOKIE_SECURE=auto explicitly to the generated .env (defense in depth so the right behaviour is preserved even if the backend default flips again later) - After migrations, ALWAYS copy ADMIN_CREDENTIALS.txt out of the backend container/data dir to the host data dir, chmod 600, and print the email + password to the install output. The credentials file remains as a backup record that the operator should delete after noting the password. Verified locally with all 4 permutations of NODE_ENV × COOKIE_SECURE: production, unset → HTTPS: secure=true ✓ HTTP: secure=false ✓ (was both true) production, =true → both: secure=true (strict opt-in preserved) production, =auto → HTTPS: secure=true HTTP: secure=false (already-correct) development, unset → both: secure=false (dev unchanged) --- backend/.env.example | 31 +++++++++++----- backend/src/utils/tokenUtils.js | 29 ++++++++++----- scripts/picpeak-setup.sh | 64 ++++++++++++++++++++++++++++----- 3 files changed, 97 insertions(+), 27 deletions(-) diff --git a/backend/.env.example b/backend/.env.example index ba134257..15ab3ddd 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -10,18 +10,31 @@ PORT=3001 JWT_SECRET=your-very-secure-jwt-secret-at-least-32-characters-long-example123456 # Auth cookie Secure flag -# unset - default: follows NODE_ENV (production=true, dev=false) -# true - always set Secure (HTTPS-only cookies; breaks plain-HTTP access) +# unset - default: 'auto' in production, false in dev (#427) +# true - always set Secure (HTTPS-only cookies; breaks plain-HTTP access — +# login appears to succeed but the browser silently drops the +# cookie, leaving you in a redirect loop. Only set this if you +# ALWAYS reach the site via HTTPS) # false - never set Secure (allows HTTP; cookies not protected on HTTPS) -# auto - decide per request: Secure on HTTPS, not on HTTP +# auto - decide per request: Secure on HTTPS, not on HTTP. Reads +# req.secure from Express which respects X-Forwarded-Proto from a +# trusted reverse proxy. This is the default and is the right +# choice for most deployments. # -# Use COOKIE_SECURE=auto if your deployment is reachable over both HTTPS -# (via a reverse proxy like Nginx Proxy Manager, Traefik, Caddy) AND plain -# HTTP (e.g. LAN access at http://192.168.x.x:3001). The backend reads -# req.secure from Express, which respects the X-Forwarded-Proto header -# when the proxy is in the trust list. +# Why 'auto' is the default in production: +# - On real HTTPS (reverse proxy with X-Forwarded-Proto), req.secure is +# true → Secure flag is still emitted. No security regression vs. true. +# - On plain HTTP (LAN access, first-time install before reverse proxy is +# wired up), req.secure is false → Secure flag is omitted → login works +# instead of silently looping back to /admin/login. # -# Requirements for auto mode: +# When you'd set this explicitly: +# - COOKIE_SECURE=true → strict HTTPS-only deployments where you want +# defense in depth against accidentally serving over HTTP. +# - COOKIE_SECURE=false → you intentionally only ever serve over HTTP and +# don't want the per-request check (rare). +# +# Requirements for 'auto' mode to detect HTTPS correctly: # 1. Your reverse proxy MUST send X-Forwarded-Proto: https on HTTPS # requests. Standard configs for NPM/Traefik/Caddy do this by default. # 2. The proxy must be on a trusted IP range. By default PicPeak trusts diff --git a/backend/src/utils/tokenUtils.js b/backend/src/utils/tokenUtils.js index e5a7963a..4d30760b 100644 --- a/backend/src/utils/tokenUtils.js +++ b/backend/src/utils/tokenUtils.js @@ -7,15 +7,24 @@ const DEFAULT_MAX_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours /** * Cookie "Secure" flag mode: - * - true → always set Secure (HTTPS-only) - * - false → never set Secure (allow plain HTTP) + * - true → always set Secure (HTTPS-only — cookie won't be sent over HTTP at all) + * - false → never set Secure (allow plain HTTP — cookie has no in-flight protection) * - 'auto' → decide per-request based on req.secure (X-Forwarded-Proto - * via Express `trust proxy`). Useful when the same deployment - * is reachable over both HTTPS (via reverse proxy) and LAN HTTP. + * via Express `trust proxy`). Emits Secure when actual HTTPS is + * detected, omits it on plain HTTP. This is the right default + * for deployments reachable via both HTTPS (reverse proxy) and + * LAN HTTP, and for first-time installs that haven't set up a + * reverse proxy yet. * - * Default: follows NODE_ENV (production → true, dev → false) — unchanged - * from previous behavior. Users who want the auto mode must opt in with - * COOKIE_SECURE=auto in their .env. + * Default: + * - production → 'auto' (#427: previously hard `true`, which caused silent + * login loops over HTTP because the browser drops the + * Secure cookie. 'auto' is strictly more lenient than `true` + * on real HTTPS — req.secure is true → Secure flag still + * emitted — so this is not a security regression for + * reverse-proxy deployments. Users who explicitly want the + * HTTPS-only behaviour can still set COOKIE_SECURE=true.) + * - dev → false (allow http://localhost in browsers without HSTS gymnastics) */ const secureCookieMode = (() => { const raw = typeof process.env.COOKIE_SECURE === 'string' @@ -24,8 +33,10 @@ const secureCookieMode = (() => { if (raw === 'auto') return 'auto'; if (raw === 'true') return true; if (raw === 'false') return false; - // No env var set → legacy default - return process.env.NODE_ENV === 'production'; + // No env var set → infer from NODE_ENV. Production defaults to 'auto' + // (per-request) rather than hard `true` so first-time HTTP installs don't + // silently fail (#427). + return process.env.NODE_ENV === 'production' ? 'auto' : false; })(); const sameSiteDefault = process.env.COOKIE_SAMESITE || 'Lax'; const cookieDomain = process.env.COOKIE_DOMAIN; diff --git a/scripts/picpeak-setup.sh b/scripts/picpeak-setup.sh index 554f00f8..6d84131c 100755 --- a/scripts/picpeak-setup.sh +++ b/scripts/picpeak-setup.sh @@ -490,6 +490,13 @@ SMTP_FROM=${SMTP_USER:-noreply@localhost} FRONTEND_URL=${DOMAIN_NAME:+https://$DOMAIN_NAME} ADMIN_URL=${DOMAIN_NAME:+https://$DOMAIN_NAME} +# Auth cookie behavior — 'auto' emits Secure on HTTPS, omits it on HTTP. +# Without this, first-time HTTP installs (no reverse proxy yet) silently +# fail at login because the browser drops Secure cookies over HTTP (#427). +# On HTTPS via reverse proxy, req.secure is true → Secure flag is still +# emitted, so this is not a security regression for production deploys. +COOKIE_SECURE=auto + # Features ENABLE_FILE_WATCHER=true ENABLE_EXPIRATION_CHECKER=true @@ -499,27 +506,27 @@ EOF # Ensure bind mounts are writable by mapped user chown -R "$host_uid":"$host_gid" "$app_dir"/storage "$app_dir"/logs "$app_dir"/backup "$app_dir"/data "$app_dir"/events 2>/dev/null || true - + # Create docker-compose.yml if it doesn't exist if [[ ! -f "$app_dir/docker-compose.yml" ]]; then log_step "Creating Docker Compose configuration..." create_docker_compose_file "$app_dir" fi - + # Set up SSL if requested if [[ "$ENABLE_SSL" == "true" ]] && [[ -n "$DOMAIN_NAME" ]]; then setup_ssl_docker "$app_dir" fi - + # Start services log_step "Starting services..." cd "$app_dir" docker compose up -d - + # Wait for services to be ready log_step "Waiting for services to initialize..." sleep 10 - + # Run database migrations log_step "Running database migrations..." docker compose exec -T backend npm run migrate @@ -532,7 +539,28 @@ EOF log_warn "Automatic admin password reset failed; run reset-admin-password.js inside the backend container." fi fi - + + # Always surface the admin credentials file (#427: iSchumi reported + # admins couldn't find the generated password — the migration writes it + # to the in-container path and we never copied it to the host unless + # --reset-admin-password was used). Best-effort: a missing file just + # means the migration ran on a pre-existing DB and didn't generate one. + if docker compose cp backend:/app/data/ADMIN_CREDENTIALS.txt "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null; then + chown "$host_uid":"$host_gid" "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true + chmod 600 "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true + log_step "Admin credentials saved to: $app_dir/data/ADMIN_CREDENTIALS.txt" + # Show the password in the install output so the operator can log + # in immediately. The file remains as a backup record. + echo + echo "--------------------------------------------------" + grep -E '^Email:|^Password:' "$app_dir/data/ADMIN_CREDENTIALS.txt" 2>/dev/null || true + echo "--------------------------------------------------" + echo " Login URL: ${DOMAIN_NAME:+https://$DOMAIN_NAME}${DOMAIN_NAME:-http://YOUR_HOST_IP:3000}/admin" + echo " Full credentials file: $app_dir/data/ADMIN_CREDENTIALS.txt" + echo " Delete the file after recording the password." + echo + fi + log_success "Docker installation completed!" } @@ -766,6 +794,9 @@ SMTP_FROM=${SMTP_USER:-noreply@localhost} FRONTEND_URL=${DOMAIN_NAME:+https://$DOMAIN_NAME} ADMIN_URL=${DOMAIN_NAME:+https://$DOMAIN_NAME} +# Auth cookie behavior — see Docker .env block above for rationale (#427). +COOKIE_SECURE=auto + # Features ENABLE_FILE_WATCHER=true ENABLE_EXPIRATION_CHECKER=true @@ -780,11 +811,11 @@ LOG_LEVEL=info SERVE_FRONTEND=true FRONTEND_DIR=$NATIVE_APP_DIR/app/frontend/dist EOF - + # Set permissions chown -R $NATIVE_APP_USER:$NATIVE_APP_USER "$NATIVE_APP_DIR" chmod 600 "$NATIVE_APP_DIR/app/backend/.env" - + # Run database migrations log_step "Initializing database..." cd "$NATIVE_APP_DIR/app/backend" @@ -796,7 +827,22 @@ EOF log_warn "Automatic admin password reset failed; please run reset-admin-password.js manually." fi fi - + + # Always surface the admin credentials file (#427). + local creds_path="$NATIVE_APP_DIR/app/backend/data/ADMIN_CREDENTIALS.txt" + if [[ -f "$creds_path" ]]; then + chmod 600 "$creds_path" 2>/dev/null || true + log_step "Admin credentials saved to: $creds_path" + echo + echo "--------------------------------------------------" + grep -E '^Email:|^Password:' "$creds_path" 2>/dev/null || true + echo "--------------------------------------------------" + echo " Login URL: ${DOMAIN_NAME:+https://$DOMAIN_NAME}${DOMAIN_NAME:-http://YOUR_HOST_IP:${CUSTOM_PORT:-$DEFAULT_PORT}}/admin" + echo " Full credentials file: $creds_path" + echo " Delete the file after recording the password." + echo + fi + # Create systemd services create_systemd_services