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

Fixes #9482: A user could be assigned to a parent group if he is already assigned to a subgroup.

Browse source
This commit is contained in:
Christoph Leistert 2022-03-17 10:50:24 +01:00 committed by Hynek Mlnařík
parent c70b4eaade
commit cc2bb96abc
7 changed files with 93 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/membership/group/GroupLDAPStorageMapper.java
View file

@ -751,7 +751,7 @@ public class GroupLDAPStorageMapper extends AbstractLDAPStorageMapper implements
@Override
public boolean isMemberOf(GroupModel group) {
return getGroupsStream().anyMatch(Predicate.isEqual(group));
return RoleUtils.isDirectMember(getGroupsStream(),group);
}
protected Stream<GroupModel> getLDAPGroupMappingsConverted() {

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

@ -392,7 +392,7 @@ public class UserAdapter implements UserModel.Streams, JpaModel<UserEntity> {
@Override
public void joinGroup(GroupModel group) {
if (isMemberOf(group)) return;
if (RoleUtils.isDirectMember(getGroupsStream(), group)) return;
joinGroupImpl(group);
}

1
model/map/src/main/java/org/keycloak/models/map/user/MapUserAdapter.java
View file

@ -255,6 +255,7 @@ public abstract class MapUserAdapter extends AbstractUserModel<MapUserEntity> {
@Override
public void joinGroup(GroupModel group) {
if (RoleUtils.isDirectMember(getGroupsStream(), group)) return;
entity.addGroupsMembership(group.getId());
}

5
server-spi-private/src/main/java/org/keycloak/storage/adapter/UpdateOnlyChangeUserModelDelegate.java
View file

@ -25,6 +25,7 @@ import java.util.stream.Collectors;
import org.keycloak.models.GroupModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.RoleUtils;
import org.keycloak.models.utils.UserModelDelegate;
import static org.keycloak.common.util.ObjectUtil.isEqualOrBothNull;
@ -170,7 +171,7 @@ public class UpdateOnlyChangeUserModelDelegate extends UserModelDelegate {
@Override
public void joinGroup(GroupModel group) {
if (!isMemberOf(group)) {
if (!RoleUtils.isDirectMember(getGroupsStream(),group)) {
delegate.joinGroup(group);
}
@ -178,7 +179,7 @@ public class UpdateOnlyChangeUserModelDelegate extends UserModelDelegate {
@Override
public void leaveGroup(GroupModel group) {
if (isMemberOf(group)) {
if (RoleUtils.isDirectMember(getGroupsStream(),group)) {
delegate.leaveGroup(group);
}
}

10
server-spi/src/main/java/org/keycloak/models/utils/RoleUtils.java
View file

@ -80,6 +80,16 @@ public class RoleUtils {
});
}
/**
*
* @param groups
* @param targetGroup
* @return true if targetGroup is in groups directly
*/
public static boolean isDirectMember(Stream<GroupModel> groups, GroupModel targetGroup) {
return groups.anyMatch(g -> targetGroup.getId().equals(g.getId()));
}
/**
* @param roles
* @param targetRole

3
services/src/main/java/org/keycloak/services/resources/admin/UserResource.java
View file

@ -50,6 +50,7 @@ import org.keycloak.models.UserModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.models.utils.RepresentationToModel;
import org.keycloak.models.utils.RoleUtils;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.oidc.utils.RedirectUtils;
import org.keycloak.provider.ProviderFactory;
@ -943,7 +944,7 @@ public class UserResource {
throw new NotFoundException("Group not found");
}
auth.groups().requireManageMembership(group);
if (!user.isMemberOf(group)){
if (!RoleUtils.isDirectMember(user.getGroupsStream(),group)){
user.joinGroup(group);
adminEvent.operation(OperationType.CREATE).resource(ResourceType.GROUP_MEMBERSHIP).representation(ModelToRepresentation.toRepresentation(group, true)).resourcePath(session.getContext().getUri()).success();
}

74
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java
View file

@ -3057,4 +3057,78 @@ public class UserTest extends AbstractAdminTest {
testRealm().roles().get("realm-role").remove();
}
}
/**
* Test for #9482
*/
@Test
public void joinParentGroupAfterSubGroup() {
String username = "user-with-sub-and-parent-group";
String parentGroupName = "parent-group";
String subGroupName = "sub-group";
UserRepresentation userRepresentation = UserBuilder.create().username(username).build();
GroupRepresentation subGroupRep = GroupBuilder.create().name(subGroupName).build();
GroupRepresentation parentGroupRep = GroupBuilder.create().name(parentGroupName).subGroups(List.of(subGroupRep)).build();
try (Creator<UserResource> u = Creator.create(realm, userRepresentation);
Creator<GroupResource> subgroup = Creator.create(realm, subGroupRep);
Creator<GroupResource> parentGroup = Creator.create(realm, parentGroupRep)) {
UserResource user = u.resource();
//when
user.joinGroup(subgroup.id());
List<GroupRepresentation> obtainedGroups = realm.users().get(u.id()).groups();
//then
assertEquals(1, obtainedGroups.size());
assertEquals(subGroupName, obtainedGroups.get(0).getName());
//when
user.joinGroup(parentGroup.id());
obtainedGroups = realm.users().get(u.id()).groups();
//then
assertEquals(2, obtainedGroups.size());
assertEquals(parentGroupName, obtainedGroups.get(0).getName());
assertEquals(subGroupName, obtainedGroups.get(1).getName());
}
}
@Test
public void joinSubGroupAfterParentGroup() {
String username = "user-with-sub-and-parent-group";
String parentGroupName = "parent-group";
String subGroupName = "sub-group";
UserRepresentation userRepresentation = UserBuilder.create().username(username).build();
GroupRepresentation subGroupRep = GroupBuilder.create().name(subGroupName).build();
GroupRepresentation parentGroupRep = GroupBuilder.create().name(parentGroupName).subGroups(List.of(subGroupRep)).build();
try (Creator<UserResource> u = Creator.create(realm, userRepresentation);
Creator<GroupResource> subgroup = Creator.create(realm, subGroupRep);
Creator<GroupResource> parentGroup = Creator.create(realm, parentGroupRep)) {
UserResource user = u.resource();
//when
user.joinGroup(parentGroup.id());
List<GroupRepresentation> obtainedGroups = realm.users().get(u.id()).groups();
//then
assertEquals(1, obtainedGroups.size());
assertEquals(parentGroupName, obtainedGroups.get(0).getName());
//when
user.joinGroup(subgroup.id());
obtainedGroups = realm.users().get(u.id()).groups();
//then
assertEquals(2, obtainedGroups.size());
assertEquals(parentGroupName, obtainedGroups.get(0).getName());
assertEquals(subGroupName, obtainedGroups.get(1).getName());
}
}
}