Skip to content

Commit

Permalink
Webhook action - make user and password secrets optional (elastic#56823)
Browse files Browse the repository at this point in the history
  • Loading branch information
Peter Schretlen committed Feb 10, 2020
1 parent 5e1b861 commit 09b352d
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 27 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,8 @@ The webhook action uses [axios](https://github.com/axios/axios) to send a POST o

|Property|Description|Type|
|---|---|---|
|user|Username for HTTP Basic authentication|string|
|password|Password for HTTP Basic authentication|string|
|user|Username for HTTP Basic authentication|string _(optional)_|
|password|Password for HTTP Basic authentication|string _(optional)_|

### `params`

Expand Down
106 changes: 98 additions & 8 deletions x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,28 @@
* you may not use this file except in compliance with the Elastic License.
*/

jest.mock('axios', () => ({
request: jest.fn(),
}));

import { getActionType } from './webhook';
import { ActionType, Services } from '../types';
import { validateConfig, validateSecrets, validateParams } from '../lib';
import { savedObjectsClientMock } from '../../../../../src/core/server/mocks';
import { configUtilsMock } from '../actions_config.mock';
import { ActionType } from '../types';
import { createActionTypeRegistry } from './index.test';
import { Logger } from '../../../../../src/core/server';
import axios from 'axios';

const axiosRequestMock = axios.request as jest.Mock;

const ACTION_TYPE_ID = '.webhook';

const services: Services = {
callCluster: async (path: string, opts: any) => {},
savedObjectsClient: savedObjectsClientMock.create(),
};

let actionType: ActionType;
let mockedLogger: jest.Mocked<Logger>;

Expand All @@ -38,20 +51,18 @@ describe('secrets validation', () => {
expect(validateSecrets(actionType, secrets)).toEqual(secrets);
});

test('fails when secret password is omitted', () => {
test('fails when secret user is provided, but password is omitted', () => {
expect(() => {
validateSecrets(actionType, { user: 'bob' });
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: [password]: expected value of type [string] but got [undefined]"`
`"error validating action type secrets: both user and password must be specified"`
);
});

test('fails when secret user is omitted', () => {
test('succeeds when basic authentication credentials are omitted', () => {
expect(() => {
validateSecrets(actionType, {});
}).toThrowErrorMatchingInlineSnapshot(
`"error validating action type secrets: [user]: expected value of type [string] but got [undefined]"`
);
validateSecrets(actionType, {}).toEqual({});
});
});
});

Expand Down Expand Up @@ -190,3 +201,82 @@ describe('params validation', () => {
});
});
});

describe('execute()', () => {
beforeAll(() => {
axiosRequestMock.mockReset();
actionType = getActionType({
logger: mockedLogger,
configurationUtilities: configUtilsMock,
});
});

beforeEach(() => {
axiosRequestMock.mockReset();
axiosRequestMock.mockResolvedValue({
status: 200,
statusText: '',
data: '',
headers: [],
config: {},
});
});

test('execute with username/password sends request with basic auth', async () => {
await actionType.executor({
actionId: 'some-id',
services,
config: {
url: 'https://abc.def/my-webhook',
method: 'post',
headers: {
aheader: 'a value',
},
},
secrets: { user: 'abc', password: '123' },
params: { body: 'some data' },
});

expect(axiosRequestMock.mock.calls[0][0]).toMatchInlineSnapshot(`
Object {
"auth": Object {
"password": "123",
"username": "abc",
},
"data": "some data",
"headers": Object {
"aheader": "a value",
},
"method": "post",
"url": "https://abc.def/my-webhook",
}
`);
});

test('execute without username/password sends request without basic auth', async () => {
await actionType.executor({
actionId: 'some-id',
services,
config: {
url: 'https://abc.def/my-webhook',
method: 'post',
headers: {
aheader: 'a value',
},
},
secrets: {},
params: { body: 'some data' },
});

expect(axiosRequestMock.mock.calls[0][0]).toMatchInlineSnapshot(`
Object {
"data": "some data",
"headers": Object {
"aheader": "a value",
},
"method": "post",
"url": "https://abc.def/my-webhook",
}
`);
});
});
36 changes: 24 additions & 12 deletions x-pack/plugins/actions/server/builtin_action_types/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { i18n } from '@kbn/i18n';
import { curry } from 'lodash';
import { curry, isString } from 'lodash';
import axios, { AxiosError, AxiosResponse } from 'axios';
import { schema, TypeOf } from '@kbn/config-schema';
import { pipe } from 'fp-ts/lib/pipeable';
Expand Down Expand Up @@ -34,10 +34,20 @@ const ConfigSchema = schema.object(configSchemaProps);
type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;

