maxmustermann/kibana
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

[Actions] Adding hasAuth to Webhook Configuration to avoid confusing UX (#81390)

Browse source 
* Adding hasAuth to server and client

* Adding migration and fixing tests

* Fixing test

* Adding spacing

* Adding functional test

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
ymao1 2020-10-27 07:45:02 -04:00 committed by GitHub
parent e6ab812891
commit fd7f6b5716
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 333 additions and 102 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts
View file

@ -90,8 +90,9 @@ describe('config validation', () => {
};
test('config validation passes when only required fields are provided', () => {
const config: Record<string, string> = {
const config: Record<string, string | boolean> = {
url: 'http://mylisteningserver:9200/endpoint',
hasAuth: true,
};
expect(validateConfig(actionType, config)).toEqual({
...defaultValues,
@ -101,9 +102,10 @@ describe('config validation', () => {
test('config validation passes when valid methods are provided', () => {
['post', 'put'].forEach((method) => {
const config: Record<string, string> = {
const config: Record<string, string | boolean> = {
url: 'http://mylisteningserver:9200/endpoint',
method,
hasAuth: true,
};
expect(validateConfig(actionType, config)).toEqual({
...defaultValues,
@ -127,8 +129,9 @@ describe('config validation', () => {
});
test('config validation passes when a url is specified', () => {
const config: Record<string, string> = {
const config: Record<string, string | boolean> = {
url: 'http://mylisteningserver:9200/endpoint',
hasAuth: true,
};
expect(validateConfig(actionType, config)).toEqual({
...defaultValues,
@ -155,6 +158,7 @@ describe('config validation', () => {
headers: {
'Content-Type': 'application/json',
},
hasAuth: true,
};
expect(validateConfig(actionType, config)).toEqual({
...defaultValues,
@ -184,6 +188,7 @@ describe('config validation', () => {
headers: {
'Content-Type': 'application/json',
},
hasAuth: true,
};
expect(validateConfig(actionType, config)).toEqual({
@ -263,6 +268,7 @@ describe('execute()', () => {
headers: {
aheader: 'a value',
},
hasAuth: true,
};
await actionType.executor({
actionId: 'some-id',
@ -320,6 +326,7 @@ describe('execute()', () => {
headers: {
aheader: 'a value',
},
hasAuth: false,
};
const secrets: ActionTypeSecretsType = { user: null, password: null };
await actionType.executor({

5
x-pack/plugins/actions/server/builtin_action_types/webhook.ts
View file

@ -42,6 +42,7 @@ const configSchemaProps = {
defaultValue: WebhookMethods.POST,
}),
headers: nullableType(HeadersSchema),
hasAuth: schema.boolean({ defaultValue: true }),
};
const ConfigSchema = schema.object(configSchemaProps);
export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;
@ -128,12 +129,12 @@ export async function executor(
execOptions: WebhookActionTypeExecutorOptions
): Promise<ActionTypeExecutorResult<unknown>> {
const actionId = execOptions.actionId;
const { method, url, headers = {} } = execOptions.config;
const { method, url, headers = {}, hasAuth } = execOptions.config;
const { body: data } = execOptions.params;
const secrets: ActionTypeSecretsType = execOptions.secrets;
const basicAuth =
isString(secrets.user) && isString(secrets.password)
hasAuth && isString(secrets.user) && isString(secrets.password)
? { auth: { username: secrets.user, password: secrets.password } }
: {};

57
x-pack/plugins/actions/server/saved_objects/migrations.test.ts
View file

@ -58,6 +58,63 @@ describe('7.10.0', () => {
});
});
describe('7.11.0', () => {
beforeEach(() => {
jest.resetAllMocks();
encryptedSavedObjectsSetup.createMigration.mockImplementation(
(shouldMigrateWhenPredicate, migration) => migration
);
});
test('add hasAuth = true for .webhook actions with user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, true);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: true,
},
},
});
});
test('add hasAuth = false for .webhook actions without user and password', () => {
const migration711 = getMigrations(encryptedSavedObjectsSetup)['7.11.0'];
const action = getMockDataForWebhook({}, false);
expect(migration711(action, context)).toMatchObject({
...action,
attributes: {
...action.attributes,
config: {
hasAuth: false,
},
},
});
});
});
function getMockDataForWebhook(
overwrites: Record<string, unknown> = {},
hasUserAndPassword: boolean
): SavedObjectUnsanitizedDoc<RawAction> {
const secrets = hasUserAndPassword
? { user: 'test', password: '123' }
: { user: '', password: '' };
return {
attributes: {
name: 'abc',
actionTypeId: '.webhook',
config: {},
secrets,
...overwrites,
},
id: uuid.v4(),
type: 'action',
};
}
function getMockDataForEmail(
overwrites: Record<string, unknown> = {}
): SavedObjectUnsanitizedDoc<RawAction> {

10
x-pack/plugins/actions/server/saved_objects/migrations.ts
View file

@ -25,8 +25,18 @@ export function getMigrations(
pipeMigrations(renameCasesConfigurationObject, addHasAuthConfigurationObject)
);
const migrationWebhookConnectorHasAuth = encryptedSavedObjects.createMigration<
RawAction,
RawAction
>(
(doc): doc is SavedObjectUnsanitizedDoc<RawAction> =>
doc.attributes.actionTypeId === '.webhook',
pipeMigrations(addHasAuthConfigurationObject)
);
return {
'7.10.0': executeMigrationWithErrorHandling(migrationActions, '7.10.0'),
'7.11.0': executeMigrationWithErrorHandling(migrationWebhookConnectorHasAuth, '7.11.0'),
};
}

1
x-pack/plugins/translations/translations/ja-JP.json
View file

@ -20245,7 +20245,6 @@
"xpack.triggersActionsUI.sections.actionsConnectorsList.unableToLoadActionTypesMessage": "アクションタイプを読み込めません",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderKeyText": "キーが必要です。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderValueText": "値が必要です。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText": "ユーザー名が必要です。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredMethodText": "メソッドが必要です",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText": "パスワードが必要です。",
"xpack.triggersActionsUI.sections.addAlert.error.greaterThenThreshold0Text": "しきい値 1 はしきい値 0 よりも大きい値にしてください。",

1
x-pack/plugins/translations/translations/zh-CN.json
View file

@ -20265,7 +20265,6 @@
"xpack.triggersActionsUI.sections.actionsConnectorsList.unableToLoadActionTypesMessage": "无法加载操作类型",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderKeyText": "“键”必填。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHeaderValueText": "“值”必填。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText": "“用户名”必填。",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredMethodText": "“方法”必填",
"xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText": "“密码”必填。",
"xpack.triggersActionsUI.sections.addAlert.error.greaterThenThreshold0Text": "阈值 1 应 > 阈值 0。",

1
x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts
View file

@ -110,6 +110,7 @@ export interface WebhookConfig {
method: string;
url: string;
headers: Record<string, string>;
hasAuth: boolean;
}
export interface WebhookSecrets {

35
x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx
View file

@ -28,7 +28,7 @@ describe('actionTypeRegistry.get() works', () => {
});
describe('webhook connector validation', () => {
test('connector validation succeeds when connector config is valid', () => {
test('connector validation succeeds when hasAuth is true and connector config is valid', () => {
const actionConnector = {
secrets: {
user: 'user',
@ -42,6 +42,35 @@ describe('webhook connector validation', () => {
method: 'PUT',
url: 'http://test.com',
headers: { 'content-type': 'text' },
hasAuth: true,
},
} as WebhookActionConnector;
expect(actionTypeModel.validateConnector(actionConnector)).toEqual({
errors: {
url: [],
method: [],
user: [],
password: [],
},
});
});
test('connector validation succeeds when hasAuth is false and connector config is valid', () => {
const actionConnector = {
secrets: {
user: '',
password: '',
},
id: 'test',
actionTypeId: '.webhook',
name: 'webhook',
isPreconfigured: false,
config: {
method: 'PUT',
url: 'http://test.com',
headers: { 'content-type': 'text' },
hasAuth: false,
},
} as WebhookActionConnector;
@ -65,6 +94,7 @@ describe('webhook connector validation', () => {
name: 'webhook',
config: {
method: 'PUT',
hasAuth: true,
},
} as WebhookActionConnector;
@ -73,7 +103,7 @@ describe('webhook connector validation', () => {
url: ['URL is required.'],
method: [],
user: [],
password: ['Password is required.'],
password: ['Password is required when username is used.'],
},
});
});
@ -90,6 +120,7 @@ describe('webhook connector validation', () => {
config: {
method: 'PUT',
url: 'invalid.url',
hasAuth: true,
},
} as WebhookActionConnector;

28
x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx
View file

@ -74,22 +74,42 @@ export function getActionType(): ActionTypeModel<
)
);
}
if (!action.secrets.user && action.secrets.password) {
if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) {
errors.user.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText',
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredAuthUserNameText',
{
defaultMessage: 'Username is required.',
}
)
);
}
if (!action.secrets.password && action.secrets.user) {
if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) {
errors.password.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredAuthPasswordText',
{
defaultMessage: 'Password is required.',
}
)
);
}
if (action.secrets.user && !action.secrets.password) {
errors.password.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText',
{
defaultMessage: 'Password is required.',
defaultMessage: 'Password is required when username is used.',
}
)
);
}
if (!action.secrets.user && action.secrets.password) {
errors.user.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredUserText',
{
defaultMessage: 'Username is required when password is used.',
}
)
);

6
x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx
View file

@ -24,6 +24,7 @@ describe('WebhookActionConnectorFields renders', () => {
method: 'PUT',
url: 'http:\\test',
headers: { 'content-type': 'text' },
hasAuth: true,
},
} as WebhookActionConnector;
const wrapper = mountWithIntl(
@ -50,7 +51,9 @@ describe('WebhookActionConnectorFields renders', () => {
secrets: {},
actionTypeId: '.webhook',
isPreconfigured: false,
config: {},
config: {
hasAuth: true,
},
} as WebhookActionConnector;
const wrapper = mountWithIntl(
<WebhookActionConnectorFields
@ -80,6 +83,7 @@ describe('WebhookActionConnectorFields renders', () => {
method: 'PUT',
url: 'http:\\test',
headers: { 'content-type': 'text' },
hasAuth: true,
},
} as WebhookActionConnector;
const wrapper = mountWithIntl(

206
x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook_connectors.tsx
View file

@ -3,7 +3,7 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import React, { Fragment, useState } from 'react';
import React, { Fragment, useEffect, useState } from 'react';
import { FormattedMessage } from '@kbn/i18n/react';
import {
@ -34,12 +34,19 @@ const WebhookActionConnectorFields: React.FunctionComponent<ActionConnectorField
WebhookActionConnector
>> = ({ action, editActionConfig, editActionSecrets, errors, readOnly }) => {
const { user, password } = action.secrets;
const { method, url, headers } = action.config;
const { method, url, headers, hasAuth } = action.config;
const [httpHeaderKey, setHttpHeaderKey] = useState<string>('');
const [httpHeaderValue, setHttpHeaderValue] = useState<string>('');
const [hasHeaders, setHasHeaders] = useState<boolean>(false);
useEffect(() => {
if (!action.id) {
editActionConfig('hasAuth', true);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
if (!method) {
editActionConfig('method', 'post'); // set method to POST by default
}
@ -268,9 +275,9 @@ const WebhookActionConnectorFields: React.FunctionComponent<ActionConnectorField
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiFlexGroup>
<EuiFlexItem>
<EuiSpacer size="m" />
<EuiTitle size="xxs">
<h4>
<FormattedMessage
@ -279,80 +286,95 @@ const WebhookActionConnectorFields: React.FunctionComponent<ActionConnectorField
/>
</h4>
</EuiTitle>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiFlexGroup>
<EuiFlexItem>
<EuiFormRow fullWidth>{getEncryptedFieldNotifyLabel(!action.id)}</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
<EuiSpacer size="m" />
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem>
<EuiFormRow
id="webhookUser"
fullWidth
error={errors.user}
isInvalid={errors.user.length > 0 && user !== undefined}
<EuiSpacer size="s" />
<EuiSwitch
label={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.userTextFieldLabel',
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.hasAuthSwitchLabel',
{
defaultMessage: 'Username',
defaultMessage: 'Require authentication for this webhook',
}
)}
>
<EuiFieldText
fullWidth
isInvalid={errors.user.length > 0 && user !== undefined}
name="user"
readOnly={readOnly}
value={user || ''}
data-test-subj="webhookUserInput"
onChange={(e) => {
editActionSecrets('user', e.target.value);
}}
onBlur={() => {
if (!user) {
editActionSecrets('user', '');
}
}}
/>
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem>
<EuiFormRow
id="webhookPassword"
fullWidth
error={errors.password}
isInvalid={errors.password.length > 0 && password !== undefined}
label={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.passwordTextFieldLabel',
{
defaultMessage: 'Password',
disabled={readOnly}
checked={hasAuth}
onChange={(e) => {
editActionConfig('hasAuth', e.target.checked);
if (!e.target.checked) {
editActionSecrets('user', null);
editActionSecrets('password', null);
}
)}
>
<EuiFieldPassword
fullWidth
name="password"
readOnly={readOnly}
isInvalid={errors.password.length > 0 && password !== undefined}
value={password || ''}
data-test-subj="webhookPasswordInput"
onChange={(e) => {
editActionSecrets('password', e.target.value);
}}
onBlur={() => {
if (!password) {
editActionSecrets('password', '');
}
}}
/>
</EuiFormRow>
}}
/>
</EuiFlexItem>
</EuiFlexGroup>
{hasAuth ? (
<>
{getEncryptedFieldNotifyLabel(!action.id)}
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem>
<EuiFormRow
id="webhookUser"
fullWidth
error={errors.user}
isInvalid={errors.user.length > 0 && user !== undefined}
label={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.userTextFieldLabel',
{
defaultMessage: 'Username',
}
)}
>
<EuiFieldText
fullWidth
isInvalid={errors.user.length > 0 && user !== undefined}
name="user"
readOnly={readOnly}
value={user || ''}
data-test-subj="webhookUserInput"
onChange={(e) => {
editActionSecrets('user', e.target.value);
}}
onBlur={() => {
if (!user) {
editActionSecrets('user', '');
}
}}
/>
</EuiFormRow>
</EuiFlexItem>
<EuiFlexItem>
<EuiFormRow
id="webhookPassword"
fullWidth
error={errors.password}
isInvalid={errors.password.length > 0 && password !== undefined}
label={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.passwordTextFieldLabel',
{
defaultMessage: 'Password',
}
)}
>
<EuiFieldPassword
fullWidth
name="password"
readOnly={readOnly}
isInvalid={errors.password.length > 0 && password !== undefined}
value={password || ''}
data-test-subj="webhookPasswordInput"
onChange={(e) => {
editActionSecrets('password', e.target.value);
}}
onBlur={() => {
if (!password) {
editActionSecrets('password', '');
}
}}
/>
</EuiFormRow>
</EuiFlexItem>
</EuiFlexGroup>
</>
) : null}
<EuiSpacer size="m" />
<EuiSwitch
data-test-subj="webhookViewHeadersSwitch"
@ -395,27 +417,35 @@ const WebhookActionConnectorFields: React.FunctionComponent<ActionConnectorField
function getEncryptedFieldNotifyLabel(isCreate: boolean) {
if (isCreate) {
return (
<EuiText size="s" data-test-subj="rememberValuesMessage">
<FormattedMessage
id="xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.rememberValuesLabel"
defaultMessage="Remember these values. You must reenter them each time you edit the connector."
/>
</EuiText>
<Fragment>
<EuiSpacer size="s" />
<EuiText size="s" data-test-subj="rememberValuesMessage">
<FormattedMessage
id="xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.rememberValuesLabel"
defaultMessage="Remember these values. You must reenter them each time you edit the connector."
/>
</EuiText>
<EuiSpacer size="s" />
</Fragment>
);
}
return (
<EuiCallOut
size="s"
iconType="iInCircle"
data-test-subj="reenterValuesMessage"
title={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.reenterValuesLabel',
{
defaultMessage:
'Username and password are encrypted. Please reenter values for these fields.',
}
)}
/>
<Fragment>
<EuiSpacer size="m" />
<EuiCallOut
size="s"
iconType="iInCircle"
data-test-subj="reenterValuesMessage"
title={i18n.translate(
'xpack.triggersActionsUI.components.builtinActionTypes.webhookAction.reenterValuesLabel',
{
defaultMessage:
'Username and password are encrypted. Please reenter values for these fields.',
}
)}
/>
<EuiSpacer size="m" />
</Fragment>
);
}

1
x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/webhook.ts
View file

@ -20,6 +20,7 @@ import {
const defaultValues: Record<string, any> = {
headers: null,
method: 'post',
hasAuth: true,
};
function parsePort(url: Record<string, string>): Record<string, string | null | number> {

17
x-pack/test/alerting_api_integration/spaces_only/tests/actions/migrations.ts
View file

@ -55,5 +55,22 @@ export default function createGetTests({ getService }: FtrProviderContext) {
projectKey: 'CK',
});
});
it('7.11.0 migrates webhook connector configurations to have `hasAuth` property', async () => {
const responseWithAuth = await supertest.get(
`${getUrlPrefix(``)}/api/actions/action/949f909b-20a0-46e3-aadb-6a4d117bb592`
);
expect(responseWithAuth.status).to.eql(200);
expect(responseWithAuth.body.config).key('hasAuth');
expect(responseWithAuth.body.config.hasAuth).to.eql(true);
const responseNoAuth = await supertest.get(
`${getUrlPrefix(``)}/api/actions/action/7434121e-045a-47d6-a0a6-0b6da752397a`
);
expect(responseNoAuth.status).to.eql(200);
expect(responseNoAuth.body.config).key('hasAuth');
expect(responseNoAuth.body.config.hasAuth).to.eql(false);
});
});
}

54
x-pack/test/functional/es_archives/actions/data.json
View file

@ -56,3 +56,57 @@
"type": "_doc"
}
}
{
"type": "doc",
"value": {
"id": "action:949f909b-20a0-46e3-aadb-6a4d117bb592",
"index": ".kibana_1",
"source": {
"action": {
"actionTypeId": ".webhook",
"config": {
"headers": null,
"method": "post",
"url": "http://localhost"
},
"name": "A webhook with auth",
"secrets": "LUqlrITACjqPmcWGlbl+H4RsGGOlw8LM0Urq8r7y6jNT7Igv3J7FjKJ2NXfNTaghVBO7e9x3wZOtiycwyoAdviTyYm1pspni24vH+OT70xaSuXcDoxfGwiLEcaG04INDnUJX4dtmRerxqR9ChktC70LNtOU3sqjYI2tWt2vOqGeq"
},
"migrationVersion": {
"action": "7.10.0"
},
"references": [
],
"type": "action",
"updated_at": "2020-10-26T21:29:47.380Z"
}
}
}
{
"type": "doc",
"value": {
"id": "action:7434121e-045a-47d6-a0a6-0b6da752397a",
"index": ".kibana_1",
"source": {
"action": {
"actionTypeId": ".webhook",
"config": {
"headers": null,
"method": "post",
"url": "http://localhost"
},
"name": "A webhook with no auth",
"secrets": "tOwFq20hbUrcp3FX7stKB5aJaQQdLNQwomSNym8BgnFaBBafPOASv5T0tGdGsTr/CA7VK+N/wYBHQPzt0apF8Z/UYl63ZXqck5tSoFDnQW77zv1VVQ5wEwN1qkAQQcfrXTXU2wYVAYZNSuHkbeRjcasfG0ty1K+J7A=="
},
"migrationVersion": {
"action": "7.10.0"
},
"references": [
],
"type": "action",
"updated_at": "2020-10-26T21:30:35.146Z"
}
}
}