From 6c48bc180994083027202e404ca5a91120e45cd3 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Fri, 30 Aug 2019 11:23:30 +0100 Subject: [PATCH 01/17] feat(action-types-refactor): pass kibana config to builtin action type instanciation --- x-pack/legacy/plugins/actions/index.ts | 8 +++ .../actions/server/actions_config.test.ts | 36 +++++++++++ .../plugins/actions/server/actions_config.ts | 39 ++++++++++++ .../server/builtin_action_types/email.test.ts | 4 +- .../server/builtin_action_types/email.ts | 23 +++---- .../builtin_action_types/es_index.test.ts | 4 +- .../server/builtin_action_types/es_index.ts | 21 ++++--- .../server/builtin_action_types/index.ts | 28 +++++---- .../builtin_action_types/pagerduty.test.ts | 4 +- .../server/builtin_action_types/pagerduty.ts | 23 +++---- .../builtin_action_types/server_log.test.ts | 4 +- .../server/builtin_action_types/server_log.ts | 19 +++--- .../server/builtin_action_types/slack.ts | 9 +-- .../builtin_action_types/webhook.test.ts | 62 ++++++++++++++++++- .../server/builtin_action_types/webhook.ts | 52 +++++++++++----- x-pack/legacy/plugins/actions/server/init.ts | 6 +- x-pack/legacy/plugins/actions/server/types.ts | 2 + .../alerting_api_integration/common/config.ts | 4 ++ .../tests/actions/index.ts | 1 + 19 files changed, 268 insertions(+), 81 deletions(-) create mode 100644 x-pack/legacy/plugins/actions/server/actions_config.test.ts create mode 100644 x-pack/legacy/plugins/actions/server/actions_config.ts diff --git a/x-pack/legacy/plugins/actions/index.ts b/x-pack/legacy/plugins/actions/index.ts index 61d3aa7273e7a..b2e75f08297de 100644 --- a/x-pack/legacy/plugins/actions/index.ts +++ b/x-pack/legacy/plugins/actions/index.ts @@ -27,6 +27,14 @@ export function actions(kibana: any) { return Joi.object() .keys({ enabled: Joi.boolean().default(false), + whitelistedHosts: Joi.alternatives() + .try( + Joi.array() + .items(Joi.string().hostname()) + .sparse(false), + Joi.string().valid('any', 'none') + ) + .default('none'), }) .default(); }, diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts new file mode 100644 index 0000000000000..ea47390ade195 --- /dev/null +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -0,0 +1,36 @@ +/* + * 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 { ActionsKibanaConfig, getActionsConfigurationUtilities } from './actions_config'; + +describe('isWhitelistedHostname', () => { + test('returns true when "any" hostnames are allowed', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: 'any' }; + expect( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) + ).toEqual(true); + }); + + test('returns false when the hostname in the requested uri is not in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) + ).toEqual(false); + }); + + test('returns true when the hostname in the requested uri is in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; + expect( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) + ).toEqual(true); + }); +}); diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts new file mode 100644 index 0000000000000..f7ca5379ac267 --- /dev/null +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -0,0 +1,39 @@ +/* + * 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 { fromNullable } from 'fp-ts/lib/Option'; + +export interface ActionsKibanaConfig { + enabled: boolean; + whitelistedHosts: 'any' | 'none' | string[]; +} + +export interface ActionsConfigurationUtilities { + isWhitelistedHostname: (uri: string) => boolean; +} + +export function getActionsConfigurationUtilities( + config: ActionsKibanaConfig +): ActionsConfigurationUtilities { + return { + isWhitelistedHostname(uri: string) { + switch (config.whitelistedHosts) { + case 'none': + return false; + case 'any': + return true; + default: + if (Array.isArray(config.whitelistedHosts)) { + const urlHostname = new URL(uri).hostname; + return fromNullable( + config.whitelistedHosts.find(hostname => hostname === urlHostname) + ).isSome(); + } + return false; + } + }, + }; +} diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts index 4989c730db1f2..92a0c59a35172 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts @@ -9,6 +9,7 @@ jest.mock('./lib/send_email', () => ({ })); import { ActionType, ActionTypeExecutorOptions } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/plugin.mock'; import { taskManagerMock } from '../../../task_manager/task_manager.mock'; @@ -22,6 +23,7 @@ const sendEmailMock = sendEmail as jest.Mock; const ACTION_TYPE_ID = '.email'; const NO_OP_FN = () => {}; +const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; const services = { log: NO_OP_FN, @@ -48,7 +50,7 @@ beforeAll(() => { getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.ts index d6c146dada224..70ffdb164a459 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.ts @@ -97,17 +97,18 @@ function validateParams(paramsObject: any): string | void { } // action type definition - -export const actionType: ActionType = { - id: '.email', - name: 'email', - validate: { - config: ConfigSchema, - secrets: SecretsSchema, - params: ParamsSchema, - }, - executor, -}; +export function getActionType(): ActionType { + return { + id: '.email', + name: 'email', + validate: { + config: ConfigSchema, + secrets: SecretsSchema, + params: ParamsSchema, + }, + executor, + }; +} // action executor diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts index 30464d7b1e507..42078f39379fc 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts @@ -9,6 +9,7 @@ jest.mock('./lib/send_email', () => ({ })); import { ActionType, ActionTypeExecutorOptions } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/plugin.mock'; import { taskManagerMock } from '../../../task_manager/task_manager.mock'; @@ -19,6 +20,7 @@ import { ActionParamsType, ActionTypeConfigType } from './es_index'; const ACTION_TYPE_ID = '.index'; const NO_OP_FN = () => {}; +const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; const services = { log: NO_OP_FN, @@ -45,7 +47,7 @@ beforeAll(() => { getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.ts index 8fe19a16921b3..a60a3729e78fe 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.ts @@ -33,16 +33,17 @@ const ParamsSchema = schema.object({ }); // action type definition - -export const actionType: ActionType = { - id: '.index', - name: 'index', - validate: { - config: ConfigSchema, - params: ParamsSchema, - }, - executor, -}; +export function getActionType(): ActionType { + return { + id: '.index', + name: 'index', + validate: { + config: ConfigSchema, + params: ParamsSchema, + }, + executor, + }; +} // action executor diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts index b717436c54777..92e0ff7511860 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts @@ -5,17 +5,23 @@ */ import { ActionTypeRegistry } from '../action_type_registry'; +import { ActionsConfigurationUtilities } from '../actions_config'; -import { actionType as serverLogActionType } from './server_log'; -import { actionType as slackActionType } from './slack'; -import { actionType as emailActionType } from './email'; -import { actionType as indexActionType } from './es_index'; -import { actionType as pagerDutyActionType } from './pagerduty'; +import { getActionType as getServerLogActionType } from './server_log'; +import { getActionType as getSlackActionType } from './slack'; +import { getActionType as getEmailActionType } from './email'; +import { getActionType as getIndexActionType } from './es_index'; +import { getActionType as getPagerDutyActionType } from './pagerduty'; +import { getActionType as getWebhookActionType } from './webhook'; -export function registerBuiltInActionTypes(actionTypeRegistry: ActionTypeRegistry) { - actionTypeRegistry.register(serverLogActionType); - actionTypeRegistry.register(slackActionType); - actionTypeRegistry.register(emailActionType); - actionTypeRegistry.register(indexActionType); - actionTypeRegistry.register(pagerDutyActionType); +export function registerBuiltInActionTypes( + actionTypeRegistry: ActionTypeRegistry, + actionsConfigUtils: ActionsConfigurationUtilities +) { + actionTypeRegistry.register(getServerLogActionType()); + actionTypeRegistry.register(getSlackActionType()); + actionTypeRegistry.register(getEmailActionType()); + actionTypeRegistry.register(getIndexActionType()); + actionTypeRegistry.register(getPagerDutyActionType()); + actionTypeRegistry.register(getWebhookActionType(actionsConfigUtils)); } diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts index 842603d4ce829..5dbf94b358034 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts @@ -9,6 +9,7 @@ jest.mock('./lib/post_pagerduty', () => ({ })); import { ActionType, Services, ActionTypeExecutorOptions } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; import { taskManagerMock } from '../../../task_manager/task_manager.mock'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/plugin.mock'; @@ -21,6 +22,7 @@ const postPagerdutyMock = postPagerduty as jest.Mock; const ACTION_TYPE_ID = '.pagerduty'; const NO_OP_FN = () => {}; +const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; const services: Services = { log: NO_OP_FN, @@ -46,7 +48,7 @@ beforeAll(() => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.ts index 1739770edfb83..7e043d5309c37 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.ts @@ -84,17 +84,18 @@ function validateParams(paramsObject: any): string | void { } // action type definition - -export const actionType: ActionType = { - id: '.pagerduty', - name: 'pagerduty', - validate: { - config: ConfigSchema, - secrets: SecretsSchema, - params: ParamsSchema, - }, - executor, -}; +export function getActionType(): ActionType { + return { + id: '.pagerduty', + name: 'pagerduty', + validate: { + config: ConfigSchema, + secrets: SecretsSchema, + params: ParamsSchema, + }, + executor, + }; +} // action executor diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts index 761a82e8528a4..ea2d01894e4c4 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts @@ -5,6 +5,7 @@ */ import { ActionType, Services } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; import { taskManagerMock } from '../../../task_manager/task_manager.mock'; import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/plugin.mock'; @@ -15,6 +16,7 @@ import { registerBuiltInActionTypes } from './index'; const ACTION_TYPE_ID = '.server-log'; const NO_OP_FN = () => {}; +const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; const services: Services = { log: NO_OP_FN, @@ -39,7 +41,7 @@ beforeAll(() => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); }); beforeEach(() => { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.ts index 1fa73d9bb8f84..9ec8e6051b6ca 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.ts @@ -21,15 +21,16 @@ const ParamsSchema = schema.object({ }); // action type definition - -export const actionType: ActionType = { - id: '.server-log', - name: 'server-log', - validate: { - params: ParamsSchema, - }, - executor, -}; +export function getActionType(): ActionType { + return { + id: '.server-log', + name: 'server-log', + validate: { + params: ParamsSchema, + }, + executor, + }; +} // action executor diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/slack.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/slack.ts index 7ded9c6c6ef29..d98243dc7d0f9 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/slack.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/slack.ts @@ -35,9 +35,9 @@ const ParamsSchema = schema.object({ // action type definition // customizing executor is only used for tests -export function getActionType({ executor }: { executor?: ExecutorType } = {}): ActionType { - if (executor == null) executor = slackExecutor; - +export function getActionType( + { executor }: { executor: ExecutorType } = { executor: slackExecutor } +): ActionType { return { id: '.slack', name: 'slack', @@ -49,9 +49,6 @@ export function getActionType({ executor }: { executor?: ExecutorType } = {}): A }; } -// the production executor for this action -export const actionType = getActionType(); - // action executor async function slackExecutor( diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index ca3182bdeaca8..33ffda317e988 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -4,11 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -import { actionType } from './webhook'; +import { getActionType } from './webhook'; import { validateConfig, validateSecrets, validateParams } from '../lib'; +import { ActionsConfigurationUtilities } from '../actions_config'; + +const configUtilsMock: ActionsConfigurationUtilities = { + isWhitelistedHostname() { + return true; + }, +}; describe('actionType', () => { test('exposes the action as `webhook` on its Id and Name', () => { + const actionType = getActionType(configUtilsMock); expect(actionType.id).toEqual('.webhook'); expect(actionType.name).toEqual('webhook'); }); @@ -16,6 +24,7 @@ describe('actionType', () => { describe('secrets validation', () => { test('succeeds when secrets is valid', () => { + const actionType = getActionType(configUtilsMock); const secrets: Record = { user: 'bob', password: 'supersecret', @@ -25,6 +34,7 @@ describe('secrets validation', () => { test('fails when secret password is omitted', () => { expect(() => { + const actionType = getActionType(configUtilsMock); validateSecrets(actionType, { user: 'bob' }); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [password]: expected value of type [string] but got [undefined]"` @@ -33,6 +43,7 @@ describe('secrets validation', () => { test('fails when secret user is omitted', () => { expect(() => { + const actionType = getActionType(configUtilsMock); validateSecrets(actionType, {}); }).toThrowErrorMatchingInlineSnapshot( `"error validating action type secrets: [user]: expected value of type [string] but got [undefined]"` @@ -47,6 +58,7 @@ describe('config validation', () => { }; test('config validation passes when only required fields are provided', () => { + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver:9200/endpoint', }; @@ -57,6 +69,7 @@ describe('config validation', () => { }); test('config validation passes when valid methods are provided', () => { + const actionType = getActionType(configUtilsMock); ['post', 'put'].forEach(method => { const config: Record = { url: 'http://mylisteningserver:9200/endpoint', @@ -70,6 +83,7 @@ describe('config validation', () => { }); test('should validate and throw error when method on config is invalid', () => { + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver:9200/endpoint', method: 'https', @@ -84,6 +98,7 @@ describe('config validation', () => { }); test('config validation passes when a url is specified', () => { + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver:9200/endpoint', }; @@ -94,6 +109,7 @@ describe('config validation', () => { }); test('config validation passes when valid headers are provided', () => { + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver:9200/endpoint', headers: { @@ -107,6 +123,7 @@ describe('config validation', () => { }); test('should validate and throw error when headers on config is invalid', () => { + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver:9200/endpoint', headers: 'application/json', @@ -119,15 +136,58 @@ describe('config validation', () => { - [headers.1]: expected value to equal [null] but got [application/json]" `); }); + + test('config validation passes when kibana config whitelists the url', () => { + const actionType = getActionType({ + isWhitelistedHostname() { + return true; + }, + }); + + const config: Record = { + url: 'http://mylisteningserver.com:9200/endpoint', + headers: { + 'Content-Type': 'application/json', + }, + }; + + expect(validateConfig(actionType, config)).toEqual({ + ...defaultValues, + ...config, + }); + }); + + test('config validation returns an error if the specified URL isnt whitelisted', () => { + const actionType = getActionType({ + isWhitelistedHostname() { + return false; + }, + }); + + const config: Record = { + url: 'http://mylisteningserver.com:9200/endpoint', + headers: { + 'Content-Type': 'application/json', + }, + }; + + expect(() => { + validateConfig(actionType, config); + }).toThrowErrorMatchingInlineSnapshot( + `"error validating action type config: an error occurred configuring webhook with unwhitelisted target url \\"http://mylisteningserver.com:9200/endpoint\\""` + ); + }); }); describe('params validation', () => { test('param validation passes when no fields are provided as none are required', () => { + const actionType = getActionType(configUtilsMock); const params: Record = {}; expect(validateParams(actionType, params)).toEqual({}); }); test('params validation passes when a valid body is provided', () => { + const actionType = getActionType(configUtilsMock); const params: Record = { body: 'count: {{ctx.payload.hits.total}}', }; diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 193e3383d34a5..19b6ab30be6c8 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -10,6 +10,7 @@ import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_hea import { nullableType } from './lib/nullable'; import { isOk, promiseResult, Result } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; // config definition enum WebhookMethods { @@ -17,42 +18,59 @@ enum WebhookMethods { PUT = 'put', } -export type ActionTypeConfigType = TypeOf; - const HeadersSchema = schema.recordOf(schema.string(), schema.string()); - -const ConfigSchema = schema.object({ +const configSchemaProps = { url: schema.string(), method: schema.oneOf([schema.literal(WebhookMethods.POST), schema.literal(WebhookMethods.PUT)], { defaultValue: WebhookMethods.POST, }), headers: nullableType(HeadersSchema), -}); +}; +const ConfigSchema = schema.object(configSchemaProps); +type ActionTypeConfigType = TypeOf; // secrets definition -export type ActionTypeSecretsType = TypeOf; +type ActionTypeSecretsType = TypeOf; const SecretsSchema = schema.object({ user: schema.string(), password: schema.string(), }); // params definition -export type ActionParamsType = TypeOf; +type ActionParamsType = TypeOf; const ParamsSchema = schema.object({ body: schema.maybe(schema.string()), }); // action type definition -export const actionType: ActionType = { - id: '.webhook', - name: 'webhook', - validate: { - config: ConfigSchema, - secrets: SecretsSchema, - params: ParamsSchema, - }, - executor, -}; +export function getActionType(configurationUtilities: ActionsConfigurationUtilities): ActionType { + return { + id: '.webhook', + name: 'webhook', + validate: { + config: schema.object(configSchemaProps, { + validate: configObject => { + const { url }: ActionTypeConfigType = configObject; + if (!configurationUtilities.isWhitelistedHostname(url)) { + return i18n.translate( + 'xpack.actions.builtin.webhook.unwhitelistedWebhookConfigurationError', + { + defaultMessage: + 'an error occurred configuring webhook with unwhitelisted target url "{url}"', + values: { + url, + }, + } + ); + } + }, + }), + secrets: SecretsSchema, + params: ParamsSchema, + }, + executor, + }; +} // action executor async function executor(execOptions: ActionTypeExecutorOptions): Promise { diff --git a/x-pack/legacy/plugins/actions/server/init.ts b/x-pack/legacy/plugins/actions/server/init.ts index 931c8e494668e..395284938fa1d 100644 --- a/x-pack/legacy/plugins/actions/server/init.ts +++ b/x-pack/legacy/plugins/actions/server/init.ts @@ -11,6 +11,7 @@ import { ActionsClient } from './actions_client'; import { ActionTypeRegistry } from './action_type_registry'; import { createExecuteFunction } from './create_execute_function'; import { ActionsPlugin, Services } from './types'; +import { ActionsKibanaConfig, getActionsConfigurationUtilities } from './actions_config'; import { EncryptedSavedObjectsPlugin } from '../../encrypted_saved_objects'; import { createRoute, @@ -110,7 +111,10 @@ export function init(server: Server) { isSecurityEnabled: config.get('xpack.security.enabled'), }); - registerBuiltInActionTypes(actionTypeRegistry); + registerBuiltInActionTypes( + actionTypeRegistry, + getActionsConfigurationUtilities(config.get('xpack.actions') as ActionsKibanaConfig) + ); // Routes createRoute(server); diff --git a/x-pack/legacy/plugins/actions/server/types.ts b/x-pack/legacy/plugins/actions/server/types.ts index 946aa136be508..77a5460253cd7 100644 --- a/x-pack/legacy/plugins/actions/server/types.ts +++ b/x-pack/legacy/plugins/actions/server/types.ts @@ -7,6 +7,7 @@ import { SavedObjectsClientContract, SavedObjectAttributes } from 'src/core/server'; import { ActionTypeRegistry } from './action_type_registry'; import { ExecuteOptions } from './create_execute_function'; +import { ActionsKibanaConfig } from './actions_config'; export type WithoutQueryAndParams = Pick>; export type GetServicesFunction = (request: any) => Services; @@ -59,6 +60,7 @@ interface ValidatorType { validate(value: any): any; } +export type ActionTypeCreator = (config?: ActionsKibanaConfig) => ActionType; export interface ActionType { id: string; name: string; diff --git a/x-pack/test/alerting_api_integration/common/config.ts b/x-pack/test/alerting_api_integration/common/config.ts index 4bd207e51a941..1bfc50e4c83db 100644 --- a/x-pack/test/alerting_api_integration/common/config.ts +++ b/x-pack/test/alerting_api_integration/common/config.ts @@ -54,6 +54,10 @@ export function createTestConfig(name: string, options: CreateTestConfigOptions) serverArgs: [ ...xPackApiIntegrationTestsConfig.get('kbnTestServer.serverArgs'), '--xpack.actions.enabled=true', + `--xpack.actions.whitelistedHosts=${JSON.stringify([ + 'localhost', + 'some.non.existent.com', + ])}`, '--xpack.alerting.enabled=true', ...disabledPlugins.map(key => `--xpack.${key}.enabled=false`), `--plugin-path=${path.join(__dirname, 'fixtures', 'plugins', 'alerts')}`, diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/index.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/index.ts index 0780efc0fc977..7f67f2f5b60e7 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/index.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/index.ts @@ -21,5 +21,6 @@ export default function actionsTests({ loadTestFile }: FtrProviderContext) { loadTestFile(require.resolve('./builtin_action_types/email')); loadTestFile(require.resolve('./builtin_action_types/es_index')); loadTestFile(require.resolve('./builtin_action_types/pagerduty')); + loadTestFile(require.resolve('./builtin_action_types/webhook')); }); } From c03104cba9013d5ac62caf1f428055c59d4c6f57 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Fri, 30 Aug 2019 12:01:11 +0100 Subject: [PATCH 02/17] fix(webhook-whitelisting): whitelist support for webhook execution --- .../plugins/actions/server/actions_config.ts | 1 + .../builtin_action_types/webhook.test.ts | 43 ++++++++++++++++++- .../server/builtin_action_types/webhook.ts | 27 +++++++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index f7ca5379ac267..9d1cec327ee04 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -5,6 +5,7 @@ */ import { fromNullable } from 'fp-ts/lib/Option'; +import { URL } from 'url'; export interface ActionsKibanaConfig { enabled: boolean; diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index 33ffda317e988..fa2999635a2f9 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -4,9 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getActionType } from './webhook'; +import { getActionType, executor } from './webhook'; import { validateConfig, validateSecrets, validateParams } from '../lib'; import { ActionsConfigurationUtilities } from '../actions_config'; +import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; +import { Services, ActionTypeExecutorOptions } from '../types'; + +const NO_OP_FN = () => {}; +const servicesMock: Services = { + log: NO_OP_FN, + callCluster: async (path: string, opts: any) => {}, + savedObjectsClient: SavedObjectsClientMock.create(), +}; const configUtilsMock: ActionsConfigurationUtilities = { isWhitelistedHostname() { @@ -196,3 +205,35 @@ describe('params validation', () => { }); }); }); + +describe('executor', () => { + test('webhook executor should fail if the url is not whitelisted', async () => { + const config: Record = { + url: 'http://mylisteningserver.com:9200/endpoint', + }; + const secrets: Record = { + user: 'bob', + password: 'supersecret', + }; + const params: Record = {}; + const result = await executor( + { + isWhitelistedHostname: uri => { + expect(uri).toEqual(config.url); + return false; + }, + }, + { + id: 'webhook', + services: servicesMock, + config, + secrets, + params, + } as ActionTypeExecutorOptions + ); + expect(result.message).toMatchInlineSnapshot( + `"an error occurred in action \\"webhook\\" calling a remote webhook: You are not permitted to trigger this webhook"` + ); + expect(result.status).toEqual('error'); + }); +}); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 19b6ab30be6c8..49bdc508e92da 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ import { i18n } from '@kbn/i18n'; +import { curry } from 'lodash'; import axios, { AxiosError, AxiosResponse } from 'axios'; import { schema, TypeOf } from '@kbn/config-schema'; import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header'; @@ -68,12 +69,15 @@ export function getActionType(configurationUtilities: ActionsConfigurationUtilit secrets: SecretsSchema, params: ParamsSchema, }, - executor, + executor: curry(executor)(configurationUtilities), }; } // action executor -async function executor(execOptions: ActionTypeExecutorOptions): Promise { +export async function executor( + configurationUtilities: ActionsConfigurationUtilities, + execOptions: ActionTypeExecutorOptions +): Promise { const log = (level: string, msg: string) => execOptions.services.log([level, 'actions', 'webhook'], msg); @@ -82,6 +86,11 @@ async function executor(execOptions: ActionTypeExecutorOptions): Promise = await promiseResult( axios.request({ method, @@ -138,6 +147,20 @@ function successResult(data: any): ActionTypeExecutorResult { return { status: 'ok', data }; } +function errorRequestInvalid(id: string): ActionTypeExecutorResult { + const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidRequestErrorMessage', { + defaultMessage: + 'an error occurred in action "{id}" calling a remote webhook: You are not permitted to trigger this webhook', + values: { + id, + }, + }); + return { + status: 'error', + message: errMessage, + }; +} + function errorResultInvalid(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidResponseErrorMessage', { defaultMessage: 'an error occurred in action "{id}" calling a remote webhook: {message}', From e46f616dbd332380eeeb4f7c82d62d9638baa7ae Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Fri, 30 Aug 2019 13:50:18 +0100 Subject: [PATCH 03/17] fix(webhook-whitelisting): fixed missing type sig --- x-pack/legacy/plugins/actions/server/actions_config.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 9d1cec327ee04..763d47849da0f 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -20,7 +20,7 @@ export function getActionsConfigurationUtilities( config: ActionsKibanaConfig ): ActionsConfigurationUtilities { return { - isWhitelistedHostname(uri: string) { + isWhitelistedHostname(uri: string): boolean { switch (config.whitelistedHosts) { case 'none': return false; From ea4580ffb020a73f5cc5e02b88563d11c5b25c73 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 09:34:37 +0100 Subject: [PATCH 04/17] refactor(webhook-whitelisting): removed the None option as we can use an empty array and Any is now an asterix --- x-pack/legacy/plugins/actions/index.ts | 4 ++-- .../plugins/actions/server/actions_config.test.ts | 8 ++++++-- x-pack/legacy/plugins/actions/server/actions_config.ts | 10 ++++++---- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/actions/index.ts b/x-pack/legacy/plugins/actions/index.ts index b2e75f08297de..d601fa50b8fb2 100644 --- a/x-pack/legacy/plugins/actions/index.ts +++ b/x-pack/legacy/plugins/actions/index.ts @@ -32,9 +32,9 @@ export function actions(kibana: any) { Joi.array() .items(Joi.string().hostname()) .sparse(false), - Joi.string().valid('any', 'none') + Joi.string().valid('*') ) - .default('none'), + .default([]), }) .default(); }, diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index ea47390ade195..57d0e445c43f7 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -4,11 +4,15 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ActionsKibanaConfig, getActionsConfigurationUtilities } from './actions_config'; +import { + ActionsKibanaConfig, + getActionsConfigurationUtilities, + WhitelistedHosts, +} from './actions_config'; describe('isWhitelistedHostname', () => { test('returns true when "any" hostnames are allowed', () => { - const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: 'any' }; + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; expect( getActionsConfigurationUtilities(config).isWhitelistedHostname( 'https://github.com/elastic/kibana' diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 763d47849da0f..3bfb560121b9c 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -7,9 +7,13 @@ import { fromNullable } from 'fp-ts/lib/Option'; import { URL } from 'url'; +export enum WhitelistedHosts { + Any = '*', +} + export interface ActionsKibanaConfig { enabled: boolean; - whitelistedHosts: 'any' | 'none' | string[]; + whitelistedHosts: WhitelistedHosts.Any | string[]; } export interface ActionsConfigurationUtilities { @@ -22,9 +26,7 @@ export function getActionsConfigurationUtilities( return { isWhitelistedHostname(uri: string): boolean { switch (config.whitelistedHosts) { - case 'none': - return false; - case 'any': + case WhitelistedHosts.Any: return true; default: if (Array.isArray(config.whitelistedHosts)) { From f169c0af9d5fc69e47abe1050ba343c029c2b362 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 11:17:28 +0100 Subject: [PATCH 05/17] refactor(webhook-whitelisting): unified whitelisting error message --- .../actions/server/actions_config.test.ts | 27 ++++++++------ .../plugins/actions/server/actions_config.ts | 18 +++++----- .../builtin_action_types/lib/result_type.ts | 7 ++++ .../builtin_action_types/webhook.test.ts | 21 +++++------ .../server/builtin_action_types/webhook.ts | 35 +++++++++---------- 5 files changed, 61 insertions(+), 47 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index 57d0e445c43f7..9e60662a85073 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { isOk, unsafeGet } from './builtin_action_types/lib/result_type'; import { ActionsKibanaConfig, getActionsConfigurationUtilities, @@ -14,27 +15,33 @@ describe('isWhitelistedHostname', () => { test('returns true when "any" hostnames are allowed', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; expect( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' + unsafeGet( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) ) - ).toEqual(true); + ).toEqual('https://github.com/elastic/kibana'); }); test('returns false when the hostname in the requested uri is not in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; - expect( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' + expect(() => + unsafeGet( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) ) - ).toEqual(false); + ).toThrowErrorMatchingInlineSnapshot(`"target url not in whitelist"`); }); test('returns true when the hostname in the requested uri is in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; expect( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' + unsafeGet( + getActionsConfigurationUtilities(config).isWhitelistedHostname( + 'https://github.com/elastic/kibana' + ) ) - ).toEqual(true); + ).toEqual('https://github.com/elastic/kibana'); }); }); diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 3bfb560121b9c..2085842435aa1 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -6,6 +6,7 @@ import { fromNullable } from 'fp-ts/lib/Option'; import { URL } from 'url'; +import { asOk, asErr, Result } from './builtin_action_types/lib/result_type'; export enum WhitelistedHosts { Any = '*', @@ -17,25 +18,26 @@ export interface ActionsKibanaConfig { } export interface ActionsConfigurationUtilities { - isWhitelistedHostname: (uri: string) => boolean; + isWhitelistedHostname: (uri: string) => Result; } +const whitelistingError = asErr('target url not in whitelist'); export function getActionsConfigurationUtilities( config: ActionsKibanaConfig ): ActionsConfigurationUtilities { return { - isWhitelistedHostname(uri: string): boolean { + isWhitelistedHostname(uri: string): Result { + const urlHostname = new URL(uri).hostname; switch (config.whitelistedHosts) { case WhitelistedHosts.Any: - return true; + return asOk(uri); default: if (Array.isArray(config.whitelistedHosts)) { - const urlHostname = new URL(uri).hostname; - return fromNullable( - config.whitelistedHosts.find(hostname => hostname === urlHostname) - ).isSome(); + return fromNullable(config.whitelistedHosts.find(hostname => hostname === urlHostname)) + .map(_ => asOk(uri) as Result) + .getOrElse(whitelistingError as Result); } - return false; + return whitelistingError; } }, }; diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts index aac93f07d7c50..2eee1921f2067 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts @@ -42,6 +42,13 @@ export function isErr(result: Result): result is Err { return !isOk(result); } +export function unsafeGet(result: Result): T { + if (isErr(result)) { + throw new Error(`${result.error}`); + } + return result.value; +} + export async function promiseResult(future: Promise): Promise> { try { return asOk(await future); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index fa2999635a2f9..c38e89e3c1d5a 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -9,6 +9,7 @@ import { validateConfig, validateSecrets, validateParams } from '../lib'; import { ActionsConfigurationUtilities } from '../actions_config'; import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; import { Services, ActionTypeExecutorOptions } from '../types'; +import { asOk, asErr } from './lib/result_type'; const NO_OP_FN = () => {}; const servicesMock: Services = { @@ -18,8 +19,8 @@ const servicesMock: Services = { }; const configUtilsMock: ActionsConfigurationUtilities = { - isWhitelistedHostname() { - return true; + isWhitelistedHostname(uri) { + return asOk(uri); }, }; @@ -148,8 +149,8 @@ describe('config validation', () => { test('config validation passes when kibana config whitelists the url', () => { const actionType = getActionType({ - isWhitelistedHostname() { - return true; + isWhitelistedHostname(uri) { + return asOk(uri); }, }); @@ -168,8 +169,8 @@ describe('config validation', () => { test('config validation returns an error if the specified URL isnt whitelisted', () => { const actionType = getActionType({ - isWhitelistedHostname() { - return false; + isWhitelistedHostname(uri) { + return asErr(`target url is not whitelisted`); }, }); @@ -183,7 +184,7 @@ describe('config validation', () => { expect(() => { validateConfig(actionType, config); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: an error occurred configuring webhook with unwhitelisted target url \\"http://mylisteningserver.com:9200/endpoint\\""` + `"error validating action type config: error configuring webhook: target url is not whitelisted"` ); }); }); @@ -218,9 +219,9 @@ describe('executor', () => { const params: Record = {}; const result = await executor( { - isWhitelistedHostname: uri => { + isWhitelistedHostname(uri) { expect(uri).toEqual(config.url); - return false; + return asErr(`${uri}`); }, }, { @@ -232,7 +233,7 @@ describe('executor', () => { } as ActionTypeExecutorOptions ); expect(result.message).toMatchInlineSnapshot( - `"an error occurred in action \\"webhook\\" calling a remote webhook: You are not permitted to trigger this webhook"` + `"an error occurred in action \\"webhook\\" calling a remote webhook: http://mylisteningserver.com:9200/endpoint"` ); expect(result.status).toEqual('error'); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 49bdc508e92da..17f35b9e4f448 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -9,7 +9,7 @@ import axios, { AxiosError, AxiosResponse } from 'axios'; import { schema, TypeOf } from '@kbn/config-schema'; import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header'; import { nullableType } from './lib/nullable'; -import { isOk, promiseResult, Result } from './lib/result_type'; +import { isOk, isErr, promiseResult, Result } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; @@ -52,17 +52,14 @@ export function getActionType(configurationUtilities: ActionsConfigurationUtilit config: schema.object(configSchemaProps, { validate: configObject => { const { url }: ActionTypeConfigType = configObject; - if (!configurationUtilities.isWhitelistedHostname(url)) { - return i18n.translate( - 'xpack.actions.builtin.webhook.unwhitelistedWebhookConfigurationError', - { - defaultMessage: - 'an error occurred configuring webhook with unwhitelisted target url "{url}"', - values: { - url, - }, - } - ); + const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); + if (isErr(whitelistValidation)) { + return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { + defaultMessage: 'error configuring webhook: {message}', + values: { + message: whitelistValidation.error, + }, + }); } }, }), @@ -85,10 +82,10 @@ export async function executor( const { method, url, headers = {} } = execOptions.config as ActionTypeConfigType; const { user: username, password } = execOptions.secrets as ActionTypeSecretsType; const { body: data } = execOptions.params as ActionParamsType; - - if (!configurationUtilities.isWhitelistedHostname(url)) { - log(`warn`, `error on ${id} webhook event: The target "${url}" ahs not been whitelisted`); - return errorRequestInvalid(id); + const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); + if (isErr(whitelistValidation)) { + log(`warn`, `error on ${id} webhook event: The target "${url}" has not been whitelisted`); + return errorRequestInvalid(id, whitelistValidation.error); } const result: Result = await promiseResult( @@ -147,11 +144,11 @@ function successResult(data: any): ActionTypeExecutorResult { return { status: 'ok', data }; } -function errorRequestInvalid(id: string): ActionTypeExecutorResult { +function errorRequestInvalid(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidRequestErrorMessage', { - defaultMessage: - 'an error occurred in action "{id}" calling a remote webhook: You are not permitted to trigger this webhook', + defaultMessage: 'an error occurred in action "{id}" calling a remote webhook: {message}', values: { + message, id, }, }); From 4855bb028bcd33cf63fb7052365cb559b539b79a Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 11:37:31 +0100 Subject: [PATCH 06/17] refactor(webhook-whitelisting): curry valdiaiton to make it a little easier to read --- .../server/builtin_action_types/webhook.ts | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 17f35b9e4f448..f9a3bd457e900 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -50,18 +50,7 @@ export function getActionType(configurationUtilities: ActionsConfigurationUtilit name: 'webhook', validate: { config: schema.object(configSchemaProps, { - validate: configObject => { - const { url }: ActionTypeConfigType = configObject; - const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); - if (isErr(whitelistValidation)) { - return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { - defaultMessage: 'error configuring webhook: {message}', - values: { - message: whitelistValidation.error, - }, - }); - } - }, + validate: curry(valdiateActionTypeConfig)(configurationUtilities), }), secrets: SecretsSchema, params: ParamsSchema, @@ -70,6 +59,22 @@ export function getActionType(configurationUtilities: ActionsConfigurationUtilit }; } +function valdiateActionTypeConfig( + configurationUtilities: ActionsConfigurationUtilities, + configObject: ActionTypeConfigType +) { + const { url } = configObject; + const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); + if (isErr(whitelistValidation)) { + return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { + defaultMessage: 'error configuring webhook: {message}', + values: { + message: whitelistValidation.error, + }, + }); + } +} + // action executor export async function executor( configurationUtilities: ActionsConfigurationUtilities, From cd873871b921e0b6fea8d139cfde78984e47946c Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 11:45:11 +0100 Subject: [PATCH 07/17] fix(webhook-whitelisting): removed unused import --- x-pack/legacy/plugins/actions/server/actions_config.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index 9e60662a85073..be0bbd6694fa8 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { isOk, unsafeGet } from './builtin_action_types/lib/result_type'; +import { unsafeGet } from './builtin_action_types/lib/result_type'; import { ActionsKibanaConfig, getActionsConfigurationUtilities, From 057a6ece99fb4a7c065c561f9579f5f4a75ecf05 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 11:54:47 +0100 Subject: [PATCH 08/17] fix(webhook-whitelisting): cleaned up messaging around webhook errors --- .../builtin_action_types/webhook.test.ts | 4 ++-- .../server/builtin_action_types/webhook.ts | 22 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index c38e89e3c1d5a..23c74d3c13bcd 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -184,7 +184,7 @@ describe('config validation', () => { expect(() => { validateConfig(actionType, config); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: error configuring webhook: target url is not whitelisted"` + `"error validating action type config: error configuring webhook action: target url is not whitelisted"` ); }); }); @@ -233,7 +233,7 @@ describe('executor', () => { } as ActionTypeExecutorOptions ); expect(result.message).toMatchInlineSnapshot( - `"an error occurred in action \\"webhook\\" calling a remote webhook: http://mylisteningserver.com:9200/endpoint"` + `"an error occurred in webhook action \\"webhook\\" calling a remote webhook: http://mylisteningserver.com:9200/endpoint"` ); expect(result.status).toEqual('error'); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index f9a3bd457e900..64d60c5073d68 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -67,7 +67,7 @@ function valdiateActionTypeConfig( const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); if (isErr(whitelistValidation)) { return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { - defaultMessage: 'error configuring webhook: {message}', + defaultMessage: 'error configuring webhook action: {message}', values: { message: whitelistValidation.error, }, @@ -89,7 +89,7 @@ export async function executor( const { body: data } = execOptions.params as ActionParamsType; const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); if (isErr(whitelistValidation)) { - log(`warn`, `error on ${id} webhook event: The target "${url}" has not been whitelisted`); + log(`warn`, `error on webhook action "${id}": The target "${url}" has not been whitelisted`); return errorRequestInvalid(id, whitelistValidation.error); } @@ -110,7 +110,7 @@ export async function executor( const { value: { status, statusText }, } = result; - log('debug', `response from ${id} webhook event: [HTTP ${status}] ${statusText}`); + log('debug', `response from webhook action "${id}": [HTTP ${status}] ${statusText}`); return successResult(data); } else { @@ -139,7 +139,7 @@ export async function executor( const message = i18n.translate('xpack.actions.builtin.webhook.unreachableRemoteWebhook', { defaultMessage: 'Unreachable Remote Webhook, are you sure the address is correct?', }); - log(`warn`, `error on ${id} webhook event: ${message}`); + log(`warn`, `error on ${id} webhook action: ${message}`); return errorResultUnreachable(id, message); } } @@ -151,7 +151,8 @@ function successResult(data: any): ActionTypeExecutorResult { function errorRequestInvalid(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidRequestErrorMessage', { - defaultMessage: 'an error occurred in action "{id}" calling a remote webhook: {message}', + defaultMessage: + 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', values: { message, id, @@ -165,7 +166,8 @@ function errorRequestInvalid(id: string, message: string): ActionTypeExecutorRes function errorResultInvalid(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidResponseErrorMessage', { - defaultMessage: 'an error occurred in action "{id}" calling a remote webhook: {message}', + defaultMessage: + 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', values: { id, message, @@ -179,7 +181,8 @@ function errorResultInvalid(id: string, message: string): ActionTypeExecutorResu function errorResultUnreachable(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.unreachableErrorMessage', { - defaultMessage: 'an error occurred in action "{id}" calling a remote webhook: {message}', + defaultMessage: + 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', values: { id, message, @@ -195,7 +198,8 @@ function retryResult(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate( 'xpack.actions.builtin.webhook.invalidResponseRetryLaterErrorMessage', { - defaultMessage: 'an error occurred in action "{id}" calling a remote webhook, retry later', + defaultMessage: + 'an error occurred in webhook action "{id}" calling a remote webhook, retry later', values: { id, }, @@ -221,7 +225,7 @@ function retryResultSeconds( 'xpack.actions.builtin.webhook.invalidResponseRetryDateErrorMessage', { defaultMessage: - 'an error occurred in action "{id}" calling a remote webhook, retry at {retryString}: {message}', + 'an error occurred in webhook action "{id}" calling a remote webhook, retry at {retryString}: {message}', values: { id, retryString, From fad46ab8fd03f441b5e7f09fdaf3dd62384019ab Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 13:35:12 +0100 Subject: [PATCH 09/17] fix(webhook-whitelisting): fixed typing of mocks --- .../actions/server/builtin_action_types/email.test.ts | 5 ++++- .../actions/server/builtin_action_types/es_index.test.ts | 5 ++++- .../actions/server/builtin_action_types/pagerduty.test.ts | 5 ++++- .../actions/server/builtin_action_types/server_log.test.ts | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts index 92a0c59a35172..e723c77d07cfb 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts @@ -8,6 +8,7 @@ jest.mock('./lib/send_email', () => ({ sendEmail: jest.fn(), })); +import { asOk } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -23,7 +24,9 @@ const sendEmailMock = sendEmail as jest.Mock; const ACTION_TYPE_ID = '.email'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG = { + isWhitelistedHostname: uri => asOk(uri), +} as ActionsConfigurationUtilities; const services = { log: NO_OP_FN, diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts index 42078f39379fc..c70431f712076 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts @@ -8,6 +8,7 @@ jest.mock('./lib/send_email', () => ({ sendEmail: jest.fn(), })); +import { asOk } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -20,7 +21,9 @@ import { ActionParamsType, ActionTypeConfigType } from './es_index'; const ACTION_TYPE_ID = '.index'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG = { + isWhitelistedHostname: uri => asOk(uri), +} as ActionsConfigurationUtilities; const services = { log: NO_OP_FN, diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts index 5dbf94b358034..ed48843ebf2f5 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts @@ -8,6 +8,7 @@ jest.mock('./lib/post_pagerduty', () => ({ postPagerduty: jest.fn(), })); +import { asOk } from './lib/result_type'; import { ActionType, Services, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -22,7 +23,9 @@ const postPagerdutyMock = postPagerduty as jest.Mock; const ACTION_TYPE_ID = '.pagerduty'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG = { + isWhitelistedHostname: uri => asOk(uri), +} as ActionsConfigurationUtilities; const services: Services = { log: NO_OP_FN, diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts index ea2d01894e4c4..285767ecd02ba 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { asOk } from './lib/result_type'; import { ActionType, Services } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -16,7 +17,9 @@ import { registerBuiltInActionTypes } from './index'; const ACTION_TYPE_ID = '.server-log'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { isWhitelistedHostname: () => true } as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG = { + isWhitelistedHostname: uri => asOk(uri), +} as ActionsConfigurationUtilities; const services: Services = { log: NO_OP_FN, From 7d8ed088dca57627cf158f84fa7ad57fdfb39b6f Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Mon, 2 Sep 2019 13:59:15 +0100 Subject: [PATCH 10/17] readme(webhook-whitelisting): added documentation of the Built-in-Action configuration options --- x-pack/legacy/plugins/actions/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/x-pack/legacy/plugins/actions/README.md b/x-pack/legacy/plugins/actions/README.md index 483d3e8b0739d..a04ff8b1241cd 100644 --- a/x-pack/legacy/plugins/actions/README.md +++ b/x-pack/legacy/plugins/actions/README.md @@ -23,6 +23,27 @@ action types. 2. Create an action by using the RESTful API (see actions -> create action). 3. Use alerts to execute actions or execute manually (see firing actions). +## Kibana Actions Configuration +Implemented under the [Actions Config](./server/actions_config.ts). + +### Configuration Options + +Built-In-Actions are configured using the _xpack.actions_ namespoace under _kibana.yml_, and have the following configuration options: + +| Namespaced Key | Description | Type | +| ------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | +| _xpack.actions._**enabled** | ? | boolean | +| _xpack.actions._**WhitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" (instead of the array) to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | "\*" or Array | + +### Configuration Utilities + +This module provides a Utilities for interacting with the configuration. + +| Method | Arguments | Description | Return Type | +| --------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| isWhitelistedHostname | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checkes the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts. If the configuration says that all URI's are whitelisted (using an "\*") then it will always returns an _Ok_. | Result: Returns a [Result type](./server/builtin_action_types/lib/result_type.ts) where an _Ok_ is returns with the whitelisted URI (if it is whitelisted) or an _Err_ with an error message if it isn't whitelisted. | + + ## Action types ### Methods From 7375745fbf6fe764641efd8fcabde9bf599d8de2 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Tue, 3 Sep 2019 16:19:42 +0100 Subject: [PATCH 11/17] refactor(webhook-whitelisting): extracted unsafeGet out of Result type as it should only be used in tests --- .../legacy/plugins/actions/server/actions_config.test.ts | 9 ++++++++- .../server/builtin_action_types/lib/result_type.ts | 7 ------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index be0bbd6694fa8..01095eec1e939 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -4,13 +4,20 @@ * you may not use this file except in compliance with the Elastic License. */ -import { unsafeGet } from './builtin_action_types/lib/result_type'; +import { isErr } from './builtin_action_types/lib/result_type'; import { ActionsKibanaConfig, getActionsConfigurationUtilities, WhitelistedHosts, } from './actions_config'; +function unsafeGet(result: Result): T { + if (isErr(result)) { + throw new Error(`${result.error}`); + } + return result.value; +} + describe('isWhitelistedHostname', () => { test('returns true when "any" hostnames are allowed', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts index 2eee1921f2067..aac93f07d7c50 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/lib/result_type.ts @@ -42,13 +42,6 @@ export function isErr(result: Result): result is Err { return !isOk(result); } -export function unsafeGet(result: Result): T { - if (isErr(result)) { - throw new Error(`${result.error}`); - } - return result.value; -} - export async function promiseResult(future: Promise): Promise> { try { return asOk(await future); From d5baa69b7186a892b0a8c28a6b71d3f9fa6487c0 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Wed, 4 Sep 2019 09:59:27 +0100 Subject: [PATCH 12/17] refactor(webhook-whitelisting): removed usage of result in whitelisting --- .../actions/server/actions_config.test.ts | 65 +++++++++++-------- .../plugins/actions/server/actions_config.ts | 58 ++++++++++++----- .../server/builtin_action_types/email.test.ts | 4 +- .../builtin_action_types/es_index.test.ts | 4 +- .../builtin_action_types/pagerduty.test.ts | 4 +- .../builtin_action_types/server_log.test.ts | 4 +- .../builtin_action_types/webhook.test.ts | 23 +++---- .../server/builtin_action_types/webhook.ts | 16 ++--- 8 files changed, 106 insertions(+), 72 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index 01095eec1e939..39cc623e9a09c 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -4,51 +4,62 @@ * you may not use this file except in compliance with the Elastic License. */ -import { isErr } from './builtin_action_types/lib/result_type'; import { ActionsKibanaConfig, getActionsConfigurationUtilities, WhitelistedHosts, + NotWhitelistedError, } from './actions_config'; -function unsafeGet(result: Result): T { - if (isErr(result)) { - throw new Error(`${result.error}`); - } - return result.value; -} - -describe('isWhitelistedHostname', () => { +describe('isWhitelistedUri', () => { test('returns true when "any" hostnames are allowed', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; expect( - unsafeGet( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' - ) - ) + getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') ).toEqual('https://github.com/elastic/kibana'); }); - test('returns false when the hostname in the requested uri is not in the whitelist', () => { + test('returns a NotWhitelistedError when the hostname in the requested uri is not in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; - expect(() => - unsafeGet( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' - ) - ) - ).toThrowErrorMatchingInlineSnapshot(`"target url not in whitelist"`); + expect( + getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') + ).toEqual(new NotWhitelistedError('target url not in whitelist')); + }); + + test('returns a NotWhitelistedError when the uri cannot be parsed as a valid URI', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect(getActionsConfigurationUtilities(config).isWhitelistedUri('github.com/elastic')).toEqual( + new NotWhitelistedError('target url not in whitelist') + ); }); test('returns true when the hostname in the requested uri is in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; expect( - unsafeGet( - getActionsConfigurationUtilities(config).isWhitelistedHostname( - 'https://github.com/elastic/kibana' - ) - ) + getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') ).toEqual('https://github.com/elastic/kibana'); }); }); + +describe('isWhitelistedHostname', () => { + test('returns true when "any" hostnames are allowed', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; + expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( + 'github.com' + ); + }); + + test('returns false when the hostname in the requested uri is not in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( + new NotWhitelistedError('target url not in whitelist') + ); + }); + + test('returns true when the hostname in the requested uri is in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; + expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( + 'github.com' + ); + }); +}); diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 2085842435aa1..8c2dae9e6d9a2 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { fromNullable } from 'fp-ts/lib/Option'; +import { tryCatch } from 'fp-ts/lib/Option'; import { URL } from 'url'; -import { asOk, asErr, Result } from './builtin_action_types/lib/result_type'; +import { curry } from 'lodash'; export enum WhitelistedHosts { Any = '*', @@ -17,28 +17,54 @@ export interface ActionsKibanaConfig { whitelistedHosts: WhitelistedHosts.Any | string[]; } +export class NotWhitelistedError extends Error { + constructor(message: string) { + super(message); // 'Error' breaks prototype chain here + Object.setPrototypeOf(this, new.target.prototype); // restore prototype chain + } +} + export interface ActionsConfigurationUtilities { - isWhitelistedHostname: (uri: string) => Result; + isWhitelistedHostname: (hostname: string) => string | NotWhitelistedError; + isWhitelistedUri: (uri: string) => string | NotWhitelistedError; } -const whitelistingError = asErr('target url not in whitelist'); +const whitelistingErrorMessage = 'target url not in whitelist'; +function isWhitelisted( + config: ActionsKibanaConfig, + hostname: string +): string | NotWhitelistedError { + switch (config.whitelistedHosts) { + case WhitelistedHosts.Any: + return hostname; + default: + if ( + Array.isArray(config.whitelistedHosts) && + config.whitelistedHosts.find(whitelistedHostname => whitelistedHostname === hostname) + ) { + return hostname; + } + return new NotWhitelistedError(whitelistingErrorMessage); + } +} export function getActionsConfigurationUtilities( config: ActionsKibanaConfig ): ActionsConfigurationUtilities { + const isWhitelistedHostname = curry(isWhitelisted)(config); return { - isWhitelistedHostname(uri: string): Result { - const urlHostname = new URL(uri).hostname; - switch (config.whitelistedHosts) { - case WhitelistedHosts.Any: - return asOk(uri); - default: - if (Array.isArray(config.whitelistedHosts)) { - return fromNullable(config.whitelistedHosts.find(hostname => hostname === urlHostname)) - .map(_ => asOk(uri) as Result) - .getOrElse(whitelistingError as Result); + isWhitelistedUri(uri: string): string | NotWhitelistedError { + return tryCatch(() => new URL(uri)) + .map(url => url.hostname) + .map(hostname => { + const result = isWhitelistedHostname(hostname); + if (result instanceof NotWhitelistedError) { + return result; + } else { + return uri; } - return whitelistingError; - } + }) + .getOrElse(new NotWhitelistedError(whitelistingErrorMessage)); }, + isWhitelistedHostname, }; } diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts index e723c77d07cfb..4f3fbaecc3bd3 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts @@ -8,7 +8,6 @@ jest.mock('./lib/send_email', () => ({ sendEmail: jest.fn(), })); -import { asOk } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -25,7 +24,8 @@ const sendEmailMock = sendEmail as jest.Mock; const ACTION_TYPE_ID = '.email'; const NO_OP_FN = () => {}; const MOCK_KIBANA_CONFIG = { - isWhitelistedHostname: uri => asOk(uri), + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, } as ActionsConfigurationUtilities; const services = { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts index c70431f712076..3d2afc97ece04 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts @@ -8,7 +8,6 @@ jest.mock('./lib/send_email', () => ({ sendEmail: jest.fn(), })); -import { asOk } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -22,7 +21,8 @@ import { ActionParamsType, ActionTypeConfigType } from './es_index'; const ACTION_TYPE_ID = '.index'; const NO_OP_FN = () => {}; const MOCK_KIBANA_CONFIG = { - isWhitelistedHostname: uri => asOk(uri), + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, } as ActionsConfigurationUtilities; const services = { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts index ed48843ebf2f5..f6bc0250d2dc4 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts @@ -8,7 +8,6 @@ jest.mock('./lib/post_pagerduty', () => ({ postPagerduty: jest.fn(), })); -import { asOk } from './lib/result_type'; import { ActionType, Services, ActionTypeExecutorOptions } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -24,7 +23,8 @@ const postPagerdutyMock = postPagerduty as jest.Mock; const ACTION_TYPE_ID = '.pagerduty'; const NO_OP_FN = () => {}; const MOCK_KIBANA_CONFIG = { - isWhitelistedHostname: uri => asOk(uri), + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, } as ActionsConfigurationUtilities; const services: Services = { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts index 285767ecd02ba..3876de96c286d 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import { asOk } from './lib/result_type'; import { ActionType, Services } from '../types'; import { ActionsConfigurationUtilities } from '../actions_config'; import { ActionTypeRegistry } from '../action_type_registry'; @@ -18,7 +17,8 @@ import { registerBuiltInActionTypes } from './index'; const ACTION_TYPE_ID = '.server-log'; const NO_OP_FN = () => {}; const MOCK_KIBANA_CONFIG = { - isWhitelistedHostname: uri => asOk(uri), + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, } as ActionsConfigurationUtilities; const services: Services = { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index 23c74d3c13bcd..56d1c73a245c4 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -6,10 +6,9 @@ import { getActionType, executor } from './webhook'; import { validateConfig, validateSecrets, validateParams } from '../lib'; -import { ActionsConfigurationUtilities } from '../actions_config'; +import { ActionsConfigurationUtilities, NotWhitelistedError } from '../actions_config'; import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; import { Services, ActionTypeExecutorOptions } from '../types'; -import { asOk, asErr } from './lib/result_type'; const NO_OP_FN = () => {}; const servicesMock: Services = { @@ -19,9 +18,8 @@ const servicesMock: Services = { }; const configUtilsMock: ActionsConfigurationUtilities = { - isWhitelistedHostname(uri) { - return asOk(uri); - }, + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, }; describe('actionType', () => { @@ -149,9 +147,8 @@ describe('config validation', () => { test('config validation passes when kibana config whitelists the url', () => { const actionType = getActionType({ - isWhitelistedHostname(uri) { - return asOk(uri); - }, + isWhitelistedUri: _ => _, + isWhitelistedHostname: _ => _, }); const config: Record = { @@ -169,9 +166,8 @@ describe('config validation', () => { test('config validation returns an error if the specified URL isnt whitelisted', () => { const actionType = getActionType({ - isWhitelistedHostname(uri) { - return asErr(`target url is not whitelisted`); - }, + isWhitelistedUri: _ => new NotWhitelistedError(`target url is not whitelisted`), + isWhitelistedHostname: _ => _, }); const config: Record = { @@ -219,10 +215,11 @@ describe('executor', () => { const params: Record = {}; const result = await executor( { - isWhitelistedHostname(uri) { + isWhitelistedUri(uri) { expect(uri).toEqual(config.url); - return asErr(`${uri}`); + return new NotWhitelistedError(`${uri}`); }, + isWhitelistedHostname: _ => _, }, { id: 'webhook', diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 64d60c5073d68..d419bfeb17a47 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -9,9 +9,9 @@ import axios, { AxiosError, AxiosResponse } from 'axios'; import { schema, TypeOf } from '@kbn/config-schema'; import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header'; import { nullableType } from './lib/nullable'; -import { isOk, isErr, promiseResult, Result } from './lib/result_type'; +import { isOk, promiseResult, Result } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; -import { ActionsConfigurationUtilities } from '../actions_config'; +import { ActionsConfigurationUtilities, NotWhitelistedError } from '../actions_config'; // config definition enum WebhookMethods { @@ -64,12 +64,12 @@ function valdiateActionTypeConfig( configObject: ActionTypeConfigType ) { const { url } = configObject; - const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); - if (isErr(whitelistValidation)) { + const whitelistValidation = configurationUtilities.isWhitelistedUri(url); + if (whitelistValidation instanceof NotWhitelistedError) { return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { defaultMessage: 'error configuring webhook action: {message}', values: { - message: whitelistValidation.error, + message: whitelistValidation.message, }, }); } @@ -87,10 +87,10 @@ export async function executor( const { method, url, headers = {} } = execOptions.config as ActionTypeConfigType; const { user: username, password } = execOptions.secrets as ActionTypeSecretsType; const { body: data } = execOptions.params as ActionParamsType; - const whitelistValidation = configurationUtilities.isWhitelistedHostname(url); - if (isErr(whitelistValidation)) { + const whitelistValidation = configurationUtilities.isWhitelistedUri(url); + if (whitelistValidation instanceof NotWhitelistedError) { log(`warn`, `error on webhook action "${id}": The target "${url}" has not been whitelisted`); - return errorRequestInvalid(id, whitelistValidation.error); + return errorRequestInvalid(id, whitelistValidation.message); } const result: Result = await promiseResult( From ac21af95342e4242b1f051eb54f8bfe83a3396aa Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Wed, 4 Sep 2019 10:07:14 +0100 Subject: [PATCH 13/17] doc(webhook-whitelisting): Updated documentation for whitelisting --- x-pack/legacy/plugins/actions/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/legacy/plugins/actions/README.md b/x-pack/legacy/plugins/actions/README.md index a04ff8b1241cd..c319552f4e305 100644 --- a/x-pack/legacy/plugins/actions/README.md +++ b/x-pack/legacy/plugins/actions/README.md @@ -41,8 +41,8 @@ This module provides a Utilities for interacting with the configuration. | Method | Arguments | Description | Return Type | | --------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| isWhitelistedHostname | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checkes the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts. If the configuration says that all URI's are whitelisted (using an "\*") then it will always returns an _Ok_. | Result: Returns a [Result type](./server/builtin_action_types/lib/result_type.ts) where an _Ok_ is returns with the whitelisted URI (if it is whitelisted) or an _Err_ with an error message if it isn't whitelisted. | - +| isWhitelistedUri | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checkes the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts and returns the Uri if it is whitelisted. If the configuration says that all URI's are whitelisted (using an "\*") then it will always return the Uri. | String \| NotWhitelistedError: Returns a the whitelisted URI as is (if it is whitelisted) or a NotWhitelistedError Error with an error message if it isn't whitelisted. | +| isWhitelistedHostname | _hostname_: The Hostname you wish to validate is whitelisted | Validates whether the Hostname is whitelisted. This checkes the configuration and validates that the hostname is in the list of whitelisted Hosts and returns the Hostname if it is whitelisted. If the configuration says that all Hostnames are whitelisted (using an "\*") then it will always return the Hostname. | String \| NotWhitelistedError: Returns a the whitelisted URI as is (if it is whitelisted) or a NotWhitelistedError Error with an error message if it isn't whitelisted. | ## Action types From b27744595ebb29cc8abc19e98d6fac00f78a2edc Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Wed, 4 Sep 2019 10:26:25 +0100 Subject: [PATCH 14/17] refactor(webhook-whitelisting): Whitelist Any url by specifying a * in the whitelist config array --- x-pack/legacy/plugins/actions/README.md | 2 +- x-pack/legacy/plugins/actions/index.ts | 3 +-- .../actions/server/actions_config.test.ts | 10 ++++++-- .../plugins/actions/server/actions_config.ts | 25 ++++++++++--------- 4 files changed, 23 insertions(+), 17 deletions(-) diff --git a/x-pack/legacy/plugins/actions/README.md b/x-pack/legacy/plugins/actions/README.md index c319552f4e305..64e43ef5b977c 100644 --- a/x-pack/legacy/plugins/actions/README.md +++ b/x-pack/legacy/plugins/actions/README.md @@ -33,7 +33,7 @@ Built-In-Actions are configured using the _xpack.actions_ namespoace under _kiba | Namespaced Key | Description | Type | | ------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | | _xpack.actions._**enabled** | ? | boolean | -| _xpack.actions._**WhitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" (instead of the array) to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | "\*" or Array | +| _xpack.actions._**WhitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" in place of a specific hostname to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | Array | ### Configuration Utilities diff --git a/x-pack/legacy/plugins/actions/index.ts b/x-pack/legacy/plugins/actions/index.ts index d601fa50b8fb2..bb982dd59b5a6 100644 --- a/x-pack/legacy/plugins/actions/index.ts +++ b/x-pack/legacy/plugins/actions/index.ts @@ -31,8 +31,7 @@ export function actions(kibana: any) { .try( Joi.array() .items(Joi.string().hostname()) - .sparse(false), - Joi.string().valid('*') + .sparse(false) ) .default([]), }) diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index 39cc623e9a09c..b89c10e250c8c 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -13,7 +13,10 @@ import { describe('isWhitelistedUri', () => { test('returns true when "any" hostnames are allowed', () => { - const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; + const config: ActionsKibanaConfig = { + enabled: false, + whitelistedHosts: [WhitelistedHosts.Any], + }; expect( getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') ).toEqual('https://github.com/elastic/kibana'); @@ -43,7 +46,10 @@ describe('isWhitelistedUri', () => { describe('isWhitelistedHostname', () => { test('returns true when "any" hostnames are allowed', () => { - const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: WhitelistedHosts.Any }; + const config: ActionsKibanaConfig = { + enabled: false, + whitelistedHosts: [WhitelistedHosts.Any], + }; expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( 'github.com' ); diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 8c2dae9e6d9a2..546e6232ac86b 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -14,7 +14,7 @@ export enum WhitelistedHosts { export interface ActionsKibanaConfig { enabled: boolean; - whitelistedHosts: WhitelistedHosts.Any | string[]; + whitelistedHosts: string[]; } export class NotWhitelistedError extends Error { @@ -30,22 +30,23 @@ export interface ActionsConfigurationUtilities { } const whitelistingErrorMessage = 'target url not in whitelist'; +const doesValueWhitelistAnyHostname = (whitelistedHostname: string): boolean => + whitelistedHostname === WhitelistedHosts.Any; + function isWhitelisted( config: ActionsKibanaConfig, hostname: string ): string | NotWhitelistedError { - switch (config.whitelistedHosts) { - case WhitelistedHosts.Any: - return hostname; - default: - if ( - Array.isArray(config.whitelistedHosts) && - config.whitelistedHosts.find(whitelistedHostname => whitelistedHostname === hostname) - ) { - return hostname; - } - return new NotWhitelistedError(whitelistingErrorMessage); + if ( + Array.isArray(config.whitelistedHosts) && + config.whitelistedHosts.find( + whitelistedHostname => + doesValueWhitelistAnyHostname(whitelistedHostname) || whitelistedHostname === hostname + ) + ) { + return hostname; } + return new NotWhitelistedError(whitelistingErrorMessage); } export function getActionsConfigurationUtilities( config: ActionsKibanaConfig From 2699dd370dfe35b37b0d034a8278b46d43a76b20 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Wed, 4 Sep 2019 10:54:04 +0100 Subject: [PATCH 15/17] doc(webhook-whitelisting): Updated documentation for actions Enabled config --- x-pack/legacy/plugins/actions/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/legacy/plugins/actions/README.md b/x-pack/legacy/plugins/actions/README.md index 64e43ef5b977c..e8a8fffb4574c 100644 --- a/x-pack/legacy/plugins/actions/README.md +++ b/x-pack/legacy/plugins/actions/README.md @@ -32,7 +32,7 @@ Built-In-Actions are configured using the _xpack.actions_ namespoace under _kiba | Namespaced Key | Description | Type | | ------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------- | -| _xpack.actions._**enabled** | ? | boolean | +| _xpack.actions._**enabled** | Feature toggle which enabled Actions in Kibana. Currently defaulted to false while Actions are experimental. | boolean | | _xpack.actions._**WhitelistedHosts** | Which _hostnames_ are whitelisted for the Built-In-Action? This list should contain hostnames of every external service you wish to interact with using Webhooks, Email or any other built in Action. Note that you may use the string "\*" in place of a specific hostname to enable Kibana to target any URL, but keep in mind the potential use of such a feature to execute [SSRF](https://www.owasp.org/index.php/Server_Side_Request_Forgery) attacks from your server. | Array | ### Configuration Utilities From 9a84b8b77d41dc950f243da0d8f94d4ec37a87e9 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 5 Sep 2019 09:50:08 +0100 Subject: [PATCH 16/17] refactor(webhook-whitelisting): Provide two ways of checking whitelists, either throw or boolean --- x-pack/legacy/plugins/actions/README.md | 6 +- .../actions/server/actions_config.test.ts | 92 ++++++++++++++++--- .../plugins/actions/server/actions_config.ts | 92 +++++++++++-------- .../server/builtin_action_types/email.test.ts | 12 ++- .../builtin_action_types/es_index.test.ts | 12 ++- .../builtin_action_types/pagerduty.test.ts | 12 ++- .../builtin_action_types/server_log.test.ts | 12 ++- .../builtin_action_types/webhook.test.ts | 25 ++--- .../server/builtin_action_types/webhook.ts | 19 ++-- 9 files changed, 189 insertions(+), 93 deletions(-) diff --git a/x-pack/legacy/plugins/actions/README.md b/x-pack/legacy/plugins/actions/README.md index e8a8fffb4574c..ba847b18a104f 100644 --- a/x-pack/legacy/plugins/actions/README.md +++ b/x-pack/legacy/plugins/actions/README.md @@ -41,8 +41,10 @@ This module provides a Utilities for interacting with the configuration. | Method | Arguments | Description | Return Type | | --------------------- | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| isWhitelistedUri | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checkes the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts and returns the Uri if it is whitelisted. If the configuration says that all URI's are whitelisted (using an "\*") then it will always return the Uri. | String \| NotWhitelistedError: Returns a the whitelisted URI as is (if it is whitelisted) or a NotWhitelistedError Error with an error message if it isn't whitelisted. | -| isWhitelistedHostname | _hostname_: The Hostname you wish to validate is whitelisted | Validates whether the Hostname is whitelisted. This checkes the configuration and validates that the hostname is in the list of whitelisted Hosts and returns the Hostname if it is whitelisted. If the configuration says that all Hostnames are whitelisted (using an "\*") then it will always return the Hostname. | String \| NotWhitelistedError: Returns a the whitelisted URI as is (if it is whitelisted) or a NotWhitelistedError Error with an error message if it isn't whitelisted. | +| isWhitelistedUri | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checks the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts and returns `true` if it is whitelisted. If the configuration says that all URI's are whitelisted (using an "\*") then it will always return `true`. | Boolean | +| isWhitelistedHostname | _hostname_: The Hostname you wish to validate is whitelisted | Validates whether the Hostname is whitelisted. This checks the configuration and validates that the hostname is in the list of whitelisted Hosts and returns `true` if it is whitelisted. If the configuration says that all Hostnames are whitelisted (using an "\*") then it will always return `true`. | Boolean | +| ensureWhitelistedUri | _uri_: The URI you wish to validate is whitelisted | Validates whether the URI is whitelisted. This checks the configuration and validates that the hostname of the URI is in the list of whitelisted Hosts and throws an error if it is not whitelisted. If the configuration says that all URI's are whitelisted (using an "\*") then it will never throw. | No return value, throws if URI isn't whitelisted | +| ensureWhitelistedHostname | _hostname_: The Hostname you wish to validate is whitelisted | Validates whether the Hostname is whitelisted. This checks the configuration and validates that the hostname is in the list of whitelisted Hosts and throws an error if it is not whitelisted. If the configuration says that all Hostnames are whitelisted (using an "\*") then it will never throw | No return value, throws if Hostname isn't whitelisted | ## Action types diff --git a/x-pack/legacy/plugins/actions/server/actions_config.test.ts b/x-pack/legacy/plugins/actions/server/actions_config.test.ts index b89c10e250c8c..2fef6bfc539cc 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.test.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.test.ts @@ -8,9 +8,79 @@ import { ActionsKibanaConfig, getActionsConfigurationUtilities, WhitelistedHosts, - NotWhitelistedError, } from './actions_config'; +describe('ensureWhitelistedUri', () => { + test('returns true when "any" hostnames are allowed', () => { + const config: ActionsKibanaConfig = { + enabled: false, + whitelistedHosts: [WhitelistedHosts.Any], + }; + expect( + getActionsConfigurationUtilities(config).ensureWhitelistedUri( + 'https://github.com/elastic/kibana' + ) + ).toBeUndefined(); + }); + + test('throws when the hostname in the requested uri is not in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect(() => + getActionsConfigurationUtilities(config).ensureWhitelistedUri( + 'https://github.com/elastic/kibana' + ) + ).toThrowErrorMatchingInlineSnapshot( + `"target url \\"https://github.com/elastic/kibana\\" is not in the Kibana whitelist"` + ); + }); + + test('throws when the uri cannot be parsed as a valid URI', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect(() => + getActionsConfigurationUtilities(config).ensureWhitelistedUri('github.com/elastic') + ).toThrowErrorMatchingInlineSnapshot( + `"target url \\"github.com/elastic\\" is not in the Kibana whitelist"` + ); + }); + + test('returns true when the hostname in the requested uri is in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; + expect( + getActionsConfigurationUtilities(config).ensureWhitelistedUri( + 'https://github.com/elastic/kibana' + ) + ).toBeUndefined(); + }); +}); + +describe('ensureWhitelistedHostname', () => { + test('returns true when "any" hostnames are allowed', () => { + const config: ActionsKibanaConfig = { + enabled: false, + whitelistedHosts: [WhitelistedHosts.Any], + }; + expect( + getActionsConfigurationUtilities(config).ensureWhitelistedHostname('github.com') + ).toBeUndefined(); + }); + + test('throws when the hostname in the requested uri is not in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; + expect(() => + getActionsConfigurationUtilities(config).ensureWhitelistedHostname('github.com') + ).toThrowErrorMatchingInlineSnapshot( + `"target hostname \\"github.com\\" is not in the Kibana whitelist"` + ); + }); + + test('returns true when the hostname in the requested uri is in the whitelist', () => { + const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; + expect( + getActionsConfigurationUtilities(config).ensureWhitelistedHostname('github.com') + ).toBeUndefined(); + }); +}); + describe('isWhitelistedUri', () => { test('returns true when "any" hostnames are allowed', () => { const config: ActionsKibanaConfig = { @@ -19,20 +89,20 @@ describe('isWhitelistedUri', () => { }; expect( getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') - ).toEqual('https://github.com/elastic/kibana'); + ).toEqual(true); }); - test('returns a NotWhitelistedError when the hostname in the requested uri is not in the whitelist', () => { + test('throws when the hostname in the requested uri is not in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; expect( getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') - ).toEqual(new NotWhitelistedError('target url not in whitelist')); + ).toEqual(false); }); - test('returns a NotWhitelistedError when the uri cannot be parsed as a valid URI', () => { + test('throws when the uri cannot be parsed as a valid URI', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; expect(getActionsConfigurationUtilities(config).isWhitelistedUri('github.com/elastic')).toEqual( - new NotWhitelistedError('target url not in whitelist') + false ); }); @@ -40,7 +110,7 @@ describe('isWhitelistedUri', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; expect( getActionsConfigurationUtilities(config).isWhitelistedUri('https://github.com/elastic/kibana') - ).toEqual('https://github.com/elastic/kibana'); + ).toEqual(true); }); }); @@ -51,21 +121,21 @@ describe('isWhitelistedHostname', () => { whitelistedHosts: [WhitelistedHosts.Any], }; expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( - 'github.com' + true ); }); - test('returns false when the hostname in the requested uri is not in the whitelist', () => { + test('throws when the hostname in the requested uri is not in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: [] }; expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( - new NotWhitelistedError('target url not in whitelist') + false ); }); test('returns true when the hostname in the requested uri is in the whitelist', () => { const config: ActionsKibanaConfig = { enabled: false, whitelistedHosts: ['github.com'] }; expect(getActionsConfigurationUtilities(config).isWhitelistedHostname('github.com')).toEqual( - 'github.com' + true ); }); }); diff --git a/x-pack/legacy/plugins/actions/server/actions_config.ts b/x-pack/legacy/plugins/actions/server/actions_config.ts index 546e6232ac86b..6e2647ff7465e 100644 --- a/x-pack/legacy/plugins/actions/server/actions_config.ts +++ b/x-pack/legacy/plugins/actions/server/actions_config.ts @@ -4,7 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { tryCatch } from 'fp-ts/lib/Option'; +import { i18n } from '@kbn/i18n'; +import { tryCatch, fromNullable } from 'fp-ts/lib/Option'; import { URL } from 'url'; import { curry } from 'lodash'; @@ -12,60 +13,73 @@ export enum WhitelistedHosts { Any = '*', } +enum WhitelistingField { + url = 'url', + hostname = 'hostname', +} + export interface ActionsKibanaConfig { enabled: boolean; whitelistedHosts: string[]; } -export class NotWhitelistedError extends Error { - constructor(message: string) { - super(message); // 'Error' breaks prototype chain here - Object.setPrototypeOf(this, new.target.prototype); // restore prototype chain - } +export interface ActionsConfigurationUtilities { + isWhitelistedHostname: (hostname: string) => boolean; + isWhitelistedUri: (uri: string) => boolean; + ensureWhitelistedHostname: (hostname: string) => void; + ensureWhitelistedUri: (uri: string) => void; } -export interface ActionsConfigurationUtilities { - isWhitelistedHostname: (hostname: string) => string | NotWhitelistedError; - isWhitelistedUri: (uri: string) => string | NotWhitelistedError; +function whitelistingErrorMessage(field: WhitelistingField, value: string) { + return i18n.translate('xpack.actions.urlWhitelistConfigurationError', { + defaultMessage: 'target {field} "{value}" is not in the Kibana whitelist', + values: { + value, + field, + }, + }); +} + +function doesValueWhitelistAnyHostname(whitelistedHostname: string): boolean { + return whitelistedHostname === WhitelistedHosts.Any; } -const whitelistingErrorMessage = 'target url not in whitelist'; -const doesValueWhitelistAnyHostname = (whitelistedHostname: string): boolean => - whitelistedHostname === WhitelistedHosts.Any; +function isWhitelisted({ whitelistedHosts }: ActionsKibanaConfig, hostname: string): boolean { + return ( + Array.isArray(whitelistedHosts) && + fromNullable( + whitelistedHosts.find( + whitelistedHostname => + doesValueWhitelistAnyHostname(whitelistedHostname) || whitelistedHostname === hostname + ) + ).isSome() + ); +} -function isWhitelisted( - config: ActionsKibanaConfig, - hostname: string -): string | NotWhitelistedError { - if ( - Array.isArray(config.whitelistedHosts) && - config.whitelistedHosts.find( - whitelistedHostname => - doesValueWhitelistAnyHostname(whitelistedHostname) || whitelistedHostname === hostname - ) - ) { - return hostname; - } - return new NotWhitelistedError(whitelistingErrorMessage); +function isWhitelistedHostnameInUri(config: ActionsKibanaConfig, uri: string): boolean { + return tryCatch(() => new URL(uri)) + .map(url => url.hostname) + .mapNullable(hostname => isWhitelisted(config, hostname)) + .getOrElse(false); } + export function getActionsConfigurationUtilities( config: ActionsKibanaConfig ): ActionsConfigurationUtilities { const isWhitelistedHostname = curry(isWhitelisted)(config); + const isWhitelistedUri = curry(isWhitelistedHostnameInUri)(config); return { - isWhitelistedUri(uri: string): string | NotWhitelistedError { - return tryCatch(() => new URL(uri)) - .map(url => url.hostname) - .map(hostname => { - const result = isWhitelistedHostname(hostname); - if (result instanceof NotWhitelistedError) { - return result; - } else { - return uri; - } - }) - .getOrElse(new NotWhitelistedError(whitelistingErrorMessage)); - }, isWhitelistedHostname, + isWhitelistedUri, + ensureWhitelistedUri(uri: string) { + if (!isWhitelistedUri(uri)) { + throw new Error(whitelistingErrorMessage(WhitelistingField.url, uri)); + } + }, + ensureWhitelistedHostname(hostname: string) { + if (!isWhitelistedHostname(hostname)) { + throw new Error(whitelistingErrorMessage(WhitelistingField.hostname, hostname)); + } + }, }; } diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts index 4f3fbaecc3bd3..62d89b01fa313 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts @@ -23,10 +23,12 @@ const sendEmailMock = sendEmail as jest.Mock; const ACTION_TYPE_ID = '.email'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, -} as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG_UTILS: ActionsConfigurationUtilities = { + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, +}; const services = { log: NO_OP_FN, @@ -53,7 +55,7 @@ beforeAll(() => { getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG_UTILS); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts index 3d2afc97ece04..dba35ae73ca11 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/es_index.test.ts @@ -20,10 +20,12 @@ import { ActionParamsType, ActionTypeConfigType } from './es_index'; const ACTION_TYPE_ID = '.index'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, -} as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG_UTILS: ActionsConfigurationUtilities = { + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, +}; const services = { log: NO_OP_FN, @@ -50,7 +52,7 @@ beforeAll(() => { getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG_UTILS); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts index f6bc0250d2dc4..50b7567b09b54 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/pagerduty.test.ts @@ -22,10 +22,12 @@ const postPagerdutyMock = postPagerduty as jest.Mock; const ACTION_TYPE_ID = '.pagerduty'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, -} as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG_UTILS: ActionsConfigurationUtilities = { + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, +}; const services: Services = { log: NO_OP_FN, @@ -51,7 +53,7 @@ beforeAll(() => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG_UTILS); actionType = actionTypeRegistry.get(ACTION_TYPE_ID); }); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts index 3876de96c286d..95ff7d9b10a97 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/server_log.test.ts @@ -16,10 +16,12 @@ import { registerBuiltInActionTypes } from './index'; const ACTION_TYPE_ID = '.server-log'; const NO_OP_FN = () => {}; -const MOCK_KIBANA_CONFIG = { - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, -} as ActionsConfigurationUtilities; +const MOCK_KIBANA_CONFIG_UTILS: ActionsConfigurationUtilities = { + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, +}; const services: Services = { log: NO_OP_FN, @@ -44,7 +46,7 @@ beforeAll(() => { spaceIdToNamespace: jest.fn().mockReturnValue(undefined), getBasePath: jest.fn().mockReturnValue(undefined), }); - registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG); + registerBuiltInActionTypes(actionTypeRegistry, MOCK_KIBANA_CONFIG_UTILS); }); beforeEach(() => { diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index 56d1c73a245c4..ca66baa89b768 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -6,7 +6,7 @@ import { getActionType, executor } from './webhook'; import { validateConfig, validateSecrets, validateParams } from '../lib'; -import { ActionsConfigurationUtilities, NotWhitelistedError } from '../actions_config'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; import { Services, ActionTypeExecutorOptions } from '../types'; @@ -18,8 +18,10 @@ const servicesMock: Services = { }; const configUtilsMock: ActionsConfigurationUtilities = { - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, }; describe('actionType', () => { @@ -146,10 +148,7 @@ describe('config validation', () => { }); test('config validation passes when kibana config whitelists the url', () => { - const actionType = getActionType({ - isWhitelistedUri: _ => _, - isWhitelistedHostname: _ => _, - }); + const actionType = getActionType(configUtilsMock); const config: Record = { url: 'http://mylisteningserver.com:9200/endpoint', @@ -166,8 +165,10 @@ describe('config validation', () => { test('config validation returns an error if the specified URL isnt whitelisted', () => { const actionType = getActionType({ - isWhitelistedUri: _ => new NotWhitelistedError(`target url is not whitelisted`), - isWhitelistedHostname: _ => _, + ...configUtilsMock, + ensureWhitelistedUri: _ => { + throw new Error(`target url is not whitelisted`); + }, }); const config: Record = { @@ -215,11 +216,11 @@ describe('executor', () => { const params: Record = {}; const result = await executor( { - isWhitelistedUri(uri) { + ...configUtilsMock, + ensureWhitelistedUri(uri) { expect(uri).toEqual(config.url); - return new NotWhitelistedError(`${uri}`); + throw new Error(`${uri}`); }, - isWhitelistedHostname: _ => _, }, { id: 'webhook', diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index d419bfeb17a47..6ee8813485c63 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -11,7 +11,7 @@ import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_hea import { nullableType } from './lib/nullable'; import { isOk, promiseResult, Result } from './lib/result_type'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; -import { ActionsConfigurationUtilities, NotWhitelistedError } from '../actions_config'; +import { ActionsConfigurationUtilities } from '../actions_config'; // config definition enum WebhookMethods { @@ -63,13 +63,13 @@ function valdiateActionTypeConfig( configurationUtilities: ActionsConfigurationUtilities, configObject: ActionTypeConfigType ) { - const { url } = configObject; - const whitelistValidation = configurationUtilities.isWhitelistedUri(url); - if (whitelistValidation instanceof NotWhitelistedError) { + try { + configurationUtilities.ensureWhitelistedUri(configObject.url); + } catch (whitelistError) { return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', { defaultMessage: 'error configuring webhook action: {message}', values: { - message: whitelistValidation.message, + message: whitelistError.message, }, }); } @@ -87,10 +87,11 @@ export async function executor( const { method, url, headers = {} } = execOptions.config as ActionTypeConfigType; const { user: username, password } = execOptions.secrets as ActionTypeSecretsType; const { body: data } = execOptions.params as ActionParamsType; - const whitelistValidation = configurationUtilities.isWhitelistedUri(url); - if (whitelistValidation instanceof NotWhitelistedError) { - log(`warn`, `error on webhook action "${id}": The target "${url}" has not been whitelisted`); - return errorRequestInvalid(id, whitelistValidation.message); + try { + configurationUtilities.ensureWhitelistedUri(url); + } catch (whitelistError) { + log(`warn`, `error on webhook action "${id}": ${whitelistError.message}`); + return errorRequestInvalid(id, whitelistError.message); } const result: Result = await promiseResult( From 8a9ddc932a3fcbaa13ba98e6096877ea762bace8 Mon Sep 17 00:00:00 2001 From: Gidi Morris Date: Thu, 5 Sep 2019 10:23:28 +0100 Subject: [PATCH 17/17] refactor(webhook-whitelisting): Removed whitelisting check in executor as config is rechecked before the executor is called --- .../builtin_action_types/webhook.test.ts | 44 +------------------ .../server/builtin_action_types/webhook.ts | 29 ++---------- .../actions/builtin_action_types/webhook.ts | 22 +++++++++- 3 files changed, 26 insertions(+), 69 deletions(-) diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts index ca66baa89b768..e8674af672fcb 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.test.ts @@ -4,18 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getActionType, executor } from './webhook'; +import { getActionType } from './webhook'; import { validateConfig, validateSecrets, validateParams } from '../lib'; import { ActionsConfigurationUtilities } from '../actions_config'; -import { SavedObjectsClientMock } from '../../../../../../src/core/server/mocks'; -import { Services, ActionTypeExecutorOptions } from '../types'; - -const NO_OP_FN = () => {}; -const servicesMock: Services = { - log: NO_OP_FN, - callCluster: async (path: string, opts: any) => {}, - savedObjectsClient: SavedObjectsClientMock.create(), -}; const configUtilsMock: ActionsConfigurationUtilities = { isWhitelistedHostname: _ => true, @@ -203,36 +194,3 @@ describe('params validation', () => { }); }); }); - -describe('executor', () => { - test('webhook executor should fail if the url is not whitelisted', async () => { - const config: Record = { - url: 'http://mylisteningserver.com:9200/endpoint', - }; - const secrets: Record = { - user: 'bob', - password: 'supersecret', - }; - const params: Record = {}; - const result = await executor( - { - ...configUtilsMock, - ensureWhitelistedUri(uri) { - expect(uri).toEqual(config.url); - throw new Error(`${uri}`); - }, - }, - { - id: 'webhook', - services: servicesMock, - config, - secrets, - params, - } as ActionTypeExecutorOptions - ); - expect(result.message).toMatchInlineSnapshot( - `"an error occurred in webhook action \\"webhook\\" calling a remote webhook: http://mylisteningserver.com:9200/endpoint"` - ); - expect(result.status).toEqual('error'); - }); -}); diff --git a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts index 6ee8813485c63..c249345a4aada 100644 --- a/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts +++ b/x-pack/legacy/plugins/actions/server/builtin_action_types/webhook.ts @@ -87,12 +87,6 @@ export async function executor( const { method, url, headers = {} } = execOptions.config as ActionTypeConfigType; const { user: username, password } = execOptions.secrets as ActionTypeSecretsType; const { body: data } = execOptions.params as ActionParamsType; - try { - configurationUtilities.ensureWhitelistedUri(url); - } catch (whitelistError) { - log(`warn`, `error on webhook action "${id}": ${whitelistError.message}`); - return errorRequestInvalid(id, whitelistError.message); - } const result: Result = await promiseResult( axios.request({ @@ -150,25 +144,10 @@ function successResult(data: any): ActionTypeExecutorResult { return { status: 'ok', data }; } -function errorRequestInvalid(id: string, message: string): ActionTypeExecutorResult { - const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidRequestErrorMessage', { - defaultMessage: - 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', - values: { - message, - id, - }, - }); - return { - status: 'error', - message: errMessage, - }; -} - function errorResultInvalid(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.invalidResponseErrorMessage', { defaultMessage: - 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', + 'Invalid Response: an error occurred in webhook action "{id}" calling a remote webhook: {message}', values: { id, message, @@ -183,7 +162,7 @@ function errorResultInvalid(id: string, message: string): ActionTypeExecutorResu function errorResultUnreachable(id: string, message: string): ActionTypeExecutorResult { const errMessage = i18n.translate('xpack.actions.builtin.webhook.unreachableErrorMessage', { defaultMessage: - 'an error occurred in webhook action "{id}" calling a remote webhook: {message}', + 'Unreachable Webhook: an error occurred in webhook action "{id}" calling a remote webhook: {message}', values: { id, message, @@ -200,7 +179,7 @@ function retryResult(id: string, message: string): ActionTypeExecutorResult { 'xpack.actions.builtin.webhook.invalidResponseRetryLaterErrorMessage', { defaultMessage: - 'an error occurred in webhook action "{id}" calling a remote webhook, retry later', + 'Invalid Response: an error occurred in webhook action "{id}" calling a remote webhook, retry later', values: { id, }, @@ -226,7 +205,7 @@ function retryResultSeconds( 'xpack.actions.builtin.webhook.invalidResponseRetryDateErrorMessage', { defaultMessage: - 'an error occurred in webhook action "{id}" calling a remote webhook, retry at {retryString}: {message}', + 'Invalid Response: an error occurred in webhook action "{id}" calling a remote webhook, retry at {retryString}: {message}', values: { id, retryString, diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts index 495691cb386c6..89fc986fd0255 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts @@ -162,6 +162,27 @@ export default function webhookTest({ getService }: FtrProviderContext) { expect(result.status).to.eql('ok'); }); + it('should handle target webhooks that are not whitelisted', async () => { + const { body: result } = await supertest + .post('/api/action') + .set('kbn-xsrf', 'test') + .send({ + description: 'A generic Webhook action', + actionTypeId: '.webhook', + secrets: { + user: 'username', + password: 'mypassphrase', + }, + config: { + url: 'http://a.none.whitelisted.webhook/endpoint', + }, + }) + .expect(400); + + expect(result.error).to.eql('Bad Request'); + expect(result.message).to.match(/not in the Kibana whitelist/); + }); + it('should handle unreachable webhook targets', async () => { const webhookActionId = await createWebhookAction('http://some.non.existent.com/endpoint'); const { body: result } = await supertest @@ -177,7 +198,6 @@ export default function webhookTest({ getService }: FtrProviderContext) { expect(result.status).to.eql('error'); expect(result.message).to.match(/Unreachable Remote Webhook/); }); - it('should handle failing webhook targets', async () => { const webhookActionId = await createWebhookAction(webhookSimulatorURL); const { body: result } = await supertest