Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
917c17d
Revert "removed ESO migration from alerting"
gmmorris Aug 19, 2020
e2df4b5
add `meta` field to Alert and mark pre 7.10 alerts as legacy
gmmorris Aug 20, 2020
5a7ee01
exempt legacy alerts from rbac
gmmorris Sep 3, 2020
c564990
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 3, 2020
6f7f628
test legacy for superuser and none actions user
gmmorris Sep 4, 2020
b226e32
test legacy for space_1_all and restricted users
gmmorris Sep 4, 2020
0714965
added test for global read user
gmmorris Sep 4, 2020
6c99cde
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 4, 2020
f661e32
migrate as legacy
gmmorris Sep 4, 2020
a184efa
simplified migration as we now migrate all alerts pre 7.10
gmmorris Sep 4, 2020
5e58006
cleaned up migration code
gmmorris Sep 4, 2020
b0a5c87
fixed tests
gmmorris Sep 7, 2020
f039444
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 7, 2020
a58eb4e
added docs and cleaned up code
gmmorris Sep 7, 2020
c8b8815
corrected name of variable
gmmorris Sep 7, 2020
2330880
use constant where possible
gmmorris Sep 7, 2020
5afd31d
added docs
gmmorris Sep 7, 2020
1ef7d8b
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 9, 2020
2a9694f
update meta field only when api key is updated
gmmorris Sep 9, 2020
2940bb2
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 10, 2020
e0a6050
Merge branch 'master' into alerting-exempt-rbac
elasticmachine Sep 14, 2020
a0fa54e
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 14, 2020
332d14a
fixed merge issue
gmmorris Sep 14, 2020
696a7cb
Merge branch 'master' into alerting-exempt-rbac
gmmorris Sep 15, 2020
b32e62e
migrate siem consumer
gmmorris Sep 15, 2020
b486243
typo
gmmorris Sep 16, 2020
e8eed81
tweaked authomode to make it clearer what the intention is
gmmorris Sep 16, 2020
e5c3b07
corrected typing
gmmorris Sep 16, 2020
e891d14
Merge branch 'alerting-exempt-rbac' of github.com:gmmorris/kibana int…
gmmorris Sep 16, 2020
c051d65
Merge branch 'master' into alerts/alerting-exempt-rbac-remnant
gmmorris Sep 16, 2020
6d61b87
removed unused files
gmmorris Sep 16, 2020
4eaa763
Merge branch 'master' into alerts/alerting-exempt-rbac-remnant
elasticmachine Sep 16, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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.
Expand Down Expand Up @@ -301,15 +304,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand All @@ -206,7 +207,7 @@ describe('ensureAuthorized', () => {
authorization,
authentication,
auditLogger,
shouldUseLegacyRbac: true,
authorizationMode: AuthorizationMode.Legacy,
});

authentication.getCurrentUser.mockReturnValueOnce(({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 1 addition & 1 deletion x-pack/plugins/actions/server/lib/action_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 10 additions & 6 deletions x-pack/plugins/actions/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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(
Expand All @@ -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({
Expand All @@ -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);

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
});
Expand Down