From 7841c6e6bf58eaf42461c4b794397ec214b65d50 Mon Sep 17 00:00:00 2001 From: Jacek Kolezynski Date: Wed, 27 Nov 2024 13:11:41 +0100 Subject: [PATCH] [Security Solution] Display cardinality for threshold rules (#201162) **Resolves #161576** ## Summary This PR fixes the description of threshold rules. The problem was that if a threshold rule contained 'Count' (cardinality) it wasn't displayed neither in a summary while creating the rule, nor in the rule details page. This PR fixes these two places, introducing similar logic to the two places in the code, to display the cardinality if it is present in the threshold object. ### BEFORE 1. overview page image 2. rule details page image ### AFTER 1. overview page image 2. rule details page image ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed Done: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7474 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7473 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7476 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7477 (cherry picked from commit 19a2ff81d5a542402a3f0c006d6b4986890d73f9) --- .../components/description_step/helpers.tsx | 39 ++++++++------ .../description_step/index.test.tsx | 54 ++++++++++++++++++- .../description_step/translations.ts | 18 +++++++ .../rule_details/rule_definition_section.tsx | 26 +++++---- .../prebuilt_rules_preview.cy.ts | 4 +- .../cypress/tasks/prebuilt_rules_preview.ts | 15 ++++-- 6 files changed, 126 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/helpers.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/helpers.tsx index 57292d91953d8..476302ab06f5b 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/helpers.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/helpers.tsx @@ -27,7 +27,6 @@ import { FilterBadgeGroup } from '@kbn/unified-search-plugin/public'; import { IntervalAbbrScreenReader } from '../../../../common/components/accessibility'; import type { RequiredFieldArray, - Threshold, AlertSuppressionMissingFieldsStrategy, } from '../../../../../common/api/detection_engine/model/rule_schema'; import { AlertSuppressionMissingFieldsStrategyEnum } from '../../../../../common/api/detection_engine/model/rule_schema'; @@ -50,6 +49,7 @@ import { defaultToEmptyTag } from '../../../../common/components/empty_value'; import { RequiredFieldIcon } from '../../../rule_management/components/rule_details/required_field_icon'; import { ThreatEuiFlexGroup } from './threat_description'; import { AlertSuppressionLabel } from './alert_suppression_label'; +import type { FieldValueThreshold } from '../threshold_input'; const NoteDescriptionContainer = styled(EuiFlexItem)` height: 105px; @@ -490,20 +490,29 @@ export const buildRuleTypeDescription = (label: string, ruleType: Type): ListIte } }; -export const buildThresholdDescription = (label: string, threshold: Threshold): ListItems[] => [ - { - title: label, - description: ( - <> - {isEmpty(threshold.field[0]) - ? `${i18n.THRESHOLD_RESULTS_ALL} >= ${threshold.value}` - : `${i18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${ - Array.isArray(threshold.field) ? threshold.field.join(',') : threshold.field - } >= ${threshold.value}`} - - ), - }, -]; +export const buildThresholdDescription = ( + label: string, + threshold: FieldValueThreshold +): ListItems[] => { + let thresholdDescription = isEmpty(threshold.field[0]) + ? `${i18n.THRESHOLD_RESULTS_ALL} >= ${threshold.value}` + : `${i18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${threshold.field.join(',')} >= ${threshold.value}`; + + if (threshold.cardinality?.value && threshold.cardinality?.field.length > 0) { + thresholdDescription = i18n.THRESHOLD_CARDINALITY( + thresholdDescription, + threshold.cardinality.field[0], + threshold.cardinality.value + ); + } + + return [ + { + title: label, + description: <>{thresholdDescription}, + }, + ]; +}; export const buildThreatMappingDescription = ( title: string, diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/index.test.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/index.test.tsx index de46d09065f4e..b45f1a50414ac 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/index.test.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/index.test.tsx @@ -451,10 +451,62 @@ describe('description_step', () => { expect(result[0].title).toEqual('Threshold label'); expect(React.isValidElement(result[0].description)).toBeTruthy(); - expect(mount(result[0].description as React.ReactElement).html()).toContain( + expect(mount(result[0].description as React.ReactElement).html()).toEqual( + 'Results aggregated by user.name >= 100' + ); + }); + + test('returns threshold description when threshold exist, field is set, and cardinality is not set', () => { + const mockThreshold = { + threshold: { + field: ['user.name'], + value: 100, + cardinality: { + field: [], + value: 0, + }, + }, + }; + const result: ListItems[] = getDescriptionItem( + 'threshold', + 'Threshold label', + mockThreshold, + mockFilterManager, + mockLicenseService + ); + + expect(result[0].title).toEqual('Threshold label'); + expect(React.isValidElement(result[0].description)).toBeTruthy(); + expect(mount(result[0].description as React.ReactElement).html()).toEqual( 'Results aggregated by user.name >= 100' ); }); + + test('returns threshold description when threshold exist, field is set and cardinality is set', () => { + const mockThreshold = { + threshold: { + field: ['user.name'], + value: 100, + cardinality: { + field: ['host.test_value'], + value: 10, + }, + }, + }; + const result: ListItems[] = getDescriptionItem( + 'threshold', + 'Threshold label', + mockThreshold, + mockFilterManager, + mockLicenseService + ); + + expect(result[0].title).toEqual('Threshold label'); + expect(React.isValidElement(result[0].description)).toBeTruthy(); + expect(mount(result[0].description as React.ReactElement).html()).toContain( + 'Results aggregated by user.name >= 100 when unique values count of host.test_value >= 10' + ); + }); }); describe('references', () => { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/translations.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/translations.ts index 5c43b9181adcb..74a9faa4efd4c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/translations.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/description_step/translations.ts @@ -126,6 +126,24 @@ export const THRESHOLD_RESULTS_AGGREGATED_BY = i18n.translate( } ); +export const THRESHOLD_CARDINALITY = ( + thresholdFieldsGroupedBy: string, + cardinalityField: string, + cardinalityValue: string | number +) => + i18n.translate( + 'xpack.securitySolution.detectionEngine.ruleDescription.thresholdResultsCardinalityDescription', + { + defaultMessage: + '{thresholdFieldsGroupedBy} when unique values count of {cardinalityField} >= {cardinalityValue}', + values: { + thresholdFieldsGroupedBy, + cardinalityField, + cardinalityValue, + }, + } + ); + export const EQL_EVENT_CATEGORY_FIELD_LABEL = i18n.translate( 'xpack.securitySolution.detectionEngine.ruleDescription.eqlEventCategoryFieldLabel', { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx index 295323017f06a..70f267ac94ba4 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/components/rule_details/rule_definition_section.tsx @@ -185,15 +185,23 @@ interface ThresholdProps { threshold: ThresholdType; } -export const Threshold = ({ threshold }: ThresholdProps) => ( -
- {isEmpty(threshold.field[0]) - ? `${descriptionStepI18n.THRESHOLD_RESULTS_ALL} >= ${threshold.value}` - : `${descriptionStepI18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${ - Array.isArray(threshold.field) ? threshold.field.join(',') : threshold.field - } >= ${threshold.value}`} -
-); +export const Threshold = ({ threshold }: ThresholdProps) => { + let thresholdDescription = isEmpty(threshold.field[0]) + ? `${descriptionStepI18n.THRESHOLD_RESULTS_ALL} >= ${threshold.value}` + : `${descriptionStepI18n.THRESHOLD_RESULTS_AGGREGATED_BY} ${ + Array.isArray(threshold.field) ? threshold.field.join(',') : threshold.field + } >= ${threshold.value}`; + + if (threshold.cardinality && threshold.cardinality.length > 0) { + thresholdDescription = descriptionStepI18n.THRESHOLD_CARDINALITY( + thresholdDescription, + threshold.cardinality[0].field, + threshold.cardinality[0].value + ); + } + + return
{thresholdDescription}
; +}; interface AnomalyThresholdProps { anomalyThreshold: number; diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts index 86291d4eb9c7f..72bb79302537e 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/rule_management/prebuilt_rules/prebuilt_rules_preview.cy.ts @@ -544,7 +544,7 @@ describe( const { threshold } = THRESHOLD_RULE_INDEX_PATTERN['security-rule'] as { threshold: Threshold; }; - assertThresholdPropertyShown(threshold.value); + assertThresholdPropertyShown(threshold); const { index } = THRESHOLD_RULE_INDEX_PATTERN['security-rule'] as { index: string[] }; assertIndexPropertyShown(index); @@ -952,7 +952,7 @@ describe( const { threshold } = UPDATED_THRESHOLD_RULE_INDEX_PATTERN['security-rule'] as { threshold: Threshold; }; - assertThresholdPropertyShown(threshold.value); + assertThresholdPropertyShown(threshold); const { index } = UPDATED_THRESHOLD_RULE_INDEX_PATTERN['security-rule'] as { index: string[]; diff --git a/x-pack/test/security_solution_cypress/cypress/tasks/prebuilt_rules_preview.ts b/x-pack/test/security_solution_cypress/cypress/tasks/prebuilt_rules_preview.ts index 6617b10ccc219..c67ab29831075 100644 --- a/x-pack/test/security_solution_cypress/cypress/tasks/prebuilt_rules_preview.ts +++ b/x-pack/test/security_solution_cypress/cypress/tasks/prebuilt_rules_preview.ts @@ -8,7 +8,10 @@ import { capitalize } from 'lodash'; import type { ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types'; import type { Module } from '@kbn/ml-plugin/common/types/modules'; -import { AlertSuppression } from '@kbn/security-solution-plugin/common/api/detection_engine/model/rule_schema'; +import { + AlertSuppression, + Threshold, +} from '@kbn/security-solution-plugin/common/api/detection_engine/model/rule_schema'; import type { Filter } from '@kbn/es-query'; import type { PrebuiltRuleAsset } from '@kbn/security-solution-plugin/server/lib/detection_engine/prebuilt_rules'; import { @@ -312,9 +315,15 @@ export const assertMachineLearningPropertiesShown = ( }); }; -export const assertThresholdPropertyShown = (thresholdValue: number) => { +export const assertThresholdPropertyShown = (threshold: Threshold) => { cy.get(THRESHOLD_TITLE).should('have.text', 'Threshold'); - cy.get(THRESHOLD_VALUE).should('contain', thresholdValue); + cy.get(THRESHOLD_VALUE).should('contain', threshold.value); + if (threshold.cardinality) { + cy.get(THRESHOLD_VALUE).should( + 'contain', + `when unique values count of ${threshold.cardinality[0].field} >= ${threshold.cardinality[0].value}` + ); + } }; export const assertEqlQueryPropertyShown = (query: string) => {