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

Jump to bottom

Make xpack.actions.rejectUnauthorized setting work #88690

Merged
merged 15 commits into from
Jan 28, 2021
Merged

Make xpack.actions.rejectUnauthorized setting work #88690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
78b7e2e
Remove ActionsConfigType due to being a duplicate
mikecote Jan 19, 2021
08ebe63
Fix rejectUnauthorized not being configured
mikecote Jan 19, 2021
1a17f8d
Move proxySettings to configurationUtilities
mikecote Jan 19, 2021
c5f6eb7
Merge
mikecote Jan 19, 2021
41fb4f1
Merge branch 'master' of github.com:elastic/kibana into actions/rejec…
mikecote Jan 20, 2021
a84503d
Fix isAxiosError check to code
mikecote Jan 20, 2021
31daebd
Merge branch 'master' of github.com:elastic/kibana into actions/rejec…
mikecote Jan 26, 2021
af638a3
Add functional test
mikecote Jan 26, 2021
0337bbd
Remove comment
mikecote Jan 26, 2021
d6be507
Merge branch 'master' into actions/reject-unauthorized
kibanamachine Jan 27, 2021
53ac649
Merge branch 'master' into actions/reject-unauthorized
kibanamachine Jan 27, 2021
e55b0f5
Merge branch 'master' of github.com:elastic/kibana into actions/rejec…
mikecote Jan 27, 2021
3631dcd
Merge branch 'actions/reject-unauthorized' of github.com:mikecote/kib…
mikecote Jan 27, 2021
013b342
Merge branch 'master' of github.com:elastic/kibana into actions/rejec…
mikecote Jan 28, 2021
b9414e3
Close webhook server
mikecote Jan 28, 2021
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
3 changes: 3 additions & 0 deletions x-pack/plugins/actions/server/actions_client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,9 @@ describe('create()', () => {
enabled: true,
enabledActionTypes: ['some-not-ignored-action-type'],
allowedHosts: ['*'],
preconfigured: {},
proxyRejectUnauthorizedCertificates: true,
rejectUnauthorized: true,
});

const localActionTypeRegistryParams = {
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/actions/server/actions_config.mock.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
66 changes: 42 additions & 24 deletions x-pack/plugins/actions/server/actions_config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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(
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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: [],
Expand All @@ -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],
Expand All @@ -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: [],
Expand All @@ -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'],
Expand All @@ -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'],
Expand All @@ -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],
Expand All @@ -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(
Expand All @@ -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'],
Expand All @@ -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'],
Expand Down
27 changes: 22 additions & 5 deletions x-pack/plugins/actions/server/actions_config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '*',
Expand All @@ -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) {
Expand All @@ -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),
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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));
Expand Down
22 changes: 20 additions & 2 deletions x-pack/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 @@ -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

Expand All @@ -286,7 +296,6 @@ describe('execute()', () => {
"subject": "the subject",
},
"hasAuth": true,
"proxySettings": undefined,
"routing": Object {
"bcc": Array [
"jimmy@example.com",
Expand Down Expand Up @@ -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

Expand All @@ -336,7 +355,6 @@ describe('execute()', () => {
"subject": "the subject",
},
"hasAuth": false,
"proxySettings": undefined,
"routing": Object {
"bcc": Array [
"jimmy@example.com",
Expand Down
Loading