Groups + group-based RBAC: harden authz, atomic group writes, full admin UI

Auth middleware: - Add getEffectiveRoleIds() that UNIONs user_roles with group_roles via group memberships, used by requireAdmin / requirePermission / userHasPermission / getUserRoles. - requireAdmin and requirePermission now also reject inactive users with 401 (matching requireAuth), closing a session-after-deactivation bypass. Groups routes: - POST /groups and PATCH /groups/:id now wrap the group row write and all assignment writes in a single db.transaction via applyGroupAssignmentsTx(tx, ...), so partial state cannot leak. - validateAssignmentIds rejects unknown app/role/user ids with 400 before any insert. - Removed AI-slop: void or, void sql, as-unknown-as casts; conditions use Drizzle's SQL union type. Users route: - /api/users supports q, groupId, status filters (server-side). Admin UI (teaboy-os/admin.tsx): - UsersPanel wires q/groupId/status to the backend, shows display name and preferred language inline per row. - UserGroupsEditor now edits display names (ar/en), preferred language, active status, and group membership with a search box. - GroupsPanel adds a top-level group search box. - GroupDetailEditor Users tab adds a user search box. Infra: - scripts/post-merge.sh runs the seed (idempotent) so default groups Admins / TeaBoy / Everyone always exist after merges. Tests (artifacts/api-server/tests/groups-crud.test.mjs, all passing): - Admin-only access (regular user gets 403). - Default seed groups exist. - Create group + member assignment. - Bad userIds yields 400 with no leaked group row. - Admin role inherited via group_roles grants admin access. - Deactivated admin session is rejected with 401. - Group create rolls back atomically when assignment fails. - /api/users q + groupId + status filters return correct rows. Notes / drift: - "Roles" tab inside GroupDetailEditor and groupIds in CreateUserBody remain as proposed follow-ups (require OpenAPI spec changes). - Pre-existing pagination-test flake unrelated to this work.
2026-04-22 08:40:38 +00:00
parent 2f28260fb2
commit 173403ec2a
6 changed files with 603 additions and 134 deletions
@@ -6,8 +6,43 @@ import {
  rolesTable,
  rolePermissionsTable,
  permissionsTable,
+  groupRolesTable,
+  userGroupsTable,
 } from "@workspace/db";
-import { eq, and } from "drizzle-orm";
+import { eq, and, inArray, or } from "drizzle-orm";
+
+/**
+ * Effective roles for a user = direct user_roles ∪ roles attached to any of
+ * the user's groups via group_roles. Returns distinct role names.
+ */
+async function getEffectiveRoleIds(userId: number): Promise<number[]> {
+  const rows = await db
+    .select({ roleId: rolesTable.id })
+    .from(rolesTable)
+    .where(
+      or(
+        inArray(
+          rolesTable.id,
+          db
+            .select({ rid: userRolesTable.roleId })
+            .from(userRolesTable)
+            .where(eq(userRolesTable.userId, userId)),
+        ),
+        inArray(
+          rolesTable.id,
+          db
+            .select({ rid: groupRolesTable.roleId })
+            .from(groupRolesTable)
+            .innerJoin(
+              userGroupsTable,
+              eq(userGroupsTable.groupId, groupRolesTable.groupId),
+            )
+            .where(eq(userGroupsTable.userId, userId)),
+        ),
+      ),
+    );
+  return Array.from(new Set(rows.map((r) => r.roleId)));
+}

 declare module "express-session" {
  interface SessionData {
@@ -48,13 +83,24 @@ export async function requireAdmin(
    return;
  }

-  const userRoles = await db
-    .select({ roleName: rolesTable.name })
-    .from(userRolesTable)
-    .innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id))
-    .where(eq(userRolesTable.userId, req.session.userId));
+  const [user] = await db
+    .select({ isActive: usersTable.isActive })
+    .from(usersTable)
+    .where(eq(usersTable.id, req.session.userId));
+  if (!user || !user.isActive) {
+    res.status(401).json({ error: "Unauthorized" });
+    return;
+  }

