Add automated test coverage for groups and access control
Original task: Task #75 — add automated tests for the groups system (CRUD + group-driven app visibility + auto-Everyone) and stabilize the flaky pagination test. Changes: - Extended artifacts/api-server/tests/groups-crud.test.mjs with four new tests: * PATCH /api/groups/:id updates fields and replaces userIds transactionally; a bad userId returns 400 and leaves prior membership unchanged. * Creating a group with invalid roleIds returns 400 and creates no rows. * DELETE /api/groups/:id removes a custom group; deleting a system group (Everyone, is_system = 1) returns 400 and the row stays. * /api/auth/register auto-assigns the new user to the Everyone system group; verified both via the response payload and a direct user_groups DB check. The test temporarily flips app_settings.registration_open to true and restores the prior value on completion. - The existing apps-group-visibility.test.mjs already exercises the /api/apps endpoint backed by getVisibleAppsForUser for group-derived admin, group member, and outsider — no changes needed there. - The previously flaky tests/admin-app-opens-pagination.test.mjs now passes consistently thanks to the existing 7d-window cleanup in its before hook; left as-is per the task's "fixed or skipped" criterion. Verification: `pnpm --filter @workspace/api-server test` reports 42/42 passing. The downstream Playwright suite (4 tests) also passes.
This commit is contained in:
Riyadh
@@ -249,6 +249,202 @@ test("group create rolls back atomically when assignment fails", async () => {
|
||||
assert.equal(leaked.rowCount, 0);
|
||||
});
|
||||
|
||||
test("PATCH /api/groups/:id updates fields and replaces assignments transactionally", async () => {
|
||||
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
||||
const create = await fetch(`${API_BASE}/api/groups`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ name: `UpdGrp_${stamp}`, descriptionEn: "before" }),
|
||||
});
|
||||
assert.equal(create.status, 201);
|
||||
const created = await create.json();
|
||||
createdGroupIds.push(created.id);
|
||||
|
||||
const newName = `UpdGrpRenamed_${stamp}`;
|
||||
const patch = await fetch(`${API_BASE}/api/groups/${created.id}`, {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({
|
||||
name: newName,
|
||||
descriptionEn: "after",
|
||||
userIds: [adminId, regularId],
|
||||
}),
|
||||
});
|
||||
assert.equal(patch.status, 200);
|
||||
const updated = await patch.json();
|
||||
assert.equal(updated.name, newName);
|
||||
assert.equal(updated.descriptionEn, "after");
|
||||
assert.deepEqual(new Set(updated.userIds), new Set([adminId, regularId]));
|
||||
|
||||
// Replace assignment with just the regular user; admin should be removed.
|
||||
const patch2 = await fetch(`${API_BASE}/api/groups/${created.id}`, {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ userIds: [regularId] }),
|
||||
});
|
||||
assert.equal(patch2.status, 200);
|
||||
const detail = await (
|
||||
await fetch(`${API_BASE}/api/groups/${created.id}`, { headers: { Cookie: adminCookie } })
|
||||
).json();
|
||||
assert.deepEqual(detail.userIds, [regularId]);
|
||||
|
||||
// Bad userId on update — must 400, must not partially apply (membership unchanged).
|
||||
const maxRow = await pool.query(`SELECT COALESCE(MAX(id), 0) AS m FROM users`);
|
||||
const badId = Number(maxRow.rows[0].m) + 100000;
|
||||
const patchBad = await fetch(`${API_BASE}/api/groups/${created.id}`, {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ userIds: [adminId, badId] }),
|
||||
});
|
||||
assert.equal(patchBad.status, 400);
|
||||
const detail2 = await (
|
||||
await fetch(`${API_BASE}/api/groups/${created.id}`, { headers: { Cookie: adminCookie } })
|
||||
).json();
|
||||
assert.deepEqual(detail2.userIds, [regularId], "membership must be unchanged after rejected update");
|
||||
});
|
||||
|
||||
test("creating/updating a group with invalid roleIds (or appIds) returns 400", async () => {
|
||||
const maxRoleRow = await pool.query(`SELECT COALESCE(MAX(id), 0) AS m FROM roles`);
|
||||
const maxAppRow = await pool.query(`SELECT COALESCE(MAX(id), 0) AS m FROM apps`);
|
||||
const badRoleId = Number(maxRoleRow.rows[0].m) + 100000;
|
||||
const badAppId = Number(maxAppRow.rows[0].m) + 100000;
|
||||
|
||||
// CREATE with bad roleId — must 400 and create no row.
|
||||
const beforeCount = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
|
||||
const createRes = await fetch(`${API_BASE}/api/groups`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ name: `BadRole_${Date.now()}`, roleIds: [badRoleId] }),
|
||||
});
|
||||
assert.equal(createRes.status, 400);
|
||||
const afterCount = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
|
||||
assert.equal(afterCount.rows[0].c, beforeCount.rows[0].c);
|
||||
|
||||
// Build a clean group, then PATCH with bad roleIds and bad appIds.
|
||||
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
||||
const seedRes = await fetch(`${API_BASE}/api/groups`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ name: `BadRolePatch_${stamp}` }),
|
||||
});
|
||||
assert.equal(seedRes.status, 201);
|
||||
const seed = await seedRes.json();
|
||||
createdGroupIds.push(seed.id);
|
||||
|
||||
const patchRoleRes = await fetch(`${API_BASE}/api/groups/${seed.id}`, {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ roleIds: [badRoleId] }),
|
||||
});
|
||||
assert.equal(patchRoleRes.status, 400);
|
||||
|
||||
const patchAppRes = await fetch(`${API_BASE}/api/groups/${seed.id}`, {
|
||||
method: "PATCH",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ appIds: [badAppId] }),
|
||||
});
|
||||
assert.equal(patchAppRes.status, 400);
|
||||
|
||||
// Confirm no role/app links leaked onto the seed group from rejected updates.
|
||||
const detail = await (
|
||||
await fetch(`${API_BASE}/api/groups/${seed.id}`, { headers: { Cookie: adminCookie } })
|
||||
).json();
|
||||
assert.deepEqual(detail.roleIds, []);
|
||||
assert.deepEqual(detail.appIds, []);
|
||||
});
|
||||
|
||||
test("DELETE /api/groups/:id removes a custom group; system groups are protected", async () => {
|
||||
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
||||
const create = await fetch(`${API_BASE}/api/groups`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json", Cookie: adminCookie },
|
||||
body: JSON.stringify({ name: `DelGrp_${stamp}` }),
|
||||
});
|
||||
assert.equal(create.status, 201);
|
||||
const created = await create.json();
|
||||
|
||||
const del = await fetch(`${API_BASE}/api/groups/${created.id}`, {
|
||||
method: "DELETE",
|
||||
headers: { Cookie: adminCookie },
|
||||
});
|
||||
assert.equal(del.status, 204);
|
||||
const after = await pool.query(`SELECT id FROM groups WHERE id = $1`, [created.id]);
|
||||
assert.equal(after.rowCount, 0, "group row should be gone");
|
||||
|
||||
// System-group guard: trying to delete Everyone must fail and leave row intact.
|
||||
const sysRow = await pool.query(
|
||||
`SELECT id FROM groups WHERE name = 'Everyone' AND is_system = 1`,
|
||||
);
|
||||
assert.ok(sysRow.rowCount > 0, "Everyone system group must exist");
|
||||
const everyoneId = sysRow.rows[0].id;
|
||||
const denied = await fetch(`${API_BASE}/api/groups/${everyoneId}`, {
|
||||
method: "DELETE",
|
||||
headers: { Cookie: adminCookie },
|
||||
});
|
||||
assert.equal(denied.status, 400);
|
||||
const stillThere = await pool.query(`SELECT id FROM groups WHERE id = $1`, [everyoneId]);
|
||||
assert.equal(stillThere.rowCount, 1, "system group must not be deleted");
|
||||
});
|
||||
|
||||
test("/auth/register auto-assigns the new user to the Everyone system group", async () => {
|
||||
// Ensure registration is open for the duration of this test, then restore.
|
||||
const prior = await pool.query(
|
||||
`SELECT registration_open FROM app_settings WHERE id = 1`,
|
||||
);
|
||||
const priorOpen = prior.rows[0]?.registration_open;
|
||||
await pool.query(
|
||||
`INSERT INTO app_settings (id, registration_open) VALUES (1, true)
|
||||
ON CONFLICT (id) DO UPDATE SET registration_open = true`,
|
||||
);
|
||||
|
||||
const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
|
||||
const username = `reg_everyone_${stamp}`;
|
||||
let newUserId;
|
||||
try {
|
||||
const res = await fetch(`${API_BASE}/api/auth/register`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
username,
|
||||
email: `${username}@example.com`,
|
||||
password: TEST_PASSWORD,
|
||||
displayNameEn: "Reg Everyone",
|
||||
preferredLanguage: "en",
|
||||
}),
|
||||
});
|
||||
assert.equal(res.status, 201, `register expected 201, got ${res.status}`);
|
||||
const body = await res.json();
|
||||
assert.ok(Array.isArray(body.groups), "auth user payload must include groups[]");
|
||||
assert.ok(
|
||||
body.groups.some((g) => g.name === "Everyone"),
|
||||
"register response must list Everyone in groups",
|
||||
);
|
||||
|
||||
// Confirm at the DB level that the membership was actually written.
|
||||
const userRow = await pool.query(`SELECT id FROM users WHERE username = $1`, [username]);
|
||||
newUserId = userRow.rows[0]?.id;
|
||||
const linkRow = await pool.query(
|
||||
`SELECT 1 FROM user_groups ug
|
||||
JOIN groups g ON g.id = ug.group_id
|
||||
WHERE ug.user_id = $1 AND g.name = 'Everyone'`,
|
||||
[newUserId],
|
||||
);
|
||||
assert.equal(linkRow.rowCount, 1, "user_groups must link new user to Everyone");
|
||||
} finally {
|
||||
if (newUserId !== undefined) {
|
||||
await pool.query(`DELETE FROM user_groups WHERE user_id = $1`, [newUserId]);
|
||||
await pool.query(`DELETE FROM user_roles WHERE user_id = $1`, [newUserId]);
|
||||
await pool.query(`DELETE FROM users WHERE id = $1`, [newUserId]);
|
||||
}
|
||||
if (priorOpen !== undefined) {
|
||||
await pool.query(
|
||||
`UPDATE app_settings SET registration_open = $1 WHERE id = 1`,
|
||||
[priorOpen],
|
||||
);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
test("GET /api/users supports q, groupId, and status filters", async () => {
|
||||
// q filter on partial username
|
||||
const qRes = await fetch(
|
||||
|
||||
Reference in New Issue
Block a user