smtp: make "auth" and "port" config fields optional

Use "plain" auth if and only if username and password are provided.

Parse port from "host" field if it's provided.

Documentation/email-configuration.md

@@ -12,14 +12,12 @@ specific fields.
 
 If using SMTP the `type` field **must** be set to `smtp`. Additionally both
 `host` and `port` are required. If you wish to use SMTP plain auth, then
-set `auth` to `plain` and specify your username and password.
+specify your username and password.
 
 ```
 {
     "type": "smtp",
-    "host": "smtp.example.org",
-    "port": 587,
-    "auth": "plain",
+    "host": "smtp.example.org:587",
     "from": "postmaster@example.com",
     "username": "postmaster@example.org",
     "password": "foo"

email/smtp.go

@@ -3,6 +3,9 @@ package email
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
+	"net"
+	"strconv"
 
 	"gopkg.in/gomail.v2"
 )
@@ -17,11 +20,16 @@ func init() {
 
 type SmtpEmailerConfig struct {
 	Host     string `json:"host"`
-	Port     int    `json:"port"`
-	Auth     string `json:"auth"`
 	Username string `json:"username"`
 	Password string `json:"password"`
 	FromAddr string `json:"from"`
+
+	// OPTIONAL: If empty and host is of form "host:port" just use that. For backward
+	// compatibility do not change this.
+	Port int `json:"port"`
+
+	// DEPRICATED: If "username" and "password" are provided, use them.
+	Auth string `json:"auth"`
 }
 
 func (cfg SmtpEmailerConfig) EmailerType() string {
@@ -37,24 +45,38 @@ func (cfg SmtpEmailerConfig) Emailer(fromAddr string) (Emailer, error) {
 	if from == "" {
 		from = fromAddr
 	}
-
 	if from == "" {
 		return nil, errors.New(`missing "from" field in email config`)
 	}
 
-	var dialer *gomail.Dialer
-	if cfg.Auth == "plain" {
-		dialer = gomail.NewPlainDialer(cfg.Host, cfg.Port, cfg.Username, cfg.Password)
-	} else {
-		dialer = &gomail.Dialer{
-			Host: cfg.Host,
-			Port: cfg.Port,
+	host, port := cfg.Host, cfg.Port
+
+	// If port hasn't been supplied, check the "host" field.
+	if port == 0 {
+		hostStr, portStr, err := net.SplitHostPort(cfg.Host)
+		if err != nil {
+			return nil, fmt.Errorf(`"host" must be in format of "host:port" %v`, err)
+		}
+		host = hostStr
+		if port, err = strconv.Atoi(portStr); err != nil {
+			return nil, fmt.Errorf(`failed to parse %q as "host:port" %v`, cfg.Host, err)
 		}
 	}
-	return &smtpEmailer{
-		dialer: dialer,
-		from:   from,
-	}, nil
+
+	if (cfg.Username == "") != (cfg.Password == "") {
+		return nil, errors.New(`must provide both "username" and "password"`)
+	}
+
+	var dialer *gomail.Dialer
+	if cfg.Username == "" {
+		// NOTE(ericchiang): Guess SSL using the same logic as gomail. We should
+		// eventually allow this to be set explicitly.
+		dialer = &gomail.Dialer{Host: host, Port: port, SSL: port == 465}
+	} else {
+		dialer = gomail.NewPlainDialer(host, port, cfg.Username, cfg.Password)
+	}
+
+	return &smtpEmailer{dialer: dialer, from: from}, nil
 }
 
 type smtpEmailerConfig SmtpEmailerConfig

email/smtp_test.go

@@ -0,0 +1,144 @@
+package email
+
+import (
+	"encoding/json"
+	"strconv"
+	"testing"
+
+	"github.com/kylelemons/godebug/pretty"
+
+	"gopkg.in/gomail.v2"
+)
+
+func TestNewSmtpEmailer(t *testing.T) {
+	// If (and only if) this port is provided, gomail assumes SSL.
+	gomailSSLPort := 465
+
+	tests := []struct {
+		config SmtpEmailerConfig
+
+		// formAddr set by the dex-worker flag
+		fromAddrFlag string
+
+		wantEmailer Emailer
+		wantErr     bool
+	}{
+		{
+			config: SmtpEmailerConfig{
+				Host:     "example.com:" + strconv.Itoa(gomailSSLPort),
+				FromAddr: "foo@example.com",
+			},
+			wantEmailer: &smtpEmailer{
+				from: "foo@example.com",
+				dialer: &gomail.Dialer{
+					Host: "example.com",
+					Port: gomailSSLPort,
+					SSL:  true,
+				},
+			},
+		},
+		{
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				Port:     gomailSSLPort,
+				FromAddr: "foo@example.com",
+			},
+			wantEmailer: &smtpEmailer{
+				from: "foo@example.com",
+				dialer: &gomail.Dialer{
+					Host: "example.com",
+					Port: gomailSSLPort,
+					SSL:  true,
+				},
+			},
+		},
+		{
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				Port:     80,
+				FromAddr: "foo@example.com",
+			},
+			wantEmailer: &smtpEmailer{
+				from: "foo@example.com",
+				dialer: &gomail.Dialer{
+					Host: "example.com",
+					Port: 80,
+				},
+			},
+		},
+		{
+			// No port provided.
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				FromAddr: "foo@example.com",
+			},
+			wantErr: true,
+		},
+		{
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				Port:     80,
+				FromAddr: "foo@example.com",
+			},
+			fromAddrFlag: "bar@example.com",
+			wantEmailer: &smtpEmailer{
+				from: "foo@example.com", // config should override flag.
+				dialer: &gomail.Dialer{
+					Host: "example.com",
+					Port: 80,
+				},
+			},
+		},
+		{
+			// No fromAddr provided as a flag or in config.
+			config:  SmtpEmailerConfig{Host: "example.com"},
+			wantErr: true,
+		},
+		{
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				Port:     80,
+				Username: "foo",
+				Password: "bar",
+				FromAddr: "foo@example.com",
+			},
+			wantEmailer: &smtpEmailer{
+				from:   "foo@example.com", // config should override flag.
+				dialer: gomail.NewPlainDialer("example.com", 80, "foo", "bar"),
+			},
+		},
+		{
+			// Password provided without username.
+			config: SmtpEmailerConfig{
+				Host:     "example.com",
+				Port:     80,
+				Password: "bar",
+				FromAddr: "foo@example.com",
+			},
+			wantErr: true,
+		},
+	}
+
+	for i, tt := range tests {
+		testCase, err := json.MarshalIndent(tt.config, "", "  ")
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		emailer, err := tt.config.Emailer(tt.fromAddrFlag)
+		if err != nil {
+			if !tt.wantErr {
+				t.Errorf("case %d %s.Emailer(): %v", i, testCase, err)
+			}
+			continue
+		}
+		if tt.wantErr {
+			t.Errorf("case %d %s.Emailer(): expected error creating emailer", i, testCase)
+			continue
+		}
+
+		if diff := pretty.Compare(emailer, tt.wantEmailer); diff != "" {
+			t.Errorf("case %d: unexpected emailer %s", i, diff)
+		}
+	}
+}