[Security Solution][Detections] Fix severity and risk score overrides…

… when field mapping exists but the mapped fields do not (#87004) * Fix Source field combobox in Severity override and Risk score override sections * Clean up * Fix unit and Cypress tests
elastic · Jan 5, 2021 · a1f949b · a1f949b
1 parent 933c6a5
commit a1f949b
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 113 deletions.
diff --git a/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts b/x-pack/plugins/security_solution/cypress/integration/alerts_detection_rules_custom.spec.ts
@@ -33,7 +33,7 @@ import {
   DEFINE_CONTINUE_BUTTON,
   DEFINE_EDIT_BUTTON,
   DEFINE_INDEX_INPUT,
-  RISK_INPUT,
+  DEFAULT_RISK_SCORE_INPUT,
   RULE_DESCRIPTION_INPUT,
   RULE_NAME_INPUT,
   SCHEDULE_INTERVAL_AMOUNT_INPUT,
@@ -318,7 +318,7 @@ describe.skip('Custom detection rules deletion and edition', () => {
       cy.get(RULE_DESCRIPTION_INPUT).should('have.text', existingRule.description);
       cy.get(TAGS_FIELD).should('have.text', existingRule.tags.join(''));
       cy.get(SEVERITY_DROPDOWN).should('have.text', existingRule.severity);
-      cy.get(RISK_INPUT).invoke('val').should('eql', existingRule.riskScore);
+      cy.get(DEFAULT_RISK_SCORE_INPUT).invoke('val').should('eql', existingRule.riskScore);
 
       goToScheduleStepTab();
 

diff --git a/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts b/x-pack/plugins/security_solution/cypress/screens/create_new_rule.ts
@@ -101,7 +101,11 @@ export const REFERENCE_URLS_INPUT =
 
 export const REFRESH_BUTTON = '[data-test-subj="refreshButton"]';
 
-export const RISK_INPUT = '.euiRangeInput';
+export const DEFAULT_RISK_SCORE_INPUT =
+  '[data-test-subj="detectionEngineStepAboutRuleRiskScore-defaultRiskRange"].euiRangeInput';
+
+export const DEFAULT_RISK_SCORE_SLIDER =
+  '[data-test-subj="detectionEngineStepAboutRuleRiskScore-defaultRiskRange"].euiRangeSlider';
 
 export const RISK_MAPPING_OVERRIDE_OPTION = '#risk_score-mapping-override';
 

diff --git a/x-pack/plugins/security_solution/cypress/tasks/common.ts b/x-pack/plugins/security_solution/cypress/tasks/common.ts
@@ -65,23 +65,20 @@ export const reload = (afterReload: () => void) => {
 };
 
 export const cleanKibana = () => {
-  cy.exec(`curl -XDELETE "${Cypress.env('ELASTICSEARCH_URL')}/.kibana\*" -k`);
+  const kibanaIndexUrl = `${Cypress.env('ELASTICSEARCH_URL')}/.kibana\*`;
 
-  // We wait until the kibana indexes are deleted
+  // Delete kibana indexes and wait until they are deleted
+  cy.request('DELETE', kibanaIndexUrl);
   cy.waitUntil(() => {
     cy.wait(500);
-    return cy
-      .request(`${Cypress.env('ELASTICSEARCH_URL')}/.kibana\*`)
-      .then((response) => JSON.stringify(response.body) === '{}');
+    return cy.request(kibanaIndexUrl).then((response) => JSON.stringify(response.body) === '{}');
   });
-  esArchiverLoadEmptyKibana();
 
-  // We wait until the kibana indexes are created
+  // Load kibana indexes and wait until they are created
+  esArchiverLoadEmptyKibana();
   cy.waitUntil(() => {
     cy.wait(500);
-    return cy
-      .request(`${Cypress.env('ELASTICSEARCH_URL')}/.kibana\*`)
-      .then((response) => JSON.stringify(response.body) !== '{}');
+    return cy.request(kibanaIndexUrl).then((response) => JSON.stringify(response.body) !== '{}');
   });
 
   removeSignalsIndex();

diff --git a/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts b/x-pack/plugins/security_solution/cypress/tasks/create_new_rule.ts
@@ -38,7 +38,7 @@ import {
   MITRE_TACTIC,
   REFERENCE_URLS_INPUT,
   REFRESH_BUTTON,
-  RISK_INPUT,
+  DEFAULT_RISK_SCORE_INPUT,
   RISK_MAPPING_OVERRIDE_OPTION,
   RISK_OVERRIDE,
   RULE_DESCRIPTION_INPUT,
@@ -91,7 +91,7 @@ export const fillAboutRule = (
   cy.get(SEVERITY_DROPDOWN).click({ force: true });
   cy.get(`#${rule.severity.toLowerCase()}`).click();
 
-  cy.get(RISK_INPUT).clear({ force: true }).type(`${rule.riskScore}`, { force: true });
+  cy.get(DEFAULT_RISK_SCORE_INPUT).type(`{selectall}${rule.riskScore}`, { force: true });
 
   rule.tags.forEach((tag) => {
     cy.get(TAGS_INPUT).type(`${tag}{enter}`, { force: true });
@@ -169,7 +169,7 @@ export const fillAboutRuleWithOverrideAndContinue = (rule: OverrideRule) => {
     cy.get(COMBO_BOX_INPUT).type(`${rule.riskOverride}{enter}`);
   });
 
-  cy.get(RISK_INPUT).clear({ force: true }).type(`${rule.riskScore}`, { force: true });
+  cy.get(DEFAULT_RISK_SCORE_INPUT).type(`{selectall}${rule.riskScore}`, { force: true });
 
   rule.tags.forEach((tag) => {
     cy.get(TAGS_INPUT).type(`${tag}{enter}`, { force: true });

diff --git a/x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx b/x-pack/plugins/security_solution/public/common/components/autocomplete/field.tsx
@@ -36,37 +36,26 @@ export const FieldComponent: React.FC<OperatorProps> = ({
   onChange,
 }): JSX.Element => {
   const [touched, setIsTouched] = useState(false);
-  const getLabel = useCallback(({ name }): string => name, []);
-  const optionsMemo = useMemo((): IFieldType[] => {
-    if (indexPattern != null) {
-      if (fieldTypeFilter.length > 0) {
-        return indexPattern.fields.filter(({ type }) => fieldTypeFilter.includes(type));
-      } else {
-        return indexPattern.fields;
-      }
-    } else {
-      return [];
-    }
-  }, [fieldTypeFilter, indexPattern]);
-  const selectedOptionsMemo = useMemo((): IFieldType[] => (selectedField ? [selectedField] : []), [
-    selectedField,
-  ]);
+
+  const { availableFields, selectedFields } = useMemo(
+    () => getComboBoxFields(indexPattern, selectedField, fieldTypeFilter),
+    [indexPattern, selectedField, fieldTypeFilter]
+  );
+
   const { comboOptions, labels, selectedComboOptions } = useMemo(
-    (): GetGenericComboBoxPropsReturn =>
-      getGenericComboBoxProps<IFieldType>({
-        options: optionsMemo,
-        selectedOptions: selectedOptionsMemo,
-        getLabel,
-      }),
-    [optionsMemo, selectedOptionsMemo, getLabel]
+    () => getComboBoxProps({ availableFields, selectedFields }),
+    [availableFields, selectedFields]
   );
 
-  const handleValuesChange = (newOptions: EuiComboBoxOptionOption[]): void => {
-    const newValues: IFieldType[] = newOptions.map(
-      ({ label }) => optionsMemo[labels.indexOf(label)]
-    );
-    onChange(newValues);
-  };
+  const handleValuesChange = useCallback(
+    (newOptions: EuiComboBoxOptionOption[]): void => {
+      const newValues: IFieldType[] = newOptions.map(
+        ({ label }) => availableFields[labels.indexOf(label)]
+      );
+      onChange(newValues);
+    },
+    [availableFields, labels, onChange]
+  );
 
   const handleTouch = useCallback((): void => {
     setIsTouched(true);
@@ -92,3 +81,57 @@ export const FieldComponent: React.FC<OperatorProps> = ({
 };
 
 FieldComponent.displayName = 'Field';
+
+interface ComboBoxFields {
+  availableFields: IFieldType[];
+  selectedFields: IFieldType[];
+}
+
+const getComboBoxFields = (
+  indexPattern: IIndexPattern | undefined,
+  selectedField: IFieldType | undefined,
+  fieldTypeFilter: string[]
+): ComboBoxFields => {
+  const existingFields = getExistingFields(indexPattern);
+  const selectedFields = getSelectedFields(selectedField);
+  const availableFields = getAvailableFields(existingFields, selectedFields, fieldTypeFilter);
+
+  return { availableFields, selectedFields };
+};
+
+const getComboBoxProps = (fields: ComboBoxFields): GetGenericComboBoxPropsReturn => {
+  const { availableFields, selectedFields } = fields;
+
+  return getGenericComboBoxProps<IFieldType>({
+    options: availableFields,
+    selectedOptions: selectedFields,
+    getLabel: (field) => field.name,
+  });
+};
+
+const getExistingFields = (indexPattern: IIndexPattern | undefined): IFieldType[] => {
+  return indexPattern != null ? indexPattern.fields : [];
+};
+
+const getSelectedFields = (selectedField: IFieldType | undefined): IFieldType[] => {
+  return selectedField ? [selectedField] : [];
+};
+
+const getAvailableFields = (
+  existingFields: IFieldType[],
+  selectedFields: IFieldType[],
+  fieldTypeFilter: string[]
+): IFieldType[] => {
+  const map = new Map<string, IFieldType>();
+
+  existingFields.forEach((f) => map.set(f.name, f));
+  selectedFields.forEach((f) => map.set(f.name, f));
+
+  const array = Array.from(map.values());
+
+  if (fieldTypeFilter.length > 0) {
+    return array.filter(({ type }) => fieldTypeFilter.includes(type));
+  }
+
+  return array;
+};
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
@@ -24,6 +24,7 @@ import { AboutStepRiskScore } from '../../../pages/detection_engine/rules/types'
 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';
+import { RiskScoreMapping } from '../../../../../common/detection_engine/schemas/common/schemas';
 
 const NestedContent = styled.div`
   margin-left: 24px;
@@ -43,7 +44,7 @@ const EuiFlexItemRiskScoreColumn = styled(EuiFlexItem)`
 
 interface RiskScoreFieldProps {
   dataTestSubj: string;
-  field: FieldHook;
+  field: FieldHook<AboutStepRiskScore>;
   idAria: string;
   indices: IIndexPattern;
   isDisabled: boolean;
@@ -58,56 +59,49 @@ export const RiskScoreField = ({
   isDisabled,
   placeholder,
 }: RiskScoreFieldProps) => {
+  const { value, isMappingChecked, mapping } = field.value;
+  const { setValue } = field;
+
   const fieldTypeFilter = useMemo(() => ['number'], []);
-  const { value: fieldValue, setValue } = field;
+  const selectedField = useMemo(() => getFieldTypeByMapping(mapping, indices), [mapping, indices]);
 
-  const handleFieldChange = useCallback(
+  const handleDefaultRiskScoreChange = useCallback(
+    (e: React.ChangeEvent<HTMLInputElement> | React.MouseEvent<HTMLButtonElement>): void => {
+      const range = (e.target as HTMLInputElement).value;
+      setValue({
+        value: Number(range.trim()),
+        isMappingChecked,
+        mapping,
+      });
+    },
+    [setValue, isMappingChecked, mapping]
+  );
+
+  const handleRiskScoreMappingChange = useCallback(
     ([newField]: IFieldType[]): void => {
-      const values = fieldValue as AboutStepRiskScore;
       setValue({
-        value: values.value,
-        isMappingChecked: values.isMappingChecked,
+        value,
+        isMappingChecked,
         mapping: [
           {
             field: newField?.name ?? '',
             operator: 'equals',
             value: '',
-            riskScore: undefined,
+            risk_score: undefined,
           },
         ],
       });
     },
-    [setValue, fieldValue]
+    [setValue, value, isMappingChecked]
   );
 
-  const handleRangeFieldChange = useCallback(
-    (e: React.ChangeEvent<HTMLInputElement> | React.MouseEvent<HTMLButtonElement>): void => {
-      const range = (e.target as HTMLInputElement).value;
-      setValue({
-        value: range.trim() === '' ? '' : +range,
-        isMappingChecked: (fieldValue as AboutStepRiskScore).isMappingChecked,
-        mapping: (fieldValue as AboutStepRiskScore).mapping,
-      });
-    },
-    [fieldValue, setValue]
-  );
-
-  const selectedField = useMemo(() => {
-    const existingField = (fieldValue as AboutStepRiskScore).mapping?.[0]?.field ?? '';
-    const [newSelectedField] = indices.fields.filter(
-      ({ name }) => existingField != null && existingField === name
-    );
-    return newSelectedField;
-  }, [fieldValue, indices]);
-
   const handleRiskScoreMappingChecked = useCallback(() => {
-    const values = fieldValue as AboutStepRiskScore;
     setValue({
-      value: values.value,
-      mapping: [...values.mapping],
-      isMappingChecked: !values.isMappingChecked,
+      value,
+      isMappingChecked: !isMappingChecked,
+      mapping: [...mapping],
     });
-  }, [fieldValue, setValue]);
+  }, [setValue, value, isMappingChecked, mapping]);
 
   const riskScoreLabel = useMemo(() => {
     return (
@@ -132,7 +126,7 @@ export const RiskScoreField = ({
           <EuiFlexItem grow={false}>
             <EuiCheckbox
               id={`risk_score-mapping-override`}
-              checked={(fieldValue as AboutStepRiskScore).isMappingChecked}
+              checked={isMappingChecked}
               disabled={isDisabled}
               onChange={handleRiskScoreMappingChecked}
             />
@@ -145,7 +139,7 @@ export const RiskScoreField = ({
         </NestedContent>
       </div>
     );
-  }, [fieldValue, handleRiskScoreMappingChecked, isDisabled]);
+  }, [isMappingChecked, handleRiskScoreMappingChecked, isDisabled]);
 
   return (
     <EuiFlexGroup direction={'column'}>
@@ -157,20 +151,20 @@ export const RiskScoreField = ({
           error={'errorMessage'}
           isInvalid={false}
           fullWidth
-          data-test-subj="detectionEngineStepAboutRuleRiskScore"
-          describedByIds={['detectionEngineStepAboutRuleRiskScore']}
+          data-test-subj={`${dataTestSubj}-defaultRisk`}
+          describedByIds={idAria ? [idAria] : undefined}
         >
           <EuiRange
-            value={(fieldValue as AboutStepRiskScore).value}
-            onChange={handleRangeFieldChange}
+            value={value}
+            onChange={handleDefaultRiskScoreChange}
             max={100}
             min={0}
             showRange
             showInput
             fullWidth={false}
             showTicks
             tickInterval={25}
-            data-test-subj="range"
+            data-test-subj={`${dataTestSubj}-defaultRiskRange`}
           />
         </EuiFormRow>
       </EuiFlexItem>
@@ -179,11 +173,7 @@ export const RiskScoreField = ({
           label={riskScoreMappingLabel}
           labelAppend={field.labelAppend}
           helpText={
-            (fieldValue as AboutStepRiskScore).isMappingChecked ? (
-              <NestedContent>{i18n.RISK_SCORE_MAPPING_DETAILS}</NestedContent>
-            ) : (
-              ''
-            )
+            isMappingChecked ? <NestedContent>{i18n.RISK_SCORE_MAPPING_DETAILS}</NestedContent> : ''
           }
           error={'errorMessage'}
           isInvalid={false}
@@ -193,7 +183,7 @@ export const RiskScoreField = ({
         >
           <NestedContent>
             <EuiSpacer size="s" />
-            {(fieldValue as AboutStepRiskScore).isMappingChecked && (
+            {isMappingChecked && (
               <EuiFlexGroup direction={'column'} gutterSize="s">
                 <EuiFlexItem>
                   <EuiFlexGroup alignItems="center" gutterSize="s">
@@ -218,7 +208,7 @@ export const RiskScoreField = ({
                         isLoading={false}
                         isClearable={false}
                         isDisabled={isDisabled}
-                        onChange={handleFieldChange}
+                        onChange={handleRiskScoreMappingChange}
                         data-test-subj={dataTestSubj}
                         aria-label={idAria}
                       />
@@ -239,3 +229,17 @@ export const RiskScoreField = ({
     </EuiFlexGroup>
   );
 };
+
+/**
+ * Looks for field metadata (IFieldType) in existing index pattern.
+ * If specified field doesn't exist, returns a stub IFieldType created based on the mapping --
+ * because the field might not have been indexed yet, but we still need to display the mapping.
+ *
+ * @param mapping Mapping of a specified field name to risk score.
+ * @param pattern Existing index pattern.
+ */
+const getFieldTypeByMapping = (mapping: RiskScoreMapping, pattern: IIndexPattern): IFieldType => {
+  const field = mapping?.[0]?.field ?? '';
+  const [knownFieldType] = pattern.fields.filter(({ name }) => field != null && field === name);
+  return knownFieldType ?? { name: field, type: 'number' };
+};