diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java index b6862f55e8..41cb41df50 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java @@ -38,6 +38,7 @@ public class RoleAdapter implements RoleModel { protected CachedRole cached; protected RealmCacheSession cacheSession; protected RealmModel realm; + protected Set composites; public RoleAdapter(CachedRole cached, RealmCacheSession session, RealmModel realm) { this.cached = cached; @@ -132,15 +133,19 @@ public class RoleAdapter implements RoleModel { @Override public Set getComposites() { if (isUpdated()) return updated.getComposites(); - Set set = new HashSet(); - for (String id : cached.getComposites()) { - RoleModel role = realm.getRoleById(id); - if (role == null) { - throw new IllegalStateException("Could not find composite in role " + getName() + ": " + id); + + if (composites == null) { + composites = new HashSet(); + for (String id : cached.getComposites()) { + RoleModel role = realm.getRoleById(id); + if (role == null) { + throw new IllegalStateException("Could not find composite in role " + getName() + ": " + id); + } + composites.add(role); } - set.add(role); } - return set; + + return composites; } @Override @@ -171,11 +176,7 @@ public class RoleAdapter implements RoleModel { @Override public boolean hasRole(RoleModel role) { - if (this.equals(role)) return true; - if (!isComposite()) return false; - - Set visited = new HashSet(); - return KeycloakModelUtils.searchFor(role, this, visited); + return this.equals(role) || KeycloakModelUtils.searchFor(role, this); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java index 3cc1553f2b..10e625291a 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java @@ -128,11 +128,7 @@ public class RoleAdapter implements RoleModel, JpaModel { @Override public boolean hasRole(RoleModel role) { - if (this.equals(role)) return true; - if (!isComposite()) return false; - - Set visited = new HashSet(); - return KeycloakModelUtils.searchFor(role, this, visited); + return this.equals(role) || KeycloakModelUtils.searchFor(role, this); } @Override diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RoleAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RoleAdapter.java index 47ca9802b0..e141ff8e17 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RoleAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RoleAdapter.java @@ -172,11 +172,7 @@ public class RoleAdapter extends AbstractMongoAdapter implement @Override public boolean hasRole(RoleModel role) { - if (this.equals(role)) return true; - if (!isComposite()) return false; - - Set visited = new HashSet(); - return KeycloakModelUtils.searchFor(role, this, visited); + return this.equals(role) || KeycloakModelUtils.searchFor(role, this); } public MongoRoleEntity getRole() { diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index c0f28db92b..cba151e25a 100755 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -177,16 +177,14 @@ public final class KeycloakModelUtils { * @param visited set of already visited roles (used for recursion) * @return true if "role" is descendant of "composite" */ - public static boolean searchFor(RoleModel role, RoleModel composite, Set visited) { - if (visited.contains(composite)) return false; - visited.add(composite); - Set composites = composite.getComposites(); - if (composites.contains(role)) return true; - for (RoleModel contained : composites) { - if (!contained.isComposite()) continue; - if (searchFor(role, contained, visited)) return true; - } - return false; + public static boolean searchFor(RoleModel role, RoleModel composite) { + return composite.isComposite() && ( + composite.getComposites().contains(role) || + composite.getComposites().stream() + .filter(x -> x.isComposite() && x.hasRole(role)) + .findFirst() + .isPresent() + ); } /** diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/CompositeRolesModelTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/CompositeRolesModelTest.java index 038c14802e..78ba7143dc 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/CompositeRolesModelTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/CompositeRolesModelTest.java @@ -66,18 +66,27 @@ public class CompositeRolesModelTest extends AbstractModelTest { @Test public void testComposites() { Set requestedRoles = getRequestedRoles("APP_COMPOSITE_APPLICATION", "APP_COMPOSITE_USER"); - Assert.assertEquals(2, requestedRoles.size()); + Assert.assertEquals(5, requestedRoles.size()); + assertContains("APP_COMPOSITE_APPLICATION", "APP_COMPOSITE_ROLE", requestedRoles); + assertContains("APP_COMPOSITE_APPLICATION", "APP_COMPOSITE_CHILD", requestedRoles); + assertContains("APP_COMPOSITE_APPLICATION", "APP_ROLE_2", requestedRoles); assertContains("APP_ROLE_APPLICATION", "APP_ROLE_1", requestedRoles); assertContains("realm", "REALM_ROLE_1", requestedRoles); requestedRoles = getRequestedRoles("APP_COMPOSITE_APPLICATION", "REALM_APP_COMPOSITE_USER"); - Assert.assertEquals(1, requestedRoles.size()); + Assert.assertEquals(4, requestedRoles.size()); assertContains("APP_ROLE_APPLICATION", "APP_ROLE_1", requestedRoles); requestedRoles = getRequestedRoles("REALM_COMPOSITE_1_APPLICATION", "REALM_COMPOSITE_1_USER"); Assert.assertEquals(1, requestedRoles.size()); assertContains("realm", "REALM_COMPOSITE_1", requestedRoles); + requestedRoles = getRequestedRoles("REALM_COMPOSITE_2_APPLICATION", "REALM_COMPOSITE_1_USER"); + Assert.assertEquals(3, requestedRoles.size()); + assertContains("realm", "REALM_COMPOSITE_1", requestedRoles); + assertContains("realm", "REALM_COMPOSITE_CHILD", requestedRoles); + assertContains("realm", "REALM_ROLE_4", requestedRoles); + requestedRoles = getRequestedRoles("REALM_ROLE_1_APPLICATION", "REALM_COMPOSITE_1_USER"); Assert.assertEquals(1, requestedRoles.size()); assertContains("realm", "REALM_ROLE_1", requestedRoles); diff --git a/testsuite/integration/src/test/resources/model/testcomposites.json b/testsuite/integration/src/test/resources/model/testcomposites.json index 740a4e198e..d9e9bb16db 100755 --- a/testsuite/integration/src/test/resources/model/testcomposites.json +++ b/testsuite/integration/src/test/resources/model/testcomposites.json @@ -21,7 +21,7 @@ "email" : "test-user1@localhost", "credentials" : [ { "type" : "password", - "value" : "password" } + "value" : "password" } ], "realmRoles": [ "REALM_COMPOSITE_1" ] }, @@ -31,7 +31,7 @@ "email" : "test-user2@localhost", "credentials" : [ { "type" : "password", - "value" : "password" } + "value" : "password" } ], "realmRoles": [ "REALM_ROLE_1"] }, @@ -41,7 +41,7 @@ "email" : "test-user3@localhost", "credentials" : [ { "type" : "password", - "value" : "password" } + "value" : "password" } ], "realmRoles": [ "REALM_APP_COMPOSITE_ROLE" ] }, @@ -51,7 +51,7 @@ "email" : "test-user4@localhost", "credentials" : [ { "type" : "password", - "value" : "password" } + "value" : "password" } ], "applicationRoles": { "APP_ROLE_APPLICATION": [ "APP_ROLE_2" ] @@ -63,7 +63,7 @@ "email" : "test-user5@localhost", "credentials" : [ { "type" : "password", - "value" : "password" } + "value" : "password" } ], "realmRoles": ["REALM_APP_COMPOSITE_ROLE", "REALM_COMPOSITE_1"] } @@ -80,6 +80,10 @@ "client": "REALM_COMPOSITE_1_APPLICATION", "roles": ["REALM_COMPOSITE_1"] }, + { + "client": "REALM_COMPOSITE_2_APPLICATION", + "roles": ["REALM_COMPOSITE_1", "REALM_COMPOSITE_CHILD", "REALM_ROLE_4"] + }, { "client": "REALM_ROLE_1_APPLICATION", "roles": ["REALM_ROLE_1"] @@ -93,7 +97,15 @@ "baseUrl": "http://localhost:8081/app", "adminUrl": "http://localhost:8081/app/logout", "secret": "password" - }, + }, + { + "name": "REALM_COMPOSITE_2_APPLICATION", + "fullScopeAllowed": false, + "enabled": true, + "baseUrl": "http://localhost:8081/app", + "adminUrl": "http://localhost:8081/app/logout", + "secret": "password" + }, { "name": "REALM_ROLE_1_APPLICATION", "fullScopeAllowed": false, @@ -130,10 +142,19 @@ { "name": "REALM_ROLE_3" }, + { + "name": "REALM_ROLE_4" + }, { "name": "REALM_COMPOSITE_1", "composites": { - "realm": ["REALM_ROLE_1"] + "realm": ["REALM_ROLE_1", "REALM_COMPOSITE_CHILD"] + } + }, + { + "name": "REALM_COMPOSITE_CHILD", + "composites": { + "realm": ["REALM_ROLE_4"] } }, { @@ -142,6 +163,9 @@ "application": { "APP_ROLE_APPLICATION" :[ "APP_ROLE_1" + ], + "APP_COMPOSITE_APPLICATION" :[ + "APP_COMPOSITE_ROLE" ] } } @@ -168,6 +192,19 @@ "application": { "APP_ROLE_APPLICATION" :[ "APP_ROLE_1" + ], + "APP_COMPOSITE_APPLICATION" :[ + "APP_COMPOSITE_CHILD" + ] + } + } + }, + { + "name": "APP_COMPOSITE_CHILD", + "composites": { + "application": { + "APP_COMPOSITE_APPLICATION" :[ + "APP_ROLE_2" ] } } @@ -184,7 +221,7 @@ "APP_ROLE_APPLICATION": [ { "client": "APP_COMPOSITE_APPLICATION", - "roles": ["APP_ROLE_2"] + "roles": ["APP_ROLE_1"] } ] }