From 75122edc972209e71b41bb3de1f6f10593f2e144 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 24 May 2023 04:24:02 -0400 Subject: [PATCH] Only validate changed columns when update user (#24867) (#24903) Backport #24867 by @lunny Fix #23211 Replace #23496 --------- Co-authored-by: Lunny Xiao (cherry picked from commit 275abd65935216c05aa2d1238d8d8a7cfc739a35) --- models/user/user.go | 25 ++++++++++++++++--------- models/user/user_test.go | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 185747ac23..0e52588be6 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -614,7 +614,7 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e } // validate data - if err := validateUser(u); err != nil { + if err := ValidateUser(u); err != nil { return err } @@ -804,19 +804,26 @@ func checkDupEmail(ctx context.Context, u *User) error { return nil } -// validateUser check if user is valid to insert / update into database -func validateUser(u *User) error { - if !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(u.Visibility) && !u.IsOrganization() { - return fmt.Errorf("visibility Mode not allowed: %s", u.Visibility.String()) +// ValidateUser check if user is valid to insert / update into database +func ValidateUser(u *User, cols ...string) error { + if len(cols) == 0 || util.SliceContainsString(cols, "visibility", true) { + if !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(u.Visibility) && !u.IsOrganization() { + return fmt.Errorf("visibility Mode not allowed: %s", u.Visibility.String()) + } } - u.Email = strings.ToLower(u.Email) - return ValidateEmail(u.Email) + if len(cols) == 0 || util.SliceContainsString(cols, "email", true) { + u.Email = strings.ToLower(u.Email) + if err := ValidateEmail(u.Email); err != nil { + return err + } + } + return nil } // UpdateUser updates user's information. func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...string) error { - err := validateUser(u) + err := ValidateUser(u, cols...) if err != nil { return err } @@ -882,7 +889,7 @@ func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...s // UpdateUserCols update user according special columns func UpdateUserCols(ctx context.Context, u *User, cols ...string) error { - if err := validateUser(u); err != nil { + if err := ValidateUser(u, cols...); err != nil { return err } diff --git a/models/user/user_test.go b/models/user/user_test.go index 8e78fee6b3..1abd0f0049 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -5,6 +5,7 @@ package user_test import ( "context" + "fmt" "math/rand" "strings" "testing" @@ -524,3 +525,21 @@ func TestIsUserVisibleToViewer(t *testing.T) { test(user31, user33, true) test(user31, nil, false) } + +func Test_ValidateUser(t *testing.T) { + oldSetting := setting.Service.AllowedUserVisibilityModesSlice + defer func() { + setting.Service.AllowedUserVisibilityModesSlice = oldSetting + }() + setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, true} + kases := map[*user_model.User]bool{ + {ID: 1, Visibility: structs.VisibleTypePublic}: true, + {ID: 2, Visibility: structs.VisibleTypeLimited}: false, + {ID: 2, Visibility: structs.VisibleTypeLimited, Email: "invalid"}: false, + {ID: 2, Visibility: structs.VisibleTypePrivate, Email: "valid@valid.com"}: true, + } + for kase, expected := range kases { + err := user_model.ValidateUser(kase) + assert.EqualValues(t, expected, err == nil, fmt.Sprintf("case: %+v", kase)) + } +}