[Security Solution][Exceptions] - Fixes exceptions builder UI where invalid values can cause overwrites of other values

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 = [];

x-pack/plugins/lists/common/shared_imports.ts

@@ -12,6 +12,8 @@ export {
   DefaultStringArray,
   DefaultVersionNumber,
   DefaultVersionNumberDecoded,
+  addIdToItem,
+  removeIdFromItem,
   exactCheck,
   getPaths,
   foldLeftRight,

x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.test.ts

@@ -7,6 +7,7 @@
 
 import { act, renderHook } from '@testing-library/react-hooks';
 
+import { ENTRIES_WITH_IDS } from '../../../common/constants.mock';
 import { coreMock } from '../../../../../../src/core/public/mocks';
 import * as api from '../api';
 import { getCreateExceptionListItemSchemaMock } from '../../../common/schemas/request/create_exception_list_item_schema.mock';
@@ -78,12 +79,45 @@ describe('usePersistExceptionItem', () => {
       >(() => usePersistExceptionItem({ http: mockKibanaHttpService, onError }));
 
       await waitForNextUpdate();
-      result.current[1](getUpdateExceptionListItemSchemaMock());
+      // NOTE: Take note here passing in an exception item where it's
+      // entries have been enriched with ids to ensure that they get stripped
+      // before the call goes through
+      result.current[1]({ ...getUpdateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS });
       await waitForNextUpdate();
 
       expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]);
       expect(addExceptionItem).not.toHaveBeenCalled();
-      expect(updateExceptionItem).toHaveBeenCalled();
+      expect(updateExceptionItem).toHaveBeenCalledWith({
+        http: mockKibanaHttpService,
+        listItem: getUpdateExceptionListItemSchemaMock(),
+        signal: new AbortController().signal,
+      });
+    });
+  });
+
+  test('it invokes "addExceptionListItem" when payload has "id"', async () => {
+    const updateExceptionItem = jest.spyOn(api, 'updateExceptionListItem');
+    const createExceptionItem = jest.spyOn(api, 'addExceptionListItem');
+    await act(async () => {
+      const { result, waitForNextUpdate } = renderHook<
+        PersistHookProps,
+        ReturnPersistExceptionItem
+      >(() => usePersistExceptionItem({ http: mockKibanaHttpService, onError }));
+
+      await waitForNextUpdate();
+      // NOTE: Take note here passing in an exception item where it's
+      // entries have been enriched with ids to ensure that they get stripped
+      // before the call goes through
+      result.current[1]({ ...getCreateExceptionListItemSchemaMock(), entries: ENTRIES_WITH_IDS });
+      await waitForNextUpdate();
+
+      expect(result.current).toEqual([{ isLoading: false, isSaved: true }, result.current[1]]);
+      expect(updateExceptionItem).not.toHaveBeenCalled();
+      expect(createExceptionItem).toHaveBeenCalledWith({
+        http: mockKibanaHttpService,
+        listItem: getCreateExceptionListItemSchemaMock(),
+        signal: new AbortController().signal,
+      });
     });
   });
 

x-pack/plugins/lists/public/exceptions/hooks/persist_exception_item.ts

@@ -9,6 +9,7 @@ import { Dispatch, useEffect, useState } from 'react';
 
 import { UpdateExceptionListItemSchema } from '../../../common/schemas';
 import { addExceptionListItem, updateExceptionListItem } from '../api';
+import { transformOutput } from '../transforms';
 import { AddExceptionListItem, PersistHookProps } from '../types';
 
 interface PersistReturnExceptionItem {
@@ -47,16 +48,20 @@ 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);
+
+          if (isUpdateExceptionItem(transformedList)) {
             await updateExceptionListItem({
               http,
-              listItem: exceptionListItem,
+              listItem: transformedList,
               signal: abortCtrl.signal,
             });
           } else {
             await addExceptionListItem({
               http,
-              listItem: exceptionListItem,
+              listItem: transformedList,
               signal: abortCtrl.signal,
             });
           }