Skip to content

Commit

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

This reverts commit efe7612.
  • Loading branch information
gmmorris authored Sep 16, 2020
1 parent f70aace commit 3b59cf7
Show file tree
Hide file tree
Showing 41 changed files with 252 additions and 4,411 deletions.
16 changes: 4 additions & 12 deletions x-pack/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -280,14 +280,12 @@ 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 @@ -298,7 +296,6 @@ await actionsClient.enqueueExecution({
subject: 'My email subject',
body: 'My email body',
},
source: asHttpRequestExecutionSource(request),
});
```

Expand All @@ -308,11 +305,10 @@ 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 |
| source | The source of the execution, either an HTTP request or a reference to a Saved Object.| object, optional |
| 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 |

## Example

Expand All @@ -328,10 +324,6 @@ 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: 3 additions & 10 deletions x-pack/plugins/actions/server/actions_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ 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 @@ -299,19 +298,13 @@ export class ActionsClient {
public async execute({
actionId,
params,
source,
}: Omit<ExecuteOptions, 'request'>): Promise<ActionTypeExecutorResult<unknown>> {
if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
await this.authorization.ensureAuthorized('execute');
}
return this.actionExecutor.execute({ actionId, params, source, request: this.request });
await this.authorization.ensureAuthorized('execute');
return this.actionExecutor.execute({ actionId, params, request: this.request });
}

public async enqueueExecution(options: EnqueueExecutionOptions): Promise<void> {
const { source } = options;
if (!(await shouldLegacyRbacApplyBySource(this.unsecuredSavedObjectsClient, source))) {
await this.authorization.ensureAuthorized('execute');
}
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,7 +9,6 @@ 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 @@ -20,13 +19,12 @@ 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, authentication };
return { authorization };
}

beforeEach(() => {
Expand Down Expand Up @@ -194,38 +192,4 @@ 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,15 +14,6 @@ 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 @@ -36,57 +27,33 @@ 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,
authentication,
auditLogger,
shouldUseLegacyRbac = false,
}: ConstructorOptions) {
constructor({ request, authorization, auditLogger }: 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)) {
if (this.isOperationExemptDueToLegacyRbac(operation)) {
this.auditLogger.actionsAuthorizationSuccess(
this.authentication?.getCurrentUser(this.request)?.username ?? '',
operation,
actionTypeId
);
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 {
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)
);
}
throw Boom.forbidden(
this.auditLogger.actionsAuthorizationFailure(username, operation, actionTypeId)
);
}
}
}

private isOperationExemptDueToLegacyRbac(operation: string) {
return this.shouldUseLegacyRbac && LEGACY_RBAC_EXEMPT_OPERATIONS.has(operation);
}
}

This file was deleted.

This file was deleted.

Loading

0 comments on commit 3b59cf7

Please sign in to comment.