*: depricate --email-from flag and move to email config files

Documentation/email-configuration.md

@@ -20,6 +20,7 @@ set `auth` to `plain` and specify your username and password.
     "host": "smtp.example.org",
     "port": 587,
     "auth": "plain",
+    "from": "postmaster@example.com",
     "username": "postmaster@example.org",
     "password": "foo"
 }
@@ -33,6 +34,7 @@ If using Mailgun the `type` field **must** be set to `mailgun`. Additionally
 ```
 {
     "type": "mailgun",
+    "from": "noreply@example.com",
     "privateAPIKey": "key-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX",
     "publicAPIKey": "YYYYYYYYYYYYYYYYYYYYYYYYYYYYYYYY",
     "domain": "sandboxZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZZ.mailgun.org"
@@ -49,4 +51,4 @@ emailer the `type` field **must** be set to `fake`.
 {
     "type": "fake"
 }
-```
+```

cmd/dex-worker/main.go

@@ -43,7 +43,7 @@ func main() {
 	emailTemplateDirs := flagutil.StringSliceFlag{"./static/email"}
 	fs.Var(&emailTemplateDirs, "email-templates", "comma separated list of directories of email template files")
 
-	emailFrom := fs.String("email-from", "", "emails sent from dex will come from this address")
+	emailFrom := fs.String("email-from", "", `DEPRICATED: use "from" field in email config.`)
 	emailConfig := fs.String("email-cfg", "./static/fixtures/emailer.json", "configures emailer.")
 
 	enableRegistration := fs.Bool("enable-registration", false, "Allows users to self-register. This flag cannot be used in combination with --enable-automatic-registration.")
@@ -132,6 +132,10 @@ func main() {
 		log.Fatalf("Only 'http' and 'https' schemes are supported")
 	}
 
+	if *emailFrom != "" {
+		log.Errorf(`--email-from flag is depricated. Use "from" field in email config.`)
+	}
+
 	scfg := server.ServerConfig{
 		IssuerURL:                *issuer,
 		TemplateDir:              *templates,

email/interface.go

@@ -28,14 +28,20 @@ type Emailer interface {
 	// SendMail queues an email to be sent to 1 or more recipients.
 	// At least one of "text" or "html" must not be blank. If text is blank, but
 	// html is not, then an html-only email should be sent and vice-versal.
-	SendMail(from, subject, text, html string, to ...string) error
+	SendMail(subject, text, html string, to ...string) error
 }
 
 //go:generate genconfig -o config.go email Emailer
 type EmailerConfig interface {
 	EmailerID() string
 	EmailerType() string
-	Emailer() (Emailer, error)
+
+	// Because emailers can either be configured through command line flags or through
+	// JSON configs, we need a way to set the fromAddr when initializing the emailer.
+	//
+	// Values passed in the JSON config should override this value. Supplying neither
+	// should result in an error.
+	Emailer(fromAddress string) (Emailer, error)
 }
 
 func newEmailerConfigFromReader(r io.Reader) (EmailerConfig, error) {
@@ -65,6 +71,7 @@ func NewEmailerConfigFromFile(loc string) (EmailerConfig, error) {
 }
 
 type FakeEmailerConfig struct {
+	FromAddr string `json:"from"`
 }
 
 func (cfg FakeEmailerConfig) EmailerType() string {
@@ -75,15 +82,26 @@ func (cfg FakeEmailerConfig) EmailerID() string {
 	return FakeEmailerType
 }
 
-func (cfg FakeEmailerConfig) Emailer() (Emailer, error) {
+func (cfg FakeEmailerConfig) Emailer(fromAddr string) (Emailer, error) {
-	return FakeEmailer{}, nil
+	from := cfg.FromAddr
+	if from == "" {
+		from = fromAddr
+	}
+	if from == "" {
+		// Since the emailer just prints to stdout, the actual value doesn't matter.
+		from = "noreply@example.com"
+	}
+
+	return FakeEmailer{from}, nil
 }
 
 // FakeEmailer is an Emailer that writes emails to stdout. Should only be used in development.
-type FakeEmailer struct{}
+type FakeEmailer struct {
+	from string
+}
 
-func (f FakeEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (f FakeEmailer) SendMail(subject, text, html string, to ...string) error {
-	fmt.Printf("From: %v\n", from)
+	fmt.Printf("From: %v\n", f.from)
 	fmt.Printf("Subject: %v\n", subject)
 	fmt.Printf("To: %v\n", strings.Join(to, ","))
 	fmt.Printf("Body(text): %v\n", text)

email/mailgun.go

@@ -17,6 +17,7 @@ func init() {
 }
 
 type MailgunEmailerConfig struct {
+	FromAddr      string `json:"from"`
 	PrivateAPIKey string `json:"privateAPIKey"`
 	PublicAPIKey  string `json:"publicAPIKey"`
 	Domain        string `json:"domain"`
@@ -30,10 +31,19 @@ func (cfg MailgunEmailerConfig) EmailerID() string {
 	return MailgunEmailerType
 }
 
-func (cfg MailgunEmailerConfig) Emailer() (Emailer, error) {
+func (cfg MailgunEmailerConfig) Emailer(fromAddr string) (Emailer, error) {
+	from := cfg.FromAddr
+	if from == "" {
+		from = fromAddr
+	}
+
+	if from == "" {
+		return nil, errors.New(`missing "from" field in email config`)
+	}
 	mg := mailgun.NewMailgun(cfg.Domain, cfg.PrivateAPIKey, cfg.PublicAPIKey)
 	return &mailgunEmailer{
-		mg: mg,
+		mg:   mg,
+		from: from,
 	}, nil
 }
 
@@ -64,11 +74,12 @@ func (cfg *MailgunEmailerConfig) UnmarshalJSON(data []byte) error {
 }
 
 type mailgunEmailer struct {
-	mg mailgun.Mailgun
+	mg   mailgun.Mailgun
+	from string
 }
 
-func (m *mailgunEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (m *mailgunEmailer) SendMail(subject, text, html string, to ...string) error {
-	msg := m.mg.NewMessage(from, subject, text, to...)
+	msg := m.mg.NewMessage(m.from, subject, text, to...)
 	if html != "" {
 		msg.SetHtml(html)
 	}

email/mailgun_test.go

@@ -9,20 +9,22 @@ import (
 
 func TestNewEmailConfigFromReader(t *testing.T) {
 	tests := []struct {
-		json    string
+		json        string
-		want    MailgunEmailerConfig
+		want        MailgunEmailerConfig
-		wantErr bool
+		wantErr     bool
+		wantInitErr bool // want error when calling Emailer() with no fromAddr
 	}{
 		{
-			json: `{"type":"mailgun","id":"mg","privateAPIKey":"private","publicAPIKey":"public","domain":"example.com"}`,
+			json: `{"type":"mailgun","id":"mg","privateAPIKey":"private","publicAPIKey":"public","domain":"example.com","from":"admin@example.com"}`,
 			want: MailgunEmailerConfig{
 				PrivateAPIKey: "private",
 				PublicAPIKey:  "public",
 				Domain:        "example.com",
+				FromAddr:      "admin@example.com",
 			},
 		},
 		{
-			json:    `{"type":"mailgun","id":"mg","publicAPIKey":"public","domain":"example.com"}`,
+			json:    `{"type":"mailgun","id":"mg","publicAPIKey":"public","domain":"example.com",""}`,
 			wantErr: true,
 		},
 		{
@@ -33,6 +35,18 @@ func TestNewEmailConfigFromReader(t *testing.T) {
 			json:    `{"type":"mailgun","id":"mg","privateAPIKey":"private","domain":"example.com"}`,
 			wantErr: true,
 		},
+		{
+			json: `{"type":"mailgun","id":"mg","privateAPIKey":"private","publicAPIKey":"public","domain":"example.com"}`,
+			want: MailgunEmailerConfig{
+				PrivateAPIKey: "private",
+				PublicAPIKey:  "public",
+				Domain:        "example.com",
+			},
+
+			// No fromAddr email provided. Calling Emailer("") should error since fromAddr needs to be provided
+			// in the config or as a command line argument.
+			wantInitErr: true,
+		},
 	}
 
 	for i, tt := range tests {
@@ -42,7 +56,6 @@ func TestNewEmailConfigFromReader(t *testing.T) {
 			if err == nil {
 				t.Errorf("case %d: want non-nil err.", i)
 			}
-			t.Logf("WHAT: %v", err)
 			continue
 		}
 		if err != nil {
@@ -52,5 +65,13 @@ func TestNewEmailConfigFromReader(t *testing.T) {
 		if diff := pretty.Compare(tt.want, ec); diff != "" {
 			t.Errorf("case %d: Compare(want, got): %v", i, diff)
 		}
+
+		_, err = ec.Emailer("")
+		if err != nil && !tt.wantInitErr {
+			t.Errorf("case %d: failed to initialize emailer: %v", i, err)
+		}
+		if err == nil && tt.wantInitErr {
+			t.Errorf("case %d: expected error initializing emailer", i)
+		}
 	}
 }

email/smtp.go

@@ -21,6 +21,7 @@ type SmtpEmailerConfig struct {
 	Auth     string `json:"auth"`
 	Username string `json:"username"`
 	Password string `json:"password"`
+	FromAddr string `json:"from"`
 }
 
 func (cfg SmtpEmailerConfig) EmailerType() string {
@@ -31,7 +32,16 @@ func (cfg SmtpEmailerConfig) EmailerID() string {
 	return SmtpEmailerType
 }
 
-func (cfg SmtpEmailerConfig) Emailer() (Emailer, error) {
+func (cfg SmtpEmailerConfig) Emailer(fromAddr string) (Emailer, error) {
+	from := cfg.FromAddr
+	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)
@@ -43,6 +53,7 @@ func (cfg SmtpEmailerConfig) Emailer() (Emailer, error) {
 	}
 	return &smtpEmailer{
 		dialer: dialer,
+		from:   from,
 	}, nil
 }
 
@@ -66,11 +77,12 @@ func (cfg *SmtpEmailerConfig) UnmarshalJSON(data []byte) error {
 
 type smtpEmailer struct {
 	dialer *gomail.Dialer
+	from   string
 }
 
-func (emailer *smtpEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (emailer *smtpEmailer) SendMail(subject, text, html string, to ...string) error {
 	msg := gomail.NewMessage()
-	msg.SetHeader("From", from)
+	msg.SetHeader("From", emailer.from)
 	msg.SetHeader("To", to...)
 	msg.SetHeader("Subject", subject)
 	msg.SetBody("text/plain", text)

email/template.go

@@ -8,7 +8,7 @@ import (
 )
 
 // NewTemplatizedEmailerFromGlobs creates a new TemplatizedEmailer, parsing the templates found in the given filepattern globs.
-func NewTemplatizedEmailerFromGlobs(textGlob, htmlGlob string, emailer Emailer) (*TemplatizedEmailer, error) {
+func NewTemplatizedEmailerFromGlobs(textGlob, htmlGlob string, emailer Emailer, fromAddr string) (*TemplatizedEmailer, error) {
 	textTemplates, err := template.ParseGlob(textGlob)
 	if err != nil {
 		return nil, err
@@ -19,15 +19,16 @@ func NewTemplatizedEmailerFromGlobs(textGlob, htmlGlob string, emailer Emailer)
 		return nil, err
 	}
 
-	return NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer), nil
+	return NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer, fromAddr), nil
 }
 
 // NewTemplatizedEmailerFromTemplates creates a new TemplatizedEmailer, given root text and html templates.
-func NewTemplatizedEmailerFromTemplates(textTemplates *template.Template, htmlTemplates *htmltemplate.Template, emailer Emailer) *TemplatizedEmailer {
+func NewTemplatizedEmailerFromTemplates(textTemplates *template.Template, htmlTemplates *htmltemplate.Template, emailer Emailer, fromAddr string) *TemplatizedEmailer {
 	return &TemplatizedEmailer{
 		emailer:       emailer,
 		textTemplates: textTemplates,
 		htmlTemplates: htmlTemplates,
+		fromAddr:      fromAddr,
 	}
 }
 
@@ -37,6 +38,7 @@ type TemplatizedEmailer struct {
 	htmlTemplates *htmltemplate.Template
 	emailer       Emailer
 	globalCtx     map[string]interface{}
+	fromAddr      string
 }
 
 func (t *TemplatizedEmailer) SetGlobalContext(ctx map[string]interface{}) {
@@ -48,7 +50,7 @@ func (t *TemplatizedEmailer) SetGlobalContext(ctx map[string]interface{}) {
 // the template names you want to base the message on instead of the actual
 // text. "to", "from" and "subject" will be added into the data map regardless
 // of if they are used.
-func (t *TemplatizedEmailer) SendMail(from, subject, tplName string, data map[string]interface{}, to string) error {
+func (t *TemplatizedEmailer) SendMail(subject, tplName string, data map[string]interface{}, to string) error {
 	if tplName == "" {
 		return errors.New("Must provide a template name")
 	}
@@ -61,7 +63,7 @@ func (t *TemplatizedEmailer) SendMail(from, subject, tplName string, data map[st
 	}
 
 	data["to"] = to
-	data["from"] = from
+	data["from"] = t.fromAddr
 	data["subject"] = subject
 
 	for k, v := range t.globalCtx {
@@ -84,5 +86,5 @@ func (t *TemplatizedEmailer) SendMail(from, subject, tplName string, data map[st
 		}
 	}
 
-	return t.emailer.SendMail(from, subject, textBuffer.String(), htmlBuffer.String(), to)
+	return t.emailer.SendMail(subject, textBuffer.String(), htmlBuffer.String(), to)
 }

email/template_test.go

@@ -22,8 +22,7 @@ type testEmailer struct {
 	to                        []string
 }
 
-func (t *testEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (t *testEmailer) SendMail(subject, text, html string, to ...string) error {
-	t.from = from
 	t.subject = subject
 	t.text = text
 	t.html = html
@@ -118,12 +117,12 @@ func TestTemplatizedEmailSendMail(t *testing.T) {
 
 	for i, tt := range tests {
 		emailer := &testEmailer{}
-		templatizer := NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer)
+		templatizer := NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer, tt.from)
 		if tt.ctx != nil {
 			templatizer.SetGlobalContext(tt.ctx)
 		}
 
-		err := templatizer.SendMail(tt.from, tt.subject, tt.tplName, tt.data, tt.to)
+		err := templatizer.SendMail(tt.subject, tt.tplName, tt.data, tt.to)
 		if tt.wantErr {
 			if err == nil {
 				t.Errorf("case %d: err == nil, want non-nil err", i)
@@ -131,9 +130,6 @@ func TestTemplatizedEmailSendMail(t *testing.T) {
 			continue
 		}
 
-		if emailer.from != tt.from {
-			t.Errorf("case %d: want=%q, got=%q", i, tt.from, emailer.from)
-		}
 		if emailer.subject != tt.subject {
 			t.Errorf("case %d: want=%q, got=%q", i, tt.subject, emailer.subject)
 		}

server/config.go

@@ -327,7 +327,7 @@ func setEmailer(srv *Server, issuerName, fromAddress, emailerConfigFile string,
 		return err
 	}
 
-	emailer, err := cfg.Emailer()
+	emailer, err := cfg.Emailer(fromAddress)
 	if err != nil {
 		return err
 	}
@@ -371,7 +371,7 @@ func setEmailer(srv *Server, issuerName, fromAddress, emailerConfigFile string,
 			return err
 		}
 	}
-	tMailer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer)
+	tMailer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer, fromAddress)
 	tMailer.SetGlobalContext(map[string]interface{}{
 		"issuer_name": issuerName,
 	})
@@ -382,7 +382,6 @@ func setEmailer(srv *Server, issuerName, fromAddress, emailerConfigFile string,
 		srv.SessionManager.ValidityWindow,
 		srv.IssuerURL,
 		tMailer,
-		fromAddress,
 		srv.absURL(httpPathResetPassword),
 		srv.absURL(httpPathEmailVerify),
 		srv.absURL(httpPathAcceptInvitation),

server/password_test.go

@@ -101,7 +101,6 @@ func TestSendResetPasswordEmailHandler(t *testing.T) {
 			wantCode: http.StatusOK,
 			wantEmailer: &testEmailer{
 				to:      str("email-1@example.com"),
-				from:    "noreply@example.com",
 				subject: "Reset Your Password",
 			},
 			wantPRUserID:   "ID-1",
@@ -137,7 +136,6 @@ func TestSendResetPasswordEmailHandler(t *testing.T) {
 			wantCode: http.StatusOK,
 			wantEmailer: &testEmailer{
 				to:      str("email-1@example.com"),
-				from:    "noreply@example.com",
 				subject: "Reset Your Password",
 			},
 			wantPRPassword: "password",
@@ -261,7 +259,7 @@ func TestSendResetPasswordEmailHandler(t *testing.T) {
 		emailer := &testEmailer{
 			sent: make(chan struct{}),
 		}
-		templatizer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer)
+		templatizer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer, "admin@example.com")
 		f.srv.UserEmailer.SetEmailer(templatizer)
 		hdlr := SendResetPasswordEmailHandler{
 			tpl:     f.srv.SendResetPasswordEmailTemplate,
@@ -584,13 +582,12 @@ func TestResetPasswordHandler(t *testing.T) {
 }
 
 type testEmailer struct {
-	from, subject, text, html string
+	subject, text, html string
-	to                        []string
+	to                  []string
-	sent                      chan struct{}
+	sent                chan struct{}
 }
 
-func (t *testEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (t *testEmailer) SendMail(subject, text, html string, to ...string) error {
-	t.from = from
 	t.subject = subject
 	t.text = text
 	t.html = html

server/testutil_test.go

@@ -198,7 +198,8 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 	emailer, err := email.NewTemplatizedEmailerFromGlobs(
 		emailTemplatesLocation+"/*.txt",
 		emailTemplatesLocation+"/*.html",
-		&email.FakeEmailer{})
+		&email.FakeEmailer{},
+		"admin@example.com")
 	if err != nil {
 		return nil, err
 	}
@@ -266,7 +267,6 @@ func makeTestFixturesWithOptions(options testFixtureOptions) (*testFixtures, err
 		srv.SessionManager.ValidityWindow,
 		srv.IssuerURL,
 		emailer,
-		"noreply@example.com",
 		srv.absURL(httpPathResetPassword),
 		srv.absURL(httpPathEmailVerify),
 		srv.absURL(httpPathAcceptInvitation),

static/fixtures/emailer.json.sample

@@ -1,3 +1,4 @@
 {
-    "type": "fake"
+    "type": "fake",
-}
+    "from": "noreply@example.com"
+}

user/email/email.go

@@ -19,7 +19,6 @@ type UserEmailer struct {
 	tokenValidityWindow time.Duration
 	issuerURL           url.URL
 	emailer             *email.TemplatizedEmailer
-	fromAddress         string
 
 	passwordResetURL url.URL
 	verifyEmailURL   url.URL
@@ -33,7 +32,6 @@ func NewUserEmailer(ur user.UserRepo,
 	tokenValidityWindow time.Duration,
 	issuerURL url.URL,
 	emailer *email.TemplatizedEmailer,
-	fromAddress string,
 	passwordResetURL url.URL,
 	verifyEmailURL url.URL,
 	invitationURL url.URL,
@@ -45,7 +43,6 @@ func NewUserEmailer(ur user.UserRepo,
 		tokenValidityWindow: tokenValidityWindow,
 		issuerURL:           issuerURL,
 		emailer:             emailer,
-		fromAddress:         fromAddress,
 		passwordResetURL:    passwordResetURL,
 		verifyEmailURL:      verifyEmailURL,
 		invitationURL:       invitationURL,
@@ -106,7 +103,7 @@ func (u *UserEmailer) SendResetPasswordEmail(email string, redirectURL url.URL,
 	resetURL.RawQuery = q.Encode()
 
 	if u.emailer != nil {
-		err = u.emailer.SendMail(u.fromAddress, "Reset Your Password", "password-reset",
+		err = u.emailer.SendMail("Reset Your Password", "password-reset",
 			map[string]interface{}{
 				"email": usr.Email,
 				"link":  resetURL.String(),
@@ -144,7 +141,7 @@ func (u *UserEmailer) SendInviteEmail(email string, redirectURL url.URL, clientI
 	resetURL.RawQuery = q.Encode()
 
 	if u.emailer != nil {
-		err = u.emailer.SendMail(u.fromAddress, "Activate Your Account", "invite",
+		err = u.emailer.SendMail("Activate Your Account", "invite",
 			map[string]interface{}{
 				"email": usr.Email,
 				"link":  resetURL.String(),
@@ -191,7 +188,7 @@ func (u *UserEmailer) SendEmailVerification(userID, clientID string, redirectURL
 	verifyURL.RawQuery = q.Encode()
 
 	if u.emailer != nil {
-		err = u.emailer.SendMail(u.fromAddress, "Please verify your email address.", "verify-email",
+		err = u.emailer.SendMail("Please verify your email address.", "verify-email",
 			map[string]interface{}{
 				"email": usr.Email,
 				"link":  verifyURL.String(),

user/email/email_test.go

@@ -29,13 +29,12 @@ var (
 )
 
 type testEmailer struct {
-	from, subject, text, html string
+	subject, text, html string
-	to                        []string
+	to                  []string
-	sent                      bool
+	sent                bool
 }
 
-func (t *testEmailer) SendMail(from, subject, text, html string, to ...string) error {
+func (t *testEmailer) SendMail(subject, text, html string, to ...string) error {
-	t.from = from
 	t.subject = subject
 	t.text = text
 	t.html = html
@@ -112,9 +111,9 @@ func makeTestFixtures() (*UserEmailer, *testEmailer, *key.PublicKey) {
 	htmlTemplates := htmltemplate.New("html")
 
 	emailer := &testEmailer{}
-	tEmailer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer)
+	tEmailer := email.NewTemplatizedEmailerFromTemplates(textTemplates, htmlTemplates, emailer, fromAddress)
 
-	userEmailer := NewUserEmailer(ur, pwr, signerFn, validityWindow, issuerURL, tEmailer, fromAddress, passwordResetURL, verifyEmailURL, acceptInvitationURL)
+	userEmailer := NewUserEmailer(ur, pwr, signerFn, validityWindow, issuerURL, tEmailer, passwordResetURL, verifyEmailURL, acceptInvitationURL)
 
 	return userEmailer, emailer, publicKey
 }
@@ -203,10 +202,6 @@ func TestSendResetPasswordEmail(t *testing.T) {
 				t.Errorf("case %d: want==%v, got==%v", i, tt.email, emailer.to[0])
 			}
 
-			if fromAddress != emailer.from {
-				t.Errorf("case %d: want==%v, got==%v", i, fromAddress, emailer.from)
-			}
-
 		} else if emailer.sent {
 			t.Errorf("case %d: want !emailer.sent", i)
 		}
@@ -306,10 +301,6 @@ func TestSendEmailVerificationEmail(t *testing.T) {
 				t.Errorf("case %d: want==%v, got==%v", i, tt.wantEmailAddress, emailer.to[0])
 			}
 
-			if fromAddress != emailer.from {
-				t.Errorf("case %d: want==%v, got==%v", i, fromAddress, emailer.from)
-			}
-
 		} else if emailer.sent {
 			t.Errorf("case %d: want !emailer.sent", i)
 		}