fix issue 19928 (#20409)

* fix issue 19928

it needs to consider the user who is in any group that has been granted with the project admin role.

Signed-off-by: wang yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2024-05-15 13:07:30 +08:00 committed by GitHub
parent 232f9ba7ea
commit 2977fec006
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 97 additions and 36 deletions

View File

@ -125,10 +125,7 @@ var (
{Resource: rbac.ResourceMember, Action: rbac.ActionRead},
{Resource: rbac.ResourceMember, Action: rbac.ActionList},
{Resource: rbac.ResourceMetadata, Action: rbac.ActionCreate},
{Resource: rbac.ResourceMetadata, Action: rbac.ActionRead},
{Resource: rbac.ResourceMetadata, Action: rbac.ActionUpdate},
{Resource: rbac.ResourceMetadata, Action: rbac.ActionDelete},
{Resource: rbac.ResourceLog, Action: rbac.ActionList},

View File

@ -19,6 +19,7 @@ import (
"fmt"
"github.com/goharbor/harbor/src/common"
commonmodels "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/core/auth"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/q"
@ -45,7 +46,7 @@ type Controller interface {
// Count get the total amount of project members
Count(ctx context.Context, projectNameOrID interface{}, query *q.Query) (int, error)
// IsProjectAdmin judges if the user is a project admin of any project
IsProjectAdmin(ctx context.Context, memberID int) (bool, error)
IsProjectAdmin(ctx context.Context, member commonmodels.User) (bool, error)
}
// Request - Project Member Request
@ -261,8 +262,8 @@ func (c *controller) Delete(ctx context.Context, projectNameOrID interface{}, me
return c.mgr.Delete(ctx, p.ProjectID, memberID)
}
func (c *controller) IsProjectAdmin(ctx context.Context, memberID int) (bool, error) {
members, err := c.projectMgr.ListAdminRolesOfUser(ctx, memberID)
func (c *controller) IsProjectAdmin(ctx context.Context, member commonmodels.User) (bool, error) {
members, err := c.projectMgr.ListAdminRolesOfUser(ctx, member)
if err != nil {
return false, err
}

View File

@ -98,7 +98,7 @@ func (suite *MemberControllerTestSuite) TestAddProjectMemberWithUserGroup() {
func (suite *MemberControllerTestSuite) TestIsProjectAdmin() {
mock.OnAnything(suite.projectMgr, "ListAdminRolesOfUser").Return([]models.Member{models.Member{ID: 2, ProjectID: 2}}, nil)
ok, err := suite.controller.IsProjectAdmin(context.Background(), 2)
ok, err := suite.controller.IsProjectAdmin(context.Background(), comModels.User{UserID: 1})
suite.NoError(err)
suite.True(ok)
}

View File

@ -18,6 +18,7 @@ import (
"context"
"time"
commonmodels "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/utils"
"github.com/goharbor/harbor/src/lib/config"
"github.com/goharbor/harbor/src/lib/log"
@ -75,8 +76,8 @@ func (m *Manager) ListRoles(ctx context.Context, projectID int64, userID int, gr
return m.delegator.ListRoles(ctx, projectID, userID, groupIDs...)
}
func (m *Manager) ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error) {
return m.delegator.ListAdminRolesOfUser(ctx, userID)
func (m *Manager) ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error) {
return m.delegator.ListAdminRolesOfUser(ctx, user)
}
func (m *Manager) Delete(ctx context.Context, id int64) error {

View File

@ -20,6 +20,7 @@ import (
"time"
"github.com/goharbor/harbor/src/common"
commonmodels "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/lib"
"github.com/goharbor/harbor/src/lib/orm"
"github.com/goharbor/harbor/src/lib/q"
@ -43,7 +44,7 @@ type DAO interface {
// ListRoles the roles of user for the specific project
ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error)
// ListAdminRolesOfUser returns the roles of user for the all projects
ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error)
ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error)
}
// New returns an instance of the default DAO
@ -202,19 +203,39 @@ func (d *dao) ListRoles(ctx context.Context, projectID int64, userID int, groupI
return roles, nil
}
func (d *dao) ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error) {
func (d *dao) ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error) {
o, err := orm.FromContext(ctx)
if err != nil {
return nil, err
}
sql := `select b.* from project as a left join project_member as b on a.project_id = b.project_id where a.deleted = 'f' and b.entity_id = ? and b.entity_type = 'u' and b.role = 1;`
var members []models.Member
_, err = o.Raw(sql, userID).QueryRows(&members)
var membersU []models.Member
sqlU := `select b.* from project as a left join project_member as b on a.project_id = b.project_id where a.deleted = 'f' and b.entity_id = ? and b.entity_type = 'u' and b.role = 1;`
_, err = o.Raw(sqlU, user.UserID).QueryRows(&membersU)
if err != nil {
return nil, err
}
var membersG []models.Member
if len(user.GroupIDs) > 0 {
var params []interface{}
params = append(params, user.GroupIDs)
sqlG := fmt.Sprintf(`select b.* from project as a
left join project_member as b on a.project_id = b.project_id
where a.deleted = 'f' and b.entity_id in ( %s ) and b.entity_type = 'g' and b.role = 1;`, orm.ParamPlaceholderForIn(len(user.GroupIDs)))
_, err = o.Raw(sqlG, params).QueryRows(&membersG)
if err != nil {
return nil, err
}
}
var members []models.Member
if len(membersU) > 0 {
members = append(members, membersU...)
}
if len(membersG) > 0 {
members = append(members, membersG...)
}
return members, nil
}

View File

@ -21,6 +21,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/goharbor/harbor/src/common"
commonmodels "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/utils"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/orm"
@ -341,6 +342,42 @@ func (suite *DaoTestSuite) TestListByMember() {
}
}
func (suite *DaoTestSuite) TestListAdminRolesOfUser() {
{
// projectAdmin and user groups
suite.WithUser(func(userID int64, username string) {
project := &models.Project{
Name: utils.GenerateRandomString(),
OwnerID: int(userID),
}
projectID, err := suite.dao.Create(orm.Context(), project)
suite.Nil(err)
defer suite.dao.Delete(orm.Context(), projectID)
suite.WithUserGroup(func(groupID int64, groupName string) {
o, err := orm.FromContext(orm.Context())
if err != nil {
suite.Fail("got error %v", err)
}
var pid int64
suite.Nil(o.Raw("INSERT INTO project_member (project_id, entity_id, role, entity_type) values (?, ?, ?, ?) RETURNING id", projectID, groupID, common.RoleProjectAdmin, "g").QueryRow(&pid))
defer o.Raw("DELETE FROM project_member WHERE id = ?", pid)
userTest := commonmodels.User{
UserID: int(userID),
GroupIDs: []int{int(groupID)},
}
roles, err := suite.dao.ListAdminRolesOfUser(orm.Context(), userTest)
suite.Nil(err)
suite.Len(roles, 2)
})
})
}
}
func (suite *DaoTestSuite) TestListRoles() {
{
// only projectAdmin

View File

@ -19,6 +19,7 @@ import (
"regexp"
"strings"
commonmodels "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/utils"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/q"
@ -47,7 +48,7 @@ type Manager interface {
ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error)
// ListAdminRolesOfUser returns the roles of user for the all projects
ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error)
ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error)
}
// New returns a default implementation of Manager
@ -124,6 +125,6 @@ func (m *manager) ListRoles(ctx context.Context, projectID int64, userID int, gr
}
// ListAdminRolesOfUser returns the roles of user for the all projects
func (m *manager) ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error) {
return m.dao.ListAdminRolesOfUser(ctx, userID)
func (m *manager) ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error) {
return m.dao.ListAdminRolesOfUser(ctx, user)
}

View File

@ -21,6 +21,7 @@ import (
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/common/security"
"github.com/goharbor/harbor/src/common/security/local"
"github.com/goharbor/harbor/src/controller/member"
"github.com/goharbor/harbor/src/controller/user"
"github.com/goharbor/harbor/src/lib/errors"
@ -57,15 +58,14 @@ func (p *permissionsAPI) GetPermissions(ctx context.Context, _ permissions.GetPe
if secCtx.IsSysAdmin() {
isSystemAdmin = true
} else {
user, err := p.uc.GetByName(ctx, secCtx.GetUsername())
if sc, ok := secCtx.(*local.SecurityContext); ok {
user := sc.User()
var err error
isProjectAdmin, err = p.mc.IsProjectAdmin(ctx, *user)
if err != nil {
return p.SendError(ctx, err)
}
is, err := p.mc.IsProjectAdmin(ctx, user.UserID)
if err != nil {
return p.SendError(ctx, err)
}
isProjectAdmin = is
}
if !isSystemAdmin && !isProjectAdmin {
return p.SendError(ctx, errors.ForbiddenError(errors.New("only admins(system and project) can access permissions")))

View File

@ -5,9 +5,12 @@ package project
import (
context "context"
models "github.com/goharbor/harbor/src/pkg/project/models"
commonmodels "github.com/goharbor/harbor/src/common/models"
mock "github.com/stretchr/testify/mock"
models "github.com/goharbor/harbor/src/pkg/project/models"
q "github.com/goharbor/harbor/src/lib/q"
)
@ -150,9 +153,9 @@ func (_m *Manager) List(ctx context.Context, query *q.Query) ([]*models.Project,
return r0, r1
}
// ListAdminRolesOfUser provides a mock function with given fields: ctx, userID
func (_m *Manager) ListAdminRolesOfUser(ctx context.Context, userID int) ([]models.Member, error) {
ret := _m.Called(ctx, userID)
// ListAdminRolesOfUser provides a mock function with given fields: ctx, user
func (_m *Manager) ListAdminRolesOfUser(ctx context.Context, user commonmodels.User) ([]models.Member, error) {
ret := _m.Called(ctx, user)
if len(ret) == 0 {
panic("no return value specified for ListAdminRolesOfUser")
@ -160,19 +163,19 @@ func (_m *Manager) ListAdminRolesOfUser(ctx context.Context, userID int) ([]mode
var r0 []models.Member
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, int) ([]models.Member, error)); ok {
return rf(ctx, userID)
if rf, ok := ret.Get(0).(func(context.Context, commonmodels.User) ([]models.Member, error)); ok {
return rf(ctx, user)
}
if rf, ok := ret.Get(0).(func(context.Context, int) []models.Member); ok {
r0 = rf(ctx, userID)
if rf, ok := ret.Get(0).(func(context.Context, commonmodels.User) []models.Member); ok {
r0 = rf(ctx, user)
} else {
if ret.Get(0) != nil {
r0 = ret.Get(0).([]models.Member)
}
}
if rf, ok := ret.Get(1).(func(context.Context, int) error); ok {
r1 = rf(ctx, userID)
if rf, ok := ret.Get(1).(func(context.Context, commonmodels.User) error); ok {
r1 = rf(ctx, user)
} else {
r1 = ret.Error(1)
}