Extended Email action configuration with hasAuth property to identify…

… if the connector require user credentials. Improved UX for Email connector (#78235) * Extended Email action configuration with hasAuth property to identify if the connector require user credentials. Improved UX for Email connector * Fixed failing tests and comments * Fixed type check and reverted logic of Add user and password switch button * Fixed due to the latest design requirenments * Fixed due to review comments
elastic · Oct 1, 2020 · 63ff060 · 63ff060
1 parent 8406e04
commit 63ff060
Show file tree

Hide file tree

Showing 13 changed files with 345 additions and 99 deletions.
diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts
@@ -55,6 +55,7 @@ describe('config validation', () => {
     const config: Record<string, unknown> = {
       service: 'gmail',
       from: 'bob@example.com',
+      hasAuth: true,
     };
     expect(validateConfig(actionType, config)).toEqual({
       ...config,
@@ -66,6 +67,7 @@ describe('config validation', () => {
     delete config.service;
     config.host = 'elastic.co';
     config.port = 8080;
+    config.hasAuth = true;
     expect(validateConfig(actionType, config)).toEqual({
       ...config,
       service: null,
@@ -233,6 +235,7 @@ describe('execute()', () => {
       port: 42,
       secure: true,
       from: 'bob@example.com',
+      hasAuth: true,
     };
     const secrets: ActionTypeSecretsType = {
       user: 'bob',
@@ -269,6 +272,7 @@ describe('execute()', () => {
               "message": "a message to you",
               "subject": "the subject",
             },
+            "hasAuth": true,
             "proxySettings": undefined,
             "routing": Object {
               "bcc": Array [
@@ -298,6 +302,7 @@ describe('execute()', () => {
       port: 42,
       secure: true,
       from: 'bob@example.com',
+      hasAuth: false,
     };
     const secrets: ActionTypeSecretsType = {
       user: null,
@@ -327,6 +332,7 @@ describe('execute()', () => {
           "message": "a message to you",
           "subject": "the subject",
         },
+        "hasAuth": false,
         "proxySettings": undefined,
         "routing": Object {
           "bcc": Array [
@@ -356,6 +362,7 @@ describe('execute()', () => {
       port: 42,
       secure: true,
       from: 'bob@example.com',
+      hasAuth: false,
     };
     const secrets: ActionTypeSecretsType = {
       user: null,

diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.ts b/x-pack/plugins/actions/server/builtin_action_types/email.ts
@@ -36,6 +36,7 @@ const ConfigSchemaProps = {
   port: schema.nullable(portSchema()),
   secure: schema.nullable(schema.boolean()),
   from: schema.string(),
+  hasAuth: schema.boolean({ defaultValue: true }),
 };
 
 const ConfigSchema = schema.object(ConfigSchemaProps);
@@ -185,6 +186,7 @@ async function executor(
       message: params.message,
     },
     proxySettings: execOptions.proxySettings,
+    hasAuth: config.hasAuth,
   };
 
   let result;

diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.test.ts
@@ -64,7 +64,7 @@ describe('send_email module', () => {
   });
 
   test('handles unauthenticated email using not secure host/port', async () => {
-    const sendEmailOptions = getSendEmailOptions(
+    const sendEmailOptions = getSendEmailOptionsNoAuth(
       {
         transport: {
           host: 'example.com',
@@ -76,12 +76,7 @@ describe('send_email module', () => {
         proxyRejectUnauthorizedCertificates: false,
       }
     );
-    // @ts-expect-error
-    delete sendEmailOptions.transport.service;
-    // @ts-expect-error
-    delete sendEmailOptions.transport.user;
-    // @ts-expect-error
-    delete sendEmailOptions.transport.password;
+
     const result = await sendEmail(mockLogger, sendEmailOptions);
     expect(result).toBe(sendMailMockResult);
     expect(createTransportMock.mock.calls[0]).toMatchInlineSnapshot(`
@@ -248,5 +243,31 @@ function getSendEmailOptions(
       password: 'changeme',
     },
     proxySettings,
+    hasAuth: true,
+  };
+}
+
+function getSendEmailOptionsNoAuth(
+  { content = {}, routing = {}, transport = {} } = {},
+  proxySettings?: ProxySettings
+) {
+  return {
+    content: {
+      ...content,
+      message: 'a message',
+      subject: 'a subject',
+    },
+    routing: {
+      ...routing,
+      from: 'fred@example.com',
+      to: ['jim@example.com'],
+      cc: ['bob@example.com', 'robert@example.com'],
+      bcc: [],
+    },
+    transport: {
+      ...transport,
+    },
+    proxySettings,
+    hasAuth: false,
   };
 }
diff --git a/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.ts b/x-pack/plugins/actions/server/builtin_action_types/lib/send_email.ts
@@ -20,6 +20,7 @@ export interface SendEmailOptions {
   content: Content;
   proxySettings?: ProxySettings;
   rejectUnauthorized?: boolean;
+  hasAuth: boolean;
 }
 
 // config validation ensures either service is set or host/port are set
@@ -46,14 +47,14 @@ export interface Content {
 
 // send an email
 export async function sendEmail(logger: Logger, options: SendEmailOptions): Promise<unknown> {
-  const { transport, routing, content, proxySettings, rejectUnauthorized } = options;
+  const { transport, routing, content, proxySettings, rejectUnauthorized, hasAuth } = options;
   const { service, host, port, secure, user, password } = transport;
   const { from, to, cc, bcc } = routing;
   const { subject, message } = content;
 
   const transportConfig: Record<string, unknown> = {};
 
-  if (user != null && password != null) {
+  if (hasAuth && user != null && password != null) {
     transportConfig.auth = {
       user,
       pass: 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
@@ -21,6 +21,20 @@ describe('7.10.0', () => {
     );
   });
 
+  test('add hasAuth config property for .email actions', () => {
+    const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
+    const action = getMockDataForEmail({});
+    expect(migration710(action, context)).toMatchObject({
+      ...action,
+      attributes: {
+        ...action.attributes,
+        config: {
+          hasAuth: true,
+        },
+      },
+    });
+  });
+
   test('rename cases configuration object', () => {
     const migration710 = getMigrations(encryptedSavedObjectsSetup)['7.10.0'];
     const action = getMockData({});
@@ -36,6 +50,22 @@ describe('7.10.0', () => {
   });
 });
 
+function getMockDataForEmail(
+  overwrites: Record<string, unknown> = {}
+): SavedObjectUnsanitizedDoc<RawAction> {
+  return {
+    attributes: {
+      name: 'abc',
+      actionTypeId: '.email',
+      config: {},
+      secrets: { user: 'test', password: '123' },
+      ...overwrites,
+    },
+    id: uuid.v4(),
+    type: 'action',
+  };
+}
+
 function getMockData(
   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
@@ -3,40 +3,87 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
-
 import {
   SavedObjectMigrationMap,
   SavedObjectUnsanitizedDoc,
   SavedObjectMigrationFn,
+  SavedObjectMigrationContext,
 } from '../../../../../src/core/server';
 import { RawAction } from '../types';
 import { EncryptedSavedObjectsPluginSetup } from '../../../encrypted_saved_objects/server';
 
+type ActionMigration = (
+  doc: SavedObjectUnsanitizedDoc<RawAction>
+) => SavedObjectUnsanitizedDoc<RawAction>;
+
 export function getMigrations(
   encryptedSavedObjects: EncryptedSavedObjectsPluginSetup
 ): SavedObjectMigrationMap {
-  return { '7.10.0': renameCasesConfigurationObject(encryptedSavedObjects) };
+  const migrationActions = encryptedSavedObjects.createMigration<RawAction, RawAction>(
+    (doc): doc is SavedObjectUnsanitizedDoc<RawAction> =>
+      !!doc.attributes.config?.casesConfiguration || doc.attributes.actionTypeId === '.email',
+    pipeMigrations(renameCasesConfigurationObject, addHasAuthConfigurationObject)
+  );
+
+  return {
+    '7.10.0': executeMigrationWithErrorHandling(migrationActions, '7.10.0'),
+  };
 }
 
-const renameCasesConfigurationObject = (
-  encryptedSavedObjects: EncryptedSavedObjectsPluginSetup
-): SavedObjectMigrationFn<RawAction, RawAction> => {
-  return encryptedSavedObjects.createMigration<RawAction, RawAction>(
-    (doc): doc is SavedObjectUnsanitizedDoc<RawAction> =>
-      !!doc.attributes.config?.casesConfiguration,
-    (doc: SavedObjectUnsanitizedDoc<RawAction>): SavedObjectUnsanitizedDoc<RawAction> => {
-      const { casesConfiguration, ...restConfiguration } = doc.attributes.config;
-
-      return {
-        ...doc,
-        attributes: {
-          ...doc.attributes,
-          config: {
-            ...restConfiguration,
-            incidentConfiguration: casesConfiguration,
-          },
-        },
-      };
+function executeMigrationWithErrorHandling(
+  migrationFunc: SavedObjectMigrationFn<RawAction, RawAction>,
+  version: string
+) {
+  return (doc: SavedObjectUnsanitizedDoc<RawAction>, context: SavedObjectMigrationContext) => {
+    try {
+      return migrationFunc(doc, context);
+    } catch (ex) {
+      context.log.error(
+        `encryptedSavedObject ${version} migration failed for action ${doc.id} with error: ${ex.message}`,
+        { actionDocument: doc }
+      );
     }
-  );
+    return doc;
+  };
+}
+
+function renameCasesConfigurationObject(
+  doc: SavedObjectUnsanitizedDoc<RawAction>
+): SavedObjectUnsanitizedDoc<RawAction> {
+  if (!doc.attributes.config?.casesConfiguration) {
+    return doc;
+  }
+  const { casesConfiguration, ...restConfiguration } = doc.attributes.config;
+
+  return {
+    ...doc,
+    attributes: {
+      ...doc.attributes,
+      config: {
+        ...restConfiguration,
+        incidentConfiguration: casesConfiguration,
+      },
+    },
+  };
+}
+
+const addHasAuthConfigurationObject = (
+  doc: SavedObjectUnsanitizedDoc<RawAction>
+): SavedObjectUnsanitizedDoc<RawAction> => {
+  const hasAuth = !!doc.attributes.secrets.user || !!doc.attributes.secrets.password;
+  return {
+    ...doc,
+    attributes: {
+      ...doc.attributes,
+      config: {
+        ...doc.attributes.config,
+        hasAuth,
+      },
+    },
+  };
 };
+
+function pipeMigrations(...migrations: ActionMigration[]): ActionMigration {
+  return (doc: SavedObjectUnsanitizedDoc<RawAction>) =>
+    migrations.reduce((migratedDoc, nextMigration) => nextMigration(migratedDoc), doc);
+}
diff --git a/...iggers_actions_ui/public/application/components/builtin_action_types/email/email.test.tsx b/...iggers_actions_ui/public/application/components/builtin_action_types/email/email.test.tsx
@@ -43,6 +43,7 @@ describe('connector validation', () => {
         port: 2323,
         host: 'localhost',
         test: 'test',
+        hasAuth: true,
       },
     } as EmailActionConnector;
 
@@ -72,6 +73,7 @@ describe('connector validation', () => {
         port: 2323,
         host: 'localhost',
         test: 'test',
+        hasAuth: false,
       },
     } as EmailActionConnector;
 
@@ -96,6 +98,7 @@ describe('connector validation', () => {
       name: 'email',
       config: {
         from: 'test@test.com',
+        hasAuth: true,
       },
     } as EmailActionConnector;
 
@@ -124,6 +127,7 @@ describe('connector validation', () => {
         port: 2323,
         host: 'localhost',
         test: 'test',
+        hasAuth: true,
       },
     } as EmailActionConnector;
 
@@ -152,6 +156,7 @@ describe('connector validation', () => {
         port: 2323,
         host: 'localhost',
         test: 'test',
+        hasAuth: true,
       },
     } as EmailActionConnector;
 

diff --git a/...ns/triggers_actions_ui/public/application/components/builtin_action_types/email/email.tsx b/...ns/triggers_actions_ui/public/application/components/builtin_action_types/email/email.tsx
@@ -75,6 +75,26 @@ export function getActionType(): ActionTypeModel<EmailConfig, EmailSecrets, Emai
           )
         );
       }
+      if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) {
+        errors.user.push(
+          i18n.translate(
+            'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredAuthUserNameText',
+            {
+              defaultMessage: 'Username is required.',
+            }
+          )
+        );
+      }
+      if (action.config.hasAuth && !action.secrets.user && !action.secrets.password) {
+        errors.password.push(
+          i18n.translate(
+            'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredAuthPasswordText',
+            {
+              defaultMessage: 'Password is required.',
+            }
+          )
+        );
+      }
       if (action.secrets.user && !action.secrets.password) {
         errors.password.push(
           i18n.translate(