Merge pull request #14783 from reasonerjt/clean-common-dao-user

Clean up user related funcs in common/dao

src/common/dao/config_test.go

@@ -1,73 +0,0 @@
-// 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 (
-	"github.com/goharbor/harbor/src/lib/orm"
-	"testing"
-
-	"github.com/goharbor/harbor/src/common/models"
-)
-
-func TestAuthModeCanBeModified(t *testing.T) {
-	ctx := orm.Context()
-	c, err := GetOrmer().QueryTable(&models.User{}).Count()
-	if err != nil {
-		t.Fatalf("failed to count users: %v", err)
-	}
-
-	if c == 2 {
-		flag, err := AuthModeCanBeModified(ctx)
-		if err != nil {
-			t.Fatalf("failed to determine whether auth mode can be modified: %v", err)
-		}
-		if !flag {
-			t.Errorf("unexpected result: %t != %t", flag, true)
-		}
-
-		user := models.User{
-			Username: "user_for_config_test",
-			Email:    "user_for_config_test@vmware.com",
-			Password: "P@ssword",
-			Realname: "user_for_config_test",
-		}
-		id, err := Register(user)
-		if err != nil {
-			t.Fatalf("failed to register user: %v", err)
-		}
-		defer func(id int64) {
-			if err := CleanUser(id); err != nil {
-				t.Fatalf("failed to delete user %d: %v", id, err)
-			}
-		}(id)
-
-		flag, err = AuthModeCanBeModified(ctx)
-		if err != nil {
-			t.Fatalf("failed to determine whether auth mode can be modified: %v", err)
-		}
-		if flag {
-			t.Errorf("unexpected result: %t != %t", flag, false)
-		}
-
-	} else {
-		flag, err := AuthModeCanBeModified(ctx)
-		if err != nil {
-			t.Fatalf("failed to determine whether auth mode can be modified: %v", err)
-		}
-		if flag {
-			t.Errorf("unexpected result: %t != %t", flag, false)
-		}
-	}
-}

src/common/dao/dao_test.go

@@ -27,7 +27,6 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 	libOrm "github.com/goharbor/harbor/src/lib/orm"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 var testCtx context.Context
@@ -101,15 +100,6 @@ const username string = "Tester01"
 const password string = "Abc12345"
 const projectName string = "test_project"
 const repositoryName string = "test_repository"
