[KEYCLOAK-10675] - Deleting an Identity Provider doesn't remove the associated IdP Mapper for that user

This commit is contained in:
Pedro Igor 2020-03-19 19:12:27 -03:00 committed by Marek Posolda
parent 1b8369c7d5
commit b812159193
13 changed files with 176 additions and 4 deletions

View file

@ -20,6 +20,7 @@ package org.keycloak.models.cache.infinispan;
import org.jboss.logging.Logger;
import org.keycloak.cluster.ClusterProvider;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.cache.infinispan.events.InvalidationEvent;
import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.component.ComponentModel;
@ -48,6 +49,7 @@ import org.keycloak.models.cache.infinispan.events.UserFederationLinkRemovedEven
import org.keycloak.models.cache.infinispan.events.UserFederationLinkUpdatedEvent;
import org.keycloak.models.cache.infinispan.events.UserFullInvalidationEvent;
import org.keycloak.models.cache.infinispan.events.UserUpdatedEvent;
import org.keycloak.models.cache.infinispan.stream.InIdentityProviderPredicate;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.models.utils.ReadOnlyUserModelDelegate;
import org.keycloak.storage.CacheableStorageProviderModel;
@ -844,6 +846,12 @@ public class UserCacheSession implements UserCache {
return getDelegate().removeFederatedIdentity(realm, user, socialProvider);
}
@Override
public void preRemove(RealmModel realm, IdentityProviderModel provider) {
cache.addInvalidations(InIdentityProviderPredicate.create().provider(provider.getAlias()), invalidations);
getDelegate().preRemove(realm, provider);
}
@Override
public void grantToAllUsers(RealmModel realm, RoleModel role) {
addRealmInvalidation(realm.getId()); // easier to just invalidate whole realm

View file

@ -29,7 +29,7 @@ import java.util.Set;
*
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
*/
public class CachedFederatedIdentityLinks extends AbstractRevisioned implements InRealm {
public class CachedFederatedIdentityLinks extends AbstractRevisioned implements InRealm, InIdentityProvider {
private final String realmId;
private final Set<FederatedIdentityModel> federatedIdentities = new HashSet<>();
@ -48,4 +48,10 @@ public class CachedFederatedIdentityLinks extends AbstractRevisioned implements
public Set<FederatedIdentityModel> getFederatedIdentities() {
return federatedIdentities;
}
@Override
public boolean contains(String alias) {
return federatedIdentities.stream().anyMatch(
federatedIdentityModel -> federatedIdentityModel.getIdentityProvider().equals(alias));
}
}

View file

@ -0,0 +1,11 @@
package org.keycloak.models.cache.infinispan.entities;
import java.util.List;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public interface InIdentityProvider extends Revisioned {
boolean contains(String providerId);
}

View file

@ -0,0 +1,70 @@
package org.keycloak.models.cache.infinispan.stream;
import org.keycloak.models.cache.infinispan.entities.InIdentityProvider;
import org.keycloak.models.cache.infinispan.entities.InRealm;
import org.keycloak.models.cache.infinispan.entities.Revisioned;
import java.io.IOException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
import java.io.Serializable;
import java.util.Map;
import java.util.function.Predicate;
import org.infinispan.commons.marshall.Externalizer;
import org.infinispan.commons.marshall.MarshallUtil;
import org.infinispan.commons.marshall.SerializeWith;
/**
* @author <a href="mailto:psilva@redhat.com">Pedro Igor</a>
*/
@SerializeWith(InIdentityProviderPredicate.ExternalizerImpl.class)
public class InIdentityProviderPredicate implements Predicate<Map.Entry<String, Revisioned>>, Serializable {
private String id;
public static InIdentityProviderPredicate create() {
return new InIdentityProviderPredicate();
}
public InIdentityProviderPredicate provider(String id) {
this.id = id;
return this;
}
@Override
public boolean test(Map.Entry<String, Revisioned> entry) {
Object value = entry.getValue();
if (value == null) return false;
if (!(value instanceof InIdentityProvider)) return false;
return ((InIdentityProvider)value).contains(id);
}
public static class ExternalizerImpl implements Externalizer<InIdentityProviderPredicate> {
private static final int VERSION_1 = 1;
@Override
public void writeObject(ObjectOutput output, InIdentityProviderPredicate obj) throws IOException {
output.writeByte(VERSION_1);
MarshallUtil.marshallString(obj.id, output);
}
@Override
public InIdentityProviderPredicate readObject(ObjectInput input) throws IOException, ClassNotFoundException {
switch (input.readByte()) {
case VERSION_1:
return readObjectVersion1(input);
default:
throw new IOException("Unknown version");
}
}
public InIdentityProviderPredicate readObjectVersion1(ObjectInput input) throws IOException, ClassNotFoundException {
InIdentityProviderPredicate res = new InIdentityProviderPredicate();
res.id = MarshallUtil.unmarshallString(input);
return res;
}
}
}

View file

@ -26,6 +26,7 @@ import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
@ -191,6 +192,13 @@ public class JpaUserProvider implements UserProvider, UserCredentialStore {
}
}
@Override
public void preRemove(RealmModel realm, IdentityProviderModel provider) {
em.createNamedQuery("deleteFederatedIdentityByProvider")
.setParameter("realmId", realm.getId())
.setParameter("providerAlias", provider.getAlias()).executeUpdate();
}
@Override
public void addConsent(RealmModel realm, String userId, UserConsentModel consent) {
String clientId = consent.getClient().getId();

View file

@ -38,6 +38,7 @@ import java.io.Serializable;
@NamedQuery(name= "findFederatedIdentityByUserAndProvider", query="select link from FederatedIdentityEntity link where link.user = :user and link.identityProvider = :identityProvider"),
@NamedQuery(name= "findUserByFederatedIdentityAndRealm", query="select link.user from FederatedIdentityEntity link where link.realmId = :realmId and link.identityProvider = :identityProvider and link.userId = :userId"),
@NamedQuery(name= "deleteFederatedIdentityByRealm", query="delete from FederatedIdentityEntity social where social.user IN (select u from UserEntity u where realmId=:realmId)"),
@NamedQuery(name= "deleteFederatedIdentityByProvider", query="delete from FederatedIdentityEntity fdi where fdi.realmId = :realmId and fdi.identityProvider = :providerAlias "),
@NamedQuery(name= "deleteFederatedIdentityByRealmAndLink", query="delete from FederatedIdentityEntity social where social.user IN (select u from UserEntity u where realmId=:realmId and u.federationLink=:link)"),
@NamedQuery(name= "deleteFederatedIdentityByUser", query="delete from FederatedIdentityEntity social where social.user = :user")
})

