From f63dd67668554da9ee3d1796e00a023298ec18fe Mon Sep 17 00:00:00 2001 From: jtricerolph Date: Wed, 26 Aug 2026 11:03:35 +0000 Subject: [PATCH] Fix kiosk login colliding with real staff login Kiosk sessions were minted under the same cookie name ('hnf_session') as a real staff session, on the same origin -- the browser can end up holding two same-named cookies with different scope with no guarantee which one it sends, so a valid staff login could get shadowed by a stale kiosk cookie (hit 2026-08-26 testing kiosk login on a desktop browser, then unable to log into the normal manage pages). Fix: kiosk sessions now live in their own cookie, 'hvac_kiosk_session', completely independent of the real staff session. hvac's requireAuth checks hnf_session first, falls back to hvac_kiosk_session. Also dropped the now-unneeded DOMAIN cross-subdomain cookie scoping -- this cookie is only ever read by hvac's own backend, so a plain host-scoped cookie is simplest. Co-Authored-By: Claude Sonnet 5 --- backend/src/auth.js | 8 +++++++- backend/src/routes/kiosk.js | 34 +++++++++++++++++++++++----------- docker-compose.yml | 4 ---- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/backend/src/auth.js b/backend/src/auth.js index 5ea5ef3..28ae80f 100644 --- a/backend/src/auth.js +++ b/backend/src/auth.js @@ -5,7 +5,13 @@ const APP_SLUG = process.env.APP_SLUG || 'hvac' const secret = new TextEncoder().encode(process.env.CENTRAL_AUTH_SECRET || '') export async function requireAuth(request, reply) { - const token = request.cookies?.hnf_session + // Real staff sessions (hnf_session, issued by central auth) take priority; the + // kiosk's own session (hvac_kiosk_session, issued locally by routes/kiosk.js) + // is a deliberately separate cookie name so the two can never collide/shadow + // each other in the same browser — see project_hvac_kiosk memory, hit + // 2026-08-26 when kiosk login on a desktop browser interfered with normal + // staff login while both shared the 'hnf_session' name. + const token = request.cookies?.hnf_session || request.cookies?.hvac_kiosk_session if (!token) return reply.status(401).send({ error: 'Not authenticated' }) let payload diff --git a/backend/src/routes/kiosk.js b/backend/src/routes/kiosk.js index 3a707ad..3d2915d 100644 --- a/backend/src/routes/kiosk.js +++ b/backend/src/routes/kiosk.js @@ -2,20 +2,28 @@ import { SignJWT } from 'jose' import bcrypt from 'bcryptjs' import { getConfig } from '../db.js' -const DOMAIN = process.env.DOMAIN || 'localhost' const secret = new TextEncoder().encode(process.env.CENTRAL_AUTH_SECRET || '') -// Same shape as central auth's own cookieOpts() (auth/src/routes/auth.js) — -// Secure only when the request genuinely arrived over HTTPS (NPM's -// X-Forwarded-Proto), domain scoped the same way, so this cookie behaves -// identically to one central auth issued. +// Deliberately its OWN cookie name — NOT 'hnf_session' (central auth's real +// staff-session cookie). Sharing that name caused a real bug (hit 2026-08-26): +// testing the kiosk login on a desktop browser left a stale 'hnf_session' +// cookie behind that could shadow/collide with a subsequent real staff login +// on the same origin, since the browser will happily hold two same-named +// cookies with slightly different scope and there's no guarantee which one it +// sends. A distinct name makes the two sessions fully independent — hvac's own +// requireAuth (../auth.js) checks hnf_session first, falls back to this one. +const KIOSK_COOKIE = 'hvac_kiosk_session' + +// No DOMAIN-based cross-subdomain scoping needed (unlike central auth's own +// cookieOpts()) — this cookie is only ever read by hvac's own backend, never +// by central auth or any other app, so a plain host-scoped cookie is both +// simplest and narrowest. function cookieOpts(request) { const proto = request.headers['x-forwarded-proto'] || request.protocol return { httpOnly: true, secure: proto === 'https', sameSite: 'lax', - domain: DOMAIN === 'localhost' ? undefined : `.${DOMAIN}`, path: '/', maxAge: KIOSK_SESSION_SECONDS, } @@ -28,11 +36,15 @@ function cookieOpts(request) { // expected consequence of shared-secret signing, not a bug. const KIOSK_SESSION_SECONDS = 60 * 60 * 24 * 3650 -// hvac's own requireAuth (../auth.js) verifies the hnf_session cookie itself +// hvac's own requireAuth (../auth.js) verifies its session cookies itself // against CENTRAL_AUTH_SECRET — it never calls back to central auth at request -// time. So hvac's backend, holding that same secret, can mint a valid session -// token directly. No central-auth changes needed at all; the resulting cookie -// is indistinguishable from one central auth issued, just narrower in scope. +// time. So hvac's backend, holding that same secret, can mint a valid token +// directly. No central-auth changes needed at all — this JWT is signed the +// same way a real staff session is, just carried in its own cookie (see +// KIOSK_COOKIE above) and narrower in scope (no user_id claim, so central +// auth's own /verify — which looks up by user_id — correctly rejects it if +// this token ever reached central auth by mistake; it never should, since +// requireAuth checks it directly). export async function kioskRoutes(app) { // Deliberately outside any requireAuth hook — this IS the login route. app.post('/api/kiosk/login', async (req, reply) => { @@ -65,7 +77,7 @@ export async function kioskRoutes(app) { .setExpirationTime(`${KIOSK_SESSION_SECONDS}s`) .sign(secret) - reply.setCookie('hnf_session', token, cookieOpts(req)) + reply.setCookie(KIOSK_COOKIE, token, cookieOpts(req)) return { ok: true } }) } diff --git a/docker-compose.yml b/docker-compose.yml index ebd00b8..bf707b7 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -6,10 +6,6 @@ services: environment: - DATABASE_URL=${DATABASE_URL} - CENTRAL_AUTH_SECRET=${CENTRAL_AUTH_SECRET} - # Only needed for the kiosk login (routes/kiosk.js), which mints its own - # hnf_session cookie locally — matches central auth's own cookie domain - # scoping (auth/src/routes/auth.js cookieOpts()) so the two are interchangeable. - - DOMAIN=${DOMAIN} - SETTINGS_URL=${SETTINGS_URL} - SETTINGS_SECRET=${SETTINGS_SECRET} - APP_SLUG=hvac