diff --git a/frontend/src/components/pages/security/tabs/acls-tab.test.tsx b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx new file mode 100644 index 0000000000..75575e7a08 --- /dev/null +++ b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx @@ -0,0 +1,159 @@ +/** + * Copyright 2026 Redpanda Data, Inc. + * + * Use of this software is governed by the Business Source License + * included in the file https://github.com/redpanda-data/redpanda/blob/dev/licenses/bsl.md + * + * As of the Change Date specified in that file, in accordance with + * the Business Source License, use of this software will be governed + * by the Apache License, Version 2.0 + */ + +import { describe, expect, rs, test } from '@rstest/core'; +import { render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import type { ReactNode } from 'react'; + +// The Playwright spec cannot reach the row menu — a fresh cluster has no ACLs — so it is covered +// here, along with the delete-option enablement. + +rs.mock('@tanstack/react-router', () => { + const actual = rs.requireActual('@tanstack/react-router'); + return { + ...actual, + Link: ({ children, to, ...props }: { children: ReactNode; to?: string; [key: string]: unknown }) => ( + + {children} + + ), + useNavigate: () => rs.fn(), + }; +}); + +rs.mock('../../../misc/section', () => ({ + default: ({ children }: { children?: ReactNode }) =>
{children}
, +})); + +rs.mock('../shared/security-tabs-nav', () => ({ + SecurityTabsNav: () => null, +})); + +rs.mock('../../../../components/misc/error-result', () => ({ + default: () => null, +})); + +rs.mock('../../../../state/backend-api', () => ({ + api: { refreshAcls: rs.fn().mockResolvedValue(undefined) }, +})); + +rs.mock('../../../../state/rest-interfaces', () => ({ + AclRequestDefault: {}, +})); + +rs.mock('../../../../state/supported-features', () => ({ + useSupportedFeaturesStore: (selector: (s: Record) => T) => + selector({ deleteUser: true, rolesApi: true }), +})); + +rs.mock('../../../../react-query/api/cluster-status', () => ({ + useGetRedpandaInfoQuery: () => ({ data: { version: 'v26.1' }, isSuccess: true }), +})); + +rs.mock('../../../../react-query/api/user', () => ({ + useInvalidateUsersCache: () => rs.fn(), + useDeleteUserMutation: () => ({ mutateAsync: rs.fn().mockResolvedValue(undefined) }), + // `acl-only` has ACLs but no account. + useListUsersQuery: () => ({ data: { users: [{ name: 'scram-admin' }, { name: 'shadowed' }] }, isLoading: false }), +})); + +rs.mock('../../../../react-query/api/acl', () => ({ + useDeleteAclMutation: () => ({ mutateAsync: rs.fn().mockResolvedValue(undefined) }), + useListACLAsPrincipalGroups: () => ({ + data: [ + { principal: 'User:scram-admin', principalType: 'User', principalName: 'scram-admin', host: '*' }, + { principal: 'User:acl-only', principalType: 'User', principalName: 'acl-only', host: '10.0.0.1' }, + { principal: 'Group:engineering', principalType: 'Group', principalName: 'engineering', host: '*' }, + // Name collides with the `shadowed` SASL user, to pin the principalType guard. + { principal: 'Group:shadowed', principalType: 'Group', principalName: 'shadowed', host: '10.0.0.9' }, + ], + isLoading: false, + isError: false, + error: null, + }), +})); + +const SORT_ASC = /Asc/; +const ACL_ROW_TESTID = /^acl-list-item-/; + +const { AclsTab } = await import('./acls-tab'); + +const openRowMenu = async (principalName: string) => { + const user = userEvent.setup(); + await user.click(screen.getByRole('button', { name: `Delete ACL for ${principalName}` })); + return { user, menu: await screen.findByRole('menu') }; +}; + +describe('AclsTab', () => { + test('lists each ACL principal with its host', () => { + render(); + + expect(screen.getByTestId('acl-list-item-scram-admin-*')).toBeInTheDocument(); + expect(screen.getByTestId('acl-list-item-acl-only-10.0.0.1')).toBeInTheDocument(); + // A wildcard host reads as "Any"; a real one prints. + expect(screen.getByText('10.0.0.1')).toBeInTheDocument(); + }); + + test('marks a Group principal', () => { + render(); + + const row = screen.getByTestId('acl-list-item-engineering-*').closest('a'); + expect(within(row as HTMLElement).getByText('Group')).toBeInTheDocument(); + }); + + test('opens the row delete menu with all three options', async () => { + render(); + const { menu } = await openRowMenu('scram-admin'); + + expect(within(menu).getByRole('menuitem', { name: 'Delete (User and ACLs)' })).toBeInTheDocument(); + expect(within(menu).getByRole('menuitem', { name: 'Delete (User only)' })).toBeInTheDocument(); + expect(within(menu).getByRole('menuitem', { name: 'Delete (ACLs only)' })).toBeInTheDocument(); + }); + + test('enables the user-delete options only for a principal that has an account', async () => { + render(); + const { menu } = await openRowMenu('scram-admin'); + + expect(within(menu).getByRole('menuitem', { name: 'Delete (User and ACLs)' })).not.toHaveAttribute('data-disabled'); + expect(within(menu).getByRole('menuitem', { name: 'Delete (ACLs only)' })).not.toHaveAttribute('data-disabled'); + }); + + test('never offers user deletes on a Group row, even when a user shares its name', async () => { + render(); + const { menu } = await openRowMenu('shadowed'); + + expect(within(menu).getByRole('menuitem', { name: 'Delete (User and ACLs)' })).toHaveAttribute('data-disabled'); + expect(within(menu).getByRole('menuitem', { name: 'Delete (User only)' })).toHaveAttribute('data-disabled'); + expect(within(menu).getByRole('menuitem', { name: 'Delete (ACLs only)' })).not.toHaveAttribute('data-disabled'); + }); + + test('sorts the Principal column by the name it renders', async () => { + render(); + const user = userEvent.setup(); + + await user.click(screen.getByRole('button', { name: 'Principal' })); + await user.click(await screen.findByRole('menuitem', { name: SORT_ASC })); + + const names = screen.getAllByTestId(ACL_ROW_TESTID).map((el) => el.textContent); + expect(names).toEqual([...names].sort()); + }); + + test('disables the user-delete options for an ACL-only principal', async () => { + render(); + const { menu } = await openRowMenu('acl-only'); + + expect(within(menu).getByRole('menuitem', { name: 'Delete (User and ACLs)' })).toHaveAttribute('data-disabled'); + expect(within(menu).getByRole('menuitem', { name: 'Delete (User only)' })).toHaveAttribute('data-disabled'); + // Deleting only the ACLs never depends on an account existing. + expect(within(menu).getByRole('menuitem', { name: 'Delete (ACLs only)' })).not.toHaveAttribute('data-disabled'); + }); +}); diff --git a/frontend/src/components/pages/security/tabs/acls-tab.tsx b/frontend/src/components/pages/security/tabs/acls-tab.tsx index e4db1d70be..4f54ae6a28 100644 --- a/frontend/src/components/pages/security/tabs/acls-tab.tsx +++ b/frontend/src/components/pages/security/tabs/acls-tab.tsx @@ -10,9 +10,13 @@ */ import { create } from '@bufbuild/protobuf'; -import { DataTable, SearchField } from '@redpanda-data/ui'; import { Link, useNavigate } from '@tanstack/react-router'; import { TrashIcon } from 'components/icons'; +import { + DataTable, + type DataTableColumnDef, + DataTableColumnHeader, +} from 'components/redpanda-ui/components/data-table'; import { InfoIcon } from 'lucide-react'; import { ACL_Operation, @@ -23,17 +27,19 @@ import { DeleteACLsRequestSchema, } from 'protogen/redpanda/api/dataplane/v1/acl_pb'; import type { FC } from 'react'; -import { useState } from 'react'; +import { createContext, useContext, useState } from 'react'; import { toast } from 'sonner'; import ErrorResult from '../../../../components/misc/error-result'; -import { useDeleteAclMutation, useListACLAsPrincipalGroups } from '../../../../react-query/api/acl'; +import { type SimpleAcl, useDeleteAclMutation, useListACLAsPrincipalGroups } from '../../../../react-query/api/acl'; import { useGetRedpandaInfoQuery } from '../../../../react-query/api/cluster-status'; import { useDeleteUserMutation, useInvalidateUsersCache, useListUsersQuery } from '../../../../react-query/api/user'; import { api } from '../../../../state/backend-api'; import { AclRequestDefault } from '../../../../state/rest-interfaces'; import { useSupportedFeaturesStore } from '../../../../state/supported-features'; import { Code as CodeEl, DefaultSkeleton } from '../../../../utils/tsx-utils'; +import { DEFAULT_TABLE_PAGE_SIZE } from '../../../constants'; +import { SearchInput } from '../../../misc/search-input'; import Section from '../../../misc/section'; import { Alert, AlertDescription } from '../../../redpanda-ui/components/alert'; import { Badge } from '../../../redpanda-ui/components/badge'; @@ -48,6 +54,161 @@ import { AlertDeleteFailed } from '../shared/alert-delete-failed'; import { filterByName } from '../shared/filter-by-name'; import { SecurityTabsNav } from '../shared/security-tabs-nav'; +/** Every ACL bound to this principal on this host, whatever the resource or operation. */ +const allAclsFor = (principal: string, host: string): DeleteACLsRequest => + create(DeleteACLsRequestSchema, { + filter: { + principal, + resourceType: ACL_ResourceType.ANY, + resourceName: undefined, + host, + operation: ACL_Operation.ANY, + permissionType: ACL_PermissionType.ANY, + resourcePatternType: ACL_ResourcePatternType.ANY, + }, + }); + +// Legacy table parity: 50 rows a page, pager only past that. No column-visibility UI, so hiding +// is off at table level. `getRowId` keeps an open row menu on its own principal across a refetch. +const TABLE_OPTIONS = { + enableHiding: false, + initialState: { pagination: { pageIndex: 0, pageSize: DEFAULT_TABLE_PAGE_SIZE } }, + getRowId: (row: SimpleAcl) => `${row.principal}:${row.host}`, +}; + +// Context, not props, so `columns` closes over nothing and lives at module scope. Queries stay in +// the parent: `useListUsersQuery` auto-fetches every page from an effect, so one per row would too. +type AclRowActionsContextValue = { + users: { name: string }[]; + canDeleteUsers: boolean; + deleteAclsForPrincipal: (principal: string, host: string) => Promise; + deleteUser: (name: string) => Promise; + invalidateUsers: () => Promise; + onFailure: (failure: { err: unknown }) => void; +}; + +const AclRowActionsContext = createContext(null); + +const AclRowActions: FC<{ record: SimpleAcl }> = ({ record }) => { + const ctx = useContext(AclRowActionsContext); + if (!ctx) { + return null; + } + const { users, canDeleteUsers, deleteAclsForPrincipal, deleteUser, invalidateUsers, onFailure } = ctx; + + // Only a User row may offer the user deletes; a same-named Group would delete an unrelated user. + const hasAccount = record.principalType === 'User' && users.some((u) => u.name === record.principalName); + const canDeleteUser = hasAccount && canDeleteUsers; + + const onDelete = async (user: boolean, acls: boolean) => { + if (acls) { + try { + await deleteAclsForPrincipal(record.principal, record.host); + } catch (err: unknown) { + // biome-ignore lint/suspicious/noConsole: error logging + console.error('failed to delete acls', { error: err }); + onFailure({ err }); + // Deleting the account too would orphan the ACLs that just failed to go. + return; + } + } + + if (user) { + try { + await deleteUser(record.principalName); + } catch (err: unknown) { + // biome-ignore lint/suspicious/noConsole: error logging + console.error('failed to delete user', { error: err }); + onFailure({ err }); + } + } + + await Promise.allSettled([api.refreshAcls(AclRequestDefault, true), invalidateUsers()]); + }; + + const handle = (user: boolean, acls: boolean) => (e: { stopPropagation: () => void }) => { + onDelete(user, acls).catch(() => { + // Error handling managed by API layer + }); + e.stopPropagation(); + }; + + return ( + + + + + } + /> + + + Delete (User and ACLs) + + + Delete (User only) + + Delete (ACLs only) + + + ); +}; + +const columns: DataTableColumnDef[] = [ + { + id: 'principal', + header: ({ column }) => , + // Sort on what the cell renders, not the `User:`-prefixed `principal`. + accessorFn: (row) => row.principalName, + cell: ({ row: { original: record } }) => ( + ({ ...prev, host: record.host })} + to="/security/acls/$aclName/details" + > + + + {record.principalName} + + {record.principalType === 'Group' && ( + + Group + + )} + + + ), + }, + { + id: 'host', + header: ({ column }) => , + accessorKey: 'host', + cell: ({ + row: { + original: { host }, + }, + }) => + !host || host === '*' ? ( + + Any + + ) : ( + host + ), + }, + { + id: 'menu', + header: '', + enableSorting: false, + cell: ({ row: { original: record } }) => , + }, +]; + const AclsTabContent: FC = () => { const featureRolesApi = useSupportedFeaturesStore((s) => s.rolesApi); const featureDeleteUser = useSupportedFeaturesStore((s) => s.deleteUser); @@ -64,30 +225,33 @@ const AclsTabContent: FC = () => { const navigate = useNavigate(); - const deleteACLsForPrincipal = async (principal: string, host: string) => { - const deleteRequest: DeleteACLsRequest = create(DeleteACLsRequestSchema, { - filter: { - principal, - resourceType: ACL_ResourceType.ANY, - resourceName: undefined, - host, - operation: ACL_Operation.ANY, - permissionType: ACL_PermissionType.ANY, - resourcePatternType: ACL_ResourcePatternType.ANY, - }, - }); - await deleteACLMutation(deleteRequest); - toast.success( - - Deleted ACLs for {principal} - - ); - }; - const aclPrincipalGroups = principalGroups?.filter((g) => g.principalType === 'User' || g.principalType === 'Group') || []; const groups = filterByName(aclPrincipalGroups, searchQuery, (g) => g.principalName); + const rowActions: AclRowActionsContextValue = { + users: usersData?.users ?? [], + canDeleteUsers: Boolean(featureDeleteUser), + deleteAclsForPrincipal: async (principal, host) => { + await deleteACLMutation(allAclsFor(principal, host)); + toast.success( + + Deleted ACLs for {principal} + + ); + }, + deleteUser: async (name) => { + await deleteUserMut({ name }); + toast.success( + + Deleted user {name} + + ); + }, + invalidateUsers: invalidateUsersCache, + onFailure: setAclFailed, + }; + if (isError && error) { return ; } @@ -112,11 +276,11 @@ const AclsTabContent: FC = () => { )} -
setAclFailed(null)} /> @@ -134,150 +298,15 @@ const AclsTabContent: FC = () => {
- - columns={[ - { - size: Number.POSITIVE_INFINITY, - header: 'Principal', - accessorKey: 'principal', - cell: ({ row: { original: record } }) => ( - ({ ...prev, host: record.host })} - to="/security/acls/$aclName/details" - > - - - {record.principalName} - - {record.principalType === 'Group' && ( - - Group - - )} - - - ), - }, - { - header: 'Host', - accessorKey: 'host', - cell: ({ - row: { - original: { host }, - }, - }) => - !host || host === '*' ? ( - - Any - - ) : ( - host - ), - }, - { - size: 60, - id: 'menu', - header: '', - cell: ({ row: { original: record } }) => { - const userExists = usersData?.users?.some((u) => u.name === record.principalName) ?? false; - - const onDelete = async (user: boolean, acls: boolean) => { - if (acls) { - try { - await deleteACLsForPrincipal(record.principal, record.host); - } catch (err: unknown) { - // biome-ignore lint/suspicious/noConsole: error logging - console.error('failed to delete acls', { error: err }); - setAclFailed({ err }); - } - } - - if (user) { - try { - await deleteUserMut({ name: record.principalName }); - toast.success( - - Deleted user {record.principalName} - - ); - } catch (err: unknown) { - // biome-ignore lint/suspicious/noConsole: error logging - console.error('failed to delete user', { error: err }); - setAclFailed({ err }); - } - } - - await Promise.allSettled([api.refreshAcls(AclRequestDefault, true), invalidateUsersCache()]); - }; - - return ( - - {}} - size="icon-sm" - variant="destructive-ghost" - > - - - } - /> - - { - onDelete(true, true).catch(() => { - // Error handling managed by API layer - }); - e.stopPropagation(); - }} - > - Delete (User and ACLs) - - { - onDelete(true, false).catch(() => { - // Error handling managed by API layer - }); - e.stopPropagation(); - }} - > - Delete (User only) - - { - onDelete(false, true).catch(() => { - // Error handling managed by API layer - }); - e.stopPropagation(); - }} - > - Delete (ACLs only) - - - - ); - }, - }, - ]} - data={groups} - pagination - sorting - /> + + + columns={columns} + data={groups} + pagination={groups.length > DEFAULT_TABLE_PAGE_SIZE} + sorting + tableOptions={TABLE_OPTIONS} + /> +
diff --git a/frontend/src/components/pages/security/tabs/roles-tab.test.tsx b/frontend/src/components/pages/security/tabs/roles-tab.test.tsx index 636cbb40a5..0ff8923174 100644 --- a/frontend/src/components/pages/security/tabs/roles-tab.test.tsx +++ b/frontend/src/components/pages/security/tabs/roles-tab.test.tsx @@ -31,163 +31,6 @@ const { historyPushMock, refreshRoleMembersMock, refreshRolesMock, deleteRoleMut deleteRoleMutationMock: rs.fn().mockResolvedValue(undefined), })); -rs.mock('@redpanda-data/ui', () => { - const Div = ({ - children, - flexDirection: _flexDirection, - ...props - }: { - children?: ReactNode; - flexDirection?: unknown; - [key: string]: unknown; - }) =>
{children}
; - - return { - Alert: Div, - AlertDescription: Div, - AlertIcon: () => , - AlertTitle: Div, - Badge: Div, - Box: Div, - Button: ({ - children, - isDisabled, - onClick, - tooltip: _tooltip, - ...props - }: { - children?: ReactNode; - isDisabled?: boolean; - onClick?: () => void; - tooltip?: unknown; - [key: string]: unknown; - }) => ( - - ), - CloseButton: ({ - children, - onClick, - ...props - }: { - children?: ReactNode; - onClick?: () => void; - [key: string]: unknown; - }) => ( - - ), - createStandaloneToast: () => ({ - ToastContainer: () => null, - toast: rs.fn(), - }), - DataTable: ({ - columns, - data, - emptyAction, - emptyText, - }: { - columns: Array<{ - cell?: (ctx: { row: { original: Record } }) => ReactNode; - header?: ReactNode; - id: string; - }>; - data: Record[]; - emptyAction?: ReactNode; - emptyText?: ReactNode; - }) => - data.length > 0 ? ( - - - {data.map((row, rowIndex) => ( - - {columns.map((column) => ( - - ))} - - ))} - -
{column.cell?.({ row: { original: row } }) ?? null}
- ) : ( -
-
{emptyText}
- {emptyAction} -
- ), - Flex: Div, - Icon: () => , - Link: ({ - as: Component, - children, - ...props - }: { - as?: ((props: Record) => ReactNode) | string; - children?: ReactNode; - [key: string]: unknown; - }) => - Component && typeof Component !== 'string' ? ( - {children} - ) : ( - {children} - ), - Menu: Div, - MenuButton: ({ - children, - onClick, - ...props - }: { - children?: ReactNode; - onClick?: () => void; - [key: string]: unknown; - }) => ( - - ), - MenuItem: ({ - children, - onClick, - ...props - }: { - children?: ReactNode; - onClick?: () => void; - [key: string]: unknown; - }) => ( - - ), - MenuList: Div, - redpandaTheme: {}, - redpandaToastOptions: { - defaultOptions: {}, - }, - SearchField: ({ - placeholderText, - searchText, - setSearchText, - ...props - }: { - placeholderText?: string; - searchText?: string; - setSearchText?: (value: string) => void; - [key: string]: unknown; - }) => ( - setSearchText?.(e.target.value)} - placeholder={placeholderText} - value={searchText ?? ''} - {...props} - /> - ), - Skeleton: Div, - Text: Div, - Tooltip: ({ children }: { children?: ReactNode }) => <>{children}, - }; -}); - rs.mock('@tanstack/react-router', () => { const actual = rs.requireActual('@tanstack/react-router'); diff --git a/frontend/src/react-query/api/acl.tsx b/frontend/src/react-query/api/acl.tsx index 80026d52fb..238c63e741 100644 --- a/frontend/src/react-query/api/acl.tsx +++ b/frontend/src/react-query/api/acl.tsx @@ -120,7 +120,7 @@ export const useCreateACLMutation = () => { // New ACL implementation // Used by ACLs tab and Permissions List tab in the security section. -type SimpleAcl = { +export type SimpleAcl = { host: string; principal: string; principalType: string; diff --git a/frontend/tests/test-variant-console/acls/acls-tab.spec.ts b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts new file mode 100644 index 0000000000..fc8af24341 --- /dev/null +++ b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts @@ -0,0 +1,29 @@ +// spec: the security ACLs tab — see the PR for the swap's contract + +import { expect, test } from '@playwright/test'; + +const PRINCIPAL_HEADER = /^Principal$/; +const HOST_HEADER = /^Host$/; +const SORT_ASC = /Asc/; + +// The Registry sorts only through `DataTableColumnHeader`, where Chakra painted an affordance on +// every header, so a swap can leave sorting unreachable with nothing failing. A fresh cluster has +// no ACLs, so this asserts the chrome rather than rows. +test.describe('Security ACLs tab', () => { + test('renders the filter, the table and reachable sorting', async ({ page }) => { + await page.goto('/security/acls'); + + await expect(page.getByTestId('create-acls')).toBeVisible(); + await expect(page.getByPlaceholder('Filter by name')).toBeVisible(); + + // Both sortable columns must expose a trigger; the action column must not. + const table = page.getByRole('table'); + await expect(table.getByRole('button', { name: PRINCIPAL_HEADER })).toBeVisible(); + await expect(table.getByRole('button', { name: HOST_HEADER })).toBeVisible(); + + // The header is a dropdown trigger, not a one-click sort. + await table.getByRole('button', { name: PRINCIPAL_HEADER }).click(); + await expect(page.getByRole('menuitem', { name: SORT_ASC })).toBeVisible(); + await page.keyboard.press('Escape'); + }); +});