Repository: casbin/casbin (go)

Issue:
[Bug] Different types of role definition wrong g function
**Want to prioritize this issue? Try:**

[![issuehunt-to-marktext](https://github.com/BoostIO/issuehunt-materials/raw/master/v1/issuehunt-button-v1.svg)](https://issuehunt.io/r/casbin/casbin)

------

**Describe the bug**
G function will be override by wrong generated g function if we have at least *2* different types role definition.

https://github.com/casbin/casbin/blob/e46d08ee10207a3fd89972e40590f794612e27d1/enforcer.go#L584-L594

**To Reproduce**
https://go.dev/play/p/blXW5-bZ1ZA

**Screenshots**
<img width="970" alt="image" src="https://github.com/casbin/casbin/assets/17849741/197c5c9b-cc7c-40b3-a183-7d907414c766">

<img width="834" alt="image" src="https://github.com/casbin/casbin/assets/17849741/3d5e2b4a-9df2-4d4a-b46e-5a02bdc2e92e">


Pull request description:
feat: fix different types of role definition wrong g function
Fix #1306 

Patch:
diff --git a/enforcer.go b/enforcer.go
index 9f9e550..9caed72 100644
--- a/enforcer.go
+++ b/enforcer.go
@@ -472,16 +472,20 @@ func (e *Enforcer) initRmMap() {
 			continue
 		}
 		if len(assertion.Tokens) <= 2 && len(assertion.ParamsTokens) == 0 {
-			e.rmMap[ptype] = defaultrolemanager.NewRoleManagerImpl(10)
+			assertion.RM = defaultrolemanager.NewRoleManagerImpl(10)
+			e.rmMap[ptype] = assertion.RM
 		}
 		if len(assertion.Tokens) <= 2 && len(assertion.ParamsTokens) != 0 {
-			e.condRmMap[ptype] = defaultrolemanager.NewConditionalRoleManager(10)
+			assertion.CondRM = defaultrolemanager.NewConditionalRoleManager(10)
+			e.condRmMap[ptype] = assertion.CondRM
 		}
 		if len(assertion.Tokens) > 2 {
 			if len(assertion.ParamsTokens) == 0 {
-				e.rmMap[ptype] = defaultrolemanager.NewRoleManager(10)
+				assertion.RM = defaultrolemanager.NewRoleManager(10)
+				e.rmMap[ptype] = assertion.RM
 			} else {
-				e.condRmMap[ptype] = defaultrolemanager.NewConditionalDomainManager(10)
+				assertion.CondRM = defaultrolemanager.NewConditionalDomainManager(10)
+				e.condRmMap[ptype] = assertion.CondRM
 			}
 			matchFun := "keyMatch(r_dom, p_dom)"
 			if strings.Contains(e.model["m"]["m"].Value, matchFun) {
@@ -584,10 +588,13 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac
 	functions := e.fm.GetFunctions()
 	if _, ok := e.model["g"]; ok {
 		for key, ast := range e.model["g"] {
-			if len(e.rmMap) != 0 {
+			// g must be a normal role definition (ast.RM != nil)
+			//   or a conditional role definition (ast.CondRM != nil)
+			// ast.RM and ast.CondRM shouldn't be nil at the same time
+			if ast.RM != nil {
 				functions[key] = util.GenerateGFunction(ast.RM)
 			}
-			if len(e.condRmMap) != 0 {
+			if ast.CondRM != nil {
 				functions[key] = util.GenerateConditionalGFunction(ast.CondRM)
 			}
 		}
diff --git a/examples/rbac_with_different_types_of_roles_model.conf b/examples/rbac_with_different_types_of_roles_model.conf
new file mode 100644
index 0000000..069f234
--- /dev/null
+++ b/examples/rbac_with_different_types_of_roles_model.conf
@@ -0,0 +1,15 @@
+[request_definition]
+r = sub, obj, act
+
+[policy_definition]
+p = sub, dom, obj, act
+
+[role_definition]
+g = _, _, _, (_, _)
+g2 = _, _
+
+[policy_effect]
+e = some(where (p.eft == allow))
+
+[matchers]
+m = g(r.sub, p.sub, p.dom) && g2(r.obj, p.dom) && regexMatch(r.act, p.act)
diff --git a/examples/rbac_with_different_types_of_roles_policy.csv b/examples/rbac_with_different_types_of_roles_policy.csv
new file mode 100644
index 0000000..67d090b
--- /dev/null
+++ b/examples/rbac_with_different_types_of_roles_policy.csv
@@ -0,0 +1,12 @@
+p, role:owner, domain1, _, (read|write)
+p, role:developer, domain1, _, read
+
+p, role:owner, domain2, _, (read|write)
+p, role:developer, domain2, _, read
+
+g, alice, role:owner, domain1, _, _
+g, bob, role:developer, domain2, _, 9999-12-30 00:00:00
+g, carol, role:owner, domain2, _, 0000-01-02 00:00:00
+
+g2, data1, domain1
+g2, data2, domain2


Test patch:
diff --git a/model_test.go b/model_test.go
index be31d5f..b3ff964 100644
--- a/model_test.go
+++ b/model_test.go
@@ -330,6 +330,31 @@ func TestRBACModelWithPattern(t *testing.T) {
 	testEnforce(t, e, "bob", "/pen2/2", "GET", true)
 }
 
+func TestRBACModelWithDifferentTypesOfRoles(t *testing.T) {
+	e, _ := NewEnforcer("examples/rbac_with_different_types_of_roles_model.conf", "examples/rbac_with_different_types_of_roles_policy.csv")
+
+	g := e.GetNamedGroupingPolicy("g")
+	for _, gp := range g {
+		if len(gp) != 5 {
+			t.Error("g parameters' num isn't 5")
+			return
+		}
+		e.AddNamedDomainLinkConditionFunc("g", gp[0], gp[1], gp[2], util.TimeMatchFunc)
+	}
+	testEnforce(t, e, "alice", "data1", "read", true)
+	testEnforce(t, e, "alice", "data1", "write", true)
+	testEnforce(t, e, "alice", "data2", "read", false)
+	testEnforce(t, e, "alice", "data2", "write", false)
+	testEnforce(t, e, "bob", "data1", "read", false)
+	testEnforce(t, e, "bob", "data1", "write", false)
+	testEnforce(t, e, "bob", "data2", "read", true)
+	testEnforce(t, e, "bob", "data2", "write", false)
+	testEnforce(t, e, "carol", "data1", "read", false)
+	testEnforce(t, e, "carol", "data1", "write", false)
+	testEnforce(t, e, "carol", "data2", "read", false)
+	testEnforce(t, e, "carol", "data2", "write", false)
+}
+
 type testCustomRoleManager struct{}
 
 func NewRoleManager() rbac.RoleManager {