From 1752a984223b17f976fc08f48f48cbedde445b28 Mon Sep 17 00:00:00 2001 From: Michael Myers Date: Wed, 8 Jan 2025 09:52:28 -0600 Subject: [PATCH] Display View button if user can list roles This fixes a regression that caused users who could list/read roles but NOT edit them to be unable to view the details of the role. This PR will change the text to "view details" if the user can _only_ view and not edit. The edit button in the role editor already has logic to prevent and tell the user they do not have permissions to edit, so no further changes need to be made there. --- .../teleport/src/Roles/RoleList/RoleList.tsx | 11 +++- .../teleport/src/Roles/Roles.test.tsx | 50 ++++++++++++++++--- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/web/packages/teleport/src/Roles/RoleList/RoleList.tsx b/web/packages/teleport/src/Roles/RoleList/RoleList.tsx index 0bff31387f739..53886c269952f 100644 --- a/web/packages/teleport/src/Roles/RoleList/RoleList.tsx +++ b/web/packages/teleport/src/Roles/RoleList/RoleList.tsx @@ -41,6 +41,7 @@ export function RoleList({ serversidePagination: SeversidePagination; rolesAcl: Access; }) { + const canView = rolesAcl.list && rolesAcl.read; const canEdit = rolesAcl.edit; const canDelete = rolesAcl.remove; @@ -74,6 +75,7 @@ export function RoleList({ altKey: 'options-btn', render: (role: RoleResource) => ( onEdit(role.id)} @@ -89,18 +91,23 @@ export function RoleList({ } const ActionCell = (props: { + canView: boolean; canEdit: boolean; canDelete: boolean; onEdit(): void; onDelete(): void; }) => { - if (!(props.canEdit || props.canDelete)) { + if (!(props.canView || props.canDelete)) { return ; } return ( - {props.canEdit && Edit} + {props.canView && ( + + {props.canEdit ? 'Edit' : 'View Details'} + + )} {props.canDelete && ( Delete )} diff --git a/web/packages/teleport/src/Roles/Roles.test.tsx b/web/packages/teleport/src/Roles/Roles.test.tsx index d3996a9d3377b..81281c9d0b3c7 100644 --- a/web/packages/teleport/src/Roles/Roles.test.tsx +++ b/web/packages/teleport/src/Roles/Roles.test.tsx @@ -119,13 +119,13 @@ describe('Roles list', () => { expect(menuItems).toHaveLength(2); }); - test('hides edit button if no access', async () => { + test('hides view/edit button if no access', async () => { const ctx = createTeleportContext(); const testState = { ...defaultState, rolesAcl: { ...defaultState.rolesAcl, - edit: false, + list: false, }, }; @@ -146,12 +146,15 @@ describe('Roles list', () => { fireEvent.click(optionsButton); const menuItems = screen.queryAllByRole('menuitem'); expect(menuItems).toHaveLength(1); - expect(menuItems.every(item => item.textContent.includes('Edit'))).not.toBe( - true - ); + expect( + menuItems.every( + item => + item.textContent.includes('View') || item.textContent.includes('Edit') + ) + ).not.toBe(true); }); - test('hides delete button if no access', async () => { + test('hides delete button if user does not have permission to delete', async () => { const ctx = createTeleportContext(); const testState = { ...defaultState, @@ -183,12 +186,14 @@ describe('Roles list', () => { ).not.toBe(true); }); - test('hides Options button if no permissions to edit or delete', async () => { + test('displays Options button if user has permission to list/read roles', async () => { const ctx = createTeleportContext(); const testState = { ...defaultState, rolesAcl: { - ...defaultState.rolesAcl, + list: true, + read: true, + create: false, remove: false, edit: false, }, @@ -202,6 +207,35 @@ describe('Roles list', () => { ); + await waitFor(() => { + expect(screen.getByText('cool-role')).toBeInTheDocument(); + }); + const optionsButton = screen.getByRole('button', { name: /options/i }); + fireEvent.click(optionsButton); + const menuItems = screen.queryAllByRole('menuitem'); + expect(menuItems).toHaveLength(1); + expect(menuItems[0]).toHaveTextContent('View'); + }); + + test('hides Options button if no permissions to view or delete', async () => { + const ctx = createTeleportContext(); + const testState = { + ...defaultState, + rolesAcl: { + ...defaultState.rolesAcl, + remove: false, + list: false, + }, + }; + + render( + + + + + + ); + await waitFor(() => { expect(screen.getByText('cool-role')).toBeInTheDocument(); });