Upstream/harbor
1
0
Fork 0
mirror of https://github.com/goharbor/harbor.git synced 2024-11-22 02:05:41 +01:00
Code Issues Projects Releases Wiki Activity

Replace comma in username to avoid casbin issue (#19505)

Browse Source 
Check username when creating user by API
   Replace comma with underscore in username for OnboardUser
   Fixes #19356

Signed-off-by: stonezdj <daojunz@vmware.com>
This commit is contained in:
stonezdj(Daojun Zhang) 2023-11-02 11:50:50 +08:00 committed by GitHub
parent f75a2f9407
commit b337f51e7e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 69 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/common/const.go
View File

@ -228,4 +228,6 @@ const (
ExecutionStatusRefreshIntervalSeconds = "execution_status_refresh_interval_seconds"
// QuotaUpdateProvider is the provider for updating quota, currently support Redis and DB
QuotaUpdateProvider = "quota_update_provider"
// IllegalCharsInUsername is the illegal chars in username
IllegalCharsInUsername = `,"~#%$`
)

10
src/common/utils/utils.go
View File

@ -247,16 +247,6 @@ func IsIllegalLength(s string, min int, max int) bool {
return (len(s) < min || len(s) > max)
}
// IsContainIllegalChar ...
func IsContainIllegalChar(s string, illegalChar []string) bool {
for _, c := range illegalChar {
if strings.Contains(s, c) {
return true
}
}
return false
}
// ParseJSONInt ...
func ParseJSONInt(value interface{}) (int, bool) {
switch v := value.(type) {

4
src/core/controllers/oidc.go
View File

@ -247,8 +247,8 @@ func (oc *OIDCController) Onboard() {
oc.SendBadRequestError(errors.New("username with illegal length"))
return
}
if utils.IsContainIllegalChar(username, []string{",", "~", "#", "$", "%"}) {
oc.SendBadRequestError(errors.New("username contains illegal characters"))
if strings.ContainsAny(username, common.IllegalCharsInUsername) {
oc.SendBadRequestError(errors.Errorf("username %v contains illegal characters: %v", username, common.IllegalCharsInUsername))
return
}

4
src/pkg/user/manager.go
View File

@ -165,6 +165,10 @@ func (m *manager) SetSysAdminFlag(ctx context.Context, id int, admin bool) error
func (m *manager) Create(ctx context.Context, user *commonmodels.User) (int, error) {
injectPasswd(user, user.Password)
// replace comma in username with underscore to avoid #19356
// if the username conflict with existing username,
// it will return error and have to update to a new username manually with sql
user.Username = strings.Replace(user.Username, ",", "_", -1)
return m.dao.Create(ctx, user)
}

24
src/pkg/user/manager_test.go
View File

@ -143,3 +143,27 @@ func TestInjectPasswd(t *testing.T) {
assert.Equal(t, "sha256", u.PasswordVersion)
assert.Equal(t, utils.Encrypt(p, u.Salt, "sha256"), u.Password)
}
func (m *mgrTestSuite) TestCreate() {
m.dao.On("Create", mock.Anything, testifymock.Anything).Return(3, nil)
u := &models.User{
Username: "test",
Email: "test@example.com",
Realname: "test",
}
id, err := m.mgr.Create(context.Background(), u)
m.Nil(err)
m.Equal(3, id)
m.Equal(u.Username, "test")
u2 := &models.User{
Username: "test,test",
Email: "test@example.com",
Realname: "test",
}
id, err = m.mgr.Create(context.Background(), u2)
m.Nil(err)
m.Equal(3, id)
m.Equal(u2.Username, "test_test", "username should be sanitized")
}

10
src/server/v2.0/handler/user.go
View File

@ -495,10 +495,18 @@ func validateUserProfile(user *commonmodels.User) error {
return errors.BadRequestError(nil).WithMessage("realname with illegal length")
}
if utils.IsContainIllegalChar(user.Realname, []string{",", "~", "#", "$", "%"}) {
if strings.ContainsAny(user.Realname, common.IllegalCharsInUsername) {
return errors.BadRequestError(nil).WithMessage("realname contains illegal characters")
}
if utils.IsIllegalLength(user.Username, 1, 255) {
return errors.BadRequestError(nil).WithMessage("usernamae 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")
}

28
src/server/v2.0/handler/user_test.go
View File

@ -2,6 +2,7 @@ package handler
import (
"context"
"fmt"
"testing"
"github.com/stretchr/testify/assert"
@ -113,3 +114,30 @@ func (uts *UserTestSuite) TestGetRandomSecret() {
func TestUserTestSuite(t *testing.T) {
suite.Run(t, &UserTestSuite{})
}
func Test_validateUserProfile(t *testing.T) {
tooLongUsername := "mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789mike012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"
type args struct {
user *commonmodels.User
}
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},
}
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))
})
}
}