diff --git a/web/packages/design/src/Alert/Alert.jsx b/web/packages/design/src/Alert/Alert.jsx index 09682267e8f87..7e792ff987810 100644 --- a/web/packages/design/src/Alert/Alert.jsx +++ b/web/packages/design/src/Alert/Alert.jsx @@ -49,15 +49,23 @@ const kind = props => { case 'outline-danger': return { background: fade(theme.colors.error.main, 0.1), - border: `${theme.radii[1]}px solid ${theme.colors.error.main}`, + border: `${theme.borders[2]} ${theme.colors.error.main}`, borderRadius: `${theme.radii[3]}px`, boxShadow: 'none', justifyContent: 'normal', }; case 'outline-info': return { - background: fade(theme.colors.link, 0.1), - border: `${theme.radii[1]}px solid ${theme.colors.link}`, + background: fade(theme.colors.accent.main, 0.1), + border: `${theme.borders[2]} ${theme.colors.accent.main}`, + borderRadius: `${theme.radii[3]}px`, + boxShadow: 'none', + justifyContent: 'normal', + }; + case 'outline-warn': + return { + background: fade(theme.colors.warning.main, 0.1), + border: `${theme.borders[2]} ${theme.colors.warning.main}`, borderRadius: `${theme.radii[3]}px`, boxShadow: 'none', justifyContent: 'normal', @@ -100,6 +108,7 @@ Alert.propTypes = { 'success', 'outline-danger', 'outline-info', + 'outline-warn', ]), ...color.propTypes, ...space.propTypes, @@ -121,3 +130,4 @@ export const OutlineDanger = props => ( ); export const OutlineInfo = props => ; +export const OutlineWarn = props => ; diff --git a/web/packages/design/src/Alert/Alert.story.js b/web/packages/design/src/Alert/Alert.story.js index a62d8ff16e9bc..6818f0c0bfc9f 100644 --- a/web/packages/design/src/Alert/Alert.story.js +++ b/web/packages/design/src/Alert/Alert.story.js @@ -33,5 +33,7 @@ export const Alerts = () => ( Some informational message This is success Text align it yourself + Text align it yourself + Text align it yourself ); diff --git a/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx b/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx index 94e5010b2a695..69ee1c3647860 100644 --- a/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx +++ b/web/packages/teleport/src/Discover/Database/EnrollRdsDatabase/EnrollRdsDatabase.tsx @@ -44,6 +44,7 @@ import { createDiscoveryConfig, } from 'teleport/services/discovery'; import useTeleport from 'teleport/useTeleport'; +import { splitAwsIamArn } from 'teleport/services/integrations/aws'; import { AutoEnrollDialog, @@ -221,13 +222,11 @@ export function EnrollRdsDatabase() { if (!requiredVpcsAndSubnets) { try { const { spec, name: integrationName } = agentMeta.awsIntegration; - const accountId = spec.roleArn - .split('arn:aws:iam::')[1] - .substring(0, 12); + const { awsAccountId } = splitAwsIamArn(spec.roleArn); requiredVpcsAndSubnets = await integrationService.fetchAwsRdsRequiredVpcs(integrationName, { region: tableData.currRegion, - accountId, + accountId: awsAccountId, }); setRequiredVpcs(requiredVpcsAndSubnets); diff --git a/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx b/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx index 42e376d9ed322..7339646b588af 100644 --- a/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx +++ b/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx @@ -15,7 +15,8 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see . */ -import { render, screen, fireEvent } from 'design/utils/testing'; +import { render, screen, fireEvent, waitFor } from 'design/utils/testing'; +import userEvent from '@testing-library/user-event'; import { Integration, @@ -25,7 +26,7 @@ import { import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog'; -test('edit without s3 fields', async () => { +test('user acknowledging script was ran when s3 bucket fields are edited', async () => { render( null} @@ -36,7 +37,7 @@ test('edit without s3 fields', async () => { name: 'some-integration-name', spec: { roleArn: 'arn:aws:iam::123456789012:role/johndoe', - issuerS3Bucket: '', + issuerS3Bucket: 'test-value', issuerS3Prefix: '', }, statusCode: IntegrationStatusCode.Running, @@ -45,18 +46,29 @@ test('edit without s3 fields', async () => { ); // Initial state. - expect(screen.getByText(/required/i)).toBeInTheDocument(); expect(screen.queryByTestId('scriptbox')).not.toBeInTheDocument(); expect(screen.queryByTestId('checkbox')).not.toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /generate command/i }) + ).not.toBeInTheDocument(); expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); - // Click on generate command: + // Fill in the s3 prefix field. + fireEvent.change(screen.getByPlaceholderText(/prefix/i), { + target: { value: 'test-value' }, + }); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); + // When clicking on generate command: // - script rendered // - checkbox to confirm user has ran command // - edit button replaces generate command button // - save button still disabled - fireEvent.click(screen.getByRole('button', { name: /generate command/i })); - screen.getByRole('button', { name: /edit/i }); + userEvent.click(screen.getByRole('button', { name: /generate command/i })); + await screen.findByRole('button', { name: /edit/i }); expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); expect( screen.queryByRole('button', { name: /generate command/i }) @@ -65,32 +77,48 @@ test('edit without s3 fields', async () => { expect(screen.getByTestId('scriptbox')).toBeInTheDocument(); // Click on checkbox should enable save button and disable edit button. - fireEvent.click(screen.getByRole('checkbox')); - expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(); + userEvent.click(screen.getByRole('checkbox')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled() + ); expect(screen.getByRole('button', { name: /edit/i })).toBeDisabled(); // Unchecking the checkbox should disable save button. - fireEvent.click(screen.getByRole('checkbox')); - expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + userEvent.click(screen.getByRole('checkbox')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled() + ); // Click on edit, should replace it with generate command - fireEvent.click(screen.getByRole('button', { name: /edit/i })); - expect( - screen.getByRole('button', { name: /generate command/i }) - ).toBeEnabled(); + userEvent.click(screen.getByRole('button', { name: /edit/i })); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); }); -test('edit with s3 fields', async () => { +test('render warning on save when leaving s3 fields empty', async () => { + const edit = jest.fn(() => Promise.resolve()); render( null} - edit={() => null} - integration={integration} + edit={edit} + integration={{ + resourceType: 'integration', + kind: IntegrationKind.AwsOidc, + name: 'some-integration-name', + spec: { + roleArn: 'arn:aws:iam::123456789012:role/johndoe', + issuerS3Bucket: '', + issuerS3Prefix: '', + }, + statusCode: IntegrationStatusCode.Running, + }} /> ); // Initial state. - expect(screen.queryByText(/required/i)).not.toBeInTheDocument(); expect(screen.queryByTestId('scriptbox')).not.toBeInTheDocument(); expect(screen.queryByTestId('checkbox')).not.toBeInTheDocument(); expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); @@ -98,21 +126,104 @@ test('edit with s3 fields', async () => { screen.queryByRole('button', { name: /generate command/i }) ).not.toBeInTheDocument(); - // Changing role arn should not render generate command. + // Enable the generate command button by changing a field. fireEvent.change(screen.getByPlaceholderText(/arn:aws:iam:/i), { - target: { value: 'something else' }, + target: { value: 'arn:aws:iam::123456789012:role/someonelse' }, }); - expect(screen.getByRole('button', { name: /save/i })).toBeEnabled(); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); + + expect(screen.queryByTestId('checkbox')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + + userEvent.click(screen.getByRole('button', { name: /generate command/i })); + await screen.findByRole('button', { name: /edit/i }); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + + userEvent.click(screen.getByTestId('checkbox')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled() + ); + + // Clicking on save without defining s3 fields, should render + // a warning. + userEvent.click(screen.getByRole('button', { name: /save/i })); + await screen.findByText(/recommended to use an S3 bucket/i); + expect(edit).not.toHaveBeenCalled(); + + // Canceling and saving should re-render the warning. + userEvent.click(screen.getByRole('button', { name: /cancel/i })); + await screen.findByRole('button', { name: /save/i }); + + userEvent.click(screen.getByRole('button', { name: /save/i })); + await screen.findByText(/recommended to use an S3 bucket/i); + + userEvent.click(screen.getByRole('button', { name: /continue/i })); + await waitFor(() => expect(edit).toHaveBeenCalledTimes(1)); +}); + +test('render warning on save when deleting existing s3 fields', async () => { + const edit = jest.fn(() => Promise.resolve()); + render( + null} + edit={edit} + integration={{ + resourceType: 'integration', + kind: IntegrationKind.AwsOidc, + name: 'some-integration-name', + spec: { + roleArn: 'arn:aws:iam::123456789012:role/johndoe', + issuerS3Bucket: 'delete-me', + issuerS3Prefix: 'delete-me', + }, + statusCode: IntegrationStatusCode.Running, + }} + /> + ); + expect( screen.queryByRole('button', { name: /generate command/i }) ).not.toBeInTheDocument(); - // Changing the s3 fields should render generate command. + // Delete the s3 fields. fireEvent.change(screen.getByPlaceholderText(/bucket/i), { - target: { value: 's3-bucket-something' }, + target: { value: '' }, }); - fireEvent.click(screen.getByRole('button', { name: /generate command/i })); + fireEvent.change(screen.getByPlaceholderText(/prefix/i), { + target: { value: '' }, + }); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); + + expect(screen.queryByTestId('checkbox')).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + + userEvent.click(screen.getByRole('button', { name: /generate command/i })); + await screen.findByRole('button', { name: /edit/i }); expect(screen.getByRole('button', { name: /save/i })).toBeDisabled(); + + userEvent.click(screen.getByTestId('checkbox')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled() + ); + + // Test for warning render. + userEvent.click(screen.getByRole('button', { name: /save/i })); + await screen.findByText(/recommended to use an S3 bucket/i); + expect(edit).not.toHaveBeenCalled(); + expect( + screen.getByText(/recommended to use an S3 bucket/i) + ).toBeInTheDocument(); + + userEvent.click(screen.getByRole('button', { name: /continue/i })); + await waitFor(() => expect(edit).toHaveBeenCalledTimes(1)); }); test('edit invalid fields', async () => { @@ -131,22 +242,18 @@ test('edit invalid fields', async () => { target: { value: 'role something else' }, }); - fireEvent.click(screen.getByRole('button', { name: /save/i })); - expect(screen.getByText(/invalid role ARN format/i)).toBeInTheDocument(); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); - // invalid s3 fields - fireEvent.change(screen.getByPlaceholderText(/bucket/i), { - target: { value: '' }, - }); - fireEvent.change(screen.getByPlaceholderText(/prefix/i), { - target: { value: '' }, - }); - fireEvent.click(screen.getByRole('button', { name: /generate command/i })); - expect(screen.queryAllByText(/required/i)).toHaveLength(2); + userEvent.click(screen.getByRole('button', { name: /generate command/i })); + await screen.findByText(/invalid role ARN format/i); }); -test('edit submit', async () => { - const mockEditFn = jest.fn(); +test('edit submit called with proper fields', async () => { + const mockEditFn = jest.fn(() => Promise.resolve()); render( null} @@ -170,9 +277,21 @@ test('edit submit', async () => { target: { value: 'other-prefix' }, }); - fireEvent.click(screen.getByRole('button', { name: /generate command/i })); - fireEvent.click(screen.getByRole('checkbox')); - fireEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => + expect( + screen.getByRole('button', { name: /generate command/i }) + ).toBeEnabled() + ); + + userEvent.click(screen.getByRole('button', { name: /generate command/i })); + await screen.findByRole('button', { name: /edit/i }); + + userEvent.click(screen.getByTestId('checkbox')); + await waitFor(() => + expect(screen.getByRole('button', { name: /save/i })).toBeEnabled() + ); + userEvent.click(screen.getByRole('button', { name: /save/i })); + await waitFor(() => expect(mockEditFn).toHaveBeenCalledTimes(1)); expect(mockEditFn).toHaveBeenCalledWith({ roleArn: 'arn:aws:iam::123456789011:role/other', diff --git a/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.tsx b/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.tsx index 515c3a7cf982f..d1ced2fea7b6a 100644 --- a/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.tsx +++ b/web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.tsx @@ -25,7 +25,6 @@ import { Alert, Text, Box, - Flex, Link, } from 'design'; import Dialog, { @@ -38,19 +37,16 @@ import useAttempt from 'shared/hooks/useAttemptNext'; import FieldInput from 'shared/components/FieldInput'; import Validation, { Validator } from 'shared/components/Validation'; import { requiredRoleArn } from 'shared/components/Validation/rules'; -import { ToolTipInfo } from 'shared/components/ToolTip'; import { CheckboxInput } from 'design/Checkbox'; import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy'; import { Integration } from 'teleport/services/integrations'; import cfg from 'teleport/config'; +import { splitAwsIamArn } from 'teleport/services/integrations/aws'; import { EditableIntegrationFields } from './Operations/useIntegrationOperation'; import { S3BucketConfiguration } from './Enroll/AwsOidc/S3BucketConfiguration'; -import { - getDefaultS3BucketName, - getDefaultS3PrefixName, -} from './Enroll/AwsOidc/Shared/utils'; +import { S3BucketWarningBanner } from './Enroll/AwsOidc/S3BucketWarningBanner'; type Props = { close(): void; @@ -62,14 +58,13 @@ export function EditAwsOidcIntegrationDialog(props: Props) { const { close, edit, integration } = props; const { attempt, run } = useAttempt(); + const [showS3BucketWarning, setShowS3BucketWarning] = useState(false); const [roleArn, setRoleArn] = useState(integration.spec.roleArn); const [s3Bucket, setS3Bucket] = useState( - () => integration.spec.issuerS3Bucket || getDefaultS3BucketName() + () => integration.spec.issuerS3Bucket ); const [s3Prefix, setS3Prefix] = useState( - () => - integration.spec.issuerS3Prefix || - getDefaultS3PrefixName(integration.spec.roleArn.split(':role/')[1]) + () => integration.spec.issuerS3Prefix ); const [scriptUrl, setScriptUrl] = useState(''); @@ -90,10 +85,12 @@ export function EditAwsOidcIntegrationDialog(props: Props) { validator.reset(); - const roleName = roleArn.split(':role/')[1]; + const { arnResourceName } = splitAwsIamArn( + roleArn || props.integration.spec.roleArn + ); const newScriptUrl = cfg.getAwsOidcConfigureIdpScriptUrl({ integrationName: integration.name, - roleName, + roleName: arnResourceName, s3Bucket: s3Bucket, s3Prefix: s3Prefix, }); @@ -102,17 +99,29 @@ export function EditAwsOidcIntegrationDialog(props: Props) { } const isProcessing = attempt.status === 'processing'; - const requiresS3 = - !integration.spec.issuerS3Bucket || !integration.spec.issuerS3Prefix; + const requiresS3BucketWarning = !s3Bucket && !s3Prefix; const showGenerateCommand = - requiresS3 || integration.spec.issuerS3Bucket !== s3Bucket || - integration.spec.issuerS3Prefix !== s3Prefix; + integration.spec.issuerS3Prefix !== s3Prefix || + integration.spec.roleArn !== roleArn; + + const changeDetected = + integration.spec.issuerS3Bucket !== s3Bucket || + integration.spec.issuerS3Prefix !== s3Prefix || + integration.spec.roleArn !== roleArn; return ( {({ validator }) => ( - + ({ + maxWidth: '650px', + width: '100%', + })} + > Edit Integration @@ -125,30 +134,22 @@ export function EditAwsOidcIntegrationDialog(props: Props) { value={integration.name} readonly={true} /> - setRoleArn(e.target.value)} - placeholder="arn:aws:iam:::role/" - toolTipContent={ - - Role ARN can be found in the format:
- {`arn:aws:iam:::role/`} - - } - disabled={scriptUrl} - /> - - {requiresS3 && ( - - Required - - This integration does not have Amazon S3 configured - - - )} + + setRoleArn(e.target.value)} + placeholder="arn:aws:iam:::role/" + toolTipContent={ + + Role ARN can be found in the format:
+ {`arn:aws:iam:::role/`} + + } + disabled={scriptUrl} + /> generateAwsOidcConfigIdpScript(validator)} - disabled={!s3Bucket || !s3Prefix || !roleArn} + disabled={ + (!requiresS3BucketWarning && (!s3Bucket || !s3Prefix)) || + !roleArn + } > Generate Command )} - + {showGenerateCommand && scriptUrl && ( @@ -217,22 +221,40 @@ export function EditAwsOidcIntegrationDialog(props: Props) { I have ran the command )} - handleEdit(validator)} - > - Save - - - Cancel - + + {requiresS3BucketWarning && showS3BucketWarning ? ( + setShowS3BucketWarning(false)} + onContinue={() => { + setShowS3BucketWarning(false); + handleEdit(validator); + }} + btnFlexWrap={true} + /> + ) : ( + <> + { + if (requiresS3BucketWarning) { + setShowS3BucketWarning(true); + } else { + handleEdit(validator); + } + }} + > + Save + + + Cancel + + + )} )} @@ -240,19 +262,8 @@ export function EditAwsOidcIntegrationDialog(props: Props) { ); } -const S3BucketBox = styled(Box)` +const EditableBox = styled(Box)` border-radius: ${p => p.theme.space[1]}px; - border: 2px solid - ${p => { - if (p.requiresS3) { - return p.theme.colors.warning.main; - } - return p.theme.colors.spotBackground[1]; - }}; - background-color: ${p => { - if (p.requiresS3) { - return p.theme.colors.interactive.tonal.alert[0]; - } - return p.theme.colors.spotBackground[0]; - }}; + border: 2px solid ${p => p.theme.colors.spotBackground[1]}; + background-color: ${p => p.theme.colors.spotBackground[0]}; `; diff --git a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.story.tsx b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.story.tsx index 37019315276b0..c7bf533f841b5 100644 --- a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.story.tsx +++ b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.story.tsx @@ -20,6 +20,7 @@ import React from 'react'; import { MemoryRouter } from 'react-router'; import { AwsOidc } from './AwsOidc'; +import { S3BucketWarningBanner } from './S3BucketWarningBanner'; export default { title: 'Teleport/Integrations/Enroll/AwsOidc', @@ -30,3 +31,15 @@ export const Flow = () => ( ); + +export const SBucketWarning = () => ( + null} onContinue={() => null} /> +); + +export const SBucketWarningWithReview = () => ( + null} + onContinue={() => null} + reviewing={true} + /> +); diff --git a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.tsx b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.tsx index dcb7f4a8e3698..79d7dac8e0b45 100644 --- a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.tsx +++ b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.tsx @@ -20,7 +20,15 @@ import React, { useEffect, useState } from 'react'; import { Link as InternalRouteLink } from 'react-router-dom'; import { useLocation } from 'react-router'; import styled from 'styled-components'; -import { Box, ButtonSecondary, Text, Link, Flex, ButtonPrimary } from 'design'; +import { + Box, + ButtonSecondary, + Text, + Link, + Flex, + ButtonPrimary, + ButtonText, +} from 'design'; import * as Icons from 'design/Icon'; import FieldInput from 'shared/components/FieldInput'; import { requiredIamRoleName } from 'shared/components/Validation/rules'; @@ -51,6 +59,7 @@ import { requiredPrefixName, validPrefixNameToolTipContent, } from './Shared/utils'; +import { S3BucketWarningBanner } from './S3BucketWarningBanner'; export function AwsOidc() { const [integrationName, setIntegrationName] = useState(''); @@ -59,6 +68,9 @@ export function AwsOidc() { const [scriptUrl, setScriptUrl] = useState(''); const [s3Bucket, setS3Bucket] = useState(() => getDefaultS3BucketName()); const [s3Prefix, setS3Prefix] = useState(''); + const [showS3BucketWarning, setShowS3BucketWarning] = useState(false); + const [confirmedS3BucketWarning, setConfirmedS3BucketWarning] = + useState(false); const [createdIntegration, setCreatedIntegration] = useState(); const { attempt, run } = useAttempt(''); @@ -69,6 +81,8 @@ export function AwsOidc() { kind: IntegrationEnrollKind.AwsOidc, }); + const requiresS3BucketWarning = !s3Bucket && !s3Prefix; + useEffect(() => { // If a user came from the discover wizard, // discover wizard will send of appropriate events. @@ -166,10 +180,9 @@ export function AwsOidc() { <> Step 1 - setIntegrationName(e.target.value)} disabled={!!scriptUrl} onBlur={() => { + // s3Bucket by default is defined. + // If empty user intentionally cleared it. + if (!integrationName || (!s3Bucket && !s3Prefix)) return; // Help come up with a default prefix name for user. if (!s3Prefix) { setS3Prefix(`${integrationName}-oidc-idp`); @@ -200,14 +216,49 @@ export function AwsOidc() { disabled={!!scriptUrl} /> - {scriptUrl ? ( - setScriptUrl('')}> + {confirmedS3BucketWarning && ( + + setShowS3BucketWarning(true)} + alignItems="center" + > + + Click to view S3 Bucket Warning + + + )} + {showS3BucketWarning ? ( + setShowS3BucketWarning(false)} + onContinue={() => { + setShowS3BucketWarning(false); + setConfirmedS3BucketWarning(true); + generateAwsOidcConfigIdpScript(validator); + }} + reviewing={confirmedS3BucketWarning} + /> + ) : scriptUrl ? ( + { + setScriptUrl(''); + setConfirmedS3BucketWarning(false); + }} + > Edit ) : ( generateAwsOidcConfigIdpScript(validator)} + onClick={() => { + if (requiresS3BucketWarning) { + setShowS3BucketWarning(true); + } else { + generateAwsOidcConfigIdpScript(validator); + } + }} > Generate Command @@ -217,7 +268,6 @@ export function AwsOidc() { <> Step 2 - Configure the required permission in your AWS account. Open{' '} { + return ( + + + + + + + + It is recommended to use an S3 bucket to host the public keys. + + + Without an S3 bucket, you will be required to append the new + certificate's thumbprint in the AWS IAM/Identity Provider section + after you have renewed and started using the new certificate. + + + + {reviewing ? ( + + Ok + + ) : ( + <> + + Continue + + + Cancel + + + )} + + + + ); +}; + +const BellIcon = styled(Notification)` + background-color: ${p => p.theme.colors.warning.hover}; + border-radius: 100px; + height: 32px; + width: 32px; + color: ${p => p.theme.colors.text.primaryInverse}; + margin-right: ${p => p.theme.space[3]}px; +`; diff --git a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.test.ts b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.test.ts index 8dc382b0b0e60..f65e9294cc6bc 100644 --- a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.test.ts +++ b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.test.ts @@ -48,6 +48,7 @@ test('getDefaultS3PrefixName', () => { }); describe('requiredPrefixName', () => { + const requiredField = true; test.each` input | valid ${''} | ${false} @@ -61,9 +62,14 @@ describe('requiredPrefixName', () => { ${'sdlfkjs/dfsd'} | ${false} ${'Asd09f-_.sdfDFs1'} | ${true} `('validity of input($input) should be ($valid)', ({ input, valid }) => { - const result = requiredPrefixName(input)(); + const result = requiredPrefixName(requiredField)(input)(); expect(result.valid).toEqual(valid); }); + + test('empty prefix name is valid if not a required field', () => { + const requiredField = false; + expect(requiredPrefixName(requiredField)('')().valid).toBeTruthy(); + }); }); describe('requiredBucketName', () => { @@ -83,7 +89,13 @@ describe('requiredBucketName', () => { ${'Asd09f-sdfDFs1'} | ${false} ${'sdf0-dfs0'} | ${true} `('validity of input($input) should be ($valid)', ({ input, valid }) => { - const result = requiredBucketName(input)(); + const requiredField = true; + const result = requiredBucketName(requiredField)(input)(); expect(result.valid).toEqual(valid); }); + + test('empty bucket name is valid if not a required field', () => { + const requiredField = false; + expect(requiredBucketName(requiredField)('')().valid).toBeTruthy(); + }); }); diff --git a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.ts b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.ts index 915a32b6b7a96..28e8e20a0fac1 100644 --- a/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.ts +++ b/web/packages/teleport/src/Integrations/Enroll/AwsOidc/Shared/utils.ts @@ -23,98 +23,104 @@ import cfg from 'teleport/config'; // Must start and end with lowercase letters or numbers. // Can have hyphens in between start and end. const bucketNameRegex = new RegExp(/^[a-z0-9][a-z0-9-]*[a-z0-9]$/); -export const requiredBucketName: Rule = inputVal => () => { - if (!inputVal) { - return { - valid: false, - message: 'required', - }; - } - - if (inputVal.length < 3 || inputVal.length > 63) { - return { - valid: false, - message: 'name should be 3-63 characters', - }; - } - - if (!bucketNameRegex.test(inputVal)) { - return { - valid: false, - message: 'name is in a invalid format', - }; - } - - if (inputVal.startsWith('xn--')) { - return { - valid: false, - message: 'cannot start with "xn--"', - }; - } - - if (inputVal.startsWith('sthree-')) { - return { - valid: false, - message: 'cannot start with "sthree-"', - }; - } +export const requiredBucketName = + (required): Rule => + inputVal => + () => { + if (!inputVal) { + return { + valid: !required, + message: required ? 'required' : '', + }; + } + + if (inputVal.length < 3 || inputVal.length > 63) { + return { + valid: false, + message: 'name should be 3-63 characters', + }; + } + + if (!bucketNameRegex.test(inputVal)) { + return { + valid: false, + message: 'name is in a invalid format', + }; + } + + if (inputVal.startsWith('xn--')) { + return { + valid: false, + message: 'cannot start with "xn--"', + }; + } + + if (inputVal.startsWith('sthree-')) { + return { + valid: false, + message: 'cannot start with "sthree-"', + }; + } + + if (inputVal.startsWith('sthree-configurator')) { + return { + valid: false, + message: 'cannot start with "sthree-configurator"', + }; + } + + if (inputVal.endsWith('-s3alias')) { + return { + valid: false, + message: 'cannot end with "-s3alias"', + }; + } + + if (inputVal.endsWith('--ol-s3')) { + return { + valid: false, + message: 'cannot end with "--ol-s3"', + }; + } - if (inputVal.startsWith('sthree-configurator')) { return { - valid: false, - message: 'cannot start with "sthree-configurator"', + valid: true, }; - } - - if (inputVal.endsWith('-s3alias')) { - return { - valid: false, - message: 'cannot end with "-s3alias"', - }; - } - - if (inputVal.endsWith('--ol-s3')) { - return { - valid: false, - message: 'cannot end with "--ol-s3"', - }; - } - - return { - valid: true, }; -}; // Must start and end with letters or numbers. // Can have hyphens, underscores, and periods in between start and end. const prefixNameRegex = new RegExp(/^[a-zA-Z0-9][a-zA-Z0-9-_.]*[a-zA-Z0-9]$/); -export const requiredPrefixName: Rule = inputVal => () => { - if (!inputVal) { - return { - valid: false, - message: 'required', - }; - } +export const requiredPrefixName = + (required): Rule => + inputVal => + () => { + if (!inputVal) { + return { + valid: !required, + message: required ? 'required' : '', + }; + } + + // Just a random hard cap. + if (inputVal.length > 63) { + return { + valid: false, + message: 'name can be max 63 characters long', + }; + } + + if (!prefixNameRegex.test(inputVal)) { + return { + valid: false, + message: 'name is in a invalid format', + }; + } - // Just a random hard cap. - if (inputVal.length > 63) { return { - valid: false, - message: 'name can be max 63 characters long', + valid: true, }; - } - - if (!prefixNameRegex.test(inputVal)) { - return { - valid: false, - message: 'name is in a invalid format', - }; - } - - return { - valid: true, }; -}; export function getDefaultS3BucketName() { const modifiedClusterName = cfg.proxyCluster.replaceAll('.', '-'); diff --git a/web/packages/teleport/src/Integrations/IntegrationList.test.tsx b/web/packages/teleport/src/Integrations/IntegrationList.test.tsx new file mode 100644 index 0000000000000..5dce1329ae977 --- /dev/null +++ b/web/packages/teleport/src/Integrations/IntegrationList.test.tsx @@ -0,0 +1,83 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { render, screen, userEvent } from 'design/utils/testing'; + +import { + IntegrationKind, + integrationService, + IntegrationStatusCode, +} from 'teleport/services/integrations'; + +import { IntegrationList } from './IntegrationList'; + +test('aws oidc row without s3 fields should render tooltip', async () => { + jest + .spyOn(integrationService, 'fetchThumbprint') + .mockResolvedValue('some-thumbprint'); + + render( + + ); + + expect(screen.getByText('aws')).toBeInTheDocument(); + expect(screen.getByText(/running/i)).toBeInTheDocument(); + await userEvent.hover(screen.getByRole('icon')); + expect(screen.queryByTestId('btn-copy')).not.toBeInTheDocument(); + + await userEvent.click(screen.getByText(/generate a new thumbprint/i)); + expect(screen.getByText(/some-thumbprint/i)).toBeInTheDocument(); +}); + +test('aws oidc row with s3 fields should NOT render tooltip', async () => { + jest + .spyOn(integrationService, 'fetchThumbprint') + .mockResolvedValue('some-thumbprint'); + + render( + + ); + + expect(screen.getByText('aws')).toBeInTheDocument(); + expect(screen.getByText(/running/i)).toBeInTheDocument(); + expect(screen.queryByRole('icon')).not.toBeInTheDocument(); +}); diff --git a/web/packages/teleport/src/Integrations/IntegrationList.tsx b/web/packages/teleport/src/Integrations/IntegrationList.tsx index ad675897a3f7e..1c13dca5928db 100644 --- a/web/packages/teleport/src/Integrations/IntegrationList.tsx +++ b/web/packages/teleport/src/Integrations/IntegrationList.tsx @@ -18,7 +18,7 @@ import React from 'react'; import styled from 'styled-components'; -import { Link } from 'react-router-dom'; +import { Link as InternalRouteLink } from 'react-router-dom'; import { Box, Flex, Image } from 'design'; import { AWSIcon } from 'design/SVGIcon'; @@ -50,6 +50,7 @@ import { import cfg from 'teleport/config'; import { ExternalAuditStorageOpType } from './Operations/useIntegrationOperation'; +import { UpdateAwsOidcThumbprint } from './UpdateAwsOidcThumbprint'; type Props = { list: IntegrationLike[]; @@ -138,7 +139,7 @@ export function IntegrationList(props: Props) { ) { } const StatusCell = ({ item }: { item: IntegrationLike }) => { + const status = getStatus(item); + if ( item.resourceType === 'integration' && item.kind === IntegrationKind.AwsOidc && @@ -191,21 +194,16 @@ const StatusCell = ({ item }: { item: IntegrationLike }) => { return ( - - Integration needs updating + + {getStatusCodeTitle(item.statusCode)} - - Requires setting up a Amazon S3 Bucket. Click on 'OPTIONS' and - 'Edit...' and fill out the 'S3 Location' input fields. - + ); } - const status = getStatus(item); const statusDescription = getStatusCodeDescription(item.statusCode); - return ( diff --git a/web/packages/teleport/src/Integrations/Integrations.story.tsx b/web/packages/teleport/src/Integrations/Integrations.story.tsx index 739d6697e80ee..c47261b869aef 100644 --- a/web/packages/teleport/src/Integrations/Integrations.story.tsx +++ b/web/packages/teleport/src/Integrations/Integrations.story.tsx @@ -26,6 +26,7 @@ import { import { IntegrationList } from './IntegrationList'; import { DeleteIntegrationDialog } from './RemoveIntegrationDialog'; import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog'; +import { UpdateAwsOidcThumbprint } from './UpdateAwsOidcThumbprint'; import { plugins, integrations } from './fixtures'; export default { @@ -36,6 +37,20 @@ export function List() { return ; } +export function UpdateAwsOidcThumbprintHoverTooltip() { + return ( + + ); +} + export function DeleteDialog() { return ( . + */ + +import { useState } from 'react'; +import styled from 'styled-components'; +import { Text, Link as ExternalLink, Flex, Box, ButtonPrimary } from 'design'; +import { TextSelectCopyMulti } from 'shared/components/TextSelectCopy'; +import { ToolTipInfo } from 'shared/components/ToolTip'; +import useAttempt from 'shared/hooks/useAttemptNext'; +import * as Icons from 'design/Icon'; + +import { Mark } from 'teleport/Discover/Shared'; +import { + Integration, + integrationService, +} from 'teleport/services/integrations'; +import { splitAwsIamArn } from 'teleport/services/integrations/aws'; +import cfg from 'teleport/config'; + +export function UpdateAwsOidcThumbprint({ + integration, +}: { + integration: Integration; +}) { + const { attempt, run } = useAttempt(); + + const [thumbprint, setThumbprint] = useState(''); + + function getThumbprint() { + run(() => integrationService.fetchThumbprint().then(setThumbprint)); + } + + const { awsAccountId, arnStartingPart } = splitAwsIamArn( + integration.spec.roleArn + ); + + const encodedOidcProviderArn = encodeURIComponent( + `${arnStartingPart}${awsAccountId}:oidc-provider/${cfg.proxyCluster}` + ); + + return ( + + + + + This integration has no S3 bucket configured. When renewing your + HTTPS certificate, if it has a different CA, a manual update of this + integration's thumbprint is required. + + You may run into issues when the thumbprint is stale. + + + + Generate a New Thumbprint + + + {thumbprint && ( + + )} + {attempt.status === 'failed' && ( + + Error + fetching thumbprint: some kind of error + + )} + + + +
    + To update thumbprint: +
  • + - Go to your{' '} + + IAM Identity Provider + {' '} + dashboard +
    • +
  • + - On Thumbprints section click on Manage{' '} + then click on Add Thumbprint +
    • +
  • - Copy and paste the generated thumbprint
    • +
+ + + ); +} + +const Ul = styled.ul` + margin-left: 0; + padding-left: 0; + list-style: none; +`; diff --git a/web/packages/teleport/src/config.test.ts b/web/packages/teleport/src/config.test.ts index 0c64e04fc5644..062c9452601c8 100644 --- a/web/packages/teleport/src/config.test.ts +++ b/web/packages/teleport/src/config.test.ts @@ -36,7 +36,7 @@ test('getDeployServiceIamConfigureScriptPath formatting', async () => { ); }); -test('getAwsOidcConfigureIdpScriptUrl formatting', async () => { +test('getAwsOidcConfigureIdpScriptUrl formatting with s3 fields', async () => { const params: UrlAwsOidcConfigureIdp = { integrationName: 'int-name', roleName: 'role-arn', @@ -50,3 +50,16 @@ test('getAwsOidcConfigureIdpScriptUrl formatting', async () => { `${base}${expected}` ); }); + +test('getAwsOidcConfigureIdpScriptUrl formatting, without s3 fields', async () => { + const params: UrlAwsOidcConfigureIdp = { + integrationName: 'int-name', + roleName: 'role-arn', + }; + const base = + 'http://localhost/webapi/scripts/integrations/configure/awsoidc-idp.sh?'; + const expected = `integrationName=int-name&role=role-arn`; + expect(cfg.getAwsOidcConfigureIdpScriptUrl(params)).toBe( + `${base}${expected}` + ); +}); diff --git a/web/packages/teleport/src/config.ts b/web/packages/teleport/src/config.ts index ad3c65a6774c0..caf5671c74766 100644 --- a/web/packages/teleport/src/config.ts +++ b/web/packages/teleport/src/config.ts @@ -275,7 +275,7 @@ const cfg = { thumbprintPath: '/v1/webapi/thumbprint', awsConfigureIamScriptOidcIdpPath: - '/webapi/scripts/integrations/configure/awsoidc-idp.sh?integrationName=:integrationName&role=:roleName&s3Bucket=:s3Bucket&s3Prefix=:s3Prefix', + '/webapi/scripts/integrations/configure/awsoidc-idp.sh?integrationName=:integrationName&role=:roleName', awsConfigureIamScriptDeployServicePath: '/webapi/scripts/integrations/configure/deployservice-iam.sh?integrationName=:integrationName&awsRegion=:region&role=:awsOidcRoleArn&taskRole=:taskRoleArn', awsConfigureIamScriptListDatabasesPath: @@ -470,10 +470,11 @@ const cfg = { }, getAwsOidcConfigureIdpScriptUrl(p: UrlAwsOidcConfigureIdp) { - return ( - cfg.baseUrl + - generatePath(cfg.api.awsConfigureIamScriptOidcIdpPath, { ...p }) - ); + let path = cfg.api.awsConfigureIamScriptOidcIdpPath; + if (p.s3Bucket && p.s3Prefix) { + path += '&s3Bucket=:s3Bucket&s3Prefix=:s3Prefix'; + } + return cfg.baseUrl + generatePath(path, { ...p }); }, getDbScriptUrl(token: string) { @@ -1171,8 +1172,8 @@ export interface UrlDeployServiceIamConfigureScriptParams { export interface UrlAwsOidcConfigureIdp { integrationName: string; roleName: string; - s3Bucket: string; - s3Prefix: string; + s3Bucket?: string; + s3Prefix?: string; } export interface UrlAwsConfigureIamScriptParams { diff --git a/web/packages/teleport/src/services/integrations/aws.test.ts b/web/packages/teleport/src/services/integrations/aws.test.ts new file mode 100644 index 0000000000000..6c22eac9bab7a --- /dev/null +++ b/web/packages/teleport/src/services/integrations/aws.test.ts @@ -0,0 +1,55 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { splitAwsIamArn } from './aws'; + +describe('splitAwsIamArn', () => { + it.each([ + { + name: 'with default partition', + arn: 'arn:aws:iam::123456789012:role/johndoe', + expected: { + awsAccountId: '123456789012', + arnStartingPart: 'arn:aws:iam::', + arnResourceName: 'johndoe', + }, + }, + { + name: 'with china partition', + arn: 'arn:aws-cn:iam::123456789012:role/johndoe', + expected: { + awsAccountId: '123456789012', + arnStartingPart: 'arn:aws-cn:iam::', + arnResourceName: 'johndoe', + }, + }, + { + name: 'with us gov partition', + arn: 'arn:aws-us-gov:iam::123456789012:role/johndoe', + expected: { + awsAccountId: '123456789012', + arnStartingPart: 'arn:aws-us-gov:iam::', + arnResourceName: 'johndoe', + }, + }, + ])('$name', ({ arn, expected }) => { + const result = splitAwsIamArn(arn); + + expect(result).toStrictEqual(expected); + }); +}); diff --git a/web/packages/teleport/src/services/integrations/aws.ts b/web/packages/teleport/src/services/integrations/aws.ts new file mode 100644 index 0000000000000..7277530a0c2b1 --- /dev/null +++ b/web/packages/teleport/src/services/integrations/aws.ts @@ -0,0 +1,66 @@ +/** + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +export const AWS_IAM_ARN_DEFAULT_PARTITION = 'arn:aws:iam::'; +export const AWS_IAM_ARN_CHINA_PARTITION = 'arn:aws-cn:iam::'; +export const AWS_IAM_ARN_USGOV_PARTITION = 'arn:aws-us-gov:iam::'; + +/** + * @returns + * - awsAccountId: the 12 digit aws account Id + * - arnStartingPart: starting part is returned in the format + * "arn:\:iam::" + * - arnResourceName: is the resource name in the resource part + * of arn: / + */ +export function splitAwsIamArn(arn: string): { + awsAccountId: string; + arnStartingPart: string; + arnResourceName: string; +} { + if (!arn) { + return { + awsAccountId: '', + arnStartingPart: '', + arnResourceName: '', + }; + } + + let awsAccountId: string; + let arnStartingPart: string; + let splitted: string[] = []; + + if (arn.startsWith(AWS_IAM_ARN_DEFAULT_PARTITION)) { + arnStartingPart = AWS_IAM_ARN_DEFAULT_PARTITION; + splitted = arn.split(AWS_IAM_ARN_DEFAULT_PARTITION); + } else if (arn.startsWith(AWS_IAM_ARN_CHINA_PARTITION)) { + arnStartingPart = AWS_IAM_ARN_CHINA_PARTITION; + splitted = arn.split(AWS_IAM_ARN_CHINA_PARTITION); + } else if (arn.startsWith(AWS_IAM_ARN_USGOV_PARTITION)) { + arnStartingPart = AWS_IAM_ARN_USGOV_PARTITION; + splitted = arn.split(AWS_IAM_ARN_USGOV_PARTITION); + } + + awsAccountId = splitted[1]?.substring(0, 12) ?? ''; + + return { + awsAccountId, + arnStartingPart, + arnResourceName: splitted[1]?.substring(12).replace(/^(:role\/)/, ''), + }; +}