humingcheng commented on a change in pull request #1014:
URL: 
https://github.com/apache/servicecomb-service-center/pull/1014#discussion_r639813394



##########
File path: server/service/rbac/decision_test.go
##########
@@ -18,55 +18,175 @@
 package rbac_test
 
 import (
-       "context"
-       "io/ioutil"
        "testing"
 
-       "github.com/go-chassis/go-archaius"
-       "github.com/go-chassis/go-chassis/v2/security/secret"
+       rbacModel "github.com/go-chassis/cari/rbac"
        "github.com/stretchr/testify/assert"
 
        "github.com/apache/servicecomb-service-center/server/service/rbac"
-       "github.com/apache/servicecomb-service-center/server/service/rbac/dao"
 )
 
-func TestAllow(t *testing.T) {
-       err := archaius.Init(archaius.WithMemorySource(), 
archaius.WithENVSource())
-       assert.NoError(t, err)
+func TestGetLabel(t *testing.T) {
+       perms := []*rbacModel.Permission{
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "b"},
+                               },
+                               {
+                                       Type:   rbac.ResourceService,
+                                       Labels: map[string]string{"e": "f"},
+                               },
+                       },
+                       Verbs: []string{"get"},
+               },
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceService,
+                                       Labels: map[string]string{"c": "d"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               },
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type: rbac.ResourceService,
+                               },
+                       },
+                       Verbs: []string{"delete"},
+               },
+       }
+       t.Run("resource and verb matched, should allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"create")
+               assert.True(t, allow)
+               assert.Equal(t, 1, len(labelList))
+       })
 
-       pri, pub, err := secret.GenRSAKeyPair(4096)
-       assert.NoError(t, err)
+       t.Run("nums of resource matched, should allow and combine their 
labels", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"get")
+               assert.True(t, allow)
+               assert.Equal(t, 2, len(labelList))
+       })
 
-       b, err := secret.RSAPrivate2Bytes(pri)
-       assert.NoError(t, err)
-       ioutil.WriteFile("./private.key", b, 0600)
-       b, err = secret.RSAPublicKey2Bytes(pub)
-       err = ioutil.WriteFile("./rbac.pub", b, 0600)
-       assert.NoError(t, err)
+       t.Run("nums of resource matched, one of them has no label, should allow 
and no label", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"delete")
+               assert.True(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+       t.Run("resource not matched, should not allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceRole, 
"delete")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+       t.Run("Verb not matched, should not allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceAccount, 
"delete")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+}
 
-       archaius.Set(rbac.InitPassword, "Complicated_password1")
+func TestGetLabelFromSinglePerm(t *testing.T) {
+       t.Run("resource and verb match, should allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "b"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceAccount, "create")
+               assert.True(t, allow)
+               assert.Equal(t, 1, len(labelList))
+               assert.Equal(t, "b", labelList[0]["a"])
+       })
 
-       dao.DeleteAccount(context.Background(), "root")
-       dao.DeleteAccount(context.Background(), "a")
-       dao.DeleteAccount(context.Background(), "b")
+       t.Run("resource not match, should no allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "a"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceService, "create")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
 
-       rbac.Init()
-       a, err := dao.GetAccount(context.Background(), "root")
-       assert.NoError(t, err)
-       assert.Equal(t, "root", a.Name)
+       t.Run("verb not match, should no allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "a"},
+                               },
+                       },
+                       Verbs: []string{"get"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceAccount, "create")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+}
 
-       t.Run("admin can operate any resource", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"admin"}, 
"default", "account", "create")
-               assert.True(t, ok)
-               ok, _ = rbac.Allow(context.TODO(), []string{"admin"}, 
"default", "service", "create")
-               assert.True(t, ok)
+func TestLabelMatched(t *testing.T) {
+       targetResourceLabel := map[string]string{
+               "a": "b",
+               "c": "d",
+       }
+       t.Run("value not match, should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "a": "h",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
        })
-       t.Run("developer can not operate account", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"developer"}, 
"default", "account", "create")
-               assert.False(t, ok)
+       t.Run("key not match, should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "h": "b",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
        })
-       t.Run("developer can operate service", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"developer"}, 
"default", "service", "create")
-               assert.True(t, ok)
+       t.Run("target resource label matches no permission resource label, 
should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "g": "h",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
        })
