[Security Solutions] Fixes deleting of alerts to also delete legacy n…

…otifications by migrating them before the deletion (#122610) (#122904)

## Summary

Bug: #122456, #113055

Previously when we brought back the legacy notifications we did not migrate them on delete so the legacy notifications end up becoming dangling alerts that run forever with warns in your log files like so:

```sh
[2022-01-06T10:09:16.593-07:00][ERROR][plugins.alerting] Executing Rule frank-test:siem.notifications:549ac8c0-6f11-11ec-b6be-193e9dcb2837 has resulted in Error: Saved object [alert/36fe8520-6e7d-11ec-83a2-150f18947658] not found
```

This fixes the bug by migrating the actions first before deleting them in the routes of:
* `delete_rules_route.ts`
* `delete_rules_bulk_route.ts`

 That allows the action to be returned in the response body on successful delete and also deletes the legacy notification. I could not find a good way to turn off the legacy notification from within its self so for now I updated the logging message to include instructions on how to remove legacy notifications that are dangling or to upgrade/update from legacy to non-legacy in the log messages.

Also, this changes the line of code for deleting legacy actions from:

```ts
 rulesClient.find({
      options: {
        hasReference: { <-- This currently will delete the first found rule that happens to have a reference of the `rule.id` That should be only legacy notifications today but tomorrow or later could be a different rule by accident.
          type: 'alert',
          id: rule.id,
        },
      },
    }),
```

to

```ts
 rulesClient.find({
      options: {
        filter: 'alert.attributes.alertTypeId:(siem.notifications)', <--- This filter should ensure we only delete `siem.notifications` as extra safety measure.
        hasReference: {
          type: 'alert',
          id: rule.id,
        },
      },
    }),
```

**Manual testing**
Either you can use the `internal` rule route endpoints or do a true upgrade test scenario. For these manual testing steps I am going to outline the developer fast way of adding a legacy rule and then deleting it from there.

Create a rule and activate it normally within security_solution:
<img width="1197" alt="create_rule" src="https://user-images.githubusercontent.com/1151048/148859970-e7587c2f-de7a-4f4c-9280-4acdaeac03dd.png">

Do not add actions to the rule at this point as we will first exercise the older legacy system. However, you want at least one action configured such as a slack notification:
<img width="575" alt="define_connector" src="https://user-images.githubusercontent.com/1151048/148859992-ad83aa8b-7c86-49ad-bae9-fcc291f6824a.png">

Within dev tools do a query for all your actions and grab one of the _id of them without their prefix:
```sh
GET .kibana/_search
{
  "query": {
    "term": {
      "type": "action"
    }
  }
}
```

Go to the file detection_engine/scripts/legacy_notifications/one_action.json and add this id to the file. Something like this:

Mine was `_id : action:879e8ff0-1be1-11ec-a722-83da1c22a481`, so I will be copying the ID of `879e8ff0-1be1-11ec-a722-83da1c22a481`:

```json
{
  "name": "Legacy notification with one action",
  "interval": "1m",  <--- You can use whatever you want. Real values are "1h", "1d", "1w". I use "1m" for testing purposes.
  "actions": [
    {
      "id": "879e8ff0-1be1-11ec-a722-83da1c22a481", <--- My action id
      "group": "default",
      "params": {
        "message": "Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts"
      },
      "actionTypeId": ".slack" <--- I am a slack action id type.
    }
  ]
}
```

Query for an alert you want to add manually add back a legacy notification to it. Such as:
```json
GET .kibana/_search
{
  "query": {
    "term": {
      "alert.alertTypeId": "siem.queryRule"
    }
  }
}
```

Grab the `_id` without the alert prefix. For mine this was `933ca720-1be1-11ec-a722-83da1c22a481`:

Within the directory of detection_engine/scripts execute the script:

```sh
./post_legacy_notification.sh 933ca720-1be1-11ec-a722-83da1c22a481
{
  "ok": "acknowledged"
}
```

which is going to do a few things. See the file detection_engine/routes/rules/legacy_create_legacy_notification.ts for the definition of the route and what it does in full, but we should notice that we have now have actions on the rule:

<img width="1050" alt="edit_rule" src="https://user-images.githubusercontent.com/1151048/148860221-4ee3b2b4-64a4-4e4e-b762-611b0e5db1d1.png">

If you query for legacy notifications you should see the one you have:

```sh
GET .kibana/_search
{
  "query": {
    "term": {
      "alert.alertTypeId": "siem.notifications"
    }
  }
}
```

If you query for saved objects associated such as `siem-detection-engine-rule-actions`:

```sh
GET .kibana/_search
{
  "query": {
    "term": {
      "type": {
        "value": "siem-detection-engine-rule-actions"
      }
    }
  }
}
```

You should also see 1. After you delete the rule, both should be gone if you repeat the query above now.

### 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

(cherry picked from commit 25d1c66)

x-pack/plugins/security_solution/server/lib/detection_engine/notifications/legacy_rules_notification_alert_type.test.ts

@@ -88,7 +88,7 @@ describe('legacyRules_notification_alert_type', () => {
       });
       await alert.executor(payload);
       expect(logger.error).toHaveBeenCalledWith(
-        `Security Solution notification (Legacy) saved object for alert ${payload.params.ruleAlertId} was not found`
+        `Security Solution notification (Legacy) saved object for alert ${payload.params.ruleAlertId} was not found with id: \"1111\". space id: \"\" This indicates a dangling (Legacy) notification alert. You should delete this rule through \"Kibana UI -> Stack Management -> Rules and Connectors\" to remove this error message.`
       );
     });
 