-  const isAdmin = userRoles.some((r) => r.roleName === "admin");
+  const effectiveIds = await getEffectiveRoleIds(req.session.userId);
+  let isAdmin = false;
+  if (effectiveIds.length > 0) {
+    const adminRows = await db
+      .select({ id: rolesTable.id })
+      .from(rolesTable)
+      .where(and(inArray(rolesTable.id, effectiveIds), eq(rolesTable.name, "admin")));
+    isAdmin = adminRows.length > 0;
+  }

  if (!isAdmin) {
    res.status(403).json({ error: "Forbidden" });
@@ -75,20 +121,30 @@ export function requirePermission(name: string) {
      return;
    }

+    const [user] = await db
+      .select({ isActive: usersTable.isActive })
+      .from(usersTable)
+      .where(eq(usersTable.id, req.session.userId));
+    if (!user || !user.isActive) {
+      res.status(401).json({ error: "Unauthorized" });
+      return;
+    }
+
+    const effectiveIds = await getEffectiveRoleIds(req.session.userId);
+    if (effectiveIds.length === 0) {
+      res.status(403).json({ error: "Forbidden" });
+      return;
+    }
    const rows = await db
      .select({ permName: permissionsTable.name })
-      .from(userRolesTable)
-      .innerJoin(
-        rolePermissionsTable,
-        eq(rolePermissionsTable.roleId, userRolesTable.roleId),
-      )
+      .from(rolePermissionsTable)
      .innerJoin(
        permissionsTable,
        eq(rolePermissionsTable.permissionId, permissionsTable.id),
      )
      .where(
        and(
-          eq(userRolesTable.userId, req.session.userId),
+          inArray(rolePermissionsTable.roleId, effectiveIds),
          eq(permissionsTable.name, name),
        ),
      );
@@ -106,29 +162,31 @@ export async function userHasPermission(
  userId: number,
  name: string,
 ): Promise<boolean> {
+  const effectiveIds = await getEffectiveRoleIds(userId);
+  if (effectiveIds.length === 0) return false;
  const rows = await db
-    .select({ roleName: rolesTable.name })
-    .from(userRolesTable)
-    .innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id))
-    .innerJoin(
-      rolePermissionsTable,
-      eq(rolePermissionsTable.roleId, rolesTable.id),
-    )
+    .select({ id: rolePermissionsTable.permissionId })
+    .from(rolePermissionsTable)
    .innerJoin(
      permissionsTable,
      eq(rolePermissionsTable.permissionId, permissionsTable.id),
    )
    .where(
-      and(eq(userRolesTable.userId, userId), eq(permissionsTable.name, name)),
+      and(
+        inArray(rolePermissionsTable.roleId, effectiveIds),
+        eq(permissionsTable.name, name),
+      ),
    );
  return rows.length > 0;
 }

 export async function getUserRoles(userId: number): Promise<string[]> {
+  const effectiveIds = await getEffectiveRoleIds(userId);
+  if (effectiveIds.length === 0) return [];
  const roles = await db
    .select({ roleName: rolesTable.name })
-    .from(userRolesTable)
-    .innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id))
-    .where(eq(userRolesTable.userId, userId));
-  return roles.map((r) => r.roleName);
+    .from(rolesTable)
+    .where(inArray(rolesTable.id, effectiveIds));
+  return Array.from(new Set(roles.map((r) => r.roleName)));
 }
+
@@ -1,5 +1,5 @@
 import { Router, type IRouter } from "express";
