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 8e6b1f19b172c..513f51f644534 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,15 +8,30 @@ jest.mock('./lib/send_email', () => ({ sendEmail: jest.fn(), })); +import { Logger } from '../../../../../../src/core/server'; +import { savedObjectsClientMock } from '../../../../../../src/core/server/mocks'; + import { ActionType, ActionTypeExecutorOptions } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; import { validateConfig, validateSecrets, validateParams } from '../lib'; -import { savedObjectsClientMock } from '../../../../../../src/core/server/mocks'; import { createActionTypeRegistry } from './index.test'; import { sendEmail } from './lib/send_email'; -import { ActionParamsType, ActionTypeConfigType, ActionTypeSecretsType } from './email'; +import { + ActionParamsType, + ActionTypeConfigType, + ActionTypeSecretsType, + getActionType, +} from './email'; const sendEmailMock = sendEmail as jest.Mock; +const configUtilsMock: ActionsConfigurationUtilities = { + isWhitelistedHostname: _ => true, + isWhitelistedUri: _ => true, + ensureWhitelistedHostname: _ => {}, + ensureWhitelistedUri: _ => {}, +}; + const ACTION_TYPE_ID = '.email'; const NO_OP_FN = () => {}; @@ -27,6 +42,7 @@ const services = { }; let actionType: ActionType; +let mockedLogger: jest.Mocked; beforeAll(() => { const { actionTypeRegistry } = createActionTypeRegistry(); @@ -69,8 +85,6 @@ describe('config validation', () => { test('config validation fails when config is not valid', () => { const baseConfig: Record = { - user: 'bob', - password: 'supersecret', from: 'bob@example.com', }; @@ -85,21 +99,21 @@ describe('config validation', () => { expect(() => { validateConfig(actionType, baseConfig); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: [user]: definition for this key is missing"` + `"error validating action type config: either [service] or [host]/[port] is required"` ); // host but no port expect(() => { validateConfig(actionType, { ...baseConfig, host: 'elastic.co' }); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: [user]: definition for this key is missing"` + `"error validating action type config: [port] is required if [service] is not provided"` ); // port but no host expect(() => { validateConfig(actionType, { ...baseConfig, port: 8080 }); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: [user]: definition for this key is missing"` + `"error validating action type config: [host] is required if [service] is not provided"` ); // invalid service @@ -109,7 +123,64 @@ describe('config validation', () => { service: 'bad-nodemailer-service', }); }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type config: [user]: definition for this key is missing"` + `"error validating action type config: [service] value 'bad-nodemailer-service' is not valid"` + ); + }); + + // nodemailer supports a service named 'AOL' that maps to the host below + const NODEMAILER_AOL_SERVICE = 'AOL'; + const NODEMAILER_AOL_SERVICE_HOST = 'smtp.aol.com'; + + test('config validation handles email host whitelisting', () => { + actionType = getActionType({ + logger: mockedLogger, + configurationUtilities: { + ...configUtilsMock, + isWhitelistedHostname: hostname => hostname === NODEMAILER_AOL_SERVICE_HOST, + }, + }); + const baseConfig = { + from: 'bob@example.com', + }; + const whitelistedConfig1 = { + ...baseConfig, + service: NODEMAILER_AOL_SERVICE, + }; + const whitelistedConfig2 = { + ...baseConfig, + host: NODEMAILER_AOL_SERVICE_HOST, + port: 42, + }; + const notWhitelistedConfig1 = { + ...baseConfig, + service: 'gmail', + }; + + const notWhitelistedConfig2 = { + ...baseConfig, + host: 'smtp.gmail.com', + port: 42, + }; + + const validatedConfig1 = validateConfig(actionType, whitelistedConfig1); + expect(validatedConfig1.service).toEqual(whitelistedConfig1.service); + expect(validatedConfig1.from).toEqual(whitelistedConfig1.from); + + const validatedConfig2 = validateConfig(actionType, whitelistedConfig2); + expect(validatedConfig2.host).toEqual(whitelistedConfig2.host); + expect(validatedConfig2.port).toEqual(whitelistedConfig2.port); + expect(validatedConfig2.from).toEqual(whitelistedConfig2.from); + + expect(() => { + validateConfig(actionType, notWhitelistedConfig1); + }).toThrowErrorMatchingInlineSnapshot( + `"error validating action type config: [service] value 'gmail' resolves to host 'smtp.gmail.com' which is not in the whitelistedHosts configuration"` + ); + + expect(() => { + validateConfig(actionType, notWhitelistedConfig2); + }).toThrowErrorMatchingInlineSnapshot( + `"error validating action type config: [host] value 'smtp.gmail.com' is not in the whitelistedHosts configuration"` ); }); }); @@ -140,16 +211,16 @@ describe('params validation', () => { message: 'this is the message', }; expect(validateParams(actionType, params)).toMatchInlineSnapshot(` -Object { - "bcc": Array [], - "cc": Array [], - "message": "this is the message", - "subject": "this is a test", - "to": Array [ - "bob@example.com", - ], -} -`); + Object { + "bcc": Array [], + "cc": Array [], + "message": "this is the message", + "subject": "this is a test", + "to": Array [ + "bob@example.com", + ], + } + `); }); test('params validation fails when params is not valid', () => { @@ -194,29 +265,29 @@ describe('execute()', () => { sendEmailMock.mockReset(); await actionType.executor(executorOptions); expect(sendEmailMock.mock.calls[0][1]).toMatchInlineSnapshot(` - Object { - "content": Object { - "message": "a message to you", - "subject": "the subject", - }, - "routing": Object { - "bcc": Array [ - "jimmy@example.com", - ], - "cc": Array [ - "james@example.com", - ], - "from": "bob@example.com", - "to": Array [ - "jim@example.com", - ], - }, - "transport": Object { - "password": "supersecret", - "service": "__json", - "user": "bob", - }, - } -`); + Object { + "content": Object { + "message": "a message to you", + "subject": "the subject", + }, + "routing": Object { + "bcc": Array [ + "jimmy@example.com", + ], + "cc": Array [ + "james@example.com", + ], + "from": "bob@example.com", + "to": Array [ + "jim@example.com", + ], + }, + "transport": Object { + "password": "supersecret", + "service": "__json", + "user": "bob", + }, + } + `); }); }); 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 c68d9acd8b174..a378d8a4b9b55 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 @@ -7,31 +7,32 @@ import { curry } from 'lodash'; import { i18n } from '@kbn/i18n'; import { schema, TypeOf } from '@kbn/config-schema'; -import nodemailerServices from 'nodemailer/lib/well-known/services.json'; +import nodemailerGetService from 'nodemailer/lib/well-known'; import { sendEmail, JSON_TRANSPORT_SERVICE } from './lib/send_email'; import { nullableType } from './lib/nullable'; import { portSchema } from './lib/schemas'; import { Logger } from '../../../../../../src/core/server'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; +import { ActionsConfigurationUtilities } from '../actions_config'; // config definition export type ActionTypeConfigType = TypeOf; -const ConfigSchema = schema.object( - { - service: nullableType(schema.string()), - host: nullableType(schema.string()), - port: nullableType(portSchema()), - secure: nullableType(schema.boolean()), - from: schema.string(), - }, - { - validate: validateConfig, - } -); +const ConfigSchemaProps = { + service: nullableType(schema.string()), + host: nullableType(schema.string()), + port: nullableType(portSchema()), + secure: nullableType(schema.boolean()), + from: schema.string(), +}; -function validateConfig(configObject: any): string | void { +const ConfigSchema = schema.object(ConfigSchemaProps); + +function validateConfig( + configurationUtilities: ActionsConfigurationUtilities, + configObject: any +): string | void { // avoids circular reference ... const config: ActionTypeConfigType = configObject; @@ -40,7 +41,9 @@ function validateConfig(configObject: any): string | void { // Note, not currently making these message translated, as will be // emitted alongside messages from @kbn/config-schema, which does not // translate messages. - if (config.service == null) { + if (config.service === JSON_TRANSPORT_SERVICE) { + return; + } else if (config.service == null) { if (config.host == null && config.port == null) { return 'either [service] or [host]/[port] is required'; } @@ -52,10 +55,17 @@ function validateConfig(configObject: any): string | void { if (config.port == null) { return '[port] is required if [service] is not provided'; } + + if (!configurationUtilities.isWhitelistedHostname(config.host)) { + return `[host] value '${config.host}' is not in the whitelistedHosts configuration`; + } } else { - // service is not null - if (!isValidService(config.service)) { - return `[service] value "${config.service}" is not valid`; + const host = getServiceNameHost(config.service); + if (host == null) { + return `[service] value '${config.service}' is not valid`; + } + if (!configurationUtilities.isWhitelistedHostname(host)) { + return `[service] value '${config.service}' resolves to host '${host}' which is not in the whitelistedHosts configuration`; } } } @@ -98,13 +108,21 @@ function validateParams(paramsObject: any): string | void { } } +interface GetActionTypeParams { + logger: Logger; + configurationUtilities: ActionsConfigurationUtilities; +} + // action type definition -export function getActionType({ logger }: { logger: Logger }): ActionType { +export function getActionType(params: GetActionTypeParams): ActionType { + const { logger, configurationUtilities } = params; return { id: '.email', name: 'email', validate: { - config: ConfigSchema, + config: schema.object(ConfigSchemaProps, { + validate: curry(validateConfig)(configurationUtilities), + }), secrets: SecretsSchema, params: ParamsSchema, }, @@ -173,31 +191,14 @@ async function executor( // utilities -const ValidServiceNames = getValidServiceNames(); - -function isValidService(service: string): boolean { - return ValidServiceNames.has(service.toLowerCase()); -} - -function getValidServiceNames(): Set { - const result = new Set(); - - // add our special json service - result.add(JSON_TRANSPORT_SERVICE); +function getServiceNameHost(service: string): string | null { + const serviceEntry = nodemailerGetService(service); + if (serviceEntry === false) return null; - const keys = Object.keys(nodemailerServices) as string[]; - for (const key of keys) { - result.add(key.toLowerCase()); - - const record = nodemailerServices[key]; - if (record.aliases == null) continue; - - for (const alias of record.aliases as string[]) { - result.add(alias.toLowerCase()); - } - } + // in theory this won't happen, but it's JS, so just to be safe ... + if (serviceEntry == null) return null; - return result; + return serviceEntry.host || null; } // Returns the secure value - whether to use TLS or not. 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 be743c84f5dfe..0fe572bb57e41 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 @@ -26,7 +26,9 @@ export function registerBuiltInActionTypes({ }) { actionTypeRegistry.register(getServerLogActionType({ logger })); actionTypeRegistry.register(getSlackActionType()); - actionTypeRegistry.register(getEmailActionType({ logger })); + actionTypeRegistry.register( + getEmailActionType({ logger, configurationUtilities: actionsConfigUtils }) + ); actionTypeRegistry.register(getIndexActionType({ logger })); actionTypeRegistry.register(getPagerDutyActionType({ logger })); actionTypeRegistry.register( diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts index 2c1cb966c6b7e..de856492e12fc 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts @@ -153,5 +153,79 @@ export default function emailTest({ getService }: FtrProviderContext) { }); }); }); + + it('should respond with a 400 Bad Request when creating an email action with non-whitelisted server', async () => { + await supertest + .post('/api/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'An email action', + actionTypeId: '.email', + config: { + service: 'gmail', // not whitelisted in the config for this test + from: 'bob@example.com', + }, + secrets: { + user: 'bob', + password: 'changeme', + }, + }) + .expect(400) + .then((resp: any) => { + expect(resp.body).to.eql({ + statusCode: 400, + error: 'Bad Request', + message: + "error validating action type config: [service] value 'gmail' resolves to host 'smtp.gmail.com' which is not in the whitelistedHosts configuration", + }); + }); + + await supertest + .post('/api/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'An email action', + actionTypeId: '.email', + config: { + host: 'stmp.gmail.com', // not whitelisted in the config for this test + port: 666, + from: 'bob@example.com', + }, + secrets: { + user: 'bob', + password: 'changeme', + }, + }) + .expect(400) + .then((resp: any) => { + expect(resp.body).to.eql({ + statusCode: 400, + error: 'Bad Request', + message: + "error validating action type config: [host] value 'stmp.gmail.com' is not in the whitelistedHosts configuration", + }); + }); + }); + + it('should handle creating an email action with a whitelisted server', async () => { + const { body: createdAction } = await supertest + .post('/api/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'An email action', + actionTypeId: '.email', + config: { + host: 'some.non.existent.com', // whitelisted in the config for this test + port: 666, + from: 'bob@example.com', + }, + secrets: { + user: 'bob', + password: 'changeme', + }, + }) + .expect(200); + expect(typeof createdAction.id).to.be('string'); + }); }); } diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts index 46c3de18a1196..a8260d4771459 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/execute.ts @@ -331,8 +331,10 @@ export default function({ getService }: FtrProviderContext) { name: 'test email action', actionTypeId: '.email', config: { - from: 'email-from@example.com', - host: 'host-is-ignored-here.example.com', + from: 'email-from-1@example.com', + // this host is specifically whitelisted in: + // x-pack/test/alerting_api_integration/common/config.ts + host: 'some.non.existent.com', port: 666, }, secrets: { @@ -349,7 +351,7 @@ export default function({ getService }: FtrProviderContext) { .send({ name: 'a test email action 2', config: { - from: 'email-from@example.com', + from: 'email-from-2@example.com', service: '__json', }, secrets: {