From 956612d071a9b9a1567ed997db15c69aba7433f3 Mon Sep 17 00:00:00 2001 From: Xavier Mouligneau Date: Wed, 4 May 2022 11:18:05 -0400 Subject: [PATCH] [RAM] Bug api find/index alerts (#131338) * fix bug * fix unit test * bing back tests alive after a long CPR * fix test and bring back recursive aggs * I need to do an intersectiona and not union * fix last integration test --- .../authorization/alerting_authorization.ts | 1 + x-pack/plugins/rule_registry/common/types.ts | 270 ++++++++++-------- .../server/alert_data_client/alerts_client.ts | 14 +- .../rule_registry/server/routes/find.ts | 8 +- .../rule_data_plugin_service.mock.ts | 2 +- .../rule_data_plugin_service.ts | 11 +- .../search_strategy/search_strategy.test.ts | 26 +- .../server/search_strategy/search_strategy.ts | 21 +- .../alerting.test.ts | 36 ++- .../feature_privilege_builder/alerting.ts | 2 +- .../security_solution/server/features.ts | 6 + .../rule_registry/alerts/data.json | 8 +- .../tests/basic/find_alerts.ts | 78 ++--- .../tests/basic/get_alerts_index.ts | 48 ++-- .../security_and_spaces/tests/basic/index.ts | 5 +- .../tests/trial/get_alert_by_id.ts | 16 +- .../spaces_only/tests/trial/update_alert.ts | 16 +- 17 files changed, 305 insertions(+), 263 deletions(-) diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts index b9acfed7dcc65..26aa7a706db0a 100644 --- a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts +++ b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts @@ -32,6 +32,7 @@ export enum ReadOperations { GetAlertSummary = 'getAlertSummary', GetExecutionLog = 'getExecutionLog', Find = 'find', + GetAuthorizedAlertsIndices = 'getAuthorizedAlertsIndices', } export enum WriteOperations { diff --git a/x-pack/plugins/rule_registry/common/types.ts b/x-pack/plugins/rule_registry/common/types.ts index 4bf5fa8b23fdc..d1c5b6706c391 100644 --- a/x-pack/plugins/rule_registry/common/types.ts +++ b/x-pack/plugins/rule_registry/common/types.ts @@ -75,9 +75,8 @@ interface BucketAggsSchemas { | { [x: string]: SortOrderSchema } | Array<{ [x: string]: SortOrderSchema }>; }; - aggs?: { - [x: string]: BucketAggsSchemas; - }; + aggs?: BucketAggsSchemas; + aggregations?: BucketAggsSchemas; } /** @@ -114,78 +113,83 @@ interface BucketAggsSchemas { * - significant_text * - variable_width_histogram */ -export const BucketAggsSchemas: t.Type = t.recursion('BucketAggsSchemas', () => - t.exact( - t.partial({ - filter: t.exact( - t.partial({ - term: t.record(t.string, t.union([t.string, t.boolean, t.number])), - }) - ), - date_histogram: t.exact( - t.partial({ - field: t.string, - fixed_interval: t.string, - min_doc_count: t.number, - extended_bounds: t.type({ - min: t.string, - max: t.string, - }), - }) - ), - histogram: t.exact( - t.partial({ - field: t.string, - interval: t.number, - min_doc_count: t.number, - extended_bounds: t.exact( - t.type({ - min: t.number, - max: t.number, - }) - ), - hard_bounds: t.exact( - t.type({ - min: t.number, - max: t.number, - }) - ), - missing: t.number, - keyed: t.boolean, - order: t.exact( - t.type({ - _count: t.string, - _key: t.string, - }) - ), - }) - ), - nested: t.type({ - path: t.string, - }), - terms: t.exact( - t.partial({ - field: t.string, - collect_mode: t.string, - exclude: t.union([t.string, t.array(t.string)]), - include: t.union([t.string, t.array(t.string)]), - execution_hint: t.string, - missing: t.union([t.number, t.string]), - min_doc_count: t.number, - size: t.number, - show_term_doc_count_error: t.boolean, - order: t.union([ - sortOrderSchema, - t.record(t.string, sortOrderSchema), - t.array(t.record(t.string, sortOrderSchema)), - ]), - }) - ), - aggs: t.record(t.string, BucketAggsSchemas), - }) - ) +const bucketAggsTempsSchemas: t.Type = t.exact( + t.partial({ + filter: t.exact( + t.partial({ + term: t.record(t.string, t.union([t.string, t.boolean, t.number])), + }) + ), + date_histogram: t.exact( + t.partial({ + field: t.string, + fixed_interval: t.string, + min_doc_count: t.number, + extended_bounds: t.type({ + min: t.string, + max: t.string, + }), + }) + ), + histogram: t.exact( + t.partial({ + field: t.string, + interval: t.number, + min_doc_count: t.number, + extended_bounds: t.exact( + t.type({ + min: t.number, + max: t.number, + }) + ), + hard_bounds: t.exact( + t.type({ + min: t.number, + max: t.number, + }) + ), + missing: t.number, + keyed: t.boolean, + order: t.exact( + t.type({ + _count: t.string, + _key: t.string, + }) + ), + }) + ), + nested: t.type({ + path: t.string, + }), + terms: t.exact( + t.partial({ + field: t.string, + collect_mode: t.string, + exclude: t.union([t.string, t.array(t.string)]), + include: t.union([t.string, t.array(t.string)]), + execution_hint: t.string, + missing: t.union([t.number, t.string]), + min_doc_count: t.number, + size: t.number, + show_term_doc_count_error: t.boolean, + order: t.union([ + sortOrderSchema, + t.record(t.string, sortOrderSchema), + t.array(t.record(t.string, sortOrderSchema)), + ]), + }) + ), + }) ); +export const bucketAggsSchemas = t.intersection([ + bucketAggsTempsSchemas, + t.partial({ + aggs: t.union([t.record(t.string, bucketAggsTempsSchemas), t.undefined]), + aggregations: t.union([t.record(t.string, bucketAggsTempsSchemas), t.undefined]), + }), +]); + /** * Schemas for the metrics Aggregations * @@ -215,57 +219,75 @@ export const BucketAggsSchemas: t.Type = t.recursion('BucketA * - t_test * - value_count */ -export const metricsAggsSchemas = t.partial({ - avg: t.partial({ - field: t.string, - missing: t.union([t.string, t.number, t.boolean]), - }), - cardinality: t.partial({ - field: t.string, - precision_threshold: t.number, - rehash: t.boolean, - missing: t.union([t.string, t.number, t.boolean]), - }), - min: t.partial({ - field: t.string, - missing: t.union([t.string, t.number, t.boolean]), - format: t.string, - }), - max: t.partial({ - field: t.string, - missing: t.union([t.string, t.number, t.boolean]), - format: t.string, - }), - sum: t.partial({ - field: t.string, - missing: t.union([t.string, t.number, t.boolean]), - }), - top_hits: t.partial({ - explain: t.boolean, - docvalue_fields: t.union([t.string, t.array(t.string)]), - stored_fields: t.union([t.string, t.array(t.string)]), - from: t.number, - size: t.number, - sort: sortSchema, - seq_no_primary_term: t.boolean, - version: t.boolean, - track_scores: t.boolean, - highlight: t.any, - _source: t.union([t.boolean, t.string, t.array(t.string)]), - }), - weighted_avg: t.partial({ - format: t.string, - value_type: t.string, - value: t.partial({ - field: t.string, - missing: t.number, - }), - weight: t.partial({ - field: t.string, - missing: t.number, - }), - }), -}); +export const metricsAggsSchemas = t.exact( + t.partial({ + avg: t.exact( + t.partial({ + field: t.string, + missing: t.union([t.string, t.number, t.boolean]), + }) + ), + cardinality: t.exact( + t.partial({ + field: t.string, + precision_threshold: t.number, + rehash: t.boolean, + missing: t.union([t.string, t.number, t.boolean]), + }) + ), + min: t.exact( + t.partial({ + field: t.string, + missing: t.union([t.string, t.number, t.boolean]), + format: t.string, + }) + ), + max: t.exact( + t.partial({ + field: t.string, + missing: t.union([t.string, t.number, t.boolean]), + format: t.string, + }) + ), + sum: t.exact( + t.partial({ + field: t.string, + missing: t.union([t.string, t.number, t.boolean]), + }) + ), + top_hits: t.exact( + t.partial({ + explain: t.boolean, + docvalue_fields: t.union([t.string, t.array(t.string)]), + stored_fields: t.union([t.string, t.array(t.string)]), + from: t.number, + size: t.number, + sort: sortSchema, + seq_no_primary_term: t.boolean, + version: t.boolean, + track_scores: t.boolean, + highlight: t.any, + _source: t.union([t.boolean, t.string, t.array(t.string)]), + }) + ), + weighted_avg: t.exact( + t.partial({ + format: t.string, + value_type: t.string, + value: t.partial({ + field: t.string, + missing: t.number, + }), + weight: t.partial({ + field: t.string, + missing: t.number, + }), + }) + ), + aggs: t.undefined, + aggregations: t.undefined, + }) +); export type PutIndexTemplateRequest = estypes.IndicesPutIndexTemplateRequest & { body?: { composed_of?: string[] }; 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 b5df5dc830d48..effd2f4f81b89 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 @@ -647,7 +647,6 @@ export class AlertsClient { [ReadOperations.Find, ReadOperations.Get, WriteOperations.Update], AlertingAuthorizationEntity.Alert ); - // 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 @@ -655,19 +654,16 @@ export class AlertsClient { for (const ruleType of augmentedRuleTypes.authorizedRuleTypes) { authorizedFeatures.add(ruleType.producer); } - const validAuthorizedFeatures = Array.from(authorizedFeatures).filter( (feature): feature is ValidFeatureId => featureIds.includes(feature) && isValidFeatureId(feature) ); - - const toReturn = validAuthorizedFeatures.flatMap((feature) => { - const indices = this.ruleDataService.findIndicesByFeature(feature, Dataset.alerts); - if (feature === 'siem') { - return indices.map((i) => `${i.baseName}-${this.spaceId}`); - } else { - return indices.map((i) => i.baseName); + const toReturn = validAuthorizedFeatures.map((feature) => { + const index = this.ruleDataService.findIndexByFeature(feature, Dataset.alerts); + if (index == null) { + throw new Error(`This feature id ${feature} should be associated to an alert index`); } + return index?.getPrimaryAlias(this.spaceId ?? '*') ?? ''; }); return toReturn; diff --git a/x-pack/plugins/rule_registry/server/routes/find.ts b/x-pack/plugins/rule_registry/server/routes/find.ts index eca0a6c2a0551..675037baa312a 100644 --- a/x-pack/plugins/rule_registry/server/routes/find.ts +++ b/x-pack/plugins/rule_registry/server/routes/find.ts @@ -14,7 +14,7 @@ import { PositiveInteger } from '@kbn/securitysolution-io-ts-types'; import { RacRequestHandlerContext } from '../types'; import { BASE_RAC_ALERTS_API_PATH } from '../../common/constants'; import { buildRouteValidation } from './utils/route_validation'; -import { BucketAggsSchemas } from '../../common/types'; +import { bucketAggsSchemas, metricsAggsSchemas } from '../../common/types'; export const findAlertsByQueryRoute = (router: IRouter) => { router.post( @@ -26,7 +26,11 @@ export const findAlertsByQueryRoute = (router: IRouter t.partial({ index: t.string, query: t.object, - aggs: t.union([t.record(t.string, BucketAggsSchemas), t.undefined]), + aggs: t.union([ + t.record(t.string, bucketAggsSchemas), + t.record(t.string, metricsAggsSchemas), + t.undefined, + ]), size: t.union([PositiveInteger, t.undefined]), track_total_hits: t.union([t.boolean, t.undefined]), _source: t.union([t.array(t.string), t.undefined]), diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts index 8bbc14cab9f82..cfbfafd0092bf 100644 --- a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts +++ b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.mock.ts @@ -16,7 +16,7 @@ export const ruleDataServiceMock = { initializeService: jest.fn(), initializeIndex: jest.fn(), findIndexByName: jest.fn(), - findIndicesByFeature: jest.fn(), + findIndexByFeature: jest.fn(), }), }; diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts index a940201841e6f..a336f73c87c79 100644 --- a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts +++ b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/rule_data_plugin_service.ts @@ -71,7 +71,7 @@ export interface IRuleDataService { * Looks up the index information associated with the given Kibana "feature". * Note: features are used in RBAC. */ - findIndicesByFeature(featureId: ValidFeatureId, dataset?: Dataset): IndexInfo[]; + findIndexByFeature(featureId: ValidFeatureId, dataset: Dataset): IndexInfo | null; } // TODO: This is a leftover. Remove its usage from the "observability" plugin and delete it. @@ -214,8 +214,13 @@ export class RuleDataService implements IRuleDataService { return this.indicesByBaseName.get(baseName) ?? null; } - public findIndicesByFeature(featureId: ValidFeatureId, dataset?: Dataset): IndexInfo[] { + public findIndexByFeature(featureId: ValidFeatureId, dataset: Dataset): IndexInfo | null { const foundIndices = this.indicesByFeatureId.get(featureId) ?? []; - return dataset ? foundIndices.filter((i) => i.indexOptions.dataset === dataset) : foundIndices; + if (dataset && foundIndices.length > 0) { + return foundIndices.filter((i) => i.indexOptions.dataset === dataset)[0]; + } else if (foundIndices.length > 0) { + return foundIndices[0]; + } + return null; } } diff --git a/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.test.ts b/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.test.ts index 79b9a8cd75c58..9bfc4d7a40640 100644 --- a/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.test.ts +++ b/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.test.ts @@ -78,12 +78,10 @@ describe('ruleRegistrySearchStrategyProvider()', () => { const searchStrategySearch = jest.fn().mockImplementation(() => of(response)); beforeEach(() => { - ruleDataService.findIndicesByFeature.mockImplementation(() => { - return [ - { - baseName: 'test', - } as IndexInfo, - ]; + ruleDataService.findIndexByFeature.mockImplementation(() => { + return { + baseName: 'test', + } as IndexInfo; }); data.search.getSearchStrategy.mockImplementation(() => { @@ -108,7 +106,7 @@ describe('ruleRegistrySearchStrategyProvider()', () => { }); afterEach(() => { - ruleDataService.findIndicesByFeature.mockClear(); + ruleDataService.findIndexByFeature.mockClear(); data.search.getSearchStrategy.mockClear(); (data.search.searchAsInternalUser.search as jest.Mock).mockClear(); getAuthzFilterSpy.mockClear(); @@ -156,12 +154,10 @@ describe('ruleRegistrySearchStrategyProvider()', () => { }; }); - ruleDataService.findIndicesByFeature.mockImplementation(() => { - return [ - { - baseName: 'myTestIndex', - } as unknown as IndexInfo, - ]; + ruleDataService.findIndexByFeature.mockImplementation(() => { + return { + baseName: 'myTestIndex', + } as unknown as IndexInfo; }); let searchRequest: RuleRegistrySearchRequest = {} as unknown as RuleRegistrySearchRequest; @@ -199,8 +195,8 @@ describe('ruleRegistrySearchStrategyProvider()', () => { request: {}, }; - ruleDataService.findIndicesByFeature.mockImplementationOnce(() => { - return []; + ruleDataService.findIndexByFeature.mockImplementationOnce(() => { + return null; }); const strategy = ruleRegistrySearchStrategyProvider( diff --git a/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.ts b/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.ts index 851ffbdc098da..255af29a9a9d3 100644 --- a/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.ts +++ b/x-pack/plugins/rule_registry/server/search_strategy/search_strategy.ts @@ -89,17 +89,16 @@ export const ruleRegistrySearchStrategyProvider = ( ); return accum; } - - return [ - ...accum, - ...ruleDataService - .findIndicesByFeature(featureId, Dataset.alerts) - .map((indexInfo) => { - return featureId === 'siem' - ? `${indexInfo.baseName}-${space?.id ?? ''}*` - : `${indexInfo.baseName}*`; - }), - ]; + const alertIndexInfo = ruleDataService.findIndexByFeature(featureId, Dataset.alerts); + if (alertIndexInfo) { + return [ + ...accum, + featureId === 'siem' + ? `${alertIndexInfo.baseName}-${space?.id ?? ''}*` + : `${alertIndexInfo.baseName}*`, + ]; + } + return accum; }, []); if (indices.length === 0) { diff --git a/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.test.ts b/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.test.ts index 8596f80d09d46..c4da245f48fcd 100644 --- a/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.test.ts +++ b/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.test.ts @@ -128,6 +128,7 @@ describe(`feature_privilege_builder`, () => { Array [ "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/getAuthorizedAlertsIndices", ] `); }); @@ -175,6 +176,7 @@ describe(`feature_privilege_builder`, () => { "alerting:1.0.0-zeta1:alert-type/my-feature/rule/find", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/getAuthorizedAlertsIndices", ] `); }); @@ -263,12 +265,13 @@ describe(`feature_privilege_builder`, () => { }); expect(alertingFeaturePrivileges.getActions(privilege, feature)).toMatchInlineSnapshot(` - Array [ - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/update", - ] - `); + Array [ + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/getAuthorizedAlertsIndices", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/update", + ] + `); }); test('grants `all` privileges to rules and alerts under feature consumer', () => { @@ -326,6 +329,7 @@ describe(`feature_privilege_builder`, () => { "alerting:1.0.0-zeta1:alert-type/my-feature/rule/unsnooze", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/getAuthorizedAlertsIndices", "alerting:1.0.0-zeta1:alert-type/my-feature/alert/update", ] `); @@ -420,14 +424,16 @@ describe(`feature_privilege_builder`, () => { }); expect(alertingFeaturePrivileges.getActions(privilege, feature)).toMatchInlineSnapshot(` - Array [ - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", - "alerting:1.0.0-zeta1:alert-type/my-feature/alert/update", - "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/get", - "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/find", - ] - `); + Array [ + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/get", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/getAuthorizedAlertsIndices", + "alerting:1.0.0-zeta1:alert-type/my-feature/alert/update", + "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/get", + "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/getAuthorizedAlertsIndices", + ] + `); }); test('grants both `all` and `read` to rules and alerts privileges under feature consumer', () => { @@ -490,9 +496,11 @@ describe(`feature_privilege_builder`, () => { "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/rule/find", "alerting:1.0.0-zeta1:another-alert-type/my-feature/alert/get", "alerting:1.0.0-zeta1:another-alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:another-alert-type/my-feature/alert/getAuthorizedAlertsIndices", "alerting:1.0.0-zeta1:another-alert-type/my-feature/alert/update", "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/get", "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/find", + "alerting:1.0.0-zeta1:readonly-alert-type/my-feature/alert/getAuthorizedAlertsIndices", ] `); }); diff --git a/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.ts b/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.ts index c23a989951f52..3a692e935cf37 100644 --- a/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.ts +++ b/x-pack/plugins/security/server/authorization/privileges/feature_privilege_builder/alerting.ts @@ -18,7 +18,7 @@ enum AlertingEntity { const readOperations: Record = { rule: ['get', 'getRuleState', 'getAlertSummary', 'getExecutionLog', 'find'], - alert: ['get', 'find'], + alert: ['get', 'find', 'getAuthorizedAlertsIndices'], }; const writeOperations: Record = { diff --git a/x-pack/plugins/security_solution/server/features.ts b/x-pack/plugins/security_solution/server/features.ts index 340c4c54c5fc6..375f66a36c3ac 100644 --- a/x-pack/plugins/security_solution/server/features.ts +++ b/x-pack/plugins/security_solution/server/features.ts @@ -133,6 +133,9 @@ export const getKibanaPrivilegesFeaturePrivileges = (ruleTypes: string[]): Kiban rule: { all: ruleTypes, }, + alert: { + all: ruleTypes, + }, }, management: { insightsAndAlerting: ['triggersActions'], @@ -156,6 +159,9 @@ export const getKibanaPrivilegesFeaturePrivileges = (ruleTypes: string[]): Kiban rule: { read: ruleTypes, }, + alert: { + all: ruleTypes, + }, }, management: { insightsAndAlerting: ['triggersActions'], 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 284c6e9519cc1..9c8d233e02074 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 @@ -57,7 +57,7 @@ "source": { "event.kind" : "signal", "@timestamp": "2020-12-16T15:16:18.570Z", - "kibana.alert.rule.rule_type_id": "siem.signals", + "kibana.alert.rule.rule_type_id": "siem.queryRule", "message": "hello world security", "kibana.alert.rule.consumer": "siem", "kibana.alert.workflow_status": "open", @@ -74,7 +74,7 @@ "source": { "event.kind" : "signal", "@timestamp": "2020-12-16T15:16:18.570Z", - "kibana.alert.rule.rule_type_id": "siem.customRule", + "kibana.alert.rule.rule_type_id": "siem.queryRule", "message": "hello world security", "kibana.alert.rule.consumer": "siem", "kibana.alert.workflow_status": "open", @@ -90,7 +90,7 @@ "id": "space1securityalert", "source": { "@timestamp": "2020-12-16T15:16:18.570Z", - "kibana.alert.rule.rule_type_id": "siem.signals", + "kibana.alert.rule.rule_type_id": "siem.queryRule", "message": "hello world security", "kibana.alert.rule.consumer": "siem", "kibana.alert.workflow_status": "open", @@ -106,7 +106,7 @@ "id": "space2securityalert", "source": { "@timestamp": "2020-12-16T15:16:18.570Z", - "kibana.alert.rule.rule_type_id": "siem.signals", + "kibana.alert.rule.rule_type_id": "siem.queryRule", "message": "hello world security", "kibana.alert.rule.consumer": "siem", "kibana.alert.workflow_status": "open", diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/find_alerts.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/find_alerts.ts index ef06fc80c485c..35461b47f6def 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/find_alerts.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/find_alerts.ts @@ -50,7 +50,6 @@ export default ({ getService }: FtrProviderContext) => { 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'; @@ -58,38 +57,7 @@ export default ({ getService }: FtrProviderContext) => { const SECURITY_SOLUTION_ALERT_ID = '020202'; const SECURITY_SOLUTION_ALERT_INDEX = '.alerts-security.alerts'; - 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.startsWith(SECURITY_SOLUTION_ALERT_INDEX) - ); - expect(securitySolution).to.eql(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`); // assert this here so we can use constants in the dynamically-defined test cases below - }; - describe('Alert - Find - RBAC - spaces', () => { - before(async () => { - await getSecuritySolutionIndexName(superUser); - await getAPMIndexName(superUser); - }); - before(async () => { await esArchiver.load('x-pack/test/functional/es_archives/rule_registry/alerts'); }); @@ -98,6 +66,32 @@ export default ({ getService }: FtrProviderContext) => { await esArchiver.unload('x-pack/test/functional/es_archives/rule_registry/alerts'); }); + it(`${superUser.username} should reject at route level when aggs contains script alerts which match query in ${SPACE1}/${SECURITY_SOLUTION_ALERT_INDEX}`, async () => { + const found = await supertestWithoutAuth + .post(`${getSpaceUrlPrefix(SPACE1)}${TEST_URL}/find`) + .auth(superUser.username, superUser.password) + .set('kbn-xsrf', 'true') + .send({ + query: { match: { [ALERT_WORKFLOW_STATUS]: 'open' } }, + aggs: { + alertsByGroupingCount: { + terms: { + field: 'kibana.alert.rule.name', + order: { + _count: 'desc', + }, + script: { + source: 'SCRIPT', + }, + size: 10000, + }, + }, + }, + index: SECURITY_SOLUTION_ALERT_INDEX, + }); + expect(found.statusCode).to.eql(400); + }); + it(`${superUser.username} should reject at route level when nested aggs contains script alerts which match query in ${SPACE1}/${SECURITY_SOLUTION_ALERT_INDEX}`, async () => { const found = await supertestWithoutAuth .post(`${getSpaceUrlPrefix(SPACE1)}${TEST_URL}/find`) @@ -164,6 +158,26 @@ export default ({ getService }: FtrProviderContext) => { expect(found.body.hits.total.value).to.be.above(0); }); + it(`${superUser.username} should allow cardinality aggs in ${SPACE1}/${SECURITY_SOLUTION_ALERT_INDEX}`, async () => { + const found = await supertestWithoutAuth + .post(`${getSpaceUrlPrefix(SPACE1)}${TEST_URL}/find`) + .auth(superUser.username, superUser.password) + .set('kbn-xsrf', 'true') + .send({ + size: 1, + aggs: { + nbr_consumer: { + cardinality: { + field: 'kibana.alert.rule.consumer', + }, + }, + }, + index: '.alerts*', + }); + expect(found.statusCode).to.eql(200); + expect(found.body.aggregations.nbr_consumer.value).to.be.equal(2); + }); + function addTests({ space, authorizedUsers, unauthorizedUsers, alertId, index }: TestCase) { authorizedUsers.forEach(({ username, password }) => { it(`${username} should finds alerts which match query in ${space}/${index}`, async () => { diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alerts_index.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alerts_index.ts index 1ba48d5d1aa27..3b30f1b64dc2b 100644 --- a/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alerts_index.ts +++ b/x-pack/test/rule_registry/security_and_spaces/tests/basic/get_alerts_index.ts @@ -20,17 +20,16 @@ export default ({ getService }: FtrProviderContext) => { const TEST_URL = '/internal/rac/alerts'; const ALERTS_INDEX_URL = `${TEST_URL}/index`; const SPACE1 = 'space1'; - const APM_ALERT_INDEX = '.alerts-observability-apm'; + const APM_ALERT_INDEX = '.alerts-observability.apm.alerts'; const SECURITY_SOLUTION_ALERT_INDEX = '.alerts-security.alerts'; - const getAPMIndexName = async (user: User, space: string, expected: number = 200) => { - const { body: indexNames }: { body: { index_name: string[] | undefined } } = - await supertestWithoutAuth - .get(`${getSpaceUrlPrefix(space)}${ALERTS_INDEX_URL}?features=apm`) - .auth(user.username, user.password) - .set('kbn-xsrf', 'true') - .expect(expected); - return indexNames; + const getAPMIndexName = async (user: User, space: string, expectedStatusCode: number = 200) => { + const resp = await supertestWithoutAuth + .get(`${getSpaceUrlPrefix(space)}${ALERTS_INDEX_URL}?features=apm`) + .auth(user.username, user.password) + .set('kbn-xsrf', 'true') + .expect(expectedStatusCode); + return resp.body.index_name as string[]; }; const getSecuritySolutionIndexName = async ( @@ -38,13 +37,13 @@ export default ({ getService }: FtrProviderContext) => { space: string, expectedStatusCode: number = 200 ) => { - const { body: indexNames }: { body: { index_name: string[] | undefined } } = - await supertestWithoutAuth - .get(`${getSpaceUrlPrefix(space)}${ALERTS_INDEX_URL}?features=siem`) - .auth(user.username, user.password) - .set('kbn-xsrf', 'true') - .expect(expectedStatusCode); - return indexNames; + const resp = await supertestWithoutAuth + .get(`${getSpaceUrlPrefix(space)}${ALERTS_INDEX_URL}?features=siem`) + .auth(user.username, user.password) + .set('kbn-xsrf', 'true') + .expect(expectedStatusCode); + + return resp.body.index_name as string[]; }; describe('Alert - Get Index - RBAC - spaces', () => { @@ -54,31 +53,22 @@ export default ({ getService }: FtrProviderContext) => { describe('Users:', () => { it(`${obsOnlySpacesAll.username} should be able to access the APM alert in ${SPACE1}`, async () => { const indexNames = await getAPMIndexName(obsOnlySpacesAll, SPACE1); - 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 + expect(indexNames.includes(`${APM_ALERT_INDEX}-${SPACE1}`)).to.eql(true); // assert this here so we can use constants in the dynamically-defined test cases below }); it(`${superUser.username} should be able to access the APM alert in ${SPACE1}`, async () => { const indexNames = await getAPMIndexName(superUser, SPACE1); - 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 + expect(indexNames.includes(`${APM_ALERT_INDEX}-${SPACE1}`)).to.eql(true); // assert this here so we can use constants in the dynamically-defined test cases below }); it(`${secOnlyRead.username} should NOT be able to access the APM alert in ${SPACE1}`, async () => { const indexNames = await getAPMIndexName(secOnlyRead, SPACE1); - expect(indexNames?.index_name?.length).to.eql(0); + expect(indexNames?.length).to.eql(0); }); it(`${secOnlyRead.username} should be able to access the security solution alert in ${SPACE1}`, async () => { const indexNames = await getSecuritySolutionIndexName(secOnlyRead, SPACE1); - const securitySolution = indexNames?.index_name?.find((indexName) => - indexName.startsWith(SECURITY_SOLUTION_ALERT_INDEX) - ); - expect(securitySolution).to.eql(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`); // assert this here so we can use constants in the dynamically-defined test cases below + expect(indexNames.includes(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`)).to.eql(true); // assert this here so we can use constants in the dynamically-defined test cases below }); }); }); 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 229f31375200a..ad63d6d1c7ef5 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 @@ -27,8 +27,9 @@ export default ({ loadTestFile, getService }: FtrProviderContext): void => { // loadTestFile(require.resolve('./get_alert_by_id')); // loadTestFile(require.resolve('./update_alert')); // loadTestFile(require.resolve('./bulk_update_alerts')); - // loadTestFile(require.resolve('./find_alerts')); - // loadTestFile(require.resolve('./get_alerts_index')); + + loadTestFile(require.resolve('./get_alerts_index')); + loadTestFile(require.resolve('./find_alerts')); loadTestFile(require.resolve('./search_strategy')); }); }; diff --git a/x-pack/test/rule_registry/spaces_only/tests/trial/get_alert_by_id.ts b/x-pack/test/rule_registry/spaces_only/tests/trial/get_alert_by_id.ts index a9969fd3bfd52..d248858f19f6d 100644 --- a/x-pack/test/rule_registry/spaces_only/tests/trial/get_alert_by_id.ts +++ b/x-pack/test/rule_registry/spaces_only/tests/trial/get_alert_by_id.ts @@ -31,10 +31,9 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) .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 + expect( + indexNames?.index_name?.filter((indexName) => indexName.startsWith(APM_ALERT_INDEX)).length + ).to.eql(1); // assert this here so we can use constants in the dynamically-defined test cases below }; const getSecuritySolutionIndexName = async (user: User) => { @@ -43,10 +42,11 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) .set('kbn-xsrf', 'true') .expect(200); - const securitySolution = indexNames?.index_name?.find((indexName) => - indexName.startsWith(SECURITY_SOLUTION_ALERT_INDEX) - ); - expect(securitySolution).to.eql(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`); // assert this here so we can use constants in the dynamically-defined test cases below + expect( + indexNames?.index_name?.filter((indexName) => + indexName.startsWith(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`) + ).length + ).to.eql(1); // assert this here so we can use constants in the dynamically-defined test cases below }; describe('Alerts - GET - RBAC', () => { diff --git a/x-pack/test/rule_registry/spaces_only/tests/trial/update_alert.ts b/x-pack/test/rule_registry/spaces_only/tests/trial/update_alert.ts index 5b5bc64a5a9bd..31c3cfdecb9ee 100644 --- a/x-pack/test/rule_registry/spaces_only/tests/trial/update_alert.ts +++ b/x-pack/test/rule_registry/spaces_only/tests/trial/update_alert.ts @@ -31,10 +31,9 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) .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 + expect( + indexNames?.index_name?.filter((indexName) => indexName.startsWith(APM_ALERT_INDEX)).length + ).to.eql(1); // assert this here so we can use constants in the dynamically-defined test cases below }; const getSecuritySolutionIndexName = async (user: User) => { @@ -43,10 +42,11 @@ export default ({ getService }: FtrProviderContext) => { .get(`${getSpaceUrlPrefix(SPACE1)}${ALERTS_INDEX_URL}`) .set('kbn-xsrf', 'true') .expect(200); - const securitySolution = indexNames?.index_name?.find((indexName) => - indexName.startsWith(SECURITY_SOLUTION_ALERT_INDEX) - ); - expect(securitySolution).to.eql(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`); // assert this here so we can use constants in the dynamically-defined test cases below + expect( + indexNames?.index_name?.filter((indexName) => + indexName.startsWith(`${SECURITY_SOLUTION_ALERT_INDEX}-${SPACE1}`) + ).length + ).to.eql(1); // assert this here so we can use constants in the dynamically-defined test cases below }; describe('Alert - Update - RBAC - spaces', () => {