This is an automated email from the ASF dual-hosted git repository. tisonkun pushed a commit to branch fix/conditional-domain-rebuild in repository https://gitbox.apache.org/repos/asf/casbin-jcasbin.git
commit b18bfe5de72476d58613d2b01af02379811575ec Author: tison <[email protected]> AuthorDate: Mon Oct 26 13:58:08 2026 +0800 fix: derive conditional domain views from explicit links --- .../jcasbin/rbac/ConditionalDomainManager.java | 89 +++++++++------------- .../org/casbin/jcasbin/main/ModelUnitTest.java | 53 +++++++++++++ .../casbin/jcasbin/main/RoleManagerUnitTest.java | 37 +++++++++ 3 files changed, 126 insertions(+), 53 deletions(-) diff --git a/src/main/java/org/casbin/jcasbin/rbac/ConditionalDomainManager.java b/src/main/java/org/casbin/jcasbin/rbac/ConditionalDomainManager.java index 30e3b4f..f9e1086 100644 --- a/src/main/java/org/casbin/jcasbin/rbac/ConditionalDomainManager.java +++ b/src/main/java/org/casbin/jcasbin/rbac/ConditionalDomainManager.java @@ -29,7 +29,9 @@ import java.util.function.Function; public class ConditionalDomainManager extends ConditionalRoleManager { private static final String DEFAULT_DOMAIN = "casbin::default"; + // Keep explicit links separate from the domain views built by matching. private Map<String, ConditionalRoleManager> rmMap; + private Map<String, ConditionalRoleManager> matchedRmMap; private SyncedLRUCache<String, Boolean> domainMatchingFuncCache; public ConditionalDomainManager(int maxHierarchyLevel) { @@ -40,53 +42,43 @@ public class ConditionalDomainManager extends ConditionalRoleManager { public void addMatchingFunc(String name, BiPredicate<String, String> matchingFunc) { this.matchingFunc = matchingFunc; this.rmMap.values().forEach(rm -> rm.addMatchingFunc(name, matchingFunc)); + this.matchedRmMap.clear(); } @Override public void addDomainMatchingFunc(String name, BiPredicate<String, String> domainMatchingFunc) { this.domainMatchingFunc = domainMatchingFunc; - rebuild(); - } - - private void rebuild() { - Map<String, ConditionalRoleManager> rmMap = new HashMap<>(this.rmMap); - clear(); - rmMap.forEach((domain, rm) -> { - ConditionalRoleManager newRm = new ConditionalRoleManager(this.maxHierarchyLevel, this.matchingFunc, this.domainMatchingFunc); - newRm.copyFrom(rm); - this.rmMap.put(domain, newRm); - }); - // Copy complete links from the snapshot so propagated links retain their conditions. - if (this.domainMatchingFunc != null) { - this.rmMap.forEach((domain, rm) -> { - rmMap.forEach((pattern, source) -> { - if (!domain.equals(pattern) && matchDomain(domain, pattern)) { - rm.copyFrom(source); - } - }); - }); - } + this.rmMap.values().forEach(rm -> rm.domainMatchingFunc = domainMatchingFunc); + this.domainMatchingFuncCache = new SyncedLRUCache<>(100); + this.matchedRmMap.clear(); } private String domainName(String... domain) { return domain.length == 0 ? DEFAULT_DOMAIN : domain[0]; } - private ConditionalRoleManager getRoleManager(String domain, boolean store) { + private ConditionalRoleManager getRoleManager(String domain) { ConditionalRoleManager rm = this.rmMap.get(domain); if (rm == null) { rm = new ConditionalRoleManager(this.maxHierarchyLevel, this.matchingFunc, this.domainMatchingFunc); - if (store) { - this.rmMap.put(domain, rm); - } - if (this.domainMatchingFunc != null) { - for (Map.Entry<String, ConditionalRoleManager> entry : this.rmMap.entrySet()) { - String domain2 = entry.getKey(); - if (!domain.equals(domain2) && matchDomain(domain, domain2)) { - rm.copyFrom(entry.getValue()); - } + this.rmMap.put(domain, rm); + } + return rm; + } + + private ConditionalRoleManager getMatchedRoleManager(String domain) { + ConditionalRoleManager rm = this.matchedRmMap.get(domain); + if (rm == null) { + rm = new ConditionalRoleManager(this.maxHierarchyLevel, this.matchingFunc, this.domainMatchingFunc); + for (Map.Entry<String, ConditionalRoleManager> entry : this.rmMap.entrySet()) { + if (domain.equals(entry.getKey()) || matchDomain(domain, entry.getKey())) { + rm.copyFrom(entry.getValue()); } } + // Do not retain views for arbitrary domains supplied by requests. + if (this.rmMap.containsKey(domain)) { + this.matchedRmMap.put(domain, rm); + } } return rm; } @@ -109,71 +101,62 @@ public class ConditionalDomainManager extends ConditionalRoleManager { public void clear() { super.clear(); this.rmMap = new HashMap<>(); + this.matchedRmMap = new HashMap<>(); this.domainMatchingFuncCache = new SyncedLRUCache<>(100); } @Override public void addLink(String name1, String name2, String... domain) { String domainName = domainName(domain); - getRoleManager(domainName, true).addLink(name1, name2, domain); - - if (this.domainMatchingFunc != null) { - this.rmMap.forEach((domain2, rm) -> { - if (!domainName.equals(domain2) && matchDomain(domain2, domainName)) { - rm.addLink(name1, name2, domain); - } - }); - } + getRoleManager(domainName).addLink(name1, name2, domain); + this.matchedRmMap.clear(); } @Override public void deleteLink(String name1, String name2, String... domain) { String domainName = domainName(domain); - getRoleManager(domainName, true).deleteLink(name1, name2, domain); - - if (this.domainMatchingFunc != null) { - this.rmMap.forEach((domain2, rm) -> { - if (!domainName.equals(domain2) && matchDomain(domain2, domainName)) { - rm.deleteLink(name1, name2, domain); - } - }); - } + getRoleManager(domainName).deleteLink(name1, name2, domain); + this.matchedRmMap.clear(); } @Override public boolean hasLink(String name1, String name2, String... domain) { - return getRoleManager(domainName(domain), false).hasLink(name1, name2, domain); + return getMatchedRoleManager(domainName(domain)).hasLink(name1, name2, domain); } @Override public List<String> getRoles(String name, String... domain) { - return getRoleManager(domainName(domain), false).getRoles(name, domain); + return getMatchedRoleManager(domainName(domain)).getRoles(name, domain); } @Override public List<String> getUsers(String name, String... domain) { - return getRoleManager(domainName(domain), false).getUsers(name, domain); + return getMatchedRoleManager(domainName(domain)).getUsers(name, domain); } @Override public void addLinkConditionFunc(String userName, String roleName, Function<String[], Boolean> fn) { this.rmMap.values().forEach(rm -> rm.addLinkConditionFunc(userName, roleName, fn)); + this.matchedRmMap.clear(); } @Override public void addDomainLinkConditionFunc(String userName, String roleName, String domain, Function<String[], Boolean> fn) { - getRoleManager(domain, true); + getRoleManager(domain); this.rmMap.values().forEach(rm -> rm.addDomainLinkConditionFunc(userName, roleName, domain, fn)); + this.matchedRmMap.clear(); } @Override public void setLinkConditionFuncParams(String userName, String roleName, String... params) { this.rmMap.values().forEach(rm -> rm.setLinkConditionFuncParams(userName, roleName, params)); + this.matchedRmMap.clear(); } @Override public void setDomainLinkConditionFuncParams(String userName, String roleName, String domain, String... params) { this.rmMap.values().forEach(rm -> rm.setDomainLinkConditionFuncParams(userName, roleName, domain, params)); + this.matchedRmMap.clear(); } @Override diff --git a/src/test/java/org/casbin/jcasbin/main/ModelUnitTest.java b/src/test/java/org/casbin/jcasbin/main/ModelUnitTest.java index 5f75594..eb2afd8 100644 --- a/src/test/java/org/casbin/jcasbin/main/ModelUnitTest.java +++ b/src/test/java/org/casbin/jcasbin/main/ModelUnitTest.java @@ -325,6 +325,59 @@ public class ModelUnitTest { assertFalse(e.enforce("bob1", "tenantA", "data", "read")); } + @DataProvider(name = "conditionalDomainReconfiguration") + public Object[][] conditionalDomainReconfiguration() { + return new Object[][] {{false, false}, {false, true}, {true, false}, {true, true}}; + } + + @Test(dataProvider = "conditionalDomainReconfiguration") + public void testConditionalDomainMatchingCanBeNarrowed(boolean matchBeforeLinks, boolean existingDomain) { + Model m = new Model(); + m.addDef("r", "r", "sub, dom, obj, act"); + m.addDef("p", "p", "sub, dom, obj, act"); + m.addDef("g", "g", "_, _, _, (_, _)"); + m.addDef("e", "e", "some(where (p.eft == allow))"); + m.addDef("m", "m", "g(r.sub, p.sub, r.dom) && r.dom == p.dom && r.obj == p.obj && r.act == p.act"); + Enforcer e = new Enforcer(m); + e.addPolicy("admin", "tenantA", "data", "read"); + e.addPolicy("admin", "tenantB", "data", "read"); + e.addPolicy("admin", "tenant*", "data", "read"); + if (matchBeforeLinks) { + e.addNamedDomainMatchingFunc("g", "keyMatch", BuiltInFunctions::keyMatch); + } + if (existingDomain) { + e.addGroupingPolicy("bob", "admin", "tenantA", "_", "_"); + } + e.addGroupingPolicy("alice", "admin", "tenant*", "deny", "_"); + e.addNamedDomainLinkConditionFunc("g", "alice", "admin", "tenant*", params -> "allow".equals(params[0])); + e.setNamedDomainLinkConditionFuncParams("g", "alice", "admin", "tenant*", "deny", "_"); + if (!matchBeforeLinks) { + e.addNamedDomainMatchingFunc("g", "keyMatch", BuiltInFunctions::keyMatch); + } + assertFalse(e.enforce("alice", "tenantA", "data", "read")); + assertFalse(e.enforce("alice", "tenantB", "data", "read")); + + e.addNamedDomainMatchingFunc("g", "exact", String::equals); + assertFalse(e.enforce("alice", "tenantA", "data", "read")); + assertFalse(e.enforce("alice", "tenantB", "data", "read")); + e.setNamedDomainLinkConditionFuncParams("g", "alice", "admin", "tenant*", "allow", "_"); + assertTrue(e.enforce("alice", "tenant*", "data", "read")); + assertFalse(e.enforce("alice", "tenantA", "data", "read")); + + e.addNamedDomainMatchingFunc("g", "keyMatch", BuiltInFunctions::keyMatch); + assertTrue(e.enforce("alice", "tenantA", "data", "read")); + assertTrue(e.enforce("alice", "tenantB", "data", "read")); + e.setNamedDomainLinkConditionFuncParams("g", "alice", "admin", "tenant*", "deny", "_"); + assertFalse(e.enforce("alice", "tenantA", "data", "read")); + assertFalse(e.enforce("alice", "tenantB", "data", "read")); + e.setNamedDomainLinkConditionFuncParams("g", "alice", "admin", "tenant*", "allow", "_"); + assertTrue(e.enforce("alice", "tenantA", "data", "read")); + assertTrue(e.removeGroupingPolicy("alice", "admin", "tenant*", "deny", "_")); + assertFalse(e.enforce("alice", "tenantA", "data", "read")); + assertFalse(e.enforce("alice", "tenantB", "data", "read")); + assertEquals(e.enforce("bob", "tenantA", "data", "read"), existingDomain); + } + @Test public void testRBACModelWithDomainsAtRuntime() { Enforcer e = new Enforcer("examples/rbac_with_domains_model.conf"); diff --git a/src/test/java/org/casbin/jcasbin/main/RoleManagerUnitTest.java b/src/test/java/org/casbin/jcasbin/main/RoleManagerUnitTest.java index ab97293..53de5ea 100644 --- a/src/test/java/org/casbin/jcasbin/main/RoleManagerUnitTest.java +++ b/src/test/java/org/casbin/jcasbin/main/RoleManagerUnitTest.java @@ -19,6 +19,7 @@ package org.casbin.jcasbin.main; import org.casbin.jcasbin.rbac.DefaultRoleManager; import org.casbin.jcasbin.rbac.ConditionalRoleManager; +import org.casbin.jcasbin.rbac.ConditionalDomainManager; import org.casbin.jcasbin.rbac.DomainManager; import org.casbin.jcasbin.rbac.RoleManager; import org.casbin.jcasbin.util.BuiltInFunctions; @@ -27,6 +28,8 @@ import org.testng.annotations.Test; import java.util.Arrays; import java.util.Collections; +import static org.testng.Assert.assertEquals; + /** * @author Yixiang Zhao (@seriouszyx) **/ @@ -47,6 +50,40 @@ public class RoleManagerUnitTest { TestUtil.testHasRole(rm, "alice", "admin", false); } + @Test + public void testDeletingPatternLinkPreservesExplicitDomainLink() { + ConditionalDomainManager rm = new ConditionalDomainManager(10); + rm.addDomainMatchingFunc("keyMatch", BuiltInFunctions::keyMatch); + rm.addLink("alice", "admin", "tenant*"); + rm.addLink("alice", "admin", "tenantA"); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", true); + TestUtil.testHasRole(rm, "alice", "admin", "tenantB", true); + + rm.deleteLink("alice", "admin", "tenant*"); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", true); + TestUtil.testHasRole(rm, "alice", "admin", "tenantB", false); + TestUtil.testGetRoles(rm, "alice", "tenantA", Collections.singletonList("admin")); + assertEquals(rm.getUsers("admin", "tenantA"), Collections.singletonList("alice")); + + rm.clear(); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", false); + } + + @Test + public void testDomainConditionsRegisteredBeforePatternLinks() { + ConditionalDomainManager rm = new ConditionalDomainManager(10); + rm.addDomainMatchingFunc("keyMatch", BuiltInFunctions::keyMatch); + rm.addDomainLinkConditionFunc("alice", "admin", "tenantA", params -> "allow".equals(params[0])); + rm.setDomainLinkConditionFuncParams("alice", "admin", "tenantA", "deny"); + rm.addLink("alice", "admin", "tenant*"); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", false); + + rm.setDomainLinkConditionFuncParams("alice", "admin", "tenantA", "allow"); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", true); + rm.addDomainLinkConditionFunc("alice", "admin", "tenantA", params -> false); + TestUtil.testHasRole(rm, "alice", "admin", "tenantA", false); + } + @Test public void testRole() { RoleManager rm = new DefaultRoleManager(3); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
