From e4fe61ddb73f8548aaabf83860d8ebc58ea55408 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Thu, 26 Sep 2024 19:09:50 +0800 Subject: [PATCH] enable robot full access (#20754) * have option to enable robot full access When the system admin enable this option, the robot can be assigned with robot/user/group/quota permissions. Signed-off-by: wang yan * robot account permission enhancement Update codes according to the proposal of https://github.com/goharbor/community/pull/249 Signed-off-by: wang yan --------- Signed-off-by: wang yan --- src/common/rbac/const.go | 89 ++++++++- src/common/rbac/const_test.go | 36 ++++ src/controller/robot/controller.go | 36 ++-- src/controller/robot/model.go | 9 +- src/controller/scan/base_controller.go | 3 +- src/controller/scan/base_controller_test.go | 3 +- .../system-robot-util.ts | 5 + src/portal/src/i18n/lang/de-de-lang.json | 7 +- src/portal/src/i18n/lang/en-us-lang.json | 7 +- src/portal/src/i18n/lang/es-es-lang.json | 7 +- src/portal/src/i18n/lang/fr-fr-lang.json | 7 +- src/portal/src/i18n/lang/ko-kr-lang.json | 7 +- src/portal/src/i18n/lang/pt-br-lang.json | 7 +- src/portal/src/i18n/lang/tr-tr-lang.json | 7 +- src/portal/src/i18n/lang/zh-cn-lang.json | 7 +- src/portal/src/i18n/lang/zh-tw-lang.json | 7 +- src/server/v2.0/handler/permissions.go | 5 +- src/server/v2.0/handler/robot.go | 165 +++++++++++++--- src/server/v2.0/handler/robot_test.go | 187 +++++++++++++++++- 19 files changed, 532 insertions(+), 69 deletions(-) create mode 100644 src/common/rbac/const_test.go diff --git a/src/common/rbac/const.go b/src/common/rbac/const.go index a783e71d4..7594efb99 100644 --- a/src/common/rbac/const.go +++ b/src/common/rbac/const.go @@ -14,7 +14,9 @@ package rbac -import "github.com/goharbor/harbor/src/pkg/permission/types" +import ( + "github.com/goharbor/harbor/src/pkg/permission/types" +) // const action variables const ( @@ -81,9 +83,88 @@ const ( ResourceSecurityHub = Resource("security-hub") ) +type scope string + +const ( + ScopeSystem = scope("System") + ScopeProject = scope("Project") +) + +// RobotPermissionProvider defines the permission provider for robot account +type RobotPermissionProvider interface { + GetPermissions(s scope) []*types.Policy +} + +// GetPermissionProvider gives the robot permission provider +func GetPermissionProvider() RobotPermissionProvider { + // TODO will determine by the ui configuration + return &NolimitProvider{} +} + +// BaseProvider ... +type BaseProvider struct { +} + +// GetPermissions ... +func (d *BaseProvider) GetPermissions(s scope) []*types.Policy { + return PoliciesMap[s] +} + +// NolimitProvider ... +type NolimitProvider struct { + BaseProvider +} + +// GetPermissions ... +func (n *NolimitProvider) GetPermissions(s scope) []*types.Policy { + if s == ScopeSystem { + return append(n.BaseProvider.GetPermissions(ScopeSystem), + &types.Policy{Resource: ResourceRobot, Action: ActionCreate}, + &types.Policy{Resource: ResourceRobot, Action: ActionRead}, + &types.Policy{Resource: ResourceRobot, Action: ActionUpdate}, + &types.Policy{Resource: ResourceRobot, Action: ActionList}, + &types.Policy{Resource: ResourceRobot, Action: ActionDelete}, + + &types.Policy{Resource: ResourceUser, Action: ActionCreate}, + &types.Policy{Resource: ResourceUser, Action: ActionRead}, + &types.Policy{Resource: ResourceUser, Action: ActionUpdate}, + &types.Policy{Resource: ResourceUser, Action: ActionList}, + &types.Policy{Resource: ResourceUser, Action: ActionDelete}, + + &types.Policy{Resource: ResourceLdapUser, Action: ActionCreate}, + &types.Policy{Resource: ResourceLdapUser, Action: ActionList}, + + &types.Policy{Resource: ResourceExportCVE, Action: ActionCreate}, + &types.Policy{Resource: ResourceExportCVE, Action: ActionRead}, + + &types.Policy{Resource: ResourceQuota, Action: ActionUpdate}, + + &types.Policy{Resource: ResourceUserGroup, Action: ActionCreate}, + &types.Policy{Resource: ResourceUserGroup, Action: ActionRead}, + &types.Policy{Resource: ResourceUserGroup, Action: ActionUpdate}, + &types.Policy{Resource: ResourceUserGroup, Action: ActionList}, + &types.Policy{Resource: ResourceUserGroup, Action: ActionDelete}) + } + if s == ScopeProject { + return append(n.BaseProvider.GetPermissions(ScopeProject), + &types.Policy{Resource: ResourceRobot, Action: ActionCreate}, + &types.Policy{Resource: ResourceRobot, Action: ActionRead}, + &types.Policy{Resource: ResourceRobot, Action: ActionUpdate}, + &types.Policy{Resource: ResourceRobot, Action: ActionList}, + &types.Policy{Resource: ResourceRobot, Action: ActionDelete}, + + &types.Policy{Resource: ResourceMember, Action: ActionCreate}, + &types.Policy{Resource: ResourceMember, Action: ActionRead}, + &types.Policy{Resource: ResourceMember, Action: ActionUpdate}, + &types.Policy{Resource: ResourceMember, Action: ActionList}, + &types.Policy{Resource: ResourceMember, Action: ActionDelete}) + } + return []*types.Policy{} +} + var ( - PoliciesMap = map[string][]*types.Policy{ - "System": { + PoliciesMap = map[scope][]*types.Policy{ + ScopeSystem: { {Resource: ResourceAuditLog, Action: ActionList}, {Resource: ResourcePreatInstance, Action: ActionRead}, @@ -154,7 +235,7 @@ var ( {Resource: ResourceQuota, Action: ActionRead}, {Resource: ResourceQuota, Action: ActionList}, }, - "Project": { + ScopeProject: { {Resource: ResourceLog, Action: ActionList}, {Resource: ResourceProject, Action: ActionRead}, diff --git a/src/common/rbac/const_test.go b/src/common/rbac/const_test.go new file mode 100644 index 000000000..9a794b864 --- /dev/null +++ b/src/common/rbac/const_test.go @@ -0,0 +1,36 @@ +package rbac + +import ( + _ "github.com/goharbor/harbor/src/pkg/config/inmemory" + + "github.com/stretchr/testify/assert" + "testing" +) + +func TestBaseProvider(t *testing.T) { + permissionProvider := &BaseProvider{} + sysPermissions := permissionProvider.GetPermissions(ScopeSystem) + + for _, per := range sysPermissions { + if per.Action == ActionCreate && per.Resource == ResourceRobot { + t.Fail() + } + } +} + +func TestNolimitProvider(t *testing.T) { + permissionProvider := &BaseProvider{} + sysPermissions := permissionProvider.GetPermissions(ScopeSystem) + + for _, per := range sysPermissions { + if per.Action == ActionCreate && per.Resource == ResourceRobot { + t.Log("no limit provider has the permission of robot account creation") + } + } +} + +func TestGetPermissionProvider(t *testing.T) { + defaultPro := GetPermissionProvider() + _, ok := defaultPro.(*NolimitProvider) + assert.True(t, ok) +} diff --git a/src/controller/robot/controller.go b/src/controller/robot/controller.go index 21b17afdf..0132eba5c 100644 --- a/src/controller/robot/controller.go +++ b/src/controller/robot/controller.go @@ -97,10 +97,6 @@ func (d *controller) Count(ctx context.Context, query *q.Query) (int64, error) { // Create ... func (d *controller) Create(ctx context.Context, r *Robot) (int64, string, error) { - if err := d.setProject(ctx, r); err != nil { - return 0, "", err - } - var expiresAt int64 if r.Duration == -1 { expiresAt = -1 @@ -327,22 +323,6 @@ func (d *controller) populatePermissions(ctx context.Context, r *Robot) error { return nil } -// set the project info if it's a project level robot -func (d *controller) setProject(ctx context.Context, r *Robot) error { - if r == nil { - return nil - } - if r.Level == LEVELPROJECT { - pro, err := d.proMgr.Get(ctx, r.Permissions[0].Namespace) - if err != nil { - return err - } - r.ProjectName = pro.Name - r.ProjectID = pro.ProjectID - } - return nil -} - // convertScope converts the db scope into robot model // /system => Kind: system Namespace: / // /project/* => Kind: project Namespace: * @@ -394,6 +374,22 @@ func (d *controller) toScope(ctx context.Context, p *Permission) (string, error) return "", errors.New(nil).WithMessage("unknown robot kind").WithCode(errors.BadRequestCode) } +// set the project info if it's a project level robot +func SetProject(ctx context.Context, r *Robot) error { + if r == nil { + return nil + } + if r.Level == LEVELPROJECT { + pro, err := project.New().Get(ctx, r.Permissions[0].Namespace) + if err != nil { + return err + } + r.ProjectName = pro.Name + r.ProjectID = pro.ProjectID + } + return nil +} + func CreateSec(salt ...string) (string, string, string, error) { var secret, pwd string options := []retry.Option{ diff --git a/src/controller/robot/model.go b/src/controller/robot/model.go index 32d6a0c8a..375483cdd 100644 --- a/src/controller/robot/model.go +++ b/src/controller/robot/model.go @@ -39,10 +39,11 @@ const ( // Robot ... type Robot struct { model.Robot - ProjectName string - Level string - Editable bool `json:"editable"` - Permissions []*Permission `json:"permissions"` + ProjectName string + ProjectNameOrID interface{} + Level string + Editable bool `json:"editable"` + Permissions []*Permission `json:"permissions"` } // IsSysLevel, true is a system level robot, others are project level. diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go index 3b087315f..04213eb4c 100644 --- a/src/controller/scan/base_controller.go +++ b/src/controller/scan/base_controller.go @@ -867,7 +867,8 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64 CreatorType: "local", CreatorRef: int64(0), }, - Level: robot.LEVELPROJECT, + ProjectName: projectName, + Level: robot.LEVELPROJECT, Permissions: []*robot.Permission{ { Kind: "project", diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go index 97b2530b9..4133babf6 100644 --- a/src/controller/scan/base_controller_test.go +++ b/src/controller/scan/base_controller_test.go @@ -238,7 +238,8 @@ func (suite *ControllerTestSuite) SetupSuite() { CreatorType: "local", CreatorRef: int64(0), }, - Level: robot.LEVELPROJECT, + ProjectName: "library", + Level: robot.LEVELPROJECT, Permissions: []*robot.Permission{ { Kind: "project", diff --git a/src/portal/src/app/base/left-side-nav/system-robot-accounts/system-robot-util.ts b/src/portal/src/app/base/left-side-nav/system-robot-accounts/system-robot-util.ts index 6e3b4097c..627d1543d 100644 --- a/src/portal/src/app/base/left-side-nav/system-robot-accounts/system-robot-util.ts +++ b/src/portal/src/app/base/left-side-nav/system-robot-accounts/system-robot-util.ts @@ -79,6 +79,11 @@ export const ACTION_RESOURCE_I18N_MAP = { 'notification-policy': 'ROBOT_ACCOUNT.NOTIFICATION_POLICY', quota: 'ROBOT_ACCOUNT.QUOTA', sbom: 'ROBOT_ACCOUNT.SBOM', + robot: 'ROBOT_ACCOUNT.ROBOT', + user: 'ROBOT_ACCOUNT.USER', + 'user-group': 'ROBOT_ACCOUNT.GROUP', + 'ldap-user': 'ROBOT_ACCOUNT.LDAPUSER', + member: 'ROBOT_ACCOUNT.MEMBER', }; export function convertKey(key: string) { diff --git a/src/portal/src/i18n/lang/de-de-lang.json b/src/portal/src/i18n/lang/de-de-lang.json index 69fa37dec..b8d406b5e 100644 --- a/src/portal/src/i18n/lang/de-de-lang.json +++ b/src/portal/src/i18n/lang/de-de-lang.json @@ -423,7 +423,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "EDIT", diff --git a/src/portal/src/i18n/lang/en-us-lang.json b/src/portal/src/i18n/lang/en-us-lang.json index d5d07f87b..08bfa13fe 100644 --- a/src/portal/src/i18n/lang/en-us-lang.json +++ b/src/portal/src/i18n/lang/en-us-lang.json @@ -423,7 +423,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "EDIT", diff --git a/src/portal/src/i18n/lang/es-es-lang.json b/src/portal/src/i18n/lang/es-es-lang.json index 5b7bc4175..18f0347be 100644 --- a/src/portal/src/i18n/lang/es-es-lang.json +++ b/src/portal/src/i18n/lang/es-es-lang.json @@ -424,7 +424,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "EDIT", diff --git a/src/portal/src/i18n/lang/fr-fr-lang.json b/src/portal/src/i18n/lang/fr-fr-lang.json index 69579d0fb..670ead404 100644 --- a/src/portal/src/i18n/lang/fr-fr-lang.json +++ b/src/portal/src/i18n/lang/fr-fr-lang.json @@ -423,7 +423,12 @@ "SELECT_PERMISSIONS": "Selectionner les permissions", "SELECT_SYSTEM_PERMISSIONS": "Selectionner les permissions système", "SELECT_PROJECT_PERMISSIONS": "Selectionner les permissions projet", - "SYSTEM_PERMISSIONS": "Permissions système" + "SYSTEM_PERMISSIONS": "Permissions système", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "Éditer", diff --git a/src/portal/src/i18n/lang/ko-kr-lang.json b/src/portal/src/i18n/lang/ko-kr-lang.json index 837807682..22ade3267 100644 --- a/src/portal/src/i18n/lang/ko-kr-lang.json +++ b/src/portal/src/i18n/lang/ko-kr-lang.json @@ -420,7 +420,12 @@ "SELECT_PERMISSIONS": "권한 선택", "SELECT_SYSTEM_PERMISSIONS": "시스템 권한 선택", "SELECT_PROJECT_PERMISSIONS": "프로젝트 권한 선택", - "SYSTEM_PERMISSIONS": "시스템 권한" + "SYSTEM_PERMISSIONS": "시스템 권한", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "편집", diff --git a/src/portal/src/i18n/lang/pt-br-lang.json b/src/portal/src/i18n/lang/pt-br-lang.json index a93b4f4d4..2cbccaf9c 100644 --- a/src/portal/src/i18n/lang/pt-br-lang.json +++ b/src/portal/src/i18n/lang/pt-br-lang.json @@ -421,7 +421,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "GROUP": { "GROUP": "Grupo", diff --git a/src/portal/src/i18n/lang/tr-tr-lang.json b/src/portal/src/i18n/lang/tr-tr-lang.json index 6a9d938b9..3841c3ac6 100644 --- a/src/portal/src/i18n/lang/tr-tr-lang.json +++ b/src/portal/src/i18n/lang/tr-tr-lang.json @@ -423,7 +423,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "DÜZENLE", diff --git a/src/portal/src/i18n/lang/zh-cn-lang.json b/src/portal/src/i18n/lang/zh-cn-lang.json index b9e4ed746..f7c17e632 100644 --- a/src/portal/src/i18n/lang/zh-cn-lang.json +++ b/src/portal/src/i18n/lang/zh-cn-lang.json @@ -421,7 +421,12 @@ "SELECT_PERMISSIONS": "选择权限", "SELECT_SYSTEM_PERMISSIONS": "选择系统权限", "SELECT_PROJECT_PERMISSIONS": "选择项目权限", - "SYSTEM_PERMISSIONS": "系统权限" + "SYSTEM_PERMISSIONS": "系统权限", + "ROBOT": "机器人账户", + "USER": "用户", + "LDAPUSER": "LDAP 用户", + "GROUP": "用户组", + "MEMBER": "项目成员" }, "WEBHOOK": { "EDIT_BUTTON": "编辑", diff --git a/src/portal/src/i18n/lang/zh-tw-lang.json b/src/portal/src/i18n/lang/zh-tw-lang.json index 91b2b12f2..6e3ace073 100644 --- a/src/portal/src/i18n/lang/zh-tw-lang.json +++ b/src/portal/src/i18n/lang/zh-tw-lang.json @@ -422,7 +422,12 @@ "SELECT_PERMISSIONS": "Select Permissions", "SELECT_SYSTEM_PERMISSIONS": "Select System Permissions", "SELECT_PROJECT_PERMISSIONS": "Select Project Permissions", - "SYSTEM_PERMISSIONS": "System Permissions" + "SYSTEM_PERMISSIONS": "System Permissions", + "ROBOT": "Robot Account", + "USER": "User", + "LDAPUSER": "LDAP User", + "GROUP": "User Group", + "MEMBER": "Project Member" }, "WEBHOOK": { "EDIT_BUTTON": "編輯", diff --git a/src/server/v2.0/handler/permissions.go b/src/server/v2.0/handler/permissions.go index 0189abf23..192e88a27 100644 --- a/src/server/v2.0/handler/permissions.go +++ b/src/server/v2.0/handler/permissions.go @@ -71,11 +71,12 @@ func (p *permissionsAPI) GetPermissions(ctx context.Context, _ permissions.GetPe return p.SendError(ctx, errors.ForbiddenError(errors.New("only admins(system and project) can access permissions"))) } + provider := rbac.GetPermissionProvider() sysPermissions := make([]*types.Policy, 0) - proPermissions := rbac.PoliciesMap["Project"] + proPermissions := provider.GetPermissions(rbac.ScopeProject) if isSystemAdmin { // project admin cannot see the system level permissions - sysPermissions = rbac.PoliciesMap["System"] + sysPermissions = provider.GetPermissions(rbac.ScopeSystem) } return permissions.NewGetPermissionsOK().WithPayload(p.convertPermissions(sysPermissions, proPermissions)) diff --git a/src/server/v2.0/handler/robot.go b/src/server/v2.0/handler/robot.go index 8137775e3..119c29fce 100644 --- a/src/server/v2.0/handler/robot.go +++ b/src/server/v2.0/handler/robot.go @@ -31,8 +31,10 @@ import ( "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/controller/robot" "github.com/goharbor/harbor/src/lib" + "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/lib/q" "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" @@ -60,12 +62,23 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo return rAPI.SendError(ctx, err) } - if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionCreate); err != nil { + sc, err := rAPI.GetSecurityContext(ctx) + if err != nil { return rAPI.SendError(ctx, err) } - sc, err := rAPI.GetSecurityContext(ctx) - if err != nil { + r := &robot.Robot{ + Robot: pkg.Robot{ + Name: params.Robot.Name, + Description: params.Robot.Description, + Duration: params.Robot.Duration, + Visible: true, + }, + Level: params.Robot.Level, + ProjectNameOrID: params.Robot.Permissions[0].Namespace, + } + + if err := rAPI.requireAccess(ctx, r, rbac.ActionCreate); err != nil { return rAPI.SendError(ctx, err) } @@ -78,23 +91,36 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo default: return rAPI.SendError(ctx, errors.New(nil).WithMessage("invalid security context")) } - - r := &robot.Robot{ - Robot: pkg.Robot{ - Name: params.Robot.Name, - Description: params.Robot.Description, - Duration: params.Robot.Duration, - Visible: true, - CreatorRef: creatorRef, - CreatorType: sc.Name(), - }, - Level: params.Robot.Level, - } + r.CreatorType = sc.Name() + r.CreatorRef = creatorRef if err := lib.JSONCopy(&r.Permissions, params.Robot.Permissions); err != nil { log.Warningf("failed to call JSONCopy on robot permission when CreateRobot, error: %v", err) } + if err := robot.SetProject(ctx, r); err != nil { + return rAPI.SendError(ctx, err) + } + + if _, ok := sc.(*robotSc.SecurityContext); ok { + creatorRobots, err := rAPI.robotCtl.List(ctx, q.New(q.KeyWords{ + "name": strings.TrimPrefix(sc.GetUsername(), config.RobotPrefix(ctx)), + "project_id": r.ProjectID, + }), &robot.Option{ + WithPermission: true, + }) + if err != nil { + return rAPI.SendError(ctx, err) + } + if len(creatorRobots) == 0 { + return rAPI.SendError(ctx, errors.DeniedError(nil)) + } + + if !isValidPermissionScope(params.Robot.Permissions, creatorRobots[0].Permissions) { + return rAPI.SendError(ctx, errors.New(nil).WithMessage("permission scope is invalid. It must be equal to or more restrictive than the creator robot's permissions: %s", creatorRobots[0].Name).WithCode(errors.DENIED)) + } + } + rid, pwd, err := rAPI.robotCtl.Create(ctx, r) if err != nil { return rAPI.SendError(ctx, err) @@ -125,7 +151,7 @@ func (rAPI *robotAPI) DeleteRobot(ctx context.Context, params operation.DeleteRo return rAPI.SendError(ctx, err) } - if err := rAPI.requireAccess(ctx, r.Level, r.ProjectID, rbac.ActionDelete); err != nil { + if err := rAPI.requireAccess(ctx, r, rbac.ActionDelete); err != nil { return rAPI.SendError(ctx, err) } @@ -174,7 +200,11 @@ func (rAPI *robotAPI) ListRobot(ctx context.Context, params operation.ListRobotP } query.Keywords["Visible"] = true - if err := rAPI.requireAccess(ctx, level, projectID, rbac.ActionList); err != nil { + r := &robot.Robot{ + ProjectNameOrID: projectID, + Level: level, + } + if err := rAPI.requireAccess(ctx, r, rbac.ActionList); err != nil { return rAPI.SendError(ctx, err) } @@ -212,7 +242,7 @@ func (rAPI *robotAPI) GetRobotByID(ctx context.Context, params operation.GetRobo if err != nil { return rAPI.SendError(ctx, err) } - if err := rAPI.requireAccess(ctx, r.Level, r.ProjectID, rbac.ActionRead); err != nil { + if err := rAPI.requireAccess(ctx, r, rbac.ActionRead); err != nil { return rAPI.SendError(ctx, err) } @@ -253,7 +283,7 @@ func (rAPI *robotAPI) RefreshSec(ctx context.Context, params operation.RefreshSe return rAPI.SendError(ctx, err) } - if err := rAPI.requireAccess(ctx, r.Level, r.ProjectID, rbac.ActionUpdate); err != nil { + if err := rAPI.requireAccess(ctx, r, rbac.ActionUpdate); err != nil { return rAPI.SendError(ctx, err) } @@ -282,12 +312,21 @@ func (rAPI *robotAPI) RefreshSec(ctx context.Context, params operation.RefreshSe return operation.NewRefreshSecOK().WithPayload(robotSec) } -func (rAPI *robotAPI) requireAccess(ctx context.Context, level string, projectIDOrName interface{}, action rbac.Action) error { - if level == robot.LEVELSYSTEM { +func (rAPI *robotAPI) requireAccess(ctx context.Context, r *robot.Robot, action rbac.Action) error { + if r.Level == robot.LEVELSYSTEM { return rAPI.RequireSystemAccess(ctx, action, rbac.ResourceRobot) - } else if level == robot.LEVELPROJECT { - return rAPI.RequireProjectAccess(ctx, projectIDOrName, action, rbac.ResourceRobot) + } else if r.Level == robot.LEVELPROJECT { + var ns interface{} + if r.ProjectNameOrID != nil { + ns = r.ProjectNameOrID + } else if r.ProjectID > 0 { + ns = r.ProjectID + } else if r.ProjectName != "" { + ns = r.ProjectName + } + return rAPI.RequireProjectAccess(ctx, ns, action, rbac.ResourceRobot) } + return errors.ForbiddenError(nil) } @@ -316,17 +355,18 @@ func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Robo return errors.New(nil).WithMessage("bad request permission").WithCode(errors.BadRequestCode) } + provider := rbac.GetPermissionProvider() // to validate the access scope for _, perm := range permissions { if perm.Kind == robot.LEVELSYSTEM { - polices := rbac.PoliciesMap["System"] + polices := provider.GetPermissions(rbac.ScopeSystem) 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"] + polices := provider.GetPermissions(rbac.ScopeProject) 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) @@ -356,7 +396,8 @@ func (rAPI *robotAPI) updateV2Robot(ctx context.Context, params operation.Update return errors.BadRequestError(nil).WithMessage("cannot update the project id of robot") } } - if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionUpdate); err != nil { + r.ProjectNameOrID = params.Robot.Permissions[0].Namespace + if err := rAPI.requireAccess(ctx, r, rbac.ActionUpdate); err != nil { return err } if params.Robot.Level != r.Level || params.Robot.Name != r.Name { @@ -380,6 +421,42 @@ func (rAPI *robotAPI) updateV2Robot(ctx context.Context, params operation.Update } } + creatorRobot, err := rAPI.robotCtl.Get(ctx, r.CreatorRef, &robot.Option{ + WithPermission: true, + }) + if err != nil && !errors.IsErr(err, errors.NotFoundCode) { + return err + } + + // for nested robot only + if creatorRobot != nil && r.CreatorType == "robot" { + sc, err := rAPI.GetSecurityContext(ctx) + if err != nil { + return err + } + if _, ok := sc.(*robotSc.SecurityContext); ok { + scRobots, err := rAPI.robotCtl.List(ctx, q.New(q.KeyWords{ + "name": strings.TrimPrefix(sc.GetUsername(), config.RobotPrefix(ctx)), + "project_id": r.ProjectID, + }), &robot.Option{ + WithPermission: true, + }) + if err != nil { + return err + } + if len(scRobots) == 0 { + return errors.DeniedError(nil) + } + if scRobots[0].ID != creatorRobot.ID && scRobots[0].ID != r.ID { + return errors.New(nil).WithMessage("as for a nested robot account, only person who has the right permission or the creator robot or nested robot itself has the permission to update").WithCode(errors.DENIED) + } + } + + if !isValidPermissionScope(params.Robot.Permissions, creatorRobot.Permissions) { + return errors.New(nil).WithMessage("permission scope is invalid. It must be equal to or more restrictive than the creator robot's permissions: %s", creatorRobot.Name).WithCode(errors.DENIED) + } + } + if err := rAPI.robotCtl.Update(ctx, r, &robot.Option{ WithPermission: true, }); err != nil { @@ -414,3 +491,39 @@ func containsAccess(policies []*types.Policy, item *models.Access) bool { } return false } + +// isValidPermissionScope checks if permission slice A is a subset of permission slice B +func isValidPermissionScope(creating []*models.RobotPermission, creator []*robot.Permission) bool { + creatorMap := make(map[string]*robot.Permission) + for _, creatorPerm := range creator { + key := fmt.Sprintf("%s:%s", creatorPerm.Kind, creatorPerm.Namespace) + creatorMap[key] = creatorPerm + } + + hasLessThanOrEqualAccess := func(creating []*models.Access, creator []*types.Policy) bool { + creatorMap := make(map[string]*types.Policy) + for _, creatorP := range creator { + key := fmt.Sprintf("%s:%s:%s", creatorP.Resource, creatorP.Action, creatorP.Effect) + creatorMap[key] = creatorP + } + for _, creatingP := range creating { + key := fmt.Sprintf("%s:%s:%s", creatingP.Resource, creatingP.Action, creatingP.Effect) + if _, found := creatorMap[key]; !found { + return false + } + } + return true + } + + for _, pCreating := range creating { + key := fmt.Sprintf("%s:%s", pCreating.Kind, pCreating.Namespace) + creatingPerm, found := creatorMap[key] + if !found { + return false + } + if !hasLessThanOrEqualAccess(pCreating.Access, creatingPerm.Access) { + return false + } + } + return true +} diff --git a/src/server/v2.0/handler/robot_test.go b/src/server/v2.0/handler/robot_test.go index e3cfe076e..9cd64de3e 100644 --- a/src/server/v2.0/handler/robot_test.go +++ b/src/server/v2.0/handler/robot_test.go @@ -1,10 +1,15 @@ package handler import ( - "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/server/v2.0/models" "math" "testing" + + "github.com/stretchr/testify/assert" + + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/controller/robot" + "github.com/goharbor/harbor/src/pkg/permission/types" + "github.com/goharbor/harbor/src/server/v2.0/models" ) func TestValidLevel(t *testing.T) { @@ -207,3 +212,181 @@ func TestContainsAccess(t *testing.T) { }) } } + +func TestValidPermissionScope(t *testing.T) { + tests := []struct { + name string + creatingPerms []*models.RobotPermission + creatorPerms []*robot.Permission + expected bool + }{ + { + name: "Project - subset", + creatingPerms: []*models.RobotPermission{ + { + Kind: "project", + Namespace: "testSubset", + Access: []*models.Access{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "project", + Namespace: "testSubset", + Access: []*types.Policy{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + {Resource: "repository", Action: "push", Effect: "allow"}, + }, + }, + }, + expected: true, + }, + { + name: "Project - not Subset", + creatingPerms: []*models.RobotPermission{ + { + Kind: "project", + Namespace: "testNotSubset", + Access: []*models.Access{ + {Resource: "repository", Action: "push", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "project", + Namespace: "testNotSubset", + Access: []*types.Policy{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + expected: false, + }, + { + name: "Project - equal", + creatingPerms: []*models.RobotPermission{ + { + Kind: "project", + Namespace: "library", + Access: []*models.Access{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "project", + Namespace: "library", + Access: []*types.Policy{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + expected: true, + }, + { + name: "Project - different", + creatingPerms: []*models.RobotPermission{ + { + Kind: "project", + Namespace: "library", + Access: []*models.Access{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "project", + Namespace: "other", + Access: []*types.Policy{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + expected: false, + }, + { + name: "Project - empty creator", + creatingPerms: []*models.RobotPermission{ + { + Kind: "project", + Namespace: "library", + Access: []*models.Access{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{}, + expected: false, + }, + { + name: "Project - empty creating", + creatingPerms: []*models.RobotPermission{}, + creatorPerms: []*robot.Permission{ + { + Kind: "project", + Namespace: "library", + Access: []*types.Policy{ + {Resource: "repository", Action: "pull", Effect: "allow"}, + }, + }, + }, + expected: true, + }, + { + name: "System - subset", + creatingPerms: []*models.RobotPermission{ + { + Kind: "system", + Namespace: "admin", + Access: []*models.Access{ + {Resource: "user", Action: "create", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "system", + Namespace: "admin", + Access: []*types.Policy{ + {Resource: "user", Action: "create", Effect: "allow"}, + {Resource: "user", Action: "delete", Effect: "allow"}, + }, + }, + }, + expected: true, + }, + { + name: "System - not subset", + creatingPerms: []*models.RobotPermission{ + { + Kind: "system", + Namespace: "admin", + Access: []*models.Access{ + {Resource: "user", Action: "delete", Effect: "allow"}, + }, + }, + }, + creatorPerms: []*robot.Permission{ + { + Kind: "system", + Namespace: "admin", + Access: []*types.Policy{ + {Resource: "user", Action: "create", Effect: "allow"}, + }, + }, + }, + expected: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isValidPermissionScope(tt.creatingPerms, tt.creatorPerms) + assert.Equal(t, tt.expected, result) + }) + } +}