From b60d3bae09c49a48cdb4d69d1379a476cb051e84 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Tue, 23 Jul 2024 03:53:11 -0500 Subject: [PATCH] [Detection Engine] ML Rule Alert Suppression - Followup (#188267) ## Summary This PR is a followup to #181926. It includes the following changes: - Refactoring some Rule Form logic with `useMemo` - Requested [in this discussion](https://github.com/elastic/kibana/pull/181926#discussion_r1656825268) - Addressed in a5fcf4d0ccd738a40746d099bd295484781825f5 - Adds FTR tests validating ML Suppression supports alert enrichment - Requested [during previous review](https://github.com/elastic/kibana/pull/181926#discussion_r1634616090) - Addressed in d5aa55159070bd716892360f0321505fb44943c0 - Disables ML Suppression fields as a group - Requested in [this comment](https://github.com/elastic/kibana/pull/181926#issuecomment-2203592643) - Addressed by 983945b8da1db8f1309db559ec74e5df7c064862 ### 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 e2150dea5eed3c6984ba2bf39227c32cccea4aa8) --- .../components/ml/hooks/use_ml_rule_config.ts | 17 ++- .../ml/hooks/use_ml_rule_validations.test.ts | 12 +- .../ml/hooks/use_ml_rule_validations.ts | 6 +- .../components/step_define_rule/index.tsx | 119 ++++++++---------- .../machine_learning_alert_suppression.ts | 62 ++++++++- 5 files changed, 130 insertions(+), 86 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_config.ts b/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_config.ts index 86551ad64b43a..af64560e58878 100644 --- a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_config.ts +++ b/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, }; }; diff --git a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.test.ts b/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.test.ts index 6f14d6fe2a736..bb38669b34533 100644 --- a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.test.ts +++ b/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 }) ); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.ts b/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.ts index 81897c5d29b82..0507165e881fe 100644 --- a/x-pack/plugins/security_solution/public/common/components/ml/hooks/use_ml_rule_validations.ts +++ b/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 }; }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx index 48c062848606f..a284e76812255 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/step_define_rule/index.tsx @@ -202,12 +202,11 @@ const StepDefineRuleComponent: FC = ({ watch: ['machineLearningJobId'], }); const { + allJobsStarted, hasMlAdminPermissions, hasMlLicense, - mlFieldsLoading, + loading: mlRuleConfigLoading, mlSuppressionFields, - noMlJobsStarted, - someMlJobsStarted, } = useMLRuleConfig({ machineLearningJobId }); const esqlQueryRef = useRef(undefined); @@ -467,82 +466,68 @@ const StepDefineRuleComponent: FC = ({ 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 = ({ disabledText: suppressionGroupByDisabledText, }} /> - {someMlJobsStarted && ( + {!allJobsStarted && ( {i18n.MACHINE_LEARNING_SUPPRESSION_INCOMPLETE_LABEL} diff --git a/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 b/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 index f4192914e0692..72818eb423b6a 100644 --- a/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 +++ b/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'); + }); + }); }); }); };