From a523bd58895405f8ca8c8a8b3468b8a9ebed0c42 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 24 May 2023 15:30:55 +0800 Subject: [PATCH] Only validate changed columns when update user (#24867) Fix #23211 Replace #23496 --- models/user/user.go | 25 ++++++++++++++++--------- models/user/user_test.go | 18 ++++++++++++++++++ 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 0be36e69ab..57b2117bb9 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -621,7 +621,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 } @@ -767,19 +767,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 } @@ -845,7 +852,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 cbfcd15463..44eaf63556 100644 --- a/models/user/user_test.go +++ b/models/user/user_test.go @@ -526,3 +526,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)) + } +}