[Response Ops][RAM] Add RBAC Support for Multi-Consumer Rule Creation in O11Y and Stack Management

x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts

@@ -249,7 +249,7 @@ describe('AlertingAuthorization', () => {
         entity: AlertingAuthorizationEntity.Rule,
       });
 
-      expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(0);
+      expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(1);
     });
 
     test('is a no-op when the security license is disabled', async () => {
@@ -271,7 +271,7 @@ describe('AlertingAuthorization', () => {
         entity: AlertingAuthorizationEntity.Rule,
       });
 
-      expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(0);
+      expect(ruleTypeRegistry.get).toHaveBeenCalledTimes(1);
     });
 
     test('ensures the user has privileges to execute rules for the specified rule type and operation without consumer when producer and consumer are the same', async () => {
@@ -304,7 +304,7 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
         'myApp',
@@ -346,7 +346,7 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
         'myApp',
@@ -388,13 +388,7 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
-      expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
-        'myType',
-        'alerts',
-        'rule',
-        'create'
-      );
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
         'myApp',
@@ -436,13 +430,7 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
-      expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
-        'myType',
-        'alerts',
-        'alert',
-        'update'
-      );
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
         'myApp',
@@ -484,24 +472,15 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
-      expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
-        'myType',
-        'myApp',
-        'rule',
-        'create'
-      );
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
         'myOtherApp',
         'rule',
         'create'
       );
       expect(checkPrivileges).toHaveBeenCalledWith({
-        kibana: [
-          mockAuthorizationAction('myType', 'myOtherApp', 'rule', 'create'),
-          mockAuthorizationAction('myType', 'myApp', 'rule', 'create'),
-        ],
+        kibana: [mockAuthorizationAction('myType', 'myOtherApp', 'rule', 'create')],
       });
     });
 
@@ -535,24 +514,57 @@ describe('AlertingAuthorization', () => {
 
       expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
 
-      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(2);
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
-        'myApp',
+        'myOtherApp',
         'alert',
         'update'
       );
+      expect(checkPrivileges).toHaveBeenCalledWith({
+        kibana: [mockAuthorizationAction('myType', 'myOtherApp', 'alert', 'update')],
+      });
+    });
+
+    test('ensures the producer is used for authorization if the consumer is `alerts`', async () => {
+      const { authorization } = mockSecurity();
+      const checkPrivileges: jest.MockedFunction<
+        ReturnType<typeof authorization.checkPrivilegesDynamicallyWithRequest>
+      > = jest.fn();
+      authorization.checkPrivilegesDynamicallyWithRequest.mockReturnValue(checkPrivileges);
+      checkPrivileges.mockResolvedValueOnce({
+        username: 'some-user',
+        hasAllRequested: true,
+        privileges: { kibana: [] },
+      });
+
+      const alertAuthorization = new AlertingAuthorization({
+        request,
+        authorization,
+        ruleTypeRegistry,
+        features,
+        getSpace,
+        getSpaceId,
+      });
+
+      await alertAuthorization.ensureAuthorized({
+        ruleTypeId: 'myType',
+        consumer: 'alerts',
+        operation: WriteOperations.Create,
+        entity: AlertingAuthorizationEntity.Rule,
+      });
+
+      expect(ruleTypeRegistry.get).toHaveBeenCalledWith('myType');
+
+      expect(authorization.actions.alerting.get).toHaveBeenCalledTimes(1);
       expect(authorization.actions.alerting.get).toHaveBeenCalledWith(
         'myType',
-        'myOtherApp',
-        'alert',
-        'update'
+        'myApp',
+        'rule',
+        'create'
       );
       expect(checkPrivileges).toHaveBeenCalledWith({
-        kibana: [
-          mockAuthorizationAction('myType', 'myOtherApp', 'alert', 'update'),
-          mockAuthorizationAction('myType', 'myApp', 'alert', 'update'),
-        ],
+        kibana: [mockAuthorizationAction('myType', 'myApp', 'rule', 'create')],
       });
     });
 
