[Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748) (#93157)

kibanamachine · yctercero · web-flow · commit a44d5a673097 · 2021-03-01T22:48:59.000-05:00
### Summary

Fixes a bug where max_signals was being reverted to it's default value when the rule was edited via the UI.

Co-authored-by: Yara Tercero &lt;yctercero@users.noreply.github.com&gt;
diff --git a/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts b/x-pack/plugins/security_solution/cypress/integration/detection_rules/custom_query_rule.spec.ts
@@ -108,6 +108,7 @@ import {
 } from '../../tasks/create_new_rule';
 import { saveEditedRule, waitForKibana } from '../../tasks/edit_rule';
 import { loginAndWaitForPageWithoutDateRange } from '../../tasks/login';
+import { activatesRule } from '../../tasks/rule_details';
 
 import { DETECTIONS_URL } from '../../urls/navigation';
 
@@ -308,6 +309,21 @@ describe('Custom detection rules deletion and edition', () => {
       reload();
     });
 
+    it('Only modifies rule active status on enable/disable', () => {
+      activatesRule();
+
+      cy.intercept('GET', `/api/detection_engine/rules?id=`).as('fetchRuleDetails');
+
+      goToRuleDetails();
+
+      cy.wait('@fetchRuleDetails').then(({ response }) => {
+        cy.wrap(response!.statusCode).should('eql', 200);
+
+        cy.wrap(response!.body.max_signals).should('eql', existingRule.maxSignals);
+        cy.wrap(response!.body.enabled).should('eql', false);
+      });
+    });
+
     it('Allows a rule to be edited', () => {
       editFirstRule();
       waitForKibana();
@@ -347,8 +363,17 @@ describe('Custom detection rules deletion and edition', () => {
       goToAboutStepTab();
       cy.get(TAGS_CLEAR_BUTTON).click({ force: true });
       fillAboutRule(editedRule);
+
+      cy.intercept('GET', '/api/detection_engine/rules?id').as('getRule');
+
       saveEditedRule();
 
+      cy.wait('@getRule').then(({ response }) => {
+        cy.wrap(response!.statusCode).should('eql', 200);
+        // ensure that editing rule does not modify max_signals
+        cy.wrap(response!.body.max_signals).should('eql', existingRule.maxSignals);
+      });
+
       cy.get(RULE_NAME_HEADER).should('have.text', `${editedRule.name}`);
       cy.get(ABOUT_RULE_DESCRIPTION).should('have.text', editedRule.description);
       cy.get(ABOUT_DETAILS).within(() => {
diff --git a/x-pack/plugins/security_solution/cypress/objects/rule.ts b/x-pack/plugins/security_solution/cypress/objects/rule.ts
@@ -54,6 +54,7 @@ export interface CustomRule {
   runsEvery: Interval;
   lookBack: Interval;
   timeline: CompleteTimeline;
+  maxSignals: number;
 }
 
 export interface ThresholdRule extends CustomRule {
@@ -174,6 +175,7 @@ export const newRule: CustomRule = {
   runsEvery,
   lookBack,
   timeline,
+  maxSignals: 100,
 };
 
 export const existingRule: CustomRule = {
@@ -192,6 +194,9 @@ export const existingRule: CustomRule = {
   runsEvery,
   lookBack,
   timeline,
+  // Please do not change, or if you do, needs
+  // to be any number other than default value
+  maxSignals: 500,
 };
 
 export const newOverrideRule: OverrideRule = {
@@ -213,6 +218,7 @@ export const newOverrideRule: OverrideRule = {
   runsEvery,
   lookBack,
   timeline,
+  maxSignals: 100,
 };
 
 export const newThresholdRule: ThresholdRule = {
@@ -232,6 +238,7 @@ export const newThresholdRule: ThresholdRule = {
   runsEvery,
   lookBack,
   timeline,
+  maxSignals: 100,
 };
 
 export const machineLearningRule: MachineLearningRule = {
@@ -265,6 +272,7 @@ export const eqlRule: CustomRule = {
   runsEvery,
   lookBack,
   timeline,
+  maxSignals: 100,
 };
 
 export const eqlSequenceRule: CustomRule = {
@@ -285,6 +293,7 @@ export const eqlSequenceRule: CustomRule = {
   runsEvery,
   lookBack,
   timeline,
+  maxSignals: 100,
 };
 
 export const newThreatIndicatorRule: ThreatIndicatorRule = {
@@ -304,6 +313,7 @@ export const newThreatIndicatorRule: ThreatIndicatorRule = {
   indicatorMapping: 'agent.id',
   indicatorIndexField: 'agent.threat',
   timeline,
+  maxSignals: 100,
 };
 
 export const severitiesOverride = ['Low', 'Medium', 'High', 'Critical'];
diff --git a/x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts b/x-pack/plugins/security_solution/cypress/tasks/api_calls/rules.ts
@@ -85,6 +85,7 @@ export const createCustomRuleActivated = (rule: CustomRule, ruleId = '1') =>
       language: 'kuery',
       enabled: true,
       tags: ['rule1'],
+      max_signals: 500,
     },
     headers: { 'kbn-xsrf': 'cypress-creds' },
     failOnStatusCode: false,
diff --git a/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts b/x-pack/plugins/security_solution/cypress/tasks/rule_details.ts
@@ -34,11 +34,6 @@ export const activatesRule = () => {
   });
 };
 
-export const deactivatesRule = () => {
-  cy.get(RULE_SWITCH).should('be.visible');
-  cy.get(RULE_SWITCH).click();
-};
-
 export const addsException = (exception: Exception) => {
   cy.get(LOADING_SPINNER).should('exist');
   cy.get(LOADING_SPINNER).should('not.exist');
diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/edit/index.tsx
@@ -251,6 +251,7 @@ const EditRulePageComponent: FC = () => {
           rule
         ),
         ...(ruleId ? { id: ruleId } : {}),
+        ...(rule != null ? { max_signals: rule.max_signals } : {}),
       });
     }
   }, [
diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_max_signals.json b/x-pack/plugins/security_solution/server/lib/detection_engine/scripts/rules/queries/query_with_max_signals.json
@@ -0,0 +1,9 @@
+{
+  "name": "Query With Max Signals",
+  "description": "Simplest query with max signals set to something other than default",
+  "risk_score": 1,
+  "severity": "high",
+  "type": "query",
+  "query": "user.name: root or user.name: admin",
+  "max_signals": 500
+}

Original file line number	Diff line number	Diff line change
`@@ -251,6 +251,7 @@ const EditRulePageComponent: FC = () => {`
`251`	`251`	`rule`
`252`	`252`	`),`
`253`	`253`	`...(ruleId ? { id: ruleId } : {}),`
	`254`	`+ ...(rule != null ? { max_signals: rule.max_signals } : {}),`
`254`	`255`	`});`
`255`	`256`	`}`
`256`	`257`	`}, [`