[8.15] [Security Solution] Fix prebuilt rule duplication logic to cop…

…y related integrations and required fields from the original rule (#191065) (#191493) # Backport This will backport the following commits from `main` to `8.15`: - [[Security Solution] Fix prebuilt rule duplication logic to copy related integrations and required fields from the original rule (#191065)](#191065)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)
elastic · Aug 27, 2024 · ed44e9a · ed44e9a
1 parent 267656f
commit ed44e9a
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 160 deletions.
diff --git a/...solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts b/...solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts
@@ -7,6 +7,7 @@
 
 import { v4 as uuidv4 } from 'uuid';
 import type { SanitizedRule } from '@kbn/alerting-plugin/common';
+
 import type { RuleParams } from '../../../rule_schema';
 import { duplicateRule } from './duplicate_rule';
 
@@ -35,13 +36,25 @@ describe('duplicateRule', () => {
       meta: undefined,
       maxSignals: 100,
       responseActions: [],
-      relatedIntegrations: [],
-      requiredFields: [],
+      relatedIntegrations: [
+        {
+          package: 'aws',
+          version: '~1.2.3',
+          integration: 'route53',
+        },
+      ],
+      requiredFields: [
+        {
+          name: 'event.action',
+          type: 'keyword',
+          ecs: true,
+        },
+      ],
       riskScore: 42,
       riskScoreMapping: [],
       severity: 'low',
       severityMapping: [],
-      setup: 'Some setup guide.',
+      setup: `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`,
       threat: [],
       to: 'now',
       references: [],
@@ -93,151 +106,64 @@ describe('duplicateRule', () => {
     jest.clearAllMocks();
   });
 
-  it('returns an object with fields copied from a given rule', async () => {
-    const rule = createTestRule();
-    const result = await duplicateRule({
-      rule,
-    });
-
-    expect(result).toEqual({
-      name: expect.anything(), // covered in a separate test
-      params: {
-        ...rule.params,
-        ruleSource: {
-          type: 'internal',
-        },
-        ruleId: expect.anything(), // covered in a separate test
-      },
-      tags: rule.tags,
-      alertTypeId: rule.alertTypeId,
-      consumer: rule.consumer,
-      schedule: rule.schedule,
-      actions: rule.actions,
-      enabled: false, // covered in a separate test
-    });
-  });
-
-  it('appends [Duplicate] to the name', async () => {
-    const rule = createTestRule();
-    rule.name = 'PowerShell Keylogging Script';
-    const result = await duplicateRule({
-      rule,
-    });
-
-    expect(result).toEqual(
-      expect.objectContaining({
-        name: 'PowerShell Keylogging Script [Duplicate]',
-      })
-    );
-  });
-
-  it('generates a new ruleId', async () => {
-    const rule = createTestRule();
-    const result = await duplicateRule({
-      rule,
-    });
-
-    expect(result).toEqual(
-      expect.objectContaining({
-        params: expect.objectContaining({
-          ruleId: 'new ruleId',
-        }),
-      })
-    );
-  });
-
-  it('makes sure the duplicated rule is disabled', async () => {
-    const rule = createTestRule();
-    rule.enabled = true;
-    const result = await duplicateRule({
-      rule,
-    });
-
-    expect(result).toEqual(
-      expect.objectContaining({
-        enabled: false,
-      })
-    );
-  });
-
-  describe('when duplicating a prebuilt (immutable) rule', () => {
-    const createPrebuiltRule = () => {
+  describe('when duplicating any kind of rule', () => {
+    it('appends [Duplicate] to the name', async () => {
       const rule = createTestRule();
-      rule.params.immutable = true;
-      return rule;
-    };
-
-    it('transforms it to a custom (mutable) rule', async () => {
-      const rule = createPrebuiltRule();
+      rule.name = 'PowerShell Keylogging Script';
       const result = await duplicateRule({
         rule,
       });
 
       expect(result).toEqual(
         expect.objectContaining({
-          params: expect.objectContaining({
-            immutable: false,
-          }),
+          name: 'PowerShell Keylogging Script [Duplicate]',
         })
       );
     });
 
-    it('resets related integrations to an empty array', async () => {
-      const rule = createPrebuiltRule();
-      rule.params.relatedIntegrations = [
-        {
-          package: 'aws',
-          version: '~1.2.3',
-          integration: 'route53',
-        },
-      ];
-
+    it('generates a new ruleId', async () => {
+      const rule = createTestRule();
       const result = await duplicateRule({
         rule,
       });
 
       expect(result).toEqual(
         expect.objectContaining({
           params: expect.objectContaining({
-            relatedIntegrations: [],
+            ruleId: 'new ruleId',
           }),
         })
       );
     });
 
-    it('resets required fields to an empty array', async () => {
-      const rule = createPrebuiltRule();
-      rule.params.requiredFields = [
-        {
-          name: 'event.action',
-          type: 'keyword',
-          ecs: true,
-        },
-      ];
-
+    it('makes sure the duplicated rule is disabled', async () => {
+      const rule = createTestRule();
+      rule.enabled = true;
       const result = await duplicateRule({
         rule,
       });
 
       expect(result).toEqual(
         expect.objectContaining({
-          params: expect.objectContaining({
-            requiredFields: [],
-          }),
+          enabled: false,
         })
       );
     });
   });
 
-  describe('when duplicating a custom (mutable) rule', () => {
-    const createCustomRule = () => {
+  describe('when duplicating a prebuilt rule', () => {
+    const createPrebuiltRule = () => {
       const rule = createTestRule();
-      rule.params.immutable = false;
+      rule.params.immutable = true;
+      rule.params.ruleSource = {
+        type: 'external',
+        isCustomized: false,
+      };
       return rule;
     };
 
-    it('keeps it custom', async () => {
-      const rule = createCustomRule();
+    it('transforms it to a custom rule', async () => {
+      const rule = createPrebuiltRule();
       const result = await duplicateRule({
         rule,
       });
@@ -246,71 +172,85 @@ describe('duplicateRule', () => {
         expect.objectContaining({
           params: expect.objectContaining({
             immutable: false,
+            ruleSource: {
+              type: 'internal',
+            },
           }),
         })
       );
     });
 
-    it('copies related integrations as is', async () => {
-      const rule = createCustomRule();
-      rule.params.relatedIntegrations = [
-        {
-          package: 'aws',
-          version: '~1.2.3',
-          integration: 'route53',
-        },
-      ];
-
+    it('copies fields from the original rule', async () => {
+      const rule = createPrebuiltRule();
       const result = await duplicateRule({
         rule,
       });
 
-      expect(result).toEqual(
-        expect.objectContaining({
-          params: expect.objectContaining({
-            relatedIntegrations: rule.params.relatedIntegrations,
-          }),
-        })
-      );
+      expect(result).toEqual({
+        name: expect.anything(), // covered in a separate test
+        params: {
+          ...rule.params,
+          ruleId: expect.anything(), // covered in a separate test
+          immutable: expect.anything(), // covered in a separate test
+          ruleSource: expect.anything(), // covered in a separate test
+        },
+        tags: rule.tags,
+        alertTypeId: rule.alertTypeId,
+        consumer: rule.consumer,
+        schedule: rule.schedule,
+        actions: rule.actions,
+        enabled: false, // covered in a separate test
+      });
     });
+  });
 
-    it('copies required fields as is', async () => {
-      const rule = createCustomRule();
-      rule.params.requiredFields = [
-        {
-          name: 'event.action',
-          type: 'keyword',
-          ecs: true,
-        },
-      ];
+  describe('when duplicating a custom rule', () => {
+    const createCustomRule = () => {
+      const rule = createTestRule();
+      rule.params.immutable = false;
+      rule.params.ruleSource = {
+        type: 'internal',
+      };
+      return rule;
+    };
 
+    it('keeps it custom', async () => {
+      const rule = createCustomRule();
       const result = await duplicateRule({
         rule,
       });
 
       expect(result).toEqual(
         expect.objectContaining({
           params: expect.objectContaining({
-            requiredFields: rule.params.requiredFields,
+            immutable: false,
+            ruleSource: {
+              type: 'internal',
+            },
           }),
         })
       );
     });
 
-    it('copies setup guide as is', async () => {
+    it('copies fields from the original rule', async () => {
       const rule = createCustomRule();
-      rule.params.setup = `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`;
       const result = await duplicateRule({
         rule,
       });
 
-      expect(result).toEqual(
-        expect.objectContaining({
-          params: expect.objectContaining({
-            setup: rule.params.setup,
-          }),
-        })
-      );
+      expect(result).toEqual({
+        name: expect.anything(), // covered in a separate test
+        params: {
+          ...rule.params,
+          ruleId: expect.anything(), // covered in a separate test
+        },
+        tags: rule.tags,
+        alertTypeId: rule.alertTypeId,
+        consumer: rule.consumer,
+        schedule: rule.schedule,
+        actions: rule.actions,
+        enabled: false, // covered in a separate test
+      });
     });
   });
 });
diff --git a/...rity_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/...rity_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts
@@ -12,7 +12,6 @@ import type { SanitizedRule } from '@kbn/alerting-plugin/common';
 import { SERVER_APP_ID } from '../../../../../../common/constants';
 import type { InternalRuleCreate, RuleParams } from '../../../rule_schema';
 import { transformToActionFrequency } from '../../normalization/rule_actions';
-import { convertImmutableToRuleSource } from '../../normalization/rule_converters';
 
 const DUPLICATE_TITLE = i18n.translate(
   'xpack.securitySolution.detectionEngine.rules.cloneRule.duplicateTitle',
@@ -27,17 +26,18 @@ interface DuplicateRuleParams {
 
 export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise<InternalRuleCreate> => {
   // Generate a new static ruleId
-  const ruleId = uuidv4();
-
-  // If it's a prebuilt rule, reset Related Integrations, Required Fields and Setup Guide.
-  // We do this because for now we don't allow the users to edit these fields for custom rules.
-  const isPrebuilt = rule.params.immutable;
-  const relatedIntegrations = isPrebuilt ? [] : rule.params.relatedIntegrations;
-  const requiredFields = isPrebuilt ? [] : rule.params.requiredFields;
-  const actions = transformToActionFrequency(rule.actions, rule.throttle);
+  const ruleId: InternalRuleCreate['params']['ruleId'] = uuidv4();
 
   // Duplicated rules are always considered custom rules
-  const immutable = false;
+  const immutable: InternalRuleCreate['params']['immutable'] = false;
+  const ruleSource: InternalRuleCreate['params']['ruleSource'] = {
+    type: 'internal',
+  };
+
+  const actions: InternalRuleCreate['actions'] = transformToActionFrequency(
+    rule.actions,
+    rule.throttle
+  );
 
   return {
     name: `${rule.name} [${DUPLICATE_TITLE}]`,
@@ -46,11 +46,9 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise<Inte
     consumer: SERVER_APP_ID,
     params: {
       ...rule.params,
-      immutable,
-      ruleSource: convertImmutableToRuleSource(immutable),
       ruleId,
-      relatedIntegrations,
-      requiredFields,
+      immutable,
+      ruleSource,
       exceptionsList: [],
     },
     schedule: rule.schedule,