Repository: casbin/casbin
Pull request #558 (2020-08-06 07:57:17)

fix: update enforcer logic when there is no policy found
When there is no policy found:
1. Provide better error message when using eval() in matcher.
2. It is unnecessary to throw a null pointer error when use EnforceEx function, "explainIndex" is return by interface, should check this value, make sure that this value does not break the rest of the logic.


Patch:
diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml
index 96603a3..e6efaee 100644
--- a/.github/FUNDING.yml
+++ b/.github/FUNDING.yml
@@ -1,8 +1,9 @@
 # These are supported funding model platforms
 
-github: # Replace with up to 4 GitHub Sponsors-enabled usernames e.g., [user1, user2]
+github: hsluoyz
 patreon: # Replace with a single Patreon username
 open_collective: casbin
+issuehunt: casbin/casbin
 ko_fi: # Replace with a single Ko-fi username
 tidelift: # Replace with a single Tidelift platform-name/package-name e.g., npm/babel
 custom: # Replace with a single custom sponsorship URL
diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md
new file mode 100644
index 0000000..1a82814
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/bug_report.md
@@ -0,0 +1,44 @@
+---
+name: Bug report
+about: Create a report to help us improve
+title: "[Bug]"
+labels: bug
+assignees: hsluoyz
+
+---
+
+**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**
+A clear and concise description of what the bug is.
+
+**To Reproduce**
+Steps to reproduce the behavior:
+1. Go to '...'
+2. Click on '....'
+3. Scroll down to '....'
+4. See error
+
+**Expected behavior**
+A clear and concise description of what you expected to happen.
+
+**Screenshots**
+If applicable, add screenshots to help explain your problem.
+
+**Desktop (please complete the following information):**
+ - OS: [e.g. iOS]
+ - Browser [e.g. chrome, safari]
+ - Version [e.g. 22]
+
+**Smartphone (please complete the following information):**
+ - Device: [e.g. iPhone6]
+ - OS: [e.g. iOS8.1]
+ - Browser [e.g. stock browser, safari]
+ - Version [e.g. 22]
+
+**Additional context**
+Add any other context about the problem here.
diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md
new file mode 100644
index 0000000..f50da48
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/feature_request.md
@@ -0,0 +1,26 @@
+---
+name: Feature request
+about: Suggest an idea for this project
+title: "[Feature]"
+labels: enhancement
+assignees: hsluoyz
+
+---
+
+**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)
+
+------
+
+**Is your feature request related to a problem? Please describe.**
+A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
+
+**Describe the solution you'd like**
+A clear and concise description of what you want to happen.
+
+**Describe alternatives you've considered**
+A clear and concise description of any alternative solutions or features you've considered.
+
+**Additional context**
+Add any other context or screenshots about the feature request here.
diff --git a/.github/ISSUE_TEMPLATE/question.md b/.github/ISSUE_TEMPLATE/question.md
new file mode 100644
index 0000000..64eaad7
--- /dev/null
+++ b/.github/ISSUE_TEMPLATE/question.md
@@ -0,0 +1,53 @@
+---
+name: Question
+about: Questions like "Why this model and policy don't work as expected?"
+title: "[Question]"
+labels: question
+assignees: hsluoyz
+
+---
+
+**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)
+
+------
+
+**What's your scenario? What do you want to achieve?**
+Your answer here
+
+**Your model:**
+
+```ini
+[request_definition]
+r = sub, obj, act
+
+[policy_definition]
+p = sub, obj, act
+
+[role_definition]
+g = _, _
+
+[policy_effect]
+e = some(where (p.eft == allow))
+
+[matchers]
+m = g(r.sub, p.sub) && r.obj == p.obj && r.act == p.act
+```
+
+**Your policy:**
+
+```
+p, alice, data1, read
+p, bob, data2, write
+p, data2_admin, data2, read
+p, data2_admin, data2, write
+
+g, alice, data2_admin
+```
+
+**Your request(s):**
+
+```
+alice, data2, read ---> false (expected: true)
+```
diff --git a/effect/default_effector.go b/effect/default_effector.go
index 8fe7f16..7fd4116 100644
--- a/effect/default_effector.go
+++ b/effect/default_effector.go
@@ -59,6 +59,17 @@ func (e *DefaultEffector) MergeEffects(expr string, effects []Effect, results []
 				break
 			}
 		}
