[Security Solution] Fix the rules coverage overview API contract (ela…

…stic#163838) **Epic:** elastic/security-team#2905 (internal) ## Summary In our API endpoints, we shouldn't expose parameters for features we don't support yet. This PR: - Removes the `CoverageOverviewRuleActivity.Available` and `CoverageOverviewRuleSource.Customized` enum values from the coverage endpoint's request schema. - Does some additional cleanup. We will add the removed parameters back when we add the corresponding enhancements to the feature.
kibanamachine · Aug 22, 2023 · 9b489ed · 9b489ed
1 parent 26389e5
commit 9b489ed
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 56 deletions.
diff --git a/.../common/api/detection_engine/rule_management/coverage_overview/coverage_overview_route.ts b/.../common/api/detection_engine/rule_management/coverage_overview/coverage_overview_route.ts
@@ -9,24 +9,18 @@ import * as t from 'io-ts';
 import { enumeration, NonEmptyArray, NonEmptyString } from '@kbn/securitysolution-io-ts-types';
 
 /**
- * Rule activity (status) filter applicable to two groups of rules
- * - installed from a Fleet package, custom and customized which are installed but customized later on which
- *  can be either enabled or disabled
- * - available to be installed from a Fleet package rules
+ * Rule activity (status) filter, which now can filter enabled and disabled rules.
+ * Later we're going to support available rules as well (prebuilt rules that are not yet installed).
  */
 export enum CoverageOverviewRuleActivity {
   /**
-   * Enabled rules (installed from a Fleet package, custom or customized)
+   * Enabled rules (prebuilt and custom)
    */
   Enabled = 'enabled',
   /**
-   * Disabled rules (installed from a Fleet package, custom or customized)
+   * Disabled rules (prebuilt and custom)
    */
   Disabled = 'disabled',
-  /**
-   * Available to be installed from a Fleet package rules (Elastic prebuilt rules)
-   */
-  Available = 'available',
 }
 export const CoverageOverviewRuleActivitySchema = enumeration(
   'CoverageOverviewRuleActivity',
@@ -45,10 +39,6 @@ export enum CoverageOverviewRuleSource {
    * Rules created manually
    */
   Custom = 'custom',
-  /**
-   * Rules installed from a Fleet package but modified later on
-   */
-  Customized = 'customized',
 }
 export const CoverageOverviewRuleSourceSchema = enumeration(
   'CoverageOverviewRuleSource',

diff --git a/...engine/rule_management/logic/coverage_overview/build_coverage_overview_dashboard_model.ts b/...engine/rule_management/logic/coverage_overview/build_coverage_overview_dashboard_model.ts
@@ -115,10 +115,13 @@ function addRule(
       id: ruleId,
       name: ruleData.name,
     });
-  } else if (ruleData.activity === CoverageOverviewRuleActivity.Available) {
-    container.availableRules.push({
-      id: ruleId,
-      name: ruleData.name,
-    });
   }
+
+  // When we add support for available (not installed) rules to this feature, add the following here:
+  // else if (ruleData.activity === CoverageOverviewRuleActivity.Available) {
+  //   container.availableRules.push({
+  //     id: ruleId,
+  //     name: ruleData.name,
+  //   });
+  // }
 }
diff --git a/...on_engine/rule_management/api/rules/coverage_overview/handle_coverage_overview_request.ts b/...on_engine/rule_management/api/rules/coverage_overview/handle_coverage_overview_request.ts
@@ -39,13 +39,7 @@ export async function handleCoverageOverviewRequest({
         filter: filter.search_term,
         showCustomRules: filter.source?.includes(CoverageOverviewRuleSource.Custom) ?? false,
         showElasticRules: filter.source?.includes(CoverageOverviewRuleSource.Prebuilt) ?? false,
-        enabled:
-          (activitySet.has(CoverageOverviewRuleActivity.Enabled) &&
-            activitySet.has(CoverageOverviewRuleActivity.Disabled)) ||
-          (!activitySet.has(CoverageOverviewRuleActivity.Enabled) &&
-            !activitySet.has(CoverageOverviewRuleActivity.Disabled))
-            ? undefined
-            : activitySet.has(CoverageOverviewRuleActivity.Enabled),
+        enabled: getIsEnabledFilter(activitySet),
       })
     : undefined;
 
@@ -69,6 +63,47 @@ export async function handleCoverageOverviewRequest({
   } as CoverageOverviewResponse);
 }
 
+function getIsEnabledFilter(activitySet: Set<CoverageOverviewRuleActivity>): boolean | undefined {
+  const bothSpecified =
+    activitySet.has(CoverageOverviewRuleActivity.Enabled) &&
+    activitySet.has(CoverageOverviewRuleActivity.Disabled);
+  const noneSpecified =
+    !activitySet.has(CoverageOverviewRuleActivity.Enabled) &&
+    !activitySet.has(CoverageOverviewRuleActivity.Disabled);
+
+  return bothSpecified || noneSpecified
+    ? undefined
+    : activitySet.has(CoverageOverviewRuleActivity.Enabled);
+}
+
+function appendRuleToResponse(
+  response: CoverageOverviewResponse,
+  rule: SanitizedRule<CoverageOverviewRuleParams>
+): CoverageOverviewResponse {
+  const categories = extractRuleMitreCategories(rule);
+
+  for (const category of categories) {
+    if (!response.coverage[category]) {
+      response.coverage[category] = [rule.id];
+    } else {
+      response.coverage[category].push(rule.id);
+    }
+  }
+
+  if (categories.length === 0) {
+    response.unmapped_rule_ids.push(rule.id);
+  }
+
+  response.rules_data[rule.id] = {
+    name: rule.name,
+    activity: rule.enabled
+      ? CoverageOverviewRuleActivity.Enabled
+      : CoverageOverviewRuleActivity.Disabled,
+  };
+
+  return response;
+}
+
 /**
  * Extracts rule's MITRE ATT&CK™ tactics, techniques and subtechniques
  *
@@ -101,31 +136,3 @@ function extractRuleMitreCategories(rule: SanitizedRule<CoverageOverviewRulePara
 
   return Array.from(categories);
 }
-
-function appendRuleToResponse(
-  response: CoverageOverviewResponse,
-  rule: SanitizedRule<CoverageOverviewRuleParams>
-): CoverageOverviewResponse {
-  const categories = extractRuleMitreCategories(rule);
-
-  for (const category of categories) {
-    if (!response.coverage[category]) {
-      response.coverage[category] = [rule.id];
-    } else {
-      response.coverage[category].push(rule.id);
-    }
-  }
-
-  if (categories.length === 0) {
-    response.unmapped_rule_ids.push(rule.id);
-  }
-
-  response.rules_data[rule.id] = {
-    name: rule.name,
-    activity: rule.enabled
-      ? CoverageOverviewRuleActivity.Enabled
-      : CoverageOverviewRuleActivity.Disabled,
-  };
-
-  return response;
-}
diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/coverage_overview.ts b/x-pack/test/detection_engine_api_integration/basic/tests/coverage_overview.ts
@@ -342,9 +342,9 @@ export default ({ getService }: FtrProviderContext): void => {
           });
         });
 
-        it('returns response filtered by enabled and disabled rules equal to response if enabled and disabled are not set', async () => {
+        it('returns all rules if both enabled and disabled filters are specified in the request', async () => {
           const expectedRule1 = await createRule(supertest, log, {
-            ...getSimpleRule('rule-1'),
+            ...getSimpleRule('rule-1', false),
             name: 'Disabled rule',
             threat: generateThreatArray(1),
           });