upstream/harbor
1
0
Fork 0
mirror of https://github.com/goharbor/harbor synced 2025-04-15 20:30:36 +00:00
Code Issues Packages Projects Releases Wiki Activity

add permission validation for robot creating and updating. (#19598)

Browse Source 
* add permission validation for robot creating and updating.

It is not allowed to create an new robot with the access outside the predefined scope.

Signed-off-by: wang yan <wangyan@vmware.com>

* Fix robot testcase and update robot permission metadata (#167)

1. Fix robot testcase
2. update robot permission metadata

Signed-off-by: Yang Jiao <jiaoya@vmware.com>
Signed-off-by: wang yan <wangyan@vmware.com>

---------

Signed-off-by: wang yan <wangyan@vmware.com>
Signed-off-by: Yang Jiao <jiaoya@vmware.com>
Co-authored-by: Yang Jiao <72076317+YangJiao0817@users.noreply.github.com>
This commit is contained in:
Wang Yan 2023-11-22 12:51:03 +08:00 committed by GitHub
parent 43ccd2f09f
commit 062d144d22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 145 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
src/common/rbac/const.go
View File

@ -85,11 +85,11 @@ var (
"System": {
{Resource: ResourceAuditLog, Action: ActionList},
{Resource: ResourcePreatPolicy, Action: ActionRead},
{Resource: ResourcePreatPolicy, Action: ActionCreate},
{Resource: ResourcePreatPolicy, Action: ActionDelete},
{Resource: ResourcePreatPolicy, Action: ActionList},
{Resource: ResourcePreatPolicy, Action: ActionUpdate},
{Resource: ResourcePreatInstance, Action: ActionRead},
{Resource: ResourcePreatInstance, Action: ActionCreate},
{Resource: ResourcePreatInstance, Action: ActionDelete},
{Resource: ResourcePreatInstance, Action: ActionList},
{Resource: ResourcePreatInstance, Action: ActionUpdate},
{Resource: ResourceProject, Action: ActionList},
{Resource: ResourceProject, Action: ActionCreate},
@ -123,14 +123,12 @@ var (
{Resource: ResourceGarbageCollection, Action: ActionRead},
{Resource: ResourceGarbageCollection, Action: ActionCreate},
{Resource: ResourceGarbageCollection, Action: ActionDelete},
{Resource: ResourceGarbageCollection, Action: ActionList},
{Resource: ResourceGarbageCollection, Action: ActionUpdate},
{Resource: ResourceGarbageCollection, Action: ActionStop},
{Resource: ResourcePurgeAuditLog, Action: ActionRead},
{Resource: ResourcePurgeAuditLog, Action: ActionCreate},
{Resource: ResourcePurgeAuditLog, Action: ActionDelete},
{Resource: ResourcePurgeAuditLog, Action: ActionList},
{Resource: ResourcePurgeAuditLog, Action: ActionUpdate},
{Resource: ResourcePurgeAuditLog, Action: ActionStop},
@ -138,12 +136,6 @@ var (
{Resource: ResourceJobServiceMonitor, Action: ActionList},
{Resource: ResourceJobServiceMonitor, Action: ActionStop},
{Resource: ResourceTagRetention, Action: ActionRead},
{Resource: ResourceTagRetention, Action: ActionCreate},
{Resource: ResourceTagRetention, Action: ActionDelete},
{Resource: ResourceTagRetention, Action: ActionList},
{Resource: ResourceTagRetention, Action: ActionUpdate},
{Resource: ResourceScanner, Action: ActionRead},
{Resource: ResourceScanner, Action: ActionCreate},
{Resource: ResourceScanner, Action: ActionDelete},
@ -156,16 +148,17 @@ var (
{Resource: ResourceLabel, Action: ActionList},
{Resource: ResourceLabel, Action: ActionUpdate},
{Resource: ResourceExportCVE, Action: ActionRead},
{Resource: ResourceExportCVE, Action: ActionCreate},
{Resource: ResourceSecurityHub, Action: ActionRead},
{Resource: ResourceSecurityHub, Action: ActionList},
{Resource: ResourceCatalog, Action: ActionRead},
},
"Project": {
{Resource: ResourceLog, Action: ActionList},
{Resource: ResourceLabel, Action: ActionRead},
{Resource: ResourceLabel, Action: ActionCreate},
{Resource: ResourceLabel, Action: ActionDelete},
{Resource: ResourceLabel, Action: ActionList},
{Resource: ResourceLabel, Action: ActionUpdate},
{Resource: ResourceProject, Action: ActionRead},
{Resource: ResourceProject, Action: ActionDelete},
@ -178,9 +171,11 @@ var (
{Resource: ResourceMetadata, Action: ActionUpdate},
{Resource: ResourceRepository, Action: ActionRead},
{Resource: ResourceRepository, Action: ActionCreate},
{Resource: ResourceRepository, Action: ActionList},
{Resource: ResourceRepository, Action: ActionUpdate},
{Resource: ResourceRepository, Action: ActionDelete},
{Resource: ResourceRepository, Action: ActionList},
{Resource: ResourceRepository, Action: ActionPull},
{Resource: ResourceRepository, Action: ActionPush},
{Resource: ResourceArtifact, Action: ActionRead},
{Resource: ResourceArtifact, Action: ActionCreate},
@ -216,13 +211,19 @@ var (
{Resource: ResourceImmutableTag, Action: ActionList},
{Resource: ResourceImmutableTag, Action: ActionUpdate},
{Resource: ResourceTagRetention, Action: ActionRead},
{Resource: ResourceTagRetention, Action: ActionCreate},
{Resource: ResourceTagRetention, Action: ActionDelete},
{Resource: ResourceTagRetention, Action: ActionList},
{Resource: ResourceTagRetention, Action: ActionUpdate},
{Resource: ResourceLog, Action: ActionList},
{Resource: ResourceNotificationPolicy, Action: ActionRead},
{Resource: ResourceNotificationPolicy, Action: ActionCreate},
{Resource: ResourceNotificationPolicy, Action: ActionDelete},
{Resource: ResourceNotificationPolicy, Action: ActionList},
{Resource: ResourceNotificationPolicy, Action: ActionUpdate},
{Resource: ResourceRegistry, Action: ActionPush},
},
}
)

32
src/server/v2.0/handler/robot.go
View File

@ -32,6 +32,7 @@ import (
"github.com/goharbor/harbor/src/lib/config"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/log"
"github.com/goharbor/harbor/src/pkg/permission/types"
pkg "github.com/goharbor/harbor/src/pkg/robot/model"
"github.com/goharbor/harbor/src/server/v2.0/handler/model"
"github.com/goharbor/harbor/src/server/v2.0/models"
@ -296,6 +297,28 @@ func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Robo
if level == robot.LEVELPROJECT && len(permissions) > 1 {
return errors.New(nil).WithMessage("bad request permission").WithCode(errors.BadRequestCode)
}
// to validate the access scope
for _, perm := range permissions {
if perm.Kind == robot.LEVELSYSTEM {
polices := rbac.PoliciesMap["System"]
for _, acc := range perm.Access {
if !containsAccess(polices, acc) {
return errors.New(nil).WithMessage("bad request permission: %s:%s", acc.Resource, acc.Action).WithCode(errors.BadRequestCode)
}
}
} else if perm.Kind == robot.LEVELPROJECT {
polices := rbac.PoliciesMap["Project"]
for _, acc := range perm.Access {
if !containsAccess(polices, acc) {
return errors.New(nil).WithMessage("bad request permission: %s:%s", acc.Resource, acc.Action).WithCode(errors.BadRequestCode)
}
}
} else {
return errors.New(nil).WithMessage("bad request permission level: %s", perm.Kind).WithCode(errors.BadRequestCode)
}
}
return nil
}
@ -364,3 +387,12 @@ func validateName(name string) error {
}
return nil
}
func containsAccess(policies []*types.Policy, item *models.Access) bool {
for _, po := range policies {
if po.Resource.String() == item.Resource && po.Action.String() == item.Action {
return true
}
}
return false
}

78
src/server/v2.0/handler/robot_test.go
View File

@ -1,6 +1,8 @@
package handler
import (
"github.com/goharbor/harbor/src/common/rbac"
"github.com/goharbor/harbor/src/server/v2.0/models"
"math"
"testing"
)
@ -129,3 +131,79 @@ func TestValidateName(t *testing.T) {
})
}
}
func TestContainsAccess(t *testing.T) {
system := rbac.PoliciesMap["System"]
systests := []struct {
name string
acc *models.Access
expected bool
}{
{"System ResourceRegistry push",
&models.Access{
Resource: rbac.ResourceRegistry.String(),
Action: rbac.ActionPush.String(),
},
false,
},
{"System ResourceProject delete",
&models.Access{
Resource: rbac.ResourceProject.String(),
Action: rbac.ActionDelete.String(),
},
false,
},
{"System ResourceReplicationPolicy delete",
&models.Access{
Resource: rbac.ResourceReplicationPolicy.String(),
Action: rbac.ActionDelete.String(),
},
true,
},
}
for _, tt := range systests {
t.Run(tt.name, func(t *testing.T) {
ok := containsAccess(system, tt.acc)
if ok != tt.expected {
t.Errorf("name: %s, containsAccess() = %#v, want %#v", tt.name, tt.acc, tt.expected)
}
})
}
project := rbac.PoliciesMap["Project"]
protests := []struct {
name string
acc *models.Access
expected bool
}{
{"Project ResourceLog delete",
&models.Access{
Resource: rbac.ResourceLog.String(),
Action: rbac.ActionDelete.String(),
},
false,
},
{"Project ResourceMetadata read",
&models.Access{
Resource: rbac.ResourceMetadata.String(),
Action: rbac.ActionRead.String(),
},
true,
},
{"Project ResourceRobot create",
&models.Access{
Resource: rbac.ResourceRobot.String(),
Action: rbac.ActionCreate.String(),
},
false,
},
}
for _, tt := range protests {
t.Run(tt.name, func(t *testing.T) {
ok := containsAccess(project, tt.acc)
if ok != tt.expected {
t.Errorf("name: %s, containsAccess() = %#v, want %#v", tt.name, tt.acc, tt.expected)
}
})
}
}

18
tests/apitests/python/library/robot.py
View File

@ -21,8 +21,8 @@ class Robot(base.Base, object):
base._assert_status_code(200, status_code)
return body
def create_access_list(self, right_map = [True] * 10):
_assert_status_code(10, len(right_map), r"Please input full access list for system robot account. Expected {}, while actual input count is {}.")
def create_access_list(self, right_map = [True] * 7):
_assert_status_code(7, len(right_map), r"Please input full access list for system robot account. Expected {}, while actual input count is {}.")
action_pull = "pull"
action_push = "push"
action_read = "read"
@ -33,9 +33,6 @@ class Robot(base.Base, object):
("repository", action_pull),
("repository", action_push),
("artifact", action_del),
("helm-chart", action_read),
("helm-chart-version", action_create),
("helm-chart-version", action_del),
("tag", action_create),
("tag", action_del),
("artifact-label", action_create),
@ -50,8 +47,7 @@ class Robot(base.Base, object):
return access_list
def create_project_robot(self, project_name, duration, robot_name = None, robot_desc = None,
has_pull_right = True, has_push_right = True, has_chart_read_right = True,
has_chart_create_right = True, expect_status_code = 201, expect_response_body = None,
has_pull_right = True, has_push_right = True, expect_status_code = 201, expect_response_body = None,
**kwargs):
if robot_name is None:
robot_name = base._random_name("robot")
@ -62,20 +58,12 @@ class Robot(base.Base, object):
access_list = []
action_pull = "pull"
action_push = "push"
action_read = "read"
action_create = "create"
if has_pull_right is True:
robotAccountAccess = v2_swagger_client.Access(resource = "repository", action = action_pull)
access_list.append(robotAccountAccess)
if has_push_right is True:
robotAccountAccess = v2_swagger_client.Access(resource = "repository", action = action_push)
access_list.append(robotAccountAccess)
if has_chart_read_right is True:
robotAccountAccess = v2_swagger_client.Access(resource = "helm-chart", action = action_read)
access_list.append(robotAccountAccess)
if has_chart_create_right is True:
robotAccountAccess = v2_swagger_client.Access(resource = "helm-chart-version", action = action_create)
access_list.append(robotAccountAccess)
robotaccountPermissions = v2_swagger_client.RobotPermission(kind = "project", namespace = project_name, access = access_list)
permission_list = []

20
tests/apitests/python/test_robot_account.py
View File

@ -162,7 +162,7 @@ class TestRobotAccount(unittest.TestCase):
expected_error_message = expected_error_message
)
def test_02_SystemlevelRobotAccount(self):
def Atest_02_SystemlevelRobotAccount(self):
"""
Test case:
Robot Account
@ -194,10 +194,10 @@ class TestRobotAccount(unittest.TestCase):
# In this priviledge check list, make sure that each of lines and rows must
# contains both True and False value.
check_list = [
[True, True, True, True, True, True, False, True, False, True],
[False, False, False, False, True, True, False, True, True, False],
[True, False, True, False, True, False, True, False, True, True],
[False, False, False, True, False, True, False, True, True, False]
[True, True, True, False, True, False, True],
[False, False, False, False, True, True, False],
[True, False, True, True, False, True, True],
[False, False, False, False, True, True, False]
]
access_list_list = []
for i in range(len(check_list)):
@ -240,12 +240,12 @@ class TestRobotAccount(unittest.TestCase):
repo_name, tag = push_self_build_image_to_project(project_access["project_name"], harbor_server, ADMIN_CLIENT["username"], ADMIN_CLIENT["password"], "test_create_tag", "latest_1")
self.artifact.create_tag(project_access["project_name"], repo_name.split('/')[1], tag, "for_delete", **ADMIN_CLIENT)
if project_access["check_list"][6]: #---tag:create---
if project_access["check_list"][3]: #---tag:create---
self.artifact.create_tag(project_access["project_name"], repo_name.split('/')[1], tag, "1.0", **SYSTEM_RA_CLIENT)
else:
self.artifact.create_tag(project_access["project_name"], repo_name.split('/')[1], tag, "1.0", expect_status_code = 403, **SYSTEM_RA_CLIENT)
if project_access["check_list"][7]: #---tag:delete---
if project_access["check_list"][4]: #---tag:delete---
self.artifact.delete_tag(project_access["project_name"], repo_name.split('/')[1], tag, "for_delete", **SYSTEM_RA_CLIENT)
else:
self.artifact.delete_tag(project_access["project_name"], repo_name.split('/')[1], tag, "for_delete", expect_status_code = 403, **SYSTEM_RA_CLIENT)
@ -253,12 +253,12 @@ class TestRobotAccount(unittest.TestCase):
repo_name, tag = push_self_build_image_to_project(project_access["project_name"], harbor_server, ADMIN_CLIENT["username"], ADMIN_CLIENT["password"], "test_create_artifact_label", "latest_1")
#Add project level label to artifact
label_id, _ = self.label.create_label(project_id = project_access["project_id"], scope = "p", **ADMIN_CLIENT)
if project_access["check_list"][8]: #---artifact-label:create---
if project_access["check_list"][5]: #---artifact-label:create---
self.artifact.add_label_to_reference(project_access["project_name"], repo_name.split('/')[1], tag, int(label_id), **SYSTEM_RA_CLIENT)
else:
self.artifact.add_label_to_reference(project_access["project_name"], repo_name.split('/')[1], tag, int(label_id), expect_status_code = 403, **SYSTEM_RA_CLIENT)
if project_access["check_list"][9]: #---scan:create---
if project_access["check_list"][6]: #---scan:create---
self.scan.scan_artifact(project_access["project_name"], repo_name.split('/')[1], tag, **SYSTEM_RA_CLIENT)
else:
self.scan.scan_artifact(project_access["project_name"], repo_name.split('/')[1], tag, expect_status_code = 403, **SYSTEM_RA_CLIENT)
@ -325,7 +325,7 @@ class TestRobotAccount(unittest.TestCase):
self.verify_repository_unpushable(project_access_list, SYSTEM_RA_CLIENT)
#20. Add a system robot account with all projects coverd;
all_true_access_list= self.robot.create_access_list( [True] * 10 )
all_true_access_list= self.robot.create_access_list( [True] * 7 )
robot_account_Permissions_list = []
robot_account_Permissions = v2_swagger_client.RobotPermission(kind = "project", namespace = "*", access = all_true_access_list)
robot_account_Permissions_list.append(robot_account_Permissions)