From f905f7e7336c756e73a8c650c9239b171697164a Mon Sep 17 00:00:00 2001
From: Paul Nothaft <paul@MacStudio-von-Paul.local>
Date: Sat, 2 May 2026 22:42:59 +0200
Subject: [PATCH] fix(auth): /auth/session must reject tokens that
 adminAuth/galleryAuth would reject
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second loop fix in the same /admin/login → /admin/dashboard → /admin/login
pattern as #355. The frontend trusts /auth/session as the source of
truth for "is the user authenticated?". When that endpoint is more
lenient than the protected middleware, every admin endpoint 401s
right after /auth/session said valid:true, the response interceptor
hard-redirects to /admin/login, /auth/session says valid again, and
the cycle closes — exactly the loop reported on v3.32.4-beta.0.

#355 fixed the issuer-claim asymmetry. This commit fixes the
remaining asymmetries: /auth/session was missing the admin-existence,
admin-active, password-change-after-iat, and gallery-existence /
gallery-archived / gallery-expired checks that adminAuth and
galleryAuth perform on every protected request.

The fix is to mirror those checks in /auth/session, scoped by token
type, and degrade gracefully when the underlying tables aren't
present (test fixtures, early bootstrap) so the endpoint never
fails-closed because of a missing table.

Reproducer that the new test covers:
  1. Admin logs in (token issued at T).
  2. Admin (or another admin) changes their own password at T+1.
  3. Browser still has the cookie from T.
  4. /auth/session says valid:true (no password-change check).
  5. /admin/dashboard fires queries; adminAuth rejects with
     PASSWORD_CHANGED 401.
  6. Frontend redirects to /admin/login.
  7. /auth/session says valid:true again. → loop.

Other surfaces this also covers:
  - admin user deactivated (admin_users.is_active = false)
  - admin user deleted
  - gallery token whose event is archived
  - gallery token whose event has expired

Tests live in __tests__/routes/authSession.symmetry.test.js — 9 cases,
mocking db / tokenRevocation / tokenUtils / recaptcha / sessionTimeout
so the suite runs without a real database.
---
 .../routes/authSession.symmetry.test.js       | 302 ++++++++++++++++++
 backend/src/routes/auth.js                    |  59 +++-
 2 files changed, 360 insertions(+), 1 deletion(-)
 create mode 100644 backend/__tests__/routes/authSession.symmetry.test.js

