[MM-13839] Check for password when updating the eMail (#10207)

* Check for password when updating the eMail

* Require password for email change

* Enhance unit testing

* Restructure error handling for update email path

* govet

api4/user.go

@@ -685,13 +685,13 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if c.App.Session.IsOAuth {
+	ouser, err := c.App.GetUser(user.Id)
-		ouser, err := c.App.GetUser(user.Id)
+	if err != nil {
-		if err != nil {
+		c.Err = err
-			c.Err = err
+		return
-			return
+	}
-		}
 
+	if c.App.Session.IsOAuth {
 		if ouser.Email != user.Email {
 			c.SetPermissionError(model.PERMISSION_EDIT_OTHER_USERS)
 			c.Err.DetailedError += ", attempted email update by oauth app"
@@ -699,6 +699,15 @@ func updateUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// If eMail update is attempted by the currently logged in user, check if correct password was provided
+	if user.Email != "" && ouser.Email != user.Email && c.App.Session.UserId == c.Params.UserId {
+		err = c.App.DoubleCheckPassword(ouser, user.Password)
+		if err != nil {
+			c.SetInvalidParam("password")
+			return
+		}
+	}
+
 	ruser, err := c.App.UpdateUserAsUser(user, c.IsSystemAdmin())
 	if err != nil {
 		c.Err = err
@@ -745,6 +754,19 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
+	// If eMail update is attempted by the currently logged in user, check if correct password was provided
+	if patch.Email != nil && ouser.Email != *patch.Email && c.App.Session.UserId == c.Params.UserId {
+		if patch.Password == nil {
+			c.SetInvalidParam("password")
+			return
+		}
+
+		if err = c.App.DoubleCheckPassword(ouser, *patch.Password); err != nil {
+			c.SetInvalidParam("password")
+			return
+		}
+	}
+
 	ruser, err := c.App.PatchUser(c.Params.UserId, patch, c.IsSystemAdmin())
 	if err != nil {
 		c.Err = err

api4/user_test.go

@@ -1037,6 +1037,15 @@ func TestUpdateUser(t *testing.T) {
 		t.Fatal("LastPasswordUpdate should not have updated")
 	}
 
+	ruser.Email = th.GenerateTestEmail()
+	_, resp = th.Client.UpdateUser(ruser)
+	CheckBadRequestStatus(t, resp)
+
+	ruser.Password = user.Password
+	ruser, resp = th.Client.UpdateUser(ruser)
+	CheckNoError(t, resp)
+	CheckUserSanitization(t, ruser)
+
 	ruser.Id = "junk"
 	_, resp = th.Client.UpdateUser(ruser)
 	CheckBadRequestStatus(t, resp)
@@ -1084,7 +1093,7 @@ func TestPatchUser(t *testing.T) {
 	th.Client.Login(user.Email, user.Password)
 
 	patch := &model.UserPatch{}
-
+	patch.Password = model.NewString("testpassword")
 	patch.Nickname = model.NewString("Joram Wilander")
 	patch.FirstName = model.NewString("Joram")
 	patch.LastName = model.NewString("Wilander")
@@ -1115,6 +1124,9 @@ func TestPatchUser(t *testing.T) {
 	if ruser.Username != user.Username {
 		t.Fatal("Username should not have updated")
 	}
+	if ruser.Password != ""{
+		t.Fatal("Password should not be returned")
+	}
 	if ruser.NotifyProps["comment"] != "somethingrandom" {
 		t.Fatal("NotifyProps did not update properly")
 	}
@@ -1128,6 +1140,34 @@ func TestPatchUser(t *testing.T) {
 		t.Fatal("manualTimezone did not update properly")
 	}
 
+	err := th.App.CheckPasswordAndAllCriteria(ruser, *patch.Password, "")
+	assert.Error(t, err, "Password should not match")
+
+	currentPassword := user.Password
+	user, err = th.App.GetUser(ruser.Id)
+	if err != nil {
+		t.Fatal("User Get shouldn't error")
+	}
+
+	err = th.App.CheckPasswordAndAllCriteria(user, currentPassword, "")
+	if err != nil {
+		t.Fatal("Password should still match")
+	}
+
+	patch = &model.UserPatch{}
+	patch.Email = model.NewString(th.GenerateTestEmail())
+
+	_, resp = th.Client.PatchUser(user.Id, patch)
+	CheckBadRequestStatus(t, resp)
+
+	patch.Password = model.NewString(currentPassword)
+	ruser, resp = th.Client.PatchUser(user.Id, patch)
+	CheckNoError(t, resp)
+
+	if ruser.Email != *patch.Email {
+		t.Fatal("Email did not update properly")
+	}
+
 	patch.Username = model.NewString(th.BasicUser2.Username)
 	_, resp = th.Client.PatchUser(user.Id, patch)
 	CheckBadRequestStatus(t, resp)

app/authentication.go

@@ -57,7 +57,7 @@ func (a *App) CheckPasswordAndAllCriteria(user *model.User, password string, mfa
 }
 
 // This to be used for places we check the users password when they are already logged in
-func (a *App) doubleCheckPassword(user *model.User, password string) *model.AppError {
+func (a *App) DoubleCheckPassword(user *model.User, password string) *model.AppError {
 	if err := checkUserLoginAttempts(user, *a.Config().ServiceSettings.MaximumLoginAttempts); err != nil {
 		return err
 	}

app/user.go

@@ -854,7 +854,7 @@ func (a *App) UpdatePasswordAsUser(userId, currentPassword, newPassword string)
 		return err
 	}
 
-	if err := a.doubleCheckPassword(user, currentPassword); err != nil {
+	if err := a.DoubleCheckPassword(user, currentPassword); err != nil {
 		if err.Id == "api.user.check_user_password.invalid.app_error" {
 			err = model.NewAppError("updatePassword", "api.user.update_password.incorrect.app_error", nil, "", http.StatusBadRequest)
 		}

model/user.go

@@ -84,6 +84,7 @@ type User struct {
 
 type UserPatch struct {
 	Username    *string   `json:"username"`
+	Password    *string   `json:"password,omitempty"`
 	Nickname    *string   `json:"nickname"`
 	FirstName   *string   `json:"first_name"`
 	LastName    *string   `json:"last_name"`