From 2cfed783a2adefe809c22b814f78217897e3f369 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 21 May 2024 11:39:31 -0400 Subject: [PATCH 01/54] working system actions for cases for create rule route security --- .../src/actions/index.ts | 2 +- .../cases/server/connectors/cases/index.ts | 2 +- .../model/rule_schema/common_attributes.gen.ts | 2 +- .../rule_schema/common_attributes.schema.yaml | 1 - .../api/rules/create_rule/route.ts | 3 +++ .../rule_management/logic/crud/create_rules.ts | 15 ++++++++++++++- .../normalization/rule_converters.ts | 8 ++++++++ .../rule_schema/model/rule_schemas.ts | 1 + 8 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts index 1ca2f7fceba40..475567fc5353a 100644 --- a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts +++ b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts @@ -110,12 +110,12 @@ export type RuleActionCamel = t.TypeOf; export const RuleActionCamel = t.exact( t.intersection([ t.type({ - group: RuleActionGroup, id: RuleActionId, actionTypeId: RuleActionTypeId, params: RuleActionParams, }), t.partial({ + group: RuleActionGroup, uuid: RuleActionUuid, alertsFilter: RuleActionAlertsFilter, frequency: RuleActionFrequency, diff --git a/x-pack/plugins/cases/server/connectors/cases/index.ts b/x-pack/plugins/cases/server/connectors/cases/index.ts index 6e9e6601cecc7..50c10888be5e3 100644 --- a/x-pack/plugins/cases/server/connectors/cases/index.ts +++ b/x-pack/plugins/cases/server/connectors/cases/index.ts @@ -56,7 +56,7 @@ export const getCasesConnectorType = ({ config: CasesConnectorConfigSchema, secrets: CasesConnectorSecretsSchema, }, - supportedFeatureIds: [UptimeConnectorFeatureId, AlertingConnectorFeatureId], + supportedFeatureIds: [UptimeConnectorFeatureId, AlertingConnectorFeatureId, 'siem'], minimumLicenseRequired: 'platinum' as const, isSystemActionType: true, getKibanaPrivileges: ({ params } = { params: { subAction: 'run', subActionParams: {} } }) => { diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.gen.ts b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.gen.ts index 6b6bc018c8e5c..6afb81faba06b 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.gen.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.gen.ts @@ -417,7 +417,7 @@ export const RuleAction = z.object({ * The action type used for sending notifications. */ action_type_id: z.string(), - group: RuleActionGroup, + group: RuleActionGroup.optional(), id: RuleActionId, params: RuleActionParams, uuid: NonEmptyString.optional(), diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.schema.yaml b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.schema.yaml index fadb38ef1ce5f..317971925a3a4 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.schema.yaml +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/common_attributes.schema.yaml @@ -461,7 +461,6 @@ components: $ref: '#/components/schemas/RuleActionFrequency' required: - action_type_id - - group - id - params diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts index e9c84c1c50f5c..572cf84102068 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts @@ -61,11 +61,13 @@ export const createRuleRoute = ( 'licensing', 'alerting', 'lists', + 'actions', ]); const rulesClient = ctx.alerting.getRulesClient(); const savedObjectsClient = ctx.core.savedObjects.client; const exceptionsClient = ctx.lists?.getExceptionListClient(); + const actionsClient = ctx.actions.getActionsClient(); if (request.body.rule_id != null) { const rule = await readRules({ @@ -105,6 +107,7 @@ export const createRuleRoute = ( await validateResponseActionsPermissions(ctx.securitySolution, request.body); const createdRule = await createRules({ + actionsClient, rulesClient, params: request.body, }); 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 0c39d0f39411f..649659ce9a6e8 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 @@ -5,8 +5,10 @@ * 2.0. */ +import { partition } from 'lodash'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RuleCreateProps } from '../../../../../../common/api/detection_engine/model/rule_schema'; import { convertCreateAPIToInternalSchema } from '../../normalization/rule_converters'; @@ -14,6 +16,7 @@ import type { RuleParams } from '../../../rule_schema'; export interface CreateRulesOptions { rulesClient: RulesClient; + actionsClient: ActionsClient; params: T; id?: string; immutable?: boolean; @@ -22,6 +25,7 @@ export interface CreateRulesOptions } export const createRules = async ({ + actionsClient, rulesClient, params, id, @@ -29,7 +33,16 @@ export const createRules = async ({ defaultEnabled = true, allowMissingConnectorSecrets, }: CreateRulesOptions): Promise> => { - const internalRule = convertCreateAPIToInternalSchema(params, immutable, defaultEnabled); + const [oldActions, systemActions] = partition(params.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) + ); + console.log('SYSTEM ACTIONS', systemActions); + console.log('OLD ACTIONS', oldActions); + const internalRule = convertCreateAPIToInternalSchema( + { ...params, actions: oldActions, systemActions }, + immutable, + defaultEnabled + ); const rule = await rulesClient.create({ options: { id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 50a1cecce88c8..df1169a565fef 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -24,6 +24,7 @@ import type { RelatedIntegrationArray, RequiredFieldArray, RuleCreateProps, + RuleAction, TypeSpecificCreateProps, TypeSpecificResponse, } from '../../../../../common/api/detection_engine/model/rule_schema'; @@ -492,6 +493,7 @@ export const convertCreateAPIToInternalSchema = ( input: RuleCreateProps & { related_integrations?: RelatedIntegrationArray; required_fields?: RequiredFieldArray; + systemActions: RuleAction[]; }, immutable = false, defaultEnabled = true @@ -499,6 +501,11 @@ export const convertCreateAPIToInternalSchema = ( const typeSpecificParams = typeSpecificSnakeToCamel(input); const newRuleId = input.rule_id ?? uuidv4(); + const alertSystemActions = input.systemActions.map((action) => { + const { group, ...ruleAction } = transformRuleToAlertAction(action); + return ruleAction; + }); + const alertActions = input.actions?.map((action) => transformRuleToAlertAction(action)) ?? []; const actions = transformToActionFrequency(alertActions, input.throttle); @@ -544,6 +551,7 @@ export const convertCreateAPIToInternalSchema = ( schedule: { interval: input.interval ?? '5m' }, enabled: input.enabled ?? defaultEnabled, actions, + systemActions: alertSystemActions, }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts index d116f6fd71209..7c1caef14a809 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts @@ -331,6 +331,7 @@ export interface InternalRuleCreate { }; enabled: IsRuleEnabled; actions: RuleActionArrayCamel; + systemActions: RuleActionArrayCamel; params: RuleParams; throttle?: RuleActionThrottle | null; notifyWhen?: RuleActionNotifyWhen | null; From 7082de6100af951f385f013b2258ae28bf50fd76 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 17 Jun 2024 12:08:03 -0400 Subject: [PATCH 02/54] fix merge conflict, re-delete file --- .../logic/crud/create_rules.ts | 55 ------------------- 1 file changed, 55 deletions(-) delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts 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 deleted file mode 100644 index 649659ce9a6e8..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/crud/create_rules.ts +++ /dev/null @@ -1,55 +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 { partition } from 'lodash'; -import type { SanitizedRule } from '@kbn/alerting-plugin/common'; -import type { RulesClient } from '@kbn/alerting-plugin/server'; -import type { ActionsClient } from '@kbn/actions-plugin/server'; - -import type { RuleCreateProps } from '../../../../../../common/api/detection_engine/model/rule_schema'; -import { convertCreateAPIToInternalSchema } from '../../normalization/rule_converters'; -import type { RuleParams } from '../../../rule_schema'; - -export interface CreateRulesOptions { - rulesClient: RulesClient; - actionsClient: ActionsClient; - params: T; - id?: string; - immutable?: boolean; - defaultEnabled?: boolean; - allowMissingConnectorSecrets?: boolean; -} - -export const createRules = async ({ - actionsClient, - rulesClient, - params, - id, - immutable = false, - defaultEnabled = true, - allowMissingConnectorSecrets, -}: CreateRulesOptions): Promise> => { - const [oldActions, systemActions] = partition(params.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) - ); - console.log('SYSTEM ACTIONS', systemActions); - console.log('OLD ACTIONS', oldActions); - const internalRule = convertCreateAPIToInternalSchema( - { ...params, actions: oldActions, systemActions }, - immutable, - defaultEnabled - ); - const rule = await rulesClient.create({ - options: { - id, - }, - data: internalRule, - allowMissingConnectorSecrets, - }); - - return rule; -}; From 5e137b39122f1fedb6591c3d896003210e631cd7 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 17 Jun 2024 16:10:33 -0400 Subject: [PATCH 03/54] create rule with system actions cases works after the rules client refactor was merged --- .../api/rules/create_rule/route.ts | 5 ++--- .../detection_rules_client.ts | 5 ++++- .../methods/create_custom_rule.ts | 18 ++++++++++++++---- .../server/request_context_factory.ts | 3 ++- .../plugins/security_solution/server/types.ts | 4 ++-- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts index f51126718ec79..683559abcdf89 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts @@ -56,11 +56,10 @@ export const createRuleRoute = (router: SecuritySolutionPluginRouter): void => { 'lists', 'actions', ]); - + const actionsClient = ctx.actions.getActionsClient(); const rulesClient = ctx.alerting.getRulesClient(); - const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(); + const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(actionsClient); const exceptionsClient = ctx.lists?.getExceptionListClient(); - const actionsClient = ctx.actions.getActionsClient(); if (request.body.rule_id != null) { const rule = await readRules({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index 7256441697e65..7566330931c92 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -6,6 +6,8 @@ */ import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import type { MlAuthz } from '../../../../machine_learning/authz'; import type { RuleAlertType } from '../../../rule_schema'; @@ -31,12 +33,13 @@ import { importRule } from './methods/import_rule'; import { withSecuritySpan } from '../../../../../utils/with_security_span'; export const createDetectionRulesClient = ( + actionsClient?: ActionsClient, rulesClient: RulesClient, mlAuthz: MlAuthz ): IDetectionRulesClient => ({ async createCustomRule(args: CreateCustomRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.createCustomRule', async () => { - return createCustomRule(rulesClient, args, mlAuthz); + return createCustomRule(actionsClient, rulesClient, args, mlAuthz); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts index c9b3b7b542a17..164d11f8122c5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts @@ -4,7 +4,9 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ +import { partition } from 'lodash'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RulesClient } from '@kbn/alerting-plugin/server'; import type { CreateCustomRuleArgs } from '../detection_rules_client_interface'; import type { MlAuthz } from '../../../../../machine_learning/authz'; @@ -14,14 +16,22 @@ import { convertCreateAPIToInternalSchema } from '../../../normalization/rule_co import { validateMlAuth } from '../utils'; export const createCustomRule = async ( + actionsClient?: ActionsClient, rulesClient: RulesClient, args: CreateCustomRuleArgs, mlAuthz: MlAuthz ): Promise => { - const { params } = args; - await validateMlAuth(mlAuthz, params.type); - - const internalRule = convertCreateAPIToInternalSchema(params, { immutable: false }); + const params = args.params; + const [oldActions, systemActions] = partition(params.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) + ); + console.log('SYSTEM ACTIONS', systemActions); + console.log('OLD ACTIONS', oldActions); + const internalRule = convertCreateAPIToInternalSchema({ + ...params, + actions: oldActions, + systemActions, + }); const rule = await rulesClient.create({ data: internalRule, }); diff --git a/x-pack/plugins/security_solution/server/request_context_factory.ts b/x-pack/plugins/security_solution/server/request_context_factory.ts index 1f36f7ecff234..c657211af689a 100644 --- a/x-pack/plugins/security_solution/server/request_context_factory.ts +++ b/x-pack/plugins/security_solution/server/request_context_factory.ts @@ -114,7 +114,7 @@ export class RequestContextFactory implements IRequestContextFactory { getAuditLogger, - getDetectionRulesClient: memoize(() => { + getDetectionRulesClient: memoize((actionsClient) => { const mlAuthz = buildMlAuthz({ license: licensing.license, ml: plugins.ml, @@ -123,6 +123,7 @@ export class RequestContextFactory implements IRequestContextFactory { }); return createDetectionRulesClient( + actionsClient, startPlugins.alerting.getRulesClientWithRequest(request), mlAuthz ); diff --git a/x-pack/plugins/security_solution/server/types.ts b/x-pack/plugins/security_solution/server/types.ts index 121eb7b1758f4..b851ab9228182 100644 --- a/x-pack/plugins/security_solution/server/types.ts +++ b/x-pack/plugins/security_solution/server/types.ts @@ -11,7 +11,7 @@ import type { IRouter, KibanaRequest, } from '@kbn/core/server'; -import type { ActionsApiRequestHandlerContext } from '@kbn/actions-plugin/server'; +import type { ActionsApiRequestHandlerContext, ActionsClient } from '@kbn/actions-plugin/server'; import type { AlertingApiRequestHandlerContext } from '@kbn/alerting-plugin/server'; import type { FleetRequestHandlerContext } from '@kbn/fleet-plugin/server'; import type { LicensingApiRequestHandlerContext } from '@kbn/licensing-plugin/server'; @@ -45,7 +45,7 @@ export interface SecuritySolutionApiRequestHandlerContext { getAppClient: () => AppClient; getSpaceId: () => string; getRuleDataService: () => IRuleDataService; - getDetectionRulesClient: () => IDetectionRulesClient; + getDetectionRulesClient: (actionsClient?: ActionsClient) => IDetectionRulesClient; getDetectionEngineHealthClient: () => IDetectionEngineHealthClient; getRuleExecutionLog: () => IRuleExecutionLogForRoutes; getRacClient: (req: KibanaRequest) => Promise; From 3600f32952d1397780715c518e9c308b024856f0 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 18 Jun 2024 21:14:58 -0400 Subject: [PATCH 04/54] isntantiate actions client within detection rules client --- .../api/rules/create_rule/route.ts | 3 +-- .../detection_rules_client.ts | 2 +- .../methods/create_custom_rule.ts | 21 +++++++++++-------- .../server/request_context_factory.ts | 3 ++- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts index 683559abcdf89..987cc620f877d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts @@ -56,9 +56,8 @@ export const createRuleRoute = (router: SecuritySolutionPluginRouter): void => { 'lists', 'actions', ]); - const actionsClient = ctx.actions.getActionsClient(); const rulesClient = ctx.alerting.getRulesClient(); - const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(actionsClient); + const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(); const exceptionsClient = ctx.lists?.getExceptionListClient(); if (request.body.rule_id != null) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index 7566330931c92..1d0b7c703d46a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -33,7 +33,7 @@ import { importRule } from './methods/import_rule'; import { withSecuritySpan } from '../../../../../utils/with_security_span'; export const createDetectionRulesClient = ( - actionsClient?: ActionsClient, + actionsClient: ActionsClient, rulesClient: RulesClient, mlAuthz: MlAuthz ): IDetectionRulesClient => ({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts index 164d11f8122c5..024258e417d72 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts @@ -16,22 +16,25 @@ import { convertCreateAPIToInternalSchema } from '../../../normalization/rule_co import { validateMlAuth } from '../utils'; export const createCustomRule = async ( - actionsClient?: ActionsClient, + actionsClient: ActionsClient, rulesClient: RulesClient, args: CreateCustomRuleArgs, mlAuthz: MlAuthz ): Promise => { - const params = args.params; + const { params } = args; + await validateMlAuth(mlAuthz, params.type); const [oldActions, systemActions] = partition(params.actions, (action) => actionsClient.isSystemAction(action.action_type_id) ); - console.log('SYSTEM ACTIONS', systemActions); - console.log('OLD ACTIONS', oldActions); - const internalRule = convertCreateAPIToInternalSchema({ - ...params, - actions: oldActions, - systemActions, - }); + + const internalRule = convertCreateAPIToInternalSchema( + { + ...params, + actions: oldActions, + systemActions, + }, + { immutable: false } + ); const rule = await rulesClient.create({ data: internalRule, }); diff --git a/x-pack/plugins/security_solution/server/request_context_factory.ts b/x-pack/plugins/security_solution/server/request_context_factory.ts index c657211af689a..129fa03b26a4e 100644 --- a/x-pack/plugins/security_solution/server/request_context_factory.ts +++ b/x-pack/plugins/security_solution/server/request_context_factory.ts @@ -69,6 +69,7 @@ export class RequestContextFactory implements IRequestContextFactory { const frameworkRequest = await buildFrameworkRequest(context, security, request); const coreContext = await context.core; const licensing = await context.licensing; + const actionsClient = await startPlugins.actions.getActionsClientWithRequest(request); const getSpaceId = (): string => startPlugins.spaces?.spacesService?.getSpaceId(request) || DEFAULT_SPACE_ID; @@ -114,7 +115,7 @@ export class RequestContextFactory implements IRequestContextFactory { getAuditLogger, - getDetectionRulesClient: memoize((actionsClient) => { + getDetectionRulesClient: memoize(() => { const mlAuthz = buildMlAuthz({ license: licensing.license, ml: plugins.ml, From 4d73f76b6c974e421f920c10d99c2d94d6775388 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 24 Jun 2024 21:05:49 -0400 Subject: [PATCH 05/54] working rule export --- .../common/detection_engine/transform_actions.ts | 3 +-- .../logic/export/get_export_by_object_ids.ts | 4 ++++ .../logic/export/get_export_rule_action_connectors.ts | 9 ++++++++- .../rule_management/normalization/rule_converters.ts | 9 ++++++++- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index 1f3727e6e4e0b..29caee8ff7bbb 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -24,7 +24,6 @@ export const transformRuleToAlertAction = ({ frequency, alerts_filter: alertsFilter, }: RuleAction): AlertingRuleAction => ({ - group, id, params: params as AlertingRuleAction['params'], actionTypeId, @@ -33,6 +32,7 @@ export const transformRuleToAlertAction = ({ }), ...(uuid && { uuid }), ...(frequency && { frequency }), + ...(group && { group }), }); export const transformAlertToRuleAction = ({ @@ -44,7 +44,6 @@ export const transformAlertToRuleAction = ({ frequency, alertsFilter, }: AlertingRuleAction): RuleAction => ({ - group, id, params, action_type_id: actionTypeId, 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 ce57b33227ca4..695c0e82624e0 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 @@ -40,6 +40,7 @@ export const getExportByObjectIds = async ( const rulesAndErrors = await fetchRulesByIds(rulesClient, ruleIds); const { rules, missingRuleIds } = rulesAndErrors; + console.error('RULES AND ERRORS', JSON.stringify(rulesAndErrors)); // Retrieve exceptions const exceptions = rules.flatMap((rule) => rule.exceptions_list ?? []); const { exportData: exceptionLists, exportDetails: exceptionDetails } = @@ -99,6 +100,7 @@ const fetchRulesByIds = async ( sortField: undefined, sortOrder: undefined, }); + console.error(JSON.stringify(rulesResult, null, 2)); const rulesMap = new Map>(); for (const rule of rulesResult.data) { @@ -144,6 +146,8 @@ const fetchRulesByIds = async ( } } + console.error('ABOUT TO RETURN RULES'); + return { rules, missingRuleIds, 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 6b0057103c8ee..7b2fc1782e2b7 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 @@ -76,7 +76,14 @@ export const getRuleActionConnectorsForExport = async ( actionConnectorDetails: defaultActionConnectorDetails, }; - let actionsIds = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; + const ids = [ + ...new Set( + rules.flatMap((rule) => rule.actions.map(({ id }) => id).filter((id) => id != null)) + ), + ]; + + console.error('WHAT ARE THE IDS', ids); + let actionsIds = ids; if (!actionsIds.length) return exportedActionConnectors; // handle preconfigured connectors diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 79db4ff372bb2..3cde45b53234b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -783,6 +783,13 @@ export const internalRuleToAPIResponse = ( const alertActions = rule.actions.map(transformAlertToRuleAction); const throttle = transformFromAlertThrottle(rule); const actions = transformToActionFrequency(alertActions, throttle); + const systemActions = rule.systemActions.map((action) => { + const { id, ...transformedAction } = transformAlertToRuleAction(action); + return transformedAction; + }); + + console.error('alertActions', JSON.stringify(alertActions)); + console.error('systemActions', JSON.stringify(systemActions)); return { // saved object properties @@ -806,7 +813,7 @@ export const internalRuleToAPIResponse = ( ...typeSpecificCamelToSnake(rule.params), // Actions throttle: undefined, - actions, + actions: [...actions, ...systemActions], // Execution summary execution_summary: executionSummary ?? undefined, }; From 2ef4df5a4f1585bfb1ec0a1f6acf9dd5e8fb23a8 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 25 Jun 2024 13:19:33 -0400 Subject: [PATCH 06/54] import rule with system action working --- .../detection_engine/transform_actions.ts | 1 + .../api/rules/import_rules/route.ts | 25 ++++++++++++++++++- .../detection_rules_client.ts | 2 +- .../methods/import_rule.ts | 23 ++++++++++++++--- .../get_export_rule_action_connectors.ts | 5 ++-- .../import_rule_action_connectors.ts | 4 +-- .../normalization/rule_converters.ts | 6 +++-- .../rule_management/utils/utils.ts | 17 +++++++++---- 8 files changed, 67 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index 29caee8ff7bbb..7d812759ed7a6 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -50,6 +50,7 @@ export const transformAlertToRuleAction = ({ ...(alertsFilter && { alerts_filter: alertsFilter }), ...(uuid && { uuid }), ...(frequency && { frequency }), + ...(group && { group }), }); export const transformNormalizedRuleToAlertAction = ({ 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 297a4cb587352..e347b54cc938c 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 @@ -101,6 +101,9 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C RuleExceptionsPromiseFromStreams[] >([request.body.file as HapiReadableStream, ...readAllStream]); + console.error('DID WE GET HERE 00'); + console.error('rules', JSON.stringify(rules)); + // import exceptions, includes validation const { errors: exceptionsErrors, @@ -116,11 +119,20 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite); + console.error('DID WE GET HERE 0'); + console.error( + 'parsedObjectsWithoutDuplicateErrors', + JSON.stringify(parsedObjectsWithoutDuplicateErrors) + ); + const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( parsedObjectsWithoutDuplicateErrors, - actionSOClient + actionSOClient, + actionsClient ); + console.error('DID WE GET HERE 1'); + // import actions-connectors const { successCount: actionConnectorSuccessCount, @@ -136,6 +148,13 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C overwrite: request.query.overwrite_action_connectors, }); + console.error('DID WE GET HERE 2'); + console.error('rulesWithMigratedActions', JSON.stringify(rulesWithMigratedActions)); + console.error( + 'migratedParsedObjectsWithoutDuplicateErrors', + JSON.stringify(migratedParsedObjectsWithoutDuplicateErrors) + ); + // rulesWithMigratedActions: Is returned only in case connectors were exported from different namespace and the // original rules actions' ids were replaced with new destinationIds const parsedRules = actionConnectorErrors.length @@ -148,6 +167,8 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C savedObjectsClient, }); + console.error('parsedRules', JSON.stringify(parsedRules)); + const chunkParseObjects = chunk(CHUNK_PARSED_OBJECT_SIZE, parsedRules); const importRuleResponse: ImportRuleResponse[] = await importRulesHelper({ @@ -158,6 +179,8 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C existingLists: foundReferencedExceptionLists, allowMissingConnectorSecrets: !!actionConnectors.length, }); + + console.error('DID WE GET HERE 3'); const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[]; const successes = importRuleResponse.filter((resp) => { if (isImportRegular(resp)) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index 1d4d912b64b35..b0e529146216e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -76,7 +76,7 @@ export const createDetectionRulesClient = ( async importRule(args: ImportRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.importRule', async () => { - return importRule(rulesClient, args, mlAuthz); + return importRule(actionsClient, rulesClient, args, mlAuthz); }); }, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts index 8761478e30eda..d2cc756aa6ef0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts @@ -5,7 +5,10 @@ * 2.0. */ +import { partition } from 'lodash'; import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import type { MlAuthz } from '../../../../../machine_learning/authz'; import type { ImportRuleArgs } from '../detection_rules_client_interface'; import type { RuleAlertType, RuleParams } from '../../../../rule_schema'; @@ -20,11 +23,13 @@ import { validateMlAuth } from '../utils'; import { readRules } from '../read_rules'; export const importRule = async ( + actionsClient: ActionsClient, rulesClient: RulesClient, importRulePayload: ImportRuleArgs, mlAuthz: MlAuthz ): Promise => { const { ruleToImport, overwriteRules, allowMissingConnectorSecrets } = importRulePayload; + console.error('RULE TO IMPORT', JSON.stringify(ruleToImport)); await validateMlAuth(mlAuthz, ruleToImport.type); @@ -35,9 +40,21 @@ export const importRule = async ( }); if (!existingRule) { - const internalRule = convertCreateAPIToInternalSchema(ruleToImport, { - immutable: false, - }); + const [oldActions, systemActions] = partition(ruleToImport.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) + ); + + console.error('OLD ACTIONS', oldActions); + console.error('sys actions 1', systemActions); + + const internalRule = convertCreateAPIToInternalSchema( + { + ...ruleToImport, + actions: oldActions, + systemActions, + }, + { immutable: false } + ); return rulesClient.create({ data: internalRule, 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 7b2fc1782e2b7..8ba929a74207f 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 @@ -47,10 +47,11 @@ const filterOutPredefinedActionConnectorsIds = async ( actionsClient: ActionsClient, actionsIdsToExport: string[] ): Promise => { - const allActions = await actionsClient.getAll(); + const allActions = await actionsClient.getAll({ includeSystemActions: true }); const predefinedActionsIds = allActions - .filter(({ isPreconfigured }) => isPreconfigured) + .filter(({ isPreconfigured, isSystemAction }) => isPreconfigured || isSystemAction) .map(({ id }) => id); + console.error('PREDEFINED ACTION IDS', predefinedActionsIds); if (predefinedActionsIds.length) return actionsIdsToExport.filter((id) => !predefinedActionsIds.includes(id)); return actionsIdsToExport; 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 db86ad158289c..bee0756887e81 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 @@ -105,9 +105,9 @@ export const importRuleActionConnectors = async ({ async function fetchPreconfiguredActionConnectors( actionsClient: ActionsClient ): Promise { - const knownConnectors = await actionsClient.getAll(); + const knownConnectors = await actionsClient.getAll({ includeSystemActions: true }); - return knownConnectors.filter((c) => c.isPreconfigured); + return knownConnectors.filter((c) => c.isPreconfigured || c.isSystemAction); } async function filterOutPreconfiguredConnectors( diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 3cde45b53234b..0ccd88202d123 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -564,11 +564,13 @@ export const convertCreateAPIToInternalSchema = ( const typeSpecificParams = typeSpecificSnakeToCamel(input); const newRuleId = input.rule_id ?? uuidv4(); - const alertSystemActions = input.systemActions.map((action) => { + const alertSystemActions = input.systemActions?.map((action) => { const { group, ...ruleAction } = transformRuleToAlertAction(action); return ruleAction; }); + console.error('alertSystemActions', JSON.stringify(alertSystemActions)); + const alertActions = input.actions?.map((action) => transformRuleToAlertAction(action)) ?? []; const actions = transformToActionFrequency(alertActions, input.throttle); @@ -784,7 +786,7 @@ export const internalRuleToAPIResponse = ( const throttle = transformFromAlertThrottle(rule); const actions = transformToActionFrequency(alertActions, throttle); const systemActions = rule.systemActions.map((action) => { - const { id, ...transformedAction } = transformAlertToRuleAction(action); + const transformedAction = transformAlertToRuleAction(action); return transformedAction; }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index 66fa635e768ad..b28b1502e4ba3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { partition } from 'lodash/fp'; +import { partition, isEmpty } from 'lodash/fp'; import pMap from 'p-map'; import { v4 as uuidv4 } from 'uuid'; @@ -224,22 +224,29 @@ export const swapActionIds = async ( */ export const migrateLegacyActionsIds = async ( rules: PromiseFromStreams[], - savedObjectsClient: SavedObjectsClientContract + savedObjectsClient: SavedObjectsClientContract, + actionsClient: ActionsClient ): Promise => { const isImportRule = (r: unknown): r is RuleToImport => !(r instanceof Error); const toReturn = await pMap( rules, async (rule) => { - if (isImportRule(rule)) { + if (isImportRule(rule) && rule.actions != null && !isEmpty(rule.actions)) { + const [systemActions, oldActions] = partition( + (action) => + action.action_type_id != null && actionsClient.isSystemAction(action.action_type_id) + )(rule.actions); // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) const newActions: Array = await pMap( - (rule.actions as RuleAction[]) ?? [], + (oldActions as RuleAction[]) ?? [], (action: RuleAction) => swapActionIds(action, savedObjectsClient), { concurrency: MAX_CONCURRENT_SEARCHES } ); + console.error('partitioned system actions', systemActions); + // were there any errors discovered while trying to migrate and swap the action connector ids? const [actionMigrationErrors, newlyMigratedActions] = partition( (item): item is Error => item instanceof Error @@ -250,7 +257,7 @@ export const migrateLegacyActionsIds = async ( } return [ - { ...rule, actions: newlyMigratedActions }, + { ...rule, actions: [...newlyMigratedActions, ...systemActions] }, new Error( JSON.stringify( createBulkErrorObject({ From 62b6ec84ccd0c674b2ce80fee20457fd932f1d2c Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 25 Jun 2024 16:11:50 -0400 Subject: [PATCH 07/54] adds support for update with system actions --- .../detection_rules_client.ts | 2 +- .../methods/update_rule.ts | 25 +++++++++++++++++-- .../normalization/rule_converters.ts | 3 +++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index b0e529146216e..dda5f8e20ce87 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -52,7 +52,7 @@ export const createDetectionRulesClient = ( async updateRule(args: UpdateRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.updateRule', async () => { - return updateRule(rulesClient, args, mlAuthz); + return updateRule(actionsClient, rulesClient, args, mlAuthz); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index a37b5eaddcee0..0ffd85a0ddc5e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -4,6 +4,8 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ +import { partition } from 'lodash'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RulesClient } from '@kbn/alerting-plugin/server'; import type { MlAuthz } from '../../../../../machine_learning/authz'; @@ -17,6 +19,7 @@ import { validateMlAuth, ClientError, toggleRuleEnabledOnUpdate } from '../utils import { readRules } from '../read_rules'; export const updateRule = async ( + actionsClient: ActionsClient, rulesClient: RulesClient, args: UpdateRuleArgs, mlAuthz: MlAuthz @@ -32,16 +35,34 @@ export const updateRule = async ( id, }); + console.error('EXISTING RULE', JSON.stringify(existingRule)); + if (existingRule == null) { const error = getIdError({ id, ruleId }); throw new ClientError(error.message, error.statusCode); } + // split system action updates? + + const [oldActions, systemActions] = partition(existingRule.actions, (action) => + actionsClient.isSystemAction(action.actionTypeId) + ); + + const [oldRuleUpdateActions, ruleUpdateSystemActions] = partition(ruleUpdate.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) + ); + const newInternalRule = convertUpdateAPIToInternalSchema({ - existingRule, - ruleUpdate, + existingRule: { ...existingRule, actions: oldActions, systemActions }, + ruleUpdate: { + ...ruleUpdate, + actions: oldRuleUpdateActions, + systemActions: ruleUpdateSystemActions, + }, }); + console.error('newInternalRule', JSON.stringify(newInternalRule)); + const update = await rulesClient.update({ id: existingRule.id, data: newInternalRule, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 0ccd88202d123..7f0802f1401b1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -440,6 +440,8 @@ export const convertUpdateAPIToInternalSchema = ({ existingRule, ruleUpdate, }: ConvertUpdateAPIToInternalSchemaProps) => { + const systemActions = + ruleUpdate.systemActions.map((action) => transformRuleToAlertAction(action)) ?? []; const alertActions = ruleUpdate.actions?.map((action) => transformRuleToAlertAction(action)) ?? []; const actions = transformToActionFrequency(alertActions, ruleUpdate.throttle); @@ -486,6 +488,7 @@ export const convertUpdateAPIToInternalSchema = ({ }, schedule: { interval: ruleUpdate.interval ?? '5m' }, actions, + systemActions, }; return newInternalRule; From 67e406b8ff28147014289128bc645b186990ba08 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 26 Jun 2024 14:02:07 -0400 Subject: [PATCH 08/54] adds support for system actions in patch, cleanup types --- .../src/actions/index.ts | 2 +- .../detection_engine/transform_actions.ts | 19 ++++- .../api/rules/bulk_actions/route.ts | 1 + .../logic/actions/duplicate_rule.ts | 18 ++++- .../detection_rules_client.ts | 6 +- .../methods/create_custom_rule.ts | 15 +--- .../methods/create_prebuilt_rule.ts | 4 +- .../methods/import_rule.ts | 12 +-- .../methods/patch_rule.ts | 4 +- .../methods/update_rule.ts | 11 +-- .../methods/upgrade_prebuilt_rule.ts | 6 +- .../normalization/rule_converters.ts | 74 ++++++++++++++----- .../rule_preview/api/preview_rules/route.ts | 3 +- .../rule_schema/model/rule_schemas.ts | 8 +- 14 files changed, 123 insertions(+), 60 deletions(-) diff --git a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts index 475567fc5353a..1ca2f7fceba40 100644 --- a/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts +++ b/packages/kbn-securitysolution-io-ts-alerting-types/src/actions/index.ts @@ -110,12 +110,12 @@ export type RuleActionCamel = t.TypeOf; export const RuleActionCamel = t.exact( t.intersection([ t.type({ + group: RuleActionGroup, id: RuleActionId, actionTypeId: RuleActionTypeId, params: RuleActionParams, }), t.partial({ - group: RuleActionGroup, uuid: RuleActionUuid, alertsFilter: RuleActionAlertsFilter, frequency: RuleActionFrequency, diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index 7d812759ed7a6..8dbc500eae930 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -5,7 +5,10 @@ * 2.0. */ -import type { RuleAction as AlertingRuleAction } from '@kbn/alerting-plugin/common'; +import type { + RuleAction as AlertingRuleAction, + RuleSystemAction as AlertingRuleSystemAction, +} from '@kbn/alerting-plugin/common'; import type { NormalizedAlertAction } from '@kbn/alerting-plugin/server/rules_client'; import type { NormalizedRuleAction } from '../api/detection_engine/rule_management'; import type { @@ -23,7 +26,7 @@ export const transformRuleToAlertAction = ({ uuid, frequency, alerts_filter: alertsFilter, -}: RuleAction): AlertingRuleAction => ({ +}: RuleAction): AlertingRuleAction | AlertingRuleSystemAction => ({ id, params: params as AlertingRuleAction['params'], actionTypeId, @@ -53,6 +56,18 @@ export const transformAlertToRuleAction = ({ ...(group && { group }), }); +export const transformAlertToRuleSystemAction = ({ + id, + actionTypeId, + params, + uuid, +}: AlertingRuleSystemAction): RuleAction => ({ + id, + params, + action_type_id: actionTypeId, + ...(uuid && { uuid }), +}); + export const transformNormalizedRuleToAlertAction = ({ group, id, 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 1177d4363fe29..81354fd120735 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 @@ -237,6 +237,7 @@ export const performBulkActionRoute = ( } const duplicateRuleToCreate = await duplicateRule({ + actionsClient, rule, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index dd22dac3adc77..746eda83b71e0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -6,9 +6,12 @@ */ import { v4 as uuidv4 } from 'uuid'; +import { partition } from 'lodash'; import { i18n } from '@kbn/i18n'; import { ruleTypeMappings } from '@kbn/securitysolution-rules'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import { SERVER_APP_ID } from '../../../../../../common/constants'; import type { InternalRuleCreate, RuleParams } from '../../../rule_schema'; import { transformToActionFrequency } from '../../normalization/rule_actions'; @@ -23,9 +26,13 @@ const DUPLICATE_TITLE = i18n.translate( interface DuplicateRuleParams { rule: SanitizedRule; + actionsClient: ActionsClient; } -export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise => { +export const duplicateRule = async ({ + actionsClient, + rule, +}: DuplicateRuleParams): Promise => { // Generate a new static ruleId const ruleId = uuidv4(); @@ -34,7 +41,13 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise !actionsClient.isSystemAction(action.actionTypeId) + ); + const actions = transformToActionFrequency(externalActions, rule.throttle); // Duplicated rules are always considered custom rules const immutable = false; @@ -56,5 +69,6 @@ export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise { return withSecuritySpan('DetectionRulesClient.createPrebuiltRule', async () => { - return createPrebuiltRule(rulesClient, args, mlAuthz); + return createPrebuiltRule(actionsClient, rulesClient, args, mlAuthz); }); }, @@ -58,7 +58,7 @@ export const createDetectionRulesClient = ( async patchRule(args: PatchRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.patchRule', async () => { - return patchRule(rulesClient, args, mlAuthz); + return patchRule(actionsClient, rulesClient, args, mlAuthz); }); }, @@ -70,7 +70,7 @@ export const createDetectionRulesClient = ( async upgradePrebuiltRule(args: UpgradePrebuiltRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.upgradePrebuiltRule', async () => { - return upgradePrebuiltRule(rulesClient, args, mlAuthz); + return upgradePrebuiltRule(actionsClient, rulesClient, args, mlAuthz); }); }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts index 96b4455c3d622..f97217871d7c8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_custom_rule.ts @@ -4,7 +4,6 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { partition } from 'lodash'; import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RulesClient } from '@kbn/alerting-plugin/server'; @@ -25,18 +24,10 @@ export const createCustomRule = async ( ): Promise => { const { params } = args; await validateMlAuth(mlAuthz, params.type); - const [oldActions, systemActions] = partition(params.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) - ); - const internalRule = convertCreateAPIToInternalSchema( - { - ...params, - actions: oldActions, - systemActions, - }, - { immutable: false } - ); + const internalRule = convertCreateAPIToInternalSchema(params, actionsClient, { + immutable: false, + }); const rule = await rulesClient.create({ data: internalRule, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_prebuilt_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_prebuilt_rule.ts index db510d9071c4f..4b9525c5bffaf 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_prebuilt_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_prebuilt_rule.ts @@ -6,6 +6,7 @@ */ import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { stringifyZodError } from '@kbn/zod-helpers'; import type { CreatePrebuiltRuleArgs } from '../detection_rules_client_interface'; import type { MlAuthz } from '../../../../../machine_learning/authz'; @@ -16,6 +17,7 @@ import { transform } from '../../../utils/utils'; import { validateMlAuth, RuleResponseValidationError } from '../utils'; export const createPrebuiltRule = async ( + actionsClient: ActionsClient, rulesClient: RulesClient, args: CreatePrebuiltRuleArgs, mlAuthz: MlAuthz @@ -24,7 +26,7 @@ export const createPrebuiltRule = async ( await validateMlAuth(mlAuthz, params.type); - const internalRule = convertCreateAPIToInternalSchema(params, { + const internalRule = convertCreateAPIToInternalSchema(params, actionsClient, { immutable: true, defaultEnabled: false, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts index d2cc756aa6ef0..32049b83adb00 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts @@ -47,14 +47,9 @@ export const importRule = async ( console.error('OLD ACTIONS', oldActions); console.error('sys actions 1', systemActions); - const internalRule = convertCreateAPIToInternalSchema( - { - ...ruleToImport, - actions: oldActions, - systemActions, - }, - { immutable: false } - ); + const internalRule = convertCreateAPIToInternalSchema(ruleToImport, actionsClient, { + immutable: false, + }); return rulesClient.create({ data: internalRule, @@ -64,6 +59,7 @@ export const importRule = async ( const newInternalRule = convertUpdateAPIToInternalSchema({ existingRule, ruleUpdate: ruleToImport, + actionsClient, }); return rulesClient.update({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts index b7c8c1539d664..1fab7d456973c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts @@ -6,6 +6,7 @@ */ import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { MlAuthz } from '../../../../../machine_learning/authz'; import type { PatchRuleArgs } from '../detection_rules_client_interface'; import type { RuleAlertType } from '../../../../rule_schema'; @@ -17,6 +18,7 @@ import { validateMlAuth, ClientError, toggleRuleEnabledOnUpdate } from '../utils import { readRules } from '../read_rules'; export const patchRule = async ( + actionsClient: ActionsClient, rulesClient: RulesClient, args: PatchRuleArgs, mlAuthz: MlAuthz @@ -37,7 +39,7 @@ export const patchRule = async ( await validateMlAuth(mlAuthz, nextParams.type ?? existingRule.params.type); - const patchedRule = convertPatchAPIToInternalSchema(nextParams, existingRule); + const patchedRule = convertPatchAPIToInternalSchema(nextParams, existingRule, actionsClient); const update = await rulesClient.update({ id: existingRule.id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index 0ffd85a0ddc5e..bc3a9159cc52c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -48,17 +48,10 @@ export const updateRule = async ( actionsClient.isSystemAction(action.actionTypeId) ); - const [oldRuleUpdateActions, ruleUpdateSystemActions] = partition(ruleUpdate.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) - ); - const newInternalRule = convertUpdateAPIToInternalSchema({ existingRule: { ...existingRule, actions: oldActions, systemActions }, - ruleUpdate: { - ...ruleUpdate, - actions: oldRuleUpdateActions, - systemActions: ruleUpdateSystemActions, - }, + ruleUpdate, + actionsClient, }); console.error('newInternalRule', JSON.stringify(newInternalRule)); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts index 528f81ac9c57b..8a0ea62812509 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts @@ -6,6 +6,8 @@ */ import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import type { MlAuthz } from '../../../../../machine_learning/authz'; import type { RuleAlertType, RuleParams } from '../../../../rule_schema'; import type { UpgradePrebuiltRuleArgs } from '../detection_rules_client_interface'; @@ -20,6 +22,7 @@ import { validateMlAuth, ClientError } from '../utils'; import { readRules } from '../read_rules'; export const upgradePrebuiltRule = async ( + actionsClient: ActionsClient, rulesClient: RulesClient, upgradePrebuiltRulePayload: UpgradePrebuiltRuleArgs, mlAuthz: MlAuthz @@ -53,6 +56,7 @@ export const upgradePrebuiltRule = async ( timeline_id: existingRule.params.timelineId, timeline_title: existingRule.params.timelineTitle, }, + actionsClient, { immutable: true, defaultEnabled: existingRule.enabled } ); @@ -63,7 +67,7 @@ export const upgradePrebuiltRule = async ( } // Else, simply patch it. - const patchedRule = convertPatchAPIToInternalSchema(ruleAsset, existingRule); + const patchedRule = convertPatchAPIToInternalSchema(ruleAsset, existingRule, actionsClient); await rulesClient.update({ id: existingRule.id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 7f0802f1401b1..50e022db2a691 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -6,13 +6,21 @@ */ import { v4 as uuidv4 } from 'uuid'; +import { partition } from 'lodash'; import { stringifyZodError } from '@kbn/zod-helpers'; import { BadRequestError } from '@kbn/securitysolution-es-utils'; import { ruleTypeMappings } from '@kbn/securitysolution-rules'; -import type { ResolvedSanitizedRule, SanitizedRule } from '@kbn/alerting-plugin/common'; +import type { + ResolvedSanitizedRule, + RuleSystemAction, + SanitizedRule, + SanitizedRuleAction, +} from '@kbn/alerting-plugin/common'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RequiredOptional } from '@kbn/zod-helpers'; +import type { RuleActionCamel } from '@kbn/securitysolution-io-ts-alerting-types'; import { DEFAULT_INDICATOR_SOURCE_PATH, DEFAULT_MAX_SIGNALS, @@ -41,6 +49,7 @@ import { import { transformAlertToRuleAction, transformAlertToRuleResponseAction, + transformAlertToRuleSystemAction, transformRuleToAlertAction, transformRuleToAlertResponseAction, } from '../../../../../common/detection_engine/transform_actions'; @@ -434,17 +443,28 @@ export const patchTypeSpecificSnakeToCamel = ( interface ConvertUpdateAPIToInternalSchemaProps { existingRule: SanitizedRule; ruleUpdate: RuleUpdateProps; + actionsClient: ActionsClient; } export const convertUpdateAPIToInternalSchema = ({ existingRule, ruleUpdate, + actionsClient, }: ConvertUpdateAPIToInternalSchemaProps) => { + const [ruleUpdateActions, ruleUpdateSystemActions] = partition( + ruleUpdate.actions, + (action) => !actionsClient.isSystemAction(action.action_type_id) + ); + const systemActions = - ruleUpdate.systemActions.map((action) => transformRuleToAlertAction(action)) ?? []; - const alertActions = - ruleUpdate.actions?.map((action) => transformRuleToAlertAction(action)) ?? []; - const actions = transformToActionFrequency(alertActions, ruleUpdate.throttle); + (ruleUpdateSystemActions ?? existingRule.systemActions).map((action) => + transformRuleToAlertAction(action) + ) ?? []; + const alertActions = ruleUpdateActions?.map((action) => transformRuleToAlertAction(action)) ?? []; + const actions = transformToActionFrequency( + alertActions as RuleActionCamel[], + ruleUpdate.throttle + ); const typeSpecificParams = typeSpecificSnakeToCamel(ruleUpdate); @@ -497,15 +517,31 @@ export const convertUpdateAPIToInternalSchema = ({ // eslint-disable-next-line complexity export const convertPatchAPIToInternalSchema = ( nextParams: PatchRuleRequestBody, - existingRule: SanitizedRule + existingRule: SanitizedRule, + actionsClient: ActionsClient ): InternalRuleUpdate => { const typeSpecificParams = patchTypeSpecificSnakeToCamel(nextParams, existingRule.params); const existingParams = existingRule.params; + const [existingRuleActions, existingRuleSystemActions]: [ + SanitizedRuleAction[], + RuleSystemAction[] + ] = partition( + existingRule.actions, + (action) => !actionsClient.isSystemAction(action.actionTypeId) + ); + + const [ruleUpdateActions, ruleUpdateSystemActions] = partition( + nextParams.actions, + (action) => !actionsClient.isSystemAction(action.action_type_id) + ); + const systemActions = + ruleUpdateSystemActions?.map((action) => transformRuleToAlertAction(action)) ?? + existingRuleSystemActions; const alertActions = - nextParams.actions?.map((action) => transformRuleToAlertAction(action)) ?? existingRule.actions; + ruleUpdateActions?.map((action) => transformRuleToAlertAction(action)) ?? existingRuleActions; const throttle = nextParams.throttle ?? transformFromAlertThrottle(existingRule); - const actions = transformToActionFrequency(alertActions, throttle); + const actions = transformToActionFrequency(alertActions as RuleActionCamel[], throttle); return { name: nextParams.name ?? existingRule.name, @@ -549,6 +585,7 @@ export const convertPatchAPIToInternalSchema = ( }, schedule: { interval: nextParams.interval ?? existingRule.schedule.interval }, actions, + systemActions: systemActions ?? existingRuleSystemActions, }; }; @@ -560,22 +597,25 @@ interface RuleCreateOptions { // eslint-disable-next-line complexity export const convertCreateAPIToInternalSchema = ( input: RuleCreateProps, + actionsClient: ActionsClient, options?: RuleCreateOptions ): InternalRuleCreate => { const { immutable = false, defaultEnabled = true } = options ?? {}; + const [externalActions, systemActions] = partition( + input.actions, + (action) => !actionsClient.isSystemAction(action.action_type_id) + ); + const typeSpecificParams = typeSpecificSnakeToCamel(input); const newRuleId = input.rule_id ?? uuidv4(); - const alertSystemActions = input.systemActions?.map((action) => { - const { group, ...ruleAction } = transformRuleToAlertAction(action); - return ruleAction; - }); + const alertSystemActions = systemActions?.map((action) => transformRuleToAlertAction(action)); console.error('alertSystemActions', JSON.stringify(alertSystemActions)); - const alertActions = input.actions?.map((action) => transformRuleToAlertAction(action)) ?? []; - const actions = transformToActionFrequency(alertActions, input.throttle); + const alertActions = externalActions?.map((action) => transformRuleToAlertAction(action)) ?? []; + const actions = transformToActionFrequency(alertActions as RuleActionCamel[], input.throttle); return { name: input.name, @@ -788,8 +828,8 @@ export const internalRuleToAPIResponse = ( const alertActions = rule.actions.map(transformAlertToRuleAction); const throttle = transformFromAlertThrottle(rule); const actions = transformToActionFrequency(alertActions, throttle); - const systemActions = rule.systemActions.map((action) => { - const transformedAction = transformAlertToRuleAction(action); + const systemActions = rule.systemActions?.map((action) => { + const transformedAction = transformAlertToRuleSystemAction(action); return transformedAction; }); @@ -818,7 +858,7 @@ export const internalRuleToAPIResponse = ( ...typeSpecificCamelToSnake(rule.params), // Actions throttle: undefined, - actions: [...actions, ...systemActions], + actions: [...actions, ...(systemActions ?? [])], // Execution summary execution_summary: executionSummary ?? undefined, }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts index c2faa464b75da..10dff8e3e6771 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts @@ -105,6 +105,7 @@ export const previewRulesRoute = ( const searchSourceClient = await data.search.searchSource.asScoped(request); const savedObjectsClient = coreContext.savedObjects.client; const siemClient = (await context.securitySolution).getAppClient(); + const actionsClient = (await context.actions).getActionsClient(); const timeframeEnd = request.body.timeframeEnd; let invocationCount = request.body.invocationCount; @@ -118,7 +119,7 @@ export const previewRulesRoute = ( }); } - const internalRule = convertCreateAPIToInternalSchema(request.body); + const internalRule = convertCreateAPIToInternalSchema(request.body, actionsClient); const previewRuleParams = internalRule.params; const mlAuthz = buildMlAuthz({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts index 3d0ab5ed6d51e..99afd5eeedda2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts @@ -4,7 +4,10 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import type { SanitizedRuleConfig } from '@kbn/alerting-plugin/common'; +import type { + SanitizedRuleConfig, + RuleSystemAction as AlertingRuleSystemAction, +} from '@kbn/alerting-plugin/common'; import type { RuleActionArrayCamel, RuleActionNotifyWhen, @@ -343,7 +346,7 @@ export interface InternalRuleCreate { }; enabled: IsRuleEnabled; actions: RuleActionArrayCamel; - systemActions: RuleActionArrayCamel; + systemActions: AlertingRuleSystemAction[]; params: RuleParams; throttle?: RuleActionThrottle | null; notifyWhen?: RuleActionNotifyWhen | null; @@ -356,6 +359,7 @@ export interface InternalRuleUpdate { interval: string; }; actions: RuleActionArrayCamel; + systemActions: AlertingRuleSystemAction[]; params: RuleParams; throttle?: RuleActionThrottle | null; notifyWhen?: RuleActionNotifyWhen | null; From 5e61574823f65568fdb41ed4eb36a6f4f8a96de9 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 26 Jun 2024 16:39:14 -0400 Subject: [PATCH 09/54] small fix with partition logic --- .../normalization/rule_converters.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 50e022db2a691..a986caadfd761 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -451,9 +451,8 @@ export const convertUpdateAPIToInternalSchema = ({ ruleUpdate, actionsClient, }: ConvertUpdateAPIToInternalSchemaProps) => { - const [ruleUpdateActions, ruleUpdateSystemActions] = partition( - ruleUpdate.actions, - (action) => !actionsClient.isSystemAction(action.action_type_id) + const [ruleUpdateActions, ruleUpdateSystemActions] = partition(ruleUpdate.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) ); const systemActions = @@ -526,14 +525,12 @@ export const convertPatchAPIToInternalSchema = ( const [existingRuleActions, existingRuleSystemActions]: [ SanitizedRuleAction[], RuleSystemAction[] - ] = partition( - existingRule.actions, - (action) => !actionsClient.isSystemAction(action.actionTypeId) + ] = partition(existingRule.actions, (action) => + actionsClient.isSystemAction(action.actionTypeId) ); - const [ruleUpdateActions, ruleUpdateSystemActions] = partition( - nextParams.actions, - (action) => !actionsClient.isSystemAction(action.action_type_id) + const [ruleUpdateActions, ruleUpdateSystemActions] = partition(nextParams.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) ); const systemActions = ruleUpdateSystemActions?.map((action) => transformRuleToAlertAction(action)) ?? @@ -602,9 +599,8 @@ export const convertCreateAPIToInternalSchema = ( ): InternalRuleCreate => { const { immutable = false, defaultEnabled = true } = options ?? {}; - const [externalActions, systemActions] = partition( - input.actions, - (action) => !actionsClient.isSystemAction(action.action_type_id) + const [externalActions, systemActions] = partition(input.actions, (action) => + actionsClient.isSystemAction(action.action_type_id) ); const typeSpecificParams = typeSpecificSnakeToCamel(input); From 7c72634937d975f7e57bd58039cbbf67a3238f92 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 26 Jun 2024 21:53:08 -0400 Subject: [PATCH 10/54] small cleanup and notes for cleanup / investigation tomorrow --- .../rule_management/api/rules/create_rule/route.ts | 2 +- .../logic/detection_rules_client/methods/import_rule.ts | 7 ------- .../logic/detection_rules_client/methods/update_rule.ts | 3 +-- .../logic/export/get_export_rule_action_connectors.ts | 2 ++ x-pack/plugins/security_solution/server/types.ts | 4 ++-- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts index e6b60cd1f59d6..aa6425b2e673c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/api/rules/create_rule/route.ts @@ -54,8 +54,8 @@ export const createRuleRoute = (router: SecuritySolutionPluginRouter): void => { 'licensing', 'alerting', 'lists', - 'actions', ]); + const rulesClient = ctx.alerting.getRulesClient(); const detectionRulesClient = ctx.securitySolution.getDetectionRulesClient(); const exceptionsClient = ctx.lists?.getExceptionListClient(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts index 32049b83adb00..a1b8fe221a006 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts @@ -40,13 +40,6 @@ export const importRule = async ( }); if (!existingRule) { - const [oldActions, systemActions] = partition(ruleToImport.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) - ); - - console.error('OLD ACTIONS', oldActions); - console.error('sys actions 1', systemActions); - const internalRule = convertCreateAPIToInternalSchema(ruleToImport, actionsClient, { immutable: false, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index bc3a9159cc52c..7e38378db9575 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -42,8 +42,7 @@ export const updateRule = async ( throw new ClientError(error.message, error.statusCode); } - // split system action updates? - + // partition existing rule actions within convertUpdate function, like the others const [oldActions, systemActions] = partition(existingRule.actions, (action) => actionsClient.isSystemAction(action.actionTypeId) ); 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 8ba929a74207f..25f2fefdf5b8c 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 @@ -77,6 +77,8 @@ export const getRuleActionConnectorsForExport = async ( actionConnectorDetails: defaultActionConnectorDetails, }; + // I should be able to undo the additional .filter + // check tomorrow const ids = [ ...new Set( rules.flatMap((rule) => rule.actions.map(({ id }) => id).filter((id) => id != null)) diff --git a/x-pack/plugins/security_solution/server/types.ts b/x-pack/plugins/security_solution/server/types.ts index b851ab9228182..121eb7b1758f4 100644 --- a/x-pack/plugins/security_solution/server/types.ts +++ b/x-pack/plugins/security_solution/server/types.ts @@ -11,7 +11,7 @@ import type { IRouter, KibanaRequest, } from '@kbn/core/server'; -import type { ActionsApiRequestHandlerContext, ActionsClient } from '@kbn/actions-plugin/server'; +import type { ActionsApiRequestHandlerContext } from '@kbn/actions-plugin/server'; import type { AlertingApiRequestHandlerContext } from '@kbn/alerting-plugin/server'; import type { FleetRequestHandlerContext } from '@kbn/fleet-plugin/server'; import type { LicensingApiRequestHandlerContext } from '@kbn/licensing-plugin/server'; @@ -45,7 +45,7 @@ export interface SecuritySolutionApiRequestHandlerContext { getAppClient: () => AppClient; getSpaceId: () => string; getRuleDataService: () => IRuleDataService; - getDetectionRulesClient: (actionsClient?: ActionsClient) => IDetectionRulesClient; + getDetectionRulesClient: () => IDetectionRulesClient; getDetectionEngineHealthClient: () => IDetectionEngineHealthClient; getRuleExecutionLog: () => IRuleExecutionLogForRoutes; getRacClient: (req: KibanaRequest) => Promise; From 7f7a4badb52ffa5916d94d72287ce21f912d65a8 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 26 Jun 2024 21:53:44 -0400 Subject: [PATCH 11/54] remove unused import --- .../logic/detection_rules_client/methods/import_rule.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts index a1b8fe221a006..a04590a5d9c7d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { partition } from 'lodash'; import type { RulesClient } from '@kbn/alerting-plugin/server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; From 098194df3352f4b6fdc2f436e0e3b4c0176b2288 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 27 Jun 2024 14:32:00 -0400 Subject: [PATCH 12/54] small updates and cleanup --- .../detections/pages/detection_engine/rules/types.ts | 7 +++++-- .../rule_management/logic/actions/duplicate_rule.ts | 5 ++--- .../detection_rules_client/methods/update_rule.ts | 7 +------ .../export/get_export_rule_action_connectors.ts | 6 +----- .../rule_management/normalization/rule_converters.ts | 12 ++++++++++-- .../detection_engine/rule_management/utils/utils.ts | 4 ++-- 6 files changed, 21 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts index 1bf915e1a122f..65ca67e12d1ea 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts @@ -17,7 +17,10 @@ import type { Type, } from '@kbn/securitysolution-io-ts-alerting-types'; import type { DataViewBase, Filter } from '@kbn/es-query'; -import type { RuleAction as AlertingRuleAction } from '@kbn/alerting-plugin/common'; +import type { + RuleAction as AlertingRuleAction, + RuleSystemAction as AlertingRuleSystemAction, +} from '@kbn/alerting-plugin/common'; import type { DataViewListItem } from '@kbn/data-views-plugin/common'; import type { FieldValueQueryBar } from '../../../../detection_engine/rule_creation_ui/components/query_bar'; @@ -190,7 +193,7 @@ export interface ScheduleStepRule { } export interface ActionsStepRule { - actions: AlertingRuleAction[]; + actions: AlertingRuleAction[] | AlertingRuleSystemAction[]; responseActions?: RuleResponseAction[]; enabled: boolean; kibanaSiemAppUrl?: string; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index 746eda83b71e0..ac3ec26cddc4a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -43,9 +43,8 @@ export const duplicateRule = async ({ const requiredFields = isPrebuilt ? [] : rule.params.requiredFields; // actions stuff - const [externalActions, systemActions] = partition( - rule.actions, - (action) => !actionsClient.isSystemAction(action.actionTypeId) + const [externalActions, systemActions] = partition(rule.actions, (action) => + actionsClient.isSystemAction(action.actionTypeId) ); const actions = transformToActionFrequency(externalActions, rule.throttle); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index 7e38378db9575..afba53eb73aec 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -42,13 +42,8 @@ export const updateRule = async ( throw new ClientError(error.message, error.statusCode); } - // partition existing rule actions within convertUpdate function, like the others - const [oldActions, systemActions] = partition(existingRule.actions, (action) => - actionsClient.isSystemAction(action.actionTypeId) - ); - const newInternalRule = convertUpdateAPIToInternalSchema({ - existingRule: { ...existingRule, actions: oldActions, systemActions }, + existingRule, ruleUpdate, actionsClient, }); 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 25f2fefdf5b8c..30b5c9d3c2376 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 @@ -79,11 +79,7 @@ export const getRuleActionConnectorsForExport = async ( // I should be able to undo the additional .filter // check tomorrow - const ids = [ - ...new Set( - rules.flatMap((rule) => rule.actions.map(({ id }) => id).filter((id) => id != null)) - ), - ]; + const ids = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; console.error('WHAT ARE THE IDS', ids); let actionsIds = ids; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index a986caadfd761..77f464d8001d8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -455,11 +455,19 @@ export const convertUpdateAPIToInternalSchema = ({ actionsClient.isSystemAction(action.action_type_id) ); + const [existingRuleUpdateActions, existingRuleUpdateSystemActions] = partition( + existingRule.actions, + (action) => actionsClient.isSystemAction(action.actionTypeId) + ); + const systemActions = - (ruleUpdateSystemActions ?? existingRule.systemActions).map((action) => + (ruleUpdateSystemActions ?? existingRuleUpdateSystemActions).map((action) => + transformRuleToAlertAction(action) + ) ?? []; + const alertActions = + (ruleUpdateActions ?? existingRuleUpdateActions).map((action) => transformRuleToAlertAction(action) ) ?? []; - const alertActions = ruleUpdateActions?.map((action) => transformRuleToAlertAction(action)) ?? []; const actions = transformToActionFrequency( alertActions as RuleActionCamel[], ruleUpdate.throttle diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index b28b1502e4ba3..b0764a3db9f77 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -233,14 +233,14 @@ export const migrateLegacyActionsIds = async ( rules, async (rule) => { if (isImportRule(rule) && rule.actions != null && !isEmpty(rule.actions)) { - const [systemActions, oldActions] = partition( + const [systemActions, extActions] = partition( (action) => action.action_type_id != null && actionsClient.isSystemAction(action.action_type_id) )(rule.actions); // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) const newActions: Array = await pMap( - (oldActions as RuleAction[]) ?? [], + (extActions as RuleAction[]) ?? [], (action: RuleAction) => swapActionIds(action, savedObjectsClient), { concurrency: MAX_CONCURRENT_SEARCHES } ); From fdce61971a467cf49c162039f4e7d6d33b104662 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 27 Jun 2024 15:22:57 -0400 Subject: [PATCH 13/54] fixes duplicate rule unit tests --- .../logic/actions/duplicate_rule.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts index 6684b7980ff6c..85c90933eab16 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts @@ -7,6 +7,8 @@ import { v4 as uuidv4 } from 'uuid'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import type { RuleParams } from '../../../rule_schema'; import { duplicateRule } from './duplicate_rule'; @@ -14,6 +16,8 @@ jest.mock('uuid', () => ({ v4: jest.fn(), })); +let actionsClient: jest.Mocked; + describe('duplicateRule', () => { const createTestRule = (): SanitizedRule => ({ id: 'some id', @@ -97,6 +101,7 @@ describe('duplicateRule', () => { const rule = createTestRule(); const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual({ @@ -113,6 +118,7 @@ describe('duplicateRule', () => { consumer: rule.consumer, schedule: rule.schedule, actions: rule.actions, + systemActions: rule.actions, enabled: false, // covered in a separate test }); }); @@ -122,6 +128,7 @@ describe('duplicateRule', () => { rule.name = 'PowerShell Keylogging Script'; const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -135,6 +142,7 @@ describe('duplicateRule', () => { const rule = createTestRule(); const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -151,6 +159,7 @@ describe('duplicateRule', () => { rule.enabled = true; const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -171,6 +180,7 @@ describe('duplicateRule', () => { const rule = createPrebuiltRule(); const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -194,6 +204,7 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -217,6 +228,7 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -240,6 +252,7 @@ describe('duplicateRule', () => { const rule = createCustomRule(); const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -263,6 +276,7 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -286,6 +300,7 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( @@ -302,6 +317,7 @@ describe('duplicateRule', () => { rule.params.setup = `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`; const result = await duplicateRule({ rule, + actionsClient, }); expect(result).toEqual( From 8d75153d7df5ca96077bcf4467afaffa58a6649b Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 27 Jun 2024 15:25:14 -0400 Subject: [PATCH 14/54] fixes createCustomeRule unit test --- .../detection_rules_client.create_custom_rule.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts index 1cf4afecedb26..838b620ded15c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts @@ -6,6 +6,7 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { getCreateRulesSchemaMock, @@ -30,6 +31,7 @@ describe('DetectionRulesClient.createCustomRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { jest.resetAllMocks(); @@ -37,7 +39,7 @@ describe('DetectionRulesClient.createCustomRule', () => { rulesClient = rulesClientMock.create(); rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('should create a rule with the correct parameters and options', async () => { From 0599aa21318cde1f03a2835a06cac3d30b206e02 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 27 Jun 2024 15:28:56 -0400 Subject: [PATCH 15/54] fixes unit test mocks for detection rules client --- .../detection_rules_client.create_prebuilt_rule.test.ts | 4 +++- .../detection_rules_client.delete_rule.test.ts | 5 ++++- .../detection_rules_client.import_rule.test.ts | 6 +++++- .../detection_rules_client.patch_rule.test.ts | 4 +++- .../detection_rules_client.update_rule.test.ts | 4 +++- .../detection_rules_client.upgrade_prebuilt_rule.test.ts | 4 +++- 6 files changed, 21 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts index fd3ac991a968f..ad6e89113302d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts @@ -6,6 +6,7 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { getCreateRulesSchemaMock, @@ -28,6 +29,7 @@ describe('DetectionRulesClient.createPrebuiltRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { jest.resetAllMocks(); @@ -35,7 +37,7 @@ describe('DetectionRulesClient.createPrebuiltRule', () => { rulesClient = rulesClientMock.create(); rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('creates a rule with the correct parameters and options', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts index 37cb8e0aa709e..4620c9da8def9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts @@ -6,6 +6,8 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import { buildMlAuthz } from '../../../../machine_learning/authz'; import { createDetectionRulesClient } from './detection_rules_client'; import type { IDetectionRulesClient } from './detection_rules_client_interface'; @@ -17,10 +19,11 @@ describe('DetectionRulesClient.deleteRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('should call rulesClient.delete passing the expected ruleId', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts index 4d2cb0ee65519..d641f0144af62 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts @@ -6,6 +6,8 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; + import { readRules } from './read_rules'; import { getCreateRulesSchemaMock } from '../../../../../../common/api/detection_engine/model/rule_schema/mocks'; import { getRuleMock } from '../../../routes/__mocks__/request_responses'; @@ -25,6 +27,8 @@ describe('DetectionRulesClient.importRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; + const immutable = false as const; // Can only take value of false const allowMissingConnectorSecrets = true; const ruleToImport = { @@ -42,7 +46,7 @@ describe('DetectionRulesClient.importRule', () => { beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('calls rulesClient.create with the correct parameters when rule_id does not match an installed rule', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts index 7f1c219888636..b1197e01a4ca8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts @@ -6,6 +6,7 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/rules_client.mock'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { getRuleMock } from '../../../routes/__mocks__/request_responses'; import { getMlRuleParams, getQueryRuleParams } from '../../../rule_schema/mocks'; @@ -29,10 +30,11 @@ describe('DetectionRulesClient.patchRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('calls the rulesClient with expected params', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts index 671460b046fea..eeb3a59d38e52 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts @@ -6,6 +6,7 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/rules_client.mock'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { getRuleMock } from '../../../routes/__mocks__/request_responses'; import { getMlRuleParams, getQueryRuleParams } from '../../../rule_schema/mocks'; @@ -29,10 +30,11 @@ describe('DetectionRulesClient.updateRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('calls the rulesClient with expected params', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts index 97a564cbf86e6..d84089a3e22c6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts @@ -6,6 +6,7 @@ */ import { rulesClientMock } from '@kbn/alerting-plugin/server/mocks'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { getCreateEqlRuleSchemaMock, @@ -31,10 +32,11 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); + let actionsClient: jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); }); it('throws if no matching rule_id is found', async () => { From 3c8a79943de4f572990d137b12a52b247f029ddd Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 27 Jun 2024 16:08:23 -0400 Subject: [PATCH 16/54] fixes types on UI --- .../step_rule_actions/notification_action.tsx | 14 +++++++++++--- .../pages/rule_creation/helpers.ts | 18 +++++++++++++++++- .../methods/update_rule.ts | 1 - .../rule_management/utils/utils.test.ts | 18 +++++++++++++----- 4 files changed, 41 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index c5325e449287c..82b46cc4aec92 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -9,7 +9,11 @@ import React from 'react'; import { EuiToolTip, EuiText, EuiSpacer, EuiFlexGroup, EuiFlexItem, EuiIcon } from '@elastic/eui'; import type { ActionType, AsApiContract } from '@kbn/actions-plugin/common'; import type { ActionResult } from '@kbn/actions-plugin/server'; -import type { RuleActionFrequency, RuleAction } from '@kbn/alerting-plugin/common'; +import type { + RuleActionFrequency, + RuleAction, + RuleSystemAction, +} from '@kbn/alerting-plugin/common'; import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin/public'; import { FormattedMessage } from '@kbn/i18n-react'; import { getTimeTypeValue } from '../../../rule_creation_ui/pages/rule_creation/helpers'; @@ -79,7 +83,7 @@ export const FrequencyDescription: React.FC<{ frequency?: RuleActionFrequency }> }; interface NotificationActionProps { - action: RuleAction; + action: RuleAction | RuleSystemAction; connectorTypes: ActionType[]; connectors: Array>; actionTypeRegistry: ActionTypeRegistryContract; @@ -99,6 +103,10 @@ export function NotificationAction({ const iconType = actionTypeRegistry.get(action.actionTypeId)?.iconClass ?? 'apps'; + // system actions do not have a frequency property + const isRuleAction = (actionObject: unknown): actionObject is RuleAction => + (actionObject as RuleAction).frequency != null; + return ( @@ -114,7 +122,7 @@ export function NotificationAction({ - + {isRuleAction(action) && } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts index f281b3b6b4a2b..a00573fa87816 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts @@ -25,10 +25,16 @@ import type { Type, } from '@kbn/securitysolution-io-ts-alerting-types'; import { ENDPOINT_LIST_ID } from '@kbn/securitysolution-list-constants'; +import type { + RuleAction as AlertingRuleAction, + RuleSystemAction as AlertingRuleSystemAction, +} from '@kbn/alerting-plugin/common'; + import { assertUnreachable } from '../../../../../common/utility_types'; import { transformAlertToRuleAction, transformAlertToRuleResponseAction, + transformAlertToRuleSystemAction, } from '../../../../../common/detection_engine/transform_actions'; import type { @@ -639,8 +645,18 @@ export const formatAboutStepData = ( export const formatActionsStepData = (actionsStepData: ActionsStepRule): ActionsStepRuleJson => { const { actions = [], responseActions, enabled, kibanaSiemAppUrl } = actionsStepData; + const isRuleAction = ( + action: AlertingRuleAction | AlertingRuleSystemAction + ): action is AlertingRuleAction => + (action as AlertingRuleAction).group != null && + (action as AlertingRuleAction).frequency != null; + return { - actions: actions.map((action) => transformAlertToRuleAction(action)), + actions: actions.map((action) => + isRuleAction(action) + ? transformAlertToRuleAction(action) + : transformAlertToRuleSystemAction(action) + ), response_actions: responseActions?.map(transformAlertToRuleResponseAction), enabled, meta: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index afba53eb73aec..afc496ee8c27c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -4,7 +4,6 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { partition } from 'lodash'; import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RulesClient } from '@kbn/alerting-plugin/server'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts index 61436a04c2675..878b42e619dd8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts @@ -10,6 +10,7 @@ import { Readable } from 'stream'; import { createPromiseFromStreams } from '@kbn/utils'; import type { RuleAction, ThreatMapping } from '@kbn/securitysolution-io-ts-alerting-types'; import type { PartialRule } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RuleToImport } from '../../../../../common/api/detection_engine/rule_management'; import { getCreateRulesSchemaMock } from '../../../../../common/api/detection_engine/model/rule_schema/mocks'; @@ -58,6 +59,9 @@ const createMockImportRule = async (rule: ReturnType { const { clients } = requestContextMock.createTools(); + const actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + } as unknown as jest.Mocked; describe('internalRuleToAPIResponse', () => { test('should work with a full data set', () => { @@ -627,7 +631,8 @@ describe('utils', () => { const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - soClient + soClient, + actionsClient ); expect(res).toEqual([{ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }]); }); @@ -649,7 +654,8 @@ describe('utils', () => { const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - soClient + soClient, + actionsClient ); expect(res).toEqual([ { @@ -679,7 +685,7 @@ describe('utils', () => { soClient.find.mockRejectedValueOnce(new Error('failed to query')); - const res = await migrateLegacyActionsIds(rules, soClient); + const res = await migrateLegacyActionsIds(rules, soClient, actionsClient); expect(soClient.find.mock.calls).toHaveLength(2); const [error, ruleRes] = partition( (item): item is Error => item instanceof Error @@ -722,7 +728,8 @@ describe('utils', () => { const res = await migrateLegacyActionsIds( // @ts-expect-error [rule], - soClient + soClient, + actionsClient ); expect(res[1] instanceof Error).toBeTruthy(); expect((res[1] as unknown as Error).message).toEqual( @@ -764,7 +771,8 @@ describe('utils', () => { const res = await migrateLegacyActionsIds( // @ts-expect-error [rule, rule], - soClient + soClient, + actionsClient ); expect(res[0]).toEqual({ ...rule, actions: [{ ...mockAction, id: 'new-post-8.0-id' }] }); expect(res[1]).toEqual({ ...rule, actions: [] }); From 1d02e4963136fbdb0aa472aebd60433d25711ae3 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 28 Jun 2024 13:03:16 -0400 Subject: [PATCH 17/54] partition on action.id, not action_type_id --- .../normalization/rule_converters.ts | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 77f464d8001d8..193ad32ab437e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -451,13 +451,13 @@ export const convertUpdateAPIToInternalSchema = ({ ruleUpdate, actionsClient, }: ConvertUpdateAPIToInternalSchemaProps) => { - const [ruleUpdateActions, ruleUpdateSystemActions] = partition(ruleUpdate.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) + const [ruleUpdateSystemActions, ruleUpdateActions] = partition(ruleUpdate.actions, (action) => + actionsClient.isSystemAction(action.id) ); - const [existingRuleUpdateActions, existingRuleUpdateSystemActions] = partition( + const [existingRuleUpdateSystemActions, existingRuleUpdateActions] = partition( existingRule.actions, - (action) => actionsClient.isSystemAction(action.actionTypeId) + (action) => actionsClient.isSystemAction(action.id) ); const systemActions = @@ -530,15 +530,13 @@ export const convertPatchAPIToInternalSchema = ( const typeSpecificParams = patchTypeSpecificSnakeToCamel(nextParams, existingRule.params); const existingParams = existingRule.params; - const [existingRuleActions, existingRuleSystemActions]: [ - SanitizedRuleAction[], - RuleSystemAction[] - ] = partition(existingRule.actions, (action) => - actionsClient.isSystemAction(action.actionTypeId) - ); + const [existingRuleSystemActions, existingRuleActions]: [ + RuleSystemAction[], + SanitizedRuleAction[] + ] = partition(existingRule.actions, (action) => actionsClient.isSystemAction(action.id)); - const [ruleUpdateActions, ruleUpdateSystemActions] = partition(nextParams.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) + const [ruleUpdateSystemActions, ruleUpdateActions] = partition(nextParams.actions, (action) => + actionsClient.isSystemAction(action.id) ); const systemActions = ruleUpdateSystemActions?.map((action) => transformRuleToAlertAction(action)) ?? @@ -607,8 +605,14 @@ export const convertCreateAPIToInternalSchema = ( ): InternalRuleCreate => { const { immutable = false, defaultEnabled = true } = options ?? {}; - const [externalActions, systemActions] = partition(input.actions, (action) => - actionsClient.isSystemAction(action.action_type_id) + const [systemActions, externalActions] = partition(input.actions, (action) => + actionsClient.isSystemAction(action.id) + ); + + console.error( + `SYSTEM ACTIONS: ${JSON.stringify(systemActions)}\nexternalActions: ${JSON.stringify( + externalActions + )}` ); const typeSpecificParams = typeSpecificSnakeToCamel(input); From b697826dc05aa0f769c2f3868ac35b54d1dba6a2 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 28 Jun 2024 15:24:09 -0400 Subject: [PATCH 18/54] fixes duplicate rule with system actions --- .../rule_management/logic/actions/duplicate_rule.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index ac3ec26cddc4a..4eae9b7ce5f4a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -43,8 +43,8 @@ export const duplicateRule = async ({ const requiredFields = isPrebuilt ? [] : rule.params.requiredFields; // actions stuff - const [externalActions, systemActions] = partition(rule.actions, (action) => - actionsClient.isSystemAction(action.actionTypeId) + const [systemActions, externalActions] = partition(rule.actions, (action) => + actionsClient.isSystemAction(action.id) ); const actions = transformToActionFrequency(externalActions, rule.throttle); From 43a779fa0b870f6b81c1d4222289c8501a8b97d8 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 28 Jun 2024 15:49:20 -0400 Subject: [PATCH 19/54] fixes duplicate rule functionality --- .../api/rules/bulk_actions/route.ts | 1 - .../logic/actions/duplicate_rule.ts | 16 +++------------- 2 files changed, 3 insertions(+), 14 deletions(-) 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 03c554f20bc7b..b2c6ae6e04468 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 @@ -238,7 +238,6 @@ export const performBulkActionRoute = ( } const duplicateRuleToCreate = await duplicateRule({ - actionsClient, rule, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts index 4eae9b7ce5f4a..9676a127ff646 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.ts @@ -6,11 +6,9 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { partition } from 'lodash'; import { i18n } from '@kbn/i18n'; import { ruleTypeMappings } from '@kbn/securitysolution-rules'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; -import type { ActionsClient } from '@kbn/actions-plugin/server'; import { SERVER_APP_ID } from '../../../../../../common/constants'; import type { InternalRuleCreate, RuleParams } from '../../../rule_schema'; @@ -26,13 +24,9 @@ const DUPLICATE_TITLE = i18n.translate( interface DuplicateRuleParams { rule: SanitizedRule; - actionsClient: ActionsClient; } -export const duplicateRule = async ({ - actionsClient, - rule, -}: DuplicateRuleParams): Promise => { +export const duplicateRule = async ({ rule }: DuplicateRuleParams): Promise => { // Generate a new static ruleId const ruleId = uuidv4(); @@ -42,11 +36,7 @@ export const duplicateRule = async ({ const relatedIntegrations = isPrebuilt ? [] : rule.params.relatedIntegrations; const requiredFields = isPrebuilt ? [] : rule.params.requiredFields; - // actions stuff - const [systemActions, externalActions] = partition(rule.actions, (action) => - actionsClient.isSystemAction(action.id) - ); - const actions = transformToActionFrequency(externalActions, rule.throttle); + const actions = transformToActionFrequency(rule.actions, rule.throttle); // Duplicated rules are always considered custom rules const immutable = false; @@ -68,6 +58,6 @@ export const duplicateRule = async ({ schedule: rule.schedule, enabled: false, actions, - systemActions, + systemActions: rule.systemActions ?? [], }; }; From ed4b40033c9b286d61c61dcf9965e49a08bc8112 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 28 Jun 2024 18:38:02 -0400 Subject: [PATCH 20/54] updates tests, render cases title for system action on rule details page --- .../import_rules/rule_to_import.test.ts | 14 -------- .../step_rule_actions/notification_action.tsx | 25 +++++++------ .../rule_preview/use_preview_route.tsx | 7 ++++ .../pages/rule_creation/helpers.test.ts | 36 +++++++++++++++---- .../pages/rule_creation/helpers.ts | 26 +++++++++----- .../pages/rule_creation/index.tsx | 5 ++- .../pages/rule_editing/index.tsx | 4 ++- .../logic/actions/duplicate_rule.test.ts | 14 -------- 8 files changed, 76 insertions(+), 55 deletions(-) diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/import_rules/rule_to_import.test.ts b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/import_rules/rule_to_import.test.ts index 87f6f08dc0fc6..0f0e676796fbb 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/import_rules/rule_to_import.test.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/import_rules/rule_to_import.test.ts @@ -776,20 +776,6 @@ describe('RuleToImport', () => { expectParseSuccess(result); }); - test('You cannot send in an array of actions that are missing "group"', () => { - const payload = getImportRulesSchemaMock({ - actions: [ - // @ts-expect-error assign unsupported value - { id: 'id', action_type_id: 'action_type_id', params: {} }, - ], - }); - - const result = RuleToImport.safeParse(payload); - expectParseError(result); - - expect(stringifyZodError(result.error)).toMatchInlineSnapshot(`"actions.0.group: Required"`); - }); - test('You cannot send in an array of actions that are missing "id"', () => { const payload = getImportRulesSchemaMock({ actions: [ diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index 82b46cc4aec92..92aa49b34ccd1 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -16,7 +16,10 @@ import type { } from '@kbn/alerting-plugin/common'; import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin/public'; import { FormattedMessage } from '@kbn/i18n-react'; -import { getTimeTypeValue } from '../../../rule_creation_ui/pages/rule_creation/helpers'; +import { + getTimeTypeValue, + isRuleAction, +} from '../../../rule_creation_ui/pages/rule_creation/helpers'; import * as i18n from './translations'; const DescriptionLine = ({ children }: { children: React.ReactNode }) => ( @@ -95,17 +98,19 @@ export function NotificationAction({ connectors, actionTypeRegistry, }: NotificationActionProps) { + const _isRuleAction = isRuleAction(action, actionTypeRegistry); const connectorType = connectorTypes.find(({ id }) => id === action.actionTypeId); - const connectorTypeName = connectorType?.name ?? ''; + const registeredAction = actionTypeRegistry.get(action.actionTypeId); - const connector = connectors.find(({ id }) => id === action.id); - const connectorName = connector?.name ?? ''; - - const iconType = actionTypeRegistry.get(action.actionTypeId)?.iconClass ?? 'apps'; + const connectorTypeName = _isRuleAction + ? connectorType?.name ?? '' + : registeredAction.actionTypeTitle ?? ''; + const iconType = registeredAction?.iconClass ?? 'apps'; - // system actions do not have a frequency property - const isRuleAction = (actionObject: unknown): actionObject is RuleAction => - (actionObject as RuleAction).frequency != null; + const connector = connectors.find(({ id }) => id === action.id); + const connectorName = _isRuleAction + ? connector?.name ?? '' + : registeredAction.actionTypeTitle ?? ''; return ( @@ -122,7 +127,7 @@ export function NotificationAction({ - {isRuleAction(action) && } + {_isRuleAction && } diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/rule_preview/use_preview_route.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/rule_preview/use_preview_route.tsx index ac5e01567443d..25952956b2538 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/rule_preview/use_preview_route.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/components/rule_preview/use_preview_route.tsx @@ -16,6 +16,7 @@ import type { ScheduleStepRule, TimeframePreviewOptions, } from '../../../../detections/pages/detection_engine/rules/types'; +import { useKibana } from '../../../../common/lib/kibana'; interface PreviewRouteParams { defineRuleData?: DefineStepRule; @@ -34,6 +35,10 @@ export const usePreviewRoute = ({ }: PreviewRouteParams) => { const [isRequestTriggered, setIsRequestTriggered] = useState(false); + const { + triggersActionsUi: { actionTypeRegistry }, + } = useKibana().services; + const { isLoading, response, rule, setRule } = usePreviewRule({ timeframeOptions, }); @@ -72,6 +77,7 @@ export const usePreviewRoute = ({ defineRuleData, aboutRuleData, scheduleRuleData, + actionTypeRegistry, exceptionsList, }) ); @@ -84,6 +90,7 @@ export const usePreviewRoute = ({ aboutRuleData, scheduleRuleData, exceptionsList, + actionTypeRegistry, ]); return { diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts index b61cdbc386ee1..ce3fdf51d7f4c 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.test.ts @@ -6,6 +6,9 @@ */ import type { List } from '@kbn/securitysolution-io-ts-list-types'; +import { actionTypeRegistryMock } from '@kbn/triggers-actions-ui-plugin/public/application/action_type_registry.mock'; +import type { ActionTypeRegistryContract } from '@kbn/alerts-ui-shared'; + import type { RuleCreateProps } from '../../../../../common/api/detection_engine/model/rule_schema'; import type { Rule } from '../../../rule_management/logic'; import { @@ -1078,13 +1081,19 @@ describe('helpers', () => { describe('formatActionsStepData', () => { let mockData: ActionsStepRule; + const actionTypeRegistry = { + ...actionTypeRegistryMock.create(), + get: jest.fn((actionTypeId: string) => ({ + isSystemAction: false, + })), + } as unknown as jest.Mocked; beforeEach(() => { mockData = mockActionsStepRule(); }); test('returns formatted object as ActionsStepRuleJson', () => { - const result = formatActionsStepData(mockData); + const result = formatActionsStepData(mockData, actionTypeRegistry); const expected: ActionsStepRuleJson = { actions: [], enabled: false, @@ -1108,7 +1117,7 @@ describe('helpers', () => { ...mockData, actions: [mockAction], }; - const result = formatActionsStepData(mockStepData); + const result = formatActionsStepData(mockStepData, actionTypeRegistry); const expected: ActionsStepRuleJson = { actions: [ { @@ -1133,6 +1142,7 @@ describe('helpers', () => { let mockDefine: DefineStepRule; let mockSchedule: ScheduleStepRule; let mockActions: ActionsStepRule; + const actionTypeRegistry = actionTypeRegistryMock.create(); beforeEach(() => { mockAbout = mockAboutStepRule(); @@ -1142,7 +1152,13 @@ describe('helpers', () => { }); test('returns rule with type of query when saved_id exists but shouldLoadQueryDynamically=false', () => { - const result = formatRule(mockDefine, mockAbout, mockSchedule, mockActions); + const result = formatRule( + mockDefine, + mockAbout, + mockSchedule, + mockActions, + actionTypeRegistry + ); expect(result.type).toEqual('query'); }); @@ -1152,7 +1168,8 @@ describe('helpers', () => { { ...mockDefine, shouldLoadQueryDynamically: true }, mockAbout, mockSchedule, - mockActions + mockActions, + actionTypeRegistry ); expect(result.type).toEqual('saved_query'); @@ -1170,14 +1187,21 @@ describe('helpers', () => { mockDefineStepRuleWithoutSavedId, mockAbout, mockSchedule, - mockActions + mockActions, + actionTypeRegistry ); expect(result.type).toEqual('query'); }); test('returns rule without id if ruleId does not exist', () => { - const result = formatRule(mockDefine, mockAbout, mockSchedule, mockActions); + const result = formatRule( + mockDefine, + mockAbout, + mockSchedule, + mockActions, + actionTypeRegistry + ); expect(result).not.toHaveProperty('id'); }); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts index a00573fa87816..947d29635ab8f 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/helpers.ts @@ -30,6 +30,8 @@ import type { RuleSystemAction as AlertingRuleSystemAction, } from '@kbn/alerting-plugin/common'; +import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin/public'; + import { assertUnreachable } from '../../../../../common/utility_types'; import { transformAlertToRuleAction, @@ -642,18 +644,20 @@ export const formatAboutStepData = ( return resp; }; -export const formatActionsStepData = (actionsStepData: ActionsStepRule): ActionsStepRuleJson => { - const { actions = [], responseActions, enabled, kibanaSiemAppUrl } = actionsStepData; +export const isRuleAction = ( + action: AlertingRuleAction | AlertingRuleSystemAction, + actionTypeRegistry: ActionTypeRegistryContract +): action is AlertingRuleAction => !actionTypeRegistry.get(action.actionTypeId).isSystemActionType; - const isRuleAction = ( - action: AlertingRuleAction | AlertingRuleSystemAction - ): action is AlertingRuleAction => - (action as AlertingRuleAction).group != null && - (action as AlertingRuleAction).frequency != null; +export const formatActionsStepData = ( + actionsStepData: ActionsStepRule, + actionTypeRegistry: ActionTypeRegistryContract +): ActionsStepRuleJson => { + const { actions = [], responseActions, enabled, kibanaSiemAppUrl } = actionsStepData; return { actions: actions.map((action) => - isRuleAction(action) + isRuleAction(action, actionTypeRegistry) ? transformAlertToRuleAction(action) : transformAlertToRuleSystemAction(action) ), @@ -673,13 +677,14 @@ export const formatRule = ( aboutStepData: AboutStepRule, scheduleData: ScheduleStepRule, actionsData: ActionsStepRule, + actionTypeRegistry: ActionTypeRegistryContract, exceptionsList?: List[] ): T => deepmerge.all([ formatDefineStepData(defineStepData), formatAboutStepData(aboutStepData, exceptionsList), formatScheduleStepData(scheduleData), - formatActionsStepData(actionsData), + formatActionsStepData(actionsData, actionTypeRegistry), ]) as unknown as T; export const formatPreviewRule = ({ @@ -687,10 +692,12 @@ export const formatPreviewRule = ({ aboutRuleData, scheduleRuleData, exceptionsList, + actionTypeRegistry, }: { defineRuleData: DefineStepRule; aboutRuleData: AboutStepRule; scheduleRuleData: ScheduleStepRule; + actionTypeRegistry: ActionTypeRegistryContract; exceptionsList?: List[]; }): RuleCreateProps => { const aboutStepData = { @@ -704,6 +711,7 @@ export const formatPreviewRule = ({ aboutStepData, scheduleRuleData, stepActionsDefaultValue, + actionTypeRegistry, exceptionsList ), }; diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx index 806ea9f336bd5..7e933931b8fd4 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_creation/index.tsx @@ -125,6 +125,7 @@ const CreateRulePageComponent: React.FC = () => { const { application, data: { dataViews }, + triggersActionsUi, } = useKibana().services; const loading = userInfoLoading || listsConfigLoading; const [activeStep, setActiveStep] = useState(RuleStep.defineRule); @@ -379,7 +380,8 @@ const CreateRulePageComponent: React.FC = () => { { ...localActionsStepData, enabled, - } + }, + triggersActionsUi.actionTypeRegistry ) ), ]); @@ -405,6 +407,7 @@ const CreateRulePageComponent: React.FC = () => { ruleType, startMlJobs, defineFieldsTransform, + triggersActionsUi.actionTypeRegistry, ] ); diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx index 47b67c8ed720a..a509d6e772a4e 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation_ui/pages/rule_editing/index.tsx @@ -84,7 +84,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { ] = useUserData(); const { loading: listsConfigLoading, needsConfiguration: needsListsConfiguration } = useListsConfig(); - const { data: dataServices, application } = useKibana().services; + const { data: dataServices, application, triggersActionsUi } = useKibana().services; const { navigateToApp } = application; const { detailName: ruleId } = useParams<{ detailName: string }>(); @@ -407,6 +407,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { aboutStepData, scheduleStepData, actionsStepData, + triggersActionsUi.actionTypeRegistry, rule?.exceptions_list ), ...(ruleId ? { id: ruleId } : {}), @@ -433,6 +434,7 @@ const EditRulePageComponent: FC<{ rule: RuleResponse }> = ({ rule }) => { ruleId, dispatchToaster, navigateToApp, + triggersActionsUi.actionTypeRegistry, ]); const onTabClick = useCallback(async (tab: EuiTabbedContentTab) => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts index 85c90933eab16..381a2dce89df8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/actions/duplicate_rule.test.ts @@ -7,7 +7,6 @@ import { v4 as uuidv4 } from 'uuid'; import type { SanitizedRule } from '@kbn/alerting-plugin/common'; -import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RuleParams } from '../../../rule_schema'; import { duplicateRule } from './duplicate_rule'; @@ -16,8 +15,6 @@ jest.mock('uuid', () => ({ v4: jest.fn(), })); -let actionsClient: jest.Mocked; - describe('duplicateRule', () => { const createTestRule = (): SanitizedRule => ({ id: 'some id', @@ -101,7 +98,6 @@ describe('duplicateRule', () => { const rule = createTestRule(); const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual({ @@ -128,7 +124,6 @@ describe('duplicateRule', () => { rule.name = 'PowerShell Keylogging Script'; const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -142,7 +137,6 @@ describe('duplicateRule', () => { const rule = createTestRule(); const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -159,7 +153,6 @@ describe('duplicateRule', () => { rule.enabled = true; const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -180,7 +173,6 @@ describe('duplicateRule', () => { const rule = createPrebuiltRule(); const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -204,7 +196,6 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -228,7 +219,6 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -252,7 +242,6 @@ describe('duplicateRule', () => { const rule = createCustomRule(); const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -276,7 +265,6 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -300,7 +288,6 @@ describe('duplicateRule', () => { const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( @@ -317,7 +304,6 @@ describe('duplicateRule', () => { rule.params.setup = `## Config\n\nThe 'Audit Detailed File Share' audit policy must be configured...`; const result = await duplicateRule({ rule, - actionsClient, }); expect(result).toEqual( From 033fe416b52d580decae857a3d835744aa633402 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 1 Jul 2024 12:36:42 -0400 Subject: [PATCH 21/54] removes console statements --- .../api/rules/import_rules/route.ts | 21 ------------------- .../logic/export/get_export_by_object_ids.ts | 4 ---- .../get_export_rule_action_connectors.ts | 2 -- .../normalization/rule_converters.ts | 11 ---------- .../rule_management/utils/utils.ts | 2 -- 5 files changed, 40 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 e347b54cc938c..bf62101f2f5a1 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 @@ -101,9 +101,6 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C RuleExceptionsPromiseFromStreams[] >([request.body.file as HapiReadableStream, ...readAllStream]); - console.error('DID WE GET HERE 00'); - console.error('rules', JSON.stringify(rules)); - // import exceptions, includes validation const { errors: exceptionsErrors, @@ -119,20 +116,12 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C const [duplicateIdErrors, parsedObjectsWithoutDuplicateErrors] = getTupleDuplicateErrorsAndUniqueRules(rules, request.query.overwrite); - console.error('DID WE GET HERE 0'); - console.error( - 'parsedObjectsWithoutDuplicateErrors', - JSON.stringify(parsedObjectsWithoutDuplicateErrors) - ); - const migratedParsedObjectsWithoutDuplicateErrors = await migrateLegacyActionsIds( parsedObjectsWithoutDuplicateErrors, actionSOClient, actionsClient ); - console.error('DID WE GET HERE 1'); - // import actions-connectors const { successCount: actionConnectorSuccessCount, @@ -148,13 +137,6 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C overwrite: request.query.overwrite_action_connectors, }); - console.error('DID WE GET HERE 2'); - console.error('rulesWithMigratedActions', JSON.stringify(rulesWithMigratedActions)); - console.error( - 'migratedParsedObjectsWithoutDuplicateErrors', - JSON.stringify(migratedParsedObjectsWithoutDuplicateErrors) - ); - // rulesWithMigratedActions: Is returned only in case connectors were exported from different namespace and the // original rules actions' ids were replaced with new destinationIds const parsedRules = actionConnectorErrors.length @@ -167,8 +149,6 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C savedObjectsClient, }); - console.error('parsedRules', JSON.stringify(parsedRules)); - const chunkParseObjects = chunk(CHUNK_PARSED_OBJECT_SIZE, parsedRules); const importRuleResponse: ImportRuleResponse[] = await importRulesHelper({ @@ -180,7 +160,6 @@ export const importRulesRoute = (router: SecuritySolutionPluginRouter, config: C allowMissingConnectorSecrets: !!actionConnectors.length, }); - console.error('DID WE GET HERE 3'); const errorsResp = importRuleResponse.filter((resp) => isBulkError(resp)) as BulkError[]; const successes = importRuleResponse.filter((resp) => { if (isImportRegular(resp)) { 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 695c0e82624e0..ce57b33227ca4 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 @@ -40,7 +40,6 @@ export const getExportByObjectIds = async ( const rulesAndErrors = await fetchRulesByIds(rulesClient, ruleIds); const { rules, missingRuleIds } = rulesAndErrors; - console.error('RULES AND ERRORS', JSON.stringify(rulesAndErrors)); // Retrieve exceptions const exceptions = rules.flatMap((rule) => rule.exceptions_list ?? []); const { exportData: exceptionLists, exportDetails: exceptionDetails } = @@ -100,7 +99,6 @@ const fetchRulesByIds = async ( sortField: undefined, sortOrder: undefined, }); - console.error(JSON.stringify(rulesResult, null, 2)); const rulesMap = new Map>(); for (const rule of rulesResult.data) { @@ -146,8 +144,6 @@ const fetchRulesByIds = async ( } } - console.error('ABOUT TO RETURN RULES'); - return { rules, missingRuleIds, 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 30b5c9d3c2376..f342411550ef4 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 @@ -51,7 +51,6 @@ const filterOutPredefinedActionConnectorsIds = async ( const predefinedActionsIds = allActions .filter(({ isPreconfigured, isSystemAction }) => isPreconfigured || isSystemAction) .map(({ id }) => id); - console.error('PREDEFINED ACTION IDS', predefinedActionsIds); if (predefinedActionsIds.length) return actionsIdsToExport.filter((id) => !predefinedActionsIds.includes(id)); return actionsIdsToExport; @@ -81,7 +80,6 @@ export const getRuleActionConnectorsForExport = async ( // check tomorrow const ids = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; - console.error('WHAT ARE THE IDS', ids); let actionsIds = ids; if (!actionsIds.length) return exportedActionConnectors; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 193ad32ab437e..11d10fe169fa9 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -609,19 +609,11 @@ export const convertCreateAPIToInternalSchema = ( actionsClient.isSystemAction(action.id) ); - console.error( - `SYSTEM ACTIONS: ${JSON.stringify(systemActions)}\nexternalActions: ${JSON.stringify( - externalActions - )}` - ); - const typeSpecificParams = typeSpecificSnakeToCamel(input); const newRuleId = input.rule_id ?? uuidv4(); const alertSystemActions = systemActions?.map((action) => transformRuleToAlertAction(action)); - console.error('alertSystemActions', JSON.stringify(alertSystemActions)); - const alertActions = externalActions?.map((action) => transformRuleToAlertAction(action)) ?? []; const actions = transformToActionFrequency(alertActions as RuleActionCamel[], input.throttle); @@ -841,9 +833,6 @@ export const internalRuleToAPIResponse = ( return transformedAction; }); - console.error('alertActions', JSON.stringify(alertActions)); - console.error('systemActions', JSON.stringify(systemActions)); - return { // saved object properties outcome: isResolvedRule(rule) ? rule.outcome : undefined, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index b0764a3db9f77..45fdefefb51d0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -245,8 +245,6 @@ export const migrateLegacyActionsIds = async ( { concurrency: MAX_CONCURRENT_SEARCHES } ); - console.error('partitioned system actions', systemActions); - // were there any errors discovered while trying to migrate and swap the action connector ids? const [actionMigrationErrors, newlyMigratedActions] = partition( (item): item is Error => item instanceof Error From b3f5498ad282a887c8db51f52c75fd6af8d4772a Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 1 Jul 2024 14:55:47 -0400 Subject: [PATCH 22/54] remove comment --- .../logic/export/get_export_rule_action_connectors.ts | 2 -- 1 file changed, 2 deletions(-) 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 f342411550ef4..07ce5fb67ec3e 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 @@ -76,8 +76,6 @@ export const getRuleActionConnectorsForExport = async ( actionConnectorDetails: defaultActionConnectorDetails, }; - // I should be able to undo the additional .filter - // check tomorrow const ids = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; let actionsIds = ids; From caa8702535ef21d7673d3bb340a19b69dc59c413 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 1 Jul 2024 14:57:08 -0400 Subject: [PATCH 23/54] more cleanup --- .../logic/export/get_export_rule_action_connectors.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 07ce5fb67ec3e..1c5580e2e17f9 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 @@ -76,9 +76,8 @@ export const getRuleActionConnectorsForExport = async ( actionConnectorDetails: defaultActionConnectorDetails, }; - const ids = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; + let actionsIds = [...new Set(rules.flatMap((rule) => rule.actions.map(({ id }) => id)))]; - let actionsIds = ids; if (!actionsIds.length) return exportedActionConnectors; // handle preconfigured connectors From 1b35aebbbf63bf040b988841ec58da3b3a100a7d Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 2 Jul 2024 10:01:44 -0400 Subject: [PATCH 24/54] fixes issue where if actions were not present in the update (i.e. patch) the aciton would be set to [] --- .../normalization/rule_converters.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 11d10fe169fa9..2e1d8cec4b963 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -6,7 +6,7 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { partition } from 'lodash'; +import { isEmpty, partition } from 'lodash'; import { stringifyZodError } from '@kbn/zod-helpers'; import { BadRequestError } from '@kbn/securitysolution-es-utils'; @@ -538,11 +538,13 @@ export const convertPatchAPIToInternalSchema = ( const [ruleUpdateSystemActions, ruleUpdateActions] = partition(nextParams.actions, (action) => actionsClient.isSystemAction(action.id) ); - const systemActions = - ruleUpdateSystemActions?.map((action) => transformRuleToAlertAction(action)) ?? - existingRuleSystemActions; - const alertActions = - ruleUpdateActions?.map((action) => transformRuleToAlertAction(action)) ?? existingRuleActions; + const systemActions = !isEmpty(ruleUpdateSystemActions) + ? ruleUpdateSystemActions.map((action) => transformRuleToAlertAction(action)) + : existingRuleSystemActions; + const alertActions = !isEmpty(ruleUpdateActions) + ? ruleUpdateActions.map((action) => transformRuleToAlertAction(action)) + : existingRuleActions; + const throttle = nextParams.throttle ?? transformFromAlertThrottle(existingRule); const actions = transformToActionFrequency(alertActions as RuleActionCamel[], throttle); @@ -588,7 +590,7 @@ export const convertPatchAPIToInternalSchema = ( }, schedule: { interval: nextParams.interval ?? existingRule.schedule.interval }, actions, - systemActions: systemActions ?? existingRuleSystemActions, + systemActions, }; }; From 9eeb2e67697195721197b02a7811705ec3a9590e Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 2 Jul 2024 10:09:53 -0400 Subject: [PATCH 25/54] remove test cases looking for 'group' property, updates actionsClient mocks --- .../model/rule_schema/rule_request_schema.test.ts | 11 ----------- .../crud/patch_rule/patch_rule_route.test.ts | 13 ------------- .../model/rule_assets/prebuilt_rule_asset.test.ts | 11 ----------- .../detection_rules_client.patch_rule.test.ts | 5 ++++- .../detection_rules_client.update_rule.test.ts | 4 +++- 5 files changed, 7 insertions(+), 37 deletions(-) diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts index b7435c7dd86e8..2d3235f123520 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/model/rule_schema/rule_request_schema.test.ts @@ -719,17 +719,6 @@ describe('rules schema', () => { ); }); - test('You cannot send in an array of actions that are missing "group"', () => { - const payload = { - ...getCreateRulesSchemaMock(), - actions: [{ id: 'id', action_type_id: 'action_type_id', params: {} }], - }; - - const result = RuleCreateProps.safeParse(payload); - expectParseError(result); - expect(stringifyZodError(result.error)).toEqual('actions.0.group: Required'); - }); - test('You cannot send in an array of actions that are missing "id"', () => { const payload = { ...getCreateRulesSchemaMock(), diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/crud/patch_rule/patch_rule_route.test.ts b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/crud/patch_rule/patch_rule_route.test.ts index 1994b3de5d453..756ef69b18f5a 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/crud/patch_rule/patch_rule_route.test.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/crud/patch_rule/patch_rule_route.test.ts @@ -766,19 +766,6 @@ describe('Patch rule request schema', () => { }); }); - test('You cannot send in an array of actions that are missing "group"', () => { - const payload: Omit = { - ...getPatchRulesSchemaMock(), - actions: [{ id: 'id', action_type_id: 'action_type_id', params: {} }], - }; - - const result = PatchRuleRequestBody.safeParse(payload); - expectParseError(result); - expect(stringifyZodError(result.error)).toMatchInlineSnapshot( - `"actions.0.group: Required, type: Invalid literal value, expected \\"eql\\", language: Invalid literal value, expected \\"eql\\", actions.0.group: Required, actions.0.group: Required, and 12 more"` - ); - }); - test('You cannot send in an array of actions that are missing "id"', () => { const payload: Omit = { ...getPatchRulesSchemaMock(), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.test.ts index 06ccf0a2b97f4..1cd8f05b19930 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/prebuilt_rules/model/rule_assets/prebuilt_rule_asset.test.ts @@ -500,17 +500,6 @@ describe('Prebuilt rule asset schema', () => { ); }); - test('You cannot send in an array of actions that are missing "group"', () => { - const payload: Omit = { - ...getPrebuiltRuleMock(), - actions: [{ id: 'id', action_type_id: 'action_type_id', params: {} }], - }; - - const result = PrebuiltRuleAsset.safeParse(payload); - expectParseError(result); - expect(stringifyZodError(result.error)).toMatchInlineSnapshot(`"actions.0.group: Required"`); - }); - test('You cannot send in an array of actions that are missing "id"', () => { const payload: Omit = { ...getPrebuiltRuleMock(), diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts index b1197e01a4ca8..4f86a32d967d1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts @@ -30,7 +30,10 @@ describe('DetectionRulesClient.patchRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - let actionsClient: jest.Mocked; + + const actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + } as unknown as jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts index eeb3a59d38e52..f3dd52b374d4e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts @@ -30,7 +30,9 @@ describe('DetectionRulesClient.updateRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - let actionsClient: jest.Mocked; + const actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + } as unknown as jest.Mocked; beforeEach(() => { rulesClient = rulesClientMock.create(); From c250336282a1459296786575fa8d7f1077294aea Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 12:05:17 -0400 Subject: [PATCH 26/54] remove hardcoded string value for siem feature id in cases action --- .../plugins/cases/server/connectors/cases/index.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/cases/server/connectors/cases/index.ts b/x-pack/plugins/cases/server/connectors/cases/index.ts index 50c10888be5e3..2a43a4c047a93 100644 --- a/x-pack/plugins/cases/server/connectors/cases/index.ts +++ b/x-pack/plugins/cases/server/connectors/cases/index.ts @@ -5,7 +5,11 @@ * 2.0. */ -import { AlertingConnectorFeatureId, UptimeConnectorFeatureId } from '@kbn/actions-plugin/common'; +import { + AlertingConnectorFeatureId, + UptimeConnectorFeatureId, + SecurityConnectorFeatureId, +} from '@kbn/actions-plugin/common'; import type { SubActionConnectorType } from '@kbn/actions-plugin/server/sub_action_framework/types'; import type { KibanaRequest } from '@kbn/core-http-server'; import type { SavedObjectsClientContract } from '@kbn/core/server'; @@ -56,7 +60,11 @@ export const getCasesConnectorType = ({ config: CasesConnectorConfigSchema, secrets: CasesConnectorSecretsSchema, }, - supportedFeatureIds: [UptimeConnectorFeatureId, AlertingConnectorFeatureId, 'siem'], + supportedFeatureIds: [ + UptimeConnectorFeatureId, + AlertingConnectorFeatureId, + SecurityConnectorFeatureId, + ], minimumLicenseRequired: 'platinum' as const, isSystemActionType: true, getKibanaPrivileges: ({ params } = { params: { subAction: 'run', subActionParams: {} } }) => { From dfb239e458bb8f79326170d9cd6e36fa731fe4bd Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 12:20:35 -0400 Subject: [PATCH 27/54] updates rules client to use named parameters --- ...ction_rules_client.create_custom_rule.test.ts | 2 +- ...ion_rules_client.create_prebuilt_rule.test.ts | 2 +- .../detection_rules_client.delete_rule.test.ts | 2 +- .../detection_rules_client.import_rule.test.ts | 2 +- .../detection_rules_client.patch_rule.test.ts | 2 +- .../detection_rules_client.ts | 16 +++++++++++----- .../detection_rules_client.update_rule.test.ts | 2 +- ...on_rules_client.upgrade_prebuilt_rule.test.ts | 2 +- 8 files changed, 18 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts index bfde141359c8a..5b99dfbcd1b4d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts @@ -37,7 +37,7 @@ describe('DetectionRulesClient.createCustomRule', () => { rulesClient = rulesClientMock.create(); rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('should create a rule with the correct parameters and options', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts index ad6e89113302d..9f9360bce225e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_prebuilt_rule.test.ts @@ -37,7 +37,7 @@ describe('DetectionRulesClient.createPrebuiltRule', () => { rulesClient = rulesClientMock.create(); rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('creates a rule with the correct parameters and options', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts index 4620c9da8def9..52f2ed69c34ed 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.delete_rule.test.ts @@ -23,7 +23,7 @@ describe('DetectionRulesClient.deleteRule', () => { beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('should call rulesClient.delete passing the expected ruleId', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts index 1a856db9ac168..39183cf66dbcc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.import_rule.test.ts @@ -48,7 +48,7 @@ describe('DetectionRulesClient.importRule', () => { rulesClient = rulesClientMock.create(); rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); rulesClient.update.mockResolvedValue(getRuleMock(getQueryRuleParams())); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('calls rulesClient.create with the correct parameters when rule_id does not match an installed rule', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts index 4f86a32d967d1..1314dc1350a8d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts @@ -37,7 +37,7 @@ describe('DetectionRulesClient.patchRule', () => { beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('calls the rulesClient with expected params', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts index 529e3b8ecbd8c..3e7dcb29f2887 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.ts @@ -32,11 +32,17 @@ import { importRule } from './methods/import_rule'; import { withSecuritySpan } from '../../../../../utils/with_security_span'; -export const createDetectionRulesClient = ( - actionsClient: ActionsClient, - rulesClient: RulesClient, - mlAuthz: MlAuthz -): IDetectionRulesClient => ({ +interface DetectionRulesClientParams { + actionsClient: ActionsClient; + rulesClient: RulesClient; + mlAuthz: MlAuthz; +} + +export const createDetectionRulesClient = ({ + actionsClient, + rulesClient, + mlAuthz, +}: DetectionRulesClientParams): IDetectionRulesClient => ({ async createCustomRule(args: CreateCustomRuleArgs): Promise { return withSecuritySpan('DetectionRulesClient.createCustomRule', async () => { return createCustomRule(actionsClient, rulesClient, args, mlAuthz); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts index f3dd52b374d4e..28d089422e7eb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts @@ -36,7 +36,7 @@ describe('DetectionRulesClient.updateRule', () => { beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('calls the rulesClient with expected params', async () => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts index 20cc038fd2ad5..ae4b7ef18afee 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts @@ -36,7 +36,7 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { beforeEach(() => { rulesClient = rulesClientMock.create(); - detectionRulesClient = createDetectionRulesClient(actionsClient, rulesClient, mlAuthz); + detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); it('throws if no matching rule_id is found', async () => { From 1fbafc3c2d24251daf2531f346a1feefb01a1e4b Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 13:30:48 -0400 Subject: [PATCH 28/54] named params in context factor, adds test for migrate legacy actions, fix logic in migrate legacy actions partition, adds comment for why we are partitioning inside legacy actions migration function --- .../rule_management/utils/utils.test.ts | 21 ++++++++++++++++++- .../rule_management/utils/utils.ts | 8 +++---- .../server/request_context_factory.ts | 8 +++---- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts index 878b42e619dd8..32c43c52a4247 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.test.ts @@ -60,7 +60,7 @@ const createMockImportRule = async (rule: ReturnType { const { clients } = requestContextMock.createTools(); const actionsClient = { - isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), } as unknown as jest.Mocked; describe('internalRuleToAPIResponse', () => { @@ -788,6 +788,25 @@ describe('utils', () => { }) ); }); + test('does not migrate system actions', async () => { + const mockSystemAction: RuleAction = { + group: 'group string', + id: 'system-connector-.cases', + action_type_id: '.case', + params: {}, + }; + const rule: ReturnType = { + ...getCreateRulesSchemaMock('rule-1'), + actions: [mockSystemAction], + }; + const res = await migrateLegacyActionsIds( + // @ts-expect-error + [rule], + soClient, + actionsClient + ); + expect(res).toEqual([{ ...rule, actions: [{ ...mockSystemAction }] }]); + }); }); describe('getInvalidConnectors', () => { beforeEach(() => { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index 45fdefefb51d0..47683aab41090 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -233,9 +233,9 @@ export const migrateLegacyActionsIds = async ( rules, async (rule) => { if (isImportRule(rule) && rule.actions != null && !isEmpty(rule.actions)) { - const [systemActions, extActions] = partition( - (action) => - action.action_type_id != null && actionsClient.isSystemAction(action.action_type_id) + // filter out system actions, since they were not part of any 7.x releases and do not need to be migrated + const [systemActions, extActions] = partition((action) => + actionsClient.isSystemAction(action.id) )(rule.actions); // can we swap the pre 8.0 action connector(s) id with the new, // post-8.0 action id (swap the originId for the new _id?) @@ -251,7 +251,7 @@ export const migrateLegacyActionsIds = async ( )(newActions); if (actionMigrationErrors == null || actionMigrationErrors.length === 0) { - return { ...rule, actions: newlyMigratedActions }; + return { ...rule, actions: [...newlyMigratedActions, ...systemActions] }; } return [ diff --git a/x-pack/plugins/security_solution/server/request_context_factory.ts b/x-pack/plugins/security_solution/server/request_context_factory.ts index 129fa03b26a4e..d3bcf332a047e 100644 --- a/x-pack/plugins/security_solution/server/request_context_factory.ts +++ b/x-pack/plugins/security_solution/server/request_context_factory.ts @@ -123,11 +123,11 @@ export class RequestContextFactory implements IRequestContextFactory { savedObjectsClient: coreContext.savedObjects.client, }); - return createDetectionRulesClient( + return createDetectionRulesClient({ actionsClient, - startPlugins.alerting.getRulesClientWithRequest(request), - mlAuthz - ); + rulesClient: startPlugins.alerting.getRulesClientWithRequest(request), + mlAuthz, + }); }), getDetectionEngineHealthClient: memoize(() => From 0e42109e4977ec8bf88410da9e27084fd32cf4a4 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 13:44:21 -0400 Subject: [PATCH 29/54] fix mistyped parameter for rule create / edit form --- .../public/detections/pages/detection_engine/rules/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts index 65ca67e12d1ea..01ccdb4c599e1 100644 --- a/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/types.ts @@ -193,7 +193,7 @@ export interface ScheduleStepRule { } export interface ActionsStepRule { - actions: AlertingRuleAction[] | AlertingRuleSystemAction[]; + actions: Array; responseActions?: RuleResponseAction[]; enabled: boolean; kibanaSiemAppUrl?: string; From 72540951a1e52af988bee4710eff7ca92c5e4990 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 15:46:32 -0400 Subject: [PATCH 30/54] adds tests for validating changes made to convertCreateAPIToInternalSchema in support of system actions works within the create function --- ...on_rules_client.create_custom_rule.test.ts | 191 +++++++++++++++++- 1 file changed, 190 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts index 5b99dfbcd1b4d..4db5010ee8318 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts @@ -29,12 +29,15 @@ describe('DetectionRulesClient.createCustomRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - let actionsClient: jest.Mocked; + const actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; beforeEach(() => { jest.resetAllMocks(); rulesClient = rulesClientMock.create(); + // creates a rule with a system action and a connector action rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); @@ -58,6 +61,192 @@ describe('DetectionRulesClient.createCustomRule', () => { ); }); + it('should create a rule with actions and system actions', async () => { + rulesClient.create.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + }) + ); + const params = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + + await detectionRulesClient.createCustomRule({ params }); + + expect(rulesClient.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + enabled: true, + params: expect.objectContaining({ + description: params.description, + immutable: false, + }), + }), + }) + ); + }); + + it('should create a rule with system actions', async () => { + rulesClient.create.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + const params = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }; + + await detectionRulesClient.createCustomRule({ params }); + + expect(rulesClient.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + enabled: true, + params: expect.objectContaining({ + description: params.description, + immutable: false, + }), + }), + }) + ); + }); + + it('should create a rule with actions', async () => { + rulesClient.create.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + }) + ); + const params = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + + await detectionRulesClient.createCustomRule({ params }); + + expect(rulesClient.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + enabled: true, + params: expect.objectContaining({ + description: params.description, + immutable: false, + }), + }), + }) + ); + }); + it('throws if mlAuth fails', async () => { (throwAuthzError as jest.Mock).mockImplementationOnce(() => { throw new Error('mocked MLAuth error'); From 77e351b86dc1f15371d8f5771e3d3f7d642a81a1 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 16:24:28 -0400 Subject: [PATCH 31/54] fixes create_custom_rule test mock --- ...on_rules_client.create_custom_rule.test.ts | 96 ++++++++++++++++--- 1 file changed, 82 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts index 4db5010ee8318..59cfef772da00 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.create_custom_rule.test.ts @@ -29,13 +29,17 @@ describe('DetectionRulesClient.createCustomRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - const actionsClient = { + let actionsClient = { isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), } as unknown as jest.Mocked; beforeEach(() => { jest.resetAllMocks(); + actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; + rulesClient = rulesClientMock.create(); // creates a rule with a system action and a connector action rulesClient.create.mockResolvedValue(getRuleMock(getQueryRuleParams())); @@ -64,6 +68,18 @@ describe('DetectionRulesClient.createCustomRule', () => { it('should create a rule with actions and system actions', async () => { rulesClient.create.mockResolvedValue( getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], systemActions: [ { id: 'system-connector-.cases', @@ -79,18 +95,6 @@ describe('DetectionRulesClient.createCustomRule', () => { uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', }, ], - actions: [ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, - group: 'default', - }, - ], }) ); const params = { @@ -126,11 +130,44 @@ describe('DetectionRulesClient.createCustomRule', () => { ], }; - await detectionRulesClient.createCustomRule({ params }); + await detectionRulesClient.createCustomRule({ + params, + }); expect(rulesClient.create).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), enabled: true, params: expect.objectContaining({ description: params.description, @@ -185,6 +222,21 @@ describe('DetectionRulesClient.createCustomRule', () => { expect(rulesClient.create).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), enabled: true, params: expect.objectContaining({ description: params.description, @@ -237,6 +289,22 @@ describe('DetectionRulesClient.createCustomRule', () => { expect(rulesClient.create).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), enabled: true, params: expect.objectContaining({ description: params.description, From 0e4f045385fac40a3705a0f2ef8fab475f5d74d1 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 17:54:45 -0400 Subject: [PATCH 32/54] updates detection rules client patch and update() unit tests --- .../detection_rules_client.patch_rule.test.ts | 364 +++++++++++++++++- ...detection_rules_client.update_rule.test.ts | 256 +++++++++++- .../normalization/rule_converters.ts | 18 +- .../rule_schema/model/rule_schemas.ts | 2 +- 4 files changed, 621 insertions(+), 19 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts index 1314dc1350a8d..18939b20cd601 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts @@ -31,11 +31,15 @@ describe('DetectionRulesClient.patchRule', () => { const mlAuthz = (buildMlAuthz as jest.Mock)(); - const actionsClient = { - isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + let actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), } as unknown as jest.Mocked; beforeEach(() => { + actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; + rulesClient = rulesClientMock.create(); detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); @@ -61,6 +65,362 @@ describe('DetectionRulesClient.patchRule', () => { ); }); + it('calls rule update with rule system actions if nextParams has system actions', async () => { + const nextParams = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], // changing this value + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + const existingRule = getRuleMock(getQueryRuleParams()); + (readRules as jest.Mock).mockResolvedValueOnce(existingRule); + rulesClient.update.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + + await detectionRulesClient.patchRule({ nextParams }); + + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), + }), + }) + ); + }); + + it('calls rule update with rule system actions if nextParams has no system actions and existing rule does have system actions', async () => { + const nextParams = { + ...getCreateRulesSchemaMock(), + enabled: true, + }; + const existingRule = getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }); + (readRules as jest.Mock).mockResolvedValueOnce(existingRule); + rulesClient.update.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + + await detectionRulesClient.patchRule({ nextParams }); + + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), + }), + }) + ); + }); + + it('patches system actions if nextParams has system actions', async () => { + const nextParams = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], // changing this value + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + const existingRule = getRuleMock(getQueryRuleParams(), { + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], // changing this value + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }); + (readRules as jest.Mock).mockResolvedValueOnce(existingRule); + rulesClient.update.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + + await detectionRulesClient.patchRule({ nextParams }); + + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), + }), + }) + ); + }); + it('returns rule enabled: true if the nexParams have enabled: true', async () => { const nextParams = { ...getCreateRulesSchemaMock(), enabled: true }; const existingRule = getRuleMock(getQueryRuleParams()); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts index 28d089422e7eb..6aaf8fc4d402e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts @@ -30,11 +30,15 @@ describe('DetectionRulesClient.updateRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - const actionsClient = { - isSystemAction: jest.fn((id: string) => id === 'system_action:id'), + let actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), } as unknown as jest.Mocked; beforeEach(() => { + actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; + rulesClient = rulesClientMock.create(); detectionRulesClient = createDetectionRulesClient({ actionsClient, rulesClient, mlAuthz }); }); @@ -60,6 +64,254 @@ describe('DetectionRulesClient.updateRule', () => { ); }); + it('calls the rulesClient with actions and system actions', async () => { + const ruleUpdate = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + const existingRule = getRuleMock(getQueryRuleParams()); + (readRules as jest.Mock).mockResolvedValueOnce(existingRule); + rulesClient.update.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + + await detectionRulesClient.updateRule({ ruleUpdate }); + + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), + }), + }) + ); + }); + + it('calls the rulesClient when updating a system action groupingBy property from agent.name to agent.type', async () => { + const ruleUpdate = { + ...getCreateRulesSchemaMock(), + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], // changing this value + }, + }, + action_type_id: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + action_type_id: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }; + const existingRule = getRuleMock(getQueryRuleParams(), { + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], // changing this value + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ], + }); + (readRules as jest.Mock).mockResolvedValueOnce(existingRule); + rulesClient.update.mockResolvedValue( + getRuleMock(getQueryRuleParams(), { + actions: [ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + systemActions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ], + }) + ); + + await detectionRulesClient.updateRule({ ruleUpdate }); + + expect(rulesClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + actions: expect.arrayContaining([ + { + id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', + frequency: { + summary: true, + notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 + throttle: null, + }, + group: 'default', + }, + ]), + systemActions: expect.arrayContaining([ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + actionTypeId: '.cases', + uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', + }, + ]), + }), + }) + ); + }); + it('returns rule enabled: true if the nexParams have enabled: true', async () => { const ruleUpdate = { ...getCreateRulesSchemaMock(), enabled: true }; const existingRule = getRuleMock(getQueryRuleParams()); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index f9224bf9abeff..533b60d54fb0a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -11,12 +11,7 @@ import { isEmpty, partition } from 'lodash'; import { stringifyZodError } from '@kbn/zod-helpers'; import { BadRequestError } from '@kbn/securitysolution-es-utils'; import { ruleTypeMappings } from '@kbn/securitysolution-rules'; -import type { - ResolvedSanitizedRule, - RuleSystemAction, - SanitizedRule, - SanitizedRuleAction, -} from '@kbn/alerting-plugin/common'; +import type { ResolvedSanitizedRule, SanitizedRule } from '@kbn/alerting-plugin/common'; import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { RequiredOptional } from '@kbn/zod-helpers'; @@ -533,20 +528,15 @@ export const convertPatchAPIToInternalSchema = ( const typeSpecificParams = patchTypeSpecificSnakeToCamel(nextParams, existingRule.params); const existingParams = existingRule.params; - const [existingRuleSystemActions, existingRuleActions]: [ - RuleSystemAction[], - SanitizedRuleAction[] - ] = partition(existingRule.actions, (action) => actionsClient.isSystemAction(action.id)); - const [ruleUpdateSystemActions, ruleUpdateActions] = partition(nextParams.actions, (action) => actionsClient.isSystemAction(action.id) ); const systemActions = !isEmpty(ruleUpdateSystemActions) ? ruleUpdateSystemActions.map((action) => transformRuleToAlertAction(action)) - : existingRuleSystemActions; + : existingRule.systemActions; const alertActions = !isEmpty(ruleUpdateActions) ? ruleUpdateActions.map((action) => transformRuleToAlertAction(action)) - : existingRuleActions; + : existingRule.actions; const throttle = nextParams.throttle ?? transformFromAlertThrottle(existingRule); const actions = transformToActionFrequency(alertActions as RuleActionCamel[], throttle); @@ -593,7 +583,7 @@ export const convertPatchAPIToInternalSchema = ( }, schedule: { interval: nextParams.interval ?? existingRule.schedule.interval }, actions, - systemActions, + ...(systemActions && { systemActions }), }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts index bc4c18f2b7aee..9eac69dc888da 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts @@ -360,7 +360,7 @@ export interface InternalRuleUpdate { interval: string; }; actions: RuleActionArrayCamel; - systemActions: AlertingRuleSystemAction[]; + systemActions?: AlertingRuleSystemAction[]; params: RuleParams; throttle?: RuleActionThrottle | null; notifyWhen?: RuleActionNotifyWhen | null; From 683504b4024abf54e2c76e7df7ac6373801a9b19 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 17:56:23 -0400 Subject: [PATCH 33/54] make system actions optional on the internal create and update types --- .../lib/detection_engine/rule_schema/model/rule_schemas.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts index 9eac69dc888da..752a4c74d6c49 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_schema/model/rule_schemas.ts @@ -347,7 +347,7 @@ export interface InternalRuleCreate { }; enabled: IsRuleEnabled; actions: RuleActionArrayCamel; - systemActions: AlertingRuleSystemAction[]; + systemActions?: AlertingRuleSystemAction[]; params: RuleParams; throttle?: RuleActionThrottle | null; notifyWhen?: RuleActionNotifyWhen | null; From 57a4a77186dd9539d29e067bc6b1f2f98d670a96 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 18:38:29 -0400 Subject: [PATCH 34/54] adds wrappers around partition and array.map -> transformRuleToAlertAction --- .../detection_engine/transform_actions.ts | 10 ++++ .../normalization/rule_converters.ts | 51 +++++++++---------- .../rule_management/utils/utils.ts | 9 ++++ 3 files changed, 42 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index 8dbc500eae930..e728ef5993088 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -5,6 +5,8 @@ * 2.0. */ +import { isEmpty } from 'lodash'; + import type { RuleAction as AlertingRuleAction, RuleSystemAction as AlertingRuleSystemAction, @@ -18,6 +20,14 @@ import type { import { ResponseActionTypesEnum } from '../api/detection_engine/model/rule_response_actions'; import type { RuleAction } from '../api/detection_engine/model'; +export const transformRuleActionsToAlertActions = ( + newActions: RuleAction[] | undefined, + existingActions: AlertingRuleAction[] | AlertingRuleSystemAction[] | undefined +): AlertingRuleAction[] | AlertingRuleSystemAction[] => + !isEmpty(newActions) && newActions != null + ? newActions.map((action) => transformRuleToAlertAction(action)) + : existingActions ?? []; + export const transformRuleToAlertAction = ({ group, id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts index 533b60d54fb0a..350115b2c2c28 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/normalization/rule_converters.ts @@ -6,7 +6,6 @@ */ import { v4 as uuidv4 } from 'uuid'; -import { isEmpty, partition } from 'lodash'; import { stringifyZodError } from '@kbn/zod-helpers'; import { BadRequestError } from '@kbn/securitysolution-es-utils'; @@ -45,6 +44,7 @@ import { transformAlertToRuleAction, transformAlertToRuleResponseAction, transformAlertToRuleSystemAction, + transformRuleActionsToAlertActions, transformRuleToAlertAction, transformRuleToAlertResponseAction, } from '../../../../../common/detection_engine/transform_actions'; @@ -86,6 +86,7 @@ import { convertAlertSuppressionToCamel, convertAlertSuppressionToSnake, migrateLegacyInvestigationFields, + separateActionsAndSystemAction, } from '../utils/utils'; import { createRuleExecutionSummary } from '../../rule_monitoring'; import type { PrebuiltRuleAsset } from '../../prebuilt_rules'; @@ -449,23 +450,15 @@ export const convertUpdateAPIToInternalSchema = ({ ruleUpdate, actionsClient, }: ConvertUpdateAPIToInternalSchemaProps) => { - const [ruleUpdateSystemActions, ruleUpdateActions] = partition(ruleUpdate.actions, (action) => - actionsClient.isSystemAction(action.id) + const [ruleUpdateSystemActions, ruleUpdateActions] = separateActionsAndSystemAction( + actionsClient, + ruleUpdate.actions ); - - const [existingRuleUpdateSystemActions, existingRuleUpdateActions] = partition( - existingRule.actions, - (action) => actionsClient.isSystemAction(action.id) + const systemActions = transformRuleActionsToAlertActions( + ruleUpdateSystemActions, + existingRule.systemActions ); - - const systemActions = - (ruleUpdateSystemActions ?? existingRuleUpdateSystemActions).map((action) => - transformRuleToAlertAction(action) - ) ?? []; - const alertActions = - (ruleUpdateActions ?? existingRuleUpdateActions).map((action) => - transformRuleToAlertAction(action) - ) ?? []; + const alertActions = transformRuleActionsToAlertActions(ruleUpdateActions, existingRule.actions); const actions = transformToActionFrequency( alertActions as RuleActionCamel[], ruleUpdate.throttle @@ -513,7 +506,7 @@ export const convertUpdateAPIToInternalSchema = ({ }, schedule: { interval: ruleUpdate.interval ?? '5m' }, actions, - systemActions, + ...(systemActions && { systemActions }), }; return newInternalRule; @@ -528,15 +521,16 @@ export const convertPatchAPIToInternalSchema = ( const typeSpecificParams = patchTypeSpecificSnakeToCamel(nextParams, existingRule.params); const existingParams = existingRule.params; - const [ruleUpdateSystemActions, ruleUpdateActions] = partition(nextParams.actions, (action) => - actionsClient.isSystemAction(action.id) + const [ruleUpdateSystemActions, ruleUpdateActions] = separateActionsAndSystemAction( + actionsClient, + nextParams.actions ); - const systemActions = !isEmpty(ruleUpdateSystemActions) - ? ruleUpdateSystemActions.map((action) => transformRuleToAlertAction(action)) - : existingRule.systemActions; - const alertActions = !isEmpty(ruleUpdateActions) - ? ruleUpdateActions.map((action) => transformRuleToAlertAction(action)) - : existingRule.actions; + const systemActions = transformRuleActionsToAlertActions( + ruleUpdateSystemActions, + existingRule.systemActions + ); + + const alertActions = transformRuleActionsToAlertActions(ruleUpdateActions, existingRule.actions); const throttle = nextParams.throttle ?? transformFromAlertThrottle(existingRule); const actions = transformToActionFrequency(alertActions as RuleActionCamel[], throttle); @@ -600,8 +594,9 @@ export const convertCreateAPIToInternalSchema = ( ): InternalRuleCreate => { const { immutable = false, defaultEnabled = true } = options ?? {}; - const [systemActions, externalActions] = partition(input.actions, (action) => - actionsClient.isSystemAction(action.id) + const [systemActions, externalActions] = separateActionsAndSystemAction( + actionsClient, + input.actions ); const typeSpecificParams = typeSpecificSnakeToCamel(input); @@ -655,7 +650,7 @@ export const convertCreateAPIToInternalSchema = ( schedule: { interval: input.interval ?? '5m' }, enabled: input.enabled ?? defaultEnabled, actions, - systemActions: alertSystemActions, + ...(alertSystemActions && { systemActions: alertSystemActions }), }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts index 47683aab41090..a6b4361e4943c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/utils/utils.ts @@ -23,6 +23,7 @@ import type { RequiredField, RequiredFieldInput, RuleResponse, + RuleAction as RuleActionSchema, } from '../../../../../common/api/detection_engine/model/rule_schema'; import type { FindRulesResponse, @@ -413,3 +414,11 @@ export const addEcsToRequiredFields = (requiredFields?: RequiredFieldInput[]): R ecs: isEcsField, }; }); + +export const separateActionsAndSystemAction = ( + actionsClient: ActionsClient, + actions: RuleActionSchema[] | undefined +) => + !isEmpty(actions) + ? partition((action: RuleActionSchema) => actionsClient.isSystemAction(action.id))(actions) + : [[], actions]; From e36a295d49d789af7751f2de02e27a7162d169f3 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 3 Jul 2024 19:51:21 -0400 Subject: [PATCH 35/54] adds FTR test which creates a rule and case system action and checks if the rule system action creates a case when it discovers alerts --- .../common/lib/api/case.ts | 44 ++++++++++++++++++- .../config/ess/config.base.ts | 1 + .../execution_logic/custom_query.ts | 39 ++++++++++++++++ 3 files changed, 83 insertions(+), 1 deletion(-) diff --git a/x-pack/test/cases_api_integration/common/lib/api/case.ts b/x-pack/test/cases_api_integration/common/lib/api/case.ts index dd1b668b429cf..c3810221759d2 100644 --- a/x-pack/test/cases_api_integration/common/lib/api/case.ts +++ b/x-pack/test/cases_api_integration/common/lib/api/case.ts @@ -7,12 +7,14 @@ import { CASES_URL } from '@kbn/cases-plugin/common'; import { Case } from '@kbn/cases-plugin/common/types/domain'; -import { CasePostRequest } from '@kbn/cases-plugin/common/types/api'; +import { CasePostRequest, CasesFindResponse } from '@kbn/cases-plugin/common/types/api'; import type SuperTest from 'supertest'; +import { ToolingLog } from '@kbn/tooling-log'; import { User } from '../authentication/types'; import { superUser } from '../authentication/users'; import { getSpaceUrlPrefix, setupAuth } from './helpers'; +import { waitFor } from '../../../../common/utils/security_solution/detections_response'; export const createCase = async ( supertest: SuperTest.Agent, @@ -35,6 +37,46 @@ export const createCase = async ( return theCase; }; +export const waitForCases = async ({ + supertest, + log, +}: { + supertest: SuperTest.Agent; + log: ToolingLog; +}): Promise => { + await waitFor( + async () => { + const response = await getCases(supertest); + + // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe + const cases = response; + + return cases != null && cases.cases.length > 0 && cases?.cases[0]?.totalAlerts > 0; + }, + 'waitForCaseToAttachAlert', + log + ); +}; + +export const getCases = async ( + supertest: SuperTest.Agent, + expectedHttpCode: number = 200, + auth: { user: User; space: string | null } | null = { user: superUser, space: null }, + headers: Record = {} +): Promise => { + const apiCall = supertest.get(`${getSpaceUrlPrefix(auth?.space)}${CASES_URL}/_find`); + + setupAuth({ apiCall, headers, auth }); + + const { body: theCase } = await apiCall + .set('kbn-xsrf', 'true') + .set('x-elastic-internal-origin', 'foo') + .set(headers) + .expect(expectedHttpCode); + + return theCase; +}; + /** * Sends a delete request for the specified case IDs. */ diff --git a/x-pack/test/security_solution_api_integration/config/ess/config.base.ts b/x-pack/test/security_solution_api_integration/config/ess/config.base.ts index a47e43bd426e6..20893496c8950 100644 --- a/x-pack/test/security_solution_api_integration/config/ess/config.base.ts +++ b/x-pack/test/security_solution_api_integration/config/ess/config.base.ts @@ -17,6 +17,7 @@ interface CreateTestConfigOptions { // test.not-enabled is specifically not enabled const enabledActionTypes = [ + '.cases', '.email', '.index', '.pagerduty', diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts index 692f986e6e6a6..d6a3a2ccce4a6 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts @@ -46,6 +46,10 @@ import { ENABLE_ASSET_CRITICALITY_SETTING, } from '@kbn/security-solution-plugin/common/constants'; import { getMaxSignalsWarning as getMaxAlertsWarning } from '@kbn/security-solution-plugin/server/lib/detection_engine/rule_types/utils/utils'; +import { + getCases, + waitForCases, +} from '../../../../../../../cases_api_integration/common/lib/api/case'; import { deleteAllExceptions } from '../../../../../lists_and_exception_lists/utils'; import { createExceptionList, @@ -104,11 +108,17 @@ export default ({ getService }: FtrProviderContext) => { useCreate: true, docsOnly: true, }); + await deleteAllRules(supertest, log); + await esArchiver.load('x-pack/test/functional/es_archives/signals/severity_risk_overrides'); }); afterEach(async () => { await esDeleteAllIndices('.preview.alerts*'); + await esArchiver.unload(auditbeatPath); + await esArchiver.unload('x-pack/test/functional/es_archives/signals/severity_risk_overrides'); + await deleteAllAlerts(supertest, log, es, ['.preview.alerts-security.alerts-*']); + await deleteAllRules(supertest, log); }); after(async () => { @@ -130,6 +140,35 @@ export default ({ getService }: FtrProviderContext) => { expect(alerts.hits.hits[0]._source?.['kibana.alert.ancestors'][0].id).toEqual(ID); }); + // creates a real rule to determine if a case was opened by the system action + it('should create a case if a rule with the cases system action finds matching alerts', async () => { + const rule: QueryRuleCreateProps = { + ...getRuleForAlertTesting(['auditbeat-*']), + query: `_id:${ID}`, + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + }, + ], + }; + const createdRule = await createRule(supertest, log, rule); + const alerts = await getAlerts(supertest, log, es, createdRule); + await waitForCases({ supertest, log }); + const cases = await getCases(supertest); + expect(cases.cases[0].totalAlerts).toBeGreaterThan(0); + expect(alerts.hits.hits.length).toBeGreaterThan(0); + expect(alerts.hits.hits[0]._source?.['kibana.alert.ancestors'][0].id).toEqual(ID); + }); + it('generates max alerts warning when circuit breaker is hit', async () => { const rule: QueryRuleCreateProps = { ...getRuleForAlertTesting(['auditbeat-*']), From ed6f8c72a5fad244f08d8aa0a08d4321e0ad2414 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 5 Jul 2024 09:42:10 -0400 Subject: [PATCH 36/54] forgot to add the actual test case --- .../add_actions.ts | 28 +++++++++++++++++++ .../execution_logic/custom_query.ts | 12 -------- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts index 335fad4c7acee..f0cf243f0d81c 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts @@ -39,6 +39,34 @@ export default ({ getService }: FtrProviderContext) => { await deleteAllRules(supertest, log); }); + it('creates rule with a new cases system action', async () => { + const ruleAction = { + id: '.system-action-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + }; + + const { body } = await supertest + .post(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .set(ELASTIC_HTTP_VERSION_HEADER, '2023-10-31') + .send( + getCustomQueryRuleParams({ + actions: [ruleAction], + }) + ) + .expect(200); + + expect(body.actions).toEqual([expect.objectContaining(ruleAction)]); + }); + it('creates rule with a new webhook action', async () => { const webhookAction = await createWebHookRuleAction(supertest); const ruleAction = { diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts index d6a3a2ccce4a6..3e7f7cf6bf8d6 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts @@ -128,18 +128,6 @@ export default ({ getService }: FtrProviderContext) => { await deleteAllRules(supertest, log); }); - // First test creates a real rule - most remaining tests use preview API - it('should have the specific audit record for _id or none of these tests below will pass', async () => { - const rule: QueryRuleCreateProps = { - ...getRuleForAlertTesting(['auditbeat-*']), - query: `_id:${ID}`, - }; - const createdRule = await createRule(supertest, log, rule); - const alerts = await getAlerts(supertest, log, es, createdRule); - expect(alerts.hits.hits.length).toBeGreaterThan(0); - expect(alerts.hits.hits[0]._source?.['kibana.alert.ancestors'][0].id).toEqual(ID); - }); - // creates a real rule to determine if a case was opened by the system action it('should create a case if a rule with the cases system action finds matching alerts', async () => { const rule: QueryRuleCreateProps = { From 69ba82ba58d929e34a33fea8bd7f6a953e31a825 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 5 Jul 2024 15:37:18 -0400 Subject: [PATCH 37/54] fixes rule edit bulk actions -> set and add actions --- .../bulk_actions/bulk_actions_route.gen.ts | 2 +- .../bulk_actions_route.schema.yaml | 1 - .../api/rules/bulk_actions/route.ts | 1 + .../action_to_rules_client_operation.ts | 27 ++++++++++++++----- .../logic/bulk_actions/bulk_edit_rules.ts | 7 ++++- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.gen.ts b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.gen.ts index b9750fd7eb06d..8f2bcfa0e0c9c 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.gen.ts +++ b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.gen.ts @@ -219,7 +219,7 @@ export const BulkActionEditTypeEnum = BulkActionEditType.enum; export type NormalizedRuleAction = z.infer; export const NormalizedRuleAction = z .object({ - group: RuleActionGroup, + group: RuleActionGroup.optional(), id: RuleActionId, params: RuleActionParams, frequency: RuleActionFrequency.optional(), diff --git a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.schema.yaml b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.schema.yaml index 184f4ec9825b6..ee9062d41e9ed 100644 --- a/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.schema.yaml +++ b/x-pack/plugins/security_solution/common/api/detection_engine/rule_management/bulk_actions/bulk_actions_route.schema.yaml @@ -329,7 +329,6 @@ components: alerts_filter: $ref: '../../model/rule_schema/common_attributes.schema.yaml#/components/schemas/RuleActionAlertsFilter' required: - - group - id - params additionalProperties: false 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 b2c6ae6e04468..3d2df63919ce7 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 @@ -141,6 +141,7 @@ export const performBulkActionRoute = ( // rulesClient method, hence there is no need to use fetchRulesByQueryOrIds utility if (body.action === BulkActionTypeEnum.edit && !isDryRun) { const { rules, errors, skipped } = await bulkEditRules({ + actionsClient, rulesClient, filter: query, ids: body.ids, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts index eac694f97944b..343b91d2efdcd 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts @@ -6,12 +6,14 @@ */ import type { BulkEditOperation } from '@kbn/alerting-plugin/server'; -import { transformNormalizedRuleToAlertAction } from '../../../../../../common/detection_engine/transform_actions'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import { transformNormalizedRuleToAlertAction } from '../../../../../../common/detection_engine/transform_actions'; import type { BulkActionEditForRuleAttributes } from '../../../../../../common/api/detection_engine/rule_management'; import { BulkActionEditTypeEnum } from '../../../../../../common/api/detection_engine/rule_management'; import { assertUnreachable } from '../../../../../../common/utility_types'; import { transformToActionFrequency } from '../../normalization/rule_actions'; +import { separateActionsAndSystemAction } from '../../utils/utils'; /** * converts bulk edit action to format of rulesClient.bulkEdit operation @@ -19,8 +21,13 @@ import { transformToActionFrequency } from '../../normalization/rule_actions'; * @returns rulesClient BulkEditOperation */ export const bulkEditActionToRulesClientOperation = ( + actionsClient: ActionsClient, action: BulkActionEditForRuleAttributes ): BulkEditOperation[] => { + const [systemActions, actions] = separateActionsAndSystemAction( + actionsClient, + action.value.actions + ); switch (action.type) { // tags actions case BulkActionEditTypeEnum.add_tags: @@ -56,9 +63,12 @@ export const bulkEditActionToRulesClientOperation = ( { field: 'actions', operation: 'add', - value: transformToActionFrequency(action.value.actions, action.value.throttle).map( - transformNormalizedRuleToAlertAction - ), + value: [ + ...(systemActions ?? []), + ...transformToActionFrequency(actions ?? [], action.value.throttle).map( + transformNormalizedRuleToAlertAction + ), + ], }, ]; @@ -67,9 +77,12 @@ export const bulkEditActionToRulesClientOperation = ( { field: 'actions', operation: 'set', - value: transformToActionFrequency(action.value.actions, action.value.throttle).map( - transformNormalizedRuleToAlertAction - ), + value: [ + ...(systemActions ?? []), + ...transformToActionFrequency(actions ?? [], action.value.throttle).map( + transformNormalizedRuleToAlertAction + ), + ], }, ]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts index 3c7e0e7af16ee..ab3a9f486d81b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/bulk_edit_rules.ts @@ -6,6 +6,7 @@ */ import type { RulesClient } from '@kbn/alerting-plugin/server'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; import type { ExperimentalFeatures } from '../../../../../../common'; import type { BulkActionEditPayload } from '../../../../../../common/api/detection_engine/rule_management'; @@ -21,6 +22,7 @@ import { validateBulkEditRule } from './validations'; import { bulkEditActionToRulesClientOperation } from './action_to_rules_client_operation'; export interface BulkEditRulesArguments { + actionsClient: ActionsClient; rulesClient: RulesClient; actions: BulkActionEditPayload[]; filter?: string; @@ -37,6 +39,7 @@ export interface BulkEditRulesArguments { * @returns edited rules and caught errors */ export const bulkEditRules = async ({ + actionsClient, rulesClient, ids, actions, @@ -45,7 +48,9 @@ export const bulkEditRules = async ({ experimentalFeatures, }: BulkEditRulesArguments) => { const { attributesActions, paramsActions } = splitBulkEditActions(actions); - const operations = attributesActions.map(bulkEditActionToRulesClientOperation).flat(); + const operations = attributesActions + .map((attribute) => bulkEditActionToRulesClientOperation(actionsClient, attribute)) + .flat(); const result = await rulesClient.bulkEdit({ ...(ids ? { ids } : { filter: enrichFilterWithRuleTypeMapping(filter) }), operations, From 475915e8a7879e2e0a0879f931a50bfd5675025a Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 5 Jul 2024 17:05:54 -0400 Subject: [PATCH 38/54] fixes tests and type checker --- .../detection_engine/transform_actions.ts | 9 +++-- .../action_to_rules_client_operation.test.ts | 15 +++++--- .../action_to_rules_client_operation.ts | 30 ++++++---------- .../logic/bulk_actions/utils.ts | 35 ++++++++++++++++++- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index e728ef5993088..63b2bd06a9d8a 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -11,7 +11,10 @@ import type { RuleAction as AlertingRuleAction, RuleSystemAction as AlertingRuleSystemAction, } from '@kbn/alerting-plugin/common'; -import type { NormalizedAlertAction } from '@kbn/alerting-plugin/server/rules_client'; +import type { + NormalizedAlertAction, + NormalizedSystemAction, +} from '@kbn/alerting-plugin/server/rules_client'; import type { NormalizedRuleAction } from '../api/detection_engine/rule_management'; import type { ResponseAction, @@ -84,8 +87,7 @@ export const transformNormalizedRuleToAlertAction = ({ params, frequency, alerts_filter: alertsFilter, -}: NormalizedRuleAction): NormalizedAlertAction => ({ - group, +}: NormalizedRuleAction): NormalizedAlertAction | NormalizedSystemAction => ({ id, params: params as AlertingRuleAction['params'], ...(alertsFilter && { @@ -95,6 +97,7 @@ export const transformNormalizedRuleToAlertAction = ({ alertsFilter: alertsFilter as AlertingRuleAction['alertsFilter'], }), ...(frequency && { frequency }), + ...(group && { group }), }); export const transformAlertToNormalizedRuleAction = ({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts index e214b7dc3b341..d9f05d1049d94 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts @@ -5,13 +5,17 @@ * 2.0. */ +import type { ActionsClient } from '@kbn/actions-plugin/server'; import { BulkActionEditTypeEnum } from '../../../../../../common/api/detection_engine/rule_management'; import { bulkEditActionToRulesClientOperation } from './action_to_rules_client_operation'; describe('bulkEditActionToRulesClientOperation', () => { + const actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; test('should transform tags bulk edit actions correctly', () => { expect( - bulkEditActionToRulesClientOperation({ + bulkEditActionToRulesClientOperation(actionsClient, { type: BulkActionEditTypeEnum.add_tags, value: ['test'], }) @@ -25,7 +29,10 @@ describe('bulkEditActionToRulesClientOperation', () => { }); expect( - bulkEditActionToRulesClientOperation({ type: BulkActionEditTypeEnum.set_tags, value: ['test'] }) + bulkEditActionToRulesClientOperation(actionsClient, { + type: BulkActionEditTypeEnum.set_tags, + value: ['test'], + }) ).toEqual([ { field: 'tags', @@ -35,7 +42,7 @@ describe('bulkEditActionToRulesClientOperation', () => { ]); expect( - bulkEditActionToRulesClientOperation({ + bulkEditActionToRulesClientOperation(actionsClient, { type: BulkActionEditTypeEnum.delete_tags, value: ['test'], }) @@ -49,7 +56,7 @@ describe('bulkEditActionToRulesClientOperation', () => { test('should transform schedule bulk edit correctly', () => { expect( - bulkEditActionToRulesClientOperation({ + bulkEditActionToRulesClientOperation(actionsClient, { type: BulkActionEditTypeEnum.set_schedule, value: { interval: '100m', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts index 343b91d2efdcd..6c62716763042 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.ts @@ -8,12 +8,10 @@ import type { BulkEditOperation } from '@kbn/alerting-plugin/server'; import type { ActionsClient } from '@kbn/actions-plugin/server'; -import { transformNormalizedRuleToAlertAction } from '../../../../../../common/detection_engine/transform_actions'; import type { BulkActionEditForRuleAttributes } from '../../../../../../common/api/detection_engine/rule_management'; import { BulkActionEditTypeEnum } from '../../../../../../common/api/detection_engine/rule_management'; import { assertUnreachable } from '../../../../../../common/utility_types'; -import { transformToActionFrequency } from '../../normalization/rule_actions'; -import { separateActionsAndSystemAction } from '../../utils/utils'; +import { parseAndTransformRuleActions } from './utils'; /** * converts bulk edit action to format of rulesClient.bulkEdit operation @@ -24,10 +22,6 @@ export const bulkEditActionToRulesClientOperation = ( actionsClient: ActionsClient, action: BulkActionEditForRuleAttributes ): BulkEditOperation[] => { - const [systemActions, actions] = separateActionsAndSystemAction( - actionsClient, - action.value.actions - ); switch (action.type) { // tags actions case BulkActionEditTypeEnum.add_tags: @@ -63,12 +57,11 @@ export const bulkEditActionToRulesClientOperation = ( { field: 'actions', operation: 'add', - value: [ - ...(systemActions ?? []), - ...transformToActionFrequency(actions ?? [], action.value.throttle).map( - transformNormalizedRuleToAlertAction - ), - ], + value: parseAndTransformRuleActions( + actionsClient, + action.value.actions, + action.value.throttle + ), }, ]; @@ -77,12 +70,11 @@ export const bulkEditActionToRulesClientOperation = ( { field: 'actions', operation: 'set', - value: [ - ...(systemActions ?? []), - ...transformToActionFrequency(actions ?? [], action.value.throttle).map( - transformNormalizedRuleToAlertAction - ), - ], + value: parseAndTransformRuleActions( + actionsClient, + action.value.actions, + action.value.throttle + ), }, ]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/utils.ts index 08b3487ede61f..cffe6a349a783 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/utils.ts @@ -4,8 +4,17 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import type { BulkActionEditType } from '../../../../../../common/api/detection_engine/rule_management'; + +import { isEmpty, partition } from 'lodash'; +import type { ActionsClient } from '@kbn/actions-plugin/server'; +import type { + BulkActionEditType, + NormalizedRuleAction, + ThrottleForBulkActions, +} from '../../../../../../common/api/detection_engine/rule_management'; import { BulkActionEditTypeEnum } from '../../../../../../common/api/detection_engine/rule_management'; +import { transformToActionFrequency } from '../../normalization/rule_actions'; +import { transformNormalizedRuleToAlertAction } from '../../../../../../common/detection_engine/transform_actions'; /** * helper utility that defines whether bulk edit action is related to index patterns, i.e. one of: @@ -36,3 +45,27 @@ export const isInvestigationFieldsBulkEditAction = (editAction: BulkActionEditTy ]; return investigationFieldsActions.includes(editAction); }; + +/** + * Separates system actions from actions and performs necessary transformations for + * alerting rules client bulk edit operations. + * @param actionsClient + * @param actions + * @param throttle + * @returns + */ +export const parseAndTransformRuleActions = ( + actionsClient: ActionsClient, + actions: NormalizedRuleAction[], + throttle: ThrottleForBulkActions | undefined +) => { + const [systemActions, extActions] = !isEmpty(actions) + ? partition(actions, (action: NormalizedRuleAction) => actionsClient.isSystemAction(action.id)) + : [[], actions]; + return [ + ...(systemActions ?? []), + ...transformToActionFrequency(extActions ?? [], throttle).map( + transformNormalizedRuleToAlertAction + ), + ]; +}; From bf2d39b1f220e33bd29249e88ec6bcb3e9485474 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 9 Jul 2024 08:20:32 -0400 Subject: [PATCH 39/54] undo ftr test --- .../add_actions.ts | 28 ------------------- 1 file changed, 28 deletions(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts index f0cf243f0d81c..335fad4c7acee 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts @@ -39,34 +39,6 @@ export default ({ getService }: FtrProviderContext) => { await deleteAllRules(supertest, log); }); - it('creates rule with a new cases system action', async () => { - const ruleAction = { - id: '.system-action-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], - }, - }, - action_type_id: '.cases', - }; - - const { body } = await supertest - .post(DETECTION_ENGINE_RULES_URL) - .set('kbn-xsrf', 'true') - .set(ELASTIC_HTTP_VERSION_HEADER, '2023-10-31') - .send( - getCustomQueryRuleParams({ - actions: [ruleAction], - }) - ) - .expect(200); - - expect(body.actions).toEqual([expect.objectContaining(ruleAction)]); - }); - it('creates rule with a new webhook action', async () => { const webhookAction = await createWebHookRuleAction(supertest); const ruleAction = { From 5827e1ec9bf5e772507e87754ca4a674168c751b Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 16 Jul 2024 15:23:03 -0400 Subject: [PATCH 40/54] change boolean name and import, move ftr test to actions --- .../step_rule_actions/notification_action.tsx | 10 +-- .../add_actions.ts | 67 ++++++++++++++++++- .../execution_logic/custom_query.ts | 25 +------ 3 files changed, 72 insertions(+), 30 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index 92aa49b34ccd1..0010c75fd1441 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -18,7 +18,7 @@ import type { ActionTypeRegistryContract } from '@kbn/triggers-actions-ui-plugin import { FormattedMessage } from '@kbn/i18n-react'; import { getTimeTypeValue, - isRuleAction, + isRuleAction as getIsRuleAction, } from '../../../rule_creation_ui/pages/rule_creation/helpers'; import * as i18n from './translations'; @@ -98,17 +98,17 @@ export function NotificationAction({ connectors, actionTypeRegistry, }: NotificationActionProps) { - const _isRuleAction = isRuleAction(action, actionTypeRegistry); + const isRuleAction = getIsRuleAction(action, actionTypeRegistry); const connectorType = connectorTypes.find(({ id }) => id === action.actionTypeId); const registeredAction = actionTypeRegistry.get(action.actionTypeId); - const connectorTypeName = _isRuleAction + const connectorTypeName = isRuleAction ? connectorType?.name ?? '' : registeredAction.actionTypeTitle ?? ''; const iconType = registeredAction?.iconClass ?? 'apps'; const connector = connectors.find(({ id }) => id === action.id); - const connectorName = _isRuleAction + const connectorName = isRuleAction ? connector?.name ?? '' : registeredAction.actionTypeTitle ?? ''; @@ -127,7 +127,7 @@ export function NotificationAction({ - {_isRuleAction && } + {isRuleAction && } diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts index 335fad4c7acee..9cd7a3c6b2691 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts @@ -8,21 +8,56 @@ import expect from 'expect'; import { ELASTIC_HTTP_VERSION_HEADER } from '@kbn/core-http-common'; import { DETECTION_ENGINE_RULES_URL } from '@kbn/security-solution-plugin/common/constants'; +import { QueryRuleCreateProps } from '@kbn/security-solution-plugin/common/api/detection_engine'; +import { getCases, waitForCases } from '../../../../../../cases_api_integration/common/lib/api'; import { deleteAllRules, waitForRuleSuccess, deleteAllAlerts, + getRuleForAlertTesting, + createRule, } from '../../../../../../common/utils/security_solution'; import { FtrProviderContext } from '../../../../../ftr_provider_context'; -import { createWebHookRuleAction, fetchRule, getCustomQueryRuleParams } from '../../../utils'; +import { + createWebHookRuleAction, + fetchRule, + getAlerts, + getCustomQueryRuleParams, +} from '../../../utils'; +import { EsArchivePathBuilder } from '../../../../../es_archive_path_builder'; + +/** + * Specific _id to use for some of the tests. If the archiver changes and you see errors + * here, update this to a new value of a chosen auditbeat record and update the tests values. + */ +const ID = 'BhbXBmkBR346wHgn4PeZ'; export default ({ getService }: FtrProviderContext) => { const supertest = getService('supertest'); - const log = getService('log'); + const esArchiver = getService('esArchiver'); const es = getService('es'); + const log = getService('log'); + // TODO: add a new service for loading archiver files similar to "getService('es')" + const config = getService('config'); + const isServerless = config.get('serverless'); + const dataPathBuilder = new EsArchivePathBuilder(isServerless); + const auditbeatPath = dataPathBuilder.getPath('auditbeat/hosts'); describe('@serverless @serverlessQA @ess add_actions', () => { describe('adding actions', () => { + before(async () => { + await esArchiver.load(auditbeatPath); + await esArchiver.load('x-pack/test/functional/es_archives/security_solution/alerts/8.8.0', { + useCreate: true, + docsOnly: true, + }); + }); + after(async () => { + await esArchiver.unload(auditbeatPath); + await esArchiver.unload( + 'x-pack/test/functional/es_archives/signals/severity_risk_overrides' + ); + }); beforeEach(async () => { await es.indices.delete({ index: 'logs-test', ignore_unavailable: true }); await es.indices.create({ @@ -39,6 +74,34 @@ export default ({ getService }: FtrProviderContext) => { await deleteAllRules(supertest, log); }); + it('should create a case if a rule with the cases system action finds matching alerts', async () => { + const rule: QueryRuleCreateProps = { + ...getRuleForAlertTesting(['auditbeat-*']), + query: `_id:${ID}`, + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + action_type_id: '.cases', + }, + ], + }; + const createdRule = await createRule(supertest, log, rule); + const alerts = await getAlerts(supertest, log, es, createdRule); + await waitForCases({ supertest, log }); + const cases = await getCases(supertest); + expect(cases.cases[0].totalAlerts).toBeGreaterThan(0); + expect(alerts.hits.hits.length).toBeGreaterThan(0); + expect(alerts.hits.hits[0]._source?.['kibana.alert.ancestors'][0].id).toEqual(ID); + }); + it('creates rule with a new webhook action', async () => { const webhookAction = await createWebHookRuleAction(supertest); const ruleAction = { diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts index 3e7f7cf6bf8d6..c4ee12345af23 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts @@ -46,10 +46,6 @@ import { ENABLE_ASSET_CRITICALITY_SETTING, } from '@kbn/security-solution-plugin/common/constants'; import { getMaxSignalsWarning as getMaxAlertsWarning } from '@kbn/security-solution-plugin/server/lib/detection_engine/rule_types/utils/utils'; -import { - getCases, - waitForCases, -} from '../../../../../../../cases_api_integration/common/lib/api/case'; import { deleteAllExceptions } from '../../../../../lists_and_exception_lists/utils'; import { createExceptionList, @@ -128,31 +124,14 @@ export default ({ getService }: FtrProviderContext) => { await deleteAllRules(supertest, log); }); - // creates a real rule to determine if a case was opened by the system action - it('should create a case if a rule with the cases system action finds matching alerts', async () => { + // First test creates a real rule - most remaining tests use preview API + it('should have the specific audit record for _id or none of these tests below will pass', async () => { const rule: QueryRuleCreateProps = { ...getRuleForAlertTesting(['auditbeat-*']), query: `_id:${ID}`, - actions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], - }, - }, - action_type_id: '.cases', - }, - ], }; const createdRule = await createRule(supertest, log, rule); const alerts = await getAlerts(supertest, log, es, createdRule); - await waitForCases({ supertest, log }); - const cases = await getCases(supertest); - expect(cases.cases[0].totalAlerts).toBeGreaterThan(0); expect(alerts.hits.hits.length).toBeGreaterThan(0); expect(alerts.hits.hits[0]._source?.['kibana.alert.ancestors'][0].id).toEqual(ID); }); From cc33a016079450fde28b3a1c055a857b5647bf77 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Tue, 16 Jul 2024 16:26:30 -0400 Subject: [PATCH 41/54] adds condition in cypress test to ensure complete tier cases action is not displayed in essentials tier --- .../rule_actions/rule_actions_pli_essentials.cy.ts | 2 ++ .../cypress/screens/common/rule_actions.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_essentials.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_essentials.cy.ts index 8e53f3c96657a..6b3e19125c6c1 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_essentials.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_essentials.cy.ts @@ -15,6 +15,7 @@ import { XMATTERS_ACTION_BTN, SERVER_LOG_ACTION_BTN, ACTION_BTN, + CASES_SYSTEM_ACTION_BTN, } from '../../../../screens/common/rule_actions'; import { createRule } from '../../../../tasks/api_calls/rules'; @@ -68,6 +69,7 @@ describe( cy.get(WEBHOOK_ACTION_BTN).should('not.exist'); cy.get(XMATTERS_ACTION_BTN).should('not.exist'); cy.get(SERVER_LOG_ACTION_BTN).should('not.exist'); + cy.get(CASES_SYSTEM_ACTION_BTN).should('not.exist'); }); } ); diff --git a/x-pack/test/security_solution_cypress/cypress/screens/common/rule_actions.ts b/x-pack/test/security_solution_cypress/cypress/screens/common/rule_actions.ts index a5c35406fb1ed..32caf47e43034 100644 --- a/x-pack/test/security_solution_cypress/cypress/screens/common/rule_actions.ts +++ b/x-pack/test/security_solution_cypress/cypress/screens/common/rule_actions.ts @@ -13,6 +13,8 @@ export const SERVER_LOG_ACTION_BTN = '[data-test-subj=".server-log-siem-ActionTy export const XMATTERS_ACTION_BTN = '[data-test-subj=".xmatters-siem-ActionTypeSelectOption"]'; +export const CASES_SYSTEM_ACTION_BTN = '[data-test-subj=".cases-siem-ActionTypeSelectOption"]'; + /** * all rule actions buttons, elements which data-test-subj attribute ends with '-siem-ActionTypeSelectOption' */ From dbf4db28824890278265423bd87505950ef21276 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 17 Jul 2024 12:10:34 -0400 Subject: [PATCH 42/54] fix unit tests --- .../detection_rules_client.patch_rule.test.ts | 250 +----------------- ...detection_rules_client.update_rule.test.ts | 2 +- ...rules_client.upgrade_prebuilt_rule.test.ts | 7 +- 3 files changed, 8 insertions(+), 251 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts index 616e4f94cb9f7..cf03962c3a2f8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.patch_rule.test.ts @@ -115,255 +115,7 @@ describe('DetectionRulesClient.patchRule', () => { }, ], }; - const existingRule = getRuleMock(getQueryRuleParams()); - (getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule); - rulesClient.update.mockResolvedValue( - getRuleMock(getQueryRuleParams(), { - actions: [ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, - group: 'default', - }, - ], - systemActions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.type'], - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ], - }) - ); - - await detectionRulesClient.patchRule({ rulePatch }); - - expect(rulesClient.update).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - actions: expect.arrayContaining([ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { - summary: true, - notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 - throttle: null, - }, - group: 'default', - }, - ]), - systemActions: expect.arrayContaining([ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.type'], - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ]), - }), - }) - ); - }); - - it('calls rule update with rule system actions if nextParams has no system actions and existing rule does have system actions', async () => { - const rulePatch = { - ...getCreateRulesSchemaMock(), - enabled: true, - }; - const existingRule = getRuleMock(getQueryRuleParams(), { - actions: [ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, - group: 'default', - }, - ], - systemActions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ], - }); - (getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule); - rulesClient.update.mockResolvedValue( - getRuleMock(getQueryRuleParams(), { - actions: [ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, - group: 'default', - }, - ], - systemActions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ], - }) - ); - - await detectionRulesClient.patchRule({ rulePatch }); - - expect(rulesClient.update).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - actions: expect.arrayContaining([ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { - summary: true, - notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 - throttle: null, - }, - group: 'default', - }, - ]), - systemActions: expect.arrayContaining([ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ]), - }), - }) - ); - }); - - it('patches system actions if nextParams has system actions', async () => { - const rulePatch = { - ...getCreateRulesSchemaMock(), - actions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.type'], // changing this value - }, - }, - action_type_id: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - action_type_id: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { - summary: true, - notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 - throttle: null, - }, - group: 'default', - }, - ], - }; - const existingRule = getRuleMock(getQueryRuleParams(), { - systemActions: [ - { - id: 'system-connector-.cases', - params: { - subAction: 'run', - subActionParams: { - timeWindow: '7d', - reopenClosedCases: false, - groupingBy: ['agent.name'], // changing this value - }, - }, - actionTypeId: '.cases', - uuid: 'e62cbe00-a0e1-44d9-9585-c39e5da63d6f', - }, - ], - actions: [ - { - id: 'b7da98d0-e1ef-4954-969f-e69c9ef5f65d', - params: { - message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', - }, - actionTypeId: '.slack', - uuid: '4c3601b5-74b9-4330-b2f3-fea4ea3dc046', - frequency: { - summary: true, - notifyWhen: 'onActiveAlert' as 'onActiveAlert', // needed for type check on line 127 - throttle: null, - }, - group: 'default', - }, - ], - }); + const existingRule = getRulesSchemaMock(); (getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule); rulesClient.update.mockResolvedValue( getRuleMock(getQueryRuleParams(), { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts index c420f4bc40e7a..fb7193b1007d3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.update_rule.test.ts @@ -115,7 +115,7 @@ describe('DetectionRulesClient.updateRule', () => { }, ], }; - const existingRule = getRuleMock(getQueryRuleParams()); + const existingRule = getRulesSchemaMock(); (getRuleByRuleId as jest.Mock).mockResolvedValueOnce(existingRule); rulesClient.update.mockResolvedValue( getRuleMock(getQueryRuleParams(), { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts index 61be75c2a6fa6..acdb7b9653930 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/detection_rules_client.upgrade_prebuilt_rule.test.ts @@ -33,9 +33,14 @@ describe('DetectionRulesClient.upgradePrebuiltRule', () => { let detectionRulesClient: IDetectionRulesClient; const mlAuthz = (buildMlAuthz as jest.Mock)(); - let actionsClient: jest.Mocked; + let actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; beforeEach(() => { + actionsClient = { + isSystemAction: jest.fn((id: string) => id === 'system-connector-.cases'), + } as unknown as jest.Mocked; rulesClient = rulesClientMock.create(); const savedObjectsClient = savedObjectsClientMock.create(); detectionRulesClient = createDetectionRulesClient({ From 991e10ae0fe4c385732ffdce5eaf2a9e14f3b5b9 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 17 Jul 2024 14:17:55 -0400 Subject: [PATCH 43/54] fixes ftr test for adding case system action to rule --- .../common/lib/api/case.ts | 17 ++++------------- .../trial_license_complete_tier/add_actions.ts | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/x-pack/test/cases_api_integration/common/lib/api/case.ts b/x-pack/test/cases_api_integration/common/lib/api/case.ts index c3810221759d2..93d1e8ef1c25b 100644 --- a/x-pack/test/cases_api_integration/common/lib/api/case.ts +++ b/x-pack/test/cases_api_integration/common/lib/api/case.ts @@ -37,13 +37,7 @@ export const createCase = async ( return theCase; }; -export const waitForCases = async ({ - supertest, - log, -}: { - supertest: SuperTest.Agent; - log: ToolingLog; -}): Promise => { +export const waitForCases = async (supertest: SuperTest.Agent, log: ToolingLog): Promise => { await waitFor( async () => { const response = await getCases(supertest); @@ -61,16 +55,13 @@ export const waitForCases = async ({ export const getCases = async ( supertest: SuperTest.Agent, expectedHttpCode: number = 200, - auth: { user: User; space: string | null } | null = { user: superUser, space: null }, headers: Record = {} ): Promise => { - const apiCall = supertest.get(`${getSpaceUrlPrefix(auth?.space)}${CASES_URL}/_find`); - - setupAuth({ apiCall, headers, auth }); - - const { body: theCase } = await apiCall + const { body: theCase } = await supertest + .get(`${CASES_URL}/_find`) .set('kbn-xsrf', 'true') .set('x-elastic-internal-origin', 'foo') + .set('elastic-api-version', '2023-10-31') .set(headers) .expect(expectedHttpCode); diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts index 9cd7a3c6b2691..5869c32dc4232 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/actions/trial_license_complete_tier/add_actions.ts @@ -95,7 +95,7 @@ export default ({ getService }: FtrProviderContext) => { }; const createdRule = await createRule(supertest, log, rule); const alerts = await getAlerts(supertest, log, es, createdRule); - await waitForCases({ supertest, log }); + await waitForCases(supertest, log); const cases = await getCases(supertest); expect(cases.cases[0].totalAlerts).toBeGreaterThan(0); expect(alerts.hits.hits.length).toBeGreaterThan(0); From 374309cf1c040c129acfc20bc77943a957c10d08 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 17 Jul 2024 16:22:25 -0400 Subject: [PATCH 44/54] undo before and after fn changes from testing --- .../execution_logic/custom_query.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts index c4ee12345af23..692f986e6e6a6 100644 --- a/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts +++ b/x-pack/test/security_solution_api_integration/test_suites/detections_response/detection_engine/rule_execution_logic/trial_license_complete_tier/execution_logic/custom_query.ts @@ -104,17 +104,11 @@ export default ({ getService }: FtrProviderContext) => { useCreate: true, docsOnly: true, }); - await deleteAllRules(supertest, log); - await esArchiver.load('x-pack/test/functional/es_archives/signals/severity_risk_overrides'); }); afterEach(async () => { await esDeleteAllIndices('.preview.alerts*'); - await esArchiver.unload(auditbeatPath); - await esArchiver.unload('x-pack/test/functional/es_archives/signals/severity_risk_overrides'); - await deleteAllAlerts(supertest, log, es, ['.preview.alerts-security.alerts-*']); - await deleteAllRules(supertest, log); }); after(async () => { From fee7dbbf71d661cd2ddbc26f8b7f2200ba1cf1eb Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 18 Jul 2024 14:10:20 -0400 Subject: [PATCH 45/54] remove unused function --- .../common/detection_engine/transform_actions.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts index 63b2bd06a9d8a..503c568f6edc9 100644 --- a/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts +++ b/x-pack/plugins/security_solution/common/detection_engine/transform_actions.ts @@ -5,8 +5,6 @@ * 2.0. */ -import { isEmpty } from 'lodash'; - import type { RuleAction as AlertingRuleAction, RuleSystemAction as AlertingRuleSystemAction, @@ -23,14 +21,6 @@ import type { import { ResponseActionTypesEnum } from '../api/detection_engine/model/rule_response_actions'; import type { RuleAction } from '../api/detection_engine/model'; -export const transformRuleActionsToAlertActions = ( - newActions: RuleAction[] | undefined, - existingActions: AlertingRuleAction[] | AlertingRuleSystemAction[] | undefined -): AlertingRuleAction[] | AlertingRuleSystemAction[] => - !isEmpty(newActions) && newActions != null - ? newActions.map((action) => transformRuleToAlertAction(action)) - : existingActions ?? []; - export const transformRuleToAlertAction = ({ group, id, From bd7c725e20b5aa41cd90f6264fc3f99efb5e7e7f Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 18 Jul 2024 14:11:52 -0400 Subject: [PATCH 46/54] adds comment to logic in notification action component --- .../components/step_rule_actions/notification_action.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index 0010c75fd1441..48091dcd410ba 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -102,6 +102,12 @@ export function NotificationAction({ const connectorType = connectorTypes.find(({ id }) => id === action.actionTypeId); const registeredAction = actionTypeRegistry.get(action.actionTypeId); + /* + since there is no "connector" for system actions, + we need to determine the title based off the action + properties in order to render helpful text on the + rule details page. + */ const connectorTypeName = isRuleAction ? connectorType?.name ?? '' : registeredAction.actionTypeTitle ?? ''; From 604a755628d7b89ac255ff3c3789af66ab845dec Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Thu, 18 Jul 2024 18:18:55 +0000 Subject: [PATCH 47/54] [CI] Auto-commit changed files from 'yarn openapi:bundle' --- ...urity_solution_detections_api_2023_10_31.bundled.schema.yaml | 2 -- ...urity_solution_detections_api_2023_10_31.bundled.schema.yaml | 2 -- 2 files changed, 4 deletions(-) diff --git a/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml index 2a239497d896e..520790d220d88 100644 --- a/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml +++ b/x-pack/plugins/security_solution/docs/openapi/ess/security_solution_detections_api_2023_10_31.bundled.schema.yaml @@ -4038,7 +4038,6 @@ components: params: $ref: '#/components/schemas/RuleActionParams' required: - - group - id - params NormalizedRuleError: @@ -4836,7 +4835,6 @@ components: $ref: '#/components/schemas/NonEmptyString' required: - action_type_id - - group - id - params RuleActionAlertsFilter: diff --git a/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml b/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml index e39ba6065675a..e956affbc3663 100644 --- a/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml +++ b/x-pack/plugins/security_solution/docs/openapi/serverless/security_solution_detections_api_2023_10_31.bundled.schema.yaml @@ -3236,7 +3236,6 @@ components: params: $ref: '#/components/schemas/RuleActionParams' required: - - group - id - params NormalizedRuleError: @@ -4034,7 +4033,6 @@ components: $ref: '#/components/schemas/NonEmptyString' required: - action_type_id - - group - id - params RuleActionAlertsFilter: From 8de96fa06e96db59e0b8d0a830f0c01f7844a874 Mon Sep 17 00:00:00 2001 From: "Devin W. Hurley" Date: Wed, 24 Jul 2024 10:23:56 -0400 Subject: [PATCH 48/54] Update x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx Co-authored-by: Vitalii Dmyterko <92328789+vitaliidm@users.noreply.github.com> --- .../components/step_rule_actions/notification_action.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index 48091dcd410ba..e1ba04710ee85 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -114,9 +114,9 @@ export function NotificationAction({ const iconType = registeredAction?.iconClass ?? 'apps'; const connector = connectors.find(({ id }) => id === action.id); - const connectorName = isRuleAction - ? connector?.name ?? '' - : registeredAction.actionTypeTitle ?? ''; + const connectorName = (isRuleAction + ? connector?.name + : registeredAction.actionTypeTitle) ?? ''; return ( From 19748e3a45fe5f602a73b35041493244dd274718 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 24 Jul 2024 10:25:41 -0400 Subject: [PATCH 49/54] adds check to ensure cases action is visible in serverless complete tier --- .../rule_actions/rule_actions_pli_complete.cy.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_complete.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_complete.cy.ts index 94c53cb6e3f13..ce7305a019fe1 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_complete.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/detection_response/detection_engine/rule_actions/rule_actions_pli_complete.cy.ts @@ -15,6 +15,7 @@ import { SERVER_LOG_ACTION_BTN, XMATTERS_ACTION_BTN, ACTION_BTN, + CASES_SYSTEM_ACTION_BTN, } from '../../../../screens/common/rule_actions'; import { createRule } from '../../../../tasks/api_calls/rules'; @@ -66,6 +67,7 @@ describe( cy.get(WEBHOOK_ACTION_BTN).should('be.visible'); cy.get(SERVER_LOG_ACTION_BTN).should('be.visible'); cy.get(XMATTERS_ACTION_BTN).should('be.visible'); + cy.get(CASES_SYSTEM_ACTION_BTN).should('be.visible'); }); } ); From 851e19a7a790070bd3e19de2d278627c52a2933d Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 24 Jul 2024 14:40:11 -0400 Subject: [PATCH 50/54] move function parameters so actionsClient is last --- .../converters/convert_rule_response_to_alerting_rule.ts | 4 ++-- .../logic/detection_rules_client/methods/create_rule.ts | 2 +- .../logic/detection_rules_client/methods/import_rule.ts | 2 +- .../logic/detection_rules_client/methods/patch_rule.ts | 2 +- .../logic/detection_rules_client/methods/update_rule.ts | 2 +- .../detection_rules_client/methods/upgrade_prebuilt_rule.ts | 2 +- .../detection_engine/rule_preview/api/preview_rules/route.ts | 4 ++-- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/convert_rule_response_to_alerting_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/convert_rule_response_to_alerting_rule.ts index 420af030a4e0b..52a00f1d6f42d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/convert_rule_response_to_alerting_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/converters/convert_rule_response_to_alerting_rule.ts @@ -40,8 +40,8 @@ type RuntimeFields = | 'execution_summary'; export const convertRuleResponseToAlertingRule = ( - actionsClient: ActionsClient, - rule: Omit + rule: Omit, + actionsClient: ActionsClient ): UpdateRuleData => { const [ruleSystemActions, ruleActions] = separateActionsAndSystemAction( actionsClient, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_rule.ts index 29cdf57bb367e..7bd3c9c46bf73 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/create_rule.ts @@ -42,7 +42,7 @@ export const createRule = async ({ const ruleWithDefaults = applyRuleDefaults(rule); const payload = { - ...convertRuleResponseToAlertingRule(actionsClient, ruleWithDefaults), + ...convertRuleResponseToAlertingRule(ruleWithDefaults, actionsClient), alertTypeId: ruleTypeMappings[rule.type], consumer: SERVER_APP_ID, enabled: rule.enabled ?? false, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts index ffed02304e243..f2532ea1a77bb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/import_rule.ts @@ -61,7 +61,7 @@ export const importRule = async ({ const updatedRule = await rulesClient.update({ id: existingRule.id, - data: convertRuleResponseToAlertingRule(actionsClient, ruleWithUpdates), + data: convertRuleResponseToAlertingRule(ruleWithUpdates, actionsClient), }); return convertAlertingRuleToRuleResponse(updatedRule); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts index a5e1619d0e157..1218991bf388e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/patch_rule.ts @@ -59,7 +59,7 @@ export const patchRule = async ({ const patchedInternalRule = await rulesClient.update({ id: existingRule.id, - data: convertRuleResponseToAlertingRule(actionsClient, patchedRule), + data: convertRuleResponseToAlertingRule(patchedRule, actionsClient), }); const { enabled } = await toggleRuleEnabledOnUpdate(rulesClient, existingRule, patchedRule); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts index 16178e4925564..cd84788026870 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/update_rule.ts @@ -58,7 +58,7 @@ export const updateRule = async ({ const updatedRule = await rulesClient.update({ id: existingRule.id, - data: convertRuleResponseToAlertingRule(actionsClient, ruleWithUpdates), + data: convertRuleResponseToAlertingRule(ruleWithUpdates, actionsClient), }); const { enabled } = await toggleRuleEnabledOnUpdate(rulesClient, existingRule, ruleWithUpdates); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts index fc283ccec7b31..ee5686e96d130 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/detection_rules_client/methods/upgrade_prebuilt_rule.ts @@ -77,7 +77,7 @@ export const upgradePrebuiltRule = async ({ const patchedInternalRule = await rulesClient.update({ id: existingRule.id, - data: convertRuleResponseToAlertingRule(actionsClient, patchedRule), + data: convertRuleResponseToAlertingRule(patchedRule, actionsClient), }); return convertAlertingRuleToRuleResponse(patchedInternalRule); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts index 0288698132070..1f33780acd441 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_preview/api/preview_rules/route.ts @@ -122,8 +122,8 @@ export const previewRulesRoute = ( } const internalRule = convertRuleResponseToAlertingRule( - actionsClient, - applyRuleDefaults(request.body) + applyRuleDefaults(request.body), + actionsClient ); const previewRuleParams = internalRule.params; From 38481967c4774607ebd1fa75768e2fe6677ea08f Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 24 Jul 2024 15:42:20 -0400 Subject: [PATCH 51/54] adds unit tests for changes to bulkEditActionToRulesClientOperation --- .../action_to_rules_client_operation.test.ts | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts index d9f05d1049d94..08bfba2613968 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/bulk_actions/action_to_rules_client_operation.test.ts @@ -71,4 +71,157 @@ describe('bulkEditActionToRulesClientOperation', () => { }, ]); }); + + test('should add_rule_actions non-system actions', () => { + expect( + bulkEditActionToRulesClientOperation(actionsClient, { + type: BulkActionEditTypeEnum.add_rule_actions, + value: { + actions: [ + { + group: 'default', + id: 'b0d183b2-3e04-428d-9fc4-ca7e23604380', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + }, + ], + }, + }) + ).toEqual([ + { + field: 'actions', + operation: 'add', + value: [ + { + group: 'default', + id: 'b0d183b2-3e04-428d-9fc4-ca7e23604380', + params: { + message: 'Rule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + }, + ], + }, + ]); + }); + + test('should add_rule_actions system actions', () => { + expect( + bulkEditActionToRulesClientOperation(actionsClient, { + type: BulkActionEditTypeEnum.add_rule_actions, + value: { + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + }, + ], + }, + }) + ).toEqual([ + { + field: 'actions', + operation: 'add', + value: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.name'], + }, + }, + }, + ], + }, + ]); + }); + + test('should set_rule_actions non-system actions', () => { + expect( + bulkEditActionToRulesClientOperation(actionsClient, { + type: BulkActionEditTypeEnum.set_rule_actions, + value: { + actions: [ + { + group: 'default', + id: 'b0d183b2-3e04-428d-9fc4-ca7e23604380', + params: { + message: + 'How many alerts were generated? This many alerts: {{state.signals_count}}', + }, + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + }, + ], + }, + }) + ).toEqual([ + { + field: 'actions', + operation: 'set', + value: [ + { + id: 'b0d183b2-3e04-428d-9fc4-ca7e23604380', + params: { + message: 'How many alerts were generated? This many alerts: {{state.signals_count}}', + }, + frequency: { summary: true, notifyWhen: 'onActiveAlert', throttle: null }, + group: 'default', + }, + ], + }, + ]); + }); + + test('should set_rule_actions system actions', () => { + expect( + bulkEditActionToRulesClientOperation(actionsClient, { + type: BulkActionEditTypeEnum.set_rule_actions, + value: { + actions: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + }, + ], + }, + }) + ).toEqual([ + { + field: 'actions', + operation: 'set', + value: [ + { + id: 'system-connector-.cases', + params: { + subAction: 'run', + subActionParams: { + timeWindow: '7d', + reopenClosedCases: false, + groupingBy: ['agent.type'], + }, + }, + }, + ], + }, + ]); + }); }); From 8d517b451741392edf8177a1e526c6c1618ba027 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Wed, 24 Jul 2024 15:43:14 -0400 Subject: [PATCH 52/54] fixes prettier problem from code suggestion commit in github --- .../components/step_rule_actions/notification_action.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index e1ba04710ee85..eac3b7c642c55 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -114,9 +114,7 @@ export function NotificationAction({ const iconType = registeredAction?.iconClass ?? 'apps'; const connector = connectors.find(({ id }) => id === action.id); - const connectorName = (isRuleAction - ? connector?.name - : registeredAction.actionTypeTitle) ?? ''; + const connectorName = (isRuleAction ? connector?.name : registeredAction.actionTypeTitle) ?? ''; return ( From 3903f00f84e17ee607ea60246f26de0b1eeba511 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 25 Jul 2024 12:53:21 -0400 Subject: [PATCH 53/54] adds frequency message for cases system action --- .../components/step_rule_actions/notification_action.tsx | 8 +++++++- .../components/step_rule_actions/translations.tsx | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx index eac3b7c642c55..c0906e0d8d652 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/notification_action.tsx @@ -131,7 +131,13 @@ export function NotificationAction({ - {isRuleAction && } + {isRuleAction ? ( + + ) : ( + // Display frequency description for system action + // same text used by stack alerting + {i18n.SYSTEM_ACTION_FREQUENCY} + )} diff --git a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/translations.tsx b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/translations.tsx index a1830186837c2..896a44bd7c29b 100644 --- a/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/translations.tsx +++ b/x-pack/plugins/security_solution/public/detection_engine/rule_creation/components/step_rule_actions/translations.tsx @@ -86,3 +86,8 @@ export const PERIODICALLY = i18n.translate( 'xpack.securitySolution.detectionEngine.actionNotifyWhen.periodically', { defaultMessage: 'Periodically' } ); + +export const SYSTEM_ACTION_FREQUENCY = i18n.translate( + 'xpack.securitySolution.detectionEngine.actionNotifyWhen.systemActionFrequency', + { defaultMessage: 'On check intervals' } +); From 256b6eaa2d03b73f3d9eae1dfec1f27eb5eeb8e3 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Fri, 26 Jul 2024 11:52:50 -0400 Subject: [PATCH 54/54] remove comment, delete whitespace --- x-pack/test/cases_api_integration/common/lib/api/case.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/x-pack/test/cases_api_integration/common/lib/api/case.ts b/x-pack/test/cases_api_integration/common/lib/api/case.ts index 93d1e8ef1c25b..24e204c648735 100644 --- a/x-pack/test/cases_api_integration/common/lib/api/case.ts +++ b/x-pack/test/cases_api_integration/common/lib/api/case.ts @@ -41,10 +41,7 @@ export const waitForCases = async (supertest: SuperTest.Agent, log: ToolingLog): await waitFor( async () => { const response = await getCases(supertest); - - // TODO: https://github.com/elastic/kibana/pull/121644 clean up, make type-safe const cases = response; - return cases != null && cases.cases.length > 0 && cases?.cases[0]?.totalAlerts > 0; }, 'waitForCaseToAttachAlert',