From 873c62bbef68b77f8c8fc5eee381dc01409b9152 Mon Sep 17 00:00:00 2001 From: Pedro Igor Date: Thu, 19 Dec 2019 21:25:48 -0300 Subject: [PATCH] [KEYCLOAK-12569] - User cannot be deleted if he has owned resources / permission tickets Co-authored-by: mhajas --- .../user/UserPolicyProviderFactory.java | 5 ++- .../syncronization/UserSynchronizer.java | 24 ++++++++++++++ .../authz/PermissionManagementTest.java | 31 +++++++++++++++++++ 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java index 6f4c0bf74d6..14622c5640d 100644 --- a/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java +++ b/authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/user/UserPolicyProviderFactory.java @@ -196,9 +196,8 @@ public class UserPolicyProviderFactory implements PolicyProviderFactory { ProviderFactory providerFactory = factory.getProviderFactory(AuthorizationProvider.class); AuthorizationProvider authorizationProvider = providerFactory.create(event.getKeycloakSession()); + removeFromUserPermissionTickets(event, authorizationProvider); removeUserResources(event, authorizationProvider); removeFromUserPolicies(event, authorizationProvider); } @@ -104,4 +107,25 @@ public class UserSynchronizer implements Synchronizer { } }); } + + private void removeFromUserPermissionTickets(UserRemovedEvent event, AuthorizationProvider authorizationProvider) { + StoreFactory storeFactory = authorizationProvider.getStoreFactory(); + PermissionTicketStore ticketStore = storeFactory.getPermissionTicketStore(); + UserModel userModel = event.getUser(); + Map attributes = new HashMap<>(); + + attributes.put(PermissionTicket.OWNER, userModel.getId()); + + for (PermissionTicket ticket : ticketStore.find(attributes, null, -1, -1)) { + ticketStore.delete(ticket.getId()); + } + + attributes = new HashMap<>(); + + attributes.put(PermissionTicket.REQUESTER, userModel.getId()); + + for (PermissionTicket ticket : ticketStore.find(attributes, null, -1, -1)) { + ticketStore.delete(ticket.getId()); + } + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java index 79b22db928e..223f3e1c858 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/authz/PermissionManagementTest.java @@ -16,9 +16,14 @@ */ package org.keycloak.testsuite.authz; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -29,6 +34,7 @@ import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.stream.Collectors; import org.junit.Test; import org.keycloak.admin.client.resource.AuthorizationResource; @@ -36,6 +42,7 @@ import org.keycloak.admin.client.resource.ResourceScopesResource; import org.keycloak.authorization.client.AuthzClient; import org.keycloak.authorization.client.util.HttpResponseException; import org.keycloak.jose.jws.JWSInput; +import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.representations.idm.authorization.AuthorizationRequest; import org.keycloak.representations.idm.authorization.Permission; import org.keycloak.representations.idm.authorization.PermissionRequest; @@ -69,6 +76,30 @@ public class PermissionManagementTest extends AbstractResourceServerTest { assertPersistence(response, resource); } + @Test + public void removeUserWithPermissionTicketTest() throws Exception { + String userToRemoveID = createUser(REALM_NAME, "user-to-remove", "password"); + + ResourceRepresentation resource = addResource("Resource A", "kolo", true); + AuthzClient authzClient = getAuthzClient(); + PermissionResponse response = authzClient.protection("user-to-remove", "password").permission().create(new PermissionRequest(resource.getId())); + AuthorizationRequest request = new AuthorizationRequest(); + request.setTicket(response.getTicket()); + request.setClaimToken(authzClient.obtainAccessToken("user-to-remove", "password").getToken()); + try { + authzClient.authorization().authorize(request); + } catch (Exception e) { + + } + assertPersistence(response, resource); + + // Remove the user and expect the user and also hers permission tickets are successfully removed + adminClient.realm(REALM_NAME).users().delete(userToRemoveID); + assertThat(adminClient.realm(REALM_NAME).users().list().stream().map(UserRepresentation::getId).collect(Collectors.toList()), + not(hasItem(userToRemoveID))); + assertThat(getAuthzClient().protection().permission().findByResource(resource.getId()), is(empty())); + } + @Test public void testCreatePermissionTicketWithResourceId() throws Exception { ResourceRepresentation resource = addResource("Resource A", "kolo", true);