move admin email notifications call to rest/api module

This commit is contained in:
Dmitry Verkhoturov
2020-04-06 16:27:19 -05:00
committed by Umputun
parent 04d3541de1
commit cab3b8a831
9 changed files with 81 additions and 141 deletions
+5 -4
View File
@@ -438,6 +438,11 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
SimpleView: s.SimpleView,
}
// enable admin notifications only if admin email is set
if s.Notify.Email.AdminNotifications && s.Admin.Shared.Email != "" {
srv.AdminEmail = s.Admin.Shared.Email
}
srv.ScoreThresholds.Low, srv.ScoreThresholds.Critical = s.LowScore, s.CriticalScore
var devAuth *provider.DevAuthServer
@@ -787,10 +792,6 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *
return tkn, nil
},
}
// enable admin notifications only if admin email is set
if s.Notify.Email.AdminNotifications && s.Admin.Shared.Email != "" {
emailParams.AdminEmail = s.Admin.Shared.Email
}
smtpParams := notify.SmtpParams{
Host: s.SMTP.Host,
Port: s.SMTP.Port,
+16 -72
View File
@@ -14,7 +14,6 @@ import (
log "github.com/go-pkgz/lgr"
"github.com/go-pkgz/repeater"
"github.com/hashicorp/go-multierror"
"github.com/pkg/errors"
)
@@ -26,7 +25,6 @@ type EmailParams struct {
VerificationTemplate string // verification message template
SubscribeURL string // full subscribe handler URL
UnsubscribeURL string // full unsubscribe handler URL
AdminEmail string // admin email for sending notifications about new messages
TokenGenFn func(userID, email, site string) (string, error) // Unsubscribe token generation function
}
@@ -242,91 +240,42 @@ func NewEmail(emailParams EmailParams, smtpParams SmtpParams) (*Email, error) {
// also sends email to site administrator if appropriate option is set.
// Thread safe
func (e *Email) Send(ctx context.Context, req Request) (err error) {
if req.Email == "" {
// this means we can't send this request via Email
return nil
}
select {
case <-ctx.Done():
return errors.Errorf("sending message to %q aborted due to canceled context", req.Email)
default:
}
var emails []emailMessage
errs := new(multierror.Error)
emailMsg, err := e.createUserEmail(req)
errs = multierror.Append(errs, errors.Wrap(err, "problem creating email notification on comment for user"))
if emailMsg != nil {
emails = append(emails, *emailMsg)
}
emailMsg, err = e.createAdminEmail(req)
errs = multierror.Append(errs, errors.Wrap(err, "problem creating email notification on comment for admin"))
if emailMsg != nil {
emails = append(emails, *emailMsg)
}
for _, msg := range emails {
err = repeater.NewDefault(5, time.Millisecond*250).
Do(ctx, func() error { return e.sendMessage(msg) })
errs = multierror.Append(errs, err)
}
return errs.ErrorOrNil()
}
// construct email for user if it's necessary:
// 1. in case user requested validation message
// 2. in case comment have is a reply and parent message owner subscribed for email notifications
func (e *Email) createUserEmail(req Request) (*emailMessage, error) {
if req.Email == "" {
// this means we can't send this request via Email
return nil, nil
}
var msg string
var err error
if req.Verification.Token != "" {
log.Printf("[DEBUG] send verification via %s, user %s", e, req.Verification.User)
msg, err = e.buildVerificationMessage(req.Verification.User, req.Email, req.Verification.Token, req.Verification.SiteID)
if err != nil {
return nil, err
return err
}
}
if req.Comment.ID != "" {
if req.parent.User.ID == req.Comment.User.ID {
if req.parent.User.ID == req.Comment.User.ID && !req.ForAdmin {
// don't send anything if if user replied to their own comment
return nil, nil
return nil
}
log.Printf("[DEBUG] send notification via %s, comment id %s", e, req.Comment.ID)
msg, err = e.buildMessageFromRequest(req, false)
msg, err = e.buildMessageFromRequest(req, req.ForAdmin)
if err != nil {
return nil, err
return err
}
}
return &emailMessage{from: e.From, to: req.Email, message: msg}, nil
}
// construct email for admin if it's enabled
func (e *Email) createAdminEmail(req Request) (*emailMessage, error) {
if req.Verification.Token != "" {
// don't notify admin on verification request
return nil, nil
}
if e.AdminEmail == "" {
// don't send anything if notifications to admin are disabled
return nil, nil
}
var msg string
var err error
log.Printf("[DEBUG] send admin notification via %s, comment id %s", e, req.Comment.ID)
msg, err = e.buildMessageFromRequest(req, true)
if err != nil {
return nil, err
}
return &emailMessage{from: e.From, to: e.AdminEmail, message: msg}, nil
return repeater.NewDefault(5, time.Millisecond*250).Do(
ctx,
func() error {
return e.sendMessage(emailMessage{from: e.From, to: req.Email, message: msg})
})
}
// buildVerificationMessage generates verification email message based on given input
@@ -365,11 +314,6 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
unsubscribeLink = ""
}
email := req.Email
if forAdmin {
email = e.AdminEmail
}
commentUrlPrefix := req.Comment.Locator.URL + uiNav
msg := bytes.Buffer{}
tmplData := msgTmplData{
@@ -379,7 +323,7 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
CommentLink: commentUrlPrefix + req.Comment.ID,
CommentDate: req.Comment.Timestamp,
PostTitle: req.Comment.PostTitle,
Email: email,
Email: req.Email,
UnsubscribeLink: unsubscribeLink,
ForAdmin: forAdmin,
}
@@ -395,7 +339,7 @@ func (e *Email) buildMessageFromRequest(req Request, forAdmin bool) (string, err
if err != nil {
return "", errors.Wrapf(err, "error executing template to build comment reply message")
}
return e.buildMessage(subject, msg.String(), email, "text/html", unsubscribeLink)
return e.buildMessage(subject, msg.String(), req.Email, "text/html", unsubscribeLink)
}
// buildMessage generates email message to send using net/smtp.Data()
+11 -38
View File
@@ -100,17 +100,15 @@ func TestEmailSendErrors(t *testing.T) {
e.verifyTmpl, err = template.New("test").Parse("{{.Test}}")
assert.NoError(t, err)
err = e.Send(context.Background(), Request{Email: "bad@example.org", Verification: VerificationMetadata{Token: "some"}})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error executing template to build verification message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.verifyTmplData")
assert.EqualError(t, e.Send(context.Background(), Request{Email: "bad@example.org", Verification: VerificationMetadata{Token: "some"}}),
"error executing template to build verification message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.verifyTmplData")
e.verifyTmpl, err = template.New("test").Parse(defaultEmailVerificationTemplate)
assert.NoError(t, err)
e.msgTmpl, err = template.New("test").Parse("{{.Test}}")
assert.NoError(t, err)
err = e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "test"}}, Email: "bad@example.org"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error executing template to build comment reply message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.msgTmplData")
assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "test"}}, Email: "bad@example.org"}),
"error executing template to build comment reply message: template: test:1:2: executing \"test\" at <.Test>: can't evaluate field Test in type notify.msgTmplData")
e.msgTmpl, err = template.New("test").Parse(defaultEmailTemplate)
assert.NoError(t, err)
@@ -120,9 +118,8 @@ func TestEmailSendErrors(t *testing.T) {
"sending message to \"bad@example.org\" aborted due to canceled context")
e.smtp = &fakeTestSMTP{}
err = e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Email: "bad@example.org"})
assert.Error(t, err)
assert.Contains(t, err.Error(), "error creating token for unsubscribe link: token generation error")
assert.EqualError(t, e.Send(context.Background(), Request{Comment: store.Comment{ID: "999"}, parent: store.Comment{User: store.User{ID: "error"}}, Email: "bad@example.org"}),
"error creating token for unsubscribe link: token generation error")
e.msgTmpl, err = template.New("test").Parse(defaultEmailTemplate)
assert.NoError(t, err)
}
@@ -199,7 +196,7 @@ func TestEmail_Send(t *testing.T) {
assert.Equal(t, 1, fakeSmtp.readQuitCount())
assert.Equal(t, "test@example.org", fakeSmtp.readRcpt())
// test buildMessageFromRequest separately for message text
res, err := email.buildMessageFromRequest(req, false)
res, err := email.buildMessageFromRequest(req, req.ForAdmin)
assert.NoError(t, err)
assert.Contains(t, res, `From: from@example.org
To: test@example.org
@@ -210,39 +207,15 @@ Content-Type: text/html; charset="UTF-8"
List-Unsubscribe-Post: List-Unsubscribe=One-Click
List-Unsubscribe: <https://remark42.com/api/v1/email/unsubscribe?site=&tkn=token>
Date: `)
}
func TestEmail_SendAdmin(t *testing.T) {
email, err := NewEmail(EmailParams{From: "from@example.org", AdminEmail: "admin@example.org"}, SmtpParams{})
assert.NoError(t, err)
assert.NotNil(t, email)
fakeSmtp := fakeTestSMTP{}
email.smtp = &fakeSmtp
email.TokenGenFn = TokenGenFn
req := Request{
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, ParentID: "1", PostTitle: "test_title"},
parent: store.Comment{ID: "1", User: store.User{ID: "999", Name: "parent_user"}},
}
assert.NoError(t, email.Send(context.TODO(), req))
assert.Equal(t, "from@example.org", fakeSmtp.readMail())
assert.Equal(t, 1, fakeSmtp.readQuitCount())
assert.Equal(t, "admin@example.org", fakeSmtp.readRcpt())
// test buildMessageFromRequest separately for message text
res, err := email.buildMessageFromRequest(req, true)
assert.NoError(t, err)
assert.Contains(t, res, `From: from@example.org
To: admin@example.org
Subject: New comment to your site for "test_title"
Content-Transfer-Encoding: quoted-printable
MIME-version: 1.0
Content-Type: text/html; charset="UTF-8"
Date: `)
// send email to admin without parent set
req = Request{
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, PostTitle: "test_title"},
Comment: store.Comment{ID: "999", User: store.User{ID: "1", Name: "test_user"}, PostTitle: "test_title"},
Email: "admin@example.org",
ForAdmin: true,
}
assert.NoError(t, email.Send(context.TODO(), req))
res, err = email.buildMessageFromRequest(req, true)
res, err = email.buildMessageFromRequest(req, req.ForAdmin)
assert.NoError(t, err)
assert.Contains(t, res, `From: from@example.org
To: admin@example.org
+12 -6
View File
@@ -37,9 +37,11 @@ type Store interface {
// Request notification either about comment or about particular user verification
type Request struct {
Comment store.Comment // if set sent notifications about new comment
parent store.Comment // fetched only in case Comment is set
Email string // if set (also) send email
Comment store.Comment // if set sent notifications about new comment
parent store.Comment // fetched only in case Comment is set
Email string // if set (also) send email
ForAdmin bool // if set, message supposed to be sent to administrator
Verification VerificationMetadata // if set sent verification notification
}
@@ -82,9 +84,13 @@ func (s *Service) Submit(req Request) {
if s.dataService != nil && req.Comment.ParentID != "" {
if p, err := s.dataService.Get(req.Comment.Locator, req.Comment.ParentID, store.User{}); err == nil {
req.parent = p
req.Email, err = s.dataService.GetUserEmail(req.Comment.Locator.SiteID, p.User.ID)
if err != nil {
log.Printf("[WARN] can't read email for %s, %v", p.User.ID, err)
// user notification, should fetch email for it.
// administrator notification comes with pre-set email
if req.Email == "" {
req.Email, err = s.dataService.GetUserEmail(req.Comment.Locator.SiteID, p.User.ID)
if err != nil {
log.Printf("[WARN] can't read email for %s, %v", p.User.ID, err)
}
}
}
}
+5
View File
@@ -90,6 +90,11 @@ func (t *Telegram) Send(ctx context.Context, req Request) error {
// verification request received, send nothing
return nil
}
if req.ForAdmin {
// request for administrator received, do nothing with it
// as we already sent message on request without this flag set
return nil
}
client := http.Client{Timeout: telegramTimeOut}
log.Printf("[DEBUG] send telegram notification to %s, comment id %s", t.channelID, req.Comment.ID)
+2
View File
@@ -50,6 +50,7 @@ type Rest struct {
AnonVote bool
WebRoot string
RemarkURL string
AdminEmail string
ReadOnlyAge int
SharedSecret string
ScoreThresholds struct {
@@ -364,6 +365,7 @@ func (s *Rest) controllerGroups() (public, private, admin, rss) {
authenticator: s.Authenticator,
notifyService: s.NotifyService,
remarkURL: s.RemarkURL,
adminEmail: s.AdminEmail,
anonVote: s.AnonVote,
}
+6
View File
@@ -39,6 +39,7 @@ type private struct {
notifyService *notify.Service
authenticator *auth.Service
remarkURL string
adminEmail string
anonVote bool
}
@@ -131,9 +132,14 @@ func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
s.cache.Flush(cache.Flusher(comment.Locator.SiteID).
Scopes(comment.Locator.URL, lastCommentsScope, comment.User.ID, comment.Locator.SiteID))
// user notification
if s.notifyService != nil {
s.notifyService.Submit(notify.Request{Comment: finalComment})
}
// admin notification
if s.notifyService != nil && s.adminEmail != "" {
s.notifyService.Submit(notify.Request{Comment: finalComment, Email: s.adminEmail, ForAdmin: true})
}
log.Printf("[DEBUG] created commend %+v", finalComment)
+20 -18
View File
@@ -596,11 +596,12 @@ func TestRest_EmailNotification(t *testing.T) {
parentComment := store.Comment{}
require.NoError(t, render.DecodeJSON(strings.NewReader(string(body)), &parentComment))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 5)
require.Equal(t, 1, len(mockDestination.Get()))
assert.Equal(t, "", mockDestination.Get()[0].Email)
time.Sleep(time.Millisecond * 30)
require.Equal(t, 2, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[0].Email)
assert.Equal(t, "admin@example.org", mockDestination.Get()[1].Email)
// create child comment from another user, no email notification expected
// create child comment from another user, email notification only to admin expected
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf(
`{"text": "test 456",
"pid": "%s",
@@ -615,9 +616,10 @@ func TestRest_EmailNotification(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 5)
require.Equal(t, 2, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[1].Email)
time.Sleep(time.Millisecond * 30)
require.Equal(t, 4, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[2].Email)
assert.Equal(t, "admin@example.org", mockDestination.Get()[3].Email)
// send confirmation token for email
req, err = http.NewRequest(http.MethodPost, ts.URL+"/api/v1/email/subscribe?site=remark42&address=good@example.com", nil)
@@ -629,10 +631,10 @@ func TestRest_EmailNotification(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 5)
require.Equal(t, 3, len(mockDestination.Get()))
require.NotEmpty(t, mockDestination.Get()[2].Verification)
verificationToken := mockDestination.Get()[2].Verification.Token
time.Sleep(time.Millisecond * 30)
require.Equal(t, 5, len(mockDestination.Get()))
require.NotEmpty(t, mockDestination.Get()[4].Verification)
verificationToken := mockDestination.Get()[4].Verification.Token
// verify email
req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", verificationToken), nil)
@@ -674,9 +676,9 @@ func TestRest_EmailNotification(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 5)
require.Equal(t, 4, len(mockDestination.Get()))
assert.Equal(t, "good@example.com", mockDestination.Get()[3].Email)
time.Sleep(time.Millisecond * 30)
require.Equal(t, 7, len(mockDestination.Get()))
assert.Equal(t, "good@example.com", mockDestination.Get()[5].Email)
// delete user's email
req, err = http.NewRequest(http.MethodDelete, ts.URL+"/api/v1/email?site=remark42", nil)
@@ -688,7 +690,7 @@ func TestRest_EmailNotification(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode, string(body))
// create child comment from another user, no email notification expected
// create child comment from another user, no email notification expected except for admin
req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(
`{"text": "test 321",
"user": {"name": "other_user"},
@@ -702,9 +704,9 @@ func TestRest_EmailNotification(t *testing.T) {
require.NoError(t, err)
require.Equal(t, http.StatusCreated, resp.StatusCode, string(body))
// wait for mock notification Submit to kick off
time.Sleep(time.Millisecond * 5)
require.Equal(t, 5, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[4].Email)
time.Sleep(time.Millisecond * 30)
require.Equal(t, 9, len(mockDestination.Get()))
assert.Empty(t, mockDestination.Get()[7].Email)
}
func TestRest_UserAllData(t *testing.T) {
+4 -3
View File
@@ -368,9 +368,10 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) {
SecretReader: token.SecretFunc(func() (string, error) { return "secret", nil }),
AvatarStore: avatar.NewLocalFS(tmp + "/ava-remark42"),
}),
Cache: memCache,
WebRoot: tmp,
RemarkURL: "https://demo.remark42.com",
Cache: memCache,
WebRoot: tmp,
RemarkURL: "https://demo.remark42.com",
AdminEmail: "admin@example.org",
ImageService: image.NewService(&image.FileSystem{
Location: tmp + "/pics-remark42",
Partitions: 100,