From 5783a44b63df9a9c3b2cd24216339e61913b0d5a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 22 Feb 2023 08:06:57 -0700 Subject: [PATCH 1/3] fix: api: disallow user self-deletion --- coderd/users.go | 8 ++++++++ coderd/users_test.go | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/coderd/users.go b/coderd/users.go index 75696e1155e2a..10cb90d6a1a4b 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -387,6 +387,7 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() auditor := *api.Auditor.Load() user := httpmw.UserParam(r) + auth := httpmw.UserAuthorization(r) aReq, commitAudit := audit.InitRequest[database.User](rw, &audit.RequestParams{ Audit: auditor, Log: api.Logger, @@ -401,6 +402,13 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) { return } + if auth.Actor.ID == user.ID.String() { + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "You cannot delete yourself!", + }) + return + } + workspaces, err := api.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{ OwnerID: user.ID, }) diff --git a/coderd/users_test.go b/coderd/users_test.go index 7217906391aa4..801aa277f1ca3 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -327,6 +327,16 @@ func TestDeleteUser(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusExpectationFailed, apiErr.StatusCode()) }) + t.Run("Self", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + err := client.DeleteUser(context.Background(), user.UserID) + var apiErr *codersdk.Error + require.Error(t, err, "should not be able to delete self") + require.ErrorAs(t, err, &apiErr, "should be a coderd error") + require.Equal(t, http.StatusForbidden, apiErr.StatusCode(), "should be forbidden") + }) } func TestPostLogout(t *testing.T) { From 936ffecf2dd30f785bcb17d77530dc490181f50d Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 22 Feb 2023 09:17:28 -0700 Subject: [PATCH 2/3] feat(site): TableRowMenu: allow disabling individual menu items --- site/src/components/TableRowMenu/TableRowMenu.stories.tsx | 7 ++++--- site/src/components/TableRowMenu/TableRowMenu.tsx | 2 ++ site/src/components/UsersTable/UsersTableBody.tsx | 4 ++++ site/src/pages/GroupsPage/GroupPage.tsx | 1 + .../TemplatePermissionsPageView.tsx | 2 ++ 5 files changed, 13 insertions(+), 3 deletions(-) diff --git a/site/src/components/TableRowMenu/TableRowMenu.stories.tsx b/site/src/components/TableRowMenu/TableRowMenu.stories.tsx index 77939665562aa..0c8a8c075dc34 100644 --- a/site/src/components/TableRowMenu/TableRowMenu.stories.tsx +++ b/site/src/components/TableRowMenu/TableRowMenu.stories.tsx @@ -18,8 +18,9 @@ export const Example = Template.bind({}) Example.args = { data: { id: "123" }, menuItems: [ - { label: "Suspend", onClick: (data) => alert(data.id) }, - { label: "Update", onClick: (data) => alert(data.id) }, - { label: "Delete", onClick: (data) => alert(data.id) }, + { label: "Suspend", onClick: (data) => alert(data.id), disabled: false }, + { label: "Update", onClick: (data) => alert(data.id), disabled: false }, + { label: "Delete", onClick: (data) => alert(data.id), disabled: false }, + { label: "Explode", onClick: (data) => alert(data.id), disabled: true }, ], } diff --git a/site/src/components/TableRowMenu/TableRowMenu.tsx b/site/src/components/TableRowMenu/TableRowMenu.tsx index 669150aa70cd2..30bc79ce30c1c 100644 --- a/site/src/components/TableRowMenu/TableRowMenu.tsx +++ b/site/src/components/TableRowMenu/TableRowMenu.tsx @@ -8,6 +8,7 @@ export interface TableRowMenuProps { data: TData menuItems: Array<{ label: string + disabled: boolean onClick: (data: TData) => void }> } @@ -47,6 +48,7 @@ export const TableRowMenu = ({ {menuItems.map((item) => ( { handleClose() item.onClick(data) diff --git a/site/src/components/UsersTable/UsersTableBody.tsx b/site/src/components/UsersTable/UsersTableBody.tsx index 93641cd8037ca..efc042be41baa 100644 --- a/site/src/components/UsersTable/UsersTableBody.tsx +++ b/site/src/components/UsersTable/UsersTableBody.tsx @@ -165,12 +165,14 @@ export const UsersTableBody: FC< { label: t("suspendMenuItem"), onClick: onSuspendUser, + disabled: false, }, ] : [ { label: t("activateMenuItem"), onClick: onActivateUser, + disabled: false, }, ] ).concat( @@ -181,10 +183,12 @@ export const UsersTableBody: FC< { label: t("listWorkspacesMenuItem"), onClick: onListWorkspaces, + disabled: false, }, { label: t("resetPasswordMenuItem"), onClick: onResetUserPassword, + disabled: false, }, ) } diff --git a/site/src/pages/GroupsPage/GroupPage.tsx b/site/src/pages/GroupsPage/GroupPage.tsx index 295e1bcf73a60..283e95df71cfb 100644 --- a/site/src/pages/GroupsPage/GroupPage.tsx +++ b/site/src/pages/GroupsPage/GroupPage.tsx @@ -193,6 +193,7 @@ export const GroupPage: React.FC = () => { userId: member.id, }) }, + disabled: false, }, ]} /> diff --git a/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx b/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx index 55cceb033fc27..b61603f784681 100644 --- a/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx +++ b/site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx @@ -281,6 +281,7 @@ export const TemplatePermissionsPageView: FC< { label: "Remove", onClick: () => onRemoveGroup(group), + disabled: false, }, ]} /> @@ -328,6 +329,7 @@ export const TemplatePermissionsPageView: FC< { label: "Remove", onClick: () => onRemoveUser(user), + disabled: false, }, ]} /> From 04a0d8c5ce787c086dd560c6b4b9e4af9cf70f89 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 22 Feb 2023 09:19:33 -0700 Subject: [PATCH 3/3] fix(site): UsersTable: disallow deleting self --- site/src/components/UsersTable/UsersTable.test.tsx | 1 + site/src/components/UsersTable/UsersTable.tsx | 3 +++ site/src/components/UsersTable/UsersTableBody.tsx | 3 +++ site/src/pages/UsersPage/UsersPage.tsx | 4 ++++ site/src/pages/UsersPage/UsersPageView.tsx | 3 +++ 5 files changed, 14 insertions(+) diff --git a/site/src/components/UsersTable/UsersTable.test.tsx b/site/src/components/UsersTable/UsersTable.test.tsx index 21ae5d57d5757..42f2683d449df 100644 --- a/site/src/components/UsersTable/UsersTable.test.tsx +++ b/site/src/components/UsersTable/UsersTable.test.tsx @@ -16,6 +16,7 @@ describe("AuditPage", () => { onResetUserPassword={() => jest.fn()} onUpdateUserRoles={() => jest.fn()} isNonInitialPage={false} + actorID="12345678-1234-1234-1234-123456789012" />, ) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 869d548a41c29..52683c6c230a4 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -33,6 +33,7 @@ export interface UsersTableProps { roles: TypesGen.Role["name"][], ) => void isNonInitialPage: boolean + actorID: string } export const UsersTable: FC> = ({ @@ -48,6 +49,7 @@ export const UsersTable: FC> = ({ canEditUsers, isLoading, isNonInitialPage, + actorID, }) => { return ( @@ -82,6 +84,7 @@ export const UsersTable: FC> = ({ onSuspendUser={onSuspendUser} onUpdateUserRoles={onUpdateUserRoles} isNonInitialPage={isNonInitialPage} + actorID={actorID} /> diff --git a/site/src/components/UsersTable/UsersTableBody.tsx b/site/src/components/UsersTable/UsersTableBody.tsx index efc042be41baa..88b1182cd88bf 100644 --- a/site/src/components/UsersTable/UsersTableBody.tsx +++ b/site/src/components/UsersTable/UsersTableBody.tsx @@ -44,6 +44,7 @@ interface UsersTableBodyProps { roles: TypesGen.Role["name"][], ) => void isNonInitialPage: boolean + actorID: string } export const UsersTableBody: FC< @@ -61,6 +62,7 @@ export const UsersTableBody: FC< canEditUsers, isLoading, isNonInitialPage, + actorID, }) => { const styles = useStyles() const { t } = useTranslation("usersPage") @@ -179,6 +181,7 @@ export const UsersTableBody: FC< { label: t("deleteMenuItem"), onClick: onDeleteUser, + disabled: user.id === actorID, }, { label: t("listWorkspacesMenuItem"), diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index d3ff81d8f3b0f..5d447519f427d 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -5,6 +5,7 @@ import { getPaginationContext, nonInitialPage, } from "components/PaginationWidget/utils" +import { useMe } from "hooks/useMe" import { usePermissions } from "hooks/usePermissions" import { FC, ReactNode } from "react" import { Helmet } from "react-helmet-async" @@ -70,6 +71,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { usersState.matches("gettingUsers") || (canEditUsers && rolesState.matches("gettingRoles")) + const me = useMe() + return ( <> @@ -126,6 +129,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }} paginationRef={paginationRef} isNonInitialPage={nonInitialPage(searchParams)} + actorID={me.id} /> void paginationRef: PaginationMachineRef isNonInitialPage: boolean + actorID: string } export const UsersPageView: FC> = ({ @@ -51,6 +52,7 @@ export const UsersPageView: FC> = ({ onFilter, paginationRef, isNonInitialPage, + actorID, }) => { const presetFilters = [ { query: userFilterQuery.active, name: Language.activeUsersFilterName }, @@ -79,6 +81,7 @@ export const UsersPageView: FC> = ({ canEditUsers={canEditUsers} isLoading={isLoading} isNonInitialPage={isNonInitialPage} + actorID={actorID} />