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
This commit is contained in:
Cian Johnston
2023-02-22 16:48:16 +00:00
committed by GitHub
parent b412ef0dbb
commit 6149905a83
11 changed files with 45 additions and 3 deletions
+8
View File
@@ -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,
})
+10
View File
@@ -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) {
@@ -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 },
],
}
@@ -8,6 +8,7 @@ export interface TableRowMenuProps<TData> {
data: TData
menuItems: Array<{
label: string
disabled: boolean
onClick: (data: TData) => void
}>
}
@@ -47,6 +48,7 @@ export const TableRowMenu = <T,>({
{menuItems.map((item) => (
<MenuItem
key={item.label}
disabled={item.disabled}
onClick={() => {
handleClose()
item.onClick(data)
@@ -16,6 +16,7 @@ describe("AuditPage", () => {
onResetUserPassword={() => jest.fn()}
onUpdateUserRoles={() => jest.fn()}
isNonInitialPage={false}
actorID="12345678-1234-1234-1234-123456789012"
/>,
)
@@ -33,6 +33,7 @@ export interface UsersTableProps {
roles: TypesGen.Role["name"][],
) => void
isNonInitialPage: boolean
actorID: string
}
export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
@@ -48,6 +49,7 @@ export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
canEditUsers,
isLoading,
isNonInitialPage,
actorID,
}) => {
return (
<TableContainer>
@@ -82,6 +84,7 @@ export const UsersTable: FC<React.PropsWithChildren<UsersTableProps>> = ({
onSuspendUser={onSuspendUser}
onUpdateUserRoles={onUpdateUserRoles}
isNonInitialPage={isNonInitialPage}
actorID={actorID}
/>
</TableBody>
</Table>
@@ -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")
@@ -165,26 +167,31 @@ export const UsersTableBody: FC<
{
label: t("suspendMenuItem"),
onClick: onSuspendUser,
disabled: false,
},
]
: [
{
label: t("activateMenuItem"),
onClick: onActivateUser,
disabled: false,
},
]
).concat(
{
label: t("deleteMenuItem"),
onClick: onDeleteUser,
disabled: user.id === actorID,
},
{
label: t("listWorkspacesMenuItem"),
onClick: onListWorkspaces,
disabled: false,
},
{
label: t("resetPasswordMenuItem"),
onClick: onResetUserPassword,
disabled: false,
},
)
}
+1
View File
@@ -193,6 +193,7 @@ export const GroupPage: React.FC = () => {
userId: member.id,
})
},
disabled: false,
},
]}
/>
@@ -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,
},
]}
/>
+4
View File
@@ -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 (
<>
<Helmet>
@@ -126,6 +129,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => {
}}
paginationRef={paginationRef}
isNonInitialPage={nonInitialPage(searchParams)}
actorID={me.id}
/>
<DeleteDialog
@@ -31,6 +31,7 @@ export interface UsersPageViewProps {
onFilter: (query: string) => void
paginationRef: PaginationMachineRef
isNonInitialPage: boolean
actorID: string
}
export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
@@ -51,6 +52,7 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
onFilter,
paginationRef,
isNonInitialPage,
actorID,
}) => {
const presetFilters = [
{ query: userFilterQuery.active, name: Language.activeUsersFilterName },
@@ -79,6 +81,7 @@ export const UsersPageView: FC<React.PropsWithChildren<UsersPageViewProps>> = ({
canEditUsers={canEditUsers}
isLoading={isLoading}
isNonInitialPage={isNonInitialPage}
actorID={actorID}
/>
<PaginationWidget numRecords={count} paginationRef={paginationRef} />