Merge pull request #15247 from stonezdj/21jun30_remove_auth_user

Delete users under auth_mode other than db_auth

make/migrations/postgresql/0070_2.4.0_schema.up.sql

@@ -0,0 +1,2 @@
+/* cleanup deleted user project members */
+DELETE FROM project_member pm WHERE pm.entity_type = 'u' AND EXISTS (SELECT NULL FROM harbor_user u WHERE pm.entity_id = u.user_id AND u.deleted = true )

src/controller/user/controller.go

@@ -17,6 +17,9 @@ package user
 import (
 	"context"
 	"fmt"
+	"github.com/goharbor/harbor/src/common"
+	"github.com/goharbor/harbor/src/lib"
+	"github.com/goharbor/harbor/src/pkg/member"
 
 	commonmodels "github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/security"
@@ -72,6 +75,7 @@ func NewController() Controller {
 	return &controller{
 		mgr:         user.New(),
 		oidcMetaMgr: oidc.NewMetaMgr(),
+		memberMgr:   member.Mgr,
 	}
 }
 
@@ -83,6 +87,7 @@ type Option struct {
 type controller struct {
 	mgr         user.Manager
 	oidcMetaMgr oidc.MetaManager
+	memberMgr   member.Manager
 }
 
 func (c *controller) UpdateOIDCMeta(ctx context.Context, ou *commonmodels.OIDCUser, cols ...string) error {
@@ -167,6 +172,16 @@ func (c *controller) Count(ctx context.Context, query *q.Query) (int64, error) {
 }
 
 func (c *controller) Delete(ctx context.Context, id int) error {
+	// cleanup project member with the user
+	if err := c.memberMgr.DeleteMemberByUserID(ctx, id); err != nil {
+		return errors.UnknownError(err).WithMessage("delete user failed, user id: %v, cannot delete project user member, error:%v", id, err)
+	}
+	// delete oidc metadata under the user
+	if lib.GetAuthMode(ctx) == common.OIDCAuth {
+		if err := c.oidcMetaMgr.DeleteByUserID(ctx, id); err != nil {
+			return errors.UnknownError(err).WithMessage("delete user failed, user id: %v, cannot delete oidc user, error:%v", id, err)
+		}
+	}
 	return c.mgr.Delete(ctx, id)
 }
 

src/pkg/member/dao/dao.go

@@ -42,6 +42,8 @@ type DAO interface {
 	UpdateProjectMemberRole(ctx context.Context, projectID int64, pmID int, role int) error
 	// DeleteProjectMemberByID - Delete Project Member by ID
 	DeleteProjectMemberByID(ctx context.Context, projectID int64, pmid int) error
+	// DeleteProjectMemberByUserID -- Delete project member by user id
+	DeleteProjectMemberByUserID(ctx context.Context, uid int) error
 	// SearchMemberByName search members of the project by entity_name
 	SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error)
 	// ListRoles lists the roles of user for the specific project
@@ -73,7 +75,7 @@ func (d *dao) GetProjectMember(ctx context.Context, queryMember models.Member, q
 		select pm.id as id, pm.project_id as project_id, u.user_id as entity_id, u.username as entity_name, u.creation_time, u.update_time, r.name as rolename,
 		r.role_id as role, pm.entity_type as entity_type from harbor_user u join project_member pm
 		on pm.project_id = ? and u.user_id = pm.entity_id
-		join role r on pm.role = r.role_id where u.deleted = false and pm.entity_type = 'u') as a where a.project_id = ? `
+		join role r on pm.role = r.role_id where pm.entity_type = 'u') as a where a.project_id = ? `
 
 	queryParam := make([]interface{}, 1)
 	// used ProjectID already
@@ -183,6 +185,16 @@ func (d *dao) DeleteProjectMemberByID(ctx context.Context, projectID int64, pmid
 	return nil
 }
 
+func (d *dao) DeleteProjectMemberByUserID(ctx context.Context, uid int) error {
+	sql := "delete from project_member where entity_type = 'u' and entity_id = ? "
+	o, err := orm.FromContext(ctx)
+	if err != nil {
+		return err
+	}
+	_, err = o.Raw(sql, uid).Exec()
+	return err
+}
+
 func (d *dao) SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error) {
 	o, err := orm.FromContext(ctx)
 	if err != nil {
@@ -195,7 +207,7 @@ func (d *dao) SearchMemberByName(ctx context.Context, projectID int64, entityNam
 			  from project_member pm
          left join harbor_user u on pm.entity_id = u.user_id and pm.entity_type = 'u'
 		 left join role r on pm.role = r.role_id
-			 where u.deleted = false and pm.project_id = ? and u.username like ?
+			 where pm.project_id = ? and u.username like ?
 			union
 		   select pm.id, pm.project_id,
 			       ug.group_name as entity_name,

src/pkg/member/dao/dao_test.go

@@ -266,6 +266,30 @@ func (s *DaoTestSuite) TestDeleteProjectMember() {
 
 }
 
+func (s *DaoTestSuite) TestDeleteProjectMemberByUserId() {
+	ctx := s.Context()
+	userID := 22
+	var addMember = models.Member{
+		ProjectID:  s.projectID,
+		EntityID:   userID,
+		EntityType: common.UserMember,
+		Role:       common.RoleDeveloper,
+	}
+	pmid, err := s.dao.AddProjectMember(ctx, addMember)
+	s.Nil(err)
+	s.True(pmid > 0)
+
+	err = s.dao.DeleteProjectMemberByUserID(ctx, userID)
+	s.Nil(err)
+
+	queryMember := models.Member{ProjectID: s.projectID, EntityID: userID, EntityType: common.UserMember}
+
+	// not exist
+	members, err := s.dao.GetProjectMember(ctx, queryMember, nil)
+	s.True(len(members) == 0)
+	s.Nil(err)
+}
+
 func TestDaoTestSuite(t *testing.T) {
 	suite.Run(t, &DaoTestSuite{})
 }

src/pkg/member/manager.go

@@ -41,6 +41,8 @@ type Manager interface {
 	UpdateRole(ctx context.Context, projectID int64, pmID int, role int) error
 	// SearchMemberByName search project member by name
 	SearchMemberByName(ctx context.Context, projectID int64, entityName string) ([]*models.Member, error)
+	// DeleteMemberByUserID delete project member by user id
+	DeleteMemberByUserID(ctx context.Context, uid int) error
 	// GetTotalOfProjectMembers get the total amount of project members
 	GetTotalOfProjectMembers(ctx context.Context, projectID int64, query *q.Query, roles ...int) (int, error)
 	// ListRoles list project roles
@@ -95,6 +97,10 @@ func (m *manager) Delete(ctx context.Context, projectID int64, memberID int) err
 	return m.dao.DeleteProjectMemberByID(ctx, projectID, memberID)
 }
 
+func (m *manager) DeleteMemberByUserID(ctx context.Context, uid int) error {
+	return m.dao.DeleteProjectMemberByUserID(ctx, uid)
+}
+
 // NewManager ...
 func NewManager() Manager {
 	return &manager{dao: dao.New()}

src/pkg/oidc/dao/meta.go

@@ -29,6 +29,8 @@ type MetaDAO interface {
 	Create(ctx context.Context, oidcUser *models.OIDCUser) (int, error)
 	// GetByUsername get the oidc meta record by the user's username
 	GetByUsername(ctx context.Context, username string) (*models.OIDCUser, error)
+	// DeleteByUserID delete the oidc metadata by user id
+	DeleteByUserID(ctx context.Context, uid int) error
 	// Update ...
 	Update(ctx context.Context, oidcUser *models.OIDCUser, props ...string) error
 	// List provides a way to query with flexible filter
@@ -42,6 +44,16 @@ func NewMetaDao() MetaDAO {
 
 type metaDAO struct{}
 
+func (md *metaDAO) DeleteByUserID(ctx context.Context, uid int) error {
+	sql := `DELETE from oidc_user where user_id = ?`
+	ormer, err := orm.FromContext(ctx)
+	if err != nil {
+		return err
+	}
+	_, err = ormer.Raw(sql, uid).Exec()
+	return err
+}
+
 func (md *metaDAO) GetByUsername(ctx context.Context, username string) (*models.OIDCUser, error) {
 	sql := `SELECT oidc_user.id, oidc_user.user_id, oidc_user.secret, oidc_user.token,
 				oidc_user.creation_time, oidc_user.update_time FROM oidc_user

src/pkg/oidc/dao/meta_test.go

@@ -27,9 +27,10 @@ import (
 
 type MetaDaoTestSuite struct {
 	htesting.Suite
-	dao      MetaDAO
-	userID   int
-	username string
+	dao          MetaDAO
+	userID       int
+	username     string
+	deleteUserID int
 }
 
 func (suite *MetaDaoTestSuite) SetupSuite() {
@@ -37,8 +38,10 @@ func (suite *MetaDaoTestSuite) SetupSuite() {
 	suite.ClearSQLs = []string{}
 	suite.dao = NewMetaDao()
 	suite.userID = 1234
+	suite.deleteUserID = 2234
 	suite.username = "oidc_meta_testuser"
 	suite.ExecSQL("INSERT INTO harbor_user (user_id, username,password,realname) VALUES(?,?,'test','test')", suite.userID, suite.username)
+	suite.ExecSQL("INSERT INTO harbor_user (user_id, username,password,realname) VALUES(?,?,'test','test')", suite.deleteUserID, suite.deleteUserID)
 	ctx := orm.Context()
 	_, err := suite.dao.Create(ctx, &models.OIDCUser{
 		UserID: suite.userID,
@@ -48,6 +51,14 @@ func (suite *MetaDaoTestSuite) SetupSuite() {
 	})
 	suite.Nil(err)
 	suite.appendClearSQL(suite.userID)
+	_, err = suite.dao.Create(ctx, &models.OIDCUser{
+		UserID: suite.deleteUserID,
+		SubIss: `ca4bb144-4b5c-4d1b-9469-69cb3768af9fhttps://sso.andrea.muellerpublic.de/auth/realms/harbor`,
+		Secret: `<enc-v1>7uBP9yqtdnVAhoA243GSv8nOXBWygqzaaEdq9Kqla+q4hOaBZmEMH9vUJi4Yjbh3`,
+		Token:  `xxxx`,
+	})
+	suite.Nil(err)
+	suite.appendClearSQL(suite.deleteUserID)
 }
 
 func (suite *MetaDaoTestSuite) TestList() {
@@ -82,6 +93,15 @@ func (suite *MetaDaoTestSuite) TestUpdate() {
 	suite.Equal("newsecret", l[0].Secret)
 }
 
+func (suite *MetaDaoTestSuite) TestDeleteByUserId() {
+	ctx := orm.Context()
+	err := suite.dao.DeleteByUserID(ctx, suite.deleteUserID)
+	suite.Nil(err)
+	l, err := suite.dao.List(ctx, q.New(q.KeyWords{"user_id": suite.deleteUserID}))
+	suite.Nil(err)
+	suite.True(len(l) == 0)
+}
+
 func (suite *MetaDaoTestSuite) appendClearSQL(uid int) {
 	suite.ClearSQLs = append(suite.ClearSQLs, fmt.Sprintf("DELETE FROM oidc_user WHERE user_id = %d", uid))
 	suite.ClearSQLs = append(suite.ClearSQLs, fmt.Sprintf("DELETE FROM harbor_user WHERE user_id = %d", uid))

src/pkg/oidc/metamanager.go

@@ -31,6 +31,8 @@ type MetaManager interface {
 	Create(ctx context.Context, oidcUser *models.OIDCUser) (int, error)
 	// GetByUserID gets the oidc meta record by user's ID
 	GetByUserID(ctx context.Context, uid int) (*models.OIDCUser, error)
+	// DeleteByUserID delete by user id
+	DeleteByUserID(ctx context.Context, uid int) error
 	// GetBySubIss gets the oidc meta record by the subject and issuer
 	GetBySubIss(ctx context.Context, sub, iss string) (*models.OIDCUser, error)
 	// SetCliSecretByUserID updates the cli secret of a user based on the user ID
@@ -43,6 +45,10 @@ type metaManager struct {
 	dao dao.MetaDAO
 }
 
+func (m *metaManager) DeleteByUserID(ctx context.Context, uid int) error {
+	return m.dao.DeleteByUserID(ctx, uid)
+}
+
 func (m *metaManager) Update(ctx context.Context, oidcUser *models.OIDCUser, cols ...string) error {
 	return m.dao.Update(ctx, oidcUser, cols...)
 }

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

@@ -394,14 +394,6 @@ func (u *usersAPI) requireDeletable(ctx context.Context, id int) error {
 	if !ok || !sctx.IsAuthenticated() {
 		return errors.UnauthorizedError(nil)
 	}
-	a, err := u.getAuth(ctx)
-	if err != nil {
-		log.G(ctx).Errorf("Failed to get authmode, error: %v", err)
-		return err
-	}
-	if a != common.DBAuth {
-		return errors.ForbiddenError(nil).WithMessage("Deleting user is not allowed under auth mode: %s", a)
-	}
 	if !sctx.Can(ctx, rbac.ActionDelete, userResource) {
 		return errors.ForbiddenError(nil).WithMessage("Not authorized to delete users")
 	}

src/testing/pkg/oidc/dao/meta_dao.go

@@ -38,6 +38,20 @@ func (_m *MetaDAO) Create(ctx context.Context, oidcUser *models.OIDCUser) (int,
 	return r0, r1
 }
 
+// DeleteByUserID provides a mock function with given fields: ctx, uid
+func (_m *MetaDAO) DeleteByUserID(ctx context.Context, uid int) error {
+	ret := _m.Called(ctx, uid)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int) error); ok {
+		r0 = rf(ctx, uid)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // GetByUsername provides a mock function with given fields: ctx, username
 func (_m *MetaDAO) GetByUsername(ctx context.Context, username string) (*models.OIDCUser, error) {
 	ret := _m.Called(ctx, username)

src/testing/pkg/oidc/meta_manager.go

@@ -35,6 +35,20 @@ func (_m *MetaManager) Create(ctx context.Context, oidcUser *models.OIDCUser) (i
 	return r0, r1
 }
 
+// DeleteByUserID provides a mock function with given fields: ctx, uid
+func (_m *MetaManager) DeleteByUserID(ctx context.Context, uid int) error {
+	ret := _m.Called(ctx, uid)
+
+	var r0 error
+	if rf, ok := ret.Get(0).(func(context.Context, int) error); ok {
+		r0 = rf(ctx, uid)
+	} else {
+		r0 = ret.Error(0)
+	}
+
+	return r0
+}
+
 // GetBySubIss provides a mock function with given fields: ctx, sub, iss
 func (_m *MetaManager) GetBySubIss(ctx context.Context, sub string, iss string) (*models.OIDCUser, error) {
 	ret := _m.Called(ctx, sub, iss)