[Actions] Adding hasAuth to Webhook Configuration to avoid confusin…

…g UX (elastic#81390) * 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>
ymao1 · Oct 27, 2020 · 129de8e · 129de8e
1 parent c87aa13
commit 129de8e
Show file tree

Hide file tree

Showing 14 changed files with 333 additions and 102 deletions.
diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts
@@ -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({

diff --git a/x-pack/plugins/actions/server/builtin_action_types/webhook.ts b/x-pack/plugins/actions/server/builtin_action_types/webhook.ts
@@ -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 } }
       : {};
 

diff --git a/x-pack/plugins/actions/server/saved_objects/migrations.test.ts b/x-pack/plugins/actions/server/saved_objects/migrations.test.ts
@@ -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> {

diff --git a/x-pack/plugins/actions/server/saved_objects/migrations.ts b/x-pack/plugins/actions/server/saved_objects/migrations.ts
@@ -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'),
   };
 }
 

diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json
@@ -20258,7 +20258,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 よりも大きい値にしてください。",

diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json
@@ -20278,7 +20278,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。",

diff --git a/...k/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts b/...k/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts
@@ -110,6 +110,7 @@ export interface WebhookConfig {
   method: string;
   url: string;
   headers: Record<string, string>;
+  hasAuth: boolean;
 }
 
 export interface WebhookSecrets {

diff --git a/...rs_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx b/...rs_actions_ui/public/application/components/builtin_action_types/webhook/webhook.test.tsx
@@ -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;
 

diff --git a/...riggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx b/...riggers_actions_ui/public/application/components/builtin_action_types/webhook/webhook.tsx
@@ -74,26 +74,46 @@ 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.requiredPasswordText',
+            '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 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.',
+            }
+          )
+        );
+      }
       return validationResult;
     },
     validateParams: (actionParams: WebhookActionParams): ValidationResult => {

diff --git a/...ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx b/...ui/public/application/components/builtin_action_types/webhook/webhook_connectors.test.tsx
@@ -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(