diff --git a/x-pack/plugins/security_solution/common/constants.ts b/x-pack/plugins/security_solution/common/constants.ts index 10cde80df480..c746ed1006e5 100644 --- a/x-pack/plugins/security_solution/common/constants.ts +++ b/x-pack/plugins/security_solution/common/constants.ts @@ -225,6 +225,11 @@ export const INTERNAL_RULE_ID_KEY = `${INTERNAL_IDENTIFIER}_rule_id` as const; export const INTERNAL_RULE_ALERT_ID_KEY = `${INTERNAL_IDENTIFIER}_rule_alert_id` as const; export const INTERNAL_IMMUTABLE_KEY = `${INTERNAL_IDENTIFIER}_immutable` as const; +/** + * Internal actions route + */ +export const UPDATE_OR_CREATE_LEGACY_ACTIONS = '/internal/api/detection/legacy/notifications'; + /** * Detection engine routes */ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts index 510a2c135170..067f7b80dfca 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_bulk_route.ts @@ -78,7 +78,7 @@ export const updateRulesBulkRoute = ( id: payloadRule.id, }); - await legacyMigrate({ + const migratedRule = await legacyMigrate({ rulesClient, savedObjectsClient, rule: existingRule, @@ -89,6 +89,8 @@ export const updateRulesBulkRoute = ( rulesClient, ruleStatusClient, defaultOutputIndex: siemClient.getSignalsIndex(), + existingRule, + migratedRule, ruleUpdate: payloadRule, isRuleRegistryEnabled, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts index 8b3bacd2e7e2..543591c415a6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.ts @@ -69,7 +69,7 @@ export const updateRulesRoute = ( id: request.body.id, }); - await legacyMigrate({ + const migratedRule = await legacyMigrate({ rulesClient, savedObjectsClient, rule: existingRule, @@ -79,6 +79,8 @@ export const updateRulesRoute = ( isRuleRegistryEnabled, rulesClient, ruleStatusClient, + existingRule, + migratedRule, ruleUpdate: request.body, spaceId: context.securitySolution.getSpaceId(), }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts index 3509e003b971..037f85091bfc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/types.ts @@ -274,6 +274,8 @@ export interface UpdateRulesOptions { ruleStatusClient: IRuleExecutionLogClient; rulesClient: RulesClient; defaultOutputIndex: string; + existingRule: SanitizedAlert | null | undefined; + migratedRule: SanitizedAlert | null | undefined; ruleUpdate: UpdateRulesSchema; } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.mock.ts index 9a7711fcc898..b98a95ed1aab 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.mock.ts @@ -12,6 +12,8 @@ import { getUpdateRulesSchemaMock, } from '../../../../common/detection_engine/schemas/request/rule_schemas.mock'; import { ruleExecutionLogClientMock } from '../rule_execution_log/__mocks__/rule_execution_log_client'; +import { getAlertMock } from '../routes/__mocks__/request_responses'; +import { getQueryRuleParams } from '../schemas/rule_schemas.mock'; export const getUpdateRulesOptionsMock = (isRuleRegistryEnabled: boolean) => ({ spaceId: 'default', @@ -19,6 +21,8 @@ export const getUpdateRulesOptionsMock = (isRuleRegistryEnabled: boolean) => ({ ruleStatusClient: ruleExecutionLogClientMock.create(), savedObjectsClient: savedObjectsClientMock.create(), defaultOutputIndex: '.siem-signals-default', + existingRule: getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), + migratedRule: getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), ruleUpdate: getUpdateRulesSchemaMock(), isRuleRegistryEnabled, }); @@ -29,6 +33,8 @@ export const getUpdateMlRulesOptionsMock = (isRuleRegistryEnabled: boolean) => ( ruleStatusClient: ruleExecutionLogClientMock.create(), savedObjectsClient: savedObjectsClientMock.create(), defaultOutputIndex: '.siem-signals-default', + existingRule: getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), + migratedRule: getAlertMock(isRuleRegistryEnabled, getQueryRuleParams()), ruleUpdate: getUpdateMachineLearningSchemaMock(), isRuleRegistryEnabled, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts index 5e2c41fd3d27..ae16d0435e3d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.ts @@ -10,13 +10,19 @@ import { validate } from '@kbn/securitysolution-io-ts-utils'; import { DEFAULT_MAX_SIGNALS } from '../../../../common/constants'; import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; import { PartialAlert } from '../../../../../alerting/server'; -import { readRules } from './read_rules'; + import { UpdateRulesOptions } from './types'; import { addTags } from './add_tags'; import { typeSpecificSnakeToCamel } from '../schemas/rule_converters'; import { internalRuleUpdate, RuleParams } from '../schemas/rule_schemas'; import { enableRule } from './enable_rule'; -import { maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils'; +import { + maybeMute, + transformToAlertThrottle, + transformToNotifyWhen, + updateActions, + updateThrottleNotifyWhen, +} from './utils'; class UpdateError extends Error { public readonly statusCode: number; @@ -27,19 +33,14 @@ class UpdateError extends Error { } export const updateRules = async ({ - isRuleRegistryEnabled, spaceId, rulesClient, ruleStatusClient, defaultOutputIndex, + existingRule, + migratedRule, ruleUpdate, }: UpdateRulesOptions): Promise | null> => { - const existingRule = await readRules({ - isRuleRegistryEnabled, - rulesClient, - ruleId: ruleUpdate.rule_id, - id: ruleUpdate.id, - }); if (existingRule == null) { return null; } @@ -85,9 +86,24 @@ export const updateRules = async ({ ...typeSpecificParams, }, schedule: { interval: ruleUpdate.interval ?? '5m' }, - actions: ruleUpdate.actions != null ? ruleUpdate.actions.map(transformRuleToAlertAction) : [], - throttle: transformToAlertThrottle(ruleUpdate.throttle), - notifyWhen: transformToNotifyWhen(ruleUpdate.throttle), + actions: updateActions( + transformRuleToAlertAction, + migratedRule?.actions, + existingRule.actions, + ruleUpdate?.actions + ), + throttle: updateThrottleNotifyWhen( + transformToAlertThrottle, + migratedRule?.throttle, + existingRule.throttle, + ruleUpdate?.throttle + ), + notifyWhen: updateThrottleNotifyWhen( + transformToNotifyWhen, + migratedRule?.notifyWhen, + existingRule.notifyWhen, + ruleUpdate?.throttle + ), }; const [validated, errors] = validate(newInternalRule, internalRuleUpdate); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts index a558024a73e3..c9e00486dc13 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { pickBy, isEmpty } from 'lodash/fp'; +import { pickBy, isEmpty, isEqual } from 'lodash/fp'; import type { FromOrUndefined, MachineLearningJobIdOrUndefined, @@ -64,10 +64,14 @@ import { RulesClient } from '../../../../../alerting/server'; // eslint-disable-next-line no-restricted-imports import { LegacyRuleActions } from '../rule_actions/legacy_types'; import { FullResponseSchema } from '../../../../common/detection_engine/schemas/request'; -import { transformAlertToRuleAction } from '../../../../common/detection_engine/transform_actions'; +import { + transformAlertToRuleAction, + transformRuleToAlertAction, +} from '../../../../common/detection_engine/transform_actions'; // eslint-disable-next-line no-restricted-imports import { legacyRuleActionsSavedObjectType } from '../rule_actions/legacy_saved_object_mappings'; import { LegacyMigrateParams } from './types'; +import { RuleAlertAction } from '../../../../common/detection_engine/types'; export const calculateInterval = ( interval: string | undefined, @@ -331,6 +335,10 @@ export const legacyMigrate = async ({ }), savedObjectsClient.find({ type: legacyRuleActionsSavedObjectType, + hasReference: { + type: 'alert', + id: rule.id, + }, }), ]); @@ -344,8 +352,10 @@ export const legacyMigrate = async ({ ) : null, ]); + + const { id, ...restOfRule } = rule; const migratedRule = { - ...rule, + ...restOfRule, actions: siemNotification.data[0].actions, throttle: siemNotification.data[0].schedule.interval, notifyWhen: transformToNotifyWhen(siemNotification.data[0].throttle), @@ -354,7 +364,39 @@ export const legacyMigrate = async ({ id: rule.id, data: migratedRule, }); - return migratedRule; + return { id: rule.id, ...migratedRule }; } return rule; }; + +export const updateThrottleNotifyWhen = ( + transform: typeof transformToAlertThrottle | typeof transformToNotifyWhen, + migratedRuleThrottle: string | null | undefined, + existingRuleThrottle: string | null | undefined, + ruleUpdateThrottle: string | null | undefined +) => { + if (existingRuleThrottle == null && ruleUpdateThrottle == null && migratedRuleThrottle != null) { + return migratedRuleThrottle; + } + return transform(ruleUpdateThrottle); +}; + +export const updateActions = ( + transform: typeof transformRuleToAlertAction, + migratedRuleActions: AlertAction[] | null | undefined, + existingRuleActions: AlertAction[] | null | undefined, + ruleUpdateActions: RuleAlertAction[] | null | undefined +) => { + // if the existing rule actions and the rule update actions are equivalent (aka no change) + // but the migrated actions and the ruleUpdateActions (or existing rule actions, associatively) + // are not equivalent, we know that the rules' actions were migrated and we need to apply + // that change to the update request so it is not overwritten by the rule update payload + if ( + existingRuleActions?.length === 0 && + ruleUpdateActions == null && + !isEqual(existingRuleActions, migratedRuleActions) + ) { + return migratedRuleActions; + } + return ruleUpdateActions != null ? ruleUpdateActions.map(transform) : []; +}; diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules.ts index d20eb0492bbc..6918dc7a1876 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules.ts @@ -21,6 +21,7 @@ import { getSimpleMlRuleOutput, createRule, getSimpleMlRule, + createLegacyRuleAction, } from '../../utils'; // eslint-disable-next-line import/no-default-export @@ -187,6 +188,46 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare).to.eql(outputRule); }); + it('should return the rule with migrated actions after the enable patch', async () => { + const [connector, rule] = await Promise.all([ + supertest + .post(`/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My action', + connector_type_id: '.slack', + secrets: { + webhookUrl: 'http://localhost:1234', + }, + }), + createRule(supertest, getSimpleRule('rule-1')), + ]); + await createLegacyRuleAction(supertest, rule.id, connector.body.id); + + // patch disable the rule + const patchResponse = await supertest + .patch(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send({ id: rule.id, enabled: false }) + .expect(200); + + const outputRule = getSimpleRuleOutput(); + outputRule.actions = [ + { + action_type_id: '.slack', + group: 'default', + id: connector.body.id, + params: { + message: + 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + }, + ]; + outputRule.throttle = '1m'; + const bodyToCompare = removeServerGeneratedProperties(patchResponse.body); + expect(bodyToCompare).to.eql(outputRule); + }); + it('should give a 404 if it is given a fake id', async () => { const { body } = await supertest .patch(DETECTION_ENGINE_RULES_URL) diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules_bulk.ts index 74d3bc8dd68d..bc388d73cac6 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/patch_rules_bulk.ts @@ -19,6 +19,7 @@ import { getSimpleRuleOutputWithoutRuleId, removeServerGeneratedPropertiesIncludingRuleId, createRule, + createLegacyRuleAction, } from '../../utils'; // eslint-disable-next-line import/no-default-export @@ -126,6 +127,55 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare2).to.eql(outputRule2); }); + it('should bulk disable two rules and migrate their actions', async () => { + const [connector, rule1, rule2] = await Promise.all([ + supertest + .post(`/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My action', + connector_type_id: '.slack', + secrets: { + webhookUrl: 'http://localhost:1234', + }, + }), + createRule(supertest, getSimpleRule('rule-1')), + createRule(supertest, getSimpleRule('rule-2')), + ]); + await Promise.all([ + createLegacyRuleAction(supertest, rule1.id, connector.body.id), + createLegacyRuleAction(supertest, rule2.id, connector.body.id), + ]); + // patch a simple rule's name + const { body } = await supertest + .patch(`${DETECTION_ENGINE_RULES_URL}/_bulk_update`) + .set('kbn-xsrf', 'true') + .send([ + { id: rule1.id, enabled: false }, + { id: rule2.id, enabled: false }, + ]) + .expect(200); + + // @ts-expect-error + body.forEach((response) => { + const outputRule = getSimpleRuleOutput(response.rule_id, false); + outputRule.actions = [ + { + action_type_id: '.slack', + group: 'default', + id: connector.body.id, + params: { + message: + 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + }, + ]; + outputRule.throttle = '1m'; + const bodyToCompare = removeServerGeneratedProperties(response); + expect(bodyToCompare).to.eql(outputRule); + }); + }); + it('should patch a single rule property of name using the auto-generated id', async () => { const createdBody = await createRule(supertest, getSimpleRule('rule-1')); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules.ts index 5a4a04f71b3d..2b91e0c3abd5 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules.ts @@ -23,6 +23,7 @@ import { getSimpleMlRuleUpdate, createRule, getSimpleRule, + createLegacyRuleAction, } from '../../utils'; // eslint-disable-next-line import/no-default-export @@ -130,6 +131,55 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare).to.eql(outputRule); }); + it('should update a single rule property of name using an auto-generated rule_id and migrate the actions', async () => { + const rule = getSimpleRule('rule-1'); + delete rule.rule_id; + const [connector, createRuleBody] = await Promise.all([ + supertest + .post(`/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My action', + connector_type_id: '.slack', + secrets: { + webhookUrl: 'http://localhost:1234', + }, + }), + createRule(supertest, rule), + ]); + await createLegacyRuleAction(supertest, createRuleBody.id, connector.body.id); + + // update a simple rule's name + const updatedRule = getSimpleRuleUpdate('rule-1'); + updatedRule.rule_id = createRuleBody.rule_id; + updatedRule.name = 'some other name'; + delete updatedRule.id; + + const { body } = await supertest + .put(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send(updatedRule) + .expect(200); + + const outputRule = getSimpleRuleOutputWithoutRuleId(); + outputRule.name = 'some other name'; + outputRule.version = 2; + outputRule.actions = [ + { + action_type_id: '.slack', + group: 'default', + id: connector.body.id, + params: { + message: + 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + }, + ]; + outputRule.throttle = '1m'; + const bodyToCompare = removeServerGeneratedPropertiesIncludingRuleId(body); + expect(bodyToCompare).to.eql(outputRule); + }); + it('should update a single rule property of name using the auto-generated id', async () => { const createdBody = await createRule(supertest, getSimpleRule('rule-1')); diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts index 7612aafb5bb6..d950b30d584a 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/update_rules_bulk.ts @@ -20,6 +20,7 @@ import { getSimpleRuleUpdate, createRule, getSimpleRule, + createLegacyRuleAction, } from '../../utils'; // eslint-disable-next-line import/no-default-export @@ -94,6 +95,64 @@ export default ({ getService }: FtrProviderContext) => { expect(bodyToCompare2).to.eql(outputRule2); }); + it('should update two rule properties of name using the two rules rule_id and migrate actions', async () => { + const [connector, rule1, rule2] = await Promise.all([ + supertest + .post(`/api/actions/connector`) + .set('kbn-xsrf', 'foo') + .send({ + name: 'My action', + connector_type_id: '.slack', + secrets: { + webhookUrl: 'http://localhost:1234', + }, + }), + createRule(supertest, getSimpleRule('rule-1')), + createRule(supertest, getSimpleRule('rule-2')), + ]); + await Promise.all([ + createLegacyRuleAction(supertest, rule1.id, connector.body.id), + createLegacyRuleAction(supertest, rule2.id, connector.body.id), + ]); + + expect(rule1.actions).to.eql([]); + expect(rule2.actions).to.eql([]); + + const updatedRule1 = getSimpleRuleUpdate('rule-1'); + updatedRule1.name = 'some other name'; + + const updatedRule2 = getSimpleRuleUpdate('rule-2'); + updatedRule2.name = 'some other name'; + + // update both rule names + const { body } = await supertest + .put(`${DETECTION_ENGINE_RULES_URL}/_bulk_update`) + .set('kbn-xsrf', 'true') + .send([updatedRule1, updatedRule2]) + .expect(200); + + // @ts-expect-error + body.forEach((response) => { + const outputRule = getSimpleRuleOutput(response.rule_id); + outputRule.name = 'some other name'; + outputRule.version = 2; + outputRule.actions = [ + { + action_type_id: '.slack', + group: 'default', + id: connector.body.id, + params: { + message: + 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + }, + ]; + outputRule.throttle = '1m'; + const bodyToCompare = removeServerGeneratedProperties(response); + expect(bodyToCompare).to.eql(outputRule); + }); + }); + it('should update a single rule property of name using an id', async () => { const createRuleBody = await createRule(supertest, getSimpleRule('rule-1')); diff --git a/x-pack/test/detection_engine_api_integration/utils.ts b/x-pack/test/detection_engine_api_integration/utils.ts index 4781df7993a4..ae769bd01b52 100644 --- a/x-pack/test/detection_engine_api_integration/utils.ts +++ b/x-pack/test/detection_engine_api_integration/utils.ts @@ -48,6 +48,7 @@ import { DETECTION_ENGINE_SIGNALS_MIGRATION_URL, INTERNAL_IMMUTABLE_KEY, INTERNAL_RULE_ID_KEY, + UPDATE_OR_CREATE_LEGACY_ACTIONS, } from '../../plugins/security_solution/common/constants'; import { RACAlert } from '../../plugins/security_solution/server/lib/detection_engine/rule_types/types'; @@ -516,6 +517,29 @@ export const createSignalsIndex = async ( }, 'createSignalsIndex'); }; +export const createLegacyRuleAction = async ( + supertest: SuperTest.SuperTest, + alertId: string, + connectorId: string +): Promise => + supertest + .post(`${UPDATE_OR_CREATE_LEGACY_ACTIONS}`) + .set('kbn-xsrf', 'true') + .query({ alert_id: alertId }) + .send({ + name: 'Legacy notification with one action', + interval: '1m', + actions: [ + { + id: connectorId, + group: 'default', + params: { + message: 'Hourly\nRule {{context.rule.name}} generated {{state.signals_count}} alerts', + }, + actionTypeId: '.slack', + }, + ], + }); /** * Deletes the signals index for use inside of afterEach blocks of tests * @param supertest The supertest client library