[Security Solution] Fix losing data upon prebuilt rule upgrade to a n…

…ew version in which the rule's type is different (elastic#176421)

**Fixes:** elastic#169480

## Summary

This PR fixes losing the following rule data upon prebuilt rule upgrade to a new version in which the rule's type is different

- Saved Object id
- exceptions list (default and shared)
- Timeline id
- Timeline title

## Details

The problem occurs when user upgrades a prebuilt rule to a newer version which has a different rule type.

Checking the code it's not so hard to find [`upgradeRule()`](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.ts#L49) function which performs prebuilt rule upgrade. It has the following comment

> If we're trying to change the type of a prepackaged rule, we need to delete the old one and replace it with the new rule, keeping the enabled setting, actions, throttle, id, and exception lists from the old rule.

Looking below in the code it's clear that only enabled state and actions get restored upon rule upgrade. Missing to restore `exceptions lists` leads to disappearing exceptions upon rule upgrade.

On top of this `execution results` and `execution events` also get lost due to missing to restore saved object `id`. Execution log isn't gone anywhere but can't be bound to a new id. Direct links to rule details page won't work neither after upgrade.

This PR fixes the problem by restoring rule bound data after upgrade.

FTR tests were restructured to accommodate extra tests to cover this bug fix. 

### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

x-pack/plugins/security_solution/docs/testing/test_plans/detection_response/prebuilt_rules/installation_and_upgrade.md

