Skip to content

Commit

Permalink
[Security Solution] Allow users to edit required_fields field for cus…
Browse files Browse the repository at this point in the history
…tom rules (#180682)

**Resolves: #173594
**Flaky test runner:**
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/5915


## Summary
This PR adds an ability to add and edit custom rule's required fields.
"Required fields" is an optional field that shows the user which
Elasticsearch fields are needed for the rule to run properly. The values
in "required fields" don't affect rule execution in any way. It's purely
documentational, similar to "setup guide" and "investigation guide".
This functionality is added to both rule creation and rule editing
screens. It's available for all rule types except ML.

<img width="650" alt="Scherm­afbeelding 2024-05-07 om 12 28 50"
src="https://github.com/elastic/kibana/assets/15949146/70ed052e-3e59-413e-80de-73146225f75a">


## Details
The basic flow goes like this: first you specify your index patterns (or
a data view), then you can set required fields for these index patterns.
Once a user adds a required field and chooses its name, he can then
choose its type from the dropdown. The first available type for the
field name selected automatically. User can also add their own custom
names and types.

### Warnings
If a field that is not present in the selected index pattern, you will
see a warning message.
This can happen in the following cases:
- You have specified an index pattern, selected a required field from
this index pattern, and then removed this index pattern.
- The index doesn't yet exist. For example, you have installed a
prebuilt rule but the data for it hasn't been ingested yet, so there's
no index yet.
 - The index was removed.
- The mappings for the index were changed and the field is no longer
present.

In any of these cases, you'll see a general warning message above the
form section. And then also a more specific warning message next to the
field that is causing the issue.

### ESQL and ML rules
Here's how available dropdown options are determined for different rule
types:

For all rule types except ESQL and ML, we take the index patterns
specified by the user and fetch their mappings. Then we use these fields
and types to populate the dropdowns.

For ESQL rules we parse index patterns out of the query since there's no
explicit index pattern form field. We then fetch the mappings for these
index patterns and use them to populate the dropdowns.

For ML rules, we don't show "required fields" at all. ML rules are a
special case.
1. The concept of "required fields" is sort of handled during creation
of the ML job itself, where the user specifies the fields that are
required for the job to run.
2. In the ML rule creation/editing interface, we don't display the index
patterns a rule operates on. So, even if we allowed specifying required
fields, the user would need to check the ML job details to see the index
patterns the job uses.
3. The ML job dropdown includes both existing and not-yet-created jobs.
We can't get index patterns for jobs that don't exist yet, so we can't
fill the dropdowns with fields and types.

## Screenshots
<img width="628" alt="screen1_"
src="https://github.com/elastic/kibana/assets/15949146/aade141f-8285-44c6-8c56-611ba1a9f17b">

<img width="601" alt="screen2_"
src="https://github.com/elastic/kibana/assets/15949146/b44fb254-c254-44b8-9600-45b47f29a421">
  • Loading branch information
nikitaindik authored May 17, 2024
1 parent 85639f6 commit 6eeffd3
Show file tree
Hide file tree
Showing 55 changed files with 2,305 additions and 210 deletions.
1 change: 1 addition & 0 deletions packages/kbn-doc-links/src/get_doc_links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ export const getDocLinks = ({ kibanaBranch, buildFlavor }: GetDocLinkOptions): D
},
privileges: `${SECURITY_SOLUTION_DOCS}endpoint-management-req.html`,
manageDetectionRules: `${SECURITY_SOLUTION_DOCS}rules-ui-management.html`,
createDetectionRules: `${SECURITY_SOLUTION_DOCS}rules-ui-create.html`,
createEsqlRuleType: `${SECURITY_SOLUTION_DOCS}rules-ui-create.html#create-esql-rule`,
ruleUiAdvancedParams: `${SECURITY_SOLUTION_DOCS}rules-ui-create.html#rule-ui-advanced-params`,
entityAnalytics: {
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-doc-links/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ export interface DocLinks {
};
readonly privileges: string;
readonly manageDetectionRules: string;
readonly createDetectionRules: string;
readonly createEsqlRuleType: string;
readonly ruleUiAdvancedParams: string;
readonly entityAnalytics: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,40 @@ export const TimestampOverride = z.string();
export type TimestampOverrideFallbackDisabled = z.infer<typeof TimestampOverrideFallbackDisabled>;
export const TimestampOverrideFallbackDisabled = z.boolean();

/**
* Describes an Elasticsearch field that is needed for the rule to function
*/
export type RequiredField = z.infer<typeof RequiredField>;
export const RequiredField = z.object({
/**
* Name of an Elasticsearch field
*/
name: NonEmptyString,
/**
* Type of the Elasticsearch field
*/
type: NonEmptyString,
/**
* Whether the field is an ECS field
*/
ecs: z.boolean(),
});

/**
* Input parameters to create a RequiredField. Does not include the `ecs` field, because `ecs` is calculated on the backend based on the field name and type.
*/
export type RequiredFieldInput = z.infer<typeof RequiredFieldInput>;
export const RequiredFieldInput = z.object({
/**
* Name of an Elasticsearch field
*/
name: NonEmptyString,
/**
* Type of an Elasticsearch field
*/
type: NonEmptyString,
});

export type RequiredFieldArray = z.infer<typeof RequiredFieldArray>;
export const RequiredFieldArray = z.array(RequiredField);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,18 +315,36 @@ components:

RequiredField:
type: object
description: Describes an Elasticsearch field that is needed for the rule to function
properties:
name:
$ref: '../../../model/primitives.schema.yaml#/components/schemas/NonEmptyString'
description: Name of an Elasticsearch field
type:
$ref: '../../../model/primitives.schema.yaml#/components/schemas/NonEmptyString'
description: Type of the Elasticsearch field
ecs:
type: boolean
description: Whether the field is an ECS field
required:
- name
- type
- ecs

RequiredFieldInput:
type: object
description: Input parameters to create a RequiredField. Does not include the `ecs` field, because `ecs` is calculated on the backend based on the field name and type.
properties:
name:
$ref: '../../../model/primitives.schema.yaml#/components/schemas/NonEmptyString'
description: Name of an Elasticsearch field
type:
$ref: '../../../model/primitives.schema.yaml#/components/schemas/NonEmptyString'
description: Type of an Elasticsearch field
required:
- name
- type

RequiredFieldArray:
type: array
items:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
ThreatArray,
SetupGuide,
RelatedIntegrationArray,
RequiredFieldInput,
RuleObjectId,
RuleSignatureId,
IsRuleImmutable,
Expand Down Expand Up @@ -137,6 +138,7 @@ export const BaseDefaultableFields = z.object({
threat: ThreatArray.optional(),
setup: SetupGuide.optional(),
related_integrations: RelatedIntegrationArray.optional(),
required_fields: z.array(RequiredFieldInput).optional(),
});

export type BaseCreateProps = z.infer<typeof BaseCreateProps>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,22 +117,32 @@ components:
author:
$ref: './common_attributes.schema.yaml#/components/schemas/RuleAuthorArray'

# False positive examples
false_positives:
$ref: './common_attributes.schema.yaml#/components/schemas/RuleFalsePositiveArray'

# Reference URLs
references:
$ref: './common_attributes.schema.yaml#/components/schemas/RuleReferenceArray'

# Max alerts per run
max_signals:
$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'

# Related integrations
related_integrations:
$ref: './common_attributes.schema.yaml#/components/schemas/RelatedIntegrationArray'

# Required fields
required_fields:
type: array
items:
$ref: './common_attributes.schema.yaml#/components/schemas/RequiredFieldInput'

BaseCreateProps:
x-inline: true
allOf:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as z from 'zod';
import {
BaseCreateProps,
ResponseFields,
RequiredFieldInput,
RuleSignatureId,
TypeSpecificCreateProps,
} from '../../model/rule_schema';
Expand All @@ -29,5 +30,13 @@ export const RuleToImport = BaseCreateProps.and(TypeSpecificCreateProps).and(
ResponseFields.partial().extend({
rule_id: RuleSignatureId,
immutable: z.literal(false).default(false),
/*
Overriding `required_fields` from ResponseFields because
in ResponseFields `required_fields` has the output type,
but for importing rules, we need to use the input type.
Otherwise importing rules without the "ecs" property in
`required_fields` will fail.
*/
required_fields: z.array(RequiredFieldInput).optional(),
})
);
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import type { FieldHook } from '../../../../shared_imports';
import type { Integration, RelatedIntegration } from '../../../../../common/api/detection_engine';
import { useIntegrations } from '../../../../detections/components/rules/related_integrations/use_integrations';
import { IntegrationStatusBadge } from './integration_status_badge';
import { DEFAULT_RELATED_INTEGRATION } from './default_related_integration';
import * as i18n from './translations';

interface RelatedIntegrationItemFormProps {
Expand Down Expand Up @@ -95,16 +94,6 @@ export function RelatedIntegrationField({
);

const hasError = Boolean(packageErrorMessage) || Boolean(versionErrorMessage);
const isLastField = relatedIntegrations.length === 1;
const isLastEmptyField = isLastField && field.value.package === '';
const handleRemove = useCallback(() => {
if (isLastField) {
field.setValue(DEFAULT_RELATED_INTEGRATION);
return;
}

onRemove();
}, [onRemove, field, isLastField]);

return (
<EuiFormRow
Expand Down Expand Up @@ -145,8 +134,8 @@ export function RelatedIntegrationField({
<EuiFlexItem grow={false}>
<EuiButtonIcon
color="danger"
onClick={handleRemove}
isDisabled={!integrations || isLastEmptyField}
onClick={onRemove}
isDisabled={!integrations}
iconType="trash"
aria-label={i18n.REMOVE_RELATED_INTEGRATION_BUTTON_ARIA_LABEL}
data-test-subj="relatedIntegrationRemove"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jest.mock('../../../../common/lib/kibana', () => ({
docLinks: {
links: {
securitySolution: {
ruleUiAdvancedParams: 'http://link-to-docs',
createDetectionRules: 'http://link-to-docs',
},
},
},
Expand Down Expand Up @@ -669,82 +669,6 @@ describe('RelatedIntegrations form part', () => {
});
});
});

describe('sticky last form row', () => {
it('does not remove the last item', async () => {
render(<TestForm />, { wrapper: createReactQueryWrapper() });

await addRelatedIntegrationRow();
await removeLastRelatedIntegrationRow();

expect(screen.getAllByTestId(RELATED_INTEGRATION_ROW)).toHaveLength(1);
});

it('disables remove button after clicking remove button on the last item', async () => {
render(<TestForm />, { wrapper: createReactQueryWrapper() });

await addRelatedIntegrationRow();
await removeLastRelatedIntegrationRow();

expect(screen.getByTestId(REMOVE_INTEGRATION_ROW_BUTTON_TEST_ID)).toBeDisabled();
});

it('clears selected integration when clicking remove the last form row button', async () => {
render(<TestForm />, { wrapper: createReactQueryWrapper() });

await addRelatedIntegrationRow();
await selectFirstEuiComboBoxOption({
comboBoxToggleButton: getLastByTestId(COMBO_BOX_TOGGLE_BUTTON_TEST_ID),
});
await removeLastRelatedIntegrationRow();

expect(screen.queryByTestId(COMBO_BOX_SELECTION_TEST_ID)).not.toBeInTheDocument();
});

it('submits an empty integration after clicking remove the last form row button', async () => {
const handleSubmit = jest.fn();

render(<TestForm onSubmit={handleSubmit} />, { wrapper: createReactQueryWrapper() });

await addRelatedIntegrationRow();
await selectFirstEuiComboBoxOption({
comboBoxToggleButton: getLastByTestId(COMBO_BOX_TOGGLE_BUTTON_TEST_ID),
});
await removeLastRelatedIntegrationRow();
await submitForm();
await waitFor(() => {
expect(handleSubmit).toHaveBeenCalled();
});

expect(handleSubmit).toHaveBeenCalledWith({
data: [{ package: '', version: '' }],
isValid: true,
});
});

it('submits an empty integration after previously saved integrations were removed', async () => {
const initialRelatedIntegrations: RelatedIntegration[] = [
{ package: 'package-a', version: '^1.2.3' },
];
const handleSubmit = jest.fn();

render(<TestForm initialState={initialRelatedIntegrations} onSubmit={handleSubmit} />, {
wrapper: createReactQueryWrapper(),
});

await waitForIntegrationsToBeLoaded();
await removeLastRelatedIntegrationRow();
await submitForm();
await waitFor(() => {
expect(handleSubmit).toHaveBeenCalled();
});

expect(handleSubmit).toHaveBeenCalledWith({
data: [{ package: '', version: '' }],
isValid: true,
});
});
});
});
});

Expand Down Expand Up @@ -778,11 +702,6 @@ function TestForm({ initialState, onSubmit }: TestFormProps): JSX.Element {
);
}

