Skip to content

Commit

Permalink
[SIEM][Detection Engine] Allow to edit actions for prepackaged rules (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
patrykkopycinski authored Mar 31, 2020
1 parent 0236376 commit 65e8f2b
Show file tree
Hide file tree
Showing 67 changed files with 791 additions and 284 deletions.
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 @@ -141,6 +141,7 @@ const StepRuleActionsComponent: FC<StepRuleActionsProps> = ({
/>
</>
)}
<UseField path="enabled" defaultValue={myStepData.enabled} component={GhostFormField} />
</Form>
</StepContentWrapper>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { FormSchema } from '../../../../../shared_imports';

export const schema: FormSchema = {
actions: {},
enabled: {},
kibanaSiemAppUrl: {},
throttle: {
label: i18n.translate(
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 { transformAlertToRuleAction } from '../../../../../common/detection_engine/transform_actions';
import { RuleType } from '../../../../../common/detection_engine/types';
import { isMlRule } from '../../../../../common/detection_engine/ml_helpers';
Expand Down Expand Up @@ -145,11 +142,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 @@ -161,9 +153,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 @@ -58,12 +58,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 @@ -40,6 +41,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 (!siemClient || !actionsClient || !alertsClient) {
Expand Down Expand Up @@ -112,7 +114,6 @@ export const createRulesBulkRoute = (router: IRouter) => {
const createdRule = await createRules({
alertsClient,
actionsClient,
actions,
anomalyThreshold,
description,
enabled,
Expand All @@ -136,7 +137,6 @@ export const createRulesBulkRoute = (router: IRouter) => {
name,
severity,
tags,
throttle,
to,
type,
threat,
Expand All @@ -145,7 +145,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 @@ -93,18 +98,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

0 comments on commit 65e8f2b

Please sign in to comment.