feat: integrate default detector into Enforcer (#1665)

Author: hsluoyz

detector/default_detector.go

@@ -68,8 +68,15 @@ func (d *DefaultDetector) Check(rm rbac.RoleManager) error {
 
 // buildGraph builds an adjacency list representation of the role inheritance graph.
 // It uses the Range method (via type assertion) to iterate through all role links.
-func (d *DefaultDetector) buildGraph(rm rbac.RoleManager) (map[string][]string, error) {
-	graph := make(map[string][]string)
+func (d *DefaultDetector) buildGraph(rm rbac.RoleManager) (graph map[string][]string, err error) {
+	graph = make(map[string][]string)
+
+	// Recover from any panics during Range iteration (e.g., nil pointer dereferences)
+	defer func() {
+		if r := recover(); r != nil {
+			err = fmt.Errorf("RoleManager is not properly initialized: %v", r)
+		}
+	}()
 
 	// Try to cast to a RoleManager implementation that supports Range
 	// This works with RoleManagerImpl and similar implementations

enforcer.go

@@ -21,6 +21,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/casbin/casbin/v3/detector"
 	"github.com/casbin/casbin/v3/effector"
 	"github.com/casbin/casbin/v3/log"
 	"github.com/casbin/casbin/v3/model"
@@ -47,6 +48,7 @@ type Enforcer struct {
 	condRmMap  map[string]rbac.ConditionalRoleManager
 	matcherMap sync.Map
 	logger     log.Logger
+	detectors  []detector.Detector
 
 	enabled              bool
 	autoSave             bool
@@ -190,6 +192,11 @@ func (e *Enforcer) initialize() {
 	e.autoNotifyWatcher = true
 	e.autoNotifyDispatcher = true
 	e.initRmMap()
+
+	// Initialize detectors with default detector if not already set
+	if e.detectors == nil {
+		e.detectors = []detector.Detector{detector.NewDefaultDetector()}
+	}
 }
 
 // LoadModel reloads the model from the model CONF file.
@@ -288,6 +295,57 @@ func (e *Enforcer) SetLogger(logger log.Logger) {
 	e.logger = logger
 }
 
+// SetDetector sets a single detector for the enforcer.
+func (e *Enforcer) SetDetector(d detector.Detector) {
+	e.detectors = []detector.Detector{d}
+}
+
+// SetDetectors sets multiple detectors for the enforcer.
+func (e *Enforcer) SetDetectors(detectors []detector.Detector) {
+	e.detectors = detectors
+}
+
+// RunDetections runs all detectors on all role managers.
+// Returns the first error encountered, or nil if all checks pass.
+// Silently skips role managers that don't support the required iteration methods.
+func (e *Enforcer) RunDetections() error {
+	if e.detectors == nil || len(e.detectors) == 0 {
+		return nil
+	}
+
+	// Run detectors on all role managers
+	for _, rm := range e.rmMap {
+		for _, d := range e.detectors {
+			err := d.Check(rm)
+			// Skip if the role manager doesn't support the required iteration or is not initialized
+			if err != nil && (strings.Contains(err.Error(), "does not support Range iteration") ||
+				strings.Contains(err.Error(), "not properly initialized")) {
+				continue
+			}
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	// Run detectors on all conditional role managers
+	for _, crm := range e.condRmMap {
+		for _, d := range e.detectors {
+			err := d.Check(crm)
+			// Skip if the role manager doesn't support the required iteration or is not initialized
+			if err != nil && (strings.Contains(err.Error(), "does not support Range iteration") ||
+				strings.Contains(err.Error(), "not properly initialized")) {
+				continue
+			}
+			if err != nil {
+				return err
+			}
+		}
+	}
+
+	return nil
+}
+
 // ClearPolicy clears all policy.
 func (e *Enforcer) ClearPolicy() {
 	e.invalidateMatcherMap()
@@ -316,6 +374,12 @@ func (e *Enforcer) LoadPolicy() error {
 
 	e.onLogAfterEventInLoadPolicy(logEntry, newModel)
 
+	// Run detectors after all policy rules are loaded
+	err = e.RunDetections()
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 

enforcer_test.go

@@ -15,9 +15,11 @@
 package casbin
 
 import (
+	"strings"
 	"sync"
 	"testing"
 
+	"github.com/casbin/casbin/v3/detector"
 	"github.com/casbin/casbin/v3/model"
 	fileadapter "github.com/casbin/casbin/v3/persist/file-adapter"
 	"github.com/casbin/casbin/v3/util"
@@ -724,3 +726,78 @@ func TestLinkConditionFunc(t *testing.T) {
 	testDomainEnforce(t, e, "alice", "domain5", "data5", "read", false)
 	testDomainEnforce(t, e, "alice", "domain5", "data5", "write", false)
 }
+
+func TestEnforcerWithDefaultDetector(t *testing.T) {
+	// Test that default detector is enabled and detects cycles
+	_, err := NewEnforcer("examples/rbac_model.conf", "examples/rbac_with_cycle_policy.csv")
+
+	// Expect an error because the policy contains a cycle
+	if err == nil {
+		t.Error("Expected cycle detection error when loading policy with cycle, but got nil")
+	} else {
+		errMsg := err.Error()
+		if !strings.Contains(errMsg, "cycle detected") {
+			t.Errorf("Expected error message to contain 'cycle detected', got: %s", errMsg)
+		}
+	}
+}
+
+func TestEnforcerRunDetections(t *testing.T) {
+	// Test explicit RunDetections() call
+	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+
+	// Should not error on valid policy
+	err := e.RunDetections()
+	if err != nil {
+		t.Errorf("Expected no error when running detections on valid policy, but got: %v", err)
+	}
+
+	// Now add a cycle manually
+	_, _ = e.AddGroupingPolicy("alice", "data2_admin")
+	_, _ = e.AddGroupingPolicy("data2_admin", "super_admin")
+	_, _ = e.AddGroupingPolicy("super_admin", "alice")
+
+	// Should detect the cycle
+	err = e.RunDetections()
+	if err == nil {
+		t.Error("Expected cycle detection error, but got nil")
+	} else {
+		errMsg := err.Error()
+		if !strings.Contains(errMsg, "cycle detected") {
+			t.Errorf("Expected error message to contain 'cycle detected', got: %s", errMsg)
+		}
+	}
+}
+
+func TestEnforcerSetDetector(t *testing.T) {
+	// Test SetDetector() method
+	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+
+	// Create a custom detector
+	customDetector := detector.NewDefaultDetector()
+	e.SetDetector(customDetector)
+
+	// Should still work with custom detector
+	err := e.RunDetections()
+	if err != nil {
+		t.Errorf("Expected no error with custom detector, but got: %v", err)
+	}
+}
+
+func TestEnforcerSetDetectors(t *testing.T) {
+	// Test SetDetectors() method
+	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
+
+	// Create multiple detectors
+	detectors := []detector.Detector{
+		detector.NewDefaultDetector(),
+		detector.NewDefaultDetector(),
+	}
+	e.SetDetectors(detectors)
+
+	// Should work with multiple detectors
+	err := e.RunDetections()
+	if err != nil {
+		t.Errorf("Expected no error with multiple detectors, but got: %v", err)
+	}
+}

examples/rbac_with_cycle_policy.csv

@@ -0,0 +1,7 @@
+p, alice, data1, read
+p, bob, data2, write
+p, data2_admin, data2, read
+p, data2_admin, data2, write
+g, alice, data2_admin
+g, data2_admin, super_admin
+g, super_admin, alice