[Security Solution] [Platform] Migrate legacy actions whenever user i…

…nteracts with the rule (elastic#115101) Migrate legacy actions whenever user interacts with the rule (elastic#115101) Co-authored-by: Kibana Machine <[email protected]>
kibanamachine · Oct 19, 2021 · 66f9ba6 · 66f9ba6
1 parent de80e69
commit 66f9ba6
Show file tree

Hide file tree

Showing 16 changed files with 183 additions and 13 deletions.
diff --git a/...security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts b/...security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.ts
@@ -171,6 +171,7 @@ export const createPrepackagedRules = async (
   );
   await updatePrepackagedRules(
     rulesClient,
+    savedObjectsClient,
     context.securitySolution.getSpaceId(),
     ruleStatusClient,
     rulesToUpdate,

diff --git a/.../plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/.../plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts
@@ -40,6 +40,7 @@ import {
 } from '../utils';
 
 import { patchRules } from '../../rules/patch_rules';
+import { legacyMigrate } from '../../rules/utils';
 import { getTupleDuplicateErrorsAndUniqueRules } from './utils';
 import { createRulesStreamFromNdJson } from '../../rules/create_rules_stream_from_ndjson';
 import { buildRouteValidation } from '../../../../utils/build_validation/route_validation';
@@ -271,8 +272,14 @@ export const importRulesRoute = (
                           status_code: 200,
                         });
                       } else if (rule != null && request.query.overwrite) {
+                        const migratedRule = await legacyMigrate({
+                          rulesClient,
+                          savedObjectsClient,
+                          rule,
+                        });
                         await patchRules({
                           rulesClient,
+                          savedObjectsClient,
                           author,
                           buildingBlockType,
                           spaceId: context.securitySolution.getSpaceId(),
@@ -291,7 +298,7 @@ export const importRulesRoute = (
                           timelineTitle,
                           meta,
                           filters,
-                          rule,
+                          rule: migratedRule,
                           index,
                           interval,
                           maxSignals,

diff --git a/...gins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts b/...gins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts
@@ -24,6 +24,7 @@ import { transformValidateBulkError } from './validate';
 import { patchRules } from '../../rules/patch_rules';
 import { readRules } from '../../rules/read_rules';
 import { PartialFilter } from '../../types';
+import { legacyMigrate } from '../../rules/utils';
 
 export const patchRulesBulkRoute = (
   router: SecuritySolutionPluginRouter,
@@ -133,9 +134,16 @@ export const patchRulesBulkRoute = (
               throwHttpError(await mlAuthz.validateRuleType(existingRule?.params.type));
             }
 
-            const rule = await patchRules({
+            const migratedRule = await legacyMigrate({
+              rulesClient,
+              savedObjectsClient,
               rule: existingRule,
+            });
+
+            const rule = await patchRules({
+              rule: migratedRule,
               rulesClient,
+              savedObjectsClient,
               author,
               buildingBlockType,
               description,

diff --git a/...k/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts b/...k/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts
@@ -24,6 +24,7 @@ import { buildSiemResponse } from '../utils';
 import { getIdError } from './utils';
 import { transformValidate } from './validate';
 import { readRules } from '../../rules/read_rules';
+import { legacyMigrate } from '../../rules/utils';
 import { PartialFilter } from '../../types';
 
 export const patchRulesRoute = (
@@ -134,8 +135,15 @@ export const patchRulesRoute = (
           throwHttpError(await mlAuthz.validateRuleType(existingRule?.params.type));
         }
 
+        const migratedRule = await legacyMigrate({
+          rulesClient,
+          savedObjectsClient,
+          rule: existingRule,
+        });
+
         const rule = await patchRules({
           rulesClient,
+          savedObjectsClient,
           author,
           buildingBlockType,
           description,
@@ -154,7 +162,7 @@ export const patchRulesRoute = (
           timelineTitle,
           meta,
           filters,
-          rule: existingRule,
+          rule: migratedRule,
           index,
           interval,
           maxSignals,

diff --git a/...ecurity_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.test.ts b/...ecurity_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.test.ts
@@ -41,6 +41,8 @@ describe.each([
       getAlertMock(isRuleRegistryEnabled, getQueryRuleParams())
     );
 
+    clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');
+
     updateRulesBulkRoute(server.router, ml, isRuleRegistryEnabled);
   });
 

diff --git a/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts b/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts
@@ -19,6 +19,8 @@ import { getIdBulkError } from './utils';
 import { transformValidateBulkError } from './validate';
 import { transformBulkError, buildSiemResponse, createBulkErrorObject } from '../utils';
 import { updateRules } from '../../rules/update_rules';
+import { legacyMigrate } from '../../rules/utils';
+import { readRules } from '../../rules/read_rules';
 
 export const updateRulesBulkRoute = (
   router: SecuritySolutionPluginRouter,
@@ -69,10 +71,24 @@ export const updateRulesBulkRoute = (
 
             throwHttpError(await mlAuthz.validateRuleType(payloadRule.type));
 
+            const existingRule = await readRules({
+              isRuleRegistryEnabled,
+              rulesClient,
+              ruleId: payloadRule.rule_id,
+              id: payloadRule.id,
+            });
+
+            await legacyMigrate({
+              rulesClient,
+              savedObjectsClient,
+              rule: existingRule,
+            });
+
             const rule = await updateRules({
               spaceId: context.securitySolution.getSpaceId(),
               rulesClient,
               ruleStatusClient,
+              savedObjectsClient,
               defaultOutputIndex: siemClient.getSignalsIndex(),
               ruleUpdate: payloadRule,
               isRuleRegistryEnabled,

diff --git a/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts b/...ins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts
@@ -44,7 +44,7 @@ describe.each([
       getAlertMock(isRuleRegistryEnabled, getQueryRuleParams())
     ); // successful update
     clients.ruleExecutionLogClient.find.mockResolvedValue([]); // successful transform: ;
-
+    clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');
     updateRulesRoute(server.router, ml, isRuleRegistryEnabled);
   });
 

diff --git a/.../plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts b/.../plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts
@@ -19,6 +19,8 @@ import { getIdError } from './utils';
 import { transformValidate } from './validate';
 import { updateRules } from '../../rules/update_rules';
 import { buildRouteValidation } from '../../../../utils/build_validation/route_validation';
+import { legacyMigrate } from '../../rules/utils';
+import { readRules } from '../../rules/read_rules';
 
 export const updateRulesRoute = (
   router: SecuritySolutionPluginRouter,
@@ -59,11 +61,25 @@ export const updateRulesRoute = (
         throwHttpError(await mlAuthz.validateRuleType(request.body.type));
 
         const ruleStatusClient = context.securitySolution.getExecutionLogClient();
+
+        const existingRule = await readRules({
+          isRuleRegistryEnabled,
+          rulesClient,
+          ruleId: request.body.rule_id,
+          id: request.body.id,
+        });
+
+        await legacyMigrate({
+          rulesClient,
+          savedObjectsClient,
+          rule: existingRule,
+        });
         const rule = await updateRules({
           defaultOutputIndex: siemClient.getSignalsIndex(),
           isRuleRegistryEnabled,
           rulesClient,
           ruleStatusClient,
+          savedObjectsClient,
           ruleUpdate: request.body,
           spaceId: context.securitySolution.getSpaceId(),
         });

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts
@@ -7,6 +7,7 @@
 
 import { PatchRulesOptions } from './types';
 import { rulesClientMock } from '../../../../../alerting/server/mocks';
+import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks';
 import { getAlertMock } from '../routes/__mocks__/request_responses';
 import { getMlRuleParams, getQueryRuleParams } from '../schemas/rule_schemas.mock';
 import { ruleExecutionLogClientMock } from '../rule_execution_log/__mocks__/rule_execution_log_client';
@@ -15,6 +16,7 @@ export const getPatchRulesOptionsMock = (isRuleRegistryEnabled: boolean): PatchR
   author: ['Elastic'],
   buildingBlockType: undefined,
   rulesClient: rulesClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   spaceId: 'default',
   ruleStatusClient: ruleExecutionLogClientMock.create(),
   anomalyThreshold: undefined,
@@ -68,6 +70,7 @@ export const getPatchMlRulesOptionsMock = (isRuleRegistryEnabled: boolean): Patc
   author: ['Elastic'],
   buildingBlockType: undefined,
   rulesClient: rulesClientMock.create(),
+  savedObjectsClient: savedObjectsClientMock.create(),
   spaceId: 'default',
   ruleStatusClient: ruleExecutionLogClientMock.create(),
   anomalyThreshold: 55,

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts
@@ -37,6 +37,7 @@ class PatchError extends Error {
 
 export const patchRules = async ({
   rulesClient,
+  savedObjectsClient,
   author,
   buildingBlockType,
   ruleStatusClient,
@@ -191,14 +192,14 @@ export const patchRules = async ({
 
   const newRule = {
     tags: addTags(tags ?? rule.tags, rule.params.ruleId, rule.params.immutable),
-    throttle: throttle !== undefined ? transformToAlertThrottle(throttle) : rule.throttle,
-    notifyWhen: throttle !== undefined ? transformToNotifyWhen(throttle) : rule.notifyWhen,
     name: calculateName({ updatedName: name, originalName: rule.name }),
     schedule: {
       interval: calculateInterval(interval, rule.schedule.interval),
     },
-    actions: actions?.map(transformRuleToAlertAction) ?? rule.actions,
     params: removeUndefined(nextParams),
+    actions: actions?.map(transformRuleToAlertAction) ?? rule.actions,
+    throttle: throttle !== undefined ? transformToAlertThrottle(throttle) : rule.throttle,
+    notifyWhen: throttle !== undefined ? transformToNotifyWhen(throttle) : rule.notifyWhen,
   };
 
   const [validated, errors] = validate(newRule, internalRuleUpdate);

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts
@@ -8,7 +8,12 @@
 import { get } from 'lodash/fp';
 import { Readable } from 'stream';
 
-import { SavedObject, SavedObjectAttributes, SavedObjectsFindResult } from 'kibana/server';
+import {
+  SavedObject,
+  SavedObjectAttributes,
+  SavedObjectsClientContract,
+  SavedObjectsFindResult,
+} from 'kibana/server';
 import type {
   MachineLearningJobIdOrUndefined,
   From,
@@ -271,12 +276,14 @@ export interface UpdateRulesOptions {
   rulesClient: RulesClient;
   defaultOutputIndex: string;
   ruleUpdate: UpdateRulesSchema;
+  savedObjectsClient: SavedObjectsClientContract;
 }
 
 export interface PatchRulesOptions {
   spaceId: string;
   ruleStatusClient: IRuleExecutionLogClient;
   rulesClient: RulesClient;
+  savedObjectsClient: SavedObjectsClientContract;
   anomalyThreshold: AnomalyThresholdOrUndefined;
   author: AuthorOrUndefined;
   buildingBlockType: BuildingBlockTypeOrUndefined;
@@ -323,7 +330,7 @@ export interface PatchRulesOptions {
   version: VersionOrUndefined;
   exceptionsList: ListArrayOrUndefined;
   actions: RuleAlertAction[] | undefined;
-  rule: SanitizedAlert<RuleParams> | null;
+  rule: SanitizedAlert<RuleParams> | null | undefined;
   namespace?: NamespaceOrUndefined;
 }
 
@@ -351,3 +358,9 @@ export interface FindRuleOptions {
   fields: FieldsOrUndefined;
   sortOrder: SortOrderOrUndefined;
 }
+
+export interface LegacyMigrateParams {
+  rulesClient: RulesClient;
+  savedObjectsClient: SavedObjectsClientContract;
+  rule: SanitizedAlert<RuleParams> | null | undefined;
+}
diff --git a/...lugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.test.ts b/...lugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.test.ts
@@ -6,6 +6,7 @@
  */
 
 import { rulesClientMock } from '../../../../../alerting/server/mocks';
+import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks';
 import { getFindResultWithSingleHit } from '../routes/__mocks__/request_responses';
 import { updatePrepackagedRules } from './update_prepacked_rules';
 import { patchRules } from './patch_rules';
@@ -19,10 +20,12 @@ describe.each([
 ])('updatePrepackagedRules - %s', (_, isRuleRegistryEnabled) => {
   let rulesClient: ReturnType<typeof rulesClientMock.create>;
   let ruleStatusClient: ReturnType<typeof ruleExecutionLogClientMock.create>;
+  let savedObjectsClient: ReturnType<typeof savedObjectsClientMock.create>;
 
   beforeEach(() => {
     rulesClient = rulesClientMock.create();
     ruleStatusClient = ruleExecutionLogClientMock.create();
+    savedObjectsClient = savedObjectsClientMock.create();
   });
 
   it('should omit actions and enabled when calling patchRules', async () => {
@@ -40,6 +43,7 @@ describe.each([
 
     await updatePrepackagedRules(
       rulesClient,
+      savedObjectsClient,
       'default',
       ruleStatusClient,
       [{ ...prepackagedRule, actions }],

diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts
@@ -6,13 +6,15 @@
  */
 
 import { chunk } from 'lodash/fp';
+import { SavedObjectsClientContract } from 'kibana/server';
 import { AddPrepackagedRulesSchemaDecoded } from '../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema';
 import { RulesClient, PartialAlert } from '../../../../../alerting/server';
 import { patchRules } from './patch_rules';
 import { readRules } from './read_rules';
 import { PartialFilter } from '../types';
 import { RuleParams } from '../schemas/rule_schemas';
 import { IRuleExecutionLogClient } from '../rule_execution_log/types';
+import { legacyMigrate } from './utils';
 
 /**
  * How many rules to update at a time is set to 50 from errors coming from
@@ -51,6 +53,7 @@ export const UPDATE_CHUNK_SIZE = 50;
  */
 export const updatePrepackagedRules = async (
   rulesClient: RulesClient,
+  savedObjectsClient: SavedObjectsClientContract,
   spaceId: string,
   ruleStatusClient: IRuleExecutionLogClient,
   rules: AddPrepackagedRulesSchemaDecoded[],
@@ -61,6 +64,7 @@ export const updatePrepackagedRules = async (
   for (const ruleChunk of ruleChunks) {
     const rulePromises = createPromises(
       rulesClient,
+      savedObjectsClient,
       spaceId,
       ruleStatusClient,
       ruleChunk,
@@ -82,6 +86,7 @@ export const updatePrepackagedRules = async (
  */
 export const createPromises = (
   rulesClient: RulesClient,
+  savedObjectsClient: SavedObjectsClientContract,
   spaceId: string,
   ruleStatusClient: IRuleExecutionLogClient,
   rules: AddPrepackagedRulesSchemaDecoded[],
@@ -146,10 +151,17 @@ export const createPromises = (
     // TODO: Fix these either with an is conversion or by better typing them within io-ts
     const filters: PartialFilter[] | undefined = filtersObject as PartialFilter[];
 
+    const migratedRule = await legacyMigrate({
+      rulesClient,
+      savedObjectsClient,
+      rule: existingRule,
+    });
+
     // Note: we do not pass down enabled as we do not want to suddenly disable
     // or enable rules on the user when they were not expecting it if a rule updates
     return patchRules({
       rulesClient,
+      savedObjectsClient,
       author,
       buildingBlockType,
       description,
@@ -160,7 +172,7 @@ export const createPromises = (
       language,
       license,
       outputIndex,
-      rule: existingRule,
+      rule: migratedRule,
       savedId,
       spaceId,
       ruleStatusClient,
-Original file line number
+Diff line change
@@ Expand Up / @@ -41,6 +41,8 @@ describe.each([ @@
           getAlertMock(isRuleRegistryEnabled, getQueryRuleParams())
         );
+        clients.appClient.getSignalsIndex.mockReturnValue('.siem-signals-test-index');
         updateRulesBulkRoute(server.router, ml, isRuleRegistryEnabled);
       });
@@ Expand Down @@