[8.x] [ResponseOps][Alerts] Fix authorization issues with `disco…

…ver` as consumers (#192321) (#194441) # Backport This will backport the following commits from `main` to `8.x`: - [[ResponseOps][Alerts] Fix authorization issues with `discover` as consumers (#192321)](#192321)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Christos Nasikas <[email protected]>
elastic · Sep 30, 2024 · 464430d · 464430d
1 parent ad413cc
commit 464430d
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 47 deletions.
diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { fromKueryExpression } from '@kbn/es-query';
+import { KueryNode, fromKueryExpression, toKqlExpression } from '@kbn/es-query';
 import { KibanaRequest } from '@kbn/core/server';
 import { ruleTypeRegistryMock } from '../rule_type_registry.mock';
 import { securityMock } from '@kbn/security-plugin/server/mocks';
@@ -910,20 +910,19 @@ describe('AlertingAuthorization', () => {
         getSpaceId,
       });
       ruleTypeRegistry.list.mockReturnValue(setOfAlertTypes);
-      expect(
-        (
-          await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
-            type: AlertingAuthorizationFilterType.KQL,
-            fieldNames: {
-              ruleTypeId: 'path.to.rule_type_id',
-              consumer: 'consumer-field',
-            },
-          })
-        ).filter
-      ).toEqual(
-        fromKueryExpression(
-          `((path.to.rule_type_id:myAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:mySecondAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (path.to.rule_type_id:myOtherAppAlertType and consumer-field:(alerts or myApp or myOtherApp or myAppWithSubFeature)))`
-        )
+
+      const filter = (
+        await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
+          type: AlertingAuthorizationFilterType.KQL,
+          fieldNames: {
+            ruleTypeId: 'path.to.rule_type_id',
+            consumer: 'consumer-field',
+          },
+        })
+      ).filter;
+
+      expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
+        `"((path.to.rule_type_id: myAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)) OR (path.to.rule_type_id: mySecondAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)) OR (path.to.rule_type_id: myOtherAppAlertType AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: myApp OR consumer-field: myOtherApp OR consumer-field: myAppWithSubFeature)))"`
       );
     });
     test('throws if user has no privileges to any rule type', async () => {
@@ -1274,6 +1273,10 @@ describe('AlertingAuthorization', () => {
                 "all": true,
                 "read": true,
               },
+              "discover": Object {
+                "all": true,
+                "read": true,
+              },
               "myApp": Object {
                 "all": true,
                 "read": true,
@@ -1311,6 +1314,10 @@ describe('AlertingAuthorization', () => {
                 "all": true,
                 "read": true,
               },
+              "discover": Object {
+                "all": true,
+                "read": true,
+              },
               "myApp": Object {
                 "all": true,
                 "read": true,
@@ -2251,20 +2258,18 @@ describe('AlertingAuthorization', () => {
         });
       });
       test('creates a filter based on the privileged types', async () => {
-        expect(
-          (
-            await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
-              type: AlertingAuthorizationFilterType.KQL,
-              fieldNames: {
-                ruleTypeId: 'path.to.rule_type_id',
-                consumer: 'consumer-field',
-              },
-            })
-          ).filter
-        ).toEqual(
-          fromKueryExpression(
-            `path.to.rule_type_id:.esQuery and consumer-field:(alerts or stackAlerts or discover)`
-          )
+        const filter = (
+          await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
+            type: AlertingAuthorizationFilterType.KQL,
+            fieldNames: {
+              ruleTypeId: 'path.to.rule_type_id',
+              consumer: 'consumer-field',
+            },
+          })
+        ).filter;
+
+        expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
+          `"(path.to.rule_type_id: .esQuery AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts))"`
         );
       });
     });
@@ -2557,21 +2562,20 @@ describe('AlertingAuthorization', () => {
           expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(1);
         });
       });
