Skip to content

Commit 8922949

Browse files
committed
fix(security): address review feedback on security page refactor
- Use ConnectError.from() for structured error messages in role creation - Include member name in remove-from-role toast notification - Replace raw <h2> with Heading registry component in acls-card - Use pluralizeWithNumber() for ACL count summary text - Use <Empty> component for inline no-ACLs state in PrincipalRow - Improve empty states in roles and users tabs with <Empty> registry component - Navigate to user details after creation in E2E security-page helper
1 parent 5b6df25 commit 8922949

7 files changed

Lines changed: 200 additions & 20 deletions

File tree

frontend/src/components/pages/security/roles/role-create-dialog.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export const RoleCreateDialog = ({ open, onOpenChange }: RoleCreateDialogProps)
5555
handleClose();
5656
navigate({ to: '/security/roles/$roleName/details', params: { roleName: encodeURIComponent(trimmed) } });
5757
} catch (err) {
58-
toast.error(`Failed to create role: ${err instanceof Error ? err.message : String(err)}`);
58+
toast.error(`Failed to create role: ${ConnectError.from(err).message}`);
5959
} finally {
6060
setIsSubmitting(false);
6161
}

frontend/src/components/pages/security/roles/role-detail-page.tsx

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
import { create } from '@bufbuild/protobuf';
1313
import { getRouteApi } from '@tanstack/react-router';
14-
import { Trash2 } from 'lucide-react';
14+
import { Trash2, Users2Icon } from 'lucide-react';
1515
import {
1616
ListRoleMembersRequestSchema,
1717
UpdateRoleMembershipRequestSchema,
@@ -26,6 +26,14 @@ import { useListUsersQuery } from '../../../../react-query/api/user';
2626
import { setPageHeader } from '../../../../state/ui-state';
2727
import { Button } from '../../../redpanda-ui/components/button';
2828
import { Combobox } from '../../../redpanda-ui/components/combobox';
29+
import {
30+
Empty,
31+
EmptyContent,
32+
EmptyDescription,
33+
EmptyHeader,
34+
EmptyMedia,
35+
EmptyTitle,
36+
} from '../../../redpanda-ui/components/empty';
2937
import { ListLayout, ListLayoutContent, ListLayoutFilters } from '../../../redpanda-ui/components/list-layout';
3038
import { Table, TableBody, TableCell, TableRow } from '../../../redpanda-ui/components/table';
3139
import { parsePrincipal } from '../shared/acl-model';
@@ -97,7 +105,7 @@ const RoleDetailPage = () => {
97105
create: false,
98106
})
99107
);
100-
toast.success('Member removed from role successfully');
108+
toast.success(`User "${parsePrincipal(memberPrincipal).name}" removed from role`);
101109
} catch {
102110
// Error handled by onError in mutation
103111
} finally {
@@ -131,7 +139,28 @@ const RoleDetailPage = () => {
131139
{membersLoading ? (
132140
<div className="py-4 text-center text-muted-foreground text-sm">Loading members...</div>
133141
) : allMembers.length === 0 ? (
134-
<p className="text-muted-foreground text-sm">No principals assigned to this role.</p>
142+
<Empty>
143+
<EmptyHeader>
144+
<EmptyMedia variant="icon">
145+
<Users2Icon />
146+
</EmptyMedia>
147+
<EmptyTitle>No principals assigned</EmptyTitle>
148+
<EmptyDescription>
149+
Assign users to this role to grant them its permissions. Use the dropdown above to add a principal.
150+
</EmptyDescription>
151+
</EmptyHeader>
152+
<EmptyContent>
153+
<Button asChild variant="link">
154+
<a
155+
href="https://docs.redpanda.com/current/manage/security/authorization/rbac/"
156+
rel="noopener noreferrer"
157+
target="_blank"
158+
>
159+
Read the docs →
160+
</a>
161+
</Button>
162+
</EmptyContent>
163+
</Empty>
135164
) : (
136165
<Table>
137166
<TableBody>

frontend/src/components/pages/security/shared/acls-card.tsx

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
*/
1111

1212
import { create } from '@bufbuild/protobuf';
13+
import { KeyRoundIcon } from 'lucide-react';
1314
import {
1415
ACL_Operation,
1516
ACL_PermissionType,
@@ -40,6 +41,14 @@ import {
4041
DialogHeader,
4142
DialogTitle,
4243
} from '../../../redpanda-ui/components/dialog';
44+
import {
45+
Empty,
46+
EmptyContent,
47+
EmptyDescription,
48+
EmptyHeader,
49+
EmptyMedia,
50+
EmptyTitle,
51+
} from '../../../redpanda-ui/components/empty';
4352
import { ListLayout, ListLayoutContent, ListLayoutFilters } from '../../../redpanda-ui/components/list-layout';
4453
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '../../../redpanda-ui/components/table';
4554
import { AddAclDialog } from '../users/add-acl-dialog';
@@ -201,11 +210,34 @@ export const AclsCard = ({ acls, principal }: AclsCardProps) => {
201210
</div>
202211
}
203212
>
204-
<h2 className="font-semibold text-base">ACLs</h2>
213+
<Heading as="h2" level={4}>
214+
ACLs
215+
</Heading>
205216
</ListLayoutFilters>
206217
<ListLayoutContent>
207218
{rows.length === 0 ? (
208-
<p>No ACLs assigned.</p>
219+
<Empty>
220+
<EmptyHeader>
221+
<EmptyMedia variant="icon">
222+
<KeyRoundIcon />
223+
</EmptyMedia>
224+
<EmptyTitle>No ACLs assigned</EmptyTitle>
225+
<EmptyDescription>
226+
Add ACLs to define what operations this role can perform on cluster resources.
227+
</EmptyDescription>
228+
</EmptyHeader>
229+
<EmptyContent>
230+
<Button asChild variant="link">
231+
<a
232+
href="https://docs.redpanda.com/current/manage/security/authorization/acls/"
233+
rel="noopener noreferrer"
234+
target="_blank"
235+
>
236+
Read the docs →
237+
</a>
238+
</Button>
239+
</EmptyContent>
240+
</Empty>
209241
) : (
210242
<Table>
211243
<TableHeader>

frontend/src/components/pages/security/tabs/permissions-list-tab.tsx

Lines changed: 48 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,16 @@
1212
import { create } from '@bufbuild/protobuf';
1313
import { Link } from '@tanstack/react-router';
1414
import { MoreHorizontalIcon } from 'components/icons';
15+
import {
16+
Empty,
17+
EmptyContent,
18+
EmptyDescription,
19+
EmptyHeader,
20+
EmptyMedia,
21+
EmptyTitle,
22+
} from 'components/redpanda-ui/components/empty';
1523
import { ListLayout, ListLayoutContent, ListLayoutFilters } from 'components/redpanda-ui/components/list-layout';
16-
import { ChevronDown, ChevronRight, ExternalLink, ShieldIcon } from 'lucide-react';
24+
import { ChevronDown, ChevronRight, ExternalLink, KeyRoundIcon, ShieldIcon } from 'lucide-react';
1725
import { parseAsString, useQueryState } from 'nuqs';
1826
import {
1927
ACL_Operation,
@@ -88,13 +96,13 @@ const PrincipalRow: FC<PrincipalRowProps> = ({ group, isExpanded, onToggle, onDe
8896

8997
const summaryText = (() => {
9098
if (group.directAclCount > 0 && group.inheritedAclCount > 0) {
91-
return `${group.directAclCount} direct ACL${group.directAclCount !== 1 ? 's' : ''}, ${group.inheritedAclCount} ACL${group.inheritedAclCount !== 1 ? 's' : ''} inherited from roles`;
99+
return `${pluralizeWithNumber(group.directAclCount, 'direct ACL')}, ${pluralizeWithNumber(group.inheritedAclCount, 'ACL')} inherited from roles`;
92100
}
93101
if (group.inheritedAclCount > 0) {
94-
return `${group.inheritedAclCount} ACL${group.inheritedAclCount !== 1 ? 's' : ''} inherited from roles`;
102+
return `${pluralizeWithNumber(group.inheritedAclCount, 'ACL')} inherited from roles`;
95103
}
96104
if (group.directAclCount > 0) {
97-
return `${group.directAclCount} direct ACL${group.directAclCount !== 1 ? 's' : ''}`;
105+
return pluralizeWithNumber(group.directAclCount, 'direct ACL');
98106
}
99107
return 'No ACLs';
100108
})();
@@ -240,7 +248,11 @@ const PrincipalRow: FC<PrincipalRowProps> = ({ group, isExpanded, onToggle, onDe
240248
)}
241249

242250
{isExpanded && !hasAcls && (
243-
<div className="border-t px-3 py-4 text-muted-foreground text-sm">No ACLs assigned.</div>
251+
<Empty className="rounded-none border-x-0 border-b-0 border-dashed py-6">
252+
<EmptyHeader>
253+
<EmptyTitle>No ACLs assigned</EmptyTitle>
254+
</EmptyHeader>
255+
</Empty>
244256
)}
245257
</div>
246258
</>
@@ -378,9 +390,37 @@ export const PermissionsListTab: FC = () => {
378390
{isAclsLoading ? (
379391
<div className="py-8 text-center text-muted-foreground text-sm">Loading...</div>
380392
) : filteredGroups.length === 0 ? (
381-
<div className="py-8 text-center text-muted-foreground text-sm">
382-
{searchQuery ? 'No principals match your search.' : 'No principals yet.'}
383-
</div>
393+
searchQuery ? (
394+
<div className="py-8 text-center text-muted-foreground text-sm">No principals match your search.</div>
395+
) : (
396+
<div className="rounded-md border">
397+
<Empty>
398+
<EmptyHeader>
399+
<EmptyMedia variant="icon">
400+
<KeyRoundIcon />
401+
</EmptyMedia>
402+
<EmptyTitle>No permissions yet</EmptyTitle>
403+
<EmptyDescription>
404+
A unified view of all principal permissions across your cluster. Create an ACL to get started.
405+
</EmptyDescription>
406+
</EmptyHeader>
407+
<EmptyContent>
408+
<div className="flex items-center gap-3">
409+
<Button onClick={() => setCreateAclOpen(true)}>Create ACL</Button>
410+
<Button asChild variant="link">
411+
<a
412+
href="https://docs.redpanda.com/current/manage/security/authorization/acls/"
413+
rel="noopener noreferrer"
414+
target="_blank"
415+
>
416+
Read the docs →
417+
</a>
418+
</Button>
419+
</div>
420+
</EmptyContent>
421+
</Empty>
422+
</div>
423+
)
384424
) : (
385425
<div className="rounded-md border">
386426
{filteredGroups.map((group) => (

frontend/src/components/pages/security/tabs/roles-tab.tsx

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,22 @@ import {
3434
DropdownMenuItem,
3535
DropdownMenuTrigger,
3636
} from 'components/redpanda-ui/components/dropdown-menu';
37+
import {
38+
Empty,
39+
EmptyContent,
40+
EmptyDescription,
41+
EmptyHeader,
42+
EmptyMedia,
43+
EmptyTitle,
44+
} from 'components/redpanda-ui/components/empty';
3745
import {
3846
ListLayout,
3947
ListLayoutContent,
4048
ListLayoutFilters,
4149
ListLayoutPagination,
4250
ListLayoutSearchInput,
4351
} from 'components/redpanda-ui/components/list-layout';
52+
import { ShieldCheckIcon } from 'lucide-react';
4453
import { parseAsString, useQueryStates } from 'nuqs';
4554
import { DeleteRoleRequestSchema } from 'protogen/redpanda/api/dataplane/v1/security_pb';
4655
import type { FC } from 'react';
@@ -264,9 +273,36 @@ export const RolesTab: FC = () => {
264273
</TableRow>
265274
))
266275
) : (
267-
<TableRow>
268-
<TableCell className="text-center text-muted-foreground" colSpan={columns.length}>
269-
No roles yet.
276+
<TableRow className="hover:bg-transparent!">
277+
<TableCell colSpan={columns.length}>
278+
<Empty>
279+
<EmptyHeader>
280+
<EmptyMedia variant="icon">
281+
<ShieldCheckIcon />
282+
</EmptyMedia>
283+
<EmptyTitle>No roles yet</EmptyTitle>
284+
<EmptyDescription>
285+
Roles are groups of ACLs that can be assigned to principals. Create one to start managing
286+
access control.
287+
</EmptyDescription>
288+
</EmptyHeader>
289+
<EmptyContent>
290+
<div className="flex items-center gap-3">
291+
<Button disabled={createRoleDisabled} onClick={() => setCreateDialogOpen(true)}>
292+
Create role
293+
</Button>
294+
<Button asChild variant="link">
295+
<a
296+
href="https://docs.redpanda.com/current/manage/security/authorization/rbac/"
297+
rel="noopener noreferrer"
298+
target="_blank"
299+
>
300+
Read the docs →
301+
</a>
302+
</Button>
303+
</div>
304+
</EmptyContent>
305+
</Empty>
270306
</TableCell>
271307
</TableRow>
272308
)}

frontend/src/components/pages/security/tabs/users-tab.tsx

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,22 @@ import {
2929
} from '@tanstack/react-table';
3030
import { MoreHorizontalIcon } from 'components/icons';
3131
import { DescriptionWithHelp } from 'components/pages/security/shared/description-with-help';
32+
import {
33+
Empty,
34+
EmptyContent,
35+
EmptyDescription,
36+
EmptyHeader,
37+
EmptyMedia,
38+
EmptyTitle,
39+
} from 'components/redpanda-ui/components/empty';
3240
import {
3341
ListLayout,
3442
ListLayoutContent,
3543
ListLayoutFilters,
3644
ListLayoutPagination,
3745
ListLayoutSearchInput,
3846
} from 'components/redpanda-ui/components/list-layout';
47+
import { UsersIcon } from 'lucide-react';
3948
import { parseAsArrayOf, parseAsString, useQueryStates } from 'nuqs';
4049
import type { FC } from 'react';
4150
import { useCallback, useLayoutEffect, useMemo, useState } from 'react';
@@ -342,9 +351,41 @@ export const UsersTab: FC = () => {
342351
</TableRow>
343352
))
344353
) : (
345-
<TableRow>
346-
<TableCell className="text-center text-muted-foreground" colSpan={columns.length}>
347-
No users yet.
354+
<TableRow className="hover:bg-transparent!">
355+
<TableCell colSpan={columns.length}>
356+
<Empty>
357+
<EmptyHeader>
358+
<EmptyMedia variant="icon">
359+
<UsersIcon />
360+
</EmptyMedia>
361+
<EmptyTitle>No users yet</EmptyTitle>
362+
<EmptyDescription>
363+
SASL-SCRAM user accounts managed by your cluster. Create one to start managing access.
364+
</EmptyDescription>
365+
</EmptyHeader>
366+
<EmptyContent>
367+
<div className="flex items-center gap-3">
368+
<Button
369+
disabled={createDisabled}
370+
onClick={() => {
371+
setCreateDialogKey((k) => k + 1);
372+
setIsCreateDialogOpen(true);
373+
}}
374+
>
375+
Create user
376+
</Button>
377+
<Button asChild variant="link">
378+
<a
379+
href="https://docs.redpanda.com/current/manage/security/authentication/scram/"
380+
rel="noopener noreferrer"
381+
target="_blank"
382+
>
383+
Read the docs →
384+
</a>
385+
</Button>
386+
</div>
387+
</EmptyContent>
388+
</Empty>
348389
</TableCell>
349390
</TableRow>
350391
)}

frontend/tests/test-variant-console/utils/security-page.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ export class SecurityPage {
6565
await this.fillUsername(username);
6666
await this.submitUserCreation();
6767
await this.page.getByTestId('user-created-successfully').waitFor({ state: 'visible' });
68+
await this.page.getByTestId('go-to-user-details-button').click();
69+
await this.page.waitForURL(`**/security/users/${username}/details`);
6870
});
6971
}
7072

0 commit comments

Comments
 (0)