Skip to content

Commit

Permalink
[ResponseOps] Cleanup alerting RBAC exception code (#197719)
Browse files Browse the repository at this point in the history 
Resolves elastic/response-ops-team#250

## Summary

This PR eliminates the alerting RBAC exemption code. It removes all
references to `getAuthorizationModeBySource` and
`bulkGetAuthorizationModeBySource`, along with the corresponding legacy
RBAC usage counters. Additionally, downstream code paths that rely on
RBAC for authorization have been updated, and all related test cases
have been removed.
  • Loading branch information
@doakalexi
doakalexi authored Oct 30, 2024
1 parent a8c54f2 commit e6c3e6e
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 1,071 deletions.
114 changes: 0 additions & 114 deletions x-pack/plugins/actions/server/actions_client/actions_client.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,7 @@ import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/us
import { actionExecutorMock } from '../lib/action_executor.mock';
import { v4 as uuidv4 } from 'uuid';
import { ActionsAuthorization } from '../authorization/actions_authorization';
import {
getAuthorizationModeBySource,
AuthorizationMode,
bulkGetAuthorizationModeBySource,
} from '../authorization/get_authorization_mode_by_source';
import { actionsAuthorizationMock } from '../authorization/actions_authorization.mock';
import { trackLegacyRBACExemption } from '../lib/track_legacy_rbac_exemption';
import { ConnectorTokenClient } from '../lib/connector_token_client';
import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
import { SavedObject } from '@kbn/core/server';
Expand All @@ -68,25 +62,6 @@ jest.mock('@kbn/core-saved-objects-utils-server', () => {
};
});

jest.mock('../lib/track_legacy_rbac_exemption', () => ({
trackLegacyRBACExemption: jest.fn(),
}));

jest.mock('../authorization/get_authorization_mode_by_source', () => {
return {
getAuthorizationModeBySource: jest.fn(() => {
return 1;
}),
bulkGetAuthorizationModeBySource: jest.fn(() => {
return 1;
}),
AuthorizationMode: {
Legacy: 0,
RBAC: 1,
},
};
});

