[Security Solution] Unfriendly error message when creating an invalid…

… exception (#168213)
elastic · Nov 7, 2023 · fd340f5 · fd340f5
1 parent 3b4a901
commit fd340f5
Show file tree

Hide file tree

Showing 12 changed files with 292 additions and 13 deletions.
diff --git a/...on/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.test.tsx b/...on/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.test.tsx
@@ -8,7 +8,7 @@
 import React from 'react';
 import type { ReactWrapper } from 'enzyme';
 import { mount, shallow } from 'enzyme';
-import { waitFor, render } from '@testing-library/react';
+import { act, fireEvent, render, waitFor } from '@testing-library/react';
 
 import { getExceptionListSchemaMock } from '@kbn/lists-plugin/common/schemas/response/exception_list_schema.mock';
 import { getExceptionBuilderComponentLazy } from '@kbn/lists-plugin/public';
@@ -35,6 +35,7 @@ import {
 import type { AlertData } from '../../utils/types';
 import { useFindRules } from '../../../rule_management/logic/use_find_rules';
 import { useFindExceptionListReferences } from '../../logic/use_find_references';
+import { MAX_COMMENT_LENGTH } from '../constants';
 
 jest.mock('../../../../detections/containers/detection_engine/alerts/use_signal_index');
 jest.mock('../../../../common/lib/kibana');
@@ -1305,5 +1306,46 @@ describe('When the add exception modal is opened', () => {
         wrapper.find('button[data-test-subj="addExceptionConfirmButton"]').getDOMNode()
       ).toBeDisabled();
     });
+
+    test('when there is a comment error has submit button disabled', async () => {
+      const { getByLabelText, queryByText, getByTestId } = render(
+        <TestProviders>
+          <AddExceptionFlyout
+            rules={[
+              {
+                ...getRulesEqlSchemaMock(),
+                query:
+                  'sequence [process where process.name = "test.exe"] [process where process.name = "explorer.exe"]',
+              } as Rule,
+            ]}
+            isBulkAction={false}
+            alertData={alertDataMock}
+            isAlertDataLoading={false}
+            alertStatus="open"
+            isEndpointItem={false}
+            showAlertCloseOptions
+            onCancel={jest.fn()}
+            onConfirm={jest.fn()}
+          />
+        </TestProviders>
+      );
+
+      const commentInput = getByLabelText('Comment Input');
+
+      const commentErrorMessage = `The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH} characters.`;
+      expect(queryByText(commentErrorMessage)).toBeNull();
+
+      // Put comment with the length above maximum allowed
+      act(() => {
+        fireEvent.change(commentInput, {
+          target: {
+            value: [...new Array(MAX_COMMENT_LENGTH + 1).keys()].map((_) => 'a').join(''),
+          },
+        });
+        fireEvent.blur(commentInput);
+      });
+      expect(queryByText(commentErrorMessage)).not.toBeNull();
+      expect(getByTestId('addExceptionConfirmButton')).toBeDisabled();
+    });
   });
 });
diff --git a/...olution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx b/...olution/public/detection_engine/rule_exceptions/components/add_exception_flyout/index.tsx
@@ -157,6 +157,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       selectedRulesToAddTo,
       exceptionListsToAddTo,
       newComment,
+      commentErrorExists,
       itemConditionValidationErrorExists,
       errorSubmitting,
       expireTime,
@@ -267,6 +268,16 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
     [dispatch]
   );
 
