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

[Security Solution] Allow users to edit setup field for custom rules #178131

Merged
merged 15 commits into from
Apr 8, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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 @@ -52,12 +52,12 @@ import {
RuleReferenceArray,
MaxSignals,
ThreatArray,
SetupGuide,
RuleObjectId,
RuleSignatureId,
IsRuleImmutable,
RelatedIntegrationArray,
RequiredFieldArray,
SetupGuide,
RuleQuery,
IndexPatternArray,
DataViewId,
Expand Down Expand Up @@ -134,6 +134,7 @@ export const BaseDefaultableFields = z.object({
references: RuleReferenceArray.optional(),
max_signals: MaxSignals.optional(),
threat: ThreatArray.optional(),
setup: SetupGuide.optional(),
});

export type BaseCreateProps = z.infer<typeof BaseCreateProps>;
Expand Down Expand Up @@ -162,7 +163,6 @@ export const ResponseFields = z.object({
revision: z.number().int().min(0),
related_integrations: RelatedIntegrationArray,
required_fields: RequiredFieldArray,
setup: SetupGuide,
execution_summary: RuleExecutionSummary.optional(),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ components:
$ref: './common_attributes.schema.yaml#/components/schemas/MaxSignals'
threat:
$ref: './common_attributes.schema.yaml#/components/schemas/ThreatArray'
setup:
$ref: './common_attributes.schema.yaml#/components/schemas/SetupGuide'

BaseCreateProps:
x-inline: true
Expand Down Expand Up @@ -174,7 +176,7 @@ components:
revision:
type: integer
minimum: 0
# NOTE: For now, Related Integrations, Required Fields and Setup Guide are
# NOTE: For now, Related Integrations and Required Fields are
# supported for prebuilt rules only. We don't want to allow users to edit these 3
# fields via the API. If we added them to baseParams.defaultable, they would
# become a part of the request schema as optional fields. This is why we add them
Expand All @@ -183,8 +185,6 @@ components:
$ref: './common_attributes.schema.yaml#/components/schemas/RelatedIntegrationArray'
required_fields:
$ref: './common_attributes.schema.yaml#/components/schemas/RequiredFieldArray'
setup:
$ref: './common_attributes.schema.yaml#/components/schemas/SetupGuide'
execution_summary:
$ref: '../../rule_monitoring/model/execution_summary.schema.yaml#/components/schemas/RuleExecutionSummary'
required:
Expand All @@ -198,7 +198,6 @@ components:
- revision
- related_integrations
- required_fields
- setup

SharedCreateProps:
x-inline: true
Expand Down Expand Up @@ -279,7 +278,7 @@ components:
$ref: './specific_attributes/eql_attributes.schema.yaml#/components/schemas/TiebreakerField'
timestamp_field:
$ref: './specific_attributes/eql_attributes.schema.yaml#/components/schemas/TimestampField'

EqlRuleCreateFields:
allOf:
- $ref: '#/components/schemas/EqlRequiredFields'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ interface MarkdownEditorProps {
height?: number;
autoFocusDisabled?: boolean;
setIsMarkdownInvalid: (value: boolean) => void;
includePlugins?: boolean;
}

type EuiMarkdownEditorRef = ElementRef<typeof EuiMarkdownEditor>;
Expand All @@ -52,6 +53,7 @@ const MarkdownEditorComponent = forwardRef<MarkdownEditorRef, MarkdownEditorProp
height,
autoFocusDisabled,
setIsMarkdownInvalid,
includePlugins = true,
},
ref
) => {
Expand All @@ -73,8 +75,8 @@ const MarkdownEditorComponent = forwardRef<MarkdownEditorRef, MarkdownEditorProp

const insightsUpsellingMessage = useUpsellingMessage('investigation_guide');
const uiPluginsWithState = useMemo(() => {
return uiPlugins({ insightsUpsellingMessage });
}, [insightsUpsellingMessage]);
return includePlugins ? uiPlugins({ insightsUpsellingMessage }) : undefined;
}, [insightsUpsellingMessage, includePlugins]);

