From dee5acb17ed643f1cf432b032f4305ba51e5799f Mon Sep 17 00:00:00 2001 From: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> Date: Tue, 19 Dec 2023 10:14:52 +0000 Subject: [PATCH] [Security Solution][Detection Engine] fixes alert suppression define step preview on rule creation (#173145) ## Summary - fixes alert suppression fields that appear on define step preview during rule creation Only `query` and `threshold` rule type support alert suppression, but suppression fields can appear for other rule types when switching between them, while suppression was configured - fixes Define step for `threshold` rule, when suppression duration wasn't showing ### Steps to reproduce 1. Go to create rule form 2. Select query rule type 3. Configure alert suppression for it 4. Switch to any other rule type 5. Complete define step 6. Spot, suppression fields on Define step #### Leaking suppression fields to other rule types https://github.com/elastic/kibana/assets/92328789/a472daf0-bb74-4a6b-841b-3b0097eb4503 #### Missing duration for threshold https://github.com/elastic/kibana/assets/92328789/7df8431d-3ee0-482a-9e06-cb87656f48e0 ### Fixed UI #### Leaking suppression fields to other rule types https://github.com/elastic/kibana/assets/92328789/03057dc6-c007-4b17-8f8b-30e78923f037 #### Missing duration for threshold https://github.com/elastic/kibana/assets/92328789/7f64c79f-7a51-4d43-bd62-b10eefe15c46 ### 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 --------- Co-authored-by: Ryland Herrick --- .../rules/description_step/index.test.tsx | 203 ++++++++++++++++++ .../rules/description_step/index.tsx | 25 ++- .../rule_creation/threshold_rule.cy.ts | 4 + 3 files changed, 231 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.test.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.test.tsx index 5ee4bb4bb1f471..113f81631a06a3 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.test.tsx @@ -14,6 +14,7 @@ import { buildListItems, getDescriptionItem, } from '.'; +import type { Type } from '@kbn/securitysolution-io-ts-alerting-types'; import { FilterManager, UI_SETTINGS } from '@kbn/data-plugin/public'; import type { Filter } from '@kbn/es-query'; @@ -534,5 +535,207 @@ describe('description_step', () => { expect(React.isValidElement(result[0].description)).toBeTruthy(); }); }); + + describe('alert suppression', () => { + const ruleTypesWithoutSuppression: Type[] = [ + 'eql', + 'esql', + 'machine_learning', + 'new_terms', + 'threat_match', + ]; + const suppressionFields = { + groupByDuration: { + unit: 'm', + value: 50, + }, + groupByRadioSelection: 'per-time-period', + enableThresholdSuppression: true, + groupByFields: ['agent.name'], + suppressionMissingFields: 'suppress', + }; + describe('groupByDuration', () => { + ruleTypesWithoutSuppression.forEach((ruleType) => { + test(`should be empty if rule is ${ruleType}`, () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType, + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + }); + + ['query', 'saved_query'].forEach((ruleType) => { + test(`should be empty if groupByFields empty for ${ruleType} rule`, () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType: 'query', + ...suppressionFields, + groupByFields: [], + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + + test(`should return item for ${ruleType} rule`, () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType: 'query', + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result[0].description).toBe('50m'); + }); + }); + + test('should return item for threshold rule', () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType: 'threshold', + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result[0].description).toBe('50m'); + }); + + test('should return item for threshold rule if groupByFields empty', () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType: 'threshold', + ...suppressionFields, + groupByFields: [], + }, + mockFilterManager, + mockLicenseService + ); + + expect(result[0].description).toBe('50m'); + }); + + test('should be empty for threshold rule if suppression not enabled', () => { + const result: ListItems[] = getDescriptionItem( + 'groupByDuration', + 'label', + { + ruleType: 'threshold', + ...suppressionFields, + enableThresholdSuppression: false, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + }); + + describe('groupByFields', () => { + [...ruleTypesWithoutSuppression, 'threshold'].forEach((ruleType) => { + test(`should be empty if rule is ${ruleType}`, () => { + const result: ListItems[] = getDescriptionItem( + 'groupByFields', + 'label', + { + ruleType, + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + }); + ['query', 'saved_query'].forEach((ruleType) => { + test(`should return item for ${ruleType} rule`, () => { + const result: ListItems[] = getDescriptionItem( + 'groupByFields', + 'label', + { + ruleType, + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + expect(mount(result[0].description as React.ReactElement).text()).toBe('agent.name'); + }); + }); + }); + + describe('suppressionMissingFields', () => { + [...ruleTypesWithoutSuppression, 'threshold'].forEach((ruleType) => { + test(`should be empty if rule is ${ruleType}`, () => { + const result: ListItems[] = getDescriptionItem( + 'suppressionMissingFields', + 'label', + { + ruleType, + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + }); + ['query', 'saved_query'].forEach((ruleType) => { + test(`should return item for ${ruleType} rule`, () => { + const result: ListItems[] = getDescriptionItem( + 'suppressionMissingFields', + 'label', + { + ruleType, + ...suppressionFields, + }, + mockFilterManager, + mockLicenseService + ); + expect(result[0].description).toContain('Suppress'); + }); + + test(`should be empty if groupByFields empty for ${ruleType} rule`, () => { + const result: ListItems[] = getDescriptionItem( + 'suppressionMissingFields', + 'label', + { + ruleType: 'query', + ...suppressionFields, + groupByFields: [], + }, + mockFilterManager, + mockLicenseService + ); + + expect(result).toEqual([]); + }); + }); + }); + }); }); }); diff --git a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx index 7dc9ca3bd41f45..883c01430e6442 100644 --- a/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx +++ b/x-pack/plugins/security_solution/public/detections/components/rules/description_step/index.tsx @@ -56,6 +56,7 @@ import { THREAT_QUERY_LABEL } from './translations'; import { filterEmptyThreats } from '../../../../detection_engine/rule_creation_ui/pages/rule_creation/helpers'; import { useLicense } from '../../../../common/hooks/use_license'; import type { LicenseService } from '../../../../../common/license'; +import { isThresholdRule, isQueryRule } from '../../../../../common/detection_engine/utils'; const DescriptionListContainer = styled(EuiDescriptionList)` max-width: 600px; @@ -204,12 +205,29 @@ export const getDescriptionItem = ( } else if (field === 'responseActions') { return []; } else if (field === 'groupByFields') { + const ruleType: Type = get('ruleType', data); + const ruleCanHaveGroupByFields = isQueryRule(ruleType); + if (!ruleCanHaveGroupByFields) { + return []; + } const values: string[] = get(field, data); return buildAlertSuppressionDescription(label, values); } else if (field === 'groupByRadioSelection') { return []; } else if (field === 'groupByDuration') { - if (get('groupByFields', data).length > 0) { + const ruleType: Type = get('ruleType', data); + const ruleCanHaveDuration = isQueryRule(ruleType) || isThresholdRule(ruleType); + if (!ruleCanHaveDuration) { + return []; + } + + // threshold rule has suppression duration without grouping fields, but suppression should be explicitly enabled by user + // query rule have suppression duration only if group by fields selected + const showDuration = isThresholdRule(ruleType) + ? get('enableThresholdSuppression', data) === true + : get('groupByFields', data).length > 0; + + if (showDuration) { const value: Duration = get(field, data); return buildAlertSuppressionWindowDescription( label, @@ -220,6 +238,11 @@ export const getDescriptionItem = ( return []; } } else if (field === 'suppressionMissingFields') { + const ruleType: Type = get('ruleType', data); + const ruleCanHaveSuppressionMissingFields = isQueryRule(ruleType); + if (!ruleCanHaveSuppressionMissingFields) { + return []; + } if (get('groupByFields', data).length > 0) { const value = get(field, data); return buildAlertSuppressionMissingFieldsDescription(label, value); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/threshold_rule.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/threshold_rule.cy.ts index 074670221bedd4..fa925131892e15 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/threshold_rule.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_creation/threshold_rule.cy.ts @@ -163,6 +163,10 @@ describe( enablesAndPopulatesThresholdSuppression(5, 'h'); fillDefineThresholdRuleAndContinue(rule); + // ensures duration displayed on define step in preview mode + cy.get(DEFINITION_DETAILS).within(() => { + getDetails(SUPPRESS_FOR_DETAILS).should('have.text', '5h'); + }); fillAboutRuleMinimumAndContinue(rule); skipScheduleRuleAction();