From cc2ccc87b049aa37f53d0209675431a5f6ef7c70 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Fri, 21 Jun 2024 08:13:47 -0300 Subject: [PATCH] Filtering organization groups when managing or processing groups Closes #30589 Signed-off-by: Pedro Igor --- .../models/cache/infinispan/GroupAdapter.java | 6 + .../cache/infinispan/RealmCacheSession.java | 11 +- .../models/cache/infinispan/UserAdapter.java | 39 +++- .../infinispan/entities/CachedGroup.java | 7 + ...JpaUpdate26_0_0_OrganizationGroupType.java | 43 ++++ .../org/keycloak/models/jpa/GroupAdapter.java | 5 + .../keycloak/models/jpa/JpaRealmProvider.java | 29 ++- .../org/keycloak/models/jpa/UserAdapter.java | 14 +- .../jpa/entities/GroupAttributeEntity.java | 1 + .../models/jpa/entities/GroupEntity.java | 34 ++- .../entities/OrganizationDomainEntity.java | 5 + .../jpa/entities/OrganizationEntity.java | 3 +- .../jpa/JpaOrganizationProvider.java | 7 +- .../jpa/JpaOrganizationProviderFactory.java | 9 +- .../organization/jpa/OrganizationAdapter.java | 8 +- .../META-INF/jpa-changelog-26.0.0.xml | 17 ++ .../exportimport/util/ExportUtils.java | 4 +- .../keycloak/storage/GroupStorageManager.java | 10 +- .../models/utils/ModelToRepresentation.java | 1 - .../java/org/keycloak/models/GroupModel.java | 31 +++ .../org/keycloak/models/GroupProvider.java | 30 ++- .../admin/resource/OrganizationsResource.java | 4 +- .../organization/utils/Organizations.java | 35 +-- .../admin/OrganizationGroupTest.java | 214 +++++------------- .../organization/admin/OrganizationTest.java | 12 +- .../exportimport/OrganizationExportTest.java | 5 +- .../OrganizationOIDCProtocolMapperTest.java | 41 ++++ .../member/OrganizationMemberTest.java | 12 - 28 files changed, 395 insertions(+), 242 deletions(-) create mode 100644 model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_OrganizationGroupType.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java index b8ef776260..223a4be2e5 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/GroupAdapter.java @@ -290,4 +290,10 @@ public class GroupAdapter implements GroupModel { public boolean escapeSlashesInGroupPath() { return KeycloakModelUtils.escapeSlashesInGroupPath(keycloakSession); } + + @Override + public Type getType() { + if (isUpdated()) return updated.getType(); + return cached.getType(); + } } 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 eeed00f4d0..397ee71638 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 @@ -41,6 +41,7 @@ import org.keycloak.models.ClientProvider; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.ClientScopeProvider; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.GroupProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakTransaction; @@ -1170,8 +1171,8 @@ public class RealmCacheSession implements CacheRealmProvider { } @Override - public GroupModel createGroup(RealmModel realm, String id, String name, GroupModel toParent) { - GroupModel group = getGroupDelegate().createGroup(realm, id, name, toParent); + public GroupModel createGroup(RealmModel realm, String id, Type type, String name, GroupModel toParent) { + GroupModel group = getGroupDelegate().createGroup(realm, id, type, name, toParent); return groupAdded(realm, group, toParent); } @@ -1603,4 +1604,10 @@ public class RealmCacheSession implements CacheRealmProvider { } return null; } + + @Override + public void preRemove(RealmModel realm) { + listInvalidations.add(realm.getId()); + getGroupDelegate().preRemove(realm); + } } 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 3379ad0e49..9d4fae7992 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 @@ -20,6 +20,7 @@ package org.keycloak.models.cache.infinispan; import org.keycloak.credential.CredentialModel; import org.keycloak.models.ClientModel; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; @@ -31,8 +32,8 @@ import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.models.utils.RoleUtils; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -395,19 +396,33 @@ public class UserAdapter implements CachedUserModel { @Override public Stream getGroupsStream() { - if (updated != null) return updated.getGroupsStream(); - Set groups = new LinkedHashSet<>(); - for (String id : cached.getGroups(modelSupplier)) { - GroupModel groupModel = keycloakSession.groups().getGroupById(realm, id); - if (groupModel == null) { - // chance that role was removed, so just delete to persistence and get user invalidated - getDelegateForUpdate(); - return updated.getGroupsStream(); - } - groups.add(groupModel); + Stream result = Stream.empty(); + if (updated != null) { + result = updated.getGroupsStream(); + } else { + Set groups = null; + for (String id : cached.getGroups(modelSupplier)) { + GroupModel groupModel = keycloakSession.groups().getGroupById(realm, id); + if (groupModel == null) { + // chance that role was removed, so just delegate to persistence and get user invalidated + getDelegateForUpdate(); + result = updated.getGroupsStream(); + break; + } else { + if (groups == null) { + groups = new HashSet<>(); + } + groups.add(groupModel); + } + } + + if (groups != null) { + result = groups.stream(); + } } - return groups.stream(); + + return result.filter(g -> Type.REALM.equals(g.getType())).sorted(Comparator.comparing(GroupModel::getName)); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java index b18a8009b4..723023f1eb 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java @@ -19,6 +19,7 @@ package org.keycloak.models.cache.infinispan.entities; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.cache.infinispan.DefaultLazyLoader; @@ -42,6 +43,7 @@ public class CachedGroup extends AbstractRevisioned implements InRealm { private final LazyLoader> roleMappings; private final LazyLoader> subGroups; private final LazyLoader subGroupsCount; + private final Type type; public CachedGroup(Long revision, RealmModel realm, GroupModel group) { super(revision, group.getId()); @@ -52,6 +54,7 @@ public class CachedGroup extends AbstractRevisioned implements InRealm { this.roleMappings = new DefaultLazyLoader<>(source -> source.getRoleMappingsStream().map(RoleModel::getId).collect(Collectors.toSet()), Collections::emptySet); this.subGroups = new DefaultLazyLoader<>(source -> source.getSubGroupsStream().map(GroupModel::getId).collect(Collectors.toSet()), Collections::emptySet); this.subGroupsCount = new DefaultLazyLoader<>(GroupModel::getSubGroupsCount, () -> 0L); + this.type = group.getType(); } public String getRealm() { @@ -85,4 +88,8 @@ public class CachedGroup extends AbstractRevisioned implements InRealm { public Long getSubGroupsCount(Supplier group) { return subGroupsCount.get(group); } + + public Type getType() { + return type; + } } diff --git a/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_OrganizationGroupType.java b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_OrganizationGroupType.java new file mode 100644 index 0000000000..2ae345c1fb --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/custom/JpaUpdate26_0_0_OrganizationGroupType.java @@ -0,0 +1,43 @@ +/* + * Copyright 2024 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.connections.jpa.updater.liquibase.custom; + +import liquibase.exception.CustomChangeException; +import liquibase.statement.core.RawSqlStatement; + +public class JpaUpdate26_0_0_OrganizationGroupType extends CustomKeycloakTask { + + @Override + protected void generateStatementsImpl() throws CustomChangeException { + String groupTable = getTableName("KEYCLOAK_GROUP"); + String orgTable = getTableName("ORG"); + + if ("mariadb".equals(database.getShortName())) { + statements.add(new RawSqlStatement("UPDATE " + groupTable + " SET TYPE = 1 WHERE CONVERT(NAME USING utf8) IN (SELECT CONVERT(ID USING utf8) FROM " + orgTable + ")")); + } else { + statements.add(new RawSqlStatement("UPDATE " + groupTable + " SET TYPE = 1 WHERE NAME IN (SELECT ID FROM " + orgTable + ")")); + } + } + + @Override + protected String getTaskId() { + return "Update type and id for organization groups"; + } + +} + + diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java index 4584b73f63..0593c7e3ce 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java @@ -317,6 +317,11 @@ public class GroupAdapter implements GroupModel , JpaModel { return getRoleMappingsStream().filter(r -> RoleUtils.isClientRole(r, app)); } + @Override + public Type getType() { + return Type.valueOf(group.getType()); + } + @Override public boolean equals(Object o) { if (this == o) return true; 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 55775d5596..ac33baae6b 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -57,6 +57,7 @@ import org.keycloak.models.GroupModel; import org.keycloak.models.GroupModel.GroupCreatedEvent; import org.keycloak.models.GroupModel.GroupPathChangeEvent; import org.keycloak.models.GroupModel.GroupUpdatedEvent; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.GroupProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.ModelDuplicateException; @@ -78,7 +79,6 @@ import org.keycloak.models.jpa.entities.RealmEntity; import org.keycloak.models.jpa.entities.RealmLocalizationTextsEntity; import org.keycloak.models.jpa.entities.RoleEntity; import org.keycloak.models.utils.KeycloakModelUtils; -import org.keycloak.organization.utils.Organizations; import org.keycloak.protocol.oidc.OIDCLoginProtocol; @@ -180,20 +180,21 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc realm.getDefaultGroupIds().clear(); em.flush(); - int num = em.createNamedQuery("deleteGroupRoleMappingsByRealm") - .setParameter("realm", realm.getId()).executeUpdate(); - session.clients().removeClients(adapter); - num = em.createNamedQuery("deleteDefaultClientScopeRealmMappingByRealm") + em.createNamedQuery("deleteDefaultClientScopeRealmMappingByRealm") .setParameter("realm", realm).executeUpdate(); session.clientScopes().removeClientScopes(adapter); session.roles().removeRoles(adapter); - session.groups().getTopLevelGroupsStream(adapter).forEach(Organizations.removeGroup(session, adapter)); + em.createNamedQuery("deleteOrganizationDomainsByRealm") + .setParameter("realmId", realm.getId()).executeUpdate(); + em.createNamedQuery("deleteOrganizationsByRealm") + .setParameter("realmId", realm.getId()).executeUpdate(); + session.groups().preRemove(adapter); - num = em.createNamedQuery("removeClientInitialAccessByRealm") + em.createNamedQuery("removeClientInitialAccessByRealm") .setParameter("realm", realm).executeUpdate(); em.remove(realm); @@ -701,7 +702,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc } @Override - public GroupModel createGroup(RealmModel realm, String id, String name, GroupModel toParent) { + public GroupModel createGroup(RealmModel realm, String id, Type type, String name, GroupModel toParent) { if (id == null) { id = KeycloakModelUtils.generateId(); } else if (GroupEntity.TOP_PARENT_ID.equals(id)) { @@ -713,6 +714,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc groupEntity.setName(name); groupEntity.setRealm(realm.getId()); groupEntity.setParentId(toParent == null? GroupEntity.TOP_PARENT_ID : toParent.getId()); + groupEntity.setType(type == null ? Type.REALM.intValue() : type.intValue()); em.persist(groupEntity); em.flush(); @@ -739,6 +741,16 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc em.createNativeQuery("delete from " + clientScopeMapping + " where ROLE_ID = :role").setParameter("role", role.getId()).executeUpdate(); } + @Override + public void preRemove(RealmModel realm) { + em.createNamedQuery("deleteGroupRoleMappingsByRealm") + .setParameter("realm", realm.getId()).executeUpdate(); + em.createNamedQuery("deleteGroupAttributesByRealm") + .setParameter("realm", realm.getId()).executeUpdate(); + em.createNamedQuery("deleteGroupsByRealm") + .setParameter("realm", realm.getId()).executeUpdate(); + } + @Override public ClientModel addClient(RealmModel realm, String clientId) { return addClient(realm, KeycloakModelUtils.generateId(), clientId); @@ -1164,6 +1176,7 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc List predicates = new ArrayList<>(); predicates.add(builder.equal(root.get("realm"), realm.getId())); + predicates.add(builder.equal(root.get("type"), Type.REALM.intValue())); for (Map.Entry entry : filteredAttributes.entrySet()) { String key = entry.getKey(); 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 d93d32df7e..d7d1d2f2ee 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 @@ -17,6 +17,8 @@ package org.keycloak.models.jpa; +import org.keycloak.common.Profile; +import org.keycloak.common.Profile.Feature; import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.common.util.ObjectUtil; import org.keycloak.credential.UserCredentialManager; @@ -45,6 +47,8 @@ import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Predicate; import jakarta.persistence.criteria.Root; +import org.keycloak.organization.OrganizationProvider; + import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -391,7 +395,15 @@ public class UserAdapter implements UserModel, JpaModel { @Override public long getGroupsCount() { - return createCountGroupsQuery().getSingleResult(); + Long result = createCountGroupsQuery().getSingleResult(); + if (Profile.isFeatureEnabled(Feature.ORGANIZATION)) { + OrganizationProvider provider = session.getProvider(OrganizationProvider.class); + if (result > 0 && provider.getByMember(this) != null) { + // remove from the count the organization group membership + result--; + } + } + return result; } @Override diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupAttributeEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupAttributeEntity.java index 08fcc899b0..c7d2c70056 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupAttributeEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/GroupAttributeEntity.java @@ -37,6 +37,7 @@ import jakarta.persistence.Table; */ @NamedQueries({ @NamedQuery(name="getGroupAttributesByNameAndValue", query="select attr from GroupAttributeEntity attr where attr.name = :name and attr.value = :value"), + @NamedQuery(name="deleteGroupAttributesByRealm", query="delete from GroupAttributeEntity a where a.group IN (select u from GroupEntity u where u.realm=:realm)") }) @Table(name="GROUP_ATTRIBUTE") @Entity 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 b2d8928aba..5f50bcc23a 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 @@ -28,17 +28,18 @@ import java.util.LinkedList; * @version $Revision: 1 $ */ @NamedQueries({ - @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent order by u.name ASC"), - @NamedQuery(name="getGroupIdsByParentAndName", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent and u.name = :search order by u.name ASC"), - @NamedQuery(name="getGroupIdsByParentAndNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.parentId = :parent and lower(u.name) like lower(concat('%',:search,'%')) order by u.name ASC"), - @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 lower(u.name) like lower(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="getGroupIdsByName", query="select u.id from GroupEntity u where u.realm = :realm and u.name = :search 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="getGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm"), - @NamedQuery(name="getGroupCountByParent", query="select count(u) from GroupEntity u where u.realm = :realm and u.parentId = :parent") + @NamedQuery(name="getGroupIdsByParent", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and u.parentId = :parent order by u.name ASC"), + @NamedQuery(name="getGroupIdsByParentAndName", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and u.parentId = :parent and u.name = :search order by u.name ASC"), + @NamedQuery(name="getGroupIdsByParentAndNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and u.parentId = :parent and lower(u.name) like lower(concat('%',:search,'%')) order by u.name ASC"), + @NamedQuery(name="getGroupIdsByRealm", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 order by u.name ASC"), + @NamedQuery(name="getGroupIdsByNameContaining", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and lower(u.name) like lower(concat('%',:search,'%')) order by u.name ASC"), + @NamedQuery(name="getGroupIdsByNameContainingFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids order by u.name ASC"), + @NamedQuery(name="getGroupIdsByName", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 and u.name = :search order by u.name ASC"), + @NamedQuery(name="getGroupIdsFromIdList", query="select u.id from GroupEntity u where u.realm = :realm and u.type = 0 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 u.type = 0 and lower(u.name) like lower(concat('%',:search,'%')) and u.id in :ids"), + @NamedQuery(name="getGroupCount", query="select count(u) from GroupEntity u where u.realm = :realm and u.type = 0"), + @NamedQuery(name="getGroupCountByParent", query="select count(u) from GroupEntity u where u.realm = :realm and u.type = 0 and u.parentId = :parent"), + @NamedQuery(name="deleteGroupsByRealm", query="delete from GroupEntity g where g.realm = :realm") }) @Entity @Table(name="KEYCLOAK_GROUP", @@ -66,6 +67,9 @@ public class GroupEntity { @Column(name = "REALM_ID") private String realm; + @Column(name = "TYPE") + private int type; + @OneToMany( cascade = CascadeType.REMOVE, orphanRemoval = true, mappedBy="group") @@ -114,6 +118,14 @@ public class GroupEntity { this.parentId = parentId; } + public int getType() { + return type; + } + + public void setType(int type) { + this.type = type; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java index cbc95c776b..5a57b6c6b4 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationDomainEntity.java @@ -25,6 +25,8 @@ import jakarta.persistence.FetchType; import jakarta.persistence.Id; import jakarta.persistence.JoinColumn; import jakarta.persistence.ManyToOne; +import jakarta.persistence.NamedQueries; +import jakarta.persistence.NamedQuery; import jakarta.persistence.Table; /** @@ -34,6 +36,9 @@ import jakarta.persistence.Table; */ @Entity @Table(name="ORG_DOMAIN") +@NamedQueries({ + @NamedQuery(name="deleteOrganizationDomainsByRealm", query="delete from OrganizationDomainEntity d where d.organization IN (select o from OrganizationEntity o where o.realmId=:realmId)") +}) public class OrganizationDomainEntity { @Id diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java index db7302ca77..e41a5da5fd 100644 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/OrganizationEntity.java @@ -43,7 +43,8 @@ import jakarta.persistence.Table; " where o.realmId = :realmId AND (o.name = :search OR d.name = :search) order by o.name ASC"), @NamedQuery(name="getByNameOrDomainContained", query="select distinct o from OrganizationEntity o inner join OrganizationDomainEntity d ON o.id = d.organization.id" + " where o.realmId = :realmId AND (lower(o.name) like concat('%',:search,'%') OR d.name like concat('%',:search,'%')) order by o.name ASC"), - @NamedQuery(name="getCount", query="select count(o) from OrganizationEntity o where o.realmId = :realmId") + @NamedQuery(name="getCount", query="select count(o) from OrganizationEntity o where o.realmId = :realmId"), + @NamedQuery(name="deleteOrganizationsByRealm", query="delete from OrganizationEntity o where o.realmId = :realmId") }) public class OrganizationEntity { diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java index f96d452190..7b58d979ff 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProvider.java @@ -39,6 +39,7 @@ import jakarta.persistence.criteria.Root; import org.keycloak.connections.jpa.JpaConnectionProvider; import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.GroupProvider; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; @@ -451,11 +452,7 @@ public class JpaOrganizationProvider implements OrganizationProvider { } private GroupModel createOrganizationGroup(String orgId) { - GroupModel group = groupProvider.createGroup(getRealm(), null, orgId); - - group.setSingleAttribute(ORGANIZATION_ATTRIBUTE, orgId); - - return group; + return groupProvider.createGroup(getRealm(), null, Type.ORGANIZATION, orgId, null); } private GroupModel getOrganizationGroup(OrganizationModel organization) { diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProviderFactory.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProviderFactory.java index 4e24104aec..00c1c4619a 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProviderFactory.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/JpaOrganizationProviderFactory.java @@ -38,6 +38,8 @@ import org.keycloak.provider.ProviderEvent; public class JpaOrganizationProviderFactory implements OrganizationProviderFactory { + public static final String ID = "jpa"; + @Override public OrganizationProvider create(KeycloakSession session) { return new JpaOrganizationProvider(session); @@ -60,7 +62,7 @@ public class JpaOrganizationProviderFactory implements OrganizationProviderFacto @Override public String getId() { - return "jpa"; + return ID; } private void handleEvents(ProviderEvent event) { @@ -68,11 +70,6 @@ public class JpaOrganizationProviderFactory implements OrganizationProviderFacto RealmModel realm = ((RealmPostCreateEvent) event).getCreatedRealm(); configureAuthenticationFlows(realm); } - if (event instanceof RealmRemovedEvent) { - KeycloakSession session = ((RealmRemovedEvent) event).getKeycloakSession(); - OrganizationProvider provider = session.getProvider(OrganizationProvider.class); - provider.removeAll(); - } if (event instanceof GroupEvent) { GroupEvent groupEvent = (GroupEvent) event; KeycloakSession session = groupEvent.getKeycloakSession(); diff --git a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java index 8252ab881c..4744bbdd87 100644 --- a/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/organization/jpa/OrganizationAdapter.java @@ -75,7 +75,7 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel attrsToRemove = getAttributes().keySet(); attrsToRemove.removeAll(attributes.keySet()); attrsToRemove.forEach(group::removeAttribute); @@ -148,9 +146,7 @@ public final class OrganizationAdapter implements OrganizationModel, JpaModel> getAttributes() { if (attributes == null) { - attributes = new HashMap<>(ofNullable(getGroup().getAttributes()).orElse(Map.of())); - // do not expose the kc.org attribute - attributes.remove(OrganizationModel.ORGANIZATION_ATTRIBUTE); + attributes = ofNullable(getGroup().getAttributes()).orElse(Map.of()); } return attributes; } diff --git a/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml b/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml index 13c20ceafe..b5f0c7d026 100644 --- a/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml +++ b/model/jpa/src/main/resources/META-INF/jpa-changelog-26.0.0.xml @@ -28,4 +28,21 @@ + + + + + + + + + + + + + + + + + diff --git a/model/storage-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java b/model/storage-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java index df9a41b06a..4a6a684212 100755 --- a/model/storage-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java +++ b/model/storage-private/src/main/java/org/keycloak/exportimport/util/ExportUtils.java @@ -31,6 +31,8 @@ import org.keycloak.exportimport.ExportOptions; import org.keycloak.models.ClientModel; import org.keycloak.models.ClientScopeModel; import org.keycloak.models.FederatedIdentityModel; +import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; @@ -458,7 +460,7 @@ public class ExportUtils { if (options.isGroupsAndRolesIncluded()) { List groups = user.getGroupsStream() - .filter(g -> !g.getAttributes().containsKey(OrganizationModel.ORGANIZATION_ATTRIBUTE)) + .filter(g -> Type.REALM.equals(g.getType())) .map(ModelToRepresentation::buildGroupPath).collect(Collectors.toList()); userRep.setGroups(groups); } diff --git a/model/storage-private/src/main/java/org/keycloak/storage/GroupStorageManager.java b/model/storage-private/src/main/java/org/keycloak/storage/GroupStorageManager.java index 05c9874a9c..4246495b64 100644 --- a/model/storage-private/src/main/java/org/keycloak/storage/GroupStorageManager.java +++ b/model/storage-private/src/main/java/org/keycloak/storage/GroupStorageManager.java @@ -19,6 +19,7 @@ package org.keycloak.storage; import java.util.Map; import java.util.stream.Stream; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.GroupProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; @@ -119,8 +120,8 @@ public class GroupStorageManager extends AbstractStorageManager toGroupHierarchy(KeycloakSession session, RealmModel realm, boolean full) { return session.groups().getTopLevelGroupsStream(realm, null, null) - .filter(g -> !g.getAttributes().containsKey(OrganizationModel.ORGANIZATION_ATTRIBUTE)) .map(g -> toGroupHierarchy(g, full)); } 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 06299267d3..22d3ab97a7 100755 --- a/server-spi/src/main/java/org/keycloak/models/GroupModel.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupModel.java @@ -30,6 +30,33 @@ import java.util.stream.Stream; */ public interface GroupModel extends RoleMapperModel { + enum Type { + REALM(0), + ORGANIZATION(1); + + private final int value; + + Type(int value) { + this.value = value; + } + + public static Type valueOf(int value) { + Type[] values = values(); + + for (int i = 0; i < values.length; i++) { + if (values[i].value == value) { + return values[i]; + } + } + + throw new IllegalArgumentException("No type found with value " + value); + } + + public int intValue() { + return value; + } + } + interface GroupEvent extends ProviderEvent { RealmModel getRealm(); GroupModel getGroup(); @@ -292,4 +319,8 @@ public interface GroupModel extends RoleMapperModel { default boolean escapeSlashesInGroupPath() { return GroupProvider.DEFAULT_ESCAPE_SLASHES; } + + default Type getType() { + return Type.REALM; + } } 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 e6ed5835b8..e1bd6524d8 100644 --- a/server-spi/src/main/java/org/keycloak/models/GroupProvider.java +++ b/server-spi/src/main/java/org/keycloak/models/GroupProvider.java @@ -17,6 +17,7 @@ package org.keycloak.models; +import org.keycloak.models.GroupModel.Type; import org.keycloak.provider.Provider; import org.keycloak.storage.group.GroupLookupProvider; @@ -158,7 +159,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 + * @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) { @@ -172,7 +173,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 + * @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) { @@ -203,7 +204,22 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @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); + default GroupModel createGroup(RealmModel realm, String id, String name, GroupModel toParent) { + return createGroup(realm, id, Type.REALM, name, toParent); + } + + /** + * Creates a new group with the given name, id, name and parent to the given realm. + * + * @param realm Realm. + * @param id Id, will be generated if {@code null}. + * @param type the group type. if not set, defaults to {@link Type#REALM} + * @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, Type type, String name, GroupModel toParent); /** * Removes the given group for the given realm. @@ -237,4 +253,12 @@ public interface GroupProvider extends Provider, GroupLookupProvider { * @throws ModelDuplicateException If there is already a top level group name with the same name */ void addTopLevelGroup(RealmModel realm, GroupModel subGroup); + + /** + * Called when a realm is removed. + * Should remove all groups that belong to the realm. + * + * @param realm a reference to the realm + */ + void preRemove(RealmModel realm); } diff --git a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java index 0f91521308..4c63695f09 100644 --- a/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java +++ b/services/src/main/java/org/keycloak/organization/admin/resource/OrganizationsResource.java @@ -134,9 +134,9 @@ public class OrganizationsResource { // check if are searching orgs by attribute. if (StringUtil.isNotBlank(searchQuery)) { Map attributes = SearchQueryUtils.getFields(searchQuery); - return provider.getAllStream(attributes, first, max).map(Organizations::toRepresentation); + return provider.getAllStream(attributes, first, max).map(Organizations::toBriefRepresentation); } else { - return provider.getAllStream(search, exact, first, max).map(Organizations::toRepresentation); + return provider.getAllStream(search, exact, first, max).map(Organizations::toBriefRepresentation); } } diff --git a/services/src/main/java/org/keycloak/organization/utils/Organizations.java b/services/src/main/java/org/keycloak/organization/utils/Organizations.java index 82720c1840..4c7485aa3b 100644 --- a/services/src/main/java/org/keycloak/organization/utils/Organizations.java +++ b/services/src/main/java/org/keycloak/organization/utils/Organizations.java @@ -36,6 +36,7 @@ import org.keycloak.http.HttpRequest; import org.keycloak.models.Constants; import org.keycloak.models.FederatedIdentityModel; import org.keycloak.models.GroupModel; +import org.keycloak.models.GroupModel.Type; import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.KeycloakSession; import org.keycloak.models.OrganizationDomainModel; @@ -51,16 +52,14 @@ import org.keycloak.utils.StringUtil; public class Organizations { public static boolean canManageOrganizationGroup(KeycloakSession session, GroupModel group) { + if (!Type.ORGANIZATION.equals(group.getType())) { + return true; + } + if (Profile.isFeatureEnabled(Feature.ORGANIZATION)) { - Object organization = session.getAttribute(OrganizationModel.class.getName()); + OrganizationModel organization = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName()); - if (organization != null) { - return true; - } - - String orgId = group.getFirstAttribute(OrganizationModel.ORGANIZATION_ATTRIBUTE); - - return StringUtil.isBlank(orgId); + return organization != null && organization.getId().equals(group.getName()); } return true; @@ -101,7 +100,7 @@ public class Organizations { public static Consumer removeGroup(KeycloakSession session, RealmModel realm) { return group -> { - if (!Profile.isFeatureEnabled(Feature.ORGANIZATION)) { + if (!Type.ORGANIZATION.equals(group.getType())) { realm.removeGroup(group); return; } @@ -109,12 +108,9 @@ public class Organizations { OrganizationModel current = (OrganizationModel) session.getAttribute(OrganizationModel.class.getName()); try { - String orgId = group.getFirstAttribute(OrganizationModel.ORGANIZATION_ATTRIBUTE); OrganizationProvider provider = session.getProvider(OrganizationProvider.class); - if (orgId != null) { - session.setAttribute(OrganizationModel.class.getName(), provider.getById(orgId)); - } + session.setAttribute(OrganizationModel.class.getName(), provider.getById(group.getName())); realm.removeGroup(group); } finally { @@ -138,6 +134,18 @@ public class Organizations { } public static OrganizationRepresentation toRepresentation(OrganizationModel model) { + OrganizationRepresentation rep = toBriefRepresentation(model); + + if (rep == null) { + return null; + } + + rep.setAttributes(model.getAttributes()); + + return rep; + } + + public static OrganizationRepresentation toBriefRepresentation(OrganizationModel model) { if (model == null) { return null; } @@ -149,7 +157,6 @@ public class Organizations { rep.setAlias(model.getAlias()); rep.setEnabled(model.isEnabled()); rep.setDescription(model.getDescription()); - rep.setAttributes(model.getAttributes()); model.getDomains().filter(Objects::nonNull).map(Organizations::toRepresentation) .forEach(rep::addDomain); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java index 7ec1fe987d..8500aa0ac5 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationGroupTest.java @@ -19,19 +19,16 @@ package org.keycloak.testsuite.organization.admin; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import static org.keycloak.testsuite.admin.group.GroupSearchTest.buildSearchQuery; -import java.util.ArrayList; -import java.util.List; +import java.util.HashSet; import java.util.Map; +import java.util.Set; -import jakarta.ws.rs.BadRequestException; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.core.Response.Status; import org.junit.Test; -import org.keycloak.admin.client.resource.GroupResource; -import org.keycloak.admin.client.resource.OrganizationResource; +import org.keycloak.admin.client.resource.UserResource; import org.keycloak.common.Profile.Feature; import org.keycloak.models.GroupModel; import org.keycloak.models.ModelValidationException; @@ -39,8 +36,9 @@ import org.keycloak.models.OrganizationModel; import org.keycloak.models.RealmModel; import org.keycloak.models.UserModel; import org.keycloak.organization.OrganizationProvider; +import org.keycloak.organization.jpa.JpaOrganizationProviderFactory; +import org.keycloak.organization.jpa.OrganizationAdapter; import org.keycloak.representations.idm.GroupRepresentation; -import org.keycloak.representations.idm.ManagementPermissionRepresentation; import org.keycloak.representations.idm.OrganizationRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; @@ -49,167 +47,39 @@ import org.keycloak.testsuite.runonserver.RunOnServer; @EnableFeature(Feature.ORGANIZATION) public class OrganizationGroupTest extends AbstractOrganizationTest { - @Test - public void testManageOrgGroupsViaDifferentAPIs() { - // test realm contains some groups initially - List getAllBefore = testRealm().groups().groups(); - long countBefore = testRealm().groups().count().get("count"); - - List orgIds = new ArrayList<>(); - // create 5 organizations - for (int i = 0; i < 5; i++) { - OrganizationRepresentation expected = createOrganization("myorg" + i); - OrganizationResource organization = testRealm().organizations().get(expected.getId()); - expected.setAttributes(Map.of()); - organization.update(expected).close(); - OrganizationRepresentation existing = organization.toRepresentation(); - orgIds.add(expected.getId()); - assertNotNull(existing); - } - - // create one top-level group and one subgroup - GroupRepresentation topGroup = createGroup(testRealm(), "top"); - GroupRepresentation level2Group = new GroupRepresentation(); - level2Group.setName("level2"); - testRealm().groups().group(topGroup.getId()).subGroup(level2Group); - - // check that count queries include org related groups - assertEquals(countBefore + 7, (long) testRealm().groups().count().get("count")); - - // check that search queries include org related groups but those can't be updated - assertEquals(getAllBefore.size() + 6, testRealm().groups().groups().size()); - // we need to pull full representation of the group, otherwise org related attributes are lost in the representation - List groups = testRealm().groups().query(buildSearchQuery(OrganizationModel.ORGANIZATION_ATTRIBUTE, orgIds.get(0)), false, 0, 10, false); - assertEquals(1, groups.size()); - GroupRepresentation orgGroupRep = groups.get(0); - GroupResource group = testRealm().groups().group(orgGroupRep.getId()); - - try { - // group to be updated is organization related group - group.update(topGroup); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success, the group could not be updated - } - - try { - // cannot update a group with the attribute reserved for organization related groups - testRealm().groups().group(topGroup.getId()).update(orgGroupRep); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success, the group could not be updated - } - - try { - // cannot remove organization related group - group.remove(); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success, the group could not be removed - } - - try { - // cannot manage organization related group permissions - group.setPermissions(new ManagementPermissionRepresentation(true)); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success, the group's permissions cannot be managed - } - - // try to add subgroup to an org related group - try (Response response = group.subGroup(topGroup)) { - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); - } - - // try to add org related group as a subgroup to a group - try (Response response = testRealm().groups().group(topGroup.getId()).subGroup(orgGroupRep)) { - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); - } - - try { - // cannot manage organization related group role mappers - group.roles().realmLevel().add(null); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - - try { - // cannot manage organization related group role mappers - group.roles().realmLevel().remove(null); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - - try { - // cannot manage organization related group role mappers - group.roles().clientLevel(testRealm().clients().findByClientId("test-app").get(0).getId()).add(null); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - - try { - // cannot manage organization related group role mappers - group.roles().clientLevel(testRealm().clients().findByClientId("test-app").get(0).getId()).remove(null); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - - // cannot add top level group with reserved attribute for organizations - String id = orgGroupRep.getId(); - String name = orgGroupRep.getName(); - orgGroupRep.setId(null); - orgGroupRep.setName(null); - try (Response response = testRealm().groups().add(orgGroupRep)) { - assertEquals(Status.BAD_REQUEST.getStatusCode(), response.getStatus()); - } finally { - orgGroupRep.setId(id); - orgGroupRep.setName(name); - } - - try { - // cannot add organization related group as a default group - testRealm().addDefaultGroup(orgGroupRep.getId()); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - - OrganizationRepresentation org = createOrganization(); - UserRepresentation userRep = addMember(testRealm().organizations().get(org.getId())); - - try { - // cannot join organization related group - testRealm().users().get(userRep.getId()).joinGroup(orgGroupRep.getId()); - fail("Expected BadRequestException"); - } catch (BadRequestException ex) { - // success - } - } - @Test public void testManagingOrganizationGroupNotInOrganizationScope() { String id = createOrganization().getId(); String memberId = addMember(testRealm().organizations().get(id)).getId(); getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) session -> { - OrganizationProvider provider = session.getProvider(OrganizationProvider.class); - OrganizationModel organization = provider.getById(id); + OrganizationProvider provider = session.getProvider(OrganizationProvider.class, JpaOrganizationProviderFactory.ID); + OrganizationAdapter organization = (OrganizationAdapter) provider.getById(id); RealmModel realm = session.getContext().getRealm(); - GroupModel orgGroup = session.groups().getGroupByName(realm, null, organization.getId()); + GroupModel orgGroup = session.groups().getGroupById(realm, organization.getGroupId()); + assertNotNull(orgGroup); try { orgGroup.setName("fail"); fail("can not manage"); } catch (ModelValidationException ignore) { + try { + orgGroup.setName(organization.getId()); + } catch (ModelValidationException ignore2) {} } try { orgGroup.setSingleAttribute(OrganizationModel.ORGANIZATION_ATTRIBUTE, "fail"); fail("can not manage"); + } catch (ModelValidationException ignore) { + try { + orgGroup.setSingleAttribute(OrganizationModel.ORGANIZATION_ATTRIBUTE, organization.getId()); + } catch (ModelValidationException ignore2) {} + } + + try { + orgGroup.setSingleAttribute("something", "fail"); + fail("can not manage"); } catch (ModelValidationException ignore) { } @@ -259,6 +129,44 @@ public class OrganizationGroupTest extends AbstractOrganizationTest { }); } + @Test + public void testOrganizationGroupsNotAvailableFromGroupAPI() { + Set orgIds = new HashSet<>(); + + for (int i = 0; i < 10; i++) { + orgIds.add(createOrganization("org-" + i).getId()); + } + + assertEquals(orgIds.size(), testRealm().organizations().getAll().size()); + assertTrue(testRealm().groups().groups().stream().map(GroupRepresentation::getId).noneMatch(orgIds::contains)); + } + + @Test + public void testOrganizationGroupsNotAvailableFromUserAPI() { + OrganizationRepresentation organization = createOrganization(); + UserRepresentation member = addMember(testRealm().organizations().get(organization.getId())); + UserResource userResource = testRealm().users().get(member.getId()); + assertTrue(userResource.groups().isEmpty()); + assertEquals(0, userResource.groupsCount(null).get("count").intValue()); + assertEquals(0, userResource.groupsCount(organization.getId()).get("count").intValue()); + } + + @Test + public void testDeleteGroupOnOrganizationRemoval() { + String id = createOrganization().getId(); + + getTestingClient().server(TEST_REALM_NAME).run((RunOnServer) session -> { + OrganizationProvider provider = session.getProvider(OrganizationProvider.class, JpaOrganizationProviderFactory.ID); + OrganizationAdapter organization = (OrganizationAdapter) provider.getById(id); + RealmModel realm = session.getContext().getRealm(); + GroupModel group = session.groups().getGroupById(realm, organization.getGroupId()); + assertNotNull(group); + provider.remove(organization); + group = session.groups().getGroupById(realm, organization.getId()); + assertNull(group); + }); + } + @Override protected OrganizationRepresentation createRepresentation(String name, String... orgDomains) { OrganizationRepresentation rep = super.createRepresentation(name, orgDomains); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java index 22f9367da3..7196fde82d 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationTest.java @@ -36,6 +36,8 @@ import java.util.ArrayList; import java.util.Comparator; import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import jakarta.ws.rs.NotFoundException; @@ -43,6 +45,8 @@ import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response.Status; import java.io.IOException; import java.util.stream.IntStream; + +import org.junit.Assert; import org.junit.Test; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.admin.client.resource.OrganizationsResource; @@ -104,12 +108,16 @@ public class OrganizationTest extends AbstractOrganizationTest { List expected = new ArrayList<>(); for (int i = 0; i < 5; i++) { - expected.add(createOrganization("kc.org." + i)); + OrganizationRepresentation organization = createOrganization("kc.org." + i); + expected.add(organization); + organization.setAttributes(Map.of("foo", List.of("foo"))); + testRealm().organizations().get(organization.getId()).update(organization).close(); } List existing = testRealm().organizations().getAll(); assertFalse(existing.isEmpty()); assertThat(expected, containsInAnyOrder(existing.toArray())); + Assert.assertTrue(existing.stream().map(OrganizationRepresentation::getAttributes).filter(Objects::nonNull).findAny().isEmpty()); } @Test @@ -129,6 +137,7 @@ public class OrganizationTest extends AbstractOrganizationTest { assertThat(orgRep.getDomains(), hasSize(2)); assertThat(orgRep.getDomain("wayneind.com"), not(nullValue())); assertThat(orgRep.getDomain("wayneind-gotham.com"), not(nullValue())); + assertThat(orgRep.getAttributes(), nullValue()); existing = testRealm().organizations().search("gtbank.net", true, 0, 10); assertThat(existing, hasSize(1)); @@ -138,6 +147,7 @@ public class OrganizationTest extends AbstractOrganizationTest { assertThat(orgRep.getDomains(), hasSize(2)); assertThat(orgRep.getDomain("gtbank.com"), not(nullValue())); assertThat(orgRep.getDomain("gtbank.net"), not(nullValue())); + assertThat(orgRep.getAttributes(), nullValue()); existing = testRealm().organizations().search("nonexistent.org", true, 0, 10); assertThat(existing, is(empty())); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java index 799eb8c5ae..26bf95045f 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/exportimport/OrganizationExportTest.java @@ -163,7 +163,10 @@ public class OrganizationExportTest extends AbstractOrganizationTest { exportImport.setAction(ExportImportConfig.ACTION_IMPORT); exportImport.setFile(targetFilePath); exportImport.runImport(); - getCleanup().addCleanup(() -> testRealm().remove()); + getCleanup().addCleanup(() -> { + testRealm().remove(); + getTestContext().getTestRealmReps().clear(); + }); return testRealm().toRepresentation(); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java index cdcb5548de..f60c49e076 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java @@ -21,14 +21,24 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import java.util.HashMap; import java.util.Map; + +import org.jetbrains.annotations.NotNull; import org.junit.Test; import org.keycloak.OAuth2Constants; import org.keycloak.TokenVerifier; +import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.OrganizationResource; import org.keycloak.common.Profile.Feature; +import org.keycloak.protocol.oidc.OIDCLoginProtocol; +import org.keycloak.protocol.oidc.mappers.GroupMembershipMapper; +import org.keycloak.protocol.oidc.mappers.OIDCAttributeMapperHelper; import org.keycloak.representations.AccessToken; +import org.keycloak.representations.idm.ClientRepresentation; +import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.testsuite.arquillian.annotation.EnableFeature; import org.keycloak.testsuite.organization.admin.AbstractOrganizationTest; import org.keycloak.testsuite.util.OAuthClient.AccessTokenResponse; @@ -55,4 +65,35 @@ public class OrganizationOIDCProtocolMapperTest extends AbstractOrganizationTest assertThat(claim, notNullValue()); assertThat(claim.get(organizationName), notNullValue()); } + + @Test + public void testOrganizationNotAddedByGroupMapper() throws Exception { + OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); + addMember(organization); + ClientRepresentation client = testRealm().clients().findByClientId("direct-grant").get(0); + ClientResource clientResource = testRealm().clients().get(client.getId()); + clientResource.getProtocolMappers().createMapper(createGroupMapper()).close(); + + oauth.clientId("direct-grant"); + oauth.scope("openid organization"); + AccessTokenResponse response = oauth.doGrantAccessTokenRequest("password", memberEmail, memberPassword); + assertThat(response.getScope(), containsString("organization")); + AccessToken accessToken = TokenVerifier.create(response.getAccessToken(), AccessToken.class).getToken(); + assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION)); + assertThat(accessToken.getOtherClaims().get("groups"), nullValue()); + } + + @NotNull + private static ProtocolMapperRepresentation createGroupMapper() { + ProtocolMapperRepresentation groupMapper = new ProtocolMapperRepresentation(); + groupMapper.setName("groups"); + groupMapper.setProtocolMapper(GroupMembershipMapper.PROVIDER_ID); + groupMapper.setProtocol(OIDCLoginProtocol.LOGIN_PROTOCOL); + Map config = new HashMap<>(); + config.put(OIDCAttributeMapperHelper.TOKEN_CLAIM_NAME, "groups.groups"); + config.put(OIDCAttributeMapperHelper.INCLUDE_IN_ACCESS_TOKEN, "true"); + config.put(OIDCAttributeMapperHelper.INCLUDE_IN_ID_TOKEN, "true"); + groupMapper.setConfig(config); + return groupMapper; + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java index 729ccf0907..5c3015fdf9 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java @@ -345,18 +345,6 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { } } - @Test - public void testDeleteGroupOnOrganizationRemoval() { - OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); - addMember(organization); - - assertTrue(testRealm().groups().groups("", 0, 100, false).stream().anyMatch(group -> group.getAttributes().containsKey("kc.org"))); - - organization.delete().close(); - - assertFalse(testRealm().groups().groups("", 0, 100, false).stream().anyMatch(group -> group.getAttributes().containsKey("kc.org"))); - } - @Test public void testSearchMembers() {