From 408cb15bc7dcfa6677fe3564d9abc4cf30ebd10e Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 26 Sep 2023 16:40:58 +0200 Subject: [PATCH 1/2] notifications: Deprecate redundant encryptions settings 'tls' and 'ssl' are duplicates of 'starttls' and 'ssltls' and have been deprecated in the upstream modules we use for sending mail notifications. Let's deprecate them as well and issue a warning when they are still used. Fixes: #7345 --- .../fix-notifications-redundant-settings.md | 8 ++++++++ services/notifications/pkg/config/config.go | 2 +- .../notifications/pkg/config/parser/parse.go | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 changelog/unreleased/fix-notifications-redundant-settings.md diff --git a/changelog/unreleased/fix-notifications-redundant-settings.md b/changelog/unreleased/fix-notifications-redundant-settings.md new file mode 100644 index 00000000000..17c8ffa4829 --- /dev/null +++ b/changelog/unreleased/fix-notifications-redundant-settings.md @@ -0,0 +1,8 @@ +Bugfix: Deprecate redundant encryptions settings for notification service + +The values `tls` and `ssl` for the `smtp_encryption` configuration setting are +duplicates of `starttls` and `ssltls`. They have been marked as deprecated. +A warning will be logged when they are still used. Please use `starttls` instead +for `tls` and `ssltls` instead of `ssl. + +https://github.com/owncloud/ocis/issues/7345 diff --git a/services/notifications/pkg/config/config.go b/services/notifications/pkg/config/config.go index 170a6c94ccb..c93ca21062f 100644 --- a/services/notifications/pkg/config/config.go +++ b/services/notifications/pkg/config/config.go @@ -44,7 +44,7 @@ type SMTP struct { Password string `yaml:"smtp_password" env:"NOTIFICATIONS_SMTP_PASSWORD" desc:"Password for the SMTP host to connect to."` Insecure bool `yaml:"insecure" env:"NOTIFICATIONS_SMTP_INSECURE" desc:"Allow insecure connections to the SMTP server."` Authentication string `yaml:"smtp_authentication" env:"NOTIFICATIONS_SMTP_AUTHENTICATION" desc:"Authentication method for the SMTP communication. Possible values are 'login', 'plain', 'crammd5', 'none'"` - Encryption string `yaml:"smtp_encryption" env:"NOTIFICATIONS_SMTP_ENCRYPTION" desc:"Encryption method for the SMTP communication. Possible values are 'starttls', 'ssl', 'ssltls', 'tls' and 'none'."` + Encryption string `yaml:"smtp_encryption" env:"NOTIFICATIONS_SMTP_ENCRYPTION" desc:"Encryption method for the SMTP communication. Possible values are 'starttls', 'ssl', 'ssltls', 'tls' and 'none'." deprecationVersion:"5.0.0" removalVersion:"6.0.0" deprecationInfo:"The NOTIFICATIONS_SMTP_ENCRYPTION values 'ssl' and 'tls' are deprecated and will be removed in the future." deprecationReplacement:"Use 'starttls' instead of 'tls' and 'ssltls' instead of 'ssl'."` } // Events combines the configuration options for the event bus. diff --git a/services/notifications/pkg/config/parser/parse.go b/services/notifications/pkg/config/parser/parse.go index af45c272621..b1742028397 100644 --- a/services/notifications/pkg/config/parser/parse.go +++ b/services/notifications/pkg/config/parser/parse.go @@ -2,10 +2,12 @@ package parser import ( "errors" + "fmt" ociscfg "github.com/owncloud/ocis/v2/ocis-pkg/config" "github.com/owncloud/ocis/v2/services/notifications/pkg/config" "github.com/owncloud/ocis/v2/services/notifications/pkg/config/defaults" + "github.com/owncloud/ocis/v2/services/notifications/pkg/logging" "github.com/owncloud/ocis/v2/ocis-pkg/config/envdecode" ) @@ -33,5 +35,22 @@ func ParseConfig(cfg *config.Config) error { } func Validate(cfg *config.Config) error { + logger := logging.Configure(cfg.Service.Name, cfg.Log) + + if cfg.Notifications.SMTP.Host != "" { + switch cfg.Notifications.SMTP.Encryption { + case "tls": + logger.Warn().Msg("The smtp_encryption value 'tls' is deprecated. Please use the value 'starttls' instead.") + case "ssl": + logger.Warn().Msg("The smtp_encryption value 'ssl' is deprecated. Please use the value 'ssltls' instead.") + case "starttls", "ssltls", "none": + break + default: + return fmt.Errorf( + "unknown value '%s' for 'smtp_encryption' in service %s. Allowed values are 'starttls', 'ssltls' or 'none'", + cfg.Notifications.SMTP.Encryption, cfg.Service.Name, + ) + } + } return nil } From 39d4df77007fa5df481a831cd35bbe4b126a1b04 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 26 Sep 2023 17:16:32 +0200 Subject: [PATCH 2/2] notifications: pick SMTP auth method automatically by default This introduces the new value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION. Which will make the notifications service automatically pick an authentication mechanism that the server supports. This is also the new default behavior. This also removes most of the other default settings for the SMTP configuration. The default values were of no real use for this service. Closes: #7356 --- .drone.star | 1 + .../enhancement-notifications-auto-auth-settings.md | 7 +++++++ services/notifications/pkg/channels/channels.go | 2 ++ services/notifications/pkg/config/config.go | 4 ++-- .../notifications/pkg/config/defaults/defaultconfig.go | 6 +----- 5 files changed, 13 insertions(+), 7 deletions(-) create mode 100644 changelog/unreleased/enhancement-notifications-auto-auth-settings.md diff --git a/.drone.star b/.drone.star index b161dd46c14..6889ae89521 100644 --- a/.drone.star +++ b/.drone.star @@ -138,6 +138,7 @@ config = { "NOTIFICATIONS_SMTP_HOST": "email", "NOTIFICATIONS_SMTP_PORT": "2500", "NOTIFICATIONS_SMTP_INSECURE": "true", + "NOTIFICATIONS_SMTP_SENDER": "ownCloud ", }, }, "apiAntivirus": { diff --git a/changelog/unreleased/enhancement-notifications-auto-auth-settings.md b/changelog/unreleased/enhancement-notifications-auto-auth-settings.md new file mode 100644 index 00000000000..7d16ab28cc0 --- /dev/null +++ b/changelog/unreleased/enhancement-notifications-auto-auth-settings.md @@ -0,0 +1,7 @@ +Enhancement: New value `auto` for NOTIFICATIONS_SMTP_AUTHENTICATION + +This cause the notifications service to automatically pick a suitable authentication +method to use with the configured SMTP server. This is also the new default behavior. +The previous default was to not use authentication at all. + +https://github.com/owncloud/ocis/issues/7356 diff --git a/services/notifications/pkg/channels/channels.go b/services/notifications/pkg/channels/channels.go index f3f8c1b76d7..d8d368a63ed 100644 --- a/services/notifications/pkg/channels/channels.go +++ b/services/notifications/pkg/channels/channels.go @@ -69,6 +69,8 @@ func (m Mail) getMailClient() (*mail.SMTPClient, error) { server.Authentication = mail.AuthCRAMMD5 case "none": server.Authentication = mail.AuthNone + case "auto", "": + server.Authentication = mail.AuthAuto default: return nil, errors.New("unknown mail authentication method") } diff --git a/services/notifications/pkg/config/config.go b/services/notifications/pkg/config/config.go index c93ca21062f..2f805854079 100644 --- a/services/notifications/pkg/config/config.go +++ b/services/notifications/pkg/config/config.go @@ -39,11 +39,11 @@ type Notifications struct { type SMTP struct { Host string `yaml:"smtp_host" env:"NOTIFICATIONS_SMTP_HOST" desc:"SMTP host to connect to."` Port int `yaml:"smtp_port" env:"NOTIFICATIONS_SMTP_PORT" desc:"Port of the SMTP host to connect to."` - Sender string `yaml:"smtp_sender" env:"NOTIFICATIONS_SMTP_SENDER" desc:"Sender address of emails that will be sent."` + Sender string `yaml:"smtp_sender" env:"NOTIFICATIONS_SMTP_SENDER" desc:"Sender address of emails that will be sent (e.g. 'ownCloud '."` Username string `yaml:"smtp_username" env:"NOTIFICATIONS_SMTP_USERNAME" desc:"Username for the SMTP host to connect to."` Password string `yaml:"smtp_password" env:"NOTIFICATIONS_SMTP_PASSWORD" desc:"Password for the SMTP host to connect to."` Insecure bool `yaml:"insecure" env:"NOTIFICATIONS_SMTP_INSECURE" desc:"Allow insecure connections to the SMTP server."` - Authentication string `yaml:"smtp_authentication" env:"NOTIFICATIONS_SMTP_AUTHENTICATION" desc:"Authentication method for the SMTP communication. Possible values are 'login', 'plain', 'crammd5', 'none'"` + Authentication string `yaml:"smtp_authentication" env:"NOTIFICATIONS_SMTP_AUTHENTICATION" desc:"Authentication method for the SMTP communication. Possible values are 'login', 'plain', 'crammd5', 'none' or 'auto'. If set to 'auto' or unset, the authentication method is automatically negotiated with the server."` Encryption string `yaml:"smtp_encryption" env:"NOTIFICATIONS_SMTP_ENCRYPTION" desc:"Encryption method for the SMTP communication. Possible values are 'starttls', 'ssl', 'ssltls', 'tls' and 'none'." deprecationVersion:"5.0.0" removalVersion:"6.0.0" deprecationInfo:"The NOTIFICATIONS_SMTP_ENCRYPTION values 'ssl' and 'tls' are deprecated and will be removed in the future." deprecationReplacement:"Use 'starttls' instead of 'tls' and 'ssltls' instead of 'ssl'."` } diff --git a/services/notifications/pkg/config/defaults/defaultconfig.go b/services/notifications/pkg/config/defaults/defaultconfig.go index ab8e2eca7d8..3a5ff261922 100644 --- a/services/notifications/pkg/config/defaults/defaultconfig.go +++ b/services/notifications/pkg/config/defaults/defaultconfig.go @@ -31,11 +31,7 @@ func DefaultConfig() *config.Config { WebUIURL: "https://localhost:9200", Notifications: config.Notifications{ SMTP: config.SMTP{ - Host: "", - Port: 1025, - Sender: "ownCloud ", - Authentication: "none", - Encryption: "none", + Encryption: "none", }, Events: config.Events{ Endpoint: "127.0.0.1:9233",