Skip to content

Commit a523bd5

Browse files
authored
Only validate changed columns when update user (#24867)
Fix #23211 Replace #23496
1 parent 37895b6 commit a523bd5

File tree

2 files changed

+34
-9
lines changed

2 files changed

+34
-9
lines changed

models/user/user.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -621,7 +621,7 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e
621621
}
622622

623623
// validate data
624-
if err := validateUser(u); err != nil {
624+
if err := ValidateUser(u); err != nil {
625625
return err
626626
}
627627

@@ -767,19 +767,26 @@ func checkDupEmail(ctx context.Context, u *User) error {
767767
return nil
768768
}
769769

770-
// validateUser check if user is valid to insert / update into database
771-
func validateUser(u *User) error {
772-
if !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(u.Visibility) && !u.IsOrganization() {
773-
return fmt.Errorf("visibility Mode not allowed: %s", u.Visibility.String())
770+
// ValidateUser check if user is valid to insert / update into database
771+
func ValidateUser(u *User, cols ...string) error {
772+
if len(cols) == 0 || util.SliceContainsString(cols, "visibility", true) {
773+
if !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(u.Visibility) && !u.IsOrganization() {
774+
return fmt.Errorf("visibility Mode not allowed: %s", u.Visibility.String())
775+
}
774776
}
775777

776-
u.Email = strings.ToLower(u.Email)
777-
return ValidateEmail(u.Email)
778+
if len(cols) == 0 || util.SliceContainsString(cols, "email", true) {
779+
u.Email = strings.ToLower(u.Email)
780+
if err := ValidateEmail(u.Email); err != nil {
781+
return err
782+
}
783+
}
784+
return nil
778785
}
779786

780787
// UpdateUser updates user's information.
781788
func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...string) error {
782-
err := validateUser(u)
789+
err := ValidateUser(u, cols...)
783790
if err != nil {
784791
return err
785792
}
@@ -845,7 +852,7 @@ func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...s
845852

846853
// UpdateUserCols update user according special columns
847854
func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
848-
if err := validateUser(u); err != nil {
855+
if err := ValidateUser(u, cols...); err != nil {
849856
return err
850857
}
851858

models/user/user_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,3 +526,21 @@ func TestIsUserVisibleToViewer(t *testing.T) {
526526
test(user31, user33, true)
527527
test(user31, nil, false)
528528
}
529+
530+
func Test_ValidateUser(t *testing.T) {
531+
oldSetting := setting.Service.AllowedUserVisibilityModesSlice
532+
defer func() {
533+
setting.Service.AllowedUserVisibilityModesSlice = oldSetting
534+
}()
535+
setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, true}
536+
kases := map[*user_model.User]bool{
537+
{ID: 1, Visibility: structs.VisibleTypePublic}: true,
538+
{ID: 2, Visibility: structs.VisibleTypeLimited}: false,
539+
{ID: 2, Visibility: structs.VisibleTypeLimited, Email: "invalid"}: false,
540+
{ID: 2, Visibility: structs.VisibleTypePrivate, Email: "[email protected]"}: true,
541+
}
542+
for kase, expected := range kases {
543+
err := user_model.ValidateUser(kase)
544+
assert.EqualValues(t, expected, err == nil, fmt.Sprintf("case: %+v", kase))
545+
}
546+
}

0 commit comments

Comments
 (0)