Skip to content

Commit

Permalink
Add license checks within alerting / actions framework (#59699)
Browse files Browse the repository at this point in the history
* Initial work

* Handle errors in update action API

* Add unit tests for APIs

* Verify action type before scheduling action task

* Make actions plugin.execute throw error if action type is disabled

* Bug fixes

* Make action executor throw when action type isn't enabled

* Add test suite for basic license

* Fix ESLint errors

* Stop action task from re-running when license check fails

* Fix failing tests

* Attempt 1 to fix CI

* ESLint fixes

* Create sendResponse function on ActionTypeDisabledError

* Make disabled action types by config return 403

* Remove switch case

* Fix ESLint

* Fix confusing assertion

* Add comment explaining double mock

* Log warning when alert action isn't scheduled
  • Loading branch information
mikecote authored Mar 10, 2020
1 parent 6b7a134 commit 9c5952e
Show file tree
Hide file tree
Showing 19 changed files with 343 additions and 53 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/actions/server/action_type_registry.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const createActionTypeRegistryMock = () => {
get: jest.fn(),
list: jest.fn(),
ensureActionTypeEnabled: jest.fn(),
isActionTypeEnabled: jest.fn(),
};
return mocked;
};
Expand Down
42 changes: 42 additions & 0 deletions x-pack/plugins/actions/server/action_type_registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,48 @@ describe('has()', () => {
});
});

describe('isActionTypeEnabled', () => {
let actionTypeRegistry: ActionTypeRegistry;
const fooActionType: ActionType = {
id: 'foo',
name: 'Foo',
minimumLicenseRequired: 'basic',
executor: async () => {},
};

beforeEach(() => {
actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
actionTypeRegistry.register(fooActionType);
});

test('should call isActionTypeEnabled of the actions config', async () => {
mockedLicenseState.isLicenseValidForActionType.mockReturnValue({ isValid: true });
actionTypeRegistry.isActionTypeEnabled('foo');
expect(mockedActionsConfig.isActionTypeEnabled).toHaveBeenCalledWith('foo');
});

test('should call isLicenseValidForActionType of the license state', async () => {
mockedLicenseState.isLicenseValidForActionType.mockReturnValue({ isValid: true });
actionTypeRegistry.isActionTypeEnabled('foo');
expect(mockedLicenseState.isLicenseValidForActionType).toHaveBeenCalledWith(fooActionType);
});

test('should return false when isActionTypeEnabled is false and isLicenseValidForActionType is true', async () => {
mockedActionsConfig.isActionTypeEnabled.mockReturnValue(false);
mockedLicenseState.isLicenseValidForActionType.mockReturnValue({ isValid: true });
expect(actionTypeRegistry.isActionTypeEnabled('foo')).toEqual(false);
});

test('should return false when isActionTypeEnabled is true and isLicenseValidForActionType is false', async () => {
mockedActionsConfig.isActionTypeEnabled.mockReturnValue(true);
mockedLicenseState.isLicenseValidForActionType.mockReturnValue({
isValid: false,
reason: 'invalid',
});
expect(actionTypeRegistry.isActionTypeEnabled('foo')).toEqual(false);
});
});

