Attempt to run snapshot Keycloak server against production DB should fail during migration

closes #30364

Signed-off-by: mposolda <mposolda@gmail.com>
Signed-off-by: Alexander Schwartz <aschwart@redhat.com>
Co-authored-by: Alexander Schwartz <aschwart@redhat.com>

model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultDatastoreProvider.java

@@ -274,7 +274,7 @@ public class DefaultDatastoreProvider implements DatastoreProvider, StoreManager
     }
 
     public MigrationManager getMigrationManager() {
-        return new DefaultMigrationManager(session);
+        return new DefaultMigrationManager(session, factory.isAllowMigrateExistingDatabaseToSnapshot());
     }
 
 }

model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultDatastoreProviderFactory.java

@@ -27,6 +27,8 @@ import org.keycloak.migration.MigrationModelManager;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.utils.PostMigrationEvent;
+import org.keycloak.provider.ProviderConfigProperty;
+import org.keycloak.provider.ProviderConfigurationBuilder;
 import org.keycloak.provider.ProviderEvent;
 import org.keycloak.provider.ProviderEventListener;
 import org.keycloak.services.scheduled.ClearExpiredAdminEvents;
@@ -47,10 +49,13 @@ public class DefaultDatastoreProviderFactory implements DatastoreProviderFactory
 
     private static final String PROVIDER_ID = "legacy";
 
+    public static final String ALLOW_MIGRATE_EXISTING_DB_TO_SNAPSHOT_OPTION = "allowMigrateExistingDatabaseToSnapshot";
+
     private static final Logger logger = Logger.getLogger(DefaultDatastoreProviderFactory.class);
 
     private long clientStorageProviderTimeout;
     private long roleStorageProviderTimeout;
+    private boolean allowMigrateExistingDatabaseToSnapshot;
     private Runnable onClose;
 
     @Override
@@ -62,6 +67,7 @@ public class DefaultDatastoreProviderFactory implements DatastoreProviderFactory
     public void init(Scope config) {
         clientStorageProviderTimeout = Config.scope("client").getLong("storageProviderTimeout", 3000L);
         roleStorageProviderTimeout = Config.scope("role").getLong("storageProviderTimeout", 3000L);
+        allowMigrateExistingDatabaseToSnapshot = config.getBoolean(ALLOW_MIGRATE_EXISTING_DB_TO_SNAPSHOT_OPTION, false);
     }
 
     @Override
@@ -82,6 +88,20 @@ public class DefaultDatastoreProviderFactory implements DatastoreProviderFactory
         return PROVIDER_ID;
     }
 
+    @Override
+    public List<ProviderConfigProperty> getConfigMetadata() {
+        return ProviderConfigurationBuilder.create()
+                .property()
+                .name(ALLOW_MIGRATE_EXISTING_DB_TO_SNAPSHOT_OPTION)
+                .type("boolean")
+                .helpText("By default, it is not allowed to run the snapshot/development server against the database, which was previously migrated to some officially released server version. As an attempt of doing this " +
+                        "indicates that you are trying to run development server against production database, which can result in a loss or corruption of data, and also does not allow upgrading. If it is really intended, you can use this option, which will allow to use " +
+                        "nightly/development server against production database when explicitly switch to true. This option is recommended just in the development environments and should be never used in the production!")
+                .defaultValue(false)
+                .add()
+                .build();
+    }
+
     public long getClientStorageProviderTimeout() {
         return clientStorageProviderTimeout;
     }
@@ -90,6 +110,10 @@ public class DefaultDatastoreProviderFactory implements DatastoreProviderFactory
         return roleStorageProviderTimeout;
     }
 
+    boolean isAllowMigrateExistingDatabaseToSnapshot() {
+        return allowMigrateExistingDatabaseToSnapshot;
+    }
+
     @Override
     public void onEvent(ProviderEvent event) {
         if (event instanceof PostMigrationEvent) {

model/storage-private/src/main/java/org/keycloak/storage/datastore/DefaultMigrationManager.java

@@ -124,9 +124,11 @@ public class DefaultMigrationManager implements MigrationManager {
     };
 
     private final KeycloakSession session;
+    private final boolean allowMigrateExistingDatabaseToSnapshot;
 
-    public DefaultMigrationManager(KeycloakSession session) {
+    public DefaultMigrationManager(KeycloakSession session, boolean allowMigrateExistingDatabaseToSnapshot) {
         this.session = session;
+        this.allowMigrateExistingDatabaseToSnapshot = allowMigrateExistingDatabaseToSnapshot;
     }
 
     @Override
@@ -139,6 +141,11 @@ public class DefaultMigrationManager implements MigrationManager {
         ModelVersion latestUpdate = migrations[migrations.length-1].getVersion();
         ModelVersion databaseVersion = model.getStoredVersion() != null ? new ModelVersion(model.getStoredVersion()) : null;
 
+        if (SNAPSHOT_VERSION.equals(currentVersion) && databaseVersion != null && databaseVersion.lessThan(SNAPSHOT_VERSION) && !allowMigrateExistingDatabaseToSnapshot) {
+            throw new ModelException("Incorrect state of migration. You are trying to run nightly server version '" + currentVersion + "' against a database, which was previously migrated to version '" + databaseVersion +
+                    "'. This indicates that you are trying to run development server version against production database, which can result in a loss or corruption of data, and also does not allow upgrading. If it is intended, " +
+                    "use the option spi-datastore-legacy-allow-migrate-existing-database-to-snapshot of the datastore provider when starting the server and explicitly set it to true.");
+        }
         if (databaseVersion == null || databaseVersion.lessThan(latestUpdate)) {
             for (Migration m : migrations) {
                 if (databaseVersion == null || databaseVersion.lessThan(m.getVersion())) {
@@ -170,7 +177,7 @@ public class DefaultMigrationManager implements MigrationManager {
     public static final ModelVersion RHSSO_VERSION_7_2_KEYCLOAK_VERSION = new ModelVersion("3.4.3");
     public static final ModelVersion RHSSO_VERSION_7_3_KEYCLOAK_VERSION = new ModelVersion("4.8.3");
     public static final ModelVersion RHSSO_VERSION_7_4_KEYCLOAK_VERSION = new ModelVersion("9.0.3");
-    public static final ModelVersion SNAPSHOT_VERSION = new ModelVersion("999.0.0-SNAPSHOT");
+    public static final ModelVersion SNAPSHOT_VERSION = new ModelVersion(Constants.SNAPSHOT_VERSION);
 
     private static final Map<Pattern, ModelVersion> PATTERN_MATCHER = new LinkedHashMap<>();
     static {

server-spi-private/src/main/java/org/keycloak/models/Constants.java

@@ -140,6 +140,8 @@ public final class Constants {
      */
     public static final String STORAGE_BATCH_SIZE = "org.keycloak.storage.batch_size";
 
+    public static final String SNAPSHOT_VERSION = "999.0.0-SNAPSHOT";
+
     // Client Polices Realm Attributes Keys
     public static final String CLIENT_PROFILES = "client-policies.profiles";
     public static final String CLIENT_POLICIES = "client-policies.policies";

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/AuthServerTestEnricher.java

@@ -235,12 +235,12 @@ public class AuthServerTestEnricher {
                     .filter(c -> c.getQualifier().startsWith("cache-server-"))
                     .sorted((a, b) -> a.getQualifier().compareTo(b.getQualifier()))
                     .forEach(containerInfo -> {
-                        
+
                         log.info(String.format("cache container: %s", containerInfo.getQualifier()));
-                        
+
                         int prefixSize = containerInfo.getQualifier().lastIndexOf("-") + 1;
                         int dcIndex = Integer.parseInt(containerInfo.getQualifier().substring(prefixSize)) - 1;
-                        
+
                         suiteContext.addCacheServerInfo(dcIndex, containerInfo);
                     });
 
@@ -294,6 +294,8 @@ public class AuthServerTestEnricher {
         }
 
         if (START_MIGRATION_CONTAINER) {
+            suiteContext.getMigrationContext().setRunningMigrationTest(true);
+
             // init migratedAuthServerInfo
             for (ContainerInfo container : suiteContext.getContainers()) {
                 // migrated auth server
@@ -375,7 +377,7 @@ public class AuthServerTestEnricher {
 
     public void startAuthContainer(@Observes(precedence = 0) StartSuiteContainers event) {
         // this property can be used to skip start of auth-server before suite
-        // it might be useful for running some specific tests locally, e.g. when running standalone ZeroDowtime*Test 
+        // it might be useful for running some specific tests locally, e.g. when running standalone ZeroDowtime*Test
         if (Boolean.getBoolean("keycloak.testsuite.skip.start.auth.server")) {
             log.debug("Skipping the start of auth server before suite");
             return;

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/SuiteContext.java

@@ -182,6 +182,10 @@ public final class SuiteContext {
         return authServerBackendsInfo.size() > 1;
     }
 
+    /**
+     * @return true during migration test, but just right before and during old container running. It returns false during lifecycle of new container running
+     * and during migration test itself. Use {@link #getMigrationContext().isRunningMigrationTest()} if want to check if we are in the context of migration (including lifecycle of the new container)
+     */
     public boolean isAuthServerMigrationEnabled() {
         return migratedAuthServerInfo != null;
     }

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/containers/AbstractQuarkusDeployableContainer.java

@@ -179,6 +179,10 @@ public abstract class AbstractQuarkusDeployableContainer implements DeployableCo
             commands.add("--http-management-port=" + configuration.getManagementPort());
         }
 
+        if (suiteContext.get().getMigrationContext().isRunningMigrationTest()) {
+            commands.add("--spi-datastore-legacy-allow-migrate-existing-database-to-snapshot=true");
+        }
+
         if (configuration.getRoute() != null) {
             commands.add("-Djboss.node.name=" + configuration.getRoute());
         }

testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/arquillian/migration/MigrationContext.java

@@ -36,6 +36,18 @@ public class MigrationContext {
 
     public static final Logger logger = Logger.getLogger(MigrationContext.class);
 
+    private boolean runningMigrationTest = false;
+
+    /**
+     * @return true if testsuite is running migration test. This returns true during whole lifecycle of migration test (Including running of the old container as well as running the new container)
+     */
+    public boolean isRunningMigrationTest() {
+        return runningMigrationTest;
+    }
+
+    public void setRunningMigrationTest(boolean runningMigrationTest) {
+        this.runningMigrationTest = runningMigrationTest;
+    }
 
     public String loadOfflineToken() throws Exception {
         String file = getOfflineTokenLocation();
@@ -93,4 +105,4 @@ public class MigrationContext {
         return tmpDir + "/offline-token.txt";
     }
 
-}
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationDeniedTest.java

@@ -22,10 +22,10 @@ package org.keycloak.testsuite.migration;
 import java.util.List;
 
 import org.junit.Assert;
-import org.junit.Assume;
 import org.junit.Test;
 import org.keycloak.common.Version;
 import org.keycloak.migration.MigrationModel;
+import org.keycloak.models.Constants;
 import org.keycloak.models.DeploymentStateProvider;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ModelException;
@@ -49,23 +49,52 @@ public class MigrationDeniedTest extends AbstractKeycloakTest {
      */
     @Test
     @ModelTest
-    public void testMigrationDenied(KeycloakSession session) {
+    public void testMigrationDeniedWithDBSnapshotAndServerNonSnapshot(KeycloakSession session) {
         MigrationModel model = session.getProvider(DeploymentStateProvider.class).getMigrationModel();
-        String databaseVersion = model.getStoredVersion() != null ? model.getStoredVersion() : null;
+        String databaseVersion = model.getStoredVersion();
-
+        Assert.assertNotNull("Stored DB version was null", model.getStoredVersion());
-        Assume.assumeTrue("Test ignored as it is working just with DB migrated in version '" + databaseVersion + "', but current DB version is " + databaseVersion,
-                DefaultMigrationManager.SNAPSHOT_VERSION.toString().equals(databaseVersion));
 
         String currentVersion = Version.VERSION;
         try {
             // Simulate to manually set runtime version of KeycloakServer to 23. Migration should fail as the version is lower than DB version.
             Version.VERSION = "23.0.0";
-            new DefaultMigrationManager(session).migrate();
+            model.setStoredVersion(Constants.SNAPSHOT_VERSION);
+
+            new DefaultMigrationManager(session, false).migrate();
             Assert.fail("Not expected to successfully run migration. DB version was " + databaseVersion + ". Keycloak version was " + currentVersion);
         } catch (ModelException expected) {
-            Assert.assertTrue(expected.getMessage().startsWith("Incorrect state of migration"));
+            Assert.assertTrue(expected.getMessage().startsWith("Incorrect state of migration. You are trying to run server version"));
         } finally {
+            // Revert both versions to the state before the test
             Version.VERSION = currentVersion;
+            model.setStoredVersion(databaseVersion);
+        }
+    }
+
+    /**
+     * Tests migration should not be allowed when DB version is set to non-snapshot version like "23.0.0", but Keycloak server version is snapshot version "999.0.0"
+     */
+    @Test
+    @ModelTest
+    public void testMigrationDeniedWithDBNonSnapshotAndServerSnapshot(KeycloakSession session) {
+        MigrationModel model = session.getProvider(DeploymentStateProvider.class).getMigrationModel();
+        String databaseVersion = model.getStoredVersion();
+        Assert.assertNotNull("Stored DB version was null", model.getStoredVersion());
+
+        String currentVersion = Version.VERSION;
+        try {
+            // Simulate to manually set DB version to 23 when server version is SNAPSHOT. Migration should fail as it is an attempt to run production DB with the development server
+            Version.VERSION = Constants.SNAPSHOT_VERSION;
+            model.setStoredVersion("23.0.0");
+
+            new DefaultMigrationManager(session, false).migrate();
+            Assert.fail("Not expected to successfully run migration. DB version was " + databaseVersion + ". Keycloak version was " + currentVersion);
+        } catch (ModelException expected) {
+            Assert.assertTrue(expected.getMessage().startsWith("Incorrect state of migration. You are trying to run nightly server version"));
+        } finally {
+            // Revert both versions to the state before the test
+            Version.VERSION = currentVersion;
+            model.setStoredVersion(databaseVersion);
         }
     }
 }