diff --git a/casbin/rbac/default_role_manager/role_manager.py b/casbin/rbac/default_role_manager/role_manager.py index 5ea0605b..038f1cdf 100644 --- a/casbin/rbac/default_role_manager/role_manager.py +++ b/casbin/rbac/default_role_manager/role_manager.py @@ -133,7 +133,7 @@ def _matching_roles(self, name, match_order=MatchOrder.STR_PATTERN): def _get_role(self, name): if name not in self.all_roles: role = Role(name) - if self.matching_func != None: + if self.matching_func is not None: for pattern_role in self._matching_roles(name): role.copy_from(pattern_role) self.all_roles[name] = role @@ -158,7 +158,7 @@ def add_link(self, name1, name2, *domain): user.add_role(role) - if self.matching_func != None: + if self.matching_func is not None: for r in self.all_roles.values(): if r.name != user.name and self._matching_fn(user.name, r.name, MatchOrder.PATTERN_STR): r.add_role(role) @@ -174,11 +174,12 @@ def delete_link(self, name1, name2, *domain): role = self._get_role(name2) user.remove_role(role) - for r in self.all_roles.values(): - if r.name != user.name and self._matching_fn(user.name, r.name, MatchOrder.PATTERN_STR): - r.remove_role(role) - if r.name != role.name and self._matching_fn(role.name, r.name, MatchOrder.PATTERN_STR): - role.remove_role(r) + if self.matching_func is not None: + for r in self.all_roles.values(): + if r.name != user.name and self._matching_fn(user.name, r.name, MatchOrder.PATTERN_STR): + r.remove_role(role) + if r.name != role.name and self._matching_fn(role.name, r.name, MatchOrder.PATTERN_STR): + role.remove_role(r) def has_link(self, name1, name2, *domain): user = self._get_role(name1) @@ -255,7 +256,7 @@ def _get_role_manager(self, *domain): domain1 = self._get_domain(*domain) domain_links = self.all_links.get(domain1, []) - if self.domain_matching_func != None: + if self.domain_matching_func is not None: for domain2, links in self.all_links.items(): if domain1 != domain2 and match_error_handler(self.domain_matching_func, domain1, domain2): domain_links = domain_links + links @@ -313,7 +314,7 @@ def _get_role_manager(self, *domain): def _affected_role_managers(self, *domain): domain_pattern = self._get_domain(*domain) - if self.domain_matching_func != None: + if self.domain_matching_func is not None: return [ self.rm_map[domain_str] for domain_str in self.rm_map.keys() diff --git a/tests/rbac/test_role_manager.py b/tests/rbac/test_role_manager.py index 40c2c1f0..e9ee2c3a 100644 --- a/tests/rbac/test_role_manager.py +++ b/tests/rbac/test_role_manager.py @@ -14,7 +14,7 @@ from unittest import TestCase from casbin.rbac import default_role_manager -from casbin.util import regex_match_func +from casbin.util import key_match, regex_match_func import time from concurrent.futures import ThreadPoolExecutor import re @@ -261,6 +261,23 @@ def test_has_link(role): for future in futures: self.assertTrue(future.result()) + def test_delete_link_without_matching_func(self): + rm = self.get_role_manager() + rm.add_matching_func(None) + rm.add_link("u*", "g1") + rm.add_link("u1", "g2") + rm.add_link("g1", "g*") + self.assertFalse(rm.has_link("u1", "g1")) + + rm.delete_link("u*", "g1") + + self.assertFalse(rm.has_link("u*", "g1")) + self.assertFalse(rm.has_link("u*", "g*")) + self.assertTrue(rm.has_link("u1", "g2")) + self.assertTrue(rm.has_link("g1", "g*")) + self.assertEqual(rm.get_roles("u*"), []) + self.assertEqual(rm.get_users("g1"), []) + class TestDomainManager(TestRoleManager): def get_role_manager(self): @@ -313,3 +330,20 @@ def test_domain_role(self): self.assertTrue(rm.has_link("alice", "users", "domain1")) self.assertTrue(rm.has_link("alice", "user", "domain2")) self.assertFalse(rm.has_link("alice", "users", "domain2")) + + def test_delete_link_without_matching_func_across_domains(self): + rm = self.get_role_manager() + rm.add_matching_func(None) + rm.add_domain_matching_func(key_match) + rm.add_link("alice", "admin", "*") + rm.add_link("bob", "admin", "*") + domains = ["d1", "d2", "d3"] + for d in domains: + self.assertTrue(rm.has_link("alice", "admin", d)) + + rm.delete_link("alice", "admin", "*") + + for d in domains: + self.assertFalse(rm.has_link("alice", "admin", d)) + self.assertTrue(rm.has_link("bob", "admin", d)) + self.assertFalse(rm.has_link("alice", "admin", "d4"))