Web: add aws oidc integration health check before editing and when selecting in discover (#48953)

* Add ping aws integration req/resp boilerplate

* Add aws health check when selecting existing aws integration for discover flow

* Add health check when editing aws oidc integration

* Fix lint

* Add health check when creating integration

lib/web/integrations_awsoidc.go

@@ -1454,6 +1454,10 @@ func (h *Handler) awsOIDCPing(w http.ResponseWriter, r *http.Request, p httprout
 		return nil, trace.Wrap(err)
 	}
 
+	if req.RoleARN != "" {
+		integrationName = ""
+	}
+
 	pingResp, err := clt.IntegrationAWSOIDCClient().Ping(ctx, &integrationv1.PingRequest{
 		Integration: integrationName,
 		RoleArn:     req.RoleARN,

web/packages/teleport/src/Discover/Shared/AwsAccount/AwsAccount.test.tsx

@@ -18,7 +18,7 @@
 
 import React from 'react';
 import { MemoryRouter } from 'react-router';
-import { render, screen } from 'design/utils/testing';
+import { render, screen, fireEvent } from 'design/utils/testing';
 
 import { ContextProvider } from 'teleport';
 import {
@@ -144,6 +144,27 @@ test('missing permissions for integrations', async () => {
   expect(screen.getByRole('button', { name: /back/i })).toBeInTheDocument();
 });
 
+test('health check is called after selecting an aws integration', async () => {
+  const { ctx, discoverCtx, spyPing } = getMockedContexts({
+    kind: ResourceKind.Application,
+    appMeta: { awsConsole: true },
+    name: '',
+    icon: undefined,
+    keywords: [],
+    event: DiscoverEventResource.ApplicationHttp,
+  });
+
+  renderAwsAccount(ctx, discoverCtx);
+
+  await screen.findByText(/AWS Integrations/i);
+
+  const selectContainer = screen.getByRole('combobox');
+  fireEvent.mouseDown(selectContainer);
+  fireEvent.keyPress(selectContainer, { key: 'Enter' });
+
+  expect(spyPing).toHaveBeenCalledTimes(1);
+});
+
 function getMockedContexts(resourceSpec: ResourceSpec) {
   const ctx = createTeleportContext();
   const discoverCtx: DiscoverContextState = {
@@ -167,7 +188,21 @@ function getMockedContexts(resourceSpec: ResourceSpec) {
     .spyOn(userEventService, 'captureDiscoverEvent')
     .mockResolvedValue(undefined as never);
 
-  return { ctx, discoverCtx };
+  const spyPing = jest
+    .spyOn(integrationService, 'fetchIntegrations')
+    .mockResolvedValue({
+      items: [
+        {
+          resourceType: 'integration',
+          name: 'aws-oidc-1',
+          kind: IntegrationKind.AwsOidc,
+          spec: { roleArn: '111' },
+          statusCode: IntegrationStatusCode.Running,
+        },
+      ],
+    });
+
+  return { ctx, discoverCtx, spyPing };
 }
 
 function renderAwsAccount(

web/packages/teleport/src/Discover/Shared/AwsAccount/AwsAccount.tsx

@@ -70,8 +70,12 @@ export function AwsAccount() {
     eventState,
     resourceSpec,
     currentStep,
+    emitErrorEvent,
   } = useDiscover();
 
+  const [selectedAwsIntegration, setSelectedAwsIntegration] =
+    useState<Option>();
+
   // if true, requires an additional step where we fetch for
   // apps matching fetched aws integrations to determine
   // if an app already exists for the integration the user
@@ -99,6 +103,18 @@ export function AwsAccount() {
     }, [clusterId, isAddingAwsApp])
   );
 
+  const [healthCheckAttempt, healthCheckSelectedIntegration] = useAsync(
+    async () => {
+      await integrationService.pingAwsOidcIntegration(
+        {
+          clusterId,
+          integrationName: selectedAwsIntegration.value.name,
+        },
+        { roleArn: '' }
+      );
+    }
+  );
+
   const integrationAccess = storeUser.getIntegrationsAccess();
 
   let roleTemplate = integrationRWE;
@@ -137,9 +153,6 @@ export function AwsAccount() {
       appAccess.read;
   }
 
-  const [selectedAwsIntegration, setSelectedAwsIntegration] =
-    useState<Option>();
-
   useEffect(() => {
     if (hasAccess && attempt.status === '') {
       fetch();
@@ -193,11 +206,17 @@ export function AwsAccount() {
     );
   }
 
-  function proceedWithExistingIntegration(validator: Validator) {
+  async function proceedWithExistingIntegration(validator: Validator) {
     if (!validator.validate()) {
       return;
     }
 
+    const [, err] = await healthCheckSelectedIntegration();
+    if (err) {
+      emitErrorEvent(`failed to health check selected aws integration: ${err}`);
+      return;
+    }
+
     if (
       isAddingAwsApp &&
       attempt.status === 'success' &&
@@ -250,6 +269,12 @@ export function AwsAccount() {
   return (
     <Box maxWidth="700px">
       <Heading />
+      {healthCheckAttempt.status === 'error' && (
+        <Alert
+          kind="danger"
+          children={`Health check failed for the selected AWS integration: ${healthCheckAttempt.statusText}`}
+        />
+      )}
       <Box mb={3}>
         <Validation>
           {({ validator }) => (
@@ -289,7 +314,11 @@ export function AwsAccount() {
               <ActionButtons
                 onPrev={prevStep}
                 onProceed={() => proceedWithExistingIntegration(validator)}
-                disableProceed={!hasAwsIntegrations || !selectedAwsIntegration}
+                disableProceed={
+                  !hasAwsIntegrations ||
+                  !selectedAwsIntegration ||
+                  healthCheckAttempt.status === 'processing'
+                }
               />
             </>
           )}

web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.test.tsx

@@ -15,16 +15,21 @@
  * You should have received a copy of the GNU Affero General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
+import { useEffect } from 'react';
 import { render, screen, fireEvent, waitFor } from 'design/utils/testing';
 import userEvent from '@testing-library/user-event';
+import { MemoryRouter } from 'react-router';
 
 import {
   Integration,
   IntegrationKind,
+  integrationService,
   IntegrationStatusCode,
 } from 'teleport/services/integrations';
+import cfg from 'teleport/config';
 
 import { EditAwsOidcIntegrationDialog } from './EditAwsOidcIntegrationDialog';
+import { useIntegrationOperation } from './Operations';
 
 test('user acknowledging script was ran when reconfiguring', async () => {
   render(
@@ -97,6 +102,46 @@ test('user acknowledging script was ran when reconfiguring', async () => {
   );
 });
 
+test('health check is called before calling update', async () => {
+  const spyPing = jest
+    .spyOn(integrationService, 'pingAwsOidcIntegration')
+    .mockResolvedValue({} as any); // response doesn't matter
+
+  const spyUpdate = jest
+    .spyOn(integrationService, 'updateIntegration')
+    .mockResolvedValue({} as any); // response doesn't matter
+
+  render(
+    <MemoryRouter initialEntries={[cfg.getClusterRoute('some-cluster')]}>
+      <ComponentWithEditOperation />
+    </MemoryRouter>
+  );
+
+  // change role arn
+  fireEvent.change(screen.getByPlaceholderText(/arn:aws:iam:/i), {
+    target: { value: 'arn:aws:iam::123456789011:role/other' },
+  });
+
+  await waitFor(() =>
+    expect(screen.getByRole('button', { name: /reconfigure/i })).toBeEnabled()
+  );
+  await userEvent.click(screen.getByRole('button', { name: /reconfigure/i }));
+
+  // Click on checkbox to enable save button.
+  await userEvent.click(screen.getByRole('checkbox'));
+  await waitFor(() =>
+    expect(screen.getByRole('button', { name: /save/i })).toBeEnabled()
+  );
+  await userEvent.click(screen.getByRole('button', { name: /save/i }));
+
+  await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
+  await waitFor(() => expect(spyUpdate).toHaveBeenCalledTimes(1));
+
+  const pingOrder = spyPing.mock.invocationCallOrder[0];
+  const createOrder = spyUpdate.mock.invocationCallOrder[0];
+  expect(pingOrder).toBeLessThan(createOrder);
+});
+
 test('render warning when s3 buckets are present', async () => {
   const edit = jest.fn(() => Promise.resolve());
   render(
@@ -205,7 +250,7 @@ test('edit submit called with proper fields', async () => {
   await userEvent.click(screen.getByRole('button', { name: /save/i }));
   await waitFor(() => expect(mockEditFn).toHaveBeenCalledTimes(1));
 
-  expect(mockEditFn).toHaveBeenCalledWith({
+  expect(mockEditFn).toHaveBeenCalledWith(integration, {
     roleArn: 'arn:aws:iam::123456789011:role/other',
   });
 });
@@ -221,3 +266,18 @@ const integration: Integration = {
   },
   statusCode: IntegrationStatusCode.Running,
 };
+
+function ComponentWithEditOperation() {
+  const integrationOps = useIntegrationOperation();
+  useEffect(() => {
+    integrationOps.onEdit(integration);
+  }, []);
+
+  return (
+    <EditAwsOidcIntegrationDialog
+      close={() => null}
+      edit={(integration, req) => integrationOps.edit(integration, req).then()}
+      integration={integration}
+    />
+  );
+}

web/packages/teleport/src/Integrations/EditAwsOidcIntegrationDialog.tsx

@@ -34,13 +34,13 @@ import Dialog, {
   DialogFooter,
 } from 'design/DialogConfirmation';
 import { OutlineInfo, OutlineWarn } from 'design/Alert/Alert';
-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 { TextSelectCopyMulti } from 'shared/components/TextSelectCopy';
 
 import { FieldCheckbox } from 'shared/components/FieldCheckbox';
+import { useAsync } from 'shared/hooks/useAsync';
 
 import {
   Integration,
@@ -54,24 +54,26 @@ import { S3BucketConfiguration } from './Enroll/AwsOidc/S3BucketConfiguration';
 
 type Props = {
   close(): void;
-  edit(req: EditableIntegrationFields): Promise<void>;
+  edit(integration: Integration, req: EditableIntegrationFields): Promise<void>;
   integration: Integration;
 };
 
 export function EditAwsOidcIntegrationDialog(props: Props) {
   const { close, edit, integration } = props;
-  const { attempt, run } = useAttempt();
+  const [updateAttempt, runUpdate] = useAsync(async () => {
+    await edit(integration, { roleArn });
+  });
 
   const [roleArn, setRoleArn] = useState(integration.spec.roleArn);
   const [scriptUrl, setScriptUrl] = useState('');
   const [confirmed, setConfirmed] = useState(false);
 
-  function handleEdit(validator: Validator) {
+  async function handleEdit(validator: Validator) {
     if (!validator.validate()) {
       return;
     }
 
-    run(() => edit({ roleArn }));
+    await runUpdate();
   }
 
   function generateAwsOidcConfigIdpScript(
@@ -100,7 +102,7 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
   const s3Prefix = integration.spec.issuerS3Prefix;
   const showReadonlyS3Fields = s3Bucket || s3Prefix;
 
-  const isProcessing = attempt.status === 'processing';
+  const isProcessing = updateAttempt.status === 'processing';
   const showGenerateCommand =
     integration.spec.roleArn !== roleArn || showReadonlyS3Fields;
 
@@ -122,8 +124,8 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
             <DialogTitle>Edit Integration</DialogTitle>
           </DialogHeader>
           <DialogContent width="650px">
-            {attempt.status === 'failed' && (
-              <Alert children={attempt.statusText} />
+            {updateAttempt.status === 'error' && (
+              <Alert children={updateAttempt.statusText} />
             )}
             <FieldInput
               label="Integration Name"
@@ -238,6 +240,7 @@ export function EditAwsOidcIntegrationDialog(props: Props) {
                 onChange={e => {
                   setConfirmed(e.target.checked);
                 }}
+                disabled={isProcessing}
               />
             )}
             <ButtonPrimary

web/packages/teleport/src/Integrations/Enroll/AwsOidc/AwsOidc.test.tsx

@@ -27,6 +27,9 @@ import { MemoryRouter } from 'react-router';
 
 import { userEventService } from 'teleport/services/userEvent';
 
+import { integrationService } from 'teleport/services/integrations';
+import { ApiError } from 'teleport/services/api/parseError';
+
 import { AwsOidc } from './AwsOidc';
 
 test('render', async () => {
@@ -56,6 +59,14 @@ test('generate command', async () => {
     .spyOn(userEventService, 'captureIntegrationEnrollEvent')
     .mockImplementation();
 
+  let spyPing = jest
+    .spyOn(integrationService, 'pingAwsOidcIntegration')
+    .mockResolvedValue({} as any); // response doesn't matter
+
+  const spyCreate = jest
+    .spyOn(integrationService, 'createIntegration')
+    .mockResolvedValue({} as any); // response doesn't matter
+
   window.prompt = jest.fn();
 
   render(
@@ -95,4 +106,42 @@ test('generate command', async () => {
   const clipboardText = await navigator.clipboard.readText();
   expect(clipboardText).toContain(`integrationName=${pluginConfig.name}`);
   expect(clipboardText).toContain(`role=${pluginConfig.roleName}`);
+
+  // Fill out arn.
+  fireEvent.change(screen.getByLabelText(/role arn/i), {
+    target: {
+      value: `arn:aws:iam::123456789012:role/${pluginConfig.roleName}`,
+    },
+  });
+
+  // Test ping is called before create.
+  fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
+  await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
+  await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(1));
+
+  let pingOrder = spyPing.mock.invocationCallOrder[0];
+  let createOrder = spyCreate.mock.invocationCallOrder[0];
+  expect(pingOrder).toBeLessThan(createOrder);
+
+  // Test create is still called with 404 ping error.
+  jest.clearAllMocks();
+  let error = new ApiError('', { status: 404 } as Response);
+  spyPing = jest
+    .spyOn(integrationService, 'pingAwsOidcIntegration')
+    .mockRejectedValue(error);
+
+  fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
+  await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
+  await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(1));
+
+  // Test create isn't called with non 404 error
+  jest.clearAllMocks();
+  error = new ApiError('', { status: 400 } as Response);
+  spyPing = jest
+    .spyOn(integrationService, 'pingAwsOidcIntegration')
+    .mockRejectedValue(error);
+
+  fireEvent.click(screen.getByRole('button', { name: /create integration/i }));
+  await waitFor(() => expect(spyPing).toHaveBeenCalledTimes(1));
+  await waitFor(() => expect(spyCreate).toHaveBeenCalledTimes(0));
 });

web/packages/teleport/src/Integrations/Enroll/AwsOidc/useAwsOidcIntegration.tsx

@@ -19,7 +19,11 @@
 import { useEffect, useState } from 'react';
 import { useLocation } from 'react-router';
 import { Validator } from 'shared/components/Validation';
-import { useAsync } from 'shared/hooks/useAsync';
+import {
+  makeErrorAttempt,
+  makeProcessingAttempt,
+  useAsync,
+} from 'shared/hooks/useAsync';
 
 import { DiscoverUrlLocationState } from 'teleport/Discover/useDiscover';
 import {
@@ -36,6 +40,8 @@ import {
   integrationService,
   AwsOidcPolicyPreset,
 } from 'teleport/services/integrations';
+import useStickyClusterId from 'teleport/useStickyClusterId';
+import { ApiError } from 'teleport/services/api/parseError';
 
 type integrationConfig = {
   name: string;
@@ -53,6 +59,7 @@ export function useAwsOidcIntegration() {
   );
   const [scriptUrl, setScriptUrl] = useState('');
   const [createdIntegration, setCreatedIntegration] = useState<Integration>();
+  const { clusterId } = useStickyClusterId();
 
   const location = useLocation<DiscoverUrlLocationState>();
 
@@ -80,18 +87,47 @@ export function useAwsOidcIntegration() {
     });
   }
 
-  const [createIntegrationAttempt, runCreateIntegration] = useAsync(
-    async (req: IntegrationCreateRequest) => {
-      const resp = await integrationService.createIntegration(req);
-      setCreatedIntegration(resp);
-      return resp;
-    }
-  );
+  const [
+    createIntegrationAttempt,
+    runCreateIntegration,
+    setCreateIntegrationAttempt,
+  ] = useAsync(async (req: IntegrationCreateRequest) => {
+    const resp = await integrationService.createIntegration(req);
+    setCreatedIntegration(resp);
+    return resp;
+  });
 
   async function handleOnCreate(validator: Validator) {
     if (!validator.validate()) {
       return;
     }
+    setCreateIntegrationAttempt(makeProcessingAttempt());
+
+    try {
+      await integrationService.pingAwsOidcIntegration(
+        {
+          integrationName: integrationConfig.name,
+          clusterId,
+        },
+        { roleArn: integrationConfig.roleArn }
+      );
+    } catch (err) {
+      // DELETE IN v18.0
+      // Ignore not found error and just allow it to create which
+      // is how it used to work before anyways.
+      //
+      // If this request went to an older proxy, that didn't set the
+      // the integrationName empty if roleArn isn't empty, then the backend
+      // will never be able to successfully health check b/c it expects
+      // integration to exist first before creating.
+      const isNotFoundErr =
+        err instanceof ApiError && err.response.status === 404;
+
+      if (!isNotFoundErr) {
+        setCreateIntegrationAttempt(makeErrorAttempt(err));
+        return;
+      }
+    }
 
     const [, err] = await runCreateIntegration({
       name: integrationConfig.name,

web/packages/teleport/src/Integrations/Integrations.tsx

@@ -61,8 +61,11 @@ export function Integrations() {
     });
   }
 
-  function editIntegration(req: EditableIntegrationFields) {
-    return integrationOps.edit(req).then(updatedIntegration => {
+  function editIntegration(
+    integration: Integration,
+    req: EditableIntegrationFields
+  ) {
+    return integrationOps.edit(integration, req).then(updatedIntegration => {
       const updatedItems = items.map(item => {
         if (item.name == integrationOps.item.name) {
           return updatedIntegration;

web/packages/teleport/src/Integrations/Operations/IntegrationOperations.tsx

@@ -32,7 +32,7 @@ type Props = {
   operation: OperationType;
   integration: Integration;
   close(): void;
-  edit(req: EditableIntegrationFields): Promise<void>;
+  edit(integration: Integration, req: EditableIntegrationFields): Promise<void>;
   remove(): Promise<void>;
 };
 

web/packages/teleport/src/Integrations/Operations/useIntegrationOperation.ts

@@ -18,11 +18,17 @@
 
 import { useState } from 'react';
 
-import { integrationService } from 'teleport/services/integrations';
+import {
+  IntegrationKind,
+  integrationService,
+} from 'teleport/services/integrations';
+import useStickyClusterId from 'teleport/useStickyClusterId';
 
 import type { Integration, Plugin } from 'teleport/services/integrations';
 
 export function useIntegrationOperation() {
+  const { clusterId } = useStickyClusterId();
+
   const [operation, setOperation] = useState({
     type: 'none',
   } as Operation);
@@ -35,7 +41,25 @@ export function useIntegrationOperation() {
     return integrationService.deleteIntegration(operation.item.name);
   }
 
-  function edit(req: EditableIntegrationFields) {
+  async function edit(
+    integration: Integration,
+    req: EditableIntegrationFields
+  ) {
+    // Health check with the new roleArn to validate that
+    // connection still works.
+    if (integration.kind === IntegrationKind.AwsOidc) {
+      try {
+        await integrationService.pingAwsOidcIntegration(
+          {
+            integrationName: integration.name,
+            clusterId,
+          },
+          { roleArn: req.roleArn }
+        );
+      } catch (err) {
+        throw new Error(`Health check failed: ${err}`);
+      }
+    }
     return integrationService.updateIntegration(operation.item.name, {
       awsoidc: {
         roleArn: req.roleArn,

web/packages/teleport/src/config.ts

@@ -320,6 +320,8 @@ const cfg = {
 
     integrationsPath: '/v1/webapi/sites/:clusterId/integrations/:name?',
     thumbprintPath: '/v1/webapi/thumbprint',
+    pingAwsOidcIntegrationPath:
+      '/v1/webapi/sites/:clusterId/integrations/aws-oidc/:name/ping',
 
     awsConfigureIamScriptOidcIdpPath:
       '/v1/webapi/scripts/integrations/configure/awsoidc-idp.sh?integrationName=:integrationName&role=:roleName&policyPreset=:policyPreset?',
@@ -964,6 +966,19 @@ const cfg = {
     });
   },
 
+  getPingAwsOidcIntegrationUrl({
+    integrationName,
+    clusterId,
+  }: {
+    integrationName: string;
+    clusterId: string;
+  }) {
+    return generatePath(cfg.api.pingAwsOidcIntegrationPath, {
+      clusterId,
+      name: integrationName,
+    });
+  },
+
   getAwsRdsDbListUrl(integrationName: string) {
     const clusterId = cfg.proxyCluster;
 

web/packages/teleport/src/services/integrations/integrations.ts

@@ -57,6 +57,8 @@ import {
   ListAwsSubnetsResponse,
   Subnet,
   AwsDatabaseVpcsResponse,
+  AwsOidcPingResponse,
+  AwsOidcPingRequest,
 } from './types';
 
 export const integrationService = {
@@ -78,6 +80,16 @@ export const integrationService = {
     return api.post(cfg.getIntegrationsUrl(), req).then(makeIntegration);
   },
 
+  pingAwsOidcIntegration(
+    urlParams: {
+      integrationName: string;
+      clusterId: string;
+    },
+    req: AwsOidcPingRequest
+  ): Promise<AwsOidcPingResponse> {
+    return api.post(cfg.getPingAwsOidcIntegrationUrl(urlParams), req);
+  },
+
   updateIntegration(
     name: string,
     req: IntegrationUpdateRequest

web/packages/teleport/src/services/integrations/types.ts

@@ -83,6 +83,23 @@ export type IntegrationSpecAwsOidc = {
   audience?: IntegrationAudience;
 };
 
+export type AwsOidcPingRequest = {
+  // Define roleArn if the ping request should
+  // use this potentially new roleArn to test the
+  // connection works, typically used with updates.
+  //
+  // Leave empty if the ping request should
+  // use the roleArn stored in the integration resource,
+  // typically used when checking integration still works.
+  roleArn?: string;
+};
+
+export type AwsOidcPingResponse = {
+  accountId: string;
+  arn: string;
+  userId: string;
+};
+
 export enum IntegrationStatusCode {
   Unknown = 0,
   Running = 1,