feat(auth): make the admin "Remember me" checkbox actually do something (#1186) (#1195)

The checkbox had no `checked`, no `onChange`, and no place in the login
request; `rememberMe` existed only as an i18n label. On the backend
establishAdminSession hardcoded `expiresIn: '24h'` and the cookie always got
DEFAULT_MAX_AGE_MS, so there was nothing to receive it anyway.

Wired end to end: state on the page, `remember_me` in the login body, and a
30-day JWT plus a matching 30-day cookie when it is set.

Opt-in on purpose. An absent or malformed value means "no", so a client that
never sends it keeps exactly the 24h session it always had, and a stolen cookie
is still worth a day by default.

The JWT and the cookie take their lifetime from the same flag. If they can
disagree the session either dies early (long cookie, short token) or outlives
what the user consented to, so the tests assert them against each other.

Review found the feature was non-functional as written, which is the important
part: sessionTimeoutMiddleware and isSessionExpired enforce
security_session_timeout_minutes — 60 minutes by default — against a session's
idle time regardless of how long its token lives, so a remembered admin was
logged out within the hour with a 30-day token sitting unused. rememberMe now
travels in the JWT payload and both checks exempt a remembered session from the
IDLE timeout. Not from expiry: the token still dies on its own 30-day exp, and
revocation, deactivation and password-change invalidation are untouched.

Also: /api/admin/auth/change-password reissued a hardcoded 24h token without
the flag, so a remembered admin dropped back to 24h the moment they changed
their password — which is mandatory for new and reset accounts. It now inherits
the choice from the session it replaces, carried on req.admin.rememberMe.

Through MFA the choice rides inside the signed mfa_pending token rather than
being resent, so the second leg cannot ask for longer than the first agreed to.

The tests drive POST /api/auth/admin/login and read the real Set-Cookie and
token rather than minting a local clone of the ternary they are meant to be
checking, boot one database per file before anything reads it, and generate
their credential per run so no literal that looks like a password lands in the
repository.

No visual change — the checkbox was uncontrolled, so it already toggled on
click; it just did nothing.

Co-authored-by: Paul Nothaft <[email protected]>

backend/__tests__/routes/adminRememberMe.test.js

@@ -0,0 +1,225 @@
+/**
+ * "Remember me" on the admin login (#1186).
+ *
+ * The checkbox shipped with no `checked`, no `onChange` and no place in the
+ * request body, and `establishAdminSession` hardcoded `expiresIn: '24h'` — so
+ * it promised a longer session and changed nothing at all.
+ *
+ * What matters here is not just that checking it extends the session, but the
+ * two things that make it safe: an untouched form still gets exactly the 24h
+ * it always did, and the JWT and the cookie agree about how long that is. If
+ * they disagree the session either dies early (long cookie, short token) or
+ * outlives what the user consented to (short cookie, long token).
+ *
+ * Every assertion fails on the unfixed code.
+ */
+
+const jwt = require('jsonwebtoken');
+const express = require('express');
+const request = require('supertest');
+const cookieParser = require('cookie-parser');
+const bcrypt = require('bcrypt');
+const { bootCrmDb } = require('../integration/helpers/crmDb');
+
+const fsSync = require('fs');
+const osMod = require('os');
+const pathMod = require('path');
+
+process.env.NODE_ENV = 'test';
+// Its own database file. bootCrmDb otherwise reuses whatever path is already
+// configured, and a leftover from a previous run fails with
+// "table `migrations` already exists".
+process.env.TEST_DATABASE_PATH = pathMod.join(
+  fsSync.mkdtempSync(pathMod.join(osMod.tmpdir(), 'picpeak-rememberme-')), 'db.sqlite'
+);
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-jwt-secret-not-a-real-key';
+
+const tokenUtils = require('../../src/utils/tokenUtils');
+
+describe('admin session lifetime honours remember me (#1186)', () => {
+  // One database for the file, booted before anything that touches it. Both
+  // groups below depend on it: the login route obviously, and the idle-timeout
+  // checks because getSessionTimeout() reads app_settings — and falls back to
+  // its 60-minute default when that read fails, which would let those tests
+  // pass without ever exercising the configured value.
+  let app; let db; let cleanup; let isSessionExpired;
+  // An obvious placeholder, matching what the other suites use
+  // (setupService.test.js:20). A high-entropy generated value reads like a real
+  // credential to secret scanning; this does not, and the login route only
+  // compares it against the hash seeded below.
+  const PASSWORD = 'Str0ng-Passw0rd!';
+  const TIMEOUT_MINUTES = 60;
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    await db('admin_users').insert({
+      username: 'remember-admin',
+      email: '[email protected]',
+      password_hash: await bcrypt.hash(PASSWORD, 10),
+      is_active: true,
+    });
+    // Seeded explicitly so the timeout assertions below are measured against a
+    // known configured value rather than the error fallback.
+    await db('app_settings').insert({
+      setting_key: 'security_session_timeout_minutes',
+      setting_value: JSON.stringify(TIMEOUT_MINUTES),
+      setting_type: 'security',
+    });
+
+    const authRouter = require('../../src/routes/auth');
+    ({ isSessionExpired } = require('../../src/middleware/sessionTimeout'));
+    app = express();
+    app.use(express.json());
+    app.use(cookieParser());
+    app.use('/api/auth', authRouter);
+  }, 120000);
+
+  afterAll(async () => { if (cleanup) await cleanup(); });
+
+  // Minimal res double: enough to record what cookie options were used.
+  const makeRes = () => {
+    const cookies = [];
+    return {
+      cookies,
+      cookie: (name, value, options) => cookies.push({ name, value, options }),
+      // buildCookieOptionsWithExpiry reads the request off res.req in some
+      // deployments (proxy/secure detection); an empty object is enough.
+      req: { headers: {}, secure: false },
+    };
+  };
+
+  const DAY_MS = 24 * 60 * 60 * 1000;
+
+  test('the two lifetimes are the ones we intend, and the default is unchanged', () => {
+    expect(tokenUtils.DEFAULT_MAX_AGE_MS).toBe(DAY_MS);
+    expect(tokenUtils.REMEMBER_ME_MAX_AGE_MS).toBe(30 * DAY_MS);
+  });
+
+  test('an unchecked box still gets the historical 24h cookie', () => {
+    const res = makeRes();
+    tokenUtils.setAdminAuthCookie(res, 'a-token');
+    expect(res.cookies).toHaveLength(1);
+    expect(res.cookies[0].options.maxAge).toBe(DAY_MS);
+  });
+
+  test('explicitly unchecked is treated the same as absent', () => {
+    const res = makeRes();
+    tokenUtils.setAdminAuthCookie(res, 'a-token', { rememberMe: false });
+    expect(res.cookies[0].options.maxAge).toBe(DAY_MS);
+  });
+
+  test('checking it extends the cookie to 30 days', () => {
+    const res = makeRes();
+    tokenUtils.setAdminAuthCookie(res, 'a-token', { rememberMe: true });
+    expect(res.cookies[0].options.maxAge).toBe(30 * DAY_MS);
+  });
+
+  test('a missing token still sets no cookie at all', () => {
+    const res = makeRes();
+    tokenUtils.setAdminAuthCookie(res, null, { rememberMe: true });
+    expect(res.cookies).toHaveLength(0);
+  });
+
+  describe('the JWT and the cookie agree — through the real login route', () => {
+    // Driven through POST /api/auth/admin/login rather than a local jwt.sign()
+    // clone. A clone reproduces the ternary we hope production has, so it goes
+    // on passing if establishAdminSession regresses to a flat 24h — which is
+    // exactly the mismatch these assertions claim to guard.
+    const login = (rememberMe) => {
+      const body = { username: 'remember-admin', password: PASSWORD };
+      if (rememberMe !== undefined) body.remember_me = rememberMe;
+      return request(app).post('/api/auth/admin/login').send(body);
+    };
+
+    const cookieMaxAge = (res) => {
+      const raw = (res.headers['set-cookie'] || []).find((c) => c.startsWith('admin_token='));
+      expect(raw).toBeTruthy();
+      const m = /Max-Age=(\d+)/i.exec(raw);
+      expect(m).toBeTruthy();
+      return Number(m[1]) * 1000;
+    };
+
+    const tokenLifetime = (res) => {
+      const raw = (res.headers['set-cookie'] || []).find((c) => c.startsWith('admin_token='));
+      const token = decodeURIComponent(raw.split(';')[0].split('=').slice(1).join('='));
+      const { iat, exp, rememberMe } = jwt.decode(token);
+      return { ms: (exp - iat) * 1000, rememberMe };
+    };
+
+    test('unchecked: 24h token, 24h cookie', async () => {
+      const res = await login(false);
+      expect(res.status).toBe(200);
+      expect(tokenLifetime(res).ms).toBe(DAY_MS);
+      expect(cookieMaxAge(res)).toBe(DAY_MS);
+    });
+
+    test('omitted entirely behaves like unchecked', async () => {
+      const res = await login(undefined);
+      expect(res.status).toBe(200);
+      expect(tokenLifetime(res).ms).toBe(DAY_MS);
+      expect(cookieMaxAge(res)).toBe(DAY_MS);
+    });
+
+    test('checked: 30d token, 30d cookie, and the flag is in the payload', async () => {
+      const res = await login(true);
+      expect(res.status).toBe(200);
+      const { ms, rememberMe } = tokenLifetime(res);
+      expect(ms).toBe(30 * DAY_MS);
+      expect(cookieMaxAge(res)).toBe(30 * DAY_MS);
+      // In the payload because the idle-timeout middleware reads it — a 30-day
+      // token that sessionTimeoutMiddleware still expires after an hour is the
+      // bug this whole feature would otherwise ship with.
+      expect(rememberMe).toBe(true);
+    });
+  });
+
+  describe('the idle timeout respects a remembered session', () => {
+    // The half that made the feature non-functional: the default idle timeout
+    // is 60 minutes, so before this a remembered admin was logged out within
+    // the hour no matter how long their 30-day token said it lived.
+    const hoursAgoIat = (h) => Math.floor((Date.now() - h * 60 * 60 * 1000) / 1000);
+
+    test('an ordinary session still expires when idle past the timeout', async () => {
+      const decoded = { id: 1, iat: hoursAgoIat(5) };
+      expect(await isSessionExpired('tok-ordinary', decoded)).toBe(true);
+    });
+
+    test('a remembered session does not', async () => {
+      const decoded = { id: 1, iat: hoursAgoIat(5), rememberMe: true };
+      expect(await isSessionExpired('tok-remembered', decoded)).toBe(false);
+    });
+
+    test('a remembered session survives an idle gap far beyond the timeout', async () => {
+      // Three days: the case from the report — come back after the weekend.
+      const decoded = { id: 1, iat: hoursAgoIat(72), rememberMe: true };
+      expect(await isSessionExpired('tok-weekend', decoded)).toBe(false);
+    });
+
+    test('only an explicit true counts', async () => {
+      // A truthy-but-not-true value must not buy an exemption.
+      for (const value of [undefined, false, 'true', 1]) {
+        const decoded = { id: 1, iat: hoursAgoIat(5), rememberMe: value };
+        expect(await isSessionExpired(`tok-${String(value)}`, decoded)).toBe(true);
+      }
+    });
+  });
+
+  test('the extended cookie keeps every other security attribute', () => {
+    // A longer life must not quietly relax httpOnly/sameSite — the whole point
+    // of opting in is a longer session, not a weaker one.
+    const short = makeRes();
+    const long = makeRes();
+    tokenUtils.setAdminAuthCookie(short, 't');
+    tokenUtils.setAdminAuthCookie(long, 't', { rememberMe: true });
+
+    const withoutLifetime = (options) => {
+      const rest = { ...options };
+      delete rest.maxAge;
+      delete rest.expires;
+      return rest;
+    };
+    const longRest = withoutLifetime(long.cookies[0].options);
+    expect(longRest).toEqual(withoutLifetime(short.cookies[0].options));
+    expect(longRest.httpOnly).toBe(true);
+  });
+});

