feat: completely remove domain from RoleManager when calling DeleteDomains (#1496)

Author: D0000M

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())

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
 }

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.

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
 }

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...)
+}

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{})
+}