From 2091666980ad957db53a0d813616072b81d47cec Mon Sep 17 00:00:00 2001 From: Peter Zenger Date: Fri, 11 Sep 2026 07:40:43 -0700 Subject: [PATCH 1/4] frontend: the security ACLs tab on Registry components, plus a smoke spec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2604 replaced the users, roles and permissions tabs with new designs and left the ACLs tab on the old one, so it kept the last Chakra import in `pages/security`. There is no `acls-tab-new`, and the route is live at `/security/acls`, so this is a parity swap rather than throwaway work: Chakra's `SearchField` becomes the shared `SearchInput` and its `DataTable` becomes the Registry one. Whether ACLs also gets the new security design is a separate product call. Both sortable columns get a `DataTableColumnHeader` — the Registry sorts only through that component, where Chakra painted an affordance on every header — and the table passes `enableHiding: false`, since there is no column-visibility toolbar to undo a hide with. Pagination keeps legacy parity at 50 a page with the pager only past that; the two `size:` props are dropped because the Registry never reads them. The row-actions menu moves into its own component that owns its queries. That leaves the columns array closing over nothing but a stable state setter, so it is built once: `DataTableColumnHeader` is a dropdown trigger, and a re-created header tears an open sort menu down. Also drops the dead `rs.mock('@redpanda-data/ui')` from `roles-tab.test.tsx` — 156 lines stubbing a package that file's tree no longer touches, since `roles-tab.tsx` is now a re-export of the Chakra-free `roles-tab-new`. Both suites are unchanged at 993/1427, so those tests now exercise the real components. The new Playwright spec covers what the swap risked: the filter renders, both headers expose a sort trigger, and the trigger opens a menu. Verified against a live container, and it fails if the `DataTableColumnHeader` is swapped back for a plain string header. Co-Authored-By: Claude Opus 5 (1M context) --- .../pages/security/tabs/acls-tab.tsx | 337 +++++++++--------- .../pages/security/tabs/roles-tab.test.tsx | 157 -------- .../acls/acls-tab.spec.ts | 32 ++ 3 files changed, 207 insertions(+), 319 deletions(-) create mode 100644 frontend/tests/test-variant-console/acls/acls-tab.spec.ts diff --git a/frontend/src/components/pages/security/tabs/acls-tab.tsx b/frontend/src/components/pages/security/tabs/acls-tab.tsx index e4db1d70be..efdd9668a3 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,7 +27,7 @@ import { DeleteACLsRequestSchema, } from 'protogen/redpanda/api/dataplane/v1/acl_pb'; import type { FC } from 'react'; -import { useState } from 'react'; +import { useMemo, useState } from 'react'; import { toast } from 'sonner'; import ErrorResult from '../../../../components/misc/error-result'; @@ -34,6 +38,8 @@ 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,29 +54,47 @@ import { AlertDeleteFailed } from '../shared/alert-delete-failed'; import { filterByName } from '../shared/filter-by-name'; import { SecurityTabsNav } from '../shared/security-tabs-nav'; -const AclsTabContent: FC = () => { - const featureRolesApi = useSupportedFeaturesStore((s) => s.rolesApi); +type AclPrincipalRow = { + principal: string; + host: string; + principalType: string; + principalName: string; +}; + +// Legacy table parity: 50 rows a page, pager only past that. No column-visibility UI, so hiding +// is off at table level. +const TABLE_OPTIONS = { + enableHiding: false, + initialState: { pagination: { pageIndex: 0, pageSize: DEFAULT_TABLE_PAGE_SIZE } }, +}; + +/** + * Owns its own queries so the columns array closes over nothing but the failure setter — a stable + * array matters because `DataTableColumnHeader` is a dropdown trigger that a re-render destroys. + */ +const AclRowActions: FC<{ record: AclPrincipalRow; onFailure: (failure: { err: unknown }) => void }> = ({ + record, + onFailure, +}) => { const featureDeleteUser = useSupportedFeaturesStore((s) => s.deleteUser); const { data: redpandaInfo, isSuccess: isRedpandaInfoSuccess } = useGetRedpandaInfoQuery(); - const isAdminApiConfigured = isRedpandaInfoSuccess && Boolean(redpandaInfo); - const { data: usersData } = useListUsersQuery(undefined, { enabled: isAdminApiConfigured }); - const { data: principalGroups, isLoading, isError, error } = useListACLAsPrincipalGroups(); + const { data: usersData } = useListUsersQuery(undefined, { + enabled: isRedpandaInfoSuccess && Boolean(redpandaInfo), + }); const { mutateAsync: deleteACLMutation } = useDeleteAclMutation(); const { mutateAsync: deleteUserMut } = useDeleteUserMutation(); const invalidateUsersCache = useInvalidateUsersCache(); - const [aclFailed, setAclFailed] = useState<{ err: unknown } | null>(null); - const [searchQuery, setSearchQuery] = useState(''); + const userExists = usersData?.users?.some((u) => u.name === record.principalName) ?? false; + const canDeleteUser = userExists && Boolean(featureDeleteUser); - const navigate = useNavigate(); - - const deleteACLsForPrincipal = async (principal: string, host: string) => { + const deleteAcls = async () => { const deleteRequest: DeleteACLsRequest = create(DeleteACLsRequestSchema, { filter: { - principal, + principal: record.principal, resourceType: ACL_ResourceType.ANY, resourceName: undefined, - host, + host: record.host, operation: ACL_Operation.ANY, permissionType: ACL_PermissionType.ANY, resourcePatternType: ACL_ResourcePatternType.ANY, @@ -79,15 +103,146 @@ const AclsTabContent: FC = () => { await deleteACLMutation(deleteRequest); toast.success( - Deleted ACLs for {principal} + Deleted ACLs for {record.principal} ); }; + const onDelete = async (user: boolean, acls: boolean) => { + if (acls) { + try { + await deleteAcls(); + } catch (err: unknown) { + // biome-ignore lint/suspicious/noConsole: error logging + console.error('failed to delete acls', { error: err }); + onFailure({ 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 }); + onFailure({ err }); + } + } + + await Promise.allSettled([api.refreshAcls(AclRequestDefault, true), invalidateUsersCache()]); + }; + + 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 AclsTabContent: FC = () => { + const featureRolesApi = useSupportedFeaturesStore((s) => s.rolesApi); + const { data: principalGroups, isLoading, isError, error } = useListACLAsPrincipalGroups(); + + const [aclFailed, setAclFailed] = useState<{ err: unknown } | null>(null); + const [searchQuery, setSearchQuery] = useState(''); + + const navigate = useNavigate(); + const aclPrincipalGroups = principalGroups?.filter((g) => g.principalType === 'User' || g.principalType === 'Group') || []; const groups = filterByName(aclPrincipalGroups, searchQuery, (g) => g.principalName); + // Built once. A fresh array hands `flexRender` new function identities, which re-creates every + // header — and `DataTableColumnHeader` is a dropdown trigger, so a re-render would tear an open + // sort menu down. `setAclFailed` is a stable setter, so there is nothing to depend on. + const columns: DataTableColumnDef[] = useMemo( + () => [ + { + id: 'principal', + header: ({ column }) => , + accessorKey: 'principal', + 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 } }) => , + }, + ], + [] + ); + if (isError && error) { return ; } @@ -112,12 +267,7 @@ const AclsTabContent: FC = () => { )} - +
setAclFailed(null)} /> @@ -134,149 +284,12 @@ 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) - - - - ); - }, - }, - ]} + + columns={columns} data={groups} - pagination + 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/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..a0e0a451a9 --- /dev/null +++ b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts @@ -0,0 +1,32 @@ +// spec: specs/security.md + +import { expect, test } from '@playwright/test'; + +const PRINCIPAL_HEADER = /^Principal$/; +const HOST_HEADER = /^Host$/; +const SORT_ASC = /Asc/; + +// The ACLs tab is the last security surface still on the old design, and it had no spec of its +// own — only an enterprise authorization test navigates here. What the Registry swap risked is +// the filter field and the table: Chakra painted a sort affordance on every header, while the +// Registry sorts only through `DataTableColumnHeader`, so a swap can leave sorting unreachable +// with nothing failing. A fresh cluster has no ACLs, so this asserts the chrome, not 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(); + + // `DataTableColumnHeader` is a dropdown trigger, so the header opens a menu rather than + // sorting on a single click. + await table.getByRole('button', { name: PRINCIPAL_HEADER }).click(); + await expect(page.getByRole('menuitem', { name: SORT_ASC })).toBeVisible(); + await page.keyboard.press('Escape'); + }); +}); From d8f8b9fb5de937f7f85aaae86703bdd86e482022 Mon Sep 17 00:00:00 2001 From: Peter Zenger Date: Fri, 11 Sep 2026 08:04:51 -0700 Subject: [PATCH 2/4] =?UTF-8?q?frontend:=20security=20=E2=80=94=20cover=20?= =?UTF-8?q?the=20ACLs=20row-actions=20menu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Playwright spec covers the table chrome, but a fresh cluster has no ACLs, so it never clicks a row menu — which is the part of this swap that moved most, into its own component. Five tests over the real components: the rows and their hosts render, a Group principal is badged, the menu opens with all three delete options, and the user-delete options are enabled only for a principal that actually has a SASL account. That last pair is the `canDeleteUser` logic the extraction restructured, and the menu opening at all confirms the trigger still works after the empty `onClick` came off its render slot. Co-Authored-By: Claude Opus 5 (1M context) --- .../pages/security/tabs/acls-tab.test.tsx | 137 ++++++++++++++++++ 1 file changed, 137 insertions(+) create mode 100644 frontend/src/components/pages/security/tabs/acls-tab.test.tsx 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..1ac9efb524 --- /dev/null +++ b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx @@ -0,0 +1,137 @@ +/** + * 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 row-actions menu moved into its own component during the Registry swap, so the delete + * options and their enablement are what needs guarding: the Playwright spec covers the table + * chrome, but a fresh cluster has no ACLs, so no row menu exists there to click. + */ + +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) }), + // Only `scram-admin` is a real SASL user; `acl-only` has ACLs but no account. + useListUsersQuery: () => ({ data: { users: [{ name: 'scram-admin' }] }, 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: '*' }, + ], + isLoading: false, + isError: false, + error: null, + }), +})); + +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('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'); + }); +}); From 706879662e479aa678cf193b16ca4b394be24dfe Mon Sep 17 00:00:00 2001 From: Peter Zenger Date: Fri, 11 Sep 2026 08:21:29 -0700 Subject: [PATCH 3/4] =?UTF-8?q?frontend:=20security=20=E2=80=94=20clear=20?= =?UTF-8?q?the=20ACLs=20review=20findings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six from a review of the swap, two of them mine and shipping-visible: - the 300px width was on `className`, which lands on the ``, not `containerClassName`, which sizes the positioned wrapper. `InputEnd` is absolute against that wrapper, so the "Clear search" X rendered at the far right of the page as soon as anyone typed. Every other call site uses `containerClassName`. - `AclRowActions` owned its own queries, and `useListUsersQuery` wraps `useInfiniteQueryWithAllPages`, which auto-fetches every page from an effect — so each row ran its own `fetchNextPage()` loop. The queries move back to the parent and reach the row through context, which is also what the stable-columns goal actually needed: `useInvalidateUsersCache` returns a new function every render, so a `useMemo` dep list could never have been stable. `columns` is now a module constant, closing over nothing. Four behaviour findings, all pre-existing in the Chakra original and folded in because the extraction put them in reach: - `canDeleteUser` matched only on name, so a Group principal sharing a name with a SASL user offered — and performed — a delete of that unrelated account. Now requires `principalType === 'User'`. - in "Delete (User and ACLs)" a failed ACL delete fell through to the account delete, orphaning the ACLs it had just failed to remove, and its `onFailure` was overwritten by the second leg's. It now returns. - the Principal sort key was the `User:`-prefixed `principal` while the cell renders `principalName`, so the newly-reachable sort ordered the column differently from what is on screen. Now an `accessorFn`. - no `getRowId`, so rows were index-keyed: a refetch that reordered the list retargeted an open action menu at a different principal. Also drops the dead `deleteButton` class, whose SCSS is nested under `.stringList .reorderableList` and includes an `opacity: 0` that would hide the button if anyone ever unscoped it. Two more tests: a Group row never offers the user deletes even when a user shares its name, and the Principal sort matches the rendered name. Both fail if their fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) --- .../pages/security/tabs/acls-tab.test.tsx | 30 +- .../pages/security/tabs/acls-tab.tsx | 264 ++++++++++-------- .../acls/acls-tab.spec.ts | 2 +- 3 files changed, 176 insertions(+), 120 deletions(-) diff --git a/frontend/src/components/pages/security/tabs/acls-tab.test.tsx b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx index 1ac9efb524..d1106bc8de 100644 --- a/frontend/src/components/pages/security/tabs/acls-tab.test.tsx +++ b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx @@ -65,8 +65,8 @@ rs.mock('../../../../react-query/api/cluster-status', () => ({ rs.mock('../../../../react-query/api/user', () => ({ useInvalidateUsersCache: () => rs.fn(), useDeleteUserMutation: () => ({ mutateAsync: rs.fn().mockResolvedValue(undefined) }), - // Only `scram-admin` is a real SASL user; `acl-only` has ACLs but no account. - useListUsersQuery: () => ({ data: { users: [{ name: 'scram-admin' }] }, isLoading: false }), + // `scram-admin` and `shadowed` are real SASL users; `acl-only` has ACLs but no account. + useListUsersQuery: () => ({ data: { users: [{ name: 'scram-admin' }, { name: 'shadowed' }] }, isLoading: false }), })); rs.mock('../../../../react-query/api/acl', () => ({ @@ -76,6 +76,8 @@ rs.mock('../../../../react-query/api/acl', () => ({ { 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 deliberately 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, @@ -83,6 +85,9 @@ rs.mock('../../../../react-query/api/acl', () => ({ }), })); +const SORT_ASC = /Asc/; +const ACL_ROW_TESTID = /^acl-list-item-/; + const { AclsTab } = await import('./acls-tab'); const openRowMenu = async (principalName: string) => { @@ -125,6 +130,27 @@ describe('AclsTab', () => { 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(); + // Same `principalName` as a SASL user, but a Group has no account to delete. + 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'); diff --git a/frontend/src/components/pages/security/tabs/acls-tab.tsx b/frontend/src/components/pages/security/tabs/acls-tab.tsx index efdd9668a3..3d899c6ab9 100644 --- a/frontend/src/components/pages/security/tabs/acls-tab.tsx +++ b/frontend/src/components/pages/security/tabs/acls-tab.tsx @@ -27,7 +27,7 @@ import { DeleteACLsRequestSchema, } from 'protogen/redpanda/api/dataplane/v1/acl_pb'; import type { FC } from 'react'; -import { useMemo, useState } from 'react'; +import { createContext, useContext, useState } from 'react'; import { toast } from 'sonner'; import ErrorResult from '../../../../components/misc/error-result'; @@ -62,71 +62,60 @@ type AclPrincipalRow = { }; // Legacy table parity: 50 rows a page, pager only past that. No column-visibility UI, so hiding -// is off at table level. +// is off at table level. `getRowId` keeps a row's open action menu on its own principal when the +// list refetches and the order shifts. const TABLE_OPTIONS = { enableHiding: false, initialState: { pagination: { pageIndex: 0, pageSize: DEFAULT_TABLE_PAGE_SIZE } }, + getRowId: (row: AclPrincipalRow) => `${row.principal}:${row.host}`, }; /** - * Owns its own queries so the columns array closes over nothing but the failure setter — a stable - * array matters because `DataTableColumnHeader` is a dropdown trigger that a re-render destroys. + * The row actions read their data from context rather than props so the columns array closes over + * nothing and can live at module scope: `DataTableColumnHeader` is a dropdown trigger, and a new + * header-function identity remounts it, tearing an open sort menu down. Queries stay in the parent + * — `useListUsersQuery` auto-fetches every page from an effect, so one instance per row would fire + * a `fetchNextPage()` per row. */ -const AclRowActions: FC<{ record: AclPrincipalRow; onFailure: (failure: { err: unknown }) => void }> = ({ - record, - onFailure, -}) => { - const featureDeleteUser = useSupportedFeaturesStore((s) => s.deleteUser); - const { data: redpandaInfo, isSuccess: isRedpandaInfoSuccess } = useGetRedpandaInfoQuery(); - const { data: usersData } = useListUsersQuery(undefined, { - enabled: isRedpandaInfoSuccess && Boolean(redpandaInfo), - }); - const { mutateAsync: deleteACLMutation } = useDeleteAclMutation(); - const { mutateAsync: deleteUserMut } = useDeleteUserMutation(); - const invalidateUsersCache = useInvalidateUsersCache(); +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 userExists = usersData?.users?.some((u) => u.name === record.principalName) ?? false; - const canDeleteUser = userExists && Boolean(featureDeleteUser); +const AclRowActionsContext = createContext(null); - const deleteAcls = async () => { - const deleteRequest: DeleteACLsRequest = create(DeleteACLsRequestSchema, { - filter: { - principal: record.principal, - resourceType: ACL_ResourceType.ANY, - resourceName: undefined, - host: record.host, - operation: ACL_Operation.ANY, - permissionType: ACL_PermissionType.ANY, - resourcePatternType: ACL_ResourcePatternType.ANY, - }, - }); - await deleteACLMutation(deleteRequest); - toast.success( - - Deleted ACLs for {record.principal} - - ); - }; +const AclRowActions: FC<{ record: AclPrincipalRow }> = ({ record }) => { + const ctx = useContext(AclRowActionsContext); + if (!ctx) { + return null; + } + const { users, canDeleteUsers, deleteAclsForPrincipal, deleteUser, invalidateUsers, onFailure } = ctx; + + // A Group principal never has a SASL account, so only a User row may offer the user deletes — + // a same-named group would otherwise 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 deleteAcls(); + 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 deleteUserMut({ name: record.principalName }); - toast.success( - - Deleted user {record.principalName} - - ); + await deleteUser(record.principalName); } catch (err: unknown) { // biome-ignore lint/suspicious/noConsole: error logging console.error('failed to delete user', { error: err }); @@ -134,7 +123,7 @@ const AclRowActions: FC<{ record: AclPrincipalRow; onFailure: (failure: { err: u } } - await Promise.allSettled([api.refreshAcls(AclRequestDefault, true), invalidateUsersCache()]); + await Promise.allSettled([api.refreshAcls(AclRequestDefault, true), invalidateUsers()]); }; const handle = (user: boolean, acls: boolean) => (e: { stopPropagation: () => void }) => { @@ -148,12 +137,7 @@ const AclRowActions: FC<{ record: AclPrincipalRow; onFailure: (failure: { err: u + } @@ -171,9 +155,71 @@ const AclRowActions: FC<{ record: AclPrincipalRow; onFailure: (failure: { err: u ); }; +const columns: DataTableColumnDef[] = [ + { + id: 'principal', + header: ({ column }) => , + // The cell shows `principalName`; sorting on the `User:`-prefixed `principal` would order + // the column differently from what is on screen. + 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); + const { data: redpandaInfo, isSuccess: isRedpandaInfoSuccess } = useGetRedpandaInfoQuery(); + const isAdminApiConfigured = isRedpandaInfoSuccess && Boolean(redpandaInfo); + const { data: usersData } = useListUsersQuery(undefined, { enabled: isAdminApiConfigured }); const { data: principalGroups, isLoading, isError, error } = useListACLAsPrincipalGroups(); + const { mutateAsync: deleteACLMutation } = useDeleteAclMutation(); + const { mutateAsync: deleteUserMut } = useDeleteUserMutation(); + const invalidateUsersCache = useInvalidateUsersCache(); const [aclFailed, setAclFailed] = useState<{ err: unknown } | null>(null); const [searchQuery, setSearchQuery] = useState(''); @@ -184,64 +230,41 @@ const AclsTabContent: FC = () => { principalGroups?.filter((g) => g.principalType === 'User' || g.principalType === 'Group') || []; const groups = filterByName(aclPrincipalGroups, searchQuery, (g) => g.principalName); - // Built once. A fresh array hands `flexRender` new function identities, which re-creates every - // header — and `DataTableColumnHeader` is a dropdown trigger, so a re-render would tear an open - // sort menu down. `setAclFailed` is a stable setter, so there is nothing to depend on. - const columns: DataTableColumnDef[] = useMemo( - () => [ - { - id: 'principal', - header: ({ column }) => , - accessorKey: 'principal', - 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 } }) => , - }, - ], - [] - ); + // Not memoised: the consumers are the row action cells, which re-render with the parent anyway. + // What has to stay stable is `columns`, and that is a module constant. + const rowActions: AclRowActionsContextValue = { + users: usersData?.users ?? [], + canDeleteUsers: Boolean(featureDeleteUser), + deleteAclsForPrincipal: async (principal, host) => { + 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} + + ); + }, + deleteUser: async (name) => { + await deleteUserMut({ name }); + toast.success( + + Deleted user {name} + + ); + }, + invalidateUsers: invalidateUsersCache, + onFailure: setAclFailed, + }; if (isError && error) { return ; @@ -267,7 +290,12 @@ const AclsTabContent: FC = () => { )} - +
setAclFailed(null)} /> @@ -284,13 +312,15 @@ const AclsTabContent: FC = () => {
- - columns={columns} - data={groups} - pagination={groups.length > DEFAULT_TABLE_PAGE_SIZE} - sorting - tableOptions={TABLE_OPTIONS} - /> + + + columns={columns} + data={groups} + pagination={groups.length > DEFAULT_TABLE_PAGE_SIZE} + sorting + tableOptions={TABLE_OPTIONS} + /> +
diff --git a/frontend/tests/test-variant-console/acls/acls-tab.spec.ts b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts index a0e0a451a9..a78cf5ae1a 100644 --- a/frontend/tests/test-variant-console/acls/acls-tab.spec.ts +++ b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts @@ -1,4 +1,4 @@ -// spec: specs/security.md +// spec: the security ACLs tab — see the PR for the swap's contract import { expect, test } from '@playwright/test'; From 9a5dbb587d3284b25515612fa6a40ba99c4a5ee8 Mon Sep 17 00:00:00 2001 From: Peter Zenger Date: Fri, 11 Sep 2026 08:35:32 -0700 Subject: [PATCH 4/4] =?UTF-8?q?frontend:=20security=20=E2=80=94=20trim=20t?= =?UTF-8?q?he=20ACLs=20comments,=20drop=20the=20duplicated=20row=20type?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comments to the constraint; the reasoning is in the PR. The context and table-options notes and both spec preambles were carrying paragraphs. Two code cleanups: `SimpleAcl` — whose own comment already says it is "used by ACLs tab and Permissions List tab" — is now exported and consumed, instead of the page hand-copying four of its five fields where the two could drift apart. And the ten-line `DeleteACLsRequest` literal buried mid-object is now a named `allAclsFor(principal, host)`. Co-Authored-By: Claude Opus 5 (1M context) --- .../pages/security/tabs/acls-tab.test.tsx | 12 ++-- .../pages/security/tabs/acls-tab.tsx | 62 +++++++------------ frontend/src/react-query/api/acl.tsx | 2 +- .../acls/acls-tab.spec.ts | 11 ++-- 4 files changed, 33 insertions(+), 54 deletions(-) diff --git a/frontend/src/components/pages/security/tabs/acls-tab.test.tsx b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx index d1106bc8de..75575e7a08 100644 --- a/frontend/src/components/pages/security/tabs/acls-tab.test.tsx +++ b/frontend/src/components/pages/security/tabs/acls-tab.test.tsx @@ -14,11 +14,8 @@ import { render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import type { ReactNode } from 'react'; -/** - * The row-actions menu moved into its own component during the Registry swap, so the delete - * options and their enablement are what needs guarding: the Playwright spec covers the table - * chrome, but a fresh cluster has no ACLs, so no row menu exists there to click. - */ +// 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'); @@ -65,7 +62,7 @@ rs.mock('../../../../react-query/api/cluster-status', () => ({ rs.mock('../../../../react-query/api/user', () => ({ useInvalidateUsersCache: () => rs.fn(), useDeleteUserMutation: () => ({ mutateAsync: rs.fn().mockResolvedValue(undefined) }), - // `scram-admin` and `shadowed` are real SASL users; `acl-only` has ACLs but no account. + // `acl-only` has ACLs but no account. useListUsersQuery: () => ({ data: { users: [{ name: 'scram-admin' }, { name: 'shadowed' }] }, isLoading: false }), })); @@ -76,7 +73,7 @@ rs.mock('../../../../react-query/api/acl', () => ({ { 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 deliberately collides with the `shadowed` SASL user, to pin the principalType guard. + // 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, @@ -132,7 +129,6 @@ describe('AclsTab', () => { test('never offers user deletes on a Group row, even when a user shares its name', async () => { render(); - // Same `principalName` as a SASL user, but a Group has no account to delete. const { menu } = await openRowMenu('shadowed'); expect(within(menu).getByRole('menuitem', { name: 'Delete (User and ACLs)' })).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 3d899c6ab9..4f54ae6a28 100644 --- a/frontend/src/components/pages/security/tabs/acls-tab.tsx +++ b/frontend/src/components/pages/security/tabs/acls-tab.tsx @@ -31,7 +31,7 @@ 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'; @@ -54,29 +54,30 @@ import { AlertDeleteFailed } from '../shared/alert-delete-failed'; import { filterByName } from '../shared/filter-by-name'; import { SecurityTabsNav } from '../shared/security-tabs-nav'; -type AclPrincipalRow = { - principal: string; - host: string; - principalType: string; - principalName: string; -}; +/** 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 a row's open action menu on its own principal when the -// list refetches and the order shifts. +// 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: AclPrincipalRow) => `${row.principal}:${row.host}`, + getRowId: (row: SimpleAcl) => `${row.principal}:${row.host}`, }; -/** - * The row actions read their data from context rather than props so the columns array closes over - * nothing and can live at module scope: `DataTableColumnHeader` is a dropdown trigger, and a new - * header-function identity remounts it, tearing an open sort menu down. Queries stay in the parent - * — `useListUsersQuery` auto-fetches every page from an effect, so one instance per row would fire - * a `fetchNextPage()` per row. - */ +// 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; @@ -88,15 +89,14 @@ type AclRowActionsContextValue = { const AclRowActionsContext = createContext(null); -const AclRowActions: FC<{ record: AclPrincipalRow }> = ({ record }) => { +const AclRowActions: FC<{ record: SimpleAcl }> = ({ record }) => { const ctx = useContext(AclRowActionsContext); if (!ctx) { return null; } const { users, canDeleteUsers, deleteAclsForPrincipal, deleteUser, invalidateUsers, onFailure } = ctx; - // A Group principal never has a SASL account, so only a User row may offer the user deletes — - // a same-named group would otherwise delete an unrelated user. + // 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; @@ -155,12 +155,11 @@ const AclRowActions: FC<{ record: AclPrincipalRow }> = ({ record }) => { ); }; -const columns: DataTableColumnDef[] = [ +const columns: DataTableColumnDef[] = [ { id: 'principal', header: ({ column }) => , - // The cell shows `principalName`; sorting on the `User:`-prefixed `principal` would order - // the column differently from what is on screen. + // Sort on what the cell renders, not the `User:`-prefixed `principal`. accessorFn: (row) => row.principalName, cell: ({ row: { original: record } }) => ( { principalGroups?.filter((g) => g.principalType === 'User' || g.principalType === 'Group') || []; const groups = filterByName(aclPrincipalGroups, searchQuery, (g) => g.principalName); - // Not memoised: the consumers are the row action cells, which re-render with the parent anyway. - // What has to stay stable is `columns`, and that is a module constant. const rowActions: AclRowActionsContextValue = { users: usersData?.users ?? [], canDeleteUsers: Boolean(featureDeleteUser), deleteAclsForPrincipal: async (principal, host) => { - 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); + await deleteACLMutation(allAclsFor(principal, host)); toast.success( Deleted ACLs for {principal} @@ -313,7 +299,7 @@ const AclsTabContent: FC = () => {
- + columns={columns} data={groups} pagination={groups.length > DEFAULT_TABLE_PAGE_SIZE} 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 index a78cf5ae1a..fc8af24341 100644 --- a/frontend/tests/test-variant-console/acls/acls-tab.spec.ts +++ b/frontend/tests/test-variant-console/acls/acls-tab.spec.ts @@ -6,11 +6,9 @@ const PRINCIPAL_HEADER = /^Principal$/; const HOST_HEADER = /^Host$/; const SORT_ASC = /Asc/; -// The ACLs tab is the last security surface still on the old design, and it had no spec of its -// own — only an enterprise authorization test navigates here. What the Registry swap risked is -// the filter field and the table: Chakra painted a sort affordance on every header, while the -// Registry sorts only through `DataTableColumnHeader`, so a swap can leave sorting unreachable -// with nothing failing. A fresh cluster has no ACLs, so this asserts the chrome, not rows. +// 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'); @@ -23,8 +21,7 @@ test.describe('Security ACLs tab', () => { await expect(table.getByRole('button', { name: PRINCIPAL_HEADER })).toBeVisible(); await expect(table.getByRole('button', { name: HOST_HEADER })).toBeVisible(); - // `DataTableColumnHeader` is a dropdown trigger, so the header opens a menu rather than - // sorting on a single click. + // 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');