Skip to content

Commit 5cbf8db

Browse files
mpolotsk-akamaismans-akamairodonnel-akamaicorya-akamaimjac0bs
authored
feat: [UIE-8871] - IAM RBAC: Firewall linodes permissions check (#12500)
* feat: [UIE-8871] - firewall linodes permissions check * Added changeset: Implement the new RBAC permission hook in Firewall Linodes tab * applying feedback: part 1 * Fixing linting errors and updating tests * Fix Kamils concerns * Delete changeset, update changelog entry --------- Co-authored-by: Sam Mans <[email protected]> Co-authored-by: rodonnel-akamai <[email protected]> Co-authored-by: Conal Ryan <[email protected]> Co-authored-by: mjac0bs <[email protected]>
1 parent d663a46 commit 5cbf8db

File tree

7 files changed

+165
-23
lines changed

7 files changed

+165
-23
lines changed

packages/manager/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
7878
- Edit node pool configuration in LKE cluster create flow ([#12552](https://github.com/linode/manager/pull/12552))
7979
- IAM RBAC: fix error message for the one account_admin in the account ([#12556](https://github.com/linode/manager/pull/12556))
8080
- Show GPU warning notice conditionally based on policy type - display for "migrate" policy but hide for "power-off-on" policy ([#12512](https://github.com/linode/manager/pull/12512))
81+
- IAM RBAC: Implement the new RBAC permission hook in Firewall Linodes tab ([#12500](https://github.com/linode/manager/pull/12500))
82+
8183

8284
## [2025-07-21] - v1.146.2
8385

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
13
import * as React from 'react';
24

35
import { renderWithTheme } from 'src/utilities/testHelpers';
@@ -15,6 +17,15 @@ const props = {
1517

1618
const queryMocks = vi.hoisted(() => ({
1719
useParams: vi.fn().mockReturnValue({}),
20+
userPermissions: vi.fn(() => ({
21+
permissions: {
22+
create_firewall_device: false,
23+
},
24+
})),
25+
}));
26+
27+
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
28+
usePermissions: queryMocks.userPermissions,
1829
}));
1930

2031
vi.mock('@tanstack/react-router', async () => {
@@ -49,4 +60,50 @@ describe('AddLinodeDrawer', () => {
4960
const { getByText } = renderWithTheme(<AddLinodeDrawer {...props} />);
5061
expect(getByText('Add')).toBeInTheDocument();
5162
});
63+
64+
it('should disable "Add" button if the user does not have create_firewall_device permission', async () => {
65+
queryMocks.userPermissions.mockReturnValue({
66+
permissions: {
67+
create_firewall_device: false,
68+
},
69+
});
70+
71+
const { getByRole } = renderWithTheme(<AddLinodeDrawer {...props} />);
72+
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);
79+
80+
const addButton = getByRole('button', {
81+
name: 'Add',
82+
});
83+
expect(addButton).toBeInTheDocument();
84+
expect(addButton).toBeDisabled();
85+
});
86+
87+
it('should enable "Add" button if the user has create_firewall_device permission', async () => {
88+
queryMocks.userPermissions.mockReturnValue({
89+
permissions: {
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');
99+
100+
const option = await screen.findByText('linode-5');
101+
await userEvent.click(option);
102+
103+
const addButton = getByRole('button', {
104+
name: 'Add',
105+
});
106+
expect(addButton).toBeInTheDocument();
107+
expect(addButton).toBeEnabled();
108+
});
52109
});

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import * as React from 'react';
2323

2424
import { Link } from 'src/components/Link';
2525
import { SupportLink } from 'src/components/SupportLink';
26+
import { usePermissions } from 'src/features/IAM/hooks/usePermissions';
2627
import { getLinodeInterfaceType } from 'src/features/Linodes/LinodesDetail/LinodeNetworking/LinodeInterfaces/utilities';
2728
import { getAPIErrorOrDefault } from 'src/utilities/errorUtils';
2829
import { useIsLinodeInterfacesEnabled } from 'src/utilities/linodes';
@@ -58,6 +59,12 @@ export const AddLinodeDrawer = (props: Props) => {
5859

5960
const firewall = data?.find((firewall) => firewall.id === Number(id));
6061

62+
const { permissions } = usePermissions(
63+
'firewall',
64+
['create_firewall_device'],
65+
firewall?.id
66+
);
67+
6168
const { data: allLinodes } = useAllLinodesQuery({}, {});
6269

6370
const linodesUsingLinodeInterfaces =
@@ -371,7 +378,7 @@ export const AddLinodeDrawer = (props: Props) => {
371378
{isLinodeInterfacesEnabled &&
372379
selectedLinodesWithMultipleInterfaces.length > 0 && (
373380
<Typography marginTop={3}>
374-
{`${selectedLinodesWithMultipleInterfaces.length === 1 ? 'This Linode has' : 'The following Linodes have'}
381+
{`${selectedLinodesWithMultipleInterfaces.length === 1 ? 'This Linode has' : 'The following Linodes have'}
375382
multiple interfaces that a firewall can be applied to. Select which interface to apply the firewall to.`}
376383
</Typography>
377384
)}
@@ -409,7 +416,9 @@ export const AddLinodeDrawer = (props: Props) => {
409416
})}
410417
<ActionsPanel
411418
primaryButtonProps={{
412-
disabled: selectedLinodes.length === 0,
419+
disabled:
420+
selectedLinodes.length === 0 ||
421+
!permissions.create_firewall_device,
413422
label: 'Add',
414423
loading: addDeviceIsLoading,
415424
onClick: handleSubmit,

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

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ const queryMocks = vi.hoisted(() => ({
1818
}),
1919
useParams: vi.fn().mockReturnValue({}),
2020
useSearch: vi.fn().mockReturnValue({}),
21+
usePermissions: vi.fn(() => ({
22+
permissions: {
23+
create_firewall_device: false,
24+
},
25+
})),
2126
}));
2227

2328
vi.mock('@tanstack/react-router', async () => {
@@ -39,10 +44,13 @@ vi.mock('src/hooks/useOrderV2', async () => {
3944
};
4045
});
4146

47+
vi.mock('src/features/IAM/hooks/usePermissions', () => ({
48+
usePermissions: queryMocks.usePermissions,
49+
}));
50+
4251
const baseProps = (
4352
type: FirewallDeviceEntityType
4453
): FirewallDeviceLandingProps => ({
45-
disabled: false,
4654
firewallId: 1,
4755
firewallLabel: 'test',
4856
type,
@@ -86,8 +94,13 @@ services.forEach((service: FirewallDeviceEntityType) => {
8694
expect(table).toBeInTheDocument();
8795
});
8896

89-
if (prop.disabled) {
97+
if (serviceName !== 'Linode') {
9098
it(`should contain a disabled Add ${serviceName} button`, () => {
99+
queryMocks.usePermissions.mockReturnValue({
100+
permissions: {
101+
create_firewall_device: false,
102+
},
103+
});
91104
const { getByTestId } = renderWithTheme(
92105
<FirewallDeviceLanding {...prop} />
93106
);
@@ -105,8 +118,13 @@ services.forEach((service: FirewallDeviceEntityType) => {
105118
});
106119
}
107120

108-
if (!prop.disabled) {
121+
if (serviceName !== 'Linode') {
109122
it(`should contain an enabled Add ${serviceName} button`, () => {
123+
queryMocks.usePermissions.mockReturnValue({
124+
permissions: {
125+
create_firewall_device: true,
126+
},
127+
});
110128
const { getByTestId } = renderWithTheme(
111129
<FirewallDeviceLanding {...prop} />
112130
);
@@ -116,6 +134,11 @@ services.forEach((service: FirewallDeviceEntityType) => {
116134
});
117135

118136
it(`should navigate to Add ${serviceName} To Firewall drawer when enabled`, async () => {
137+
queryMocks.usePermissions.mockReturnValue({
138+
permissions: {
139+
create_firewall_device: true,
140+
},
141+
});
119142
const mockNavigate = vi.fn();
120143
queryMocks.useNavigate.mockReturnValue(mockNavigate);
121144

@@ -136,6 +159,43 @@ services.forEach((service: FirewallDeviceEntityType) => {
136159
});
137160
});
138161
}
162+
163+
if (serviceName === 'Linode') {
164+
it('should disable "Add Linodes to Firewall" button if the user does not have create_firewall_device permission', async () => {
165+
queryMocks.usePermissions.mockReturnValue({
166+
permissions: {
167+
create_firewall_device: false,
168+
},
169+
});
170+
171+
const { getByTestId } = await renderWithTheme(
172+
<FirewallDeviceLanding {...prop} />,
173+
{
174+
initialRoute: `/firewalls/1/linodes`,
175+
}
176+
);
177+
const addButton = getByTestId('add-device-button');
178+
expect(addButton).toBeInTheDocument();
179+
expect(addButton).toBeDisabled();
180+
});
181+
it('should enable "Add Linodes to Firewall" button if the user has create_firewall_device permission', async () => {
182+
queryMocks.usePermissions.mockReturnValue({
183+
permissions: {
184+
create_firewall_device: true,
185+
},
186+
});
187+
188+
const { getByTestId } = await renderWithTheme(
189+
<FirewallDeviceLanding {...prop} />,
190+
{
191+
initialRoute: `/firewalls/1/linodes`,
192+
}
193+
);
194+
const addButton = getByTestId('add-device-button');
195+
expect(addButton).toBeInTheDocument();
196+
expect(addButton).toBeEnabled();
197+
});
198+
}
139199
});
140200
});
141201
});

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { useLocation, useNavigate } from '@tanstack/react-router';
55
import * as React from 'react';
66

77
import { DebouncedSearchTextField } from 'src/components/DebouncedSearchTextField';
8+
import { usePermissions } from 'src/features/IAM/hooks/usePermissions';
89

910
import { AddLinodeDrawer } from './AddLinodeDrawer';
1011
import { AddNodebalancerDrawer } from './AddNodebalancerDrawer';
@@ -15,18 +16,22 @@ import { RemoveDeviceDialog } from './RemoveDeviceDialog';
1516
import type { FirewallDevice, FirewallDeviceEntityType } from '@linode/api-v4';
1617

1718
export interface FirewallDeviceLandingProps {
18-
disabled: boolean;
1919
firewallId: number;
2020
firewallLabel: string;
2121
type: FirewallDeviceEntityType;
2222
}
2323

2424
export const FirewallDeviceLanding = React.memo(
2525
(props: FirewallDeviceLandingProps) => {
26-
const { disabled, firewallId, firewallLabel, type } = props;
26+
const { firewallId, firewallLabel, type } = props;
2727
const theme = useTheme();
2828
const navigate = useNavigate();
2929
const location = useLocation();
30+
const { permissions } = usePermissions(
31+
'firewall',
32+
['create_firewall_device', 'delete_firewall_device'],
33+
firewallId
34+
);
3035
const helperText =
3136
'Assign one or more services to this firewall. You can add services later if you want to customize your rules first.';
3237

@@ -60,6 +65,8 @@ export const FirewallDeviceLanding = React.memo(
6065
);
6166

6267
const formattedType = formattedTypes[type];
68+
const isCreateDeviceDisabled = !permissions.create_firewall_device;
69+
const isRemoveDeviceDisabled = !permissions.delete_firewall_device;
6370

6471
// If the user initiates a history -/+ to a /remove route and the device is not found,
6572
// push navigation to the appropriate /linodes or /nodebalancers route.
@@ -76,8 +83,9 @@ export const FirewallDeviceLanding = React.memo(
7683
}, [device, location.pathname, firewallId, type, navigate]);
7784

7885
return (
86+
// TODO: Matching old behavior. Do we want separate messages for when the user can't create or remove devices?
7987
<>
80-
{disabled ? (
88+
{permissions && isCreateDeviceDisabled && isRemoveDeviceDisabled ? (
8189
<Notice
8290
text={
8391
"You don't have permissions to modify this Firewall. Please contact an account administrator for details."
@@ -119,7 +127,7 @@ export const FirewallDeviceLanding = React.memo(
119127
<Button
120128
buttonType="primary"
121129
data-testid="add-device-button"
122-
disabled={disabled}
130+
disabled={isCreateDeviceDisabled}
123131
onClick={handleOpen}
124132
>
125133
Add {formattedType}s to Firewall
@@ -129,7 +137,7 @@ export const FirewallDeviceLanding = React.memo(
129137
</Grid>
130138
<FirewallDeviceTable
131139
deviceType={type}
132-
disabled={disabled}
140+
disabled={isRemoveDeviceDisabled}
133141
firewallId={firewallId}
134142
handleRemoveDevice={(device) => {
135143
setDevice(device);

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useSnackbar } from 'notistack';
99
import * as React from 'react';
1010

1111
import { ConfirmationDialog } from 'src/components/ConfirmationDialog/ConfirmationDialog';
12+
import { usePermissions } from 'src/features/IAM/hooks/usePermissions';
1213

1314
import { formattedTypes } from './constants';
1415

@@ -51,6 +52,22 @@ export const RemoveDeviceDialog = React.memo((props: Props) => {
5152

5253
const deviceDialog = formattedTypes[deviceType ?? 'linode'];
5354

55+
const { permissions: firewallPermissions } = usePermissions(
56+
'firewall',
57+
['delete_firewall_device'],
58+
firewallId
59+
);
60+
61+
const { permissions: linodePermissions } = usePermissions(
62+
'linode',
63+
['update_linode'],
64+
device?.entity.id
65+
);
66+
67+
const deleteDisabled =
68+
!firewallPermissions.delete_firewall_device ||
69+
!linodePermissions.update_linode;
70+
5471
const onDelete = async () => {
5572
if (!device) {
5673
return;
@@ -120,6 +137,7 @@ export const RemoveDeviceDialog = React.memo((props: Props) => {
120137
label: primaryButtonText,
121138
loading: isPending,
122139
onClick: onDelete,
140+
disabled: deleteDisabled,
123141
}}
124142
secondaryButtonProps={{
125143
label: 'Cancel',

0 commit comments

Comments
 (0)