[Actions] Removed double parsing when passing action url for validation (#87928)

* Removed double parsing when passing action url for validation

* Fixing functional test

* PR fixes

* PR fixes

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
ymao1 2021-01-19 13:40:49 -05:00 committed by GitHub
parent 3fce1c6457
commit 60f8b24529
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 21 additions and 22 deletions

View file

@ -6,7 +6,7 @@
import { i18n } from '@kbn/i18n';
import { tryCatch, map, mapNullable, getOrElse } from 'fp-ts/lib/Option';
import { URL } from 'url';
import url from 'url';
import { curry } from 'lodash';
import { pipe } from 'fp-ts/lib/pipeable';
@ -22,7 +22,7 @@ export enum EnabledActionTypes {
}
enum AllowListingField {
url = 'url',
URL = 'url',
hostname = 'hostname',
}
@ -56,17 +56,17 @@ function disabledActionTypeErrorMessage(actionType: string) {
});
}
function isAllowed({ allowedHosts }: ActionsConfigType, hostname: string): boolean {
function isAllowed({ allowedHosts }: ActionsConfigType, hostname: string | null): boolean {
const allowed = new Set(allowedHosts);
if (allowed.has(AllowedHosts.Any)) return true;
if (allowed.has(hostname)) return true;
if (hostname && allowed.has(hostname)) return true;
return false;
}
function isHostnameAllowedInUri(config: ActionsConfigType, uri: string): boolean {
return pipe(
tryCatch(() => new URL(uri)),
map((url) => url.hostname),
tryCatch(() => url.parse(uri)),
map((parsedUrl) => parsedUrl.hostname),
mapNullable((hostname) => isAllowed(config, hostname)),
getOrElse<boolean>(() => false)
);
@ -94,7 +94,7 @@ export function getActionsConfigurationUtilities(
isActionTypeEnabled,
ensureUriAllowed(uri: string) {
if (!isUriAllowed(uri)) {
throw new Error(allowListErrorMessage(AllowListingField.url, uri));
throw new Error(allowListErrorMessage(AllowListingField.URL, uri));
}
},
ensureHostnameAllowed(hostname: string) {

View file

@ -117,7 +117,7 @@ describe('validateActionTypeSecrets()', () => {
logger: mockedLogger,
configurationUtilities: {
...actionsConfigMock.create(),
ensureHostnameAllowed: () => {
ensureUriAllowed: () => {
throw new Error(`target hostname is not added to allowedHosts`);
},
},

View file

@ -70,7 +70,7 @@ export function getActionType({
}),
validate: {
secrets: schema.object(secretsSchemaProps, {
validate: curry(valdiateActionTypeConfig)(configurationUtilities),
validate: curry(validateActionTypeConfig)(configurationUtilities),
}),
params: ParamsSchema,
},
@ -88,13 +88,13 @@ function renderParameterTemplates(
};
}
function valdiateActionTypeConfig(
function validateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
secretsObject: ActionTypeSecretsType
) {
let url: URL;
const configuredUrl = secretsObject.webhookUrl;
try {
url = new URL(secretsObject.webhookUrl);
new URL(configuredUrl);
} catch (err) {
return i18n.translate('xpack.actions.builtin.slack.slackConfigurationErrorNoHostname', {
defaultMessage: 'error configuring slack action: unable to parse host name from webhookUrl',
@ -102,7 +102,7 @@ function valdiateActionTypeConfig(
}
try {
configurationUtilities.ensureHostnameAllowed(url.hostname);
configurationUtilities.ensureUriAllowed(configuredUrl);
} catch (allowListError) {
return i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', {
defaultMessage: 'error configuring slack action: {message}',

View file

@ -117,7 +117,7 @@ describe('validateActionTypeSecrets()', () => {
logger: mockedLogger,
configurationUtilities: {
...actionsConfigMock.create(),
ensureHostnameAllowed: () => {
ensureUriAllowed: () => {
throw new Error(`target hostname is not added to allowedHosts`);
},
},

View file

@ -71,9 +71,9 @@ function validateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
secretsObject: ActionTypeSecretsType
) {
let url: URL;
const configuredUrl = secretsObject.webhookUrl;
try {
url = new URL(secretsObject.webhookUrl);
new URL(configuredUrl);
} catch (err) {
return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationErrorNoHostname', {
defaultMessage: 'error configuring teams action: unable to parse host name from webhookUrl',
@ -81,7 +81,7 @@ function validateActionTypeConfig(
}
try {
configurationUtilities.ensureHostnameAllowed(url.hostname);
configurationUtilities.ensureUriAllowed(configuredUrl);
} catch (allowListError) {
return i18n.translate('xpack.actions.builtin.teams.teamsConfigurationError', {
defaultMessage: 'error configuring teams action: {message}',

View file

@ -112,9 +112,9 @@ function validateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
configObject: ActionTypeConfigType
) {
let url: URL;
const configuredUrl = configObject.url;
try {
url = new URL(configObject.url);
new URL(configuredUrl);
} catch (err) {
return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationErrorNoHostname', {
defaultMessage: 'error configuring webhook action: unable to parse url: {err}',
@ -125,7 +125,7 @@ function validateActionTypeConfig(
}
try {
configurationUtilities.ensureUriAllowed(url.toString());
configurationUtilities.ensureUriAllowed(configuredUrl);
} catch (allowListError) {
return i18n.translate('xpack.actions.builtin.webhook.webhookConfigurationError', {
defaultMessage: 'error configuring webhook action: {message}',

View file

@ -114,8 +114,7 @@ export default function slackTest({ getService }: FtrProviderContext) {
expect(resp.body).to.eql({
statusCode: 400,
error: 'Bad Request',
message:
'error validating action type secrets: error configuring slack action: target hostname "slack.mynonexistent.com" is not added to the Kibana config xpack.actions.allowedHosts',
message: `error validating action type secrets: error configuring slack action: target url \"http://slack.mynonexistent.com/other/stuff/in/the/path\" is not added to the Kibana config xpack.actions.allowedHosts`,
});
});
});