diff --git a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/perform_bulk_action_schema.ts b/x-pack/plugins/security_solution/common/detection_engine/schemas/response/perform_bulk_action_schema.ts deleted file mode 100644 index 2bc3ea8448ee9..0000000000000 --- a/x-pack/plugins/security_solution/common/detection_engine/schemas/response/perform_bulk_action_schema.ts +++ /dev/null @@ -1,37 +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 * as t from 'io-ts'; -import { PositiveInteger, PositiveIntegerGreaterThanZero } from '@kbn/securitysolution-io-ts-types'; - -const rule = t.type({ - id: t.string, - name: t.union([t.string, t.undefined]), -}); - -const error = t.type({ - status_code: PositiveIntegerGreaterThanZero, - message: t.string, - rules: t.array(rule), -}); - -export const bulkActionPartialErrorResponseSchema = t.exact( - t.type({ - status_code: PositiveIntegerGreaterThanZero, - message: t.string, - attributes: t.type({ - errors: t.array(error), - rules: t.type({ - failed: PositiveInteger, - succeeded: PositiveInteger, - total: PositiveInteger, - }), - }), - }) -); - -export type BulkActionPartialErrorResponse = t.TypeOf; diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts index c765db6eaecd8..427cf28ef8f2f 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/api.ts @@ -45,7 +45,7 @@ import { BulkRuleResponse, PatchRuleProps, BulkActionProps, - BulkActionResponse, + BulkActionResponseMap, PreviewRulesProps, } from './types'; import { KibanaServices } from '../../../../common/lib/kibana'; @@ -266,16 +266,19 @@ export const performBulkAction = async ({ query, edit, ids, -}: BulkActionProps): Promise> => - KibanaServices.get().http.fetch>(DETECTION_ENGINE_RULES_BULK_ACTION, { - method: 'POST', - body: JSON.stringify({ - action, - ...(edit ? { edit } : {}), - ...(ids ? { ids } : {}), - ...(query !== undefined ? { query } : {}), - }), - }); +}: BulkActionProps): Promise> => + KibanaServices.get().http.fetch>( + DETECTION_ENGINE_RULES_BULK_ACTION, + { + method: 'POST', + body: JSON.stringify({ + action, + ...(edit ? { edit } : {}), + ...(ids ? { ids } : {}), + ...(query !== undefined ? { query } : {}), + }), + } + ); /** * Create Prepackaged Rules diff --git a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts index 7943bca613488..af5e4fd568a64 100644 --- a/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/containers/detection_engine/rules/types.ts @@ -236,18 +236,41 @@ export interface BulkActionProps { edit?: BulkActionEditPayload[]; } +export interface BulkActionSummary { + failed: number; + succeeded: number; + total: number; +} + export interface BulkActionResult { - success: boolean; - rules_count: number; + updated: Rule[]; + created: Rule[]; + deleted: Rule[]; +} + +export interface BulkActionAggregatedError { + message: string; + status_code: number; + rules: Array<{ id: string; name?: string }>; +} + +export interface BulkActionResponse { + success?: boolean; + rules_count?: number; + attributes: { + summary: BulkActionSummary; + results: BulkActionResult; + errors?: BulkActionAggregatedError[]; + }; } -export type BulkActionResponse = { - [BulkAction.delete]: BulkActionResult; - [BulkAction.disable]: BulkActionResult; - [BulkAction.enable]: BulkActionResult; - [BulkAction.duplicate]: BulkActionResult; +export type BulkActionResponseMap = { + [BulkAction.delete]: BulkActionResponse; + [BulkAction.disable]: BulkActionResponse; + [BulkAction.enable]: BulkActionResponse; + [BulkAction.duplicate]: BulkActionResponse; [BulkAction.export]: Blob; - [BulkAction.edit]: BulkActionResult; + [BulkAction.edit]: BulkActionResponse; }[Action]; export interface BasicFetchProps { diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts index 5b2fdbc5e290e..29b374f3fb26e 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/actions.ts @@ -209,7 +209,7 @@ const executeRulesBulkAction = async ({ } else { const response = await performBulkAction({ ...search, action, edit: payload?.edit }); - onSuccess?.({ rulesCount: response.rules_count }); + onSuccess?.({ rulesCount: response.attributes.summary.succeeded }); } } catch (e) { if (onError) { diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx index 4d3767e73a59e..556213d985db9 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/all/bulk_actions/use_bulk_actions.tsx @@ -45,8 +45,10 @@ import { useAppToasts } from '../../../../../../common/hooks/use_app_toasts'; import { useIsExperimentalFeatureEnabled } from '../../../../../../common/hooks/use_experimental_features'; import { convertRulesFilterToKQL } from '../../../../../containers/detection_engine/rules/utils'; -import type { FilterOptions } from '../../../../../containers/detection_engine/rules/types'; -import type { BulkActionPartialErrorResponse } from '../../../../../../../common/detection_engine/schemas/response/perform_bulk_action_schema'; +import type { + BulkActionResponse, + FilterOptions, +} from '../../../../../containers/detection_engine/rules/types'; import type { HTTPError } from '../../../../../../../common/detection_engine/types'; import { useInvalidateRules } from '../../../../../containers/detection_engine/rules/use_find_rules_query'; @@ -300,8 +302,8 @@ export const useBulkActions = ({ hideWarningToast(); // if response doesn't have number of failed rules, it means the whole bulk action failed // and general error toast will be shown. Otherwise - error toast for partial failure - const failedRulesCount = (error?.body as BulkActionPartialErrorResponse)?.attributes - ?.rules?.failed; + const failedRulesCount = (error?.body as BulkActionResponse)?.attributes?.summary + ?.failed; if (isNaN(failedRulesCount)) { toasts.addError(error, { title: i18n.BULK_ACTION_FAILED }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts index f5d652ef34236..bfbd396851cd8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/test_adapters.ts @@ -35,6 +35,11 @@ const buildResponses = (method: Method, calls: MockCall[]): ResponseCall[] => { status: call.statusCode, body: JSON.parse(call.body), })); + case 'customError': + return calls.map(([call]) => ({ + status: call.statusCode, + body: call.body, + })); default: throw new Error(`Encountered unexpected call to response.${method}`); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts index a5a982a6d78c6..74f777b29ca01 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts @@ -17,7 +17,7 @@ import { } from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { readRules } from '../../rules/read_rules'; import { getDuplicates } from './utils'; import { transformValidateBulkError } from './validate'; @@ -92,7 +92,7 @@ export const createRulesBulkRoute = ( }); } - throwHttpError(await mlAuthz.validateRuleType(internalRule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(internalRule.params.type)); const finalIndex = internalRule.params.outputIndex; const indexExists = await getIndexExists(esClient.asCurrentUser, finalIndex); if (!isRuleRegistryEnabled && !indexExists) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts index ef5c7368841db..fe39c5cb9680e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts @@ -14,7 +14,7 @@ import { import { SetupPlugins } from '../../../../plugin'; import type { SecuritySolutionPluginRouter } from '../../../../types'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { readRules } from '../../rules/read_rules'; import { buildSiemResponse } from '../utils'; @@ -79,7 +79,7 @@ export const createRulesRoute = ( request, savedObjectsClient, }); - throwHttpError(await mlAuthz.validateRuleType(internalRule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(internalRule.params.type)); const indexExists = await getIndexExists( esClient.asCurrentUser, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts index 55c05dd73e33b..58d364cb34b5c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts @@ -17,7 +17,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../types'; import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { transformBulkError, buildSiemResponse } from '../utils'; import { getIdBulkError } from './utils'; import { transformValidateBulkError } from './validate'; @@ -117,7 +117,7 @@ export const patchRulesBulkRoute = ( try { if (type) { // reject an unauthorized "promotion" to ML - throwHttpError(await mlAuthz.validateRuleType(type)); + throwAuthzError(await mlAuthz.validateRuleType(type)); } const existingRule = await readRules({ @@ -128,7 +128,7 @@ export const patchRulesBulkRoute = ( }); if (existingRule?.params.type) { // reject an unauthorized modification of an ML rule - throwHttpError(await mlAuthz.validateRuleType(existingRule?.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(existingRule?.params.type)); } const migratedRule = await legacyMigrate({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts index 0767e86c4e22c..a5bc76cc5ef2e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts @@ -17,7 +17,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../types'; import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { patchRules } from '../../rules/patch_rules'; import { buildSiemResponse } from '../utils'; @@ -118,7 +118,7 @@ export const patchRulesRoute = ( }); if (type) { // reject an unauthorized "promotion" to ML - throwHttpError(await mlAuthz.validateRuleType(type)); + throwAuthzError(await mlAuthz.validateRuleType(type)); } const existingRule = await readRules({ @@ -129,7 +129,7 @@ export const patchRulesRoute = ( }); if (existingRule?.params.type) { // reject an unauthorized modification of an ML rule - throwHttpError(await mlAuthz.validateRuleType(existingRule?.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(existingRule?.params.type)); } const migratedRule = await legacyMigrate({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.test.ts index 386b87fefc40c..ecf925bda97e5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.test.ts @@ -51,14 +51,36 @@ describe.each([ it('returns 200 when performing bulk action with all dependencies present', async () => { const response = await server.inject(getBulkActionRequest(), context); expect(response.status).toEqual(200); - expect(response.body).toEqual({ success: true, rules_count: 1 }); + expect(response.body).toEqual({ + success: true, + rules_count: 1, + attributes: { + results: someBulkActionResults(), + summary: { + failed: 0, + succeeded: 1, + total: 1, + }, + }, + }); }); it("returns 200 when provided filter query doesn't match any rules", async () => { clients.rulesClient.find.mockResolvedValue(getEmptyFindResult()); const response = await server.inject(getBulkActionRequest(), context); expect(response.status).toEqual(200); - expect(response.body).toEqual({ success: true, rules_count: 0 }); + expect(response.body).toEqual({ + success: true, + rules_count: 0, + attributes: { + results: someBulkActionResults(), + summary: { + failed: 0, + succeeded: 0, + total: 0, + }, + }, + }); }); it('returns 400 when provided filter query matches too many rules', async () => { @@ -93,7 +115,7 @@ describe.each([ errors: [ { message: 'Elastic rule can`t be edited', - status_code: 403, + status_code: 400, rules: [ { id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', @@ -102,7 +124,8 @@ describe.each([ ], }, ], - rules: { + results: someBulkActionResults(), + summary: { failed: 1, succeeded: 0, total: 1, @@ -133,7 +156,8 @@ describe.each([ ], }, ], - rules: { + results: someBulkActionResults(), + summary: { failed: 1, succeeded: 0, total: 1, @@ -165,7 +189,8 @@ describe.each([ ], }, ], - rules: { + results: someBulkActionResults(), + summary: { failed: 1, succeeded: 0, total: 1, @@ -202,7 +227,7 @@ describe.each([ expect(response.status).toEqual(500); expect(response.body).toEqual({ attributes: { - rules: { + summary: { failed: 1, succeeded: 0, total: 1, @@ -220,6 +245,7 @@ describe.each([ ], }, ], + results: someBulkActionResults(), }, message: 'Bulk edit failed', status_code: 500, @@ -252,7 +278,7 @@ describe.each([ expect(response.status).toEqual(500); expect(response.body).toEqual({ attributes: { - rules: { + summary: { failed: 1, succeeded: 0, total: 1, @@ -269,6 +295,7 @@ describe.each([ ], }, ], + results: someBulkActionResults(), }, message: 'Bulk edit failed', status_code: 500, @@ -303,7 +330,7 @@ describe.each([ expect(response.status).toEqual(500); expect(response.body).toEqual({ attributes: { - rules: { + summary: { failed: 3, succeeded: 2, total: 5, @@ -334,6 +361,7 @@ describe.each([ ], }, ], + results: someBulkActionResults(), }, message: 'Bulk edit partially failed', status_code: 500, @@ -369,14 +397,14 @@ describe.each([ expect(response.status).toEqual(500); expect(response.body).toEqual({ attributes: { - rules: { + summary: { failed: 1, succeeded: 1, total: 2, }, errors: [ { - message: 'Can`t fetch a rule', + message: 'Rule not found', status_code: 500, rules: [ { @@ -385,6 +413,7 @@ describe.each([ ], }, ], + results: someBulkActionResults(), }, message: 'Bulk edit partially failed', status_code: 500, @@ -498,6 +527,23 @@ describe.each([ const response = await server.inject(getBulkActionEditRequest(), context); expect(response.status).toEqual(200); - expect(response.body).toEqual({ success: true, rules_count: rulesNumber }); + expect(response.body).toEqual( + expect.objectContaining({ + success: true, + rules_count: rulesNumber, + attributes: { + summary: { failed: 0, succeeded: rulesNumber, total: rulesNumber }, + results: someBulkActionResults(), + }, + }) + ); }); }); + +function someBulkActionResults() { + return { + created: expect.any(Array), + deleted: expect.any(Array), + updated: expect.any(Array), + }; +} diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts index 89f92448847a2..1e1c894ad097c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts @@ -5,11 +5,12 @@ * 2.0. */ +import { truncate } from 'lodash'; import moment from 'moment'; -import { transformError } from '@kbn/securitysolution-es-utils'; -import { Logger } from 'src/core/server'; +import { BadRequestError, transformError } from '@kbn/securitysolution-es-utils'; +import { KibanaResponseFactory, Logger } from 'src/core/server'; -import { RuleAlertType as Rule } from '../../rules/types'; +import { RuleAlertType } from '../../rules/types'; import type { RulesClient } from '../../../../../../alerting/server'; @@ -24,9 +25,13 @@ import { SetupPlugins } from '../../../../plugin'; import type { SecuritySolutionPluginRouter } from '../../../../types'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { routeLimitedConcurrencyTag } from '../../../../utils/route_limited_concurrency_tag'; -import { initPromisePool } from '../../../../utils/promise_pool'; +import { + initPromisePool, + PromisePoolError, + PromisePoolOutcome, +} from '../../../../utils/promise_pool'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { deleteRules } from '../../rules/delete_rules'; import { duplicateRule } from '../../rules/duplicate_rule'; import { findRules } from '../../rules/find_rules'; @@ -35,28 +40,13 @@ import { patchRules } from '../../rules/patch_rules'; import { applyBulkActionEditToRule } from '../../rules/bulk_action_edit'; import { getExportByObjectIds } from '../../rules/get_export_by_object_ids'; import { buildSiemResponse } from '../utils'; +import { AbortError } from '../../../../../../../../src/plugins/kibana_utils/common'; +import { internalRuleToAPIResponse } from '../../schemas/rule_converters'; const MAX_RULES_TO_PROCESS_TOTAL = 10000; const MAX_ERROR_MESSAGE_LENGTH = 1000; const MAX_ROUTE_CONCURRENCY = 5; -type RuleActionFn = (rule: Rule) => Promise; - -type RuleActionSuccess = undefined; - -type RuleActionResult = RuleActionSuccess | RuleActionError; - -interface RuleActionError { - error: { - message: string; - statusCode: number; - }; - rule: { - id: string; - name: string; - }; -} - interface NormalizedRuleError { message: string; status_code: number; @@ -66,18 +56,25 @@ interface NormalizedRuleError { }>; } -const normalizeErrorResponse = (errors: RuleActionError[]): NormalizedRuleError[] => { +const normalizeErrorResponse = ( + errors: Array | PromisePoolError> +): NormalizedRuleError[] => { const errorsMap = new Map(); - errors.forEach((ruleError) => { - const { message } = ruleError.error; + errors.forEach(({ error, item }) => { + const { message, statusCode } = + error instanceof Error ? transformError(error) : { message: String(error), statusCode: 500 }; + // The promise pool item is either a rule ID string or a rule object. We have + // string IDs when we fail to fetch rules. Rule objects come from other + // situations when we found a rule but failed somewhere else. + const rule = typeof item === 'string' ? { id: item } : { id: item.id, name: item.name }; + if (errorsMap.has(message)) { - errorsMap.get(message).rules.push(ruleError.rule); + errorsMap.get(message).rules.push(rule); } else { - const { error, rule } = ruleError; errorsMap.set(message, { - message: error.message, - status_code: error.statusCode, + message: truncate(message, { length: MAX_ERROR_MESSAGE_LENGTH }), + status_code: statusCode, rules: [rule], }); } @@ -86,93 +83,58 @@ const normalizeErrorResponse = (errors: RuleActionError[]): NormalizedRuleError[ return Array.from(errorsMap, ([_, normalizedError]) => normalizedError); }; -const getErrorResponseBody = (errors: RuleActionError[], rulesCount: number) => { - const errorsCount = errors.length; - return { - message: errorsCount === rulesCount ? 'Bulk edit failed' : 'Bulk edit partially failed', - status_code: 500, - attributes: { - errors: normalizeErrorResponse(errors).map(({ message, ...error }) => ({ - ...error, - message: - message.length > MAX_ERROR_MESSAGE_LENGTH - ? `${message.slice(0, MAX_ERROR_MESSAGE_LENGTH - 3)}...` - : message, - })), - rules: { - total: rulesCount, - failed: errorsCount, - succeeded: rulesCount - errorsCount, - }, - }, +const buildBulkResponse = ( + response: KibanaResponseFactory, + fetchRulesOutcome: PromisePoolOutcome, + bulkActionOutcome: PromisePoolOutcome +) => { + const errors = [...fetchRulesOutcome.errors, ...bulkActionOutcome.errors]; + const summary = { + failed: errors.length, + succeeded: bulkActionOutcome.results.length, + total: bulkActionOutcome.results.length + errors.length, }; -}; -const executeActionAndHandleErrors = async ( - rule: Rule, - action: RuleActionFn -): Promise => { - try { - await action(rule); - } catch (err) { - const { message, statusCode } = transformError(err); - return { - error: { message, statusCode }, - rule: { id: rule.id, name: rule.name }, - }; - } -}; - -const executeBulkAction = async (rules: Rule[], action: RuleActionFn, abortSignal: AbortSignal) => - initPromisePool({ - concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, - items: rules, - executor: async (rule) => executeActionAndHandleErrors(rule, action), - abortSignal, - }); - -const getRulesByIds = async ({ - ids, - rulesClient, - isRuleRegistryEnabled, - abortSignal, -}: { - ids: string[]; - rulesClient: RulesClient; - isRuleRegistryEnabled: boolean; - abortSignal: AbortSignal; -}) => { - const readRulesExecutor = async (id: string) => { - try { - const rule = await readRules({ id, rulesClient, isRuleRegistryEnabled, ruleId: undefined }); - if (rule == null) { - throw Error('Can`t fetch a rule'); - } - return { rule }; - } catch (err) { - const { message, statusCode } = transformError(err); - return { - error: { message, statusCode }, - rule: { id }, - }; - } + const results = { + updated: bulkActionOutcome.results + .filter(({ item, result }) => item.id === result?.id) + .map(({ result }) => result && internalRuleToAPIResponse(result)), + created: bulkActionOutcome.results + .filter(({ item, result }) => result != null && result.id !== item.id) + .map(({ result }) => result && internalRuleToAPIResponse(result)), + deleted: bulkActionOutcome.results + .filter(({ result }) => result == null) + .map(({ item }) => internalRuleToAPIResponse(item)), }; - const { results } = await initPromisePool({ - concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, - items: ids, - executor: readRulesExecutor, - abortSignal, - }); + if (errors.length > 0) { + return response.custom({ + headers: { 'content-type': 'application/json' }, + body: Buffer.from( + JSON.stringify({ + message: summary.succeeded > 0 ? 'Bulk edit partially failed' : 'Bulk edit failed', + status_code: 500, + attributes: { + errors: normalizeErrorResponse(errors), + results, + summary, + }, + }) + ), + statusCode: 500, + }); + } - return { - total: ids.length, - rules: results.filter((rule) => rule.error === undefined).map(({ rule }) => rule) as Rule[], - fetchErrors: results.filter((rule): rule is RuleActionError => rule.error !== undefined), - }; + return response.ok({ + body: { + success: true, + rules_count: summary.total, + attributes: { results, summary }, + }, + }); }; -const fetchRules = async ({ +const fetchRulesByQueryOrIds = async ({ query, ids, rulesClient, @@ -184,12 +146,18 @@ const fetchRules = async ({ rulesClient: RulesClient; isRuleRegistryEnabled: boolean; abortSignal: AbortSignal; -}) => { +}): Promise> => { if (ids) { - return getRulesByIds({ - ids, - rulesClient, - isRuleRegistryEnabled, + return initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: ids, + executor: async (id: string) => { + const rule = await readRules({ id, rulesClient, isRuleRegistryEnabled, ruleId: undefined }); + if (rule == null) { + throw Error('Rule not found'); + } + return rule; + }, abortSignal, }); } @@ -205,10 +173,15 @@ const fetchRules = async ({ fields: undefined, }); + if (total > MAX_RULES_TO_PROCESS_TOTAL) { + throw new BadRequestError( + `More than ${MAX_RULES_TO_PROCESS_TOTAL} rules matched the filter query. Try to narrow it down.` + ); + } + return { - rules: data, - total, - fetchErrors: [] as RuleActionError[], + results: data.map((rule) => ({ item: rule.id, result: rule })), + errors: [], }; }; @@ -268,7 +241,7 @@ export const performBulkActionRoute = ( savedObjectsClient, }); - const { rules, total, fetchErrors } = await fetchRules({ + const fetchRulesOutcome = await fetchRulesByQueryOrIds({ isRuleRegistryEnabled, rulesClient, query: body.query, @@ -276,68 +249,77 @@ export const performBulkActionRoute = ( abortSignal: abortController.signal, }); - if (total > MAX_RULES_TO_PROCESS_TOTAL) { - return siemResponse.error({ - body: `More than ${MAX_RULES_TO_PROCESS_TOTAL} rules matched the filter query. Try to narrow it down.`, - statusCode: 400, - }); - } + const rules = fetchRulesOutcome.results.map(({ result }) => result); + let bulkActionOutcome: PromisePoolOutcome; - let processingResponse: { - results: RuleActionResult[]; - } = { - results: [], - }; switch (body.action) { case BulkAction.enable: - processingResponse = await executeBulkAction( - rules, - async (rule) => { + bulkActionOutcome = await initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: rules, + executor: async (rule) => { if (!rule.enabled) { - throwHttpError(await mlAuthz.validateRuleType(rule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(rule.params.type)); await rulesClient.enable({ id: rule.id }); } + + return { + ...rule, + enabled: true, + }; }, - abortController.signal - ); + abortSignal: abortController.signal, + }); break; case BulkAction.disable: - processingResponse = await executeBulkAction( - rules, - async (rule) => { + bulkActionOutcome = await initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: rules, + executor: async (rule) => { if (rule.enabled) { - throwHttpError(await mlAuthz.validateRuleType(rule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(rule.params.type)); await rulesClient.disable({ id: rule.id }); } + + return { + ...rule, + enabled: false, + }; }, - abortController.signal - ); + abortSignal: abortController.signal, + }); break; case BulkAction.delete: - processingResponse = await executeBulkAction( - rules, - async (rule) => { + bulkActionOutcome = await initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: rules, + executor: async (rule) => { await deleteRules({ ruleId: rule.id, rulesClient, ruleExecutionLog, }); + + return null; }, - abortController.signal - ); + abortSignal: abortController.signal, + }); break; case BulkAction.duplicate: - processingResponse = await executeBulkAction( - rules, - async (rule) => { - throwHttpError(await mlAuthz.validateRuleType(rule.params.type)); + bulkActionOutcome = await initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: rules, + executor: async (rule) => { + throwAuthzError(await mlAuthz.validateRuleType(rule.params.type)); - await rulesClient.create({ + const createdRule = await rulesClient.create({ data: duplicateRule(rule, isRuleRegistryEnabled), }); + + return createdRule; }, - abortController.signal - ); + abortSignal: abortController.signal, + }); break; case BulkAction.export: const exported = await getExportByObjectIds( @@ -359,15 +341,15 @@ export const performBulkActionRoute = ( body: responseBody, }); case BulkAction.edit: - processingResponse = await executeBulkAction( - rules, - async (rule) => { - throwHttpError({ - valid: !rule.params.immutable, - message: 'Elastic rule can`t be edited', - }); + bulkActionOutcome = await initPromisePool({ + concurrency: MAX_RULES_TO_UPDATE_IN_PARALLEL, + items: rules, + executor: async (rule) => { + if (rule.params.immutable) { + throw new BadRequestError('Elastic rule can`t be edited'); + } - throwHttpError(await mlAuthz.validateRuleType(rule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(rule.params.type)); const editedRule = body[BulkAction.edit].reduce( (acc, action) => applyBulkActionEditToRule(acc, action), @@ -385,40 +367,19 @@ export const performBulkActionRoute = ( timelineTitle, timelineId, }); + + return editedRule; }, - abortController.signal - ); + abortSignal: abortController.signal, + }); + break; } if (abortController.signal.aborted === true) { - throw Error('Bulk action was aborted'); + throw new AbortError('Bulk action was aborted'); } - const errors = [ - ...fetchErrors, - ...processingResponse.results.filter( - (resp): resp is RuleActionError => resp?.error !== undefined - ), - ]; - - if (errors.length > 0) { - const responseBody = getErrorResponseBody(errors, total); - - return response.custom({ - headers: { - 'content-type': 'application/json', - }, - body: Buffer.from(JSON.stringify(responseBody)), - statusCode: 500, - }); - } - - return response.ok({ - body: { - success: true, - rules_count: total, - }, - }); + return buildBulkResponse(response, fetchRulesOutcome, bulkActionOutcome); } catch (err) { const error = transformError(err); return siemResponse.error({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts index e4626aaa93216..775817dcb8a0c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/preview_rules_route.ts @@ -14,7 +14,7 @@ import { RuleParams } from '../../schemas/rule_schemas'; import { createPreviewRuleExecutionLogger } from '../../signals/preview/preview_rule_execution_logger'; import { parseInterval } from '../../signals/utils'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { SetupPlugins } from '../../../../plugin'; import type { SecuritySolutionPluginRouter } from '../../../../types'; @@ -106,7 +106,7 @@ export const previewRulesRoute = async ( request, savedObjectsClient, }); - throwHttpError(await mlAuthz.validateRuleType(internalRule.params.type)); + throwAuthzError(await mlAuthz.validateRuleType(internalRule.params.type)); await context.lists?.getExceptionListClient().createEndpointList(); const spaceId = siemClient.getSpaceId(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts index f560230cd531b..d1df5713914df 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts @@ -14,7 +14,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../types'; import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { getIdBulkError } from './utils'; import { transformValidateBulkError } from './validate'; import { transformBulkError, buildSiemResponse, createBulkErrorObject } from '../utils'; @@ -65,7 +65,7 @@ export const updateRulesBulkRoute = ( }); } - throwHttpError(await mlAuthz.validateRuleType(payloadRule.type)); + throwAuthzError(await mlAuthz.validateRuleType(payloadRule.type)); const existingRule = await readRules({ isRuleRegistryEnabled, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts index 4f63e49cd1690..8ac90748d9217 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts @@ -12,7 +12,7 @@ import type { SecuritySolutionPluginRouter } from '../../../../types'; import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; -import { throwHttpError } from '../../../machine_learning/validation'; +import { throwAuthzError } from '../../../machine_learning/validation'; import { buildSiemResponse } from '../utils'; import { getIdError } from './utils'; @@ -54,7 +54,7 @@ export const updateRulesRoute = ( request, savedObjectsClient, }); - throwHttpError(await mlAuthz.validateRuleType(request.body.type)); + throwAuthzError(await mlAuthz.validateRuleType(request.body.type)); const existingRule = await readRules({ isRuleRegistryEnabled, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index db04d8eded869..0ca665bb10584 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -10,7 +10,6 @@ import { createPromiseFromStreams } from '@kbn/utils'; import { Action, ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types'; import { - transformAlertToRule, getIdError, transformFindAlerts, transform, @@ -39,6 +38,7 @@ import { getQueryRuleParams, getThreatRuleParams, } from '../../schemas/rule_schemas.mock'; +import { internalRuleToAPIResponse } from '../../schemas/rule_converters'; import { requestContextMock } from '../__mocks__'; // eslint-disable-next-line no-restricted-imports @@ -70,17 +70,17 @@ describe.each([ ])('utils - %s', (_, isRuleRegistryEnabled) => { const { clients } = requestContextMock.createTools(); - describe('transformAlertToRule', () => { + describe('internalRuleToAPIResponse', () => { test('should work with a full data set', () => { const fullRule = getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()); - const rule = transformAlertToRule(fullRule); + const rule = internalRuleToAPIResponse(fullRule); expect(rule).toEqual(getOutputRuleAlertForRest()); }); test('should omit note if note is undefined', () => { const fullRule = getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()); fullRule.params.note = undefined; - const rule = transformAlertToRule(fullRule); + const rule = internalRuleToAPIResponse(fullRule); const { note, ...expectedWithoutNote } = getOutputRuleAlertForRest(); expect(rule).toEqual(expectedWithoutNote); }); @@ -88,7 +88,7 @@ describe.each([ test('should return enabled is equal to false', () => { const fullRule = getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()); fullRule.enabled = false; - const ruleWithEnabledFalse = transformAlertToRule(fullRule); + const ruleWithEnabledFalse = internalRuleToAPIResponse(fullRule); const expected = getOutputRuleAlertForRest(); expected.enabled = false; expect(ruleWithEnabledFalse).toEqual(expected); @@ -97,7 +97,7 @@ describe.each([ test('should return immutable is equal to false', () => { const fullRule = getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()); fullRule.params.immutable = false; - const ruleWithEnabledFalse = transformAlertToRule(fullRule); + const ruleWithEnabledFalse = internalRuleToAPIResponse(fullRule); const expected = getOutputRuleAlertForRest(); expect(ruleWithEnabledFalse).toEqual(expected); }); @@ -105,7 +105,7 @@ describe.each([ test('should work with tags but filter out any internal tags', () => { const fullRule = getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()); fullRule.tags = ['tag 1', 'tag 2', `${INTERNAL_IDENTIFIER}_some_other_value`]; - const rule = transformAlertToRule(fullRule); + const rule = internalRuleToAPIResponse(fullRule); const expected = getOutputRuleAlertForRest(); expected.tags = ['tag 1', 'tag 2']; expect(rule).toEqual(expected); @@ -117,7 +117,7 @@ describe.each([ mlRule.params.machineLearningJobId = ['some_job_id']; mlRule.params.type = 'machine_learning'; - const rule = transformAlertToRule(mlRule); + const rule = internalRuleToAPIResponse(mlRule); expect(rule).toEqual( expect.objectContaining({ anomaly_threshold: 55, @@ -165,7 +165,7 @@ describe.each([ threatRule.params.threatMapping = threatMapping; threatRule.params.threatQuery = '*:*'; - const rule = transformAlertToRule(threatRule); + const rule = internalRuleToAPIResponse(threatRule); expect(rule).toEqual( expect.objectContaining({ threat_index: ['index-123'], @@ -183,7 +183,7 @@ describe.each([ lists: [], ...getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), }; - const rule = transformAlertToRule(result); + const rule = internalRuleToAPIResponse(result); expect(rule).toEqual( expect.not.objectContaining({ lists: [], @@ -198,7 +198,7 @@ describe.each([ exceptions_list: [], ...getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), }; - const rule = transformAlertToRule(result); + const rule = internalRuleToAPIResponse(result); expect(rule).toEqual( expect.not.objectContaining({ exceptions_list: [], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 7e9f03cf828a8..ff2eab1f799b9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -22,7 +22,6 @@ import { RuleAlertType, isAlertType } from '../../rules/types'; import { createBulkErrorObject, BulkError, OutputError } from '../utils'; import { internalRuleToAPIResponse } from '../../schemas/rule_converters'; import { RuleParams } from '../../schemas/rule_schemas'; -import { SanitizedAlert } from '../../../../../../alerting/common'; // eslint-disable-next-line no-restricted-imports import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rule_actions_saved_object'; import { RuleExecutionSummariesByRuleId } from '../../rule_execution_log'; @@ -93,21 +92,11 @@ export const transformTags = (tags: string[]): string[] => { return tags.filter((tag) => !tag.startsWith(INTERNAL_IDENTIFIER)); }; -// Transforms the data but will remove any null or undefined it encounters and not include -// those on the export -export const transformAlertToRule = ( - rule: SanitizedAlert, - ruleExecutionSummary?: RuleExecutionSummary | null, - legacyRuleActions?: LegacyRulesActionsSavedObject | null -): Partial => { - return internalRuleToAPIResponse(rule, ruleExecutionSummary, legacyRuleActions); -}; - export const transformAlertsToRules = ( rules: RuleAlertType[], legacyRuleActions: Record ): Array> => { - return rules.map((rule) => transformAlertToRule(rule, null, legacyRuleActions[rule.id])); + return rules.map((rule) => internalRuleToAPIResponse(rule, null, legacyRuleActions[rule.id])); }; export const transformFindAlerts = ( @@ -138,7 +127,7 @@ export const transform = ( legacyRuleActions?: LegacyRulesActionsSavedObject | null ): Partial | null => { if (isAlertType(isRuleRegistryEnabled ?? false, rule)) { - return transformAlertToRule(rule, ruleExecutionSummary, legacyRuleActions); + return internalRuleToAPIResponse(rule, ruleExecutionSummary, legacyRuleActions); } return null; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils/import_rules_utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils/import_rules_utils.ts index 121e54c768856..fd4c751d7fb84 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils/import_rules_utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils/import_rules_utils.ts @@ -21,7 +21,7 @@ import { readRules } from '../../../rules/read_rules'; import { patchRules } from '../../../rules/patch_rules'; import { ImportRulesSchemaDecoded } from '../../../../../../common/detection_engine/schemas/request/import_rules_schema'; import { MlAuthz } from '../../../../machine_learning/authz'; -import { throwHttpError } from '../../../../machine_learning/validation'; +import { throwAuthzError } from '../../../../machine_learning/validation'; import { RulesClient } from '../../../../../../../../plugins/alerting/server'; import { ExceptionListClient } from '../../../../../../../../plugins/lists/server'; import { checkRuleExceptionReferences } from './check_rule_exception_references'; @@ -165,7 +165,7 @@ export const importRules = async ({ const language = !isMlRule(type) && languageOrUndefined == null ? 'kuery' : languageOrUndefined; // TODO: Fix these either with an is conversion or by better typing them within io-ts const filters: PartialFilter[] | undefined = filtersRest as PartialFilter[]; - throwHttpError(await mlAuthz.validateRuleType(type)); + throwAuthzError(await mlAuthz.validateRuleType(type)); const rule = await readRules({ isRuleRegistryEnabled, rulesClient, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts index 54a1b3521f2b1..e743e26e8da3f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts @@ -19,10 +19,11 @@ import { import { PartialAlert } from '../../../../../../alerting/server'; import { isAlertType } from '../../rules/types'; import { createBulkErrorObject, BulkError } from '../utils'; -import { transform, transformAlertToRule } from './utils'; +import { transform } from './utils'; import { RuleParams } from '../../schemas/rule_schemas'; // eslint-disable-next-line no-restricted-imports import { LegacyRulesActionsSavedObject } from '../../rule_actions/legacy_get_rule_actions_saved_object'; +import { internalRuleToAPIResponse } from '../../schemas/rule_converters'; export const transformValidate = ( rule: PartialAlert, @@ -69,7 +70,7 @@ export const transformValidateBulkError = ( isRuleRegistryEnabled?: boolean ): RulesSchema | BulkError => { if (isAlertType(isRuleRegistryEnabled ?? false, rule)) { - const transformed = transformAlertToRule(rule, ruleExecutionSummary); + const transformed = internalRuleToAPIResponse(rule, ruleExecutionSummary); const [validated, errors] = validateNonExact(transformed, rulesSchema); if (errors != null || validated == null) { return createBulkErrorObject({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/legacy_get_bulk_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/legacy_get_bulk_rule_actions_saved_object.ts index 72cfada909cdc..feaaa7f3e6c08 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/legacy_get_bulk_rule_actions_saved_object.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/legacy_get_bulk_rule_actions_saved_object.ts @@ -62,7 +62,7 @@ export const legacyGetBulkRuleActionsSavedObject = async ({ throw new AggregateError(errors, 'Error fetching rule actions'); } - const savedObjects = results.flatMap((result) => result.saved_objects); + const savedObjects = results.flatMap(({ result }) => result.saved_objects); return savedObjects.reduce( (acc: { [key: string]: LegacyRulesActionsSavedObject }, savedObject) => { const ruleAlertId = savedObject.references.find((reference) => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/client_for_routes/client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/client_for_routes/client.ts index 05647089bbd45..a4c528d941ba4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/client_for_routes/client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/client_for_routes/client.ts @@ -30,7 +30,7 @@ export const createClientForRoutes = ( return { /** * Get the current rule execution summary for each of the given rule IDs. - * This method splits work into chunks so not to owerwhelm Elasticsearch + * This method splits work into chunks so not to overwhelm Elasticsearch * when fetching statuses for a big number of rules. * * @param ruleIds A list of rule IDs (`rule.id`) to fetch summaries for @@ -71,7 +71,7 @@ export const createClientForRoutes = ( } // Merge all rule statuses into a single dict - return Object.assign({}, ...results); + return Object.assign({}, ...results.map(({ result }) => result)); } catch (e) { const ruleIdsString = `[${truncateList(ruleIds).join(', ')}]`; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts index 7381e2c3a1f02..1181995e0ae4a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts @@ -16,13 +16,13 @@ import { RulesClient, AlertServices } from '../../../../../alerting/server'; import { getExportDetailsNdjson } from './get_export_details_ndjson'; import { isAlertType } from '../rules/types'; -import { transformAlertToRule } from '../routes/rules/utils'; import { INTERNAL_RULE_ID_KEY } from '../../../../common/constants'; import { findRules } from './find_rules'; import { getRuleExceptionsForExport } from './get_export_rule_exceptions'; // eslint-disable-next-line no-restricted-imports import { legacyGetBulkRuleActionsSavedObject } from '../rule_actions/legacy_get_bulk_rule_actions_saved_object'; +import { internalRuleToAPIResponse } from '../schemas/rule_converters'; interface ExportSuccessRule { statusCode: 200; @@ -127,7 +127,7 @@ export const getRulesFromObjects = async ( ) { return { statusCode: 200, - rule: transformAlertToRule(matchingRule, null, legacyActions[matchingRule.id]), + rule: internalRuleToAPIResponse(matchingRule, null, legacyActions[matchingRule.id]), }; } else { return { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts index f726434b279f2..74fb5bfe672a0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts @@ -199,13 +199,13 @@ export interface CreateRulesOptions { export interface UpdateRulesOptions { rulesClient: RulesClient; defaultOutputIndex: string; - existingRule: SanitizedAlert | null | undefined; + existingRule: RuleAlertType | null | undefined; ruleUpdate: UpdateRulesSchema; } export interface PatchRulesOptions extends Partial { rulesClient: RulesClient; - rule: SanitizedAlert | null | undefined; + rule: RuleAlertType | null | undefined; } interface PatchRulesFieldsOptions { diff --git a/x-pack/plugins/security_solution/server/lib/machine_learning/validation.test.ts b/x-pack/plugins/security_solution/server/lib/machine_learning/validation.test.ts index 53d6279b06f63..61f305c22d796 100644 --- a/x-pack/plugins/security_solution/server/lib/machine_learning/validation.test.ts +++ b/x-pack/plugins/security_solution/server/lib/machine_learning/validation.test.ts @@ -5,29 +5,29 @@ * 2.0. */ -import { toHttpError, throwHttpError } from './validation'; +import { toAuthzError, throwAuthzError } from './validation'; -describe('toHttpError', () => { +describe('toAuthzError', () => { it('returns nothing if validation is valid', () => { - expect(toHttpError({ valid: true, message: undefined })).toBeUndefined(); + expect(toAuthzError({ valid: true, message: undefined })).toBeUndefined(); }); it('returns an HTTP error if validation is invalid', () => { - const error = toHttpError({ valid: false, message: 'validation message' }); + const error = toAuthzError({ valid: false, message: 'validation message' }); expect(error?.statusCode).toEqual(403); expect(error?.message).toEqual('validation message'); }); }); -describe('throwHttpError', () => { +describe('throwAuthzError', () => { it('does nothing if validation is valid', () => { - expect(() => throwHttpError({ valid: true, message: undefined })).not.toThrowError(); + expect(() => throwAuthzError({ valid: true, message: undefined })).not.toThrowError(); }); it('throws an error if validation is invalid', () => { let error; try { - throwHttpError({ valid: false, message: 'validation failed' }); + throwAuthzError({ valid: false, message: 'validation failed' }); } catch (e) { error = e; } diff --git a/x-pack/plugins/security_solution/server/lib/machine_learning/validation.ts b/x-pack/plugins/security_solution/server/lib/machine_learning/validation.ts index 614eff01c2bbf..45d6ebf7a905f 100644 --- a/x-pack/plugins/security_solution/server/lib/machine_learning/validation.ts +++ b/x-pack/plugins/security_solution/server/lib/machine_learning/validation.ts @@ -20,14 +20,14 @@ export class HttpAuthzError extends Error { } } -export const toHttpError = (validation: Validation): HttpAuthzError | undefined => { +export const toAuthzError = (validation: Validation): HttpAuthzError | undefined => { if (!validation.valid) { return new HttpAuthzError(validation.message); } }; -export const throwHttpError = (validation: Validation): void => { - const error = toHttpError(validation); +export const throwAuthzError = (validation: Validation): void => { + const error = toAuthzError(validation); if (error) { throw error; } diff --git a/x-pack/plugins/security_solution/server/utils/promise_pool.test.ts b/x-pack/plugins/security_solution/server/utils/promise_pool.test.ts index 585044de5856a..1203c35a4dc61 100644 --- a/x-pack/plugins/security_solution/server/utils/promise_pool.test.ts +++ b/x-pack/plugins/security_solution/server/utils/promise_pool.test.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { AbortError } from '../../../../../src/plugins/kibana_utils/common'; import { initPromisePool } from './promise_pool'; const nextTick = () => new Promise((resolve) => setImmediate(resolve)); @@ -50,7 +51,11 @@ describe('initPromisePool', () => { executor: async (x) => x, }); - expect(results).toEqual([1, 2, 3]); + expect(results).toEqual([ + { item: 1, result: 1 }, + { item: 2, result: 2 }, + { item: 3, result: 3 }, + ]); expect(errors).toEqual([]); }); @@ -65,9 +70,9 @@ describe('initPromisePool', () => { expect(results).toEqual([]); expect(errors).toEqual([ - new Error(`Error processing 1`), - new Error(`Error processing 2`), - new Error(`Error processing 3`), + { item: 1, error: new Error(`Error processing 1`) }, + { item: 2, error: new Error(`Error processing 2`) }, + { item: 3, error: new Error(`Error processing 3`) }, ]); }); @@ -104,7 +109,7 @@ describe('initPromisePool', () => { asyncTasks[3].resolve(); await nextTick(); - // Check that all taks have been settled + // Check that all tasks have been settled expect(asyncTasks).toEqual({ 1: expect.objectContaining({ status: 'resolved' }), 2: expect.objectContaining({ status: 'rejected' }), @@ -114,8 +119,11 @@ describe('initPromisePool', () => { const { results, errors } = await promisePool; // Check final results - expect(results).toEqual([1, 3]); - expect(errors).toEqual([new Error(`Error processing 2`)]); + expect(results).toEqual([ + { item: 1, result: 1 }, + { item: 3, result: 3 }, + ]); + expect(errors).toEqual([{ item: 2, error: new Error(`Error processing 2`) }]); }); it('should be possible to configure concurrency', async () => { @@ -157,7 +165,7 @@ describe('initPromisePool', () => { asyncTasks[5].resolve(); await nextTick(); - // Check that all taks have been settled + // Check that all tasks have been settled expect(asyncTasks).toEqual({ 1: expect.objectContaining({ status: 'resolved' }), 2: expect.objectContaining({ status: 'rejected' }), @@ -169,8 +177,15 @@ describe('initPromisePool', () => { const { results, errors } = await promisePool; // Check final results - expect(results).toEqual([1, 4, 5]); - expect(errors).toEqual([new Error(`Error processing 2`), new Error(`Error processing 3`)]); + expect(results).toEqual([ + { item: 1, result: 1 }, + { item: 4, result: 4 }, + { item: 5, result: 5 }, + ]); + expect(errors).toEqual([ + { item: 2, error: new Error(`Error processing 2`) }, + { item: 3, error: new Error(`Error processing 3`) }, + ]); }); it('should not execute tasks if abortSignal is aborted', async () => { @@ -183,12 +198,17 @@ describe('initPromisePool', () => { abortSignal as AbortSignal ); - const { results, errors, abortedExecutionsCount } = await promisePool; + const { results, errors } = await promisePool; // Check final results expect(results).toEqual([]); - expect(errors).toEqual([]); - expect(abortedExecutionsCount).toEqual(5); + expect(errors).toEqual([ + { item: 1, error: new AbortError() }, + { item: 2, error: new AbortError() }, + { item: 3, error: new AbortError() }, + { item: 4, error: new AbortError() }, + { item: 5, error: new AbortError() }, + ]); }); it('should abort executions of tasks if abortSignal was set to aborted during execution', async () => { @@ -209,11 +229,13 @@ describe('initPromisePool', () => { abortSignal.aborted = true; - const { results, errors, abortedExecutionsCount } = await promisePool; + const { results, errors } = await promisePool; // Check final results - expect(results).toEqual([1]); - expect(errors).toEqual([]); - expect(abortedExecutionsCount).toEqual(2); + expect(results).toEqual([{ item: 1, result: 1 }]); + expect(errors).toEqual([ + { item: 2, error: new AbortError() }, + { item: 3, error: new AbortError() }, + ]); }); }); diff --git a/x-pack/plugins/security_solution/server/utils/promise_pool.ts b/x-pack/plugins/security_solution/server/utils/promise_pool.ts index ed0922b952c77..766ba873fad53 100644 --- a/x-pack/plugins/security_solution/server/utils/promise_pool.ts +++ b/x-pack/plugins/security_solution/server/utils/promise_pool.ts @@ -5,6 +5,8 @@ * 2.0. */ +import { AbortError } from '../../../../../src/plugins/kibana_utils/common'; + interface PromisePoolArgs { concurrency?: number; items: Item[]; @@ -12,6 +14,21 @@ interface PromisePoolArgs { abortSignal?: AbortSignal; } +export interface PromisePoolError { + item: Item; + error: Error; +} + +export interface PromisePoolResult { + item: Item; + result: Result; +} + +export interface PromisePoolOutcome { + results: Array>; + errors: Array>; +} + /** * Runs promises in batches. It ensures that the number of running async tasks * doesn't exceed the concurrency parameter passed to the function. @@ -24,15 +41,14 @@ interface PromisePoolArgs { * @returns Struct holding results or errors of async tasks, aborted executions count if applicable */ -export const initPromisePool = async ({ +export const initPromisePool = async ({ concurrency = 1, items, executor, abortSignal, -}: PromisePoolArgs) => { +}: PromisePoolArgs): Promise> => { const tasks: Array> = []; - const results: Result[] = []; - const errors: unknown[] = []; + const outcome: PromisePoolOutcome = { results: [], errors: [] }; for (const item of items) { // Check if the pool is full @@ -41,17 +57,20 @@ export const initPromisePool = async ({ await Promise.race(tasks); } - // if abort signal was sent stop processing tasks further - if (abortSignal?.aborted === true) { - break; - } + const executeItem = async () => { + // if abort signal was sent stop processing tasks further + if (abortSignal?.aborted === true) { + throw new AbortError(); + } + return executor(item); + }; - const task: Promise = executor(item) + const task: Promise = executeItem() .then((result) => { - results.push(result); + outcome.results.push({ item, result }); }) .catch(async (error) => { - errors.push(error); + outcome.errors.push({ item, error }); }) .finally(() => { tasks.splice(tasks.indexOf(task), 1); @@ -63,10 +82,5 @@ export const initPromisePool = async ({ // Wait for all remaining tasks to finish await Promise.all(tasks); - const aborted = - abortSignal?.aborted === true - ? { abortedExecutionsCount: items.length - results.length - errors.length } - : undefined; - - return { results, errors, ...aborted }; + return outcome; }; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/perform_bulk_action.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/perform_bulk_action.ts index 06223fbca7070..1bbfafd8f0b14 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/perform_bulk_action.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/perform_bulk_action.ts @@ -79,15 +79,20 @@ export default ({ getService }: FtrProviderContext): void => { it('should delete rules', async () => { const ruleId = 'ruleId'; - await createRule(supertest, log, getSimpleRule(ruleId)); + const testRule = getSimpleRule(ruleId); + await createRule(supertest, log, testRule); const { body } = await postBulkAction() .send({ query: '', action: BulkAction.delete }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + + // Check that the deleted rule is returned with the response + expect(body.attributes.results.deleted[0].name).to.eql(testRule.name); - await await fetchRule(ruleId).expect(404); + // Check that the updates have been persisted + await fetchRule(ruleId).expect(404); }); it('should enable rules', async () => { @@ -98,16 +103,14 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkAction.enable }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); - - const { body: ruleBody } = await fetchRule(ruleId).expect(200); - - const referenceRule = getSimpleRuleOutput(ruleId); - referenceRule.enabled = true; + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); - const storedRule = removeServerGeneratedProperties(ruleBody); + // Check that the updated rule is returned with the response + expect(body.attributes.results.updated[0].enabled).to.eql(true); - expect(storedRule).to.eql(referenceRule); + // Check that the updates have been persisted + const { body: ruleBody } = await fetchRule(ruleId).expect(200); + expect(ruleBody.enabled).to.eql(true); }); it('should disable rules', async () => { @@ -118,26 +121,31 @@ export default ({ getService }: FtrProviderContext): void => { .send({ query: '', action: BulkAction.disable }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); - - const { body: ruleBody } = await fetchRule(ruleId).expect(200); + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); - const referenceRule = getSimpleRuleOutput(ruleId); - const storedRule = removeServerGeneratedProperties(ruleBody); + // Check that the updated rule is returned with the response + expect(body.attributes.results.updated[0].enabled).to.eql(false); - expect(storedRule).to.eql(referenceRule); + // Check that the updates have been persisted + const { body: ruleBody } = await fetchRule(ruleId).expect(200); + expect(ruleBody.enabled).to.eql(false); }); it('should duplicate rules', async () => { const ruleId = 'ruleId'; - await createRule(supertest, log, getSimpleRule(ruleId)); + const ruleToDuplicate = getSimpleRule(ruleId); + await createRule(supertest, log, ruleToDuplicate); const { body } = await postBulkAction() .send({ query: '', action: BulkAction.duplicate }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + + // Check that the duplicated rule is returned with the response + expect(body.attributes.results.created[0].name).to.eql(`${ruleToDuplicate.name} [Duplicate]`); + // Check that the updates have been persisted const { body: rulesResponse } = await supertest .get(`${DETECTION_ENGINE_RULES_URL}/_find`) .set('kbn-xsrf', 'true') @@ -165,8 +173,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setTagsBody).to.eql({ success: true, rules_count: 1 }); + expect(setTagsBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + // Check that the updated rule is returned with the response + expect(setTagsBody.attributes.results.updated[0].tags).to.eql(['reset-tag']); + + // Check that the updates have been persisted const { body: setTagsRule } = await fetchRule(ruleId).expect(200); expect(setTagsRule.tags).to.eql(['reset-tag']); @@ -184,8 +196,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(addTagsBody).to.eql({ success: true, rules_count: 1 }); + expect(addTagsBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + + // Check that the updated rule is returned with the response + expect(addTagsBody.attributes.results.updated[0].tags).to.eql(['reset-tag', ...tags]); + // Check that the updates have been persisted const { body: addedTagsRule } = await fetchRule(ruleId).expect(200); expect(addedTagsRule.tags).to.eql(['reset-tag', ...tags]); @@ -226,8 +242,12 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(setIndexBody).to.eql({ success: true, rules_count: 1 }); + expect(setIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + // Check that the updated rule is returned with the response + expect(setIndexBody.attributes.results.updated[0].index).to.eql(['initial-index-*']); + + // Check that the updates have been persisted const { body: setIndexRule } = await fetchRule(ruleId).expect(200); expect(setIndexRule.index).to.eql(['initial-index-*']); @@ -245,8 +265,15 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(addIndexBody).to.eql({ success: true, rules_count: 1 }); + expect(addIndexBody.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + + // Check that the updated rule is returned with the response + expect(addIndexBody.attributes.results.updated[0].index).to.eql([ + 'initial-index-*', + ...indices, + ]); + // Check that the updates have been persisted const { body: addIndexRule } = await fetchRule(ruleId).expect(200); expect(addIndexRule.index).to.eql(['initial-index-*', ...indices]); @@ -291,8 +318,13 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + // Check that the updated rule is returned with the response + expect(body.attributes.results.updated[0].timeline_id).to.eql(timelineId); + expect(body.attributes.results.updated[0].timeline_title).to.eql(timelineTitle); + + // Check that the updates have been persisted const { body: rule } = await fetchRule(ruleId).expect(200); expect(rule.timeline_id).to.eql(timelineId); @@ -352,8 +384,13 @@ export default ({ getService }: FtrProviderContext): void => { }) .expect(200); - expect(body).to.eql({ success: true, rules_count: 1 }); + expect(body.attributes.summary).to.eql({ failed: 0, succeeded: 1, total: 1 }); + + // Check that the updated rule is returned with the response + expect(body.attributes.results.updated[0].timeline_id).to.eql(timelineId); + expect(body.attributes.results.updated[0].timeline_title).to.eql(timelineTitle); + // Check that the updates have been persisted const { body: rule } = await fetchRule(ruleId).expect(200); expect(rule.timeline_id).to.eql(timelineId);