diff --git a/x-pack/plugins/actions/server/actions_client.ts b/x-pack/plugins/actions/server/actions_client.ts index 01015f9046654..4079a6ddeeb8a 100644 --- a/x-pack/plugins/actions/server/actions_client.ts +++ b/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,7 +320,10 @@ export class ActionsClient { params, source, }: Omit): Promise> { - 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 }); @@ -325,7 +331,10 @@ export class ActionsClient { public async enqueueExecution(options: EnqueueExecutionOptions): Promise { 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); diff --git a/x-pack/plugins/actions/server/authorization/actions_authorization.test.ts b/x-pack/plugins/actions/server/authorization/actions_authorization.test.ts index 08c4472f8007b..a19a662f8323c 100644 --- a/x-pack/plugins/actions/server/authorization/actions_authorization.test.ts +++ b/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 { authorization, authentication, auditLogger, - shouldUseLegacyRbac: true, + authorizationMode: AuthorizationMode.Legacy, }); authentication.getCurrentUser.mockReturnValueOnce(({ diff --git a/x-pack/plugins/actions/server/authorization/actions_authorization.ts b/x-pack/plugins/actions/server/authorization/actions_authorization.ts index bd6e355c2cf9d..cad58bed50981 100644 --- a/x-pack/plugins/actions/server/authorization/actions_authorization.ts +++ b/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) + ); } } diff --git a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts b/x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.test.ts similarity index 67% rename from x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts rename to x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.test.ts index 03062994adeb6..4980c476e60ea 100644 --- a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.test.ts +++ b/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); }); }); diff --git a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts b/x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.ts similarity index 55% rename from x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts rename to x-pack/plugins/actions/server/authorization/get_authorization_mode_by_source.ts index 06d5776003ede..85d646c75defa 100644 --- a/x-pack/plugins/actions/server/authorization/should_legacy_rbac_apply_by_source.ts +++ b/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 -): Promise { +): Promise { 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; } diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts index a607dc0de0bda..73434d5c1eaa2 100644 --- a/x-pack/plugins/actions/server/lib/action_executor.ts +++ b/x-pack/plugins/actions/server/lib/action_executor.ts @@ -27,7 +27,7 @@ export interface ActionExecutorContext { getServices: GetServicesFunction; getActionsClientWithRequest: ( request: KibanaRequest, - executionSource?: ActionExecutionSource + authorizationContext?: ActionExecutionSource ) => Promise>; encryptedSavedObjectsClient: EncryptedSavedObjectsClient; actionTypeRegistry: ActionTypeRegistryContract; diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index 97cefafad4385..dca1114f0ae44 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/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, Plugi const getActionsClientWithRequest = async ( request: KibanaRequest, - source?: ActionExecutionSource + authorizationContext?: ActionExecutionSource ) => { if (isESOUsingEphemeralEncryptionKey === true) { throw new Error( @@ -303,7 +306,7 @@ export class ActionsPlugin implements Plugin, 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, 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, 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( diff --git a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts b/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts index bd8c72f57d8f2..79fc8470a8ec2 100644 --- a/x-pack/test/alerting_api_integration/common/fixtures/plugins/alerts/server/routes.ts +++ b/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'], });