Task #159: Block non-admin users from changing role permissions (tests)

## Original task The existing role-permission tests cover the system-role guard, unknown-permission 404, and the admin happy paths for PUT/POST/DELETE /api/roles/:id/permissions, but they never exercised the requireAdmin middleware with a real non-admin session. A silent regression of requireAdmin on these routes would let regular users edit role permissions undetected. ## What changed Added a new test file: artifacts/api-server/tests/role-permissions-non-admin.test.mjs It mirrors the style of role-permissions-assign.test.mjs: - Creates an admin user (used only to seed a target role with two known permissions). - Creates a regular user with no role assignments. - For each of PUT, POST, and DELETE on /api/roles/:id/permissions, logs in as the regular user and asserts the response status is 403. - After every rejected call, asserts the role's permission set in role_permissions is byte-for-byte unchanged. - For POST it deliberately picks a permission the role does NOT already have, so any regression would change the row count. - Cleans up created users, roles, role_permissions, and role_permission_audit rows in after(). ## Verification `pnpm --filter @workspace/api-server test` — all 192 tests pass, including the 3 new ones. ## Deviations None. Scope kept tight to the task description. ## Follow-up Proposed #226: parallel non-admin 403 coverage for POST/PATCH/DELETE /api/roles (CRUD), which today have no non-admin rejection tests either. Replit-Task-Id: 87446596-45f9-4241-acd6-299888b4feb4
2026-04-30 17:36:40 +00:00
parent 40534b7c8f
commit 4470ceba5e
1 changed files with 210 additions and 0 deletions
@@ -0,0 +1,210 @@
+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;
+let consumerId;
+let consumerUsername;
+let consumerCookie;
+let targetRoleId;
+let seededPermissionIds = [];
+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="));
+}
+
+async function getDbPermissionIdsForRole(roleId) {
+  const rows = await pool.query(
+    `SELECT permission_id FROM role_permissions WHERE role_id = $1 ORDER BY permission_id`,
+    [roleId],
+  );
+  return rows.rows.map((r) => r.permission_id);
+}
+
+before(async () => {
+  const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
+
+  // Admin user — used only to seed the target role with a known permission set
+  // so we can verify later that the rejected non-admin calls did not mutate it.
+  adminUsername = `role_perm_nonadmin_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 Perm NonAdmin 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);
+
+  // Regular (non-admin) user — has no role assignments at all.
+  consumerUsername = `role_perm_nonadmin_user_${stamp}`;
+  const c = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Role Perm NonAdmin User', 'en', true) RETURNING id`,
+    [consumerUsername, `${consumerUsername}@example.com`, TEST_PASSWORD_HASH],
+  );
+  consumerId = c.rows[0].id;
+  createdUserIds.push(consumerId);
+  await pool.query(
+    `INSERT INTO user_groups (user_id, group_id)
+     SELECT $1, id FROM groups WHERE name = 'Everyone'
+     ON CONFLICT DO NOTHING`,
+    [consumerId],
+  );
+  consumerCookie = await loginAndGetCookie(consumerUsername, TEST_PASSWORD);
+
+  // Create a target role via the admin API and seed it with two permissions
+  // so DELETE has something concrete to attempt to remove and we can detect
+  // any unwanted mutation in either direction.
+  const roleName = `role_perm_nonadmin_target_${stamp}`;
+  const roleRes = await fetch(`${API_BASE}/api/roles`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ name: roleName }),
+  });
+  assert.equal(roleRes.status, 201);
+  const roleBody = await roleRes.json();
+  targetRoleId = roleBody.id;
+  createdRoleIds.push(targetRoleId);
+
+  const permRows = await pool.query(
+    `SELECT id FROM permissions ORDER BY id LIMIT 2`,
+  );
+  assert.ok(permRows.rowCount >= 2, "need at least 2 seeded permissions");
+  seededPermissionIds = [permRows.rows[0].id, permRows.rows[1].id];
+
+  const seed = await fetch(
+    `${API_BASE}/api/roles/${targetRoleId}/permissions`,
+    {
+      method: "PUT",
+      headers: { "Content-Type": "application/json", Cookie: adminCookie },
+      body: JSON.stringify({ permissionIds: seededPermissionIds }),
+    },
+  );
+  assert.equal(seed.status, 200);
+  assert.deepEqual(
+    await getDbPermissionIdsForRole(targetRoleId),
+    [...seededPermissionIds].sort((a, b) => a - b),
+  );
+});
+
+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();
+});
+
+test("PUT /api/roles/:id/permissions returns 403 for a non-admin user and does not mutate the role", async () => {
+  const before = await getDbPermissionIdsForRole(targetRoleId);
+
+  const res = await fetch(
+    `${API_BASE}/api/roles/${targetRoleId}/permissions`,
+    {
+      method: "PUT",
+      headers: { "Content-Type": "application/json", Cookie: consumerCookie },
+      body: JSON.stringify({ permissionIds: [] }),
+    },
+  );
+  assert.equal(res.status, 403, `expected 403, got ${res.status}`);
+
+  // The role's permission set must be byte-for-byte identical after the
+  // rejected call — the requireAdmin guard must short-circuit before any
+  // DB write happens.
+  assert.deepEqual(await getDbPermissionIdsForRole(targetRoleId), before);
+});
+
+test("POST /api/roles/:id/permissions returns 403 for a non-admin user and does not mutate the role", async () => {
+  const before = await getDbPermissionIdsForRole(targetRoleId);
+
+  // Pick a permission id that is NOT already on the role so that, if the
+  // guard ever regressed, we'd see the row count change.
+  const extraRow = await pool.query(
+    `SELECT id FROM permissions WHERE id <> ALL($1::int[]) ORDER BY id LIMIT 1`,
+    [before],
+  );
+  assert.ok(extraRow.rowCount > 0, "need a permission not already on the role");
+  const extraPermissionId = extraRow.rows[0].id;
+
+  const res = await fetch(
+    `${API_BASE}/api/roles/${targetRoleId}/permissions`,
+    {
+      method: "POST",
+      headers: { "Content-Type": "application/json", Cookie: consumerCookie },
+      body: JSON.stringify({ permissionId: extraPermissionId }),
+    },
+  );
+  assert.equal(res.status, 403, `expected 403, got ${res.status}`);
+
+  assert.deepEqual(await getDbPermissionIdsForRole(targetRoleId), before);
+});
+
+test("DELETE /api/roles/:id/permissions/:permissionId returns 403 for a non-admin user and does not mutate the role", async () => {
+  const before = await getDbPermissionIdsForRole(targetRoleId);
+  assert.ok(
+    before.length > 0,
+    "target role should still have its seeded permissions",
+  );
+  const targetPermissionId = before[0];
+
+  const res = await fetch(
+    `${API_BASE}/api/roles/${targetRoleId}/permissions/${targetPermissionId}`,
+    { method: "DELETE", headers: { Cookie: consumerCookie } },
+  );
+  assert.equal(res.status, 403, `expected 403, got ${res.status}`);
+
+  assert.deepEqual(await getDbPermissionIdsForRole(targetRoleId), before);
+});