Skip to content

Commit 453fae1

Browse files
feat: [UIE-9204] - IAM RBAC: replace grants in Firewalls (linode#12902)
* feat: [UIE-9204] - IAM RBAC: replace grants for nodebalancer * replace grants for linodes * fix menu button * fix perm check for menu * Added changeset: IAM RBAC: replace grants with usePermission hook for Firewalls * clean up * minor changes * add loading state to usePermissions hook * fix loading state
1 parent 7c97d28 commit 453fae1

17 files changed

+206
-205
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@linode/manager": Changed
3+
---
4+
5+
IAM RBAC: replace grants with usePermission hook for Firewalls ([#12902](https://github.com/linode/manager/pull/12902))
Lines changed: 18 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import { screen } from '@testing-library/react';
2-
import userEvent from '@testing-library/user-event';
1+
import { waitFor } from '@testing-library/react';
32
import * as React from 'react';
43

54
import { renderWithTheme } from 'src/utilities/testHelpers';
@@ -13,19 +12,20 @@ const props = {
1312
helperText,
1413
onClose,
1514
open: true,
15+
disabled: true,
1616
};
1717

1818
const queryMocks = vi.hoisted(() => ({
1919
useParams: vi.fn().mockReturnValue({}),
20-
userPermissions: vi.fn(() => ({
21-
data: {
22-
create_firewall_device: false,
23-
},
24-
})),
20+
useQueryWithPermissions: vi.fn().mockReturnValue({
21+
data: [],
22+
isLoading: false,
23+
isError: false,
24+
}),
2525
}));
2626

2727
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
28-
usePermissions: queryMocks.userPermissions,
28+
useQueryWithPermissions: queryMocks.useQueryWithPermissions,
2929
}));
3030

