Merge pull request #3304 from hmlnarik/KEYCLOAK-2964
KEYCLOAK-2964 - Fix groups not applied for authentication of admin operations
This commit is contained in:
commit
940237ee78
12 changed files with 309 additions and 30 deletions
|
@ -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();
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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) {
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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();
|
||||
|
|
|
@ -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<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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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);
|
||||
|
||||
|
|
|
@ -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()));
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
}
|
Loading…
Reference in a new issue