Skip to content

Commit

Permalink
Whitelist email server in built-in email server action - second try (e…
Browse files Browse the repository at this point in the history 
…lastic#52221)

resolves elastic#50721

note this branch was previously merged into master and then reverted: elastic#51489 (prior PR made shape changes this one didn't take into account)

Uses the same whitelist config value / utilities that the webhook
action already uses. Was already mentioned in the README doc
that email uses this whitelist config value :-)

Required a change to the functional tests to use a host already
whitelisted in config, made for the the webhook action tests.

Also realized some jest tests on email were bogus, so fixed those
(was passing `user` in config, which is invalid, and masking the
actual thing being tested).
  • Loading branch information
@pmuellr
pmuellr committed Dec 4, 2019
1 parent 31c8f0c commit 5c4af30
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 89 deletions.
155 changes: 113 additions & 42 deletions x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {};

Expand All @@ -27,6 +42,7 @@ const services = {
};

let actionType: ActionType;
let mockedLogger: jest.Mocked<Logger>;

beforeAll(() => {
const { actionTypeRegistry } = createActionTypeRegistry();
Expand Down Expand Up @@ -69,8 +85,6 @@ describe('config validation', () => {

test('config validation fails when config is not valid', () => {
const baseConfig: Record<string, any> = {
user: 'bob',
password: 'supersecret',
from: '[email protected]',
};

Expand All @@ -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
Expand All @@ -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: '[email protected]',
};
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"`
);
});
});
Expand Down Expand Up @@ -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 [
"[email protected]",
],
}
`);
Object {
"bcc": Array [],
"cc": Array [],
"message": "this is the message",
"subject": "this is a test",
"to": Array [
"[email protected]",
],
}
`);
});

test('params validation fails when params is not valid', () => {
Expand Down Expand Up @@ -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 [
"[email protected]",
],
"cc": Array [
"[email protected]",
],
"from": "[email protected]",
"to": Array [
"[email protected]",
],
},
"transport": Object {
"password": "supersecret",
"service": "__json",
"user": "bob",
},
}
`);
Object {
"content": Object {
"message": "a message to you",
"subject": "the subject",
},
"routing": Object {
"bcc": Array [
"[email protected]",
],
"cc": Array [
"[email protected]",
],
"from": "[email protected]",
"to": Array [
"[email protected]",
],
},
"transport": Object {
"password": "supersecret",
"service": "__json",
"user": "bob",
},
}
`);
});
});
87 changes: 44 additions & 43 deletions x-pack/legacy/plugins/actions/server/builtin_action_types/email.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof ConfigSchema>;

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;

Expand All @@ -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';
}
Expand All @@ -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`;
}
}
}
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -173,31 +191,14 @@ async function executor(

// utilities

const ValidServiceNames = getValidServiceNames();

function isValidService(service: string): boolean {
return ValidServiceNames.has(service.toLowerCase());
}

function getValidServiceNames(): Set<string> {
const result = new Set<string>();

// 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.
Expand Down
4 changes: 3 additions & 1 deletion x-pack/legacy/plugins/actions/server/builtin_action_types/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 5c4af30

Please sign in to comment.