Allow empty email attribute for ldap/oidc user

Define user.Email as sql.NullString to avoid unique constraint when email is empty in LDAP/OIDC
  Separate the common/models/User with the pkg/user/dao/User
  Fixes #10400
Signed-off-by: stonezdj <stonezdj@gmail.com>

make/migrations/postgresql/0061_2.3.4_schema.up.sql

@@ -0,0 +1 @@
+UPDATE harbor_user SET email=NULL WHERE email=''

src/common/dao/base.go

@@ -18,6 +18,7 @@ import (
 	"errors"
 	"fmt"
 	proModels "github.com/goharbor/harbor/src/pkg/project/models"
+	userModels "github.com/goharbor/harbor/src/pkg/user/models"
 	"strconv"
 	"sync"
 
@@ -111,7 +112,7 @@ func ClearTable(table string) error {
 	if table == proModels.ProjectTable {
 		sql = fmt.Sprintf("delete from %s where project_id > 1", table)
 	}
-	if table == models.UserTable {
+	if table == userModels.UserTable {
 		sql = fmt.Sprintf("delete from %s where user_id > 2", table)
 	}
 	if table == "project_member" { // make sure admin in library

src/common/dao/testutils.go

@@ -135,12 +135,6 @@ func ExecuteBatchSQL(sqls []string) {
 	}
 }
 
-// CleanUser - Clean this user information from DB, this is a shortcut for UT.
-func CleanUser(id int64) error {
-	_, err := GetOrmer().QueryTable(&models.User{}).Filter("UserID", id).Delete()
-	return err
-}
-
 // ArrayEqual ...
 func ArrayEqual(arrayA, arrayB []int) bool {
 	if len(arrayA) != len(arrayB) {

src/common/models/base.go

@@ -20,7 +20,6 @@ import (
 
 func init() {
 	orm.RegisterModel(
-		new(User),
 		new(Role),
 		new(ResourceLabel),
 		new(OIDCUser),

src/common/models/user.go

@@ -15,58 +15,39 @@
 package models
 
 import (
-	"context"
 	"time"
-
-	"github.com/astaxie/beego/orm"
 )
 
-// UserTable is the name of table in DB that holds the user object
-const UserTable = "harbor_user"
-
 // User holds the details of a user.
 type User struct {
-	UserID          int    `orm:"pk;auto;column(user_id)" json:"user_id"`
+	UserID          int    `json:"user_id"`
-	Username        string `orm:"column(username)" json:"username" sort:"default"`
+	Username        string `json:"username" sort:"default"`
-	Email           string `orm:"column(email)" json:"email"`
+	Email           string `json:"email"`
-	Password        string `orm:"column(password)" json:"password"`
+	Password        string `json:"password"`
-	PasswordVersion string `orm:"column(password_version)" json:"password_version"`
+	PasswordVersion string `json:"password_version"`
-	Realname        string `orm:"column(realname)" json:"realname"`
+	Realname        string `json:"realname"`
-	Comment         string `orm:"column(comment)" json:"comment"`
+	Comment         string `json:"comment"`
-	Deleted         bool   `orm:"column(deleted)" json:"deleted"`
+	Deleted         bool   `json:"deleted"`
-	Rolename        string `orm:"-" json:"role_name"`
+	Rolename        string `json:"role_name"`
-	// if this field is named as "RoleID", beego orm can not map role_id
+	Role            int    `json:"role_id"`
-	// to it.
+	SysAdminFlag    bool   `json:"sysadmin_flag"`
-	Role         int  `orm:"-" json:"role_id"`
-	SysAdminFlag bool `orm:"column(sysadmin_flag)" json:"sysadmin_flag"`
 	// AdminRoleInAuth to store the admin privilege granted by external authentication provider
-	AdminRoleInAuth bool      `orm:"-" json:"admin_role_in_auth"`
+	AdminRoleInAuth bool      `json:"admin_role_in_auth"`
-	ResetUUID       string    `orm:"column(reset_uuid)" json:"reset_uuid"`
+	ResetUUID       string    `json:"reset_uuid"`
-	Salt            string    `orm:"column(salt)" json:"-"`
+	Salt            string    `json:"-"`
-	CreationTime    time.Time `orm:"column(creation_time);auto_now_add" json:"creation_time"`
+	CreationTime    time.Time `json:"creation_time"`
-	UpdateTime      time.Time `orm:"column(update_time);auto_now" json:"update_time"`
+	UpdateTime      time.Time `json:"update_time"`
-	GroupIDs        []int     `orm:"-" json:"-"`
+	GroupIDs        []int     `json:"-"`
-	OIDCUserMeta    *OIDCUser `orm:"-" json:"oidc_user_meta,omitempty"`
+	OIDCUserMeta    *OIDCUser `json:"oidc_user_meta,omitempty"`
 }
 
-// TableName ...
+type Users []*User
-func (u *User) TableName() string {
-	return UserTable
-}
 
-// FilterByUsernameOrEmail generates the query setter to match username or email column to the same value
+// MapByUserID returns map which key is UserID of the user and value is the user itself
-func (u *User) FilterByUsernameOrEmail(ctx context.Context, qs orm.QuerySeter, key string, value interface{}) orm.QuerySeter {
+func (users Users) MapByUserID() map[int]*User {
-	usernameOrEmail, ok := value.(string)
+	m := map[int]*User{}
-	if !ok {
+	for _, user := range users {
-		return qs
+		m[user.UserID] = user
 	}
-	subCond := orm.NewCondition()
+	return m
-	subCond = subCond.Or("Username", usernameOrEmail).Or("Email", usernameOrEmail)
-
-	conds := qs.GetCond()
-	if conds == nil {
-		conds = orm.NewCondition()
-	}
-	qs = qs.SetCond(conds.AndCond(subCond))
-	return qs
 }

src/controller/project/controller_test.go

@@ -17,6 +17,7 @@ package project
 import (
 	"context"
 	"fmt"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"testing"
 
 	"github.com/goharbor/harbor/src/lib/errors"
@@ -24,7 +25,6 @@ import (
 	"github.com/goharbor/harbor/src/lib/q"
 	models2 "github.com/goharbor/harbor/src/pkg/allowlist/models"
 	"github.com/goharbor/harbor/src/pkg/project/models"
-	usermodels "github.com/goharbor/harbor/src/pkg/user/models"
 	ormtesting "github.com/goharbor/harbor/src/testing/lib/orm"
 	"github.com/goharbor/harbor/src/testing/mock"
 	allowlisttesting "github.com/goharbor/harbor/src/testing/pkg/allowlist"
@@ -122,8 +122,8 @@ func (suite *ControllerTestSuite) TestWithOwner() {
 	}, nil)
 
 	userMgr := &user.Manager{}
-	userMgr.On("List", ctx, mock.Anything).Return(usermodels.Users{
+	userMgr.On("List", ctx, mock.Anything).Return(commonmodels.Users{
-		&usermodels.User{UserID: 1, Username: "admin"},
+		&commonmodels.User{UserID: 1, Username: "admin"},
 	}, nil)
 
 	c := controller{projectMgr: mgr, userMgr: userMgr}

src/controller/user/controller.go

@@ -44,29 +44,29 @@ type Controller interface {
 	// UpdatePassword ...
 	UpdatePassword(ctx context.Context, id int, password string) error
 	// List ...
-	List(ctx context.Context, query *q.Query, options ...models.Option) (models.Users, error)
+	List(ctx context.Context, query *q.Query, options ...models.Option) ([]*commonmodels.User, error)
 	// Create ...
-	Create(ctx context.Context, u *models.User) (int, error)
+	Create(ctx context.Context, u *commonmodels.User) (int, error)
 	// Count ...
 	Count(ctx context.Context, query *q.Query) (int64, error)
 	// Get ...
-	Get(ctx context.Context, id int, opt *Option) (*models.User, error)
+	Get(ctx context.Context, id int, opt *Option) (*commonmodels.User, error)
 	// GetByName gets the user model by username, it only supports getting the basic and does not support opt
-	GetByName(ctx context.Context, username string) (*models.User, error)
+	GetByName(ctx context.Context, username string) (*commonmodels.User, error)
 	// GetBySubIss gets the user model by subject and issuer, the result will contain the basic user model and does not support opt
-	GetBySubIss(ctx context.Context, sub, iss string) (*models.User, error)
+	GetBySubIss(ctx context.Context, sub, iss string) (*commonmodels.User, error)
 	// Delete ...
 	Delete(ctx context.Context, id int) error
 	// UpdateProfile update the profile based on the ID and data in the model in parm, only a subset of attributes in the model
 	// will be update, see the implementation of manager.
-	UpdateProfile(ctx context.Context, u *models.User, cols ...string) error
+	UpdateProfile(ctx context.Context, u *commonmodels.User, cols ...string) error
 	// SetCliSecret sets the OIDC CLI secret for a user
 	SetCliSecret(ctx context.Context, id int, secret string) error
 	// UpdateOIDCMeta updates the OIDC metadata of a user, if the cols are not provided, by default the field of token and secret will be updated
 	UpdateOIDCMeta(ctx context.Context, ou *commonmodels.OIDCUser, cols ...string) error
 	// OnboardOIDCUser inserts the record for basic user info and the oidc metadata
 	// if the onboard process is successful the input parm of user model will be populated with user id
-	OnboardOIDCUser(ctx context.Context, u *models.User) error
+	OnboardOIDCUser(ctx context.Context, u *commonmodels.User) error
 }
 
 // NewController ...
@@ -97,7 +97,7 @@ func (c *controller) UpdateOIDCMeta(ctx context.Context, ou *commonmodels.OIDCUs
 	return c.oidcMetaMgr.Update(ctx, ou, cols...)
 }
 
-func (c *controller) OnboardOIDCUser(ctx context.Context, u *models.User) error {
+func (c *controller) OnboardOIDCUser(ctx context.Context, u *commonmodels.User) error {
 	if u == nil {
 		return errors.BadRequestError(nil).WithMessage("user model is nil")
 	}
@@ -119,7 +119,7 @@ func (c *controller) OnboardOIDCUser(ctx context.Context, u *models.User) error
 	return nil
 }
 
-func (c *controller) GetBySubIss(ctx context.Context, sub, iss string) (*models.User, error) {
+func (c *controller) GetBySubIss(ctx context.Context, sub, iss string) (*commonmodels.User, error) {
 	oidcMeta, err := c.oidcMetaMgr.GetBySubIss(ctx, sub, iss)
 	if err != nil {
 		return nil, err
@@ -127,7 +127,7 @@ func (c *controller) GetBySubIss(ctx context.Context, sub, iss string) (*models.
 	return c.Get(ctx, oidcMeta.UserID, nil)
 }
 
-func (c *controller) GetByName(ctx context.Context, username string) (*models.User, error) {
+func (c *controller) GetByName(ctx context.Context, username string) (*commonmodels.User, error) {
 	return c.mgr.GetByName(ctx, username)
 }
 
@@ -135,15 +135,15 @@ func (c *controller) SetCliSecret(ctx context.Context, id int, secret string) er
 	return c.oidcMetaMgr.SetCliSecretByUserID(ctx, id, secret)
 }
 
-func (c *controller) Create(ctx context.Context, u *models.User) (int, error) {
+func (c *controller) Create(ctx context.Context, u *commonmodels.User) (int, error) {
 	return c.mgr.Create(ctx, u)
 }
 
-func (c *controller) UpdateProfile(ctx context.Context, u *models.User, cols ...string) error {
+func (c *controller) UpdateProfile(ctx context.Context, u *commonmodels.User, cols ...string) error {
 	return c.mgr.UpdateProfile(ctx, u, cols...)
 }
 
-func (c *controller) Get(ctx context.Context, id int, opt *Option) (*models.User, error) {
+func (c *controller) Get(ctx context.Context, id int, opt *Option) (*commonmodels.User, error) {
 	u, err := c.mgr.Get(ctx, id)
 	if err != nil {
 		return nil, err
@@ -181,7 +181,7 @@ func (c *controller) Delete(ctx context.Context, id int) error {
 	return c.mgr.Delete(ctx, id)
 }
 
-func (c *controller) List(ctx context.Context, query *q.Query, options ...models.Option) (models.Users, error) {
+func (c *controller) List(ctx context.Context, query *q.Query, options ...models.Option) ([]*commonmodels.User, error) {
 	return c.mgr.List(ctx, query, options...)
 }
 

src/core/auth/ldap/ldap_test.go

@@ -31,6 +31,7 @@ import (
 	memberModels "github.com/goharbor/harbor/src/pkg/member/models"
 	"github.com/goharbor/harbor/src/pkg/project"
 	userpkg "github.com/goharbor/harbor/src/pkg/user"
+	userDao "github.com/goharbor/harbor/src/pkg/user/dao"
 	"github.com/goharbor/harbor/src/pkg/usergroup"
 	ugModel "github.com/goharbor/harbor/src/pkg/usergroup/model"
 	"github.com/stretchr/testify/assert"
@@ -241,7 +242,7 @@ func TestOnBoardUser_02(t *testing.T) {
 	}
 
 	assert.Equal(t, "", user.Email)
-	dao.CleanUser(int64(user.UserID))
+	userDao.New().Delete(ctx, user.UserID)
 }
 
 func TestOnBoardUser_03(t *testing.T) {
@@ -260,7 +261,7 @@ func TestOnBoardUser_03(t *testing.T) {
 	}
 
 	assert.Equal(t, "sample03@example.com", user.Email)
-	dao.CleanUser(int64(user.UserID))
+	userDao.New().Delete(ctx, user.UserID)
 }
 
 func TestAuthenticateHelperOnBoardUser(t *testing.T) {
@@ -363,14 +364,14 @@ func TestPostAuthentication(t *testing.T) {
 		t.Fatalf("Failed to get user, error %v", err)
 	}
 	assert.EqualValues("test003@example.com", dbUser.Email)
-	dao.CleanUser(int64(dbUser.UserID))
+	userDao.New().Delete(ctx, dbUser.UserID)
 }
 
 func TestSearchAndOnBoardUser(t *testing.T) {
 	ctx := orm.Context()
 
 	userID, err := auth.SearchAndOnBoardUser(ctx, "mike02")
-	defer dao.CleanUser(int64(userID))
+	defer userDao.New().Delete(ctx, userID)
 	if err != nil {
 		t.Errorf("Error occurred when SearchAndOnBoardUser: %v", err)
 	}

src/core/auth/uaa/uaa_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/goharbor/harbor/src/common/utils/uaa"
 	"github.com/goharbor/harbor/src/lib/config"
 	userpkg "github.com/goharbor/harbor/src/pkg/user"
+	userModels "github.com/goharbor/harbor/src/pkg/user/models"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -90,7 +91,7 @@ func TestAuthenticate(t *testing.T) {
 	u2, err2 := auth.Authenticate(ctx, m2)
 	assert.NotNil(err2)
 	assert.Nil(u2)
-	err3 := dao.ClearTable(models.UserTable)
+	err3 := dao.ClearTable(userModels.UserTable)
 	assert.Nil(err3)
 }
 
@@ -116,7 +117,7 @@ func TestOnBoardUser(t *testing.T) {
 	assert.Equal("test", user.Realname)
 	assert.Equal("test", user.Username)
 	assert.Equal("", user.Email)
-	err3 := dao.ClearTable(models.UserTable)
+	err3 := dao.ClearTable(userModels.UserTable)
 	assert.Nil(err3)
 }
 
@@ -155,7 +156,7 @@ func TestPostAuthenticate(t *testing.T) {
 	assert.Equal(user3.UserID, um3.UserID)
 	assert.Equal("", user3.Email)
 	assert.Equal("test", user3.Realname)
-	err4 := dao.ClearTable(models.UserTable)
+	err4 := dao.ClearTable(userModels.UserTable)
 	assert.Nil(err4)
 }
 

src/core/main.go

@@ -32,7 +32,7 @@ import (
 
 	"github.com/goharbor/harbor/src/common/dao"
 	common_http "github.com/goharbor/harbor/src/common/http"
-	"github.com/goharbor/harbor/src/common/models"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	configCtl "github.com/goharbor/harbor/src/controller/config"
 	_ "github.com/goharbor/harbor/src/controller/event/handler"
 	"github.com/goharbor/harbor/src/controller/health"
@@ -122,7 +122,7 @@ func main() {
 		if err != nil {
 			panic("bad _REDIS_URL:" + redisURL)
 		}
-		gob.Register(models.User{})
+		gob.Register(commonmodels.User{})
 		if u.Scheme == "redis+sentinel" {
 			ps := strings.Split(u.Path, "/")
 			if len(ps) < 2 {

src/pkg/member/dao/dao_test.go

@@ -15,14 +15,15 @@
 package dao
 
 import (
+	"database/sql"
 	"github.com/goharbor/harbor/src/common"
 	_ "github.com/goharbor/harbor/src/common/dao"
 	testDao "github.com/goharbor/harbor/src/common/dao"
-	comModels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/pkg/member/models"
 	"github.com/goharbor/harbor/src/pkg/project"
 	"github.com/goharbor/harbor/src/pkg/user"
+	userDao "github.com/goharbor/harbor/src/pkg/user/dao"
 	"github.com/goharbor/harbor/src/pkg/usergroup"
 	ugModel "github.com/goharbor/harbor/src/pkg/usergroup/model"
 	htesting "github.com/goharbor/harbor/src/testing"
@@ -125,9 +126,9 @@ func (s *DaoTestSuite) TestUpdateProjectMemberRole() {
 	proj, err := s.projectMgr.Get(ctx, "member_test_01")
 	s.Nil(err)
 	s.NotNil(proj)
-	user := comModels.User{
+	user := userDao.User{
 		Username: "pm_sample",
-		Email:    "pm_sample@example.com",
+		Email:    sql.NullString{String: "pm_sample@example.com", Valid: true},
 		Realname: "pm_sample",
 		Password: "1234567d",
 	}

src/pkg/user/dao/dao.go

@@ -16,23 +16,24 @@ package dao
 
 import (
 	"context"
-
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
-	"github.com/goharbor/harbor/src/pkg/user/models"
 )
 
 // DAO is the data access object interface for user
 type DAO interface {
 	// Create create a user record in the table, it will return the ID of the user
-	Create(ctx context.Context, user *models.User) (int, error)
+	Create(ctx context.Context, user *commonmodels.User) (int, error)
 	// List list users
-	List(ctx context.Context, query *q.Query) ([]*models.User, error)
+	List(ctx context.Context, query *q.Query) ([]*commonmodels.User, error)
 	// Count counts the number of users
 	Count(ctx context.Context, query *q.Query) (int64, error)
 	// Update updates the user record based on the model the parm props are the columns will be updated
-	Update(ctx context.Context, user *models.User, props ...string) error
+	Update(ctx context.Context, user *commonmodels.User, props ...string) error
+	// Delete delete user
+	Delete(ctx context.Context, userID int) error
 }
 
 // New returns an instance of the default DAO
@@ -41,22 +42,33 @@ func New() DAO {
 }
 
 func init() {
-	// TODO	beegoorm.RegisterModel(new(models.User))
+	orm.RegisterModel(
+		new(User),
+	)
 }
 
 type dao struct{}
 
+func (d *dao) Delete(ctx context.Context, userID int) error {
+	ormer, err := orm.FromContext(ctx)
+	if err != nil {
+		return err
+	}
+	_, err = ormer.Delete(&User{UserID: userID})
+	return err
+}
+
 func (d *dao) Count(ctx context.Context, query *q.Query) (int64, error) {
 	query = q.MustClone(query)
 	query.Keywords["deleted"] = false
-	qs, err := orm.QuerySetterForCount(ctx, &models.User{}, query)
+	qs, err := orm.QuerySetterForCount(ctx, &User{}, query)
 	if err != nil {
 		return 0, err
 	}
 	return qs.Count()
 }
 
-func (d *dao) Create(ctx context.Context, user *models.User) (int, error) {
+func (d *dao) Create(ctx context.Context, user *commonmodels.User) (int, error) {
 	if user.UserID > 0 {
 		return 0, errors.BadRequestError(nil).WithMessage("user ID is set when creating user: %d", user.UserID)
 	}
@@ -64,19 +76,19 @@ func (d *dao) Create(ctx context.Context, user *models.User) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	id, err := ormer.Insert(user)
+	id, err := ormer.Insert(toDBUser(user))
 	if err != nil {
 		return 0, orm.WrapConflictError(err, "user %s or email %s already exists", user.Username, user.Email)
 	}
 	return int(id), nil
 }
 
-func (d *dao) Update(ctx context.Context, user *models.User, props ...string) error {
+func (d *dao) Update(ctx context.Context, user *commonmodels.User, props ...string) error {
 	ormer, err := orm.FromContext(ctx)
 	if err != nil {
 		return err
 	}
-	n, err := ormer.Update(user, props...)
+	n, err := ormer.Update(toDBUser(user), props...)
 	if err != nil {
 		return err
 	}
@@ -87,19 +99,25 @@ func (d *dao) Update(ctx context.Context, user *models.User, props ...string) er
 }
 
 // List list users
-func (d *dao) List(ctx context.Context, query *q.Query) ([]*models.User, error) {
+func (d *dao) List(ctx context.Context, query *q.Query) ([]*commonmodels.User, error) {
 	query = q.MustClone(query)
 	query.Keywords["deleted"] = false
 
-	qs, err := orm.QuerySetter(ctx, &models.User{}, query)
+	qs, err := orm.QuerySetter(ctx, &User{}, query)
 	if err != nil {
 		return nil, err
 	}
 
-	var users []*models.User
+	var users []*User
 	if _, err := qs.All(&users); err != nil {
 		return nil, err
 	}
 
-	return users, nil
+	var retUsers []*commonmodels.User
+	for _, u := range users {
+		mU := toCommonUser(u)
+		retUsers = append(retUsers, mU)
+	}
+
+	return retUsers, nil
 }

src/pkg/user/dao/dao_test.go

@@ -17,9 +17,9 @@ import (
 	"fmt"
 	"testing"
 
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
-	"github.com/goharbor/harbor/src/pkg/user/models"
 	htesting "github.com/goharbor/harbor/src/testing"
 	"github.com/stretchr/testify/suite"
 )
@@ -48,7 +48,7 @@ func (suite *DaoTestSuite) TestCount() {
 
 		n, err := suite.dao.Count(ctx, nil)
 		suite.Nil(err)
-		id, err := suite.dao.Create(ctx, &models.User{
+		id, err := suite.dao.Create(ctx, &commonmodels.User{
 			Username:        "testuser2",
 			Realname:        "user test",
 			Email:           "testuser@test.com",
@@ -60,7 +60,7 @@ func (suite *DaoTestSuite) TestCount() {
 		n2, err := suite.dao.Count(ctx, nil)
 		suite.Nil(err)
 		suite.Equal(n+1, n2)
-		err2 := suite.dao.Update(ctx, &models.User{
+		err2 := suite.dao.Update(ctx, &commonmodels.User{
 			UserID:  id,
 			Deleted: true,
 		})
@@ -86,7 +86,7 @@ func (suite *DaoTestSuite) TestList() {
 		suite.Nil(err)
 		suite.Len(users, 1)
 	}
-	id, err := suite.dao.Create(ctx, &models.User{
+	id, err := suite.dao.Create(ctx, &commonmodels.User{
 		Username:        "list_test",
 		Realname:        "list test",
 		Email:           "list_test@test.com",
@@ -116,12 +116,12 @@ func (suite *DaoTestSuite) TestList() {
 func (suite *DaoTestSuite) TestCreate() {
 	cases := []struct {
 		name     string
-		input    *models.User
+		input    *commonmodels.User
 		hasError bool
 	}{
 		{
 			name: "create with user ID",
-			input: &models.User{
+			input: &commonmodels.User{
 				UserID:          3,
 				Username:        "testuser",
 				Realname:        "user test",
@@ -133,7 +133,7 @@ func (suite *DaoTestSuite) TestCreate() {
 		},
 		{
 			name: "create without user ID",
-			input: &models.User{
+			input: &commonmodels.User{
 				Username:        "testuser",
 				Realname:        "user test",
 				Email:           "testuser@test.com",
@@ -142,6 +142,28 @@ func (suite *DaoTestSuite) TestCreate() {
 			},
 			hasError: false,
 		},
+		{
+			name: "create with empty email_1",
+			input: &commonmodels.User{
+				Username:        "emptyemail1",
+				Realname:        "empty test",
+				Email:           "",
+				Password:        "somepassword",
+				PasswordVersion: "sha256",
+			},
+			hasError: false,
+		},
+		{
+			name: "create with empty email_2",
+			input: &commonmodels.User{
+				Username:        "emptyemail2",
+				Realname:        "empty test2",
+				Email:           "",
+				Password:        "somepassword",
+				PasswordVersion: "sha256",
+			},
+			hasError: false,
+		},
 	}
 	for _, c := range cases {
 		suite.Run(c.name, func() {

src/pkg/user/dao/user.go

@@ -0,0 +1,110 @@
+//  Copyright Project Harbor Authors
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package dao
+
+import (
+	"context"
+	"database/sql"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/lib/orm"
+	"github.com/goharbor/harbor/src/pkg/user/models"
+	"time"
+)
+
+// User holds the details of a user.
+// only used in DAO, for other place, use the User model in common/models
+type User struct {
+	UserID   int    `orm:"pk;auto;column(user_id)" json:"user_id"`
+	Username string `orm:"column(username)" json:"username" sort:"default"`
+	// Email defined as sql.NullString because sometimes email is missing in LDAP/OIDC auth,
+	// set it to null to avoid unique constraint check
+	Email           sql.NullString `orm:"column(email)" json:"email"`
+	Password        string         `orm:"column(password)" json:"password"`
+	PasswordVersion string         `orm:"column(password_version)" json:"password_version"`
+	Realname        string         `orm:"column(realname)" json:"realname"`
+	Comment         string         `orm:"column(comment)" json:"comment"`
+	Deleted         bool           `orm:"column(deleted)" json:"deleted"`
+	SysAdminFlag    bool           `orm:"column(sysadmin_flag)" json:"sysadmin_flag"`
+	ResetUUID       string         `orm:"column(reset_uuid)" json:"reset_uuid"`
+	Salt            string         `orm:"column(salt)" json:"-"`
+	CreationTime    time.Time      `orm:"column(creation_time);auto_now_add" json:"creation_time"`
+	UpdateTime      time.Time      `orm:"column(update_time);auto_now" json:"update_time"`
+}
+
+// TableName ...
+func (u *User) TableName() string {
+	return models.UserTable
+}
+
+// toDBUser ...
+func toDBUser(u *commonmodels.User) *User {
+	user := &User{}
+
+	user.UserID = u.UserID
+	user.Username = u.Username
+	user.Email = sql.NullString{}
+	if u.Email != "" {
+		user.Email = sql.NullString{String: u.Email, Valid: true}
+	}
+	user.Password = u.Password
+	user.PasswordVersion = u.PasswordVersion
+	user.Realname = u.Realname
+	user.Comment = u.Comment
+	user.Deleted = u.Deleted
+	user.SysAdminFlag = u.SysAdminFlag
+	user.ResetUUID = u.ResetUUID
+	user.Salt = u.Salt
+	user.CreationTime = u.CreationTime
+	user.UpdateTime = u.UpdateTime
+	return user
+}
+
+// toCommonUser ...
+func toCommonUser(u *User) *commonmodels.User {
+	user := &commonmodels.User{}
+	user.UserID = u.UserID
+	user.Username = u.Username
+	user.Email = u.Email.String
+
+	user.Password = u.Password
+	user.PasswordVersion = u.PasswordVersion
+	user.Realname = u.Realname
+	user.Comment = u.Comment
+	user.Deleted = u.Deleted
+	user.SysAdminFlag = u.SysAdminFlag
+	user.ResetUUID = u.ResetUUID
+	user.Salt = u.Salt
+	user.CreationTime = u.CreationTime
+	user.UpdateTime = u.UpdateTime
+	user.GroupIDs = make([]int, 0)
+	return user
+}
+
+// FilterByUsernameOrEmail generates the query setter to match username or email column to the same value
+func (u *User) FilterByUsernameOrEmail(ctx context.Context, qs orm.QuerySeter, key string, value interface{}) orm.QuerySeter {
+	usernameOrEmail, ok := value.(string)
+	if !ok {
+		return qs
+	}
+	subCond := orm.NewCondition()
+	subCond = subCond.Or("Username", usernameOrEmail).Or("Email", usernameOrEmail)
+
+	conds := qs.GetCond()
+	if conds == nil {
+		conds = orm.NewCondition()
+	}
+	qs = qs.SetCond(conds.AndCond(subCond))
+	return qs
+}

src/pkg/user/manager.go

@@ -17,6 +17,7 @@ package user
 import (
 	"context"
 	"fmt"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/errors"
@@ -34,30 +35,30 @@ var (
 // Manager is used for user management
 type Manager interface {
 	// Get get user by user id
-	Get(ctx context.Context, id int) (*models.User, error)
+	Get(ctx context.Context, id int) (*commonmodels.User, error)
 	// GetByName get user by username, it will return an error if the user does not exist
-	GetByName(ctx context.Context, username string) (*models.User, error)
+	GetByName(ctx context.Context, username string) (*commonmodels.User, error)
 	// List users according to the query
-	List(ctx context.Context, query *q.Query, options ...models.Option) (models.Users, error)
+	List(ctx context.Context, query *q.Query, options ...models.Option) (commonmodels.Users, error)
 	// Count counts the number of users according to the query
 	Count(ctx context.Context, query *q.Query) (int64, error)
 	// Create creates the user, the password of input should be plaintext
-	Create(ctx context.Context, user *models.User) (int, error)
+	Create(ctx context.Context, user *commonmodels.User) (int, error)
 	// Delete deletes the user by updating user's delete flag and update the name and Email
 	Delete(ctx context.Context, id int) error
 	// SetSysAdminFlag sets the system admin flag of the user in local DB
 	SetSysAdminFlag(ctx context.Context, id int, admin bool) error
 	// UpdateProfile updates the user's profile
-	UpdateProfile(ctx context.Context, user *models.User, col ...string) error
+	UpdateProfile(ctx context.Context, user *commonmodels.User, col ...string) error
 	// UpdatePassword updates user's password
 	UpdatePassword(ctx context.Context, id int, newPassword string) error
 	// MatchLocalPassword tries to match the record in DB based on the input, the first return value is
 	// the user model corresponding to the entry in DB
-	MatchLocalPassword(ctx context.Context, username, password string) (*models.User, error)
+	MatchLocalPassword(ctx context.Context, username, password string) (*commonmodels.User, error)
 	// Onboard will check if a user exists in user table, if not insert the user and
 	// put the id in the pointer of user model, if it does exist, return the user's profile.
 	// This is used for ldap and uaa authentication, such the user can have an ID in Harbor.
-	Onboard(ctx context.Context, user *models.User) error
+	Onboard(ctx context.Context, user *commonmodels.User) error
 }
 
 // New returns a default implementation of Manager
@@ -69,7 +70,7 @@ type manager struct {
 	dao dao.DAO
 }
 
-func (m *manager) Onboard(ctx context.Context, user *models.User) error {
+func (m *manager) Onboard(ctx context.Context, user *commonmodels.User) error {
 	u, err := m.GetByName(ctx, user.Username)
 	if err == nil {
 		user.Email = u.Email
@@ -101,7 +102,7 @@ func (m *manager) Delete(ctx context.Context, id int) error {
 	return m.dao.Update(ctx, u, "username", "email", "deleted")
 }
 
-func (m *manager) MatchLocalPassword(ctx context.Context, usernameOrEmail, password string) (*models.User, error) {
+func (m *manager) MatchLocalPassword(ctx context.Context, usernameOrEmail, password string) (*commonmodels.User, error) {
 	l, err := m.dao.List(ctx, q.New(q.KeyWords{"username_or_email": usernameOrEmail}))
 	if err != nil {
 		return nil, err
@@ -119,7 +120,7 @@ func (m *manager) Count(ctx context.Context, query *q.Query) (int64, error) {
 	return m.dao.Count(ctx, query)
 }
 
-func (m *manager) UpdateProfile(ctx context.Context, user *models.User, cols ...string) error {
+func (m *manager) UpdateProfile(ctx context.Context, user *commonmodels.User, cols ...string) error {
 	if cols == nil || len(cols) == 0 {
 		cols = []string{"Email", "Realname", "Comment"}
 	}
@@ -127,7 +128,7 @@ func (m *manager) UpdateProfile(ctx context.Context, user *models.User, cols ...
 }
 
 func (m *manager) UpdatePassword(ctx context.Context, id int, newPassword string) error {
-	user := &models.User{
+	user := &commonmodels.User{
 		UserID: id,
 	}
 	injectPasswd(user, newPassword)
@@ -135,20 +136,20 @@ func (m *manager) UpdatePassword(ctx context.Context, id int, newPassword string
 }
 
 func (m *manager) SetSysAdminFlag(ctx context.Context, id int, admin bool) error {
-	u := &models.User{
+	u := &commonmodels.User{
 		UserID:       id,
 		SysAdminFlag: admin,
 	}
 	return m.dao.Update(ctx, u, "sysadmin_flag")
 }
 
-func (m *manager) Create(ctx context.Context, user *models.User) (int, error) {
+func (m *manager) Create(ctx context.Context, user *commonmodels.User) (int, error) {
 	injectPasswd(user, user.Password)
 	return m.dao.Create(ctx, user)
 }
 
 // Get get user by user id
-func (m *manager) Get(ctx context.Context, id int) (*models.User, error) {
+func (m *manager) Get(ctx context.Context, id int) (*commonmodels.User, error) {
 	users, err := m.dao.List(ctx, q.New(q.KeyWords{"user_id": id}))
 	if err != nil {
 		return nil, err
@@ -162,7 +163,7 @@ func (m *manager) Get(ctx context.Context, id int) (*models.User, error) {
 }
 
 // GetByName get user by username
-func (m *manager) GetByName(ctx context.Context, username string) (*models.User, error) {
+func (m *manager) GetByName(ctx context.Context, username string) (*commonmodels.User, error) {
 	users, err := m.dao.List(ctx, q.New(q.KeyWords{"username": username}))
 	if err != nil {
 		return nil, err
@@ -176,7 +177,7 @@ func (m *manager) GetByName(ctx context.Context, username string) (*models.User,
 }
 
 // List users according to the query
-func (m *manager) List(ctx context.Context, query *q.Query, options ...models.Option) (models.Users, error) {
+func (m *manager) List(ctx context.Context, query *q.Query, options ...models.Option) (commonmodels.Users, error) {
 	query = q.MustClone(query)
 	for key := range query.Keywords {
 		str := strings.ToLower(key)
@@ -195,7 +196,7 @@ func (m *manager) List(ctx context.Context, query *q.Query, options ...models.Op
 	return m.dao.List(ctx, query)
 }
 
-func injectPasswd(u *models.User, password string) {
+func injectPasswd(u *commonmodels.User, password string) {
 	salt := utils.GenerateRandomString()
 	u.Password = utils.Encrypt(password, salt, utils.SHA256)
 	u.Salt = salt

src/pkg/user/models/model.go

@@ -14,30 +14,13 @@
 
 package models
 
-import (
+// UserTable is the name of table in DB that holds the user object
-	// "time"
+const UserTable = "harbor_user"
-
-	commonmodels "github.com/goharbor/harbor/src/common/models"
-)
-
-// User ...
-type User = commonmodels.User
-
-// Users the collection for User
-type Users []*User
-
-// MapByUserID returns map which key is UserID of the user and value is the user itself
-func (users Users) MapByUserID() map[int]*User {
-	m := map[int]*User{}
-	for _, user := range users {
-		m[user.UserID] = user
-	}
-
-	return m
-}
 
+// Option ...
 type Option func(*Options)
 
+// Options ...
 type Options struct {
 	IncludeDefaultAdmin bool
 }

src/server/v2.0/handler/model/user.go

@@ -2,13 +2,13 @@ package model
 
 import (
 	"github.com/go-openapi/strfmt"
-	"github.com/goharbor/harbor/src/pkg/user/models"
+	comModels "github.com/goharbor/harbor/src/common/models"
 	svrmodels "github.com/goharbor/harbor/src/server/v2.0/models"
 )
 
 // User ...
 type User struct {
-	*models.User
+	*comModels.User
 }
 
 // ToSearchRespItem ...

src/server/v2.0/handler/user.go

@@ -17,6 +17,7 @@ package handler
 import (
 	"context"
 	"fmt"
+	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"regexp"
 	"strings"
 
@@ -34,7 +35,6 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/permission/types"
-	usermodels "github.com/goharbor/harbor/src/pkg/user/models"
 	"github.com/goharbor/harbor/src/server/v2.0/handler/model"
 	"github.com/goharbor/harbor/src/server/v2.0/models"
 	operation "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/user"
@@ -77,7 +77,7 @@ func (u *usersAPI) CreateUser(ctx context.Context, params operation.CreateUserPa
 	if err := requireValidSecret(params.UserReq.Password); err != nil {
 		return u.SendError(ctx, err)
 	}
-	m := &usermodels.User{
+	m := &commonmodels.User{
 		Username: params.UserReq.Username,
 		Realname: params.UserReq.Realname,
 		Email:    params.UserReq.Email,
@@ -241,7 +241,7 @@ func (u *usersAPI) UpdateUserProfile(ctx context.Context, params operation.Updat
 	if err := u.requireModifiable(ctx, uid); err != nil {
 		return u.SendError(ctx, err)
 	}
-	m := &usermodels.User{
+	m := &commonmodels.User{
 		UserID:   uid,
 		Realname: params.Profile.Realname,
 		Email:    params.Profile.Email,
@@ -446,7 +446,7 @@ func requireValidSecret(in string) error {
 	return errors.BadRequestError(nil).WithMessage("the password or secret must be longer than 8 chars with at least 1 uppercase letter, 1 lowercase letter and 1 number")
 }
 
-func validateUserProfile(user *usermodels.User) error {
+func validateUserProfile(user *commonmodels.User) 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")

src/testing/controller/user/controller.go

@@ -146,7 +146,7 @@ func (_m *Controller) GetBySubIss(ctx context.Context, sub string, iss string) (
 }
 
 // List provides a mock function with given fields: ctx, query, options
-func (_m *Controller) List(ctx context.Context, query *q.Query, options ...usermodels.Option) (usermodels.Users, error) {
+func (_m *Controller) List(ctx context.Context, query *q.Query, options ...usermodels.Option) ([]*models.User, error) {
 	_va := make([]interface{}, len(options))
 	for _i := range options {
 		_va[_i] = options[_i]
@@ -156,12 +156,12 @@ func (_m *Controller) List(ctx context.Context, query *q.Query, options ...userm
 	_ca = append(_ca, _va...)
 	ret := _m.Called(_ca...)
 
-	var r0 usermodels.Users
+	var r0 []*models.User
-	if rf, ok := ret.Get(0).(func(context.Context, *q.Query, ...usermodels.Option) usermodels.Users); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *q.Query, ...usermodels.Option) []*models.User); ok {
 		r0 = rf(ctx, query, options...)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(usermodels.Users)
+			r0 = ret.Get(0).([]*models.User)
 		}
 	}
 

src/testing/pkg/user/dao/dao.go

@@ -59,6 +59,20 @@ func (_m *DAO) Create(ctx context.Context, user *models.User) (int, error) {
 	return r0, r1
 }
 
+// Delete provides a mock function with given fields: ctx, userID
+func (_m *DAO) Delete(ctx context.Context, userID int) error {
+	ret := _m.Called(ctx, userID)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int) error); ok {
+		r0 = rf(ctx, userID)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // List provides a mock function with given fields: ctx, query
 func (_m *DAO) List(ctx context.Context, query *q.Query) ([]*models.User, error) {
 	ret := _m.Called(ctx, query)

src/testing/pkg/user/manager.go

@@ -121,7 +121,7 @@ func (_m *Manager) GetByName(ctx context.Context, username string) (*models.User
 }
 
 // List provides a mock function with given fields: ctx, query, options
-func (_m *Manager) List(ctx context.Context, query *q.Query, options ...usermodels.Option) (usermodels.Users, error) {
+func (_m *Manager) List(ctx context.Context, query *q.Query, options ...usermodels.Option) (models.Users, error) {
 	_va := make([]interface{}, len(options))
 	for _i := range options {
 		_va[_i] = options[_i]
@@ -131,12 +131,12 @@ func (_m *Manager) List(ctx context.Context, query *q.Query, options ...usermode
 	_ca = append(_ca, _va...)
 	ret := _m.Called(_ca...)
 
-	var r0 usermodels.Users
+	var r0 models.Users
-	if rf, ok := ret.Get(0).(func(context.Context, *q.Query, ...usermodels.Option) usermodels.Users); ok {
+	if rf, ok := ret.Get(0).(func(context.Context, *q.Query, ...usermodels.Option) models.Users); ok {
 		r0 = rf(ctx, query, options...)
 	} else {
 		if ret.Get(0) != nil {
-			r0 = ret.Get(0).(usermodels.Users)
+			r0 = ret.Get(0).(models.Users)
 		}
 	}