[Alerting] allow email action to not require auth #60839

pmuellr · 2020-03-21T00:17:47Z

resolves #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.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

resolves elastic#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.

elasticmachine · 2020-03-21T00:17:50Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

pmuellr · 2020-03-23T13:10:22Z

x-pack/plugins/actions/server/builtin_action_types/email.ts

@@ -20,10 +19,10 @@ import { ActionsConfigurationUtilities } from '../actions_config';
 export type ActionTypeConfigType = TypeOf<typeof ConfigSchema>;

 const ConfigSchemaProps = {
-  service: nullableType(schema.string()),


the nullableType function was a pre-cursor to having schema.nullable() available, so I removed it here.

pmuellr · 2020-03-23T13:16:59Z

It's not clear to me that this is a complete solution for using the cloud relay mail server, as I tested this with some "stand alone" smtp server (eg, maildev), and node mailer returned errors "self signed certificate". Not clear what that even means, but I don't fully understand the smtp protocol as it's used in practice today - it sounds like it wants to use TLS, even if we have the "secure" option off.

The changes in the PR are a first step in the right direction though, and could work in some environments.

mikecote

Code LGTM

pmuellr · 2020-03-23T13:43:29Z

@elasticmachine merge upstream

YulNaumenko

I think that we should handle a validation case, when user entered a user name and didn't set a password like we do for Webhook. With current changes I'm able to set password only or user only value and successfully save.

YulNaumenko · 2020-03-23T14:59:24Z

x-pack/plugins/triggers_actions_ui/public/application/components/builtin_action_types/email.tsx

-            }
-          )
-        );
-      }


Maybe it makes sense to do like we have for a webhook:

if (!action.secrets.user && action.secrets.password) {
errors.user.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredHostText',
{
defaultMessage: 'Username is required.',
}
)
);
}
if (!action.secrets.password && action.secrets.user) {
errors.password.push(
i18n.translate(
'xpack.triggersActionsUI.sections.addAction.webhookAction.error.requiredPasswordText',
{
defaultMessage: 'Password is required.',
}
)
);
}

makes sense, I'll do that

YulNaumenko

Changes LGTM!

pmuellr · 2020-03-23T21:06:44Z

@elasticmachine merge upstream

kibanamachine · 2020-03-23T22:54:08Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 3a2b9c8

History

💚 Build #35528 succeeded 97d5c0b
💚 Build #35393 succeeded 977a95f
💛 Build #35282 was flaky 3eda5fe
💛 Build #35131 was flaky 9dcc8c7

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

resolves elastic#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.

resolves #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.

* master: (34 commits) [APM] add service map config options to legacy plugin (elastic#61002) [App Arch] migrate legacy CSS to new platform (core_plugins/kibana_react) (elastic#59882) Migrated styles for "share" plugin to new platform (elastic#59981) [ML] Module setup with dynamic model memory estimation (elastic#60656) Drilldowns (elastic#59632) Upgrade mocha dev-dependency from 6.2.2 to 7.1.1 (elastic#60779) [SIEM] Overview: Recent cases widget (elastic#60993) [ML] Functional tests - stabilize df analytics clone tests (elastic#60497) [SIEM] Updates process and TLS tables to use ECS 1.5 fields (elastic#60854) Migrate doc view part of discover (elastic#58094) Revert "[APM] Collect telemetry about data/API performance (elastic#51612)" fix(NA): log rotation watchers usage (elastic#60956) [SIEM] [CASES] Build lego blocks case details view (elastic#60864) Create Painless Lab app (elastic#57538) [SIEM] Move Timeline Template field to first step of rule creation (elastic#60840) [Reporting/New Platform Migration] Use a new config service on server-side (elastic#55882) [Alerting] allow email action to not require auth (elastic#60839) [Maps] Default ES document layer scaling type to clusters and show scaling UI in the create wizard (elastic#60668) [APM] Collect telemetry about data/API performance (elastic#51612) Implement Kibana Login Selector (elastic#53010) ...

PR elastic#60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

PR #60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

…c#63086) PR elastic#60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

#63412) PR #60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

#63411) PR #60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

PR #60839 added support for unauthenticated emails, but didn't actually do enough to make it work. This PR completes that support, and adds some tests. You can do manual testing now with [maildev](http://maildev.github.io/maildev/).

[Alerting] allow email action to not require auth

9dcc8c7

resolves elastic#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.

pmuellr added Feature:Alerting v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 21, 2020

allow empty/null user and password in UI

3eda5fe

pmuellr commented Mar 23, 2020

View reviewed changes

pmuellr marked this pull request as ready for review March 23, 2020 13:17

pmuellr requested a review from a team as a code owner March 23, 2020 13:17

mikecote approved these changes Mar 23, 2020

View reviewed changes

Merge branch 'master' into alerting/opt-email-auth

977a95f

YulNaumenko reviewed Mar 23, 2020

View reviewed changes

add user/password verification per PR review

97d5c0b

YulNaumenko approved these changes Mar 23, 2020

View reviewed changes

Merge branch 'master' into alerting/opt-email-auth

3a2b9c8

pmuellr merged commit 72bc0ea into elastic:master Mar 23, 2020

pmuellr mentioned this pull request Mar 23, 2020

[7.x] [Alerting] allow email action to not require auth (#60839) #61013

Merged

pmuellr added the backported label Mar 24, 2020

pmuellr mentioned this pull request Apr 9, 2020

[Alerting] set correct parameters for unauthenticated email action #63086

Merged

1 task

mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alerting] allow email action to not require auth #60839

[Alerting] allow email action to not require auth #60839

pmuellr commented Mar 21, 2020

elasticmachine commented Mar 21, 2020

pmuellr Mar 23, 2020

pmuellr commented Mar 23, 2020

mikecote left a comment

pmuellr commented Mar 23, 2020

YulNaumenko left a comment

YulNaumenko Mar 23, 2020

pmuellr Mar 23, 2020

YulNaumenko left a comment

pmuellr commented Mar 23, 2020

kibanamachine commented Mar 23, 2020

[Alerting] allow email action to not require auth #60839

[Alerting] allow email action to not require auth #60839

Conversation

pmuellr commented Mar 21, 2020

For maintainers

elasticmachine commented Mar 21, 2020

pmuellr Mar 23, 2020

Choose a reason for hiding this comment

pmuellr commented Mar 23, 2020

mikecote left a comment

Choose a reason for hiding this comment

pmuellr commented Mar 23, 2020

YulNaumenko left a comment

Choose a reason for hiding this comment

YulNaumenko Mar 23, 2020

Choose a reason for hiding this comment

pmuellr Mar 23, 2020

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

pmuellr commented Mar 23, 2020

kibanamachine commented Mar 23, 2020

💚 Build Succeeded

History