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

Admin password reset generated a ~2^21-entropy password from a small
wordlist 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 user could keep using the old
session/password indefinitely.

backend/__tests__/middleware/authMustChangePassword.test.js

@@ -0,0 +1,124 @@
+/**
+ * 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/sessionCutoff', () => ({ isTokenBeforeCutoff: 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: '[email protected]', 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__/routes/adminUsage.test.js

@@ -64,6 +64,11 @@ beforeAll(async () => {
     t.integer('role_id');
     t.boolean('is_active');
     t.timestamp('password_changed_at');
+    // adminAuth() now selects this on every request (GHSA-h4w8-57xq-53fx
+    // must_change_password enforcement) — without the column the join
+    // throws and every route in this file 401s before reaching the
+    // permission check it's meant to test.
+    t.boolean('must_change_password');
   });
   await mockDb.schema.createTable('permissions', (t) => {
     t.increments('id');

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: '[email protected]',
+      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: '[email protected]', 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

@@ -3,6 +3,19 @@ const sessionAccess = require('../services/sessionAccessService');
 const logger = require('../utils/logger');
 const { getAdminTokenFromRequest } = require('../utils/tokenUtils');
 
+// GHSA-h4w8-57xq-53fx: must_change_password was written on reset (and on
+// invitation/OIDC-bypass 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. The
+// frontend already renders a blocking modal for it (MandatoryPasswordChangeModal),
+// 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
  */
@@ -12,7 +25,7 @@ async function adminAuth(req, res, next) {
     if (!token) {
       return res.status(401).json({ error: 'No token provided' });
     }
-    
+
     let decoded;
     try {
       decoded = jwt.verify(token, process.env.JWT_SECRET, {
@@ -27,13 +40,23 @@ async function adminAuth(req, res, next) {
       }
       return res.status(401).json({ error: 'Invalid token' });
     }
-    
+
-    const admin = await sessionAccess.admin(decoded);
+    // includeProfile: true — need must_change_password for the enforcement
+    // check below on every request, not just the profile/session-check routes.
+    const admin = await sessionAccess.admin(decoded, { includeProfile: true });
     const requestIp = req.ip || req.connection?.remoteAddress;
     if (decoded.ip && requestIp && decoded.ip !== requestIp) {
       logger.info('admin session IP changed', { accountId: admin.id, tokenIp: decoded.ip, requestIp });
     }
 
+    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,
@@ -41,6 +64,7 @@ async function adminAuth(req, res, next) {
       email: admin.email,
       roleId: admin.role_id,
       roleName: admin.role_name,
+      mustChangePassword: !!admin.must_change_password,
       // 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
@@ -48,7 +72,7 @@ async function adminAuth(req, res, next) {
       rememberMe: decoded.rememberMe === true
     };
     req.token = token; // Store token for potential revocation
-    
+
     next();
   } catch (error) {
     logger.error('Auth middleware error:', error);

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 { getAbsoluteFrontendUrl } = require('../utils/frontendUrl');
 const { queueEmail } = require('./emailProcessor');
@@ -474,7 +474,10 @@ async function resetAdminPassword(id, resetById) {
     throw new ValidationError('This account is managed by your identity provider (SSO) — reset the password there.');
   }
 
-  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({