From 010088d0a5b19cd78d3be43287dbc2f6f8bdd324 Mon Sep 17 00:00:00 2001 From: Ievgen Sorokopud Date: Mon, 26 Aug 2024 11:11:16 +0200 Subject: [PATCH] [Security Solution] Editing rules independently of source data (#180407) --- .../save_with_errors_confirmation/index.tsx | 53 ++++ .../translations.ts | 36 +++ .../rule_creation_ui/pages/form.tsx | 87 +++++- .../pages/rule_creation/index.tsx | 250 ++++++++++++------ .../pages/rule_editing/index.tsx | 103 ++++++-- .../rule_creation_ui/pages/translations.ts | 6 + .../rule_creation/eql_rule.cy.ts | 20 +- .../rule_creation/esql_rule.cy.ts | 16 ++ .../rule_edit/eql_query_rule.cy.ts | 47 ++++ .../rule_edit/esql_rule.cy.ts | 30 ++- .../cypress/screens/create_new_rule.ts | 4 + .../cypress/tasks/create_new_rule.ts | 10 + .../cypress/tasks/edit_rule.ts | 12 + 13 files changed, 566 insertions(+), 108 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx create mode 100644 x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts create mode 100644 x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx new file mode 100644 index 0000000000000..3f14945bedadc --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/index.tsx @@ -0,0 +1,53 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { EuiConfirmModal, EuiSpacer, EuiText } from '@elastic/eui'; + +import * as i18n from './translations'; + +interface SaveWithErrorsModalProps { + errors: string[]; + onCancel: () => void; + onConfirm: () => void; +} + +const SaveWithErrorsModalComponent = ({ + errors, + onCancel, + onConfirm, +}: SaveWithErrorsModalProps) => { + return ( + + <> + {i18n.SAVE_WITH_ERRORS_MODAL_MESSAGE(errors.length)} + +
    + {errors.map((validationError, idx) => { + return ( +
  • + {validationError} +
    • + ); + })} +
+ + + ); +}; + +export const SaveWithErrorsModal = React.memo(SaveWithErrorsModalComponent); +SaveWithErrorsModal.displayName = 'SaveWithErrorsModal'; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts new file mode 100644 index 0000000000000..e470b06c7e829 --- /dev/null +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/save_with_errors_confirmation/translations.ts @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { i18n } from '@kbn/i18n'; + +export const SAVE_WITH_ERRORS_MODAL_TITLE = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsModalTitle', + { + defaultMessage: 'This rule has validation errors', + } +); + +export const SAVE_WITH_ERRORS_CANCEL_BUTTON = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsCancelButton', + { + defaultMessage: 'Cancel', + } +); + +export const SAVE_WITH_ERRORS_CONFIRM_BUTTON = i18n.translate( + 'xpack.securitySolution.detectionEngine.createRule.saveWithErrorsConfirmButton', + { + defaultMessage: 'Confirm', + } +); + +export const SAVE_WITH_ERRORS_MODAL_MESSAGE = (errorsCount: number) => + i18n.translate('xpack.securitySolution.detectionEngine.createRule.saveWithErrorsModalMessage', { + defaultMessage: + 'This rule has {errorsCount} validation {errorsCount, plural, one {error} other {errors}} which can lead to failed rule executions, save anyway?', + values: { errorsCount }, + }); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx index 690dedafaf852..d7e3f4566a887 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/form.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import { useState, useMemo, useEffect } from 'react'; +import { useState, useMemo, useEffect, useCallback } from 'react'; import type { DataViewBase } from '@kbn/es-query'; import { isThreatMatchRule } from '../../../../common/detection_engine/utils'; import type { @@ -16,6 +16,7 @@ import type { } from '../../../detections/pages/detection_engine/rules/types'; import { DataSourceType } from '../../../detections/pages/detection_engine/rules/types'; import { useKibana } from '../../../common/lib/kibana'; +import type { FormHook, ValidationError } from '../../../shared_imports'; import { useForm, useFormData } from '../../../shared_imports'; import { schema as defineRuleSchema } from '../components/step_define_rule/schema'; import type { EqlOptionsSelected } from '../../../../common/search_strategy'; @@ -26,6 +27,9 @@ import { import { schema as scheduleRuleSchema } from '../components/step_schedule_rule/schema'; import { getSchema as getActionsRuleSchema } from '../../rule_creation/components/step_rule_actions/get_schema'; import { useFetchIndex } from '../../../common/containers/source'; +import { ERROR_CODES as ESQL_ERROR_CODES } from '../../rule_creation/logic/esql_validator'; +import { EQL_ERROR_CODES } from '../../../common/hooks/eql/api'; +import * as i18n from './translations'; export interface UseRuleFormsProps { defineStepDefault: DefineStepRule; @@ -156,3 +160,84 @@ export const useRuleIndexPattern = ({ }, [dataSourceType, isIndexPatternLoading, data, dataViewId, initIndexPattern]); return { indexPattern, isIndexPatternLoading, browserFields }; }; + +export interface UseRuleFormsErrors { + defineStepForm?: FormHook; + aboutStepForm?: FormHook; + scheduleStepForm?: FormHook; + actionsStepForm?: FormHook; +} + +const getFieldErrorMessages = (fieldError: ValidationError) => { + if (fieldError.message.length > 0) { + return [fieldError.message]; + } else if (Array.isArray(fieldError.messages)) { + return fieldError.messages.map((message) => JSON.stringify(message)); + } + return []; +}; + +const NON_BLOCKING_QUERY_BAR_ERROR_CODES = [ + ESQL_ERROR_CODES.INVALID_ESQL, + EQL_ERROR_CODES.FAILED_REQUEST, + EQL_ERROR_CODES.INVALID_EQL, + EQL_ERROR_CODES.MISSING_DATA_SOURCE, +]; + +const isNonBlockingQueryBarErrorCode = (errorCode?: string) => { + return !!NON_BLOCKING_QUERY_BAR_ERROR_CODES.find((code) => code === errorCode); +}; + +const NON_BLOCKING_ERROR_CODES = [...NON_BLOCKING_QUERY_BAR_ERROR_CODES]; + +const isNonBlockingErrorCode = (errorCode?: string) => { + return !!NON_BLOCKING_ERROR_CODES.find((code) => code === errorCode); +}; + +const transformValidationError = ({ + errorCode, + errorMessage, +}: { + errorCode?: string; + errorMessage: string; +}) => { + if (isNonBlockingQueryBarErrorCode(errorCode)) { + return i18n.QUERY_BAR_VALIDATION_ERROR(errorMessage); + } + return errorMessage; +}; + +export const useRuleFormsErrors = () => { + const getRuleFormsErrors = useCallback( + ({ defineStepForm, aboutStepForm, scheduleStepForm, actionsStepForm }: UseRuleFormsErrors) => { + const blockingErrors: string[] = []; + const nonBlockingErrors: string[] = []; + + for (const [_, fieldHook] of Object.entries(defineStepForm?.getFields() ?? {})) { + fieldHook.errors.forEach((fieldError) => { + const messages = getFieldErrorMessages(fieldError); + if (isNonBlockingErrorCode(fieldError.code)) { + nonBlockingErrors.push( + ...messages.map((message) => + transformValidationError({ errorCode: fieldError.code, errorMessage: message }) + ) + ); + } else { + blockingErrors.push(...messages); + } + }); + } + + const blockingForms = [aboutStepForm, scheduleStepForm, actionsStepForm]; + blockingForms.forEach((form) => { + for (const [_, fieldHook] of Object.entries(form?.getFields() ?? {})) { + blockingErrors.push(...fieldHook.errors.map((fieldError) => fieldError.message)); + } + }); + return { blockingErrors, nonBlockingErrors }; + }, + [] + ); + + return { getRuleFormsErrors }; +}; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx index 6e93d6927b6be..97b89630392d8 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx @@ -80,8 +80,9 @@ import { RulePreview } from '../../components/rule_preview'; import { getIsRulePreviewDisabled } from '../../components/rule_preview/helpers'; import { useStartMlJobs } from '../../../rule_management/logic/use_start_ml_jobs'; import { NextStep } from '../../components/next_step'; -import { useRuleForms, useRuleIndexPattern } from '../form'; +import { useRuleForms, useRuleFormsErrors, useRuleIndexPattern } from '../form'; import { CustomHeaderPageMemo } from '..'; +import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; const MyEuiPanel = styled(EuiPanel)<{ zindex?: number; @@ -210,6 +211,12 @@ const CreateRulePageComponent: React.FC = () => { const [isQueryBarValid, setIsQueryBarValid] = useState(false); const [isThreatQueryBarValid, setIsThreatQueryBarValid] = useState(false); + const [isSaveWithErrorsModalVisible, setIsSaveWithErrorsModalVisible] = useState(false); + const [enableRuleAfterConfirmation, setEnableRuleAfterConfirmation] = useState(false); + const [nonBlockingRuleErrors, setNonBlockingRuleErrors] = useState([]); + + const { getRuleFormsErrors } = useRuleFormsErrors(); + const esqlQueryForAboutStep = useEsqlQueryForAboutStep({ defineStepData, activeStep }); const esqlIndex = useEsqlIndex(defineStepData.queryBar.query.query, ruleType); @@ -328,89 +335,173 @@ const CreateRulePageComponent: React.FC = () => { const validateStep = useCallback( async (step: RuleStep) => { switch (step) { - case RuleStep.defineRule: - return defineStepForm.validate(); - case RuleStep.aboutRule: - return aboutStepForm.validate(); - case RuleStep.scheduleRule: - return scheduleStepForm.validate(); - case RuleStep.ruleActions: - return actionsStepForm.validate(); + case RuleStep.defineRule: { + const valid = await defineStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ defineStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.aboutRule: { + const valid = await aboutStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ aboutStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.scheduleRule: { + const valid = await scheduleStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ scheduleStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } + case RuleStep.ruleActions: { + const valid = await actionsStepForm.validate(); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ actionsStepForm }); + return { valid, blockingErrors, nonBlockingErrors }; + } } }, - [aboutStepForm, actionsStepForm, defineStepForm, scheduleStepForm] + [aboutStepForm, actionsStepForm, defineStepForm, getRuleFormsErrors, scheduleStepForm] + ); + + const validateEachStep = useCallback(async () => { + const { + valid: defineStepFormValid, + blockingErrors: defineStepBlockingErrors, + nonBlockingErrors: defineStepNonBlockingErrors, + } = await validateStep(RuleStep.defineRule); + const { + valid: aboutStepFormValid, + blockingErrors: aboutStepBlockingErrors, + nonBlockingErrors: aboutStepNonBlockingErrors, + } = await validateStep(RuleStep.aboutRule); + const { + valid: scheduleStepFormValid, + blockingErrors: scheduleStepBlockingErrors, + nonBlockingErrors: scheduleStepNonBlockingErrors, + } = await validateStep(RuleStep.scheduleRule); + const { + valid: actionsStepFormValid, + blockingErrors: actionsStepBlockingErrors, + nonBlockingErrors: actionsStepNonBlockingErrors, + } = await validateStep(RuleStep.ruleActions); + const valid = + defineStepFormValid && aboutStepFormValid && scheduleStepFormValid && actionsStepFormValid; + + const blockingErrors = [ + ...defineStepBlockingErrors, + ...aboutStepBlockingErrors, + ...scheduleStepBlockingErrors, + ...actionsStepBlockingErrors, + ]; + const nonBlockingErrors = [ + ...defineStepNonBlockingErrors, + ...aboutStepNonBlockingErrors, + ...scheduleStepNonBlockingErrors, + ...actionsStepNonBlockingErrors, + ]; + + return { valid, blockingErrors, nonBlockingErrors }; + }, [validateStep]); + + const isValidStep = useCallback( + async (step: RuleStep) => { + const { valid, blockingErrors } = await validateStep(step); + return valid || !blockingErrors.length; + }, + [validateStep] ); const editStep = useCallback( async (step: RuleStep) => { - const valid = await validateStep(activeStep); - + const valid = await isValidStep(activeStep); if (valid) { goToStep(step); } }, - [activeStep, validateStep, goToStep] + [isValidStep, activeStep, goToStep] ); - const submitRule = useCallback( - async (step: RuleStep, enabled: boolean) => { - const valid = await validateStep(step); - - if (valid) { - const localDefineStepData: DefineStepRule = defineFieldsTransform({ - ...defineStepForm.getFormData(), - eqlOptions: eqlOptionsSelected, - }); - const localAboutStepData = aboutStepForm.getFormData(); - const localScheduleStepData = scheduleStepForm.getFormData(); - const localActionsStepData = actionsStepForm.getFormData(); - const startMlJobsIfNeeded = async () => { - if (!isMlRule(ruleType) || !enabled) { - return; - } - await startMlJobs(localDefineStepData.machineLearningJobId); - }; - const [, createdRule] = await Promise.all([ - startMlJobsIfNeeded(), - createRule( - formatRule( - localDefineStepData, - localAboutStepData, - localScheduleStepData, - { - ...localActionsStepData, - enabled, - }, - triggersActionsUi.actionTypeRegistry - ) - ), - ]); - - addSuccess(i18n.SUCCESSFULLY_CREATED_RULES(createdRule.name)); - - navigateToApp(APP_UI_ID, { - deepLinkId: SecurityPageName.rules, - path: getRuleDetailsUrl(createdRule.id), - }); - } + const createRuleFromFormData = useCallback( + async (enabled: boolean) => { + const localDefineStepData: DefineStepRule = defineFieldsTransform({ + ...defineStepForm.getFormData(), + eqlOptions: eqlOptionsSelected, + }); + const localAboutStepData = aboutStepForm.getFormData(); + const localScheduleStepData = scheduleStepForm.getFormData(); + const localActionsStepData = actionsStepForm.getFormData(); + const startMlJobsIfNeeded = async () => { + if (!isMlRule(ruleType) || !enabled) { + return; + } + await startMlJobs(localDefineStepData.machineLearningJobId); + }; + const [, createdRule] = await Promise.all([ + startMlJobsIfNeeded(), + createRule( + formatRule( + localDefineStepData, + localAboutStepData, + localScheduleStepData, + { + ...localActionsStepData, + enabled, + }, + triggersActionsUi.actionTypeRegistry + ) + ), + ]); + + addSuccess(i18n.SUCCESSFULLY_CREATED_RULES(createdRule.name)); + + navigateToApp(APP_UI_ID, { + deepLinkId: SecurityPageName.rules, + path: getRuleDetailsUrl(createdRule.id), + }); }, [ - validateStep, - defineStepForm, - eqlOptionsSelected, aboutStepForm, - scheduleStepForm, actionsStepForm, - createRule, addSuccess, + createRule, + defineFieldsTransform, + defineStepForm, + eqlOptionsSelected, navigateToApp, ruleType, + scheduleStepForm, startMlJobs, - defineFieldsTransform, triggersActionsUi.actionTypeRegistry, ] ); + const submitRule = useCallback( + async (enabled: boolean) => { + const { valid, blockingErrors, nonBlockingErrors } = await validateEachStep(); + if (valid) { + // There are no validation errors, thus proceed to rule creation + createRuleFromFormData(enabled); + return; + } + + if (blockingErrors.length > 0) { + // There are blocking validation errors, thus do not allow user to create a rule + return; + } + if (nonBlockingErrors.length > 0) { + // There are non-blocking validation errors, thus confirm that user understand that this can cause rule failures + setEnableRuleAfterConfirmation(enabled); + setNonBlockingRuleErrors(nonBlockingErrors); + showSaveWithErrorsModal(); + } + }, + [createRuleFromFormData, validateEachStep] + ); + + const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); + const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await createRuleFromFormData(enableRuleAfterConfirmation); + }, [createRuleFromFormData, enableRuleAfterConfirmation]); + const defineRuleButtonType = activeStep === RuleStep.defineRule ? 'active' : defineStepForm.isValid ? 'valid' : 'passive'; const defineRuleButton = useMemo( @@ -418,14 +509,14 @@ const CreateRulePageComponent: React.FC = () => { [defineRuleButtonType] ); const defineRuleNextStep = useCallback(async () => { - const valid = await defineStepForm.validate(); + const valid = await isValidStep(RuleStep.defineRule); if (valid) { const nextStep = getNextStep(RuleStep.defineRule); if (nextStep) { goToStep(nextStep); } } - }, [defineStepForm, goToStep]); + }, [goToStep, isValidStep]); const aboutRuleButtonType = activeStep === RuleStep.aboutRule ? 'active' : aboutStepForm.isValid ? 'valid' : 'passive'; @@ -434,14 +525,14 @@ const CreateRulePageComponent: React.FC = () => { [aboutRuleButtonType] ); const aboutRuleNextStep = useCallback(async () => { - const valid = await aboutStepForm.validate(); + const valid = await isValidStep(RuleStep.aboutRule); if (valid) { const nextStep = getNextStep(RuleStep.aboutRule); if (nextStep) { goToStep(nextStep); } } - }, [aboutStepForm, goToStep]); + }, [goToStep, isValidStep]); const scheduleRuleButtonType = activeStep === RuleStep.scheduleRule @@ -454,14 +545,14 @@ const CreateRulePageComponent: React.FC = () => { [scheduleRuleButtonType] ); const scheduleRuleNextStep = useCallback(async () => { - const valid = await scheduleStepForm.validate(); + const valid = await isValidStep(RuleStep.scheduleRule); if (valid) { const nextStep = getNextStep(RuleStep.scheduleRule); if (nextStep) { goToStep(nextStep); } } - }, [scheduleStepForm, goToStep]); + }, [isValidStep, goToStep]); const actionsRuleButtonType = activeStep === RuleStep.ruleActions ? 'active' : actionsStepForm.isValid ? 'valid' : 'passive'; @@ -470,10 +561,10 @@ const CreateRulePageComponent: React.FC = () => { [actionsRuleButtonType] ); const submitRuleDisabled = useCallback(() => { - submitRule(RuleStep.ruleActions, false); + submitRule(false); }, [submitRule]); const submitRuleEnabled = useCallback(() => { - submitRule(RuleStep.ruleActions, true); + submitRule(true); }, [submitRule]); const memoDefineStepReadOnly = useMemo( @@ -559,7 +650,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoDefineStepExtraAction = useMemo( () => - defineStepForm.isValid && ( + activeStep !== RuleStep.defineRule && ( { {i18n.EDIT_RULE} ), - [defineStepForm.isValid, editStep] + [activeStep, editStep] ); const memoAboutStepReadOnly = useMemo( @@ -629,7 +720,7 @@ const CreateRulePageComponent: React.FC = () => { ); const memoAboutStepExtraAction = useMemo( () => - aboutStepForm.isValid && ( + activeStep !== RuleStep.aboutRule && ( { {i18n.EDIT_RULE} ), - [aboutStepForm.isValid, editStep] + [activeStep, editStep] ); const memoStepScheduleRule = useMemo( @@ -682,12 +773,12 @@ const CreateRulePageComponent: React.FC = () => { ); const memoScheduleStepExtraAction = useMemo( () => - scheduleStepForm.isValid && ( + activeStep !== RuleStep.scheduleRule && ( editStep(RuleStep.scheduleRule)}> {i18n.EDIT_RULE} ), - [editStep, scheduleStepForm.isValid] + [activeStep, editStep] ); const memoStepRuleActions = useMemo( @@ -762,12 +853,12 @@ const CreateRulePageComponent: React.FC = () => { ); const memoActionsStepExtraAction = useMemo( () => - actionsStepForm.isValid && ( + activeStep !== RuleStep.ruleActions && ( editStep(RuleStep.ruleActions)}> {i18n.EDIT_RULE} ), - [actionsStepForm.isValid, editStep] + [activeStep, editStep] ); const onToggleCollapsedMemo = useCallback( @@ -798,6 +889,13 @@ const CreateRulePageComponent: React.FC = () => { return ( <> + {isSaveWithErrorsModalVisible && ( + + )} {(EuiResizablePanel, EuiResizableButton, { togglePanel }) => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx index 5e6ed40d4d282..0544f01bb1b7a 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx @@ -67,9 +67,10 @@ import { import { useStartTransaction } from '../../../../common/lib/apm/use_start_transaction'; import { SINGLE_RULE_ACTIONS } from '../../../../common/lib/apm/user_actions'; import { useGetSavedQuery } from '../../../../detections/pages/detection_engine/rules/use_get_saved_query'; -import { useRuleForms, useRuleIndexPattern } from '../form'; +import { useRuleForms, useRuleFormsErrors, useRuleIndexPattern } from '../form'; import { useEsqlIndex, useEsqlQueryForAboutStep } from '../../hooks'; import { CustomHeaderPageMemo } from '..'; +import { SaveWithErrorsModal } from '../../components/save_with_errors_confirmation'; const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const [, dispatchToaster] = useStateToaster(); @@ -99,6 +100,9 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const [isQueryBarValid, setIsQueryBarValid] = useState(false); const [isThreatQueryBarValid, setIsThreatQueryBarValid] = useState(false); + const [isSaveWithErrorsModalVisible, setIsSaveWithErrorsModalVisible] = useState(false); + const [nonBlockingRuleErrors, setNonBlockingRuleErrors] = useState([]); + useEffect(() => { const fetchDataViews = async () => { const dataViewsRefs = await dataServices.dataViews.getIdsWithTitle(); @@ -149,6 +153,8 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { actionsStepDefault: ruleActionsData, }); + const { getRuleFormsErrors } = useRuleFormsErrors(); + const esqlQueryForAboutStep = useEsqlQueryForAboutStep({ defineStepData, activeStep }); const esqlIndex = useEsqlIndex(defineStepData.queryBar.query.query, defineStepData.ruleType); @@ -386,7 +392,43 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { const { startTransaction } = useStartTransaction(); + const saveChanges = useCallback(async () => { + startTransaction({ name: SINGLE_RULE_ACTIONS.SAVE }); + await updateRule({ + ...formatRule( + defineStepData, + aboutStepData, + scheduleStepData, + actionsStepData, + triggersActionsUi.actionTypeRegistry, + rule?.exceptions_list + ), + ...(ruleId ? { id: ruleId } : {}), + }); + + displaySuccessToast(i18n.SUCCESSFULLY_SAVED_RULE(rule?.name ?? ''), dispatchToaster); + navigateToApp(APP_UI_ID, { + deepLinkId: SecurityPageName.rules, + path: getRuleDetailsUrl(ruleId ?? ''), + }); + }, [ + aboutStepData, + actionsStepData, + defineStepData, + dispatchToaster, + navigateToApp, + rule?.exceptions_list, + rule?.name, + ruleId, + scheduleStepData, + startTransaction, + triggersActionsUi.actionTypeRegistry, + updateRule, + ]); + const onSubmit = useCallback(async () => { + setNonBlockingRuleErrors([]); + const defineStepFormValid = await defineStepForm.validate(); const aboutStepFormValid = await aboutStepForm.validate(); const scheduleStepFormValid = await scheduleStepForm.validate(); @@ -398,43 +440,39 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { scheduleStepFormValid && actionsStepFormValid ) { - startTransaction({ name: SINGLE_RULE_ACTIONS.SAVE }); - await updateRule({ - ...formatRule( - defineStepData, - aboutStepData, - scheduleStepData, - actionsStepData, - triggersActionsUi.actionTypeRegistry, - rule?.exceptions_list - ), - ...(ruleId ? { id: ruleId } : {}), - }); + await saveChanges(); + return; + } - displaySuccessToast(i18n.SUCCESSFULLY_SAVED_RULE(rule?.name ?? ''), dispatchToaster); - navigateToApp(APP_UI_ID, { - deepLinkId: SecurityPageName.rules, - path: getRuleDetailsUrl(ruleId ?? ''), - }); + const { blockingErrors, nonBlockingErrors } = getRuleFormsErrors({ + defineStepForm, + aboutStepForm, + scheduleStepForm, + actionsStepForm, + }); + if (blockingErrors.length > 0) { + return; + } + if (nonBlockingErrors.length > 0) { + setNonBlockingRuleErrors(nonBlockingErrors); + showSaveWithErrorsModal(); } }, [ defineStepForm, aboutStepForm, scheduleStepForm, actionsStepForm, - startTransaction, - updateRule, - defineStepData, - aboutStepData, - scheduleStepData, - actionsStepData, - rule, - ruleId, - dispatchToaster, - navigateToApp, - triggersActionsUi.actionTypeRegistry, + getRuleFormsErrors, + saveChanges, ]); + const showSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(true); + const closeSaveWithErrorsModal = () => setIsSaveWithErrorsModalVisible(false); + const onConfirmSaveWithErrors = useCallback(async () => { + closeSaveWithErrorsModal(); + await saveChanges(); + }, [saveChanges]); + const onTabClick = useCallback(async (tab: EuiTabbedContentTab) => { const targetStep = tab.id as RuleStep; setActiveStep(targetStep); @@ -488,6 +526,13 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { return ( <> + {isSaveWithErrorsModalVisible && ( + + )} {(EuiResizablePanel, EuiResizableButton, { togglePanel }) => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts index e602b8be712c2..7a2064dd4f35c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/translations.ts @@ -13,3 +13,9 @@ export const RULE_PREVIEW_TITLE = i18n.translate( defaultMessage: 'Rule preview', } ); + +export const QUERY_BAR_VALIDATION_ERROR = (vlidationError: string) => + i18n.translate('xpack.securitySolution.detectionEngine.createRule.validationError', { + values: { vlidationError }, + defaultMessage: 'Query bar: {vlidationError}', + }); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts index 2b0b9f3fdab61..a14218fcdda56 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/eql_rule.cy.ts @@ -46,10 +46,13 @@ import { getDetails } from '../../../../tasks/rule_details'; import { expectNumberOfRules, goToRuleDetailsOf } from '../../../../tasks/alerts_detection_rules'; import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common'; import { + continueFromDefineStep, createAndEnableRule, + createRuleWithNonBlockingErrors, fillAboutRuleAndContinue, fillDefineEqlRuleAndContinue, fillScheduleRuleAndContinue, + getDefineContinueButton, getIndexPatternClearButton, getRuleIndexInput, selectEqlRuleType, @@ -225,6 +228,8 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { }); describe('EQL query validation', () => { + const rule = getEqlRule(); + it('validates missing data source', () => { login(); visit(CREATE_RULE_URL); @@ -236,14 +241,20 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).should('be.visible'); cy.get(RULES_CREATION_FORM).find(EQL_QUERY_INPUT).type('any where true'); + const expectedValidationError = `index_not_found_exception\n\tCaused by:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]\n\tRoot causes:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]`; cy.get(EQL_QUERY_VALIDATION_ERROR).should('be.visible'); cy.get(EQL_QUERY_VALIDATION_ERROR).should('have.text', '1'); cy.get(EQL_QUERY_VALIDATION_ERROR).click(); cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should('be.visible'); cy.get(EQL_QUERY_VALIDATION_ERROR_CONTENT).should( 'have.text', - `EQL Validation Errorsindex_not_found_exception\n\tCaused by:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]\n\tRoot causes:\n\t\tverification_exception: Found 1 problem\nline -1:-1: Unknown index [*,-*]` + `EQL Validation Errors${expectedValidationError}` ); + continueFromDefineStep(); + + fillAboutRuleAndContinue(rule); + fillScheduleRuleAndContinue(rule); + createRuleWithNonBlockingErrors(); }); it('validates missing data fields', () => { @@ -263,6 +274,11 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { 'have.text', 'EQL Validation ErrorsFound 1 problem\nline 1:11: Unknown column [field1]' ); + continueFromDefineStep(); + + fillAboutRuleAndContinue(rule); + fillScheduleRuleAndContinue(rule); + createRuleWithNonBlockingErrors(); }); it('validates syntax errors', () => { @@ -282,6 +298,8 @@ describe('EQL rules', { tags: ['@ess', '@serverless'] }, () => { 'have.text', `EQL Validation Errorsline 1:6: extraneous input 'any' expecting 'where'` ); + continueFromDefineStep(); + getDefineContinueButton().should('exist'); }); }); }); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts index 348133b1d2802..3727c2ccbd501 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/esql_rule.cy.ts @@ -49,6 +49,7 @@ import { fillAboutRuleMinimumAndContinue, skipScheduleRuleAction, interceptEsqlQueryFieldsRequest, + createRuleWithNonBlockingErrors, } from '../../../../tasks/create_new_rule'; import { login } from '../../../../tasks/login'; import { visit } from '../../../../tasks/navigation'; @@ -191,6 +192,21 @@ describe( `Error validating ES|QL: "SyntaxError: extraneous input 'test' expecting "` ); }); + + it('shows confirmation modal about existing non-blocking validation errors', function () { + const nonExistingDataSourceQuery = 'from fake* metadata _id, _version, _index | limit 5'; + selectEsqlRuleType(); + fillEsqlQueryBar(nonExistingDataSourceQuery); + getDefineContinueButton().click(); + + fillRuleName(); + fillDescription(); + getAboutContinueButton().click(); + + fillScheduleRuleAndContinue(rule); + + createRuleWithNonBlockingErrors(); + }); }); describe('ES|QL investigation fields', () => { diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts new file mode 100644 index 0000000000000..994dbb2eb8ce8 --- /dev/null +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/eql_query_rule.cy.ts @@ -0,0 +1,47 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { getEqlRule } from '../../../../objects/rule'; + +import { createRule } from '../../../../tasks/api_calls/rules'; +import { deleteAlertsAndRules } from '../../../../tasks/api_calls/common'; +import { + saveEditedRuleWithNonBlockingErrors, + visitEditRulePage, +} from '../../../../tasks/edit_rule'; +import { login } from '../../../../tasks/login'; + +describe('EQL query rules', { tags: ['@ess', '@serverless'] }, () => { + context('Editing rule with non-blocking query validation errors', () => { + beforeEach(() => { + login(); + deleteAlertsAndRules(); + }); + + it('should allow user to save a rule and show confirmation modal when data source does not exist', () => { + const rule = { + ...getEqlRule(), + index: ['fake*'], + }; + createRule(rule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + + it('should allow user to save a rule and show confirmation modal when data field does not exist', () => { + const rule = { + ...getEqlRule(), + query: 'any where hello.world', + }; + createRule(rule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + }); +}); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts index 43655b1358b29..a180e1a0b50c2 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_edit/esql_rule.cy.ts @@ -43,7 +43,11 @@ import { login } from '../../../../tasks/login'; import { editFirstRule } from '../../../../tasks/alerts_detection_rules'; -import { saveEditedRule } from '../../../../tasks/edit_rule'; +import { + saveEditedRule, + saveEditedRuleWithNonBlockingErrors, + visitEditRulePage, +} from '../../../../tasks/edit_rule'; import { visit } from '../../../../tasks/navigation'; const rule = getEsqlRule(); @@ -192,5 +196,29 @@ describe( }); }); }); + + describe('Editing rule with non-blocking query validation errors', () => { + it('should allow user to save a rule and show confirmation modal when data source does not exist', () => { + const esqlRule = { + ...rule, + query: 'from fake-* metadata _id, _version, _index | keep agent.*,_id | eval test_id=_id', + }; + createRule(esqlRule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + + it('should allow user to save a rule and show confirmation modal when data field does not exist', () => { + const esqlRule = { + ...rule, + query: 'from auditbeat-* metadata _id, _version, _index | keep hello.world', + }; + createRule(esqlRule).then((createdRule) => { + visitEditRulePage(createdRule.body.id); + saveEditedRuleWithNonBlockingErrors(); + }); + }); + }); } ); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts b/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts index 4e6df1ed3a750..72d1104985d77 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/create_new_rule.ts @@ -56,6 +56,10 @@ export const CREATE_AND_ENABLE_BTN = '[data-test-subj="create-enable"]'; export const CREATE_WITHOUT_ENABLING_BTN = '[data-test-subj="create-enabled-false"]'; +export const SAVE_WITH_ERRORS_MODAL = '[data-test-subj="save-with-errors-confirmation-modal"]'; + +export const SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN = '[data-test-subj="confirmModalConfirmButton"]'; + export const CUSTOM_QUERY_INPUT = '[data-test-subj="queryInput"]'; export const CUSTOM_QUERY_BAR = '[data-test-subj="detectionEngineStepDefineRuleQueryBar"]'; diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts b/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts index 4251ef8ee0ec8..045c40da4b2cb 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/create_new_rule.ts @@ -128,6 +128,8 @@ import { MAX_SIGNALS_INPUT, SETUP_GUIDE_TEXTAREA, RELATED_INTEGRATION_COMBO_BOX_INPUT, + SAVE_WITH_ERRORS_MODAL, + SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN, } from '../screens/create_new_rule'; import { INDEX_SELECTOR, @@ -162,6 +164,14 @@ export const createRuleWithoutEnabling = () => { cy.get(CREATE_WITHOUT_ENABLING_BTN).should('not.exist'); }; +export const createRuleWithNonBlockingErrors = () => { + cy.get(CREATE_AND_ENABLE_BTN).click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('exist'); + cy.get(SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN).first().click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('not.exist'); + cy.get(CREATE_AND_ENABLE_BTN).should('not.exist'); +}; + export const fillAboutRule = (rule: RuleCreateProps) => { cy.get(RULE_NAME_INPUT).clear({ force: true }); cy.get(RULE_NAME_INPUT).type(rule.name, { force: true }); diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts b/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts index 14c9ef05aa878..4ea919231e08f 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/edit_rule.ts @@ -5,6 +5,10 @@ * 2.0. */ +import { + SAVE_WITH_ERRORS_MODAL, + SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN, +} from '../screens/create_new_rule'; import { BACK_TO_RULE_DETAILS, EDIT_SUBMIT_BUTTON } from '../screens/edit_rule'; import { editRuleUrl } from '../urls/edit_rule'; import { visit } from './navigation'; @@ -18,6 +22,14 @@ export const saveEditedRule = () => { cy.get(EDIT_SUBMIT_BUTTON).should('not.exist'); }; +export const saveEditedRuleWithNonBlockingErrors = () => { + cy.get(EDIT_SUBMIT_BUTTON).click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('exist'); + cy.get(SAVE_WITH_ERRORS_MODAL_CONFIRM_BTN).first().click(); + cy.get(SAVE_WITH_ERRORS_MODAL).should('not.exist'); + cy.get(EDIT_SUBMIT_BUTTON).should('not.exist'); +}; + export const goBackToRuleDetails = () => { cy.get(BACK_TO_RULE_DETAILS).should('exist').click(); cy.get(BACK_TO_RULE_DETAILS).should('not.exist');