diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java index e27f3efd48..460ab62ced 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java @@ -48,6 +48,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Set; +import org.keycloak.models.RoleModel; /** * @author Marek Posolda @@ -560,6 +561,11 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl this.ldapUser = ldapUser; } + @Override + public boolean hasRole(RoleModel role) { + return super.hasRole(role) || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); + } + @Override public Set getGroups() { Set ldapGroupMappings = getLDAPGroupMappingsConverted(); diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java index 2a23001fcb..9d6e2a1282 100644 --- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java @@ -348,7 +348,8 @@ public class RoleLDAPFederationMapper extends AbstractLDAPFederationMapper imple @Override public boolean hasRole(RoleModel role) { Set roles = getRoleMappings(); - return KeycloakModelUtils.hasRole(roles, role); + return KeycloakModelUtils.hasRole(roles, role) + || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java index edbc1863b5..6030a66e8e 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java @@ -301,7 +301,7 @@ public class UserAdapter implements CachedUserModel { for (RoleModel mapping: mappings) { if (mapping.hasRole(role)) return true; } - return false; + return KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index 7c5fe1ff0c..5ee5490282 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -352,7 +352,8 @@ public class UserAdapter implements UserModel, JpaModel { @Override public boolean hasRole(RoleModel role) { Set roles = getRoleMappings(); - return KeycloakModelUtils.hasRole(roles, role); + return KeycloakModelUtils.hasRole(roles, role) + || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } protected TypedQuery getUserRoleMappingEntityTypedQuery(RoleModel role) { diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java index 5453c0b4ee..972f4409b7 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java @@ -268,7 +268,8 @@ public class UserAdapter extends AbstractMongoAdapter implement @Override public boolean hasRole(RoleModel role) { Set roles = getRoleMappings(); - return KeycloakModelUtils.hasRole(roles, role); + return KeycloakModelUtils.hasRole(roles, role) + || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } @Override diff --git a/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java b/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java index fd25372d7e..85f1fd3dfa 100755 --- a/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java @@ -28,8 +28,24 @@ public interface RoleMapperModel { Set getClientRoleMappings(ClientModel app); + /** + * Returns {@code true} if this object is directly or indirectly assigned the given role, {@code false} otherwise. + *

+ * For example, {@code true} is returned for hasRole(R) if: + *

    + *
  • R is directly assigned to this object
  • + *
  • R is not assigned to this object but this object belongs to a group G which is assigned the role R
  • + *
  • R is not assigned to this object but this object belongs to a group G, and G belongs to group H which is assigned the role R
  • + *
+ * @param role + * @return see description + */ boolean hasRole(RoleModel role); + /** + * Grants the given role to this object. + * @param role + */ void grantRole(RoleModel role); Set getRoleMappings(); diff --git a/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java b/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java index 1eae85946c..230d15f137 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java @@ -70,6 +70,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.stream.StreamSupport; /** * Set of helper methods, which are useful in various model implementations. @@ -266,6 +267,43 @@ public final class KeycloakModelUtils { return false; } + /** + * Checks whether the {@code targetRole} is contained in the given group or its parents + * (if requested) + * @param group Group to check role for + * @param targetRole + * @param checkParentGroup When {@code true}, also parent group is recursively checked for role + * @return true if targetRole is in roles (directly or indirectly via composite role) + */ + public static boolean hasRoleFromGroup(GroupModel group, RoleModel targetRole, boolean checkParentGroup) { + if (group.hasRole(targetRole)) + return true; + + if (checkParentGroup) { + GroupModel parent = group.getParent(); + return parent != null && hasRoleFromGroup(parent, targetRole, true); + } + + return false; + } + + /** + * Checks whether the {@code targetRole} is contained in any of the {@code groups} or their parents + * (if requested) + * @param groups + * @param targetRole + * @param checkParentGroup When {@code true}, also parent group is recursively checked for role + * @return true if targetRole is in roles (directly or indirectly via composite role) + */ + public static boolean hasRoleFromGroup(Iterable groups, RoleModel targetRole, boolean checkParentGroup) { + if (groups == null) { + return false; + } + + return StreamSupport.stream(groups.spliterator(), false) + .anyMatch(group -> hasRoleFromGroup(group, targetRole, checkParentGroup)); + } + /** * * @param groups diff --git a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapter.java b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapter.java index 49d22889ed..df6c37ab2a 100644 --- a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapter.java +++ b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapter.java @@ -172,7 +172,8 @@ public abstract class AbstractUserAdapter implements UserModel { @Override public boolean hasRole(RoleModel role) { Set roles = getRoleMappings(); - return KeycloakModelUtils.hasRole(roles, role); + return KeycloakModelUtils.hasRole(roles, role) + || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } @Override diff --git a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java index 68e068998b..ed8759bd08 100644 --- a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java +++ b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java @@ -177,7 +177,8 @@ public abstract class AbstractUserAdapterFederatedStorage implements UserModel { @Override public boolean hasRole(RoleModel role) { Set roles = getRoleMappings(); - return KeycloakModelUtils.hasRole(roles, role); + return KeycloakModelUtils.hasRole(roles, role) + || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true); } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java index 5bd733cfdf..676a40f052 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java @@ -278,6 +278,14 @@ public abstract class AbstractKeycloakTest { } } + /** + * Creates a user in the given realm and returns its ID. + * @param realm Realm name + * @param username Username + * @param password Password + * @param requiredActions + * @return ID of the newly created user + */ public String createUser(String realm, String username, String password, String ... requiredActions) { List requiredUserActions = Arrays.asList(requiredActions); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java index f44cc2e668..7e61f511db 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java @@ -45,20 +45,30 @@ import javax.ws.rs.core.Response; import java.io.IOException; import java.net.URI; import java.util.Collections; -import java.util.HashMap; import java.util.LinkedList; import java.util.List; -import java.util.Map; +import java.util.UUID; +import javax.ws.rs.ClientErrorException; +import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; +import org.junit.Rule; +import org.junit.rules.ExpectedException; +import org.keycloak.admin.client.Keycloak; +import org.keycloak.models.AdminRoles; import static org.keycloak.testsuite.Assert.assertNames; +import org.keycloak.testsuite.arquillian.AuthServerTestEnricher; +import org.keycloak.testsuite.auth.page.AuthRealm; +import org.keycloak.testsuite.util.GroupBuilder; /** * @author Marko Strukelj */ public class GroupTest extends AbstractGroupTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Override public void addTestRealms(List testRealms) { RealmRepresentation testRealmRep = loadTestRealm(testRealms); @@ -293,26 +303,24 @@ public class GroupTest extends AbstractGroupTest { @Test public void updateGroup() { RealmResource realm = adminClient.realms().realm("test"); + final String groupName = "group-" + UUID.randomUUID(); - GroupRepresentation group = new GroupRepresentation(); - group.setName("group"); - - Map> attrs = new HashMap<>(); - attrs.put("attr1", Collections.singletonList("attrval1")); - attrs.put("attr2", Collections.singletonList("attrval2")); - group.setAttributes(attrs); + GroupRepresentation group = GroupBuilder.create() + .name(groupName) + .singleAttribute("attr1", "attrval1") + .singleAttribute("attr2", "attrval2") + .build(); createGroup(realm, group); - group = realm.getGroupByPath("/group"); + group = realm.getGroupByPath("/" + groupName); Assert.assertNotNull(group); - assertEquals("group", group.getName()); - assertEquals(2, group.getAttributes().size()); - assertEquals(1, group.getAttributes().get("attr1").size()); - assertEquals("attrval1", group.getAttributes().get("attr1").get(0)); - assertEquals(1, group.getAttributes().get("attr2").size()); - assertEquals("attrval2", group.getAttributes().get("attr2").get(0)); + assertThat(group.getName(), is(groupName)); + assertThat(group.getAttributes().keySet(), containsInAnyOrder("attr1", "attr2")); + assertThat(group.getAttributes(), hasEntry(is("attr1"), contains("attrval1"))); + assertThat(group.getAttributes(), hasEntry(is("attr2"), contains("attrval2"))); - group.setName("group-new"); + final String groupNewName = "group-" + UUID.randomUUID(); + group.setName(groupNewName); group.getAttributes().remove("attr1"); group.getAttributes().get("attr2").add("attrval2-2"); @@ -321,12 +329,12 @@ public class GroupTest extends AbstractGroupTest { realm.groups().group(group.getId()).update(group); assertAdminEvents.assertEvent("test", OperationType.UPDATE, AdminEventPaths.groupPath(group.getId()), group, ResourceType.GROUP); - group = realm.getGroupByPath("/group-new"); + group = realm.getGroupByPath("/" + groupNewName); - assertEquals("group-new", group.getName()); - assertEquals(2, group.getAttributes().size()); - assertEquals(2, group.getAttributes().get("attr2").size()); - assertEquals(1, group.getAttributes().get("attr3").size()); + assertThat(group.getName(), is(groupNewName)); + assertThat(group.getAttributes().keySet(), containsInAnyOrder("attr2", "attr3")); + assertThat(group.getAttributes(), hasEntry(is("attr2"), containsInAnyOrder("attrval2", "attrval2-2"))); + assertThat(group.getAttributes(), hasEntry(is("attr3"), contains("attrval2"))); } @Test @@ -457,4 +465,117 @@ public class GroupTest extends AbstractGroupTest { assertNames(roles.clientLevel(clientId).listAll(), "client-composite"); } + + /** + * Verifies that the user does not have access to Keycloak Admin endpoint when role is not + * assigned to that user. + * @link https://issues.jboss.org/browse/KEYCLOAK-2964 + */ + @Test + public void noAdminEndpointAccessWhenNoRoleAssigned() { + String userName = "user-" + UUID.randomUUID(); + final String realmName = AuthRealm.MASTER; + createUser(realmName, userName, "pwd"); + + Keycloak userClient = Keycloak.getInstance(AuthServerTestEnricher.getAuthServerContextRoot() + "/auth", + realmName, userName, "pwd", Constants.ADMIN_CLI_CLIENT_ID); + + expectedException.expect(ClientErrorException.class); + expectedException.expectMessage(String.valueOf(Response.Status.FORBIDDEN.getStatusCode())); + userClient.realms().findAll(); // Any admin operation will do + } + + /** + * Verifies that the role assigned to a user is correctly handled by Keycloak Admin endpoint. + * @link https://issues.jboss.org/browse/KEYCLOAK-2964 + */ + @Test + public void adminEndpointAccessibleWhenAdminRoleAssignedToUser() { + String userName = "user-" + UUID.randomUUID(); + + final String realmName = AuthRealm.MASTER; + RealmResource realm = adminClient.realms().realm(realmName); + RoleRepresentation adminRole = realm.roles().get(AdminRoles.ADMIN).toRepresentation(); + assertThat(adminRole, notNullValue()); + assertThat(adminRole.getId(), notNullValue()); + + String userId = createUser(realmName, userName, "pwd"); + assertThat(userId, notNullValue()); + + RoleMappingResource mappings = realm.users().get(userId).roles(); + mappings.realmLevel().add(Collections.singletonList(adminRole)); + + Keycloak userClient = Keycloak.getInstance(AuthServerTestEnricher.getAuthServerContextRoot() + "/auth", + realmName, userName, "pwd", Constants.ADMIN_CLI_CLIENT_ID); + + assertThat(userClient.realms().findAll(), // Any admin operation will do + not(empty())); + } + + /** + * Verifies that the role assigned to a user's group is correctly handled by Keycloak Admin endpoint. + * @link https://issues.jboss.org/browse/KEYCLOAK-2964 + */ + @Test + public void adminEndpointAccessibleWhenAdminRoleAssignedToGroup() { + String userName = "user-" + UUID.randomUUID(); + String groupName = "group-" + UUID.randomUUID(); + + final String realmName = AuthRealm.MASTER; + RealmResource realm = adminClient.realms().realm(realmName); + RoleRepresentation adminRole = realm.roles().get(AdminRoles.ADMIN).toRepresentation(); + assertThat(adminRole, notNullValue()); + assertThat(adminRole.getId(), notNullValue()); + + String userId = createUser(realmName, userName, "pwd"); + GroupRepresentation group = GroupBuilder.create().name(groupName).build(); + Response response = realm.groups().add(group); + String groupId = ApiUtil.getCreatedId(response); + response.close(); + + RoleMappingResource mappings = realm.groups().group(groupId).roles(); + mappings.realmLevel().add(Collections.singletonList(adminRole)); + + realm.users().get(userId).joinGroup(groupId); + + Keycloak userClient = Keycloak.getInstance(AuthServerTestEnricher.getAuthServerContextRoot() + "/auth", + realmName, userName, "pwd", Constants.ADMIN_CLI_CLIENT_ID); + + assertThat(userClient.realms().findAll(), // Any admin operation will do + not(empty())); + } + + + /** + * Verifies that the role assigned to a user's group is correctly handled by Keycloak Admin endpoint. + * @link https://issues.jboss.org/browse/KEYCLOAK-2964 + */ + @Test + public void adminEndpointAccessibleWhenAdminRoleAssignedToGroupAfterUserJoinedIt() { + String userName = "user-" + UUID.randomUUID(); + String groupName = "group-" + UUID.randomUUID(); + + final String realmName = AuthRealm.MASTER; + RealmResource realm = adminClient.realms().realm(realmName); + RoleRepresentation adminRole = realm.roles().get(AdminRoles.ADMIN).toRepresentation(); + assertThat(adminRole, notNullValue()); + assertThat(adminRole.getId(), notNullValue()); + + String userId = createUser(realmName, userName, "pwd"); + GroupRepresentation group = GroupBuilder.create().name(groupName).build(); + Response response = realm.groups().add(group); + String groupId = ApiUtil.getCreatedId(response); + response.close(); + + realm.users().get(userId).joinGroup(groupId); + + RoleMappingResource mappings = realm.groups().group(groupId).roles(); + mappings.realmLevel().add(Collections.singletonList(adminRole)); + + Keycloak userClient = Keycloak.getInstance(AuthServerTestEnricher.getAuthServerContextRoot() + "/auth", + realmName, userName, "pwd", Constants.ADMIN_CLI_CLIENT_ID); + + assertThat(userClient.realms().findAll(), // Any admin operation will do + not(empty())); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/GroupBuilder.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/GroupBuilder.java new file mode 100644 index 0000000000..0968ead475 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/GroupBuilder.java @@ -0,0 +1,85 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.testsuite.util; + +import java.util.List; +import java.util.Map; +import org.keycloak.representations.idm.GroupRepresentation; + +/** + * + * @author Hynek Mlnarik + */ +public class GroupBuilder { + + private final GroupRepresentation rep; + + public static GroupBuilder create() { + final GroupRepresentation rep = new GroupRepresentation(); + return new GroupBuilder(rep); + } + + private GroupBuilder(GroupRepresentation rep) { + this.rep = rep; + } + + public GroupRepresentation build() { + return rep; + } + + public GroupBuilder id(String id) { + rep.setId(id); + return this; + } + + public GroupBuilder name(String name) { + rep.setName(name); + return this; + } + + public GroupBuilder path(String path) { + rep.setPath(path); + return this; + } + + public GroupBuilder realmRoles(List realmRoles) { + rep.setRealmRoles(realmRoles); + return this; + } + + public GroupBuilder clientRoles(Map> clientRoles) { + rep.setClientRoles(clientRoles); + return this; + } + + public GroupBuilder attributes(Map> attributes) { + rep.setAttributes(attributes); + return this; + } + + public GroupBuilder singleAttribute(String name, String value) { + rep.singleAttribute(name, value); + return this; + } + + public GroupBuilder subGroups(List subGroups) { + rep.setSubGroups(subGroups); + return this; + } + +}