diff --git a/backend/app/rest/api/rest_private.go b/backend/app/rest/api/rest_private.go index 4a8cd27b..f1930232 100644 --- a/backend/app/rest/api/rest_private.go +++ b/backend/app/rest/api/rest_private.go @@ -307,6 +307,8 @@ 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. +// In case user is logged in with the same email, and auto_confirm is true, confirm it right away. +// In case of quick confirmation, "updated" is set to true, otherwise - to false. // 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 @@ -314,17 +316,19 @@ func (s *private) sendEmailConfirmationCtrl(w http.ResponseWriter, r *http.Reque user := rest.MustGetUserInfo(r) subscribe := struct { - Site string - Address string - }{} + Site string + Address string + autoConfirm bool + }{autoConfirm: true} 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 + // old behavior fallback, reading from the query params. Auto confirm is false in this case. subscribe.Address = r.URL.Query().Get("address") subscribe.Site = r.URL.Query().Get("site") + subscribe.autoConfirm = false } if subscribe.Address == "" { @@ -332,15 +336,28 @@ func (s *private) sendEmailConfirmationCtrl(w http.ResponseWriter, r *http.Reque fmt.Errorf("missing parameter"), "address parameter is required", rest.ErrInternal) return } - 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) + existingAddress, getErr := s.dataService.GetUserEmail(subscribe.Site, user.ID) + if getErr != nil { + log.Printf("[WARN] can't read email for %s, %v", user.ID, getErr) } 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 } + + // in case the user logged in with the same email as they try to subscribe with, confirm it right away + // this behavior is different from the previous one and is hidden behind the autoConfirm flag, + // which is true for the new API, and false for the old one + // + // nolint:gosec // this is not used for security purposes + if subscribe.autoConfirm && + strings.HasPrefix(user.ID, "email_") && + strings.TrimPrefix(user.ID, "email_") == token.HashID(sha1.New(), subscribe.Address) { + s.setEmail(w, r, user.ID, subscribe.Site, subscribe.Address) + return + } + claims := token.Claims{ Handshake: &token.Handshake{ID: user.ID + "::" + subscribe.Address}, StandardClaims: jwt.StandardClaims{ @@ -366,7 +383,7 @@ func (s *private) sendEmailConfirmationCtrl(w http.ResponseWriter, r *http.Reque }, ) - render.JSON(w, r, R.JSON{"user": user, "address": subscribe.Address}) + render.JSON(w, r, R.JSON{"user": user, "address": subscribe.Address, "updated": false}) } // telegramSubscribeCtrl generates and verifies telegram notification request @@ -470,17 +487,20 @@ func (s *private) setConfirmedEmailCtrl(w http.ResponseWriter, r *http.Request) return } address := elems[1] + s.setEmail(w, r, user.ID, confirm.Site, address) +} - log.Printf("[DEBUG] set email for user %s", user.ID) +func (s *private) setEmail(w http.ResponseWriter, r *http.Request, userID, siteID, address string) { + log.Printf("[DEBUG] set email for user %s", userID) - val, err := s.dataService.SetUserEmail(confirm.Site, user.ID, address) + val, err := s.dataService.SetUserEmail(siteID, userID, address) if err != nil { code := parseError(err, rest.ErrInternal) rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't set email for user", code) return } - // update User.Email from the token + // update User.Email field claims, _, err := s.authenticator.TokenService().Get(r) if err != nil { rest.SendErrorJSON(w, r, http.StatusForbidden, err, "failed to verify confirmation token", rest.ErrInternal) diff --git a/backend/app/rest/api/rest_private_test.go b/backend/app/rest/api/rest_private_test.go index 5cb66d46..0e412863 100644 --- a/backend/app/rest/api/rest_private_test.go +++ b/backend/app/rest/api/rest_private_test.go @@ -849,7 +849,7 @@ func TestRest_EmailNotification(t *testing.T) { // send confirmation token for email req, err = http.NewRequest( http.MethodPost, - ts.URL+"/api/v1/email/subscribe?site=remark42&address=", + ts.URL+"/api/v1/email/subscribe", io.NopCloser(strings.NewReader(`{"site": "remark42", "address": "good@example.com"}`)), ) require.NoError(t, err) @@ -866,6 +866,25 @@ func TestRest_EmailNotification(t *testing.T) { assert.Equal(t, "good@example.com", mockDestination.GetVerify()[0].Email) verificationToken := mockDestination.GetVerify()[0].Token + // get user information to verify lack of the subscription + 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) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + var clearUser store.User + err = json.Unmarshal(body, &clearUser) + assert.NoError(t, err) + assert.Equal(t, store.User{Name: "developer one", ID: "dev", EmailSubscription: false, + Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, clearUser) + // verify email req, err = http.NewRequest( http.MethodPost, @@ -894,11 +913,11 @@ func TestRest_EmailNotification(t *testing.T) { require.NoError(t, err) require.NoError(t, resp.Body.Close()) require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) - var user store.User - err = json.Unmarshal(body, &user) + var subscribedUser store.User + err = json.Unmarshal(body, &subscribedUser) assert.NoError(t, err) assert.Equal(t, store.User{Name: "developer one", ID: "dev", EmailSubscription: true, - Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, user) + Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, subscribedUser) // create child comment from another user, email notification expected req, err = http.NewRequest("POST", ts.URL+"/api/v1/comment", strings.NewReader(fmt.Sprintf( @@ -949,6 +968,80 @@ func TestRest_EmailNotification(t *testing.T) { time.Sleep(time.Millisecond * 30) require.Equal(t, 4, len(mockDestination.Get())) assert.Empty(t, mockDestination.Get()[3].Emails) + + // confirm email via subscribe call with query params, old behavior, email notification is expected + req, err = http.NewRequest( + http.MethodPost, + ts.URL+"/api/v1/email/subscribe?site=remark42&address=good@example.com", + http.NoBody, + ) + require.NoError(t, err) + req.Header.Add("X-JWT", emailUserToken) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + // wait for mock notification Submit to kick off + time.Sleep(time.Millisecond * 30) + require.Equal(t, 2, len(mockDestination.GetVerify()), "verification email was sent") + + // get email user information to verify there is no subscription yet + req, err = http.NewRequest( + http.MethodGet, + ts.URL+"/api/v1/user?site=remark42", + http.NoBody) + require.NoError(t, err) + req.Header.Add("X-JWT", emailUserToken) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + var unsubscribedEmailUser store.User + err = json.Unmarshal(body, &unsubscribedEmailUser) + assert.NoError(t, err) + assert.Equal(t, store.User{Name: "good@example.com test user", ID: "email_f5dfe9d2e6bd75fc74ea5fabf273b45b5baeb195", EmailSubscription: false, + Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, unsubscribedEmailUser) + + // confirm email via subscribe call, no email notification is expected + req, err = http.NewRequest( + http.MethodPost, + ts.URL+"/api/v1/email/subscribe", + io.NopCloser(strings.NewReader(`{"site": "remark42", "address": "good@example.com"}`)), + ) + require.NoError(t, err) + req.Header.Add("X-JWT", emailUserToken) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + // wait for mock notification Submit to kick off + time.Sleep(time.Millisecond * 30) + require.Equal(t, 2, len(mockDestination.GetVerify()), "no new verification email was sent") + + // get email user information to verify the subscription happened without the confirmation call + req, err = http.NewRequest( + http.MethodGet, + ts.URL+"/api/v1/user?site=remark42", + http.NoBody) + require.NoError(t, err) + req.Header.Add("X-JWT", emailUserToken) + resp, err = client.Do(req) + require.NoError(t, err) + body, err = io.ReadAll(resp.Body) + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + require.Equal(t, http.StatusOK, resp.StatusCode, string(body)) + var subscribedEmailUser store.User + err = json.Unmarshal(body, &subscribedEmailUser) + assert.NoError(t, err) + assert.Equal(t, store.User{Name: "good@example.com test user", ID: "email_f5dfe9d2e6bd75fc74ea5fabf273b45b5baeb195", EmailSubscription: true, + Picture: "http://example.com/pic.png", IP: "127.0.0.1", SiteID: "remark42"}, subscribedEmailUser) } func TestRest_TelegramNotification(t *testing.T) { diff --git a/backend/app/rest/api/rest_test.go b/backend/app/rest/api/rest_test.go index 0667da53..80280052 100644 --- a/backend/app/rest/api/rest_test.go +++ b/backend/app/rest/api/rest_test.go @@ -41,6 +41,8 @@ var devToken = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcms0MiIsImV var anonToken = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcms0MiIsImV4cCI6Mzc4OTE5MTgyMiwianRpIjoicmFuZG9tIGlkIiwiaXNzIjoicmVtYXJrNDIiLCJuYmYiOjE1MjE4ODQyMjIsInVzZXIiOnsibmFtZSI6ImFub255bW91cyB0ZXN0IHVzZXIiLCJpZCI6ImFub255bW91c190ZXN0X3VzZXIiLCJwaWN0dXJlIjoiaHR0cDovL2V4YW1wbGUuY29tL3BpYy5wbmciLCJpcCI6IjEyNy4wLjAuMSIsImVtYWlsIjoiYW5vbkBleGFtcGxlLmNvbSJ9fQ.gAae2WMxZNZE5ebVboptPEyQ7Nk6EQxciNnGJ_mPOuU` +var emailUserToken = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcms0MiIsImV4cCI6Mzc4OTE5MTgyMiwianRpIjoicmFuZG9tIGlkIiwiaXNzIjoicmVtYXJrNDIiLCJuYmYiOjE1MjE4ODQyMjIsInVzZXIiOnsibmFtZSI6Imdvb2RAZXhhbXBsZS5jb20gdGVzdCB1c2VyIiwiaWQiOiJlbWFpbF9mNWRmZTlkMmU2YmQ3NWZjNzRlYTVmYWJmMjczYjQ1YjViYWViMTk1IiwicGljdHVyZSI6Imh0dHA6Ly9leGFtcGxlLmNvbS9waWMucG5nIiwiaXAiOiIxMjcuMC4wLjEiLCJlbWFpbCI6Imdvb2RAZXhhbXBsZS5jb20ifX0.vH2HN1JpuXL8okTJq1A-zGHQ-l2ILcwxvDDEmu2zwks` + var devTokenBadAud = `eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcms0Ml9iYWQiLCJleHAiOjM3ODkxOTE4MjIsImp0aSI6InJhbmRvbSBpZCIsImlzcyI6InJlbWFyazQyIiwibmJmIjoxNTIxODg0MjIyLCJ1c2VyIjp7Im5hbWUiOiJkZXZlbG9wZXIgb25lIiwiaWQiOiJkZXYiLCJwaWN0dXJlIjoiaHR0cDovL2V4YW1wbGUuY29tL3BpYy5wbmciLCJpcCI6IjEyNy4wLjAuMSIsImVtYWlsIjoibWVAZXhhbXBsZS5jb20ifX0.FuTTocVtcxr4VjpfIICvU2yOb3su28VkDzj94H9Q3xY` var adminUmputunToken = `eyJhbGciOiJIUzI1NiJ9.eyJhdWQiOiJyZW1hcms0MiIsImV4cCI6MTk1NDU5Nzk4MCwianRpIjoiOTdhMmUwYWM0ZGM3ZDVmNjkyNmQ1ZTg2MjBhY2VmOWE0MGMwIiwiaWF0IjoxNDU0NTk3NjgwLCJpc3MiOiJyZW1hcms0MiIsInVzZXIiOnsibmFtZSI6IlVtcHV0dW4iLCJpZCI6ImdpdGh1Yl9lZjBmNzA2YTciLCJwaWN0dXJlIjoiaHR0cHM6Ly9yZW1hcms0Mi5yYWRpby10LmNvbS9hcGkvdjEvYXZhdGFyL2NiNDJmZjQ5M2FkZTY5NmQ4OGEzYTU5MGYxMzZhZTllMzRkZTdjMWIuaW1hZ2UiLCJhdHRycyI6eyJhZG1pbiI6dHJ1ZSwiYmxvY2tlZCI6ZmFsc2V9fX0.dZiOjWHguo9f42XCMooMcv4EmYFzifl_-LEvPZHCtks` diff --git a/backend/remark.rest b/backend/remark.rest index eca6c6d1..7febc218 100644 --- a/backend/remark.rest +++ b/backend/remark.rest @@ -119,6 +119,8 @@ 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. +### in case the user logged in with the same email, it will be confirmed right away with "updated" set to "true" in the response, +### and no email will be sent. POST {{host}}/api/v1/email/subscribe X-JWT: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJyZW1hcmsiLCJleHAiOjE5NzYwNTY3NTYsImp0aSI6IjJlOGJmMTE5OTI0MjQxMDRjYjFhZGRlODllMWYwNGFiMTg4YWZjMzQiLCJpYXQiOjE1NzYwNTY0NTYsImlzcyI6InJlbWFyazQyIiwidXNlciI6eyJuYW1lIjoiZGV2X3VzZXIiLCJpZCI6ImRldl91c2VyIiwicGljdHVyZSI6Imh0dHA6Ly8xMjcuMC4wLjE6ODA4MC9hcGkvdjEvYXZhdGFyL2NjZmEyYWJkMDE2Njc2MDViNGUxZmM0ZmNiOTFiMWUxYWYzMjMyNDAuaW1hZ2UiLCJhdHRycyI6eyJhZG1pbiI6dHJ1ZSwiYmxvY2tlZCI6ZmFsc2V9fX0.6Qt5s2enBMRC-Jmsua01yViVYI95Dx6BPBMaNjj36d4 Content-Type: application/json