View file

@ -27,6 +27,7 @@ import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelDuplicateException;
import org.keycloak.models.ModelException;
@ -210,6 +211,13 @@ public class JpaUserFederatedStorageProvider implements
return true;
}
@Override
public void preRemove(RealmModel realm, IdentityProviderModel provider) {
em.createNamedQuery("deleteBrokerLinkByIdentityProvider")
.setParameter("realmId", realm.getId())
.setParameter("providerAlias", provider.getAlias());
}
private BrokerLinkEntity getBrokerLinkEntity(RealmModel realm, String userId, String socialProvider) {
TypedQuery<BrokerLinkEntity> query = em.createNamedQuery("findBrokerLinkByUserAndProvider", BrokerLinkEntity.class)
.setParameter("userId", userId)

View file

@ -38,6 +38,7 @@ import java.io.Serializable;
@NamedQuery(name= "findUserByBrokerLinkAndRealm", query="select link.userId from BrokerLinkEntity link where link.realmId = :realmId and link.identityProvider = :identityProvider and link.brokerUserId = :brokerUserId"),
@NamedQuery(name= "deleteBrokerLinkByStorageProvider", query="delete from BrokerLinkEntity social where social.storageProviderId = :storageProviderId"),
@NamedQuery(name= "deleteBrokerLinkByRealm", query="delete from BrokerLinkEntity social where social.realmId = :realmId"),
@NamedQuery(name= "deleteBrokerLinkByIdentityProvider", query="delete from BrokerLinkEntity b where b.realmId = :realmId and b.identityProvider = :providerAlias"),
@NamedQuery(name= "deleteBrokerLinkByRealmAndLink", query="delete from BrokerLinkEntity social where social.userId IN (select u.id from UserEntity u where realmId=:realmId and u.federationLink=:link)"),
@NamedQuery(name= "deleteBrokerLinkByUser", query="delete from BrokerLinkEntity social where social.userId = :userId and social.realmId = :realmId")
})

View file

@ -40,6 +40,7 @@ public interface UserProvider extends Provider,
void addFederatedIdentity(RealmModel realm, UserModel user, FederatedIdentityModel socialLink);
boolean removeFederatedIdentity(RealmModel realm, UserModel user, String socialProvider);
void preRemove(RealmModel realm, IdentityProviderModel provider);
void updateFederatedIdentity(RealmModel realm, UserModel federatedUser, FederatedIdentityModel federatedIdentityModel);
Set<FederatedIdentityModel> getFederatedIdentities(UserModel user, RealmModel realm);
FederatedIdentityModel getFederatedIdentity(UserModel user, String socialProvider, RealmModel realm);
@ -97,5 +98,4 @@ public interface UserProvider extends Provider,
void close();
void preRemove(RealmModel realm, ComponentModel component);
}

View file

