Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitelist email server in built-in email server action - second try #52221

Merged
merged 6 commits into from
Dec 4, 2019
Merged

Whitelist email server in built-in email server action - second try #52221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
918a6a3
Whitelist email server in built-in email server action
pmuellr Nov 22, 2019
41c99c8
apply changes from code review
pmuellr Nov 26, 2019
9694760
Merge branch 'master' into actions/email-whitelist
elasticmachine Dec 3, 2019
dd1ae2e
Merge branch 'master' into actions/email-whitelist
elasticmachine Dec 3, 2019
62522fc
Merge branch 'master' into actions/email-whitelist
elasticmachine Dec 4, 2019
6fc179e
change action prop description to name in email function tests
pmuellr Dec 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 113 additions & 42 deletions x-pack/legacy/plugins/actions/server/builtin_action_types/email.test.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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