Skip to content

Commit

Permalink
Skip to validate username when update user profile
Browse files Browse the repository at this point in the history
  fixes goharbor#19528

Signed-off-by: stonezdj <daojunz@vmware.com>
  • Loading branch information
stonezdj committed Nov 7, 2023
1 parent e1a4423 commit 35842a6
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 19 deletions.
21 changes: 13 additions & 8 deletions src/server/v2.0/handler/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (u *usersAPI) CreateUser(ctx context.Context, params operation.CreateUserPa
Comment: params.UserReq.Comment,
Password: params.UserReq.Password,
}
if err := validateUserProfile(m); err != nil {
if err := validateUserProfile(m, true); err != nil {
return u.SendError(ctx, err)
}
uid, err := u.ctl.Create(ctx, m)
Expand Down Expand Up @@ -253,7 +253,7 @@ func (u *usersAPI) UpdateUserProfile(ctx context.Context, params operation.Updat
Email: params.Profile.Email,
Comment: params.Profile.Comment,
}
if err := validateUserProfile(m); err != nil {
if err := validateUserProfile(m, false); err != nil {
return u.SendError(ctx, err)
}
if err := u.ctl.UpdateProfile(ctx, m); err != nil {
Expand Down Expand Up @@ -482,7 +482,7 @@ func getRandomSecret() (string, error) {
return cliSecret, nil
}

func validateUserProfile(user *commonmodels.User) error {
func validateUserProfile(user *commonmodels.User, create bool) error {
if len(user.Email) > 0 {
if m, _ := regexp.MatchString(`^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$`, user.Email); !m {
return errors.BadRequestError(nil).WithMessage("email with illegal format")
Expand All @@ -499,17 +499,22 @@ func validateUserProfile(user *commonmodels.User) error {
return errors.BadRequestError(nil).WithMessage("realname contains illegal characters")
}

if utils.IsIllegalLength(user.Comment, -1, 30) {
return errors.BadRequestError(nil).WithMessage("comment with illegal length")
}

// skip to validate username for update because username is empty in the request
if !create {
return nil
}

if utils.IsIllegalLength(user.Username, 1, 255) {
return errors.BadRequestError(nil).WithMessage("usernamae with illegal length")
return errors.BadRequestError(nil).WithMessage("username with illegal length")
}

if strings.ContainsAny(user.Username, common.IllegalCharsInUsername) {
return errors.BadRequestError(nil).WithMessage("username contains illegal characters")
}

if utils.IsIllegalLength(user.Comment, -1, 30) {
return errors.BadRequestError(nil).WithMessage("comment with illegal length")
}

return nil
}
24 changes: 13 additions & 11 deletions src/server/v2.0/handler/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,26 +118,28 @@ func TestUserTestSuite(t *testing.T) {
func Test_validateUserProfile(t *testing.T) {
tooLongUsername := "mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
type args struct {
user *commonmodels.User
user *commonmodels.User
create bool
}
tests := []struct {
name string
args args
wantErr assert.ErrorAssertionFunc
}{
{"normal_test", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike@example.com"}}, assert.NoError},
{"illegall_username_,", args{&commonmodels.User{Username: "mike,mike", Realname: "mike", Email: "mike@example.com"}}, assert.Error},
{"illegall_username_$", args{&commonmodels.User{Username: "mike$mike", Realname: "mike", Email: "mike@example.com"}}, assert.Error},
{"illegall_username_%", args{&commonmodels.User{Username: "mike%mike", Realname: "mike", Email: "mike@example.com"}}, assert.Error},
{"illegall_username_#", args{&commonmodels.User{Username: "mike#mike", Realname: "mike", Email: "mike@example.com"}}, assert.Error},
{"illegall_realname", args{&commonmodels.User{Username: "mike", Realname: "mike,mike", Email: "mike@example.com"}}, assert.Error},
{"username_too_long", args{&commonmodels.User{Username: tooLongUsername, Realname: "mike", Email: "mike@example.com"}}, assert.Error},
{"invalid_email", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike#example.com"}}, assert.Error},
{"invalid_comment", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike@example.com", Comment: tooLongUsername}}, assert.Error},
{"normal_test", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike@example.com"}, true}, assert.NoError},
{"illegall_username_,", args{&commonmodels.User{Username: "mike,mike", Realname: "mike", Email: "mike@example.com"}, true}, assert.Error},
{"illegall_username_$", args{&commonmodels.User{Username: "mike$mike", Realname: "mike", Email: "mike@example.com"}, true}, assert.Error},
{"illegall_username_%", args{&commonmodels.User{Username: "mike%mike", Realname: "mike", Email: "mike@example.com"}, true}, assert.Error},
{"illegall_username_#", args{&commonmodels.User{Username: "mike#mike", Realname: "mike", Email: "mike@example.com"}, true}, assert.Error},
{"illegall_realname", args{&commonmodels.User{Username: "mike", Realname: "mike,mike", Email: "mike@example.com"}, true}, assert.Error},
{"update_profile", args{&commonmodels.User{Username: "", Realname: "mike", Email: "mike@example.com"}, false}, assert.NoError},
{"username_too_long", args{&commonmodels.User{Username: tooLongUsername, Realname: "mike", Email: "mike@example.com"}, true}, assert.Error},
{"invalid_email", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike#example.com"}, true}, assert.Error},
{"invalid_comment", args{&commonmodels.User{Username: "mike", Realname: "mike", Email: "mike@example.com", Comment: tooLongUsername}, true}, assert.Error},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tt.wantErr(t, validateUserProfile(tt.args.user), fmt.Sprintf("validateUserProfile(%v)", tt.args.user))
tt.wantErr(t, validateUserProfile(tt.args.user, tt.args.create), fmt.Sprintf("validateUserProfile(%v)", tt.args.user))
})
}
}

0 comments on commit 35842a6

Please sign in to comment.