From b9c820120202dc44296e080550e87c93bd37dd55 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 26 Aug 2020 10:16:17 -0400 Subject: [PATCH] [Security Solution][Exceptions] - Improve UX for missing exception list associated with rule (#75898) ## Summary **Current behavior:** - **Scenario 1:** User is in the exceptions viewer flow, they select to edit an exception item, but the list the item is associated with has since been deleted (let's say by another user) - a user is able to open modal to edit exception item and on save, an error toaster shows but no information is given to the user to indicate the issue. - **Scenario 2:** User exports rules from space 'X' and imports into space 'Y'. The exception lists associated with their newly imported rules do not exist in space 'Y' - a user goes to add an exception item and gets a modal with an error, unable to add any exceptions. - **Workaround:** current workaround exists only via API - user would need to remove the exception list from their rule via API **New behavior:** - **Scenario 1:** User is still able to oped edit modal, but on save they see an error explaining that the associated exception list does not exist and prompts them to remove the exception list --> now they're able to add exceptions to their rule - **Scenario 2:** User navigates to exceptions after importing their rule, tries to add exception, modal pops up with error informing them that they need to remove association to missing exception list, button prompts them to do so --> now can continue adding exceptions to rule --- .../exceptions/add_exception_modal/index.tsx | 90 +++++++--- .../edit_exception_modal/index.test.tsx | 5 + .../exceptions/edit_exception_modal/index.tsx | 91 +++++++--- .../exceptions/error_callout.test.tsx | 160 +++++++++++++++++ .../components/exceptions/error_callout.tsx | 169 ++++++++++++++++++ .../components/exceptions/translations.ts | 49 +++++ .../exceptions/use_add_exception.test.tsx | 44 +++++ .../exceptions/use_add_exception.tsx | 8 +- ...tch_or_create_rule_exception_list.test.tsx | 2 +- ...se_fetch_or_create_rule_exception_list.tsx | 8 +- .../components/exceptions/viewer/index.tsx | 2 + .../use_dissasociate_exception_list.test.tsx | 52 ++++++ .../rules/use_dissasociate_exception_list.tsx | 80 +++++++++ 13 files changed, 706 insertions(+), 54 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx create mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx create mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.tsx diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx index 03051ead357c..21f82c6ab4c9 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/add_exception_modal/index.tsx @@ -18,7 +18,6 @@ import { EuiCheckbox, EuiSpacer, EuiFormRow, - EuiCallOut, EuiText, } from '@elastic/eui'; import { Status } from '../../../../../common/detection_engine/schemas/common/schemas'; @@ -28,6 +27,7 @@ import { ExceptionListType, } from '../../../../../public/lists_plugin_deps'; import * as i18n from './translations'; +import * as sharedI18n from '../translations'; import { TimelineNonEcsData, Ecs } from '../../../../graphql/types'; import { useAppToasts } from '../../../hooks/use_app_toasts'; import { useKibana } from '../../../lib/kibana'; @@ -35,6 +35,7 @@ import { ExceptionBuilderComponent } from '../builder'; import { Loader } from '../../loader'; import { useAddOrUpdateException } from '../use_add_exception'; import { useSignalIndex } from '../../../../detections/containers/detection_engine/alerts/use_signal_index'; +import { useRuleAsync } from '../../../../detections/containers/detection_engine/rules/use_rule_async'; import { useFetchOrCreateRuleExceptionList } from '../use_fetch_or_create_rule_exception_list'; import { AddExceptionComments } from '../add_exception_comments'; import { @@ -46,6 +47,7 @@ import { entryHasNonEcsType, getMappedNonEcsValue, } from '../helpers'; +import { ErrorInfo, ErrorCallout } from '../error_callout'; import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules'; export interface AddExceptionModalBaseProps { @@ -107,13 +109,14 @@ export const AddExceptionModal = memo(function AddExceptionModal({ }: AddExceptionModalProps) { const { http } = useKibana().services; const [comment, setComment] = useState(''); + const { rule: maybeRule } = useRuleAsync(ruleId); const [shouldCloseAlert, setShouldCloseAlert] = useState(false); const [shouldBulkCloseAlert, setShouldBulkCloseAlert] = useState(false); const [shouldDisableBulkClose, setShouldDisableBulkClose] = useState(false); const [exceptionItemsToAdd, setExceptionItemsToAdd] = useState< Array >([]); - const [fetchOrCreateListError, setFetchOrCreateListError] = useState(false); + const [fetchOrCreateListError, setFetchOrCreateListError] = useState(null); const { addError, addSuccess } = useAppToasts(); const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex(); const [ @@ -164,17 +167,41 @@ export const AddExceptionModal = memo(function AddExceptionModal({ }, [onRuleChange] ); - const onFetchOrCreateExceptionListError = useCallback( - (error: Error) => { - setFetchOrCreateListError(true); + + const handleDissasociationSuccess = useCallback( + (id: string): void => { + handleRuleChange(true); + addSuccess(sharedI18n.DISSASOCIATE_LIST_SUCCESS(id)); + onCancel(); + }, + [handleRuleChange, addSuccess, onCancel] + ); + + const handleDissasociationError = useCallback( + (error: Error): void => { + addError(error, { title: sharedI18n.DISSASOCIATE_EXCEPTION_LIST_ERROR }); + onCancel(); + }, + [addError, onCancel] + ); + + const handleFetchOrCreateExceptionListError = useCallback( + (error: Error, statusCode: number | null, message: string | null) => { + setFetchOrCreateListError({ + reason: error.message, + code: statusCode, + details: message, + listListId: null, + }); }, [setFetchOrCreateListError] ); + const [isLoadingExceptionList, ruleExceptionList] = useFetchOrCreateRuleExceptionList({ http, ruleId, exceptionListType, - onError: onFetchOrCreateExceptionListError, + onError: handleFetchOrCreateExceptionListError, onSuccess: handleRuleChange, }); @@ -279,7 +306,9 @@ export const AddExceptionModal = memo(function AddExceptionModal({ ]); const isSubmitButtonDisabled = useMemo( - () => fetchOrCreateListError || exceptionItemsToAdd.every((item) => item.entries.length === 0), + () => + fetchOrCreateListError != null || + exceptionItemsToAdd.every((item) => item.entries.length === 0), [fetchOrCreateListError, exceptionItemsToAdd] ); @@ -295,19 +324,27 @@ export const AddExceptionModal = memo(function AddExceptionModal({ - {fetchOrCreateListError === true && ( - -

{i18n.ADD_EXCEPTION_FETCH_ERROR}

- + {fetchOrCreateListError != null && ( + + + )} - {fetchOrCreateListError === false && + {fetchOrCreateListError == null && (isLoadingExceptionList || isIndexPatternLoading || isSignalIndexLoading || isSignalIndexPatternLoading) && ( )} - {fetchOrCreateListError === false && + {fetchOrCreateListError == null && !isSignalIndexLoading && !isSignalIndexPatternLoading && !isLoadingExceptionList && @@ -377,20 +414,21 @@ export const AddExceptionModal = memo(function AddExceptionModal({ )} + {fetchOrCreateListError == null && ( + + {i18n.CANCEL} - - {i18n.CANCEL} - - - {i18n.ADD_EXCEPTION} - - + + {i18n.ADD_EXCEPTION} + + + )} ); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx index 6ff218ca0605..c724e6a2c711 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/edit_exception_modal/index.test.tsx @@ -77,6 +77,7 @@ describe('When the edit exception modal is opened', () => { ({ eui: euiLightVars, darkMode: false })}> { ({ eui: euiLightVars, darkMode: false })}> { ({ eui: euiLightVars, darkMode: false })}> { ({ eui: euiLightVars, darkMode: false })}> { ({ eui: euiLightVars, darkMode: false })}> void; onConfirm: () => void; + onRuleChange?: () => void; } const Modal = styled(EuiModal)` @@ -83,14 +88,18 @@ const ModalBodySection = styled.section` export const EditExceptionModal = memo(function EditExceptionModal({ ruleName, + ruleId, ruleIndices, exceptionItem, exceptionListType, onCancel, onConfirm, + onRuleChange, }: EditExceptionModalProps) { const { http } = useKibana().services; const [comment, setComment] = useState(''); + const { rule: maybeRule } = useRuleAsync(ruleId); + const [updateError, setUpdateError] = useState(null); const [hasVersionConflict, setHasVersionConflict] = useState(false); const [shouldBulkCloseAlert, setShouldBulkCloseAlert] = useState(false); const [shouldDisableBulkClose, setShouldDisableBulkClose] = useState(false); @@ -108,18 +117,44 @@ export const EditExceptionModal = memo(function EditExceptionModal({ 'rules' ); - const onError = useCallback( - (error) => { + const handleExceptionUpdateError = useCallback( + (error: Error, statusCode: number | null, message: string | null) => { if (error.message.includes('Conflict')) { setHasVersionConflict(true); } else { - addError(error, { title: i18n.EDIT_EXCEPTION_ERROR }); - onCancel(); + setUpdateError({ + reason: error.message, + code: statusCode, + details: message, + listListId: exceptionItem.list_id, + }); } }, + [setUpdateError, setHasVersionConflict, exceptionItem.list_id] + ); + + const handleDissasociationSuccess = useCallback( + (id: string): void => { + addSuccess(sharedI18n.DISSASOCIATE_LIST_SUCCESS(id)); + + if (onRuleChange) { + onRuleChange(); + } + + onCancel(); + }, + [addSuccess, onCancel, onRuleChange] + ); + + const handleDissasociationError = useCallback( + (error: Error): void => { + addError(error, { title: sharedI18n.DISSASOCIATE_EXCEPTION_LIST_ERROR }); + onCancel(); + }, [addError, onCancel] ); - const onSuccess = useCallback(() => { + + const handleExceptionUpdateSuccess = useCallback((): void => { addSuccess(i18n.EDIT_EXCEPTION_SUCCESS); onConfirm(); }, [addSuccess, onConfirm]); @@ -127,8 +162,8 @@ export const EditExceptionModal = memo(function EditExceptionModal({ const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException( { http, - onSuccess, - onError, + onSuccess: handleExceptionUpdateSuccess, + onError: handleExceptionUpdateError, } ); @@ -222,11 +257,9 @@ export const EditExceptionModal = memo(function EditExceptionModal({ {ruleName} - {(addExceptionIsLoading || isIndexPatternLoading || isSignalIndexLoading) && ( )} - {!isSignalIndexLoading && !addExceptionIsLoading && !isIndexPatternLoading && ( <> @@ -280,7 +313,18 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} - + {updateError != null && ( + + + + )} {hasVersionConflict && ( @@ -288,20 +332,21 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} + {updateError == null && ( + + {i18n.CANCEL} - - {i18n.CANCEL} - - - {i18n.EDIT_EXCEPTION_SAVE_BUTTON} - - + + {i18n.EDIT_EXCEPTION_SAVE_BUTTON} + + + )} ); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx new file mode 100644 index 000000000000..c9efa5e54dcc --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx @@ -0,0 +1,160 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { ThemeProvider } from 'styled-components'; +import { mountWithIntl } from 'test_utils/enzyme_helpers'; +import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; + +import { getListMock } from '../../../../common/detection_engine/schemas/types/lists.mock'; +import { useDissasociateExceptionList } from '../../../detections/containers/detection_engine/rules/use_dissasociate_exception_list'; +import { createKibanaCoreStartMock } from '../../mock/kibana_core'; +import { ErrorCallout } from './error_callout'; +import { savedRuleMock } from '../../../detections/containers/detection_engine/rules/mock'; + +jest.mock('../../../detections/containers/detection_engine/rules/use_dissasociate_exception_list'); + +const mockKibanaHttpService = createKibanaCoreStartMock().http; + +describe('ErrorCallout', () => { + const mockDissasociate = jest.fn(); + + beforeEach(() => { + (useDissasociateExceptionList as jest.Mock).mockReturnValue([false, mockDissasociate]); + }); + + it('it renders error details', () => { + const wrapper = mountWithIntl( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="errorCalloutContainer"] .euiCallOutHeader__title').text() + ).toEqual('Error: error reason (500)'); + expect(wrapper.find('[data-test-subj="errorCalloutMessage"]').at(0).text()).toEqual( + 'Error fetching exception list' + ); + }); + + it('it invokes "onCancel" when cancel button clicked', () => { + const mockOnCancel = jest.fn(); + const wrapper = mountWithIntl( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + wrapper.find('[data-test-subj="errorCalloutCancelButton"]').at(0).simulate('click'); + + expect(mockOnCancel).toHaveBeenCalled(); + }); + + it('it does not render status code if not available', () => { + const wrapper = mountWithIntl( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="errorCalloutContainer"] .euiCallOutHeader__title').text() + ).toEqual('Error: not found'); + expect(wrapper.find('[data-test-subj="errorCalloutMessage"]').at(0).text()).toEqual( + 'Error fetching exception list' + ); + expect(wrapper.find('[data-test-subj="errorCalloutDissasociateButton"]').exists()).toBeFalsy(); + }); + + it('it renders specific missing exceptions list error', () => { + const wrapper = mountWithIntl( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + expect( + wrapper.find('[data-test-subj="errorCalloutContainer"] .euiCallOutHeader__title').text() + ).toEqual('Error: not found (404)'); + expect(wrapper.find('[data-test-subj="errorCalloutMessage"]').at(0).text()).toEqual( + 'The associated exception list (some_uuid) no longer exists. Please remove the missing exception list to add additional exceptions to the detection rule.' + ); + expect(wrapper.find('[data-test-subj="errorCalloutDissasociateButton"]').exists()).toBeTruthy(); + }); + + it('it dissasociates list from rule when remove exception list clicked ', () => { + const wrapper = mountWithIntl( + ({ eui: euiLightVars, darkMode: false })}> + + + ); + + wrapper.find('[data-test-subj="errorCalloutDissasociateButton"]').at(0).simulate('click'); + + expect(mockDissasociate).toHaveBeenCalledWith([]); + }); +}); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx new file mode 100644 index 000000000000..a2419ef16df3 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx @@ -0,0 +1,169 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { useMemo, useEffect, useState, useCallback } from 'react'; +import { + EuiButtonEmpty, + EuiAccordion, + EuiCodeBlock, + EuiButton, + EuiCallOut, + EuiText, + EuiSpacer, +} from '@elastic/eui'; + +import { HttpSetup } from '../../../../../../../src/core/public'; +import { List } from '../../../../common/detection_engine/schemas/types/lists'; +import { Rule } from '../../../detections/containers/detection_engine/rules/types'; +import * as i18n from './translations'; +import { useDissasociateExceptionList } from '../../../detections/containers/detection_engine/rules/use_dissasociate_exception_list'; + +export interface ErrorInfo { + reason: string | null; + code: number | null; + details: string | null; + listListId: string | null; +} + +export interface ErrorCalloutProps { + http: HttpSetup; + rule: Rule | null; + errorInfo: ErrorInfo; + onCancel: () => void; + onSuccess: (listId: string) => void; + onError: (arg: Error) => void; +} + +const ErrorCalloutComponent = ({ + http, + rule, + errorInfo, + onCancel, + onError, + onSuccess, +}: ErrorCalloutProps): JSX.Element => { + const [listToDelete, setListToDelete] = useState(null); + const [errorTitle, setErrorTitle] = useState(''); + const [errorMessage, setErrorMessage] = useState(i18n.ADD_EXCEPTION_FETCH_ERROR); + + const handleOnSuccess = useCallback((): void => { + onSuccess(listToDelete != null ? listToDelete.id : ''); + }, [onSuccess, listToDelete]); + + const [isDissasociatingList, handleDissasociateExceptionList] = useDissasociateExceptionList({ + http, + ruleRuleId: rule != null ? rule.rule_id : '', + onSuccess: handleOnSuccess, + onError, + }); + + const canDisplay404Actions = useMemo( + (): boolean => + errorInfo.code === 404 && + rule != null && + listToDelete != null && + handleDissasociateExceptionList != null, + [errorInfo.code, listToDelete, handleDissasociateExceptionList, rule] + ); + + useEffect((): void => { + // Yes, it's redundant, unfortunately typescript wasn't picking up + // that `listToDelete` is checked in canDisplay404Actions + if (canDisplay404Actions && listToDelete != null) { + setErrorMessage(i18n.ADD_EXCEPTION_FETCH_404_ERROR(listToDelete.id)); + } + + setErrorTitle(`${errorInfo.reason}${errorInfo.code != null ? ` (${errorInfo.code})` : ''}`); + }, [errorInfo.reason, errorInfo.code, listToDelete, canDisplay404Actions]); + + const handleDissasociateList = useCallback((): void => { + // Yes, it's redundant, unfortunately typescript wasn't picking up + // that `handleDissasociateExceptionList` and `list` are checked in + // canDisplay404Actions + if ( + canDisplay404Actions && + rule != null && + listToDelete != null && + handleDissasociateExceptionList != null + ) { + const exceptionLists = (rule.exceptions_list ?? []).filter( + ({ id }) => id !== listToDelete.id + ); + + handleDissasociateExceptionList(exceptionLists); + } + }, [handleDissasociateExceptionList, listToDelete, canDisplay404Actions, rule]); + + useEffect((): void => { + if (errorInfo.code === 404 && rule != null && rule.exceptions_list != null) { + const [listFound] = rule.exceptions_list.filter( + ({ id, list_id: listId }) => + (errorInfo.details != null && errorInfo.details.includes(id)) || + errorInfo.listListId === listId + ); + setListToDelete(listFound); + } + }, [rule, errorInfo.details, errorInfo.code, errorInfo.listListId]); + + return ( + + +

{errorMessage}

+ + + {listToDelete != null && ( + +

{i18n.MODAL_ERROR_ACCORDION_TEXT}

+ + } + > + + {JSON.stringify(listToDelete)} + + + )} + + + {i18n.CANCEL} + + {canDisplay404Actions && ( + + {i18n.CLEAR_EXCEPTIONS_LABEL} + + )} + + ); +}; + +ErrorCalloutComponent.displayName = 'ErrorCalloutComponent'; + +export const ErrorCallout = React.memo(ErrorCalloutComponent); + +ErrorCallout.displayName = 'ErrorCallout'; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/translations.ts b/x-pack/plugins/security_solution/public/common/components/exceptions/translations.ts index 13e9d0df549f..484a3d593026 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/translations.ts @@ -190,3 +190,52 @@ export const TOTAL_ITEMS_FETCH_ERROR = i18n.translate( defaultMessage: 'Error getting exception item totals', } ); + +export const CLEAR_EXCEPTIONS_LABEL = i18n.translate( + 'xpack.securitySolution.exceptions.clearExceptionsLabel', + { + defaultMessage: 'Remove Exception List', + } +); + +export const ADD_EXCEPTION_FETCH_404_ERROR = (listId: string) => + i18n.translate('xpack.securitySolution.exceptions.fetch404Error', { + values: { listId }, + defaultMessage: + 'The associated exception list ({listId}) no longer exists. Please remove the missing exception list to add additional exceptions to the detection rule.', + }); + +export const ADD_EXCEPTION_FETCH_ERROR = i18n.translate( + 'xpack.securitySolution.exceptions.fetchError', + { + defaultMessage: 'Error fetching exception list', + } +); + +export const ERROR = i18n.translate('xpack.securitySolution.exceptions.errorLabel', { + defaultMessage: 'Error', +}); + +export const CANCEL = i18n.translate('xpack.securitySolution.exceptions.cancelLabel', { + defaultMessage: 'Cancel', +}); + +export const MODAL_ERROR_ACCORDION_TEXT = i18n.translate( + 'xpack.securitySolution.exceptions.modalErrorAccordionText', + { + defaultMessage: 'Show rule reference information:', + } +); + +export const DISSASOCIATE_LIST_SUCCESS = (id: string) => + i18n.translate('xpack.securitySolution.exceptions.dissasociateListSuccessText', { + values: { id }, + defaultMessage: 'Exception list ({id}) has successfully been removed', + }); + +export const DISSASOCIATE_EXCEPTION_LIST_ERROR = i18n.translate( + 'xpack.securitySolution.exceptions.dissasociateExceptionListError', + { + defaultMessage: 'Failed to remove exception list', + } +); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx index 6611ee2385d1..46923e07d225 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.test.tsx @@ -148,6 +148,50 @@ describe('useAddOrUpdateException', () => { }); }); + it('invokes "onError" if call to add exception item fails', async () => { + const mockError = new Error('error adding item'); + + addExceptionListItem = jest + .spyOn(listsApi, 'addExceptionListItem') + .mockRejectedValue(mockError); + + await act(async () => { + const { rerender, result, waitForNextUpdate } = render(); + const addOrUpdateItems = await waitForAddOrUpdateFunc({ + rerender, + result, + waitForNextUpdate, + }); + if (addOrUpdateItems) { + addOrUpdateItems(...addOrUpdateItemsArgs); + } + await waitForNextUpdate(); + expect(onError).toHaveBeenCalledWith(mockError, null, null); + }); + }); + + it('invokes "onError" if call to update exception item fails', async () => { + const mockError = new Error('error updating item'); + + updateExceptionListItem = jest + .spyOn(listsApi, 'updateExceptionListItem') + .mockRejectedValue(mockError); + + await act(async () => { + const { rerender, result, waitForNextUpdate } = render(); + const addOrUpdateItems = await waitForAddOrUpdateFunc({ + rerender, + result, + waitForNextUpdate, + }); + if (addOrUpdateItems) { + addOrUpdateItems(...addOrUpdateItemsArgs); + } + await waitForNextUpdate(); + expect(onError).toHaveBeenCalledWith(mockError, null, null); + }); + }); + describe('when alertIdToClose is not passed in', () => { it('should not update the alert status', async () => { await act(async () => { diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx index 9d45a411b513..be289b0e85e6 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_add_exception.tsx @@ -42,7 +42,7 @@ export type ReturnUseAddOrUpdateException = [ export interface UseAddOrUpdateExceptionProps { http: HttpStart; - onError: (arg: Error) => void; + onError: (arg: Error, code: number | null, message: string | null) => void; onSuccess: () => void; } @@ -157,7 +157,11 @@ export const useAddOrUpdateException = ({ } catch (error) { if (isSubscribed) { setIsLoading(false); - onError(error); + if (error.body != null) { + onError(error, error.body.status_code, error.body.message); + } else { + onError(error, null, null); + } } } }; diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.test.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.test.tsx index 39d88bd8e472..f20a58b9ffa3 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.test.tsx @@ -379,7 +379,7 @@ describe('useFetchOrCreateRuleExceptionList', () => { await waitForNextUpdate(); await waitForNextUpdate(); expect(onError).toHaveBeenCalledTimes(1); - expect(onError).toHaveBeenCalledWith(error); + expect(onError).toHaveBeenCalledWith(error, null, null); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx index 0d367e03a799..944631d4e9fb 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/use_fetch_or_create_rule_exception_list.tsx @@ -30,7 +30,7 @@ export interface UseFetchOrCreateRuleExceptionListProps { http: HttpStart; ruleId: Rule['id']; exceptionListType: ExceptionListSchema['type']; - onError: (arg: Error) => void; + onError: (arg: Error, code: number | null, message: string | null) => void; onSuccess?: (ruleWasChanged: boolean) => void; } @@ -179,7 +179,11 @@ export const useFetchOrCreateRuleExceptionList = ({ if (isSubscribed) { setIsLoading(false); setExceptionList(null); - onError(error); + if (error.body != null) { + onError(error, error.body.status_code, error.body.message); + } else { + onError(error, null, null); + } } } } diff --git a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx index 7482068454a9..c97895cdfe23 100644 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/exceptions/viewer/index.tsx @@ -322,11 +322,13 @@ const ExceptionsViewerComponent = ({ exceptionListTypeToEdit != null && ( )} diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx new file mode 100644 index 000000000000..6b1938655dc3 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx @@ -0,0 +1,52 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { act, renderHook } from '@testing-library/react-hooks'; + +import { createKibanaCoreStartMock } from '../../../../common/mock/kibana_core'; + +import * as api from './api'; +import { ruleMock } from './mock'; +import { + ReturnUseDissasociateExceptionList, + UseDissasociateExceptionListProps, + useDissasociateExceptionList, +} from './use_dissasociate_exception_list'; + +const mockKibanaHttpService = createKibanaCoreStartMock().http; + +describe('useDissasociateExceptionList', () => { + const onError = jest.fn(); + const onSuccess = jest.fn(); + + beforeEach(() => { + jest.spyOn(api, 'patchRule').mockResolvedValue(ruleMock); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + test('initializes hook', async () => { + await act(async () => { + const { result, waitForNextUpdate } = renderHook< + UseDissasociateExceptionListProps, + ReturnUseDissasociateExceptionList + >(() => + useDissasociateExceptionList({ + http: mockKibanaHttpService, + ruleRuleId: 'rule_id', + onError, + onSuccess, + }) + ); + + await waitForNextUpdate(); + + expect(result.current).toEqual([false, null]); + }); + }); +}); diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.tsx b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.tsx new file mode 100644 index 000000000000..dffba3e6e043 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.tsx @@ -0,0 +1,80 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { useEffect, useState, useRef } from 'react'; + +import { HttpStart } from '../../../../../../../../src/core/public'; +import { List } from '../../../../../common/detection_engine/schemas/types/lists'; +import { patchRule } from './api'; + +type Func = (lists: List[]) => void; +export type ReturnUseDissasociateExceptionList = [boolean, Func | null]; + +export interface UseDissasociateExceptionListProps { + http: HttpStart; + ruleRuleId: string; + onError: (arg: Error) => void; + onSuccess: () => void; +} + +/** + * Hook for removing an exception list reference from a rule + * + * @param http Kibana http service + * @param ruleRuleId a rule_id (NOT id) + * @param onError error callback + * @param onSuccess success callback + * + */ +export const useDissasociateExceptionList = ({ + http, + ruleRuleId, + onError, + onSuccess, +}: UseDissasociateExceptionListProps): ReturnUseDissasociateExceptionList => { + const [isLoading, setLoading] = useState(false); + const dissasociateList = useRef(null); + + useEffect(() => { + let isSubscribed = true; + const abortCtrl = new AbortController(); + + const dissasociateListFromRule = (id: string) => async ( + exceptionLists: List[] + ): Promise => { + try { + if (isSubscribed) { + setLoading(true); + + await patchRule({ + ruleProperties: { + rule_id: id, + exceptions_list: exceptionLists, + }, + signal: abortCtrl.signal, + }); + + onSuccess(); + setLoading(false); + } + } catch (err) { + if (isSubscribed) { + setLoading(false); + onError(err); + } + } + }; + + dissasociateList.current = dissasociateListFromRule(ruleRuleId); + + return (): void => { + isSubscribed = false; + abortCtrl.abort(); + }; + }, [http, ruleRuleId, onError, onSuccess]); + + return [isLoading, dissasociateList.current]; +};