[Security Solution][Detections][7.12] Critical Threshold Rule Fixes (e…

…lastic#92667) (elastic#93141) * Threshold cardinality validation * Remove comments * Fix legacy threshold signal dupe mitigation * Add find_threshold_signals tests * remove comment * bug fixes * Fix edit form value initialization for cardinality_value * Fix test * Type and test fixes * Tests/types * Reenable threshold cypress test * Schema fixes * Types and tests, normalize threshold field util * Continue cleaning up types * Some more pre-7.12 tests * Limit cardinality_field to length 1 for now * Cardinality to array * Cardinality to array * Tests/types * cardinality can be null * Handle empty threshold field in bulk_create_threshold_signals * Remove cardinality_field, cardinality_value Co-authored-by: Kibana Machine <[email protected]>
dej611 · Mar 2, 2021 · f2123b3 · f2123b3
1 parent 8eed46f
commit f2123b3
Show file tree

Hide file tree

Showing 28 changed files with 1,392 additions and 352 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
@@ -465,26 +465,56 @@ export type Threats = t.TypeOf<typeof threats>;
 export const threatsOrUndefined = t.union([threats, t.undefined]);
 export type ThreatsOrUndefined = t.TypeOf<typeof threatsOrUndefined>;
 
+export const thresholdField = t.exact(
+  t.type({
+    field: t.union([t.string, t.array(t.string)]), // Covers pre- and post-7.12
+    value: PositiveIntegerGreaterThanZero,
+  })
+);
+export type ThresholdField = t.TypeOf<typeof thresholdField>;
+
+export const thresholdFieldNormalized = t.exact(
+  t.type({
+    field: t.array(t.string),
+    value: PositiveIntegerGreaterThanZero,
+  })
+);
+export type ThresholdFieldNormalized = t.TypeOf<typeof thresholdFieldNormalized>;
+
+export const thresholdCardinalityField = t.exact(
+  t.type({
+    field: t.string,
+    value: PositiveInteger,
+  })
+);
+export type ThresholdCardinalityField = t.TypeOf<typeof thresholdCardinalityField>;
+
 export const threshold = t.intersection([
-  t.exact(
-    t.type({
-      field: t.union([t.string, t.array(t.string)]),
-      value: PositiveIntegerGreaterThanZero,
-    })
-  ),
+  thresholdField,
   t.exact(
     t.partial({
-      cardinality_field: t.union([t.string, t.array(t.string), t.undefined, t.null]),
-      cardinality_value: t.union([PositiveInteger, t.undefined, t.null]), // TODO: cardinality_value should be set if cardinality_field is set
+      cardinality: t.union([t.array(thresholdCardinalityField), t.null]),
     })
   ),
 ]);
-// TODO: codec to transform threshold field string to string[] ?
 export type Threshold = t.TypeOf<typeof threshold>;
 
 export const thresholdOrUndefined = t.union([threshold, t.undefined]);
 export type ThresholdOrUndefined = t.TypeOf<typeof thresholdOrUndefined>;
 
+export const thresholdNormalized = t.intersection([
+  thresholdFieldNormalized,
+  t.exact(
+    t.partial({
+      cardinality: t.union([t.array(thresholdCardinalityField), t.null]),
+    })
+  ),
+]);
+export type ThresholdNormalized = t.TypeOf<typeof thresholdNormalized>;
+
+export const thresholdNormalizedOrUndefined = t.union([thresholdNormalized, t.undefined]);
+export type ThresholdNormalizedOrUndefined = t.TypeOf<typeof thresholdNormalizedOrUndefined>;
+
 export const created_at = IsoDateString;
 export const updated_at = IsoDateString;
 export const updated_by = t.string;

diff --git a/x-pack/plugins/security_solution/common/detection_engine/utils.test.ts b/x-pack/plugins/security_solution/common/detection_engine/utils.test.ts
@@ -5,7 +5,13 @@
  * 2.0.
  */
 
-import { hasEqlSequenceQuery, hasLargeValueList, hasNestedEntry, isThreatMatchRule } from './utils';
+import {
+  hasEqlSequenceQuery,
+  hasLargeValueList,
+  hasNestedEntry,
+  isThreatMatchRule,
+  normalizeThresholdField,
+} from './utils';
 import { EntriesArray } from '../shared_imports';
 
 describe('#hasLargeValueList', () => {
@@ -151,3 +157,21 @@ describe('#hasEqlSequenceQuery', () => {
     });
   });
 });
