From f6fd501841816f5e5c1c9a30fbc4c9a2a6f987bc Mon Sep 17 00:00:00 2001 From: zeripath Date: Sun, 27 Nov 2022 10:08:40 +0000 Subject: [PATCH] Correct the fallbacks for mailer configuration (#21945) Unfortunately the fallback configuration code for [mailer] that were added in #18982 are incorrect. When you read a value from an ini section that key is added. This leads to a failure of the fallback mechanism. Further there is also a spelling mistake in the startTLS configuration. This PR restructures the mailer code to first map the deprecated settings on to the new ones - and then use ini.MapTo to map those on to the struct with additional validation as necessary. Ref #21744 Signed-off-by: Andrew Thornton --- custom/conf/app.example.ini | 2 +- .../doc/advanced/config-cheat-sheet.en-us.md | 2 +- modules/setting/mailer.go | 180 +++++++++--------- modules/setting/setting.go | 2 +- services/mailer/mailer.go | 4 +- 5 files changed, 96 insertions(+), 94 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index e7ddda4b8a..5822dc1e4a 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1590,7 +1590,7 @@ ROUTER = console ;; Prefix displayed before subject in mail ;SUBJECT_PREFIX = ;; -;; Mail server protocol. One of "smtp", "smtps", "smtp+startls", "smtp+unix", "sendmail", "dummy". +;; Mail server protocol. One of "smtp", "smtps", "smtp+starttls", "smtp+unix", "sendmail", "dummy". ;; - sendmail: use the operating system's `sendmail` command instead of SMTP. This is common on Linux systems. ;; - dummy: send email messages to the log as a testing phase. ;; If your provider does not explicitly say which protocol it uses but does provide a port, diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index 468c6d5edc..853f0c67f2 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -713,7 +713,7 @@ and [Gitea 1.17 configuration document](https://github.com/go-gitea/gitea/blob/release/v1.17/docs/content/doc/advanced/config-cheat-sheet.en-us.md) - `ENABLED`: **false**: Enable to use a mail service. -- `PROTOCOL`: **\**: Mail server protocol. One of "smtp", "smtps", "smtp+startls", "smtp+unix", "sendmail", "dummy". _Before 1.18, this was inferred from a combination of `MAILER_TYPE` and `IS_TLS_ENABLED`._ +- `PROTOCOL`: **\**: Mail server protocol. One of "smtp", "smtps", "smtp+starttls", "smtp+unix", "sendmail", "dummy". _Before 1.18, this was inferred from a combination of `MAILER_TYPE` and `IS_TLS_ENABLED`._ - SMTP family, if your provider does not explicitly say which protocol it uses but does provide a port, you can set SMTP_PORT instead and this will be inferred. - **sendmail** Use the operating system's `sendmail` command instead of SMTP. This is common on Linux systems. - **dummy** Send email messages to the log as a testing phase. diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index d6f1dae0f7..5a9b7054b9 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -18,32 +18,33 @@ import ( // Mailer represents mail service. type Mailer struct { // Mailer - Name string - From string - EnvelopeFrom string - OverrideEnvelopeFrom bool `ini:"-"` - FromName string - FromEmail string - SendAsPlainText bool - SubjectPrefix string + Name string `ini:"NAME"` + From string `ini:"FROM"` + EnvelopeFrom string `ini:"ENVELOPE_FROM"` + OverrideEnvelopeFrom bool `ini:"-"` + FromName string `ini:"-"` + FromEmail string `ini:"-"` + SendAsPlainText bool `ini:"SEND_AS_PLAIN_TEXT"` + SubjectPrefix string `ini:"SUBJECT_PREFIX"` // SMTP sender - Protocol string - SMTPAddr string - SMTPPort string - User, Passwd string - EnableHelo bool - HeloHostname string - ForceTrustServerCert bool - UseClientCert bool - ClientCertFile string - ClientKeyFile string + Protocol string `ini:"PROTOCOL"` + SMTPAddr string `ini:"SMTP_ADDR"` + SMTPPort string `ini:"SMTP_PORT"` + User string `ini:"USER"` + Passwd string `ini:"PASSWD"` + EnableHelo bool `ini:"ENABLE_HELO"` + HeloHostname string `ini:"HELO_HOSTNAME"` + ForceTrustServerCert bool `ini:"FORCE_TRUST_SERVER_CERT"` + UseClientCert bool `ini:"USE_CLIENT_CERT"` + ClientCertFile string `ini:"CLIENT_CERT_FILE"` + ClientKeyFile string `ini:"CLIENT_KEY_FILE"` // Sendmail sender - SendmailPath string - SendmailArgs []string - SendmailTimeout time.Duration - SendmailConvertCRLF bool + SendmailPath string `ini:"SENDMAIL_PATH"` + SendmailArgs []string `ini:"-"` + SendmailTimeout time.Duration `ini:"SENDMAIL_TIMEOUT"` + SendmailConvertCRLF bool `ini:"SENDMAIL_CONVERT_CRLF"` } // MailService the global mailer @@ -56,35 +57,12 @@ func newMailService() { return } - MailService = &Mailer{ - Name: sec.Key("NAME").MustString(AppName), - SendAsPlainText: sec.Key("SEND_AS_PLAIN_TEXT").MustBool(false), - - Protocol: sec.Key("PROTOCOL").In("", []string{"smtp", "smtps", "smtp+startls", "smtp+unix", "sendmail", "dummy"}), - SMTPAddr: sec.Key("SMTP_ADDR").String(), - SMTPPort: sec.Key("SMTP_PORT").String(), - User: sec.Key("USER").String(), - Passwd: sec.Key("PASSWD").String(), - EnableHelo: sec.Key("ENABLE_HELO").MustBool(true), - HeloHostname: sec.Key("HELO_HOSTNAME").String(), - ForceTrustServerCert: sec.Key("FORCE_TRUST_SERVER_CERT").MustBool(false), - UseClientCert: sec.Key("USE_CLIENT_CERT").MustBool(false), - ClientCertFile: sec.Key("CLIENT_CERT_FILE").String(), - ClientKeyFile: sec.Key("CLIENT_KEY_FILE").String(), - SubjectPrefix: sec.Key("SUBJECT_PREFIX").MustString(""), - - SendmailPath: sec.Key("SENDMAIL_PATH").MustString("sendmail"), - SendmailTimeout: sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute), - SendmailConvertCRLF: sec.Key("SENDMAIL_CONVERT_CRLF").MustBool(true), - } - MailService.From = sec.Key("FROM").MustString(MailService.User) - MailService.EnvelopeFrom = sec.Key("ENVELOPE_FROM").MustString("") - + // Handle Deprecations and map on to new configuration // FIXME: DEPRECATED to be removed in v1.19.0 deprecatedSetting("mailer", "MAILER_TYPE", "mailer", "PROTOCOL") if sec.HasKey("MAILER_TYPE") && !sec.HasKey("PROTOCOL") { if sec.Key("MAILER_TYPE").String() == "sendmail" { - MailService.Protocol = "sendmail" + sec.Key("PROTOCOL").MustString("sendmail") } } @@ -96,31 +74,91 @@ func newMailService() { if err != nil { log.Fatal("Invalid mailer.HOST (%s): %v", givenHost, err) } - MailService.SMTPAddr = addr - MailService.SMTPPort = port + sec.Key("SMTP_ADDR").MustString(addr) + sec.Key("SMTP_PORT").MustString(port) } // FIXME: DEPRECATED to be removed in v1.19.0 deprecatedSetting("mailer", "IS_TLS_ENABLED", "mailer", "PROTOCOL") if sec.HasKey("IS_TLS_ENABLED") && !sec.HasKey("PROTOCOL") { if sec.Key("IS_TLS_ENABLED").MustBool() { - MailService.Protocol = "smtps" + sec.Key("PROTOCOL").MustString("smtps") } else { - MailService.Protocol = "smtp+startls" + sec.Key("PROTOCOL").MustString("smtp+starttls") } } + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO") + if sec.HasKey("DISABLE_HELO") && !sec.HasKey("ENABLE_HELO") { + sec.Key("ENABLE_HELO").MustBool(!sec.Key("DISABLE_HELO").MustBool()) + } + + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT") + if sec.HasKey("SKIP_VERIFY") && !sec.HasKey("FORCE_TRUST_SERVER_CERT") { + sec.Key("FORCE_TRUST_SERVER_CERT").MustBool(sec.Key("SKIP_VERIFY").MustBool()) + } + + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT") + if sec.HasKey("USE_CERTIFICATE") && !sec.HasKey("USE_CLIENT_CERT") { + sec.Key("USE_CLIENT_CERT").MustBool(sec.Key("USE_CERTIFICATE").MustBool()) + } + + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE") + if sec.HasKey("CERT_FILE") && !sec.HasKey("CLIENT_CERT_FILE") { + sec.Key("CERT_FILE").MustString(sec.Key("CERT_FILE").String()) + } + + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE") + if sec.HasKey("KEY_FILE") && !sec.HasKey("CLIENT_KEY_FILE") { + sec.Key("KEY_FILE").MustString(sec.Key("KEY_FILE").String()) + } + + // FIXME: DEPRECATED to be removed in v1.19.0 + deprecatedSetting("mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT") + if sec.HasKey("ENABLE_HTML_ALTERNATIVE") && !sec.HasKey("SEND_AS_PLAIN_TEXT") { + sec.Key("SEND_AS_PLAIN_TEXT").MustBool(!sec.Key("ENABLE_HTML_ALTERNATIVE").MustBool(false)) + } + + if sec.HasKey("PROTOCOL") && sec.Key("PROTOCOL").String() == "smtp+startls" { + log.Error("Deprecated fallback `[mailer]` `PROTOCOL = smtp+startls` present. Use `[mailer]` `PROTOCOL = smtp+starttls`` instead. This fallback will be removed in v1.19.0") + sec.Key("PROTOCOL").SetValue("smtp+starttls") + } + + // Set default values & validate + sec.Key("NAME").MustString(AppName) + sec.Key("PROTOCOL").In("", []string{"smtp", "smtps", "smtp+starttls", "smtp+unix", "sendmail", "dummy"}) + sec.Key("ENABLE_HELO").MustBool(true) + sec.Key("FORCE_TRUST_SERVER_CERT").MustBool(false) + sec.Key("USE_CLIENT_CERT").MustBool(false) + sec.Key("SENDMAIL_PATH").MustString("sendmail") + sec.Key("SENDMAIL_TIMEOUT").MustDuration(5 * time.Minute) + sec.Key("SENDMAIL_CONVERT_CRLF").MustBool(true) + sec.Key("FROM").MustString(sec.Key("USER").String()) + + // Now map the values on to the MailService + MailService = &Mailer{} + if err := sec.MapTo(MailService); err != nil { + log.Fatal("Unable to map [mailer] section on to MailService. Error: %v", err) + } + + // Infer SMTPPort if not set if MailService.SMTPPort == "" { switch MailService.Protocol { case "smtp": MailService.SMTPPort = "25" case "smtps": MailService.SMTPPort = "465" - case "smtp+startls": + case "smtp+starttls": MailService.SMTPPort = "587" } } + // Infer Protocol if MailService.Protocol == "" { if strings.ContainsAny(MailService.SMTPAddr, "/\\") { MailService.Protocol = "smtp+unix" @@ -131,7 +169,7 @@ func newMailService() { case "465": MailService.Protocol = "smtps" case "587": - MailService.Protocol = "smtp+startls" + MailService.Protocol = "smtp+starttls" default: log.Error("unable to infer unspecified mailer.PROTOCOL from mailer.SMTP_PORT = %q, assume using smtps", MailService.SMTPPort) MailService.Protocol = "smtps" @@ -151,42 +189,6 @@ func newMailService() { } } - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "DISABLE_HELO", "mailer", "ENABLE_HELO") - if sec.HasKey("DISABLE_HELO") && !sec.HasKey("ENABLE_HELO") { - MailService.EnableHelo = !sec.Key("DISABLE_HELO").MustBool() - } - - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "SKIP_VERIFY", "mailer", "FORCE_TRUST_SERVER_CERT") - if sec.HasKey("SKIP_VERIFY") && !sec.HasKey("FORCE_TRUST_SERVER_CERT") { - MailService.ForceTrustServerCert = sec.Key("SKIP_VERIFY").MustBool() - } - - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "USE_CERTIFICATE", "mailer", "USE_CLIENT_CERT") - if sec.HasKey("USE_CERTIFICATE") && !sec.HasKey("USE_CLIENT_CERT") { - MailService.UseClientCert = sec.Key("USE_CLIENT_CERT").MustBool() - } - - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "CERT_FILE", "mailer", "CLIENT_CERT_FILE") - if sec.HasKey("CERT_FILE") && !sec.HasKey("CLIENT_CERT_FILE") { - MailService.ClientCertFile = sec.Key("CERT_FILE").String() - } - - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "KEY_FILE", "mailer", "CLIENT_KEY_FILE") - if sec.HasKey("KEY_FILE") && !sec.HasKey("CLIENT_KEY_FILE") { - MailService.ClientKeyFile = sec.Key("KEY_FILE").String() - } - - // FIXME: DEPRECATED to be removed in v1.19.0 - deprecatedSetting("mailer", "ENABLE_HTML_ALTERNATIVE", "mailer", "SEND_AS_PLAIN_TEXT") - if sec.HasKey("ENABLE_HTML_ALTERNATIVE") && !sec.HasKey("SEND_AS_PLAIN_TEXT") { - MailService.SendAsPlainText = !sec.Key("ENABLE_HTML_ALTERNATIVE").MustBool(false) - } - if MailService.From != "" { parsed, err := mail.ParseAddress(MailService.From) if err != nil { diff --git a/modules/setting/setting.go b/modules/setting/setting.go index fb425c8717..df7310b09b 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -605,7 +605,7 @@ func LoadForTest(extraConfigs ...string) { func deprecatedSetting(oldSection, oldKey, newSection, newKey string) { if Cfg.Section(oldSection).HasKey(oldKey) { - log.Error("Deprecated fallback `[%s]` `%s` present. Use `[%s]` `%s` instead. This fallback will be removed in v1.18.0", oldSection, oldKey, newSection, newKey) + log.Error("Deprecated fallback `[%s]` `%s` present. Use `[%s]` `%s` instead. This fallback will be removed in v1.19.0", oldSection, oldKey, newSection, newKey) } } diff --git a/services/mailer/mailer.go b/services/mailer/mailer.go index 46b0c8e2f4..2663b6b2ba 100644 --- a/services/mailer/mailer.go +++ b/services/mailer/mailer.go @@ -166,7 +166,7 @@ func (s *smtpSender) Send(from string, to []string, msg io.WriterTo) error { defer conn.Close() var tlsconfig *tls.Config - if opts.Protocol == "smtps" || opts.Protocol == "smtp+startls" { + if opts.Protocol == "smtps" || opts.Protocol == "smtp+starttls" { tlsconfig = &tls.Config{ InsecureSkipVerify: opts.ForceTrustServerCert, ServerName: opts.SMTPAddr, @@ -208,7 +208,7 @@ func (s *smtpSender) Send(from string, to []string, msg io.WriterTo) error { } } - if opts.Protocol == "smtp+startls" { + if opts.Protocol == "smtp+starttls" { hasStartTLS, _ := client.Extension("STARTTLS") if hasStartTLS { if err = client.StartTLS(tlsconfig); err != nil {