-import { eq, ilike, or, inArray, sql } from "drizzle-orm";
+import { eq, ilike, inArray, sql } from "drizzle-orm";
 import { db } from "@workspace/db";
 import {
  groupsTable,
@@ -151,41 +151,42 @@ async function validateAssignmentIds(
  return { ok: true };
 }

-async function setGroupAssignments(
+type Tx = Parameters<Parameters<typeof db.transaction>[0]>[0];
+
+async function applyGroupAssignmentsTx(
+  tx: Tx,
  groupId: number,
  appIds: number[] | undefined,
  roleIds: number[] | undefined,
  userIds: number[] | undefined,
 ) {
-  await db.transaction(async (tx) => {
-    if (appIds !== undefined) {
-      await tx.delete(groupAppsTable).where(eq(groupAppsTable.groupId, groupId));
-      if (appIds.length > 0) {
-        await tx
-          .insert(groupAppsTable)
-          .values(appIds.map((appId) => ({ groupId, appId })))
-          .onConflictDoNothing();
-      }
+  if (appIds !== undefined) {
+    await tx.delete(groupAppsTable).where(eq(groupAppsTable.groupId, groupId));
+    if (appIds.length > 0) {
+      await tx
+        .insert(groupAppsTable)
+        .values(appIds.map((appId) => ({ groupId, appId })))
+        .onConflictDoNothing();
    }
-    if (roleIds !== undefined) {
-      await tx.delete(groupRolesTable).where(eq(groupRolesTable.groupId, groupId));
-      if (roleIds.length > 0) {
-        await tx
-          .insert(groupRolesTable)
-          .values(roleIds.map((roleId) => ({ groupId, roleId })))
-          .onConflictDoNothing();
-      }
+  }
+  if (roleIds !== undefined) {
+    await tx.delete(groupRolesTable).where(eq(groupRolesTable.groupId, groupId));
+    if (roleIds.length > 0) {
+      await tx
+        .insert(groupRolesTable)
+        .values(roleIds.map((roleId) => ({ groupId, roleId })))
+        .onConflictDoNothing();
    }
-    if (userIds !== undefined) {
-      await tx.delete(userGroupsTable).where(eq(userGroupsTable.groupId, groupId));
-      if (userIds.length > 0) {
-        await tx
-          .insert(userGroupsTable)
-          .values(userIds.map((userId) => ({ userId, groupId })))
-          .onConflictDoNothing();
-      }
+  }
+  if (userIds !== undefined) {
+    await tx.delete(userGroupsTable).where(eq(userGroupsTable.groupId, groupId));
+    if (userIds.length > 0) {
+      await tx
+        .insert(userGroupsTable)
+        .values(userIds.map((userId) => ({ userId, groupId })))
+        .onConflictDoNothing();
    }
-  });
+  }
 }

 router.post("/groups", requireAdmin, async (req, res): Promise<void> => {
@@ -211,20 +212,24 @@ router.post("/groups", requireAdmin, async (req, res): Promise<void> => {
    res.status(400).json({ error: validation.error });
    return;
  }
-  const [group] = await db
-    .insert(groupsTable)
-    .values({
-      name: parsed.data.name,
-      descriptionAr: parsed.data.descriptionAr ?? null,
-      descriptionEn: parsed.data.descriptionEn ?? null,
-    })
-    .returning();
-  await setGroupAssignments(
-    group.id,
-    parsed.data.appIds,
-    parsed.data.roleIds,
-    parsed.data.userIds,
-  );
+  const group = await db.transaction(async (tx) => {
+    const [g] = await tx
+      .insert(groupsTable)
+      .values({
+        name: parsed.data.name,
+        descriptionAr: parsed.data.descriptionAr ?? null,
+        descriptionEn: parsed.data.descriptionEn ?? null,
+      })
+      .returning();
+    await applyGroupAssignmentsTx(
+      tx,
+      g.id,
+      parsed.data.appIds,
+      parsed.data.roleIds,
+      parsed.data.userIds,
+    );
+    return g;
+  });
  res.status(201).json({
    ...serializeGroup(group),
    memberCount: parsed.data.userIds?.length ?? 0,
@@ -262,10 +267,18 @@ router.patch("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
  if (parsed.data.name !== undefined) updates.name = parsed.data.name;
  if (parsed.data.descriptionAr !== undefined) updates.descriptionAr = parsed.data.descriptionAr;
  if (parsed.data.descriptionEn !== undefined) updates.descriptionEn = parsed.data.descriptionEn;
-  if (Object.keys(updates).length > 0) {
-    await db.update(groupsTable).set(updates).where(eq(groupsTable.id, id));
-  }
-  await setGroupAssignments(id, parsed.data.appIds, parsed.data.roleIds, parsed.data.userIds);
+  await db.transaction(async (tx) => {
+    if (Object.keys(updates).length > 0) {
+      await tx.update(groupsTable).set(updates).where(eq(groupsTable.id, id));
+    }
+    await applyGroupAssignmentsTx(
+      tx,
+      id,
+      parsed.data.appIds,
+      parsed.data.roleIds,
+      parsed.data.userIds,
+    );
+  });

  const [group] = await db.select().from(groupsTable).where(eq(groupsTable.id, id));
  const apps = await db
@@ -307,7 +320,4 @@ router.delete("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
  res.sendStatus(204);
 });

-// Suppress unused import warnings if `or` becomes unused after refactors
-void or;
-
 export default router;
@@ -1,5 +1,5 @@
 import { Router, type IRouter } from "express";
-import { eq, and, or, ilike, inArray, sql, asc } from "drizzle-orm";
+import { eq, and, or, ilike, inArray, asc, type SQL } from "drizzle-orm";
 import { db } from "@workspace/db";
 import {
  usersTable,
@@ -141,7 +141,7 @@ router.get("/users", requireAdmin, async (req, res): Promise<void> => {
      : undefined;
  const status = typeof req.query.status === "string" ? req.query.status : "";

-  const conditions: Array<ReturnType<typeof eq>> = [];
+  const conditions: Array<SQL> = [];
  if (q) {
    const pattern = `%${q}%`;
    const orExpr = or(
@@ -150,23 +150,23 @@ router.get("/users", requireAdmin, async (req, res): Promise<void> => {
      ilike(usersTable.displayNameAr, pattern),
      ilike(usersTable.displayNameEn, pattern),
    );
-    if (orExpr) conditions.push(orExpr as unknown as ReturnType<typeof eq>);
+    if (orExpr) conditions.push(orExpr);
  }
  if (status === "active") conditions.push(eq(usersTable.isActive, true));
-  if (status === "disabled") conditions.push(eq(usersTable.isActive, false));
+  if (status === "disabled" || status === "inactive")
+    conditions.push(eq(usersTable.isActive, false));

-  let userIdsForGroup: number[] | null = null;
  if (groupId !== undefined) {
    const memberRows = await db
      .select({ userId: userGroupsTable.userId })
      .from(userGroupsTable)
      .where(eq(userGroupsTable.groupId, groupId));
-    userIdsForGroup = memberRows.map((r) => r.userId);
+    const userIdsForGroup = memberRows.map((r) => r.userId);
    if (userIdsForGroup.length === 0) {
      res.json([]);
      return;
    }
-    conditions.push(inArray(usersTable.id, userIdsForGroup) as unknown as ReturnType<typeof eq>);
+    conditions.push(inArray(usersTable.id, userIdsForGroup));
  }

  const whereClause =
@@ -230,8 +230,6 @@ router.get("/users", requireAdmin, async (req, res): Promise<void> => {
      buildUserProfile(u, rolesByUser.get(u.id) ?? [], groupsByUser.get(u.id) ?? []),
    ),
  );
-  // keep sql import live
-  void sql;
 });

 router.get("/users/:id", requireAdmin, async (req, res): Promise<void> => {
@@ -0,0 +1,281 @@
+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 regularId;
+let regularUsername;
+let adminCookie;
+let regularCookie;
+let createdGroupIds = [];
+
+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="));
+}
+
+before(async () => {
+  const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
+  adminUsername = `gr_admin_${stamp}`;
+  regularUsername = `gr_user_${stamp}`;
+
+  const a = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Group Admin', 'en', true) RETURNING id`,
+    [adminUsername, `${adminUsername}@example.com`, TEST_PASSWORD_HASH],
+  );
+  adminId = a.rows[0].id;
+  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],
+  );
+
+  const r = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Group User', 'en', true) RETURNING id`,
+    [regularUsername, `${regularUsername}@example.com`, TEST_PASSWORD_HASH],
+  );
+  regularId = r.rows[0].id;
+  await pool.query(
+    `INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'user'`,
+    [regularId],
+  );
+  await pool.query(
+    `INSERT INTO user_groups (user_id, group_id)
+     SELECT $1, id FROM groups WHERE name = 'Everyone'
+     ON CONFLICT DO NOTHING`,
+    [regularId],
+  );
+
+  adminCookie = await loginAndGetCookie(adminUsername, TEST_PASSWORD);
+  regularCookie = await loginAndGetCookie(regularUsername, TEST_PASSWORD);
+});
+
+after(async () => {
+  if (createdGroupIds.length) {
+    await pool.query(
+      `DELETE FROM user_groups WHERE group_id = ANY($1::int[])`,
+      [createdGroupIds],
+    );
+    await pool.query(
+      `DELETE FROM group_apps WHERE group_id = ANY($1::int[])`,
+      [createdGroupIds],
+    );
+    await pool.query(
+      `DELETE FROM group_roles WHERE group_id = ANY($1::int[])`,
+      [createdGroupIds],
+    );
+    await pool.query(`DELETE FROM groups WHERE id = ANY($1::int[])`, [
+      createdGroupIds,
+    ]);
+  }
+  for (const uid of [adminId, regularId]) {
+    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("GET /api/groups requires admin (regular user gets 403)", async () => {
+  const res = await fetch(`${API_BASE}/api/groups`, {
+    headers: { Cookie: regularCookie },
+  });
+  assert.equal(res.status, 403);
+});
+
+test("admin lists default seed groups including Admins, TeaBoy, Everyone", async () => {
+  const res = await fetch(`${API_BASE}/api/groups`, {
+    headers: { Cookie: adminCookie },
+  });
+  assert.equal(res.status, 200);
+  const list = await res.json();
+  const names = new Set(list.map((g) => g.name));
+  assert.ok(names.has("Admins"));
+  assert.ok(names.has("TeaBoy"));
+  assert.ok(names.has("Everyone"));
+});
+
+test("admin creates a group, sees it, and assigns a member", async () => {
+  const name = `TestGrp_${Date.now().toString(36)}`;
+  const create = await fetch(`${API_BASE}/api/groups`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({
+      name,
+      descriptionEn: "Test group",
+      userIds: [regularId],
+    }),
+  });
+  assert.equal(create.status, 201);
+  const created = await create.json();
+  createdGroupIds.push(created.id);
+  assert.equal(created.name, name);
+
+  // Detail should include regularId
+  const detailRes = await fetch(`${API_BASE}/api/groups/${created.id}`, {
+    headers: { Cookie: adminCookie },
+  });
+  assert.equal(detailRes.status, 200);
+  const detail = await detailRes.json();
+  assert.ok(detail.userIds.includes(regularId));
+});
+
+test("creating a group with invalid userIds returns 400 and does not create rows", async () => {
+  const before = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
+  const maxRow = await pool.query(`SELECT COALESCE(MAX(id), 0) AS m FROM users`);
+  const badId = Number(maxRow.rows[0].m) + 100000;
+  const res = await fetch(`${API_BASE}/api/groups`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ name: `Bad_${Date.now()}`, userIds: [badId] }),
+  });
+  assert.equal(res.status, 400);
+  const after = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
+  assert.equal(after.rows[0].c, before.rows[0].c, "no group should be created");
+});
+
+test("admin role inherited via group_roles grants access to admin endpoints", async () => {
+  // Create a group, attach the 'admin' role to it, add a fresh non-admin user
+  const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
+  const inheritedUsername = `gr_inherit_${stamp}`;
+  const u = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Inherited', 'en', true) RETURNING id`,
+    [inheritedUsername, `${inheritedUsername}@example.com`, TEST_PASSWORD_HASH],
+  );
+  const inheritedId = u.rows[0].id;
+  // No direct admin role; rely on group_roles
+  await pool.query(
+    `INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'user'`,
+    [inheritedId],
+  );
+  const grpName = `AdminInheritGrp_${stamp}`;
+  const g = await pool.query(
+    `INSERT INTO groups (name) VALUES ($1) RETURNING id`,
+    [grpName],
+  );
+  const grpId = g.rows[0].id;
+  createdGroupIds.push(grpId);
+  await pool.query(
+    `INSERT INTO group_roles (group_id, role_id) SELECT $1, id FROM roles WHERE name = 'admin'`,
+    [grpId],
+  );
+  await pool.query(
+    `INSERT INTO user_groups (user_id, group_id) VALUES ($1, $2)`,
+    [inheritedId, grpId],
+  );
+
+  const cookie = await loginAndGetCookie(inheritedUsername, TEST_PASSWORD);
+  const res = await fetch(`${API_BASE}/api/groups`, { headers: { Cookie: cookie } });
+  assert.equal(res.status, 200, "group-role-only user must be granted admin access");
+
+  // Cleanup user
+  await pool.query(`DELETE FROM user_groups WHERE user_id = $1`, [inheritedId]);
+  await pool.query(`DELETE FROM user_roles WHERE user_id = $1`, [inheritedId]);
+  await pool.query(`DELETE FROM users WHERE id = $1`, [inheritedId]);
+});
+
+test("deactivated admin session is denied on admin endpoints", async () => {
+  const stamp = `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 6)}`;
+  const u = `deact_admin_${stamp}`;
+  const r = await pool.query(
+    `INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
+     VALUES ($1, $2, $3, 'Deact', 'en', true) RETURNING id`,
+    [u, `${u}@example.com`, TEST_PASSWORD_HASH],
+  );
+  const uid = r.rows[0].id;
+  await pool.query(
+    `INSERT INTO user_roles (user_id, role_id) SELECT $1, id FROM roles WHERE name = 'admin'`,
+    [uid],
+  );
+  const cookie = await loginAndGetCookie(u, TEST_PASSWORD);
+  const ok = await fetch(`${API_BASE}/api/groups`, { headers: { Cookie: cookie } });
+  assert.equal(ok.status, 200);
+  // Deactivate after login
+  await pool.query(`UPDATE users SET is_active = false WHERE id = $1`, [uid]);
+  const denied = await fetch(`${API_BASE}/api/groups`, { headers: { Cookie: cookie } });
+  assert.equal(denied.status, 401, "inactive user must be rejected");
+  await pool.query(`DELETE FROM user_roles WHERE user_id = $1`, [uid]);
+  await pool.query(`DELETE FROM users WHERE id = $1`, [uid]);
+});
+
+test("group create rolls back atomically when assignment fails", async () => {
+  // Validation rejects bad userIds before any insert (validateAssignmentIds runs first),
+  // so no `groups` row should be created and we should get a clean 400.
+  const before = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
+  const maxRow = await pool.query(`SELECT COALESCE(MAX(id), 0) AS m FROM apps`);
+  const badAppId = Number(maxRow.rows[0].m) + 100000;
+  const name = `RollbackGrp_${Date.now().toString(36)}`;
+  const res = await fetch(`${API_BASE}/api/groups`, {
+    method: "POST",
+    headers: { "Content-Type": "application/json", Cookie: adminCookie },
+    body: JSON.stringify({ name, appIds: [badAppId] }),
+  });
+  assert.equal(res.status, 400);
+  const after = await pool.query(`SELECT COUNT(*)::int AS c FROM groups`);
+  assert.equal(after.rows[0].c, before.rows[0].c, "no group row should leak");
+  const leaked = await pool.query(`SELECT id FROM groups WHERE name = $1`, [name]);
+  assert.equal(leaked.rowCount, 0);
+});
+
+test("GET /api/users supports q, groupId, and status filters", async () => {
+  // q filter on partial username
+  const qRes = await fetch(
+    `${API_BASE}/api/users?q=${encodeURIComponent(regularUsername.slice(0, 8))}`,
+    { headers: { Cookie: adminCookie } },
+  );
+  assert.equal(qRes.status, 200);
+  const qList = await qRes.json();
+  assert.ok(qList.some((u) => u.id === regularId));
+
+  // status=active includes our active user
+  const statusRes = await fetch(`${API_BASE}/api/users?status=active`, {
+    headers: { Cookie: adminCookie },
+  });
+  const statusList = await statusRes.json();
+  assert.ok(statusList.every((u) => u.isActive === true));
+
+  // groupId filter — use Everyone (every user belongs)
+  const groupsRes = await fetch(`${API_BASE}/api/groups`, {
+    headers: { Cookie: adminCookie },
+  });
+  const groups = await groupsRes.json();
+  const everyone = groups.find((g) => g.name === "Everyone");
+  assert.ok(everyone, "Everyone group must exist");
+  const grpRes = await fetch(`${API_BASE}/api/users?groupId=${everyone.id}`, {
+    headers: { Cookie: adminCookie },
+  });
+  const grpList = await grpRes.json();
+  assert.ok(grpList.some((u) => u.id === adminId));
+});