+       t.Run("target resource label matches part permission resource label, 
should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "g": "h",
+                       "a": "b",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
+       })
+       t.Run("target resource label matches  permission resource label, should 
not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "a": "b",
+               }
+               assert.True(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
+       })
+}
+func TestFilterLabel(t *testing.T) {
+       targetResourceLabel := []map[string]string{
+               {"a": "b", "c": "d"},

Review comment:
       改成了environment/serviceName等业务相关的内容

##########
File path: server/service/rbac/decision_test.go
##########
@@ -18,55 +18,175 @@
 package rbac_test
 
 import (
-       "context"
-       "io/ioutil"
        "testing"
 
-       "github.com/go-chassis/go-archaius"
-       "github.com/go-chassis/go-chassis/v2/security/secret"
+       rbacModel "github.com/go-chassis/cari/rbac"
        "github.com/stretchr/testify/assert"
 
        "github.com/apache/servicecomb-service-center/server/service/rbac"
-       "github.com/apache/servicecomb-service-center/server/service/rbac/dao"
 )
 
-func TestAllow(t *testing.T) {
-       err := archaius.Init(archaius.WithMemorySource(), 
archaius.WithENVSource())
-       assert.NoError(t, err)
+func TestGetLabel(t *testing.T) {
+       perms := []*rbacModel.Permission{
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "b"},
+                               },
+                               {
+                                       Type:   rbac.ResourceService,
+                                       Labels: map[string]string{"e": "f"},
+                               },
+                       },
+                       Verbs: []string{"get"},
+               },
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceService,
+                                       Labels: map[string]string{"c": "d"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               },
+               &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type: rbac.ResourceService,
+                               },
+                       },
+                       Verbs: []string{"delete"},
+               },
+       }
+       t.Run("resource and verb matched, should allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"create")
+               assert.True(t, allow)
+               assert.Equal(t, 1, len(labelList))
+       })
 
-       pri, pub, err := secret.GenRSAKeyPair(4096)
-       assert.NoError(t, err)
+       t.Run("nums of resource matched, should allow and combine their 
labels", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"get")
+               assert.True(t, allow)
+               assert.Equal(t, 2, len(labelList))
+       })
 
-       b, err := secret.RSAPrivate2Bytes(pri)
-       assert.NoError(t, err)
-       ioutil.WriteFile("./private.key", b, 0600)
-       b, err = secret.RSAPublicKey2Bytes(pub)
-       err = ioutil.WriteFile("./rbac.pub", b, 0600)
-       assert.NoError(t, err)
+       t.Run("nums of resource matched, one of them has no label, should allow 
and no label", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceService, 
"delete")
+               assert.True(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+       t.Run("resource not matched, should not allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceRole, 
"delete")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+       t.Run("Verb not matched, should not allow", func(t *testing.T) {
+               allow, labelList := rbac.GetLabel(perms, rbac.ResourceAccount, 
"delete")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+}
 
-       archaius.Set(rbac.InitPassword, "Complicated_password1")
+func TestGetLabelFromSinglePerm(t *testing.T) {
+       t.Run("resource and verb match, should allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "b"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceAccount, "create")
+               assert.True(t, allow)
+               assert.Equal(t, 1, len(labelList))
+               assert.Equal(t, "b", labelList[0]["a"])
+       })
 
-       dao.DeleteAccount(context.Background(), "root")
-       dao.DeleteAccount(context.Background(), "a")
-       dao.DeleteAccount(context.Background(), "b")
+       t.Run("resource not match, should no allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "a"},
+                               },
+                       },
+                       Verbs: []string{"*"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceService, "create")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
 
-       rbac.Init()
-       a, err := dao.GetAccount(context.Background(), "root")
-       assert.NoError(t, err)
-       assert.Equal(t, "root", a.Name)
+       t.Run("verb not match, should no allow", func(t *testing.T) {
+               perms := &rbacModel.Permission{
+                       Resources: []*rbacModel.Resource{
+                               {
+                                       Type:   rbac.ResourceAccount,
+                                       Labels: map[string]string{"a": "a"},
+                               },
+                       },
+                       Verbs: []string{"get"},
+               }
+               allow, labelList := rbac.GetLabelFromSinglePerm(perms, 
rbac.ResourceAccount, "create")
+               assert.False(t, allow)
+               assert.Equal(t, 0, len(labelList))
+       })
+}
 
-       t.Run("admin can operate any resource", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"admin"}, 
"default", "account", "create")
-               assert.True(t, ok)
-               ok, _ = rbac.Allow(context.TODO(), []string{"admin"}, 
"default", "service", "create")
-               assert.True(t, ok)
+func TestLabelMatched(t *testing.T) {
+       targetResourceLabel := map[string]string{
+               "a": "b",
+               "c": "d",
+       }
+       t.Run("value not match, should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "a": "h",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
        })
-       t.Run("developer can not operate account", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"developer"}, 
"default", "account", "create")
-               assert.False(t, ok)
+       t.Run("key not match, should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "h": "b",
+               }
+               assert.False(t, rbac.LabelMatched(targetResourceLabel, 
permResourceLabel))
        })
-       t.Run("developer can operate service", func(t *testing.T) {
-               ok, _ := rbac.Allow(context.TODO(), []string{"developer"}, 
"default", "service", "create")
-               assert.True(t, ok)
+       t.Run("target resource label matches no permission resource label, 
should not match", func(t *testing.T) {
+               permResourceLabel := map[string]string{
+                       "g": "h",

Review comment:
       改成了environment/serviceName等业务相关的内容




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to