This is an automated email from the ASF dual-hosted git repository.
hsluoyz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/casbin.git
The following commit(s) were added to refs/heads/master by this push:
new 75f29b7 fix: don't write to adapter in Self* APIs
75f29b7 is described below
commit 75f29b71985344323dc2db744a40c8c4e949084d
Author: Yang Luo <[email protected]>
AuthorDate: Fri Aug 7 00:31:05 2026 +0800
fix: don't write to adapter in Self* APIs
---
enforcer_context.go | 58 +++++-----
internal_api.go | 46 ++++----
management_api.go | 34 ++++--
model/model_test.go | 28 +++++
transaction_test.go | 45 ++++++++
watcher_self_sync_test.go | 275 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 428 insertions(+), 58 deletions(-)
diff --git a/enforcer_context.go b/enforcer_context.go
index b7e4f8a..56252c8 100644
--- a/enforcer_context.go
+++ b/enforcer_context.go
@@ -375,47 +375,47 @@ func (e *ContextEnforcer)
UpdateNamedGroupingPoliciesCtx(ctx context.Context, pt
// SelfAddPolicyCtx adds a policy rule to the current policy with context.
func (e *ContextEnforcer) SelfAddPolicyCtx(ctx context.Context, sec string,
ptype string, rule []string) (bool, error) {
- return e.addPolicyWithoutNotifyCtx(ctx, sec, ptype, rule)
+ return e.addPolicyWithoutNotifyCtx(ctx, nil, sec, ptype, rule)
}
// SelfAddPoliciesCtx adds policy rules to the current policy with context.
func (e *ContextEnforcer) SelfAddPoliciesCtx(ctx context.Context, sec string,
ptype string, rules [][]string) (bool, error) {
- return e.addPoliciesWithoutNotifyCtx(ctx, sec, ptype, rules, false)
+ return e.addPoliciesWithoutNotifyCtx(ctx, nil, sec, ptype, rules, false)
}
func (e *ContextEnforcer) SelfAddPoliciesExCtx(ctx context.Context, sec
string, ptype string, rules [][]string) (bool, error) {
- return e.addPoliciesWithoutNotifyCtx(ctx, sec, ptype, rules, true)
+ return e.addPoliciesWithoutNotifyCtx(ctx, nil, sec, ptype, rules, true)
}
// SelfRemovePolicyCtx removes a policy rule from the current policy with
context.
func (e *ContextEnforcer) SelfRemovePolicyCtx(ctx context.Context, sec string,
ptype string, rule []string) (bool, error) {
- return e.removePolicyWithoutNotifyCtx(ctx, sec, ptype, rule)
+ return e.removePolicyWithoutNotifyCtx(ctx, nil, sec, ptype, rule)
}
// SelfRemovePoliciesCtx removes policy rules from the current policy with
context.
func (e *ContextEnforcer) SelfRemovePoliciesCtx(ctx context.Context, sec
string, ptype string, rules [][]string) (bool, error) {
- return e.removePoliciesWithoutNotifyCtx(ctx, sec, ptype, rules)
+ return e.removePoliciesWithoutNotifyCtx(ctx, nil, sec, ptype, rules)
}
// SelfRemoveFilteredPolicyCtx removes policy rules that match the filter from
the current policy with context.
func (e *ContextEnforcer) SelfRemoveFilteredPolicyCtx(ctx context.Context, sec
string, ptype string, fieldIndex int, fieldValues ...string) (bool, error) {
- return e.removeFilteredPolicyWithoutNotifyCtx(ctx, sec, ptype,
fieldIndex, fieldValues)
+ return e.removeFilteredPolicyWithoutNotifyCtx(ctx, nil, sec, ptype,
fieldIndex, fieldValues)
}
// SelfUpdatePolicyCtx updates a policy rule in the current policy with
context.
func (e *ContextEnforcer) SelfUpdatePolicyCtx(ctx context.Context, sec string,
ptype string, oldRule, newRule []string) (bool, error) {
- return e.updatePolicyWithoutNotifyCtx(ctx, sec, ptype, oldRule, newRule)
+ return e.updatePolicyWithoutNotifyCtx(ctx, nil, sec, ptype, oldRule,
newRule)
}
// SelfUpdatePoliciesCtx updates policy rules in the current policy with
context.
func (e *ContextEnforcer) SelfUpdatePoliciesCtx(ctx context.Context, sec
string, ptype string, oldRules, newRules [][]string) (bool, error) {
- return e.updatePoliciesWithoutNotifyCtx(ctx, sec, ptype, oldRules,
newRules)
+ return e.updatePoliciesWithoutNotifyCtx(ctx, nil, sec, ptype, oldRules,
newRules)
}
// Internal API methods with context support
// addPolicyWithoutNotifyCtx adds a rule to the current policy with context.
-func (e *ContextEnforcer) addPolicyWithoutNotifyCtx(ctx context.Context, sec
string, ptype string, rule []string) (bool, error) {
+func (e *ContextEnforcer) addPolicyWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, sec string, ptype string, rule []string) (bool,
error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.AddPolicies(sec, ptype,
[][]string{rule})
}
@@ -425,7 +425,7 @@ func (e *ContextEnforcer) addPolicyWithoutNotifyCtx(ctx
context.Context, sec str
return false, err
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err = e.adapterCtx.AddPolicyCtx(ctx, sec, ptype, rule); err
!= nil {
if err.Error() != notImplemented {
return false, err
@@ -449,7 +449,7 @@ func (e *ContextEnforcer) addPolicyWithoutNotifyCtx(ctx
context.Context, sec str
}
// addPoliciesWithoutNotifyCtx adds rules to the current policy with context.
-func (e *ContextEnforcer) addPoliciesWithoutNotifyCtx(ctx context.Context, sec
string, ptype string, rules [][]string, autoRemoveRepeat bool) (bool, error) {
+func (e *ContextEnforcer) addPoliciesWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, sec string, ptype string, rules [][]string,
autoRemoveRepeat bool) (bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.AddPolicies(sec, ptype, rules)
}
@@ -461,7 +461,7 @@ func (e *ContextEnforcer) addPoliciesWithoutNotifyCtx(ctx
context.Context, sec s
}
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapterCtx.(persist.ContextBatchAdapter).AddPoliciesCtx(ctx, sec, ptype,
rules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -490,12 +490,12 @@ func (e *ContextEnforcer) addPoliciesWithoutNotifyCtx(ctx
context.Context, sec s
}
// removePolicyWithoutNotifyCtx removes a rule from the current policy with
context.
-func (e *ContextEnforcer) removePolicyWithoutNotifyCtx(ctx context.Context,
sec string, ptype string, rule []string) (bool, error) {
+func (e *ContextEnforcer) removePolicyWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, sec string, ptype string, rule []string) (bool,
error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.RemovePolicies(sec, ptype,
[][]string{rule})
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapterCtx.RemovePolicyCtx(ctx, sec, ptype, rule);
err != nil {
if err.Error() != notImplemented {
return false, err
@@ -519,7 +519,7 @@ func (e *ContextEnforcer) removePolicyWithoutNotifyCtx(ctx
context.Context, sec
}
// removePoliciesWithoutNotifyCtx removes rules from the current policy with
context.
-func (e *ContextEnforcer) removePoliciesWithoutNotifyCtx(ctx context.Context,
sec string, ptype string, rules [][]string) (bool, error) {
+func (e *ContextEnforcer) removePoliciesWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, sec string, ptype string, rules [][]string) (bool,
error) {
if hasPolicies, err := e.model.HasPolicies(sec, ptype, rules);
!hasPolicies || err != nil {
return hasPolicies, err
}
@@ -528,7 +528,7 @@ func (e *ContextEnforcer)
removePoliciesWithoutNotifyCtx(ctx context.Context, se
return true, e.dispatcher.RemovePolicies(sec, ptype, rules)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapterCtx.(persist.ContextBatchAdapter).RemovePoliciesCtx(ctx, sec, ptype,
rules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -551,7 +551,7 @@ func (e *ContextEnforcer)
removePoliciesWithoutNotifyCtx(ctx context.Context, se
}
// removeFilteredPolicyWithoutNotifyCtx removes policy rules that match the
filter from the current policy with context.
-func (e *ContextEnforcer) removeFilteredPolicyWithoutNotifyCtx(ctx
context.Context, sec string, ptype string, fieldIndex int, fieldValues
[]string) (bool, error) {
+func (e *ContextEnforcer) removeFilteredPolicyWithoutNotifyCtx(ctx
context.Context, shouldPersist func() bool, sec string, ptype string,
fieldIndex int, fieldValues []string) (bool, error) {
if len(fieldValues) == 0 {
return false, Err.ErrInvalidFieldValuesParameter
}
@@ -560,7 +560,7 @@ func (e *ContextEnforcer)
removeFilteredPolicyWithoutNotifyCtx(ctx context.Conte
return true, e.dispatcher.RemoveFilteredPolicy(sec, ptype,
fieldIndex, fieldValues...)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapterCtx.RemoveFilteredPolicyCtx(ctx, sec, ptype,
fieldIndex, fieldValues...); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -584,12 +584,12 @@ func (e *ContextEnforcer)
removeFilteredPolicyWithoutNotifyCtx(ctx context.Conte
}
// updatePolicyWithoutNotifyCtx updates a policy rule in the current policy
with context.
-func (e *ContextEnforcer) updatePolicyWithoutNotifyCtx(ctx context.Context,
sec string, ptype string, oldRule, newRule []string) (bool, error) {
+func (e *ContextEnforcer) updatePolicyWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, sec string, ptype string, oldRule, newRule []string)
(bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.UpdatePolicy(sec, ptype, oldRule,
newRule)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapterCtx.(persist.ContextUpdatableAdapter).UpdatePolicyCtx(ctx, sec, ptype,
oldRule, newRule); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -615,7 +615,7 @@ func (e *ContextEnforcer) updatePolicyWithoutNotifyCtx(ctx
context.Context, sec
return ruleUpdated, nil
}
-func (e *ContextEnforcer) updatePoliciesWithoutNotifyCtx(ctx context.Context,
sec string, ptype string, oldRules [][]string, newRules [][]string) (bool,
error) {
+func (e *ContextEnforcer) updatePoliciesWithoutNotifyCtx(ctx context.Context,
shouldPersist func() bool, 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))
}
@@ -624,7 +624,7 @@ func (e *ContextEnforcer)
updatePoliciesWithoutNotifyCtx(ctx context.Context, se
return true, e.dispatcher.UpdatePolicies(sec, ptype, oldRules,
newRules)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapterCtx.(persist.ContextUpdatableAdapter).UpdatePoliciesCtx(ctx, sec,
ptype, oldRules, newRules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -652,7 +652,7 @@ func (e *ContextEnforcer)
updatePoliciesWithoutNotifyCtx(ctx context.Context, se
}
func (e *ContextEnforcer) addPolicyCtx(ctx context.Context, sec string, ptype
string, rule []string) (bool, error) {
- ok, err := e.addPolicyWithoutNotifyCtx(ctx, sec, ptype, rule)
+ ok, err := e.addPolicyWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, rule)
if !ok || err != nil {
return ok, err
}
@@ -671,7 +671,7 @@ func (e *ContextEnforcer) addPolicyCtx(ctx context.Context,
sec string, ptype st
}
func (e *ContextEnforcer) addPoliciesCtx(ctx context.Context, sec string,
ptype string, rules [][]string, autoRemoveRepeat bool) (bool, error) {
- ok, err := e.addPoliciesWithoutNotifyCtx(ctx, sec, ptype, rules,
autoRemoveRepeat)
+ ok, err := e.addPoliciesWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, rules, autoRemoveRepeat)
if !ok || err != nil {
return ok, err
}
@@ -690,7 +690,7 @@ func (e *ContextEnforcer) addPoliciesCtx(ctx
context.Context, sec string, ptype
}
func (e *ContextEnforcer) updatePolicyCtx(ctx context.Context, sec string,
ptype string, oldRule []string, newRule []string) (bool, error) {
- ok, err := e.updatePolicyWithoutNotifyCtx(ctx, sec, ptype, oldRule,
newRule)
+ ok, err := e.updatePolicyWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, oldRule, newRule)
if !ok || err != nil {
return ok, err
}
@@ -709,7 +709,7 @@ func (e *ContextEnforcer) updatePolicyCtx(ctx
context.Context, sec string, ptype
}
func (e *ContextEnforcer) updatePoliciesCtx(ctx context.Context, sec string,
ptype string, oldRules [][]string, newRules [][]string) (bool, error) {
- ok, err := e.updatePoliciesWithoutNotifyCtx(ctx, sec, ptype, oldRules,
newRules)
+ ok, err := e.updatePoliciesWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, oldRules, newRules)
if !ok || err != nil {
return ok, err
}
@@ -728,7 +728,7 @@ func (e *ContextEnforcer) updatePoliciesCtx(ctx
context.Context, sec string, pty
}
func (e *ContextEnforcer) removePolicyCtx(ctx context.Context, sec string,
ptype string, rule []string) (bool, error) {
- ok, err := e.removePolicyWithoutNotifyCtx(ctx, sec, ptype, rule)
+ ok, err := e.removePolicyWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, rule)
if !ok || err != nil {
return ok, err
}
@@ -747,7 +747,7 @@ func (e *ContextEnforcer) removePolicyCtx(ctx
context.Context, sec string, ptype
}
func (e *ContextEnforcer) removePoliciesCtx(ctx context.Context, sec string,
ptype string, rules [][]string) (bool, error) {
- ok, err := e.removePoliciesWithoutNotifyCtx(ctx, sec, ptype, rules)
+ ok, err := e.removePoliciesWithoutNotifyCtx(ctx, e.shouldPersist, sec,
ptype, rules)
if !ok || err != nil {
return ok, err
}
@@ -767,7 +767,7 @@ func (e *ContextEnforcer) removePoliciesCtx(ctx
context.Context, sec string, pty
// removeFilteredPolicy removes rules based on field filters from the current
policy.
func (e *ContextEnforcer) removeFilteredPolicyCtx(ctx context.Context, sec
string, ptype string, fieldIndex int, fieldValues []string) (bool, error) {
- ok, err := e.removeFilteredPolicyWithoutNotifyCtx(ctx, sec, ptype,
fieldIndex, fieldValues)
+ ok, err := e.removeFilteredPolicyWithoutNotifyCtx(ctx, e.shouldPersist,
sec, ptype, fieldIndex, fieldValues)
if !ok || err != nil {
return ok, err
}
diff --git a/internal_api.go b/internal_api.go
index 04883af..1381ccf 100644
--- a/internal_api.go
+++ b/internal_api.go
@@ -45,7 +45,11 @@ func (e *Enforcer) validateConstraintsForGroupingPolicy()
error {
}
// addPolicy adds a rule to the current policy.
-func (e *Enforcer) addPolicyWithoutNotify(sec string, ptype string, rule
[]string) (bool, error) {
+// shouldPersist reports whether the change must also be written to the
adapter.
+// It is nil when the change originates from another instance (a watcher
callback
+// through the Self* APIs), in which case the rule is already in the storage
and
+// writing it again would fail on databases with a unique constraint.
+func (e *Enforcer) addPolicyWithoutNotify(shouldPersist func() bool, sec
string, ptype string, rule []string) (bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.AddPolicies(sec, ptype,
[][]string{rule})
}
@@ -55,7 +59,7 @@ func (e *Enforcer) addPolicyWithoutNotify(sec string, ptype
string, rule []strin
return false, err
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err = e.adapter.AddPolicy(sec, ptype, rule); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -86,7 +90,7 @@ func (e *Enforcer) addPolicyWithoutNotify(sec string, ptype
string, rule []strin
// addPoliciesWithoutNotify adds rules to the current policy without notify
// If autoRemoveRepeat == true, existing rules are automatically filtered
// Otherwise, false is returned directly.
-func (e *Enforcer) addPoliciesWithoutNotify(sec string, ptype string, rules
[][]string, autoRemoveRepeat bool) (bool, error) {
+func (e *Enforcer) addPoliciesWithoutNotify(shouldPersist func() bool, sec
string, ptype string, rules [][]string, autoRemoveRepeat bool) (bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.AddPolicies(sec, ptype, rules)
}
@@ -98,7 +102,7 @@ func (e *Enforcer) addPoliciesWithoutNotify(sec string,
ptype string, rules [][]
}
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapter.(persist.BatchAdapter).AddPolicies(sec,
ptype, rules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -132,12 +136,12 @@ func (e *Enforcer) addPoliciesWithoutNotify(sec string,
ptype string, rules [][]
}
// removePolicy removes a rule from the current policy.
-func (e *Enforcer) removePolicyWithoutNotify(sec string, ptype string, rule
[]string) (bool, error) {
+func (e *Enforcer) removePolicyWithoutNotify(shouldPersist func() bool, sec
string, ptype string, rule []string) (bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.RemovePolicies(sec, ptype,
[][]string{rule})
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapter.RemovePolicy(sec, ptype, rule); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -165,12 +169,12 @@ func (e *Enforcer) removePolicyWithoutNotify(sec string,
ptype string, rule []st
return ruleRemoved, nil
}
-func (e *Enforcer) updatePolicyWithoutNotify(sec string, ptype string, oldRule
[]string, newRule []string) (bool, error) {
+func (e *Enforcer) updatePolicyWithoutNotify(shouldPersist func() bool, sec
string, ptype string, oldRule []string, newRule []string) (bool, error) {
if e.dispatcher != nil && e.autoNotifyDispatcher {
return true, e.dispatcher.UpdatePolicy(sec, ptype, oldRule,
newRule)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapter.(persist.UpdatableAdapter).UpdatePolicy(sec, ptype, oldRule,
newRule); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -201,7 +205,7 @@ func (e *Enforcer) updatePolicyWithoutNotify(sec string,
ptype string, oldRule [
return ruleUpdated, nil
}
-func (e *Enforcer) updatePoliciesWithoutNotify(sec string, ptype string,
oldRules [][]string, newRules [][]string) (bool, error) {
+func (e *Enforcer) updatePoliciesWithoutNotify(shouldPersist func() bool, 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))
}
@@ -210,7 +214,7 @@ func (e *Enforcer) updatePoliciesWithoutNotify(sec string,
ptype string, oldRule
return true, e.dispatcher.UpdatePolicies(sec, ptype, oldRules,
newRules)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err :=
e.adapter.(persist.UpdatableAdapter).UpdatePolicies(sec, ptype, oldRules,
newRules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -243,7 +247,7 @@ func (e *Enforcer) updatePoliciesWithoutNotify(sec string,
ptype string, oldRule
}
// removePolicies removes rules from the current policy.
-func (e *Enforcer) removePoliciesWithoutNotify(sec string, ptype string, rules
[][]string) (bool, error) {
+func (e *Enforcer) removePoliciesWithoutNotify(shouldPersist func() bool, sec
string, ptype string, rules [][]string) (bool, error) {
if hasPolicies, err := e.model.HasPolicies(sec, ptype, rules);
!hasPolicies || err != nil {
return hasPolicies, err
}
@@ -252,7 +256,7 @@ func (e *Enforcer) removePoliciesWithoutNotify(sec string,
ptype string, rules [
return true, e.dispatcher.RemovePolicies(sec, ptype, rules)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapter.(persist.BatchAdapter).RemovePolicies(sec,
ptype, rules); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -280,7 +284,7 @@ func (e *Enforcer) removePoliciesWithoutNotify(sec string,
ptype string, rules [
}
// removeFilteredPolicy removes rules based on field filters from the current
policy.
-func (e *Enforcer) removeFilteredPolicyWithoutNotify(sec string, ptype string,
fieldIndex int, fieldValues []string) (bool, error) {
+func (e *Enforcer) removeFilteredPolicyWithoutNotify(shouldPersist func()
bool, sec string, ptype string, fieldIndex int, fieldValues []string) (bool,
error) {
if len(fieldValues) == 0 {
return false, Err.ErrInvalidFieldValuesParameter
}
@@ -289,7 +293,7 @@ func (e *Enforcer) removeFilteredPolicyWithoutNotify(sec
string, ptype string, f
return true, e.dispatcher.RemoveFilteredPolicy(sec, ptype,
fieldIndex, fieldValues...)
}
- if e.shouldPersist() {
+ if shouldPersist != nil && shouldPersist() {
if err := e.adapter.RemoveFilteredPolicy(sec, ptype,
fieldIndex, fieldValues...); err != nil {
if err.Error() != notImplemented {
return false, err
@@ -380,7 +384,7 @@ func (e *Enforcer) updateFilteredPoliciesWithoutNotify(sec
string, ptype string,
// addPolicy adds a rule to the current policy.
func (e *Enforcer) addPolicy(sec string, ptype string, rule []string) (bool,
error) {
ok, err := e.logPolicyOperation(log.EventAddPolicy, sec, rule, func()
(bool, error) {
- return e.addPolicyWithoutNotify(sec, ptype, rule)
+ return e.addPolicyWithoutNotify(e.shouldPersist, sec, ptype,
rule)
})
if !ok || err != nil {
@@ -404,7 +408,7 @@ func (e *Enforcer) addPolicy(sec string, ptype string, rule
[]string) (bool, err
// If autoRemoveRepeat == true, existing rules are automatically filtered
// Otherwise, false is returned directly.
func (e *Enforcer) addPolicies(sec string, ptype string, rules [][]string,
autoRemoveRepeat bool) (bool, error) {
- ok, err := e.addPoliciesWithoutNotify(sec, ptype, rules,
autoRemoveRepeat)
+ ok, err := e.addPoliciesWithoutNotify(e.shouldPersist, sec, ptype,
rules, autoRemoveRepeat)
if !ok || err != nil {
return ok, err
}
@@ -425,7 +429,7 @@ func (e *Enforcer) addPolicies(sec string, ptype string,
rules [][]string, autoR
// removePolicy removes a rule from the current policy.
func (e *Enforcer) removePolicy(sec string, ptype string, rule []string)
(bool, error) {
ok, err := e.logPolicyOperation(log.EventRemovePolicy, sec, rule,
func() (bool, error) {
- return e.removePolicyWithoutNotify(sec, ptype, rule)
+ return e.removePolicyWithoutNotify(e.shouldPersist, sec, ptype,
rule)
})
if !ok || err != nil {
@@ -446,7 +450,7 @@ func (e *Enforcer) removePolicy(sec string, ptype string,
rule []string) (bool,
}
func (e *Enforcer) updatePolicy(sec string, ptype string, oldRule []string,
newRule []string) (bool, error) {
- ok, err := e.updatePolicyWithoutNotify(sec, ptype, oldRule, newRule)
+ ok, err := e.updatePolicyWithoutNotify(e.shouldPersist, sec, ptype,
oldRule, newRule)
if !ok || err != nil {
return ok, err
}
@@ -465,7 +469,7 @@ 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) {
- ok, err := e.updatePoliciesWithoutNotify(sec, ptype, oldRules, newRules)
+ ok, err := e.updatePoliciesWithoutNotify(e.shouldPersist, sec, ptype,
oldRules, newRules)
if !ok || err != nil {
return ok, err
}
@@ -485,7 +489,7 @@ func (e *Enforcer) updatePolicies(sec string, ptype string,
oldRules [][]string,
// removePolicies removes rules from the current policy.
func (e *Enforcer) removePolicies(sec string, ptype string, rules [][]string)
(bool, error) {
- ok, err := e.removePoliciesWithoutNotify(sec, ptype, rules)
+ ok, err := e.removePoliciesWithoutNotify(e.shouldPersist, sec, ptype,
rules)
if !ok || err != nil {
return ok, err
}
@@ -505,7 +509,7 @@ func (e *Enforcer) removePolicies(sec string, ptype string,
rules [][]string) (b
// removeFilteredPolicy removes rules based on field filters from the current
policy.
func (e *Enforcer) removeFilteredPolicy(sec string, ptype string, fieldIndex
int, fieldValues []string) (bool, error) {
- ok, err := e.removeFilteredPolicyWithoutNotify(sec, ptype, fieldIndex,
fieldValues)
+ ok, err := e.removeFilteredPolicyWithoutNotify(e.shouldPersist, sec,
ptype, fieldIndex, fieldValues)
if !ok || err != nil {
return ok, err
}
diff --git a/management_api.go b/management_api.go
index 0491ee5..5279c9f 100644
--- a/management_api.go
+++ b/management_api.go
@@ -486,34 +486,52 @@ func (e *Enforcer) AddFunction(name string, function
govaluate.ExpressionFunctio
e.fm.AddFunction(name, function)
}
+// The Self* APIs apply a change that has already been made by another Casbin
+// instance to this instance's in-memory policy. They neither notify the
watcher
+// nor write to the adapter: the originating instance already persisted the
+// change, so persisting it again is at best a redundant write and at worst a
+// failure, e.g. a duplicate INSERT rejected by a unique constraint, which
would
+// leave this instance out of sync. Use AddPolicy() and friends to change both
+// the model and the storage.
+
+// SelfAddPolicy adds a rule to the current policy without notifying the
watcher or persisting it.
func (e *Enforcer) SelfAddPolicy(sec string, ptype string, rule []string)
(bool, error) {
- return e.addPolicyWithoutNotify(sec, ptype, rule)
+ return e.addPolicyWithoutNotify(nil, sec, ptype, rule)
}
+// SelfAddPolicies adds rules to the current policy without notifying the
watcher or persisting them.
func (e *Enforcer) SelfAddPolicies(sec string, ptype string, rules [][]string)
(bool, error) {
- return e.addPoliciesWithoutNotify(sec, ptype, rules, false)
+ return e.addPoliciesWithoutNotify(nil, sec, ptype, rules, false)
}
+// SelfAddPoliciesEx adds rules to the current policy without notifying the
watcher or persisting them,
+// existing rules are automatically filtered.
func (e *Enforcer) SelfAddPoliciesEx(sec string, ptype string, rules
[][]string) (bool, error) {
- return e.addPoliciesWithoutNotify(sec, ptype, rules, true)
+ return e.addPoliciesWithoutNotify(nil, sec, ptype, rules, true)
}
+// SelfRemovePolicy removes a rule from the current policy without notifying
the watcher or persisting it.
func (e *Enforcer) SelfRemovePolicy(sec string, ptype string, rule []string)
(bool, error) {
- return e.removePolicyWithoutNotify(sec, ptype, rule)
+ return e.removePolicyWithoutNotify(nil, sec, ptype, rule)
}
+// SelfRemovePolicies removes rules from the current policy without notifying
the watcher or persisting them.
func (e *Enforcer) SelfRemovePolicies(sec string, ptype string, rules
[][]string) (bool, error) {
- return e.removePoliciesWithoutNotify(sec, ptype, rules)
+ return e.removePoliciesWithoutNotify(nil, sec, ptype, rules)
}
+// SelfRemoveFilteredPolicy removes rules based on field filters from the
current policy
+// without notifying the watcher or persisting the change.
func (e *Enforcer) SelfRemoveFilteredPolicy(sec string, ptype string,
fieldIndex int, fieldValues ...string) (bool, error) {
- return e.removeFilteredPolicyWithoutNotify(sec, ptype, fieldIndex,
fieldValues)
+ return e.removeFilteredPolicyWithoutNotify(nil, sec, ptype, fieldIndex,
fieldValues)
}
+// SelfUpdatePolicy updates a rule in the current policy without notifying the
watcher or persisting it.
func (e *Enforcer) SelfUpdatePolicy(sec string, ptype string, oldRule, newRule
[]string) (bool, error) {
- return e.updatePolicyWithoutNotify(sec, ptype, oldRule, newRule)
+ return e.updatePolicyWithoutNotify(nil, sec, ptype, oldRule, newRule)
}
+// SelfUpdatePolicies updates rules in the current policy without notifying
the watcher or persisting them.
func (e *Enforcer) SelfUpdatePolicies(sec string, ptype string, oldRules,
newRules [][]string) (bool, error) {
- return e.updatePoliciesWithoutNotify(sec, ptype, oldRules, newRules)
+ return e.updatePoliciesWithoutNotify(nil, sec, ptype, oldRules,
newRules)
}
diff --git a/model/model_test.go b/model/model_test.go
index 1b3c8c1..3ba32a9 100644
--- a/model/model_test.go
+++ b/model/model_test.go
@@ -25,6 +25,8 @@ import (
"github.com/casbin/casbin/v3/config"
"github.com/casbin/casbin/v3/constant"
+ "github.com/casbin/casbin/v3/rbac"
+ defaultrolemanager
"github.com/casbin/casbin/v3/rbac/default-role-manager"
)
var (
@@ -162,3 +164,29 @@ func testModelToText(t *testing.T, mData, mExpected
string) {
}
}
}
+
+// TestModelCopyKeepsRoleManager is a regression test for
+// https://github.com/casbin/casbin/issues/1656 ("Undefined function g").
+//
+// Copy() must carry the role manager over to the new assertions. The enforcer
+// derives the g() matcher function from Assertion.RM, so a copy that loses it
+// makes the matcher fail to compile with "Undefined function g".
+func TestModelCopyKeepsRoleManager(t *testing.T) {
+ m, err := NewModelFromFile(filepath.Join("..", "examples",
"rbac_model.conf"))
+ if err != nil {
+ t.Fatalf("Failed to load model: %v", err)
+ }
+
+ rm := defaultrolemanager.NewRoleManagerImpl(10)
+ m["g"]["g"].RM = rm
+ m["g"]["g"].ParamsTokens = []string{"p_param"}
+
+ newModel := m.Copy()
+
+ if newModel["g"]["g"].RM != rbac.RoleManager(rm) {
+ t.Fatal("Copy() should carry the role manager over to the
copied assertion")
+ }
+ if len(newModel["g"]["g"].ParamsTokens) != 1 ||
newModel["g"]["g"].ParamsTokens[0] != "p_param" {
+ t.Fatalf("Copy() should carry ParamsTokens over, got %v",
newModel["g"]["g"].ParamsTokens)
+ }
+}
diff --git a/transaction_test.go b/transaction_test.go
index 555699f..9dcd518 100644
--- a/transaction_test.go
+++ b/transaction_test.go
@@ -338,3 +338,48 @@ func TestTransactionBuffer(t *testing.T) {
tx.Rollback()
}
+
+// TestTransactionKeepsRoleManagerAfterCommit is a regression test for
+// https://github.com/casbin/casbin/issues/1656 ("Undefined function g").
+//
+// Committing a transaction replaces the enforcer's model with a copy. Role
+// links are only rebuilt when the transaction touched the "g" section, so a
+// transaction that only changes "p" rules relies on the copy carrying over the
+// role manager. When Assertion.copy() dropped RM, the g() function disappeared
+// from the matcher's function map and Enforce() failed to even parse the
+// matcher, until LoadPolicy() was called by hand.
+func TestTransactionKeepsRoleManagerAfterCommit(t *testing.T) {
+ adapter := NewMockTransactionalAdapter()
+ e, err := NewTransactionalEnforcer("examples/rbac_model.conf", adapter)
+ if err != nil {
+ t.Fatalf("Failed to create transactional enforcer: %v", err)
+ }
+
+ if _, err := e.AddGroupingPolicy("alice", "admin"); err != nil {
+ t.Fatalf("Failed to add grouping policy: %v", err)
+ }
+
+ // A transaction that only touches the "p" section, so no role link
rebuild.
+ tx, err := e.BeginTransaction(context.Background())
+ if err != nil {
+ t.Fatalf("Failed to begin transaction: %v", err)
+ }
+ if _, err := tx.AddPolicy("admin", "data1", "read"); err != nil {
+ t.Fatalf("Failed to add policy in transaction: %v", err)
+ }
+ if err := tx.Commit(); err != nil {
+ t.Fatalf("Failed to commit transaction: %v", err)
+ }
+
+ if e.GetModel()["g"]["g"].RM == nil {
+ t.Fatal("The role manager should survive the model copy made on
commit")
+ }
+
+ ok, err := e.Enforce("alice", "data1", "read")
+ if err != nil {
+ t.Fatalf("Enforce failed after commit: %v", err)
+ }
+ if !ok {
+ t.Fatal("alice should inherit read access to data1 from the
admin role")
+ }
+}
diff --git a/watcher_self_sync_test.go b/watcher_self_sync_test.go
new file mode 100644
index 0000000..d70e079
--- /dev/null
+++ b/watcher_self_sync_test.go
@@ -0,0 +1,275 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you 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 (
+ "errors"
+ "strings"
+ "sync"
+ "testing"
+
+ "github.com/casbin/casbin/v3/model"
+ "github.com/casbin/casbin/v3/persist"
+)
+
+// uniqueConstraintAdapter mimics a SQL adapter whose policy table carries a
unique
+// index over (ptype, v0, v1, ...), i.e. what a user gets by adding a unique
index to
+// the casbin_rule table of gorm-adapter. Inserting a row that is already
stored fails
+// instead of silently succeeding.
+type uniqueConstraintAdapter struct {
+ mutex sync.Mutex
+ rows []string
+}
+
+var errDuplicateEntry = errors.New("Error 1062: Duplicate entry for key
'idx_casbin_rule'")
+
+func (a *uniqueConstraintAdapter) key(ptype string, rule []string) string {
+ return ptype + "," + strings.Join(rule, ",")
+}
+
+func (a *uniqueConstraintAdapter) LoadPolicy(m model.Model) error {
+ a.mutex.Lock()
+ defer a.mutex.Unlock()
+ for _, row := range a.rows {
+ if err := persist.LoadPolicyLine(row, m); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) SavePolicy(model.Model) error {
+ return errors.New("not implemented")
+}
+
+func (a *uniqueConstraintAdapter) AddPolicy(_ string, ptype string, rule
[]string) error {
+ a.mutex.Lock()
+ defer a.mutex.Unlock()
+ key := a.key(ptype, rule)
+ for _, row := range a.rows {
+ if row == key {
+ return errDuplicateEntry
+ }
+ }
+ a.rows = append(a.rows, key)
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) AddPolicies(sec string, ptype string, rules
[][]string) error {
+ for _, rule := range rules {
+ if err := a.AddPolicy(sec, ptype, rule); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) RemovePolicy(_ string, ptype string, rule
[]string) error {
+ a.mutex.Lock()
+ defer a.mutex.Unlock()
+ key := a.key(ptype, rule)
+ for i, row := range a.rows {
+ if row == key {
+ a.rows = append(a.rows[:i], a.rows[i+1:]...)
+ return nil
+ }
+ }
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) RemovePolicies(sec string, ptype string,
rules [][]string) error {
+ for _, rule := range rules {
+ if err := a.RemovePolicy(sec, ptype, rule); err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) RemoveFilteredPolicy(string, string, int,
...string) error {
+ return errors.New("not implemented")
+}
+
+func (a *uniqueConstraintAdapter) UpdatePolicy(sec string, ptype string,
oldRule, newRule []string) error {
+ if err := a.RemovePolicy(sec, ptype, oldRule); err != nil {
+ return err
+ }
+ return a.AddPolicy(sec, ptype, newRule)
+}
+
+func (a *uniqueConstraintAdapter) UpdatePolicies(sec string, ptype string,
oldRules, newRules [][]string) error {
+ for i := range oldRules {
+ if err := a.UpdatePolicy(sec, ptype, oldRules[i], newRules[i]);
err != nil {
+ return err
+ }
+ }
+ return nil
+}
+
+func (a *uniqueConstraintAdapter) UpdateFilteredPolicies(string, string,
[][]string, int, ...string) ([][]string, error) {
+ return nil, errors.New("not implemented")
+}
+
+func (a *uniqueConstraintAdapter) rowCount() int {
+ a.mutex.Lock()
+ defer a.mutex.Unlock()
+ return len(a.rows)
+}
+
+// newInstance builds an enforcer that shares the given storage, standing in
for one
+// process of a multi-instance deployment.
+func newInstance(t *testing.T, adapter *uniqueConstraintAdapter) *Enforcer {
+ t.Helper()
+ e, err := NewEnforcer("examples/rbac_model.conf", adapter)
+ if err != nil {
+ t.Fatal(err)
+ }
+ return e
+}
+
+// TestSelfAddPolicyDoesNotPersist covers
https://github.com/casbin/casbin/issues/1581:
+// instance A adds a rule and notifies the cluster, then instance B applies
the very
+// same rule through the watcher callback. B must not write the rule to the
shared
+// storage a second time, otherwise the unique index rejects the insert and B
silently
+// keeps an outdated model.
+func TestSelfAddPolicyDoesNotPersist(t *testing.T) {
+ adapter := &uniqueConstraintAdapter{}
+ instanceA := newInstance(t, adapter)
+ instanceB := newInstance(t, adapter)
+
+ if _, err := instanceA.AddPolicy("alice", "data1", "read"); err != nil {
+ t.Fatal(err)
+ }
+
+ ok, err := instanceB.SelfAddPolicy("p", "p", []string{"alice", "data1",
"read"})
+ if err != nil {
+ t.Fatalf("SelfAddPolicy should not hit the storage, got: %v",
err)
+ }
+ if !ok {
+ t.Fatal("SelfAddPolicy should report the rule as added")
+ }
+
+ if hasPolicy, err := instanceB.HasPolicy("alice", "data1", "read"); err
!= nil {
+ t.Fatal(err)
+ } else if !hasPolicy {
+ t.Fatal("the rule should be in the model of instance B")
+ }
+ if allowed, err := instanceB.Enforce("alice", "data1", "read"); err !=
nil {
+ t.Fatal(err)
+ } else if !allowed {
+ t.Fatal("instance B should enforce the rule it was notified
about")
+ }
+ if count := adapter.rowCount(); count != 1 {
+ t.Fatalf("the storage should hold exactly the row written by
instance A, got %d rows", count)
+ }
+}
+
+func TestSelfAddPoliciesDoesNotPersist(t *testing.T) {
+ adapter := &uniqueConstraintAdapter{}
+ instanceA := newInstance(t, adapter)
+ instanceB := newInstance(t, adapter)
+
+ rules := [][]string{{"alice", "data1", "read"}, {"bob", "data2",
"write"}}
+ if _, err := instanceA.AddPolicies(rules); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := instanceB.SelfAddPolicies("p", "p", rules); err != nil {
+ t.Fatalf("SelfAddPolicies should not hit the storage, got: %v",
err)
+ }
+ if _, err := instanceB.SelfAddPoliciesEx("p", "p", rules); err != nil {
+ t.Fatalf("SelfAddPoliciesEx should not hit the storage, got:
%v", err)
+ }
+
+ for _, rule := range rules {
+ if hasPolicy, err := instanceB.HasPolicy(rule); err != nil {
+ t.Fatal(err)
+ } else if !hasPolicy {
+ t.Fatalf("the rule %v should be in the model of
instance B", rule)
+ }
+ }
+ if count := adapter.rowCount(); count != 2 {
+ t.Fatalf("the storage should hold exactly the rows written by
instance A, got %d rows", count)
+ }
+}
+
+// TestSelfRemoveAndUpdateDoNotPersist makes sure the whole Self* family
leaves the
+// shared storage alone, not only the add path that issue #1581 reported.
+func TestSelfRemoveAndUpdateDoNotPersist(t *testing.T) {
+ adapter := &uniqueConstraintAdapter{}
+ instanceA := newInstance(t, adapter)
+ instanceB := newInstance(t, adapter)
+
+ if _, err := instanceA.AddPolicy("alice", "data1", "read"); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := instanceB.SelfAddPolicy("p", "p", []string{"alice",
"data1", "read"}); err != nil {
+ t.Fatal(err)
+ }
+
+ if _, err := instanceA.UpdatePolicy([]string{"alice", "data1", "read"},
[]string{"alice", "data1", "write"}); err != nil {
+ t.Fatal(err)
+ }
+ if _, err := instanceB.SelfUpdatePolicy("p", "p", []string{"alice",
"data1", "read"}, []string{"alice", "data1", "write"}); err != nil {
+ t.Fatal(err)
+ }
+ if count := adapter.rowCount(); count != 1 {
+ t.Fatalf("expected 1 row after the update, got %d", count)
+ }
+
+ if _, err := instanceA.RemovePolicy("alice", "data1", "write"); err !=
nil {
+ t.Fatal(err)
+ }
+ if _, err := instanceB.SelfRemovePolicy("p", "p", []string{"alice",
"data1", "write"}); err != nil {
+ t.Fatal(err)
+ }
+ if hasPolicy, err := instanceB.HasPolicy("alice", "data1", "write");
err != nil {
+ t.Fatal(err)
+ } else if hasPolicy {
+ t.Fatal("the rule should be gone from the model of instance B")
+ }
+ if count := adapter.rowCount(); count != 0 {
+ t.Fatalf("expected an empty storage, got %d rows", count)
+ }
+}
+
+// TestAddPolicyStillPersists guards the regular path: only the Self* APIs
skip the
+// adapter, AddPolicy() must keep writing through and keep reporting storage
errors.
+func TestAddPolicyStillPersists(t *testing.T) {
+ adapter := &uniqueConstraintAdapter{}
+ instanceA := newInstance(t, adapter)
+ instanceB := newInstance(t, adapter)
+
+ if _, err := instanceA.AddPolicy("alice", "data1", "read"); err != nil {
+ t.Fatal(err)
+ }
+ if count := adapter.rowCount(); count != 1 {
+ t.Fatalf("AddPolicy should have written 1 row, got %d", count)
+ }
+
+ // A rule that another instance already inserted is still a storage
error for the
+ // regular path, since the caller asked for the rule to be written.
+ if _, err := instanceB.AddPolicy("alice", "data1", "read");
!errors.Is(err, errDuplicateEntry) {
+ t.Fatalf("AddPolicy should surface the storage error, got: %v",
err)
+ }
+ if hasPolicy, err := instanceB.HasPolicy("alice", "data1", "read"); err
!= nil {
+ t.Fatal(err)
+ } else if hasPolicy {
+ t.Fatal("a rule that failed to persist must not end up in the
model")
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