-const repoTag string = "test1.1"
-const repoTag2 string = "test1.2"
-const SysAdmin int = 1
-const projectAdmin int = 2
-const developer int = 3
-const guest int = 4
-
-const publicityOn = 1
-const publicityOff = 0
 
 func TestMain(m *testing.M) {
 	databases := []string{"postgresql"}
@@ -281,22 +271,6 @@ func TestGetUser(t *testing.T) {
 	assert.NotNil(t, err)
 }
 
-func TestListUsers(t *testing.T) {
-	users, err := ListUsers(nil)
-	require.Nil(t, err)
-	assert.Greater(t, len(users), 0)
-	users2, err := ListUsers(&models.UserQuery{Username: username})
-	require.Nil(t, err)
-	assert.Equal(t, 1, len(users2))
-	assert.Equal(t, username, users2[0].Username)
-	users3, err := ListUsers(&models.UserQuery{Username: username, Pagination: &models.Pagination{Page: 2, Size: 1}})
-	require.Nil(t, err)
-	assert.Equal(t, 0, len(users3))
-	users4, err := ListUsers(&models.UserQuery{Username: "__"})
-	require.Nil(t, err)
-	assert.Equal(t, 0, len(users4))
-}
-
 func TestResetUserPassword(t *testing.T) {
 	uuid := utils.GenerateRandomString()
 
@@ -425,60 +399,6 @@ func TestGetProjects(t *testing.T) {
 	}
 }
 
-// isAdminRole returns whether the user is admin.
-func isAdminRole(userIDOrUsername interface{}) (bool, error) {
-	u := models.User{}
-
-	switch v := userIDOrUsername.(type) {
-	case int:
-		u.UserID = v
-	case string:
-		u.Username = v
-	default:
-		return false, fmt.Errorf("invalid parameter, only int and string are supported: %v", userIDOrUsername)
-	}
-
-	if u.UserID == NonExistUserID && len(u.Username) == 0 {
-		return false, nil
-	}
-
-	user, err := GetUser(u)
-	if err != nil {
-		return false, err
-	}
-
-	if user == nil {
-		return false, nil
-	}
-
-	return user.SysAdminFlag, nil
-}
-
-func TestToggleAdminRole(t *testing.T) {
-	err := ToggleUserAdminRole(currentUser.UserID, true)
-	if err != nil {
-		t.Errorf("Error in toggle ToggleUserAdmin role: %v, user: %+v", err, currentUser)
-	}
-	isAdmin, err := isAdminRole(currentUser.UserID)
-	if err != nil {
-		t.Errorf("Error in IsAdminRole: %v, user id: %d", err, currentUser.UserID)
-	}
-	if !isAdmin {
-		t.Errorf("User is not admin after toggled, user id: %d", currentUser.UserID)
-	}
-	err = ToggleUserAdminRole(currentUser.UserID, false)
-	if err != nil {
-		t.Errorf("Error in toggle ToggleUserAdmin role: %v, user: %+v", err, currentUser)
-	}
-	isAdmin, err = isAdminRole(currentUser.UserID)
-	if err != nil {
-		t.Errorf("Error in IsAdminRole: %v, user id: %d", err, currentUser.UserID)
-	}
-	if isAdmin {
-		t.Errorf("User is still admin after toggled, user id: %d", currentUser.UserID)
-	}
-}
-
 func TestChangeUserProfile(t *testing.T) {
 	user := models.User{UserID: currentUser.UserID, Email: username + "@163.com", Realname: "test", Comment: "Unit Test"}
 	err := ChangeUserProfile(user)

src/common/dao/oidc_user_test.go

@@ -60,11 +60,6 @@ func TestOIDCUserMetaDaoMethods(t *testing.T) {
 	require.NotNil(t, err)
 	assert.Equal(t, "unable to onboard as empty oidc user", err.Error())
 
-	// test get by userID
-	oidcUser2, err := GetOIDCUserByUserID(user111.UserID)
-	require.Nil(t, err)
-	assert.Equal(t, "QWE123123RT1", oidcUser2.SubIss)
-
 	// test get by sub and iss
 	userGetBySubIss, err := GetUserBySubIss("QWE123", "123RT1")
 	require.Nil(t, err)

src/common/dao/user.go

@@ -15,14 +15,10 @@
 package dao
 
 import (
-	"context"
 	"errors"
 	"fmt"
-	libOrm "github.com/goharbor/harbor/src/lib/orm"
 	"time"
 
-	"github.com/astaxie/beego/orm"
-
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/lib/log"
@@ -98,68 +94,6 @@ func LoginByDb(auth models.AuthModel) (*models.User, error) {
 	return &user, nil
 }
 
-// GetTotalOfUsers ...
-func GetTotalOfUsers(query *models.UserQuery) (int64, error) {
-	return userQueryConditions(query).Count()
-}
-
-// ListUsers lists all users according to different conditions.
-func ListUsers(query *models.UserQuery) ([]models.User, error) {
-	qs := userQueryConditions(query)
-	if query != nil && query.Pagination != nil {
-		offset := (query.Pagination.Page - 1) * query.Pagination.Size
-		qs = qs.Offset(offset).Limit(query.Pagination.Size)
-	}
-	users := []models.User{}
-	_, err := qs.OrderBy("username").All(&users)
-	return users, err
-}
-
-func userQueryConditions(query *models.UserQuery) orm.QuerySeter {
-	qs := GetOrmer().QueryTable(&models.User{}).Filter("deleted", 0)
-
-	if query == nil {
-		// Exclude admin account, see https://github.com/goharbor/harbor/issues/2527
-		return qs.Filter("user_id__gt", 1)
-	}
-
-	if len(query.UserIDs) > 0 {
-		qs = qs.Filter("user_id__in", query.UserIDs)
-	} else {
-		// Exclude admin account when not filter by UserIDs, see https://github.com/goharbor/harbor/issues/2527
-		qs = qs.Filter("user_id__gt", 1)
-	}
-
-	if len(query.Username) > 0 {
-		qs = qs.Filter("username__contains", libOrm.Escape(query.Username))
-	}
-
-	if len(query.Email) > 0 {
-		qs = qs.Filter("email__contains", libOrm.Escape(query.Email))
-	}
-
-	return qs
-}
-
-// ToggleUserAdminRole gives a user admin role.
-func ToggleUserAdminRole(userID int, hasAdmin bool) error {
-	o := GetOrmer()
-	queryParams := make([]interface{}, 1)
-	sql := `update harbor_user set sysadmin_flag = ? where user_id = ?`
-	queryParams = append(queryParams, hasAdmin)
-	queryParams = append(queryParams, userID)
-	r, err := o.Raw(sql, queryParams).Exec()
-	if err != nil {
-		return err
-	}
-
-	if _, err := r.RowsAffected(); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // ChangeUserPassword ...
 func ChangeUserPassword(u models.User) error {
 	u.UpdateTime = time.Now()
@@ -204,26 +138,6 @@ func UpdateUserResetUUID(u models.User) error {
 	return err
 }
 
-// DeleteUser ...
-func DeleteUser(userID int) error {
-	o := GetOrmer()
-
-	user, err := GetUser(models.User{
-		UserID: userID,
-	})
-	if err != nil {
-		return err
-	}
-
-	name := fmt.Sprintf("%s#%d", user.Username, user.UserID)
-	email := fmt.Sprintf("%s#%d", user.Email, user.UserID)
-
-	_, err = o.Raw(`update harbor_user
-		set deleted = true, username = ?, email = ?
-		where user_id = ?`, name, email, userID).Exec()
-	return err
-}
-
 // ChangeUserProfile - Update user in local db,
 // cols to specify the columns need to update,
 // Email, and RealName, Comment are updated by default.
@@ -294,19 +208,3 @@ func CleanUser(id int64) error {
 func matchPassword(u *models.User, password string) bool {
 	return utils.Encrypt(password, u.Salt, u.PasswordVersion) == u.Password
 }
-
-// AuthModeCanBeModified determines whether auth mode can be
-// modified or not. Auth mode can modified when there is only admin
-// user in database.
-func AuthModeCanBeModified(ctx context.Context) (bool, error) {
-	o, err := libOrm.FromContext(ctx)
-	if err != nil {
-		return false, err
-	}
-	c, err := o.QueryTable(&models.User{}).Count()
-	if err != nil {
-		return false, err
-	}
-	// admin and anonymous
-	return c == 2, nil
-}

src/common/dao/user_test.go

@@ -15,64 +15,12 @@
 package dao
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/stretchr/testify/assert"
 )
 
-func TestDeleteUser(t *testing.T) {
-	username := "user_for_test"
-	email := "user_for_test@vmware.com"
-	password := "P@ssword"
-	realname := "user_for_test"
-
-	u := models.User{
-		Username: username,
-		Email:    email,
-		Password: password,
-		Realname: realname,
-	}
-	id, err := Register(u)
-	if err != nil {
-		t.Fatalf("failed to register user: %v", err)
-	}
-	defer func(id int64) {
-		if err := CleanUser(id); err != nil {
-			t.Fatalf("failed to delete user %d: %v", id, err)
-		}
-	}(id)
-
-	err = DeleteUser(int(id))
-	if err != nil {
-		t.Fatalf("Error occurred in DeleteUser: %v", err)
-	}
-
-	user := &models.User{}
-	sql := "select * from harbor_user where user_id = ?"
-	if err = GetOrmer().Raw(sql, id).
-		QueryRow(user); err != nil {
-		t.Fatalf("failed to query user: %v", err)
-	}
-
-	if user.Deleted != true {
-		t.Error("user is not deleted")
-	}
-
-	expected := fmt.Sprintf("%s#%d", u.Username, id)
-	if user.Username != expected {
-		t.Errorf("unexpected username: %s != %s", user.Username,
-			expected)
-	}
-
-	expected = fmt.Sprintf("%s#%d", u.Email, id)
-	if user.Email != expected {
-		t.Errorf("unexpected email: %s != %s", user.Email,
-			expected)
-	}
-}
-
 func TestOnBoardUser(t *testing.T) {
 	assert := assert.New(t)
 	u := &models.User{

src/common/models/user.go

@@ -46,14 +46,6 @@ type User struct {
 	OIDCUserMeta    *OIDCUser `orm:"-" json:"oidc_user_meta,omitempty"`
 }
 
-// UserQuery ...
-type UserQuery struct {
-	UserIDs    []int
-	Username   string
-	Email      string
-	Pagination *Pagination
-}
-
 // TableName ...
 func (u *User) TableName() string {
 	return UserTable

src/controller/quota/driver/project/util.go

@@ -18,11 +18,11 @@ import (
 	"context"
 	"strconv"
 
-	"github.com/goharbor/harbor/src/common/dao"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/project"
+	"github.com/goharbor/harbor/src/pkg/user"
 	"github.com/graph-gophers/dataloader"
 )
 
@@ -49,22 +49,24 @@ func getProjectsBatchFn(ctx context.Context, keys dataloader.Keys) []*dataloader
 		return handleError(err)
 	}
 
-	var ownerIDs []int
+	var ownerIDs []interface{}
 	var projectsMap = make(map[int64]*models.Project, len(projectIDs))
 	for _, project := range projects {
 		ownerIDs = append(ownerIDs, project.OwnerID)
 		projectsMap[project.ProjectID] = project
 	}
 
-	// TODO: change to use user manager
+	owners, err := user.Mgr.List(ctx, q.New(q.KeyWords{
-	owners, err := dao.ListUsers(&models.UserQuery{UserIDs: ownerIDs})
+		"UserID": q.NewOrList(ownerIDs),
+	}))
+
 	if err != nil {
 		return handleError(err)
 	}
 
 	var ownersMap = make(map[int]*models.User, len(owners))
 	for i, owner := range owners {
-		ownersMap[owner.UserID] = &owners[i]
+		ownersMap[owner.UserID] = owners[i]
 	}
 
 	var results []*dataloader.Result

src/core/api/api_test.go

@@ -16,7 +16,6 @@ package api
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/goharbor/harbor/src/lib/orm"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -28,6 +27,8 @@ import (
 
 	"github.com/goharbor/harbor/src/chartserver"
 	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/lib/orm"
+	"github.com/goharbor/harbor/src/pkg/user"
 
 	"github.com/dghubble/sling"
 	"github.com/goharbor/harbor/src/common/dao"
@@ -338,16 +339,17 @@ func prepare() error {
 }
 
 func clean() {
+	ctx := orm.Context()
 	pmids := []int{projAdminPMID, projDeveloperPMID, projGuestPMID}
 
 	for _, id := range pmids {
-		if err := member.Mgr.Delete(orm.Context(), 1, id); err != nil {
+		if err := member.Mgr.Delete(ctx, 1, id); err != nil {
 			fmt.Printf("failed to clean up member %d from project library: %v", id, err)
 		}
 	}
 	userids := []int64{nonSysAdminID, projAdminID, projDeveloperID, projGuestID}
 	for _, id := range userids {
-		if err := dao.DeleteUser(int(id)); err != nil {
+		if err := user.Mgr.Delete(ctx, int(id)); err != nil {
 			fmt.Printf("failed to clean up user %d: %v \n", id, err)
 		}
 	}

src/core/api/dataprepare_test.go

@@ -1,94 +0,0 @@
-// Copyright 2018 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 api
-
-import (
-	"github.com/goharbor/harbor/src/common/dao"
-	"github.com/goharbor/harbor/src/common/models"
-)
-
-const (
-	// Prepare Test info
-	TestUserName     = "testUser0001"
-	TestUserPwd      = "testUser0001"
-	TestUserEmail    = "testUser0001@mydomain.com"
-	TestProName      = "testProject0001"
-	TestRegistryName = "testRegistry0001"
-	TestRepoName     = "testRepo0001"
-)
-
-func CommonAddUser() {
-
-	commonUser := models.User{
-		Username: TestUserName,
-		Password: TestUserPwd,
-		Email:    TestUserEmail,
-	}
-
-	_, _ = dao.Register(commonUser)
-
-}
-
-func CommonGetUserID() int {
-	queryUser := &models.User{
-		Username: TestUserName,
-	}
-	commonUser, _ := dao.GetUser(*queryUser)
-	return commonUser.UserID
-}
-
-func CommonDelUser() {
-	queryUser := &models.User{
-		Username: TestUserName,
-	}
-	commonUser, _ := dao.GetUser(*queryUser)
-	_ = dao.DeleteUser(commonUser.UserID)
-
-}
-
-func CommonAddProject() {
-
-	queryUser := &models.User{
-		Username: "admin",
-	}
-	adminUser, _ := dao.GetUser(*queryUser)
-	commonProject := &models.Project{
-		Name:    TestProName,
-		OwnerID: adminUser.UserID,
-	}
-
-	_, _ = dao.AddProject(*commonProject)
-
-}
-
-func CommonDelProject() {
-	commonProject, _ := dao.GetProjectByName(TestProName)
-
-	_ = dao.DeleteProject(commonProject.ProjectID)
-}
-
-func CommonAddRepository() {
-	commonRepository := &models.RepoRecord{
-		RepositoryID: 31,
-		Name:         TestRepoName,
-		ProjectID:    1,
-		PullCount:    1,
-	}
-	_ = dao.AddRepository(*commonRepository)
-}
-
-func CommonDelRepository() {
-	_ = dao.DeleteRepository(TestRepoName)
-}

src/core/api/harborapi_test.go

@@ -47,6 +47,8 @@ import (
 )
 
 const (
+	TestUserName     = "testUser0001"
+	TestUserPwd      = "testUser0001"
 	jsonAcceptHeader = "application/json"
 	testAcceptHeader = "text/plain"
 	adminName        = "admin"