From e2b6a3cb78da710d79e753fd343f2d53266b6062 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Thu, 12 Aug 2021 17:18:24 +0800 Subject: [PATCH] support robot to list project 1, add permission check for API of List Projects 2, add permission check for API of List Repositories 3, use the self defined query to handle both names and public query Signed-off-by: wang yan --- src/common/security/robot/context.go | 46 ++++--- src/common/security/robot/context_test.go | 145 +++++++++++++++------- src/pkg/project/models/project.go | 30 +++++ src/server/middleware/security/robot.go | 19 +-- src/server/v2.0/handler/project.go | 23 ++++ src/server/v2.0/handler/repository.go | 22 ++++ 6 files changed, 204 insertions(+), 81 deletions(-) diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index 14b9350a206..9071ee3b656 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -16,6 +16,7 @@ package robot import ( "context" + "fmt" rbac_project "github.com/goharbor/harbor/src/common/rbac/project" "github.com/goharbor/harbor/src/common/rbac/system" "github.com/goharbor/harbor/src/controller/robot" @@ -27,26 +28,21 @@ import ( "github.com/goharbor/harbor/src/pkg/permission/evaluator" "github.com/goharbor/harbor/src/pkg/permission/types" "github.com/goharbor/harbor/src/pkg/project/models" - "github.com/goharbor/harbor/src/pkg/robot/model" ) // SecurityContext implements security.Context interface based on database type SecurityContext struct { - robot *model.Robot - isSystemLevel bool - ctl project.Controller - policies []*types.Policy - evaluator evaluator.Evaluator - once sync.Once + robot *robot.Robot + ctl project.Controller + evaluator evaluator.Evaluator + once sync.Once } // NewSecurityContext ... -func NewSecurityContext(robot *model.Robot, isSystemLevel bool, policy []*types.Policy) *SecurityContext { +func NewSecurityContext(r *robot.Robot) *SecurityContext { return &SecurityContext{ - ctl: project.Ctl, - robot: robot, - policies: policy, - isSystemLevel: isSystemLevel, + ctl: project.Ctl, + robot: r, } } @@ -69,6 +65,11 @@ func (s *SecurityContext) GetUsername() string { return s.robot.Name } +// User get the current user +func (s *SecurityContext) User() *robot.Robot { + return s.robot +} + // IsSysAdmin robot cannot be a system admin func (s *SecurityContext) IsSysAdmin() bool { return false @@ -81,12 +82,27 @@ func (s *SecurityContext) IsSolutionUser() bool { // Can returns whether the robot can do action on resource func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource types.Resource) bool { + if s.robot == nil { + return false + } + s.once.Do(func() { - if s.isSystemLevel { + var accesses []*types.Policy + for _, p := range s.robot.Permissions { + for _, a := range p.Access { + accesses = append(accesses, &types.Policy{ + Action: a.Action, + Effect: a.Effect, + Resource: types.Resource(fmt.Sprintf("%s/%s", p.Scope, a.Resource)), + }) + } + } + + if s.robot.Level == robot.LEVELSYSTEM { var proPolicies []*types.Policy var sysPolicies []*types.Policy var evaluators evaluator.Evaluators - for _, p := range s.policies { + for _, p := range accesses { if strings.HasPrefix(p.Resource.String(), robot.SCOPESYSTEM) { sysPolicies = append(sysPolicies, p) } else if strings.HasPrefix(p.Resource.String(), robot.SCOPEPROJECT) { @@ -101,7 +117,7 @@ func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource s.evaluator = evaluators } else { - s.evaluator = rbac_project.NewEvaluator(s.ctl, rbac_project.NewBuilderForPolicies(s.GetUsername(), s.policies, filterRobotPolicies)) + s.evaluator = rbac_project.NewEvaluator(s.ctl, rbac_project.NewBuilderForPolicies(s.GetUsername(), accesses, filterRobotPolicies)) } }) diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go index 871b6f83fbf..80bc8b79b5a 100644 --- a/src/common/security/robot/context_test.go +++ b/src/common/security/robot/context_test.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "github.com/goharbor/harbor/src/common/rbac/project" + "github.com/goharbor/harbor/src/controller/robot" "reflect" "testing" @@ -32,117 +33,165 @@ import ( var ( private = &proModels.Project{ - Name: "testrobot", - OwnerID: 1, + ProjectID: 1, + Name: "testrobot", + OwnerID: 1, } ) func TestIsAuthenticated(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, false, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsAuthenticated()) // authenticated - ctx = NewSecurityContext(&model.Robot{ - Name: "test", - Disabled: false, - }, false, nil) + ctx = NewSecurityContext(&robot.Robot{ + Robot: model.Robot{ + Name: "test", + Disabled: false, + }, + }) assert.True(t, ctx.IsAuthenticated()) } func TestGetUsername(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, false, nil) + ctx := NewSecurityContext(nil) assert.Equal(t, "", ctx.GetUsername()) // authenticated - ctx = NewSecurityContext(&model.Robot{ - Name: "test", - Disabled: false, - }, false, nil) + ctx = NewSecurityContext(&robot.Robot{ + Robot: model.Robot{ + Name: "test", + Disabled: false, + }, + }) assert.Equal(t, "test", ctx.GetUsername()) } +func TestGetUser(t *testing.T) { + // unauthenticated + ctx := NewSecurityContext(nil) + assert.Equal(t, "", ctx.GetUsername()) + + // authenticated + ctx = NewSecurityContext(&robot.Robot{ + Robot: model.Robot{ + ID: 123, + Name: "test", + Disabled: false, + }, + }) + assert.Equal(t, "test", ctx.User().Name) + assert.Equal(t, int64(123), ctx.User().ID) +} + func TestIsSysAdmin(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, false, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsSysAdmin()) // authenticated, non admin - ctx = NewSecurityContext(&model.Robot{ - Name: "test", - Disabled: false, - }, false, nil) + ctx = NewSecurityContext(&robot.Robot{ + Robot: model.Robot{ + Name: "test", + Disabled: false, + }, + }) assert.False(t, ctx.IsSysAdmin()) } func TestIsSolutionUser(t *testing.T) { - ctx := NewSecurityContext(nil, false, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsSolutionUser()) } func TestHasPullPerm(t *testing.T) { - policies := []*types.Policy{ - { - Resource: rbac.Resource(fmt.Sprintf("/project/%d/repository", private.ProjectID)), - Action: rbac.ActionPull, + robot := &robot.Robot{ + Robot: model.Robot{ + Name: "test_robot_1", + Description: "desc", + }, + Permissions: []*robot.Permission{ + { + Kind: "project", + Namespace: "library", + Access: []*types.Policy{ + { + Resource: rbac.Resource(fmt.Sprintf("project/%d/repository", private.ProjectID)), + Action: rbac.ActionPull, + }, + }, + }, }, - } - robot := &model.Robot{ - Name: "test_robot_1", - Description: "desc", } ctl := &projecttesting.Controller{} mock.OnAnything(ctl, "Get").Return(private, nil) - ctx := NewSecurityContext(robot, false, policies) + ctx := NewSecurityContext(robot) ctx.ctl = ctl resource := project.NewNamespace(private.ProjectID).Resource(rbac.ResourceRepository) assert.True(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) } func TestHasPushPerm(t *testing.T) { - policies := []*types.Policy{ - { - Resource: rbac.Resource(fmt.Sprintf("/project/%d/repository", private.ProjectID)), - Action: rbac.ActionPush, + robot := &robot.Robot{ + Robot: model.Robot{ + Name: "test", + Disabled: false, + }, + Permissions: []*robot.Permission{ + { + Kind: "project", + Namespace: "library", + Access: []*types.Policy{ + { + Resource: rbac.Resource(fmt.Sprintf("project/%d/repository", private.ProjectID)), + Action: rbac.ActionPush, + }, + }, + }, }, - } - robot := &model.Robot{ - Name: "test_robot_2", - Description: "desc", } ctl := &projecttesting.Controller{} mock.OnAnything(ctl, "Get").Return(private, nil) - ctx := NewSecurityContext(robot, false, policies) + ctx := NewSecurityContext(robot) ctx.ctl = ctl resource := project.NewNamespace(private.ProjectID).Resource(rbac.ResourceRepository) assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) } func TestHasPushPullPerm(t *testing.T) { - policies := []*types.Policy{ - { - Resource: rbac.Resource(fmt.Sprintf("/project/%d/repository", private.ProjectID)), - Action: rbac.ActionPush, + robot := &robot.Robot{ + Robot: model.Robot{ + Name: "test_robot_3", + Description: "desc", }, - { - Resource: rbac.Resource(fmt.Sprintf("/project/%d/repository", private.ProjectID)), - Action: rbac.ActionPull, + Permissions: []*robot.Permission{ + { + Kind: "project", + Namespace: "library", + Access: []*types.Policy{ + { + Resource: rbac.Resource(fmt.Sprintf("project/%d/repository", private.ProjectID)), + Action: rbac.ActionPush, + }, + { + Resource: rbac.Resource(fmt.Sprintf("project/%d/repository", private.ProjectID)), + Action: rbac.ActionPull, + }, + }, + }, }, } - robot := &model.Robot{ - Name: "test_robot_3", - Description: "desc", - } ctl := &projecttesting.Controller{} mock.OnAnything(ctl, "Get").Return(private, nil) - ctx := NewSecurityContext(robot, false, policies) + ctx := NewSecurityContext(robot) ctx.ctl = ctl resource := project.NewNamespace(private.ProjectID).Resource(rbac.ResourceRepository) assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) diff --git a/src/pkg/project/models/project.go b/src/pkg/project/models/project.go index 3cfa4249360..1ce7556b956 100644 --- a/src/pkg/project/models/project.go +++ b/src/pkg/project/models/project.go @@ -56,6 +56,12 @@ type Project struct { RegistryID int64 `orm:"column(registry_id)" json:"registry_id"` } +// NamesQuery ... +type NamesQuery struct { + Names []string // the names of project + WithPublic bool // include the public projects +} + // GetMetadata ... func (p *Project) GetMetadata(key string) (string, bool) { if len(p.Metadata) == 0 { @@ -182,6 +188,30 @@ func (p *Project) FilterByMember(ctx context.Context, qs orm.QuerySeter, key str return qs.FilterRaw("project_id", fmt.Sprintf("IN (%s)", subQuery)) } +// FilterByNames returns orm.QuerySeter with name filter +func (p *Project) FilterByNames(ctx context.Context, qs orm.QuerySeter, key string, value interface{}) orm.QuerySeter { + query, ok := value.(*NamesQuery) + if !ok { + return qs + } + + if len(query.Names) == 0 { + return qs + } + + var names []string + for _, v := range query.Names { + names = append(names, `'`+v+`'`) + } + subQuery := fmt.Sprintf("SELECT project_id FROM project where name IN (%s)", strings.Join(names, ",")) + + if query.WithPublic { + subQuery = fmt.Sprintf("(%s) UNION (SELECT project_id FROM project_metadata WHERE name = 'public' AND value = 'true')", subQuery) + } + + return qs.FilterRaw("project_id", fmt.Sprintf("IN (%s)", subQuery)) +} + func isTrue(i interface{}) bool { switch value := i.(type) { case bool: diff --git a/src/server/middleware/security/robot.go b/src/server/middleware/security/robot.go index b30864332e3..507b5802b62 100644 --- a/src/server/middleware/security/robot.go +++ b/src/server/middleware/security/robot.go @@ -15,7 +15,6 @@ package security import ( - "fmt" "github.com/goharbor/harbor/src/common/security" robotCtx "github.com/goharbor/harbor/src/common/security/robot" "github.com/goharbor/harbor/src/common/utils" @@ -23,11 +22,9 @@ import ( "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/q" - "github.com/goharbor/harbor/src/pkg/permission/types" "strings" "time" - "github.com/goharbor/harbor/src/pkg/robot/model" "net/http" ) @@ -71,20 +68,6 @@ func (r *robot) Generate(req *http.Request) security.Context { return nil } - var accesses []*types.Policy - for _, p := range robot.Permissions { - for _, a := range p.Access { - accesses = append(accesses, &types.Policy{ - Action: a.Action, - Effect: a.Effect, - Resource: types.Resource(fmt.Sprintf("%s/%s", p.Scope, a.Resource)), - }) - } - } - - modelRobot := &model.Robot{ - Name: name, - } log.Infof("a robot security context generated for request %s %s", req.Method, req.URL.Path) - return robotCtx.NewSecurityContext(modelRobot, robot.Level == robot_ctl.LEVELSYSTEM, accesses) + return robotCtx.NewSecurityContext(robot) } diff --git a/src/server/v2.0/handler/project.go b/src/server/v2.0/handler/project.go index 0708c592fd0..2d60817f33c 100644 --- a/src/server/v2.0/handler/project.go +++ b/src/server/v2.0/handler/project.go @@ -27,12 +27,14 @@ import ( "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/local" + robotSec "github.com/goharbor/harbor/src/common/security/robot" "github.com/goharbor/harbor/src/controller/p2p/preheat" "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/controller/quota" "github.com/goharbor/harbor/src/controller/registry" "github.com/goharbor/harbor/src/controller/repository" "github.com/goharbor/harbor/src/controller/retention" + robotCtr "github.com/goharbor/harbor/src/controller/robot" "github.com/goharbor/harbor/src/controller/scanner" "github.com/goharbor/harbor/src/core/api" "github.com/goharbor/harbor/src/lib" @@ -44,6 +46,7 @@ import ( "github.com/goharbor/harbor/src/pkg/audit" "github.com/goharbor/harbor/src/pkg/member" "github.com/goharbor/harbor/src/pkg/project/metadata" + pkgModels "github.com/goharbor/harbor/src/pkg/project/models" "github.com/goharbor/harbor/src/pkg/quota/types" "github.com/goharbor/harbor/src/pkg/retention/policy" "github.com/goharbor/harbor/src/pkg/robot" @@ -414,6 +417,26 @@ func (a *projectAPI) ListProjects(ctx context.Context, params operation.ListProj } query.Keywords["member"] = member + } else if r, ok := secCtx.(*robotSec.SecurityContext); ok { + // for the system level robot that covers all the project, see it as the system admin. + var coverAll bool + var names []string + for _, p := range r.User().Permissions { + if p.Scope == robotCtr.SCOPEALLPROJECT { + coverAll = true + break + } + names = append(names, p.Namespace) + } + if !coverAll { + namesQuery := &pkgModels.NamesQuery{ + Names: names, + } + if public, ok := query.Keywords["public"]; !ok || lib.ToBool(public) { + namesQuery.WithPublic = true + } + query.Keywords["names"] = namesQuery + } } else { // can't get the user info, force to return public projects query.Keywords["public"] = true diff --git a/src/server/v2.0/handler/repository.go b/src/server/v2.0/handler/repository.go index fe73e2c3826..acf314cef12 100644 --- a/src/server/v2.0/handler/repository.go +++ b/src/server/v2.0/handler/repository.go @@ -17,6 +17,9 @@ package handler import ( "context" "fmt" + "github.com/goharbor/harbor/src/common/security/robot" + robotCtr "github.com/goharbor/harbor/src/controller/robot" + pkgModels "github.com/goharbor/harbor/src/pkg/project/models" "github.com/go-openapi/runtime/middleware" "github.com/goharbor/harbor/src/common/rbac" @@ -123,6 +126,25 @@ func (r *repositoryAPI) listAuthorizedProjectIDs(ctx context.Context) ([]int64, GroupIDs: currentUser.GroupIDs, WithPublic: true, } + case *robot.SecurityContext: + // for the system level robot that covers all the project, see it as the system admin. + var coverAll bool + var names []string + r := secCtx.(*robot.SecurityContext).User() + for _, p := range r.Permissions { + if p.Scope == robotCtr.SCOPEALLPROJECT { + coverAll = true + break + } + names = append(names, p.Namespace) + } + if !coverAll { + namesQuery := &pkgModels.NamesQuery{ + Names: names, + WithPublic: true, + } + query.Keywords["names"] = namesQuery + } default: query.Keywords["public"] = true }