Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIEM][Detection Engine] Allow to edit actions for prepackaged rules #61312

Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
78f0ade
[SIEM][Detection Engine] Allow to edit actions for prepackaged rules
patrykkopycinski Mar 25, 2020
5a550ac
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 26, 2020
64cb485
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 27, 2020
9e1f908
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 27, 2020
5c31470
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 27, 2020
cf920ae
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 28, 2020
0297078
WIP
patrykkopycinski Mar 28, 2020
cc72d4d
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 28, 2020
4c3808b
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 29, 2020
59158fb
update routes
patrykkopycinski Mar 29, 2020
dd0105e
cleanup
patrykkopycinski Mar 29, 2020
40ca8f1
fix type
patrykkopycinski Mar 29, 2020
23c3c11
update_rules_notifications
patrykkopycinski Mar 29, 2020
368adf3
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 30, 2020
12c5d37
cleanup
patrykkopycinski Mar 30, 2020
5219ae3
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 30, 2020
e06bba4
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 30, 2020
b3793bf
update integration tests
patrykkopycinski Mar 30, 2020
0133fc4
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 30, 2020
9c641b5
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 30, 2020
61aa3b9
Merge branch 'master' into feat/siem-prepackedrules-actions-edit
elasticmachine Mar 30, 2020
aad8054
Merge branch 'master' into pr/61312
rylnd Mar 30, 2020
6c38780
Merge branch 'master' into pr/61312
rylnd Mar 30, 2020
55212dc
Fix failing integration test
rylnd Mar 31, 2020
7081f28
Merge branch 'feat/siem-prepackedrules-actions-edit' of github.com:pa…
patrykkopycinski Mar 31, 2020
b64ad29
Merge branch 'master' of github.com:elastic/kibana into feat/siem-pre…
patrykkopycinski Mar 31, 2020
485e66f
fix rule enabled
patrykkopycinski Mar 31, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const mockRule = (id: string): Rule => ({
to: 'now',
type: 'saved_query',
threat: [],
throttle: null,
throttle: 'no_actions',
note: '# this is some markdown documentation',
version: 1,
});
Expand Down Expand Up @@ -145,7 +145,7 @@ export const mockRuleWithEverything = (id: string): Rule => ({
],
},
],
throttle: null,
throttle: 'no_actions',
note: '# this is some markdown documentation',
version: 1,
});
Expand Down Expand Up @@ -184,7 +184,7 @@ export const mockActionsStepRule = (isNew = false, enabled = false): ActionsStep
actions: [],
kibanaSiemAppUrl: 'http://localhost:5601/app/siem',
enabled,
throttle: null,
throttle: 'no_actions',
});

