From 073cc891c65769dafcb06f73db00727921a0e11a Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 22 Apr 2024 15:58:19 +0200 Subject: [PATCH 1/2] setting: Allow aliases for some email settings The keys for setting the username and password for incoming and outgoing mail are inconsisent: one uses `USERNAME` and `PASSWORD`, the other uses `USER` and `PASSWD`. To make things simpler, allow both to be configured by either, thus, `[mailer].USERNAME` and `[mailer.PASSWORD]` will be aliases for `.USER` and `.PASSWD`, and similarly, `[email.incoming].USER` and `[email.incoming].PASSWD` will be aliases for `.USERNAME` and `.PASSWORD`. Fixes #3355. Signed-off-by: Gergely Nagy --- modules/setting/incoming_email.go | 9 +++++++++ modules/setting/incoming_email_test.go | 24 ++++++++++++++++++++++++ modules/setting/mailer.go | 8 ++++++++ modules/setting/mailer_test.go | 13 +++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 modules/setting/incoming_email_test.go diff --git a/modules/setting/incoming_email.go b/modules/setting/incoming_email.go index 75337a312f..f5ee1368f1 100644 --- a/modules/setting/incoming_email.go +++ b/modules/setting/incoming_email.go @@ -38,6 +38,15 @@ func loadIncomingEmailFrom(rootCfg ConfigProvider) { return } + // Handle aliases + sec := rootCfg.Section("email.incoming") + if sec.HasKey("USER") && !sec.HasKey("USERNAME") { + IncomingEmail.Username = sec.Key("USER").String() + } + if sec.HasKey("PASSWD") && !sec.HasKey("PASSWORD") { + IncomingEmail.Password = sec.Key("PASSWD").String() + } + if err := checkReplyToAddress(IncomingEmail.ReplyToAddress); err != nil { log.Fatal("Invalid incoming_mail.REPLY_TO_ADDRESS (%s): %v", IncomingEmail.ReplyToAddress, err) } diff --git a/modules/setting/incoming_email_test.go b/modules/setting/incoming_email_test.go new file mode 100644 index 0000000000..9e8ee0e6b7 --- /dev/null +++ b/modules/setting/incoming_email_test.go @@ -0,0 +1,24 @@ +// Copyright 2024 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package setting + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_loadIncomingEmailFrom(t *testing.T) { + cfg, _ := NewConfigProviderFromData("") + sec := cfg.Section("email.incoming") + sec.NewKey("ENABLED", "true") + sec.NewKey("USER", "jane.doe@example.com") + sec.NewKey("PASSWD", "y0u'll n3v3r gUess th1S!!1") + sec.NewKey("REPLY_TO_ADDRESS", "forge+%{token}@example.com") + + loadIncomingEmailFrom(cfg) + + assert.EqualValues(t, "jane.doe@example.com", IncomingEmail.Username) + assert.EqualValues(t, "y0u'll n3v3r gUess th1S!!1", IncomingEmail.Password) +} diff --git a/modules/setting/mailer.go b/modules/setting/mailer.go index a2bc2df444..e9ce640c7f 100644 --- a/modules/setting/mailer.go +++ b/modules/setting/mailer.go @@ -134,6 +134,14 @@ func loadMailerFrom(rootCfg ConfigProvider) { sec.Key("PROTOCOL").SetValue("smtp+starttls") } + // Handle aliases + if sec.HasKey("USERNAME") && !sec.HasKey("USER") { + sec.Key("USER").SetValue(sec.Key("USERNAME").String()) + } + if sec.HasKey("PASSWORD") && !sec.HasKey("PASSWD") { + sec.Key("PASSWD").SetValue(sec.Key("PASSWORD").String()) + } + // Set default values & validate sec.Key("NAME").MustString(AppName) sec.Key("PROTOCOL").In("", []string{"smtp", "smtps", "smtp+starttls", "smtp+unix", "sendmail", "dummy"}) diff --git a/modules/setting/mailer_test.go b/modules/setting/mailer_test.go index fbabf11378..f8af4a78c1 100644 --- a/modules/setting/mailer_test.go +++ b/modules/setting/mailer_test.go @@ -38,4 +38,17 @@ func Test_loadMailerFrom(t *testing.T) { assert.EqualValues(t, kase.SMTPPort, MailService.SMTPPort) }) } + + t.Run("property aliases", func(t *testing.T) { + cfg, _ := NewConfigProviderFromData("") + sec := cfg.Section("mailer") + sec.NewKey("ENABLED", "true") + sec.NewKey("USERNAME", "jane.doe@example.com") + sec.NewKey("PASSWORD", "y0u'll n3v3r gUess th1S!!1") + + loadMailerFrom(cfg) + + assert.EqualValues(t, "jane.doe@example.com", MailService.User) + assert.EqualValues(t, "y0u'll n3v3r gUess th1S!!1", MailService.Passwd) + }) } From 0cb46f63dfd09507e148e1787fcc80674a49993e Mon Sep 17 00:00:00 2001 From: Gergely Nagy Date: Mon, 22 Apr 2024 16:27:32 +0200 Subject: [PATCH 2/2] setting: Infer [email.incoming].PORT from .USE_TLS If `[email.incoming].USE_TLS` is set, but the port isn't, infer the default from `.USE_TLS`: set the port to 993 if using tls, and to 143 otherwise. Explicitly setting a port overrides this. Fixes #3357. Signed-off-by: Gergely Nagy --- modules/setting/incoming_email.go | 9 ++++ modules/setting/incoming_email_test.go | 68 ++++++++++++++++++++++---- 2 files changed, 68 insertions(+), 9 deletions(-) diff --git a/modules/setting/incoming_email.go b/modules/setting/incoming_email.go index f5ee1368f1..314fdea137 100644 --- a/modules/setting/incoming_email.go +++ b/modules/setting/incoming_email.go @@ -47,6 +47,15 @@ func loadIncomingEmailFrom(rootCfg ConfigProvider) { IncomingEmail.Password = sec.Key("PASSWD").String() } + // Infer Port if not set + if IncomingEmail.Port == 0 { + if IncomingEmail.UseTLS { + IncomingEmail.Port = 993 + } else { + IncomingEmail.Port = 143 + } + } + if err := checkReplyToAddress(IncomingEmail.ReplyToAddress); err != nil { log.Fatal("Invalid incoming_mail.REPLY_TO_ADDRESS (%s): %v", IncomingEmail.ReplyToAddress, err) } diff --git a/modules/setting/incoming_email_test.go b/modules/setting/incoming_email_test.go index 9e8ee0e6b7..0fdd44d333 100644 --- a/modules/setting/incoming_email_test.go +++ b/modules/setting/incoming_email_test.go @@ -10,15 +10,65 @@ import ( ) func Test_loadIncomingEmailFrom(t *testing.T) { - cfg, _ := NewConfigProviderFromData("") - sec := cfg.Section("email.incoming") - sec.NewKey("ENABLED", "true") - sec.NewKey("USER", "jane.doe@example.com") - sec.NewKey("PASSWD", "y0u'll n3v3r gUess th1S!!1") - sec.NewKey("REPLY_TO_ADDRESS", "forge+%{token}@example.com") + makeBaseConfig := func() (ConfigProvider, ConfigSection) { + cfg, _ := NewConfigProviderFromData("") + sec := cfg.Section("email.incoming") + sec.NewKey("ENABLED", "true") + sec.NewKey("REPLY_TO_ADDRESS", "forge+%{token}@example.com") - loadIncomingEmailFrom(cfg) + return cfg, sec + } + resetIncomingEmailPort := func() func() { + return func() { + IncomingEmail.Port = 0 + } + } - assert.EqualValues(t, "jane.doe@example.com", IncomingEmail.Username) - assert.EqualValues(t, "y0u'll n3v3r gUess th1S!!1", IncomingEmail.Password) + t.Run("aliases", func(t *testing.T) { + cfg, sec := makeBaseConfig() + sec.NewKey("USER", "jane.doe@example.com") + sec.NewKey("PASSWD", "y0u'll n3v3r gUess th1S!!1") + + loadIncomingEmailFrom(cfg) + + assert.EqualValues(t, "jane.doe@example.com", IncomingEmail.Username) + assert.EqualValues(t, "y0u'll n3v3r gUess th1S!!1", IncomingEmail.Password) + }) + + t.Run("Port settings", func(t *testing.T) { + t.Run("no port, no tls", func(t *testing.T) { + defer resetIncomingEmailPort()() + cfg, sec := makeBaseConfig() + + // False is the default, but we test it explicitly. + sec.NewKey("USE_TLS", "false") + + loadIncomingEmailFrom(cfg) + + assert.EqualValues(t, 143, IncomingEmail.Port) + }) + + t.Run("no port, with tls", func(t *testing.T) { + defer resetIncomingEmailPort()() + cfg, sec := makeBaseConfig() + + sec.NewKey("USE_TLS", "true") + + loadIncomingEmailFrom(cfg) + + assert.EqualValues(t, 993, IncomingEmail.Port) + }) + + t.Run("port overrides tls", func(t *testing.T) { + defer resetIncomingEmailPort()() + cfg, sec := makeBaseConfig() + + sec.NewKey("PORT", "1993") + sec.NewKey("USE_TLS", "true") + + loadIncomingEmailFrom(cfg) + + assert.EqualValues(t, 1993, IncomingEmail.Port) + }) + }) }