keycloak-uncached

Merge pull request #3304 from hmlnarik/KEYCLOAK-2964 KEYCLOAK-2964

10/18/2016 10:50:12 AM

Details

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 e27f3ef..460ab62 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 <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@@ -561,6 +562,11 @@ public class GroupLDAPFederationMapper extends AbstractLDAPFederationMapper impl
         }
 
         @Override
+        public boolean hasRole(RoleModel role) {
+            return super.hasRole(role) || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true);
+        }
+
+        @Override
         public Set<GroupModel> getGroups() {
             Set<GroupModel> ldapGroupMappings = getLDAPGroupMappingsConverted();
             if (config.getMode() == LDAPGroupMapperMode.LDAP_ONLY) {
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 2a23001..9d6e2a1 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<RoleModel> 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 edbc186..6030a66 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 7c5fe1f..5ee5490 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<UserEntity> {
     @Override
     public boolean hasRole(RoleModel role) {
         Set<RoleModel> roles = getRoleMappings();
-        return KeycloakModelUtils.hasRole(roles, role);
+        return KeycloakModelUtils.hasRole(roles, role)
+          || KeycloakModelUtils.hasRoleFromGroup(getGroups(), role, true);
     }
 
     protected TypedQuery<UserRoleMappingEntity> 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 5453c0b..972f440 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<MongoUserEntity> implement
     @Override
     public boolean hasRole(RoleModel role) {
         Set<RoleModel> 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 fd25372..85f1fd3 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<RoleModel> getClientRoleMappings(ClientModel app);
 
+    /**
+     * Returns {@code true} if this object is directly or indirectly assigned the given role, {@code false} otherwise.
+     * <p>
+     * For example, {@code true} is returned for hasRole(R) if:
+     * <ul>
+     *  <li>R is directly assigned to this object</li>
+     *  <li>R is not assigned to this object but this object belongs to a group G which is assigned the role R</li>
+     *  <li>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</li>
+     * </ul>
+     * @param role
+     * @return see description
+     */
     boolean hasRole(RoleModel role);
 
+    /**
+     * Grants the given role to this object.
+     * @param role
+     */
     void grantRole(RoleModel role);
 
     Set<RoleModel> 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 1eae859..230d15f 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.
@@ -267,6 +268,43 @@ public final class KeycloakModelUtils {
     }
 
     /**
+     * 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<GroupModel> groups, RoleModel targetRole, boolean checkParentGroup) {
+        if (groups == null) {
+            return false;
+        }
+
+        return StreamSupport.stream(groups.spliterator(), false)
+          .anyMatch(group -> hasRoleFromGroup(group, targetRole, checkParentGroup));
+    }
+
+    /**
      *
      * @param groups
      * @param targetGroup
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 49d2288..df6c37a 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<RoleModel> 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 68e0689..ed8759b 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<RoleModel> 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 5bd733c..676a40f 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<String> 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 f44cc2e..7e61f51 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 static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import java.util.UUID;
+import javax.ws.rs.ClientErrorException;
+import static org.hamcrest.Matchers.*;
+
+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 <a href="mailto:mstrukel@redhat.com">Marko Strukelj</a>
  */
 public class GroupTest extends AbstractGroupTest {
 
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     @Override
     public void addTestRealms(List<RealmRepresentation> 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<String, List<String>> 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 0000000..0968ead
--- /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 <a href="mailto:hmlnarik@redhat.com">Hynek Mlnarik</a>
+ */
+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<String> realmRoles) {
+        rep.setRealmRoles(realmRoles);
+        return this;
+    }
+
+    public GroupBuilder clientRoles(Map<String, List<String>> clientRoles) {
+        rep.setClientRoles(clientRoles);
+        return this;
+    }
+
+    public GroupBuilder attributes(Map<String, List<String>> attributes) {
+        rep.setAttributes(attributes);
+        return this;
+    }
+
+    public GroupBuilder singleAttribute(String name, String value) {
+        rep.singleAttribute(name, value);
+        return this;
+    }
+
+    public GroupBuilder subGroups(List<GroupRepresentation> subGroups) {
+        rep.setSubGroups(subGroups);
+        return this;
+    }
+
+}