Skip to content

Commit

Permalink
[Security Solution] Display cardinality for threshold rules (#201162)
Browse files Browse the repository at this point in the history
**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
<img width="1027" alt="image"
src="https://github.com/user-attachments/assets/b927b4e0-f2a0-41ba-87e0-441a53760cce">

2. rule details page
<img width="762" alt="image"
src="https://github.com/user-attachments/assets/486f8616-8582-45ea-9422-bfd554e2ae83">



### AFTER
1. overview page
<img width="1015" alt="image"
src="https://github.com/user-attachments/assets/06a5e0d1-76ef-434e-9c1c-cce6c3ff504f">

2. rule details page
<img width="893" alt="image"
src="https://github.com/user-attachments/assets/40acd7d4-4058-40c0-aa19-e5f489c53c2c">


### 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
  • Loading branch information
jkelas authored Nov 27, 2024
1 parent 2ec351d commit 19a2ff8
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,15 +185,23 @@ interface ThresholdProps {
threshold: ThresholdType;
}

export const Threshold = ({ threshold }: ThresholdProps) => (
<div data-test-subj="thresholdPropertyValue">
{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}`}
</div>
);
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 <div data-test-subj="thresholdPropertyValue">{thresholdDescription}</div>;
};

interface AnomalyThresholdProps {
anomalyThreshold: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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[];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) => {
Expand Down

0 comments on commit 19a2ff8

Please sign in to comment.