Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution] Unfriendly error message when creating an invalid exception (#168213) #170764

Merged
merged 5 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,3 +505,8 @@ export const DEFAULT_ALERT_TAGS_VALUE = [
i18n.FALSE_POSITIVE,
i18n.FURTHER_INVESTIGATION_REQUIRED,
] as const;

/**
* Max length for the comments within security solution
*/
export const MAX_COMMENT_LENGTH = 30000 as const;
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 '../../../../../common/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
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 '../../../../../common/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
Loading