From 72bc0eae3268b1dfb7c314e80438c7a88f61352e Mon Sep 17 00:00:00 2001 From: Patrick Mueller Date: Mon, 23 Mar 2020 19:02:28 -0400 Subject: [PATCH] [Alerting] allow email action to not require auth (#60839) resolves https://github.com/elastic/kibana/issues/57143 Currently, the built-in email action requires user/password properties to be set in it's secrets parameters. This PR changes that requirement, so they are no longer required. --- .../server/builtin_action_types/email.test.ts | 14 ++-- .../server/builtin_action_types/email.ts | 25 +++--- .../builtin_action_types/email.test.tsx | 81 +++++++++++++++++++ .../components/builtin_action_types/email.tsx | 52 ++++++------ .../components/builtin_action_types/types.ts | 4 +- .../actions/builtin_action_types/email.ts | 56 +++++++++++++ 6 files changed, 185 insertions(+), 47 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 index 0bd3992de30e..469df4fd86e2 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.test.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.test.ts @@ -184,12 +184,14 @@ describe('secrets validation', () => { expect(validateSecrets(actionType, secrets)).toEqual(secrets); }); - test('secrets validation fails when secrets is not valid', () => { - expect(() => { - validateSecrets(actionType, {}); - }).toThrowErrorMatchingInlineSnapshot( - `"error validating action type secrets: [user]: expected value of type [string] but got [undefined]"` - ); + test('secrets validation succeeds when secrets props are null/undefined', () => { + const secrets: Record = { + user: null, + password: null, + }; + expect(validateSecrets(actionType, {})).toEqual(secrets); + expect(validateSecrets(actionType, { user: null })).toEqual(secrets); + expect(validateSecrets(actionType, { password: null })).toEqual(secrets); }); }); diff --git a/x-pack/plugins/actions/server/builtin_action_types/email.ts b/x-pack/plugins/actions/server/builtin_action_types/email.ts index 16e0168a7deb..7992920fdfcb 100644 --- a/x-pack/plugins/actions/server/builtin_action_types/email.ts +++ b/x-pack/plugins/actions/server/builtin_action_types/email.ts @@ -10,7 +10,6 @@ import { schema, TypeOf } from '@kbn/config-schema'; import nodemailerGetService from 'nodemailer/lib/well-known'; import { sendEmail, JSON_TRANSPORT_SERVICE } from './lib/send_email'; -import { nullableType } from './lib/nullable'; import { portSchema } from './lib/schemas'; import { Logger } from '../../../../../src/core/server'; import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types'; @@ -20,10 +19,10 @@ import { ActionsConfigurationUtilities } from '../actions_config'; export type ActionTypeConfigType = TypeOf; const ConfigSchemaProps = { - service: nullableType(schema.string()), - host: nullableType(schema.string()), - port: nullableType(portSchema()), - secure: nullableType(schema.boolean()), + service: schema.nullable(schema.string()), + host: schema.nullable(schema.string()), + port: schema.nullable(portSchema()), + secure: schema.nullable(schema.boolean()), from: schema.string(), }; @@ -75,8 +74,8 @@ function validateConfig( export type ActionTypeSecretsType = TypeOf; const SecretsSchema = schema.object({ - user: schema.string(), - password: schema.string(), + user: schema.nullable(schema.string()), + password: schema.nullable(schema.string()), }); // params definition @@ -144,10 +143,14 @@ async function executor( const secrets = execOptions.secrets as ActionTypeSecretsType; const params = execOptions.params as ActionParamsType; - const transport: any = { - user: secrets.user, - password: secrets.password, - }; + const transport: any = {}; + + if (secrets.user != null) { + transport.user = secrets.user; + } + if (secrets.password != null) { + transport.password = secrets.password; + } if (config.service !== null) { transport.service = config.service; diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.test.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.test.tsx index 49a611167cf1..a7d479f922ed 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.test.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.test.tsx @@ -58,6 +58,33 @@ describe('connector validation', () => { }); }); + test('connector validation succeeds when connector config is valid with empty user/password', () => { + const actionConnector = { + secrets: { + user: null, + password: null, + }, + id: 'test', + actionTypeId: '.email', + name: 'email', + config: { + from: 'test@test.com', + port: 2323, + host: 'localhost', + test: 'test', + }, + } as EmailActionConnector; + + expect(actionTypeModel.validateConnector(actionConnector)).toEqual({ + errors: { + from: [], + port: [], + host: [], + user: [], + password: [], + }, + }); + }); test('connector validation fails when connector config is not valid', () => { const actionConnector = { secrets: { @@ -82,6 +109,60 @@ describe('connector validation', () => { }, }); }); + test('connector validation fails when user specified but not password', () => { + const actionConnector = { + secrets: { + user: 'user', + password: null, + }, + id: 'test', + actionTypeId: '.email', + name: 'email', + config: { + from: 'test@test.com', + port: 2323, + host: 'localhost', + test: 'test', + }, + } as EmailActionConnector; + + expect(actionTypeModel.validateConnector(actionConnector)).toEqual({ + errors: { + from: [], + port: [], + host: [], + user: [], + password: ['Password is required when username is used.'], + }, + }); + }); + test('connector validation fails when password specified but not user', () => { + const actionConnector = { + secrets: { + user: null, + password: 'password', + }, + id: 'test', + actionTypeId: '.email', + name: 'email', + config: { + from: 'test@test.com', + port: 2323, + host: 'localhost', + test: 'test', + }, + } as EmailActionConnector; + + expect(actionTypeModel.validateConnector(actionConnector)).toEqual({ + errors: { + from: [], + port: [], + host: [], + user: ['Username is required when password is used.'], + password: [], + }, + }); + }); }); describe('action params validation', () => { diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.tsx b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.tsx index 6c994051ec98..f17180ee74e5 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.tsx +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.tsx @@ -97,22 +97,22 @@ export function getActionType(): ActionTypeModel { ) ); } - if (!action.secrets.user) { - errors.user.push( - i18n.translate( - 'xpack.triggersActionsUI.components.builtinActionTypes.error.requiredUserText', - { - defaultMessage: 'Username is required.', - } - ) - ); - } - if (!action.secrets.password) { + if (action.secrets.user && !action.secrets.password) { errors.password.push( i18n.translate( 'xpack.triggersActionsUI.components.builtinActionTypes.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.components.builtinActionTypes.error.requiredUserText', + { + defaultMessage: 'Username is required when password is used.', } ) ); @@ -303,7 +303,7 @@ const EmailActionConnectorFields: React.FunctionComponent 0 && user !== undefined} + isInvalid={errors.user.length > 0} label={i18n.translate( 'xpack.triggersActionsUI.sections.builtinActionTypes.emailAction.userTextFieldLabel', { @@ -313,17 +313,12 @@ const EmailActionConnectorFields: React.FunctionComponent 0 && user !== undefined} + isInvalid={errors.user.length > 0} name="user" value={user || ''} data-test-subj="emailUserInput" onChange={e => { - editActionSecrets('user', e.target.value); - }} - onBlur={() => { - if (!user) { - editActionSecrets('user', ''); - } + editActionSecrets('user', nullableString(e.target.value)); }} /> @@ -333,7 +328,7 @@ const EmailActionConnectorFields: React.FunctionComponent 0 && password !== undefined} + isInvalid={errors.password.length > 0} label={i18n.translate( 'xpack.triggersActionsUI.sections.builtinActionTypes.emailAction.passwordFieldLabel', { @@ -343,17 +338,12 @@ const EmailActionConnectorFields: React.FunctionComponent 0 && password !== undefined} + isInvalid={errors.password.length > 0} name="password" value={password || ''} data-test-subj="emailPasswordInput" onChange={e => { - editActionSecrets('password', e.target.value); - }} - onBlur={() => { - if (!password) { - editActionSecrets('password', ''); - } + editActionSecrets('password', nullableString(e.target.value)); }} /> @@ -624,3 +614,9 @@ const EmailParamsFields: React.FunctionComponent ); }; + +// if the string == null or is empty, return null, else return string +function nullableString(str: string | null | undefined) { + if (str == null || str.trim() === '') return null; + return str; +} diff --git a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts index c0ddd6791e90..2e0576d933f9 100644 --- a/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts +++ b/x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/types.ts @@ -72,8 +72,8 @@ interface EmailConfig { } interface EmailSecrets { - user: string; - password: string; + user: string | null; + password: string | null; } export interface EmailActionConnector extends ActionConnector { diff --git a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts index de856492e12f..e228f6c1f81c 100644 --- a/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts +++ b/x-pack/test/alerting_api_integration/security_and_spaces/tests/actions/builtin_action_types/email.ts @@ -227,5 +227,61 @@ export default function emailTest({ getService }: FtrProviderContext) { .expect(200); expect(typeof createdAction.id).to.be('string'); }); + + it('should handle an email action with no auth', async () => { + const { body: createdAction } = await supertest + .post('/api/action') + .set('kbn-xsrf', 'foo') + .send({ + name: 'An email action with no auth', + actionTypeId: '.email', + config: { + service: '__json', + from: 'jim@example.com', + }, + }) + .expect(200); + + await supertest + .post(`/api/action/${createdAction.id}/_execute`) + .set('kbn-xsrf', 'foo') + .send({ + params: { + to: ['kibana-action-test@elastic.co'], + subject: 'email-subject', + message: 'email-message', + }, + }) + .expect(200) + .then((resp: any) => { + expect(resp.body.data.message.messageId).to.be.a('string'); + expect(resp.body.data.messageId).to.be.a('string'); + + delete resp.body.data.message.messageId; + delete resp.body.data.messageId; + + expect(resp.body.data).to.eql({ + envelope: { + from: 'jim@example.com', + to: ['kibana-action-test@elastic.co'], + }, + message: { + from: { address: 'jim@example.com', name: '' }, + to: [ + { + address: 'kibana-action-test@elastic.co', + name: '', + }, + ], + cc: null, + bcc: null, + subject: 'email-subject', + html: '

email-message

\n', + text: 'email-message', + headers: {}, + }, + }); + }); + }); }); }