+  const setCommentError = useCallback(
+    (errorExists: boolean): void => {
+      dispatch({
+        type: 'setCommentError',
+        errorExists,
+      });
+    },
+    [dispatch]
+  );
+
   const setBulkCloseIndex = useCallback(
     (index: string[] | undefined): void => {
       dispatch({
@@ -445,6 +456,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       exceptionItemName.trim() === '' ||
       exceptionItems.every((item) => item.entries.length === 0) ||
       itemConditionValidationErrorExists ||
+      commentErrorExists ||
       expireErrorExists ||
       (addExceptionToRadioSelection === 'add_to_lists' && isEmpty(exceptionListsToAddTo)) ||
       (addExceptionToRadioSelection === 'select_rules_to_add_to' &&
@@ -462,6 +474,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
       expireErrorExists,
       selectedRulesToAddTo,
       listType,
+      commentErrorExists,
     ]
   );
 
@@ -555,6 +568,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
             initialIsOpen={!!newComment}
             newCommentValue={newComment}
             newCommentOnChange={setComment}
+            setCommentError={setCommentError}
           />
           {listType !== ExceptionListTypeEnum.ENDPOINT && (
             <>

diff --git a/...lution/public/detection_engine/rule_exceptions/components/add_exception_flyout/reducer.ts b/...lution/public/detection_engine/rule_exceptions/components/add_exception_flyout/reducer.ts
@@ -21,6 +21,7 @@ export interface State {
   initialItems: ExceptionsBuilderExceptionItem[];
   exceptionItems: ExceptionsBuilderReturnExceptionItem[];
   newComment: string;
+  commentErrorExists: boolean;
   addExceptionToRadioSelection: string;
   itemConditionValidationErrorExists: boolean;
   closeSingleAlert: boolean;
@@ -40,6 +41,7 @@ export const initialState: State = {
   exceptionItems: [],
   exceptionItemMeta: { name: '' },
   newComment: '',
+  commentErrorExists: false,
   itemConditionValidationErrorExists: false,
   closeSingleAlert: false,
   bulkCloseAlerts: false,
@@ -76,6 +78,10 @@ export type Action =
       type: 'setComment';
       comment: string;
     }
+  | {
+      type: 'setCommentError';
+      errorExists: boolean;
+    }
   | {
       type: 'setCloseSingleAlert';
       close: boolean;
@@ -127,6 +133,7 @@ export type Action =
 
 export const createExceptionItemsReducer =
   () =>
+  /* eslint complexity: ["error", 21]*/
   (state: State, action: Action): State => {
     switch (action.type) {
       case 'setExceptionItemMeta': {
@@ -172,6 +179,14 @@ export const createExceptionItemsReducer =
           newComment: comment,
         };
       }
+      case 'setCommentError': {
+        const { errorExists } = action;
+
+        return {
+          ...state,
+          commentErrorExists: errorExists,
+        };
+      }
       case 'setCloseSingleAlert': {
         const { close } = action;
 

diff --git a/...plugins/security_solution/public/detection_engine/rule_exceptions/components/constants.ts b/...plugins/security_solution/public/detection_engine/rule_exceptions/components/constants.ts
@@ -0,0 +1,8 @@
+/*
+ * 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.
+ */
+
+export const MAX_COMMENT_LENGTH = 30000 as const;
diff --git a/...n/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.test.tsx b/...n/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.test.tsx
@@ -6,7 +6,7 @@
  */
 
 import React from 'react';
-import { waitFor } from '@testing-library/react';
+import { act, fireEvent, render, waitFor } from '@testing-library/react';
 import { ThemeProvider } from 'styled-components';
 import type { ReactWrapper } from 'enzyme';
 import { mount } from 'enzyme';
@@ -34,6 +34,7 @@ import { useFetchIndexPatterns } from '../../logic/use_exception_flyout_data';
 import { useCreateOrUpdateException } from '../../logic/use_create_update_exception';
 import { useFindExceptionListReferences } from '../../logic/use_find_references';
 import * as i18n from './translations';
+import { MAX_COMMENT_LENGTH } from '../constants';
 
 const mockTheme = getMockTheme({
   eui: {
@@ -693,5 +694,60 @@ describe('When the edit exception modal is opened', () => {
         wrapper.find('button[data-test-subj="editExceptionConfirmButton"]').getDOMNode()
       ).toBeDisabled();
     });
+
+    test('when there is a comment error has submit button disabled', async () => {
+      const { getByLabelText, queryByText, getByTestId } = render(
+        <TestProviders>
+          <EditExceptionFlyout
+            list={{
+              ...getExceptionListSchemaMock(),
+              type: 'detection',
+              namespace_type: 'single',
+              list_id: 'my_list_id',
+              id: '1234',
+            }}
+            rule={
+              {
+                ...getRulesEqlSchemaMock(),
+                id: '345',
+                name: 'My rule',
+                rule_id: 'my_rule_id',
+                query:
+                  'sequence [process where process.name = "test.exe"] [process where process.name = "explorer.exe"]',
+                exceptions_list: [
+                  {
+                    id: '1234',
+                    list_id: 'my_list_id',
+                    type: 'detection',
+                    namespace_type: 'single',
+                  },
+                ],
+              } as Rule
+            }
+            itemToEdit={getExceptionListItemSchemaMock()}
+            showAlertCloseOptions
+            onCancel={jest.fn()}
+            onConfirm={jest.fn()}
+          />
+        </TestProviders>
+      );
+
+      const commentInput = getByLabelText('Comment Input');
+
+      const commentErrorMessage = `The length of the comment is too long. The maximum length is ${MAX_COMMENT_LENGTH} characters.`;
+      expect(queryByText(commentErrorMessage)).toBeNull();
+
+      // Put comment with the length above maximum allowed
+      act(() => {
+        fireEvent.change(commentInput, {
+          target: {
+            value: [...new Array(MAX_COMMENT_LENGTH + 1).keys()].map((_) => 'a').join(''),
+          },
+        });
+        fireEvent.blur(commentInput);
+      });
+      expect(queryByText(commentErrorMessage)).not.toBeNull();
+      expect(getByTestId('editExceptionConfirmButton')).toBeDisabled();
+    });
   });
 });
diff --git a/...lution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx b/...lution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/index.tsx
@@ -117,6 +117,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
       exceptionItems,
       exceptionItemMeta: { name: exceptionItemName },
       newComment,
+      commentErrorExists,
       bulkCloseAlerts,
       disableBulkClose,
       bulkCloseIndex,
@@ -129,6 +130,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
     exceptionItems: [itemToEdit],
     exceptionItemMeta: { name: itemToEdit.name },
     newComment: '',
+    commentErrorExists: false,
     bulkCloseAlerts: false,
     disableBulkClose: true,
     bulkCloseIndex: undefined,
@@ -197,6 +199,16 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
     [dispatch]
   );
 
+  const setCommentError = useCallback(
+    (errorExists: boolean): void => {
+      dispatch({
+        type: 'setCommentError',
+        errorExists,
+      });
+    },
+    [dispatch]
+  );
+
   const setBulkCloseAlerts = useCallback(
     (bulkClose: boolean): void => {
       dispatch({
@@ -337,8 +349,17 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
       exceptionItems.every((item) => item.entries.length === 0) ||
       isLoading ||
       entryErrorExists ||
+      expireErrorExists ||
+      commentErrorExists,
+    [
+      isLoading,
+      entryErrorExists,
+      exceptionItems,
+      isSubmitting,
+      isClosingAlerts,
       expireErrorExists,
-    [isLoading, entryErrorExists, exceptionItems, isSubmitting, isClosingAlerts, expireErrorExists]
+      commentErrorExists,
+    ]
   );
 
   return (
@@ -398,6 +419,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
           exceptionItemComments={itemToEdit.comments}
           newCommentValue={newComment}
           newCommentOnChange={setComment}
+          setCommentError={setCommentError}
         />
         {listType !== ExceptionListTypeEnum.ENDPOINT && (
           <>

diff --git a/...ution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/reducer.ts b/...ution/public/detection_engine/rule_exceptions/components/edit_exception_flyout/reducer.ts
@@ -12,6 +12,7 @@ export interface State {
   exceptionItems: ExceptionsBuilderReturnExceptionItem[];
   exceptionItemMeta: { name: string };
   newComment: string;
+  commentErrorExists: boolean;
   bulkCloseAlerts: boolean;
   disableBulkClose: boolean;
   bulkCloseIndex: string[] | undefined;
@@ -29,6 +30,10 @@ export type Action =
       type: 'setComment';
       comment: string;
     }
+  | {
+      type: 'setCommentError';
+      errorExists: boolean;
+    }
   | {
       type: 'setBulkCloseAlerts';
       bulkClose: boolean;
@@ -81,6 +86,14 @@ export const createExceptionItemsReducer =
           newComment: comment,
         };
       }
+      case 'setCommentError': {
+        const { errorExists } = action;
+
+        return {
+          ...state,
+          commentErrorExists: errorExists,
+        };
+      }
       case 'setBulkCloseAlerts': {
         const { bulkClose } = action;