@@ -596,7 +608,7 @@ describe('AlertingAuthorization', () => {
           entity: AlertingAuthorizationEntity.Rule,
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Unauthorized to create a \\"myType\\" rule for \\"myOtherApp\\""`
+        `"Unauthorized by \\"myOtherApp\\" to create \\"myType\\" rule"`
       );
     });
 
@@ -644,7 +656,7 @@ describe('AlertingAuthorization', () => {
           entity: AlertingAuthorizationEntity.Alert,
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Unauthorized to update a \\"myType\\" alert for \\"myAppRulesOnly\\""`
+        `"Unauthorized by \\"myAppRulesOnly\\" to update \\"myType\\" alert"`
       );
     });
 
@@ -688,7 +700,7 @@ describe('AlertingAuthorization', () => {
           entity: AlertingAuthorizationEntity.Alert,
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Unauthorized to update a \\"myType\\" alert by \\"myApp\\""`
+        `"Unauthorized by \\"myOtherApp\\" to update \\"myType\\" alert"`
       );
     });
 
@@ -732,7 +744,7 @@ describe('AlertingAuthorization', () => {
           entity: AlertingAuthorizationEntity.Alert,
         })
       ).rejects.toThrowErrorMatchingInlineSnapshot(
-        `"Unauthorized to create a \\"myType\\" alert for \\"myOtherApp\\""`
+        `"Unauthorized by \\"myOtherApp\\" to create \\"myType\\" alert"`
       );
     });
   });