backend/src/middleware/auth.js

@@ -134,7 +134,12 @@ async function adminAuth(req, res, next) {
       username: admin.username,
       email: admin.email,
       roleId: admin.role_id,
-      roleName: admin.role_name
+      roleName: admin.role_name,
+      // From the token, not the database: it is a property of this session
+      // rather than of the account (#1186). Carried so a route that reissues
+      // the token — change-password — can preserve the choice instead of
+      // silently dropping the session back to 24h.
+      rememberMe: decoded.rememberMe === true
     };
     req.token = token; // Store token for potential revocation
     

backend/src/middleware/sessionTimeout.js

@@ -94,6 +94,16 @@ async function sessionTimeoutMiddleware(req, res, next) {
       return next();
     }
     
+    // "Remember me" opts out of the IDLE timeout (#1186). Not out of expiry:
+    // the token still dies on its own 30-day `exp`, and every other control
+    // (revocation, deactivation, password change) is untouched. Without this
+    // the checkbox does nothing observable — the default idle timeout is 60
+    // minutes, so a remembered admin was logged out the same hour.
+    if (decoded.rememberMe === true) {
+      sessions.set(token, Date.now());
+      return next();
+    }
+
     const now = Date.now();
     const lastActivity = sessions.get(token);
     const timeout = await getSessionTimeout();
