Skip to content

Commit fe3c864

Browse files
authored
[Security Solution][Detections] Fixes Risk Score and Severity mapping issues (#73233) (#73543)
## Summary Fixes the following issues around Risk Score/Severity mapping: * Severity override option cannot be unselected during rule creation * Risk score override option cannot be unselected during rule creation * Cannot fill Critical Severity override at the first attempt * Cannot create a rule with just a Critical severity override Note: When editing rules there is the possibility of the mapping fields remaining `disabled` as they are locked to the 'isLoading' flag from the gql `useFetchIndexPatterns` call, which can sometimes not return/get stuck as loading. @patrykkopycinski has a draft PR to fix this here: #73199 cc @MadameSheema ##### Severity Mapping Fixes: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/88497829-b653de00-cf7e-11ea-8e14-c351117b4282.gif" /> </p> Now distinguishes between empty string/value <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/88497776-94f2f200-cf7e-11ea-821e-3766b7bed3dc.png" /> </p> ##### Risk Score Mapping Fixes: <p align="center"> <img width="500" src="https://user-images.githubusercontent.com/2946766/88497842-c075dc80-cf7e-11ea-8c41-606b20a6ac1c.gif" /> </p> ### Checklist Delete any items that are not applicable to this PR. - [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/master/packages/kbn-i18n/README.md) - [X] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials * Working with @benskelker on API docs. This PR adds `risk_score` (can be `undefined`) to `risk_score.mapping` for future compatibility with mapping to specific risk score values. - [X] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios
1 parent 7f1ae4c commit fe3c864

File tree

14 files changed

+294
-215
lines changed

14 files changed

+294
-215
lines changed

x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,9 @@ export const risk_score_mapping_value = t.string;
222222
export const risk_score_mapping_item = t.exact(
223223
t.type({
224224
field: risk_score_mapping_field,
225-
operator,
226225
value: risk_score_mapping_value,
226+
operator,
227+
risk_score: riskScoreOrUndefined,
227228
})
228229
);
229230

x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.test.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,7 @@ describe('helpers', () => {
331331
const result: ListItems[] = buildSeverityDescription({
332332
value: 'low',
333333
mapping: [{ field: 'host.name', operator: 'equals', value: 'hello', severity: 'high' }],
334+
isMappingChecked: true,
334335
});
335336

336337
expect(result[0].title).toEqual('Severity');

x-pack/plugins/security_solution/public/detections/components/rules/description_step/helpers.tsx

Lines changed: 64 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { SeverityBadge } from '../severity_badge';
3535
import ListTreeIcon from './assets/list_tree_icon.svg';
3636
import { assertUnreachable } from '../../../../common/lib/helpers';
3737
import { AboutStepRiskScore, AboutStepSeverity } from '../../../pages/detection_engine/rules/types';
38+
import { defaultToEmptyTag } from '../../../../common/components/empty_value';
3839

3940
const NoteDescriptionContainer = styled(EuiFlexItem)`
4041
height: 105px;
@@ -236,63 +237,76 @@ export const buildSeverityDescription = (severity: AboutStepSeverity): ListItems
236237
title: i18nSeverity.DEFAULT_SEVERITY,
237238
description: <SeverityBadge value={severity.value} />,
238239
},
239-
...severity.mapping.map((severityItem, index) => {
240-
return {
241-
title: index === 0 ? i18nSeverity.SEVERITY_MAPPING : '',
242-
description: (
243-
<EuiFlexGroup alignItems="center">
244-
<OverrideColumn>
245-
<EuiToolTip
246-
content={severityItem.field}
247-
data-test-subj={`severityOverrideField${index}`}
248-
>
249-
<>{severityItem.field}</>
250-
</EuiToolTip>
251-
</OverrideColumn>
252-
<EuiToolTip content={severityItem.value} data-test-subj={`severityOverrideValue${index}`}>
253-
<>{severityItem.value}</>
254-
</EuiToolTip>
255-
<EuiFlexItem grow={false}>
256-
<EuiIcon type={'sortRight'} />
257-
</EuiFlexItem>
258-
<EuiFlexItem>
259-
<SeverityBadge
260-
data-test-subj={`severityOverrideSeverity${index}`}
261-
value={severityItem.severity}
262-
/>
263-
</EuiFlexItem>
264-
</EuiFlexGroup>
265-
),
266-
};
267-
}),
240+
...(severity.isMappingChecked
241+
? severity.mapping
242+
.filter((severityItem) => severityItem.field !== '')
243+
.map((severityItem, index) => {
244+
return {
245+
title: index === 0 ? i18nSeverity.SEVERITY_MAPPING : '',
246+
description: (
247+
<EuiFlexGroup alignItems="center">
248+
<OverrideColumn>
249+
<EuiToolTip
250+
content={severityItem.field}
251+
data-test-subj={`severityOverrideField${index}`}
252+
>
253+
<>{`${severityItem.field}:`}</>
254+
</EuiToolTip>
255+
</OverrideColumn>
256+
<OverrideColumn>
257+
<EuiToolTip
258+
content={severityItem.value}
259+
data-test-subj={`severityOverrideValue${index}`}
260+
>
261+
{defaultToEmptyTag(severityItem.value)}
262+
</EuiToolTip>
263+
</OverrideColumn>
264+
<EuiFlexItem grow={false}>
265+
<EuiIcon type={'sortRight'} />
266+
</EuiFlexItem>
267+
<EuiFlexItem>
268+
<SeverityBadge
269+
data-test-subj={`severityOverrideSeverity${index}`}
270+
value={severityItem.severity}
271+
/>
272+
</EuiFlexItem>
273+
</EuiFlexGroup>
274+
),
275+
};
276+
})
277+
: []),
268278
];
269279

270280
export const buildRiskScoreDescription = (riskScore: AboutStepRiskScore): ListItems[] => [
271281
{
272282
title: i18nRiskScore.RISK_SCORE,
273283
description: riskScore.value,
274284
},
275-
...riskScore.mapping.map((riskScoreItem, index) => {
276-
return {
277-
title: index === 0 ? i18nRiskScore.RISK_SCORE_MAPPING : '',
278-
description: (
279-
<EuiFlexGroup alignItems="center">
280-
<EuiFlexItem>
281-
<EuiToolTip
282-
content={riskScoreItem.field}
283-
data-test-subj={`riskScoreOverrideField${index}`}
284-
>
285-
<>{riskScoreItem.field}</>
286-
</EuiToolTip>
287-
</EuiFlexItem>
288-
<EuiFlexItem grow={false}>
289-
<EuiIcon type={'sortRight'} />
290-
</EuiFlexItem>
291-
<EuiFlexItem>{'signal.rule.risk_score'}</EuiFlexItem>
292-
</EuiFlexGroup>
293-
),
294-
};
295-
}),
285+
...(riskScore.isMappingChecked
286+
? riskScore.mapping
287+
.filter((riskScoreItem) => riskScoreItem.field !== '')
288+
.map((riskScoreItem, index) => {
289+
return {
290+
title: index === 0 ? i18nRiskScore.RISK_SCORE_MAPPING : '',
291+
description: (
292+
<EuiFlexGroup alignItems="center">
293+
<OverrideColumn>
294+
<EuiToolTip
295+
content={riskScoreItem.field}
296+
data-test-subj={`riskScoreOverrideField${index}`}
297+
>
298+
<>{riskScoreItem.field}</>
299+
</EuiToolTip>
300+
</OverrideColumn>
301+
<EuiFlexItem grow={false}>
302+
<EuiIcon type={'sortRight'} />
303+
</EuiFlexItem>
304+
<EuiFlexItem>{'signal.rule.risk_score'}</EuiFlexItem>
305+
</EuiFlexGroup>
306+
),
307+
};
308+
})
309+
: []),
296310
];
297311

298312
const MyRefUrlLink = styled(EuiLink)`

x-pack/plugins/security_solution/public/detections/components/rules/risk_score_mapping/index.tsx

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,9 @@ import {
1414
EuiIcon,
1515
EuiSpacer,
1616
} from '@elastic/eui';
17-
import React, { useCallback, useEffect, useMemo, useState } from 'react';
17+
import React, { useCallback, useMemo } from 'react';
1818
import styled from 'styled-components';
19+
import { noop } from 'lodash/fp';
1920
import * as i18n from './translations';
2021
import { FieldHook } from '../../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib';
2122
import { CommonUseField } from '../../../../cases/components/create';
@@ -24,6 +25,10 @@ import { FieldComponent } from '../../../../common/components/autocomplete/field
2425
import { IFieldType } from '../../../../../../../../src/plugins/data/common/index_patterns/fields';
2526
import { IIndexPattern } from '../../../../../../../../src/plugins/data/common/index_patterns';
2627

28+
const RiskScoreMappingEuiFormRow = styled(EuiFormRow)`
29+
width: 468px;
30+
`;
31+
2732
const NestedContent = styled.div`
2833
margin-left: 24px;
2934
`;
@@ -41,6 +46,7 @@ interface RiskScoreFieldProps {
4146
field: FieldHook;
4247
idAria: string;
4348
indices: IIndexPattern;
49+
isDisabled: boolean;
4450
placeholder?: string;
4551
}
4652

@@ -49,40 +55,23 @@ export const RiskScoreField = ({
4955
field,
5056
idAria,
5157
indices,
58+
isDisabled,
5259
placeholder,
5360
}: RiskScoreFieldProps) => {
54-
const [isRiskScoreMappingChecked, setIsRiskScoreMappingChecked] = useState(false);
55-
const [initialFieldCheck, setInitialFieldCheck] = useState(true);
56-
5761
const fieldTypeFilter = useMemo(() => ['number'], []);
5862

59-
useEffect(() => {
60-
if (
61-
!isRiskScoreMappingChecked &&
62-
initialFieldCheck &&
63-
(field.value as AboutStepRiskScore).mapping?.length > 0
64-
) {
65-
setIsRiskScoreMappingChecked(true);
66-
setInitialFieldCheck(false);
67-
}
68-
}, [
69-
field,
70-
initialFieldCheck,
71-
isRiskScoreMappingChecked,
72-
setIsRiskScoreMappingChecked,
73-
setInitialFieldCheck,
74-
]);
75-
7663
const handleFieldChange = useCallback(
7764
([newField]: IFieldType[]): void => {
7865
const values = field.value as AboutStepRiskScore;
7966
field.setValue({
8067
value: values.value,
68+
isMappingChecked: values.isMappingChecked,
8169
mapping: [
8270
{
8371
field: newField?.name ?? '',
8472
operator: 'equals',
85-
value: '',
73+
value: undefined,
74+
riskScore: undefined,
8675
},
8776
],
8877
});
@@ -99,8 +88,13 @@ export const RiskScoreField = ({
9988
}, [field.value, indices]);
10089

10190
const handleRiskScoreMappingChecked = useCallback(() => {
102-
setIsRiskScoreMappingChecked(!isRiskScoreMappingChecked);
103-
}, [isRiskScoreMappingChecked, setIsRiskScoreMappingChecked]);
91+
const values = field.value as AboutStepRiskScore;
92+
field.setValue({
93+
value: values.value,
94+
mapping: [...values.mapping],
95+
isMappingChecked: !values.isMappingChecked,
96+
});
97+
}, [field]);
10498

10599
const riskScoreLabel = useMemo(() => {
106100
return (
@@ -117,11 +111,16 @@ export const RiskScoreField = ({
117111
const riskScoreMappingLabel = useMemo(() => {
118112
return (
119113
<div>
120-
<EuiFlexGroup alignItems="center" gutterSize="s" onClick={handleRiskScoreMappingChecked}>
114+
<EuiFlexGroup
115+
alignItems="center"
116+
gutterSize="s"
117+
onClick={!isDisabled ? handleRiskScoreMappingChecked : noop}
118+
>
121119
<EuiFlexItem grow={false}>
122120
<EuiCheckbox
123121
id={`risk_score-mapping-override`}
124-
checked={isRiskScoreMappingChecked}
122+
checked={(field.value as AboutStepRiskScore).isMappingChecked}
123+
disabled={isDisabled}
125124
onChange={handleRiskScoreMappingChecked}
126125
/>
127126
</EuiFlexItem>
@@ -133,7 +132,7 @@ export const RiskScoreField = ({
133132
</NestedContent>
134133
</div>
135134
);
136-
}, [handleRiskScoreMappingChecked, isRiskScoreMappingChecked]);
135+
}, [field.value, handleRiskScoreMappingChecked, isDisabled]);
137136

138137
return (
139138
<EuiFlexGroup>
@@ -153,6 +152,7 @@ export const RiskScoreField = ({
153152
componentProps={{
154153
idAria: 'detectionEngineStepAboutRuleRiskScore',
155154
'data-test-subj': 'detectionEngineStepAboutRuleRiskScore',
155+
isDisabled,
156156
euiFieldProps: {
157157
max: 100,
158158
min: 0,
@@ -166,11 +166,11 @@ export const RiskScoreField = ({
166166
</EuiFormRow>
167167
</EuiFlexItem>
168168
<EuiFlexItem>
169-
<EuiFormRow
169+
<RiskScoreMappingEuiFormRow
170170
label={riskScoreMappingLabel}
171171
labelAppend={field.labelAppend}
172172
helpText={
173-
isRiskScoreMappingChecked ? (
173+
(field.value as AboutStepRiskScore).isMappingChecked ? (
174174
<NestedContent>{i18n.RISK_SCORE_MAPPING_DETAILS}</NestedContent>
175175
) : (
176176
''
@@ -184,7 +184,7 @@ export const RiskScoreField = ({
184184
>
185185
<NestedContent>
186186
<EuiSpacer size="s" />
187-
{isRiskScoreMappingChecked && (
187+
{(field.value as AboutStepRiskScore).isMappingChecked && (
188188
<EuiFlexGroup direction={'column'} gutterSize="s">
189189
<EuiFlexItem>
190190
<EuiFlexGroup alignItems="center" gutterSize="s">
@@ -208,11 +208,11 @@ export const RiskScoreField = ({
208208
fieldTypeFilter={fieldTypeFilter}
209209
isLoading={false}
210210
isClearable={false}
211-
isDisabled={false}
211+
isDisabled={isDisabled}
212212
onChange={handleFieldChange}
213213
data-test-subj={dataTestSubj}
214214
aria-label={idAria}
215-
fieldInputWidth={230}
215+
fieldInputWidth={270}
216216
/>
217217
</EuiFlexItem>
218218
<EuiFlexItemIconColumn grow={false}>
@@ -226,7 +226,7 @@ export const RiskScoreField = ({
226226
</EuiFlexGroup>
227227
)}
228228
</NestedContent>
229-
</EuiFormRow>
229+
</RiskScoreMappingEuiFormRow>
230230
</EuiFlexItem>
231231
</EuiFlexGroup>
232232
);

0 commit comments

Comments
 (0)