From 65891e369f69559b8b20686ae1cc79a688f3de5d Mon Sep 17 00:00:00 2001 From: wang yan Date: Mon, 27 Nov 2023 17:11:19 +0800 Subject: [PATCH] fix robot account access issue fixes #19622 Resolve the 403 issue occurring when a robot account, equipped with both system and project scope, attempts to access project resources. Signed-off-by: wang yan --- src/common/security/robot/context.go | 4 +- src/common/security/robot/context_test.go | 52 +++++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index 2754f90fb28..5c175046d94 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -111,7 +111,8 @@ func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource } if len(sysPolicies) != 0 { evaluators = evaluators.Add(system.NewEvaluator(s.GetUsername(), sysPolicies)) - } else if len(proPolicies) != 0 { + } + if len(proPolicies) != 0 { evaluators = evaluators.Add(rbac_project.NewEvaluator(s.ctl, rbac_project.NewBuilderForPolicies(s.GetUsername(), proPolicies))) } s.evaluator = evaluators @@ -119,7 +120,6 @@ func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource s.evaluator = rbac_project.NewEvaluator(s.ctl, rbac_project.NewBuilderForPolicies(s.GetUsername(), accesses, filterRobotPolicies)) } }) - return s.evaluator != nil && s.evaluator.HasPermission(ctx, resource, action) } diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go index c499ed24b7d..32e760d62b4 100644 --- a/src/common/security/robot/context_test.go +++ b/src/common/security/robot/context_test.go @@ -24,6 +24,7 @@ import ( "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/rbac/project" + "github.com/goharbor/harbor/src/common/rbac/system" "github.com/goharbor/harbor/src/controller/robot" "github.com/goharbor/harbor/src/pkg/permission/types" proModels "github.com/goharbor/harbor/src/pkg/project/models" @@ -198,6 +199,57 @@ func TestHasPushPullPerm(t *testing.T) { assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) } +func TestSysAndProPerm(t *testing.T) { + robot := &robot.Robot{ + Level: "system", + Robot: model.Robot{ + Name: "test_robot_4", + Description: "desc", + }, + Permissions: []*robot.Permission{ + { + Kind: "system", + Namespace: "/", + Access: []*types.Policy{ + { + Resource: rbac.Resource(fmt.Sprintf("system/%s", rbac.ResourceRepository)), + Action: rbac.ActionList, + }, + { + Resource: rbac.Resource(fmt.Sprintf("system/%s", rbac.ResourceGarbageCollection)), + Action: rbac.ActionCreate, + }, + }, + }, + { + 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, + }, + }, + }, + }, + } + + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + 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)) + + resource = system.NewNamespace().Resource(rbac.ResourceGarbageCollection) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionCreate, resource)) +} + func Test_filterRobotPolicies(t *testing.T) { type args struct { p *proModels.Project