+
+describe('normalizeThresholdField', () => {
+  it('converts a string to a string array', () => {
+    expect(normalizeThresholdField('host.name')).toEqual(['host.name']);
+  });
+  it('returns a string array when a string array is passed in', () => {
+    expect(normalizeThresholdField(['host.name'])).toEqual(['host.name']);
+  });
+  it('converts undefined to an empty array', () => {
+    expect(normalizeThresholdField(undefined)).toEqual([]);
+  });
+  it('converts null to an empty array', () => {
+    expect(normalizeThresholdField(null)).toEqual([]);
+  });
+  it('converts an empty string to an empty array', () => {
+    expect(normalizeThresholdField('')).toEqual([]);
+  });
+});
diff --git a/x-pack/plugins/security_solution/common/detection_engine/utils.ts b/x-pack/plugins/security_solution/common/detection_engine/utils.ts
@@ -5,6 +5,8 @@
  * 2.0.
  */
 
+import { isEmpty } from 'lodash';
+
 import {
   CreateExceptionListItemSchema,
   EntriesArray,
@@ -42,3 +44,13 @@ export const isQueryRule = (ruleType: Type | undefined): boolean =>
   ruleType === 'query' || ruleType === 'saved_query';
 export const isThreatMatchRule = (ruleType: Type | undefined): boolean =>
   ruleType === 'threat_match';
+
+export const normalizeThresholdField = (
+  thresholdField: string | string[] | null | undefined
+): string[] => {
+  return Array.isArray(thresholdField)
+    ? thresholdField
+    : isEmpty(thresholdField)
+    ? []
+    : [thresholdField!];
+};
diff --git a/...gins/security_solution/common/search_strategy/security_solution/matrix_histogram/index.ts b/...gins/security_solution/common/search_strategy/security_solution/matrix_histogram/index.ts
@@ -40,8 +40,10 @@ export interface MatrixHistogramRequestOptions extends RequestBasicOptions {
     | {
         field: string | string[] | undefined;
         value: number;
-        cardinality_field?: string | undefined;
-        cardinality_value?: number | undefined;
+        cardinality?: {
+          field: string[];
+          value: number;
+        };
       }
     | undefined;
   inspect?: Maybe<Inspect>;

diff --git a/x-pack/plugins/security_solution/cypress/integration/detection_rules/threshold_rule.spec.ts b/x-pack/plugins/security_solution/cypress/integration/detection_rules/threshold_rule.spec.ts
@@ -79,103 +79,100 @@ import { loginAndWaitForPageWithoutDateRange } from '../../tasks/login';
 
 import { DETECTIONS_URL } from '../../urls/navigation';
 
-// Skipped until post-FF for 7.12
-describe.skip('Threshold Rules', () => {
-  describe('Detection rules, threshold', () => {
-    const expectedUrls = newThresholdRule.referenceUrls.join('');
-    const expectedFalsePositives = newThresholdRule.falsePositivesExamples.join('');
-    const expectedTags = newThresholdRule.tags.join('');
-    const expectedMitre = formatMitreAttackDescription(newThresholdRule.mitre);
-
-    const rule = { ...newThresholdRule };
-
-    beforeEach(() => {
-      cleanKibana();
-      createTimeline(newThresholdRule.timeline).then((response) => {
-        rule.timeline.id = response.body.data.persistTimeline.timeline.savedObjectId;
-      });
+describe('Detection rules, threshold', () => {
+  const expectedUrls = newThresholdRule.referenceUrls.join('');
+  const expectedFalsePositives = newThresholdRule.falsePositivesExamples.join('');
+  const expectedTags = newThresholdRule.tags.join('');
+  const expectedMitre = formatMitreAttackDescription(newThresholdRule.mitre);
+
+  const rule = { ...newThresholdRule };
+
+  beforeEach(() => {
+    cleanKibana();
+    createTimeline(newThresholdRule.timeline).then((response) => {
+      rule.timeline.id = response.body.data.persistTimeline.timeline.savedObjectId;
     });
+  });
 
-    it('Creates and activates a new threshold rule', () => {
-      loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
-      waitForAlertsPanelToBeLoaded();
-      waitForAlertsIndexToBeCreated();
-      goToManageAlertsDetectionRules();
-      waitForRulesTableToBeLoaded();
-      goToCreateNewRule();
-      selectThresholdRuleType();
-      fillDefineThresholdRuleAndContinue(rule);
-      fillAboutRuleAndContinue(rule);
-      fillScheduleRuleAndContinue(rule);
-      createAndActivateRule();
-
-      cy.get(CUSTOM_RULES_BTN).should('have.text', 'Custom rules (1)');
-
-      changeRowsPerPageTo300();
-
-      const expectedNumberOfRules = 1;
-      cy.get(RULES_TABLE).then(($table) => {
-        cy.wrap($table.find(RULES_ROW).length).should('eql', expectedNumberOfRules);
-      });
+  it('Creates and activates a new threshold rule', () => {
+    loginAndWaitForPageWithoutDateRange(DETECTIONS_URL);
+    waitForAlertsPanelToBeLoaded();
+    waitForAlertsIndexToBeCreated();
+    goToManageAlertsDetectionRules();
+    waitForRulesTableToBeLoaded();
+    goToCreateNewRule();
+    selectThresholdRuleType();
+    fillDefineThresholdRuleAndContinue(rule);
+    fillAboutRuleAndContinue(rule);
+    fillScheduleRuleAndContinue(rule);
+    createAndActivateRule();
+
+    cy.get(CUSTOM_RULES_BTN).should('have.text', 'Custom rules (1)');
+
+    changeRowsPerPageTo300();
+
+    const expectedNumberOfRules = 1;
+    cy.get(RULES_TABLE).then(($table) => {
+      cy.wrap($table.find(RULES_ROW).length).should('eql', expectedNumberOfRules);
+    });
 
-      filterByCustomRules();
+    filterByCustomRules();
 
-      cy.get(RULES_TABLE).then(($table) => {
-        cy.wrap($table.find(RULES_ROW).length).should('eql', 1);
-      });
-      cy.get(RULE_NAME).should('have.text', rule.name);
-      cy.get(RISK_SCORE).should('have.text', rule.riskScore);
-      cy.get(SEVERITY).should('have.text', rule.severity);
-      cy.get(RULE_SWITCH).should('have.attr', 'aria-checked', 'true');
-
-      goToRuleDetails();
-
-      cy.get(RULE_NAME_HEADER).should('have.text', `${rule.name}`);
-      cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', rule.description);
-      cy.get(ABOUT_DETAILS).within(() => {
-        getDetails(SEVERITY_DETAILS).should('have.text', rule.severity);
-        getDetails(RISK_SCORE_DETAILS).should('have.text', rule.riskScore);
-        getDetails(REFERENCE_URLS_DETAILS).should((details) => {
-          expect(removeExternalLinkText(details.text())).equal(expectedUrls);
-        });
-        getDetails(FALSE_POSITIVES_DETAILS).should('have.text', expectedFalsePositives);
-        getDetails(MITRE_ATTACK_DETAILS).should((mitre) => {
-          expect(removeExternalLinkText(mitre.text())).equal(expectedMitre);
-        });
-        getDetails(TAGS_DETAILS).should('have.text', expectedTags);
-      });
-      cy.get(INVESTIGATION_NOTES_TOGGLE).click({ force: true });
-      cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
-      cy.get(DEFINITION_DETAILS).within(() => {
-        getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
-        getDetails(CUSTOM_QUERY_DETAILS).should('have.text', rule.customQuery);
-        getDetails(RULE_TYPE_DETAILS).should('have.text', 'Threshold');
-        getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
-        getDetails(THRESHOLD_DETAILS).should(
-          'have.text',
-          `Results aggregated by ${rule.thresholdField} >= ${rule.threshold}`
-        );
+    cy.get(RULES_TABLE).then(($table) => {
+      cy.wrap($table.find(RULES_ROW).length).should('eql', 1);
+    });
+    cy.get(RULE_NAME).should('have.text', rule.name);
+    cy.get(RISK_SCORE).should('have.text', rule.riskScore);
+    cy.get(SEVERITY).should('have.text', rule.severity);
+    cy.get(RULE_SWITCH).should('have.attr', 'aria-checked', 'true');
+
+    goToRuleDetails();
+
+    cy.get(RULE_NAME_HEADER).should('have.text', `${rule.name}`);
+    cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', rule.description);
+    cy.get(ABOUT_DETAILS).within(() => {
+      getDetails(SEVERITY_DETAILS).should('have.text', rule.severity);
+      getDetails(RISK_SCORE_DETAILS).should('have.text', rule.riskScore);
+      getDetails(REFERENCE_URLS_DETAILS).should((details) => {
+        expect(removeExternalLinkText(details.text())).equal(expectedUrls);
       });
-      cy.get(SCHEDULE_DETAILS).within(() => {
-        getDetails(RUNS_EVERY_DETAILS).should(
-          'have.text',
-          `${rule.runsEvery.interval}${rule.runsEvery.type}`
-        );
-        getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should(
-          'have.text',
-          `${rule.lookBack.interval}${rule.lookBack.type}`
-        );
+      getDetails(FALSE_POSITIVES_DETAILS).should('have.text', expectedFalsePositives);
+      getDetails(MITRE_ATTACK_DETAILS).should((mitre) => {
+        expect(removeExternalLinkText(mitre.text())).equal(expectedMitre);
       });
+      getDetails(TAGS_DETAILS).should('have.text', expectedTags);
+    });
+    cy.get(INVESTIGATION_NOTES_TOGGLE).click({ force: true });
+    cy.get(ABOUT_INVESTIGATION_NOTES).should('have.text', INVESTIGATION_NOTES_MARKDOWN);
+    cy.get(DEFINITION_DETAILS).within(() => {
+      getDetails(INDEX_PATTERNS_DETAILS).should('have.text', indexPatterns.join(''));
+      getDetails(CUSTOM_QUERY_DETAILS).should('have.text', rule.customQuery);
+      getDetails(RULE_TYPE_DETAILS).should('have.text', 'Threshold');
+      getDetails(TIMELINE_TEMPLATE_DETAILS).should('have.text', 'None');
+      getDetails(THRESHOLD_DETAILS).should(
+        'have.text',
+        `Results aggregated by ${rule.thresholdField} >= ${rule.threshold}`
+      );
+    });
+    cy.get(SCHEDULE_DETAILS).within(() => {
+      getDetails(RUNS_EVERY_DETAILS).should(
+        'have.text',
+        `${rule.runsEvery.interval}${rule.runsEvery.type}`
+      );
+      getDetails(ADDITIONAL_LOOK_BACK_DETAILS).should(
+        'have.text',
+        `${rule.lookBack.interval}${rule.lookBack.type}`
+      );
+    });
 
-      waitForTheRuleToBeExecuted();
-      waitForAlertsToPopulate();
+    waitForTheRuleToBeExecuted();
+    waitForAlertsToPopulate();
 
-      cy.get(NUMBER_OF_ALERTS).should(($count) => expect(+$count.text()).to.be.lt(100));
-      cy.get(ALERT_RULE_NAME).first().should('have.text', rule.name);
-      cy.get(ALERT_RULE_VERSION).first().should('have.text', '1');
-      cy.get(ALERT_RULE_METHOD).first().should('have.text', 'threshold');
-      cy.get(ALERT_RULE_SEVERITY).first().should('have.text', rule.severity.toLowerCase());
-      cy.get(ALERT_RULE_RISK_SCORE).first().should('have.text', rule.riskScore);
-    });
+    cy.get(NUMBER_OF_ALERTS).should(($count) => expect(+$count.text()).to.be.lt(100));
+    cy.get(ALERT_RULE_NAME).first().should('have.text', rule.name);
+    cy.get(ALERT_RULE_VERSION).first().should('have.text', '1');
+    cy.get(ALERT_RULE_METHOD).first().should('have.text', 'threshold');
+    cy.get(ALERT_RULE_SEVERITY).first().should('have.text', rule.severity.toLowerCase());
+    cy.get(ALERT_RULE_RISK_SCORE).first().should('have.text', rule.riskScore);
   });
 });
diff --git a/x-pack/plugins/security_solution/public/common/components/matrix_histogram/types.ts b/x-pack/plugins/security_solution/public/common/components/matrix_histogram/types.ts
@@ -78,8 +78,10 @@ export interface MatrixHistogramQueryProps {
     | {
         field: string | string[] | undefined;
         value: number;
-        cardinality_field?: string | undefined;
-        cardinality_value?: number | undefined;
+        cardinality?: {
+          field: string[];
+          value: number;
+        };
       }
     | undefined;
   skip?: boolean;

diff --git a/...plugins/security_solution/public/detections/components/rules/query_preview/index.test.tsx b/...plugins/security_solution/public/detections/components/rules/query_preview/index.test.tsx
@@ -291,8 +291,10 @@ describe('PreviewQuery', () => {
             threshold={{
               field: 'agent.hostname',
               value: 200,
-              cardinality_field: 'user.name',
-              cardinality_value: 2,
+              cardinality: {
+                field: ['user.name'],
+                value: 2,
+              },
             }}
             isDisabled={false}
           />
@@ -338,8 +340,10 @@ describe('PreviewQuery', () => {
             threshold={{
               field: 'agent.hostname',
               value: 200,
-              cardinality_field: 'user.name',
-              cardinality_value: 2,
+              cardinality: {
+                field: ['user.name'],
+                value: 2,
+              },
             }}
             isDisabled={false}
           />
@@ -382,8 +386,10 @@ describe('PreviewQuery', () => {
             threshold={{
               field: undefined,
               value: 200,
-              cardinality_field: 'user.name',
-              cardinality_value: 2,
+              cardinality: {
+                field: ['user.name'],
+                value: 2,
+              },
             }}
             isDisabled={false}
           />
@@ -414,8 +420,10 @@ describe('PreviewQuery', () => {
             threshold={{
               field: '   ',
               value: 200,
-              cardinality_field: 'user.name',
-              cardinality_value: 2,
+              cardinality: {
+                field: ['user.name'],
+                value: 2,
+              },
             }}
             isDisabled={false}
           />