From 649f9eae0c7f5e1e5095f77dc6882befa6503f8b Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Tue, 1 Jun 2021 14:34:48 -0700 Subject: [PATCH 1/2] cleaning up PR and adding basic unit tests --- .../server/alert_data_client/alert_client.ts | 400 ------------------ .../alert_data_client/alerts_client.mock.ts | 27 ++ .../server/alert_data_client/alerts_client.ts | 162 +++++++ .../alerts_client_factory.test.ts | 94 ++++ ...nt_factory.ts => alerts_client_factory.ts} | 25 +- .../alert_data_client/audit_events.test.ts | 87 ++++ .../server/alert_data_client/audit_events.ts | 61 +++ .../server/alert_data_client/audit_logger.ts | 89 ---- .../alert_data_client/tests/get.test.ts | 164 +++++++ .../server/alert_data_client/utils.ts | 72 ---- x-pack/plugins/rule_registry/server/plugin.ts | 13 +- .../server/rule_data_plugin_service/index.ts | 2 +- .../rule_data_plugin_service.mock.ts | 31 ++ 13 files changed, 643 insertions(+), 584 deletions(-) delete mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/alert_client.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts rename x-pack/plugins/rule_registry/server/alert_data_client/{alert_client_factory.ts => alerts_client_factory.ts} (71%) create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/audit_events.test.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/audit_events.ts delete mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/audit_logger.ts create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts delete mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/utils.ts create mode 100644 x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alert_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alert_client.ts deleted file mode 100644 index 30bed3026e58ca..00000000000000 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alert_client.ts +++ /dev/null @@ -1,400 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ -import Boom from '@hapi/boom'; -import { estypes } from '@elastic/elasticsearch'; -import { PublicMethodsOf } from '@kbn/utility-types'; - -import { SanitizedAlert } from '../../../alerting/common'; -import { - AlertTypeParams, - // PartialAlert -} from '../../../alerting/server'; -import { - ReadOperations, - // AlertingAuthorizationFilterType, - AlertingAuthorization, - WriteOperations, - AlertingAuthorizationEntity, -} from '../../../alerting/server'; -import { Logger, ElasticsearchClient, HttpResponsePayload } from '../../../../../src/core/server'; -import { ParsedTechnicalFields } from '../../common/parse_technical_fields'; -import { RacAuthorizationAuditLogger } from './audit_logger'; -import { RuleDataPluginService } from '../rule_data_plugin_service'; - -export interface ConstructorOptions { - logger: Logger; - authorization: PublicMethodsOf; - spaceId?: string; - auditLogger: RacAuthorizationAuditLogger; - esClient: ElasticsearchClient; - index: string; - ruleDataService: RuleDataPluginService; -} - -interface IndexType { - [key: string]: unknown; -} - -export interface FindOptions extends IndexType { - perPage?: number; - page?: number; - search?: string; - defaultSearchOperator?: 'AND' | 'OR'; - searchFields?: string[]; - sortField?: string; - sortOrder?: estypes.SortOrder; - hasReference?: { - type: string; - id: string; - }; - fields?: string[]; - filter?: string; -} - -export interface CreateAlertParams { - esClient: ElasticsearchClient; - owner: 'observability' | 'securitySolution'; -} - -export interface FindResult { - page: number; - perPage: number; - total: number; - data: Array>; -} - -export interface UpdateOptions { - id: string; - owner: string; - data: { - status: string; - }; - assetName: string; // observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 -} - -export interface BulkUpdateOptions { - ids: string[]; - owner: string; - data: { - status: string; - }; - query: unknown; - assetName: string; -} - -interface GetAlertParams { - id: string; - assetName: string; // observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 -} - -export interface GetAlertInstanceSummaryParams { - id: string; - dateStart?: string; -} - -// const alertingAuthorizationFilterOpts: AlertingAuthorizationFilterOpts = { -// type: AlertingAuthorizationFilterType.ESDSL, -// fieldNames: { ruleTypeId: 'alert.alertTypeId', consumer: 'alert.owner' }, -// }; - -export class AlertsClient { - private readonly logger: Logger; - private readonly auditLogger: RacAuthorizationAuditLogger; - // private readonly spaceId?: string; - // private readonly alertsIndex: string; - private readonly authorization: PublicMethodsOf; - private readonly esClient: ElasticsearchClient; - private readonly ruleDataService: RuleDataPluginService; - - constructor({ - auditLogger, - authorization, - logger, - spaceId, - esClient, - index, - ruleDataService, - }: ConstructorOptions) { - this.logger = logger; - // this.spaceId = spaceId; - this.authorization = authorization; - this.esClient = esClient; - this.auditLogger = auditLogger; - // this.alertsIndex = index; - this.ruleDataService = ruleDataService; - } - - /** - * we are "hard coding" this string similar to how rule registry is doing it - * x-pack/plugins/apm/server/plugin.ts:191 - */ - public getAlertsIndex(assetName: string) { - // possibly append spaceId here? - return this.ruleDataService.getFullAssetName(assetName); // await this.authorization.getAuthorizedAlertsIndices(); - } - - // TODO: Type out alerts (rule registry fields + alerting alerts type) - public async get({ - id, - assetName, - }: GetAlertParams): Promise { - // first search for the alert specified, then check if user has access to it - // and return search results - // const query = buildAlertsSearchQuery({ - // index: this.getAlertsIndex(assetName), // '.alerts-observability-apm', - // alertId: id, - // }); - // TODO: Type out alerts (rule registry fields + alerting alerts type) - try { - const { body: result } = await this.esClient.get({ - index: this.getAlertsIndex(assetName), // '.alerts-observability-apm', - id, - }); - if ( - result == null || - result._source == null || - result._source['rule.id'] == null || - result._source['kibana.rac.alert.owner'] == null - ) { - return undefined; - } - - try { - // use security plugin routes to check what URIs user is authorized to - await this.authorization.ensureAuthorized({ - ruleTypeId: result._source['rule.id'], - consumer: result._source['kibana.rac.alert.owner'], - operation: ReadOperations.Get, - entity: AlertingAuthorizationEntity.Alert, - }); - } catch (error) { - throw Boom.forbidden( - this.auditLogger.racAuthorizationFailure({ - owner: result._source['kibana.rac.alert.owner'], - operation: ReadOperations.Get, - type: 'access', - }) - ); - } - - return result; - } catch (exc) { - throw exc; - } - } - - // public async find({ owner }: { owner: string }): Promise { - // let authorizationTuple; - // try { - // authorizationTuple = await this.authorization.getFindAuthorizationFilter( - // AlertingAuthorizationEntity.Alert, - // alertingAuthorizationFilterOpts - // ); - // } catch (error) { - // this.auditLogger.racAuthorizationFailure({ - // owner, - // operation: ReadOperations.Find, - // type: 'access', - // }); - // throw error; - // } - - // const { - // filter: authorizationFilter, - // ensureRuleTypeIsAuthorized, - // logSuccessfulAuthorization, - // } = authorizationTuple; - - // try { - // ensureRuleTypeIsAuthorized('siem.signals', owner, AlertingAuthorizationEntity.Alert); - // } catch (error) { - // this.logger.error(`Unable to bulk find alerts for ${owner}. Error follows: ${error}`); - // throw error; - // } - // } - - public async update({ - id, - owner, - data, - assetName, - }: UpdateOptions): Promise { - // TODO: Type out alerts (rule registry fields + alerting alerts type) - // TODO: use MGET - const { body: result } = await this.esClient.get({ - index: this.getAlertsIndex(assetName), // '.alerts-observability-apm', // '.siem-signals-devin-hurley-default', - id, - }); - const hits = result._source; - if (hits == null || hits['rule.id'] == null || hits['kibana.rac.alert.owner'] == null) { - return undefined; - } - - try { - // ASSUMPTION: user bulk updating alerts from single owner/space - // may need to iterate to support rules shared across spaces - await this.authorization.ensureAuthorized({ - ruleTypeId: hits['rule.id'], - consumer: hits['kibana.rac.alert.owner'], - operation: WriteOperations.Update, - entity: AlertingAuthorizationEntity.Alert, - }); - - try { - const index = this.getAlertsIndex(assetName); // this.authorization.getAuthorizedAlertsIndices(hits['kibana.rac.alert.owner']); - - const updateParameters = { - id, - index, - body: { - doc: { - 'kibana.rac.alert.status': data.status, - }, - }, - }; - - const res = await this.esClient.update( - updateParameters - ); - return res.body.get?._source; - } catch (error) { - // TODO: Update error message - this.logger.error(''); - throw error; - } - } catch (error) { - throw Boom.forbidden( - this.auditLogger.racAuthorizationFailure({ - owner: hits['kibana.rac.alert.owner'], - operation: ReadOperations.Get, - type: 'access', - }) - ); - } - } - - // public async bulkUpdate({ - // ids, - // query, - // assetName, - // data, - // }: BulkUpdateOptions): Promise> { - // const { status } = data; - // let queryObject; - // if (ids) { - // // maybe use an aggs query to make this fast - // queryObject = { - // ids: { values: ids }, - // // USE AGGS and then get returned fields against ensureAuthorizedForAllRuleTypes - // aggs: { - // ...(await this.authorization.getFindAuthorizationFilter( - // AlertingAuthorizationEntity.Alert, - // { - // type: AlertingAuthorizationFilterType.ESDSL, - // fieldNames: { consumer: 'kibana.rac.alert.owner', ruleTypeId: 'rule.id' }, - // }, - // WriteOperations.Update - // )), - // }, - // }; - // } - // console.error('QUERY OBJECT', JSON.stringify(queryObject, null, 2)); - // if (query) { - // queryObject = { - // bool: { - // ...query, - // }, - // }; - // } - // try { - // const result = await this.esClient.updateByQuery({ - // index: this.getAlertsIndex(assetName), - // conflicts: 'abort', // conflicts ?? 'abort', - // // @ts-expect-error refresh should allow for 'wait_for' - // refresh: 'wait_for', - // body: { - // script: { - // source: `ctx._source.signal.status = '${status}'`, - // lang: 'painless', - // }, - // query: queryObject, - // }, - // ignoreUnavailable: true, - // }); - // return result; - // } catch (err) { - // // TODO: Update error message - // this.logger.error(''); - // console.error('UPDATE ERROR', JSON.stringify(err, null, 2)); - // throw err; - // } - // // Looking like we may need to first fetch the alerts to ensure we are - // // pulling the correct ruleTypeId and owner - // // await this.esClient.mget() - - // // try { - // // // ASSUMPTION: user bulk updating alerts from single owner/space - // // // may need to iterate to support rules shared across spaces - - // // const ruleTypes = await this.authorization.ensureAuthorizedForAllRuleTypes({ - // // owner, - // // operation: WriteOperations.Update, - // // entity: AlertingAuthorizationEntity.Alert, - // // }); - - // // const totalRuleTypes = this.authorization.getRuleTypesByProducer(owner); - - // // console.error('RULE TYPES', ruleTypes); - - // // // await this.authorization.ensureAuthorized({ - // // // ruleTypeId: 'siem.signals', // can they update multiple at once or will a single one just be passed in? - // // // consumer: owner, - // // // operation: WriteOperations.Update, - // // // entity: AlertingAuthorizationEntity.Alert, - // // // }); - - // // try { - // // const index = this.authorization.getAuthorizedAlertsIndices(owner); - // // if (index == null) { - // // throw Error(`cannot find authorized index for owner: ${owner}`); - // // } - - // // const body = ids.flatMap((id) => [ - // // { - // // update: { - // // _id: id, - // // _index: this.authorization.getAuthorizedAlertsIndices(ruleTypes[0].producer), - // // }, - // // }, - // // { - // // doc: { 'kibana.rac.alert.status': data.status }, - // // }, - // // ]); - - // // const result = await this.esClient.bulk({ - // // index, - // // body, - // // }); - // // return result; - // // } catch (updateError) { - // // this.logger.error( - // // `Unable to bulk update alerts for ${owner}. Error follows: ${updateError}` - // // ); - // // throw updateError; - // // } - // // } catch (error) { - // // console.error("HERE'S THE ERROR", error); - // // throw Boom.forbidden( - // // this.auditLogger.racAuthorizationFailure({ - // // owner, - // // operation: ReadOperations.Get, - // // type: 'access', - // // }) - // // ); - // // } - // } -} diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts new file mode 100644 index 00000000000000..26da30ce3c194d --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.mock.ts @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { PublicMethodsOf } from '@kbn/utility-types'; +import { AlertsClient } from './alerts_client'; + +type Schema = PublicMethodsOf; +export type AlertsClientMock = jest.Mocked; + +const createAlertsClientMock = () => { + const mocked: AlertsClientMock = { + get: jest.fn(), + getAlertsIndex: jest.fn(), + update: jest.fn(), + }; + return mocked; +}; + +export const alertsClientMock: { + create: () => AlertsClientMock; +} = { + create: createAlertsClientMock, +}; diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts new file mode 100644 index 00000000000000..56227d7664217b --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts @@ -0,0 +1,162 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { PublicMethodsOf } from '@kbn/utility-types'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { RawAlert } from '../../../alerting/server/types'; +import { AlertTypeParams, PartialAlert } from '../../../alerting/server'; +import { + ReadOperations, + AlertingAuthorization, + WriteOperations, + AlertingAuthorizationEntity, + // eslint-disable-next-line @kbn/eslint/no-restricted-paths +} from '../../../alerting/server/authorization'; +import { Logger, ElasticsearchClient, HttpResponsePayload } from '../../../../../src/core/server'; +import { alertAuditEvent, AlertAuditAction } from './audit_events'; +import { RuleDataPluginService } from '../rule_data_plugin_service'; +import { AuditLogger } from '../../../security/server'; + +export interface ConstructorOptions { + logger: Logger; + authorization: PublicMethodsOf; + auditLogger?: AuditLogger; + esClient: ElasticsearchClient; + ruleDataService: RuleDataPluginService; +} + +export interface UpdateOptions { + id: string; + owner: string; + data: { + status: string; + }; + // observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 + assetName: string; +} + +interface GetAlertParams { + id: string; + // observability-apm see here: x-pack/plugins/apm/server/plugin.ts:191 + assetName: string; +} + +export class AlertsClient { + private readonly logger: Logger; + private readonly auditLogger?: AuditLogger; + private readonly authorization: PublicMethodsOf; + private readonly esClient: ElasticsearchClient; + private readonly ruleDataService: RuleDataPluginService; + + constructor({ + auditLogger, + authorization, + logger, + esClient, + ruleDataService, + }: ConstructorOptions) { + this.logger = logger; + this.authorization = authorization; + this.esClient = esClient; + this.auditLogger = auditLogger; + this.ruleDataService = ruleDataService; + } + + /** + * we are "hard coding" this string similar to how rule registry is doing it + * x-pack/plugins/apm/server/plugin.ts:191 + */ + public getAlertsIndex(assetName: string) { + return this.ruleDataService?.getFullAssetName(assetName); + } + + // TODO: Type out alerts (rule registry fields + alerting alerts type) + public async get({ id, assetName }: GetAlertParams): Promise { + try { + // first search for the alert by id, then use the alert info to check if user has access to it + const { body: result } = await this.esClient.get({ + index: this.getAlertsIndex(assetName), + id, + }); + + // this.authorization leverages the alerting plugin's authorization + // client exposed to us for reuse + await this.authorization.ensureAuthorized({ + ruleTypeId: result._source['rule.id'], + consumer: result._source['kibana.rac.alert.owner'], + operation: ReadOperations.Get, + entity: AlertingAuthorizationEntity.Alert, + }); + + this.auditLogger?.log( + alertAuditEvent({ + action: AlertAuditAction.GET, + id, + }) + ); + + return result; + } catch (error) { + this.logger.debug(`[rac] - Error fetching alert with id of "${id}"`); + this.auditLogger?.log( + alertAuditEvent({ + action: AlertAuditAction.GET, + id, + error, + }) + ); + throw error; + } + } + + public async update({ + id, + owner, + data, + assetName, + }: UpdateOptions): Promise> { + try { + // TODO: Type out alerts (rule registry fields + alerting alerts type) + const result = await this.esClient.get({ + index: this.getAlertsIndex(assetName), + id, + }); + const hits = result.body._source; + + // ASSUMPTION: user bulk updating alerts from single owner/space + // may need to iterate to support rules shared across spaces + await this.authorization.ensureAuthorized({ + ruleTypeId: hits['rule.id'], + consumer: hits['kibana.rac.alert.owner'], + operation: WriteOperations.Update, + entity: AlertingAuthorizationEntity.Alert, + }); + + const index = this.getAlertsIndex(assetName); + + const updateParameters = { + id, + index, + body: { + doc: { + 'kibana.rac.alert.status': data.status, + }, + }, + }; + + return this.esClient.update(updateParameters); + } catch (error) { + this.auditLogger?.log( + alertAuditEvent({ + action: AlertAuditAction.GET, + id, + error, + }) + ); + throw error; + } + } +} diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts new file mode 100644 index 00000000000000..8afc33c8362934 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.test.ts @@ -0,0 +1,94 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { Request } from '@hapi/hapi'; + +import { AlertsClientFactory, AlertsClientFactoryProps } from './alerts_client_factory'; +import { ElasticsearchClient, KibanaRequest } from 'src/core/server'; +import { loggingSystemMock } from 'src/core/server/mocks'; +import { securityMock } from '../../../security/server/mocks'; +import { AuditLogger } from '../../../security/server'; +import { ruleDataPluginServiceMock } from '../rule_data_plugin_service/rule_data_plugin_service.mock'; +import { alertingAuthorizationMock } from '../../../alerting/server/authorization/alerting_authorization.mock'; +import { RuleDataPluginServiceConstructorOptions } from '../rule_data_plugin_service'; + +jest.mock('./alerts_client'); + +const securityPluginSetup = securityMock.createSetup(); +const ruleDataServiceMock = ruleDataPluginServiceMock.create( + {} as RuleDataPluginServiceConstructorOptions +); +const alertingAuthMock = alertingAuthorizationMock.create(); + +const alertsClientFactoryParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + getAlertingAuthorization: (_: KibanaRequest) => alertingAuthMock, + securityPluginSetup, + esClient: {} as ElasticsearchClient, + ruleDataService: ruleDataServiceMock, +}; + +const fakeRequest = ({ + app: {}, + headers: {}, + getBasePath: () => '', + path: '/', + route: { settings: {} }, + url: { + href: '/', + }, + raw: { + req: { + url: '/', + }, + }, +} as unknown) as Request; + +const auditLogger = { + log: jest.fn(), +} as jest.Mocked; + +beforeEach(() => { + jest.resetAllMocks(); + + securityPluginSetup.audit.asScoped.mockReturnValue(auditLogger); +}); + +test('creates an alerts client with proper constructor arguments', async () => { + const factory = new AlertsClientFactory(); + factory.initialize({ ...alertsClientFactoryParams }); + const request = KibanaRequest.from(fakeRequest); + await factory.create(request); + + expect(jest.requireMock('./alerts_client').AlertsClient).toHaveBeenCalledWith({ + authorization: alertingAuthMock, + logger: alertsClientFactoryParams.logger, + auditLogger, + esClient: {}, + ruleDataService: ruleDataServiceMock, + }); +}); + +test('throws an error if already initialized', () => { + const factory = new AlertsClientFactory(); + factory.initialize({ ...alertsClientFactoryParams }); + + expect(() => + factory.initialize({ ...alertsClientFactoryParams }) + ).toThrowErrorMatchingInlineSnapshot(`"AlertsClientFactory (RAC) already initialized"`); +}); + +test('throws an error if ruleDataService not available', () => { + const factory = new AlertsClientFactory(); + + expect(() => + factory.initialize({ + ...alertsClientFactoryParams, + ruleDataService: null, + }) + ).toThrowErrorMatchingInlineSnapshot(`"Rule registry data service required for alerts client"`); +}); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alert_client_factory.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.ts similarity index 71% rename from x-pack/plugins/rule_registry/server/alert_data_client/alert_client_factory.ts rename to x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.ts index 5d110222ca7885..f494e2e6e829a3 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alert_client_factory.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client_factory.ts @@ -10,31 +10,28 @@ import { PublicMethodsOf } from '@kbn/utility-types'; import { SecurityPluginSetup } from '../../../security/server'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths import { AlertingAuthorization } from '../../../alerting/server/authorization'; -import { AlertsClient } from './alert_client'; -import { RacAuthorizationAuditLogger } from './audit_logger'; +import { AlertsClient } from './alerts_client'; import { RuleDataPluginService } from '../rule_data_plugin_service'; -export interface RacClientFactoryOpts { +export interface AlertsClientFactoryProps { logger: Logger; - getSpaceId: (request: KibanaRequest) => string | undefined; esClient: ElasticsearchClient; getAlertingAuthorization: (request: KibanaRequest) => PublicMethodsOf; securityPluginSetup: SecurityPluginSetup | undefined; - ruleDataService: RuleDataPluginService | undefined; + ruleDataService: RuleDataPluginService | null; } export class AlertsClientFactory { private isInitialized = false; private logger!: Logger; - private getSpaceId!: (request: KibanaRequest) => string | undefined; private esClient!: ElasticsearchClient; private getAlertingAuthorization!: ( request: KibanaRequest ) => PublicMethodsOf; private securityPluginSetup!: SecurityPluginSetup | undefined; - private ruleDataService!: RuleDataPluginService | undefined; + private ruleDataService!: RuleDataPluginService; - public initialize(options: RacClientFactoryOpts) { + public initialize(options: AlertsClientFactoryProps) { /** * This should be called by the plugin's start() method. */ @@ -42,25 +39,25 @@ export class AlertsClientFactory { throw new Error('AlertsClientFactory (RAC) already initialized'); } + if (options.ruleDataService == null) { + throw new Error('Rule registry data service required for alerts client'); + } + this.getAlertingAuthorization = options.getAlertingAuthorization; this.isInitialized = true; this.logger = options.logger; - this.getSpaceId = options.getSpaceId; this.esClient = options.esClient; this.securityPluginSetup = options.securityPluginSetup; this.ruleDataService = options.ruleDataService; } - public async create(request: KibanaRequest, index: string): Promise { + public async create(request: KibanaRequest): Promise { const { securityPluginSetup, getAlertingAuthorization, logger } = this; - const spaceId = this.getSpaceId(request); return new AlertsClient({ - spaceId, logger, - index, authorization: getAlertingAuthorization(request), - auditLogger: new RacAuthorizationAuditLogger(securityPluginSetup?.audit.asScoped(request)), + auditLogger: securityPluginSetup?.audit.asScoped(request), esClient: this.esClient, ruleDataService: this.ruleDataService!, }); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.test.ts new file mode 100644 index 00000000000000..9536a9a640a00c --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.test.ts @@ -0,0 +1,87 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AlertAuditAction, alertAuditEvent } from './audit_events'; + +describe('#alertAuditEvent', () => { + test('creates event with `unknown` outcome', () => { + expect( + alertAuditEvent({ + action: AlertAuditAction.GET, + outcome: 'unknown', + id: '123', + }) + ).toMatchInlineSnapshot(` + Object { + "error": undefined, + "event": Object { + "action": "alert_get", + "category": Array [ + "database", + ], + "outcome": "unknown", + "type": Array [ + "access", + ], + }, + "message": "User is accessing alert [id=123]", + } + `); + }); + + test('creates event with `success` outcome', () => { + expect( + alertAuditEvent({ + action: AlertAuditAction.GET, + id: '123', + }) + ).toMatchInlineSnapshot(` + Object { + "error": undefined, + "event": Object { + "action": "alert_get", + "category": Array [ + "database", + ], + "outcome": "success", + "type": Array [ + "access", + ], + }, + "message": "User has accessed alert [id=123]", + } + `); + }); + + test('creates event with `failure` outcome', () => { + expect( + alertAuditEvent({ + action: AlertAuditAction.GET, + id: '123', + error: new Error('ERROR_MESSAGE'), + }) + ).toMatchInlineSnapshot(` + Object { + "error": Object { + "code": "Error", + "message": "ERROR_MESSAGE", + }, + "event": Object { + "action": "alert_get", + "category": Array [ + "database", + ], + "outcome": "failure", + "type": Array [ + "access", + ], + }, + "message": "Failed attempt to access alert [id=123]", + } + `); + }); +}); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.ts b/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.ts new file mode 100644 index 00000000000000..d07c23c7fbe9fc --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/audit_events.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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { EcsEventOutcome, EcsEventType } from 'src/core/server'; +import { AuditEvent } from '../../../security/server'; + +export enum AlertAuditAction { + GET = 'alert_get', + UPDATE = 'alert_update', + FIND = 'alert_find', +} + +type VerbsTuple = [string, string, string]; + +const eventVerbs: Record = { + alert_get: ['access', 'accessing', 'accessed'], + alert_update: ['update', 'updating', 'updated'], + alert_find: ['access', 'accessing', 'accessed'], +}; + +const eventTypes: Record = { + alert_get: 'access', + alert_update: 'change', + alert_find: 'access', +}; + +export interface AlertAuditEventParams { + action: AlertAuditAction; + outcome?: EcsEventOutcome; + id?: string; + error?: Error; +} + +export function alertAuditEvent({ action, id, outcome, error }: AlertAuditEventParams): AuditEvent { + const doc = id ? `alert [id=${id}]` : 'an alert'; + const [present, progressive, past] = eventVerbs[action]; + const message = error + ? `Failed attempt to ${present} ${doc}` + : outcome === 'unknown' + ? `User is ${progressive} ${doc}` + : `User has ${past} ${doc}`; + const type = eventTypes[action]; + + return { + message, + event: { + action, + category: ['database'], + type: type ? [type] : undefined, + outcome: outcome ?? (error ? 'failure' : 'success'), + }, + error: error && { + code: error.name, + message: error.message, + }, + }; +} diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/audit_logger.ts b/x-pack/plugins/rule_registry/server/alert_data_client/audit_logger.ts deleted file mode 100644 index cdbf9a3b48d42e..00000000000000 --- a/x-pack/plugins/rule_registry/server/alert_data_client/audit_logger.ts +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -import { EcsEventType } from '@kbn/logging'; -import { AuditLogger } from '../../../security/server'; - -export enum AuthorizationResult { - Unauthorized = 'Unauthorized', - Authorized = 'Authorized', -} - -export enum Result { - Success = 'Success', - Failure = 'Failure', -} - -export class RacAuthorizationAuditLogger { - private readonly auditLogger: AuditLogger; - - constructor(auditLogger: AuditLogger = { log() {} }) { - this.auditLogger = auditLogger; - } - - public getAuthorizationMessage( - authorizationResult: AuthorizationResult, - owner: string, - operation: string - ): string { - return `${authorizationResult} to ${operation} "${owner}" alert"`; - } - - public racAuthorizationFailure({ - owner, - operation, - type, - error, - }: { - owner: string; - operation: string; - type: EcsEventType; - error?: Error; - }): string { - const message = this.getAuthorizationMessage( - AuthorizationResult.Unauthorized, - owner, - operation - ); - this.auditLogger.log({ - message, - event: { - action: 'rac_authorization_failure', - category: ['authentication'], - type: [type], - outcome: 'failure', - }, - error: error && { - code: error.name, - message: error.message, - }, - }); - return message; - } - - public racAuthorizationSuccess({ - owner, - operation, - type, - }: { - owner: string; - operation: string; - type: EcsEventType; - }): string { - const message = this.getAuthorizationMessage(AuthorizationResult.Authorized, owner, operation); - this.auditLogger.log({ - message, - event: { - action: 'rac_authorization_success', - category: ['authentication'], - type: [type], - outcome: 'success', - }, - }); - return message; - } -} diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts new file mode 100644 index 00000000000000..ced46551eea391 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts @@ -0,0 +1,164 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AlertsClient, ConstructorOptions } from '../alerts_client'; +import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; +import { ruleDataPluginServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock'; +import { RuleDataPluginServiceConstructorOptions } from '../../rule_data_plugin_service'; +import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; +import { AuditLogger } from '../../../../security/server'; + +const ruleDataServiceMock = ruleDataPluginServiceMock.create( + {} as RuleDataPluginServiceConstructorOptions +); +const alertingAuthMock = alertingAuthorizationMock.create(); +const esClientMock = elasticsearchClientMock.createElasticsearchClient(); +const auditLogger = { + log: jest.fn(), +} as jest.Mocked; + +const alertsClientParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + ruleDataService: { + ...ruleDataServiceMock, + getFullAssetName: (_?: string | undefined) => '.alerts-observability-apm', + }, + authorization: alertingAuthMock, + esClient: esClientMock, + auditLogger, +}; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe('get()', () => { + test('calls ES client with given params', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.get.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'open', + }, + }, + }) + ); + const result = await alertsClient.get({ id: '1', assetName: 'observability-apm' }); + expect(result).toMatchInlineSnapshot(` + Object { + "_id": "NoxgpHkBqbdrfX07MqXV", + "_index": ".alerts-observability-apm", + "_source": Object { + "kibana.rac.alert.owner": "apm", + "kibana.rac.alert.status": "open", + "message": "hello world 1", + "rule.id": "apm.error_rate", + }, + } + `); + expect(esClientMock.get).toHaveBeenCalledTimes(1); + expect(esClientMock.get.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "id": "1", + "index": ".alerts-observability-apm", + }, + ] + `); + expect(auditLogger.log).toHaveBeenCalledWith({ + error: undefined, + event: { action: 'alert_get', category: ['database'], outcome: 'success', type: ['access'] }, + message: 'User has accessed alert [id=1]', + }); + }); + + test(`throws an error if ES client get fails`, async () => { + const error = new Error('something when wrong'); + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.get.mockRejectedValue(error); + + await expect( + alertsClient.get({ id: '1', assetName: 'observability-apm' }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"something when wrong"`); + expect(auditLogger.log).toHaveBeenCalledWith({ + error: { code: 'Error', message: 'something when wrong' }, + event: { action: 'alert_get', category: ['database'], outcome: 'failure', type: ['access'] }, + message: 'Failed attempt to access alert [id=1]', + }); + }); + + describe('authorization', () => { + beforeEach(() => { + esClientMock.get.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'open', + }, + }, + }) + ); + }); + + test('returns alert if user is authorized to read alert under the consumer', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + const result = await alertsClient.get({ id: '1', assetName: 'observability-apm' }); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + entity: 'alert', + consumer: 'apm', + operation: 'get', + ruleTypeId: 'apm.error_rate', + }); + expect(result).toMatchInlineSnapshot(` + Object { + "_id": "NoxgpHkBqbdrfX07MqXV", + "_index": ".alerts-observability-apm", + "_source": Object { + "kibana.rac.alert.owner": "apm", + "kibana.rac.alert.status": "open", + "message": "hello world 1", + "rule.id": "apm.error_rate", + }, + } + `); + }); + + test('throws when user is not authorized to get this type of alert', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + alertingAuthMock.ensureAuthorized.mockRejectedValue( + new Error(`Unauthorized to get a "apm.error_rate" alert for "apm"`) + ); + + await expect( + alertsClient.get({ id: '1', assetName: 'observability-apm' }) + ).rejects.toMatchInlineSnapshot( + `[Error: Unauthorized to get a "apm.error_rate" alert for "apm"]` + ); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + entity: 'alert', + consumer: 'apm', + operation: 'get', + ruleTypeId: 'apm.error_rate', + }); + }); + }); +}); diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/utils.ts b/x-pack/plugins/rule_registry/server/alert_data_client/utils.ts deleted file mode 100644 index 0cadbafd63916b..00000000000000 --- a/x-pack/plugins/rule_registry/server/alert_data_client/utils.ts +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -interface BuildAlertsSearchQuery { - alertId: string; - index: string; - from?: string; - to?: string; - size?: number; -} - -export const buildAlertsSearchQuery = ({ - alertId, - index, - from, - to, - size, -}: BuildAlertsSearchQuery) => ({ - index, - body: { - size, - query: { - bool: { - filter: [ - { - bool: { - should: { - match: { - _id: alertId, - }, - }, - minimum_should_match: 1, - }, - }, - // { - // range: { - // '@timestamp': { - // gt: from, - // lte: to, - // format: 'epoch_millis', - // }, - // }, - // }, - ], - }, - }, - }, -}); - -interface BuildAlertsUpdateParams { - ids: string[]; - index: string; - status: string; -} - -export const buildAlertsUpdateParameters = ({ ids, index, status }: BuildAlertsUpdateParams) => ({ - index, - body: ids.flatMap((id) => [ - { - update: { - _id: id, - }, - }, - { - doc: { 'kibana.rac.alert.status': status }, - }, - ]), -}); diff --git a/x-pack/plugins/rule_registry/server/plugin.ts b/x-pack/plugins/rule_registry/server/plugin.ts index 4a213ecfbc8c68..9e63b0a39230bb 100644 --- a/x-pack/plugins/rule_registry/server/plugin.ts +++ b/x-pack/plugins/rule_registry/server/plugin.ts @@ -14,7 +14,7 @@ import { IContextProvider, } from 'src/core/server'; import { SecurityPluginSetup } from '../../security/server'; -import { AlertsClientFactory } from './alert_data_client/alert_client_factory'; +import { AlertsClientFactory } from './alert_data_client/alerts_client_factory'; import { PluginStartContract as AlertingStart } from '../../alerting/server'; import { RacApiRequestHandlerContext, RacRequestHandlerContext } from './types'; import { defineRoutes } from './routes'; @@ -61,7 +61,7 @@ export class RuleRegistryPlugin private readonly logger: Logger; private eventLogService: EventLogService | null; private readonly alertsClientFactory: AlertsClientFactory; - private ruleDataService: RuleDataPluginService | null | undefined; + private ruleDataService: RuleDataPluginService | null; private security: SecurityPluginSetup | undefined; constructor(initContext: PluginInitializerContext) { @@ -143,9 +143,6 @@ export class RuleRegistryPlugin alertsClientFactory.initialize({ logger, - getSpaceId(request: KibanaRequest) { - return plugins.spaces?.spacesService.getSpaceId(request); - }, esClient: core.elasticsearch.client.asInternalUser, // NOTE: Alerts share the authorization client with the alerting plugin getAlertingAuthorization(request: KibanaRequest) { @@ -156,7 +153,7 @@ export class RuleRegistryPlugin }); const getRacClientWithRequest = (request: KibanaRequest) => { - return alertsClientFactory.create(request, this.config.index); + return alertsClientFactory.create(request); }; return { @@ -166,14 +163,14 @@ export class RuleRegistryPlugin } private createRouteHandlerContext = (): IContextProvider => { - const { alertsClientFactory, config } = this; + const { alertsClientFactory } = this; return async function alertsRouteHandlerContext( context, request ): Promise { return { getAlertsClient: async () => { - const createdClient = alertsClientFactory.create(request, config.index); + const createdClient = alertsClientFactory.create(request); return createdClient; }, }; diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts index 159e9b81525974..91b4294861982c 100644 --- a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts +++ b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts @@ -19,7 +19,7 @@ import { ClusterPutComponentTemplateBody, PutIndexTemplateRequest } from '../../ const BOOTSTRAP_TIMEOUT = 60000; -interface RuleDataPluginServiceConstructorOptions { +export interface RuleDataPluginServiceConstructorOptions { getClusterClient: () => Promise; logger: Logger; isWriteEnabled: boolean; diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts new file mode 100644 index 00000000000000..949a434ecdf6f4 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import type { PublicMethodsOf } from '@kbn/utility-types'; +import { RuleDataPluginService, RuleDataPluginServiceConstructorOptions } from './'; + +type Schema = PublicMethodsOf; + +const createRuleDataPluginServiceMock = (_: RuleDataPluginServiceConstructorOptions) => { + const mocked: jest.Mocked = { + init: jest.fn(), + isReady: jest.fn(), + wait: jest.fn(), + isWriteEnabled: jest.fn(), + getFullAssetName: jest.fn(), + createOrUpdateComponentTemplate: jest.fn(), + createOrUpdateIndexTemplate: jest.fn(), + createOrUpdateLifecyclePolicy: jest.fn(), + }; + return (mocked as unknown) as jest.Mocked; +}; + +export const ruleDataPluginServiceMock: { + create: (_: RuleDataPluginServiceConstructorOptions) => jest.Mocked; +} = { + create: createRuleDataPluginServiceMock, +}; From 7436c26e6ca39c7e91d7c8123711032c5e804408 Mon Sep 17 00:00:00 2001 From: Yara Tercero Date: Wed, 2 Jun 2021 00:54:29 -0700 Subject: [PATCH 2/2] still need to clean up types in tests and update one test file --- .../server/alert_data_client/alerts_client.ts | 79 ++++-- .../alert_data_client/tests/get.test.ts | 22 +- .../alert_data_client/tests/update.test.ts | 257 ++++++++++++++++++ .../server/routes/get_alert_by_id.ts | 3 - 4 files changed, 319 insertions(+), 42 deletions(-) create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts index 56227d7664217b..21ecd997b114c5 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts @@ -5,9 +5,7 @@ * 2.0. */ import { PublicMethodsOf } from '@kbn/utility-types'; -// eslint-disable-next-line @kbn/eslint/no-restricted-paths -import { RawAlert } from '../../../alerting/server/types'; -import { AlertTypeParams, PartialAlert } from '../../../alerting/server'; +import { AlertTypeParams } from '../../../alerting/server'; import { ReadOperations, AlertingAuthorization, @@ -15,10 +13,11 @@ import { AlertingAuthorizationEntity, // eslint-disable-next-line @kbn/eslint/no-restricted-paths } from '../../../alerting/server/authorization'; -import { Logger, ElasticsearchClient, HttpResponsePayload } from '../../../../../src/core/server'; +import { Logger, ElasticsearchClient } from '../../../../../src/core/server'; import { alertAuditEvent, AlertAuditAction } from './audit_events'; import { RuleDataPluginService } from '../rule_data_plugin_service'; import { AuditLogger } from '../../../security/server'; +import { ParsedTechnicalFields } from '../../common/parse_technical_fields'; export interface ConstructorOptions { logger: Logger; @@ -30,7 +29,6 @@ export interface ConstructorOptions { export interface UpdateOptions { id: string; - owner: string; data: { status: string; }; @@ -73,20 +71,46 @@ export class AlertsClient { return this.ruleDataService?.getFullAssetName(assetName); } - // TODO: Type out alerts (rule registry fields + alerting alerts type) - public async get({ id, assetName }: GetAlertParams): Promise { + private async fetchAlert({ id, assetName }: GetAlertParams): Promise { try { - // first search for the alert by id, then use the alert info to check if user has access to it - const { body: result } = await this.esClient.get({ + const result = await this.esClient.get({ index: this.getAlertsIndex(assetName), id, }); + if ( + result == null || + result.body == null || + result.body._source == null || + result.body._source['rule.id'] == null || + result.body._source['kibana.rac.alert.owner'] == null + ) { + const errorMessage = `[rac] - Unable to retrieve alert details for alert with id of "${id}".`; + this.logger.debug(errorMessage); + throw new Error(errorMessage); + } + + return result.body._source; + } catch (error) { + const errorMessage = `[rac] - Unable to retrieve alert with id of "${id}".`; + this.logger.debug(errorMessage); + throw error; + } + } + + public async get({ id, assetName }: GetAlertParams): Promise { + try { + // first search for the alert by id, then use the alert info to check if user has access to it + const alert = await this.fetchAlert({ + id, + assetName, + }); + // this.authorization leverages the alerting plugin's authorization // client exposed to us for reuse await this.authorization.ensureAuthorized({ - ruleTypeId: result._source['rule.id'], - consumer: result._source['kibana.rac.alert.owner'], + ruleTypeId: alert['rule.id'], + consumer: alert['kibana.rac.alert.owner'], operation: ReadOperations.Get, entity: AlertingAuthorizationEntity.Alert, }); @@ -98,7 +122,7 @@ export class AlertsClient { }) ); - return result; + return alert; } catch (error) { this.logger.debug(`[rac] - Error fetching alert with id of "${id}"`); this.auditLogger?.log( @@ -114,23 +138,19 @@ export class AlertsClient { public async update({ id, - owner, data, assetName, - }: UpdateOptions): Promise> { + }: UpdateOptions): Promise { try { - // TODO: Type out alerts (rule registry fields + alerting alerts type) - const result = await this.esClient.get({ - index: this.getAlertsIndex(assetName), + // TODO: use MGET + const alert = await this.fetchAlert({ id, + assetName, }); - const hits = result.body._source; - // ASSUMPTION: user bulk updating alerts from single owner/space - // may need to iterate to support rules shared across spaces await this.authorization.ensureAuthorized({ - ruleTypeId: hits['rule.id'], - consumer: hits['kibana.rac.alert.owner'], + ruleTypeId: alert['rule.id'], + consumer: alert['kibana.rac.alert.owner'], operation: WriteOperations.Update, entity: AlertingAuthorizationEntity.Alert, }); @@ -147,11 +167,22 @@ export class AlertsClient { }, }; - return this.esClient.update(updateParameters); + const res = await this.esClient.update( + updateParameters + ); + + this.auditLogger?.log( + alertAuditEvent({ + action: AlertAuditAction.UPDATE, + id, + }) + ); + + return res.body.get?._source; } catch (error) { this.auditLogger?.log( alertAuditEvent({ - action: AlertAuditAction.GET, + action: AlertAuditAction.UPDATE, id, error, }) diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts index ced46551eea391..f57631ecb1d7af 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/get.test.ts @@ -58,14 +58,10 @@ describe('get()', () => { const result = await alertsClient.get({ id: '1', assetName: 'observability-apm' }); expect(result).toMatchInlineSnapshot(` Object { - "_id": "NoxgpHkBqbdrfX07MqXV", - "_index": ".alerts-observability-apm", - "_source": Object { - "kibana.rac.alert.owner": "apm", - "kibana.rac.alert.status": "open", - "message": "hello world 1", - "rule.id": "apm.error_rate", - }, + "kibana.rac.alert.owner": "apm", + "kibana.rac.alert.status": "open", + "message": "hello world 1", + "rule.id": "apm.error_rate", } `); expect(esClientMock.get).toHaveBeenCalledTimes(1); @@ -128,17 +124,13 @@ describe('get()', () => { ruleTypeId: 'apm.error_rate', }); expect(result).toMatchInlineSnapshot(` - Object { - "_id": "NoxgpHkBqbdrfX07MqXV", - "_index": ".alerts-observability-apm", - "_source": Object { + Object { "kibana.rac.alert.owner": "apm", "kibana.rac.alert.status": "open", "message": "hello world 1", "rule.id": "apm.error_rate", - }, - } - `); + } + `); }); test('throws when user is not authorized to get this type of alert', async () => { diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts new file mode 100644 index 00000000000000..439c6b68a6bf4c --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts @@ -0,0 +1,257 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { AlertsClient, ConstructorOptions } from '../alerts_client'; +import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; +// eslint-disable-next-line @kbn/eslint/no-restricted-paths +import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; +import { ruleDataPluginServiceMock } from '../../rule_data_plugin_service/rule_data_plugin_service.mock'; +import { RuleDataPluginServiceConstructorOptions } from '../../rule_data_plugin_service'; +import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; +import { AuditLogger } from '../../../../security/server'; + +const ruleDataServiceMock = ruleDataPluginServiceMock.create( + {} as RuleDataPluginServiceConstructorOptions +); +const alertingAuthMock = alertingAuthorizationMock.create(); +const esClientMock = elasticsearchClientMock.createElasticsearchClient(); +const auditLogger = { + log: jest.fn(), +} as jest.Mocked; + +const alertsClientParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + ruleDataService: { + ...ruleDataServiceMock, + getFullAssetName: (_?: string | undefined) => '.alerts-observability-apm', + }, + authorization: alertingAuthMock, + esClient: esClientMock, + auditLogger, +}; + +beforeEach(() => { + jest.resetAllMocks(); +}); + +describe('update()', () => { + test('calls ES client with given params', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.get.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'open', + }, + }, + }) + ); + esClientMock.update.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + get: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'closed', + }, + }, + }, + }) + ); + const result = await alertsClient.update({ + id: '1', + data: { status: 'closed' }, + assetName: 'observability-apm', + }); + expect(result).toMatchInlineSnapshot(` + Object { + "kibana.rac.alert.owner": "apm", + "kibana.rac.alert.status": "closed", + "message": "hello world 1", + "rule.id": "apm.error_rate", + } + `); + expect(esClientMock.update).toHaveBeenCalledTimes(1); + expect(esClientMock.update.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "body": Object { + "doc": Object { + "kibana.rac.alert.status": "closed", + }, + }, + "id": "1", + "index": ".alerts-observability-apm", + }, + ] + `); + expect(auditLogger.log).toHaveBeenCalledWith({ + error: undefined, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'success', + type: ['change'], + }, + message: 'User has updated alert [id=1]', + }); + }); + + test(`throws an error if ES client get fails`, async () => { + const error = new Error('something when wrong on get'); + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.get.mockRejectedValue(error); + + await expect( + alertsClient.update({ + id: '1', + data: { status: 'closed' }, + assetName: 'observability-apm', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"something when wrong on get"`); + expect(auditLogger.log).toHaveBeenCalledWith({ + error: { code: 'Error', message: 'something when wrong on get' }, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + message: 'Failed attempt to update alert [id=1]', + }); + }); + + test(`throws an error if ES client update fails`, async () => { + const error = new Error('something when wrong on update'); + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.get.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'open', + }, + }, + }) + ); + esClientMock.update.mockRejectedValue(error); + + await expect( + alertsClient.update({ + id: '1', + data: { status: 'closed' }, + assetName: 'observability-apm', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"something when wrong on update"`); + expect(auditLogger.log).toHaveBeenCalledWith({ + error: { code: 'Error', message: 'something when wrong on update' }, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + message: 'Failed attempt to update alert [id=1]', + }); + }); + + describe('authorization', () => { + beforeEach(() => { + esClientMock.get.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'open', + }, + }, + }) + ); + esClientMock.update.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + get: { + _index: '.alerts-observability-apm', + _id: 'NoxgpHkBqbdrfX07MqXV', + _source: { + 'rule.id': 'apm.error_rate', + message: 'hello world 1', + 'kibana.rac.alert.owner': 'apm', + 'kibana.rac.alert.status': 'closed', + }, + }, + }, + }) + ); + }); + + test('returns alert if user is authorized to update alert under the consumer', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + const result = await alertsClient.update({ + id: '1', + data: { status: 'closed' }, + assetName: 'observability-apm', + }); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + entity: 'alert', + consumer: 'apm', + operation: 'update', + ruleTypeId: 'apm.error_rate', + }); + expect(result).toMatchInlineSnapshot(` + Object { + "kibana.rac.alert.owner": "apm", + "kibana.rac.alert.status": "closed", + "message": "hello world 1", + "rule.id": "apm.error_rate", + } + `); + }); + + test('throws when user is not authorized to update this type of alert', async () => { + const alertsClient = new AlertsClient(alertsClientParams); + alertingAuthMock.ensureAuthorized.mockRejectedValue( + new Error(`Unauthorized to get a "apm.error_rate" alert for "apm"`) + ); + + await expect( + alertsClient.update({ + id: '1', + data: { status: 'closed' }, + assetName: 'observability-apm', + }) + ).rejects.toMatchInlineSnapshot( + `[Error: Unauthorized to get a "apm.error_rate" alert for "apm"]` + ); + + expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ + entity: 'alert', + consumer: 'apm', + operation: 'update', + ruleTypeId: 'apm.error_rate', + }); + }); + }); +}); diff --git a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.ts b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.ts index ec8d83b9337052..039c10d4c37a18 100644 --- a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.ts +++ b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.ts @@ -37,9 +37,6 @@ export const getAlertByIdRoute = (router: IRouter) => const alertsClient = await context.rac.getAlertsClient(); const { id, assetName } = request.query; const alert = await alertsClient.get({ id, assetName }); - if (alert == null) { - throw new Error('could not get alert'); - } return response.ok({ body: alert, });