+	} else if expr == "some(where (p_eft == allow)) || !some(where (p_eft == deny))" {
+		result = true
+		for i, eft := range effects {
+			if eft == Allow {
+				result = true
+				explainIndex = i
+				break
+			} else if eft == Deny {
+				result = false
+			}
+		}
 	} else if expr == "priority(p_eft) || deny" {
 		result = false
 		for i, eft := range effects {
diff --git a/enforcer.go b/enforcer.go
index 3b4d1e6..d18eeb6 100644
--- a/enforcer.go
+++ b/enforcer.go
@@ -258,10 +258,7 @@ func (e *Enforcer) LoadPolicy() error {
 	return nil
 }
 
-// LoadFilteredPolicy reloads a filtered policy from file/database.
-func (e *Enforcer) LoadFilteredPolicy(filter interface{}) error {
-	e.model.ClearPolicy()
-
+func (e *Enforcer) loadFilteredPolicy(filter interface{}) error {
 	var filteredAdapter persist.FilteredAdapter
 
 	// Attempt to cast the Adapter as a FilteredAdapter
@@ -285,6 +282,18 @@ func (e *Enforcer) LoadFilteredPolicy(filter interface{}) error {
 	return nil
 }
 
+// LoadFilteredPolicy reloads a filtered policy from file/database.
+func (e *Enforcer) LoadFilteredPolicy(filter interface{}) error {
+	e.model.ClearPolicy()
+
+	return e.loadFilteredPolicy(filter)
+}
+
+// LoadIncrementalFilteredPolicy append a filtered policy from file/database.
+func (e *Enforcer) LoadIncrementalFilteredPolicy(filter interface{}) error {
+	return e.loadFilteredPolicy(filter)
+}
+
 // IsFiltered returns true if the loaded policy has been filtered.
 func (e *Enforcer) IsFiltered() bool {
 	filteredAdapter, ok := e.adapter.(persist.FilteredAdapter)
@@ -437,7 +446,11 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac
 				for _, ruleName := range ruleNames {
 					if j, ok := parameters.pTokens[ruleName]; ok {
 						rule := util.EscapeAssertion(pvals[j])
-						expWithRule = util.ReplaceEval(expWithRule, rule)
+						if strings.Contains(rule, ">") || strings.Contains(rule, "<") || strings.Contains(rule, "=") {
+							expWithRule = util.ReplaceEval(expWithRule, rule)
+						} else {
+							expWithRule = util.ReplaceEval(expWithRule, "false")
+						}
 					} else {
 						return false, errors.New("please make sure rule exists in policy when using eval() in matcher")
 					}
@@ -493,6 +506,10 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac
 
 		}
 	} else {
+		if hasEval && len(e.model["p"]["p"].Policy) == 0 {
+			return false, errors.New("please make sure rule exists in policy when using eval() in matcher")
+		}
+
 		policyEffects = make([]effect.Effect, 1)
 		matcherResults = make([]float64, 1)
 
@@ -520,7 +537,7 @@ func (e *Enforcer) enforce(matcher string, explains *[]string, rvals ...interfac
 	}
 
 	if explains != nil {
-		if explainIndex != -1 {
+		if explainIndex != -1 && len(e.model["p"]["p"].Policy) > explainIndex {
 			*explains = e.model["p"]["p"].Policy[explainIndex]
 		}
 	}
diff --git a/enforcer_interface.go b/enforcer_interface.go
index f414c1d..e392ae7 100644
--- a/enforcer_interface.go
+++ b/enforcer_interface.go
@@ -45,6 +45,7 @@ type IEnforcer interface {
 	ClearPolicy()
 	LoadPolicy() error
 	LoadFilteredPolicy(filter interface{}) error
+	LoadIncrementalFilteredPolicy(filter interface{}) error
 	IsFiltered() bool
 	SavePolicy() error
 	EnableEnforce(enable bool)
diff --git a/enforcer_synced.go b/enforcer_synced.go
index 4523979..2bed382 100644
--- a/enforcer_synced.go
+++ b/enforcer_synced.go
@@ -116,6 +116,13 @@ func (e *SyncedEnforcer) LoadFilteredPolicy(filter interface{}) error {
 	return e.Enforcer.LoadFilteredPolicy(filter)
 }
 
+// LoadIncrementalFilteredPolicy reloads a filtered policy from file/database.
+func (e *SyncedEnforcer) LoadIncrementalFilteredPolicy(filter interface{}) error {
+	e.m.Lock()
+	defer e.m.Unlock()
+	return e.Enforcer.LoadIncrementalFilteredPolicy(filter)
+}
+
 // SavePolicy saves the current policy (usually after changed with Casbin API) back to file/database.
 func (e *SyncedEnforcer) SavePolicy() error {
 	e.m.RLock()
diff --git a/examples/abac_with_rbac.conf b/examples/abac_with_rbac.conf
new file mode 100644
index 0000000..28e167e
--- /dev/null
+++ b/examples/abac_with_rbac.conf
@@ -0,0 +1,14 @@
+[request_definition]
+r = sub, obj, act
+
+[policy_definition]
+p = sub, obj, act
+
+[policy_effect]
+e = some(where (p.eft == allow))
+
+[role_definition]
+g = _, _
+
+[matchers]
+m = (g(r.sub, p.sub) || eval(p.sub)) && r.act == p.act
diff --git a/examples/abac_with_rbac.csv b/examples/abac_with_rbac.csv
new file mode 100644
index 0000000..bc4cb47
--- /dev/null
+++ b/examples/abac_with_rbac.csv
@@ -0,0 +1,4 @@
+p, admin, post, write
+p, r.sub == r.obj.Author, post, write
+
+g, alice, admin
diff --git a/frontend.go b/frontend.go
new file mode 100644
index 0000000..ce37aaa
--- /dev/null
+++ b/frontend.go
@@ -0,0 +1,32 @@
+// Copyright 2020 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package casbin
+
+import (
+	"encoding/json"
+)
+
+func CasbinJsGetPermissionForUser(e *Enforcer, user string) ([]byte, error) {
+	policy, err := e.GetImplicitPermissionsForUser(user)
+	if err != nil {
+		return nil, err
+	}
+	permission := make(map[string][]string)
+	for i := 0; i < len(policy); i++ {
+		permission[policy[i][2]] = append(permission[policy[i][2]], policy[i][1])
+	}
+	b, _ := json.Marshal(permission)
+	return b, nil
+}
diff --git a/management_api.go b/management_api.go
index 3d60ed2..48c9810 100644
--- a/management_api.go
+++ b/management_api.go
@@ -215,7 +215,7 @@ func (e *Enforcer) AddGroupingPolicy(params ...interface{}) (bool, error) {
 	return e.AddNamedGroupingPolicy("g", params...)
 }
 
-// AddGroupingPolicies adds role inheritance rulea to the current policy.
+// AddGroupingPolicies adds role inheritance rules to the current policy.
 // If the rule already exists, the function returns false for the corresponding policy rule and the rule will not be added.
 // Otherwise the function returns true for the corresponding policy rule by adding the new rule.
 func (e *Enforcer) AddGroupingPolicies(rules [][]string) (bool, error) {
@@ -254,7 +254,7 @@ func (e *Enforcer) RemoveGroupingPolicy(params ...interface{}) (bool, error) {
 	return e.RemoveNamedGroupingPolicy("g", params...)
 }
 
-// RemoveGroupingPolicies removes role inheritance rulea from the current policy.
+// RemoveGroupingPolicies removes role inheritance rules from the current policy.
 func (e *Enforcer) RemoveGroupingPolicies(rules [][]string) (bool, error) {
 	return e.RemoveNamedGroupingPolicies("g", rules)
 }


Test patch:
diff --git a/enforcer_test.go b/enforcer_test.go
index 33d5f22..9225803 100644
--- a/enforcer_test.go
+++ b/enforcer_test.go
@@ -420,7 +420,7 @@ func TestInitEmpty(t *testing.T) {
 
 	testEnforce(t, e, "alice", "/alice_data/resource1", "GET", true)
 }
-func testEnforceEx(t *testing.T, e *Enforcer, sub string, obj string, act string, res []string) {
+func testEnforceEx(t *testing.T, e *Enforcer, sub, obj, act interface{}, res []string) {
 	t.Helper()
 	_, myRes, _ := e.EnforceEx(sub, obj, act)
 
@@ -461,6 +461,10 @@ func TestEnforceEx(t *testing.T) {
 	testEnforceEx(t, e, "bob", "data1", "write", []string{})
 	testEnforceEx(t, e, "bob", "data2", "read", []string{"data2_allow_group", "data2", "read", "allow"})
 	testEnforceEx(t, e, "bob", "data2", "write", []string{"bob", "data2", "write", "deny"})
+
+	e, _ = NewEnforcer("examples/abac_model.conf")
+	obj := struct{ Owner string }{Owner: "alice"}
+	testEnforceEx(t, e, "alice", obj, "write", []string{})
 }
 
 func TestEnforceExLog(t *testing.T) {
diff --git a/error_test.go b/error_test.go
index 1e950d4..75e66a2 100644
--- a/error_test.go
+++ b/error_test.go
@@ -17,7 +17,7 @@ package casbin
 import (
 	"testing"
 
-	"github.com/casbin/casbin/v2/persist/file-adapter"
+	fileadapter "github.com/casbin/casbin/v2/persist/file-adapter"
 )
 
 func TestPathError(t *testing.T) {
@@ -70,7 +70,6 @@ func TestModelError(t *testing.T) {
 
 func TestEnforceError(t *testing.T) {
 	e, _ := NewEnforcer("examples/basic_model.conf", "examples/basic_policy.csv")
-
 	_, err := e.Enforce("wrong", "wrong")
 	if err == nil {
 		t.Errorf("Should be error here.")
@@ -78,6 +77,15 @@ func TestEnforceError(t *testing.T) {
 		t.Log("Test on error: ")
 		t.Log(err.Error())
 	}
+
+	e, _ = NewEnforcer("examples/abac_rule_model.conf")
+	_, err = e.Enforce("wang", "wang", "wang")
+	if err == nil {
+		t.Errorf("Should be error here.")
+	} else {
+		t.Log("Test on error: ")
+		t.Log(err.Error())
+	}
 }
 
 func TestNoError(t *testing.T) {
diff --git a/filter_test.go b/filter_test.go
index 93411f9..005b324 100644
--- a/filter_test.go
+++ b/filter_test.go
@@ -17,7 +17,7 @@ package casbin
 import (
 	"testing"
 
-	"github.com/casbin/casbin/v2/persist/file-adapter"
+	fileadapter "github.com/casbin/casbin/v2/persist/file-adapter"
 )
 
 func TestInitFilteredAdapter(t *testing.T) {
@@ -65,6 +65,46 @@ func TestLoadFilteredPolicy(t *testing.T) {
 	}
 }
 
+func TestAppendFilteredPolicy(t *testing.T) {
+	e, _ := NewEnforcer()
+
+	adapter := fileadapter.NewFilteredAdapter("examples/rbac_with_domains_policy.csv")
+	_ = e.InitWithAdapter("examples/rbac_with_domains_model.conf", adapter)
+	if err := e.LoadPolicy(); err != nil {
+		t.Errorf("unexpected error in LoadPolicy: %v", err)
+	}
+
+	// validate initial conditions
+	testHasPolicy(t, e, []string{"admin", "domain1", "data1", "read"}, true)
+	testHasPolicy(t, e, []string{"admin", "domain2", "data2", "read"}, true)
+
+	if err := e.LoadFilteredPolicy(&fileadapter.Filter{
+		P: []string{"", "domain1"},
+		G: []string{"", "", "domain1"},
+	}); err != nil {
+		t.Errorf("unexpected error in LoadFilteredPolicy: %v", err)
+	}
+	if !e.IsFiltered() {
+		t.Errorf("adapter did not set the filtered flag correctly")
+	}
+
+	// only policies for domain1 should be loaded
+	testHasPolicy(t, e, []string{"admin", "domain1", "data1", "read"}, true)
+	testHasPolicy(t, e, []string{"admin", "domain2", "data2", "read"}, false)
+
+	//disable clear policy and load second domain
+	if err := e.LoadIncrementalFilteredPolicy(&fileadapter.Filter{
+		P: []string{"", "domain2"},
+		G: []string{"", "", "domain2"},
+	}); err != nil {
+		t.Errorf("unexpected error in LoadFilteredPolicy: %v", err)
+	}
+
+	//both domain policies should be loaded
+	testHasPolicy(t, e, []string{"admin", "domain1", "data1", "read"}, true)
+	testHasPolicy(t, e, []string{"admin", "domain2", "data2", "read"}, true)
+}
+
 func TestFilteredPolicyInvalidFilter(t *testing.T) {
 	e, _ := NewEnforcer()
 
diff --git a/frontend_test.go b/frontend_test.go
new file mode 100644
index 0000000..2766879
--- /dev/null
+++ b/frontend_test.go
@@ -0,0 +1,93 @@
+// Copyright 2020 The casbin Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package casbin
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+func contains(arr []string, target string) bool {
+	for _, item := range arr {
+		if item == target {
+			return true
+		}
+	}
+	return false
+}
+
+func TestCasbinJsGetPermissionForUser(t *testing.T) {
+	e, err := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+	if err != nil {
+		panic(err)
+	}
+	target_str, _ := CasbinJsGetPermissionForUser(e, "alice")
+	t.Log("GetPermissionForUser Alice", string(target_str))
+	alice_target := make(map[string][]string)
+	err = json.Unmarshal(target_str, &alice_target)
+	if err != nil {
+		t.Errorf("Test error: %s", err)
+	}
+	perm, ok := alice_target["read"]
+	if !ok {
+		t.Errorf("Test error: Alice doesn't have read permission")
+	}
+	if !contains(perm, "data1") {
+		t.Errorf("Test error: Alice cannot read data1")
+	}
+	if !contains(perm, "data2") {
+		t.Errorf("Test error: Alice cannot read data2")
+	}
+	perm, ok = alice_target["write"]
+	if !ok {
+		t.Errorf("Test error: Alice doesn't have write permission")
+	}
+	if contains(perm, "data1") {
+		t.Errorf("Test error: Alice can write data1")
+	}
+	if !contains(perm, "data2") {
+		t.Errorf("Test error: Alice cannot write data2")
+	}
+
+	target_str, _ = CasbinJsGetPermissionForUser(e, "bob")
+	t.Log("GetPermissionForUser Bob", string(target_str))
+	bob_target := make(map[string][]string)
+	err = json.Unmarshal(target_str, &bob_target)
+	if err != nil {
+		t.Errorf("Test error: %s", err)
+	}
+	_, ok = bob_target["read"]
+	if ok {
+		t.Errorf("Test error: Bob has read permission")
+	}
+	perm, ok = bob_target["write"]
+	if !ok {
+		t.Errorf("Test error: Bob doesn't have permission")
+	}
+	if !contains(perm, "data2") {
+		t.Errorf("Test error: Bob cannot write data2")
+	}
+	if contains(perm, "data1") {
+		t.Errorf("Test error: Bob can write data1")
+	}
+	if contains(perm, "data_not_exist") {
+		t.Errorf("Test error: Bob can access a non-existing data")
+	}
+
+	_, ok = bob_target["rm_rf"]
+	if ok {
+		t.Errorf("Someone can have a non-existing action (rm -rf)")
+	}
+}
diff --git a/model_test.go b/model_test.go
index 46b85ee..e6af17a 100644
--- a/model_test.go
+++ b/model_test.go
@@ -561,6 +561,32 @@ func TestABACPolicy(t *testing.T) {
 	testEnforce(t, e, sub3, "/data2", "write", false)
 }
 
+type testAuthor struct {
+	Author string
+}
+
+func newTestAuthor(name string) testAuthor {
+	s := testAuthor{}
+	s.Author = name
+	return s
+}
+
+func TestABACWithRBACPolicy(t *testing.T) {
+	e, _ := NewEnforcer("examples/abac_with_rbac.conf", "examples/abac_with_rbac.csv")
+
+	post := newTestAuthor("bob")
+
+	ok, err := e.Enforce("alice", post, "write")
+	if err != nil || ok != true {
+		t.Fatal("expected Enforce returns true, nil, but got", ok, err)
+	}
+
+	ok, err = e.Enforce("bob", post, "write")
+	if err != nil || ok != true {
+		t.Fatal("expected Enforce returns true, nil, but got", ok, err)
+	}
+}
+
 func TestCommentModel(t *testing.T) {
 	e, _ := NewEnforcer("examples/comment_model.conf", "examples/basic_policy.csv")
 	testEnforce(t, e, "alice", "data1", "read", true)