diff --git a/src/common/rbac/const.go b/src/common/rbac/const.go index e0894d763..b0e1de59b 100644 --- a/src/common/rbac/const.go +++ b/src/common/rbac/const.go @@ -38,11 +38,14 @@ const ( ResourceHelmChartVersion = Resource("helm-chart-version") ResourceHelmChartVersionLabel = Resource("helm-chart-version-label") ResourceLabel = Resource("label") + ResourceLabelResource = Resource("label-resource") ResourceLog = Resource("log") ResourceMember = Resource("member") + ResourceMetadata = Resource("metadata") ResourceReplication = Resource("replication") ResourceReplicationJob = Resource("replication-job") ResourceRepository = Resource("repository") + ResourceRepositoryLabel = Resource("repository-label") ResourceRepositoryTag = Resource("repository-tag") ResourceRepositoryTagLabel = Resource("repository-tag-label") ResourceRepositoryTagManifest = Resource("repository-tag-manifest") diff --git a/src/common/rbac/namespace.go b/src/common/rbac/namespace.go index ad85cc5e6..7f4f0f6a3 100644 --- a/src/common/rbac/namespace.go +++ b/src/common/rbac/namespace.go @@ -52,6 +52,10 @@ func (ns *projectNamespace) IsPublic() bool { } // NewProjectNamespace returns namespace for project -func NewProjectNamespace(projectIDOrName interface{}, isPublic bool) Namespace { - return &projectNamespace{projectIDOrName: projectIDOrName, isPublic: isPublic} +func NewProjectNamespace(projectIDOrName interface{}, isPublic ...bool) Namespace { + isPublicNamespace := false + if len(isPublic) > 0 { + isPublicNamespace = isPublic[0] + } + return &projectNamespace{projectIDOrName: projectIDOrName, isPublic: isPublicNamespace} } diff --git a/src/common/rbac/project/util.go b/src/common/rbac/project/util.go index 447e7c9d1..75dd8b13d 100644 --- a/src/common/rbac/project/util.go +++ b/src/common/rbac/project/util.go @@ -23,9 +23,23 @@ var ( publicProjectPolicies = []*rbac.Policy{ {Resource: rbac.ResourceSelf, Action: rbac.ActionRead}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, + + {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, + {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionList}, + + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, + + {Resource: rbac.ResourceRepositoryTagVulnerability, Action: rbac.ActionList}, + + {Resource: rbac.ResourceRepositoryTagManifest, Action: rbac.ActionRead}, + {Resource: rbac.ResourceHelmChart, Action: rbac.ActionRead}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionList}, @@ -44,10 +58,16 @@ var ( {Resource: rbac.ResourceMember, Action: rbac.ActionDelete}, {Resource: rbac.ResourceMember, Action: rbac.ActionList}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionRead}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionUpdate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceLog, Action: rbac.ActionList}, {Resource: rbac.ResourceReplication, Action: rbac.ActionList}, {Resource: rbac.ResourceReplication, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceReplication, Action: rbac.ActionRead}, {Resource: rbac.ResourceReplication, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceReplication, Action: rbac.ActionDelete}, @@ -56,17 +76,25 @@ var ( {Resource: rbac.ResourceReplicationJob, Action: rbac.ActionList}, {Resource: rbac.ResourceLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, {Resource: rbac.ResourceLabel, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceLabel, Action: rbac.ActionDelete}, {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceLabelResource, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepository, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceRepository, Action: rbac.ActionDelete}, {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, - {Resource: rbac.ResourceRepository, Action: rbac.ActionPushPull}, // compatible with security all perm of project - {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionPushPull}, // compatible with security all perm of project + + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionDelete}, @@ -81,12 +109,14 @@ var ( {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionCreate}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionRead}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionDelete}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionList}, + {Resource: rbac.ResourceHelmChartVersion, Action: rbac.ActionCreate}, {Resource: rbac.ResourceHelmChartVersion, Action: rbac.ActionRead}, {Resource: rbac.ResourceHelmChartVersion, Action: rbac.ActionDelete}, {Resource: rbac.ResourceHelmChartVersion, Action: rbac.ActionList}, diff --git a/src/common/rbac/project/visitor_role.go b/src/common/rbac/project/visitor_role.go index ac499887d..97aeae87f 100644 --- a/src/common/rbac/project/visitor_role.go +++ b/src/common/rbac/project/visitor_role.go @@ -31,6 +31,11 @@ var ( {Resource: rbac.ResourceMember, Action: rbac.ActionDelete}, {Resource: rbac.ResourceMember, Action: rbac.ActionList}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionRead}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionUpdate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceLog, Action: rbac.ActionList}, {Resource: rbac.ResourceReplication, Action: rbac.ActionRead}, @@ -40,18 +45,25 @@ var ( {Resource: rbac.ResourceReplicationJob, Action: rbac.ActionList}, {Resource: rbac.ResourceLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, {Resource: rbac.ResourceLabel, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceLabel, Action: rbac.ActionDelete}, {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceLabelResource, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepository, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceRepository, Action: rbac.ActionDelete}, {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, - {Resource: rbac.ResourceRepository, Action: rbac.ActionPushPull}, // compatible with security all perm of project - {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, - {Resource: rbac.ResourceRepository, Action: rbac.ActionPushPull}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionPushPull}, // compatible with security all perm of project + + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionDelete}, @@ -66,6 +78,7 @@ var ( {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionCreate}, // upload helm chart {Resource: rbac.ResourceHelmChart, Action: rbac.ActionRead}, // download helm chart @@ -95,23 +108,34 @@ var ( {Resource: rbac.ResourceMember, Action: rbac.ActionList}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionRead}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionUpdate}, + {Resource: rbac.ResourceMetadata, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceLog, Action: rbac.ActionList}, {Resource: rbac.ResourceReplication, Action: rbac.ActionRead}, {Resource: rbac.ResourceReplication, Action: rbac.ActionList}, {Resource: rbac.ResourceLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, {Resource: rbac.ResourceLabel, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceLabel, Action: rbac.ActionDelete}, {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceRepository, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepository, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceRepository, Action: rbac.ActionDelete}, {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionDelete}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionList}, @@ -125,6 +149,7 @@ var ( {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionCreate}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionRead}, @@ -140,7 +165,9 @@ var ( {Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionDelete}, {Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead}, - {Resource: rbac.ResourceConfiguration, Action: rbac.ActionUpdate}, + + {Resource: rbac.ResourceRobot, Action: rbac.ActionRead}, + {Resource: rbac.ResourceRobot, Action: rbac.ActionList}, }, "developer": { @@ -150,11 +177,20 @@ var ( {Resource: rbac.ResourceLog, Action: rbac.ActionList}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionRead}, + {Resource: rbac.ResourceRepository, Action: rbac.ActionUpdate}, {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPush}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionList}, @@ -164,6 +200,7 @@ var ( {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionDelete}, + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionCreate}, {Resource: rbac.ResourceHelmChart, Action: rbac.ActionRead}, @@ -177,6 +214,9 @@ var ( {Resource: rbac.ResourceHelmChartVersionLabel, Action: rbac.ActionDelete}, {Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceRobot, Action: rbac.ActionRead}, + {Resource: rbac.ResourceRobot, Action: rbac.ActionList}, }, "guest": { @@ -186,12 +226,20 @@ var ( {Resource: rbac.ResourceLog, Action: rbac.ActionList}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionRead}, + {Resource: rbac.ResourceLabel, Action: rbac.ActionList}, + + {Resource: rbac.ResourceRepository, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepository, Action: rbac.ActionList}, {Resource: rbac.ResourceRepository, Action: rbac.ActionPull}, + {Resource: rbac.ResourceRepositoryLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionRead}, {Resource: rbac.ResourceRepositoryTag, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepositoryTagLabel, Action: rbac.ActionList}, + {Resource: rbac.ResourceRepositoryTagVulnerability, Action: rbac.ActionList}, {Resource: rbac.ResourceRepositoryTagManifest, Action: rbac.ActionRead}, @@ -203,6 +251,9 @@ var ( {Resource: rbac.ResourceHelmChartVersion, Action: rbac.ActionList}, {Resource: rbac.ResourceConfiguration, Action: rbac.ActionRead}, + + {Resource: rbac.ResourceRobot, Action: rbac.ActionRead}, + {Resource: rbac.ResourceRobot, Action: rbac.ActionList}, }, } ) diff --git a/src/common/security/admiral/context.go b/src/common/security/admiral/context.go index 9abc5faea..962a6dafb 100644 --- a/src/common/security/admiral/context.go +++ b/src/common/security/admiral/context.go @@ -69,24 +69,6 @@ func (s *SecurityContext) IsSolutionUser() bool { return false } -// HasReadPerm returns whether the user has read permission to the project -func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasWritePerm returns whether the user has write permission to the project -func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPush, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasAllPerm returns whether the user has all permissions to the project -func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPushPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - // Can returns whether the user can do action on resource func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { ns, err := resource.GetNamespace() diff --git a/src/common/security/context.go b/src/common/security/context.go index d1b9af92b..4b879a218 100644 --- a/src/common/security/context.go +++ b/src/common/security/context.go @@ -29,12 +29,6 @@ type Context interface { IsSysAdmin() bool // IsSolutionUser returns whether the user is solution user IsSolutionUser() bool - // HasReadPerm returns whether the user has read permission to the project - HasReadPerm(projectIDOrName interface{}) bool - // HasWritePerm returns whether the user has write permission to the project - HasWritePerm(projectIDOrName interface{}) bool - // HasAllPerm returns whether the user has all permissions to the project - HasAllPerm(projectIDOrName interface{}) bool // Get current user's all project GetMyProjects() ([]*models.Project, error) // Get user's role in provided project diff --git a/src/common/security/local/context.go b/src/common/security/local/context.go index f0d33ceed..655fe34b1 100644 --- a/src/common/security/local/context.go +++ b/src/common/security/local/context.go @@ -67,24 +67,6 @@ func (s *SecurityContext) IsSolutionUser() bool { return false } -// HasReadPerm returns whether the user has read permission to the project -func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasWritePerm returns whether the user has write permission to the project -func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPush, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasAllPerm returns whether the user has all permissions to the project -func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPushPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - // Can returns whether the user can do action on resource func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { ns, err := resource.GetNamespace() diff --git a/src/common/security/local/context_test.go b/src/common/security/local/context_test.go index 976237fd6..80b40818b 100644 --- a/src/common/security/local/context_test.go +++ b/src/common/security/local/context_test.go @@ -23,6 +23,7 @@ import ( "github.com/goharbor/harbor/src/common/dao" "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/log" "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" @@ -210,66 +211,73 @@ func TestIsSolutionUser(t *testing.T) { func TestHasReadPerm(t *testing.T) { // public project ctx := NewSecurityContext(nil, pm) - assert.True(t, ctx.HasReadPerm("library")) + + resource := rbac.NewProjectNamespace("library").Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(rbac.ActionPull, resource)) // private project, unauthenticated ctx = NewSecurityContext(nil, pm) - assert.False(t, ctx.HasReadPerm(private.Name)) + resource = rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + assert.False(t, ctx.Can(rbac.ActionPull, resource)) // private project, authenticated, has no perm ctx = NewSecurityContext(&models.User{ Username: "test", }, pm) - assert.False(t, ctx.HasReadPerm(private.Name)) + assert.False(t, ctx.Can(rbac.ActionPull, resource)) // private project, authenticated, has read perm ctx = NewSecurityContext(guestUser, pm) - assert.True(t, ctx.HasReadPerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPull, resource)) // private project, authenticated, system admin ctx = NewSecurityContext(&models.User{ Username: "admin", HasAdminRole: true, }, pm) - assert.True(t, ctx.HasReadPerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPull, resource)) } func TestHasWritePerm(t *testing.T) { + resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + // unauthenticated ctx := NewSecurityContext(nil, pm) - assert.False(t, ctx.HasWritePerm(private.Name)) + assert.False(t, ctx.Can(rbac.ActionPush, resource)) // authenticated, has read perm ctx = NewSecurityContext(guestUser, pm) - assert.False(t, ctx.HasWritePerm(private.Name)) + assert.False(t, ctx.Can(rbac.ActionPush, resource)) // authenticated, has write perm ctx = NewSecurityContext(developerUser, pm) - assert.True(t, ctx.HasWritePerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPush, resource)) // authenticated, system admin ctx = NewSecurityContext(&models.User{ Username: "admin", HasAdminRole: true, }, pm) - assert.True(t, ctx.HasReadPerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPush, resource)) } func TestHasAllPerm(t *testing.T) { + resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + // unauthenticated ctx := NewSecurityContext(nil, pm) - assert.False(t, ctx.HasAllPerm(private.Name)) + assert.False(t, ctx.Can(rbac.ActionPushPull, resource)) // authenticated, has all perms ctx = NewSecurityContext(projectAdminUser, pm) - assert.True(t, ctx.HasAllPerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPushPull, resource)) // authenticated, system admin ctx = NewSecurityContext(&models.User{ Username: "admin", HasAdminRole: true, }, pm) - assert.True(t, ctx.HasAllPerm(private.Name)) + assert.True(t, ctx.Can(rbac.ActionPushPull, resource)) } func TestHasAllPermWithGroup(t *testing.T) { @@ -285,10 +293,13 @@ func TestHasAllPermWithGroup(t *testing.T) { developer.GroupList = []*models.UserGroup{ {GroupName: "test_group", GroupType: 1, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}, } + + resource := rbac.NewProjectNamespace(project.Name).Resource(rbac.ResourceRepository) + ctx := NewSecurityContext(developer, pm) - assert.False(t, ctx.HasAllPerm(project.Name)) - assert.True(t, ctx.HasWritePerm(project.Name)) - assert.True(t, ctx.HasReadPerm(project.Name)) + assert.False(t, ctx.Can(rbac.ActionPushPull, resource)) + assert.True(t, ctx.Can(rbac.ActionPush, resource)) + assert.True(t, ctx.Can(rbac.ActionPull, resource)) } func TestGetMyProjects(t *testing.T) { diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index 3b48b91bc..49d80ef35 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -60,24 +60,6 @@ func (s *SecurityContext) IsSolutionUser() bool { return false } -// HasReadPerm returns whether the user has read permission to the project -func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasWritePerm returns whether the user has write permission to the project -func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPush, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - -// HasAllPerm returns whether the user has all permissions to the project -func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - isPublicProject, _ := s.pm.IsPublic(projectIDOrName) - return s.Can(rbac.ActionPushPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(rbac.ResourceRepository)) -} - // GetMyProjects no implementation func (s *SecurityContext) GetMyProjects() ([]*models.Project, error) { return nil, nil diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go index df7869a90..46225b52a 100644 --- a/src/common/security/robot/context_test.go +++ b/src/common/security/robot/context_test.go @@ -16,6 +16,7 @@ package robot import ( "os" + "strconv" "testing" "github.com/goharbor/harbor/src/common/dao" @@ -26,7 +27,6 @@ import ( "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "strconv" ) var ( @@ -147,7 +147,8 @@ func TestHasReadPerm(t *testing.T) { } ctx := NewSecurityContext(robot, pm, policies) - assert.True(t, ctx.HasReadPerm(private.Name)) + resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(rbac.ActionPull, resource)) } func TestHasWritePerm(t *testing.T) { @@ -164,7 +165,8 @@ func TestHasWritePerm(t *testing.T) { } ctx := NewSecurityContext(robot, pm, policies) - assert.True(t, ctx.HasWritePerm(private.Name)) + resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(rbac.ActionPush, resource)) } func TestHasAllPerm(t *testing.T) { @@ -180,7 +182,8 @@ func TestHasAllPerm(t *testing.T) { } ctx := NewSecurityContext(robot, pm, policies) - assert.True(t, ctx.HasAllPerm(private.Name)) + resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository) + assert.True(t, ctx.Can(rbac.ActionPushPull, resource)) } func TestGetMyProjects(t *testing.T) { diff --git a/src/common/security/secret/context.go b/src/common/security/secret/context.go index 0155320d8..5dc06137a 100644 --- a/src/common/security/secret/context.go +++ b/src/common/security/secret/context.go @@ -71,34 +71,9 @@ func (s *SecurityContext) IsSolutionUser() bool { return s.IsAuthenticated() } -// HasReadPerm returns true if the corresponding user of the secret -// is jobservice or core service, otherwise returns false -func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - if s.store == nil { - return false - } - return s.store.GetUsername(s.secret) == secret.JobserviceUser || s.store.GetUsername(s.secret) == secret.CoreUser -} - -// HasWritePerm returns true if the corresponding user of the secret -// is jobservice or core service, otherwise returns false -func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - if s.store == nil { - return false - } - return s.store.GetUsername(s.secret) == secret.JobserviceUser || s.store.GetUsername(s.secret) == secret.CoreUser -} - -// HasAllPerm returns true if the corresponding user of the secret -// is jobservice or core service, otherwise returns false -func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - if s.store == nil { - return false - } - return s.store.GetUsername(s.secret) == secret.JobserviceUser || s.store.GetUsername(s.secret) == secret.CoreUser -} - // 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 rbac.Action, resource rbac.Resource) bool { if s.store == nil { return false @@ -114,7 +89,9 @@ func (s *SecurityContext) GetMyProjects() ([]*models.Project, error) { // GetProjectRoles return guest role if has read permission, otherwise return nil func (s *SecurityContext) GetProjectRoles(projectIDOrName interface{}) []int { roles := []int{} - if s.HasReadPerm(projectIDOrName) { + if s.store != nil && + (s.store.GetUsername(s.secret) == secret.JobserviceUser || + s.store.GetUsername(s.secret) == secret.CoreUser) { roles = append(roles, common.RoleGuest) } return roles diff --git a/src/common/security/secret/context_test.go b/src/common/security/secret/context_test.go index ace3d5dc6..2e743da2b 100644 --- a/src/common/security/secret/context_test.go +++ b/src/common/security/secret/context_test.go @@ -18,6 +18,7 @@ import ( "testing" "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/secret" "github.com/stretchr/testify/assert" ) @@ -96,9 +97,11 @@ func TestIsSolutionUser(t *testing.T) { } func TestHasReadPerm(t *testing.T) { + readAction := rbac.Action("pull") + resource := rbac.Resource("/project/project_name/repository") // secret store is null context := NewSecurityContext("", nil) - hasReadPerm := context.HasReadPerm("project_name") + hasReadPerm := context.Can(readAction, resource) assert.False(t, hasReadPerm) // invalid secret @@ -106,7 +109,7 @@ func TestHasReadPerm(t *testing.T) { secret.NewStore(map[string]string{ "jobservice_secret": secret.JobserviceUser, })) - hasReadPerm = context.HasReadPerm("project_name") + hasReadPerm = context.Can(readAction, resource) assert.False(t, hasReadPerm) // valid secret, project name @@ -114,11 +117,12 @@ func TestHasReadPerm(t *testing.T) { secret.NewStore(map[string]string{ "jobservice_secret": secret.JobserviceUser, })) - hasReadPerm = context.HasReadPerm("project_name") + hasReadPerm = context.Can(readAction, resource) assert.True(t, hasReadPerm) // valid secret, project ID - hasReadPerm = context.HasReadPerm(1) + resource = rbac.Resource("/project/1/repository") + hasReadPerm = context.Can(readAction, resource) assert.True(t, hasReadPerm) } @@ -128,12 +132,16 @@ func TestHasWritePerm(t *testing.T) { "secret": "username", })) + writeAction := rbac.Action("push") + // project name - hasWritePerm := context.HasWritePerm("project_name") + resource := rbac.Resource("/project/project_name/repository") + hasWritePerm := context.Can(writeAction, resource) assert.False(t, hasWritePerm) // project ID - hasWritePerm = context.HasWritePerm(1) + resource = rbac.Resource("/project/1/repository") + hasWritePerm = context.Can(writeAction, resource) assert.False(t, hasWritePerm) } @@ -143,12 +151,16 @@ func TestHasAllPerm(t *testing.T) { "secret": "username", })) + allAction := rbac.Action("push+pull") + // project name - hasAllPerm := context.HasAllPerm("project_name") + resource := rbac.Resource("/project/project_name/repository") + hasAllPerm := context.Can(allAction, resource) assert.False(t, hasAllPerm) // project ID - hasAllPerm = context.HasAllPerm(1) + resource = rbac.Resource("/project/1/repository") + hasAllPerm = context.Can(allAction, resource) assert.False(t, hasAllPerm) } diff --git a/src/core/api/chart_label.go b/src/core/api/chart_label.go index 2b6f697e0..f3de48c9c 100644 --- a/src/core/api/chart_label.go +++ b/src/core/api/chart_label.go @@ -6,6 +6,7 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" ) const ( @@ -45,12 +46,6 @@ func (cla *ChartLabelAPI) Prepare() { } cla.project = existingProject - // Check permission - if !cla.checkPermissions(project) { - cla.SendForbiddenError(errors.New(cla.SecurityCtx.GetUsername())) - return - } - // Check the existence of target chart chartName := cla.GetStringFromPath(nameParam) version := cla.GetStringFromPath(versionParam) @@ -62,8 +57,23 @@ func (cla *ChartLabelAPI) Prepare() { cla.chartFullName = fmt.Sprintf("%s/%s:%s", project, chartName, version) } +func (cla *ChartLabelAPI) requireAccess(action rbac.Action) bool { + resource := rbac.NewProjectNamespace(cla.project.ProjectID).Resource(rbac.ResourceHelmChartVersionLabel) + + if !cla.SecurityCtx.Can(action, resource) { + cla.HandleForbidden(cla.SecurityCtx.GetUsername()) + return false + } + + return true +} + // MarkLabel handles the request of marking label to chart. func (cla *ChartLabelAPI) MarkLabel() { + if !cla.requireAccess(rbac.ActionCreate) { + return + } + l := &models.Label{} cla.DecodeJSONReq(l) @@ -83,6 +93,10 @@ func (cla *ChartLabelAPI) MarkLabel() { // RemoveLabel handles the request of removing label from chart. func (cla *ChartLabelAPI) RemoveLabel() { + if !cla.requireAccess(rbac.ActionDelete) { + return + } + lID, err := cla.GetInt64FromPath(idParam) if err != nil { cla.SendInternalServerError(err) diff --git a/src/core/api/chart_repository.go b/src/core/api/chart_repository.go index c595a790b..927bf2c09 100644 --- a/src/core/api/chart_repository.go +++ b/src/core/api/chart_repository.go @@ -15,6 +15,7 @@ import ( "github.com/goharbor/harbor/src/core/label" "github.com/goharbor/harbor/src/chartserver" + "github.com/goharbor/harbor/src/common/rbac" hlog "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/config" ) @@ -84,10 +85,35 @@ func (cra *ChartRepositoryAPI) Prepare() { cra.labelManager = &label.BaseManager{} } +func (cra *ChartRepositoryAPI) requireAccess(action rbac.Action, subresource ...rbac.Resource) bool { + if len(subresource) == 0 { + subresource = append(subresource, rbac.ResourceHelmChart) + } + resource := rbac.NewProjectNamespace(cra.namespace).Resource(subresource...) + + if !cra.SecurityCtx.Can(action, resource) { + if !cra.SecurityCtx.IsAuthenticated() { + cra.SendUnAuthorizedError(errors.New("Unauthorized")) + } else { + cra.HandleForbidden(cra.SecurityCtx.GetUsername()) + } + + return false + } + + return true +} + // GetHealthStatus handles GET /api/chartrepo/health func (cra *ChartRepositoryAPI) GetHealthStatus() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelSystem) { + if !cra.SecurityCtx.IsAuthenticated() { + cra.SendUnAuthorizedError(errors.New("Unauthorized")) + return + } + + if !cra.SecurityCtx.IsSysAdmin() { + cra.HandleForbidden(cra.SecurityCtx.GetUsername()) return } @@ -98,7 +124,7 @@ func (cra *ChartRepositoryAPI) GetHealthStatus() { // GetIndexByRepo handles GET /:repo/index.yaml func (cra *ChartRepositoryAPI) GetIndexByRepo() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelRead) { + if !cra.requireAccess(rbac.ActionRead) { return } @@ -109,7 +135,13 @@ func (cra *ChartRepositoryAPI) GetIndexByRepo() { // GetIndex handles GET /index.yaml func (cra *ChartRepositoryAPI) GetIndex() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelSystem) { + if !cra.SecurityCtx.IsAuthenticated() { + cra.SendUnAuthorizedError(errors.New("Unauthorized")) + return + } + + if !cra.SecurityCtx.IsSysAdmin() { + cra.HandleForbidden(cra.SecurityCtx.GetUsername()) return } @@ -136,7 +168,7 @@ func (cra *ChartRepositoryAPI) GetIndex() { // DownloadChart handles GET /:repo/charts/:filename func (cra *ChartRepositoryAPI) DownloadChart() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelRead) { + if !cra.requireAccess(rbac.ActionRead) { return } @@ -147,7 +179,7 @@ func (cra *ChartRepositoryAPI) DownloadChart() { // ListCharts handles GET /api/:repo/charts func (cra *ChartRepositoryAPI) ListCharts() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelRead) { + if !cra.requireAccess(rbac.ActionList) { return } @@ -163,7 +195,7 @@ func (cra *ChartRepositoryAPI) ListCharts() { // ListChartVersions GET /api/:repo/charts/:name func (cra *ChartRepositoryAPI) ListChartVersions() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelRead) { + if !cra.requireAccess(rbac.ActionList, rbac.ResourceHelmChartVersion) { return } @@ -191,7 +223,7 @@ func (cra *ChartRepositoryAPI) ListChartVersions() { // GetChartVersion handles GET /api/:repo/charts/:name/:version func (cra *ChartRepositoryAPI) GetChartVersion() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelRead) { + if !cra.requireAccess(rbac.ActionRead, rbac.ResourceHelmChartVersion) { return } @@ -219,7 +251,7 @@ func (cra *ChartRepositoryAPI) GetChartVersion() { // DeleteChartVersion handles DELETE /api/:repo/charts/:name/:version func (cra *ChartRepositoryAPI) DeleteChartVersion() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelAll) { + if !cra.requireAccess(rbac.ActionDelete, rbac.ResourceHelmChartVersion) { return } @@ -244,7 +276,7 @@ func (cra *ChartRepositoryAPI) UploadChartVersion() { hlog.Debugf("Header of request of uploading chart: %#v, content-len=%d", cra.Ctx.Request.Header, cra.Ctx.Request.ContentLength) // Check access - if !cra.requireAccess(cra.namespace, accessLevelWrite) { + if !cra.requireAccess(rbac.ActionCreate, rbac.ResourceHelmChartVersion) { return } @@ -272,7 +304,7 @@ func (cra *ChartRepositoryAPI) UploadChartVersion() { // UploadChartProvFile handles POST /api/:repo/prov func (cra *ChartRepositoryAPI) UploadChartProvFile() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelWrite) { + if !cra.requireAccess(rbac.ActionCreate) { return } @@ -297,7 +329,7 @@ func (cra *ChartRepositoryAPI) UploadChartProvFile() { // DeleteChart deletes all the chart versions of the specified chart. func (cra *ChartRepositoryAPI) DeleteChart() { // Check access - if !cra.requireAccess(cra.namespace, accessLevelWrite) { + if !cra.requireAccess(rbac.ActionDelete) { return } @@ -365,62 +397,6 @@ func (cra *ChartRepositoryAPI) requireNamespace(namespace string) bool { return true } -// Check if the related access match the expected requirement -// If with right access, return true -// If without right access, return false -func (cra *ChartRepositoryAPI) requireAccess(namespace string, accessLevel uint) bool { - if accessLevel == accessLevelPublic { - return true // do nothing - } - - theLevel := accessLevel - // If repo is empty, system admin role must be required - if len(namespace) == 0 { - theLevel = accessLevelSystem - } - - var err error - - switch theLevel { - // Should be system admin role - case accessLevelSystem: - if !cra.SecurityCtx.IsSysAdmin() { - err = errors.New("permission denied: system admin role is required") - } - case accessLevelAll: - if !cra.SecurityCtx.HasAllPerm(namespace) { - err = errors.New("permission denied: project admin or higher role is required") - } - case accessLevelWrite: - if !cra.SecurityCtx.HasWritePerm(namespace) { - err = errors.New("permission denied: developer or higher role is required") - } - case accessLevelRead: - if !cra.SecurityCtx.HasReadPerm(namespace) { - err = errors.New("permission denied: guest or higher role is required") - } - default: - // access rejected for invalid scope - cra.SendForbiddenError(errors.New("unrecognized access scope")) - return false - } - - // Access is not granted, check if user has authenticated - if err != nil { - // Unauthenticated, return 401 - if !cra.SecurityCtx.IsAuthenticated() { - cra.SendUnAuthorizedError(errors.New("Unauthorized")) - return false - } - - // Authenticated, return 403 - cra.SendForbiddenError(err) - return false - } - - return true -} - // formFile is used to represent the uploaded files in the form type formFile struct { // form field key contains the form file diff --git a/src/core/api/chart_repository_test.go b/src/core/api/chart_repository_test.go index 05a3c138f..d095ca71f 100644 --- a/src/core/api/chart_repository_test.go +++ b/src/core/api/chart_repository_test.go @@ -17,29 +17,6 @@ var ( crMockServer *httptest.Server ) -// Test access checking -func TestRequireAccess(t *testing.T) { - chartAPI := &ChartRepositoryAPI{} - chartAPI.SecurityCtx = &mockSecurityContext{} - - ns := "library" - if !chartAPI.requireAccess(ns, accessLevelPublic) { - t.Fatal("expect true result (public access level is granted) but got false") - } - if !chartAPI.requireAccess(ns, accessLevelAll) { - t.Fatal("expect true result (admin has all perm) but got false") - } - if !chartAPI.requireAccess(ns, accessLevelRead) { - t.Fatal("expect true result (admin has read perm) but got false") - } - if !chartAPI.requireAccess(ns, accessLevelWrite) { - t.Fatal("expect true result (admin has write perm) but got false") - } - if !chartAPI.requireAccess(ns, accessLevelSystem) { - t.Fatal("expect true result (admin has system perm) but got false") - } -} - func TestIsMultipartFormData(t *testing.T) { req, err := createRequest(http.MethodPost, "/api/chartrepo/charts") if err != nil { @@ -205,7 +182,7 @@ func TestDeleteChart(t *testing.T) { request: &testingRequest{ url: "/api/chartrepo/library/charts/harbor", method: http.MethodDelete, - credential: projDeveloper, + credential: projAdmin, }, code: http.StatusOK, }) @@ -310,21 +287,6 @@ func (msc *mockSecurityContext) IsSolutionUser() bool { return false } -// HasReadPerm returns whether the user has read permission to the project -func (msc *mockSecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - return msc.Can(rbac.ActionPull, rbac.NewProjectNamespace(projectIDOrName, false).Resource(rbac.ResourceRepository)) -} - -// HasWritePerm returns whether the user has write permission to the project -func (msc *mockSecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - return msc.Can(rbac.ActionPush, rbac.NewProjectNamespace(projectIDOrName, false).Resource(rbac.ResourceRepository)) -} - -// HasAllPerm returns whether the user has all permissions to the project -func (msc *mockSecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - return msc.HasReadPerm(projectIDOrName) && msc.HasWritePerm(projectIDOrName) -} - // Can returns whether the user can do action on resource func (msc *mockSecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { namespace, err := resource.GetNamespace() diff --git a/src/core/api/label.go b/src/core/api/label.go index 1bf0b18c2..702e3cc37 100644 --- a/src/core/api/label.go +++ b/src/core/api/label.go @@ -22,6 +22,7 @@ import ( "github.com/goharbor/harbor/src/common" "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/replication" "github.com/goharbor/harbor/src/replication/core" rep_models "github.com/goharbor/harbor/src/replication/models" @@ -65,15 +66,36 @@ func (l *LabelAPI) Prepare() { return } - if label.Scope == common.LabelScopeGlobal && !l.SecurityCtx.IsSysAdmin() || - label.Scope == common.LabelScopeProject && !l.SecurityCtx.HasAllPerm(label.ProjectID) { - l.HandleForbidden(l.SecurityCtx.GetUsername()) - return - } l.label = label } } +func (l *LabelAPI) requireAccess(label *models.Label, action rbac.Action, subresources ...rbac.Resource) bool { + var hasPermission bool + + switch label.Scope { + case common.LabelScopeGlobal: + hasPermission = l.SecurityCtx.IsSysAdmin() + case common.LabelScopeProject: + if len(subresources) == 0 { + subresources = append(subresources, rbac.ResourceLabel) + } + resource := rbac.NewProjectNamespace(label.ProjectID).Resource(subresources...) + hasPermission = l.SecurityCtx.Can(action, resource) + } + + if !hasPermission { + if !l.SecurityCtx.IsAuthenticated() { + l.HandleUnauthorized() + } else { + l.HandleForbidden(l.SecurityCtx.GetUsername()) + } + return false + } + + return true +} + // Post creates a label func (l *LabelAPI) Post() { label := &models.Label{} @@ -82,10 +104,6 @@ func (l *LabelAPI) Post() { switch label.Scope { case common.LabelScopeGlobal: - if !l.SecurityCtx.IsSysAdmin() { - l.HandleForbidden(l.SecurityCtx.GetUsername()) - return - } label.ProjectID = 0 case common.LabelScopeProject: exist, err := l.ProjectMgr.Exists(label.ProjectID) @@ -98,10 +116,10 @@ func (l *LabelAPI) Post() { l.HandleNotFound(fmt.Sprintf("project %d not found", label.ProjectID)) return } - if !l.SecurityCtx.HasAllPerm(label.ProjectID) { - l.HandleForbidden(l.SecurityCtx.GetUsername()) - return - } + } + + if !l.requireAccess(label, rbac.ActionCreate) { + return } labels, err := dao.ListLabels(&models.LabelQuery{ @@ -147,15 +165,8 @@ func (l *LabelAPI) Get() { return } - if label.Scope == common.LabelScopeProject { - if !l.SecurityCtx.HasReadPerm(label.ProjectID) { - if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() - return - } - l.HandleForbidden(l.SecurityCtx.GetUsername()) - return - } + if !l.requireAccess(label, rbac.ActionRead) { + return } l.Data["json"] = label @@ -189,7 +200,8 @@ func (l *LabelAPI) List() { return } - if !l.SecurityCtx.HasReadPerm(projectID) { + resource := rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceLabel) + if !l.SecurityCtx.Can(rbac.ActionList, resource) { if !l.SecurityCtx.IsAuthenticated() { l.HandleUnauthorized() return @@ -221,6 +233,10 @@ func (l *LabelAPI) List() { // Put updates the label func (l *LabelAPI) Put() { + if !l.requireAccess(l.label, rbac.ActionUpdate) { + return + } + label := &models.Label{} l.DecodeJSONReq(label) @@ -259,6 +275,10 @@ func (l *LabelAPI) Put() { // Delete the label func (l *LabelAPI) Delete() { + if !l.requireAccess(l.label, rbac.ActionDelete) { + return + } + id := l.label.ID if err := dao.DeleteResourceLabelByLabel(id); err != nil { l.HandleInternalServerError(fmt.Sprintf("failed to delete resource label mappings of label %d: %v", id, err)) @@ -272,11 +292,6 @@ func (l *LabelAPI) Delete() { // ListResources lists the resources that the label is referenced by func (l *LabelAPI) ListResources() { - if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() - return - } - id, err := l.GetInt64FromPath(":id") if err != nil || id <= 0 { l.HandleBadRequest("invalid label ID") @@ -294,9 +309,7 @@ func (l *LabelAPI) ListResources() { return } - if label.Scope == common.LabelScopeGlobal && !l.SecurityCtx.IsSysAdmin() || - label.Scope == common.LabelScopeProject && !l.SecurityCtx.HasAllPerm(label.ProjectID) { - l.HandleForbidden(l.SecurityCtx.GetUsername()) + if !l.requireAccess(label, rbac.ActionList, rbac.ResourceLabelResource) { return } diff --git a/src/core/api/label_resource.go b/src/core/api/label_resource.go index 4fc96637b..807b11029 100644 --- a/src/core/api/label_resource.go +++ b/src/core/api/label_resource.go @@ -22,23 +22,6 @@ func (lra *LabelResourceAPI) Prepare() { lra.labelManager = &label.BaseManager{} } -func (lra *LabelResourceAPI) checkPermissions(project string) bool { - if lra.Ctx.Request.Method == http.MethodPost || - lra.Ctx.Request.Method == http.MethodDelete { - if lra.SecurityCtx.HasWritePerm(project) { - return true - } - } - - if lra.Ctx.Request.Method == http.MethodGet { - if lra.SecurityCtx.HasReadPerm(project) { - return true - } - } - - return false -} - func (lra *LabelResourceAPI) getLabelsOfResource(rType string, rIDOrName interface{}) { labels, err := lra.labelManager.GetLabelsOfResource(rType, rIDOrName) if err != nil { diff --git a/src/core/api/metadata.go b/src/core/api/metadata.go index 090b7ca5c..146d6de09 100644 --- a/src/core/api/metadata.go +++ b/src/core/api/metadata.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/promgr/metamgr" ) @@ -72,24 +73,6 @@ func (m *MetadataAPI) Prepare() { m.project = project - switch m.Ctx.Request.Method { - case http.MethodGet, http.MethodPost, http.MethodPut, http.MethodDelete: - if !(m.Ctx.Request.Method == http.MethodGet && project.IsPublic()) { - if !m.SecurityCtx.IsAuthenticated() { - m.HandleUnauthorized() - return - } - if !m.SecurityCtx.HasReadPerm(project.ProjectID) { - m.HandleForbidden(m.SecurityCtx.GetUsername()) - return - } - } - default: - log.Debugf("%s method not allowed", m.Ctx.Request.Method) - m.RenderError(http.StatusMethodNotAllowed, "") - return - } - name := m.GetStringFromPath(":name") if len(name) > 0 { m.name = name @@ -105,8 +88,27 @@ func (m *MetadataAPI) Prepare() { } } +func (m *MetadataAPI) requireAccess(action rbac.Action) bool { + resource := rbac.NewProjectNamespace(m.project.ProjectID).Resource(rbac.ResourceMetadata) + + if !m.SecurityCtx.Can(action, resource) { + if !m.SecurityCtx.IsAuthenticated() { + m.HandleUnauthorized() + } else { + m.HandleForbidden(m.SecurityCtx.GetUsername()) + } + return false + } + + return true +} + // Get ... func (m *MetadataAPI) Get() { + if !m.requireAccess(rbac.ActionRead) { + return + } + var metas map[string]string var err error if len(m.name) > 0 { @@ -125,6 +127,10 @@ func (m *MetadataAPI) Get() { // Post ... func (m *MetadataAPI) Post() { + if !m.requireAccess(rbac.ActionCreate) { + return + } + var metas map[string]string m.DecodeJSONReq(&metas) @@ -161,6 +167,10 @@ func (m *MetadataAPI) Post() { // Put ... func (m *MetadataAPI) Put() { + if !m.requireAccess(rbac.ActionUpdate) { + return + } + var metas map[string]string m.DecodeJSONReq(&metas) @@ -188,6 +198,10 @@ func (m *MetadataAPI) Put() { // Delete ... func (m *MetadataAPI) Delete() { + if !m.requireAccess(rbac.ActionDelete) { + return + } + if err := m.metaMgr.Delete(m.project.ProjectID, m.name); err != nil { m.HandleInternalServerError(fmt.Sprintf("failed to delete metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) return diff --git a/src/core/api/project.go b/src/core/api/project.go index ac9ce2ce0..7771b7d77 100644 --- a/src/core/api/project.go +++ b/src/core/api/project.go @@ -22,6 +22,7 @@ import ( "github.com/goharbor/harbor/src/common" "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" errutil "github.com/goharbor/harbor/src/common/utils/error" "github.com/goharbor/harbor/src/common/utils/log" @@ -77,6 +78,25 @@ func (p *ProjectAPI) Prepare() { } } +func (p *ProjectAPI) requireAccess(action rbac.Action, subresource ...rbac.Resource) bool { + if len(subresource) == 0 { + subresource = append(subresource, rbac.ResourceSelf) + } + resource := rbac.NewProjectNamespace(p.project.ProjectID).Resource(subresource...) + + if !p.SecurityCtx.Can(action, resource) { + if !p.SecurityCtx.IsAuthenticated() { + p.HandleUnauthorized() + } else { + p.HandleForbidden(p.SecurityCtx.GetUsername()) + } + + return false + } + + return true +} + // Post ... func (p *ProjectAPI) Post() { if !p.SecurityCtx.IsAuthenticated() { @@ -187,16 +207,8 @@ func (p *ProjectAPI) Head() { // Get ... func (p *ProjectAPI) Get() { - if !p.project.IsPublic() { - if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() - return - } - - if !p.SecurityCtx.HasReadPerm(p.project.ProjectID) { - p.HandleForbidden(p.SecurityCtx.GetUsername()) - return - } + if !p.requireAccess(rbac.ActionRead) { + return } p.populateProperties(p.project) @@ -207,13 +219,7 @@ func (p *ProjectAPI) Get() { // Delete ... func (p *ProjectAPI) Delete() { - if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() - return - } - - if !p.SecurityCtx.HasAllPerm(p.project.ProjectID) { - p.HandleForbidden(p.SecurityCtx.GetUsername()) + if !p.requireAccess(rbac.ActionDelete) { return } @@ -248,13 +254,7 @@ func (p *ProjectAPI) Delete() { // Deletable ... func (p *ProjectAPI) Deletable() { - if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() - return - } - - if !p.SecurityCtx.HasAllPerm(p.project.ProjectID) { - p.HandleForbidden(p.SecurityCtx.GetUsername()) + if !p.requireAccess(rbac.ActionDelete) { return } @@ -433,13 +433,7 @@ func (p *ProjectAPI) populateProperties(project *models.Project) { // Put ... func (p *ProjectAPI) Put() { - if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() - return - } - - if !p.SecurityCtx.HasAllPerm(p.project.ProjectID) { - p.HandleForbidden(p.SecurityCtx.GetUsername()) + if !p.requireAccess(rbac.ActionUpdate) { return } @@ -458,13 +452,7 @@ func (p *ProjectAPI) Put() { // Logs ... func (p *ProjectAPI) Logs() { - if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() - return - } - - if !p.SecurityCtx.HasReadPerm(p.project.ProjectID) { - p.HandleForbidden(p.SecurityCtx.GetUsername()) + if !p.requireAccess(rbac.ActionList, rbac.ResourceLog) { return } diff --git a/src/core/api/projectmember.go b/src/core/api/projectmember.go index d94ae1e0f..f4e52d672 100644 --- a/src/core/api/projectmember.go +++ b/src/core/api/projectmember.go @@ -24,6 +24,7 @@ import ( "github.com/goharbor/harbor/src/common" "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/log" "github.com/goharbor/harbor/src/core/auth" ) @@ -73,12 +74,6 @@ func (pma *ProjectMemberAPI) Prepare() { } pma.project = project - if !(pma.Ctx.Input.IsGet() && pma.SecurityCtx.HasReadPerm(pid) || - pma.SecurityCtx.HasAllPerm(pid)) { - pma.HandleForbidden(pma.SecurityCtx.GetUsername()) - return - } - pmid, err := pma.GetInt64FromPath(":pmid") if err != nil { log.Warningf("Failed to get pmid from path, error %v", err) @@ -90,6 +85,22 @@ func (pma *ProjectMemberAPI) Prepare() { pma.id = int(pmid) } +func (pma *ProjectMemberAPI) requireAccess(action rbac.Action) bool { + resource := rbac.NewProjectNamespace(pma.project.ProjectID).Resource(rbac.ResourceMember) + + if !pma.SecurityCtx.Can(action, resource) { + if !pma.SecurityCtx.IsAuthenticated() { + pma.HandleUnauthorized() + } else { + pma.HandleForbidden(pma.SecurityCtx.GetUsername()) + } + + return false + } + + return true +} + // Get ... func (pma *ProjectMemberAPI) Get() { projectID := pma.project.ProjectID @@ -97,6 +108,9 @@ func (pma *ProjectMemberAPI) Get() { queryMember.ProjectID = projectID pma.Data["json"] = make([]models.Member, 0) if pma.id == 0 { + if !pma.requireAccess(rbac.ActionList) { + return + } entityname := pma.GetString("entityname") memberList, err := project.SearchMemberByName(projectID, entityname) if err != nil { @@ -119,6 +133,10 @@ func (pma *ProjectMemberAPI) Get() { pma.HandleNotFound(fmt.Sprintf("The project member does not exit, pmid:%v", pma.id)) return } + + if !pma.requireAccess(rbac.ActionRead) { + return + } pma.Data["json"] = memberList[0] } pma.ServeJSON() @@ -126,6 +144,9 @@ func (pma *ProjectMemberAPI) Get() { // Post ... Add a project member func (pma *ProjectMemberAPI) Post() { + if !pma.requireAccess(rbac.ActionCreate) { + return + } projectID := pma.project.ProjectID var request models.MemberReq pma.DecodeJSONReq(&request) @@ -156,6 +177,9 @@ func (pma *ProjectMemberAPI) Post() { // Put ... Update an exist project member func (pma *ProjectMemberAPI) Put() { + if !pma.requireAccess(rbac.ActionUpdate) { + return + } pid := pma.project.ProjectID pmID := pma.id var req models.Member @@ -173,6 +197,9 @@ func (pma *ProjectMemberAPI) Put() { // Delete ... func (pma *ProjectMemberAPI) Delete() { + if !pma.requireAccess(rbac.ActionDelete) { + return + } pmid := pma.id err := project.DeleteProjectMemberByID(pmid) if err != nil { diff --git a/src/core/api/replication_job.go b/src/core/api/replication_job.go index c871f6f14..32ee1a7b5 100644 --- a/src/core/api/replication_job.go +++ b/src/core/api/replication_job.go @@ -24,6 +24,7 @@ import ( common_http "github.com/goharbor/harbor/src/common/http" common_job "github.com/goharbor/harbor/src/common/job" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" api_models "github.com/goharbor/harbor/src/core/api/models" "github.com/goharbor/harbor/src/core/utils" @@ -80,7 +81,8 @@ func (ra *RepJobAPI) List() { return } - if !ra.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) { + resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplicationJob) + if !ra.SecurityCtx.Can(rbac.ActionList, resource) { ra.HandleForbidden(ra.SecurityCtx.GetUsername()) return } @@ -190,7 +192,8 @@ func (ra *RepJobAPI) GetLog() { return } - if !ra.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) { + resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplicationJob) + if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { ra.HandleForbidden(ra.SecurityCtx.GetUsername()) return } diff --git a/src/core/api/replication_policy.go b/src/core/api/replication_policy.go index 642cf35d7..ac45fbdad 100644 --- a/src/core/api/replication_policy.go +++ b/src/core/api/replication_policy.go @@ -22,6 +22,7 @@ import ( "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/log" api_models "github.com/goharbor/harbor/src/core/api/models" "github.com/goharbor/harbor/src/core/promgr" @@ -63,7 +64,8 @@ func (pa *RepPolicyAPI) Get() { return } - if !pa.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) { + resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplication) + if !pa.SecurityCtx.Can(rbac.ActionRead, resource) { pa.HandleForbidden(pa.SecurityCtx.GetUsername()) return } @@ -105,7 +107,8 @@ func (pa *RepPolicyAPI) List() { if result != nil { total = result.Total for _, policy := range result.Policies { - if !pa.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) { + resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplication) + if !pa.SecurityCtx.Can(rbac.ActionRead, resource) { continue } ply, err := convertFromRepPolicy(pa.ProjectMgr, *policy) diff --git a/src/core/api/repository.go b/src/core/api/repository.go index 463365335..a96fdc898 100644 --- a/src/core/api/repository.go +++ b/src/core/api/repository.go @@ -30,6 +30,7 @@ import ( "github.com/goharbor/harbor/src/common/dao" commonhttp "github.com/goharbor/harbor/src/common/http" "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/common/utils/clair" "github.com/goharbor/harbor/src/common/utils/log" @@ -131,7 +132,8 @@ func (ra *RepositoryAPI) Get() { return } - if !ra.SecurityCtx.HasReadPerm(projectID) { + resource := rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return @@ -247,7 +249,8 @@ func (ra *RepositoryAPI) Delete() { return } - if !ra.SecurityCtx.HasAllPerm(projectName) { + resource := rbac.NewProjectNamespace(project.ProjectID).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionDelete, resource) { ra.HandleForbidden(ra.SecurityCtx.GetUsername()) return } @@ -393,7 +396,8 @@ func (ra *RepositoryAPI) GetTag() { return } project, _ := utils.ParseRepository(repository) - if !ra.SecurityCtx.HasReadPerm(project) { + resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepositoryTag) + if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return @@ -488,14 +492,16 @@ func (ra *RepositoryAPI) Retag() { } // Check whether user has read permission to source project - if !ra.SecurityCtx.HasReadPerm(srcImage.Project) { + srcResource := rbac.NewProjectNamespace(srcImage.Project).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionPull, srcResource) { log.Errorf("user has no read permission to project '%s'", srcImage.Project) ra.HandleForbidden(fmt.Sprintf("%s has no read permission to project %s", ra.SecurityCtx.GetUsername(), srcImage.Project)) return } // Check whether user has write permission to target project - if !ra.SecurityCtx.HasWritePerm(project) { + destResource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionPush, destResource) { log.Errorf("user has no write permission to project '%s'", project) ra.HandleForbidden(fmt.Sprintf("%s has no write permission to project %s", ra.SecurityCtx.GetUsername(), project)) return @@ -533,7 +539,8 @@ func (ra *RepositoryAPI) GetTags() { return } - if !ra.SecurityCtx.HasReadPerm(projectName) { + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTag) + if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return @@ -741,7 +748,8 @@ func (ra *RepositoryAPI) GetManifests() { return } - if !ra.SecurityCtx.HasReadPerm(projectName) { + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagManifest) + if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return @@ -872,7 +880,8 @@ func (ra *RepositoryAPI) Put() { } project, _ := utils.ParseRepository(name) - if !ra.SecurityCtx.HasWritePerm(project) { + resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionUpdate, resource) { ra.HandleForbidden(ra.SecurityCtx.GetUsername()) return } @@ -906,7 +915,8 @@ func (ra *RepositoryAPI) GetSignatures() { return } - if !ra.SecurityCtx.HasReadPerm(projectName) { + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepository) + if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return @@ -949,7 +959,9 @@ func (ra *RepositoryAPI) ScanImage() { ra.HandleUnauthorized() return } - if !ra.SecurityCtx.HasAllPerm(projectName) { + + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagScanJob) + if !ra.SecurityCtx.Can(rbac.ActionCreate, resource) { ra.HandleForbidden(ra.SecurityCtx.GetUsername()) return } @@ -980,7 +992,9 @@ func (ra *RepositoryAPI) VulnerabilityDetails() { return } project, _ := utils.ParseRepository(repository) - if !ra.SecurityCtx.HasReadPerm(project) { + + resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepositoryTagVulnerability) + if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { ra.HandleUnauthorized() return diff --git a/src/core/api/repository_label.go b/src/core/api/repository_label.go index 547fe5c82..5b658e424 100644 --- a/src/core/api/repository_label.go +++ b/src/core/api/repository_label.go @@ -22,7 +22,7 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/common/utils" + "github.com/goharbor/harbor/src/common/rbac" coreutils "github.com/goharbor/harbor/src/core/utils" ) @@ -45,12 +45,6 @@ func (r *RepositoryLabelAPI) Prepare() { } repository := r.GetString(":splat") - project, _ := utils.ParseRepository(repository) - if !r.checkPermissions(project) { - r.SendForbiddenError(errors.New(r.SecurityCtx.GetUsername())) - return - } - repo, err := dao.GetRepositoryByName(repository) if err != nil { r.SendInternalServerError(fmt.Errorf("failed to get repository %s: %v", repository, err)) @@ -77,25 +71,6 @@ func (r *RepositoryLabelAPI) Prepare() { r.tag = tag } - if r.Ctx.Request.Method == http.MethodPost { - p, err := r.ProjectMgr.Get(project) - if err != nil { - r.SendInternalServerError(err) - return - } - - l := &models.Label{} - r.DecodeJSONReq(l) - - label, ok := r.validate(l.ID, p.ProjectID) - if !ok { - return - } - r.label = label - - return - } - if r.Ctx.Request.Method == http.MethodDelete { labelID, err := r.GetInt64FromPath(":id") if err != nil { @@ -112,13 +87,59 @@ func (r *RepositoryLabelAPI) Prepare() { } } +func (r *RepositoryLabelAPI) requireAccess(action rbac.Action, subresource ...rbac.Resource) bool { + if len(subresource) == 0 { + subresource = append(subresource, rbac.ResourceRepositoryLabel) + } + resource := rbac.NewProjectNamespace(r.repository.ProjectID).Resource(rbac.ResourceRepositoryLabel) + + if !r.SecurityCtx.Can(action, resource) { + if !r.SecurityCtx.IsAuthenticated() { + r.SendUnAuthorizedError(errors.New("UnAuthorized")) + } else { + r.SendForbiddenError(errors.New(r.SecurityCtx.GetUsername())) + } + + return false + } + + return true +} + +func (r *RepositoryLabelAPI) isValidLabelReq() bool { + p, err := r.ProjectMgr.Get(r.repository.ProjectID) + if err != nil { + r.SendInternalServerError(err) + return false + } + + l := &models.Label{} + r.DecodeJSONReq(l) + + label, ok := r.validate(l.ID, p.ProjectID) + if !ok { + return false + } + r.label = label + + return true +} + // GetOfImage returns labels of an image func (r *RepositoryLabelAPI) GetOfImage() { + if !r.requireAccess(rbac.ActionList, rbac.ResourceRepositoryTagLabel) { + return + } + r.getLabelsOfResource(common.ResourceTypeImage, fmt.Sprintf("%s:%s", r.repository.Name, r.tag)) } // AddToImage adds the label to an image func (r *RepositoryLabelAPI) AddToImage() { + if !r.requireAccess(rbac.ActionCreate, rbac.ResourceRepositoryTagLabel) || !r.isValidLabelReq() { + return + } + rl := &models.ResourceLabel{ LabelID: r.label.ID, ResourceType: common.ResourceTypeImage, @@ -129,17 +150,29 @@ func (r *RepositoryLabelAPI) AddToImage() { // RemoveFromImage removes the label from an image func (r *RepositoryLabelAPI) RemoveFromImage() { + if !r.requireAccess(rbac.ActionDelete, rbac.ResourceRepositoryTagLabel) { + return + } + r.removeLabelFromResource(common.ResourceTypeImage, fmt.Sprintf("%s:%s", r.repository.Name, r.tag), r.label.ID) } // GetOfRepository returns labels of a repository func (r *RepositoryLabelAPI) GetOfRepository() { + if !r.requireAccess(rbac.ActionList) { + return + } + r.getLabelsOfResource(common.ResourceTypeRepository, r.repository.RepositoryID) } // AddToRepository adds the label to a repository func (r *RepositoryLabelAPI) AddToRepository() { + if !r.requireAccess(rbac.ActionCreate) || !r.isValidLabelReq() { + return + } + rl := &models.ResourceLabel{ LabelID: r.label.ID, ResourceType: common.ResourceTypeRepository, @@ -150,6 +183,10 @@ func (r *RepositoryLabelAPI) AddToRepository() { // RemoveFromRepository removes the label from a repository func (r *RepositoryLabelAPI) RemoveFromRepository() { + if !r.requireAccess(rbac.ActionDelete) { + return + } + r.removeLabelFromResource(common.ResourceTypeRepository, r.repository.RepositoryID, r.label.ID) } diff --git a/src/core/api/robot.go b/src/core/api/robot.go index 920ce312c..03850f90f 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -16,12 +16,14 @@ package api import ( "fmt" + "net/http" + "strconv" + "github.com/goharbor/harbor/src/common" "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/token" - "net/http" - "strconv" ) // RobotAPI ... @@ -83,17 +85,24 @@ func (r *RobotAPI) Prepare() { r.robot = robot } +} - if !(r.Ctx.Input.IsGet() && r.SecurityCtx.HasReadPerm(pid) || - r.SecurityCtx.HasAllPerm(pid)) { +func (r *RobotAPI) requireAccess(action rbac.Action) bool { + resource := rbac.NewProjectNamespace(r.project.ProjectID).Resource(rbac.ResourceRobot) + if !r.SecurityCtx.Can(action, resource) { r.HandleForbidden(r.SecurityCtx.GetUsername()) - return + return false } + return true } // Post ... func (r *RobotAPI) Post() { + if !r.requireAccess(rbac.ActionCreate) { + return + } + var robotReq models.RobotReq r.DecodeJSONReq(&robotReq) createdName := common.RobotPrefix + robotReq.Name @@ -147,6 +156,10 @@ func (r *RobotAPI) Post() { // List list all the robots of a project func (r *RobotAPI) List() { + if !r.requireAccess(rbac.ActionList) { + return + } + query := models.RobotQuery{ ProjectID: r.project.ProjectID, } @@ -171,6 +184,10 @@ func (r *RobotAPI) List() { // Get get robot by id func (r *RobotAPI) Get() { + if !r.requireAccess(rbac.ActionRead) { + return + } + id, err := r.GetInt64FromPath(":id") if err != nil || id <= 0 { r.HandleBadRequest(fmt.Sprintf("invalid robot ID: %s", r.GetStringFromPath(":id"))) @@ -193,6 +210,10 @@ func (r *RobotAPI) Get() { // Put disable or enable a robot account func (r *RobotAPI) Put() { + if !r.requireAccess(rbac.ActionUpdate) { + return + } + var robotReq models.RobotReq r.DecodeJSONReqAndValidate(&robotReq) r.robot.Disabled = robotReq.Disabled @@ -206,6 +227,10 @@ func (r *RobotAPI) Put() { // Delete delete robot by id func (r *RobotAPI) Delete() { + if !r.requireAccess(rbac.ActionDelete) { + return + } + if err := dao.DeleteRobot(r.robot.ID); err != nil { r.HandleInternalServerError(fmt.Sprintf("failed to delete robot %d: %v", r.robot.ID, err)) return diff --git a/src/core/api/scan_job.go b/src/core/api/scan_job.go index 9f3110cc4..9489d57ed 100644 --- a/src/core/api/scan_job.go +++ b/src/core/api/scan_job.go @@ -17,6 +17,7 @@ package api import ( "github.com/goharbor/harbor/src/common/dao" common_http "github.com/goharbor/harbor/src/common/http" + "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/utils" @@ -54,7 +55,9 @@ func (sj *ScanJobAPI) Prepare() { sj.CustomAbort(http.StatusInternalServerError, "Failed to get Job data") } projectName := strings.SplitN(data.Repository, "/", 2)[0] - if !sj.SecurityCtx.HasReadPerm(projectName) { + + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagScanJob) + if !sj.SecurityCtx.Can(rbac.ActionRead, resource) { log.Errorf("User does not have read permission for project: %s", projectName) sj.HandleForbidden(sj.SecurityCtx.GetUsername()) } diff --git a/src/core/service/token/creator.go b/src/core/service/token/creator.go index c84798001..fbd229783 100644 --- a/src/core/service/token/creator.go +++ b/src/core/service/token/creator.go @@ -22,6 +22,7 @@ import ( "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/common/utils/log" "github.com/goharbor/harbor/src/core/config" @@ -158,24 +159,25 @@ func (rep repositoryFilter) filter(ctx security.Context, pm promgr.ProjectManage if err != nil { return err } - project := img.namespace + projectName := img.namespace permission := "" - exist, err := pm.Exists(project) + exist, err := pm.Exists(projectName) if err != nil { return err } if !exist { - log.Debugf("project %s does not exist, set empty permission", project) + log.Debugf("project %s does not exist, set empty permission", projectName) a.Actions = []string{} return nil } - if ctx.HasAllPerm(project) { + resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepository) + if ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource) { permission = "RWM" - } else if ctx.HasWritePerm(project) { + } else if ctx.Can(rbac.ActionPush, resource) { permission = "RW" - } else if ctx.HasReadPerm(project) { + } else if ctx.Can(rbac.ActionPull, resource) { permission = "R" } diff --git a/src/core/service/token/token_test.go b/src/core/service/token/token_test.go index 3869eb105..7337357fd 100644 --- a/src/core/service/token/token_test.go +++ b/src/core/service/token/token_test.go @@ -252,15 +252,6 @@ func (f *fakeSecurityContext) IsSysAdmin() bool { func (f *fakeSecurityContext) IsSolutionUser() bool { return false } -func (f *fakeSecurityContext) HasReadPerm(projectIDOrName interface{}) bool { - return false -} -func (f *fakeSecurityContext) HasWritePerm(projectIDOrName interface{}) bool { - return false -} -func (f *fakeSecurityContext) HasAllPerm(projectIDOrName interface{}) bool { - return false -} func (f *fakeSecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { return false }