Skip to content

Commit 6149905

Browse files
johnstcnjohnstcn
authored
fix: disallow deleting self (#6306)
* fix: api: disallow user self-deletion * feat(site): TableRowMenu: allow disabling individual menu items * fix(site): UsersTable: disallow deleting self
1 parent b412ef0 commit 6149905

File tree

11 files changed

+45
-3
lines changed

11 files changed

+45
-3
lines changed

‎coderd/users.go

+8
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,7 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
387387
ctx := r.Context()
388388
auditor := *api.Auditor.Load()
389389
user := httpmw.UserParam(r)
390+
auth := httpmw.UserAuthorization(r)
390391
aReq, commitAudit := audit.InitRequest[database.User](rw, &audit.RequestParams{
391392
Audit: auditor,
392393
Log: api.Logger,
@@ -401,6 +402,13 @@ func (api *API) deleteUser(rw http.ResponseWriter, r *http.Request) {
401402
return
402403
}
403404

405+
if auth.Actor.ID == user.ID.String() {
406+
httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
407+
Message: "You cannot delete yourself!",
408+
})
409+
return
410+
}
411+
404412
workspaces, err := api.Database.GetWorkspaces(ctx, database.GetWorkspacesParams{
405413
OwnerID: user.ID,
406414
})

‎coderd/users_test.go

+10
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,16 @@ func TestDeleteUser(t *testing.T) {
327327
require.ErrorAs(t, err, &apiErr)
328328
require.Equal(t, http.StatusExpectationFailed, apiErr.StatusCode())
329329
})
330+
t.Run("Self", func(t *testing.T) {
331+
t.Parallel()
332+
client := coderdtest.New(t, nil)
333+
user := coderdtest.CreateFirstUser(t, client)
334+
err := client.DeleteUser(context.Background(), user.UserID)
335+
var apiErr *codersdk.Error
336+
require.Error(t, err, "should not be able to delete self")
337+
require.ErrorAs(t, err, &apiErr, "should be a coderd error")
338+
require.Equal(t, http.StatusForbidden, apiErr.StatusCode(), "should be forbidden")
339+
})
330340
}
331341

332342
func TestPostLogout(t *testing.T) {

‎site/src/components/TableRowMenu/TableRowMenu.stories.tsx

+4-3
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@ export const Example = Template.bind({})
1818
Example.args = {
1919
data: { id: "123" },
2020
menuItems: [
21-
{ label: "Suspend", onClick: (data) => alert(data.id) },
22-
{ label: "Update", onClick: (data) => alert(data.id) },
23-
{ label: "Delete", onClick: (data) => alert(data.id) },
21+
{ label: "Suspend", onClick: (data) => alert(data.id), disabled: false },
22+
{ label: "Update", onClick: (data) => alert(data.id), disabled: false },
23+
{ label: "Delete", onClick: (data) => alert(data.id), disabled: false },
24+
{ label: "Explode", onClick: (data) => alert(data.id), disabled: true },
2425
],
2526
}

‎site/src/components/TableRowMenu/TableRowMenu.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export interface TableRowMenuProps<TData> {
88
data: TData
99
menuItems: Array<{
1010
label: string
11+
disabled: boolean
1112
onClick: (data: TData) => void
1213
}>
1314
}
@@ -47,6 +48,7 @@ export const TableRowMenu = <T,>({
4748
{menuItems.map((item) => (
4849
<MenuItem
4950
key={item.label}
51+
disabled={item.disabled}
5052
onClick={() => {
5153
handleClose()
5254
item.onClick(data)

‎site/src/components/UsersTable/UsersTable.test.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ describe("AuditPage", () => {
1616
onResetUserPassword={() => jest.fn()}
1717
onUpdateUserRoles={() => jest.fn()}
1818
isNonInitialPage={false}
19+
actorID="12345678-1234-1234-1234-123456789012"
1920
/>,
2021
)
2122

‎site/src/components/UsersTable/UsersTable.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ export interface UsersTableProps {
3333
roles: TypesGen.Role["name"][],
3434
) => void
3535
isNonInitialPage: boolean
36+
actorID: string
3637
}
3738

3839
export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
@@ -48,6 +49,7 @@ export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
4849
canEditUsers,
4950
isLoading,
5051
isNonInitialPage,
52+
actorID,
5153
}) => {
5254
return (
5355
<TableContainer>
@@ -82,6 +84,7 @@ export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
8284
onSuspendUser={onSuspendUser}
8385
onUpdateUserRoles={onUpdateUserRoles}
8486
isNonInitialPage={isNonInitialPage}
87+
actorID={actorID}
8588
/>
8689
</TableBody>
8790
</Table>

‎site/src/components/UsersTable/UsersTableBody.tsx

+7
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ interface UsersTableBodyProps {
4444
roles: TypesGen.Role["name"][],
4545
) => void
4646
isNonInitialPage: boolean
47+
actorID: string
4748
}
4849

4950
export const UsersTableBody: FC<
@@ -61,6 +62,7 @@ export const UsersTableBody: FC<
6162
canEditUsers,
6263
isLoading,
6364
isNonInitialPage,
65+
actorID,
6466
}) => {
6567
const styles = useStyles()
6668
const { t } = useTranslation("usersPage")
@@ -165,26 +167,31 @@ export const UsersTableBody: FC<
165167
{
166168
label: t("suspendMenuItem"),
167169
onClick: onSuspendUser,
170+
disabled: false,
168171
},
169172
]
170173
: [
171174
{
172175
label: t("activateMenuItem"),
173176
onClick: onActivateUser,
177+
disabled: false,
174178
},
175179
]
176180
).concat(
177181
{
178182
label: t("deleteMenuItem"),
179183
onClick: onDeleteUser,
184+
disabled: user.id === actorID,
180185
},
181186
{
182187
label: t("listWorkspacesMenuItem"),
183188
onClick: onListWorkspaces,
189+
disabled: false,
184190
},
185191
{
186192
label: t("resetPasswordMenuItem"),
187193
onClick: onResetUserPassword,
194+
disabled: false,
188195
},
189196
)
190197
}

‎site/src/pages/GroupsPage/GroupPage.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ export const GroupPage: React.FC = () => {
193193
userId: member.id,
194194
})
195195
},
196+
disabled: false,
196197
},
197198
]}
198199
/>

