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] Incorporates EQL options in EQL query validation on both Rule Creation and Timeline #178468

Merged
merged 25 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
02a0c50
Document current EQL rule behavior with timestamp fields
rylnd Feb 23, 2024
7838039
Add similar tests for data with @timestamp
rylnd Feb 23, 2024
413ed8c
Add tests validating that our EQL search strategy respects certain pa…
rylnd Feb 24, 2024
cedecbb
EQL validation function accepts EQL options as a parameter
rylnd Mar 11, 2024
b2fa55c
Makes eqlOptions a used field on our Define Step form
rylnd Mar 12, 2024
3dc18c1
Ensure that changes to EQL options cause the EQL query to be revalidated
rylnd Mar 12, 2024
4105e69
Add a test validating how onOptionsChange is called
rylnd Mar 12, 2024
39d005b
Remove unused interface
rylnd Mar 12, 2024
daf87a4
Add an eqlOptions form field to the timeline EQL form
rylnd Mar 12, 2024
10a47fd
Remove default values from timeline EQL options
rylnd Mar 12, 2024
2676ada
Merge branch 'main' into rylnd/bugfix/eql_rule_timestamp_override
rylnd Mar 12, 2024
bbd6a8d
Rename new archive to be more accurate
rylnd Mar 12, 2024
fa03d22
Rename archive, again
rylnd Mar 12, 2024
b2aec76
More accurate cypress selector
rylnd Mar 12, 2024
0d8ad8b
Add a cypress test around the EQL option/validation functionality
rylnd Mar 12, 2024
3a9d040
Prevent component props from being passed as DOM attributes
rylnd Mar 13, 2024
532dd15
Revert "Remove default values from timeline EQL options"
rylnd Mar 13, 2024
5b18854
Guard against using timeline default of '' for tibreakerField
rylnd Mar 13, 2024
e837f19
validateEql options parameter is optional
rylnd Mar 13, 2024
f6739dd
Merge branch 'main' into rylnd/bugfix/eql_rule_timestamp_override
rylnd Mar 14, 2024
c970f74
Merge branch 'main' into rylnd/bugfix/eql_rule_timestamp_override
rylnd Mar 15, 2024
7ac409a
Remove duplicated es_archive thanks to new cypress task functionality
rylnd Mar 15, 2024
92a2bd2
Merge branch 'main' into rylnd/bugfix/eql_rule_timestamp_override
rylnd Mar 18, 2024
6767182
Fix type incompatibility with EQL search strategy
rylnd Mar 18, 2024
f3e55d5
Fix test descriptions to align with test bodies
rylnd Mar 18, 2024
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 @@ -277,6 +277,51 @@ describe('EQL search strategy', () => {

expect(requestOptions).toEqual({ ignore: [400], meta: true, signal: undefined });
});

describe('EQL-specific arguments', () => {
it('passes along a timestamp_field argument', async () => {
const eqlSearch = eqlSearchStrategyProvider(mockSearchConfig, mockLogger);
const request: EqlSearchStrategyRequest = {
// @ts-expect-error timestamp_field not allowed at top level when using `typesWithBodyKey`
params: { index: 'all', timestamp_field: 'timestamp' },
};

await firstValueFrom(eqlSearch.search(request, {}, mockDeps));
const [[actualParams]] = mockEqlSearch.mock.calls;

expect(actualParams).toEqual(expect.objectContaining({ timestamp_field: 'timestamp' }));
});

it('passes along an event_category_field argument', async () => {
const eqlSearch = eqlSearchStrategyProvider(mockSearchConfig, mockLogger);
const request: EqlSearchStrategyRequest = {
// @ts-expect-error timestamp_field not allowed at top level when using `typesWithBodyKey`
params: { index: 'all', event_category_field: 'event_category' },
};

await firstValueFrom(eqlSearch.search(request, {}, mockDeps));
const [[actualParams]] = mockEqlSearch.mock.calls;

expect(actualParams).toEqual(
expect.objectContaining({ event_category_field: 'event_category' })
);
});

it('passes along a tiebreaker_field argument', async () => {
const eqlSearch = eqlSearchStrategyProvider(mockSearchConfig, mockLogger);
const request: EqlSearchStrategyRequest = {
// @ts-expect-error tiebreaker_field not allowed at top level when using `typesWithBodyKey`
params: { index: 'all', tiebreaker_field: 'event_category' },
};

await firstValueFrom(eqlSearch.search(request, {}, mockDeps));
const [[actualParams]] = mockEqlSearch.mock.calls;

expect(actualParams).toEqual(
expect.objectContaining({ tiebreaker_field: 'event_category' })
);
});
});
});

