[KEYCLOAK-12569] - User cannot be deleted if he has owned resources / permission tickets

Co-authored-by: mhajas <mhajas@redhat.com>
This commit is contained in:
Pedro Igor 2019-12-19 21:25:48 -03:00 committed by Stian Thorgersen
parent 8a022da30d
commit 873c62bbef
3 changed files with 57 additions and 3 deletions

View file

@ -196,9 +196,8 @@ public class UserPolicyProviderFactory implements PolicyProviderFactory<UserPoli
} }
try { try {
if (users.isEmpty()) { // just update the policy, let the UserSynchronizer to actually remove the policy if necessary
policyStore.delete(policy.getId()); if (!users.isEmpty()) {
} else {
policy.putConfig("users", JsonSerialization.writeValueAsString(users)); policy.putConfig("users", JsonSerialization.writeValueAsString(users));
} }
} catch (IOException e) { } catch (IOException e) {

View file

@ -23,9 +23,11 @@ import java.util.Map;
import java.util.Set; import java.util.Set;
import org.keycloak.authorization.AuthorizationProvider; import org.keycloak.authorization.AuthorizationProvider;
import org.keycloak.authorization.model.PermissionTicket;
import org.keycloak.authorization.model.Policy; import org.keycloak.authorization.model.Policy;
import org.keycloak.authorization.model.ResourceServer; import org.keycloak.authorization.model.ResourceServer;
import org.keycloak.authorization.policy.provider.PolicyProviderFactory; import org.keycloak.authorization.policy.provider.PolicyProviderFactory;
import org.keycloak.authorization.store.PermissionTicketStore;
import org.keycloak.authorization.store.PolicyStore; import org.keycloak.authorization.store.PolicyStore;
import org.keycloak.authorization.store.ResourceServerStore; import org.keycloak.authorization.store.ResourceServerStore;
import org.keycloak.authorization.store.ResourceStore; import org.keycloak.authorization.store.ResourceStore;
@ -47,6 +49,7 @@ public class UserSynchronizer implements Synchronizer<UserRemovedEvent> {
ProviderFactory<AuthorizationProvider> providerFactory = factory.getProviderFactory(AuthorizationProvider.class); ProviderFactory<AuthorizationProvider> providerFactory = factory.getProviderFactory(AuthorizationProvider.class);
AuthorizationProvider authorizationProvider = providerFactory.create(event.getKeycloakSession()); AuthorizationProvider authorizationProvider = providerFactory.create(event.getKeycloakSession());
removeFromUserPermissionTickets(event, authorizationProvider);
removeUserResources(event, authorizationProvider); removeUserResources(event, authorizationProvider);
removeFromUserPolicies(event, authorizationProvider); removeFromUserPolicies(event, authorizationProvider);
} }
@ -104,4 +107,25 @@ public class UserSynchronizer implements Synchronizer<UserRemovedEvent> {
} }
}); });
} }
private void removeFromUserPermissionTickets(UserRemovedEvent event, AuthorizationProvider authorizationProvider) {
StoreFactory storeFactory = authorizationProvider.getStoreFactory();
PermissionTicketStore ticketStore = storeFactory.getPermissionTicketStore();
UserModel userModel = event.getUser();
Map<String, String> 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());
}
}
} }

View file

@ -16,9 +16,14 @@
*/ */
package org.keycloak.testsuite.authz; 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.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
@ -29,6 +34,7 @@ import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.Iterator; import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.stream.Collectors;
import org.junit.Test; import org.junit.Test;
import org.keycloak.admin.client.resource.AuthorizationResource; 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.AuthzClient;
import org.keycloak.authorization.client.util.HttpResponseException; import org.keycloak.authorization.client.util.HttpResponseException;
import org.keycloak.jose.jws.JWSInput; 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.AuthorizationRequest;
import org.keycloak.representations.idm.authorization.Permission; import org.keycloak.representations.idm.authorization.Permission;
import org.keycloak.representations.idm.authorization.PermissionRequest; import org.keycloak.representations.idm.authorization.PermissionRequest;
@ -69,6 +76,30 @@ public class PermissionManagementTest extends AbstractResourceServerTest {
assertPersistence(response, resource); 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 @Test
public void testCreatePermissionTicketWithResourceId() throws Exception { public void testCreatePermissionTicketWithResourceId() throws Exception {
ResourceRepresentation resource = addResource("Resource A", "kolo", true); ResourceRepresentation resource = addResource("Resource A", "kolo", true);