Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-13050-changed-1762189590928.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Changed
---

IAM: fix permissiom's check for vpc for assigning/unassigning linodes ([#13050](https://github.com/linode/manager/pull/13050))
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ describe('VPC assign/unassign flows', () => {
.click();
});

cy.wait(['@createSubnet', '@getVPC', '@getSubnets', '@getLinodes']);
cy.wait(['@createSubnet', '@getVPC', '@getSubnets']);

mockGetSubnet(mockVPC.id, mockSubnet.id, mockSubnet);

Expand All @@ -139,6 +139,8 @@ describe('VPC assign/unassign flows', () => {
.should('be.visible')
.click();

cy.wait(['@getLinodes']);

ui.drawer
.findByTitle(`Assign Linodes to subnet: ${mockSubnet.label}`)
.should('be.visible')
Expand Down Expand Up @@ -395,7 +397,7 @@ describe('VPC assign/unassign flows', () => {
mockGetLinodes([mockLinode, mockSecondLinode]).as('getLinodes');

cy.visitWithLogin(`/vpcs/${mockVPC.id}`);
cy.wait(['@getVPC', '@getSubnets', '@getLinodes', '@getFeatureFlags']);
cy.wait(['@getVPC', '@getSubnets', '@getFeatureFlags']);

// confirm that subnet should get displayed on VPC's detail page
cy.findByText(mockVPC.label).should('be.visible');
Expand All @@ -415,6 +417,8 @@ describe('VPC assign/unassign flows', () => {
.should('be.visible')
.click();

cy.wait(['@getLinodes']);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aaleksee-akamai because of deferring the getLinodes call to when the drawer is open we have to adjust the test accordingly. All good here πŸ‘


ui.drawer
.findByTitle(
`Unassign Linodes from subnet: ${mockSubnet.label} (0.0.0.0/0)`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { SubnetActionMenu } from './SubnetActionMenu';
const queryMocks = vi.hoisted(() => ({
userPermissions: vi.fn(() => ({
data: {
update_linode: true,
delete_linode: true,
update_vpc: true,
delete_vpc: true,
},
Expand Down Expand Up @@ -128,45 +126,9 @@ describe('SubnetActionMenu', () => {
expect(props.handleAssignLinodes).toHaveBeenCalled();
});

it('should disable the Assign Linodes button if user does not have update_linode permission', async () => {
queryMocks.userPermissions.mockReturnValue({
data: {
update_linode: false,
delete_linode: false,
update_vpc: false,
delete_vpc: false,
},
});
const view = renderWithTheme(<SubnetActionMenu {...props} />);
const actionMenu = view.getByLabelText(`Action menu for Subnet subnet-1`);
await userEvent.click(actionMenu);

const assignButton = view.getByRole('menuitem', { name: 'Assign Linodes' });
expect(assignButton).toHaveAttribute('aria-disabled', 'true');
});

it('should enable the Assign Linodes button if user has update_linode and update_vpc permissions', async () => {
queryMocks.userPermissions.mockReturnValue({
data: {
update_linode: true,
delete_linode: false,
update_vpc: true,
delete_vpc: false,
},
});
const view = renderWithTheme(<SubnetActionMenu {...props} />);
const actionMenu = view.getByLabelText(`Action menu for Subnet subnet-1`);
await userEvent.click(actionMenu);

const assignButton = view.getByRole('menuitem', { name: 'Assign Linodes' });
expect(assignButton).not.toHaveAttribute('aria-disabled', 'true');
});

it('should disable the Edit button if user does not have update_vpc permission', async () => {
queryMocks.userPermissions.mockReturnValue({
data: {
update_linode: false,
delete_linode: false,
update_vpc: false,
delete_vpc: false,
},
Expand All @@ -182,8 +144,6 @@ describe('SubnetActionMenu', () => {
it('should enable the Edit button if user has update_vpc permission', async () => {
queryMocks.userPermissions.mockReturnValue({
data: {
update_linode: false,
delete_linode: false,
update_vpc: true,
delete_vpc: false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const SubnetActionMenu = (props: Props) => {
['update_vpc', 'delete_vpc'],
vpcId
);

const canUpdateVPC = permissions?.update_vpc;
const canDeleteVPC = permissions?.delete_vpc;

Expand All @@ -49,20 +50,12 @@ export const SubnetActionMenu = (props: Props) => {
handleAssignLinodes(subnet);
},
title: 'Assign Linodes',
disabled: !canUpdateVPC,
tooltip: !canUpdateVPC
? 'You do not have permission to assign Linode to this subnet.'
: undefined,
},
{
onClick: () => {
handleUnassignLinodes(subnet);
},
title: 'Unassign Linodes',
disabled: !canUpdateVPC,
tooltip: !canUpdateVPC
? 'You do not have permission to unassign Linode from this subnet.'
: undefined,
},
{
onClick: () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ import { DownloadCSV } from 'src/components/DownloadCSV/DownloadCSV';
import { Link } from 'src/components/Link';
import { RemovableSelectionsListTable } from 'src/components/RemovableSelectionsList/RemovableSelectionsListTable';
import { FirewallSelect } from 'src/features/Firewalls/components/FirewallSelect';
import {
usePermissions,
useQueryWithPermissions,
} from 'src/features/IAM/hooks/usePermissions';
import { useQueryWithPermissions } from 'src/features/IAM/hooks/usePermissions';
import { getDefaultFirewallForInterfacePurpose } from 'src/features/Linodes/LinodeCreate/Networking/utilities';
import {
REMOVABLE_SELECTIONS_LINODES_TABLE_HEADERS,
Expand Down Expand Up @@ -164,43 +161,22 @@ export const SubnetAssignLinodesDrawer = (
csvRef.current.link.click();
};

const { data: permissions } = usePermissions('vpc', ['update_vpc'], vpcId);
// TODO: change update_linode to create_linode_config_profile_interface once it's available
// TODO: change delete_linode to delete_linode_config_profile_interface once it's available
// TODO: refactor useQueryWithPermissions once API filter is available
const { data: filteredLinodes, isLoading: isLoadingFilteredLinodes } =
useQueryWithPermissions<Linode>(
query,
'linode',
['update_linode', 'delete_linode'],
open
);
useQueryWithPermissions<Linode>(query, 'linode', ['update_linode'], open);
Comment thread
mpolotsk-akamai marked this conversation as resolved.

const userCanAssignLinodes = filteredLinodes?.length > 0;

const userCanAssignLinodes =
permissions?.update_vpc && filteredLinodes?.length > 0;
// We need to filter to the linodes from this region that are not already
// assigned to this subnet
const findUnassignedLinodes = React.useCallback(() => {
const linodeOptionsToAssign = React.useMemo(() => {
// We need to filter to the linodes from this region that are not already
// assigned to this subnet
if (!filteredLinodes) return [];

return filteredLinodes?.filter((linode) => {
return !subnet?.linodes.some((linodeInfo) => linodeInfo.id === linode.id);
});
}, [subnet, filteredLinodes]);

const [linodeOptionsToAssign, setLinodeOptionsToAssign] = React.useState<
Linode[]
>([]);

// Moved the list of linodes that are currently assignable to a subnet into a state variable (linodeOptionsToAssign)
// and update that list whenever this subnet or the list of all linodes in this subnet's region changes. This takes
// care of the MUI invalid value warning that was occurring before in the Linodes autocomplete [M3-6752]
React.useEffect(() => {
Comment thread
abailly-akamai marked this conversation as resolved.
if (filteredLinodes) {
setLinodeOptionsToAssign(findUnassignedLinodes() ?? []);
}
}, [filteredLinodes, setLinodeOptionsToAssign, findUnassignedLinodes]);

// Determine the configId based on the number of configurations
function getConfigId(inputs: {
isLinodeInterface: boolean;
Expand Down Expand Up @@ -594,12 +570,6 @@ export const SubnetAssignLinodesDrawer = (
open={open}
title={`Assign Linodes to subnet: ${subnet?.label ?? 'Unknown'}`}
>
{!userCanAssignLinodes && (
<Notice
text={`You don't have permissions to assign Linodes to ${subnet?.label}. Please contact an account administrator for details.`}
variant="error"
/>
)}
{assignLinodesErrors.none && (
<Notice text={assignLinodesErrors.none} variant="error" />
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@

import { DownloadCSV } from 'src/components/DownloadCSV/DownloadCSV';
import { RemovableSelectionsListTable } from 'src/components/RemovableSelectionsList/RemovableSelectionsListTable';
import {
usePermissions,
useQueryWithPermissions,
} from 'src/features/IAM/hooks/usePermissions';
import { useQueryWithPermissions } from 'src/features/IAM/hooks/usePermissions';
import { REMOVABLE_SELECTIONS_LINODES_TABLE_HEADERS } from 'src/features/VPCs/constants';
import { useUnassignLinode } from 'src/hooks/useUnassignLinode';
import { useVPCDualStack } from 'src/hooks/useVPCDualStack';
Expand Down Expand Up @@ -97,47 +94,37 @@

const hasError = React.useRef(false); // This flag is used to prevent the drawer from closing if an error occurs.

const [linodeOptionsToUnassign, setLinodeOptionsToUnassign] =
React.useState<Linode[]>([]);
const [interfacesToDelete, setInterfacesToDelete] = React.useState<
DeleteInterfaceIds[]
>([]);

const { linodes: subnetLinodeIds } = subnet || {};

// 1. We need to get all the linodes.
// TODO: change to 'delete_linode_config_profile_interface' once it's available

Check warning on line 104 in packages/manager/src/features/VPCs/VPCDetail/SubnetUnassignLinodesDrawer.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Complete the task associated to this "TODO" comment. Raw Output: {"ruleId":"sonarjs/todo-tag","severity":1,"message":"Complete the task associated to this \"TODO\" comment.","line":104,"column":8,"nodeType":null,"messageId":"completeTODO","endLine":104,"endColumn":12}
const {
data: linodes,
data: filteredLinodes,
error: linodesError,
isLoading: isLoadingFilteredLinodes,
refetch: getCSVData,
} = useAllLinodesQuery();
} = useQueryWithPermissions<Linode>(
useAllLinodesQuery({}, {}, open),
'linode',
['delete_linode'],
open
);
const userCanUnassignLinodes = filteredLinodes?.length > 0;

// 2. We need to filter only the linodes that are assigned to the subnet.
const findAssignedLinodes = React.useCallback(() => {
return linodes?.filter((linode) => {
const linodeOptionsToUnassign = React.useMemo(() => {
// 2. We need to filter only the linodes that are assigned to the subnet.
if (!filteredLinodes) return [];

return filteredLinodes?.filter((linode) => {
return subnetLinodeIds?.some(
(linodeInfo) => linodeInfo.id === linode.id
);
});
}, [linodes, subnetLinodeIds]);

const { data: permissions } = usePermissions('vpc', ['update_vpc'], vpcId);
// TODO: change to 'delete_linode_config_profile_interface' once it's available
const { data: filteredLinodes, isLoading: isLoadingFilteredLinodes } =

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue that we didn't see filtered linodes by permission on the autocomplete because we didn't use filteredLinodes instead it was findAssignedLinodes

useQueryWithPermissions<Linode>(
useAllLinodesQuery({}, {}, open),
'linode',
['delete_linode'],
open
);
const userCanUnassignLinodes =
permissions.update_vpc && filteredLinodes?.length > 0;

React.useEffect(() => {
if (linodes) {
setLinodeOptionsToUnassign(findAssignedLinodes() ?? []);
}
}, [linodes, setLinodeOptionsToUnassign, findAssignedLinodes]);
}, [subnetLinodeIds, filteredLinodes]);

// 3. When a linode is selected, we need to get the VPC interface to unassign.
const getVPCInterface = React.useCallback(
Expand Down Expand Up @@ -340,12 +327,6 @@
subnet?.ipv4 ?? subnet?.ipv6 ?? 'Unknown'
})`}
>
{!userCanUnassignLinodes && linodeOptionsToUnassign.length > 0 && (
<Notice
text={`You don't have permissions to unassign Linodes from ${subnet?.label}. Please contact an account administrator for details.`}
variant="error"
/>
)}
{unassignLinodesErrors.length > 0 && (
<Notice text={unassignLinodesErrors[0].reason} variant="error" />
)}
Expand Down