From 339224578edd944fc3426458ef374276dc646132 Mon Sep 17 00:00:00 2001 From: Daniel Fesenmeyer Date: Thu, 22 Apr 2021 16:00:07 +0200 Subject: [PATCH] KEYCLOAK-10603 adjust assignments to roles (user-role and group-role assignments, client-scope and client "scope mappings"): allow assignments of roles which are already indirectly assigned (e.g. by composite role) - extend RoleMapperModel with method hasDirectRole(RoleModel), which only checks for direct assignment in contrast to the existing method hasRole(RoleModel) - extend ScopeContainerModel with method hasDirectScope(RoleModel), which only checks for direct scope mapping in contrast to the existing method hasScope(RoleModel) - use the new hasDirectRole and hasDirectScope methods to check whether a role is in the "available" list and whether it can be assigned (previously, the hasRole method was used for this purpose) - add hint to UI that available roles contain effectively assigned roles which are not directly assigned - adjust and extend tests --- .../HardcodedLDAPRoleStorageMapper.java | 5 + .../cache/infinispan/ClientAdapter.java | 11 +- .../cache/infinispan/ClientScopeAdapter.java | 7 + .../models/cache/infinispan/GroupAdapter.java | 7 + .../models/cache/infinispan/UserAdapter.java | 6 + .../org/keycloak/models/jpa/GroupAdapter.java | 2 +- .../org/keycloak/models/jpa/UserAdapter.java | 2 +- .../models/map/client/MapClientAdapter.java | 10 ++ .../models/map/group/MapGroupAdapter.java | 7 +- .../models/map/user/MapUserAdapter.java | 7 +- .../delegate/ClientModelLazyDelegate.java | 5 + .../UpdateOnlyChangeUserModelDelegate.java | 4 +- .../java/org/keycloak/models/ClientModel.java | 7 + .../org/keycloak/models/RoleMapperModel.java | 13 ++ .../keycloak/models/ScopeContainerModel.java | 21 +++ .../models/utils/UserModelDelegate.java | 1 - .../AbstractUserAdapterFederatedStorage.java | 1 - .../admin/ClientRoleMappingsResource.java | 2 +- .../resources/admin/RoleMapperResource.java | 2 +- .../admin/ScopeMappedClientResource.java | 2 +- .../resources/admin/ScopeMappedResource.java | 2 +- .../keycloak/testsuite/admin/ClientTest.java | 108 ++++++++++-- .../keycloak/testsuite/admin/UserTest.java | 101 ++++++++++- .../admin/client/ClientScopeTest.java | 163 ++++++++++++------ .../testsuite/admin/group/GroupTest.java | 113 +++++++++--- .../messages/admin-messages_de.properties | 8 +- .../messages/admin-messages_en.properties | 14 +- 27 files changed, 512 insertions(+), 119 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java index a5d2cbb0d3..39eee70fb0 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/HardcodedLDAPRoleStorageMapper.java @@ -77,6 +77,11 @@ public class HardcodedLDAPRoleStorageMapper extends AbstractLDAPStorageMapper { return clientRoleMappings; } + @Override + public boolean hasDirectRole(RoleModel role) { + return super.hasDirectRole(role) || role.equals(getRole(realm)); + } + @Override public boolean hasRole(RoleModel role) { return super.hasRole(role) || role.equals(getRole(realm)); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java index abfca1ec94..ae7faecd87 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientAdapter.java @@ -625,6 +625,15 @@ public class ClientAdapter implements ClientModel, CachedObject { updated.unregisterNode(nodeHost); } + @Override + public boolean hasDirectScope(RoleModel role) { + if (isUpdated()) return updated.hasDirectScope(role); + + if (cached.getScope().contains(role.getId())) return true; + + return getRolesStream().anyMatch(r -> Objects.equals(r, role)); + } + @Override public boolean hasScope(RoleModel role) { if (isUpdated()) return updated.hasScope(role); @@ -633,7 +642,7 @@ public class ClientAdapter implements ClientModel, CachedObject { if (RoleUtils.hasRole(getScopeMappingsStream(), role)) return true; - return getRolesStream().anyMatch(r -> (Objects.equals(r, role) || r.hasRole(role))); + return RoleUtils.hasRole(getRolesStream(), role); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java index 5ae50e85f7..e932cd1edb 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/ClientScopeAdapter.java @@ -180,6 +180,13 @@ public class ClientScopeAdapter implements ClientScopeModel { return getScopeMappingsStream().filter(r -> RoleUtils.isRealmRole(r, cachedRealm)); } + @Override + public boolean hasDirectScope(RoleModel role) { + if (isUpdated()) return updated.hasDirectScope(role); + + return cached.getScope().contains(role.getId()); + } + @Override public boolean hasScope(RoleModel role) { if (isUpdated()) return updated.hasScope(role); 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 71911f699c..ad3841572b 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 @@ -160,6 +160,13 @@ public class GroupAdapter implements GroupModel.Streams { return getRoleMappingsStream().filter(r -> RoleUtils.isClientRole(r, app)); } + @Override + public boolean hasDirectRole(RoleModel role) { + if (isUpdated()) return updated.hasDirectRole(role); + + return cached.getRoleMappings(modelSupplier).contains(role.getId()); + } + @Override public boolean hasRole(RoleModel role) { if (isUpdated()) return updated.hasRole(role); 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 9d4048d258..0a9ad46ea4 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 @@ -290,6 +290,12 @@ public class UserAdapter implements CachedUserModel.Streams { return getRoleMappingsStream().filter(r -> RoleUtils.isClientRole(r, app)); } + @Override + public boolean hasDirectRole(RoleModel role) { + if (updated != null) return updated.hasDirectRole(role); + return cached.getRoleMappings(modelSupplier).contains(role.getId()); + } + @Override public boolean hasRole(RoleModel role) { if (updated != null) return updated.hasRole(role); 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 2ae1c828f5..7c1d766ee1 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 @@ -226,7 +226,7 @@ public class GroupAdapter implements GroupModel.Streams , JpaModel @Override public void grantRole(RoleModel role) { - if (hasRole(role)) return; + if (hasDirectRole(role)) return; GroupRoleMappingEntity entity = new GroupRoleMappingEntity(); entity.setGroup(getEntity()); entity.setRoleId(role.getId()); 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 c59b375db7..020b5b8a71 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 @@ -446,7 +446,7 @@ public class UserAdapter implements UserModel.Streams, JpaModel { @Override public void grantRole(RoleModel role) { - if (hasRole(role)) return; + if (hasDirectRole(role)) return; grantRoleImpl(role); } diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java index 2b1196d64e..a6f7bf09f1 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java @@ -419,6 +419,16 @@ public abstract class MapClientAdapter extends AbstractClientModel (Objects.equals(r, role))); + } + @Override public boolean hasScope(RoleModel role) { if (isFullScopeAllowed()) return true; 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 index ace1e1e3b7..370f96162d 100644 --- 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 @@ -146,10 +146,15 @@ public class MapGroupAdapter extends AbstractGroupModel { } @Override - public boolean hasRole(RoleModel role) { + public boolean hasDirectRole(RoleModel role) { return entity.getGrantedRoles().contains(role.getId()); } + @Override + public boolean hasRole(RoleModel role) { + return hasDirectRole(role); + } + @Override public void grantRole(RoleModel role) { entity.addGrantedRole(role.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/user/MapUserAdapter.java b/model/map/src/main/java/org/keycloak/models/map/user/MapUserAdapter.java index eff64b57a6..54f31fc655 100644 --- a/model/map/src/main/java/org/keycloak/models/map/user/MapUserAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/user/MapUserAdapter.java @@ -293,10 +293,15 @@ public abstract class MapUserAdapter extends AbstractUserModel { } @Override - public boolean hasRole(RoleModel role) { + public boolean hasDirectRole(RoleModel role) { return entity.getRolesMembership().contains(role.getId()); } + @Override + public boolean hasRole(RoleModel role) { + return hasDirectRole(role); + } + @Override public void grantRole(RoleModel role) { entity.addRolesMembership(role.getId()); diff --git a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java index fd050528bf..4e03535554 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java +++ b/server-spi-private/src/main/java/org/keycloak/models/delegate/ClientModelLazyDelegate.java @@ -538,6 +538,11 @@ public class ClientModelLazyDelegate implements ClientModel { getDelegate().deleteScopeMapping(role); } + @Override + public boolean hasDirectScope(RoleModel role) { + return getDelegate().hasDirectScope(role); + } + @Override public boolean hasScope(RoleModel role) { return getDelegate().hasScope(role); diff --git a/server-spi-private/src/main/java/org/keycloak/storage/adapter/UpdateOnlyChangeUserModelDelegate.java b/server-spi-private/src/main/java/org/keycloak/storage/adapter/UpdateOnlyChangeUserModelDelegate.java index 8759f3d60d..7c3a556f6c 100644 --- a/server-spi-private/src/main/java/org/keycloak/storage/adapter/UpdateOnlyChangeUserModelDelegate.java +++ b/server-spi-private/src/main/java/org/keycloak/storage/adapter/UpdateOnlyChangeUserModelDelegate.java @@ -135,14 +135,14 @@ public class UpdateOnlyChangeUserModelDelegate extends UserModelDelegate { @Override public void grantRole(RoleModel role) { - if (!hasRole(role)) { + if (!hasDirectRole(role)) { delegate.grantRole(role); } } @Override public void deleteRoleMapping(RoleModel role) { - if (hasRole(role)) { + if (hasDirectRole(role)) { delegate.deleteRoleMapping(role); } } diff --git a/server-spi/src/main/java/org/keycloak/models/ClientModel.java b/server-spi/src/main/java/org/keycloak/models/ClientModel.java index 97abce87e7..e4eb5e5961 100755 --- a/server-spi/src/main/java/org/keycloak/models/ClientModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ClientModel.java @@ -18,6 +18,7 @@ package org.keycloak.models; import java.util.Map; +import java.util.Objects; import java.util.Set; import org.keycloak.common.util.ObjectUtil; @@ -192,6 +193,12 @@ public interface ClientModel extends ClientScopeModel, RoleContainerModel, Prot boolean isFullScopeAllowed(); void setFullScopeAllowed(boolean value); + @Override + default boolean hasDirectScope(RoleModel role) { + if (getScopeMappingsStream().anyMatch(r -> Objects.equals(r, role))) return true; + + return getRolesStream().anyMatch(r -> Objects.equals(r, role)); + } boolean isPublicClient(); void setPublicClient(boolean flag); diff --git a/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java b/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java index 563f17f719..2f96cbadb5 100755 --- a/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RoleMapperModel.java @@ -17,6 +17,7 @@ package org.keycloak.models; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -62,6 +63,17 @@ public interface RoleMapperModel { return value != null ? value.stream() : Stream.empty(); } + /** + * Returns {@code true}, if this object is directly assigned the given role. + * + * @param role the role + * @return see description + * @see #hasRole(RoleModel) if you want to check whether this object is directly or indirectly assigned to a role + */ + default boolean hasDirectRole(RoleModel role) { + return getRoleMappingsStream().anyMatch(r -> Objects.equals(r, role)); + } + /** * Returns {@code true} if this object is directly or indirectly assigned the given role, {@code false} otherwise. *

@@ -73,6 +85,7 @@ public interface RoleMapperModel { * * @param role * @return see description + * @see #hasDirectRole(RoleModel) if you want to check if this object is directly assigned to a role */ boolean hasRole(RoleModel role); diff --git a/server-spi/src/main/java/org/keycloak/models/ScopeContainerModel.java b/server-spi/src/main/java/org/keycloak/models/ScopeContainerModel.java index 619140ea73..83646c7a0f 100755 --- a/server-spi/src/main/java/org/keycloak/models/ScopeContainerModel.java +++ b/server-spi/src/main/java/org/keycloak/models/ScopeContainerModel.java @@ -17,6 +17,7 @@ package org.keycloak.models; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -63,6 +64,26 @@ public interface ScopeContainerModel { void deleteScopeMapping(RoleModel role); + /** + * Returns {@code true}, if this object has the given role directly in its scope. + * + * @param role the role + * @return see description + * @see #hasScope(RoleModel) if you want to check whether this object has the given role directly or indirectly in + * its scope + */ + default boolean hasDirectScope(RoleModel role) { + return getScopeMappingsStream().anyMatch(r -> Objects.equals(r, role)); + } + + /** + * Returns {@code true}, if this object has the given role directly or indirectly in its scope, {@code false} + * otherwise. + * + * @param role the role + * @return see description + * @see #hasDirectScope(RoleModel) if you want to check if this object has the given role directly in its scope + */ boolean hasScope(RoleModel role); } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/UserModelDelegate.java b/server-spi/src/main/java/org/keycloak/models/utils/UserModelDelegate.java index 3acca8400b..7d2b51d4d6 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/UserModelDelegate.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/UserModelDelegate.java @@ -24,7 +24,6 @@ import org.keycloak.models.UserModel; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.stream.Stream; /** diff --git a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java index 1800cc4e3b..3ebd181a76 100644 --- a/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java +++ b/server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java @@ -177,7 +177,6 @@ public abstract class AbstractUserAdapterFederatedStorage extends UserModelDefau return getRoleMappings().stream().filter(r -> RoleUtils.isClientRole(r, app)).collect(Collectors.toSet()); } - @Override public boolean hasRole(RoleModel role) { return RoleUtils.hasRole(getRoleMappings().stream(), role) diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java index 1e5703eda6..03f6e4d342 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientRoleMappingsResource.java @@ -134,7 +134,7 @@ public class ClientRoleMappingsResource { return client.getRolesStream() .filter(auth.roles()::canMapRole) - .filter(((Predicate) user::hasRole).negate()) + .filter(((Predicate) user::hasDirectRole).negate()) .map(ModelToRepresentation::toBriefRepresentation); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java index 83af76b1ae..3d75378e78 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleMapperResource.java @@ -200,7 +200,7 @@ public class RoleMapperResource { return realm.getRolesStream() .filter(this::canMapRole) - .filter(((Predicate) roleMapper::hasRole).negate()) + .filter(((Predicate) roleMapper::hasDirectRole).negate()) .map(ModelToRepresentation::toBriefRepresentation); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedClientResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedClientResource.java index 70edf7eb81..b134c79222 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedClientResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedClientResource.java @@ -106,7 +106,7 @@ public class ScopeMappedClientResource { viewPermission.require(); return scopedClient.getRolesStream() - .filter(((Predicate) scopeContainer::hasScope).negate()) + .filter(((Predicate) scopeContainer::hasDirectScope).negate()) .filter(auth.roles()::canMapClientScope) .map(ModelToRepresentation::toBriefRepresentation); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedResource.java index 87e7b3a627..67165b7556 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ScopeMappedResource.java @@ -155,7 +155,7 @@ public class ScopeMappedResource { } return realm.getRolesStream() - .filter(((Predicate) scopeContainer::hasScope).negate()) + .filter(((Predicate) scopeContainer::hasDirectScope).negate()) .filter(auth.roles()::canMapClientScope) .map(ModelToRepresentation::toBriefRepresentation); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java index df7d9f2aaa..d2bf3da238 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/ClientTest.java @@ -560,20 +560,18 @@ public class ClientTest extends AbstractAdminTest { RoleMappingResource scopesResource = realm.clients().get(id).getScopeMappings(); - RoleRepresentation roleRep1 = RoleBuilder.create().name("role1").build(); - RoleRepresentation roleRep2 = RoleBuilder.create().name("role2").build(); - realm.roles().create(roleRep1); - realm.roles().create(roleRep2); + RoleRepresentation roleRep1 = createRealmRole("realm-composite"); + RoleRepresentation roleRep2 = createRealmRole("realm-child"); - assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("role1"), roleRep1, ResourceType.REALM_ROLE); - assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("role2"), roleRep2, ResourceType.REALM_ROLE); + assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("realm-composite"), roleRep1, ResourceType.REALM_ROLE); + assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath("realm-child"), roleRep2, ResourceType.REALM_ROLE); - roleRep1 = realm.roles().get("role1").toRepresentation(); - roleRep2 = realm.roles().get("role2").toRepresentation(); + roleRep1 = realm.roles().get("realm-composite").toRepresentation(); + roleRep2 = realm.roles().get("realm-child").toRepresentation(); - realm.roles().get("role1").addComposites(Collections.singletonList(roleRep2)); + realm.roles().get("realm-composite").addComposites(Collections.singletonList(roleRep2)); - assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourceCompositesPath("role1"), Collections.singletonList(roleRep2), ResourceType.REALM_ROLE); + assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourceCompositesPath("realm-composite"), Collections.singletonList(roleRep2), ResourceType.REALM_ROLE); String accountMgmtId = realm.clients().findByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).get(0).getId(); RoleRepresentation viewAccountRoleRep = realm.clients().get(accountMgmtId).roles().get(AccountRoles.VIEW_PROFILE).toRepresentation(); @@ -584,16 +582,17 @@ public class ClientTest extends AbstractAdminTest { scopesResource.clientLevel(accountMgmtId).add(Collections.singletonList(viewAccountRoleRep)); assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.clientScopeMappingsClientLevelPath(id, accountMgmtId), Collections.singletonList(viewAccountRoleRep), ResourceType.CLIENT_SCOPE_MAPPING); - Assert.assertNames(scopesResource.realmLevel().listAll(), "role1"); - Assert.assertNames(scopesResource.realmLevel().listEffective(), "role1", "role2"); - Assert.assertNames(scopesResource.realmLevel().listAvailable(), "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); + Assert.assertNames(scopesResource.realmLevel().listAll(), "realm-composite"); + Assert.assertNames(scopesResource.realmLevel().listEffective(), "realm-composite", "realm-child"); + Assert.assertNames(scopesResource.realmLevel().listAvailable(), "realm-child", "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listAll(), AccountRoles.VIEW_PROFILE); Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listEffective(), AccountRoles.VIEW_PROFILE); Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listAvailable(), AccountRoles.MANAGE_ACCOUNT, AccountRoles.MANAGE_ACCOUNT_LINKS, AccountRoles.VIEW_APPLICATIONS, AccountRoles.VIEW_CONSENT, AccountRoles.MANAGE_CONSENT, AccountRoles.DELETE_ACCOUNT); - Assert.assertNames(scopesResource.getAll().getRealmMappings(), "role1"); + Assert.assertNames(scopesResource.getAll().getRealmMappings(), "realm-composite"); Assert.assertNames(scopesResource.getAll().getClientMappings().get(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).getMappings(), AccountRoles.VIEW_PROFILE); scopesResource.realmLevel().remove(Collections.singletonList(roleRep1)); @@ -604,7 +603,7 @@ public class ClientTest extends AbstractAdminTest { Assert.assertNames(scopesResource.realmLevel().listAll()); Assert.assertNames(scopesResource.realmLevel().listEffective()); - Assert.assertNames(scopesResource.realmLevel().listAvailable(), "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, "role1", "role2", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); + Assert.assertNames(scopesResource.realmLevel().listAvailable(), "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, "realm-composite", "realm-child", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listAll()); Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listAvailable(), AccountRoles.VIEW_PROFILE, AccountRoles.MANAGE_ACCOUNT, AccountRoles.MANAGE_ACCOUNT_LINKS, AccountRoles.VIEW_APPLICATIONS, AccountRoles.VIEW_CONSENT, AccountRoles.MANAGE_CONSENT, AccountRoles.DELETE_ACCOUNT); @@ -612,6 +611,73 @@ public class ClientTest extends AbstractAdminTest { Assert.assertNames(scopesResource.clientLevel(accountMgmtId).listEffective()); } + /** + * Test for KEYCLOAK-10603. + */ + @Test + public void rolesCanBeAddedToScopeEvenWhenTheyAreAlreadyIndirectlyAssigned() { + Response response = + realm.clients().create(ClientBuilder.create().clientId("test-client").fullScopeEnabled(false).build()); + String testedClientUuid = ApiUtil.getCreatedId(response); + getCleanup().addClientUuid(testedClientUuid); + response.close(); + + createRealmRole("realm-composite"); + createRealmRole("realm-child"); + realm.roles().get("realm-composite") + .addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + + response = realm.clients().create(ClientBuilder.create().clientId("role-container-client").build()); + String roleContainerClientUuid = ApiUtil.getCreatedId(response); + getCleanup().addClientUuid(roleContainerClientUuid); + response.close(); + + RoleRepresentation clientCompositeRole = RoleBuilder.create().name("client-composite").build(); + realm.clients().get(roleContainerClientUuid).roles().create(clientCompositeRole); + realm.clients().get(roleContainerClientUuid).roles().create(RoleBuilder.create().name("client-child").build()); + realm.clients().get(roleContainerClientUuid).roles().get("client-composite").addComposites(Collections + .singletonList( + realm.clients().get(roleContainerClientUuid).roles().get("client-child").toRepresentation())); + + // Make indirect assignments: assign composite roles + RoleMappingResource scopesResource = realm.clients().get(testedClientUuid).getScopeMappings(); + scopesResource.realmLevel() + .add(Collections.singletonList(realm.roles().get("realm-composite").toRepresentation())); + scopesResource.clientLevel(roleContainerClientUuid).add(Collections + .singletonList(realm.clients().get(roleContainerClientUuid).roles().get("client-composite") + .toRepresentation())); + + // check state before making the direct assignments + Assert.assertNames(scopesResource.realmLevel().listAll(), "realm-composite"); + Assert.assertNames(scopesResource.realmLevel().listAvailable(), "realm-child", "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); + Assert.assertNames(scopesResource.realmLevel().listEffective(), "realm-composite", "realm-child"); + + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAll(), "client-composite"); + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAvailable(), "client-child"); + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite", + "client-child"); + + // Make direct assignments for roles which are already indirectly assigned + scopesResource.realmLevel().add(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + scopesResource.clientLevel(roleContainerClientUuid).add(Collections + .singletonList( + realm.clients().get(roleContainerClientUuid).roles().get("client-child").toRepresentation())); + + // List realm roles + Assert.assertNames(scopesResource.realmLevel().listAll(), "realm-composite", "realm-child"); + Assert.assertNames(scopesResource.realmLevel().listAvailable(), "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-" + REALM_NAME); + Assert.assertNames(scopesResource.realmLevel().listEffective(), "realm-composite", "realm-child"); + + // List client roles + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAll(), "client-composite", + "client-child"); + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAvailable()); + Assert.assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite", + "client-child"); + } + @Test public void scopesRoleRemoval() { // clientA to test scope mappins @@ -631,8 +697,7 @@ public class ClientTest extends AbstractAdminTest { RoleMappingResource scopesResource = realm.clients().get(idA).getScopeMappings(); // create a realm role and a role in clientB - RoleRepresentation realmRoleRep = RoleBuilder.create().name("realm-role").build(); - realm.roles().create(realmRoleRep); + RoleRepresentation realmRoleRep = createRealmRole("realm-role"); assertAdminEvents.assertEvent(realmId, OperationType.CREATE, AdminEventPaths.roleResourcePath(realmRoleRep.getName()), realmRoleRep, ResourceType.REALM_ROLE); RoleRepresentation clientBRoleRep = RoleBuilder.create().name("clientB-role").build(); realm.clients().get(idB).roles().create(clientBRoleRep); @@ -831,4 +896,13 @@ public class ClientTest extends AbstractAdminTest { } } + private RoleRepresentation createRealmRole(String roleName) { + RoleRepresentation role = RoleBuilder.create().name(roleName).build(); + realm.roles().create(role); + + String createdId = realm.roles().get(role.getName()).toRepresentation().getId(); + getCleanup().addRoleId(createdId); + + return role; + } } 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 02b6f7b984..4f554423d3 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 @@ -2092,7 +2092,7 @@ public class UserTest extends AbstractAdminTest { // List realm roles assertNames(roles.realmLevel().listAll(), "realm-role", "realm-composite", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); - assertNames(roles.realmLevel().listAvailable(), "admin", "customer-user-premium", "realm-composite-role", "sample-realm-role", "attribute-role"); + assertNames(roles.realmLevel().listAvailable(), "realm-child", "admin", "customer-user-premium", "realm-composite-role", "sample-realm-role", "attribute-role", "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION); assertNames(roles.realmLevel().listEffective(), "realm-role", "realm-composite", "realm-child", "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); // List realm effective role with full representation @@ -2103,7 +2103,7 @@ public class UserTest extends AbstractAdminTest { // List client roles assertNames(roles.clientLevel(clientUuid).listAll(), "client-role", "client-composite"); - assertNames(roles.clientLevel(clientUuid).listAvailable(), "client-role2"); + assertNames(roles.clientLevel(clientUuid).listAvailable(), "client-role2", "client-child"); assertNames(roles.clientLevel(clientUuid).listEffective(), "client-role", "client-composite", "client-child"); // List client effective role with full representation @@ -2133,6 +2133,103 @@ public class UserTest extends AbstractAdminTest { assertNames(roles.clientLevel(clientUuid).listAll(), "client-composite"); } + /** + * Test for KEYCLOAK-10603. + */ + @Test + public void rolesCanBeAssignedEvenWhenTheyAreAlreadyIndirectlyAssigned() { + RealmResource realm = adminClient.realms().realm("test"); + + RoleRepresentation realmCompositeRole = RoleBuilder.create().name("realm-composite").build(); + realm.roles().create(realmCompositeRole); + realm.roles().create(RoleBuilder.create().name("realm-child").build()); + realm.roles().get("realm-composite") + .addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + realm.roles().create(RoleBuilder.create().name("realm-role-in-group").build()); + + Response response = realm.clients().create(ClientBuilder.create().clientId("myclient").build()); + String clientUuid = ApiUtil.getCreatedId(response); + response.close(); + + RoleRepresentation clientCompositeRole = RoleBuilder.create().name("client-composite").build(); + realm.clients().get(clientUuid).roles().create(clientCompositeRole); + realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-child").build()); + realm.clients().get(clientUuid).roles().get("client-composite").addComposites(Collections + .singletonList(realm.clients().get(clientUuid).roles().get("client-child").toRepresentation())); + realm.clients().get(clientUuid).roles().create(RoleBuilder.create().name("client-role-in-group").build()); + + GroupRepresentation group = GroupBuilder.create().name("mygroup").build(); + response = realm.groups().add(group); + String groupId = ApiUtil.getCreatedId(response); + response.close(); + + response = realm.users().create(UserBuilder.create().username("myuser").build()); + String userId = ApiUtil.getCreatedId(response); + response.close(); + + // Make indirect assignments + // .. add roles to the group and add it to the user + realm.groups().group(groupId).roles().realmLevel() + .add(Collections.singletonList(realm.roles().get("realm-role-in-group").toRepresentation())); + realm.groups().group(groupId).roles().clientLevel(clientUuid).add(Collections + .singletonList(realm.clients().get(clientUuid).roles().get("client-role-in-group").toRepresentation())); + realm.users().get(userId).joinGroup(groupId); + // .. assign composite roles + RoleMappingResource userRoles = realm.users().get(userId).roles(); + userRoles.realmLevel().add(Collections.singletonList(realm.roles().get("realm-composite").toRepresentation())); + userRoles.clientLevel(clientUuid).add(Collections + .singletonList(realm.clients().get(clientUuid).roles().get("client-composite").toRepresentation())); + + // check state before making the direct assignments + assertNames(userRoles.realmLevel().listAll(), "realm-composite", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(userRoles.realmLevel().listAvailable(), "realm-child", "realm-role-in-group", + "admin", "customer-user-premium", "realm-composite-role", + "sample-realm-role", + "attribute-role", "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION); + assertNames(userRoles.realmLevel().listEffective(), "realm-composite", "realm-child", "realm-role-in-group", + "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, + Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + + assertNames(userRoles.clientLevel(clientUuid).listAll(), "client-composite"); + assertNames(userRoles.clientLevel(clientUuid).listAvailable(), "client-child", + "client-role-in-group"); + assertNames(userRoles.clientLevel(clientUuid).listEffective(), "client-composite", "client-child", + "client-role-in-group"); + + // Make direct assignments for roles which are already indirectly assigned + userRoles.realmLevel().add(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + userRoles.realmLevel() + .add(Collections.singletonList(realm.roles().get("realm-role-in-group").toRepresentation())); + userRoles.clientLevel(clientUuid).add(Collections + .singletonList(realm.clients().get(clientUuid).roles().get("client-child").toRepresentation())); + userRoles.clientLevel(clientUuid).add(Collections + .singletonList(realm.clients().get(clientUuid).roles().get("client-role-in-group").toRepresentation())); + + // List realm roles + assertNames(userRoles.realmLevel().listAll(), "realm-composite", + "realm-child", "realm-role-in-group", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(userRoles.realmLevel().listAvailable(), "admin", "customer-user-premium", "realm-composite-role", + "sample-realm-role", "attribute-role", "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION); + assertNames(userRoles.realmLevel().listEffective(), "realm-composite", "realm-child", "realm-role-in-group", + "user", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, + Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + + // List client roles + assertNames(userRoles.clientLevel(clientUuid).listAll(), "client-composite", "client-child", + "client-role-in-group"); + assertNames(userRoles.clientLevel(clientUuid).listAvailable()); + assertNames(userRoles.clientLevel(clientUuid).listEffective(), "client-composite", "client-child", + "client-role-in-group"); + + // Get mapping representation + MappingsRepresentation all = userRoles.getAll(); + assertNames(all.getRealmMappings(), "realm-composite", + "realm-child", "realm-role-in-group", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertEquals(1, all.getClientMappings().size()); + assertNames(all.getClientMappings().get("myclient").getMappings(), "client-composite", "client-child", + "client-role-in-group"); + } + @Test public void defaultMaxResults() { UsersResource users = adminClient.realms().realm("test").users(); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java index cd27d053c1..5f9ed8fb59 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java @@ -22,6 +22,7 @@ import org.junit.Test; import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientScopesResource; import org.keycloak.admin.client.resource.ProtocolMappersResource; +import org.keycloak.admin.client.resource.RealmResource; import org.keycloak.admin.client.resource.RoleMappingResource; import org.keycloak.common.util.ObjectUtil; import org.keycloak.events.admin.OperationType; @@ -38,19 +39,18 @@ import org.keycloak.representations.idm.ProtocolMapperRepresentation; import org.keycloak.representations.idm.RoleRepresentation; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.util.AdminEventPaths; +import org.keycloak.testsuite.util.ClientBuilder; import org.keycloak.testsuite.util.Matchers; +import org.keycloak.testsuite.util.RoleBuilder; import javax.ws.rs.ClientErrorException; import javax.ws.rs.NotFoundException; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @@ -58,6 +58,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.keycloak.testsuite.Assert.assertNames; /** * @author Marek Posolda @@ -209,15 +210,12 @@ public class ClientScopeTest extends AbstractClientTest { @Test public void testScopes() { - // Add realm role1 - RoleRepresentation roleRep1 = createRealmRole("role1"); - - // Add realm role2 - RoleRepresentation roleRep2 = createRealmRole("role2"); - - // Add role2 as composite to role1 - testRealmResource().roles().get("role1").addComposites(Collections.singletonList(roleRep2)); - assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.roleResourceCompositesPath("role1"), Collections.singletonList(roleRep2), ResourceType.REALM_ROLE); + RoleRepresentation realmCompositeRole = createRealmRole("realm-composite"); + RoleRepresentation realmChildRole = createRealmRole("realm-child"); + testRealmResource().roles().get("realm-composite").addComposites(Collections.singletonList(realmChildRole)); + assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, + AdminEventPaths.roleResourceCompositesPath("realm-composite"), + Collections.singletonList(realmChildRole), ResourceType.REALM_ROLE); // create client scope ClientScopeRepresentation scopeRep = new ClientScopeRepresentation(); @@ -225,15 +223,21 @@ public class ClientScopeTest extends AbstractClientTest { String scopeId = createClientScope(scopeRep); // update with some scopes - String accountMgmtId = testRealmResource().clients().findByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).get(0).getId(); - RoleRepresentation viewAccountRoleRep = testRealmResource().clients().get(accountMgmtId).roles().get(AccountRoles.VIEW_PROFILE).toRepresentation(); + String accountMgmtId = + testRealmResource().clients().findByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).get(0).getId(); + RoleRepresentation viewAccountRoleRep = testRealmResource().clients().get(accountMgmtId).roles() + .get(AccountRoles.VIEW_PROFILE).toRepresentation(); RoleMappingResource scopesResource = clientScopes().get(scopeId).getScopeMappings(); - scopesResource.realmLevel().add(Collections.singletonList(roleRep1)); - assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientScopeRoleMappingsRealmLevelPath(scopeId), Collections.singletonList(roleRep1), ResourceType.REALM_SCOPE_MAPPING); + scopesResource.realmLevel().add(Collections.singletonList(realmCompositeRole)); + assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, + AdminEventPaths.clientScopeRoleMappingsRealmLevelPath(scopeId), + Collections.singletonList(realmCompositeRole), ResourceType.REALM_SCOPE_MAPPING); scopesResource.clientLevel(accountMgmtId).add(Collections.singletonList(viewAccountRoleRep)); - assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.clientScopeRoleMappingsClientLevelPath(scopeId, accountMgmtId), Collections.singletonList(viewAccountRoleRep), ResourceType.CLIENT_SCOPE_MAPPING); + assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, + AdminEventPaths.clientScopeRoleMappingsClientLevelPath(scopeId, accountMgmtId), + Collections.singletonList(viewAccountRoleRep), ResourceType.CLIENT_SCOPE_MAPPING); // test that scopes are available (also through composite role) List allRealm = scopesResource.realmLevel().listAll(); @@ -241,61 +245,110 @@ public class ClientScopeTest extends AbstractClientTest { List effectiveRealm = scopesResource.realmLevel().listEffective(); List accountRoles = scopesResource.clientLevel(accountMgmtId).listAll(); - assertRolesPresent(allRealm, "role1"); - assertRolesNotPresent(availableRealm, "role1", "role2"); - assertRolesPresent(effectiveRealm, "role1", "role2"); - assertRolesPresent(accountRoles, AccountRoles.VIEW_PROFILE); + assertNames(allRealm, "realm-composite"); + assertNames(availableRealm, "realm-child", Constants.OFFLINE_ACCESS_ROLE, Constants.AUTHZ_UMA_AUTHORIZATION, + Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(effectiveRealm, "realm-composite", "realm-child"); + assertNames(accountRoles, AccountRoles.VIEW_PROFILE); MappingsRepresentation mappingsRep = clientScopes().get(scopeId).getScopeMappings().getAll(); - assertRolesPresent(mappingsRep.getRealmMappings(), "role1"); - assertRolesPresent(mappingsRep.getClientMappings().get(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).getMappings(), AccountRoles.VIEW_PROFILE); + assertNames(mappingsRep.getRealmMappings(), "realm-composite"); + assertNames(mappingsRep.getClientMappings().get(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).getMappings(), + AccountRoles.VIEW_PROFILE); // remove scopes - scopesResource.realmLevel().remove(Collections.singletonList(roleRep1)); - assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, AdminEventPaths.clientScopeRoleMappingsRealmLevelPath(scopeId), Collections.singletonList(roleRep1), ResourceType.REALM_SCOPE_MAPPING); + scopesResource.realmLevel().remove(Collections.singletonList(realmCompositeRole)); + assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, + AdminEventPaths.clientScopeRoleMappingsRealmLevelPath(scopeId), + Collections.singletonList(realmCompositeRole), ResourceType.REALM_SCOPE_MAPPING); scopesResource.clientLevel(accountMgmtId).remove(Collections.singletonList(viewAccountRoleRep)); - assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, AdminEventPaths.clientScopeRoleMappingsClientLevelPath(scopeId, accountMgmtId), Collections.singletonList(viewAccountRoleRep), ResourceType.CLIENT_SCOPE_MAPPING); + assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, + AdminEventPaths.clientScopeRoleMappingsClientLevelPath(scopeId, accountMgmtId), + Collections.singletonList(viewAccountRoleRep), ResourceType.CLIENT_SCOPE_MAPPING); // assert scopes are removed allRealm = scopesResource.realmLevel().listAll(); availableRealm = scopesResource.realmLevel().listAvailable(); effectiveRealm = scopesResource.realmLevel().listEffective(); accountRoles = scopesResource.clientLevel(accountMgmtId).listAll(); - assertRolesNotPresent(allRealm, "role1"); - assertRolesPresent(availableRealm, "role1", "role2"); - assertRolesNotPresent(effectiveRealm, "role1", "role2"); - assertRolesNotPresent(accountRoles, AccountRoles.VIEW_PROFILE); + assertNames(allRealm); + assertNames(availableRealm, "realm-composite", "realm-child", Constants.OFFLINE_ACCESS_ROLE, + Constants.AUTHZ_UMA_AUTHORIZATION, + Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(effectiveRealm); + assertNames(accountRoles); // remove scope removeClientScope(scopeId); } - private void assertRolesPresent(List roles, String... expectedRoleNames) { - String[] expectedList = expectedRoleNames; + /** + * Test for KEYCLOAK-10603. + */ + @Test + public void rolesCanBeAddedToScopeEvenWhenTheyAreAlreadyIndirectlyAssigned() { + RealmResource realm = testRealmResource(); + ClientScopeRepresentation clientScopeRep = new ClientScopeRepresentation(); + clientScopeRep.setName("my-scope"); + String clientScopeId = createClientScope(clientScopeRep); - Set presentRoles = new HashSet<>(); - for (RoleRepresentation roleRep : roles) { - presentRoles.add(roleRep.getName()); - } + createRealmRole("realm-composite"); + createRealmRole("realm-child"); + realm.roles().get("realm-composite") + .addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); - for (String expected : expectedList) { - if (!presentRoles.contains(expected)) { - Assert.fail("Expected role " + expected + " not available"); - } - } + Response response = realm.clients().create(ClientBuilder.create().clientId("role-container-client").build()); + String roleContainerClientUuid = ApiUtil.getCreatedId(response); + getCleanup().addClientUuid(roleContainerClientUuid); + response.close(); + + RoleRepresentation clientCompositeRole = RoleBuilder.create().name("client-composite").build(); + realm.clients().get(roleContainerClientUuid).roles().create(clientCompositeRole); + realm.clients().get(roleContainerClientUuid).roles().create(RoleBuilder.create().name("client-child").build()); + realm.clients().get(roleContainerClientUuid).roles().get("client-composite").addComposites(Collections + .singletonList( + realm.clients().get(roleContainerClientUuid).roles().get("client-child").toRepresentation())); + + // Make indirect assignments: assign composite roles + RoleMappingResource scopesResource = realm.clientScopes().get(clientScopeId).getScopeMappings(); + scopesResource.realmLevel() + .add(Collections.singletonList(realm.roles().get("realm-composite").toRepresentation())); + scopesResource.clientLevel(roleContainerClientUuid).add(Collections + .singletonList(realm.clients().get(roleContainerClientUuid).roles().get("client-composite") + .toRepresentation())); + + // check state before making the direct assignments + assertNames(scopesResource.realmLevel().listAll(), "realm-composite"); + assertNames(scopesResource.realmLevel().listAvailable(), "realm-child", "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(scopesResource.realmLevel().listEffective(), "realm-composite", "realm-child"); + + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAll(), "client-composite"); + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAvailable(), "client-child"); + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite", + "client-child"); + + // Make direct assignments for roles which are already indirectly assigned + scopesResource.realmLevel().add(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + scopesResource.clientLevel(roleContainerClientUuid).add(Collections + .singletonList( + realm.clients().get(roleContainerClientUuid).roles().get("client-child").toRepresentation())); + + // List realm roles + assertNames(scopesResource.realmLevel().listAll(), "realm-composite", "realm-child"); + assertNames(scopesResource.realmLevel().listAvailable(), "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(scopesResource.realmLevel().listEffective(), "realm-composite", "realm-child"); + + // List client roles + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAll(), "client-composite", + "client-child"); + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listAvailable()); + assertNames(scopesResource.clientLevel(roleContainerClientUuid).listEffective(), "client-composite", + "client-child"); } - - private void assertRolesNotPresent(List roles, String... notExpectedRoleNames) { - List notExpectedList = Arrays.asList(notExpectedRoleNames); - for (RoleRepresentation roleRep : roles) { - if (notExpectedList.contains(roleRep.getName())) { - Assert.fail("Role " + roleRep.getName() + " wasn't expected to be available"); - } - } - } - - + // KEYCLOAK-2809 @Test public void testRemoveScopedRole() { @@ -334,7 +387,11 @@ public class ClientScopeTest extends AbstractClientTest { assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.roleResourcePath(roleName), roleRep, ResourceType.REALM_ROLE); - return testRealmResource().roles().get(roleName).toRepresentation(); + RoleRepresentation createdRole = testRealmResource().roles().get(roleName).toRepresentation(); + + getCleanup().addRoleId(createdRole.getId()); + + return createdRole; } @Test 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 bf0b53c5da..9a2fff1a49 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 @@ -170,6 +170,14 @@ public class GroupTest extends AbstractGroupTest { } } + private RoleRepresentation createRealmRole(RealmResource realm, RoleRepresentation role) { + realm.roles().create(role); + + RoleRepresentation created = realm.roles().get(role.getName()).toRepresentation(); + getCleanup().addRoleId(created.getId()); + return created; + } + @Test public void doNotAllowSameGroupNameAtSameLevel() throws Exception { RealmResource realm = adminClient.realms().realm("test"); @@ -363,24 +371,9 @@ public class GroupTest extends AbstractGroupTest { @Test public void createAndTestGroups() throws Exception { RealmResource realm = adminClient.realms().realm("test"); - { - RoleRepresentation groupRole = new RoleRepresentation(); - groupRole.setName("topRole"); - realm.roles().create(groupRole); - } - RoleRepresentation topRole = realm.roles().get("topRole").toRepresentation(); - { - RoleRepresentation groupRole = new RoleRepresentation(); - groupRole.setName("level2Role"); - realm.roles().create(groupRole); - } - RoleRepresentation level2Role = realm.roles().get("level2Role").toRepresentation(); - { - RoleRepresentation groupRole = new RoleRepresentation(); - groupRole.setName("level3Role"); - realm.roles().create(groupRole); - } - RoleRepresentation level3Role = realm.roles().get("level3Role").toRepresentation(); + RoleRepresentation topRole = createRealmRole(realm, RoleBuilder.create().name("topRole").build()); + RoleRepresentation level2Role = createRealmRole(realm, RoleBuilder.create().name("level2Role").build()); + RoleRepresentation level3Role = createRealmRole(realm, RoleBuilder.create().name("level3Role").build()); // Role events tested elsewhere assertAdminEvents.clear(); @@ -689,13 +682,14 @@ public class GroupTest extends AbstractGroupTest { @Test public void roleMappings() { RealmResource realm = adminClient.realms().realm("test"); - realm.roles().create(RoleBuilder.create().name("realm-role").build()); - realm.roles().create(RoleBuilder.create().name("realm-composite").build()); - realm.roles().create(RoleBuilder.create().name("realm-child").build()); + createRealmRole(realm, RoleBuilder.create().name("realm-role").build()); + createRealmRole(realm, RoleBuilder.create().name("realm-composite").build()); + createRealmRole(realm, RoleBuilder.create().name("realm-child").build()); realm.roles().get("realm-composite").addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); try (Response response = realm.clients().create(ClientBuilder.create().clientId("myclient").build())) { String clientId = ApiUtil.getCreatedId(response); + getCleanup().addClientUuid(clientId); realm.clients().get(clientId).roles().create(RoleBuilder.create().name("client-role").build()); realm.clients().get(clientId).roles().create(RoleBuilder.create().name("client-role2").build()); @@ -731,12 +725,12 @@ public class GroupTest extends AbstractGroupTest { // List realm roles assertNames(roles.realmLevel().listAll(), "realm-role", "realm-composite"); - assertNames(roles.realmLevel().listAvailable(), "admin", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, "user", "customer-user-premium", "realm-composite-role", "sample-realm-role", "attribute-role", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(roles.realmLevel().listAvailable(), "realm-child", "admin", "offline_access", Constants.AUTHZ_UMA_AUTHORIZATION, "user", "customer-user-premium", "realm-composite-role", "sample-realm-role", "attribute-role", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); assertNames(roles.realmLevel().listEffective(), "realm-role", "realm-composite", "realm-child"); // List client roles assertNames(roles.clientLevel(clientId).listAll(), "client-role", "client-composite"); - assertNames(roles.clientLevel(clientId).listAvailable(), "client-role2"); + assertNames(roles.clientLevel(clientId).listAvailable(), "client-role2", "client-child"); assertNames(roles.clientLevel(clientId).listEffective(), "client-role", "client-composite", "client-child"); // Get mapping representation @@ -759,6 +753,79 @@ public class GroupTest extends AbstractGroupTest { } } + /** + * Test for KEYCLOAK-10603. + */ + @Test + public void rolesCanBeAssignedEvenWhenTheyAreAlreadyIndirectlyAssigned() { + RealmResource realm = adminClient.realms().realm("test"); + + createRealmRole(realm, RoleBuilder.create().name("realm-composite").build()); + createRealmRole(realm, RoleBuilder.create().name("realm-child").build()); + realm.roles().get("realm-composite") + .addComposites(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + + try (Response response = realm.clients().create(ClientBuilder.create().clientId("myclient").build())) { + String clientId = ApiUtil.getCreatedId(response); + getCleanup().addClientUuid(clientId); + + realm.clients().get(clientId).roles().create(RoleBuilder.create().name("client-composite").build()); + realm.clients().get(clientId).roles().create(RoleBuilder.create().name("client-child").build()); + realm.clients().get(clientId).roles().get("client-composite").addComposites(Collections + .singletonList(realm.clients().get(clientId).roles().get("client-child").toRepresentation())); + + GroupRepresentation group = new GroupRepresentation(); + group.setName("group"); + + // Roles+clients tested elsewhere + assertAdminEvents.clear(); + + String groupId = createGroup(realm, group).getId(); + + RoleMappingResource roles = realm.groups().group(groupId).roles(); + // Make indirect assignments: assign composite roles + roles.realmLevel() + .add(Collections.singletonList(realm.roles().get("realm-composite").toRepresentation())); + RoleRepresentation clientComposite = + realm.clients().get(clientId).roles().get("client-composite").toRepresentation(); + roles.clientLevel(clientId).add(Collections.singletonList(clientComposite)); + + // Check state before making the direct assignments + assertNames(roles.realmLevel().listAll(), "realm-composite"); + assertNames(roles.realmLevel().listAvailable(), "realm-child", "admin", "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, "user", "customer-user-premium", "realm-composite-role", + "sample-realm-role", "attribute-role", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(roles.realmLevel().listEffective(), "realm-composite", "realm-child"); + + assertNames(roles.clientLevel(clientId).listAll(), "client-composite"); + assertNames(roles.clientLevel(clientId).listAvailable(), "client-child"); + assertNames(roles.clientLevel(clientId).listEffective(), "client-composite", "client-child"); + + // Make direct assignments for roles which are already indirectly assigned + roles.realmLevel().add(Collections.singletonList(realm.roles().get("realm-child").toRepresentation())); + RoleRepresentation clientChild = + realm.clients().get(clientId).roles().get("client-child").toRepresentation(); + roles.clientLevel(clientId).add(Collections.singletonList(clientChild)); + + // List realm roles + assertNames(roles.realmLevel().listAll(), "realm-composite", "realm-child"); + assertNames(roles.realmLevel().listAvailable(), "admin", "offline_access", + Constants.AUTHZ_UMA_AUTHORIZATION, "user", "customer-user-premium", "realm-composite-role", + "sample-realm-role", "attribute-role", Constants.DEFAULT_ROLES_ROLE_PREFIX + "-test"); + assertNames(roles.realmLevel().listEffective(), "realm-composite", "realm-child"); + + // List client roles + assertNames(roles.clientLevel(clientId).listAll(), "client-composite", "client-child"); + assertNames(roles.clientLevel(clientId).listAvailable()); + assertNames(roles.clientLevel(clientId).listEffective(), "client-composite", "client-child"); + + // Get mapping representation + MappingsRepresentation all = roles.getAll(); + assertNames(all.getRealmMappings(), "realm-composite", "realm-child"); + assertEquals(1, all.getClientMappings().size()); + assertNames(all.getClientMappings().get("myclient").getMappings(), "client-composite", "client-child"); + } + } /** * Verifies that the user does not have access to Keycloak Admin endpoint when role is not diff --git a/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties b/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties index 809afe9e1c..d45e5cfe37 100644 --- a/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties +++ b/themes/src/main/resources-community/theme/base/admin/messages/admin-messages_de.properties @@ -440,7 +440,7 @@ type=Typ #scope-mappings=Scope Mappings #full-scope-allowed=Full Scope Allowed #full-scope-allowed.tooltip=Allows you to disable all restrictions. -#scope.available-roles.tooltip=Realm level roles that can be assigned to scope. +#scope.available-roles.tooltip=Realm level roles that can be assigned to scope. Contains effectively assigned roles which are not directly assigned. assigned-roles=Zugewiesene Rollen #assigned-roles.tooltip=Realm level roles assigned to scope. effective-roles=Effektive Rollen @@ -494,7 +494,7 @@ last-refresh=Letzte Aktualisierung #encryption-key=Encryption Key #saml-encryption-key.tooltip=SAML Encryption Key. #service-accounts=Service Accounts -#service-account.available-roles.tooltip=Realm level roles that can be assigned to service account. +#service-account.available-roles.tooltip=Realm level roles that can be assigned to service account. Contains effectively assigned roles which are not directly assigned. #service-account.assigned-roles.tooltip=Realm level roles assigned to service account. #service-account-is-not-enabled-for=Service account is not enabled for {{client}} #create-protocol-mappers=Create Protocol Mappers @@ -713,7 +713,7 @@ groups=Gruppen group.add-selected.tooltip=Realm-Rollen die zu der Gruppen hinzugef\u00FCgt werden k\u00F6nnen. group.assigned-roles.tooltip=Realm-Rollen die zur Gruppe zugeordnet sind #group.effective-roles.tooltip=All realm role mappings. Some roles here might be inherited from a mapped composite role. -#group.available-roles.tooltip=Assignable roles from this client. +#group.available-roles.tooltip=Assignable roles from this client. Contains effectively assigned roles which are not directly assigned. #group.assigned-roles-client.tooltip=Role mappings for this client. #group.effective-roles-client.tooltip=Role mappings for this client. Some roles here might be inherited from a mapped composite role. @@ -737,7 +737,7 @@ users=Benutzer user.add-selected.tooltip=Realm-Rollen, die dem Benutzer zugewiesen werden k\u00F6nnen. user.assigned-roles.tooltip=Realm-Rollen, die dem Benutzer zugewiesen sind. user.effective-roles.tooltip=Alle Realm-Rollen-Zuweisungen. Einige Rollen hier k\u00F6nnen von zusammengesetzten Rollen geerbt sein. -#user.available-roles.tooltip=Assignable roles from this client. +#user.available-roles.tooltip=Assignable roles from this client. Contains effectively assigned roles which are not directly assigned. #user.assigned-roles-client.tooltip=Role mappings for this client. #user.effective-roles-client.tooltip=Role mappings for this client. Some roles here might be inherited from a mapped composite role. diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties index 0295589f0c..d69c90c78a 100644 --- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties +++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties @@ -545,13 +545,13 @@ add-builtin-protocol-mapper=Add Builtin Protocol Mapper scope-mappings=Scope Mappings full-scope-allowed=Full Scope Allowed full-scope-allowed.tooltip=Allows you to disable all restrictions. -scope.available-roles.tooltip=Realm level roles that can be assigned to scope. +scope.available-roles.tooltip=Realm level roles that can be assigned to scope. Contains effectively assigned roles which are not directly assigned. assigned-roles=Assigned Roles assigned-roles.tooltip=Realm level roles assigned to scope. effective-roles=Effective Roles realm.effective-roles.tooltip=Assigned realm level roles that may have been inherited from a composite role. select-client-roles.tooltip=Select client to view roles for client -assign.available-roles.tooltip=Client roles available to be assigned. +assign.available-roles.tooltip=Client roles available to be assigned. Contains effectively assigned roles which are not directly assigned. client.assigned-roles.tooltip=Assigned client roles. client.effective-roles.tooltip=Assigned client roles that may have been inherited from a composite role. basic-configuration=Basic configuration @@ -599,7 +599,7 @@ export=Export encryption-key=Encryption Key saml-encryption-key.tooltip=SAML Encryption Key. service-accounts=Service Accounts -service-account.available-roles.tooltip=Realm level roles that can be assigned to service account. +service-account.available-roles.tooltip=Realm level roles that can be assigned to service account. Contains effectively assigned roles which are not directly assigned. service-account.assigned-roles.tooltip=Realm level roles assigned to service account. service-account-is-not-enabled-for=Service account is not enabled for {{client}} create-protocol-mappers=Create Protocol Mappers @@ -917,10 +917,10 @@ client-updater-source-roles.tooltip=The condition is checked during client regis groups=Groups -group.add-selected.tooltip=Realm roles that can be assigned to the group. +group.add-selected.tooltip=Realm roles that can be assigned to the group. Contains effectively assigned roles which are not directly assigned. group.assigned-roles.tooltip=Realm roles mapped to the group group.effective-roles.tooltip=All realm role mappings. Some roles here might be inherited from a mapped composite role. -group.available-roles.tooltip=Assignable roles from this client. +group.available-roles.tooltip=Assignable roles from this client. Contains effectively assigned roles which are not directly assigned. group.assigned-roles-client.tooltip=Role mappings for this client. group.effective-roles-client.tooltip=Role mappings for this client. Some roles here might be inherited from a mapped composite role. @@ -941,10 +941,10 @@ default-roles=Default Roles no-realm-roles-available=No realm roles available users=Users -user.add-selected.tooltip=Realm roles that can be assigned to the user. +user.add-selected.tooltip=Realm roles that can be assigned to the user. Contains effectively assigned roles which are not directly assigned. user.assigned-roles.tooltip=Realm roles mapped to the user user.effective-roles.tooltip=All realm role mappings. Some roles here might be inherited from a mapped composite role. -user.available-roles.tooltip=Assignable roles from this client. +user.available-roles.tooltip=Assignable roles from this client. Contains effectively assigned roles which are not directly assigned. user.assigned-roles-client.tooltip=Role mappings for this client. user.effective-roles-client.tooltip=Role mappings for this client. Some roles here might be inherited from a mapped composite role.