Merge remote-tracking branch 'origin/main' into refactor/codebase-cleanup

# Conflicts:
#	backend/src/routes/adminEvents.js
#	backend/src/routes/protectedImages.js
#	frontend/src/pages/admin/EventDetailsPage.tsx

backend/__tests__/integration/picpeakExport.test.js

@@ -0,0 +1,94 @@
+'use strict';
+
+// Validates the engine-neutral .picpeak export: it must produce a real zip with
+// a manifest + per-table NDJSON, exclude knex bookkeeping, and honour the photo
+// toggle. Uses the shared CRM DB harness (temp SQLite) — no docker needed.
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret-at-least-32-characters-long!!';
+
+const fs = require('fs');
+const path = require('path');
+const StreamZip = require('node-stream-zip');
+const { bootCrmDb } = require('./helpers/crmDb');
+
+let db;
+let cleanup;
+let tmpDir;
+let createPicpeak;
+
+// bootCrmDb MUST run before requiring the service (which transitively requires
+// db.js) so the export reads this test's DB, not the default path.
+beforeAll(async () => {
+  ({ db, cleanup, tmpDir } = await bootCrmDb());
+  process.env.STORAGE_PATH = tmpDir; // isolate file collection to the temp dir
+  ({ createPicpeak } = require('../../src/services/picpeakExportService'));
+}, 60000);
+
+afterAll(async () => {
+  await cleanup();
+});
+
+async function readZip(filePath) {
+  const zip = new StreamZip.async({ file: filePath });
+  const entries = Object.keys(await zip.entries());
+  const manifest = JSON.parse((await zip.entryData('manifest.json')).toString('utf8'));
+  await zip.close();
+  return { entries, manifest };
+}
+
+describe('picpeak export (.picpeak logical export)', () => {
+  it('produces a .picpeak with a manifest and per-table NDJSON', async () => {
+    const { filePath, manifest } = await createPicpeak({ includePhotos: false });
+    try {
+      expect(filePath.endsWith('.picpeak')).toBe(true);
+      expect(fs.existsSync(filePath)).toBe(true);
+
+      expect(manifest.format).toBe(1);
+      expect(manifest.kind).toBe('picpeak-backup');
+      expect(manifest.database.engine).toBe('sqlite');
+      expect(manifest.options.includePhotos).toBe(false);
+      expect(manifest.contains_secrets).toBe(true);
+      // Migrations seed real tables (e.g. app_settings) — expect several.
+      expect(Object.keys(manifest.tables).length).toBeGreaterThan(0);
+      expect(Object.keys(manifest.tables)).toContain('app_settings');
+
+      const { entries, manifest: zipped } = await readZip(filePath);
+      expect(entries).toContain('manifest.json');
+      expect(entries.some((n) => n.startsWith('data/') && n.endsWith('.ndjson'))).toBe(true);
+      expect(entries).toContain('data/app_settings.ndjson');
+      // Manifest inside the zip matches the returned one.
+      expect(zipped.tables).toEqual(manifest.tables);
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+    }
+  });
+
+  it('never exports knex bookkeeping tables', async () => {
+    const { filePath, manifest } = await createPicpeak({ includePhotos: false });
+    try {
+      const names = Object.keys(manifest.tables);
+      expect(names).not.toContain('knex_migrations');
+      expect(names).not.toContain('knex_migrations_lock');
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+    }
+  });
+
+  it('row counts in the manifest match the NDJSON line counts', async () => {
+    // Insert a couple of settings so at least one table is non-empty.
+    await db('app_settings')
+      .insert({ setting_key: 'picpeak_export_test_a', setting_value: JSON.stringify('1'), setting_type: 'string' })
+      .onConflict('setting_key').merge();
+
+    const { filePath, manifest } = await createPicpeak({ includePhotos: false });
+    try {
+      const zip = new StreamZip.async({ file: filePath });
+      const buf = await zip.entryData('data/app_settings.ndjson');
+      await zip.close();
+      const lines = buf.toString('utf8').split('\n').filter((l) => l.trim().length > 0);
+      expect(lines.length).toBe(manifest.tables.app_settings.rowCount);
+      expect(manifest.tables.app_settings.rowCount).toBeGreaterThan(0);
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+    }
+  });
+});

backend/__tests__/integration/picpeakRoundtrip.test.js

