diff --git a/x-pack/plugins/lists/common/constants.mock.ts b/x-pack/plugins/lists/common/constants.mock.ts index 6f8782c148d4..27e0fa29b1e5 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'; @@ -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', + } 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']; export const TAGS = []; diff --git a/x-pack/plugins/lists/common/shared_imports.ts b/x-pack/plugins/lists/common/shared_imports.ts index 7e96b13c036e..2483c1f7dd99 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/persist_exception_item.test.ts b/x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts index ed16a405a510..7a39bd565101 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'; @@ -69,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, @@ -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(addExceptionListItem).not.toHaveBeenCalled(); + expect(updateExceptionListItem).toHaveBeenCalledWith({ + http: mockKibanaHttpService, + listItem: getUpdateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }); + }); + }); + + 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, + 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(updateExceptionListItem).not.toHaveBeenCalled(); + expect(addExceptionListItem).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 995c6b8703bf..6135d14aef6a 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,9 +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 { AddExceptionListItem, PersistHookProps } from '../types'; +import { transformNewItemOutput, transformOutput } from '../transforms'; +import { PersistHookProps } from '../types'; interface PersistReturnExceptionItem { isLoading: boolean; @@ -18,7 +22,7 @@ interface PersistReturnExceptionItem { export type ReturnPersistExceptionItem = [ PersistReturnExceptionItem, - Dispatch + Dispatch ]; /** @@ -32,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 => @@ -47,16 +53,25 @@ export const usePersistExceptionItem = ({ if (exceptionListItem != null) { try { setIsLoading(true); + 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: exceptionListItem, + 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: exceptionListItem, + listItem: transformedList, signal: abortCtrl.signal, }); } 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 e61e74ca3323..62f959cb386a 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 { getUpdateExceptionListItemSchemaMock } from '../../../common/schemas/request/update_exception_list_item_schema.mock'; import { coreMock } from '../../../../../../src/core/public/mocks'; import * as api from '../api'; @@ -24,6 +25,10 @@ import { import { ExceptionsApi, useApi } from './use_api'; +jest.mock('uuid', () => ({ + v4: jest.fn().mockReturnValue('123'), +})); + const mockKibanaHttpService = coreMock.createStart().http; describe('useApi', () => { @@ -34,397 +39,428 @@ 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); + 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(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - const { id, namespace_type: namespaceType } = payload; + const { id, namespace_type: namespaceType } = payload; - await result.current.deleteExceptionItem({ - id, - namespaceType, - onError: jest.fn(), - onSuccess: onSuccessMock, + 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); - }); - }); - - 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(); - }); - }); - - 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(); - - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - - await result.current.deleteExceptionList({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); - - 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, - }); - - 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); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - - await result.current.getExceptionItem({ - id, - namespaceType, - onError: onErrorMock, - onSuccess: jest.fn(), - }); - - 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, - }); - - 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); - - await act(async () => { - const { result, waitForNextUpdate } = renderHook(() => - useApi(mockKibanaHttpService) - ); - await waitForNextUpdate(); - - const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); - - 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, - }); - - 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, - }); - - expect(spyOnFetchExceptionListsItemsByListIds).not.toHaveBeenCalled(); - expect(onSuccessMock).toHaveBeenCalledWith({ - exceptions: [], - pagination: { - page: 0, - perPage: 20, - total: 0, - }, + expect(onErrorMock).toHaveBeenCalledWith(mockError); }); }); }); - test('invokes "onError" callback if "fetchExceptionListsItemsByListIds" fails', async () => { - const mockError = new Error('failed to delete item'); - jest.spyOn(api, 'fetchExceptionListsItemsByListIds').mockRejectedValue(mockError); + 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(); + 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, + 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(); }); + }); - expect(onErrorMock).toHaveBeenCalledWith(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(); + + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + + await result.current.deleteExceptionList({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); + + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); }); }); - test('it invokes "addExceptionListItem" when "addExceptionListItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const itemToCreate = getCreateExceptionListItemSchemaMock(); - const spyOnFetchExceptionListItemById = jest - .spyOn(api, 'addExceptionListItem') - .mockResolvedValue(payload); + 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(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - await result.current.addExceptionListItem({ - listItem: itemToCreate, + 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: AddExceptionListItemProps = { - http: mockKibanaHttpService, - listItem: itemToCreate, - signal: new AbortController().signal, - }; + test('invokes "onError" callback if "fetchExceptionListItemById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'fetchExceptionListItemById').mockRejectedValue(mockError); - expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + + await result.current.getExceptionItem({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); + + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); }); }); - test('it invokes "updateExceptionListItem" when "getExceptionItem" used', async () => { - const payload = getExceptionListItemSchemaMock(); - const itemToUpdate = getUpdateExceptionListItemSchemaMock(); - const spyOnUpdateExceptionListItem = jest - .spyOn(api, 'updateExceptionListItem') - .mockResolvedValue(payload); + 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(); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); - await result.current.updateExceptionListItem({ - listItem: itemToUpdate, + 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: UpdateExceptionListItemProps = { - http: mockKibanaHttpService, - listItem: itemToUpdate, - signal: new AbortController().signal, - }; + test('invokes "onError" callback if "fetchExceptionListById" fails', async () => { + const mockError = new Error('failed to delete item'); + jest.spyOn(api, 'fetchExceptionListById').mockRejectedValue(mockError); - expect(spyOnUpdateExceptionListItem).toHaveBeenCalledWith(expected); + await act(async () => { + const { result, waitForNextUpdate } = renderHook(() => + useApi(mockKibanaHttpService) + ); + await waitForNextUpdate(); + + const { id, namespace_type: namespaceType } = getExceptionListSchemaMock(); + + await result.current.getExceptionList({ + id, + namespaceType, + onError: onErrorMock, + onSuccess: jest.fn(), + }); + + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); + }); + }); + + 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, + }, + }); + }); + }); + + 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, + }); + + expect(onErrorMock).toHaveBeenCalledWith(mockError); + }); + }); + }); + + 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: getCreateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }; + + expect(spyOnFetchExceptionListItemById).toHaveBeenCalledWith(expected); + }); + }); + }); + + 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: getUpdateExceptionListItemSchemaMock(), + signal: new AbortController().signal, + }; + + 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 b0c831ef3b85..9e4e338b09db 100644 --- a/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts +++ b/x-pack/plugins/lists/public/exceptions/hooks/use_api.ts @@ -17,6 +17,7 @@ import { } from '../../../common/schemas'; import { ApiCallFindListsItemsMemoProps, ApiCallMemoProps, ApiListExportProps } from '../types'; import { getIdsAndNamespaces } from '../utils'; +import { transformInput, transformNewItemOutput, transformOutput } from '../transforms'; export interface ExceptionsApi { addExceptionListItem: (arg: { @@ -46,10 +47,11 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: CreateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); + const sanitizedItem: CreateExceptionListItemSchema = transformNewItemOutput(listItem); return Api.addExceptionListItem({ http, - listItem, + listItem: sanitizedItem, signal: abortCtrl.signal, }); }, @@ -124,12 +126,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); @@ -187,7 +191,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, @@ -214,10 +221,11 @@ export const useApi = (http: HttpStart): ExceptionsApi => { listItem: UpdateExceptionListItemSchema; }): Promise { const abortCtrl = new AbortController(); + const sanitizedItem: UpdateExceptionListItemSchema = transformOutput(listItem); return Api.updateExceptionListItem({ http, - listItem, + listItem: sanitizedItem, signal: abortCtrl.signal, }); }, 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 d5d263878187..1191b240d27b 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 50271530b42e..b9a8628d2cea 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,24 @@ export const useExceptionListItems = ({ signal: abortCtrl.signal, }); + // 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) { 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 000000000000..12b0f0bd8624 --- /dev/null +++ b/x-pack/plugins/lists/public/exceptions/transforms.test.ts @@ -0,0 +1,282 @@ +/* + * 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 { Entry, EntryMatch, EntryNested } from '../../common/schemas'; + +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: ExceptionListItemSchema = { + ...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', + } as Entry & { id: string }, + ], + }; + + expect(output).toEqual(expectedOutput); + }); + + it('returns same output as input with added ids per nested entry', () => { + const mockExceptionItem: ExceptionListItemSchema = { + ...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', + } as EntryMatch & { id: string }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + } as EntryNested & { id: string }, + ], + }; + + 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', + } as Entry & { id: string }, + ], + }; + 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', + } as EntryMatch & { id: string }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + } as EntryNested & { id: string }, + ], + }; + 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', + } as Entry & { id: string }, + ], + }; + 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', + } as EntryMatch & { id: string }, + ], + field: 'some.parentField', + id: '123', + type: 'nested', + } as EntryNested & { id: string }, + ], + }; + 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 000000000000..0791760611bf --- /dev/null +++ b/x-pack/plugins/lists/public/exceptions/transforms.ts @@ -0,0 +1,114 @@ +/* + * 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, + EntriesArray, + Entry, + 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: 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. + * + * 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: T +): T => { + const { entries } = exceptionItem; + const entriesNoId = entries.map((entry) => { + if (entry.type === 'nested') { + return removeIdFromItem({ + ...entry, + entries: entry.entries.map((nestedEntry) => removeIdFromItem(nestedEntry)), + }); + } else { + return removeIdFromItem(entry); + } + }); + 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 e37c03978c9f..03cae387711f 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/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 3e70361655c0..f10aaf45dcac 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/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 000000000000..154e90d509c6 --- /dev/null +++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/exceptions_modal.spec.ts @@ -0,0 +1,190 @@ +/* + * 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, + addExceptionEntryFieldValueOfItemX, + closeExceptionBuilderModal, +} from '../../tasks/exceptions'; +import { + ADD_AND_BTN, + ADD_OR_BTN, + ADD_NESTED_BTN, + ENTRY_DELETE_BTN, + FIELD_INPUT, + LOADING_SPINNER, + EXCEPTION_ITEM_CONTAINER, + ADD_EXCEPTIONS_BTN, +} 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(); + loginAndWaitForPageWithoutDateRange(DETECTIONS_URL); + waitForAlertsIndexToBeCreated(); + createCustomRule(newRule); + goToManageAlertsDetectionRules(); + goToRuleDetails(); + + 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(); + }); + + after(() => { + esArchiverUnload('exceptions'); + }); + + it('Does not overwrite values and-ed together', () => { + 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(); + 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', 'agent.name'); + cy.get(FIELD_INPUT).eq(1).should('have.text', 'c'); + + closeExceptionBuilderModal(); + }); + + 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(); + addExceptionEntryFieldValueOfItemX('user.id.keyword', 0, 1); + + // exception item 2 + cy.get(ADD_OR_BTN).click(); + addExceptionEntryFieldValueOfItemX('user.first', 1, 0); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValueOfItemX('user.last', 1, 1); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValueOfItemX('e', 1, 2); + + // delete single entry from exception item 2 + cy.get(ENTRY_DELETE_BTN).eq(3).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', '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(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 values of nested entry items', () => { + openExceptionModalFromRuleSettings(); + cy.get(LOADING_SPINNER).should('not.exist'); + + // exception item 1 + addExceptionEntryFieldValueOfItemX('agent.name', 0, 0); + cy.get(ADD_AND_BTN).click(); + addExceptionEntryFieldValueOfItemX('b', 0, 1); + + // exception item 2 with nested field + cy.get(ADD_OR_BTN).click(); + 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(); + }); +}); diff --git a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts index 7cd273b1db74..2479b76cf1de 100644 --- a/x-pack/plugins/security_solution/cypress/screens/exceptions.ts +++ b/x-pack/plugins/security_solution/cypress/screens/exceptions.ts @@ -24,6 +24,20 @@ 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"]'; + export const EXCEPTIONS_TABLE_TAB = '[data-test-subj="allRulesTableTab-exceptions"]'; export const EXCEPTIONS_TABLE = '[data-test-subj="exceptions-table"]'; @@ -43,3 +57,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 new file mode 100644 index 000000000000..97e93ef8194a --- /dev/null +++ b/x-pack/plugins/security_solution/cypress/tasks/exceptions.ts @@ -0,0 +1,62 @@ +/* + * 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, + 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(); +}; + +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 57037e9f269b..411f326a0ace 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts @@ -54,6 +54,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 b083ecbafeb2..3a2170d126a2 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} + { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { wrapper = mount( { }).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 ); }); @@ -445,6 +456,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { }).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 ); }); @@ -480,6 +492,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { }).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 ); }); @@ -515,6 +528,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { }).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 ); }); @@ -550,6 +564,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { expect(mockOnChange).toHaveBeenCalledWith( { + id: '123', field: 'ip', operator: 'excluded', type: 'list', @@ -590,6 +606,7 @@ describe('BuilderEntryItem', () => { wrapper = mount( { wrapper = mount( = ({ ); } 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 402496ef00f6..f9afa48408e3 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.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/builder/helpers.test.tsx index ea948ab9b3b5..8d0f042e7a49 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, EntryNested } from '../../../../lists_plugin_deps'; +import { + EntryMatch, + EntryMatchAny, + EntryNested, + EntryList, + EntryExists, + OperatorTypeEnum, + OperatorEnum, +} from '../../../../shared_imports'; import { getEntryFromOperator, @@ -46,6 +54,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', @@ -54,6 +87,7 @@ const getMockIndexPattern = (): IIndexPattern => ({ }); const getMockBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: getField('ip'), operator: isOperator, value: 'some value', @@ -64,15 +98,16 @@ const getMockBuilderEntry = (): FormattedBuilderEntry => ({ }); const getMockNestedBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: getField('nestedField.child'), operator: isOperator, value: 'some value', nested: 'child', parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'nestedField', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }, parentIndex: 0, }, @@ -81,6 +116,7 @@ const getMockNestedBuilderEntry = (): FormattedBuilderEntry => ({ }); const getMockNestedParentBuilderEntry = (): FormattedBuilderEntry => ({ + id: '123', field: { ...getField('nestedField.child'), name: 'nestedField', esTypes: ['nested'] }, operator: isOperator, value: undefined, @@ -225,15 +261,16 @@ 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', nested: 'child', parent: { parent: { - ...getEntryNestedMock(), + ...getEntryNestedWithIdMock(), field: 'file.Ext.code_signature', - entries: [{ ...getEntryMatchMock(), field: 'child' }], + entries: [{ ...getEntryMatchWithIdMock(), field: 'child' }], }, parentIndex: 0, }, @@ -351,7 +388,7 @@ describe('Exception builder helpers', () => { ], }; const payloadItem: BuilderEntry = { - ...getEntryMatchMock(), + ...getEntryMatchWithIdMock(), field: 'machine.os.raw.text', value: 'some os', }; @@ -363,6 +400,7 @@ describe('Exception builder helpers', () => { undefined ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { name: 'machine.os.raw.text', @@ -385,11 +423,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, @@ -399,6 +437,7 @@ describe('Exception builder helpers', () => { 1 ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { aggregatable: false, @@ -419,9 +458,10 @@ describe('Exception builder helpers', () => { operator: isOperator, parent: { parent: { + id: '123', entries: [{ ...payloadItem }], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, parentIndex: 1, }, @@ -433,7 +473,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, @@ -442,6 +486,7 @@ describe('Exception builder helpers', () => { undefined ); const expected: FormattedBuilderEntry = { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -465,14 +510,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); @@ -482,10 +527,11 @@ 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[] = [ { + id: '123', entryIndex: 0, field: undefined, nested: undefined, @@ -501,12 +547,13 @@ 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[] = [ { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -525,6 +572,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 1, field: { aggregatable: true, @@ -549,18 +597,19 @@ 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 }, ]; const output = getFormattedBuilderEntries(payloadIndexPattern, payloadItems); const expected: FormattedBuilderEntry[] = [ { + id: '123', entryIndex: 0, field: { aggregatable: true, @@ -579,6 +628,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 1, field: { aggregatable: false, @@ -594,6 +644,7 @@ describe('Exception builder helpers', () => { correspondingKeywordField: undefined, }, { + id: '123', entryIndex: 0, field: { aggregatable: false, @@ -614,16 +665,18 @@ describe('Exception builder helpers', () => { operator: isOperator, parent: { parent: { + id: '123', entries: [ { + id: '123', field: 'child', - operator: 'included', - type: 'match', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some host name', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, parentIndex: 1, }, @@ -637,15 +690,19 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some value', }, ], @@ -658,15 +715,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); }); @@ -676,8 +735,8 @@ describe('Exception builder helpers', () => { ...getExceptionListItemSchemaMock(), entries: [ { - ...getEntryNestedMock(), - entries: [{ ...getEntryExistsMock() }], + ...getEntryNestedWithIdMock(), + entries: [{ ...getEntryExistsWithIdMock() }], }, ], }; @@ -698,10 +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); @@ -715,10 +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); @@ -732,10 +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); @@ -749,10 +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); @@ -766,10 +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); @@ -783,10 +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); @@ -799,7 +864,8 @@ 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', type: 'exists', @@ -814,9 +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); @@ -830,9 +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); @@ -846,9 +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' }, }; @@ -943,12 +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; 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: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, + value: '', + }, + ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -959,24 +1037,34 @@ 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: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, + value: '', + }, + getEntryMatchAnyWithIdMock(), ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -986,12 +1074,13 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: '', }, }; @@ -1004,8 +1093,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: OperatorTypeEnum.MATCH, + value: 'some value', + operator: 'excluded', + }, index: 0, }; expect(output).toEqual(expected); @@ -1015,8 +1110,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: OperatorTypeEnum.MATCH_ANY, + value: [], + operator: OperatorEnum.INCLUDED, + }, index: 0, }; expect(output).toEqual(expected); @@ -1026,19 +1127,21 @@ 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', + operator: OperatorEnum.EXCLUDED, + type: OperatorTypeEnum.MATCH, value: 'some value', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1048,19 +1151,21 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: [], }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1071,8 +1176,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: OperatorTypeEnum.MATCH, + value: 'jibber jabber', + operator: OperatorEnum.INCLUDED, + }, index: 0, }; expect(output).toEqual(expected); @@ -1081,8 +1192,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: OperatorTypeEnum.MATCH, + value: 'jibber jabber', + operator: OperatorEnum.INCLUDED, + }, index: 0, }; expect(output).toEqual(expected); @@ -1091,19 +1208,21 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1112,19 +1231,21 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH, value: 'jibber jabber', }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1139,12 +1260,13 @@ 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', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1159,12 +1281,13 @@ 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', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1176,27 +1299,29 @@ 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', + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], + operator: OperatorEnum.INCLUDED, }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1208,27 +1333,29 @@ 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', + operator: OperatorEnum.INCLUDED, + type: OperatorTypeEnum.MATCH_ANY, value: ['jibber jabber'], }, ], field: 'nestedField', - type: 'nested', + type: OperatorTypeEnum.NESTED, }, }; expect(output).toEqual(expected); @@ -1243,12 +1370,13 @@ 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' }, - operator: 'included', + operator: OperatorEnum.INCLUDED, }, index: 0, }; @@ -1263,12 +1391,13 @@ 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' }, - 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 a08f869b41d6..8afdbce68c69 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,15 +5,15 @@ * 2.0. */ +import uuid from 'uuid'; + +import { addIdToItem } from '../../../../../common/add_remove_id_to_item'; import { IIndexPattern, IFieldType } from '../../../../../../../../src/plugins/data/common'; import { Entry, OperatorTypeEnum, EntryNested, ExceptionListType, - EntryMatch, - EntryMatchAny, - EntryExists, entriesList, ListSchema, OperatorEnum, @@ -160,6 +160,7 @@ export const getFormattedBuilderEntry = ( ? { ...foundField, name: foundField.name.split('.').slice(-1)[0] } : foundField, correspondingKeywordField, + id: item.id ?? `${itemIndex}`, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), nested: 'child', @@ -169,6 +170,7 @@ export const getFormattedBuilderEntry = ( } else { return { field: foundField, + id: item.id ?? `${itemIndex}`, correspondingKeywordField, operator: getExceptionOperatorSelect(item), value: getEntryValue(item), @@ -215,6 +217,7 @@ export const getFormattedBuilderEntries = ( } else { const parentEntry: FormattedBuilderEntry = { operator: isOperator, + id: item.id ?? `${index}`, nested: 'parent', field: isNewNestedEntry ? undefined @@ -265,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), ]; @@ -282,6 +285,7 @@ export const getUpdatedEntriesOnDelete = ( const { field } = itemOfInterest; const updatedItemOfInterest: EntryNested | EmptyNestedEntry = { field, + id: itemOfInterest.id ?? `${entryIndex}`, type: OperatorTypeEnum.NESTED, entries: updatedEntryEntries, }; @@ -317,12 +321,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, @@ -331,6 +336,7 @@ export const getEntryFromOperator = ( }; case 'match_any': return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.MATCH_ANY, operator: selectedOperator.operator, @@ -338,6 +344,7 @@ export const getEntryFromOperator = ( }; case 'list': return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.LIST, operator: selectedOperator.operator, @@ -345,6 +352,7 @@ export const getEntryFromOperator = ( }; default: return { + id: currentEntry.id, field: fieldValue, type: OperatorTypeEnum.EXISTS, operator: selectedOperator.operator, @@ -397,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" @@ -408,15 +416,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, @@ -428,6 +437,7 @@ export const getEntryOnFieldChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: newChildFieldValue ?? '', type: OperatorTypeEnum.MATCH, operator: isOperator.operator, @@ -441,6 +451,7 @@ export const getEntryOnFieldChange = ( } else { return { updatedEntry: { + id: item.id, field: newField != null ? newField.name : '', type: OperatorTypeEnum.MATCH, operator: isOperator.operator, @@ -508,6 +519,7 @@ export const getEntryOnMatchChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: fieldName, type: OperatorTypeEnum.MATCH, operator: operator.operator, @@ -521,6 +533,7 @@ export const getEntryOnMatchChange = ( } else { return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.MATCH, operator: operator.operator, @@ -554,6 +567,7 @@ export const getEntryOnMatchAnyChange = ( entries: [ ...parent.parent.entries.slice(0, entryIndex), { + id: item.id, field: fieldName, type: OperatorTypeEnum.MATCH_ANY, operator: operator.operator, @@ -567,6 +581,7 @@ export const getEntryOnMatchAnyChange = ( } else { return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.MATCH_ANY, operator: operator.operator, @@ -594,6 +609,7 @@ export const getEntryOnListChange = ( return { updatedEntry: { + id: item.id, field: field != null ? field.name : '', type: OperatorTypeEnum.LIST, operator: operator.operator, @@ -604,6 +620,7 @@ export const getEntryOnListChange = ( }; export const getDefaultEmptyEntry = (): EmptyEntry => ({ + id: uuid.v4(), field: '', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, @@ -611,6 +628,7 @@ export const getDefaultEmptyEntry = (): EmptyEntry => ({ }); export const getDefaultNestedEmptyEntry = (): EmptyNestedEntry => ({ + id: uuid.v4(), field: '', type: OperatorTypeEnum.NESTED, entries: [], 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 46e3ec08e7c5..3789d8e75fa2 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: '', - }, + }), ], }, ], @@ -352,7 +351,7 @@ export const ExceptionBuilderComponent = ({ }, []); return ( - + {exceptions.map((exceptionListItem, index) => ( 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 0741f561c193..dbac7d325b63 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/edit_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx index 336732016e93..954a75fc370b 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.tsx @@ -382,7 +382,9 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} {updateError == null && ( - {i18n.CANCEL} + + {i18n.CANCEL} + ({ + v4: jest.fn().mockReturnValue('123'), +})); + describe('Exception helpers', () => { beforeEach(() => { moment.tz.setDefault('UTC'); @@ -229,9 +237,22 @@ 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 = { + id: '123', field: 'host.name', type: OperatorTypeEnum.MATCH, operator: OperatorEnum.INCLUDED, @@ -250,6 +271,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, @@ -270,6 +292,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, @@ -290,6 +313,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 13ee06e8cbac..c44de4f05e7f 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 @@ -44,6 +44,7 @@ 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 @@ -150,12 +151,12 @@ export const getNewExceptionItem = ({ comments: [], description: `${ruleName} - exception list item`, entries: [ - { + addIdToItem({ field: '', operator: 'included', type: 'match', value: '', - }, + }), ], item_id: undefined, list_id: listId, @@ -175,26 +176,32 @@ export const filterExceptionItems = ( return exceptions.reduce>( (acc, exception) => { const entries = exception.entries.reduce((nestedAcc, singleEntry) => { - if (singleEntry.type === 'nested') { - const nestedEntriesArray = singleEntry.entries.filter((singleNestedEntry) => { - const [validatedNestedEntry] = validate(singleNestedEntry, nestedEntryItem); + const strippedSingleEntry = removeIdFromItem(singleEntry); + + if (entriesNested.is(strippedSingleEntry)) { + const nestedEntriesArray = strippedSingleEntry.entries.filter((singleNestedEntry) => { + const noIdSingleNestedEntry = removeIdFromItem(singleNestedEntry); + const [validatedNestedEntry] = validate(noIdSingleNestedEntry, nestedEntryItem); return validatedNestedEntry != null; }); + const noIdNestedEntries = nestedEntriesArray.map((singleNestedEntry) => + removeIdFromItem(singleNestedEntry) + ); const [validatedNestedEntry] = validate( - { ...singleEntry, entries: nestedEntriesArray }, + { ...strippedSingleEntry, entries: noIdNestedEntries }, entriesNested ); if (validatedNestedEntry != null) { - return [...nestedAcc, validatedNestedEntry]; + return [...nestedAcc, { ...singleEntry, entries: nestedEntriesArray }]; } return nestedAcc; } else { - const [validatedEntry] = validate(singleEntry, entry); + const [validatedEntry] = validate(strippedSingleEntry, entry); if (validatedEntry != null) { - return [...nestedAcc, validatedEntry]; + return [...nestedAcc, singleEntry]; } return nestedAcc; } 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 6108a21ce562..c7a125daa54f 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 @@ -60,16 +60,18 @@ export interface ExceptionsPagination { } export interface FormattedBuilderEntry { + id: string; field: IFieldType | undefined; operator: OperatorOption; 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; } export interface EmptyEntry { + id: string; field: string | undefined; operator: OperatorEnum; type: OperatorTypeEnum.MATCH | OperatorTypeEnum.MATCH_ANY; @@ -77,6 +79,7 @@ export interface EmptyEntry { } export interface EmptyListEntry { + id: string; field: string | undefined; operator: OperatorEnum; type: OperatorTypeEnum.LIST; @@ -84,12 +87,31 @@ export interface EmptyListEntry { } 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/plugins/security_solution/public/common/components/threat_match/helpers.tsx b/x-pack/plugins/security_solution/public/common/components/threat_match/helpers.tsx index eced8d785792..ef3e9280e6e6 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 45961454b9c7..7eb91e259a72 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, 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 000000000000..b7de2dba02d1 --- /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 000000000000..e63b86392756 --- /dev/null +++ b/x-pack/test/security_solution_cypress/es_archives/exceptions/mappings.json @@ -0,0 +1,42 @@ +{ + "type": "index", + "value": { + "aliases": { + "exceptions": { + "is_write_index": false + } + }, + "settings": { + "index": { + "refresh_interval": "5s" + } + }, + "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" + } + } + } + } + } + } +}