+
       test('creates a filter based on the privileged types', async () => {
-        expect(
-          (
-            await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
-              type: AlertingAuthorizationFilterType.KQL,
-              fieldNames: {
-                ruleTypeId: 'path.to.rule_type_id',
-                consumer: 'consumer-field',
-              },
-            })
-          ).filter
-        ).toEqual(
-          fromKueryExpression(
-            `(path.to.rule_type_id:.esQuery and consumer-field:(alerts or stackAlerts or logs or discover)) or (path.to.rule_type_id:.logs-threshold-o11y and consumer-field:(alerts or stackAlerts or logs or discover)) or (path.to.rule_type_id:.threshold-rule-o11y and consumer-field:(alerts or stackAlerts or logs or discover))`
-          )
+        const filter = (
+          await alertAuthorization.getFindAuthorizationFilter(AlertingAuthorizationEntity.Rule, {
+            type: AlertingAuthorizationFilterType.KQL,
+            fieldNames: {
+              ruleTypeId: 'path.to.rule_type_id',
+              consumer: 'consumer-field',
+            },
+          })
+        ).filter;
+
+        expect(toKqlExpression(filter as KueryNode)).toMatchInlineSnapshot(
+          `"((path.to.rule_type_id: .esQuery AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)) OR (path.to.rule_type_id: .logs-threshold-o11y AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)) OR (path.to.rule_type_id: .threshold-rule-o11y AND (consumer-field: alerts OR consumer-field: discover OR consumer-field: stackAlerts OR consumer-field: logs)))"`
         );
       });
     });

diff --git a/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts b/x-pack/plugins/alerting/server/authorization/alerting_authorization.ts
@@ -13,6 +13,7 @@ import { KueryNode } from '@kbn/es-query';
 import { SecurityPluginSetup } from '@kbn/security-plugin/server';
 import { FeaturesPluginStart } from '@kbn/features-plugin/server';
 import { Space } from '@kbn/spaces-plugin/server';
