From 6cbb6df9f8ae595678c0c25766ee6ee4a865497d Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 23 May 2023 09:47:49 +0800 Subject: [PATCH 1/2] Only validate changed columns when update user --- models/user/user.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 0be36e69abaf1..9cc19082a7d4a 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -768,18 +768,25 @@ func checkDupEmail(ctx context.Context, u *User) error { } // 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()) +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 } From d9df961abca88750e7ca4cdcbea28ca7f81f576e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 24 May 2023 15:00:55 +0800 Subject: [PATCH 2/2] Add tests for validateUser --- models/user/user.go | 10 +++++----- models/user/user_test.go | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/models/user/user.go b/models/user/user.go index 9cc19082a7d4a..57b2117bb9645 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,8 +767,8 @@ 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, cols ...string) error { +// 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()) @@ -786,7 +786,7 @@ func validateUser(u *User, cols ...string) error { // UpdateUser updates user's information. func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...string) error { - err := validateUser(u, cols...) + err := ValidateUser(u, cols...) if err != nil { return err } @@ -852,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, cols...); 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 cbfcd15463a88..44eaf63556baa 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)) + } +}