@@ -0,0 +1,180 @@
+'use strict';
+
+// Full .picpeak roundtrip on a temp SQLite DB:
+//   1. seed a "backup" instance (admin A + a marker setting)
+//   2. export → .picpeak
+//   3. simulate a reinstall: wipe, create a DIFFERENT current admin B, mutate data
+//   4. import the backup with currentAdminId = B
+//   5. assert the backup data is restored AND the current account (B) survives,
+//      while the backup's admin (A) is also present (different email → added).
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'test-secret-at-least-32-characters-long!!';
+
+const fs = require('fs');
+const path = require('path');
+const { bootCrmDb } = require('./helpers/crmDb');
+
+let db;
+let cleanup;
+let tmpDir;
+let createPicpeak;
+let importFromPicpeak;
+let validateManifest;
+let superAdminRoleId;
+
+beforeAll(async () => {
+  ({ db, cleanup, tmpDir } = await bootCrmDb());
+  process.env.STORAGE_PATH = tmpDir;
+  ({ createPicpeak } = require('../../src/services/picpeakExportService'));
+  ({ importFromPicpeak, validateManifest } = require('../../src/services/picpeakImportService'));
+  const role = await db('roles').where({ name: 'super_admin' }).first();
+  superAdminRoleId = role.id;
+}, 60000);
+
+afterAll(async () => {
+  await cleanup();
+});
+
+const adminRow = (email, hash) => ({
+  username: email,
+  email,
+  password_hash: hash,
+  role_id: superAdminRoleId,
+  is_active: true,
+  must_change_password: false,
+  created_at: new Date(),
+  updated_at: new Date(),
+});
+
+async function setMarker(value) {
+  await db('app_settings')
+    .insert({ setting_key: 'roundtrip_marker', setting_value: JSON.stringify(value), setting_type: 'string' })
+    .onConflict('setting_key').merge();
+}
+async function getMarker() {
+  const row = await db('app_settings').where({ setting_key: 'roundtrip_marker' }).first();
+  return row ? JSON.parse(row.setting_value) : null;
+}
+
+describe('.picpeak roundtrip (export → import)', () => {
+  it('restores backup data and preserves the current account', async () => {
+    // 1. Seed the "source" instance.
+    await db('admin_users').del();
+    await db('admin_users').insert(adminRow('[email protected]', 'HASH_A'));
+    await setMarker('from_backup');
+
+    // 2. Export.
+    const { filePath } = await createPicpeak({ includePhotos: false });
+
+    try {
+      // 3. Simulate a reinstall: fresh current admin B, mutated data.
+      await db('admin_users').del();
+      const [bId] = await db('admin_users').insert(adminRow('[email protected]', 'HASH_B')).returning('id');
+      const currentAdminId = typeof bId === 'object' ? bId.id : bId;
+      await setMarker('mutated_after_backup');
+
+      // 4. Import, preserving the current admin.
+      const result = await importFromPicpeak({ filePath: undefined, picpeakPath: filePath, currentAdminId });
+      expect(result.restored).toBe(true);
+      expect(result.tables).toBeGreaterThan(0);
+
+      // 5a. Backup data restored (marker reverted to the backup value).
+      expect(await getMarker()).toBe('from_backup');
+
+      // 5b. The backup's admin is present (different email → added).
+      const a = await db('admin_users').whereRaw('lower(email) = lower(?)', ['[email protected]']).first();
+      expect(a).toBeTruthy();
+      expect(a.password_hash).toBe('HASH_A');
+
+      // 5c. The current account SURVIVES the override, with its own credentials.
+      const b = await db('admin_users').whereRaw('lower(email) = lower(?)', ['[email protected]']).first();
+      expect(b).toBeTruthy();
+      expect(b.password_hash).toBe('HASH_B');
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+    }
+  });
+
+  it('overwrites a backup admin that collides with the current account email', async () => {
+    // Source has an admin at the SAME email the current operator will use.
+    await db('admin_users').del();
+    await db('admin_users').insert(adminRow('[email protected]', 'OLD_HASH'));
+    await setMarker('collision_case');
+    const { filePath } = await createPicpeak({ includePhotos: false });
+
+    try {
+      // Reinstall: current admin uses the same email but a NEW password.
+      await db('admin_users').del();
+      const [id] = await db('admin_users').insert(adminRow('[email protected]', 'NEW_HASH')).returning('id');
+      const currentAdminId = typeof id === 'object' ? id.id : id;
+
+      await importFromPicpeak({ picpeakPath: filePath, currentAdminId });
+
+      // Exactly one admin at that email, and it keeps the CURRENT password.
+      const rows = await db('admin_users').whereRaw('lower(email) = lower(?)', ['[email protected]']);
+      expect(rows).toHaveLength(1);
+      expect(rows[0].password_hash).toBe('NEW_HASH');
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+    }
+  });
+
+  it('restores files/ and reports filesRestored', async () => {
+    // A business-doc that lives in storage → travels in the backup.
+    const docDir = path.join(tmpDir, 'business-docs');
+    const marker = path.join(docDir, 'roundtrip-doc.txt');
+    fs.mkdirSync(docDir, { recursive: true });
+    fs.writeFileSync(marker, 'hello');
+    await db('admin_users').del();
+    const [id] = await db('admin_users').insert(adminRow('[email protected]', 'H')).returning('id');
+    const currentAdminId = typeof id === 'object' ? id.id : id;
+
+    const { filePath } = await createPicpeak({ includePhotos: false });
+    try {
+      fs.rmSync(marker); // delete on disk so the restore must bring it back
+      const result = await importFromPicpeak({ picpeakPath: filePath, currentAdminId });
+      expect(result.filesRestored).toBeGreaterThanOrEqual(1);
+      expect(fs.existsSync(marker)).toBe(true);
+      expect(fs.readFileSync(marker, 'utf8')).toBe('hello');
+    } finally {
+      fs.rmSync(path.dirname(filePath), { recursive: true, force: true });
+      fs.rmSync(docDir, { recursive: true, force: true });
+    }
+  });
+});
+
+describe('.picpeak manifest validation', () => {
+  it('rejects a database-engine mismatch', async () => {
+    // Harness runs on SQLite, so a pg manifest must be refused.
+    const blockers = await validateManifest({
+      kind: 'picpeak-backup', format: 1, database: { engine: 'pg' }, tables: {},
+    });
+    expect(blockers.some((b) => /engine/i.test(b))).toBe(true);
+  });
+
+  it('rejects a backup from a newer schema (forward-only)', async () => {
+    // validateManifest reads knex_migrations for the target's latest migration;
+    // the harness has none, so create it with an older migration than the backup.
+    await db.schema.createTable('knex_migrations', (t) => {
+      t.increments('id');
+      t.string('name');
+      t.integer('batch');
+      t.timestamp('migration_time');
+    });
+    try {
+      await db('knex_migrations').insert({ name: '100_baseline', batch: 1 });
+      const blockers = await validateManifest({
+        kind: 'picpeak-backup', format: 1,
+        database: { engine: 'sqlite', latest_migration: '999_from_the_future' },
+        tables: {},
+      });
+      expect(blockers.some((b) => /newer/i.test(b))).toBe(true);
+    } finally {
+      await db.schema.dropTableIfExists('knex_migrations');
+    }
+  });
+
+  it('rejects a file that is not a PicPeak backup', async () => {
+    const blockers = await validateManifest({ some: 'random-json' });
+    expect(blockers.length).toBeGreaterThan(0);
+  });
+});

