Skip to content

Commit

Permalink
[SIEM] Move Timeline Template field to first step of rule creation (#…
Browse files Browse the repository at this point in the history 
…60840) (#61014)

* Move timeline template to Define step of Rule creation

This required a refactor/simplification of the step_define_rule logic to
make things work. In retrospect I think that the issue was we were not
handling incoming `defaultValues` props well, which was causing local
component state to be lost.

Now that we're doing a merge and removed a few unneeded local useStates,
things are a) working and b) cleaner

* Fix Rule details/edit view with updated data

We need to fix the other side of the equation to get these to work: the
timeline data was moved to a different step during creation, but when
viewing on the frontend we split the rule data back into the separate
"steps."

* Remove unused import

* Fix bug in formatDefineStepData

I neglected to pass through index in a previous commit.

* Update tests now that timeline has movied to a different step

* Fix more tests

* Update StepRuleDescription snapshots

* Fix cypress Rule Creation test

Timeline template moved, and so tests broke.

* Add unit tests for filterRuleFieldsForType
  • Loading branch information
@rylnd
rylnd authored Mar 24, 2020
1 parent d8c1bbf commit e76ad68
Show file tree
Hide file tree
Showing 15 changed files with 308 additions and 348 deletions.
10 changes: 5 additions & 5 deletions x-pack/legacy/plugins/siem/cypress/integration/signal_detection_rules.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import {
ABOUT_SEVERITY,
ABOUT_STEP,
ABOUT_TAGS,
ABOUT_TIMELINE,
ABOUT_URLS,
DEFINITION_CUSTOM_QUERY,
DEFINITION_INDEX_PATTERNS,
DEFINITION_TIMELINE,
DEFINITION_STEP,
RULE_NAME_HEADER,
SCHEDULE_LOOPBACK,
Expand Down Expand Up @@ -170,10 +170,6 @@ describe('Signal detection rules', () => {
.eq(ABOUT_RISK)
.invoke('text')
.should('eql', newRule.riskScore);
cy.get(ABOUT_STEP)
.eq(ABOUT_TIMELINE)
.invoke('text')
.should('eql', 'Default blank timeline');
cy.get(ABOUT_STEP)
.eq(ABOUT_URLS)
.invoke('text')
Expand Down Expand Up @@ -202,6 +198,10 @@ describe('Signal detection rules', () => {
.eq(DEFINITION_CUSTOM_QUERY)
.invoke('text')
.should('eql', `${newRule.customQuery} `);
cy.get(DEFINITION_STEP)
.eq(DEFINITION_TIMELINE)
.invoke('text')
.should('eql', 'Default blank timeline');

cy.get(SCHEDULE_STEP)
.eq(SCHEDULE_RUNS)
Expand Down
12 changes: 6 additions & 6 deletions x-pack/legacy/plugins/siem/cypress/screens/rule_details.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
* you may not use this file except in compliance with the Elastic License.
*/

export const ABOUT_FALSE_POSITIVES = 4;
export const ABOUT_FALSE_POSITIVES = 3;

export const ABOUT_MITRE = 5;
export const ABOUT_MITRE = 4;

export const ABOUT_RULE_DESCRIPTION = '[data-test-subj=stepAboutRuleDetailsToggleDescriptionText]';

Expand All @@ -16,14 +16,14 @@ export const ABOUT_SEVERITY = 0;

export const ABOUT_STEP = '[data-test-subj="aboutRule"] .euiDescriptionList__description';

export const ABOUT_TAGS = 6;
export const ABOUT_TAGS = 5;

export const ABOUT_TIMELINE = 2;

export const ABOUT_URLS = 3;
export const ABOUT_URLS = 2;

export const DEFINITION_CUSTOM_QUERY = 1;

export const DEFINITION_TIMELINE = 3;

export const DEFINITION_INDEX_PATTERNS =
'[data-test-subj=definitionRule] [data-test-subj="listItemColumnStepRuleDescription"] .euiDescriptionList__description .euiBadge__text';

Expand Down
8 changes: 4 additions & 4 deletions x-pack/legacy/plugins/siem/public/pages/detection_engine/rules/all/__mocks__/mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,6 @@ export const mockAboutStepRule = (isNew = false): AboutStepRule => ({
references: ['www.test.co'],
falsePositives: ['test'],
tags: ['tag1', 'tag2'],
timeline: {
id: '86aa74d0-2136-11ea-9864-ebc8cc1cb8c2',
title: 'Titled timeline',
},
threat: [
{
framework: 'mockFramework',
Expand Down Expand Up @@ -186,6 +182,10 @@ export const mockDefineStepRule = (isNew = false): DefineStepRule => ({
machineLearningJobId: '',
index: ['filebeat-'],
queryBar: mockQueryBar,
timeline: {
id: '86aa74d0-2136-11ea-9864-ebc8cc1cb8c2',
title: 'Titled timeline',
},
});

export const mockScheduleStepRule = (isNew = false, enabled = false): ScheduleStepRule => ({
Expand Down
34 changes: 11 additions & 23 deletions ...ages/detection_engine/rules/components/description_step/__snapshots__/index.test.tsx.snap
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions ...ugins/siem/public/pages/detection_engine/rules/components/description_step/index.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
Filter,
FilterManager,
} from '../../../../../../../../../../src/plugins/data/public';
import { mockAboutStepRule } from '../../all/__mocks__/mock';
import { mockAboutStepRule, mockDefineStepRule } from '../../all/__mocks__/mock';
import { coreMock } from '../../../../../../../../../../src/core/public/mocks';
import { DEFAULT_TIMELINE_TITLE } from '../../../../../components/timeline/translations';
import * as i18n from './translations';
Expand Down Expand Up @@ -263,7 +263,7 @@ describe('description_step', () => {
test('returns expected ListItems array when given valid inputs', () => {
const result: ListItems[] = buildListItems(mockAboutStep, schema, mockFilterManager);

expect(result.length).toEqual(10);
expect(result.length).toEqual(9);
});
});

Expand Down Expand Up @@ -431,10 +431,11 @@ describe('description_step', () => {

describe('timeline', () => {
test('returns timeline title if one exists', () => {
const mockDefineStep = mockDefineStepRule();
const result: ListItems[] = getDescriptionItem(
'timeline',
'Timeline label',
mockAboutStep,
mockDefineStep,
mockFilterManager
);

Expand All @@ -444,7 +445,7 @@ describe('description_step', () => {

test('returns default timeline title if none exists', () => {
const mockStep = {
...mockAboutStep,
...mockDefineStepRule(),
timeline: {
id: '12345',
},
Expand Down
5 changes: 0 additions & 5 deletions ...gins/siem/public/pages/detection_engine/rules/components/step_about_rule/default_value.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
*/

import { AboutStepRule } from '../../types';
import { DEFAULT_TIMELINE_TITLE } from '../../../../../components/timeline/translations';

export const threatDefault = [
{
Expand All @@ -24,10 +23,6 @@ export const stepAboutDefaultValue: AboutStepRule = {
references: [''],
falsePositives: [''],
tags: [],
timeline: {
id: null,
title: DEFAULT_TIMELINE_TITLE,
},
threat: threatDefault,
note: '',
};
10 changes: 0 additions & 10 deletions ...acy/plugins/siem/public/pages/detection_engine/rules/components/step_about_rule/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { stepAboutDefaultValue } from './default_value';
import { isUrlInvalid } from './helpers';
import { schema } from './schema';
import * as I18n from './translations';
import { PickTimeline } from '../pick_timeline';
import { StepContentWrapper } from '../step_content_wrapper';
import { MarkdownEditorForm } from '../../../../../components/markdown_editor/form';

Expand Down Expand Up @@ -216,15 +215,6 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
buttonContent={AdvancedSettingsAccordionButton}
>
<EuiSpacer size="m" />
<UseField
path="timeline"
component={PickTimeline}
componentProps={{
idAria: 'detectionEngineStepAboutRuleTimeline',
isDisabled: isLoading,
dataTestSubj: 'detectionEngineStepAboutRuleTimeline',
}}
/>
<UseField
path="references"
component={AddItem}
Expand Down
15 changes: 0 additions & 15 deletions ...cy/plugins/siem/public/pages/detection_engine/rules/components/step_about_rule/schema.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,6 @@ export const schema: FormSchema = {
}
),
},
timeline: {
label: i18n.translate(
'xpack.siem.detectionEngine.createRule.stepAboutRule.fieldTimelineTemplateLabel',
{
defaultMessage: 'Timeline template',
}
),
helpText: i18n.translate(
'xpack.siem.detectionEngine.createRule.stepAboutRule.fieldTimelineTemplateHelpText',
{
defaultMessage:
'Select an existing timeline to use as a template when investigating generated signals.',
}
),
},
references: {
label: i18n.translate(
'xpack.siem.detectionEngine.createRule.stepAboutRule.fieldReferenceUrlsLabel',
Expand Down
75 changes: 32 additions & 43 deletions ...cy/plugins/siem/public/pages/detection_engine/rules/components/step_define_rule/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@ import {
EuiFormRow,
EuiButton,
} from '@elastic/eui';
import { isEmpty } from 'lodash/fp';
import React, { FC, memo, useCallback, useState, useEffect, useContext } from 'react';
import styled from 'styled-components';
import deepEqual from 'fast-deep-equal';

import { IIndexPattern } from '../../../../../../../../../../src/plugins/data/public';
import { useFetchIndexPatterns } from '../../../../../containers/detection_engine/rules';
import { DEFAULT_INDEX_KEY } from '../../../../../../common/constants';
import { DEFAULT_TIMELINE_TITLE } from '../../../../../components/timeline/translations';
import { MlCapabilitiesContext } from '../../../../../components/ml/permissions/ml_capabilities_provider';
import { useUiSetting$ } from '../../../../../lib/kibana';
import { setFieldValue, isMlRule } from '../../helpers';
Expand All @@ -30,6 +30,7 @@ import { QueryBarDefineRule } from '../query_bar';
import { SelectRuleType } from '../select_rule_type';
import { AnomalyThresholdSlider } from '../anomaly_threshold_slider';
import { MlJobSelect } from '../ml_job_select';
import { PickTimeline } from '../pick_timeline';
import { StepContentWrapper } from '../step_content_wrapper';
import {
Field,
Expand Down Expand Up @@ -61,6 +62,10 @@ const stepDefineDefaultValue: DefineStepRule = {
filters: [],
saved_id: undefined,
},
timeline: {
id: null,
title: DEFAULT_TIMELINE_TITLE,
},
};

const MyLabelButton = styled(EuiButtonEmpty)`
Expand All @@ -77,23 +82,6 @@ MyLabelButton.defaultProps = {
flush: 'right',
};

const getStepDefaultValue = (
indicesConfig: string[],
defaultValues: DefineStepRule | null
): DefineStepRule => {
if (defaultValues != null) {
return {
...defaultValues,
isNew: false,
};
} else {
return {
...stepDefineDefaultValue,
index: indicesConfig != null ? indicesConfig : [],
};
}
};

const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
addPadding = false,
defaultValues,
Expand All @@ -106,18 +94,16 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
}) => {
const mlCapabilities = useContext(MlCapabilitiesContext);
const [openTimelineSearch, setOpenTimelineSearch] = useState(false);
const [localUseIndicesConfig, setLocalUseIndicesConfig] = useState(false);
const [indexModified, setIndexModified] = useState(false);
const [localIsMlRule, setIsMlRule] = useState(false);
const [indicesConfig] = useUiSetting$<string[]>(DEFAULT_INDEX_KEY);
const [mylocalIndicesConfig, setMyLocalIndicesConfig] = useState(
defaultValues != null ? defaultValues.index : indicesConfig ?? []
);
const [myStepData, setMyStepData] = useState<DefineStepRule>({
...stepDefineDefaultValue,
index: indicesConfig ?? [],
});
const [
{ browserFields, indexPatterns: indexPatternQueryBar, isLoading: indexPatternLoadingQueryBar },
] = useFetchIndexPatterns(mylocalIndicesConfig);
const [myStepData, setMyStepData] = useState<DefineStepRule>(
getStepDefaultValue(indicesConfig, null)
);
] = useFetchIndexPatterns(myStepData.index);

const { form } = useForm({
defaultValue: myStepData,
Expand All @@ -138,15 +124,13 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
}, [form]);

useEffect(() => {
if (indicesConfig != null && defaultValues != null) {
const myDefaultValues = getStepDefaultValue(indicesConfig, defaultValues);
if (!deepEqual(myDefaultValues, myStepData)) {
setMyStepData(myDefaultValues);
setLocalUseIndicesConfig(deepEqual(myDefaultValues.index, indicesConfig));
setFieldValue(form, schema, myDefaultValues);
}
const { isNew, ...values } = myStepData;
if (defaultValues != null && !deepEqual(values, defaultValues)) {
const newValues = { ...values, ...defaultValues, isNew: false };
setMyStepData(newValues);
setFieldValue(form, schema, newValues);
}
}, [defaultValues, indicesConfig]);
}, [defaultValues, setMyStepData, setFieldValue]);

useEffect(() => {
if (setForm != null) {
Expand Down Expand Up @@ -195,7 +179,7 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
path="index"
config={{
...schema.index,
labelAppend: !localUseIndicesConfig ? (
labelAppend: indexModified ? (
<MyLabelButton onClick={handleResetIndices} iconType="refresh">
{i18n.RESET_DEFAULT_INDEX}
</MyLabelButton>
Expand Down Expand Up @@ -253,17 +237,22 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
/>
</>
</EuiFormRow>
<UseField
path="timeline"
component={PickTimeline}
componentProps={{
idAria: 'detectionEngineStepDefineRuleTimeline',
isDisabled: isLoading,
dataTestSubj: 'detectionEngineStepDefineRuleTimeline',
}}
/>
<FormDataProvider pathsToWatch={['index', 'ruleType']}>
{({ index, ruleType }) => {
if (index != null) {
if (deepEqual(index, indicesConfig) && !localUseIndicesConfig) {
setLocalUseIndicesConfig(true);
}
if (!deepEqual(index, indicesConfig) && localUseIndicesConfig) {
setLocalUseIndicesConfig(false);
}
if (index != null && !isEmpty(index) && !deepEqual(index, mylocalIndicesConfig)) {
setMyLocalIndicesConfig(index);
if (deepEqual(index, indicesConfig) && indexModified) {
setIndexModified(false);
} else if (!deepEqual(index, indicesConfig) && !indexModified) {
setIndexModified(true);
}
}

Expand Down
Loading

0 comments on commit e76ad68

Please sign in to comment.