diff --git a/app/email.go b/app/email.go index cf54a2af2d..d0166072d8 100644 --- a/app/email.go +++ b/app/email.go @@ -7,7 +7,6 @@ import ( "bytes" "fmt" "io" - "net/mail" "net/url" "path" "strings" @@ -525,8 +524,6 @@ func (a *App) SendMail(to, subject, htmlBody string) *model.AppError { func (a *App) SendMailWithEmbeddedFiles(to, subject, htmlBody string, embeddedFiles map[string]io.Reader) *model.AppError { license := a.License() config := a.Config() - fromMail := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.FeedbackEmail} - replyTo := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.ReplyToAddress} - return mailservice.SendMailUsingConfigAdvanced(to, to, fromMail, replyTo, subject, htmlBody, nil, embeddedFiles, nil, config, license != nil && *license.Features.Compliance) + return mailservice.SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, embeddedFiles, config, license != nil && *license.Features.Compliance) } diff --git a/services/mailservice/mail.go b/services/mailservice/mail.go index c02594f89f..bd18576d72 100644 --- a/services/mailservice/mail.go +++ b/services/mailservice/mail.go @@ -24,6 +24,18 @@ import ( "github.com/mattermost/mattermost-server/utils" ) +type mailData struct { + mimeTo string + smtpTo string + from mail.Address + replyTo mail.Address + subject string + htmlBody string + attachments []*model.FileInfo + embeddedFiles map[string]io.Reader + mimeHeaders map[string]string +} + // smtpClient is implemented by an smtp.Client. See https://golang.org/pkg/net/smtp/#Client. // type smtpClient interface { @@ -204,15 +216,29 @@ func TestConnection(config *model.Config) { defer c.Close() } -func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool) *model.AppError { +func SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody string, embeddedFiles map[string]io.Reader, config *model.Config, enableComplianceFeatures bool) *model.AppError { fromMail := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.FeedbackEmail} replyTo := mail.Address{Name: *config.EmailSettings.FeedbackName, Address: *config.EmailSettings.ReplyToAddress} - return SendMailUsingConfigAdvanced(to, to, fromMail, replyTo, subject, htmlBody, nil, nil, nil, config, enableComplianceFeatures) + mail := mailData{ + mimeTo: to, + smtpTo: to, + from: fromMail, + replyTo: replyTo, + subject: subject, + htmlBody: htmlBody, + embeddedFiles: embeddedFiles, + } + + return sendMailUsingConfigAdvanced(mail, config, enableComplianceFeatures) +} + +func SendMailUsingConfig(to, subject, htmlBody string, config *model.Config, enableComplianceFeatures bool) *model.AppError { + return SendMailWithEmbeddedFilesUsingConfig(to, subject, htmlBody, nil, config, enableComplianceFeatures) } // allows for sending an email with attachments and differing MIME/SMTP recipients -func SendMailUsingConfigAdvanced(mimeTo, smtpTo string, from, replyTo mail.Address, subject, htmlBody string, attachments []*model.FileInfo, embeddedFiles map[string]io.Reader, mimeHeaders map[string]string, config *model.Config, enableComplianceFeatures bool) *model.AppError { +func sendMailUsingConfigAdvanced(mail mailData, config *model.Config, enableComplianceFeatures bool) *model.AppError { if len(*config.EmailSettings.SMTPServer) == 0 { return nil } @@ -235,34 +261,34 @@ func SendMailUsingConfigAdvanced(mimeTo, smtpTo string, from, replyTo mail.Addre return err } - return SendMail(c, mimeTo, smtpTo, from, replyTo, subject, htmlBody, attachments, embeddedFiles, mimeHeaders, fileBackend, time.Now()) + return SendMail(c, mail, fileBackend, time.Now()) } -func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, subject, htmlBody string, attachments []*model.FileInfo, embeddedFiles map[string]io.Reader, mimeHeaders map[string]string, fileBackend filesstore.FileBackend, date time.Time) *model.AppError { - mlog.Debug("sending mail", mlog.String("to", smtpTo), mlog.String("subject", subject)) +func SendMail(c smtpClient, mail mailData, fileBackend filesstore.FileBackend, date time.Time) *model.AppError { + mlog.Debug("sending mail", mlog.String("to", mail.smtpTo), mlog.String("subject", mail.subject)) - htmlMessage := "\r\n" + htmlBody + "" + htmlMessage := "\r\n" + mail.htmlBody + "" - txtBody, err := html2text.FromString(htmlBody) + txtBody, err := html2text.FromString(mail.htmlBody) if err != nil { mlog.Warn("Unable to convert html body to text", mlog.Err(err)) txtBody = "" } headers := map[string][]string{ - "From": {from.String()}, - "To": {mimeTo}, - "Subject": {encodeRFC2047Word(subject)}, + "From": {mail.from.String()}, + "To": {mail.mimeTo}, + "Subject": {encodeRFC2047Word(mail.subject)}, "Content-Transfer-Encoding": {"8bit"}, "Auto-Submitted": {"auto-generated"}, "Precedence": {"bulk"}, } - if len(replyTo.Address) > 0 { - headers["Reply-To"] = []string{replyTo.String()} + if len(mail.replyTo.Address) > 0 { + headers["Reply-To"] = []string{mail.replyTo.String()} } - for k, v := range mimeHeaders { + for k, v := range mail.mimeHeaders { headers[k] = []string{encodeRFC2047Word(v)} } @@ -272,11 +298,11 @@ func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, s m.SetBody("text/plain", txtBody) m.AddAlternative("text/html", htmlMessage) - for name, reader := range embeddedFiles { + for name, reader := range mail.embeddedFiles { m.EmbedReader(name, reader) } - for _, fileInfo := range attachments { + for _, fileInfo := range mail.attachments { bytes, err := fileBackend.ReadFile(fileInfo.Path) if err != nil { return err @@ -290,11 +316,11 @@ func SendMail(c smtpClient, mimeTo, smtpTo string, from, replyTo mail.Address, s })) } - if err = c.Mail(from.Address); err != nil { + if err = c.Mail(mail.from.Address); err != nil { return model.NewAppError("SendMail", "utils.mail.send_mail.from_address.app_error", nil, err.Error(), http.StatusInternalServerError) } - if err = c.Rcpt(smtpTo); err != nil { + if err = c.Rcpt(mail.smtpTo); err != nil { return model.NewAppError("SendMail", "utils.mail.send_mail.to_address.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/services/mailservice/mail_test.go b/services/mailservice/mail_test.go index 2e2b87302f..9686f8d92e 100644 --- a/services/mailservice/mail_test.go +++ b/services/mailservice/mail_test.go @@ -128,6 +128,50 @@ func TestSendMailUsingConfig(t *testing.T) { } } +func TestSendMailWithEmbeddedFilesUsingConfig(t *testing.T) { + utils.T = utils.GetUserTranslations("en") + + fs, err := config.NewFileStore("config.json", false) + require.Nil(t, err) + + cfg := fs.Get() + + var emailTo = "test@example.com" + var emailSubject = "Testing this email" + var emailBody = "This is a test from autobot" + + //Delete all the messages before check the sample email + DeleteMailBox(emailTo) + + embeddedFiles := map[string]io.Reader{ + "test1.png": bytes.NewReader([]byte("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")), + "test2.png": bytes.NewReader([]byte("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx")), + } + err2 := SendMailWithEmbeddedFilesUsingConfig(emailTo, emailSubject, emailBody, embeddedFiles, cfg, true) + require.Nil(t, err2, "Should connect to the SMTP Server") + + //Check if the email was send to the right email address + var resultsMailbox JSONMessageHeaderInbucket + err3 := RetryInbucket(5, func() error { + var err error + resultsMailbox, err = GetMailBox(emailTo) + return err + }) + if err3 != nil { + t.Log(err3) + t.Log("No email was received, maybe due load on the server. Skipping this verification") + } else { + if len(resultsMailbox) > 0 { + require.Contains(t, resultsMailbox[0].To[0], emailTo, "Wrong To: recipient") + resultsEmail, err := GetMessageFromMailbox(emailTo, resultsMailbox[0].ID) + require.Nil(t, err, "Could not get message from mailbox") + require.Contains(t, emailBody, resultsEmail.Body.Text, "Wrong received message %s", resultsEmail.Body.Text) + // Usign the message size because the inbucket API doesn't return embedded attachments through the API + require.Greater(t, resultsEmail.Size, 1500, "the file size should be more because the embedded attachemtns") + } + } +} + func TestSendMailUsingConfigAdvanced(t *testing.T) { utils.T = utils.GetUserTranslations("en") @@ -136,15 +180,8 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { cfg := fs.Get() - var mimeTo = "test@example.com" - var smtpTo = "test2@example.com" - var from = mail.Address{Name: "Nobody", Address: "nobody@mattermost.com"} - var replyTo = mail.Address{Name: "ReplyTo", Address: "reply_to@mattermost.com"} - var emailSubject = "Testing this email" - var emailBody = "This is a test from autobot" - //Delete all the messages before check the sample email - DeleteMailBox(smtpTo) + DeleteMailBox("test2@example.com") fileBackend, err := filesstore.NewFileBackend(&cfg.FileSettings, true) assert.Nil(t, err) @@ -178,31 +215,43 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { headers := make(map[string]string) headers["TestHeader"] = "TestValue" - err = SendMailUsingConfigAdvanced(mimeTo, smtpTo, from, replyTo, emailSubject, emailBody, attachments, embeddedFiles, headers, cfg, true) + mail := mailData{ + mimeTo: "test@example.com", + smtpTo: "test2@example.com", + from: mail.Address{Name: "Nobody", Address: "nobody@mattermost.com"}, + replyTo: mail.Address{Name: "ReplyTo", Address: "reply_to@mattermost.com"}, + subject: "Testing this email", + htmlBody: "This is a test from autobot", + attachments: attachments, + embeddedFiles: embeddedFiles, + mimeHeaders: headers, + } + + err = sendMailUsingConfigAdvanced(mail, cfg, true) require.Nil(t, err, "Should connect to the STMP Server: %v", err) //Check if the email was send to the right email address var resultsMailbox JSONMessageHeaderInbucket err = RetryInbucket(5, func() error { var mailErr error - resultsMailbox, mailErr = GetMailBox(smtpTo) + resultsMailbox, mailErr = GetMailBox(mail.smtpTo) return mailErr }) - require.Nil(t, err, "No emails found for address %s. error: %v", smtpTo, err) + require.Nil(t, err, "No emails found for address %s. error: %v", mail.smtpTo, err) require.NotEqual(t, len(resultsMailbox), 0) - require.Contains(t, resultsMailbox[0].To[0], mimeTo, "Wrong To recipient") + require.Contains(t, resultsMailbox[0].To[0], mail.mimeTo, "Wrong To recipient") - resultsEmail, err := GetMessageFromMailbox(smtpTo, resultsMailbox[0].ID) + resultsEmail, err := GetMessageFromMailbox(mail.smtpTo, resultsMailbox[0].ID) require.Nil(t, err) - require.Contains(t, emailBody, resultsEmail.Body.Text, "Wrong received message") + require.Contains(t, mail.htmlBody, resultsEmail.Body.Text, "Wrong received message") // verify that the To header of the email message is set to the MIME recipient, even though we got it out of the SMTP recipient's email inbox - assert.Equal(t, mimeTo, resultsEmail.Header["To"][0]) + assert.Equal(t, mail.mimeTo, resultsEmail.Header["To"][0]) // verify that the MIME from address is correct - unfortunately, we can't verify the SMTP from address - assert.Equal(t, from.String(), resultsEmail.Header["From"][0]) + assert.Equal(t, mail.from.String(), resultsEmail.Header["From"][0]) // check that the custom mime headers came through - header case seems to get mutated assert.Equal(t, "TestValue", resultsEmail.Header["Testheader"][0]) @@ -330,7 +379,8 @@ func TestSendMail(t *testing.T) { for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { - appErr = SendMail(mocm, "", "", mail.Address{}, tc.replyTo, "", "", nil, nil, nil, mockBackend, time.Now()) + mail := mailData{"", "", mail.Address{}, tc.replyTo, "", "", nil, nil, nil} + appErr = SendMail(mocm, mail, mockBackend, time.Now()) require.Nil(t, appErr) if len(tc.contains) > 0 { require.Contains(t, string(mocm.data), tc.contains)