Skip to content

Commit

Permalink
change slack action to only report on whitelisted host name (#57582) (#…
Browse files Browse the repository at this point in the history
…57740)

Previously, when using the slack action with a url which was not whitelisted, the entire URL was reported in the error.  With this change, only the hostname is reported in the error.
  • Loading branch information
pmuellr authored Feb 15, 2020
1 parent 4d71322 commit 02a6eaa
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 6 deletions.
12 changes: 9 additions & 3 deletions x-pack/plugins/actions/server/builtin_action_types/slack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ describe('validateActionTypeSecrets()', () => {
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: [webhookUrl]: expected value of type [string] but got [number]"`
);

expect(() => {
validateSecrets(actionType, { webhookUrl: 'fee-fi-fo-fum' });
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: error configuring slack action: unable to parse host name from webhookUrl"`
);
});

test('should validate and pass when the slack webhookUrl is whitelisted', () => {
Expand All @@ -95,16 +101,16 @@ describe('validateActionTypeSecrets()', () => {
actionType = getActionType({
configurationUtilities: {
...configUtilsMock,
ensureWhitelistedUri: url => {
throw new Error(`target url is not whitelisted`);
ensureWhitelistedHostname: url => {
throw new Error(`target hostname is not whitelisted`);
},
},
});

expect(() => {
validateSecrets(actionType, { webhookUrl: 'https://api.slack.com/' });
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: error configuring slack action: target url is not whitelisted"`
`"error validating action type secrets: error configuring slack action: target hostname is not whitelisted"`
);
});
});
Expand Down
12 changes: 11 additions & 1 deletion x-pack/plugins/actions/server/builtin_action_types/slack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { URL } from 'url';
import { curry } from 'lodash';
import { i18n } from '@kbn/i18n';
import { schema, TypeOf } from '@kbn/config-schema';
Expand Down Expand Up @@ -66,8 +67,17 @@ function valdiateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
secretsObject: ActionTypeSecretsType
) {
let url: URL;
try {
configurationUtilities.ensureWhitelistedUri(secretsObject.webhookUrl);
url = new URL(secretsObject.webhookUrl);
} catch (err) {
return i18n.translate('xpack.actions.builtin.slack.slackConfigurationErrorNoHostname', {
defaultMessage: 'error configuring slack action: unable to parse host name from webhookUrl',
});
}

try {
configurationUtilities.ensureWhitelistedHostname(url.hostname);
} catch (whitelistError) {
return i18n.translate('xpack.actions.builtin.slack.slackConfigurationError', {
defaultMessage: 'error configuring slack action: {message}',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export default function slackTest({ getService }: FtrProviderContext) {
name: 'A slack action',
actionTypeId: '.slack',
secrets: {
webhookUrl: 'http://slack.mynonexistent.com',
webhookUrl: 'http://slack.mynonexistent.com/other/stuff/in/the/path',
},
})
.expect(400)
Expand All @@ -103,7 +103,29 @@ export default function slackTest({ getService }: FtrProviderContext) {
statusCode: 400,
error: 'Bad Request',
message:
'error validating action type secrets: error configuring slack action: target url "http://slack.mynonexistent.com" is not whitelisted in the Kibana config xpack.actions.whitelistedHosts',
'error validating action type secrets: error configuring slack action: target hostname "slack.mynonexistent.com" is not whitelisted in the Kibana config xpack.actions.whitelistedHosts',
});
});
});

it('should respond with a 400 Bad Request when creating a slack action with a webhookUrl with no hostname', async () => {
await supertest
.post('/api/action')
.set('kbn-xsrf', 'foo')
.send({
name: 'A slack action',
actionTypeId: '.slack',
secrets: {
webhookUrl: 'fee-fi-fo-fum',
},
})
.expect(400)
.then((resp: any) => {
expect(resp.body).to.eql({
statusCode: 400,
error: 'Bad Request',
message:
'error validating action type secrets: error configuring slack action: unable to parse host name from webhookUrl',
});
});
});
Expand Down

0 comments on commit 02a6eaa

Please sign in to comment.