87b16fd256
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)
372 lines
14 KiB
JavaScript
372 lines
14 KiB
JavaScript
import { test, before, after } from "node:test";
|
|
import assert from "node:assert/strict";
|
|
import pg from "pg";
|
|
import { io as ioClient } from "socket.io-client";
|
|
|
|
// #216: when an app's required-permission set changes via the per-permission
|
|
// CRUD endpoints, holders of the affected permission must receive an
|
|
// `apps_changed` broadcast so /api/apps refetches without a manual reload.
|
|
// Users who do not hold the permission must NOT be woken up — that contract
|
|
// is what these tests pin down. The full-replace PUT path on /api/roles is
|
|
// covered by `role-permissions-realtime.test.mjs`; this file is the mirror
|
|
// for the app-side endpoints.
|
|
|
|
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 adminCookie;
|
|
|
|
let holderId;
|
|
let holderCookie;
|
|
|
|
let groupHolderId;
|
|
let groupHolderCookie;
|
|
let helperGroupId;
|
|
|
|
let outsiderId;
|
|
let outsiderCookie;
|
|
|
|
let testRoleId;
|
|
let testPermId;
|
|
let secondPermId;
|
|
let testAppId;
|
|
|
|
const createdRoleIds = [];
|
|
const createdGroupIds = [];
|
|
const createdAppIds = [];
|
|
const createdPermIds = [];
|
|
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 makeUser(prefix, stamp) {
|
|
const username = `${prefix}_${stamp}`;
|
|
const r = await pool.query(
|
|
`INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
|
|
VALUES ($1, $2, $3, $4, 'en', true) RETURNING id`,
|
|
[username, `${username}@example.com`, TEST_PASSWORD_HASH, `${prefix} User`],
|
|
);
|
|
const id = r.rows[0].id;
|
|
createdUserIds.push(id);
|
|
return { id, username };
|
|
}
|
|
|
|
// Mirrors the connectSocket helper from role-permissions-realtime.test.mjs
|
|
// but listens for `apps_changed` (a payload-less event) rather than
|
|
// `role_permissions_changed`. Each call resolves on socket `connect` so the
|
|
// user is already joined to its `user:<id>` room when the test fires the
|
|
// triggering write — without that, the broadcast can race the join and the
|
|
// holder appears to silently miss the event.
|
|
function connectSocket(cookie) {
|
|
return new Promise((resolve, reject) => {
|
|
const events = [];
|
|
const waiters = [];
|
|
const socket = ioClient(API_BASE, {
|
|
path: "/api/socket.io",
|
|
transports: ["websocket"],
|
|
forceNew: true,
|
|
reconnection: false,
|
|
extraHeaders: { Cookie: cookie },
|
|
});
|
|
socket.on("apps_changed", (payload) => {
|
|
events.push(payload ?? null);
|
|
while (waiters.length > 0) waiters.shift()(payload ?? null);
|
|
});
|
|
socket.on("connect", () =>
|
|
resolve({
|
|
socket,
|
|
events,
|
|
waitForEvent(timeoutMs = 2000) {
|
|
if (events.length > 0) return Promise.resolve(events[events.length - 1]);
|
|
return new Promise((res, rej) => {
|
|
const timer = setTimeout(() => {
|
|
const idx = waiters.indexOf(once);
|
|
if (idx >= 0) waiters.splice(idx, 1);
|
|
rej(new Error(`Timed out waiting for apps_changed after ${timeoutMs}ms`));
|
|
}, timeoutMs);
|
|
const once = (payload) => {
|
|
clearTimeout(timer);
|
|
res(payload);
|
|
};
|
|
waiters.push(once);
|
|
});
|
|
},
|
|
}),
|
|
);
|
|
socket.on("connect_error", (err) => reject(err));
|
|
});
|
|
}
|
|
|
|
function waitMs(ms) {
|
|
return new Promise((r) => setTimeout(r, ms));
|
|
}
|
|
|
|
before(async () => {
|
|
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
|
|
|
// Admin makes the per-permission writes via the requireAdmin gate.
|
|
const admin = await makeUser("apr_admin", stamp);
|
|
adminId = admin.id;
|
|
await pool.query(
|
|
`INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'admin'`,
|
|
[adminId],
|
|
);
|
|
adminCookie = await loginAndGetCookie(admin.username, TEST_PASSWORD);
|
|
|
|
// Two fresh permissions: one we'll attach/detach to the test app (the
|
|
// "subject"), and a second one the holder will own that should NOT trigger
|
|
// a broadcast — proves the emit is keyed on the affected permission, not
|
|
// a blanket fan-out to every permission holder in the system.
|
|
const p1 = await pool.query(
|
|
`INSERT INTO permissions (name, description_en) VALUES ($1, $2) RETURNING id`,
|
|
[`apr.subject.${stamp}`, "apps-permissions-realtime subject"],
|
|
);
|
|
testPermId = p1.rows[0].id;
|
|
createdPermIds.push(testPermId);
|
|
const p2 = await pool.query(
|
|
`INSERT INTO permissions (name, description_en) VALUES ($1, $2) RETURNING id`,
|
|
[`apr.unrelated.${stamp}`, "apps-permissions-realtime unrelated"],
|
|
);
|
|
secondPermId = p2.rows[0].id;
|
|
createdPermIds.push(secondPermId);
|
|
|
|
// Non-system role that holds the subject permission. Holder gets this role
|
|
// directly via user_roles so they're a permission holder via the canonical
|
|
// path getRoleHolderUserIds uses internally.
|
|
const roleRow = await pool.query(
|
|
`INSERT INTO roles (name, description_en) VALUES ($1, $2) RETURNING id`,
|
|
[`apr_role_${stamp}`, "apps-permissions-realtime role"],
|
|
);
|
|
testRoleId = roleRow.rows[0].id;
|
|
createdRoleIds.push(testRoleId);
|
|
await pool.query(
|
|
`INSERT INTO role_permissions (role_id, permission_id) VALUES ($1, $2)`,
|
|
[testRoleId, testPermId],
|
|
);
|
|
|
|
const holder = await makeUser("apr_holder", stamp);
|
|
holderId = holder.id;
|
|
await pool.query(
|
|
`INSERT INTO user_roles (user_id, role_id) VALUES ($1, $2)`,
|
|
[holderId, testRoleId],
|
|
);
|
|
holderCookie = await loginAndGetCookie(holder.username, TEST_PASSWORD);
|
|
|
|
// groupHolder gets the permission transitively: user_groups -> group_roles
|
|
// -> role_permissions. Mirrors the second leg of the helper's resolution
|
|
// logic so a regression that drops the group-derived path would surface
|
|
// here instead of slipping past with only the direct path tested.
|
|
const grouped = await makeUser("apr_grouped", stamp);
|
|
groupHolderId = grouped.id;
|
|
const groupRow = await pool.query(
|
|
`INSERT INTO groups (name, description_en) VALUES ($1, $2) RETURNING id`,
|
|
[`apr_group_${stamp}`, "apps-permissions-realtime group"],
|
|
);
|
|
helperGroupId = groupRow.rows[0].id;
|
|
createdGroupIds.push(helperGroupId);
|
|
await pool.query(
|
|
`INSERT INTO user_groups (user_id, group_id) VALUES ($1, $2)`,
|
|
[groupHolderId, helperGroupId],
|
|
);
|
|
await pool.query(
|
|
`INSERT INTO group_roles (group_id, role_id) VALUES ($1, $2)`,
|
|
[helperGroupId, testRoleId],
|
|
);
|
|
groupHolderCookie = await loginAndGetCookie(grouped.username, TEST_PASSWORD);
|
|
|
|
// Outsider holds NO role and NO permission, so they must never receive
|
|
// apps_changed for these writes.
|
|
const outsider = await makeUser("apr_outsider", stamp);
|
|
outsiderId = outsider.id;
|
|
outsiderCookie = await loginAndGetCookie(outsider.username, TEST_PASSWORD);
|
|
|
|
// The app the per-permission CRUD will mutate. Created without any
|
|
// permission rows so each test starts from a known state.
|
|
const slug = `apr_app_${stamp}`;
|
|
const appRow = await pool.query(
|
|
`INSERT INTO apps (slug, name_ar, name_en, route, is_active, sort_order)
|
|
VALUES ($1, $2, $3, $4, true, 999)
|
|
RETURNING id`,
|
|
[slug, slug, slug, `/${slug}`],
|
|
);
|
|
testAppId = appRow.rows[0].id;
|
|
createdAppIds.push(testAppId);
|
|
});
|
|
|
|
after(async () => {
|
|
if (createdAppIds.length) {
|
|
await pool.query(
|
|
`DELETE FROM app_permissions WHERE app_id = ANY($1::int[])`,
|
|
[createdAppIds],
|
|
);
|
|
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]);
|
|
}
|
|
if (createdRoleIds.length) {
|
|
await pool.query(`DELETE FROM user_roles WHERE role_id = ANY($1::int[])`, [createdRoleIds]);
|
|
await pool.query(`DELETE FROM group_roles 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]);
|
|
}
|
|
if (createdGroupIds.length) {
|
|
await pool.query(`DELETE FROM group_roles WHERE group_id = ANY($1::int[])`, [createdGroupIds]);
|
|
await pool.query(`DELETE FROM user_groups WHERE group_id = ANY($1::int[])`, [createdGroupIds]);
|
|
await pool.query(`DELETE FROM groups WHERE id = ANY($1::int[])`, [createdGroupIds]);
|
|
}
|
|
if (createdPermIds.length) {
|
|
await pool.query(
|
|
`DELETE FROM app_permissions WHERE permission_id = ANY($1::int[])`,
|
|
[createdPermIds],
|
|
);
|
|
await pool.query(
|
|
`DELETE FROM role_permissions WHERE permission_id = ANY($1::int[])`,
|
|
[createdPermIds],
|
|
);
|
|
await pool.query(`DELETE FROM permissions WHERE id = ANY($1::int[])`, [createdPermIds]);
|
|
}
|
|
for (const uid of createdUserIds) {
|
|
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("POST /api/apps/:id/permissions emits apps_changed to direct + group permission holders, not outsiders", async () => {
|
|
const direct = await connectSocket(holderCookie);
|
|
const grouped = await connectSocket(groupHolderCookie);
|
|
const outsider = await connectSocket(outsiderCookie);
|
|
|
|
try {
|
|
const res = await fetch(
|
|
`${API_BASE}/api/apps/${testAppId}/permissions`,
|
|
{
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: testPermId }),
|
|
},
|
|
);
|
|
assert.equal(res.status, 201, "POST /apps/:id/permissions should succeed");
|
|
|
|
// Both legs of the helper's resolution path must wake up: direct via
|
|
// user_roles and indirect via user_groups -> group_roles -> role_permissions.
|
|
await Promise.all([
|
|
direct.waitForEvent(2000),
|
|
grouped.waitForEvent(2000),
|
|
]);
|
|
assert.equal(direct.events.length, 1);
|
|
assert.equal(grouped.events.length, 1);
|
|
|
|
// Negative case: give the loop a moment after the holders' events so a
|
|
// (regression) extra emit to the outsider would have time to arrive,
|
|
// then assert nothing landed.
|
|
await waitMs(100);
|
|
assert.equal(
|
|
outsider.events.length,
|
|
0,
|
|
`outsider must not receive apps_changed, got ${outsider.events.length} event(s)`,
|
|
);
|
|
} finally {
|
|
direct.socket.disconnect();
|
|
grouped.socket.disconnect();
|
|
outsider.socket.disconnect();
|
|
}
|
|
});
|
|
|
|
test("DELETE /api/apps/:id/permissions/:permissionId emits apps_changed to direct + group permission holders, not outsiders", async () => {
|
|
// Sanity: the previous test left the (testAppId, testPermId) row in
|
|
// place, which is what the DELETE will remove. Asserting the precondition
|
|
// here makes a future test-order change fail loudly instead of silently
|
|
// turning this test into a no-op.
|
|
const pre = await pool.query(
|
|
`SELECT 1 FROM app_permissions WHERE app_id = $1 AND permission_id = $2`,
|
|
[testAppId, testPermId],
|
|
);
|
|
assert.equal(pre.rowCount, 1, "precondition: the pair must exist before DELETE");
|
|
|
|
const direct = await connectSocket(holderCookie);
|
|
const grouped = await connectSocket(groupHolderCookie);
|
|
const outsider = await connectSocket(outsiderCookie);
|
|
|
|
try {
|
|
const res = await fetch(
|
|
`${API_BASE}/api/apps/${testAppId}/permissions/${testPermId}`,
|
|
{ method: "DELETE", headers: { Cookie: adminCookie } },
|
|
);
|
|
assert.equal(res.status, 204, "DELETE /apps/:id/permissions/:pid should succeed");
|
|
|
|
await Promise.all([
|
|
direct.waitForEvent(2000),
|
|
grouped.waitForEvent(2000),
|
|
]);
|
|
assert.equal(direct.events.length, 1);
|
|
assert.equal(grouped.events.length, 1);
|
|
|
|
await waitMs(100);
|
|
assert.equal(
|
|
outsider.events.length,
|
|
0,
|
|
`outsider must not receive apps_changed, got ${outsider.events.length} event(s)`,
|
|
);
|
|
} finally {
|
|
direct.socket.disconnect();
|
|
grouped.socket.disconnect();
|
|
outsider.socket.disconnect();
|
|
}
|
|
});
|
|
|
|
test("DELETE no-op (pair already gone) does not emit apps_changed", async () => {
|
|
// Re-deleting the same pair from the previous test should be a 204 no-op
|
|
// (the route is idempotent) AND must not trigger a spurious broadcast,
|
|
// since nothing actually changed in app_permissions.
|
|
const holder = await connectSocket(holderCookie);
|
|
|
|
try {
|
|
const res = await fetch(
|
|
`${API_BASE}/api/apps/${testAppId}/permissions/${testPermId}`,
|
|
{ method: "DELETE", headers: { Cookie: adminCookie } },
|
|
);
|
|
assert.equal(res.status, 204, "idempotent re-DELETE should still be 204");
|
|
|
|
// Wait long enough that a real broadcast would have arrived, then assert
|
|
// the holder's event buffer is empty.
|
|
await waitMs(200);
|
|
assert.equal(
|
|
holder.events.length,
|
|
0,
|
|
`no-op DELETE must not emit apps_changed, got ${holder.events.length} event(s)`,
|
|
);
|
|
} finally {
|
|
holder.socket.disconnect();
|
|
}
|
|
});
|