From 9dc76c18231f158ab71dfcfae2f1659a4f2a5396 Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Tue, 7 Feb 2017 09:35:58 -0800 Subject: Implement PUT /users/{user_id}/password endpoint for APIv4 (#5243) --- api4/apitestlib.go | 6 +++- api4/user.go | 38 +++++++++++++++++++++++++ api4/user_test.go | 77 +++++++++++++++++++++++++++++++++++++++++++++++++-- app/authentication.go | 8 +++--- app/user.go | 9 ++---- model/client4.go | 11 ++++++++ utils/config.go | 2 +- 7 files changed, 136 insertions(+), 15 deletions(-) diff --git a/api4/apitestlib.go b/api4/apitestlib.go index f647ffa9c..2eaed4fd0 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -61,6 +61,8 @@ func Setup() *TestHelper { } func TearDown() { + utils.DisableDebugLogForTest() + options := map[string]bool{} options[store.USER_SEARCH_OPTION_NAMES_ONLY_NO_FULL_NAME] = true if result := <-app.Srv.Store.User().Search("", "fakeuser", options); result.Err != nil { @@ -86,6 +88,8 @@ func TearDown() { } } } + + utils.EnableDebugLogForTest() } func (me *TestHelper) InitBasic() *TestHelper { @@ -208,7 +212,7 @@ func (me *TestHelper) LoginBasicWithClient(client *model.Client4) { func (me *TestHelper) LoginBasic2WithClient(client *model.Client4) { utils.DisableDebugLogForTest() - client.Login(me.BasicUser.Email, me.BasicUser.Password) + client.Login(me.BasicUser2.Email, me.BasicUser2.Password) utils.EnableDebugLogForTest() } diff --git a/api4/user.go b/api4/user.go index cf9bb4ead..56cfc5d90 100644 --- a/api4/user.go +++ b/api4/user.go @@ -23,6 +23,7 @@ func InitUser() { BaseRoutes.User.Handle("", ApiSessionRequired(updateUser)).Methods("PUT") 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("/login", ApiHandler(login)).Methods("POST") BaseRoutes.Users.Handle("/logout", ApiHandler(logout)).Methods("POST") @@ -281,6 +282,43 @@ func updateUserRoles(c *Context, w http.ResponseWriter, r *http.Request) { ReturnStatusOK(w) } +func updatePassword(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireUserId() + if c.Err != nil { + return + } + + props := model.MapFromJson(r.Body) + + newPassword := props["new_password"] + + c.LogAudit("attempted") + + var err *model.AppError + if c.Params.UserId == c.Session.UserId { + currentPassword := props["current_password"] + if len(currentPassword) <= 0 { + c.SetInvalidParam("current_password") + return + } + + err = app.UpdatePasswordAsUser(c.Params.UserId, currentPassword, newPassword, c.GetSiteURL()) + } else if app.SessionHasPermissionTo(c.Session, model.PERMISSION_MANAGE_SYSTEM) { + err = app.UpdatePasswordByUserIdSendEmail(c.Params.UserId, newPassword, c.T("api.user.reset_password.method"), c.GetSiteURL()) + } else { + err = model.NewAppError("updatePassword", "api.user.update_password.context.app_error", nil, "", http.StatusForbidden) + } + + if err != nil { + c.LogAudit("failed") + c.Err = err + return + } else { + c.LogAudit("completed") + 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 dc8a82310..bf4612635 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -149,7 +149,6 @@ func TestGetUserByEmail(t *testing.T) { ruser, resp = Client.GetUserByEmail(user.Email, resp.Etag) CheckEtag(t, ruser, resp) - _, resp = Client.GetUserByEmail(GenerateTestUsername(), "") CheckBadRequestStatus(t, resp) @@ -287,7 +286,7 @@ func TestUpdateUser(t *testing.T) { func TestDeleteUser(t *testing.T) { th := Setup().InitBasic().InitSystemAdmin() Client := th.Client - + user := th.BasicUser th.LoginBasic() @@ -296,7 +295,7 @@ func TestDeleteUser(t *testing.T) { CheckForbiddenStatus(t, resp) Client.Logout() - + _, resp = Client.DeleteUser(user.Id) CheckUnauthorizedStatus(t, resp) @@ -510,3 +509,75 @@ func TestGetUsersNotInChannel(t *testing.T) { _, resp = th.SystemAdminClient.GetUsersNotInChannel(teamId, channelId, 0, 60, "") CheckNoError(t, resp) } + +func TestUpdateUserPassword(t *testing.T) { + th := Setup().InitBasic().InitSystemAdmin() + defer TearDown() + Client := th.Client + + password := "newpassword1" + pass, resp := Client.UpdateUserPassword(th.BasicUser.Id, th.BasicUser.Password, password) + CheckNoError(t, resp) + + if !pass { + t.Fatal("should have returned true") + } + + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, password, "") + CheckBadRequestStatus(t, resp) + + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, password, "junk") + CheckBadRequestStatus(t, resp) + + _, resp = Client.UpdateUserPassword("junk", password, password) + CheckBadRequestStatus(t, resp) + + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, "", password) + CheckBadRequestStatus(t, resp) + + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, "junk", password) + CheckBadRequestStatus(t, resp) + + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, password, th.BasicUser.Password) + CheckNoError(t, resp) + + Client.Logout() + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, password, password) + CheckUnauthorizedStatus(t, resp) + + th.LoginBasic2() + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, password, password) + CheckForbiddenStatus(t, resp) + + th.LoginBasic() + + // Test lockout + passwordAttempts := utils.Cfg.ServiceSettings.MaximumLoginAttempts + defer func() { + utils.Cfg.ServiceSettings.MaximumLoginAttempts = passwordAttempts + }() + utils.Cfg.ServiceSettings.MaximumLoginAttempts = 2 + + // Fail twice + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, "badpwd", "newpwd") + CheckBadRequestStatus(t, resp) + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, "badpwd", "newpwd") + CheckBadRequestStatus(t, resp) + + // Should fail because account is locked out + _, resp = Client.UpdateUserPassword(th.BasicUser.Id, th.BasicUser.Password, "newpwd") + CheckErrorMessage(t, resp, "api.user.check_user_login_attempts.too_many.app_error") + CheckForbiddenStatus(t, resp) + + // System admin can update another user's password + adminSetPassword := "pwdsetbyadmin" + pass, resp = th.SystemAdminClient.UpdateUserPassword(th.BasicUser.Id, "", adminSetPassword) + CheckNoError(t, resp) + + if !pass { + t.Fatal("should have returned true") + } + + _, resp = Client.Login(th.BasicUser.Email, adminSetPassword) + CheckNoError(t, resp) +} diff --git a/app/authentication.go b/app/authentication.go index 0561ff821..3691c7c3e 100644 --- a/app/authentication.go +++ b/app/authentication.go @@ -4,12 +4,12 @@ package app import ( + "net/http" + "strings" + "github.com/mattermost/platform/einterfaces" "github.com/mattermost/platform/model" "github.com/mattermost/platform/utils" - - "net/http" - "strings" ) func CheckPasswordAndAllCriteria(user *model.User, password string, mfaToken string) *model.AppError { @@ -123,7 +123,7 @@ func CheckUserMfa(user *model.User, token string) *model.AppError { func checkUserLoginAttempts(user *model.User) *model.AppError { if user.FailedAttempts >= utils.Cfg.ServiceSettings.MaximumLoginAttempts { - return model.NewLocAppError("checkUserLoginAttempts", "api.user.check_user_login_attempts.too_many.app_error", nil, "user_id="+user.Id) + return model.NewAppError("checkUserLoginAttempts", "api.user.check_user_login_attempts.too_many.app_error", nil, "user_id="+user.Id, http.StatusForbidden) } return nil diff --git a/app/user.go b/app/user.go index c302db511..2fc308421 100644 --- a/app/user.go +++ b/app/user.go @@ -764,22 +764,19 @@ func UpdatePasswordAsUser(userId, currentPassword, newPassword, siteURL string) } if user == nil { - err = model.NewLocAppError("updatePassword", "api.user.update_password.valid_account.app_error", nil, "") - err.StatusCode = http.StatusBadRequest + err = model.NewAppError("updatePassword", "api.user.update_password.valid_account.app_error", nil, "", http.StatusBadRequest) return err } if user.AuthData != nil && *user.AuthData != "" { - err = model.NewLocAppError("updatePassword", "api.user.update_password.oauth.app_error", nil, "auth_service="+user.AuthService) - err.StatusCode = http.StatusBadRequest + err = model.NewAppError("updatePassword", "api.user.update_password.oauth.app_error", nil, "auth_service="+user.AuthService, http.StatusBadRequest) return err } if err := doubleCheckPassword(user, currentPassword); err != nil { if err.Id == "api.user.check_user_password.invalid.app_error" { - err = model.NewLocAppError("updatePassword", "api.user.update_password.incorrect.app_error", nil, "") + err = model.NewAppError("updatePassword", "api.user.update_password.incorrect.app_error", nil, "", http.StatusBadRequest) } - err.StatusCode = http.StatusForbidden return err } diff --git a/model/client4.go b/model/client4.go index 88082869e..41963edba 100644 --- a/model/client4.go +++ b/model/client4.go @@ -292,6 +292,17 @@ func (c *Client4) UpdateUser(user *User) (*User, *Response) { } } +// UpdateUserPassword updates a user's password. Must be logged in as the user or be a system administrator. +func (c *Client4) UpdateUserPassword(userId, currentPassword, newPassword string) (bool, *Response) { + requestBody := map[string]string{"current_password": currentPassword, "new_password": newPassword} + if r, err := c.DoApiPut(c.GetUserRoute(userId)+"/password", MapToJson(requestBody)); err != nil { + return false, &Response{StatusCode: r.StatusCode, Error: err} + } else { + defer closeBody(r) + return CheckStatusOK(r), BuildResponse(r) + } +} + // UpdateUserRoles updates a user's roles in the system. A user can have "system_user" and "system_admin" roles. func (c *Client4) UpdateUserRoles(userId, roles string) (bool, *Response) { requestBody := map[string]string{"roles": roles} diff --git a/utils/config.go b/utils/config.go index 243e2b984..a1f647b3f 100644 --- a/utils/config.go +++ b/utils/config.go @@ -68,7 +68,7 @@ func FindDir(dir string) string { func DisableDebugLogForTest() { if l4g.Global["stdout"] != nil { originalDisableDebugLvl = l4g.Global["stdout"].Level - l4g.Global["stdout"].Level = l4g.WARNING + l4g.Global["stdout"].Level = l4g.ERROR } } -- cgit v1.2.3-1-g7c22