[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
2021-01-06 00:53:13 +01:00 · 2021-01-06 00:53:13 +01:00 · a1f949ba9c
parent 933c6a5ae9
commit a1f949ba9c
8 changed files with 167 additions and 116 deletions
--- 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();

--- 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';

--- 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();
--- 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 });
--- 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 { comboOptions, labels, selectedComboOptions } = useMemo(
-    (): GetGenericComboBoxPropsReturn =>
-      getGenericComboBoxProps<IFieldType>({
-        options: optionsMemo,
-        selectedOptions: selectedOptionsMemo,
-        getLabel,
-      }),
-    [optionsMemo, selectedOptionsMemo, getLabel]
+
+  const { availableFields, selectedFields } = useMemo(
+    () => getComboBoxFields(indexPattern, selectedField, fieldTypeFilter),
+    [indexPattern, selectedField, fieldTypeFilter]
  );

-  const handleValuesChange = (newOptions: EuiComboBoxOptionOption[]): void => {
-    const newValues: IFieldType[] = newOptions.map(
-      ({ label }) => optionsMemo[labels.indexOf(label)]
-    );
-    onChange(newValues);
-  };
+  const { comboOptions, labels, selectedComboOptions } = useMemo(
+    () => getComboBoxProps({ availableFields, selectedFields }),
+    [availableFields, selectedFields]
+  );
+
+  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;
+};
--- a/x-pack/plugins/security_solution/public/detections/components/rules/risk_score_mapping/index.tsx
+++ b/x-pack/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 fieldTypeFilter = useMemo(() => ['number'], []);
-  const { value: fieldValue, setValue } = field;
+  const { value, isMappingChecked, mapping } = field.value;
+  const { setValue } = field;

-  const handleFieldChange = useCallback(
-    ([newField]: IFieldType[]): void => {
-      const values = fieldValue as AboutStepRiskScore;
+  const fieldTypeFilter = useMemo(() => ['number'], []);
+  const selectedField = useMemo(() => getFieldTypeByMapping(mapping, indices), [mapping, indices]);
+
+  const handleDefaultRiskScoreChange = useCallback(
+    (e: React.ChangeEvent<HTMLInputElement> | React.MouseEvent<HTMLButtonElement>): void => {
+      const range = (e.target as HTMLInputElement).value;
      setValue({
-        value: values.value,
-        isMappingChecked: values.isMappingChecked,
+        value: Number(range.trim()),
+        isMappingChecked,
+        mapping,
+      });
+    },
+    [setValue, isMappingChecked, mapping]
+  );
+
+  const handleRiskScoreMappingChange = useCallback(
+    ([newField]: IFieldType[]): void => {
+      setValue({
+        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,12 +151,12 @@ 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
@ -170,7 +164,7 @@ export const RiskScoreField = ({
            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' };
+};
--- a/x-pack/plugins/security_solution/public/detections/components/rules/severity_mapping/index.tsx
+++ b/x-pack/plugins/security_solution/public/detections/components/rules/severity_mapping/index.tsx
@ -52,7 +52,7 @@ const EuiFlexItemSeverityColumn = styled(EuiFlexItem)`

 interface SeverityFieldProps {
  dataTestSubj: string;
-  field: FieldHook;
+  field: FieldHook<AboutStepSeverity>;
  idAria: string;
  indices: IIndexPattern;
  isDisabled: boolean;
@ -67,8 +67,8 @@ export const SeverityField = ({
  isDisabled,
  options,
 }: SeverityFieldProps) => {
+  const { value, isMappingChecked, mapping } = field.value;
  const { setValue } = field;
-  const { value, isMappingChecked, mapping } = field.value as AboutStepSeverity;

  const handleFieldValueChange = useCallback(
    (newMappingItems: SeverityMapping, index: number): void => {
@ -97,8 +97,8 @@ export const SeverityField = ({
    [mapping, handleFieldValueChange]
  );

-  const handleSecurityLevelChange = useCallback(
-    (newValue: string) => {
+  const handleDefaultSeverityChange = useCallback(
+    (newValue: Severity) => {
      setValue({
        value: newValue,
        isMappingChecked,
@ -124,14 +124,6 @@ export const SeverityField = ({
    [mapping, handleFieldValueChange]
  );

-  const getIFieldTypeFromFieldName = (
-    fieldName: string | undefined,
-    iIndexPattern: IIndexPattern
-  ): IFieldType | undefined => {
-    const [iFieldType] = iIndexPattern.fields.filter(({ name }) => fieldName === name);
-    return iFieldType;
-  };
-
  const handleSeverityMappingChecked = useCallback(() => {
    setValue({
      value,
@ -195,7 +187,7 @@ export const SeverityField = ({
            fullWidth={false}
            disabled={false}
            valueOfSelected={value}
-            onChange={handleSecurityLevelChange}
+            onChange={handleDefaultSeverityChange}
            options={options}
            data-test-subj="select"
          />
@ -244,10 +236,7 @@ export const SeverityField = ({
                      <EuiFlexItemComboBoxColumn>
                        <FieldComponent
                          placeholder={''}
-                          selectedField={getIFieldTypeFromFieldName(
-                            severityMappingItem.field,
-                            indices
-                          )}
+                          selectedField={getFieldTypeByMapping(severityMappingItem, indices)}
                          isLoading={false}
                          isDisabled={isDisabled}
                          isClearable={false}
@ -265,10 +254,7 @@ export const SeverityField = ({
                      <EuiFlexItemComboBoxColumn>
                        <AutocompleteFieldMatchComponent
                          placeholder={''}
-                          selectedField={getIFieldTypeFromFieldName(
-                            severityMappingItem.field,
-                            indices
-                          )}
+                          selectedField={getFieldTypeByMapping(severityMappingItem, indices)}
                          selectedValue={severityMappingItem.value}
                          isClearable={false}
                          isDisabled={isDisabled}
@ -303,3 +289,20 @@ export const SeverityField = ({
    </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 + value to a certain severity value.
+ * @param pattern Existing index pattern.
+ */
+const getFieldTypeByMapping = (
+  mapping: SeverityMappingItem,
+  pattern: IIndexPattern
+): IFieldType => {
+  const { field } = mapping;
+  const [knownFieldType] = pattern.fields.filter(({ name }) => field === name);
+  return knownFieldType ?? { name: field, type: 'string' };
+};
--- a/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.test.tsx
+++ b/x-pack/plugins/security_solution/public/detections/components/rules/step_about_rule/index.test.tsx
@ -206,7 +206,7 @@ describe('StepAboutRuleComponent', () => {
      .simulate('change', { target: { value: 'Test description text' } });

    wrapper
-      .find('[data-test-subj="detectionEngineStepAboutRuleRiskScore"] input')
+      .find('[data-test-subj="detectionEngineStepAboutRuleRiskScore-defaultRisk"] input')
      .first()
      .simulate('change', { target: { value: '80' } });