diff --git a/packages/manager/.changeset/pr-13050-changed-1762189590928.md b/packages/manager/.changeset/pr-13050-changed-1762189590928.md new file mode 100644 index 00000000000..32de1a27641 --- /dev/null +++ b/packages/manager/.changeset/pr-13050-changed-1762189590928.md @@ -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)) diff --git a/packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts b/packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts index 6a5615b8c00..8454fa75054 100644 --- a/packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts +++ b/packages/manager/cypress/e2e/core/vpc/vpc-linodes-update.spec.ts @@ -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); @@ -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') @@ -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'); @@ -415,6 +417,8 @@ describe('VPC assign/unassign flows', () => { .should('be.visible') .click(); + cy.wait(['@getLinodes']); + ui.drawer .findByTitle( `Unassign Linodes from subnet: ${mockSubnet.label} (0.0.0.0/0)` diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.test.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.test.tsx index 06bf9522042..3a30c590f83 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.test.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.test.tsx @@ -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, }, @@ -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(); - 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(); - 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, }, @@ -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, }, diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.tsx index 878e317956d..1fa8a73baa8 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetActionMenu.tsx @@ -40,6 +40,7 @@ export const SubnetActionMenu = (props: Props) => { ['update_vpc', 'delete_vpc'], vpcId ); + const canUpdateVPC = permissions?.update_vpc; const canDeleteVPC = permissions?.delete_vpc; @@ -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: () => { diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx index c8ff57284f2..4b8df610e09 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetAssignLinodesDrawer.tsx @@ -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, @@ -164,23 +161,15 @@ 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( - query, - 'linode', - ['update_linode', 'delete_linode'], - open - ); + useQueryWithPermissions(query, 'linode', ['update_linode'], open); + + 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) => { @@ -188,19 +177,6 @@ export const SubnetAssignLinodesDrawer = ( }); }, [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(() => { - if (filteredLinodes) { - setLinodeOptionsToAssign(findUnassignedLinodes() ?? []); - } - }, [filteredLinodes, setLinodeOptionsToAssign, findUnassignedLinodes]); - // Determine the configId based on the number of configurations function getConfigId(inputs: { isLinodeInterface: boolean; @@ -594,12 +570,6 @@ export const SubnetAssignLinodesDrawer = ( open={open} title={`Assign Linodes to subnet: ${subnet?.label ?? 'Unknown'}`} > - {!userCanAssignLinodes && ( - - )} {assignLinodesErrors.none && ( )} diff --git a/packages/manager/src/features/VPCs/VPCDetail/SubnetUnassignLinodesDrawer.tsx b/packages/manager/src/features/VPCs/VPCDetail/SubnetUnassignLinodesDrawer.tsx index 7a39a12c019..82fad4c4878 100644 --- a/packages/manager/src/features/VPCs/VPCDetail/SubnetUnassignLinodesDrawer.tsx +++ b/packages/manager/src/features/VPCs/VPCDetail/SubnetUnassignLinodesDrawer.tsx @@ -15,10 +15,7 @@ import * as React from 'react'; 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'; @@ -97,8 +94,6 @@ export const SubnetUnassignLinodesDrawer = React.memo( const hasError = React.useRef(false); // This flag is used to prevent the drawer from closing if an error occurs. - const [linodeOptionsToUnassign, setLinodeOptionsToUnassign] = - React.useState([]); const [interfacesToDelete, setInterfacesToDelete] = React.useState< DeleteInterfaceIds[] >([]); @@ -106,38 +101,30 @@ export const SubnetUnassignLinodesDrawer = React.memo( const { linodes: subnetLinodeIds } = subnet || {}; // 1. We need to get all the linodes. + // TODO: change to 'delete_linode_config_profile_interface' once it's available const { - data: linodes, + data: filteredLinodes, error: linodesError, + isLoading: isLoadingFilteredLinodes, refetch: getCSVData, - } = useAllLinodesQuery(); + } = useQueryWithPermissions( + 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 } = - useQueryWithPermissions( - 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( @@ -340,12 +327,6 @@ export const SubnetUnassignLinodesDrawer = React.memo( subnet?.ipv4 ?? subnet?.ipv6 ?? 'Unknown' })`} > - {!userCanUnassignLinodes && linodeOptionsToUnassign.length > 0 && ( - - )} {unassignLinodesErrors.length > 0 && ( )}