diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProvider.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProvider.java index 1f445e651fe..d76eb749072 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProvider.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/group/GroupPolicyProvider.java @@ -104,7 +104,7 @@ public class GroupPolicyProvider implements PolicyProvider, PartialEvaluationPol } @Override - public Stream getPermissions(KeycloakSession session, ResourceType resourceType, UserModel user) { + public Stream getPermissions(KeycloakSession session, ResourceType resourceType, ResourceType groupResourceType, UserModel user) { AuthorizationProvider provider = session.getProvider(AuthorizationProvider.class); RealmModel realm = session.getContext().getRealm(); ClientModel adminPermissionsClient = realm.getAdminPermissionsClient(); @@ -113,7 +113,7 @@ public class GroupPolicyProvider implements PolicyProvider, PartialEvaluationPol PolicyStore policyStore = storeFactory.getPolicyStore(); List groupIds = user.getGroupsStream().map(GroupModel::getId).toList(); - return policyStore.findDependentPolicies(resourceServer, resourceType.getType(), GroupPolicyProviderFactory.ID, "groups", groupIds); + return policyStore.findDependentPolicies(resourceServer, resourceType.getType(), groupResourceType == null ? null : groupResourceType.getType(), GroupPolicyProviderFactory.ID, "groups", groupIds); } @Override diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProvider.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProvider.java index 1e1a20b1c4e..50d63f91f28 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProvider.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/role/RolePolicyProvider.java @@ -134,7 +134,7 @@ public class RolePolicyProvider implements PolicyProvider, PartialEvaluationPoli } @Override - public Stream getPermissions(KeycloakSession session, ResourceType resourceType, UserModel subject) { + public Stream getPermissions(KeycloakSession session, ResourceType resourceType, ResourceType groupResourceType, UserModel subject) { AuthorizationProvider provider = session.getProvider(AuthorizationProvider.class); RealmModel realm = session.getContext().getRealm(); ClientModel adminPermissionsClient = realm.getAdminPermissionsClient(); @@ -144,7 +144,7 @@ public class RolePolicyProvider implements PolicyProvider, PartialEvaluationPoli List roleIds = getDeepUserRoleMappings(subject).stream().map(RoleModel::getId).toList(); Stream policies = Stream.of(); - return Stream.concat(policies, policyStore.findDependentPolicies(resourceServer, resourceType.getType(), RolePolicyProviderFactory.ID, "roles", roleIds)); + return Stream.concat(policies, policyStore.findDependentPolicies(resourceServer, resourceType.getType(), groupResourceType == null ? null : groupResourceType.getType(), RolePolicyProviderFactory.ID, "roles", roleIds)); } @Override diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProvider.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProvider.java index 29b4a563751..e57438be968 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProvider.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProvider.java @@ -64,7 +64,7 @@ public class UserPolicyProvider implements PolicyProvider, PartialEvaluationPoli } @Override - public Stream getPermissions(KeycloakSession session, ResourceType resourceType, UserModel subject) { + public Stream getPermissions(KeycloakSession session, ResourceType resourceType, ResourceType groupResourceType, UserModel subject) { AuthorizationProvider provider = session.getProvider(AuthorizationProvider.class); RealmModel realm = session.getContext().getRealm(); ClientModel adminPermissionsClient = realm.getAdminPermissionsClient(); @@ -72,7 +72,7 @@ public class UserPolicyProvider implements PolicyProvider, PartialEvaluationPoli ResourceServer resourceServer = storeFactory.getResourceServerStore().findByClient(adminPermissionsClient); PolicyStore policyStore = storeFactory.getPolicyStore(); - return policyStore.findDependentPolicies(resourceServer, resourceType.getType(), UserPolicyProviderFactory.ID, "users", subject.getId()); + return policyStore.findDependentPolicies(resourceServer, resourceType.getType(), groupResourceType == null ? null : groupResourceType.getType(), UserPolicyProviderFactory.ID, "users", subject.getId()); } @Override diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java index c3601c18fd5..9bc9e501e69 100644 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/authorization/StoreFactoryCacheSession.java @@ -1046,13 +1046,13 @@ public class StoreFactoryCacheSession implements CachedStoreFactoryProvider { } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, String configValue) { - return getPolicyStoreDelegate().findDependentPolicies(resourceServer, resourceType, associatedPolicyType, configKey, configValue); + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, String configValue) { + return getPolicyStoreDelegate().findDependentPolicies(resourceServer, resourceType, groupResourceType, associatedPolicyType, configKey, configValue); } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, List configValue) { - return getPolicyStoreDelegate().findDependentPolicies(resourceServer, resourceType, associatedPolicyType, configKey, configValue); + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, List configValue) { + return getPolicyStoreDelegate().findDependentPolicies(resourceServer, resourceType, groupResourceType, associatedPolicyType, configKey, configValue); } private List cacheQuery(String cacheKey, Class queryType, Supplier> resultSupplier, BiFunction, Q> querySupplier, ResourceServer resourceServer) { diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/PolicyEntity.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/PolicyEntity.java index 4dbd570dbe6..c55a38795f0 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/PolicyEntity.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/entities/PolicyEntity.java @@ -41,7 +41,6 @@ import jakarta.persistence.OneToMany; import jakarta.persistence.Table; import jakarta.persistence.UniqueConstraint; -import org.keycloak.authorization.fgap.AdminPermissionsSchema; import org.keycloak.representations.idm.authorization.DecisionStrategy; import org.keycloak.representations.idm.authorization.Logic; @@ -69,7 +68,7 @@ import org.hibernate.annotations.Nationalized; @NamedQuery(name="findPolicyIdByResourceType", query="select p from PolicyEntity p inner join p.config c inner join fetch p.associatedPolicies a where p.resourceServer.id = :serverId and KEY(c) = 'defaultResourceType' and c like :type"), @NamedQuery(name="findPolicyIdByDependentPolices", query="select p.id from PolicyEntity p inner join p.associatedPolicies ap where p.resourceServer.id = :serverId and (ap.resourceServer.id = :serverId and ap.id = :policyId)"), @NamedQuery(name="deletePolicyByResourceServer", query="delete from PolicyEntity p where p.resourceServer.id = :serverId"), - @NamedQuery(name="findDependentPolicyByResourceTypeAndConfig", query="select p.id from PolicyEntity p inner join p.scopes s inner join p.config c inner join p.associatedPolicies ap inner join ap.config ac where p.resourceServer.id = :serverId and (s.name in ('" + AdminPermissionsSchema.VIEW + "', '" + AdminPermissionsSchema.VIEW_MEMBERS + "')) and ap.resourceServer.id = :serverId and ap.type = :associatedPolicyType and (KEY(c) = 'defaultResourceType' and c like :resourceType) and (KEY(ac) = :configKey and ac like :configValue)") + @NamedQuery(name="findDependentPolicyByResourceTypeAndConfig", query="select p.id from PolicyEntity p inner join p.scopes s inner join p.config c inner join p.associatedPolicies ap inner join ap.config ac where p.resourceServer.id = :serverId and (s.name = :scopeName) and ap.resourceServer.id = :serverId and ap.type = :associatedPolicyType and (KEY(c) = 'defaultResourceType' and c like :resourceType) and (KEY(ac) = :configKey and ac like :configValue)") } ) diff --git a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java index 61fb2a665de..0f1bf01192c 100644 --- a/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java +++ b/model/jpa/src/main/java/org/keycloak/authorization/jpa/store/JPAPolicyStore.java @@ -330,12 +330,12 @@ public class JPAPolicyStore implements PolicyStore { } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, String configValue) { - return findDependentPolicies(resourceServer, resourceType, associatedPolicyType, configKey, List.of(configValue)); + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, String configValue) { + return findDependentPolicies(resourceServer, resourceType, groupResourceType, associatedPolicyType, configKey, List.of(configValue)); } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, List configValues) { + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, List configValues) { String dbProductName = entityManager.unwrap(Session.class).doReturningWork(connection -> connection.getMetaData().getDatabaseProductName()); if (dbProductName.equals("Oracle")) { @@ -350,6 +350,12 @@ public class JPAPolicyStore implements PolicyStore { query.setParameter("configKey", configKey); query.setParameter("configValue", "%" + value + "%"); + if (AdminPermissionsSchema.GROUPS.getType().equals(groupResourceType)) { + query.setParameter("scopeName", AdminPermissionsSchema.VIEW_MEMBERS); + } else { + query.setParameter("scopeName", AdminPermissionsSchema.VIEW); + } + PolicyStore policyStore = provider.getStoreFactory().getPolicyStore(); result = Stream.concat(result, query.getResultStream().map((id) -> policyStore.findById(resourceServer, id)).filter(Objects::nonNull)); @@ -372,7 +378,13 @@ public class JPAPolicyStore implements PolicyStore { List predicates = new LinkedList<>(); predicates.add(cb.equal(from.get("resourceServer").get("id"), resourceServer.getId())); - predicates.add(scope.get("name").in(AdminPermissionsSchema.VIEW, AdminPermissionsSchema.VIEW_MEMBERS)); + + if (AdminPermissionsSchema.GROUPS.getType().equals(groupResourceType)) { + predicates.add(cb.equal(scope.get("name"), AdminPermissionsSchema.VIEW_MEMBERS)); + } else { + predicates.add(cb.equal(scope.get("name"), AdminPermissionsSchema.VIEW)); + } + predicates.add(cb.equal(associatedPolicy.get("type"), associatedPolicyType)); predicates.add(cb.equal(config.key(), "defaultResourceType")); predicates.add(cb.equal(config.value(), resourceType)); diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java index 2632614e4bc..778fcbf866f 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/AuthorizationProvider.java @@ -443,13 +443,13 @@ public final class AuthorizationProvider implements Provider { } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, String configValue) { - return policyStore.findDependentPolicies(resourceServer, resourceType, associatedPolicyType, configKey, configValue); + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, String configValue) { + return policyStore.findDependentPolicies(resourceServer, resourceType, groupResourceType, associatedPolicyType, configKey, configValue); } @Override - public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, List configValues) { - return policyStore.findDependentPolicies(resourceServer, resourceType, associatedPolicyType, configKey, configValues); + public Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, List configValues) { + return policyStore.findDependentPolicies(resourceServer, resourceType, groupResourceType, associatedPolicyType, configKey, configValues); } @Override diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluationPolicyProvider.java b/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluationPolicyProvider.java index b77ec7689a3..0c283809eb2 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluationPolicyProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluationPolicyProvider.java @@ -37,12 +37,13 @@ public interface PartialEvaluationPolicyProvider { * Returns a list of {@link Policy} instances representing the permissions that apply to a given {@code subject} when * partially evaluating the realm resources that can be accessed. * - * @param session the session - * @param resourceType the type of the resource - * @param subject the subject + * @param session the session + * @param resourceType the type of the resource + * @param groupResourceType + * @param subject the subject * @return the permissions that apply to the given {@code subject} */ - Stream getPermissions(KeycloakSession session, ResourceType resourceType, UserModel subject); + Stream getPermissions(KeycloakSession session, ResourceType resourceType, ResourceType groupResourceType, UserModel subject); /** * If partial evaluation is supported for the given {@code policy}. diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluator.java b/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluator.java index a33d6f5930a..b859d9c81ef 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluator.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/fgap/evaluation/partial/PartialEvaluator.java @@ -66,12 +66,12 @@ public final class PartialEvaluator { } // collect the result from the partial evaluation so that the filters can be applied - PartialEvaluationContext context = runEvaluation(session, adminUser, resourceType, storage, builder, queryBuilder, path); + PartialEvaluationContext context = runEvaluation(session, adminUser, resourceType, null, storage, builder, queryBuilder, path); return buildPredicates(context); } - private PartialEvaluationContext runEvaluation(KeycloakSession session, UserModel adminUser, ResourceType resourceType, PartialEvaluationStorageProvider storage, CriteriaBuilder builder, CriteriaQuery queryBuilder, Path path) { + private PartialEvaluationContext runEvaluation(KeycloakSession session, UserModel adminUser, ResourceType resourceType, ResourceType groupResourceType, PartialEvaluationStorageProvider storage, CriteriaBuilder builder, CriteriaQuery queryBuilder, Path path) { Map> cache = session.getAttributeOrDefault(PARTIAL_EVALUATION_CONTEXT_CACHE, Map.of()); if (cache.getOrDefault(adminUser.getId(), Map.of()).containsKey(resourceType.getType())) { @@ -88,7 +88,7 @@ public final class PartialEvaluator { List policyProviders = getPartialEvaluationPolicyProviders(session); for (PartialEvaluationPolicyProvider policyProvider : policyProviders) { - policyProvider.getPermissions(session, resourceType, adminUser).forEach(permission -> { + policyProvider.getPermissions(session, resourceType, groupResourceType, adminUser).forEach(permission -> { Set ids = permission.getResourceNames(); Set policies = permission.getAssociatedPolicies(); @@ -199,7 +199,7 @@ public final class PartialEvaluator { return context; } - PartialEvaluationContext evaluateGroups = runEvaluation(session, adminUser, groupResourceType, storage, builder, queryBuilder, path); + PartialEvaluationContext evaluateGroups = runEvaluation(session, adminUser, groupResourceType, groupResourceType, storage, builder, queryBuilder, path); context.setAllowedGroups(evaluateGroups.getAllowedResources()); context.setDeniedGroups(evaluateGroups.getDeniedResources()); } diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java index bf3197b67a9..3ff43d356e5 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/PolicyStore.java @@ -189,7 +189,7 @@ public interface PolicyStore { */ List findDependentPolicies(ResourceServer resourceServer, String id); - Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, String configValue); + Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, String configValue); - Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String associatedPolicyType, String configKey, List configValues); + Stream findDependentPolicies(ResourceServer resourceServer, String resourceType, String groupResourceType, String associatedPolicyType, String configKey, List configValues); } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java index 710318204b5..48d46c514de 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/AbstractPermissionTest.java @@ -47,6 +47,7 @@ import org.keycloak.testframework.realm.ManagedRealm; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; public abstract class AbstractPermissionTest { @@ -56,7 +57,7 @@ public abstract class AbstractPermissionTest { @InjectClient(attachTo = Constants.ADMIN_PERMISSIONS_CLIENT_ID) ManagedClient client; - protected static PermissionsResource getPermissionsResource(ManagedClient client) { + protected PermissionsResource getPermissionsResource(ManagedClient client) { return client.admin().authorization().permissions(); } @@ -64,17 +65,22 @@ public abstract class AbstractPermissionTest { return client.admin().authorization().policies(); } - protected static ScopePermissionsResource getScopePermissionsResource(ManagedClient client) { + protected ScopePermissionsResource getScopePermissionsResource(ManagedClient client) { return getPermissionsResource(client).scope(); } - protected static void createPermission(ManagedClient client, ScopePermissionRepresentation permission) { + protected void createPermission(ManagedClient client, ScopePermissionRepresentation permission) { createPermission(client, permission, Response.Status.CREATED); } - protected static void createPermission(ManagedClient client, ScopePermissionRepresentation permission, Response.Status expected) { + protected void createPermission(ManagedClient client, ScopePermissionRepresentation permission, Response.Status expected) { try (Response response = getScopePermissionsResource(client).create(permission)) { assertEquals(expected.getStatusCode(), response.getStatus()); + if (Response.Status.CREATED.equals(expected)) { + ScopePermissionRepresentation created = getScopePermissionsResource(client).findByName(permission.getName()); + assertNotNull(created); + permission.setId(created.getId()); + } } } @@ -115,11 +121,11 @@ public abstract class AbstractPermissionTest { } } - protected static UserPolicyRepresentation createUserPolicy(ManagedRealm realm, ManagedClient client, String name, String... userIds) { + protected UserPolicyRepresentation createUserPolicy(ManagedRealm realm, ManagedClient client, String name, String... userIds) { return createUserPolicy(Logic.POSITIVE, realm, client, name, userIds); } - protected static UserPolicyRepresentation createUserPolicy(Logic logic, ManagedRealm realm, ManagedClient client, String name, String... userIds) { + protected UserPolicyRepresentation createUserPolicy(Logic logic, ManagedRealm realm, ManagedClient client, String name, String... userIds) { UserPolicyRepresentation policy = new UserPolicyRepresentation(); policy.setName(name); for (String userId : userIds) { @@ -138,10 +144,10 @@ public abstract class AbstractPermissionTest { return policy; } - protected static GroupPolicyRepresentation createGroupPolicy(ManagedRealm realm, ManagedClient client, String name, String groupId, Logic logic) { + protected GroupPolicyRepresentation createGroupPolicy(ManagedRealm realm, ManagedClient client, String name, Logic logic, String... groupIds) { GroupPolicyRepresentation policy = new GroupPolicyRepresentation(); policy.setName(name); - policy.addGroup(groupId); + policy.addGroup(groupIds); policy.setLogic(logic); try (Response response = client.admin().authorization().policies().group().create(policy)) { assertThat(response.getStatus(), equalTo(Response.Status.CREATED.getStatusCode())); @@ -153,7 +159,7 @@ public abstract class AbstractPermissionTest { return policy; } - protected static RolePolicyRepresentation createRolePolicy(ManagedRealm realm, ManagedClient client, String name, String roleId, Logic logic) { + protected RolePolicyRepresentation createRolePolicy(ManagedRealm realm, ManagedClient client, String name, String roleId, Logic logic) { RolePolicyRepresentation policy = new RolePolicyRepresentation(); policy.setName(name); policy.addRole(roleId); @@ -168,7 +174,7 @@ public abstract class AbstractPermissionTest { return policy; } - protected static ClientPolicyRepresentation createClientPolicy(ManagedRealm realm, ManagedClient client, String name, String... clientIds) { + protected ClientPolicyRepresentation createClientPolicy(ManagedRealm realm, ManagedClient client, String name, String... clientIds) { ClientPolicyRepresentation policy = new ClientPolicyRepresentation(); policy.setName(name); for (String clientId : clientIds) { @@ -187,7 +193,7 @@ public abstract class AbstractPermissionTest { return policy; } - protected static ScopePermissionRepresentation createAllPermission(ManagedClient client, String resourceType, AbstractPolicyRepresentation policy, Set scopes) { + protected ScopePermissionRepresentation createAllPermission(ManagedClient client, String resourceType, AbstractPolicyRepresentation policy, Set scopes) { ScopePermissionRepresentation permission = PermissionBuilder.create() .resourceType(resourceType) .scopes(scopes) @@ -217,6 +223,9 @@ public abstract class AbstractPermissionTest { } protected ScopePermissionRepresentation createGroupPermission(GroupRepresentation group, Set scopes, AbstractPolicyRepresentation... policies) { - return createPermission(client, group.getId(), AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, scopes, policies); + return createGroupPermission(Set.of(group), scopes, policies); + } + protected ScopePermissionRepresentation createGroupPermission(Set groups, Set scopes, AbstractPolicyRepresentation... policies) { + return createPermission(client, groups.stream().map(GroupRepresentation::getId).collect(Collectors.toSet()), AdminPermissionsSchema.GROUPS_RESOURCE_TYPE, scopes, policies); } } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java index 3d87cb513f7..0ee8885f93d 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/FineGrainedPermissionsUsersTest.java @@ -21,7 +21,7 @@ import org.keycloak.testframework.util.ApiUtil; import org.junit.jupiter.api.Test; -import static org.keycloak.authorization.fgap.AdminPermissionsSchema.VIEW; +import static org.keycloak.authorization.fgap.AdminPermissionsSchema.VIEW_MEMBERS; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -189,7 +189,7 @@ public class FineGrainedPermissionsUsersTest extends AbstractPermissionTest { if (grp1ViewPermissions) { UserPolicyRepresentation userPolicy = createUserPolicy(realm, client, "allow-test-user", testUserId); - createGroupPermission(groups.get(0), Set.of(VIEW), userPolicy); + createGroupPermission(groups.get(0), Set.of(VIEW_MEMBERS), userPolicy); } Keycloak testUserClient = KeycloakBuilder.builder() diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeFilteringTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeFilteringTest.java index 7bcccf63936..bc98a719ae3 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeFilteringTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/GroupResourceTypeFilteringTest.java @@ -26,21 +26,31 @@ import jakarta.ws.rs.core.Response; import org.keycloak.admin.client.Keycloak; import org.keycloak.admin.client.resource.GroupResource; import org.keycloak.representations.idm.GroupRepresentation; +import org.keycloak.representations.idm.UserRepresentation; +import org.keycloak.representations.idm.authorization.GroupPolicyRepresentation; import org.keycloak.representations.idm.authorization.Logic; +import org.keycloak.representations.idm.authorization.ScopePermissionRepresentation; import org.keycloak.representations.idm.authorization.UserPolicyRepresentation; import org.keycloak.testframework.annotations.InjectAdminClient; import org.keycloak.testframework.annotations.InjectUser; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import org.keycloak.testframework.realm.GroupConfigBuilder; import org.keycloak.testframework.realm.ManagedUser; +import org.keycloak.testframework.realm.UserConfigBuilder; import org.keycloak.testframework.util.ApiUtil; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.keycloak.authorization.fgap.AdminPermissionsSchema.GROUPS_RESOURCE_TYPE; +import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE_MEMBERS; +import static org.keycloak.authorization.fgap.AdminPermissionsSchema.MANAGE_MEMBERSHIP; import static org.keycloak.authorization.fgap.AdminPermissionsSchema.USERS_RESOURCE_TYPE; import static org.keycloak.authorization.fgap.AdminPermissionsSchema.VIEW; +import static org.keycloak.authorization.fgap.AdminPermissionsSchema.VIEW_MEMBERS; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -172,4 +182,67 @@ public class GroupResourceTypeFilteringTest extends AbstractPermissionTest { groups = userAlice.admin().groups(); assertEquals(2, groups.size()); } + + @Test + public void testViewGroupNotAffectQueryUsers() { + // create groups + String companyId = ApiUtil.getCreatedId(realm.admin().groups().add(GroupConfigBuilder.create().name("Company").build())); + String sub1Id = ApiUtil.getCreatedId(realm.admin().groups().group(companyId).subGroup(GroupConfigBuilder.create().name("Sub1").build())); + String sub2Id = ApiUtil.getCreatedId(realm.admin().groups().group(companyId).subGroup(GroupConfigBuilder.create().name("Sub2").build())); + String depAId = ApiUtil.getCreatedId(realm.admin().groups().group(sub1Id).subGroup(GroupConfigBuilder.create().name("DepartmentA").build())); + String depBId = ApiUtil.getCreatedId(realm.admin().groups().group(sub1Id).subGroup(GroupConfigBuilder.create().name("DepartmentB").build())); + String depCId = ApiUtil.getCreatedId(realm.admin().groups().group(sub2Id).subGroup(GroupConfigBuilder.create().name("DepartmentC").build())); + String depDId = ApiUtil.getCreatedId(realm.admin().groups().group(sub2Id).subGroup(GroupConfigBuilder.create().name("DepartmentD").build())); + realm.cleanup().add(r -> r.groups().group(companyId).remove()); + + GroupRepresentation company = realm.admin().groups().group(companyId).toRepresentation(); + GroupRepresentation sub1 = realm.admin().groups().group(sub1Id).toRepresentation(); + GroupRepresentation sub2 = realm.admin().groups().group(sub2Id).toRepresentation(); + GroupRepresentation depA = realm.admin().groups().group(depAId).toRepresentation(); + GroupRepresentation depB = realm.admin().groups().group(depBId).toRepresentation(); + GroupRepresentation depC = realm.admin().groups().group(depCId).toRepresentation(); + GroupRepresentation depD = realm.admin().groups().group(depDId).toRepresentation(); + + // create members + List userIds = List.of( + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("company-admin").groups("/Company").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("sub1-admin").groups("/Company/Sub1").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("sub2-admin").groups("/Company/Sub2").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("department-a-member").groups("/Company/Sub1/DepartmentA").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("department-b-member").groups("/Company/Sub1/DepartmentB").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("department-c-member").groups("/Company/Sub2/DepartmentC").build())), + ApiUtil.getCreatedId(realm.admin().users().create(UserConfigBuilder.create().username("department-d-member").groups("/Company/Sub2/DepartmentD").build())) + ); + realm.cleanup().add(r -> userIds.forEach(userId -> r.users().delete(userId).close())); + + // add myadmin as member of Sub1 + UserRepresentation myadmin = realm.admin().users().search("myadmin").get(0); + realm.admin().users().get(myadmin.getId()).joinGroup(sub1.getId()); + + // create policies + GroupPolicyRepresentation allowCompany = createGroupPolicy(realm, client, "Allow Company", Logic.POSITIVE, sub1.getId(), sub2.getId()); + GroupPolicyRepresentation allowSub1 = createGroupPolicy(realm, client, "Allow Sub1", Logic.POSITIVE, sub1.getId()); + GroupPolicyRepresentation allowSub2 = createGroupPolicy(realm, client, "Allow Sub2", Logic.POSITIVE, sub2.getId()); + + // create permissions + createGroupPermission(Set.of(sub1, depA, depB), Set.of(VIEW, VIEW_MEMBERS, MANAGE_MEMBERS, MANAGE_MEMBERSHIP), allowSub1); + createGroupPermission(Set.of(sub2, depC, depD), Set.of(VIEW, VIEW_MEMBERS, MANAGE_MEMBERS, MANAGE_MEMBERSHIP), allowSub2); + ScopePermissionRepresentation companyPermission = createGroupPermission(company, Set.of(VIEW), allowCompany); + + // test listing users + List search = realmAdminClient.realm(realm.getName()).users().search(null, -1, -1).stream().map(UserRepresentation::getUsername).toList(); + assertThat(search, containsInAnyOrder("myadmin", "sub1-admin", "department-a-member", "department-b-member")); + + // grant access to view company members + companyPermission.setScopes(Set.of(VIEW, VIEW_MEMBERS)); + realm.admin().clients().get(client.getId()).authorization().permissions().scope().findById(companyPermission.getId()).update(companyPermission); + search = realmAdminClient.realm(realm.getName()).users().search(null, -1, -1).stream().map(UserRepresentation::getUsername).toList(); + assertThat(search, containsInAnyOrder("company-admin", "myadmin", "sub1-admin", "department-a-member", "department-b-member")); + + // create negative permission on view-members of company group + GroupPolicyRepresentation disallowSubAdmins = createGroupPolicy(realm, client, "Disallow Subadmins", Logic.NEGATIVE, sub1.getId(), sub2.getId()); + createGroupPermission(company, Set.of(VIEW_MEMBERS), disallowSubAdmins); + search = realmAdminClient.realm(realm.getName()).users().search(null, -1, -1).stream().map(UserRepresentation::getUsername).toList(); + assertThat(search, containsInAnyOrder("myadmin", "sub1-admin", "department-a-member", "department-b-member")); + } } diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java index f1dc35f2e48..cc23e987752 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeEvaluationTest.java @@ -497,11 +497,11 @@ public class UserResourceTypeEvaluationTest extends AbstractPermissionTest { realm.admin().users().get(myadmin.getId()).joinGroup(testAdminsGroup.getId()); // Create user permission allowing to 'view' all users by members of 'test_admins' group - GroupPolicyRepresentation allowAdmins = createGroupPolicy(realm, client, "Allow 'test_admins'", testAdminsGroup.getId(), Logic.POSITIVE); + GroupPolicyRepresentation allowAdmins = createGroupPolicy(realm, client, "Allow 'test_admins'", Logic.POSITIVE, testAdminsGroup.getId()); createAllPermission(client, usersType, allowAdmins, Set.of(VIEW)); // Create group permission denying to 'manage' specific group: 'test_admins' by members of 'test_admins' - GroupPolicyRepresentation denyAdmins = createGroupPolicy(realm, client, "Deny Policy", testAdminsGroup.getId(), Logic.NEGATIVE); + GroupPolicyRepresentation denyAdmins = createGroupPolicy(realm, client, "Deny Policy", Logic.NEGATIVE, testAdminsGroup.getId()); createGroupPermission(testAdminsGroup, Set.of(MANAGE), denyAdmins); UserRepresentation representation = realmAdminClient.realm(realm.getName()).users().get(myadmin.getId()).toRepresentation(); diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java index d95f40b10b5..aab16d9d7a4 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/authz/fgap/UserResourceTypeFilteringTest.java @@ -165,7 +165,7 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest { String adminUserId = realm.admin().users().search("myadmin").get(0).getId(); String groupId = ApiUtil.getCreatedId(response); realm.admin().users().get(adminUserId).joinGroup(groupId); - GroupPolicyRepresentation policy = createGroupPolicy(realm, client, "Admin Group Policy", groupId, Logic.POSITIVE); + GroupPolicyRepresentation policy = createGroupPolicy(realm, client, "Admin Group Policy", Logic.POSITIVE, groupId); createPermission(client, "user-9", usersType, Set.of(VIEW), policy); } @@ -216,7 +216,7 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest { try (Response response = realm.admin().groups().add(rep)) { String groupId = ApiUtil.getCreatedId(response); realm.admin().users().get(adminUserId).joinGroup(groupId); - groupPolicy = createGroupPolicy(realm, client, "Admin Group Policy", groupId, Logic.POSITIVE); + groupPolicy = createGroupPolicy(realm, client, "Admin Group Policy", Logic.POSITIVE, groupId); } createPermission(client, "user-9", usersType, Set.of(VIEW), rolePolicy, groupPolicy); @@ -511,7 +511,7 @@ public class UserResourceTypeFilteringTest extends AbstractPermissionTest { .email(username + "@test") .build())); realm.admin().users().get(userId).roles().clientLevel(testClient.getId()).add(List.of(role)); - realm.cleanup().add(r -> r.users().delete(userId)); + realm.cleanup().add(r -> r.users().delete(userId).close()); } // Grant myadmin permission to view user_x and user_y, and to view the test client