From cd4d020db9d11a12cd7ac95b2ab8442b4d905b8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mike=20C=C3=B4t=C3=A9?= Date: Wed, 9 Oct 2019 10:13:59 -0400 Subject: [PATCH] Handle scenario security plugin is enabled but Elasticsearch security is disabled (#47504) (#47690) --- .../server/create_execute_function.test.ts | 21 ------ .../actions/server/create_execute_function.ts | 6 +- .../server/lib/task_runner_factory.test.ts | 71 +------------------ .../actions/server/lib/task_runner_factory.ts | 6 +- .../legacy/plugins/actions/server/plugin.ts | 2 - .../server/lib/task_runner_factory.test.ts | 6 +- .../server/lib/task_runner_factory.ts | 6 +- .../legacy/plugins/alerting/server/plugin.ts | 1 - 8 files changed, 5 insertions(+), 114 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/create_execute_function.test.ts b/x-pack/legacy/plugins/actions/server/create_execute_function.test.ts index eff7f673da98a..e928eb491c1b5 100644 --- a/x-pack/legacy/plugins/actions/server/create_execute_function.test.ts +++ b/x-pack/legacy/plugins/actions/server/create_execute_function.test.ts @@ -18,7 +18,6 @@ describe('execute()', () => { test('schedules the action with all given parameters', async () => { const executeFn = createExecuteFunction({ getBasePath, - isSecurityEnabled: true, taskManager: mockTaskManager, getScopedSavedObjectsClient: jest.fn().mockReturnValueOnce(savedObjectsClient), }); @@ -72,7 +71,6 @@ describe('execute()', () => { getBasePath, taskManager: mockTaskManager, getScopedSavedObjectsClient, - isSecurityEnabled: true, }); savedObjectsClient.get.mockResolvedValueOnce({ id: '123', @@ -117,7 +115,6 @@ describe('execute()', () => { test(`doesn't use API keys when not provided`, async () => { const getScopedSavedObjectsClient = jest.fn().mockReturnValueOnce(savedObjectsClient); const executeFn = createExecuteFunction({ - isSecurityEnabled: false, getBasePath, taskManager: mockTaskManager, getScopedSavedObjectsClient, @@ -157,22 +154,4 @@ describe('execute()', () => { }, }); }); - - test(`throws an error when isSecurityEnabled is true and key not passed in`, async () => { - const executeFn = createExecuteFunction({ - getBasePath, - taskManager: mockTaskManager, - getScopedSavedObjectsClient: jest.fn().mockReturnValueOnce(savedObjectsClient), - isSecurityEnabled: true, - }); - await expect( - executeFn({ - id: '123', - params: { baz: false }, - spaceId: 'default', - }) - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"API key is required. The attribute \\"apiKey\\" is missing."` - ); - }); }); diff --git a/x-pack/legacy/plugins/actions/server/create_execute_function.ts b/x-pack/legacy/plugins/actions/server/create_execute_function.ts index c441b256dccb6..451be9354006e 100644 --- a/x-pack/legacy/plugins/actions/server/create_execute_function.ts +++ b/x-pack/legacy/plugins/actions/server/create_execute_function.ts @@ -9,7 +9,6 @@ import { TaskManagerStartContract } from './shim'; import { GetBasePathFunction } from './types'; interface CreateExecuteFunctionOptions { - isSecurityEnabled: boolean; taskManager: TaskManagerStartContract; getScopedSavedObjectsClient: (request: any) => SavedObjectsClientContract; getBasePath: GetBasePathFunction; @@ -25,15 +24,12 @@ export interface ExecuteOptions { export function createExecuteFunction({ getBasePath, taskManager, - isSecurityEnabled, getScopedSavedObjectsClient, }: CreateExecuteFunctionOptions) { return async function execute({ id, params, spaceId, apiKey }: ExecuteOptions) { const requestHeaders: Record = {}; - if (isSecurityEnabled && !apiKey) { - throw new Error('API key is required. The attribute "apiKey" is missing.'); - } else if (isSecurityEnabled) { + if (apiKey) { requestHeaders.authorization = `ApiKey ${apiKey}`; } diff --git a/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.test.ts b/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.test.ts index 8415adfcbc146..cc18c7b169429 100644 --- a/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.test.ts +++ b/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.test.ts @@ -67,7 +67,6 @@ const taskRunnerFactoryInitializerParams = { spaceIdToNamespace, encryptedSavedObjectsPlugin: mockedEncryptedSavedObjectsPlugin, getBasePath: jest.fn().mockReturnValue(undefined), - isSecurityEnabled: true, }; beforeEach(() => { @@ -217,52 +216,7 @@ test('uses API key when provided', async () => { test(`doesn't use API key when not provided`, async () => { const factory = new TaskRunnerFactory(mockedActionExecutor); - factory.initialize({ - ...taskRunnerFactoryInitializerParams, - isSecurityEnabled: false, - }); - const taskRunner = factory.create({ taskInstance: mockedTaskInstance }); - - mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok' }); - spaceIdToNamespace.mockReturnValueOnce('namespace-test'); - mockedEncryptedSavedObjectsPlugin.getDecryptedAsInternalUser.mockResolvedValueOnce({ - id: '3', - type: 'action_task_params', - attributes: { - actionId: '2', - params: { baz: true }, - }, - references: [], - }); - - await taskRunner.run(); - - expect(mockedActionExecutor.execute).toHaveBeenCalledWith({ - actionId: '2', - params: { baz: true }, - request: { - getBasePath: expect.anything(), - headers: {}, - path: '/', - route: { settings: {} }, - url: { - href: '/', - }, - raw: { - req: { - url: '/', - }, - }, - }, - }); -}); - -test(`doesn't use API key when provided and isSecurityEnabled is set to false`, async () => { - const factory = new TaskRunnerFactory(mockedActionExecutor); - factory.initialize({ - ...taskRunnerFactoryInitializerParams, - isSecurityEnabled: false, - }); + factory.initialize(taskRunnerFactoryInitializerParams); const taskRunner = factory.create({ taskInstance: mockedTaskInstance }); mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok' }); @@ -273,7 +227,6 @@ test(`doesn't use API key when provided and isSecurityEnabled is set to false`, attributes: { actionId: '2', params: { baz: true }, - apiKey: Buffer.from('123:abc').toString('base64'), }, references: [], }); @@ -299,25 +252,3 @@ test(`doesn't use API key when provided and isSecurityEnabled is set to false`, }, }); }); - -test(`throws an error when isSecurityEnabled is true but key isn't provided`, async () => { - const taskRunner = taskRunnerFactory.create({ - taskInstance: mockedTaskInstance, - }); - - mockedActionExecutor.execute.mockResolvedValueOnce({ status: 'ok' }); - spaceIdToNamespace.mockReturnValueOnce('namespace-test'); - mockedEncryptedSavedObjectsPlugin.getDecryptedAsInternalUser.mockResolvedValueOnce({ - id: '3', - type: 'action_task_params', - attributes: { - actionId: '2', - params: { baz: true }, - }, - references: [], - }); - - await expect(taskRunner.run()).rejects.toThrowErrorMatchingInlineSnapshot( - `"API key is required. The attribute \\"apiKey\\" is missing."` - ); -}); diff --git a/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.ts b/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.ts index 202c4d0d62a1d..efe9ce7fa8be5 100644 --- a/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.ts +++ b/x-pack/legacy/plugins/actions/server/lib/task_runner_factory.ts @@ -14,7 +14,6 @@ export interface TaskRunnerContext { encryptedSavedObjectsPlugin: EncryptedSavedObjectsStartContract; spaceIdToNamespace: SpaceIdToNamespaceFunction; getBasePath: GetBasePathFunction; - isSecurityEnabled: boolean; } export class TaskRunnerFactory { @@ -44,7 +43,6 @@ export class TaskRunnerFactory { encryptedSavedObjectsPlugin, spaceIdToNamespace, getBasePath, - isSecurityEnabled, } = this.taskRunnerContext!; return { @@ -61,9 +59,7 @@ export class TaskRunnerFactory { ); const requestHeaders: Record = {}; - if (isSecurityEnabled && !apiKey) { - throw new ExecutorError('API key is required. The attribute "apiKey" is missing.'); - } else if (isSecurityEnabled) { + if (apiKey) { requestHeaders.authorization = `ApiKey ${apiKey}`; } diff --git a/x-pack/legacy/plugins/actions/server/plugin.ts b/x-pack/legacy/plugins/actions/server/plugin.ts index fa2c2320504eb..618c1d120c37a 100644 --- a/x-pack/legacy/plugins/actions/server/plugin.ts +++ b/x-pack/legacy/plugins/actions/server/plugin.ts @@ -170,14 +170,12 @@ export class Plugin { encryptedSavedObjectsPlugin: plugins.encrypted_saved_objects, getBasePath, spaceIdToNamespace, - isSecurityEnabled: !!plugins.security, }); const executeFn = createExecuteFunction({ taskManager: plugins.task_manager, getScopedSavedObjectsClient: core.savedObjects.getScopedSavedObjectsClient, getBasePath, - isSecurityEnabled: !!plugins.security, }); return { diff --git a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts index 5a3c4a4e91eaa..ccc91ae2a9034 100644 --- a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts +++ b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.test.ts @@ -60,7 +60,6 @@ const services = { }; const taskRunnerFactoryInitializerParams: jest.Mocked = { - isSecurityEnabled: true, getServices: jest.fn().mockReturnValue(services), executeAction: jest.fn(), encryptedSavedObjectsPlugin, @@ -315,10 +314,7 @@ test('uses API key when provided', async () => { test(`doesn't use API key when not provided`, async () => { const factory = new TaskRunnerFactory(); - factory.initialize({ - ...taskRunnerFactoryInitializerParams, - isSecurityEnabled: false, - }); + factory.initialize(taskRunnerFactoryInitializerParams); const taskRunner = factory.create(alertType, { taskInstance: mockedTaskInstance, }); diff --git a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts index cf4cb4bf9f50d..ca11dc8533996 100644 --- a/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts +++ b/x-pack/legacy/plugins/alerting/server/lib/task_runner_factory.ts @@ -24,7 +24,6 @@ import { export interface TaskRunnerContext { logger: Logger; - isSecurityEnabled: boolean; getServices: GetServicesFunction; executeAction: ActionsPluginStartContract['execute']; encryptedSavedObjectsPlugin: EncryptedSavedObjectsStartContract; @@ -51,7 +50,6 @@ export class TaskRunnerFactory { const { logger, - isSecurityEnabled, getServices, executeAction, encryptedSavedObjectsPlugin, @@ -74,9 +72,7 @@ export class TaskRunnerFactory { { namespace } ); - if (isSecurityEnabled && !apiKey) { - throw new Error('API key is required. The attribute "apiKey" is missing.'); - } else if (isSecurityEnabled) { + if (apiKey) { requestHeaders.authorization = `ApiKey ${apiKey}`; } diff --git a/x-pack/legacy/plugins/alerting/server/plugin.ts b/x-pack/legacy/plugins/alerting/server/plugin.ts index 150343f447c1d..d6c6d5907e7ac 100644 --- a/x-pack/legacy/plugins/alerting/server/plugin.ts +++ b/x-pack/legacy/plugins/alerting/server/plugin.ts @@ -139,7 +139,6 @@ export class Plugin { this.taskRunnerFactory.initialize({ logger: this.logger, - isSecurityEnabled: !!plugins.security, getServices(request: Hapi.Request): Services { return { callCluster: (...args) =>