Details
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 7db1b47..8cd0678 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<UserFederationMapperEntity> 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<UserFederationMapperEntity> 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 aa25be7..2b8c397 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<MongoRealmEntity> 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<UserFederationMapperEntity> 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<MongoRealmEntity> impleme
updateRealm();
}
+ private void removeFederationMappersForProvider(String federationProviderId) {
+ Set<UserFederationMapperEntity> 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<MongoRealmEntity> impleme
KeycloakModelUtils.ensureUniqueDisplayName(currentProvider.getDisplayName(), currentProvider, providers);
}
- List<UserFederationProviderEntity> entities = new LinkedList<UserFederationProviderEntity>();
+ List<UserFederationProviderEntity> existingProviders = realm.getUserFederationProviders();
+ List<UserFederationProviderEntity> 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<UserFederationProviderModel> add = new LinkedList<UserFederationProviderModel>();
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<MongoRealmEntity> 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 b27ce49..c94fa6e 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 51fc446..17e0e31 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<String, String>(), 1, "my-cool-provider", -1, -1, 0);
+ realm.setUserFederationProviders(Arrays.asList(ldapProvider));
+
+ commit();
+
+ realm = realmManager.getRealmByName("test-realm");
+ List<UserFederationProviderModel> fedProviders = realm.getUserFederationProviders();
+ Assert.assertEquals(1, fedProviders.size());
+ ldapProvider = fedProviders.get(0);
+ Set<UserFederationMapperModel> fedMappers = realmManager.getRealmByName("test-realm").getUserFederationMappersByFederationProvider(ldapProvider.getId());
+
+ UserFederationProviderModel dummyProvider = new UserFederationProviderModel(null, "dummy", new TreeMap<String, String>(), 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<UserFederationProviderModel>());
+
+ 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);
