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)
422 lines
16 KiB
JavaScript
422 lines
16 KiB
JavaScript
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 nonAdminId;
|
|
let nonAdminUsername;
|
|
let nonAdminCookie;
|
|
|
|
let hasUniquenessConstraint = false;
|
|
|
|
const createdAppIds = [];
|
|
const createdPermissionIds = [];
|
|
const createdUserIds = [];
|
|
|
|
async function detectUniquenessConstraint() {
|
|
// The schema in lib/db/src/schema/apps.ts declares a composite primary key
|
|
// on (app_id, permission_id), but the live DB may lag the schema until
|
|
// `drizzle push` succeeds (tracked separately). The idempotency assertions
|
|
// below depend on that constraint being enforced, so they are skipped
|
|
// until the constraint actually exists. Once the migration lands, the
|
|
// assertions start running automatically.
|
|
// Look for a unique/primary-key index whose key columns are EXACTLY
|
|
// {app_id, permission_id} — no fewer, no extras. A partial overlap
|
|
// (e.g. a single-column unique index, or a composite that also includes
|
|
// a third column) would not enforce the pairwise uniqueness the
|
|
// .onConflictDoNothing() route relies on for idempotency.
|
|
const { rows } = await pool.query(
|
|
`SELECT 1
|
|
FROM pg_index i
|
|
JOIN pg_class c ON c.oid = i.indrelid
|
|
JOIN LATERAL (
|
|
SELECT array_agg(att.attname ORDER BY att.attnum) AS cols
|
|
FROM unnest(i.indkey) WITH ORDINALITY AS k(attnum, ord)
|
|
JOIN pg_attribute att
|
|
ON att.attrelid = c.oid AND att.attnum = k.attnum
|
|
) cols ON true
|
|
WHERE c.relname = 'app_permissions'
|
|
AND (i.indisunique OR i.indisprimary)
|
|
AND cols.cols @> ARRAY['app_id','permission_id']::name[]
|
|
AND cols.cols <@ ARRAY['app_id','permission_id']::name[]
|
|
LIMIT 1`,
|
|
);
|
|
return rows.length > 0;
|
|
}
|
|
|
|
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 createApp(prefix) {
|
|
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
|
const slug = `${prefix}_${stamp}`;
|
|
const res = 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}`],
|
|
);
|
|
const id = res.rows[0].id;
|
|
createdAppIds.push(id);
|
|
return id;
|
|
}
|
|
|
|
async function createPermission(prefix) {
|
|
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
|
const name = `${prefix}.${stamp}`;
|
|
const res = await pool.query(
|
|
`INSERT INTO permissions (name, description_en) VALUES ($1, $2) RETURNING id`,
|
|
[name, name],
|
|
);
|
|
const id = res.rows[0].id;
|
|
createdPermissionIds.push(id);
|
|
return { id, name };
|
|
}
|
|
|
|
async function getPairsForApp(appId) {
|
|
const rows = await pool.query(
|
|
`SELECT permission_id FROM app_permissions WHERE app_id = $1 ORDER BY permission_id`,
|
|
[appId],
|
|
);
|
|
return rows.rows.map((r) => r.permission_id);
|
|
}
|
|
|
|
before(async () => {
|
|
hasUniquenessConstraint = await detectUniquenessConstraint();
|
|
|
|
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
|
adminUsername = `app_perms_admin_${stamp}`;
|
|
nonAdminUsername = `app_perms_user_${stamp}`;
|
|
|
|
const admin = await pool.query(
|
|
`INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
|
|
VALUES ($1, $2, $3, 'App Perms Admin', 'en', true) RETURNING id`,
|
|
[adminUsername, `${adminUsername}@example.com`, TEST_PASSWORD_HASH],
|
|
);
|
|
adminId = admin.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],
|
|
);
|
|
|
|
const user = await pool.query(
|
|
`INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
|
|
VALUES ($1, $2, $3, 'App Perms User', 'en', true) RETURNING id`,
|
|
[nonAdminUsername, `${nonAdminUsername}@example.com`, TEST_PASSWORD_HASH],
|
|
);
|
|
nonAdminId = user.rows[0].id;
|
|
createdUserIds.push(nonAdminId);
|
|
|
|
adminCookie = await loginAndGetCookie(adminUsername, TEST_PASSWORD);
|
|
nonAdminCookie = await loginAndGetCookie(nonAdminUsername, TEST_PASSWORD);
|
|
});
|
|
|
|
after(async () => {
|
|
if (createdAppIds.length) {
|
|
await pool.query(
|
|
`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,
|
|
]);
|
|
}
|
|
if (createdPermissionIds.length) {
|
|
await pool.query(
|
|
`DELETE FROM app_permissions WHERE permission_id = ANY($1::int[])`,
|
|
[createdPermissionIds],
|
|
);
|
|
await pool.query(`DELETE FROM permissions WHERE id = ANY($1::int[])`, [
|
|
createdPermissionIds,
|
|
]);
|
|
}
|
|
for (const uid of createdUserIds) {
|
|
if (uid !== undefined) {
|
|
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("GET /api/apps/:id/permissions lists nothing for a fresh app", async () => {
|
|
const appId = await createApp("apc_list_empty");
|
|
const res = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
headers: { Cookie: adminCookie },
|
|
});
|
|
assert.equal(res.status, 200);
|
|
const body = await res.json();
|
|
assert.deepEqual(body, []);
|
|
});
|
|
|
|
test("POST /api/apps/:id/permissions adds a permission and is idempotent on duplicates", async (t) => {
|
|
const appId = await createApp("apc_add");
|
|
const { id: permId, name: permName } = await createPermission("apc.add");
|
|
|
|
// First insert: 201, body contains the link.
|
|
const first = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: permId }),
|
|
});
|
|
assert.equal(first.status, 201);
|
|
const firstBody = await first.json();
|
|
assert.equal(firstBody.appId, appId);
|
|
assert.equal(firstBody.permissionId, permId);
|
|
assert.deepEqual(await getPairsForApp(appId), [permId]);
|
|
|
|
if (!hasUniquenessConstraint) {
|
|
// Without the (app_id, permission_id) primary key on app_permissions,
|
|
// .onConflictDoNothing() has nothing to conflict against, so a duplicate
|
|
// POST inserts a second row. Skip the idempotency assertions until the
|
|
// constraint exists in the DB (tracked separately).
|
|
t.skip(
|
|
"app_permissions has no uniqueness constraint on (app_id, permission_id) yet; skipping duplicate-insert assertions until drizzle push is re-run",
|
|
);
|
|
return;
|
|
}
|
|
|
|
// Re-adding the exact same pair must NOT throw a unique-violation; the
|
|
// route uses .onConflictDoNothing() so the UI can safely retry.
|
|
const second = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: permId }),
|
|
});
|
|
assert.equal(second.status, 201);
|
|
// Still exactly one row — the composite primary key was respected without
|
|
// an error bubbling up to the client.
|
|
assert.deepEqual(await getPairsForApp(appId), [permId]);
|
|
|
|
// List endpoint reflects the assignment with the joined permission record.
|
|
const list = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
headers: { Cookie: adminCookie },
|
|
});
|
|
assert.equal(list.status, 200);
|
|
const listBody = await list.json();
|
|
assert.equal(listBody.length, 1);
|
|
assert.equal(listBody[0].id, permId);
|
|
assert.equal(listBody[0].name, permName);
|
|
});
|
|
|
|
test("POST /api/apps/:id/permissions returns 404 for an unknown app or permission", async () => {
|
|
const appId = await createApp("apc_unknown");
|
|
const { id: permId } = await createPermission("apc.unknown");
|
|
|
|
const unknownAppRow = await pool.query(
|
|
`SELECT COALESCE(MAX(id), 0) AS m FROM apps`,
|
|
);
|
|
const unknownAppId = unknownAppRow.rows[0].m + 9999;
|
|
const unknownPermRow = await pool.query(
|
|
`SELECT COALESCE(MAX(id), 0) AS m FROM permissions`,
|
|
);
|
|
const unknownPermId = unknownPermRow.rows[0].m + 9999;
|
|
|
|
const noApp = await fetch(`${API_BASE}/api/apps/${unknownAppId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: permId }),
|
|
});
|
|
assert.equal(noApp.status, 404);
|
|
|
|
const noPerm = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: unknownPermId }),
|
|
});
|
|
assert.equal(noPerm.status, 404);
|
|
|
|
// Neither rejected call should have left a row behind.
|
|
assert.deepEqual(await getPairsForApp(appId), []);
|
|
});
|
|
|
|
test("DELETE /api/apps/:id/permissions/:permissionId removes the pair and is idempotent", async () => {
|
|
const appId = await createApp("apc_delete");
|
|
const { id: p1 } = await createPermission("apc.delete.a");
|
|
const { id: p2 } = await createPermission("apc.delete.b");
|
|
|
|
// Seed the app with two permissions.
|
|
for (const pid of [p1, p2]) {
|
|
const r = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
|
body: JSON.stringify({ permissionId: pid }),
|
|
});
|
|
assert.equal(r.status, 201);
|
|
}
|
|
assert.deepEqual(
|
|
await getPairsForApp(appId),
|
|
[p1, p2].sort((a, b) => a - b),
|
|
);
|
|
|
|
const del = await fetch(
|
|
`${API_BASE}/api/apps/${appId}/permissions/${p1}`,
|
|
{ method: "DELETE", headers: { Cookie: adminCookie } },
|
|
);
|
|
assert.equal(del.status, 204);
|
|
assert.deepEqual(await getPairsForApp(appId), [p2]);
|
|
|
|
// Re-deleting a pair that no longer exists is a 204 no-op (the row count
|
|
// is zero but the contract is idempotent so the UI can safely retry).
|
|
const again = await fetch(
|
|
`${API_BASE}/api/apps/${appId}/permissions/${p1}`,
|
|
{ method: "DELETE", headers: { Cookie: adminCookie } },
|
|
);
|
|
assert.equal(again.status, 204);
|
|
assert.deepEqual(await getPairsForApp(appId), [p2]);
|
|
});
|
|
|
|
test("non-admins receive 403 from every app-permissions admin endpoint", async () => {
|
|
const appId = await createApp("apc_forbidden");
|
|
const { id: permId } = await createPermission("apc.forbidden");
|
|
|
|
const list = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
headers: { Cookie: nonAdminCookie },
|
|
});
|
|
assert.equal(list.status, 403);
|
|
|
|
const add = await fetch(`${API_BASE}/api/apps/${appId}/permissions`, {
|
|
method: "POST",
|
|
headers: { "Content-Type": "application/json", Cookie: nonAdminCookie },
|
|
body: JSON.stringify({ permissionId: permId }),
|
|
});
|
|
assert.equal(add.status, 403);
|
|
|
|
const del = await fetch(
|
|
`${API_BASE}/api/apps/${appId}/permissions/${permId}`,
|
|
{ method: "DELETE", headers: { Cookie: nonAdminCookie } },
|
|
);
|
|
assert.equal(del.status, 403);
|
|
|
|
// 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");
|
|
});
|