From b3a502b1154ff125d3988b8b41749591c59e2e6c Mon Sep 17 00:00:00 2001 From: Ivaylo Novakov Date: Fri, 20 May 2022 12:12:31 +0200 Subject: [PATCH 1/2] Fix a panic when the email configuration is invalid. We now return a proper error as we intended to. Thanks @pcfreak30! --- email/sender.go | 11 +++++++++-- email/sender_test.go | 11 ++++++++++- main.go | 4 ++-- main_test.go | 5 +++-- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/email/sender.go b/email/sender.go index cac1c110..55e6917b 100644 --- a/email/sender.go +++ b/email/sender.go @@ -3,6 +3,7 @@ package email import ( "context" "crypto/tls" + "fmt" "net/url" "regexp" "strconv" @@ -23,6 +24,9 @@ const ( ) var ( + // ErrInvalidEmailConfiguration is returned when the email URI given in the + // environment (ACCOUNTS_EMAIL_URI) is either empty or otherwise invalid. + ErrInvalidEmailConfiguration = errors.New("missing or invalid email configuration field ACCOUNTS_EMAIL_URI") // From is the address we send emails from. It defaults to the user // from DefaultConnectionURI but can be overridden by the ACCOUNTS_EMAIL_FROM // environment variable. @@ -202,6 +206,9 @@ func (s Sender) sendMultiple(m ...*mail.Message) error { // values from it. func config(connURI string) (emailConfig, error) { match := matchPattern.FindStringSubmatch(connURI) + if len(match) == 0 { + return emailConfig{}, ErrInvalidEmailConfiguration + } result := make(map[string]string) for i, name := range matchPattern.SubexpNames() { if i != 0 && name != "" { @@ -216,14 +223,14 @@ func config(connURI string) (emailConfig, error) { // These fields are obligatory, so we return an error if any of them are // missing. if !(e1 && e2 && e3 && e4) { - return emailConfig{}, errors.New("missing obligatory email configuration field. One of server, port, user, or password is missing") + return emailConfig{}, ErrInvalidEmailConfiguration } user, err1 := url.QueryUnescape(user) password, err2 := url.QueryUnescape(password) port, err3 := strconv.Atoi(portStr) err := errors.Compose(err1, err2, err3) if err != nil { - return emailConfig{}, errors.AddContext(err, "invalid value for username, password, or port in email connection string") + return emailConfig{}, errors.Compose(err, ErrInvalidEmailConfiguration) } skip := result["skip_ssl_verify"] return emailConfig{ diff --git a/email/sender_test.go b/email/sender_test.go index ea23a222..dd38f6d1 100644 --- a/email/sender_test.go +++ b/email/sender_test.go @@ -2,10 +2,13 @@ package email import ( "testing" + + "gitlab.com/NebulousLabs/errors" ) // TestConfig ensures that config properly parses email connection URIs. func TestConfig(t *testing.T) { + // Valid URI with skip_ssl_verify. s := "smtps://test:test1@mailslurper:1025/?skip_ssl_verify=true" c, err := config(s) if err != nil { @@ -14,7 +17,7 @@ func TestConfig(t *testing.T) { if c.Server != "mailslurper" || c.Port != 1025 || c.User != "test" || c.Pass != "test1" || !c.InsecureSkipVerify { t.Fatal("Unexpected result.") } - + // Valid URI without skip_ssl_verify. s = "smtps://asdf:fdsa@mail.siasky.net:999" c, err = config(s) if err != nil { @@ -23,6 +26,12 @@ func TestConfig(t *testing.T) { if c.Server != "mail.siasky.net" || c.Port != 999 || c.User != "asdf" || c.Pass != "fdsa" || c.InsecureSkipVerify { t.Fatal("Unexpected result.") } + // Invalid URI (missing port). + s = "smtps://asdf:fdsa@mail.siasky.net" + c, err = config(s) + if err == nil || !errors.Contains(err, ErrInvalidEmailConfiguration) { + t.Fatalf("Expected error '%s', got '%s'", ErrInvalidEmailConfiguration, err) + } } // TestServerLockID make sure that ServerLockID is set in testing mode. If it's diff --git a/main.go b/main.go index 9a592f9c..70c4a7ad 100644 --- a/main.go +++ b/main.go @@ -171,12 +171,12 @@ func parseConfiguration(logger *logrus.Logger) (ServiceConfig, error) { config.EmailURI = os.Getenv(envEmailURI) { if config.EmailURI == "" { - return ServiceConfig{}, errors.New(envEmailURI + " is empty") + return ServiceConfig{}, email.ErrInvalidEmailConfiguration } // Validate the given URI. uri, err := url.Parse(config.EmailURI) if err != nil || uri.Host == "" || uri.User == nil { - return ServiceConfig{}, errors.New("invalid email URI given in " + envEmailURI) + return ServiceConfig{}, email.ErrInvalidEmailConfiguration } // Set the FROM address to outgoing emails. This can be overridden by // the ACCOUNTS_EMAIL_FROM optional environment variable. diff --git a/main_test.go b/main_test.go index be666a31..109d8916 100644 --- a/main_test.go +++ b/main_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/SkynetLabs/skynet-accounts/database" + "github.com/SkynetLabs/skynet-accounts/email" "github.com/sirupsen/logrus" "gitlab.com/NebulousLabs/errors" ) @@ -143,7 +144,7 @@ func TestParseConfiguration(t *testing.T) { t.Fatal(err) } _, err = parseConfiguration(logger) - if err == nil || !strings.Contains(err.Error(), envEmailURI+" is empty") { + if err == nil || !errors.Contains(err, email.ErrInvalidEmailConfiguration) { t.Fatal("Failed to error out on empty", envEmailURI) } // Invalid ACCOUNTS_EMAIL_URI @@ -152,7 +153,7 @@ func TestParseConfiguration(t *testing.T) { t.Fatal(err) } _, err = parseConfiguration(logger) - if err == nil || !strings.Contains(err.Error(), "invalid email URI") { + if err == nil || !errors.Contains(err, email.ErrInvalidEmailConfiguration) { t.Log(err) t.Fatal("Failed to error out on invalid", envEmailURI) } From 308c44f8017e02312c6b5cbabaa02d57f3974e2c Mon Sep 17 00:00:00 2001 From: Ivaylo Novakov Date: Fri, 20 May 2022 12:14:48 +0200 Subject: [PATCH 2/2] Cleanup. --- email/sender.go | 1 - 1 file changed, 1 deletion(-) diff --git a/email/sender.go b/email/sender.go index 55e6917b..aeefb524 100644 --- a/email/sender.go +++ b/email/sender.go @@ -3,7 +3,6 @@ package email import ( "context" "crypto/tls" - "fmt" "net/url" "regexp" "strconv"