libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-2964 - Fix groups not applied for authentication of admin operations

Browse source
This commit is contained in:
Hynek Mlnarik 2016-10-10 09:47:03 +02:00
parent a1fd85c52a
commit 03cf9bad2e
12 changed files with 309 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/group/GroupLDAPFederationMapper.java
View file

@ -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>
@ -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<GroupModel> getGroups() {
Set<GroupModel> ldapGroupMappings = getLDAPGroupMappingsConverted();

3
federation/ldap/src/main/java/org/keycloak/federation/ldap/mappers/membership/role/RoleLDAPFederationMapper.java
View file

@ -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

2
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/UserAdapter.java vendored
View file

@ -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

3
model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java
View file

@ -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) {

3
model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/UserAdapter.java
View file

@ -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

16
server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java
View file

@ -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();

38
server-spi/src/main/java/org/keycloak/models/utils/KeycloakModelUtils.java
View file

@ -69,6 +69,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.
@ -328,6 +329,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<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

3
server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapter.java
View file

@ -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

3
server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java
View file

@ -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

8
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/AbstractKeycloakTest.java
View file

@ -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);

169
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/group/GroupTest.java
View file

@ -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 <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()));
}
}

85
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/util/GroupBuilder.java Normal file
View file

@ -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;
}
}