From 28d02fa81d874296120498ce87c6da98de17fd89 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 02:24:32 -0500 Subject: [PATCH 1/6] Fixe updating of ML rules * This wasn't caught by typescript * We should add a unit test to catch this the other routes besides create probably are similarly broken. --- .../siem/server/lib/detection_engine/rules/update_rules.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.ts index b2a1d2a6307d2..ae8ea9dd32cd2 100644 --- a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.ts +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.ts @@ -46,6 +46,8 @@ export const updateRules = async ({ throttle, note, lists, + anomalyThreshold, + machineLearningJobId, }: UpdateRuleParams): Promise => { const rule = await readRules({ alertsClient, ruleId, id }); if (rule == null) { @@ -78,6 +80,8 @@ export const updateRules = async ({ version, throttle, note, + anomalyThreshold, + machineLearningJobId, }); // TODO: Remove this and use regular lists once the feature is stable for a release @@ -115,6 +119,8 @@ export const updateRules = async ({ references, note, version: calculatedVersion, + anomalyThreshold, + machineLearningJobId, ...listsParam, }, }, From c018814b532ac6aa64945b9c30aa1d1008a17cfa Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 10:43:24 -0500 Subject: [PATCH 2/6] Add a regression test for updating ML Rules --- .../routes/__mocks__/request_responses.ts | 18 +++++++ .../rules/update_rules.test.ts | 51 +++++++++++++++++++ 2 files changed, 69 insertions(+) create mode 100644 x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/request_responses.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/request_responses.ts index 01f5c364ae420..cf8e2b32869d8 100644 --- a/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/request_responses.ts +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/request_responses.ts @@ -454,6 +454,24 @@ export const getResult = (): RuleAlertType => ({ scheduledTaskId: '2dabe330-0702-11ea-8b50-773b89126888', }); +export const getMlResult = (): RuleAlertType => { + const result = getResult(); + + return { + ...result, + params: { + ...result.params, + query: undefined, + language: undefined, + filters: undefined, + index: undefined, + type: 'machine_learning', + anomalyThreshold: 44, + machineLearningJobId: 'some_job_id', + }, + }; +}; + export const updateActionResult = (): ActionResult => ({ id: 'result-1', actionTypeId: 'action-id-1', diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts new file mode 100644 index 0000000000000..7ac124ff9d1bf --- /dev/null +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { savedObjectsClientMock } from '../../../../../../../../src/core/server/mocks'; +import { alertsClientMock } from '../../../../../../../plugins/alerting/server/mocks'; +import { actionsClientMock } from '../../../../../../../plugins/actions/server/mocks'; +import { getMlResult } from '../routes/__mocks__/request_responses'; +import { updateRules } from './update_rules'; + +describe('updateRules', () => { + beforeEach(() => {}); + + it('calls the alertsClient with ML params', async () => { + const savedObjectsClient = savedObjectsClientMock.create(); + const alertsClient = alertsClientMock.create(); + const actionsClient = actionsClientMock.create(); + alertsClient.get.mockResolvedValue(getMlResult()); + + const params = { + ...getMlResult().params, + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }; + + await updateRules({ + alertsClient, + actionsClient, + savedObjectsClient, + id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', + ...params, + enabled: true, + interval: '', + name: '', + tags: [], + }); + + expect(alertsClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + params: expect.objectContaining({ + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }), + }), + }) + ); + }); +}); From 7ee9da4b99f31ae7c2e2577cd39c77d3177bfa86 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 11:02:25 -0500 Subject: [PATCH 3/6] Allow ML Rules to be patched And adds a regression unit test. --- .../rules/patch_rules.test.ts | 51 +++++++++++++++++++ .../lib/detection_engine/rules/patch_rules.ts | 6 +++ .../rules/update_rules.test.ts | 13 +++-- 3 files changed, 66 insertions(+), 4 deletions(-) create mode 100644 x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.test.ts diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.test.ts new file mode 100644 index 0000000000000..b424d2912ebc8 --- /dev/null +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.test.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { savedObjectsClientMock } from '../../../../../../../../src/core/server/mocks'; +import { alertsClientMock } from '../../../../../../../plugins/alerting/server/mocks'; +import { actionsClientMock } from '../../../../../../../plugins/actions/server/mocks'; +import { getMlResult } from '../routes/__mocks__/request_responses'; +import { patchRules } from './patch_rules'; + +describe('patchRules', () => { + let actionsClient: ReturnType; + let alertsClient: ReturnType; + let savedObjectsClient: ReturnType; + + beforeEach(() => { + actionsClient = actionsClientMock.create(); + alertsClient = alertsClientMock.create(); + savedObjectsClient = savedObjectsClientMock.create(); + }); + + it('calls the alertsClient with ML params', async () => { + alertsClient.get.mockResolvedValue(getMlResult()); + const params = { + ...getMlResult().params, + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }; + + await patchRules({ + alertsClient, + actionsClient, + savedObjectsClient, + id: '04128c15-0d1b-4716-a4c5-46997ac7f3bd', + ...params, + }); + + expect(alertsClient.update).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + params: expect.objectContaining({ + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }), + }), + }) + ); + }); +}); diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.ts index 4fb73235854c0..a8da01f87a6fb 100644 --- a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.ts +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/patch_rules.ts @@ -46,6 +46,8 @@ export const patchRules = async ({ version, throttle, lists, + anomalyThreshold, + machineLearningJobId, }: PatchRuleParams): Promise => { const rule = await readRules({ alertsClient, ruleId, id }); if (rule == null) { @@ -79,6 +81,8 @@ export const patchRules = async ({ throttle, note, lists, + anomalyThreshold, + machineLearningJobId, }); const nextParams = defaults( @@ -109,6 +113,8 @@ export const patchRules = async ({ note, version: calculatedVersion, lists, + anomalyThreshold, + machineLearningJobId, } ); diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts index 7ac124ff9d1bf..5ee740a8b8845 100644 --- a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/update_rules.test.ts @@ -11,12 +11,17 @@ import { getMlResult } from '../routes/__mocks__/request_responses'; import { updateRules } from './update_rules'; describe('updateRules', () => { - beforeEach(() => {}); + let actionsClient: ReturnType; + let alertsClient: ReturnType; + let savedObjectsClient: ReturnType; + + beforeEach(() => { + actionsClient = actionsClientMock.create(); + alertsClient = alertsClientMock.create(); + savedObjectsClient = savedObjectsClientMock.create(); + }); it('calls the alertsClient with ML params', async () => { - const savedObjectsClient = savedObjectsClientMock.create(); - const alertsClient = alertsClientMock.create(); - const actionsClient = actionsClientMock.create(); alertsClient.get.mockResolvedValue(getMlResult()); const params = { From 19c9e1a0139b5af659748a3effc645920a576e17 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 11:09:38 -0500 Subject: [PATCH 4/6] Allow ML rule params to be imported when overwriting --- .../lib/detection_engine/routes/rules/import_rules_route.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts index 920cf97d32a7a..d95ef595e5c40 100644 --- a/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/rules/import_rules_route.ts @@ -228,6 +228,8 @@ export const importRulesRoute = (router: IRouter, config: LegacyServices['config references, note, version, + anomalyThreshold, + machineLearningJobId, }); resolve({ rule_id: ruleId, status_code: 200 }); } else if (rule != null) { From a2a0d0cdc12e1bb71ad38fa08656f94ca2a28c03 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 11:18:31 -0500 Subject: [PATCH 5/6] Add a basic regression test for creating a rule with ML params --- .../rules/create_rules.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/create_rules.test.ts diff --git a/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/create_rules.test.ts b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/create_rules.test.ts new file mode 100644 index 0000000000000..4c8d0f51f251b --- /dev/null +++ b/x-pack/legacy/plugins/siem/server/lib/detection_engine/rules/create_rules.test.ts @@ -0,0 +1,50 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { alertsClientMock } from '../../../../../../../plugins/alerting/server/mocks'; +import { actionsClientMock } from '../../../../../../../plugins/actions/server/mocks'; +import { getMlResult } from '../routes/__mocks__/request_responses'; +import { createRules } from './create_rules'; + +describe('createRules', () => { + let actionsClient: ReturnType; + let alertsClient: ReturnType; + + beforeEach(() => { + actionsClient = actionsClientMock.create(); + alertsClient = alertsClientMock.create(); + }); + + it('calls the alertsClient with ML params', async () => { + const params = { + ...getMlResult().params, + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }; + + await createRules({ + alertsClient, + actionsClient, + ...params, + ruleId: 'new-rule-id', + enabled: true, + interval: '', + name: '', + tags: [], + }); + + expect(alertsClient.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ + params: expect.objectContaining({ + anomalyThreshold: 55, + machineLearningJobId: 'new_job_id', + }), + }), + }) + ); + }); +}); From 9341e063f472bb1b0746c23332082916fbd50605 Mon Sep 17 00:00:00 2001 From: Ryland Herrick Date: Thu, 19 Mar 2020 12:05:43 -0500 Subject: [PATCH 6/6] Prevent users from changing an existing Rule's type --- .../rules/components/select_rule_type/index.tsx | 5 ++++- .../rules/components/step_define_rule/index.tsx | 8 +++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx index b3b35699914f6..229ccde54ecab 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/select_rule_type/index.tsx @@ -14,9 +14,10 @@ import { isMlRule } from '../../helpers'; interface SelectRuleTypeProps { field: FieldHook; + isReadOnly: boolean; } -export const SelectRuleType: React.FC = ({ field }) => { +export const SelectRuleType: React.FC = ({ field, isReadOnly = false }) => { const ruleType = field.value as RuleType; const setType = useCallback( (type: RuleType) => { @@ -37,6 +38,7 @@ export const SelectRuleType: React.FC = ({ field }) => { description={i18n.QUERY_TYPE_DESCRIPTION} icon={} selectable={{ + isDisabled: isReadOnly, onClick: setQuery, isSelected: !isMlRule(ruleType), }} @@ -49,6 +51,7 @@ export const SelectRuleType: React.FC = ({ field }) => { isDisabled={!license} icon={} selectable={{ + isDisabled: isReadOnly, onClick: setMl, isSelected: isMlRule(ruleType), }} diff --git a/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx b/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx index 6b1a9a828d950..d3ef185f3786b 100644 --- a/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx +++ b/x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx @@ -178,7 +178,13 @@ const StepDefineRuleComponent: FC = ({ <>
- + <>