[Security Solution][Rules Management] Fix importing rules with connectors from one space to another

x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts

@@ -12,6 +12,7 @@ import { createPromiseFromStreams } from '@kbn/utils';
 import { chunk } from 'lodash/fp';
 import { extname } from 'path';
 import { buildRouteValidationWithZod } from '@kbn/zod-helpers';
+import type { RuleToImport } from '../../../../../../../common/api/detection_engine/rule_management';
 import {
   ImportRulesRequestQuery,
   ImportRulesResponse,
@@ -115,34 +116,60 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C
           const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] =
             getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite);
 
-          const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds(
-            parsedObjectsWithoutDuplicateErrors,
-            actionSOClient,
-            actionsClient
-          );
-
           // import actions-connectors
           const {
             successCount: actionConnectorSuccessCount,
             success: actionConnectorSuccess,
             warnings: actionConnectorWarnings,
             errors: actionConnectorErrors,
-            rulesWithMigratedActions,
           } = await importRuleActionConnectors({
             actionConnectors,
             actionsClient,
             actionsImporter,
-            rules: migratedParsedObjectsWithoutDuplicateErrors,
             overwrite: request.query.overwrite_action_connectors,
           });
 
-          // rulesWithMigratedActions: Is returned only in case connectors were exported from different namespace and the
-          // original rules actions' ids were replaced with new destinationIds
-          const parsedRules = actionConnectorErrors.length
-            ? []
-            : rulesWithMigratedActions || migratedParsedObjectsWithoutDuplicateErrors;
+          const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds(
+            parsedObjectsWithoutDuplicateErrors,
+            actionSOClient,
+            actionsClient
+          );
+
+          const rulesToImport = migratedParsedObjectsWithoutDuplicateErrors.filter(
+            (ruleOrError) => !(ruleOrError instanceof Error)
+          ) as RuleToImport[];
+
+          // After importing the actions and migrating action IDs on rules to import,
+          // validate that all actions referenced by rules exist
+          // Filter out rules that reference non-existent actions
+          const missingActionErrors: BulkError[] = [];
+          const allActions = await actionsClient.getAll();
+          const validatedActionRules = rulesToImport.filter((rule) => {
+            if (rule.actions == null || rule.actions.length === 0) {
+              return true;
+            }
+
+            const missingActions = rule.actions.filter(
+              (action) => !allActions.some((installedAction) => action.id === installedAction.id)
+            );
+
+            if (missingActions.length > 0) {
+              missingActionErrors.push({
+                id: rule.id,
+                rule_id: rule.rule_id,
+                error: {
+                  status_code: 404,
+                  message: `Rule actions reference the following missing action IDs: ${missingActions
+                    .map((action) => action.id)
+                    .join(',')}`,
+                },
+              });
+              return false;
+            }
+            return true;
+          });
 
