Skip to content

Commit

Permalink
use kuery nodes instead of kuery query
Browse files Browse the repository at this point in the history
  • Loading branch information
gmmorris committed Sep 9, 2020
1 parent 53a2c38 commit eb1ae15
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 135 deletions.
14 changes: 9 additions & 5 deletions x-pack/plugins/alerts/server/alerts_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import uuid from 'uuid';
import { schema } from '@kbn/config-schema';
import { AlertsClient, CreateOptions, ConstructorOptions } from './alerts_client';
import { savedObjectsClientMock, loggingSystemMock } from '../../../../src/core/server/mocks';
import { nodeTypes } from '../../../../src/plugins/data/common';
import { esKuery } from '../../../../src/plugins/data/server';
import { taskManagerMock } from '../../task_manager/server/task_manager.mock';
import { alertTypeRegistryMock } from './alert_type_registry.mock';
import { alertsAuthorizationMock } from './authorization/alerts_authorization.mock';
Expand Down Expand Up @@ -2722,6 +2724,7 @@ describe('find()', () => {
Array [
Object {
"fields": undefined,
"filter": undefined,
"type": "alert",
},
]
Expand All @@ -2730,9 +2733,11 @@ describe('find()', () => {

describe('authorization', () => {
test('ensures user is query filter types down to those the user is authorized to find', async () => {
const filter = esKuery.fromKueryExpression(
'((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))'
);
authorization.getFindAuthorizationFilter.mockResolvedValue({
filter:
'((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))',
filter,
ensureAlertTypeIsAuthorized() {},
logSuccessfulAuthorization() {},
});
Expand All @@ -2741,8 +2746,8 @@ describe('find()', () => {
await alertsClient.find({ options: { filter: 'someTerm' } });

const [options] = unsecuredSavedObjectsClient.find.mock.calls[0];
expect(options.filter).toMatchInlineSnapshot(
`"someTerm and ((alert.attributes.alertTypeId:myType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myApp) or (alert.attributes.alertTypeId:myOtherType and alert.attributes.consumer:myOtherApp))"`
expect(options.filter).toEqual(
nodeTypes.function.buildNode('and', [esKuery.fromKueryExpression('someTerm'), filter])
);
expect(authorization.getFindAuthorizationFilter).toHaveBeenCalledTimes(1);
});
Expand All @@ -2759,7 +2764,6 @@ describe('find()', () => {
const ensureAlertTypeIsAuthorized = jest.fn();
const logSuccessfulAuthorization = jest.fn();
authorization.getFindAuthorizationFilter.mockResolvedValue({
filter: '',
ensureAlertTypeIsAuthorized,
logSuccessfulAuthorization,
});
Expand Down
16 changes: 6 additions & 10 deletions x-pack/plugins/alerts/server/alerts_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
SavedObjectReference,
SavedObject,
} from 'src/core/server';
import { esKuery } from '../../../../src/plugins/data/server';
import { ActionsClient, ActionsAuthorization } from '../../actions/server';
import {
Alert,
Expand All @@ -37,11 +38,7 @@ import { TaskManagerStartContract } from '../../task_manager/server';
import { taskInstanceToAlertTaskInstance } from './task_runner/alert_task_instance';
import { deleteTaskIfItExists } from './lib/delete_task_if_it_exists';
import { RegistryAlertType } from './alert_type_registry';
import {
AlertsAuthorization,
WriteOperations,
ReadOperations,
} from './authorization/alerts_authorization';
import { AlertsAuthorization, WriteOperations, ReadOperations, and } from './authorization';
import { IEventLogClient } from '../../../plugins/event_log/server';
import { parseIsoOrRelativeDate } from './lib/iso_or_relative_date';
import { alertInstanceSummaryFromEventLog } from './lib/alert_instance_summary_from_event_log';
Expand Down Expand Up @@ -339,18 +336,17 @@ export class AlertsClient {
logSuccessfulAuthorization,
} = await this.authorization.getFindAuthorizationFilter();

if (authorizationFilter) {
options.filter = options.filter
? `${options.filter} and ${authorizationFilter}`
: authorizationFilter;
}
const {
page,
per_page: perPage,
total,
saved_objects: data,
} = await this.unsecuredSavedObjectsClient.find<RawAlert>({
...options,
filter:
(authorizationFilter && options.filter
? and([esKuery.fromKueryExpression(options.filter), authorizationFilter])
: authorizationFilter) ?? options.filter,
fields: fields ? this.includeFieldsRequiredForAuthentication(fields) : fields,
type: 'alert',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@ import { securityMock } from '../../../../plugins/security/server/mocks';
import { esKuery } from '../../../../../src/plugins/data/server';
import { PluginStartContract as FeaturesStartContract, Feature } from '../../../features/server';
import { featuresPluginMock } from '../../../features/server/mocks';
import {
AlertsAuthorization,
ensureFieldIsSafeForQuery,
WriteOperations,
ReadOperations,
} from './alerts_authorization';
import { AlertsAuthorization, WriteOperations, ReadOperations } from './alerts_authorization';
import { alertsAuthorizationAuditLoggerMock } from './audit_logger.mock';
import { AlertsAuthorizationAuditLogger, AuthorizationResult } from './audit_logger';
import uuid from 'uuid';
Expand Down Expand Up @@ -607,9 +602,9 @@ describe('AlertsAuthorization', () => {
alertTypeRegistry.list.mockReturnValue(setOfAlertTypes);

expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toEqual(
// esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
// )
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
)
);

expect(auditLogger.alertsAuthorizationSuccess).not.toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import Boom from 'boom';
import { map, mapValues, remove, fromPairs, has } from 'lodash';
import { map, mapValues, fromPairs, has } from 'lodash';
import { KibanaRequest } from 'src/core/server';
import { ALERTS_FEATURE_ID } from '../../common';
import { AlertTypeRegistry } from '../types';
Expand All @@ -15,6 +15,7 @@ import { PluginStartContract as FeaturesPluginStart } from '../../../features/se
import { AlertsAuthorizationAuditLogger, ScopeType } from './audit_logger';
import { Space } from '../../../spaces/server';
import { asFiltersByAlertTypeAndConsumer } from './alerts_authorization_kuery';
import { KueryNode } from '../../../../../src/plugins/data/server';

export enum ReadOperations {
Get = 'get',
Expand Down Expand Up @@ -215,7 +216,7 @@ export class AlertsAuthorization {
}

public async getFindAuthorizationFilter(): Promise<{
filter?: string;
filter?: KueryNode;
ensureAlertTypeIsAuthorized: (alertTypeId: string, consumer: string) => void;
logSuccessfulAuthorization: () => void;
}> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@ import {
} from './alerts_authorization_kuery';

describe('asFiltersByAlertTypeAndConsumer', () => {
// test('1', async () => {
// expect(asFiltersByAlertTypeAndConsumer()).toEqual(
// `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
// );
// });

test('1', async () => {
test('constructs filter for single alert type with single authorized consumer', async () => {
expect(
asFiltersByAlertTypeAndConsumer(
new Set([
Expand All @@ -27,128 +21,71 @@ describe('asFiltersByAlertTypeAndConsumer', () => {
name: 'myAppAlertType',
producer: 'myApp',
authorizedConsumers: {
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myOtherAppAlertType',
name: 'myOtherAppAlertType',
producer: 'alerts',
authorizedConsumers: {
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'mySecondAppAlertType',
name: 'mySecondAppAlertType',
producer: 'myApp',
authorizedConsumers: {
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
])
)
).toEqual(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp)))`
)
);
});

test('2', async () => {
test('constructs filter for single alert type with multiple authorized consumer', async () => {
expect(
asFiltersByAlertTypeAndConsumer(
new Set([
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myOtherAppAlertType',
name: 'myOtherAppAlertType',
producer: 'alerts',
authorizedConsumers: { myApp: { read: true, all: false } },
},
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myAppAlertType',
name: 'myAppAlertType',
producer: 'myApp',
authorizedConsumers: {
myApp: { read: true, all: false },
alerts: { read: true, all: false },
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
},
},
])
)
).toEqual(
`((alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(myApp)) or (alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp or alerts)))`
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))`
)
);
});

test('3', async () => {
test('constructs filter for multiple alert types across authorized consumer', async () => {
expect(
asFiltersByAlertTypeAndConsumer(
new Set([
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myOtherAppAlertType',
name: 'myOtherAppAlertType',
producer: 'alerts',
authorizedConsumers: { myApp: { read: true, all: false } },
},
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myAppAlertType',
name: 'myAppAlertType',
producer: 'myApp',
authorizedConsumers: {
myApp: { read: true, all: false },
alerts: { read: true, all: false },
myOtherApp: { read: true, all: false },
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
])
)
).toEqual(
`((alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(myApp)) or (alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp or alerts or myOtherApp)))`
);
});

test('4', async () => {
expect(
asFiltersByAlertTypeAndConsumer(
new Set([
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myOtherAppAlertType',
name: 'myOtherAppAlertType',
producer: 'alerts',
authorizedConsumers: { myApp: { read: true, all: false } },
},
{
actionGroups: [],
defaultActionGroupId: 'default',
id: 'myAppAlertType',
name: 'myAppAlertType',
producer: 'myApp',
authorizedConsumers: {
myApp: { read: true, all: false },
alerts: { read: true, all: false },
myOtherApp: { read: true, all: false },
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
{
Expand All @@ -158,15 +95,18 @@ describe('asFiltersByAlertTypeAndConsumer', () => {
name: 'mySecondAppAlertType',
producer: 'myApp',
authorizedConsumers: {
myApp: { read: true, all: false },
alerts: { read: true, all: false },
myOtherApp: { read: true, all: false },
alerts: { read: true, all: true },
myApp: { read: true, all: true },
myOtherApp: { read: true, all: true },
myAppWithSubFeature: { read: true, all: true },
},
},
])
)
).toEqual(
`((alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(myApp)) or (alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp or alerts or myOtherApp)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(myApp or alerts or myOtherApp)))`
esKuery.fromKueryExpression(
`((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
)
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,40 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Boom from 'boom';
import { map, mapValues, remove, fromPairs, has } from 'lodash';
import { KibanaRequest } from 'src/core/server';
import { ALERTS_FEATURE_ID } from '../../common';
import { AlertTypeRegistry } from '../types';
import { SecurityPluginSetup } from '../../../security/server';
import { RegistryAlertType } from '../alert_type_registry';
import { PluginStartContract as FeaturesPluginStart } from '../../../features/server';
import { AlertsAuthorizationAuditLogger, ScopeType } from './audit_logger';
import { Space } from '../../../spaces/server';
import { remove } from 'lodash';
import { nodeTypes } from '../../../../../src/plugins/data/common';
import { KueryNode } from '../../../../../src/plugins/data/server';
import { RegistryAlertTypeWithAuth } from './alerts_authorization';

export const is = (fieldName: string, value: string | KueryNode) =>
nodeTypes.function.buildNode('is', fieldName, value);

export const or = ([first, ...args]: KueryNode[]): KueryNode =>
args.length ? nodeTypes.function.buildNode('or', [first, or(args)]) : first;

export const and = ([first, ...args]: KueryNode[]): KueryNode =>
args.length ? nodeTypes.function.buildNode('and', [first, and(args)]) : first;

export function asFiltersByAlertTypeAndConsumer(
alertTypes: Set<RegistryAlertTypeWithAuth>
): string[] {
return `(${Array.from(alertTypes)
.reduce<string[]>((filters, { id, authorizedConsumers }) => {
): KueryNode {
return or(
Array.from(alertTypes).reduce<KueryNode[]>((filters, { id, authorizedConsumers }) => {
ensureFieldIsSafeForQuery('alertTypeId', id);
filters.push(
`(alert.attributes.alertTypeId:${id} and alert.attributes.consumer:(${Object.keys(
authorizedConsumers
)
.map((consumer) => {
ensureFieldIsSafeForQuery('alertTypeId', id);
return consumer;
})
.join(' or ')}))`
and([
is(`alert.attributes.alertTypeId`, id),
or(
Object.keys(authorizedConsumers).map((consumer) => {
ensureFieldIsSafeForQuery('consumer', consumer);
return is(`alert.attributes.consumer`, consumer);
})
),
])
);
return filters;
}, [])
.join(' or ')})`;
);
}

export function ensureFieldIsSafeForQuery(field: string, value: string): boolean {
Expand Down
Loading

0 comments on commit eb1ae15

Please sign in to comment.