describe('ensureActionTypeEnabled', () => {
let actionTypeRegistry: ActionTypeRegistry;
const fooActionType: ActionType = {
Expand Down
10 changes: 10 additions & 0 deletions x-pack/plugins/actions/server/action_type_registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ export class ActionTypeRegistry {
this.licenseState.ensureLicenseForActionType(this.get(id));
}

/**
* Returns true if action type is enabled in the config and a valid license is used.
*/
public isActionTypeEnabled(id: string) {
return (
this.actionsConfigUtils.isActionTypeEnabled(id) &&
this.licenseState.isLicenseValidForActionType(this.get(id)).isValid === true
);
}

/**
* Registers an action type to the action type registry
*/
Expand Down
37 changes: 37 additions & 0 deletions x-pack/plugins/actions/server/create_execute_function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import { taskManagerMock } from '../../task_manager/server/task_manager.mock';
import { createExecuteFunction } from './create_execute_function';
import { savedObjectsClientMock } from '../../../../src/core/server/mocks';
import { actionTypeRegistryMock } from './action_type_registry.mock';

const mockTaskManager = taskManagerMock.start();
const savedObjectsClient = savedObjectsClientMock.create();
Expand All @@ -19,6 +20,7 @@ describe('execute()', () => {
const executeFn = createExecuteFunction({
getBasePath,
taskManager: mockTaskManager,
actionTypeRegistry: actionTypeRegistryMock.create(),
getScopedSavedObjectsClient: jest.fn().mockReturnValueOnce(savedObjectsClient),
isESOUsingEphemeralEncryptionKey: false,
});
Expand Down Expand Up @@ -73,6 +75,7 @@ describe('execute()', () => {
taskManager: mockTaskManager,
getScopedSavedObjectsClient,
isESOUsingEphemeralEncryptionKey: false,
actionTypeRegistry: actionTypeRegistryMock.create(),
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '123',
Expand Down Expand Up @@ -121,6 +124,7 @@ describe('execute()', () => {
taskManager: mockTaskManager,
getScopedSavedObjectsClient,
isESOUsingEphemeralEncryptionKey: false,
actionTypeRegistry: actionTypeRegistryMock.create(),
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '123',
Expand Down Expand Up @@ -166,6 +170,7 @@ describe('execute()', () => {
taskManager: mockTaskManager,
getScopedSavedObjectsClient,
isESOUsingEphemeralEncryptionKey: true,
actionTypeRegistry: actionTypeRegistryMock.create(),
});
await expect(
executeFn({
Expand All @@ -178,4 +183,36 @@ describe('execute()', () => {
`"Unable to execute action due to the Encrypted Saved Objects plugin using an ephemeral encryption key. Please set xpack.encryptedSavedObjects.encryptionKey in kibana.yml"`
);
});

test('should ensure action type is enabled', async () => {
const mockedActionTypeRegistry = actionTypeRegistryMock.create();
const getScopedSavedObjectsClient = jest.fn().mockReturnValueOnce(savedObjectsClient);
const executeFn = createExecuteFunction({
getBasePath,
taskManager: mockTaskManager,
getScopedSavedObjectsClient,
isESOUsingEphemeralEncryptionKey: false,
actionTypeRegistry: mockedActionTypeRegistry,
});
mockedActionTypeRegistry.ensureActionTypeEnabled.mockImplementation(() => {
throw new Error('Fail');
});
savedObjectsClient.get.mockResolvedValueOnce({
id: '123',
type: 'action',
attributes: {
actionTypeId: 'mock-action',
},
references: [],
});

await expect(
executeFn({
id: '123',
params: { baz: false },
spaceId: 'default',
apiKey: null,
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Fail"`);
});
});
7 changes: 6 additions & 1 deletion x-pack/plugins/actions/server/create_execute_function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@

import { SavedObjectsClientContract } from '../../../../src/core/server';
import { TaskManagerStartContract } from '../../task_manager/server';
import { GetBasePathFunction, RawAction } from './types';
import { GetBasePathFunction, RawAction, ActionTypeRegistryContract } from './types';

interface CreateExecuteFunctionOptions {
taskManager: TaskManagerStartContract;
getScopedSavedObjectsClient: (request: any) => SavedObjectsClientContract;
getBasePath: GetBasePathFunction;
isESOUsingEphemeralEncryptionKey: boolean;
actionTypeRegistry: ActionTypeRegistryContract;
}

export interface ExecuteOptions {
Expand All @@ -25,6 +26,7 @@ export interface ExecuteOptions {
export function createExecuteFunction({
getBasePath,
taskManager,
actionTypeRegistry,
getScopedSavedObjectsClient,
isESOUsingEphemeralEncryptionKey,
}: CreateExecuteFunctionOptions) {
Expand Down Expand Up @@ -60,6 +62,9 @@ export function createExecuteFunction({

const savedObjectsClient = getScopedSavedObjectsClient(fakeRequest);
const actionSavedObject = await savedObjectsClient.get<RawAction>('action', id);

actionTypeRegistry.ensureActionTypeEnabled(actionSavedObject.attributes.actionTypeId);

const actionTaskParamsRecord = await savedObjectsClient.create('action_task_params', {
actionId: id,
params,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/actions/server/lib/license_state.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const createLicenseStateMock = () => {
clean: jest.fn(),
getLicenseInformation: jest.fn(),
ensureLicenseForActionType: jest.fn(),
isLicenseValidForActionType: jest.fn(),
checkLicense: jest.fn().mockResolvedValue({
state: LICENSE_CHECK_STATE.Valid,
}),
Expand Down
61 changes: 61 additions & 0 deletions x-pack/plugins/actions/server/lib/license_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,67 @@ describe('checkLicense()', () => {
});
});

describe('isLicenseValidForActionType', () => {
let license: BehaviorSubject<ILicense>;
let licenseState: ILicenseState;
const fooActionType: ActionType = {
id: 'foo',
name: 'Foo',
minimumLicenseRequired: 'gold',
executor: async () => {},
};

beforeEach(() => {
license = new BehaviorSubject(null as any);
licenseState = new LicenseState(license);
});

test('should return false when license not defined', () => {
expect(licenseState.isLicenseValidForActionType(fooActionType)).toEqual({
isValid: false,
reason: 'unavailable',
});
});

test('should return false when license not available', () => {
license.next({ isAvailable: false } as any);
expect(licenseState.isLicenseValidForActionType(fooActionType)).toEqual({
isValid: false,
reason: 'unavailable',
});
});

test('should return false when license is expired', () => {
const expiredLicense = licensingMock.createLicense({ license: { status: 'expired' } });
license.next(expiredLicense);
expect(licenseState.isLicenseValidForActionType(fooActionType)).toEqual({
isValid: false,
reason: 'expired',
});
});

test('should return false when license is invalid', () => {
const basicLicense = licensingMock.createLicense({
license: { status: 'active', type: 'basic' },
});
license.next(basicLicense);
expect(licenseState.isLicenseValidForActionType(fooActionType)).toEqual({
isValid: false,
reason: 'invalid',
});
});

test('should return true when license is valid', () => {
const goldLicense = licensingMock.createLicense({
license: { status: 'active', type: 'gold' },
});
license.next(goldLicense);
expect(licenseState.isLicenseValidForActionType(fooActionType)).toEqual({
isValid: true,
});
});
});

describe('ensureLicenseForActionType()', () => {
let license: BehaviorSubject<ILicense>;
let licenseState: ILicenseState;
Expand Down
58 changes: 40 additions & 18 deletions x-pack/plugins/actions/server/lib/license_state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,46 +42,68 @@ export class LicenseState {
return this.licenseInformation;
}

public ensureLicenseForActionType(actionType: ActionType) {
public isLicenseValidForActionType(
actionType: ActionType
): { isValid: true } | { isValid: false; reason: 'unavailable' | 'expired' | 'invalid' } {
if (!this.license?.isAvailable) {
throw new ActionTypeDisabledError(
i18n.translate('xpack.actions.serverSideErrors.unavailableLicenseErrorMessage', {
defaultMessage:
'Action type {actionTypeId} is disabled because license information is not available at this time.',
values: {
actionTypeId: actionType.id,
},
}),
'license_unavailable'
);
return { isValid: false, reason: 'unavailable' };
}

const check = this.license.check(actionType.id, actionType.minimumLicenseRequired);

switch (check.state) {
case LICENSE_CHECK_STATE.Expired:
return { isValid: false, reason: 'expired' };
case LICENSE_CHECK_STATE.Invalid:
return { isValid: false, reason: 'invalid' };
case LICENSE_CHECK_STATE.Unavailable:
return { isValid: false, reason: 'unavailable' };
case LICENSE_CHECK_STATE.Valid:
return { isValid: true };
default:
return assertNever(check.state);
}
}

public ensureLicenseForActionType(actionType: ActionType) {
const check = this.isLicenseValidForActionType(actionType);

if (check.isValid) {
return;
}

switch (check.reason) {
case 'unavailable':
throw new ActionTypeDisabledError(
i18n.translate('xpack.actions.serverSideErrors.unavailableLicenseErrorMessage', {
defaultMessage:
'Action type {actionTypeId} is disabled because license information is not available at this time.',
values: {
actionTypeId: actionType.id,
},
}),
'license_unavailable'
);
case 'expired':
throw new ActionTypeDisabledError(
i18n.translate('xpack.actions.serverSideErrors.expirerdLicenseErrorMessage', {
defaultMessage:
'Action type {actionTypeId} is disabled because your {licenseType} license has expired.',
values: { actionTypeId: actionType.id, licenseType: this.license.type },
values: { actionTypeId: actionType.id, licenseType: this.license!.type },
}),
'license_expired'
);
case LICENSE_CHECK_STATE.Invalid:
case LICENSE_CHECK_STATE.Unavailable:
case 'invalid':
throw new ActionTypeDisabledError(
i18n.translate('xpack.actions.serverSideErrors.invalidLicenseErrorMessage', {
defaultMessage:
'Action type {actionTypeId} is disabled because your {licenseType} license does not support it. Please upgrade your license.',
values: { actionTypeId: actionType.id, licenseType: this.license.type },
values: { actionTypeId: actionType.id, licenseType: this.license!.type },
}),
'license_invalid'
);
case LICENSE_CHECK_STATE.Valid:
break;
default:
return assertNever(check.state);
assertNever(check.reason);
}
}

Expand Down
31 changes: 31 additions & 0 deletions x-pack/plugins/actions/server/lib/task_runner_factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { actionExecutorMock } from './action_executor.mock';
import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks';
import { savedObjectsClientMock, loggingServiceMock } from 'src/core/server/mocks';
import { eventLoggerMock } from '../../../event_log/server/mocks';
import { ActionTypeDisabledError } from './errors';

const spaceIdToNamespace = jest.fn();
const actionTypeRegistry = actionTypeRegistryMock.create();
Expand Down Expand Up @@ -63,6 +64,7 @@ const actionExecutorInitializerParams = {
};
const taskRunnerFactoryInitializerParams = {
spaceIdToNamespace,
actionTypeRegistry,
logger: loggingServiceMock.create().get(),
encryptedSavedObjectsPlugin: mockedEncryptedSavedObjectsPlugin,
getBasePath: jest.fn().mockReturnValue(undefined),
Expand Down Expand Up @@ -308,3 +310,32 @@ test(`doesn't use API key when not provided`, async () => {
},
});
});

test(`throws an error when license doesn't support the action type`, async () => {
const taskRunner = taskRunnerFactory.create({
taskInstance: mockedTaskInstance,
});

mockedEncryptedSavedObjectsPlugin.getDecryptedAsInternalUser.mockResolvedValueOnce({
id: '3',
type: 'action_task_params',
attributes: {
actionId: '2',
params: { baz: true },
apiKey: Buffer.from('123:abc').toString('base64'),
},
references: [],
});
mockedActionExecutor.execute.mockImplementation(() => {
throw new ActionTypeDisabledError('Fail', 'license_invalid');
});

try {
await taskRunner.run();
throw new Error('Should have thrown');
} catch (e) {
expect(e instanceof ExecutorError).toEqual(true);
expect(e.data).toEqual({});
expect(e.retry).toEqual(false);
}
});
Loading

0 comments on commit 9c5952e

Please sign in to comment.