[Detection Engine] ML Rule Alert Suppression - Followup (elastic#188267)

## Summary

This PR is a followup to elastic#181926. It includes the following changes:

- Refactoring some Rule Form logic with `useMemo`
- Requested [in this
discussion](elastic#181926 (comment))
  - Addressed in a5fcf4d
- Adds FTR tests validating ML Suppression supports alert enrichment
- Requested [during previous
review](elastic#181926 (comment))
  - Addressed in d5aa551
- Disables ML Suppression fields as a group
- Requested in [this
comment](elastic#181926 (comment))
  - Addressed by 983945b

### Checklist

- [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
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))

(cherry picked from commit e2150de)

x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_config.ts

@@ -19,11 +19,10 @@ import { hasMlLicense } from '../../../../../common/machine_learning/has_ml_lice
 export interface UseMlRuleConfigReturn {
   hasMlAdminPermissions: boolean;
   hasMlLicense: boolean;
+  loading: boolean;
   mlFields: DataViewFieldBase[];
-  mlFieldsLoading: boolean;
   mlSuppressionFields: BrowserField[];
-  noMlJobsStarted: boolean;
-  someMlJobsStarted: boolean;
+  allJobsStarted: boolean;
 }
 
 /**
@@ -37,11 +36,12 @@ export interface UseMlRuleConfigReturn {
 export const useMLRuleConfig = ({
   machineLearningJobId,
 }: {
-  machineLearningJobId: string[];
+  machineLearningJobId: string[] | undefined;
 }): UseMlRuleConfigReturn => {
   const mlCapabilities = useMlCapabilities();
-  const { someJobsStarted: someMlJobsStarted, noJobsStarted: noMlJobsStarted } =
-    useMlRuleValidations({ machineLearningJobId });
+  const { loading: validationsLoading, allJobsStarted } = useMlRuleValidations({
+    machineLearningJobId,
+  });
   const { loading: mlFieldsLoading, fields: mlFields } = useRuleFields({
     machineLearningJobId,
   });
@@ -54,9 +54,8 @@ export const useMLRuleConfig = ({
     hasMlAdminPermissions: hasMlAdminPermissions(mlCapabilities),
     hasMlLicense: hasMlLicense(mlCapabilities),
     mlFields,
-    mlFieldsLoading,
+    loading: validationsLoading || mlFieldsLoading,
     mlSuppressionFields,
-    noMlJobsStarted,
-    someMlJobsStarted,
+    allJobsStarted,
   };
 };

x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.test.ts

@@ -40,17 +40,17 @@ describe('useMlRuleValidations', () => {
     expect(result.current).toEqual(expect.objectContaining({ loading: false }));
   });
 
-  it('returns no jobs started when no jobs are started', () => {
+  it('returns "no jobs started" when no jobs are started', () => {
     const { result } = renderHook(() => useMlRuleValidations({ machineLearningJobId }), {
       wrapper: TestProviders,
     });
 
     expect(result.current).toEqual(
-      expect.objectContaining({ noJobsStarted: true, someJobsStarted: false })
+      expect.objectContaining({ noJobsStarted: true, allJobsStarted: false })
     );
   });
 
-  it('returns some jobs started when some jobs are started', () => {
+  it('returns a unique state when only some jobs are started', () => {
     (useInstalledSecurityJobs as jest.Mock).mockReturnValueOnce({
       loading: false,
       jobs: getJobsSummaryResponseMock([
@@ -70,11 +70,11 @@ describe('useMlRuleValidations', () => {
     });
 
     expect(result.current).toEqual(
-      expect.objectContaining({ noJobsStarted: false, someJobsStarted: true })
+      expect.objectContaining({ noJobsStarted: false, allJobsStarted: false })
     );
   });
 
-  it('returns neither "no jobs started" nor "some jobs started" when all jobs are started', () => {
+  it('returns a unique state when all jobs are started', () => {
     (useInstalledSecurityJobs as jest.Mock).mockReturnValueOnce({
       loading: false,
       jobs: getJobsSummaryResponseMock([
@@ -96,7 +96,7 @@ describe('useMlRuleValidations', () => {
     });
 
     expect(result.current).toEqual(
-      expect.objectContaining({ noJobsStarted: false, someJobsStarted: false })
+      expect.objectContaining({ noJobsStarted: false, allJobsStarted: true })
     );
   });
 });

x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.ts

@@ -15,7 +15,7 @@ export interface UseMlRuleValidationsParams {
 export interface UseMlRuleValidationsReturn {
   loading: boolean;
   noJobsStarted: boolean;
-  someJobsStarted: boolean;
+  allJobsStarted: boolean;
 }
 
 /**
@@ -35,7 +35,7 @@ export const useMlRuleValidations = ({
     isJobStarted(job.jobState, job.datafeedState)
   ).length;
   const noMlJobsStarted = numberOfRuleMlJobsStarted === 0;
-  const someMlJobsStarted = !noMlJobsStarted && numberOfRuleMlJobsStarted !== ruleMlJobs.length;
+  const allMlJobsStarted = !noMlJobsStarted && numberOfRuleMlJobsStarted === ruleMlJobs.length;
 
-  return { loading, noJobsStarted: noMlJobsStarted, someJobsStarted: someMlJobsStarted };
+  return { loading, noJobsStarted: noMlJobsStarted, allJobsStarted: allMlJobsStarted };
 };

x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx

@@ -202,12 +202,11 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
     watch: ['machineLearningJobId'],
   });
   const {
+    allJobsStarted,
     hasMlAdminPermissions,
     hasMlLicense,
-    mlFieldsLoading,
+    loading: mlRuleConfigLoading,
     mlSuppressionFields,
-    noMlJobsStarted,
-    someMlJobsStarted,
   } = useMLRuleConfig({ machineLearningJobId });
 
   const esqlQueryRef = useRef<DefineStepRule['queryBar'] | undefined>(undefined);
@@ -467,82 +466,68 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
       indexPatternsFields: indexPattern.fields,
     });
 
+  /** Suppression fields being selected is a special case for our form logic, as we can't
+   * disable these fields and leave users in a bad state that they cannot change.
+   * The exception is threshold rules, which use an existing threshold field for the same
+   * purpose and so are treated as if the field is always selected.  */
+  const areSuppressionFieldsSelected = isThresholdRule || groupByFields.length > 0;
+
   const areSuppressionFieldsDisabledBySequence =
     isEqlRule(ruleType) &&
     isEqlSequenceQuery(queryBar?.query?.query as string) &&
     groupByFields.length === 0;
 
-  const isSuppressionGroupByDisabled =
+  /** If we don't have ML field information, users can't meaningfully interact with suppression fields */
+  const areSuppressionFieldsDisabledByMlFields =
+    isMlRule(ruleType) && (mlRuleConfigLoading || !mlSuppressionFields.length);
+
+  const isThresholdSuppressionDisabled = isThresholdRule && !enableThresholdSuppression;
+
+  /** Suppression fields are generally disabled if either:
+   * - License is insufficient (i.e. less than platinum)
+   * - An EQL Sequence is used
+   * - ML Field information is not available
+   */
+  const areSuppressionFieldsDisabled =
     !isAlertSuppressionLicenseValid ||
     areSuppressionFieldsDisabledBySequence ||
-    isEsqlSuppressionLoading ||
-    (isMlRule(ruleType) && (noMlJobsStarted || mlFieldsLoading || !mlSuppressionFields.length));
+    areSuppressionFieldsDisabledByMlFields;
 
-  const suppressionGroupByDisabledText = areSuppressionFieldsDisabledBySequence
-    ? i18n.EQL_SEQUENCE_SUPPRESSION_DISABLE_TOOLTIP
-    : isMlRule(ruleType) && noMlJobsStarted
-    ? i18n.MACHINE_LEARNING_SUPPRESSION_DISABLED_LABEL
-    : alertSuppressionUpsellingMessage;
+  const isSuppressionGroupByDisabled =
+    (areSuppressionFieldsDisabled || isEsqlSuppressionLoading) && !areSuppressionFieldsSelected;
+
+  const suppressionGroupByDisabledText = useMemo(() => {
+    if (areSuppressionFieldsDisabledBySequence) {
+      return i18n.EQL_SEQUENCE_SUPPRESSION_DISABLE_TOOLTIP;
+    } else if (areSuppressionFieldsDisabledByMlFields) {
+      return i18n.MACHINE_LEARNING_SUPPRESSION_DISABLED_LABEL;
+    } else {
+      return alertSuppressionUpsellingMessage;
+    }
+  }, [
+    alertSuppressionUpsellingMessage,
+    areSuppressionFieldsDisabledByMlFields,
+    areSuppressionFieldsDisabledBySequence,
+  ]);
 
-  const suppressionGroupByFields = isEsqlRule(ruleType)
-    ? esqlSuppressionFields
-    : isMlRule(ruleType)
-    ? mlSuppressionFields
-    : termsAggregationFields;
+  const suppressionGroupByFields = useMemo(() => {
+    if (isEsqlRule(ruleType)) {
+      return esqlSuppressionFields;
+    } else if (isMlRule(ruleType)) {
+      return mlSuppressionFields;
+    } else {
+      return termsAggregationFields;
+    }
+  }, [esqlSuppressionFields, mlSuppressionFields, ruleType, termsAggregationFields]);
 
-  /**
-   * Component that allows selection of suppression intervals disabled:
-   *  - if suppression license is not valid(i.e. less than platinum)
-   *  - or for not threshold rule - when groupBy fields not selected
-   * - Eql sequence is used
-   */
   const isGroupByChildrenDisabled =
-    areSuppressionFieldsDisabledBySequence || !isAlertSuppressionLicenseValid || isThresholdRule
-      ? false
-      : !groupByFields?.length;
-
-  /**
-   * Per rule execution radio option is disabled
-   *  - if suppression license is not valid(i.e. less than platinum)
-   *  - always disabled for threshold rule
-   *  - Eql sequence is used and suppression fields are in the default state
-   */
-  const isPerRuleExecutionDisabled =
-    areSuppressionFieldsDisabledBySequence || !isAlertSuppressionLicenseValid || isThresholdRule;
-
-  /**
-   * Per time period execution radio option is disabled
-   *  - if suppression license is not valid(i.e. less than platinum)
-   *  - disabled for threshold rule when enabled suppression is not checked
-   * - Eql sequence is used and suppression fields are in the default state
-   */
+    areSuppressionFieldsDisabled || isThresholdSuppressionDisabled || !areSuppressionFieldsSelected;
+  const isPerRuleExecutionDisabled = areSuppressionFieldsDisabled || isThresholdRule;
   const isPerTimePeriodDisabled =
-    areSuppressionFieldsDisabledBySequence ||
-    !isAlertSuppressionLicenseValid ||
-    (isThresholdRule && !enableThresholdSuppression);
-
-  /**
-   * Suppression duration is disabled when
-   *  - if suppression license is not valid(i.e. less than platinum)
-   *  - when suppression by rule execution is selected in radio button
-   *  - when threshold suppression is not enabled and no group by fields selected
-   * -  Eql sequence is used and suppression fields are in the default state
-   * */
+    areSuppressionFieldsDisabled || isThresholdSuppressionDisabled || !areSuppressionFieldsSelected;
   const isDurationDisabled =
-    areSuppressionFieldsDisabledBySequence ||
-    !isAlertSuppressionLicenseValid ||
-    (!enableThresholdSuppression && groupByFields?.length === 0);
-
-  /**
-   * Suppression missing fields is disabled when
-   *  - if suppression license is not valid(i.e. less than platinum)
-   *  - when no group by fields selected
-   * -  Eql sequence is used and suppression fields are in the default state
-   * */
-  const isMissingFieldsDisabled =
-    areSuppressionFieldsDisabledBySequence ||
-    !isAlertSuppressionLicenseValid ||
-    !groupByFields.length;
+    areSuppressionFieldsDisabled || isThresholdSuppressionDisabled || !areSuppressionFieldsSelected;
+  const isMissingFieldsDisabled = areSuppressionFieldsDisabled || !areSuppressionFieldsSelected;
 
   const GroupByChildren = useCallback(
     ({ groupByRadioSelection, groupByDurationUnit, groupByDurationValue }) => (
@@ -1102,7 +1087,7 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
                     disabledText: suppressionGroupByDisabledText,
                   }}
                 />
-                {someMlJobsStarted && (
+                {!allJobsStarted && (
                   <EuiText size="xs" color="warning">
                     {i18n.MACHINE_LEARNING_SUPPRESSION_INCOMPLETE_LABEL}
                   </EuiText>

x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/machine_learning_alert_suppression.ts

@@ -22,7 +22,10 @@ import {
   TIMESTAMP,
 } from '@kbn/rule-data-utils';
 import { ALERT_ORIGINAL_TIME } from '@kbn/security-solution-plugin/common/field_maps/field_names';
-import { DETECTION_ENGINE_SIGNALS_STATUS_URL as DETECTION_ENGINE_ALERTS_STATUS_URL } from '@kbn/security-solution-plugin/common/constants';
+import {
+  DETECTION_ENGINE_SIGNALS_STATUS_URL as DETECTION_ENGINE_ALERTS_STATUS_URL,
+  ENABLE_ASSET_CRITICALITY_SETTING,
+} from '@kbn/security-solution-plugin/common/constants';
 import { EsArchivePathBuilder } from '../../../../../../es_archive_path_builder';
 import { FtrProviderContext } from '../../../../../../ftr_provider_context';
 import {
@@ -1099,6 +1102,63 @@ export default ({ getService }: FtrProviderContext) => {
           });
         });
       });
+
+      describe('with enrichments', () => {
+        const kibanaServer = getService('kibanaServer');
+
+        before(async () => {
+          await esArchiver.load('x-pack/test/functional/es_archives/entity/risks');
+          await esArchiver.load('x-pack/test/functional/es_archives/asset_criticality');
+          await kibanaServer.uiSettings.update({
+            [ENABLE_ASSET_CRITICALITY_SETTING]: true,
+          });
+        });
+
+        after(async () => {
+          await esArchiver.unload('x-pack/test/functional/es_archives/entity/risks');
+          await esArchiver.unload('x-pack/test/functional/es_archives/asset_criticality');
+        });
+
+        beforeEach(async () => {
+          const timestamp = new Date().toISOString();
+          const anomalyWithKnownEntities = {
+            ...baseAnomaly,
+            timestamp,
+            user: { name: 'root' },
+            host: { name: 'zeek-newyork-sha-aa8df15' },
+          };
+          await indexListOfDocuments([anomalyWithKnownEntities]);
+
+          ruleProps = {
+            ...baseRuleProps,
+            from: timestamp,
+            alert_suppression: {
+              group_by: ['host.name'],
+              missing_fields_strategy: 'suppress',
+            },
+          };
+        });
+
+        it('should be enriched with host risk score', async () => {
+          const { previewId } = await previewRule({ supertest, rule: ruleProps });
+          const previewAlerts = await getPreviewAlerts({ es, previewId });
+          expect(previewAlerts).toHaveLength(1);
+          const alertSource = previewAlerts[0]._source;
+
+          expect(alertSource?.host?.risk?.calculated_level).toBe('Low');
+          expect(alertSource?.host?.risk?.calculated_score_norm).toBe(23);
+        });
+
+        it('should be enriched alert with criticality_level', async () => {
+          const { previewId } = await previewRule({ supertest, rule: ruleProps });
+          const previewAlerts = await getPreviewAlerts({ es, previewId });
+          expect(previewAlerts).toHaveLength(1);
+          const fullAlert = previewAlerts[0]._source;
+
+          expect(fullAlert?.['host.asset.criticality']).toBe('medium_impact');
+          expect(fullAlert?.['user.asset.criticality']).toBe('extreme_impact');
+        });
+      });
     });
   });
 };