diff --git a/x-pack/plugins/alerts/server/alerts_client.test.ts b/x-pack/plugins/alerts/server/alerts_client.test.ts index 801c2c8775361..b20e6c6be2ebf 100644 --- a/x-pack/plugins/alerts/server/alerts_client.test.ts +++ b/x-pack/plugins/alerts/server/alerts_client.test.ts @@ -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'; @@ -2722,6 +2724,7 @@ describe('find()', () => { Array [ Object { "fields": undefined, + "filter": undefined, "type": "alert", }, ] @@ -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() {}, }); @@ -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); }); @@ -2759,7 +2764,6 @@ describe('find()', () => { const ensureAlertTypeIsAuthorized = jest.fn(); const logSuccessfulAuthorization = jest.fn(); authorization.getFindAuthorizationFilter.mockResolvedValue({ - filter: '', ensureAlertTypeIsAuthorized, logSuccessfulAuthorization, }); diff --git a/x-pack/plugins/alerts/server/alerts_client.ts b/x-pack/plugins/alerts/server/alerts_client.ts index 0703a1e13937c..f8da17f4bf089 100644 --- a/x-pack/plugins/alerts/server/alerts_client.ts +++ b/x-pack/plugins/alerts/server/alerts_client.ts @@ -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, @@ -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'; @@ -339,11 +336,6 @@ export class AlertsClient { logSuccessfulAuthorization, } = await this.authorization.getFindAuthorizationFilter(); - if (authorizationFilter) { - options.filter = options.filter - ? `${options.filter} and ${authorizationFilter}` - : authorizationFilter; - } const { page, per_page: perPage, @@ -351,6 +343,10 @@ export class AlertsClient { saved_objects: data, } = await this.unsecuredSavedObjectsClient.find({ ...options, + filter: + (authorizationFilter && options.filter + ? and([esKuery.fromKueryExpression(options.filter), authorizationFilter]) + : authorizationFilter) ?? options.filter, fields: fields ? this.includeFieldsRequiredForAuthentication(fields) : fields, type: 'alert', }); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts index c2506381b9df9..9515987af8dd9 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.test.ts @@ -6,17 +6,13 @@ import { KibanaRequest } from 'kibana/server'; import { alertTypeRegistryMock } from '../alert_type_registry.mock'; import { securityMock } from '../../../../plugins/security/server/mocks'; +import { esKuery } from '../../../../../src/plugins/data/server'; import { PluginStartContract as FeaturesStartContract, KibanaFeature, } 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'; @@ -616,8 +612,10 @@ describe('AlertsAuthorization', () => { }); alertTypeRegistry.list.mockReturnValue(setOfAlertTypes); - expect((await alertAuthorization.getFindAuthorizationFilter()).filter).toMatchInlineSnapshot( - `"((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((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)))` + ) ); expect(auditLogger.alertsAuthorizationSuccess).not.toHaveBeenCalled(); @@ -1246,36 +1244,4 @@ describe('AlertsAuthorization', () => { `); }); }); - - describe('ensureFieldIsSafeForQuery', () => { - test('throws if field contains character that isnt safe in a KQL query', () => { - expect(() => ensureFieldIsSafeForQuery('id', 'alert-*')).toThrowError( - `expected id not to include invalid character: *` - ); - - expect(() => ensureFieldIsSafeForQuery('id', '<=""')).toThrowError( - `expected id not to include invalid character: <=` - ); - - expect(() => ensureFieldIsSafeForQuery('id', '>=""')).toThrowError( - `expected id not to include invalid character: >=` - ); - - expect(() => ensureFieldIsSafeForQuery('id', '1 or alertid:123')).toThrowError( - `expected id not to include whitespace and invalid character: :` - ); - - expect(() => ensureFieldIsSafeForQuery('id', ') or alertid:123')).toThrowError( - `expected id not to include whitespace and invalid characters: ), :` - ); - - expect(() => ensureFieldIsSafeForQuery('id', 'some space')).toThrowError( - `expected id not to include whitespace` - ); - }); - - test('doesnt throws if field is safe as part of a KQL query', () => { - expect(() => ensureFieldIsSafeForQuery('id', '123-0456-678')).not.toThrow(); - }); - }); }); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts index 9dda006c1eb8e..20b9fecd601e6 100644 --- a/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization.ts @@ -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'; @@ -14,6 +14,8 @@ 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 { asFiltersByAlertTypeAndConsumer } from './alerts_authorization_kuery'; +import { KueryNode } from '../../../../../src/plugins/data/server'; export enum ReadOperations { Get = 'get', @@ -215,7 +217,7 @@ export class AlertsAuthorization { } public async getFindAuthorizationFilter(): Promise<{ - filter?: string; + filter?: KueryNode; ensureAlertTypeIsAuthorized: (alertTypeId: string, consumer: string) => void; logSuccessfulAuthorization: () => void; }> { @@ -244,7 +246,7 @@ export class AlertsAuthorization { const authorizedEntries: Map> = new Map(); return { - filter: `(${this.asFiltersByAlertTypeAndConsumer(authorizedAlertTypes).join(' or ')})`, + filter: asFiltersByAlertTypeAndConsumer(authorizedAlertTypes), ensureAlertTypeIsAuthorized: (alertTypeId: string, consumer: string) => { if (!authorizedAlertTypeIdsToConsumers.has(`${alertTypeId}/${consumer}`)) { throw Boom.forbidden( @@ -400,39 +402,6 @@ export class AlertsAuthorization { })) ); } - - private asFiltersByAlertTypeAndConsumer(alertTypes: Set): string[] { - return Array.from(alertTypes).reduce((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 ')}))` - ); - return filters; - }, []); - } -} - -export function ensureFieldIsSafeForQuery(field: string, value: string): boolean { - const invalid = value.match(/([>=<\*:()]+|\s+)/g); - if (invalid) { - const whitespace = remove(invalid, (chars) => chars.trim().length === 0); - const errors = []; - if (whitespace.length) { - errors.push(`whitespace`); - } - if (invalid.length) { - errors.push(`invalid character${invalid.length > 1 ? `s` : ``}: ${invalid?.join(`, `)}`); - } - throw new Error(`expected ${field} not to include ${errors.join(' and ')}`); - } - return true; } function mergeHasPrivileges(left: HasPrivileges, right?: HasPrivileges): HasPrivileges { diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts new file mode 100644 index 0000000000000..e4b9f8c54c38d --- /dev/null +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts @@ -0,0 +1,144 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { esKuery } from '../../../../../src/plugins/data/server'; +import { + asFiltersByAlertTypeAndConsumer, + ensureFieldIsSafeForQuery, +} from './alerts_authorization_kuery'; + +describe('asFiltersByAlertTypeAndConsumer', () => { + test('constructs filter for single alert type with single authorized consumer', async () => { + expect( + asFiltersByAlertTypeAndConsumer( + new Set([ + { + actionGroups: [], + defaultActionGroupId: 'default', + id: 'myAppAlertType', + name: 'myAppAlertType', + producer: 'myApp', + authorizedConsumers: { + myApp: { read: true, all: true }, + }, + }, + ]) + ) + ).toEqual( + esKuery.fromKueryExpression( + `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp)))` + ) + ); + }); + + test('constructs filter for single alert type with multiple authorized consumer', async () => { + expect( + asFiltersByAlertTypeAndConsumer( + new Set([ + { + actionGroups: [], + defaultActionGroupId: 'default', + id: 'myAppAlertType', + name: 'myAppAlertType', + producer: 'myApp', + authorizedConsumers: { + alerts: { read: true, all: true }, + myApp: { read: true, all: true }, + myOtherApp: { read: true, all: true }, + }, + }, + ]) + ) + ).toEqual( + esKuery.fromKueryExpression( + `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))` + ) + ); + }); + + test('constructs filter for multiple alert types across authorized consumer', async () => { + expect( + asFiltersByAlertTypeAndConsumer( + new Set([ + { + actionGroups: [], + defaultActionGroupId: 'default', + id: 'myAppAlertType', + 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( + 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)))` + ) + ); + }); +}); + +describe('ensureFieldIsSafeForQuery', () => { + test('throws if field contains character that isnt safe in a KQL query', () => { + expect(() => ensureFieldIsSafeForQuery('id', 'alert-*')).toThrowError( + `expected id not to include invalid character: *` + ); + + expect(() => ensureFieldIsSafeForQuery('id', '<=""')).toThrowError( + `expected id not to include invalid character: <=` + ); + + expect(() => ensureFieldIsSafeForQuery('id', '>=""')).toThrowError( + `expected id not to include invalid character: >=` + ); + + expect(() => ensureFieldIsSafeForQuery('id', '1 or alertid:123')).toThrowError( + `expected id not to include whitespace and invalid character: :` + ); + + expect(() => ensureFieldIsSafeForQuery('id', ') or alertid:123')).toThrowError( + `expected id not to include whitespace and invalid characters: ), :` + ); + + expect(() => ensureFieldIsSafeForQuery('id', 'some space')).toThrowError( + `expected id not to include whitespace` + ); + }); + + test('doesnt throws if field is safe as part of a KQL query', () => { + expect(() => ensureFieldIsSafeForQuery('id', '123-0456-678')).not.toThrow(); + }); +}); diff --git a/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.ts b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.ts new file mode 100644 index 0000000000000..f236ee7f3c258 --- /dev/null +++ b/x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.ts @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +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.buildNodeWithArgumentNodes('is', [ + nodeTypes.literal.buildNode(fieldName), + typeof value === 'string' ? nodeTypes.literal.buildNode(value) : value, + nodeTypes.literal.buildNode(false), + ]); + +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 +): KueryNode { + return or( + Array.from(alertTypes).reduce((filters, { id, authorizedConsumers }) => { + ensureFieldIsSafeForQuery('alertTypeId', id); + filters.push( + and([ + is(`alert.attributes.alertTypeId`, id), + or( + Object.keys(authorizedConsumers).map((consumer) => { + ensureFieldIsSafeForQuery('consumer', consumer); + return is(`alert.attributes.consumer`, consumer); + }) + ), + ]) + ); + return filters; + }, []) + ); +} + +export function ensureFieldIsSafeForQuery(field: string, value: string): boolean { + const invalid = value.match(/([>=<\*:()]+|\s+)/g); + if (invalid) { + const whitespace = remove(invalid, (chars) => chars.trim().length === 0); + const errors = []; + if (whitespace.length) { + errors.push(`whitespace`); + } + if (invalid.length) { + errors.push(`invalid character${invalid.length > 1 ? `s` : ``}: ${invalid?.join(`, `)}`); + } + throw new Error(`expected ${field} not to include ${errors.join(' and ')}`); + } + return true; +} diff --git a/x-pack/plugins/alerts/server/authorization/index.ts b/x-pack/plugins/alerts/server/authorization/index.ts new file mode 100644 index 0000000000000..66c7f0afd0122 --- /dev/null +++ b/x-pack/plugins/alerts/server/authorization/index.ts @@ -0,0 +1,7 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +export * from './alerts_authorization'; +export * from './alerts_authorization_kuery';