smtp: STARTTLS before querying auth mechanisms

Some servers (notably smtp.gmail.com) require you to use STARTTLS before
they will even list PLAIN authentication. This commit makes using gmail
for smtp notifications work.

Aside from STARTTLS being required for authentication, it’s of course
also a good idea to just use STARTTLS whenever possible, regardless of
the authentication requirements :).

manager/notifier.go

@@ -516,9 +516,9 @@ func writeEmailBodyWithTime(w io.Writer, from, to, status string, a *Alert, mome
 	return nil
 }
 
-func getSMTPAuth(hasAuth bool, mechs string) (smtp.Auth, *tls.Config, error) {
+func getSMTPAuth(hasAuth bool, mechs string) (smtp.Auth, error) {
 	if !hasAuth {
-		return nil, nil, nil
+		return nil, nil
 	}
 
 	username := os.Getenv("SMTP_AUTH_USERNAME")
@@ -530,7 +530,7 @@ func getSMTPAuth(hasAuth bool, mechs string) (smtp.Auth, *tls.Config, error) {
 			if secret == "" {
 				continue
 			}
-			return smtp.CRAMMD5Auth(username, secret), nil, nil
+			return smtp.CRAMMD5Auth(username, secret), nil
 		case "PLAIN":
 			password := os.Getenv("SMTP_AUTH_PASSWORD")
 			if password == "" {
@@ -541,15 +541,14 @@ func getSMTPAuth(hasAuth bool, mechs string) (smtp.Auth, *tls.Config, error) {
 			// We need to know the hostname for both auth and TLS.
 			host, _, err := net.SplitHostPort(*smtpSmartHost)
 			if err != nil {
-				return nil, nil, fmt.Errorf("invalid address: %s", err)
+				return nil, fmt.Errorf("invalid address: %s", err)
 			}
 
 			auth := smtp.PlainAuth(identity, username, password, host)
-			cfg := &tls.Config{ServerName: host}
-			return auth, cfg, nil
+			return auth, nil
 		}
 	}
-	return nil, nil, nil
+	return nil, nil
 }
 
 func (n *notifier) sendEmailNotification(to string, op notificationOp, a *Alert) error {
@@ -567,16 +566,21 @@ func (n *notifier) sendEmailNotification(to string, op notificationOp, a *Alert)
 	}
 	defer c.Quit()
 
-	// Authenticate if we and the server are both configured for it.
-	auth, tlsConfig, err := getSMTPAuth(c.Extension("AUTH"))
+	// We need to know the hostname for both auth and TLS.
+	host, _, err := net.SplitHostPort(*smtpSmartHost)
 	if err != nil {
-		return err
+		return fmt.Errorf("invalid address: %s", err)
 	}
 
-	if tlsConfig != nil {
-		if err := c.StartTLS(tlsConfig); err != nil {
-			return fmt.Errorf("starttls failed: %s", err)
-		}
+	tlsConfig := &tls.Config{ServerName: host}
+	if err := c.StartTLS(tlsConfig); err != nil {
+		return fmt.Errorf("starttls failed: %s", err)
+	}
+
+	// Authenticate if we and the server are both configured for it.
+	auth, err := getSMTPAuth(c.Extension("AUTH"))
+	if err != nil {
+		return err
 	}
 
 	if auth != nil {

manager/notifier_test.go

@@ -15,7 +15,6 @@ package manager
 
 import (
 	"bytes"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"html"
@@ -85,7 +84,7 @@ type authTestCase struct {
 }
 
 func (tc *authTestCase) test(t *testing.T) {
-	auth, cfg, err := getSMTPAuth(tc.hasAuth, tc.mechs)
+	auth, err := getSMTPAuth(tc.hasAuth, tc.mechs)
 	if err != nil {
 		tc.fail(t, "unexpected error: %s", err)
 		return
@@ -94,21 +93,10 @@ func (tc *authTestCase) test(t *testing.T) {
 		if auth != nil {
 			tc.fail(t, "expected auth to be nil, got %T", auth)
 		}
-		if cfg != nil {
-			tc.fail(t, "expected tls config to be nil, got %v", cfg)
-		}
 	} else {
 		if fmt.Sprintf("%T", auth) != tc.expAuthType {
 			tc.fail(t, "expected auth to be %s, got %T", tc.expAuthType, auth)
 		}
-		if tc.expAuthType == "*smtp.plainAuth" {
-			if cfg == nil {
-				tc.fail(t, "expected tls config")
-			} else if cfg.ServerName != "testSMTPHost" {
-				tc.fail(t, "expected tls config to be %v, got %v",
-					&tls.Config{ServerName: "testSMTPHost"}, cfg)
-			}
-		}
 	}
 }
 
@@ -179,8 +167,8 @@ func TestGetSMTPAuth(t *testing.T) {
 		{true, "PLAIN", ""},
 	})
 	os.Setenv("SMTP_AUTH_PASSWORD", "p")
-	if auth, cfg, err := getSMTPAuth(true, "PLAIN"); err == nil {
-		t.Errorf("PLAIN auth with bad host-port: expected error but got %T, %v", auth, cfg)
+	if auth, err := getSMTPAuth(true, "PLAIN"); err == nil {
+		t.Errorf("PLAIN auth with bad host-port: expected error but got %T", auth)
 	}
 }