Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix issue 19928 #20409

Merged
merged 7 commits into from
May 15, 2024
Merged

fix issue 19928 #20409

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6dd128
fix issue 19928
wy65701436 May 9, 2024
525aa8e
get user from the security
wy65701436 May 11, 2024
8e4386d
fix
wy65701436 May 11, 2024
a5f0835
resolve the review comments
wy65701436 May 13, 2024
adf6694
fix ut
wy65701436 May 13, 2024
3c98c17
fix
wy65701436 May 14, 2024
60044f1
Merge branch 'main' into fix-19928
wy65701436 May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/controller/member/controller.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"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 @@
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"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 @@
// 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 @@
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 {
wy65701436 marked this conversation as resolved.
Show resolved Hide resolved
members = append(members, membersU...)
}
if len(membersG) > 0 {
wy65701436 marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"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 @@
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 @@
}

// 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
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
View file
Open in desktop

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