jest.mock('../lib/get_oauth_jwt_access_token', () => ({
getOAuthJwtAccessToken: jest.fn(),
}));
Expand Down Expand Up @@ -2745,9 +2720,6 @@ describe('update()', () => {
describe('execute()', () => {
describe('authorization', () => {
test('ensures user is authorised to excecute actions', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
unsecuredSavedObjectsClient.get.mockResolvedValueOnce(actionTypeIdFromSavedObjectMock());
await actionsClient.execute({
actionId: 'action-id',
Expand All @@ -2764,9 +2736,6 @@ describe('execute()', () => {
});

test('throws when user is not authorised to create the type of action', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
authorization.ensureAuthorized.mockRejectedValue(
new Error(`Unauthorized to execute all actions`)
);
Expand All @@ -2790,28 +2759,7 @@ describe('execute()', () => {
});
});

test('tracks legacy RBAC', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.Legacy;
});

await actionsClient.execute({
actionId: 'action-id',
params: {
name: 'my name',
},
source: asHttpRequestExecutionSource(request),
});

expect(trackLegacyRBACExemption as jest.Mock).toBeCalledWith('execute', mockUsageCounter);
expect(authorization.ensureAuthorized).not.toHaveBeenCalled();
});

test('ensures that system actions privileges are being authorized correctly', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});

actionsClient = new ActionsClient({
inMemoryConnectors: [
{
Expand Down Expand Up @@ -2872,10 +2820,6 @@ describe('execute()', () => {
});

test('does not authorize kibana privileges for non system actions', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});

actionsClient = new ActionsClient({
inMemoryConnectors: [
{
Expand Down Expand Up @@ -2939,10 +2883,6 @@ describe('execute()', () => {
});

test('pass the params to the actionTypeRegistry when authorizing system actions', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});

const getKibanaPrivileges = jest.fn().mockReturnValue(['test/create']);

actionsClient = new ActionsClient({
Expand Down Expand Up @@ -3106,9 +3046,6 @@ describe('execute()', () => {
describe('bulkEnqueueExecution()', () => {
describe('authorization', () => {
test('ensures user is authorised to execute actions', async () => {
(bulkGetAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return { [AuthorizationMode.RBAC]: 1, [AuthorizationMode.Legacy]: 0 };
});
await actionsClient.bulkEnqueueExecution([
{
id: uuidv4(),
Expand Down Expand Up @@ -3136,9 +3073,6 @@ describe('bulkEnqueueExecution()', () => {
});

test('throws when user is not authorised to create the type of action', async () => {
(bulkGetAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return { [AuthorizationMode.RBAC]: 1, [AuthorizationMode.Legacy]: 0 };
});
authorization.ensureAuthorized.mockRejectedValue(
new Error(`Unauthorized to execute all actions`)
);
Expand Down Expand Up @@ -3170,45 +3104,9 @@ describe('bulkEnqueueExecution()', () => {
operation: 'execute',
});
});

test('tracks legacy RBAC', async () => {
(bulkGetAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return { [AuthorizationMode.RBAC]: 0, [AuthorizationMode.Legacy]: 2 };
});

await actionsClient.bulkEnqueueExecution([
{
id: uuidv4(),
params: {},
spaceId: 'default',
executionId: '123abc',
apiKey: null,
source: asHttpRequestExecutionSource(request),
actionTypeId: 'my-action-type',
},
{
id: uuidv4(),
params: {},
spaceId: 'default',
executionId: '456def',
apiKey: null,
source: asHttpRequestExecutionSource(request),
actionTypeId: 'my-action-type',
},
]);

expect(trackLegacyRBACExemption as jest.Mock).toBeCalledWith(
'bulkEnqueueExecution',
mockUsageCounter,
2
);
});
});

test('calls the bulkExecutionEnqueuer with the appropriate parameters', async () => {
(bulkGetAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return { [AuthorizationMode.RBAC]: 0, [AuthorizationMode.Legacy]: 0 };
});
const opts = [
{
id: uuidv4(),
Expand Down Expand Up @@ -3504,17 +3402,11 @@ describe('getGlobalExecutionLogWithAuth()', () => {
test('ensures user is authorised to access logs', async () => {
eventLogClient.aggregateEventsWithAuthFilter.mockResolvedValue(results);

(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
await actionsClient.getGlobalExecutionLogWithAuth(opts);
expect(authorization.ensureAuthorized).toHaveBeenCalledWith({ operation: 'get' });
});

test('throws when user is not authorised to access logs', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
authorization.ensureAuthorized.mockRejectedValue(new Error(`Unauthorized to access logs`));

await expect(actionsClient.getGlobalExecutionLogWithAuth(opts)).rejects.toMatchInlineSnapshot(
Expand Down Expand Up @@ -3563,17 +3455,11 @@ describe('getGlobalExecutionKpiWithAuth()', () => {
test('ensures user is authorised to access kpi', async () => {
eventLogClient.aggregateEventsWithAuthFilter.mockResolvedValue(results);

(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
await actionsClient.getGlobalExecutionKpiWithAuth(opts);
expect(authorization.ensureAuthorized).toHaveBeenCalledWith({ operation: 'get' });
});

test('throws when user is not authorised to access kpi', async () => {
(getAuthorizationModeBySource as jest.Mock).mockImplementationOnce(() => {
return AuthorizationMode.RBAC;
});
authorization.ensureAuthorized.mockRejectedValue(new Error(`Unauthorized to access kpi`));

await expect(actionsClient.getGlobalExecutionKpiWithAuth(opts)).rejects.toMatchInlineSnapshot(
Expand Down
65 changes: 17 additions & 48 deletions x-pack/plugins/actions/server/actions_client/actions_client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import url from 'url';
import { UsageCounter } from '@kbn/usage-collection-plugin/server';

import { i18n } from '@kbn/i18n';
import { compact, uniq } from 'lodash';
import { uniq } from 'lodash';
import {
IScopedClusterClient,
SavedObjectsClientContract,
Expand All @@ -35,7 +35,7 @@ import {
IExecutionLogResult,
} from '../../common';
import { ActionTypeRegistry } from '../action_type_registry';
import { ActionExecutorContract, ActionExecutionSource, parseDate } from '../lib';
import { ActionExecutorContract, parseDate } from '../lib';
import {
ActionResult,
RawAction,
Expand All @@ -52,13 +52,7 @@ import {
ExecutionResponse,
} from '../create_execute_function';
import { ActionsAuthorization } from '../authorization/actions_authorization';
import {
getAuthorizationModeBySource,
bulkGetAuthorizationModeBySource,
AuthorizationMode,
} from '../authorization/get_authorization_mode_by_source';
import { connectorAuditEvent, ConnectorAuditAction } from '../lib/audit_events';
import { trackLegacyRBACExemption } from '../lib/track_legacy_rbac_exemption';
import { ActionsConfigurationUtilities } from '../actions_config';
import {
OAuthClientCredentialsParams,
Expand Down Expand Up @@ -496,51 +490,26 @@ export class ActionsClient {
public async bulkEnqueueExecution(
options: EnqueueExecutionOptions[]
): Promise<ExecutionResponse> {
const sources: Array<ActionExecutionSource<unknown>> = compact(
(options ?? []).map((option) => option.source)
);

const authModes = await bulkGetAuthorizationModeBySource(
this.context.logger,
this.context.unsecuredSavedObjectsClient,
sources
/**
* For scheduled executions the additional authorization check
* for system actions (kibana privileges) will be performed
* inside the ActionExecutor at execution time
*/
await this.context.authorization.ensureAuthorized({ operation: 'execute' });
await Promise.all(
uniq(options.map((o) => o.actionTypeId)).map((actionTypeId) =>
this.context.authorization.ensureAuthorized({ operation: 'execute', actionTypeId })
)
);
if (authModes[AuthorizationMode.RBAC] > 0) {
/**
* For scheduled executions the additional authorization check
* for system actions (kibana privileges) will be performed
* inside the ActionExecutor at execution time
*/
await this.context.authorization.ensureAuthorized({ operation: 'execute' });
await Promise.all(
uniq(options.map((o) => o.actionTypeId)).map((actionTypeId) =>
this.context.authorization.ensureAuthorized({ operation: 'execute', actionTypeId })
)
);
}
if (authModes[AuthorizationMode.Legacy] > 0) {
trackLegacyRBACExemption(
'bulkEnqueueExecution',
this.context.usageCounter,
authModes[AuthorizationMode.Legacy]
);
}
return this.context.bulkExecutionEnqueuer(this.context.unsecuredSavedObjectsClient, options);
}

public async ephemeralEnqueuedExecution(options: EnqueueExecutionOptions): Promise<RunNowResult> {
const { source } = options;
if (
(await getAuthorizationModeBySource(this.context.unsecuredSavedObjectsClient, source)) ===
AuthorizationMode.RBAC
) {
await this.context.authorization.ensureAuthorized({
operation: 'execute',
actionTypeId: options.actionTypeId,
});
} else {
trackLegacyRBACExemption('ephemeralEnqueuedExecution', this.context.usageCounter);
}
await this.context.authorization.ensureAuthorized({
operation: 'execute',
actionTypeId: options.actionTypeId,
});

return this.context.ephemeralExecutionEnqueuer(
this.context.unsecuredSavedObjectsClient,
options
Expand Down
64 changes: 25 additions & 39 deletions x-pack/plugins/actions/server/application/connector/methods/execute/execute.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ import { RawAction, ActionTypeExecutorResult } from '../../../../types';
import { getSystemActionKibanaPrivileges } from '../../../../lib/get_system_action_kibana_privileges';
import { isPreconfigured } from '../../../../lib/is_preconfigured';
import { isSystemAction } from '../../../../lib/is_system_action';
import {
getAuthorizationModeBySource,
AuthorizationMode,
} from '../../../../authorization/get_authorization_mode_by_source';
import { trackLegacyRBACExemption } from '../../../../lib/track_legacy_rbac_exemption';
import { ConnectorExecuteParams } from './types';
import { ACTION_SAVED_OBJECT_TYPE } from '../../../../constants/saved_objects';
import { ActionsClientContext } from '../../../../actions_client';
Expand All @@ -25,43 +20,34 @@ export async function execute(
): Promise<ActionTypeExecutorResult<unknown>> {
const log = context.logger;
const { actionId, params, source, relatedSavedObjects } = connectorExecuteParams;

if (
(await getAuthorizationModeBySource(context.unsecuredSavedObjectsClient, source)) ===
AuthorizationMode.RBAC
) {
const additionalPrivileges = getSystemActionKibanaPrivileges(context, actionId, params);
let actionTypeId: string | undefined;

try {
if (isPreconfigured(context, actionId) || isSystemAction(context, actionId)) {
const connector = context.inMemoryConnectors.find(
(inMemoryConnector) => inMemoryConnector.id === actionId
);

actionTypeId = connector?.actionTypeId;
} else {
// TODO: Optimize so we don't do another get on top of getAuthorizationModeBySource and within the actionExecutor.execute
const { attributes } = await context.unsecuredSavedObjectsClient.get<RawAction>(
ACTION_SAVED_OBJECT_TYPE,
actionId
);

actionTypeId = attributes.actionTypeId;
}
} catch (err) {
log.debug(`Failed to retrieve actionTypeId for action [${actionId}]`, err);
const additionalPrivileges = getSystemActionKibanaPrivileges(context, actionId, params);
let actionTypeId: string | undefined;

try {
if (isPreconfigured(context, actionId) || isSystemAction(context, actionId)) {
const connector = context.inMemoryConnectors.find(
(inMemoryConnector) => inMemoryConnector.id === actionId
);

actionTypeId = connector?.actionTypeId;
} else {
const { attributes } = await context.unsecuredSavedObjectsClient.get<RawAction>(
ACTION_SAVED_OBJECT_TYPE,
actionId
);

actionTypeId = attributes.actionTypeId;
}

await context.authorization.ensureAuthorized({
operation: 'execute',
additionalPrivileges,
actionTypeId,
});
} else {
trackLegacyRBACExemption('execute', context.usageCounter);
} catch (err) {
log.debug(`Failed to retrieve actionTypeId for action [${actionId}]`, err);
}

await context.authorization.ensureAuthorized({
operation: 'execute',
additionalPrivileges,
actionTypeId,
});

return context.actionExecutor.execute({
actionId,
params,
Expand Down
Loading

0 comments on commit e6c3e6e

Please sign in to comment.