Skip to content

Commit

Permalink
[RAM][SECURITYSOLUTION][ALERTS] - Integrate Alert summary inside of s…
Browse files Browse the repository at this point in the history
…ecurity solution rule page (#154990)

## Summary

[Main ticket](#151916)
This PR dependant on [these
changes](#153101)

These changes cover next two tickets:
- [RAM][SECURITYSOLUTION][ALERTS] - Integrate per-action frequency field
in security solution APIs #154532
- [RAM][SECURITYSOLUTION][ALERTS] - Integrate per-action frequency UI in
security solution #154534

With this PR we will integrate per-action `frequency` field which
already works within alert framework and will update security solution
UI to incorporate the possibility to select "summary" vs "for each
alert" type of actions.



![](https://user-images.githubusercontent.com/616158/227377473-f34a330e-81ce-42b4-af1b-e6e302c6319d.png)


## NOTES:
- There will be no more "Perform no actions" option which mutes all the
actions of the rule. For back compatibility, we need to show that rule
is muted in the UI cc @peluja1012 @ARWNightingale
- The ability to generate per-alert action will be done as part of
#153611


## Technical Notes:
Here are the overview of the conversions and transformations that we are
going to do as part of these changes for devs who are going to review.

On rule **create**/**read**/**update**/**patch**:
- We always gonna set rule level `throttle` to `undefined` from now on
- If each action has `frequency` attribute set, then we just use those
values
- If actions do not have `frequency` attribute set (or for some reason
there is a mix of actions with some of them having `frequency` attribute
and some not), then we transform rule level `throttle` into `frequency`
and set it for each action in the rule

On rule **bulk editing**:
- We always gonna set rule level `throttle` to `undefined`
- If each action has `frequency` attribute set, then we just use those
values
- If actions do not have `frequency` attribute set, then we transform
rule level `throttle` into `frequency` and set it for each action in the
rule
- If user passed only `throttle` attribute with empty actions (`actions
= []`), this will only remove all actions from the rule

This will bring breaking changes which we agreed on with the Advanced
Correlation Group
cc @XavierM @vitaliidm @peluja1012 


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Maxim Palenov <[email protected]>
  • Loading branch information
3 people authored Apr 23, 2023
1 parent 118daf7 commit 68719bd
Show file tree
Hide file tree
Showing 81 changed files with 1,720 additions and 672 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export * from './src/default_severity_mapping_array';
export * from './src/default_threat_array';
export * from './src/default_to_string';
export * from './src/default_uuid';
export * from './src/frequency';
export * from './src/language';
export * from './src/machine_learning_job_id';
export * from './src/max_signals';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { NonEmptyString } from '@kbn/securitysolution-io-ts-types';

import * as t from 'io-ts';
import { saved_object_attributes } from '../saved_object_attributes';
import { RuleActionFrequency } from '../frequency';

export type RuleActionGroup = t.TypeOf<typeof RuleActionGroup>;
export const RuleActionGroup = t.string;
Expand Down Expand Up @@ -94,7 +95,11 @@ export const RuleAction = t.exact(
action_type_id: RuleActionTypeId,
params: RuleActionParams,
}),
t.partial({ uuid: RuleActionUuid, alerts_filter: RuleActionAlertsFilter }),
t.partial({
uuid: RuleActionUuid,
alerts_filter: RuleActionAlertsFilter,
frequency: RuleActionFrequency,
}),
])
);

Expand All @@ -110,7 +115,11 @@ export const RuleActionCamel = t.exact(
actionTypeId: RuleActionTypeId,
params: RuleActionParams,
}),
t.partial({ uuid: RuleActionUuid, alertsFilter: RuleActionAlertsFilter }),
t.partial({
uuid: RuleActionUuid,
alertsFilter: RuleActionAlertsFilter,
frequency: RuleActionFrequency,
}),
])
);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import * as t from 'io-ts';

import { RuleActionThrottle } from '../throttle';

/**
* Action summary indicates whether we will send a summary notification about all the generate alerts or notification per individual alert
*/
export type RuleActionSummary = t.TypeOf<typeof RuleActionSummary>;
export const RuleActionSummary = t.boolean;

/**
* The condition for throttling the notification: `onActionGroupChange`, `onActiveAlert`, or `onThrottleInterval`
*/
export type RuleActionNotifyWhen = t.TypeOf<typeof RuleActionNotifyWhen>;
export const RuleActionNotifyWhen = t.union([
t.literal('onActionGroupChange'),
t.literal('onActiveAlert'),
t.literal('onThrottleInterval'),
]);

/**
* The action frequency defines when the action runs (for example, only on rule execution or at specific time intervals).
*/
export type RuleActionFrequency = t.TypeOf<typeof RuleActionFrequency>;
export const RuleActionFrequency = t.type({
summary: RuleActionSummary,
notifyWhen: RuleActionNotifyWhen,
throttle: t.union([RuleActionThrottle, t.null]),
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export type RuleActionThrottle = t.TypeOf<typeof RuleActionThrottle>;
export const RuleActionThrottle = t.union([
t.literal('no_actions'),
t.literal('rule'),
TimeDuration({ allowedUnits: ['h', 'd'] }),
TimeDuration({ allowedUnits: ['s', 'm', 'h', 'd'] }),
]);
11 changes: 11 additions & 0 deletions x-pack/plugins/alerting/server/routes/bulk_edit_rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,17 @@ const ruleActionSchema = schema.object({
id: schema.string(),
params: schema.recordOf(schema.string(), schema.any(), { defaultValue: {} }),
uuid: schema.maybe(schema.string()),
frequency: schema.maybe(
schema.object({
summary: schema.boolean(),
throttle: schema.nullable(schema.string()),
notifyWhen: schema.oneOf([
schema.literal('onActionGroupChange'),
schema.literal('onActiveAlert'),
schema.literal('onThrottleInterval'),
]),
})
),
});

const operationsSchema = schema.arrayOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { injectReferencesIntoActions } from '../../common';
import { transformToNotifyWhen } from './transform_to_notify_when';
import { transformFromLegacyActions } from './transform_legacy_actions';
import { LegacyIRuleActionsAttributes, legacyRuleActionsSavedObjectType } from './types';
import { transformToAlertThrottle } from './transform_to_alert_throttle';

/**
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
Expand Down Expand Up @@ -136,7 +137,9 @@ export const formatLegacyActions = async <T extends Rule>(
return {
...rule,
actions: [...rule.actions, ...legacyRuleActions],
throttle: (legacyRuleActions.length ? ruleThrottle : rule.throttle) ?? 'no_actions',
throttle: transformToAlertThrottle(
(legacyRuleActions.length ? ruleThrottle : rule.throttle) ?? 'no_actions'
),
notifyWhen: transformToNotifyWhen(ruleThrottle),
// muteAll property is disregarded in further rule processing in Security Solution when legacy actions are present.
// So it should be safe to set it as false, so it won't be displayed to user as w/o actions see transformFromAlertThrottle method
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ describe('transformFromLegacyActions', () => {
(transformToNotifyWhen as jest.Mock).mockReturnValueOnce(null);
const actions = transformFromLegacyActions(legacyActionsAttr, references);

expect(actions[0].frequency?.notifyWhen).toBe('onThrottleInterval');
expect(actions[0].frequency?.notifyWhen).toBe('onActiveAlert');
});

it('should return transformed legacy actions', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { SavedObjectReference } from '@kbn/core/server';
import { RawRuleAction } from '../../../types';
import { transformToNotifyWhen } from './transform_to_notify_when';
import { LegacyIRuleActionsAttributes } from './types';
import { transformToAlertThrottle } from './transform_to_alert_throttle';

/**
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
Expand Down Expand Up @@ -50,8 +51,8 @@ export const transformFromLegacyActions = (
actionTypeId,
frequency: {
summary: true,
notifyWhen: transformToNotifyWhen(legacyActionsAttr.ruleThrottle) ?? 'onThrottleInterval',
throttle: legacyActionsAttr.ruleThrottle,
notifyWhen: transformToNotifyWhen(legacyActionsAttr.ruleThrottle) ?? 'onActiveAlert',
throttle: transformToAlertThrottle(legacyActionsAttr.ruleThrottle),
},
},
];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/*
* 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 { transformToAlertThrottle } from './transform_to_alert_throttle';

describe('transformToAlertThrottle', () => {
it('should return null when throttle is null OR no_actions', () => {
expect(transformToAlertThrottle(null)).toBeNull();
expect(transformToAlertThrottle('rule')).toBeNull();
expect(transformToAlertThrottle('no_actions')).toBeNull();
});
it('should return same value for other throttle values', () => {
expect(transformToAlertThrottle('1h')).toBe('1h');
expect(transformToAlertThrottle('1m')).toBe('1m');
expect(transformToAlertThrottle('1d')).toBe('1d');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* 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.
*/

/**
* 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 || throttle === 'rule' || throttle === 'no_actions') {
return null;
} else {
return throttle;
}
};
15 changes: 1 addition & 14 deletions x-pack/plugins/alerting/server/rules_client/methods/bulk_edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import pMap from 'p-map';
import Boom from '@hapi/boom';
import { cloneDeep, omit } from 'lodash';
import { cloneDeep } from 'lodash';
import { AlertConsumers } from '@kbn/rule-data-utils';
import { KueryNode, nodeBuilder } from '@kbn/es-query';
import {
Expand Down Expand Up @@ -638,19 +638,6 @@ async function getUpdatedAttributesFromOperations(
isAttributesUpdateSkipped = false;
}

// TODO https://github.com/elastic/kibana/issues/148414
// If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
const firstFrequency = updatedOperation.value.find(
(action) => action?.frequency
)?.frequency;
if (rule.attributes.consumer === AlertConsumers.SIEM && firstFrequency) {
ruleActions.actions = ruleActions.actions.map((action) => omit(action, 'frequency'));
if (!attributes.notifyWhen) {
attributes.notifyWhen = firstFrequency.notifyWhen;
attributes.throttle = firstFrequency.throttle;
}
}

break;
}
case 'snoozeSchedule': {
Expand Down
13 changes: 0 additions & 13 deletions x-pack/plugins/alerting/server/rules_client/methods/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
*/
import Semver from 'semver';
import Boom from '@hapi/boom';
import { omit } from 'lodash';
import { AlertConsumers } from '@kbn/rule-data-utils';
import { SavedObjectsUtils } from '@kbn/core/server';
import { withSpan } from '@kbn/apm-utils';
import { parseDuration } from '../../../common/parse_duration';
Expand Down Expand Up @@ -111,17 +109,6 @@ export async function create<Params extends RuleTypeParams = never>(
throw Boom.badRequest(`Error creating rule: could not create API key - ${error.message}`);
}

// TODO https://github.com/elastic/kibana/issues/148414
// If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
const firstFrequency = data.actions.find((action) => action?.frequency)?.frequency;
if (data.consumer === AlertConsumers.SIEM && firstFrequency) {
data.actions = data.actions.map((action) => omit(action, 'frequency'));
if (!data.notifyWhen) {
data.notifyWhen = firstFrequency.notifyWhen;
data.throttle = firstFrequency.throttle;
}
}

await withSpan({ name: 'validateActions', type: 'rules' }, () =>
validateActions(context, ruleType, data, allowMissingConnectorSecrets)
);
Expand Down
14 changes: 1 addition & 13 deletions x-pack/plugins/alerting/server/rules_client/methods/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
*/

import Boom from '@hapi/boom';
import { isEqual, omit } from 'lodash';
import { isEqual } from 'lodash';
import { SavedObject } from '@kbn/core/server';
import { AlertConsumers } from '@kbn/rule-data-utils';
import type { ShouldIncrementRevision } from './bulk_edit';
import {
PartialRule,
Expand Down Expand Up @@ -186,17 +185,6 @@ async function updateAlert<Params extends RuleTypeParams>(

const ruleType = context.ruleTypeRegistry.get(attributes.alertTypeId);

// TODO https://github.com/elastic/kibana/issues/148414
// If any action-level frequencies get pushed into a SIEM rule, strip their frequencies
const firstFrequency = data.actions.find((action) => action?.frequency)?.frequency;
if (attributes.consumer === AlertConsumers.SIEM && firstFrequency) {
data.actions = data.actions.map((action) => omit(action, 'frequency'));
if (!attributes.notifyWhen) {
attributes.notifyWhen = firstFrequency.notifyWhen;
attributes.throttle = firstFrequency.throttle;
}
}

// Validate
const validatedAlertTypeParams = validateRuleTypeParams(data.params, ruleType.validate.params);
await validateActions(context, ruleType, data, allowMissingConnectorSecrets);
Expand Down
11 changes: 11 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* 2.0.
*/

import { RuleNotifyWhen } from '@kbn/alerting-plugin/common';

/**
* as const
*
Expand Down Expand Up @@ -377,9 +379,18 @@ export const ML_GROUP_ID = 'security' as const;
export const LEGACY_ML_GROUP_ID = 'siem' as const;
export const ML_GROUP_IDS = [ML_GROUP_ID, LEGACY_ML_GROUP_ID] as const;

/**
* Rule Actions
*/
export const NOTIFICATION_THROTTLE_NO_ACTIONS = 'no_actions' as const;
export const NOTIFICATION_THROTTLE_RULE = 'rule' as const;

export const NOTIFICATION_DEFAULT_FREQUENCY = {
notifyWhen: RuleNotifyWhen.ACTIVE,
throttle: null,
summary: true,
};

export const showAllOthersBucket: string[] = [
'destination.ip',
'event.action',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,28 +515,6 @@ describe('Perform bulk action request schema', () => {
expect(message.schema).toEqual({});
});

test('invalid request: missing throttle in payload', () => {
const payload = {
query: 'name: test',
action: BulkActionType.edit,
[BulkActionType.edit]: [
{
type: BulkActionEditType.add_rule_actions,
value: {
actions: [],
},
},
],
};

const message = retrieveValidationMessage(payload);

expect(getPaths(left(message.errors))).toEqual(
expect.arrayContaining(['Invalid value "undefined" supplied to "edit,value,throttle"'])
);
expect(message.schema).toEqual({});
});

test('invalid request: missing actions in payload', () => {
const payload = {
query: 'name: test',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as t from 'io-ts';

import { NonEmptyArray, TimeDuration } from '@kbn/securitysolution-io-ts-types';
import {
RuleActionFrequency,
RuleActionGroup,
RuleActionId,
RuleActionParams,
Expand Down Expand Up @@ -96,11 +97,14 @@ const BulkActionEditPayloadTimeline = t.type({
*/
type NormalizedRuleAction = t.TypeOf<typeof NormalizedRuleAction>;
const NormalizedRuleAction = t.exact(
t.type({
group: RuleActionGroup,
id: RuleActionId,
params: RuleActionParams,
})
t.intersection([
t.type({
group: RuleActionGroup,
id: RuleActionId,
params: RuleActionParams,
}),
t.partial({ frequency: RuleActionFrequency }),
])
);

export type BulkActionEditPayloadRuleActions = t.TypeOf<typeof BulkActionEditPayloadRuleActions>;
Expand All @@ -109,10 +113,12 @@ export const BulkActionEditPayloadRuleActions = t.type({
t.literal(BulkActionEditType.add_rule_actions),
t.literal(BulkActionEditType.set_rule_actions),
]),
value: t.type({
throttle: ThrottleForBulkActions,
actions: t.array(NormalizedRuleAction),
}),
value: t.intersection([
t.partial({ throttle: ThrottleForBulkActions }),
t.type({
actions: t.array(NormalizedRuleAction),
}),
]),
});

type BulkActionEditPayloadSchedule = t.TypeOf<typeof BulkActionEditPayloadSchedule>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ export const baseSchema = buildRuleSchemas({
output_index: AlertsIndex,
namespace: AlertsIndexNamespace,
meta: RuleMetadata,
// Throttle
throttle: RuleActionThrottle,
},
defaultable: {
// Main attributes
Expand All @@ -134,7 +136,6 @@ export const baseSchema = buildRuleSchemas({
to: RuleIntervalTo,
// Rule actions
actions: RuleActionArray,
throttle: RuleActionThrottle,
// Rule exceptions
exceptions_list: ExceptionListArray,
// Misc attributes
Expand Down
Loading

0 comments on commit 68719bd

Please sign in to comment.