From 6706d8a2771866bea5b5defe68112ad1ab876d4b Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 09:38:06 -0400 Subject: [PATCH 1/9] wip undo me more WIP I think this is working now getting there.. working integration tests fixes type check errors fix jest somehow this was deleted after rebasing... fix early rejection error ids array XOR query string in bulk update route if ids, use elasticsearch bulk update method otherwise use update by query fix tests refresh true fix tests general cleanup fixes apm jest test fixes jest tests code cleanup and fixed jest tests WIP - trying to fix integration tests after refactor fix tests, throw appropriate HTTP errors in alerts client fix issues after rebasing with master updates alerts client docs general cleanup fix trial integration tests possibly fixes loader issue in package updates getSafeSortIds param type to include 'null' in param type, renames getFindAuthorizationFilter -> getAuthorizationFilter and renames usage elsewhere, utilizes const for test in apm make 'operation' parameter non-optional in getAuthorizationFilter and retain backwards compatibility by pointing the getFindAuthorizationFilter to getAuthorizationFilter and operation = ReadOperations.Find adds an exported type to be used when setting a status on an alert moves es query config out of client definition and into a function to allow for customized configurations update by query should check for siem signals status field as well return 200 when no alerts are found, not a 401 error, updates integration tests with stricter expects updates alerts clients docs fixing merge with master conflicts adds missing required space ids field to mocked alerts to fix failing apm jest tests implements suggested changes with auditing failures in bulk update by ids function, removes audit logging in catch blocks, cleans up other jest tests execute ensure authorized in the search after loop, adds jest tests to make sure we log an error to the audit log for the case when ensureAuthorized throws an authorization error --- packages/kbn-rule-data-utils/BUILD.bazel | 2 + .../src/alerts_as_data_rbac.ts | 58 ++ .../alerting_authorization.mock.ts | 1 + .../authorization/alerting_authorization.ts | 14 +- .../helper/get_alert_annotations.test.tsx | 1 + .../docs/alerts_client/alerts_client_api.md | 1 + .../alerts_client/classes/alertsclient.md | 188 +++++-- .../interfaces/bulkupdateoptions.md | 58 ++ .../interfaces/constructoroptions.md | 8 +- .../alerts_client/interfaces/updateoptions.md | 8 +- .../alert_data_client/alerts_client.mock.ts | 2 +- .../server/alert_data_client/alerts_client.ts | 517 ++++++++++++++---- .../server/alert_data_client/audit_events.ts | 7 + .../tests/bulk_update.test.ts | 282 ++++++++++ .../alert_data_client/tests/get.test.ts | 90 ++- .../alert_data_client/tests/update.test.ts | 84 +-- .../server/routes/bulk_update_alerts.ts | 93 ++++ .../server/routes/get_alert_by_id.test.ts | 1 + .../rule_registry/server/routes/index.ts | 2 + .../bulk_update_observability_alert_by_ids.sh | 30 + ...ulk_update_observability_alert_by_query.sh | 28 + ...te_old_security_solution_alert_by_query.sh | 28 + .../server/scripts/get_observability_alert.sh | 2 +- .../scripts/hunter/detections_role.json | 7 +- .../scripts/observer/detections_role.json | 2 +- .../rule_execution_field_map.ts | 3 + .../rule_registry_log_client.ts | 8 + .../rule_registry/alerts/data.json | 32 ++ .../tests/basic/bulk_update_alerts.ts | 225 ++++++++ .../tests/basic/get_alert_by_id.ts | 6 +- .../security_and_spaces/tests/basic/index.ts | 1 + .../tests/basic/update_alert.ts | 20 +- .../tests/trial/get_alerts.ts | 4 +- .../tests/trial/update_alert.ts | 4 +- 34 files changed, 1505 insertions(+), 312 deletions(-) create mode 100644 x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md create mode 100644 x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts create mode 100644 x-pack/plugins/rule_registry/server/routes/bulk_update_alerts.ts create mode 100755 x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_ids.sh create mode 100755 x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_query.sh create mode 100755 x-pack/plugins/rule_registry/server/scripts/bulk_update_old_security_solution_alert_by_query.sh create mode 100644 x-pack/test/rule_registry/security_and_spaces/tests/basic/bulk_update_alerts.ts diff --git a/packages/kbn-rule-data-utils/BUILD.bazel b/packages/kbn-rule-data-utils/BUILD.bazel index ccd1793feb161..62a48b66b4bce 100644 --- a/packages/kbn-rule-data-utils/BUILD.bazel +++ b/packages/kbn-rule-data-utils/BUILD.bazel @@ -22,8 +22,10 @@ NPM_MODULE_EXTRA_FILES = [ ] SRC_DEPS = [ + "//packages/kbn-es-query", "@npm//tslib", "@npm//utility-types", + "@npm//@elastic/elasticsearch", ] TYPES_DEPS = [ diff --git a/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts b/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts index d3d20edffa286..795d194ee8a92 100644 --- a/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts +++ b/packages/kbn-rule-data-utils/src/alerts_as_data_rbac.ts @@ -6,6 +6,9 @@ * Side Public License, v 1. */ +import type { estypes } from '@elastic/elasticsearch'; +import type { EsQueryConfig } from '@kbn/es-query'; + /** * registering a new instance of the rule data client * in a new plugin will require updating the below data structure @@ -24,6 +27,7 @@ export const AlertConsumers = { SYNTHETICS: 'synthetics', } as const; export type AlertConsumers = typeof AlertConsumers[keyof typeof AlertConsumers]; +export type STATUS_VALUES = 'open' | 'acknowledged' | 'closed'; export const mapConsumerToIndexName: Record = { apm: '.alerts-observability-apm', @@ -38,3 +42,57 @@ export type ValidFeatureId = keyof typeof mapConsumerToIndexName; export const validFeatureIds = Object.keys(mapConsumerToIndexName); export const isValidFeatureId = (a: unknown): a is ValidFeatureId => typeof a === 'string' && validFeatureIds.includes(a); + +/** + * Prevent javascript from returning Number.MAX_SAFE_INTEGER when Elasticsearch expects + * Java's Long.MAX_VALUE. This happens when sorting fields by date which are + * unmapped in the provided index + * + * Ref: https://github.com/elastic/elasticsearch/issues/28806#issuecomment-369303620 + * + * return stringified Long.MAX_VALUE if we receive Number.MAX_SAFE_INTEGER + * @param sortIds estypes.SearchSortResults | undefined + * @returns SortResults + */ +export const getSafeSortIds = (sortIds: estypes.SearchSortResults | null | undefined) => { + if (sortIds == null) { + return sortIds; + } + return sortIds.map((sortId) => { + // haven't determined when we would receive a null value for a sort id + // but in case we do, default to sending the stringified Java max_int + if (sortId == null || sortId === '' || sortId >= Number.MAX_SAFE_INTEGER) { + return '9223372036854775807'; + } + return sortId; + }); +}; + +interface GetEsQueryConfigParamType { + allowLeadingWildcards?: EsQueryConfig['allowLeadingWildcards']; + queryStringOptions?: EsQueryConfig['queryStringOptions']; + ignoreFilterIfFieldNotInIndex?: EsQueryConfig['ignoreFilterIfFieldNotInIndex']; + dateFormatTZ?: EsQueryConfig['dateFormatTZ']; +} + +type ConfigKeys = keyof GetEsQueryConfigParamType; + +export const getEsQueryConfig = (params?: GetEsQueryConfigParamType): EsQueryConfig => { + const defaultConfigValues = { + allowLeadingWildcards: true, + queryStringOptions: { analyze_wildcard: true }, + ignoreFilterIfFieldNotInIndex: false, + dateFormatTZ: 'Zulu', + }; + if (params == null) { + return defaultConfigValues; + } + const paramKeysWithValues = Object.keys(params).reduce((acc: EsQueryConfig, key) => { + const configKey = key as ConfigKeys; + if (params[configKey] != null) { + return { [key]: params[configKey], ...acc }; + } + return { [key]: defaultConfigValues[configKey], ...acc }; + }, {} as EsQueryConfig); + return paramKeysWithValues; +}; diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.mock.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.mock.ts index 8cd98b17f5d37..8560e9a2d9335 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.mock.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.mock.ts @@ -15,6 +15,7 @@ const createAlertingAuthorizationMock = () => { const mocked: AlertingAuthorizationMock = { ensureAuthorized: jest.fn(), filterByRuleTypeAuthorization: jest.fn(), + getAuthorizationFilter: jest.fn(), getFindAuthorizationFilter: jest.fn(), getAugmentedRuleTypesWithAuthorization: jest.fn(), getSpaceId: jest.fn(), diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts index fd57298f7d93b..dcde577bb5fd8 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts @@ -281,11 +281,23 @@ export class AlertingAuthorization { filter?: KueryNode | JsonObject; ensureRuleTypeIsAuthorized: (ruleTypeId: string, consumer: string, auth: string) => void; logSuccessfulAuthorization: () => void; + }> { + return this.getAuthorizationFilter(authorizationEntity, filterOpts, ReadOperations.Find); + } + + public async getAuthorizationFilter( + authorizationEntity: AlertingAuthorizationEntity, + filterOpts: AlertingAuthorizationFilterOpts, + operation: WriteOperations | ReadOperations + ): Promise<{ + filter?: KueryNode | JsonObject; + ensureRuleTypeIsAuthorized: (ruleTypeId: string, consumer: string, auth: string) => void; + logSuccessfulAuthorization: () => void; }> { if (this.authorization && this.shouldCheckAuthorization()) { const { username, authorizedRuleTypes } = await this.augmentRuleTypesWithAuthorization( this.ruleTypeRegistry.list(), - [ReadOperations.Find], + [operation], authorizationEntity ); diff --git a/x-pack/plugins/apm/public/components/shared/charts/helper/get_alert_annotations.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/helper/get_alert_annotations.test.tsx index 4f3ac31517020..81c4af44c90a3 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/helper/get_alert_annotations.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/helper/get_alert_annotations.test.tsx @@ -41,6 +41,7 @@ const alert: Alert = { 'rule.name': ['Latency threshold | frontend-rum'], [ALERT_DURATION]: [62879000], [ALERT_STATUS]: ['open'], + [SPACE_IDS]: ['myfakespaceid'], tags: ['apm', 'service.name:frontend-rum'], 'transaction.type': ['page-load'], [ALERT_PRODUCER]: ['apm'], diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/alerts_client_api.md b/x-pack/plugins/rule_registry/docs/alerts_client/alerts_client_api.md index b94a19f8e3f38..9a317a995d718 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/alerts_client_api.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/alerts_client_api.md @@ -10,5 +10,6 @@ Alerts as data client API Interface ### Interfaces +- [BulkUpdateOptions](interfaces/bulkupdateoptions.md) - [ConstructorOptions](interfaces/constructoroptions.md) - [UpdateOptions](interfaces/updateoptions.md) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md b/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md index 6b18ad0206797..11a5a4a1c98fe 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md @@ -22,10 +22,13 @@ on alerts as data. ### Methods -- [fetchAlert](alertsclient.md#fetchalert) +- [buildEsQueryWithAuthz](alertsclient.md#buildesquerywithauthz) +- [bulkUpdate](alertsclient.md#bulkupdate) +- [fetchAlertAndAudit](alertsclient.md#fetchalertandaudit) +- [fetchAlertAuditOperate](alertsclient.md#fetchalertauditoperate) - [get](alertsclient.md#get) -- [getAlertsIndex](alertsclient.md#getalertsindex) - [getAuthorizedAlertsIndices](alertsclient.md#getauthorizedalertsindices) +- [queryAndAuditAllAlerts](alertsclient.md#queryandauditallalerts) - [update](alertsclient.md#update) ## Constructors @@ -36,13 +39,13 @@ on alerts as data. #### Parameters -| Name | Type | -| :------ | :------ | +| Name | Type | +| :------------------ | :-------------------------------------------------------- | | `__namedParameters` | [ConstructorOptions](../interfaces/constructoroptions.md) | #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:66](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L66) +[alerts_client.ts:90](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L90) ## Properties @@ -52,9 +55,9 @@ on alerts as data. #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:63](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L63) +[alerts_client.ts:87](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L87) -___ +--- ### authorization @@ -62,9 +65,9 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:64](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L64) +[alerts_client.ts:88](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L88) -___ +--- ### esClient @@ -72,9 +75,9 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:65](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L65) +[alerts_client.ts:89](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L89) -___ +--- ### logger @@ -82,80 +85,138 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:62](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L62) +[alerts_client.ts:86](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L86) -___ +--- ### spaceId -• `Private` `Readonly` **spaceId**: `Promise` +• `Private` `Readonly` **spaceId**: `undefined` \| `string` #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:66](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L66) +[alerts_client.ts:90](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L90) ## Methods -### fetchAlert +### buildEsQueryWithAuthz -▸ `Private` **fetchAlert**(`__namedParameters`): `Promise`\>, ``"kibana.alert.owner"`` \| ``"rule.id"``\> & { `kibana.alert.owner`: `string` ; `rule.id`: `string` } & { `_version`: `undefined` \| `string` }\> +▸ `Private` **buildEsQueryWithAuthz**(`query`, `id`, `alertSpaceId`, `operation`, `config`): `Promise`<`Object`\> #### Parameters -| Name | Type | -| :------ | :------ | -| `__namedParameters` | `GetAlertParams` | +| Name | Type | +| :------------- | :-------------------------------- | +| `query` | `undefined` \| `null` \| `string` | +| `id` | `undefined` \| `null` \| `string` | +| `alertSpaceId` | `string` | +| `operation` | `Update` \| `Get` \| `Find` | +| `config` | `EsQueryConfig` | #### Returns -`Promise`\>, ``"kibana.alert.owner"`` \| ``"rule.id"``\> & { `kibana.alert.owner`: `string` ; `rule.id`: `string` } & { `_version`: `undefined` \| `string` }\> +`Promise`<`Object`\> #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:87](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L87) +[alerts_client.ts:242](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L242) -___ +--- -### get +### bulkUpdate + +▸ **bulkUpdate**(`__namedParameters`): `Promise` \| ApiResponse\> -▸ **get**(`__namedParameters`): `Promise`\>\> +#### Type parameters + +| Name | Type | +| :------- | :------------------------------------ | +| `Params` | `Params`: `AlertTypeParams` = `never` | #### Parameters -| Name | Type | -| :------ | :------ | -| `__namedParameters` | `GetAlertParams` | +| Name | Type | +| :------------------ | :--------------------------------------------------------------- | +| `__namedParameters` | [BulkUpdateOptions](../interfaces/bulkupdateoptions.md) | #### Returns -`Promise`\>\> +`Promise` \| ApiResponse\> #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:134](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L134) +[alerts_client.ts:424](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L424) + +--- -___ +### fetchAlertAndAudit -### getAlertsIndex +▸ `Private` **fetchAlertAndAudit**(`__namedParameters`): `Promise`\>\>\> -▸ **getAlertsIndex**(`featureIds`, `operations`): `Promise`<`Object`\> +This will be used as a part of the "find" api +In the future we will add an "aggs" param #### Parameters -| Name | Type | -| :------ | :------ | -| `featureIds` | `string`[] | -| `operations` | (`ReadOperations` \| `WriteOperations`)[] | +| Name | Type | +| :------------------ | :------------------------- | +| `__namedParameters` | `FetchAndAuditAlertParams` | #### Returns -`Promise`<`Object`\> +`Promise`\>\>\> + +#### Defined in + +[alerts_client.ts:108](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L108) + +--- + +### fetchAlertAuditOperate + +▸ `Private` **fetchAlertAuditOperate**(`__namedParameters`): `Promise`\> + +When an update by ids is requested, do a multi-get, ensure authz and audit alerts, then execute bulk update + +#### Parameters + +| Name | Type | +| :---------------------------- | :------------------------------------ | +| `__namedParameters` | `Object` | +| `__namedParameters.ids` | `string`[] | +| `__namedParameters.indexName` | `string` | +| `__namedParameters.operation` | `WriteOperations` \| `ReadOperations` | +| `__namedParameters.status` | `STATUS\_VALUES` | + +#### Returns + +`Promise`\> + +#### Defined in + +[alerts_client.ts:185](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L185) + +--- + +### get + +▸ **get**(`__namedParameters`): `Promise`\>\> + +#### Parameters + +| Name | Type | +| :------------------ | :--------------- | +| `__namedParameters` | `GetAlertParams` | + +#### Returns + +`Promise`\>\> #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:76](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L76) +[alerts_client.ts:344](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L344) -___ +--- ### getAuthorizedAlertsIndices @@ -163,8 +224,8 @@ ___ #### Parameters -| Name | Type | -| :------ | :------ | +| Name | Type | +| :----------- | :--------- | | `featureIds` | `string`[] | #### Returns @@ -173,30 +234,55 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:238](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L238) +[alerts_client.ts:481](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L481) + +--- + +### queryAndAuditAllAlerts + +▸ `Private` **queryAndAuditAllAlerts**(`__namedParameters`): `Promise` + +executes a search after to find alerts with query (+ authz filter) + +#### Parameters + +| Name | Type | +| :---------------------------- | :-------------------------- | +| `__namedParameters` | `Object` | +| `__namedParameters.index` | `string` | +| `__namedParameters.operation` | `Update` \| `Get` \| `Find` | +| `__namedParameters.query` | `string` | + +#### Returns + +`Promise` + +#### Defined in + +[alerts_client.ts:280](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L280) -___ +--- ### update -▸ **update**(`__namedParameters`): `Promise` +▸ **update**(`__namedParameters`): `Promise`<`Object`\> #### Type parameters -| Name | Type | -| :------ | :------ | +| Name | Type | +| :------- | :------------------------------------ | | `Params` | `Params`: `AlertTypeParams` = `never` | #### Parameters -| Name | Type | -| :------ | :------ | +| Name | Type | +| :------------------ | :------------------------------------------------------- | | `__namedParameters` | [UpdateOptions](../interfaces/updateoptions.md) | #### Returns -`Promise` +`Promise`<`Object`\> #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:179](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L179) +[alerts_client.ts:375](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L375) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md new file mode 100644 index 0000000000000..c01750aaa1b5d --- /dev/null +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md @@ -0,0 +1,58 @@ +[Alerts as data client API Interface](../alerts_client_api.md) / BulkUpdateOptions + +# Interface: BulkUpdateOptions + +## Type parameters + +| Name | Type | +| :------ | :------ | +| `Params` | `Params`: `AlertTypeParams` | + +## Table of contents + +### Properties + +- [ids](bulkupdateoptions.md#ids) +- [index](bulkupdateoptions.md#index) +- [query](bulkupdateoptions.md#query) +- [status](bulkupdateoptions.md#status) + +## Properties + +### ids + +• **ids**: `undefined` \| ``null`` \| `string`[] + +#### Defined in + +[alerts_client.ts:61](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L61) + +___ + +### index + +• **index**: `string` + +#### Defined in + +[alerts_client.ts:63](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L63) + +___ + +### query + +• **query**: `undefined` \| ``null`` \| `string` + +#### Defined in + +[alerts_client.ts:64](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L64) + +___ + +### status + +• **status**: `STATUS\_VALUES` + +#### Defined in + +[alerts_client.ts:62](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L62) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md index 23c5b94ad0d52..eae65b9d7df0a 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md @@ -19,7 +19,7 @@ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:40](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L40) +[alerts_client.ts:49](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L49) ___ @@ -29,7 +29,7 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:39](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L39) +[alerts_client.ts:48](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L48) ___ @@ -39,7 +39,7 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:41](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L41) +[alerts_client.ts:50](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L50) ___ @@ -49,4 +49,4 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:38](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L38) +[alerts_client.ts:47](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L47) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md index a8bff815c250f..b462eb2f4c964 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md @@ -25,7 +25,7 @@ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:47](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L47) +[alerts_client.ts:56](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L56) ___ @@ -35,7 +35,7 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:45](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L45) +[alerts_client.ts:54](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L54) ___ @@ -45,7 +45,7 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:48](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L48) +[alerts_client.ts:57](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L57) ___ @@ -55,4 +55,4 @@ ___ #### Defined in -[rule_registry/server/alert_data_client/alerts_client.ts:46](https://github.com/elastic/kibana/blob/48e1b91d751/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L46) +[alerts_client.ts:55](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L55) 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 index 73c6b4dd40526..ee81a39052522 100644 --- 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 @@ -14,9 +14,9 @@ export type AlertsClientMock = jest.Mocked; const createAlertsClientMock = () => { const mocked: AlertsClientMock = { get: jest.fn(), - getAlertsIndex: jest.fn(), update: jest.fn(), getAuthorizedAlertsIndices: jest.fn(), + bulkUpdate: jest.fn(), }; return mocked; }; 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 91282edf3778a..18f8d477c73a0 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 @@ -4,15 +4,20 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ +import Boom from '@hapi/boom'; import { PublicMethodsOf } from '@kbn/utility-types'; +import { Filter, buildEsQuery, EsQueryConfig } from '@kbn/es-query'; import { decodeVersion, encodeHitVersion } from '@kbn/securitysolution-es-utils'; import { mapConsumerToIndexName, - validFeatureIds, isValidFeatureId, + getSafeSortIds, + STATUS_VALUES, + getEsQueryConfig, } from '@kbn/rule-data-utils/target/alerts_as_data_rbac'; -import { AlertTypeParams } from '../../../alerting/server'; +import { InlineScript, QueryDslQueryContainer } from '@elastic/elasticsearch/api/types'; +import { AlertTypeParams, AlertingAuthorizationFilterType } from '../../../alerting/server'; import { ReadOperations, AlertingAuthorization, @@ -20,7 +25,7 @@ import { AlertingAuthorizationEntity, } from '../../../alerting/server'; import { Logger, ElasticsearchClient } from '../../../../../src/core/server'; -import { alertAuditEvent, AlertAuditAction } from './audit_events'; +import { alertAuditEvent, operationAlertAuditActionMap } from './audit_events'; import { AuditLogger } from '../../../security/server'; import { ALERT_STATUS, @@ -33,10 +38,13 @@ import { ParsedTechnicalFields } from '../../common/parse_technical_fields'; // TODO: Fix typings https://github.com/elastic/kibana/issues/101776 type NonNullableProps = Omit & { [K in Props]-?: NonNullable }; -type AlertType = NonNullableProps; +type AlertType = NonNullableProps< + ParsedTechnicalFields, + typeof RULE_ID | typeof ALERT_OWNER | typeof SPACE_IDS +>; const isValidAlert = (source?: ParsedTechnicalFields): source is AlertType => { - return source?.[RULE_ID] != null && source?.[ALERT_OWNER] != null; + return source?.[RULE_ID] != null && source?.[ALERT_OWNER] != null && source?.[SPACE_IDS] != null; }; export interface ConstructorOptions { logger: Logger; @@ -52,11 +60,26 @@ export interface UpdateOptions { index: string; } +export interface BulkUpdateOptions { + ids: string[] | undefined | null; + status: STATUS_VALUES; + index: string; + query: string | undefined | null; +} + interface GetAlertParams { id: string; index?: string; } +interface FetchAndAuditAlertParams { + id: string | null | undefined; + query: string | null | undefined; + index?: string; + operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get; + lastSortIds: Array | undefined; +} + /** * Provides apis to interact with alerts as data * ensures the request is authorized to perform read / write actions @@ -79,105 +102,323 @@ export class AlertsClient { this.spaceId = this.authorization.getSpaceId(); } - public async getAlertsIndex( - featureIds: string[], - operations: Array + private async ensureAllAuthorized( + items: Array<{ + _id: string; + // this is typed kind of crazy to fit the output of es api response to this + _source?: + | { [RULE_ID]?: string | null | undefined; [ALERT_OWNER]?: string | null | undefined } + | null + | undefined; + }>, + operation: ReadOperations.Find | ReadOperations.Get | WriteOperations.Update ) { - return this.authorization.getAugmentedRuleTypesWithAuthorization( - featureIds.length !== 0 ? featureIds : validFeatureIds, - operations, - AlertingAuthorizationEntity.Alert + const { hitIds, ownersAndRuleTypeIds } = items.reduce( + (acc, hit) => ({ + hitIds: [hit._id, ...acc.hitIds], + ownersAndRuleTypeIds: [ + { + [RULE_ID]: hit?._source?.[RULE_ID], + [ALERT_OWNER]: hit?._source?.[ALERT_OWNER], + }, + ], + }), + { hitIds: [], ownersAndRuleTypeIds: [] } as { + hitIds: string[]; + ownersAndRuleTypeIds: Array<{ + [RULE_ID]: string | null | undefined; + [ALERT_OWNER]: string | null | undefined; + }>; + } ); + + const assertString = (hit: unknown): hit is string => hit !== null && hit !== undefined; + + return Promise.all( + ownersAndRuleTypeIds.map((hit) => { + const alertOwner = hit?.[ALERT_OWNER]; + const ruleId = hit?.[RULE_ID]; + if (hit != null && assertString(alertOwner) && assertString(ruleId)) { + return this.authorization.ensureAuthorized({ + ruleTypeId: ruleId, + consumer: alertOwner, + operation, + entity: AlertingAuthorizationEntity.Alert, + }); + } + }) + ).catch((error) => { + for (const hitId of hitIds) { + this.auditLogger?.log( + alertAuditEvent({ + action: operationAlertAuditActionMap[operation], + id: hitId, + error, + }) + ); + } + throw error; + }); } - private async fetchAlert({ + /** + * This will be used as a part of the "find" api + * In the future we will add an "aggs" param + * @param param0 + * @returns + */ + private async fetchAlertAndAudit({ id, + query, index, - }: GetAlertParams): Promise<(AlertType & { _version: string | undefined }) | null | undefined> { + operation, + lastSortIds = [], + }: FetchAndAuditAlertParams) { try { const alertSpaceId = this.spaceId; if (alertSpaceId == null) { - this.logger.error('Failed to acquire spaceId from authorization client'); - return; + const errorMessage = 'Failed to acquire spaceId from authorization client'; + this.logger.error(`fetchAlertAndAudit threw an error: ${errorMessage}`); + throw Boom.failedDependency(`fetchAlertAndAudit threw an error: ${errorMessage}`); } + + const config = getEsQueryConfig(); + + let queryBody = { + query: await this.buildEsQueryWithAuthz(query, id, alertSpaceId, operation, config), + sort: [ + { + '@timestamp': { + order: 'asc', + unmapped_type: 'date', + }, + }, + ], + }; + + if (lastSortIds.length > 0) { + queryBody = { + ...queryBody, + // @ts-expect-error + search_after: lastSortIds, + }; + } + const result = await this.esClient.search({ - // Context: Originally thought of always just searching `.alerts-*` but that could - // result in a big performance hit. If the client already knows which index the alert - // belongs to, passing in the index will speed things up index: index ?? '.alerts-*', ignore_unavailable: true, - body: { - query: { - bool: { - filter: [{ term: { _id: id } }, { term: { [SPACE_IDS]: alertSpaceId } }], - }, - }, - }, + // @ts-expect-error + body: queryBody, seq_no_primary_term: true, }); - if (result == null || result.body == null || result.body.hits.hits.length === 0) { - return; + if (!result?.body.hits.hits.every((hit) => isValidAlert(hit._source))) { + const errorMessage = `Invalid alert found with id of "${id}" or with query "${query}" and operation ${operation}`; + this.logger.error(errorMessage); + throw Boom.badData(errorMessage); } - if (!isValidAlert(result.body.hits.hits[0]._source)) { - const errorMessage = `Unable to retrieve alert details for alert with id of "${id}".`; - this.logger.debug(errorMessage); - throw new Error(errorMessage); - } + await this.ensureAllAuthorized(result.body.hits.hits, operation); - return { - ...result.body.hits.hits[0]._source, - _version: encodeHitVersion(result.body.hits.hits[0]), - }; + result?.body.hits.hits.map((item) => + this.auditLogger?.log( + alertAuditEvent({ + action: operationAlertAuditActionMap[operation], + id: item._id, + outcome: 'unknown', + }) + ) + ); + + return result.body; } catch (error) { - const errorMessage = `Unable to retrieve alert with id of "${id}".`; - this.logger.debug(errorMessage); - throw error; + const errorMessage = `Unable to retrieve alert details for alert with id of "${id}" or with query "${query}" and operation ${operation} \nError: ${error}`; + this.logger.error(errorMessage); + throw Boom.notFound(errorMessage); } } - public async get({ - id, - index, - }: GetAlertParams): Promise { + /** + * When an update by ids is requested, do a multi-get, ensure authz and audit alerts, then execute bulk update + * @param param0 + * @returns + */ + private async fetchAlertAuditOperate({ + ids, + status, + indexName, + operation, + }: { + ids: string[]; + status: STATUS_VALUES; + indexName: string; + operation: ReadOperations.Find | ReadOperations.Get | WriteOperations.Update; + }) { 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, - index, + const mgetRes = await this.esClient.mget({ + index: indexName, + body: { + ids, + }, }); - if (alert == null) { - return; + await this.ensureAllAuthorized(mgetRes.body.docs, operation); + + for (const id of ids) { + this.auditLogger?.log( + alertAuditEvent({ + action: operationAlertAuditActionMap[operation], + id, + ...(operation === WriteOperations.Update ? { outcome: 'unknown' } : { operation }), + }) + ); } - // this.authorization leverages the alerting plugin's authorization - // client exposed to us for reuse - await this.authorization.ensureAuthorized({ - ruleTypeId: alert[RULE_ID], - consumer: alert[ALERT_OWNER], - operation: ReadOperations.Get, - entity: AlertingAuthorizationEntity.Alert, + const bulkUpdateRequest = mgetRes.body.docs.flatMap((item) => [ + { + update: { + _index: item._index, + _id: item._id, + }, + }, + { + doc: { [ALERT_STATUS]: status }, + }, + ]); + + const bulkUpdateResponse = await this.esClient.bulk({ + body: bulkUpdateRequest, }); + return bulkUpdateResponse; + } catch (exc) { + this.logger.error(`error in fetchAlertAuditOperate ${exc}`); + throw exc; + } + } - this.auditLogger?.log( - alertAuditEvent({ - action: AlertAuditAction.GET, - id, - }) + private async buildEsQueryWithAuthz( + query: string | null | undefined, + id: string | null | undefined, + alertSpaceId: string, + operation: WriteOperations.Update | ReadOperations.Get | ReadOperations.Find, + config: EsQueryConfig + ) { + try { + const { filter: authzFilter } = await this.authorization.getAuthorizationFilter( + AlertingAuthorizationEntity.Alert, + { + type: AlertingAuthorizationFilterType.ESDSL, + fieldNames: { consumer: ALERT_OWNER, ruleTypeId: RULE_ID }, + }, + operation + ); + return buildEsQuery( + undefined, + { query: query == null ? `_id:${id}` : query, language: 'kuery' }, + [ + (authzFilter as unknown) as Filter, + ({ term: { [SPACE_IDS]: alertSpaceId } } as unknown) as Filter, + ], + config ); + } catch (exc) { + this.logger.error(exc); + throw Boom.expectationFailed( + `buildEsQueryWithAuthz threw an error: unable to get authorization filter \n ${exc}` + ); + } + } + + /** + * executes a search after to find alerts with query (+ authz filter) + * @param param0 + * @returns + */ + private async queryAndAuditAllAlerts({ + index, + query, + operation, + }: { + index: string; + query: string; + operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get; + }) { + let lastSortIds; + let hasSortIds = true; + const alertSpaceId = this.spaceId; + if (alertSpaceId == null) { + this.logger.error('Failed to acquire spaceId from authorization client'); + return; + } + + const config = getEsQueryConfig(); - return alert; + const authorizedQuery = await this.buildEsQueryWithAuthz( + query, + null, + alertSpaceId, + operation, + config + ); + + while (hasSortIds) { + try { + const result = await this.fetchAlertAndAudit({ + id: null, + query, + index, + operation, + lastSortIds, + }); + + if (lastSortIds != null && result?.hits.hits.length === 0) { + return { auditedAlerts: true, authorizedQuery }; + } + if (result == null) { + this.logger.error('RESULT WAS EMPTY'); + return { auditedAlerts: false, authorizedQuery }; + } + if (result.hits.hits.length === 0) { + this.logger.error('Search resulted in no hits'); + return { auditedAlerts: true, authorizedQuery }; + } + + lastSortIds = getSafeSortIds(result.hits.hits[result.hits.hits.length - 1]?.sort); + if (lastSortIds != null && lastSortIds.length !== 0) { + hasSortIds = true; + } else { + hasSortIds = false; + return { auditedAlerts: true, authorizedQuery }; + } + } catch (error) { + const errorMessage = `queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query "${query}" and operation ${operation} \n ${error}`; + this.logger.error(errorMessage); + throw Boom.notFound(errorMessage); + } + } + } + + public async get({ id, index }: GetAlertParams) { + 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.fetchAlertAndAudit({ + id, + query: null, + index, + operation: ReadOperations.Get, + lastSortIds: undefined, + }); + + if (alert == null || alert.hits.hits.length === 0) { + const errorMessage = `Unable to retrieve alert details for alert with id of "${id}" and operation ${ReadOperations.Get}`; + this.logger.error(errorMessage); + throw Boom.notFound(errorMessage); + } + + // move away from pulling data from _source in the future + return alert.hits.hits[0]._source; } catch (error) { - this.logger.debug(`Error fetching alert with id of "${id}"`); - this.auditLogger?.log( - alertAuditEvent({ - action: AlertAuditAction.GET, - id, - error, - }) - ); + this.logger.error(`get threw an error: ${error}`); throw error; } } @@ -189,29 +430,19 @@ export class AlertsClient { index, }: UpdateOptions) { try { - const alert = await this.fetchAlert({ + const alert = await this.fetchAlertAndAudit({ id, + query: null, index, - }); - - if (alert == null) { - return; - } - - await this.authorization.ensureAuthorized({ - ruleTypeId: alert[RULE_ID], - consumer: alert[ALERT_OWNER], operation: WriteOperations.Update, - entity: AlertingAuthorizationEntity.Alert, + lastSortIds: undefined, }); - this.auditLogger?.log( - alertAuditEvent({ - action: AlertAuditAction.UPDATE, - id, - outcome: 'unknown', - }) - ); + if (alert == null || alert.hits.hits.length === 0) { + const errorMessage = `Unable to retrieve alert details for alert with id of "${id}" and operation ${ReadOperations.Get}`; + this.logger.error(errorMessage); + throw Boom.notFound(errorMessage); + } const { body: response } = await this.esClient.update({ ...decodeVersion(_version), @@ -230,39 +461,97 @@ export class AlertsClient { _version: encodeHitVersion(response), }; } catch (error) { - this.auditLogger?.log( - alertAuditEvent({ - action: AlertAuditAction.UPDATE, - id, - error, - }) - ); + this.logger.error(`update threw an error: ${error}`); throw error; } } - public async getAuthorizedAlertsIndices(featureIds: string[]): Promise { - const augmentedRuleTypes = await this.authorization.getAugmentedRuleTypesWithAuthorization( - featureIds, - [ReadOperations.Find, ReadOperations.Get, WriteOperations.Update], - AlertingAuthorizationEntity.Alert - ); + public async bulkUpdate({ + ids, + query, + index, + status, + }: BulkUpdateOptions) { + // rejects at the route level if more than 1000 id's are passed in + if (ids != null) { + return this.fetchAlertAuditOperate({ + ids, + status, + indexName: index, + operation: WriteOperations.Update, + }); + } else if (query != null) { + try { + // execute search after with query + authorization filter + // audit results of that query + const fetchAndAuditResponse = await this.queryAndAuditAllAlerts({ + query, + index, + operation: WriteOperations.Update, + }); - // As long as the user can read a minimum of one type of rule type produced by the provided feature, - // the user should be provided that features' alerts index. - // Limiting which alerts that user can read on that index will be done via the findAuthorizationFilter - const authorizedFeatures = new Set(); - for (const ruleType of augmentedRuleTypes.authorizedRuleTypes) { - authorizedFeatures.add(ruleType.producer); + if (!fetchAndAuditResponse?.auditedAlerts) { + throw Boom.unauthorized('Failed to audit alerts'); + } + + // executes updateByQuery with query + authorization filter + // used in the queryAndAuditAllAlerts function + const result = await this.esClient.updateByQuery({ + index, + conflicts: 'proceed', + refresh: true, + body: { + script: { + source: `if (ctx._source['${ALERT_STATUS}'] != null) { + ctx._source['${ALERT_STATUS}'] = '${status}' + } + if (ctx._source['signal.status'] != null) { + ctx._source['signal.status'] = '${status}' + }`, + lang: 'painless', + } as InlineScript, + query: fetchAndAuditResponse.authorizedQuery as Omit, + }, + ignore_unavailable: true, + }); + return result; + } catch (err) { + this.logger.error(`bulkUpdate threw an error: ${err}`); + throw err; + } + } else { + throw Boom.badRequest('no ids or query were provided for updating'); } + } + + public async getAuthorizedAlertsIndices(featureIds: string[]): Promise { + try { + const augmentedRuleTypes = await this.authorization.getAugmentedRuleTypesWithAuthorization( + featureIds, + [ReadOperations.Find, ReadOperations.Get, WriteOperations.Update], + AlertingAuthorizationEntity.Alert + ); - const toReturn = Array.from(authorizedFeatures).flatMap((feature) => { - if (isValidFeatureId(feature)) { - return mapConsumerToIndexName[feature]; + // As long as the user can read a minimum of one type of rule type produced by the provided feature, + // the user should be provided that features' alerts index. + // Limiting which alerts that user can read on that index will be done via the findAuthorizationFilter + const authorizedFeatures = new Set(); + for (const ruleType of augmentedRuleTypes.authorizedRuleTypes) { + authorizedFeatures.add(ruleType.producer); } - return []; - }); - return toReturn; + const toReturn = Array.from(authorizedFeatures).flatMap((feature) => { + if (isValidFeatureId(feature)) { + return mapConsumerToIndexName[feature]; + } + return []; + }); + + return toReturn; + } catch (exc) { + const errMessage = `getAuthorizedAlertsIndices failed to get authorized rule types: ${exc}`; + this.logger.error(errMessage); + throw Boom.failedDependency(errMessage); + } } } 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 index d07c23c7fbe9f..4b983e986f859 100644 --- 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 @@ -7,6 +7,7 @@ import { EcsEventOutcome, EcsEventType } from 'src/core/server'; import { AuditEvent } from '../../../security/server'; +import { ReadOperations, WriteOperations } from '../../../alerting/server'; export enum AlertAuditAction { GET = 'alert_get', @@ -14,6 +15,12 @@ export enum AlertAuditAction { FIND = 'alert_find', } +export const operationAlertAuditActionMap = { + [WriteOperations.Update]: AlertAuditAction.UPDATE, + [ReadOperations.Find]: AlertAuditAction.FIND, + [ReadOperations.Get]: AlertAuditAction.GET, +}; + type VerbsTuple = [string, string, string]; const eventVerbs: Record = { diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts new file mode 100644 index 0000000000000..41edb904dc67a --- /dev/null +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts @@ -0,0 +1,282 @@ +/* + * 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 { ALERT_OWNER, ALERT_STATUS, SPACE_IDS, RULE_ID } from '@kbn/rule-data-utils'; +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 { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; +import { AuditLogger } from '../../../../security/server'; + +const alertingAuthMock = alertingAuthorizationMock.create(); +const esClientMock = elasticsearchClientMock.createElasticsearchClient(); +const auditLogger = { + log: jest.fn(), +} as jest.Mocked; + +const alertsClientParams: jest.Mocked = { + logger: loggingSystemMock.create().get(), + authorization: alertingAuthMock, + esClient: esClientMock, + auditLogger, +}; + +const DEFAULT_SPACE = 'test_default_space_id'; + +beforeEach(() => { + jest.resetAllMocks(); + alertingAuthMock.getSpaceId.mockImplementation(() => 'test_default_space_id'); + // @ts-expect-error + alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => + Promise.resolve({ filter: [] }) + ); +}); + +describe('bulkUpdate()', () => { + describe('ids', () => { + test('logs successful event in audit logger', async () => { + const fakeAlertId = 'myfakeid1'; + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.mget.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + docs: [ + { + _id: fakeAlertId, + _index: indexName, + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }) + ); + esClientMock.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + errors: false, + took: 1, + items: [ + { + update: { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + result: 'updated', + status: 200, + }, + }, + ], + }, + }) + ); + await alertsClient.bulkUpdate({ + ids: [fakeAlertId], + query: undefined, + index: indexName, + status: 'closed', + }); + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `User is updating alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'unknown', + type: ['change'], + }, + error: undefined, + }); + }); + + test('audit error access if user is unauthorized for given alert', async () => { + const fakeAlertId = 'myfakeid1'; + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.mget.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + docs: [ + { + _id: fakeAlertId, + _index: indexName, + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }) + ); + alertingAuthMock.ensureAuthorized.mockRejectedValueOnce( + new Error('bulk update by ids test error') + ); + await expect( + alertsClient.bulkUpdate({ + ids: [fakeAlertId], + query: undefined, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"bulk update by ids test error"`); + + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to update alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'bulk update by ids test error', + }, + }); + }); + // test('throws an error if ES client fetch fails', async () => {}); + // test('throws an error if ES client bulk update fails', async () => {}); + // test('throws an error if ES client updateByQuery fails', async () => {}); + }); + describe('query', () => { + test('logs successful event in audit logger', async () => { + const fakeAlertId = 'myfakeid1'; + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + + esClientMock.updateByQuery.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + updated: 1, + }, + }) + ); + + await alertsClient.bulkUpdate({ + ids: undefined, + query: `${ALERT_STATUS}: open`, + index: indexName, + status: 'closed', + }); + expect(auditLogger.log).toHaveBeenCalledWith({ + message: `User is updating alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'unknown', + type: ['change'], + }, + error: undefined, + }); + }); + + test('audit error access if user is unauthorized for given alert', async () => { + const fakeAlertId = 'myfakeid1'; + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + alertingAuthMock.ensureAuthorized.mockRejectedValueOnce( + new Error('bulk update by query test error') + ); + await expect( + alertsClient.bulkUpdate({ + ids: undefined, + query: `${ALERT_STATUS}: open`, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update + Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update + Error: Error: bulk update by query test error" + `); + + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to update alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'bulk update by query test error', + }, + }); + }); + // test('throws an error if ES client fetch fails', async () => {}); + // test('throws an error if ES client bulk update fails', async () => {}); + // test('throws an error if ES client updateByQuery fails', async () => {}); + }); +}); 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 51789a09234e2..8b9ea944dc354 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 @@ -29,6 +29,10 @@ const alertsClientParams: jest.Mocked = { beforeEach(() => { jest.resetAllMocks(); alertingAuthMock.getSpaceId.mockImplementation(() => 'test_default_space_id'); + // @ts-expect-error + alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => + Promise.resolve({ filter: [] }) + ); }); describe('get()', () => { @@ -73,10 +77,9 @@ describe('get()', () => { const result = await alertsClient.get({ id: '1', index: '.alerts-observability-apm' }); expect(result).toMatchInlineSnapshot(` Object { - "_version": "WzM2MiwyXQ==", - "${ALERT_OWNER}": "apm", - "${ALERT_STATUS}": "open", - "${SPACE_IDS}": Array [ + "kibana.alert.owner": "apm", + "kibana.alert.status": "open", + "kibana.space_ids": Array [ "test_default_space_id", ], "message": "hello world 1", @@ -92,18 +95,37 @@ describe('get()', () => { "bool": Object { "filter": Array [ Object { - "term": Object { - "_id": "1", + "bool": Object { + "minimum_should_match": 1, + "should": Array [ + Object { + "match": Object { + "_id": "1", + }, + }, + ], }, }, + Object {}, Object { "term": Object { "kibana.space_ids": "test_default_space_id", }, }, ], + "must": Array [], + "must_not": Array [], + "should": Array [], }, }, + "sort": Array [ + Object { + "@timestamp": Object { + "order": "asc", + "unmapped_type": "date", + }, + }, + ], }, "ignore_unavailable": true, "index": ".alerts-observability-apm", @@ -151,12 +173,12 @@ describe('get()', () => { }, }) ); - await alertsClient.get({ id: '1', index: '.alerts-observability-apm' }); + await alertsClient.get({ id: 'NoxgpHkBqbdrfX07MqXV', 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]', + event: { action: 'alert_get', category: ['database'], outcome: 'unknown', type: ['access'] }, + message: 'User is accessing alert [id=NoxgpHkBqbdrfX07MqXV]', }); }); @@ -166,13 +188,11 @@ describe('get()', () => { esClientMock.search.mockRejectedValue(error); await expect( - alertsClient.get({ id: '1', index: '.alerts-observability-apm' }) - ).rejects.toThrowErrorMatchingInlineSnapshot(`"something went wrong"`); - expect(auditLogger.log).toHaveBeenCalledWith({ - error: { code: 'Error', message: 'something went wrong' }, - event: { action: 'alert_get', category: ['database'], outcome: 'failure', type: ['access'] }, - message: 'Failed attempt to access alert [id=1]', - }); + alertsClient.get({ id: 'NoxgpHkBqbdrfX07MqXV', index: '.alerts-observability-apm' }) + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "Unable to retrieve alert details for alert with id of \\"NoxgpHkBqbdrfX07MqXV\\" or with query \\"null\\" and operation get + Error: Error: something went wrong" + `); }); describe('authorization', () => { @@ -217,20 +237,16 @@ describe('get()', () => { 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', index: '.alerts-observability-apm' }); - - expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ - entity: 'alert', - consumer: 'apm', - operation: 'get', - ruleTypeId: 'apm.error_rate', + const result = await alertsClient.get({ + id: 'NoxgpHkBqbdrfX07MqXV', + index: '.alerts-observability-apm', }); + expect(result).toMatchInlineSnapshot(` Object { - "_version": "WzM2MiwyXQ==", - "${ALERT_OWNER}": "apm", - "${ALERT_STATUS}": "open", - "${SPACE_IDS}": Array [ + "kibana.alert.owner": "apm", + "kibana.alert.status": "open", + "kibana.space_ids": Array [ "test_default_space_id", ], "message": "hello world 1", @@ -238,25 +254,5 @@ describe('get()', () => { } `); }); - - 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', index: '.alerts-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/tests/update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/update.test.ts index 47a86c718d8de..63b97dad4cb3f 100644 --- 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 @@ -5,7 +5,7 @@ * 2.0. */ -import { ALERT_OWNER, ALERT_STATUS, SPACE_IDS } from '@kbn/rule-data-utils'; +import { ALERT_OWNER, ALERT_STATUS, SPACE_IDS, RULE_ID } from '@kbn/rule-data-utils'; import { AlertsClient, ConstructorOptions } from '../alerts_client'; import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; // eslint-disable-next-line @kbn/eslint/no-restricted-paths @@ -26,9 +26,15 @@ const alertsClientParams: jest.Mocked = { auditLogger, }; +const DEFAULT_SPACE = 'test_default_space_id'; + beforeEach(() => { jest.resetAllMocks(); - alertingAuthMock.getSpaceId.mockImplementation(() => 'test_default_space_id'); + alertingAuthMock.getSpaceId.mockImplementation(() => DEFAULT_SPACE); + // @ts-expect-error + alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => + Promise.resolve({ filter: [] }) + ); }); describe('update()', () => { @@ -55,11 +61,11 @@ describe('update()', () => { _index: '.alerts-observability-apm', _id: 'NoxgpHkBqbdrfX07MqXV', _source: { - 'rule.id': 'apm.error_rate', + [RULE_ID]: 'apm.error_rate', message: 'hello world 1', [ALERT_OWNER]: 'apm', [ALERT_STATUS]: 'open', - [SPACE_IDS]: ['test_default_space_id'], + [SPACE_IDS]: [DEFAULT_SPACE], }, }, ], @@ -145,7 +151,7 @@ describe('update()', () => { message: 'hello world 1', [ALERT_OWNER]: 'apm', [ALERT_STATUS]: 'open', - [SPACE_IDS]: ['test_default_space_id'], + [SPACE_IDS]: [DEFAULT_SPACE], }, }, ], @@ -167,7 +173,7 @@ describe('update()', () => { }) ); await alertsClient.update({ - id: '1', + id: 'NoxgpHkBqbdrfX07MqXV', status: 'closed', _version: undefined, index: '.alerts-observability-apm', @@ -181,33 +187,26 @@ describe('update()', () => { outcome: 'unknown', type: ['change'], }, - message: 'User is updating alert [id=1]', + message: 'User is updating alert [id=NoxgpHkBqbdrfX07MqXV]', }); }); test(`throws an error if ES client get fails`, async () => { - const error = new Error('something went wrong on get'); + const error = new Error('something went wrong on update'); const alertsClient = new AlertsClient(alertsClientParams); esClientMock.search.mockRejectedValue(error); await expect( alertsClient.update({ - id: '1', + id: 'NoxgpHkBqbdrfX07MqXV', status: 'closed', _version: undefined, index: '.alerts-observability-apm', }) - ).rejects.toThrowErrorMatchingInlineSnapshot(`"something went wrong on get"`); - expect(auditLogger.log).toHaveBeenCalledWith({ - error: { code: 'Error', message: 'something went wrong on get' }, - event: { - action: 'alert_update', - category: ['database'], - outcome: 'failure', - type: ['change'], - }, - message: 'Failed attempt to update alert [id=1]', - }); + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "Unable to retrieve alert details for alert with id of \\"NoxgpHkBqbdrfX07MqXV\\" or with query \\"null\\" and operation update + Error: Error: something went wrong on update" + `); }); test(`throws an error if ES client update fails`, async () => { @@ -238,7 +237,7 @@ describe('update()', () => { message: 'hello world 1', [ALERT_OWNER]: 'apm', [ALERT_STATUS]: 'open', - [SPACE_IDS]: ['test_default_space_id'], + [SPACE_IDS]: [DEFAULT_SPACE], }, }, ], @@ -250,21 +249,21 @@ describe('update()', () => { await expect( alertsClient.update({ - id: '1', + id: 'NoxgpHkBqbdrfX07MqXV', status: 'closed', _version: undefined, index: '.alerts-observability-apm', }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"something went wrong on update"`); expect(auditLogger.log).toHaveBeenCalledWith({ - error: { code: 'Error', message: 'something went wrong on update' }, + error: undefined, event: { action: 'alert_update', category: ['database'], - outcome: 'failure', + outcome: 'unknown', type: ['change'], }, - message: 'Failed attempt to update alert [id=1]', + message: 'User is updating alert [id=NoxgpHkBqbdrfX07MqXV]', }); }); @@ -298,7 +297,7 @@ describe('update()', () => { message: 'hello world 1', [ALERT_OWNER]: 'apm', [ALERT_STATUS]: 'open', - [SPACE_IDS]: ['test_default_space_id'], + [SPACE_IDS]: [DEFAULT_SPACE], }, }, ], @@ -325,18 +324,12 @@ describe('update()', () => { 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', + id: 'NoxgpHkBqbdrfX07MqXV', status: 'closed', _version: undefined, index: '.alerts-observability-apm', }); - expect(alertingAuthMock.ensureAuthorized).toHaveBeenCalledWith({ - entity: 'alert', - consumer: 'apm', - operation: 'update', - ruleTypeId: 'apm.error_rate', - }); expect(result).toMatchInlineSnapshot(` Object { "_id": "NoxgpHkBqbdrfX07MqXV", @@ -353,30 +346,5 @@ describe('update()', () => { } `); }); - - 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', - status: 'closed', - _version: undefined, - index: '.alerts-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/bulk_update_alerts.ts b/x-pack/plugins/rule_registry/server/routes/bulk_update_alerts.ts new file mode 100644 index 0000000000000..40f27f7eb7e23 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/routes/bulk_update_alerts.ts @@ -0,0 +1,93 @@ +/* + * 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 { IRouter } from 'kibana/server'; +import * as t from 'io-ts'; +import { id as _id } from '@kbn/securitysolution-io-ts-list-types'; +import { transformError } from '@kbn/securitysolution-es-utils'; + +import { buildRouteValidation } from './utils/route_validation'; +import { RacRequestHandlerContext } from '../types'; +import { BASE_RAC_ALERTS_API_PATH } from '../../common/constants'; + +export const bulkUpdateAlertsRoute = (router: IRouter) => { + router.post( + { + path: `${BASE_RAC_ALERTS_API_PATH}/bulk_update`, + validate: { + body: buildRouteValidation( + t.union([ + t.strict({ + status: t.union([t.literal('open'), t.literal('closed')]), + index: t.string, + ids: t.array(t.string), + query: t.undefined, + }), + t.strict({ + status: t.union([t.literal('open'), t.literal('closed')]), + index: t.string, + ids: t.undefined, + query: t.string, + }), + ]) + ), + }, + options: { + tags: ['access:rac'], + }, + }, + async (context, req, response) => { + try { + const alertsClient = await context.rac.getAlertsClient(); + const { status, ids, index, query } = req.body; + + if (ids != null && ids.length > 1000) { + return response.badRequest({ + body: { + message: 'cannot use more than 1000 ids', + }, + }); + } + + const updatedAlert = await alertsClient.bulkUpdate({ + ids, + status, + query, + index, + }); + + if (updatedAlert == null) { + return response.notFound({ + body: { message: `alerts with ids ${ids} and index ${index} not found` }, + }); + } + + return response.ok({ body: { success: true, ...updatedAlert } }); + } catch (exc) { + const err = transformError(exc); + + const contentType = { + 'content-type': 'application/json', + }; + const defaultedHeaders = { + ...contentType, + }; + + return response.customError({ + headers: defaultedHeaders, + statusCode: err.statusCode, + body: { + message: err.message, + attributes: { + success: false, + }, + }, + }); + } + } + ); +}; diff --git a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts index 073a48248f89a..31f25477d310a 100644 --- a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts +++ b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts @@ -12,6 +12,7 @@ import { ALERT_STATUS, CONSUMERS, ECS_VERSION, + SPACE_IDS, RULE_ID, SPACE_IDS, TIMESTAMP, diff --git a/x-pack/plugins/rule_registry/server/routes/index.ts b/x-pack/plugins/rule_registry/server/routes/index.ts index 6698cd7717268..0b900f26e56e6 100644 --- a/x-pack/plugins/rule_registry/server/routes/index.ts +++ b/x-pack/plugins/rule_registry/server/routes/index.ts @@ -10,9 +10,11 @@ import { RacRequestHandlerContext } from '../types'; import { getAlertByIdRoute } from './get_alert_by_id'; import { updateAlertByIdRoute } from './update_alert_by_id'; import { getAlertsIndexRoute } from './get_alert_index'; +import { bulkUpdateAlertsRoute } from './bulk_update_alerts'; export function defineRoutes(router: IRouter) { getAlertByIdRoute(router); updateAlertByIdRoute(router); getAlertsIndexRoute(router); + bulkUpdateAlertsRoute(router); } diff --git a/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_ids.sh b/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_ids.sh new file mode 100755 index 0000000000000..7c7c7d0ab679b --- /dev/null +++ b/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_ids.sh @@ -0,0 +1,30 @@ +#!/bin/sh + +# +# 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. +# + +set -e + +IDS=${1:-[\"Do4JnHoBqkRSppNZ6vre\"]} +STATUS=${2} + +echo $IDS +echo "'"$STATUS"'" + +cd ./hunter && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd ../observer && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd .. + +# Example: ./update_observability_alert.sh [\"my-alert-id\",\"another-alert-id\"] +curl -s -k \ + -H 'Content-Type: application/json' \ + -H 'kbn-xsrf: 123' \ + -u observer:changeme \ + -X POST ${KIBANA_URL}${SPACE_URL}/internal/rac/alerts/bulk_update \ +-d "{\"ids\": $IDS, \"status\":\"$STATUS\", \"index\":\".alerts-observability-apm\"}" | jq . +# -d "{\"ids\": $IDS, \"query\": \"kibana.rac.alert.status: open\", \"status\":\"$STATUS\", \"index\":\".alerts-observability-apm\"}" | jq . +# -d "{\"ids\": $IDS, \"status\":\"$STATUS\", \"index\":\".alerts-observability-apm\"}" | jq . diff --git a/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_query.sh b/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_query.sh new file mode 100755 index 0000000000000..4bcfd53b12299 --- /dev/null +++ b/x-pack/plugins/rule_registry/server/scripts/bulk_update_observability_alert_by_query.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +# +# 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. +# + +set -e + +QUERY=${1:-"kibana.rac.alert.status: open"} +STATUS=${2} + +echo $IDS +echo "'"$STATUS"'" + +cd ./hunter && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd ../observer && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd .. + +# Example: ./update_observability_alert.sh "kibana.rac.alert.stats: open" +curl -s -k \ + -H 'Content-Type: application/json' \ + -H 'kbn-xsrf: 123' \ + -u observer:changeme \ + -X POST ${KIBANA_URL}${SPACE_URL}/internal/rac/alerts/bulk_update \ +-d "{\"query\": \"$QUERY\", \"status\":\"$STATUS\", \"index\":\".alerts-observability-apm\"}" | jq . diff --git a/x-pack/plugins/rule_registry/server/scripts/bulk_update_old_security_solution_alert_by_query.sh b/x-pack/plugins/rule_registry/server/scripts/bulk_update_old_security_solution_alert_by_query.sh new file mode 100755 index 0000000000000..8725e791d8efa --- /dev/null +++ b/x-pack/plugins/rule_registry/server/scripts/bulk_update_old_security_solution_alert_by_query.sh @@ -0,0 +1,28 @@ +#!/bin/sh + +# +# 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. +# + +set -e + +QUERY=${1:-"signal.status: open"} +STATUS=${2} + +echo $IDS +echo "'"$STATUS"'" + +cd ./hunter && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd ../observer && sh ./post_detections_role.sh && sh ./post_detections_user.sh +cd .. + +# Example: ./update_observability_alert.sh "kibana.rac.alert.stats: open" +curl -s -k \ + -H 'Content-Type: application/json' \ + -H 'kbn-xsrf: 123' \ + -u hunter:changeme \ + -X POST ${KIBANA_URL}${SPACE_URL}/internal/rac/alerts/bulk_update \ +-d "{\"query\": \"$QUERY\", \"status\":\"$STATUS\", \"index\":\".siem-signals*\"}" | jq . diff --git a/x-pack/plugins/rule_registry/server/scripts/get_observability_alert.sh b/x-pack/plugins/rule_registry/server/scripts/get_observability_alert.sh index 6fbd0eb3dc816..1b34910c2b737 100755 --- a/x-pack/plugins/rule_registry/server/scripts/get_observability_alert.sh +++ b/x-pack/plugins/rule_registry/server/scripts/get_observability_alert.sh @@ -10,7 +10,7 @@ set -e USER=${1:-'observer'} -ID=${2:-'DHEnOXoB8br9Z2X1fq_l'} +ID=${2:-'Do4JnHoBqkRSppNZ6vre'} cd ./hunter && sh ./post_detections_role.sh && sh ./post_detections_user.sh cd ../observer && sh ./post_detections_role.sh && sh ./post_detections_user.sh diff --git a/x-pack/plugins/rule_registry/server/scripts/hunter/detections_role.json b/x-pack/plugins/rule_registry/server/scripts/hunter/detections_role.json index 80f63f80b849c..c1d5572672b8f 100644 --- a/x-pack/plugins/rule_registry/server/scripts/hunter/detections_role.json +++ b/x-pack/plugins/rule_registry/server/scripts/hunter/detections_role.json @@ -6,12 +6,7 @@ "kibana": [ { "feature": { - "ml": ["read"], - "siem": ["all"], - "actions": ["read"], - "ruleRegistry": ["all"], - "builtInAlerts": ["all"], - "alerting": ["all"] + "siem": ["all"] }, "spaces": ["*"] } diff --git a/x-pack/plugins/rule_registry/server/scripts/observer/detections_role.json b/x-pack/plugins/rule_registry/server/scripts/observer/detections_role.json index ee4a1d84f0b7e..4df9097a4f82d 100644 --- a/x-pack/plugins/rule_registry/server/scripts/observer/detections_role.json +++ b/x-pack/plugins/rule_registry/server/scripts/observer/detections_role.json @@ -6,7 +6,7 @@ "kibana": [ { "feature": { - "apm": ["read", "alerts_all"] + "apm": ["all"] }, "spaces": ["*"] } diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_execution_field_map.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_execution_field_map.ts index b3c70cd56d9e6..700ce66e2770f 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_execution_field_map.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_execution_field_map.ts @@ -18,6 +18,9 @@ import { * @deprecated ruleExecutionFieldMap is kept here only as a reference. It will be superseded with EventLog implementation */ export const ruleExecutionFieldMap = { + // [ALERT_OWNER]: { type: 'keyword', required: true }, + // [SPACE_IDS]: { type: 'keyword', array: true, required: true }, + // [RULE_ID]: { type: 'keyword', required: true }, [MESSAGE]: { type: 'keyword' }, [EVENT_SEQUENCE]: { type: 'long' }, [EVENT_END]: { type: 'date' }, diff --git a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_registry_log_client.ts b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_registry_log_client.ts index e2c5c98702f12..f5971475e2b16 100644 --- a/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_registry_log_client.ts +++ b/x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/rule_registry_log_client/rule_registry_log_client.ts @@ -213,6 +213,14 @@ export class RuleRegistryLogClient implements IRuleRegistryLogClient { ); } + // { [x: string]: string | string[] | ExecutionMetricValue; + // [x: number]: string; + // "kibana.space_ids": string[]; + // "event.action": T; + // "event.kind": string; + // "rule.id": string; + // "@timestamp": string; } + public async logExecutionMetric({ ruleId, namespace, diff --git a/x-pack/test/functional/es_archives/rule_registry/alerts/data.json b/x-pack/test/functional/es_archives/rule_registry/alerts/data.json index c8c01883197f5..940ebe5321b9d 100644 --- a/x-pack/test/functional/es_archives/rule_registry/alerts/data.json +++ b/x-pack/test/functional/es_archives/rule_registry/alerts/data.json @@ -82,3 +82,35 @@ } } } + +{ + "type": "doc", + "value": { + "index": ".alerts-security.alerts", + "id": "space1securityalert", + "source": { + "@timestamp": "2020-12-16T15:16:18.570Z", + "rule.id": "siem.signals", + "message": "hello world security", + "kibana.alert.owner": "siem", + "kibana.alert.status": "open", + "kibana.space_ids": ["space1"] + } + } +} + +{ + "type": "doc", + "value": { + "index": ".alerts-security.alerts", + "id": "space2securityalert", + "source": { + "@timestamp": "2020-12-16T15:16:18.570Z", + "rule.id": "siem.signals", + "message": "hello world security", + "kibana.alert.owner": "siem", + "kibana.alert.status": "open", + "kibana.space_ids": ["space2"] + } + } +} diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/bulk_update_alerts.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/bulk_update_alerts.ts new file mode 100644 index 0000000000000..aa8eb6bc8fcaa --- /dev/null +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/bulk_update_alerts.ts @@ -0,0 +1,225 @@ +/* + * 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 expect from '@kbn/expect'; + +import { + superUser, + globalRead, + obsOnly, + obsOnlyRead, + obsSec, + obsSecRead, + secOnly, + secOnlyRead, + secOnlySpace2, + secOnlyReadSpace2, + obsSecAllSpace2, + obsSecReadSpace2, + obsOnlySpace2, + obsOnlyReadSpace2, + obsOnlySpacesAll, + obsSecSpacesAll, + secOnlySpacesAll, + noKibanaPrivileges, +} from '../../../common/lib/authentication/users'; +import type { User } from '../../../common/lib/authentication/types'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { getSpaceUrlPrefix } from '../../../common/lib/authentication/spaces'; + +interface TestCase { + /** The space where the alert exists */ + space: string; + /** The ID of the alert */ + alertId: string; + /** The index of the alert */ + index: string; + /** Authorized users */ + authorizedUsers: User[]; + /** Unauthorized users */ + unauthorizedUsers: User[]; +} + +// eslint-disable-next-line import/no-default-export +export default ({ getService }: FtrProviderContext) => { + const supertestWithoutAuth = getService('supertestWithoutAuth'); + const esArchiver = getService('esArchiver'); + + const TEST_URL = '/internal/rac/alerts'; + const ALERTS_INDEX_URL = `${TEST_URL}/index`; + const SPACE1 = 'space1'; + const SPACE2 = 'space2'; + const APM_ALERT_ID = 'NoxgpHkBqbdrfX07MqXV'; + const APM_ALERT_INDEX = '.alerts-observability-apm'; + const SECURITY_SOLUTION_ALERT_ID = '020202'; + const SECURITY_SOLUTION_ALERT_INDEX = '.alerts-security.alerts'; + // const ALERT_VERSION = Buffer.from(JSON.stringify([0, 1]), 'utf8').toString('base64'); // required for optimistic concurrency control + + const getAPMIndexName = async (user: User) => { + const { + body: indexNames, + }: { body: { index_name: string[] | undefined } } = await supertestWithoutAuth + .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) + .auth(user.username, user.password) + .set('kbn-xsrf', 'true') + .expect(200); + const observabilityIndex = indexNames?.index_name?.find( + (indexName) => indexName === APM_ALERT_INDEX + ); + expect(observabilityIndex).to.eql(APM_ALERT_INDEX); // assert this here so we can use constants in the dynamically-defined test cases below + }; + + const getSecuritySolutionIndexName = async (user: User) => { + const { + body: indexNames, + }: { body: { index_name: string[] | undefined } } = await supertestWithoutAuth + .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) + .auth(user.username, user.password) + .set('kbn-xsrf', 'true') + .expect(200); + const securitySolution = indexNames?.index_name?.find( + (indexName) => indexName === SECURITY_SOLUTION_ALERT_INDEX + ); + expect(securitySolution).to.eql(SECURITY_SOLUTION_ALERT_INDEX); // assert this here so we can use constants in the dynamically-defined test cases below + }; + + describe('Alert - Bulk Update - RBAC - spaces', () => { + before(async () => { + await getSecuritySolutionIndexName(superUser); + await getAPMIndexName(superUser); + }); + + before(async () => { + await esArchiver.load('x-pack/test/functional/es_archives/rule_registry/alerts'); + }); + + after(async () => { + await esArchiver.unload('x-pack/test/functional/es_archives/rule_registry/alerts'); + }); + + function addTests({ space, authorizedUsers, unauthorizedUsers, alertId, index }: TestCase) { + authorizedUsers.forEach(({ username, password }) => { + it(`${username} should bulk update alert with given id ${alertId} in ${space}/${index}`, async () => { + await esArchiver.load('x-pack/test/functional/es_archives/rule_registry/alerts'); // since this is a success case, reload the test data immediately beforehand + const { body: updated } = await supertestWithoutAuth + .post(`${getSpaceUrlPrefix(space)}${TEST_URL}/bulk_update`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({ + ids: [alertId], + status: 'closed', + index, + }); + expect(updated.statusCode).to.eql(200); + const items = updated.body.items; + // @ts-expect-error + items.map((item) => expect(item.update.result).to.eql('updated')); + }); + + it(`${username} should bulk update alerts which match query in ${space}/${index}`, async () => { + const { body: updated } = await supertestWithoutAuth + .post(`${getSpaceUrlPrefix(space)}${TEST_URL}/bulk_update`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({ + status: 'closed', + query: 'kibana.alert.status: open', + index, + }); + expect(updated.statusCode).to.eql(200); + expect(updated.body.updated).to.greaterThan(0); + }); + }); + + unauthorizedUsers.forEach(({ username, password }) => { + it(`${username} should NOT be able to update alert ${alertId} in ${space}/${index}`, async () => { + const res = await supertestWithoutAuth + .post(`${getSpaceUrlPrefix(space)}${TEST_URL}/bulk_update`) + .auth(username, password) + .set('kbn-xsrf', 'true') + .send({ + ids: [alertId], + status: 'closed', + index, + }); + expect([403, 404]).to.contain(res.statusCode); + }); + }); + } + + // Alert - Update - RBAC - spaces Security Solution superuser should bulk update alerts which match query in space1/.alerts-security.alerts + // Alert - Update - RBAC - spaces superuser should bulk update alert with given id 020202 in space1/.alerts-security.alerts + describe('Security Solution', () => { + const authorizedInAllSpaces = [superUser, secOnlySpacesAll, obsSecSpacesAll]; + const authorizedOnlyInSpace1 = [secOnly, obsSec]; + const authorizedOnlyInSpace2 = [secOnlySpace2, obsSecAllSpace2]; + const unauthorized = [ + // these users are not authorized to update alerts for the Security Solution in any space + globalRead, + secOnlyRead, + obsSecRead, + secOnlyReadSpace2, + obsSecReadSpace2, + obsOnly, + obsOnlyRead, + obsOnlySpace2, + obsOnlyReadSpace2, + obsOnlySpacesAll, + noKibanaPrivileges, + ]; + + addTests({ + space: SPACE1, + alertId: SECURITY_SOLUTION_ALERT_ID, + index: SECURITY_SOLUTION_ALERT_INDEX, + authorizedUsers: [...authorizedInAllSpaces, ...authorizedOnlyInSpace1], + unauthorizedUsers: [...authorizedOnlyInSpace2, ...unauthorized], + }); + addTests({ + space: SPACE2, + alertId: SECURITY_SOLUTION_ALERT_ID, + index: SECURITY_SOLUTION_ALERT_INDEX, + authorizedUsers: [...authorizedInAllSpaces, ...authorizedOnlyInSpace2], + unauthorizedUsers: [...authorizedOnlyInSpace1, ...unauthorized], + }); + }); + + describe('APM', () => { + const authorizedInAllSpaces = [superUser, obsOnlySpacesAll, obsSecSpacesAll]; + const authorizedOnlyInSpace1 = [obsOnly, obsSec]; + const authorizedOnlyInSpace2 = [obsOnlySpace2, obsSecAllSpace2]; + const unauthorized = [ + // these users are not authorized to update alerts for APM in any space + globalRead, + obsOnlyRead, + obsSecRead, + obsOnlyReadSpace2, + obsSecReadSpace2, + secOnly, + secOnlyRead, + secOnlySpace2, + secOnlyReadSpace2, + secOnlySpacesAll, + noKibanaPrivileges, + ]; + + addTests({ + space: SPACE1, + alertId: APM_ALERT_ID, + index: APM_ALERT_INDEX, + authorizedUsers: [...authorizedInAllSpaces, ...authorizedOnlyInSpace1], + unauthorizedUsers: [...authorizedOnlyInSpace2, ...unauthorized], + }); + addTests({ + space: SPACE2, + alertId: APM_ALERT_ID, + index: APM_ALERT_INDEX, + authorizedUsers: [...authorizedInAllSpaces, ...authorizedOnlyInSpace2], + unauthorizedUsers: [...authorizedOnlyInSpace1, ...unauthorized], + }); + }); + }); +}; diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alert_by_id.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alert_by_id.ts index 05b55128fab36..15729f83ebcf1 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alert_by_id.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alert_by_id.ts @@ -152,11 +152,11 @@ export default ({ getService }: FtrProviderContext) => { unauthorizedUsers.forEach(({ username, password }) => { it(`${username} should NOT be able to access alert ${alertId} in ${space}/${index}`, async () => { - await supertestWithoutAuth + const res = await supertestWithoutAuth .get(`${getSpaceUrlPrefix(space)}${TEST_URL}?id=${alertId}&index=${index}`) .auth(username, password) - .set('kbn-xsrf', 'true') - .expect(403); + .set('kbn-xsrf', 'true'); + expect([403, 404]).to.contain(res.statusCode); }); }); } diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/index.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/index.ts index baea62c157218..26bd5b72771d7 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/index.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/index.ts @@ -25,5 +25,6 @@ export default ({ loadTestFile, getService }: FtrProviderContext): void => { // Basic loadTestFile(require.resolve('./get_alert_by_id')); loadTestFile(require.resolve('./update_alert')); + loadTestFile(require.resolve('./bulk_update_alerts')); }); }; diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/update_alert.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/update_alert.ts index 4fb087e813768..917cb31869bcc 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/update_alert.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/update_alert.ts @@ -145,25 +145,11 @@ export default ({ getService }: FtrProviderContext) => { }) .expect(404); }); - - it(`${username} should return a 404 when superuser accesses not-existent alerts as data index`, async () => { - await supertestWithoutAuth - .get(`${getSpaceUrlPrefix(space)}${TEST_URL}?id=${APM_ALERT_ID}&index=myfakeindex`) - .auth(username, password) - .set('kbn-xsrf', 'true') - .send({ - ids: [APM_ALERT_ID], - status: 'closed', - index: 'this index does not exist', - _version: ALERT_VERSION, - }) - .expect(404); - }); }); unauthorizedUsers.forEach(({ username, password }) => { it(`${username} should NOT be able to update alert ${alertId} in ${space}/${index}`, async () => { - await supertestWithoutAuth + const res = await supertestWithoutAuth .post(`${getSpaceUrlPrefix(space)}${TEST_URL}`) .auth(username, password) .set('kbn-xsrf', 'true') @@ -172,8 +158,8 @@ export default ({ getService }: FtrProviderContext) => { status: 'closed', index, _version: ALERT_VERSION, - }) - .expect(403); + }); + expect([403, 404]).to.contain(res.statusCode); }); }); } diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/trial/get_alerts.ts b/x-pack/test/rule_registry/security_and_spaces/tests/trial/get_alerts.ts index a38f6cf3263b1..56c2ba94c5ad3 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/trial/get_alerts.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/trial/get_alerts.ts @@ -73,7 +73,7 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${TEST_URL}?id=NoxgpHkBqbdrfX07MqXV&index=${apmIndex}`) .auth(obsMinRead.username, obsMinRead.password) .set('kbn-xsrf', 'true') - .expect(403); + .expect(404); }); it(`${obsMinReadSpacesAll.username} should NOT be able to access the APM alert in ${SPACE1}`, async () => { @@ -82,7 +82,7 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${TEST_URL}?id=NoxgpHkBqbdrfX07MqXV&index=${apmIndex}`) .auth(obsMinReadSpacesAll.username, obsMinReadSpacesAll.password) .set('kbn-xsrf', 'true') - .expect(403); + .expect(404); }); }); diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts b/x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts index 14d903c7ac556..0442e25fcf20c 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/trial/update_alert.ts @@ -166,7 +166,7 @@ export default ({ getService }: FtrProviderContext) => { index: apmIndex, _version: Buffer.from(JSON.stringify([0, 1]), 'utf8').toString('base64'), }) - .expect(403); + .expect(404); }); it(`${obsMinAllSpacesAll.username} should NOT be able to update the APM alert in ${SPACE1}`, async () => { @@ -181,7 +181,7 @@ export default ({ getService }: FtrProviderContext) => { index: apmIndex, _version: Buffer.from(JSON.stringify([0, 1]), 'utf8').toString('base64'), }) - .expect(403); + .expect(404); }); }); }); From 60e35bb06092312da3c68e6bb6ec7598237b339c Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 10:09:15 -0400 Subject: [PATCH 2/9] removes duplicate import after rebase with master --- .../plugins/rule_registry/server/routes/get_alert_by_id.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts index 31f25477d310a..073a48248f89a 100644 --- a/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts +++ b/x-pack/plugins/rule_registry/server/routes/get_alert_by_id.test.ts @@ -12,7 +12,6 @@ import { ALERT_STATUS, CONSUMERS, ECS_VERSION, - SPACE_IDS, RULE_ID, SPACE_IDS, TIMESTAMP, From ce41d563af85e68a78c2a2e69f477df5b30cd491 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 13:40:12 -0400 Subject: [PATCH 3/9] adds more jest tests --- .../tests/bulk_update.test.ts | 568 ++++++++++++------ 1 file changed, 372 insertions(+), 196 deletions(-) diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts index 41edb904dc67a..62fec1319d112 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts @@ -12,6 +12,7 @@ import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; import { AuditLogger } from '../../../../security/server'; +import { AlertingAuthorizationEntity } from '../../../../alerting/server'; const alertingAuthMock = alertingAuthorizationMock.create(); const esClientMock = elasticsearchClientMock.createElasticsearchClient(); @@ -35,145 +36,105 @@ beforeEach(() => { alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => Promise.resolve({ filter: [] }) ); + alertingAuthMock.ensureAuthorized.mockImplementation( + // @ts-expect-error + async ({ + ruleTypeId, + consumer, + operation, + entity, + }: { + ruleTypeId: string; + consumer: string; + operation: string; + entity: typeof AlertingAuthorizationEntity.Alert; + }) => { + if (ruleTypeId === 'apm.error_rate' && consumer === 'apm') { + return Promise.resolve(); + } + return Promise.reject(new Error(`Unauthorized for ${ruleTypeId} and ${consumer}`)); + } + ); }); +const fakeAlertId = 'myfakeid1'; +const successfulAuthzHit = 'successfulAuthzHit'; +const unsuccessfulAuthzHit = 'unsuccessfulAuthzHit'; +// fakeRuleTypeId will cause authz to fail +const fakeRuleTypeId = 'fake.rule'; + describe('bulkUpdate()', () => { describe('ids', () => { - test('logs successful event in audit logger', async () => { - const fakeAlertId = 'myfakeid1'; - const indexName = '.alerts-observability-apm.alerts'; - const alertsClient = new AlertsClient(alertsClientParams); - esClientMock.mget.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - docs: [ - { - _id: fakeAlertId, - _index: indexName, - _source: { - [RULE_ID]: 'apm.error_rate', - [ALERT_OWNER]: 'apm', - [ALERT_STATUS]: 'open', - [SPACE_IDS]: [DEFAULT_SPACE], - }, - }, - ], - }, - }) - ); - esClientMock.bulk.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - errors: false, - took: 1, - items: [ - { - update: { + describe('audit log', () => { + test('logs successful event in audit logger', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.mget.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + docs: [ + { _id: fakeAlertId, - _index: '.alerts-observability-apm.alerts', - result: 'updated', - status: 200, + _index: indexName, + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, }, - }, - ], - }, - }) - ); - await alertsClient.bulkUpdate({ - ids: [fakeAlertId], - query: undefined, - index: indexName, - status: 'closed', - }); - expect(auditLogger.log).toHaveBeenLastCalledWith({ - message: `User is updating alert [id=${fakeAlertId}]`, - event: { - action: 'alert_update', - category: ['database'], - outcome: 'unknown', - type: ['change'], - }, - error: undefined, - }); - }); - - test('audit error access if user is unauthorized for given alert', async () => { - const fakeAlertId = 'myfakeid1'; - const indexName = '.alerts-observability-apm.alerts'; - const alertsClient = new AlertsClient(alertsClientParams); - esClientMock.mget.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - docs: [ - { - _id: fakeAlertId, - _index: indexName, - _source: { - [RULE_ID]: 'apm.error_rate', - [ALERT_OWNER]: 'apm', - [ALERT_STATUS]: 'open', - [SPACE_IDS]: [DEFAULT_SPACE], + ], + }, + }) + ); + esClientMock.bulk.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + errors: false, + took: 1, + items: [ + { + update: { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + result: 'updated', + status: 200, + }, }, - }, - ], - }, - }) - ); - alertingAuthMock.ensureAuthorized.mockRejectedValueOnce( - new Error('bulk update by ids test error') - ); - await expect( - alertsClient.bulkUpdate({ + ], + }, + }) + ); + await alertsClient.bulkUpdate({ ids: [fakeAlertId], query: undefined, index: indexName, status: 'closed', - }) - ).rejects.toThrowErrorMatchingInlineSnapshot(`"bulk update by ids test error"`); - - expect(auditLogger.log).toHaveBeenLastCalledWith({ - message: `Failed attempt to update alert [id=${fakeAlertId}]`, - event: { - action: 'alert_update', - category: ['database'], - outcome: 'failure', - type: ['change'], - }, - error: { - code: 'Error', - message: 'bulk update by ids test error', - }, + }); + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `User is updating alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'unknown', + type: ['change'], + }, + error: undefined, + }); }); - }); - // test('throws an error if ES client fetch fails', async () => {}); - // test('throws an error if ES client bulk update fails', async () => {}); - // test('throws an error if ES client updateByQuery fails', async () => {}); - }); - describe('query', () => { - test('logs successful event in audit logger', async () => { - const fakeAlertId = 'myfakeid1'; - const indexName = '.alerts-observability-apm.alerts'; - const alertsClient = new AlertsClient(alertsClientParams); - esClientMock.search.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - took: 5, - timed_out: false, - _shards: { - total: 1, - successful: 1, - failed: 0, - skipped: 0, - }, - hits: { - total: 1, - max_score: 999, - hits: [ + + test('audit error access if user is unauthorized for given alert', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.mget.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + docs: [ { _id: fakeAlertId, - _index: '.alerts-observability-apm.alerts', + _index: indexName, _source: { - [RULE_ID]: 'apm.error_rate', + [RULE_ID]: fakeRuleTypeId, [ALERT_OWNER]: 'apm', [ALERT_STATUS]: 'open', [SPACE_IDS]: [DEFAULT_SPACE], @@ -181,58 +142,43 @@ describe('bulkUpdate()', () => { }, ], }, - }, - }) - ); + }) + ); - esClientMock.updateByQuery.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - updated: 1, - }, - }) - ); + await expect( + alertsClient.bulkUpdate({ + ids: [fakeAlertId], + query: undefined, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized for fake.rule and apm"`); - await alertsClient.bulkUpdate({ - ids: undefined, - query: `${ALERT_STATUS}: open`, - index: indexName, - status: 'closed', - }); - expect(auditLogger.log).toHaveBeenCalledWith({ - message: `User is updating alert [id=${fakeAlertId}]`, - event: { - action: 'alert_update', - category: ['database'], - outcome: 'unknown', - type: ['change'], - }, - error: undefined, + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to update alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); }); - }); - test('audit error access if user is unauthorized for given alert', async () => { - const fakeAlertId = 'myfakeid1'; - const indexName = '.alerts-observability-apm.alerts'; - const alertsClient = new AlertsClient(alertsClientParams); - esClientMock.search.mockResolvedValueOnce( - elasticsearchClientMock.createApiResponse({ - body: { - took: 5, - timed_out: false, - _shards: { - total: 1, - successful: 1, - failed: 0, - skipped: 0, - }, - hits: { - total: 1, - max_score: 999, - hits: [ + test('logs multiple error events in audit logger', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.mget.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + docs: [ { - _id: fakeAlertId, - _index: '.alerts-observability-apm.alerts', + _id: successfulAuthzHit, + _index: indexName, _source: { [RULE_ID]: 'apm.error_rate', [ALERT_OWNER]: 'apm', @@ -240,39 +186,269 @@ describe('bulkUpdate()', () => { [SPACE_IDS]: [DEFAULT_SPACE], }, }, + { + _id: unsuccessfulAuthzHit, + _index: indexName, + _source: { + [RULE_ID]: fakeRuleTypeId, + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, ], }, + }) + ); + + await expect( + alertsClient.bulkUpdate({ + ids: [successfulAuthzHit, unsuccessfulAuthzHit], + query: undefined, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized for fake.rule and apm"`); + expect(auditLogger.log.mock.calls).toHaveLength(2); + expect(auditLogger.log.mock.calls[0][0]).toEqual({ + message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); + expect(auditLogger.log.mock.calls[1][0]).toEqual({ + message: `Failed attempt to update alert [id=${successfulAuthzHit}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], }, - }) - ); - alertingAuthMock.ensureAuthorized.mockRejectedValueOnce( - new Error('bulk update by query test error') - ); - await expect( - alertsClient.bulkUpdate({ + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); + }); + }); + + // test('throws an error if ES client fetch fails', async () => {}); + // test('throws an error if ES client bulk update fails', async () => {}); + // test('throws an error if ES client updateByQuery fails', async () => {}); + }); + describe('query', () => { + describe('audit log', () => { + test('logs successful event in audit logger', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + + esClientMock.updateByQuery.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + updated: 1, + }, + }) + ); + + await alertsClient.bulkUpdate({ ids: undefined, query: `${ALERT_STATUS}: open`, index: indexName, status: 'closed', - }) - ).rejects.toThrowErrorMatchingInlineSnapshot(` - "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update - Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update - Error: Error: bulk update by query test error" - `); + }); + expect(auditLogger.log).toHaveBeenCalledWith({ + message: `User is updating alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'unknown', + type: ['change'], + }, + error: undefined, + }); + }); + + test('audit error access if user is unauthorized for given alert', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + _id: fakeAlertId, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: fakeRuleTypeId, + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + await expect( + alertsClient.bulkUpdate({ + ids: undefined, + query: `${ALERT_STATUS}: open`, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update + Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update + Error: Error: Unauthorized for fake.rule and apm" + `); - expect(auditLogger.log).toHaveBeenLastCalledWith({ - message: `Failed attempt to update alert [id=${fakeAlertId}]`, - event: { - action: 'alert_update', - category: ['database'], - outcome: 'failure', - type: ['change'], - }, - error: { - code: 'Error', - message: 'bulk update by query test error', - }, + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to update alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); + }); + + test('logs multiple error events in audit logger', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 2, + max_score: 999, + hits: [ + { + _id: successfulAuthzHit, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: 'apm.error_rate', + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + { + _id: unsuccessfulAuthzHit, + _index: '.alerts-observability-apm.alerts', + _source: { + [RULE_ID]: fakeRuleTypeId, + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + await expect( + alertsClient.bulkUpdate({ + ids: undefined, + query: `${ALERT_STATUS}: open`, + index: indexName, + status: 'closed', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update + Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update + Error: Error: Unauthorized for fake.rule and apm" + `); + + expect(auditLogger.log.mock.calls).toHaveLength(2); + expect(auditLogger.log.mock.calls[0][0]).toEqual({ + message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); + expect(auditLogger.log.mock.calls[1][0]).toEqual({ + message: `Failed attempt to update alert [id=${successfulAuthzHit}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); }); }); // test('throws an error if ES client fetch fails', async () => {}); From c3ad00fc12f7b4cf1589549f506caf7991d1bbb1 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 14:53:16 -0400 Subject: [PATCH 4/9] rename private functions in alerts as data client and add missing whitespace char to inline snapshots --- .../server/alert_data_client/alerts_client.ts | 14 +++++++------- .../alert_data_client/tests/bulk_update.test.ts | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) 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 18f8d477c73a0..e1e3473817aaf 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 @@ -167,7 +167,7 @@ export class AlertsClient { * @param param0 * @returns */ - private async fetchAlertAndAudit({ + private async singleSearchAfterAndAduti({ id, query, index, @@ -243,7 +243,7 @@ export class AlertsClient { * @param param0 * @returns */ - private async fetchAlertAuditOperate({ + private async mgetAlertsAuditOperate({ ids, status, indexName, @@ -291,7 +291,7 @@ export class AlertsClient { }); return bulkUpdateResponse; } catch (exc) { - this.logger.error(`error in fetchAlertAuditOperate ${exc}`); + this.logger.error(`error in mgetAlertsAuditOperate ${exc}`); throw exc; } } @@ -363,7 +363,7 @@ export class AlertsClient { while (hasSortIds) { try { - const result = await this.fetchAlertAndAudit({ + const result = await this.singleSearchAfterAndAduti({ id: null, query, index, @@ -401,7 +401,7 @@ export class AlertsClient { public async get({ id, index }: GetAlertParams) { 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.fetchAlertAndAudit({ + const alert = await this.singleSearchAfterAndAduti({ id, query: null, index, @@ -430,7 +430,7 @@ export class AlertsClient { index, }: UpdateOptions) { try { - const alert = await this.fetchAlertAndAudit({ + const alert = await this.singleSearchAfterAndAduti({ id, query: null, index, @@ -474,7 +474,7 @@ export class AlertsClient { }: BulkUpdateOptions) { // rejects at the route level if more than 1000 id's are passed in if (ids != null) { - return this.fetchAlertAuditOperate({ + return this.mgetAlertsAuditOperate({ ids, status, indexName: index, diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts index 62fec1319d112..e739458848310 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts @@ -346,8 +346,8 @@ describe('bulkUpdate()', () => { status: 'closed', }) ).rejects.toThrowErrorMatchingInlineSnapshot(` - "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update - Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update + "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update + Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update Error: Error: Unauthorized for fake.rule and apm" `); @@ -417,8 +417,8 @@ describe('bulkUpdate()', () => { status: 'closed', }) ).rejects.toThrowErrorMatchingInlineSnapshot(` - "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update - Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update + "queryAndAuditAllAlerts threw an error: Unable to retrieve alerts with query \\"kibana.alert.status: open\\" and operation update + Error: Unable to retrieve alert details for alert with id of \\"null\\" or with query \\"kibana.alert.status: open\\" and operation update Error: Error: Unauthorized for fake.rule and apm" `); From d4e81644eba35697c9880fa24be815f8cffa0c83 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 15:00:19 -0400 Subject: [PATCH 5/9] fixes typo from renaming --- .../server/alert_data_client/alerts_client.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 e1e3473817aaf..c4022582ede30 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 @@ -72,7 +72,7 @@ interface GetAlertParams { index?: string; } -interface FetchAndAuditAlertParams { +interface SingleSearchAfterAndAudit { id: string | null | undefined; query: string | null | undefined; index?: string; @@ -102,6 +102,12 @@ export class AlertsClient { this.spaceId = this.authorization.getSpaceId(); } + /** + * Accepts an array of ES documents and executes ensureAuthorized for the given operation + * @param items + * @param operation + * @returns + */ private async ensureAllAuthorized( items: Array<{ _id: string; @@ -167,13 +173,13 @@ export class AlertsClient { * @param param0 * @returns */ - private async singleSearchAfterAndAduti({ + private async singleSearchAfterAndAudit({ id, query, index, operation, lastSortIds = [], - }: FetchAndAuditAlertParams) { + }: SingleSearchAfterAndAudit) { try { const alertSpaceId = this.spaceId; if (alertSpaceId == null) { @@ -363,7 +369,7 @@ export class AlertsClient { while (hasSortIds) { try { - const result = await this.singleSearchAfterAndAduti({ + const result = await this.singleSearchAfterAndAudit({ id: null, query, index, @@ -401,7 +407,7 @@ export class AlertsClient { public async get({ id, index }: GetAlertParams) { 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.singleSearchAfterAndAduti({ + const alert = await this.singleSearchAfterAndAudit({ id, query: null, index, @@ -430,7 +436,7 @@ export class AlertsClient { index, }: UpdateOptions) { try { - const alert = await this.singleSearchAfterAndAduti({ + const alert = await this.singleSearchAfterAndAudit({ id, query: null, index, From 8dd966f0e0321e27c5a177f181a9599b90e1c8b7 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Thu, 5 Aug 2021 15:01:13 -0400 Subject: [PATCH 6/9] update alerts as data client docs --- .../alerts_client/classes/alertsclient.md | 192 ++++++++++-------- .../interfaces/bulkupdateoptions.md | 8 +- .../interfaces/constructoroptions.md | 8 +- .../alerts_client/interfaces/updateoptions.md | 8 +- 4 files changed, 120 insertions(+), 96 deletions(-) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md b/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md index 11a5a4a1c98fe..7c79e0a5e4c0f 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/classes/alertsclient.md @@ -24,11 +24,12 @@ on alerts as data. - [buildEsQueryWithAuthz](alertsclient.md#buildesquerywithauthz) - [bulkUpdate](alertsclient.md#bulkupdate) -- [fetchAlertAndAudit](alertsclient.md#fetchalertandaudit) -- [fetchAlertAuditOperate](alertsclient.md#fetchalertauditoperate) +- [ensureAllAuthorized](alertsclient.md#ensureallauthorized) - [get](alertsclient.md#get) - [getAuthorizedAlertsIndices](alertsclient.md#getauthorizedalertsindices) +- [mgetAlertsAuditOperate](alertsclient.md#mgetalertsauditoperate) - [queryAndAuditAllAlerts](alertsclient.md#queryandauditallalerts) +- [singleSearchAfterAndAudit](alertsclient.md#singlesearchafterandaudit) - [update](alertsclient.md#update) ## Constructors @@ -39,13 +40,13 @@ on alerts as data. #### Parameters -| Name | Type | -| :------------------ | :-------------------------------------------------------- | +| Name | Type | +| :------ | :------ | | `__namedParameters` | [ConstructorOptions](../interfaces/constructoroptions.md) | #### Defined in -[alerts_client.ts:90](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L90) +[alerts_client.ts:93](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L93) ## Properties @@ -55,9 +56,9 @@ on alerts as data. #### Defined in -[alerts_client.ts:87](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L87) +[alerts_client.ts:90](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L90) ---- +___ ### authorization @@ -65,9 +66,9 @@ on alerts as data. #### Defined in -[alerts_client.ts:88](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L88) +[alerts_client.ts:91](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L91) ---- +___ ### esClient @@ -75,9 +76,9 @@ on alerts as data. #### Defined in -[alerts_client.ts:89](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L89) +[alerts_client.ts:92](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L92) ---- +___ ### logger @@ -85,9 +86,9 @@ on alerts as data. #### Defined in -[alerts_client.ts:86](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L86) +[alerts_client.ts:89](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L89) ---- +___ ### spaceId @@ -95,7 +96,7 @@ on alerts as data. #### Defined in -[alerts_client.ts:90](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L90) +[alerts_client.ts:93](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L93) ## Methods @@ -105,13 +106,13 @@ on alerts as data. #### Parameters -| Name | Type | -| :------------- | :-------------------------------- | -| `query` | `undefined` \| `null` \| `string` | -| `id` | `undefined` \| `null` \| `string` | -| `alertSpaceId` | `string` | -| `operation` | `Update` \| `Get` \| `Find` | -| `config` | `EsQueryConfig` | +| Name | Type | +| :------ | :------ | +| `query` | `undefined` \| ``null`` \| `string` | +| `id` | `undefined` \| ``null`` \| `string` | +| `alertSpaceId` | `string` | +| `operation` | `Get` \| `Find` \| `Update` | +| `config` | `EsQueryConfig` | #### Returns @@ -119,9 +120,9 @@ on alerts as data. #### Defined in -[alerts_client.ts:242](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L242) +[alerts_client.ts:305](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L305) ---- +___ ### bulkUpdate @@ -129,14 +130,14 @@ on alerts as data. #### Type parameters -| Name | Type | -| :------- | :------------------------------------ | +| Name | Type | +| :------ | :------ | | `Params` | `Params`: `AlertTypeParams` = `never` | #### Parameters -| Name | Type | -| :------------------ | :--------------------------------------------------------------- | +| Name | Type | +| :------ | :------ | | `__namedParameters` | [BulkUpdateOptions](../interfaces/bulkupdateoptions.md) | #### Returns @@ -145,123 +146,146 @@ on alerts as data. #### Defined in -[alerts_client.ts:424](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L424) +[alerts_client.ts:475](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L475) ---- +___ -### fetchAlertAndAudit +### ensureAllAuthorized -▸ `Private` **fetchAlertAndAudit**(`__namedParameters`): `Promise`\>\>\> +▸ `Private` **ensureAllAuthorized**(`items`, `operation`): `Promise`<(undefined \| void)[]\> -This will be used as a part of the "find" api -In the future we will add an "aggs" param +Accepts an array of ES documents and executes ensureAuthorized for the given operation #### Parameters -| Name | Type | -| :------------------ | :------------------------- | -| `__namedParameters` | `FetchAndAuditAlertParams` | +| Name | Type | +| :------ | :------ | +| `items` | { `_id`: `string` ; `_source?`: ``null`` \| { `kibana.alert.owner?`: ``null`` \| `string` ; `rule.id?`: ``null`` \| `string` } }[] | +| `operation` | `Get` \| `Find` \| `Update` | #### Returns -`Promise`\>\>\> +`Promise`<(undefined \| void)[]\> #### Defined in -[alerts_client.ts:108](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L108) +[alerts_client.ts:111](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L111) ---- +___ -### fetchAlertAuditOperate - -▸ `Private` **fetchAlertAuditOperate**(`__namedParameters`): `Promise`\> +### get -When an update by ids is requested, do a multi-get, ensure authz and audit alerts, then execute bulk update +▸ **get**(`__namedParameters`): `Promise`\>\> #### Parameters -| Name | Type | -| :---------------------------- | :------------------------------------ | -| `__namedParameters` | `Object` | -| `__namedParameters.ids` | `string`[] | -| `__namedParameters.indexName` | `string` | -| `__namedParameters.operation` | `WriteOperations` \| `ReadOperations` | -| `__namedParameters.status` | `STATUS\_VALUES` | +| Name | Type | +| :------ | :------ | +| `__namedParameters` | `GetAlertParams` | #### Returns -`Promise`\> +`Promise`\>\> #### Defined in -[alerts_client.ts:185](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L185) +[alerts_client.ts:407](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L407) ---- +___ -### get +### getAuthorizedAlertsIndices -▸ **get**(`__namedParameters`): `Promise`\>\> +▸ **getAuthorizedAlertsIndices**(`featureIds`): `Promise` #### Parameters -| Name | Type | -| :------------------ | :--------------- | -| `__namedParameters` | `GetAlertParams` | +| Name | Type | +| :------ | :------ | +| `featureIds` | `string`[] | #### Returns -`Promise`\>\> +`Promise` #### Defined in -[alerts_client.ts:344](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L344) +[alerts_client.ts:533](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L533) ---- +___ -### getAuthorizedAlertsIndices +### mgetAlertsAuditOperate -▸ **getAuthorizedAlertsIndices**(`featureIds`): `Promise` +▸ `Private` **mgetAlertsAuditOperate**(`__namedParameters`): `Promise`\> + +When an update by ids is requested, do a multi-get, ensure authz and audit alerts, then execute bulk update #### Parameters -| Name | Type | -| :----------- | :--------- | -| `featureIds` | `string`[] | +| Name | Type | +| :------ | :------ | +| `__namedParameters` | `Object` | +| `__namedParameters.ids` | `string`[] | +| `__namedParameters.indexName` | `string` | +| `__namedParameters.operation` | `Get` \| `Find` \| `Update` | +| `__namedParameters.status` | `STATUS\_VALUES` | #### Returns -`Promise` +`Promise`\> #### Defined in -[alerts_client.ts:481](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L481) +[alerts_client.ts:252](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L252) ---- +___ ### queryAndAuditAllAlerts -▸ `Private` **queryAndAuditAllAlerts**(`__namedParameters`): `Promise` +▸ `Private` **queryAndAuditAllAlerts**(`__namedParameters`): `Promise` executes a search after to find alerts with query (+ authz filter) #### Parameters -| Name | Type | -| :---------------------------- | :-------------------------- | -| `__namedParameters` | `Object` | -| `__namedParameters.index` | `string` | -| `__namedParameters.operation` | `Update` \| `Get` \| `Find` | -| `__namedParameters.query` | `string` | +| Name | Type | +| :------ | :------ | +| `__namedParameters` | `Object` | +| `__namedParameters.index` | `string` | +| `__namedParameters.operation` | `Get` \| `Find` \| `Update` | +| `__namedParameters.query` | `string` | + +#### Returns + +`Promise` + +#### Defined in + +[alerts_client.ts:343](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L343) + +___ + +### singleSearchAfterAndAudit + +▸ `Private` **singleSearchAfterAndAudit**(`__namedParameters`): `Promise`\>\>\> + +This will be used as a part of the "find" api +In the future we will add an "aggs" param + +#### Parameters + +| Name | Type | +| :------ | :------ | +| `__namedParameters` | `SingleSearchAfterAndAudit` | #### Returns -`Promise` +`Promise`\>\>\> #### Defined in -[alerts_client.ts:280](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L280) +[alerts_client.ts:176](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L176) ---- +___ ### update @@ -269,14 +293,14 @@ executes a search after to find alerts with query (+ authz filter) #### Type parameters -| Name | Type | -| :------- | :------------------------------------ | +| Name | Type | +| :------ | :------ | | `Params` | `Params`: `AlertTypeParams` = `never` | #### Parameters -| Name | Type | -| :------------------ | :------------------------------------------------------- | +| Name | Type | +| :------ | :------ | | `__namedParameters` | [UpdateOptions](../interfaces/updateoptions.md) | #### Returns @@ -285,4 +309,4 @@ executes a search after to find alerts with query (+ authz filter) #### Defined in -[alerts_client.ts:375](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L375) +[alerts_client.ts:432](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L432) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md index c01750aaa1b5d..28c49c3519f6e 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/bulkupdateoptions.md @@ -25,7 +25,7 @@ #### Defined in -[alerts_client.ts:61](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L61) +[alerts_client.ts:64](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L64) ___ @@ -35,7 +35,7 @@ ___ #### Defined in -[alerts_client.ts:63](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L63) +[alerts_client.ts:66](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L66) ___ @@ -45,7 +45,7 @@ ___ #### Defined in -[alerts_client.ts:64](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L64) +[alerts_client.ts:67](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L67) ___ @@ -55,4 +55,4 @@ ___ #### Defined in -[alerts_client.ts:62](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L62) +[alerts_client.ts:65](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L65) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md index eae65b9d7df0a..c371719dbced3 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/constructoroptions.md @@ -19,7 +19,7 @@ #### Defined in -[alerts_client.ts:49](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L49) +[alerts_client.ts:52](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L52) ___ @@ -29,7 +29,7 @@ ___ #### Defined in -[alerts_client.ts:48](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L48) +[alerts_client.ts:51](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L51) ___ @@ -39,7 +39,7 @@ ___ #### Defined in -[alerts_client.ts:50](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L50) +[alerts_client.ts:53](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L53) ___ @@ -49,4 +49,4 @@ ___ #### Defined in -[alerts_client.ts:47](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L47) +[alerts_client.ts:50](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L50) diff --git a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md index b462eb2f4c964..f05a061b279d9 100644 --- a/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md +++ b/x-pack/plugins/rule_registry/docs/alerts_client/interfaces/updateoptions.md @@ -25,7 +25,7 @@ #### Defined in -[alerts_client.ts:56](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L56) +[alerts_client.ts:59](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L59) ___ @@ -35,7 +35,7 @@ ___ #### Defined in -[alerts_client.ts:54](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L54) +[alerts_client.ts:57](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L57) ___ @@ -45,7 +45,7 @@ ___ #### Defined in -[alerts_client.ts:57](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L57) +[alerts_client.ts:60](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L60) ___ @@ -55,4 +55,4 @@ ___ #### Defined in -[alerts_client.ts:55](https://github.com/elastic/kibana/blob/84a50dc4bb6/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L55) +[alerts_client.ts:58](https://github.com/elastic/kibana/blob/daf6871ba4b/x-pack/plugins/rule_registry/server/alert_data_client/alerts_client.ts#L58) From ba1857efe6747f0eae50853ac4869a762836e253 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 9 Aug 2021 10:52:42 -0400 Subject: [PATCH 7/9] use jest-specific functions instead of reaching into mock calls for exepcted values --- .../alert_data_client/tests/bulk_update.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts index e739458848310..b26d44b001dcc 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts @@ -209,8 +209,8 @@ describe('bulkUpdate()', () => { status: 'closed', }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized for fake.rule and apm"`); - expect(auditLogger.log.mock.calls).toHaveLength(2); - expect(auditLogger.log.mock.calls[0][0]).toEqual({ + expect(auditLogger.log).toHaveBeenCalledTimes(2); + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`, event: { action: 'alert_update', @@ -223,7 +223,7 @@ describe('bulkUpdate()', () => { message: 'Unauthorized for fake.rule and apm', }, }); - expect(auditLogger.log.mock.calls[1][0]).toEqual({ + expect(auditLogger.log).toHaveBeenNthCalledWith(2, { message: `Failed attempt to update alert [id=${successfulAuthzHit}]`, event: { action: 'alert_update', @@ -422,8 +422,8 @@ describe('bulkUpdate()', () => { Error: Error: Unauthorized for fake.rule and apm" `); - expect(auditLogger.log.mock.calls).toHaveLength(2); - expect(auditLogger.log.mock.calls[0][0]).toEqual({ + expect(auditLogger.log).toHaveBeenCalledTimes(2); + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to update alert [id=${unsuccessfulAuthzHit}]`, event: { action: 'alert_update', @@ -436,7 +436,7 @@ describe('bulkUpdate()', () => { message: 'Unauthorized for fake.rule and apm', }, }); - expect(auditLogger.log.mock.calls[1][0]).toEqual({ + expect(auditLogger.log).toHaveBeenNthCalledWith(2, { message: `Failed attempt to update alert [id=${successfulAuthzHit}]`, event: { action: 'alert_update', From 36838ab2e10ba9b2640665345758373075e0fa44 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 9 Aug 2021 12:04:51 -0400 Subject: [PATCH 8/9] only writes an outcome of 'success' if operation is an access operation, adds jest tests for when user is unauthorized to access given alert --- .../server/alert_data_client/alerts_client.ts | 14 ++- .../alert_data_client/tests/get.test.ts | 94 ++++++++++++++++++- .../alert_data_client/tests/update.test.ts | 90 ++++++++++++++++++ 3 files changed, 191 insertions(+), 7 deletions(-) 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 c4022582ede30..2a7419b20570e 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 @@ -24,7 +24,7 @@ import { WriteOperations, AlertingAuthorizationEntity, } from '../../../alerting/server'; -import { Logger, ElasticsearchClient } from '../../../../../src/core/server'; +import { Logger, ElasticsearchClient, EcsEventOutcome } from '../../../../../src/core/server'; import { alertAuditEvent, operationAlertAuditActionMap } from './audit_events'; import { AuditLogger } from '../../../security/server'; import { @@ -102,6 +102,14 @@ export class AlertsClient { this.spaceId = this.authorization.getSpaceId(); } + private getOutcome( + operation: WriteOperations.Update | ReadOperations.Find | ReadOperations.Get + ): { outcome: EcsEventOutcome } { + return { + outcome: operation === WriteOperations.Update ? 'unknown' : 'success', + }; + } + /** * Accepts an array of ES documents and executes ensureAuthorized for the given operation * @param items @@ -231,7 +239,7 @@ export class AlertsClient { alertAuditEvent({ action: operationAlertAuditActionMap[operation], id: item._id, - outcome: 'unknown', + ...this.getOutcome(operation), }) ) ); @@ -275,7 +283,7 @@ export class AlertsClient { alertAuditEvent({ action: operationAlertAuditActionMap[operation], id, - ...(operation === WriteOperations.Update ? { outcome: 'unknown' } : { operation }), + ...this.getOutcome(operation), }) ); } 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 8b9ea944dc354..7bd1ab3e75271 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 @@ -5,13 +5,14 @@ * 2.0. */ -import { ALERT_OWNER, ALERT_STATUS, SPACE_IDS } from '@kbn/rule-data-utils'; +import { ALERT_OWNER, ALERT_STATUS, RULE_ID, SPACE_IDS } from '@kbn/rule-data-utils'; 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 { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; import { AuditLogger } from '../../../../security/server'; +import { AlertingAuthorizationEntity } from '../../../../alerting/server'; const alertingAuthMock = alertingAuthorizationMock.create(); const esClientMock = elasticsearchClientMock.createElasticsearchClient(); @@ -26,13 +27,35 @@ const alertsClientParams: jest.Mocked = { auditLogger, }; +const DEFAULT_SPACE = 'test_default_space_id'; + beforeEach(() => { jest.resetAllMocks(); - alertingAuthMock.getSpaceId.mockImplementation(() => 'test_default_space_id'); + alertingAuthMock.getSpaceId.mockImplementation(() => DEFAULT_SPACE); // @ts-expect-error alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => Promise.resolve({ filter: [] }) ); + + alertingAuthMock.ensureAuthorized.mockImplementation( + // @ts-expect-error + async ({ + ruleTypeId, + consumer, + operation, + entity, + }: { + ruleTypeId: string; + consumer: string; + operation: string; + entity: typeof AlertingAuthorizationEntity.Alert; + }) => { + if (ruleTypeId === 'apm.error_rate' && consumer === 'apm') { + return Promise.resolve(); + } + return Promise.reject(new Error(`Unauthorized for ${ruleTypeId} and ${consumer}`)); + } + ); }); describe('get()', () => { @@ -177,8 +200,71 @@ describe('get()', () => { expect(auditLogger.log).toHaveBeenCalledWith({ error: undefined, - event: { action: 'alert_get', category: ['database'], outcome: 'unknown', type: ['access'] }, - message: 'User is accessing alert [id=NoxgpHkBqbdrfX07MqXV]', + event: { action: 'alert_get', category: ['database'], outcome: 'success', type: ['access'] }, + message: 'User has accessed alert [id=NoxgpHkBqbdrfX07MqXV]', + }); + }); + + test('audit error access if user is unauthorized for given alert', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const fakeAlertId = 'myfakeid1'; + // fakeRuleTypeId will cause authz to fail + const fakeRuleTypeId = 'fake.rule'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + found: true, + _type: 'alert', + _version: 1, + _seq_no: 362, + _primary_term: 2, + _id: fakeAlertId, + _index: indexName, + _source: { + [RULE_ID]: fakeRuleTypeId, + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + + await expect(alertsClient.get({ id: fakeAlertId, index: '.alerts-observability-apm' })).rejects + .toThrowErrorMatchingInlineSnapshot(` + "Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"null\\" and operation get + Error: Error: Unauthorized for fake.rule and apm" + `); + + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to access alert [id=${fakeAlertId}]`, + event: { + action: 'alert_get', + category: ['database'], + outcome: 'failure', + type: ['access'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, }); }); 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 index 63b97dad4cb3f..9d786a73281c0 100644 --- 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 @@ -12,6 +12,7 @@ import { loggingSystemMock } from '../../../../../../src/core/server/mocks'; import { elasticsearchClientMock } from 'src/core/server/elasticsearch/client/mocks'; import { alertingAuthorizationMock } from '../../../../alerting/server/authorization/alerting_authorization.mock'; import { AuditLogger } from '../../../../security/server'; +import { AlertingAuthorizationEntity } from '../../../../alerting/server'; const alertingAuthMock = alertingAuthorizationMock.create(); const esClientMock = elasticsearchClientMock.createElasticsearchClient(); @@ -35,6 +36,26 @@ beforeEach(() => { alertingAuthMock.getAuthorizationFilter.mockImplementation(async () => Promise.resolve({ filter: [] }) ); + + alertingAuthMock.ensureAuthorized.mockImplementation( + // @ts-expect-error + async ({ + ruleTypeId, + consumer, + operation, + entity, + }: { + ruleTypeId: string; + consumer: string; + operation: string; + entity: typeof AlertingAuthorizationEntity.Alert; + }) => { + if (ruleTypeId === 'apm.error_rate' && consumer === 'apm') { + return Promise.resolve(); + } + return Promise.reject(new Error(`Unauthorized for ${ruleTypeId} and ${consumer}`)); + } + ); }); describe('update()', () => { @@ -191,6 +212,75 @@ describe('update()', () => { }); }); + test('audit error update if user is unauthorized for given alert', async () => { + const indexName = '.alerts-observability-apm.alerts'; + const fakeAlertId = 'myfakeid1'; + // fakeRuleTypeId will cause authz to fail + const fakeRuleTypeId = 'fake.rule'; + const alertsClient = new AlertsClient(alertsClientParams); + esClientMock.search.mockResolvedValueOnce( + elasticsearchClientMock.createApiResponse({ + body: { + took: 5, + timed_out: false, + _shards: { + total: 1, + successful: 1, + failed: 0, + skipped: 0, + }, + hits: { + total: 1, + max_score: 999, + hits: [ + { + found: true, + _type: 'alert', + _version: 1, + _seq_no: 362, + _primary_term: 2, + _id: fakeAlertId, + _index: indexName, + _source: { + [RULE_ID]: fakeRuleTypeId, + [ALERT_OWNER]: 'apm', + [ALERT_STATUS]: 'open', + [SPACE_IDS]: [DEFAULT_SPACE], + }, + }, + ], + }, + }, + }) + ); + + await expect( + alertsClient.update({ + id: fakeAlertId, + status: 'closed', + _version: '1', + index: '.alerts-observability-apm', + }) + ).rejects.toThrowErrorMatchingInlineSnapshot(` + "Unable to retrieve alert details for alert with id of \\"myfakeid1\\" or with query \\"null\\" and operation update + Error: Error: Unauthorized for fake.rule and apm" + `); + + expect(auditLogger.log).toHaveBeenLastCalledWith({ + message: `Failed attempt to update alert [id=${fakeAlertId}]`, + event: { + action: 'alert_update', + category: ['database'], + outcome: 'failure', + type: ['change'], + }, + error: { + code: 'Error', + message: 'Unauthorized for fake.rule and apm', + }, + }); + }); + test(`throws an error if ES client get fails`, async () => { const error = new Error('something went wrong on update'); const alertsClient = new AlertsClient(alertsClientParams); From 04044f564b1dbdf7c0f33882258dc29d14d0d5e6 Mon Sep 17 00:00:00 2001 From: Devin Hurley Date: Mon, 9 Aug 2021 12:07:56 -0400 Subject: [PATCH 9/9] replaces toHaveBeenLastCalledWith with toHaveBeenNthCalledWith --- .../server/alert_data_client/tests/bulk_update.test.ts | 6 +++--- .../server/alert_data_client/tests/get.test.ts | 2 +- .../server/alert_data_client/tests/update.test.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts index b26d44b001dcc..97a19935fa787 100644 --- a/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts +++ b/x-pack/plugins/rule_registry/server/alert_data_client/tests/bulk_update.test.ts @@ -111,7 +111,7 @@ describe('bulkUpdate()', () => { index: indexName, status: 'closed', }); - expect(auditLogger.log).toHaveBeenLastCalledWith({ + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `User is updating alert [id=${fakeAlertId}]`, event: { action: 'alert_update', @@ -154,7 +154,7 @@ describe('bulkUpdate()', () => { }) ).rejects.toThrowErrorMatchingInlineSnapshot(`"Unauthorized for fake.rule and apm"`); - expect(auditLogger.log).toHaveBeenLastCalledWith({ + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to update alert [id=${fakeAlertId}]`, event: { action: 'alert_update', @@ -351,7 +351,7 @@ describe('bulkUpdate()', () => { Error: Error: Unauthorized for fake.rule and apm" `); - expect(auditLogger.log).toHaveBeenLastCalledWith({ + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to update alert [id=${fakeAlertId}]`, event: { action: 'alert_update', 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 7bd1ab3e75271..651d728b1983c 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 @@ -253,7 +253,7 @@ describe('get()', () => { Error: Error: Unauthorized for fake.rule and apm" `); - expect(auditLogger.log).toHaveBeenLastCalledWith({ + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to access alert [id=${fakeAlertId}]`, event: { action: 'alert_get', 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 index 9d786a73281c0..435b6e310ffdf 100644 --- 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 @@ -266,7 +266,7 @@ describe('update()', () => { Error: Error: Unauthorized for fake.rule and apm" `); - expect(auditLogger.log).toHaveBeenLastCalledWith({ + expect(auditLogger.log).toHaveBeenNthCalledWith(1, { message: `Failed attempt to update alert [id=${fakeAlertId}]`, event: { action: 'alert_update',