Remove push+pull action (#7571)

Signed-off-by: He Weiwei <hweiwei@vmware.com>

src/common/rbac/const.go

@@ -18,9 +18,8 @@ package rbac
 const (
 	ActionAll = Action("*") // action match any other actions
 
-	ActionPull     = Action("pull")      // pull repository tag
+	ActionPull = Action("pull") // pull repository tag
-	ActionPush     = Action("push")      // push repository tag
+	ActionPush = Action("push") // push repository tag
-	ActionPushPull = Action("push+pull") // compatible with security all perm of project
 
 	// create, read, update, delete, list actions compatible with restful api methods
 	ActionCreate = Action("create")

src/common/rbac/project/util.go

@@ -102,7 +102,6 @@ var (
 		{Resource: rbac.ResourceRepository, Action: rbac.ActionList},
 		{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},

src/common/rbac/project/visitor_role.go

@@ -59,7 +59,6 @@ var (
 			{Resource: rbac.ResourceRepository, Action: rbac.ActionList},
 			{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},

src/common/security/local/context_test.go

@@ -171,7 +171,7 @@ func TestIsSolutionUser(t *testing.T) {
 	assert.False(t, ctx.IsSolutionUser())
 }
 
-func TestHasReadPerm(t *testing.T) {
+func TestHasPullPerm(t *testing.T) {
 	// public project
 	ctx := NewSecurityContext(nil, pm)
 
@@ -201,7 +201,7 @@ func TestHasReadPerm(t *testing.T) {
 	assert.True(t, ctx.Can(rbac.ActionPull, resource))
 }
 
-func TestHasWritePerm(t *testing.T) {
+func TestHasPushPerm(t *testing.T) {
 	resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository)
 
 	// unauthenticated
@@ -224,26 +224,26 @@ func TestHasWritePerm(t *testing.T) {
 	assert.True(t, ctx.Can(rbac.ActionPush, resource))
 }
 
-func TestHasAllPerm(t *testing.T) {
+func TestHasPushPullPerm(t *testing.T) {
 	resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository)
 
 	// unauthenticated
 	ctx := NewSecurityContext(nil, pm)
-	assert.False(t, ctx.Can(rbac.ActionPushPull, resource))
+	assert.False(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource))
 
 	// authenticated, has all perms
 	ctx = NewSecurityContext(projectAdminUser, pm)
-	assert.True(t, ctx.Can(rbac.ActionPushPull, resource))
+	assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource))
 
 	// authenticated, system admin
 	ctx = NewSecurityContext(&models.User{
 		Username:     "admin",
 		HasAdminRole: true,
 	}, pm)
-	assert.True(t, ctx.Can(rbac.ActionPushPull, resource))
+	assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource))
 }
 
-func TestHasAllPermWithGroup(t *testing.T) {
+func TestHasPushPullPermWithGroup(t *testing.T) {
 	PrepareGroupTest()
 	project, err := dao.GetProjectByName("group_project")
 	if err != nil {
@@ -260,7 +260,6 @@ func TestHasAllPermWithGroup(t *testing.T) {
 	resource := rbac.NewProjectNamespace(project.Name).Resource(rbac.ResourceRepository)
 
 	ctx := NewSecurityContext(developer, pm)
-	assert.False(t, ctx.Can(rbac.ActionPushPull, resource))
 	assert.True(t, ctx.Can(rbac.ActionPush, resource))
 	assert.True(t, ctx.Can(rbac.ActionPull, resource))
 }

src/common/security/robot/context_test.go

@@ -133,14 +133,13 @@ func TestIsSolutionUser(t *testing.T) {
 	assert.False(t, ctx.IsSolutionUser())
 }
 
-func TestHasReadPerm(t *testing.T) {
+func TestHasPullPerm(t *testing.T) {
-
+	policies := []*rbac.Policy{
-	rbacPolicy := &rbac.Policy{
+		{
-		Resource: "/project/testrobot/repository",
+			Resource: "/project/testrobot/repository",
-		Action:   "pull",
+			Action:   rbac.ActionPull,
+		},
 	}
-	policies := []*rbac.Policy{}
-	policies = append(policies, rbacPolicy)
 	robot := &models.Robot{
 		Name:        "test_robot_1",
 		Description: "desc",
@@ -151,14 +150,13 @@ func TestHasReadPerm(t *testing.T) {
 	assert.True(t, ctx.Can(rbac.ActionPull, resource))
 }
 
-func TestHasWritePerm(t *testing.T) {
+func TestHasPushPerm(t *testing.T) {
-
+	policies := []*rbac.Policy{
-	rbacPolicy := &rbac.Policy{
+		{
-		Resource: "/project/testrobot/repository",
+			Resource: "/project/testrobot/repository",
-		Action:   "push",
+			Action:   rbac.ActionPush,
+		},
 	}
-	policies := []*rbac.Policy{}
-	policies = append(policies, rbacPolicy)
 	robot := &models.Robot{
 		Name:        "test_robot_2",
 		Description: "desc",
@@ -169,13 +167,17 @@ func TestHasWritePerm(t *testing.T) {
 	assert.True(t, ctx.Can(rbac.ActionPush, resource))
 }
 
-func TestHasAllPerm(t *testing.T) {
+func TestHasPushPullPerm(t *testing.T) {
-	rbacPolicy := &rbac.Policy{
+	policies := []*rbac.Policy{
-		Resource: "/project/testrobot/repository",
+		{
-		Action:   "push+pull",
+			Resource: "/project/testrobot/repository",
+			Action:   rbac.ActionPush,
+		},
+		{
+			Resource: "/project/testrobot/repository",
+			Action:   rbac.ActionPull,
+		},
 	}
-	policies := []*rbac.Policy{}
-	policies = append(policies, rbacPolicy)
 	robot := &models.Robot{
 		Name:        "test_robot_3",
 		Description: "desc",
@@ -183,7 +185,7 @@ func TestHasAllPerm(t *testing.T) {
 
 	ctx := NewSecurityContext(robot, pm, policies)
 	resource := rbac.NewProjectNamespace(private.Name).Resource(rbac.ResourceRepository)
-	assert.True(t, ctx.Can(rbac.ActionPushPull, resource))
+	assert.True(t, ctx.Can(rbac.ActionPush, resource) && ctx.Can(rbac.ActionPull, resource))
 }
 
 func TestGetMyProjects(t *testing.T) {

src/common/security/secret/context_test.go

@@ -96,12 +96,11 @@ func TestIsSolutionUser(t *testing.T) {
 	assert.True(t, isSolutionUser)
 }
 
-func TestHasReadPerm(t *testing.T) {
+func TestHasPullPerm(t *testing.T) {
-	readAction := rbac.Action("pull")
 	resource := rbac.Resource("/project/project_name/repository")
 	// secret store is null
 	context := NewSecurityContext("", nil)
-	hasReadPerm := context.Can(readAction, resource)
+	hasReadPerm := context.Can(rbac.ActionPull, resource)
 	assert.False(t, hasReadPerm)
 
 	// invalid secret
@@ -109,7 +108,7 @@ func TestHasReadPerm(t *testing.T) {
 		secret.NewStore(map[string]string{
 			"jobservice_secret": secret.JobserviceUser,
 		}))
-	hasReadPerm = context.Can(readAction, resource)
+	hasReadPerm = context.Can(rbac.ActionPull, resource)
 	assert.False(t, hasReadPerm)
 
 	// valid secret, project name
@@ -117,51 +116,43 @@ func TestHasReadPerm(t *testing.T) {
 		secret.NewStore(map[string]string{
 			"jobservice_secret": secret.JobserviceUser,
 		}))
-	hasReadPerm = context.Can(readAction, resource)
+	hasReadPerm = context.Can(rbac.ActionPull, resource)
 	assert.True(t, hasReadPerm)
 
 	// valid secret, project ID
 	resource = rbac.Resource("/project/1/repository")
-	hasReadPerm = context.Can(readAction, resource)
+	hasReadPerm = context.Can(rbac.ActionPull, resource)
 	assert.True(t, hasReadPerm)
 }
 
-func TestHasWritePerm(t *testing.T) {
+func TestHasPushPerm(t *testing.T) {
 	context := NewSecurityContext("secret",
 		secret.NewStore(map[string]string{
 			"secret": "username",
 		}))
 
-	writeAction := rbac.Action("push")
-
 	// project name
 	resource := rbac.Resource("/project/project_name/repository")
-	hasWritePerm := context.Can(writeAction, resource)
+	assert.False(t, context.Can(rbac.ActionPush, resource))
-	assert.False(t, hasWritePerm)
 
 	// project ID
 	resource = rbac.Resource("/project/1/repository")
-	hasWritePerm = context.Can(writeAction, resource)
+	assert.False(t, context.Can(rbac.ActionPush, resource))
-	assert.False(t, hasWritePerm)
 }
 
-func TestHasAllPerm(t *testing.T) {
+func TestHasPushPullPerm(t *testing.T) {
 	context := NewSecurityContext("secret",
 		secret.NewStore(map[string]string{
 			"secret": "username",
 		}))
 
-	allAction := rbac.Action("push+pull")
-
 	// project name
 	resource := rbac.Resource("/project/project_name/repository")
-	hasAllPerm := context.Can(allAction, resource)
+	assert.False(t, context.Can(rbac.ActionPush, resource) && context.Can(rbac.ActionPull, resource))
-	assert.False(t, hasAllPerm)
 
 	// project ID
 	resource = rbac.Resource("/project/1/repository")
-	hasAllPerm = context.Can(allAction, resource)
+	assert.False(t, context.Can(rbac.ActionPush, resource) && context.Can(rbac.ActionPull, resource))
-	assert.False(t, hasAllPerm)
 }
 
 func TestGetMyProjects(t *testing.T) {