[Alerting] refactor action whitelisting functions

x-pack/plugins/actions/server/action_type_registry.mock.ts

@@ -5,6 +5,7 @@
  */
 
 import { ActionTypeRegistryContract } from './types';
+import { createValidationServiceMock } from './mocks';
 
 const createActionTypeRegistryMock = () => {
   const mocked: jest.Mocked<ActionTypeRegistryContract> = {
@@ -15,6 +16,7 @@ const createActionTypeRegistryMock = () => {
     ensureActionTypeEnabled: jest.fn(),
     isActionTypeEnabled: jest.fn(),
     isActionExecutable: jest.fn(),
+    getValidationService: jest.fn(() => createValidationServiceMock()),
   };
   return mocked;
 };

x-pack/plugins/actions/server/action_type_registry.ts

@@ -8,9 +8,9 @@ import Boom from 'boom';
 import { i18n } from '@kbn/i18n';
 import { RunContext, TaskManagerSetupContract } from '../../task_manager/server';
 import { ExecutorError, TaskRunnerFactory, ILicenseState } from './lib';
-import { ActionType, PreConfiguredAction } from './types';
+import { ActionType, PreConfiguredAction, ActionValidationService } from './types';
 import { ActionType as CommonActionType } from '../common';
-import { ActionsConfigurationUtilities } from './actions_config';
+import { ActionsConfigurationUtilities, getValidationService } from './actions_config';
 
 export interface ActionTypeRegistryOpts {
   taskManager: TaskManagerSetupContract;
@@ -27,13 +27,15 @@ export class ActionTypeRegistry {
   private readonly actionsConfigUtils: ActionsConfigurationUtilities;
   private readonly licenseState: ILicenseState;
   private readonly preconfiguredActions: PreConfiguredAction[];
+  private readonly validationService: ActionValidationService;
 
   constructor(constructorParams: ActionTypeRegistryOpts) {
     this.taskManager = constructorParams.taskManager;
     this.taskRunnerFactory = constructorParams.taskRunnerFactory;
     this.actionsConfigUtils = constructorParams.actionsConfigUtils;
     this.licenseState = constructorParams.licenseState;
     this.preconfiguredActions = constructorParams.preconfiguredActions;
+    this.validationService = getValidationService(this.actionsConfigUtils);
   }
 
   /**
@@ -43,6 +45,13 @@ export class ActionTypeRegistry {
     return this.actionTypes.has(id);
   }
 
+  /**
+   * Return the validation service.
+   */
+  public getValidationService(): ActionValidationService {
+    return this.validationService;
+  }
+
   /**
    * Throws error if action type is not enabled.
    */

x-pack/plugins/actions/server/actions_client.ts

@@ -75,9 +75,10 @@ export class ActionsClient {
    */
   public async create({ action }: CreateOptions): Promise<ActionResult> {
     const { actionTypeId, name, config, secrets } = action;
+    const validationService = this.actionTypeRegistry.getValidationService();
     const actionType = this.actionTypeRegistry.get(actionTypeId);
-    const validatedActionTypeConfig = validateConfig(actionType, config);
-    const validatedActionTypeSecrets = validateSecrets(actionType, secrets);
+    const validatedActionTypeConfig = validateConfig(actionType, config, validationService);
+    const validatedActionTypeSecrets = validateSecrets(actionType, secrets, validationService);
 
     this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
 
@@ -119,8 +120,9 @@ export class ActionsClient {
     const { actionTypeId } = existingObject.attributes;
     const { name, config, secrets } = action;
     const actionType = this.actionTypeRegistry.get(actionTypeId);
-    const validatedActionTypeConfig = validateConfig(actionType, config);
-    const validatedActionTypeSecrets = validateSecrets(actionType, secrets);
+    const validationService = this.actionTypeRegistry.getValidationService();
+    const validatedActionTypeConfig = validateConfig(actionType, config, validationService);
+    const validatedActionTypeSecrets = validateSecrets(actionType, secrets, validationService);
 
     this.actionTypeRegistry.ensureActionTypeEnabled(actionTypeId);
 

x-pack/plugins/actions/server/actions_config.ts

@@ -10,7 +10,7 @@ import { URL } from 'url';
 import { curry } from 'lodash';
 import { pipe } from 'fp-ts/lib/pipeable';
 
-import { ActionsConfigType } from './types';
+import { ActionsConfigType, ActionValidationService } from './types';
 import { ActionTypeDisabledError } from './lib';
 
 export enum WhitelistedHosts {
@@ -109,3 +109,13 @@ export function getActionsConfigurationUtilities(
     },
   };
 }
+
+export function getValidationService(
+  configUtils: ActionsConfigurationUtilities
+): ActionValidationService {
+  const { isWhitelistedHostname, isWhitelistedUri } = configUtils;
+  return {
+    isWhitelistedHostname,
+    isWhitelistedUri,
+  };
+}

x-pack/plugins/actions/server/builtin_action_types/email.test.ts

@@ -8,29 +8,21 @@ jest.mock('./lib/send_email', () => ({
   sendEmail: jest.fn(),
 }));
 
-import { Logger } from '../../../../../src/core/server';
-
 import { ActionType, ActionTypeExecutorOptions } from '../types';
-import { actionsConfigMock } from '../actions_config.mock';
 import { validateConfig, validateSecrets, validateParams } from '../lib';
 import { createActionTypeRegistry } from './index.test';
 import { sendEmail } from './lib/send_email';
 import { actionsMock } from '../mocks';
-import {
-  ActionParamsType,
-  ActionTypeConfigType,
-  ActionTypeSecretsType,
-  getActionType,
-} from './email';
+import { ActionParamsType, ActionTypeConfigType, ActionTypeSecretsType } from './email';
 
 const sendEmailMock = sendEmail as jest.Mock;
 
 const ACTION_TYPE_ID = '.email';
 
 const services = actionsMock.createServices();
+const validationService = actionsMock.createValidationService();
 
 let actionType: ActionType;
-let mockedLogger: jest.Mocked<Logger>;
 
 beforeEach(() => {
   jest.resetAllMocks();
@@ -51,7 +43,8 @@ describe('config validation', () => {
       service: 'gmail',
       from: '[email protected]',
     };
-    expect(validateConfig(actionType, config)).toEqual({
+    validationService.isWhitelistedHostname.mockReturnValue(true);
+    expect(validateConfig(actionType, config, validationService)).toEqual({
       ...config,
       host: null,
       port: null,
@@ -61,7 +54,7 @@ describe('config validation', () => {
     delete config.service;
     config.host = 'elastic.co';
     config.port = 8080;
-    expect(validateConfig(actionType, config)).toEqual({
+    expect(validateConfig(actionType, config, validationService)).toEqual({
       ...config,
       service: null,
       secure: null,
@@ -75,38 +68,42 @@ describe('config validation', () => {
 
     // empty object
     expect(() => {
-      validateConfig(actionType, {});
+      validateConfig(actionType, {}, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: [from]: expected value of type [string] but got [undefined]"`
     );
 
     // no service or host/port
     expect(() => {
-      validateConfig(actionType, baseConfig);
+      validateConfig(actionType, baseConfig, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: either [service] or [host]/[port] is required"`
     );
 
     // host but no port
     expect(() => {
-      validateConfig(actionType, { ...baseConfig, host: 'elastic.co' });
+      validateConfig(actionType, { ...baseConfig, host: 'elastic.co' }, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: [port] is required if [service] is not provided"`
     );
 
     // port but no host
     expect(() => {
-      validateConfig(actionType, { ...baseConfig, port: 8080 });
+      validateConfig(actionType, { ...baseConfig, port: 8080 }, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: [host] is required if [service] is not provided"`
     );
 
     // invalid service
     expect(() => {
-      validateConfig(actionType, {
-        ...baseConfig,
-        service: 'bad-nodemailer-service',
-      });
+      validateConfig(
+        actionType,
+        {
+          ...baseConfig,
+          service: 'bad-nodemailer-service',
+        },
+        validationService
+      );
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: [service] value 'bad-nodemailer-service' is not valid"`
     );
@@ -117,13 +114,9 @@ describe('config validation', () => {
   const NODEMAILER_AOL_SERVICE_HOST = 'smtp.aol.com';
 
   test('config validation handles email host whitelisting', () => {
-    actionType = getActionType({
-      logger: mockedLogger,
-      configurationUtilities: {
-        ...actionsConfigMock.create(),
-        isWhitelistedHostname: (hostname) => hostname === NODEMAILER_AOL_SERVICE_HOST,
-      },
-    });
+    validationService.isWhitelistedHostname.mockImplementation(
+      (hostname) => hostname === NODEMAILER_AOL_SERVICE_HOST
+    );
     const baseConfig = {
       from: '[email protected]',
     };
@@ -147,23 +140,23 @@ describe('config validation', () => {
       port: 42,
     };
 
-    const validatedConfig1 = validateConfig(actionType, whitelistedConfig1);
+    const validatedConfig1 = validateConfig(actionType, whitelistedConfig1, validationService);
     expect(validatedConfig1.service).toEqual(whitelistedConfig1.service);
     expect(validatedConfig1.from).toEqual(whitelistedConfig1.from);
 
-    const validatedConfig2 = validateConfig(actionType, whitelistedConfig2);
+    const validatedConfig2 = validateConfig(actionType, whitelistedConfig2, validationService);
     expect(validatedConfig2.host).toEqual(whitelistedConfig2.host);
     expect(validatedConfig2.port).toEqual(whitelistedConfig2.port);
     expect(validatedConfig2.from).toEqual(whitelistedConfig2.from);
 
     expect(() => {
-      validateConfig(actionType, notWhitelistedConfig1);
+      validateConfig(actionType, notWhitelistedConfig1, validationService);
     }).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);
+      validateConfig(actionType, notWhitelistedConfig2, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action type config: [host] value 'smtp.gmail.com' is not in the whitelistedHosts configuration"`
     );
@@ -176,17 +169,17 @@ describe('secrets validation', () => {
       user: 'bob',
       password: 'supersecret',
     };
-    expect(validateSecrets(actionType, secrets)).toEqual(secrets);
+    expect(validateSecrets(actionType, secrets, validationService)).toEqual(secrets);
   });
 
   test('secrets validation succeeds when secrets props are null/undefined', () => {
     const secrets: Record<string, unknown> = {
       user: null,
       password: null,
     };
-    expect(validateSecrets(actionType, {})).toEqual(secrets);
-    expect(validateSecrets(actionType, { user: null })).toEqual(secrets);
-    expect(validateSecrets(actionType, { password: null })).toEqual(secrets);
+    expect(validateSecrets(actionType, {}, validationService)).toEqual(secrets);
+    expect(validateSecrets(actionType, { user: null }, validationService)).toEqual(secrets);
+    expect(validateSecrets(actionType, { password: null }, validationService)).toEqual(secrets);
   });
 });
 
@@ -197,7 +190,7 @@ describe('params validation', () => {
       subject: 'this is a test',
       message: 'this is the message',
     };
-    expect(validateParams(actionType, params)).toMatchInlineSnapshot(`
+    expect(validateParams(actionType, params, validationService)).toMatchInlineSnapshot(`
       Object {
         "bcc": Array [],
         "cc": Array [],
@@ -213,7 +206,7 @@ describe('params validation', () => {
   test('params validation fails when params is not valid', () => {
     // empty object
     expect(() => {
-      validateParams(actionType, {});
+      validateParams(actionType, {}, validationService);
     }).toThrowErrorMatchingInlineSnapshot(
       `"error validating action params: [subject]: expected value of type [string] but got [undefined]"`
     );