diff --git a/artifacts/api-server/src/middlewares/auth.ts b/artifacts/api-server/src/middlewares/auth.ts index 38a079d0..cbae1daa 100644 --- a/artifacts/api-server/src/middlewares/auth.ts +++ b/artifacts/api-server/src/middlewares/auth.ts @@ -190,6 +190,20 @@ export async function getUserRoles(userId: number): Promise { return Array.from(new Set(roles.map((r) => r.roleName))); } +// Roles assigned directly to the user (user_roles table only), excluding any +// roles inherited via group membership. The admin Users page contrasts this +// with the effective set so a toggle for an inherited-only role can be +// disabled — otherwise removing the direct row (which doesn't exist) is a +// no-op and the Switch silently flips back on after refetch. +export async function getDirectUserRoles(userId: number): Promise { + const rows = await db + .select({ roleName: rolesTable.name }) + .from(userRolesTable) + .innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id)) + .where(eq(userRolesTable.userId, userId)); + return Array.from(new Set(rows.map((r) => r.roleName))); +} + const EXECUTIVE_READ_ROLES: ReadonlyArray = [ "admin", "executive_ceo", diff --git a/artifacts/api-server/src/routes/users.ts b/artifacts/api-server/src/routes/users.ts index c9537854..d61b7efc 100644 --- a/artifacts/api-server/src/routes/users.ts +++ b/artifacts/api-server/src/routes/users.ts @@ -6,13 +6,14 @@ import { userRolesTable, rolesTable, groupsTable, + groupRolesTable, userGroupsTable, notesTable, serviceOrdersTable, servicesTable, auditLogsTable, } from "@workspace/db"; -import { requireAuth, requireAdmin, getUserRoles } from "../middlewares/auth"; +import { requireAuth, requireAdmin, getUserRoles, getDirectUserRoles } from "../middlewares/auth"; import { emitAppsChangedToUsers } from "../lib/realtime"; import { listPermissionAudit, @@ -77,6 +78,7 @@ type DependencyCounts = { function buildUserProfile( user: typeof usersTable.$inferSelect, roles: string[], + directRoles: string[], groups: GroupSummary[], counts?: DependencyCounts, ) { @@ -92,6 +94,7 @@ function buildUserProfile( avatarUrl: user.avatarUrl, isActive: user.isActive, roles, + directRoles, groups, createdAt: user.createdAt, ...(counts ?? {}), @@ -174,7 +177,7 @@ router.post("/users", requireAdmin, async (req, res): Promise => { }); const groups = await getGroupsForUser(user.id); - res.status(201).json(buildUserProfile(user, [], groups)); + res.status(201).json(buildUserProfile(user, [], [], groups)); }); router.get("/users", requireAdmin, async (req, res): Promise => { @@ -232,17 +235,38 @@ router.get("/users", requireAdmin, async (req, res): Promise => { const userIds = users.map((u) => u.id); - // Batch load roles - const roleRows = await db + // Batch load direct roles (user_roles table only). + const directRoleRows = await db .select({ userId: userRolesTable.userId, roleName: rolesTable.name }) .from(userRolesTable) .innerJoin(rolesTable, eq(userRolesTable.roleId, rolesTable.id)) .where(inArray(userRolesTable.userId, userIds)); - const rolesByUser = new Map(); - for (const r of roleRows) { - const list = rolesByUser.get(r.userId) ?? []; + const directRolesByUser = new Map(); + for (const r of directRoleRows) { + const list = directRolesByUser.get(r.userId) ?? []; list.push(r.roleName); - rolesByUser.set(r.userId, list); + directRolesByUser.set(r.userId, list); + } + + // Batch load roles inherited via group membership so the effective `roles` + // field matches GET /users/:id and the auth middleware. Without this the + // admin Users list shows direct-only roles while the actual permission + // check uses the effective set — confusing when a user receives orders + // via a group but the row in the list shows no order_receiver role. + const inheritedRoleRows = await db + .select({ userId: userGroupsTable.userId, roleName: rolesTable.name }) + .from(userGroupsTable) + .innerJoin(groupRolesTable, eq(groupRolesTable.groupId, userGroupsTable.groupId)) + .innerJoin(rolesTable, eq(groupRolesTable.roleId, rolesTable.id)) + .where(inArray(userGroupsTable.userId, userIds)); + const rolesByUser = new Map>(); + for (const [uid, names] of directRolesByUser) { + rolesByUser.set(uid, new Set(names)); + } + for (const r of inheritedRoleRows) { + const set = rolesByUser.get(r.userId) ?? new Set(); + set.add(r.roleName); + rolesByUser.set(r.userId, set); } // Batch load groups @@ -297,7 +321,8 @@ router.get("/users", requireAdmin, async (req, res): Promise => { users.map((u) => buildUserProfile( u, - rolesByUser.get(u.id) ?? [], + Array.from(rolesByUser.get(u.id) ?? []), + directRolesByUser.get(u.id) ?? [], groupsByUser.get(u.id) ?? [], { noteCount: noteMap.get(u.id) ?? 0, @@ -475,8 +500,9 @@ router.get("/users/:id", requireAdmin, async (req, res): Promise => { } const roles = await getUserRoles(user.id); + const directRoles = await getDirectUserRoles(user.id); const groups = await getGroupsForUser(user.id); - res.json(buildUserProfile(user, roles, groups)); + res.json(buildUserProfile(user, roles, directRoles, groups)); }); router.patch("/users/:id", requireAdmin, async (req, res): Promise => { @@ -584,8 +610,9 @@ router.patch("/users/:id", requireAdmin, async (req, res): Promise => { } const roles = await getUserRoles(user.id); + const directRoles = await getDirectUserRoles(user.id); const groups = await getGroupsForUser(user.id); - res.json(buildUserProfile(user, roles, groups)); + res.json(buildUserProfile(user, roles, directRoles, groups)); }); router.post("/users/:id/roles", requireAdmin, async (req, res): Promise => { @@ -647,8 +674,9 @@ router.post("/users/:id/roles", requireAdmin, async (req, res): Promise => } const roles = await getUserRoles(user.id); + const directRoles = await getDirectUserRoles(user.id); const groups = await getGroupsForUser(user.id); - res.json(buildUserProfile(user, roles, groups)); + res.json(buildUserProfile(user, roles, directRoles, groups)); }); router.delete( @@ -715,8 +743,9 @@ router.delete( } const roles = await getUserRoles(user.id); + const directRoles = await getDirectUserRoles(user.id); const groups = await getGroupsForUser(user.id); - res.json(buildUserProfile(user, roles, groups)); + res.json(buildUserProfile(user, roles, directRoles, groups)); }, ); diff --git a/artifacts/tx-os/src/locales/ar.json b/artifacts/tx-os/src/locales/ar.json index 40f7a24c..9c06afde 100644 --- a/artifacts/tx-os/src/locales/ar.json +++ b/artifacts/tx-os/src/locales/ar.json @@ -521,6 +521,7 @@ "active": "نشط", "issueResetLink": "إصدار رابط إعادة تعيين كلمة المرور", "orderReceiverRole": "مستلم الطلبات", + "roleInheritedFromGroup": "موروث من المجموعة", "resetLinkModal": { "title": "رابط إعادة تعيين كلمة المرور للمستخدم {{username}}", "description": "شارك هذا الرابط لمرة واحدة مع المستخدم. يمكن استخدامه مرة واحدة فقط.", diff --git a/artifacts/tx-os/src/locales/en.json b/artifacts/tx-os/src/locales/en.json index 55d706af..1d87c4dc 100644 --- a/artifacts/tx-os/src/locales/en.json +++ b/artifacts/tx-os/src/locales/en.json @@ -497,6 +497,7 @@ "active": "Active", "issueResetLink": "Issue password reset link", "orderReceiverRole": "Order Receiver", + "roleInheritedFromGroup": "Inherited from group", "resetLinkModal": { "title": "Password reset link for {{username}}", "description": "Share this one-time link with the user. It can only be used once.", diff --git a/artifacts/tx-os/src/pages/admin.tsx b/artifacts/tx-os/src/pages/admin.tsx index af73cdd1..448e8af4 100644 --- a/artifacts/tx-os/src/pages/admin.tsx +++ b/artifacts/tx-os/src/pages/admin.tsx @@ -4355,20 +4355,46 @@ function UsersPanel({ > - + {(() => { + // The Switch toggles the DIRECT user_roles row only. If + // the role is inherited via a group (present in `roles` + // but not in `directRoles`), removing the direct row is a + // no-op and the Switch would flip back on after refetch. + // Disable it and show which group grants the role so the + // admin edits group membership instead. + const hasEffective = u.roles?.includes("order_receiver") ?? false; + const hasDirect = u.directRoles?.includes("order_receiver") ?? false; + const isInherited = hasEffective && !hasDirect; + return ( + + ); + })()} {u.id !== currentUserId && (