diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index 7db1b47010..8cd0678960 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -804,18 +804,23 @@ public class RealmAdapter implements RealmModel { if (entity.getId().equals(provider.getId())) { session.users().preRemove(this, provider); + removeFederationMappersForProvider(provider.getId()); - Set mappers = getUserFederationMapperEntitiesByFederationProvider(provider.getId()); - for (UserFederationMapperEntity mapper : mappers) { - realm.getUserFederationMappers().remove(mapper); - em.remove(mapper); - } it.remove(); em.remove(entity); return; } } } + + private void removeFederationMappersForProvider(String federationProviderId) { + Set mappers = getUserFederationMapperEntitiesByFederationProvider(federationProviderId); + for (UserFederationMapperEntity mapper : mappers) { + realm.getUserFederationMappers().remove(mapper); + em.remove(mapper); + } + } + @Override public void updateUserFederationProvider(UserFederationProviderModel model) { KeycloakModelUtils.ensureUniqueDisplayName(model.getDisplayName(), model, getUserFederationProviders()); @@ -855,10 +860,9 @@ public class RealmAdapter implements RealmModel { entity.setConfig(model.getConfig()); entity.setPriority(model.getPriority()); entity.setProviderName(model.getProviderName()); - entity.setPriority(model.getPriority()); String displayName = model.getDisplayName(); if (displayName != null) { - entity.setDisplayName(model.getDisplayName()); + entity.setDisplayName(displayName); } entity.setFullSyncPeriod(model.getFullSyncPeriod()); entity.setChangedSyncPeriod(model.getChangedSyncPeriod()); @@ -871,6 +875,8 @@ public class RealmAdapter implements RealmModel { if (found) continue; session.users().preRemove(this, new UserFederationProviderModel(entity.getId(), entity.getProviderName(), entity.getConfig(), entity.getPriority(), entity.getDisplayName(), entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); + removeFederationMappersForProvider(entity.getId()); + it.remove(); em.remove(entity); } diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index aa25be77b5..2b8c397f04 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -877,11 +877,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme if (entity.getId().equals(provider.getId())) { session.users().preRemove(this, new UserFederationProviderModel(entity.getId(), entity.getProviderName(), entity.getConfig(), entity.getPriority(), entity.getDisplayName(), entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); - - Set mappers = getUserFederationMapperEntitiesByFederationProvider(provider.getId()); - for (UserFederationMapperEntity mapper : mappers) { - getMongoEntity().getUserFederationMappers().remove(mapper); - } + removeFederationMappersForProvider(provider.getId()); it.remove(); } @@ -889,6 +885,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme updateRealm(); } + private void removeFederationMappersForProvider(String federationProviderId) { + Set mappers = getUserFederationMapperEntitiesByFederationProvider(federationProviderId); + for (UserFederationMapperEntity mapper : mappers) { + getMongoEntity().getUserFederationMappers().remove(mapper); + } + } + @Override public void updateUserFederationProvider(UserFederationProviderModel model) { KeycloakModelUtils.ensureUniqueDisplayName(model.getDisplayName(), model, getUserFederationProviders()); @@ -943,8 +946,52 @@ public class RealmAdapter extends AbstractMongoAdapter impleme KeycloakModelUtils.ensureUniqueDisplayName(currentProvider.getDisplayName(), currentProvider, providers); } - List entities = new LinkedList(); + List existingProviders = realm.getUserFederationProviders(); + List toRemove = new LinkedList<>(); + for (UserFederationProviderEntity entity : existingProviders) { + boolean found = false; + for (UserFederationProviderModel model : providers) { + if (entity.getId().equals(model.getId())) { + entity.setConfig(model.getConfig()); + entity.setPriority(model.getPriority()); + entity.setProviderName(model.getProviderName()); + String displayName = model.getDisplayName(); + if (displayName != null) { + entity.setDisplayName(displayName); + } + entity.setFullSyncPeriod(model.getFullSyncPeriod()); + entity.setChangedSyncPeriod(model.getChangedSyncPeriod()); + entity.setLastSync(model.getLastSync()); + found = true; + break; + } + + } + if (found) continue; + session.users().preRemove(this, new UserFederationProviderModel(entity.getId(), entity.getProviderName(), entity.getConfig(), entity.getPriority(), entity.getDisplayName(), + entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); + removeFederationMappersForProvider(entity.getId()); + + toRemove.add(entity); + } + + for (UserFederationProviderEntity entity : toRemove) { + realm.getUserFederationProviders().remove(entity); + } + + List add = new LinkedList(); for (UserFederationProviderModel model : providers) { + boolean found = false; + for (UserFederationProviderEntity entity : realm.getUserFederationProviders()) { + if (entity.getId().equals(model.getId())) { + found = true; + break; + } + } + if (!found) add.add(model); + } + + for (UserFederationProviderModel model : add) { UserFederationProviderEntity entity = new UserFederationProviderEntity(); if (model.getId() != null) { entity.setId(model.getId()); @@ -964,12 +1011,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme entity.setFullSyncPeriod(model.getFullSyncPeriod()); entity.setChangedSyncPeriod(model.getChangedSyncPeriod()); entity.setLastSync(model.getLastSync()); - entities.add(entity); + realm.getUserFederationProviders().add(entity); session.getKeycloakSessionFactory().publish(new UserFederationProviderCreationEventImpl(this, model)); } - realm.setUserFederationProviders(entities); updateRealm(); } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java index b27ce49430..c94fa6ec7c 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java @@ -193,8 +193,9 @@ public class RealmAdminResource { } catch (PatternSyntaxException e) { return ErrorResponse.error("Specified regex pattern(s) is invalid.", Response.Status.BAD_REQUEST); } catch (ModelDuplicateException e) { - return ErrorResponse.exists("Realm " + rep.getRealm() + " already exists."); - } catch (Exception e) { + throw e; + } catch (Exception e) { + logger.error(e); return ErrorResponse.error("Failed to update " + rep.getRealm() + " Realm.", Response.Status.INTERNAL_SERVER_ERROR); } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserFederationModelTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserFederationModelTest.java index 51fc446caf..17e0e31c3e 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserFederationModelTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/UserFederationModelTest.java @@ -1,5 +1,9 @@ package org.keycloak.testsuite.model; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; @@ -97,6 +101,43 @@ public class UserFederationModelTest extends AbstractModelTest { commit(); } + @Test + public void federationProvidersSetTest() { + RealmModel realm = realmManager.createRealm("test-realm"); + UserFederationProviderModel ldapProvider = new UserFederationProviderModel(null, "ldap", new TreeMap(), 1, "my-cool-provider", -1, -1, 0); + realm.setUserFederationProviders(Arrays.asList(ldapProvider)); + + commit(); + + realm = realmManager.getRealmByName("test-realm"); + List fedProviders = realm.getUserFederationProviders(); + Assert.assertEquals(1, fedProviders.size()); + ldapProvider = fedProviders.get(0); + Set fedMappers = realmManager.getRealmByName("test-realm").getUserFederationMappersByFederationProvider(ldapProvider.getId()); + + UserFederationProviderModel dummyProvider = new UserFederationProviderModel(null, "dummy", new TreeMap(), 1, "my-cool-provider", -1, -1, 0); + try { + realm.setUserFederationProviders(Arrays.asList(ldapProvider, dummyProvider)); + commit(); + Assert.fail("Don't expect to end here"); + } catch (ModelDuplicateException expected) { + } + + dummyProvider.setDisplayName("my-cool-provider2"); + realm.setUserFederationProviders(Arrays.asList(ldapProvider, dummyProvider)); + + commit(); + + realm = realmManager.getRealmByName("test-realm"); + Assert.assertEquals(fedMappers.size(), realm.getUserFederationMappersByFederationProvider(ldapProvider.getId()).size()); + realm.setUserFederationProviders(new ArrayList()); + + commit(); + + realm = realmManager.getRealmByName("test-realm"); + Assert.assertTrue(realm.getUserFederationMappersByFederationProvider(ldapProvider.getId()).isEmpty()); + } + private UserFederationMapperModel createMapper(String name, String fedProviderId, String... config) { UserFederationMapperModel mapperModel = new UserFederationMapperModel(); mapperModel.setName(name);