mirrors/remark42
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move email unsubscription endpoint outside of API and make it HTML (#500)

Browse Source 
* move email unsubscription page outside of API and make it HTML

* make separate HTML template for SendErrorHTML

* fix error template name

* add test for SendErrorHTML, introduce MustExecute function

* fix content check in test of TestSendErrorHTML

* fix logging test to be more generic and not depend on line numbers
This commit is contained in:
Dmitry Verkhoturov
2019-12-27 01:04:57 -06:00
committed by Umputun
parent b055c61be7
commit 9df2b2a9ed
6 changed files with 124 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/cmd/server.go
+1 -1
View File
@@ -702,7 +702,7 @@ func (s *ServerCommand) makeNotify(dataStore *service.DataStore, authenticator *
emailParams := notify.EmailParams{
From: s.Notify.Email.From,
VerificationSubject: s.Notify.Email.VerificationSubject,
UnsubscribeURL: s.RemarkURL + "/api/v1/email/unsubscribe",
UnsubscribeURL: s.RemarkURL + "/email/unsubscribe.html",
TokenGenFn: func(userID, email, site string) (string, error) {
claims := token.Claims{
Handshake: &token.Handshake{ID: userID + "::" + email},
backend/app/rest/api/rest.go
+2 -2
View File
@@ -236,8 +236,6 @@ func (s *Rest) routes() chi.Router {
ropen.Get("/list", s.pubRest.listCtrl)
ropen.Post("/preview", s.pubRest.previewCommentCtrl)
ropen.Get("/info", s.pubRest.infoCtrl)
ropen.Get("/email/unsubscribe", s.privRest.emailUnsubscribeCtrl)
ropen.Post("/email/unsubscribe", s.privRest.emailUnsubscribeCtrl)
ropen.Get("/img", s.ImageProxy.Handler)
ropen.Route("/rss", func(rrss chi.Router) {
@@ -334,6 +332,8 @@ func (s *Rest) routes() chi.Router {
rroot.Use(tollbooth_chi.LimitHandler(tollbooth.NewLimiter(50, nil)))
rroot.Get("/index.html", s.pubRest.getStartedCtrl)
rroot.Get("/robots.txt", s.pubRest.robotsCtrl)
rroot.Get("/email/unsubscribe.html", s.privRest.emailUnsubscribeCtrl)
rroot.Post("/email/unsubscribe.html", s.privRest.emailUnsubscribeCtrl)
})
// file server for static content from /web
backend/app/rest/api/rest_private.go
+39 -10
View File
@@ -1,10 +1,13 @@
package api
import (
"bytes"
"compress/gzip"
"encoding/json"
"errors"
"fmt"
"html/template"
"io"
"net/http"
"strings"
"time"
@@ -55,6 +58,21 @@ type privStore interface {
Info(locator store.Locator, readonlyAge int) (store.PostInfo, error)
}
const unsubscribeHtml = `<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width"/>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
</head>
<body>
<div style="text-align: center; font-family: Arial, sans-serif; font-size: 18px;">
<h1 style="position: relative; color: #4fbbd6; margin-top: 0.2em;">Remark42</h1>
<p style="position: relative; max-width: 20em; margin: 0 auto 1em auto; line-height: 1.4em;">Successfully unsubscribed</p>
</div>
</body>
</html>
`
// POST /comment - adds comment, resets all immutable fields
func (s *private) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
@@ -343,29 +361,29 @@ func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request)
render.JSON(w, r, R.JSON{"updated": true, "address": val})
}
// POST/GET /email/unsubscribe?site=siteID&tkn=jwt - unsubscribe the user in token from email notifications
// POST/GET /email/unsubscribe.html?site=siteID&tkn=jwt - unsubscribe the user in token from email notifications
func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) {
tkn := r.URL.Query().Get("tkn")
if tkn == "" {
rest.SendErrorJSON(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)
return
}
locator := store.Locator{SiteID: r.URL.Query().Get("site")}
confClaims, err := s.authenticator.TokenService().Parse(tkn)
if err != nil {
rest.SendErrorJSON(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)
return
}
if s.authenticator.TokenService().IsExpired(confClaims) {
rest.SendErrorJSON(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)
return
}
elems := strings.Split(confClaims.Handshake.ID, "::")
if len(elems) != 2 {
rest.SendErrorJSON(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)
return
}
userID := elems[0]
@@ -376,11 +394,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.SendErrorJSON(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)
return
}
if address != existingAddress {
rest.SendErrorJSON(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)
return
}
@@ -388,7 +406,7 @@ func (s *private) emailUnsubscribeCtrl(w http.ResponseWriter, r *http.Request) {
if err := s.dataService.DeleteUserDetail(locator, userID, engine.UserEmail); err != nil {
code := parseError(err, rest.ErrInternal)
rest.SendErrorJSON(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)
return
}
// clean User.Email from the token, if user has the token
@@ -399,11 +417,22 @@ 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.SendErrorJSON(w, r, http.StatusInternalServerError, err, "failed to set token", rest.ErrInternal)
rest.SendErrorHTML(w, r, http.StatusInternalServerError, err, "failed to set token", rest.ErrInternal)
return
}
}
render.JSON(w, r, R.JSON{"unsubscribed": true})
// 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("unsubscribe").Parse(unsubscribeHtml))
msg := bytes.Buffer{}
MustExecute(tmpl, &msg, nil)
render.HTML(w, r, msg.String())
}
// DELETE /email?site=siteID - removes user's email
backend/app/rest/api/rest_private_test.go
+4 -4
View File
@@ -543,10 +543,10 @@ func TestRest_Email(t *testing.T) {
{description: "delete user email", url: "/api/v1/email?site=remark42", method: http.MethodDelete, responseCode: http.StatusOK},
{description: "send another confirmation", url: "/api/v1/email/subscribe?site=remark42&address=good@example.com", method: http.MethodPost, responseCode: http.StatusOK},
{description: "set user email, token is good", url: fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusOK, cookieEmail: "good@example.com"},
{description: "unsubscribe user, no token", url: "/api/v1/email/unsubscribe?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest},
{description: "unsubscribe user, wrong token", url: "/api/v1/email/unsubscribe?site=remark42&tkn=jwt", method: http.MethodPost, responseCode: http.StatusForbidden},
{description: "unsubscribe user, good token", url: fmt.Sprintf("/api/v1/email/unsubscribe?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusOK},
{description: "unsubscribe user second time, good token", url: fmt.Sprintf("/api/v1/email/unsubscribe?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusConflict},
{description: "unsubscribe user, no token", url: "/email/unsubscribe.html?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest},
{description: "unsubscribe user, wrong token", url: "/email/unsubscribe.html?site=remark42&tkn=jwt", method: http.MethodGet, responseCode: http.StatusForbidden},
{description: "unsubscribe user, good token", url: fmt.Sprintf("/email/unsubscribe.html?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusOK},
{description: "unsubscribe user second time, good token", url: fmt.Sprintf("/email/unsubscribe.html?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusConflict},
}
client := http.Client{}
for _, x := range testData {
backend/app/rest/httperrors.go
+45
View File
@@ -1,7 +1,10 @@
package rest
import (
"bytes"
"fmt"
"html/template"
"io"
"net/http"
"net/url"
"runtime"
@@ -35,6 +38,48 @@ const (
ErrAssetNotFound = 18 // requested file not found
)
const errorHtml = `<!DOCTYPE html>
<html>
<head>
<meta name="viewport" content="width=device-width"/>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
</head>
<body>
<div style="text-align: center; font-family: Arial, sans-serif; font-size: 18px;">
<h1 style="position: relative; color: #4fbbd6; margin-top: 0.2em;">Remark42</h1>
<p style="position: relative; max-width: 20em; margin: 0 auto 1em auto; line-height: 1.4em;">{{.Error}}: {{.Details}}.</p>
</div>
</body>
</html>
`
// errTmplData store data for error message
type errTmplData struct {
Error string
Details string
}
// 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) {
// 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))
log.Printf("[WARN] %s", errDetailsMsg(r, httpStatusCode, err, details, errCode))
render.Status(r, httpStatusCode)
msg := bytes.Buffer{}
MustExecute(tmpl, &msg, errTmplData{
Error: err.Error(),
Details: details,
})
render.HTML(w, r, msg.String())
}
// SendErrorJSON makes {error: blah, details: blah} json body and responds with error code
func SendErrorJSON(w http.ResponseWriter, r *http.Request, httpStatusCode int, err error, details string, errCode int) {
log.Printf("[WARN] %s", errDetailsMsg(r, httpStatusCode, err, details, errCode))
backend/app/rest/httperrors_test.go
+33 -3
View File
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/umputun/remark/backend/app/store"
)
@@ -36,13 +37,41 @@ 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) {
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)
return
}
w.WriteHeader(404)
}))
defer ts.Close()
resp, err := http.Get(ts.URL + "/error")
require.Nil(t, err)
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
require.Nil(t, err)
assert.Equal(t, 500, resp.StatusCode)
assert.NotContains(t, string(body), `987`, "user html should not contain internal error code")
assert.Contains(t, string(body), `error details 123456`)
assert.Contains(t, string(body), `error 500`)
}
func TestErrorDetailsMsg(t *testing.T) {
callerFn := func() {
req, err := http.NewRequest("GET", "https://example.com/test?k1=v1&k2=v2", nil)
require.Nil(t, err)
req.RemoteAddr = "1.2.3.4"
msg := errDetailsMsg(req, 500, errors.New("error 500"), "error details 123456", 123)
assert.Equal(t, "error details 123456 - error 500 - 500 (123) - https://example.com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:47 rest.TestErrorDetailsMsg]", msg)
assert.Contains(t, msg, "error details 123456 - error 500 - 500 (123) - https://example.com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:")
// error line in the middle of the message is not checked
assert.Contains(t, msg, " rest.TestErrorDetailsMsg]")
}
callerFn()
}
@@ -55,8 +84,9 @@ func TestErrorDetailsMsgWithUser(t *testing.T) {
req = SetUserInfo(req, store.User{Name: "test", ID: "id"})
require.Nil(t, err)
msg := errDetailsMsg(req, 500, errors.New("error 500"), "error details 123456", 34567)
assert.Equal(t, "error details 123456 - error 500 - 500 (34567) - test/id - https://example."+
"com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:61 rest.TestErrorDetailsMsgWithUser]", msg)
assert.Contains(t, msg, "error details 123456 - error 500 - 500 (34567) - test/id - https://example.com/test?k1=v1&k2=v2 - [app/rest/httperrors_test.go:")
// error line in the middle of the message is not checked
assert.Contains(t, msg, " rest.TestErrorDetailsMsgWithUser]")
}
callerFn()
}