diff --git a/src/common/rbac/rbac.go b/src/common/rbac/rbac.go index 45d91dcfe..62f0a2d5d 100644 --- a/src/common/rbac/rbac.go +++ b/src/common/rbac/rbac.go @@ -110,6 +110,10 @@ func (p *Policy) GetEffect() string { return eft.String() } +func (p *Policy) String() string { + return p.Resource.String() + ":" + p.Action.String() + ":" + p.GetEffect() +} + // Role the interface of rbac role type Role interface { // GetRoleName returns the role identity, if empty string role's policies will be ignore diff --git a/src/common/security/robot/robot.go b/src/common/security/robot/robot.go index 9bfec53a9..c900f672d 100644 --- a/src/common/security/robot/robot.go +++ b/src/common/security/robot/robot.go @@ -9,7 +9,7 @@ import ( type robot struct { username string namespace rbac.Namespace - policy []*rbac.Policy + policies []*rbac.Policy } // GetUserName get the robot name. @@ -23,7 +23,7 @@ func (r *robot) GetPolicies() []*rbac.Policy { if r.namespace.IsPublic() { policies = append(policies, project.PoliciesForPublicProject(r.namespace)...) } - policies = append(policies, r.policy...) + policies = append(policies, r.policies...) return policies } @@ -33,10 +33,30 @@ func (r *robot) GetRoles() []rbac.Role { } // NewRobot ... -func NewRobot(username string, namespace rbac.Namespace, policy []*rbac.Policy) rbac.User { +func NewRobot(username string, namespace rbac.Namespace, policies []*rbac.Policy) rbac.User { return &robot{ username: username, namespace: namespace, - policy: policy, + policies: filterPolicies(namespace, policies), } } + +func filterPolicies(namespace rbac.Namespace, policies []*rbac.Policy) []*rbac.Policy { + var results []*rbac.Policy + if len(policies) == 0 { + return results + } + + mp := map[string]bool{} + for _, policy := range project.GetAllPolicies(namespace) { + mp[policy.String()] = true + } + + for _, policy := range policies { + if mp[policy.String()] { + results = append(results, policy) + } + } + + return results +} diff --git a/src/common/security/robot/robot_test.go b/src/common/security/robot/robot_test.go index ba89fac41..617c4c3fa 100644 --- a/src/common/security/robot/robot_test.go +++ b/src/common/security/robot/robot_test.go @@ -33,10 +33,21 @@ func TestGetPolicies(t *testing.T) { robot := robot{ username: "test", namespace: rbac.NewProjectNamespace(1, false), - policy: policies, + policies: policies, } assert.Equal(t, robot.GetUserName(), "test") assert.NotNil(t, robot.GetPolicies()) assert.Nil(t, robot.GetRoles()) } + +func TestNewRobot(t *testing.T) { + policies := []*rbac.Policy{ + {Resource: "/project/1/repository", Action: "pull"}, + {Resource: "/project/library/repository", Action: "pull"}, + {Resource: "/project/library/repository", Action: "push"}, + } + + robot := NewRobot("test", rbac.NewProjectNamespace(1, false), policies) + assert.Len(t, robot.GetPolicies(), 1) +} diff --git a/src/core/api/robot.go b/src/core/api/robot.go index 651edb9a6..2d4c91012 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -25,6 +25,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/rbac/project" "github.com/goharbor/harbor/src/common/token" "github.com/goharbor/harbor/src/core/config" ) @@ -107,6 +108,11 @@ func (r *RobotAPI) Post() { return } + if err := validateRobotReq(r.project, &robotReq); err != nil { + r.SendBadRequestError(err) + return + } + // Token duration in minutes tokenDuration := time.Duration(config.RobotTokenDuration()) * time.Minute expiresAt := time.Now().UTC().Add(tokenDuration).Unix() @@ -254,3 +260,25 @@ func (r *RobotAPI) Delete() { return } } + +func validateRobotReq(p *models.Project, robotReq *models.RobotReq) error { + if len(robotReq.Access) == 0 { + return errors.New("access required") + } + + namespace, _ := rbac.Resource(fmt.Sprintf("/project/%d", p.ProjectID)).GetNamespace() + policies := project.GetAllPolicies(namespace) + + mp := map[string]bool{} + for _, policy := range policies { + mp[policy.String()] = true + } + + for _, policy := range robotReq.Access { + if !mp[policy.String()] { + return fmt.Errorf("%s action of %s resource not exist in project %s", policy.Action, policy.Resource, p.Name) + } + } + + return nil +} diff --git a/src/core/api/robot_test.go b/src/core/api/robot_test.go index baecb67b5..6316e2602 100644 --- a/src/core/api/robot_test.go +++ b/src/core/api/robot_test.go @@ -16,10 +16,11 @@ package api import ( "fmt" - "github.com/goharbor/harbor/src/common/models" - "github.com/goharbor/harbor/src/common/rbac" "net/http" "testing" + + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" ) var ( @@ -28,9 +29,10 @@ var ( ) func TestRobotAPIPost(t *testing.T) { + res := rbac.Resource("/project/1") rbacPolicy := &rbac.Policy{ - Resource: "/project/libray/repository", + Resource: res.Subresource(rbac.ResourceRepository), Action: "pull", } policies := []*rbac.Policy{} @@ -83,6 +85,51 @@ func TestRobotAPIPost(t *testing.T) { }, code: http.StatusBadRequest, }, + { + request: &testingRequest{ + method: http.MethodPost, + url: robotPath, + bodyJSON: &models.RobotReq{ + Name: "test", + Description: "resource not exist", + Access: []*rbac.Policy{ + {Resource: res.Subresource("foo"), Action: rbac.ActionCreate}, + }, + }, + credential: projAdmin4Robot, + }, + code: http.StatusBadRequest, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: robotPath, + bodyJSON: &models.RobotReq{ + Name: "test", + Description: "action not exist", + Access: []*rbac.Policy{ + {Resource: res.Subresource(rbac.ResourceRepository), Action: "foo"}, + }, + }, + credential: projAdmin4Robot, + }, + code: http.StatusBadRequest, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: robotPath, + bodyJSON: &models.RobotReq{ + Name: "test", + Description: "policy not exit", + Access: []*rbac.Policy{ + {Resource: res.Subresource(rbac.ResourceMember), Action: rbac.ActionPush}, + }, + }, + credential: projAdmin4Robot, + }, + code: http.StatusBadRequest, + }, // 403 -- developer { request: &testingRequest{ diff --git a/tests/apitests/python/library/project.py b/tests/apitests/python/library/project.py index 975457ab8..ee9969c28 100644 --- a/tests/apitests/python/library/project.py +++ b/tests/apitests/python/library/project.py @@ -182,19 +182,14 @@ class Project(base.Base): has_pull_right = True access_list = [] resource_by_project_id = "/project/"+str(project_id)+"/repository" - resource_by_project_name = "/project/"+project_name+"/repository" action_pull = "pull" action_push = "push" if has_pull_right is True: robotAccountAccess = swagger_client.RobotAccountAccess(resource = resource_by_project_id, action = action_pull) access_list.append(robotAccountAccess) - robotAccountAccess = swagger_client.RobotAccountAccess(resource = resource_by_project_name, action = action_pull) - access_list.append(robotAccountAccess) if has_push_right is True: robotAccountAccess = swagger_client.RobotAccountAccess(resource = resource_by_project_id, action = action_push) access_list.append(robotAccountAccess) - robotAccountAccess = swagger_client.RobotAccountAccess(resource = resource_by_project_name, action = action_push) - access_list.append(robotAccountAccess) robotAccountCreate = swagger_client.RobotAccountCreate(robot_name, robot_desc, access_list) client = self._get_client(**kwargs) data = []