diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts index 98269ab3a7b2bf..d1851566e16c80 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.test.ts @@ -27,8 +27,12 @@ import { savedObjectWith500Error, disabledRuleForBulkDisable1, disabledRuleForBulkDisable2, + returnedRuleForBulkDisableWithActions1, + returnedRuleForBulkDisableWithActions2, enabledRuleForBulkOps1, enabledRuleForBulkOps2, + disabledRuleForBulkOpsWithActions1, + disabledRuleForBulkOpsWithActions2, returnedRuleForBulkDisable1, returnedRuleForBulkDisable2, siemRuleForBulkOps1, @@ -36,6 +40,7 @@ import { } from '../../../../rules_client/tests/test_helpers'; import { migrateLegacyActions } from '../../../../rules_client/lib'; import { ConnectorAdapterRegistry } from '../../../../connector_adapters/connector_adapter_registry'; +import { ActionsClient } from '@kbn/actions-plugin/server'; jest.mock('../../../../task_runner/alert_task_instance', () => ({ taskInstanceToAlertTaskInstance: jest.fn(), @@ -95,6 +100,7 @@ const rulesClientParams: jest.Mocked = { isAuthenticationTypeAPIKey: jest.fn(), getAuthenticationAPIKey: jest.fn(), connectorAdapterRegistry: new ConnectorAdapterRegistry(), + isSystemAction: jest.fn(), getAlertIndicesAlias: jest.fn(), alertsService: null, }; @@ -108,6 +114,8 @@ setGlobalDate(); describe('bulkDisableRules', () => { let rulesClient: RulesClient; + let actionsClient: jest.Mocked; + const mockCreatePointInTimeFinderAsInternalUser = ( response = { saved_objects: [enabledRule1, enabledRule2] } ) => { @@ -143,6 +151,9 @@ describe('bulkDisableRules', () => { beforeEach(async () => { rulesClient = new RulesClient(rulesClientParams); + actionsClient = (await rulesClientParams.getActionsClient()) as jest.Mocked; + rulesClientParams.getActionsClient.mockResolvedValue(actionsClient); + authorization.getFindAuthorizationFilter.mockResolvedValue({ ensureRuleTypeIsAuthorized() {}, }); @@ -153,6 +164,7 @@ describe('bulkDisableRules', () => { resultedActions: [], resultedReferences: [], }); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action:id'); }); test('should disable two rule', async () => { @@ -188,6 +200,39 @@ describe('bulkDisableRules', () => { }); }); + test('should disable two rule and return right actions', async () => { + unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ + saved_objects: [disabledRuleForBulkOpsWithActions1, disabledRuleForBulkOpsWithActions2], + }); + + const result = await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); + + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledTimes(1); + expect(unsecuredSavedObjectsClient.bulkCreate).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ + id: 'id1', + attributes: expect.objectContaining({ + enabled: false, + }), + }), + expect.objectContaining({ + id: 'id2', + attributes: expect.objectContaining({ + enabled: false, + }), + }), + ]), + { overwrite: true } + ); + + expect(result).toStrictEqual({ + errors: [], + rules: [returnedRuleForBulkDisableWithActions1, returnedRuleForBulkDisableWithActions2], + total: 2, + }); + }); + test('should try to disable rules, one successful and one with 500 error', async () => { unsecuredSavedObjectsClient.bulkCreate.mockResolvedValue({ saved_objects: [disabledRuleForBulkDisable1, savedObjectWith500Error], @@ -656,25 +701,25 @@ describe('bulkDisableRules', () => { await rulesClient.bulkDisableRules({ filter: 'fake_filter' }); expect(migrateLegacyActions).toHaveBeenCalledTimes(4); - expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), { + expect(migrateLegacyActions).toHaveBeenNthCalledWith(1, expect.any(Object), { attributes: enabledRuleForBulkOps1.attributes, ruleId: enabledRuleForBulkOps1.id, actions: [], references: [], }); - expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), { + expect(migrateLegacyActions).toHaveBeenNthCalledWith(2, expect.any(Object), { attributes: enabledRuleForBulkOps2.attributes, ruleId: enabledRuleForBulkOps2.id, actions: [], references: [], }); - expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), { + expect(migrateLegacyActions).toHaveBeenNthCalledWith(3, expect.any(Object), { attributes: expect.objectContaining({ consumer: AlertConsumers.SIEM }), ruleId: siemRuleForBulkOps1.id, actions: [], references: [], }); - expect(migrateLegacyActions).toHaveBeenCalledWith(expect.any(Object), { + expect(migrateLegacyActions).toHaveBeenNthCalledWith(4, expect.any(Object), { attributes: expect.objectContaining({ consumer: AlertConsumers.SIEM }), ruleId: siemRuleForBulkOps2.id, actions: [], diff --git a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts index e981a30181a494..e48d0f9bca983b 100644 --- a/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts +++ b/x-pack/plugins/alerting/server/application/rule/methods/bulk_disable/bulk_disable_rules.ts @@ -50,6 +50,7 @@ export const bulkDisableRules = async ( } const { ids, filter } = options; + const actionsClient = await context.getActionsClient(); const kueryNodeFilter = ids ? convertRuleIdsToKueryNode(ids) : buildKueryNodeFilter(filter); const authorizationFilter = await getAuthorizationFilter(context, { action: 'DISABLE' }); @@ -93,13 +94,17 @@ export const bulkDisableRules = async ( // fix the type cast from SavedObjectsBulkUpdateObject to SavedObjectsBulkUpdateObject // when we are doing the bulk disable and this should fix itself const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId!); - const ruleDomain = transformRuleAttributesToRuleDomain(attributes as RuleAttributes, { - id, - logger: context.logger, - ruleType, - references, - omitGeneratedValues: false, - }); + const ruleDomain = transformRuleAttributesToRuleDomain( + attributes as RuleAttributes, + { + id, + logger: context.logger, + ruleType, + references, + omitGeneratedValues: false, + }, + (connectorId: string) => actionsClient.isSystemAction(connectorId) + ); try { ruleDomainSchema.validate(ruleDomain); diff --git a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.test.ts b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.test.ts index a53501060a39f6..0aad303f63b16f 100644 --- a/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.test.ts +++ b/x-pack/plugins/alerting/server/routes/rule/apis/bulk_disable/bulk_disable_rules_route.test.ts @@ -13,6 +13,13 @@ import { mockHandlerArguments } from '../../../_mock_handler_arguments'; import { rulesClientMock } from '../../../../rules_client.mock'; import { RuleTypeDisabledError } from '../../../../lib/errors/rule_type_disabled'; import { verifyApiAccess } from '../../../../lib/license_api_access'; +import { actionsClientMock } from '@kbn/actions-plugin/server/mocks'; +import { + RuleActionTypes, + RuleDefaultAction, + RuleSystemAction, + SanitizedRule, +} from '../../../../types'; const rulesClient = rulesClientMock.create(); @@ -123,4 +130,120 @@ describe('bulkDisableRulesRoute', () => { expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } }); }); + + describe('actions', () => { + const mockedRule: SanitizedRule<{}> = { + id: '1', + alertTypeId: '1', + schedule: { interval: '10s' }, + params: { + bar: true, + }, + createdAt: new Date(), + updatedAt: new Date(), + actions: [ + { + group: 'default', + id: '2', + actionTypeId: 'test', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.DEFAULT, + }, + ], + consumer: 'bar', + name: 'abc', + tags: ['foo'], + enabled: false, + muteAll: false, + notifyWhen: 'onActionGroupChange', + createdBy: '', + updatedBy: '', + apiKeyOwner: '', + throttle: '30s', + mutedInstanceIds: [], + executionStatus: { + status: 'unknown', + lastExecutionDate: new Date('2020-08-20T19:23:38Z'), + }, + revision: 0, + }; + + const action: RuleDefaultAction = { + actionTypeId: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.DEFAULT, + }; + + const systemAction: RuleSystemAction = { + actionTypeId: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + uuid: '123-456', + type: RuleActionTypes.SYSTEM, + }; + + const mockedRules: Array> = [ + { ...mockedRule, actions: [action, systemAction] }, + ]; + + const bulkDisableActionsResult = { + rules: mockedRules, + errors: [], + total: 1, + skipped: [], + }; + + it('removes the type from the actions correctly before sending the response', async () => { + const licenseState = licenseStateMock.create(); + const router = httpServiceMock.createRouter(); + const actionsClient = actionsClientMock.create(); + actionsClient.isSystemAction.mockImplementation((id: string) => id === 'system_action-id'); + + bulkDisableRulesRoute({ router, licenseState }); + const [_, handler] = router.patch.mock.calls[0]; + + rulesClient.bulkDisableRules.mockResolvedValueOnce(bulkDisableActionsResult); + + const [context, req, res] = mockHandlerArguments( + { rulesClient, actionsClient }, + { + body: bulkDisableRequest, + }, + ['ok'] + ); + + const routeRes = await handler(context, req, res); + + // @ts-expect-error: body exists + expect(routeRes.body.rules[0].actions).toEqual([ + { + connector_type_id: 'test', + group: 'default', + id: '2', + params: { + foo: true, + }, + uuid: '123-456', + }, + { + connector_type_id: 'test-2', + id: 'system_action-id', + params: { + foo: true, + }, + uuid: '123-456', + }, + ]); + }); + }); }); diff --git a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts index 75bcb39e522b91..da14fe60e47fbd 100644 --- a/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts +++ b/x-pack/plugins/alerting/server/rules_client/lib/siem_legacy_actions/migrate_legacy_actions.ts @@ -7,15 +7,13 @@ import Boom from '@hapi/boom'; import { i18n } from '@kbn/i18n'; - import { AlertConsumers } from '@kbn/rule-data-utils'; - import type { SavedObjectReference } from '@kbn/core/server'; import type { RulesClientContext } from '../..'; import { RawRuleAction, RawRule } from '../../../types'; import { validateActions } from '../validate_actions'; -import { injectReferencesIntoActions } from '../../common'; import { retrieveMigratedLegacyActions } from './retrieve_migrated_legacy_actions'; +import { transformRawActionsToDomainActions } from '../../../application/rule/transforms/transform_raw_actions_to_domain_actions'; type MigrateLegacyActions = ( context: RulesClientContext, @@ -66,7 +64,12 @@ export const migrateLegacyActions: MigrateLegacyActions = async ( // set to undefined to avoid both per-actin and rule level values clashing throttle: undefined, notifyWhen: undefined, - actions: injectReferencesIntoActions(ruleId, legacyActions, legacyActionsReferences), + actions: transformRawActionsToDomainActions({ + ruleId, + actions: legacyActions, + references: legacyActionsReferences, + isSystemAction: context.isSystemAction, + }), }); }; diff --git a/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts b/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts index 8b2db346865770..ac67d90ec9d510 100644 --- a/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts +++ b/x-pack/plugins/alerting/server/rules_client/tests/test_helpers.ts @@ -147,6 +147,64 @@ export const enabledRuleForBulkOps2 = { }, }; +export const disabledRuleForBulkOpsWithActions1 = { + ...defaultRuleForBulkDelete, + attributes: { + ...defaultRuleForBulkDelete.attributes, + enabled: false, + scheduledTaskId: 'id1', + apiKey: Buffer.from('123:abc').toString('base64'), + actions: [ + { + uuid: '1', + id: 'system_action:id', + actionTypeId: '1', + actionRef: '1', + params: { + foo: true, + }, + }, + ], + }, + references: [ + { + id: 'system_action:id', + name: '1', + type: 'action', + }, + ], +}; + +export const disabledRuleForBulkOpsWithActions2 = { + ...defaultRuleForBulkDelete, + id: 'id2', + attributes: { + ...defaultRuleForBulkDelete.attributes, + enabled: false, + scheduledTaskId: 'id2', + apiKey: Buffer.from('321:abc').toString('base64'), + actions: [ + { + uuid: '2', + id: 'default_action:id', + group: 'default', + actionTypeId: '2', + actionRef: '2', + params: { + foo: true, + }, + }, + ], + }, + references: [ + { + id: 'default_action:id', + name: '2', + type: 'action', + }, + ], +}; + export const enabledRuleForBulkOps3 = { ...defaultRuleForBulkDelete, id: 'id3', @@ -389,6 +447,37 @@ export const returnedRuleForBulkDisable2 = { enabled: false, }; +export const returnedRuleForBulkDisableWithActions1 = { + ...returnedRuleForBulkDisable1, + actions: [ + { + actionTypeId: '1', + id: 'system_action:id', + params: { + foo: true, + }, + type: 'system', + uuid: '1', + }, + ], +}; + +export const returnedRuleForBulkDisableWithActions2 = { + ...returnedRuleForBulkDisable2, + actions: [ + { + actionTypeId: '2', + group: 'default', + id: 'default_action:id', + params: { + foo: true, + }, + type: 'default', + uuid: '2', + }, + ], +}; + export const returnedDisabledRule1 = { ...returnedRule1, enabled: false,