fix(backend): use the strong password generator for resets and enforce must_change_password

Stable backport of 9bca4046 (main) for GHSA-h4w8-57xq-53fx.

Admin password reset generated a ~2^21-entropy password from a small
wordlist (generateReadablePassword) instead of the already-available
generateSecurePassword(16), and must_change_password was written on
reset but never checked by any route-blocking logic — a reset admin
could keep using the old/weak password indefinitely since the flag
only ever reached the frontend as a response field.

adminAuth() (backend/src/middleware/auth.js) is adapted for stable's
inline admin-lookup query (main's equivalent goes through the
sessionAccess.admin() abstraction, which doesn't exist on this branch):
both the roles-join select and its missing-roles-table fallback select
now also fetch must_change_password, and a flagged admin gets 403
MUST_CHANGE_PASSWORD on every adminAuth-gated route except
/api/admin/auth/change-password and /api/admin/auth/logout (verified
against this branch's actual routes/adminAuth.js).

resetAdminPassword() (backend/src/services/userManagementService.js)
now calls generateSecurePassword(16), which already exists on stable
with the same signature as main. generateReadablePassword itself is
left untouched since it's still used by the separate gallery-password
reset path (routes/adminEvents/resets.js).

Frontend already renders MandatoryPasswordChangeModal off
user.mustChangePassword (AdminAuthContext/AdminLayout), so this is
purely a server-side backstop, same as on main.

backend/__tests__/middleware/authMustChangePassword.test.js

@@ -0,0 +1,123 @@
+/**
+ * GHSA-h4w8-57xq-53fx enforcement half: `must_change_password` was written
+ * by the admin password-reset flow (userManagementService.resetAdminPassword)
+ * and returned in a few response payloads, but no route-blocking logic ever
+ * checked it — a reset admin could keep using the old/weak password on every
+ * protected route indefinitely. adminAuth() is now the server-side backstop:
+ * a flagged admin gets 403 MUST_CHANGE_PASSWORD on everything except the
+ * routes they need to clear the flag (change-password) or leave (logout).
+ *
+ * Mirrors the mocking shape of adminAuthRoleFallback.test.js — a stub `db`
+ * chain, no real SQLite needed, so this stays a fast unit test.
+ */
+
+const jwt = require('jsonwebtoken');
+
+jest.mock('../../src/utils/tokenRevocation', () => ({ isTokenRevoked: jest.fn().mockResolvedValue(false) }));
+jest.mock('../../src/utils/logger', () => ({ warn: jest.fn(), error: jest.fn(), debug: jest.fn(), info: jest.fn() }));
+
+let mockMustChangePassword = false;
+const mockAdminRow = { id: 7, username: 'scoped', email: 's@example.com', password_changed_at: null, role_id: 1, role_name: 'editor' };
+
+jest.mock('../../src/database/db', () => ({
+  db: () => ({
+    leftJoin() { return this; },
+    where() { return this; },
+    select() { return this; },
+    first: () => Promise.resolve({ ...mockAdminRow, must_change_password: mockMustChangePassword }),
+  }),
+}));
+
+const { adminAuth } = require('../../src/middleware/auth');
+
+const SECRET = 'test-secret-for-must-change-password';
+
+function makeReq(originalUrl) {
+  const token = jwt.sign(
+    { id: mockAdminRow.id, type: 'admin' },
+    SECRET,
+    { algorithm: 'HS256', issuer: 'picpeak-auth' },
+  );
+  return { headers: { authorization: `Bearer ${token}` }, ip: '127.0.0.1', connection: {}, originalUrl };
+}
+
+function makeRes() {
+  return {
+    statusCode: null,
+    body: null,
+    status(code) { this.statusCode = code; return this; },
+    json(payload) { this.body = payload; return this; },
+  };
+}
+
+describe('adminAuth must_change_password enforcement (GHSA-h4w8-57xq-53fx)', () => {
+  const OLD_SECRET = process.env.JWT_SECRET;
+  beforeAll(() => { process.env.JWT_SECRET = SECRET; });
+  afterAll(() => { process.env.JWT_SECRET = OLD_SECRET; });
+  beforeEach(() => { mockMustChangePassword = false; });
+
+  it('blocks an arbitrary protected route with 403 MUST_CHANGE_PASSWORD when the flag is set', async () => {
+    mockMustChangePassword = true;
+    const req = makeReq('/api/admin/dashboard/stats');
+    const res = makeRes();
+    const next = jest.fn();
+
+    await adminAuth(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(res.statusCode).toBe(403);
+    expect(res.body).toEqual(expect.objectContaining({ code: 'MUST_CHANGE_PASSWORD' }));
+    expect(req.admin).toBeUndefined();
+  });
+
+  it('does not block when the flag is not set', async () => {
+    mockMustChangePassword = false;
+    const req = makeReq('/api/admin/dashboard/stats');
+    const res = makeRes();
+    const next = jest.fn();
+
+    await adminAuth(req, res, next);
+
+    expect(next).toHaveBeenCalled();
+    expect(req.admin.mustChangePassword).toBe(false);
+  });
+
+  it.each([
+    ['/api/admin/auth/change-password'],
+    ['/api/admin/auth/logout'],
+  ])('still allows %s through when the flag is set', async (originalUrl) => {
+    mockMustChangePassword = true;
+    const req = makeReq(originalUrl);
+    const res = makeRes();
+    const next = jest.fn();
+
+    await adminAuth(req, res, next);
+
+    expect(next).toHaveBeenCalled();
+    expect(req.admin.mustChangePassword).toBe(true);
+    expect(res.statusCode).toBeNull();
+  });
+
+  it('allows the exempt change-password path even with a query string', async () => {
+    mockMustChangePassword = true;
+    const req = makeReq('/api/admin/auth/change-password?foo=bar');
+    const res = makeRes();
+    const next = jest.fn();
+
+    await adminAuth(req, res, next);
+
+    expect(next).toHaveBeenCalled();
+  });
+
+  it('does not exempt a route that merely starts with the change-password path', async () => {
+    mockMustChangePassword = true;
+    const req = makeReq('/api/admin/auth/change-password-history');
+    const res = makeRes();
+    const next = jest.fn();
+
+    await adminAuth(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(res.statusCode).toBe(403);
+  });
+});

backend/__tests__/services/userManagementService.resetPassword.test.js

@@ -0,0 +1,100 @@
+/**
+ * GHSA-h4w8-57xq-53fx entropy half: resetAdminPassword used to mint the
+ * emailed temp password with generateReadablePassword() — 10 adjectives x
+ * 10 nouns x crypto.randomInt(1000,9999) x 5 specials, ~2^21 possibilities,
+ * brute-forceable. It now uses generateSecurePassword(16) (90-char charset),
+ * same as every other security-sensitive password path in this file.
+ *
+ * Verified against a real SQLite DB (full core-migration set) so the
+ * emailed plaintext, the stored hash, and must_change_password are all
+ * checked end to end rather than against a mock.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+// bootCrmDb() sets TEST_DATABASE_PATH itself, but only in time for requires
+// that happen AFTER it runs (inside beforeAll). userManagementService.js
+// requires database/db.js at module load — i.e. before beforeAll — so that
+// connection has to be pointed at a fresh, unused test DB up front, or it
+// falls back to the shared default path and collides with whatever another
+// test file already migrated onto it. Same workaround as
+// userManagementService.activateDelete.test.js.
+const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-reset-pw-test-'));
+process.env.NODE_ENV = 'test';
+process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'db.sqlite');
+process.env.STORAGE_PATH = path.join(tmpDir, 'storage');
+fs.mkdirSync(process.env.STORAGE_PATH, { recursive: true });
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'reset-pw-test-secret';
+
+const bcrypt = require('bcrypt');
+
+const { bootCrmDb, seedMinimal, assignAdminRole } = require('../integration/helpers/crmDb');
+const userManagementService = require('../../src/services/userManagementService');
+
+// The wordlist generateReadablePassword() used to produce:
+//   <Adjective><Noun><4 digits><1 special>, e.g. "SwiftEagle4821!"
+const READABLE_WORDLIST_PATTERN = /^(Swift|Bright|Strong|Happy|Clever|Brave|Noble|Quick|Sharp|Bold)(Eagle|Mountain|River|Thunder|Forest|Ocean|Falcon|Dragon|Phoenix|Tiger)\d{4}[!@#$%]$/;
+
+describe('userManagementService.resetAdminPassword (GHSA-h4w8-57xq-53fx)', () => {
+  let db;
+  let cleanup;
+  let actorId;
+  let targetId;
+
+  beforeAll(async () => {
+    ({ db, cleanup } = await bootCrmDb());
+    ({ adminId: actorId } = await seedMinimal(db));
+    await assignAdminRole(db, actorId, 'super_admin');
+
+    const editor = await db('roles').where({ name: 'editor' }).first();
+    const targetInsert = await db('admin_users').insert({
+      username: 'reset-target', email: 'reset-target@example.com',
+      password_hash: await bcrypt.hash('old-password', 4),
+      role_id: editor?.id || null,
+      is_active: 1, must_change_password: false, created_at: new Date().toISOString(),
+    }).returning('id');
+    targetId = targetInsert[0]?.id ?? targetInsert[0];
+  }, 120000);
+
+  afterAll(async () => { if (cleanup) await cleanup(); });
+
+  it('generates a high-entropy password, not one drawn from the adjective/noun wordlist', async () => {
+    const before = await db('admin_users').where({ id: targetId }).first();
+
+    await userManagementService.resetAdminPassword(targetId, actorId);
+
+    const emailRow = await db('email_queue')
+      .where({ recipient_email: 'reset-target@example.com', email_type: 'admin_password_reset' })
+      .orderBy('id', 'desc')
+      .first();
+    expect(emailRow).toBeDefined();
+    const emailData = JSON.parse(emailRow.email_data);
+    const newPassword = emailData.new_password;
+
+    // generateSecurePassword(16): fixed 16-char length, not the wordlist's
+    // variable-length "WordWord####!" shape.
+    expect(newPassword).toHaveLength(16);
+    expect(newPassword).not.toMatch(READABLE_WORDLIST_PATTERN);
+    // generateSecurePassword guarantees at least one of each character class.
+    expect(newPassword).toMatch(/[a-z]/);
+    expect(newPassword).toMatch(/[A-Z]/);
+    expect(newPassword).toMatch(/[0-9]/);
+    expect(newPassword).toMatch(/[!@#$%^&*()_+\-=[\]{}|;:,.<>?]/);
+
+    // The emailed plaintext actually matches what got persisted.
+    const after = await db('admin_users').where({ id: targetId }).first();
+    expect(after.password_hash).not.toBe(before.password_hash);
+    await expect(bcrypt.compare(newPassword, after.password_hash)).resolves.toBe(true);
+  });
+
+  it('sets must_change_password so the enforcement backstop kicks in on next login', async () => {
+    await db('admin_users').where({ id: targetId }).update({ must_change_password: false });
+
+    await userManagementService.resetAdminPassword(targetId, actorId);
+
+    const after = await db('admin_users').where({ id: targetId }).first();
+    expect(after.must_change_password === true || after.must_change_password === 1).toBe(true);
+  });
+});

backend/src/middleware/auth.js

@@ -6,6 +6,18 @@ const { isTokenRevoked } = require('../utils/tokenRevocation');
 const logger = require('../utils/logger');
 const { getAdminTokenFromRequest } = require('../utils/tokenUtils');
 
+// GHSA-h4w8-57xq-53fx: must_change_password was written on reset (and on
+// invitation paths) but nothing server-side ever checked it — a forced-reset
+// admin could keep using the old/weak password indefinitely because the flag
+// only ever reached the frontend as a response field. This is the backstop
+// for callers that skip the UI entirely. Every route gated by adminAuth() is
+// blocked except the ones a flagged admin needs to clear the flag or leave:
+// change their password, and log out.
+const MUST_CHANGE_PASSWORD_EXEMPT_PATHS = new Set([
+  '/api/admin/auth/change-password',
+  '/api/admin/auth/logout',
+]);
+
 /**
  * Enhanced admin authentication middleware with revocation checking
  */
@@ -71,6 +83,7 @@ async function adminAuth(req, res, next) {
           'admin_users.username',
           'admin_users.email',
           'admin_users.password_changed_at',
+          'admin_users.must_change_password',
           'roles.id as role_id',
           'roles.name as role_name'
         )
@@ -89,7 +102,7 @@ async function adminAuth(req, res, next) {
       logger.debug('Roles table not available, falling back to basic auth', { error: joinError.message });
       admin = await db('admin_users')
         .where({ id: decoded.id, is_active: formatBoolean(true) })
-        .select('id', 'username', 'email', 'password_changed_at')
+        .select('id', 'username', 'email', 'password_changed_at', 'must_change_password')
         .first();
       if (admin) {
         admin.role_id = null;
@@ -119,13 +132,22 @@ async function adminAuth(req, res, next) {
       }
     }
 
+    if (admin.must_change_password
+      && !MUST_CHANGE_PASSWORD_EXEMPT_PATHS.has(req.originalUrl.split('?')[0])) {
+      return res.status(403).json({
+        error: 'Password change required before continuing',
+        code: 'MUST_CHANGE_PASSWORD'
+      });
+    }
+
     // Add user info to request (enhanced with role)
     req.admin = {
       id: admin.id,
       username: admin.username,
       email: admin.email,
       roleId: admin.role_id,
-      roleName: admin.role_name
+      roleName: admin.role_name,
+      mustChangePassword: !!admin.must_change_password
     };
     req.token = token; // Store token for potential revocation
     

backend/src/services/userManagementService.js

@@ -7,7 +7,7 @@ const bcrypt = require('bcrypt');
 const crypto = require('crypto');
 const { db, logActivity } = require('../database/db');
 const { formatBoolean } = require('../utils/dbCompat');
-const { generateReadablePassword } = require('../utils/passwordGenerator');
+const { generateSecurePassword } = require('../utils/passwordGenerator');
 const { getBcryptRounds } = require('../utils/passwordValidation');
 const { queueEmail } = require('./emailProcessor');
 const logger = require('../utils/logger');
@@ -459,7 +459,10 @@ async function resetAdminPassword(id, resetById) {
     throw new NotFoundError('Admin user', id);
   }
 
-  const newPassword = generateReadablePassword();
+  // GHSA-h4w8-57xq-53fx: this password is emailed to the admin and is a live
+  // credential until they change it, so it needs real entropy — not the
+  // ~2^21 wordlist-based generateReadablePassword() used for gallery resets.
+  const newPassword = generateSecurePassword(16);
   const passwordHash = await bcrypt.hash(newPassword, getBcryptRounds());
 
   await db('admin_users').where('id', id).update({