function getLastByTestId(testId: string): HTMLElement {
// getAllByTestId throws an error when there are no `testId` elements found
return screen.getAllByTestId(testId).at(-1)!;
}

function waitForIntegrationsToBeLoaded(): Promise<void> {
return waitForElementToBeRemoved(screen.queryAllByRole('progressbar'));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function RelatedIntegrationsHelpInfo(): JSX.Element {
defaultMessage="Choose the {integrationsDocLink} this rule depends on, and correct if necessary each integration’s version constraint in {semverLink} format. Only tilde, caret, and plain versions are supported, such as ~1.2.3, ^1.2.3, or 1.2.3."
values={{
integrationsDocLink: (
<EuiLink href={docLinks.links.securitySolution.ruleUiAdvancedParams} target="_blank">
<EuiLink href={docLinks.links.securitySolution.createDetectionRules} target="_blank">
<FormattedMessage
id="xpack.securitySolution.detectionEngine.ruleDescription.relatedIntegrations.integrationsLink"
defaultMessage="integrations"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export { RequiredFields } from './required_fields';
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import type { RequiredFieldInput } from '../../../../../common/api/detection_engine/model/rule_schema/common_attributes.gen';
import type { ERROR_CODE, FormData, ValidationFunc } from '../../../../shared_imports';
import * as i18n from './translations';

export function makeValidateRequiredField(parentFieldPath: string) {
return function validateRequiredField(
...args: Parameters<ValidationFunc<FormData, string, RequiredFieldInput>>
): ReturnType<ValidationFunc<{}, ERROR_CODE>> | undefined {
const [{ value, path, form }] = args;

const formData = form.getFormData();
const parentFieldData: RequiredFieldInput[] = formData[parentFieldPath];

const isFieldNameUsedMoreThanOnce =
parentFieldData.filter((field) => field.name === value.name).length > 1;

if (isFieldNameUsedMoreThanOnce) {
return {
code: 'ERR_FIELD_FORMAT',
path: `${path}.name`,
message: i18n.FIELD_NAME_USED_MORE_THAN_ONCE(value.name),
};
}

/* Allow empty rows. They are going to be removed before submission. */
if (value.name.trim().length === 0 && value.type.trim().length === 0) {
return;
}

if (value.name.trim().length === 0) {
return {
code: 'ERR_FIELD_MISSING',
path: `${path}.name`,
message: i18n.FIELD_NAME_REQUIRED,
};
}

if (value.type.trim().length === 0) {
return {
code: 'ERR_FIELD_MISSING',
path: `${path}.type`,
message: i18n.FIELD_TYPE_REQUIRED,
};
}
};
}
Loading

0 comments on commit 6eeffd3

Please sign in to comment.