@ -17,6 +17,7 @@
package org.keycloak.storage.federated;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.RealmModel;
import java.util.Set;
@ -29,6 +30,7 @@ public interface UserBrokerLinkFederatedStorage {
String getUserByFederatedIdentity(FederatedIdentityModel socialLink, RealmModel realm);
void addFederatedIdentity(RealmModel realm, String userId, FederatedIdentityModel socialLink);
boolean removeFederatedIdentity(RealmModel realm, String userId, String socialProvider);
void preRemove(RealmModel realm, IdentityProviderModel provider);
void updateFederatedIdentity(RealmModel realm, String userId, FederatedIdentityModel federatedIdentityModel);
Set<FederatedIdentityModel> getFederatedIdentities(String userId, RealmModel realm);
FederatedIdentityModel getFederatedIdentity(String userId, String socialProvider, RealmModel realm);

View file

@ -130,6 +130,7 @@ public class IdentityProviderResource {
}
String alias = this.identityProviderModel.getAlias();
session.users().preRemove(realm, identityProviderModel);
this.realm.removeIdentityProviderByAlias(alias);
Set<IdentityProviderMapperModel> mappers = this.realm.getIdentityProviderMappersByAlias(alias);

View file

@ -25,6 +25,7 @@ import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.GroupModel;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.KeycloakSessionTask;
import org.keycloak.models.ModelException;
@ -224,6 +225,13 @@ public class UserStorageManager implements UserProvider, OnUserCache, OnCreateCo
return getFederatedStorage().removeFederatedIdentity(realm, user.getId(), socialProvider);
}
}
@Override
public void preRemove(RealmModel realm, IdentityProviderModel provider) {
localStorage().preRemove(realm, provider);
getFederatedStorage().preRemove(realm, provider);
}
@Override
public void addConsent(RealmModel realm, String userId, UserConsentModel consent) {

View file

@ -20,8 +20,14 @@ import org.jboss.arquillian.graphene.page.Page;
import org.junit.Before;
import org.junit.Test;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.admin.client.resource.UsersResource;
import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.models.FederatedIdentityModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.representations.idm.ComponentRepresentation;
import org.keycloak.representations.idm.FederatedIdentityRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.storage.UserStorageProvider;
@ -33,7 +39,10 @@ import org.keycloak.testsuite.pages.LoginPage;
import org.keycloak.testsuite.pages.UpdateAccountInformationPage;
import java.util.List;
import java.util.Set;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.keycloak.testsuite.admin.ApiUtil.createUserAndResetPasswordWithAdminClient;
import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude;
@ -120,7 +129,6 @@ public class AccountLinkTest extends AbstractKeycloakTest {
String childIdp = CHILD_IDP;
testAccountLink(childUsername, childPassword, childIdp);
}
@Test
@ -133,7 +141,46 @@ public class AccountLinkTest extends AbstractKeycloakTest {
}
@Test
public void testDeleteIdentityOnProviderRemoval() {
String childUsername = "child";
String childPassword = "password";
String childIdp = CHILD_IDP;
assertFederatedIdentity(childUsername, childPassword, childIdp);
RealmResource realm = adminClient.realm(CHILD_IDP);
UsersResource users = realm.users();
List<UserRepresentation> search = users.search(childUsername);
assertFalse(search.isEmpty());
String userId = search.get(0).getId();
List<FederatedIdentityRepresentation> identities = users.get(userId).getFederatedIdentity();
assertFalse(identities.isEmpty());
realm.identityProviders().get(PARENT_IDP).remove();
identities = users.get(userId).getFederatedIdentity();
assertTrue(identities.isEmpty());
getTestingClient().server(CHILD_IDP).run(AccountLinkTest::checkEmptyFederatedIdentities);
}
private static void checkEmptyFederatedIdentities(KeycloakSession session) {
RealmModel realm = session.getContext().getRealm();
UserModel user = session.users().getUserByUsername("child", realm);
Set<FederatedIdentityModel> identities1 = session.users()
.getFederatedIdentities(user, realm);
assertTrue(identities1.isEmpty());
assertNull(session.users().getFederatedIdentity(user, PARENT_IDP, realm));
}
protected void testAccountLink(String childUsername, String childPassword, String childIdp) {
assertFederatedIdentity(childUsername, childPassword, childIdp);
assertRemoveFederatedIdentity();
}
private void assertFederatedIdentity(String childUsername, String childPassword, String childIdp) {
accountFederatedIdentityPage.realm(childIdp);
accountFederatedIdentityPage.open();
loginPage.isCurrent();
@ -161,12 +208,13 @@ public class AccountLinkTest extends AbstractKeycloakTest {
System.out.println(driver.getPageSource());
assertTrue(accountFederatedIdentityPage.isCurrent());
assertTrue(driver.getPageSource().contains("id=\"remove-link-" + PARENT_IDP + "\""));
}
private void assertRemoveFederatedIdentity() {
// Unlink my "test-user"
accountFederatedIdentityPage.clickRemoveProvider(PARENT_IDP);
assertTrue(driver.getPageSource().contains("id=\"add-link-" + PARENT_IDP + "\""));
// Logout from account management
accountFederatedIdentityPage.logout();