Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement/better iam selector #777

Merged
merged 3 commits into from
Sep 24, 2024
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
8 changes: 8 additions & 0 deletions src/js/IAMClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
4 changes: 3 additions & 1 deletion src/react/DataServiceRoleProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -105,6 +106,7 @@ const DataServiceRoleProvider = ({
DoNotChangePropsWithRedux = true,
}: {
children: JSX.Element;
inlineLoader?: boolean;
DoNotChangePropsWithRedux?: boolean;
}) => {
const [role, setRoleState] = useState<{ roleArn: string }>({
Expand Down Expand Up @@ -181,7 +183,7 @@ const DataServiceRoleProvider = ({

if (role.roleArn && !assumedRole) {
//@ts-expect-error fix this when you are working on it
return <Loader>Loading...</Loader>;
return inlineLoader ? <div>loading...</div> : <Loader>Loading...</Loader>;
}

if (DoNotChangePropsWithRedux) {
Expand Down
187 changes: 135 additions & 52 deletions src/react/ui-elements/SelectAccountIAMRole.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -128,7 +128,7 @@ const InternalProvider = ({
]}
>
<Route path="/accounts/:accountName">
<DataServiceRoleProvider DoNotChangePropsWithRedux={false}>
<DataServiceRoleProvider DoNotChangePropsWithRedux={false} inlineLoader>
<AccountsLocationsEndpointsAdapterProvider>
<AccessibleAccountsAdapterProvider
DoNotChangePropsWithEventDispatcher={false}
Expand All @@ -146,9 +146,16 @@ const InternalProvider = ({
};

type SelectAccountIAMRoleProps = {
onChange: (account: Account, role: IAM.Role) => 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 & {
Expand All @@ -169,6 +176,12 @@ const SelectAccountIAMRoleWithAccount = (
const [role, setRole] = useState<IAM.Role | null>(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();
Expand Down Expand Up @@ -197,66 +210,133 @@ 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 (
<Stack>
<Select
id="select-account"
value={account?.name ?? defaultValue?.accountName}
onChange={(accountName) => {
const selectedAccount = accounts.find(
(account) => account.name === accountName,
);
<Form layout={{ kind: 'tab' }}>
<FormSection>
<FormGroup
label="Account"
id="select-account"
content={
<Select
id="select-account"
value={account?.name ?? defaultValue?.accountName}
onChange={(accountName) => {
const selectedAccount = accounts.find(
(account) => account.name === accountName,
);

setAssumedRole({
roleArn: selectedAccount.preferredAssumableRoleArn,
});
history.push(`/accounts/${accountName}`);
setAssumedRole({
roleArn: selectedAccount.preferredAssumableRoleArn,
});
history.push(`/accounts/${accountName}`);

setAccount(selectedAccount);
setRole(null);
queryClient.invalidateQueries(rolesQuery.queryKey);
}}
size="1/2"
placeholder="Select Account"
>
{accounts.map((account) => (
<Select.Option key={`${account.name}`} value={account.name}>
{account.name}
</Select.Option>
))}
</Select>
setAccount(selectedAccount);
setRole(null);
queryClient.invalidateQueries(rolesQuery.queryKey);
}}
menuPosition={props.menuPosition}
placeholder="Select Account"
>
{accounts.map((account) => (
<Select.Option key={`${account.name}`} value={account.name}>
{account.name}
</Select.Option>
))}
</Select>
}
/>

{roles.length > 0 ? (
<Select
<FormGroup
label="Role"
id="select-account-role"
value={role?.RoleName ?? defaultRole}
onChange={(roleName) => {
const selectedRole = roles.find(
(role) => role.RoleName === roleName,
);
onChange(account, selectedRole);
setRole(selectedRole);
}}
size="2/3"
placeholder="Select Role"
>
{roles.map((role) => (
<Select.Option key={`${role.RoleName}`} value={role.RoleName}>
{role.RoleName}
</Select.Option>
))}
</Select>
) : null}
</Stack>
content={
roles.length > 0 ? (
<Select
id="select-account-role"
value={role?.RoleName ?? defaultRole}
onChange={(roleName) => {
const selectedRole = roles.find(
(role) => role.RoleName === roleName,
);
getIAMRoleMutation.mutate(roleName, {
onSuccess: (data) => {
const assumeRolePolicyDocument: {
Statement: {
Effect: 'Allow' | 'Deny';
Principal: { Federated?: string };
Action: 'sts:AssumeRoleWithWebIdentity';
Condition: {
StringEquals: { ['keycloak:roles']: string };
};
}[];
} = JSON.parse(
decodeURIComponent(data.Role.AssumeRolePolicyDocument),
);
const keycloakRoleName =
assumeRolePolicyDocument.Statement.find(
(statement) =>
(props.identityProviderUrl
? statement.Principal?.Federated?.startsWith(
props.identityProviderUrl,
)
: true) &&
statement.Condition.StringEquals[
'keycloak:roles'
] &&
statement.Effect === 'Allow' &&
statement.Action ===
'sts:AssumeRoleWithWebIdentity',
).Condition.StringEquals['keycloak:roles'];
onChange(account, selectedRole, keycloakRoleName);
},
});
setRole(selectedRole);
}}
menuPosition={props.menuPosition}
placeholder="Select Role"
>
{roles.map((role) => (
<Select.Option key={`${role.RoleName}`} value={role.RoleName}>
{role.RoleName}
</Select.Option>
))}
</Select>
) : (
<Select
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"
>
<Select.Option value={'Please select an account'}>
Please select an account
</Select.Option>
</Select>
)
}
/>
</FormSection>
</Form>
);
};

Expand All @@ -282,6 +362,9 @@ export const _SelectAccountIAMRole = (props: SelectAccountIAMRoleProps) => {
defaultValue={defaultValue}
hideAccountRoles={hideAccountRoles}
onChange={onChange}
menuPosition={props.menuPosition}
filterOutInternalRoles={props.filterOutInternalRoles}
identityProviderUrl={props.identityProviderUrl}
/>
);
} else {
Expand Down
56 changes: 54 additions & 2 deletions src/react/ui-elements/__tests__/SelectAccountIAMRole.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,28 @@ const genFn = (getPayloadFn: jest.Mock) => {
}),
);
}

if (params.get('Action') === 'GetRole') {
return res(
ctx.xml(`
<GetRoleResponse xmlns="https://iam.amazonaws.com/doc/2010-05-08/">
<GetRoleResult>
<Role>
<AssumeRolePolicyDocument>%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</AssumeRolePolicyDocument>
<Description>Has S3 read and write accesses to 11112 S3 Buckets. Cannot create or delete S3 Buckets.</Description>
<Path>/scality-internal/</Path>
<RoleName>data-consumer-role</RoleName>
<RoleId>ZX41BHX0Z4JYLB711HLUKWL2BBVM5DFZ</RoleId>
<Arn>arn:aws:iam::087998292579:role/scality-internal/data-consumer-role</Arn>
<CreateDate>2024-05-06T14:09:38Z</CreateDate>
</Role>
</GetRoleResult>
<ResponseMetadata>
<RequestId>6273e485a54abdb41527</RequestId>
</ResponseMetadata>
</GetRoleResponse>`),
);
}
});
};

Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -370,6 +392,7 @@ describe('SelectAccountIAMRole', () => {
RoleName: 'backbeat-gc-1',
Tags: [],
},
'11112::DataConsumer',
);

await userEvent.click(seletors.accountSelect());
Expand Down Expand Up @@ -533,8 +556,8 @@ describe('SelectAccountIAMRole', () => {
RoleName: 'yanjin-custom-role',
Tags: [],
},
'11112::DataConsumer',
);
debug();
});

it('renders with default value', async () => {
Expand Down Expand Up @@ -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(
<LocalWrapper>
<SelectAccountIAMRole onChange={onChange} filterOutInternalRoles />
</LocalWrapper>,
);

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);
});
});
2 changes: 2 additions & 0 deletions src/react/utils/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down
Loading