[Security Solution][Exceptions] - Fix bug where fields not showing up when editing exceptions (#137138)

## Summary Addresses https://github.com/elastic/kibana/issues/137030 User can now edit exceptions. Added a cypress test to hopefully catch such an issue in the future.
2025-04-24 01:38:56 -04:00 · 2022-07-28 12:46:09 -07:00 · 2022-07-28 12:46:09 -07:00 · 2d2dd15139
commit 2d2dd15139
parent 7778027cd7
4 changed files with 133 additions and 56 deletions
--- a/x-pack/plugins/security_solution/cypress/integration/exceptions/add_edit_exception.spec.ts
+++ b/x-pack/plugins/security_solution/cypress/integration/exceptions/add_edit_exception.spec.ts
@ -19,6 +19,7 @@ import { login, visitWithoutDateRange } from '../../tasks/login';
 import {
  addsException,
  addsExceptionFromRuleSettings,
+  editException,
  goToAlertsTab,
  goToExceptionsTab,
  removeException,
@ -27,6 +28,15 @@ import {

 import { DETECTIONS_RULE_MANAGEMENT_URL } from '../../urls/navigation';
 import { deleteAlertsAndRules } from '../../tasks/common';
+import {
+  EXCEPTION_EDIT_FLYOUT_SAVE_BTN,
+  EXCEPTION_ITEM_CONTAINER,
+  FIELD_INPUT,
+} from '../../screens/exceptions';
+import {
+  addExceptionEntryFieldValueOfItemX,
+  addExceptionEntryFieldValueValue,
+} from '../../tasks/exceptions';

 describe('Adds rule exception', () => {
  const NUMBER_OF_AUDITBEAT_EXCEPTIONS_ALERTS = '1 alert';
@ -122,4 +132,26 @@ describe('Adds rule exception', () => {
    cy.get(ALERTS_COUNT).should('exist');
    cy.get(NUMBER_OF_ALERTS).should('have.text', `${NUMBER_OF_AUDITBEAT_EXCEPTIONS_ALERTS}`);
  });
+
+  it('Edits an exception', () => {
+    goToExceptionsTab();
+    addsExceptionFromRuleSettings(getException());
+
+    editException();
+
+    // check that the existing item's field is being populated
+    cy.get(EXCEPTION_ITEM_CONTAINER)
+      .eq(0)
+      .find(FIELD_INPUT)
+      .eq(0)
+      .should('have.text', 'agent.name');
+
+    // check that you can select a different field
+    addExceptionEntryFieldValueOfItemX('user.name{downarrow}{enter}', 0, 0);
+    addExceptionEntryFieldValueValue('test', 0);
+
+    cy.get(EXCEPTION_EDIT_FLYOUT_SAVE_BTN).click();
+    cy.get(EXCEPTION_EDIT_FLYOUT_SAVE_BTN).should('have.attr', 'disabled');
+    cy.get(EXCEPTION_EDIT_FLYOUT_SAVE_BTN).should('not.exist');
+  });
 });
--- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_flyout/index.test.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_flyout/index.test.tsx
@ -8,20 +8,20 @@
 import React from 'react';
 import type { ReactWrapper } from 'enzyme';
 import { mount } from 'enzyme';
-import { act, waitFor } from '@testing-library/react';
+import { waitFor } from '@testing-library/react';

 import { AddExceptionFlyout } from '.';
 import { getExceptionBuilderComponentLazy } from '@kbn/lists-plugin/public';
 import { useAsync } from '@kbn/securitysolution-hook-utils';
 import { getExceptionListSchemaMock } from '@kbn/lists-plugin/common/schemas/response/exception_list_schema.mock';
 import { useFetchIndex } from '../../../containers/source';
-import { createIndexPatternFieldStub, stubIndexPattern } from '@kbn/data-plugin/common/stubs';
+import { createStubIndexPattern, stubIndexPattern } from '@kbn/data-plugin/common/stubs';
 import { useAddOrUpdateException } from '../use_add_exception';
 import { useFetchOrCreateRuleExceptionList } from '../use_fetch_or_create_rule_exception_list';
 import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index';
 import * as helpers from '../helpers';
 import { getExceptionListItemSchemaMock } from '@kbn/lists-plugin/common/schemas/response/exception_list_item_schema.mock';
-import type { EntriesArray, ExceptionListItemSchema } from '@kbn/securitysolution-io-ts-list-types';
+import type { EntriesArray } from '@kbn/securitysolution-io-ts-list-types';

 import { TestProviders } from '../../../mock';

@ -346,52 +346,74 @@ describe('When the add exception modal is opened', () => {

  describe('when there is bulk-closeable alert data passed to an endpoint list exception', () => {
    let wrapper: ReactWrapper;
-    const stubbed = [
-      { name: 'file.path.caseless', type: 'string', aggregatable: true, searchable: true },
-      { name: 'subject_name', type: 'string', aggregatable: true, searchable: true },
-      { name: 'trusted', type: 'string', aggregatable: true, searchable: true },
-      { name: 'file.hash.sha256', type: 'string', aggregatable: true, searchable: true },
-      { name: 'event.code', type: 'string', aggregatable: true, searchable: true },
-    ].map((item) => createIndexPatternFieldStub({ spec: item }));
-    let callProps: {
-      onChange: (props: { exceptionItems: ExceptionListItemSchema[] }) => void;
-      exceptionListItems: ExceptionListItemSchema[];
-    };
+
    beforeEach(async () => {
-      const stubbedIndexPattern = stubIndexPattern;
-      // Mocks the index patterns to contain the pre-populated endpoint fields so that the exception qualifies as bulk closable
      mockUseFetchIndex.mockImplementation(() => [
        false,
        {
-          indexPatterns: {
-            ...stubbedIndexPattern,
-            fields: stubbed,
-          },
+          indexPatterns: createStubIndexPattern({
+            spec: {
+              id: '1234',
+              title: 'filebeat-*',
+              fields: {
+                'event.code': {
+                  name: 'event.code',
+                  type: 'string',
+                  aggregatable: true,
+                  searchable: true,
+                },
+                'file.path.caseless': {
+                  name: 'file.path.caseless',
+                  type: 'string',
+                  aggregatable: true,
+                  searchable: true,
+                },
+                subject_name: {
+                  name: 'subject_name',
+                  type: 'string',
+                  aggregatable: true,
+                  searchable: true,
+                },
+                trusted: {
+                  name: 'trusted',
+                  type: 'string',
+                  aggregatable: true,
+                  searchable: true,
+                },
+                'file.hash.sha256': {
+                  name: 'file.hash.sha256',
+                  type: 'string',
+                  aggregatable: true,
+                  searchable: true,
+                },
+              },
+            },
+          }),
        },
      ]);
+
      const alertDataMock: AlertData = {
        '@timestamp': '1234567890',
        _id: 'test-id',
        file: { path: 'test/path' },
      };
-      await act(async () => {
-        wrapper = mount(
-          <TestProviders>
-            <AddExceptionFlyout
-              ruleId={'123'}
-              ruleIndices={['filebeat-*']}
-              ruleName={ruleName}
-              exceptionListType={'endpoint'}
-              onCancel={jest.fn()}
-              onConfirm={jest.fn()}
-              alertData={alertDataMock}
-            />
-          </TestProviders>
-        );
-      });
-      await waitFor(() => {
-        callProps = mockGetExceptionBuilderComponentLazy.mock.calls[0][0];
+      wrapper = mount(
+        <TestProviders>
+          <AddExceptionFlyout
+            ruleId={'123'}
+            ruleIndices={['filebeat-*']}
+            ruleName={ruleName}
+            exceptionListType={'endpoint'}
+            onCancel={jest.fn()}
+            onConfirm={jest.fn()}
+            alertData={alertDataMock}
+            isAlertDataLoading={false}
+          />
+        </TestProviders>
+      );

+      const callProps = mockGetExceptionBuilderComponentLazy.mock.calls[0][0];
+      await waitFor(() => {
        return callProps.onChange({ exceptionItems: [...callProps.exceptionListItems] });
      });
    });
@ -423,6 +445,8 @@ describe('When the add exception modal is opened', () => {
    });
    describe('when a "is in list" entry is added', () => {
      it('should have the bulk close checkbox disabled', async () => {
+        const callProps = mockGetExceptionBuilderComponentLazy.mock.calls[0][0];
+
        await waitFor(() =>
          callProps.onChange({
            exceptionItems: [
--- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_flyout/index.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_flyout/index.tsx
@ -5,7 +5,9 @@
 * 2.0.
 */

-/* eslint complexity: ["error", 30]*/
+// Component being re-implemented in 8.5
+
+/* eslint complexity: ["error", 35]*/

 import React, { memo, useEffect, useState, useCallback, useMemo } from 'react';
 import styled, { css } from 'styled-components';
@ -164,26 +166,28 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
    maybeRule?.machine_learning_job_id,
    ruleIndices
  );
-
-  const [isIndexPatternLoading, { indexPatterns: indexIndexPatterns }] =
-    useFetchIndex(memoRuleIndices);
-
-  const [indexPattern, setIndexPattern] = useState<DataViewBase>(indexIndexPatterns);
+  const hasDataViewId = dataViewId || maybeRule?.data_view_id || null;
+  const [dataViewIndexPatterns, setDataViewIndexPatterns] = useState<DataViewBase | null>(null);

  useEffect(() => {
    const fetchSingleDataView = async () => {
-      const hasDataViewId = dataViewId || maybeRule?.data_view_id || null;
      if (hasDataViewId) {
        const dv = await data.dataViews.get(hasDataViewId);
-        setIndexPattern(dv);
+        setDataViewIndexPatterns(dv);
      }
    };

    fetchSingleDataView();
-  }, [data.dataViews, dataViewId, maybeRule?.data_view_id, setIndexPattern]);
+  }, [hasDataViewId, data.dataViews, setDataViewIndexPatterns]);

-  const selectedIndexPattern =
-    dataViewId || maybeRule?.data_view_id ? indexPattern : indexIndexPatterns;
+  const [isIndexPatternLoading, { indexPatterns: indexIndexPatterns }] = useFetchIndex(
+    hasDataViewId ? [] : memoRuleIndices
+  );
+
+  const indexPattern = useMemo(
+    (): DataViewBase | null => (hasDataViewId ? dataViewIndexPatterns : indexIndexPatterns),
+    [hasDataViewId, dataViewIndexPatterns, indexIndexPatterns]
+  );

  const handleBuilderOnChange = useCallback(
    ({
@ -481,6 +485,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
          <Loader data-test-subj="loadingAddExceptionFlyout" size="xl" />
        )}
      {fetchOrCreateListError == null &&
+        indexPattern != null &&
        !isSignalIndexLoading &&
        !isSignalIndexPatternLoading &&
        !isLoadingExceptionList &&
@ -532,7 +537,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
                listNamespaceType: ruleExceptionList.namespace_type,
                listTypeSpecificIndexPatternFilter: filterIndexPatterns,
                ruleName,
-                indexPatterns: selectedIndexPattern,
+                indexPatterns: indexPattern,
                isOrDisabled: isExceptionBuilderFormDisabled,
                isAndDisabled: isExceptionBuilderFormDisabled,
                isNestedDisabled: isExceptionBuilderFormDisabled,
--- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_flyout/index.tsx
+++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_flyout/index.tsx
@ -5,6 +5,10 @@
 * 2.0.
 */

+// Component being re-implemented in 8.5
+
+/* eslint-disable complexity */
+
 import React, { memo, useState, useCallback, useEffect, useMemo } from 'react';
 import styled, { css } from 'styled-components';
 import {
@ -143,20 +147,31 @@ export const EditExceptionFlyout = memo(function EditExceptionFlyout({
    ruleIndices
  );

-  const [isIndexPatternLoading, { indexPatterns: indexIndexPatterns }] =
-    useFetchIndex(memoRuleIndices);
-  const [indexPattern, setIndexPattern] = useState<DataViewBase>(indexIndexPatterns);
+  const hasDataViewId = dataViewId || maybeRule?.data_view_id || null;
+  const [dataViewIndexPatterns, setDataViewIndexPatterns] = useState<DataViewBase | null>(null);

  useEffect(() => {
    const fetchSingleDataView = async () => {
-      if (dataViewId != null && dataViewId !== '') {
-        const dv = await data.dataViews.get(dataViewId);
-        setIndexPattern(dv);
+      if (hasDataViewId) {
+        const dv = await data.dataViews.get(hasDataViewId);
+        setDataViewIndexPatterns(dv);
      }
    };

    fetchSingleDataView();
-  }, [data.dataViews, dataViewId, setIndexPattern]);
+  }, [hasDataViewId, data.dataViews, setDataViewIndexPatterns]);
+
+  // Don't fetch indices if rule has data view id (currently rule can technically have
+  // both defined and in that case we'd be doing unnecessary work here if all we want is
+  // the data view fields)
+  const [isIndexPatternLoading, { indexPatterns: indexIndexPatterns }] = useFetchIndex(
+    hasDataViewId ? [] : memoRuleIndices
+  );
+
+  const indexPattern = useMemo(
+    (): DataViewBase | null => (hasDataViewId ? dataViewIndexPatterns : indexIndexPatterns),
+    [hasDataViewId, dataViewIndexPatterns, indexIndexPatterns]
+  );

  const handleExceptionUpdateError = useCallback(
    (error: Error, statusCode: number | null, message: string | null) => {
@ -341,6 +356,7 @@ export const EditExceptionFlyout = memo(function EditExceptionFlyout({
        <Loader data-test-subj="loadingEditExceptionFlyout" size="xl" />
      )}
      {!isSignalIndexLoading &&
+        indexPattern != null &&
        !addExceptionIsLoading &&
        !isIndexPatternLoading &&
        !isRuleLoading &&