Make xpack.actions.rejectUnauthorized setting work (elastic#88690)

* Remove ActionsConfigType due to being a duplicate * Fix rejectUnauthorized not being configured * Move proxySettings to configurationUtilities * Fix isAxiosError check to code * Add functional test * Remove comment * Close webhook server Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
mikecote · Jan 28, 2021 · 6ff3bc2 · 6ff3bc2
1 parent 4fea9ee
commit 6ff3bc2
Show file tree

Hide file tree

Showing 38 changed files with 597 additions and 371 deletions.
diff --git a/x-pack/plugins/actions/server/actions_client.test.ts b/x-pack/plugins/actions/server/actions_client.test.ts
@@ -402,6 +402,9 @@ describe('create()', () => {
       enabled: true,
       enabledActionTypes: ['some-not-ignored-action-type'],
       allowedHosts: ['*'],
+      preconfigured: {},
+      proxyRejectUnauthorizedCertificates: true,
+      rejectUnauthorized: true,
     });
 
     const localActionTypeRegistryParams = {

diff --git a/x-pack/plugins/actions/server/actions_config.mock.ts b/x-pack/plugins/actions/server/actions_config.mock.ts
@@ -14,6 +14,8 @@ const createActionsConfigMock = () => {
     ensureHostnameAllowed: jest.fn().mockReturnValue({}),
     ensureUriAllowed: jest.fn().mockReturnValue({}),
     ensureActionTypeEnabled: jest.fn().mockReturnValue({}),
+    isRejectUnauthorizedCertificatesEnabled: jest.fn().mockReturnValue(true),
+    getProxySettings: jest.fn().mockReturnValue(undefined),
   };
   return mocked;
 };

diff --git a/x-pack/plugins/actions/server/actions_config.test.ts b/x-pack/plugins/actions/server/actions_config.test.ts
@@ -4,22 +4,26 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { ActionsConfigType } from './types';
+import { ActionsConfig } from './config';
 import {
   getActionsConfigurationUtilities,
   AllowedHosts,
   EnabledActionTypes,
 } from './actions_config';
 
-const DefaultActionsConfig: ActionsConfigType = {
+const defaultActionsConfig: ActionsConfig = {
   enabled: false,
   allowedHosts: [],
   enabledActionTypes: [],
+  preconfigured: {},
+  proxyRejectUnauthorizedCertificates: true,
+  rejectUnauthorized: true,
 };
 
 describe('ensureUriAllowed', () => {
   test('returns true when "any" hostnames are allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [AllowedHosts.Any],
       enabledActionTypes: [],
@@ -30,7 +34,7 @@ describe('ensureUriAllowed', () => {
   });
 
   test('throws when the hostname in the requested uri is not in the allowedHosts', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(() =>
       getActionsConfigurationUtilities(config).ensureUriAllowed('https://github.com/elastic/kibana')
     ).toThrowErrorMatchingInlineSnapshot(
@@ -39,7 +43,7 @@ describe('ensureUriAllowed', () => {
   });
 
   test('throws when the uri cannot be parsed as a valid URI', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(() =>
       getActionsConfigurationUtilities(config).ensureUriAllowed('github.com/elastic')
     ).toThrowErrorMatchingInlineSnapshot(
@@ -48,7 +52,8 @@ describe('ensureUriAllowed', () => {
   });
 
   test('returns true when the hostname in the requested uri is in the allowedHosts', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: ['github.com'],
       enabledActionTypes: [],
@@ -61,7 +66,8 @@ describe('ensureUriAllowed', () => {
 
 describe('ensureHostnameAllowed', () => {
   test('returns true when "any" hostnames are allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [AllowedHosts.Any],
       enabledActionTypes: [],
@@ -72,7 +78,7 @@ describe('ensureHostnameAllowed', () => {
   });
 
   test('throws when the hostname in the requested uri is not in the allowedHosts', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(() =>
       getActionsConfigurationUtilities(config).ensureHostnameAllowed('github.com')
     ).toThrowErrorMatchingInlineSnapshot(
@@ -81,7 +87,8 @@ describe('ensureHostnameAllowed', () => {
   });
 
   test('returns true when the hostname in the requested uri is in the allowedHosts', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: ['github.com'],
       enabledActionTypes: [],
@@ -94,7 +101,8 @@ describe('ensureHostnameAllowed', () => {
 
 describe('isUriAllowed', () => {
   test('returns true when "any" hostnames are allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [AllowedHosts.Any],
       enabledActionTypes: [],
@@ -105,21 +113,22 @@ describe('isUriAllowed', () => {
   });
 
   test('throws when the hostname in the requested uri is not in the allowedHosts', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(
       getActionsConfigurationUtilities(config).isUriAllowed('https://github.com/elastic/kibana')
     ).toEqual(false);
   });
 
   test('throws when the uri cannot be parsed as a valid URI', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(getActionsConfigurationUtilities(config).isUriAllowed('github.com/elastic')).toEqual(
       false
     );
   });
 
   test('returns true when the hostname in the requested uri is in the allowedHosts', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: ['github.com'],
       enabledActionTypes: [],
@@ -132,7 +141,8 @@ describe('isUriAllowed', () => {
 
 describe('isHostnameAllowed', () => {
   test('returns true when "any" hostnames are allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [AllowedHosts.Any],
       enabledActionTypes: [],
@@ -141,12 +151,13 @@ describe('isHostnameAllowed', () => {
   });
 
   test('throws when the hostname in the requested uri is not in the allowedHosts', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(getActionsConfigurationUtilities(config).isHostnameAllowed('github.com')).toEqual(false);
   });
 
   test('returns true when the hostname in the requested uri is in the allowedHosts', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: ['github.com'],
       enabledActionTypes: [],
@@ -157,7 +168,8 @@ describe('isHostnameAllowed', () => {
 
 describe('isActionTypeEnabled', () => {
   test('returns true when "any" actionTypes are allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['ignore', EnabledActionTypes.Any],
@@ -166,7 +178,8 @@ describe('isActionTypeEnabled', () => {
   });
 
   test('returns false when no actionType is allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: [],
@@ -175,7 +188,8 @@ describe('isActionTypeEnabled', () => {
   });
 
   test('returns false when the actionType is not in the enabled list', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['foo'],
@@ -184,7 +198,8 @@ describe('isActionTypeEnabled', () => {
   });
 
   test('returns true when the actionType is in the enabled list', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['ignore', 'foo'],
@@ -195,7 +210,8 @@ describe('isActionTypeEnabled', () => {
 
 describe('ensureActionTypeEnabled', () => {
   test('does not throw when any actionType is allowed', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['ignore', EnabledActionTypes.Any],
@@ -204,7 +220,7 @@ describe('ensureActionTypeEnabled', () => {
   });
 
   test('throws when no actionType is not allowed', () => {
-    const config: ActionsConfigType = DefaultActionsConfig;
+    const config: ActionsConfig = defaultActionsConfig;
     expect(() =>
       getActionsConfigurationUtilities(config).ensureActionTypeEnabled('foo')
     ).toThrowErrorMatchingInlineSnapshot(
@@ -213,7 +229,8 @@ describe('ensureActionTypeEnabled', () => {
   });
 
   test('throws when actionType is not enabled', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['ignore'],
@@ -226,7 +243,8 @@ describe('ensureActionTypeEnabled', () => {
   });
 
   test('does not throw when actionType is enabled', () => {
-    const config: ActionsConfigType = {
+    const config: ActionsConfig = {
+      ...defaultActionsConfig,
       enabled: false,
       allowedHosts: [],
       enabledActionTypes: ['ignore', 'foo'],

diff --git a/x-pack/plugins/actions/server/actions_config.ts b/x-pack/plugins/actions/server/actions_config.ts
@@ -10,8 +10,9 @@ import url from 'url';
 import { curry } from 'lodash';
 import { pipe } from 'fp-ts/lib/pipeable';
 
-import { ActionsConfigType } from './types';
+import { ActionsConfig } from './config';
 import { ActionTypeDisabledError } from './lib';
+import { ProxySettings } from './types';
 
 export enum AllowedHosts {
   Any = '*',
@@ -33,6 +34,8 @@ export interface ActionsConfigurationUtilities {
   ensureHostnameAllowed: (hostname: string) => void;
   ensureUriAllowed: (uri: string) => void;
   ensureActionTypeEnabled: (actionType: string) => void;
+  isRejectUnauthorizedCertificatesEnabled: () => boolean;
+  getProxySettings: () => undefined | ProxySettings;
 }
 
 function allowListErrorMessage(field: AllowListingField, value: string) {
@@ -56,14 +59,14 @@ function disabledActionTypeErrorMessage(actionType: string) {
   });
 }
 
-function isAllowed({ allowedHosts }: ActionsConfigType, hostname: string | null): boolean {
+function isAllowed({ allowedHosts }: ActionsConfig, hostname: string | null): boolean {
   const allowed = new Set(allowedHosts);
   if (allowed.has(AllowedHosts.Any)) return true;
   if (hostname && allowed.has(hostname)) return true;
   return false;
 }
 
-function isHostnameAllowedInUri(config: ActionsConfigType, uri: string): boolean {
+function isHostnameAllowedInUri(config: ActionsConfig, uri: string): boolean {
   return pipe(
     tryCatch(() => url.parse(uri)),
     map((parsedUrl) => parsedUrl.hostname),
@@ -73,7 +76,7 @@ function isHostnameAllowedInUri(config: ActionsConfigType, uri: string): boolean
 }
 
 function isActionTypeEnabledInConfig(
-  { enabledActionTypes }: ActionsConfigType,
+  { enabledActionTypes }: ActionsConfig,
   actionType: string
 ): boolean {
   const enabled = new Set(enabledActionTypes);
@@ -82,8 +85,20 @@ function isActionTypeEnabledInConfig(
   return false;
 }
 
+function getProxySettingsFromConfig(config: ActionsConfig): undefined | ProxySettings {
+  if (!config.proxyUrl) {
+    return undefined;
+  }
+
+  return {
+    proxyUrl: config.proxyUrl,
+    proxyHeaders: config.proxyHeaders,
+    proxyRejectUnauthorizedCertificates: config.proxyRejectUnauthorizedCertificates,
+  };
+}
+
 export function getActionsConfigurationUtilities(
-  config: ActionsConfigType
+  config: ActionsConfig
 ): ActionsConfigurationUtilities {
   const isHostnameAllowed = curry(isAllowed)(config);
   const isUriAllowed = curry(isHostnameAllowedInUri)(config);
@@ -92,6 +107,8 @@ export function getActionsConfigurationUtilities(
     isHostnameAllowed,
     isUriAllowed,
     isActionTypeEnabled,
+    getProxySettings: () => getProxySettingsFromConfig(config),
+    isRejectUnauthorizedCertificatesEnabled: () => config.rejectUnauthorized,
     ensureUriAllowed(uri: string) {
       if (!isUriAllowed(uri)) {
         throw new Error(allowListErrorMessage(AllowListingField.URL, uri));

diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts
@@ -277,6 +277,16 @@ describe('execute()', () => {
     `);
     expect(sendEmailMock.mock.calls[0][1]).toMatchInlineSnapshot(`
       Object {
+        "configurationUtilities": Object {
+          "ensureActionTypeEnabled": [MockFunction],
+          "ensureHostnameAllowed": [MockFunction],
+          "ensureUriAllowed": [MockFunction],
+          "getProxySettings": [MockFunction],
+          "isActionTypeEnabled": [MockFunction],
+          "isHostnameAllowed": [MockFunction],
+          "isRejectUnauthorizedCertificatesEnabled": [MockFunction],
+          "isUriAllowed": [MockFunction],
+        },
         "content": Object {
           "message": "a message to you
 
@@ -286,7 +296,6 @@ describe('execute()', () => {
           "subject": "the subject",
         },
         "hasAuth": true,
-        "proxySettings": undefined,
         "routing": Object {
           "bcc": Array [
             "jimmy@example.com",
@@ -327,6 +336,16 @@ describe('execute()', () => {
     await actionType.executor(customExecutorOptions);
     expect(sendEmailMock.mock.calls[0][1]).toMatchInlineSnapshot(`
       Object {
+        "configurationUtilities": Object {
+          "ensureActionTypeEnabled": [MockFunction],
+          "ensureHostnameAllowed": [MockFunction],
+          "ensureUriAllowed": [MockFunction],
+          "getProxySettings": [MockFunction],
+          "isActionTypeEnabled": [MockFunction],
+          "isHostnameAllowed": [MockFunction],
+          "isRejectUnauthorizedCertificatesEnabled": [MockFunction],
+          "isUriAllowed": [MockFunction],
+        },
         "content": Object {
           "message": "a message to you
 
@@ -336,7 +355,6 @@ describe('execute()', () => {
           "subject": "the subject",
         },
         "hasAuth": false,
-        "proxySettings": undefined,
         "routing": Object {
           "bcc": Array [
             "jimmy@example.com",