mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-08-19 09:30:21 +00:00
fix(ui): show delete user action only for the current user (#11447)
Co-authored-by: Pepe Fagoaga <pepe@prowler.com>
This commit is contained in:
co-authored by
Pepe Fagoaga
parent
e60a4462e5
commit
eb7949c884
@@ -47,7 +47,11 @@ Follow these steps to remove a user of your account:
|
||||
1. Navigate to **Users** from the side menu.
|
||||
2. Click the delete button of your current user.
|
||||
|
||||
> **Note: Each user will be able to delete himself and not others, regardless of his permissions.**
|
||||
> **Note: Each user can only delete their own account, regardless of their permissions. For this reason, the delete button is only shown on your own row and not on other users' rows.**
|
||||
|
||||
Deleting a user removes the **entire user account** from Prowler, not just its membership in your organization. Because a single account can belong to more than one tenant, allowing one administrator to delete it outright could affect organizations they don't manage and irreversibly remove another person's identity. To keep this destructive action under the control of the account owner, the API only permits a user to delete themselves (it rejects any other target with a `400` response), and the UI mirrors this by showing the delete button exclusively on your own row.
|
||||
|
||||
To remove **another** user from your organization, use the [_Expel from organization_](/user-guide/tutorials/prowler-app-multi-tenant#expelling-a-user-from-an-organization) action instead. Expelling removes the user's membership, role grants, and active sessions for your tenant only, and deletes the underlying account just for that user if your organization was their last remaining membership. This action is reserved for tenant **owners**.
|
||||
|
||||
<img src="/images/prowler-app/rbac/user_remove.png" alt="Remove User" width="700" />
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ All notable changes to the **Prowler UI** are documented in this file.
|
||||
### 🐞 Fixed
|
||||
|
||||
- Add Provider modal now closes without reloading the providers page [(#11424)](https://github.com/prowler-cloud/prowler/pull/11424)
|
||||
- Users page now shows the "Delete User" action only on the current user's row, matching the backend rule that a user can only delete their own account [(#11447)](https://github.com/prowler-cloud/prowler/pull/11447)
|
||||
|
||||
### 🔐 Security
|
||||
|
||||
|
||||
@@ -109,6 +109,9 @@ const SSRDataTable = async ({
|
||||
roles,
|
||||
canBeExpelled,
|
||||
currentTenantId: canBeExpelled ? currentTenantId : undefined,
|
||||
// Users may only delete their own account; gate the delete action so the
|
||||
// UI matches the backend rule and never offers an action that would fail.
|
||||
isCurrentUser: user.id === currentUserId,
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,159 @@
|
||||
import { Row } from "@tanstack/react-table";
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
|
||||
// The forms pull in server actions (`@/actions/users/users`) that can't run in
|
||||
// jsdom, so stub them with identifiable markers to assert which modal opens.
|
||||
vi.mock("../forms", () => ({
|
||||
DeleteForm: ({ userId }: { userId: string }) => (
|
||||
<div data-testid="delete-form">delete-form:{userId}</div>
|
||||
),
|
||||
EditForm: ({ userId }: { userId: string }) => (
|
||||
<div data-testid="edit-form">edit-form:{userId}</div>
|
||||
),
|
||||
ExpelUserForm: ({ userId }: { userId: string }) => (
|
||||
<div data-testid="expel-form">expel-form:{userId}</div>
|
||||
),
|
||||
}));
|
||||
|
||||
import { DataTableRowActions } from "./data-table-row-actions";
|
||||
|
||||
interface RowOptions {
|
||||
id?: string;
|
||||
isCurrentUser?: boolean;
|
||||
canBeExpelled?: boolean;
|
||||
currentTenantId?: string;
|
||||
}
|
||||
|
||||
const createRow = ({
|
||||
id = "user-1",
|
||||
isCurrentUser,
|
||||
canBeExpelled,
|
||||
currentTenantId,
|
||||
}: RowOptions = {}) =>
|
||||
({
|
||||
original: {
|
||||
id,
|
||||
attributes: {
|
||||
name: "Jane Doe",
|
||||
email: "jane@example.com",
|
||||
company_name: "Acme",
|
||||
role: { name: "admin" },
|
||||
},
|
||||
isCurrentUser,
|
||||
canBeExpelled,
|
||||
currentTenantId,
|
||||
},
|
||||
}) as unknown as Row<{ id: string }>;
|
||||
|
||||
const openMenu = async (user: ReturnType<typeof userEvent.setup>) => {
|
||||
await user.click(screen.getByRole("button", { name: "Open actions menu" }));
|
||||
};
|
||||
|
||||
describe("DataTableRowActions (users)", () => {
|
||||
it("always renders the Edit User action", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<DataTableRowActions row={createRow()} />);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
expect(screen.getByText("Edit User")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows Delete User only for the current user's row", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<DataTableRowActions row={createRow({ isCurrentUser: true })} />);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
expect(screen.getByText("Delete User")).toBeInTheDocument();
|
||||
expect(screen.getByText("Danger zone")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does NOT show Delete User for another user's row", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<DataTableRowActions row={createRow({ isCurrentUser: false })} />);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
expect(screen.queryByText("Delete User")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("does NOT show Delete User when isCurrentUser is undefined", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<DataTableRowActions row={createRow({})} />);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
expect(screen.queryByText("Delete User")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("hides the Danger zone entirely when the user can neither be deleted nor expelled", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<DataTableRowActions
|
||||
row={createRow({ isCurrentUser: false, canBeExpelled: false })}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
// Only the non-destructive Edit action remains.
|
||||
expect(screen.getByText("Edit User")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Danger zone")).not.toBeInTheDocument();
|
||||
expect(screen.queryByText("Delete User")).not.toBeInTheDocument();
|
||||
expect(
|
||||
screen.queryByText("Expel from organization"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("shows Expel but not Delete User for an expellable, non-current user", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<DataTableRowActions
|
||||
row={createRow({
|
||||
isCurrentUser: false,
|
||||
canBeExpelled: true,
|
||||
currentTenantId: "tenant-1",
|
||||
})}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
expect(screen.getByText("Danger zone")).toBeInTheDocument();
|
||||
expect(screen.getByText("Expel from organization")).toBeInTheDocument();
|
||||
expect(screen.queryByText("Delete User")).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders Delete User with destructive styling", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(<DataTableRowActions row={createRow({ isCurrentUser: true })} />);
|
||||
|
||||
await openMenu(user);
|
||||
|
||||
const menuItem = screen
|
||||
.getByText("Delete User")
|
||||
.closest("[role='menuitem']");
|
||||
expect(menuItem).toBeInTheDocument();
|
||||
expect(menuItem).toHaveClass("text-text-error-primary");
|
||||
});
|
||||
|
||||
it("opens the delete confirmation modal when Delete User is selected", async () => {
|
||||
const user = userEvent.setup();
|
||||
render(
|
||||
<DataTableRowActions
|
||||
row={createRow({ id: "user-42", isCurrentUser: true })}
|
||||
/>,
|
||||
);
|
||||
|
||||
await openMenu(user);
|
||||
await user.click(screen.getByText("Delete User"));
|
||||
|
||||
expect(screen.getByText("Are you absolutely sure?")).toBeInTheDocument();
|
||||
expect(screen.getByTestId("delete-form")).toHaveTextContent(
|
||||
"delete-form:user-42",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -29,6 +29,7 @@ interface UserRowData {
|
||||
attributes?: UserRowAttributes;
|
||||
canBeExpelled?: boolean;
|
||||
currentTenantId?: string;
|
||||
isCurrentUser?: boolean;
|
||||
}
|
||||
|
||||
interface DataTableRowActionsProps<UserProps extends UserRowData> {
|
||||
@@ -57,6 +58,10 @@ export function DataTableRowActions<UserProps extends UserRowData>({
|
||||
row.original.canBeExpelled === true && !!row.original.currentTenantId;
|
||||
const currentTenantId = row.original.currentTenantId;
|
||||
|
||||
// A user can only delete their own account (enforced by the backend), so the
|
||||
// delete action is shown exclusively for the current user's row.
|
||||
const canDeleteUser = row.original.isCurrentUser === true;
|
||||
|
||||
return (
|
||||
<>
|
||||
<Modal
|
||||
@@ -74,14 +79,16 @@ export function DataTableRowActions<UserProps extends UserRowData>({
|
||||
setIsOpen={setIsEditOpen}
|
||||
/>
|
||||
</Modal>
|
||||
<Modal
|
||||
open={isDeleteOpen}
|
||||
onOpenChange={setIsDeleteOpen}
|
||||
title="Are you absolutely sure?"
|
||||
description="This action cannot be undone. This will permanently delete your user account and remove your data from the server."
|
||||
>
|
||||
<DeleteForm userId={userId} setIsOpen={setIsDeleteOpen} />
|
||||
</Modal>
|
||||
{canDeleteUser && (
|
||||
<Modal
|
||||
open={isDeleteOpen}
|
||||
onOpenChange={setIsDeleteOpen}
|
||||
title="Are you absolutely sure?"
|
||||
description="This action cannot be undone. This will permanently delete your user account and remove your data from the server."
|
||||
>
|
||||
<DeleteForm userId={userId} setIsOpen={setIsDeleteOpen} />
|
||||
</Modal>
|
||||
)}
|
||||
{canExpelUser && currentTenantId && (
|
||||
<Modal
|
||||
open={isExpelOpen}
|
||||
@@ -104,22 +111,26 @@ export function DataTableRowActions<UserProps extends UserRowData>({
|
||||
label="Edit User"
|
||||
onSelect={() => setIsEditOpen(true)}
|
||||
/>
|
||||
<ActionDropdownDangerZone>
|
||||
{canExpelUser && (
|
||||
<ActionDropdownItem
|
||||
icon={<UserMinus aria-hidden="true" />}
|
||||
label="Expel from organization"
|
||||
destructive
|
||||
onSelect={() => setIsExpelOpen(true)}
|
||||
/>
|
||||
)}
|
||||
<ActionDropdownItem
|
||||
icon={<Trash2 aria-hidden="true" />}
|
||||
label="Delete User"
|
||||
destructive
|
||||
onSelect={() => setIsDeleteOpen(true)}
|
||||
/>
|
||||
</ActionDropdownDangerZone>
|
||||
{(canExpelUser || canDeleteUser) && (
|
||||
<ActionDropdownDangerZone>
|
||||
{canExpelUser && (
|
||||
<ActionDropdownItem
|
||||
icon={<UserMinus aria-hidden="true" />}
|
||||
label="Expel from organization"
|
||||
destructive
|
||||
onSelect={() => setIsExpelOpen(true)}
|
||||
/>
|
||||
)}
|
||||
{canDeleteUser && (
|
||||
<ActionDropdownItem
|
||||
icon={<Trash2 aria-hidden="true" />}
|
||||
label="Delete User"
|
||||
destructive
|
||||
onSelect={() => setIsDeleteOpen(true)}
|
||||
/>
|
||||
)}
|
||||
</ActionDropdownDangerZone>
|
||||
)}
|
||||
</ActionDropdown>
|
||||
</div>
|
||||
</>
|
||||
|
||||
Reference in New Issue
Block a user