From 641dee0f62fe3b246be84e1a916fbbfc7bc6a42f Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 9 Oct 2020 13:29:34 +0300 Subject: [PATCH] [7.10] [Security Solution][Case] Fix bug when changing connectors (#80002) (#80095) --- .../case/server/routes/api/cases/post_case.ts | 2 +- .../use_get_case_user_actions.test.tsx | 330 +++++++++++++++++- .../containers/use_get_case_user_actions.tsx | 108 ++++-- .../user_actions/get_all_user_actions.ts | 4 +- 4 files changed, 414 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/case/server/routes/api/cases/post_case.ts b/x-pack/plugins/case/server/routes/api/cases/post_case.ts index 20d8bb7a19c1b7..5d8113b685741e 100644 --- a/x-pack/plugins/case/server/routes/api/cases/post_case.ts +++ b/x-pack/plugins/case/server/routes/api/cases/post_case.ts @@ -66,7 +66,7 @@ export function initPostCaseApi({ actionAt: createdDate, actionBy: { username, full_name, email }, caseId: newCase.id, - fields: ['description', 'status', 'tags', 'title'], + fields: ['description', 'status', 'tags', 'title', 'connector'], newValue: JSON.stringify(query), }), ], diff --git a/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.test.tsx b/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.test.tsx index b00df5524c8b50..fdfe740e5123da 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.test.tsx +++ b/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.test.tsx @@ -111,6 +111,7 @@ describe('useGetCaseUserActions', () => { }); }); }); + describe('getPushedInfo', () => { it('Correctly marks first/last index - hasDataToPush: false', () => { const userActions = [...caseUserActions, getUserAction(['pushed'], 'push-to-service')]; @@ -226,7 +227,7 @@ describe('useGetCaseUserActions', () => { }); }); - it('Does not count connector_id update as a reason to push', () => { + it('Does not count connector update as a reason to push', () => { const userActions = [ ...caseUserActions, getUserAction(['pushed'], 'push-to-service'), @@ -246,6 +247,7 @@ describe('useGetCaseUserActions', () => { }, }); }); + it('Correctly handles multiple push actions', () => { const userActions = [ ...caseUserActions, @@ -267,6 +269,7 @@ describe('useGetCaseUserActions', () => { }, }); }); + it('Correctly handles comment update with multiple push actions', () => { const userActions = [ ...caseUserActions, @@ -298,6 +301,7 @@ describe('useGetCaseUserActions', () => { connector_name: 'other connector name', external_id: 'other_external_id', }; + const pushAction456 = { ...getUserAction(['pushed'], 'push-to-service'), newValue: JSON.stringify(push456), @@ -309,7 +313,9 @@ describe('useGetCaseUserActions', () => { getUserAction(['comment'], 'create'), pushAction456, ]; + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ hasDataToPush: true, caseServices: { @@ -342,6 +348,7 @@ describe('useGetCaseUserActions', () => { connector_name: 'other connector name', external_id: 'other_external_id', }; + const pushAction456 = { ...getUserAction(['pushed'], 'push-to-service'), newValue: JSON.stringify(push456), @@ -353,6 +360,7 @@ describe('useGetCaseUserActions', () => { getUserAction(['comment'], 'create'), pushAction456, ]; + const result = getPushedInfo(userActions, '456'); expect(result).toEqual({ hasDataToPush: false, @@ -377,5 +385,325 @@ describe('useGetCaseUserActions', () => { }, }); }); + + it('Change fields of current connector - hasDataToPush: true', () => { + const userActions = [ + ...caseUserActions, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: true, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 3, + lastPushIndex: 3, + commentsToUpdate: [], + hasDataToPush: true, + }, + }, + }); + }); + + it('Change current connector - hasDataToPush: true', () => { + const userActions = [ + ...caseUserActions, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: false, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 3, + lastPushIndex: 3, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); + + it('Change connector and back - hasDataToPush: true', () => { + const userActions = [ + ...caseUserActions, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: false, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 3, + lastPushIndex: 3, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); + + it('Change fields and connector after push - hasDataToPush: true', () => { + const userActions = [ + ...caseUserActions, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'Low' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: true, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 4, + lastPushIndex: 4, + commentsToUpdate: [], + hasDataToPush: true, + }, + }, + }); + }); + + it('Change only connector after push - hasDataToPush: false', () => { + const userActions = [ + ...caseUserActions, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: false, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 4, + lastPushIndex: 4, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); + + it('Change connectors and fields - multiple pushes', () => { + const pushAction123 = getUserAction(['pushed'], 'push-to-service'); + const push456 = { + ...basicPushSnake, + connector_id: '456', + connector_name: 'other connector name', + external_id: 'other_external_id', + }; + + const pushAction456 = { + ...getUserAction(['pushed'], 'push-to-service'), + newValue: JSON.stringify(push456), + }; + + const userActions = [ + ...caseUserActions, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + pushAction123, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + pushAction456, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'Low' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'Low' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'Low' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: true, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 4, + lastPushIndex: 4, + commentsToUpdate: [], + hasDataToPush: true, + }, + '456': { + ...basicPush, + connectorId: '456', + connectorName: 'other connector name', + externalId: 'other_external_id', + firstPushIndex: 6, + lastPushIndex: 6, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); + + it('pushing other connectors does not count as an update', () => { + const pushAction123 = getUserAction(['pushed'], 'push-to-service'); + const push456 = { + ...basicPushSnake, + connector_id: '456', + connector_name: 'other connector name', + external_id: 'other_external_id', + }; + + const pushAction456 = { + ...getUserAction(['pushed'], 'push-to-service'), + newValue: JSON.stringify(push456), + }; + const userActions = [ + ...caseUserActions, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + pushAction123, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + pushAction456, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: false, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 4, + lastPushIndex: 4, + commentsToUpdate: [], + hasDataToPush: false, + }, + '456': { + ...basicPush, + connectorId: '456', + connectorName: 'other connector name', + externalId: 'other_external_id', + firstPushIndex: 6, + lastPushIndex: 6, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); + + it('Changing other connectors fields does not count as an update', () => { + const userActions = [ + ...caseUserActions, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: null } }), + newValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + }, + getUserAction(['pushed'], 'push-to-service'), + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '123', fields: { issueType: '10006', priority: 'High' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + }, + { + ...getUserAction(['connector'], 'update'), + oldValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '6' } }), + newValue: JSON.stringify({ id: '456', fields: { issueTypes: ['10'], severity: '3' } }), + }, + ]; + + const result = getPushedInfo(userActions, '123'); + expect(result).toEqual({ + hasDataToPush: false, + caseServices: { + '123': { + ...basicPush, + firstPushIndex: 4, + lastPushIndex: 4, + commentsToUpdate: [], + hasDataToPush: false, + }, + }, + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.tsx b/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.tsx index afbd1b163cec64..ccc8a69df96ee0 100644 --- a/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.tsx +++ b/x-pack/plugins/security_solution/public/cases/containers/use_get_case_user_actions.tsx @@ -12,7 +12,7 @@ import { errorToToaster, useStateToaster } from '../../common/components/toaster import { CaseFullExternalService } from '../../../../case/common/api/cases'; import { getCaseUserActions } from './api'; import * as i18n from './translations'; -import { CaseExternalService, CaseUserActions, ElasticUser } from './types'; +import { CaseConnector, CaseExternalService, CaseUserActions, ElasticUser } from './types'; import { convertToCamelCase, parseString } from './utils'; export interface CaseService extends CaseExternalService { @@ -51,27 +51,65 @@ export interface UseGetCaseUserActions extends CaseUserActionsState { const getExternalService = (value: string): CaseExternalService | null => convertToCamelCase(parseString(`${value}`)); -const connectorHasChangedFields = (action: CaseUserActions, connectorId: string): boolean => { - if (action.action !== 'update' || action.actionField[0] !== 'connector') { - return false; - } +const groupConnectorFields = ( + userActions: CaseUserActions[] +): Record> => + userActions.reduce((acc, mua) => { + if (mua.actionField[0] !== 'connector') { + return acc; + } - const oldValue = parseString(`${action.oldValue}`); - const newValue = parseString(`${action.newValue}`); + const oldValue = parseString(`${mua.oldValue}`); + const newValue = parseString(`${mua.newValue}`); + + if (oldValue == null || newValue == null) { + return acc; + } - if (oldValue == null || newValue == null) { + return { + ...acc, + [oldValue.id]: [ + ...(acc[oldValue.id] || []), + ...(oldValue.id === newValue.id ? [oldValue.fields, newValue.fields] : [oldValue.fields]), + ], + [newValue.id]: [ + ...(acc[newValue.id] || []), + ...(oldValue.id === newValue.id ? [oldValue.fields, newValue.fields] : [newValue.fields]), + ], + }; + }, {} as Record>); + +const connectorHasChangedFields = ({ + connectorFieldsBeforePush, + connectorFieldsAfterPush, + connectorId, +}: { + connectorFieldsBeforePush: Record> | null; + connectorFieldsAfterPush: Record> | null; + connectorId: string; +}): boolean => { + if (connectorFieldsAfterPush == null || connectorFieldsAfterPush[connectorId] == null) { return false; } - if (oldValue.id !== connectorId || newValue.id !== connectorId) { - return false; + const fieldsAfterPush = connectorFieldsAfterPush[connectorId]; + + if (connectorFieldsBeforePush != null && connectorFieldsBeforePush[connectorId] != null) { + const fieldsBeforePush = connectorFieldsBeforePush[connectorId]; + return !deepEqual( + fieldsBeforePush[fieldsBeforePush.length - 1], + fieldsAfterPush[fieldsAfterPush.length - 1] + ); } - if (oldValue.id !== newValue.id) { - return false; + if (fieldsAfterPush.length >= 2) { + return !deepEqual( + fieldsAfterPush[fieldsAfterPush.length - 2], + fieldsAfterPush[fieldsAfterPush.length - 1] + ); } - return !deepEqual(oldValue.fields, newValue.fields); + return false; }; interface CommentsAndIndex { @@ -86,22 +124,40 @@ export const getPushedInfo = ( caseServices: CaseServices; hasDataToPush: boolean; } => { - const hasDataToPushForConnector = (connectorId: string) => { - const userActionsForPushLessServiceUpdates = caseUserActions.filter((mua) => { - if (mua.action !== 'push-to-service') { - if (mua.action === 'update' && mua.actionField[0] === 'connector') { - return connectorHasChangedFields(mua, connectorId); - } else { - return true; - } - } else { - return connectorId === getExternalService(`${mua.newValue}`)?.connectorId; - } + const hasDataToPushForConnector = (connectorId: string): boolean => { + const caseUserActionsReversed = [...caseUserActions].reverse(); + const lastPushOfConnectorReversedIndex = caseUserActionsReversed.findIndex( + (mua) => + mua.action === 'push-to-service' && + getExternalService(`${mua.newValue}`)?.connectorId === connectorId + ); + + if (lastPushOfConnectorReversedIndex === -1) { + return true; + } + + const lastPushOfConnectorIndex = + caseUserActionsReversed.length - lastPushOfConnectorReversedIndex - 1; + + const actionsBeforePush = caseUserActions.slice(0, lastPushOfConnectorIndex); + const actionsAfterPush = caseUserActions.slice( + lastPushOfConnectorIndex + 1, + caseUserActionsReversed.length + ); + + const connectorFieldsBeforePush = groupConnectorFields(actionsBeforePush); + const connectorFieldsAfterPush = groupConnectorFields(actionsAfterPush); + + const connectorHasChanged = connectorHasChangedFields({ + connectorFieldsBeforePush, + connectorFieldsAfterPush, + connectorId, }); return ( - userActionsForPushLessServiceUpdates[userActionsForPushLessServiceUpdates.length - 1] - .action !== 'push-to-service' + actionsAfterPush.some( + (mua) => mua.actionField[0] !== 'connector' && mua.action !== 'push-to-service' + ) || connectorHasChanged ); }; diff --git a/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts b/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts index 594bd727d910fc..e53013348c66b6 100644 --- a/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts +++ b/x-pack/test/case_api_integration/basic/tests/cases/user_actions/get_all_user_actions.ts @@ -35,7 +35,7 @@ export default ({ getService }: FtrProviderContext): void => { await actionsRemover.removeAll(); }); - it(`on new case, user action: 'create' should be called with actionFields: ['description', 'status', 'tags', 'title']`, async () => { + it(`on new case, user action: 'create' should be called with actionFields: ['description', 'status', 'tags', 'title', 'connector']`, async () => { const { body: postedCase } = await supertest .post(CASES_URL) .set('kbn-xsrf', 'true') @@ -48,7 +48,7 @@ export default ({ getService }: FtrProviderContext): void => { .expect(200); expect(body.length).to.eql(1); - expect(body[0].action_field).to.eql(['description', 'status', 'tags', 'title']); + expect(body[0].action_field).to.eql(['description', 'status', 'tags', 'title', 'connector']); expect(body[0].action).to.eql('create'); expect(body[0].old_value).to.eql(null); expect(body[0].new_value).to.eql(JSON.stringify(postCaseReq));