diff --git a/src/common/rbac/project_evaluator.go b/src/common/rbac/project_evaluator.go index bac7d8e21..9ceafda78 100644 --- a/src/common/rbac/project_evaluator.go +++ b/src/common/rbac/project_evaluator.go @@ -15,10 +15,10 @@ package rbac import ( - pro "github.com/goharbor/harbor/src/common/dao/project" + "context" + "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/common/security" - "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/evaluator" "github.com/goharbor/harbor/src/pkg/permission/evaluator/namespace" @@ -26,62 +26,81 @@ import ( "github.com/goharbor/harbor/src/pkg/permission/types" ) -// NewProjectUserEvaluator returns permission evaluator for project -func NewProjectUserEvaluator(user *models.User, pm promgr.ProjectManager) evaluator.Evaluator { - return namespace.New(ProjectNamespaceKind, func(ns types.Namespace) evaluator.Evaluator { - project, err := pm.Get(ns.Identity()) - if err != nil || project == nil { - if err != nil { - log.Warningf("Failed to get info of project %d for permission evaluator, error: %v", ns.Identity(), err) +// ProjectRBACUserBuilder builder to make types.RBACUser for the project +type ProjectRBACUserBuilder func(context.Context, *models.Project) types.RBACUser + +// NewBuilderForUser create a builder for the local user +func NewBuilderForUser(user *models.User, ctl project.Controller) ProjectRBACUserBuilder { + return func(ctx context.Context, p *models.Project) types.RBACUser { + if user == nil { + // anonymous access + return &projectRBACUser{ + project: p, + username: "anonymous", } + } + + roles, err := ctl.ListRoles(ctx, p.ProjectID, user) + if err != nil { + log.Errorf("failed to list roles: %v", err) return nil } - if user != nil { - roles, err := pro.ListRoles(user, project.ProjectID) - if err != nil { - log.Errorf("failed to list roles: %v", err) - return nil - } - return rbac.New(NewProjectRBACUser(project, user.Username, roles...)) - } else if project.IsPublic() { - // anonymous access and the project is public - return rbac.New(NewProjectRBACUser(project, "anonymous")) - } else { - return nil + return &projectRBACUser{ + project: p, + username: user.Username, + projectRoles: roles, } - }) + } } -// NewProjectRobotEvaluator returns robot permission evaluator for project -func NewProjectRobotEvaluator(ctx security.Context, pm promgr.ProjectManager, - robotFactory func(types.Namespace) types.RBACUser) evaluator.Evaluator { +// NewBuilderForPolicies create a builder for the policies +func NewBuilderForPolicies(username string, policies []*types.Policy, + filters ...func(*models.Project, []*types.Policy) []*types.Policy) ProjectRBACUserBuilder { - return namespace.New(ProjectNamespaceKind, func(ns types.Namespace) evaluator.Evaluator { - project, err := pm.Get(ns.Identity()) - if err != nil || project == nil { + return func(ctx context.Context, p *models.Project) types.RBACUser { + for _, filter := range filters { + policies = filter(p, policies) + } + + return &projectRBACUser{ + project: p, + username: username, + policies: policies, + } + } +} + +// NewProjectEvaluator create evaluator for the project by builders +func NewProjectEvaluator(ctl project.Controller, builders ...ProjectRBACUserBuilder) evaluator.Evaluator { + return namespace.New(ProjectNamespaceKind, func(ctx context.Context, ns types.Namespace) evaluator.Evaluator { + p, err := ctl.Get(ctx, ns.Identity().(int64), project.Metadata(true)) + if err != nil { if err != nil { log.Warningf("Failed to get info of project %d for permission evaluator, error: %v", ns.Identity(), err) } return nil } - if ctx.IsAuthenticated() { - evaluators := evaluator.Evaluators{ - rbac.New(robotFactory(ns)), // robot account access + var rbacUsers []types.RBACUser + for _, builder := range builders { + if rbacUser := builder(ctx, p); rbacUser != nil { + rbacUsers = append(rbacUsers, rbacUser) } + } - if project.IsPublic() { - // authenticated access and the project is public - evaluators = evaluators.Add(rbac.New(NewProjectRBACUser(project, ctx.GetUsername()))) + switch len(rbacUsers) { + case 0: + return nil + case 1: + return rbac.New(rbacUsers[0]) + default: + var evaluators evaluator.Evaluators + for _, rbacUser := range rbacUsers { + evaluators = evaluators.Add(rbac.New(rbacUser)) } return evaluators - } else if project.IsPublic() { - // anonymous access and the project is public - return rbac.New(NewProjectRBACUser(project, "anonymous")) - } else { - return nil } }) } diff --git a/src/common/rbac/project_evaluator_test.go b/src/common/rbac/project_evaluator_test.go index 70df19d0d..85ae0186b 100644 --- a/src/common/rbac/project_evaluator_test.go +++ b/src/common/rbac/project_evaluator_test.go @@ -15,130 +15,127 @@ package rbac import ( - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/dao/project" - "github.com/goharbor/harbor/src/common/models" - promgr "github.com/goharbor/harbor/src/core/promgr/mocks" - "github.com/goharbor/harbor/src/pkg/permission/types" - "github.com/goharbor/harbor/src/testing/common/security" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" + "context" "testing" + + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/models" + projecttesting "github.com/goharbor/harbor/src/testing/controller/project" + "github.com/goharbor/harbor/src/testing/mock" + "github.com/stretchr/testify/assert" ) var ( - projectID = int64(1) + public = &models.Project{ + ProjectID: 1, + Name: "public_project", + OwnerID: 1, + Metadata: map[string]string{ + "public": "true", + }, + } - projectAdminSecurity = makeMockSecurity("projectAdmin", common.RoleProjectAdmin) - guestSecurity = makeMockSecurity("guest", common.RoleGuest) - anonymousSecurity = makeMockSecurity("") - - publicProjectManager = makeMockProjectManager(projectID, true) - privateProjectManager = makeMockProjectManager(projectID, false) + private = &models.Project{ + ProjectID: 2, + Name: "private_project", + OwnerID: 1, + Metadata: map[string]string{ + "public": "false", + }, + } ) -func makeMockSecurity(username string, roles ...int) *security.Context { - var isAuthenticated bool - if username != "" { - isAuthenticated = true - } - - ctx := &security.Context{} - ctx.On("IsAuthenticated").Return(isAuthenticated) - ctx.On("GetUsername").Return(username) - ctx.On("GetProjectRoles", mock.AnythingOfType("int64")).Return(roles) - - return ctx -} - -func makeMockProjectManager(projectID int64, isPublic bool) *promgr.ProjectManager { - pm := &promgr.ProjectManager{} - - project := &models.Project{ProjectID: projectID} - if isPublic { - project.SetMetadata(models.ProMetaPublic, "true") - } else { - project.SetMetadata(models.ProMetaPublic, "false") - } - - pm.On("Get", projectID).Return(project, nil) - - return pm -} - -func makeResource(subresource ...types.Resource) types.Resource { - return NewProjectNamespace(projectID).Resource(subresource...) -} - func TestAnonymousAccess(t *testing.T) { assert := assert.New(t) - evaluator1 := NewProjectUserEvaluator(nil, publicProjectManager) - assert.True(evaluator1.HasPermission(makeResource(ResourceRepository), ActionPull)) - evaluator2 := NewProjectUserEvaluator(nil, privateProjectManager) - assert.False(evaluator2.HasPermission(makeResource(ResourceRepository), ActionPull)) + { + // anonymous to access public project + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) - evaluator3 := NewProjectRobotEvaluator(anonymousSecurity, publicProjectManager, func(ns types.Namespace) types.RBACUser { return nil }) - assert.True(evaluator3.HasPermission(makeResource(ResourceRepository), ActionPull)) + resource := NewProjectNamespace(public.ProjectID).Resource(ResourceRepository) - evaluator4 := NewProjectRobotEvaluator(anonymousSecurity, privateProjectManager, func(ns types.Namespace) types.RBACUser { return nil }) - assert.False(evaluator4.HasPermission(makeResource(ResourceRepository), ActionPull)) + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(nil, ctl)) + assert.True(evaluator.HasPermission(context.TODO(), resource, ActionPull)) + } + + { + // anonymous to access private project + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + resource := NewProjectNamespace(private.ProjectID).Resource(ResourceRepository) + + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(nil, ctl)) + assert.False(evaluator.HasPermission(context.TODO(), resource, ActionPull)) + } } func TestProjectRoleAccess(t *testing.T) { assert := assert.New(t) - dao.PrepareTestForPostgresSQL() - projectID, err := dao.AddProject(models.Project{ - OwnerID: 1, - Name: "project_for_test_evaluator", - }) - require.Nil(t, err) - defer dao.DeleteProject(projectID) + { + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleProjectAdmin}, nil) - pm := makeMockProjectManager(projectID, true) + user := &models.User{ + UserID: 1, + Username: "username", + } + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(user, ctl)) + resorce := NewProjectNamespace(public.ProjectID).Resource(ResourceRepository) + assert.True(evaluator.HasPermission(context.TODO(), resorce, ActionPush)) + } - memberID, err := project.AddProjectMember(models.Member{ - ProjectID: projectID, - Role: common.RoleProjectAdmin, - EntityID: 1, - EntityType: "u", - }) - require.Nil(t, err) - defer project.DeleteProjectMemberByID(memberID) - evaluator1 := NewProjectUserEvaluator(&models.User{ - UserID: 1, - Username: "admin", - }, pm) - assert.True(evaluator1.HasPermission(NewProjectNamespace(projectID).Resource(ResourceRepository), ActionPush)) + { + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleGuest}, nil) - project.UpdateProjectMemberRole(memberID, common.RoleGuest) - evaluator2 := NewProjectUserEvaluator(&models.User{ - UserID: 1, - Username: "admin", - }, pm) - assert.False(evaluator2.HasPermission(NewProjectNamespace(projectID).Resource(ResourceRepository), ActionPush)) -} - -func BenchmarkProjectRBACEvaluator(b *testing.B) { - evaluator := NewProjectUserEvaluator(nil, publicProjectManager) - resource := NewProjectNamespace(projectID).Resource(ResourceRepository) - - b.ResetTimer() - for i := 0; i < b.N; i++ { - evaluator.HasPermission(resource, ActionPull) + user := &models.User{ + UserID: 1, + Username: "username", + } + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(user, ctl)) + resorce := NewProjectNamespace(public.ProjectID).Resource(ResourceRepository) + assert.False(evaluator.HasPermission(context.TODO(), resorce, ActionPush)) } } -func BenchmarkProjectRBACEvaluatorParallel(b *testing.B) { - evaluator := NewProjectUserEvaluator(nil, publicProjectManager) - resource := NewProjectNamespace(projectID).Resource(ResourceRepository) +func BenchmarkProjectEvaluator(b *testing.B) { + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleProjectAdmin}, nil) + + user := &models.User{ + UserID: 1, + Username: "username", + } + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(user, ctl)) + resource := NewProjectNamespace(public.ProjectID).Resource(ResourceRepository) + + b.ResetTimer() + for i := 0; i < b.N; i++ { + evaluator.HasPermission(context.TODO(), resource, ActionPull) + } +} + +func BenchmarkProjectEvaluatorParallel(b *testing.B) { + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleProjectAdmin}, nil) + + user := &models.User{ + UserID: 1, + Username: "username", + } + evaluator := NewProjectEvaluator(ctl, NewBuilderForUser(user, ctl)) + resource := NewProjectNamespace(public.ProjectID).Resource(ResourceRepository) b.RunParallel(func(pb *testing.PB) { for pb.Next() { - evaluator.HasPermission(resource, ActionPull) + evaluator.HasPermission(context.TODO(), resource, ActionPull) } }) } diff --git a/src/common/rbac/project_rbac_user.go b/src/common/rbac/project_rbac_user.go index 308675596..a93814bbd 100644 --- a/src/common/rbac/project_rbac_user.go +++ b/src/common/rbac/project_rbac_user.go @@ -23,37 +23,31 @@ type projectRBACUser struct { project *models.Project username string projectRoles []int + policies []*types.Policy } // GetUserName returns username of the visitor -func (user *projectRBACUser) GetUserName() string { - return user.username +func (pru *projectRBACUser) GetUserName() string { + return pru.username } // GetPolicies returns policies of the visitor -func (user *projectRBACUser) GetPolicies() []*types.Policy { - if user.project.IsPublic() { - return getPoliciesForPublicProject(user.project.ProjectID) +func (pru *projectRBACUser) GetPolicies() []*types.Policy { + policies := pru.policies + + if pru.project.IsPublic() { + policies = append(policies, getPoliciesForPublicProject(pru.project.ProjectID)...) } - return nil + return policies } // GetRoles returns roles of the visitor -func (user *projectRBACUser) GetRoles() []types.RBACRole { +func (pru *projectRBACUser) GetRoles() []types.RBACRole { roles := []types.RBACRole{} - for _, roleID := range user.projectRoles { - roles = append(roles, &projectRBACRole{projectID: user.project.ProjectID, roleID: roleID}) + for _, roleID := range pru.projectRoles { + roles = append(roles, &projectRBACRole{projectID: pru.project.ProjectID, roleID: roleID}) } return roles } - -// NewProjectRBACUser returns RBACUser for the project -func NewProjectRBACUser(project *models.Project, username string, projectRoles ...int) types.RBACUser { - return &projectRBACUser{ - project: project, - username: username, - projectRoles: projectRoles, - } -} diff --git a/src/common/security/context.go b/src/common/security/context.go index 8fb04ece9..2bb69f581 100644 --- a/src/common/security/context.go +++ b/src/common/security/context.go @@ -33,7 +33,7 @@ type Context interface { // IsSolutionUser returns whether the user is solution user IsSolutionUser() bool // Can returns whether the user can do action on resource - Can(action types.Action, resource types.Resource) bool + Can(ctx context.Context, action types.Action, resource types.Resource) bool } type securityKey struct{} diff --git a/src/common/security/local/context.go b/src/common/security/local/context.go index 23d69fd4c..7e6077680 100644 --- a/src/common/security/local/context.go +++ b/src/common/security/local/context.go @@ -15,11 +15,12 @@ package local import ( + "context" "sync" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/pkg/permission/evaluator" "github.com/goharbor/harbor/src/pkg/permission/evaluator/admin" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -28,16 +29,16 @@ import ( // SecurityContext implements security.Context interface based on database type SecurityContext struct { user *models.User - pm promgr.ProjectManager + ctl project.Controller evaluator evaluator.Evaluator once sync.Once } // NewSecurityContext ... -func NewSecurityContext(user *models.User, pm promgr.ProjectManager) *SecurityContext { +func NewSecurityContext(user *models.User) *SecurityContext { return &SecurityContext{ user: user, - pm: pm, + ctl: project.Ctl, } } @@ -80,16 +81,17 @@ func (s *SecurityContext) IsSolutionUser() bool { } // Can returns whether the user can do action on resource -func (s *SecurityContext) Can(action types.Action, resource types.Resource) bool { +func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource types.Resource) bool { s.once.Do(func() { var evaluators evaluator.Evaluators if s.IsSysAdmin() { evaluators = evaluators.Add(admin.New(s.GetUsername())) } - evaluators = evaluators.Add(rbac.NewProjectUserEvaluator(s.User(), s.pm)) + + evaluators = evaluators.Add(rbac.NewProjectEvaluator(s.ctl, rbac.NewBuilderForUser(s.user, s.ctl))) s.evaluator = evaluators }) - return s.evaluator != nil && s.evaluator.HasPermission(resource, action) + return s.evaluator != nil && s.evaluator.HasPermission(ctx, resource, action) } diff --git a/src/common/security/local/context_test.go b/src/common/security/local/context_test.go index 388cfceb0..0f73fa24c 100644 --- a/src/common/security/local/context_test.go +++ b/src/common/security/local/context_test.go @@ -15,26 +15,35 @@ package local import ( - "os" + "context" "testing" "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/dao" - "github.com/goharbor/harbor/src/common/dao/group" - "github.com/goharbor/harbor/src/common/dao/project" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/common/utils/test" - "github.com/goharbor/harbor/src/core/promgr" - "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" - "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/controller/project" + projecttesting "github.com/goharbor/harbor/src/testing/controller/project" + "github.com/goharbor/harbor/src/testing/mock" "github.com/stretchr/testify/assert" ) var ( + public = &project.Project{ + ProjectID: 1, + Name: "public_project", + OwnerID: 1, + Metadata: map[string]string{ + "public": "true", + }, + } + private = &models.Project{ - Name: "private_project", - OwnerID: 1, + ProjectID: 2, + Name: "private_project", + OwnerID: 1, + Metadata: map[string]string{ + "public": "false", + }, } projectAdminUser = &models.User{ @@ -49,245 +58,199 @@ var ( Username: "guestUser", Email: "guestUser@vmware.com", } - - pm = promgr.NewDefaultProjectManager(local.NewDriver(), true) ) -func TestMain(m *testing.M) { - - test.InitDatabaseFromEnv() - - // regiser users - id, err := dao.Register(*projectAdminUser) - if err != nil { - log.Fatalf("failed to register user: %v", err) - } - projectAdminUser.UserID = int(id) - defer dao.DeleteUser(int(id)) - - id, err = dao.Register(*developerUser) - if err != nil { - log.Fatalf("failed to register user: %v", err) - } - developerUser.UserID = int(id) - defer dao.DeleteUser(int(id)) - - id, err = dao.Register(*guestUser) - if err != nil { - log.Fatalf("failed to register user: %v", err) - } - guestUser.UserID = int(id) - defer dao.DeleteUser(int(id)) - - // add project - id, err = dao.AddProject(*private) - if err != nil { - log.Fatalf("failed to add project: %v", err) - } - private.ProjectID = id - defer dao.DeleteProject(id) - - var projectAdminPMID, developerUserPMID, guestUserPMID int - // add project members - projectAdminPMID, err = project.AddProjectMember(models.Member{ - ProjectID: private.ProjectID, - EntityID: projectAdminUser.UserID, - EntityType: common.UserMember, - Role: common.RoleProjectAdmin, - }) - if err != nil { - log.Fatalf("failed to add member: %v", err) - } - defer project.DeleteProjectMemberByID(projectAdminPMID) - - developerUserPMID, err = project.AddProjectMember(models.Member{ - ProjectID: private.ProjectID, - EntityID: developerUser.UserID, - EntityType: common.UserMember, - Role: common.RoleDeveloper, - }) - if err != nil { - log.Fatalf("failed to add member: %v", err) - } - defer project.DeleteProjectMemberByID(developerUserPMID) - guestUserPMID, err = project.AddProjectMember(models.Member{ - ProjectID: private.ProjectID, - EntityID: guestUser.UserID, - EntityType: common.UserMember, - Role: common.RoleGuest, - }) - if err != nil { - log.Fatalf("failed to add member: %v", err) - } - defer project.DeleteProjectMemberByID(guestUserPMID) - os.Exit(m.Run()) -} - func TestIsAuthenticated(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsAuthenticated()) // authenticated ctx = NewSecurityContext(&models.User{ Username: "test", - }, nil) + }) assert.True(t, ctx.IsAuthenticated()) } func TestGetUsername(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil) + ctx := NewSecurityContext(nil) assert.Equal(t, "", ctx.GetUsername()) // authenticated ctx = NewSecurityContext(&models.User{ Username: "test", - }, nil) + }) assert.Equal(t, "test", ctx.GetUsername()) } func TestIsSysAdmin(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsSysAdmin()) // authenticated, non admin ctx = NewSecurityContext(&models.User{ Username: "test", - }, nil) + }) assert.False(t, ctx.IsSysAdmin()) // authenticated, admin ctx = NewSecurityContext(&models.User{ Username: "test", SysAdminFlag: true, - }, nil) + }) assert.True(t, ctx.IsSysAdmin()) } func TestIsSolutionUser(t *testing.T) { - ctx := NewSecurityContext(nil, nil) + ctx := NewSecurityContext(nil) assert.False(t, ctx.IsSolutionUser()) } func TestHasPullPerm(t *testing.T) { - // public project - ctx := NewSecurityContext(nil, pm) + { + // public project + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(public, nil) - resource := rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository) - assert.True(t, ctx.Can(rbac.ActionPull, resource)) + ctx := NewSecurityContext(nil) + ctx.ctl = ctl + resource := rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } - // private project, unauthenticated - ctx = NewSecurityContext(nil, pm) - resource = rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - assert.False(t, ctx.Can(rbac.ActionPull, resource)) + { + // private project, unauthenticated + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) - // private project, authenticated, has no perm - ctx = NewSecurityContext(&models.User{ - Username: "test", - }, pm) - assert.False(t, ctx.Can(rbac.ActionPull, resource)) + ctx := NewSecurityContext(nil) + ctx.ctl = ctl + resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) + assert.False(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } - // private project, authenticated, has read perm - ctx = NewSecurityContext(guestUser, pm) - assert.True(t, ctx.Can(rbac.ActionPull, resource)) + { + // private project, authenticated, has no perm + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{}, nil) - // private project, authenticated, system admin - ctx = NewSecurityContext(&models.User{ - Username: "admin", - SysAdminFlag: true, - }, pm) - assert.True(t, ctx.Can(rbac.ActionPull, resource)) + ctx := NewSecurityContext(&models.User{Username: "test"}) + ctx.ctl = ctl + resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) + assert.False(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } + + { + // private project, authenticated, has read perm + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleGuest}, nil) + + ctx := NewSecurityContext(guestUser) + ctx.ctl = ctl + resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } + + { + // private project, authenticated, system admin + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + ctx := NewSecurityContext(&models.User{ + Username: "admin", + SysAdminFlag: true, + }) + ctx.ctl = ctl + resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } } func TestHasPushPerm(t *testing.T) { resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - // unauthenticated - ctx := NewSecurityContext(nil, pm) - assert.False(t, ctx.Can(rbac.ActionPush, resource)) + { + // unauthenticated + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) - // authenticated, has read perm - ctx = NewSecurityContext(guestUser, pm) - assert.False(t, ctx.Can(rbac.ActionPush, resource)) + ctx := NewSecurityContext(nil) + ctx.ctl = ctl + assert.False(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) + } - // authenticated, has write perm - ctx = NewSecurityContext(developerUser, pm) - assert.True(t, ctx.Can(rbac.ActionPush, resource)) + { + // authenticated, has read perm + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleGuest}, nil) - // authenticated, system admin - ctx = NewSecurityContext(&models.User{ - Username: "admin", - SysAdminFlag: true, - }, pm) - assert.True(t, ctx.Can(rbac.ActionPush, resource)) + ctx := NewSecurityContext(guestUser) + ctx.ctl = ctl + assert.False(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) + } + + { + // authenticated, has write perm + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleDeveloper}, nil) + + ctx := NewSecurityContext(developerUser) + ctx.ctl = ctl + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) + } + + { + // authenticated, system admin + ctl := &projecttesting.Controller{} + ctx := NewSecurityContext(&models.User{ + Username: "admin", + SysAdminFlag: true, + }) + ctx.ctl = ctl + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) + } } func TestHasPushPullPerm(t *testing.T) { resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - // unauthenticated - ctx := NewSecurityContext(nil, pm) - assert.False(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource)) + { + // unauthenticated + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) - // authenticated, has all perms - ctx = NewSecurityContext(projectAdminUser, pm) - assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource)) + ctx := NewSecurityContext(nil) + ctx.ctl = ctl + assert.False(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } - // authenticated, system admin - ctx = NewSecurityContext(&models.User{ - Username: "admin", - SysAdminFlag: true, - }, pm) - assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource)) -} - -func TestHasPushPullPermWithGroup(t *testing.T) { - PrepareGroupTest() - project, err := dao.GetProjectByName("group_project") - if err != nil { - t.Errorf("Error occurred when GetProjectByName: %v", err) - } - developer, err := dao.GetUser(models.User{Username: "sample01"}) - if err != nil { - t.Errorf("Error occurred when GetUser: %v", err) - } - - userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LDAPGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) - if err != nil { - t.Errorf("Failed to query user group %v", err) - } - if len(userGroups) < 1 { - t.Errorf("Failed to retrieve user group") - } - - developer.GroupIDs = []int{userGroups[0].ID} - - resource := rbac.NewProjectNamespace(project.ProjectID).Resource(rbac.ResourceRepository) - - ctx := NewSecurityContext(developer, pm) - assert.True(t, ctx.Can(rbac.ActionPush, resource)) - assert.True(t, ctx.Can(rbac.ActionPull, resource)) -} - -func PrepareGroupTest() { - initSqls := []string{ - `insert into user_group (group_name, group_type, ldap_group_dn) values ('harbor_group_01', 1, 'cn=harbor_user,dc=example,dc=com')`, - `insert into harbor_user (username, email, password, realname) values ('sample01', 'sample01@example.com', 'harbor12345', 'sample01')`, - `insert into project (name, owner_id) values ('group_project', 1)`, - `insert into project (name, owner_id) values ('group_project_private', 1)`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project'), 'public', 'false')`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project_private'), 'public', 'false')`, - `insert into project_member (project_id, entity_id, entity_type, role) values ((select project_id from project where name = 'group_project'), (select id from user_group where group_name = 'harbor_group_01'),'g', 2)`, - } - - clearSqls := []string{ - `delete from project_metadata where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from project where name in ('group_project', 'group_project_private')`, - `delete from project_member where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from user_group where group_name = 'harbor_group_01'`, - `delete from harbor_user where username = 'sample01'`, - } - dao.PrepareTestData(clearSqls, initSqls) + { + // authenticated, has all perms + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + mock.OnAnything(ctl, "ListRoles").Return([]int{common.RoleProjectAdmin}, nil) + + ctx := NewSecurityContext(projectAdminUser) + ctx.ctl = ctl + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } + + { + // authenticated, system admin + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + ctx := NewSecurityContext(&models.User{ + Username: "admin", + SysAdminFlag: true, + }) + ctx.ctl = ctl + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) + } } diff --git a/src/common/security/proxycachesecret/context.go b/src/common/security/proxycachesecret/context.go index 13a496f83..4847def31 100644 --- a/src/common/security/proxycachesecret/context.go +++ b/src/common/security/proxycachesecret/context.go @@ -17,12 +17,11 @@ package proxycachesecret import ( "context" - "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/types" - "github.com/goharbor/harbor/src/pkg/project" ) // const definition @@ -34,16 +33,14 @@ const ( // SecurityContext is the security context for proxy cache secret type SecurityContext struct { repository string - getProject func(interface{}) (*models.Project, error) + ctl project.Controller } // NewSecurityContext returns an instance of the proxy cache secret security context -func NewSecurityContext(ctx context.Context, repository string) *SecurityContext { +func NewSecurityContext(repository string) *SecurityContext { return &SecurityContext{ repository: repository, - getProject: func(i interface{}) (*models.Project, error) { - return project.Mgr.Get(ctx, i) - }, + ctl: project.Ctl, } } @@ -73,7 +70,7 @@ func (s *SecurityContext) IsSolutionUser() bool { } // Can returns true only when requesting pull/push operation against the specific project -func (s *SecurityContext) Can(action types.Action, resource types.Resource) bool { +func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource types.Resource) bool { if !(action == rbac.ActionPull || action == rbac.ActionPush) { log.Debugf("unauthorized for action %s", action) return false @@ -83,18 +80,16 @@ func (s *SecurityContext) Can(action types.Action, resource types.Resource) bool log.Debugf("got no namespace from the resource %s", resource) return false } - project, err := s.getProject(namespace.Identity()) + + p, err := s.ctl.Get(ctx, namespace.Identity().(int64)) if err != nil { log.Errorf("failed to get project %v: %v", namespace.Identity(), err) return false } - if project == nil { - log.Debugf("project not found %v", namespace.Identity()) - return false - } + pro, _ := utils.ParseRepository(s.repository) - if project.Name != pro { - log.Debugf("unauthorized for project %s", project.Name) + if p.Name != pro { + log.Debugf("unauthorized for project %s", p.Name) return false } return true diff --git a/src/common/security/proxycachesecret/context_test.go b/src/common/security/proxycachesecret/context_test.go index b8bd41ec1..238e42e08 100644 --- a/src/common/security/proxycachesecret/context_test.go +++ b/src/common/security/proxycachesecret/context_test.go @@ -16,28 +16,27 @@ package proxycachesecret import ( "context" + "errors" "testing" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" + projecttesting "github.com/goharbor/harbor/src/testing/controller/project" "github.com/goharbor/harbor/src/testing/mock" - "github.com/goharbor/harbor/src/testing/pkg/project" "github.com/stretchr/testify/suite" ) type proxyCacheSecretTestSuite struct { suite.Suite sc *SecurityContext - mgr *project.Manager + ctl *projecttesting.Controller } func (p *proxyCacheSecretTestSuite) SetupTest() { - p.mgr = &project.Manager{} + p.ctl = &projecttesting.Controller{} p.sc = &SecurityContext{ repository: "library/hello-world", - getProject: func(i interface{}) (*models.Project, error) { - return p.mgr.Get(context.TODO(), i) - }, + ctl: p.ctl, } } @@ -65,19 +64,19 @@ func (p *proxyCacheSecretTestSuite) TestCan() { // the action isn't pull/push action := rbac.ActionDelete resource := rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository) - p.False(p.sc.Can(action, resource)) + p.False(p.sc.Can(context.TODO(), action, resource)) // the resource isn't repository action = rbac.ActionPull resource = rbac.ResourceConfiguration - p.False(p.sc.Can(action, resource)) + p.False(p.sc.Can(context.TODO(), action, resource)) // the requested project not found action = rbac.ActionPull resource = rbac.NewProjectNamespace(2).Resource(rbac.ResourceRepository) - p.mgr.On("Get", mock.Anything, mock.Anything).Return(nil, nil) - p.False(p.sc.Can(action, resource)) - p.mgr.AssertExpectations(p.T()) + p.ctl.On("Get", mock.Anything, mock.Anything).Return(nil, errors.New("not found")) + p.False(p.sc.Can(context.TODO(), action, resource)) + p.ctl.AssertExpectations(p.T()) // reset the mock p.SetupTest() @@ -85,12 +84,12 @@ func (p *proxyCacheSecretTestSuite) TestCan() { // pass for action pull action = rbac.ActionPull resource = rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository) - p.mgr.On("Get", mock.Anything, mock.Anything).Return(&models.Project{ + p.ctl.On("Get", mock.Anything, mock.Anything).Return(&models.Project{ ProjectID: 1, Name: "library", }, nil) - p.True(p.sc.Can(action, resource)) - p.mgr.AssertExpectations(p.T()) + p.True(p.sc.Can(context.TODO(), action, resource)) + p.ctl.AssertExpectations(p.T()) // reset the mock p.SetupTest() @@ -98,12 +97,12 @@ func (p *proxyCacheSecretTestSuite) TestCan() { // pass for action push action = rbac.ActionPush resource = rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository) - p.mgr.On("Get", mock.Anything, mock.Anything).Return(&models.Project{ + p.ctl.On("Get", mock.Anything, mock.Anything).Return(&models.Project{ ProjectID: 1, Name: "library", }, nil) - p.True(p.sc.Can(action, resource)) - p.mgr.AssertExpectations(p.T()) + p.True(p.sc.Can(context.TODO(), action, resource)) + p.ctl.AssertExpectations(p.T()) } func TestProxyCacheSecretTestSuite(t *testing.T) { diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index 6cb011a35..39a2ac193 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -15,10 +15,12 @@ package robot import ( + "context" "sync" + "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/pkg/permission/evaluator" "github.com/goharbor/harbor/src/pkg/permission/types" "github.com/goharbor/harbor/src/pkg/robot/model" @@ -27,17 +29,17 @@ import ( // SecurityContext implements security.Context interface based on database type SecurityContext struct { robot *model.Robot - pm promgr.ProjectManager + ctl project.Controller policy []*types.Policy evaluator evaluator.Evaluator once sync.Once } // NewSecurityContext ... -func NewSecurityContext(robot *model.Robot, pm promgr.ProjectManager, policy []*types.Policy) *SecurityContext { +func NewSecurityContext(robot *model.Robot, policy []*types.Policy) *SecurityContext { return &SecurityContext{ + ctl: project.Ctl, robot: robot, - pm: pm, policy: policy, } } @@ -72,14 +74,26 @@ func (s *SecurityContext) IsSolutionUser() bool { } // Can returns whether the robot can do action on resource -func (s *SecurityContext) Can(action types.Action, resource types.Resource) bool { +func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource types.Resource) bool { s.once.Do(func() { - robotFactory := func(ns types.Namespace) types.RBACUser { - return NewRobot(s.GetUsername(), ns, s.policy) - } - - s.evaluator = rbac.NewProjectRobotEvaluator(s, s.pm, robotFactory) + s.evaluator = rbac.NewProjectEvaluator(s.ctl, rbac.NewBuilderForPolicies(s.GetUsername(), s.policy, filterRobotPolicies)) }) - return s.evaluator != nil && s.evaluator.HasPermission(resource, action) + return s.evaluator != nil && s.evaluator.HasPermission(ctx, resource, action) +} + +func filterRobotPolicies(p *models.Project, policies []*types.Policy) []*types.Policy { + namespace := rbac.NewProjectNamespace(p.ProjectID) + + var results []*types.Policy + for _, policy := range policies { + if types.ResourceAllowedInNamespace(policy.Resource, namespace) { + results = append(results, policy) + // give the PUSH action a pull access + if policy.Action == rbac.ActionPush { + results = append(results, &types.Policy{Resource: policy.Resource, Action: rbac.ActionPull}) + } + } + } + return results } diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go index 5fdaaef0a..4fa973cc6 100644 --- a/src/common/security/robot/context_test.go +++ b/src/common/security/robot/context_test.go @@ -15,19 +15,17 @@ package robot import ( + "context" "fmt" - "os" + "reflect" "testing" - "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/common/utils/test" - "github.com/goharbor/harbor/src/core/promgr" - "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" - "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/types" "github.com/goharbor/harbor/src/pkg/robot/model" + projecttesting "github.com/goharbor/harbor/src/testing/controller/project" + "github.com/goharbor/harbor/src/testing/mock" "github.com/stretchr/testify/assert" ) @@ -36,64 +34,49 @@ var ( Name: "testrobot", OwnerID: 1, } - pm = promgr.NewDefaultProjectManager(local.NewDriver(), true) ) -func TestMain(m *testing.M) { - test.InitDatabaseFromEnv() - - // add project - id, err := dao.AddProject(*private) - if err != nil { - log.Fatalf("failed to add project: %v", err) - } - private.ProjectID = id - defer dao.DeleteProject(id) - - os.Exit(m.Run()) -} - func TestIsAuthenticated(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil, nil) + ctx := NewSecurityContext(nil, nil) assert.False(t, ctx.IsAuthenticated()) // authenticated ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, - }, nil, nil) + }, nil) assert.True(t, ctx.IsAuthenticated()) } func TestGetUsername(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil, nil) + ctx := NewSecurityContext(nil, nil) assert.Equal(t, "", ctx.GetUsername()) // authenticated ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, - }, nil, nil) + }, nil) assert.Equal(t, "test", ctx.GetUsername()) } func TestIsSysAdmin(t *testing.T) { // unauthenticated - ctx := NewSecurityContext(nil, nil, nil) + ctx := NewSecurityContext(nil, nil) assert.False(t, ctx.IsSysAdmin()) // authenticated, non admin ctx = NewSecurityContext(&model.Robot{ Name: "test", Disabled: false, - }, nil, nil) + }, nil) assert.False(t, ctx.IsSysAdmin()) } func TestIsSolutionUser(t *testing.T) { - ctx := NewSecurityContext(nil, nil, nil) + ctx := NewSecurityContext(nil, nil) assert.False(t, ctx.IsSolutionUser()) } @@ -109,9 +92,13 @@ func TestHasPullPerm(t *testing.T) { Description: "desc", } - ctx := NewSecurityContext(robot, pm, policies) + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + ctx := NewSecurityContext(robot, policies) + ctx.ctl = ctl resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - assert.True(t, ctx.Can(rbac.ActionPull, resource)) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPull, resource)) } func TestHasPushPerm(t *testing.T) { @@ -126,9 +113,13 @@ func TestHasPushPerm(t *testing.T) { Description: "desc", } - ctx := NewSecurityContext(robot, pm, policies) + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + ctx := NewSecurityContext(robot, policies) + ctx.ctl = ctl resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - assert.True(t, ctx.Can(rbac.ActionPush, resource)) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource)) } func TestHasPushPullPerm(t *testing.T) { @@ -147,7 +138,56 @@ func TestHasPushPullPerm(t *testing.T) { Description: "desc", } - ctx := NewSecurityContext(robot, pm, policies) + ctl := &projecttesting.Controller{} + mock.OnAnything(ctl, "Get").Return(private, nil) + + ctx := NewSecurityContext(robot, policies) + ctx.ctl = ctl resource := rbac.NewProjectNamespace(private.ProjectID).Resource(rbac.ResourceRepository) - assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource)) + assert.True(t, ctx.Can(context.TODO(), rbac.ActionPush, resource) && ctx.Can(context.TODO(), rbac.ActionPull, resource)) +} + +func Test_filterRobotPolicies(t *testing.T) { + type args struct { + p *models.Project + policies []*types.Policy + } + tests := []struct { + name string + args args + want []*types.Policy + }{ + { + "policies of one project", + args{ + &models.Project{ProjectID: 1}, + []*types.Policy{ + {Resource: "/project/1/repository", Action: "pull", Effect: "allow"}, + }, + }, + []*types.Policy{ + {Resource: "/project/1/repository", Action: "pull", Effect: "allow"}, + }, + }, + { + "policies of multi projects", + args{ + &models.Project{ProjectID: 1}, + []*types.Policy{ + {Resource: "/project/1/repository", Action: "pull", Effect: "allow"}, + {Resource: "/project/2/repository", Action: "pull", Effect: "allow"}, + }, + }, + []*types.Policy{ + {Resource: "/project/1/repository", Action: "pull", Effect: "allow"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := filterRobotPolicies(tt.args.p, tt.args.policies); !reflect.DeepEqual(got, tt.want) { + t.Errorf("filterRobotPolicies() = %v, want %v", got, tt.want) + } + }) + } } diff --git a/src/common/security/robot/robot.go b/src/common/security/robot/robot.go deleted file mode 100644 index 48d9f7a62..000000000 --- a/src/common/security/robot/robot.go +++ /dev/null @@ -1,51 +0,0 @@ -package robot - -import ( - "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/pkg/permission/types" -) - -// robot implement the rbac.User interface for project robot account -type robot struct { - username string - namespace types.Namespace - policies []*types.Policy -} - -// GetUserName get the robot name. -func (r *robot) GetUserName() string { - return r.username -} - -// GetPolicies ... -func (r *robot) GetPolicies() []*types.Policy { - return r.policies -} - -// GetRoles robot has no definition of role, always return nil here. -func (r *robot) GetRoles() []types.RBACRole { - return nil -} - -// NewRobot ... -func NewRobot(username string, namespace types.Namespace, policies []*types.Policy) types.RBACUser { - return &robot{ - username: username, - namespace: namespace, - policies: filterPolicies(namespace, policies), - } -} - -func filterPolicies(namespace types.Namespace, policies []*types.Policy) []*types.Policy { - var results []*types.Policy - for _, policy := range policies { - if types.ResourceAllowedInNamespace(policy.Resource, namespace) { - results = append(results, policy) - // give the PUSH action a pull access - if policy.Action == rbac.ActionPush { - results = append(results, &types.Policy{Resource: policy.Resource, Action: rbac.ActionPull}) - } - } - } - return results -} diff --git a/src/common/security/robot/robot_test.go b/src/common/security/robot/robot_test.go deleted file mode 100644 index 1bf3abe4b..000000000 --- a/src/common/security/robot/robot_test.go +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright Project Harbor Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package robot - -import ( - "testing" - - "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/pkg/permission/types" - "github.com/stretchr/testify/assert" -) - -func TestGetPolicies(t *testing.T) { - - rbacPolicy := &types.Policy{ - Resource: "/project/libray/repository", - Action: "pull", - } - policies := []*types.Policy{} - policies = append(policies, rbacPolicy) - - robot := robot{ - username: "test", - namespace: rbac.NewProjectNamespace(1), - policies: policies, - } - - assert.Equal(t, robot.GetUserName(), "test") - assert.NotNil(t, robot.GetPolicies()) - assert.Nil(t, robot.GetRoles()) -} - -func TestNewRobot(t *testing.T) { - policies := []*types.Policy{ - {Resource: "/project/1/repository", Action: "push"}, - {Resource: "/project/1/repository", Action: "scanner-pull"}, - {Resource: "/project/library/repository", Action: "pull"}, - {Resource: "/project/library/repository", Action: "push"}, - } - - robot := NewRobot("test", rbac.NewProjectNamespace(1), policies) - assert.Len(t, robot.GetPolicies(), 3) -} diff --git a/src/common/security/secret/context.go b/src/common/security/secret/context.go index bb6f72561..6f639ff6a 100644 --- a/src/common/security/secret/context.go +++ b/src/common/security/secret/context.go @@ -15,6 +15,8 @@ package secret import ( + "context" + "github.com/goharbor/harbor/src/common/secret" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -75,7 +77,7 @@ func (s *SecurityContext) IsSolutionUser() bool { // Can returns whether the user can do action on resource // returns true if the corresponding user of the secret // is jobservice or core service, otherwise returns false -func (s *SecurityContext) Can(action types.Action, resource types.Resource) bool { +func (s *SecurityContext) Can(ctx context.Context, action types.Action, resource types.Resource) bool { if s.store == nil { return false } diff --git a/src/common/security/secret/context_test.go b/src/common/security/secret/context_test.go index a17328a8b..25167a222 100644 --- a/src/common/security/secret/context_test.go +++ b/src/common/security/secret/context_test.go @@ -15,6 +15,7 @@ package secret import ( + libctx "context" "testing" "github.com/goharbor/harbor/src/common/rbac" @@ -99,7 +100,7 @@ func TestHasPullPerm(t *testing.T) { resource := rbac.Resource("/project/project_name/repository") // secret store is null context := NewSecurityContext("", nil) - hasReadPerm := context.Can(rbac.ActionPull, resource) + hasReadPerm := context.Can(libctx.TODO(), rbac.ActionPull, resource) assert.False(t, hasReadPerm) // invalid secret @@ -107,7 +108,7 @@ func TestHasPullPerm(t *testing.T) { secret.NewStore(map[string]string{ "jobservice_secret": secret.JobserviceUser, })) - hasReadPerm = context.Can(rbac.ActionPull, resource) + hasReadPerm = context.Can(libctx.TODO(), rbac.ActionPull, resource) assert.False(t, hasReadPerm) // valid secret, project name @@ -115,12 +116,12 @@ func TestHasPullPerm(t *testing.T) { secret.NewStore(map[string]string{ "jobservice_secret": secret.JobserviceUser, })) - hasReadPerm = context.Can(rbac.ActionPull, resource) + hasReadPerm = context.Can(libctx.TODO(), rbac.ActionPull, resource) assert.True(t, hasReadPerm) // valid secret, project ID resource = rbac.Resource("/project/1/repository") - hasReadPerm = context.Can(rbac.ActionPull, resource) + hasReadPerm = context.Can(libctx.TODO(), rbac.ActionPull, resource) assert.True(t, hasReadPerm) } @@ -132,11 +133,11 @@ func TestHasPushPerm(t *testing.T) { // project name resource := rbac.Resource("/project/project_name/repository") - assert.False(t, context.Can(rbac.ActionPush, resource)) + assert.False(t, context.Can(libctx.TODO(), rbac.ActionPush, resource)) // project ID resource = rbac.Resource("/project/1/repository") - assert.False(t, context.Can(rbac.ActionPush, resource)) + assert.False(t, context.Can(libctx.TODO(), rbac.ActionPush, resource)) } func TestHasPushPullPerm(t *testing.T) { @@ -147,9 +148,9 @@ func TestHasPushPullPerm(t *testing.T) { // project name resource := rbac.Resource("/project/project_name/repository") - assert.False(t, context.Can(rbac.ActionPush, resource) && context.Can(rbac.ActionPull, resource)) + assert.False(t, context.Can(libctx.TODO(), rbac.ActionPush, resource) && context.Can(libctx.TODO(), rbac.ActionPull, resource)) // project ID resource = rbac.Resource("/project/1/repository") - assert.False(t, context.Can(rbac.ActionPush, resource) && context.Can(rbac.ActionPull, resource)) + assert.False(t, context.Can(libctx.TODO(), rbac.ActionPush, resource) && context.Can(libctx.TODO(), rbac.ActionPull, resource)) } diff --git a/src/common/security/v2token/context.go b/src/common/security/v2token/context.go index d794adde3..03f682674 100644 --- a/src/common/security/v2token/context.go +++ b/src/common/security/v2token/context.go @@ -7,9 +7,9 @@ import ( registry_token "github.com/docker/distribution/registry/auth/token" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/types" - "github.com/goharbor/harbor/src/pkg/project" "github.com/goharbor/harbor/src/pkg/project/models" ) @@ -17,10 +17,10 @@ import ( // The intention for this guy is only for support CLI push/pull. It should not be used in other scenario without careful review // Each request should have a different instance of tokenSecurityCtx type tokenSecurityCtx struct { - logger *log.Logger - name string - accessMap map[string]map[types.Action]struct{} - getProject func(int64) (*models.Project, error) + logger *log.Logger + name string + accessMap map[string]map[types.Action]struct{} + ctl project.Controller } func (t *tokenSecurityCtx) Name() string { @@ -51,7 +51,7 @@ func (t *tokenSecurityCtx) GetProjectRoles(projectIDOrName interface{}) []int { return []int{} } -func (t *tokenSecurityCtx) Can(action types.Action, resource types.Resource) bool { +func (t *tokenSecurityCtx) Can(ctx context.Context, action types.Action, resource types.Resource) bool { if !strings.HasSuffix(resource.String(), rbac.ResourceRepository.String()) { return false } @@ -65,7 +65,7 @@ func (t *tokenSecurityCtx) Can(action types.Action, resource types.Resource) boo t.logger.Warningf("Failed to get project id from namespace: %s", ns) return false } - p, err := t.getProject(pid) + p, err := t.ctl.Get(ctx, pid) if err != nil { t.logger.Warningf("Failed to get project, id: %d, error: %v", pid, err) return false @@ -114,8 +114,6 @@ func New(ctx context.Context, name string, access []*registry_token.ResourceActi logger: logger, name: name, accessMap: m, - getProject: func(id int64) (*models.Project, error) { - return project.Mgr.Get(ctx, id) - }, + ctl: project.Ctl, } } diff --git a/src/common/security/v2token/context_test.go b/src/common/security/v2token/context_test.go index 881c71757..cd1a52f5a 100644 --- a/src/common/security/v2token/context_test.go +++ b/src/common/security/v2token/context_test.go @@ -22,17 +22,17 @@ import ( "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/pkg/permission/types" "github.com/goharbor/harbor/src/pkg/project/models" - "github.com/goharbor/harbor/src/testing/pkg/project" + "github.com/goharbor/harbor/src/testing/controller/project" "github.com/stretchr/testify/assert" ) func TestAll(t *testing.T) { ctx := context.TODO() - mgr := &project.Manager{} - mgr.On("Get", ctx, int64(1)).Return(&models.Project{ProjectID: 1, Name: "library"}, nil) - mgr.On("Get", ctx, int64(2)).Return(&models.Project{ProjectID: 2, Name: "test"}, nil) - mgr.On("Get", ctx, int64(3)).Return(&models.Project{ProjectID: 3, Name: "development"}, nil) + ctl := &project.Controller{} + ctl.On("Get", ctx, int64(1)).Return(&models.Project{ProjectID: 1, Name: "library"}, nil) + ctl.On("Get", ctx, int64(2)).Return(&models.Project{ProjectID: 2, Name: "test"}, nil) + ctl.On("Get", ctx, int64(3)).Return(&models.Project{ProjectID: 3, Name: "development"}, nil) access := []*token.ResourceActions{ { @@ -63,9 +63,7 @@ func TestAll(t *testing.T) { } sc := New(context.Background(), "jack", access) tsc := sc.(*tokenSecurityCtx) - tsc.getProject = func(id int64) (*models.Project, error) { - return mgr.Get(ctx, id) - } + tsc.ctl = ctl cases := []struct { resource types.Resource @@ -115,6 +113,6 @@ func TestAll(t *testing.T) { } for _, c := range cases { - assert.Equal(t, c.expect, sc.Can(c.action, c.resource)) + assert.Equal(t, c.expect, sc.Can(ctx, c.action, c.resource)) } } diff --git a/src/controller/project/controller.go b/src/controller/project/controller.go index 1f95704c3..1f540fe90 100644 --- a/src/controller/project/controller.go +++ b/src/controller/project/controller.go @@ -58,6 +58,8 @@ type Controller interface { List(ctx context.Context, query *q.Query, options ...Option) ([]*models.Project, error) // Update update the project Update(ctx context.Context, project *models.Project) error + // ListRoles lists the roles of user for the specific project + ListRoles(ctx context.Context, projectID int64, u *user.User) ([]int, error) } // NewController creates an instance of the default project controller @@ -226,6 +228,10 @@ func (c *controller) Update(ctx context.Context, p *models.Project) error { return nil } +func (c *controller) ListRoles(ctx context.Context, projectID int64, u *user.User) ([]int, error) { + return c.projectMgr.ListRoles(ctx, projectID, u.UserID, u.GroupIDs...) +} + func (c *controller) assembleProjects(ctx context.Context, projects models.Projects, options ...Option) error { opts := newOptions(options...) if opts.WithMetadata { diff --git a/src/core/api/base.go b/src/core/api/base.go index 3c22aaa38..1304aaf8e 100644 --- a/src/core/api/base.go +++ b/src/core/api/base.go @@ -105,7 +105,7 @@ func (b *BaseController) HasProjectPermission(projectIDOrName interface{}, actio } resource := rbac.NewProjectNamespace(project.ProjectID).Resource(subresource...) - if !b.SecurityCtx.Can(action, resource) { + if !b.SecurityCtx.Can(b.Ctx.Request.Context(), action, resource) { return false, nil } diff --git a/src/core/api/user.go b/src/core/api/user.go index e42f2f138..7ca5b3748 100644 --- a/src/core/api/user.go +++ b/src/core/api/user.go @@ -486,9 +486,10 @@ func (ua *UserAPI) ListUserPermissions() { scope := rbac.Resource(ua.Ctx.Input.Query("scope")) policies := []*types.Policy{} + ctx := ua.Ctx.Request.Context() if ns, ok := types.NamespaceFromResource(scope); ok { for _, policy := range ns.GetPolicies() { - if ua.SecurityCtx.Can(policy.Action, policy.Resource) { + if ua.SecurityCtx.Can(ctx, policy.Action, policy.Resource) { policies = append(policies, policy) } } diff --git a/src/core/service/token/authutils.go b/src/core/service/token/authutils.go index e5ba6e8a1..5cba3e1bc 100644 --- a/src/core/service/token/authutils.go +++ b/src/core/service/token/authutils.go @@ -15,6 +15,8 @@ package token import ( + "context" + "fmt" "strings" "time" @@ -24,11 +26,11 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/core/config" - "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/lib/log" tokenpkg "github.com/goharbor/harbor/src/pkg/token" - "github.com/goharbor/harbor/src/pkg/token/claims/v2" + v2 "github.com/goharbor/harbor/src/pkg/token/claims/v2" ) const ( @@ -81,8 +83,12 @@ func GetResourceActions(scopes []string) []*token.ResourceActions { } // filterAccess iterate a list of resource actions and try to use the filter that matches the resource type to filter the actions. -func filterAccess(access []*token.ResourceActions, ctx security.Context, - pm promgr.ProjectManager, filters map[string]accessFilter) error { +func filterAccess(ctx context.Context, access []*token.ResourceActions, + ctl project.Controller, filters map[string]accessFilter) error { + secCtx, ok := security.FromContext(ctx) + if !ok { + return fmt.Errorf("failed to get security context from request") + } var err error for _, a := range access { f, ok := filters[a.Type] @@ -91,8 +97,8 @@ func filterAccess(access []*token.ResourceActions, ctx security.Context, log.Warningf("No filter found for access type: %s, skip filter, the access of resource '%s' will be set empty.", a.Type, a.Name) continue } - err = f.filter(ctx, pm, a) - log.Debugf("user: %s, access: %v", ctx.GetUsername(), a) + err = f.filter(ctx, ctl, a) + log.Debugf("user: %s, access: %v", secCtx.GetUsername(), a) if err != nil { log.Errorf("Failed to handle the resource %s:%s, due to error %v, returning empty access for it.", a.Type, a.Name, err) diff --git a/src/core/service/token/creator.go b/src/core/service/token/creator.go index de9347ff9..319844d7f 100644 --- a/src/core/service/token/creator.go +++ b/src/core/service/token/creator.go @@ -15,6 +15,7 @@ package token import ( + "context" "fmt" "net/http" "net/url" @@ -24,8 +25,9 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/controller/project" "github.com/goharbor/harbor/src/core/config" - "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" ) @@ -127,19 +129,21 @@ func parseImg(s string) (*image, error) { // An accessFilter will filter access based on userinfo type accessFilter interface { - filter(ctx security.Context, pm promgr.ProjectManager, a *token.ResourceActions) error + filter(ctx context.Context, ctl project.Controller, a *token.ResourceActions) error } type registryFilter struct { } -func (reg registryFilter) filter(ctx security.Context, pm promgr.ProjectManager, +func (reg registryFilter) filter(ctx context.Context, ctl project.Controller, a *token.ResourceActions) error { // Do not filter if the request is to access registry catalog if a.Name != "catalog" { return fmt.Errorf("Unable to handle, type: %s, name: %s", a.Type, a.Name) } - if !ctx.IsSysAdmin() { + + secCtx, ok := security.FromContext(ctx) + if !ok || !secCtx.IsSysAdmin() { // Set the actions to empty is the user is not admin a.Actions = []string{} } @@ -151,7 +155,7 @@ type repositoryFilter struct { parser imageParser } -func (rep repositoryFilter) filter(ctx security.Context, pm promgr.ProjectManager, +func (rep repositoryFilter) filter(ctx context.Context, ctl project.Controller, a *token.ResourceActions) error { // clear action list to assign to new acess element after perm check. img, err := rep.parser.parse(a.Name) @@ -161,24 +165,26 @@ func (rep repositoryFilter) filter(ctx security.Context, pm promgr.ProjectManage projectName := img.namespace permission := "" - project, err := pm.Get(projectName) + project, err := ctl.GetByName(ctx, projectName) if err != nil { + if errors.IsNotFoundErr(err) { + log.Debugf("project %s does not exist, set empty permission", projectName) + a.Actions = []string{} + return nil + } return err } - if project == nil { - log.Debugf("project %s does not exist, set empty permission", projectName) - a.Actions = []string{} - return nil - } + + secCtx, _ := security.FromContext(ctx) resource := rbac.NewProjectNamespace(project.ProjectID).Resource(rbac.ResourceRepository) - if ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource) { + if secCtx.Can(ctx, rbac.ActionPush, resource) && secCtx.Can(ctx, rbac.ActionPull, resource) { permission = "RWM" - } else if ctx.Can(rbac.ActionPush, resource) { + } else if secCtx.Can(ctx, rbac.ActionPush, resource) { permission = "RW" - } else if ctx.Can(rbac.ActionScannerPull, resource) { + } else if secCtx.Can(ctx, rbac.ActionScannerPull, resource) { permission = "RS" - } else if ctx.Can(rbac.ActionPull, resource) { + } else if secCtx.Can(ctx, rbac.ActionPull, resource) { permission = "R" } @@ -207,8 +213,6 @@ func (g generalCreator) Create(r *http.Request) (*models.Token, error) { return nil, fmt.Errorf("failed to get security context from request") } - pm := config.GlobalProjectMgr - // for docker login if !ctx.IsAuthenticated() { if len(scopes) == 0 { @@ -216,7 +220,7 @@ func (g generalCreator) Create(r *http.Request) (*models.Token, error) { } } access := GetResourceActions(scopes) - err = filterAccess(access, ctx, pm, g.filterMap) + err = filterAccess(r.Context(), access, project.Ctl, g.filterMap) if err != nil { return nil, err } diff --git a/src/core/service/token/token_test.go b/src/core/service/token/token_test.go index 6651dc194..bff9c6ef4 100644 --- a/src/core/service/token/token_test.go +++ b/src/core/service/token/token_test.go @@ -14,10 +14,7 @@ package token import ( - jwt "github.com/dgrijalva/jwt-go" - "github.com/docker/distribution/registry/auth/token" - "github.com/stretchr/testify/assert" - + "context" "crypto/rsa" "crypto/x509" "encoding/pem" @@ -29,9 +26,13 @@ import ( "runtime" "testing" + jwt "github.com/dgrijalva/jwt-go" + "github.com/docker/distribution/registry/auth/token" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/core/config" + "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { @@ -243,7 +244,7 @@ func (f *fakeSecurityContext) IsSysAdmin() bool { func (f *fakeSecurityContext) IsSolutionUser() bool { return false } -func (f *fakeSecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { +func (f *fakeSecurityContext) Can(ctx context.Context, action rbac.Action, resource rbac.Resource) bool { return false } func (f *fakeSecurityContext) GetMyProjects() ([]*models.Project, error) { @@ -271,21 +272,26 @@ func TestFilterAccess(t *testing.T) { Name: "catalog", Actions: []string{}, } - err = filterAccess(a1, &fakeSecurityContext{ + + ctx := func(secCtx security.Context) context.Context { + return security.NewContext(context.TODO(), secCtx) + } + + err = filterAccess(ctx(&fakeSecurityContext{ isAdmin: true, - }, nil, registryFilterMap) + }), a1, nil, registryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra1, *a1[0], "Mismatch after registry filter Map") - err = filterAccess(a2, &fakeSecurityContext{ + err = filterAccess(ctx(&fakeSecurityContext{ isAdmin: true, - }, nil, notaryFilterMap) + }), a2, nil, notaryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra2, *a2[0], "Mismatch after notary filter Map") - err = filterAccess(a3, &fakeSecurityContext{ + err = filterAccess(ctx(&fakeSecurityContext{ isAdmin: false, - }, nil, registryFilterMap) + }), a3, nil, registryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra2, *a3[0], "Mismatch after registry filter Map") } diff --git a/src/lib/orm/query.go b/src/lib/orm/query.go index db82155a7..e7ab62534 100644 --- a/src/lib/orm/query.go +++ b/src/lib/orm/query.go @@ -27,10 +27,19 @@ import ( "github.com/goharbor/harbor/src/lib/q" ) -// Params ... +// NewCondition alias function of orm.NewCondition +var NewCondition = orm.NewCondition + +// Condition alias to orm.Condition +type Condition = orm.Condition + +// Params alias to orm.Params type Params = orm.Params -// QuerySeter ... +// ParamsList alias to orm.ParamsList +type ParamsList = orm.ParamsList + +// QuerySeter alias to orm.QuerySeter type QuerySeter = orm.QuerySeter // Escape special characters diff --git a/src/pkg/permission/evaluator/admin/admin.go b/src/pkg/permission/evaluator/admin/admin.go index 8d4ec1c65..9bde5d8ee 100644 --- a/src/pkg/permission/evaluator/admin/admin.go +++ b/src/pkg/permission/evaluator/admin/admin.go @@ -15,6 +15,8 @@ package admin import ( + "context" + "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/evaluator" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -28,7 +30,7 @@ type Evaluator struct { } // HasPermission always return true for the system administrator -func (e *Evaluator) HasPermission(resource types.Resource, action types.Action) bool { +func (e *Evaluator) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { log.Debugf("system administrator %s require %s action for resource %s", e.username, action, resource) return true } diff --git a/src/pkg/permission/evaluator/evaluator.go b/src/pkg/permission/evaluator/evaluator.go index d6e3380f4..0f31fbfdb 100644 --- a/src/pkg/permission/evaluator/evaluator.go +++ b/src/pkg/permission/evaluator/evaluator.go @@ -15,13 +15,15 @@ package evaluator import ( + "context" + "github.com/goharbor/harbor/src/pkg/permission/types" ) // Evaluator the permission evaluator type Evaluator interface { // HasPermission returns true when user has action permission for the resource - HasPermission(resource types.Resource, action types.Action) bool + HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool } // Evaluators evaluator set @@ -53,9 +55,9 @@ func (evaluators Evaluators) Add(newEvaluators ...Evaluator) Evaluators { } // HasPermission returns true when one of evaluator has action permission for the resource -func (evaluators Evaluators) HasPermission(resource types.Resource, action types.Action) bool { +func (evaluators Evaluators) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { for _, evaluator := range evaluators { - if evaluator != nil && evaluator.HasPermission(resource, action) { + if evaluator != nil && evaluator.HasPermission(ctx, resource, action) { return true } } diff --git a/src/pkg/permission/evaluator/evaluator_test.go b/src/pkg/permission/evaluator/evaluator_test.go index cdd53bc10..15ffc327b 100644 --- a/src/pkg/permission/evaluator/evaluator_test.go +++ b/src/pkg/permission/evaluator/evaluator_test.go @@ -15,6 +15,7 @@ package evaluator import ( + "context" "testing" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -25,7 +26,7 @@ type mockEvaluator struct { name string } -func (e *mockEvaluator) HasPermission(resource types.Resource, action types.Action) bool { +func (e *mockEvaluator) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { return true } diff --git a/src/pkg/permission/evaluator/lazy/lazy.go b/src/pkg/permission/evaluator/lazy/lazy.go index 88efc786d..9596ce473 100644 --- a/src/pkg/permission/evaluator/lazy/lazy.go +++ b/src/pkg/permission/evaluator/lazy/lazy.go @@ -15,6 +15,7 @@ package lazy import ( + "context" "sync" "github.com/goharbor/harbor/src/pkg/permission/evaluator" @@ -34,12 +35,12 @@ type Evaluator struct { } // HasPermission returns true when user has action permission for the resource -func (l *Evaluator) HasPermission(resource types.Resource, action types.Action) bool { +func (l *Evaluator) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { l.once.Do(func() { l.evaluator = l.factory() }) - return l.evaluator != nil && l.evaluator.HasPermission(resource, action) + return l.evaluator != nil && l.evaluator.HasPermission(ctx, resource, action) } // New returns lazy evaluator diff --git a/src/pkg/permission/evaluator/namespace/namespace.go b/src/pkg/permission/evaluator/namespace/namespace.go index 5b55f2315..cbf442497 100644 --- a/src/pkg/permission/evaluator/namespace/namespace.go +++ b/src/pkg/permission/evaluator/namespace/namespace.go @@ -15,6 +15,7 @@ package namespace import ( + "context" "fmt" "sync" @@ -23,7 +24,7 @@ import ( ) // EvaluatorFactory returns the evaluator.Evaluator of the namespace -type EvaluatorFactory func(types.Namespace) evaluator.Evaluator +type EvaluatorFactory func(context.Context, types.Namespace) evaluator.Evaluator var _ evaluator.Evaluator = &Evaluator{} @@ -35,21 +36,21 @@ type Evaluator struct { } // HasPermission returns true when user has action permission for the resource -func (e *Evaluator) HasPermission(resource types.Resource, action types.Action) bool { +func (e *Evaluator) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { ns, ok := types.NamespaceFromResource(resource) if ok && ns.Kind() == e.namespaceKind { var eva evaluator.Evaluator - key := fmt.Sprintf("%s:%v", ns.Kind(), ns.Identity()) + key := fmt.Sprintf("%p:%s:%v", ctx, ns.Kind(), ns.Identity()) value, ok := e.cache.Load(key) if !ok { - eva = e.factory(ns) + eva = e.factory(ctx, ns) e.cache.Store(key, eva) } else { eva, _ = value.(evaluator.Evaluator) // maybe value is nil } - return eva != nil && eva.HasPermission(resource, action) + return eva != nil && eva.HasPermission(ctx, resource, action) } return false diff --git a/src/pkg/permission/evaluator/rbac/rbac.go b/src/pkg/permission/evaluator/rbac/rbac.go index 03c162155..1cddecc42 100644 --- a/src/pkg/permission/evaluator/rbac/rbac.go +++ b/src/pkg/permission/evaluator/rbac/rbac.go @@ -15,6 +15,7 @@ package rbac import ( + "context" "sync" "github.com/casbin/casbin" @@ -32,7 +33,7 @@ type Evaluator struct { } // HasPermission returns true when the rbac user has action permission for the resource -func (e *Evaluator) HasPermission(resource types.Resource, action types.Action) bool { +func (e *Evaluator) HasPermission(ctx context.Context, resource types.Resource, action types.Action) bool { e.once.Do(func() { e.enforcer = makeEnforcer(e.rbacUser) }) diff --git a/src/pkg/project/dao/dao.go b/src/pkg/project/dao/dao.go index 4a6533ee7..d6eed48ad 100644 --- a/src/pkg/project/dao/dao.go +++ b/src/pkg/project/dao/dao.go @@ -39,6 +39,8 @@ type DAO interface { GetByName(ctx context.Context, name string) (*models.Project, error) // List list projects List(ctx context.Context, query *q.Query) ([]*models.Project, error) + // Lists the roles of user for the specific project + ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error) } // New returns an instance of the default DAO @@ -173,3 +175,34 @@ func (d *dao) List(ctx context.Context, query *q.Query) ([]*models.Project, erro return projects, nil } + +func (d *dao) ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error) { + qs, err := orm.QuerySetter(ctx, &Member{}, nil) + if err != nil { + return nil, err + } + + conds := []*orm.Condition{ + orm.NewCondition().And("entity_type", "u").And("project_id", projectID).And("entity_id", userID), + } + if len(groupIDs) > 0 { + conds = append(conds, orm.NewCondition().And("entity_type", "g").And("project_id", projectID).And("entity_id__in", groupIDs)) + } + + cond := orm.NewCondition() + for _, c := range conds { + cond = cond.OrCond(c) + } + + var values orm.ParamsList + if _, err := qs.SetCond(cond).ValuesFlat(&values, "role"); err != nil { + return nil, err + } + + var roles []int + for _, value := range values { + roles = append(roles, int(value.(int64))) + } + + return roles, nil +} diff --git a/src/pkg/project/dao/dao_test.go b/src/pkg/project/dao/dao_test.go index bb9c367b0..7b6f4c9cc 100644 --- a/src/pkg/project/dao/dao_test.go +++ b/src/pkg/project/dao/dao_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/lib/q" @@ -339,6 +340,58 @@ func (suite *DaoTestSuite) TestListByMember() { } } +func (suite *DaoTestSuite) TestListRoles() { + { + // only projectAdmin + 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) + + roles, err := suite.dao.ListRoles(orm.Context(), projectID, int(userID)) + suite.Nil(err) + suite.Len(roles, 1) + suite.Contains(roles, common.RoleProjectAdmin) + }) + } + + { + // 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.RoleGuest, "g").QueryRow(&pid)) + defer o.Raw("DELETE FROM project_member WHERE id = ?", pid) + + roles, err := suite.dao.ListRoles(orm.Context(), projectID, int(userID), int(groupID)) + suite.Nil(err) + suite.Len(roles, 2) + suite.Contains(roles, common.RoleProjectAdmin) + suite.Contains(roles, common.RoleGuest) + }) + }) + } +} + func TestDaoTestSuite(t *testing.T) { suite.Run(t, &DaoTestSuite{}) } diff --git a/src/pkg/project/manager.go b/src/pkg/project/manager.go index 353defeb8..b7ff44145 100644 --- a/src/pkg/project/manager.go +++ b/src/pkg/project/manager.go @@ -46,6 +46,9 @@ type Manager interface { // List projects according to the query List(ctx context.Context, query *q.Query) ([]*models.Project, error) + + // ListRoles returns the roles of user for the specific project + ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error) } // New returns a default implementation of Manager @@ -111,3 +114,8 @@ func (m *manager) Get(ctx context.Context, idOrName interface{}) (*models.Projec func (m *manager) List(ctx context.Context, query *q.Query) ([]*models.Project, error) { return m.dao.List(ctx, query) } + +// Lists the roles of user for the specific project +func (m *manager) ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error) { + return m.dao.ListRoles(ctx, projectID, userID, groupIDs...) +} diff --git a/src/pkg/user/manager.go b/src/pkg/user/manager.go index ad3d08509..e075af356 100644 --- a/src/pkg/user/manager.go +++ b/src/pkg/user/manager.go @@ -24,6 +24,9 @@ import ( "github.com/goharbor/harbor/src/pkg/user/models" ) +// User alias to models.User +type User = models.User + var ( // Mgr is the global project manager Mgr = New() diff --git a/src/server/middleware/repoproxy/proxy_test.go b/src/server/middleware/repoproxy/proxy_test.go index 7c83d7669..4b7ee3d75 100644 --- a/src/server/middleware/repoproxy/proxy_test.go +++ b/src/server/middleware/repoproxy/proxy_test.go @@ -21,15 +21,13 @@ import ( "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/proxycachesecret" securitySecret "github.com/goharbor/harbor/src/common/security/secret" - "github.com/goharbor/harbor/src/core/config" ) func TestIsProxySession(t *testing.T) { - config.Init() - sc1 := securitySecret.NewSecurityContext("123456789", config.SecretStore) + sc1 := securitySecret.NewSecurityContext("123456789", nil) otherCtx := security.NewContext(context.Background(), sc1) - sc2 := proxycachesecret.NewSecurityContext(context.Background(), "library/hello-world") + sc2 := proxycachesecret.NewSecurityContext("library/hello-world") proxyCtx := security.NewContext(context.Background(), sc2) cases := []struct { name string diff --git a/src/server/middleware/security/auth_proxy.go b/src/server/middleware/security/auth_proxy.go index 363900d96..a8f1e6477 100644 --- a/src/server/middleware/security/auth_proxy.go +++ b/src/server/middleware/security/auth_proxy.go @@ -93,7 +93,7 @@ func (a *authProxy) Generate(req *http.Request) security.Context { } user.GroupIDs = u2.GroupIDs log.Debugf("an auth proxy security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(user, config.GlobalProjectMgr) + return local.NewSecurityContext(user) } func (a *authProxy) matchAuthProxyUserName(name string) (string, bool) { diff --git a/src/server/middleware/security/basic_auth.go b/src/server/middleware/security/basic_auth.go index 2d6c97a73..7f91a48aa 100644 --- a/src/server/middleware/security/basic_auth.go +++ b/src/server/middleware/security/basic_auth.go @@ -21,7 +21,6 @@ import ( "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/local" "github.com/goharbor/harbor/src/core/auth" - "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/lib/log" ) @@ -46,5 +45,5 @@ func (b *basicAuth) Generate(req *http.Request) security.Context { return nil } log.Debugf("a basic auth security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(user, config.GlobalProjectMgr) + return local.NewSecurityContext(user) } diff --git a/src/server/middleware/security/idtoken.go b/src/server/middleware/security/idtoken.go index 323065198..c37e326d3 100644 --- a/src/server/middleware/security/idtoken.go +++ b/src/server/middleware/security/idtoken.go @@ -68,5 +68,5 @@ func (i *idToken) Generate(req *http.Request) security.Context { } } log.Debugf("an ID token security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(u, config.GlobalProjectMgr) + return local.NewSecurityContext(u) } diff --git a/src/server/middleware/security/oidc_cli.go b/src/server/middleware/security/oidc_cli.go index 7790b39e4..76dd6b197 100644 --- a/src/server/middleware/security/oidc_cli.go +++ b/src/server/middleware/security/oidc_cli.go @@ -25,7 +25,6 @@ import ( "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/local" "github.com/goharbor/harbor/src/common/utils/oidc" - "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/log" ) @@ -61,7 +60,7 @@ func (o *oidcCli) Generate(req *http.Request) security.Context { return nil } logger.Debugf("an OIDC CLI security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(user, config.GlobalProjectMgr) + return local.NewSecurityContext(user) } func (o *oidcCli) valid(req *http.Request) bool { diff --git a/src/server/middleware/security/proxy_cache_secret.go b/src/server/middleware/security/proxy_cache_secret.go index 30ac757c0..2045d3229 100644 --- a/src/server/middleware/security/proxy_cache_secret.go +++ b/src/server/middleware/security/proxy_cache_secret.go @@ -38,5 +38,5 @@ func (p *proxyCacheSecret) Generate(req *http.Request) security.Context { return nil } log.Debugf("a proxy cache secret security context generated for request %s %s", req.Method, req.URL.Path) - return proxycachesecret.NewSecurityContext(req.Context(), artifact.Repository) + return proxycachesecret.NewSecurityContext(artifact.Repository) } diff --git a/src/server/middleware/security/robot.go b/src/server/middleware/security/robot.go index b289639f2..c34ba12a1 100644 --- a/src/server/middleware/security/robot.go +++ b/src/server/middleware/security/robot.go @@ -21,7 +21,6 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/security" robotCtx "github.com/goharbor/harbor/src/common/security/robot" - "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/lib/log" pkgrobot "github.com/goharbor/harbor/src/pkg/robot" pkg_token "github.com/goharbor/harbor/src/pkg/token" @@ -66,5 +65,5 @@ func (r *robot) Generate(req *http.Request) security.Context { return nil } log.Debugf("a robot security context generated for request %s %s", req.Method, req.URL.Path) - return robotCtx.NewSecurityContext(robot, config.GlobalProjectMgr, rtk.Claims.(*robot_claim.Claim).Access) + return robotCtx.NewSecurityContext(robot, rtk.Claims.(*robot_claim.Claim).Access) } diff --git a/src/server/middleware/security/session.go b/src/server/middleware/security/session.go index 5f6f8467d..2209cec82 100644 --- a/src/server/middleware/security/session.go +++ b/src/server/middleware/security/session.go @@ -22,7 +22,6 @@ import ( "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/local" - "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/lib/log" ) @@ -45,5 +44,5 @@ func (s *session) Generate(req *http.Request) security.Context { return nil } log.Debugf("a session security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(&user, config.GlobalProjectMgr) + return local.NewSecurityContext(&user) } diff --git a/src/server/middleware/security/unauthorized.go b/src/server/middleware/security/unauthorized.go index 06f420a34..f602595a3 100644 --- a/src/server/middleware/security/unauthorized.go +++ b/src/server/middleware/security/unauthorized.go @@ -19,7 +19,6 @@ import ( "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/security/local" - "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/lib/log" ) @@ -27,5 +26,5 @@ type unauthorized struct{} func (u *unauthorized) Generate(req *http.Request) security.Context { log.G(req.Context()).Debugf("an unauthorized security context generated for request %s %s", req.Method, req.URL.Path) - return local.NewSecurityContext(nil, config.GlobalProjectMgr) + return local.NewSecurityContext(nil) } diff --git a/src/server/middleware/util/util.go b/src/server/middleware/util/util.go index 16dc91f05..615cfa274 100644 --- a/src/server/middleware/util/util.go +++ b/src/server/middleware/util/util.go @@ -61,7 +61,7 @@ func SkipPolicyChecking(ctx context.Context, projectID int64) bool { // only scanner pull access can bypass. if ok && secCtx.Name() == "v2token" && - secCtx.Can(rbac.ActionScannerPull, rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceRepository)) { + secCtx.Can(ctx, rbac.ActionScannerPull, rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceRepository)) { return true } diff --git a/src/server/middleware/v2auth/auth.go b/src/server/middleware/v2auth/auth.go index 21db2cd3f..44d4aa9f0 100644 --- a/src/server/middleware/v2auth/auth.go +++ b/src/server/middleware/v2auth/auth.go @@ -16,8 +16,6 @@ package v2auth import ( "fmt" - "github.com/goharbor/harbor/src/lib" - lib_http "github.com/goharbor/harbor/src/lib/http" "net/http" "net/url" "strings" @@ -28,7 +26,9 @@ import ( "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/core/service/token" + "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/lib/errors" + lib_http "github.com/goharbor/harbor/src/lib/http" "github.com/goharbor/harbor/src/lib/log" ) @@ -63,7 +63,7 @@ func (rc *reqChecker) check(req *http.Request) (string, error) { return "", err } resource := rbac.NewProjectNamespace(pid).Resource(rbac.ResourceRepository) - if !securityCtx.Can(a.action, resource) { + if !securityCtx.Can(req.Context(), a.action, resource) { return getChallenge(req, al), fmt.Errorf("unauthorized to access repository: %s, action: %s", a.name, a.action) } } diff --git a/src/server/middleware/v2auth/auth_test.go b/src/server/middleware/v2auth/auth_test.go index b56084b92..f411ed197 100644 --- a/src/server/middleware/v2auth/auth_test.go +++ b/src/server/middleware/v2auth/auth_test.go @@ -108,7 +108,7 @@ func TestMiddleware(t *testing.T) { sc := &securitytesting.Context{} sc.On("IsAuthenticated").Return(true) sc.On("IsSysAdmin").Return(false) - mock.OnAnything(sc, "Can").Return(func(action types.Action, resource types.Resource) bool { + mock.OnAnything(sc, "Can").Return(func(ctx context.Context, action types.Action, resource types.Resource) bool { perms := map[string]map[rbac.Action]struct{}{ "/project/1/repository": { rbac.ActionPull: {}, diff --git a/src/server/v2.0/handler/base.go b/src/server/v2.0/handler/base.go index b3fe1ced0..e06d6d4df 100644 --- a/src/server/v2.0/handler/base.go +++ b/src/server/v2.0/handler/base.go @@ -57,7 +57,7 @@ func (*BaseAPI) HasPermission(ctx context.Context, action rbac.Action, resource return false } - return s.Can(action, resource) + return s.Can(ctx, action, resource) } // HasProjectPermission returns true when the request has action permission on project subresource diff --git a/src/testing/common/security/context.go b/src/testing/common/security/context.go index 27dfc706a..ce2646957 100644 --- a/src/testing/common/security/context.go +++ b/src/testing/common/security/context.go @@ -3,6 +3,8 @@ package security import ( + context "context" + mock "github.com/stretchr/testify/mock" types "github.com/goharbor/harbor/src/pkg/permission/types" @@ -13,13 +15,13 @@ type Context struct { mock.Mock } -// Can provides a mock function with given fields: action, resource -func (_m *Context) Can(action types.Action, resource types.Resource) bool { - ret := _m.Called(action, resource) +// Can provides a mock function with given fields: ctx, action, resource +func (_m *Context) Can(ctx context.Context, action types.Action, resource types.Resource) bool { + ret := _m.Called(ctx, action, resource) var r0 bool - if rf, ok := ret.Get(0).(func(types.Action, types.Resource) bool); ok { - r0 = rf(action, resource) + if rf, ok := ret.Get(0).(func(context.Context, types.Action, types.Resource) bool); ok { + r0 = rf(ctx, action, resource) } else { r0 = ret.Get(0).(bool) } diff --git a/src/testing/controller/project/controller.go b/src/testing/controller/project/controller.go index 19889b64b..c8c0ca7ad 100644 --- a/src/testing/controller/project/controller.go +++ b/src/testing/controller/project/controller.go @@ -164,6 +164,29 @@ func (_m *Controller) List(ctx context.Context, query *q.Query, options ...proje return r0, r1 } +// ListRoles provides a mock function with given fields: ctx, projectID, u +func (_m *Controller) ListRoles(ctx context.Context, projectID int64, u *models.User) ([]int, error) { + ret := _m.Called(ctx, projectID, u) + + var r0 []int + if rf, ok := ret.Get(0).(func(context.Context, int64, *models.User) []int); ok { + r0 = rf(ctx, projectID, u) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]int) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, int64, *models.User) error); ok { + r1 = rf(ctx, projectID, u) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // Update provides a mock function with given fields: ctx, _a1 func (_m *Controller) Update(ctx context.Context, _a1 *models.Project) error { ret := _m.Called(ctx, _a1) diff --git a/src/testing/pkg/project/manager.go b/src/testing/pkg/project/manager.go index 5444509c6..16e1e68e9 100644 --- a/src/testing/pkg/project/manager.go +++ b/src/testing/pkg/project/manager.go @@ -117,3 +117,33 @@ func (_m *Manager) List(ctx context.Context, query *q.Query) ([]*models.Project, return r0, r1 } + +// ListRoles provides a mock function with given fields: ctx, projectID, userID, groupIDs +func (_m *Manager) ListRoles(ctx context.Context, projectID int64, userID int, groupIDs ...int) ([]int, error) { + _va := make([]interface{}, len(groupIDs)) + for _i := range groupIDs { + _va[_i] = groupIDs[_i] + } + var _ca []interface{} + _ca = append(_ca, ctx, projectID, userID) + _ca = append(_ca, _va...) + ret := _m.Called(_ca...) + + var r0 []int + if rf, ok := ret.Get(0).(func(context.Context, int64, int, ...int) []int); ok { + r0 = rf(ctx, projectID, userID, groupIDs...) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]int) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(context.Context, int64, int, ...int) error); ok { + r1 = rf(ctx, projectID, userID, groupIDs...) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +}