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 32f34a5727..1a1c1e9253 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 @@ -1845,6 +1845,7 @@ public class RealmAdapter implements RealmModel, JpaModel { ComponentEntity c = em.find(ComponentEntity.class, component.getId()); if (c == null) return; + ComponentModel old = entityToModel(c); c.setName(component.getName()); c.setProviderId(component.getProviderId()); c.setProviderType(component.getProviderType()); @@ -1853,7 +1854,7 @@ public class RealmAdapter implements RealmModel, JpaModel { em.createNamedQuery("deleteComponentConfigByComponent").setParameter("component", c).executeUpdate(); em.flush(); setConfig(component, c); - ComponentUtil.notifyUpdated(session, this, component); + ComponentUtil.notifyUpdated(session, this, old, component); } 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 62d3be8fb3..3854989a87 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 @@ -1748,14 +1748,17 @@ public class RealmAdapter extends AbstractMongoAdapter impleme public void updateComponent(ComponentModel model) { ComponentUtil.getComponentFactory(session, model).validateConfiguration(session, this, model); + ComponentModel old = null; for (ComponentEntity entity : realm.getComponentEntities()) { if (entity.getId().equals(model.getId())) { + old = entityToModel(entity); updateComponentEntity(entity, model); - + break; } } + if (old == null) return; // wasn't updated updateRealm(); - ComponentUtil.notifyUpdated(session, this, model); + ComponentUtil.notifyUpdated(session, this, old, model); } diff --git a/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java b/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java index 0d103124dc..6bd0a37639 100644 --- a/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java +++ b/server-spi-private/src/main/java/org/keycloak/models/utils/ComponentUtil.java @@ -26,6 +26,7 @@ import org.keycloak.provider.ProviderConfigProperty; import org.keycloak.provider.ProviderFactory; import org.keycloak.representations.idm.ComponentRepresentation; import org.keycloak.storage.OnCreateComponent; +import org.keycloak.storage.OnUpdateComponent; import org.keycloak.storage.UserStorageProviderFactory; import java.util.HashMap; @@ -94,9 +95,12 @@ public class ComponentUtil { ((OnCreateComponent)session.userStorageManager()).onCreate(session, realm, model); } } - public static void notifyUpdated(KeycloakSession session, RealmModel realm, ComponentModel model) { - ComponentFactory factory = getComponentFactory(session, model); - factory.onUpdate(session, realm, model); + public static void notifyUpdated(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel) { + ComponentFactory factory = getComponentFactory(session, newModel); + factory.onUpdate(session, realm, newModel); + if (factory instanceof UserStorageProviderFactory) { + ((OnUpdateComponent)session.userStorageManager()).onUpdate(session, realm, oldModel, newModel); + } } } diff --git a/server-spi-private/src/main/java/org/keycloak/storage/OnUpdateComponent.java b/server-spi-private/src/main/java/org/keycloak/storage/OnUpdateComponent.java new file mode 100644 index 0000000000..057a45b388 --- /dev/null +++ b/server-spi-private/src/main/java/org/keycloak/storage/OnUpdateComponent.java @@ -0,0 +1,32 @@ +/* + * Copyright 2016 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.storage; + +import org.keycloak.component.ComponentModel; +import org.keycloak.models.KeycloakSession; +import org.keycloak.models.RealmModel; + +/** + * Callback for component update. Only hardcoded classes like UserStorageManager implement it. In future we + * may allow anybody to implement this interface. + * + * @author Bill Burke + * @version $Revision: 1 $ + */ +public interface OnUpdateComponent { + void onUpdate(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel); +} diff --git a/services/src/main/java/org/keycloak/storage/UserStorageManager.java b/services/src/main/java/org/keycloak/storage/UserStorageManager.java index 1134b86071..92e17bbeeb 100755 --- a/services/src/main/java/org/keycloak/storage/UserStorageManager.java +++ b/services/src/main/java/org/keycloak/storage/UserStorageManager.java @@ -60,7 +60,7 @@ import static org.keycloak.models.utils.KeycloakModelUtils.runJobInTransaction; * @author Bill Burke * @version $Revision: 1 $ */ -public class UserStorageManager implements UserProvider, OnUserCache, OnCreateComponent { +public class UserStorageManager implements UserProvider, OnUserCache, OnCreateComponent, OnUpdateComponent { private static final Logger logger = Logger.getLogger(UserStorageManager.class); @@ -640,6 +640,16 @@ public class UserStorageManager implements UserProvider, OnUserCache, OnCreateCo } + @Override + public void onUpdate(KeycloakSession session, RealmModel realm, ComponentModel oldModel, ComponentModel newModel) { + ComponentFactory factory = ComponentUtil.getComponentFactory(session, newModel); + if (!(factory instanceof UserStorageProviderFactory)) return; + UserStorageProviderModel old = new UserStorageProviderModel(oldModel); + UserStorageProviderModel newP= new UserStorageProviderModel(newModel); + if (old.getChangedSyncPeriod() != newP.getChangedSyncPeriod() || old.getFullSyncPeriod() != newP.getFullSyncPeriod() + || old.isImportEnabled() != newP.isImportEnabled()) { + new UserStorageSyncManager().notifyToRefreshPeriodicSync(session, realm, new UserStorageProviderModel(newModel), false); + } - + } } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java index 4967bcc6c1..0394ced6fd 100644 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/sync/SyncFederationTest.java @@ -61,9 +61,19 @@ public class SyncFederationTest { }); + /** + * Test that period sync is triggered when creating a synchronized User Storage Provider + * + */ @Test - public void test01PeriodicSync() { + public void test01PeriodicSyncOnCreate() { + KeycloakSession session = keycloakRule.startSession(); + KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory(); + DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory) sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME); + int full = dummyFedFactory.getFullSyncCounter(); + int changed = dummyFedFactory.getChangedSyncCounter(); + keycloakRule.stopSession(session, false); // Enable timer for SyncDummyUserFederationProvider keycloakRule.update(new KeycloakRule.KeycloakSetup() { @@ -81,16 +91,11 @@ public class SyncFederationTest { }); - KeycloakSession session = keycloakRule.startSession(); + session = keycloakRule.startSession(); try { - KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory(); - DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory)sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME); - int full = dummyFedFactory.getFullSyncCounter(); - int changed = dummyFedFactory.getChangedSyncCounter(); // Assert that after some period was DummyUserFederationProvider triggered UserStorageSyncManager usersSyncManager = new UserStorageSyncManager(); - usersSyncManager.bootstrapPeriodic(sessionFactory, session.getProvider(TimerProvider.class)); sleep(1800); // Cancel timer @@ -117,8 +122,8 @@ public class SyncFederationTest { // Assert that dummy provider won't be invoked anymore sleep(1800); Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter()); - int newestChanged = dummyFedFactory.getChangedSyncCounter(); - Assert.assertEquals("Assertion failed. newChanged=" + newChanged + ", newestChanged=" + newestChanged, newChanged, newestChanged); + int newestChanged = dummyFedFactory.getChangedSyncCounter(); + Assert.assertEquals("Assertion failed. newChanged=" + newChanged + ", newestChanged=" + newestChanged, newChanged, newestChanged); } finally { keycloakRule.stopSession(session, true); } @@ -134,8 +139,114 @@ public class SyncFederationTest { }); } + /** + * Test that period sync is triggered when updating a synchronized User Storage Provider to have a non-negative sync period + * + */ @Test - public void test02ConcurrentSync() throws Exception { + public void test02PeriodicSyncOnUpdate() { + + KeycloakSession session = keycloakRule.startSession(); + KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory(); + DummyUserFederationProviderFactory dummyFedFactory = (DummyUserFederationProviderFactory) sessionFactory.getProviderFactory(UserStorageProvider.class, DummyUserFederationProviderFactory.PROVIDER_NAME); + int full = dummyFedFactory.getFullSyncCounter(); + int changed = dummyFedFactory.getChangedSyncCounter(); + keycloakRule.stopSession(session, false); + // Enable timer for SyncDummyUserFederationProvider + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + UserStorageProviderModel model = new UserStorageProviderModel(); + model.setProviderId(DummyUserFederationProviderFactory.PROVIDER_NAME); + model.setPriority(1); + model.setName("test-sync-dummy"); + model.setFullSyncPeriod(-1); + model.setChangedSyncPeriod(-1); + model.setLastSync(0); + dummyModel = new UserStorageProviderModel(appRealm.addComponentModel(model)); + } + + }); + + session = keycloakRule.startSession(); + try { + + // Assert that after some period was DummyUserFederationProvider triggered + UserStorageSyncManager usersSyncManager = new UserStorageSyncManager(); + // Assert that dummy provider won't be invoked anymore + sleep(1800); + Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter()); + int newestChanged = dummyFedFactory.getChangedSyncCounter(); + Assert.assertEquals("Assertion failed. newChanged=" + changed + ", newestChanged=" + newestChanged, changed, newestChanged); + } finally { + keycloakRule.stopSession(session, true); + } + + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + dummyModel.setChangedSyncPeriod(1); + appRealm.updateComponent(dummyModel); + } + + }); + + + session = keycloakRule.startSession(); + try { + + // Assert that after some period was DummyUserFederationProvider triggered + UserStorageSyncManager usersSyncManager = new UserStorageSyncManager(); + sleep(1800); + + // Cancel timer + RealmModel appRealm = session.realms().getRealmByName("test"); + usersSyncManager.notifyToRefreshPeriodicSync(session, appRealm, dummyModel, true); + log.infof("Notified sync manager about cancel periodic sync"); + + // This sync is here just to ensure that we have lock (doublecheck that periodic sync, which was possibly triggered before canceling timer is finished too) + while (true) { + SynchronizationResult result = usersSyncManager.syncChangedUsers(session.getKeycloakSessionFactory(), appRealm.getId(), dummyModel); + if (result.isIgnored()) { + log.infof("Still waiting for lock before periodic sync is finished", result.toString()); + sleep(1000); + } else { + break; + } + } + + // Assert that DummyUserFederationProviderFactory.syncChangedUsers was invoked at least 2 times (once periodically and once for us) + int newChanged = dummyFedFactory.getChangedSyncCounter(); + Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter()); + log.info("Asserting. newChanged=" + newChanged + " > changed=" + changed); + Assert.assertTrue("Assertion failed. newChanged=" + newChanged + ", changed=" + changed, newChanged > (changed + 1)); + + // Assert that dummy provider won't be invoked anymore + sleep(1800); + Assert.assertEquals(full, dummyFedFactory.getFullSyncCounter()); + int newestChanged = dummyFedFactory.getChangedSyncCounter(); + Assert.assertEquals("Assertion failed. newChanged=" + newChanged + ", newestChanged=" + newestChanged, newChanged, newestChanged); + } finally { + keycloakRule.stopSession(session, true); + } + + + // remove dummyProvider + keycloakRule.update(new KeycloakRule.KeycloakSetup() { + + @Override + public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { + appRealm.removeComponent(dummyModel); + } + + }); + } + + + @Test + public void test03ConcurrentSync() throws Exception { SyncDummyUserFederationProviderFactory.restartLatches(); // Enable timer for SyncDummyUserFederationProvider