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.
This commit is contained in:
riyadhafraa
2026-04-22 08:40:38 +00:00
parent f5273af19f
commit bf0768948e
6 changed files with 603 additions and 134 deletions
+83 -25
View File
@@ -6,8 +6,43 @@ import {
rolesTable, rolesTable,
rolePermissionsTable, rolePermissionsTable,
permissionsTable, permissionsTable,
groupRolesTable,
userGroupsTable,
} from "@workspace/db"; } 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" { declare module "express-session" {
interface SessionData { interface SessionData {
@@ -48,13 +83,24 @@ export async function requireAdmin(
return; return;
} }
const userRoles = await db const [user] = await db
.select({ roleName: rolesTable.name }) .select({ isActive: usersTable.isActive })
.from(userRolesTable) .from(usersTable)
.innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id)) .where(eq(usersTable.id, req.session.userId));
.where(eq(userRolesTable.userId, 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) { if (!isAdmin) {
res.status(403).json({ error: "Forbidden" }); res.status(403).json({ error: "Forbidden" });
@@ -75,20 +121,30 @@ export function requirePermission(name: string) {
return; 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 const rows = await db
.select({ permName: permissionsTable.name }) .select({ permName: permissionsTable.name })
.from(userRolesTable) .from(rolePermissionsTable)
.innerJoin(
rolePermissionsTable,
eq(rolePermissionsTable.roleId, userRolesTable.roleId),
)
.innerJoin( .innerJoin(
permissionsTable, permissionsTable,
eq(rolePermissionsTable.permissionId, permissionsTable.id), eq(rolePermissionsTable.permissionId, permissionsTable.id),
) )
.where( .where(
and( and(
eq(userRolesTable.userId, req.session.userId), inArray(rolePermissionsTable.roleId, effectiveIds),
eq(permissionsTable.name, name), eq(permissionsTable.name, name),
), ),
); );
@@ -106,29 +162,31 @@ export async function userHasPermission(
userId: number, userId: number,
name: string, name: string,
): Promise<boolean> { ): Promise<boolean> {
const effectiveIds = await getEffectiveRoleIds(userId);
if (effectiveIds.length === 0) return false;
const rows = await db const rows = await db
.select({ roleName: rolesTable.name }) .select({ id: rolePermissionsTable.permissionId })
.from(userRolesTable) .from(rolePermissionsTable)
.innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id))
.innerJoin(
rolePermissionsTable,
eq(rolePermissionsTable.roleId, rolesTable.id),
)
.innerJoin( .innerJoin(
permissionsTable, permissionsTable,
eq(rolePermissionsTable.permissionId, permissionsTable.id), eq(rolePermissionsTable.permissionId, permissionsTable.id),
) )
.where( .where(
and(eq(userRolesTable.userId, userId), eq(permissionsTable.name, name)), and(
inArray(rolePermissionsTable.roleId, effectiveIds),
eq(permissionsTable.name, name),
),
); );
return rows.length > 0; return rows.length > 0;
} }
export async function getUserRoles(userId: number): Promise<string[]> { export async function getUserRoles(userId: number): Promise<string[]> {
const effectiveIds = await getEffectiveRoleIds(userId);
if (effectiveIds.length === 0) return [];
const roles = await db const roles = await db
.select({ roleName: rolesTable.name }) .select({ roleName: rolesTable.name })
.from(userRolesTable) .from(rolesTable)
.innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id)) .where(inArray(rolesTable.id, effectiveIds));
.where(eq(userRolesTable.userId, userId)); return Array.from(new Set(roles.map((r) => r.roleName)));
return roles.map((r) => r.roleName);
} }
+59 -49
View File
@@ -1,5 +1,5 @@
import { Router, type IRouter } from "express"; 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 { db } from "@workspace/db";
import { import {
groupsTable, groupsTable,
@@ -151,41 +151,42 @@ async function validateAssignmentIds(
return { ok: true }; return { ok: true };
} }
async function setGroupAssignments( type Tx = Parameters<Parameters<typeof db.transaction>[0]>[0];
async function applyGroupAssignmentsTx(
tx: Tx,
groupId: number, groupId: number,
appIds: number[] | undefined, appIds: number[] | undefined,
roleIds: number[] | undefined, roleIds: number[] | undefined,
userIds: number[] | undefined, userIds: number[] | undefined,
) { ) {
await db.transaction(async (tx) => { if (appIds !== undefined) {
if (appIds !== undefined) { await tx.delete(groupAppsTable).where(eq(groupAppsTable.groupId, groupId));
await tx.delete(groupAppsTable).where(eq(groupAppsTable.groupId, groupId)); if (appIds.length > 0) {
if (appIds.length > 0) { await tx
await tx .insert(groupAppsTable)
.insert(groupAppsTable) .values(appIds.map((appId) => ({ groupId, appId })))
.values(appIds.map((appId) => ({ groupId, appId }))) .onConflictDoNothing();
.onConflictDoNothing();
}
} }
if (roleIds !== undefined) { }
await tx.delete(groupRolesTable).where(eq(groupRolesTable.groupId, groupId)); if (roleIds !== undefined) {
if (roleIds.length > 0) { await tx.delete(groupRolesTable).where(eq(groupRolesTable.groupId, groupId));
await tx if (roleIds.length > 0) {
.insert(groupRolesTable) await tx
.values(roleIds.map((roleId) => ({ groupId, roleId }))) .insert(groupRolesTable)
.onConflictDoNothing(); .values(roleIds.map((roleId) => ({ groupId, roleId })))
} .onConflictDoNothing();
} }
if (userIds !== undefined) { }
await tx.delete(userGroupsTable).where(eq(userGroupsTable.groupId, groupId)); if (userIds !== undefined) {
if (userIds.length > 0) { await tx.delete(userGroupsTable).where(eq(userGroupsTable.groupId, groupId));
await tx if (userIds.length > 0) {
.insert(userGroupsTable) await tx
.values(userIds.map((userId) => ({ userId, groupId }))) .insert(userGroupsTable)
.onConflictDoNothing(); .values(userIds.map((userId) => ({ userId, groupId })))
} .onConflictDoNothing();
} }
}); }
} }
router.post("/groups", requireAdmin, async (req, res): Promise<void> => { 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 }); res.status(400).json({ error: validation.error });
return; return;
} }
const [group] = await db const group = await db.transaction(async (tx) => {
.insert(groupsTable) const [g] = await tx
.values({ .insert(groupsTable)
name: parsed.data.name, .values({
descriptionAr: parsed.data.descriptionAr ?? null, name: parsed.data.name,
descriptionEn: parsed.data.descriptionEn ?? null, descriptionAr: parsed.data.descriptionAr ?? null,
}) descriptionEn: parsed.data.descriptionEn ?? null,
.returning(); })
await setGroupAssignments( .returning();
group.id, await applyGroupAssignmentsTx(
parsed.data.appIds, tx,
parsed.data.roleIds, g.id,
parsed.data.userIds, parsed.data.appIds,
); parsed.data.roleIds,
parsed.data.userIds,
);
return g;
});
res.status(201).json({ res.status(201).json({
...serializeGroup(group), ...serializeGroup(group),
memberCount: parsed.data.userIds?.length ?? 0, 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.name !== undefined) updates.name = parsed.data.name;
if (parsed.data.descriptionAr !== undefined) updates.descriptionAr = parsed.data.descriptionAr; if (parsed.data.descriptionAr !== undefined) updates.descriptionAr = parsed.data.descriptionAr;
if (parsed.data.descriptionEn !== undefined) updates.descriptionEn = parsed.data.descriptionEn; if (parsed.data.descriptionEn !== undefined) updates.descriptionEn = parsed.data.descriptionEn;
if (Object.keys(updates).length > 0) { await db.transaction(async (tx) => {
await db.update(groupsTable).set(updates).where(eq(groupsTable.id, id)); if (Object.keys(updates).length > 0) {
} await tx.update(groupsTable).set(updates).where(eq(groupsTable.id, id));
await setGroupAssignments(id, parsed.data.appIds, parsed.data.roleIds, parsed.data.userIds); }
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 [group] = await db.select().from(groupsTable).where(eq(groupsTable.id, id));
const apps = await db const apps = await db
@@ -307,7 +320,4 @@ router.delete("/groups/:id", requireAdmin, async (req, res): Promise<void> => {
res.sendStatus(204); res.sendStatus(204);
}); });
// Suppress unused import warnings if `or` becomes unused after refactors
void or;
export default router; export default router;
+7 -9
View File
@@ -1,5 +1,5 @@
import { Router, type IRouter } from "express"; 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 { db } from "@workspace/db";
import { import {
usersTable, usersTable,
@@ -141,7 +141,7 @@ router.get("/users", requireAdmin, async (req, res): Promise<void> => {
: undefined; : undefined;
const status = typeof req.query.status === "string" ? req.query.status : ""; const status = typeof req.query.status === "string" ? req.query.status : "";
const conditions: Array<ReturnType<typeof eq>> = []; const conditions: Array<SQL> = [];
if (q) { if (q) {
const pattern = `%${q}%`; const pattern = `%${q}%`;
const orExpr = or( const orExpr = or(
@@ -150,23 +150,23 @@ router.get("/users", requireAdmin, async (req, res): Promise<void> => {
ilike(usersTable.displayNameAr, pattern), ilike(usersTable.displayNameAr, pattern),
ilike(usersTable.displayNameEn, 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 === "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) { if (groupId !== undefined) {
const memberRows = await db const memberRows = await db
.select({ userId: userGroupsTable.userId }) .select({ userId: userGroupsTable.userId })
.from(userGroupsTable) .from(userGroupsTable)
.where(eq(userGroupsTable.groupId, groupId)); .where(eq(userGroupsTable.groupId, groupId));
userIdsForGroup = memberRows.map((r) => r.userId); const userIdsForGroup = memberRows.map((r) => r.userId);
if (userIdsForGroup.length === 0) { if (userIdsForGroup.length === 0) {
res.json([]); res.json([]);
return; return;
} }
conditions.push(inArray(usersTable.id, userIdsForGroup) as unknown as ReturnType<typeof eq>); conditions.push(inArray(usersTable.id, userIdsForGroup));
} }
const whereClause = 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) ?? []), 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> => { 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));
});
+170 -51
View File
@@ -1,4 +1,4 @@
import { useState, useEffect, useRef, type ReactNode } from "react"; import { useState, useEffect, useRef, useMemo, type ReactNode } from "react";
import { useTranslation } from "react-i18next"; import { useTranslation } from "react-i18next";
import { useLocation } from "wouter"; import { useLocation } from "wouter";
import { import {
@@ -50,7 +50,9 @@ import type {
GetAdminStatsQueryError, GetAdminStatsQueryError,
GetAdminStatsParams, GetAdminStatsParams,
Group, Group,
ListUsersParams,
} from "@workspace/api-client-react"; } from "@workspace/api-client-react";
import { ListUsersStatus } from "@workspace/api-client-react";
import { useQueryClient } from "@tanstack/react-query"; import { useQueryClient } from "@tanstack/react-query";
import { useAuth } from "@/contexts/AuthContext"; import { useAuth } from "@/contexts/AuthContext";
import { useUpload, type UploadResponse } from "@workspace/object-storage-web"; import { useUpload, type UploadResponse } from "@workspace/object-storage-web";
@@ -1966,7 +1968,15 @@ function UsersPanel({
const [newUserOpen, setNewUserOpen] = useState(false); const [newUserOpen, setNewUserOpen] = useState(false);
const [newUserForm, setNewUserForm] = useState({ username: "", email: "", password: "" }); const [newUserForm, setNewUserForm] = useState({ username: "", email: "", password: "" });
const { data: users } = useListUsers(undefined, { query: { queryKey: getListUsersQueryKey() } }); const listParams = useMemo<ListUsersParams>(() => {
const p: ListUsersParams = {};
if (search.trim()) p.q = search.trim();
if (groupFilter !== "all") p.groupId = groupFilter;
if (statusFilter === "inactive") p.status = ListUsersStatus.disabled;
else if (statusFilter === "active") p.status = ListUsersStatus.active;
return p;
}, [search, groupFilter, statusFilter]);
const { data: users } = useListUsers(listParams, { query: { queryKey: getListUsersQueryKey(listParams) } });
const { data: groups } = useListGroups(undefined, { query: { queryKey: getListGroupsQueryKey() } }); const { data: groups } = useListGroups(undefined, { query: { queryKey: getListGroupsQueryKey() } });
const createUser = useCreateUser(); const createUser = useCreateUser();
const updateUser = useUpdateUser(); const updateUser = useUpdateUser();
@@ -1974,27 +1984,17 @@ function UsersPanel({
const addUserRole = useAddUserRole(); const addUserRole = useAddUserRole();
const removeUserRole = useRemoveUserRole(); const removeUserRole = useRemoveUserRole();
const filtered = (users ?? []) const filtered = (users ?? []).slice().sort((a, b) => {
.filter((u) => { const av =
if (search) { sortKey === "createdAt" ? a.createdAt : sortKey === "email" ? a.email : a.username;
const q = search.toLowerCase(); const bv =
if (!u.username.toLowerCase().includes(q) && !u.email.toLowerCase().includes(q)) return false; sortKey === "createdAt" ? b.createdAt : sortKey === "email" ? b.email : b.username;
} const cmp = String(av).localeCompare(String(bv));
if (groupFilter !== "all") { return sortDir === "asc" ? cmp : -cmp;
if (!u.groups?.some((g) => g.id === groupFilter)) return false; });
}
if (statusFilter === "active" && !u.isActive) return false;
if (statusFilter === "inactive" && u.isActive) return false;
return true;
})
.sort((a, b) => {
const av = sortKey === "createdAt" ? a.createdAt : sortKey === "email" ? a.email : a.username;
const bv = sortKey === "createdAt" ? b.createdAt : sortKey === "email" ? b.email : b.username;
const cmp = String(av).localeCompare(String(bv));
return sortDir === "asc" ? cmp : -cmp;
});
const invalidateUsers = () => queryClient.invalidateQueries({ queryKey: getListUsersQueryKey() }); const invalidateUsers = () =>
queryClient.invalidateQueries({ queryKey: [`/api/users`] });
return ( return (
<div className="space-y-3"> <div className="space-y-3">
@@ -2073,15 +2073,29 @@ function UsersPanel({
)} )}
data-testid={`user-row-${u.id}`} data-testid={`user-row-${u.id}`}
> >
<div className="font-medium text-sm text-foreground truncate"> <div className="min-w-0">
{u.username} <div className="font-medium text-sm text-foreground truncate">
{!u.isActive && ( {u.username}
<span className="ms-2 text-[10px] uppercase px-1.5 py-0.5 rounded-full bg-slate-200 text-muted-foreground"> {!u.isActive && (
{t("admin.users.statusInactive")} <span className="ms-2 text-[10px] uppercase px-1.5 py-0.5 rounded-full bg-slate-200 text-muted-foreground">
</span> {t("admin.users.statusInactive")}
</span>
)}
</div>
{(u.displayNameAr || u.displayNameEn) && (
<div className="text-[11px] text-muted-foreground truncate" data-testid={`user-displayname-${u.id}`}>
{u.displayNameAr || u.displayNameEn}
</div>
)}
</div>
<div className="min-w-0">
<div className="text-xs text-muted-foreground truncate">{u.email}</div>
{u.preferredLanguage && (
<div className="text-[11px] uppercase tracking-wide text-muted-foreground" data-testid={`user-lang-${u.id}`}>
{u.preferredLanguage}
</div>
)} )}
</div> </div>
<div className="text-xs text-muted-foreground truncate">{u.email}</div>
<div className="text-xs text-muted-foreground"> <div className="text-xs text-muted-foreground">
{u.createdAt ? new Date(u.createdAt).toLocaleDateString() : "—"} {u.createdAt ? new Date(u.createdAt).toLocaleDateString() : "—"}
</div> </div>
@@ -2222,6 +2236,11 @@ function UserGroupsEditor({
const { toast } = useToast(); const { toast } = useToast();
const updateUser = useUpdateUser(); const updateUser = useUpdateUser();
const [selected, setSelected] = useState<Set<number>>(new Set(user.groups?.map((g) => g.id) ?? [])); const [selected, setSelected] = useState<Set<number>>(new Set(user.groups?.map((g) => g.id) ?? []));
const [displayNameAr, setDisplayNameAr] = useState(user.displayNameAr ?? "");
const [displayNameEn, setDisplayNameEn] = useState(user.displayNameEn ?? "");
const [preferredLanguage, setPreferredLanguage] = useState(user.preferredLanguage || "ar");
const [isActive, setIsActive] = useState(user.isActive);
const [groupSearch, setGroupSearch] = useState("");
const toggle = (id: number) => { const toggle = (id: number) => {
const next = new Set(selected); const next = new Set(selected);
@@ -2230,19 +2249,71 @@ function UserGroupsEditor({
setSelected(next); setSelected(next);
}; };
const visibleGroups = groups.filter((g) =>
!groupSearch.trim() || g.name.toLowerCase().includes(groupSearch.trim().toLowerCase()),
);
return ( return (
<div className="fixed inset-0 bg-slate-900/30 backdrop-blur-sm z-50 flex items-center justify-center p-4"> <div className="fixed inset-0 bg-slate-900/30 backdrop-blur-sm z-50 flex items-center justify-center p-4">
<div className="glass-panel rounded-3xl p-6 w-full max-w-md space-y-3 max-h-[85vh] overflow-y-auto"> <div className="glass-panel rounded-3xl p-6 w-full max-w-md space-y-3 max-h-[85vh] overflow-y-auto">
<h2 className="font-semibold text-foreground">{t("admin.users.editUserGroups", { username: user.username })}</h2> <h2 className="font-semibold text-foreground">{t("admin.users.editUserGroups", { username: user.username })}</h2>
<div className="space-y-1"> <div className="grid grid-cols-2 gap-2">
{groups.map((g) => ( <div className="space-y-1">
<label key={g.id} className="flex items-center gap-2 px-3 py-2 rounded-lg hover:bg-slate-100 cursor-pointer"> <Label className="text-xs">{t("admin.users.col.displayNameAr")}</Label>
<input type="checkbox" checked={selected.has(g.id)} onChange={() => toggle(g.id)} /> <Input
<span className="text-sm flex-1">{g.name}</span> value={displayNameAr}
{g.isSystem && <span className="text-[10px] px-1.5 py-0.5 rounded-full bg-slate-200 text-muted-foreground">{t("admin.groups.system")}</span>} onChange={(e) => setDisplayNameAr(e.target.value)}
</label> className="bg-white/70 border-slate-200"
))} data-testid="edit-user-display-ar"
{groups.length === 0 && <p className="text-sm text-muted-foreground p-2">{t("admin.groups.empty")}</p>} />
</div>
<div className="space-y-1">
<Label className="text-xs">{t("admin.users.col.displayNameEn")}</Label>
<Input
value={displayNameEn}
onChange={(e) => setDisplayNameEn(e.target.value)}
className="bg-white/70 border-slate-200"
data-testid="edit-user-display-en"
/>
</div>
</div>
<div className="grid grid-cols-2 gap-2 items-end">
<div className="space-y-1">
<Label className="text-xs">{t("admin.users.col.language")}</Label>
<select
value={preferredLanguage}
onChange={(e) => setPreferredLanguage(e.target.value)}
className="bg-white/70 border border-slate-200 rounded-lg px-3 py-2 text-sm w-full"
data-testid="edit-user-language"
>
<option value="ar">العربية</option>
<option value="en">English</option>
</select>
</div>
<label className="flex items-center gap-2 pb-2 text-sm">
<Switch checked={isActive} onCheckedChange={setIsActive} />
<span>{t("admin.users.statusActive")}</span>
</label>
</div>
<div className="pt-1">
<Label className="text-xs">{t("admin.users.editGroups")}</Label>
<Input
placeholder={t("admin.groups.searchPlaceholder")}
value={groupSearch}
onChange={(e) => setGroupSearch(e.target.value)}
className="bg-white/70 border-slate-200 mt-1 mb-2"
data-testid="edit-user-group-search"
/>
<div className="space-y-1 max-h-48 overflow-y-auto">
{visibleGroups.map((g) => (
<label key={g.id} className="flex items-center gap-2 px-3 py-2 rounded-lg hover:bg-slate-100 cursor-pointer">
<input type="checkbox" checked={selected.has(g.id)} onChange={() => toggle(g.id)} />
<span className="text-sm flex-1">{g.name}</span>
{g.isSystem && <span className="text-[10px] px-1.5 py-0.5 rounded-full bg-slate-200 text-muted-foreground">{t("admin.groups.system")}</span>}
</label>
))}
{visibleGroups.length === 0 && <p className="text-sm text-muted-foreground p-2">{t("admin.groups.empty")}</p>}
</div>
</div> </div>
<div className="flex gap-2 pt-2"> <div className="flex gap-2 pt-2">
<Button variant="outline" className="flex-1" onClick={onClose}>{t("common.cancel")}</Button> <Button variant="outline" className="flex-1" onClick={onClose}>{t("common.cancel")}</Button>
@@ -2250,7 +2321,16 @@ function UserGroupsEditor({
className="flex-1" className="flex-1"
onClick={() => onClick={() =>
updateUser.mutate( updateUser.mutate(
{ id: user.id, data: { groupIds: Array.from(selected) } }, {
id: user.id,
data: {
groupIds: Array.from(selected),
displayNameAr: displayNameAr.trim() || null,
displayNameEn: displayNameEn.trim() || null,
preferredLanguage,
isActive,
},
},
{ {
onSuccess: () => { toast({ title: t("common.success") }); onSaved(); }, onSuccess: () => { toast({ title: t("common.success") }); onSaved(); },
onError: () => toast({ title: t("common.error"), variant: "destructive" }), onError: () => toast({ title: t("common.error"), variant: "destructive" }),
@@ -2277,12 +2357,27 @@ function GroupsPanel() {
const [editingGroupId, setEditingGroupId] = useState<number | null>(null); const [editingGroupId, setEditingGroupId] = useState<number | null>(null);
const [createOpen, setCreateOpen] = useState(false); const [createOpen, setCreateOpen] = useState(false);
const [createForm, setCreateForm] = useState({ name: "", descriptionAr: "", descriptionEn: "" }); const [createForm, setCreateForm] = useState({ name: "", descriptionAr: "", descriptionEn: "" });
const [groupSearch, setGroupSearch] = useState("");
const invalidate = () => queryClient.invalidateQueries({ queryKey: getListGroupsQueryKey() }); const invalidate = () => queryClient.invalidateQueries({ queryKey: getListGroupsQueryKey() });
const visibleGroups = (groups ?? []).filter((g) =>
!groupSearch.trim() ||
g.name.toLowerCase().includes(groupSearch.trim().toLowerCase()) ||
(g.descriptionAr ?? "").toLowerCase().includes(groupSearch.trim().toLowerCase()) ||
(g.descriptionEn ?? "").toLowerCase().includes(groupSearch.trim().toLowerCase()),
);
return ( return (
<div className="space-y-3"> <div className="space-y-3">
<div className="flex justify-end"> <div className="flex items-center gap-2">
<Input
placeholder={t("admin.groups.searchPlaceholder")}
value={groupSearch}
onChange={(e) => setGroupSearch(e.target.value)}
className="bg-white/70 border-slate-200 flex-1"
data-testid="group-search"
/>
<Button size="sm" onClick={() => { setCreateForm({ name: "", descriptionAr: "", descriptionEn: "" }); setCreateOpen(true); }} data-testid="create-group-btn"> <Button size="sm" onClick={() => { setCreateForm({ name: "", descriptionAr: "", descriptionEn: "" }); setCreateOpen(true); }} data-testid="create-group-btn">
<Plus size={14} className="me-1" /> <Plus size={14} className="me-1" />
{t("admin.groups.addGroup")} {t("admin.groups.addGroup")}
@@ -2290,7 +2385,7 @@ function GroupsPanel() {
</div> </div>
<div className="grid gap-3 md:grid-cols-2"> <div className="grid gap-3 md:grid-cols-2">
{groups?.map((g) => ( {visibleGroups.map((g) => (
<div key={g.id} className="glass-panel rounded-2xl p-4 space-y-2" data-testid={`group-card-${g.id}`}> <div key={g.id} className="glass-panel rounded-2xl p-4 space-y-2" data-testid={`group-card-${g.id}`}>
<div className="flex items-start justify-between gap-2"> <div className="flex items-start justify-between gap-2">
<div className="min-w-0"> <div className="min-w-0">
@@ -2346,7 +2441,7 @@ function GroupsPanel() {
</div> </div>
</div> </div>
))} ))}
{(!groups || groups.length === 0) && ( {visibleGroups.length === 0 && (
<div className="glass-panel rounded-2xl p-6 text-center text-sm text-muted-foreground md:col-span-2"> <div className="glass-panel rounded-2xl p-6 text-center text-sm text-muted-foreground md:col-span-2">
{t("admin.groups.empty")} {t("admin.groups.empty")}
</div> </div>
@@ -2414,6 +2509,18 @@ function GroupDetailEditor({ groupId, onClose, onSaved }: { groupId: number; onC
const [appIds, setAppIds] = useState<Set<number>>(new Set()); const [appIds, setAppIds] = useState<Set<number>>(new Set());
const [userIds, setUserIds] = useState<Set<number>>(new Set()); const [userIds, setUserIds] = useState<Set<number>>(new Set());
const [tab, setTab] = useState<"info" | "apps" | "users">("info"); const [tab, setTab] = useState<"info" | "apps" | "users">("info");
const [userSearch, setUserSearch] = useState("");
const visibleUsers = (users ?? []).filter((u) => {
const q = userSearch.trim().toLowerCase();
if (!q) return true;
return (
u.username.toLowerCase().includes(q) ||
u.email.toLowerCase().includes(q) ||
(u.displayNameAr ?? "").toLowerCase().includes(q) ||
(u.displayNameEn ?? "").toLowerCase().includes(q)
);
});
useEffect(() => { useEffect(() => {
if (group) { if (group) {
@@ -2504,15 +2611,27 @@ function GroupDetailEditor({ groupId, onClose, onSaved }: { groupId: number; onC
)} )}
{tab === "users" && ( {tab === "users" && (
<div className="space-y-1 max-h-72 overflow-y-auto"> <div className="space-y-2">
<p className="text-xs text-muted-foreground px-1 pb-2">{t("admin.groups.usersHint")}</p> <p className="text-xs text-muted-foreground px-1">{t("admin.groups.usersHint")}</p>
{users?.map((u) => ( <Input
<label key={u.id} className="flex items-center gap-2 px-3 py-2 rounded-lg hover:bg-slate-100 cursor-pointer"> placeholder={t("admin.users.searchPlaceholder")}
<input type="checkbox" checked={userIds.has(u.id)} onChange={() => toggle(userIds, u.id, setUserIds)} /> value={userSearch}
<span className="text-sm flex-1">{u.username}</span> onChange={(e) => setUserSearch(e.target.value)}
<span className="text-[10px] text-muted-foreground">{u.email}</span> className="bg-white/70 border-slate-200"
</label> data-testid="group-user-search"
))} />
<div className="space-y-1 max-h-60 overflow-y-auto">
{visibleUsers.map((u) => (
<label key={u.id} className="flex items-center gap-2 px-3 py-2 rounded-lg hover:bg-slate-100 cursor-pointer">
<input type="checkbox" checked={userIds.has(u.id)} onChange={() => toggle(userIds, u.id, setUserIds)} />
<span className="text-sm flex-1">{u.username}</span>
<span className="text-[10px] text-muted-foreground">{u.email}</span>
</label>
))}
{visibleUsers.length === 0 && (
<p className="text-sm text-muted-foreground p-2">{t("admin.users.empty")}</p>
)}
</div>
</div> </div>
)} )}
+3
View File
@@ -2,4 +2,7 @@
set -e set -e
pnpm install --frozen-lockfile pnpm install --frozen-lockfile
pnpm --filter db run push-force pnpm --filter db run push-force
# Idempotent: ensure the default groups exist and existing users are mapped
# (Admins, TeaBoy, Everyone). Safe to run repeatedly.
pnpm --filter scripts run seed
pnpm --filter @workspace/teaboy-os exec playwright install chromium pnpm --filter @workspace/teaboy-os exec playwright install chromium