[Security Solution] [Case] Fixes "Case connector cannot be updated wh…

…en created with a wrong field" (elastic#87223) (elastic#87399)
bmcconaghy · Jan 6, 2021 · 1097e31 · 1097e31
1 parent 374cace
commit 1097e31
Show file tree

Hide file tree

Showing 19 changed files with 331 additions and 65 deletions.
diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/service.test.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/service.test.ts
@@ -583,6 +583,19 @@ describe('Jira service', () => {
         '[Action][Jira]: Unable to get capabilities. Error: An error has occurred. Reason: Could not get capabilities'
       );
     });
+
+    test('it should throw an auth error', async () => {
+      requestMock.mockImplementation(() => {
+        const error = new Error('An error has occurred');
+        // @ts-ignore this can happen!
+        error.response = { data: 'Unauthorized' };
+        throw error;
+      });
+
+      await expect(service.getCapabilities()).rejects.toThrow(
+        '[Action][Jira]: Unable to get capabilities. Error: An error has occurred. Reason: Unauthorized'
+      );
+    });
   });
 
   describe('getIssueTypes', () => {

diff --git a/x-pack/plugins/actions/server/builtin_action_types/jira/service.ts b/x-pack/plugins/actions/server/builtin_action_types/jira/service.ts
@@ -102,10 +102,14 @@ export const createExternalService = (
     return fields;
   };
 
-  const createErrorMessage = (errorResponse: ResponseError | null | undefined): string => {
+  const createErrorMessage = (errorResponse: ResponseError | string | null | undefined): string => {
     if (errorResponse == null) {
       return '';
     }
+    if (typeof errorResponse === 'string') {
+      // Jira error.response.data can be string!!
+      return errorResponse;
+    }
 
     const { errorMessages, errors } = errorResponse;
 

diff --git a/x-pack/plugins/case/common/api/cases/configure.ts b/x-pack/plugins/case/common/api/cases/configure.ts
@@ -41,6 +41,7 @@ export const CaseConfigureResponseRt = rt.intersection([
   ConnectorMappingsRt,
   rt.type({
     version: rt.string,
+    error: rt.union([rt.string, rt.null]),
   }),
 ]);
 

diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/get_configure.test.ts b/x-pack/plugins/case/server/routes/api/cases/configure/get_configure.test.ts
@@ -19,6 +19,7 @@ import { initGetCaseConfigure } from './get_configure';
 import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
 import { mappings } from '../../../../client/configure/mock';
 import { ConnectorTypes } from '../../../../../common/api/connectors';
+import { CaseClient } from '../../../../client';
 
 describe('GET configuration', () => {
   let routeHandler: RequestHandler<any, any, any>;
@@ -43,6 +44,7 @@ describe('GET configuration', () => {
     expect(res.status).toEqual(200);
     expect(res.payload).toEqual({
       ...mockCaseConfigure[0].attributes,
+      error: null,
       mappings: mappings[ConnectorTypes.jira],
       version: mockCaseConfigure[0].version,
     });
@@ -77,6 +79,7 @@ describe('GET configuration', () => {
         email: 'testemail@elastic.co',
         username: 'elastic',
       },
+      error: null,
       mappings: mappings[ConnectorTypes.jira],
       updated_at: '2020-04-09T09:43:51.778Z',
       updated_by: {
@@ -122,4 +125,40 @@ describe('GET configuration', () => {
     expect(res.status).toEqual(404);
     expect(res.payload.isBoom).toEqual(true);
   });
+
+  it('returns an error when mappings request throws', async () => {
+    const req = httpServerMock.createKibanaRequest({
+      path: CASE_CONFIGURE_URL,
+      method: 'get',
+    });
+
+    const context = await createRouteContext(
+      createMockSavedObjectsRepository({
+        caseConfigureSavedObject: mockCaseConfigure,
+        caseMappingsSavedObject: [],
+      })
+    );
+    const mockThrowContext = {
+      ...context,
+      case: {
+        ...context.case,
+        getCaseClient: () =>
+          ({
+            ...context?.case?.getCaseClient(),
+            getMappings: () => {
+              throw new Error();
+            },
+          } as CaseClient),
+      },
+    };
+
+    const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
+    expect(res.status).toEqual(200);
+    expect(res.payload).toEqual({
+      ...mockCaseConfigure[0].attributes,
+      error: 'Error connecting to My connector 3 instance',
+      mappings: [],
+      version: mockCaseConfigure[0].version,
+    });
+  });
 });
diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/get_configure.ts b/x-pack/plugins/case/server/routes/api/cases/configure/get_configure.ts
@@ -19,6 +19,7 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
     },
     async (context, request, response) => {
       try {
+        let error = null;
         const client = context.core.savedObjects.client;
 
         const myCaseConfigure = await caseConfigureService.find({ client });
@@ -35,12 +36,18 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
           if (actionsClient == null) {
             throw Boom.notFound('Action client have not been found');
           }
-          mappings = await caseClient.getMappings({
-            actionsClient,
-            caseClient,
-            connectorId: connector.id,
-            connectorType: connector.type,
-          });
+          try {
+            mappings = await caseClient.getMappings({
+              actionsClient,
+              caseClient,
+              connectorId: connector.id,
+              connectorType: connector.type,
+            });
+          } catch (e) {
+            error = e.isBoom
+              ? e.output.payload.message
+              : `Error connecting to ${connector.name} instance`;
+          }
         }
 
         return response.ok({
@@ -51,6 +58,7 @@ export function initGetCaseConfigure({ caseConfigureService, router }: RouteDeps
                   connector: transformESConnectorToCaseConnector(connector),
                   mappings,
                   version: myCaseConfigure.saved_objects[0].version ?? '',
+                  error,
                 })
               : {},
         });

diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/patch_configure.test.ts b/x-pack/plugins/case/server/routes/api/cases/configure/patch_configure.test.ts
@@ -18,6 +18,7 @@ import { mockCaseConfigure } from '../../__fixtures__/mock_saved_objects';
 import { initPatchCaseConfigure } from './patch_configure';
 import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
 import { ConnectorTypes } from '../../../../../common/api/connectors';
+import { CaseClient } from '../../../../client';
 
 describe('PATCH configuration', () => {
   let routeHandler: RequestHandler<any, any, any>;
@@ -135,6 +136,52 @@ describe('PATCH configuration', () => {
     );
   });
 
+  it('patch configuration with error message for getMappings throw', async () => {
+    const req = httpServerMock.createKibanaRequest({
+      path: CASE_CONFIGURE_URL,
+      method: 'patch',
+      body: {
+        closure_type: 'close-by-pushing',
+        connector: {
+          id: 'connector-new',
+          name: 'New connector',
+          type: '.jira',
+          fields: null,
+        },
+        version: mockCaseConfigure[0].version,
+      },
+    });
+
+    const context = await createRouteContext(
+      createMockSavedObjectsRepository({
+        caseConfigureSavedObject: mockCaseConfigure,
+        caseMappingsSavedObject: [],
+      })
+    );
+    const mockThrowContext = {
+      ...context,
+      case: {
+        ...context.case,
+        getCaseClient: () =>
+          ({
+            ...context?.case?.getCaseClient(),
+            getMappings: () => {
+              throw new Error();
+            },
+          } as CaseClient),
+      },
+    };
+
+    const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
+
+    expect(res.status).toEqual(200);
+    expect(res.payload).toEqual(
+      expect.objectContaining({
+        mappings: [],
+        error: 'Error connecting to New connector instance',
+      })
+    );
+  });
   it('throw error when configuration have not being created', async () => {
     const req = httpServerMock.createKibanaRequest({
       path: CASE_CONFIGURE_URL,

diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/patch_configure.ts b/x-pack/plugins/case/server/routes/api/cases/configure/patch_configure.ts
@@ -33,6 +33,7 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
     },
     async (context, request, response) => {
       try {
+        let error = null;
         const client = context.core.savedObjects.client;
         const query = pipe(
           CasesConfigurePatchRt.decode(request.body),
@@ -68,12 +69,18 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
           if (actionsClient == null) {
             throw Boom.notFound('Action client have not been found');
           }
-          mappings = await caseClient.getMappings({
-            actionsClient,
-            caseClient,
-            connectorId: connector.id,
-            connectorType: connector.type,
-          });
+          try {
+            mappings = await caseClient.getMappings({
+              actionsClient,
+              caseClient,
+              connectorId: connector.id,
+              connectorType: connector.type,
+            });
+          } catch (e) {
+            error = e.isBoom
+              ? e.output.payload.message
+              : `Error connecting to ${connector.name} instance`;
+          }
         }
         const patch = await caseConfigureService.patch({
           client,
@@ -96,6 +103,7 @@ export function initPatchCaseConfigure({ caseConfigureService, caseService, rout
             ),
             mappings,
             version: patch.version ?? '',
+            error,
           }),
         });
       } catch (error) {

diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/post_configure.test.ts b/x-pack/plugins/case/server/routes/api/cases/configure/post_configure.test.ts
@@ -19,6 +19,7 @@ import { initPostCaseConfigure } from './post_configure';
 import { newConfiguration } from '../../__mocks__/request_responses';
 import { CASE_CONFIGURE_URL } from '../../../../../common/constants';
 import { ConnectorTypes } from '../../../../../common/api/connectors';
+import { CaseClient } from '../../../../client';
 
 describe('POST configuration', () => {
   let routeHandler: RequestHandler<any, any, any>;
@@ -64,6 +65,43 @@ describe('POST configuration', () => {
       })
     );
   });
+  it('create configuration with error message for getMappings throw', async () => {
+    const req = httpServerMock.createKibanaRequest({
+      path: CASE_CONFIGURE_URL,
+      method: 'post',
+      body: newConfiguration,
+    });
+
+    const context = await createRouteContext(
+      createMockSavedObjectsRepository({
+        caseConfigureSavedObject: mockCaseConfigure,
+        caseMappingsSavedObject: [],
+      })
+    );
+    const mockThrowContext = {
+      ...context,
+      case: {
+        ...context.case,
+        getCaseClient: () =>
+          ({
+            ...context?.case?.getCaseClient(),
+            getMappings: () => {
+              throw new Error();
+            },
+          } as CaseClient),
+      },
+    };
+
+    const res = await routeHandler(mockThrowContext, req, kibanaResponseFactory);
+
+    expect(res.status).toEqual(200);
+    expect(res.payload).toEqual(
+      expect.objectContaining({
+        mappings: [],
+        error: 'Error connecting to My connector 2 instance',
+      })
+    );
+  });
 
   it('create configuration without authentication', async () => {
     routeHandler = await createRoute(initPostCaseConfigure, 'post', true);

diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/post_configure.ts b/x-pack/plugins/case/server/routes/api/cases/configure/post_configure.ts
@@ -13,6 +13,7 @@ import {
   CasesConfigureRequestRt,
   CaseConfigureResponseRt,
   throwErrors,
+  ConnectorMappingsAttributes,
 } from '../../../../../common/api';
 import { RouteDeps } from '../../types';
 import { wrapError, escapeHatch } from '../../utils';
@@ -32,6 +33,7 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
     },
     async (context, request, response) => {
       try {
+        let error = null;
         if (!context.case) {
           throw Boom.badRequest('RouteHandlerContext is not registered for cases');
         }
@@ -58,12 +60,19 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
         const { email, full_name, username } = await caseService.getUser({ request, response });
 
         const creationDate = new Date().toISOString();
-        const mappings = await caseClient.getMappings({
-          actionsClient,
-          caseClient,
-          connectorId: query.connector.id,
-          connectorType: query.connector.type,
-        });
+        let mappings: ConnectorMappingsAttributes[] = [];
+        try {
+          mappings = await caseClient.getMappings({
+            actionsClient,
+            caseClient,
+            connectorId: query.connector.id,
+            connectorType: query.connector.type,
+          });
+        } catch (e) {
+          error = e.isBoom
+            ? e.output.payload.message
+            : `Error connecting to ${query.connector.name} instance`;
+        }
         const post = await caseConfigureService.post({
           client,
           attributes: {
@@ -83,6 +92,7 @@ export function initPostCaseConfigure({ caseConfigureService, caseService, route
             connector: transformESConnectorToCaseConnector(post.attributes.connector),
             mappings,
             version: post.version ?? '',
+            error,
           }),
         });
       } catch (error) {

diff --git a/x-pack/plugins/security_solution/cypress/integration/cases_connectors.spec.ts b/x-pack/plugins/security_solution/cypress/integration/cases_connectors.spec.ts
@@ -29,6 +29,7 @@ describe('Cases connectors', () => {
     closure_type: 'close-by-user',
     created_at: '2020-12-01T16:28:09.219Z',
     created_by: { email: null, full_name: null, username: 'elastic' },
+    error: null,
     updated_at: null,
     updated_by: null,
     mappings: [
@@ -47,6 +48,23 @@ describe('Cases connectors', () => {
         res.send(200, { ...configureResult, connector });
       });
     }).as('saveConnector');
+    cy.intercept('GET', '/api/cases/configure', (req) => {
+      req.reply((res) => {
+        const resBody =
+          res.body.version != null
+            ? {
+                ...res.body,
+                error: null,
+                mappings: [
+                  { source: 'title', target: 'short_description', action_type: 'overwrite' },
+                  { source: 'description', target: 'description', action_type: 'overwrite' },
+                  { source: 'comments', target: 'comments', action_type: 'append' },
+                ],
+              }
+            : res.body;
+        res.send(200, resBody);
+      });
+    });
   });
 
   it('Configures a new connector', () => {