Skip to content

Commit

Permalink
fix issue 19928
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
wy65701436 committed May 10, 2024
1 parent beb5f3f commit e6dd128
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 26 deletions.
7 changes: 4 additions & 3 deletions src/controller/member/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion src/controller/member/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions src/pkg/cached/project/redis/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Check warning on line 80 in src/pkg/cached/project/redis/manager.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/cached/project/redis/manager.go#L79-L80

Added lines #L79 - L80 were not covered by tests
}

func (m *Manager) Delete(ctx context.Context, id int64) error {
Expand Down
33 changes: 27 additions & 6 deletions src/pkg/project/dao/dao.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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) {

Check warning on line 206 in src/pkg/project/dao/dao.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/dao/dao.go#L206

Added line #L206 was not covered by tests
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)

Check warning on line 214 in src/pkg/project/dao/dao.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/dao/dao.go#L212-L214

Added lines #L212 - L214 were not covered by tests
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
}

Check warning on line 229 in src/pkg/project/dao/dao.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/dao/dao.go#L219-L229

Added lines #L219 - L229 were not covered by tests
}

var members []models.Member
if len(membersU) > 0 {
members = append(members, membersU...)
}
if len(membersG) > 0 {
members = append(members, membersG...)
}

Check warning on line 238 in src/pkg/project/dao/dao.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/dao/dao.go#L232-L238

Added lines #L232 - L238 were not covered by tests

return members, nil
}
7 changes: 4 additions & 3 deletions src/pkg/project/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)

Check warning on line 129 in src/pkg/project/manager.go

View check run for this annotation

Codecov / codecov/patch

src/pkg/project/manager.go#L128-L129

Added lines #L128 - L129 were not covered by tests
}
2 changes: 1 addition & 1 deletion src/server/v2.0/handler/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (p *permissionsAPI) GetPermissions(ctx context.Context, _ permissions.GetPe
if err != nil {
return p.SendError(ctx, err)
}
is, err := p.mc.IsProjectAdmin(ctx, user.UserID)
is, err := p.mc.IsProjectAdmin(ctx, *user)

Check warning on line 64 in src/server/v2.0/handler/permissions.go

View check run for this annotation

Codecov / codecov/patch

src/server/v2.0/handler/permissions.go#L64

Added line #L64 was not covered by tests
if err != nil {
return p.SendError(ctx, err)
}
Expand Down
23 changes: 13 additions & 10 deletions src/testing/pkg/project/manager.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e6dd128

Please sign in to comment.