Audit role permission changes (Task #100)

Record an audit trail every time a role's permissions change and surface recent history inside the role edit dialog. Schema (lib/db): - New `role_permission_audit` table: id, roleId (FK roles), actorUserId (FK users, nullable on delete), previousPermissionIds int[], newPermissionIds int[], createdAt. Created via raw SQL because `drizzle push` currently fails on a pre-existing app_permissions duplicate (followup #148 tracks fixing this). API (artifacts/api-server): - PUT /api/roles/:id/permissions now wraps the permission delete/insert, the legacy audit_logs row, and the new role_permission_audit row in a *single* transaction so the permission set and its audit trail always commit (or roll back) together. Previous IDs are also read inside the transaction to avoid races. - A role_permission_audit row is written on EVERY PUT call (per task spec), including no-op saves; the GET handler computes addedPermissionIds/removedPermissionIds so the History UI can distinguish meaningful changes from no-op saves. - New GET /api/roles/:id/audit (admin-only, ?limit=1..50, default 10) returns recent entries newest-first with computed added/removedPermissionIds and joined actor info. - New tests in tests/role-permission-audit.test.mjs cover write, every-call write (incl. no-op), GET ordering+diff+actor, no-op diff reporting, 404, and limit. Drive-by fixes: - Fixed pre-existing syntax corruption in tests/apps-open.test.mjs (stray `ccaPassa,` line from a prior bad merge that prevented the whole api-server test workflow from running). - Made tests/roles-crud.test.mjs "rejects an invalid name" robust to parallel test execution by scoping the leak check to the bad name instead of relying on a global row count. API spec / codegen (lib/api-spec, lib/api-client-react): - Added `getRolePermissionAudit` operation and `RolePermissionAuditEntry` schema; ran orval codegen. Frontend (artifacts/tx-os): - New RolePermissionHistory component renders inside the role edit dialog (between permissions and Save/Cancel) using useGetRolePermissionAudit. Shows timestamp, actor, added/removed permission names (resolved via permissionsById), and total count. - Save invalidates the audit query so the new entry appears immediately on the next open. - Bilingual i18n strings (en + ar with full Arabic plural variants: zero/one/two/few/many/other) under admin.roles.history*. Verification: - tx-os typecheck passes. - All 5 new audit tests + 13 existing roles tests pass. - E2E (testing skill) verified the full flow: empty state on a fresh role, save adds a permission, reopened dialog shows the new entry with actor name in Arabic. Docs: - replit.md updated to list `role_permission_audit` in Database Tables. Follow-ups proposed: #146 paginate/filter history, #147 audit other admin permission changes, #148 fix drizzle push duplicate. Replit-Task-Id: 9b8886a2-6e76-4072-b345-a772421fa161
2026-04-29 13:16:02 +00:00
parent 19b5dc788e
commit a14b589006
15 changed files with 896 additions and 32 deletions
@@ -57,7 +57,6 @@ before(async () => {
  testUsername = `open_test_${Date.now().toString(36)}_${Math.random()
    .toString(36)
    .slice(2, 6)}`;
- ccaPassa,
  const userRes = await pool.query(
    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
     VALUES ($1, $2, $3, 'Open Test', 'en', true) RETURNING id`,
@@ -0,0 +1,287 @@
+import { test, before, after } from "node:test";
+import assert from "node:assert/strict";
+import pg from "pg";
+
+const API_BASE = process.env.TEST_API_BASE ?? "http://localhost:8080";
+const DATABASE_URL = process.env.DATABASE_URL;
+if (!DATABASE_URL) {
+  throw new Error("DATABASE_URL must be set to run these tests");
+}
+
+const TEST_PASSWORD = "TestPass123!";
+const TEST_PASSWORD_HASH =
+  "$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu";
+
+const pool = new pg.Pool({ connectionString: DATABASE_URL });
+
+let adminId;
+let adminUsername;
+let adminCookie;
+const createdRoleIds = [];
+const createdUserIds = [];
+
+async function loginAndGetCookie(username, password) {
+  const res = await fetch(`${API_BASE}/api/auth/login`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json" },
+    body: JSON.stringify({ username, password }),
+  });
+  assert.equal(res.status, 200, `login expected 200, got ${res.status}`);
+  const setCookie = res.headers.get("set-cookie");
+  return setCookie
+    .split(",")
+    .map((c) => c.split(";")[0].trim())
+    .find((c) => c.startsWith("connect.sid="));
+}
+
+before(async () => {
+  const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
+  adminUsername = `role_audit_admin_${stamp}`;
+
+  const a = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Role Audit Admin', 'en', true) RETURNING id`,
+    [adminUsername, `${adminUsername}@example.com`, TEST_PASSWORD_HASH],
+  );
+  adminId = a.rows[0].id;
+  createdUserIds.push(adminId);
+  await pool.query(
+    `INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'admin'`,
+    [adminId],
+  );
+  await pool.query(
+    `INSERT INTO user_groups (user_id, group_id)
+     SELECT $1, id FROM groups WHERE name IN ('Everyone', 'Admins')
+     ON CONFLICT DO NOTHING`,
+    [adminId],
+  );
+
+  adminCookie = await loginAndGetCookie(adminUsername, TEST_PASSWORD);
+});
+
+after(async () => {
+  if (createdRoleIds.length) {
+    await pool.query(
+      `DELETE FROM role_permission_audit WHERE role_id = ANY($1::int[])`,
+      [createdRoleIds],
+    );
+    await pool.query(
+      `DELETE FROM role_permissions WHERE role_id = ANY($1::int[])`,
+      [createdRoleIds],
+    );
+    await pool.query(`DELETE FROM roles WHERE id = ANY($1::int[])`, [
+      createdRoleIds,
+    ]);
+  }
+  for (const uid of createdUserIds) {
+    if (uid !== undefined) {
+      await pool.query(`DELETE FROM user_groups WHERE user_id = $1`, [uid]);
+      await pool.query(`DELETE FROM user_roles WHERE user_id = $1`, [uid]);
+      await pool.query(`DELETE FROM users WHERE id = $1`, [uid]);
+    }
+  }
+  await pool.end();
+});
+
+function uniqueRoleName(prefix) {
+  const stamp = `${Date.now().toString(36)}${Math.random().toString(36).slice(2, 6)}`;
+  return `${prefix}_${stamp}`;
+}
+
+async function createRole() {
+  const res = await fetch(`${API_BASE}/api/roles`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ name: uniqueRoleName("role_audit") }),
+  });
+  assert.equal(res.status, 201);
+  const body = await res.json();
+  createdRoleIds.push(body.id);
+  return body;
+}
+
+async function getTwoPermissionIds() {
+  const rows = await pool.query(
+    `SELECT id FROM permissions ORDER BY id LIMIT 2`,
+  );
+  assert.ok(rows.rowCount >= 2, "need at least 2 seeded permissions");
+  return [rows.rows[0].id, rows.rows[1].id];
+}
+
+test("PUT /api/roles/:id/permissions writes a role_permission_audit row", async () => {
+  const role = await createRole();
+  const [p1, p2] = await getTwoPermissionIds();
+
+  const before = await pool.query(
+    `SELECT COUNT(*)::int AS c FROM role_permission_audit WHERE role_id = $1`,
+    [role.id],
+  );
+  assert.equal(before.rows[0].c, 0);
+
+  const put = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ permissionIds: [p1, p2] }),
+  });
+  assert.equal(put.status, 200);
+
+  const dbRows = await pool.query(
+    `SELECT actor_user_id, role_id, previous_permission_ids, new_permission_ids
+       FROM role_permission_audit
+      WHERE role_id = $1`,
+    [role.id],
+  );
+  assert.equal(dbRows.rowCount, 1);
+  const row = dbRows.rows[0];
+  assert.equal(row.actor_user_id, adminId);
+  assert.equal(row.role_id, role.id);
+  assert.deepEqual(row.previous_permission_ids, []);
+  assert.deepEqual(
+    [...row.new_permission_ids].sort((a, b) => a - b),
+    [p1, p2].sort((a, b) => a - b),
+  );
+});
+
+test("PUT /api/roles/:id/permissions writes an audit row on every call (including no-op replaces)", async () => {
+  const role = await createRole();
+  const [p1, p2] = await getTwoPermissionIds();
+
+  const first = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ permissionIds: [p1, p2] }),
+  });
+  assert.equal(first.status, 200);
+
+  // Re-submit the same set in a different order — the resulting permission
+  // set is identical, but the spec requires recording every PUT call.
+  const second = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ permissionIds: [p2, p1] }),
+  });
+  assert.equal(second.status, 200);
+
+  const dbRows = await pool.query(
+    `SELECT previous_permission_ids, new_permission_ids
+       FROM role_permission_audit
+      WHERE role_id = $1
+      ORDER BY id`,
+    [role.id],
+  );
+  assert.equal(
+    dbRows.rowCount,
+    2,
+    "every PUT call must append an audit row, including no-op saves",
+  );
+  const sorted = [p1, p2].sort((a, b) => a - b);
+  // Second row's previous and new arrays must be identical to the first
+  // row's "new" array, capturing the no-op save.
+  assert.deepEqual([...dbRows.rows[1].previous_permission_ids].sort((a, b) => a - b), sorted);
+  assert.deepEqual([...dbRows.rows[1].new_permission_ids].sort((a, b) => a - b), sorted);
+});
+
+test("GET /api/roles/:id/audit reports no-op saves with empty added/removed", async () => {
+  const role = await createRole();
+  const [p1] = await getTwoPermissionIds();
+
+  for (const set of [[p1], [p1]]) {
+    const r = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+      method: "PUT",
+      headers: { "Content-Type": "application/json", Cookie: adminCookie },
+      body: JSON.stringify({ permissionIds: set }),
+    });
+    assert.equal(r.status, 200);
+    await new Promise((r) => setTimeout(r, 10));
+  }
+
+  const get = await fetch(`${API_BASE}/api/roles/${role.id}/audit`, {
+    headers: { Cookie: adminCookie },
+  });
+  assert.equal(get.status, 200);
+  const entries = await get.json();
+  assert.equal(entries.length, 2);
+  // Latest entry is the no-op save: same set in/out, empty diffs.
+  assert.deepEqual(entries[0].previousPermissionIds, [p1]);
+  assert.deepEqual(entries[0].newPermissionIds, [p1]);
+  assert.deepEqual(entries[0].addedPermissionIds, []);
+  assert.deepEqual(entries[0].removedPermissionIds, []);
+});
+
+test("GET /api/roles/:id/audit returns entries newest first with diff fields and actor", async () => {
+  const role = await createRole();
+  const [p1, p2] = await getTwoPermissionIds();
+
+  // First change: add p1
+  const r1 = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ permissionIds: [p1] }),
+  });
+  assert.equal(r1.status, 200);
+
+  // Ensure the second change has a strictly later created_at so ordering is
+  // deterministic regardless of clock resolution.
+  await new Promise((r) => setTimeout(r, 25));
+
+  // Second change: swap p1 -> p2
+  const r2 = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+    method: "PUT",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ permissionIds: [p2] }),
+  });
+  assert.equal(r2.status, 200);
+
+  const get = await fetch(`${API_BASE}/api/roles/${role.id}/audit`, {
+    headers: { Cookie: adminCookie },
+  });
+  assert.equal(get.status, 200);
+  const entries = await get.json();
+  assert.ok(Array.isArray(entries));
+  assert.equal(entries.length, 2);
+
+  const [latest, earlier] = entries;
+  assert.deepEqual(latest.previousPermissionIds, [p1]);
+  assert.deepEqual(latest.newPermissionIds, [p2]);
+  assert.deepEqual(latest.addedPermissionIds, [p2]);
+  assert.deepEqual(latest.removedPermissionIds, [p1]);
+  assert.ok(latest.actor);
+  assert.equal(latest.actor.id, adminId);
+  assert.equal(latest.actor.username, adminUsername);
+
+  assert.deepEqual(earlier.previousPermissionIds, []);
+  assert.deepEqual(earlier.newPermissionIds, [p1]);
+  assert.deepEqual(earlier.addedPermissionIds, [p1]);
+  assert.deepEqual(earlier.removedPermissionIds, []);
+});
+
+test("GET /api/roles/:id/audit returns 404 for an unknown role", async () => {
+  const res = await fetch(`${API_BASE}/api/roles/99999999/audit`, {
+    headers: { Cookie: adminCookie },
+  });
+  assert.equal(res.status, 404);
+});
+
+test("GET /api/roles/:id/audit honours the limit parameter", async () => {
+  const role = await createRole();
+  const [p1, p2] = await getTwoPermissionIds();
+
+  const sets = [[p1], [p2], [p1, p2], []];
+  for (const s of sets) {
+    const r = await fetch(`${API_BASE}/api/roles/${role.id}/permissions`, {
+      method: "PUT",
+      headers: { "Content-Type": "application/json", Cookie: adminCookie },
+      body: JSON.stringify({ permissionIds: s }),
+    });
+    assert.equal(r.status, 200);
+    await new Promise((r) => setTimeout(r, 5));
+  }
+
+  const limited = await fetch(
+    `${API_BASE}/api/roles/${role.id}/audit?limit=2`,
+    { headers: { Cookie: adminCookie } },
+  );
+  assert.equal(limited.status, 200);
+  const entries = await limited.json();
+  assert.equal(entries.length, 2);
+});
@@ -180,15 +180,19 @@ test("POST /api/roles with a duplicate name returns 409", async () => {
 });

 test("POST /api/roles rejects an invalid name with 400", async () => {
-  const before = await pool.query(`SELECT COUNT(*)::int AS c FROM roles`);
+  const badName = "1bad name!";
  const res = await fetch(`${API_BASE}/api/roles`, {
    method: "POST",
    headers: { "Content-Type": "application/json", Cookie: adminCookie },
-    body: JSON.stringify({ name: "1bad name!" }),
+    body: JSON.stringify({ name: badName }),
  });
  assert.equal(res.status, 400);
-  const after = await pool.query(`SELECT COUNT(*)::int AS c FROM roles`);
-  assert.equal(after.rows[0].c, before.rows[0].c, "no role row should leak");
+  // Scope the leak check to the specific bad name so this test is robust
+  // when run in parallel with other suites that legitimately create roles.
+  const leak = await pool.query(`SELECT id FROM roles WHERE name = $1`, [
+    badName,
+  ]);
+  assert.equal(leak.rowCount, 0, "no role row should leak");
 });

 test("PATCH /api/roles/:id updates description without touching the name", async () => {