x-pack/plugins/security_solution/server/lib/detection_engine/notifications/legacy_rules_notification_alert_type.ts

@@ -51,29 +51,34 @@ export const legacyRulesNotificationAlertType = ({
   },
   minimumLicenseRequired: 'basic',
   isExportable: false,
-  async executor({ startedAt, previousStartedAt, alertId, services, params }) {
-    // TODO: Change this to be a link to documentation on how to migrate: https://github.com/elastic/kibana/issues/113055
-    logger.warn(
-      'Security Solution notification (Legacy) system detected still running. Please see documentation on how to migrate to the new notification system.'
-    );
+  async executor({ startedAt, previousStartedAt, alertId, services, params, spaceId }) {
     const ruleAlertSavedObject = await services.savedObjectsClient.get<AlertAttributes>(
       'alert',
       params.ruleAlertId
     );
 
     if (!ruleAlertSavedObject.attributes.params) {
       logger.error(
-        `Security Solution notification (Legacy) saved object for alert ${params.ruleAlertId} was not found`
+        [
+          `Security Solution notification (Legacy) saved object for alert ${params.ruleAlertId} was not found with`,
+          `id: "${alertId}".`,
+          `space id: "${spaceId}"`,
+          'This indicates a dangling (Legacy) notification alert.',
+          'You should delete this rule through "Kibana UI -> Stack Management -> Rules and Connectors" to remove this error message.',
+        ].join(' ')
       );
       return;
     }
+
     logger.warn(
       [
         'Security Solution notification (Legacy) system still active for alert with',
         `name: "${ruleAlertSavedObject.attributes.name}"`,
         `description: "${ruleAlertSavedObject.attributes.params.description}"`,
         `id: "${ruleAlertSavedObject.id}".`,
-        `Please see documentation on how to migrate to the new notification system.`,
+        `space id: "${spaceId}"`,
+        'Editing or updating this rule through "Kibana UI -> Security -> Alerts -> Manage Rules"',
+        'will auto-migrate the rule to the new notification system and remove this warning message.',
       ].join(' ')
     );
 

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts

@@ -25,6 +25,7 @@ import { transformValidateBulkError } from './validate';
 import { transformBulkError, buildSiemResponse, createBulkErrorObject } from '../utils';
 import { deleteRules } from '../../rules/delete_rules';
 import { readRules } from '../../rules/read_rules';
+import { legacyMigrate } from '../../rules/utils';
 
 type Config = RouteConfig<unknown, unknown, QueryRulesBulkSchemaDecoded, 'delete' | 'post'>;
 type Handler = RequestHandler<
@@ -60,6 +61,7 @@ export const deleteRulesBulkRoute = (
     }
 
     const ruleStatusClient = context.securitySolution.getExecutionLogClient();
+    const savedObjectsClient = context.core.savedObjects.client;
 
     const rules = await Promise.all(
       request.body.map(async (payloadRule) => {
@@ -76,22 +78,27 @@ export const deleteRulesBulkRoute = (
 
         try {
           const rule = await readRules({ rulesClient, id, ruleId, isRuleRegistryEnabled });
-          if (!rule) {
+          const migratedRule = await legacyMigrate({
+            rulesClient,
+            savedObjectsClient,
+            rule,
+          });
+          if (!migratedRule) {
             return getIdBulkError({ id, ruleId });
           }
 
           const ruleStatus = await ruleStatusClient.getCurrentStatus({
-            ruleId: rule.id,
+            ruleId: migratedRule.id,
             spaceId: context.securitySolution.getSpaceId(),
           });
           await deleteRules({
-            ruleId: rule.id,
+            ruleId: migratedRule.id,
             rulesClient,
             ruleStatusClient,
           });
           return transformValidateBulkError(
             idOrRuleIdOrUnknown,
-            rule,
+            migratedRule,
             ruleStatus,
             isRuleRegistryEnabled
           );

x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts

@@ -19,6 +19,7 @@ import { getIdError, transform } from './utils';
 import { buildSiemResponse } from '../utils';
 
 import { readRules } from '../../rules/read_rules';
+import { legacyMigrate } from '../../rules/utils';
 
 export const deleteRulesRoute = (
   router: SecuritySolutionPluginRouter,
@@ -47,14 +48,20 @@ export const deleteRulesRoute = (
         const { id, rule_id: ruleId } = request.query;
 
         const rulesClient = context.alerting?.getRulesClient();
-
+        const savedObjectsClient = context.core.savedObjects.client;
         if (!rulesClient) {
           return siemResponse.error({ statusCode: 404 });
         }
 
         const ruleStatusClient = context.securitySolution.getExecutionLogClient();
         const rule = await readRules({ isRuleRegistryEnabled, rulesClient, id, ruleId });
-        if (!rule) {
+        const migratedRule = await legacyMigrate({
+          rulesClient,
+          savedObjectsClient,
+          rule,
+        });
+
+        if (!migratedRule) {
           const error = getIdError({ id, ruleId });
           return siemResponse.error({
             body: error.message,
@@ -63,15 +70,16 @@ export const deleteRulesRoute = (
         }
 
         const currentStatus = await ruleStatusClient.getCurrentStatus({
-          ruleId: rule.id,
+          ruleId: migratedRule.id,
           spaceId: context.securitySolution.getSpaceId(),
         });
+
         await deleteRules({
-          ruleId: rule.id,
+          ruleId: migratedRule.id,
           rulesClient,
           ruleStatusClient,
         });
-        const transformed = transform(rule, currentStatus, isRuleRegistryEnabled);
+        const transformed = transform(migratedRule, currentStatus, isRuleRegistryEnabled);
         if (transformed == null) {
           return siemResponse.error({ statusCode: 500, body: 'failed to transform alert' });
         } else {

x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts

@@ -316,7 +316,7 @@ export const legacyMigrate = async ({
   }
   /**
    * On update / patch I'm going to take the actions as they are, better off taking rules client.find (siem.notification) result
-   * and putting that into the actions array of the rule, then set the rules onThrottle property, notifyWhen and throttle from null -> actualy value (1hr etc..)
+   * and putting that into the actions array of the rule, then set the rules onThrottle property, notifyWhen and throttle from null -> actual value (1hr etc..)
    * Then use the rules client to delete the siem.notification
    * Then with the legacy Rule Actions saved object type, just delete it.
    */
@@ -325,6 +325,7 @@ export const legacyMigrate = async ({
   const [siemNotification, legacyRuleActionsSO] = await Promise.all([
     rulesClient.find({
       options: {
+        filter: 'alert.attributes.alertTypeId:(siem.notifications)',
         hasReference: {
           type: 'alert',
           id: rule.id,

x-pack/test/detection_engine_api_integration/security_and_spaces/tests/delete_rules.ts

@@ -7,9 +7,11 @@
 
 import expect from '@kbn/expect';
 
+import { BASE_ALERTING_API_PATH } from '../../../../plugins/alerting/common';
 import { DETECTION_ENGINE_RULES_URL } from '../../../../plugins/security_solution/common/constants';
 import { FtrProviderContext } from '../../common/ftr_provider_context';
 import {
+  createLegacyRuleAction,
   createRule,
   createSignalsIndex,
   deleteAllAlerts,
@@ -18,6 +20,7 @@ import {
   getSimpleRuleOutput,
   getSimpleRuleOutputWithoutRuleId,
   getSimpleRuleWithoutRuleId,
+  getWebHookAction,
   removeServerGeneratedProperties,
   removeServerGeneratedPropertiesIncludingRuleId,
 } from '../../utils';
@@ -100,6 +103,119 @@ export default ({ getService }: FtrProviderContext): void => {
           status_code: 404,
         });
       });
+
+      /**
+       * @deprecated Once the legacy notification system is removed, remove this test too.
+       */
+      it('should have exactly 1 legacy action before a delete within alerting', async () => {
+        // create an action
+        const { body: hookAction } = await supertest
+          .post('/api/actions/action')
+          .set('kbn-xsrf', 'true')
+          .send(getWebHookAction())
+          .expect(200);
+
+        // create a rule without actions
+        const createRuleBody = await createRule(supertest, log, getSimpleRule('rule-1'));
+
+        // Add a legacy rule action to the body of the rule
+        await createLegacyRuleAction(supertest, createRuleBody.id, hookAction.id);
+
+        // Test to ensure that we have exactly 1 legacy action by querying the Alerting client REST API directly
+        // See: https://www.elastic.co/guide/en/kibana/current/find-rules-api.html
+        // Note: We specifically query for both the filter of type "siem.notifications" and the "has_reference" to keep it very specific
+        const { body: alertFind } = await supertest
+          .get(`${BASE_ALERTING_API_PATH}/rules/_find`)
+          .query({
+            page: 1,
+            per_page: 10,
+            filter: 'alert.attributes.alertTypeId:(siem.notifications)',
+            has_reference: JSON.stringify({ id: createRuleBody.id, type: 'alert' }),
+          })
+          .set('kbn-xsrf', 'true')
+          .send()
+          .expect(200);
+
+        // Expect that we have exactly 1 legacy rule before the deletion
+        expect(alertFind.total).to.eql(1);
+      });
+
+      /**
+       * @deprecated Once the legacy notification system is removed, remove this test too.
+       */
+      it('should return the legacy action in the response body when it deletes a rule that has one', async () => {
+        // create an action
+        const { body: hookAction } = await supertest
+          .post('/api/actions/action')
+          .set('kbn-xsrf', 'true')
+          .send(getWebHookAction())
+          .expect(200);
+
+        // create a rule without actions
+        const createRuleBody = await createRule(supertest, log, getSimpleRule('rule-1'));
+
+        // Add a legacy rule action to the body of the rule
+        await createLegacyRuleAction(supertest, createRuleBody.id, hookAction.id);
+
+        // delete the rule with the legacy action
+        const { body } = await supertest
+          .delete(`${DETECTION_ENGINE_RULES_URL}?id=${createRuleBody.id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // ensure the actions contains the response
+        expect(body.actions).to.eql([
+          {
+            id: hookAction.id,
+            action_type_id: hookAction.actionTypeId,
+            group: 'default',
+            params: {
+              message:
+                'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts',
+            },
+          },
+        ]);
+      });
+
+      /**
+       * @deprecated Once the legacy notification system is removed, remove this test too.
+       */
+      it('should delete a legacy action when it deletes a rule that has one', async () => {
+        // create an action
+        const { body: hookAction } = await supertest
+          .post('/api/actions/action')
+          .set('kbn-xsrf', 'true')
+          .send(getWebHookAction())
+          .expect(200);
+
+        // create a rule without actions
+        const createRuleBody = await createRule(supertest, log, getSimpleRule('rule-1'));
+
+        // Add a legacy rule action to the body of the rule
+        await createLegacyRuleAction(supertest, createRuleBody.id, hookAction.id);
+
+        await supertest
+          .delete(`${DETECTION_ENGINE_RULES_URL}?id=${createRuleBody.id}`)
+          .set('kbn-xsrf', 'true')
+          .expect(200);
+
+        // Test to ensure that we have exactly 0 legacy actions by querying the Alerting client REST API directly
+        // See: https://www.elastic.co/guide/en/kibana/current/find-rules-api.html
+        // Note: We specifically query for both the filter of type "siem.notifications" and the "has_reference" to keep it very specific
+        const { body: bodyAfterDelete } = await supertest
+          .get(`${BASE_ALERTING_API_PATH}/rules/_find`)
+          .query({
+            page: 1,
+            per_page: 10,
+            filter: 'alert.attributes.alertTypeId:(siem.notifications)',
+            has_reference: JSON.stringify({ id: createRuleBody.id, type: 'alert' }),
+          })
+          .set('kbn-xsrf', 'true')
+          .send();
+
+        // Expect that we have exactly 0 legacy rules after the deletion
+        expect(bodyAfterDelete.total).to.eql(0);
+      });
     });
   });
 };