[Security Solution] Rule is created when the conditional logic "If al…

…ert matches a query" is left blank (elastic#159690) ## Summary Original ticket: elastic#156706 These changes prevent user from creating/updating the rule when alert filter is selected and query left blank on the rule's action page. We gonna show an error saying "A custom query is required." in this case. <img width="1739" alt="Screenshot 2023-06-14 at 14 36 35" src="https://github.com/elastic/kibana/assets/2700761/0456f211-603c-44d9-9271-9cfdf59f12b6"> Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 9b6ad72)
kibanamachine · Jul 11, 2023 · d92dbbf · d92dbbf
1 parent 29376ed
commit d92dbbf
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 16 deletions.
diff --git a/...plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx b/...plugins/security_solution/public/detections/components/rules/rule_actions_field/index.tsx
@@ -119,7 +119,7 @@ export const RuleActionsField: React.FC<Props> = ({
 }) => {
   const [fieldErrors, setFieldErrors] = useState<string | null>(null);
   const form = useFormContext();
-  const { isSubmitted, isSubmitting, isValid } = form;
+  const { isValid } = form;
   const {
     triggersActionsUi: { getActionForm },
   } = useKibana().services;
@@ -231,6 +231,7 @@ export const RuleActionsField: React.FC<Props> = ({
     [field]
   );
 
+  const isFormValidated = isValid !== undefined;
   const actionForm = useMemo(
     () =>
       getActionForm({
@@ -251,6 +252,7 @@ export const RuleActionsField: React.FC<Props> = ({
         hasSummary: true,
         notifyWhenSelectOptions: NOTIFY_WHEN_OPTIONS,
         defaultRuleFrequency: NOTIFICATION_DEFAULT_FREQUENCY,
+        disableErrorMessages: !isFormValidated,
       }),
     [
       actions,
@@ -262,18 +264,17 @@ export const RuleActionsField: React.FC<Props> = ({
       setActionParamsProperty,
       setAlertActionsProperty,
       setActionAlertsFilterProperty,
+      isFormValidated,
     ]
   );
 
   useEffect(() => {
-    if (isSubmitting || !field.errors.length) {
-      return setFieldErrors(null);
-    }
-    if (isSubmitted && !isSubmitting && isValid === false && field.errors.length) {
+    if (isValid === false) {
       const errorsString = field.errors.map(({ message }) => message).join('\n');
       return setFieldErrors(errorsString);
     }
-  }, [isSubmitted, isSubmitting, field.isChangingValue, isValid, field.errors, setFieldErrors]);
+    return setFieldErrors(null);
+  }, [field.errors, isValid]);
 
   return (
     <ContainerActions $caseIndexes={caseActionIndexes}>

diff --git a/...tainers/detection_engine/rules/validate_rule_actions_field/validate_rule_actions_field.ts b/...tainers/detection_engine/rules/validate_rule_actions_field/validate_rule_actions_field.ts
@@ -16,6 +16,7 @@ import type {
   RuleAction,
   ActionTypeRegistryContract,
 } from '@kbn/triggers-actions-ui-plugin/public';
+import { validateActionFilterQuery } from '@kbn/triggers-actions-ui-plugin/public';
 import type { RuleActionsFormData } from '../../../../../detection_engine/rule_management_ui/components/rules_table/bulk_actions/forms/rule_actions_form';
 import type { ActionsStepRule } from '../../../../pages/detection_engine/rules/types';
 import type { ValidationFunc, ERROR_CODE } from '../../../../../shared_imports';
@@ -29,8 +30,9 @@ export const validateSingleAction = async (
 ): Promise<string[]> => {
   const actionParamsErrors = await validateActionParams(actionItem, actionTypeRegistry);
   const mustacheErrors = validateMustache(actionItem.params);
+  const queryErrors = validateActionFilterQuery(actionItem);
 
-  return [...actionParamsErrors, ...mustacheErrors];
+  return [...actionParamsErrors, ...mustacheErrors, ...(queryErrors ? [queryErrors] : [])];
 };
 
 export const validateRuleActionsField =

diff --git a/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts b/x-pack/plugins/triggers_actions_ui/public/application/lib/value_validators.ts
@@ -7,7 +7,23 @@
 
 import { set } from '@kbn/safer-lodash-set';
 import { constant, get } from 'lodash';
-import { UserConfiguredActionConnector, IErrorObject, Rule } from '../../types';
+import { i18n } from '@kbn/i18n';
+import { UserConfiguredActionConnector, IErrorObject, Rule, RuleAction } from '../../types';
+
+const filterQueryRequiredError = i18n.translate(
+  'xpack.triggersActionsUI.sections.actionTypeForm.error.requiredFilterQuery',
+  {
+    defaultMessage: 'A custom query is required.',
+  }
+);
+
+export const validateActionFilterQuery = (actionItem: RuleAction): string | null => {
+  const query = actionItem.alertsFilter?.query;
+  if (query && !query.kql) {
+    return filterQueryRequiredError;
+  }
+  return null;
+};
 
 export function throwIfAbsent<T>(message: string) {
   return (value: T | undefined): T => {

diff --git a/...ins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx b/...ins/triggers_actions_ui/public/application/sections/action_connector_form/action_form.tsx
@@ -86,6 +86,7 @@ export interface ActionAccordionFormProps {
   defaultRuleFrequency?: RuleActionFrequency;
   ruleTypeId?: string;
   hasFieldsForAAD?: boolean;
+  disableErrorMessages?: boolean;
 }
 
 interface ActiveActionConnectorState {
@@ -122,6 +123,7 @@ export const ActionForm = ({
   ruleTypeId,
   producerId,
   hasFieldsForAAD,
+  disableErrorMessages,
 }: ActionAccordionFormProps) => {
   const {
     http,
@@ -499,6 +501,7 @@ export const ActionForm = ({
               producerId={producerId}
               ruleTypeId={ruleTypeId}
               hasFieldsForAAD={hasFieldsForAAD}
+              disableErrorMessages={disableErrorMessages}
             />
           );
         })}

diff --git a/...riggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx b/...riggers_actions_ui/public/application/sections/action_connector_form/action_type_form.tsx
@@ -63,6 +63,7 @@ import { ActionNotifyWhen } from './action_notify_when';
 import { validateParamsForWarnings } from '../../lib/validate_params_for_warnings';
 import { ActionAlertsFilterTimeframe } from './action_alerts_filter_timeframe';
 import { ActionAlertsFilterQuery } from './action_alerts_filter_query';
+import { validateActionFilterQuery } from '../../lib/value_validators';
 
 export type ActionTypeFormProps = {
   actionItem: RuleAction;
@@ -92,6 +93,7 @@ export type ActionTypeFormProps = {
   producerId: string;
   ruleTypeId?: string;
   hasFieldsForAAD?: boolean;
+  disableErrorMessages?: boolean;
 } & Pick<
   ActionAccordionFormProps,
   | 'defaultActionGroupId'
@@ -142,6 +144,7 @@ export const ActionTypeForm = ({
   featureId,
   ruleTypeId,
   hasFieldsForAAD,
+  disableErrorMessages,
 }: ActionTypeFormProps) => {
   const {
     application: { capabilities },
@@ -247,13 +250,28 @@ export const ActionTypeForm = ({
 
   useEffect(() => {
     (async () => {
+      if (disableErrorMessages) {
+        setActionParamsErrors({ errors: {} });
+        return;
+      }
       const res: { errors: IErrorObject } = await actionTypeRegistry
         .get(actionItem.actionTypeId)
         ?.validateParams(actionItem.params);
       setActionParamsErrors(res);
     })();
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [actionItem]);
+  }, [actionItem, disableErrorMessages]);
+
+  const [queryError, setQueryError] = useState<string | null>(null);
+  useEffect(() => {
+    (async () => {
+      if (disableErrorMessages) {
+        setQueryError(null);
+        return;
+      }
+      setQueryError(validateActionFilterQuery(actionItem));
+    })();
+  }, [actionItem, disableErrorMessages]);
 
   const canSave = hasSaveActionsCapability(capabilities);
 
@@ -424,13 +442,15 @@ export const ActionTypeForm = ({
         {showActionAlertsFilter && (
           <>
             {!hideNotifyWhen && <EuiSpacer size="xl" />}
-            <ActionAlertsFilterQuery
-              state={actionItem.alertsFilter?.query}
-              onChange={(query) => setActionAlertsFilterProperty('query', query, index)}
-              featureIds={[producerId as ValidFeatureId]}
-              appName={featureId!}
-              ruleTypeId={ruleTypeId}
-            />
+            <EuiFormRow error={queryError} isInvalid={!!queryError} fullWidth>
+              <ActionAlertsFilterQuery
+                state={actionItem.alertsFilter?.query}
+                onChange={(query) => setActionAlertsFilterProperty('query', query, index)}
+                featureIds={[producerId as ValidFeatureId]}
+                appName={featureId!}
+                ruleTypeId={ruleTypeId}
+              />
+            </EuiFormRow>
             <EuiSpacer size="s" />
             <ActionAlertsFilterTimeframe
               state={actionItem.alertsFilter?.timeframe}

diff --git a/x-pack/plugins/triggers_actions_ui/public/index.ts b/x-pack/plugins/triggers_actions_ui/public/index.ts
@@ -147,3 +147,5 @@ export const getNotifyWhenOptions = async () => {
 };
 
 export { transformRule } from './application/lib/rule_api/common_transformations';
+
+export { validateActionFilterQuery } from './application/lib/value_validators';
Original file line number	Diff line number	Diff line change
Expand Up		@@ -147,3 +147,5 @@ export const getNotifyWhenOptions = async () => {
		};

		export { transformRule } from './application/lib/rule_api/common_transformations';

		export { validateActionFilterQuery } from './application/lib/value_validators';