From a7e93f5b600a2a305f5132f3b19edb8fbcf445cb Mon Sep 17 00:00:00 2001 From: vlieven Date: Thu, 24 Sep 2026 10:12:18 +0200 Subject: [PATCH 1/3] fix: skip pattern matching in delete_link when no matching func is set RoleManager.add_link only walks all_roles to apply pattern-derived links when a matching function is set, but delete_link walked it unconditionally. With matching_func = None, every iteration called match_error_handler(None, ...), which raised and swallowed a TypeError, so the result was correct but each delete was O(roles) wasted work. The cost multiplies with a domain matching function on g (e.g. `g, user, group, *` with key_match), because DomainManager forwards the delete to every cached domain's role manager. Guard the loop with the same check add_link uses. ConditionalRoleManager, DomainManager and ConditionalDomainManager all delegate to this method, so they are covered too. Add tests that check delete_link behavior without a matching function, both on its own and across cached domains. --- .../rbac/default_role_manager/role_manager.py | 13 +++---- tests/rbac/test_role_manager.py | 36 ++++++++++++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/casbin/rbac/default_role_manager/role_manager.py b/casbin/rbac/default_role_manager/role_manager.py index 5ea0605b..92cb8c16 100644 --- a/casbin/rbac/default_role_manager/role_manager.py +++ b/casbin/rbac/default_role_manager/role_manager.py @@ -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) diff --git a/tests/rbac/test_role_manager.py b/tests/rbac/test_role_manager.py index 40c2c1f0..9e366ed7 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")) \ No newline at end of file From 79e73838e80b8b528ea18292f9f3da8e4c9d19e2 Mon Sep 17 00:00:00 2001 From: vlieven Date: Thu, 24 Sep 2026 10:12:18 +0200 Subject: [PATCH 2/3] fix: skip pattern matching in delete_link when no matching func is set RoleManager.add_link only walks all_roles to apply pattern-derived links when a matching function is set, but delete_link walked it unconditionally. With matching_func = None, every iteration called match_error_handler(None, ...), which raised and swallowed a TypeError, so the result was correct but each delete was O(roles) wasted work. The cost multiplies with a domain matching function on g (e.g. `g, user, group, *` with key_match), because DomainManager forwards the delete to every cached domain's role manager. Guard the loop with the same check add_link uses. ConditionalRoleManager, DomainManager and ConditionalDomainManager all delegate to this method, so they are covered too. Add tests that check delete_link behavior without a matching function, both on its own and across cached domains. --- casbin/rbac/default_role_manager/role_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/casbin/rbac/default_role_manager/role_manager.py b/casbin/rbac/default_role_manager/role_manager.py index 92cb8c16..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 @@ -256,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 @@ -314,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() From b9daa124bdcf7c267e2d7e9161b16bdea6c0c4e7 Mon Sep 17 00:00:00 2001 From: vlieven Date: Thu, 24 Sep 2026 10:30:45 +0200 Subject: [PATCH 3/3] ensure file ends with a newline in test_role_manager.py --- tests/rbac/test_role_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rbac/test_role_manager.py b/tests/rbac/test_role_manager.py index 9e366ed7..e9ee2c3a 100644 --- a/tests/rbac/test_role_manager.py +++ b/tests/rbac/test_role_manager.py @@ -346,4 +346,4 @@ def test_delete_link_without_matching_func_across_domains(self): 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")) \ No newline at end of file + self.assertFalse(rm.has_link("alice", "admin", "d4"))