diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e07048a8d9..60e251c5e3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,4 +71,4 @@ jobs: - name: Build testsuite run: mvn clean install -B -DskipTests -f testsuite/pom.xml - name: Run base tests - undertow - run: mvn clean install -B -f testsuite/integration-arquillian/tests/base/pom.xml -Dkeycloak.client.provider=map | misc/log/trimmer.sh; exit ${PIPESTATUS[0]} \ No newline at end of file + run: mvn clean install -B -f testsuite/integration-arquillian/tests/base/pom.xml -Dkeycloak.client.provider=map -Dkeycloak.group.provider=map | misc/log/trimmer.sh; exit ${PIPESTATUS[0]} \ No newline at end of file diff --git a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java index d6b323e37e..11186aac92 100755 --- a/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java +++ b/integration/admin-client/src/main/java/org/keycloak/admin/client/resource/UserResource.java @@ -82,6 +82,10 @@ public interface UserResource { @QueryParam("max") Integer maxResults, @QueryParam("briefRepresentation") @DefaultValue("true") boolean briefRepresentation); + @Path("groups/count") + @GET + Map groupsCount(@QueryParam("search") String search); + @Path("groups/{groupId}") @PUT void joinGroup(@PathParam("groupId") String groupId); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java index dd3af804b9..a7e7e95a68 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmCacheSession.java @@ -466,7 +466,7 @@ public class RealmCacheSession implements CacheRealmProvider { static String getRealmByNameCacheKey(String name) { return "realm.query.by.name." + name; } - + @Override public List getRealmsWithProviderType(Class type) { // Retrieve realms from backend @@ -589,12 +589,12 @@ public class RealmCacheSession implements CacheRealmProvider { if (client.isServiceAccountsEnabled()) { UserModel serviceAccount = session.users().getServiceAccount(client); - + if (serviceAccount != null) { session.users().removeUser(realm, serviceAccount); } } - + return getClientDelegate().removeClient(realm, id); } @@ -687,7 +687,7 @@ public class RealmCacheSession implements CacheRealmProvider { } return list.stream(); } - + @Override public Stream getRealmRolesStream(RealmModel realm, Integer first, Integer max) { return getRoleDelegate().getRealmRolesStream(realm, first, max); @@ -697,7 +697,7 @@ public class RealmCacheSession implements CacheRealmProvider { public Stream getClientRolesStream(ClientModel client, Integer first, Integer max) { return getRoleDelegate().getClientRolesStream(client, first, max); } - + @Override public Stream searchForClientRolesStream(ClientModel client, String search, Integer first, Integer max) { return getRoleDelegate().searchForClientRolesStream(client, search, first, max); @@ -899,7 +899,16 @@ public class RealmCacheSession implements CacheRealmProvider { list.add(group); } - return list.stream().sorted(Comparator.comparing(GroupModel::getName)); + return list.stream().sorted(GroupModel.COMPARE_BY_NAME); + } + + public Stream getGroupsStream(RealmModel realm, Stream ids, String search, Integer first, Integer max) { + return getGroupDelegate().getGroupsStream(realm, ids, search, first, max); + } + + @Override + public Long getGroupsCount(RealmModel realm, Stream ids, String search) { + return getGroupDelegate().getGroupsCount(realm, ids, search); } @Override @@ -916,9 +925,9 @@ public class RealmCacheSession implements CacheRealmProvider { public Long getGroupsCountByNameContaining(RealmModel realm, String search) { return getGroupDelegate().getGroupsCountByNameContaining(realm, search); } - + @Override - public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, int firstResult, int maxResults) { + public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) { return getGroupDelegate().getGroupsByRoleStream(realm, role, firstResult, maxResults); } @@ -956,13 +965,14 @@ public class RealmCacheSession implements CacheRealmProvider { list.add(group); } - return list.stream().sorted(Comparator.comparing(GroupModel::getName)); + return list.stream().sorted(GroupModel.COMPARE_BY_NAME); } @Override public Stream getTopLevelGroupsStream(RealmModel realm, Integer first, Integer max) { String cacheKey = getTopGroupsQueryCacheKey(realm.getId() + first + max); - boolean queryDB = invalidations.contains(cacheKey) || listInvalidations.contains(realm.getId() + first + max); + boolean queryDB = invalidations.contains(cacheKey) || listInvalidations.contains(realm.getId() + first + max) + || listInvalidations.contains(realm.getId()); if (queryDB) { return getGroupDelegate().getTopLevelGroupsStream(realm, first, max); } @@ -993,7 +1003,7 @@ public class RealmCacheSession implements CacheRealmProvider { list.add(group); } - return list.stream().sorted(Comparator.comparing(GroupModel::getName)); + return list.stream().sorted(GroupModel.COMPARE_BY_NAME); } @Override @@ -1043,6 +1053,11 @@ public class RealmCacheSession implements CacheRealmProvider { } + @Override + public void preRemove(RealmModel realm, RoleModel role) { + getGroupDelegate().preRemove(realm, role); + } + private void addGroupEventIfAbsent(InvalidationEvent eventToAdd) { String groupId = eventToAdd.getId(); 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 0c499f79a2..66c142accf 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 @@ -341,6 +341,12 @@ public class UserAdapter implements CachedUserModel { return groups.stream(); } + @Override + public long getGroupsCountByNameContaining(String search) { + if (updated != null) return updated.getGroupsCountByNameContaining(search); + return modelSupplier.get().getGroupsCountByNameContaining(search); + } + @Override public void joinGroup(GroupModel group) { getDelegateForUpdate(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 29a8af000b..978a5d4dcb 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -108,7 +108,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro RealmAdapter adapter = new RealmAdapter(session, em, realm); return adapter; } - + @Override public List getRealmsWithProviderType(Class providerType) { TypedQuery query = em.createNamedQuery("getRealmIdsWithProviderType", String.class); @@ -154,7 +154,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro final RealmAdapter adapter = new RealmAdapter(session, em, realm); session.users().preRemove(adapter); - realm.getDefaultGroups().clear(); + realm.getDefaultGroupIds().clear(); em.flush(); int num = em.createNamedQuery("deleteGroupRoleMappingsByRealm") @@ -171,8 +171,8 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro removeRoles(adapter); - adapter.getGroupsStream().forEach(adapter::removeGroup); - + adapter.getTopLevelGroupsStream().forEach(adapter::removeGroup); + num = em.createNamedQuery("removeClientInitialAccessByRealm") .setParameter("realm", realm).executeUpdate(); @@ -272,12 +272,12 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro if (roles.isEmpty()) return null; return session.roles().getRoleById(client.getRealm(), roles.get(0)); } - + @Override public Stream getRealmRolesStream(RealmModel realm, Integer first, Integer max) { TypedQuery query = em.createNamedQuery("getRealmRoles", RoleEntity.class); query.setParameter("realm", realm.getId()); - + return getRolesStream(query, realm, first, max); } @@ -299,22 +299,22 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro return closing(results.map(role -> new RoleAdapter(session, realm, em, role))); } - + @Override public Stream searchForClientRolesStream(ClientModel client, String search, Integer first, Integer max) { TypedQuery query = em.createNamedQuery("searchForClientRoles", RoleEntity.class); query.setParameter("client", client.getId()); return searchForRoles(query, client.getRealm(), search, first, max); } - + @Override public Stream searchForRolesStream(RealmModel realm, String search, Integer first, Integer max) { TypedQuery query = em.createNamedQuery("searchForRealmRoles", RoleEntity.class); query.setParameter("realm", realm.getId()); - + return searchForRoles(query, realm, search, first, max); } - + protected Stream searchForRoles(TypedQuery query, RealmModel realm, String search, Integer first, Integer max) { query.setParameter("search", "%" + search.trim().toLowerCase() + "%"); @@ -322,9 +322,9 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro && first >= 0 && max >= 0) { query= query.setFirstResult(first).setMaxResults(max); } - + Stream results = query.getResultStream(); - + return closing(results.map(role -> new RoleAdapter(session, realm, em, role))); } @@ -352,7 +352,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro em.createNativeQuery("delete from " + compositeRoleTable + " where CHILD_ROLE = :role").setParameter("role", roleEntity).executeUpdate(); realm.getClientsStream().forEach(c -> c.deleteScopeMapping(role)); em.createNamedQuery("deleteClientScopeRoleMappingByRole").setParameter("role", roleEntity).executeUpdate(); - int val = em.createNamedQuery("deleteGroupRoleMappingsByRole").setParameter("roleId", roleEntity.getId()).executeUpdate(); + session.groups().preRemove(realm, role); em.flush(); em.remove(roleEntity); @@ -415,15 +415,79 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro group.setParent(toParent); if (toParent != null) toParent.addChild(group); else session.groups().addTopLevelGroup(realm, group); + + // TODO: Remove em.flush(), currently this needs to be there to translate ConstraintViolationException to + // DuplicateModelException {@link PersistenceExceptionConverter} is not called if the + // ConstraintViolationException is not thrown in method called directly from EntityManager + em.flush(); } @Override public Stream getGroupsStream(RealmModel realm) { - RealmEntity ref = em.getReference(RealmEntity.class, realm.getId()); + return closing(em.createNamedQuery("getGroupIdsByRealm", String.class) + .setParameter("realm", realm.getId()) + .getResultStream()) + .map(g -> session.groups().getGroupById(realm, g)); + } - return ref.getGroups().stream() - .map(g -> session.groups().getGroupById(realm, g.getId())) - .sorted(Comparator.comparing(GroupModel::getName)); + @Override + public Stream getGroupsStream(RealmModel realm, Stream ids, String search, Integer first, Integer max) { + if (search == null || search.isEmpty()) return getGroupsStream(realm, ids, first, max); + + TypedQuery query = em.createNamedQuery("getGroupIdsByNameContainingFromIdList", String.class) + .setParameter("realm", realm.getId()) + .setParameter("search", search) + .setParameter("ids", ids.collect(Collectors.toList())); + + return closing(paginateQuery(query, first, max).getResultStream()) + .map(g -> session.groups().getGroupById(realm, g)); + } + + @Override + public Stream getGroupsStream(RealmModel realm, Stream ids, Integer first, Integer max) { + if (first == null && max == null) { + return getGroupsStream(realm, ids); + } + + TypedQuery query = em.createNamedQuery("getGroupIdsFromIdList", String.class) + .setParameter("realm", realm.getId()) + .setParameter("ids", ids.collect(Collectors.toList())); + + + return closing(paginateQuery(query, first, max).getResultStream()) + .map(g -> session.groups().getGroupById(realm, g)); + } + + private static TypedQuery paginateQuery(TypedQuery query, Integer first, Integer max) { + if (first != null && first > 0) { + query = query.setFirstResult(first); + } + + if (max != null && max >= 0) { + query = query.setMaxResults(max); + } + + return query; + } + + @Override + public Stream getGroupsStream(RealmModel realm, Stream ids) { + return ids.map(id -> session.groups().getGroupById(realm, id)).sorted(GroupModel.COMPARE_BY_NAME); + } + + @Override + public Long getGroupsCount(RealmModel realm, Stream ids, String search) { + TypedQuery query; + if (search != null && !search.isEmpty()) { + query = em.createNamedQuery("getGroupCountByNameContainingFromIdList", Long.class) + .setParameter("search", search); + } else { + query = em.createNamedQuery("getGroupIdsFromIdList", Long.class); + } + + return query.setParameter("realm", realm.getId()) + .setParameter("ids", ids.collect(Collectors.toList())) + .getSingleResult(); } @Override @@ -452,44 +516,39 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro public Long getGroupsCountByNameContaining(RealmModel realm, String search) { return searchForGroupByNameStream(realm, search, null, null).count(); } - + @Override - public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, int firstResult, int maxResults) { + public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) { TypedQuery query = em.createNamedQuery("groupsInRole", GroupEntity.class); query.setParameter("roleId", role.getId()); - if (firstResult != -1) { - query.setFirstResult(firstResult); + if (firstResult != null && firstResult > 0) { + query = query.setFirstResult(firstResult); } - if (maxResults != -1) { - query.setMaxResults(maxResults); + if (maxResults != null && maxResults > 0) { + query = query.setMaxResults(maxResults); } Stream results = query.getResultStream(); return closing(results .map(g -> (GroupModel) new GroupAdapter(realm, em, g)) - .sorted(Comparator.comparing(GroupModel::getName))); + .sorted(GroupModel.COMPARE_BY_NAME)); } @Override public Stream getTopLevelGroupsStream(RealmModel realm) { - RealmEntity ref = em.getReference(RealmEntity.class, realm.getId()); - - return ref.getGroups().stream() - .filter(g -> GroupEntity.TOP_PARENT_ID.equals(g.getParentId())) - .map(g -> session.groups().getGroupById(realm, g.getId())) - .sorted(Comparator.comparing(GroupModel::getName)); + return getTopLevelGroupsStream(realm, null, null); } @Override public Stream getTopLevelGroupsStream(RealmModel realm, Integer first, Integer max) { - Stream groupIds = em.createNamedQuery("getTopLevelGroupIds", String.class) + TypedQuery groupsQuery = em.createNamedQuery("getTopLevelGroupIds", String.class) .setParameter("realm", realm.getId()) - .setParameter("parent", GroupEntity.TOP_PARENT_ID) - .setFirstResult(first) - .setMaxResults(max) - .getResultStream(); + .setParameter("parent", GroupEntity.TOP_PARENT_ID); - return closing(groupIds.map(realm::getGroupById)); + return closing(paginateQuery(groupsQuery, first, max).getResultStream() + .map(realm::getGroupById) + .sorted(GroupModel.COMPARE_BY_NAME) + ); } @Override @@ -527,9 +586,6 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro } em.createNamedQuery("deleteGroupRoleMappingsByGroup").setParameter("group", groupEntity).executeUpdate(); - RealmEntity realmEntity = em.getReference(RealmEntity.class, realm.getId()); - realmEntity.getGroups().remove(groupEntity); - em.remove(groupEntity); return true; @@ -547,15 +603,12 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro GroupEntity groupEntity = new GroupEntity(); groupEntity.setId(id); groupEntity.setName(name); - RealmEntity realmEntity = em.getReference(RealmEntity.class, realm.getId()); - groupEntity.setRealm(realmEntity.getId()); + groupEntity.setRealm(realm.getId()); groupEntity.setParentId(toParent == null? GroupEntity.TOP_PARENT_ID : toParent.getId()); em.persist(groupEntity); em.flush(); - realmEntity.getGroups().add(groupEntity); - GroupAdapter adapter = new GroupAdapter(realm, em, groupEntity); - return adapter; + return new GroupAdapter(realm, em, groupEntity); } @Override @@ -563,6 +616,13 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro subGroup.setParent(null); } + @Override + public void preRemove(RealmModel realm, RoleModel role) { + // GroupProvider method implementation starts here + em.createNamedQuery("deleteGroupRoleMappingsByRole").setParameter("roleId", role.getId()).executeUpdate(); + // GroupProvider method implementation ends here + } + @Override public ClientModel addClient(RealmModel realm, String clientId) { return addClient(realm, KeycloakModelUtils.generateId(), clientId); @@ -733,10 +793,8 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro TypedQuery query = em.createNamedQuery("getGroupIdsByNameContaining", String.class) .setParameter("realm", realm.getId()) .setParameter("search", search); - if(Objects.nonNull(first) && Objects.nonNull(max)) { - query= query.setFirstResult(first).setMaxResults(max); - } - Stream groups = query.getResultStream(); + + Stream groups = paginateQuery(query, first, max).getResultStream(); return closing(groups.map(id -> { GroupModel groupById = session.groups().getGroupById(realm, id); @@ -744,7 +802,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, GroupPro groupById = session.groups().getGroupById(realm, groupById.getParentId()); } return groupById; - }).sorted(Comparator.comparing(GroupModel::getName)).distinct()); + }).sorted(GroupModel.COMPARE_BY_NAME).distinct()); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 1ef1252dbb..2d43a8d67d 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -789,38 +789,27 @@ public class RealmAdapter implements RealmModel, JpaModel { @Override public Stream getDefaultGroupsStream() { - Collection entities = realm.getDefaultGroups(); - if (entities == null || entities.isEmpty()) return Stream.empty(); - return entities.stream().map(GroupEntity::getId).map(this::getGroupById); + return realm.getDefaultGroupIds().stream().map(this::getGroupById); } @Override public void addDefaultGroup(GroupModel group) { - Collection entities = realm.getDefaultGroups(); - for (GroupEntity entity : entities) { - if (entity.getId().equals(group.getId())) return; - } - GroupEntity groupEntity = GroupAdapter.toEntity(group, em); - realm.getDefaultGroups().add(groupEntity); - em.flush(); + Collection groupsIds = realm.getDefaultGroupIds(); + if (groupsIds.contains(group.getId())) return; + groupsIds.add(group.getId()); + em.flush(); } @Override public void removeDefaultGroup(GroupModel group) { - GroupEntity found = null; - for (GroupEntity defaultGroup : realm.getDefaultGroups()) { - if (defaultGroup.getId().equals(group.getId())) { - found = defaultGroup; - break; - } - } - if (found != null) { - realm.getDefaultGroups().remove(found); + Collection groupIds = realm.getDefaultGroupIds(); + + if (groupIds.remove(group.getId())) { em.flush(); } - } + @Override public Stream getClientsStream() { return session.clients().getClientsStream(this); 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 3b8b62a62b..c7d141d3f4 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 @@ -43,12 +43,14 @@ import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; import java.util.Objects; +import java.util.stream.Collectors; import java.util.stream.Stream; import javax.persistence.LockModeType; @@ -356,7 +358,7 @@ public class UserAdapter implements UserModel, JpaModel { user.setEmailVerified(verified); } - private TypedQuery createGetGroupsQuery(String search, Integer first, Integer max) { + private TypedQuery createGetGroupsQuery() { // we query ids only as the group might be cached and following the @ManyToOne will result in a load // even if we're getting just the id. CriteriaBuilder builder = em.getCriteriaBuilder(); @@ -365,23 +367,14 @@ public class UserAdapter implements UserModel, JpaModel { List predicates = new ArrayList<>(); predicates.add(builder.equal(root.get("user"), getEntity())); - Join join = root.join("group"); - if (Objects.nonNull(search) && !search.isEmpty()) { - predicates.add(builder.like(builder.lower(join.get("name")), builder.lower(builder.literal("%" + search + "%")))); - } queryBuilder.select(root.get("groupId")); queryBuilder.where(predicates.toArray(new Predicate[0])); - queryBuilder.orderBy(builder.asc(join.get("name"))); - TypedQuery query = em.createQuery(queryBuilder); - if (Objects.nonNull(first) && Objects.nonNull(max)) { - query.setFirstResult(first).setMaxResults(max); - } - return query; + return em.createQuery(queryBuilder); } - private TypedQuery createCountGroupsQuery(String search) { + private TypedQuery createCountGroupsQuery() { // we query ids only as the group might be cached and following the @ManyToOne will result in a load // even if we're getting just the id. CriteriaBuilder builder = em.getCriteriaBuilder(); @@ -390,38 +383,31 @@ public class UserAdapter implements UserModel, JpaModel { List predicates = new ArrayList<>(); predicates.add(builder.equal(root.get("user"), getEntity())); - if (Objects.nonNull(search) && !search.isEmpty()) { - Join join = root.join("group"); - predicates.add(builder.like(join.get("name"), builder.literal("%" + search + "%"))); - } queryBuilder.select(builder.count(root)); queryBuilder.where(predicates.toArray(new Predicate[0])); return em.createQuery(queryBuilder); } - private Stream getGroupModels(Stream groupIds) { - return groupIds.map(realm::getGroupById); - } - @Override public Stream getGroupsStream() { - return closing(getGroupModels(createGetGroupsQuery(null, null, null).getResultStream())); + return getGroupsStream(null, null, null); } @Override - public Stream getGroupsStream(String search, int first, int max) { - return closing(getGroupModels(createGetGroupsQuery(search, first, max).getResultStream())); + public Stream getGroupsStream(String search, Integer first, Integer max) { + return session.groups().getGroupsStream(realm, closing(createGetGroupsQuery().getResultStream()), search, first, max); } @Override public long getGroupsCount() { - return createCountGroupsQuery(null).getSingleResult(); + return createCountGroupsQuery().getSingleResult(); } @Override public long getGroupsCountByNameContaining(String search) { - return createCountGroupsQuery(search).getSingleResult(); + if (search == null) return getGroupsCount(); + return session.groups().getGroupsCount(realm, closing(createGetGroupsQuery().getResultStream()), search); } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java index 141c5fd31d..adf0164040 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupEntity.java @@ -29,7 +29,11 @@ import java.util.LinkedList; */ @NamedQueries({ @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.parentId = :parent"), + @NamedQuery(name="getGroupIdsByRealm", query="select u.id from GroupEntity u where u.realm = :realm order by u.name ASC"), @NamedQuery(name="getGroupIdsByNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.name like concat('%',:search,'%') order by u.name ASC"), + @NamedQuery(name="getGroupIdsByNameContainingFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids order by u.name ASC"), + @NamedQuery(name="getGroupIdsFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and u.id in :ids order by u.name ASC"), + @NamedQuery(name="getGroupCountByNameContainingFromIdList", query="select count(u) from GroupEntity u where u.realm = :realm and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids"), @NamedQuery(name="getTopLevelGroupIds", query="select u.id from GroupEntity u where u.parentId = :parent and u.realm = :realm order by u.name ASC"), @NamedQuery(name="getGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm"), @NamedQuery(name="getTopLevelGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm and u.parentId = :parent") diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java index be5a166651..b6c2b312f2 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/RealmEntity.java @@ -161,12 +161,10 @@ public class RealmEntity { @JoinTable(name="REALM_DEFAULT_ROLES", joinColumns = { @JoinColumn(name="REALM_ID")}, inverseJoinColumns = { @JoinColumn(name="ROLE_ID")}) protected Collection defaultRoles; - @OneToMany(fetch = FetchType.LAZY, cascade ={CascadeType.REMOVE}, orphanRemoval = true) - @JoinTable(name="REALM_DEFAULT_GROUPS", joinColumns = { @JoinColumn(name="REALM_ID")}, inverseJoinColumns = { @JoinColumn(name="GROUP_ID")}) - protected Collection defaultGroups; - - @OneToMany(fetch = FetchType.LAZY, cascade ={CascadeType.REMOVE}, orphanRemoval = true, mappedBy = "realm") - protected Collection groups; + @ElementCollection + @Column(name="GROUP_ID") + @CollectionTable(name="REALM_DEFAULT_GROUPS", joinColumns={ @JoinColumn(name="REALM_ID") }) + protected Set defaultGroupIds; @Column(name="EVENTS_ENABLED") protected boolean eventsEnabled; @@ -467,26 +465,15 @@ public class RealmEntity { this.defaultRoles = defaultRoles; } - public Collection getDefaultGroups() { - if (defaultGroups == null) { - defaultGroups = new LinkedList<>(); + public Set getDefaultGroupIds() { + if (defaultGroupIds == null) { + defaultGroupIds = new HashSet<>(); } - return defaultGroups; + return defaultGroupIds; } - public void setDefaultGroups(Collection defaultGroups) { - this.defaultGroups = defaultGroups; - } - - public Collection getGroups() { - if (groups == null) { - groups = new LinkedList<>(); - } - return groups; - } - - public void setGroups(Collection groups) { - this.groups = groups; + public void setDefaultGroupIds(Set defaultGroups) { + this.defaultGroupIds = defaultGroups; } public String getPasswordPolicy() { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserGroupMembershipEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserGroupMembershipEntity.java index 4f350c0b44..87c1bdfbe7 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserGroupMembershipEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserGroupMembershipEntity.java @@ -42,8 +42,8 @@ import java.io.Serializable; @NamedQuery(name="deleteUserGroupMembershipsByGroup", query="delete from UserGroupMembershipEntity m where m.groupId = :groupId"), @NamedQuery(name="deleteUserGroupMembershipsByUser", query="delete from UserGroupMembershipEntity m where m.user = :user"), @NamedQuery(name="searchForUserCountInGroups", query="select count(m.user) from UserGroupMembershipEntity m where m.user.realmId = :realmId and (m.user.serviceAccountClientLink is null) and " + - "( lower(m.user.username) like :search or lower(concat(m.user.firstName, ' ', m.user.lastName)) like :search or m.user.email like :search ) and m.group.id in :groupIds"), - @NamedQuery(name="userCountInGroups", query="select count(m.user) from UserGroupMembershipEntity m where m.user.realmId = :realmId and m.group.id in :groupIds") + "( lower(m.user.username) like :search or lower(concat(m.user.firstName, ' ', m.user.lastName)) like :search or m.user.email like :search ) and m.groupId in :groupIds"), + @NamedQuery(name="userCountInGroups", query="select count(m.user) from UserGroupMembershipEntity m where m.user.realmId = :realmId and m.groupId in :groupIds") }) @Table(name="USER_GROUP_MEMBERSHIP") @Entity @@ -55,11 +55,6 @@ public class UserGroupMembershipEntity { @JoinColumn(name="USER_ID") protected UserEntity user; - @Id - @ManyToOne(fetch= FetchType.LAZY) - @JoinColumn(name="GROUP_ID", insertable=false, updatable=false) - protected GroupEntity group; - @Id @Column(name = "GROUP_ID") protected String groupId; diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-12.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-12.0.0.xml new file mode 100644 index 0000000000..b68b8303f2 --- /dev/null +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-12.0.0.xml @@ -0,0 +1,24 @@ + + + + + + + + + diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml index 4317a1ec84..67299d8768 100755 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-master.xml @@ -67,5 +67,6 @@ + diff --git a/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupEntity.java b/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupEntity.java new file mode 100644 index 0000000000..0b68af0320 --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupEntity.java @@ -0,0 +1,134 @@ +/* + * Copyright 2020 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.models.map.group; + +import org.keycloak.models.map.common.AbstractEntity; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +/** + * + * @author mhajas + */ +public abstract class AbstractGroupEntity implements AbstractEntity { + + private final K id; + private final String realmId; + + private String name; + private String parentId; + private Map> attributes = new HashMap<>(); + private Set grantedRoles = new HashSet<>(); + + /** + * Flag signalizing that any of the setters has been meaningfully used. + */ + protected boolean updated; + + protected AbstractGroupEntity() { + this.id = null; + this.realmId = null; + } + + public AbstractGroupEntity(K id, String realmId) { + Objects.requireNonNull(id, "id"); + Objects.requireNonNull(realmId, "realmId"); + + this.id = id; + this.realmId = realmId; + } + + @Override + public K getId() { + return this.id; + } + + @Override + public boolean isUpdated() { + return this.updated; + } + + + public String getName() { + return name; + } + + public void setName(String name) { + this.updated |= ! Objects.equals(this.name, name); + this.name = name; + } + + public String getParentId() { + return parentId; + } + + public void setParentId(String parentId) { + this.updated |= !Objects.equals(this.parentId, parentId); + this.parentId = parentId; + } + + public Map> getAttributes() { + return attributes; + } + + public void setAttributes(Map> attributes) { + this.updated |= ! Objects.equals(this.attributes, attributes); + this.attributes = attributes; + } + + public void setAttribute(String name, List value) { + this.updated |= !this.attributes.containsKey(name) || !this.attributes.get(name).equals(value); + this.attributes.put(name, value); + } + + public void removeAttribute(String name) { + this.updated |= this.attributes.remove(name) != null; + } + + public List getAttribute(String name) { + return this.attributes.get(name); + } + + public String getRealmId() { + return this.realmId; + } + + public Set getGrantedRoles() { + return grantedRoles; + } + + public void setGrantedRoles(Set grantedRoles) { + this.updated |= !Objects.equals(this.grantedRoles, grantedRoles); + this.grantedRoles = grantedRoles; + } + + public void removeRole(String role) { + this.updated |= this.grantedRoles.contains(role); + this.grantedRoles.remove(role); + } + + public void addGrantedRole(String role) { + this.updated |= !this.grantedRoles.contains(role); + this.grantedRoles.add(role); + } +} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupModel.java b/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupModel.java new file mode 100644 index 0000000000..30a345f96c --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/AbstractGroupModel.java @@ -0,0 +1,55 @@ +/* + * Copyright 2020 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.models.map.group; + +import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.map.common.AbstractEntity; + +import java.util.Objects; + +public abstract class AbstractGroupModel implements GroupModel { + + protected final KeycloakSession session; + protected final RealmModel realm; + protected final E entity; + + public AbstractGroupModel(KeycloakSession session, RealmModel realm, E entity) { + Objects.requireNonNull(entity, "entity"); + Objects.requireNonNull(realm, "realm"); + + this.session = session; + this.realm = realm; + this.entity = entity; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof GroupModel)) return false; + + GroupModel that = (GroupModel) o; + return Objects.equals(that.getId(), getId()); + } + + @Override + public int hashCode() { + return getId().hashCode(); + } +} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupAdapter.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupAdapter.java new file mode 100644 index 0000000000..df776bd0c4 --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupAdapter.java @@ -0,0 +1,168 @@ +/* + * Copyright 2020 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.models.map.group; + +import org.keycloak.models.ClientModel; +import org.keycloak.models.GroupModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; + +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Stream; + + +public class MapGroupAdapter extends AbstractGroupModel { + public MapGroupAdapter(KeycloakSession session, RealmModel realm, MapGroupEntity entity) { + super(session, realm, entity); + } + + @Override + public String getId() { + return entity.getId().toString(); + } + + @Override + public String getName() { + return entity.getName(); + } + + @Override + public void setName(String name) { + entity.setName(name); + } + + @Override + public void setSingleAttribute(String name, String value) { + entity.setAttribute(name, Collections.singletonList(value)); + } + + @Override + public void setAttribute(String name, List values) { + entity.setAttribute(name, values); + } + + @Override + public void removeAttribute(String name) { + entity.removeAttribute(name); + } + + @Override + public String getFirstAttribute(String name) { + List attributeValues = this.entity.getAttribute(name); + if (attributeValues == null) { + return null; + } + + return attributeValues.get(0); + } + + @Override + public Stream getAttributeStream(String name) { + List attributes = entity.getAttribute(name); + if (attributes == null || attributes.isEmpty()) return Stream.empty(); + return attributes.stream(); + } + + @Override + public Map> getAttributes() { + return entity.getAttributes(); + } + + @Override + public GroupModel getParent() { + String parentId = getParentId(); + if (parentId == null) { + return null; + } + + return session.groups().getGroupById(realm, parentId); + } + + @Override + public String getParentId() { + return entity.getParentId(); + } + + @Override + public Stream getSubGroupsStream() { + return session.groups().getGroupsStream(realm) + .filter(groupModel -> getId().equals(groupModel.getParentId())); + } + + @Override + public void setParent(GroupModel group) { + if (group == null) { + entity.setParentId(null); + return; + } + + if (!getId().equals(group.getId())) { + entity.setParentId(group.getId()); + } + } + + @Override + public void addChild(GroupModel subGroup) { + subGroup.setParent(this); + } + + @Override + public void removeChild(GroupModel subGroup) { + if (getId().equals(subGroup.getParentId())) { + subGroup.setParent(null); + } + } + + @Override + public Stream getRealmRoleMappingsStream() { + return getRoleMappingsStream() + .filter(roleModel -> roleModel.getContainer() instanceof RealmModel); + } + + @Override + public Stream getClientRoleMappingsStream(ClientModel app) { + final String clientId = app.getId(); + return getRoleMappingsStream() + .filter(roleModel -> roleModel.getContainer() instanceof ClientModel) + .filter(roleModel -> roleModel.getContainer().getId().equals(clientId)); + } + + @Override + public boolean hasRole(RoleModel role) { + return entity.getGrantedRoles().contains(role.getId()); + } + + @Override + public void grantRole(RoleModel role) { + entity.addGrantedRole(role.getId()); + } + + @Override + public Stream getRoleMappingsStream() { + return entity.getGrantedRoles().stream() + .map(roleId -> session.roles().getRoleById(realm, roleId)); + } + + @Override + public void deleteRoleMapping(RoleModel role) { + entity.removeRole(role.getId()); + } +} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java new file mode 100644 index 0000000000..73c789ed93 --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupEntity.java @@ -0,0 +1,31 @@ +/* + * Copyright 2020 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.models.map.group; + +import java.util.UUID; + +public class MapGroupEntity extends AbstractGroupEntity { + + protected MapGroupEntity() { + super(); + } + + public MapGroupEntity(UUID id, String realmId) { + super(id, realmId); + } +} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java new file mode 100644 index 0000000000..091ed5ee4b --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProvider.java @@ -0,0 +1,325 @@ +/* + * Copyright 2020 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.models.map.group; + +import org.jboss.logging.Logger; +import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupProvider; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.ModelDuplicateException; +import org.keycloak.models.RealmModel; +import org.keycloak.models.RoleModel; +import org.keycloak.models.map.common.Serialization; +import org.keycloak.models.map.storage.MapKeycloakTransaction; +import org.keycloak.models.map.storage.MapStorage; + +import java.util.Comparator; +import java.util.Objects; +import java.util.UUID; +import java.util.function.Function; +import java.util.function.Predicate; +import java.util.stream.Stream; + +import static org.keycloak.common.util.StackUtil.getShortStackTrace; + +public class MapGroupProvider implements GroupProvider { + + private static final Logger LOG = Logger.getLogger(MapGroupProvider.class); + private static final Predicate ALWAYS_FALSE = c -> { return false; }; + private final KeycloakSession session; + final MapKeycloakTransaction tx; + private final MapStorage groupStore; + + public MapGroupProvider(KeycloakSession session, MapStorage groupStore) { + this.session = session; + this.groupStore = groupStore; + this.tx = new MapKeycloakTransaction<>(groupStore); + session.getTransactionManager().enlist(tx); + } + + private MapGroupEntity registerEntityForChanges(MapGroupEntity origEntity) { + final MapGroupEntity res = Serialization.from(origEntity); + tx.putIfChanged(origEntity.getId(), res, MapGroupEntity::isUpdated); + return res; + } + + private Function entityToAdapterFunc(RealmModel realm) { + // Clone entity before returning back, to avoid giving away a reference to the live object to the caller + return origEntity -> new MapGroupAdapter(session, realm, registerEntityForChanges(origEntity)); + } + + private Predicate entityRealmFilter(RealmModel realm) { + if (realm == null || realm.getId() == null) { + return MapGroupProvider.ALWAYS_FALSE; + } + String realmId = realm.getId(); + return entity -> Objects.equals(realmId, entity.getRealmId()); + } + + @Override + public GroupModel getGroupById(RealmModel realm, String id) { + if (id == null) { + return null; + } + + LOG.tracef("getGroupById(%s, %s)%s", realm, id, getShortStackTrace()); + + + UUID uid; + try { + uid = UUID.fromString(id); + } catch (IllegalArgumentException ex) { + return null; + } + + MapGroupEntity entity = tx.get(uid, groupStore::get); + return (entity == null || ! entityRealmFilter(realm).test(entity)) + ? null + : entityToAdapterFunc(realm).apply(entity); + } + + private Stream getNotRemovedUpdatedGroupsStream() { + Stream updatedAndNotRemovedGroupsStream = groupStore.entrySet().stream() + .map(tx::getUpdated) // If the group has been removed, tx.get will return null, otherwise it will return me.getValue() + .filter(Objects::nonNull); + return Stream.concat(tx.createdValuesStream(groupStore.keySet()), updatedAndNotRemovedGroupsStream); + } + + private Stream getUnsortedGroupEntitiesStream(RealmModel realm) { + return getNotRemovedUpdatedGroupsStream() + .filter(entityRealmFilter(realm)); + } + + @Override + public Stream getGroupsStream(RealmModel realm) { + LOG.tracef("getGroupsStream(%s)%s", realm, getShortStackTrace()); + return getUnsortedGroupEntitiesStream(realm) + .map(entityToAdapterFunc(realm)) + .sorted(GroupModel.COMPARE_BY_NAME) + ; + } + + @Override + public Stream getGroupsStream(RealmModel realm, Stream ids, String search, Integer first, Integer max) { + Stream groupModelStream = ids.map(id -> session.groups().getGroupById(realm, id)) + .sorted(Comparator.comparing(GroupModel::getName)); + + if (search != null) { + String s = search.toLowerCase(); + groupModelStream = groupModelStream.filter(groupModel -> groupModel.getName().toLowerCase().contains(s)); + } + + if (first != null && first > 0) { + groupModelStream = groupModelStream.skip(first); + } + + if (max != null && max >= 0) { + groupModelStream = groupModelStream.limit(max); + } + + return groupModelStream; + } + + @Override + public Long getGroupsCount(RealmModel realm, Boolean onlyTopGroups) { + LOG.tracef("getGroupsCount(%s, %s)%s", realm, onlyTopGroups, getShortStackTrace()); + Stream groupModelStream = getUnsortedGroupEntitiesStream(realm); + + if (onlyTopGroups) { + groupModelStream = groupModelStream.filter(groupEntity -> Objects.isNull(groupEntity.getParentId())); + } + + return groupModelStream.count(); + } + + @Override + public Long getGroupsCountByNameContaining(RealmModel realm, String search) { + return searchForGroupByNameStream(realm, search, null, null).count(); + } + + @Override + public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) { + LOG.tracef("getGroupsByRole(%s, %s, %d, %d)%s", realm, role, firstResult, maxResults, getShortStackTrace()); + Stream groupModelStream = getGroupsStream(realm).filter(groupModel -> groupModel.hasRole(role)); + + if (firstResult != null && firstResult > 0) { + groupModelStream = groupModelStream.skip(firstResult); + } + + if (maxResults != null && maxResults >= 0) { + groupModelStream = groupModelStream.limit(maxResults); + } + + return groupModelStream; + } + + @Override + public Stream getTopLevelGroupsStream(RealmModel realm) { + LOG.tracef("getTopLevelGroupsStream(%s)%s", realm, getShortStackTrace()); + return getGroupsStream(realm) + .filter(groupModel -> Objects.isNull(groupModel.getParentId())); + } + + @Override + public Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults) { + Stream groupModelStream = getTopLevelGroupsStream(realm); + + if (firstResult != null && firstResult > 0) { + groupModelStream = groupModelStream.skip(firstResult); + } + + if (maxResults != null && maxResults >= 0) { + groupModelStream = groupModelStream.limit(maxResults); + } + + return groupModelStream; + + } + + @Override + public Stream searchForGroupByNameStream(RealmModel realm, String search, Integer firstResult, Integer maxResults) { + LOG.tracef("searchForGroupByNameStream(%s, %s, %d, %d)%s", realm, search, firstResult, maxResults, getShortStackTrace()); + Stream groupModelStream = getGroupsStream(realm) + .filter(groupModel -> groupModel.getName().contains(search)); + + if (firstResult != null && firstResult > 0) { + groupModelStream = groupModelStream.skip(firstResult); + } + + if (maxResults != null && maxResults >= 0) { + groupModelStream = groupModelStream.limit(maxResults); + } + + return groupModelStream; + } + + @Override + public GroupModel createGroup(RealmModel realm, String id, String name, GroupModel toParent) { + LOG.tracef("createGroup(%s, %s, %s, %s)%s", realm, id, name, toParent, getShortStackTrace()); + final UUID entityId = id == null ? UUID.randomUUID() : UUID.fromString(id); + + // Check Db constraint: uniqueConstraints = { @UniqueConstraint(columnNames = {"REALM_ID", "PARENT_GROUP", "NAME"})} + if (getUnsortedGroupEntitiesStream(realm) + .anyMatch(groupEntity -> + Objects.equals(groupEntity.getParentId(), toParent == null ? null : toParent.getId()) && + Objects.equals(groupEntity.getName(), name))) { + throw new ModelDuplicateException("Group with name '" + name + "' in realm " + realm.getName() + " already exists for requested parent" ); + } + + MapGroupEntity entity = new MapGroupEntity(entityId, realm.getId()); + entity.setName(name); + entity.setParentId(toParent == null ? null : toParent.getId()); + if (tx.get(entity.getId(), groupStore::get) != null) { + throw new ModelDuplicateException("Group exists: " + entityId); + } + tx.putIfAbsent(entity.getId(), entity); + + return entityToAdapterFunc(realm).apply(entity); + } + + @Override + public boolean removeGroup(RealmModel realm, GroupModel group) { + LOG.tracef("removeGroup(%s, %s)%s", realm, group, getShortStackTrace()); + if (group == null) return false; + + // TODO: Sending an event (, user group removal and realm default groups) should be extracted to store layer + session.getKeycloakSessionFactory().publish(new GroupModel.GroupRemovedEvent() { + + @Override + public RealmModel getRealm() { + return realm; + } + + @Override + public GroupModel getGroup() { + return group; + } + + @Override + public KeycloakSession getKeycloakSession() { + return session; + } + }); + + session.users().preRemove(realm, group); + realm.removeDefaultGroup(group); + + group.getSubGroupsStream().forEach(subGroup -> session.groups().removeGroup(realm, subGroup)); + + // TODO: ^^^^^^^ Up to here + + tx.remove(UUID.fromString(group.getId())); + + return true; + } + + /* TODO: investigate following two methods, it seems they could be moved to model layer */ + + @Override + public void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent) { + LOG.tracef("moveGroup(%s, %s, %s)%s", realm, group, toParent, getShortStackTrace()); + + if (toParent != null && group.getId().equals(toParent.getId())) { + return; + } + + String parentId = toParent == null ? null : toParent.getId(); + Stream possibleSiblings = getUnsortedGroupEntitiesStream(realm) + .filter(mapGroupEntity -> Objects.equals(mapGroupEntity.getParentId(), parentId)); + + if (possibleSiblings.map(MapGroupEntity::getName).anyMatch(Predicate.isEqual(group.getName()))) { + throw new ModelDuplicateException("Parent already contains subgroup named '" + group.getName() + "'"); + } + + if (group.getParentId() != null) { + group.getParent().removeChild(group); + } + group.setParent(toParent); + if (toParent != null) toParent.addChild(group); + } + + @Override + public void addTopLevelGroup(RealmModel realm, GroupModel subGroup) { + LOG.tracef("addTopLevelGroup(%s, %s)%s", realm, subGroup, getShortStackTrace()); + + Stream possibleSiblings = getUnsortedGroupEntitiesStream(realm) + .filter(mapGroupEntity -> mapGroupEntity.getParentId() == null); + + if (possibleSiblings.map(MapGroupEntity::getName).anyMatch(Predicate.isEqual(subGroup.getName()))) { + throw new ModelDuplicateException("There is already a top level group named '" + subGroup.getName() + "'"); + } + + subGroup.setParent(null); + } + + @Override + public void preRemove(RealmModel realm, RoleModel role) { + LOG.tracef("preRemove(%s, %s)%s", realm, role, getShortStackTrace()); + final String roleId = role.getId(); + getUnsortedGroupEntitiesStream(realm) + .filter(groupEntity -> groupEntity.getGrantedRoles().contains(roleId)) + .map(groupEntity -> session.groups().getGroupById(realm, groupEntity.getId().toString())) + .forEach(groupModel -> groupModel.deleteRoleMapping(role)); + } + + @Override + public void close() { + + } + +} diff --git a/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java new file mode 100644 index 0000000000..838e20d804 --- /dev/null +++ b/model/map/src/main/java/org/keycloak/models/map/group/MapGroupProviderFactory.java @@ -0,0 +1,49 @@ +/* + * Copyright 2020 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.models.map.group; + +import org.keycloak.models.GroupProvider; +import org.keycloak.models.GroupProviderFactory; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.KeycloakSessionFactory; +import org.keycloak.models.map.common.AbstractMapProviderFactory; +import org.keycloak.models.map.storage.MapStorage; +import org.keycloak.models.map.storage.MapStorageProvider; + +import java.util.UUID; + +/** + * + * @author mhajas + */ +public class MapGroupProviderFactory extends AbstractMapProviderFactory implements GroupProviderFactory { + + private MapStorage store; + + @Override + public void postInit(KeycloakSessionFactory factory) { + MapStorageProvider sp = (MapStorageProvider) factory.getProviderFactory(MapStorageProvider.class); + this.store = sp.getStorage("groups", UUID.class, MapGroupEntity.class); + } + + + @Override + public GroupProvider create(KeycloakSession session) { + return new MapGroupProvider(session, store); + } +} diff --git a/model/map/src/main/resources/META-INF/services/org.keycloak.models.GroupProviderFactory b/model/map/src/main/resources/META-INF/services/org.keycloak.models.GroupProviderFactory new file mode 100644 index 0000000000..54f11f477b --- /dev/null +++ b/model/map/src/main/resources/META-INF/services/org.keycloak.models.GroupProviderFactory @@ -0,0 +1,18 @@ +# +# Copyright 2020 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. +# + +org.keycloak.models.map.group.MapGroupProviderFactory \ No newline at end of file diff --git a/server-spi/src/main/java/org/keycloak/models/GroupModel.java b/server-spi/src/main/java/org/keycloak/models/GroupModel.java index 755ea8f223..c7971a9a1d 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java @@ -19,6 +19,7 @@ package org.keycloak.models; import org.keycloak.provider.ProviderEvent; +import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Set; @@ -35,6 +36,9 @@ public interface GroupModel extends RoleMapperModel { GroupModel getGroup(); KeycloakSession getKeycloakSession(); } + + Comparator COMPARE_BY_NAME = Comparator.comparing(GroupModel::getName); + String getId(); String getName(); diff --git a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java index 51333418fe..8a403ab58a 100644 --- a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java @@ -64,6 +64,56 @@ public interface GroupProvider extends Provider, GroupLookupProvider { */ Stream getGroupsStream(RealmModel realm); + /** + * Returns a list of groups with given ids. + * Effectively the same as {@code getGroupsStream(realm, ids, null, null, null)}. + * + * @param realm Realm. + * @param ids List of ids. + * @return List of GroupModels with the specified ids + */ + default Stream getGroupsStream(RealmModel realm, Stream ids) { + return getGroupsStream(realm, ids, null, null, null); + } + + /** + * Returns a paginated stream of groups with given ids and given search value in group names. + * + * @param realm Realm. + * @param ids List of ids. + * @param search Case insensitive string which will be searched for. Ignored if null. + * @param first Index of the first result to return. Ignored if negative or {@code null}. + * @param max Maximum number of results to return. Ignored if negative or {@code null}. + * @return List of desired groups. + */ + Stream getGroupsStream(RealmModel realm, Stream ids, String search, Integer first, Integer max); + + /** + * Returns a paginated list of groups with given ids. + * Effectively the same as {@code getGroupsStream(realm, ids, null, first, max)}. + * + * @param realm Realm. + * @param ids List of ids. + * @param first Index of the first result to return. Ignored if negative or {@code null}. + * @param max Maximum number of results to return. Ignored if negative or {@code null}. + * @return List of GroupModels with the specified ids + */ + default Stream getGroupsStream(RealmModel realm, Stream ids, Integer first, Integer max) { + return getGroupsStream(realm, ids, null, first, max); + } + + /** + * Returns a number of groups that contains the search string in the name + * + * @param realm Realm. + * @param ids List of ids. + * @param search Case insensitive string which will be searched for. Ignored if null. + * @return Number of groups. + */ + default Long getGroupsCount(RealmModel realm, Stream ids, String search) { + return getGroupsStream(realm, ids, search, null, null).count(); + } + /** * Returns a number of groups/top level groups (i.e. groups without parent group) for the given realm. * @@ -77,7 +127,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * Returns number of groups with the given string in name for the given realm. * * @param realm Realm. - * @param search Searched string. + * @param search Case insensitive string which will be searched for. * @return Number of groups with the given string in its name. */ Long getGroupsCountByNameContaining(RealmModel realm, String search); @@ -90,7 +140,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param firstResult First result to return. Ignored if negative. * @param maxResults Maximum number of results to return. Ignored if negative. * @return List of groups with the given role. - * @deprecated Use {@link #getGroupsByRoleStream(RealmModel, RoleModel, int, int)} getGroupsByRoleStream} instead. + * @deprecated Use {@link #getGroupsByRoleStream(RealmModel, RoleModel, Integer, Integer)} getGroupsByRoleStream} instead. */ @Deprecated default List getGroupsByRole(RealmModel realm, RoleModel role, int firstResult, int maxResults) { @@ -102,11 +152,11 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * * @param realm Realm. * @param role Role. - * @param firstResult First result to return. Ignored if negative. - * @param maxResults Maximum number of results to return. Ignored if negative. + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. * @return Stream of groups with the given role. */ - Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, int firstResult, int maxResults); + Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults); /** * Returns all top level groups (i.e. groups without parent group) for the given realm. @@ -132,8 +182,8 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * Returns top level groups (i.e. groups without parent group) for the given realm. * * @param realm Realm. - * @param firstResult First result to return. - * @param maxResults Maximum number of results to return. + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. * @return List of top level groups in the realm. * @deprecated Use {@link #getTopLevelGroupsStream(RealmModel, Integer, Integer)} getTopLevelGroupsStream} instead. */ @@ -146,8 +196,8 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * Returns top level groups (i.e. groups without parent group) for the given realm. * * @param realm Realm. - * @param firstResult First result to return. - * @param maxResults Maximum number of results to return. + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. * @return Stream of top level groups in the realm. */ Stream getTopLevelGroupsStream(RealmModel realm, Integer firstResult, Integer maxResults); @@ -158,6 +208,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * * @param realm Realm. * @param name Name. + * @throws ModelDuplicateException If there is already a top-level group with the given name * @return Model of the created group. */ default GroupModel createGroup(RealmModel realm, String name) { @@ -171,6 +222,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param realm Realm. * @param id Id. * @param name Name. + * @throws ModelDuplicateException If a group with given id already exists or there is a top-level group with the given name * @return Model of the created group */ default GroupModel createGroup(RealmModel realm, String id, String name) { @@ -184,6 +236,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param realm Realm. * @param name Name. * @param toParent Parent group. + * @throws ModelDuplicateException If the toParent group already has a subgroup with the given name * @return Model of the created group. */ default GroupModel createGroup(RealmModel realm, String name, GroupModel toParent) { @@ -197,6 +250,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param id Id, will be generated if {@code null}. * @param name Name. * @param toParent Parent group, or {@code null} if the group is top level group + * @throws ModelDuplicateException If a group with the given id already exists or the toParent group has a subgroup with the given name * @return Model of the created group */ GroupModel createGroup(RealmModel realm, String id, String name, GroupModel toParent); @@ -221,6 +275,7 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @param realm Realm owning this group. * @param group Group to update. * @param toParent New parent group, or {@code null} if we are moving the group to top level group. + * @throws ModelDuplicateException If there is already a group with group.name under the toParent group (or top-level if toParent is null) */ void moveGroup(RealmModel realm, GroupModel group, GroupModel toParent); @@ -229,6 +284,15 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * * @param realm Realm. * @param subGroup Group. + * @throws ModelDuplicateException If there is already a top level group name with the same name */ void addTopLevelGroup(RealmModel realm, GroupModel subGroup); + + /** + * This function is called when a role is removed; this serves for removing references from groups to roles. + * + * @param realm Realm. + * @param role Role which will be removed. + */ + void preRemove(RealmModel realm, RoleModel role); } diff --git a/server-spi/src/main/java/org/keycloak/models/UserModel.java b/server-spi/src/main/java/org/keycloak/models/UserModel.java index 3d2de374f1..2cc40eb4e5 100755 --- a/server-spi/src/main/java/org/keycloak/models/UserModel.java +++ b/server-spi/src/main/java/org/keycloak/models/UserModel.java @@ -137,11 +137,29 @@ public interface UserModel extends RoleMapperModel { .collect(Collectors.toCollection(LinkedHashSet::new)); } - default Stream getGroupsStream(String search, int first, int max) { - return getGroupsStream() - .filter(group -> search == null || group.getName().toLowerCase().contains(search.toLowerCase())) - .skip(first) - .limit(max); + /** + * Returns a paginated stream of groups within this.realm with search in the name + * + * @param search Case insensitive string which will be searched for. Ignored if null. + * @param first Index of first group to return. Ignored if negative or {@code null}. + * @param max Maximum number of records to return. Ignored if negative or {@code null}. + * @return Stream of desired groups. + */ + default Stream getGroupsStream(String search, Integer first, Integer max) { + if (search != null) search = search.toLowerCase(); + final String finalSearch = search; + Stream groupModelStream = getGroupsStream() + .filter(group -> finalSearch == null || group.getName().toLowerCase().contains(finalSearch)); + + if (first != null && first > 0) { + groupModelStream = groupModelStream.skip(first); + } + + if (max != null && max >= 0) { + groupModelStream = groupModelStream.limit(max); + } + + return groupModelStream; } default long getGroupsCount() { diff --git a/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java b/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java index 136ef7b351..150620c7e6 100644 --- a/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java +++ b/server-spi/src/main/java/org/keycloak/storage/group/GroupLookupProvider.java @@ -38,7 +38,7 @@ public interface GroupLookupProvider { * Returns groups with the given string in name for the given realm. * * @param realm Realm. - * @param search Searched string. + * @param search Case sensitive searched string. * @param firstResult First result to return. Ignored if {@code null}. * @param maxResults Maximum number of results to return. Ignored if {@code null}. * @return List of groups with the given string in name. @@ -53,9 +53,9 @@ public interface GroupLookupProvider { * Returns groups with the given string in name for the given realm. * * @param realm Realm. - * @param search Searched string. - * @param firstResult First result to return. Ignored if {@code null}. - * @param maxResults Maximum number of results to return. Ignored if {@code null}. + * @param search Case sensitive searched string. + * @param firstResult First result to return. Ignored if negative or {@code null}. + * @param maxResults Maximum number of results to return. Ignored if negative or {@code null}. * @return Stream of groups with the given string in name. */ Stream searchForGroupByNameStream(RealmModel realm, String search, Integer firstResult, Integer maxResults); diff --git a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java index b38c5a1347..21e1d43cb3 100644 --- a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java +++ b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java @@ -10,6 +10,7 @@ import org.keycloak.common.util.Resteasy; import org.keycloak.forms.login.freemarker.model.UrlBean; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakTransaction; +import org.keycloak.models.ModelDuplicateException; import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.OAuth2ErrorRepresentation; import org.keycloak.services.managers.RealmManager; @@ -107,6 +108,11 @@ public class KeycloakErrorHandler implements ExceptionMapper { if (throwable instanceof JsonParseException) { status = Response.Status.BAD_REQUEST.getStatusCode(); } + + if (throwable instanceof ModelDuplicateException) { + status = Response.Status.CONFLICT.getStatusCode(); + } + return status; } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java index 6f7c62bc22..9149d00c61 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/GroupResource.java @@ -146,10 +146,6 @@ public class GroupResource { public Response addChild(GroupRepresentation rep) { this.auth.groups().requireManage(group); - if (group.getSubGroupsStream().map(GroupModel::getName).anyMatch(Predicate.isEqual(rep.getName()))) { - return ErrorResponse.exists("Parent already contains subgroup named '" + rep.getName() + "'"); - } - Response.ResponseBuilder builder = Response.status(204); GroupModel child = null; if (rep.getId() != null) { diff --git a/services/src/main/java/org/keycloak/storage/GroupStorageManager.java b/services/src/main/java/org/keycloak/storage/GroupStorageManager.java index 9c85932464..c3ca91a5e6 100644 --- a/services/src/main/java/org/keycloak/storage/GroupStorageManager.java +++ b/services/src/main/java/org/keycloak/storage/GroupStorageManager.java @@ -74,6 +74,11 @@ public class GroupStorageManager extends AbstractStorageManager getGroupsStream(RealmModel realm, Stream ids, String search, Integer first, Integer max) { + return session.groupLocalStorage().getGroupsStream(realm, ids, search, first, max); + } + @Override public Long getGroupsCount(RealmModel realm, Boolean onlyTopGroups) { return session.groupLocalStorage().getGroupsCount(realm, onlyTopGroups); @@ -85,7 +90,7 @@ public class GroupStorageManager extends AbstractStorageManager getGroupsByRoleStream(RealmModel realm, RoleModel role, int firstResult, int maxResults) { + public Stream getGroupsByRoleStream(RealmModel realm, RoleModel role, Integer firstResult, Integer maxResults) { return session.groupLocalStorage().getGroupsByRoleStream(realm, role, firstResult, maxResults); } @@ -119,6 +124,11 @@ public class GroupStorageManager extends AbstractStorageManagerBill Burke @@ -176,6 +177,7 @@ public class UserStorageConsentTest extends AbstractServletsAdapterTest { .build("demo").toString(); driver.navigate().to(logoutUri); + waitForPageToLoad(); assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); productPortal.navigateTo(); assertCurrentUrlStartsWithLoginUrlOf(testRealmPage); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java index 8528203499..7006383bd8 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserTest.java @@ -2226,10 +2226,12 @@ public class UserTest extends AbstractAdminTest { } List groups = realm.users().get(userId).groups("-3", 0, 10); + assertThat(realm.users().get(userId).groupsCount("-3").get("count"), is(1L)); assertEquals(1, groups.size()); assertNames(groups, "group-3"); List groups2 = realm.users().get(userId).groups("1", 0, 10); + assertThat(realm.users().get(userId).groupsCount("1").get("count"), is(2L)); assertEquals(2, groups2.size()); assertNames(groups2, "group-1", "group-10"); @@ -2237,6 +2239,7 @@ public class UserTest extends AbstractAdminTest { assertEquals(0, groups3.size()); List groups4 = realm.users().get(userId).groups("gr", 2, 10); + assertThat(realm.users().get(userId).groupsCount("gr").get("count"), is(10L)); assertEquals(8, groups4.size()); List groups5 = realm.users().get(userId).groups("Gr", 2, 10); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java index 7fe84b2821..301f515b6e 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/concurrency/ConcurrencyTest.java @@ -233,7 +233,7 @@ public class ConcurrencyTest extends AbstractConcurrencyTest { c = realm.groups().group(id).toRepresentation(); assertNotNull(c); - assertTrue("Group " + name + " not found in group list", + assertTrue("Group " + name + " [" + id + "] " + " not found in group list", realm.groups().groups().stream() .map(GroupRepresentation::getName) .filter(Objects::nonNull) 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 32b0a42a44..50afcf4d56 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 @@ -515,6 +515,18 @@ public class GroupTest extends AbstractGroupTest { assertNames(group1.getSubGroups(), "mygroup2"); Assert.assertEquals("/mygroup1/mygroup2", group2.getPath()); + assertAdminEvents.clear(); + + // Create top level group with the same name + group = GroupBuilder.create() + .name("mygroup2") + .build(); + GroupRepresentation group3 = createGroup(realm, group); + // Try to move top level "mygroup2" as child of "mygroup1". It should fail as there is already a child group + // of "mygroup1" with name "mygroup2" + response = realm.groups().group(group1.getId()).subGroup(group3); + Assert.assertEquals(409, response.getStatus()); + realm.groups().group(group3.getId()).remove(); // Move "mygroup2" back under parent response = realm.groups().add(group2); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java index 2e29bda656..68f3b8bd23 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPGroupMapperSyncWithGroupsPathTest.java @@ -44,6 +44,9 @@ import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPTestUtils; +import java.util.Set; +import java.util.stream.Collectors; + @FixMethodOrder(MethodSorters.NAME_ASCENDING) public class LDAPGroupMapperSyncWithGroupsPathTest extends AbstractLDAPTest { @@ -92,7 +95,11 @@ public class LDAPGroupMapperSyncWithGroupsPathTest extends AbstractLDAPTest { RealmModel realm = ctx.getRealm(); GroupModel groupsPathGroup = KeycloakModelUtils.findGroupByPath(realm, LDAP_GROUPS_PATH); - groupsPathGroup.getSubGroupsStream().forEach(realm::removeGroup); + + // Subgroup stream needs to be collected, because otherwise we can end up with finding group with id that is + // already removed + groupsPathGroup.getSubGroupsStream().collect(Collectors.toSet()) + .forEach(realm::removeGroup); }); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java index 67b1c682f6..ae0559673a 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java @@ -69,6 +69,7 @@ import javax.ws.rs.core.Response; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.UUID; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; @@ -653,11 +654,12 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest { @Test public void testHardcodedGroupMapper() { + final String uuid = UUID.randomUUID().toString(); testingClient.server().run(session -> { LDAPTestContext ctx = LDAPTestContext.init(session); RealmModel appRealm = ctx.getRealm(); - GroupModel hardcodedGroup = appRealm.createGroup("hardcoded-group", "hardcoded-group"); + GroupModel hardcodedGroup = appRealm.createGroup(uuid, "hardcoded-group"); // assert that user "johnkeycloak" doesn't have hardcoded group UserModel john = session.users().getUserByUsername("johnkeycloak", appRealm); @@ -673,7 +675,7 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest { LDAPTestContext ctx = LDAPTestContext.init(session); RealmModel appRealm = ctx.getRealm(); - GroupModel hardcodedGroup = appRealm.getGroupById("hardcoded-group"); + GroupModel hardcodedGroup = appRealm.getGroupById(uuid); // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName UserModel john = session.users().getUserByUsername("johnkeycloak", appRealm);