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 dcba8cbffd..3a1bb83b59 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 @@ -166,8 +166,7 @@ public class JPAPolicyStore implements PolicyStore { } } break; - case OWNER_IS_NOT_NULL: - predicates.add(builder.isNotNull(root.get("owner"))); + case ANY_OWNER: break; case CONFIG: if (value.length != 2) { @@ -186,7 +185,7 @@ public class JPAPolicyStore implements PolicyStore { } }); - if (!attributes.containsKey(Policy.FilterOption.OWNER) && !attributes.containsKey(Policy.FilterOption.OWNER_IS_NOT_NULL)) { + if (!attributes.containsKey(Policy.FilterOption.OWNER) && !attributes.containsKey(Policy.FilterOption.ANY_OWNER)) { predicates.add(builder.isNull(root.get("owner"))); } diff --git a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java index b4d0c85df9..51d9172542 100644 --- a/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java +++ b/model/map/src/main/java/org/keycloak/models/map/authorization/MapPolicyStore.java @@ -36,6 +36,7 @@ import org.keycloak.representations.idm.authorization.AbstractPolicyRepresentati import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -144,10 +145,11 @@ public class MapPolicyStore implements PolicyStore { ModelCriteriaBuilder mcb = forResourceServer(resourceServerId).and( attributes.entrySet().stream() .map(this::filterEntryToModelCriteriaBuilder) + .filter(Objects::nonNull) .toArray(ModelCriteriaBuilder[]::new) ); - if (!attributes.containsKey(Policy.FilterOption.OWNER) && !attributes.containsKey(Policy.FilterOption.OWNER_IS_NOT_NULL)) { + if (!attributes.containsKey(Policy.FilterOption.OWNER) && !attributes.containsKey(Policy.FilterOption.ANY_OWNER)) { mcb = mcb.compare(SearchableFields.OWNER, Operator.NOT_EXISTS); } @@ -180,9 +182,8 @@ public class MapPolicyStore implements PolicyStore { return mcb; } - case OWNER_IS_NOT_NULL: - return policyStore.createCriteriaBuilder() - .compare(SearchableFields.OWNER, Operator.EXISTS); + case ANY_OWNER: + return null; case CONFIG: if (value.length != 2) { throw new IllegalArgumentException("Config filter option requires value with two items: [config_name, expected_config_value]"); diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/model/Policy.java b/server-spi-private/src/main/java/org/keycloak/authorization/model/Policy.java index 8d5d35ba60..109e31afd1 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/model/Policy.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/model/Policy.java @@ -48,13 +48,14 @@ public interface Policy { ID("id", SearchableFields.ID), PERMISSION("permission", SearchableFields.TYPE), OWNER("owner", SearchableFields.OWNER), - OWNER_IS_NOT_NULL("owner_is_not_null", SearchableFields.OWNER), + ANY_OWNER("owner.any", SearchableFields.OWNER), RESOURCE_ID("resources.id", SearchableFields.RESOURCE_ID), SCOPE_ID("scopes.id", SearchableFields.SCOPE_ID), CONFIG("config", SearchableFields.CONFIG), TYPE("type", SearchableFields.TYPE), NAME("name", SearchableFields.NAME); + public static final String[] EMPTY_FILTER = new String[0]; private final String name; private final SearchableModelField searchableModelField; diff --git a/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/GroupSynchronizer.java b/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/GroupSynchronizer.java index 4eabeb9329..c10ea5f1a6 100644 --- a/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/GroupSynchronizer.java +++ b/server-spi-private/src/main/java/org/keycloak/authorization/store/syncronization/GroupSynchronizer.java @@ -49,6 +49,7 @@ public class GroupSynchronizer implements Synchronizer search = policyStore.findByResourceServer(attributes, null, -1, -1); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java index d5f927fff0..b5f4a180d3 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/UserManagedPermissionServiceTest.java @@ -89,6 +89,7 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest .subGroups(Arrays.asList(GroupBuilder.create().name("group_b").build())) .build()) .group(GroupBuilder.create().name("group_c").build()) + .group(GroupBuilder.create().name("group_remove").build()) .user(UserBuilder.create().username("marta").password("password") .addRoles("uma_authorization", "uma_protection") .role("resource-server-test", "uma_protection")) @@ -937,6 +938,60 @@ public class UserManagedPermissionServiceTest extends AbstractResourceServerTest assertTrue(policies.isEmpty()); } + @Test + public void testRemovePoliciesOnGroupDelete() { + ResourceRepresentation resource = new ResourceRepresentation(); + + resource.setName("Resource A"); + resource.setOwnerManagedAccess(true); + resource.setOwner("marta"); + resource.addScope("Scope A", "Scope B", "Scope C"); + + resource = getAuthzClient().protection().resource().create(resource); + + UmaPermissionRepresentation newPermission = new UmaPermissionRepresentation(); + + newPermission.setName("Custom User-Managed Permission"); + newPermission.addGroup("/group_remove"); + + ProtectionResource protection = getAuthzClient().protection("marta", "password"); + + protection.policy(resource.getId()).create(newPermission); + + getTestingClient().server().run((RunOnServer) UserManagedPermissionServiceTest::testRemovePoliciesOnGroupDelete); + } + + private static void testRemovePoliciesOnGroupDelete(KeycloakSession session) { + RealmModel realm = session.realms().getRealmByName("authz-test"); + ClientModel client = realm.getClientByClientId("resource-server-test"); + AuthorizationProvider provider = session.getProvider(AuthorizationProvider.class); + UserModel user = session.users().getUserByUsername(realm, "marta"); + Map filters = new HashMap<>(); + + filters.put(Policy.FilterOption.TYPE, new String[] {"uma"}); + filters.put(OWNER, new String[] {user.getId()}); + + List policies = provider.getStoreFactory().getPolicyStore() + .findByResourceServer(filters, client.getId(), -1, -1); + assertEquals(1, policies.size()); + + Policy policy = policies.get(0); + assertFalse(policy.getResources().isEmpty()); + + Resource resource = policy.getResources().iterator().next(); + assertEquals("Resource A", resource.getName()); + + realm.removeGroup(realm.searchForGroupByNameStream("group_remove", -1, -1).findAny().get()); + + filters = new HashMap<>(); + + filters.put(OWNER, new String[] {user.getId()}); + + policies = provider.getStoreFactory().getPolicyStore() + .findByResourceServer(filters, client.getId(), -1, -1); + assertTrue(policies.isEmpty()); + } + private List getAssociatedPolicies(UmaPermissionRepresentation permission) { return getClient(getRealm()).authorization().policies().policy(permission.getId()).associatedPolicies(); }