Correct the fallbacks for mailer configuration (#21945) (#21953)

Backport #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 <art27@cantab.net>

diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini
index ec0b7c5235d5512519c12d8de2755513ccd344db..3f5ce8046f2994bba0ca275ef15b059b18d974c1 100644 (file)
--- a/custom/conf/app.example.ini
+++ b/custom/conf/app.example.ini
@@ -1550,7 +1550,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 3fd853fb148fee6c151a4651881044e958cb2765..955b9ac22875f28c63f5ba497496254219d05889 100644 (file)
--- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md
+++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md
@@ -672,7 +672,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`: **\<empty\>**: 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`: **\<empty\>**: 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 d6f1dae0f7156ae0777293a4a8a14e61ce04c106..5a9b7054b9da54eece7628d4fa9b99e07e41aa3c 100644 (file)
--- 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 bd035403c8c03f7f9673f119292472206f38ad22..bb2281f13b244fee67a728a96bb738e495bf7cc6 100644 (file)
--- a/modules/setting/setting.go
+++ b/modules/setting/setting.go
@@ -603,7 +603,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 46b0c8e2f45927544ebcf6513908c64c44de72bf..2663b6b2bab1137c9a328690d97d1302ecfcf077 100644 (file)
--- 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 {