[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';
@@ -69,21 +70,54 @@ 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,
         ReturnPersistExceptionItem
       >(() => 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,
+      });
     });
   });
 

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<AddExceptionListItem | null>
+  Dispatch<CreateExceptionListItemSchema | UpdateExceptionListItemSchema | null>
 ];
 
 /**
@@ -32,7 +36,9 @@ export const usePersistExceptionItem = ({
   http,
   onError,
 }: PersistHookProps): ReturnPersistExceptionItem => {
-  const [exceptionListItem, setExceptionItem] = useState<AddExceptionListItem | null>(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,
             });
           }