[Alerting] renames code in alerting RBAC exemption to make it easier …

…to maintain (#77598)

Refactor of code to make it a little clearer what it's doing and improve maintenance.

x-pack/plugins/actions/server/actions_client.ts

@@ -31,7 +31,10 @@ import {
 } from './create_execute_function';
 import { ActionsAuthorization } from './authorization/actions_authorization';
 import { ActionType } from '../common';
-import { shouldLegacyRbacApplyBySource } from './authorization/should_legacy_rbac_apply_by_source';
+import {
+  getAuthorizationModeBySource,
+  AuthorizationMode,
+} from './authorization/get_authorization_mode_by_source';
 
 // We are assuming there won't be many actions. This is why we will load
 // all the actions in advance and assume the total count to not go over 10000.
@@ -317,15 +320,21 @@ export class ActionsClient {
     params,
     source,
   }: Omit<ExecuteOptions, 'request'>): Promise<ActionTypeExecutorResult<unknown>> {
-    if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
+    if (
+      (await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) ===
+      AuthorizationMode.RBAC
+    ) {
       await this.authorization.ensureAuthorized('execute');
     }
     return this.actionExecutor.execute({ actionId, params, source, request: this.request });
   }
 
   public async enqueueExecution(options: EnqueueExecutionOptions): Promise<void> {
     const { source } = options;
-    if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
+    if (
+      (await getAuthorizationModeBySource(this.unsecuredSavedObjectsClient, source)) ===
+      AuthorizationMode.RBAC
+    ) {
       await this.authorization.ensureAuthorized('execute');
     }
     return this.executionEnqueuer(this.unsecuredSavedObjectsClient, options);

x-pack/plugins/actions/server/authorization/actions_authorization.test.ts

@@ -10,6 +10,7 @@ import { actionsAuthorizationAuditLoggerMock } from './audit_logger.mock';
 import { ActionsAuthorizationAuditLogger, AuthorizationResult } from './audit_logger';
 import { ACTION_SAVED_OBJECT_TYPE, ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from '../saved_objects';
 import { AuthenticatedUser } from '../../../security/server';
+import { AuthorizationMode } from './get_authorization_mode_by_source';
 
 const request = {} as KibanaRequest;
 
@@ -195,7 +196,7 @@ describe('ensureAuthorized', () => {
     `);
   });
 
-  test('exempts users from requiring privileges to execute actions when shouldUseLegacyRbac is true', async () => {
+  test('exempts users from requiring privileges to execute actions when authorizationMode is Legacy', async () => {
     const { authorization, authentication } = mockSecurity();
     const checkPrivileges: jest.MockedFunction<ReturnType<
       typeof authorization.checkPrivilegesDynamicallyWithRequest
@@ -206,7 +207,7 @@ describe('ensureAuthorized', () => {
       authorization,
       authentication,
       auditLogger,
-      shouldUseLegacyRbac: true,
+      authorizationMode: AuthorizationMode.Legacy,
     });
 
     authentication.getCurrentUser.mockReturnValueOnce(({

x-pack/plugins/actions/server/authorization/actions_authorization.ts

@@ -9,6 +9,7 @@ import { KibanaRequest } from 'src/core/server';
 import { SecurityPluginSetup } from '../../../security/server';
 import { ActionsAuthorizationAuditLogger } from './audit_logger';
 import { ACTION_SAVED_OBJECT_TYPE, ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE } from '../saved_objects';
+import { AuthorizationMode } from './get_authorization_mode_by_source';
 
 export interface ConstructorOptions {
   request: KibanaRequest;
@@ -22,7 +23,7 @@ export interface ConstructorOptions {
   // actions to continue to execute - which requires that we exempt auth on
   // `get` for Connectors and `execute` for Action execution when used by
   // these legacy alerts
-  shouldUseLegacyRbac?: boolean;
+  authorizationMode?: AuthorizationMode;
 }
 
 const operationAlias: Record<
@@ -43,20 +44,19 @@ export class ActionsAuthorization {
   private readonly authorization?: SecurityPluginSetup['authz'];
   private readonly authentication?: SecurityPluginSetup['authc'];
   private readonly auditLogger: ActionsAuthorizationAuditLogger;
-  private readonly shouldUseLegacyRbac: boolean;
-
+  private readonly authorizationMode: AuthorizationMode;
   constructor({
     request,
     authorization,
     authentication,
     auditLogger,
-    shouldUseLegacyRbac = false,
+    authorizationMode = AuthorizationMode.RBAC,
   }: ConstructorOptions) {
     this.request = request;
     this.authorization = authorization;
     this.authentication = authentication;
     this.auditLogger = auditLogger;
-    this.shouldUseLegacyRbac = shouldUseLegacyRbac;
+    this.authorizationMode = authorizationMode;
   }
 
   public async ensureAuthorized(operation: string, actionTypeId?: string) {
@@ -87,6 +87,9 @@ export class ActionsAuthorization {
   }
 
   private isOperationExemptDueToLegacyRbac(operation: string) {
-    return this.shouldUseLegacyRbac && LEGACY_RBAC_EXEMPT_OPERATIONS.has(operation);
+    return (
+      this.authorizationMode === AuthorizationMode.Legacy &&
+      LEGACY_RBAC_EXEMPT_OPERATIONS.has(operation)
+    );
   }
 }

x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts → x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.test.ts

@@ -3,88 +3,93 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-import { shouldLegacyRbacApplyBySource } from './should_legacy_rbac_apply_by_source';
+import {
+  getAuthorizationModeBySource,
+  AuthorizationMode,
+} from './get_authorization_mode_by_source';
 import { savedObjectsClientMock } from '../../../../../src/core/server/mocks';
 import uuid from 'uuid';
 import { asSavedObjectExecutionSource } from '../lib';
 
 const unsecuredSavedObjectsClient = savedObjectsClientMock.create();
 
-describe(`#shouldLegacyRbacApplyBySource`, () => {
-  test('should return false if no source is provided', async () => {
-    expect(await shouldLegacyRbacApplyBySource(unsecuredSavedObjectsClient)).toEqual(false);
+describe(`#getAuthorizationModeBySource`, () => {
+  test('should return RBAC if no source is provided', async () => {
+    expect(await getAuthorizationModeBySource(unsecuredSavedObjectsClient)).toEqual(
+      AuthorizationMode.RBAC
+    );
   });
 
-  test('should return false if source is not an alert', async () => {
+  test('should return RBAC if source is not an alert', async () => {
     expect(
-      await shouldLegacyRbacApplyBySource(
+      await getAuthorizationModeBySource(
         unsecuredSavedObjectsClient,
         asSavedObjectExecutionSource({
           type: 'action',
           id: uuid.v4(),
         })
       )
-    ).toEqual(false);
+    ).toEqual(AuthorizationMode.RBAC);
   });
 
-  test('should return false if source alert is not marked as legacy', async () => {
+  test('should return RBAC if source alert is not marked as legacy', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(mockAlert({ id }));
     expect(
-      await shouldLegacyRbacApplyBySource(
+      await getAuthorizationModeBySource(
         unsecuredSavedObjectsClient,
         asSavedObjectExecutionSource({
           type: 'alert',
           id,
         })
       )
-    ).toEqual(false);
+    ).toEqual(AuthorizationMode.RBAC);
   });
 
-  test('should return true if source alert is marked as legacy', async () => {
+  test('should return Legacy if source alert is marked as legacy', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(
       mockAlert({ id, attributes: { meta: { versionApiKeyLastmodified: 'pre-7.10.0' } } })
     );
     expect(
-      await shouldLegacyRbacApplyBySource(
+      await getAuthorizationModeBySource(
         unsecuredSavedObjectsClient,
         asSavedObjectExecutionSource({
           type: 'alert',
           id,
         })
       )
-    ).toEqual(true);
+    ).toEqual(AuthorizationMode.Legacy);
   });
 
-  test('should return false if source alert is marked as modern', async () => {
+  test('should return RBAC if source alert is marked as modern', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(
       mockAlert({ id, attributes: { meta: { versionApiKeyLastmodified: '7.10.0' } } })
     );
     expect(
-      await shouldLegacyRbacApplyBySource(
+      await getAuthorizationModeBySource(
         unsecuredSavedObjectsClient,
         asSavedObjectExecutionSource({
           type: 'alert',
           id,
         })
       )
-    ).toEqual(false);
+    ).toEqual(AuthorizationMode.RBAC);
   });
 
-  test('should return false if source alert is marked with a last modified version', async () => {
+  test('should return RBAC if source alert doesnt have a last modified version', async () => {
     const id = uuid.v4();
     unsecuredSavedObjectsClient.get.mockResolvedValue(mockAlert({ id, attributes: { meta: {} } }));
     expect(
-      await shouldLegacyRbacApplyBySource(
+      await getAuthorizationModeBySource(
         unsecuredSavedObjectsClient,
         asSavedObjectExecutionSource({
           type: 'alert',
           id,
         })
       )
-    ).toEqual(false);
+    ).toEqual(AuthorizationMode.RBAC);
   });
 });
 

x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts → x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.ts

@@ -10,18 +10,24 @@ import { ALERT_SAVED_OBJECT_TYPE } from '../saved_objects';
 
 const LEGACY_VERSION = 'pre-7.10.0';
 
-export async function shouldLegacyRbacApplyBySource(
+export enum AuthorizationMode {
+  Legacy,
+  RBAC,
+}
+
+export async function getAuthorizationModeBySource(
   unsecuredSavedObjectsClient: SavedObjectsClientContract,
   executionSource?: ActionExecutionSource<unknown>
-): Promise<boolean> {
+): Promise<AuthorizationMode> {
   return isSavedObjectExecutionSource(executionSource) &&
-    executionSource?.source?.type === ALERT_SAVED_OBJECT_TYPE
-    ? (
-        await unsecuredSavedObjectsClient.get<{
-          meta?: {
-            versionApiKeyLastmodified?: string;
-          };
-        }>(ALERT_SAVED_OBJECT_TYPE, executionSource.source.id)
-      ).attributes.meta?.versionApiKeyLastmodified === LEGACY_VERSION
-    : false;
+    executionSource?.source?.type === ALERT_SAVED_OBJECT_TYPE &&
+    (
+      await unsecuredSavedObjectsClient.get<{
+        meta?: {
+          versionApiKeyLastmodified?: string;
+        };
+      }>(ALERT_SAVED_OBJECT_TYPE, executionSource.source.id)
+    ).attributes.meta?.versionApiKeyLastmodified === LEGACY_VERSION
+    ? AuthorizationMode.Legacy
+    : AuthorizationMode.RBAC;
 }

x-pack/plugins/actions/server/lib/action_executor.ts

@@ -27,7 +27,7 @@ export interface ActionExecutorContext {
   getServices: GetServicesFunction;
   getActionsClientWithRequest: (
     request: KibanaRequest,
-    executionSource?: ActionExecutionSource<unknown>
+    authorizationContext?: ActionExecutionSource<unknown>
   ) => Promise<PublicMethodsOf<ActionsClient>>;
   encryptedSavedObjectsClient: EncryptedSavedObjectsClient;
   actionTypeRegistry: ActionTypeRegistryContract;

x-pack/plugins/actions/server/plugin.ts

@@ -71,7 +71,10 @@ import { ACTIONS_FEATURE } from './feature';
 import { ActionsAuthorization } from './authorization/actions_authorization';
 import { ActionsAuthorizationAuditLogger } from './authorization/audit_logger';
 import { ActionExecutionSource } from './lib/action_execution_source';
-import { shouldLegacyRbacApplyBySource } from './authorization/should_legacy_rbac_apply_by_source';
+import {
+  getAuthorizationModeBySource,
+  AuthorizationMode,
+} from './authorization/get_authorization_mode_by_source';
 
 const EVENT_LOG_PROVIDER = 'actions';
 export const EVENT_LOG_ACTIONS = {
@@ -281,7 +284,7 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
 
     const getActionsClientWithRequest = async (
       request: KibanaRequest,
-      source?: ActionExecutionSource<unknown>
+      authorizationContext?: ActionExecutionSource<unknown>
     ) => {
       if (isESOUsingEphemeralEncryptionKey === true) {
         throw new Error(
@@ -303,7 +306,7 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
         request,
         authorization: instantiateAuthorization(
           request,
-          await shouldLegacyRbacApplyBySource(unsecuredSavedObjectsClient, source)
+          await getAuthorizationModeBySource(unsecuredSavedObjectsClient, authorizationContext)
         ),
         actionExecutor: actionExecutor!,
         executionEnqueuer: createExecutionEnqueuerFunction({
@@ -316,7 +319,8 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
     };
 
     // Ensure the public API cannot be used to circumvent authorization
-    // using our legacy exemption mechanism
+    // using our legacy exemption mechanism by passing in a legacy SO
+    // as authorizationContext which would then set a Legacy AuthorizationMode
     const secureGetActionsClientWithRequest = (request: KibanaRequest) =>
       getActionsClientWithRequest(request);
 
@@ -389,11 +393,11 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
 
   private instantiateAuthorization = (
     request: KibanaRequest,
-    shouldUseLegacyRbac: boolean = false
+    authorizationMode?: AuthorizationMode
   ) => {
     return new ActionsAuthorization({
       request,
-      shouldUseLegacyRbac,
+      authorizationMode,
       authorization: this.security?.authz,
       authentication: this.security?.authc,
       auditLogger: new ActionsAuthorizationAuditLogger(

x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts

@@ -50,6 +50,8 @@ export function defineRoutes(
         includedHiddenTypes: ['alert'],
       });
       const savedObjectsWithAlerts = await savedObjects.getScopedClient(req, {
+        // Exclude the security and spaces wrappers to get around the safeguards those have in place to prevent
+        // us from doing what we want to do - brute force replace the ApiKey
         excludedWrappers: ['security', 'spaces'],
         includedHiddenTypes: ['alert'],
       });