diff --git a/site/src/components/Navbar/Navbar.test.tsx b/site/src/components/Navbar/Navbar.test.tsx index 25c610de22..3a014d4153 100644 --- a/site/src/components/Navbar/Navbar.test.tsx +++ b/site/src/components/Navbar/Navbar.test.tsx @@ -11,10 +11,10 @@ beforeEach(() => { }) describe("Navbar", () => { - describe("when user has permission to read all users", () => { + describe("when user has permission to update users", () => { it("displays the admin menu", async () => { const checkUserPermissionsSpy = jest.spyOn(API, "checkUserPermissions").mockResolvedValueOnce({ - [checks.readAllUsers]: true, + [checks.updateUsers]: true, }) renderWithAuth() @@ -25,10 +25,10 @@ describe("Navbar", () => { }) }) - describe("when user has NO permission to read all users", () => { + describe("when user has NO permission to update users", () => { it("does not display the admin menu", async () => { const checkUserPermissionsSpy = jest.spyOn(API, "checkUserPermissions").mockResolvedValueOnce({ - [checks.readAllUsers]: false, + [checks.updateUsers]: false, }) renderWithAuth() diff --git a/site/src/components/Navbar/Navbar.tsx b/site/src/components/Navbar/Navbar.tsx index 64ca212dd9..90a3c46767 100644 --- a/site/src/components/Navbar/Navbar.tsx +++ b/site/src/components/Navbar/Navbar.tsx @@ -11,7 +11,7 @@ export const Navbar: React.FC = () => { const permissions = useSelector(xServices.authXService, selectPermissions) // When we have more options in the admin dropdown we may want to check this // for more permissions - const displayAdminDropdown = !!permissions?.readAllUsers + const displayAdminDropdown = !!permissions?.updateUsers const onSignOut = () => authSend("SIGN_OUT") return diff --git a/site/src/components/UsersTable/UsersTable.stories.tsx b/site/src/components/UsersTable/UsersTable.stories.tsx index ddd025b547..bb806ea2f9 100644 --- a/site/src/components/UsersTable/UsersTable.stories.tsx +++ b/site/src/components/UsersTable/UsersTable.stories.tsx @@ -14,6 +14,14 @@ export const Example = Template.bind({}) Example.args = { users: [MockUser, MockUser2], roles: MockSiteRoles, + canEditUsers: false, +} + +export const Editable = Template.bind({}) +Editable.args = { + users: [MockUser, MockUser2], + roles: MockSiteRoles, + canEditUsers: true, } export const Empty = Template.bind({}) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index c961c4b2f9..62075d66d5 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -8,10 +8,8 @@ import React from "react" import * as TypesGen from "../../api/typesGenerated" import { EmptyState } from "../EmptyState/EmptyState" import { RoleSelect } from "../RoleSelect/RoleSelect" -import { TableHeaderRow } from "../TableHeaders/TableHeaders" import { TableLoader } from "../TableLoader/TableLoader" import { TableRowMenu } from "../TableRowMenu/TableRowMenu" -import { TableTitle } from "../TableTitle/TableTitle" import { UserCell } from "../UserCell/UserCell" export const Language = { @@ -28,6 +26,8 @@ export interface UsersTableProps { users?: TypesGen.User[] roles?: TypesGen.Role[] isUpdatingUserRoles?: boolean + canEditUsers?: boolean + isLoading?: boolean onSuspendUser: (user: TypesGen.User) => void onResetUserPassword: (user: TypesGen.User) => void onUpdateUserRoles: (user: TypesGen.User, roles: TypesGen.Role["name"][]) => void @@ -40,52 +40,57 @@ export const UsersTable: React.FC = ({ onResetUserPassword, onUpdateUserRoles, isUpdatingUserRoles, + canEditUsers, + isLoading, }) => { - const isLoading = !users || !roles - return ( - - - {Language.usernameLabel} - {Language.rolesLabel} + + {Language.usernameLabel} + {Language.rolesLabel} {/* 1% is a trick to make the table cell width fit the content */} - - + {canEditUsers && } + {isLoading && } - {users && - roles && + {!isLoading && + users && users.map((u) => ( {" "} - onUpdateUserRoles(u, roles)} - /> - - - + {canEditUsers ? ( + onUpdateUserRoles(u, roles)} + /> + ) : ( + <>{u.roles.map((r) => r.display_name).join(", ")} + )} + {canEditUsers && ( + + + + )} ))} diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 579924832f..b52cf1c69a 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -1,8 +1,9 @@ -import { useActor } from "@xstate/react" +import { useActor, useSelector } from "@xstate/react" import React, { useContext, useEffect } from "react" import { useNavigate } from "react-router" import { ConfirmDialog } from "../../components/ConfirmDialog/ConfirmDialog" import { ResetPasswordDialog } from "../../components/ResetPasswordDialog/ResetPasswordDialog" +import { selectPermissions } from "../../xServices/auth/authSelectors" import { XServiceContext } from "../../xServices/StateContext" import { UsersPageView } from "./UsersPageView" @@ -12,39 +13,38 @@ export const Language = { suspendDialogMessagePrefix: "Do you want to suspend the user", } -const useRoles = () => { - const xServices = useContext(XServiceContext) - const [rolesState, rolesSend] = useActor(xServices.siteRolesXService) - const { roles } = rolesState.context - - /** - * Fetch roles on component mount - */ - useEffect(() => { - rolesSend({ - type: "GET_ROLES", - }) - }, [rolesSend]) - - return roles -} - export const UsersPage: React.FC = () => { const xServices = useContext(XServiceContext) const [usersState, usersSend] = useActor(xServices.usersXService) + const [rolesState, rolesSend] = useActor(xServices.siteRolesXService) const { users, getUsersError, userIdToSuspend, userIdToResetPassword, newUserPassword } = usersState.context const navigate = useNavigate() const userToBeSuspended = users?.find((u) => u.id === userIdToSuspend) const userToResetPassword = users?.find((u) => u.id === userIdToResetPassword) - const roles = useRoles() + const permissions = useSelector(xServices.authXService, selectPermissions) + const canEditUsers = permissions && permissions.updateUsers + const { roles } = rolesState.context + // Is loading if + // - permissions are not loaded or + // - users are not loaded or + // - the user can edit the users but the roles are not loaded yet + const isLoading = !permissions || !users || (canEditUsers && !roles) - /** - * Fetch users on component mount - */ + // Fetch users on component mount useEffect(() => { usersSend("GET_USERS") }, [usersSend]) + // Fetch roles on component mount + useEffect(() => { + // Only fetch the roles if the user has permission for it + if (canEditUsers) { + rolesSend({ + type: "GET_ROLES", + }) + } + }, [canEditUsers, rolesSend]) + return ( <> { }} error={getUsersError} isUpdatingUserRoles={usersState.matches("updatingUserRoles")} + isLoading={isLoading} + canEditUsers={canEditUsers} /> void onSuspendUser: (user: TypesGen.User) => void onResetUserPassword: (user: TypesGen.User) => void @@ -31,6 +33,8 @@ export const UsersPageView: React.FC = ({ onUpdateUserRoles, error, isUpdatingUserRoles, + canEditUsers, + isLoading, }) => { return ( @@ -46,6 +50,8 @@ export const UsersPageView: React.FC = ({ onResetUserPassword={onResetUserPassword} onUpdateUserRoles={onUpdateUserRoles} isUpdatingUserRoles={isUpdatingUserRoles} + canEditUsers={canEditUsers} + isLoading={isLoading} /> )} diff --git a/site/src/xServices/auth/authXService.ts b/site/src/xServices/auth/authXService.ts index 6ca72d406b..deba485615 100644 --- a/site/src/xServices/auth/authXService.ts +++ b/site/src/xServices/auth/authXService.ts @@ -11,6 +11,7 @@ export const Language = { export const checks = { readAllUsers: "readAllUsers", + updateUsers: "updateUsers", createTemplates: "createTemplates", } as const @@ -21,6 +22,12 @@ export const permissionsToCheck = { }, action: "read", }, + [checks.updateUsers]: { + object: { + resource_type: "user", + }, + action: "update", + }, [checks.createTemplates]: { object: { resource_type: "template",