Skip to content

Commit

Permalink
[Cases] Removing escapeKuery calls in nodeBuilder (elastic#159815)
Browse files Browse the repository at this point in the history 
Fixes: elastic#159741

This PR removes calls to `escapeKuery` where the result is passed
directly to `nodeBuilder`. `nodeBuilder` expects the raw contents that
should not be escaped. Escaping is only need when the result is passed
to `fromKueryExpression` or `buildNode`.


## Testing

Try creating a case with tags that have kql characters like: `\\():<>"*`
and try filtering the cases list page for those tags. The case should be
displayed in the table.

## Release Notes

Cases fixed an issue where the following special characters could not be
included in the case tags: `\\():<>"*` because it resulted in a bug
where the case would not be displayed in the cases table when filtered
for those tags. These characters are now handled correctly and the cases
will be shown in the table.
  • Loading branch information
@jonathan-buttner
jonathan-buttner authored Jun 19, 2023
1 parent 52978a6 commit f91ee9c
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 205 deletions.
259 changes: 101 additions & 158 deletions x-pack/plugins/cases/server/client/utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { CaseSeverity } from '../../common/api';
import { createSavedObjectsSerializerMock } from './mocks';
import {
arraysDifference,
buildNestedFilter,
buildFilter,
buildRangeFilter,
constructQueryOptions,
constructSearch,
Expand All @@ -24,6 +24,106 @@ import {
import { CasePersistedSeverity, CasePersistedStatus } from '../common/types/case';

describe('utils', () => {
describe('buildFilter', () => {
it('returns undefined if filters is undefined', () => {
expect(buildFilter({ filters: undefined, field: 'abc', operator: 'or' })).toBeUndefined();
});

it('returns undefined if filters is is an empty array', () => {
expect(buildFilter({ filters: [], field: 'abc', operator: 'or' })).toBeUndefined();
});

it('returns a KueryNode using or operator', () => {
expect(buildFilter({ filters: ['value1'], field: 'abc', operator: 'or' }))
.toMatchInlineSnapshot(`
Object {
"arguments": Array [
Object {
"isQuoted": false,
"type": "literal",
"value": "cases.attributes.abc",
},
Object {
"isQuoted": false,
"type": "literal",
"value": "value1",
},
],
"function": "is",
"type": "function",
}
`);
});

it("returns multiple nodes or'd together", () => {
expect(buildFilter({ filters: ['value1', 'value2'], field: 'abc', operator: 'or' }))
.toMatchInlineSnapshot(`
Object {
"arguments": Array [
Object {
"arguments": Array [
Object {
"isQuoted": false,
"type": "literal",
"value": "cases.attributes.abc",
},
Object {
"isQuoted": false,
"type": "literal",
"value": "value1",
},
],
"function": "is",
"type": "function",
},
Object {
"arguments": Array [
Object {
"isQuoted": false,
"type": "literal",
"value": "cases.attributes.abc",
},
Object {
"isQuoted": false,
"type": "literal",
"value": "value2",
},
],
"function": "is",
"type": "function",
},
],
"function": "or",
"type": "function",
}
`);
});

it('does not escape special kql characters in the filter values', () => {
const specialCharacters = 'awesome:()\\<>"*';

expect(buildFilter({ filters: [specialCharacters], field: 'abc', operator: 'or' }))
.toMatchInlineSnapshot(`
Object {
"arguments": Array [
Object {
"isQuoted": false,
"type": "literal",
"value": "cases.attributes.abc",
},
Object {
"isQuoted": false,
"type": "literal",
"value": "awesome:()\\\\<>\\"*",
},
],
"function": "is",
"type": "function",
}
`);
});
});

describe('convertSortField', () => {
it('transforms status correctly', () => {
expect(convertSortField('status')).toBe('status');
Expand Down Expand Up @@ -589,163 +689,6 @@ describe('utils', () => {
});
});

describe('buildNestedFilter', () => {
it('returns undefined if filters is undefined', () => {
expect(buildNestedFilter({ field: '', nestedField: '', operator: 'or' })).toBeUndefined();
});

it('returns undefined when the filters array is empty', () => {
expect(
buildNestedFilter({ filters: [], field: '', nestedField: '', operator: 'or' })
).toBeUndefined();
});

it('returns a KueryNode for a single filter', () => {
expect(
toElasticsearchQuery(
buildNestedFilter({
filters: ['hello'],
field: 'uid',
nestedField: 'nestedField',
operator: 'or',
})!
)
).toMatchInlineSnapshot(`
Object {
"nested": Object {
"path": "cases.attributes.nestedField",
"query": Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"cases.attributes.nestedField.uid": "hello",
},
},
],
},
},
"score_mode": "none",
},
}
`);
});

it("returns a KueryNode for multiple filters or'd together", () => {
expect(
toElasticsearchQuery(
buildNestedFilter({
filters: ['uid1', 'uid2'],
field: 'uid',
nestedField: 'nestedField',
operator: 'or',
})!
)
).toMatchInlineSnapshot(`
Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"nested": Object {
"path": "cases.attributes.nestedField",
"query": Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"cases.attributes.nestedField.uid": "uid1",
},
},
],
},
},
"score_mode": "none",
},
},
Object {
"nested": Object {
"path": "cases.attributes.nestedField",
"query": Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"cases.attributes.nestedField.uid": "uid2",
},
},
],
},
},
"score_mode": "none",
},
},
],
},
}
`);
});

it("returns a KueryNode for multiple filters and'ed together", () => {
expect(
toElasticsearchQuery(
buildNestedFilter({
filters: ['uid1', 'uid2'],
field: 'uid',
nestedField: 'nestedField',
operator: 'and',
})!
)
).toMatchInlineSnapshot(`
Object {
"bool": Object {
"filter": Array [
Object {
"nested": Object {
"path": "cases.attributes.nestedField",
"query": Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"cases.attributes.nestedField.uid": "uid1",
},
},
],
},
},
"score_mode": "none",
},
},
Object {
"nested": Object {
"path": "cases.attributes.nestedField",
"query": Object {
"bool": Object {
"minimum_should_match": 1,
"should": Array [
Object {
"match": Object {
"cases.attributes.nestedField.uid": "uid2",
},
},
],
},
},
"score_mode": "none",
},
},
],
},
}
`);
});
});

describe('arraysDifference', () => {
it('returns null if originalValue is null', () => {
expect(arraysDifference(null, [])).toBeNull();
Expand Down
49 changes: 2 additions & 47 deletions x-pack/plugins/cases/server/client/utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,10 +197,6 @@ interface FilterField {
type?: string;
}

interface NestedFilterField extends FilterField {
nestedField: string;
}

export const buildFilter = ({
filters,
field,
Expand All @@ -218,48 +214,7 @@ export const buildFilter = ({
}

return nodeBuilder[operator](
filtersAsArray.map((filter) =>
nodeBuilder.is(`${escapeKuery(type)}.attributes.${escapeKuery(field)}`, escapeKuery(filter))
)
);
};

/**
* Creates a KueryNode filter for the Saved Object find API's filter field. This handles constructing a filter for
* a nested field.
*
* @param filters is a string or array of strings that defines the values to search for
* @param field is the location to search for
* @param nestedField is the field in the saved object that has a type of 'nested'
* @param operator whether to 'or'/'and' the created filters together
* @type the type of saved object being searched
* @returns a constructed KueryNode representing the filter or undefined if one could not be built
*/
export const buildNestedFilter = ({
filters,
field,
nestedField,
operator,
type = CASE_SAVED_OBJECT,
}: NestedFilterField): KueryNode | undefined => {
if (filters === undefined) {
return;
}

const filtersAsArray = Array.isArray(filters) ? filters : [filters];

if (filtersAsArray.length === 0) {
return;
}

return nodeBuilder[operator](
filtersAsArray.map((filter) =>
fromKueryExpression(
`${escapeKuery(type)}.attributes.${escapeKuery(nestedField)}:{ ${escapeKuery(
field
)}: ${escapeKuery(filter)} }`
)
)
filtersAsArray.map((filter) => nodeBuilder.is(`${type}.attributes.${field}`, filter))
);
};

Expand Down Expand Up @@ -365,7 +320,7 @@ export const buildAssigneesFilter = ({
);

const assigneesFilter = assigneesWithoutNone.map((filter) =>
nodeBuilder.is(`${CASE_SAVED_OBJECT}.attributes.assignees.uid`, escapeKuery(filter))
nodeBuilder.is(`${CASE_SAVED_OBJECT}.attributes.assignees.uid`, filter)
);

if (!hasNoneAssignee) {
Expand Down
23 changes: 23 additions & 0 deletions x-pack/test/cases_api_integration/security_and_spaces/tests/common/cases/find_cases.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,29 @@ export default ({ getService }: FtrProviderContext): void => {
});
});

it('can filter by reserved kql characters in tags', async () => {
const tagsColon = ['super:bad:case'];
const tagsSlashQuote = ['awesome\\"'];

const postedCase1 = await createCase(supertest, { ...postCaseReq, tags: tagsSlashQuote });
const postedCase2 = await createCase(supertest, {
...postCaseReq,
tags: tagsColon,
});

const cases = await findCases({
supertest,
query: { tags: [...tagsColon, ...tagsSlashQuote] },
});

expect(cases).to.eql({
...findCasesResp,
total: 2,
cases: [postedCase1, postedCase2],
count_open_cases: 2,
});
});

it('filters by status', async () => {
await createCase(supertest, postCaseReq);
const toCloseCase = await createCase(supertest, postCaseReq);
Expand Down
Loading

0 comments on commit f91ee9c

Please sign in to comment.