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

resolves https://github.com/elastic/kibana/issues/50721

note this branch was previously merged into master and then reverted: https://github.com/elastic/kibana/pull/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).

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 { ActionType, ActionTypeExecutorOptions } from '../types';
-import { validateConfig, validateSecrets, validateParams } from '../lib';
+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 { 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<Logger>;
 
 beforeAll(() => {
   const { actionTypeRegistry } = createActionTypeRegistry();
@@ -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: '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",
+            },
+          }
+    `);
   });
 });

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<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;
 
@@ -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 getServiceNameHost(service: string): string | null {
+  const serviceEntry = nodemailerGetService(service);
+  if (serviceEntry === false) return null;
 
-function isValidService(service: string): boolean {
-  return ValidServiceNames.has(service.toLowerCase());
-}
+  // in theory this won't happen, but it's JS, so just to be safe ...
+  if (serviceEntry == null) return null;
 
-function getValidServiceNames(): Set<string> {
-  const result = new Set<string>();
-
-  // add our special json service
-  result.add(JSON_TRANSPORT_SERVICE);
-
-  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());
-    }
-  }
-
-  return result;
+  return serviceEntry.host || null;
 }
 
 // Returns the secure value - whether to use TLS or not.

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(

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');
+    });
   });
 }

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: {