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 21f82c6ab4c98..03051ead357c9 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,6 +18,7 @@ import { EuiCheckbox, EuiSpacer, EuiFormRow, + EuiCallOut, EuiText, } from '@elastic/eui'; import { Status } from '../../../../../common/detection_engine/schemas/common/schemas'; @@ -27,7 +28,6 @@ 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,7 +35,6 @@ 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 { @@ -47,7 +46,6 @@ import { entryHasNonEcsType, getMappedNonEcsValue, } from '../helpers'; -import { ErrorInfo, ErrorCallout } from '../error_callout'; import { useFetchIndexPatterns } from '../../../../detections/containers/detection_engine/rules'; export interface AddExceptionModalBaseProps { @@ -109,14 +107,13 @@ 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(null); + const [fetchOrCreateListError, setFetchOrCreateListError] = useState(false); const { addError, addSuccess } = useAppToasts(); const { loading: isSignalIndexLoading, signalIndexName } = useSignalIndex(); const [ @@ -167,41 +164,17 @@ export const AddExceptionModal = memo(function AddExceptionModal({ }, [onRuleChange] ); - - 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, - }); + const onFetchOrCreateExceptionListError = useCallback( + (error: Error) => { + setFetchOrCreateListError(true); }, [setFetchOrCreateListError] ); - const [isLoadingExceptionList, ruleExceptionList] = useFetchOrCreateRuleExceptionList({ http, ruleId, exceptionListType, - onError: handleFetchOrCreateExceptionListError, + onError: onFetchOrCreateExceptionListError, onSuccess: handleRuleChange, }); @@ -306,9 +279,7 @@ export const AddExceptionModal = memo(function AddExceptionModal({ ]); const isSubmitButtonDisabled = useMemo( - () => - fetchOrCreateListError != null || - exceptionItemsToAdd.every((item) => item.entries.length === 0), + () => fetchOrCreateListError || exceptionItemsToAdd.every((item) => item.entries.length === 0), [fetchOrCreateListError, exceptionItemsToAdd] ); @@ -324,27 +295,19 @@ export const AddExceptionModal = memo(function AddExceptionModal({ - {fetchOrCreateListError != null && ( - - - + {fetchOrCreateListError === true && ( + +

{i18n.ADD_EXCEPTION_FETCH_ERROR}

+
)} - {fetchOrCreateListError == null && + {fetchOrCreateListError === false && (isLoadingExceptionList || isIndexPatternLoading || isSignalIndexLoading || isSignalIndexPatternLoading) && ( )} - {fetchOrCreateListError == null && + {fetchOrCreateListError === false && !isSignalIndexLoading && !isSignalIndexPatternLoading && !isLoadingExceptionList && @@ -414,21 +377,20 @@ export const AddExceptionModal = memo(function AddExceptionModal({ )} - {fetchOrCreateListError == null && ( - - {i18n.CANCEL} - - {i18n.ADD_EXCEPTION} - - - )} + + {i18n.CANCEL} + + + {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 c724e6a2c711f..6ff218ca06059 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,7 +77,6 @@ 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)` @@ -88,18 +83,14 @@ 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); @@ -117,44 +108,18 @@ export const EditExceptionModal = memo(function EditExceptionModal({ 'rules' ); - const handleExceptionUpdateError = useCallback( - (error: Error, statusCode: number | null, message: string | null) => { + const onError = useCallback( + (error) => { if (error.message.includes('Conflict')) { setHasVersionConflict(true); } else { - setUpdateError({ - reason: error.message, - code: statusCode, - details: message, - listListId: exceptionItem.list_id, - }); + addError(error, { title: i18n.EDIT_EXCEPTION_ERROR }); + onCancel(); } }, - [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 handleExceptionUpdateSuccess = useCallback((): void => { + const onSuccess = useCallback(() => { addSuccess(i18n.EDIT_EXCEPTION_SUCCESS); onConfirm(); }, [addSuccess, onConfirm]); @@ -162,8 +127,8 @@ export const EditExceptionModal = memo(function EditExceptionModal({ const [{ isLoading: addExceptionIsLoading }, addOrUpdateExceptionItems] = useAddOrUpdateException( { http, - onSuccess: handleExceptionUpdateSuccess, - onError: handleExceptionUpdateError, + onSuccess, + onError, } ); @@ -257,9 +222,11 @@ export const EditExceptionModal = memo(function EditExceptionModal({ {ruleName} + {(addExceptionIsLoading || isIndexPatternLoading || isSignalIndexLoading) && ( )} + {!isSignalIndexLoading && !addExceptionIsLoading && !isIndexPatternLoading && ( <> @@ -313,18 +280,7 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} - {updateError != null && ( - - - - )} + {hasVersionConflict && ( @@ -332,21 +288,20 @@ export const EditExceptionModal = memo(function EditExceptionModal({ )} - {updateError == null && ( - - {i18n.CANCEL} - - {i18n.EDIT_EXCEPTION_SAVE_BUTTON} - - - )} + + {i18n.CANCEL} + + + {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 deleted file mode 100644 index c9efa5e54dccf..0000000000000 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx +++ /dev/null @@ -1,160 +0,0 @@ -/* - * 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 deleted file mode 100644 index a2419ef16df3a..0000000000000 --- a/x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx +++ /dev/null @@ -1,169 +0,0 @@ -/* - * 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 484a3d593026e..13e9d0df549f8 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,52 +190,3 @@ 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 46923e07d225a..6611ee2385d10 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,50 +148,6 @@ 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 be289b0e85e66..9d45a411b5130 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, code: number | null, message: string | null) => void; + onError: (arg: Error) => void; onSuccess: () => void; } @@ -157,11 +157,7 @@ export const useAddOrUpdateException = ({ } catch (error) { if (isSubscribed) { setIsLoading(false); - if (error.body != null) { - onError(error, error.body.status_code, error.body.message); - } else { - onError(error, null, null); - } + onError(error); } } }; 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 f20a58b9ffa36..39d88bd8e4724 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, null, null); + expect(onError).toHaveBeenCalledWith(error); }); }); 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 944631d4e9fb5..0d367e03a799f 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, code: number | null, message: string | null) => void; + onError: (arg: Error) => void; onSuccess?: (ruleWasChanged: boolean) => void; } @@ -179,11 +179,7 @@ export const useFetchOrCreateRuleExceptionList = ({ if (isSubscribed) { setIsLoading(false); setExceptionList(null); - if (error.body != null) { - onError(error, error.body.status_code, error.body.message); - } else { - onError(error, null, null); - } + onError(error); } } } 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 c97895cdfe236..7482068454a97 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,13 +322,11 @@ 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 deleted file mode 100644 index 6b1938655dc33..0000000000000 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx +++ /dev/null @@ -1,52 +0,0 @@ -/* - * 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 deleted file mode 100644 index dffba3e6e0436..0000000000000 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.tsx +++ /dev/null @@ -1,80 +0,0 @@ -/* - * 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]; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts index 3eefd3e665cd6..593ada470b118 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts @@ -79,6 +79,84 @@ describe('rules_notification_alert_type', () => { ); }); + it('should resolve results_link when meta is undefined to use "/app/security"', async () => { + const ruleAlert = getResult(); + delete ruleAlert.params.meta; + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + alertServices.callCluster.mockResolvedValue({ + count: 10, + }); + + await alert.executor(payload); + expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); + + const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; + expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( + 'default', + expect.objectContaining({ + results_link: + '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', + }) + ); + }); + + it('should resolve results_link when meta is an empty object to use "/app/security"', async () => { + const ruleAlert = getResult(); + ruleAlert.params.meta = {}; + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + alertServices.callCluster.mockResolvedValue({ + count: 10, + }); + await alert.executor(payload); + expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); + + const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; + expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( + 'default', + expect.objectContaining({ + results_link: + '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', + }) + ); + }); + + it('should resolve results_link to custom kibana link when given one', async () => { + const ruleAlert = getResult(); + ruleAlert.params.meta = { + kibana_siem_app_url: 'http://localhost', + }; + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + alertServices.callCluster.mockResolvedValue({ + count: 10, + }); + await alert.executor(payload); + expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); + + const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; + expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( + 'default', + expect.objectContaining({ + results_link: + 'http://localhost/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', + }) + ); + }); + it('should not call alertInstanceFactory if signalsCount was 0', async () => { const ruleAlert = getResult(); alertServices.savedObjectsClient.get.mockResolvedValue({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts index ab824957087fc..2eb34529d044c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts @@ -64,8 +64,8 @@ export const rulesNotificationAlertType = ({ from: fromInMs, to: toInMs, id: ruleAlertSavedObject.id, - kibanaSiemAppUrl: (ruleAlertParams.meta as { kibana_siem_app_url?: string }) - .kibana_siem_app_url, + kibanaSiemAppUrl: (ruleAlertParams.meta as { kibana_siem_app_url?: string } | undefined) + ?.kibana_siem_app_url, }); logger.info( diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/test_cases/queries/action_without_meta.json b/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/test_cases/queries/action_without_meta.json new file mode 100644 index 0000000000000..6569a641de3a2 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/test_cases/queries/action_without_meta.json @@ -0,0 +1,42 @@ +{ + "type": "query", + "index": [ + "apm-*-transaction*", + "auditbeat-*", + "endgame-*", + "filebeat-*", + "logs-*", + "packetbeat-*", + "winlogbeat-*" + ], + "filters": [], + "language": "kuery", + "query": "host.name: *", + "author": [], + "false_positives": [], + "references": [], + "risk_score": 50, + "risk_score_mapping": [], + "severity": "low", + "severity_mapping": [], + "threat": [], + "name": "Host Name Test", + "description": "Host Name Test", + "tags": [], + "license": "", + "interval": "5m", + "from": "now-30s", + "to": "now", + "actions": [ + { + "group": "default", + "id": "4c42ecf2-5e9b-4ce6-8a7a-ab620fd8b169", + "params": { + "body": "{}" + }, + "action_type_id": ".webhook" + } + ], + "enabled": true, + "throttle": "rule" +} diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts index b0c855afa8be9..b29d15f5e5c72 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts @@ -16,6 +16,7 @@ import { getListsClient, getExceptions, sortExceptionItems, + parseScheduleDates, } from './utils'; import { RuleExecutorOptions } from './types'; import { searchAfterAndBulkCreate } from './search_after_bulk_create'; @@ -227,6 +228,105 @@ describe('rules_notification_alert_type', () => { ); }); + it('should resolve results_link when meta is an empty object to use "/app/security"', async () => { + const ruleAlert = getResult(); + ruleAlert.params.meta = {}; + ruleAlert.actions = [ + { + actionTypeId: '.slack', + params: { + message: + 'Rule generated {{state.signals_count}} signals\n\n{{context.rule.name}}\n{{{context.results_link}}}', + }, + group: 'default', + id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', + }, + ]; + + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + (parseScheduleDates as jest.Mock).mockReturnValue(moment(100)); + payload.params.meta = {}; + await alert.executor(payload); + + expect(scheduleNotificationActions).toHaveBeenCalledWith( + expect.objectContaining({ + resultsLink: + '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:100,kind:absolute,to:100)),timeline:(linkTo:!(global),timerange:(from:100,kind:absolute,to:100)))', + }) + ); + }); + + it('should resolve results_link when meta is undefined use "/app/security"', async () => { + const ruleAlert = getResult(); + delete ruleAlert.params.meta; + ruleAlert.actions = [ + { + actionTypeId: '.slack', + params: { + message: + 'Rule generated {{state.signals_count}} signals\n\n{{context.rule.name}}\n{{{context.results_link}}}', + }, + group: 'default', + id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', + }, + ]; + + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + (parseScheduleDates as jest.Mock).mockReturnValue(moment(100)); + delete payload.params.meta; + await alert.executor(payload); + + expect(scheduleNotificationActions).toHaveBeenCalledWith( + expect.objectContaining({ + resultsLink: + '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:100,kind:absolute,to:100)),timeline:(linkTo:!(global),timerange:(from:100,kind:absolute,to:100)))', + }) + ); + }); + + it('should resolve results_link with a custom link', async () => { + const ruleAlert = getResult(); + ruleAlert.params.meta = { kibana_siem_app_url: 'http://localhost' }; + ruleAlert.actions = [ + { + actionTypeId: '.slack', + params: { + message: + 'Rule generated {{state.signals_count}} signals\n\n{{context.rule.name}}\n{{{context.results_link}}}', + }, + group: 'default', + id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', + }, + ]; + + alertServices.savedObjectsClient.get.mockResolvedValue({ + id: 'rule-id', + type: 'type', + references: [], + attributes: ruleAlert, + }); + (parseScheduleDates as jest.Mock).mockReturnValue(moment(100)); + payload.params.meta = { kibana_siem_app_url: 'http://localhost' }; + await alert.executor(payload); + + expect(scheduleNotificationActions).toHaveBeenCalledWith( + expect.objectContaining({ + resultsLink: + 'http://localhost/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:100,kind:absolute,to:100)),timeline:(linkTo:!(global),timerange:(from:100,kind:absolute,to:100)))', + }) + ); + }); + describe('ML rule', () => { it('should throw an error if ML plugin was not available', async () => { const ruleAlert = getMlResult(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index 0e859ecef31c6..b5cbf80b084f7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -356,7 +356,8 @@ export const signalRulesAlertType = ({ from: fromInMs, to: toInMs, id: savedObject.id, - kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string }).kibana_siem_app_url, + kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined) + ?.kibana_siem_app_url, }); logger.info( diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/add_actions.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/add_actions.ts new file mode 100644 index 0000000000000..2468851237047 --- /dev/null +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/add_actions.ts @@ -0,0 +1,134 @@ +/* + * 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 expect from '@kbn/expect'; + +import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { + createSignalsIndex, + deleteAllAlerts, + deleteSignalsIndex, + removeServerGeneratedProperties, + waitFor, + getWebHookAction, + getRuleWithWebHookAction, + getSimpleRuleOutputWithWebHookAction, +} from '../../utils'; +import { CreateRulesSchema } from '../../../../plugins/security_solution/common/detection_engine/schemas/request/create_rules_schema'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + const es = getService('es'); + + describe('add_actions', () => { + describe('adding actions', () => { + beforeEach(async () => { + await createSignalsIndex(supertest); + }); + + afterEach(async () => { + await deleteSignalsIndex(supertest); + await deleteAllAlerts(es); + }); + + it('should be able to create a new webhook action and attach it to a rule', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + // create a rule with the action attached + const { body } = await supertest + .post(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send(getRuleWithWebHookAction(hookAction.id)) + .expect(200); + + const bodyToCompare = removeServerGeneratedProperties(body); + expect(bodyToCompare).to.eql( + getSimpleRuleOutputWithWebHookAction(`${bodyToCompare?.actions?.[0].id}`) + ); + }); + + it('should be able to create a new webhook action and attach it to a rule without a meta field and run it correctly', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + // create a rule with the action attached + const { body: rule } = await supertest + .post(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send(getRuleWithWebHookAction(hookAction.id)) + .expect(200); + + // wait for Task Manager to execute the rule and its update status + await waitFor(async () => { + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`) + .set('kbn-xsrf', 'true') + .send({ ids: [rule.id] }) + .expect(200); + return body[rule.id].current_status?.status === 'succeeded'; + }); + + // expected result for status should be 'succeeded' + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`) + .set('kbn-xsrf', 'true') + .send({ ids: [rule.id] }) + .expect(200); + expect(body[rule.id].current_status.status).to.eql('succeeded'); + }); + + it('should be able to create a new webhook action and attach it to a rule with a meta field and run it correctly', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + // create a rule with the action attached and a meta field + const ruleWithAction: CreateRulesSchema = { + ...getRuleWithWebHookAction(hookAction.id), + meta: {}, + }; + + const { body: rule } = await supertest + .post(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send(ruleWithAction) + .expect(200); + + // wait for Task Manager to execute the rule and update status + await waitFor(async () => { + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`) + .set('kbn-xsrf', 'true') + .send({ ids: [rule.id] }) + .expect(200); + return body[rule.id].current_status?.status === 'succeeded'; + }); + + // expected result for status should be 'succeeded' + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_find_statuses`) + .set('kbn-xsrf', 'true') + .send({ ids: [rule.id] }) + .expect(200); + expect(body[rule.id].current_status.status).to.eql('succeeded'); + }); + }); + }); +}; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index a480e63ff4a92..779205377621d 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -11,6 +11,7 @@ export default ({ loadTestFile }: FtrProviderContext): void => { describe('detection engine api security and spaces enabled', function () { this.tags('ciGroup1'); + loadTestFile(require.resolve('./add_actions')); loadTestFile(require.resolve('./add_prepackaged_rules')); loadTestFile(require.resolve('./create_rules')); loadTestFile(require.resolve('./create_rules_bulk')); diff --git a/x-pack/test/detection_engine_api_integration/utils.ts b/x-pack/test/detection_engine_api_integration/utils.ts index 604133a1c2dc7..4cbbc142edd40 100644 --- a/x-pack/test/detection_engine_api_integration/utils.ts +++ b/x-pack/test/detection_engine_api_integration/utils.ts @@ -557,6 +557,49 @@ export const getComplexRuleOutput = (ruleId = 'rule-1'): Partial => exceptions_list: [], }); +export const getWebHookAction = () => ({ + actionTypeId: '.webhook', + config: { + method: 'post', + url: 'http://localhost', + }, + secrets: { + user: 'example', + password: 'example', + }, + name: 'Some connector', +}); + +export const getRuleWithWebHookAction = (id: string): CreateRulesSchema => ({ + ...getSimpleRule(), + throttle: 'rule', + actions: [ + { + group: 'default', + id, + params: { + body: '{}', + }, + action_type_id: '.webhook', + }, + ], +}); + +export const getSimpleRuleOutputWithWebHookAction = (actionId: string): Partial => ({ + ...getSimpleRuleOutput(), + throttle: 'rule', + actions: [ + { + action_type_id: '.webhook', + group: 'default', + id: actionId, + params: { + body: '{}', + }, + }, + ], +}); + // Similar to ReactJs's waitFor from here: https://testing-library.com/docs/dom-testing-library/api-async#waitfor export const waitFor = async ( functionToTest: () => Promise,