3131
vi.mock('@tanstack/react-router', async () => {
@@ -62,20 +62,11 @@ describe('AddLinodeDrawer', () => {
6262
});
6363

6464
it('should disable "Add" button if the user does not have create_firewall_device permission', async () => {
65-
queryMocks.userPermissions.mockReturnValue({
66-
data: {
67-
create_firewall_device: false,
68-
},
69-
});
70-
7165
const { getByRole } = renderWithTheme(<AddLinodeDrawer {...props} />);
7266

73-
const autocomplete = screen.getByRole('combobox');
74-
await userEvent.click(autocomplete);
75-
await userEvent.type(autocomplete, 'linode-5');
76-
77-
const option = await screen.findByText('linode-5');
78-
await userEvent.click(option);
67+
const select = getByRole('combobox');
68+
expect(select).toBeInTheDocument();
69+
expect(select).toBeDisabled();
7970

8071
const addButton = getByRole('button', {
8172
name: 'Add',
@@ -85,25 +76,15 @@ describe('AddLinodeDrawer', () => {
8576
});
8677

8778
it('should enable "Add" button if the user has create_firewall_device permission', async () => {
88-
queryMocks.userPermissions.mockReturnValue({
89-
data: {
90-
create_firewall_device: true,
91-
},
92-
});
93-
94-
const { getByRole } = renderWithTheme(<AddLinodeDrawer {...props} />);
95-
96-
const autocomplete = screen.getByRole('combobox');
97-
await userEvent.click(autocomplete);
98-
await userEvent.type(autocomplete, 'linode-5');
79+
const { getByRole } = renderWithTheme(
80+
<AddLinodeDrawer {...props} disabled={false} />
81+
);
9982

100-
const option = await screen.findByText('linode-5');
101-
await userEvent.click(option);
83+
const select = getByRole('combobox');
84+
expect(select).toBeInTheDocument();
10285

103-
const addButton = getByRole('button', {
104-
name: 'Add',
86+
await waitFor(() => {
87+
expect(select).toBeEnabled();
10588
});
106-
expect(addButton).toBeInTheDocument();
107-
expect(addButton).toBeEnabled();
10889
});
10990
});

packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddLinodeDrawer.tsx

Lines changed: 24 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ import {
33
useAddFirewallDeviceMutation,
44
useAllFirewallsQuery,
55
useAllLinodesQuery,
6-
useGrants,
7-
useProfile,
86
} from '@linode/queries';
97
import { LinodeSelect } from '@linode/shared';
108
import {
@@ -14,7 +12,6 @@ import {
1412
Notice,
1513
Typography,
1614
} from '@linode/ui';
17-
import { getEntityIdsByPermission } from '@linode/utilities';
1815
import { useTheme } from '@mui/material';
1916
import { useQueries } from '@tanstack/react-query';
2017
import { useParams } from '@tanstack/react-router';
@@ -23,7 +20,8 @@ import * as React from 'react';
2320

2421
import { Link } from 'src/components/Link';
2522
import { SupportLink } from 'src/components/SupportLink';
26-
import { usePermissions } from 'src/features/IAM/hooks/usePermissions';
23+
import { getRestrictedResourceText } from 'src/features/Account/utils';
24+
import { useQueryWithPermissions } from 'src/features/IAM/hooks/usePermissions';
2725
import { getLinodeInterfaceType } from 'src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/utilities';
2826
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
2927
import { useIsLinodeInterfacesEnabled } from 'src/utilities/linodes';
@@ -32,6 +30,7 @@ import { sanitizeHTML } from 'src/utilities/sanitizeHTML';
3230
import type { Linode, LinodeInterfaces } from '@linode/api-v4';
3331

3432
interface Props {
33+
disabled: boolean;
3534
helperText: string;
3635
onClose: () => void;
3736
open: boolean;
@@ -44,31 +43,28 @@ interface InterfaceDeviceInfo {
4443
}
4544

4645
export const AddLinodeDrawer = (props: Props) => {
47-
const { helperText, onClose, open } = props;
46+
const { helperText, onClose, open, disabled } = props;
4847

4948
const { id } = useParams({ strict: false });
5049

5150
const { enqueueSnackbar } = useSnackbar();
5251

53-
const { data: grants } = useGrants();
54-
const { data: profile } = useProfile();
5552
const { isLinodeInterfacesEnabled } = useIsLinodeInterfacesEnabled();
56-
const isRestrictedUser = Boolean(profile?.restricted);
5753

5854
const { data, error, isLoading } = useAllFirewallsQuery();
5955

6056
const firewall = data?.find((firewall) => firewall.id === Number(id));
6157

62-
const { data: permissions } = usePermissions(
63-
'firewall',
64-
['create_firewall_device'],
65-
firewall?.id
66-
);
67-
68-
const { data: allLinodes } = useAllLinodesQuery({}, {});
58+
const { data: availableLinodes, isLoading: availableLinodesLoading } =
59+
useQueryWithPermissions<Linode>(
60+
useAllLinodesQuery({}, {}),
61+
'linode',
62+
['update_linode'],
63+
open
64+
);
6965

7066
const linodesUsingLinodeInterfaces =
71-
allLinodes?.filter((l) => l.interface_generation === 'linode') ?? [];
67+
availableLinodes?.filter((l) => l.interface_generation === 'linode') ?? [];
7268

7369
const allFirewallEntities = React.useMemo(
7470
() => data?.map((firewall) => firewall.entities).flat() ?? [],
@@ -90,15 +86,6 @@ export const AddLinodeDrawer = (props: Props) => {
9086
[allFirewallEntities]
9187
);
9288

93-
// If a user is restricted, they can not add a read-only Linode to a firewall.
94-
const readOnlyLinodeIds = React.useMemo(
95-
() =>
96-
isRestrictedUser
97-
? getEntityIdsByPermission(grants, 'linode', 'read_only')
98-
: [],
99-
[grants, isRestrictedUser]
100-
);
101-
10289
// Keeps track of Linode and its eligible Linode Interfaces if they exist (eligible = a non-vlan interface that isn't already assigned to a firewall)
10390
// Key is Linode ID. Value is an object containing the Linode object and the Linode's interfaces
10491
const linodesAndEligibleInterfaces = useQueries({
@@ -130,12 +117,7 @@ export const AddLinodeDrawer = (props: Props) => {
130117
},
131118
});
132119

133-
const linodeOptions = allLinodes?.filter((linode) => {
134-
// Exclude read only Linodes
135-
if (readOnlyLinodeIds.includes(linode.id)) {
136-
return false;
137-
}
138-
120+
const linodeOptions = availableLinodes?.filter((linode) => {
139121
// Exclude a Linode if it uses Linode Interfaces but has no eligible interfaces
140122
if (linode.interface_generation === 'linode') {
141123
return Boolean(linodesAndEligibleInterfaces[linode.id]);
@@ -366,10 +348,19 @@ export const AddLinodeDrawer = (props: Props) => {
366348
handleSubmit();
367349
}}
368350
>
351+
{disabled && (
352+
<Notice
353+
text={getRestrictedResourceText({
354+
resourceType: 'Firewalls',
355+
})}
356+
variant="error"
357+
/>
358+
)}
369359
{localError ? errorNotice() : null}
370360
<LinodeSelect
371-
disabled={isLoading}
361+
disabled={isLoading || availableLinodesLoading || disabled}
372362
helperText={helperText}
363+
loading={availableLinodesLoading}
373364
multiple
374365
onSelectionChange={(linodes) => onSelectionChange(linodes)}
375366
options={linodeOptions}
@@ -416,9 +407,7 @@ export const AddLinodeDrawer = (props: Props) => {
416407
})}
417408
<ActionsPanel
418409
primaryButtonProps={{
419-
disabled:
420-
selectedLinodes.length === 0 ||
421-
!permissions.create_firewall_device,
410+
disabled: selectedLinodes.length === 0 || disabled,
422411
label: 'Add',
423412
loading: addDeviceIsLoading,
424413
onClick: handleSubmit,

packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddNodebalancerDrawer.test.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ const props = {
1717

1818
const queryMocks = vi.hoisted(() => ({
1919
useParams: vi.fn().mockReturnValue({}),
20+
userPermissions: vi.fn(() => ({
21+
data: {
22+
create_firewall_device: true,
23+
},
24+
})),
25+
useQueryWithPermissions: vi.fn().mockReturnValue({
26+
data: [],
27+
isLoading: false,
28+
isError: false,
29+
}),
30+
}));
31+
32+
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
33+
usePermissions: queryMocks.userPermissions,
34+
useQueryWithPermissions: queryMocks.useQueryWithPermissions,
2035
}));
2136

2237
vi.mock('@tanstack/react-router', async () => {

packages/manager/src/features/Firewalls/FirewallDetail/Devices/AddNodebalancerDrawer.tsx

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
import {
22
useAddFirewallDeviceMutation,
33
useAllFirewallsQuery,
4-
useGrants,
5-
useProfile,
64
} from '@linode/queries';
75
import { ActionsPanel, Drawer, Notice } from '@linode/ui';
8-
import { getEntityIdsByPermission } from '@linode/utilities';
96
import { useTheme } from '@mui/material';
107
import { useParams } from '@tanstack/react-router';
118
import { useSnackbar } from 'notistack';
@@ -32,9 +29,6 @@ export const AddNodebalancerDrawer = (props: Props) => {
3229
const { helperText, onClose, open, disabled } = props;
3330
const { enqueueSnackbar } = useSnackbar();
3431
const { id } = useParams({ strict: false });
35-
const { data: grants } = useGrants();
36-
const { data: profile } = useProfile();
37-
const isRestrictedUser = Boolean(profile?.restricted);
3832

3933
const { data, error, isLoading } = useAllFirewallsQuery(open);
4034

@@ -149,20 +143,14 @@ export const AddNodebalancerDrawer = (props: Props) => {
149143
}
150144
};
151145

152-
// If a user is restricted, they can not add a read-only Nodebalancer to a firewall.
153-
const readOnlyNodebalancerIds = isRestrictedUser
154-
? getEntityIdsByPermission(grants, 'nodebalancer', 'read_only')
155-
: [];
156-
157146
const assignedNodeBalancers = data
158147
?.map((firewall) => firewall.entities)
159148
.flat()
160149
?.filter((service) => service.type === 'nodebalancer');
161150

162151
const nodebalancerOptionsFilter = (nodebalancer: NodeBalancer) => {
163-
return (
164-
!readOnlyNodebalancerIds.includes(nodebalancer.id) &&
165-
!assignedNodeBalancers?.some((service) => service.id === nodebalancer.id)
152+
return !assignedNodeBalancers?.some(
153+
(service) => service.id === nodebalancer.id
166154
);
167155
};
168156

packages/manager/src/features/Firewalls/FirewallDetail/Devices/FirewallDeviceActionMenu.tsx

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ export interface ActionHandlers {
66
handleRemoveDevice: (device: FirewallDevice) => void;
77
}
88

9+
import { usePermissions } from 'src/features/IAM/hooks/usePermissions';
10+
911
import type { FirewallDevice } from '@linode/api-v4';
1012

1113
export interface FirewallDeviceActionMenuProps extends ActionHandlers {
@@ -17,12 +19,48 @@ export const FirewallDeviceActionMenu = React.memo(
1719
(props: FirewallDeviceActionMenuProps) => {
1820
const { device, disabled, handleRemoveDevice } = props;
1921

22+
const { type } = device.entity;
23+
24+
const { data: linodePermissions, isLoading: isLinodePermissionsLoading } =
25+
usePermissions(
26+
'linode',
27+
['update_linode'],
28+
device?.entity.id,
29+
type !== 'nodebalancer'
30+
);
31+
32+
const {
33+
data: nodebalancerPermissions,
34+
isLoading: isNodebalancerPermissionsLoading,
35+
} = usePermissions(
36+
'nodebalancer',
37+
['update_nodebalancer'],
38+
device?.entity.id,
39+
type === 'nodebalancer'
40+
);
41+
42+
const disabledDueToPermissions =
43+
type === 'nodebalancer'
44+
? !nodebalancerPermissions?.update_nodebalancer
45+
: !linodePermissions?.update_linode;
46+
47+
const isPermissionsLoading =
48+
type === 'nodebalancer'
49+
? isNodebalancerPermissionsLoading
50+
: isLinodePermissionsLoading;
51+
2052
return (
2153
<InlineMenuAction
2254
actionText="Remove"
23-
disabled={disabled}
55+
disabled={disabled || disabledDueToPermissions}
2456
key="Remove"
57+
loading={isPermissionsLoading}
2558
onClick={() => handleRemoveDevice(device)}
59+
tooltip={
60+
disabledDueToPermissions
61+
? `You do not have permission to modify this ${type === 'nodebalancer' ? 'NodeBalancer' : 'Linode'}.`
62+
: undefined
63+
}
2664
/>
2765
);
2866
}

packages/manager/src/features/Firewalls/FirewallDetail/Devices/FirewallDeviceLanding.test.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ const queryMocks = vi.hoisted(() => ({
2323
create_firewall_device: false,
2424
},
2525
})),
26+
useQueryWithPermissions: vi.fn().mockReturnValue({
27+
data: [],
28+
isLoading: false,
29+
isError: false,
30+
}),
2631
}));
2732

2833
vi.mock('@tanstack/react-router', async () => {
@@ -46,6 +51,7 @@ vi.mock('src/hooks/useOrderV2', async () => {
4651

4752
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
4853
usePermissions: queryMocks.usePermissions,
54+
useQueryWithPermissions: queryMocks.useQueryWithPermissions,
4955
}));
5056

5157
const baseProps = (

0 commit comments

Comments
 (0)