From 27fc339e36834b98b05c6d1e1506c23bcd1abf37 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Sun, 8 Jan 2023 22:40:34 +0100 Subject: [PATCH] use the request body for email subscription endpoints Previously, the endpoints were using query parameters. After this change, the body is tried to be parsed. --- backend/app/rest/api/rest_private.go | 64 ++++++++++++++++------- backend/app/rest/api/rest_private_test.go | 39 ++++++++++---- backend/remark.rest | 17 +++++- 3 files changed, 90 insertions(+), 30 deletions(-) diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index dae03d19..4a8cd27b 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -187,7 +187,7 @@ func (s *private) updateCommentCtrl(w http.ResponseWriter, r *http.Request) { }{} if err := render.DecodeJSON(http.MaxBytesReader(w, r.Body, hardBodyLimit), &edit); err != nil { - rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't bind comment", rest.ErrDecode) + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't read comment details from body", rest.ErrDecode) return } @@ -307,28 +307,42 @@ func (s *private) getEmailCtrl(w http.ResponseWriter, r *http.Request) { } // sendEmailConfirmationCtrl gets address and siteID from query, makes confirmation token and sends it to user. -// GET /email/subscribe?site=siteID&address=someone@example.com +// POST /email/subscribe with site and address in json body +// //nolint:dupl // too hard to deduplicate that logic, as then it's tricky to use SendErrorJSON func (s *private) sendEmailConfirmationCtrl(w http.ResponseWriter, r *http.Request) { user := rest.MustGetUserInfo(r) - address := r.URL.Query().Get("address") - siteID := r.URL.Query().Get("site") - if address == "" { + + subscribe := struct { + Site string + Address string + }{} + if err := render.DecodeJSON(http.MaxBytesReader(w, r.Body, hardBodyLimit), &subscribe); err != nil { + if err != io.EOF { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't parse request body", rest.ErrDecode) + return + } + // old behavior fallback, reading from the query params + subscribe.Address = r.URL.Query().Get("address") + subscribe.Site = r.URL.Query().Get("site") + } + + if subscribe.Address == "" { rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("missing parameter"), "address parameter is required", rest.ErrInternal) return } - existingAddress, err := s.dataService.GetUserEmail(siteID, user.ID) + existingAddress, err := s.dataService.GetUserEmail(subscribe.Site, user.ID) if err != nil { log.Printf("[WARN] can't read email for %s, %v", user.ID, err) } - if address == existingAddress { + if subscribe.Address == existingAddress { rest.SendErrorJSON(w, r, http.StatusConflict, fmt.Errorf("already verified"), "email address is already verified for this user", rest.ErrInternal) return } claims := token.Claims{ - Handshake: &token.Handshake{ID: user.ID + "::" + address}, + Handshake: &token.Handshake{ID: user.ID + "::" + subscribe.Address}, StandardClaims: jwt.StandardClaims{ Audience: r.URL.Query().Get("site"), ExpiresAt: time.Now().Add(30 * time.Minute).Unix(), @@ -345,14 +359,14 @@ func (s *private) sendEmailConfirmationCtrl(w http.ResponseWriter, r *http.Reque s.notifyService.SubmitVerification( notify.VerificationRequest{ - SiteID: siteID, + SiteID: subscribe.Site, User: user.Name, - Email: address, + Email: subscribe.Address, Token: tkn, }, ) - render.JSON(w, r, R.JSON{"user": user, "address": address}) + render.JSON(w, r, R.JSON{"user": user, "address": subscribe.Address}) } // telegramSubscribeCtrl generates and verifies telegram notification request @@ -416,17 +430,29 @@ func (s *private) telegramSubscribeCtrl(w http.ResponseWriter, r *http.Request) } // setConfirmedEmailCtrl uses provided token parameter (generated by sendEmailConfirmationCtrl) to set email and add it to user token -// PUT /email/confirm?site=siteID&tkn=jwt +// POST /email/confirm with site and token in json body func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request) { - tkn := r.URL.Query().Get("tkn") - if tkn == "" { + user := rest.MustGetUserInfo(r) + + confirm := struct { + Site string + Token string + }{} + if err := render.DecodeJSON(http.MaxBytesReader(w, r.Body, hardBodyLimit), &confirm); err != nil { + if err != io.EOF { + rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't parse request body", rest.ErrDecode) + return + } + // old behavior fallback, reading from the query params + confirm.Token = r.URL.Query().Get("tkn") + confirm.Site = r.URL.Query().Get("site") + } + + if confirm.Token == "" { rest.SendErrorJSON(w, r, http.StatusBadRequest, fmt.Errorf("missing parameter"), "token parameter is required", rest.ErrInternal) return } - user := rest.MustGetUserInfo(r) - siteID := r.URL.Query().Get("site") - - confClaims, err := s.authenticator.TokenService().Parse(tkn) + confClaims, err := s.authenticator.TokenService().Parse(confirm.Token) if err != nil { rest.SendErrorJSON(w, r, http.StatusForbidden, err, "failed to verify confirmation token", rest.ErrInternal) return @@ -447,7 +473,7 @@ func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request) log.Printf("[DEBUG] set email for user %s", user.ID) - val, err := s.dataService.SetUserEmail(siteID, user.ID, address) + val, err := s.dataService.SetUserEmail(confirm.Site, user.ID, address) if err != nil { code := parseError(err, rest.ErrInternal) rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't set email for user", code) diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index d3938338..5cb66d46 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -728,19 +728,25 @@ func TestRest_EmailAndTelegram(t *testing.T) { responseCode int noAuth bool cookieEmail string + body string }{ {description: "issue delete request without auth", url: "/api/v1/email", method: http.MethodDelete, responseCode: http.StatusUnauthorized, noAuth: true}, {description: "issue delete request without site_id", url: "/api/v1/email", method: http.MethodDelete, responseCode: http.StatusBadRequest}, {description: "delete non-existent user email", url: "/api/v1/email?site=remark42", method: http.MethodDelete, responseCode: http.StatusOK}, - {description: "set user email, token not set", url: "/api/v1/email/confirm?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest}, - {description: "send email confirmation without address", url: "/api/v1/email/subscribe?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest}, - {description: "send email 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: "set user email, token not set", url: "/api/v1/email/confirm", method: http.MethodPost, responseCode: http.StatusBadRequest, body: `{"site":"remark42"}`}, + {description: "set user email, token not set, old query param", url: "/api/v1/email/confirm?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest}, + {description: "send email confirmation without address", url: "/api/v1/email/subscribe", method: http.MethodPost, responseCode: http.StatusBadRequest, body: `{"site":"remark42"}`}, + {description: "send email confirmation without address, old query param", url: "/api/v1/email/subscribe?site=remark42", method: http.MethodPost, responseCode: http.StatusBadRequest}, + {description: "send email confirmation", url: "/api/v1/email/subscribe", method: http.MethodPost, responseCode: http.StatusOK, body: `{"site":"remark42","address":"good@example.com"}`}, + {description: "send email confirmation, old query param", 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: "/api/v1/email/confirm", method: http.MethodPost, responseCode: http.StatusOK, cookieEmail: "good@example.com", body: fmt.Sprintf(`{"site":"remark42","token":%q}`, goodToken)}, + {description: "set user email, token is good, old query param", url: fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", goodToken), method: http.MethodPost, responseCode: http.StatusOK, cookieEmail: "good@example.com"}, {description: "send confirmation with same address", url: "/api/v1/email/subscribe?site=remark42&address=good@example.com", method: http.MethodPost, responseCode: http.StatusConflict}, {description: "get user email", url: "/api/v1/email?site=remark42", method: http.MethodGet, responseCode: http.StatusOK}, {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: "set user email, token is good", url: "/api/v1/email/confirm", method: http.MethodPost, responseCode: http.StatusOK, cookieEmail: "good@example.com", body: fmt.Sprintf(`{"site":"remark42","token":%q}`, goodToken)}, + {description: "set user email, token is good, old query param", 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: "/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}, @@ -761,7 +767,11 @@ func TestRest_EmailAndTelegram(t *testing.T) { for _, x := range testData { x := x t.Run(x.description, func(t *testing.T) { - req, err := http.NewRequest(x.method, ts.URL+x.url, http.NoBody) + reqBody := io.NopCloser(strings.NewReader(x.body)) + if x.body == "" { + reqBody = http.NoBody + } + req, err := http.NewRequest(x.method, ts.URL+x.url, reqBody) require.NoError(t, err) if !x.noAuth { req.Header.Add("X-JWT", devToken) @@ -837,7 +847,11 @@ func TestRest_EmailNotification(t *testing.T) { assert.Empty(t, mockDestination.Get()[1].Emails) // send confirmation token for email - req, err = http.NewRequest(http.MethodPost, ts.URL+"/api/v1/email/subscribe?site=remark42&address=good@example.com", http.NoBody) + req, err = http.NewRequest( + http.MethodPost, + ts.URL+"/api/v1/email/subscribe?site=remark42&address=", + io.NopCloser(strings.NewReader(`{"site": "remark42", "address": "good@example.com"}`)), + ) require.NoError(t, err) req.Header.Add("X-JWT", devToken) resp, err = client.Do(req) @@ -853,7 +867,11 @@ func TestRest_EmailNotification(t *testing.T) { verificationToken := mockDestination.GetVerify()[0].Token // verify email - req, err = http.NewRequest(http.MethodPost, ts.URL+fmt.Sprintf("/api/v1/email/confirm?site=remark42&tkn=%s", verificationToken), http.NoBody) + req, err = http.NewRequest( + http.MethodPost, + ts.URL+"/api/v1/email/confirm", + io.NopCloser(strings.NewReader(fmt.Sprintf(`{"site": "remark42", "token": %q}`, verificationToken))), + ) require.NoError(t, err) req.Header.Add("X-JWT", devToken) resp, err = client.Do(req) @@ -864,7 +882,10 @@ func TestRest_EmailNotification(t *testing.T) { require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) // get user information to verify the subscription - req, err = http.NewRequest(http.MethodGet, ts.URL+"/api/v1/user?site=remark42", http.NoBody) + req, err = http.NewRequest( + http.MethodGet, + ts.URL+"/api/v1/user?site=remark42", + http.NoBody) require.NoError(t, err) req.Header.Add("X-JWT", devToken) resp, err = client.Do(req) diff --git a/backend/remark.rest b/backend/remark.rest index 6fe12d37..eca6c6d1 100644 --- a/backend/remark.rest +++ b/backend/remark.rest @@ -119,12 +119,25 @@ GET {{host}}/api/v1/rss/reply?site={{site}}&user={{user}} GET {{host}}/api/v1/avatar/blah ### send confirmation token for current user to specified email. auth token for dev user for secret=12345. -POST {{host}}/api/v1/email/subscribe?site={{site}}&address={{email}} +POST {{host}}/api/v1/email/subscribe X-JWT: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcmsiLCJleHAiOjE5NzYwNTY3NTYsImp0aSI6IjJlOGJmMTE5OTI0MjQxMDRjYjFhZGRlODllMWYwNGFiMTg4YWZjMzQiLCJpYXQiOjE1NzYwNTY0NTYsImlzcyI6InJlbWFyazQyIiwidXNlciI6eyJuYW1lIjoiZGV2X3VzZXIiLCJpZCI6ImRldl91c2VyIiwicGljdHVyZSI6Imh0dHA6Ly8xMjcuMC4wLjE6ODA4MC9hcGkvdjEvYXZhdGFyL2NjZmEyYWJkMDE2Njc2MDViNGUxZmM0ZmNiOTFiMWUxYWYzMjMyNDAuaW1hZ2UiLCJhdHRycyI6eyJhZG1pbiI6dHJ1ZSwiYmxvY2tlZCI6ZmFsc2V9fX0.6Qt5s2enBMRC-Jmsua01yViVYI95Dx6BPBMaNjj36d4 +Content-Type: application/json + +{ + "site": "{{site}}", + "address": "{{email}}" +} + ### add email for notifications for current user via token from email. auth token for dev user for secret=12345. -POST {{host}}/api/v1/email/confirm?site={{site}}&tkn={{token}} +POST {{host}}/api/v1/email/confirm X-JWT: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcmsiLCJleHAiOjE5NzYwNTY3NTYsImp0aSI6IjJlOGJmMTE5OTI0MjQxMDRjYjFhZGRlODllMWYwNGFiMTg4YWZjMzQiLCJpYXQiOjE1NzYwNTY0NTYsImlzcyI6InJlbWFyazQyIiwidXNlciI6eyJuYW1lIjoiZGV2X3VzZXIiLCJpZCI6ImRldl91c2VyIiwicGljdHVyZSI6Imh0dHA6Ly8xMjcuMC4wLjE6ODA4MC9hcGkvdjEvYXZhdGFyL2NjZmEyYWJkMDE2Njc2MDViNGUxZmM0ZmNiOTFiMWUxYWYzMjMyNDAuaW1hZ2UiLCJhdHRycyI6eyJhZG1pbiI6dHJ1ZSwiYmxvY2tlZCI6ZmFsc2V9fX0.6Qt5s2enBMRC-Jmsua01yViVYI95Dx6BPBMaNjj36d4 +Content-Type: application/json + +{ + "site": "{{site}}", + "token": "{{token}}" +} ### get current user email. auth token for dev user for secret=12345. GET {{host}}/api/v1/email?site={{site}}