Task #244: Permissions impact preview + live update test sweep (focused subset)

Landed 3 of 11 umbrella items, deferred the rest as 3 well-scoped follow-ups.

#231 — POST /apps with permissionIds[] is now pinned by two tests in
app-permissions-crud.test.mjs: success commits the app + permission rows
together with an audit_logs row, and an unknown permissionId returns 404
without leaving an orphan app row or a stray app.create audit row. Extended
the after() to clean up audit_logs + permission_audit so reruns stay
idempotent.

#215 — Added two socket tests in role-permissions-realtime.test.mjs for the
per-permission POST and DELETE endpoints, mirroring the existing PUT
coverage. Both assert direct + group-derived holders receive
role_permissions_changed and outsiders do not. Each test creates a fresh
role via makeFreshRoleWithMembers() so prior state can't bleed in.

#216 — Found a real gap: apps.ts emitted nothing when an app's required-
permission set changed. Added emitAppsChangedToPermissionHolders() to
lib/realtime.ts (resolves users via role_permissions -> user_roles and
group_roles -> user_groups, dedupes, reuses emitAppsChangedToUsers), and
wired it into POST/DELETE /apps/:id/permissions — only emitted when an
actual row was inserted/deleted, not on no-op retries. New test file
apps-permissions-realtime.test.mjs covers direct holder + group-derived
holder receipt and an idempotent no-op DELETE NOT emitting.

Skipped (already done): #226 (non-admin gates already covered),
#229 (impact-preview already handles the removal branch).

Validation: 13/13 tests across the 3 modified files pass; 66/66 across
related permission/audit suites pass; full server suite is 236/238 with
the 2 failures (executive-meetings notifications, service-orders status
matrix) being pre-existing in untouched files.

Architect review: APPROVED with no critical/high findings; took the
optional hardening suggestion to add group-holder coverage to the #216
tests so both legs of the helper's resolution path are exercised.

Files: artifacts/api-server/src/lib/realtime.ts,
       artifacts/api-server/src/routes/apps.ts,
       artifacts/api-server/tests/app-permissions-crud.test.mjs,
       artifacts/api-server/tests/role-permissions-realtime.test.mjs,
       artifacts/api-server/tests/apps-permissions-realtime.test.mjs (new)
This commit is contained in:
Riyadh
2026-05-01 07:12:12 +00:00
parent ea196ea24f
commit 07753bb3e9
5 changed files with 635 additions and 0 deletions
@@ -144,6 +144,17 @@ after(async () => {
`DELETE FROM app_permissions WHERE app_id = ANY($1::int[])`,
[createdAppIds],
);
// The new #231 tests below exercise POST /api/apps which writes
// audit_logs + permission_audit rows alongside the app row. Clean both
// up here so reruns stay idempotent and don't accumulate stamped rows.
await pool.query(
`DELETE FROM permission_audit WHERE target_kind = 'app' AND target_id = ANY($1::int[])`,
[createdAppIds],
);
await pool.query(
`DELETE FROM audit_logs WHERE target_type = 'app' AND target_id = ANY($1::int[])`,
[createdAppIds],
);
await pool.query(`DELETE FROM apps WHERE id = ANY($1::int[])`, [
createdAppIds,
]);
@@ -318,3 +329,93 @@ test("non-admins receive 403 from every app-permissions admin endpoint", async (
// None of the rejected calls should have written a row.
assert.deepEqual(await getPairsForApp(appId), []);
});
// #231: POST /apps with permissionIds[] commits the app row and its permission
// rows in a single transaction. The route also pre-validates every permission
// id so it can reject the whole call with 404 before touching apps_table — the
// two tests below pin both halves of that contract so a future refactor can't
// silently start leaving orphaned app rows behind on a bad permission id.
test("POST /api/apps with permissionIds[] commits the app and its permission rows together", async () => {
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
const slug = `apc_create_with_perms_${stamp}`;
const { id: p1 } = await createPermission("apc.create.a");
const { id: p2 } = await createPermission("apc.create.b");
const res = await fetch(`${API_BASE}/api/apps`, {
method: "POST",
headers: { "Content-Type": "application/json", Cookie: adminCookie },
body: JSON.stringify({
slug,
nameAr: slug,
nameEn: slug,
iconName: "Box",
route: `/${slug}`,
color: "#000000",
permissionIds: [p1, p2],
}),
});
assert.equal(res.status, 201);
const created = await res.json();
assert.ok(Number.isInteger(created.id));
createdAppIds.push(created.id);
// Both permission rows landed atomically with the app.
assert.deepEqual(
await getPairsForApp(created.id),
[p1, p2].sort((a, b) => a - b),
);
// The audit_logs row for app.create exists and records the gating
// permissionIds the admin requested, so the history view shows the gate
// was set at create time (not in a separate POST).
const audit = await pool.query(
`SELECT metadata FROM audit_logs
WHERE action = 'app.create' AND target_type = 'app' AND target_id = $1`,
[created.id],
);
assert.equal(audit.rowCount, 1);
assert.deepEqual(
[...audit.rows[0].metadata.permissionIds].sort((a, b) => a - b),
[p1, p2].sort((a, b) => a - b),
);
});
test("POST /api/apps with an unknown permissionId rolls back: no orphan app row, no audit row", async () => {
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
const slug = `apc_create_rollback_${stamp}`;
const { id: realPerm } = await createPermission("apc.rollback.real");
const unknownPermRow = await pool.query(
`SELECT COALESCE(MAX(id), 0) AS m FROM permissions`,
);
const unknownPermId = unknownPermRow.rows[0].m + 9999;
const res = await fetch(`${API_BASE}/api/apps`, {
method: "POST",
headers: { "Content-Type": "application/json", Cookie: adminCookie },
body: JSON.stringify({
slug,
nameAr: slug,
nameEn: slug,
iconName: "Box",
route: `/${slug}`,
color: "#000000",
permissionIds: [realPerm, unknownPermId],
}),
});
assert.equal(res.status, 404);
// The slug must NOT exist in apps — even partial creation would leave the
// app reachable in an "unrestricted" state, defeating the gate the admin
// asked for. Using the slug (not an id) is intentional: the request never
// got an id back, so this is the only handle we have on the would-be row.
const orphan = await pool.query(`SELECT id FROM apps WHERE slug = $1`, [slug]);
assert.equal(orphan.rowCount, 0, "no orphan app row should exist after a 404");
// And no audit_logs row should claim we created it either.
const audit = await pool.query(
`SELECT 1 FROM audit_logs
WHERE action = 'app.create' AND metadata->>'slug' = $1`,
[slug],
);
assert.equal(audit.rowCount, 0, "no app.create audit row should exist after a 404");
});