Skip to content

Commit

Permalink
[Security Solution] Unfriendly error message when creating an invalid…
Browse files Browse the repository at this point in the history
… exception (elastic#168213)
  • Loading branch information
e40pud committed Nov 7, 2023
1 parent 3b4a901 commit fd340f5
Show file tree
Hide file tree
Showing 12 changed files with 292 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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');
Expand Down Expand Up @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
selectedRulesToAddTo,
exceptionListsToAddTo,
newComment,
commentErrorExists,
itemConditionValidationErrorExists,
errorSubmitting,
expireTime,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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' &&
Expand All @@ -462,6 +474,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
expireErrorExists,
selectedRulesToAddTo,
listType,
commentErrorExists,
]
);

Expand Down Expand Up @@ -555,6 +568,7 @@ export const AddExceptionFlyout = memo(function AddExceptionFlyout({
initialIsOpen={!!newComment}
newCommentValue={newComment}
newCommentOnChange={setComment}
setCommentError={setCommentError}
/>
{listType !== ExceptionListTypeEnum.ENDPOINT && (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export interface State {
initialItems: ExceptionsBuilderExceptionItem[];
exceptionItems: ExceptionsBuilderReturnExceptionItem[];
newComment: string;
commentErrorExists: boolean;
addExceptionToRadioSelection: string;
itemConditionValidationErrorExists: boolean;
closeSingleAlert: boolean;
Expand All @@ -40,6 +41,7 @@ export const initialState: State = {
exceptionItems: [],
exceptionItemMeta: { name: '' },
newComment: '',
commentErrorExists: false,
itemConditionValidationErrorExists: false,
closeSingleAlert: false,
bulkCloseAlerts: false,
Expand Down Expand Up @@ -76,6 +78,10 @@ export type Action =
type: 'setComment';
comment: string;
}
| {
type: 'setCommentError';
errorExists: boolean;
}
| {
type: 'setCloseSingleAlert';
close: boolean;
Expand Down Expand Up @@ -127,6 +133,7 @@ export type Action =

export const createExceptionItemsReducer =
() =>
/* eslint complexity: ["error", 21]*/
(state: State, action: Action): State => {
switch (action.type) {
case 'setExceptionItemMeta': {
Expand Down Expand Up @@ -172,6 +179,14 @@ export const createExceptionItemsReducer =
newComment: comment,
};
}
case 'setCommentError': {
const { errorExists } = action;

return {
...state,
commentErrorExists: errorExists,
};
}
case 'setCloseSingleAlert': {
const { close } = action;

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -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();
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
exceptionItems,
exceptionItemMeta: { name: exceptionItemName },
newComment,
commentErrorExists,
bulkCloseAlerts,
disableBulkClose,
bulkCloseIndex,
Expand All @@ -129,6 +130,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
exceptionItems: [itemToEdit],
exceptionItemMeta: { name: itemToEdit.name },
newComment: '',
commentErrorExists: false,
bulkCloseAlerts: false,
disableBulkClose: true,
bulkCloseIndex: undefined,
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -398,6 +419,7 @@ const EditExceptionFlyoutComponent: React.FC<EditExceptionFlyoutProps> = ({
exceptionItemComments={itemToEdit.comments}
newCommentValue={newComment}
newCommentOnChange={setComment}
setCommentError={setCommentError}
/>
{listType !== ExceptionListTypeEnum.ENDPOINT && (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface State {
exceptionItems: ExceptionsBuilderReturnExceptionItem[];
exceptionItemMeta: { name: string };
newComment: string;
commentErrorExists: boolean;
bulkCloseAlerts: boolean;
disableBulkClose: boolean;
bulkCloseIndex: string[] | undefined;
Expand All @@ -29,6 +30,10 @@ export type Action =
type: 'setComment';
comment: string;
}
| {
type: 'setCommentError';
errorExists: boolean;
}
| {
type: 'setBulkCloseAlerts';
bulkClose: boolean;
Expand Down Expand Up @@ -81,6 +86,14 @@ export const createExceptionItemsReducer =
newComment: comment,
};
}
case 'setCommentError': {
const { errorExists } = action;

return {
...state,
commentErrorExists: errorExists,
};
}
case 'setBulkCloseAlerts': {
const { bulkClose } = action;

Expand Down
Loading

0 comments on commit fd340f5

Please sign in to comment.