From b90d076fdfa7c12259e922bacc3ecd02e30e589c Mon Sep 17 00:00:00 2001 From: Pavel Mineev Date: Sun, 10 May 2020 22:30:40 +0300 Subject: [PATCH] Move html to separete files --- Dockerfile | 4 +- Dockerfile.artifacts | 4 +- backend/app/cmd/server.go | 86 ++++----- backend/app/cmd/server_test.go | 18 +- backend/app/notify/email.go | 170 +++++------------- backend/app/notify/email_test.go | 150 +++++++++------- backend/app/notify/testdata/bad.html.tmpl | 1 + backend/app/notify/testdata/msg.html.tmpl | 20 +++ .../notify/testdata/verification.html.tmpl | 7 + backend/app/rest/api/rest.go | 2 + backend/app/rest/api/rest_private.go | 44 ++--- backend/app/rest/api/rest_private_test.go | 9 +- backend/app/rest/httperrors.go | 30 ++-- backend/app/rest/httperrors_test.go | 11 +- backend/app/templates/templates.go | 38 ++++ backend/app/templates/templates_test.go | 24 +++ .../app/templates/testdata/template.html.tmpl | 1 + .../email_confirmation_login.html.tmpl | 19 ++ .../email_confirmation_subscription.html.tmpl | 24 +++ backend/templates/email_reply.html.tmpl | 68 +++++++ backend/templates/email_unsubscribe.html.tmpl | 13 ++ backend/templates/error_response.html.tmpl | 13 ++ docs/developers-guide/email-templates.md | 13 ++ 23 files changed, 479 insertions(+), 290 deletions(-) create mode 100644 backend/app/notify/testdata/bad.html.tmpl create mode 100644 backend/app/notify/testdata/msg.html.tmpl create mode 100644 backend/app/notify/testdata/verification.html.tmpl create mode 100644 backend/app/templates/templates.go create mode 100644 backend/app/templates/templates_test.go create mode 100644 backend/app/templates/testdata/template.html.tmpl create mode 100644 backend/templates/email_confirmation_login.html.tmpl create mode 100644 backend/templates/email_confirmation_subscription.html.tmpl create mode 100644 backend/templates/email_reply.html.tmpl create mode 100644 backend/templates/email_unsubscribe.html.tmpl create mode 100644 backend/templates/error_response.html.tmpl create mode 100644 docs/developers-guide/email-templates.md diff --git a/Dockerfile b/Dockerfile index 542a6b4a..d8b056c9 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM umputun/baseimage:buildgo-latest as build-backend +FROM umputun/baseimage:buildgo as build-backend ARG CI ARG DRONE @@ -30,6 +30,8 @@ RUN \ if [ -z "$DRONE" ] ; then echo "runs outside of drone" && version="$(/script/git-rev.sh)" ; \ else version=${DRONE_TAG}${DRONE_BRANCH}${DRONE_PULL_REQUEST}-${DRONE_COMMIT:0:7}-$(date +%Y%m%d-%H:%M:%S) ; fi && \ echo "version=$version" && \ + statik --src=/build/backend/templates --dest=/build/backend/app -p templates -ns templates -f && \ + ls -la /build/backend/app/templates/statik.go && \ go build -o remark42 -ldflags "-X main.revision=${version} -s -w" ./app FROM node:10.11-alpine as build-frontend-deps diff --git a/Dockerfile.artifacts b/Dockerfile.artifacts index d7ce3ba4..0d696a0f 100644 --- a/Dockerfile.artifacts +++ b/Dockerfile.artifacts @@ -26,7 +26,7 @@ RUN cd /srv/frontend && \ npm run build && \ rm -rf ./node_modules -FROM umputun/baseimage:buildgo-latest as build-backend +FROM umputun/baseimage:buildgo as build-backend ARG GITHUB_TOKEN ENV SKIP_BACKEND_TEST=true @@ -46,6 +46,8 @@ RUN \ export WEB_ROOT=/build/backend/web && \ sed -i "s|https://demo.remark42.com|http://127.0.0.1:8080|g" ${WEB_ROOT}/*.js && \ statik --src=${WEB_ROOT} --dest=/build/backend/app/rest -p api -f && \ + statik --src=/build/backend/templates --dest=/build/backend/app -p templates -ns templates -f && \ + ls -la /build/backend/app/templates/statik.go && \ ls -la /build/backend/app/rest/api/statik.go && \ ls -la /build/backend/web/ diff --git a/backend/app/cmd/server.go b/backend/app/cmd/server.go index 36918ff4..0e6776fa 100644 --- a/backend/app/cmd/server.go +++ b/backend/app/cmd/server.go @@ -1,9 +1,9 @@ package cmd import ( + "io/ioutil" "context" "fmt" - "io/ioutil" "net/http" "net/url" "os" @@ -37,6 +37,7 @@ import ( "github.com/umputun/remark/backend/app/store/engine" "github.com/umputun/remark/backend/app/store/image" "github.com/umputun/remark/backend/app/store/service" + "github.com/umputun/remark/backend/app/templates" ) // ServerCommand with command line flags and env @@ -97,7 +98,7 @@ type ServerCommand struct { SMTPUserName string `long:"user" env:"USER" description:"[deprecated, use --smtp.username] enable TLS"` TLS bool `long:"tls" env:"TLS" description:"[deprecated, use --smtp.tls] SMTP TCP connection timeout"` TimeOut time.Duration `long:"timeout" env:"TIMEOUT" default:"10s" description:"[deprecated, use --smtp.timeout] SMTP TCP connection timeout"` - MsgTemplate string `long:"template" env:"TEMPLATE" description:"message template file"` + MsgTemplate string `long:"template" env:"TEMPLATE" description:"[deprecated, message template file]" default:"email_confirmation_login.html.tmpl"` } `group:"email" namespace:"email" env-namespace:"EMAIL"` } `group:"auth" namespace:"auth" env-namespace:"AUTH"` @@ -303,6 +304,9 @@ func (s *ServerCommand) HandleDeprecatedFlags() (result []DeprecatedFlag) { s.SMTP.TimeOut = s.Auth.Email.TimeOut result = append(result, DeprecatedFlag{Old: "auth.email.timeout", New: "smtp.timeout", RemoveVersion: "1.7.0"}) } + if s.Auth.Email.MsgTemplate != "email_confirmation_login.html.tmpl" { + result = append(result, DeprecatedFlag{Old: "auth.email.template", RemoveVersion: "1.9.0"}) + } if s.LegacyImageProxy && !s.ImageProxy.HTTP2HTTPS { s.ImageProxy.HTTP2HTTPS = s.LegacyImageProxy result = append(result, DeprecatedFlag{Old: "img-proxy", New: "image-proxy.http2https", RemoveVersion: "1.7.0"}) @@ -362,7 +366,10 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) { if err != nil { return nil, errors.Wrap(err, "failed to make avatar store") } - authenticator := s.makeAuthenticator(dataService, avatarStore, adminStore) + authenticator, err := s.makeAuthenticator(dataService, avatarStore, adminStore) + if err != nil { + return nil, errors.Wrap(err, "failed to make authenticator") + } exporter := &migrator.Native{DataStore: dataService} @@ -659,29 +666,7 @@ func (s *ServerCommand) makeCache() (LoadingCache, error) { return nil, errors.Errorf("unsupported cache type %s", s.Cache.Type) } -var msgTemplate = ` - - - - - - - -
-

Remark42

-

Confirmation for {{.User}} on site {{.Site}}

-
-

TOKEN

-

Copy and paste this text into “token” field on comments page

-

{{.Token}}

-
-

Sent to {{.Address}}

-
- - -` - -func (s *ServerCommand) addAuthProviders(authenticator *auth.Service) { +func (s *ServerCommand) addAuthProviders(authenticator *auth.Service) error { providers := 0 if s.Auth.Google.CID != "" && s.Auth.Google.CSEC != "" { @@ -724,7 +709,11 @@ func (s *ServerCommand) addAuthProviders(authenticator *auth.Service) { ContentType: s.Auth.Email.ContentType, } sndr := sender.NewEmailClient(params, log.Default()) - authenticator.AddVerifProvider("email", s.loadEmailTemplate(), sndr) + tmpl, err := s.loadEmailTemplate() + if err != nil { + return err + } + authenticator.AddVerifProvider("email", tmpl, sndr) } if s.Auth.Anonymous { @@ -748,22 +737,29 @@ func (s *ServerCommand) addAuthProviders(authenticator *auth.Service) { if providers == 0 { log.Printf("[WARN] no auth providers defined") } + + return nil } -// loadEmailTemplate trying to get template from opts MsgTemplate and default to embedded -// if not defined or failed to load -func (s *ServerCommand) loadEmailTemplate() string { - tmpl := msgTemplate - if s.Auth.Email.MsgTemplate != "" { - log.Printf("[DEBUG] load email template from %s", s.Auth.Email.MsgTemplate) - b, err := ioutil.ReadFile(s.Auth.Email.MsgTemplate) - if err == nil { - tmpl = string(b) - } else { - log.Printf("[WARN] failed to load email template from %s, %v", s.Auth.Email.MsgTemplate, err) - } +// loadEmailTemplate trying to get template from statik +func (s *ServerCommand) loadEmailTemplate() (string, error) { + var file []byte + var err error + + if s.Auth.Email.MsgTemplate == "email_confirmation_login.html.tmpl" { + fs := templates.NewFS() + file, err = fs.ReadFile(s.Auth.Email.MsgTemplate) + } else { + // deprecated loading from an external file, should be removed before v1.9.0 + file, err = ioutil.ReadFile(s.Auth.Email.MsgTemplate) + log.Printf("[INFO] template %s will be read from disk", s.Auth.Email.MsgTemplate) } - return tmpl + + if err != nil { + return "", errors.Wrapf(err, "failed to read file %s", s.Auth.Email.MsgTemplate) + } + + return string(file), nil } func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *auth.Service) (*notify.Service, error) { @@ -859,7 +855,7 @@ func (s *ServerCommand) makeSSLConfig() (config api.SSLConfig, err error) { return config, err } -func (s *ServerCommand) makeAuthenticator(ds *service.DataStore, avas avatar.Store, admns admin.Store) *auth.Service { +func (s *ServerCommand) makeAuthenticator(ds *service.DataStore, avas avatar.Store, admns admin.Store) (*auth.Service, error) { authenticator := auth.NewService(auth.Opts{ URL: strings.TrimSuffix(s.RemarkURL, "/"), Issuer: "remark42", @@ -915,8 +911,12 @@ func (s *ServerCommand) makeAuthenticator(ds *service.DataStore, avas avatar.Sto RefreshCache: newAuthRefreshCache(), UseGravatar: true, }) - s.addAuthProviders(authenticator) - return authenticator + + if err := s.addAuthProviders(authenticator); err != nil { + return nil, err + } + + return authenticator, nil } // authRefreshCache used by authenticator to minimize repeatable token refreshes diff --git a/backend/app/cmd/server_test.go b/backend/app/cmd/server_test.go index ac2b00ab..defbcc85 100644 --- a/backend/app/cmd/server_test.go +++ b/backend/app/cmd/server_test.go @@ -372,6 +372,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { "--auth.email.user=test_user", "--auth.email.passwd=test_password", "--auth.email.timeout=15s", + "--auth.email.template=file.tmpl", } assert.Empty(t, s.SMTP.Host) assert.Empty(t, s.SMTP.Port) @@ -390,6 +391,7 @@ func TestServerApp_DeprecatedArgs(t *testing.T) { {Old: "auth.email.user", New: "smtp.username", RemoveVersion: "1.7.0"}, {Old: "auth.email.passwd", New: "smtp.password", RemoveVersion: "1.7.0"}, {Old: "auth.email.timeout", New: "smtp.timeout", RemoveVersion: "1.7.0"}, + {Old: "auth.email.template", RemoveVersion: "1.9.0"}, }, deprecatedFlags) assert.Equal(t, "smtp.example.org", s.SMTP.Host) @@ -490,7 +492,7 @@ func TestServerAuthHooks(t *testing.T) { require.NoError(t, err) t.Logf("no-aud claims: %s", tkNoAud) req, err = http.NewRequest("POST", fmt.Sprintf("http://localhost:%d/api/v1/comment", port), - strings.NewReader(`{"text": "test 123", "locator":{"url": "https://radio-t.com/p/2018/12/29/podcast-631/", + strings.NewReader(`{"text": "test 123", "locator":{"url": "https://radio-t.com/p/2018/12/29/podcast-631/", "site": "remark"}}`)) require.NoError(t, err) req.Header.Set("X-JWT", tkNoAud) @@ -534,16 +536,14 @@ func TestServerAuthHooks(t *testing.T) { func TestServer_loadEmailTemplate(t *testing.T) { cmd := ServerCommand{} cmd.Auth.Email.MsgTemplate = "testdata/email.tmpl" - r := cmd.loadEmailTemplate() + r, err := cmd.loadEmailTemplate() + assert.NoError(t, err) assert.Equal(t, "The token is {{.Token}}", r) - cmd.Auth.Email.MsgTemplate = "" - r = cmd.loadEmailTemplate() - assert.Contains(t, r, "Remark42") - - cmd.Auth.Email.MsgTemplate = "bad-file" - r = cmd.loadEmailTemplate() - assert.Contains(t, r, "Remark42") + cmd.Auth.Email.MsgTemplate = "badpath.tmpl" + r, err = cmd.loadEmailTemplate() + assert.EqualError(t, err, "failed to read file badpath.tmpl: open badpath.tmpl: no such file or directory") + assert.Equal(t, r, "") } func chooseRandomUnusedPort() (port int) { diff --git a/backend/app/notify/email.go b/backend/app/notify/email.go index 33eb4a0a..a8daf008 100644 --- a/backend/app/notify/email.go +++ b/backend/app/notify/email.go @@ -15,16 +15,18 @@ import ( log "github.com/go-pkgz/lgr" "github.com/go-pkgz/repeater" "github.com/pkg/errors" + + "github.com/umputun/remark/backend/app/templates" ) // EmailParams contain settings for email notifications type EmailParams struct { - From string // from email address - MsgTemplate string // request message template - VerificationSubject string // verification message subject - VerificationTemplate string // verification message template - SubscribeURL string // full subscribe handler URL - UnsubscribeURL string // full unsubscribe handler URL + From string // from email address + MsgTemplatePath string // path to request message template + VerificationSubject string // verification message sub + VerificationTemplatePath string // path to verification template + SubscribeURL string // full subscribe handler URL + UnsubscribeURL string // full unsubscribe handler URL TokenGenFn func(userID, email, site string) (string, error) // Unsubscribe token generation function } @@ -101,139 +103,65 @@ type verifyTmplData struct { } const ( - defaultVerificationSubject = "Email verification" - defaultEmailTimeout = 10 * time.Second - defaultEmailTemplate = ` - - - - - - - - -
-

Remark42

- {{- if .ForAdmin}} -
New comment from {{.UserName}} on your site {{if .PostTitle}} to «{{.PostTitle}}»{{ end }}
- {{- else }} -
New reply from {{.UserName}} on your comment{{if .PostTitle}} to «{{.PostTitle}}»{{ end }}
- {{- end }} -
- {{- if .ParentCommentText}} -
- - {{.ParentUserName}} - {{.ParentCommentDate.Format "02.01.2006 at 15:04"}} - Show -
-
- {{.ParentCommentText}} -
- {{- end }} -
-
- - {{.UserName}} - {{.CommentDate.Format "02.01.2006 at 15:04"}} - Reply -
-
{{.CommentText}}
-
-
-
- Sent to {{.Email}}{{if not .ForAdmin}} for {{.ParentUserName}}{{ end }} -
- {{- if .UnsubscribeLink}} - Unsubscribe - {{- end }} - -
[{{.CommentDate.Format "02.01.2006 at 15:04"}}]
-
-
- - -` - defaultEmailVerificationTemplate = ` - - - - - - - -
-

Remark42

-

Confirmation for {{.User}} on site {{.Site}}

- {{- if .SubscribeURL}} -

Click here to subscribe to email notifications

-

Alternatively, you can use code below for subscription.

- {{- end }} -
-

TOKEN

-

Copy and paste this text into “token” field on comments page

-

{{.Token}}

-
-

Sent to {{.Email}}

-
- - -` + defaultVerificationSubject = "Email verification" + defaultEmailTimeout = 10 * time.Second + defaultEmailTemplatePath = "email_reply.html.tmpl" + defaultEmailVerificationTemplatePath = "email_confirmation_subscription.html.tmpl" ) // NewEmail makes new Email object, returns error in case of e.MsgTemplate or e.VerificationTemplate parsing error func NewEmail(emailParams EmailParams, smtpParams SMTPParams) (*Email, error) { // set up Email emailParams res := Email{EmailParams: emailParams} - if res.MsgTemplate == "" { - res.MsgTemplate = defaultEmailTemplate - } - if res.VerificationTemplate == "" { - res.VerificationTemplate = defaultEmailVerificationTemplate - } - if res.VerificationSubject == "" { - res.VerificationSubject = defaultVerificationSubject - } - - // set up SMTP emailParams res.smtp = &emailClient{} res.SMTPParams = smtpParams if res.TimeOut <= 0 { res.TimeOut = defaultEmailTimeout } + if res.VerificationSubject == "" { + res.VerificationSubject = defaultVerificationSubject + } + + // initialize templates + err := res.setTemplates() + if err != nil { + return nil, errors.Wrap(err, "can't set templates") + } + log.Printf("[DEBUG] Create new email notifier for server %s with user %s, timeout=%s", res.Host, res.Username, res.TimeOut) - // initialize templates + return &res, nil +} + +func (e *Email) setTemplates() error { var err error - if res.msgTmpl, err = template.New("messageFromRequest").Parse(res.MsgTemplate); err != nil { - return nil, errors.Wrapf(err, "can't parse message template") + var msgTmplFile, verifyTmplFile []byte + fs := templates.NewFS() + + if e.VerificationTemplatePath == "" { + e.VerificationTemplatePath = defaultEmailVerificationTemplatePath } - if res.verifyTmpl, err = template.New("messageFromRequest").Parse(res.VerificationTemplate); err != nil { - return nil, errors.Wrapf(err, "can't parse verification template") + + if e.MsgTemplatePath == "" { + e.MsgTemplatePath = defaultEmailTemplatePath } - return &res, err + + if msgTmplFile, err = fs.ReadFile(e.MsgTemplatePath); err != nil { + return errors.Wrapf(err, "can't read message template") + } + if verifyTmplFile, err = fs.ReadFile(e.VerificationTemplatePath); err != nil { + return errors.Wrapf(err, "can't read verification template") + } + if e.msgTmpl, err = template.New("msgTmpl").Parse(string(msgTmplFile)); err != nil { + return errors.Wrapf(err, "can't parse message template") + } + if e.verifyTmpl, err = template.New("verifyTmpl").Parse(string(verifyTmplFile)); err != nil { + return errors.Wrapf(err, "can't parse verification template") + } + + return nil } // Send email about comment reply to Request.Email if it's set, diff --git a/backend/app/notify/email_test.go b/backend/app/notify/email_test.go index 127cd16f..b12e10e6 100644 --- a/backend/app/notify/email_test.go +++ b/backend/app/notify/email_test.go @@ -17,79 +17,86 @@ import ( ) func TestEmailNew(t *testing.T) { - var testSet = []struct { - name string - err bool - errText string + emailParams := EmailParams{ + From: "test@from", + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", + } + smtpParams := SMTPParams{ + Host: "test@host", + Port: 1000, + TLS: true, + Username: "test@username", + Password: "test@password", + TimeOut: time.Second, + } + + email, err := NewEmail(emailParams, smtpParams) + + assert.NoError(t, err) + assert.NotNil(t, email, "email returned") + + assert.NotNil(t, email.msgTmpl, "e.template is set") + assert.Equal(t, emailParams.From, email.EmailParams.From, "emailParams.From unchanged after creation") + if smtpParams.TimeOut == 0 { + assert.Equal(t, defaultEmailTimeout, email.TimeOut, "empty emailParams.TimeOut changed to default") + } else { + assert.Equal(t, smtpParams.TimeOut, email.TimeOut, "emailParams.TimOut unchanged after creation") + } + assert.Equal(t, smtpParams.Host, email.Host, "emailParams.Host unchanged after creation") + assert.Equal(t, smtpParams.Username, email.Username, "emailParams.Username unchanged after creation") + assert.Equal(t, smtpParams.Password, email.Password, "emailParams.Password unchanged after creation") + assert.Equal(t, smtpParams.Port, email.Port, "emailParams.Port unchanged after creation") + assert.Equal(t, smtpParams.TLS, email.TLS, "emailParams.TLS unchanged after creation") +} + +func Test_initTemplates(t *testing.T) { + testSet := []struct{ + name string + errText string emailParams EmailParams - smtpParams SMTPParams }{ - {name: "empty"}, - {name: "with template parse error", - err: true, errText: "can't parse message template: template: messageFromRequest:1: unexpected unclosed action in command", + { + name: "with wrong path to verification template", + errText: "can't read verification template: open notfount.tmpl: no such file or directory", emailParams: EmailParams{ - MsgTemplate: "{{", - }}, - {name: "with verification template parse error", - err: true, errText: "can't parse verification template: template: messageFromRequest:1: unexpected unclosed action in command", - emailParams: EmailParams{ - From: "test@from", - VerificationTemplate: "{{", - }, - smtpParams: SMTPParams{ - Host: "test@host", - Port: 1000, - TLS: true, - Username: "test@username", - Password: "test@password", - TimeOut: time.Second, + VerificationTemplatePath: "notfount.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", }, }, - {name: "normal creation", - err: false, errText: "can't parse verification template: template: messageFromRequest:1: unexpected unclosed action in command", + { + name: "with wrong path to message template", + errText: "can't read message template: open notfount.tmpl: no such file or directory", emailParams: EmailParams{ - From: "test@from", + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "notfount.tmpl", }, - smtpParams: SMTPParams{ - Host: "test@host", - Port: 1000, - TLS: true, - Username: "test@username", - Password: "test@password", - TimeOut: time.Second, + }, + { + name: "with error on read verification template", + errText: "can't parse verification template: template: verifyTmpl:1: unexpected unclosed action in command", + emailParams: EmailParams{ + VerificationTemplatePath: "testdata/bad.html.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", + }, + }, + { + name: "with error on read message template", + errText: "can't parse message template: template: msgTmpl:1: unexpected unclosed action in command", + emailParams: EmailParams{ + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "testdata/bad.html.tmpl", }, }, } + for _, d := range testSet { d := d t.Run(d.name, func(t *testing.T) { - email, err := NewEmail(d.emailParams, d.smtpParams) + e := Email{EmailParams: d.emailParams} + err := e.setTemplates() - if d.err && d.errText == "" { - assert.Error(t, err) - assert.Nil(t, email) - } else if d.err && d.errText != "" { - assert.EqualError(t, err, d.errText) - assert.Nil(t, email) - } else { - assert.NoError(t, err) - assert.NotNil(t, email, "email returned") - - assert.NotNil(t, email.msgTmpl, "e.template is set") - assert.Equal(t, defaultEmailTemplate, email.EmailParams.MsgTemplate, "empty emailParams.MsgTemplate changed to default") - assert.Equal(t, defaultEmailVerificationTemplate, email.EmailParams.VerificationTemplate, "empty emailParams.VerificationTemplate changed to default") - assert.Equal(t, d.emailParams.From, email.EmailParams.From, "emailParams.From unchanged after creation") - if d.smtpParams.TimeOut == 0 { - assert.Equal(t, defaultEmailTimeout, email.TimeOut, "empty emailParams.TimeOut changed to default") - } else { - assert.Equal(t, d.smtpParams.TimeOut, email.TimeOut, "emailParams.TimOut unchanged after creation") - } - assert.Equal(t, d.smtpParams.Host, email.Host, "emailParams.Host unchanged after creation") - assert.Equal(t, d.smtpParams.Username, email.Username, "emailParams.Username unchanged after creation") - assert.Equal(t, d.smtpParams.Password, email.Password, "emailParams.Password unchanged after creation") - assert.Equal(t, d.smtpParams.Port, email.Port, "emailParams.Port unchanged after creation") - assert.Equal(t, d.smtpParams.TLS, email.TLS, "emailParams.TLS unchanged after creation") - } + assert.EqualError(t, err, d.errText) }) } } @@ -103,15 +110,11 @@ func TestEmailSendErrors(t *testing.T) { assert.NoError(t, err) 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) 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) ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -121,12 +124,13 @@ func TestEmailSendErrors(t *testing.T) { e.smtp = &fakeTestSMTP{} 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) } func TestEmailSend_ExitConditions(t *testing.T) { - email, err := NewEmail(EmailParams{}, SMTPParams{}) + email, err := NewEmail(EmailParams{ + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", + }, SMTPParams{}) assert.NoError(t, err) assert.NotNil(t, email, "expecting email returned") // prevent triggering e.autoFlush creation @@ -181,7 +185,11 @@ func TestEmailSendClientError(t *testing.T) { } func TestEmail_Send(t *testing.T) { - email, err := NewEmail(EmailParams{From: "from@example.org"}, SMTPParams{}) + email, err := NewEmail(EmailParams{ + From: "from@example.org", + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", + }, SMTPParams{}) assert.NoError(t, err) assert.NotNil(t, email) fakeSMTP := fakeTestSMTP{} @@ -229,7 +237,11 @@ Date: `) } func TestEmail_SendVerification(t *testing.T) { - email, err := NewEmail(EmailParams{From: "from@example.org"}, SMTPParams{}) + email, err := NewEmail(EmailParams{ + From: "from@example.org", + VerificationTemplatePath: "testdata/verification.html.tmpl", + MsgTemplatePath: "testdata/msg.html.tmpl", + }, SMTPParams{}) assert.NoError(t, err) assert.NotNil(t, email) fakeSMTP := fakeTestSMTP{} @@ -247,7 +259,7 @@ func TestEmail_SendVerification(t *testing.T) { assert.Equal(t, "from@example.org", fakeSMTP.readMail()) assert.Equal(t, 1, fakeSMTP.readQuitCount()) assert.Equal(t, "test@example.org", fakeSMTP.readRcpt()) - // test buildMessageFromRequest separately for message text + // test buildVerificationMessage separately for message text res, err := email.buildVerificationMessage(req.Verification.User, req.Email, req.Verification.Token, req.Verification.SiteID) assert.NoError(t, err) assert.Contains(t, res, `From: from@example.org diff --git a/backend/app/notify/testdata/bad.html.tmpl b/backend/app/notify/testdata/bad.html.tmpl new file mode 100644 index 00000000..e1c0a767 --- /dev/null +++ b/backend/app/notify/testdata/bad.html.tmpl @@ -0,0 +1 @@ +{{ diff --git a/backend/app/notify/testdata/msg.html.tmpl b/backend/app/notify/testdata/msg.html.tmpl new file mode 100644 index 00000000..c1f89bf6 --- /dev/null +++ b/backend/app/notify/testdata/msg.html.tmpl @@ -0,0 +1,20 @@ +{{- if .ForAdmin}} +New comment from {{.UserName}} on your site {{if .PostTitle}} to «{{.PostTitle}}»{{ end }} +{{- else }} + New reply from {{.UserName}} on your comment{{if .PostTitle}} to «{{.PostTitle}}»{{ end }} +{{- end }} +{{- if .ParentCommentText}} + {{.ParentUserPicture}} + {{.ParentUserName}} + {{.ParentCommentDate.Format "02.01.2006 at 15:04"}} + Parent comment link: {{.ParentCommentLink}} + {{.ParentCommentText}} +{{- end }} + +User: {{.UserName}} +{{.CommentDate.Format "02.01.2006 at 15:04"}} +Comment: {{.CommentText}} +{{.Email}} {{if not .ForAdmin}} for {{.ParentUserName}}{{ end }} +{{- if .UnsubscribeLink}} +Unsubscribe link: {{.UnsubscribeLink}} +{{- end }} diff --git a/backend/app/notify/testdata/verification.html.tmpl b/backend/app/notify/testdata/verification.html.tmpl new file mode 100644 index 00000000..eb80d09e --- /dev/null +++ b/backend/app/notify/testdata/verification.html.tmpl @@ -0,0 +1,7 @@ +Confirmation for {{.User}} on site {{.Site}} +{{- if .SubscribeURL}} +Subscribe url: {{.SubscribeURL}}{{.Token}} +{{- end }} +Token:{{.Token}} +Sent to {{.Email}} + diff --git a/backend/app/rest/api/rest.go b/backend/app/rest/api/rest.go index 0f0dbde5..02b9b064 100644 --- a/backend/app/rest/api/rest.go +++ b/backend/app/rest/api/rest.go @@ -31,6 +31,7 @@ import ( "github.com/umputun/remark/backend/app/store" "github.com/umputun/remark/backend/app/store/image" "github.com/umputun/remark/backend/app/store/service" + "github.com/umputun/remark/backend/app/templates" ) // Rest is a rest access server @@ -367,6 +368,7 @@ func (s *Rest) controllerGroups() (public, private, admin, rss) { remarkURL: s.RemarkURL, adminEmail: s.AdminEmail, anonVote: s.AnonVote, + templates: templates.NewFS(), } admGrp := admin{ diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index fa2d3894..a0cd5d82 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -28,6 +28,7 @@ import ( "github.com/umputun/remark/backend/app/store/engine" "github.com/umputun/remark/backend/app/store/image" "github.com/umputun/remark/backend/app/store/service" + "github.com/umputun/remark/backend/app/templates" ) type private struct { @@ -41,6 +42,7 @@ type private struct { remarkURL string adminEmail string anonVote bool + templates templates.FileReader } type privStore interface { @@ -59,21 +61,6 @@ type privStore interface { Info(locator store.Locator, readonlyAge int) (store.PostInfo, error) } -const unsubscribeHTML = ` - - - - - - -
-

Remark42

-

Successfully unsubscribed

-
- - -` - // POST /comment - adds comment, resets all immutable fields func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) { @@ -379,25 +366,25 @@ func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request) func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) { tkn := r.URL.Query().Get("tkn") if tkn == "" { - rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New("missing parameter"), "token parameter is required", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New("missing parameter"), "token parameter is required", rest.ErrInternal, s.templates) return } siteID := r.URL.Query().Get("site") confClaims, err := s.authenticator.TokenService().Parse(tkn) if err != nil { - rest.SendErrorHTML(w, r, http.StatusForbidden, err, "failed to verify confirmation token", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusForbidden, err, "failed to verify confirmation token", rest.ErrInternal, s.templates) return } if s.authenticator.TokenService().IsExpired(confClaims) { - rest.SendErrorHTML(w, r, http.StatusForbidden, errors.New("expired"), "failed to verify confirmation token", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusForbidden, errors.New("expired"), "failed to verify confirmation token", rest.ErrInternal, s.templates) return } elems := strings.Split(confClaims.Handshake.ID, "::") if len(elems) != 2 { - rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New(confClaims.Handshake.ID), "invalid handshake token", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New(confClaims.Handshake.ID), "invalid handshake token", rest.ErrInternal, s.templates) return } userID := elems[0] @@ -408,11 +395,11 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) { log.Printf("[WARN] can't read email for %s, %v", userID, err) } if existingAddress == "" { - rest.SendErrorHTML(w, r, http.StatusConflict, errors.New("user is not subscribed"), "user does not have active email subscription", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusConflict, errors.New("user is not subscribed"), "user does not have active email subscription", rest.ErrInternal, s.templates) return } if address != existingAddress { - rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New("wrong email unsubscription"), "email address in request does not match known for this user", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusBadRequest, errors.New("wrong email unsubscription"), "email address in request does not match known for this user", rest.ErrInternal, s.templates) return } @@ -420,7 +407,7 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) { if err = s.dataService.DeleteUserDetail(siteID, userID, engine.UserEmail); err != nil { code := parseError(err, rest.ErrInternal) - rest.SendErrorHTML(w, r, http.StatusBadRequest, err, "can't delete email for user", code) + rest.SendErrorHTML(w, r, http.StatusBadRequest, err, "can't delete email for user", code, s.templates) return } // clean User.Email from the token, if user has the token @@ -431,7 +418,7 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) { if claims.User != nil && claims.User.Email != "" { claims.User.Email = "" if _, err = s.authenticator.TokenService().Set(w, claims); err != nil { - rest.SendErrorHTML(w, r, http.StatusInternalServerError, err, "failed to set token", rest.ErrInternal) + rest.SendErrorHTML(w, r, http.StatusInternalServerError, err, "failed to set token", rest.ErrInternal, s.templates) return } } @@ -442,8 +429,15 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) { panic(err) } } - - tmpl := template.Must(template.New("unsubscribe").Parse(unsubscribeHTML)) + MustRead := func(path string) string { + file, err := s.templates.ReadFile(path) + if err != nil { + panic(err) + } + return string(file) + } + tmplstr := MustRead("unsubscribe.html.tmpl") + tmpl := template.Must(template.New("unsubscribe").Parse(tmplstr)) msg := bytes.Buffer{} MustExecute(tmpl, &msg, nil) render.HTML(w, r, msg.String()) diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 31e95cf6..a8be4cd5 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -120,7 +120,7 @@ func TestRest_CreateWithRestrictedWord(t *testing.T) { ts, _, teardown := startupT(t) defer teardown() - badComment := fmt.Sprintf(`{"text": "What the duck is that?", "locator":{"url": "https://radio-t.com/blah1", + badComment := fmt.Sprintf(`{"text": "What the duck is that?", "locator":{"url": "https://radio-t.com/blah1", "site": "remark42"}}`) resp, err := post(t, ts.URL+"/api/v1/comment", badComment) @@ -503,10 +503,17 @@ func TestRest_AnonVote(t *testing.T) { assert.Equal(t, map[string]bool(nil), cr.Votes) } +type MockFS struct {} +func (fs *MockFS) ReadFile(path string) ([]byte, error) { + return []byte(fmt.Sprintf("template %s", path)), nil +} + func TestRest_Email(t *testing.T) { ts, srv, teardown := startupT(t) defer teardown() + srv.privRest.templates = &MockFS{} + // issue good token claims := token.Claims{ Handshake: &token.Handshake{ID: "dev::good@example.com"}, diff --git a/backend/app/rest/httperrors.go b/backend/app/rest/httperrors.go index cc487b2a..c174fe0b 100644 --- a/backend/app/rest/httperrors.go +++ b/backend/app/rest/httperrors.go @@ -13,6 +13,8 @@ import ( "github.com/go-chi/render" log "github.com/go-pkgz/lgr" "github.com/go-pkgz/rest" + + "github.com/umputun/remark/backend/app/templates" ) // All error codes for UI mapping and translation @@ -38,21 +40,6 @@ const ( ErrAssetNotFound = 18 // requested file not found ) -const errorHTML = ` - - - - - - -
-

Remark42

-

{{.Error}}: {{.Details}}.

-
- - -` - // errTmplData store data for error message type errTmplData struct { Error string @@ -61,15 +48,22 @@ type errTmplData struct { // SendErrorHTML makes html body with provided template and responds with provided http status code, // error code is not included in render as it is intended for UI developers and not for the users -func SendErrorHTML(w http.ResponseWriter, r *http.Request, httpStatusCode int, err error, details string, errCode int) { +func SendErrorHTML(w http.ResponseWriter, r *http.Request, httpStatusCode int, err error, details string, errCode int, t templates.FileReader) { // MustExecute behaves like template.Execute, but panics if an error occurs. MustExecute := func(tmpl *template.Template, wr io.Writer, data interface{}) { if err = tmpl.Execute(wr, data); err != nil { panic(err) } } - - tmpl := template.Must(template.New("error").Parse(errorHTML)) + MustRead := func(path string) string { + file, e := t.ReadFile(path) + if e != nil { + panic(e) + } + return string(file) + } + tmplstr := MustRead("error_response.html.tmpl") + tmpl := template.Must(template.New("error").Parse(tmplstr)) log.Printf("[WARN] %s", errDetailsMsg(r, httpStatusCode, err, details, errCode)) render.Status(r, httpStatusCode) msg := bytes.Buffer{} diff --git a/backend/app/rest/httperrors_test.go b/backend/app/rest/httperrors_test.go index 59eb3c78..960a975c 100644 --- a/backend/app/rest/httperrors_test.go +++ b/backend/app/rest/httperrors_test.go @@ -2,6 +2,7 @@ package rest import ( "errors" + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -37,12 +38,18 @@ func TestSendErrorJSON(t *testing.T) { assert.Equal(t, `{"code":123,"details":"error details 123456","error":"error 500"}`+"\n", string(body)) } -func TestSendErrorHTML(t *testing.T) { +type MockFS struct {} +func (fs *MockFS) ReadFile(path string) ([]byte, error) { + return []byte(fmt.Sprintf("{{.Error}}{{.Details}} %s", path)), nil +} + +func TestSendErrorHTML(t *testing.T) { + fs := &MockFS{} ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/error" { t.Log("http err request", r.URL) - SendErrorHTML(w, r, 500, errors.New("error 500"), "error details 123456", 987) + SendErrorHTML(w, r, 500, errors.New("error 500"), "error details 123456", 987, fs) return } w.WriteHeader(404) diff --git a/backend/app/templates/templates.go b/backend/app/templates/templates.go new file mode 100644 index 00000000..4185b770 --- /dev/null +++ b/backend/app/templates/templates.go @@ -0,0 +1,38 @@ +package templates + +import ( + "io/ioutil" + "net/http" + "path/filepath" + + log "github.com/go-pkgz/lgr" + "github.com/rakyll/statik/fs" +) + +// FS stores link to statikFS if it exists +type FS struct { + statik http.FileSystem +} + +// FileReader describes methods of filesystem +type FileReader interface { + ReadFile(path string) ([]byte, error) +} + +// NewFS returns new FS instance, which will read from statik if it's available and from fs otherwise +func NewFS() *FS { + f := &FS{} + if statikFS, err := fs.NewWithNamespace("templates"); err == nil { + log.Printf("[INFO] templates will be read from statik") + f.statik = statikFS + } + return f +} + +// ReadFile depends on statik achieve exists +func (f *FS) ReadFile(path string) ([]byte, error) { + if f.statik != nil { + return fs.ReadFile(f.statik, filepath.Join("/", path)) + } + return ioutil.ReadFile(filepath.Join("./",filepath.Clean(path))) +} diff --git a/backend/app/templates/templates_test.go b/backend/app/templates/templates_test.go new file mode 100644 index 00000000..3ccd968e --- /dev/null +++ b/backend/app/templates/templates_test.go @@ -0,0 +1,24 @@ +package templates + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewFS(t *testing.T) { + fs := NewFS() + assert.NotNil(t, &fs) +} + +func TestFS_ReadFile(t *testing.T) { + fs := NewFS() + + file, err := fs.ReadFile("testdata/template.html.tmpl") + assert.NoError(t, err) + assert.Equal(t, []byte("template\n"), file) + + file, err = fs.ReadFile("testdata/bad_path.html.tmpl") + assert.Error(t, err) + assert.Nil(t, file) +} diff --git a/backend/app/templates/testdata/template.html.tmpl b/backend/app/templates/testdata/template.html.tmpl new file mode 100644 index 00000000..3236cba6 --- /dev/null +++ b/backend/app/templates/testdata/template.html.tmpl @@ -0,0 +1 @@ +template diff --git a/backend/templates/email_confirmation_login.html.tmpl b/backend/templates/email_confirmation_login.html.tmpl new file mode 100644 index 00000000..b5fd6b75 --- /dev/null +++ b/backend/templates/email_confirmation_login.html.tmpl @@ -0,0 +1,19 @@ + + + + + + + +
+

Remark42

+

Confirmation for {{.User}} on site {{.Site}}

+
+

TOKEN

+

Copy and paste this text into “token” field on comments page

+

{{.Token}}

+
+

Sent to {{.Address}}

+
+ + diff --git a/backend/templates/email_confirmation_subscription.html.tmpl b/backend/templates/email_confirmation_subscription.html.tmpl new file mode 100644 index 00000000..3c34c7e7 --- /dev/null +++ b/backend/templates/email_confirmation_subscription.html.tmpl @@ -0,0 +1,24 @@ + + + + + + + + +
+

Remark42

+

Confirmation for {{.User}} on site {{.Site}}

+ {{- if .SubscribeURL}} +

Click here to subscribe to email notifications

+

Alternatively, you can use code below for subscription.

+ {{- end }} +
+

TOKEN

+

Copy and paste this text into “token” field on comments page

+

{{.Token}}

+
+

Sent to {{.Email}}

+
+ + diff --git a/backend/templates/email_reply.html.tmpl b/backend/templates/email_reply.html.tmpl new file mode 100644 index 00000000..6755e84f --- /dev/null +++ b/backend/templates/email_reply.html.tmpl @@ -0,0 +1,68 @@ + + + + + + + + + +
+

Remark42

+ {{- if .ForAdmin}} +
New comment from {{.UserName}} on your site {{if .PostTitle}} to «{{.PostTitle}}»{{ end }}
+ {{- else }} +
New reply from {{.UserName}} on your comment{{if .PostTitle}} to «{{.PostTitle}}»{{ end }}
+ {{- end }} +
+ {{- if .ParentCommentText}} +
+ + {{.ParentUserName}} + {{.ParentCommentDate.Format "02.01.2006 at 15:04"}} + Show +
+
{{.ParentCommentText}}
+ {{- end }} +
+
+ + {{.UserName}} + {{.CommentDate.Format "02.01.2006 at 15:04"}} + Reply +
+
{{.CommentText}}
+
+
+
+ Sent to {{.Email}}{{if not .ForAdmin}} for {{.ParentUserName}}{{ end }} +
+ {{- if .UnsubscribeLink}} + Unsubscribe + {{- end }} + +
[{{.CommentDate.Format "02.01.2006 at 15:04"}}]
+
+
+ + diff --git a/backend/templates/email_unsubscribe.html.tmpl b/backend/templates/email_unsubscribe.html.tmpl new file mode 100644 index 00000000..f7ca9881 --- /dev/null +++ b/backend/templates/email_unsubscribe.html.tmpl @@ -0,0 +1,13 @@ + + + + + + + +
+

Remark42

+

Successfully unsubscribed

+
+ + diff --git a/backend/templates/error_response.html.tmpl b/backend/templates/error_response.html.tmpl new file mode 100644 index 00000000..57245efe --- /dev/null +++ b/backend/templates/error_response.html.tmpl @@ -0,0 +1,13 @@ + + + + + + + +
+

Remark42

+

{{.Error}}: {{.Details}}.

+
+ + diff --git a/docs/developers-guide/email-templates.md b/docs/developers-guide/email-templates.md new file mode 100644 index 00000000..77968948 --- /dev/null +++ b/docs/developers-guide/email-templates.md @@ -0,0 +1,13 @@ +### Email templating + +We use golang templates for email templating. +templates located in `backend/templates` and embeded into binary by statik + +For getting access to files you can use package `templates` from `backend/app/templates` + +Now we have following templates: +- `email_confirmation_login.html.tmpl` – used for confirmation of login +- `email_confirmation_subscription.html.tmpl` – used for confirmation of subscription +– `email_reply.html.tmpl` – used for sending replies to user comments (when user subscribed to it) and for noticing admins about new comments on a site +– `email_unsubscribe.html.tmpl` – used for notification about successful unsubscribe from replies +– `error_response.html.tmpl` – used for ...