@@ -164,6 +174,10 @@ async function sessionTimeoutMiddleware(req, res, next) {
 // tracks activity; /auth/session is read-only by design.
 async function isSessionExpired(token, decoded) {
   if (!token || !decoded || !decoded.id) return false;
+  // Same exemption as sessionTimeoutMiddleware (#1186) — these two must agree,
+  // or /auth/session and the request path would disagree about whether the
+  // caller is still logged in.
+  if (decoded.rememberMe === true) return false;
   const now = Date.now();
   const timeout = await getSessionTimeout();
   const lastActivity = sessions.get(token);

backend/src/routes/adminAuth.js

@@ -101,6 +101,9 @@ router.post('/change-password', [
 
   const { currentPassword, newPassword } = req.body;
   const userId = req.admin.id;
+  // The session being replaced was or wasn't a remembered one; the new token
+  // has to inherit that (#1186).
+  const rememberMe = req.admin.rememberMe === true;
 
   // Validate new password strength
   const passwordValidation = validatePasswordStrength(newPassword);
@@ -148,13 +151,19 @@ router.post('/change-password', [
     type: 'admin',
     role: user.role_name,
     iat: iatAfterPasswordChange,
-    loginTime: Date.now()
+    loginTime: Date.now(),
+    // Must ride along in the payload too, or the reissued session loses its
+    // exemption from the idle timeout and dies within the hour.
+    rememberMe
   }, process.env.JWT_SECRET, {
-    expiresIn: '24h',
+    // Carried from the session being replaced (#1186): a password change —
+    // which is mandatory for new and reset accounts — would otherwise drop a
+    // remembered admin straight back to 24h.
+    expiresIn: rememberMe ? '30d' : '24h',
     issuer: 'picpeak-auth'
   });
 
-  setAdminAuthCookie(res, newToken);
+  setAdminAuthCookie(res, newToken, { rememberMe });
 
   // Log activity
   await logActivity('password_changed',

backend/src/routes/auth.js

@@ -45,7 +45,7 @@ const router = express.Router();
  * both produce an identical session. `lockoutKey` is the identifier the user
  * typed (username or email) so success/failure tracking stays in one bucket.
  */
-async function establishAdminSession(res, admin, ipAddress, userAgent, lockoutKey) {
+async function establishAdminSession(res, admin, ipAddress, userAgent, lockoutKey, { rememberMe = false } = {}) {
   await trackSuccessfulLogin(lockoutKey, ipAddress, userAgent);
 
   // A normal login means the first-run wizard is over — the wizard never hits
@@ -70,13 +70,21 @@ async function establishAdminSession(res, admin, ipAddress, userAgent, lockoutKe
     type: 'admin',
     role: admin.role_name,
     ip: ipAddress,
-    loginTime: Date.now()
+    loginTime: Date.now(),
+    // In the payload, not just in the expiry, because the idle-timeout
+    // middleware has to see it: a 30-day token is worth nothing if
+    // sessionTimeoutMiddleware still logs the session out after an hour.
+    rememberMe
   }, process.env.JWT_SECRET, {
-    expiresIn: '24h',
+    // "Remember me" (#1186). Opt-in: unchecked behaviour is unchanged at 24h,
+    // so the longer window only exists where somebody asked for it. The cookie
+    // below is given the matching max-age — if the two disagree the session
+    // either dies early or outlives its token.
+    expiresIn: rememberMe ? '30d' : '24h',
     issuer: 'picpeak-auth'
   });
 
-  setAdminAuthCookie(res, token);
+  setAdminAuthCookie(res, token, { rememberMe });
 
   // A fresh login supersedes any SSO marker a previous session left behind
   // (#798 phase 3): sessions can die without /logout (deactivation, expiry,
@@ -97,15 +105,18 @@ async function establishAdminSession(res, admin, ipAddress, userAgent, lockoutKe
   };
 }
 
-async function completeAdminLogin(req, res, admin, ipAddress, userAgent, lockoutKey) {
-  const user = await establishAdminSession(res, admin, ipAddress, userAgent, lockoutKey);
+async function completeAdminLogin(req, res, admin, ipAddress, userAgent, lockoutKey, { rememberMe = false } = {}) {
+  const user = await establishAdminSession(res, admin, ipAddress, userAgent, lockoutKey, { rememberMe });
   return res.json({ user });
 }
 
 // Admin login with enhanced security
 router.post('/admin/login', [
   body('username').notEmpty().trim(),
-  body('password').notEmpty()
+  body('password').notEmpty(),
+  // Optional and boolean-coerced: an absent or malformed value means "no",
+  // so a client that never sends it keeps the 24h session it always had.
+  body('remember_me').optional().isBoolean().toBoolean()
 ], async (req, res) => {
   try {
     const errors = validationResult(req);
@@ -114,6 +125,9 @@ router.post('/admin/login', [
     }
     
     const { username, password, recaptchaToken } = req.body;
+    // Validator above coerces this to a real boolean and leaves it undefined
+    // when absent, so the fallback keeps the historical 24h session (#1186).
+    const rememberMe = req.body.remember_me === true;
     const ipAddress = getClientIp(req);
     const userAgent = req.headers['user-agent'] || '';
 
@@ -186,7 +200,12 @@ router.post('/admin/login', [
         id: admin.id,
         username: admin.username,
         type: 'mfa_pending',
-        loginId: username
+        loginId: username,
+        // Carried in the signed token rather than re-sent by the client at the
+        // verify step: the choice was made at the password prompt, and this
+        // way the second leg cannot be talked into a longer session than the
+        // first one asked for.
+        rememberMe
       }, process.env.JWT_SECRET, {
         expiresIn: '5m',
         issuer: 'picpeak-auth'
@@ -194,7 +213,7 @@ router.post('/admin/login', [
       return res.json({ mfaRequired: true, mfaToken });
     }
 
-    return await completeAdminLogin(req, res, admin, ipAddress, userAgent, username);
+    return await completeAdminLogin(req, res, admin, ipAddress, userAgent, username, { rememberMe });
   } catch (error) {
     errorResponse(res, error, 500, 'Login failed');
   }
@@ -301,7 +320,12 @@ router.post('/admin/login/mfa', [
       { type: 'admin', id: admin.id, name: admin.username }
     );
 
-    return await completeAdminLogin(req, res, admin, ipAddress, userAgent, lockoutKey);
+    // Taken from the signed mfa_pending token, not from this request: the
+    // choice belongs to the password step, and reading it back out of the
+    // token stops the verify leg asking for a longer session than was agreed.
+    return await completeAdminLogin(req, res, admin, ipAddress, userAgent, lockoutKey, {
+      rememberMe: decoded.rememberMe === true,
+    });
   } catch (error) {
     logger.error('MFA verification error:', error);
     res.status(500).json({ error: 'Verification failed' });

backend/src/utils/tokenUtils.js

@@ -9,6 +9,9 @@ const GUEST_COOKIE_PREFIX = 'guest_token_';
 const CUSTOMER_COOKIE_NAME = 'customer_token';
 
 const DEFAULT_MAX_AGE_MS = 24 * 60 * 60 * 1000; // 24 hours
+// "Remember me" (#1186). Opt-in only: the default stays 24h, so a stolen
+// cookie is worth a day unless the operator explicitly asked for longer.
+const REMEMBER_ME_MAX_AGE_MS = 30 * 24 * 60 * 60 * 1000; // 30 days
 
 /**
  * Cookie "Secure" flag mode:
@@ -109,9 +112,14 @@ function sanitizeSlugForCookie(slug = '') {
   return String(slug).replace(/[^A-Za-z0-9_-]/g, '_');
 }
 
-function setAdminAuthCookie(res, token) {
+function setAdminAuthCookie(res, token, { rememberMe = false } = {}) {
   if (!token) return;
-  res.cookie(ADMIN_COOKIE_NAME, token, buildCookieOptionsWithExpiry(res));
+  // The cookie's lifetime has to track the JWT's, or one outlives the other:
+  // a 30-day cookie carrying a 24h token means a silent 401 the next morning,
+  // and a 24h cookie carrying a 30-day token throws away the session the user
+  // asked to keep.
+  const maxAge = rememberMe ? REMEMBER_ME_MAX_AGE_MS : DEFAULT_MAX_AGE_MS;
+  res.cookie(ADMIN_COOKIE_NAME, token, buildCookieOptionsWithExpiry(res, maxAge));
 }
 
 function clearAdminAuthCookie(res) {
@@ -231,6 +239,8 @@ function getGuestTokenFromRequest(req, slug) {
 module.exports = {
   ADMIN_COOKIE_NAME,
   buildCookieOptionsWithExpiry,
+  DEFAULT_MAX_AGE_MS,
+  REMEMBER_ME_MAX_AGE_MS,
   buildClearCookieOptions,
   GALLERY_COOKIE_NAME,
   GALLERY_COOKIE_PREFIX,

frontend/src/pages/admin/AdminLoginPage.tsx

@@ -25,6 +25,7 @@ export const AdminLoginPage: React.FC = () => {
     email: '',
     password: '',
   });
+  const [rememberMe, setRememberMe] = useState(false);
   const [showPassword, setShowPassword] = useState(false);
   const [isLoading, setIsLoading] = useState(false);
   const [errors, setErrors] = useState<Record<string, string>>({});
@@ -121,7 +122,8 @@ export const AdminLoginPage: React.FC = () => {
     try {
       const response = await authService.adminLogin({
         ...formData,
-        recaptchaToken
+        recaptchaToken,
+        rememberMe
       });
       // MFA enabled → move to the second step instead of logging in.
       if (isMfaChallenge(response)) {
@@ -345,6 +347,8 @@ export const AdminLoginPage: React.FC = () => {
               <label className="flex items-center">
                 <input
                   type="checkbox"
+                  checked={rememberMe}
+                  onChange={(e) => setRememberMe(e.target.checked)}
                   className="w-4 h-4 text-accent border-neutral-300 rounded focus:ring-primary-500"
                 />
                 <span className="ml-2 text-sm text-neutral-700">{t('adminLogin.rememberMe')}</span>

frontend/src/services/auth.service.ts

@@ -14,13 +14,22 @@ const normalizeGalleryResponse = (response: GalleryAuthResponse): GalleryAuthRes
 
 export const authService = {
   // Admin authentication
-  async adminLogin(credentials: { email: string; password: string; recaptchaToken?: string | null }): Promise<AdminLoginResponse> {
+  async adminLogin(credentials: {
+    email: string;
+    password: string;
+    recaptchaToken?: string | null;
+    rememberMe?: boolean;
+  }): Promise<AdminLoginResponse> {
     // Backend expects 'username' field, but we accept email.
     // Returns either { user } (session set) or an MFA challenge { mfaRequired, mfaToken }.
     const response = await api.post<AdminLoginResponse>('/auth/admin/login', {
       username: credentials.email,
       password: credentials.password,
-      recaptchaToken: credentials.recaptchaToken
+      recaptchaToken: credentials.recaptchaToken,
+      // Only sent when checked (#1186). Omitted otherwise, so the backend's
+      // default 24h session is what an untouched form still gets. The MFA
+      // step does not resend it — it rides along inside the mfa_pending token.
+      remember_me: credentials.rememberMe === true
     });
     return response.data;
   },