[Security Solution][Detections] Fixes Risk Score and Severity mapping…

… issues (elastic#73233) ## 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: elastic#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
spong · Jul 28, 2020 · 7c6afea · 7c6afea
1 parent 9957fe1
commit 7c6afea
Show file tree

Hide file tree

Showing 14 changed files with 294 additions and 215 deletions.
diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/common/schemas.ts
@@ -222,8 +222,9 @@ export const risk_score_mapping_value = t.string;
 export const risk_score_mapping_item = t.exact(
   t.type({
     field: risk_score_mapping_field,
-    operator,
     value: risk_score_mapping_value,
+    operator,
+    risk_score: riskScoreOrUndefined,
   })
 );
 

diff --git a/...ns/security_solution/public/detections/components/rules/description_step/helpers.test.tsx b/...ns/security_solution/public/detections/components/rules/description_step/helpers.test.tsx
@@ -331,6 +331,7 @@ describe('helpers', () => {
       const result: ListItems[] = buildSeverityDescription({
         value: 'low',
         mapping: [{ field: 'host.name', operator: 'equals', value: 'hello', severity: 'high' }],
+        isMappingChecked: true,
       });
 
       expect(result[0].title).toEqual('Severity');

diff --git a/...plugins/security_solution/public/detections/components/rules/description_step/helpers.tsx b/...plugins/security_solution/public/detections/components/rules/description_step/helpers.tsx
@@ -35,6 +35,7 @@ import { SeverityBadge } from '../severity_badge';
 import ListTreeIcon from './assets/list_tree_icon.svg';
 import { assertUnreachable } from '../../../../common/lib/helpers';
 import { AboutStepRiskScore, AboutStepSeverity } from '../../../pages/detection_engine/rules/types';
+import { defaultToEmptyTag } from '../../../../common/components/empty_value';
 
 const NoteDescriptionContainer = styled(EuiFlexItem)`
   height: 105px;
@@ -236,63 +237,76 @@ export const buildSeverityDescription = (severity: AboutStepSeverity): ListItems
     title: i18nSeverity.DEFAULT_SEVERITY,
     description: <SeverityBadge value={severity.value} />,
   },
-  ...severity.mapping.map((severityItem, index) => {
-    return {
-      title: index === 0 ? i18nSeverity.SEVERITY_MAPPING : '',
-      description: (
-        <EuiFlexGroup alignItems="center">
-          <OverrideColumn>
-            <EuiToolTip
-              content={severityItem.field}
-              data-test-subj={`severityOverrideField${index}`}
-            >
-              <>{severityItem.field}</>
-            </EuiToolTip>
-          </OverrideColumn>
-          <EuiToolTip content={severityItem.value} data-test-subj={`severityOverrideValue${index}`}>
-            <>{severityItem.value}</>
-          </EuiToolTip>
-          <EuiFlexItem grow={false}>
-            <EuiIcon type={'sortRight'} />
-          </EuiFlexItem>
-          <EuiFlexItem>
-            <SeverityBadge
-              data-test-subj={`severityOverrideSeverity${index}`}
-              value={severityItem.severity}
-            />
-          </EuiFlexItem>
-        </EuiFlexGroup>
-      ),
-    };
-  }),
+  ...(severity.isMappingChecked
+    ? severity.mapping
+        .filter((severityItem) => severityItem.field !== '')
+        .map((severityItem, index) => {
+          return {
+            title: index === 0 ? i18nSeverity.SEVERITY_MAPPING : '',
+            description: (
+              <EuiFlexGroup alignItems="center">
+                <OverrideColumn>
+                  <EuiToolTip
+                    content={severityItem.field}
+                    data-test-subj={`severityOverrideField${index}`}
+                  >
+                    <>{`${severityItem.field}:`}</>
+                  </EuiToolTip>
+                </OverrideColumn>
+                <OverrideColumn>
+                  <EuiToolTip
+                    content={severityItem.value}
+                    data-test-subj={`severityOverrideValue${index}`}
+                  >
+                    {defaultToEmptyTag(severityItem.value)}
+                  </EuiToolTip>
+                </OverrideColumn>
+                <EuiFlexItem grow={false}>
+                  <EuiIcon type={'sortRight'} />
+                </EuiFlexItem>
+                <EuiFlexItem>
+                  <SeverityBadge
+                    data-test-subj={`severityOverrideSeverity${index}`}
+                    value={severityItem.severity}
+                  />
+                </EuiFlexItem>
+              </EuiFlexGroup>
+            ),
+          };
+        })
+    : []),
 ];
 
 export const buildRiskScoreDescription = (riskScore: AboutStepRiskScore): ListItems[] => [
   {
     title: i18nRiskScore.RISK_SCORE,
     description: riskScore.value,
   },
-  ...riskScore.mapping.map((riskScoreItem, index) => {
-    return {
-      title: index === 0 ? i18nRiskScore.RISK_SCORE_MAPPING : '',
-      description: (
-        <EuiFlexGroup alignItems="center">
-          <EuiFlexItem>
-            <EuiToolTip
-              content={riskScoreItem.field}
-              data-test-subj={`riskScoreOverrideField${index}`}
-            >
-              <>{riskScoreItem.field}</>
-            </EuiToolTip>
-          </EuiFlexItem>
-          <EuiFlexItem grow={false}>
-            <EuiIcon type={'sortRight'} />
-          </EuiFlexItem>
-          <EuiFlexItem>{'signal.rule.risk_score'}</EuiFlexItem>
-        </EuiFlexGroup>
-      ),
-    };
-  }),
+  ...(riskScore.isMappingChecked
+    ? riskScore.mapping
+        .filter((riskScoreItem) => riskScoreItem.field !== '')
+        .map((riskScoreItem, index) => {
+          return {
+            title: index === 0 ? i18nRiskScore.RISK_SCORE_MAPPING : '',
+            description: (
+              <EuiFlexGroup alignItems="center">
+                <OverrideColumn>
+                  <EuiToolTip
+                    content={riskScoreItem.field}
+                    data-test-subj={`riskScoreOverrideField${index}`}
+                  >
+                    <>{riskScoreItem.field}</>
+                  </EuiToolTip>
+                </OverrideColumn>
+                <EuiFlexItem grow={false}>
+                  <EuiIcon type={'sortRight'} />
+                </EuiFlexItem>
+                <EuiFlexItem>{'signal.rule.risk_score'}</EuiFlexItem>
+              </EuiFlexGroup>
+            ),
+          };
+        })
+    : []),
 ];
 
 const MyRefUrlLink = styled(EuiLink)`

diff --git a/...plugins/security_solution/public/detections/components/rules/risk_score_mapping/index.tsx b/...plugins/security_solution/public/detections/components/rules/risk_score_mapping/index.tsx
@@ -14,8 +14,9 @@ import {
   EuiIcon,
   EuiSpacer,
 } from '@elastic/eui';
-import React, { useCallback, useEffect, useMemo, useState } from 'react';
+import React, { useCallback, useMemo } from 'react';
 import styled from 'styled-components';
+import { noop } from 'lodash/fp';
 import * as i18n from './translations';
 import { FieldHook } from '../../../../../../../../src/plugins/es_ui_shared/static/forms/hook_form_lib';
 import { CommonUseField } from '../../../../cases/components/create';
@@ -24,6 +25,10 @@ import { FieldComponent } from '../../../../common/components/autocomplete/field
 import { IFieldType } from '../../../../../../../../src/plugins/data/common/index_patterns/fields';
 import { IIndexPattern } from '../../../../../../../../src/plugins/data/common/index_patterns';
 
+const RiskScoreMappingEuiFormRow = styled(EuiFormRow)`
+  width: 468px;
+`;
+
 const NestedContent = styled.div`
   margin-left: 24px;
 `;
@@ -41,6 +46,7 @@ interface RiskScoreFieldProps {
   field: FieldHook;
   idAria: string;
   indices: IIndexPattern;
+  isDisabled: boolean;
   placeholder?: string;
 }
 
@@ -49,40 +55,23 @@ export const RiskScoreField = ({
   field,
   idAria,
   indices,
+  isDisabled,
   placeholder,
 }: RiskScoreFieldProps) => {
-  const [isRiskScoreMappingChecked, setIsRiskScoreMappingChecked] = useState(false);
-  const [initialFieldCheck, setInitialFieldCheck] = useState(true);
-
   const fieldTypeFilter = useMemo(() => ['number'], []);
 
-  useEffect(() => {
-    if (
-      !isRiskScoreMappingChecked &&
-      initialFieldCheck &&
-      (field.value as AboutStepRiskScore).mapping?.length > 0
-    ) {
-      setIsRiskScoreMappingChecked(true);
-      setInitialFieldCheck(false);
-    }
-  }, [
-    field,
-    initialFieldCheck,
-    isRiskScoreMappingChecked,
-    setIsRiskScoreMappingChecked,
-    setInitialFieldCheck,
-  ]);
-
   const handleFieldChange = useCallback(
     ([newField]: IFieldType[]): void => {
       const values = field.value as AboutStepRiskScore;
       field.setValue({
         value: values.value,
+        isMappingChecked: values.isMappingChecked,
         mapping: [
           {
             field: newField?.name ?? '',
             operator: 'equals',
-            value: '',
+            value: undefined,
+            riskScore: undefined,
           },
         ],
       });
@@ -99,8 +88,13 @@ export const RiskScoreField = ({
   }, [field.value, indices]);
 
   const handleRiskScoreMappingChecked = useCallback(() => {
-    setIsRiskScoreMappingChecked(!isRiskScoreMappingChecked);
-  }, [isRiskScoreMappingChecked, setIsRiskScoreMappingChecked]);
+    const values = field.value as AboutStepRiskScore;
+    field.setValue({
+      value: values.value,
+      mapping: [...values.mapping],
+      isMappingChecked: !values.isMappingChecked,
+    });
+  }, [field]);
 
   const riskScoreLabel = useMemo(() => {
     return (
@@ -117,11 +111,16 @@ export const RiskScoreField = ({
   const riskScoreMappingLabel = useMemo(() => {
     return (
       <div>
-        <EuiFlexGroup alignItems="center" gutterSize="s" onClick={handleRiskScoreMappingChecked}>
+        <EuiFlexGroup
+          alignItems="center"
+          gutterSize="s"
+          onClick={!isDisabled ? handleRiskScoreMappingChecked : noop}
+        >
           <EuiFlexItem grow={false}>
             <EuiCheckbox
               id={`risk_score-mapping-override`}
-              checked={isRiskScoreMappingChecked}
+              checked={(field.value as AboutStepRiskScore).isMappingChecked}
+              disabled={isDisabled}
               onChange={handleRiskScoreMappingChecked}
             />
           </EuiFlexItem>
@@ -133,7 +132,7 @@ export const RiskScoreField = ({
         </NestedContent>
       </div>
     );
-  }, [handleRiskScoreMappingChecked, isRiskScoreMappingChecked]);
+  }, [field.value, handleRiskScoreMappingChecked, isDisabled]);
 
   return (
     <EuiFlexGroup>
@@ -153,6 +152,7 @@ export const RiskScoreField = ({
             componentProps={{
               idAria: 'detectionEngineStepAboutRuleRiskScore',
               'data-test-subj': 'detectionEngineStepAboutRuleRiskScore',
+              isDisabled,
               euiFieldProps: {
                 max: 100,
                 min: 0,
@@ -166,11 +166,11 @@ export const RiskScoreField = ({
         </EuiFormRow>
       </EuiFlexItem>
       <EuiFlexItem>
-        <EuiFormRow
+        <RiskScoreMappingEuiFormRow
           label={riskScoreMappingLabel}
           labelAppend={field.labelAppend}
           helpText={
-            isRiskScoreMappingChecked ? (
+            (field.value as AboutStepRiskScore).isMappingChecked ? (
               <NestedContent>{i18n.RISK_SCORE_MAPPING_DETAILS}</NestedContent>
             ) : (
               ''
@@ -184,7 +184,7 @@ export const RiskScoreField = ({
         >
           <NestedContent>
             <EuiSpacer size="s" />
-            {isRiskScoreMappingChecked && (
+            {(field.value as AboutStepRiskScore).isMappingChecked && (
               <EuiFlexGroup direction={'column'} gutterSize="s">
                 <EuiFlexItem>
                   <EuiFlexGroup alignItems="center" gutterSize="s">
@@ -208,11 +208,11 @@ export const RiskScoreField = ({
                         fieldTypeFilter={fieldTypeFilter}
                         isLoading={false}
                         isClearable={false}
-                        isDisabled={false}
+                        isDisabled={isDisabled}
                         onChange={handleFieldChange}
                         data-test-subj={dataTestSubj}
                         aria-label={idAria}
-                        fieldInputWidth={230}
+                        fieldInputWidth={270}
                       />
                     </EuiFlexItem>
                     <EuiFlexItemIconColumn grow={false}>
@@ -226,7 +226,7 @@ export const RiskScoreField = ({
               </EuiFlexGroup>
             )}
           </NestedContent>
-        </EuiFormRow>
+        </RiskScoreMappingEuiFormRow>
       </EuiFlexItem>
     </EuiFlexGroup>
   );