Skip to content

Commit

Permalink
[8.8] [Security Solution] Add support for multiple values in cell act…
Browse files Browse the repository at this point in the history
…ions (#158060) (#158104)

# Backport

This will backport the following commits from `main` to `8.8`:
- [[Security Solution] Add support for multiple values in cell actions
(#158060)](#158060)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Sergi
Massaneda","email":"sergi.massaneda@elastic.co"},"sourceCommit":{"committedDate":"2023-05-18T18:49:47Z","message":"[Security
Solution] Add support for multiple values in cell actions
(#158060)\n\n## Summary\r\n\r\nfixes:
#157237:
#157887 \r\n\r\nEnables cell
actions package to support multi-valuated cells.
Actions\r\naffected:\r\n- Filter In\r\n- Filter Out\r\n- Add To
Timeline\r\n- Copy To Clipboard\r\n\r\n###
Recording\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/17747913/a192173d-5fca-4b33-91a7-664ecd71550b\r\n\r\n####
Caveat\r\nThe `FilterManager` does not recognize the duplicated filter
when using\r\nthe new `Combined` filters (the ones that allow AND/OR
operations), so\r\nwhen adding two opposite combined filters, it does
not remove the first\r\none and both are applied at the same
time:\r\n\r\n\r\n![combined_filters_problem](https://github.com/elastic/kibana/assets/17747913/dc2f60db-28e7-4656-aaa2-d8550e8ef128)","sha":"4dd1373571c99540d8ca04846f1e2aeb55fbe80f","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Threat
Hunting","Team:Threat
Hunting:Explore","v8.9.0","v8.8.1"],"number":158060,"url":"#158060
Solution] Add support for multiple values in cell actions
(#158060)\n\n## Summary\r\n\r\nfixes:
#157237:
#157887 \r\n\r\nEnables cell
actions package to support multi-valuated cells.
Actions\r\naffected:\r\n- Filter In\r\n- Filter Out\r\n- Add To
Timeline\r\n- Copy To Clipboard\r\n\r\n###
Recording\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/17747913/a192173d-5fca-4b33-91a7-664ecd71550b\r\n\r\n####
Caveat\r\nThe `FilterManager` does not recognize the duplicated filter
when using\r\nthe new `Combined` filters (the ones that allow AND/OR
operations), so\r\nwhen adding two opposite combined filters, it does
not remove the first\r\none and both are applied at the same
time:\r\n\r\n\r\n![combined_filters_problem](#158060
Solution] Add support for multiple values in cell actions
(#158060)\n\n## Summary\r\n\r\nfixes:
#157237:
#157887 \r\n\r\nEnables cell
actions package to support multi-valuated cells.
Actions\r\naffected:\r\n- Filter In\r\n- Filter Out\r\n- Add To
Timeline\r\n- Copy To Clipboard\r\n\r\n###
Recording\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/17747913/a192173d-5fca-4b33-91a7-664ecd71550b\r\n\r\n####
Caveat\r\nThe `FilterManager` does not recognize the duplicated filter
when using\r\nthe new `Combined` filters (the ones that allow AND/OR
operations), so\r\nwhen adding two opposite combined filters, it does
not remove the first\r\none and both are applied at the same
time:\r\n\r\n\r\n![combined_filters_problem](https://github.com/elastic/kibana/assets/17747913/dc2f60db-28e7-4656-aaa2-d8550e8ef128)","sha":"4dd1373571c99540d8ca04846f1e2aeb55fbe80f"}},{"branch":"8.8","label":"v8.8.1","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Sergi Massaneda <sergi.massaneda@elastic.co>
  • Loading branch information
kibanamachine and semd committed Jun 6, 2023
1 parent 89771e7 commit ec659b1
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 85 deletions.
Expand Up @@ -48,5 +48,25 @@ describe('Default createCopyToClipboardActionFactory', () => {
expect(mockCopy).toHaveBeenCalledWith('user.name: "the value"');
expect(mockSuccessToast).toHaveBeenCalled();
});

it('should escape value', async () => {
await copyToClipboardAction.execute({
...context,
field: { ...context.field, value: 'the "value"' },
});
expect(mockCopy).toHaveBeenCalledWith('user.name: "the \\"value\\""');
expect(mockSuccessToast).toHaveBeenCalled();
});

it('should suport multiple values', async () => {
await copyToClipboardAction.execute({
...context,
field: { ...context.field, value: ['the "value"', 'another value', 'last value'] },
});
expect(mockCopy).toHaveBeenCalledWith(
'user.name: "the \\"value\\"" AND "another value" AND "last value"'
);
expect(mockSuccessToast).toHaveBeenCalled();
});
});
});
Expand Up @@ -23,6 +23,8 @@ const COPY_TO_CLIPBOARD_SUCCESS = i18n.translate(
}
);

const escapeValue = (value: string) => value.replace(/"/g, '\\"');

export const createCopyToClipboardActionFactory = createCellActionFactory(
({ notifications }: { notifications: NotificationsStart }) => ({
type: COPY_CELL_ACTION_TYPE,
Expand All @@ -34,8 +36,8 @@ export const createCopyToClipboardActionFactory = createCellActionFactory(
let textValue: undefined | string;
if (field.value != null) {
textValue = Array.isArray(field.value)
? field.value.map((value) => `"${value}"`).join(', ')
: `"${field.value}"`;
? field.value.map((value) => `"${escapeValue(value)}"`).join(' AND ')
: `"${escapeValue(field.value)}"`;
}
const text = textValue ? `${field.name}: ${textValue}` : field.name;
const isSuccess = copy(text, { debug: true });
Expand Down
41 changes: 27 additions & 14 deletions packages/kbn-cell-actions/src/actions/filter/create_filter.test.ts
Expand Up @@ -18,21 +18,17 @@ describe('createFilter', () => {
])('should return filter with $caseName value', ({ caseValue }) => {
expect(createFilter({ key: field, value: caseValue, negate: false })).toEqual({
meta: {
alias: null,
disabled: false,
type: 'phrase',
key: field,
negate: false,
value,
params: {
query: value,
},
},
query: {
match: {
match_phrase: {
[field]: {
query: value,
type: 'phrase',
},
},
},
Expand All @@ -45,27 +41,48 @@ describe('createFilter', () => {
])('should return negate filter with $caseName value', ({ caseValue }) => {
expect(createFilter({ key: field, value: caseValue, negate: true })).toEqual({
meta: {
alias: null,
disabled: false,
type: 'phrase',
key: field,
negate: true,
value,
params: {
query: value,
},
},
query: {
match: {
match_phrase: {
[field]: {
query: value,
type: 'phrase',
},
},
},
});
});

it.each([
{ caseName: 'non-negated', negate: false },
{ caseName: 'negated', negate: true },
])('should return combined filter with multiple $caseName values', ({ negate }) => {
const value2 = 'the-value2';
expect(createFilter({ key: field, value: [value, value2], negate })).toEqual({
meta: {
type: 'combined',
relation: 'AND',
key: field,
negate,
params: [
{
meta: { type: 'phrase', key: field, params: { query: value } },
query: { match_phrase: { [field]: { query: value } } },
},
{
meta: { type: 'phrase', key: field, params: { query: value2 } },
query: { match_phrase: { [field]: { query: value2 } } },
},
],
},
});
});

it.each([
{ caseName: 'null', caseValue: null },
{ caseName: 'undefined', caseValue: undefined },
Expand All @@ -79,8 +96,6 @@ describe('createFilter', () => {
},
},
meta: {
alias: null,
disabled: false,
key: field,
negate: false,
type: 'exists',
Expand All @@ -102,8 +117,6 @@ describe('createFilter', () => {
},
},
meta: {
alias: null,
disabled: false,
key: field,
negate: true,
type: 'exists',
Expand Down
94 changes: 57 additions & 37 deletions packages/kbn-cell-actions/src/actions/filter/create_filter.ts
Expand Up @@ -5,10 +5,54 @@
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import type { Filter } from '@kbn/es-query';
import {
BooleanRelation,
FILTERS,
type CombinedFilter,
type ExistsFilter,
type PhraseFilter,
type Filter,
} from '@kbn/es-query';

export const isEmptyFilterValue = (value: string[] | string | null | undefined) =>
value == null || value.length === 0;
export const isEmptyFilterValue = (
value: string[] | string | null | undefined
): value is null | undefined | never[] => value == null || value.length === 0;

const createExistsFilter = ({ key, negate }: { key: string; negate: boolean }): ExistsFilter => ({
meta: { key, negate, type: FILTERS.EXISTS, value: 'exists' },
query: { exists: { field: key } },
});

const createPhraseFilter = ({
key,
negate,
value,
}: {
value: string;
key: string;
negate?: boolean;
}): PhraseFilter => ({
meta: { key, negate, type: FILTERS.PHRASE, params: { query: value } },
query: { match_phrase: { [key]: { query: value } } },
});

const createCombinedFilter = ({
values,
key,
negate,
}: {
values: string[];
key: string;
negate: boolean;
}): CombinedFilter => ({
meta: {
key,
negate,
type: FILTERS.COMBINED,
relation: BooleanRelation.AND,
params: values.map((value) => createPhraseFilter({ key, value })),
},
});

export const createFilter = ({
key,
Expand All @@ -19,39 +63,15 @@ export const createFilter = ({
value: string[] | string | null | undefined;
negate: boolean;
}): Filter => {
const queryValue = !isEmptyFilterValue(value) ? (Array.isArray(value) ? value[0] : value) : null;
const meta = { alias: null, disabled: false, key, negate };

if (queryValue == null) {
return {
query: {
exists: {
field: key,
},
},
meta: {
...meta,
type: 'exists',
value: 'exists',
},
};
if (isEmptyFilterValue(value)) {
return createExistsFilter({ key, negate });
}
if (Array.isArray(value)) {
if (value.length > 1) {
return createCombinedFilter({ key, negate, values: value });
} else {
return createPhraseFilter({ key, negate, value: value[0] });
}
}
return {
meta: {
...meta,
type: 'phrase',
value: queryValue,
params: {
query: queryValue,
},
},
query: {
match: {
[key]: {
query: queryValue,
type: 'phrase',
},
},
},
};
return createPhraseFilter({ key, negate, value });
};
Expand Up @@ -12,6 +12,7 @@ import { createAddToTimelineCellActionFactory } from './add_to_timeline';
import type { CellActionExecutionContext } from '@kbn/cell-actions';
import { GEO_FIELD_TYPE } from '../../../timelines/components/timeline/body/renderers/constants';
import { createStartServicesMock } from '../../../common/lib/kibana/kibana_react.mock';
import { set } from 'lodash/fp';

const services = createStartServicesMock();
const mockWarningToast = services.notifications.toasts.addWarning;
Expand All @@ -28,24 +29,24 @@ const context = {
} as CellActionExecutionContext;

const defaultDataProvider = {
and: [],
enabled: true,
excluded: false,
id: 'event-field-default-timeline-1-user_name-0-the-value',
kqlQuery: '',
name: 'user.name',
queryMatch: {
field: 'user.name',
operator: ':',
value: 'the-value',
},
};

const defaultDataProviderAction = {
type: addProvider.type,
payload: {
id: TimelineId.active,
providers: [
{
and: [],
enabled: true,
excluded: false,
id: 'event-field-default-timeline-1-user_name-0-the-value',
kqlQuery: '',
name: 'user.name',
queryMatch: {
field: 'user.name',
operator: ':',
value: 'the-value',
},
},
],
providers: [defaultDataProvider],
},
};

Expand Down Expand Up @@ -80,9 +81,62 @@ describe('createAddToTimelineCellAction', () => {
});

describe('execute', () => {
it('should execute normally', async () => {
it('should execute with default value', async () => {
await addToTimelineAction.execute(context);
expect(mockDispatch).toHaveBeenCalledWith(defaultDataProvider);
expect(mockDispatch).toHaveBeenCalledWith(defaultDataProviderAction);
expect(mockWarningToast).not.toHaveBeenCalled();
});

it('should execute with null value', async () => {
await addToTimelineAction.execute({
field: { name: 'user.name', value: null, type: 'text' },
} as CellActionExecutionContext);
expect(mockDispatch).toHaveBeenCalledWith(
set(
'payload.providers[0]',
{
...defaultDataProvider,
id: 'empty-value-timeline-1-user_name-0',
excluded: true,
queryMatch: {
field: 'user.name',
value: '',
operator: ':*',
},
},
defaultDataProviderAction
)
);
expect(mockWarningToast).not.toHaveBeenCalled();
});

it('should execute with multiple values', async () => {
const value2 = 'value2';
const value3 = 'value3';
await addToTimelineAction.execute({
field: { name: 'user.name', value: [value, value2, value3], type: 'text' },
} as CellActionExecutionContext);
expect(mockDispatch).toHaveBeenCalledWith(
set(
'payload.providers[0]',
{
...defaultDataProvider,
and: [
{
...defaultDataProvider,
id: 'event-field-default-timeline-1-user_name-0-value2',
queryMatch: { ...defaultDataProvider.queryMatch, value: value2 },
},
{
...defaultDataProvider,
id: 'event-field-default-timeline-1-user_name-0-value3',
queryMatch: { ...defaultDataProvider.queryMatch, value: value3 },
},
],
},
defaultDataProviderAction
)
);
expect(mockWarningToast).not.toHaveBeenCalled();
});

Expand All @@ -106,7 +160,7 @@ describe('createAddToTimelineCellAction', () => {
negateFilters: false,
},
});
expect(mockDispatch).toHaveBeenCalledWith(defaultDataProvider);
expect(mockDispatch).toHaveBeenCalledWith(defaultDataProviderAction);
expect(mockWarningToast).not.toHaveBeenCalled();
});

Expand All @@ -118,10 +172,10 @@ describe('createAddToTimelineCellAction', () => {
},
});
expect(mockDispatch).toHaveBeenCalledWith({
...defaultDataProvider,
...defaultDataProviderAction,
payload: {
...defaultDataProvider.payload,
providers: [{ ...defaultDataProvider.payload.providers[0], excluded: true }],
...defaultDataProviderAction.payload,
providers: [{ ...defaultDataProviderAction.payload.providers[0], excluded: true }],
},
});
expect(mockWarningToast).not.toHaveBeenCalled();
Expand Down

0 comments on commit ec659b1

Please sign in to comment.