From cc30b585e058eae4920e96a3285ea2aa094a6f41 Mon Sep 17 00:00:00 2001
From: Qiu Jian <qiujian@yunionyun.com>
Date: Thu, 21 Mar 2019 10:16:55 +0800
Subject: [PATCH] fix: refine policy match conditions

---
 pkg/util/rbacutils/rabc.go      |  9 +++------
 pkg/util/rbacutils/rabc_test.go | 29 +++++++++++++++++++++--------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/pkg/util/rbacutils/rabc.go b/pkg/util/rbacutils/rabc.go
index 7ef001dd65..e95b9663e0 100644
--- a/pkg/util/rbacutils/rabc.go
+++ b/pkg/util/rbacutils/rabc.go
@@ -437,13 +437,10 @@ func intersect(s1 []string, s2 []string) bool {
 }
 
 func (policy *SRbacPolicy) Match(userCred mcclient.TokenCredential) bool {
-	if len(policy.Projects) > 0 && userCred != nil && !contains(policy.Projects, userCred.GetProjectName()) {
-		return false
+	if (len(policy.Projects) == 0 || (userCred != nil && contains(policy.Projects, userCred.GetProjectName()))) && (len(policy.Roles) == 0 || (userCred != nil && intersect(policy.Roles, userCred.GetRoles()))) {
+		return true
 	}
-	if len(policy.Roles) > 0 && userCred != nil && !intersect(policy.Roles, userCred.GetRoles()) {
-		return false
-	}
-	return true
+	return false
 }
 
 func (policy *SRbacPolicy) Allow(userCred mcclient.TokenCredential, service, resource, action string, extra ...string) TRbacResult {
diff --git a/pkg/util/rbacutils/rabc_test.go b/pkg/util/rbacutils/rabc_test.go
index 02c7755796..401f6c7e1a 100644
--- a/pkg/util/rbacutils/rabc_test.go
+++ b/pkg/util/rbacutils/rabc_test.go
@@ -240,19 +240,24 @@ func TestConditionParser(t *testing.T) {
 func TestSRbacPolicyMatch(t *testing.T) {
 	cases := []struct {
 		policy   SRbacPolicy
-		userCred mcclient.SSimpleToken
+		userCred mcclient.TokenCredential
 		want     bool
 	}{
 		{
 			SRbacPolicy{},
-			mcclient.SSimpleToken{},
+			&mcclient.SSimpleToken{},
+			true,
+		},
+		{
+			SRbacPolicy{},
+			nil,
 			true,
 		},
 		{
 			SRbacPolicy{
 				Projects: []string{"system"},
 			},
-			mcclient.SSimpleToken{
+			&mcclient.SSimpleToken{
 				Project: "system",
 			},
 			true,
@@ -261,7 +266,7 @@ func TestSRbacPolicyMatch(t *testing.T) {
 			SRbacPolicy{
 				Projects: []string{"system"},
 			},
-			mcclient.SSimpleToken{
+			&mcclient.SSimpleToken{
 				Project: "demo",
 			},
 			false,
@@ -271,7 +276,7 @@ func TestSRbacPolicyMatch(t *testing.T) {
 				Projects: []string{"system"},
 				Roles:    []string{"admin"},
 			},
-			mcclient.SSimpleToken{
+			&mcclient.SSimpleToken{
 				Project: "system",
 				Roles:   "admin",
 			},
@@ -282,7 +287,7 @@ func TestSRbacPolicyMatch(t *testing.T) {
 				Projects: []string{"system"},
 				Roles:    []string{"admin"},
 			},
-			mcclient.SSimpleToken{
+			&mcclient.SSimpleToken{
 				Project: "system",
 				Roles:   "admin,_member_",
 			},
@@ -293,15 +298,23 @@ func TestSRbacPolicyMatch(t *testing.T) {
 				Projects: []string{"system"},
 				Roles:    []string{"admin"},
 			},
-			mcclient.SSimpleToken{
+			&mcclient.SSimpleToken{
 				Project: "system",
 				Roles:   "_member_",
 			},
 			false,
 		},
+		{
+			SRbacPolicy{
+				Projects: []string{"system"},
+				Roles:    []string{"admin"},
+			},
+			nil,
+			false,
+		},
 	}
 	for _, c := range cases {
-		got := c.policy.Match(&c.userCred)
+		got := c.policy.Match(c.userCred)
 		if got != c.want {
 			t.Errorf("%#v %#v got %v want %v", c.policy, c.userCred, got, c.want)
 		}