@@ -13,6 +13,7 @@ Status: `in progress`. The current test plan matches `Milestone 2` of the [Rule
   - [Non-functional requirements](#non-functional-requirements)
   - [Functional requirements](#functional-requirements)
 - [Scenarios](#scenarios)
+
   - [Package installation](#package-installation)
     - [**Scenario: Package is installed via Fleet**](#scenario-package-is-installed-via-fleet)
     - [**Scenario: Package is installed via bundled Fleet package in Kibana**](#scenario-package-is-installed-via-bundled-fleet-package-in-kibana)
@@ -63,6 +64,9 @@ Status: `in progress`. The current test plan matches `Milestone 2` of the [Rule
     - [**Scenario: Properties with semantically equal values should not be shown as modified**](#scenario-properties-with-semantically-equal-values-should-not-be-shown-as-modified)
     - [**Scenario: Unchanged sections of a rule should be hidden by default**](#scenario-unchanged-sections-of-a-rule-should-be-hidden-by-default)
     - [**Scenario: Properties should be sorted alphabetically**](#scenario-properties-should-be-sorted-alphabetically)
+  - [Rule upgrade workflow: preserving rule bound data](#rule-upgrade-workflow-preserving-rule-bound-data)
+    - [**Scenario: Rule bound data is preserved after upgrading a rule to a newer version with the same rule type**](#scenario-rule-bound-data-is-preserved-after-upgrading-a-rule-to-a-newer-version-with-the-same-rule-type)
+    - [**Scenario: Rule bound data is preserved after upgrading a rule to a newer version with a different rule type**](#scenario-rule-bound-data-is-preserved-after-upgrading-a-rule-to-a-newer-version-with-a-different-rule-type)
   - [Rule upgrade workflow: misc cases](#rule-upgrade-workflow-misc-cases)
     - [**Scenario: User doesn't see the Rule Updates tab until the package installation is completed**](#scenario-user-doesnt-see-the-rule-updates-tab-until-the-package-installation-is-completed)
   - [Error handling](#error-handling)
@@ -949,6 +953,36 @@ When a user expands all hidden sections
 Then all properties of the rule should be sorted alphabetically
 ```
 
+### Rule upgrade workflow: preserving rule bound data
+
+#### **Scenario: Rule bound data is preserved after upgrading a rule to a newer version with the same rule type**
+
+**Automation**: 1 unit test per case, 1 integration test
+
+```Gherkin
+Given a prebuilt rule is installed in Kibana
+And this rule has an update available
+And the update has the same rule type
+When a user upgrades the rule
+Then the rule bound data should be preserved
+```
+
+Examples: generated alerts, exception lists (rule exception list, shared exception list, endpoint exception list), timeline reference, actions, enabled state, execution results and execution events.
+
+#### **Scenario: Rule bound data is preserved after upgrading a rule to a newer version with a different rule type**
+
+**Automation**: 1 unit test per case, 1 integration test
+
+```Gherkin
+Given a prebuilt rule is installed in Kibana
+And this rule has an update available
+And the update has a different rule type
+When a user upgrades the rule
+Then the rule bound data should be preserved
+```
+
+Examples: generated alerts, exception lists (rule exception list, shared exception list, endpoint exception list), timeline reference, actions, enabled state, execution results and execution events.
+
 ### Rule upgrade workflow: misc cases
 
 #### **Scenario: User doesn't see the Rule Updates tab until the package installation is completed**

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.test.ts

@@ -5,17 +5,23 @@
  * 2.0.
  */
 
+import { omit } from 'lodash';
 import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks';
 import {
   getRuleMock,
   getFindResultWithSingleHit,
+  getFindResultWithMultiHits,
 } from '../../../routes/__mocks__/request_responses';
 import { upgradePrebuiltRules } from './upgrade_prebuilt_rules';
 import { patchRules } from '../../../rule_management/logic/crud/patch_rules';
+import { createRules } from '../../../rule_management/logic/crud/create_rules';
+import { deleteRules } from '../../../rule_management/logic/crud/delete_rules';
 import { getPrebuiltRuleMock, getPrebuiltThreatMatchRuleMock } from '../../mocks';
-import { getThreatRuleParams } from '../../../rule_schema/mocks';
+import { getQueryRuleParams, getThreatRuleParams } from '../../../rule_schema/mocks';
 
 jest.mock('../../../rule_management/logic/crud/patch_rules');
+jest.mock('../../../rule_management/logic/crud/create_rules');
+jest.mock('../../../rule_management/logic/crud/delete_rules');
 
 describe('updatePrebuiltRules', () => {
   let rulesClient: ReturnType<typeof rulesClientMock.create>;
@@ -24,35 +30,173 @@ describe('updatePrebuiltRules', () => {
     rulesClient = rulesClientMock.create();
   });
 
-  it('should omit actions and enabled when calling patchRules', async () => {
+  describe('when upgrading a prebuilt rule to a newer version with the same rule type', () => {
+    const prepackagedRule = getPrebuiltRuleMock({
+      rule_id: 'rule-to-upgrade',
+    });
+
+    beforeEach(() => {
+      const installedRule = getRuleMock(
+        getQueryRuleParams({
+          ruleId: 'rule-to-upgrade',
+        })
+      );
+
+      rulesClient.find.mockResolvedValue(
+        getFindResultWithMultiHits({
+          data: [installedRule],
+        })
+      );
+    });
+
+    it('patches existing rule with incoming version data', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(patchRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          nextParams: expect.objectContaining(prepackagedRule),
+        })
+      );
+    });
+
+    it('makes sure enabled state is preserved', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(patchRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          nextParams: expect.objectContaining({
+            enabled: undefined,
+          }),
+        })
+      );
+    });
+  });
+
+  describe('when upgrading a prebuilt rule to a newer version with a different rule type', () => {
+    const prepackagedRule = getPrebuiltRuleMock({
+      rule_id: 'rule-to-upgrade',
+      type: 'eql',
+      language: 'eql',
+      query: 'host where host.name == "something"',
+    });
     const actions = [
       {
         group: 'group',
         id: 'id',
-        action_type_id: 'action_type_id',
+        actionTypeId: 'action_type_id',
         params: {},
       },
     ];
-    const prepackagedRule = getPrebuiltRuleMock();
-    rulesClient.find.mockResolvedValue(getFindResultWithSingleHit());
+    const installedRule = getRuleMock(
+      getQueryRuleParams({
+        ruleId: 'rule-to-upgrade',
+        type: 'query',
+        exceptionsList: [
+          {
+            id: 'exception_list_1',
+            list_id: 'exception_list_1',
+            namespace_type: 'agnostic',
+            type: 'rule_default',
+          },
+        ],
+        timelineId: 'some-timeline-id',
+        timelineTitle: 'Some timeline title',
+      }),
+      {
+        id: 'installed-rule-so-id',
+        actions,
+      }
+    );
 
-    await upgradePrebuiltRules(rulesClient, [{ ...prepackagedRule, actions }]);
+    beforeEach(() => {
+      rulesClient.find.mockResolvedValue(
+        getFindResultWithMultiHits({
+          data: [installedRule],
+        })
+      );
+    });
 
-    expect(patchRules).toHaveBeenCalledWith(
-      expect.objectContaining({
-        nextParams: expect.objectContaining({
-          actions: undefined,
-        }),
-      })
-    );
+    it('deletes rule before creation', async () => {
+      let lastCalled!: string;
 
-    expect(patchRules).toHaveBeenCalledWith(
-      expect.objectContaining({
-        nextParams: expect.objectContaining({
-          enabled: undefined,
-        }),
-      })
-    );
+      (deleteRules as jest.Mock).mockImplementation(() => (lastCalled = 'deleteRules'));
+      (createRules as jest.Mock).mockImplementation(() => (lastCalled = 'createRules'));
+
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(deleteRules).toHaveBeenCalledTimes(1);
+      expect(createRules).toHaveBeenCalledTimes(1);
+      expect(lastCalled).toBe('createRules');
+    });
+
+    it('recreates a rule with incoming version data', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          immutable: true,
+          params: expect.objectContaining(prepackagedRule),
+        })
+      );
+    });
+
+    it('restores saved object id', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          id: 'installed-rule-so-id',
+        })
+      );
+    });
+
+    it('restores enabled state', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({ enabled: installedRule.enabled }),
+        })
+      );
+    });
+
+    it('restores actions', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({
+            actions: actions.map((a) => ({
+              ...omit(a, 'actionTypeId'),
+              action_type_id: a.actionTypeId,
+            })),
+          }),
+        })
+      );
+    });
+
+    it('restores exceptions list', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({ exceptions_list: installedRule.params.exceptionsList }),
+        })
+      );
+    });
+
+    it('restores timeline reference', async () => {
+      await upgradePrebuiltRules(rulesClient, [prepackagedRule]);
+
+      expect(createRules).toHaveBeenCalledWith(
+        expect.objectContaining({
+          params: expect.objectContaining({
+            timeline_id: installedRule.params.timelineId,
+            timeline_title: installedRule.params.timelineTitle,
+          }),
+        })
+      );
+    });
   });
 
   it('should update threat match rules', async () => {

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/logic/rule_objects/upgrade_prebuilt_rules.ts

@@ -72,36 +72,39 @@ const upgradeRule = async (
     return createRules({
       rulesClient,
       immutable: true,
+      id: existingRule.id,
       params: {
         ...rule,
         // Force the prepackaged rule to use the enabled state from the existing rule,
         // regardless of what the prepackaged rule says
         enabled: existingRule.enabled,
+        exceptions_list: existingRule.params.exceptionsList,
         actions: existingRule.actions.map(transformAlertToRuleAction),
+        timeline_id: existingRule.params.timelineId,
+        timeline_title: existingRule.params.timelineTitle,
       },
     });
-  } else {
-    await patchRules({
-      rulesClient,
-      existingRule,
-      nextParams: {
-        ...rule,
-        // Force enabled to use the enabled state from the existing rule by passing in undefined to patchRules
-        enabled: undefined,
-        actions: undefined,
-      },
-    });
+  }
 
-    const updatedRule = await readRules({
-      rulesClient,
-      ruleId: rule.rule_id,
-      id: undefined,
-    });
+  await patchRules({
+    rulesClient,
+    existingRule,
+    nextParams: {
+      ...rule,
+      // Force enabled to use the enabled state from the existing rule by passing in undefined to patchRules
+      enabled: undefined,
+    },
+  });
 
-    if (!updatedRule) {
-      throw new PrepackagedRulesError(`Rule ${rule.rule_id} not found after upgrade`, 500);
-    }
+  const updatedRule = await readRules({
+    rulesClient,
+    ruleId: rule.rule_id,
+    id: undefined,
+  });
 
-    return updatedRule;
+  if (!updatedRule) {
+    throw new PrepackagedRulesError(`Rule ${rule.rule_id} not found after upgrade`, 500);
   }
+
+  return updatedRule;
 };

x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.mock.ts

@@ -7,17 +7,19 @@
 
 import type { PrebuiltRuleAsset } from './prebuilt_rule_asset';
 
-export const getPrebuiltRuleMock = (): PrebuiltRuleAsset => ({
-  description: 'some description',
-  name: 'Query with a rule id',
-  query: 'user.name: root or user.name: admin',
-  severity: 'high',
-  type: 'query',
-  risk_score: 55,
-  language: 'kuery',
-  rule_id: 'rule-1',
-  version: 1,
-});
+export const getPrebuiltRuleMock = (rewrites?: Partial<PrebuiltRuleAsset>): PrebuiltRuleAsset =>
+  ({
+    description: 'some description',
+    name: 'Query with a rule id',
+    query: 'user.name: root or user.name: admin',
+    severity: 'high',
+    type: 'query',
+    risk_score: 55,
+    language: 'kuery',
+    rule_id: 'rule-1',
+    version: 1,
+    ...rewrites,
+  } as PrebuiltRuleAsset);
 
 export const getPrebuiltRuleWithExceptionsMock = (): PrebuiltRuleAsset => ({
   description: 'A rule with an exception list',

x-pack/test/security_solution_api_integration/config/ess/config.base.ts

@@ -8,6 +8,7 @@
 import { CA_CERT_PATH } from '@kbn/dev-utils';
 import { FtrConfigProviderContext, kbnTestConfig, kibanaTestUser } from '@kbn/test';
 import { services } from '../../../api_integration/services';
+import { PRECONFIGURED_ACTION_CONNECTORS } from '../shared';
 
 interface CreateTestConfigOptions {
   license: string;
@@ -85,20 +86,7 @@ export function createTestConfig(options: CreateTestConfigOptions, testFiles?: s
             'alertSuppressionForIndicatorMatchRuleEnabled',
           ])}`,
           '--xpack.task_manager.poll_interval=1000',
-          `--xpack.actions.preconfigured=${JSON.stringify({
-            'my-test-email': {
-              actionTypeId: '.email',
-              name: 'TestEmail#xyz',
-              config: {
-                from: '[email protected]',
-                service: '__json',
-              },
-              secrets: {
-                user: 'user',
-                password: 'password',
-              },
-            },
-          })}`,
+          `--xpack.actions.preconfigured=${JSON.stringify(PRECONFIGURED_ACTION_CONNECTORS)}`,
           ...(ssl
             ? [
                 `--elasticsearch.hosts=${servers.elasticsearch.protocol}://${servers.elasticsearch.hostname}:${servers.elasticsearch.port}`,