Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a panic when the email configuration is invalid. #209

Merged
merged 2 commits into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions email/sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,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.
Expand Down Expand Up @@ -202,6 +205,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 {
ro-tex marked this conversation as resolved.
Show resolved Hide resolved
return emailConfig{}, ErrInvalidEmailConfiguration
}
result := make(map[string]string)
for i, name := range matchPattern.SubexpNames() {
if i != 0 && name != "" {
Expand All @@ -216,14 +222,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{
Expand Down
11 changes: 10 additions & 1 deletion email/sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down