-          const chunkParseObjects = chunk(CHUNK_PARSED_OBJECT_SIZE, parsedRules);
+          const chunkParseObjects = chunk(CHUNK_PARSED_OBJECT_SIZE, validatedActionRules);
 
           const importRuleResponse: ImportRuleResponse[] = await importRulesHelper({
             ruleChunks: chunkParseObjects,

x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts

@@ -11,16 +11,11 @@ import type { SavedObject } from '@kbn/core-saved-objects-server';
 import type { ActionsClient } from '@kbn/actions-plugin/server';
 import type { ConnectorWithExtraFindData } from '@kbn/actions-plugin/server/application/connector/types';
 
-import type { RuleToImport } from '../../../../../../../common/api/detection_engine/rule_management';
 import type { WarningSchema } from '../../../../../../../common/api/detection_engine';
 import {
-  checkIfActionsHaveMissingConnectors,
   filterExistingActionConnectors,
-  getActionConnectorRules,
-  handleActionsHaveNoConnectors,
   mapSOErrorToRuleError,
   returnErroredImportResult,
-  updateRuleActionsWithMigratedResults,
 } from './utils';
 import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types';
 
@@ -35,67 +30,35 @@ export const importRuleActionConnectors = async ({
   actionConnectors,
   actionsClient,
   actionsImporter,
-  rules,
   overwrite,
 }: ImportRuleActionConnectorsParams): Promise<ImportRuleActionConnectorsResult> => {
   try {
-    const connectorIdToRuleIdsMap = getActionConnectorRules(rules);
-    const referencedConnectorIds = await filterOutPreconfiguredConnectors(
-      actionsClient,
-      Object.keys(connectorIdToRuleIdsMap)
-    );
-
-    if (!referencedConnectorIds.length) {
-      return NO_ACTION_RESULT;
-    }
-
-    if (overwrite && !actionConnectors.length) {
-      return handleActionsHaveNoConnectors(referencedConnectorIds, connectorIdToRuleIdsMap);
-    }
-
     let actionConnectorsToImport: SavedObject[] = actionConnectors;
 
     if (!overwrite) {
-      const newIdsToAdd = await filterExistingActionConnectors(
+      actionConnectorsToImport = await filterExistingActionConnectors(
         actionsClient,
-        referencedConnectorIds
-      );
-
-      const foundMissingConnectors = checkIfActionsHaveMissingConnectors(
-        actionConnectors,
-        newIdsToAdd,
-        connectorIdToRuleIdsMap
+        actionConnectors
       );
-      if (foundMissingConnectors) return foundMissingConnectors;
-      // filter out existing connectors
-      actionConnectorsToImport = actionConnectors.filter(({ id }) => newIdsToAdd.includes(id));
     }
+
     if (!actionConnectorsToImport.length) {
       return NO_ACTION_RESULT;
     }
 
     const readStream = Readable.from(actionConnectorsToImport);
-    const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse =
+    const { success, successCount, warnings, errors }: SavedObjectsImportResponse =
       await actionsImporter.import({
         readStream,
         overwrite,
         createNewCopies: false,
       });
-    /*
-      // When a connector is exported from one namespace and imported to another, it does not result in an error, but instead a new object is created with
-      // new destination id and id will have the old  origin id, so in order to be able to use the newly generated Connectors id, this util is used to swap the old id with the
-      // new destination Id
-      */
-    let rulesWithMigratedActions: Array<RuleToImport | Error> | undefined;
-    if (successResults?.some((res) => res.destinationId))
-      rulesWithMigratedActions = updateRuleActionsWithMigratedResults(rules, successResults);
 
     return {
       success,
       successCount,
       errors: errors ? mapSOErrorToRuleError(errors) : [],
       warnings: (warnings as WarningSchema[]) || [],
-      rulesWithMigratedActions,
     };
   } catch (error) {
     return returnErroredImportResult(error);

x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts

@@ -23,7 +23,6 @@ export interface ImportRuleActionConnectorsParams {
   actionConnectors: SavedObject[];
   actionsClient: ActionsClient;
   actionsImporter: ISavedObjectsImporter;
-  rules: Array<RuleToImport | Error>;
   overwrite: boolean;
 }
 

x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts

@@ -4,23 +4,13 @@
  * 2.0; you may not use this file except in compliance with the Elastic License
  * 2.0.
  */
-import { pick } from 'lodash';
-import type {
-  SavedObjectsImportFailure,
-  SavedObjectsImportSuccess,
-} from '@kbn/core-saved-objects-common';
+
+import type { SavedObjectsImportFailure } from '@kbn/core-saved-objects-common';
 import type { SavedObject } from '@kbn/core-saved-objects-server';
 import type { ActionsClient } from '@kbn/actions-plugin/server';
 import type { BulkError } from '../../../../../routes/utils';
 import { createBulkErrorObject } from '../../../../../routes/utils';
-import type { RuleToImport } from '../../../../../../../../common/api/detection_engine/rule_management';
-import type {
-  ActionRules,
-  ConflictError,
-  ErrorType,
-  ImportRuleActionConnectorsResult,
-  SOError,
-} from '../types';
+import type { ConflictError, ErrorType, ImportRuleActionConnectorsResult, SOError } from '../types';
 
 export const returnErroredImportResult = (error: ErrorType): ImportRuleActionConnectorsResult => ({
   success: false,
@@ -29,41 +19,6 @@ export const returnErroredImportResult = (error: ErrorType): ImportRuleActionCon
   warnings: [],
 });
 
-export const handleActionsHaveNoConnectors = (
-  actionsIds: string[],
-  actionConnectorRules: ActionRules
-): ImportRuleActionConnectorsResult => {
-  const ruleIds: string = [...new Set(Object.values(actionConnectorRules).flat())].join();
-
-  if (actionsIds && actionsIds.length) {
-    const errors: BulkError[] = [];
-    const errorMessage =
-      actionsIds.length > 1
-        ? 'connectors are missing. Connector ids missing are:'
-        : 'connector is missing. Connector id missing is:';
-    errors.push(
-      createBulkErrorObject({
-        id: actionsIds.join(),
-        statusCode: 404,
-        message: `${actionsIds.length} ${errorMessage} ${actionsIds.join(', ')}`,
-        ruleId: ruleIds,
-      })
-    );
-    return {
-      success: false,
-      errors,
-      successCount: 0,
-      warnings: [],
-    };
-  }
-  return {
-    success: true,
-    errors: [],
-    successCount: 0,
-    warnings: [],
-  };
-};
-
 export const handleActionConnectorsErrors = (error: ErrorType, id?: string): BulkError => {
   let statusCode: number | null = null;
   let message: string = '';
@@ -105,69 +60,9 @@ export const mapSOErrorToRuleError = (errors: SavedObjectsImportFailure[]): Bulk
 
 export const filterExistingActionConnectors = async (
   actionsClient: ActionsClient,
-  actionsIds: string[]
+  actions: Array<SavedObject<unknown>>
 ) => {
   const storedConnectors = await actionsClient.getAll();
   const storedActionIds: string[] = storedConnectors.map(({ id }) => id);
-  return actionsIds.filter((id) => !storedActionIds.includes(id));
-};
-export const getActionConnectorRules = (rules: Array<RuleToImport | Error>) =>
-  rules.reduce((acc: { [actionsIds: string]: string[] }, rule) => {
-    if (rule instanceof Error) return acc;
-    rule.actions?.forEach(({ id }) => (acc[id] = [...(acc[id] || []), rule.rule_id]));
-    return acc;
-  }, {});
-export const checkIfActionsHaveMissingConnectors = (
-  actionConnectors: SavedObject[],
-  newIdsToAdd: string[],
-  actionConnectorRules: ActionRules
-) => {
-  // if new action-connectors don't have exported connectors will fail with missing connectors
-  if (actionConnectors.length < newIdsToAdd.length) {
-    const actionConnectorsIds = actionConnectors.map(({ id }) => id);
-    const missingActionConnector = newIdsToAdd.filter((id) => !actionConnectorsIds.includes(id));
-    const missingActionRules = pick(actionConnectorRules, [...missingActionConnector]);
-    return handleActionsHaveNoConnectors(missingActionConnector, missingActionRules);
-  }
-  return null;
-};
-
-export const mapActionIdToNewDestinationId = (
-  connectorsImportResult: SavedObjectsImportSuccess[]
-) => {
-  return connectorsImportResult.reduce(
-    (acc: { [actionId: string]: string }, { destinationId, id }) => {
-      acc[id] = destinationId || id;
-      return acc;
-    },
-    {}
-  );
-};
-
-export const swapNonDefaultSpaceIdWithDestinationId = (
-  rule: RuleToImport,
-  actionIdDestinationIdLookup: { [actionId: string]: string }
-) => {
-  return rule.actions?.map((action) => {
-    const destinationId = actionIdDestinationIdLookup[action.id];
-    return { ...action, id: destinationId };
-  });
-};
-/*
-// When a connector is exported from one namespace and imported to another, it does not result in an error, but instead a new object is created with
-// new destination id and id will have the old  origin id, so in order to be able to use the newly generated Connectors id, this util is used to swap the old id with the
-// new destination Id
-*/
-export const updateRuleActionsWithMigratedResults = (
-  rules: Array<RuleToImport | Error>,
-  connectorsImportResult: SavedObjectsImportSuccess[]
-): Array<RuleToImport | Error> => {
-  const actionIdDestinationIdLookup = mapActionIdToNewDestinationId(connectorsImportResult);
-  return rules.map((rule) => {
-    if (rule instanceof Error) return rule;
-    return {
-      ...rule,
-      actions: swapNonDefaultSpaceIdWithDestinationId(rule, actionIdDestinationIdLookup),
-    };
-  });
+  return actions.filter(({ id }) => !storedActionIds.includes(id));
 };

x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts

@@ -31,12 +31,11 @@ export interface RuleExceptionsPromiseFromStreams {
  * @param ruleChunks {array} - rules being imported
  * @param rulesResponseAcc {array} - the accumulation of success and
  * error messages gathered through the rules import logic
- * @param mlAuthz {object}
  * @param overwriteRules {boolean} - whether to overwrite existing rules
  * with imported rules if their rule_id matches
  * @param detectionRulesClient {object}
- * @param existingLists {object} - all exception lists referenced by
- * rules that were found to exist
+ * @param allowMissingConnectorSecrets {boolean}
+ * @param savedObjecsClient {object}
  * @returns {Promise} an array of error and success messages from import
  */
 export const importRules = async ({

x-pack/test/security_solution_api_integration/test_suites/detections_response/rules_management/rule_import_export/trial_license_complete_tier/import_rules.ts

@@ -1142,6 +1142,47 @@ export default ({ getService }: FtrProviderContext): void => {
               ],
             });
           });
+
+          it('should import rules and update references correctly after overwriting an existing connector', async () => {
+            const defaultSpaceConnectorId = '8fbf6d10-a21a-11ed-84a4-a33e4c2558c9';
+
+            const spaceId = '4567-space';
+            const buffer = getImportRuleWithConnectorsBuffer(defaultSpaceConnectorId);
+
+            await supertest
+              .post(`${DETECTION_ENGINE_RULES_URL}/_import`)
+              .set('kbn-xsrf', 'true')
+              .set('elastic-api-version', '2023-10-31')
+              .attach('file', buffer, 'rules.ndjson')
+              .expect(200);
+
+            await supertest
+              .post(`/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import`)
+              .set('kbn-xsrf', 'true')
+              .set('elastic-api-version', '2023-10-31')
+              .attach('file', buffer, 'rules.ndjson')
+              .expect(200);
+
+            const { body: overwriteResponseBody } = await supertest
+              .post(
+                `/s/${spaceId}${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true&overwrite_action_connectors=true`
+              )
+              .set('kbn-xsrf', 'true')
+              .set('elastic-api-version', '2023-10-31')
+              .attach('file', buffer, 'rules.ndjson')
+              .expect(200);
+
+            expect(overwriteResponseBody).toMatchObject({
+              success: true,
+              success_count: 1,
+              rules_count: 1,
+              errors: [],
+              action_connectors_success: true,
+              action_connectors_success_count: 1,
+              action_connectors_warnings: [],
+              action_connectors_errors: [],
+            });
+          });
         });
       });