From 4e1b1b5d9e3ab98e177c5b8c763d08918784aad7 Mon Sep 17 00:00:00 2001 From: Bhavya RM Date: Wed, 26 Aug 2020 10:02:10 -0400 Subject: [PATCH 1/6] adding test user to auto fit to bounds test (#75914) --- x-pack/test/functional/apps/maps/auto_fit_to_bounds.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/x-pack/test/functional/apps/maps/auto_fit_to_bounds.js b/x-pack/test/functional/apps/maps/auto_fit_to_bounds.js index d3d4fe054ec3..0e8775fa611b 100644 --- a/x-pack/test/functional/apps/maps/auto_fit_to_bounds.js +++ b/x-pack/test/functional/apps/maps/auto_fit_to_bounds.js @@ -6,17 +6,23 @@ import expect from '@kbn/expect'; -export default function ({ getPageObjects }) { +export default function ({ getPageObjects, getService }) { const PageObjects = getPageObjects(['maps']); + const security = getService('security'); describe('auto fit map to bounds', () => { describe('initial location', () => { before(async () => { + await security.testUser.setRoles(['global_maps_all', 'test_logstash_reader']); await PageObjects.maps.loadSavedMap( 'document example - auto fit to bounds for initial location' ); }); + after(async () => { + await security.testUser.restoreDefaults(); + }); + it('should automatically fit to bounds on initial map load', async () => { const hits = await PageObjects.maps.getHits(); expect(hits).to.equal('6'); From b9c820120202dc44296e080550e87c93bd37dd55 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 26 Aug 2020 10:16:17 -0400 Subject: [PATCH 2/6] [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]; +}; From d6c45a2e70a20d552c91f3df89da1cd081077209 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Wed, 26 Aug 2020 09:01:32 -0600 Subject: [PATCH 3/6] Fixes runtime error with meta when it is missing (#75844) ## Summary Found in 7.9.0, if you post a rule with an action that has a missing "meta" then you are going to get errors in your UI that look something like: ```ts An error occurred during rule execution: message: "Cannot read property 'kibana_siem_app_url' of null" name: "Unusual Windows Remote User" id: "1cc27e7e-d7c7-4f6a-b918-8c272fc6b1a3" rule id: "1781d055-5c66-4adf-9e93-fc0fa69550c9" signals index: ".siem-signals-default" ``` This fixes the accidental referencing of the null/undefined property and adds both integration and unit tests in that area of code. If you have an action id handy you can manually test this by editing the json file of: ```ts test_cases/queries/action_without_meta.json ``` to have your action id and then posting it like so: ```ts ./post_rule.sh ./rules/test_cases/queries/action_without_meta.json ``` ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --- .../rules_notification_alert_type.test.ts | 78 ++++++++++ .../rules_notification_alert_type.ts | 4 +- .../queries/action_without_meta.json | 42 ++++++ .../signals/signal_rule_alert_type.test.ts | 100 +++++++++++++ .../signals/signal_rule_alert_type.ts | 3 +- .../security_and_spaces/tests/add_actions.ts | 134 ++++++++++++++++++ .../security_and_spaces/tests/index.ts | 1 + .../detection_engine_api_integration/utils.ts | 43 ++++++ 8 files changed, 402 insertions(+), 3 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/test_cases/queries/action_without_meta.json create mode 100644 x-pack/test/detection_engine_api_integration/security_and_spaces/tests/add_actions.ts 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 3eefd3e665cd..593ada470b11 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 ab824957087f..2eb34529d044 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 000000000000..6569a641de3a --- /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 b0c855afa8be..b29d15f5e5c7 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 0e859ecef31c..b5cbf80b084f 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 000000000000..246885123704 --- /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 a480e63ff4a9..779205377621 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 604133a1c2dc..4cbbc142edd4 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, From e773f221a3814700d55284bc34bd4637cc7312bd Mon Sep 17 00:00:00 2001 From: Tyler Smalley Date: Wed, 26 Aug 2020 08:41:09 -0700 Subject: [PATCH 4/6] Revert "[Security Solution][Exceptions] - Improve UX for missing exception list associated with rule (#75898)" This reverts commit b9c820120202dc44296e080550e87c93bd37dd55. --- .../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, 54 insertions(+), 706 deletions(-) delete mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.test.tsx delete mode 100644 x-pack/plugins/security_solution/public/common/components/exceptions/error_callout.tsx delete mode 100644 x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/use_dissasociate_exception_list.test.tsx delete 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 21f82c6ab4c9..03051ead357c 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 c724e6a2c711..6ff218ca0605 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 c9efa5e54dcc..000000000000 --- 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 a2419ef16df3..000000000000 --- 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 484a3d593026..13e9d0df549f 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 46923e07d225..6611ee2385d1 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 be289b0e85e6..9d45a411b513 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 f20a58b9ffa3..39d88bd8e472 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 944631d4e9fb..0d367e03a799 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 c97895cdfe23..7482068454a9 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 6b1938655dc3..000000000000 --- 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 dffba3e6e043..000000000000 --- 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]; -}; From 5a9d227eee1b53673c6445c00746a0846bb69e48 Mon Sep 17 00:00:00 2001 From: Tyler Smalley Date: Wed, 26 Aug 2020 08:03:12 -0700 Subject: [PATCH 5/6] Downloads Chrome 84 and adds to PATH Signed-off-by: Tyler Smalley --- src/dev/ci_setup/setup_env.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/dev/ci_setup/setup_env.sh b/src/dev/ci_setup/setup_env.sh index 72ec73ad810e..a82ca011b8a5 100644 --- a/src/dev/ci_setup/setup_env.sh +++ b/src/dev/ci_setup/setup_env.sh @@ -10,6 +10,7 @@ installNode=$1 dir="$(pwd)" cacheDir="$HOME/.kibana" +downloads="$cacheDir/downloads" RED='\033[0;31m' C_RESET='\033[0m' # Reset color @@ -133,6 +134,26 @@ export CYPRESS_DOWNLOAD_MIRROR="https://us-central1-elastic-kibana-184716.cloudf export CHECKS_REPORTER_ACTIVE=false +### +### Download Chrome and install to this shell +### + +# Available using the version information search at https://omahaproxy.appspot.com/ +chromeVersion=84 + +mkdir -p "$downloads" + +if [ -d $cacheDir/chrome-$chromeVersion/chrome-linux ]; then + echo " -- Chrome already downloaded and extracted" +else + mkdir -p "$cacheDir/chrome-$chromeVersion" + + echo " -- Downloading and extracting Chrome" + curl -o "$downloads/chrome.zip" -L "https://us-central1-elastic-kibana-184716.cloudfunctions.net/kibana-ci-proxy-cache/chrome_$chromeVersion.zip" + unzip -o "$downloads/chrome.zip" -d "$cacheDir/chrome-$chromeVersion" + export PATH="$cacheDir/chrome-$chromeVersion/chrome-linux:$PATH" +fi + # This is mainly for release-manager builds, which run in an environment that doesn't have Chrome installed if [[ "$(which google-chrome-stable)" || "$(which google-chrome)" ]]; then echo "Chrome detected, setting DETECT_CHROMEDRIVER_VERSION=true" From 1ca76514933463220e32f4b246c5ba14a553d9a9 Mon Sep 17 00:00:00 2001 From: spalger Date: Wed, 26 Aug 2020 09:28:22 -0700 Subject: [PATCH 6/6] Revert "Downloads Chrome 84 and adds to PATH" This reverts commit 5a9d227eee1b53673c6445c00746a0846bb69e48. --- src/dev/ci_setup/setup_env.sh | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/src/dev/ci_setup/setup_env.sh b/src/dev/ci_setup/setup_env.sh index a82ca011b8a5..72ec73ad810e 100644 --- a/src/dev/ci_setup/setup_env.sh +++ b/src/dev/ci_setup/setup_env.sh @@ -10,7 +10,6 @@ installNode=$1 dir="$(pwd)" cacheDir="$HOME/.kibana" -downloads="$cacheDir/downloads" RED='\033[0;31m' C_RESET='\033[0m' # Reset color @@ -134,26 +133,6 @@ export CYPRESS_DOWNLOAD_MIRROR="https://us-central1-elastic-kibana-184716.cloudf export CHECKS_REPORTER_ACTIVE=false -### -### Download Chrome and install to this shell -### - -# Available using the version information search at https://omahaproxy.appspot.com/ -chromeVersion=84 - -mkdir -p "$downloads" - -if [ -d $cacheDir/chrome-$chromeVersion/chrome-linux ]; then - echo " -- Chrome already downloaded and extracted" -else - mkdir -p "$cacheDir/chrome-$chromeVersion" - - echo " -- Downloading and extracting Chrome" - curl -o "$downloads/chrome.zip" -L "https://us-central1-elastic-kibana-184716.cloudfunctions.net/kibana-ci-proxy-cache/chrome_$chromeVersion.zip" - unzip -o "$downloads/chrome.zip" -d "$cacheDir/chrome-$chromeVersion" - export PATH="$cacheDir/chrome-$chromeVersion/chrome-linux:$PATH" -fi - # This is mainly for release-manager builds, which run in an environment that doesn't have Chrome installed if [[ "$(which google-chrome-stable)" || "$(which google-chrome)" ]]; then echo "Chrome detected, setting DETECT_CHROMEDRIVER_VERSION=true"