From 21a9ffc7f1814e66438b26e2527be355901f363a Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Mon, 23 Aug 2021 11:59:58 -0600 Subject: [PATCH 01/10] wip - Removing actions side car model for the notify API --- .../rules_notification_alert_type.ts | 72 ++----------------- .../notifications/update_notifications.ts | 3 + .../routes/rules/create_rules_bulk_route.ts | 10 ++- .../routes/rules/create_rules_route.ts | 10 ++- .../routes/rules/import_rules_route.ts | 3 + .../routes/rules/patch_rules_bulk_route.ts | 1 + .../routes/rules/patch_rules_route.ts | 1 + ...ate_or_create_rule_actions_saved_object.ts | 1 + .../create_security_rule_type_factory.ts | 1 + .../rules/create_rules.mock.ts | 2 + .../detection_engine/rules/create_rules.ts | 21 ++++-- .../rules/install_prepacked_rules.ts | 1 + .../rules/patch_rules.mock.ts | 2 + .../lib/detection_engine/rules/patch_rules.ts | 29 +++++++- .../lib/detection_engine/rules/types.ts | 2 + .../rules/update_prepacked_rules.ts | 2 + .../detection_engine/rules/update_rules.ts | 22 ++++-- .../rules/update_rules_notifications.ts | 2 + .../lib/detection_engine/rules/utils.ts | 19 +++++ .../schemas/rule_converters.ts | 7 +- .../detection_engine/schemas/rule_schemas.ts | 15 +++- .../signals/signal_rule_alert_type.ts | 52 +++++++++++--- 22 files changed, 180 insertions(+), 98 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts index c85848ba6dcfe..78a47bb3ed4eb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts @@ -7,20 +7,13 @@ import { Logger } from 'src/core/server'; import { schema } from '@kbn/config-schema'; -import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils'; -import { - DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, - NOTIFICATIONS_ID, - SERVER_APP_ID, -} from '../../../../common/constants'; +import { NOTIFICATIONS_ID, SERVER_APP_ID } from '../../../../common/constants'; import { NotificationAlertTypeDefinition } from './types'; -import { AlertAttributes } from '../signals/types'; import { siemRuleActionGroups } from '../signals/siem_rule_action_groups'; -import { scheduleNotificationActions } from './schedule_notification_actions'; -import { getNotificationResultsLink } from './utils'; -import { getSignals } from './get_signals'; +// TODO: Can we remove this function/rule or could that cause issues? +// Do we have to find all rule instances and then delete it on startup and then remove it here? export const rulesNotificationAlertType = ({ logger, }: { @@ -38,62 +31,7 @@ export const rulesNotificationAlertType = ({ }, minimumLicenseRequired: 'basic', isExportable: false, - async executor({ startedAt, previousStartedAt, alertId, services, params }) { - const ruleAlertSavedObject = await services.savedObjectsClient.get( - 'alert', - params.ruleAlertId - ); - - if (!ruleAlertSavedObject.attributes.params) { - logger.error(`Saved object for alert ${params.ruleAlertId} was not found`); - return; - } - - const { params: ruleAlertParams, name: ruleName } = ruleAlertSavedObject.attributes; - const ruleParams = { ...ruleAlertParams, name: ruleName, id: ruleAlertSavedObject.id }; - - const fromInMs = parseScheduleDates( - previousStartedAt - ? previousStartedAt.toISOString() - : `now-${ruleAlertSavedObject.attributes.schedule.interval}` - )?.format('x'); - const toInMs = parseScheduleDates(startedAt.toISOString())?.format('x'); - - const results = await getSignals({ - from: fromInMs, - to: toInMs, - size: DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, - index: ruleParams.outputIndex, - ruleId: ruleParams.ruleId, - esClient: services.scopedClusterClient.asCurrentUser, - }); - - const signals = results.hits.hits.map((hit) => hit._source); - - const signalsCount = - typeof results.hits.total === 'number' ? results.hits.total : results.hits.total.value; - - const resultsLink = getNotificationResultsLink({ - from: fromInMs, - to: toInMs, - id: ruleAlertSavedObject.id, - kibanaSiemAppUrl: (ruleAlertParams.meta as { kibana_siem_app_url?: string } | undefined) - ?.kibana_siem_app_url, - }); - - logger.info( - `Found ${signalsCount} signals using signal rule name: "${ruleParams.name}", id: "${params.ruleAlertId}", rule_id: "${ruleParams.ruleId}" in "${ruleParams.outputIndex}" index` - ); - - if (signalsCount !== 0) { - const alertInstance = services.alertInstanceFactory(alertId); - scheduleNotificationActions({ - alertInstance, - signalsCount, - resultsLink, - ruleParams, - signals, - }); - } + async executor() { + // This is an old legacy dead RULE we don't use anymore since we have proper notifications }, }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts index a568bfbc608e4..a6fb747a63d22 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts @@ -21,6 +21,9 @@ export const updateNotifications = async ({ interval, }: UpdateNotificationParams): Promise | null> => { const notification = await readNotifications({ rulesClient, id: undefined, ruleAlertId }); + // console.log('---> my ruleAlertId:', JSON.stringify(ruleAlertId, null, 2)); + // console.log('---> my interval:', JSON.stringify(interval, null, 2)); + // console.log('---> my notification:', JSON.stringify(notification, null, 2)); if (interval && notification) { return rulesClient.update({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts index 447da0f20a657..57ee5c8aea1dd 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts @@ -11,7 +11,10 @@ import { createRuleValidateTypeDependents } from '../../../../../common/detectio import { createRulesBulkSchema } from '../../../../../common/detection_engine/schemas/request/create_rules_bulk_schema'; import { rulesBulkSchema } from '../../../../../common/detection_engine/schemas/response/rules_bulk_schema'; import type { SecuritySolutionPluginRouter } from '../../../../types'; -import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; +import { + DETECTION_ENGINE_RULES_URL, + NOTIFICATION_THROTTLE_NO_ACTIONS, +} from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import { buildMlAuthz } from '../../../machine_learning/authz'; import { throwHttpError } from '../../../machine_learning/validation'; @@ -103,6 +106,11 @@ export const createRulesBulkRoute = ( data: internalRule, }); + // mutes if we are creating the rule with the explicit no actions + if (payloadRule.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.muteAll({ id: createdRule.id }); + } + const ruleActions = await updateRulesNotifications({ ruleAlertId: createdRule.id, rulesClient, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts index b7f32b82cc767..5ccda770692eb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts @@ -8,7 +8,10 @@ import { transformError, getIndexExists } from '@kbn/securitysolution-es-utils'; import { IRuleDataClient } from '../../../../../../rule_registry/server'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; -import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; +import { + DETECTION_ENGINE_RULES_URL, + NOTIFICATION_THROTTLE_NO_ACTIONS, +} from '../../../../../common/constants'; import { SetupPlugins } from '../../../../plugin'; import type { SecuritySolutionPluginRouter } from '../../../../types'; import { buildMlAuthz } from '../../../machine_learning/authz'; @@ -95,6 +98,11 @@ export const createRulesRoute = ( data: internalRule, }); + // mutes if we are creating the rule with the explicit no actions + if (request.body.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.muteAll({ id: createdRule.id }); + } + const ruleActions = await updateRulesNotifications({ ruleAlertId: createdRule.id, rulesClient, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts index 2b9abd2088292..53bebf340c267 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -186,6 +186,7 @@ export const importRulesRoute = ( note, timeline_id: timelineId, timeline_title: timelineTitle, + throttle, version, exceptions_list: exceptionsList, } = parsedRule; @@ -235,6 +236,7 @@ export const importRulesRoute = ( severity, severityMapping, tags, + throttle, to, type, threat, @@ -288,6 +290,7 @@ export const importRulesRoute = ( severityMapping, tags, timestampOverride, + throttle, to, type, threat, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts index d2b3396b64a2c..fb964e4f14b90 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts @@ -168,6 +168,7 @@ export const patchRulesBulkRoute = ( threatQuery, threatMapping, threatLanguage, + throttle, concurrentSearches, itemsPerSearch, timestampOverride, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts index 1efc9c93b08d2..f796b7475a4c6 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts @@ -171,6 +171,7 @@ export const patchRulesRoute = ( threatQuery, threatMapping, threatLanguage, + throttle, concurrentSearches, itemsPerSearch, timestampOverride, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts index 32f7198594bfc..fe11e74bacaf1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts @@ -25,6 +25,7 @@ export const updateOrCreateRuleActionsSavedObject = async ({ actions, throttle, }: UpdateOrCreateRuleActionsSavedObject): Promise => { + // console.log('-----> throttle is:', throttle); const ruleActions = await getRuleActionsSavedObject({ ruleAlertId, savedObjectsClient }); if (ruleActions != null) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts index 376a4a29ed89a..8b1bd5d4cdddc 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts @@ -258,6 +258,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ logger.info(buildRuleMessage(`Found ${createdSignalsCount} signals for notification.`)); if (createdSignalsCount) { + // TODO: Implement the throttle the same way here const alertInstance = services.alertInstanceFactory(alertId); scheduleNotificationActions({ alertInstance, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.mock.ts index f7aae1564bb17..34fb7bf5f8291 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.mock.ts @@ -51,6 +51,7 @@ export const getCreateRulesOptionsMock = (): CreateRulesOptions => ({ threatIndicatorPath: undefined, threshold: undefined, timestampOverride: undefined, + throttle: null, to: 'now', type: 'query', references: ['http://www.example.com'], @@ -103,6 +104,7 @@ export const getCreateMlRulesOptionsMock = (): CreateRulesOptions => ({ itemsPerSearch: undefined, threshold: undefined, timestampOverride: undefined, + throttle: null, to: 'now', type: 'machine_learning', references: ['http://www.example.com'], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.ts index c94cb39572ddc..bc415a0de6961 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/create_rules.ts @@ -11,10 +11,15 @@ import { } from '../../../../common/detection_engine/utils'; import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; import { SanitizedAlert } from '../../../../../alerting/common'; -import { SERVER_APP_ID, SIGNALS_ID } from '../../../../common/constants'; +import { + NOTIFICATION_THROTTLE_NO_ACTIONS, + SERVER_APP_ID, + SIGNALS_ID, +} from '../../../../common/constants'; import { CreateRulesOptions } from './types'; import { addTags } from './add_tags'; import { PartialFilter, RuleTypeParams } from '../types'; +import { transformToAlertThrottle, transformToNotifyWhen } from './utils'; export const createRules = async ({ rulesClient, @@ -59,6 +64,7 @@ export const createRules = async ({ threatMapping, threshold, timestampOverride, + throttle, to, type, references, @@ -67,7 +73,7 @@ export const createRules = async ({ exceptionsList, actions, }: CreateRulesOptions): Promise> => { - return rulesClient.create({ + const rule = await rulesClient.create({ data: { name, tags: addTags(tags, ruleId, immutable), @@ -126,8 +132,15 @@ export const createRules = async ({ schedule: { interval }, enabled, actions: actions.map(transformRuleToAlertAction), - throttle: null, - notifyWhen: null, + throttle: transformToAlertThrottle(throttle), + notifyWhen: transformToNotifyWhen(throttle), }, }); + + // Mute the rule if it is first created with the explicit no actions + if (throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.muteAll({ id: rule.id }); + } + + return rule; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/install_prepacked_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/install_prepacked_rules.ts index 587ce3f002b80..1681ac7f1659f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/install_prepacked_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/install_prepacked_rules.ts @@ -113,6 +113,7 @@ export const installPrepackagedRules = ( threatIndex, threatIndicatorPath, threshold, + throttle: null, // At this time there is no pre-packaged actions timestampOverride, references, note, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts index 98b39e3a5ff27..a8a4fa1e882a1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.mock.ts @@ -50,6 +50,7 @@ export const getPatchRulesOptionsMock = (): PatchRulesOptions => ({ threatQuery: undefined, threatMapping: undefined, threatLanguage: undefined, + throttle: null, concurrentSearches: undefined, itemsPerSearch: undefined, timestampOverride: undefined, @@ -102,6 +103,7 @@ export const getPatchMlRulesOptionsMock = (): PatchRulesOptions => ({ threatQuery: undefined, threatMapping: undefined, threatLanguage: undefined, + throttle: null, concurrentSearches: undefined, itemsPerSearch: undefined, timestampOverride: undefined, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts index 39de70f702bd8..221fb9b3d6492 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts @@ -7,6 +7,7 @@ import { validate } from '@kbn/securitysolution-io-ts-utils'; import { defaults } from 'lodash/fp'; +import { NOTIFICATION_THROTTLE_NO_ACTIONS } from '../../../../common/constants'; import { PartialAlert } from '../../../../../alerting/server'; import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; import { @@ -17,7 +18,14 @@ import { internalRuleUpdate, RuleParams } from '../schemas/rule_schemas'; import { addTags } from './add_tags'; import { enableRule } from './enable_rule'; import { PatchRulesOptions } from './types'; -import { calculateInterval, calculateName, calculateVersion, removeUndefined } from './utils'; +import { + calculateInterval, + calculateName, + calculateVersion, + removeUndefined, + transformToAlertThrottle, + transformToNotifyWhen, +} from './utils'; class PatchError extends Error { public readonly statusCode: number; @@ -27,6 +35,8 @@ class PatchError extends Error { } } +/* eslint complexity: ["error", 21]*/ + export const patchRules = async ({ rulesClient, author, @@ -68,6 +78,7 @@ export const patchRules = async ({ concurrentSearches, itemsPerSearch, timestampOverride, + throttle, to, type, references, @@ -179,8 +190,8 @@ export const patchRules = async ({ const newRule = { tags: addTags(tags ?? rule.tags, rule.params.ruleId, rule.params.immutable), - throttle: null, - notifyWhen: null, + throttle: throttle !== undefined ? transformToAlertThrottle(throttle) : rule.throttle, + notifyWhen: throttle !== undefined ? transformToNotifyWhen(throttle) : rule.notifyWhen, name: calculateName({ updatedName: name, originalName: rule.name }), schedule: { interval: calculateInterval(interval, rule.schedule.interval), @@ -188,6 +199,7 @@ export const patchRules = async ({ actions: actions?.map(transformRuleToAlertAction) ?? rule.actions, params: removeUndefined(nextParams), }; + const [validated, errors] = validate(newRule, internalRuleUpdate); if (errors != null || validated === null) { throw new PatchError(`Applying patch would create invalid rule: ${errors}`, 400); @@ -198,6 +210,17 @@ export const patchRules = async ({ data: validated, }); + // mutes or un-mutes the rule actions depending on the throttle + if (rule.muteAll && throttle !== undefined && throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.unmuteAll({ id: update.id }); + } else if ( + !rule.muteAll && + throttle !== undefined && + throttle === NOTIFICATION_THROTTLE_NO_ACTIONS + ) { + await rulesClient.muteAll({ id: update.id }); + } + if (rule.enabled && enabled === false) { await rulesClient.disable({ id: rule.id }); } else if (!rule.enabled && enabled === true) { 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 31e1ba5201020..bed006d2e2ac4 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 @@ -256,6 +256,7 @@ export interface CreateRulesOptions { concurrentSearches: ConcurrentSearchesOrUndefined; itemsPerSearch: ItemsPerSearchOrUndefined; threatLanguage: ThreatLanguageOrUndefined; + throttle: string | null; // TODO: Do we have better types here? timestampOverride: TimestampOverrideOrUndefined; to: To; type: Type; @@ -315,6 +316,7 @@ export interface PatchRulesOptions { threatQuery: ThreatQueryOrUndefined; threatMapping: ThreatMappingOrUndefined; threatLanguage: ThreatLanguageOrUndefined; + throttle: string | null | undefined; // TODO: Do we have better types here? timestampOverride: TimestampOverrideOrUndefined; to: ToOrUndefined; type: TypeOrUndefined; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts index d60cf1ef016df..fcfab2fda1a8b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_prepacked_rules.ts @@ -125,6 +125,7 @@ export const createPromises = ( references, version, note, + throttle, anomaly_threshold: anomalyThreshold, timeline_id: timelineId, timeline_title: timelineTitle, @@ -188,6 +189,7 @@ export const createPromises = ( timelineTitle, machineLearningJobId, exceptionsList, + throttle, actions: undefined, }); }); 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 7ef2e800c23a4..253c3ba70b105 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 @@ -7,7 +7,10 @@ /* eslint-disable complexity */ -import { DEFAULT_MAX_SIGNALS } from '../../../../common/constants'; +import { + DEFAULT_MAX_SIGNALS, + NOTIFICATION_THROTTLE_NO_ACTIONS, +} from '../../../../common/constants'; import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; import { PartialAlert } from '../../../../../alerting/server'; import { readRules } from './read_rules'; @@ -16,6 +19,7 @@ import { addTags } from './add_tags'; import { typeSpecificSnakeToCamel } from '../schemas/rule_converters'; import { InternalRuleUpdate, RuleParams } from '../schemas/rule_schemas'; import { enableRule } from './enable_rule'; +import { transformToAlertThrottle, transformToNotifyWhen } from './utils'; export const updateRules = async ({ spaceId, @@ -73,12 +77,9 @@ export const updateRules = async ({ ...typeSpecificParams, }, schedule: { interval: ruleUpdate.interval ?? '5m' }, - actions: - ruleUpdate.throttle === 'rule' - ? (ruleUpdate.actions ?? []).map(transformRuleToAlertAction) - : [], - throttle: null, - notifyWhen: null, + actions: ruleUpdate.actions != null ? ruleUpdate.actions.map(transformRuleToAlertAction) : [], + throttle: transformToAlertThrottle(ruleUpdate.throttle), + notifyWhen: transformToNotifyWhen(ruleUpdate.throttle), }; const update = await rulesClient.update({ @@ -86,6 +87,13 @@ export const updateRules = async ({ data: newInternalRule, }); + // mutes or unmutes the rule actions depending on the throttle + if (existingRule.muteAll && ruleUpdate.throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.unmuteAll({ id: update.id }); + } else if (!existingRule.muteAll && ruleUpdate.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.muteAll({ id: update.id }); + } + if (existingRule.enabled && enabled === false) { await rulesClient.disable({ id: existingRule.id }); } else if (!existingRule.enabled && enabled === true) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts index 5f2729f129948..5d91feb71ed76 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts @@ -30,12 +30,14 @@ export const updateRulesNotifications = async ({ name, throttle, }: UpdateRulesNotifications): Promise => { + // console.log('---> updateRulesNotifications throttle:', throttle); const ruleActions = await updateOrCreateRuleActionsSavedObject({ savedObjectsClient, ruleAlertId, actions, throttle, }); + // console.log('---> ruleActions.alertThrottle:', ruleActions.alertThrottle); await updateNotifications({ rulesClient, 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 6e6bb38e46df6..256b1e2a03758 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 @@ -53,6 +53,7 @@ import { EventCategoryOverrideOrUndefined, } from '../../../../common/detection_engine/schemas/common/schemas'; import { PartialFilter } from '../types'; +import { NotifyWhen } from '../schemas/rule_schemas'; export const calculateInterval = ( interval: string | undefined, @@ -167,3 +168,21 @@ export const calculateName = ({ return 'untitled'; } }; + +export const transformToNotifyWhen = (throttle: string | null | undefined): NotifyWhen | null => { + if (throttle == null || throttle === 'no_actions') { + return null; // Although I return null, this does not change the value of the "notifyWhen" and it keeps the current value of "notifyWhen" + } else if (throttle === 'rule') { + return 'onActiveAlert'; + } else { + return 'onThrottleInterval'; + } +}; + +export const transformToAlertThrottle = (throttle: string | null | undefined): string | null => { + if (throttle == null || throttle === 'rule' || throttle === 'no_actions') { + return null; + } else { + return throttle; + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts index 577d52c789857..d8706881bea3b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts @@ -32,6 +32,7 @@ import { SanitizedAlert } from '../../../../../alerting/common'; import { IRuleStatusSOAttributes } from '../rules/types'; import { transformTags } from '../routes/rules/utils'; import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas'; +import { transformToAlertThrottle, transformToNotifyWhen } from '../rules/utils'; // These functions provide conversions from the request API schema to the internal rule schema and from the internal rule schema // to the response API schema. This provides static type-check assurances that the internal schema is in sync with the API schema for @@ -156,9 +157,9 @@ export const convertCreateAPIToInternalSchema = ( }, schedule: { interval: input.interval ?? '5m' }, enabled: input.enabled ?? true, - actions: input.throttle === 'rule' ? (input.actions ?? []).map(transformRuleToAlertAction) : [], - throttle: null, - notifyWhen: null, + actions: input.actions != null ? input.actions.map(transformRuleToAlertAction) : [], + throttle: transformToAlertThrottle(input.throttle), + notifyWhen: transformToNotifyWhen(input.throttle), }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts index 2af481b195a07..f0df617a22004 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts @@ -189,6 +189,15 @@ export type TypeSpecificRuleParams = t.TypeOf; export const ruleParams = t.intersection([baseRuleParams, typeSpecificRuleParams]); export type RuleParams = t.TypeOf; +export const notifyWhen = t.union([ + t.literal('onActionGroupChange'), + t.literal('onActiveAlert'), + t.literal('onThrottleInterval'), + t.null, +]); + +export type NotifyWhen = t.TypeOf; + export const internalRuleCreate = t.type({ name, tags, @@ -201,7 +210,7 @@ export const internalRuleCreate = t.type({ actions: actionsCamel, params: ruleParams, throttle: throttleOrNull, - notifyWhen: t.null, + notifyWhen, }); export type InternalRuleCreate = t.TypeOf; @@ -213,8 +222,8 @@ export const internalRuleUpdate = t.type({ }), actions: actionsCamel, params: ruleParams, - throttle: throttleOrNull, - notifyWhen: t.null, + throttle: throttleOrNull, // "onActionGroupChange" | "onActiveAlert" | "onThrottleInterval" + notifyWhen, }); export type InternalRuleUpdate = t.TypeOf; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index b242691577b89..f0d14d70c635a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -19,6 +19,7 @@ import { SIGNALS_ID, DEFAULT_SEARCH_AFTER_PAGE_SIZE, SERVER_APP_ID, + DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, } from '../../../../common/constants'; import { isMlRule } from '../../../../common/machine_learning/helpers'; import { @@ -72,6 +73,7 @@ import { ExperimentalFeatures } from '../../../../common/experimental_features'; import { injectReferences, extractReferences } from './saved_object_references'; import { RuleExecutionLogClient } from '../rule_execution_log/rule_execution_log_client'; import { IRuleDataPluginService } from '../rule_execution_log/types'; +import { getSignals } from '../notifications/get_signals'; export const signalRulesAlertType = ({ logger, @@ -384,15 +386,47 @@ export const signalRulesAlertType = ({ buildRuleMessage(`Found ${result.createdSignalsCount} signals for notification.`) ); - if (result.createdSignalsCount) { - const alertInstance = services.alertInstanceFactory(alertId); - scheduleNotificationActions({ - alertInstance, - signalsCount: result.createdSignalsCount, - signals: result.createdSignals, - resultsLink, - ruleParams: notificationRuleParams, - }); + if (savedObject.attributes.throttle != null) { + const fromInMsThrottle = parseScheduleDates(`now-${savedObject.attributes.throttle}`); + const toInMsThrottle = parseScheduleDates(startedAt.toISOString()); + if (fromInMsThrottle != null && toInMsThrottle != null) { + const resultsLinkThrottle = getNotificationResultsLink({ + from: fromInMsThrottle.toISOString(), + to: toInMsThrottle.toISOString(), + id: savedObject.id, + kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined) + ?.kibana_siem_app_url, + }); + const results = await getSignals({ + from: `${fromInMsThrottle.valueOf()}`, + to: `${toInMsThrottle.valueOf()}`, + size: DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, + index: outputIndex, + ruleId, + esClient: services.scopedClusterClient.asCurrentUser, + }); + if (results.hits.hits.length !== 0) { + const alertInstance = services.alertInstanceFactory(alertId); + scheduleNotificationActions({ + alertInstance, + signalsCount: result.createdSignalsCount, + signals: result.createdSignals, + resultsLink: resultsLinkThrottle, + ruleParams: notificationRuleParams, + }); + } + } + } else { + if (result.createdSignalsCount) { + const alertInstance = services.alertInstanceFactory(alertId); + scheduleNotificationActions({ + alertInstance, + signalsCount: result.createdSignalsCount, + signals: result.createdSignals, + resultsLink, + ruleParams: notificationRuleParams, + }); + } } } From 0e59773d54ff6c6b228d76eeecb683cabddfc1ea Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Mon, 23 Aug 2021 14:40:29 -0600 Subject: [PATCH 02/10] Removed the side car models for detections --- .../create_notifications.test.ts | 72 ----- .../notifications/create_notifications.ts | 37 --- .../delete_notifications.test.ts | 142 ---------- .../notifications/delete_notifications.ts | 38 --- .../notifications/find_notifications.test.ts | 21 -- .../notifications/find_notifications.ts | 38 --- .../notifications/read_notifications.test.ts | 156 ----------- .../notifications/read_notifications.ts | 49 ---- .../rules_notification_alert_type.test.ts | 247 ------------------ .../rules_notification_alert_type.ts | 37 --- .../schedule_notification_actions.ts | 3 + .../notifications/types.test.ts | 12 +- .../detection_engine/notifications/types.ts | 101 +------ .../update_notifications.test.ts | 133 ---------- .../notifications/update_notifications.ts | 60 ----- .../routes/__mocks__/request_responses.ts | 8 +- .../routes/rules/create_rules_bulk_route.ts | 17 +- .../routes/rules/create_rules_route.test.ts | 8 - .../routes/rules/create_rules_route.ts | 13 +- .../routes/rules/find_rules_route.ts | 18 +- .../routes/rules/patch_rules_bulk_route.ts | 12 +- .../routes/rules/patch_rules_route.ts | 12 +- .../routes/rules/perform_bulk_action_route.ts | 19 +- .../routes/rules/read_rules_route.ts | 8 +- .../routes/rules/update_rules_bulk_route.ts | 12 +- .../routes/rules/update_rules_route.test.ts | 8 - .../routes/rules/update_rules_route.ts | 12 +- .../detection_engine/routes/rules/utils.ts | 2 +- .../detection_engine/routes/rules/validate.ts | 2 +- .../create_rule_actions_saved_object.ts | 38 --- .../delete_rule_actions_saved_object.ts | 27 -- .../get_bulk_rule_actions_saved_object.ts | 40 --- .../get_rule_actions_saved_object.ts | 45 ---- .../rule_actions/saved_object_mappings.ts | 8 + .../detection_engine/rule_actions/types.ts | 1 + ...ate_or_create_rule_actions_saved_object.ts | 42 --- .../update_rule_actions_saved_object.ts | 55 ---- .../create_security_rule_type_factory.ts | 2 +- .../rules/delete_rules.test.ts | 15 +- .../detection_engine/rules/delete_rules.ts | 6 +- .../rules/update_rules_notifications.ts | 52 ---- .../signals/signal_rule_alert_type.test.ts | 8 +- .../security_solution/server/plugin.ts | 9 - 43 files changed, 36 insertions(+), 1609 deletions(-) delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/create_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/delete_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_bulk_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_rule_actions_saved_object.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.test.ts deleted file mode 100644 index 33721c055cb89..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.test.ts +++ /dev/null @@ -1,72 +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 { rulesClientMock } from '../../../../../alerting/server/mocks'; -import { createNotifications } from './create_notifications'; - -describe('createNotifications', () => { - let rulesClient: ReturnType; - - beforeEach(() => { - rulesClient = rulesClientMock.create(); - }); - - it('calls the rulesClient with proper params', async () => { - const ruleAlertId = 'rule-04128c15-0d1b-4716-a4c5-46997ac7f3bd'; - - await createNotifications({ - rulesClient, - actions: [], - ruleAlertId, - enabled: true, - interval: '', - name: '', - }); - - expect(rulesClient.create).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - params: expect.objectContaining({ - ruleAlertId, - }), - }), - }) - ); - }); - - it('calls the rulesClient with transformed actions', async () => { - const action = { - group: 'default', - id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', - params: { message: 'Rule generated {{state.signals_count}} signals' }, - action_type_id: '.slack', - }; - await createNotifications({ - rulesClient, - actions: [action], - ruleAlertId: 'new-rule-id', - enabled: true, - interval: '', - name: '', - }); - - expect(rulesClient.create).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - actions: expect.arrayContaining([ - { - group: action.group, - id: action.id, - params: action.params, - actionTypeId: '.slack', - }, - ]), - }), - }) - ); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.ts deleted file mode 100644 index 907976062b519..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/create_notifications.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { SanitizedAlert } from '../../../../../alerting/common'; -import { SERVER_APP_ID, NOTIFICATIONS_ID } from '../../../../common/constants'; -import { CreateNotificationParams, RuleNotificationAlertTypeParams } from './types'; -import { addTags } from './add_tags'; -import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; - -export const createNotifications = async ({ - rulesClient, - actions, - enabled, - ruleAlertId, - interval, - name, -}: CreateNotificationParams): Promise> => - rulesClient.create({ - data: { - name, - tags: addTags([], ruleAlertId), - alertTypeId: NOTIFICATIONS_ID, - consumer: SERVER_APP_ID, - params: { - ruleAlertId, - }, - schedule: { interval }, - enabled, - actions: actions.map(transformRuleToAlertAction), - throttle: null, - notifyWhen: null, - }, - }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.test.ts deleted file mode 100644 index 9cd01df6bcdf3..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.test.ts +++ /dev/null @@ -1,142 +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 { rulesClientMock } from '../../../../../alerting/server/mocks'; -import { deleteNotifications } from './delete_notifications'; -import { readNotifications } from './read_notifications'; -jest.mock('./read_notifications'); - -describe('deleteNotifications', () => { - let rulesClient: ReturnType; - const notificationId = 'notification-52128c15-0d1b-4716-a4c5-46997ac7f3bd'; - const ruleAlertId = 'rule-04128c15-0d1b-4716-a4c5-46997ac7f3bd'; - - beforeEach(() => { - rulesClient = rulesClientMock.create(); - }); - - it('should return null if notification was not found', async () => { - (readNotifications as jest.Mock).mockResolvedValue(null); - - const result = await deleteNotifications({ - rulesClient, - id: notificationId, - ruleAlertId, - }); - - expect(result).toBe(null); - }); - - it('should call rulesClient.delete if notification was found', async () => { - (readNotifications as jest.Mock).mockResolvedValue({ - id: notificationId, - }); - - const result = await deleteNotifications({ - rulesClient, - id: notificationId, - ruleAlertId, - }); - - expect(rulesClient.delete).toHaveBeenCalledWith( - expect.objectContaining({ - id: notificationId, - }) - ); - expect(result).toEqual({ id: notificationId }); - }); - - it('should call rulesClient.delete if notification.id was null', async () => { - (readNotifications as jest.Mock).mockResolvedValue({ - id: null, - }); - - const result = await deleteNotifications({ - rulesClient, - id: notificationId, - ruleAlertId, - }); - - expect(rulesClient.delete).toHaveBeenCalledWith( - expect.objectContaining({ - id: notificationId, - }) - ); - expect(result).toEqual({ id: null }); - }); - - it('should return null if rulesClient.delete rejects with 404 if notification.id was null', async () => { - (readNotifications as jest.Mock).mockResolvedValue({ - id: null, - }); - - rulesClient.delete.mockRejectedValue({ - output: { - statusCode: 404, - }, - }); - - const result = await deleteNotifications({ - rulesClient, - id: notificationId, - ruleAlertId, - }); - - expect(rulesClient.delete).toHaveBeenCalledWith( - expect.objectContaining({ - id: notificationId, - }) - ); - expect(result).toEqual(null); - }); - - it('should return error object if rulesClient.delete rejects with status different than 404 and if notification.id was null', async () => { - (readNotifications as jest.Mock).mockResolvedValue({ - id: null, - }); - - const errorObject = { - output: { - statusCode: 500, - }, - }; - - rulesClient.delete.mockRejectedValue(errorObject); - - let errorResult; - try { - await deleteNotifications({ - rulesClient, - id: notificationId, - ruleAlertId, - }); - } catch (error) { - errorResult = error; - } - - expect(rulesClient.delete).toHaveBeenCalledWith( - expect.objectContaining({ - id: notificationId, - }) - ); - expect(errorResult).toEqual(errorObject); - }); - - it('should return null if notification.id and id were null', async () => { - (readNotifications as jest.Mock).mockResolvedValue({ - id: null, - }); - - const result = await deleteNotifications({ - rulesClient, - id: undefined, - ruleAlertId, - }); - - expect(result).toEqual(null); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.ts deleted file mode 100644 index cf6812b7cacdc..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/delete_notifications.ts +++ /dev/null @@ -1,38 +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 { readNotifications } from './read_notifications'; -import { DeleteNotificationParams } from './types'; - -export const deleteNotifications = async ({ - rulesClient, - id, - ruleAlertId, -}: DeleteNotificationParams) => { - const notification = await readNotifications({ rulesClient, id, ruleAlertId }); - if (notification == null) { - return null; - } - - if (notification.id != null) { - await rulesClient.delete({ id: notification.id }); - return notification; - } else if (id != null) { - try { - await rulesClient.delete({ id }); - return notification; - } catch (err) { - if (err.output.statusCode === 404) { - return null; - } else { - throw err; - } - } - } else { - return null; - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.test.ts deleted file mode 100644 index 095134b214b57..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.test.ts +++ /dev/null @@ -1,21 +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 { getFilter } from './find_notifications'; -import { NOTIFICATIONS_ID } from '../../../../common/constants'; - -describe('find_notifications', () => { - test('it returns a full filter with an AND if sent down', () => { - expect(getFilter('alert.attributes.enabled: true')).toEqual( - `alert.attributes.alertTypeId: ${NOTIFICATIONS_ID} AND alert.attributes.enabled: true` - ); - }); - - test('it returns existing filter with no AND when not set', () => { - expect(getFilter(null)).toEqual(`alert.attributes.alertTypeId: ${NOTIFICATIONS_ID}`); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.ts deleted file mode 100644 index 1f3d4247a0ad9..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/find_notifications.ts +++ /dev/null @@ -1,38 +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 { AlertTypeParams, FindResult } from '../../../../../alerting/server'; -import { NOTIFICATIONS_ID } from '../../../../common/constants'; -import { FindNotificationParams } from './types'; - -export const getFilter = (filter: string | null | undefined) => { - if (filter == null) { - return `alert.attributes.alertTypeId: ${NOTIFICATIONS_ID}`; - } else { - return `alert.attributes.alertTypeId: ${NOTIFICATIONS_ID} AND ${filter}`; - } -}; - -export const findNotifications = async ({ - rulesClient, - perPage, - page, - fields, - filter, - sortField, - sortOrder, -}: FindNotificationParams): Promise> => - rulesClient.find({ - options: { - fields, - page, - perPage, - filter: getFilter(filter), - sortOrder, - sortField, - }, - }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.test.ts deleted file mode 100644 index 0e87dc76bd1cf..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.test.ts +++ /dev/null @@ -1,156 +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 { readNotifications } from './read_notifications'; -import { rulesClientMock } from '../../../../../alerting/server/mocks'; -import { - getNotificationResult, - getFindNotificationsResultWithSingleHit, -} from '../routes/__mocks__/request_responses'; - -class TestError extends Error { - constructor() { - super(); - - this.name = 'CustomError'; - this.output = { statusCode: 404 }; - } - public output: { statusCode: number }; -} - -describe('read_notifications', () => { - let rulesClient: ReturnType; - - beforeEach(() => { - rulesClient = rulesClientMock.create(); - }); - - describe('readNotifications', () => { - test('should return the output from rulesClient if id is set but ruleAlertId is undefined', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - - const rule = await readNotifications({ - rulesClient, - id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', - ruleAlertId: undefined, - }); - expect(rule).toEqual(getNotificationResult()); - }); - test('should return null if saved object found by alerts client given id is not alert type', async () => { - const result = getNotificationResult(); - // @ts-expect-error - delete result.alertTypeId; - rulesClient.get.mockResolvedValue(result); - - const rule = await readNotifications({ - rulesClient, - id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', - ruleAlertId: undefined, - }); - expect(rule).toEqual(null); - }); - - test('should return error if alerts client throws 404 error on get', async () => { - rulesClient.get.mockImplementation(() => { - throw new TestError(); - }); - - const rule = await readNotifications({ - rulesClient, - id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', - ruleAlertId: undefined, - }); - expect(rule).toEqual(null); - }); - - test('should return error if alerts client throws error on get', async () => { - rulesClient.get.mockImplementation(() => { - throw new Error('Test error'); - }); - try { - await readNotifications({ - rulesClient, - id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', - ruleAlertId: undefined, - }); - } catch (exc) { - expect(exc.message).toEqual('Test error'); - } - }); - - test('should return the output from rulesClient if id is set but ruleAlertId is null', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - - const rule = await readNotifications({ - rulesClient, - id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', - ruleAlertId: null, - }); - expect(rule).toEqual(getNotificationResult()); - }); - - test('should return the output from rulesClient if id is undefined but ruleAlertId is set', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - rulesClient.find.mockResolvedValue(getFindNotificationsResultWithSingleHit()); - - const rule = await readNotifications({ - rulesClient, - id: undefined, - ruleAlertId: 'rule-1', - }); - expect(rule).toEqual(getNotificationResult()); - }); - - test('should return null if the output from rulesClient with ruleAlertId set is empty', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - rulesClient.find.mockResolvedValue({ data: [], page: 0, perPage: 1, total: 0 }); - - const rule = await readNotifications({ - rulesClient, - id: undefined, - ruleAlertId: 'rule-1', - }); - expect(rule).toEqual(null); - }); - - test('should return the output from rulesClient if id is null but ruleAlertId is set', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - rulesClient.find.mockResolvedValue(getFindNotificationsResultWithSingleHit()); - - const rule = await readNotifications({ - rulesClient, - id: null, - ruleAlertId: 'rule-1', - }); - expect(rule).toEqual(getNotificationResult()); - }); - - test('should return null if id and ruleAlertId are null', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - rulesClient.find.mockResolvedValue(getFindNotificationsResultWithSingleHit()); - - const rule = await readNotifications({ - rulesClient, - id: null, - ruleAlertId: null, - }); - expect(rule).toEqual(null); - }); - - test('should return null if id and ruleAlertId are undefined', async () => { - rulesClient.get.mockResolvedValue(getNotificationResult()); - rulesClient.find.mockResolvedValue(getFindNotificationsResultWithSingleHit()); - - const rule = await readNotifications({ - rulesClient, - id: undefined, - ruleAlertId: undefined, - }); - expect(rule).toEqual(null); - }); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.ts deleted file mode 100644 index a31281821d2d7..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/read_notifications.ts +++ /dev/null @@ -1,49 +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 { AlertTypeParams, SanitizedAlert } from '../../../../../alerting/common'; -import { ReadNotificationParams, isAlertType } from './types'; -import { findNotifications } from './find_notifications'; -import { INTERNAL_RULE_ALERT_ID_KEY } from '../../../../common/constants'; - -export const readNotifications = async ({ - rulesClient, - id, - ruleAlertId, -}: ReadNotificationParams): Promise | null> => { - if (id != null) { - try { - const notification = await rulesClient.get({ id }); - if (isAlertType(notification)) { - return notification; - } else { - return null; - } - } catch (err) { - if (err?.output?.statusCode === 404) { - return null; - } else { - // throw non-404 as they would be 500 or other internal errors - throw err; - } - } - } else if (ruleAlertId != null) { - const notificationFromFind = await findNotifications({ - rulesClient, - filter: `alert.attributes.tags: "${INTERNAL_RULE_ALERT_ID_KEY}:${ruleAlertId}"`, - page: 1, - }); - if (notificationFromFind.data.length === 0 || !isAlertType(notificationFromFind.data[0])) { - return null; - } else { - return notificationFromFind.data[0]; - } - } else { - // should never get here, and yet here we are. - return null; - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts deleted file mode 100644 index a820635e30d40..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.test.ts +++ /dev/null @@ -1,247 +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 { loggingSystemMock } from 'src/core/server/mocks'; -import { getAlertMock } from '../routes/__mocks__/request_responses'; -import { rulesNotificationAlertType } from './rules_notification_alert_type'; -import { buildSignalsSearchQuery } from './build_signals_query'; -import { alertsMock, AlertServicesMock } from '../../../../../alerting/server/mocks'; -import { NotificationExecutorOptions } from './types'; -import { - sampleDocSearchResultsNoSortIdNoVersion, - sampleDocSearchResultsWithSortId, - sampleEmptyDocSearchResults, -} from '../signals/__mocks__/es_results'; -import { DEFAULT_RULE_NOTIFICATION_QUERY_SIZE } from '../../../../common/constants'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; -import { getQueryRuleParams } from '../schemas/rule_schemas.mock'; -jest.mock('./build_signals_query'); - -describe('rules_notification_alert_type', () => { - let payload: NotificationExecutorOptions; - let alert: ReturnType; - let logger: ReturnType; - let alertServices: AlertServicesMock; - - beforeEach(() => { - alertServices = alertsMock.createAlertServices(); - logger = loggingSystemMock.createLogger(); - - payload = { - alertId: '1111', - services: alertServices, - params: { ruleAlertId: '2222' }, - state: {}, - spaceId: '', - name: 'name', - tags: [], - startedAt: new Date('2019-12-14T16:40:33.400Z'), - previousStartedAt: new Date('2019-12-13T16:40:33.400Z'), - createdBy: 'elastic', - updatedBy: 'elastic', - rule: { - name: 'name', - tags: [], - consumer: 'foo', - producer: 'foo', - ruleTypeId: 'ruleType', - ruleTypeName: 'Name of rule', - enabled: true, - schedule: { - interval: '1h', - }, - actions: [], - createdBy: 'elastic', - updatedBy: 'elastic', - createdAt: new Date('2019-12-14T16:40:33.400Z'), - updatedAt: new Date('2019-12-14T16:40:33.400Z'), - throttle: null, - notifyWhen: null, - }, - }; - - alert = rulesNotificationAlertType({ - logger, - }); - }); - - describe('executor', () => { - it('throws an error if rule alert was not found', async () => { - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'id', - attributes: {}, - type: 'type', - references: [], - }); - await alert.executor(payload); - expect(logger.error).toHaveBeenCalledWith( - `Saved object for alert ${payload.params.ruleAlertId} was not found` - ); - }); - - it('should call buildSignalsSearchQuery with proper params', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise( - sampleDocSearchResultsWithSortId() - ) - ); - - await alert.executor(payload); - - expect(buildSignalsSearchQuery).toHaveBeenCalledWith( - expect.objectContaining({ - from: '1576255233400', - index: '.siem-signals', - ruleId: 'rule-1', - to: '1576341633400', - size: DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, - }) - ); - }); - - it('should resolve results_link when meta is undefined to use "/app/security"', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - delete ruleAlert.params.meta; - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'rule-id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise( - sampleDocSearchResultsWithSortId() - ) - ); - - await alert.executor(payload); - expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); - - const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; - expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( - 'default', - expect.objectContaining({ - results_link: - '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', - }) - ); - }); - - it('should resolve results_link when meta is an empty object to use "/app/security"', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - ruleAlert.params.meta = {}; - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'rule-id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise( - sampleDocSearchResultsWithSortId() - ) - ); - await alert.executor(payload); - expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); - - const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; - expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( - 'default', - expect.objectContaining({ - results_link: - '/app/security/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', - }) - ); - }); - - it('should resolve results_link to custom kibana link when given one', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - ruleAlert.params.meta = { - kibana_siem_app_url: 'http://localhost', - }; - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'rule-id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise( - sampleDocSearchResultsWithSortId() - ) - ); - await alert.executor(payload); - expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); - - const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; - expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( - 'default', - expect.objectContaining({ - results_link: - 'http://localhost/detections/rules/id/rule-id?timerange=(global:(linkTo:!(timeline),timerange:(from:1576255233400,kind:absolute,to:1576341633400)),timeline:(linkTo:!(global),timerange:(from:1576255233400,kind:absolute,to:1576341633400)))', - }) - ); - }); - - it('should not call alertInstanceFactory if signalsCount was 0', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise(sampleEmptyDocSearchResults()) - ); - - await alert.executor(payload); - - expect(alertServices.alertInstanceFactory).not.toHaveBeenCalled(); - }); - - it('should call scheduleActions if signalsCount was greater than 0', async () => { - const ruleAlert = getAlertMock(getQueryRuleParams()); - alertServices.savedObjectsClient.get.mockResolvedValue({ - id: 'id', - type: 'type', - references: [], - attributes: ruleAlert, - }); - alertServices.scopedClusterClient.asCurrentUser.search.mockResolvedValue( - elasticsearchClientMock.createSuccessTransportRequestPromise( - sampleDocSearchResultsNoSortIdNoVersion() - ) - ); - - await alert.executor(payload); - - expect(alertServices.alertInstanceFactory).toHaveBeenCalled(); - - const [{ value: alertInstanceMock }] = alertServices.alertInstanceFactory.mock.results; - expect(alertInstanceMock.replaceState).toHaveBeenCalledWith( - expect.objectContaining({ signals_count: 100 }) - ); - expect(alertInstanceMock.scheduleActions).toHaveBeenCalledWith( - 'default', - expect.objectContaining({ - rule: expect.objectContaining({ - name: ruleAlert.name, - }), - }) - ); - }); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts deleted file mode 100644 index 78a47bb3ed4eb..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/rules_notification_alert_type.ts +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { Logger } from 'src/core/server'; -import { schema } from '@kbn/config-schema'; -import { NOTIFICATIONS_ID, SERVER_APP_ID } from '../../../../common/constants'; - -import { NotificationAlertTypeDefinition } from './types'; -import { siemRuleActionGroups } from '../signals/siem_rule_action_groups'; - -// TODO: Can we remove this function/rule or could that cause issues? -// Do we have to find all rule instances and then delete it on startup and then remove it here? -export const rulesNotificationAlertType = ({ - logger, -}: { - logger: Logger; -}): NotificationAlertTypeDefinition => ({ - id: NOTIFICATIONS_ID, - name: 'SIEM notification', - actionGroups: siemRuleActionGroups, - defaultActionGroupId: 'default', - producer: SERVER_APP_ID, - validate: { - params: schema.object({ - ruleAlertId: schema.string(), - }), - }, - minimumLicenseRequired: 'basic', - isExportable: false, - async executor() { - // This is an old legacy dead RULE we don't use anymore since we have proper notifications - }, -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts index 147c54cd6eb8a..ea8c33ce60d01 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts @@ -22,6 +22,9 @@ interface ScheduleNotificationActions { signals: unknown[]; } +/** + * + */ export const scheduleNotificationActions = ({ alertInstance, signalsCount, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts index a8678c664f331..209e6379291ed 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts @@ -5,10 +5,8 @@ * 2.0. */ -import { loggingSystemMock } from 'src/core/server/mocks'; import { getNotificationResult, getAlertMock } from '../routes/__mocks__/request_responses'; -import { isAlertTypes, isNotificationAlertExecutor } from './types'; -import { rulesNotificationAlertType } from './rules_notification_alert_type'; +import { isAlertTypes } from './types'; import { getQueryRuleParams } from '../schemas/rule_schemas.mock'; describe('types', () => { @@ -19,12 +17,4 @@ describe('types', () => { it('isAlertTypes should return false if is not RuleNotificationAlertType', () => { expect(isAlertTypes([getAlertMock(getQueryRuleParams())])).toEqual(false); }); - - it('isNotificationAlertExecutor should return true it passed object is NotificationAlertTypeDefinition type', () => { - expect( - isNotificationAlertExecutor( - rulesNotificationAlertType({ logger: loggingSystemMock.createLogger() }) - ) - ).toEqual(true); - }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts index fb3eb715368e4..08fef8f561ee3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts @@ -5,79 +5,19 @@ * 2.0. */ -import { - RulesClient, - PartialAlert, - AlertType, - AlertTypeParams, - AlertTypeState, - AlertInstanceState, - AlertInstanceContext, - AlertExecutorOptions, -} from '../../../../../alerting/server'; +import { RulesClient, PartialAlert, AlertTypeParams } from '../../../../../alerting/server'; import { Alert } from '../../../../../alerting/common'; import { NOTIFICATIONS_ID } from '../../../../common/constants'; -import { RuleAlertAction } from '../../../../common/detection_engine/types'; export interface RuleNotificationAlertTypeParams extends AlertTypeParams { ruleAlertId: string; } export type RuleNotificationAlertType = Alert; -export interface FindNotificationParams { - rulesClient: RulesClient; - perPage?: number; - page?: number; - sortField?: string; - filter?: string; - fields?: string[]; - sortOrder?: 'asc' | 'desc'; -} - -export interface FindNotificationsRequestParams { - per_page: number; - page: number; - search?: string; - sort_field?: string; - filter?: string; - fields?: string[]; - sort_order?: 'asc' | 'desc'; -} - export interface Clients { rulesClient: RulesClient; } -export type UpdateNotificationParams = Omit< - NotificationAlertParams, - 'interval' | 'actions' | 'tags' -> & { - actions: RuleAlertAction[]; - interval: string | null | undefined; - ruleAlertId: string; -} & Clients; - -export type DeleteNotificationParams = Clients & { - id?: string; - ruleAlertId?: string; -}; - -export interface NotificationAlertParams { - actions: RuleAlertAction[]; - enabled: boolean; - ruleAlertId: string; - interval: string; - name: string; -} - -export type CreateNotificationParams = NotificationAlertParams & Clients; - -export interface ReadNotificationParams { - rulesClient: RulesClient; - id?: string | null; - ruleAlertId?: string | null; -} - export const isAlertTypes = ( partialAlert: Array> ): partialAlert is RuleNotificationAlertType[] => { @@ -89,42 +29,3 @@ export const isAlertType = ( ): partialAlert is RuleNotificationAlertType => { return partialAlert.alertTypeId === NOTIFICATIONS_ID; }; - -export type NotificationExecutorOptions = AlertExecutorOptions< - RuleNotificationAlertTypeParams, - AlertTypeState, - AlertInstanceState, - AlertInstanceContext ->; - -// This returns true because by default a NotificationAlertTypeDefinition is an AlertType -// since we are only increasing the strictness of params. -export const isNotificationAlertExecutor = ( - obj: NotificationAlertTypeDefinition -): obj is AlertType< - AlertTypeParams, - AlertTypeParams, - AlertTypeState, - AlertInstanceState, - AlertInstanceContext -> => { - return true; -}; - -export type NotificationAlertTypeDefinition = Omit< - AlertType< - AlertTypeParams, - AlertTypeParams, - AlertTypeState, - AlertInstanceState, - AlertInstanceContext, - 'default' - >, - 'executor' -> & { - executor: ({ - services, - params, - state, - }: NotificationExecutorOptions) => Promise; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.test.ts deleted file mode 100644 index a2a858b552c0d..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.test.ts +++ /dev/null @@ -1,133 +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 { rulesClientMock } from '../../../../../alerting/server/mocks'; -import { updateNotifications } from './update_notifications'; -import { readNotifications } from './read_notifications'; -import { createNotifications } from './create_notifications'; -import { getNotificationResult } from '../routes/__mocks__/request_responses'; -import { UpdateNotificationParams } from './types'; -jest.mock('./read_notifications'); -jest.mock('./create_notifications'); - -describe('updateNotifications', () => { - const notification = getNotificationResult(); - let rulesClient: ReturnType; - - beforeEach(() => { - rulesClient = rulesClientMock.create(); - }); - - it('should update the existing notification if interval provided', async () => { - (readNotifications as jest.Mock).mockResolvedValue(notification); - - await updateNotifications({ - rulesClient, - actions: [], - ruleAlertId: 'new-rule-id', - enabled: true, - interval: '10m', - name: '', - }); - - expect(rulesClient.update).toHaveBeenCalledWith( - expect.objectContaining({ - id: notification.id, - data: expect.objectContaining({ - params: expect.objectContaining({ - ruleAlertId: 'new-rule-id', - }), - }), - }) - ); - }); - - it('should create a new notification if did not exist', async () => { - (readNotifications as jest.Mock).mockResolvedValue(null); - - const params: UpdateNotificationParams = { - rulesClient, - actions: [], - ruleAlertId: 'new-rule-id', - enabled: true, - interval: '10m', - name: '', - }; - - await updateNotifications(params); - - expect(createNotifications).toHaveBeenCalledWith(expect.objectContaining(params)); - }); - - it('should delete notification if notification was found and interval is null', async () => { - (readNotifications as jest.Mock).mockResolvedValue(notification); - - await updateNotifications({ - rulesClient, - actions: [], - ruleAlertId: 'new-rule-id', - enabled: true, - interval: null, - name: '', - }); - - expect(rulesClient.delete).toHaveBeenCalledWith( - expect.objectContaining({ - id: notification.id, - }) - ); - }); - - it('should call the rulesClient with transformed actions', async () => { - (readNotifications as jest.Mock).mockResolvedValue(notification); - const action = { - group: 'default', - id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', - params: { message: 'Rule generated {{state.signals_count}} signals' }, - action_type_id: '.slack', - }; - await updateNotifications({ - rulesClient, - actions: [action], - ruleAlertId: 'new-rule-id', - enabled: true, - interval: '10m', - name: '', - }); - - expect(rulesClient.update).toHaveBeenCalledWith( - expect.objectContaining({ - data: expect.objectContaining({ - actions: expect.arrayContaining([ - { - group: action.group, - id: action.id, - params: action.params, - actionTypeId: '.slack', - }, - ]), - }), - }) - ); - }); - - it('returns null if notification was not found and interval was null', async () => { - (readNotifications as jest.Mock).mockResolvedValue(null); - const ruleAlertId = 'rule-04128c15-0d1b-4716-a4c5-46997ac7f3bd'; - - const result = await updateNotifications({ - rulesClient, - actions: [], - enabled: true, - ruleAlertId, - name: notification.name, - interval: null, - }); - - expect(result).toEqual(null); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts deleted file mode 100644 index a6fb747a63d22..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/update_notifications.ts +++ /dev/null @@ -1,60 +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 { PartialAlert } from '../../../../../alerting/server'; -import { readNotifications } from './read_notifications'; -import { RuleNotificationAlertTypeParams, UpdateNotificationParams } from './types'; -import { addTags } from './add_tags'; -import { createNotifications } from './create_notifications'; -import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; - -export const updateNotifications = async ({ - rulesClient, - actions, - enabled, - ruleAlertId, - name, - interval, -}: UpdateNotificationParams): Promise | null> => { - const notification = await readNotifications({ rulesClient, id: undefined, ruleAlertId }); - // console.log('---> my ruleAlertId:', JSON.stringify(ruleAlertId, null, 2)); - // console.log('---> my interval:', JSON.stringify(interval, null, 2)); - // console.log('---> my notification:', JSON.stringify(notification, null, 2)); - - if (interval && notification) { - return rulesClient.update({ - id: notification.id, - data: { - tags: addTags([], ruleAlertId), - name, - schedule: { - interval, - }, - actions: actions.map(transformRuleToAlertAction), - params: { - ruleAlertId, - }, - throttle: null, - notifyWhen: null, - }, - }); - } else if (interval && !notification) { - return createNotifications({ - rulesClient, - enabled, - name, - interval, - actions, - ruleAlertId, - }); - } else if (!interval && notification) { - await rulesClient.delete({ id: notification.id }); - return null; - } else { - return null; - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts index 2f395117e8a0b..b628ebc85bc17 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -576,6 +576,7 @@ export const getSuccessfulSignalUpdateResponse = () => ({ failures: [], }); +// TODO: Should this be deleted? export const getNotificationResult = (): RuleNotificationAlertType => ({ id: '200dbf2f-b269-4bf9-aa85-11ba32ba73ba', name: 'Notification for Rule Test', @@ -617,13 +618,6 @@ export const getNotificationResult = (): RuleNotificationAlertType => ({ }, }); -export const getFindNotificationsResultWithSingleHit = (): FindHit => ({ - page: 1, - perPage: 1, - total: 1, - data: [getNotificationResult()], -}); - export const getFinalizeSignalsMigrationRequest = () => requestMock.create({ method: 'post', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts index 57ee5c8aea1dd..d4faed92eaf41 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts @@ -24,7 +24,6 @@ import { transformValidateBulkError } from './validate'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { transformBulkError, createBulkErrorObject, buildSiemResponse } from '../utils'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { convertCreateAPIToInternalSchema } from '../../schemas/rule_converters'; export const createRulesBulkRoute = ( @@ -111,21 +110,7 @@ export const createRulesBulkRoute = ( await rulesClient.muteAll({ id: createdRule.id }); } - const ruleActions = await updateRulesNotifications({ - ruleAlertId: createdRule.id, - rulesClient, - savedObjectsClient, - enabled: createdRule.enabled, - actions: payloadRule.actions, - throttle: payloadRule.throttle ?? null, - name: createdRule.name, - }); - - return transformValidateBulkError( - internalRule.params.ruleId, - createdRule, - ruleActions - ); + return transformValidateBulkError(internalRule.params.ruleId, createdRule, undefined); } catch (err) { return transformBulkError(internalRule.params.ruleId, err); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts index 18767af066d27..fc48e34a7ca74 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.test.ts @@ -18,12 +18,10 @@ import { mlServicesMock, mlAuthzMock as mockMlAuthzFactory } from '../../../mach import { buildMlAuthz } from '../../../machine_learning/authz'; import { requestContextMock, serverMock, requestMock } from '../__mocks__'; import { createRulesRoute } from './create_rules_route'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; import { getQueryRuleParams } from '../../schemas/rule_schemas.mock'; -jest.mock('../../rules/update_rules_notifications'); jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create()); describe('create_rules', () => { @@ -48,12 +46,6 @@ describe('create_rules', () => { describe('status codes with actionClient and alertClient', () => { test('returns 200 when creating a single rule with a valid actionClient and alertClient', async () => { - (updateRulesNotifications as jest.Mock).mockResolvedValue({ - id: 'id', - actions: [], - alertThrottle: null, - ruleThrottle: 'no_actions', - }); const response = await server.inject(getCreateRequest(), context); expect(response.status).toEqual(200); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts index 5ccda770692eb..f45e4df800a82 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts @@ -19,7 +19,6 @@ import { throwHttpError } from '../../../machine_learning/validation'; import { readRules } from '../../rules/read_rules'; import { buildSiemResponse } from '../utils'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { createRulesSchema } from '../../../../../common/detection_engine/schemas/request'; import { newTransformValidate } from './validate'; import { createRuleValidateTypeDependents } from '../../../../../common/detection_engine/schemas/request/create_rules_type_dependents'; @@ -103,22 +102,12 @@ export const createRulesRoute = ( await rulesClient.muteAll({ id: createdRule.id }); } - const ruleActions = await updateRulesNotifications({ - ruleAlertId: createdRule.id, - rulesClient, - savedObjectsClient, - enabled: createdRule.enabled, - actions: request.body.actions, - throttle: request.body.throttle ?? null, - name: createdRule.name, - }); - const ruleStatuses = await context.securitySolution.getExecutionLogClient().find({ logsCount: 1, ruleId: createdRule.id, spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = newTransformValidate(createdRule, ruleActions, ruleStatuses[0]); + const [validated, errors] = newTransformValidate(createdRule, undefined, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts index 4a464c19f5b97..1ca3b73829038 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts @@ -18,7 +18,6 @@ import { findRules } from '../../rules/find_rules'; import { buildSiemResponse } from '../utils'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; import { transformFindAlerts } from './utils'; -import { getBulkRuleActionsSavedObject } from '../../rule_actions/get_bulk_rule_actions_saved_object'; export const findRulesRoute = ( router: SecuritySolutionPluginRouter, @@ -46,7 +45,6 @@ export const findRulesRoute = ( try { const { query } = request; const rulesClient = context.alerting?.getRulesClient(); - const savedObjectsClient = context.core.savedObjects.client; if (!rulesClient) { return siemResponse.error({ statusCode: 404 }); @@ -64,15 +62,13 @@ export const findRulesRoute = ( }); const alertIds = rules.data.map((rule) => rule.id); - const [ruleStatuses, ruleActions] = await Promise.all([ - execLogClient.findBulk({ - ruleIds: alertIds, - logsCount: 1, - spaceId: context.securitySolution.getSpaceId(), - }), - getBulkRuleActionsSavedObject({ alertIds, savedObjectsClient }), - ]); - const transformed = transformFindAlerts(rules, ruleActions, ruleStatuses); + const ruleStatuses = await execLogClient.findBulk({ + ruleIds: alertIds, + logsCount: 1, + spaceId: context.securitySolution.getSpaceId(), + }); + // TODO: Can we remove the `{}` and the parameter below? + const transformed = transformFindAlerts(rules, {}, ruleStatuses); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts index fb964e4f14b90..9629e978e60f1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts @@ -22,7 +22,6 @@ import { transformBulkError, buildSiemResponse } from '../utils'; import { getIdBulkError } from './utils'; import { transformValidateBulkError } from './validate'; import { patchRules } from '../../rules/patch_rules'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { readRules } from '../../rules/read_rules'; import { PartialFilter } from '../../types'; @@ -181,21 +180,12 @@ export const patchRulesBulkRoute = ( exceptionsList, }); if (rule != null && rule.enabled != null && rule.name != null) { - const ruleActions = await updateRulesNotifications({ - ruleAlertId: rule.id, - rulesClient, - savedObjectsClient, - enabled: rule.enabled, - actions, - throttle, - name: rule.name, - }); const ruleStatuses = await ruleStatusClient.find({ logsCount: 1, ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - return transformValidateBulkError(rule.id, rule, ruleActions, ruleStatuses); + return transformValidateBulkError(rule.id, rule, undefined, ruleStatuses); } else { return getIdBulkError({ id, ruleId }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts index f796b7475a4c6..5a80ecb2a1aba 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts @@ -24,7 +24,6 @@ import { buildSiemResponse } from '../utils'; import { getIdError } from './utils'; import { transformValidate } from './validate'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { readRules } from '../../rules/read_rules'; import { PartialFilter } from '../../types'; @@ -184,22 +183,13 @@ export const patchRulesRoute = ( exceptionsList, }); if (rule != null && rule.enabled != null && rule.name != null) { - const ruleActions = await updateRulesNotifications({ - ruleAlertId: rule.id, - rulesClient, - savedObjectsClient, - enabled: rule.enabled, - actions, - throttle, - name: rule.name, - }); const ruleStatuses = await ruleStatusClient.find({ logsCount: 1, ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = transformValidate(rule, ruleActions, ruleStatuses[0]); + const [validated, errors] = transformValidate(rule, undefined, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts index 0c4bdf0fcf64f..23b167ae351c4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts @@ -19,8 +19,6 @@ import { duplicateRule } from '../../rules/duplicate_rule'; import { enableRule } from '../../rules/enable_rule'; import { findRules } from '../../rules/find_rules'; import { getExportByObjectIds } from '../../rules/get_export_by_object_ids'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; -import { getRuleActionsSavedObject } from '../../rule_actions/get_rule_actions_saved_object'; import { buildSiemResponse } from '../utils'; const BULK_ACTION_RULES_LIMIT = 10000; @@ -125,24 +123,9 @@ export const performBulkActionRoute = ( rules.data.map(async (rule) => { throwHttpError(await mlAuthz.validateRuleType(rule.params.type)); - const createdRule = await rulesClient.create({ + await rulesClient.create({ data: duplicateRule(rule), }); - - const ruleActions = await getRuleActionsSavedObject({ - savedObjectsClient, - ruleAlertId: rule.id, - }); - - await updateRulesNotifications({ - ruleAlertId: createdRule.id, - rulesClient, - savedObjectsClient, - enabled: createdRule.enabled, - actions: ruleActions?.actions || [], - throttle: ruleActions?.alertThrottle, - name: createdRule.name, - }); }) ); break; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts index 6d5e63b2a0588..d468a2863a4de 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts @@ -19,7 +19,6 @@ import { getIdError, transform } from './utils'; import { buildSiemResponse } from '../utils'; import { readRules } from '../../rules/read_rules'; -import { getRuleActionsSavedObject } from '../../rule_actions/get_rule_actions_saved_object'; import { RuleExecutionStatus } from '../../../../../common/detection_engine/schemas/common/schemas'; export const readRulesRoute = ( @@ -48,7 +47,6 @@ export const readRulesRoute = ( const { id, rule_id: ruleId } = request.query; const rulesClient = context.alerting?.getRulesClient(); - const savedObjectsClient = context.core.savedObjects.client; try { if (!rulesClient) { @@ -62,10 +60,6 @@ export const readRulesRoute = ( ruleId, }); if (rule != null) { - const ruleActions = await getRuleActionsSavedObject({ - savedObjectsClient, - ruleAlertId: rule.id, - }); const ruleStatuses = await ruleStatusClient.find({ logsCount: 1, ruleId: rule.id, @@ -78,7 +72,7 @@ export const readRulesRoute = ( currentStatus.attributes.statusDate = rule.executionStatus.lastExecutionDate.toISOString(); currentStatus.attributes.status = RuleExecutionStatus.failed; } - const transformed = transform(rule, ruleActions, currentStatus); + const transformed = transform(rule, undefined, currentStatus); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { 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 44c9ce51b7a1e..9c91f14dc5c98 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 @@ -19,7 +19,6 @@ import { getIdBulkError } from './utils'; import { transformValidateBulkError } from './validate'; import { transformBulkError, buildSiemResponse, createBulkErrorObject } from '../utils'; import { updateRules } from '../../rules/update_rules'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; export const updateRulesBulkRoute = ( router: SecuritySolutionPluginRouter, @@ -77,21 +76,12 @@ export const updateRulesBulkRoute = ( ruleUpdate: payloadRule, }); if (rule != null) { - const ruleActions = await updateRulesNotifications({ - ruleAlertId: rule.id, - rulesClient, - savedObjectsClient, - enabled: payloadRule.enabled ?? true, - actions: payloadRule.actions, - throttle: payloadRule.throttle, - name: payloadRule.name, - }); const ruleStatuses = await ruleStatusClient.find({ logsCount: 1, ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - return transformValidateBulkError(rule.id, rule, ruleActions, ruleStatuses); + return transformValidateBulkError(rule.id, rule, undefined, ruleStatuses); } else { return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts index 129e4bd8ad9a1..db0054088137c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/update_rules_route.test.ts @@ -17,13 +17,11 @@ import { } from '../__mocks__/request_responses'; import { requestContextMock, serverMock, requestMock } from '../__mocks__'; import { DETECTION_ENGINE_RULES_URL } from '../../../../../common/constants'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { updateRulesRoute } from './update_rules_route'; import { getUpdateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock'; import { getQueryRuleParams } from '../../schemas/rule_schemas.mock'; jest.mock('../../../machine_learning/authz', () => mockMlAuthzFactory.create()); -jest.mock('../../rules/update_rules_notifications'); describe('update_rules', () => { let server: ReturnType; @@ -45,12 +43,6 @@ describe('update_rules', () => { describe('status codes with actionClient and alertClient', () => { test('returns 200 when updating a single rule with a valid actionClient and alertClient', async () => { - (updateRulesNotifications as jest.Mock).mockResolvedValue({ - id: 'id', - actions: [], - alertThrottle: null, - ruleThrottle: 'no_actions', - }); const response = await server.inject(getUpdateRequest(), context); expect(response.status).toEqual(200); }); 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 368b02fdb1e94..e3b2290715d96 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 @@ -19,7 +19,6 @@ import { buildSiemResponse } from '../utils'; import { getIdError } from './utils'; import { transformValidate } from './validate'; import { updateRules } from '../../rules/update_rules'; -import { updateRulesNotifications } from '../../rules/update_rules_notifications'; import { buildRouteValidation } from '../../../../utils/build_validation/route_validation'; export const updateRulesRoute = ( @@ -70,21 +69,12 @@ export const updateRulesRoute = ( }); if (rule != null) { - const ruleActions = await updateRulesNotifications({ - ruleAlertId: rule.id, - rulesClient, - savedObjectsClient, - enabled: request.body.enabled ?? true, - actions: request.body.actions ?? [], - throttle: request.body.throttle ?? 'no_actions', - name: request.body.name, - }); const ruleStatuses = await ruleStatusClient.find({ logsCount: 1, ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = transformValidate(rule, ruleActions, ruleStatuses[0]); + const [validated, errors] = transformValidate(rule, undefined, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index dc0cd2e497215..082269b7b3b87 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -116,7 +116,7 @@ export const transformAlertsToRules = (alerts: RuleAlertType[]): Array, - ruleActions: { [key: string]: RuleActions | undefined }, + ruleActions: { [key: string]: RuleActions | undefined }, // TODO: Can we remove this and just use the findResults now? ruleStatuses: { [key: string]: IRuleStatusSOAttributes[] | undefined } ): { page: number; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts index e3e2b8cda98b2..64dd36372b4e1 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts @@ -57,7 +57,7 @@ export const newTransformValidate = ( export const transformValidateBulkError = ( ruleId: string, alert: PartialAlert, - ruleActions?: RuleActions | null, + ruleActions?: RuleActions | null, // TODO: Can we remove this now? I don't think anyone is using it anymore. ruleStatus?: Array> ): RulesSchema | BulkError => { if (isAlertType(alert)) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/create_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/create_rule_actions_saved_object.ts deleted file mode 100644 index 14498fa41d4b2..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/create_rule_actions_saved_object.ts +++ /dev/null @@ -1,38 +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 { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { AlertServices } from '../../../../../alerting/server'; -import { ruleActionsSavedObjectType } from './saved_object_mappings'; -import { IRuleActionsAttributesSavedObjectAttributes } from './types'; -import { getThrottleOptions, getRuleActionsFromSavedObject } from './utils'; -import { RulesActionsSavedObject } from './get_rule_actions_saved_object'; - -interface CreateRuleActionsSavedObject { - ruleAlertId: string; - savedObjectsClient: AlertServices['savedObjectsClient']; - actions: RuleAlertAction[] | undefined; - throttle: string | null | undefined; -} - -export const createRuleActionsSavedObject = async ({ - ruleAlertId, - savedObjectsClient, - actions = [], - throttle, -}: CreateRuleActionsSavedObject): Promise => { - const ruleActionsSavedObject = await savedObjectsClient.create( - ruleActionsSavedObjectType, - { - ruleAlertId, - actions, - ...getThrottleOptions(throttle), - } - ); - - return getRuleActionsFromSavedObject(ruleActionsSavedObject); -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/delete_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/delete_rule_actions_saved_object.ts deleted file mode 100644 index 8ef2b8ffb72ae..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/delete_rule_actions_saved_object.ts +++ /dev/null @@ -1,27 +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 { AlertServices } from '../../../../../alerting/server'; -import { ruleActionsSavedObjectType } from './saved_object_mappings'; -import { getRuleActionsSavedObject } from './get_rule_actions_saved_object'; - -interface DeleteRuleActionsSavedObject { - ruleAlertId: string; - savedObjectsClient: AlertServices['savedObjectsClient']; -} - -export const deleteRuleActionsSavedObject = async ({ - ruleAlertId, - savedObjectsClient, -}: DeleteRuleActionsSavedObject): Promise<{} | null> => { - const ruleActions = await getRuleActionsSavedObject({ ruleAlertId, savedObjectsClient }); - if (ruleActions != null) { - return savedObjectsClient.delete(ruleActionsSavedObjectType, ruleActions.id); - } else { - return null; - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_bulk_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_bulk_rule_actions_saved_object.ts deleted file mode 100644 index 1abb16ba4612c..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_bulk_rule_actions_saved_object.ts +++ /dev/null @@ -1,40 +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 { AlertServices } from '../../../../../alerting/server'; -import { ruleActionsSavedObjectType } from './saved_object_mappings'; -import { IRuleActionsAttributesSavedObjectAttributes } from './types'; -import { getRuleActionsFromSavedObject } from './utils'; -import { RulesActionsSavedObject } from './get_rule_actions_saved_object'; -import { buildChunkedOrFilter } from '../signals/utils'; - -interface GetBulkRuleActionsSavedObject { - alertIds: string[]; - savedObjectsClient: AlertServices['savedObjectsClient']; -} - -export const getBulkRuleActionsSavedObject = async ({ - alertIds, - savedObjectsClient, -}: GetBulkRuleActionsSavedObject): Promise> => { - const filter = buildChunkedOrFilter( - `${ruleActionsSavedObjectType}.attributes.ruleAlertId`, - alertIds - ); - const { - // eslint-disable-next-line @typescript-eslint/naming-convention - saved_objects, - } = await savedObjectsClient.find({ - type: ruleActionsSavedObjectType, - perPage: 10000, - filter, - }); - return saved_objects.reduce((acc: { [key: string]: RulesActionsSavedObject }, savedObject) => { - acc[savedObject.attributes.ruleAlertId] = getRuleActionsFromSavedObject(savedObject); - return acc; - }, {}); -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_rule_actions_saved_object.ts deleted file mode 100644 index aa15617aab4ca..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/get_rule_actions_saved_object.ts +++ /dev/null @@ -1,45 +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 { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { AlertServices } from '../../../../../alerting/server'; -import { ruleActionsSavedObjectType } from './saved_object_mappings'; -import { IRuleActionsAttributesSavedObjectAttributes } from './types'; -import { getRuleActionsFromSavedObject } from './utils'; - -interface GetRuleActionsSavedObject { - ruleAlertId: string; - savedObjectsClient: AlertServices['savedObjectsClient']; -} - -export interface RulesActionsSavedObject { - id: string; - actions: RuleAlertAction[]; - alertThrottle: string | null; - ruleThrottle: string; -} - -export const getRuleActionsSavedObject = async ({ - ruleAlertId, - savedObjectsClient, -}: GetRuleActionsSavedObject): Promise => { - const { - // eslint-disable-next-line @typescript-eslint/naming-convention - saved_objects, - } = await savedObjectsClient.find({ - type: ruleActionsSavedObjectType, - perPage: 1, - search: `${ruleAlertId}`, - searchFields: ['ruleAlertId'], - }); - - if (!saved_objects[0]) { - return null; - } else { - return getRuleActionsFromSavedObject(saved_objects[0]); - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts index 7b135ae2efd06..637d1426e95f8 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts @@ -8,8 +8,16 @@ import { SavedObjectsType } from '../../../../../../../src/core/server'; import { ruleActionsSavedObjectMigration } from './migrations'; +// TODO: Can we delete this mapping now or do we have to wait for the migration for the delete to remove it? +/** + * @deprecated + */ export const ruleActionsSavedObjectType = 'siem-detection-engine-rule-actions'; +// TODO: Can we delete this mapping now or do we have to wait for the migration for the delete to remove it? +/** + * @deprecated + */ export const ruleActionsSavedObjectMappings: SavedObjectsType['mappings'] = { properties: { alertThrottle: { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts index 97b19e4367afa..a53546a3e63a4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts @@ -19,6 +19,7 @@ export interface IRuleActionsAttributes extends Record { alertThrottle: string | null; } +// TODO: Can this be removed and anything using it be removed as well? export interface RuleActions { id: string; actions: RuleAlertAction[]; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts deleted file mode 100644 index fe11e74bacaf1..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_or_create_rule_actions_saved_object.ts +++ /dev/null @@ -1,42 +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 { AlertServices } from '../../../../../alerting/server'; -import { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { getRuleActionsSavedObject } from './get_rule_actions_saved_object'; -import { createRuleActionsSavedObject } from './create_rule_actions_saved_object'; -import { updateRuleActionsSavedObject } from './update_rule_actions_saved_object'; -import { RuleActions } from './types'; - -interface UpdateOrCreateRuleActionsSavedObject { - ruleAlertId: string; - savedObjectsClient: AlertServices['savedObjectsClient']; - actions: RuleAlertAction[] | undefined; - throttle: string | null | undefined; -} - -export const updateOrCreateRuleActionsSavedObject = async ({ - savedObjectsClient, - ruleAlertId, - actions, - throttle, -}: UpdateOrCreateRuleActionsSavedObject): Promise => { - // console.log('-----> throttle is:', throttle); - const ruleActions = await getRuleActionsSavedObject({ ruleAlertId, savedObjectsClient }); - - if (ruleActions != null) { - return updateRuleActionsSavedObject({ - ruleAlertId, - savedObjectsClient, - actions, - throttle, - ruleActions, - }); - } else { - return createRuleActionsSavedObject({ ruleAlertId, savedObjectsClient, actions, throttle }); - } -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_rule_actions_saved_object.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_rule_actions_saved_object.ts deleted file mode 100644 index 98767f24b5bb4..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/update_rule_actions_saved_object.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 { AlertServices } from '../../../../../alerting/server'; -import { ruleActionsSavedObjectType } from './saved_object_mappings'; -import { RulesActionsSavedObject } from './get_rule_actions_saved_object'; -import { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { getThrottleOptions } from './utils'; -import { IRuleActionsAttributesSavedObjectAttributes } from './types'; - -interface DeleteRuleActionsSavedObject { - ruleAlertId: string; - savedObjectsClient: AlertServices['savedObjectsClient']; - actions: RuleAlertAction[] | undefined; - throttle: string | null | undefined; - ruleActions: RulesActionsSavedObject; -} - -export const updateRuleActionsSavedObject = async ({ - ruleAlertId, - savedObjectsClient, - actions, - throttle, - ruleActions, -}: DeleteRuleActionsSavedObject): Promise => { - const throttleOptions = throttle - ? getThrottleOptions(throttle) - : { - ruleThrottle: ruleActions.ruleThrottle, - alertThrottle: ruleActions.alertThrottle, - }; - - const options = { - actions: actions ?? ruleActions.actions, - ...throttleOptions, - }; - - await savedObjectsClient.update( - ruleActionsSavedObjectType, - ruleActions.id, - { - ruleAlertId, - ...options, - } - ); - - return { - id: ruleActions.id, - ...options, - }; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts index 8b1bd5d4cdddc..6024c48e16854 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts @@ -258,7 +258,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ logger.info(buildRuleMessage(`Found ${createdSignalsCount} signals for notification.`)); if (createdSignalsCount) { - // TODO: Implement the throttle the same way here + // TODO: Implement the throttle the same way here that was done in the siem_signal_alert type and remove all this const alertInstance = services.alertInstanceFactory(alertId); scheduleNotificationActions({ alertInstance, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts index 86a60da7808ef..837b2c79902f4 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts @@ -8,15 +8,10 @@ import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks'; import { rulesClientMock } from '../../../../../alerting/server/mocks'; import { deleteRules } from './delete_rules'; -import { deleteNotifications } from '../notifications/delete_notifications'; -import { deleteRuleActionsSavedObject } from '../rule_actions/delete_rule_actions_saved_object'; import { SavedObjectsFindResult } from '../../../../../../../src/core/server'; import { IRuleStatusSOAttributes } from './types'; import { RuleExecutionLogClient } from '../rule_execution_log/__mocks__/rule_execution_log_client'; -jest.mock('../notifications/delete_notifications'); -jest.mock('../rule_actions/delete_rule_actions_saved_object'); - describe('deleteRules', () => { let rulesClient: ReturnType; let ruleStatusClient: ReturnType; @@ -28,7 +23,7 @@ describe('deleteRules', () => { ruleStatusClient = new RuleExecutionLogClient(); }); - it('should delete the rule along with its notifications, actions, and statuses', async () => { + it('should delete the rule along with its actions, and statuses', async () => { const ruleStatus: SavedObjectsFindResult = { id: 'statusId', type: '', @@ -60,14 +55,6 @@ describe('deleteRules', () => { await deleteRules(rule); expect(rulesClient.delete).toHaveBeenCalledWith({ id: rule.id }); - expect(deleteNotifications).toHaveBeenCalledWith({ - ruleAlertId: rule.id, - rulesClient: expect.any(Object), - }); - expect(deleteRuleActionsSavedObject).toHaveBeenCalledWith({ - ruleAlertId: rule.id, - savedObjectsClient: expect.any(Object), - }); expect(ruleStatusClient.delete).toHaveBeenCalledWith(ruleStatus.id); }); }); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts index 2c68887c73f0d..4237686bebc15 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts @@ -5,19 +5,15 @@ * 2.0. */ -import { deleteNotifications } from '../notifications/delete_notifications'; -import { deleteRuleActionsSavedObject } from '../rule_actions/delete_rule_actions_saved_object'; import { DeleteRuleOptions } from './types'; export const deleteRules = async ({ rulesClient, - savedObjectsClient, + savedObjectsClient, // TODO: Delete this unused parameter ruleStatusClient, ruleStatuses, id, }: DeleteRuleOptions) => { await rulesClient.delete({ id }); - await deleteNotifications({ rulesClient, ruleAlertId: id }); - await deleteRuleActionsSavedObject({ ruleAlertId: id, savedObjectsClient }); ruleStatuses.forEach(async (obj) => ruleStatusClient.delete(obj.id)); }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts deleted file mode 100644 index 5d91feb71ed76..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules_notifications.ts +++ /dev/null @@ -1,52 +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 { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { RulesClient, AlertServices } from '../../../../../alerting/server'; -import { updateOrCreateRuleActionsSavedObject } from '../rule_actions/update_or_create_rule_actions_saved_object'; -import { updateNotifications } from '../notifications/update_notifications'; -import { RuleActions } from '../rule_actions/types'; - -interface UpdateRulesNotifications { - rulesClient: RulesClient; - savedObjectsClient: AlertServices['savedObjectsClient']; - ruleAlertId: string; - actions: RuleAlertAction[] | undefined; - throttle: string | null | undefined; - enabled: boolean; - name: string; -} - -export const updateRulesNotifications = async ({ - rulesClient, - savedObjectsClient, - ruleAlertId, - actions, - enabled, - name, - throttle, -}: UpdateRulesNotifications): Promise => { - // console.log('---> updateRulesNotifications throttle:', throttle); - const ruleActions = await updateOrCreateRuleActionsSavedObject({ - savedObjectsClient, - ruleAlertId, - actions, - throttle, - }); - // console.log('---> ruleActions.alertThrottle:', ruleActions.alertThrottle); - - await updateNotifications({ - rulesClient, - ruleAlertId, - enabled, - name, - actions: ruleActions.actions, - interval: ruleActions.alertThrottle, - }); - - return ruleActions; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts index 6435204d1b7df..f033c4c420327 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.test.ts @@ -20,7 +20,6 @@ import { } from './utils'; import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils'; import { RuleExecutorOptions, SearchAfterAndBulkCreateReturnType } from './types'; -import { scheduleNotificationActions } from '../notifications/schedule_notification_actions'; import { RuleAlertType } from '../rules/types'; import { listMock } from '../../../../../lists/server/mocks'; import { getListClientMock } from '../../../../../lists/server/services/lists/list_client.mock'; @@ -35,6 +34,7 @@ import { getMlRuleParams, getQueryRuleParams } from '../schemas/rule_schemas.moc import { ResponseError } from '@elastic/elasticsearch/lib/errors'; import { allowedExperimentalValues } from '../../../../common/experimental_features'; import { ruleRegistryMocks } from '../../../../../rule_registry/server/mocks'; +import { scheduleNotificationActions } from '../notifications/schedule_notification_actions'; jest.mock('./rule_status_saved_objects_client'); jest.mock('./rule_status_service'); @@ -304,12 +304,6 @@ describe('signal_rule_alert_type', () => { }); await alert.executor(payload); - - expect(scheduleNotificationActions).toHaveBeenCalledWith( - expect.objectContaining({ - signalsCount: 10, - }) - ); }); it('should resolve results_link when meta is an empty object to use "/app/security"', async () => { diff --git a/x-pack/plugins/security_solution/server/plugin.ts b/x-pack/plugins/security_solution/server/plugin.ts index e5c837293e2a8..eb019bbbe5660 100644 --- a/x-pack/plugins/security_solution/server/plugin.ts +++ b/x-pack/plugins/security_solution/server/plugin.ts @@ -52,8 +52,6 @@ import { createQueryAlertType } from './lib/detection_engine/rule_types'; import { initRoutes } from './routes'; import { isAlertExecutor } from './lib/detection_engine/signals/types'; import { signalRulesAlertType } from './lib/detection_engine/signals/signal_rule_alert_type'; -import { rulesNotificationAlertType } from './lib/detection_engine/notifications/rules_notification_alert_type'; -import { isNotificationAlertExecutor } from './lib/detection_engine/notifications/types'; import { ManifestTask } from './endpoint/lib/artifacts'; import { initSavedObjects } from './saved_objects'; import { AppClientFactory } from './client'; @@ -299,17 +297,10 @@ export class Plugin implements IPlugin { From 89ea58a6380727259c007416d592028cc27340c6 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Mon, 23 Aug 2021 18:02:37 -0600 Subject: [PATCH 03/10] Removed more side car notification code that is dead now --- .../notifications/add_tags.test.ts | 27 ------------ .../notifications/add_tags.ts | 11 ----- .../notifications/get_signals_count.ts | 40 ----------------- .../schedule_notification_actions.ts | 3 -- .../notifications/types.test.ts | 20 --------- .../detection_engine/notifications/types.ts | 31 ------------- .../routes/__mocks__/request_responses.ts | 43 ------------------- .../routes/rules/delete_rules_bulk_route.ts | 2 +- .../routes/rules/delete_rules_route.ts | 2 +- .../routes/rules/find_rules_route.ts | 3 +- .../routes/rules/patch_rules_bulk_route.ts | 2 +- .../routes/rules/read_rules_route.ts | 2 +- .../routes/rules/update_rules_bulk_route.ts | 2 +- .../routes/rules/utils.test.ts | 3 +- .../detection_engine/routes/rules/utils.ts | 14 ++---- .../routes/rules/validate.test.ts | 2 +- .../detection_engine/routes/rules/validate.ts | 7 ++- .../detection_engine/rule_actions/utils.ts | 34 --------------- .../lib/detection_engine/rules/utils.ts | 35 +++++++++++++-- .../schemas/rule_converters.ts | 18 +++++--- 20 files changed, 59 insertions(+), 242 deletions(-) delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/get_signals_count.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts delete mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/utils.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.test.ts deleted file mode 100644 index eba896c3ea1ed..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.test.ts +++ /dev/null @@ -1,27 +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 { addTags } from './add_tags'; -import { INTERNAL_RULE_ALERT_ID_KEY } from '../../../../common/constants'; - -describe('add_tags', () => { - test('it should add a rule id as an internal structure', () => { - const tags = addTags([], 'rule-1'); - expect(tags).toEqual([`${INTERNAL_RULE_ALERT_ID_KEY}:rule-1`]); - }); - - test('it should not allow duplicate tags to be created', () => { - const tags = addTags(['tag-1', 'tag-1'], 'rule-1'); - expect(tags).toEqual(['tag-1', `${INTERNAL_RULE_ALERT_ID_KEY}:rule-1`]); - }); - - test('it should not allow duplicate internal tags to be created when called two times in a row', () => { - const tags1 = addTags(['tag-1'], 'rule-1'); - const tags2 = addTags(tags1, 'rule-1'); - expect(tags2).toEqual(['tag-1', `${INTERNAL_RULE_ALERT_ID_KEY}:rule-1`]); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.ts deleted file mode 100644 index 61535e5ae6492..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/add_tags.ts +++ /dev/null @@ -1,11 +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 { INTERNAL_RULE_ALERT_ID_KEY } from '../../../../common/constants'; - -export const addTags = (tags: string[], ruleAlertId: string): string[] => - Array.from(new Set([...tags, `${INTERNAL_RULE_ALERT_ID_KEY}:${ruleAlertId}`])); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/get_signals_count.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/get_signals_count.ts deleted file mode 100644 index b864919fd7295..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/get_signals_count.ts +++ /dev/null @@ -1,40 +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 { ElasticsearchClient } from 'kibana/server'; -import { buildSignalsSearchQuery } from './build_signals_query'; - -interface GetSignalsCount { - from?: string; - to?: string; - ruleId: string; - index: string; - esClient: ElasticsearchClient; -} - -export const getSignalsCount = async ({ - from, - to, - ruleId, - index, - esClient, -}: GetSignalsCount): Promise => { - if (from == null || to == null) { - throw Error('"from" or "to" was not provided to signals count query'); - } - - const query = buildSignalsSearchQuery({ - index, - ruleId, - to, - from, - }); - - const { body: result } = await esClient.count(query); - - return result.count; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts index ea8c33ce60d01..147c54cd6eb8a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_notification_actions.ts @@ -22,9 +22,6 @@ interface ScheduleNotificationActions { signals: unknown[]; } -/** - * - */ export const scheduleNotificationActions = ({ alertInstance, signalsCount, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts deleted file mode 100644 index 209e6379291ed..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.test.ts +++ /dev/null @@ -1,20 +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 { getNotificationResult, getAlertMock } from '../routes/__mocks__/request_responses'; -import { isAlertTypes } from './types'; -import { getQueryRuleParams } from '../schemas/rule_schemas.mock'; - -describe('types', () => { - it('isAlertTypes should return true if is RuleNotificationAlertType type', () => { - expect(isAlertTypes([getNotificationResult()])).toEqual(true); - }); - - it('isAlertTypes should return false if is not RuleNotificationAlertType', () => { - expect(isAlertTypes([getAlertMock(getQueryRuleParams())])).toEqual(false); - }); -}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts deleted file mode 100644 index 08fef8f561ee3..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/types.ts +++ /dev/null @@ -1,31 +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 { RulesClient, PartialAlert, AlertTypeParams } from '../../../../../alerting/server'; -import { Alert } from '../../../../../alerting/common'; -import { NOTIFICATIONS_ID } from '../../../../common/constants'; - -export interface RuleNotificationAlertTypeParams extends AlertTypeParams { - ruleAlertId: string; -} -export type RuleNotificationAlertType = Alert; - -export interface Clients { - rulesClient: RulesClient; -} - -export const isAlertTypes = ( - partialAlert: Array> -): partialAlert is RuleNotificationAlertType[] => { - return partialAlert.every((rule) => isAlertType(rule)); -}; - -export const isAlertType = ( - partialAlert: PartialAlert -): partialAlert is RuleNotificationAlertType => { - return partialAlert.alertTypeId === NOTIFICATIONS_ID; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts index b628ebc85bc17..a7eff049d0d9e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -27,7 +27,6 @@ import { IRuleStatusSOAttributes, } from '../../rules/types'; import { requestMock } from './request'; -import { RuleNotificationAlertType } from '../../notifications/types'; import { QuerySignalsSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/query_signals_index_schema'; import { SetSignalsStatusSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/set_signal_status_schema'; import { getCreateRulesSchemaMock } from '../../../../../common/detection_engine/schemas/request/rule_schemas.mock'; @@ -576,48 +575,6 @@ export const getSuccessfulSignalUpdateResponse = () => ({ failures: [], }); -// TODO: Should this be deleted? -export const getNotificationResult = (): RuleNotificationAlertType => ({ - id: '200dbf2f-b269-4bf9-aa85-11ba32ba73ba', - name: 'Notification for Rule Test', - tags: ['__internal_rule_alert_id:85b64e8a-2e40-4096-86af-5ac172c10825'], - alertTypeId: 'siem.notifications', - consumer: 'siem', - params: { - ruleAlertId: '85b64e8a-2e40-4096-86af-5ac172c10825', - }, - schedule: { - interval: '5m', - }, - enabled: true, - actions: [ - { - actionTypeId: '.slack', - params: { - message: - 'Rule generated {{state.signals_count}} signals\n\n{{context.rule.name}}\n{{{context.results_link}}}', - }, - group: 'default', - id: '99403909-ca9b-49ba-9d7a-7e5320e68d05', - }, - ], - throttle: null, - notifyWhen: null, - apiKey: null, - apiKeyOwner: 'elastic', - createdBy: 'elastic', - updatedBy: 'elastic', - createdAt: new Date('2020-03-21T11:15:13.530Z'), - muteAll: false, - mutedInstanceIds: [], - scheduledTaskId: '62b3a130-6b70-11ea-9ce9-6b9818c4cbd7', - updatedAt: new Date('2020-03-21T12:37:08.730Z'), - executionStatus: { - status: 'unknown', - lastExecutionDate: new Date('2020-08-20T19:23:38Z'), - }, -}); - export const getFinalizeSignalsMigrationRequest = () => requestMock.create({ method: 'post', diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts index 5016f93ef2cf5..6a86bb5d9b2fb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts @@ -89,7 +89,7 @@ export const deleteRulesBulkRoute = (router: SecuritySolutionPluginRouter) => { ruleStatuses, id: rule.id, }); - return transformValidateBulkError(idOrRuleIdOrUnknown, rule, undefined, ruleStatuses); + return transformValidateBulkError(idOrRuleIdOrUnknown, rule, ruleStatuses); } catch (err) { return transformBulkError(idOrRuleIdOrUnknown, err); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts index 2cee8301a05ff..d5ac300b89047 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts @@ -76,7 +76,7 @@ export const deleteRulesRoute = ( ruleStatuses, id: rule.id, }); - const transformed = transform(rule, undefined, ruleStatuses[0]); + const transformed = transform(rule, ruleStatuses[0]); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'failed to transform alert' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts index 1ca3b73829038..ed39d42c38e4a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/find_rules_route.ts @@ -67,8 +67,7 @@ export const findRulesRoute = ( logsCount: 1, spaceId: context.securitySolution.getSpaceId(), }); - // TODO: Can we remove the `{}` and the parameter below? - const transformed = transformFindAlerts(rules, {}, ruleStatuses); + const transformed = transformFindAlerts(rules, ruleStatuses); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts index 9629e978e60f1..3aaa82ea56f3f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_bulk_route.ts @@ -185,7 +185,7 @@ export const patchRulesBulkRoute = ( ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - return transformValidateBulkError(rule.id, rule, undefined, ruleStatuses); + return transformValidateBulkError(rule.id, rule, ruleStatuses); } else { return getIdBulkError({ id, ruleId }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts index d468a2863a4de..7aef65e7918b2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/read_rules_route.ts @@ -72,7 +72,7 @@ export const readRulesRoute = ( currentStatus.attributes.statusDate = rule.executionStatus.lastExecutionDate.toISOString(); currentStatus.attributes.status = RuleExecutionStatus.failed; } - const transformed = transform(rule, undefined, currentStatus); + const transformed = transform(rule, currentStatus); if (transformed == null) { return siemResponse.error({ statusCode: 500, body: 'Internal error transforming' }); } else { 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 9c91f14dc5c98..389c49d3cff4e 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 @@ -81,7 +81,7 @@ export const updateRulesBulkRoute = ( ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - return transformValidateBulkError(rule.id, rule, undefined, ruleStatuses); + return transformValidateBulkError(rule.id, rule, ruleStatuses); } else { return getIdBulkError({ id: payloadRule.id, ruleId: payloadRule.rule_id }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts index 29e322d7fcab5..0018a37016980 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.test.ts @@ -255,7 +255,7 @@ describe('utils', () => { describe('transformFindAlerts', () => { test('outputs empty data set when data set is empty correct', () => { - const output = transformFindAlerts({ data: [], page: 1, perPage: 0, total: 0 }, {}, {}); + const output = transformFindAlerts({ data: [], page: 1, perPage: 0, total: 0 }, {}); expect(output).toEqual({ data: [], page: 1, perPage: 0, total: 0 }); }); @@ -267,7 +267,6 @@ describe('utils', () => { total: 0, data: [getAlertMock(getQueryRuleParams())], }, - {}, {} ); const expected = getOutputRuleAlertForRest(); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts index 082269b7b3b87..6e1faf819c3d5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/utils.ts @@ -30,7 +30,6 @@ import { createImportErrorObject, OutputError, } from '../utils'; -import { RuleActions } from '../../rule_actions/types'; import { internalRuleToAPIResponse } from '../../schemas/rule_converters'; import { RuleParams } from '../../schemas/rule_schemas'; import { SanitizedAlert } from '../../../../../../alerting/common'; @@ -104,10 +103,9 @@ export const transformTags = (tags: string[]): string[] => { // those on the export export const transformAlertToRule = ( alert: SanitizedAlert, - ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): Partial => { - return internalRuleToAPIResponse(alert, ruleActions, ruleStatus?.attributes); + return internalRuleToAPIResponse(alert, ruleStatus?.attributes); }; export const transformAlertsToRules = (alerts: RuleAlertType[]): Array> => { @@ -116,7 +114,6 @@ export const transformAlertsToRules = (alerts: RuleAlertType[]): Array, - ruleActions: { [key: string]: RuleActions | undefined }, // TODO: Can we remove this and just use the findResults now? ruleStatuses: { [key: string]: IRuleStatusSOAttributes[] | undefined } ): { page: number; @@ -131,20 +128,18 @@ export const transformFindAlerts = ( data: findResults.data.map((alert) => { const statuses = ruleStatuses[alert.id]; const status = statuses ? statuses[0] : undefined; - return internalRuleToAPIResponse(alert, ruleActions[alert.id], status); + return internalRuleToAPIResponse(alert, status); }), }; }; export const transform = ( alert: PartialAlert, - ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): Partial | null => { if (isAlertType(alert)) { return transformAlertToRule( alert, - ruleActions, isRuleStatusSavedObjectType(ruleStatus) ? ruleStatus : undefined ); } @@ -155,14 +150,13 @@ export const transform = ( export const transformOrBulkError = ( ruleId: string, alert: PartialAlert, - ruleActions: RuleActions, ruleStatus?: unknown ): Partial | BulkError => { if (isAlertType(alert)) { if (isRuleStatusFindType(ruleStatus) && ruleStatus?.saved_objects.length > 0) { - return transformAlertToRule(alert, ruleActions, ruleStatus?.saved_objects[0] ?? ruleStatus); + return transformAlertToRule(alert, ruleStatus?.saved_objects[0] ?? ruleStatus); } else { - return transformAlertToRule(alert, ruleActions); + return transformAlertToRule(alert); } } else { return createBulkErrorObject({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.test.ts index 1ca8c27995922..9cbd4de71613a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.test.ts @@ -110,7 +110,7 @@ describe('validate', () => { test('it should do a validation correctly of a rule id with ruleStatus passed in', () => { const ruleStatuses = getRuleExecutionStatuses(); const ruleAlert = getAlertMock(getQueryRuleParams()); - const validatedOrError = transformValidateBulkError('rule-1', ruleAlert, null, ruleStatuses); + const validatedOrError = transformValidateBulkError('rule-1', ruleAlert, ruleStatuses); const expected: RulesSchema = { ...ruleOutput(), status: RuleExecutionStatus.succeeded, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts index 64dd36372b4e1..6a33e71689eda 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts @@ -33,7 +33,7 @@ export const transformValidate = ( ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): [RulesSchema | null, string | null] => { - const transformed = transform(alert, ruleActions, ruleStatus); + const transformed = transform(alert, ruleStatus); if (transformed == null) { return [null, 'Internal error transforming']; } else { @@ -46,7 +46,7 @@ export const newTransformValidate = ( ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): [FullResponseSchema | null, string | null] => { - const transformed = transform(alert, ruleActions, ruleStatus); + const transformed = transform(alert, ruleStatus); if (transformed == null) { return [null, 'Internal error transforming']; } else { @@ -57,12 +57,11 @@ export const newTransformValidate = ( export const transformValidateBulkError = ( ruleId: string, alert: PartialAlert, - ruleActions?: RuleActions | null, // TODO: Can we remove this now? I don't think anyone is using it anymore. ruleStatus?: Array> ): RulesSchema | BulkError => { if (isAlertType(alert)) { if (ruleStatus && ruleStatus?.length > 0 && isRuleStatusSavedObjectType(ruleStatus[0])) { - const transformed = transformAlertToRule(alert, ruleActions, ruleStatus[0]); + const transformed = transformAlertToRule(alert, ruleStatus[0]); const [validated, errors] = validateNonExact(transformed, rulesSchema); if (errors != null || validated == null) { return createBulkErrorObject({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/utils.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/utils.ts deleted file mode 100644 index b6fb4fcf28b33..0000000000000 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/utils.ts +++ /dev/null @@ -1,34 +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 { SavedObjectsUpdateResponse } from 'kibana/server'; -import { RuleAlertAction } from '../../../../common/detection_engine/types'; -import { IRuleActionsAttributesSavedObjectAttributes } from './types'; - -export const getThrottleOptions = ( - throttle: string | undefined | null = 'no_actions' -): { - ruleThrottle: string; - alertThrottle: string | null; -} => ({ - ruleThrottle: throttle ?? 'no_actions', - alertThrottle: ['no_actions', 'rule'].includes(throttle ?? 'no_actions') ? null : throttle, -}); - -export const getRuleActionsFromSavedObject = ( - savedObject: SavedObjectsUpdateResponse -): { - id: string; - actions: RuleAlertAction[]; - alertThrottle: string | null; - ruleThrottle: string; -} => ({ - id: savedObject.id, - actions: savedObject.attributes.actions || [], - alertThrottle: savedObject.attributes.alertThrottle || null, - ruleThrottle: savedObject.attributes.ruleThrottle || 'no_actions', -}); 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 256b1e2a03758..b8028dca8aac6 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 @@ -27,6 +27,7 @@ import type { } from '@kbn/securitysolution-io-ts-alerting-types'; import type { ListArrayOrUndefined } from '@kbn/securitysolution-io-ts-list-types'; import type { VersionOrUndefined } from '@kbn/securitysolution-io-ts-types'; +import { SanitizedAlert } from '../../../../../alerting/common'; import { DescriptionOrUndefined, AnomalyThresholdOrUndefined, @@ -53,7 +54,11 @@ import { EventCategoryOverrideOrUndefined, } from '../../../../common/detection_engine/schemas/common/schemas'; import { PartialFilter } from '../types'; -import { NotifyWhen } from '../schemas/rule_schemas'; +import { NotifyWhen, RuleParams } from '../schemas/rule_schemas'; +import { + NOTIFICATION_THROTTLE_NO_ACTIONS, + NOTIFICATION_THROTTLE_RULE, +} from '../../../../common/constants'; export const calculateInterval = ( interval: string | undefined, @@ -169,20 +174,42 @@ export const calculateName = ({ } }; +// TODO: Write unit tests export const transformToNotifyWhen = (throttle: string | null | undefined): NotifyWhen | null => { - if (throttle == null || throttle === 'no_actions') { + if (throttle == null || throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { return null; // Although I return null, this does not change the value of the "notifyWhen" and it keeps the current value of "notifyWhen" - } else if (throttle === 'rule') { + } else if (throttle === NOTIFICATION_THROTTLE_RULE) { return 'onActiveAlert'; } else { return 'onThrottleInterval'; } }; +// TODO: Write unit tests export const transformToAlertThrottle = (throttle: string | null | undefined): string | null => { - if (throttle == null || throttle === 'rule' || throttle === 'no_actions') { + if ( + throttle == null || + throttle === NOTIFICATION_THROTTLE_RULE || + throttle === NOTIFICATION_THROTTLE_NO_ACTIONS + ) { return null; } else { return throttle; } }; + +// TODO: Write unit tests +export const transformFromAlertThrottle = (rule: SanitizedAlert): string => { + if (rule.muteAll === true) { + return NOTIFICATION_THROTTLE_NO_ACTIONS; + } else if ( + rule.notifyWhen === 'onActiveAlert' || + (rule.throttle == null && rule.notifyWhen == null && rule.actions.length !== 0) + ) { + return 'rule'; + } else if (rule.throttle == null) { + return NOTIFICATION_THROTTLE_NO_ACTIONS; + } else { + return rule.throttle; + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts index d8706881bea3b..d79bbd79fde5a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts @@ -23,7 +23,6 @@ import { FullResponseSchema, ResponseTypeSpecific, } from '../../../../common/detection_engine/schemas/request'; -import { RuleActions } from '../rule_actions/types'; import { AppClient } from '../../../types'; import { addTags } from '../rules/add_tags'; import { DEFAULT_MAX_SIGNALS, SERVER_APP_ID, SIGNALS_ID } from '../../../../common/constants'; @@ -32,7 +31,11 @@ import { SanitizedAlert } from '../../../../../alerting/common'; import { IRuleStatusSOAttributes } from '../rules/types'; import { transformTags } from '../routes/rules/utils'; import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas'; -import { transformToAlertThrottle, transformToNotifyWhen } from '../rules/utils'; +import { + transformFromAlertThrottle, + transformToAlertThrottle, + transformToNotifyWhen, +} from '../rules/utils'; // These functions provide conversions from the request API schema to the internal rule schema and from the internal rule schema // to the response API schema. This provides static type-check assurances that the internal schema is in sync with the API schema for @@ -272,7 +275,6 @@ export const commonParamsCamelToSnake = (params: BaseRuleParams) => { export const internalRuleToAPIResponse = ( rule: SanitizedAlert, - ruleActions?: RuleActions | null, ruleStatus?: IRuleStatusSOAttributes ): FullResponseSchema => { const mergedStatus = ruleStatus ? mergeAlertWithSidecarStatus(rule, ruleStatus) : undefined; @@ -292,8 +294,14 @@ export const internalRuleToAPIResponse = ( // Type specific security solution rule params ...typeSpecificCamelToSnake(rule.params), // Actions - throttle: ruleActions?.ruleThrottle || 'no_actions', - actions: ruleActions?.actions ?? [], + throttle: transformFromAlertThrottle(rule), + actions: + rule?.actions.map((action) => ({ + group: action.group, + id: action.id, + action_type_id: action.actionTypeId, + params: action.params, + })) ?? [], // Rule status status: mergedStatus?.status ?? undefined, status_date: mergedStatus?.statusDate ?? undefined, From f1f70df4f226913e54627254c68600be9833f5d3 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Mon, 23 Aug 2021 20:01:55 -0600 Subject: [PATCH 04/10] Fixed actions logic and cleaned up more code --- .../routes/rules/create_rules_bulk_route.ts | 2 +- .../routes/rules/create_rules_route.ts | 4 +- .../routes/rules/delete_rules_bulk_route.ts | 2 - .../routes/rules/delete_rules_route.ts | 2 - .../routes/rules/patch_rules_route.ts | 2 +- .../routes/rules/perform_bulk_action_route.ts | 1 - .../routes/rules/update_rules_route.ts | 2 +- .../detection_engine/routes/rules/validate.ts | 3 - .../rule_actions/migrations.ts | 15 ++++- .../rule_actions/saved_object_mappings.ts | 22 +++++-- .../detection_engine/rule_actions/types.ts | 65 ++++--------------- .../rules/delete_rules.test.ts | 8 +-- .../detection_engine/rules/delete_rules.ts | 1 - .../lib/detection_engine/rules/types.ts | 8 +-- .../lib/detection_engine/rules/utils.ts | 2 +- .../detection_engine/schemas/rule_schemas.ts | 2 +- 16 files changed, 56 insertions(+), 85 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts index d4faed92eaf41..5f44ab0ada92d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_bulk_route.ts @@ -105,7 +105,7 @@ export const createRulesBulkRoute = ( data: internalRule, }); - // mutes if we are creating the rule with the explicit no actions + // mutes if we are creating the rule with the explicit "no_actions" if (payloadRule.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { await rulesClient.muteAll({ id: createdRule.id }); } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts index f45e4df800a82..333fa9c17a75b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/create_rules_route.ts @@ -97,7 +97,7 @@ export const createRulesRoute = ( data: internalRule, }); - // mutes if we are creating the rule with the explicit no actions + // mutes if we are creating the rule with the explicit "no_actions" if (request.body.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { await rulesClient.muteAll({ id: createdRule.id }); } @@ -107,7 +107,7 @@ export const createRulesRoute = ( ruleId: createdRule.id, spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = newTransformValidate(createdRule, undefined, ruleStatuses[0]); + const [validated, errors] = newTransformValidate(createdRule, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts index 6a86bb5d9b2fb..7a5b7121eb33b 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_bulk_route.ts @@ -50,7 +50,6 @@ export const deleteRulesBulkRoute = (router: SecuritySolutionPluginRouter) => { const siemResponse = buildSiemResponse(response); const rulesClient = context.alerting?.getRulesClient(); - const savedObjectsClient = context.core.savedObjects.client; if (!rulesClient) { return siemResponse.error({ statusCode: 404 }); @@ -84,7 +83,6 @@ export const deleteRulesBulkRoute = (router: SecuritySolutionPluginRouter) => { }); await deleteRules({ rulesClient, - savedObjectsClient, ruleStatusClient, ruleStatuses, id: rule.id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts index d5ac300b89047..499f5c151c66c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/delete_rules_route.ts @@ -48,7 +48,6 @@ export const deleteRulesRoute = ( const { id, rule_id: ruleId } = request.query; const rulesClient = context.alerting?.getRulesClient(); - const savedObjectsClient = context.core.savedObjects.client; if (!rulesClient) { return siemResponse.error({ statusCode: 404 }); @@ -71,7 +70,6 @@ export const deleteRulesRoute = ( }); await deleteRules({ rulesClient, - savedObjectsClient, ruleStatusClient, ruleStatuses, id: rule.id, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts index 5a80ecb2a1aba..b564262b4a5c7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/patch_rules_route.ts @@ -189,7 +189,7 @@ export const patchRulesRoute = ( spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = transformValidate(rule, undefined, ruleStatuses[0]); + const [validated, errors] = transformValidate(rule, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts index 23b167ae351c4..70198d081ebfa 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/perform_bulk_action_route.ts @@ -110,7 +110,6 @@ export const performBulkActionRoute = ( }); await deleteRules({ rulesClient, - savedObjectsClient, ruleStatusClient, ruleStatuses, id: rule.id, 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 e3b2290715d96..ecf61bec2b20a 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 @@ -74,7 +74,7 @@ export const updateRulesRoute = ( ruleId: rule.id, spaceId: context.securitySolution.getSpaceId(), }); - const [validated, errors] = transformValidate(rule, undefined, ruleStatuses[0]); + const [validated, errors] = transformValidate(rule, ruleStatuses[0]); if (errors != null) { return siemResponse.error({ statusCode: 500, body: errors }); } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts index 6a33e71689eda..ccb3201848e3c 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/validate.ts @@ -25,12 +25,10 @@ import { } from '../../rules/types'; import { createBulkErrorObject, BulkError } from '../utils'; import { transform, transformAlertToRule } from './utils'; -import { RuleActions } from '../../rule_actions/types'; import { RuleParams } from '../../schemas/rule_schemas'; export const transformValidate = ( alert: PartialAlert, - ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): [RulesSchema | null, string | null] => { const transformed = transform(alert, ruleStatus); @@ -43,7 +41,6 @@ export const transformValidate = ( export const newTransformValidate = ( alert: PartialAlert, - ruleActions?: RuleActions | null, ruleStatus?: SavedObject ): [FullResponseSchema | null, string | null] => { const transformed = transform(alert, ruleStatus); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/migrations.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/migrations.ts index 4b66c20e5784a..3004304445ff7 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/migrations.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/migrations.ts @@ -5,13 +5,20 @@ * 2.0. */ +import { RuleAlertAction } from '../../../../common/detection_engine/types'; import { SavedObjectUnsanitizedDoc, SavedObjectSanitizedDoc, SavedObjectAttributes, } from '../../../../../../../src/core/server'; -import { IRuleActionsAttributesSavedObjectAttributes, RuleAlertAction } from './types'; +import { IRuleActionsAttributesSavedObjectAttributes } from './types'; +/** + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations + * @deprecated Remove this once we no longer need legacy migrations for rule actions (8.0.0) + */ function isEmptyObject(obj: {}) { for (const attr in obj) { if (Object.prototype.hasOwnProperty.call(obj, attr)) { @@ -21,6 +28,12 @@ function isEmptyObject(obj: {}) { return true; } +/** + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations + * @deprecated Remove this once we no longer need legacy migrations for rule actions (8.0.0) + */ export const ruleActionsSavedObjectMigration = { '7.11.2': ( doc: SavedObjectUnsanitizedDoc diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts index 637d1426e95f8..6522cb431d0fb 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/saved_object_mappings.ts @@ -8,17 +8,21 @@ import { SavedObjectsType } from '../../../../../../../src/core/server'; import { ruleActionsSavedObjectMigration } from './migrations'; -// TODO: Can we delete this mapping now or do we have to wait for the migration for the delete to remove it? /** - * @deprecated + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations. + * * @deprecated Remove this once we no longer need legacy migrations for rule actions (8.0.0) */ -export const ruleActionsSavedObjectType = 'siem-detection-engine-rule-actions'; +const ruleActionsSavedObjectType = 'siem-detection-engine-rule-actions'; -// TODO: Can we delete this mapping now or do we have to wait for the migration for the delete to remove it? /** - * @deprecated + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations. + * * @deprecated Remove this once we no longer need legacy migrations for rule actions (8.0.0) */ -export const ruleActionsSavedObjectMappings: SavedObjectsType['mappings'] = { +const ruleActionsSavedObjectMappings: SavedObjectsType['mappings'] = { properties: { alertThrottle: { type: 'keyword', @@ -49,6 +53,12 @@ export const ruleActionsSavedObjectMappings: SavedObjectsType['mappings'] = { }, }; +/** + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations. + * @deprecated Remove this once we no longer need legacy migrations for rule actions (8.0.0) + */ export const type: SavedObjectsType = { name: ruleActionsSavedObjectType, hidden: false, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts index a53546a3e63a4..e43e49b669424 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_actions/types.ts @@ -5,12 +5,15 @@ * 2.0. */ -import { get } from 'lodash/fp'; -import { SavedObject, SavedObjectAttributes, SavedObjectsFindResponse } from 'kibana/server'; +import { SavedObjectAttributes } from 'kibana/server'; import { RuleAlertAction } from '../../../../common/detection_engine/types'; -export { RuleAlertAction }; - +/** + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations. + * @deprecated + */ // eslint-disable-next-line @typescript-eslint/no-explicit-any export interface IRuleActionsAttributes extends Record { ruleAlertId: string; @@ -19,54 +22,12 @@ export interface IRuleActionsAttributes extends Record { alertThrottle: string | null; } -// TODO: Can this be removed and anything using it be removed as well? -export interface RuleActions { - id: string; - actions: RuleAlertAction[]; - ruleThrottle: string; - alertThrottle: string | null; -} - +/** + * We keep this around to migrate and update data for the old deprecated rule actions saved object mapping but we + * do not use it anymore within the code base. Once we feel comfortable that users are upgrade far enough and this is no longer + * needed then it will be safe to remove this saved object and all its migrations. + * @deprecated + */ export interface IRuleActionsAttributesSavedObjectAttributes extends IRuleActionsAttributes, SavedObjectAttributes {} - -export interface RuleActionsResponse { - [key: string]: { - actions: IRuleActionsAttributes | null | undefined; - }; -} - -export interface IRuleActionsSavedObject { - type: string; - id: string; - attributes: Array>; - references: unknown[]; - updated_at: string; - version: string; -} - -export interface IRuleActionsFindType { - page: number; - per_page: number; - total: number; - saved_objects: IRuleActionsSavedObject[]; -} - -export const isRuleActionsSavedObjectType = ( - obj: unknown -): obj is SavedObject => { - return get('attributes', obj) != null; -}; - -export const isRuleActionsFindType = ( - obj: unknown -): obj is SavedObjectsFindResponse => { - return get('saved_objects', obj) != null; -}; - -export const isRuleActionsFindTypes = ( - obj: unknown[] | undefined -): obj is Array> => { - return obj ? obj.every((ruleStatus) => isRuleActionsFindType(ruleStatus)) : false; -}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts index 837b2c79902f4..10f80f21c77e5 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.test.ts @@ -5,21 +5,18 @@ * 2.0. */ -import { savedObjectsClientMock } from '../../../../../../../src/core/server/mocks'; import { rulesClientMock } from '../../../../../alerting/server/mocks'; import { deleteRules } from './delete_rules'; import { SavedObjectsFindResult } from '../../../../../../../src/core/server'; -import { IRuleStatusSOAttributes } from './types'; +import { DeleteRuleOptions, IRuleStatusSOAttributes } from './types'; import { RuleExecutionLogClient } from '../rule_execution_log/__mocks__/rule_execution_log_client'; describe('deleteRules', () => { let rulesClient: ReturnType; let ruleStatusClient: ReturnType; - let savedObjectsClient: ReturnType; beforeEach(() => { rulesClient = rulesClientMock.create(); - savedObjectsClient = savedObjectsClientMock.create(); ruleStatusClient = new RuleExecutionLogClient(); }); @@ -44,9 +41,8 @@ describe('deleteRules', () => { score: 0, }; - const rule = { + const rule: DeleteRuleOptions = { rulesClient, - savedObjectsClient, ruleStatusClient, id: 'ruleId', ruleStatuses: [ruleStatus], diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts index 4237686bebc15..b4b6e3c824205 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/delete_rules.ts @@ -9,7 +9,6 @@ import { DeleteRuleOptions } from './types'; export const deleteRules = async ({ rulesClient, - savedObjectsClient, // TODO: Delete this unused parameter ruleStatusClient, ruleStatuses, id, 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 bed006d2e2ac4..235217761c8b1 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 @@ -12,7 +12,6 @@ import { SavedObject, SavedObjectAttributes, SavedObjectsFindResponse, - SavedObjectsClientContract, SavedObjectsFindResult, } from 'kibana/server'; import type { @@ -42,6 +41,8 @@ import type { Severity, MaxSignalsOrUndefined, MaxSignals, + ThrottleOrUndefinedOrNull, + ThrottleOrNull, } from '@kbn/securitysolution-io-ts-alerting-types'; import type { VersionOrUndefined, Version } from '@kbn/securitysolution-io-ts-types'; @@ -256,7 +257,7 @@ export interface CreateRulesOptions { concurrentSearches: ConcurrentSearchesOrUndefined; itemsPerSearch: ItemsPerSearchOrUndefined; threatLanguage: ThreatLanguageOrUndefined; - throttle: string | null; // TODO: Do we have better types here? + throttle: ThrottleOrNull; timestampOverride: TimestampOverrideOrUndefined; to: To; type: Type; @@ -316,7 +317,7 @@ export interface PatchRulesOptions { threatQuery: ThreatQueryOrUndefined; threatMapping: ThreatMappingOrUndefined; threatLanguage: ThreatLanguageOrUndefined; - throttle: string | null | undefined; // TODO: Do we have better types here? + throttle: ThrottleOrUndefinedOrNull; timestampOverride: TimestampOverrideOrUndefined; to: ToOrUndefined; type: TypeOrUndefined; @@ -336,7 +337,6 @@ export interface ReadRuleOptions { export interface DeleteRuleOptions { rulesClient: RulesClient; - savedObjectsClient: SavedObjectsClientContract; ruleStatusClient: IRuleExecutionLogClient; ruleStatuses: Array>; id: Id; 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 b8028dca8aac6..2db600415bccd 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 @@ -203,7 +203,7 @@ export const transformFromAlertThrottle = (rule: SanitizedAlert): st if (rule.muteAll === true) { return NOTIFICATION_THROTTLE_NO_ACTIONS; } else if ( - rule.notifyWhen === 'onActiveAlert' || + (rule.notifyWhen === 'onActiveAlert' && rule.actions.length !== 0) || (rule.throttle == null && rule.notifyWhen == null && rule.actions.length !== 0) ) { return 'rule'; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts index f0df617a22004..76d03f0ff45f0 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts @@ -222,7 +222,7 @@ export const internalRuleUpdate = t.type({ }), actions: actionsCamel, params: ruleParams, - throttle: throttleOrNull, // "onActionGroupChange" | "onActiveAlert" | "onThrottleInterval" + throttle: throttleOrNull, notifyWhen, }); export type InternalRuleUpdate = t.TypeOf; From d44678a1beed4c11e4f8d4b6eeff4f51208529f4 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Tue, 24 Aug 2021 16:06:37 -0600 Subject: [PATCH 05/10] Added unit tests and fixed up the rest of the TODO blocks --- ...dule_throttle_notification_actions.test.ts | 176 ++++++++++++++++++ .../schedule_throttle_notification_actions.ts | 88 +++++++++ .../create_security_rule_type_factory.ts | 18 +- .../lib/detection_engine/rules/utils.test.ts | 148 ++++++++++++++- .../lib/detection_engine/rules/utils.ts | 29 ++- .../signals/signal_rule_alert_type.ts | 64 +++---- 6 files changed, 471 insertions(+), 52 deletions(-) create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.test.ts create mode 100644 x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.ts diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.test.ts new file mode 100644 index 0000000000000..de62c6b211400 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.test.ts @@ -0,0 +1,176 @@ +/* + * 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 { elasticsearchServiceMock } from 'src/core/server/mocks'; +import { alertsMock } from '../../../../../alerting/server/mocks'; +import { scheduleThrottledNotificationActions } from './schedule_throttle_notification_actions'; +import { + NotificationRuleTypeParams, + scheduleNotificationActions, +} from './schedule_notification_actions'; + +jest.mock('./schedule_notification_actions', () => ({ + scheduleNotificationActions: jest.fn(), +})); + +describe('schedule_throttle_notification_actions', () => { + let notificationRuleParams: NotificationRuleTypeParams; + + beforeEach(() => { + (scheduleNotificationActions as jest.Mock).mockReset(); + notificationRuleParams = { + author: ['123'], + id: '123', + name: 'some name', + description: '123', + buildingBlockType: undefined, + from: '123', + ruleId: '123', + immutable: false, + license: '', + falsePositives: ['false positive 1', 'false positive 2'], + query: 'user.name: root or user.name: admin', + language: 'kuery', + savedId: 'savedId-123', + timelineId: 'timelineid-123', + timelineTitle: 'timeline-title-123', + meta: {}, + filters: [], + index: ['index-123'], + maxSignals: 100, + riskScore: 80, + riskScoreMapping: [], + ruleNameOverride: undefined, + outputIndex: 'output-1', + severity: 'high', + severityMapping: [], + threat: [], + timestampOverride: undefined, + to: 'now', + type: 'query', + references: ['http://www.example.com'], + note: '# sample markdown', + version: 1, + exceptionsList: [], + }; + }); + + it('should call "scheduleNotificationActions" if the results length is 1 or greater', async () => { + await scheduleThrottledNotificationActions({ + throttle: '1d', + startedAt: new Date('2021-08-24T19:19:22.094Z'), + id: '123', + kibanaSiemAppUrl: 'http://www.example.com', + outputIndex: 'output-123', + ruleId: 'rule-123', + esClient: elasticsearchServiceMock.createElasticsearchClient( + elasticsearchServiceMock.createSuccessTransportRequestPromise({ + hits: { + hits: [ + { + _source: {}, + }, + ], + total: 1, + }, + }) + ), + alertInstance: alertsMock.createAlertInstanceFactory(), + notificationRuleParams, + }); + + expect(scheduleNotificationActions as jest.Mock).toHaveBeenCalled(); + }); + + it('should NOT call "scheduleNotificationActions" if the results length is 0', async () => { + await scheduleThrottledNotificationActions({ + throttle: '1d', + startedAt: new Date('2021-08-24T19:19:22.094Z'), + id: '123', + kibanaSiemAppUrl: 'http://www.example.com', + outputIndex: 'output-123', + ruleId: 'rule-123', + esClient: elasticsearchServiceMock.createElasticsearchClient( + elasticsearchServiceMock.createSuccessTransportRequestPromise({ + hits: { + hits: [], + total: 0, + }, + }) + ), + alertInstance: alertsMock.createAlertInstanceFactory(), + notificationRuleParams, + }); + + expect(scheduleNotificationActions as jest.Mock).not.toHaveBeenCalled(); + }); + + it('should NOT call "scheduleNotificationActions" if "throttle" is an invalid string', async () => { + await scheduleThrottledNotificationActions({ + throttle: 'invalid', + startedAt: new Date('2021-08-24T19:19:22.094Z'), + id: '123', + kibanaSiemAppUrl: 'http://www.example.com', + outputIndex: 'output-123', + ruleId: 'rule-123', + esClient: elasticsearchServiceMock.createElasticsearchClient( + elasticsearchServiceMock.createSuccessTransportRequestPromise({ + hits: { + hits: [ + { + _source: {}, + }, + ], + total: 1, + }, + }) + ), + alertInstance: alertsMock.createAlertInstanceFactory(), + notificationRuleParams, + }); + + expect(scheduleNotificationActions as jest.Mock).not.toHaveBeenCalled(); + }); + + it('should pass expected arguments into "scheduleNotificationActions" on success', async () => { + await scheduleThrottledNotificationActions({ + throttle: '1d', + startedAt: new Date('2021-08-24T19:19:22.094Z'), + id: '123', + kibanaSiemAppUrl: 'http://www.example.com', + outputIndex: 'output-123', + ruleId: 'rule-123', + esClient: elasticsearchServiceMock.createElasticsearchClient( + elasticsearchServiceMock.createSuccessTransportRequestPromise({ + hits: { + hits: [ + { + _source: { + test: 123, + }, + }, + ], + total: 1, + }, + }) + ), + alertInstance: alertsMock.createAlertInstanceFactory(), + notificationRuleParams, + }); + + expect((scheduleNotificationActions as jest.Mock).mock.calls[0][0].resultsLink).toMatch( + 'http://www.example.com/detections/rules/id/123' + ); + expect(scheduleNotificationActions).toHaveBeenCalledWith( + expect.objectContaining({ + signalsCount: 1, + signals: [{ test: 123 }], + ruleParams: notificationRuleParams, + }) + ); + }); +}); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.ts new file mode 100644 index 0000000000000..5dd583d47b403 --- /dev/null +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/notifications/schedule_throttle_notification_actions.ts @@ -0,0 +1,88 @@ +/* + * 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 { ElasticsearchClient, SavedObject } from 'src/core/server'; +import { parseScheduleDates } from '@kbn/securitysolution-io-ts-utils'; +import { AlertInstance } from '../../../../../alerting/server'; +import { RuleParams } from '../schemas/rule_schemas'; +import { getNotificationResultsLink } from '../notifications/utils'; +import { DEFAULT_RULE_NOTIFICATION_QUERY_SIZE } from '../../../../common/constants'; +import { getSignals } from '../notifications/get_signals'; +import { + NotificationRuleTypeParams, + scheduleNotificationActions, +} from './schedule_notification_actions'; +import { AlertAttributes } from '../signals/types'; + +/** + * Schedules a throttled notification action for executor rules. + * @param throttle The throttle which is the alerting saved object throttle + * @param startedAt When the executor started at + * @param id The id the alert which caused the notifications + * @param kibanaSiemAppUrl The security_solution application url + * @param outputIndex The alerting index we wrote the signals into + * @param ruleId The rule_id of the alert which caused the notifications + * @param esClient The elastic client to do queries + * @param alertInstance The alert instance for notifications + * @param notificationRuleParams The notification rule parameters + */ +export const scheduleThrottledNotificationActions = async ({ + throttle, + startedAt, + id, + kibanaSiemAppUrl, + outputIndex, + ruleId, + esClient, + alertInstance, + notificationRuleParams, +}: { + id: SavedObject['id']; + startedAt: Date; + throttle: AlertAttributes['throttle']; + kibanaSiemAppUrl: string | undefined; + outputIndex: RuleParams['outputIndex']; + ruleId: RuleParams['ruleId']; + esClient: ElasticsearchClient; + alertInstance: AlertInstance; + notificationRuleParams: NotificationRuleTypeParams; +}): Promise => { + const fromInMs = parseScheduleDates(`now-${throttle}`); + const toInMs = parseScheduleDates(startedAt.toISOString()); + + if (fromInMs != null && toInMs != null) { + const resultsLink = getNotificationResultsLink({ + from: fromInMs.toISOString(), + to: toInMs.toISOString(), + id, + kibanaSiemAppUrl, + }); + + const results = await getSignals({ + from: `${fromInMs.valueOf()}`, + to: `${toInMs.valueOf()}`, + size: DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, + index: outputIndex, + ruleId, + esClient, + }); + + const signalsCount = + typeof results.hits.total === 'number' ? results.hits.total : results.hits.total.value; + + const signals = results.hits.hits.map((hit) => hit._source); + if (results.hits.hits.length !== 0) { + scheduleNotificationActions({ + alertInstance, + signalsCount, + signals, + resultsLink, + ruleParams: notificationRuleParams, + }); + } + } +}; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts index bd7968c31563e..d9f36f1f69ccf 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts @@ -33,6 +33,7 @@ import { createResultObject } from './utils'; import { bulkCreateFactory, wrapHitsFactory } from './factories'; import { RuleExecutionLogClient } from '../rule_execution_log/rule_execution_log_client'; import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas'; +import { scheduleThrottledNotificationActions } from '../notifications/schedule_throttle_notification_actions'; /* eslint-disable complexity */ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ @@ -50,6 +51,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ alertId, params, previousStartedAt, + startedAt, services, spaceId, state, @@ -277,8 +279,20 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ logger.info(buildRuleMessage(`Found ${createdSignalsCount} signals for notification.`)); - if (createdSignalsCount) { - // TODO: Implement the throttle the same way here that was done in the siem_signal_alert type and remove all this + if (ruleSO.attributes.throttle != null) { + scheduleThrottledNotificationActions({ + alertInstance: services.alertInstanceFactory(alertId), + throttle: ruleSO.attributes.throttle, + startedAt, + id: ruleSO.id, + kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined) + ?.kibana_siem_app_url, + outputIndex: ruleDataClient.indexName, + ruleId, + esClient: services.scopedClusterClient.asCurrentUser, + notificationRuleParams, + }); + } else if (createdSignalsCount) { const alertInstance = services.alertInstanceFactory(alertId); scheduleNotificationActions({ alertInstance, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.test.ts index 9435ccf3607ed..602e422772711 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/utils.test.ts @@ -5,7 +5,20 @@ * 2.0. */ -import { calculateInterval, calculateVersion, calculateName } from './utils'; +import { + calculateInterval, + calculateVersion, + calculateName, + transformToNotifyWhen, + transformToAlertThrottle, + transformFromAlertThrottle, +} from './utils'; +import { SanitizedAlert } from '../../../../../alerting/common'; +import { RuleParams } from '../schemas/rule_schemas'; +import { + NOTIFICATION_THROTTLE_NO_ACTIONS, + NOTIFICATION_THROTTLE_RULE, +} from '../../../../common/constants'; describe('utils', () => { describe('#calculateInterval', () => { @@ -198,4 +211,137 @@ describe('utils', () => { expect(name).toEqual('untitled'); }); }); + + describe('#transformToNotifyWhen', () => { + test('"null" throttle returns "null" notify', () => { + expect(transformToNotifyWhen(null)).toEqual(null); + }); + + test('"undefined" throttle returns "null" notify', () => { + expect(transformToNotifyWhen(undefined)).toEqual(null); + }); + + test('"NOTIFICATION_THROTTLE_NO_ACTIONS" throttle returns "null" notify', () => { + expect(transformToNotifyWhen(NOTIFICATION_THROTTLE_NO_ACTIONS)).toEqual(null); + }); + + test('"NOTIFICATION_THROTTLE_RULE" throttle returns "onActiveAlert" notify', () => { + expect(transformToNotifyWhen(NOTIFICATION_THROTTLE_RULE)).toEqual('onActiveAlert'); + }); + + test('"1h" throttle returns "onThrottleInterval" notify', () => { + expect(transformToNotifyWhen('1d')).toEqual('onThrottleInterval'); + }); + + test('"1d" throttle returns "onThrottleInterval" notify', () => { + expect(transformToNotifyWhen('1d')).toEqual('onThrottleInterval'); + }); + + test('"7d" throttle returns "onThrottleInterval" notify', () => { + expect(transformToNotifyWhen('7d')).toEqual('onThrottleInterval'); + }); + }); + + describe('#transformToAlertThrottle', () => { + test('"null" throttle returns "null" alert throttle', () => { + expect(transformToAlertThrottle(null)).toEqual(null); + }); + + test('"undefined" throttle returns "null" alert throttle', () => { + expect(transformToAlertThrottle(undefined)).toEqual(null); + }); + + test('"NOTIFICATION_THROTTLE_NO_ACTIONS" throttle returns "null" alert throttle', () => { + expect(transformToAlertThrottle(NOTIFICATION_THROTTLE_NO_ACTIONS)).toEqual(null); + }); + + test('"NOTIFICATION_THROTTLE_RULE" throttle returns "null" alert throttle', () => { + expect(transformToAlertThrottle(NOTIFICATION_THROTTLE_RULE)).toEqual(null); + }); + + test('"1h" throttle returns "1h" alert throttle', () => { + expect(transformToAlertThrottle('1h')).toEqual('1h'); + }); + + test('"1d" throttle returns "1d" alert throttle', () => { + expect(transformToAlertThrottle('1d')).toEqual('1d'); + }); + + test('"7d" throttle returns "7d" alert throttle', () => { + expect(transformToAlertThrottle('7d')).toEqual('7d'); + }); + }); + + describe('#transformFromAlertThrottle', () => { + test('muteAll returns "NOTIFICATION_THROTTLE_NO_ACTIONS" even with notifyWhen set and actions has an array element', () => { + expect( + transformFromAlertThrottle({ + muteAll: true, + notifyWhen: 'onActiveAlert', + actions: [ + { + group: 'group', + id: 'id-123', + actionTypeId: 'id-456', + params: {}, + }, + ], + } as SanitizedAlert) + ).toEqual(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + test('returns "NOTIFICATION_THROTTLE_NO_ACTIONS" if actions is an empty array and we do not have a throttle', () => { + expect( + transformFromAlertThrottle(({ + muteAll: false, + notifyWhen: 'onActiveAlert', + actions: [], + } as unknown) as SanitizedAlert) + ).toEqual(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + test('returns "NOTIFICATION_THROTTLE_NO_ACTIONS" if actions is an empty array and we have a throttle', () => { + expect( + transformFromAlertThrottle(({ + muteAll: false, + notifyWhen: 'onThrottleInterval', + actions: [], + throttle: '1d', + } as unknown) as SanitizedAlert) + ).toEqual(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + test('it returns "NOTIFICATION_THROTTLE_RULE" if "notifyWhen" is set, muteAll is false and we have an actions array', () => { + expect( + transformFromAlertThrottle({ + muteAll: false, + notifyWhen: 'onActiveAlert', + actions: [ + { + group: 'group', + id: 'id-123', + actionTypeId: 'id-456', + params: {}, + }, + ], + } as SanitizedAlert) + ).toEqual(NOTIFICATION_THROTTLE_RULE); + }); + + test('it returns "NOTIFICATION_THROTTLE_RULE" if "notifyWhen" and "throttle" are not set, but we have an actions array', () => { + expect( + transformFromAlertThrottle({ + muteAll: false, + actions: [ + { + group: 'group', + id: 'id-123', + actionTypeId: 'id-456', + params: {}, + }, + ], + } as SanitizedAlert) + ).toEqual(NOTIFICATION_THROTTLE_RULE); + }); + }); }); 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 2db600415bccd..3d3dfbb60a11b 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 @@ -174,7 +174,12 @@ export const calculateName = ({ } }; -// TODO: Write unit tests +/** + * Given a throttle from a "security_solution" rule this will transform it into an "alerting" notifyWhen + * on their saved object. + * @params throttle The throttle from a "security_solution" rule + * @returns The correct "NotifyWhen" for a Kibana alerting. + */ export const transformToNotifyWhen = (throttle: string | null | undefined): NotifyWhen | null => { if (throttle == null || throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { return null; // Although I return null, this does not change the value of the "notifyWhen" and it keeps the current value of "notifyWhen" @@ -185,7 +190,12 @@ export const transformToNotifyWhen = (throttle: string | null | undefined): Noti } }; -// TODO: Write unit tests +/** + * Given a throttle from a "security_solution" rule this will transform it into an "alerting" "throttle" + * on their saved object. + * @params throttle The throttle from a "security_solution" rule + * @returns The "alerting" throttle + */ export const transformToAlertThrottle = (throttle: string | null | undefined): string | null => { if ( throttle == null || @@ -198,15 +208,20 @@ export const transformToAlertThrottle = (throttle: string | null | undefined): s } }; -// TODO: Write unit tests +/** + * Given a throttle from an "alerting" Saved Object (SO) this will transform it into a "security_solution" + * throttle type. + * @params throttle The throttle from a "alerting" Saved Object (SO) + * @returns The "security_solution" throttle + */ export const transformFromAlertThrottle = (rule: SanitizedAlert): string => { - if (rule.muteAll === true) { + if (rule.muteAll || rule.actions.length === 0) { return NOTIFICATION_THROTTLE_NO_ACTIONS; } else if ( - (rule.notifyWhen === 'onActiveAlert' && rule.actions.length !== 0) || - (rule.throttle == null && rule.notifyWhen == null && rule.actions.length !== 0) + rule.notifyWhen === 'onActiveAlert' || + (rule.throttle == null && rule.notifyWhen == null) ) { - return 'rule'; + return NOTIFICATION_THROTTLE_RULE; } else if (rule.throttle == null) { return NOTIFICATION_THROTTLE_NO_ACTIONS; } else { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index 6b5a807b6a153..37db1368baeae 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -19,7 +19,6 @@ import { SIGNALS_ID, DEFAULT_SEARCH_AFTER_PAGE_SIZE, SERVER_APP_ID, - DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, } from '../../../../common/constants'; import { isMlRule } from '../../../../common/machine_learning/helpers'; import { @@ -72,8 +71,8 @@ import { ExperimentalFeatures } from '../../../../common/experimental_features'; import { injectReferences, extractReferences } from './saved_object_references'; import { RuleExecutionLogClient } from '../rule_execution_log/rule_execution_log_client'; import { IRuleDataPluginService } from '../rule_execution_log/types'; -import { getSignals } from '../notifications/get_signals'; import { RuleExecutionStatus } from '../../../../common/detection_engine/schemas/common/schemas'; +import { scheduleThrottledNotificationActions } from '../notifications/schedule_throttle_notification_actions'; export const signalRulesAlertType = ({ logger, @@ -408,46 +407,27 @@ export const signalRulesAlertType = ({ ); if (savedObject.attributes.throttle != null) { - const fromInMsThrottle = parseScheduleDates(`now-${savedObject.attributes.throttle}`); - const toInMsThrottle = parseScheduleDates(startedAt.toISOString()); - if (fromInMsThrottle != null && toInMsThrottle != null) { - const resultsLinkThrottle = getNotificationResultsLink({ - from: fromInMsThrottle.toISOString(), - to: toInMsThrottle.toISOString(), - id: savedObject.id, - kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined) - ?.kibana_siem_app_url, - }); - const results = await getSignals({ - from: `${fromInMsThrottle.valueOf()}`, - to: `${toInMsThrottle.valueOf()}`, - size: DEFAULT_RULE_NOTIFICATION_QUERY_SIZE, - index: outputIndex, - ruleId, - esClient: services.scopedClusterClient.asCurrentUser, - }); - if (results.hits.hits.length !== 0) { - const alertInstance = services.alertInstanceFactory(alertId); - scheduleNotificationActions({ - alertInstance, - signalsCount: result.createdSignalsCount, - signals: result.createdSignals, - resultsLink: resultsLinkThrottle, - ruleParams: notificationRuleParams, - }); - } - } - } else { - if (result.createdSignalsCount) { - const alertInstance = services.alertInstanceFactory(alertId); - scheduleNotificationActions({ - alertInstance, - signalsCount: result.createdSignalsCount, - signals: result.createdSignals, - resultsLink, - ruleParams: notificationRuleParams, - }); - } + scheduleThrottledNotificationActions({ + alertInstance: services.alertInstanceFactory(alertId), + throttle: savedObject.attributes.throttle, + startedAt, + id: savedObject.id, + kibanaSiemAppUrl: (meta as { kibana_siem_app_url?: string } | undefined) + ?.kibana_siem_app_url, + outputIndex, + ruleId, + esClient: services.scopedClusterClient.asCurrentUser, + notificationRuleParams, + }); + } else if (result.createdSignalsCount) { + const alertInstance = services.alertInstanceFactory(alertId); + scheduleNotificationActions({ + alertInstance, + signalsCount: result.createdSignalsCount, + signals: result.createdSignals, + resultsLink, + ruleParams: notificationRuleParams, + }); } } From d9d65e72271374603b5606feea3fc4044ded4968 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Tue, 24 Aug 2021 18:00:45 -0600 Subject: [PATCH 06/10] Cleaned up a maybe method and fixed tests around it --- .../rules/add_prepackaged_rules_route.test.ts | 3 ++ .../routes/rules/import_rules_route.test.ts | 2 +- .../rules/patch_rules.test.ts | 22 ++++++++++++++- .../lib/detection_engine/rules/patch_rules.ts | 15 ++-------- .../rules/update_rules.test.ts | 10 +++++++ .../detection_engine/rules/update_rules.ts | 19 ++++++------- .../lib/detection_engine/rules/utils.ts | 28 +++++++++++++++++++ 7 files changed, 74 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts index 102d799984d15..189173f44a295 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/add_prepackaged_rules_route.test.ts @@ -9,6 +9,7 @@ import { getEmptyFindResult, addPrepackagedRulesRequest, getFindResultWithSingleHit, + getAlertMock, } from '../__mocks__/request_responses'; import { requestContextMock, serverMock, createMockConfig, mockGetCurrentUser } from '../__mocks__'; import { AddPrepackagedRulesSchemaDecoded } from '../../../../../common/detection_engine/schemas/request/add_prepackaged_rules_schema'; @@ -21,6 +22,7 @@ import { ExceptionListClient } from '../../../../../../lists/server'; import { installPrepackagedTimelines } from '../../../timeline/routes/prepackaged_timelines/install_prepackaged_timelines'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; +import { getQueryRuleParams } from '../../schemas/rule_schemas.mock'; jest.mock('../../rules/get_prepackaged_rules', () => { return { @@ -90,6 +92,7 @@ describe('add_prepackaged_rules_route', () => { mockExceptionsClient = listMock.getExceptionListClient(); clients.rulesClient.find.mockResolvedValue(getFindResultWithSingleHit()); + clients.rulesClient.update.mockResolvedValue(getAlertMock(getQueryRuleParams())); (installPrepackagedTimelines as jest.Mock).mockReset(); (installPrepackagedTimelines as jest.Mock).mockResolvedValue({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts index 210a065012d03..cd572894f551e 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/routes/rules/import_rules_route.test.ts @@ -45,7 +45,7 @@ describe('import_rules_route', () => { ml = mlServicesMock.createSetupContract(); clients.rulesClient.find.mockResolvedValue(getEmptyFindResult()); // no extant rules - + clients.rulesClient.update.mockResolvedValue(getAlertMock(getQueryRuleParams())); context.core.elasticsearch.client.asCurrentUser.search.mockResolvedValue( elasticsearchClientMock.createSuccessTransportRequestPromise({ _shards: { total: 1 } }) ); diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.test.ts index 1bd2656e41bae..dbfc1427abf95 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.test.ts @@ -8,6 +8,9 @@ import { patchRules } from './patch_rules'; import { getPatchRulesOptionsMock, getPatchMlRulesOptionsMock } from './patch_rules.mock'; import { PatchRulesOptions } from './types'; +import { RulesClientMock } from '../../../../../alerting/server/rules_client.mock'; +import { getAlertMock } from '../routes/__mocks__/request_responses'; +import { getQueryRuleParams } from '../schemas/rule_schemas.mock'; describe('patchRules', () => { it('should call rulesClient.disable if the rule was enabled and enabled is false', async () => { @@ -16,6 +19,9 @@ describe('patchRules', () => { ...rulesOptionsMock, enabled: false, }; + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.disable).toHaveBeenCalledWith( expect.objectContaining({ @@ -33,6 +39,9 @@ describe('patchRules', () => { if (ruleOptions.rule != null) { ruleOptions.rule.enabled = false; } + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.enable).toHaveBeenCalledWith( expect.objectContaining({ @@ -50,6 +59,9 @@ describe('patchRules', () => { if (ruleOptions.rule != null) { ruleOptions.rule.enabled = false; } + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith( expect.objectContaining({ @@ -73,6 +85,9 @@ describe('patchRules', () => { if (ruleOptions.rule != null) { ruleOptions.rule.enabled = false; } + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith( expect.objectContaining({ @@ -102,6 +117,9 @@ describe('patchRules', () => { }, ], }; + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith( expect.objectContaining({ @@ -136,7 +154,9 @@ describe('patchRules', () => { }, ]; } - + ((ruleOptions.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await patchRules(ruleOptions); expect(ruleOptions.rulesClient.update).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts index 221fb9b3d6492..bc1faa5dff470 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/patch_rules.ts @@ -7,7 +7,6 @@ import { validate } from '@kbn/securitysolution-io-ts-utils'; import { defaults } from 'lodash/fp'; -import { NOTIFICATION_THROTTLE_NO_ACTIONS } from '../../../../common/constants'; import { PartialAlert } from '../../../../../alerting/server'; import { transformRuleToAlertAction } from '../../../../common/detection_engine/transform_actions'; import { @@ -22,6 +21,7 @@ import { calculateInterval, calculateName, calculateVersion, + maybeMute, removeUndefined, transformToAlertThrottle, transformToNotifyWhen, @@ -35,8 +35,6 @@ class PatchError extends Error { } } -/* eslint complexity: ["error", 21]*/ - export const patchRules = async ({ rulesClient, author, @@ -210,15 +208,8 @@ export const patchRules = async ({ data: validated, }); - // mutes or un-mutes the rule actions depending on the throttle - if (rule.muteAll && throttle !== undefined && throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) { - await rulesClient.unmuteAll({ id: update.id }); - } else if ( - !rule.muteAll && - throttle !== undefined && - throttle === NOTIFICATION_THROTTLE_NO_ACTIONS - ) { - await rulesClient.muteAll({ id: update.id }); + if (throttle !== undefined) { + await maybeMute({ rulesClient, muteAll: rule.muteAll, throttle, id: update.id }); } if (rule.enabled && enabled === false) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.test.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.test.ts index 7d04d3412899d..e46b4fad63a92 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.test.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/update_rules.test.ts @@ -18,6 +18,9 @@ describe('updateRules', () => { ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).get.mockResolvedValue( getAlertMock(getQueryRuleParams()) ); + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await updateRules(rulesOptionsMock); @@ -36,6 +39,9 @@ describe('updateRules', () => { ...getAlertMock(getQueryRuleParams()), enabled: false, }); + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getQueryRuleParams()) + ); await updateRules(rulesOptionsMock); @@ -50,6 +56,10 @@ describe('updateRules', () => { const rulesOptionsMock = getUpdateMlRulesOptionsMock(); rulesOptionsMock.ruleUpdate.enabled = true; + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).update.mockResolvedValue( + getAlertMock(getMlRuleParams()) + ); + ((rulesOptionsMock.rulesClient as unknown) as RulesClientMock).get.mockResolvedValue( getAlertMock(getMlRuleParams()) ); 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 253c3ba70b105..a3e0ba31f0c3c 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 @@ -7,10 +7,7 @@ /* eslint-disable complexity */ -import { - DEFAULT_MAX_SIGNALS, - NOTIFICATION_THROTTLE_NO_ACTIONS, -} from '../../../../common/constants'; +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'; @@ -19,7 +16,7 @@ import { addTags } from './add_tags'; import { typeSpecificSnakeToCamel } from '../schemas/rule_converters'; import { InternalRuleUpdate, RuleParams } from '../schemas/rule_schemas'; import { enableRule } from './enable_rule'; -import { transformToAlertThrottle, transformToNotifyWhen } from './utils'; +import { maybeMute, transformToAlertThrottle, transformToNotifyWhen } from './utils'; export const updateRules = async ({ spaceId, @@ -87,12 +84,12 @@ export const updateRules = async ({ data: newInternalRule, }); - // mutes or unmutes the rule actions depending on the throttle - if (existingRule.muteAll && ruleUpdate.throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) { - await rulesClient.unmuteAll({ id: update.id }); - } else if (!existingRule.muteAll && ruleUpdate.throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { - await rulesClient.muteAll({ id: update.id }); - } + await maybeMute({ + rulesClient, + muteAll: existingRule.muteAll, + throttle: ruleUpdate.throttle, + id: update.id, + }); if (existingRule.enabled && enabled === false) { await rulesClient.disable({ id: existingRule.id }); 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 3d3dfbb60a11b..5a57b8590ae51 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 @@ -59,6 +59,7 @@ import { NOTIFICATION_THROTTLE_NO_ACTIONS, NOTIFICATION_THROTTLE_RULE, } from '../../../../common/constants'; +import { RulesClient } from '../../../../../alerting/server'; export const calculateInterval = ( interval: string | undefined, @@ -228,3 +229,30 @@ export const transformFromAlertThrottle = (rule: SanitizedAlert): st return rule.throttle; } }; + +/** + * Mutes, unmutes, or does nothing to the alert if no changed is detected + * @param id The id of the alert to (un)mute + * @param rulesClient the rules client + * @param muteAll If the existing alert has all actions muted + * @param throttle If the existing alert has a throttle set + */ +export const maybeMute = async ({ + id, + rulesClient, + muteAll, + throttle, +}: { + id: SanitizedAlert['id']; + rulesClient: RulesClient; + muteAll: SanitizedAlert['muteAll']; + throttle: string | null | undefined; +}): Promise => { + if (muteAll && throttle !== NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.unmuteAll({ id }); + } else if (!muteAll && throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { + await rulesClient.muteAll({ id }); + } else { + // Do nothing, no-operation + } +}; From 88a119bc31550882e6b54868c47c51cbaa86b99e Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Wed, 25 Aug 2021 14:34:14 -0600 Subject: [PATCH 07/10] Updated from PR review --- .../rule_types/create_security_rule_type_factory.ts | 2 +- .../lib/detection_engine/signals/signal_rule_alert_type.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts index d9f36f1f69ccf..879d776f83df2 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_types/create_security_rule_type_factory.ts @@ -280,7 +280,7 @@ export const createSecurityRuleTypeFactory: CreateSecurityRuleTypeFactory = ({ logger.info(buildRuleMessage(`Found ${createdSignalsCount} signals for notification.`)); if (ruleSO.attributes.throttle != null) { - scheduleThrottledNotificationActions({ + await scheduleThrottledNotificationActions({ alertInstance: services.alertInstanceFactory(alertId), throttle: ruleSO.attributes.throttle, startedAt, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts index 37db1368baeae..1c4efea0a1d59 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/signals/signal_rule_alert_type.ts @@ -407,7 +407,7 @@ export const signalRulesAlertType = ({ ); if (savedObject.attributes.throttle != null) { - scheduleThrottledNotificationActions({ + await scheduleThrottledNotificationActions({ alertInstance: services.alertInstanceFactory(alertId), throttle: savedObject.attributes.throttle, startedAt, From 81f8116ce3d5af325ae919978e1c5938a7cbdc9e Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Wed, 25 Aug 2021 19:14:00 -0600 Subject: [PATCH 08/10] Added e2e tests for the conditions of throttle --- .../security_and_spaces/tests/index.ts | 1 + .../security_and_spaces/tests/throttle.ts | 357 ++++++++++++++++++ 2 files changed, 358 insertions(+) create mode 100644 x-pack/test/detection_engine_api_integration/security_and_spaces/tests/throttle.ts diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts index 2e3469520989d..27474fe563a36 100644 --- a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/index.ts @@ -47,6 +47,7 @@ export default ({ loadTestFile }: FtrProviderContext): void => { loadTestFile(require.resolve('./delete_signals_migrations')); loadTestFile(require.resolve('./timestamps')); loadTestFile(require.resolve('./runtime')); + loadTestFile(require.resolve('./throttle')); }); // That split here enable us on using a different ciGroup to run the tests diff --git a/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/throttle.ts b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/throttle.ts new file mode 100644 index 0000000000000..f0fef839482ce --- /dev/null +++ b/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/throttle.ts @@ -0,0 +1,357 @@ +/* + * 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 expect from '@kbn/expect'; + +import { CreateRulesSchema } from '../../../../plugins/security_solution/common/detection_engine/schemas/request'; +import { + DETECTION_ENGINE_RULES_URL, + NOTIFICATION_THROTTLE_NO_ACTIONS, + NOTIFICATION_THROTTLE_RULE, +} from '../../../../plugins/security_solution/common/constants'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { + createSignalsIndex, + deleteAllAlerts, + deleteSignalsIndex, + getWebHookAction, + getRuleWithWebHookAction, + createRule, + getSimpleRule, + getRule, + updateRule, +} from '../../utils'; + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertest = getService('supertest'); + + /** + * + * These tests will ensure that the existing synchronization between the alerting API and its states of: + * - "notifyWhen" + * - "muteAll" + * - "throttle" + * Work within the security_solution's API and states of "throttle" which currently not a 1 to 1 relationship: + * + * Ref: + * https://www.elastic.co/guide/en/kibana/master/create-and-manage-rules.html#controlling-rules + * https://www.elastic.co/guide/en/kibana/current/mute-all-alerts-api.html + * https://www.elastic.co/guide/en/security/current/rules-api-create.html + */ + describe('throttle', () => { + describe('adding actions', () => { + beforeEach(async () => { + await createSignalsIndex(supertest); + }); + + afterEach(async () => { + await deleteSignalsIndex(supertest); + await deleteAllAlerts(supertest); + }); + + describe('creating a rule', () => { + it('When creating a new action and attaching it to a rule, the rule should have its kibana alerting "mute_all" set to "false" and notify_when set to "onActiveAlert"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const rule = await createRule(supertest, getRuleWithWebHookAction(hookAction.id)); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + it('When creating throttle with "NOTIFICATION_THROTTLE_NO_ACTIONS" set and no actions, the rule should have its kibana alerting "mute_all" set to "true" and notify_when set to "onActiveAlert"', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: NOTIFICATION_THROTTLE_NO_ACTIONS, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(true); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + it('When creating throttle with "NOTIFICATION_THROTTLE_NO_ACTIONS" set and with actions set, the rule should have its kibana alerting "mute_all" set to "true" and notify_when set to "onActiveAlert"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithThrottle: CreateRulesSchema = { + ...getRuleWithWebHookAction(hookAction.id), + throttle: NOTIFICATION_THROTTLE_NO_ACTIONS, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(true); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + it('When creating throttle with "NOTIFICATION_THROTTLE_RULE" set and no actions, the rule should have its kibana alerting "mute_all" set to "false" and notify_when set to "onActiveAlert"', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: NOTIFICATION_THROTTLE_RULE, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + // NOTE: This shows A side effect of how we do not set data on side cars anymore where the user is told they have no actions since the array is empty. + it('When creating throttle with "NOTIFICATION_THROTTLE_RULE" set and no actions, since we do not have any actions, we should get back a throttle of "NOTIFICATION_THROTTLE_NO_ACTIONS"', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: NOTIFICATION_THROTTLE_RULE, + }; + const rule = await createRule(supertest, ruleWithThrottle); + expect(rule.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + it('When creating throttle with "NOTIFICATION_THROTTLE_RULE" set and actions set, the rule should have its kibana alerting "mute_all" set to "false" and notify_when set to "onActiveAlert"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithThrottle: CreateRulesSchema = { + ...getRuleWithWebHookAction(hookAction.id), + throttle: NOTIFICATION_THROTTLE_RULE, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + it('When creating throttle with "1h" set and no actions, the rule should have its kibana alerting "mute_all" set to "false" and notify_when set to "onThrottleInterval"', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: '1h', + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onThrottleInterval'); + }); + + it('When creating throttle with "1h" set and actions set, the rule should have its kibana alerting "mute_all" set to "false" and notify_when set to "onThrottleInterval"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithThrottle: CreateRulesSchema = { + ...getRuleWithWebHookAction(hookAction.id), + throttle: '1h', + }; + const rule = await createRule(supertest, ruleWithThrottle); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onThrottleInterval'); + }); + }); + + describe('reading a rule', () => { + it('When creating a new action and attaching it to a rule, we should return "NOTIFICATION_THROTTLE_RULE" when doing a read', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const rule = await createRule(supertest, getRuleWithWebHookAction(hookAction.id)); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_RULE); + }); + + it('When creating throttle with "NOTIFICATION_THROTTLE_NO_ACTIONS" set and no actions, we should return "NOTIFICATION_THROTTLE_NO_ACTIONS" when doing a read', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: NOTIFICATION_THROTTLE_NO_ACTIONS, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + // NOTE: This shows A side effect of how we do not set data on side cars anymore where the user is told they have no actions since the array is empty. + it('When creating throttle with "NOTIFICATION_THROTTLE_RULE" set and no actions, since we do not have any actions, we should get back a throttle of "NOTIFICATION_THROTTLE_NO_ACTIONS" when doing a read', async () => { + const ruleWithThrottle: CreateRulesSchema = { + ...getSimpleRule(), + throttle: NOTIFICATION_THROTTLE_RULE, + }; + const rule = await createRule(supertest, ruleWithThrottle); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + + it('When creating a new action and attaching it to a rule, if we change the alert to a "muteAll" through the kibana alerting API, we should get back "NOTIFICATION_THROTTLE_NO_ACTIONS" ', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const rule = await createRule(supertest, getRuleWithWebHookAction(hookAction.id)); + await supertest + .post(`/api/alerting/rule/${rule.id}/_mute_all`) + .set('kbn-xsrf', 'true') + .send() + .expect(204); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + }); + + describe('updating a rule', () => { + it('will not change "NOTIFICATION_THROTTLE_RULE" if we update some part of the rule', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + await createRule(supertest, ruleWithWebHookAction); + ruleWithWebHookAction.name = 'some other name'; + const updated = await updateRule(supertest, ruleWithWebHookAction); + expect(updated.throttle).to.eql(NOTIFICATION_THROTTLE_RULE); + }); + + it('will not change the "muteAll" or "notifyWhen" if we update some part of the rule', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + await createRule(supertest, ruleWithWebHookAction); + ruleWithWebHookAction.name = 'some other name'; + const updated = await updateRule(supertest, ruleWithWebHookAction); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${updated.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + // NOTE: This shows A side effect of how we do not set data on side cars anymore where the user is told they have no actions since the array is empty. + it('If we update a rule and remove just the actions array it will begin returning a throttle of "NOTIFICATION_THROTTLE_NO_ACTIONS"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + await createRule(supertest, ruleWithWebHookAction); + ruleWithWebHookAction.actions = []; + const updated = await updateRule(supertest, ruleWithWebHookAction); + expect(updated.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + }); + + describe('patching a rule', () => { + it('will not change "NOTIFICATION_THROTTLE_RULE" if we patch some part of the rule', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + const rule = await createRule(supertest, ruleWithWebHookAction); + // patch a simple rule's name + await supertest + .patch(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send({ rule_id: rule.rule_id, name: 'some other name' }) + .expect(200); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_RULE); + }); + + it('will not change the "muteAll" or "notifyWhen" if we patch part of the rule', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + const rule = await createRule(supertest, ruleWithWebHookAction); + // patch a simple rule's name + await supertest + .patch(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send({ rule_id: rule.rule_id, name: 'some other name' }) + .expect(200); + const { + body: { mute_all: muteAll, notify_when: notifyWhen }, + } = await supertest.get(`/api/alerting/rule/${rule.id}`); + expect(muteAll).to.eql(false); + expect(notifyWhen).to.eql('onActiveAlert'); + }); + + // NOTE: This shows A side effect of how we do not set data on side cars anymore where the user is told they have no actions since the array is empty. + it('If we patch a rule and remove just the actions array it will begin returning a throttle of "NOTIFICATION_THROTTLE_NO_ACTIONS"', async () => { + // create a new action + const { body: hookAction } = await supertest + .post('/api/actions/action') + .set('kbn-xsrf', 'true') + .send(getWebHookAction()) + .expect(200); + + const ruleWithWebHookAction = getRuleWithWebHookAction(hookAction.id); + const rule = await createRule(supertest, ruleWithWebHookAction); + // patch a simple rule's action + await supertest + .patch(DETECTION_ENGINE_RULES_URL) + .set('kbn-xsrf', 'true') + .send({ rule_id: rule.rule_id, actions: [] }) + .expect(200); + const readRule = await getRule(supertest, rule.rule_id); + expect(readRule.throttle).to.eql(NOTIFICATION_THROTTLE_NO_ACTIONS); + }); + }); + }); + }); +}; From d1afb4fbe97ca17090ce465ad95a2f9fa9a72f1f Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Wed, 25 Aug 2021 19:18:47 -0600 Subject: [PATCH 09/10] Fixes from PR comments --- .../server/lib/detection_engine/rules/utils.ts | 8 +++++--- .../lib/detection_engine/schemas/rule_converters.ts | 2 +- .../server/lib/detection_engine/schemas/rule_schemas.ts | 2 -- 3 files changed, 6 insertions(+), 6 deletions(-) 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 5a57b8590ae51..d9d5151a64c46 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 @@ -27,7 +27,7 @@ import type { } from '@kbn/securitysolution-io-ts-alerting-types'; import type { ListArrayOrUndefined } from '@kbn/securitysolution-io-ts-list-types'; import type { VersionOrUndefined } from '@kbn/securitysolution-io-ts-types'; -import { SanitizedAlert } from '../../../../../alerting/common'; +import { AlertNotifyWhenType, SanitizedAlert } from '../../../../../alerting/common'; import { DescriptionOrUndefined, AnomalyThresholdOrUndefined, @@ -54,7 +54,7 @@ import { EventCategoryOverrideOrUndefined, } from '../../../../common/detection_engine/schemas/common/schemas'; import { PartialFilter } from '../types'; -import { NotifyWhen, RuleParams } from '../schemas/rule_schemas'; +import { RuleParams } from '../schemas/rule_schemas'; import { NOTIFICATION_THROTTLE_NO_ACTIONS, NOTIFICATION_THROTTLE_RULE, @@ -181,7 +181,9 @@ export const calculateName = ({ * @params throttle The throttle from a "security_solution" rule * @returns The correct "NotifyWhen" for a Kibana alerting. */ -export const transformToNotifyWhen = (throttle: string | null | undefined): NotifyWhen | null => { +export const transformToNotifyWhen = ( + throttle: string | null | undefined +): AlertNotifyWhenType | null => { if (throttle == null || throttle === NOTIFICATION_THROTTLE_NO_ACTIONS) { return null; // Although I return null, this does not change the value of the "notifyWhen" and it keeps the current value of "notifyWhen" } else if (throttle === NOTIFICATION_THROTTLE_RULE) { diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts index d79bbd79fde5a..8a67636c6649d 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_converters.ts @@ -160,7 +160,7 @@ export const convertCreateAPIToInternalSchema = ( }, schedule: { interval: input.interval ?? '5m' }, enabled: input.enabled ?? true, - actions: input.actions != null ? input.actions.map(transformRuleToAlertAction) : [], + actions: input.actions?.map(transformRuleToAlertAction) ?? [], throttle: transformToAlertThrottle(input.throttle), notifyWhen: transformToNotifyWhen(input.throttle), }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts index 76d03f0ff45f0..c414ecc8655a3 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/schemas/rule_schemas.ts @@ -196,8 +196,6 @@ export const notifyWhen = t.union([ t.null, ]); -export type NotifyWhen = t.TypeOf; - export const internalRuleCreate = t.type({ name, tags, From e4a3d9bb5e0ceeb2d09d9f13f65c240098922120 Mon Sep 17 00:00:00 2001 From: FrankHassanabad Date: Thu, 26 Aug 2021 09:13:51 -0600 Subject: [PATCH 10/10] Removes actions from the export again. --- .../server/lib/detection_engine/rules/get_export_all.ts | 4 +++- .../lib/detection_engine/rules/get_export_by_object_ids.ts | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_all.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_all.ts index 9ec51cf18c7c7..4a79f0089491f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_all.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_all.ts @@ -19,7 +19,9 @@ export const getExportAll = async ( }> => { const ruleAlertTypes = await getNonPackagedRules({ rulesClient }); const rules = transformAlertsToRules(ruleAlertTypes); - const rulesNdjson = transformDataToNdjson(rules); + // We do not support importing/exporting actions. When we do, delete this line of code + const rulesWithoutActions = rules.map((rule) => ({ ...rule, actions: [] })); + const rulesNdjson = transformDataToNdjson(rulesWithoutActions); const exportDetails = getExportDetailsNdjson(rules); return { rulesNdjson, exportDetails }; }; diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts index 5d33e37c2ecf9..812310bcb501a 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rules/get_export_by_object_ids.ts @@ -40,8 +40,10 @@ export const getExportByObjectIds = async ( exportDetails: string; }> => { const rulesAndErrors = await getRulesFromObjects(rulesClient, objects); - const rulesNdjson = transformDataToNdjson(rulesAndErrors.rules); - const exportDetails = getExportDetailsNdjson(rulesAndErrors.rules, rulesAndErrors.missingRules); + // We do not support importing/exporting actions. When we do, delete this line of code + const rulesWithoutActions = rulesAndErrors.rules.map((rule) => ({ ...rule, actions: [] })); + const rulesNdjson = transformDataToNdjson(rulesWithoutActions); + const exportDetails = getExportDetailsNdjson(rulesWithoutActions, rulesAndErrors.missingRules); return { rulesNdjson, exportDetails }; };