backend/__tests__/integration/resetAdminMfaCli.test.js

@@ -0,0 +1,82 @@
+/**
+ * CLI test for scripts/reset-admin-mfa.js — break-glass MFA reset (#738).
+ *
+ * Boots a temp-SQLite DB, seeds an admin with MFA fully enabled, then runs
+ * the script in a child process (--email <addr> --yes) pointed at the same
+ * DB file, and asserts the four MFA columns are zeroed. The script runs in
+ * its own process with its own knex connection; the parent connection is
+ * idle during the spawn so the SQLite write lock isn't contended.
+ */
+
+const path = require('path');
+const { execFileSync } = require('child_process');
+
+const { bootCrmDb } = require('./helpers/crmDb');
+
+jest.setTimeout(60000);
+
+let db;
+let cleanup;
+
+beforeAll(async () => {
+  ({ db, cleanup } = await bootCrmDb());
+}, 60000);
+
+afterAll(async () => {
+  if (cleanup) await cleanup();
+});
+
+const SCRIPT = path.resolve(__dirname, '..', '..', 'scripts', 'reset-admin-mfa.js');
+
+async function seedEnrolledAdmin(email) {
+  const inserted = await db('admin_users').insert({
+    username: email.split('@')[0],
+    email,
+    password_hash: 'x',
+    is_active: true,
+    two_factor_enabled: true,
+    two_factor_secret: 'iv.tag.ct',
+    two_factor_recovery_codes: JSON.stringify(['$2b$10$fakehashfakehashfakehashfa']),
+    two_factor_enrolled_at: new Date(),
+    created_at: new Date(),
+  }).returning('id');
+  return inserted[0]?.id ?? inserted[0];
+}
+
+it('zeroes the four MFA columns for the targeted admin', async () => {
+  const email = '[email protected]';
+  const id = await seedEnrolledAdmin(email);
+
+  execFileSync('node', [SCRIPT, '--email', email, '--yes'], {
+    env: {
+      ...process.env,
+      NODE_ENV: 'test',
+      TEST_DATABASE_PATH: process.env.TEST_DATABASE_PATH,
+    },
+    stdio: 'pipe',
+  });
+
+  const row = await db('admin_users').where({ id }).first();
+  expect(Number(row.two_factor_enabled)).toBe(0);
+  expect(row.two_factor_secret).toBeNull();
+  expect(row.two_factor_recovery_codes).toBeNull();
+  expect(row.two_factor_enrolled_at).toBeNull();
+});
+
+it('leaves a different admin untouched', async () => {
+  const targetEmail = '[email protected]';
+  const bystanderEmail = '[email protected]';
+  const targetId = await seedEnrolledAdmin(targetEmail);
+  const bystanderId = await seedEnrolledAdmin(bystanderEmail);
+
+  execFileSync('node', [SCRIPT, '--email', targetEmail, '--yes'], {
+    env: { ...process.env, NODE_ENV: 'test', TEST_DATABASE_PATH: process.env.TEST_DATABASE_PATH },
+    stdio: 'pipe',
+  });
+
+  const target = await db('admin_users').where({ id: targetId }).first();
+  const bystander = await db('admin_users').where({ id: bystanderId }).first();
+  expect(Number(target.two_factor_enabled)).toBe(0);
+  expect(Number(bystander.two_factor_enabled)).toBe(1);
+  expect(bystander.two_factor_secret).toBe('iv.tag.ct');
+});

backend/__tests__/middleware/ownership.filterOwnedEventIds.test.js

@@ -0,0 +1,78 @@
+/**
+ * Regression test for the bulk archive/delete ownership bypass.
+ *
+ * bulk-archive and bulk-delete acted on body-supplied event ids with no
+ * ownership filter, so an admin/editor scoped to their own events (the
+ * single-event routes enforce requireEventOwnership) could archive or
+ * cascade-delete ANY event by id. filterOwnedEventIds is the helper those
+ * routes now use to drop foreign/non-existent ids.
+ */
+
+// events owned by admin 7; event 3 owned by someone else; event 4 is
+// ownerless (legacy). The mock models:
+//   whereIn('id', ids).andWhere(created_by IS NULL OR created_by = admin.id)
+const EVENTS = [
+  { id: 1, created_by: 7 },
+  { id: 2, created_by: 7 },
+  { id: 3, created_by: 99 },   // foreign
+  { id: 4, created_by: null }, // ownerless/legacy
+];
+
+jest.mock('../../src/database/db', () => ({
+  db: () => {
+    const q = {
+      _ids: null,
+      _adminId: null,
+      whereIn(_col, ids) { this._ids = ids; return this; },
+      andWhere(cb) {
+        // Emulate the (created_by IS NULL OR created_by = admin.id) builder
+        // by capturing the admin id the callback closes over via a probe.
+        const probe = {
+          _adminId: null,
+          whereNull() { return this; },
+          orWhere(_col, id) { this._adminId = id; return this; },
+        };
+        cb(probe);
+        this._adminId = probe._adminId;
+        return this;
+      },
+      select() {
+        return Promise.resolve(
+          EVENTS
+            .filter((e) => this._ids.includes(e.id))
+            .filter((e) => e.created_by === null || e.created_by === this._adminId)
+            .map((e) => ({ id: e.id }))
+        );
+      },
+    };
+    return q;
+  },
+}));
+
+const { filterOwnedEventIds } = require('../../src/middleware/ownership');
+
+describe('filterOwnedEventIds', () => {
+  it('super_admin gets every id, nothing denied', async () => {
+    const { allowed, denied } = await filterOwnedEventIds(
+      { id: 7, roleName: 'super_admin' }, [1, 3, 4, 999]
+    );
+    expect(allowed).toEqual([1, 3, 4, 999]);
+    expect(denied).toEqual([]);
+  });
+
+  it('non-super_admin keeps owned + ownerless, denies foreign and non-existent', async () => {
+    const { allowed, denied } = await filterOwnedEventIds(
+      { id: 7, roleName: 'admin' }, [1, 2, 3, 4, 999]
+    );
+    expect(allowed.sort()).toEqual([1, 2, 4]);   // owns 1,2; 4 is ownerless
+    expect(denied.sort()).toEqual([3, 999]);      // 3 foreign, 999 missing
+  });
+
+  it('foreign-only request yields empty allowed', async () => {
+    const { allowed, denied } = await filterOwnedEventIds(
+      { id: 7, roleName: 'editor' }, [3]
+    );
+    expect(allowed).toEqual([]);
+    expect(denied).toEqual([3]);
+  });
+});