describe('response', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { EqlSearchStrategyRequest, EqlSearchStrategyResponse } from '@kbn/d
import { EQL_SEARCH_STRATEGY } from '@kbn/data-plugin/common';
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';

import type { EqlOptionsSelected } from '../../../../common/search_strategy';
import {
getValidationErrors,
isErrorResponse,
Expand All @@ -23,6 +24,7 @@ interface Params {
data: DataPublicPluginStart;
signal: AbortSignal;
runtimeMappings: estypes.MappingRuntimeFields | undefined;
options: Omit<EqlOptionsSelected, 'query' | 'size'> | undefined;
}

export const validateEql = async ({
Expand All @@ -31,13 +33,18 @@ export const validateEql = async ({
query,
signal,
runtimeMappings,
options,
}: Params): Promise<{ valid: boolean; errors: string[] }> => {
const { rawResponse: response } = await firstValueFrom(
data.search.search<EqlSearchStrategyRequest, EqlSearchStrategyResponse>(
{
params: {
index: dataViewTitle,
body: { query, runtime_mappings: runtimeMappings, size: 0 },
// @ts-expect-error top-level keys are unexpected in {@link EqlSearchRequest}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this error happens here?
I don't see any top level await introduced here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These @ts-expect-errors were due to an incompatibility between the actual EQL endpoint, and the types in the search strategy. I managed to extend the type to make this all work without the need for the @ts-expect-error: 6767182.

I'm not sure what you were referring to with the await comment, though.

timestamp_field: options?.timestampField,
tiebreaker_field: options?.tiebreakerField || undefined,
event_category_field: options?.eventCategoryField,
},
options: { ignore: [400] },
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { mockQueryBar } from '../../../rule_management_ui/components/rules_table
import type { EqlQueryBarProps } from './eql_query_bar';
import { EqlQueryBar } from './eql_query_bar';
import { getEqlValidationError } from './validators.mock';
import { fireEvent, render, within } from '@testing-library/react';

jest.mock('../../../../common/lib/kibana');

Expand Down Expand Up @@ -117,4 +118,38 @@ describe('EqlQueryBar', () => {

expect(wrapper.find('[data-test-subj="eql-validation-errors-popover"]').exists()).toEqual(true);
});

describe('EQL options interaction', () => {
const mockOptionsData = {
keywordFields: [],
dateFields: [{ label: 'timestamp', value: 'timestamp' }],
nonDateFields: [],
};

it('invokes onOptionsChange when the EQL options change', () => {
const onOptionsChangeMock = jest.fn();

const { getByTestId, getByText } = render(
<TestProviders>
<EqlQueryBar
dataTestSubj="myQueryBar"
field={mockField}
isLoading={false}
optionsData={mockOptionsData}
indexPattern={mockIndexPattern}
onOptionsChange={onOptionsChangeMock}
/>
</TestProviders>
);

// open options popover
fireEvent.click(getByTestId('eql-settings-trigger'));
// display combobox options
within(getByTestId(`eql-timestamp-field`)).getByRole('combobox').focus();
// select timestamp
getByText('timestamp').click();

expect(onOptionsChangeMock).toHaveBeenCalledWith('timestampField', 'timestamp');
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Subscription } from 'rxjs';
import styled from 'styled-components';
import deepEqual from 'fast-deep-equal';
import { EuiFormRow, EuiSpacer, EuiTextArea } from '@elastic/eui';
import type { DataViewBase, Filter, Query } from '@kbn/es-query';
import type { DataViewBase } from '@kbn/es-query';
import { FilterManager } from '@kbn/data-plugin/public';

import type { FieldHook } from '../../../../shared_imports';
Expand Down Expand Up @@ -58,12 +58,6 @@ const StyledFormRow = styled(EuiFormRow)`
}
`;

export interface FieldValueQueryBar {
filters: Filter[];
query: Query;
saved_id?: string;
}

export interface EqlQueryBarProps {
dataTestSubj: string;
field: FieldHook<DefineStepRule['queryBar']>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const eqlValidator = async (
const [{ value, formData }] = args;
const { query: queryValue } = value as FieldValueQueryBar;
const query = queryValue.query as string;
const { dataViewId, index, ruleType } = formData as DefineStepRule;
const { dataViewId, index, ruleType, eqlOptions } = formData as DefineStepRule;

const needsValidation =
(ruleType === undefined && !isEmpty(query)) || (isEqlRule(ruleType) && !isEmpty(query));
Expand All @@ -82,7 +82,14 @@ export const eqlValidator = async (
}

const signal = new AbortController().signal;
const response = await validateEql({ data, query, signal, dataViewTitle, runtimeMappings });
const response = await validateEql({
data,
query,
signal,
dataViewTitle,
runtimeMappings,
options: eqlOptions,
});

if (response?.valid === false) {
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@ import { StepContentWrapper } from '../../../rule_creation/components/step_conte
import { ThresholdInput } from '../threshold_input';
import { SuppressionInfoIcon } from '../suppression_info_icon';
import { EsqlInfoIcon } from '../../../rule_creation/components/esql_info_icon';
import { Field, Form, getUseField, UseField, UseMultiFields } from '../../../../shared_imports';
import {
Field,
Form,
getUseField,
HiddenField,
UseField,
UseMultiFields,
} from '../../../../shared_imports';
import type { FormHook } from '../../../../shared_imports';
import { schema } from './schema';
import { getTermsAggregationFields } from './utils';
Expand Down Expand Up @@ -768,14 +775,20 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
onOpenTimeline,
]
);

const onOptionsChange = useCallback(
(field: FieldsEqlOptions, value: string | undefined) => {
setOptionsSelected((prevOptions) => ({
...prevOptions,
[field]: value,
}));
setOptionsSelected((prevOptions) => {
const newOptions = {
...prevOptions,
[field]: value,
};

setFieldValue('eqlOptions', newOptions);
return newOptions;
});
},
[setOptionsSelected]
[setFieldValue, setOptionsSelected]
);

const optionsData = useMemo(
Expand Down Expand Up @@ -814,17 +827,16 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
<>
<StepContentWrapper addPadding={!isUpdateView}>
<Form form={form} data-test-subj="stepDefineRule">
<StyledVisibleContainer isVisible={false}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wholly unrelated to this PR, but react was throwing dev warnings on this page and I couldn't help but fix it. See 3a9d040 for more details.

<UseField
path="dataSourceType"
componentProps={{
euiFieldProps: {
fullWidth: true,
placeholder: '',
},
}}
/>
</StyledVisibleContainer>
<UseField
path="dataSourceType"
component={HiddenField}
componentProps={{
euiFieldProps: {
fullWidth: true,
placeholder: '',
},
}}
/>
<UseField
path="ruleType"
component={SelectRuleType}
Expand All @@ -838,29 +850,31 @@ const StepDefineRuleComponent: FC<StepDefineRuleProps> = ({
</StyledVisibleContainer>
<EuiSpacer size="s" />
{isEqlRule(ruleType) ? (
<UseField
key="EqlQueryBar"
path="queryBar"
component={EqlQueryBar}
componentProps={{
optionsData,
optionsSelected,
isSizeOptionDisabled: true,
onOptionsChange,
onValidityChange: setIsQueryBarValid,
idAria: 'detectionEngineStepDefineRuleEqlQueryBar',
isDisabled: isLoading,
isLoading: isIndexPatternLoading,
indexPattern,
showFilterBar: true,
// isLoading: indexPatternsLoading,
dataTestSubj: 'detectionEngineStepDefineRuleEqlQueryBar',
}}
config={{
...schema.queryBar,
label: i18n.EQL_QUERY_BAR_LABEL,
}}
/>
<>
<UseField
key="EqlQueryBar"
path="queryBar"
component={EqlQueryBar}
componentProps={{
optionsData,
optionsSelected,
isSizeOptionDisabled: true,
onOptionsChange,
onValidityChange: setIsQueryBarValid,
idAria: 'detectionEngineStepDefineRuleEqlQueryBar',
isDisabled: isLoading,
isLoading: isIndexPatternLoading,
indexPattern,
showFilterBar: true,
dataTestSubj: 'detectionEngineStepDefineRuleEqlQueryBar',
}}
config={{
...schema.queryBar,
label: i18n.EQL_QUERY_BAR_LABEL,
}}
/>
<UseField path="eqlOptions" component={HiddenField} />
</>
) : isEsqlRule(ruleType) ? (
EsqlQueryBarMemo
) : (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,10 @@ export const schema: FormSchema<DefineStepRule> = {
),
validations: [],
},
eqlOptions: {},
eqlOptions: {
fieldsToValidateOnChange: ['eqlOptions', 'queryBar'],
},
queryBar: {
fieldsToValidateOnChange: ['queryBar'],
validations: [
{
validator: (
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/security_solution/public/shared_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ export {
useFormData,
VALIDATION_TYPES,
} from '@kbn/es-ui-shared-plugin/static/forms/hook_form_lib';
export { Field, SelectField } from '@kbn/es-ui-shared-plugin/static/forms/components';
export { Field, SelectField, HiddenField } from '@kbn/es-ui-shared-plugin/static/forms/components';
export { fieldValidators } from '@kbn/es-ui-shared-plugin/static/forms/helpers';
export type { ERROR_CODE } from '@kbn/es-ui-shared-plugin/static/forms/helpers/field_validators/types';
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import React, { memo, useCallback, useEffect, useMemo, useRef, useState } from '
import { useDispatch } from 'react-redux';
import styled from 'styled-components';

import type { FieldsEqlOptions } from '../../../../../../common/search_strategy';
import type {
EqlOptionsSelected,
FieldsEqlOptions,
} from '../../../../../../common/search_strategy';
import { useSourcererDataView } from '../../../../../common/containers/sourcerer';
import { useDeepEqualSelector } from '../../../../../common/hooks/use_selector';
import { SourcererScopeName } from '../../../../../common/store/sourcerer/model';
Expand All @@ -31,6 +34,7 @@ import { getEqlOptions } from './selectors';
interface TimelineEqlQueryBar {
index: string[];
eqlQueryBar: FieldValueQueryBar;
eqlOptions: EqlOptionsSelected;
}

const defaultValues = {
Expand All @@ -40,13 +44,17 @@ const defaultValues = {
filters: [],
saved_id: null,
},
eqlOptions: {},
};

const schema: FormSchema<TimelineEqlQueryBar> = {
index: {
fieldsToValidateOnChange: ['index', 'eqlQueryBar'],
validations: [],
},
eqlOptions: {
fieldsToValidateOnChange: ['eqlOptions', 'eqlQueryBar'],
},
eqlQueryBar: {
validations: [
{
Expand Down Expand Up @@ -89,18 +97,20 @@ export const EqlQueryBarTimeline = memo(({ timelineId }: { timelineId: string })
options: { stripEmptyFields: false },
schema,
});
const { getFields } = form;
const { getFields, setFieldValue } = form;

const onOptionsChange = useCallback(
(field: FieldsEqlOptions, value: string | undefined) =>
(field: FieldsEqlOptions, value: string | undefined) => {
dispatch(
timelineActions.updateEqlOptions({
id: timelineId,
field,
value,
})
),
[dispatch, timelineId]
);
setFieldValue('eqlOptions', { ...optionsSelected, [field]: value });
},
[dispatch, optionsSelected, setFieldValue, timelineId]
);

const [{ eqlQueryBar: formEqlQueryBar }] = useFormData<TimelineEqlQueryBar>({
Expand Down Expand Up @@ -179,6 +189,7 @@ export const EqlQueryBarTimeline = memo(({ timelineId }: { timelineId: string })
return (
<Form form={form} data-test-subj="EqlQueryBarTimeline">
<HiddenUseField key="Index" path="index" />
<HiddenUseField key="EqlOptions" path="eqlOptions" />
<UseField
key="EqlQueryBar"
path="eqlQueryBar"
Expand Down
Loading