// secrets definition
type ActionTypeSecretsType = TypeOf<typeof SecretsSchema>;
const SecretsSchema = schema.object({
user: schema.string(),
password: schema.string(),
export type ActionTypeSecretsType = TypeOf<typeof SecretsSchema>;
const secretSchemaProps = {
user: schema.nullable(schema.string()),
password: schema.nullable(schema.string()),
};
const SecretsSchema = schema.object(secretSchemaProps, {
validate: secrets => {
// user and password must be set together (or not at all)
if (!secrets.password && !secrets.user) return;
if (secrets.password && secrets.user) return;
return i18n.translate('xpack.actions.builtin.webhook.invalidUsernamePassword', {
defaultMessage: 'both user and password must be specified',
});
},
});

// params definition
Expand All @@ -61,7 +71,7 @@ export function getActionType({
}),
validate: {
config: schema.object(configSchemaProps, {
validate: curry(valdiateActionTypeConfig)(configurationUtilities),
validate: curry(validateActionTypeConfig)(configurationUtilities),
}),
secrets: SecretsSchema,
params: ParamsSchema,
Expand All @@ -70,7 +80,7 @@ export function getActionType({
};
}

function valdiateActionTypeConfig(
function validateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
configObject: ActionTypeConfigType
) {
Expand All @@ -93,17 +103,19 @@ export async function executor(
): Promise<ActionTypeExecutorResult> {
const actionId = execOptions.actionId;
const { method, url, headers = {} } = execOptions.config as ActionTypeConfigType;
const { user: username, password } = execOptions.secrets as ActionTypeSecretsType;
const { body: data } = execOptions.params as ActionParamsType;

const secrets: ActionTypeSecretsType = execOptions.secrets as ActionTypeSecretsType;
const basicAuth =
isString(secrets.user) && isString(secrets.password)
? { auth: { username: secrets.user, password: secrets.password } }
: {};

const result: Result<AxiosResponse, AxiosError> = await promiseResult(
axios.request({
method,
url,
auth: {
username,
password,
},
...basicAuth,
headers,
data,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ export function getActionType(): ActionTypeModel {
)
);
}
if (!action.secrets.user) {
if (!action.secrets.user && action.secrets.password) {
errors.user.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText',
Expand All @@ -83,7 +83,7 @@ export function getActionType(): ActionTypeModel {
)
);
}
if (!action.secrets.password) {
if (!action.secrets.password && action.secrets.user) {
errors.password.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ function webhookHandler(request: WebhookRequest, h: any) {
return validateRequestUsesMethod(request, h, 'post');
case 'success_put_method':
return validateRequestUsesMethod(request, h, 'put');
case 'faliure':
case 'failure':
return htmlResponse(h, 500, `Error`);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ export default function webhookTest({ getService }: FtrProviderContext) {
.expect(200);

expect(result.status).to.eql('error');
expect(result.message).to.match(/error calling webhook, invalid response/);
expect(result.serviceMessage).to.eql('[400] Bad Request');
expect(result.message).to.match(/error calling webhook, retry later/);
expect(result.serviceMessage).to.eql('[500] Internal Server Error');
});
});
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import expect from '@kbn/expect';
import { URL, format as formatUrl } from 'url';
import { FtrProviderContext } from '../../../../common/ftr_provider_context';
import {
getExternalServiceSimulatorPath,
ExternalServiceSimulator,
} from '../../../../common/fixtures/plugins/actions';

// eslint-disable-next-line import/no-default-export
export default function webhookTest({ getService }: FtrProviderContext) {
const supertest = getService('supertest');
const esArchiver = getService('esArchiver');
const kibanaServer = getService('kibanaServer');

async function createWebhookAction(
urlWithCreds: string,
config: Record<string, string | Record<string, string>> = {}
): Promise<string> {
const url = formatUrl(new URL(urlWithCreds), { auth: false });
const composedConfig = {
headers: {
'Content-Type': 'text/plain',
},
...config,
url,
};

const { body: createdAction } = await supertest
.post('/api/action')
.set('kbn-xsrf', 'test')
.send({
name: 'A generic Webhook action',
actionTypeId: '.webhook',
secrets: {},
config: composedConfig,
})
.expect(200);

return createdAction.id;
}

describe('webhook action', () => {
let webhookSimulatorURL: string = '<could not determine kibana url>';

// need to wait for kibanaServer to settle ...
before(() => {
webhookSimulatorURL = kibanaServer.resolveUrl(
getExternalServiceSimulatorPath(ExternalServiceSimulator.WEBHOOK)
);
});

after(() => esArchiver.unload('empty_kibana'));

it('webhook can be executed without username and password', async () => {
const webhookActionId = await createWebhookAction(webhookSimulatorURL);
const { body: result } = await supertest
.post(`/api/action/${webhookActionId}/_execute`)
.set('kbn-xsrf', 'test')
.send({
params: {
body: 'success',
},
})
.expect(200);

expect(result.status).to.eql('ok');
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export default function actionsTests({ loadTestFile }: FtrProviderContext) {
loadTestFile(require.resolve('./update'));
loadTestFile(require.resolve('./execute'));
loadTestFile(require.resolve('./builtin_action_types/es_index'));
loadTestFile(require.resolve('./builtin_action_types/webhook'));
loadTestFile(require.resolve('./type_not_enabled'));
});
}

0 comments on commit 09b352d

Please sign in to comment.