// @ts-expect-error update types
useImperativeHandle(ref, () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ type MarkdownEditorFormProps = EuiMarkdownEditorProps & {
idAria: string;
isDisabled?: boolean;
bottomRightContent?: React.ReactNode;
includePlugins?: boolean;
};
/* eslint-enable react/no-unused-prop-types */

Expand All @@ -34,7 +35,7 @@ const BottomContentWrapper = styled(EuiFlexGroup)`

export const MarkdownEditorForm = React.memo(
forwardRef<MarkdownEditorRef, MarkdownEditorFormProps>(
({ id, field, dataTestSubj, idAria, bottomRightContent }, ref) => {
({ id, field, dataTestSubj, idAria, bottomRightContent, includePlugins }, ref) => {
const { isInvalid, errorMessage } = getFieldValidityAndErrorMessage(field);
const [isMarkdownInvalid, setIsMarkdownInvalid] = useState(false);

Expand All @@ -58,6 +59,7 @@ export const MarkdownEditorForm = React.memo(
value={field.value as string}
data-test-subj={`${dataTestSubj}-markdown-editor`}
setIsMarkdownInvalid={setIsMarkdownInvalid}
includePlugins={includePlugins}
/>
{bottomRightContent && (
<BottomContentWrapper justifyContent={'flexEnd'}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ describe('StepAboutRuleToggleDetails', () => {
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: '',
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand Down Expand Up @@ -82,28 +82,30 @@ describe('StepAboutRuleToggleDetails', () => {
});

describe('note value is empty string', () => {
test('it does not render toggle buttons', () => {
test('it does render toggle buttons if setup is not empty', () => {
const mockAboutStepWithoutNote = {
...stepDataMock,
note: '',
};
const wrapper = shallow(
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: '',
description: stepDataMock.description,
setup: '',
}}
stepData={mockAboutStepWithoutNote}
rule={mockRule('mocked-rule-id')}
/>
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: '',
description: stepDataMock.description,
setup: stepDataMock.setup,
}}
stepData={mockAboutStepWithoutNote}
rule={mockRule('mocked-rule-id')}
/>
</ThemeProvider>
);

expect(wrapper.find('[data-test-subj="stepAboutDetailsToggle"]').exists()).toBeFalsy();
expect(wrapper.find(EuiButtonGroup).exists()).toBeTruthy();
expect(wrapper.find('#details').at(0).prop('isSelected')).toBeTruthy();
expect(wrapper.find('#setup').at(0).prop('isSelected')).toBeFalsy();
expect(wrapper.find('[data-test-subj="stepAboutDetailsNoteContent"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="stepAboutDetailsSetupContent"]').exists()).toBeFalsy();
expect(wrapper.find('[data-test-subj="stepAboutDetailsContent"]').exists()).toBeTruthy();
});
});

Expand All @@ -116,7 +118,7 @@ describe('StepAboutRuleToggleDetails', () => {
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: '',
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand All @@ -137,7 +139,7 @@ describe('StepAboutRuleToggleDetails', () => {
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: '',
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand Down Expand Up @@ -212,7 +214,7 @@ describe('StepAboutRuleToggleDetails', () => {
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: stepDataMock.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand All @@ -234,7 +236,7 @@ describe('StepAboutRuleToggleDetails', () => {
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: stepDataMock.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand All @@ -253,15 +255,15 @@ describe('StepAboutRuleToggleDetails', () => {
expect(wrapper.find('[idSelected="setup"]').exists()).toBeTruthy();
});

test('it displays notes markdown when user toggles to "setup"', () => {
test('it displays setup markdown when user toggles to "setup"', () => {
const wrapper = mount(
<ThemeProvider theme={mockTheme}>
<StepAboutRuleToggleDetails
loading={false}
stepDataDetails={{
note: stepDataMock.note,
description: stepDataMock.description,
setup: stepDataMock.note, // TODO: Update to mockRule.setup once supported in UI (and mock can be updated)
setup: stepDataMock.setup,
}}
stepData={stepDataMock}
rule={mockRule('mocked-rule-id')}
Expand All @@ -273,7 +275,7 @@ describe('StepAboutRuleToggleDetails', () => {

expect(wrapper.find('EuiButtonGroup[idSelected="setup"]').exists()).toBeTruthy();
expect(wrapper.find('div.euiMarkdownFormat').text()).toEqual(
'this is some markdown documentation'
'this is some setup documentation'
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -635,3 +635,21 @@ export const buildAlertSuppressionMissingFieldsDescription = (
},
];
};

export const buildSetupDescription = (label: string, setup: string): ListItems[] => {
if (setup.trim() !== '') {
return [
{
title: label,
description: (
<NoteDescriptionContainer>
dplumlee marked this conversation as resolved.
Show resolved Hide resolved
<div data-test-subj="setupDescriptionItem" className="eui-yScrollWithShadows">
{setup}
</div>
</NoteDescriptionContainer>
),
},
];
}
return [];
};
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ describe('description_step', () => {
mockLicenseService
);

expect(result.length).toEqual(12);
expect(result.length).toEqual(13);
});
});

Expand Down Expand Up @@ -536,6 +536,21 @@ describe('description_step', () => {
});
});

describe('setup', () => {
test('returns default "setup" description', () => {
const result: ListItems[] = getDescriptionItem(
'setup',
'Setup guide',
mockAboutStep,
mockFilterManager,
mockLicenseService
);

expect(result[0].title).toEqual('Setup guide');
expect(React.isValidElement(result[0].description)).toBeTruthy();
});
});

describe('alert suppression', () => {
const ruleTypesWithoutSuppression: Type[] = ['eql', 'esql', 'machine_learning', 'new_terms'];
const suppressionFields = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
buildAlertSuppressionWindowDescription,
buildAlertSuppressionMissingFieldsDescription,
buildHighlightedFieldsOverrideDescription,
buildSetupDescription,
} from './helpers';
import * as i18n from './translations';
import { buildMlJobsDescription } from './build_ml_jobs_description';
Expand Down Expand Up @@ -301,6 +302,9 @@ export const getDescriptionItem = (
} else if (field === 'note') {
const val: string = get(field, data);
return buildNoteDescription(label, val);
} else if (field === 'setup') {
const val: string = get(field, data);
return buildSetupDescription(label, val);
} else if (field === 'ruleType') {
const ruleType: Type = get(field, data);
return buildRuleTypeDescription(label, ruleType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ export const stepAboutDefaultValue: AboutStepRule = {
timestampOverride: '',
threat: threatDefault,
note: '',
setup: '',
};
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ describe('StepAboutRuleComponent', () => {
falsePositives: [''],
name: 'Test name text',
note: '',
setup: '',
references: [''],
riskScore: { value: 21, mapping: [], isMappingChecked: false },
severity: { value: 'low', mapping: fillEmptySeverityMappings([]), isMappingChecked: false },
Expand Down Expand Up @@ -329,6 +330,7 @@ describe('StepAboutRuleComponent', () => {
falsePositives: [''],
name: 'Test name text',
note: '',
setup: '',
references: [''],
riskScore: { value: 80, mapping: [], isMappingChecked: false },
severity: { value: 'low', mapping: fillEmptySeverityMappings([]), isMappingChecked: false },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,18 @@ const StepAboutRuleComponent: FC<StepAboutRuleProps> = ({
}}
/>
<EuiSpacer size="l" />
<UseField
path="setup"
component={MarkdownEditorForm}
componentProps={{
idAria: 'detectionEngineStepAboutRuleSetup',
isDisabled: isLoading,
dataTestSubj: 'detectionEngineStepAboutRuleSetup',
placeholder: I18n.ADD_RULE_SETUP_HELP_TEXT,
includePlugins: false,
}}
/>
<EuiSpacer size="l" />
<UseField
path="note"
component={MarkdownEditorForm}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,23 @@ export const schema: FormSchema<AboutStepRule> = {
),
labelAppend: OptionalFieldLabel,
},
setup: {
type: FIELD_TYPES.TEXTAREA,
label: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.setupLabel',
{
defaultMessage: 'Setup guide',
}
),
helpText: i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepAboutRule.setupHelpText',
{
defaultMessage:
'This guide will appear on the rule details page and in timelines (as notes) created from detection alerts generated by this rule.',
dplumlee marked this conversation as resolved.
Show resolved Hide resolved
}
),
labelAppend: OptionalFieldLabel,
},
};

const threatIndicatorPathRequiredSchemaValue = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,10 @@ export const ADD_RULE_NOTE_HELP_TEXT = i18n.translate(
defaultMessage: 'Add rule investigation guide...',
}
);

export const ADD_RULE_SETUP_HELP_TEXT = i18n.translate(
'xpack.securitySolution.detectionEngine.createRule.stepAboutrule.setupHelpText',
{
defaultMessage: 'Add rule setup guide...',
}
);
Loading