backend/__tests__/middleware/photoAuth.thumbnailScope.test.js

@@ -0,0 +1,103 @@
+/**
+ * Regression test for the cross-event thumbnail enumeration leak.
+ *
+ * Thumbnails are served flat from /thumbnails/thumb_<name> with
+ * deterministic, enumerable filenames. photoAuth previously granted any
+ * holder of a gallery token for ANY active event access to ANY thumbnail
+ * (it set eventSlug=null and returned next() as long as the token's event
+ * existed), so a visitor to one gallery could pull another (password-
+ * protected) gallery's entire thumbnail set. The fix scopes thumbnail
+ * access to the token's event by matching the requested file against
+ * photos.thumbnail_path for that event_id.
+ */
+
+process.env.JWT_SECRET = 'test-secret-thumbnail-scope-000000000000';
+
+const jwt = require('jsonwebtoken');
+
+// Two events, each owning one thumbnail. The photos mock resolves a row
+// only when BOTH event_id and thumbnail_path match — i.e. it models the
+// real ownership query.
+const EVENTS = [
+  { id: 10, slug: 'event-a', is_active: 1 },
+  { id: 20, slug: 'event-b', is_active: 1 },
+];
+const PHOTOS = [
+  { id: 1, event_id: 10, thumbnail_path: 'thumbnails/thumb_event-a_ceremony_0001.jpg' },
+  { id: 2, event_id: 20, thumbnail_path: 'thumbnails/thumb_event-b_ceremony_0001.jpg' },
+];
+
+jest.mock('../../src/database/db', () => ({
+  db: (table) => ({
+    _cond: null,
+    where(cond) { this._cond = cond; return this; },
+    first() {
+      if (table === 'events') {
+        return Promise.resolve(EVENTS.find((e) => e.id === this._cond.id) || null);
+      }
+      if (table === 'photos') {
+        return Promise.resolve(
+          PHOTOS.find((p) => p.event_id === this._cond.event_id
+            && p.thumbnail_path === this._cond.thumbnail_path) || null
+        );
+      }
+      return Promise.resolve(null);
+    },
+  }),
+}));
+
+jest.mock('../../src/utils/logger', () => ({
+  info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn(),
+}));
+
+const photoAuth = require('../../src/middleware/photoAuth');
+
+function galleryToken(eventId) {
+  return jwt.sign({ type: 'gallery', eventId }, process.env.JWT_SECRET, { issuer: 'picpeak-auth' });
+}
+
+function makeReqRes(token, thumbPath) {
+  const req = { path: thumbPath, headers: { authorization: `Bearer ${token}` }, cookies: {} };
+  const res = {
+    statusCode: null,
+    body: null,
+    status(code) { this.statusCode = code; return this; },
+    json(payload) { this.body = payload; return this; },
+  };
+  return { req, res };
+}
+
+describe('photoAuth — thumbnail ownership scoping', () => {
+  it('denies a gallery token for event A fetching event B\'s thumbnail', async () => {
+    const { req, res } = makeReqRes(galleryToken(10), '/thumb_event-b_ceremony_0001.jpg');
+    const next = jest.fn();
+
+    await photoAuth(req, res, next);
+
+    // Access denied: middleware must not pass the request through.
+    expect(next).not.toHaveBeenCalled();
+    expect(res.statusCode).toBeGreaterThanOrEqual(400);
+    expect(req.event).toBeUndefined();
+  });
+
+  it('allows a gallery token to fetch its own event\'s thumbnail', async () => {
+    const { req, res } = makeReqRes(galleryToken(20), '/thumb_event-b_ceremony_0001.jpg');
+    const next = jest.fn();
+
+    await photoAuth(req, res, next);
+
+    expect(next).toHaveBeenCalled();
+    expect(req.event).toMatchObject({ id: 20 });
+  });
+
+  it('denies a traversal / foreign filename that matches no owned thumbnail', async () => {
+    const { req, res } = makeReqRes(galleryToken(10), '/thumb_../../etc/passwd');
+    const next = jest.fn();
+
+    await photoAuth(req, res, next);
+
+    expect(next).not.toHaveBeenCalled();
+    expect(res.statusCode).toBeGreaterThanOrEqual(400);
+    expect(req.event).toBeUndefined();
+  });
+});

backend/__tests__/routes/adminMfa.test.js

