From 77fa7146c6c7f9b1efdc06c24ef7d1f278871e13 Mon Sep 17 00:00:00 2001 From: KN4CK3R Date: Sat, 22 May 2021 08:47:16 +0200 Subject: [PATCH] Add email headers (#15939) * Added additional email headers. * Added tests. Co-authored-by: Lunny Xiao --- services/mailer/mail.go | 52 ++++++++++++++-- services/mailer/mail_issue.go | 4 +- services/mailer/mail_test.go | 109 +++++++++++++++++----------------- 3 files changed, 104 insertions(+), 61 deletions(-) diff --git a/services/mailer/mail.go b/services/mailer/mail.go index f22140c9f7..ea3edaa90d 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -11,6 +11,7 @@ import ( "html/template" "mime" "regexp" + "strconv" "strings" texttmpl "text/template" @@ -174,7 +175,7 @@ func SendCollaboratorMail(u, doer *models.User, repo *models.Repository) { SendAsync(msg) } -func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []string, fromMention bool, info string) ([]*Message, error) { +func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipients []*models.User, fromMention bool, info string) ([]*Message, error) { var ( subject string link string @@ -265,9 +266,9 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []str } // Make sure to compose independent messages to avoid leaking user emails - msgs := make([]*Message, 0, len(tos)) - for _, to := range tos { - msg := NewMessageFrom([]string{to}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) + msgs := make([]*Message, 0, len(recipients)) + for _, recipient := range recipients { + msg := NewMessageFrom([]string{recipient.Email}, ctx.Doer.DisplayName(), setting.MailService.FromEmail, subject, mailBody.String()) msg.Info = fmt.Sprintf("Subject: %s, %s", subject, info) // Set Message-ID on first message so replies know what to reference @@ -277,12 +278,51 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, tos []str msg.SetHeader("In-Reply-To", "<"+ctx.Issue.ReplyReference()+">") msg.SetHeader("References", "<"+ctx.Issue.ReplyReference()+">") } + + for key, value := range generateAdditionalHeaders(ctx, actType, recipient) { + msg.SetHeader(key, value) + } + msgs = append(msgs, msg) } return msgs, nil } +func generateAdditionalHeaders(ctx *mailCommentContext, reason string, recipient *models.User) map[string]string { + repo := ctx.Issue.Repo + + return map[string]string{ + // https://datatracker.ietf.org/doc/html/rfc2919 + "List-ID": fmt.Sprintf("%s <%s.%s.%s>", repo.FullName(), repo.Name, repo.OwnerName, setting.Domain), + + // https://datatracker.ietf.org/doc/html/rfc2369 + "List-Archive": fmt.Sprintf("<%s>", repo.HTMLURL()), + //"List-Post": https://github.com/go-gitea/gitea/pull/13585 + //"List-Unsubscribe": https://github.com/go-gitea/gitea/issues/10808, https://github.com/go-gitea/gitea/issues/13283 + + "X-Gitea-Reason": reason, + "X-Gitea-Sender": ctx.Doer.DisplayName(), + "X-Gitea-Recipient": recipient.DisplayName(), + "X-Gitea-Recipient-Address": recipient.Email, + "X-Gitea-Repository": repo.Name, + "X-Gitea-Repository-Path": repo.FullName(), + "X-Gitea-Repository-Link": repo.HTMLURL(), + "X-Gitea-Issue-ID": strconv.FormatInt(ctx.Issue.Index, 10), + "X-Gitea-Issue-Link": ctx.Issue.HTMLURL(), + + "X-GitHub-Reason": reason, + "X-GitHub-Sender": ctx.Doer.DisplayName(), + "X-GitHub-Recipient": recipient.DisplayName(), + "X-GitHub-Recipient-Address": recipient.Email, + + "X-GitLab-NotificationReason": reason, + "X-GitLab-Project": repo.Name, + "X-GitLab-Project-Path": repo.FullName(), + "X-GitLab-Issue-IID": strconv.FormatInt(ctx.Issue.Index, 10), + } +} + func sanitizeSubject(subject string) string { runes := []rune(strings.TrimSpace(subjectRemoveSpaces.ReplaceAllLiteralString(subject, " "))) if len(runes) > mailMaxSubjectRunes { @@ -294,9 +334,9 @@ func sanitizeSubject(subject string) string { // SendIssueAssignedMail composes and sends issue assigned email func SendIssueAssignedMail(issue *models.Issue, doer *models.User, content string, comment *models.Comment, recipients []*models.User) error { - langMap := make(map[string][]string) + langMap := make(map[string][]*models.User) for _, user := range recipients { - langMap[user.Language] = append(langMap[user.Language], user.Email) + langMap[user.Language] = append(langMap[user.Language], user) } for lang, tos := range langMap { diff --git a/services/mailer/mail_issue.go b/services/mailer/mail_issue.go index bb541d27a0..cf5265872b 100644 --- a/services/mailer/mail_issue.go +++ b/services/mailer/mail_issue.go @@ -116,7 +116,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visite checkUnit = models.UnitTypePullRequests } - langMap := make(map[string][]string) + langMap := make(map[string][]*models.User) for _, user := range users { // At this point we exclude: // user that don't have all mails enabled or users only get mail on mention and this is one ... @@ -138,7 +138,7 @@ func mailIssueCommentBatch(ctx *mailCommentContext, users []*models.User, visite continue } - langMap[user.Language] = append(langMap[user.Language], user.Email) + langMap[user.Language] = append(langMap[user.Language], user) } for lang, receivers := range langMap { diff --git a/services/mailer/mail_test.go b/services/mailer/mail_test.go index 813e51c0d2..0a9112f3be 100644 --- a/services/mailer/mail_test.go +++ b/services/mailer/mail_test.go @@ -39,7 +39,7 @@ const bodyTpl = ` ` -func TestComposeIssueCommentMessage(t *testing.T) { +func prepareMailerTest(t *testing.T) (doer *models.User, repo *models.Repository, issue *models.Issue, comment *models.Comment) { assert.NoError(t, models.PrepareTestDatabase()) var mailService = setting.Mailer{ From: "test@gitea.com", @@ -48,18 +48,24 @@ func TestComposeIssueCommentMessage(t *testing.T) { setting.MailService = &mailService setting.Domain = "localhost" - doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository) - issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue) - comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment) + doer = models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + repo = models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository) + issue = models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue) + assert.NoError(t, issue.LoadRepo()) + comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment) + return +} + +func TestComposeIssueCommentMessage(t *testing.T) { + doer, _, issue, comment := prepareMailerTest(t) stpl := texttmpl.Must(texttmpl.New("issue/comment").Parse(subjectTpl)) btpl := template.Must(template.New("issue/comment").Parse(bodyTpl)) InitMailRender(stpl, btpl) - tos := []string{"test@gitea.com", "test2@gitea.com"} + recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}} msgs, err := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue, - Content: "test body", Comment: comment}, "en-US", tos, false, "issue comment") + Content: "test body", Comment: comment}, "en-US", recipients, false, "issue comment") assert.NoError(t, err) assert.Len(t, msgs, 2) gomailMsg := msgs[0].ToMessage() @@ -76,25 +82,15 @@ func TestComposeIssueCommentMessage(t *testing.T) { } func TestComposeIssueMessage(t *testing.T) { - assert.NoError(t, models.PrepareTestDatabase()) - var mailService = setting.Mailer{ - From: "test@gitea.com", - } - - setting.MailService = &mailService - setting.Domain = "localhost" - - doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository) - issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue) + doer, _, issue, _ := prepareMailerTest(t) stpl := texttmpl.Must(texttmpl.New("issue/new").Parse(subjectTpl)) btpl := template.Must(template.New("issue/new").Parse(bodyTpl)) InitMailRender(stpl, btpl) - tos := []string{"test@gitea.com", "test2@gitea.com"} + recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}, {Name: "Test2", Email: "test2@gitea.com"}} msgs, err := composeIssueCommentMessages(&mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue, - Content: "test body"}, "en-US", tos, false, "issue create") + Content: "test body"}, "en-US", recipients, false, "issue create") assert.NoError(t, err) assert.Len(t, msgs, 2) @@ -111,18 +107,8 @@ func TestComposeIssueMessage(t *testing.T) { } func TestTemplateSelection(t *testing.T) { - assert.NoError(t, models.PrepareTestDatabase()) - var mailService = setting.Mailer{ - From: "test@gitea.com", - } - - setting.MailService = &mailService - setting.Domain = "localhost" - - doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository) - issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue) - tos := []string{"test@gitea.com"} + doer, repo, issue, comment := prepareMailerTest(t) + recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}} stpl := texttmpl.Must(texttmpl.New("issue/default").Parse("issue/default/subject")) texttmpl.Must(stpl.New("issue/new").Parse("issue/new/subject")) @@ -146,38 +132,26 @@ func TestTemplateSelection(t *testing.T) { } msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCreateIssue, - Content: "test body"}, tos, false, "TestTemplateSelection") + Content: "test body"}, recipients, false, "TestTemplateSelection") expect(t, msg, "issue/new/subject", "issue/new/body") - comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment) msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCommentIssue, - Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") + Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection") expect(t, msg, "issue/default/subject", "issue/default/body") pull := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 2, Repo: repo, Poster: doer}).(*models.Issue) comment = models.AssertExistsAndLoadBean(t, &models.Comment{ID: 4, Issue: pull}).(*models.Comment) msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: pull, Doer: doer, ActionType: models.ActionCommentPull, - Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") + Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection") expect(t, msg, "pull/comment/subject", "pull/comment/body") msg = testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: models.ActionCloseIssue, - Content: "test body", Comment: comment}, tos, false, "TestTemplateSelection") + Content: "test body", Comment: comment}, recipients, false, "TestTemplateSelection") expect(t, msg, "Re: [user2/repo1] issue1 (#1)", "issue/close/body") } func TestTemplateServices(t *testing.T) { - assert.NoError(t, models.PrepareTestDatabase()) - var mailService = setting.Mailer{ - From: "test@gitea.com", - } - - setting.MailService = &mailService - setting.Domain = "localhost" - - doer := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) - repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1, Owner: doer}).(*models.Repository) - issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: 1, Repo: repo, Poster: doer}).(*models.Issue) - comment := models.AssertExistsAndLoadBean(t, &models.Comment{ID: 2, Issue: issue}).(*models.Comment) + doer, _, issue, comment := prepareMailerTest(t) assert.NoError(t, issue.LoadRepo()) expect := func(t *testing.T, issue *models.Issue, comment *models.Comment, doer *models.User, @@ -187,9 +161,9 @@ func TestTemplateServices(t *testing.T) { btpl := template.Must(template.New("issue/default").Parse(tplBody)) InitMailRender(stpl, btpl) - tos := []string{"test@gitea.com"} + recipients := []*models.User{{Name: "Test", Email: "test@gitea.com"}} msg := testComposeIssueCommentMessage(t, &mailCommentContext{Issue: issue, Doer: doer, ActionType: actionType, - Content: "test body", Comment: comment}, tos, fromMention, "TestTemplateServices") + Content: "test body", Comment: comment}, recipients, fromMention, "TestTemplateServices") subject := msg.ToMessage().GetHeader("Subject") msgbuf := new(bytes.Buffer) @@ -219,9 +193,38 @@ func TestTemplateServices(t *testing.T) { "//Re: //") } -func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, tos []string, fromMention bool, info string) *Message { - msgs, err := composeIssueCommentMessages(ctx, "en-US", tos, fromMention, info) +func testComposeIssueCommentMessage(t *testing.T, ctx *mailCommentContext, recipients []*models.User, fromMention bool, info string) *Message { + msgs, err := composeIssueCommentMessages(ctx, "en-US", recipients, fromMention, info) assert.NoError(t, err) assert.Len(t, msgs, 1) return msgs[0] } + +func TestGenerateAdditionalHeaders(t *testing.T) { + doer, _, issue, _ := prepareMailerTest(t) + + ctx := &mailCommentContext{Issue: issue, Doer: doer} + recipient := &models.User{Name: "Test", Email: "test@gitea.com"} + + headers := generateAdditionalHeaders(ctx, "dummy-reason", recipient) + + expected := map[string]string{ + "List-ID": "user2/repo1 ", + "List-Archive": "", + "X-Gitea-Reason": "dummy-reason", + "X-Gitea-Sender": "< Ur Tw ><", + "X-Gitea-Recipient": "Test", + "X-Gitea-Recipient-Address": "test@gitea.com", + "X-Gitea-Repository": "repo1", + "X-Gitea-Repository-Path": "user2/repo1", + "X-Gitea-Repository-Link": "https://try.gitea.io/user2/repo1", + "X-Gitea-Issue-ID": "1", + "X-Gitea-Issue-Link": "https://try.gitea.io/user2/repo1/issues/1", + } + + for key, value := range expected { + if assert.Contains(t, headers, key) { + assert.Equal(t, value, headers[key]) + } + } +}