From f67b20dc2b20d85b9cf817c6b844c4e6adb025ec Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 16 Oct 2020 18:07:11 +0300 Subject: [PATCH] [Security Solution][Cases] Fix bug with case connectors (#80642) * Fix bug with case connectors * Improve isCaseOwned function --- .../routes/api/__mocks__/request_responses.ts | 39 ++++ .../cases/configure/get_connectors.test.ts | 67 ++++++- .../api/cases/configure/get_connectors.ts | 44 +++-- .../basic/tests/configure/get_connectors.ts | 167 ++++++++++++++++-- .../case_api_integration/common/lib/utils.ts | 47 ++++- 5 files changed, 327 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/case/server/routes/api/__mocks__/request_responses.ts b/x-pack/plugins/case/server/routes/api/__mocks__/request_responses.ts index bd276bc91ca3e..ce35b99750419 100644 --- a/x-pack/plugins/case/server/routes/api/__mocks__/request_responses.ts +++ b/x-pack/plugins/case/server/routes/api/__mocks__/request_responses.ts @@ -62,6 +62,45 @@ export const getActions = (): FindActionResult[] => [ isPreconfigured: false, referencedByCount: 0, }, + { + id: '456', + actionTypeId: '.jira', + name: 'Connector without isCaseOwned', + config: { + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'short_description', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + apiUrl: 'https://elastic.jira.com', + }, + isPreconfigured: false, + referencedByCount: 0, + }, + { + id: '789', + actionTypeId: '.resilient', + name: 'Connector without mapping', + config: { + apiUrl: 'https://elastic.resilient.com', + }, + isPreconfigured: false, + referencedByCount: 0, + }, ]; export const newConfiguration: CasesConfigureRequest = { diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.test.ts b/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.test.ts index d7a01ef069867..ee4dcc8e81b95 100644 --- a/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.test.ts +++ b/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.test.ts @@ -15,7 +15,6 @@ import { import { mockCaseConfigure } from '../../__fixtures__/mock_saved_objects'; import { initCaseConfigureGetActionConnector } from './get_connectors'; -import { getActions } from '../../__mocks__/request_responses'; import { CASE_CONFIGURE_CONNECTORS_URL } from '../../../../../common/constants'; describe('GET connectors', () => { @@ -24,7 +23,7 @@ describe('GET connectors', () => { routeHandler = await createRoute(initCaseConfigureGetActionConnector, 'get'); }); - it('returns the connectors', async () => { + it('returns case owned connectors', async () => { const req = httpServerMock.createKibanaRequest({ path: `${CASE_CONFIGURE_CONNECTORS_URL}/_find`, method: 'get', @@ -38,9 +37,67 @@ describe('GET connectors', () => { const res = await routeHandler(context, req, kibanaResponseFactory); expect(res.status).toEqual(200); - expect(res.payload).toEqual( - getActions().filter((action) => action.actionTypeId === '.servicenow') - ); + expect(res.payload).toEqual([ + { + id: '123', + actionTypeId: '.servicenow', + name: 'ServiceNow', + config: { + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'short_description', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + apiUrl: 'https://dev102283.service-now.com', + isCaseOwned: true, + }, + isPreconfigured: false, + referencedByCount: 0, + }, + { + id: '456', + actionTypeId: '.jira', + name: 'Connector without isCaseOwned', + config: { + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'short_description', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + apiUrl: 'https://elastic.jira.com', + }, + isPreconfigured: false, + referencedByCount: 0, + }, + ]); }); it('it throws an error when actions client is null', async () => { diff --git a/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.ts b/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.ts index 545ccf82c3d78..c3e565a404e97 100644 --- a/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.ts +++ b/x-pack/plugins/case/server/routes/api/cases/configure/get_connectors.ts @@ -7,15 +7,44 @@ import Boom from 'boom'; import { RouteDeps } from '../../types'; import { wrapError } from '../../utils'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { FindActionResult } from '../../../../../../actions/server/types'; import { CASE_CONFIGURE_CONNECTORS_URL, - SUPPORTED_CONNECTORS, SERVICENOW_ACTION_TYPE_ID, JIRA_ACTION_TYPE_ID, RESILIENT_ACTION_TYPE_ID, } from '../../../../../common/constants'; +/** + * We need to take into account connectors that have been created within cases and + * they do not have the isCaseOwned field. Checking for the existence of + * the mapping attribute ensures that the connector is indeed a case connector. + * Cases connector should always have a mapping. + */ + +interface CaseAction extends FindActionResult { + config?: { + isCaseOwned?: boolean; + incidentConfiguration?: Record; + }; +} + +const isCaseOwned = (action: CaseAction): boolean => { + if ( + [SERVICENOW_ACTION_TYPE_ID, JIRA_ACTION_TYPE_ID, RESILIENT_ACTION_TYPE_ID].includes( + action.actionTypeId + ) + ) { + if (action.config?.isCaseOwned === true || action.config?.incidentConfiguration?.mapping) { + return true; + } + } + + return false; +}; + /* * Be aware that this api will only return 20 connectors */ @@ -34,18 +63,7 @@ export function initCaseConfigureGetActionConnector({ caseService, router }: Rou throw Boom.notFound('Action client have not been found'); } - const results = (await actionsClient.getAll()).filter( - (action) => - SUPPORTED_CONNECTORS.includes(action.actionTypeId) && - // Need this filtering temporary to display only Case owned ServiceNow connectors - (![SERVICENOW_ACTION_TYPE_ID, JIRA_ACTION_TYPE_ID, RESILIENT_ACTION_TYPE_ID].includes( - action.actionTypeId - ) || - ([SERVICENOW_ACTION_TYPE_ID, JIRA_ACTION_TYPE_ID, RESILIENT_ACTION_TYPE_ID].includes( - action.actionTypeId - ) && - action.config?.isCaseOwned === true)) - ); + const results = (await actionsClient.getAll()).filter(isCaseOwned); return response.ok({ body: results }); } catch (error) { return response.customError(wrapError(error)); diff --git a/x-pack/test/case_api_integration/basic/tests/configure/get_connectors.ts b/x-pack/test/case_api_integration/basic/tests/configure/get_connectors.ts index 5ba1aac4c8f92..5195d28d84830 100644 --- a/x-pack/test/case_api_integration/basic/tests/configure/get_connectors.ts +++ b/x-pack/test/case_api_integration/basic/tests/configure/get_connectors.ts @@ -13,6 +13,8 @@ import { getServiceNowConnector, getJiraConnector, getResilientConnector, + getConnectorWithoutCaseOwned, + getConnectorWithoutMapping, } from '../../../common/lib/utils'; // eslint-disable-next-line import/no-default-export @@ -36,13 +38,13 @@ export default ({ getService }: FtrProviderContext): void => { }); it('should return the correct connectors', async () => { - const { body: connectorOne } = await supertest + const { body: snConnector } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'true') .send(getServiceNowConnector()) .expect(200); - const { body: connectorTwo } = await supertest + const { body: emailConnector } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'true') .send({ @@ -59,22 +61,36 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - const { body: connectorThree } = await supertest + const { body: jiraConnector } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'true') .send(getJiraConnector()) .expect(200); - const { body: connectorFour } = await supertest + const { body: resilientConnector } = await supertest .post('/api/actions/action') .set('kbn-xsrf', 'true') .send(getResilientConnector()) .expect(200); - actionsRemover.add('default', connectorOne.id, 'action', 'actions'); - actionsRemover.add('default', connectorTwo.id, 'action', 'actions'); - actionsRemover.add('default', connectorThree.id, 'action', 'actions'); - actionsRemover.add('default', connectorFour.id, 'action', 'actions'); + const { body: connectorWithoutCaseOwned } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getConnectorWithoutCaseOwned()) + .expect(200); + + const { body: connectorNoMapping } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getConnectorWithoutMapping()) + .expect(200); + + actionsRemover.add('default', snConnector.id, 'action', 'actions'); + actionsRemover.add('default', emailConnector.id, 'action', 'actions'); + actionsRemover.add('default', jiraConnector.id, 'action', 'actions'); + actionsRemover.add('default', resilientConnector.id, 'action', 'actions'); + actionsRemover.add('default', connectorWithoutCaseOwned.id, 'action', 'actions'); + actionsRemover.add('default', connectorNoMapping.id, 'action', 'actions'); const { body: connectors } = await supertest .get(`${CASE_CONFIGURE_CONNECTORS_URL}/_find`) @@ -82,16 +98,131 @@ export default ({ getService }: FtrProviderContext): void => { .send() .expect(200); - expect(connectors.length).to.equal(3); - expect( - connectors.some((c: { actionTypeId: string }) => c.actionTypeId === '.servicenow') - ).to.equal(true); - expect(connectors.some((c: { actionTypeId: string }) => c.actionTypeId === '.jira')).to.equal( - true - ); - expect( - connectors.some((c: { actionTypeId: string }) => c.actionTypeId === '.resilient') - ).to.equal(true); + expect(connectors).to.eql([ + { + id: connectorWithoutCaseOwned.id, + actionTypeId: '.resilient', + name: 'Connector without isCaseOwned', + config: { + apiUrl: 'http://some.non.existent.com', + orgId: 'pkey', + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'name', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + isCaseOwned: null, + }, + isPreconfigured: false, + referencedByCount: 0, + }, + { + id: jiraConnector.id, + actionTypeId: '.jira', + name: 'Jira Connector', + config: { + apiUrl: 'http://some.non.existent.com', + projectKey: 'pkey', + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'summary', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + isCaseOwned: true, + }, + isPreconfigured: false, + referencedByCount: 0, + }, + { + id: resilientConnector.id, + actionTypeId: '.resilient', + name: 'Resilient Connector', + config: { + apiUrl: 'http://some.non.existent.com', + orgId: 'pkey', + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'name', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + isCaseOwned: true, + }, + isPreconfigured: false, + referencedByCount: 0, + }, + { + id: snConnector.id, + actionTypeId: '.servicenow', + name: 'ServiceNow Connector', + config: { + apiUrl: 'http://some.non.existent.com', + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'short_description', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'append', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + isCaseOwned: true, + }, + isPreconfigured: false, + referencedByCount: 0, + }, + ]); }); }); }; diff --git a/x-pack/test/case_api_integration/common/lib/utils.ts b/x-pack/test/case_api_integration/common/lib/utils.ts index 8d28f647ce43b..262e14fac6d8c 100644 --- a/x-pack/test/case_api_integration/common/lib/utils.ts +++ b/x-pack/test/case_api_integration/common/lib/utils.ts @@ -116,7 +116,7 @@ export const getResilientConnector = () => ({ mapping: [ { source: 'title', - target: 'summary', + target: 'name', actionType: 'overwrite', }, { @@ -135,6 +135,51 @@ export const getResilientConnector = () => ({ }, }); +export const getConnectorWithoutCaseOwned = () => ({ + name: 'Connector without isCaseOwned', + actionTypeId: '.resilient', + secrets: { + apiKeyId: 'id', + apiKeySecret: 'secret', + }, + config: { + apiUrl: 'http://some.non.existent.com', + orgId: 'pkey', + incidentConfiguration: { + mapping: [ + { + source: 'title', + target: 'name', + actionType: 'overwrite', + }, + { + source: 'description', + target: 'description', + actionType: 'overwrite', + }, + { + source: 'comments', + target: 'comments', + actionType: 'append', + }, + ], + }, + }, +}); + +export const getConnectorWithoutMapping = () => ({ + name: 'Connector without mapping', + actionTypeId: '.resilient', + secrets: { + apiKeyId: 'id', + apiKeySecret: 'secret', + }, + config: { + apiUrl: 'http://some.non.existent.com', + orgId: 'pkey', + }, +}); + export const removeServerGeneratedPropertiesFromConfigure = ( config: Partial ): Partial => {