From ea2319e4e4fed9813ce2c1cdb0be78f2775ffc36 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 10 Jan 2023 21:18:31 +0200 Subject: [PATCH 01/40] use getImporter and getExporter from Saved Object --- .../api/rules/import_rules/response_schema.ts | 4 + .../api/rules/bulk_actions/route.ts | 12 ++- .../api/rules/export_rules/route.ts | 27 +++++-- .../api/rules/import_rules/route.ts | 21 ++++- .../logic/export/get_export_all.ts | 20 ++++- .../logic/export/get_export_by_object_ids.ts | 25 ++++-- .../logic/export/get_export_details_ndjson.ts | 22 +++-- .../get_export_rule_action_connectors.ts | 81 +++++++++++++++++++ .../import/create_rules_stream_from_ndjson.ts | 9 +++ .../import/import_rule_action_connectors.ts | 62 ++++++++++++++ .../logic/import/import_rules_utils.ts | 3 +- 11 files changed, 258 insertions(+), 28 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts index 77ccd0812c2c9..ac43f394734e9 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts @@ -19,5 +19,9 @@ export const ImportRulesResponse = t.exact( success: t.boolean, success_count: PositiveInteger, errors: t.array(errorSchema), + // action_connectors_errors: t.array(errorSchema), + // action_connectors_warnings: t.array(), + action_connectors_success: t.boolean, + action_connectors_success_count: PositiveInteger, }) ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts index 82660cba2a870..d00a09d4f7c92 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/bulk_actions/route.ts @@ -326,6 +326,7 @@ export const performBulkActionRoute = ( 'alerting', 'licensing', 'lists', + 'actions', ]); const rulesClient = ctx.alerting.getRulesClient(); @@ -333,6 +334,11 @@ export const performBulkActionRoute = ( const exceptionsClient = ctx.lists?.getExceptionListClient(); const savedObjectsClient = ctx.core.savedObjects.client; + const { getExporter, getClient } = (await ctx.core).savedObjects; + const client = getClient({ includedHiddenTypes: ['action'] }); + + const exporter = getExporter(client); + const mlAuthz = buildMlAuthz({ license: ctx.licensing.license, ml, @@ -558,10 +564,12 @@ export const performBulkActionRoute = ( exceptionsClient, savedObjectsClient, rules.map(({ params }) => ({ rule_id: params.ruleId })), - logger + logger, + exporter, + request ); - const responseBody = `${exported.rulesNdjson}${exported.exceptionLists}${exported.exportDetails}`; + const responseBody = `${exported.rulesNdjson}${exported.exceptionLists}${exported.actionConnectors}${exported.exportDetails}`; return response.ok({ headers: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts index ec96f38f5f86e..a1a5f798fc002 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/export_rules/route.ts @@ -45,8 +45,14 @@ export const exportRulesRoute = ( const siemResponse = buildSiemResponse(response); const rulesClient = (await context.alerting).getRulesClient(); const exceptionsClient = (await context.lists)?.getExceptionListClient(); - const savedObjectsClient = (await context.core).savedObjects.client; + const { + getExporter, + getClient, + client: savedObjectsClient, + } = (await context.core).savedObjects; + const client = getClient({ includedHiddenTypes: ['action'] }); + const actionsExporter = getExporter(client); try { const exportSizeLimit = config.maxRuleImportExportSize; if (request.body?.objects != null && request.body.objects.length > exportSizeLimit) { @@ -66,20 +72,29 @@ export const exportRulesRoute = ( } } - const exportedRulesAndExceptions = + const exportedRulesAndReferences = request.body?.objects != null ? await getExportByObjectIds( rulesClient, exceptionsClient, savedObjectsClient, request.body.objects, - logger + logger, + actionsExporter, + request ) - : await getExportAll(rulesClient, exceptionsClient, savedObjectsClient, logger); + : await getExportAll( + rulesClient, + exceptionsClient, + savedObjectsClient, + logger, + actionsExporter, + request + ); const responseBody = request.query.exclude_export_details - ? exportedRulesAndExceptions.rulesNdjson - : `${exportedRulesAndExceptions.rulesNdjson}${exportedRulesAndExceptions.exceptionLists}${exportedRulesAndExceptions.exportDetails}`; + ? exportedRulesAndReferences.rulesNdjson + : `${exportedRulesAndReferences.rulesNdjson}${exportedRulesAndReferences.exceptionLists}${exportedRulesAndReferences.actionConnectors}${exportedRulesAndReferences.exportDetails}`; return response.ok({ headers: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index e306caac12194..6bb12762d4492 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -38,6 +38,7 @@ import { importRules as importRulesHelper } from '../../../logic/import/import_r import { getReferencedExceptionLists } from '../../../logic/import/gather_referenced_exceptions'; import { importRuleExceptions } from '../../../logic/import/import_rule_exceptions'; import type { HapiReadableStream } from '../../../logic/import/hapi_readable_stream'; +import { importRuleActionConnectors } from '../../../logic/import/import_rule_action_connectors'; const CHUNK_PARSED_OBJECT_SIZE = 50; @@ -81,6 +82,8 @@ export const importRulesRoute = ( const actionSOClient = ctx.core.savedObjects.getClient({ includedHiddenTypes: ['action'], }); + const actionsImporter = ctx.core.savedObjects.getImporter(actionSOClient); + const savedObjectsClient = ctx.core.savedObjects.client; const exceptionsClient = ctx.lists?.getExceptionListClient(); @@ -104,7 +107,7 @@ export const importRulesRoute = ( // parse file to separate out exceptions from rules const readAllStream = createRulesAndExceptionsStreamFromNdJson(objectLimit); - const [{ exceptions, rules }] = await createPromiseFromStreams< + const [{ exceptions, rules, actionConnectors }] = await createPromiseFromStreams< RuleExceptionsPromiseFromStreams[] >([request.body.file as HapiReadableStream, ...readAllStream]); @@ -120,6 +123,18 @@ export const importRulesRoute = ( maxExceptionsImportSize: objectLimit, }); + // import actions-connectors + const { + successCount: actionConnectorSuccessCount, + success: actionConnectorSuccess, + warnings: actionConnectorWarnings, + errors: actionConnectorErrors, + } = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter, + }); + // report on duplicate rules const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite); @@ -179,6 +194,10 @@ export const importRulesRoute = ( exceptions_errors: exceptionsErrors, exceptions_success: exceptionsSuccess, exceptions_success_count: exceptionsSuccessCount, + // action_connectors_errors: actionConnectorErrors, + // action_connectors_warnings: actionConnectorWarnings, + action_connectors_success: actionConnectorSuccess, + action_connectors_success_count: actionConnectorSuccessCount, }; const [validated, errors] = validate(importRules, ImportRulesResponse); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.ts index 058559de7db59..88f3e9019d493 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.ts @@ -7,13 +7,14 @@ import { transformDataToNdjson } from '@kbn/securitysolution-utils'; -import type { Logger } from '@kbn/core/server'; +import type { ISavedObjectsExporter, KibanaRequest, Logger } from '@kbn/core/server'; import type { ExceptionListClient } from '@kbn/lists-plugin/server'; import type { RulesClient, RuleExecutorServices } from '@kbn/alerting-plugin/server'; import { getNonPackagedRules } from '../search/get_existing_prepackaged_rules'; import { getExportDetailsNdjson } from './get_export_details_ndjson'; import { transformAlertsToRules } from '../../utils/utils'; import { getRuleExceptionsForExport } from './get_export_rule_exceptions'; +import { getRuleActionConnectorsForExport } from './get_export_rule_action_connectors'; // eslint-disable-next-line no-restricted-imports import { legacyGetBulkRuleActionsSavedObject } from '../../../rule_actions_legacy'; @@ -22,11 +23,14 @@ export const getExportAll = async ( rulesClient: RulesClient, exceptionsClient: ExceptionListClient | undefined, savedObjectsClient: RuleExecutorServices['savedObjectsClient'], - logger: Logger + logger: Logger, + actionsExporter: ISavedObjectsExporter, + request: KibanaRequest ): Promise<{ rulesNdjson: string; exportDetails: string; exceptionLists: string | null; + actionConnectors: string; }> => { const ruleAlertTypes = await getNonPackagedRules({ rulesClient }); const alertIds = ruleAlertTypes.map((rule) => rule.id); @@ -44,7 +48,15 @@ export const getExportAll = async ( const { exportData: exceptionLists, exportDetails: exceptionDetails } = await getRuleExceptionsForExport(exceptions, exceptionsClient); + // Retrieve Action-Connectors + const { actionConnectors, actionConnectorDetails } = await getRuleActionConnectorsForExport( + rules, + actionsExporter, + request + ); + const rulesNdjson = transformDataToNdjson(rules); - const exportDetails = getExportDetailsNdjson(rules, [], exceptionDetails); - return { rulesNdjson, exportDetails, exceptionLists }; + const exportDetails = getExportDetailsNdjson(rules, [], exceptionDetails, actionConnectorDetails); + + return { rulesNdjson, exportDetails, exceptionLists, actionConnectors }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts index 67da643d503e4..40a6438954bb2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts @@ -6,9 +6,10 @@ */ import { chunk } from 'lodash'; + import { transformDataToNdjson } from '@kbn/securitysolution-utils'; -import type { Logger } from '@kbn/core/server'; +import type { ISavedObjectsExporter, KibanaRequest, Logger } from '@kbn/core/server'; import type { ExceptionListClient } from '@kbn/lists-plugin/server'; import type { RulesClient, RuleExecutorServices } from '@kbn/alerting-plugin/server'; @@ -17,6 +18,7 @@ import { getExportDetailsNdjson } from './get_export_details_ndjson'; import { isAlertType } from '../../../rule_schema'; import { findRules } from '../search/find_rules'; import { getRuleExceptionsForExport } from './get_export_rule_exceptions'; +import { getRuleActionConnectorsForExport } from './get_export_rule_action_connectors'; // eslint-disable-next-line no-restricted-imports import { legacyGetBulkRuleActionsSavedObject } from '../../../rule_actions_legacy'; @@ -44,11 +46,14 @@ export const getExportByObjectIds = async ( exceptionsClient: ExceptionListClient | undefined, savedObjectsClient: RuleExecutorServices['savedObjectsClient'], objects: Array<{ rule_id: string }>, - logger: Logger + logger: Logger, + actionsExporter: ISavedObjectsExporter, + request: KibanaRequest ): Promise<{ rulesNdjson: string; exportDetails: string; exceptionLists: string | null; + actionConnectors: string; }> => { const rulesAndErrors = await getRulesFromObjects( rulesClient, @@ -56,23 +61,29 @@ export const getExportByObjectIds = async ( objects, logger ); + const { rules, missingRules } = rulesAndErrors; // Retrieve exceptions - const exceptions = rulesAndErrors.rules.flatMap((rule) => rule.exceptions_list ?? []); + const exceptions = rules.flatMap((rule) => rule.exceptions_list ?? []); const { exportData: exceptionLists, exportDetails: exceptionDetails } = await getRuleExceptionsForExport(exceptions, exceptionsClient); - const rulesNdjson = transformDataToNdjson(rulesAndErrors.rules); + // Retrieve Action-Connectors + const actionConnectors = await getRuleActionConnectorsForExport(rules, actionsExporter, request); + + const rulesNdjson = transformDataToNdjson(rules); const exportDetails = getExportDetailsNdjson( - rulesAndErrors.rules, - rulesAndErrors.missingRules, - exceptionDetails + rules, + missingRules, + exceptionDetails, + actionConnectors.actionConnectorDetails ); return { rulesNdjson, exportDetails, exceptionLists, + actionConnectors: actionConnectors.actionConnectors, }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_details_ndjson.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_details_ndjson.ts index 465c4b53b1e51..8aedcc4b645fd 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_details_ndjson.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_details_ndjson.ts @@ -8,23 +8,31 @@ import type { ExportExceptionDetails } from '@kbn/securitysolution-io-ts-list-types'; import type { ExportRulesDetails } from '../../../../../../common/detection_engine/rule_management'; import type { RuleResponse } from '../../../../../../common/detection_engine/rule_schema'; +import type { DefaultActionConnectorDetails } from './get_export_rule_action_connectors'; export const getExportDetailsNdjson = ( rules: RuleResponse[], missingRules: Array<{ rule_id: string }> = [], - exceptionDetails?: ExportExceptionDetails + exceptionDetails?: ExportExceptionDetails, + actionConnectorDetails?: DefaultActionConnectorDetails ): string => { + let exportedCount = rules.length; + + // TODO check what will be the default + if (actionConnectorDetails != null) + exportedCount += actionConnectorDetails.exported_action_connector_count; + if (exceptionDetails != null) + exportedCount += + exceptionDetails.exported_exception_list_count + + exceptionDetails.exported_exception_list_item_count; + const stringified: ExportRulesDetails = { - exported_count: - exceptionDetails == null - ? rules.length - : rules.length + - exceptionDetails.exported_exception_list_count + - exceptionDetails.exported_exception_list_item_count, + exported_count: exportedCount, exported_rules_count: rules.length, missing_rules: missingRules, missing_rules_count: missingRules.length, ...exceptionDetails, + ...actionConnectorDetails, }; return `${JSON.stringify(stringified)}\n`; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts new file mode 100644 index 0000000000000..332d5b1fc9cd6 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts @@ -0,0 +1,81 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import type { KibanaRequest } from '@kbn/core-http-server'; +import type { SavedObject, SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common'; +import type { + SavedObjectsExportResultDetails, + ISavedObjectsExporter, + SavedObjectsExportExcludedObject, +} from '@kbn/core-saved-objects-server'; +import { createConcatStream, createMapStream, createPromiseFromStreams } from '@kbn/utils'; +import type { RuleResponse } from '../../../../../../common/detection_engine/rule_schema'; + +export interface DefaultActionConnectorDetails { + exported_action_connector_count: number; + missing_action_connection_count: number; + missing_action_connections: SavedObjectTypeIdTuple[]; + excluded_action_connection_count: number; + excluded_action_connections: SavedObjectsExportExcludedObject[]; +} + +const defaultActionConnectorDetails: DefaultActionConnectorDetails = { + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], + excluded_action_connection_count: 0, + excluded_action_connections: [], +}; + +const mapExportedActionConnectorsDetailsToDefault = ( + exportDetails: SavedObjectsExportResultDetails +): DefaultActionConnectorDetails => { + return { + exported_action_connector_count: exportDetails.exportedCount, + missing_action_connection_count: exportDetails.missingRefCount, + missing_action_connections: exportDetails.missingReferences, + excluded_action_connection_count: exportDetails.excludedObjectsCount, + excluded_action_connections: exportDetails.excludedObjects, + }; +}; + +export const getRuleActionConnectorsForExport = async ( + rules: RuleResponse[], + actionsExporter: ISavedObjectsExporter, + request: KibanaRequest +) => { + const exportedActionConnectors: { + actionConnectors: string; + actionConnectorDetails: DefaultActionConnectorDetails; + } = { + actionConnectors: '', + actionConnectorDetails: defaultActionConnectorDetails, + }; + + // TODO combine line 60 and 64 + const actionsIds = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; + + if (!actionsIds.length) return exportedActionConnectors; + + const getActionsByObjectsParam = actionsIds.map((id) => ({ type: 'action', id })); + const actionDetails = await actionsExporter.exportByObjects({ + objects: getActionsByObjectsParam, + request, + }); + + const actionsConnectorsToExport: SavedObject[] = await createPromiseFromStreams([ + actionDetails, + createMapStream((obj: SavedObject | SavedObjectsExportResultDetails) => { + if ('exportedCount' in obj) + exportedActionConnectors.actionConnectorDetails = + mapExportedActionConnectorsDetailsToDefault(obj); + else return JSON.stringify(obj); + }), + createConcatStream([]), + ]); + exportedActionConnectors.actionConnectors = actionsConnectorsToExport.join('\n'); + return exportedActionConnectors; +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts index e325496225d9f..53a30980b8334 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts @@ -24,6 +24,7 @@ import type { ImportExceptionsListSchema, } from '@kbn/securitysolution-io-ts-list-types'; +import type { SavedObject } from '@kbn/core-saved-objects-common'; import { RuleToImport, validateRuleToImport, @@ -41,7 +42,9 @@ export const validateRulesStream = (): Transform => { return createMapStream<{ exceptions: Array; rules: Array; + actionConnectors: SavedObject[]; }>((items) => ({ + actionConnectors: items.actionConnectors, exceptions: items.exceptions, rules: validateRules(items.rules), })); @@ -80,10 +83,15 @@ export const sortImports = (): Transform => { return createReduceStream<{ exceptions: Array; rules: Array; + actionConnectors: SavedObject[]; }>( (acc, importItem) => { if (has('list_id', importItem) || has('item_id', importItem) || has('entries', importItem)) { return { ...acc, exceptions: [...acc.exceptions, importItem] }; + } + // if (has('actionTypeId', importItem)) { + if (has('attributes', importItem)) { + return { ...acc, actionConnectors: [...acc.actionConnectors, importItem] }; } else { return { ...acc, rules: [...acc.rules, importItem] }; } @@ -91,6 +99,7 @@ export const sortImports = (): Transform => { { exceptions: [], rules: [], + actionConnectors: [], } ); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts new file mode 100644 index 0000000000000..081c7c76c06ad --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts @@ -0,0 +1,62 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { Readable } from 'stream'; + +import type { ActionResult, ActionsClient } from '@kbn/actions-plugin/server'; +import type { SavedObject, SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; +import type { ISavedObjectsImporter } from '@kbn/core-saved-objects-server'; + +interface ImportRuleActionConnectorsParams { + actionConnectors: SavedObject[]; + actionsClient: ActionsClient; + actionsImporter: ISavedObjectsImporter; +} + +export const importRuleActionConnectors = async ({ + actionConnectors, + actionsClient, + actionsImporter, +}: ImportRuleActionConnectorsParams): Promise => { + const importResult: SavedObjectsImportResponse = { + success: true, + errors: [], + successCount: 0, + warnings: [], + }; + + if (!actionConnectors.length) return importResult; + let actionConnectorsToImport: SavedObject[] = actionConnectors; + let storedActionConnectors: ActionResult[] | [] = []; + + const actionConnectorIds = actionConnectors.map(({ id }) => id); + try { + // getBulk throws 404 error if the saved_oject wasn't found + storedActionConnectors = await actionsClient.getBulk(actionConnectorIds); + } catch (error) { + // TODO ask if there's a better way + if (error.output.statusCode === 400) storedActionConnectors = []; + } + + if (storedActionConnectors.length) + actionConnectorsToImport = actionConnectors.filter( + ({ id }) => !storedActionConnectors.find((stored) => stored.id === id) + ); + + if (!actionConnectorsToImport.length) return importResult; + + const readStream = Readable.from(actionConnectors); + + // TODO add overwrite & createNewCopies based on request query + const result: SavedObjectsImportResponse = await actionsImporter.import({ + readStream, + overwrite: false, + createNewCopies: false, + }); + + // TODO map to errorSchema of the rule + return result; +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index 694bc916fefe3..eb01fe76f6d5c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -5,7 +5,7 @@ * 2.0. */ -import type { SavedObjectsClientContract } from '@kbn/core/server'; +import type { SavedObject, SavedObjectsClientContract } from '@kbn/core/server'; import type { ImportExceptionsListSchema, ImportExceptionListItemSchema, @@ -31,6 +31,7 @@ export type PromiseFromStreams = RuleToImport | Error; export interface RuleExceptionsPromiseFromStreams { rules: PromiseFromStreams[]; exceptions: Array; + actionConnectors: SavedObject[]; } /** From dddba87f632d53c95e9c49ca5b647d3b0a2018bf Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 11 Jan 2023 08:52:44 +0000 Subject: [PATCH 02/40] [CI] Auto-commit changed files from 'node scripts/ts_project_linter --fix' --- x-pack/plugins/security_solution/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/security_solution/tsconfig.json b/x-pack/plugins/security_solution/tsconfig.json index ced495e35fbe7..fb9640bca44f4 100644 --- a/x-pack/plugins/security_solution/tsconfig.json +++ b/x-pack/plugins/security_solution/tsconfig.json @@ -129,6 +129,7 @@ "@kbn/cypress-config", "@kbn/controls-plugin", "@kbn/shared-ux-utility", + "@kbn/core-saved-objects-common", ], "exclude": [ "target/**/*", From 605ba892cdd095392422f36dc46a786a73b85004 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 11 Jan 2023 13:16:28 +0200 Subject: [PATCH 03/40] fix tests --- .../import_rules/response_schema.test.ts | 23 +++++++++++++++++++ .../api/rules/import_rules/response_schema.ts | 1 + .../export_rules_details_schema.mock.ts | 11 +++++++++ .../logic/export/get_export_all.test.ts | 20 +++++++++++++--- .../logic/export/get_export_by_object_ids.ts | 10 +++++--- 5 files changed, 59 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts index 2f11e1a9c120f..33a59f9b1561c 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts @@ -14,6 +14,7 @@ import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts import type { ErrorSchema } from '../../../../schemas/response/error_schema'; import { ImportRulesResponse } from './response_schema'; +// TODO add tests for the action_connectors_success describe('Import rules response schema', () => { test('it should validate an empty import response with no errors', () => { const payload: ImportRulesResponse = { @@ -24,6 +25,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -42,6 +45,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -60,6 +65,8 @@ describe('Import rules response schema', () => { exceptions_errors: [{ error: { status_code: 400, message: 'some message' } }], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -81,6 +88,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -102,6 +111,8 @@ describe('Import rules response schema', () => { ], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -120,6 +131,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -140,6 +153,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: -1, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -178,6 +193,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded as UnsafeCastForTest); @@ -217,6 +234,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: 'hello', exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded as UnsafeCastForTest); @@ -238,6 +257,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -262,6 +283,8 @@ describe('Import rules response schema', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts index ac43f394734e9..23da5791e3044 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts @@ -19,6 +19,7 @@ export const ImportRulesResponse = t.exact( success: t.boolean, success_count: PositiveInteger, errors: t.array(errorSchema), + // TODO need to map SavedObjectsImportFailure to errorschema // action_connectors_errors: t.array(errorSchema), // action_connectors_warnings: t.array(), action_connectors_success: t.boolean, diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts index 64b82abdb3755..5e23fd3218917 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts @@ -8,6 +8,7 @@ import { getExceptionExportDetailsMock } from '@kbn/lists-plugin/common/schemas/response/exception_export_details_schema.mock'; import type { ExportExceptionDetailsMock } from '@kbn/lists-plugin/common/schemas/response/exception_export_details_schema.mock'; import type { ExportRulesDetails } from './export_rules_details_schema'; +import type { DefaultActionConnectorDetails } from '../../../../../server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors'; interface RuleDetailsMock { totalCount?: number; @@ -16,6 +17,15 @@ interface RuleDetailsMock { missingRules?: Array>; } +// TODO add the correct type DefaultActionConnectorDetails +export const getActionConnectorDetailsMock = (): DefaultActionConnectorDetails => ({ + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], + excluded_action_connection_count: 0, + excluded_action_connections: [], +}); + export const getOutputDetailsSample = (ruleDetails?: RuleDetailsMock): ExportRulesDetails => ({ exported_count: ruleDetails?.totalCount ?? 0, exported_rules_count: ruleDetails?.rulesCount ?? 0, @@ -29,6 +39,7 @@ export const getOutputDetailsSampleWithExceptions = ( ): ExportRulesDetails => ({ ...getOutputDetailsSample(ruleDetails), ...getExceptionExportDetailsMock(exceptionDetails), + ...getActionConnectorDetailsMock(), }); export const getSampleDetailsAsNdjson = (sample: ExportRulesDetails): string => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts index b7395236a2152..093baded97fd3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts @@ -24,13 +24,17 @@ import { getQueryRuleParams } from '../../../rule_schema/mocks'; import { getExceptionListClientMock } from '@kbn/lists-plugin/server/services/exception_lists/exception_list_client.mock'; import type { loggingSystemMock } from '@kbn/core/server/mocks'; import { requestContextMock } from '../../../routes/__mocks__/request_context'; +import { savedObjectsExporterMock } from '@kbn/core-saved-objects-import-export-server-mocks'; +import { mockRouter } from '@kbn/core-http-router-server-mocks'; const exceptionsClient = getExceptionListClientMock(); +// TODO add tests for connectors describe('getExportAll', () => { let logger: ReturnType; const { clients } = requestContextMock.createTools(); - + const exporterMock = savedObjectsExporterMock.create(); + const requestMock = mockRouter.createKibanaRequest(); beforeEach(async () => { clients.savedObjectsClient.find.mockResolvedValue(getEmptySavedObjectsResponse()); }); @@ -55,7 +59,9 @@ describe('getExportAll', () => { rulesClient, exceptionsClient, clients.savedObjectsClient, - logger + logger, + exporterMock, + requestMock ); const rulesJson = JSON.parse(exports.rulesNdjson); const detailsJson = JSON.parse(exports.exportDetails); @@ -114,6 +120,11 @@ describe('getExportAll', () => { missing_exception_lists_count: 0, missing_rules: [], missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], }); }); @@ -133,12 +144,15 @@ describe('getExportAll', () => { rulesClient, exceptionsClient, clients.savedObjectsClient, - logger + logger, + exporterMock, + requestMock ); expect(exports).toEqual({ rulesNdjson: '', exportDetails: getSampleDetailsAsNdjson(details), exceptionLists: '', + actionConnectors: '', }); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts index 40a6438954bb2..7269e98a170fb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.ts @@ -69,21 +69,25 @@ export const getExportByObjectIds = async ( await getRuleExceptionsForExport(exceptions, exceptionsClient); // Retrieve Action-Connectors - const actionConnectors = await getRuleActionConnectorsForExport(rules, actionsExporter, request); + const { actionConnectors, actionConnectorDetails } = await getRuleActionConnectorsForExport( + rules, + actionsExporter, + request + ); const rulesNdjson = transformDataToNdjson(rules); const exportDetails = getExportDetailsNdjson( rules, missingRules, exceptionDetails, - actionConnectors.actionConnectorDetails + actionConnectorDetails ); return { rulesNdjson, exportDetails, exceptionLists, - actionConnectors: actionConnectors.actionConnectors, + actionConnectors, }; }; From c9a2656d40e3f724cc16cf4d5fcf85525da43fc2 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 11 Jan 2023 13:18:32 +0200 Subject: [PATCH 04/40] fix get export by id --- .../export/get_export_by_object_ids.test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts index 02b83342cd846..c13b3f457ea3e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_by_object_ids.test.ts @@ -22,14 +22,19 @@ import { } from '../../../../../../common/detection_engine/rule_management/mocks'; import { getQueryRuleParams } from '../../../rule_schema/mocks'; import { getExceptionListClientMock } from '@kbn/lists-plugin/server/services/exception_lists/exception_list_client.mock'; +import { savedObjectsExporterMock } from '@kbn/core-saved-objects-import-export-server-mocks'; +import { mockRouter } from '@kbn/core-http-router-server-mocks'; const exceptionsClient = getExceptionListClientMock(); import type { loggingSystemMock } from '@kbn/core/server/mocks'; import { requestContextMock } from '../../../routes/__mocks__/request_context'; +// TODO add tests for connectors describe('get_export_by_object_ids', () => { let logger: ReturnType; const { clients } = requestContextMock.createTools(); + const exporterMock = savedObjectsExporterMock.create(); + const requestMock = mockRouter.createKibanaRequest(); beforeEach(() => { jest.resetAllMocks(); @@ -50,7 +55,9 @@ describe('get_export_by_object_ids', () => { exceptionsClient, clients.savedObjectsClient, objects, - logger + logger, + exporterMock, + requestMock ); const exportsObj = { rulesNdjson: JSON.parse(exports.rulesNdjson), @@ -112,6 +119,11 @@ describe('get_export_by_object_ids', () => { missing_exception_lists_count: 0, missing_rules: [], missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], }, }); }); @@ -137,7 +149,9 @@ describe('get_export_by_object_ids', () => { exceptionsClient, clients.savedObjectsClient, objects, - logger + logger, + exporterMock, + requestMock ); const details = getOutputDetailsSampleWithExceptions({ missingRules: [{ rule_id: 'rule-1' }], @@ -147,6 +161,7 @@ describe('get_export_by_object_ids', () => { rulesNdjson: '', exportDetails: getSampleDetailsAsNdjson(details), exceptionLists: '', + actionConnectors: '', }); }); }); From 9b2cb0e778624cbf83ffd163a346f384ef30d76a Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 11 Jan 2023 11:22:57 +0000 Subject: [PATCH 05/40] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/security_solution/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/security_solution/tsconfig.json b/x-pack/plugins/security_solution/tsconfig.json index 8824971b0fab1..f51e3c8b5a3ba 100644 --- a/x-pack/plugins/security_solution/tsconfig.json +++ b/x-pack/plugins/security_solution/tsconfig.json @@ -134,5 +134,6 @@ "@kbn/controls-plugin", "@kbn/shared-ux-utility", "@kbn/core-saved-objects-common", + "@kbn/core-saved-objects-import-export-server-mocks", ] } From f5a199b117cb850f2e6715111b7d78a78035b3f6 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 11 Jan 2023 15:59:40 +0200 Subject: [PATCH 06/40] comment warnings until decide if needed --- .../rule_management/api/rules/import_rules/route.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index 6bb12762d4492..5ca5bac729767 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -127,8 +127,9 @@ export const importRulesRoute = ( const { successCount: actionConnectorSuccessCount, success: actionConnectorSuccess, - warnings: actionConnectorWarnings, - errors: actionConnectorErrors, + // TODO revert back after the mapping + // warnings: actionConnectorWarnings, + // errors: actionConnectorErrors, } = await importRuleActionConnectors({ actionConnectors, actionsClient, From 0a0032681d69fda1e0d3a6605a62276ec505a05f Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 11 Jan 2023 19:28:37 +0200 Subject: [PATCH 07/40] fix tests --- .../export_rules_details_schema.test.ts | 9 ++-- .../export/export_rules_details_schema.ts | 46 +++++++++++++------ .../api/rules/import_rules/route.test.ts | 26 +++++++++++ 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts index 2c84a34b34928..907f23f1dc985 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts @@ -21,12 +21,13 @@ import { getOutputDetailsSampleWithExceptions, } from './export_rules_details_schema.mock'; import type { ExportRulesDetails } from './export_rules_details_schema'; -import { exportRulesDetailsWithExceptionsSchema } from './export_rules_details_schema'; +import { exportRulesDetailsWithExceptionsAndConnectorsSchema } from './export_rules_details_schema'; +// TODO add tests for connectors describe('exportRulesDetailsWithExceptionsSchema', () => { test('it should validate export details response', () => { const payload = getOutputDetailsSample(); - const decoded = exportRulesDetailsWithExceptionsSchema.decode(payload); + const decoded = exportRulesDetailsWithExceptionsAndConnectorsSchema.decode(payload); const checked = exactCheck(payload, decoded); const message = pipe(checked, foldLeftRight); @@ -36,7 +37,7 @@ describe('exportRulesDetailsWithExceptionsSchema', () => { test('it should validate export details with exceptions details response', () => { const payload = getOutputDetailsSampleWithExceptions(); - const decoded = exportRulesDetailsWithExceptionsSchema.decode(payload); + const decoded = exportRulesDetailsWithExceptionsAndConnectorsSchema.decode(payload); const checked = exactCheck(payload, decoded); const message = pipe(checked, foldLeftRight); @@ -49,7 +50,7 @@ describe('exportRulesDetailsWithExceptionsSchema', () => { extraKey?: string; } = getOutputDetailsSample(); payload.extraKey = 'some extra key'; - const decoded = exportRulesDetailsWithExceptionsSchema.decode(payload); + const decoded = exportRulesDetailsWithExceptionsAndConnectorsSchema.decode(payload); const message = pipe(decoded, foldLeftRight); expect(getPaths(left(message.errors))).toEqual([]); diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.ts index 85b423135566b..205861bdd42a7 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.ts @@ -9,13 +9,6 @@ import * as t from 'io-ts'; import { exportExceptionDetails } from '@kbn/securitysolution-io-ts-list-types'; import { NonEmptyString } from '@kbn/securitysolution-io-ts-types'; -const createSchema = ( - requiredFields: Required, - optionalFields: Optional -) => { - return t.intersection([t.exact(t.type(requiredFields)), t.exact(t.partial(optionalFields))]); -}; - const exportRulesDetails = { exported_count: t.number, exported_rules_count: t.number, @@ -28,11 +21,38 @@ const exportRulesDetails = { ), missing_rules_count: t.number, }; +const excludedActionConnectors = t.intersection([ + t.exact( + t.type({ + id: NonEmptyString, + type: NonEmptyString, + }) + ), + t.exact(t.partial({ reason: t.string })), +]); + +const exportRuleActionConnectorsDetails = { + exported_action_connector_count: t.number, + missing_action_connection_count: t.number, + missing_action_connections: t.array( + t.exact( + t.type({ + id: NonEmptyString, + type: NonEmptyString, + }) + ) + ), + excluded_action_connection_count: t.number, + excluded_action_connections: t.array(excludedActionConnectors), +}; -// With exceptions -export const exportRulesDetailsWithExceptionsSchema = createSchema( - exportRulesDetails, - exportExceptionDetails -); +// With exceptions and connectors +export const exportRulesDetailsWithExceptionsAndConnectorsSchema = t.intersection([ + t.exact(t.type(exportRulesDetails)), + t.exact(t.partial(exportExceptionDetails)), + t.exact(t.partial(exportRuleActionConnectorsDetails)), +]); -export type ExportRulesDetails = t.TypeOf; +export type ExportRulesDetails = t.TypeOf< + typeof exportRulesDetailsWithExceptionsAndConnectorsSchema +>; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts index 3d466d2505074..0483d4abd60f5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts @@ -34,6 +34,8 @@ import { importRulesRoute } from './route'; jest.mock('../../../../../machine_learning/authz'); +// TODO add tests for connectors + describe('Import rules route', () => { let config: ReturnType; let server: ReturnType; @@ -107,6 +109,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -146,6 +150,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -171,6 +177,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -196,6 +204,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -218,6 +228,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); }); @@ -242,6 +254,8 @@ describe('Import rules route', () => { rules_count: 2, exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -262,6 +276,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -302,6 +318,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -332,6 +350,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -353,6 +373,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); }); @@ -387,6 +409,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); @@ -407,6 +431,8 @@ describe('Import rules route', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, }); }); }); From eca3cf3919589d7fa581fc40a643acbd0a23048e Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 12:09:51 +0200 Subject: [PATCH 08/40] enable Rule importing even if connectors are missing secrets --- .../server/rules_client/methods/create.ts | 13 ++-- .../api/rules/import_rules/response_schema.ts | 7 +-- .../schemas/response/index.ts | 1 + .../schemas/response/warning_schema/index.ts | 24 ++++++++ .../components/import_data_modal/index.tsx | 60 +++++++++++++++++-- .../rule_management/logic/types.ts | 4 ++ .../api/rules/import_rules/route.ts | 33 ++++------ .../logic/crud/create_rules.ts | 3 + .../import_rule_action_connectors.ts | 38 ++++++------ .../logic/import/action_connectors/types.ts | 25 ++++++++ ...alidate_action_connectors_import_result.ts | 24 ++++++++ .../logic/import/import_rules_utils.ts | 3 + 12 files changed, 179 insertions(+), 56 deletions(-) create mode 100644 x-pack/plugins/security_solution/common/detection_engine/schemas/response/warning_schema/index.ts rename x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/{ => action_connectors}/import_rule_action_connectors.ts (62%) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts diff --git a/x-pack/plugins/alerting/server/rules_client/methods/create.ts b/x-pack/plugins/alerting/server/rules_client/methods/create.ts index e8dcefe4a9ef1..ef78f83edaac2 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/create.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/create.ts @@ -46,11 +46,12 @@ export interface CreateOptions { | 'nextRun' > & { actions: NormalizedAlertAction[] }; options?: SavedObjectOptions; + validateRuleActionConnectors?: boolean; } export async function create( context: RulesClientContext, - { data, options }: CreateOptions + { data, options, validateRuleActionConnectors }: CreateOptions ): Promise> { const id = options?.id || SavedObjectsUtils.generateId(); @@ -104,10 +105,12 @@ export async function create( } } - await validateActions(context, ruleType, data); - await withSpan({ name: 'validateActions', type: 'rules' }, () => - validateActions(context, ruleType, data) - ); + if (validateRuleActionConnectors) { + await validateActions(context, ruleType, data); + await withSpan({ name: 'validateActions', type: 'rules' }, () => + validateActions(context, ruleType, data) + ); + } // Throw error if schedule interval is less than the minimum and we are enforcing it const intervalInMs = parseDuration(data.schedule.interval); if ( diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts index 23da5791e3044..99212b902c4d3 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.ts @@ -7,7 +7,7 @@ import * as t from 'io-ts'; import { PositiveInteger } from '@kbn/securitysolution-io-ts-types'; -import { errorSchema } from '../../../../schemas/response/error_schema'; +import { errorSchema, warningSchema } from '../../../../schemas/response'; export type ImportRulesResponse = t.TypeOf; export const ImportRulesResponse = t.exact( @@ -19,9 +19,8 @@ export const ImportRulesResponse = t.exact( success: t.boolean, success_count: PositiveInteger, errors: t.array(errorSchema), - // TODO need to map SavedObjectsImportFailure to errorschema - // action_connectors_errors: t.array(errorSchema), - // action_connectors_warnings: t.array(), + action_connectors_errors: t.array(errorSchema), + action_connectors_warnings: t.array(warningSchema), action_connectors_success: t.boolean, action_connectors_success_count: PositiveInteger, }) diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts index b20a956525e2e..76da687604028 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/index.ts @@ -6,3 +6,4 @@ */ export * from './error_schema'; +export * from './warning_schema'; diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/warning_schema/index.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/warning_schema/index.ts new file mode 100644 index 0000000000000..1a401d1941cb0 --- /dev/null +++ b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/warning_schema/index.ts @@ -0,0 +1,24 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import * as t from 'io-ts'; + +const partial = t.exact( + t.partial({ + buttonLabel: t.string, + }) +); +const required = t.exact( + t.type({ + type: t.string, + message: t.string, + actionPath: t.string, + }) +); + +export const warningSchema = t.intersection([partial, required]); +export type WarningSchema = t.TypeOf; diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index a291265bc234d..525d6b826ee15 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -5,11 +5,15 @@ * 2.0. */ +import React, { useCallback, useState } from 'react'; import { EuiButton, EuiButtonEmpty, + EuiCallOut, EuiCheckbox, EuiFilePicker, + EuiFlexGroup, + EuiFlexItem, EuiModal, EuiModalBody, EuiModalFooter, @@ -18,7 +22,7 @@ import { EuiSpacer, EuiText, } from '@elastic/eui'; -import React, { useCallback, useState } from 'react'; +import type { WarningSchema } from '../../../../common/detection_engine/schemas/response'; import type { ImportDataResponse, @@ -27,6 +31,7 @@ import type { import { useAppToasts } from '../../hooks/use_app_toasts'; import * as i18n from './translations'; import { showToasterMessage } from './utils'; +import { useKibana } from '../../lib/kibana/kibana_react'; interface ImportDataModalProps { checkBoxLabel: string; @@ -70,6 +75,14 @@ export const ImportDataModalComponent = ({ const [overwrite, setOverwrite] = useState(false); const [overwriteExceptions, setOverwriteExceptions] = useState(false); const { addError, addSuccess } = useAppToasts(); + // TODO Ask if we use schema on the FE + const [actionConnectorsWarnings, setActionConnectorsWarnings] = useState( + [] + ); + const { + http, + application: { navigateToApp }, + } = useKibana().services; const cleanupAndCloseModal = useCallback(() => { setIsImporting(false); @@ -85,12 +98,14 @@ export const ImportDataModalComponent = ({ const abortCtrl = new AbortController(); try { - const importResponse = await importData({ + const { action_connectors_warnings: warnings, ...importResponse } = await importData({ fileToImport: selectedFiles[0], overwrite, overwriteExceptions, signal: abortCtrl.signal, }); + const showWarnings = !!warnings?.length; + setActionConnectorsWarnings(warnings as WarningSchema[]); showToasterMessage({ importResponse, @@ -102,19 +117,24 @@ export const ImportDataModalComponent = ({ addSuccess, }); - importComplete(); - cleanupAndCloseModal(); + if (!showWarnings) { + importComplete(); + return cleanupAndCloseModal(); + } + + setIsImporting(false); + setSelectedFiles(null); } catch (error) { cleanupAndCloseModal(); addError(error, { title: errorMessage(1) }); } } }, [ + showExceptionsCheckBox, selectedFiles, - importData, overwrite, overwriteExceptions, - showExceptionsCheckBox, + importData, successMessage, errorMessage, failedDetailed, @@ -163,6 +183,34 @@ export const ImportDataModalComponent = ({ isLoading={isImporting} /> + {!!actionConnectorsWarnings?.length && + actionConnectorsWarnings.map(({ message, actionPath, buttonLabel }) => ( + + + + {message} + + + {buttonLabel && ( + + + {buttonLabel} + + + )} + + + ))} + + {showCheckBox && ( <> !(rule instanceof Error)); - - if (actualRules.some((rule) => rule.actions && rule.actions.length > 0)) { - const [nonExistentActionErrors, uniqueParsedObjects] = await getInvalidConnectors( - migratedParsedObjectsWithoutDuplicateErrors, - actionsClient - ); - parsedRules = uniqueParsedObjects; - actionErrors = nonExistentActionErrors; - } else { - parsedRules = migratedParsedObjectsWithoutDuplicateErrors; - } + // TODO check why we need to parse rules?? + const parsedRules = migratedParsedObjectsWithoutDuplicateErrors; + // gather all exception lists that the imported rules reference const foundReferencedExceptionLists = await getReferencedExceptionLists({ rules: parsedRules, @@ -169,7 +155,7 @@ export const importRulesRoute = ( const importRuleResponse: ImportRuleResponse[] = await importRulesHelper({ ruleChunks: chunkParseObjects, - rulesResponseAcc: [...actionErrors, ...duplicateIdErrors], + rulesResponseAcc: [...actionConnectorErrors, ...duplicateIdErrors], mlAuthz, overwriteRules: request.query.overwrite, rulesClient, @@ -177,6 +163,7 @@ export const importRulesRoute = ( exceptionsClient, spaceId: ctx.securitySolution.getSpaceId(), existingLists: foundReferencedExceptionLists, + validateRuleActionConnectors: !actionConnectors.length, }); const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[]; @@ -195,10 +182,10 @@ export const importRulesRoute = ( exceptions_errors: exceptionsErrors, exceptions_success: exceptionsSuccess, exceptions_success_count: exceptionsSuccessCount, - // action_connectors_errors: actionConnectorErrors, - // action_connectors_warnings: actionConnectorWarnings, action_connectors_success: actionConnectorSuccess, action_connectors_success_count: actionConnectorSuccessCount, + action_connectors_errors: actionConnectorErrors, + action_connectors_warnings: actionConnectorWarnings, }; const [validated, errors] = validate(importRules, ImportRulesResponse); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts index f621542fe433f..0b2069fef75d0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts @@ -19,6 +19,7 @@ export interface CreateRulesOptions id?: string; immutable?: boolean; defaultEnabled?: boolean; + validateRuleActionConnectors?: boolean; } export const createRules = async ({ @@ -27,6 +28,7 @@ export const createRules = async ({ id, immutable = false, defaultEnabled = true, + validateRuleActionConnectors, }: CreateRulesOptions): Promise> => { const internalRule = convertCreateAPIToInternalSchema(params, immutable, defaultEnabled); const rule = await rulesClient.create({ @@ -34,6 +36,7 @@ export const createRules = async ({ id, }, data: internalRule, + validateRuleActionConnectors, }); // Mute the rule if it is first created with the explicit no actions diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts similarity index 62% rename from x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts rename to x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 081c7c76c06ad..83388ab6673ac 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -6,22 +6,18 @@ */ import { Readable } from 'stream'; -import type { ActionResult, ActionsClient } from '@kbn/actions-plugin/server'; +import type { ActionResult } from '@kbn/actions-plugin/server'; import type { SavedObject, SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; -import type { ISavedObjectsImporter } from '@kbn/core-saved-objects-server'; - -interface ImportRuleActionConnectorsParams { - actionConnectors: SavedObject[]; - actionsClient: ActionsClient; - actionsImporter: ISavedObjectsImporter; -} +import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; +import { mapSOErrorToRuleError } from './validate_action_connectors_import_result'; +import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types'; export const importRuleActionConnectors = async ({ actionConnectors, actionsClient, actionsImporter, -}: ImportRuleActionConnectorsParams): Promise => { - const importResult: SavedObjectsImportResponse = { +}: ImportRuleActionConnectorsParams): Promise => { + const importResult: ImportRuleActionConnectorsResult = { success: true, errors: [], successCount: 0, @@ -51,12 +47,18 @@ export const importRuleActionConnectors = async ({ const readStream = Readable.from(actionConnectors); // TODO add overwrite & createNewCopies based on request query - const result: SavedObjectsImportResponse = await actionsImporter.import({ - readStream, - overwrite: false, - createNewCopies: false, - }); - - // TODO map to errorSchema of the rule - return result; + const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse = + await actionsImporter.import({ + readStream, + overwrite: false, + createNewCopies: false, + }); + + return { + success, + successCount, + successResults, + errors: mapSOErrorToRuleError(errors) || [], + warnings: (warnings as WarningSchema[]) || [], + }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts new file mode 100644 index 0000000000000..37d16c518adcd --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -0,0 +1,25 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import type { ISavedObjectsImporter } from '@kbn/core-saved-objects-server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import type { SavedObject, SavedObjectsImportSuccess } from '@kbn/core-saved-objects-common'; +import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; +import type { BulkError } from '../../../../routes/utils'; + +export interface ImportRuleActionConnectorsResult { + success: boolean; + successCount: number; + successResults?: SavedObjectsImportSuccess[]; + errors: BulkError[] | []; + warnings: WarningSchema[] | []; +} + +export interface ImportRuleActionConnectorsParams { + actionConnectors: SavedObject[]; + actionsClient: ActionsClient; + actionsImporter: ISavedObjectsImporter; +} diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts new file mode 100644 index 0000000000000..23f06a407e018 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts @@ -0,0 +1,24 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { + SavedObjectsImportFailure, + SavedObjectsImportUnknownError, +} from '@kbn/core-saved-objects-common'; +import type { BulkError } from '../../../../routes/utils'; +import { createBulkErrorObject } from '../../../../routes/utils'; + +export const mapSOErrorToRuleError = (errors?: SavedObjectsImportFailure[]): BulkError[] => { + if (!errors) return []; + return errors.map(({ id, error }) => + createBulkErrorObject({ + id, + statusCode: (error as SavedObjectsImportUnknownError).statusCode, + message: (error as SavedObjectsImportUnknownError).message, + }) + ); +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index eb01fe76f6d5c..a275de8af6eb7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -61,6 +61,7 @@ export const importRules = async ({ exceptionsClient, spaceId, existingLists, + validateRuleActionConnectors, }: { ruleChunks: PromiseFromStreams[][]; rulesResponseAcc: ImportRuleResponse[]; @@ -71,6 +72,7 @@ export const importRules = async ({ exceptionsClient: ExceptionListClient | undefined; spaceId: string; existingLists: Record; + validateRuleActionConnectors: boolean; }) => { let importRuleResponse: ImportRuleResponse[] = [...rulesResponseAcc]; @@ -119,6 +121,7 @@ export const importRules = async ({ ...parsedRule, exceptions_list: [...exceptions], }, + validateRuleActionConnectors, }); resolve({ rule_id: parsedRule.rule_id, From 912b6c9c7925c8c30d2a20818a675face335c44c Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 15:12:46 +0200 Subject: [PATCH 09/40] test: Import rules response schema --- .../import_rules/response_schema.test.ts | 157 +++++++++++++++++- 1 file changed, 156 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts index 33a59f9b1561c..a63a2006041e2 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts @@ -14,7 +14,6 @@ import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts import type { ErrorSchema } from '../../../../schemas/response/error_schema'; import { ImportRulesResponse } from './response_schema'; -// TODO add tests for the action_connectors_success describe('Import rules response schema', () => { test('it should validate an empty import response with no errors', () => { const payload: ImportRulesResponse = { @@ -27,6 +26,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -47,6 +48,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -67,6 +70,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -90,6 +95,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -113,6 +120,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -133,6 +142,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -155,6 +166,8 @@ describe('Import rules response schema', () => { exceptions_success_count: -1, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -195,6 +208,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded as UnsafeCastForTest); @@ -236,6 +251,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded as UnsafeCastForTest); @@ -259,6 +276,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -285,6 +304,8 @@ describe('Import rules response schema', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }; const decoded = ImportRulesResponse.decode(payload); const checked = exactCheck(payload, decoded); @@ -293,4 +314,138 @@ describe('Import rules response schema', () => { expect(getPaths(left(message.errors))).toEqual(['invalid keys "invalid_data"']); expect(message.schema).toEqual({}); }); + + test('it should validate an empty import response with a single connectors error', () => { + const payload: ImportRulesResponse = { + success: false, + success_count: 0, + rules_count: 0, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [{ error: { status_code: 400, message: 'some message' } }], + action_connectors_warnings: [], + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + test('it should validate an empty import response with multiple errors', () => { + const payload: ImportRulesResponse = { + success: false, + success_count: 0, + rules_count: 0, + errors: [ + { error: { status_code: 400, message: 'some message' } }, + { error: { status_code: 500, message: 'some message' } }, + ], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [{ error: { status_code: 400, message: 'some message' } }], + action_connectors_warnings: [], + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); + test('it should NOT validate action_connectors_success that is not boolean', () => { + type UnsafeCastForTest = Either< + Errors, + { + success: boolean; + action_connectors_success: string; + success_count: number; + errors: Array< + { + id?: string | undefined; + rule_id?: string | undefined; + } & { + error: { + status_code: number; + message: string; + }; + } + >; + } + >; + const payload: Omit & { + action_connectors_success: string; + } = { + success: true, + success_count: 0, + rules_count: 0, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: 'invalid', + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded as UnsafeCastForTest); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "invalid" supplied to "action_connectors_success"', + ]); + expect(message.schema).toEqual({}); + }); + test('it should NOT validate a action_connectors_success_count that is a negative number', () => { + const payload: ImportRulesResponse = { + success: false, + success_count: 0, + rules_count: 0, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: -1, + action_connectors_errors: [], + action_connectors_warnings: [], + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "-1" supplied to "action_connectors_success_count"', + ]); + expect(message.schema).toEqual({}); + }); + test('it should validate a action_connectors_warnings after importing successfully', () => { + const payload: ImportRulesResponse = { + success: false, + success_count: 0, + rules_count: 0, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 1, + action_connectors_errors: [], + action_connectors_warnings: [{ type: 'type', message: 'message', actionPath: 'actionPath' }], + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); }); From d898d3b53fd02f1917505adfc972d9e51c65440e Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 16:02:16 +0200 Subject: [PATCH 10/40] rename validateRuleActionConnectors to skipActionConnectorsValidations to negate the boolean --- .../server/rules_client/methods/create.ts | 6 +- .../server/rules_client/tests/create.test.ts | 172 ++++++++++++++++++ .../import_rules/response_schema.test.ts | 44 +++++ .../api/rules/import_rules/route.ts | 2 +- .../logic/crud/create_rules.ts | 6 +- .../logic/import/import_rules_utils.ts | 6 +- 6 files changed, 226 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/methods/create.ts b/x-pack/plugins/alerting/server/rules_client/methods/create.ts index ef78f83edaac2..cb9560c415f32 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/create.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/create.ts @@ -46,12 +46,12 @@ export interface CreateOptions { | 'nextRun' > & { actions: NormalizedAlertAction[] }; options?: SavedObjectOptions; - validateRuleActionConnectors?: boolean; + skipActionConnectorsValidations?: boolean; } export async function create( context: RulesClientContext, - { data, options, validateRuleActionConnectors }: CreateOptions + { data, options, skipActionConnectorsValidations }: CreateOptions ): Promise> { const id = options?.id || SavedObjectsUtils.generateId(); @@ -105,7 +105,7 @@ export async function create( } } - if (validateRuleActionConnectors) { + if (!skipActionConnectorsValidations) { await validateActions(context, ruleType, data); await withSpan({ name: 'validateActions', type: 'rules' }, () => validateActions(context, ruleType, data) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts index 050de16a68a84..fc0b248ed358a 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts @@ -2889,4 +2889,176 @@ describe('create()', () => { expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); }); + + test('should create a rule and not throw an error when skipActionConnectorsValidations is true even when some actions are missing frequency params', async () => { + const data = getMockData({ + actions: [ + { + group: 'default', + id: '1', + params: { + foo: true, + }, + }, + { + group: 'default', + id: '1', + params: { + foo: true, + }, + }, + { + group: 'default', + id: '2', + params: { + foo: true, + }, + }, + ], + }); + actionsClient.getBulk.mockReset(); + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: true, + name: 'email connector', + isPreconfigured: false, + isDeprecated: false, + }, + { + id: '2', + actionTypeId: 'test', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: false, + name: 'email connector', + isPreconfigured: false, + isDeprecated: false, + }, + ]); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'alert', + attributes: { + alertTypeId: '123', + schedule: { interval: '1m' }, + params: { + bar: true, + }, + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + notifyWhen: null, + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'default', + actionRef: 'action_1', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'default', + actionRef: 'action_2', + actionTypeId: 'test2', + params: { + foo: true, + }, + }, + ], + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + { + name: 'action_1', + type: 'action', + id: '1', + }, + { + name: 'action_2', + type: 'action', + id: '2', + }, + ], + }); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'alert', + attributes: { + actions: [], + scheduledTaskId: 'task-123', + }, + references: [], + }); + const result = await rulesClient.create({ data, skipActionConnectorsValidations: true }); + expect(result).toMatchInlineSnapshot(` + Object { + "actions": Array [ + Object { + "actionTypeId": "test", + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + Object { + "actionTypeId": "test", + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + Object { + "actionTypeId": "test2", + "group": "default", + "id": "2", + "params": Object { + "foo": true, + }, + }, + ], + "alertTypeId": "123", + "createdAt": 2019-02-12T21:01:22.479Z, + "id": "1", + "notifyWhen": null, + "params": Object { + "bar": true, + }, + "schedule": Object { + "interval": "1m", + }, + "scheduledTaskId": "task-123", + "updatedAt": 2019-02-12T21:01:22.479Z, + } + `); + }); }); diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts index a63a2006041e2..b2ff257ba2c4e 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/api/rules/import_rules/response_schema.test.ts @@ -448,4 +448,48 @@ describe('Import rules response schema', () => { expect(getPaths(left(message.errors))).toEqual([]); expect(message.schema).toEqual(payload); }); + test('it should NOT validate a action_connectors_warnings that is not WarningSchema', () => { + type UnsafeCastForTest = Either< + Errors, + { + success: boolean; + action_connectors_warnings: string; + success_count: number; + errors: Array< + { + id?: string | undefined; + rule_id?: string | undefined; + } & { + error: { + status_code: number; + message: string; + }; + } + >; + } + >; + const payload: Omit & { + action_connectors_warnings: string; + } = { + success: true, + success_count: 0, + rules_count: 0, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: 'invalid', + }; + const decoded = ImportRulesResponse.decode(payload); + const checked = exactCheck(payload, decoded as UnsafeCastForTest); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "invalid" supplied to "action_connectors_warnings"', + ]); + expect(message.schema).toEqual({}); + }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index 9b166f7d726f1..ec207b1e525f1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -163,7 +163,7 @@ export const importRulesRoute = ( exceptionsClient, spaceId: ctx.securitySolution.getSpaceId(), existingLists: foundReferencedExceptionLists, - validateRuleActionConnectors: !actionConnectors.length, + skipActionConnectorsValidations: !actionConnectors.length, }); const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts index 0b2069fef75d0..d8bbc4783eb89 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts @@ -19,7 +19,7 @@ export interface CreateRulesOptions id?: string; immutable?: boolean; defaultEnabled?: boolean; - validateRuleActionConnectors?: boolean; + skipActionConnectorsValidations?: boolean; } export const createRules = async ({ @@ -28,7 +28,7 @@ export const createRules = async ({ id, immutable = false, defaultEnabled = true, - validateRuleActionConnectors, + skipActionConnectorsValidations, }: CreateRulesOptions): Promise> => { const internalRule = convertCreateAPIToInternalSchema(params, immutable, defaultEnabled); const rule = await rulesClient.create({ @@ -36,7 +36,7 @@ export const createRules = async ({ id, }, data: internalRule, - validateRuleActionConnectors, + skipActionConnectorsValidations, }); // Mute the rule if it is first created with the explicit no actions diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index a275de8af6eb7..1eef058fbfa40 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -61,7 +61,7 @@ export const importRules = async ({ exceptionsClient, spaceId, existingLists, - validateRuleActionConnectors, + skipActionConnectorsValidations, }: { ruleChunks: PromiseFromStreams[][]; rulesResponseAcc: ImportRuleResponse[]; @@ -72,7 +72,7 @@ export const importRules = async ({ exceptionsClient: ExceptionListClient | undefined; spaceId: string; existingLists: Record; - validateRuleActionConnectors: boolean; + skipActionConnectorsValidations: boolean; }) => { let importRuleResponse: ImportRuleResponse[] = [...rulesResponseAcc]; @@ -121,7 +121,7 @@ export const importRules = async ({ ...parsedRule, exceptions_list: [...exceptions], }, - validateRuleActionConnectors, + skipActionConnectorsValidations, }); resolve({ rule_id: parsedRule.rule_id, From f69890b749909484332ef679fa894f232a1ca9d4 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 16:04:09 +0200 Subject: [PATCH 11/40] remove unused var --- .../public/common/components/import_data_modal/index.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index 525d6b826ee15..bcf6565f8318b 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -79,10 +79,7 @@ export const ImportDataModalComponent = ({ const [actionConnectorsWarnings, setActionConnectorsWarnings] = useState( [] ); - const { - http, - application: { navigateToApp }, - } = useKibana().services; + const { http } = useKibana().services; const cleanupAndCloseModal = useCallback(() => { setIsImporting(false); From b9f4fb381092f1a6f6ff424e237b30099f6eaabc Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 16:06:22 +0200 Subject: [PATCH 12/40] optional skipActionConnectorsValidations --- .../rule_management/logic/import/import_rules_utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index 1eef058fbfa40..d7dda57213545 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -72,7 +72,7 @@ export const importRules = async ({ exceptionsClient: ExceptionListClient | undefined; spaceId: string; existingLists: Record; - skipActionConnectorsValidations: boolean; + skipActionConnectorsValidations?: boolean; }) => { let importRuleResponse: ImportRuleResponse[] = [...rulesResponseAcc]; From 3b6106b9dc9d35cf17c86ae6d1e77499e31570da Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 16:08:54 +0200 Subject: [PATCH 13/40] fix /rules/import_rules/route.test.ts --- .../api/rules/import_rules/route.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts index 0483d4abd60f5..201335207754a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts @@ -111,6 +111,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -152,6 +154,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -179,6 +183,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -206,6 +212,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -230,6 +238,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); }); @@ -256,6 +266,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -278,6 +290,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -320,6 +334,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -352,6 +368,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -375,6 +393,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); }); @@ -411,6 +431,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); @@ -433,6 +455,8 @@ describe('Import rules route', () => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [], }); }); }); From 2d7c90375a7e211aca8c47575920cfe087847f35 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 17 Jan 2023 17:34:20 +0200 Subject: [PATCH 14/40] fix condition --- .../rule_management/api/rules/import_rules/route.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index ec207b1e525f1..d44ee84be3881 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -163,7 +163,7 @@ export const importRulesRoute = ( exceptionsClient, spaceId: ctx.securitySolution.getSpaceId(), existingLists: foundReferencedExceptionLists, - skipActionConnectorsValidations: !actionConnectors.length, + skipActionConnectorsValidations: !!actionConnectors.length, }); const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[]; From 67e800659a1c532890ed15d41dc2f7e92488bfef Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 18 Jan 2023 13:55:30 +0200 Subject: [PATCH 15/40] refactor closing Modal --- .../__snapshots__/index.test.tsx.snap | 276 ++++++++++++++++++ .../import_data_modal/index.test.tsx | 49 ++++ .../components/import_data_modal/index.tsx | 55 ++-- 3 files changed, 355 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap index 47dd897f71ffb..9b1106043cb6a 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap @@ -90,6 +90,282 @@ Object {
+
+
+ +
+ +
+
+
+
+ + +
+
+ + + , + "container":
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; + +exports[`ImportDataModal should import file, with warnings 1`] = ` +Object { + "asFragment": [Function], + "baseElement": +
+
+
+
+ +
+

+ title +

+
+
+
+
+

+ description +

+
+
+
+
+ +
+
+
+
+
+
+

+

+
+
+
+
+ message +
+
+
+ +
+
+
+
+
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx index 515019785534f..3675ac58e083b 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx @@ -12,6 +12,11 @@ import { ImportDataModalComponent } from '.'; jest.mock('../../lib/kibana'); +jest.mock('../../lib/kibana/kibana_react', () => ({ + useKibana: jest.fn().mockReturnValue({ + services: { http: { basePath: { prepend: jest.fn() } } }, + }), +})); jest.mock('../../hooks/use_app_toasts', () => ({ useAppToasts: jest.fn().mockReturnValue({ addError: jest.fn(), @@ -25,6 +30,9 @@ const importData = jest.fn().mockReturnValue({ success: true, errors: [] }); const file = new File(['file'], 'image1.png', { type: 'image/png' }); describe('ImportDataModal', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); test('renders correctly against snapshot', () => { const wrapper = render( { expect(overwriteCheckbox.checked).toBeFalsy(); expect(exceptionCheckbox.checked).toBeFalsy(); }); + + // TODO: might change after the UX feedback + test('should import file, with warnings', async () => { + const importWithWarning = jest.fn().mockReturnValue({ + ...importData(), + action_connectors_warnings: [ + { message: 'message', actionPath: 'path', buttonLabel: 'buttonLabel' }, + ], + }); + const wrapper = render( + 'successMessage')} + title="title" + /> + ); + const { queryByTestId } = wrapper; + await waitFor(() => { + fireEvent.change(queryByTestId('rule-file-picker') as HTMLInputElement, { + target: { files: [file] }, + }); + }); + await waitFor(() => { + fireEvent.click(queryByTestId('import-data-modal-button') as HTMLButtonElement); + }); + expect(wrapper).toMatchSnapshot(); + expect(queryByTestId('actionConnectorsWarningsCallOut')).toBeInTheDocument(); + expect(importWithWarning).toHaveBeenCalled(); + expect(closeModal).not.toHaveBeenCalled(); + expect(importComplete).not.toHaveBeenCalled(); + }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index bcf6565f8318b..2df499e6f88c2 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -82,12 +82,23 @@ export const ImportDataModalComponent = ({ const { http } = useKibana().services; const cleanupAndCloseModal = useCallback(() => { - setIsImporting(false); - setSelectedFiles(null); closeModal(); setOverwrite(false); setOverwriteExceptions(false); - }, [setIsImporting, setSelectedFiles, closeModal, setOverwrite, setOverwriteExceptions]); + }, [closeModal, setOverwrite, setOverwriteExceptions]); + + const onImportComplete = useCallback( + (callCleanup: boolean) => { + setIsImporting(false); + setSelectedFiles(null); + + if (callCleanup) { + importComplete(); + cleanupAndCloseModal(); + } + }, + [cleanupAndCloseModal, importComplete] + ); const importDataCallback = useCallback(async () => { if (selectedFiles != null) { @@ -101,7 +112,6 @@ export const ImportDataModalComponent = ({ overwriteExceptions, signal: abortCtrl.signal, }); - const showWarnings = !!warnings?.length; setActionConnectorsWarnings(warnings as WarningSchema[]); showToasterMessage({ @@ -113,31 +123,24 @@ export const ImportDataModalComponent = ({ addError, addSuccess, }); - - if (!showWarnings) { - importComplete(); - return cleanupAndCloseModal(); - } - - setIsImporting(false); - setSelectedFiles(null); + onImportComplete(!warnings?.length); } catch (error) { cleanupAndCloseModal(); addError(error, { title: errorMessage(1) }); } } }, [ - showExceptionsCheckBox, selectedFiles, + importData, overwrite, overwriteExceptions, - importData, + showExceptionsCheckBox, successMessage, errorMessage, failedDetailed, addError, addSuccess, - importComplete, + onImportComplete, cleanupAndCloseModal, ]); @@ -180,14 +183,15 @@ export const ImportDataModalComponent = ({ isLoading={isImporting} /> - {!!actionConnectorsWarnings?.length && - actionConnectorsWarnings.map(({ message, actionPath, buttonLabel }) => ( - - + {!!actionConnectorsWarnings?.length && ( + + {actionConnectorsWarnings.map(({ message, actionPath, buttonLabel }, index) => ( + {message} @@ -204,8 +208,9 @@ export const ImportDataModalComponent = ({ )} - - ))} + ))} + + )} {showCheckBox && ( From 75c3c7f5d4ebbff93f3250b2152f8c7599f4d0d7 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 18 Jan 2023 15:54:47 +0200 Subject: [PATCH 16/40] fixing export e2e test --- x-pack/plugins/security_solution/cypress/objects/rule.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugins/security_solution/cypress/objects/rule.ts b/x-pack/plugins/security_solution/cypress/objects/rule.ts index d60ef8dd54d51..f3f14b47f1b57 100644 --- a/x-pack/plugins/security_solution/cypress/objects/rule.ts +++ b/x-pack/plugins/security_solution/cypress/objects/rule.ts @@ -573,6 +573,11 @@ export const expectedExportedRule = (ruleResponse: Cypress.Response Date: Wed, 18 Jan 2023 18:00:15 +0200 Subject: [PATCH 17/40] fix api integrations --- .../basic/tests/export_rules.ts | 5 ++ .../group1/export_rules.ts | 5 ++ .../group10/import_rules.ts | 68 +++++++++++++++++ .../security_and_spaces/tests/import_rules.ts | 76 +++++++++++++++++++ 4 files changed, 154 insertions(+) diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts index 833b9ccb9ee1f..2d1ddd342a549 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/export_rules.ts @@ -87,6 +87,11 @@ export default ({ getService }: FtrProviderContext): void => { missing_exception_lists_count: 0, missing_rules: [], missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], }); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts index b30e16681d488..60af78ec6cff9 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group1/export_rules.ts @@ -88,6 +88,11 @@ export default ({ getService }: FtrProviderContext): void => { missing_exception_lists_count: 0, missing_rules: [], missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], }); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index 48c4408e8555e..058dce0039dcc 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -170,6 +170,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); it('should not import rules with actions when a user has no actions privileges', async () => { @@ -220,6 +224,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); }); @@ -367,6 +375,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -404,6 +416,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -430,6 +446,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -448,6 +468,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -480,6 +504,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -504,6 +532,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -568,6 +600,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -607,6 +643,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -722,6 +762,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -774,6 +818,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -834,6 +882,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -974,6 +1026,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -1045,6 +1101,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -1144,6 +1204,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -1274,6 +1338,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); }); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 3ff748fa5259d..5dc69d4a0013b 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -133,6 +133,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); it('should successfully import rules with actions when user has "read" actions privileges', async () => { @@ -168,6 +172,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); it('should not import rules with actions when a user has no actions privileges', async () => { @@ -218,6 +226,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); }); @@ -268,6 +280,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -305,6 +321,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -331,6 +351,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -349,6 +373,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -381,6 +409,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -405,6 +437,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -469,6 +505,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -508,6 +548,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -589,6 +633,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -623,6 +671,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -675,6 +727,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -735,6 +791,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -875,6 +935,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -946,6 +1010,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -1045,6 +1113,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -1175,6 +1247,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 1, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); }); From 16795bd7cb47bdbb5fd1452f45fc6dbac05c435e Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 18 Jan 2023 18:13:39 +0200 Subject: [PATCH 18/40] fix import rules --- .../basic/tests/import_rules.ts | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts index d32e93b481547..2630174529b75 100644 --- a/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/basic/tests/import_rules.ts @@ -73,6 +73,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -147,6 +151,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -170,6 +178,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -227,6 +239,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -245,6 +261,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -277,6 +297,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -301,6 +325,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -365,6 +393,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); @@ -404,6 +436,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); From 5e203633051e7e498f2ee344c6547c6b771ce9a5 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Thu, 19 Jan 2023 18:26:59 +0200 Subject: [PATCH 19/40] implement overwriting of connectors --- .../src/import_query_schema/index.ts | 4 +++- .../components/import_data_modal/index.tsx | 19 +++++++++++++++++++ .../import_data_modal/translations.ts | 7 ++++++- .../rule_management/api/api.ts | 7 ++++++- .../rule_management/logic/types.ts | 1 + .../pages/rule_management/index.tsx | 1 + .../api/rules/import_rules/route.ts | 1 + .../import_rule_action_connectors.ts | 6 +++--- .../logic/import/action_connectors/types.ts | 1 + .../group10/import_rules.ts | 4 ++++ .../group10/perform_bulk_action.ts | 5 +++++ 11 files changed, 50 insertions(+), 6 deletions(-) diff --git a/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.ts b/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.ts index ca0e35c4aa176..5b1ee38bb0855 100644 --- a/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.ts +++ b/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.ts @@ -14,6 +14,7 @@ export const importQuerySchema = t.exact( t.partial({ overwrite: DefaultStringBooleanFalse, overwrite_exceptions: DefaultStringBooleanFalse, + overwrite_action_connectors: DefaultStringBooleanFalse, as_new_list: DefaultStringBooleanFalse, }) ); @@ -21,9 +22,10 @@ export const importQuerySchema = t.exact( export type ImportQuerySchema = t.TypeOf; export type ImportQuerySchemaDecoded = Omit< ImportQuerySchema, - 'overwrite' | 'overwrite_exceptions' | 'as_new_list' + 'overwrite' | 'overwrite_exceptions' | 'as_new_list' | 'overwrite_action_connectors' > & { overwrite: boolean; overwrite_exceptions: boolean; + overwrite_action_connectors: boolean; as_new_list: boolean; }; diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index 2df499e6f88c2..d43960491b710 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -48,6 +48,7 @@ interface ImportDataModalProps { subtitle: string; successMessage: (totalCount: number) => string; title: string; + showActionConnectorsCheckBox?: boolean; } /** @@ -64,6 +65,7 @@ export const ImportDataModalComponent = ({ importData, showCheckBox = true, showExceptionsCheckBox = false, + showActionConnectorsCheckBox = false, showModal, submitBtnText, subtitle, @@ -74,6 +76,7 @@ export const ImportDataModalComponent = ({ const [isImporting, setIsImporting] = useState(false); const [overwrite, setOverwrite] = useState(false); const [overwriteExceptions, setOverwriteExceptions] = useState(false); + const [overwriteActionConnectors, setOverwriteActionConnectors] = useState(false); const { addError, addSuccess } = useAppToasts(); // TODO Ask if we use schema on the FE const [actionConnectorsWarnings, setActionConnectorsWarnings] = useState( @@ -85,6 +88,8 @@ export const ImportDataModalComponent = ({ closeModal(); setOverwrite(false); setOverwriteExceptions(false); + setOverwriteActionConnectors(false); + setActionConnectorsWarnings([]); }, [closeModal, setOverwrite, setOverwriteExceptions]); const onImportComplete = useCallback( @@ -110,6 +115,7 @@ export const ImportDataModalComponent = ({ fileToImport: selectedFiles[0], overwrite, overwriteExceptions, + overwriteActionConnectors, signal: abortCtrl.signal, }); setActionConnectorsWarnings(warnings as WarningSchema[]); @@ -134,6 +140,7 @@ export const ImportDataModalComponent = ({ importData, overwrite, overwriteExceptions, + overwriteActionConnectors, showExceptionsCheckBox, successMessage, errorMessage, @@ -156,6 +163,9 @@ export const ImportDataModalComponent = ({ setOverwriteExceptions((shouldOverwrite) => !shouldOverwrite); }, []); + const handleActionConnectorsCheckboxClick = useCallback(() => { + setOverwriteActionConnectors((shouldOverwrite) => !shouldOverwrite); + }, []); return ( <> {showModal && ( @@ -231,6 +241,15 @@ export const ImportDataModalComponent = ({ onChange={handleExceptionsCheckboxClick} /> )} + {showActionConnectorsCheckBox && ( + + )} )} diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts index 763396ae62d79..4f9501d8ff38a 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts @@ -20,7 +20,12 @@ export const OVERWRITE_EXCEPTIONS_LABEL = i18n.translate( defaultMessage: 'Overwrite existing exception lists with conflicting "list_id"', } ); - +export const OVERWRITE_ACTION_CONNECTORS_LABEL = i18n.translate( + 'xpack.securitySolution.detectionEngine.components.importRuleModal.overwriteActionConnectorsLabel', + { + defaultMessage: 'Overwrite existing action connectors with conflicting "id"', + } +); export const SUCCESSFULLY_IMPORTED_EXCEPTIONS = (totalExceptions: number) => i18n.translate( 'xpack.securitySolution.detectionEngine.components.importRuleModal.exceptionsSuccessLabel', diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts index c867b6b3fd792..aec165167db59 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.ts @@ -341,6 +341,7 @@ export const importRules = async ({ fileToImport, overwrite = false, overwriteExceptions = false, + overwriteActionConnectors = false, signal, }: ImportDataProps): Promise => { const formData = new FormData(); @@ -351,7 +352,11 @@ export const importRules = async ({ { method: 'POST', headers: { 'Content-Type': undefined }, - query: { overwrite, overwrite_exceptions: overwriteExceptions }, + query: { + overwrite, + overwrite_exceptions: overwriteExceptions, + overwrite_action_connectors: overwriteActionConnectors, + }, body: formData, signal, } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/types.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/types.ts index afc3da3ca6dd0..83d95de1877b8 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/types.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/logic/types.ts @@ -267,6 +267,7 @@ export interface ImportDataProps { fileToImport: File; overwrite?: boolean; overwriteExceptions?: boolean; + overwriteActionConnectors?: boolean; signal: AbortSignal; } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx index f02075809a46d..a26517e447d7a 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management_ui/pages/rule_management/index.tsx @@ -108,6 +108,7 @@ const RulesPageComponent: React.FC = () => { title={i18n.IMPORT_RULE} showExceptionsCheckBox showCheckBox + showActionConnectorsCheckBox /> diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index d44ee84be3881..6d714d9ed234c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -131,6 +131,7 @@ export const importRulesRoute = ( actionConnectors, actionsClient, actionsImporter, + overwrite: request.query.overwrite_action_connectors, }); // report on duplicate rules diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 83388ab6673ac..354bf3169f852 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -16,6 +16,7 @@ export const importRuleActionConnectors = async ({ actionConnectors, actionsClient, actionsImporter, + overwrite, }: ImportRuleActionConnectorsParams): Promise => { const importResult: ImportRuleActionConnectorsResult = { success: true, @@ -42,15 +43,14 @@ export const importRuleActionConnectors = async ({ ({ id }) => !storedActionConnectors.find((stored) => stored.id === id) ); - if (!actionConnectorsToImport.length) return importResult; + if (!actionConnectorsToImport.length && !overwrite) return importResult; const readStream = Readable.from(actionConnectors); - // TODO add overwrite & createNewCopies based on request query const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse = await actionsImporter.import({ readStream, - overwrite: false, + overwrite, createNewCopies: false, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts index 37d16c518adcd..4aab734dffdf3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -22,4 +22,5 @@ export interface ImportRuleActionConnectorsParams { actionConnectors: SavedObject[]; actionsClient: ActionsClient; actionsImporter: ISavedObjectsImporter; + overwrite: boolean; } diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index 058dce0039dcc..4035cc4b48402 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -135,6 +135,10 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); it('should successfully import rules with actions when user has "read" actions privileges', async () => { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts index 06e55fdfdda1b..ee9624f8acff3 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/perform_bulk_action.ts @@ -106,6 +106,11 @@ export default ({ getService }: FtrProviderContext): void => { missing_exception_lists_count: 0, missing_rules: [], missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 0, + missing_action_connection_count: 0, + missing_action_connections: [], }); }); From 0d0b61ecb450c7ff6c657773a9291c89adab6600 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Thu, 19 Jan 2023 19:34:04 +0200 Subject: [PATCH 20/40] fix tests --- .../src/import_query_schema/index.test.ts | 21 +++++++++++++++++++ .../rule_management/api/api.test.ts | 10 +++++++++ 2 files changed, 31 insertions(+) diff --git a/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.test.ts b/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.test.ts index d2f84b4ee3708..ab1f35c54b5db 100644 --- a/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.test.ts +++ b/packages/kbn-securitysolution-io-ts-types/src/import_query_schema/index.test.ts @@ -16,6 +16,7 @@ describe('importQuerySchema', () => { as_new_list: false, overwrite: true, overwrite_exceptions: true, + overwrite_action_connectors: true, }; const decoded = importQuerySchema.decode(payload); const checked = exactCheck(payload, decoded); @@ -30,6 +31,7 @@ describe('importQuerySchema', () => { as_new_list: false, overwrite: 'wrong', overwrite_exceptions: true, + overwrite_action_connectors: true, }; const decoded = importQuerySchema.decode(payload); const checked = exactCheck(payload, decoded); @@ -48,6 +50,7 @@ describe('importQuerySchema', () => { as_new_list: false, overwrite: true, overwrite_exceptions: 'wrong', + overwrite_action_connectors: true, }; const decoded = importQuerySchema.decode(payload); const checked = exactCheck(payload, decoded); @@ -58,6 +61,24 @@ describe('importQuerySchema', () => { ]); expect(message.schema).toEqual({}); }); + test('it should NOT validate a non boolean value for "overwrite_action_connectors"', () => { + const payload: Omit & { + overwrite_action_connectors: string; + } = { + as_new_list: false, + overwrite: true, + overwrite_exceptions: true, + overwrite_action_connectors: 'wrong', + }; + const decoded = importQuerySchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = foldLeftRight(checked); + + expect(getPaths(left(message.errors))).toEqual([ + 'Invalid value "wrong" supplied to "overwrite_action_connectors"', + ]); + expect(message.schema).toEqual({}); + }); test('it should NOT allow an extra key to be sent in', () => { const payload: ImportQuerySchema & { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.test.ts index bf1ac32d2bc78..a1d59efa4c30a 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.test.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_management/api/api.test.ts @@ -493,6 +493,7 @@ describe('Detections Rules API', () => { }, query: { overwrite: false, + overwrite_action_connectors: false, overwrite_exceptions: false, }, }); @@ -510,6 +511,7 @@ describe('Detections Rules API', () => { query: { overwrite: true, overwrite_exceptions: false, + overwrite_action_connectors: false, }, }); }); @@ -523,6 +525,10 @@ describe('Detections Rules API', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); const resp = await importRules({ fileToImport, signal: abortCtrl.signal }); expect(resp).toEqual({ @@ -533,6 +539,10 @@ describe('Detections Rules API', () => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); }); From f3e6161186db184c844d5b9dc5694d5847ff5d8d Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 24 Jan 2023 13:40:36 +0000 Subject: [PATCH 21/40] adding error handling --- .../import_data_modal/translations.ts | 2 +- .../api/rules/import_rules/route.ts | 13 +- .../import_rule_action_connectors.ts | 125 +++++++++++++----- .../logic/import/action_connectors/types.ts | 5 +- .../import/create_rules_stream_from_ndjson.ts | 3 +- .../group10/import_rules.ts | 37 ++++-- 6 files changed, 137 insertions(+), 48 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts index 4f9501d8ff38a..18c8e12617bd3 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts @@ -23,7 +23,7 @@ export const OVERWRITE_EXCEPTIONS_LABEL = i18n.translate( export const OVERWRITE_ACTION_CONNECTORS_LABEL = i18n.translate( 'xpack.securitySolution.detectionEngine.components.importRuleModal.overwriteActionConnectorsLabel', { - defaultMessage: 'Overwrite existing action connectors with conflicting "id"', + defaultMessage: 'Overwrite existing action connectors with conflicting "action_type_id"', } ); export const SUCCESSFULLY_IMPORTED_EXCEPTIONS = (totalExceptions: number) => diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index 6d714d9ed234c..4fc83ae2adbeb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -122,6 +122,13 @@ export const importRulesRoute = ( }); // import actions-connectors + const rulesActionsIds = rules.reduce((acc: string[], rule) => { + if (rule instanceof Error) return acc; + + const actionIds = rule.actions?.length ? rule.actions.map(({ id }) => id) : []; + + return [...new Set(actionIds)]; + }, []); const { successCount: actionConnectorSuccessCount, success: actionConnectorSuccess, @@ -131,6 +138,7 @@ export const importRulesRoute = ( actionConnectors, actionsClient, actionsImporter, + actionsIds: [...rulesActionsIds], overwrite: request.query.overwrite_action_connectors, }); @@ -143,8 +151,9 @@ export const importRulesRoute = ( actionSOClient ); - // TODO check why we need to parse rules?? - const parsedRules = migratedParsedObjectsWithoutDuplicateErrors; + const parsedRules = actionConnectorErrors.length + ? [] + : migratedParsedObjectsWithoutDuplicateErrors; // gather all exception lists that the imported rules reference const foundReferencedExceptionLists = await getReferencedExceptionLists({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 354bf3169f852..2c3a0bffaaf51 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -7,35 +7,91 @@ import { Readable } from 'stream'; import type { ActionResult } from '@kbn/actions-plugin/server'; -import type { SavedObject, SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; +import type { SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; +import type { SavedObject } from '@kbn/core-saved-objects-server'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import { mapSOErrorToRuleError } from './validate_action_connectors_import_result'; import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types'; +import type { BulkError } from '../../../../routes/utils'; +import { createBulkErrorObject } from '../../../../routes/utils'; + +const checkIfActionsHaveNoConnectors = (actionsIds: string[]): ImportRuleActionConnectorsResult => { + 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({ + statusCode: 404, + message: `${actionsIds.length} ${errorMessage} ${actionsIds.join(', ')}`, + }) + ); + return { + success: false, + errors, + successCount: 0, + warnings: [], + }; + } + return { + success: true, + errors: [], + successCount: 0, + warnings: [], + }; +}; + +const handleActionConnectorsErrors = (error: { + output: { statusCode: number; payload: { message: string } }; +}): ImportRuleActionConnectorsResult => { + const statusCode = error?.output.statusCode; + const message = error.output.payload.message; + if (statusCode === 403) + return { + success: false, + errors: [ + createBulkErrorObject({ + statusCode, + message: `You may not have actions privileges required to import rules with actions: ${message}`, + }), + ], + successCount: 0, + warnings: [], + }; + + return { + success: false, + errors: [ + createBulkErrorObject({ + statusCode, + message, + }), + ], + successCount: 0, + warnings: [], + }; +}; export const importRuleActionConnectors = async ({ actionConnectors, actionsClient, actionsImporter, + actionsIds, overwrite, }: ImportRuleActionConnectorsParams): Promise => { - const importResult: ImportRuleActionConnectorsResult = { - success: true, - errors: [], - successCount: 0, - warnings: [], - }; + if (!actionConnectors.length) return checkIfActionsHaveNoConnectors(actionsIds); - if (!actionConnectors.length) return importResult; let actionConnectorsToImport: SavedObject[] = actionConnectors; let storedActionConnectors: ActionResult[] | [] = []; - const actionConnectorIds = actionConnectors.map(({ id }) => id); try { - // getBulk throws 404 error if the saved_oject wasn't found - storedActionConnectors = await actionsClient.getBulk(actionConnectorIds); + // getBulk throws 404 error if the saved_oject wasn't found, is there a better + storedActionConnectors = await actionsClient.getBulk(actionsIds); } catch (error) { - // TODO ask if there's a better way - if (error.output.statusCode === 400) storedActionConnectors = []; + if (error.message.includes('404')) storedActionConnectors = []; + else return handleActionConnectorsErrors(error); } if (storedActionConnectors.length) @@ -43,22 +99,29 @@ export const importRuleActionConnectors = async ({ ({ id }) => !storedActionConnectors.find((stored) => stored.id === id) ); - if (!actionConnectorsToImport.length && !overwrite) return importResult; - - const readStream = Readable.from(actionConnectors); - - const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse = - await actionsImporter.import({ - readStream, - overwrite, - createNewCopies: false, - }); - - return { - success, - successCount, - successResults, - errors: mapSOErrorToRuleError(errors) || [], - warnings: (warnings as WarningSchema[]) || [], - }; + if (!actionConnectorsToImport.length && !overwrite) + return { + success: true, + errors: [], + successCount: 0, + warnings: [], + }; + try { + const readStream = Readable.from(actionConnectors); + const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse = + await actionsImporter.import({ + readStream, + overwrite, + createNewCopies: false, + }); + return { + success, + successCount, + successResults, + errors: mapSOErrorToRuleError(errors) || [], + warnings: (warnings as WarningSchema[]) || [], + }; + } catch (error) { + return handleActionConnectorsErrors(error); + } }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts index 4aab734dffdf3..9f92addefe67e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -4,9 +4,9 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import type { ISavedObjectsImporter } from '@kbn/core-saved-objects-server'; +import type { ISavedObjectsImporter, SavedObject } from '@kbn/core-saved-objects-server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; -import type { SavedObject, SavedObjectsImportSuccess } from '@kbn/core-saved-objects-common'; +import type { SavedObjectsImportSuccess } from '@kbn/core-saved-objects-common'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import type { BulkError } from '../../../../routes/utils'; @@ -22,5 +22,6 @@ export interface ImportRuleActionConnectorsParams { actionConnectors: SavedObject[]; actionsClient: ActionsClient; actionsImporter: ISavedObjectsImporter; + actionsIds: string[]; overwrite: boolean; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts index 53a30980b8334..4414b6063c473 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/create_rules_stream_from_ndjson.ts @@ -24,7 +24,7 @@ import type { ImportExceptionsListSchema, } from '@kbn/securitysolution-io-ts-list-types'; -import type { SavedObject } from '@kbn/core-saved-objects-common'; +import type { SavedObject } from '@kbn/core-saved-objects-server'; import { RuleToImport, validateRuleToImport, @@ -89,7 +89,6 @@ export const sortImports = (): Transform => { if (has('list_id', importItem) || has('item_id', importItem) || has('entries', importItem)) { return { ...acc, exceptions: [...acc.exceptions, importItem] }; } - // if (has('actionTypeId', importItem)) { if (has('attributes', importItem)) { return { ...acc, actionConnectors: [...acc.actionConnectors, importItem] }; } else { diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index 4035cc4b48402..d86cca8a967d6 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -168,15 +168,24 @@ export default ({ getService }: FtrProviderContext): void => { expect(body).to.eql({ errors: [], - success: true, - success_count: 1, - rules_count: 1, + success: false, + success_count: 0, + rules_count: 0, exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, - action_connectors_success: true, + action_connectors_success: false, action_connectors_success_count: 0, - action_connectors_errors: [], + action_connectors_errors: [ + { + error: { + message: + '1 connector is missing. Connector id missing is: a52bee60-9be1-11ed-8d37-5538d9e066c5', + status_code: 404, + }, + rule_id: '(unknown id)', + }, + ], action_connectors_warnings: [], }); }); @@ -228,7 +237,7 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, - action_connectors_success: true, + action_connectors_success: false, action_connectors_success_count: 0, action_connectors_errors: [], action_connectors_warnings: [], @@ -872,11 +881,11 @@ export default ({ getService }: FtrProviderContext): void => { expect(body).to.eql({ success: false, - success_count: 1, + success_count: 0, rules_count: 2, errors: [ { - rule_id: 'rule-2', + rule_id: '(unknown id)', // 'rule-2' do we need the rule_id error: { status_code: 404, message: '1 connector is missing. Connector id missing is: 123', @@ -886,9 +895,17 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, - action_connectors_success: true, + action_connectors_success: false, action_connectors_success_count: 0, - action_connectors_errors: [], + action_connectors_errors: [ + { + error: { + status_code: 404, + message: '1 connector is missing. Connector id missing is: 123', + }, + rule_id: '(unknown id)', + }, + ], action_connectors_warnings: [], }); }); From 42e9ae8ad78744cc9f54a6c9a291200941d7fb7e Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 25 Jan 2023 14:34:14 +0000 Subject: [PATCH 22/40] apply ux feedback --- .../__snapshots__/index.test.tsx.snap | 244 ++++++++++++++++-- .../action_connectors_warning.tsx | 56 ++++ .../import_data_modal/index.test.tsx | 44 +++- .../components/import_data_modal/index.tsx | 52 ++-- .../import_data_modal/translations.ts | 26 ++ 5 files changed, 368 insertions(+), 54 deletions(-) create mode 100644 x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap index 9b1106043cb6a..336f9789ca686 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap @@ -316,7 +316,7 @@ Object { class="euiPanel euiPanel--warning euiPanel--paddingMedium euiCallOut euiCallOut--warning emotion-euiPanel-none-m-warning-euiCallOut" data-test-subj="actionConnectorsWarningsCallOut" > -

1 connector imported -

+
- message + 1 connector has sensitive information that requires updates, review in +
+
+
+
+
+
+ +
+ +
+
+
+
+ + +
+
+
+
+ , + "container":
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; + +exports[`ImportDataModal should import file, with warnings but no action_connectors_success_count 1`] = ` +Object { + "asFragment": [Function], + "baseElement": +
+
+
+
+ +
+

+ title +

+
+
+
+
+

+ description +

+
+
+
+
+ +
+ +
+
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx new file mode 100644 index 0000000000000..11f6cd4b6cc49 --- /dev/null +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx @@ -0,0 +1,56 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import React from 'react'; +import type { FC } from 'react'; + +import { EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiLink, EuiText } from '@elastic/eui'; +import type { WarningSchema } from '../../../../common/detection_engine/schemas/response'; +import { useKibana } from '../../lib/kibana/kibana_react'; +import * as i18n from './translations'; + +interface ActionConnectorWarningsComponentProps { + actionConnectorsWarnings: WarningSchema[]; + importedActionConnectorsCount?: number; +} +const ActionConnectorWarningsComponent: FC = ({ + actionConnectorsWarnings, + importedActionConnectorsCount, +}) => { + const { http } = useKibana().services; + + if (!importedActionConnectorsCount) return null; + const { actionPath } = actionConnectorsWarnings[0]; + + return ( + + + + + {i18n.ACTION_CONNECTORS_WARNING_MESSAGE(actionConnectorsWarnings.length)} + + + {i18n.ACTION_CONNECTORS_WARNING_LINK} + + + + + + ); +}; + +ActionConnectorWarningsComponent.displayName = 'ActionConnectorWarningsComponent'; + +export const ActionConnectorWarnings = React.memo(ActionConnectorWarningsComponent); + +ActionConnectorWarnings.displayName = 'ActionConnectorWarnings'; diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx index 3675ac58e083b..2fdfed05cb3ca 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx @@ -178,13 +178,53 @@ describe('ImportDataModal', () => { expect(exceptionCheckbox.checked).toBeFalsy(); }); - // TODO: might change after the UX feedback + test('should import file, with warnings but no action_connectors_success_count', async () => { + const importWithWarning = jest.fn().mockReturnValue({ + ...importData(), + action_connectors_warnings: [ + { message: 'message', actionPath: 'path', buttonLabel: 'buttonLabel' }, + ], + action_connectors_success_count: 0, + }); + const wrapper = render( + 'successMessage')} + title="title" + /> + ); + const { queryByTestId } = wrapper; + await waitFor(() => { + fireEvent.change(queryByTestId('rule-file-picker') as HTMLInputElement, { + target: { files: [file] }, + }); + }); + await waitFor(() => { + fireEvent.click(queryByTestId('import-data-modal-button') as HTMLButtonElement); + }); + expect(wrapper).toMatchSnapshot(); + expect(queryByTestId('actionConnectorsWarningsCallOut')).not.toBeInTheDocument(); + expect(importWithWarning).toHaveBeenCalled(); + expect(closeModal).toHaveBeenCalled(); + expect(importComplete).toHaveBeenCalled(); + }); test('should import file, with warnings', async () => { const importWithWarning = jest.fn().mockReturnValue({ ...importData(), action_connectors_warnings: [ { message: 'message', actionPath: 'path', buttonLabel: 'buttonLabel' }, ], + action_connectors_success_count: 1, }); const wrapper = render( { expect(wrapper).toMatchSnapshot(); expect(queryByTestId('actionConnectorsWarningsCallOut')).toBeInTheDocument(); expect(importWithWarning).toHaveBeenCalled(); + expect(importComplete).toHaveBeenCalled(); expect(closeModal).not.toHaveBeenCalled(); - expect(importComplete).not.toHaveBeenCalled(); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index d43960491b710..901060c925457 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -9,11 +9,8 @@ import React, { useCallback, useState } from 'react'; import { EuiButton, EuiButtonEmpty, - EuiCallOut, EuiCheckbox, EuiFilePicker, - EuiFlexGroup, - EuiFlexItem, EuiModal, EuiModalBody, EuiModalFooter, @@ -31,7 +28,7 @@ import type { import { useAppToasts } from '../../hooks/use_app_toasts'; import * as i18n from './translations'; import { showToasterMessage } from './utils'; -import { useKibana } from '../../lib/kibana/kibana_react'; +import { ActionConnectorWarnings } from './action_connectors_warning'; interface ImportDataModalProps { checkBoxLabel: string; @@ -82,8 +79,9 @@ export const ImportDataModalComponent = ({ const [actionConnectorsWarnings, setActionConnectorsWarnings] = useState( [] ); - const { http } = useKibana().services; - + const [importedActionConnectorsCount, setImportedActionConnectorsCount] = useState< + number | undefined + >(0); const cleanupAndCloseModal = useCallback(() => { closeModal(); setOverwrite(false); @@ -96,6 +94,7 @@ export const ImportDataModalComponent = ({ (callCleanup: boolean) => { setIsImporting(false); setSelectedFiles(null); + importComplete(); if (callCleanup) { importComplete(); @@ -111,7 +110,11 @@ export const ImportDataModalComponent = ({ const abortCtrl = new AbortController(); try { - const { action_connectors_warnings: warnings, ...importResponse } = await importData({ + const { + action_connectors_warnings: warnings, + action_connectors_success_count: connectorsCount, + ...importResponse + } = await importData({ fileToImport: selectedFiles[0], overwrite, overwriteExceptions, @@ -119,6 +122,7 @@ export const ImportDataModalComponent = ({ signal: abortCtrl.signal, }); setActionConnectorsWarnings(warnings as WarningSchema[]); + setImportedActionConnectorsCount(connectorsCount); showToasterMessage({ importResponse, @@ -129,7 +133,7 @@ export const ImportDataModalComponent = ({ addError, addSuccess, }); - onImportComplete(!warnings?.length); + onImportComplete(!connectorsCount); } catch (error) { cleanupAndCloseModal(); addError(error, { title: errorMessage(1) }); @@ -193,34 +197,12 @@ export const ImportDataModalComponent = ({ isLoading={isImporting} /> - {!!actionConnectorsWarnings?.length && ( - - {actionConnectorsWarnings.map(({ message, actionPath, buttonLabel }, index) => ( - - - {message} - - {buttonLabel && ( - - - {buttonLabel} - - - )} - - ))} - - )} + + {showCheckBox && ( diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts index 18c8e12617bd3..ec360354b069d 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts @@ -45,3 +45,29 @@ export const IMPORT_FAILED = (totalExceptions: number) => 'Failed to import {totalExceptions} {totalExceptions, plural, =1 {exception} other {exceptions}}', } ); + +export const ACTION_CONNECTORS_WARNING_TITLE = (totalConnectors: number) => + i18n.translate( + 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningTitle', + { + values: { totalConnectors }, + defaultMessage: '{totalConnectors} connector imported', + } + ); + +export const ACTION_CONNECTORS_WARNING_MESSAGE = (totalConnectors: number) => + i18n.translate( + 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningMessage', + { + values: { totalConnectors }, + defaultMessage: + '{totalConnectors} {totalConnectors, plural, =1 {connector has} other {connectors have}} sensitive information that requires updates, review in ', + } + ); + +export const ACTION_CONNECTORS_WARNING_LINK = i18n.translate( + 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningLink', + { + defaultMessage: 'connectors', + } +); From c2c9ef7f4ea59238f659759d300fa74d894873bc Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 25 Jan 2023 14:41:30 +0000 Subject: [PATCH 23/40] add tests for overwrite checkbox --- .../__snapshots__/index.test.tsx.snap | 18 ++++++++-------- .../import_data_modal/index.test.tsx | 21 +++++++++++++++---- .../components/import_data_modal/index.tsx | 8 +++---- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap index 336f9789ca686..604be62266628 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/__snapshots__/index.test.tsx.snap @@ -98,8 +98,8 @@ Object { >
@@ -359,8 +359,8 @@ Object { >
@@ -581,8 +581,8 @@ Object { >
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx index 2fdfed05cb3ca..27e89623ace2e 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx @@ -100,17 +100,22 @@ describe('ImportDataModal', () => { successMessage={jest.fn((totalCount) => 'successMessage')} title="title" showExceptionsCheckBox={true} + showActionConnectorsCheckBox={true} /> ); const overwriteCheckbox: HTMLInputElement = queryByTestId( - 'import-data-modal-checkbox-label' + 'importDataModalCheckboxLabel' ) as HTMLInputElement; const exceptionCheckbox: HTMLInputElement = queryByTestId( - 'import-data-modal-exceptions-checkbox-label' + 'importDataModalExceptionsCheckboxLabel' + ) as HTMLInputElement; + const connectorsCheckbox: HTMLInputElement = queryByTestId( + 'importDataModalActionConnectorsCheckbox' ) as HTMLInputElement; await waitFor(() => fireEvent.click(overwriteCheckbox)); await waitFor(() => fireEvent.click(exceptionCheckbox)); + await waitFor(() => fireEvent.click(connectorsCheckbox)); await waitFor(() => fireEvent.change(queryByTestId('rule-file-picker') as HTMLInputElement, { @@ -119,6 +124,7 @@ describe('ImportDataModal', () => { ); expect(overwriteCheckbox.checked).toBeTruthy(); expect(exceptionCheckbox.checked).toBeTruthy(); + expect(connectorsCheckbox.checked).toBeTruthy(); await waitFor(() => { fireEvent.click(queryByTestId('import-data-modal-button') as HTMLButtonElement); @@ -128,6 +134,7 @@ describe('ImportDataModal', () => { expect(overwriteCheckbox.checked).toBeFalsy(); expect(exceptionCheckbox.checked).toBeFalsy(); + expect(connectorsCheckbox.checked).toBeFalsy(); }); test('should uncheck the selected checkboxes after closing the Flyout', async () => { const { queryByTestId, getAllByRole } = render( @@ -146,20 +153,25 @@ describe('ImportDataModal', () => { successMessage={jest.fn((totalCount) => 'successMessage')} title="title" showExceptionsCheckBox={true} + showActionConnectorsCheckBox={true} /> ); const closeButton = getAllByRole('button')[0]; const overwriteCheckbox: HTMLInputElement = queryByTestId( - 'import-data-modal-checkbox-label' + 'importDataModalCheckboxLabel' ) as HTMLInputElement; const exceptionCheckbox: HTMLInputElement = queryByTestId( - 'import-data-modal-exceptions-checkbox-label' + 'importDataModalExceptionsCheckboxLabel' + ) as HTMLInputElement; + const connectorsCheckbox: HTMLInputElement = queryByTestId( + 'importDataModalActionConnectorsCheckbox' ) as HTMLInputElement; await waitFor(() => fireEvent.click(overwriteCheckbox)); await waitFor(() => fireEvent.click(exceptionCheckbox)); + await waitFor(() => fireEvent.click(connectorsCheckbox)); await waitFor(() => fireEvent.change(queryByTestId('rule-file-picker') as HTMLInputElement, { @@ -176,6 +188,7 @@ describe('ImportDataModal', () => { expect(overwriteCheckbox.checked).toBeFalsy(); expect(exceptionCheckbox.checked).toBeFalsy(); + expect(connectorsCheckbox.checked).toBeFalsy(); }); test('should import file, with warnings but no action_connectors_success_count', async () => { diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index 901060c925457..f9c0f083a41d0 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -208,16 +208,16 @@ export const ImportDataModalComponent = ({ {showCheckBox && ( <> {showExceptionsCheckBox && ( Date: Thu, 26 Jan 2023 15:25:46 +0000 Subject: [PATCH 24/40] add export test --- .../export_rules_details_schema.mock.ts | 10 +- .../export_rules_details_schema.test.ts | 13 +- .../logic/export/get_export_all.test.ts | 174 +++++++++++++++++- .../get_export_rule_action_connectors.ts | 15 +- 4 files changed, 206 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts index 5e23fd3218917..9a26b1a09f066 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.mock.ts @@ -17,7 +17,6 @@ interface RuleDetailsMock { missingRules?: Array>; } -// TODO add the correct type DefaultActionConnectorDetails export const getActionConnectorDetailsMock = (): DefaultActionConnectorDetails => ({ exported_action_connector_count: 0, missing_action_connection_count: 0, @@ -25,6 +24,15 @@ export const getActionConnectorDetailsMock = (): DefaultActionConnectorDetails = excluded_action_connection_count: 0, excluded_action_connections: [], }); +export const getOutputDetailsSampleWithActionConnectors = (): ExportRulesDetails => ({ + ...getOutputDetailsSample(), + ...getExceptionExportDetailsMock(), + exported_action_connector_count: 1, + missing_action_connection_count: 0, + missing_action_connections: [], + excluded_action_connection_count: 0, + excluded_action_connections: [], +}); export const getOutputDetailsSample = (ruleDetails?: RuleDetailsMock): ExportRulesDetails => ({ exported_count: ruleDetails?.totalCount ?? 0, diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts index 907f23f1dc985..a91f59924ef72 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/export/export_rules_details_schema.test.ts @@ -18,13 +18,13 @@ import { exactCheck, foldLeftRight, getPaths } from '@kbn/securitysolution-io-ts import { getOutputDetailsSample, + getOutputDetailsSampleWithActionConnectors, getOutputDetailsSampleWithExceptions, } from './export_rules_details_schema.mock'; import type { ExportRulesDetails } from './export_rules_details_schema'; import { exportRulesDetailsWithExceptionsAndConnectorsSchema } from './export_rules_details_schema'; -// TODO add tests for connectors -describe('exportRulesDetailsWithExceptionsSchema', () => { +describe('exportRulesDetailsWithExceptionsAndConnectorsSchema', () => { test('it should validate export details response', () => { const payload = getOutputDetailsSample(); const decoded = exportRulesDetailsWithExceptionsAndConnectorsSchema.decode(payload); @@ -45,6 +45,15 @@ describe('exportRulesDetailsWithExceptionsSchema', () => { expect(message.schema).toEqual(payload); }); + test('it should validate export details with action connectors details response', () => { + const payload = getOutputDetailsSampleWithActionConnectors(); + const decoded = exportRulesDetailsWithExceptionsAndConnectorsSchema.decode(payload); + const checked = exactCheck(payload, decoded); + const message = pipe(checked, foldLeftRight); + + expect(getPaths(left(message.errors))).toEqual([]); + expect(message.schema).toEqual(payload); + }); test('it should strip out extra keys', () => { const payload: ExportRulesDetails & { extraKey?: string; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts index 093baded97fd3..192ee361d874f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_all.test.ts @@ -26,10 +26,10 @@ import type { loggingSystemMock } from '@kbn/core/server/mocks'; import { requestContextMock } from '../../../routes/__mocks__/request_context'; import { savedObjectsExporterMock } from '@kbn/core-saved-objects-import-export-server-mocks'; import { mockRouter } from '@kbn/core-http-router-server-mocks'; +import { Readable } from 'stream'; const exceptionsClient = getExceptionListClientMock(); -// TODO add tests for connectors describe('getExportAll', () => { let logger: ReturnType; const { clients } = requestContextMock.createTools(); @@ -155,4 +155,176 @@ describe('getExportAll', () => { actionConnectors: '', }); }); + test('it will export with rule and action connectors', async () => { + const rulesClient = rulesClientMock.create(); + const result = getFindResultWithSingleHit(); + const alert = { + ...getRuleMock(getQueryRuleParams()), + actions: [ + { + group: 'default', + id: '123', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + }, + ], + }; + + alert.params = { + ...alert.params, + filters: [{ query: { match_phrase: { 'host.name': 'some-host' } } }], + threat: getThreatMock(), + meta: { someMeta: 'someField' }, + timelineId: 'some-timeline-id', + timelineTitle: 'some-timeline-title', + }; + result.data = [alert]; + rulesClient.find.mockResolvedValue(result); + let eventCount = 0; + const readable = new Readable({ + objectMode: true, + read() { + if (eventCount === 0) { + eventCount += 1; + return this.push({ + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-11T11:30:31.683Z', + created_at: '2023-01-11T11:30:31.683Z', + version: 'WzE2MDYsMV0=', + attributes: { + actionTypeId: '.slack', + name: 'slack', + isMissingSecrets: true, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }); + } + if (eventCount === 1) { + eventCount += 1; + return this.push({ + exportedCount: 1, + missingRefCount: 0, + missingReferences: [], + excludedObjectsCount: 0, + excludedObjects: [], + }); + } + return this.push(null); + }, + }); + + const exporterMockWithConnector = { + exportByObjects: () => jest.fn().mockReturnValueOnce(readable), + + exportByTypes: jest.fn(), + }; + const exports = await getExportAll( + rulesClient, + exceptionsClient, + clients.savedObjectsClient, + logger, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + exporterMockWithConnector as any, + requestMock + ); + const rulesJson = JSON.parse(exports.rulesNdjson); + const detailsJson = JSON.parse(exports.exportDetails); + const actionConnectorsJSON = JSON.parse(exports.actionConnectors); + expect(rulesJson).toEqual({ + author: ['Elastic'], + actions: [ + { + group: 'default', + id: '123', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + }, + ], + building_block_type: 'default', + created_at: '2019-12-13T16:40:33.400Z', + updated_at: '2019-12-13T16:40:33.400Z', + created_by: 'elastic', + description: 'Detecting root and admin users', + enabled: true, + false_positives: [], + filters: [{ query: { match_phrase: { 'host.name': 'some-host' } } }], + from: 'now-6m', + id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', + immutable: false, + index: ['auditbeat-*', 'filebeat-*', 'packetbeat-*', 'winlogbeat-*'], + interval: '5m', + rule_id: 'rule-1', + language: 'kuery', + license: 'Elastic License', + output_index: '.siem-signals', + max_signals: 10000, + risk_score: 50, + risk_score_mapping: [], + name: 'Detect Root/Admin Users', + query: 'user.name: root or user.name: admin', + references: ['http://example.com', 'https://example.com'], + related_integrations: [], + required_fields: [], + setup: '', + timeline_id: 'some-timeline-id', + timeline_title: 'some-timeline-title', + meta: { someMeta: 'someField' }, + severity: 'high', + severity_mapping: [], + updated_by: 'elastic', + tags: [], + to: 'now', + type: 'query', + threat: getThreatMock(), + throttle: 'rule', + note: '# Investigative notes', + version: 1, + exceptions_list: getListArrayMock(), + }); + expect(detailsJson).toEqual({ + exported_exception_list_count: 1, + exported_exception_list_item_count: 1, + exported_count: 4, + exported_rules_count: 1, + missing_exception_list_item_count: 0, + missing_exception_list_items: [], + missing_exception_lists: [], + missing_exception_lists_count: 0, + missing_rules: [], + missing_rules_count: 0, + excluded_action_connection_count: 0, + excluded_action_connections: [], + exported_action_connector_count: 1, + missing_action_connection_count: 0, + missing_action_connections: [], + }); + expect(actionConnectorsJSON).toEqual({ + attributes: { + actionTypeId: '.slack', + config: {}, + isMissingSecrets: true, + name: 'slack', + secrets: {}, + }, + coreMigrationVersion: '8.7.0', + created_at: '2023-01-11T11:30:31.683Z', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + migrationVersion: { + action: '8.3.0', + }, + references: [], + type: 'action', + updated_at: '2023-01-11T11:30:31.683Z', + version: 'WzE2MDYsMV0=', + }); + }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts index 332d5b1fc9cd6..04472ff6b4598 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts @@ -5,11 +5,12 @@ * 2.0. */ import type { KibanaRequest } from '@kbn/core-http-server'; -import type { SavedObject, SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common'; +import type { SavedObjectTypeIdTuple } from '@kbn/core-saved-objects-common'; import type { SavedObjectsExportResultDetails, ISavedObjectsExporter, SavedObjectsExportExcludedObject, + SavedObject, } from '@kbn/core-saved-objects-server'; import { createConcatStream, createMapStream, createPromiseFromStreams } from '@kbn/utils'; import type { RuleResponse } from '../../../../../../common/detection_engine/rule_schema'; @@ -55,7 +56,6 @@ export const getRuleActionConnectorsForExport = async ( actionConnectorDetails: defaultActionConnectorDetails, }; - // TODO combine line 60 and 64 const actionsIds = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; if (!actionsIds.length) return exportedActionConnectors; @@ -66,6 +66,17 @@ export const getRuleActionConnectorsForExport = async ( request, }); + if (!actionDetails) { + exportedActionConnectors.actionConnectorDetails = { + exported_action_connector_count: 0, + missing_action_connection_count: actionsIds.length, + missing_action_connections: [], // TODO: check how to generate SO + excluded_action_connection_count: 0, + excluded_action_connections: [], + }; + return exportedActionConnectors; + } + const actionsConnectorsToExport: SavedObject[] = await createPromiseFromStreams([ actionDetails, createMapStream((obj: SavedObject | SavedObjectsExportResultDetails) => { From fb1a282a99abc500bc3d44ed70f86b0fa9d8e874 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Fri, 27 Jan 2023 17:37:58 +0000 Subject: [PATCH 25/40] fixing read only access --- .../model/import/rule_to_import.mock.ts | 38 +++ .../cypress/screens/alerts_detection_rules.ts | 7 +- .../cypress/tasks/alerts_detection_rules.ts | 8 +- .../api/rules/import_rules/route.ts | 27 +-- .../import_rule_action_connectors.test.ts | 111 +++++++++ .../import_rule_action_connectors.ts | 21 +- .../logic/import/action_connectors/types.ts | 3 +- .../group10/import_rules.ts | 219 +++++++++++++++--- .../security_and_spaces/tests/import_rules.ts | 12 +- 9 files changed, 387 insertions(+), 59 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts diff --git a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.mock.ts b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.mock.ts index d1dc9e8ac4663..cec58d1c5fdc6 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.mock.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/rule_management/model/import/rule_to_import.mock.ts @@ -80,3 +80,41 @@ export const getImportThreatMatchRulesSchemaMock = (ruleId = 'rule-1'): RuleToIm }, ], }); + +export const webHookConnector = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: false, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', +}; + +export const ruleWithConnectorNdJSON = (): string => { + const items = [ + { + ...getImportRulesSchemaMock(), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + action_type_id: '.webhook', + params: {}, + }, + ], + }, + webHookConnector, + ]; + const stringOfExceptions = items.map((item) => JSON.stringify(item)); + + return stringOfExceptions.join('\n'); +}; diff --git a/x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts b/x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts index a90609fc30531..870faea26a360 100644 --- a/x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts +++ b/x-pack/plugins/security_solution/cypress/screens/alerts_detection_rules.ts @@ -123,10 +123,13 @@ export const TOASTER_BODY = '[data-test-subj="globalToastList"] [data-test-subj= export const TOASTER_ERROR_BTN = '[data-test-subj="errorToastBtn"]'; -export const RULE_IMPORT_OVERWRITE_CHECKBOX = '[id="import-data-modal-checkbox-label"]'; +export const RULE_IMPORT_OVERWRITE_CHECKBOX = '[id="importDataModalCheckboxLabel"]'; export const RULE_IMPORT_OVERWRITE_EXCEPTIONS_CHECKBOX = - '[id="import-data-modal-exceptions-checkbox-label"]'; + '[id="importDataModalExceptionsCheckboxLabel"]'; + +export const RULE_IMPORT_OVERWRITE_CONNECTORS_CHECKBOX = + '[id="importDataModalActionConnectorsCheckbox"]'; export const RULES_TAGS_POPOVER_BTN = '[data-test-subj="tagsDisplayPopoverButton"]'; diff --git a/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts b/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts index 821b4c415fdd8..83492ef9c4437 100644 --- a/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts +++ b/x-pack/plugins/security_solution/cypress/tasks/alerts_detection_rules.ts @@ -56,6 +56,7 @@ import { MODAL_CONFIRMATION_CANCEL_BTN, MODAL_CONFIRMATION_BODY, RULE_SEARCH_FIELD, + RULE_IMPORT_OVERWRITE_CONNECTORS_CHECKBOX, } from '../screens/alerts_detection_rules'; import { EUI_CHECKBOX } from '../screens/common/controls'; import { ALL_ACTIONS } from '../screens/rule_details'; @@ -382,13 +383,18 @@ const selectOverwriteExceptionsRulesImport = () => { .pipe(($el) => $el.trigger('click')) .should('be.checked'); }; - +const selectOverwriteConnectorsRulesImport = () => { + cy.get(RULE_IMPORT_OVERWRITE_CONNECTORS_CHECKBOX) + .pipe(($el) => $el.trigger('click')) + .should('be.checked'); +}; export const importRulesWithOverwriteAll = (rulesFile: string) => { cy.get(RULE_IMPORT_MODAL).click(); cy.get(INPUT_FILE).should('exist'); cy.get(INPUT_FILE).trigger('click', { force: true }).attachFile(rulesFile).trigger('change'); selectOverwriteRulesImport(); selectOverwriteExceptionsRulesImport(); + selectOverwriteConnectorsRulesImport(); cy.get(RULE_IMPORT_MODAL_BUTTON).last().click({ force: true }); cy.get(INPUT_FILE).should('not.exist'); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts index 4fc83ae2adbeb..27dd69f960761 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.ts @@ -120,15 +120,16 @@ export const importRulesRoute = ( overwrite: request.query.overwrite_exceptions, maxExceptionsImportSize: objectLimit, }); + // report on duplicate rules + const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = + getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite); - // import actions-connectors - const rulesActionsIds = rules.reduce((acc: string[], rule) => { - if (rule instanceof Error) return acc; - - const actionIds = rule.actions?.length ? rule.actions.map(({ id }) => id) : []; + const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( + parsedObjectsWithoutDuplicateErrors, + actionSOClient + ); - return [...new Set(actionIds)]; - }, []); + // import actions-connectors const { successCount: actionConnectorSuccessCount, success: actionConnectorSuccess, @@ -138,19 +139,9 @@ export const importRulesRoute = ( actionConnectors, actionsClient, actionsImporter, - actionsIds: [...rulesActionsIds], + rules: migratedParsedObjectsWithoutDuplicateErrors, overwrite: request.query.overwrite_action_connectors, }); - - // report on duplicate rules - const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = - getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite); - - const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( - parsedObjectsWithoutDuplicateErrors, - actionSOClient - ); - const parsedRules = actionConnectorErrors.length ? [] : migratedParsedObjectsWithoutDuplicateErrors; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts new file mode 100644 index 0000000000000..ab46765183072 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts @@ -0,0 +1,111 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { actionsClientMock } from '@kbn/actions-plugin/server/actions_client.mock'; +import { + getImportRulesSchemaMock, + webHookConnector, +} from '../../../../../../../common/detection_engine/rule_management/model/import/rule_to_import.mock'; +import { importRuleActionConnectors } from './import_rule_action_connectors'; +import { coreMock } from '@kbn/core/server/mocks'; + +const rules = [ + { + ...getImportRulesSchemaMock(), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + action_type_id: '.webhook', + params: {}, + }, + ], + }, +]; +const actionConnectors = [webHookConnector]; +const actionsClient = actionsClientMock.create(); +actionsClient.getBulk.mockResolvedValue([]); +const core = coreMock.createRequestHandlerContext(); +core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ + import: () => ({ + success: true, + successCount: 1, + successResults: [], + errors: [], + warnings: [], + }), +}); +const actionsImporter = core.savedObjects.getImporter; + +describe('checkRuleExceptionReferences', () => { + it('should return import 1 connector successfully', async () => { + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter() as never, + rules, + overwrite: false, + }); + + expect(res).toEqual({ + success: true, + successCount: 1, + successResults: [], + errors: [], + warnings: [], + }); + }); + it('should return import 2 connector successfully', async () => { + core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ + import: () => ({ + success: true, + successCount: 2, + successResults: [], + errors: [], + warnings: [], + }), + }); + const actionsImporter2 = core.savedObjects.getImporter; + const ruleWith2Connectors = [ + { + ...getImportRulesSchemaMock(), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + }, + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + }, + ], + }, + ]; + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter2() as never, + rules: ruleWith2Connectors, + overwrite: false, + }); + + expect(res).toEqual({ + success: true, + successCount: 2, + successResults: [], + errors: [], + warnings: [], + }); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 2c3a0bffaaf51..2a135d4a9a2db 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -9,13 +9,17 @@ import { Readable } from 'stream'; import type { ActionResult } from '@kbn/actions-plugin/server'; import type { SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; import type { SavedObject } from '@kbn/core-saved-objects-server'; +import type { RuleToImport } from '../../../../../../../common/detection_engine/rule_management'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import { mapSOErrorToRuleError } from './validate_action_connectors_import_result'; import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types'; import type { BulkError } from '../../../../routes/utils'; import { createBulkErrorObject } from '../../../../routes/utils'; -const checkIfActionsHaveNoConnectors = (actionsIds: string[]): ImportRuleActionConnectorsResult => { +const checkIfActionsHaveNoConnectors = ( + actionsIds: string[], + ruleIds: string +): ImportRuleActionConnectorsResult => { if (actionsIds && actionsIds.length) { const errors: BulkError[] = []; const errorMessage = @@ -26,6 +30,7 @@ const checkIfActionsHaveNoConnectors = (actionsIds: string[]): ImportRuleActionC createBulkErrorObject({ statusCode: 404, message: `${actionsIds.length} ${errorMessage} ${actionsIds.join(', ')}`, + ruleId: ruleIds, }) ); return { @@ -73,15 +78,25 @@ const handleActionConnectorsErrors = (error: { warnings: [], }; }; +const getActionConnectorRules = (rules: Array) => + 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 importRuleActionConnectors = async ({ actionConnectors, actionsClient, actionsImporter, - actionsIds, + rules, overwrite, }: ImportRuleActionConnectorsParams): Promise => { - if (!actionConnectors.length) return checkIfActionsHaveNoConnectors(actionsIds); + const actionConnectorRules = getActionConnectorRules(rules); + const actionsIds: string[] = Object.keys(actionConnectorRules); + const ruleIds: string = [...new Set(...Object.values(actionConnectorRules))].join(); + + if (!actionConnectors.length) return checkIfActionsHaveNoConnectors(actionsIds, ruleIds); let actionConnectorsToImport: SavedObject[] = actionConnectors; let storedActionConnectors: ActionResult[] | [] = []; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts index 9f92addefe67e..39e6cfe4d3910 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -7,6 +7,7 @@ import type { ISavedObjectsImporter, SavedObject } from '@kbn/core-saved-objects-server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { SavedObjectsImportSuccess } from '@kbn/core-saved-objects-common'; +import type { RuleToImport } from '../../../../../../../common/detection_engine/rule_management'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import type { BulkError } from '../../../../routes/utils'; @@ -22,6 +23,6 @@ export interface ImportRuleActionConnectorsParams { actionConnectors: SavedObject[]; actionsClient: ActionsClient; actionsImporter: ISavedObjectsImporter; - actionsIds: string[]; + rules: Array; overwrite: boolean; } diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index d86cca8a967d6..29f0afd9ab0b4 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -141,7 +141,7 @@ export default ({ getService }: FtrProviderContext): void => { action_connectors_warnings: [], }); }); - it('should successfully import rules with actions when user has "read" actions privileges', async () => { + it('should not import rules with actions when user has "read" actions privileges', async () => { // create a new action const { body: hookAction } = await supertest .post('/api/actions/action') @@ -159,18 +159,49 @@ export default ({ getService }: FtrProviderContext): void => { }, ], }; + const ruleWithConnector = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: true, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; + const { body } = await supertestWithoutAuth .post(`${DETECTION_ENGINE_RULES_URL}/_import`) .auth(ROLES.hunter, 'changeme') .set('kbn-xsrf', 'true') - .attach('file', ruleToNdjson(simpleRule), 'rules.ndjson') + .attach( + 'file', + Buffer.from(toNdJsonString([simpleRule, ruleWithConnector])), + 'rules.ndjson' + ) .expect(200); expect(body).to.eql({ - errors: [], + errors: [ + { + error: { + message: + 'You may not have actions privileges required to import rules with actions: Unable to bulk_create action', + status_code: 403, + }, + rule_id: '(unknown id)', + }, + ], success: false, success_count: 0, - rules_count: 0, + rules_count: 1, exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, @@ -180,8 +211,8 @@ export default ({ getService }: FtrProviderContext): void => { { error: { message: - '1 connector is missing. Connector id missing is: a52bee60-9be1-11ed-8d37-5538d9e066c5', - status_code: 404, + 'You may not have actions privileges required to import rules with actions: Unable to bulk_create action', + status_code: 403, }, rule_id: '(unknown id)', }, @@ -207,11 +238,33 @@ export default ({ getService }: FtrProviderContext): void => { }, ], }; + const ruleWithConnector = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: true, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; + const { body } = await supertestWithoutAuth .post(`${DETECTION_ENGINE_RULES_URL}/_import`) .auth(ROLES.hunter_no_actions, 'changeme') .set('kbn-xsrf', 'true') - .attach('file', ruleToNdjson(simpleRule), 'rules.ndjson') + .attach( + 'file', + Buffer.from(toNdJsonString([simpleRule, ruleWithConnector])), + 'rules.ndjson' + ) .expect(200); expect(body).to.eql({ success: false, @@ -225,13 +278,6 @@ export default ({ getService }: FtrProviderContext): void => { }, rule_id: '(unknown id)', }, - { - error: { - message: `1 connector is missing. Connector id missing is: ${hookAction.id}`, - status_code: 404, - }, - rule_id: 'rule-1', - }, ], rules_count: 1, exceptions_errors: [], @@ -239,7 +285,16 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_success_count: 0, action_connectors_success: false, action_connectors_success_count: 0, - action_connectors_errors: [], + action_connectors_errors: [ + { + error: { + message: + 'You may not have actions privileges required to import rules with actions: Unauthorized to get actions', + status_code: 403, + }, + rule_id: '(unknown id)', + }, + ], action_connectors_warnings: [], }); }); @@ -741,10 +796,22 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, + action_connectors_success: false, + action_connectors_success_count: 0, + action_connectors_warnings: [], + action_connectors_errors: [ + { + rule_id: 'rule-1', + error: { + status_code: 404, + message: '1 connector is missing. Connector id missing is: 123', + }, + }, + ], }); }); - it('should be able to import a rule with an action connector that exists', async () => { + it('should be able to import a rule with an action connector that exists and return warning if there are missing secrets', async () => { // create a new action const { body: hookAction } = await supertest .post('/api/actions/action') @@ -762,10 +829,32 @@ export default ({ getService }: FtrProviderContext): void => { }, ], }; + const ruleWithConnector = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: true, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; + const { body } = await supertest .post(`${DETECTION_ENGINE_RULES_URL}/_import`) .set('kbn-xsrf', 'true') - .attach('file', ruleToNdjson(simpleRule), 'rules.ndjson') + .attach( + 'file', + Buffer.from(toNdJsonString([simpleRule, ruleWithConnector])), + 'rules.ndjson' + ) .expect(200); expect(body).to.eql({ success: true, @@ -776,9 +865,16 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_success: true, exceptions_success_count: 0, action_connectors_success: true, - action_connectors_success_count: 0, + action_connectors_success_count: 1, action_connectors_errors: [], - action_connectors_warnings: [], + action_connectors_warnings: [ + { + actionPath: '/app/management/insightsAndAlerting/triggersActionsConnectors', + buttonLabel: 'Go to connectors', + message: '1 connector has sensitive information that require updates.', + type: 'action_required', + }, + ], }); }); @@ -795,7 +891,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: hookAction.id, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', action_type_id: hookAction.actionTypeId, params: {}, }, @@ -807,32 +903,71 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: hookAction.id, - action_type_id: hookAction.actionTypeId, + id: 'f4e74ab0-9e59-11ed-a3db-f9134a9ce951', + action_type_id: '.index', params: {}, }, ], }; + + const connector1 = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: false, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; + const connector2 = { + id: 'f4e74ab0-9e59-11ed-a3db-f9134a9ce951', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.index', + name: 'index', + isMissingSecrets: false, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; const rule1String = JSON.stringify(rule1); const rule2String = JSON.stringify(rule2); - const buffer = Buffer.from(`${rule1String}\n${rule2String}\n`); + const connector12String = JSON.stringify(connector1); + const connector22String = JSON.stringify(connector2); + const buffer = Buffer.from( + `${rule1String}\n${rule2String}\n${connector12String}\n${connector22String}\n` + ); const { body } = await supertest - .post(`${DETECTION_ENGINE_RULES_URL}/_import`) + .post(`${DETECTION_ENGINE_RULES_URL}/_import?overwrite=true`) .set('kbn-xsrf', 'true') .attach('file', buffer, 'rules.ndjson') .expect(200); expect(body).to.eql({ success: true, - success_count: 2, + success_count: 4, rules_count: 2, errors: [], exceptions_errors: [], exceptions_success: true, exceptions_success_count: 0, action_connectors_success: true, - action_connectors_success_count: 0, + action_connectors_success_count: 2, action_connectors_errors: [], action_connectors_warnings: [], }); @@ -851,7 +986,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: hookAction.id, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', action_type_id: hookAction.actionTypeId, params: {}, }, @@ -863,15 +998,35 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: '123', // <-- This does not exist - action_type_id: hookAction.actionTypeId, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aa22', // <-- This does not exist + action_type_id: 'invalid type', params: {}, }, ], }; + + const connector = { + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + type: 'action', + updated_at: '2023-01-25T14:35:52.852Z', + created_at: '2023-01-25T14:35:52.852Z', + version: 'WzUxNTksMV0=', + attributes: { + actionTypeId: '.webhook', + name: 'webhook', + isMissingSecrets: false, + config: {}, + secrets: {}, + }, + references: [], + migrationVersion: { action: '8.3.0' }, + coreMigrationVersion: '8.7.0', + }; const rule1String = JSON.stringify(rule1); const rule2String = JSON.stringify(rule2); - const buffer = Buffer.from(`${rule1String}\n${rule2String}\n`); + const connector2String = JSON.stringify(connector); + + const buffer = Buffer.from(`${rule1String}\n${rule2String}\n${connector2String}\n`); const { body } = await supertest .post(`${DETECTION_ENGINE_RULES_URL}/_import`) @@ -885,7 +1040,7 @@ export default ({ getService }: FtrProviderContext): void => { rules_count: 2, errors: [ { - rule_id: '(unknown id)', // 'rule-2' do we need the rule_id + rule_id: 'rule-2', error: { status_code: 404, message: '1 connector is missing. Connector id missing is: 123', @@ -903,7 +1058,7 @@ export default ({ getService }: FtrProviderContext): void => { status_code: 404, message: '1 connector is missing. Connector id missing is: 123', }, - rule_id: '(unknown id)', + rule_id: 'rule-2', }, ], action_connectors_warnings: [], diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts index 5dc69d4a0013b..135bf49c15ff9 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/import_rules.ts @@ -210,7 +210,7 @@ export default ({ getService }: FtrProviderContext): void => { { error: { message: - 'You may not have actions privileges required to import rules with actions: Unauthorized to get actions', + 'You may not have actions privileges required to import rules with actions: Unable to bulk_create action', status_code: 403, }, rule_id: '(unknown id)', @@ -635,7 +635,15 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 0, - action_connectors_errors: [], + action_connectors_errors: [ + { + rule_id: 'rule-1', + error: { + status_code: 404, + message: '1 connector is missing. Connector id missing is: 123', + }, + }, + ], action_connectors_warnings: [], }); }); From 9d2aa0be60956d29c1927cfddb5ccfa832ff228d Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Mon, 30 Jan 2023 19:48:03 +0000 Subject: [PATCH 26/40] handle migration errors, overwrite rules, handle not importing connectors already existing --- .../server/rules_client/methods/update.ts | 17 ++- .../action_connectors_warning.tsx | 2 +- .../components/import_data_modal/index.tsx | 2 +- .../rule_management/logic/crud/patch_rules.ts | 3 + .../import_rule_action_connectors.ts | 144 ++++++------------ .../logic/import/action_connectors/types.ts | 15 +- .../import/action_connectors/utils/index.ts | 84 ++++++++++ ...alidate_action_connectors_import_result.ts | 24 --- .../logic/import/import_rules_utils.ts | 1 + .../group10/import_rules.ts | 86 ++++++++--- 10 files changed, 226 insertions(+), 152 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts diff --git a/x-pack/plugins/alerting/server/rules_client/methods/update.ts b/x-pack/plugins/alerting/server/rules_client/methods/update.ts index 8fa3855b95707..fe5ea4d18188e 100644 --- a/x-pack/plugins/alerting/server/rules_client/methods/update.ts +++ b/x-pack/plugins/alerting/server/rules_client/methods/update.ts @@ -38,22 +38,23 @@ export interface UpdateOptions { throttle?: string | null; notifyWhen?: RuleNotifyWhenType | null; }; + skipActionConnectorsValidations?: boolean; } export async function update( context: RulesClientContext, - { id, data }: UpdateOptions + { id, data, skipActionConnectorsValidations }: UpdateOptions ): Promise> { return await retryIfConflicts( context.logger, `rulesClient.update('${id}')`, - async () => await updateWithOCC(context, { id, data }) + async () => await updateWithOCC(context, { id, data, skipActionConnectorsValidations }) ); } async function updateWithOCC( context: RulesClientContext, - { id, data }: UpdateOptions + { id, data, skipActionConnectorsValidations }: UpdateOptions ): Promise> { let alertSavedObject: SavedObject; @@ -99,7 +100,11 @@ async function updateWithOCC( context.ruleTypeRegistry.ensureRuleTypeEnabled(alertSavedObject.attributes.alertTypeId); - const updateResult = await updateAlert(context, { id, data }, alertSavedObject); + const updateResult = await updateAlert( + context, + { id, data, skipActionConnectorsValidations }, + alertSavedObject + ); await Promise.all([ alertSavedObject.attributes.apiKey @@ -138,7 +143,7 @@ async function updateWithOCC( async function updateAlert( context: RulesClientContext, - { id, data }: UpdateOptions, + { id, data, skipActionConnectorsValidations }: UpdateOptions, { attributes, version }: SavedObject ): Promise> { const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId); @@ -156,7 +161,7 @@ async function updateAlert( // Validate const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate?.params); - await validateActions(context, ruleType, data); + if (!skipActionConnectorsValidations) await validateActions(context, ruleType, data); // Throw error if schedule interval is less than the minimum and we are enforcing it const intervalInMs = parseDuration(data.schedule.interval); diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx index 11f6cd4b6cc49..23287e661ba10 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning.tsx @@ -22,7 +22,7 @@ const ActionConnectorWarningsComponent: FC { const { http } = useKibana().services; - if (!importedActionConnectorsCount) return null; + if (!importedActionConnectorsCount || !actionConnectorsWarnings.length) return null; const { actionPath } = actionConnectorsWarnings[0]; return ( diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx index f9c0f083a41d0..1420c8d83169a 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.tsx @@ -133,7 +133,7 @@ export const ImportDataModalComponent = ({ addError, addSuccess, }); - onImportComplete(!connectorsCount); + onImportComplete(!warnings?.length); } catch (error) { cleanupAndCloseModal(); addError(error, { title: errorMessage(1) }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts index 656366ec9f92b..1678ecefa4c1c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/patch_rules.ts @@ -16,12 +16,14 @@ export interface PatchRulesOptions { rulesClient: RulesClient; nextParams: PatchRuleRequestBody; existingRule: RuleAlertType | null | undefined; + skipActionConnectorsValidations?: boolean; } export const patchRules = async ({ rulesClient, existingRule, nextParams, + skipActionConnectorsValidations, }: PatchRulesOptions): Promise | null> => { if (existingRule == null) { return null; @@ -32,6 +34,7 @@ export const patchRules = async ({ const update = await rulesClient.update({ id: existingRule.id, data: patchedRule, + skipActionConnectorsValidations, }); if (nextParams.throttle !== undefined) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 2a135d4a9a2db..731303658de45 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -6,78 +6,18 @@ */ import { Readable } from 'stream'; -import type { ActionResult } from '@kbn/actions-plugin/server'; import type { SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; import type { SavedObject } from '@kbn/core-saved-objects-server'; +import { pick } from 'lodash'; import type { RuleToImport } from '../../../../../../../common/detection_engine/rule_management'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; -import { mapSOErrorToRuleError } from './validate_action_connectors_import_result'; +import { + handleActionsHaveNoConnectors, + mapSOErrorToRuleError, + returnErroredImportResult, +} from './utils'; import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types'; -import type { BulkError } from '../../../../routes/utils'; -import { createBulkErrorObject } from '../../../../routes/utils'; -const checkIfActionsHaveNoConnectors = ( - actionsIds: string[], - ruleIds: string -): ImportRuleActionConnectorsResult => { - 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({ - statusCode: 404, - message: `${actionsIds.length} ${errorMessage} ${actionsIds.join(', ')}`, - ruleId: ruleIds, - }) - ); - return { - success: false, - errors, - successCount: 0, - warnings: [], - }; - } - return { - success: true, - errors: [], - successCount: 0, - warnings: [], - }; -}; - -const handleActionConnectorsErrors = (error: { - output: { statusCode: number; payload: { message: string } }; -}): ImportRuleActionConnectorsResult => { - const statusCode = error?.output.statusCode; - const message = error.output.payload.message; - if (statusCode === 403) - return { - success: false, - errors: [ - createBulkErrorObject({ - statusCode, - message: `You may not have actions privileges required to import rules with actions: ${message}`, - }), - ], - successCount: 0, - warnings: [], - }; - - return { - success: false, - errors: [ - createBulkErrorObject({ - statusCode, - message, - }), - ], - successCount: 0, - warnings: [], - }; -}; const getActionConnectorRules = (rules: Array) => rules.reduce((acc: { [actionsIds: string]: string[] }, rule) => { if (rule instanceof Error) return acc; @@ -92,37 +32,53 @@ export const importRuleActionConnectors = async ({ rules, overwrite, }: ImportRuleActionConnectorsParams): Promise => { - const actionConnectorRules = getActionConnectorRules(rules); - const actionsIds: string[] = Object.keys(actionConnectorRules); - const ruleIds: string = [...new Set(...Object.values(actionConnectorRules))].join(); + try { + const actionConnectorRules = getActionConnectorRules(rules); + const actionsIds: string[] = Object.keys(actionConnectorRules); - if (!actionConnectors.length) return checkIfActionsHaveNoConnectors(actionsIds, ruleIds); + if (!actionsIds.length) + return { + success: true, + errors: [], + successCount: 0, + warnings: [], + }; - let actionConnectorsToImport: SavedObject[] = actionConnectors; - let storedActionConnectors: ActionResult[] | [] = []; + const ruleIds: string = [...new Set(...Object.values(actionsIds))].join(); - try { - // getBulk throws 404 error if the saved_oject wasn't found, is there a better - storedActionConnectors = await actionsClient.getBulk(actionsIds); - } catch (error) { - if (error.message.includes('404')) storedActionConnectors = []; - else return handleActionConnectorsErrors(error); - } + if (overwrite && !actionConnectors.length) + return handleActionsHaveNoConnectors(actionsIds, ruleIds); - if (storedActionConnectors.length) - actionConnectorsToImport = actionConnectors.filter( - ({ id }) => !storedActionConnectors.find((stored) => stored.id === id) - ); + let actionConnectorsToImport: SavedObject[] = actionConnectors; + if (!overwrite) { + const storedActionIds: string[] = await (await actionsClient.getAll()).map(({ id }) => id); - if (!actionConnectorsToImport.length && !overwrite) - return { - success: true, - errors: [], - successCount: 0, - warnings: [], - }; - try { - const readStream = Readable.from(actionConnectors); + const newIdsToAdd = actionsIds.filter((id) => !storedActionIds.includes(id)); + + // if new action-connectors don't have exported connectors will fail with missing + if (actionConnectors.length < newIdsToAdd.length) { + const actionConnectorsIds = actionConnectors.map(({ id }) => id); + const missingActionConnector = newIdsToAdd.filter( + (id) => !actionConnectorsIds.includes(id) + ); + const missingActionRules = pick(actionConnectorRules, [...missingActionConnector]); + + const missingRuleIds: string = [...new Set(...Object.values(missingActionRules))].join(); + + return handleActionsHaveNoConnectors(missingActionConnector, missingRuleIds); + } + // Incase connectors imported before + actionConnectorsToImport = actionConnectors.filter(({ id }) => newIdsToAdd.includes(id)); + } + if (!actionConnectorsToImport.length) + return { + success: true, + errors: [], + successCount: 0, + warnings: [], + }; + + const readStream = Readable.from(actionConnectorsToImport); const { success, successCount, successResults, warnings, errors }: SavedObjectsImportResponse = await actionsImporter.import({ readStream, @@ -133,10 +89,10 @@ export const importRuleActionConnectors = async ({ success, successCount, successResults, - errors: mapSOErrorToRuleError(errors) || [], + errors: errors ? mapSOErrorToRuleError(errors) : [], warnings: (warnings as WarningSchema[]) || [], }; } catch (error) { - return handleActionConnectorsErrors(error); + return returnErroredImportResult(error); } }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts index 39e6cfe4d3910..8eb915dc83612 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -6,7 +6,10 @@ */ import type { ISavedObjectsImporter, SavedObject } from '@kbn/core-saved-objects-server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; -import type { SavedObjectsImportSuccess } from '@kbn/core-saved-objects-common'; +import type { + SavedObjectsImportFailure, + SavedObjectsImportSuccess, +} from '@kbn/core-saved-objects-common'; import type { RuleToImport } from '../../../../../../../common/detection_engine/rule_management'; import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import type { BulkError } from '../../../../routes/utils'; @@ -26,3 +29,13 @@ export interface ImportRuleActionConnectorsParams { rules: Array; overwrite: boolean; } + +export interface SOError { + output: { statusCode: number; payload: { message: string } }; +} + +export interface ConflictError { + type: string; +} + +export type ErrorType = SOError | ConflictError | SavedObjectsImportFailure | Error; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts new file mode 100644 index 0000000000000..b69a932394bd2 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts @@ -0,0 +1,84 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { SavedObjectsImportFailure } from '@kbn/core-saved-objects-common'; +import type { BulkError } from '../../../../../routes/utils'; +import { createBulkErrorObject } from '../../../../../routes/utils'; +import type { ConflictError, ErrorType, ImportRuleActionConnectorsResult, SOError } from '../types'; + +export const returnErroredImportResult = (error: ErrorType): ImportRuleActionConnectorsResult => ({ + success: false, + errors: [handleActionConnectorsErrors(error)], + successCount: 0, + warnings: [], +}); + +export const handleActionsHaveNoConnectors = ( + actionsIds: string[], + ruleIds: string +): ImportRuleActionConnectorsResult => { + 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({ + 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 = ''; + if ('output' in error) { + statusCode = (error as SOError).output.statusCode; + message = (error as SOError).output.payload?.message; + } + switch (statusCode) { + case null: + return createBulkErrorObject({ + statusCode: 500, + message: (error as ConflictError)?.type === 'conflict' ? 'There is a conflict' : '', // TODO : choose a message when conflict happen + }); + + case 403: + return createBulkErrorObject({ + id, + statusCode, + message: `You may not have actions privileges required to import rules with actions: ${message}`, + }); + + default: + return createBulkErrorObject({ + id, + statusCode, + message, + }); + } +}; + +export const mapSOErrorToRuleError = (errors: SavedObjectsImportFailure[]): BulkError[] => { + return errors.map(({ id, error }) => handleActionConnectorsErrors(error, id)); +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts deleted file mode 100644 index 23f06a407e018..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/validate_action_connectors_import_result.ts +++ /dev/null @@ -1,24 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import type { - SavedObjectsImportFailure, - SavedObjectsImportUnknownError, -} from '@kbn/core-saved-objects-common'; -import type { BulkError } from '../../../../routes/utils'; -import { createBulkErrorObject } from '../../../../routes/utils'; - -export const mapSOErrorToRuleError = (errors?: SavedObjectsImportFailure[]): BulkError[] => { - if (!errors) return []; - return errors.map(({ id, error }) => - createBulkErrorObject({ - id, - statusCode: (error as SavedObjectsImportUnknownError).statusCode, - message: (error as SavedObjectsImportUnknownError).message, - }) - ); -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts index d7dda57213545..99eb2c8dacce6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/import_rules_utils.ts @@ -140,6 +140,7 @@ export const importRules = async ({ ...parsedRule, exceptions_list: [...exceptions], }, + skipActionConnectorsValidations, }); resolve({ rule_id: parsedRule.rule_id, diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts index 29f0afd9ab0b4..169fe074604fa 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/group10/import_rules.ts @@ -141,6 +141,7 @@ export default ({ getService }: FtrProviderContext): void => { action_connectors_warnings: [], }); }); + it('should not import rules with actions when user has "read" actions privileges', async () => { // create a new action const { body: hookAction } = await supertest @@ -153,7 +154,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: hookAction.id, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', action_type_id: hookAction.actionTypeId, params: {}, }, @@ -232,14 +233,14 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: hookAction.id, + id: 'cabc78e0-9031-11ed-b076-53cc4d57axy1', action_type_id: hookAction.actionTypeId, params: {}, }, ], }; const ruleWithConnector = { - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57axy1', type: 'action', updated_at: '2023-01-25T14:35:52.852Z', created_at: '2023-01-25T14:35:52.852Z', @@ -810,27 +811,20 @@ export default ({ getService }: FtrProviderContext): void => { ], }); }); - - it('should be able to import a rule with an action connector that exists and return warning if there are missing secrets', async () => { - // create a new action - const { body: hookAction } = await supertest - .post('/api/actions/action') - .set('kbn-xsrf', 'true') - .send(getWebHookAction()) - .expect(200); + it('should give single connector warning back if we have a single connector missing secret', async () => { const simpleRule: ReturnType = { ...getSimpleRule('rule-1'), actions: [ { group: 'default', - id: hookAction.id, - action_type_id: hookAction.actionTypeId, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf9', + action_type_id: '.webhook', params: {}, }, ], }; const ruleWithConnector = { - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf9', type: 'action', updated_at: '2023-01-25T14:35:52.852Z', created_at: '2023-01-25T14:35:52.852Z', @@ -846,7 +840,6 @@ export default ({ getService }: FtrProviderContext): void => { migrationVersion: { action: '8.3.0' }, coreMigrationVersion: '8.7.0', }; - const { body } = await supertest .post(`${DETECTION_ENGINE_RULES_URL}/_import`) .set('kbn-xsrf', 'true') @@ -856,6 +849,7 @@ export default ({ getService }: FtrProviderContext): void => { 'rules.ndjson' ) .expect(200); + expect(body).to.eql({ success: true, success_count: 1, @@ -866,7 +860,6 @@ export default ({ getService }: FtrProviderContext): void => { exceptions_success_count: 0, action_connectors_success: true, action_connectors_success_count: 1, - action_connectors_errors: [], action_connectors_warnings: [ { actionPath: '/app/management/insightsAndAlerting/triggersActionsConnectors', @@ -875,10 +868,50 @@ export default ({ getService }: FtrProviderContext): void => { type: 'action_required', }, ], + action_connectors_errors: [], + }); + }); + + it('should be able to import a rule with an action connector that exists', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + const simpleRule: ReturnType = { + ...getSimpleRule('rule-1'), + actions: [ + { + group: 'default', + id: hookAction.id, + action_type_id: hookAction.actionTypeId, + params: {}, + }, + ], + }; + + const { body } = await supertest + .post(`${DETECTION_ENGINE_RULES_URL}/_import`) + .set('kbn-xsrf', 'true') + .attach('file', ruleToNdjson(simpleRule), 'rules.ndjson') + .expect(200); + expect(body).to.eql({ + success: true, + success_count: 1, + rules_count: 1, + errors: [], + exceptions_errors: [], + exceptions_success: true, + exceptions_success_count: 0, + action_connectors_success: true, + action_connectors_success_count: 0, + action_connectors_errors: [], + action_connectors_warnings: [], }); }); - it('should be able to import 2 rules with action connectors that exist', async () => { + it('should be able to import 2 rules with action connectors', async () => { // create a new action const { body: hookAction } = await supertest .post('/api/actions/action') @@ -891,7 +924,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57abc6', action_type_id: hookAction.actionTypeId, params: {}, }, @@ -911,7 +944,7 @@ export default ({ getService }: FtrProviderContext): void => { }; const connector1 = { - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57abc6', type: 'action', updated_at: '2023-01-25T14:35:52.852Z', created_at: '2023-01-25T14:35:52.852Z', @@ -960,7 +993,7 @@ export default ({ getService }: FtrProviderContext): void => { expect(body).to.eql({ success: true, - success_count: 4, + success_count: 2, rules_count: 2, errors: [], exceptions_errors: [], @@ -986,7 +1019,7 @@ export default ({ getService }: FtrProviderContext): void => { actions: [ { group: 'default', - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aayo', action_type_id: hookAction.actionTypeId, params: {}, }, @@ -999,14 +1032,14 @@ export default ({ getService }: FtrProviderContext): void => { { group: 'default', id: 'cabc78e0-9031-11ed-b076-53cc4d57aa22', // <-- This does not exist - action_type_id: 'invalid type', + action_type_id: '.index', params: {}, }, ], }; const connector = { - id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aayo', type: 'action', updated_at: '2023-01-25T14:35:52.852Z', created_at: '2023-01-25T14:35:52.852Z', @@ -1022,6 +1055,7 @@ export default ({ getService }: FtrProviderContext): void => { migrationVersion: { action: '8.3.0' }, coreMigrationVersion: '8.7.0', }; + const rule1String = JSON.stringify(rule1); const rule2String = JSON.stringify(rule2); const connector2String = JSON.stringify(connector); @@ -1043,7 +1077,8 @@ export default ({ getService }: FtrProviderContext): void => { rule_id: 'rule-2', error: { status_code: 404, - message: '1 connector is missing. Connector id missing is: 123', + message: + '1 connector is missing. Connector id missing is: cabc78e0-9031-11ed-b076-53cc4d57aa22', }, }, ], @@ -1056,7 +1091,8 @@ export default ({ getService }: FtrProviderContext): void => { { error: { status_code: 404, - message: '1 connector is missing. Connector id missing is: 123', + message: + '1 connector is missing. Connector id missing is: cabc78e0-9031-11ed-b076-53cc4d57aa22', }, rule_id: 'rule-2', }, From b299057159f56df5228581dba93d0c74326597bd Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Mon, 30 Jan 2023 20:24:47 +0000 Subject: [PATCH 27/40] fix test --- .../public/common/components/import_data_modal/index.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx index 27e89623ace2e..853415e59eecf 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/index.test.tsx @@ -228,7 +228,7 @@ describe('ImportDataModal', () => { expect(wrapper).toMatchSnapshot(); expect(queryByTestId('actionConnectorsWarningsCallOut')).not.toBeInTheDocument(); expect(importWithWarning).toHaveBeenCalled(); - expect(closeModal).toHaveBeenCalled(); + expect(closeModal).not.toHaveBeenCalled(); expect(importComplete).toHaveBeenCalled(); }); test('should import file, with warnings', async () => { From efbda93b4f2c18bf21302fafe6dee9693b117f58 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 31 Jan 2023 11:04:17 +0000 Subject: [PATCH 28/40] add tests to import connectors and utils --- .../import_rule_action_connectors.test.ts | 307 +++++++++++++++++- .../import_rule_action_connectors.ts | 4 +- .../import/action_connectors/utils/index.ts | 7 +- 3 files changed, 299 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts index ab46765183072..785597753c40c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.test.ts @@ -25,23 +25,71 @@ const rules = [ ], }, ]; +const rulesWithoutActions = [ + { + ...getImportRulesSchemaMock(), + actions: [], + }, +]; const actionConnectors = [webHookConnector]; const actionsClient = actionsClientMock.create(); -actionsClient.getBulk.mockResolvedValue([]); +actionsClient.getAll.mockResolvedValue([]); const core = coreMock.createRequestHandlerContext(); -core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ - import: () => ({ - success: true, - successCount: 1, - successResults: [], - errors: [], - warnings: [], - }), -}); -const actionsImporter = core.savedObjects.getImporter; describe('checkRuleExceptionReferences', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it('should show an error message when the user has a Read Actions permission and stops the importing ', async () => { + // jest.resetAllMocks(); + const newCore = coreMock.createRequestHandlerContext(); + const error = { + output: { payload: { message: 'Unable to bulk_create action' }, statusCode: 403 }, + }; + newCore.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ + import: jest.fn().mockImplementation(() => { + throw error; + }), + }); + const actionsImporter2 = newCore.savedObjects.getImporter; + + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter2() as never, + rules, + overwrite: false, + }); + + expect(res).toEqual({ + success: false, + successCount: 0, + errors: [ + { + error: { + message: + 'You may not have actions privileges required to import rules with actions: Unable to bulk_create action', + status_code: 403, + }, + rule_id: '(unknown id)', + }, + ], + warnings: [], + }); + }); + it('should return import 1 connector successfully', async () => { + core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ + import: jest.fn().mockResolvedValue({ + success: true, + successCount: 1, + successResults: [], + errors: [], + warnings: [], + }), + }); + const actionsImporter = core.savedObjects.getImporter; + const res = await importRuleActionConnectors({ actionConnectors, actionsClient, @@ -58,17 +106,18 @@ describe('checkRuleExceptionReferences', () => { warnings: [], }); }); - it('should return import 2 connector successfully', async () => { + it('should return import 1 connector successfully only if id is duplicated', async () => { core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ - import: () => ({ + import: jest.fn().mockResolvedValue({ success: true, - successCount: 2, + successCount: 1, successResults: [], errors: [], warnings: [], }), }); - const actionsImporter2 = core.savedObjects.getImporter; + const actionsImporter = core.savedObjects.getImporter; + const ruleWith2Connectors = [ { ...getImportRulesSchemaMock(), @@ -95,17 +144,241 @@ describe('checkRuleExceptionReferences', () => { const res = await importRuleActionConnectors({ actionConnectors, actionsClient, - actionsImporter: actionsImporter2() as never, + actionsImporter: actionsImporter() as never, rules: ruleWith2Connectors, overwrite: false, }); expect(res).toEqual({ success: true, - successCount: 2, + successCount: 1, successResults: [], errors: [], warnings: [], }); }); + + it('should show an error message when the user has an old imported rule with a missing connector data', async () => { + const actionsImporter = core.savedObjects.getImporter; + + const res = await importRuleActionConnectors({ + actionConnectors: [], + actionsClient, + actionsImporter: actionsImporter() as never, + rules, + overwrite: false, + }); + + expect(res).toEqual({ + success: false, + successCount: 0, + errors: [ + { + error: { + message: + '1 connector is missing. Connector id missing is: cabc78e0-9031-11ed-b076-53cc4d57aaf1', + status_code: 404, + }, + rule_id: 'rule-1', + }, + ], + warnings: [], + }); + }); + it('should show an error message when the user has an old imported rule with a 2 missing connectors data', async () => { + const actionsImporter = core.savedObjects.getImporter; + + const res = await importRuleActionConnectors({ + actionConnectors: [], + actionsClient, + actionsImporter: actionsImporter() as never, + rules: [ + { + ...getImportRulesSchemaMock(), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + action_type_id: '.webhook', + params: {}, + }, + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf2', + action_type_id: '.webhook', + params: {}, + }, + ], + }, + ], + overwrite: false, + }); + + expect(res).toEqual({ + success: false, + successCount: 0, + errors: [ + { + error: { + message: + '2 connectors are missing. Connector ids missing are: cabc78e0-9031-11ed-b076-53cc4d57aaf1, cabc78e0-9031-11ed-b076-53cc4d57aaf2', + status_code: 404, + }, + rule_id: 'rule-1', + }, + ], + warnings: [], + }); + }); + it('should show an error message when the user has 2 imported rules with a 2 missing connectors data', async () => { + const actionsImporter = core.savedObjects.getImporter; + + const res = await importRuleActionConnectors({ + actionConnectors: [], + actionsClient, + actionsImporter: actionsImporter() as never, + rules: [ + { + ...getImportRulesSchemaMock(), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + action_type_id: '.webhook', + params: {}, + }, + ], + }, + { + ...getImportRulesSchemaMock('rule-2'), + actions: [ + { + group: 'default', + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf2', + action_type_id: '.webhook', + params: {}, + }, + ], + }, + ], + overwrite: false, + }); + + expect(res).toEqual({ + success: false, + successCount: 0, + errors: [ + { + error: { + message: + '2 connectors are missing. Connector ids missing are: cabc78e0-9031-11ed-b076-53cc4d57aaf1, cabc78e0-9031-11ed-b076-53cc4d57aaf2', + status_code: 404, + }, + rule_id: 'rule-1,rule-2', + }, + ], + warnings: [], + }); + }); + + it('should skip importing the action-connectors if the actions array is empty, even if the user has exported-connectors in the file', async () => { + core.savedObjects.getImporter = jest.fn().mockReturnValue({ + import: jest.fn().mockResolvedValue({ + success: true, + successCount: 2, + successResults: [], + errors: [], + warnings: [], + }), + }); + const actionsImporter2 = core.savedObjects.getImporter; + const actionsImporter2Import = actionsImporter2().import; + + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter2Import as never, + rules: rulesWithoutActions, + overwrite: false, + }); + + expect(res).toEqual({ + success: true, + successCount: 0, + errors: [], + warnings: [], + }); + expect(actionsImporter2Import).not.toBeCalled(); + }); + + it('should skip importing the action-connectors if all connectors have been imported/created before', async () => { + actionsClient.getAll.mockResolvedValue([ + { + actionTypeId: '.webhook', + name: 'webhook', + isPreconfigured: true, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + referencedByCount: 1, + isDeprecated: false, + }, + ]); + const actionsImporter2 = core.savedObjects.getImporter; + const actionsImporter2Import = actionsImporter2().import; + + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter2Import as never, + rules, + overwrite: false, + }); + + expect(res).toEqual({ + success: true, + successCount: 0, + errors: [], + warnings: [], + }); + expect(actionsImporter2Import).not.toBeCalled(); + }); + + it('should not skip importing the action-connectors if all connectors have been imported/created before when overwrite is true', async () => { + core.savedObjects.getImporter = jest.fn().mockReturnValueOnce({ + import: jest.fn().mockResolvedValue({ + success: true, + successCount: 1, + successResults: [], + errors: [], + warnings: [], + }), + }); + const actionsImporter = core.savedObjects.getImporter; + + actionsClient.getAll.mockResolvedValue([ + { + actionTypeId: '.webhook', + name: 'webhook', + isPreconfigured: true, + id: 'cabc78e0-9031-11ed-b076-53cc4d57aaf1', + referencedByCount: 1, + isDeprecated: false, + }, + ]); + + const res = await importRuleActionConnectors({ + actionConnectors, + actionsClient, + actionsImporter: actionsImporter() as never, + rules, + overwrite: true, + }); + + expect(res).toEqual({ + success: true, + successCount: 1, + errors: [], + warnings: [], + successResults: [], + }); + }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 731303658de45..9c03303f838a6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -63,7 +63,9 @@ export const importRuleActionConnectors = async ({ ); const missingActionRules = pick(actionConnectorRules, [...missingActionConnector]); - const missingRuleIds: string = [...new Set(...Object.values(missingActionRules))].join(); + const missingRuleIds: string = [ + ...new Set(Object.values(missingActionRules).flat()), + ].join(); return handleActionsHaveNoConnectors(missingActionConnector, missingRuleIds); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts index b69a932394bd2..ce3cd9061ca14 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts @@ -60,7 +60,12 @@ export const handleActionConnectorsErrors = (error: ErrorType, id?: string): Bul case null: return createBulkErrorObject({ statusCode: 500, - message: (error as ConflictError)?.type === 'conflict' ? 'There is a conflict' : '', // TODO : choose a message when conflict happen + message: + (error as ConflictError)?.type === 'conflict' + ? 'There is a conflict' // TODO : choose a message when conflict happen + : (error as Error).message + ? (error as Error).message + : '', }); case 403: From 87b604dac8c8b75258fd05059799d3700996d7b8 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 31 Jan 2023 12:11:29 +0000 Subject: [PATCH 29/40] add test for skipActionConnectorsValidations in update rules --- .../server/rules_client/tests/update.test.ts | 250 ++++++++++++++++++ 1 file changed, 250 insertions(+) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts index 0de571c72916b..234bee1666a47 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/update.test.ts @@ -2020,6 +2020,256 @@ describe('update()', () => { expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); }); + test('should not validate actions when skipActionConnectorsValidations is true even if connector has missing secrets and update the rule', async () => { + // Reset from default behaviour + actionsClient.getBulk.mockReset(); + actionsClient.getBulk.mockResolvedValue([ + { + id: '1', + actionTypeId: 'test', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: true, + name: 'email connector', + isPreconfigured: false, + isDeprecated: false, + }, + { + id: '2', + actionTypeId: 'test2', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: false, + name: 'another email connector', + isPreconfigured: false, + isDeprecated: false, + }, + { + id: 'preconfigured', + actionTypeId: 'test', + config: { + from: 'me@me.com', + hasAuth: false, + host: 'hello', + port: 22, + secure: null, + service: null, + }, + isMissingSecrets: false, + name: 'preconfigured email connector', + isPreconfigured: true, + isDeprecated: false, + }, + ]); + actionsClient.isPreconfigured.mockReset(); + actionsClient.isPreconfigured.mockReturnValueOnce(false); + actionsClient.isPreconfigured.mockReturnValueOnce(true); + actionsClient.isPreconfigured.mockReturnValueOnce(true); + unsecuredSavedObjectsClient.create.mockResolvedValueOnce({ + id: '1', + type: 'alert', + attributes: { + enabled: true, + schedule: { interval: '1m' }, + params: { + bar: true, + }, + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'default', + actionRef: 'preconfigured:preconfigured', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'custom', + actionRef: 'preconfigured:preconfigured', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + ], + notifyWhen: 'onActiveAlert', + scheduledTaskId: 'task-123', + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }, + references: [ + { + name: 'action_0', + type: 'action', + id: '1', + }, + { + name: 'param:soRef_0', + type: 'someSavedObjectType', + id: '9', + }, + ], + }); + const result = await rulesClient.update({ + id: '1', + data: { + schedule: { interval: '1m' }, + name: 'abc', + tags: ['foo'], + params: { + bar: true, + }, + throttle: null, + notifyWhen: 'onActiveAlert', + actions: [ + { + group: 'default', + id: '1', + params: { + foo: true, + }, + }, + { + group: 'default', + id: 'preconfigured', + params: { + foo: true, + }, + }, + { + group: 'custom', + id: 'preconfigured', + params: { + foo: true, + }, + }, + ], + }, + skipActionConnectorsValidations: true, + }); + + expect(unsecuredSavedObjectsClient.create).toHaveBeenNthCalledWith( + 1, + 'alert', + { + actions: [ + { + group: 'default', + actionRef: 'action_0', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'default', + actionRef: 'preconfigured:preconfigured', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + { + group: 'custom', + actionRef: 'preconfigured:preconfigured', + actionTypeId: 'test', + params: { + foo: true, + }, + }, + ], + alertTypeId: 'myType', + apiKey: null, + apiKeyOwner: null, + consumer: 'myApp', + enabled: true, + meta: { versionApiKeyLastmodified: 'v7.10.0' }, + name: 'abc', + notifyWhen: 'onActiveAlert', + params: { bar: true }, + schedule: { interval: '1m' }, + scheduledTaskId: 'task-123', + tags: ['foo'], + throttle: null, + updatedAt: '2019-02-12T21:01:22.479Z', + updatedBy: 'elastic', + }, + { + id: '1', + overwrite: true, + references: [{ id: '1', name: 'action_0', type: 'action' }], + version: '123', + } + ); + + expect(result).toMatchInlineSnapshot(` + Object { + "actions": Array [ + Object { + "actionTypeId": "test", + "group": "default", + "id": "1", + "params": Object { + "foo": true, + }, + }, + Object { + "actionTypeId": "test", + "group": "default", + "id": "preconfigured", + "params": Object { + "foo": true, + }, + }, + Object { + "actionTypeId": "test", + "group": "custom", + "id": "preconfigured", + "params": Object { + "foo": true, + }, + }, + ], + "createdAt": 2019-02-12T21:01:22.479Z, + "enabled": true, + "id": "1", + "notifyWhen": "onActiveAlert", + "params": Object { + "bar": true, + }, + "schedule": Object { + "interval": "1m", + }, + "scheduledTaskId": "task-123", + "updatedAt": 2019-02-12T21:01:22.479Z, + } + `); + expect(encryptedSavedObjects.getDecryptedAsInternalUser).toHaveBeenCalledWith('alert', '1', { + namespace: 'default', + }); + expect(unsecuredSavedObjectsClient.get).not.toHaveBeenCalled(); + expect(actionsClient.isPreconfigured).toHaveBeenCalledTimes(3); + }); test('logs warning when creating with an interval less than the minimum configured one when enforce = false', async () => { actionsClient.getBulk.mockReset(); From 5d04232920be7821608c677749e6529bfb242e31 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Tue, 31 Jan 2023 16:25:17 +0000 Subject: [PATCH 30/40] refactor importing connectors --- .../import_rule_action_connectors.ts | 44 ++++++------------ .../logic/import/action_connectors/types.ts | 3 ++ .../import/action_connectors/utils/index.ts | 46 +++++++++++++++++-- 3 files changed, 60 insertions(+), 33 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts index 9c03303f838a6..3d2fc9fb7c6b6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/import_rule_action_connectors.ts @@ -8,23 +8,18 @@ import { Readable } from 'stream'; import type { SavedObjectsImportResponse } from '@kbn/core-saved-objects-common'; import type { SavedObject } from '@kbn/core-saved-objects-server'; -import { pick } from 'lodash'; -import type { RuleToImport } from '../../../../../../../common/detection_engine/rule_management'; + import type { WarningSchema } from '../../../../../../../common/detection_engine/schemas/response'; import { + checkIfActionsHaveMissingConnectors, + filterExistingActionConnectors, + getActionConnectorRules, handleActionsHaveNoConnectors, mapSOErrorToRuleError, returnErroredImportResult, } from './utils'; import type { ImportRuleActionConnectorsParams, ImportRuleActionConnectorsResult } from './types'; -const getActionConnectorRules = (rules: Array) => - 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 importRuleActionConnectors = async ({ actionConnectors, actionsClient, @@ -44,32 +39,21 @@ export const importRuleActionConnectors = async ({ warnings: [], }; - const ruleIds: string = [...new Set(...Object.values(actionsIds))].join(); - if (overwrite && !actionConnectors.length) - return handleActionsHaveNoConnectors(actionsIds, ruleIds); + return handleActionsHaveNoConnectors(actionsIds, actionConnectorRules); let actionConnectorsToImport: SavedObject[] = actionConnectors; - if (!overwrite) { - const storedActionIds: string[] = await (await actionsClient.getAll()).map(({ id }) => id); - const newIdsToAdd = actionsIds.filter((id) => !storedActionIds.includes(id)); - - // if new action-connectors don't have exported connectors will fail with missing - if (actionConnectors.length < newIdsToAdd.length) { - const actionConnectorsIds = actionConnectors.map(({ id }) => id); - const missingActionConnector = newIdsToAdd.filter( - (id) => !actionConnectorsIds.includes(id) - ); - const missingActionRules = pick(actionConnectorRules, [...missingActionConnector]); - - const missingRuleIds: string = [ - ...new Set(Object.values(missingActionRules).flat()), - ].join(); + if (!overwrite) { + const newIdsToAdd = await filterExistingActionConnectors(actionsClient, actionsIds); - return handleActionsHaveNoConnectors(missingActionConnector, missingRuleIds); - } - // Incase connectors imported before + const foundMissingConnectors = checkIfActionsHaveMissingConnectors( + actionConnectors, + newIdsToAdd, + actionConnectorRules + ); + if (foundMissingConnectors) return foundMissingConnectors; + // filter out existing connectors actionConnectorsToImport = actionConnectors.filter(({ id }) => newIdsToAdd.includes(id)); } if (!actionConnectorsToImport.length) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts index 8eb915dc83612..8ba6c41c42dde 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/types.ts @@ -39,3 +39,6 @@ export interface ConflictError { } export type ErrorType = SOError | ConflictError | SavedObjectsImportFailure | Error; +export interface ActionRules { + [actionsIds: string]: string[]; +} diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts index ce3cd9061ca14..122cd93e0881a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts @@ -4,11 +4,20 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ - +import { pick } from 'lodash'; 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 { ConflictError, ErrorType, ImportRuleActionConnectorsResult, SOError } from '../types'; +import type { RuleToImport } from '../../../../../../../../common/detection_engine/rule_management'; +import type { + ActionRules, + ConflictError, + ErrorType, + ImportRuleActionConnectorsResult, + SOError, +} from '../types'; export const returnErroredImportResult = (error: ErrorType): ImportRuleActionConnectorsResult => ({ success: false, @@ -19,8 +28,10 @@ export const returnErroredImportResult = (error: ErrorType): ImportRuleActionCon export const handleActionsHaveNoConnectors = ( actionsIds: string[], - ruleIds: string + actionConnectorRules: ActionRules ): ImportRuleActionConnectorsResult => { + const ruleIds: string = [...new Set(Object.values(actionConnectorRules).flat())].join(); + if (actionsIds && actionsIds.length) { const errors: BulkError[] = []; const errorMessage = @@ -87,3 +98,32 @@ export const handleActionConnectorsErrors = (error: ErrorType, id?: string): Bul export const mapSOErrorToRuleError = (errors: SavedObjectsImportFailure[]): BulkError[] => { return errors.map(({ id, error }) => handleActionConnectorsErrors(error, id)); }; + +export const filterExistingActionConnectors = async ( + actionsClient: ActionsClient, + actionsIds: string[] +) => { + const storedConnectors = await actionsClient.getAll(); + const storedActionIds: string[] = storedConnectors.map(({ id }) => id); + return actionsIds.filter((id) => !storedActionIds.includes(id)); +}; +export const getActionConnectorRules = (rules: Array) => + 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; +}; From 816cfe514b1231cb31b55210f5d2bba147b7e998 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 1 Feb 2023 10:42:57 +0000 Subject: [PATCH 31/40] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../plugins/alerting/server/rules_client/tests/create.test.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts index 993178a1e951f..d3e877396e073 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts @@ -3047,7 +3047,7 @@ describe('create()', () => { expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); }); - test('should create a rule and not throw an error when skipActionConnectorsValidations is true even when some actions are missing frequency params', async () => { + test('should create a rule and not throw an error when skipActionConnectorsValidations is true even when some actions are missing frequency params', async () => { const data = getMockData({ actions: [ { @@ -3218,5 +3218,4 @@ describe('create()', () => { } `); }); - }); From 31f596325ff0e726a684b5897a75830833a6dc08 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Wed, 1 Feb 2023 11:15:22 +0000 Subject: [PATCH 32/40] using warning message from the warning array and update the tests --- .../server/rules_client/tests/create.test.ts | 3 +-- .../action_connectors_warning.test.tsx.snap | 8 +++---- .../action_connectors_warning.test.tsx | 21 ++++++++++++++----- .../action_connectors_warning/index.tsx | 5 ++--- .../import_data_modal/translations.ts | 7 +++---- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts index 993178a1e951f..d3e877396e073 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/create.test.ts @@ -3047,7 +3047,7 @@ describe('create()', () => { expect(unsecuredSavedObjectsClient.create).not.toHaveBeenCalled(); expect(taskManager.schedule).not.toHaveBeenCalled(); }); - test('should create a rule and not throw an error when skipActionConnectorsValidations is true even when some actions are missing frequency params', async () => { + test('should create a rule and not throw an error when skipActionConnectorsValidations is true even when some actions are missing frequency params', async () => { const data = getMockData({ actions: [ { @@ -3218,5 +3218,4 @@ describe('create()', () => { } `); }); - }); diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap index 6f3ec5cd3116a..05eb8a84324eb 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap @@ -159,7 +159,7 @@ Object {
- 1 connector has sensitive information that requires updates, review in + 1 connector has sensitive information that requires updates. review in
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap index c0dcd159fdac5..fba4aa3d0fb96 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/__snapshots__/action_connectors_warning.test.tsx.snap @@ -150,7 +150,7 @@ Object { class="euiText emotion-euiText-s-euiTextColor-default-euiCallOut__description" >
- 1 connector has sensitive information that requires updates. review in + 1 connector has sensitive information that requires updates. +
+
+
+
@@ -197,7 +214,7 @@ Object { class="euiText emotion-euiText-s-euiTextColor-default-euiCallOut__description" >
- 1 connector has sensitive information that requires updates. review in + 1 connector has sensitive information that requires updates. +
+
+
+
+
+
+
+
+
+
, + "debug": [Function], + "findAllByAltText": [Function], + "findAllByDisplayValue": [Function], + "findAllByLabelText": [Function], + "findAllByPlaceholderText": [Function], + "findAllByRole": [Function], + "findAllByTestId": [Function], + "findAllByText": [Function], + "findAllByTitle": [Function], + "findByAltText": [Function], + "findByDisplayValue": [Function], + "findByLabelText": [Function], + "findByPlaceholderText": [Function], + "findByRole": [Function], + "findByTestId": [Function], + "findByText": [Function], + "findByTitle": [Function], + "getAllByAltText": [Function], + "getAllByDisplayValue": [Function], + "getAllByLabelText": [Function], + "getAllByPlaceholderText": [Function], + "getAllByRole": [Function], + "getAllByTestId": [Function], + "getAllByText": [Function], + "getAllByTitle": [Function], + "getByAltText": [Function], + "getByDisplayValue": [Function], + "getByLabelText": [Function], + "getByPlaceholderText": [Function], + "getByRole": [Function], + "getByTestId": [Function], + "getByText": [Function], + "getByTitle": [Function], + "queryAllByAltText": [Function], + "queryAllByDisplayValue": [Function], + "queryAllByLabelText": [Function], + "queryAllByPlaceholderText": [Function], + "queryAllByRole": [Function], + "queryAllByTestId": [Function], + "queryAllByText": [Function], + "queryAllByTitle": [Function], + "queryByAltText": [Function], + "queryByDisplayValue": [Function], + "queryByLabelText": [Function], + "queryByPlaceholderText": [Function], + "queryByRole": [Function], + "queryByTestId": [Function], + "queryByText": [Function], + "queryByTitle": [Function], + "rerender": [Function], + "unmount": [Function], +} +`; + +exports[`ActionConnectorWarnings should render if 2 connectors were imported and use the button label when is set 1`] = ` +Object { + "asFragment": [Function], + "baseElement": +
+
+

+

+
+
+
+
+ 2 connectors have sensitive information that requires updates. +
+
+
+
+ +
+
+
+
+
+
+ , + "container":
+
+

+

+
+
+
+
+ 2 connectors have sensitive information that requires updates. +
+
+
+
+
@@ -301,7 +520,7 @@ Object { class="euiText emotion-euiText-s-euiTextColor-default-euiCallOut__description" >
- 2 connectors have sensitive information that requires updates. review in + 2 connectors have sensitive information that requires updates. +
+
+
+
@@ -348,7 +584,7 @@ Object { class="euiText emotion-euiText-s-euiTextColor-default-euiCallOut__description" >
- 2 connectors have sensitive information that requires updates. review in + 2 connectors have sensitive information that requires updates. +
+
+
+
diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/action_connectors_warning.test.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/action_connectors_warning.test.tsx index dcbea82715103..a71ce2beeaebe 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/action_connectors_warning.test.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/action_connectors_warning.test.tsx @@ -48,7 +48,7 @@ describe('ActionConnectorWarnings', () => { '1 connector imported' ); expect(getByTestId('actionConnectorsWarningsCallOutMessage').textContent).toBe( - '1 connector has sensitive information that requires updates. review in connectors' + '1 connector has sensitive information that requires updates.' ); }); test('should render if 2 connectors were imported and use the warning message with the correct imported number', () => { @@ -71,7 +71,34 @@ describe('ActionConnectorWarnings', () => { '2 connectors imported' ); expect(getByTestId('actionConnectorsWarningsCallOutMessage').textContent).toBe( - '2 connectors have sensitive information that requires updates. review in connectors' + '2 connectors have sensitive information that requires updates.' ); + expect(getByTestId('actionConnectorsWarningsCallOutButton').textContent).toBe( + 'Go to connectors' + ); + }); + test('should render if 2 connectors were imported and use the button label when is set', () => { + const wrapper = render( + + ); + const { getByTestId } = wrapper; + expect(wrapper).toMatchSnapshot(); + expect(getByTestId('actionConnectorsWarningsCallOutTitle').textContent).toBe( + '2 connectors imported' + ); + expect(getByTestId('actionConnectorsWarningsCallOutMessage').textContent).toBe( + '2 connectors have sensitive information that requires updates.' + ); + expect(getByTestId('actionConnectorsWarningsCallOutButton').textContent).toBe('Connectors'); }); }); diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/index.tsx b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/index.tsx index 527bc0d38a004..ad72893f66d87 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/index.tsx +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/action_connectors_warning/index.tsx @@ -7,7 +7,7 @@ import React from 'react'; import type { FC } from 'react'; -import { EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiLink, EuiText } from '@elastic/eui'; +import { EuiButton, EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiText } from '@elastic/eui'; import type { WarningSchema } from '../../../../../common/detection_engine/schemas/response'; import { useKibana } from '../../../lib/kibana/kibana_react'; import * as i18n from '../translations'; @@ -23,7 +23,7 @@ const ActionConnectorWarningsComponent: FC - + - - {i18n.ACTION_CONNECTORS_WARNING_MESSAGE(message)} - - {i18n.ACTION_CONNECTORS_WARNING_LINK} - - + {message} + + + + + {buttonLabel || i18n.ACTION_CONNECTORS_WARNING_BUTTON} + + diff --git a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts index fe3801e3b57a6..11ae7dd972a57 100644 --- a/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts +++ b/x-pack/plugins/security_solution/public/common/components/import_data_modal/translations.ts @@ -74,19 +74,10 @@ export const ACTION_CONNECTORS_WARNING_TITLE = (totalConnectors: number) => } ); -export const ACTION_CONNECTORS_WARNING_MESSAGE = (message: string) => - i18n.translate( - 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningMessage', - { - values: { message }, - defaultMessage: '{message} review in ', - } - ); - -export const ACTION_CONNECTORS_WARNING_LINK = i18n.translate( - 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningLink', +export const ACTION_CONNECTORS_WARNING_BUTTON = i18n.translate( + 'xpack.securitySolution.detectionEngine.components.importRuleModal.actionConnectorsWarningButton', { - defaultMessage: 'connectors', + defaultMessage: 'Go to connectors', } ); From eb6dfe5eb7e3ed0c512592356b20b543eb9058b1 Mon Sep 17 00:00:00 2001 From: wafaanasr Date: Mon, 6 Feb 2023 14:15:23 +0000 Subject: [PATCH 40/40] remove todos already addressed --- .../api/rules/import_rules/route.test.ts | 33 ------------------- .../import/action_connectors/utils/index.ts | 2 +- 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts index 9812c5873f23d..bb1bb7d55244a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/import_rules/route.test.ts @@ -34,8 +34,6 @@ import { importRulesRoute } from './route'; jest.mock('../../../../../machine_learning/authz'); -// TODO add tests for connectors - describe('Import rules route', () => { let config: ReturnType; let server: ReturnType; @@ -115,37 +113,6 @@ describe('Import rules route', () => { action_connectors_errors: [], }); }); - test('returns a 403 error object if importing actions connectors fails', async () => { - (buildMlAuthz as jest.Mock).mockReturnValueOnce({ - validateRuleType: jest - .fn() - .mockResolvedValue({ valid: false, message: 'mocked validation message' }), - }); - - const response = await server.inject(request, requestContextMock.convertContext(context)); - expect(response.status).toEqual(200); - expect(response.body).toEqual({ - errors: [ - { - error: { - message: 'mocked validation message', - status_code: 403, - }, - rule_id: 'rule-1', - }, - ], - success: false, - success_count: 0, - rules_count: 1, - exceptions_errors: [], - exceptions_success: true, - exceptions_success_count: 0, - action_connectors_success: true, - action_connectors_success_count: 0, - action_connectors_warnings: [], - action_connectors_errors: [], - }); - }); test('returns error if createRulesAndExceptionsStreamFromNdJson throws error', async () => { const transformMock = jest diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts index 122cd93e0881a..6d9b3b9da9e6d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/import/action_connectors/utils/index.ts @@ -73,7 +73,7 @@ export const handleActionConnectorsErrors = (error: ErrorType, id?: string): Bul statusCode: 500, message: (error as ConflictError)?.type === 'conflict' - ? 'There is a conflict' // TODO : choose a message when conflict happen + ? 'There is a conflict' : (error as Error).message ? (error as Error).message : '',