Revert "[Actions] Adding hasAuth to Webhook Configuration to avoid …

…confusing UX (#81390)" This reverts commit fd7f6b5.
elastic · Oct 27, 2020 · 9d602e3 · 9d602e3
1 parent 2d0106d
commit 9d602e3
Show file tree

Hide file tree

Showing 14 changed files with 102 additions and 333 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,9 +90,8 @@ describe('config validation', () => {
   };
 
   test('config validation passes when only required fields are provided', () => {
-    const config: Record<string, string | boolean> = {
+    const config: Record<string, string> = {
       url: 'http://mylisteningserver:9200/endpoint',
-      hasAuth: true,
     };
     expect(validateConfig(actionType, config)).toEqual({
       ...defaultValues,
@@ -102,10 +101,9 @@ describe('config validation', () => {
 
   test('config validation passes when valid methods are provided', () => {
     ['post', 'put'].forEach((method) => {
-      const config: Record<string, string | boolean> = {
+      const config: Record<string, string> = {
         url: 'http://mylisteningserver:9200/endpoint',
         method,
-        hasAuth: true,
       };
       expect(validateConfig(actionType, config)).toEqual({
         ...defaultValues,
@@ -129,9 +127,8 @@ describe('config validation', () => {
   });
 
   test('config validation passes when a url is specified', () => {
-    const config: Record<string, string | boolean> = {
+    const config: Record<string, string> = {
       url: 'http://mylisteningserver:9200/endpoint',
-      hasAuth: true,
     };
     expect(validateConfig(actionType, config)).toEqual({
       ...defaultValues,
@@ -158,7 +155,6 @@ describe('config validation', () => {
       headers: {
         'Content-Type': 'application/json',
       },
-      hasAuth: true,
     };
     expect(validateConfig(actionType, config)).toEqual({
       ...defaultValues,
@@ -188,7 +184,6 @@ describe('config validation', () => {
       headers: {
         'Content-Type': 'application/json',
       },
-      hasAuth: true,
     };
 
     expect(validateConfig(actionType, config)).toEqual({
@@ -268,7 +263,6 @@ describe('execute()', () => {
       headers: {
         aheader: 'a value',
       },
-      hasAuth: true,
     };
     await actionType.executor({
       actionId: 'some-id',
@@ -326,7 +320,6 @@ 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,7 +42,6 @@ const configSchemaProps = {
     defaultValue: WebhookMethods.POST,
   }),
   headers: nullableType(HeadersSchema),
-  hasAuth: schema.boolean({ defaultValue: true }),
 };
 const ConfigSchema = schema.object(configSchemaProps);
 export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;
@@ -129,12 +128,12 @@ export async function executor(
   execOptions: WebhookActionTypeExecutorOptions
 ): Promise<ActionTypeExecutorResult<unknown>> {
   const actionId = execOptions.actionId;
-  const { method, url, headers = {}, hasAuth } = execOptions.config;
+  const { method, url, headers = {} } = execOptions.config;
   const { body: data } = execOptions.params;
 
   const secrets: ActionTypeSecretsType = execOptions.secrets;
   const basicAuth =
-    hasAuth && isString(secrets.user) && isString(secrets.password)
+    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,63 +58,6 @@ 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,18 +25,8 @@ 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
@@ -20244,6 +20244,7 @@
     "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
@@ -20264,6 +20264,7 @@
     "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,7 +110,6 @@ 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 hasAuth is true and connector config is valid', () => {
+  test('connector validation succeeds when connector config is valid', () => {
     const actionConnector = {
       secrets: {
         user: 'user',
@@ -42,35 +42,6 @@ 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;
 
@@ -94,7 +65,6 @@ describe('webhook connector validation', () => {
       name: 'webhook',
       config: {
         method: 'PUT',
-        hasAuth: true,
       },
     } as WebhookActionConnector;
 
@@ -103,7 +73,7 @@ describe('webhook connector validation', () => {
         url: ['URL is required.'],
         method: [],
         user: [],
-        password: ['Password is required when username is used.'],
+        password: ['Password is required.'],
       },
     });
   });
@@ -120,7 +90,6 @@ 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,42 +74,22 @@ export function getActionType(): ActionTypeModel<
           )
         );
       }
-      if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) {
+      if (!action.secrets.user && action.secrets.password) {
         errors.user.push(
           i18n.translate(
-            'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredAuthUserNameText',
+            'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText',
             {
               defaultMessage: 'Username is required.',
             }
           )
         );
       }
-      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) {
+      if (!action.secrets.password && action.secrets.user) {
         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.',
+              defaultMessage: 'Password is required.',
             }
           )
         );

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