From 7a35075e21f9f2148acd3e8cab16a45bfe9a7c6b Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Fri, 5 Feb 2021 13:05:40 -0800 Subject: [PATCH 01/13] added ids to incoming exception item entries, no ui component changes yet --- x-pack/plugins/lists/common/constants.mock.ts | 28 + x-pack/plugins/lists/common/shared_imports.ts | 2 + .../public/exceptions/hooks/use_api.test.ts | 612 +++++++++--------- .../lists/public/exceptions/hooks/use_api.ts | 20 +- .../hooks/use_exception_list_items.test.ts | 10 +- .../hooks/use_exception_list_items.ts | 22 +- .../public/exceptions/transforms.test.ts | 281 ++++++++ .../lists/public/exceptions/transforms.ts | 107 +++ .../add_remove_id_to_item.test.ts | 0 .../utils => common}/add_remove_id_to_item.ts | 0 .../common/shared_exports.ts | 1 + .../components/threat_match/helpers.tsx | 2 +- .../detection_engine/rules/transforms.ts | 2 +- 13 files changed, 780 insertions(+), 307 deletions(-) create mode 100644 x-pack/plugins/lists/public/exceptions/transforms.test.ts create mode 100644 x-pack/plugins/lists/public/exceptions/transforms.ts rename x-pack/plugins/security_solution/{public/common/utils => common}/add_remove_id_to_item.test.ts (100%) rename x-pack/plugins/security_solution/{public/common/utils => common}/add_remove_id_to_item.ts (100%) diff --git a/x-pack/plugins/lists/common/constants.mock.ts b/x-pack/plugins/lists/common/constants.mock.ts index 6f8782c148d452..22a5b2d9cc55bc 100644 --- a/x-pack/plugins/lists/common/constants.mock.ts +++ b/x-pack/plugins/lists/common/constants.mock.ts @@ -72,6 +72,34 @@ export const ENDPOINT_ENTRIES: EndpointEntriesArray = [ }, { field: 'some.not.nested.field', operator: 'included', type: 'match', value: 'some value' }, ]; +// ENTRIES_WITH_IDS should only be used to mock out functionality of a collection of transforms +// that are UI specific and useful for UI concerns that are inserted between the +// API and the actual user interface. In some ways these might be viewed as +// technical debt or to compensate for the differences and preferences +// of how ReactJS might prefer data vs. how we want to model data. +export const ENTRIES_WITH_IDS: EntriesArray = [ + { + entries: [ + { + field: 'nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + }, + { + field: 'some.not.nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, +]; export const ITEM_TYPE = 'simple'; export const OS_TYPES: OsTypeArray = ['windows']; export const TAGS = []; diff --git a/x-pack/plugins/lists/common/shared_imports.ts b/x-pack/plugins/lists/common/shared_imports.ts index 7e96b13c036ec5..2483c1f7dd9926 100644 --- a/x-pack/plugins/lists/common/shared_imports.ts +++ b/x-pack/plugins/lists/common/shared_imports.ts @@ -12,6 +12,8 @@ export { DefaultStringArray, DefaultVersionNumber, DefaultVersionNumberDecoded, + addIdToItem, + removeIdFromItem, exactCheck, getPaths, foldLeftRight, diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts index 0fec2ab23994b2..cd6696d5d2baed 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts @@ -7,6 +7,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; +import { ENTRIES_WITH_IDS } from '../../../common/constants.mock'; import { coreMock } from '../../../../../../src/core/public/mocks'; import * as api from '../api'; import { getExceptionListSchemaMock } from '../../../common/schemas/response/exception_list_schema.mock'; @@ -17,6 +18,10 @@ import { ApiCallByIdProps, ApiCallByListIdProps } from '../types'; import { ExceptionsApi, useApi } from './use_api'; +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + const mockKibanaHttpService = coreMock.createStart().http; describe('useApi', () => { @@ -27,343 +32,370 @@ describe('useApi', () => { jest.clearAllMocks(); }); - test('it invokes "deleteExceptionListItemById" when "deleteExceptionItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnDeleteExceptionListItemById = jest - .spyOn(api, 'deleteExceptionListItemById') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = payload; - - await result.current.deleteExceptionItem({ - id, - namespaceType, - onError: jest.fn(), - onSuccess: onSuccessMock, + describe('deleteExceptionItem', () => { + test('it invokes "deleteExceptionListItemById" when "deleteExceptionItem" used', async () => { + const payload = getExceptionListItemSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnDeleteExceptionListItemById = jest + .spyOn(api, 'deleteExceptionListItemById') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = payload; + + await result.current.deleteExceptionItem({ + id, + namespaceType, + onError: jest.fn(), + onSuccess: onSuccessMock, + }); + + const expected: ApiCallByIdProps = { + http: mockKibanaHttpService, + id, + namespaceType, + signal: new AbortController().signal, + }; + + expect(spyOnDeleteExceptionListItemById).toHaveBeenCalledWith(expected); + expect(onSuccessMock).toHaveBeenCalled(); }); - - const expected: ApiCallByIdProps = { - http: mockKibanaHttpService, - id, - namespaceType, - signal: new AbortController().signal, - }; - - expect(spyOnDeleteExceptionListItemById).toHaveBeenCalledWith(expected); - expect(onSuccessMock).toHaveBeenCalled(); }); - }); - test('invokes "onError" callback if "deleteExceptionListItemById" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'deleteExceptionListItemById').mockRejectedValue(mockError); + test('invokes "onError" callback if "deleteExceptionListItemById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'deleteExceptionListItemById').mockRejectedValue(mockError); - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - const { id, namespace_type: namespaceType } = getExceptionListItemSchemaMock(); + const { id, namespace_type: namespaceType } = getExceptionListItemSchemaMock(); - await result.current.deleteExceptionItem({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); + await result.current.deleteExceptionItem({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); - expect(onErrorMock).toHaveBeenCalledWith(mockError); + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); }); }); - test('it invokes "deleteExceptionListById" when "deleteExceptionList" used', async () => { - const payload = getExceptionListSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnDeleteExceptionListById = jest - .spyOn(api, 'deleteExceptionListById') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = payload; - - await result.current.deleteExceptionList({ - id, - namespaceType, - onError: jest.fn(), - onSuccess: onSuccessMock, + describe('deleteExceptionList', () => { + test('it invokes "deleteExceptionListById" when "deleteExceptionList" used', async () => { + const payload = getExceptionListSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnDeleteExceptionListById = jest + .spyOn(api, 'deleteExceptionListById') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = payload; + + await result.current.deleteExceptionList({ + id, + namespaceType, + onError: jest.fn(), + onSuccess: onSuccessMock, + }); + + const expected: ApiCallByIdProps = { + http: mockKibanaHttpService, + id, + namespaceType, + signal: new AbortController().signal, + }; + + expect(spyOnDeleteExceptionListById).toHaveBeenCalledWith(expected); + expect(onSuccessMock).toHaveBeenCalled(); }); - - const expected: ApiCallByIdProps = { - http: mockKibanaHttpService, - id, - namespaceType, - signal: new AbortController().signal, - }; - - expect(spyOnDeleteExceptionListById).toHaveBeenCalledWith(expected); - expect(onSuccessMock).toHaveBeenCalled(); }); - }); - test('invokes "onError" callback if "deleteExceptionListById" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'deleteExceptionListById').mockRejectedValue(mockError); + test('invokes "onError" callback if "deleteExceptionListById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'deleteExceptionListById').mockRejectedValue(mockError); - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - await result.current.deleteExceptionList({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); + await result.current.deleteExceptionList({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); - expect(onErrorMock).toHaveBeenCalledWith(mockError); + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); }); }); - test('it invokes "fetchExceptionListItemById" when "getExceptionItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnFetchExceptionListItemById = jest - .spyOn(api, 'fetchExceptionListItemById') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = payload; - - await result.current.getExceptionItem({ - id, - namespaceType, - onError: jest.fn(), - onSuccess: onSuccessMock, + describe('getExceptionItem', () => { + test('it invokes "fetchExceptionListItemById" when "getExceptionItem" used', async () => { + const payload = getExceptionListItemSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnFetchExceptionListItemById = jest + .spyOn(api, 'fetchExceptionListItemById') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = payload; + + await result.current.getExceptionItem({ + id, + namespaceType, + onError: jest.fn(), + onSuccess: onSuccessMock, + }); + + const expected: ApiCallByIdProps = { + http: mockKibanaHttpService, + id, + namespaceType, + signal: new AbortController().signal, + }; + const expectedExceptionListItem = { + ...getExceptionListItemSchemaMock(), + entries: ENTRIES_WITH_IDS, + }; + + expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); + expect(onSuccessMock).toHaveBeenCalledWith(expectedExceptionListItem); }); - - const expected: ApiCallByIdProps = { - http: mockKibanaHttpService, - id, - namespaceType, - signal: new AbortController().signal, - }; - - expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); - expect(onSuccessMock).toHaveBeenCalled(); }); - }); - test('invokes "onError" callback if "fetchExceptionListItemById" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'fetchExceptionListItemById').mockRejectedValue(mockError); + test('invokes "onError" callback if "fetchExceptionListItemById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'fetchExceptionListItemById').mockRejectedValue(mockError); - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - await result.current.getExceptionItem({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); + await result.current.getExceptionItem({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); - expect(onErrorMock).toHaveBeenCalledWith(mockError); + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); }); }); - test('it invokes "fetchExceptionListById" when "getExceptionList" used', async () => { - const payload = getExceptionListSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnFetchExceptionListById = jest - .spyOn(api, 'fetchExceptionListById') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = payload; - - await result.current.getExceptionList({ - id, - namespaceType, - onError: jest.fn(), - onSuccess: onSuccessMock, + describe('getExceptionList', () => { + test('it invokes "fetchExceptionListById" when "getExceptionList" used', async () => { + const payload = getExceptionListSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnFetchExceptionListById = jest + .spyOn(api, 'fetchExceptionListById') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = payload; + + await result.current.getExceptionList({ + id, + namespaceType, + onError: jest.fn(), + onSuccess: onSuccessMock, + }); + + const expected: ApiCallByIdProps = { + http: mockKibanaHttpService, + id, + namespaceType, + signal: new AbortController().signal, + }; + + expect(spyOnFetchExceptionListById).toHaveBeenCalledWith(expected); + expect(onSuccessMock).toHaveBeenCalled(); }); - - const expected: ApiCallByIdProps = { - http: mockKibanaHttpService, - id, - namespaceType, - signal: new AbortController().signal, - }; - - expect(spyOnFetchExceptionListById).toHaveBeenCalledWith(expected); - expect(onSuccessMock).toHaveBeenCalled(); }); - }); - test('invokes "onError" callback if "fetchExceptionListById" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'fetchExceptionListById').mockRejectedValue(mockError); + test('invokes "onError" callback if "fetchExceptionListById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'fetchExceptionListById').mockRejectedValue(mockError); - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - await result.current.getExceptionList({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); + await result.current.getExceptionList({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); - expect(onErrorMock).toHaveBeenCalledWith(mockError); - }); - }); - - test('it invokes "fetchExceptionListsItemsByListIds" when "getExceptionItem" used', async () => { - const output = getFoundExceptionListItemSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnFetchExceptionListsItemsByListIds = jest - .spyOn(api, 'fetchExceptionListsItemsByListIds') - .mockResolvedValue(output); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - await result.current.getExceptionListsItems({ - filterOptions: [], - lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }], - onError: jest.fn(), - onSuccess: onSuccessMock, - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - showDetectionsListsOnly: false, - showEndpointListsOnly: false, + expect(onErrorMock).toHaveBeenCalledWith(mockError); }); - - const expected: ApiCallByListIdProps = { - filterOptions: [], - http: mockKibanaHttpService, - listIds: ['list_id'], - namespaceTypes: ['single'], - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - signal: new AbortController().signal, - }; - - expect(spyOnFetchExceptionListsItemsByListIds).toHaveBeenCalledWith(expected); - expect(onSuccessMock).toHaveBeenCalled(); }); }); - test('it does not invoke "fetchExceptionListsItemsByListIds" if no listIds', async () => { - const output = getFoundExceptionListItemSchemaMock(); - const onSuccessMock = jest.fn(); - const spyOnFetchExceptionListsItemsByListIds = jest - .spyOn(api, 'fetchExceptionListsItemsByListIds') - .mockResolvedValue(output); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - await result.current.getExceptionListsItems({ - filterOptions: [], - lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }], - onError: jest.fn(), - onSuccess: onSuccessMock, - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - showDetectionsListsOnly: false, - showEndpointListsOnly: true, + describe('getExceptionListsItems', () => { + test('it invokes "fetchExceptionListsItemsByListIds" when "getExceptionListsItems" used', async () => { + const output = getFoundExceptionListItemSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnFetchExceptionListsItemsByListIds = jest + .spyOn(api, 'fetchExceptionListsItemsByListIds') + .mockResolvedValue(output); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + await result.current.getExceptionListsItems({ + filterOptions: [], + lists: [ + { id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }, + ], + onError: jest.fn(), + onSuccess: onSuccessMock, + pagination: { + page: 1, + perPage: 1, + total: 0, + }, + showDetectionsListsOnly: false, + showEndpointListsOnly: false, + }); + + const expected: ApiCallByListIdProps = { + filterOptions: [], + http: mockKibanaHttpService, + listIds: ['list_id'], + namespaceTypes: ['single'], + pagination: { + page: 1, + perPage: 1, + total: 0, + }, + signal: new AbortController().signal, + }; + + expect(spyOnFetchExceptionListsItemsByListIds).toHaveBeenCalledWith(expected); + expect(onSuccessMock).toHaveBeenCalledWith({ + exceptions: [{ ...getExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }], + pagination: { + page: 1, + perPage: 1, + total: 1, + }, + }); }); + }); - expect(spyOnFetchExceptionListsItemsByListIds).not.toHaveBeenCalled(); - expect(onSuccessMock).toHaveBeenCalledWith({ - exceptions: [], - pagination: { - page: 0, - perPage: 20, - total: 0, - }, + test('it does not invoke "fetchExceptionListsItemsByListIds" if no listIds', async () => { + const output = getFoundExceptionListItemSchemaMock(); + const onSuccessMock = jest.fn(); + const spyOnFetchExceptionListsItemsByListIds = jest + .spyOn(api, 'fetchExceptionListsItemsByListIds') + .mockResolvedValue(output); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + await result.current.getExceptionListsItems({ + filterOptions: [], + lists: [ + { id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }, + ], + onError: jest.fn(), + onSuccess: onSuccessMock, + pagination: { + page: 1, + perPage: 20, + total: 0, + }, + showDetectionsListsOnly: false, + showEndpointListsOnly: true, + }); + + expect(spyOnFetchExceptionListsItemsByListIds).not.toHaveBeenCalled(); + expect(onSuccessMock).toHaveBeenCalledWith({ + exceptions: [], + pagination: { + page: 0, + perPage: 20, + total: 0, + }, + }); }); }); - }); - test('invokes "onError" callback if "fetchExceptionListsItemsByListIds" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'fetchExceptionListsItemsByListIds').mockRejectedValue(mockError); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - await result.current.getExceptionListsItems({ - filterOptions: [], - lists: [{ id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }], - onError: onErrorMock, - onSuccess: jest.fn(), - pagination: { - page: 1, - perPage: 20, - total: 0, - }, - showDetectionsListsOnly: false, - showEndpointListsOnly: false, + test('invokes "onError" callback if "fetchExceptionListsItemsByListIds" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'fetchExceptionListsItemsByListIds').mockRejectedValue(mockError); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + await result.current.getExceptionListsItems({ + filterOptions: [], + lists: [ + { id: 'myListId', listId: 'list_id', namespaceType: 'single', type: 'detection' }, + ], + onError: onErrorMock, + onSuccess: jest.fn(), + pagination: { + page: 1, + perPage: 20, + total: 0, + }, + showDetectionsListsOnly: false, + showEndpointListsOnly: false, + }); + + expect(onErrorMock).toHaveBeenCalledWith(mockError); }); - - expect(onErrorMock).toHaveBeenCalledWith(mockError); }); }); }); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts index 91050c5fff795c..9dc774889b3087 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts @@ -12,6 +12,7 @@ import { HttpStart } from '../../../../../../src/core/public'; import { ExceptionListItemSchema, ExceptionListSchema } from '../../../common/schemas'; import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types'; import { getIdsAndNamespaces } from '../utils'; +import { transformInput } from '../transforms'; export interface ExceptionsApi { deleteExceptionItem: (arg: ApiCallMemoProps) => Promise; @@ -100,12 +101,14 @@ export const useApi = (http: HttpStart): ExceptionsApi => { const abortCtrl = new AbortController(); try { - const item = await Api.fetchExceptionListItemById({ - http, - id, - namespaceType, - signal: abortCtrl.signal, - }); + const item = transformInput( + await Api.fetchExceptionListItemById({ + http, + id, + namespaceType, + signal: abortCtrl.signal, + }) + ); onSuccess(item); } catch (error) { onError(error); @@ -163,7 +166,10 @@ export const useApi = (http: HttpStart): ExceptionsApi => { signal: abortCtrl.signal, }); onSuccess({ - exceptions: data, + // This data transform is UI specific and useful for UI concerns + // to compensate for the differences and preferences of how ReactJS might prefer + // data vs. how we want to model data. View `transformInput` for more details + exceptions: data.map((item) => transformInput(item)), pagination: { page, perPage, diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.test.ts index d5d26387818792..1191b240d27bbd 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.test.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.test.ts @@ -12,9 +12,14 @@ import * as api from '../api'; import { getFoundExceptionListItemSchemaMock } from '../../../common/schemas/response/found_exception_list_item_schema.mock'; import { ExceptionListItemSchema } from '../../../common/schemas'; import { UseExceptionListItemsSuccess, UseExceptionListProps } from '../types'; +import { transformInput } from '../transforms'; import { ReturnExceptionListAndItems, useExceptionListItems } from './use_exception_list_items'; +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + const mockKibanaHttpService = coreMock.createStart().http; describe('useExceptionListItems', () => { @@ -99,8 +104,9 @@ describe('useExceptionListItems', () => { await waitForNextUpdate(); await waitForNextUpdate(); - const expectedListItemsResult: ExceptionListItemSchema[] = getFoundExceptionListItemSchemaMock() - .data; + const expectedListItemsResult: ExceptionListItemSchema[] = getFoundExceptionListItemSchemaMock().data.map( + (item) => transformInput(item) + ); const expectedResult: UseExceptionListItemsSuccess = { exceptions: expectedListItemsResult, pagination: { page: 1, perPage: 1, total: 1 }, diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts index 50271530b42e79..69fca90512f9e7 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts @@ -11,6 +11,7 @@ import { fetchExceptionListsItemsByListIds } from '../api'; import { FilterExceptionsOptions, Pagination, UseExceptionListProps } from '../types'; import { ExceptionListItemSchema } from '../../../common/schemas'; import { getIdsAndNamespaces } from '../utils'; +import { transformInput } from '../transforms'; type Func = () => void; export type ReturnExceptionListAndItems = [ @@ -95,8 +96,12 @@ export const useExceptionListItems = ({ } setLoading(false); } else { - // eslint-disable-next-line @typescript-eslint/naming-convention - const { page, per_page, total, data } = await fetchExceptionListsItemsByListIds({ + const { + page, + per_page: perPage, + total, + data, + } = await fetchExceptionListsItemsByListIds({ filterOptions: filters, http, listIds: ids, @@ -108,20 +113,25 @@ export const useExceptionListItems = ({ signal: abortCtrl.signal, }); + // This data transform is UI specific and useful for UI concerns + // to compensate for the differences and preferences of how ReactJS might prefer + // data vs. how we want to model data. View `transformInput` for more details + const transformedData = data.map((item) => transformInput(item)); + if (isSubscribed) { setPagination({ page, - perPage: per_page, + perPage, total, }); - setExceptionListItems(data); + setExceptionListItems(transformedData); if (onSuccess != null) { onSuccess({ - exceptions: data, + exceptions: transformedData, pagination: { page, - perPage: per_page, + perPage, total, }, }); diff --git a/x-pack/plugins/lists/public/exceptions/transforms.test.ts b/x-pack/plugins/lists/public/exceptions/transforms.test.ts new file mode 100644 index 00000000000000..649b18a07a5c92 --- /dev/null +++ b/x-pack/plugins/lists/public/exceptions/transforms.test.ts @@ -0,0 +1,281 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ExceptionListItemSchema } from '../../common/schemas/response/exception_list_item_schema'; +import { UpdateExceptionListItemSchema } from '../../common/schemas/request/update_exception_list_item_schema'; +import { CreateExceptionListItemSchema } from '../../common/schemas/request/create_exception_list_item_schema'; +import { getCreateExceptionListItemSchemaMock } from '../../common/schemas/request/create_exception_list_item_schema.mock'; +import { getUpdateExceptionListItemSchemaMock } from '../../common/schemas/request/update_exception_list_item_schema.mock'; +import { getExceptionListItemSchemaMock } from '../../common/schemas/response/exception_list_item_schema.mock'; +import { ENTRIES_WITH_IDS } from '../../common/constants.mock'; + +import { + addIdToExceptionItemEntries, + removeIdFromExceptionItemsEntries, + transformInput, + transformOutput, +} from './transforms'; + +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + +describe('Exceptions transforms', () => { + describe('transformOutput', () => { + it('returns same output as input with stripped ids per entry - CreateExceptionListItemSchema', () => { + const mockCreateExceptionItem = { + ...getCreateExceptionListItemSchemaMock(), + entries: ENTRIES_WITH_IDS, + }; + const output = transformOutput(mockCreateExceptionItem); + const expectedOutput: CreateExceptionListItemSchema = getCreateExceptionListItemSchemaMock(); + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with stripped ids per entry - UpdateExceptionListItemSchema', () => { + const mockUpdateExceptionItem = { + ...getUpdateExceptionListItemSchemaMock(), + entries: ENTRIES_WITH_IDS, + }; + const output = transformOutput(mockUpdateExceptionItem); + const expectedOutput: UpdateExceptionListItemSchema = getUpdateExceptionListItemSchemaMock(); + + expect(output).toEqual(expectedOutput); + }); + }); + + describe('transformInput', () => { + it('returns same output as input with added ids per entry', () => { + const mockExceptionItem = getExceptionListItemSchemaMock(); + const output = transformInput(mockExceptionItem); + const expectedOutput: ExceptionListItemSchema = { + ...getExceptionListItemSchemaMock(), + entries: ENTRIES_WITH_IDS, + }; + + expect(output).toEqual(expectedOutput); + }); + }); + + describe('addIdToExceptionItemEntries', () => { + it('returns same output as input with added ids per entry', () => { + const mockExceptionItem = { + ...getExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + const output = addIdToExceptionItemEntries(mockExceptionItem); + const expectedOutput: ExceptionListItemSchema = { + ...getExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with added ids per nested entry', () => { + const mockExceptionItem = { + ...getExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + type: 'nested', + }, + ], + }; + const output = addIdToExceptionItemEntries(mockExceptionItem); + const expectedOutput: ExceptionListItemSchema = { + ...getExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + }); + + describe('removeIdFromExceptionItemsEntries', () => { + it('returns same output as input with stripped ids per entry - CreateExceptionListItemSchema', () => { + const mockCreateExceptionItem = { + ...getCreateExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + const output = removeIdFromExceptionItemsEntries(mockCreateExceptionItem); + const expectedOutput: CreateExceptionListItemSchema = { + ...getCreateExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with stripped ids per nested entry - CreateExceptionListItemSchema', () => { + const mockCreateExceptionItem = { + ...getCreateExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + }, + ], + }; + const output = removeIdFromExceptionItemsEntries(mockCreateExceptionItem); + const expectedOutput: CreateExceptionListItemSchema = { + ...getCreateExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + type: 'nested', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with stripped ids per entry - UpdateExceptionListItemSchema', () => { + const mockUpdateExceptionItem = { + ...getUpdateExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + const output = removeIdFromExceptionItemsEntries(mockUpdateExceptionItem); + const expectedOutput: UpdateExceptionListItemSchema = { + ...getUpdateExceptionListItemSchemaMock(), + entries: [ + { + field: 'some.not.nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with stripped ids per nested entry - UpdateExceptionListItemSchema', () => { + const mockUpdateExceptionItem = { + ...getUpdateExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + id: '123', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + }, + ], + }; + const output = removeIdFromExceptionItemsEntries(mockUpdateExceptionItem); + const expectedOutput: UpdateExceptionListItemSchema = { + ...getUpdateExceptionListItemSchemaMock(), + entries: [ + { + entries: [ + { + field: 'nested.field', + operator: 'included', + type: 'match', + value: 'some value', + }, + ], + field: 'some.parentField', + type: 'nested', + }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + }); +}); diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts new file mode 100644 index 00000000000000..5dbba9f5ecec69 --- /dev/null +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -0,0 +1,107 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { flow } from 'fp-ts/lib/function'; + +import { + CreateExceptionListItemSchema, + ExceptionListItemSchema, + UpdateExceptionListItemSchema, +} from '../../common'; +import { addIdToItem, removeIdFromItem } from '../../common/shared_imports'; + +// These are a collection of transforms that are UI specific and useful for UI concerns +// that are inserted between the API and the actual user interface. In some ways these +// might be viewed as technical debt or to compensate for the differences and preferences +// of how ReactJS might prefer data vs. how we want to model data. Each function should have +// a description giving context around the transform. + +/** + * Transforms the output of exception items to compensate for technical debt or UI concerns such as + * ReactJS preferences for having ids within arrays if the data is not modeled that way. + * + * If you add a new transform of the output called "myNewTransform" do it + * in the form of: + * flow(removeIdFromExceptionItemsEntries, myNewTransform)(exceptionItem) + * + * @param exceptionItem The exceptionItem to transform the output of + * @returns The exceptionItem transformed from the output + */ +export const transformOutput = ( + exceptionItem: CreateExceptionListItemSchema | UpdateExceptionListItemSchema +): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => + flow(removeIdFromExceptionItemsEntries)(exceptionItem); + +/** + * Transforms the output of rules to compensate for technical debt or UI concerns such as + * ReactJS preferences for having ids within arrays if the data is not modeled that way. + * + * If you add a new transform of the input called "myNewTransform" do it + * in the form of: + * flow(addIdToExceptionItemEntries, myNewTransform)(exceptionItem) + * + * @param exceptionItem The exceptionItem to transform the output of + * @returns The exceptionItem transformed from the output + */ +export const transformInput = (exceptionItem: ExceptionListItemSchema): ExceptionListItemSchema => + flow(addIdToExceptionItemEntries)(exceptionItem); + +/** + * This adds an id to the incoming exception item entries as ReactJS prefers to have + * an id added to them for use as a stable id. Later if we decide to change the data + * model to have id's within the array then this code should be removed. If not, then + * this code should stay as an adapter for ReactJS. + * + * This does break the type system slightly as we are lying a bit to the type system as we return + * the same exceptionItem as we have previously but are augmenting the arrays with an id which TypeScript + * doesn't mind us doing here. However, downstream you will notice that you have an id when the type + * does not indicate it. In that case use (ExceptionItem & { id: string }) temporarily if you're using the id. If you're not, + * you can ignore the id and just use the normal TypeScript with ReactJS. + * + * @param exceptionItem The exceptionItem to add an id to the threat matches. + * @returns exceptionItem The exceptionItem but with id added to the exception item entries + */ +export const addIdToExceptionItemEntries = ( + exceptionItem: ExceptionListItemSchema +): ExceptionListItemSchema => { + const entries = exceptionItem.entries.map((entry) => { + if (entry.type === 'nested') { + return addIdToItem({ + ...entry, + entries: entry.entries.map((nestedEntry) => addIdToItem(nestedEntry)), + }); + } else { + return addIdToItem(entry); + } + }); + return { ...exceptionItem, entries }; +}; + +/** + * This removes an id from the exceptionItem entries as ReactJS prefers to have + * an id added to them for use as a stable id. Later if we decide to change the data + * model to have id's within the array then this code should be removed. If not, then + * this code should stay as an adapter for ReactJS. + * + * @param exceptionItem The exceptionItem to remove an id from the entries. + * @returns exceptionItem The exceptionItem but with id removed from the entries + */ +export const removeIdFromExceptionItemsEntries = ( + exceptionItem: CreateExceptionListItemSchema | UpdateExceptionListItemSchema +): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => { + const entries = exceptionItem.entries.map((entry) => { + if (entry.type === 'nested') { + return removeIdFromItem({ + ...entry, + entries: entry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), + }); + } else { + return removeIdFromItem(entry); + } + }); + return { ...exceptionItem, entries }; +}; diff --git a/x-pack/plugins/security_solution/public/common/utils/add_remove_id_to_item.test.ts b/x-pack/plugins/security_solution/common/add_remove_id_to_item.test.ts similarity index 100% rename from x-pack/plugins/security_solution/public/common/utils/add_remove_id_to_item.test.ts rename to x-pack/plugins/security_solution/common/add_remove_id_to_item.test.ts diff --git a/x-pack/plugins/security_solution/public/common/utils/add_remove_id_to_item.ts b/x-pack/plugins/security_solution/common/add_remove_id_to_item.ts similarity index 100% rename from x-pack/plugins/security_solution/public/common/utils/add_remove_id_to_item.ts rename to x-pack/plugins/security_solution/common/add_remove_id_to_item.ts diff --git a/x-pack/plugins/security_solution/common/shared_exports.ts b/x-pack/plugins/security_solution/common/shared_exports.ts index 3e70361655c01a..f10aaf45dcac33 100644 --- a/x-pack/plugins/security_solution/common/shared_exports.ts +++ b/x-pack/plugins/security_solution/common/shared_exports.ts @@ -19,3 +19,4 @@ export { validate, validateEither } from './validate'; export { formatErrors } from './format_errors'; export { migratePackagePolicyToV7110 } from './endpoint/policy/migrations/to_v7_11_0'; export { migratePackagePolicyToV7120 } from './endpoint/policy/migrations/to_v7_12_0'; +export { addIdToItem, removeIdFromItem } from './add_remove_id_to_item'; diff --git a/x-pack/plugins/security_solution/public/common/components/threat_match/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/threat_match/helpers.tsx index eced8d785792d5..ef3e9280e6e6b0 100644 --- a/x-pack/plugins/security_solution/public/common/components/threat_match/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/threat_match/helpers.tsx @@ -14,7 +14,7 @@ import { import { IndexPattern, IFieldType } from '../../../../../../../src/plugins/data/common'; import { Entry, FormattedEntry, ThreatMapEntries, EmptyEntry } from './types'; -import { addIdToItem } from '../../utils/add_remove_id_to_item'; +import { addIdToItem } from '../../../../common/add_remove_id_to_item'; /** * Formats the entry into one that is easily usable for the UI. diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/transforms.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/transforms.ts index 45961454b9c78b..7eb91e259a72f1 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/transforms.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/transforms.ts @@ -6,7 +6,7 @@ */ import { flow } from 'fp-ts/lib/function'; -import { addIdToItem, removeIdFromItem } from '../../../../common/utils/add_remove_id_to_item'; +import { addIdToItem, removeIdFromItem } from '../../../../../common/add_remove_id_to_item'; import { CreateRulesSchema, UpdateRulesSchema, From ec75e51a7554c57fe65bf6f943d766b37f88fd59 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Fri, 5 Feb 2021 13:50:42 -0800 Subject: [PATCH 02/13] added code to strip ids from outgoing exception items, no ui component changes yet --- .../hooks/persist_exception_item.test.ts | 38 ++++++++++++++++++- .../hooks/persist_exception_item.ts | 5 ++- .../public/exceptions/transforms.test.ts | 23 +++++------ .../lists/public/exceptions/transforms.ts | 13 +++++-- 4 files changed, 61 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts index ed16a405a51073..3244115516c101 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts @@ -7,6 +7,7 @@ import { act, renderHook } from '@testing-library/react-hooks'; +import { ENTRIES_WITH_IDS } from '../../../common/constants.mock'; import { coreMock } from '../../../../../../src/core/public/mocks'; import * as api from '../api'; import { getCreateExceptionListItemSchemaMock } from '../../../common/schemas/request/create_exception_list_item_schema.mock'; @@ -78,12 +79,45 @@ describe('usePersistExceptionItem', () => { >(() => usePersistExceptionItem({ http: mockKibanaHttpService, onError })); await waitForNextUpdate(); - result.current[1](getUpdateExceptionListItemSchemaMock()); + // NOTE: Take note here passing in an exception item where it's + // entries have been enriched with ids to ensure that they get stripped + // before the call goes through + result.current[1]({ ...getUpdateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }); await waitForNextUpdate(); expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); expect(addExceptionItem).not.toHaveBeenCalled(); - expect(updateExceptionItem).toHaveBeenCalled(); + expect(updateExceptionItem).toHaveBeenCalledWith({ + http: mockKibanaHttpService, + listItem: getUpdateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }); + }); + }); + + test('it invokes "addExceptionListItem" when payload has "id"', async () => { + const updateExceptionItem = jest.spyOn(api, 'updateExceptionListItem'); + const createExceptionItem = jest.spyOn(api, 'addExceptionListItem'); + await act(async () => { + const { result, waitForNextUpdate } = renderHook< + PersistHookProps, + ReturnPersistExceptionItem + >(() => usePersistExceptionItem({ http: mockKibanaHttpService, onError })); + + await waitForNextUpdate(); + // NOTE: Take note here passing in an exception item where it's + // entries have been enriched with ids to ensure that they get stripped + // before the call goes through + result.current[1]({ ...getCreateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }); + await waitForNextUpdate(); + + expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); + expect(updateExceptionItem).not.toHaveBeenCalled(); + expect(createExceptionItem).toHaveBeenCalledWith({ + http: mockKibanaHttpService, + listItem: getCreateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }); }); }); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts index 995c6b8703bf43..d3e5f5c97944de 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts @@ -9,6 +9,7 @@ import { Dispatch, useEffect, useState } from 'react'; import { UpdateExceptionListItemSchema } from '../../../common/schemas'; import { addExceptionListItem, updateExceptionListItem } from '../api'; +import { transformOutput } from '../transforms'; import { AddExceptionListItem, PersistHookProps } from '../types'; interface PersistReturnExceptionItem { @@ -50,13 +51,13 @@ export const usePersistExceptionItem = ({ if (isUpdateExceptionItem(exceptionListItem)) { await updateExceptionListItem({ http, - listItem: exceptionListItem, + listItem: transformOutput(exceptionListItem), signal: abortCtrl.signal, }); } else { await addExceptionListItem({ http, - listItem: exceptionListItem, + listItem: transformOutput(exceptionListItem), signal: abortCtrl.signal, }); } diff --git a/x-pack/plugins/lists/public/exceptions/transforms.test.ts b/x-pack/plugins/lists/public/exceptions/transforms.test.ts index 649b18a07a5c92..12b0f0bd8624a4 100644 --- a/x-pack/plugins/lists/public/exceptions/transforms.test.ts +++ b/x-pack/plugins/lists/public/exceptions/transforms.test.ts @@ -12,6 +12,7 @@ import { getCreateExceptionListItemSchemaMock } from '../../common/schemas/reque import { getUpdateExceptionListItemSchemaMock } from '../../common/schemas/request/update_exception_list_item_schema.mock'; import { getExceptionListItemSchemaMock } from '../../common/schemas/response/exception_list_item_schema.mock'; import { ENTRIES_WITH_IDS } from '../../common/constants.mock'; +import { Entry, EntryMatch, EntryNested } from '../../common/schemas'; import { addIdToExceptionItemEntries, @@ -64,7 +65,7 @@ describe('Exceptions transforms', () => { describe('addIdToExceptionItemEntries', () => { it('returns same output as input with added ids per entry', () => { - const mockExceptionItem = { + const mockExceptionItem: ExceptionListItemSchema = { ...getExceptionListItemSchemaMock(), entries: [ { @@ -85,7 +86,7 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as Entry & { id: string }, ], }; @@ -93,7 +94,7 @@ describe('Exceptions transforms', () => { }); it('returns same output as input with added ids per nested entry', () => { - const mockExceptionItem = { + const mockExceptionItem: ExceptionListItemSchema = { ...getExceptionListItemSchemaMock(), entries: [ { @@ -122,12 +123,12 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as EntryMatch & { id: string }, ], field: 'some.parentField', id: '123', type: 'nested', - }, + } as EntryNested & { id: string }, ], }; @@ -146,7 +147,7 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as Entry & { id: string }, ], }; const output = removeIdFromExceptionItemsEntries(mockCreateExceptionItem); @@ -177,12 +178,12 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as EntryMatch & { id: string }, ], field: 'some.parentField', id: '123', type: 'nested', - }, + } as EntryNested & { id: string }, ], }; const output = removeIdFromExceptionItemsEntries(mockCreateExceptionItem); @@ -217,7 +218,7 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as Entry & { id: string }, ], }; const output = removeIdFromExceptionItemsEntries(mockUpdateExceptionItem); @@ -248,12 +249,12 @@ describe('Exceptions transforms', () => { operator: 'included', type: 'match', value: 'some value', - }, + } as EntryMatch & { id: string }, ], field: 'some.parentField', id: '123', type: 'nested', - }, + } as EntryNested & { id: string }, ], }; const output = removeIdFromExceptionItemsEntries(mockUpdateExceptionItem); diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts index 5dbba9f5ecec69..91c2e3fb207607 100644 --- a/x-pack/plugins/lists/public/exceptions/transforms.ts +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -9,6 +9,11 @@ import { flow } from 'fp-ts/lib/function'; import { CreateExceptionListItemSchema, + Entry, + EntryExists, + EntryMatch, + EntryMatchAny, + EntryNested, ExceptionListItemSchema, UpdateExceptionListItemSchema, } from '../../common'; @@ -95,12 +100,14 @@ export const removeIdFromExceptionItemsEntries = ( ): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => { const entries = exceptionItem.entries.map((entry) => { if (entry.type === 'nested') { - return removeIdFromItem({ + return removeIdFromItem({ ...entry, - entries: entry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), + entries: entry.entries.map((nestedEntry) => + removeIdFromItem(nestedEntry) + ), }); } else { - return removeIdFromItem(entry); + return removeIdFromItem(entry); } }); return { ...exceptionItem, entries }; From 76e2f6218d4654c2120a80c4060b8cb7fddc29fb Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Fri, 5 Feb 2021 14:48:05 -0800 Subject: [PATCH 03/13] made updates to UI portion to now make use of the entry ids --- .../hooks/persist_exception_item.ts | 4 ++ .../hooks/use_exception_list_items.ts | 5 +- .../exceptions/builder/entry_item.tsx | 12 +++- .../exceptions/builder/exception_item.tsx | 59 ++++++++++--------- .../components/exceptions/builder/helpers.tsx | 30 ++++++---- .../components/exceptions/builder/index.tsx | 7 +-- .../exceptions/builder/reducer.test.ts | 4 ++ .../components/exceptions/helpers.test.tsx | 22 ++++++- .../common/components/exceptions/helpers.tsx | 39 +++++++++--- .../common/components/exceptions/types.ts | 1 + 10 files changed, 124 insertions(+), 59 deletions(-) diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts index d3e5f5c97944de..3a7d0327003d20 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts @@ -51,12 +51,16 @@ export const usePersistExceptionItem = ({ if (isUpdateExceptionItem(exceptionListItem)) { await updateExceptionListItem({ http, + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` listItem: transformOutput(exceptionListItem), signal: abortCtrl.signal, }); } else { await addExceptionListItem({ http, + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` listItem: transformOutput(exceptionListItem), signal: abortCtrl.signal, }); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts index 69fca90512f9e7..b9a8628d2ceac1 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_exception_list_items.ts @@ -113,9 +113,8 @@ export const useExceptionListItems = ({ signal: abortCtrl.signal, }); - // This data transform is UI specific and useful for UI concerns - // to compensate for the differences and preferences of how ReactJS might prefer - // data vs. how we want to model data. View `transformInput` for more details + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` const transformedData = data.map((item) => transformInput(item)); if (isSubscribed) { diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx index e90e91fa66827c..af3b5362cbbf21 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.tsx @@ -138,7 +138,11 @@ export const BuilderEntryItem: React.FC = ({ ); } else { - return comboBox; + return ( + + {comboBox} + + ); } }, [handleFieldChange, indexPattern, entry, listType] @@ -176,7 +180,11 @@ export const BuilderEntryItem: React.FC = ({ ); } else { - return comboBox; + return ( + + {comboBox} + + ); } }; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx index 402496ef00f66d..f9afa48408e398 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/exception_item.tsx @@ -111,35 +111,38 @@ export const BuilderExceptionListItemComponent = React.memo - {entries.map((item, index) => ( - - - {item.nested === 'child' && } - - { + const key = (item as typeof item & { id?: string }).id ?? `${index}`; + return ( + + + {item.nested === 'child' && } + + + + - - - - - ))} + + + ); + })} diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index a08f869b41d6f2..1eae18c9f7c8bf 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -37,6 +37,7 @@ import { } from '../types'; import { getEntryValue, getExceptionOperatorSelect } from '../helpers'; import exceptionableFields from '../exceptionable_fields.json'; +import { addIdToItem } from '../../../../../common'; /** * Returns filtered index patterns based on the field - if a user selects to @@ -160,6 +161,7 @@ export const getFormattedBuilderEntry = ( ? { ...foundField, name: foundField.name.split('.').slice(-1)[0] } : foundField, correspondingKeywordField, + id: item.id, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), nested: 'child', @@ -169,6 +171,7 @@ export const getFormattedBuilderEntry = ( } else { return { field: foundField, + id: item.id, correspondingKeywordField, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), @@ -215,6 +218,7 @@ export const getFormattedBuilderEntries = ( } else { const parentEntry: FormattedBuilderEntry = { operator: isOperator, + id: item.id, nested: 'parent', field: isNewNestedEntry ? undefined @@ -603,18 +607,20 @@ export const getEntryOnListChange = ( }; }; -export const getDefaultEmptyEntry = (): EmptyEntry => ({ - field: '', - type: OperatorTypeEnum.MATCH, - operator: OperatorEnum.INCLUDED, - value: '', -}); - -export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => ({ - field: '', - type: OperatorTypeEnum.NESTED, - entries: [], -}); +export const getDefaultEmptyEntry = (): EmptyEntry => + addIdToItem({ + field: '', + type: OperatorTypeEnum.MATCH, + operator: OperatorEnum.INCLUDED, + value: '', + }); + +export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => + addIdToItem({ + field: '', + type: OperatorTypeEnum.NESTED, + entries: [], + }); export const containsValueListEntry = (items: ExceptionsBuilderExceptionItem[]): boolean => items.some((item) => item.entries.some((entry) => entry.type === OperatorTypeEnum.LIST)); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx index 46e3ec08e7c508..a77f355af11b46 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx @@ -9,6 +9,7 @@ import React, { useCallback, useEffect, useReducer } from 'react'; import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import styled from 'styled-components'; +import { addIdToItem } from '../../../../../common'; import { Type } from '../../../../../common/detection_engine/schemas/common/schemas'; import { BuilderExceptionListItemComponent } from './exception_item'; import { IIndexPattern } from '../../../../../../../../src/plugins/data/common'; @@ -240,8 +241,6 @@ export const ExceptionBuilderComponent = ({ entries: [...entries, isNested ? getDefaultNestedEmptyEntry() : getDefaultEmptyEntry()], }; - // setAndLogicIncluded(updatedException.entries.length > 1); - setUpdateExceptions([...exceptions.slice(0, exceptions.length - 1), { ...updatedException }]); }, [setUpdateExceptions, exceptions] @@ -287,12 +286,12 @@ export const ExceptionBuilderComponent = ({ ...lastEntry, entries: [ ...lastEntry.entries, - { + addIdToItem({ field: '', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, value: '', - }, + }), ], }, ], diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts index 0741f561c19338..dbac7d325b63ad 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/reducer.test.ts @@ -14,6 +14,10 @@ import { ExceptionsBuilderExceptionItem } from '../types'; import { Action, State, exceptionsBuilderReducer } from './reducer'; import { getDefaultEmptyEntry } from './helpers'; +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + const initialState: State = { disableAnd: false, disableNested: false, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx index f40df27c6ba7f9..8b8faa0d75d4c7 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx @@ -47,7 +47,11 @@ import { getEntryMatchAnyMock } from '../../../../../lists/common/schemas/types/ import { getEntryExistsMock } from '../../../../../lists/common/schemas/types/entry_exists.mock'; import { getEntryListMock } from '../../../../../lists/common/schemas/types/entry_list.mock'; import { getCommentsArrayMock } from '../../../../../lists/common/schemas/types/comment.mock'; -import { ENTRIES, OLD_DATE_RELATIVE_TO_DATE_NOW } from '../../../../../lists/common/constants.mock'; +import { + ENTRIES, + ENTRIES_WITH_IDS, + OLD_DATE_RELATIVE_TO_DATE_NOW, +} from '../../../../../lists/common/constants.mock'; import { CreateExceptionListItemSchema, ExceptionListItemSchema, @@ -56,6 +60,10 @@ import { } from '../../../../../lists/common/schemas'; import { IIndexPattern } from 'src/plugins/data/common'; +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + describe('Exception helpers', () => { beforeEach(() => { moment.tz.setDefault('UTC'); @@ -228,6 +236,18 @@ describe('Exception helpers', () => { }); describe('#filterExceptionItems', () => { + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` + test('it correctly validates entries that include a temporary `id`', () => { + const output: Array< + ExceptionListItemSchema | CreateExceptionListItemSchema + > = filterExceptionItems([ + { ...getExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }, + ]); + + expect(output).toEqual([{ ...getExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }]); + }); + test('it removes entry items with "value" of "undefined"', () => { const { entries, ...rest } = getExceptionListItemSchemaMock(); const mockEmptyException: EmptyEntry = { diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx index 1a66dd2f27cc2b..4fb838d4b46c1d 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx @@ -36,12 +36,16 @@ import { UpdateExceptionListItemSchema, EntryNested, OsTypeArray, + EntryExists, + EntryMatch, + EntryMatchAny, } from '../../../shared_imports'; import { IIndexPattern } from '../../../../../../../src/plugins/data/common'; import { validate } from '../../../../common/validate'; import { Ecs } from '../../../../common/ecs'; import { CodeSignature } from '../../../../common/ecs/file'; import { WithCopyToClipboard } from '../../lib/clipboard/with_copy_to_clipboard'; +import { addIdToItem, removeIdFromItem } from '../../../../common'; /** * Returns the operator type, may not need this if using io-ts types @@ -148,12 +152,12 @@ export const getNewExceptionItem = ({ comments: [], description: `${ruleName} - exception list item`, entries: [ - { + addIdToItem({ field: '', operator: 'included', type: 'match', value: '', - }, + }), ], item_id: undefined, list_id: listId, @@ -172,15 +176,32 @@ export const filterExceptionItems = ( ): Array => { return exceptions.reduce>( (acc, exception) => { - const entries = exception.entries.filter((t) => { - const [validatedEntry] = validate(t, entry); - const [validatedNestedEntry] = validate(t, entriesNested); + // NOTE: The temporary ids that are attached to the exception item entries for the + // benefit of React keys functionality do not get stripped here. Chose to leave the + // adding and stripping of the temporary ids to the api boundaries, the exception being + // in the builder where an id is added to the exception item during creation + const entries = exception.entries.filter((singleEntry) => { + if (singleEntry.type === 'nested') { + const strippedSingleEntry = removeIdFromItem({ + ...singleEntry, + entries: singleEntry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), + }); + const [validatedNestedEntry] = validate(strippedSingleEntry, entriesNested); - if (validatedEntry != null || validatedNestedEntry != null) { - return true; - } + if (validatedNestedEntry != null) { + return true; + } else { + return false; + } + } else { + const [validatedEntry] = validate(removeIdFromItem(singleEntry), entry); - return false; + if (validatedEntry != null) { + return true; + } else { + return false; + } + } }); const item = { ...exception, entries }; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts index 3593fe3d054910..e945f7067d9fe8 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts @@ -58,6 +58,7 @@ export interface ExceptionsPagination { } export interface FormattedBuilderEntry { + id: string; field: IFieldType | undefined; operator: OperatorOption; value: string | string[] | undefined; From 2320f62d9e03b80219dd1afb7b41d927a3c4fde1 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Mon, 8 Feb 2021 07:31:34 -0800 Subject: [PATCH 04/13] finicky types, attempting to get types right --- x-pack/plugins/lists/common/constants.mock.ts | 8 ++-- .../hooks/persist_exception_item.ts | 8 ++-- .../lists/public/exceptions/transforms.ts | 18 +++---- .../exceptions/builder/entry_item.test.tsx | 17 +++++++ .../exceptions/builder/helpers.test.tsx | 13 +++++ .../components/exceptions/builder/helpers.tsx | 48 ++++++++++--------- .../components/exceptions/helpers.test.tsx | 4 ++ .../common/components/exceptions/helpers.tsx | 11 ++--- .../common/components/exceptions/types.ts | 3 ++ 9 files changed, 81 insertions(+), 49 deletions(-) diff --git a/x-pack/plugins/lists/common/constants.mock.ts b/x-pack/plugins/lists/common/constants.mock.ts index 22a5b2d9cc55bc..27e0fa29b1e550 100644 --- a/x-pack/plugins/lists/common/constants.mock.ts +++ b/x-pack/plugins/lists/common/constants.mock.ts @@ -8,7 +8,7 @@ import moment from 'moment'; import { OsTypeArray } from './schemas/common'; -import { EntriesArray } from './schemas/types'; +import { EntriesArray, Entry, EntryMatch, EntryNested } from './schemas/types'; import { EndpointEntriesArray } from './schemas/types/endpoint'; export const DATE_NOW = '2020-04-20T15:25:31.830Z'; export const OLD_DATE_RELATIVE_TO_DATE_NOW = '2020-04-19T15:25:31.830Z'; @@ -86,19 +86,19 @@ export const ENTRIES_WITH_IDS: EntriesArray = [ operator: 'included', type: 'match', value: 'some value', - }, + } as EntryMatch & { id: string }, ], field: 'some.parentField', id: '123', type: 'nested', - }, + } as EntryNested & { id: string }, { field: 'some.not.nested.field', id: '123', operator: 'included', type: 'match', value: 'some value', - }, + } as Entry & { id: string }, ]; export const ITEM_TYPE = 'simple'; export const OS_TYPES: OsTypeArray = ['windows']; diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts index 3a7d0327003d20..627b9b56b749e7 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts @@ -48,12 +48,14 @@ export const usePersistExceptionItem = ({ if (exceptionListItem != null) { try { setIsLoading(true); - if (isUpdateExceptionItem(exceptionListItem)) { + const transformedList = transformOutput(exceptionListItem); + + if (isUpdateExceptionItem(transformedList)) { await updateExceptionListItem({ http, // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes // for context around the temporary `id` - listItem: transformOutput(exceptionListItem), + listItem: transformedList, signal: abortCtrl.signal, }); } else { @@ -61,7 +63,7 @@ export const usePersistExceptionItem = ({ http, // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes // for context around the temporary `id` - listItem: transformOutput(exceptionListItem), + listItem: transformedList, signal: abortCtrl.signal, }); } diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts index 91c2e3fb207607..809a0b471ed6a8 100644 --- a/x-pack/plugins/lists/public/exceptions/transforms.ts +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -9,11 +9,8 @@ import { flow } from 'fp-ts/lib/function'; import { CreateExceptionListItemSchema, + EntriesArray, Entry, - EntryExists, - EntryMatch, - EntryMatchAny, - EntryNested, ExceptionListItemSchema, UpdateExceptionListItemSchema, } from '../../common'; @@ -98,17 +95,16 @@ export const addIdToExceptionItemEntries = ( export const removeIdFromExceptionItemsEntries = ( exceptionItem: CreateExceptionListItemSchema | UpdateExceptionListItemSchema ): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => { - const entries = exceptionItem.entries.map((entry) => { + const { entries, ...itemInfo } = exceptionItem; + const entriesNoId = entries.map((entry) => { if (entry.type === 'nested') { - return removeIdFromItem({ - ...entry, - entries: entry.entries.map((nestedEntry) => - removeIdFromItem(nestedEntry) - ), + return removeIdFromItem({ + ...itemInfo, + entries: entry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), }); } else { return removeIdFromItem(entry); } }); - return { ...exceptionItem, entries }; + return { ...itemInfo, entries: entriesNoId as EntriesArray }; }; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx index da236d130e9309..9657bff37ecc1e 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/entry_item.test.tsx @@ -58,6 +58,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( ({ }); const getMockBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: getField('ip'), operator: isOperator, value: 'some value', @@ -64,6 +65,7 @@ const getMockBuilderEntry = (): FormattedBuilderEntry => ({ }); const getMockNestedBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: getField('nestedField.child'), operator: isOperator, value: 'some value', @@ -81,6 +83,7 @@ const getMockNestedBuilderEntry = (): FormattedBuilderEntry => ({ }); const getMockNestedParentBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: { ...getField('nestedField.child'), name: 'nestedField', esTypes: ['nested'] }, operator: isOperator, value: undefined, @@ -225,6 +228,7 @@ describe('Exception builder helpers', () => { test('it returns nested fields that match parent value when "item.nested" is "child"', () => { const payloadItem: FormattedBuilderEntry = { + id: '123', field: getEndpointField('file.Ext.code_signature.status'), operator: isOperator, value: 'some value', @@ -363,6 +367,7 @@ describe('Exception builder helpers', () => { undefined ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { name: 'machine.os.raw.text', @@ -399,6 +404,7 @@ describe('Exception builder helpers', () => { 1 ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { aggregatable: false, @@ -442,6 +448,7 @@ describe('Exception builder helpers', () => { undefined ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -486,6 +493,7 @@ describe('Exception builder helpers', () => { const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ { + id: '123', entryIndex: 0, field: undefined, nested: undefined, @@ -507,6 +515,7 @@ describe('Exception builder helpers', () => { const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -525,6 +534,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 1, field: { aggregatable: true, @@ -561,6 +571,7 @@ describe('Exception builder helpers', () => { const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -579,6 +590,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 1, field: { aggregatable: false, @@ -594,6 +606,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 0, field: { aggregatable: false, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index 1eae18c9f7c8bf..e51f6aa1ff197b 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -5,18 +5,20 @@ * 2.0. */ +import uuid from 'uuid'; + import { IIndexPattern, IFieldType } from '../../../../../../../../src/plugins/data/common'; import { Entry, OperatorTypeEnum, EntryNested, ExceptionListType, - EntryMatch, - EntryMatchAny, - EntryExists, entriesList, ListSchema, OperatorEnum, + EntryMatch, + EntryMatchAny, + EntryExists, } from '../../../../lists_plugin_deps'; import { isOperator, @@ -37,7 +39,6 @@ import { } from '../types'; import { getEntryValue, getExceptionOperatorSelect } from '../helpers'; import exceptionableFields from '../exceptionable_fields.json'; -import { addIdToItem } from '../../../../../common'; /** * Returns filtered index patterns based on the field - if a user selects to @@ -141,7 +142,7 @@ export const getCorrespondingKeywordField = ({ */ export const getFormattedBuilderEntry = ( indexPattern: IIndexPattern, - item: BuilderEntry, + item: BuilderEntry & { id?: string }, itemIndex: number, parent: EntryNested | undefined, parentIndex: number | undefined @@ -161,7 +162,7 @@ export const getFormattedBuilderEntry = ( ? { ...foundField, name: foundField.name.split('.').slice(-1)[0] } : foundField, correspondingKeywordField, - id: item.id, + id: (item as typeof item & { id?: string }).id ?? `${itemIndex}`, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), nested: 'child', @@ -171,7 +172,7 @@ export const getFormattedBuilderEntry = ( } else { return { field: foundField, - id: item.id, + id: (item as typeof item & { id?: string }).id ?? `${itemIndex}`, correspondingKeywordField, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), @@ -200,7 +201,7 @@ export const isEntryNested = (item: BuilderEntry): item is EntryNested => { */ export const getFormattedBuilderEntries = ( indexPattern: IIndexPattern, - entries: BuilderEntry[], + entries: Array, parent?: EntryNested, parentIndex?: number ): FormattedBuilderEntry[] => { @@ -218,7 +219,7 @@ export const getFormattedBuilderEntries = ( } else { const parentEntry: FormattedBuilderEntry = { operator: isOperator, - id: item.id, + id: (item as typeof item & { id?: string }).id ?? `${index}`, nested: 'parent', field: isNewNestedEntry ? undefined @@ -286,6 +287,7 @@ export const getUpdatedEntriesOnDelete = ( const { field } = itemOfInterest; const updatedItemOfInterest: EntryNested | EmptyNestedEntry = { field, + id: (itemOfInterest as typeof itemOfInterest & { id?: string }).id ?? `${entryIndex}`, type: OperatorTypeEnum.NESTED, entries: updatedEntryEntries, }; @@ -607,20 +609,20 @@ export const getEntryOnListChange = ( }; }; -export const getDefaultEmptyEntry = (): EmptyEntry => - addIdToItem({ - field: '', - type: OperatorTypeEnum.MATCH, - operator: OperatorEnum.INCLUDED, - value: '', - }); - -export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => - addIdToItem({ - field: '', - type: OperatorTypeEnum.NESTED, - entries: [], - }); +export const getDefaultEmptyEntry = (): EmptyEntry => ({ + id: uuid.v4(), + field: '', + type: OperatorTypeEnum.MATCH, + operator: OperatorEnum.INCLUDED, + value: '', +}); + +export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => ({ + id: uuid.v4(), + field: '', + type: OperatorTypeEnum.NESTED, + entries: [], +}); export const containsValueListEntry = (items: ExceptionsBuilderExceptionItem[]): boolean => items.some((item) => item.entries.some((entry) => entry.type === OperatorTypeEnum.LIST)); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx index 8b8faa0d75d4c7..e8ab4aa8d0ca11 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.test.tsx @@ -251,6 +251,7 @@ describe('Exception helpers', () => { test('it removes entry items with "value" of "undefined"', () => { const { entries, ...rest } = getExceptionListItemSchemaMock(); const mockEmptyException: EmptyEntry = { + id: '123', field: 'host.name', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, @@ -269,6 +270,7 @@ describe('Exception helpers', () => { test('it removes "match" entry items with "value" of empty string', () => { const { entries, ...rest } = { ...getExceptionListItemSchemaMock() }; const mockEmptyException: EmptyEntry = { + id: '123', field: 'host.name', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, @@ -289,6 +291,7 @@ describe('Exception helpers', () => { test('it removes "match" entry items with "field" of empty string', () => { const { entries, ...rest } = { ...getExceptionListItemSchemaMock() }; const mockEmptyException: EmptyEntry = { + id: '123', field: '', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, @@ -309,6 +312,7 @@ describe('Exception helpers', () => { test('it removes "match_any" entry items with "field" of empty string', () => { const { entries, ...rest } = { ...getExceptionListItemSchemaMock() }; const mockEmptyException: EmptyEntry = { + id: '123', field: '', type: OperatorTypeEnum.MATCH_ANY, operator: OperatorEnum.INCLUDED, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx index 4fb838d4b46c1d..31a1328f21be60 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx @@ -36,9 +36,6 @@ import { UpdateExceptionListItemSchema, EntryNested, OsTypeArray, - EntryExists, - EntryMatch, - EntryMatchAny, } from '../../../shared_imports'; import { IIndexPattern } from '../../../../../../../src/plugins/data/common'; import { validate } from '../../../../common/validate'; @@ -181,11 +178,9 @@ export const filterExceptionItems = ( // adding and stripping of the temporary ids to the api boundaries, the exception being // in the builder where an id is added to the exception item during creation const entries = exception.entries.filter((singleEntry) => { + const strippedSingleEntry = removeIdFromItem(singleEntry); + if (singleEntry.type === 'nested') { - const strippedSingleEntry = removeIdFromItem({ - ...singleEntry, - entries: singleEntry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), - }); const [validatedNestedEntry] = validate(strippedSingleEntry, entriesNested); if (validatedNestedEntry != null) { @@ -194,7 +189,7 @@ export const filterExceptionItems = ( return false; } } else { - const [validatedEntry] = validate(removeIdFromItem(singleEntry), entry); + const [validatedEntry] = validate(strippedSingleEntry, entry); if (validatedEntry != null) { return true; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts index e945f7067d9fe8..621d0e5ac06866 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts @@ -69,6 +69,7 @@ export interface FormattedBuilderEntry { } export interface EmptyEntry { + id: string; field: string | undefined; operator: OperatorEnum; type: OperatorTypeEnum.MATCH | OperatorTypeEnum.MATCH_ANY; @@ -76,6 +77,7 @@ export interface EmptyEntry { } export interface EmptyListEntry { + id: string; field: string | undefined; operator: OperatorEnum; type: OperatorTypeEnum.LIST; @@ -83,6 +85,7 @@ export interface EmptyListEntry { } export interface EmptyNestedEntry { + id: string; field: string | undefined; type: OperatorTypeEnum.NESTED; entries: Array; From a76e1b2ad383c7fd23b1e9901c073d2fc0558820 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 11 Feb 2021 15:11:17 -0800 Subject: [PATCH 05/13] updating to work with nested --- .../exceptions/exceptions_modal.spec.ts | 119 ++++++++++ .../cypress/screens/exceptions.ts | 14 ++ .../cypress/tasks/exceptions.ts | 48 ++++ .../cypress/tasks/rule_details.ts | 6 + .../exceptions/add_exception_modal/index.tsx | 4 +- .../exceptions/builder/entry_item.test.tsx | 9 +- .../exceptions/builder/helpers.test.tsx | 208 +++++++++++++----- .../components/exceptions/builder/helpers.tsx | 35 ++- .../components/exceptions/builder/index.tsx | 2 +- .../common/components/exceptions/helpers.tsx | 10 +- .../auditbeat_for_exceptions/mappings.json | 8 + .../es_archives/exceptions/data.json | 23 ++ .../es_archives/exceptions/mappings.json | 37 ++++ 13 files changed, 447 insertions(+), 76 deletions(-) create mode 100644 x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts create mode 100644 x-pack/plugins/security_solution/cypress/tasks/exceptions.ts create mode 100644 x-pack/test/security_solution_cypress/es_archives/exceptions/data.json create mode 100644 x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json diff --git a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts new file mode 100644 index 00000000000000..13a427e8f8e65c --- /dev/null +++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts @@ -0,0 +1,119 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { newRule } from '../../objects/rule'; + +import { RULE_STATUS } from '../../screens/create_new_rule'; + +import { goToManageAlertsDetectionRules, waitForAlertsIndexToBeCreated } from '../../tasks/alerts'; +import { createCustomRule } from '../../tasks/api_calls/rules'; +import { goToRuleDetails } from '../../tasks/alerts_detection_rules'; +import { esArchiverLoad, esArchiverUnload } from '../../tasks/es_archiver'; +import { loginAndWaitForPageWithoutDateRange } from '../../tasks/login'; +import { openExceptionModalFromRuleSettings, goToExceptionsTab } from '../../tasks/rule_details'; +import { addExceptionEntryFieldValue, closeExceptionBuilderModal } from '../../tasks/exceptions'; +import { + ADD_AND_BTN, + ADD_OR_BTN, + ADD_NESTED_BTN, + ENTRY_DELETE_BTN, + FIELD_INPUT, + LOADING_SPINNER, +} from '../../screens/exceptions'; + +import { DETECTIONS_URL } from '../../urls/navigation'; +import { cleanKibana } from '../../tasks/common'; + +describe('Exceptions modal', () => { + before(() => { + cleanKibana(); + loginAndWaitForPageWithoutDateRange(DETECTIONS_URL); + waitForAlertsIndexToBeCreated(); + createCustomRule(newRule); + goToManageAlertsDetectionRules(); + goToRuleDetails(); + + cy.get(RULE_STATUS).should('have.text', '—'); + + esArchiverLoad('auditbeat_for_exceptions'); + + goToExceptionsTab(); + }); + + after(() => { + esArchiverUnload('auditbeat_for_exceptions'); + }); + + it('Does not overwrite invalid values and-ed together', () => { + openExceptionModalFromRuleSettings(); + + // add multiple entries with invalid field values + addExceptionEntryFieldValue('a', 0); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('b', 1); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('c', 2); + + // delete second item, invalid values 'a' and 'c' should remain + cy.get(ENTRY_DELETE_BTN).eq(1).click(); + cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); + cy.get(FIELD_INPUT).eq(1).should('have.text', 'c'); + + closeExceptionBuilderModal(); + }); + + it('Does not overwrite invalid values or-ed together', () => { + openExceptionModalFromRuleSettings(); + cy.get(LOADING_SPINNER).should('not.exist'); + + // exception item 1 + addExceptionEntryFieldValue('a', 0); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('b', 1); + + // exception item 2 + cy.get(ADD_OR_BTN).click(); + addExceptionEntryFieldValue('c', 2); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('d', 3); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('e', 4); + + // delete single entry from exception item 2 + cy.get(ENTRY_DELETE_BTN).eq(3).click(); + cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); + cy.get(FIELD_INPUT).eq(1).should('have.text', 'b'); + cy.get(FIELD_INPUT).eq(2).should('have.text', 'c'); + cy.get(FIELD_INPUT).eq(3).should('have.text', 'e'); + + // delete remaining entries in exception item 2 + cy.get(ENTRY_DELETE_BTN).eq(2).click(); + cy.get(ENTRY_DELETE_BTN).eq(2).click(); + cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); + cy.get(FIELD_INPUT).eq(1).should('have.text', 'b'); + cy.get(FIELD_INPUT).eq(2).should('not.exist'); + + closeExceptionBuilderModal(); + }); + + it('Does not overwrite invalid values of nested entry items', () => { + openExceptionModalFromRuleSettings(); + cy.get(LOADING_SPINNER).should('not.exist'); + + // exception item 1 + addExceptionEntryFieldValue('a', 0); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValue('b', 1); + + // exception item 2 with nested field + cy.get(ADD_OR_BTN).click(); + addExceptionEntryFieldValue('c', 2); + cy.get(ADD_NESTED_BTN).click(); + + // closeExceptionBuilderModal(); + }); +}); diff --git a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts index dbd55a293f6a0a..705d7450e2d8a4 100644 --- a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts +++ b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts @@ -23,3 +23,17 @@ export const OPERATOR_INPUT = '[data-test-subj="operatorAutocompleteComboBox"]'; export const VALUES_INPUT = '[data-test-subj="valuesAutocompleteMatch"] [data-test-subj="comboBoxInput"]'; + +export const ADD_AND_BTN = '[data-test-subj="exceptionsAndButton"]'; + +export const ADD_OR_BTN = '[data-test-subj="exceptionsOrButton"]'; + +export const ADD_NESTED_BTN = '[data-test-subj="exceptionsNestedButton"]'; + +export const ENTRY_DELETE_BTN = '[data-test-subj="builderItemEntryDeleteButton"]'; + +export const FIELD_INPUT_LIST_BTN = '[data-test-subj="comboBoxToggleListButton"]'; + +export const CANCEL_BTN = '[data-test-subj="cancelExceptionAddButton"]'; + +export const BUILDER_MODAL_BODY = '[data-test-subj="exceptionsBuilderWrapper"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts b/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts new file mode 100644 index 00000000000000..bb87ff2a94f161 --- /dev/null +++ b/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts @@ -0,0 +1,48 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Exception } from '../objects/exception'; +import { + FIELD_INPUT, + OPERATOR_INPUT, + VALUES_INPUT, + CANCEL_BTN, + BUILDER_MODAL_BODY, +} from '../screens/exceptions'; + +export const addExceptionEntryFieldValue = (field: string, index = 0) => { + cy.get(FIELD_INPUT).eq(index).type(`${field}{enter}`); + cy.get(BUILDER_MODAL_BODY).click(); +}; + +export const addExceptionEntryOperatorValue = (operator: string, index = 0) => { + cy.get(OPERATOR_INPUT).eq(index).type(`${operator}{enter}`); + cy.get(BUILDER_MODAL_BODY).click(); +}; + +export const addExceptionEntryValue = (values: string[], index = 0) => { + values.forEach((value) => { + cy.get(VALUES_INPUT).eq(index).type(`${value}{enter}`); + }); + cy.get(BUILDER_MODAL_BODY).click(); +}; + +export const addExceptionEntry = (exception: Exception, index = 0) => { + addExceptionEntryFieldValue(exception.field, index); + addExceptionEntryOperatorValue(exception.operator, index); + addExceptionEntryValue(exception.values, index); +}; + +export const addNestedExceptionEntry = (exception: Exception, index = 0) => { + addExceptionEntryFieldValue(exception.field, index); + addExceptionEntryOperatorValue(exception.operator, index); + addExceptionEntryValue(exception.values, index); +}; + +export const closeExceptionBuilderModal = () => { + cy.get(CANCEL_BTN).click(); +}; diff --git a/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts b/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts index 06c4fb572650b9..945c226cc5b4cf 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts @@ -53,6 +53,12 @@ export const addsException = (exception: Exception) => { cy.get(CONFIRM_BTN).should('not.exist'); }; +export const openExceptionModalFromRuleSettings = () => { + cy.get(ADD_EXCEPTIONS_BTN).click(); + cy.get(LOADING_SPINNER).should('not.exist'); + cy.get(FIELD_INPUT).should('be.visible'); +}; + export const addsExceptionFromRuleSettings = (exception: Exception) => { cy.get(ADD_EXCEPTIONS_BTN).click(); cy.get(LOADING_SPINNER).should('exist'); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index dc7388438c012a..eca56fce080d12 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -461,7 +461,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({ )} {fetchOrCreateListError == null && ( - {i18n.CANCEL} + + {i18n.CANCEL} + { }).onChange([{ label: 'machine.os' }]); expect(mockOnChange).toHaveBeenCalledWith( - { field: 'machine.os', operator: 'included', type: 'match', value: '' }, + { id: '123', field: 'machine.os', operator: 'included', type: 'match', value: '' }, 0 ); }); @@ -482,7 +482,7 @@ describe('BuilderEntryItem', () => { }).onChange([{ label: 'is not' }]); expect(mockOnChange).toHaveBeenCalledWith( - { field: 'ip', operator: 'excluded', type: 'match', value: '1234' }, + { id: '123', field: 'ip', operator: 'excluded', type: 'match', value: '1234' }, 0 ); }); @@ -518,7 +518,7 @@ describe('BuilderEntryItem', () => { }).onCreateOption('126.45.211.34'); expect(mockOnChange).toHaveBeenCalledWith( - { field: 'ip', operator: 'excluded', type: 'match', value: '126.45.211.34' }, + { id: '123', field: 'ip', operator: 'excluded', type: 'match', value: '126.45.211.34' }, 0 ); }); @@ -554,7 +554,7 @@ describe('BuilderEntryItem', () => { }).onCreateOption('126.45.211.34'); expect(mockOnChange).toHaveBeenCalledWith( - { field: 'ip', operator: 'included', type: 'match_any', value: ['126.45.211.34'] }, + { id: '123', field: 'ip', operator: 'included', type: 'match_any', value: ['126.45.211.34'] }, 0 ); }); @@ -591,6 +591,7 @@ describe('BuilderEntryItem', () => { expect(mockOnChange).toHaveBeenCalledWith( { + id: '123', field: 'ip', operator: 'excluded', type: 'list', diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx index 551dda7e20f1d6..45820b5c2b7026 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx @@ -28,7 +28,7 @@ import { } from '../../autocomplete/operators'; import { BuilderEntry, ExceptionsBuilderExceptionItem, FormattedBuilderEntry } from '../types'; import { IFieldType, IIndexPattern } from '../../../../../../../../src/plugins/data/common'; -import { Entry, EntryNested } from '../../../../lists_plugin_deps'; +import { Entry, EntryMatch, EntryNested } from '../../../../lists_plugin_deps'; import { getEntryFromOperator, @@ -46,6 +46,31 @@ import { getCorrespondingKeywordField, } from './helpers'; import { OperatorOption } from '../../autocomplete/types'; +import { ENTRIES_WITH_IDS } from '../../../../../../lists/common/constants.mock'; + +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + +const getEntryNestedWithIdMock = () => ({ + id: '123', + ...getEntryNestedMock(), +}); + +const getEntryExistsWithIdMock = () => ({ + id: '123', + ...getEntryExistsMock(), +}); + +const getEntryMatchWithIdMock = () => ({ + id: '123', + ...getEntryMatchMock(), +}); + +const getEntryMatchAnyWithIdMock = () => ({ + id: '123', + ...getEntryMatchAnyMock(), +}); const getMockIndexPattern = (): IIndexPattern => ({ id: '1234', @@ -72,9 +97,9 @@ const getMockNestedBuilderEntry = (): FormattedBuilderEntry => ({ nested: 'child', parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }, parentIndex: 0, }, @@ -235,9 +260,9 @@ describe('Exception builder helpers', () => { nested: 'child', parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'file.Ext.code_signature', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }, parentIndex: 0, }, @@ -355,7 +380,7 @@ describe('Exception builder helpers', () => { ], }; const payloadItem: BuilderEntry = { - ...getEntryMatchMock(), + ...getEntryMatchWithIdMock(), field: 'machine.os.raw.text', value: 'some os', }; @@ -390,11 +415,11 @@ describe('Exception builder helpers', () => { test('it returns "FormattedBuilderEntry" with value "nested" of "child" when "parent" and "parentIndex" are defined', () => { const payloadIndexPattern: IIndexPattern = getMockIndexPattern(); - const payloadItem: BuilderEntry = { ...getEntryMatchMock(), field: 'child' }; + const payloadItem: BuilderEntry = { ...getEntryMatchWithIdMock(), field: 'child' }; const payloadParent: EntryNested = { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }; const output = getFormattedBuilderEntry( payloadIndexPattern, @@ -425,10 +450,11 @@ describe('Exception builder helpers', () => { operator: isOperator, parent: { parent: { + id: '123', entries: [{ ...payloadItem }], field: 'nestedField', type: 'nested', - }, + } as EntryNested & { id: string }, parentIndex: 1, }, value: 'some host name', @@ -439,7 +465,11 @@ describe('Exception builder helpers', () => { test('it returns non nested "FormattedBuilderEntry" when "parent" and "parentIndex" are not defined', () => { const payloadIndexPattern: IIndexPattern = getMockIndexPattern(); - const payloadItem: BuilderEntry = { ...getEntryMatchMock(), field: 'ip', value: 'some ip' }; + const payloadItem: BuilderEntry = { + ...getEntryMatchWithIdMock(), + field: 'ip', + value: 'some ip', + }; const output = getFormattedBuilderEntry( payloadIndexPattern, payloadItem, @@ -472,14 +502,14 @@ describe('Exception builder helpers', () => { describe('#isEntryNested', () => { test('it returns "false" if payload is not of type EntryNested', () => { - const payload: BuilderEntry = getEntryMatchMock(); + const payload: BuilderEntry = getEntryMatchWithIdMock(); const output = isEntryNested(payload); const expected = false; expect(output).toEqual(expected); }); test('it returns "true if payload is of type EntryNested', () => { - const payload: EntryNested = getEntryNestedMock(); + const payload: EntryNested = getEntryNestedWithIdMock(); const output = isEntryNested(payload); const expected = true; expect(output).toEqual(expected); @@ -489,7 +519,7 @@ describe('Exception builder helpers', () => { describe('#getFormattedBuilderEntries', () => { test('it returns formatted entry with field undefined if it unable to find a matching index pattern field', () => { const payloadIndexPattern: IIndexPattern = getMockIndexPattern(); - const payloadItems: BuilderEntry[] = [getEntryMatchMock()]; + const payloadItems: BuilderEntry[] = [getEntryMatchWithIdMock()]; const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ { @@ -509,8 +539,8 @@ describe('Exception builder helpers', () => { test('it returns formatted entries when no nested entries exist', () => { const payloadIndexPattern: IIndexPattern = getMockIndexPattern(); const payloadItems: BuilderEntry[] = [ - { ...getEntryMatchMock(), field: 'ip', value: 'some ip' }, - { ...getEntryMatchAnyMock(), field: 'extension', value: ['some extension'] }, + { ...getEntryMatchWithIdMock(), field: 'ip', value: 'some ip' }, + { ...getEntryMatchAnyWithIdMock(), field: 'extension', value: ['some extension'] }, ]; const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ @@ -559,12 +589,12 @@ describe('Exception builder helpers', () => { test('it returns formatted entries when nested entries exist', () => { const payloadIndexPattern: IIndexPattern = getMockIndexPattern(); const payloadParent: EntryNested = { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }; const payloadItems: BuilderEntry[] = [ - { ...getEntryMatchMock(), field: 'ip', value: 'some ip' }, + { ...getEntryMatchWithIdMock(), field: 'ip', value: 'some ip' }, { ...payloadParent }, ]; @@ -627,17 +657,19 @@ describe('Exception builder helpers', () => { operator: isOperator, parent: { parent: { + id: '123', entries: [ { + id: '123', field: 'child', operator: 'included', type: 'match', value: 'some host name', - }, + } as EntryMatch & { id: string }, ], field: 'nestedField', type: 'nested', - }, + } as EntryNested & { id: string }, parentIndex: 1, }, value: 'some host name', @@ -650,12 +682,16 @@ describe('Exception builder helpers', () => { describe('#getUpdatedEntriesOnDelete', () => { test('it removes entry corresponding to "entryIndex"', () => { - const payloadItem: ExceptionsBuilderExceptionItem = { ...getExceptionListItemSchemaMock() }; + const payloadItem: ExceptionsBuilderExceptionItem = { + ...getExceptionListItemSchemaMock(), + entries: ENTRIES_WITH_IDS, + }; const output = getUpdatedEntriesOnDelete(payloadItem, 0, null); const expected: ExceptionsBuilderExceptionItem = { ...getExceptionListItemSchemaMock(), entries: [ { + id: '123', field: 'some.not.nested.field', operator: 'included', type: 'match', @@ -671,15 +707,17 @@ describe('Exception builder helpers', () => { ...getExceptionListItemSchemaMock(), entries: [ { - ...getEntryNestedMock(), - entries: [{ ...getEntryExistsMock() }, { ...getEntryMatchAnyMock() }], + ...getEntryNestedWithIdMock(), + entries: [{ ...getEntryExistsWithIdMock() }, { ...getEntryMatchAnyWithIdMock() }], }, ], }; const output = getUpdatedEntriesOnDelete(payloadItem, 0, 0); const expected: ExceptionsBuilderExceptionItem = { ...getExceptionListItemSchemaMock(), - entries: [{ ...getEntryNestedMock(), entries: [{ ...getEntryMatchAnyMock() }] }], + entries: [ + { ...getEntryNestedWithIdMock(), entries: [{ ...getEntryMatchAnyWithIdMock() }] }, + ], }; expect(output).toEqual(expected); }); @@ -689,8 +727,8 @@ describe('Exception builder helpers', () => { ...getExceptionListItemSchemaMock(), entries: [ { - ...getEntryNestedMock(), - entries: [{ ...getEntryExistsMock() }], + ...getEntryNestedWithIdMock(), + entries: [{ ...getEntryExistsWithIdMock() }], }, ], }; @@ -712,6 +750,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'excluded', type: 'match', @@ -729,6 +768,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'match', @@ -746,6 +786,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'match', @@ -763,6 +804,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'excluded', type: 'match_any', @@ -780,6 +822,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'match_any', @@ -797,6 +840,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'match_any', @@ -813,6 +857,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'excluded', type: 'exists', @@ -828,6 +873,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'exists', @@ -844,6 +890,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'exists', @@ -860,6 +907,7 @@ describe('Exception builder helpers', () => { }; const output = getEntryFromOperator(payloadOperator, payloadEntry); const expected: Entry = { + id: '123', field: 'ip', operator: 'included', type: 'list', @@ -956,10 +1004,11 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedParentBuilderEntry(); const payloadIFieldType: IFieldType = getField('nestedField.child'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { - entries: [{ field: 'child', operator: 'included', type: 'match', value: '' }], + id: '123', + entries: [{ id: '123', field: 'child', operator: 'included', type: 'match', value: '' }], field: 'nestedField', type: 'nested', }, @@ -972,21 +1021,25 @@ describe('Exception builder helpers', () => { ...getMockNestedBuilderEntry(), parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchMock(), field: 'child' }, getEntryMatchAnyMock()], + entries: [ + { ...getEntryMatchWithIdMock(), field: 'child' }, + getEntryMatchAnyWithIdMock(), + ], }, parentIndex: 0, }, }; const payloadIFieldType: IFieldType = getField('nestedField.child'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ - { field: 'child', operator: 'included', type: 'match', value: '' }, - getEntryMatchAnyMock(), + { id: '123', field: 'child', operator: 'included', type: 'match', value: '' }, + getEntryMatchAnyWithIdMock(), ], field: 'nestedField', type: 'nested', @@ -999,9 +1052,10 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadIFieldType: IFieldType = getField('ip'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', field: 'ip', operator: 'included', type: 'match', @@ -1017,8 +1071,14 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadOperator: OperatorOption = isNotOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry; index: number } = { - updatedEntry: { field: 'ip', type: 'match', value: 'some value', operator: 'excluded' }, + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + updatedEntry: { + id: '123', + field: 'ip', + type: 'match', + value: 'some value', + operator: 'excluded', + }, index: 0, }; expect(output).toEqual(expected); @@ -1028,8 +1088,14 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadOperator: OperatorOption = isOneOfOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry; index: number } = { - updatedEntry: { field: 'ip', type: 'match_any', value: [], operator: 'included' }, + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + updatedEntry: { + id: '123', + field: 'ip', + type: 'match_any', + value: [], + operator: 'included', + }, index: 0, }; expect(output).toEqual(expected); @@ -1039,11 +1105,13 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedBuilderEntry(); const payloadOperator: OperatorOption = isNotOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: 'child', operator: 'excluded', type: 'match', @@ -1061,11 +1129,13 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedBuilderEntry(); const payloadOperator: OperatorOption = isOneOfOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: 'child', operator: 'included', type: 'match_any', @@ -1084,8 +1154,14 @@ describe('Exception builder helpers', () => { test('it returns entry with updated value', () => { const payload: FormattedBuilderEntry = getMockBuilderEntry(); const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry; index: number } = { - updatedEntry: { field: 'ip', type: 'match', value: 'jibber jabber', operator: 'included' }, + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + updatedEntry: { + id: '123', + field: 'ip', + type: 'match', + value: 'jibber jabber', + operator: 'included', + }, index: 0, }; expect(output).toEqual(expected); @@ -1094,8 +1170,14 @@ describe('Exception builder helpers', () => { test('it returns entry with updated value and "field" of empty string if entry does not have a "field" defined', () => { const payload: FormattedBuilderEntry = { ...getMockBuilderEntry(), field: undefined }; const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry; index: number } = { - updatedEntry: { field: '', type: 'match', value: 'jibber jabber', operator: 'included' }, + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + updatedEntry: { + id: '123', + field: '', + type: 'match', + value: 'jibber jabber', + operator: 'included', + }, index: 0, }; expect(output).toEqual(expected); @@ -1104,11 +1186,13 @@ describe('Exception builder helpers', () => { test('it returns nested entry with updated value', () => { const payload: FormattedBuilderEntry = getMockNestedBuilderEntry(); const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: 'child', operator: 'included', type: 'match', @@ -1125,11 +1209,13 @@ describe('Exception builder helpers', () => { test('it returns nested entry with updated value and "field" of empty string if entry does not have a "field" defined', () => { const payload: FormattedBuilderEntry = { ...getMockNestedBuilderEntry(), field: undefined }; const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: '', operator: 'included', type: 'match', @@ -1152,8 +1238,9 @@ describe('Exception builder helpers', () => { value: ['some value'], }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { updatedEntry: { + id: '123', field: 'ip', type: 'match_any', value: ['jibber jabber'], @@ -1172,8 +1259,9 @@ describe('Exception builder helpers', () => { field: undefined, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { updatedEntry: { + id: '123', field: '', type: 'match_any', value: ['jibber jabber'], @@ -1189,19 +1277,21 @@ describe('Exception builder helpers', () => { ...getMockNestedBuilderEntry(), parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchAnyMock(), field: 'child' }], + entries: [{ ...getEntryMatchAnyWithIdMock(), field: 'child' }], }, parentIndex: 0, }, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: 'child', operator: 'included', type: 'match_any', @@ -1221,19 +1311,21 @@ describe('Exception builder helpers', () => { field: undefined, parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchAnyMock(), field: 'child' }], + entries: [{ ...getEntryMatchAnyWithIdMock(), field: 'child' }], }, parentIndex: 0, }, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { index: 0, updatedEntry: { + id: '123', entries: [ { + id: '123', field: '', operator: 'included', type: 'match_any', @@ -1256,8 +1348,9 @@ describe('Exception builder helpers', () => { value: '1234', }; const output = getEntryOnListChange(payload, getListResponseMock()); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { updatedEntry: { + id: '123', field: 'ip', type: 'list', list: { id: 'some-list-id', type: 'ip' }, @@ -1276,8 +1369,9 @@ describe('Exception builder helpers', () => { field: undefined, }; const output = getEntryOnListChange(payload, getListResponseMock()); - const expected: { updatedEntry: BuilderEntry; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { updatedEntry: { + id: '123', field: '', type: 'list', list: { id: 'some-list-id', type: 'ip' }, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index e51f6aa1ff197b..806049f04a4f44 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -7,6 +7,7 @@ import uuid from 'uuid'; +import { addIdToItem } from '../../../../../common/add_remove_id_to_item'; import { IIndexPattern, IFieldType } from '../../../../../../../../src/plugins/data/common'; import { Entry, @@ -323,12 +324,13 @@ export const getUpdatedEntriesOnDelete = ( export const getEntryFromOperator = ( selectedOperator: OperatorOption, currentEntry: FormattedBuilderEntry -): Entry => { +): Entry & { id: string } => { const isSameOperatorType = currentEntry.operator.type === selectedOperator.type; const fieldValue = currentEntry.field != null ? currentEntry.field.name : ''; switch (selectedOperator.type) { case 'match': return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.MATCH, operator: selectedOperator.operator, @@ -337,6 +339,7 @@ export const getEntryFromOperator = ( }; case 'match_any': return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.MATCH_ANY, operator: selectedOperator.operator, @@ -344,6 +347,7 @@ export const getEntryFromOperator = ( }; case 'list': return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.LIST, operator: selectedOperator.operator, @@ -351,6 +355,7 @@ export const getEntryFromOperator = ( }; default: return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.EXISTS, operator: selectedOperator.operator, @@ -397,7 +402,7 @@ export const getOperatorOptions = ( export const getEntryOnFieldChange = ( item: FormattedBuilderEntry, newField: IFieldType -): { updatedEntry: BuilderEntry; index: number } => { +): { updatedEntry: BuilderEntry & { id: string }; index: number } => { const { parent, entryIndex, nested } = item; const newChildFieldValue = newField != null ? newField.name.split('.').slice(-1)[0] : ''; @@ -414,15 +419,16 @@ export const getEntryOnFieldChange = ( return { updatedEntry: { + id: item.id, field: newParentFieldValue, type: OperatorTypeEnum.NESTED, entries: [ - { + addIdToItem({ field: newChildFieldValue ?? '', type: OperatorTypeEnum.MATCH, operator: isOperator.operator, value: '', - }, + }), ], }, index: entryIndex, @@ -434,11 +440,12 @@ export const getEntryOnFieldChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: newChildFieldValue ?? '', type: OperatorTypeEnum.MATCH, operator: isOperator.operator, value: '', - }, + } as EntryMatch & { id: string }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -447,6 +454,7 @@ export const getEntryOnFieldChange = ( } else { return { updatedEntry: { + id: item.id, field: newField != null ? newField.name : '', type: OperatorTypeEnum.MATCH, operator: isOperator.operator, @@ -467,7 +475,7 @@ export const getEntryOnFieldChange = ( export const getEntryOnOperatorChange = ( item: FormattedBuilderEntry, newOperator: OperatorOption -): { updatedEntry: BuilderEntry; index: number } => { +): { updatedEntry: BuilderEntry & { id: string }; index: number } => { const { parent, entryIndex, field, nested } = item; const newEntry = getEntryFromOperator(newOperator, item); @@ -502,7 +510,7 @@ export const getEntryOnOperatorChange = ( export const getEntryOnMatchChange = ( item: FormattedBuilderEntry, newField: string -): { updatedEntry: BuilderEntry; index: number } => { +): { updatedEntry: BuilderEntry & { id: string }; index: number } => { const { nested, parent, entryIndex, field, operator } = item; if (nested != null && parent != null) { @@ -514,11 +522,12 @@ export const getEntryOnMatchChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: fieldName, type: OperatorTypeEnum.MATCH, operator: operator.operator, value: newField, - }, + } as EntryMatch & { id: string }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -527,6 +536,7 @@ export const getEntryOnMatchChange = ( } else { return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.MATCH, operator: operator.operator, @@ -548,7 +558,7 @@ export const getEntryOnMatchChange = ( export const getEntryOnMatchAnyChange = ( item: FormattedBuilderEntry, newField: string[] -): { updatedEntry: BuilderEntry; index: number } => { +): { updatedEntry: BuilderEntry & { id: string }; index: number } => { const { nested, parent, entryIndex, field, operator } = item; if (nested != null && parent != null) { @@ -560,11 +570,12 @@ export const getEntryOnMatchAnyChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: fieldName, type: OperatorTypeEnum.MATCH_ANY, operator: operator.operator, value: newField, - }, + } as EntryMatchAny & { id: string }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -573,6 +584,7 @@ export const getEntryOnMatchAnyChange = ( } else { return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.MATCH_ANY, operator: operator.operator, @@ -594,12 +606,13 @@ export const getEntryOnMatchAnyChange = ( export const getEntryOnListChange = ( item: FormattedBuilderEntry, newField: ListSchema -): { updatedEntry: BuilderEntry; index: number } => { +): { updatedEntry: BuilderEntry & { id: string }; index: number } => { const { entryIndex, field, operator } = item; const { id, type } = newField; return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.LIST, operator: operator.operator, diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx index a77f355af11b46..3789d8e75fa2e7 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/index.tsx @@ -351,7 +351,7 @@ export const ExceptionBuilderComponent = ({ }, []); return ( - + {exceptions.map((exceptionListItem, index) => ( diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx index 31a1328f21be60..e1717c634ec2a9 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx @@ -180,8 +180,14 @@ export const filterExceptionItems = ( const entries = exception.entries.filter((singleEntry) => { const strippedSingleEntry = removeIdFromItem(singleEntry); - if (singleEntry.type === 'nested') { - const [validatedNestedEntry] = validate(strippedSingleEntry, entriesNested); + if (entriesNested.is(strippedSingleEntry)) { + const strippedNestedEntries = strippedSingleEntry.entries.map((nestedEntry) => + removeIdFromItem(nestedEntry) + ); + const [validatedNestedEntry] = validate( + { ...strippedSingleEntry, entries: strippedNestedEntries }, + entriesNested + ); if (validatedNestedEntry != null) { return true; diff --git a/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json b/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json index 4e5c6e9955310d..3e0c2a66a298c2 100644 --- a/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json +++ b/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json @@ -2279,6 +2279,14 @@ } } }, + "nestedField": { + "type": "nested", + "properties": { + "child": { + "type": "keyword" + } + } + }, "related": { "properties": { "ip": { diff --git a/x-pack/test/security_solution_cypress/es_archives/exceptions/data.json b/x-pack/test/security_solution_cypress/es_archives/exceptions/data.json new file mode 100644 index 00000000000000..b7de2dba02d196 --- /dev/null +++ b/x-pack/test/security_solution_cypress/es_archives/exceptions/data.json @@ -0,0 +1,23 @@ +{ + "type": "doc", + "value": { + "id": "_aZE5nwBOpWiDweSth_D", + "index": "exceptions-0001", + "source": { + "@timestamp": "2019-09-01T00:41:06.527Z", + "agent": { + "name": "bond" + }, + "user" : [ + { + "name" : "john", + "id" : "c5baec68-e774-46dc-b728-417e71d68444" + }, + { + "name" : "alice", + "id" : "6e831997-deab-4e56-9218-a90ef045556e" + } + ] + } + } +} diff --git a/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json b/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json new file mode 100644 index 00000000000000..477f43d7759a81 --- /dev/null +++ b/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json @@ -0,0 +1,37 @@ +{ + "type": "index", + "value": { + "aliases": { + "exceptions": { + "is_write_index": false + } + }, + "index": "exceptions-0001", + "mappings": { + "properties": { + "@timestamp": { + "type": "date" + }, + "agent": { + "properties": { + "name": { + "ignore_above": 1024, + "type": "keyword" + } + } + }, + "user": { + "type": "nested", + "properties": { + "first": { + "type": "keyword" + }, + "last": { + "type": "keyword" + } + } + } + } + } + } +} From 5c6b7a8ffe680f1065b2725c197d84417f7b2e30 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 11 Feb 2021 17:49:00 -0800 Subject: [PATCH 06/13] updated types to remove so many castings --- .../hooks/persist_exception_item.ts | 6 +- .../exceptions/exceptions_modal.spec.ts | 4 +- .../exceptions/builder/helpers.test.tsx | 196 ++++++++++-------- .../components/exceptions/builder/helpers.tsx | 27 ++- .../common/components/exceptions/types.ts | 24 ++- .../auditbeat_for_exceptions/mappings.json | 8 - 6 files changed, 146 insertions(+), 119 deletions(-) diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts index 627b9b56b749e7..f08d53d83ca505 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts @@ -48,21 +48,19 @@ export const usePersistExceptionItem = ({ if (exceptionListItem != null) { try { setIsLoading(true); + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` const transformedList = transformOutput(exceptionListItem); if (isUpdateExceptionItem(transformedList)) { await updateExceptionListItem({ http, - // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes - // for context around the temporary `id` listItem: transformedList, signal: abortCtrl.signal, }); } else { await addExceptionListItem({ http, - // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes - // for context around the temporary `id` listItem: transformedList, signal: abortCtrl.signal, }); diff --git a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts index 13a427e8f8e65c..4c7a9e6519a7f0 100644 --- a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts @@ -39,13 +39,13 @@ describe('Exceptions modal', () => { cy.get(RULE_STATUS).should('have.text', '—'); - esArchiverLoad('auditbeat_for_exceptions'); + esArchiverLoad('exceptions'); goToExceptionsTab(); }); after(() => { - esArchiverUnload('auditbeat_for_exceptions'); + esArchiverUnload('exceptions'); }); it('Does not overwrite invalid values and-ed together', () => { diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx index 45820b5c2b7026..8d0f042e7a4988 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx @@ -28,7 +28,15 @@ import { } from '../../autocomplete/operators'; import { BuilderEntry, ExceptionsBuilderExceptionItem, FormattedBuilderEntry } from '../types'; import { IFieldType, IIndexPattern } from '../../../../../../../../src/plugins/data/common'; -import { Entry, EntryMatch, EntryNested } from '../../../../lists_plugin_deps'; +import { + EntryMatch, + EntryMatchAny, + EntryNested, + EntryList, + EntryExists, + OperatorTypeEnum, + OperatorEnum, +} from '../../../../shared_imports'; import { getEntryFromOperator, @@ -453,8 +461,8 @@ describe('Exception builder helpers', () => { id: '123', entries: [{ ...payloadItem }], field: 'nestedField', - type: 'nested', - } as EntryNested & { id: string }, + type: OperatorTypeEnum.NESTED, + }, parentIndex: 1, }, value: 'some host name', @@ -662,14 +670,14 @@ describe('Exception builder helpers', () => { { id: '123', field: 'child', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some host name', - } as EntryMatch & { id: string }, + }, ], field: 'nestedField', - type: 'nested', - } as EntryNested & { id: string }, + type: OperatorTypeEnum.NESTED, + }, parentIndex: 1, }, value: 'some host name', @@ -693,8 +701,8 @@ describe('Exception builder helpers', () => { { id: '123', field: 'some.not.nested.field', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some value', }, ], @@ -749,11 +757,11 @@ describe('Exception builder helpers', () => { value: 'I should stay the same', }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatch & { id?: string } = { id: '123', field: 'ip', operator: 'excluded', - type: 'match', + type: OperatorTypeEnum.MATCH, value: 'I should stay the same', }; expect(output).toEqual(expected); @@ -767,11 +775,11 @@ describe('Exception builder helpers', () => { value: 'I should stay the same', }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatch & { id?: string } = { id: '123', field: 'ip', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'I should stay the same', }; expect(output).toEqual(expected); @@ -785,11 +793,11 @@ describe('Exception builder helpers', () => { value: ['I should stay the same'], }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatch & { id?: string } = { id: '123', field: 'ip', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: '', }; expect(output).toEqual(expected); @@ -803,11 +811,11 @@ describe('Exception builder helpers', () => { value: ['I should stay the same'], }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatchAny & { id?: string } = { id: '123', field: 'ip', operator: 'excluded', - type: 'match_any', + type: OperatorTypeEnum.MATCH_ANY, value: ['I should stay the same'], }; expect(output).toEqual(expected); @@ -821,11 +829,11 @@ describe('Exception builder helpers', () => { value: ['I should stay the same'], }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatchAny & { id?: string } = { id: '123', field: 'ip', - operator: 'included', - type: 'match_any', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: ['I should stay the same'], }; expect(output).toEqual(expected); @@ -839,11 +847,11 @@ describe('Exception builder helpers', () => { value: 'I should stay the same', }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryMatchAny & { id?: string } = { id: '123', field: 'ip', - operator: 'included', - type: 'match_any', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: [], }; expect(output).toEqual(expected); @@ -856,7 +864,7 @@ describe('Exception builder helpers', () => { operator: existsOperator, }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryExists & { id?: string } = { id: '123', field: 'ip', operator: 'excluded', @@ -872,10 +880,10 @@ describe('Exception builder helpers', () => { operator: doesNotExistOperator, }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryExists & { id?: string } = { id: '123', field: 'ip', - operator: 'included', + operator: OperatorEnum.INCLUDED, type: 'exists', }; expect(output).toEqual(expected); @@ -889,10 +897,10 @@ describe('Exception builder helpers', () => { value: 'I should stay the same', }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryExists & { id?: string } = { id: '123', field: 'ip', - operator: 'included', + operator: OperatorEnum.INCLUDED, type: 'exists', }; expect(output).toEqual(expected); @@ -906,10 +914,10 @@ describe('Exception builder helpers', () => { value: 'I should stay the same', }; const output = getEntryFromOperator(payloadOperator, payloadEntry); - const expected: Entry = { + const expected: EntryList & { id?: string } = { id: '123', field: 'ip', - operator: 'included', + operator: OperatorEnum.INCLUDED, type: 'list', list: { id: '', type: 'ip' }, }; @@ -1004,13 +1012,21 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedParentBuilderEntry(); const payloadIFieldType: IFieldType = getField('nestedField.child'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', - entries: [{ id: '123', field: 'child', operator: 'included', type: 'match', value: '' }], + entries: [ + { + id: '123', + field: 'child', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, + value: '', + }, + ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1033,16 +1049,22 @@ describe('Exception builder helpers', () => { }; const payloadIFieldType: IFieldType = getField('nestedField.child'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', entries: [ - { id: '123', field: 'child', operator: 'included', type: 'match', value: '' }, + { + id: '123', + field: 'child', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, + value: '', + }, getEntryMatchAnyWithIdMock(), ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1052,13 +1074,13 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadIFieldType: IFieldType = getField('ip'); const output = getEntryOnFieldChange(payloadItem, payloadIFieldType); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', field: 'ip', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: '', }, }; @@ -1071,11 +1093,11 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadOperator: OperatorOption = isNotOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: 'ip', - type: 'match', + type: OperatorTypeEnum.MATCH, value: 'some value', operator: 'excluded', }, @@ -1088,13 +1110,13 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockBuilderEntry(); const payloadOperator: OperatorOption = isOneOfOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: 'ip', - type: 'match_any', + type: OperatorTypeEnum.MATCH_ANY, value: [], - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1105,7 +1127,7 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedBuilderEntry(); const payloadOperator: OperatorOption = isNotOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1113,13 +1135,13 @@ describe('Exception builder helpers', () => { { id: '123', field: 'child', - operator: 'excluded', - type: 'match', + operator: OperatorEnum.EXCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some value', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1129,7 +1151,7 @@ describe('Exception builder helpers', () => { const payloadItem: FormattedBuilderEntry = getMockNestedBuilderEntry(); const payloadOperator: OperatorOption = isOneOfOperator; const output = getEntryOnOperatorChange(payloadItem, payloadOperator); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1137,13 +1159,13 @@ describe('Exception builder helpers', () => { { id: '123', field: 'child', - operator: 'included', - type: 'match_any', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: [], }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1154,13 +1176,13 @@ describe('Exception builder helpers', () => { test('it returns entry with updated value', () => { const payload: FormattedBuilderEntry = getMockBuilderEntry(); const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: 'ip', - type: 'match', + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1170,13 +1192,13 @@ describe('Exception builder helpers', () => { test('it returns entry with updated value and "field" of empty string if entry does not have a "field" defined', () => { const payload: FormattedBuilderEntry = { ...getMockBuilderEntry(), field: undefined }; const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: '', - type: 'match', + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1186,7 +1208,7 @@ describe('Exception builder helpers', () => { test('it returns nested entry with updated value', () => { const payload: FormattedBuilderEntry = getMockNestedBuilderEntry(); const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1194,13 +1216,13 @@ describe('Exception builder helpers', () => { { id: '123', field: 'child', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1209,7 +1231,7 @@ describe('Exception builder helpers', () => { test('it returns nested entry with updated value and "field" of empty string if entry does not have a "field" defined', () => { const payload: FormattedBuilderEntry = { ...getMockNestedBuilderEntry(), field: undefined }; const output = getEntryOnMatchChange(payload, 'jibber jabber'); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1217,13 +1239,13 @@ describe('Exception builder helpers', () => { { id: '123', field: '', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1238,13 +1260,13 @@ describe('Exception builder helpers', () => { value: ['some value'], }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: 'ip', - type: 'match_any', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1259,13 +1281,13 @@ describe('Exception builder helpers', () => { field: undefined, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: '', - type: 'match_any', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1285,7 +1307,7 @@ describe('Exception builder helpers', () => { }, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1293,13 +1315,13 @@ describe('Exception builder helpers', () => { { id: '123', field: 'child', - operator: 'included', - type: 'match_any', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], + operator: OperatorEnum.INCLUDED, }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1319,7 +1341,7 @@ describe('Exception builder helpers', () => { }, }; const output = getEntryOnMatchAnyChange(payload, ['jibber jabber']); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { index: 0, updatedEntry: { id: '123', @@ -1327,13 +1349,13 @@ describe('Exception builder helpers', () => { { id: '123', field: '', - operator: 'included', - type: 'match_any', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1348,13 +1370,13 @@ describe('Exception builder helpers', () => { value: '1234', }; const output = getEntryOnListChange(payload, getListResponseMock()); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: 'ip', type: 'list', list: { id: 'some-list-id', type: 'ip' }, - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1369,13 +1391,13 @@ describe('Exception builder helpers', () => { field: undefined, }; const output = getEntryOnListChange(payload, getListResponseMock()); - const expected: { updatedEntry: BuilderEntry & { id: string }; index: number } = { + const expected: { updatedEntry: BuilderEntry & { id?: string }; index: number } = { updatedEntry: { id: '123', field: '', type: 'list', list: { id: 'some-list-id', type: 'ip' }, - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index 806049f04a4f44..0628e565938279 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -17,9 +17,6 @@ import { entriesList, ListSchema, OperatorEnum, - EntryMatch, - EntryMatchAny, - EntryExists, } from '../../../../lists_plugin_deps'; import { isOperator, @@ -143,7 +140,7 @@ export const getCorrespondingKeywordField = ({ */ export const getFormattedBuilderEntry = ( indexPattern: IIndexPattern, - item: BuilderEntry & { id?: string }, + item: BuilderEntry, itemIndex: number, parent: EntryNested | undefined, parentIndex: number | undefined @@ -202,7 +199,7 @@ export const isEntryNested = (item: BuilderEntry): item is EntryNested => { */ export const getFormattedBuilderEntries = ( indexPattern: IIndexPattern, - entries: Array, + entries: BuilderEntry[], parent?: EntryNested, parentIndex?: number ): FormattedBuilderEntry[] => { @@ -271,7 +268,7 @@ export const getUpdatedEntriesOnDelete = ( const itemOfInterest: BuilderEntry = exceptionItem.entries[nestedParentIndex ?? entryIndex]; if (nestedParentIndex != null && itemOfInterest.type === OperatorTypeEnum.NESTED) { - const updatedEntryEntries: Array = [ + const updatedEntryEntries = [ ...itemOfInterest.entries.slice(0, entryIndex), ...itemOfInterest.entries.slice(entryIndex + 1), ]; @@ -324,7 +321,7 @@ export const getUpdatedEntriesOnDelete = ( export const getEntryFromOperator = ( selectedOperator: OperatorOption, currentEntry: FormattedBuilderEntry -): Entry & { id: string } => { +): Entry & { id?: string } => { const isSameOperatorType = currentEntry.operator.type === selectedOperator.type; const fieldValue = currentEntry.field != null ? currentEntry.field.name : ''; switch (selectedOperator.type) { @@ -402,7 +399,7 @@ export const getOperatorOptions = ( export const getEntryOnFieldChange = ( item: FormattedBuilderEntry, newField: IFieldType -): { updatedEntry: BuilderEntry & { id: string }; index: number } => { +): { updatedEntry: BuilderEntry; index: number } => { const { parent, entryIndex, nested } = item; const newChildFieldValue = newField != null ? newField.name.split('.').slice(-1)[0] : ''; @@ -445,7 +442,7 @@ export const getEntryOnFieldChange = ( type: OperatorTypeEnum.MATCH, operator: isOperator.operator, value: '', - } as EntryMatch & { id: string }, + }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -475,7 +472,7 @@ export const getEntryOnFieldChange = ( export const getEntryOnOperatorChange = ( item: FormattedBuilderEntry, newOperator: OperatorOption -): { updatedEntry: BuilderEntry & { id: string }; index: number } => { +): { updatedEntry: BuilderEntry; index: number } => { const { parent, entryIndex, field, nested } = item; const newEntry = getEntryFromOperator(newOperator, item); @@ -510,7 +507,7 @@ export const getEntryOnOperatorChange = ( export const getEntryOnMatchChange = ( item: FormattedBuilderEntry, newField: string -): { updatedEntry: BuilderEntry & { id: string }; index: number } => { +): { updatedEntry: BuilderEntry; index: number } => { const { nested, parent, entryIndex, field, operator } = item; if (nested != null && parent != null) { @@ -527,7 +524,7 @@ export const getEntryOnMatchChange = ( type: OperatorTypeEnum.MATCH, operator: operator.operator, value: newField, - } as EntryMatch & { id: string }, + }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -558,7 +555,7 @@ export const getEntryOnMatchChange = ( export const getEntryOnMatchAnyChange = ( item: FormattedBuilderEntry, newField: string[] -): { updatedEntry: BuilderEntry & { id: string }; index: number } => { +): { updatedEntry: BuilderEntry; index: number } => { const { nested, parent, entryIndex, field, operator } = item; if (nested != null && parent != null) { @@ -575,7 +572,7 @@ export const getEntryOnMatchAnyChange = ( type: OperatorTypeEnum.MATCH_ANY, operator: operator.operator, value: newField, - } as EntryMatchAny & { id: string }, + }, ...parent.parent.entries.slice(entryIndex + 1), ], }, @@ -606,7 +603,7 @@ export const getEntryOnMatchAnyChange = ( export const getEntryOnListChange = ( item: FormattedBuilderEntry, newField: ListSchema -): { updatedEntry: BuilderEntry & { id: string }; index: number } => { +): { updatedEntry: BuilderEntry; index: number } => { const { entryIndex, field, operator } = item; const { id, type } = newField; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts index 621d0e5ac06866..bca850c05d2e0e 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/types.ts @@ -64,7 +64,7 @@ export interface FormattedBuilderEntry { value: string | string[] | undefined; nested: 'parent' | 'child' | undefined; entryIndex: number; - parent: { parent: EntryNested; parentIndex: number } | undefined; + parent: { parent: BuilderEntryNested; parentIndex: number } | undefined; correspondingKeywordField: IFieldType | undefined; } @@ -88,10 +88,28 @@ export interface EmptyNestedEntry { id: string; field: string | undefined; type: OperatorTypeEnum.NESTED; - entries: Array; + entries: Array< + | (EntryMatch & { id?: string }) + | (EntryMatchAny & { id?: string }) + | (EntryExists & { id?: string }) + >; } -export type BuilderEntry = Entry | EmptyListEntry | EmptyEntry | EntryNested | EmptyNestedEntry; +export type BuilderEntry = + | (Entry & { id?: string }) + | EmptyListEntry + | EmptyEntry + | BuilderEntryNested + | EmptyNestedEntry; + +export type BuilderEntryNested = Omit & { + id?: string; + entries: Array< + | (EntryMatch & { id?: string }) + | (EntryMatchAny & { id?: string }) + | (EntryExists & { id?: string }) + >; +}; export type ExceptionListItemBuilderSchema = Omit & { entries: BuilderEntry[]; diff --git a/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json b/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json index 3e0c2a66a298c2..4e5c6e9955310d 100644 --- a/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json +++ b/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json @@ -2279,14 +2279,6 @@ } } }, - "nestedField": { - "type": "nested", - "properties": { - "child": { - "type": "keyword" - } - } - }, "related": { "properties": { "ip": { From 65ee43e8429992aae33acc1b1569a949e150ab96 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Thu, 11 Feb 2021 19:09:03 -0800 Subject: [PATCH 07/13] finalizing tests --- .../exceptions/exceptions_modal.spec.ts | 123 ++++++++++++++---- .../cypress/screens/exceptions.ts | 2 + .../cypress/tasks/exceptions.ts | 14 ++ 3 files changed, 111 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts index 4c7a9e6519a7f0..14b5cd5cddc59f 100644 --- a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts @@ -15,7 +15,11 @@ import { goToRuleDetails } from '../../tasks/alerts_detection_rules'; import { esArchiverLoad, esArchiverUnload } from '../../tasks/es_archiver'; import { loginAndWaitForPageWithoutDateRange } from '../../tasks/login'; import { openExceptionModalFromRuleSettings, goToExceptionsTab } from '../../tasks/rule_details'; -import { addExceptionEntryFieldValue, closeExceptionBuilderModal } from '../../tasks/exceptions'; +import { + addExceptionEntryFieldValue, + addExceptionEntryFieldValueOfItemX, + closeExceptionBuilderModal, +} from '../../tasks/exceptions'; import { ADD_AND_BTN, ADD_OR_BTN, @@ -23,11 +27,17 @@ import { ENTRY_DELETE_BTN, FIELD_INPUT, LOADING_SPINNER, + EXCEPTION_ITEM_CONTAINER, } from '../../screens/exceptions'; import { DETECTIONS_URL } from '../../urls/navigation'; import { cleanKibana } from '../../tasks/common'; +// NOTE: You might look at these tests and feel they're overkill, +// but the exceptions modal has a lot of logic making it difficult +// to test in enzyme and very small changes can inadvertently add +// bugs. As the complexity within the builder grows, these should +// ensure the most basic logic holds. describe('Exceptions modal', () => { before(() => { cleanKibana(); @@ -39,6 +49,9 @@ describe('Exceptions modal', () => { cy.get(RULE_STATUS).should('have.text', '—'); + // this is a made-up index that has just the necessary + // mappings to conduct tests, avoiding loading large + // amounts of data like in auditbeat_exceptions esArchiverLoad('exceptions'); goToExceptionsTab(); @@ -48,72 +61,126 @@ describe('Exceptions modal', () => { esArchiverUnload('exceptions'); }); - it('Does not overwrite invalid values and-ed together', () => { - openExceptionModalFromRuleSettings(); - + it('Does not overwrite values and-ed together', () => { + // VALID VALUES // add multiple entries with invalid field values - addExceptionEntryFieldValue('a', 0); + addExceptionEntryFieldValue('agent.name', 0); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValue('b', 1); + addExceptionEntryFieldValue('@timestamp', 1); cy.get(ADD_AND_BTN).click(); addExceptionEntryFieldValue('c', 2); // delete second item, invalid values 'a' and 'c' should remain cy.get(ENTRY_DELETE_BTN).eq(1).click(); - cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); + cy.get(FIELD_INPUT).eq(0).should('have.text', 'agent.name'); cy.get(FIELD_INPUT).eq(1).should('have.text', 'c'); closeExceptionBuilderModal(); }); - it('Does not overwrite invalid values or-ed together', () => { - openExceptionModalFromRuleSettings(); - cy.get(LOADING_SPINNER).should('not.exist'); - + it('Does not overwrite values or-ed together', () => { // exception item 1 - addExceptionEntryFieldValue('a', 0); + addExceptionEntryFieldValueOfItemX('agent.name', 0, 0); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValue('b', 1); + addExceptionEntryFieldValueOfItemX('user.id.keyword', 0, 1); // exception item 2 cy.get(ADD_OR_BTN).click(); - addExceptionEntryFieldValue('c', 2); + addExceptionEntryFieldValueOfItemX('user.first', 1, 0); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValue('d', 3); + addExceptionEntryFieldValueOfItemX('user.last', 1, 1); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValue('e', 4); + addExceptionEntryFieldValueOfItemX('e', 0, 2); // delete single entry from exception item 2 cy.get(ENTRY_DELETE_BTN).eq(3).click(); - cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); - cy.get(FIELD_INPUT).eq(1).should('have.text', 'b'); - cy.get(FIELD_INPUT).eq(2).should('have.text', 'c'); - cy.get(FIELD_INPUT).eq(3).should('have.text', 'e'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(0) + .should('have.text', 'agent.name'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(1) + .should('have.text', 'user.id.keyword'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(1) + .find(FIELD_INPUT) + .eq(0) + .should('have.text', 'user.first'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).find(FIELD_INPUT).eq(1).should('have.text', 'e'); // delete remaining entries in exception item 2 cy.get(ENTRY_DELETE_BTN).eq(2).click(); cy.get(ENTRY_DELETE_BTN).eq(2).click(); - cy.get(FIELD_INPUT).eq(0).should('have.text', 'a'); - cy.get(FIELD_INPUT).eq(1).should('have.text', 'b'); - cy.get(FIELD_INPUT).eq(2).should('not.exist'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(0) + .should('have.text', 'agent.name'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(1) + .should('have.text', 'user.id.keyword'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).should('not.exist'); closeExceptionBuilderModal(); }); - it('Does not overwrite invalid values of nested entry items', () => { + it('Does not overwrite values of nested entry items', () => { openExceptionModalFromRuleSettings(); cy.get(LOADING_SPINNER).should('not.exist'); // exception item 1 - addExceptionEntryFieldValue('a', 0); + addExceptionEntryFieldValueOfItemX('agent.name', 0, 0); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValue('b', 1); + addExceptionEntryFieldValueOfItemX('b', 0, 1); // exception item 2 with nested field cy.get(ADD_OR_BTN).click(); - addExceptionEntryFieldValue('c', 2); + addExceptionEntryFieldValueOfItemX('c', 1, 0); cy.get(ADD_NESTED_BTN).click(); + addExceptionEntryFieldValueOfItemX('user.id{downarrow}{enter}', 1, 1); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValueOfItemX('last{downarrow}{enter}', 1, 3); + // This button will now read `Add non-nested button` + cy.get(ADD_NESTED_BTN).click(); + addExceptionEntryFieldValueOfItemX('@timestamp', 1, 4); + + // should have only deleted `user.id` + cy.get(ENTRY_DELETE_BTN).eq(4).click(); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(0) + .should('have.text', 'agent.name'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(0).find(FIELD_INPUT).eq(1).should('have.text', 'b'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).find(FIELD_INPUT).eq(0).should('have.text', 'c'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).find(FIELD_INPUT).eq(1).should('have.text', 'user'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).find(FIELD_INPUT).eq(2).should('have.text', 'last'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(1) + .find(FIELD_INPUT) + .eq(3) + .should('have.text', '@timestamp'); + + // deleting the last value of a nested entry, should delete the child and parent + cy.get(ENTRY_DELETE_BTN).eq(4).click(); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(0) + .find(FIELD_INPUT) + .eq(0) + .should('have.text', 'agent.name'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(0).find(FIELD_INPUT).eq(1).should('have.text', 'b'); + cy.get(EXCEPTION_ITEM_CONTAINER).eq(1).find(FIELD_INPUT).eq(0).should('have.text', 'c'); + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(1) + .find(FIELD_INPUT) + .eq(1) + .should('have.text', '@timestamp'); - // closeExceptionBuilderModal(); + closeExceptionBuilderModal(); }); }); diff --git a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts index 0e020dc36842c5..3d7fe58fc0c776 100644 --- a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts +++ b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts @@ -56,3 +56,5 @@ export const EXCEPTIONS_TABLE_LIST_NAME = '[data-test-subj="exceptionsTableName" export const EXCEPTIONS_TABLE_MODAL = '[data-test-subj="referenceErrorModal"]'; export const EXCEPTIONS_TABLE_MODAL_CONFIRM_BTN = '[data-test-subj="confirmModalConfirmButton"]'; + +export const EXCEPTION_ITEM_CONTAINER = '[data-test-subj="exceptionEntriesContainer"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts b/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts index bb87ff2a94f161..97e93ef8194a47 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts @@ -12,8 +12,22 @@ import { VALUES_INPUT, CANCEL_BTN, BUILDER_MODAL_BODY, + EXCEPTION_ITEM_CONTAINER, } from '../screens/exceptions'; +export const addExceptionEntryFieldValueOfItemX = ( + field: string, + itemIndex = 0, + fieldIndex = 0 +) => { + cy.get(EXCEPTION_ITEM_CONTAINER) + .eq(itemIndex) + .find(FIELD_INPUT) + .eq(fieldIndex) + .type(`${field}{enter}`); + cy.get(BUILDER_MODAL_BODY).click(); +}; + export const addExceptionEntryFieldValue = (field: string, index = 0) => { cy.get(FIELD_INPUT).eq(index).type(`${field}{enter}`); cy.get(BUILDER_MODAL_BODY).click(); From 259fc9dc691c37288abed1d173cc8b2d26f2a7d5 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Mon, 22 Feb 2021 19:34:24 -0800 Subject: [PATCH 08/13] fixed up tests --- .../public/common/components/exceptions/helpers.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx index ae865b5e8151b1..c44de4f05e7f66 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/helpers.tsx @@ -180,24 +180,28 @@ export const filterExceptionItems = ( if (entriesNested.is(strippedSingleEntry)) { const nestedEntriesArray = strippedSingleEntry.entries.filter((singleNestedEntry) => { - const [validatedNestedEntry] = validate(singleNestedEntry, nestedEntryItem); + const noIdSingleNestedEntry = removeIdFromItem(singleNestedEntry); + const [validatedNestedEntry] = validate(noIdSingleNestedEntry, nestedEntryItem); return validatedNestedEntry != null; }); + const noIdNestedEntries = nestedEntriesArray.map((singleNestedEntry) => + removeIdFromItem(singleNestedEntry) + ); const [validatedNestedEntry] = validate( - { ...strippedSingleEntry, entries: nestedEntriesArray }, + { ...strippedSingleEntry, entries: noIdNestedEntries }, entriesNested ); if (validatedNestedEntry != null) { - return [...nestedAcc, validatedNestedEntry]; + return [...nestedAcc, { ...singleEntry, entries: nestedEntriesArray }]; } return nestedAcc; } else { const [validatedEntry] = validate(strippedSingleEntry, entry); if (validatedEntry != null) { - return [...nestedAcc, validatedEntry]; + return [...nestedAcc, singleEntry]; } return nestedAcc; } From a12f211e50449959d577ccfb2757f5a542ad2c1d Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 23 Feb 2021 15:56:52 -0800 Subject: [PATCH 09/13] merged with master and fixed tests --- .../public/exceptions/hooks/use_api.test.ts | 92 ++++++++++--------- .../lists/public/exceptions/hooks/use_api.ts | 22 +++-- .../lists/public/exceptions/transforms.ts | 16 +++- 3 files changed, 73 insertions(+), 57 deletions(-) diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts index b15338986cabef..62f959cb386a07 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.test.ts @@ -406,57 +406,61 @@ describe('useApi', () => { }); }); - test('it invokes "addExceptionListItem" when "addExceptionListItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const itemToCreate = getCreateExceptionListItemSchemaMock(); - const spyOnFetchExceptionListItemById = jest - .spyOn(api, 'addExceptionListItem') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - await result.current.addExceptionListItem({ - listItem: itemToCreate, - }); + describe('addExceptionListItem', () => { + test('it removes exception item entry ids', async () => { + const payload = getExceptionListItemSchemaMock(); + const itemToCreate = { ...getCreateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }; + const spyOnFetchExceptionListItemById = jest + .spyOn(api, 'addExceptionListItem') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + await result.current.addExceptionListItem({ + listItem: itemToCreate, + }); - const expected: AddExceptionListItemProps = { - http: mockKibanaHttpService, - listItem: itemToCreate, - signal: new AbortController().signal, - }; + const expected: AddExceptionListItemProps = { + http: mockKibanaHttpService, + listItem: getCreateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }; - expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); + expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); + }); }); }); - test('it invokes "updateExceptionListItem" when "getExceptionItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const itemToUpdate = getUpdateExceptionListItemSchemaMock(); - const spyOnUpdateExceptionListItem = jest - .spyOn(api, 'updateExceptionListItem') - .mockResolvedValue(payload); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - await result.current.updateExceptionListItem({ - listItem: itemToUpdate, - }); + describe('updateExceptionListItem', () => { + test('it removes exception item entry ids', async () => { + const payload = getExceptionListItemSchemaMock(); + const itemToUpdate = { ...getUpdateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS }; + const spyOnUpdateExceptionListItem = jest + .spyOn(api, 'updateExceptionListItem') + .mockResolvedValue(payload); + + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + await result.current.updateExceptionListItem({ + listItem: itemToUpdate, + }); - const expected: UpdateExceptionListItemProps = { - http: mockKibanaHttpService, - listItem: itemToUpdate, - signal: new AbortController().signal, - }; + const expected: UpdateExceptionListItemProps = { + http: mockKibanaHttpService, + listItem: getUpdateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }; - expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected); + expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected); + }); }); }); }); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts index 648691d626998e..28cfcb167ca8cf 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts @@ -14,10 +14,11 @@ import { ExceptionListItemSchema, ExceptionListSchema, UpdateExceptionListItemSchema, + createExceptionListItemSchema, } from '../../../common/schemas'; import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types'; import { getIdsAndNamespaces } from '../utils'; -import { transformInput } from '../transforms'; +import { transformInput, transformOutput } from '../transforms'; export interface ExceptionsApi { addExceptionListItem: (arg: { @@ -47,12 +48,16 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: CreateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); - - return Api.addExceptionListItem({ - http, - listItem, - signal: abortCtrl.signal, - }); + const sanitizedItem = transformOutput(listItem); + if (createExceptionListItemSchema.is(sanitizedItem)) { + return Api.addExceptionListItem({ + http, + listItem: sanitizedItem, + signal: abortCtrl.signal, + }); + } else { + throw new Error('Unable to create exception item. Item malformed.'); + } }, async deleteExceptionItem({ id, @@ -220,10 +225,11 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: UpdateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); + const sanitizedItem = transformOutput(listItem); return Api.updateExceptionListItem({ http, - listItem, + listItem: sanitizedItem, signal: abortCtrl.signal, }); }, diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts index 809a0b471ed6a8..92c2cdb3ba337d 100644 --- a/x-pack/plugins/lists/public/exceptions/transforms.ts +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -34,8 +34,11 @@ import { addIdToItem, removeIdFromItem } from '../../common/shared_imports'; * @returns The exceptionItem transformed from the output */ export const transformOutput = ( - exceptionItem: CreateExceptionListItemSchema | UpdateExceptionListItemSchema -): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => + exceptionItem: + | CreateExceptionListItemSchema + | UpdateExceptionListItemSchema + | ExceptionListItemSchema +): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema => flow(removeIdFromExceptionItemsEntries)(exceptionItem); /** @@ -93,13 +96,16 @@ export const addIdToExceptionItemEntries = ( * @returns exceptionItem The exceptionItem but with id removed from the entries */ export const removeIdFromExceptionItemsEntries = ( - exceptionItem: CreateExceptionListItemSchema | UpdateExceptionListItemSchema -): CreateExceptionListItemSchema | UpdateExceptionListItemSchema => { + exceptionItem: + | CreateExceptionListItemSchema + | UpdateExceptionListItemSchema + | ExceptionListItemSchema +): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema => { const { entries, ...itemInfo } = exceptionItem; const entriesNoId = entries.map((entry) => { if (entry.type === 'nested') { return removeIdFromItem({ - ...itemInfo, + ...entry, entries: entry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), }); } else { From e679318a8cb23c4cb89db28aaa79e96c97d858f7 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 23 Feb 2021 17:53:55 -0800 Subject: [PATCH 10/13] cleanup of types --- .../common/components/exceptions/builder/helpers.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx index 0628e565938279..8afdbce68c69a1 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.tsx @@ -160,7 +160,7 @@ export const getFormattedBuilderEntry = ( ? { ...foundField, name: foundField.name.split('.').slice(-1)[0] } : foundField, correspondingKeywordField, - id: (item as typeof item & { id?: string }).id ?? `${itemIndex}`, + id: item.id ?? `${itemIndex}`, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), nested: 'child', @@ -170,7 +170,7 @@ export const getFormattedBuilderEntry = ( } else { return { field: foundField, - id: (item as typeof item & { id?: string }).id ?? `${itemIndex}`, + id: item.id ?? `${itemIndex}`, correspondingKeywordField, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), @@ -217,7 +217,7 @@ export const getFormattedBuilderEntries = ( } else { const parentEntry: FormattedBuilderEntry = { operator: isOperator, - id: (item as typeof item & { id?: string }).id ?? `${index}`, + id: item.id ?? `${index}`, nested: 'parent', field: isNewNestedEntry ? undefined @@ -285,7 +285,7 @@ export const getUpdatedEntriesOnDelete = ( const { field } = itemOfInterest; const updatedItemOfInterest: EntryNested | EmptyNestedEntry = { field, - id: (itemOfInterest as typeof itemOfInterest & { id?: string }).id ?? `${entryIndex}`, + id: itemOfInterest.id ?? `${entryIndex}`, type: OperatorTypeEnum.NESTED, entries: updatedEntryEntries, }; @@ -405,7 +405,7 @@ export const getEntryOnFieldChange = ( if (nested === 'parent') { // For nested entries, when user first selects to add a nested - // entry, they first see a row similiar to what is shown for when + // entry, they first see a row similar to what is shown for when // a user selects "exists", as soon as they make a selection // we can now identify the 'parent' and 'child' this is where // we first convert the entry into type "nested" From a4fd2f35aa96ef706148fb4998c053a0293a806c Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 23 Feb 2021 18:06:10 -0800 Subject: [PATCH 11/13] updated to use i18n in one throw --- .../lists/public/exceptions/hooks/translations.ts | 15 +++++++++++++++ .../lists/public/exceptions/hooks/use_api.ts | 9 ++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/lists/public/exceptions/hooks/translations.ts diff --git a/x-pack/plugins/lists/public/exceptions/hooks/translations.ts b/x-pack/plugins/lists/public/exceptions/hooks/translations.ts new file mode 100644 index 00000000000000..41c64216159626 --- /dev/null +++ b/x-pack/plugins/lists/public/exceptions/hooks/translations.ts @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { i18n } from '@kbn/i18n'; + +export const ADD_EXCEPTION_ITEM_TYPE_ERROR = i18n.translate( + 'xpack.lists.exceptions.hooks.addExceptionItemTypeError', + { + defaultMessage: 'Unable to create exception item. Item malformed.', + } +); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts index 28cfcb167ca8cf..878b5e3836afe8 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts @@ -20,6 +20,7 @@ import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } import { getIdsAndNamespaces } from '../utils'; import { transformInput, transformOutput } from '../transforms'; +import * as i18n from './translations'; export interface ExceptionsApi { addExceptionListItem: (arg: { listItem: CreateExceptionListItemSchema; @@ -49,6 +50,12 @@ export const useApi = (http: HttpStart): ExceptionsApi => { }): Promise { const abortCtrl = new AbortController(); const sanitizedItem = transformOutput(listItem); + + // This was added to satisfy Typescript, which I don't love, + // but felt better than doing an `as` cast. Wasn't able to + // figure out how to do a generic to specify that input is of + // type A or B, and if it is A returns A, if B returns B. Can + // delete this if/else statement if figured out if (createExceptionListItemSchema.is(sanitizedItem)) { return Api.addExceptionListItem({ http, @@ -56,7 +63,7 @@ export const useApi = (http: HttpStart): ExceptionsApi => { signal: abortCtrl.signal, }); } else { - throw new Error('Unable to create exception item. Item malformed.'); + throw new Error(i18n.ADD_EXCEPTION_ITEM_TYPE_ERROR); } }, async deleteExceptionItem({ From 7825c7b1227e99bf3ff2495188bd733817432d7f Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 24 Feb 2021 12:05:17 -0800 Subject: [PATCH 12/13] updated types to remove casting --- .../hooks/persist_exception_item.ts | 28 +++++++++++++------ .../public/exceptions/hooks/translations.ts | 15 ---------- .../lists/public/exceptions/hooks/use_api.ts | 27 ++++++------------ .../lists/public/exceptions/transforms.ts | 24 ++++++++-------- .../plugins/lists/public/exceptions/types.ts | 2 -- .../exceptions/exceptions_modal.spec.ts | 8 ++++-- .../cypress/screens/exceptions.ts | 1 + .../exceptions/add_exception_modal/index.tsx | 4 ++- .../exceptions/edit_exception_modal/index.tsx | 4 ++- 9 files changed, 51 insertions(+), 62 deletions(-) delete mode 100644 x-pack/plugins/lists/public/exceptions/hooks/translations.ts diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts index f08d53d83ca505..6135d14aef6a49 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts @@ -7,10 +7,13 @@ import { Dispatch, useEffect, useState } from 'react'; -import { UpdateExceptionListItemSchema } from '../../../common/schemas'; +import { + CreateExceptionListItemSchema, + UpdateExceptionListItemSchema, +} from '../../../common/schemas'; import { addExceptionListItem, updateExceptionListItem } from '../api'; -import { transformOutput } from '../transforms'; -import { AddExceptionListItem, PersistHookProps } from '../types'; +import { transformNewItemOutput, transformOutput } from '../transforms'; +import { PersistHookProps } from '../types'; interface PersistReturnExceptionItem { isLoading: boolean; @@ -19,7 +22,7 @@ interface PersistReturnExceptionItem { export type ReturnPersistExceptionItem = [ PersistReturnExceptionItem, - Dispatch + Dispatch ]; /** @@ -33,7 +36,9 @@ export const usePersistExceptionItem = ({ http, onError, }: PersistHookProps): ReturnPersistExceptionItem => { - const [exceptionListItem, setExceptionItem] = useState(null); + const [exceptionListItem, setExceptionItem] = useState< + CreateExceptionListItemSchema | UpdateExceptionListItemSchema | null + >(null); const [isSaved, setIsSaved] = useState(false); const [isLoading, setIsLoading] = useState(false); const isUpdateExceptionItem = (item: unknown): item is UpdateExceptionListItemSchema => @@ -48,17 +53,22 @@ export const usePersistExceptionItem = ({ if (exceptionListItem != null) { try { setIsLoading(true); - // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes - // for context around the temporary `id` - const transformedList = transformOutput(exceptionListItem); - if (isUpdateExceptionItem(transformedList)) { + if (isUpdateExceptionItem(exceptionListItem)) { + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` + const transformedList = transformOutput(exceptionListItem); + await updateExceptionListItem({ http, listItem: transformedList, signal: abortCtrl.signal, }); } else { + // Please see `x-pack/plugins/lists/public/exceptions/transforms.ts` doc notes + // for context around the temporary `id` + const transformedList = transformNewItemOutput(exceptionListItem); + await addExceptionListItem({ http, listItem: transformedList, diff --git a/x-pack/plugins/lists/public/exceptions/hooks/translations.ts b/x-pack/plugins/lists/public/exceptions/hooks/translations.ts deleted file mode 100644 index 41c64216159626..00000000000000 --- a/x-pack/plugins/lists/public/exceptions/hooks/translations.ts +++ /dev/null @@ -1,15 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { i18n } from '@kbn/i18n'; - -export const ADD_EXCEPTION_ITEM_TYPE_ERROR = i18n.translate( - 'xpack.lists.exceptions.hooks.addExceptionItemTypeError', - { - defaultMessage: 'Unable to create exception item. Item malformed.', - } -); diff --git a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts index 878b5e3836afe8..9e4e338b09dbf3 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts @@ -14,13 +14,11 @@ import { ExceptionListItemSchema, ExceptionListSchema, UpdateExceptionListItemSchema, - createExceptionListItemSchema, } from '../../../common/schemas'; import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types'; import { getIdsAndNamespaces } from '../utils'; -import { transformInput, transformOutput } from '../transforms'; +import { transformInput, transformNewItemOutput, transformOutput } from '../transforms'; -import * as i18n from './translations'; export interface ExceptionsApi { addExceptionListItem: (arg: { listItem: CreateExceptionListItemSchema; @@ -49,22 +47,13 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: CreateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); - const sanitizedItem = transformOutput(listItem); + const sanitizedItem: CreateExceptionListItemSchema = transformNewItemOutput(listItem); - // This was added to satisfy Typescript, which I don't love, - // but felt better than doing an `as` cast. Wasn't able to - // figure out how to do a generic to specify that input is of - // type A or B, and if it is A returns A, if B returns B. Can - // delete this if/else statement if figured out - if (createExceptionListItemSchema.is(sanitizedItem)) { - return Api.addExceptionListItem({ - http, - listItem: sanitizedItem, - signal: abortCtrl.signal, - }); - } else { - throw new Error(i18n.ADD_EXCEPTION_ITEM_TYPE_ERROR); - } + return Api.addExceptionListItem({ + http, + listItem: sanitizedItem, + signal: abortCtrl.signal, + }); }, async deleteExceptionItem({ id, @@ -232,7 +221,7 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: UpdateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); - const sanitizedItem = transformOutput(listItem); + const sanitizedItem: UpdateExceptionListItemSchema = transformOutput(listItem); return Api.updateExceptionListItem({ http, diff --git a/x-pack/plugins/lists/public/exceptions/transforms.ts b/x-pack/plugins/lists/public/exceptions/transforms.ts index 92c2cdb3ba337d..0791760611bf51 100644 --- a/x-pack/plugins/lists/public/exceptions/transforms.ts +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -34,13 +34,14 @@ import { addIdToItem, removeIdFromItem } from '../../common/shared_imports'; * @returns The exceptionItem transformed from the output */ export const transformOutput = ( - exceptionItem: - | CreateExceptionListItemSchema - | UpdateExceptionListItemSchema - | ExceptionListItemSchema -): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema => + exceptionItem: UpdateExceptionListItemSchema | ExceptionListItemSchema +): UpdateExceptionListItemSchema | ExceptionListItemSchema => flow(removeIdFromExceptionItemsEntries)(exceptionItem); +export const transformNewItemOutput = ( + exceptionItem: CreateExceptionListItemSchema +): CreateExceptionListItemSchema => flow(removeIdFromExceptionItemsEntries)(exceptionItem); + /** * Transforms the output of rules to compensate for technical debt or UI concerns such as * ReactJS preferences for having ids within arrays if the data is not modeled that way. @@ -95,13 +96,10 @@ export const addIdToExceptionItemEntries = ( * @param exceptionItem The exceptionItem to remove an id from the entries. * @returns exceptionItem The exceptionItem but with id removed from the entries */ -export const removeIdFromExceptionItemsEntries = ( - exceptionItem: - | CreateExceptionListItemSchema - | UpdateExceptionListItemSchema - | ExceptionListItemSchema -): CreateExceptionListItemSchema | UpdateExceptionListItemSchema | ExceptionListItemSchema => { - const { entries, ...itemInfo } = exceptionItem; +export const removeIdFromExceptionItemsEntries = ( + exceptionItem: T +): T => { + const { entries } = exceptionItem; const entriesNoId = entries.map((entry) => { if (entry.type === 'nested') { return removeIdFromItem({ @@ -112,5 +110,5 @@ export const removeIdFromExceptionItemsEntries = ( return removeIdFromItem(entry); } }); - return { ...itemInfo, entries: entriesNoId as EntriesArray }; + return { ...exceptionItem, entries: entriesNoId }; }; diff --git a/x-pack/plugins/lists/public/exceptions/types.ts b/x-pack/plugins/lists/public/exceptions/types.ts index e37c03978c9f6d..03cae387711f85 100644 --- a/x-pack/plugins/lists/public/exceptions/types.ts +++ b/x-pack/plugins/lists/public/exceptions/types.ts @@ -33,8 +33,6 @@ export interface Pagination { export type AddExceptionList = UpdateExceptionListSchema | CreateExceptionListSchema; -export type AddExceptionListItem = CreateExceptionListItemSchema | UpdateExceptionListItemSchema; - export interface PersistHookProps { http: HttpStart; onError: (arg: Error) => void; diff --git a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts index 14b5cd5cddc59f..154e90d509c612 100644 --- a/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts +++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts @@ -28,6 +28,7 @@ import { FIELD_INPUT, LOADING_SPINNER, EXCEPTION_ITEM_CONTAINER, + ADD_EXCEPTIONS_BTN, } from '../../screens/exceptions'; import { DETECTIONS_URL } from '../../urls/navigation'; @@ -62,7 +63,8 @@ describe('Exceptions modal', () => { }); it('Does not overwrite values and-ed together', () => { - // VALID VALUES + cy.get(ADD_EXCEPTIONS_BTN).click({ force: true }); + // add multiple entries with invalid field values addExceptionEntryFieldValue('agent.name', 0); cy.get(ADD_AND_BTN).click(); @@ -79,6 +81,8 @@ describe('Exceptions modal', () => { }); it('Does not overwrite values or-ed together', () => { + cy.get(ADD_EXCEPTIONS_BTN).click({ force: true }); + // exception item 1 addExceptionEntryFieldValueOfItemX('agent.name', 0, 0); cy.get(ADD_AND_BTN).click(); @@ -90,7 +94,7 @@ describe('Exceptions modal', () => { cy.get(ADD_AND_BTN).click(); addExceptionEntryFieldValueOfItemX('user.last', 1, 1); cy.get(ADD_AND_BTN).click(); - addExceptionEntryFieldValueOfItemX('e', 0, 2); + addExceptionEntryFieldValueOfItemX('e', 1, 2); // delete single entry from exception item 2 cy.get(ENTRY_DELETE_BTN).eq(3).click(); diff --git a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts index 3d7fe58fc0c776..2479b76cf1de47 100644 --- a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts +++ b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts @@ -37,6 +37,7 @@ export const FIELD_INPUT_LIST_BTN = '[data-test-subj="comboBoxToggleListButton"] export const CANCEL_BTN = '[data-test-subj="cancelExceptionAddButton"]'; export const BUILDER_MODAL_BODY = '[data-test-subj="exceptionsBuilderWrapper"]'; + export const EXCEPTIONS_TABLE_TAB = '[data-test-subj="allRulesTableTab-exceptions"]'; export const EXCEPTIONS_TABLE = '[data-test-subj="exceptions-table"]'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index b083ecbafeb2c1..3a2170d126a241 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -466,7 +466,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({ )} {fetchOrCreateListError == null && ( - {i18n.CANCEL} + + {i18n.CANCEL} + - {i18n.CANCEL} + + {i18n.CANCEL} + Date: Wed, 24 Feb 2021 16:15:19 -0800 Subject: [PATCH 13/13] adding feedback --- .../hooks/persist_exception_item.test.ts | 18 +++++++++--------- .../es_archives/exceptions/mappings.json | 5 +++++ 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts index 3244115516c101..7a39bd5651014f 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts @@ -70,8 +70,8 @@ describe('usePersistExceptionItem', () => { }); test('it invokes "updateExceptionListItem" when payload has "id"', async () => { - const addExceptionItem = jest.spyOn(api, 'addExceptionListItem'); - const updateExceptionItem = jest.spyOn(api, 'updateExceptionListItem'); + const addExceptionListItem = jest.spyOn(api, 'addExceptionListItem'); + const updateExceptionListItem = jest.spyOn(api, 'updateExceptionListItem'); await act(async () => { const { result, waitForNextUpdate } = renderHook< PersistHookProps, @@ -86,8 +86,8 @@ describe('usePersistExceptionItem', () => { await waitForNextUpdate(); expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); - expect(addExceptionItem).not.toHaveBeenCalled(); - expect(updateExceptionItem).toHaveBeenCalledWith({ + expect(addExceptionListItem).not.toHaveBeenCalled(); + expect(updateExceptionListItem).toHaveBeenCalledWith({ http: mockKibanaHttpService, listItem: getUpdateExceptionListItemSchemaMock(), signal: new AbortController().signal, @@ -95,9 +95,9 @@ describe('usePersistExceptionItem', () => { }); }); - test('it invokes "addExceptionListItem" when payload has "id"', async () => { - const updateExceptionItem = jest.spyOn(api, 'updateExceptionListItem'); - const createExceptionItem = jest.spyOn(api, 'addExceptionListItem'); + test('it invokes "addExceptionListItem" when payload does not have "id"', async () => { + const updateExceptionListItem = jest.spyOn(api, 'updateExceptionListItem'); + const addExceptionListItem = jest.spyOn(api, 'addExceptionListItem'); await act(async () => { const { result, waitForNextUpdate } = renderHook< PersistHookProps, @@ -112,8 +112,8 @@ describe('usePersistExceptionItem', () => { await waitForNextUpdate(); expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]); - expect(updateExceptionItem).not.toHaveBeenCalled(); - expect(createExceptionItem).toHaveBeenCalledWith({ + expect(updateExceptionListItem).not.toHaveBeenCalled(); + expect(addExceptionListItem).toHaveBeenCalledWith({ http: mockKibanaHttpService, listItem: getCreateExceptionListItemSchemaMock(), signal: new AbortController().signal, diff --git a/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json b/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json index 477f43d7759a81..e63b86392756fb 100644 --- a/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json +++ b/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json @@ -6,6 +6,11 @@ "is_write_index": false } }, + "settings": { + "index": { + "refresh_interval": "5s" + } + }, "index": "exceptions-0001", "mappings": { "properties": {