@@ -950,7 +962,7 @@ describe('AlertingAuthorization', () => {
       expect(() => {
         ensureRuleTypeIsAuthorized('myAppAlertType', 'myOtherApp', 'alert');
       }).toThrowErrorMatchingInlineSnapshot(
-        `"Unauthorized to find a \\"myAppAlertType\\" alert for \\"myOtherApp\\""`
+        `"Unauthorized by \\"myOtherApp\\" to find \\"myAppAlertType\\" alert"`
       );
     });
     test('creates an `ensureRuleTypeIsAuthorized` function which is no-op if type is authorized', async () => {

x-pack/plugins/alerting/server/authorization/alerting_authorization.ts

@@ -6,7 +6,7 @@
  */
 
 import Boom from '@hapi/boom';
-import { map, mapValues, fromPairs, has } from 'lodash';
+import { fromPairs, has } from 'lodash';
 import { KibanaRequest } from '@kbn/core/server';
 import { JsonObject } from '@kbn/utility-types';
 import { KueryNode } from '@kbn/es-query';
@@ -169,42 +169,24 @@ export class AlertingAuthorization {
     );
   }
 
-  public async ensureAuthorized({ ruleTypeId, consumer, operation, entity }: EnsureAuthorizedOpts) {
+  public async ensureAuthorized({
+    ruleTypeId,
+    consumer: legacyConsumer,
+    operation,
+    entity,
+  }: EnsureAuthorizedOpts) {
     const { authorization } = this;
+    const ruleType = this.ruleTypeRegistry.get(ruleTypeId);
+    // We have some rules with consumer of "alerts" which indirectly means the same as
+    // a consumer of the rule type producer. Let's simplify the code and set it accordingly
+    const consumer = legacyConsumer === ALERTS_FEATURE_ID ? ruleType.producer : legacyConsumer;
 
     const isAvailableConsumer = has(await this.allPossibleConsumers, consumer);
     if (authorization && this.shouldCheckAuthorization()) {
-      const ruleType = this.ruleTypeRegistry.get(ruleTypeId);
-      const requiredPrivilegesByScope = {
-        consumer: authorization.actions.alerting.get(ruleTypeId, consumer, entity, operation),
-        producer: authorization.actions.alerting.get(
-          ruleTypeId,
-          ruleType.producer,
-          entity,
-          operation
-        ),
-      };
-
-      // Skip authorizing consumer if consumer is the Rules Management consumer (`alerts`)
-      // This means that rules and their derivative alerts created in the Rules Management UI
-      // will only be subject to checking if user has access to the rule producer.
-      const shouldAuthorizeConsumer = consumer !== ALERTS_FEATURE_ID;
-
       const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
-      const { hasAllRequested, privileges } = await checkPrivileges({
-        kibana:
-          shouldAuthorizeConsumer && consumer !== ruleType.producer
-            ? [
-                // check for access at consumer level
-                requiredPrivilegesByScope.consumer,
-                // check for access at producer level
-                requiredPrivilegesByScope.producer,
-              ]
-            : [
-                // skip consumer privilege checks under `alerts` as all rule types can
-                // be created under `alerts` if you have producer level privileges
-                requiredPrivilegesByScope.producer,
-              ],
+
+      const { hasAllRequested } = await checkPrivileges({
+        kibana: [authorization.actions.alerting.get(ruleTypeId, consumer, entity, operation)],
       });
 
       if (!isAvailableConsumer) {
@@ -215,40 +197,14 @@ export class AlertingAuthorization {
          * as Privileged.
          * This check will ensure we don't accidentally let these through
          */
-        throw Boom.forbidden(
-          getUnauthorizedMessage(ruleTypeId, ScopeType.Consumer, consumer, operation, entity)
-        );
+        throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, legacyConsumer, operation, entity));
       }
 
       if (!hasAllRequested) {
-        const authorizedPrivileges = map(
-          privileges.kibana.filter((privilege) => privilege.authorized),
-          'privilege'
-        );
-        const unauthorizedScopes = mapValues(
-          requiredPrivilegesByScope,
-          (privilege) => !authorizedPrivileges.includes(privilege)
-        );
-
-        const [unauthorizedScopeType, unauthorizedScope] =
-          shouldAuthorizeConsumer && unauthorizedScopes.consumer
-            ? [ScopeType.Consumer, consumer]
-            : [ScopeType.Producer, ruleType.producer];
-
-        throw Boom.forbidden(
-          getUnauthorizedMessage(
-            ruleTypeId,
-            unauthorizedScopeType,
-            unauthorizedScope,
-            operation,
-            entity
-          )
-        );
+        throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, consumer, operation, entity));
       }
     } else if (!isAvailableConsumer) {
-      throw Boom.forbidden(
-        getUnauthorizedMessage(ruleTypeId, ScopeType.Consumer, consumer, operation, entity)
-      );
+      throw Boom.forbidden(getUnauthorizedMessage(ruleTypeId, consumer, operation, entity));
     }
   }
 
@@ -300,13 +256,7 @@ export class AlertingAuthorization {
         ensureRuleTypeIsAuthorized: (ruleTypeId: string, consumer: string, authType: string) => {
           if (!authorizedRuleTypeIdsToConsumers.has(`${ruleTypeId}/${consumer}/${authType}`)) {
             throw Boom.forbidden(
-              getUnauthorizedMessage(
-                ruleTypeId,
-                ScopeType.Consumer,
-                consumer,
-                'find',
-                authorizationEntity
-              )
+              getUnauthorizedMessage(ruleTypeId, consumer, 'find', authorizationEntity)
             );
           } else {
             if (authorizedEntries.has(ruleTypeId)) {
@@ -460,19 +410,11 @@ function asAuthorizedConsumers(
   return fromPairs(consumers.map((feature) => [feature, hasPrivileges]));
 }
 
-enum ScopeType {
-  Consumer,
-  Producer,
-}
-
 function getUnauthorizedMessage(
   alertTypeId: string,
-  scopeType: ScopeType,
   scope: string,
   operation: string,
   entity: string
 ): string {
-  return `Unauthorized to ${operation} a "${alertTypeId}" ${entity} ${
-    scopeType === ScopeType.Consumer ? `for "${scope}"` : `by "${scope}"`
-  }`;
+  return `Unauthorized by "${scope}" to ${operation} "${alertTypeId}" ${entity}`;
 }