Skip to content

Commit

Permalink
[Security Solution] Setup field form component (#178131)
Browse files Browse the repository at this point in the history
## Summary

Addresses #173626

Adds a markdown component in the create and edit rule forms so that
users are able to add their own setup guides to custom rules. Also
updates the `create` and `update` rule schemas and route logic to handle
these new cases through the API.

[Flaky test run
(internal)](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5603)

### Screenshots
![Screenshot 2024-03-08 at 11 12
25 AM](https://github.com/elastic/kibana/assets/56367316/5a00b007-d02d-4f1e-b1ba-ca7ba7f68bbd)


![Screenshot 2024-03-06 at 10 25
47 AM](https://github.com/elastic/kibana/assets/56367316/a3973e10-1c82-4981-b38d-69faf06a5993)


### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
dplumlee and kibanamachine authored Apr 8, 2024
1 parent 4047cc8 commit 9f8433e
Show file tree
Hide file tree
Showing 26 changed files with 245 additions and 71 deletions.
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 @@ -56,6 +56,11 @@ const NoteDescriptionContainer = styled(EuiFlexItem)`
overflow-y: hidden;
`;

const SetupDescriptionContainer = styled(EuiFlexItem)`
height: 105px;
overflow-y: hidden;
`;

export const isNotEmptyArray = (values: string[]) => !isEmpty(values.join(''));

const EuiBadgeWrap = styled(EuiBadge)`
Expand Down Expand Up @@ -647,3 +652,21 @@ export const buildAlertSuppressionMissingFieldsDescription = (
},
];
};

export const buildSetupDescription = (label: string, setup: string): ListItems[] => {
if (setup.trim() !== '') {
return [
{
title: label,
description: (
<SetupDescriptionContainer>
<div data-test-subj="setupDescriptionItem" className="eui-yScrollWithShadows">
{setup}
</div>
</SetupDescriptionContainer>
),
},
];
}
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 @@ -559,6 +559,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,
getQueryLabel,
} from './helpers';
import * as i18n from './translations';
Expand Down Expand Up @@ -305,6 +306,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 @@ -253,6 +253,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:
'Provide instructions on rule prerequisites such as required integrations, configuration steps, and anything else needed for the rule to work correctly.',
}
),
labelAppend: OptionalFieldLabel,
},
};

const threatIndicatorPathRequiredSchemaValue = {
Expand Down
Loading

0 comments on commit 9f8433e

Please sign in to comment.