Repository: casbin/casbin
Pull request #1496 (2025-07-05 06:29:35)

feat: completely remove domain from RoleManager when calling DeleteDomains
close #1492 
The original `DeleteDomains()` only performed the task of `DeleteAllUsersByDomain()`, but now `DeleteDomains() ` will delete all associated policies as well.










hsluoyz: @tangyang9464 plz review
tangyang9464: @D0000M 
I looked through the historical commits ((#784 #790 )) of the `DeleteDomains`-related APIs and  found that their function is to delete both the `g` and `p` policies (the comments and documentation of the related APIs are a bit vague, and they describe it as `would delete all associated users and roles`). 

The key problem of this issue #1492 is that `DomainRoleManager` is a later implementation that does not take into account the `DeleteDomains`-related APIs. 
Therefore, I think this fix should only require adding the deletion of `RoleManager` in the `DeleteDomains` and other APIs, and there is no need to modify the deletion logic of `DeleteDomains` for the policy, because it is correct.

Patch:
diff --git a/rbac/default-role-manager/role_manager.go b/rbac/default-role-manager/role_manager.go
index ee1ee3f..9fc03b9 100644
--- a/rbac/default-role-manager/role_manager.go
+++ b/rbac/default-role-manager/role_manager.go
@@ -15,6 +15,7 @@
 package defaultrolemanager
 
 import (
+	"errors"
 	"fmt"
 	"strings"
 	"sync"
@@ -717,6 +718,12 @@ func (dm *DomainManager) BuildRelationship(name1 string, name2 string, domain ..
 	return nil
 }
 
+// DeleteDomain deletes the specified domain from DomainManager.
+func (dm *DomainManager) DeleteDomain(domain string) error {
+	dm.rmMap.Delete(domain)
+	return nil
+}
+
 type RoleManager struct {
 	*DomainManager
 }
@@ -727,6 +734,11 @@ func NewRoleManager(maxHierarchyLevel int) *RoleManager {
 	return rm
 }
 
+// DeleteDomain does nothing for RoleManagerImpl (no domain concept).
+func (rm *RoleManagerImpl) DeleteDomain(domain string) error {
+	return errors.New("DeleteDomain is not supported by RoleManagerImpl (no domain concept)")
+}
+
 type ConditionalRoleManager struct {
 	RoleManagerImpl
 }
diff --git a/rbac/role_manager.go b/rbac/role_manager.go
index 28b40a3..bf7cc75 100644
--- a/rbac/role_manager.go
+++ b/rbac/role_manager.go
@@ -55,6 +55,8 @@ type RoleManager interface {
 	AddMatchingFunc(name string, fn MatchingFunc)
 	// AddDomainMatchingFunc adds the domain matching function
 	AddDomainMatchingFunc(name string, fn MatchingFunc)
+	// DeleteDomain deletes all data of a domain in the role manager.
+	DeleteDomain(domain string) error
 }
 
 // ConditionalRoleManager provides interface to define the operations for managing roles.
diff --git a/rbac_api_with_domains.go b/rbac_api_with_domains.go
index f6fc4a2..9641b6a 100644
--- a/rbac_api_with_domains.go
+++ b/rbac_api_with_domains.go
@@ -144,17 +144,26 @@ func (e *Enforcer) DeleteAllUsersByDomain(domain string) (bool, error) {
 	return true, nil
 }
 
-// DeleteDomains would delete all associated users and roles.
+// DeleteDomains would delete all associated policies.
 // It would delete all domains if parameter is not provided.
 func (e *Enforcer) DeleteDomains(domains ...string) (bool, error) {
 	if len(domains) == 0 {
-		e.ClearPolicy()
-		return true, nil
+		var err error
+		domains, err = e.GetAllDomains()
+		if err != nil {
+			return false, err
+		}
 	}
 	for _, domain := range domains {
 		if _, err := e.DeleteAllUsersByDomain(domain); err != nil {
 			return false, err
 		}
+		// remove the domain from the RoleManager.
+		if e.GetRoleManager() != nil {
+			if err := e.GetRoleManager().DeleteDomain(domain); err != nil {
+				return false, err
+			}
+		}
 	}
 	return true, nil
 }
diff --git a/rbac_api_with_domains_synced.go b/rbac_api_with_domains_synced.go
index 26f6ce4..02fadb1 100644
--- a/rbac_api_with_domains_synced.go
+++ b/rbac_api_with_domains_synced.go
@@ -58,3 +58,11 @@ func (e *SyncedEnforcer) DeleteRolesForUserInDomain(user string, domain string)
 	defer e.m.Unlock()
 	return e.Enforcer.DeleteRolesForUserInDomain(user, domain)
 }
+
+// DeleteDomains deletes domains from the model.
+// Returns false if the domain does not exist (aka not affected).
+func (e *SyncedEnforcer) DeleteDomains(domains ...string) (bool, error) {
+	e.m.Lock()
+	defer e.m.Unlock()
+	return e.Enforcer.DeleteDomains(domains...)
+}


Test patch:
diff --git a/model_test.go b/model_test.go
index 9bb1e9e..9075217 100644
--- a/model_test.go
+++ b/model_test.go
@@ -411,6 +411,10 @@ func (rm *testCustomRoleManager) AddDomainLinkConditionFunc(user string, role st
 func (rm *testCustomRoleManager) SetDomainLinkConditionFuncParams(user string, role string, domain string, params ...string) {
 }
 
+func (rm *testCustomRoleManager) DeleteDomain(domain string) error {
+	return nil
+}
+
 func TestRBACModelWithCustomRoleManager(t *testing.T) {
 	e, _ := NewEnforcer("examples/rbac_model.conf", "examples/rbac_policy.csv")
 	e.SetRoleManager(NewRoleManager())
diff --git a/rbac_api_with_domains_test.go b/rbac_api_with_domains_test.go
index 963415d..9ea8606 100644
--- a/rbac_api_with_domains_test.go
+++ b/rbac_api_with_domains_test.go
@@ -296,3 +296,47 @@ func TestGetAllRolesByDomain(t *testing.T) {
 	testGetAllRolesByDomain(t, e, "domain2", []string{"admin"})
 	testGetAllRolesByDomain(t, e, "domain3", []string{"user"})
 }
+
+func testDeleteDomains(t *testing.T, domains []string, expectedPolicy, expectedGroupingPolicy [][]string, expectedDomains []string) {
+	e, _ := NewEnforcer("examples/rbac_with_domains_model.conf", "examples/rbac_with_domains_policy.csv")
+
+	_, _ = e.DeleteDomains(domains...)
+	policy, err := e.GetPolicy()
+	if err != nil {
+		t.Error(err)
+	}
+	if !util.Array2DEquals(policy, expectedPolicy) {
+		t.Errorf("policy after deleting domains %v: %v, supposed to be %v\n", domains, policy, expectedPolicy)
+	}
+
+	policies, err := e.GetGroupingPolicy()
+	if err != nil {
+		t.Error(err)
+	}
+	if !util.Array2DEquals(policies, expectedGroupingPolicy) {
+		t.Errorf("grouping policy after deleting domains %v: %v, supposed to be %v\n", domains, policies, expectedGroupingPolicy)
+	}
+
+	domainsAfterRemoval, _ := e.GetAllDomains()
+	if !util.SetEquals(domainsAfterRemoval, expectedDomains) {
+		t.Errorf("domains after deleting %v: %v, supposed to be %v\n", domains, domainsAfterRemoval, expectedDomains)
+	}
+}
+
+func TestDeleteDomains(t *testing.T) {
+	testDeleteDomains(t, []string{"domain1"}, [][]string{
+		{"admin", "domain2", "data2", "read"},
+		{"admin", "domain2", "data2", "write"},
+	}, [][]string{
+		{"bob", "admin", "domain2"},
+	}, []string{"domain2"})
+
+	testDeleteDomains(t, []string{"domain2"}, [][]string{
+		{"admin", "domain1", "data1", "read"},
+		{"admin", "domain1", "data1", "write"},
+	}, [][]string{
+		{"alice", "admin", "domain1"},
+	}, []string{"domain1"})
+
+	testDeleteDomains(t, []string{}, [][]string{}, [][]string{}, []string{})
+}