Repository: casbin/casbin
Pull request #1044 (2022-07-01 05:34:58)

fix: add tests for SetFieldIndex&GetFieldIndex
fix https://github.com/casbin/casbin/issues/1015#issuecomment-1171915149

casbin-bot:  @tangyang9464 @closetool @sagilio please review
hsluoyz: @tangyang9464 plz review
hsluoyz: @JalinWang fix:

![image](https://user-images.githubusercontent.com/3787410/176830630-728056cb-e2b5-462e-be19-00b7c94c0512.png)

JalinWang: > @JalinWang fix:
> 
> ![image](https://user-images.githubusercontent.com/3787410/176830630-728056cb-e2b5-462e-be19-00b7c94c0512.png)

done
tangyang9464: @JalinWang Please add the appropriate documentation for this. When users use RBAC or priority-related APIs, the default Index they should follow or how to customize FieldIndex
JalinWang: > @JalinWang Please add the appropriate documentation for this. When users use RBAC or priority-related APIs, the default Index they should follow or how to customize FieldIndex

ok
JalinWang: During the document composition, I add the additional test case of the customized priority token. 

![image](https://user-images.githubusercontent.com/20621784/176843326-8ab2df83-39c3-4f3c-bbbb-d28b69367d33.png)


Patch:
diff --git a/examples/priority_model_explicit_customized.conf b/examples/priority_model_explicit_customized.conf
new file mode 100644
index 0000000..5071fa7
--- /dev/null
+++ b/examples/priority_model_explicit_customized.conf
@@ -0,0 +1,14 @@
+[request_definition]
+r = subject, obj, act
+
+[policy_definition]
+p = customized_priority, obj, act, eft, subject
+
+[role_definition]
+g = _, _
+
+[policy_effect]
+e = priority(p.eft) || deny
+
+[matchers]
+m = g(r.subject, p.subject) && r.obj == p.obj && r.act == p.act
\ No newline at end of file
diff --git a/examples/priority_policy_explicit_customized.csv b/examples/priority_policy_explicit_customized.csv
new file mode 100644
index 0000000..a861e2b
--- /dev/null
+++ b/examples/priority_policy_explicit_customized.csv
@@ -0,0 +1,12 @@
+p, 10, data1, read, deny, data1_deny_group
+p, 10, data1, write, deny, data1_deny_group
+p, 10, data2, read, allow, data2_allow_group
+p, 10, data2, write, allow, data2_allow_group
+
+
+p, 1, data1, write, allow, alice
+p, 1, data1, read, allow, alice
+p, 1, data2, read, deny, bob
+
+g, bob, data2_allow_group
+g, alice, data1_deny_group
diff --git a/examples/rbac_with_domain_pattern_policy.csv b/examples/rbac_with_domain_pattern_policy.csv
index c8abd35..783f721 100644
--- a/examples/rbac_with_domain_pattern_policy.csv
+++ b/examples/rbac_with_domain_pattern_policy.csv
@@ -2,6 +2,7 @@ p, admin, domain1, data1, read
 p, admin, domain1, data1, write
 p, admin, domain2, data2, read
 p, admin, domain2, data2, write
+p, admin, *, data3, read
 
 g, alice, admin, *
 g, bob, admin, domain2
\ No newline at end of file
diff --git a/internal_api.go b/internal_api.go
index 8f84006..d0ab8f3 100644
--- a/internal_api.go
+++ b/internal_api.go
@@ -15,6 +15,8 @@
 package casbin
 
 import (
+	"fmt"
+
 	Err "github.com/casbin/casbin/v2/errors"
 	"github.com/casbin/casbin/v2/model"
 	"github.com/casbin/casbin/v2/persist"
@@ -190,6 +192,10 @@ func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string, newR
 }
 
 func (e *Enforcer) updatePolicies(sec string, ptype string, oldRules [][]string, newRules [][]string) (bool, error) {
+	if len(newRules) != len(oldRules) {
+		return false, fmt.Errorf("the length of oldRules should be equal to the length of newRules, but got the length of oldRules is %d, the length of newRules is %d", len(oldRules), len(newRules))
+	}
+
 	if e.dispatcher != nil && e.autoNotifyDispatcher {
 		return true, e.dispatcher.UpdatePolicies(sec, ptype, oldRules, newRules)
 	}
diff --git a/model/model.go b/model/model.go
index 50ec866..25ce824 100644
--- a/model/model.go
+++ b/model/model.go
@@ -72,6 +72,8 @@ func (model Model) AddDef(sec string, key string, value string) bool {
 		for i := range ast.Tokens {
 			ast.Tokens[i] = key + "_" + strings.TrimSpace(ast.Tokens[i])
 		}
+	} else if sec == "g" {
+		ast.Tokens = strings.Split(ast.Value, ",")
 	} else {
 		ast.Value = util.RemoveComments(util.EscapeAssertion(ast.Value))
 	}
diff --git a/model/policy.go b/model/policy.go
index c02e092..d16669f 100644
--- a/model/policy.go
+++ b/model/policy.go
@@ -15,6 +15,7 @@
 package model
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 
@@ -126,6 +127,30 @@ func (model Model) GetFilteredPolicy(sec string, ptype string, fieldIndex int, f
 	return res
 }
 
+// HasPolicyEx determines whether a model has the specified policy rule with error.
+func (model Model) HasPolicyEx(sec string, ptype string, rule []string) (bool, error) {
+	assertion := model[sec][ptype]
+	switch sec {
+	case "p":
+		if len(rule) != len(assertion.Tokens) {
+			return false, fmt.Errorf(
+				"invalid policy rule size: expected %d, got %d, rule: %v",
+				len(model["p"][ptype].Tokens),
+				len(rule),
+				rule)
+		}
+	case "g":
+		if len(rule) < len(assertion.Tokens) {
+			return false, fmt.Errorf(
+				"invalid policy rule size: expected %d, got %d, rule: %v",
+				len(model["g"][ptype].Tokens),
+				len(rule),
+				rule)
+		}
+	}
+	return model.HasPolicy(sec, ptype, rule), nil
+}
+
 // HasPolicy determines whether a model has the specified policy rule.
 func (model Model) HasPolicy(sec string, ptype string, rule []string) bool {
 	_, ok := model[sec][ptype].PolicyMap[strings.Join(rule, DefaultSep)]
diff --git a/persist/adapter.go b/persist/adapter.go
index 5d9507c..f76389a 100644
--- a/persist/adapter.go
+++ b/persist/adapter.go
@@ -44,7 +44,11 @@ func LoadPolicyLine(line string, m model.Model) {
 func LoadPolicyArray(rule []string, m model.Model) {
 	key := rule[0]
 	sec := key[:1]
-	if m.HasPolicy(sec, key, rule[1:]) {
+	ok, err := m.HasPolicyEx(sec, key, rule[1:])
+	if err != nil {
+		return
+	}
+	if ok {
 		return
 	}
 	m.AddPolicy(sec, key, rule[1:])
diff --git a/rbac/default-role-manager/role_manager.go b/rbac/default-role-manager/role_manager.go
index 4e3edf7..6e69cf8 100644
--- a/rbac/default-role-manager/role_manager.go
+++ b/rbac/default-role-manager/role_manager.go
@@ -201,12 +201,17 @@ func (rm *RoleManagerImpl) rebuild() {
 	})
 }
 
-func (rm *RoleManagerImpl) match(str string, pattern string) bool {
+func (rm *RoleManagerImpl) Match(str string, pattern string) bool {
 	cacheKey := strings.Join([]string{str, pattern}, "$$")
 	if v, has := rm.matchingFuncCache.Get(cacheKey); has {
 		return v.(bool)
 	} else {
-		matched := rm.matchingFunc(str, pattern)
+		var matched bool
+		if rm.matchingFunc != nil {
+			matched = rm.matchingFunc(str, pattern)
+		} else {
+			matched = str == pattern
+		}
 		rm.matchingFuncCache.Put(cacheKey, matched)
 		return matched
 	}
@@ -215,9 +220,9 @@ func (rm *RoleManagerImpl) match(str string, pattern string) bool {
 func (rm *RoleManagerImpl) rangeMatchingRoles(name string, isPattern bool, fn func(role *Role) bool) {
 	rm.allRoles.Range(func(key, value interface{}) bool {
 		name2 := key.(string)
-		if isPattern && name != name2 && rm.match(name2, name) {
+		if isPattern && name != name2 && rm.Match(name2, name) {
 			fn(value.(*Role))
-		} else if !isPattern && name != name2 && rm.match(name, name2) {
+		} else if !isPattern && name != name2 && rm.Match(name, name2) {
 			fn(value.(*Role))
 		}
 		return true
@@ -313,7 +318,7 @@ func (rm *RoleManagerImpl) DeleteLink(name1 string, name2 string, domains ...str
 
 // HasLink determines whether role: name1 inherits role: name2.
 func (rm *RoleManagerImpl) HasLink(name1 string, name2 string, domains ...string) (bool, error) {
-	if name1 == name2 || (rm.matchingFunc != nil && rm.match(name1, name2)) {
+	if name1 == name2 || (rm.matchingFunc != nil && rm.Match(name1, name2)) {
 		return true, nil
 	}
 
@@ -337,7 +342,7 @@ func (rm *RoleManagerImpl) hasLinkHelper(targetName string, roles map[string]*Ro
 
 	nextRoles := map[string]*Role{}
 	for _, role := range roles {
-		if targetName == role.name || (rm.matchingFunc != nil && rm.match(role.name, targetName)) {
+		if targetName == role.name || (rm.matchingFunc != nil && rm.Match(role.name, targetName)) {
 			return true
 		}
 		role.rangeRoles(func(key, value interface{}) bool {
@@ -507,12 +512,17 @@ func (dm *DomainManager) getDomain(domains ...string) (domain string, err error)
 	}
 }
 
-func (dm *DomainManager) match(str string, pattern string) bool {
+func (dm *DomainManager) Match(str string, pattern string) bool {
 	cacheKey := strings.Join([]string{str, pattern}, "$$")
 	if v, has := dm.matchingFuncCache.Get(cacheKey); has {
 		return v.(bool)
 	} else {
-		matched := dm.domainMatchingFunc(str, pattern)
+		var matched bool
+		if dm.domainMatchingFunc != nil {
+			matched = dm.domainMatchingFunc(str, pattern)
+		} else {
+			matched = str == pattern
+		}
 		dm.matchingFuncCache.Put(cacheKey, matched)
 		return matched
 	}
@@ -522,7 +532,7 @@ func (dm *DomainManager) rangeAffectedRoleManagers(domain string, fn func(rm *Ro
 	if dm.domainMatchingFunc != nil {
 		dm.rmMap.Range(func(key, value interface{}) bool {
 			domain2 := key.(string)
-			if domain != domain2 && dm.match(domain2, domain) {
+			if domain != domain2 && dm.Match(domain2, domain) {
 				fn(value.(*RoleManagerImpl))
 			}
 			return true
@@ -551,7 +561,7 @@ func (dm *DomainManager) getRoleManager(domain string, store bool) *RoleManagerI
 			dm.rmMap.Range(func(key, value interface{}) bool {
 				domain2 := key.(string)
 				rm2 := value.(*RoleManagerImpl)
-				if domain != domain2 && dm.match(domain, domain2) {
+				if domain != domain2 && dm.Match(domain, domain2) {
 					rm.copyFrom(rm2)
 				}
 				return true
diff --git a/rbac_api.go b/rbac_api.go
index b366a26..4f6c253 100644
--- a/rbac_api.go
+++ b/rbac_api.go
@@ -17,6 +17,7 @@ package casbin
 import (
 	"github.com/casbin/casbin/v2/constant"
 	"github.com/casbin/casbin/v2/errors"
+	"github.com/casbin/casbin/v2/rbac/default-role-manager"
 	"github.com/casbin/casbin/v2/util"
 )
 
@@ -294,22 +295,32 @@ func (e *Enforcer) GetImplicitPermissionsForUser(user string, domain ...string)
 // GetImplicitPermissionsForUser("alice") can only get: [["admin", "data1", "read"]], whose policy is default policy "p"
 // But you can specify the named policy "p2" to get: [["admin", "create"]] by    GetNamedImplicitPermissionsForUser("p2","alice")
 func (e *Enforcer) GetNamedImplicitPermissionsForUser(ptype string, user string, domain ...string) ([][]string, error) {
-	roles, err := e.GetImplicitRolesForUser(user, domain...)
-	if err != nil {
-		return nil, err
-	}
-
-	roles = append([]string{user}, roles...)
-
-	var res [][]string
-	var permissions [][]string
-	for _, role := range roles {
-		permissions = e.GetNamedPermissionsForUser(ptype, role, domain...)
-
-		res = append(res, permissions...)
+	permission := make([][]string, 0)
+	rm := e.GetRoleManager()
+	domainIndex, _ := e.GetFieldIndex(ptype, constant.DomainIndex)
+	for _, rule := range e.model["p"][ptype].Policy {
+		if len(domain) == 0 {
+			matched, _ := rm.HasLink(user, rule[0])
+			if matched {
+				permission = append(permission, deepCopyPolicy(rule))
+			}
+		} else {
+			for _, d := range domain {
+				matched := rm.(*defaultrolemanager.RoleManager).Match(d, rule[domainIndex])
+				if !matched {
+					continue
+				}
+				matched, _ = rm.HasLink(user, rule[0], d)
+				if matched {
+					newRule := deepCopyPolicy(rule)
+					newRule[domainIndex] = d
+					permission = append(permission, newRule)
+					break
+				}
+			}
+		}
 	}
-
-	return res, nil
+	return permission, nil
 }
 
 // GetImplicitUsersForPermission gets implicit users for a permission.
@@ -397,3 +408,10 @@ func (e *Enforcer) GetImplicitResourcesForUser(user string, domain ...string) ([
 	}
 	return res, nil
 }
+
+// deepCopyPolicy returns a deepcopy version of the policy to prevent changing policies through returned slice
+func deepCopyPolicy(src []string) []string {
+	newRule := make([]string, len(src))
+	copy(newRule, src)
+	return newRule
+}


Test patch:
diff --git a/rbac_api_test.go b/rbac_api_test.go
index b2ba1b8..0d7101d 100644
--- a/rbac_api_test.go
+++ b/rbac_api_test.go
@@ -15,6 +15,7 @@
 package casbin
 
 import (
+	"github.com/casbin/casbin/v2/constant"
 	"sort"
 	"testing"
 
@@ -314,9 +315,9 @@ func TestImplicitRoleAPI(t *testing.T) {
 	testGetRoles(t, e, []string{"/book/1/2/3/4/5", "pen_admin"}, "cathy")
 }
 
-func testGetImplicitPermissions(t *testing.T, e *Enforcer, name string, res [][]string) {
+func testGetImplicitPermissions(t *testing.T, e *Enforcer, name string, res [][]string, domain ...string) {
 	t.Helper()
-	myRes, _ := e.GetImplicitPermissionsForUser(name)
+	myRes, _ := e.GetImplicitPermissionsForUser(name, domain...)
 	t.Log("Implicit permissions for ", name, ": ", myRes)
 
 	if !util.Set2DEquals(res, myRes) {
@@ -353,10 +354,17 @@ func TestImplicitPermissionAPI(t *testing.T) {
 	testGetImplicitPermissions(t, e, "alice", [][]string{{"alice", "data1", "read"}, {"data1_admin", "data1", "read"}, {"data1_admin", "data1", "write"}, {"data2_admin", "data2", "read"}, {"data2_admin", "data2", "write"}})
 	testGetImplicitPermissions(t, e, "bob", [][]string{{"bob", "data2", "write"}})
 
+	e, _ = NewEnforcer("examples/rbac_with_domain_pattern_model.conf", "examples/rbac_with_domain_pattern_policy.csv")
+	e.AddNamedDomainMatchingFunc("g", "KeyMatch", util.KeyMatch)
+
+	testGetImplicitPermissions(t, e, "admin", [][]string{{"admin", "domain1", "data1", "read"}, {"admin", "domain1", "data1", "write"}, {"admin", "domain1", "data3", "read"}}, "domain1")
+	testGetImplicitPermissions(t, e, "admin", [][]string{{"admin", "domain1", "data1", "read"}, {"admin", "domain1", "data1", "write"}, {"admin", "domain2", "data2", "read"}, {"admin", "domain2", "data2", "write"}, {"admin", "domain1", "data3", "read"}}, "domain1", "domain2")
+
 	e, _ = NewEnforcer("examples/rbac_with_multiple_policy_model.conf", "examples/rbac_with_multiple_policy_policy.csv")
 
 	testGetNamedImplicitPermissions(t, e, "p", "alice", [][]string{{"user", "/data", "GET"}, {"admin", "/data", "POST"}})
 	testGetNamedImplicitPermissions(t, e, "p2", "alice", [][]string{{"user", "view"}, {"admin", "create"}})
+
 }
 
 func TestImplicitPermissionAPIWithDomain(t *testing.T) {
@@ -461,3 +469,69 @@ func testGetImplicitUsersForRole(t *testing.T, e *Enforcer, name string, res []s
 		t.Error("Implicit users for ", name, ": ", myRes, ", supposed to be ", res)
 	}
 }
+
+func TestExplicitPriorityModify(t *testing.T) {
+	e, _ := NewEnforcer("examples/priority_model_explicit.conf", "examples/priority_policy_explicit.csv")
+
+	testEnforce(t, e, "bob", "data2", "write", true)
+	_, err := e.AddPolicy("1", "bob", "data2", "write", "deny")
+	if err != nil {
+		t.Fatalf("AddPolicy: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", false)
+
+	_, err = e.DeletePermissionsForUser("bob")
+	if err != nil {
+		t.Fatalf("DeletePermissionForUser: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", true)
+
+	_, err = e.DeleteRole("data2_allow_group")
+	if err != nil {
+		t.Fatalf("DeleteRole: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", false)
+}
+
+func TestCustomizedFieldIndex(t *testing.T) {
+	e, _ := NewEnforcer("examples/priority_model_explicit_customized.conf",
+		"examples/priority_policy_explicit_customized.csv")
+
+	// Due to the customized priority token, the enforcer failed to handle the priority.
+	testEnforce(t, e, "bob", "data2", "read", true)
+
+	// set PriorityIndex and reload
+	e.SetFieldIndex("p", constant.PriorityIndex, 0)
+	err := e.LoadPolicy()
+	if err != nil {
+		t.Fatalf("LoadPolicy: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "read", false)
+
+	testEnforce(t, e, "bob", "data2", "write", true)
+	_, err = e.AddPolicy("1", "data2", "write", "deny", "bob")
+	if err != nil {
+		t.Fatalf("AddPolicy: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", false)
+
+	// Due to the customized subject token, the enforcer will raise an error before SetFieldIndex.
+	_, err = e.DeletePermissionsForUser("bob")
+	if err == nil {
+		t.Fatalf("Failed to warning SetFieldIndex")
+	}
+
+	e.SetFieldIndex("p", constant.SubjectIndex, 4)
+
+	_, err = e.DeletePermissionsForUser("bob")
+	if err != nil {
+		t.Fatalf("DeletePermissionForUser: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", true)
+
+	_, err = e.DeleteRole("data2_allow_group")
+	if err != nil {
+		t.Fatalf("DeleteRole: %v", err)
+	}
+	testEnforce(t, e, "bob", "data2", "write", false)
+}