+import { STACK_ALERTS_FEATURE_ID } from '@kbn/rule-data-utils';
 import { RegistryRuleType } from '../rule_type_registry';
 import { ALERTING_FEATURE_ID, RuleTypeRegistry } from '../types';
 import {
@@ -88,6 +89,8 @@ export interface ConstructorOptions {
   authorization?: SecurityPluginSetup['authz'];
 }
 
+const DISCOVER_FEATURE_ID = 'discover';
+
 export class AlertingAuthorization {
   private readonly ruleTypeRegistry: RuleTypeRegistry;
   private readonly request: KibanaRequest;
@@ -135,7 +138,7 @@ export class AlertingAuthorization {
 
     this.allPossibleConsumers = this.featuresIds.then((featuresIds) => {
       return featuresIds.size
-        ? asAuthorizedConsumers([ALERTING_FEATURE_ID, ...featuresIds], {
+        ? asAuthorizedConsumers([ALERTING_FEATURE_ID, DISCOVER_FEATURE_ID, ...featuresIds], {
             read: true,
             all: true,
           })
@@ -328,7 +331,22 @@ export class AlertingAuthorization {
     hasAllRequested: boolean;
     authorizedRuleTypes: Set<RegistryAlertTypeWithAuth>;
   }> {
-    const fIds = featuresIds ?? (await this.featuresIds);
+    const fIds = new Set(featuresIds ?? (await this.featuresIds));
+
+    /**
+     * Temporary hack to fix issues with the discover consumer.
+     * Issue: https://github.com/elastic/kibana/issues/184595.
+     * PR https://github.com/elastic/kibana/pull/183756 will
+     * remove the hack and fix it in a generic way.
+     *
+     * The discover consumer should be authorized
+     * as the stackAlerts consumer.
+     */
+    if (fIds.has(DISCOVER_FEATURE_ID)) {
+      fIds.delete(DISCOVER_FEATURE_ID);
+      fIds.add(STACK_ALERTS_FEATURE_ID);
+    }
+
     if (this.authorization && this.shouldCheckAuthorization()) {
       const checkPrivileges = this.authorization.checkPrivilegesDynamicallyWithRequest(
         this.request
@@ -347,11 +365,15 @@ export class AlertingAuthorization {
       >();
       const allPossibleConsumers = await this.allPossibleConsumers;
       const addLegacyConsumerPrivileges = (legacyConsumer: string) =>
-        legacyConsumer === ALERTING_FEATURE_ID || isEmpty(featuresIds);
+        legacyConsumer === ALERTING_FEATURE_ID ||
+        legacyConsumer === DISCOVER_FEATURE_ID ||
+        isEmpty(featuresIds);
+
       for (const feature of fIds) {
         const featureDef = this.features
           .getKibanaFeatures()
           .find((kFeature) => kFeature.id === feature);
+
         for (const ruleTypeId of featureDef?.alerting ?? []) {
           const ruleTypeAuth = ruleTypesWithAuthorization.find((rtwa) => rtwa.id === ruleTypeId);
           if (ruleTypeAuth) {

diff --git a/.../triggers_actions_ui/public/application/sections/alerts_table/cells/render_cell_value.tsx b/.../triggers_actions_ui/public/application/sections/alerts_table/cells/render_cell_value.tsx
@@ -138,7 +138,7 @@ export function getAlertFormatters(fieldFormats: FieldFormatsRegistry) {
         const producer = rowData?.find(({ field }) => field === ALERT_RULE_PRODUCER)?.value?.[0];
         const consumer: AlertConsumers = observabilityFeatureIds.includes(producer)
           ? 'observability'
-          : producer && (value === 'alerts' || value === 'stackAlerts')
+          : producer && (value === 'alerts' || value === 'stackAlerts' || value === 'discover')
           ? producer
           : value;
         const consumerData = alertProducersData[consumer];

diff --git a/x-pack/test/functional/es_archives/observability/alerts/data.json.gz b/x-pack/test/functional/es_archives/observability/alerts/data.json.gz
diff --git a/x-pack/test/rule_registry/common/lib/authentication/roles.ts b/x-pack/test/rule_registry/common/lib/authentication/roles.ts
@@ -265,6 +265,23 @@ export const logsOnlyAllSpacesAll: Role = {
   },
 };
 
+export const stackAlertsOnlyAllSpacesAll: Role = {
+  name: 'stack_alerts_only_all_spaces_all',
+  privileges: {
+    elasticsearch: {
+      indices: [],
+    },
+    kibana: [
+      {
+        feature: {
+          stackAlerts: ['all'],
+        },
+        spaces: ['*'],
+      },
+    ],
+  },
+};
+
 /**
  * This role exists to test that the alert search strategy allows
  * users who do not have access to security solutions the ability
@@ -494,6 +511,7 @@ export const allRoles = [
   securitySolutionOnlyReadSpacesAll,
   observabilityOnlyAllSpacesAll,
   logsOnlyAllSpacesAll,
+  stackAlertsOnlyAllSpacesAll,
   observabilityOnlyReadSpacesAll,
   observabilityOnlyAllSpacesAllWithReadESIndices,
   observabilityMinReadAlertsRead,

diff --git a/x-pack/test/rule_registry/common/lib/authentication/users.ts b/x-pack/test/rule_registry/common/lib/authentication/users.ts
@@ -30,6 +30,7 @@ import {
   observabilityMinReadAlertsAllSpacesAll,
   observabilityOnlyAllSpacesAllWithReadESIndices,
   securitySolutionOnlyAllSpacesAllWithReadESIndices,
+  stackAlertsOnlyAllSpacesAll,
 } from './roles';
 import { User } from './types';
 
@@ -176,6 +177,12 @@ export const logsOnlySpacesAll: User = {
   roles: [logsOnlyAllSpacesAll.name],
 };
 
+export const stackAlertsOnlySpacesAll: User = {
+  username: 'stack_alerts_only_all_spaces_all',
+  password: 'stack_alerts_only_all_spaces_all',
+  roles: [stackAlertsOnlyAllSpacesAll.name],
+};
+
 export const obsOnlySpacesAllEsRead: User = {
   username: 'obs_only_all_spaces_all_es_read',
   password: 'obs_only_all_spaces_all_es_read',
@@ -290,6 +297,7 @@ export const allUsers = [
   secOnlyReadSpacesAll,
   obsOnlySpacesAll,
   logsOnlySpacesAll,
+  stackAlertsOnlySpacesAll,
   obsSecSpacesAll,
   obsSecReadSpacesAll,
   obsMinReadAlertsRead,

diff --git a/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts b/x-pack/test/rule_registry/security_and_spaces/tests/basic/search_strategy.ts
@@ -13,6 +13,8 @@ import {
   obsOnlySpacesAll,
   logsOnlySpacesAll,
   secOnlySpacesAllEsReadAll,
+  stackAlertsOnlySpacesAll,
+  superUser,
 } from '../../../common/lib/authentication/users';
 
 type RuleRegistrySearchResponseWithErrors = RuleRegistrySearchResponse & {
@@ -346,6 +348,85 @@ export default ({ getService }: FtrProviderContext) => {
       });
     });
 
+    describe('discover', () => {
+      before(async () => {
+        await esArchiver.load('x-pack/test/functional/es_archives/observability/alerts');
+      });
+      after(async () => {
+        await esArchiver.unload('x-pack/test/functional/es_archives/observability/alerts');
+      });
+
+      it('should return alerts from .es-query rule type with consumer discover with access only to stack rules', async () => {
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: stackAlertsOnlySpacesAll.username,
+            password: stackAlertsOnlySpacesAll.password,
+          },
+          referer: 'test',
+          kibanaVersion,
+          internalOrigin: 'Kibana',
+          options: {
+            featureIds: ['discover'],
+          },
+          strategy: 'privateRuleRegistryAlertsSearchStrategy',
+        });
+
+        expect(result.rawResponse.hits.total).to.eql(1);
+
+        const consumers = result.rawResponse.hits.hits.map((hit) => {
+          return hit.fields?.['kibana.alert.rule.consumer'];
+        });
+
+        expect(consumers.every((consumer) => consumer === 'discover'));
+      });
+
+      it('should return alerts from .es-query rule type with consumer discover as superuser', async () => {
+        const result = await secureBsearch.send<RuleRegistrySearchResponse>({
+          supertestWithoutAuth,
+          auth: {
+            username: superUser.username,
+            password: superUser.password,
+          },
+          referer: 'test',
+          kibanaVersion,
+          internalOrigin: 'Kibana',
+          options: {
+            featureIds: ['discover'],
+          },
+          strategy: 'privateRuleRegistryAlertsSearchStrategy',
+        });
+
+        expect(result.rawResponse.hits.total).to.eql(1);
+
+        const consumers = result.rawResponse.hits.hits.map((hit) => {
+          return hit.fields?.['kibana.alert.rule.consumer'];
+        });
+
+        expect(consumers.every((consumer) => consumer === 'discover'));
+      });
+
+      it('should not return alerts from .es-query rule type with consumer discover without access to stack rules', async () => {
+        const result = await secureBsearch.send<RuleRegistrySearchResponseWithErrors>({
+          supertestWithoutAuth,
+          auth: {
+            username: logsOnlySpacesAll.username,
+            password: logsOnlySpacesAll.password,
+          },
+          referer: 'test',
+          kibanaVersion,
+          internalOrigin: 'Kibana',
+          options: {
+            featureIds: ['discover'],
+          },
+          strategy: 'privateRuleRegistryAlertsSearchStrategy',
+        });
+
+        expect(result.statusCode).to.be(500);
+        expect(result.message).to.be('Unauthorized to find alerts for any rule types');
+      });
+    });
+
     describe('empty response', () => {
       it('should return an empty response', async () => {
         const result = await secureBsearch.send<RuleRegistrySearchResponse>({