diff --git a/backend/__tests__/routes/authSession.symmetry.test.js b/backend/__tests__/routes/authSession.symmetry.test.js
new file mode 100644
index 00000000..b4137a8e
--- /dev/null
+++ b/backend/__tests__/routes/authSession.symmetry.test.js
@@ -0,0 +1,302 @@
+/**
+ * Regression test for the /admin/login → /admin/dashboard → /admin/login
+ * redirect loop reported on v3.32.4-beta.0.
+ *
+ * Cause: GET /auth/session was less strict than the adminAuth middleware.
+ * The session endpoint accepted tokens that the protected endpoints
+ * subsequently rejected with 401, which the frontend's interceptor
+ * translated into a hard redirect to /admin/login. /auth/session then
+ * said "valid: true" again on the next page load and the cycle closed.
+ *
+ * /auth/session must reject the same admin tokens adminAuth would
+ * reject, specifically: deactivated admin user, deleted admin user,
+ * password changed since iat. Same for gallery: archived event.
+ */
+
+const express = require('express');
+const request = require('supertest');
+const jwt = require('jsonwebtoken');
+
+process.env.JWT_SECRET = 'session-symmetry-test-secret';
+
+const fakeDb = {
+  adminUsers: [],
+  events: [],
+  revokedTokens: [],
+};
+
+jest.mock('../../src/database/db', () => {
+  const formatBoolean = (v) => (v ? 1 : 0);
+  void formatBoolean;
+  function dbFn(table) {
+    if (table === 'admin_users') {
+      let rowFilter = () => true;
+      return {
+        where(criteria) {
+          rowFilter = (row) => {
+            return Object.entries(criteria).every(([k, v]) => {
+              if (k === 'is_active') return Boolean(row.is_active) === Boolean(v);
+              return row[k] === v;
+            });
+          };
+          return this;
+        },
+        select(...cols) {
+          this._cols = cols;
+          return this;
+        },
+        async first() {
+          const row = fakeDb.adminUsers.find(rowFilter);
+          if (!row) return undefined;
+          if (!this._cols) return row;
+          const out = {};
+          for (const c of this._cols) out[c] = row[c];
+          return out;
+        },
+      };
+    }
+    if (table === 'events') {
+      let rowFilter = () => true;
+      return {
+        where(criteria) {
+          rowFilter = (row) =>
+            Object.entries(criteria).every(([k, v]) => {
+              if (k === 'is_active') return Boolean(row.is_active) === Boolean(v);
+              if (k === 'is_archived') return Boolean(row.is_archived) === Boolean(v);
+              return row[k] === v;
+            });
+          return this;
+        },
+        async first() {
+          return fakeDb.events.find(rowFilter);
+        },
+      };
+    }
+    throw new Error(`Unexpected table: ${table}`);
+  }
+  return { db: dbFn, formatBoolean: () => 1 };
+});
+
+jest.mock('../../src/utils/dbCompat', () => ({
+  formatBoolean: (v) => (v ? 1 : 0),
+}));
+
+jest.mock('../../src/utils/tokenRevocation', () => ({
+  isTokenRevoked: jest.fn(async (decoded) => fakeDb.revokedTokens.includes(decoded.id)),
+  revokeToken: jest.fn(),
+}));
+
+jest.mock('../../src/utils/tokenUtils', () => ({
+  getAdminTokenFromRequest: (req) => {
+    const auth = req.headers.authorization;
+    if (auth && auth.startsWith('Bearer ')) return auth.slice(7);
+    return null;
+  },
+  getGalleryTokenFromRequest: () => null,
+  setAdminAuthCookie: jest.fn(),
+  setGalleryAuthCookies: jest.fn(),
+  clearAdminAuthCookie: jest.fn(),
+  clearGalleryAuthCookies: jest.fn(),
+  buildCookieOptionsWithExpiry: () => ({}),
+}));
+
+jest.mock('../../src/services/recaptcha', () => ({ verifyRecaptcha: () => Promise.resolve(true) }));
+jest.mock('../../src/middleware/sessionTimeout', () => ({ endSession: jest.fn() }));
+jest.mock('../../src/utils/logger', () => ({
+  info: jest.fn(),
+  warn: jest.fn(),
+  error: jest.fn(),
+  debug: jest.fn(),
+}));
+
+const authRouter = require('../../src/routes/auth');
+
+function makeApp() {
+  const app = express();
+  app.use(express.json());
+  app.use('/auth', authRouter);
+  return app;
+}
+
+function signAdminToken({ id = 1, username = 'admin', iat, exp }) {
+  const issuedAt = iat ?? Math.floor(Date.now() / 1000);
+  // Note: do NOT pass noTimestamp:true here — that strips iat from the
+  // payload entirely, defeating the password-change comparison. Provide
+  // iat (and exp) via the payload directly instead.
+  return jwt.sign(
+    { id, username, type: 'admin', iat: issuedAt, exp: exp ?? issuedAt + 3600 },
+    process.env.JWT_SECRET,
+    { issuer: 'picpeak-auth' }
+  );
+}
+
+function signGalleryToken({ eventId = 100, eventSlug = 'wedding' } = {}) {
+  return jwt.sign(
+    { eventId, eventSlug, type: 'gallery' },
+    process.env.JWT_SECRET,
+    { expiresIn: '1h', issuer: 'picpeak-auth' }
+  );
+}
+
+describe('GET /auth/session — symmetry with protected middleware', () => {
+  beforeEach(() => {
+    fakeDb.adminUsers = [];
+    fakeDb.events = [];
+    fakeDb.revokedTokens = [];
+  });
+
+  it('returns valid:true for an active admin token', async () => {
+    fakeDb.adminUsers.push({
+      id: 1,
+      username: 'admin',
+      email: 'a@b.com',
+      is_active: true,
+      password_changed_at: null,
+    });
+    const token = signAdminToken({ id: 1 });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(true);
+    expect(res.body.type).toBe('admin');
+  });
+
+  it('returns valid:false when the admin user has been deactivated', async () => {
+    fakeDb.adminUsers.push({
+      id: 1,
+      username: 'admin',
+      email: 'a@b.com',
+      is_active: false,
+      password_changed_at: null,
+    });
+    const token = signAdminToken({ id: 1 });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(false);
+  });
+
+  it('returns valid:false when the admin user no longer exists', async () => {
+    // adminUsers is empty
+    const token = signAdminToken({ id: 999 });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(false);
+  });
+
+  it('returns valid:false when password was changed after the token was issued', async () => {
+    // iat must be in the past, exp must be in the future so jwt.verify
+    // doesn't reject the token before /auth/session even gets to look
+    // at password_changed_at.
+    const tokenIssuedAt = Math.floor(Date.now() / 1000) - 60; // 1 min ago
+    const tokenExp = tokenIssuedAt + 86400;
+    fakeDb.adminUsers.push({
+      id: 1,
+      username: 'admin',
+      email: 'a@b.com',
+      is_active: true,
+      password_changed_at: new Date((tokenIssuedAt + 30) * 1000), // 30s after iat
+    });
+    const token = signAdminToken({ id: 1, iat: tokenIssuedAt, exp: tokenExp });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(false);
+  });
+
+  it('returns valid:true when password was changed BEFORE the token was issued', async () => {
+    const tokenIssuedAt = Math.floor(Date.now() / 1000) - 60;
+    const tokenExp = tokenIssuedAt + 86400;
+    fakeDb.adminUsers.push({
+      id: 1,
+      username: 'admin',
+      email: 'a@b.com',
+      is_active: true,
+      password_changed_at: new Date((tokenIssuedAt - 3600) * 1000), // 1h before iat
+    });
+    const token = signAdminToken({ id: 1, iat: tokenIssuedAt, exp: tokenExp });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(true);
+  });
+
+  it('returns valid:false for a gallery token whose event is archived', async () => {
+    fakeDb.events.push({
+      id: 100,
+      slug: 'wedding',
+      is_active: true,
+      is_archived: true,
+      expires_at: null,
+    });
+    const token = signGalleryToken();
+
+    const res = await request(makeApp())
+      .get('/auth/session?slug=wedding')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(false);
+  });
+
+  it('returns valid:false for a gallery token whose event is expired', async () => {
+    fakeDb.events.push({
+      id: 100,
+      slug: 'wedding',
+      is_active: true,
+      is_archived: false,
+      expires_at: new Date(Date.now() - 86400_000),
+    });
+    const token = signGalleryToken();
+
+    const res = await request(makeApp())
+      .get('/auth/session?slug=wedding')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(false);
+  });
+
+  it('returns valid:true for an active gallery token', async () => {
+    fakeDb.events.push({
+      id: 100,
+      slug: 'wedding',
+      is_active: true,
+      is_archived: false,
+      expires_at: new Date(Date.now() + 86400_000),
+    });
+    const token = signGalleryToken();
+
+    const res = await request(makeApp())
+      .get('/auth/session?slug=wedding')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.valid).toBe(true);
+  });
+
+  it('returns valid:false when the token is revoked', async () => {
+    fakeDb.adminUsers.push({
+      id: 1,
+      username: 'admin',
+      is_active: true,
+      password_changed_at: null,
+    });
+    fakeDb.revokedTokens.push(1);
+    const token = signAdminToken({ id: 1 });
+
+    const res = await request(makeApp())
+      .get('/auth/session')
+      .set('Authorization', `Bearer ${token}`);
+    expect(res.status).toBe(401);
+    expect(res.body.valid).toBe(false);
+  });
+});
diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js
index aab00b23..219d7ad7 100644
--- a/backend/src/routes/auth.js
+++ b/backend/src/routes/auth.js
@@ -477,7 +477,7 @@ router.get('/session', async (req, res) => {
   try {
     const { slug } = req.query;
     const token = getAdminTokenFromRequest(req) || getGalleryTokenFromRequest(req, slug);
-    
+
     if (!token) {
       return res.status(401).json({ error: 'No token provided' });
     }
@@ -498,6 +498,63 @@ router.get('/session', async (req, res) => {
         return res.status(401).json({ valid: false, error: 'Session has been invalidated' });
       }
 
+      // The redirect loop reported on the v3.32.4-beta.0 release came
+      // from /auth/session reporting valid: true while the protected
+      // adminAuth / galleryAuth middleware rejected the same token for
+      // reasons /auth/session never checked: the admin user was
+      // deactivated, the admin's password had been changed since iat,
+      // or the gallery event was archived/deleted. Mirror those checks
+      // here so the session endpoint is always at least as strict as
+      // what the protected endpoints will enforce next.
+      if (decoded.type === 'admin') {
+        let admin = null;
+        try {
+          admin = await db('admin_users')
+            .where({ id: decoded.id, is_active: formatBoolean(true) })
+            .select('id', 'username', 'email', 'password_changed_at')
+            .first();
+        } catch (lookupErr) {
+          // admin_users table not present (test fixture, fresh DB) — fall
+          // through and trust the token. Real deployments always have it.
+          admin = null;
+          // intentional swallow; if the table is missing we do not want
+          // to fail-closed during e.g. early bootstrap.
+        }
+
+        if (admin === null) {
+          // Lookup didn't run because the table is missing; skip the
+          // existence/password checks and treat the token as valid.
+        } else if (!admin) {
+          return res.json({ valid: false, error: 'Admin account no longer active' });
+        } else if (admin.password_changed_at) {
+          const passwordChangedSeconds = Math.floor(
+            new Date(admin.password_changed_at).getTime() / 1000
+          );
+          if (decoded.iat < passwordChangedSeconds) {
+            return res.json({ valid: false, error: 'Token invalid due to password change' });
+          }
+        }
+      } else if (decoded.type === 'gallery') {
+        try {
+          const event = await db('events')
+            .where({
+              id: decoded.eventId,
+              is_active: formatBoolean(true),
+              is_archived: formatBoolean(false),
+            })
+            .first();
+          if (!event) {
+            return res.json({ valid: false, error: 'Gallery no longer available' });
+          }
+          if (event.expires_at && new Date(event.expires_at) < new Date()) {
+            return res.json({ valid: false, error: 'Gallery has expired' });
+          }
+        } catch (galleryLookupErr) {
+          // events table missing in this context — same fallback as
+          // admin path; trust the token rather than fail-closed.
+        }
+      }
+
       // Calculate remaining time
       const now = Date.now() / 1000;
       const remainingTime = Math.max(0, decoded.exp - now);