Skip to content

Commit

Permalink
[Alerting] Exempt Alerts pre 7.10 from RBAC on their Action execution…
Browse files Browse the repository at this point in the history
… until updated (#75563)

Marks all Alerts with a `versionApiKeyLastmodified ` field that tracks what version the alert's Api Key was last updated in. We then use this field to exempt legacy alerts (created pre `7.10.0`) in order to use a _dialed down_ version of RBAC which should allow old alerts to continue to function after the upgrade, until they are updates (at which point they will no longer be **Legacy**).

More details here: #74858 (comment)
  • Loading branch information
gmmorris authored Sep 16, 2020
1 parent 599d55a commit efe7612
Show file tree
Hide file tree
Showing 41 changed files with 4,411 additions and 252 deletions.
16 changes: 12 additions & 4 deletions x-pack/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,14 @@ The following table describes the properties of the `options` object.
| params | The `params` value to give the action type executor. | object |
| spaceId | The space id the action is within. | string |
| apiKey | The Elasticsearch API key to use for context. (Note: only required and used when security is enabled). | string |
| source | The source of the execution, either an HTTP request or a reference to a Saved Object. | object, optional |

## Example

This example makes action `3c5b2bd4-5424-4e4b-8cf5-c0a58c762cc5` send an email. The action plugin will load the saved object and find what action type to call with `params`.

```typescript
const request: KibanaRequest = { ... };
const actionsClient = await server.plugins.actions.getActionsClientWithRequest(request);
await actionsClient.enqueueExecution({
id: '3c5b2bd4-5424-4e4b-8cf5-c0a58c762cc5',
Expand All @@ -296,6 +298,7 @@ await actionsClient.enqueueExecution({
subject: 'My email subject',
body: 'My email body',
},
source: asHttpRequestExecutionSource(request),
});
```

Expand All @@ -305,10 +308,11 @@ This api runs the action and asynchronously returns the result of running the ac

The following table describes the properties of the `options` object.

| Property | Description | Type |
| -------- | ---------------------------------------------------- | ------ |
| id | The id of the action you want to execute. | string |
| params | The `params` value to give the action type executor. | object |
| Property | Description | Type |
| -------- | ------------------------------------------------------------------------------------ | ------ |
| id | The id of the action you want to execute. | string |
| params | The `params` value to give the action type executor. | object |
| source | The source of the execution, either an HTTP request or a reference to a Saved Object.| object, optional |

## Example

Expand All @@ -324,6 +328,10 @@ const result = await actionsClient.execute({
subject: 'My email subject',
body: 'My email body',
},
source: asSavedObjectExecutionSource({
id: '573891ae-8c48-49cb-a197-0cd5ec34a88b',
type: 'alert'
}),
});
```

Expand Down
13 changes: 10 additions & 3 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ 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';

// 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 @@ -298,13 +299,19 @@ export class ActionsClient {
public async execute({
actionId,
params,
source,
}: Omit<ExecuteOptions, 'request'>): Promise<ActionTypeExecutorResult<unknown>> {
await this.authorization.ensureAuthorized('execute');
return this.actionExecutor.execute({ actionId, params, request: this.request });
if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
await this.authorization.ensureAuthorized('execute');
}
return this.actionExecutor.execute({ actionId, params, source, request: this.request });
}

public async enqueueExecution(options: EnqueueExecutionOptions): Promise<void> {
await this.authorization.ensureAuthorized('execute');
const { source } = options;
if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
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 @@ -9,6 +9,7 @@ import { ActionsAuthorization } from './actions_authorization';
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';

const request = {} as KibanaRequest;

Expand All @@ -19,12 +20,13 @@ const mockAuthorizationAction = (type: string, operation: string) => `${type}/${
function mockSecurity() {
const security = securityMock.createSetup();
const authorization = security.authz;
const authentication = security.authc;
// typescript is having trouble inferring jest's automocking
(authorization.actions.savedObject.get as jest.MockedFunction<
typeof authorization.actions.savedObject.get
>).mockImplementation(mockAuthorizationAction);
authorization.mode.useRbacForRequest.mockReturnValue(true);
return { authorization };
return { authorization, authentication };
}

beforeEach(() => {
Expand Down Expand Up @@ -192,4 +194,38 @@ describe('ensureAuthorized', () => {
]
`);
});

test('exempts users from requiring privileges to execute actions when shouldUseLegacyRbac is true', async () => {
const { authorization, authentication } = mockSecurity();
const checkPrivileges: jest.MockedFunction<ReturnType<
typeof authorization.checkPrivilegesDynamicallyWithRequest
>> = jest.fn();
authorization.checkPrivilegesDynamicallyWithRequest.mockReturnValue(checkPrivileges);
const actionsAuthorization = new ActionsAuthorization({
request,
authorization,
authentication,
auditLogger,
shouldUseLegacyRbac: true,
});

authentication.getCurrentUser.mockReturnValueOnce(({
username: 'some-user',
} as unknown) as AuthenticatedUser);

await actionsAuthorization.ensureAuthorized('execute', 'myType');

expect(authorization.actions.savedObject.get).not.toHaveBeenCalled();
expect(checkPrivileges).not.toHaveBeenCalled();

expect(auditLogger.actionsAuthorizationSuccess).toHaveBeenCalledTimes(1);
expect(auditLogger.actionsAuthorizationFailure).not.toHaveBeenCalled();
expect(auditLogger.actionsAuthorizationSuccess.mock.calls[0]).toMatchInlineSnapshot(`
Array [
"some-user",
"execute",
"myType",
]
`);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,15 @@ export interface ConstructorOptions {
request: KibanaRequest;
auditLogger: ActionsAuthorizationAuditLogger;
authorization?: SecurityPluginSetup['authz'];
authentication?: SecurityPluginSetup['authc'];
// In order to support legacy Alerts which predate the introduction of the
// Actions feature in Kibana we need a way of "dialing down" the level of
// authorization for certain opearations.
// Specifically, we want to allow these old alerts and their scheduled
// 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;
}

const operationAlias: Record<
Expand All @@ -27,33 +36,57 @@ const operationAlias: Record<
list: (authorization) => authorization.actions.savedObject.get(ACTION_SAVED_OBJECT_TYPE, 'find'),
};

const LEGACY_RBAC_EXEMPT_OPERATIONS = new Set(['get', 'execute']);

export class ActionsAuthorization {
private readonly request: KibanaRequest;
private readonly authorization?: SecurityPluginSetup['authz'];
private readonly authentication?: SecurityPluginSetup['authc'];
private readonly auditLogger: ActionsAuthorizationAuditLogger;
private readonly shouldUseLegacyRbac: boolean;

constructor({ request, authorization, auditLogger }: ConstructorOptions) {
constructor({
request,
authorization,
authentication,
auditLogger,
shouldUseLegacyRbac = false,
}: ConstructorOptions) {
this.request = request;
this.authorization = authorization;
this.authentication = authentication;
this.auditLogger = auditLogger;
this.shouldUseLegacyRbac = shouldUseLegacyRbac;
}

public async ensureAuthorized(operation: string, actionTypeId?: string) {
const { authorization } = this;
if (authorization?.mode?.useRbacForRequest(this.request)) {
const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
const { hasAllRequested, username } = await checkPrivileges({
kibana: operationAlias[operation]
? operationAlias[operation](authorization)
: authorization.actions.savedObject.get(ACTION_SAVED_OBJECT_TYPE, operation),
});
if (hasAllRequested) {
this.auditLogger.actionsAuthorizationSuccess(username, operation, actionTypeId);
} else {
throw Boom.forbidden(
this.auditLogger.actionsAuthorizationFailure(username, operation, actionTypeId)
if (this.isOperationExemptDueToLegacyRbac(operation)) {
this.auditLogger.actionsAuthorizationSuccess(
this.authentication?.getCurrentUser(this.request)?.username ?? '',
operation,
actionTypeId
);
} else {
const checkPrivileges = authorization.checkPrivilegesDynamicallyWithRequest(this.request);
const { hasAllRequested, username } = await checkPrivileges({
kibana: operationAlias[operation]
? operationAlias[operation](authorization)
: authorization.actions.savedObject.get(ACTION_SAVED_OBJECT_TYPE, operation),
});
if (hasAllRequested) {
this.auditLogger.actionsAuthorizationSuccess(username, operation, actionTypeId);
} else {
throw Boom.forbidden(
this.auditLogger.actionsAuthorizationFailure(username, operation, actionTypeId)
);
}
}
}
}

private isOperationExemptDueToLegacyRbac(operation: string) {
return this.shouldUseLegacyRbac && LEGACY_RBAC_EXEMPT_OPERATIONS.has(operation);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* 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 { 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);
});

test('should return false if source is not an alert', async () => {
expect(
await shouldLegacyRbacApplyBySource(
unsecuredSavedObjectsClient,
asSavedObjectExecutionSource({
type: 'action',
id: uuid.v4(),
})
)
).toEqual(false);
});

test('should return false if source alert is not marked as legacy', async () => {
const id = uuid.v4();
unsecuredSavedObjectsClient.get.mockResolvedValue(mockAlert({ id }));
expect(
await shouldLegacyRbacApplyBySource(
unsecuredSavedObjectsClient,
asSavedObjectExecutionSource({
type: 'alert',
id,
})
)
).toEqual(false);
});

test('should return true 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(
unsecuredSavedObjectsClient,
asSavedObjectExecutionSource({
type: 'alert',
id,
})
)
).toEqual(true);
});

test('should return false 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(
unsecuredSavedObjectsClient,
asSavedObjectExecutionSource({
type: 'alert',
id,
})
)
).toEqual(false);
});

test('should return false if source alert is marked with a last modified version', async () => {
const id = uuid.v4();
unsecuredSavedObjectsClient.get.mockResolvedValue(mockAlert({ id, attributes: { meta: {} } }));
expect(
await shouldLegacyRbacApplyBySource(
unsecuredSavedObjectsClient,
asSavedObjectExecutionSource({
type: 'alert',
id,
})
)
).toEqual(false);
});
});

const mockAlert = (overrides: Record<string, unknown> = {}) => ({
id: '1',
type: 'alert',
attributes: {
consumer: 'myApp',
schedule: { interval: '10s' },
alertTypeId: 'myType',
enabled: false,
actions: [
{
group: 'default',
id: '1',
actionTypeId: '1',
actionRef: '1',
params: {
foo: true,
},
},
],
},
version: '123',
references: [],
...overrides,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { SavedObjectsClientContract } from 'src/core/server';
import { ActionExecutionSource, isSavedObjectExecutionSource } from '../lib';
import { ALERT_SAVED_OBJECT_TYPE } from '../saved_objects';

const LEGACY_VERSION = 'pre-7.10.0';

export async function shouldLegacyRbacApplyBySource(
unsecuredSavedObjectsClient: SavedObjectsClientContract,
executionSource?: ActionExecutionSource<unknown>
): Promise<boolean> {
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;
}
Loading

0 comments on commit efe7612

Please sign in to comment.