export const mockDefineStepRule = (isNew = false): DefineStepRule => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export const getActions = (
icon: 'controlsHorizontal',
name: i18n.EDIT_RULE_SETTINGS,
onClick: (rowItem: Rule) => editRuleAction(rowItem, history),
enabled: (rowItem: Rule) => !rowItem.immutable,
},
{
description: i18n.DUPLICATE_RULE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,9 @@ describe('helpers', () => {
actions: [],
enabled: false,
meta: {
throttle: 'no_actions',
kibanaSiemAppUrl: 'http://localhost:5601/app/siem',
},
throttle: null,
throttle: 'no_actions',
};

expect(result).toEqual(expected);
Expand All @@ -534,10 +533,9 @@ describe('helpers', () => {
actions: [],
enabled: false,
meta: {
throttle: mockStepData.throttle,
kibanaSiemAppUrl: mockStepData.kibanaSiemAppUrl,
},
throttle: null,
throttle: 'no_actions',
};

expect(result).toEqual(expected);
Expand Down Expand Up @@ -568,10 +566,9 @@ describe('helpers', () => {
],
enabled: false,
meta: {
throttle: mockStepData.throttle,
kibanaSiemAppUrl: mockStepData.kibanaSiemAppUrl,
},
throttle: null,
throttle: 'rule',
};

expect(result).toEqual(expected);
Expand Down Expand Up @@ -602,7 +599,6 @@ describe('helpers', () => {
],
enabled: false,
meta: {
throttle: mockStepData.throttle,
kibanaSiemAppUrl: mockStepData.kibanaSiemAppUrl,
},
throttle: mockStepData.throttle,
Expand Down Expand Up @@ -635,10 +631,9 @@ describe('helpers', () => {
],
enabled: false,
meta: {
throttle: null,
kibanaSiemAppUrl: mockStepData.kibanaSiemAppUrl,
},
throttle: null,
throttle: 'no_actions',
};

expect(result).toEqual(expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@ import { has, isEmpty } from 'lodash/fp';
import moment from 'moment';
import deepmerge from 'deepmerge';

import {
NOTIFICATION_THROTTLE_RULE,
NOTIFICATION_THROTTLE_NO_ACTIONS,
} from '../../../../../common/constants';
import { NOTIFICATION_THROTTLE_NO_ACTIONS } from '../../../../../common/constants';
import { NewRule, RuleType } from '../../../../containers/detection_engine/rules';
import { transformAlertToRuleAction } from '../../../../../common/detection_engine/transform_actions';

Expand Down Expand Up @@ -144,11 +141,6 @@ export const formatAboutStepData = (aboutStepData: AboutStepRule): AboutStepRule
};
};

export const getAlertThrottle = (throttle: string | null) =>
throttle && ![NOTIFICATION_THROTTLE_NO_ACTIONS, NOTIFICATION_THROTTLE_RULE].includes(throttle)
? throttle
: null;

export const formatActionsStepData = (actionsStepData: ActionsStepRule): ActionsStepRuleJson => {
const {
actions = [],
Expand All @@ -160,9 +152,8 @@ export const formatActionsStepData = (actionsStepData: ActionsStepRule): Actions
return {
actions: actions.map(transformAlertToRuleAction),
enabled,
throttle: actions.length ? getAlertThrottle(throttle) : null,
throttle: actions.length ? throttle : NOTIFICATION_THROTTLE_NO_ACTIONS,
meta: {
throttle: actions.length ? throttle : NOTIFICATION_THROTTLE_NO_ACTIONS,
kibanaSiemAppUrl,
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ const RuleDetailsPageComponent: FC<PropsFromRedux> = ({
<EuiButton
href={getEditRuleUrl(ruleId ?? '')}
iconType="controlsHorizontal"
isDisabled={(userHasNoPermissions || rule?.immutable) ?? true}
isDisabled={userHasNoPermissions ?? true}
>
{ruleI18n.EDIT_RULE_SETTINGS}
</EuiButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ const EditRulePageComponent: FC = () => {
{
id: RuleStep.defineRule,
name: ruleI18n.DEFINITION,
disabled: rule?.immutable,
content: (
<>
<EuiSpacer />
Expand All @@ -140,6 +141,7 @@ const EditRulePageComponent: FC = () => {
{
id: RuleStep.aboutRule,
name: ruleI18n.ABOUT,
disabled: rule?.immutable,
content: (
<>
<EuiSpacer />
Expand All @@ -161,6 +163,7 @@ const EditRulePageComponent: FC = () => {
{
id: RuleStep.scheduleRule,
name: ruleI18n.SCHEDULE,
disabled: rule?.immutable,
content: (
<>
<EuiSpacer />
Expand Down Expand Up @@ -203,6 +206,7 @@ const EditRulePageComponent: FC = () => {
},
],
[
rule,
loading,
initLoading,
isLoading,
Expand Down Expand Up @@ -331,10 +335,11 @@ const EditRulePageComponent: FC = () => {
}, [rule]);

useEffect(() => {
setSelectedTab(tabs[0]);
}, []);
const tabIndex = rule?.immutable ? 3 : 0;
setSelectedTab(tabs[tabIndex]);
}, [rule]);

if (isSaved || (rule != null && rule.immutable)) {
if (isSaved) {
displaySuccessToast(i18n.SUCCESSFULLY_SAVED_RULE(rule?.name ?? ''), dispatchToaster);
return <Redirect to={`/${DETECTION_ENGINE_PAGE_NAME}/rules/id/${ruleId}`} />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ describe('rule helpers', () => {
],
};
const scheduleRuleStepData = { from: '0s', interval: '5m', isNew: false };
const ruleActionsStepData = { enabled: true, throttle: undefined, isNew: false, actions: [] };
const ruleActionsStepData = {
enabled: true,
throttle: 'no_actions',
isNew: false,
actions: [],
};
const aboutRuleDataDetailsData = {
note: '# this is some markdown documentation',
description: '24/7',
Expand Down Expand Up @@ -303,7 +308,7 @@ describe('rule helpers', () => {
actions: [],
enabled: mockedRule.enabled,
isNew: false,
throttle: undefined,
throttle: 'no_actions',
};

expect(result).toEqual(expected);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ export const getStepsData = ({
export const getActionsStepsData = (
rule: Omit<Rule, 'actions'> & { actions: RuleAlertAction[] }
): ActionsStepRule => {
const { enabled, actions = [], meta } = rule;
const { enabled, throttle, meta, actions = [] } = rule;

return {
actions: actions?.map(transformRuleToAlertAction),
isNew: false,
throttle: meta?.throttle,
throttle,
kibanaSiemAppUrl: meta?.kibanaSiemAppUrl,
enabled,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export type UpdateNotificationParams = Omit<NotificationAlertParams, 'interval'
actions: RuleAlertAction[];
id?: string;
tags?: string[];
interval: string | null;
interval: string | null | undefined;
ruleAlertId: string;
} & Clients;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export const getOutputRuleAlertForRest = (): Omit<
severity: 'high',
updated_by: 'elastic',
tags: [],
throttle: 'no_actions',
threat: [
{
framework: 'MITRE ATT&CK',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
} from '../utils';
import { createRulesBulkSchema } from '../schemas/create_rules_bulk_schema';
import { rulesBulkSchema } from '../schemas/response/rules_bulk_schema';
import { updateRulesNotifications } from '../../rules/update_rules_notifications';

export const createRulesBulkRoute = (router: IRouter) => {
router.post(
Expand All @@ -43,6 +44,7 @@ export const createRulesBulkRoute = (router: IRouter) => {
const alertsClient = context.alerting.getAlertsClient();
const actionsClient = context.actions.getActionsClient();
const clusterClient = context.core.elasticsearch.dataClient;
const savedObjectsClient = context.core.savedObjects.client;
const siemClient = context.siem.getSiemClient();

if (!actionsClient || !alertsClient) {
Expand Down Expand Up @@ -115,7 +117,6 @@ export const createRulesBulkRoute = (router: IRouter) => {
const createdRule = await createRules({
alertsClient,
actionsClient,
actions,
anomalyThreshold,
description,
enabled,
Expand All @@ -139,7 +140,6 @@ export const createRulesBulkRoute = (router: IRouter) => {
name,
severity,
tags,
throttle,
to,
type,
threat,
Expand All @@ -148,7 +148,18 @@ export const createRulesBulkRoute = (router: IRouter) => {
version,
lists,
});
return transformValidateBulkError(ruleIdOrUuid, createdRule);

const ruleActions = await updateRulesNotifications({
ruleAlertId: createdRule.id,
alertsClient,
savedObjectsClient,
enabled,
actions,
throttle,
name,
});

return transformValidateBulkError(ruleIdOrUuid, createdRule, ruleActions);
} catch (err) {
return transformBulkError(ruleIdOrUuid, err);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,12 @@ import {
getEmptyIndex,
getFindResultWithSingleHit,
createMlRuleRequest,
createRuleWithActionsRequest,
} from '../__mocks__/request_responses';
import { requestContextMock, serverMock, requestMock } from '../__mocks__';
import { createRulesRoute } from './create_rules_route';
import { setFeatureFlagsForTestsOnly, unSetFeatureFlagsForTestsOnly } from '../../feature_flags';
import { createNotifications } from '../../notifications/create_notifications';
jest.mock('../../notifications/create_notifications');
import { updateRulesNotifications } from '../../rules/update_rules_notifications';
jest.mock('../../rules/update_rules_notifications');

describe('create_rules', () => {
let server: ReturnType<typeof serverMock.create>;
Expand Down Expand Up @@ -49,6 +48,12 @@ 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);
});
Expand Down Expand Up @@ -86,18 +91,6 @@ describe('create_rules', () => {
});
});

describe('creating a Notification if throttle and actions were provided ', () => {
it('is successful', async () => {
const response = await server.inject(createRuleWithActionsRequest(), context);
expect(response.status).toEqual(200);
expect(createNotifications).toHaveBeenCalledWith(
expect.objectContaining({
ruleAlertId: '04128c15-0d1b-4716-a4c5-46997ac7f3bd',
})
);
});
});

describe('unhappy paths', () => {
test('it returns a 400 if the index does not exist', async () => {
clients.clusterClient.callAsCurrentUser.mockResolvedValue(getEmptyIndex());
Expand Down
Loading