‎site/src/pages/TemplatePage/TemplatePermissionsPage/TemplatePermissionsPageView.tsx

+2
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ export const TemplatePermissionsPageView: FC<
281281
{
282282
label: "Remove",
283283
onClick: () => onRemoveGroup(group),
284+
disabled: false,
284285
},
285286
]}
286287
/>
@@ -328,6 +329,7 @@ export const TemplatePermissionsPageView: FC<
328329
{
329330
label: "Remove",
330331
onClick: () => onRemoveUser(user),
332+
disabled: false,
331333
},
332334
]}
333335
/>

‎site/src/pages/UsersPage/UsersPage.tsx

+4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
getPaginationContext,
66
nonInitialPage,
77
} from "components/PaginationWidget/utils"
8+
import { useMe } from "hooks/useMe"
89
import { usePermissions } from "hooks/usePermissions"
910
import { FC, ReactNode } from "react"
1011
import { Helmet } from "react-helmet-async"
@@ -70,6 +71,8 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
7071
usersState.matches("gettingUsers") ||
7172
(canEditUsers && rolesState.matches("gettingRoles"))
7273

74+
const me = useMe()
75+
7376
return (
7477
<>
7578
<Helmet>
@@ -126,6 +129,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
126129
}}
127130
paginationRef={paginationRef}
128131
isNonInitialPage={nonInitialPage(searchParams)}
132+
actorID={me.id}
129133
/>
130134

131135
<DeleteDialog

‎site/src/pages/UsersPage/UsersPageView.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ export interface UsersPageViewProps {
3131
onFilter: (query: string) => void
3232
paginationRef: PaginationMachineRef
3333
isNonInitialPage: boolean
34+
actorID: string
3435
}
3536

3637
export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
@@ -51,6 +52,7 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
5152
onFilter,
5253
paginationRef,
5354
isNonInitialPage,
55+
actorID,
5456
}) => {
5557
const presetFilters = [
5658
{ query: userFilterQuery.active, name: Language.activeUsersFilterName },
@@ -79,6 +81,7 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
7981
canEditUsers={canEditUsers}
8082
isLoading={isLoading}
8183
isNonInitialPage={isNonInitialPage}
84+
actorID={actorID}
8285
/>
8386

8487
<PaginationWidget numRecords={count} paginationRef={paginationRef} />

0 commit comments

Comments
 (0)