Skip to content

Commit

Permalink
add validation for nested filter in find API (#51847)
Browse files Browse the repository at this point in the history
  • Loading branch information
@XavierM
XavierM committed Dec 4, 2019
1 parent b80ae63 commit 58aa0dc
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 53 deletions.
113 changes: 83 additions & 30 deletions src/core/server/saved_objects/service/lib/filter_utils.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,28 @@ const mockMappings = {
},
},
},
alert: {
properties: {
actions: {
type: 'nested',
properties: {
group: {
type: 'keyword',
},
actionRef: {
type: 'keyword',
},
actionTypeId: {
type: 'keyword',
},
params: {
enabled: false,
type: 'object',
},
},
},
},
},
hiddenType: {
properties: {
description: {
Expand Down Expand Up @@ -108,6 +130,16 @@ describe('Filter Utils', () => {
);
});

test('Assemble filter with a nested filter', () => {
expect(
validateConvertFilterToKueryNode(
['alert'],
'alert.attributes.actions:{ actionTypeId: ".server-log" }',
mockMappings
)
).toEqual(esKuery.fromKueryExpression('alert.actions:{ actionTypeId: ".server-log" }'));
});

test('Lets make sure that we are throwing an exception if we get an error', () => {
expect(() => {
validateConvertFilterToKueryNode(
Expand All @@ -129,13 +161,13 @@ describe('Filter Utils', () => {

describe('#validateFilterKueryNode', () => {
test('Validate filter query through KueryNode - happy path', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression(
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)'
),
['foo'],
mockMappings
);
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
Expand Down Expand Up @@ -183,14 +215,34 @@ describe('Filter Utils', () => {
]);
});

test('Validate nested filter query through KueryNode - happy path', () => {
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'alert.attributes.actions:{ actionTypeId: ".server-log" }'
),
types: ['alert'],
indexMapping: mockMappings,
hasNestedKey: true,
});
expect(validationObject).toEqual([
{
astPath: 'arguments.1',
error: null,
isSavedObjectAttr: false,
key: 'alert.attributes.actions.actionTypeId',
type: 'alert',
},
]);
});

test('Return Error if key is not wrapper by a saved object type', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression(
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)'
),
['foo'],
mockMappings
);
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
Expand Down Expand Up @@ -239,13 +291,13 @@ describe('Filter Utils', () => {
});

test('Return Error if key of a saved object type is not wrapped with attributes', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression(
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'foo.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.description :*)'
),
['foo'],
mockMappings
);
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
Expand Down Expand Up @@ -296,13 +348,13 @@ describe('Filter Utils', () => {
});

test('Return Error if filter is not using an allowed type', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression(
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'bar.updatedAt: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.title: "best" and (foo.attributes.description: t* or foo.attributes.description :*)'
),
['foo'],
mockMappings
);
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
Expand Down Expand Up @@ -351,13 +403,13 @@ describe('Filter Utils', () => {
});

test('Return Error if filter is using an non-existing key in the index patterns of the saved object type', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression(
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression(
'foo.updatedAt33: 5678654567 and foo.attributes.bytes > 1000 and foo.attributes.bytes < 8000 and foo.attributes.header: "best" and (foo.attributes.description: t* or foo.attributes.description :*)'
),
['foo'],
mockMappings
);
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
Expand Down Expand Up @@ -407,11 +459,12 @@ describe('Filter Utils', () => {
});

test('Return Error if filter is using an non-existing key null key', () => {
const validationObject = validateFilterKueryNode(
esKuery.fromKueryExpression('foo.attributes.description: hello AND bye'),
['foo'],
mockMappings
);
const validationObject = validateFilterKueryNode({
astFilter: esKuery.fromKueryExpression('foo.attributes.description: hello AND bye'),
types: ['foo'],
indexMapping: mockMappings,
});

expect(validationObject).toEqual([
{
astPath: 'arguments.0',
Expand Down
77 changes: 54 additions & 23 deletions src/core/server/saved_objects/service/lib/filter_utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { IndexMapping } from '../../mappings';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { esKuery } from '../../../../../plugins/data/server';

const astFunctionType = ['is', 'range', 'nested'];

export const validateConvertFilterToKueryNode = (
allowedTypes: string[],
filter: string,
Expand All @@ -31,12 +33,14 @@ export const validateConvertFilterToKueryNode = (
if (filter && filter.length > 0 && indexMapping) {
const filterKueryNode = esKuery.fromKueryExpression(filter);

const validationFilterKuery = validateFilterKueryNode(
filterKueryNode,
allowedTypes,
const validationFilterKuery = validateFilterKueryNode({
astFilter: filterKueryNode,
types: allowedTypes,
indexMapping,
filterKueryNode.type === 'function' && ['is', 'range'].includes(filterKueryNode.function)
);
storeValue:
filterKueryNode.type === 'function' && astFunctionType.includes(filterKueryNode.function),
hasNestedKey: filterKueryNode.type === 'function' && filterKueryNode.function === 'nested',
});

if (validationFilterKuery.length === 0) {
throw SavedObjectsErrorHelpers.createBadRequestError(
Expand Down Expand Up @@ -90,26 +94,44 @@ interface ValidateFilterKueryNode {
type: string | null;
}

export const validateFilterKueryNode = (
astFilter: esKuery.KueryNode,
types: string[],
indexMapping: IndexMapping,
storeValue: boolean = false,
path: string = 'arguments'
): ValidateFilterKueryNode[] => {
interface ValidateFilterKueryNodeParams {
astFilter: esKuery.KueryNode;
types: string[];
indexMapping: IndexMapping;
hasNestedKey?: boolean;
nestedKeys?: string;
storeValue?: boolean;
path?: string;
}

export const validateFilterKueryNode = ({
astFilter,
types,
indexMapping,
hasNestedKey = false,
nestedKeys,
storeValue = false,
path = 'arguments',
}: ValidateFilterKueryNodeParams): ValidateFilterKueryNode[] => {
let localNestedKeys: string | undefined;
return astFilter.arguments.reduce(
(kueryNode: string[], ast: esKuery.KueryNode, index: number) => {
if (hasNestedKey && ast.type === 'literal' && ast.value != null) {
localNestedKeys = ast.value;
}
if (ast.arguments) {
const myPath = `${path}.${index}`;
return [
...kueryNode,
...validateFilterKueryNode(
ast,
...validateFilterKueryNode({
astFilter: ast,
types,
indexMapping,
ast.type === 'function' && ['is', 'range'].includes(ast.function),
`${myPath}.arguments`
),
storeValue: ast.type === 'function' && astFunctionType.includes(ast.function),
path: `${myPath}.arguments`,
hasNestedKey: ast.type === 'function' && ast.function === 'nested',
nestedKeys: localNestedKeys,
}),
];
}
if (storeValue && index === 0) {
Expand All @@ -118,10 +140,17 @@ export const validateFilterKueryNode = (
...kueryNode,
{
astPath: splitPath.slice(0, splitPath.length - 1).join('.'),
error: hasFilterKeyError(ast.value, types, indexMapping),
isSavedObjectAttr: isSavedObjectAttr(ast.value, indexMapping),
key: ast.value,
type: getType(ast.value),
error: hasFilterKeyError(
nestedKeys != null ? `${nestedKeys}.${ast.value}` : ast.value,
types,
indexMapping
),
isSavedObjectAttr: isSavedObjectAttr(
nestedKeys != null ? `${nestedKeys}.${ast.value}` : ast.value,
indexMapping
),
key: nestedKeys != null ? `${nestedKeys}.${ast.value}` : ast.value,
type: getType(nestedKeys != null ? `${nestedKeys}.${ast.value}` : ast.value),
},
];
}
Expand Down Expand Up @@ -164,7 +193,6 @@ export const hasFilterKeyError = (
return `This key '${key}' need to be wrapped by a saved object type like ${types.join()}`;
} else if (key.includes('.')) {
const keySplit = key.split('.');

if (keySplit.length <= 1 || !types.includes(keySplit[0])) {
return `This type ${keySplit[0]} is not allowed`;
}
Expand All @@ -177,7 +205,10 @@ export const hasFilterKeyError = (
if (
(keySplit.length === 2 && !fieldDefined(indexMapping, keySplit[1])) ||
(keySplit.length > 2 &&
!fieldDefined(indexMapping, keySplit[0] + '.' + keySplit.slice(2, keySplit.length)))
!fieldDefined(
indexMapping,
`${keySplit[0]}.${keySplit.slice(2, keySplit.length).join('.')}`
))
) {
return `This key '${key}' does NOT exist in ${types.join()} saved object index patterns`;
}
Expand Down

0 comments on commit 58aa0dc

Please sign in to comment.