From eb767d2c1cb65724f25479144d68a9d102d32dfa Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Tue, 7 Feb 2017 10:46:40 -0800 Subject: Implement password reset endpoints for APIv4 (#5256) --- api4/user.go | 51 +++++++++++++++++++++++-- api4/user_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++ app/user.go | 8 ++-- model/client4.go | 23 ++++++++++++ store/sql_recovery_store.go | 9 ++++- 5 files changed, 173 insertions(+), 8 deletions(-) diff --git a/api4/user.go b/api4/user.go index 56cfc5d90..348ccf46c 100644 --- a/api4/user.go +++ b/api4/user.go @@ -24,6 +24,8 @@ func InitUser() { BaseRoutes.User.Handle("", ApiSessionRequired(deleteUser)).Methods("DELETE") BaseRoutes.User.Handle("/roles", ApiSessionRequired(updateUserRoles)).Methods("PUT") BaseRoutes.User.Handle("/password", ApiSessionRequired(updatePassword)).Methods("PUT") + BaseRoutes.Users.Handle("/password/reset", ApiHandler(resetPassword)).Methods("POST") + BaseRoutes.Users.Handle("/password/reset/send", ApiHandler(sendPasswordReset)).Methods("POST") BaseRoutes.Users.Handle("/login", ApiHandler(login)).Methods("POST") BaseRoutes.Users.Handle("/logout", ApiHandler(logout)).Methods("POST") @@ -224,7 +226,7 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) { } } -func deleteUser(c *Context, w http.ResponseWriter, r *http.Request){ +func deleteUser(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireUserId() if c.Err != nil { return @@ -236,7 +238,7 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request){ c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS) return } - + var user *model.User var err *model.AppError @@ -247,7 +249,7 @@ func deleteUser(c *Context, w http.ResponseWriter, r *http.Request){ if _, err := app.UpdateActive(user, false); err != nil { c.Err = err - return + return } ReturnStatusOK(w) @@ -319,6 +321,49 @@ func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) { } } +func resetPassword(c *Context, w http.ResponseWriter, r *http.Request) { + props := model.MapFromJson(r.Body) + + code := props["code"] + if len(code) != model.PASSWORD_RECOVERY_CODE_SIZE { + c.SetInvalidParam("code") + return + } + + newPassword := props["new_password"] + + c.LogAudit("attempt - code=" + code) + + if err := app.ResetPasswordFromCode(code, newPassword, c.GetSiteURL()); err != nil { + c.LogAudit("fail - code=" + code) + c.Err = err + return + } + + c.LogAudit("success - code=" + code) + + ReturnStatusOK(w) +} + +func sendPasswordReset(c *Context, w http.ResponseWriter, r *http.Request) { + props := model.MapFromJson(r.Body) + + email := props["email"] + if len(email) == 0 { + c.SetInvalidParam("email") + return + } + + if sent, err := app.SendPasswordReset(email, c.GetSiteURL()); err != nil { + c.Err = err + return + } else if sent { + c.LogAudit("sent=" + email) + } + + ReturnStatusOK(w) +} + func login(c *Context, w http.ResponseWriter, r *http.Request) { props := model.MapFromJson(r.Body) diff --git a/api4/user_test.go b/api4/user_test.go index bf4612635..37f251c6d 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -6,8 +6,10 @@ package api4 import ( "net/http" "strconv" + "strings" "testing" + "github.com/mattermost/platform/app" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" ) @@ -581,3 +583,91 @@ func TestUpdateUserPassword(t *testing.T) { _, resp = Client.Login(th.BasicUser.Email, adminSetPassword) CheckNoError(t, resp) } + +func TestResetPassword(t *testing.T) { + th := Setup().InitBasic() + Client := th.Client + + Client.Logout() + + user := th.BasicUser + + // Delete all the messages before check the reset password + utils.DeleteMailBox(user.Email) + + success, resp := Client.SendPasswordResetEmail(user.Email) + CheckNoError(t, resp) + if !success { + t.Fatal("should have succeeded") + } + + _, resp = Client.SendPasswordResetEmail("") + CheckBadRequestStatus(t, resp) + + // Should not leak whether the email is attached to an account or not + success, resp = Client.SendPasswordResetEmail("notreal@example.com") + CheckNoError(t, resp) + if !success { + t.Fatal("should have succeeded") + } + + var recovery *model.PasswordRecovery + if result := <-app.Srv.Store.PasswordRecovery().Get(user.Id); result.Err != nil { + t.Fatal(result.Err) + } else { + recovery = result.Data.(*model.PasswordRecovery) + } + + // Check if the email was send to the right email address and the recovery key match + if resultsMailbox, err := utils.GetMailBox(user.Email); err != nil && !strings.ContainsAny(resultsMailbox[0].To[0], user.Email) { + t.Fatal("Wrong To recipient") + } else { + if resultsEmail, err := utils.GetMessageFromMailbox(user.Email, resultsMailbox[0].ID); err == nil { + if !strings.Contains(resultsEmail.Body.Text, recovery.Code) { + t.Log(resultsEmail.Body.Text) + t.Log(recovery.Code) + t.Fatal("Received wrong recovery code") + } + } + } + + _, resp = Client.ResetPassword(recovery.Code, "") + CheckBadRequestStatus(t, resp) + + _, resp = Client.ResetPassword(recovery.Code, "newp") + CheckBadRequestStatus(t, resp) + + _, resp = Client.ResetPassword("", "newpwd") + CheckBadRequestStatus(t, resp) + + _, resp = Client.ResetPassword("junk", "newpwd") + CheckBadRequestStatus(t, resp) + + code := "" + for i := 0; i < model.PASSWORD_RECOVERY_CODE_SIZE; i++ { + code += "a" + } + + _, resp = Client.ResetPassword(code, "newpwd") + CheckBadRequestStatus(t, resp) + + success, resp = Client.ResetPassword(recovery.Code, "newpwd") + CheckNoError(t, resp) + if !success { + t.Fatal("should have succeeded") + } + + Client.Login(user.Email, "newpwd") + Client.Logout() + + _, resp = Client.ResetPassword(recovery.Code, "newpwd") + CheckBadRequestStatus(t, resp) + + authData := model.NewId() + if result := <-app.Srv.Store.User().UpdateAuthData(user.Id, "random", &authData, "", true); result.Err != nil { + t.Fatal(result.Err) + } + + _, resp = Client.SendPasswordResetEmail(user.Email) + CheckBadRequestStatus(t, resp) +} diff --git a/app/user.go b/app/user.go index 2fc308421..badcc1676 100644 --- a/app/user.go +++ b/app/user.go @@ -966,7 +966,7 @@ func SendPasswordReset(email string, siteURL string) (bool, *model.AppError) { } if user.AuthData != nil && len(*user.AuthData) != 0 { - return false, model.NewLocAppError("SendPasswordReset", "api.user.send_password_reset.sso.app_error", nil, "userId="+user.Id) + return false, model.NewAppError("SendPasswordReset", "api.user.send_password_reset.sso.app_error", nil, "userId="+user.Id, http.StatusBadRequest) } var recovery *model.PasswordRecovery @@ -1001,7 +1001,7 @@ func ResetPasswordFromCode(code, newPassword, siteURL string) *model.AppError { return err } else { if model.GetMillis()-recovery.CreateAt >= model.PASSWORD_RECOVER_EXPIRY_TIME { - return model.NewLocAppError("resetPassword", "api.user.reset_password.link_expired.app_error", nil, "") + return model.NewAppError("resetPassword", "api.user.reset_password.link_expired.app_error", nil, "", http.StatusBadRequest) } } @@ -1011,7 +1011,7 @@ func ResetPasswordFromCode(code, newPassword, siteURL string) *model.AppError { } if user.IsSSOUser() { - return model.NewLocAppError("ResetPasswordFromCode", "api.user.reset_password.sso.app_error", nil, "userId="+user.Id) + return model.NewAppError("ResetPasswordFromCode", "api.user.reset_password.sso.app_error", nil, "userId="+user.Id, http.StatusBadRequest) } T := utils.GetUserTranslations(user.Locale) @@ -1040,7 +1040,7 @@ func CreatePasswordRecovery(userId string) (*model.PasswordRecovery, *model.AppE func GetPasswordRecovery(code string) (*model.PasswordRecovery, *model.AppError) { if result := <-Srv.Store.PasswordRecovery().GetByCode(code); result.Err != nil { - return nil, model.NewLocAppError("GetPasswordRecovery", "api.user.reset_password.invalid_link.app_error", nil, result.Err.Error()) + return nil, model.NewAppError("GetPasswordRecovery", "api.user.reset_password.invalid_link.app_error", nil, result.Err.Error(), http.StatusBadRequest) } else { return result.Data.(*model.PasswordRecovery), nil } diff --git a/model/client4.go b/model/client4.go index 41963edba..b086839f9 100644 --- a/model/client4.go +++ b/model/client4.go @@ -324,6 +324,29 @@ func (c *Client4) DeleteUser(userId string) (bool, *Response) { } } +// SendPasswordResetEmail will send a link for password resetting to a user with the +// provided email. +func (c *Client4) SendPasswordResetEmail(email string) (bool, *Response) { + requestBody := map[string]string{"email": email} + if r, err := c.DoApiPost(c.GetUsersRoute()+"/password/reset/send", MapToJson(requestBody)); err != nil { + return false, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return CheckStatusOK(r), BuildResponse(r) + } +} + +// ResetPassword uses a recovery code to update reset a user's password. +func (c *Client4) ResetPassword(code, newPassword string) (bool, *Response) { + requestBody := map[string]string{"code": code, "new_password": newPassword} + if r, err := c.DoApiPost(c.GetUsersRoute()+"/password/reset", MapToJson(requestBody)); err != nil { + return false, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return CheckStatusOK(r), BuildResponse(r) + } +} + // Team Section // CreateTeam creates a team in the system based on the provided team struct. diff --git a/store/sql_recovery_store.go b/store/sql_recovery_store.go index c43b9bbfa..d62993822 100644 --- a/store/sql_recovery_store.go +++ b/store/sql_recovery_store.go @@ -4,6 +4,9 @@ package store import ( + "database/sql" + "net/http" + "github.com/mattermost/platform/model" ) @@ -108,7 +111,11 @@ func (s SqlPasswordRecoveryStore) GetByCode(code string) StoreChannel { recovery := model.PasswordRecovery{} if err := s.GetReplica().SelectOne(&recovery, "SELECT * FROM PasswordRecovery WHERE Code = :Code", map[string]interface{}{"Code": code}); err != nil { - result.Err = model.NewLocAppError("SqlPasswordRecoveryStore.GetByCode", "store.sql_recover.get_by_code.app_error", nil, "") + if err == sql.ErrNoRows { + result.Err = model.NewAppError("SqlPasswordRecoveryStore.GetByCode", "store.sql_recover.get_by_code.app_error", nil, "", http.StatusBadRequest) + } else { + result.Err = model.NewAppError("SqlPasswordRecoveryStore.GetByCode", "store.sql_recover.get_by_code.app_error", nil, "", http.StatusInternalServerError) + } } result.Data = &recovery -- cgit v1.2.3-1-g7c22