@@ -0,0 +1,345 @@
+/**
+ * HTTP-level tests for the admin TOTP MFA feature (#738).
+ *
+ * Two surfaces:
+ *   1. Enrollment (adminAuth-gated) — POST /mfa/setup, /mfa/enable,
+ *      GET /mfa/status, POST /mfa/disable — mounted like server.js at
+ *      /api/admin/auth (src/routes/adminAuth.js).
+ *   2. Login challenge — POST /admin/login + POST /admin/login/mfa
+ *      (src/routes/auth.js, mounted /api/auth).
+ *
+ * Uses the same real-SQLite harness as the CRM route tests
+ * (bootCrmDb + seedMinimal + mintAdminToken). Valid TOTP codes are
+ * generated in-test via otplib's authenticator against the secret the
+ * /setup endpoint returns in plaintext.
+ *
+ * NOTE: env (TEST_DATABASE_PATH / JWT_SECRET) must be set BEFORE the
+ * first require of db.js — mirror adminCrmAuth.test.js exactly.
+ */
+
+const path = require('path');
+const fs = require('fs');
+const os = require('os');
+
+const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'picpeak-adminmfa-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 || 'mfa-route-test-secret';
+// reCAPTCHA disabled (default) → verifyRecaptcha returns true, so login
+// tests don't need a token. Be explicit so a leaked env can't flip it on.
+delete process.env.RECAPTCHA_SECRET_KEY;
+
+const request = require('supertest');
+const bcrypt = require('bcrypt');
+const { authenticator } = require('otplib');
+
+const {
+  bootCrmDb, mintAdminToken, buildRouteApp,
+} = require('../integration/helpers/crmDb');
+
+jest.setTimeout(60000);
+
+let db;
+let cleanup;
+let adminApp; // /api/admin/auth  (enrollment)
+let authApp; // /api/auth        (login challenge)
+
+/**
+ * Seed a bare admin (password known) and return its id + login creds.
+ * seedMinimal always creates username 'tester'; we need distinct rows per
+ * scenario, so insert directly with a unique username/email.
+ */
+async function seedAdmin({ username, superAdmin = false } = {}) {
+  const password = 'correct-horse';
+  const passwordHash = await bcrypt.hash(password, 4);
+  const uname = username || `admin-${Math.random().toString(36).slice(2, 8)}`;
+  const row = {
+    username: uname,
+    email: `${uname}@example.com`,
+    password_hash: passwordHash,
+    must_change_password: false,
+    is_active: true,
+    created_at: new Date(),
+  };
+  if (superAdmin) {
+    const role = await db('roles').where({ name: 'super_admin' }).first();
+    if (!role) throw new Error('super_admin role not seeded');
+    row.role_id = role.id;
+  }
+  const inserted = await db('admin_users').insert(row).returning('id');
+  const id = inserted[0]?.id ?? inserted[0];
+  return { id, username: uname, password };
+}
+
+/** Run the full setup→enable enrollment against the live app. Returns
+ * the plaintext TOTP secret (for later login codes) and recovery codes. */
+async function enroll(adminId) {
+  const token = mintAdminToken(adminId);
+  const setup = await request(adminApp)
+    .post('/api/admin/auth/mfa/setup')
+    .set('Authorization', `Bearer ${token}`);
+  expect(setup.status).toBe(200);
+  const secret = setup.body.secret;
+
+  const enable = await request(adminApp)
+    .post('/api/admin/auth/mfa/enable')
+    .set('Authorization', `Bearer ${token}`)
+    .send({ code: authenticator.generate(secret) });
+  expect(enable.status).toBe(200);
+  return { secret, recoveryCodes: enable.body.recoveryCodes, token };
+}
+
+beforeAll(async () => {
+  ({ db, cleanup } = await bootCrmDb());
+  adminApp = buildRouteApp('/api/admin/auth', require('../../src/routes/adminAuth'));
+  authApp = buildRouteApp('/api/auth', require('../../src/routes/auth'));
+}, 60000);
+
+afterAll(async () => {
+  if (cleanup) await cleanup();
+});
+
+describe('MFA enrollment — /api/admin/auth/mfa/*', () => {
+  it('setup returns a secret + otpauth URI + QR and does NOT enable yet', async () => {
+    const admin = await seedAdmin();
+    const token = mintAdminToken(admin.id);
+
+    const res = await request(adminApp)
+      .post('/api/admin/auth/mfa/setup')
+      .set('Authorization', `Bearer ${token}`);
+
+    expect(res.status).toBe(200);
+    expect(res.body.secret).toEqual(expect.any(String));
+    expect(res.body.otpauthUri).toMatch(/^otpauth:\/\/totp\//);
+    expect(res.body.qr).toMatch(/^data:image\/png;base64,/);
+
+    // Not yet enabled: status must still report disabled.
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(status.body.enabled).toBe(false);
+
+    // And the row stores an encrypted secret (not the plaintext one).
+    const row = await db('admin_users').where({ id: admin.id }).first();
+    expect(row.two_factor_secret).toBeTruthy();
+    expect(row.two_factor_secret).not.toBe(res.body.secret);
+    expect(Number(row.two_factor_enabled)).toBe(0);
+  });
+
+  it('full flow: setup → enable(valid TOTP) → status shows enabled + 10 recovery codes', async () => {
+    const admin = await seedAdmin();
+    const { recoveryCodes, token } = await enroll(admin.id);
+
+    expect(Array.isArray(recoveryCodes)).toBe(true);
+    expect(recoveryCodes).toHaveLength(10);
+
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(status.status).toBe(200);
+    expect(status.body.enabled).toBe(true);
+    expect(status.body.recoveryCodesRemaining).toBe(10);
+    expect(status.body.enrolledAt).toBeTruthy();
+  });
+
+  it('enable with a WRONG code is rejected (400) and MFA stays off', async () => {
+    const admin = await seedAdmin();
+    const token = mintAdminToken(admin.id);
+    const setup = await request(adminApp)
+      .post('/api/admin/auth/mfa/setup')
+      .set('Authorization', `Bearer ${token}`);
+    const valid = authenticator.generate(setup.body.secret);
+    const wrong = valid === '000000' ? '111111' : '000000';
+
+    const res = await request(adminApp)
+      .post('/api/admin/auth/mfa/enable')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ code: wrong });
+    expect(res.status).toBe(400);
+
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(status.body.enabled).toBe(false);
+  });
+
+  it('enable before setup is rejected', async () => {
+    const admin = await seedAdmin();
+    const token = mintAdminToken(admin.id);
+    const res = await request(adminApp)
+      .post('/api/admin/auth/mfa/enable')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ code: '123456' });
+    // No provisional secret → ValidationError (400).
+    expect(res.status).toBe(400);
+  });
+
+  it('all enrollment endpoints require a valid admin token (401 without one)', async () => {
+    const noToken = await request(adminApp).get('/api/admin/auth/mfa/status');
+    expect(noToken.status).toBe(401);
+    const setup = await request(adminApp).post('/api/admin/auth/mfa/setup');
+    expect(setup.status).toBe(401);
+  });
+
+  // Regression guard for #735: super_admin used to be blocked from enrolling.
+  // Enrollment operates on req.admin.id and is role-agnostic — assert a
+  // super_admin can complete the full setup→enable flow.
+  it('#735 regression — a super_admin can enroll in MFA', async () => {
+    const admin = await seedAdmin({ superAdmin: true });
+    const { recoveryCodes, token } = await enroll(admin.id);
+    expect(recoveryCodes).toHaveLength(10);
+
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(status.body.enabled).toBe(true);
+  });
+});
+
+describe('MFA disable — /api/admin/auth/mfa/disable', () => {
+  it('requires a valid code; a wrong code is rejected and state persists', async () => {
+    const admin = await seedAdmin();
+    const { token } = await enroll(admin.id);
+
+    const bad = await request(adminApp)
+      .post('/api/admin/auth/mfa/disable')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ code: '000000' });
+    expect(bad.status).toBe(400);
+
+    const stillOn = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(stillOn.body.enabled).toBe(true);
+  });
+
+  it('a valid TOTP disables MFA and clears the stored secret', async () => {
+    const admin = await seedAdmin();
+    const { secret, token } = await enroll(admin.id);
+
+    const res = await request(adminApp)
+      .post('/api/admin/auth/mfa/disable')
+      .set('Authorization', `Bearer ${token}`)
+      .send({ code: authenticator.generate(secret) });
+    expect(res.status).toBe(200);
+
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${token}`);
+    expect(status.body.enabled).toBe(false);
+    expect(status.body.recoveryCodesRemaining).toBe(0);
+
+    const row = await db('admin_users').where({ id: admin.id }).first();
+    expect(row.two_factor_secret).toBeNull();
+    expect(row.two_factor_recovery_codes).toBeNull();
+  });
+});
+
+describe('Admin login challenge — /api/auth/admin/login[/mfa]', () => {
+  it('an enrolled admin gets mfaRequired + mfaToken, NO session cookie', async () => {
+    const admin = await seedAdmin();
+    await enroll(admin.id);
+
+    const res = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+
+    expect(res.status).toBe(200);
+    expect(res.body.mfaRequired).toBe(true);
+    expect(res.body.mfaToken).toEqual(expect.any(String));
+    expect(res.body.user).toBeUndefined(); // no completed session
+    // No admin auth cookie should have been set on the challenge response.
+    const cookies = res.headers['set-cookie'] || [];
+    expect(cookies.join(';')).not.toMatch(/adminToken/i);
+  });
+
+  it('a NON-enrolled admin logs in directly (no mfaRequired)', async () => {
+    const admin = await seedAdmin();
+    const res = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+    expect(res.status).toBe(200);
+    expect(res.body.mfaRequired).toBeUndefined();
+    expect(res.body.user).toBeDefined();
+    expect(res.body.user.username).toBe(admin.username);
+  });
+
+  it('login/mfa with a valid TOTP completes the session', async () => {
+    const admin = await seedAdmin();
+    const { secret } = await enroll(admin.id);
+
+    const challenge = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+    const { mfaToken } = challenge.body;
+
+    const res = await request(authApp)
+      .post('/api/auth/admin/login/mfa')
+      .send({ mfaToken, code: authenticator.generate(secret) });
+
+    expect(res.status).toBe(200);
+    expect(res.body.user).toBeDefined();
+    expect(res.body.user.id).toBe(admin.id);
+  });
+
+  it('login/mfa with a wrong code is 401 MFA_INVALID', async () => {
+    const admin = await seedAdmin();
+    const { secret } = await enroll(admin.id);
+    const challenge = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+
+    const valid = authenticator.generate(secret);
+    const wrong = valid === '000000' ? '111111' : '000000';
+    const res = await request(authApp)
+      .post('/api/auth/admin/login/mfa')
+      .send({ mfaToken: challenge.body.mfaToken, code: wrong });
+
+    expect(res.status).toBe(401);
+    expect(res.body.code).toBe('MFA_INVALID');
+    expect(res.body.user).toBeUndefined();
+  });
+
+  it('a recovery code logs in and is then single-use (second use fails)', async () => {
+    const admin = await seedAdmin();
+    const { recoveryCodes } = await enroll(admin.id);
+    const recovery = recoveryCodes[0];
+
+    // First challenge + recovery-code exchange succeeds.
+    const c1 = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+    const first = await request(authApp)
+      .post('/api/auth/admin/login/mfa')
+      .send({ mfaToken: c1.body.mfaToken, code: recovery });
+    expect(first.status).toBe(200);
+    expect(first.body.user).toBeDefined();
+
+    // recoveryCodesRemaining dropped by one.
+    const status = await request(adminApp)
+      .get('/api/admin/auth/mfa/status')
+      .set('Authorization', `Bearer ${mintAdminToken(admin.id)}`);
+    expect(status.body.recoveryCodesRemaining).toBe(9);
+
+    // Second use of the SAME recovery code must fail.
+    const c2 = await request(authApp)
+      .post('/api/auth/admin/login')
+      .send({ username: admin.username, password: admin.password });
+    const second = await request(authApp)
+      .post('/api/auth/admin/login/mfa')
+      .send({ mfaToken: c2.body.mfaToken, code: recovery });
+    expect(second.status).toBe(401);
+    expect(second.body.code).toBe('MFA_INVALID');
+  });
+
+  it('login/mfa rejects a non-mfa_pending token (e.g. a normal admin JWT)', async () => {
+    const admin = await seedAdmin();
+    await enroll(admin.id);
+    const res = await request(authApp)
+      .post('/api/auth/admin/login/mfa')
+      .send({ mfaToken: mintAdminToken(admin.id), code: '123456' });
+    expect(res.status).toBe(401);
+  });
+});

backend/__tests__/services/mfaService.test.js

@@ -0,0 +1,193 @@
+/**
+ * Unit tests for mfaService — admin TOTP MFA (#738).
+ *
+ * Pure unit: no DB, no Express. Exercises the crypto/verification surface
+ * directly. JWT_SECRET is set at the top so getEncryptionKey()'s scrypt
+ * derivation has key material (the service derives the AES key from
+ * MFA_ENCRYPTION_KEY, falling back to JWT_SECRET).
+ */
+
+// Must be set BEFORE the service is required — the key is derived lazily per
+// call, but keep it explicit and stable so encrypt/decrypt round-trips.
+process.env.JWT_SECRET = process.env.JWT_SECRET || 'mfa-unit-test-secret';
+delete process.env.MFA_ENCRYPTION_KEY; // ensure we derive from JWT_SECRET
+
+const { authenticator } = require('otplib');
+const mfaService = require('../../src/services/mfaService');
+
+describe('mfaService — secret encryption (AES-256-GCM)', () => {
+  it('round-trips encrypt → decrypt to the original secret', () => {
+    const secret = mfaService.generateSecret();
+    const blob = mfaService.encryptSecret(secret);
+    expect(blob).toEqual(expect.any(String));
+    expect(blob).not.toContain(secret); // stored form is not plaintext
+    expect(blob.split('.')).toHaveLength(3); // iv.tag.ciphertext
+    expect(mfaService.decryptSecret(blob)).toBe(secret);
+  });
+
+  it('produces a different ciphertext each time (random IV) but decrypts identically', () => {
+    const secret = mfaService.generateSecret();
+    const a = mfaService.encryptSecret(secret);
+    const b = mfaService.encryptSecret(secret);
+    expect(a).not.toBe(b);
+    expect(mfaService.decryptSecret(a)).toBe(secret);
+    expect(mfaService.decryptSecret(b)).toBe(secret);
+  });
+
+  it('throws when decrypting a malformed blob (wrong segment count)', () => {
+    expect(() => mfaService.decryptSecret('garbage')).toThrow();
+    expect(() => mfaService.decryptSecret('only.two')).toThrow();
+  });
+
+  it('throws when the auth tag / ciphertext is tampered with', () => {
+    const secret = mfaService.generateSecret();
+    const [iv, tag, ct] = mfaService.encryptSecret(secret).split('.');
+    // Flip a character in the ciphertext → GCM auth check must fail.
+    const tampered = ct.slice(0, -2) + (ct.slice(-2) === 'AA' ? 'BB' : 'AA');
+    expect(() => mfaService.decryptSecret([iv, tag, tampered].join('.'))).toThrow();
+  });
+});
+
+describe('mfaService — TOTP verification', () => {
+  it('accepts a freshly generated code for the plaintext secret', () => {
+    const secret = mfaService.generateSecret();
+    const code = authenticator.generate(secret);
+    expect(mfaService.verifyTotp(code, secret)).toBe(true);
+  });
+
+  it('tolerates whitespace in the submitted code', () => {
+    const secret = mfaService.generateSecret();
+    const code = authenticator.generate(secret);
+    expect(mfaService.verifyTotp(` ${code} `, secret)).toBe(true);
+  });
+
+  it('rejects a wrong code', () => {
+    const secret = mfaService.generateSecret();
+    const code = authenticator.generate(secret);
+    const wrong = code === '000000' ? '111111' : '000000';
+    expect(mfaService.verifyTotp(wrong, secret)).toBe(false);
+  });
+
+  it('returns false for empty inputs rather than throwing', () => {
+    const secret = mfaService.generateSecret();
+    expect(mfaService.verifyTotp('', secret)).toBe(false);
+    expect(mfaService.verifyTotp('123456', '')).toBe(false);
+    expect(mfaService.verifyTotp(null, secret)).toBe(false);
+  });
+
+  it('verifies through the encrypted blob (verifyTotpEncrypted)', () => {
+    const secret = mfaService.generateSecret();
+    const stored = mfaService.encryptSecret(secret);
+    const code = authenticator.generate(secret);
+    expect(mfaService.verifyTotpEncrypted(code, stored)).toBe(true);
+
+    const wrong = code === '000000' ? '111111' : '000000';
+    expect(mfaService.verifyTotpEncrypted(wrong, stored)).toBe(false);
+  });
+
+  it('verifyTotpEncrypted returns false (no throw) for a corrupt blob', () => {
+    const secret = mfaService.generateSecret();
+    const code = authenticator.generate(secret);
+    expect(mfaService.verifyTotpEncrypted(code, 'not-a-valid-blob')).toBe(false);
+  });
+});
+
+describe('mfaService — otpauth URI / QR', () => {
+  it('builds an otpauth:// URI containing issuer, account and secret', () => {
+    const secret = mfaService.generateSecret();
+    const uri = mfaService.buildOtpauthUri('[email protected]', secret);
+    expect(uri).toMatch(/^otpauth:\/\/totp\//);
+    expect(uri).toContain(encodeURIComponent(mfaService.ISSUER));
+    expect(uri).toContain(`secret=${secret}`);
+  });
+
+  it('builds a PNG data-URL QR for the URI', async () => {
+    const secret = mfaService.generateSecret();
+    const uri = mfaService.buildOtpauthUri('[email protected]', secret);
+    const qr = await mfaService.buildQrDataUrl(uri);
+    expect(qr).toMatch(/^data:image\/png;base64,/);
+  });
+});
+
+describe('mfaService — recovery codes', () => {
+  it('generates 10 distinct plaintext codes and 10 distinct hashes', async () => {
+    const { plain, hashed } = await mfaService.generateRecoveryCodes();
+    expect(plain).toHaveLength(mfaService.RECOVERY_CODE_COUNT);
+    expect(hashed).toHaveLength(mfaService.RECOVERY_CODE_COUNT);
+    expect(new Set(plain).size).toBe(10);
+    expect(new Set(hashed).size).toBe(10);
+    // Hashes are bcrypt, not the plaintext.
+    hashed.forEach((h) => expect(h).toMatch(/^\$2[aby]\$/));
+    plain.forEach((p) => expect(hashed).not.toContain(p));
+  });
+
+  it('formats a raw code into 4-char groups', () => {
+    expect(mfaService.formatRecoveryCode('abcdefghij')).toBe('abcd-efgh-ij');
+  });
+
+  it('consumes a valid recovery code once and removes it (single-use)', async () => {
+    const { plain, hashed } = await mfaService.generateRecoveryCodes();
+    const target = plain[3];
+
+    const first = await mfaService.consumeRecoveryCode(target, hashed);
+    expect(first.matched).toBe(true);
+    expect(first.remainingHashes).toHaveLength(9);
+
+    // Reusing the same code against the reduced set must now fail.
+    const reuse = await mfaService.consumeRecoveryCode(target, first.remainingHashes);
+    expect(reuse.matched).toBe(false);
+    expect(reuse.remainingHashes).toHaveLength(9);
+  });
+
+  it('matches case-insensitively and trims whitespace', async () => {
+    const { plain, hashed } = await mfaService.generateRecoveryCodes();
+    const res = await mfaService.consumeRecoveryCode(`  ${plain[0].toUpperCase()}  `, hashed);
+    expect(res.matched).toBe(true);
+  });
+
+  it('rejects a wrong code and leaves the hash set unchanged', async () => {
+    const { hashed } = await mfaService.generateRecoveryCodes();
+    const res = await mfaService.consumeRecoveryCode('zzzz-zzzz-zz', hashed);
+    expect(res.matched).toBe(false);
+    expect(res.remainingHashes).toHaveLength(10);
+  });
+
+  it('handles empty / missing input safely', async () => {
+    const { hashed } = await mfaService.generateRecoveryCodes();
+    const res = await mfaService.consumeRecoveryCode('', hashed);
+    expect(res.matched).toBe(false);
+    expect(res.remainingHashes).toBe(hashed);
+    const noHashes = await mfaService.consumeRecoveryCode('abcd-efgh-ij', null);
+    expect(noHashes.matched).toBe(false);
+    expect(noHashes.remainingHashes).toEqual([]);
+  });
+});
+
+describe('mfaService — parseRecoveryCodes', () => {
+  it('parses a JSON string array', () => {
+    expect(mfaService.parseRecoveryCodes(JSON.stringify(['a', 'b']))).toEqual(['a', 'b']);
+  });
+  it('passes an already-array through', () => {
+    expect(mfaService.parseRecoveryCodes(['a', 'b'])).toEqual(['a', 'b']);
+  });
+  it('returns [] for null / garbage / non-array JSON', () => {
+    expect(mfaService.parseRecoveryCodes(null)).toEqual([]);
+    expect(mfaService.parseRecoveryCodes('{not json')).toEqual([]);
+    expect(mfaService.parseRecoveryCodes(JSON.stringify({ a: 1 }))).toEqual([]);
+  });
+});
+
+describe('mfaService — isEnrolled coercion', () => {
+  it('treats true / 1 / "1" as enrolled', () => {
+    expect(mfaService.isEnrolled({ two_factor_enabled: true })).toBe(true);
+    expect(mfaService.isEnrolled({ two_factor_enabled: 1 })).toBe(true);
+    expect(mfaService.isEnrolled({ two_factor_enabled: '1' })).toBe(true);
+  });
+  it('treats false / 0 / null / missing as not enrolled', () => {
+    expect(mfaService.isEnrolled({ two_factor_enabled: false })).toBe(false);
+    expect(mfaService.isEnrolled({ two_factor_enabled: 0 })).toBe(false);
+    expect(mfaService.isEnrolled({ two_factor_enabled: null })).toBe(false);
+    expect(mfaService.isEnrolled({})).toBe(false);
+    expect(mfaService.isEnrolled(null)).toBe(false);
+  });
+});