From 27cac71cb17c9c1f9937e8d4dd6b669c55bd0d0f Mon Sep 17 00:00:00 2001 From: Jean-Baptiste WATENBERG Date: Mon, 23 Sep 2024 16:37:59 +0200 Subject: [PATCH 1/3] Enhance IAM Role Selector with ability to select only interresting roles and review design --- src/js/IAMClient.ts | 8 + src/react/DataServiceRoleProvider.tsx | 4 +- .../ui-elements/SelectAccountIAMRole.tsx | 186 +++++++++++++----- src/react/utils/hooks.ts | 2 + 4 files changed, 147 insertions(+), 53 deletions(-) diff --git a/src/js/IAMClient.ts b/src/js/IAMClient.ts index 5037c08b7..6f7c213bd 100644 --- a/src/js/IAMClient.ts +++ b/src/js/IAMClient.ts @@ -189,6 +189,14 @@ export default class IAMClient implements IAMClientInterface { .promise(); } + getRole(roleName: string) { + return notFalsyTypeGuard(this.client) + .getRole({ + RoleName: roleName, + }) + .promise(); + } + listOwnAccessKeys() { return notFalsyTypeGuard(this.client).listAccessKeys().promise(); } diff --git a/src/react/DataServiceRoleProvider.tsx b/src/react/DataServiceRoleProvider.tsx index b31ec9256..143d3742c 100644 --- a/src/react/DataServiceRoleProvider.tsx +++ b/src/react/DataServiceRoleProvider.tsx @@ -97,6 +97,7 @@ export const useCurrentAccount = () => { const DataServiceRoleProvider = ({ children, + inlineLoader = false, /** * DoNotChangePropsWithRedux is a static props. * When set, it must not be changed, otherwise it will break the hook rules. @@ -105,6 +106,7 @@ const DataServiceRoleProvider = ({ DoNotChangePropsWithRedux = true, }: { children: JSX.Element; + inlineLoader?: boolean; DoNotChangePropsWithRedux?: boolean; }) => { const [role, setRoleState] = useState<{ roleArn: string }>({ @@ -181,7 +183,7 @@ const DataServiceRoleProvider = ({ if (role.roleArn && !assumedRole) { //@ts-expect-error fix this when you are working on it - return Loading...; + return inlineLoader ?
loading...
: Loading...; } if (DoNotChangePropsWithRedux) { diff --git a/src/react/ui-elements/SelectAccountIAMRole.tsx b/src/react/ui-elements/SelectAccountIAMRole.tsx index d813b750c..e81d5d840 100644 --- a/src/react/ui-elements/SelectAccountIAMRole.tsx +++ b/src/react/ui-elements/SelectAccountIAMRole.tsx @@ -1,9 +1,9 @@ -import { Stack } from '@scality/core-ui'; +import { Form, FormGroup, FormSection, Stack } from '@scality/core-ui'; import { Select } from '@scality/core-ui/dist/next'; import { IAM } from 'aws-sdk'; import { Bucket } from 'aws-sdk/clients/s3'; import { PropsWithChildren, useState } from 'react'; -import { useQuery, useQueryClient } from 'react-query'; +import { useMutation, useQuery, useQueryClient } from 'react-query'; import { MemoryRouter, Route, useHistory, useParams } from 'react-router-dom'; import DataServiceRoleProvider, { useAssumedRole, @@ -20,7 +20,7 @@ import { } from '../next-architecture/ui/AccessibleAccountsAdapterProvider'; import { AccountsLocationsEndpointsAdapterProvider } from '../next-architecture/ui/AccountsLocationsEndpointsAdapterProvider'; import { getListRolesQuery } from '../queries'; -import { regexArn } from '../utils/hooks'; +import { SCALITY_INTERNAL_ROLES, regexArn } from '../utils/hooks'; class NoOpMetricsAdapter implements IMetricsAdapter { async listBucketsLatestUsedCapacity( @@ -128,7 +128,7 @@ const InternalProvider = ({ ]} > - + void; + onChange: ( + account: Account, + role: IAM.Role, + keycloakRoleName: string, + ) => void; defaultValue?: { accountName: string; roleName: string }; hideAccountRoles?: { accountName: string; roleName: string }[]; + menuPosition?: 'absolute' | 'fixed'; + identityProviderUrl?: string; + filterOutInternalRoles?: boolean; }; type SelectAccountIAMRoleWithAccountProps = SelectAccountIAMRoleProps & { @@ -169,6 +176,12 @@ const SelectAccountIAMRoleWithAccount = ( const [role, setRole] = useState(null); const assumedRole = useAssumedRole(); + const getIAMRoleMutation = useMutation({ + mutationFn: (roleName: string) => { + return IAMClient.getRole(roleName); + }, + }); + const accountName = account ? account.name : ''; const rolesQuery = getListRolesQuery(accountName, IAMClient); const queryClient = useQueryClient(); @@ -197,66 +210,132 @@ const SelectAccountIAMRoleWithAccount = ( }; const roleQueryData = useQuery(listRolesQuery); - const roles = filterRoles( + const allRolesExceptHiddenOnes = filterRoles( accountName, roleQueryData?.data?.Roles ?? [], hideAccountRoles, ); + const roles = props.filterOutInternalRoles + ? allRolesExceptHiddenOnes.filter((role) => { + return ( + SCALITY_INTERNAL_ROLES.includes(role.RoleName) || + !role.Arn.includes('role/scality-internal') + ); + }) + : allRolesExceptHiddenOnes; const isDefaultAccountSelected = account?.name === defaultValue?.accountName; const defaultRole = isDefaultAccountSelected ? defaultValue?.roleName : null; return ( - - + setAccount(selectedAccount); + setRole(null); + queryClient.invalidateQueries(rolesQuery.queryKey); + }} + menuPosition={props.menuPosition} + placeholder="Select Account" + > + {accounts.map((account) => ( + + {account.name} + + ))} + + } + /> - {roles.length > 0 ? ( - - ) : null} - + content={ + roles.length > 0 ? ( + + ) : ( + + ) + } + /> + + ); }; @@ -282,6 +361,9 @@ export const _SelectAccountIAMRole = (props: SelectAccountIAMRoleProps) => { defaultValue={defaultValue} hideAccountRoles={hideAccountRoles} onChange={onChange} + menuPosition={props.menuPosition} + filterOutInternalRoles={props.filterOutInternalRoles} + identityProviderUrl={props.identityProviderUrl} /> ); } else { diff --git a/src/react/utils/hooks.ts b/src/react/utils/hooks.ts index 4aa24f0b5..588caed7a 100644 --- a/src/react/utils/hooks.ts +++ b/src/react/utils/hooks.ts @@ -154,10 +154,12 @@ export const regexArn = export const STORAGE_MANAGER_ROLE = 'storage-manager-role'; export const STORAGE_ACCOUNT_OWNER_ROLE = 'storage-account-owner-role'; const DATA_CONSUMER_ROLE = 'data-consumer-role'; +const DATA_ACCESSOR_ROLE = 'data-accessor-role'; export const SCALITY_INTERNAL_ROLES = [ STORAGE_MANAGER_ROLE, STORAGE_ACCOUNT_OWNER_ROLE, DATA_CONSUMER_ROLE, + DATA_ACCESSOR_ROLE, ]; const reduxBasedEventDispatcher = () => { From c15c2868e3077e5b6ac45e4d805461b7f71adf90 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste WATENBERG Date: Tue, 24 Sep 2024 14:45:09 +0200 Subject: [PATCH 2/3] Disable eslint rule on empty onChange handler --- src/react/ui-elements/SelectAccountIAMRole.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/react/ui-elements/SelectAccountIAMRole.tsx b/src/react/ui-elements/SelectAccountIAMRole.tsx index e81d5d840..7d1ac58f1 100644 --- a/src/react/ui-elements/SelectAccountIAMRole.tsx +++ b/src/react/ui-elements/SelectAccountIAMRole.tsx @@ -323,6 +323,7 @@ const SelectAccountIAMRoleWithAccount = ( id="select-account-role" value={'Please select an account'} disabled + // eslint-disable-next-line @typescript-eslint/no-empty-function onChange={() => {}} menuPosition={props.menuPosition} placeholder="Select Role" From 09c61354177844f43da91744129139f1681248c7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste WATENBERG Date: Tue, 24 Sep 2024 15:09:03 +0200 Subject: [PATCH 3/3] Update tests --- .../__tests__/SelectAccountIAMRole.test.tsx | 56 ++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/src/react/ui-elements/__tests__/SelectAccountIAMRole.test.tsx b/src/react/ui-elements/__tests__/SelectAccountIAMRole.test.tsx index cb11ae4b9..5873f1b6e 100644 --- a/src/react/ui-elements/__tests__/SelectAccountIAMRole.test.tsx +++ b/src/react/ui-elements/__tests__/SelectAccountIAMRole.test.tsx @@ -209,6 +209,28 @@ const genFn = (getPayloadFn: jest.Mock) => { }), ); } + + if (params.get('Action') === 'GetRole') { + return res( + ctx.xml(` + + + + %7B%22Statement%22%3A%5B%7B%22Action%22%3A%22sts%3AAssumeRoleWithWebIdentity%22%2C%22Condition%22%3A%7B%22StringEquals%22%3A%7B%22keycloak%3Aroles%22%3A%2211112%3A%3ADataConsumer%22%7D%7D%2C%22Effect%22%3A%22Allow%22%2C%22Principal%22%3A%7B%22Federated%22%3A%22https%3A%2F%2Fui.pod-choco.local%2Fauth%2Frealms%2Fartesca%22%7D%7D%2C%7B%22Action%22%3A%22sts%3AAssumeRoleWithWebIdentity%22%2C%22Condition%22%3A%7B%22StringEquals%22%3A%7B%22keycloak%3Aroles%22%3A%2211112%3A%3ADataConsumer%22%7D%7D%2C%22Effect%22%3A%22Allow%22%2C%22Principal%22%3A%7B%22Federated%22%3A%22https%3A%2F%2F13.48.197.10%3A8443%2Fauth%2Frealms%2Fartesca%22%7D%7D%5D%2C%22Version%22%3A%222012-10-17%22%7D + Has S3 read and write accesses to 11112 S3 Buckets. Cannot create or delete S3 Buckets. + /scality-internal/ + data-consumer-role + ZX41BHX0Z4JYLB711HLUKWL2BBVM5DFZ + arn:aws:iam::087998292579:role/scality-internal/data-consumer-role + 2024-05-06T14:09:38Z + + + + 6273e485a54abdb41527 + + `), + ); + } }); }; @@ -307,7 +329,7 @@ describe('SelectAccountIAMRole', () => { RoleName: 'backbeat-gc-1', Tags: [], }; - expect(onChange).toHaveBeenCalledWith(account, role); + expect(onChange).toHaveBeenCalledWith(account, role, '11112::DataConsumer'); }); it('test the change of account and role', async () => { @@ -370,6 +392,7 @@ describe('SelectAccountIAMRole', () => { RoleName: 'backbeat-gc-1', Tags: [], }, + '11112::DataConsumer', ); await userEvent.click(seletors.accountSelect()); @@ -533,8 +556,8 @@ describe('SelectAccountIAMRole', () => { RoleName: 'yanjin-custom-role', Tags: [], }, + '11112::DataConsumer', ); - debug(); }); it('renders with default value', async () => { @@ -615,4 +638,33 @@ describe('SelectAccountIAMRole', () => { expect(screen.getByText(/no options/i)).toBeInTheDocument(); }); + + it('renders with hidden internal roles', async () => { + const getPayloadFn = jest.fn(); + server.use(genFn(getPayloadFn)); + const onChange = jest.fn(); + render( + + + , + ); + + await waitFor(() => { + expect(seletors.accountSelect()).toBeInTheDocument(); + }); + + await userEvent.click(seletors.accountSelect()); + + await userEvent.click(seletors.selectOption(/no-bucket/i)); + + await waitFor(() => { + expect(seletors.roleSelect()).toBeInTheDocument(); + }); + + await userEvent.click(seletors.roleSelect()); + + expect( + screen.queryAllByRole('option', { name: /backbeat-gc-1/i }), + ).toHaveLength(0); + }); });