From 2161e72872d336d76a0d98500e25a4819c28f99b Mon Sep 17 00:00:00 2001 From: Stefan Guilhen Date: Fri, 9 Feb 2024 10:41:35 -0300 Subject: [PATCH] Add migration for the useTruststoreSpi config property in LDAP user storage provider - legacy `ldapsOnly` value now migrated to `always`. Closes #25912 Signed-off-by: Stefan Guilhen --- .../org/keycloak/storage/ldap/LDAPConfig.java | 3 +- .../migration/migrators/MigrateTo24_0_0.java | 14 ++++++++ .../migration/AbstractMigrationTest.java | 30 ++++++++++++++-- .../JsonFileImport1903MigrationTest.java | 2 +- .../testsuite/migration/MigrationTest.java | 2 +- .../migration-realm-19.0.3.json | 36 +++++++++++++++++++ 6 files changed, 81 insertions(+), 6 deletions(-) diff --git a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java index d72b65412bb..bebb82b473c 100644 --- a/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java +++ b/federation/ldap/src/main/java/org/keycloak/storage/ldap/LDAPConfig.java @@ -65,8 +65,7 @@ public class LDAPConfig { } public String getUseTruststoreSpi() { - String value = config.getFirst(LDAPConstants.USE_TRUSTSTORE_SPI); - return LDAPConstants.USE_TRUSTSTORE_LDAPS_ONLY.equals(value) ? LDAPConstants.USE_TRUSTSTORE_ALWAYS : value; + return config.getFirst(LDAPConstants.USE_TRUSTSTORE_SPI); } public String getUsersDn() { diff --git a/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo24_0_0.java b/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo24_0_0.java index b460faec87b..019781e148d 100644 --- a/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo24_0_0.java +++ b/model/storage-private/src/main/java/org/keycloak/migration/migrators/MigrateTo24_0_0.java @@ -23,10 +23,12 @@ import org.jboss.logging.Logger; import org.keycloak.migration.ModelVersion; import org.keycloak.models.KeycloakContext; import org.keycloak.models.KeycloakSession; +import org.keycloak.models.LDAPConstants; import org.keycloak.models.RealmModel; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.userprofile.config.UPConfig; import org.keycloak.representations.userprofile.config.UPConfig.UnmanagedAttributePolicy; +import org.keycloak.storage.UserStorageProvider; import org.keycloak.userprofile.UserProfileProvider; public class MigrateTo24_0_0 implements Migration { @@ -55,6 +57,7 @@ public class MigrateTo24_0_0 implements Migration { try { context.setRealm(realm); updateUserProfileSettings(session); + updateLdapProviderConfig(session); } finally { context.setRealm(null); } @@ -82,4 +85,15 @@ public class MigrateTo24_0_0 implements Migration { LOG.debugf("Enabled the declarative user profile to realm %s with support for unmanaged attributes", realm.getName()); } + + private void updateLdapProviderConfig(final KeycloakSession session) { + RealmModel realm = session.getContext().getRealm(); + // ensure `ldapsOnly` value for `useTruststoreSpi` in LDAP providers is migrated to `always`. + realm.getComponentsStream(realm.getId(), UserStorageProvider.class.getName()) + .filter(c -> LDAPConstants.USE_TRUSTSTORE_LDAPS_ONLY.equals(c.getConfig().getFirst(LDAPConstants.USE_TRUSTSTORE_SPI))) + .forEach(c -> { + c.getConfig().putSingle(LDAPConstants.USE_TRUSTSTORE_SPI, LDAPConstants.USE_TRUSTSTORE_ALWAYS); + realm.updateComponent(c); + }); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java index 57a7e52d0be..58c8f45ded5 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/AbstractMigrationTest.java @@ -32,6 +32,7 @@ import org.keycloak.authentication.authenticators.browser.OTPFormAuthenticatorFa import org.keycloak.authentication.authenticators.conditional.ConditionalUserConfiguredAuthenticatorFactory; import org.keycloak.broker.provider.util.SimpleHttp; import org.keycloak.common.constants.KerberosConstants; +import org.keycloak.common.util.MultivaluedHashMap; import org.keycloak.component.PrioritizedComponentModel; import org.keycloak.keys.KeyProvider; import org.keycloak.models.AccountRoles; @@ -398,13 +399,16 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { testRegistrationProfileFormActionRemoved(migrationRealm2); } - protected void testMigrationTo24_0_0(boolean testUserProfileMigration) { + protected void testMigrationTo24_0_0(boolean testUserProfileMigration, boolean testLdapUseTruststoreSpiMigration) { if (testUserProfileMigration) { testUserProfileEnabledByDefault(migrationRealm); testUnmanagedAttributePolicySet(migrationRealm, UnmanagedAttributePolicy.ENABLED); testUserProfileEnabledByDefault(migrationRealm2); testUnmanagedAttributePolicySet(migrationRealm2, null); } + if (testLdapUseTruststoreSpiMigration) { + testLdapUseTruststoreSpiMigration(migrationRealm2); + } } protected void testDeleteAccount(RealmResource realm) { @@ -1092,7 +1096,11 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { } protected void testMigrationTo24_x(boolean testUserProfileMigration) { - testMigrationTo24_0_0(testUserProfileMigration); + testMigrationTo24_0_0(testUserProfileMigration, false); + } + + protected void testMigrationTo24_x(boolean testUserProfileMigration, boolean testLdapUseTruststoreSpiMigration) { + testMigrationTo24_0_0(testUserProfileMigration, testLdapUseTruststoreSpiMigration); } protected void testMigrationTo7_x(boolean supportedAuthzServices) { @@ -1211,4 +1219,22 @@ public abstract class AbstractMigrationTest extends AbstractKeycloakTest { UPConfig upConfig = realm.users().userProfile().getConfiguration(); assertEquals(policy, upConfig.getUnmanagedAttributePolicy()); } + + /** + * Checks if the {@code useTruststoreSpi} flag in the LDAP federation provider present in realm {@code Migration2} + * was properly migrated from the old value {@code ldapsOnly} to {@code always}. + *

+ * This provider was added to the file migration-realm-19.0.3.json as a disabled provider, so it doesn't get involved + * in actual user searches and is there just to test the migration of the {@code useTruststoreSpi} config attribute. + * + * @param realm the migrated realm resource. + */ + private void testLdapUseTruststoreSpiMigration(final RealmResource realm) { + RealmRepresentation rep = realm.toRepresentation(); + List componentsRep = realm.components().query(rep.getId(), UserStorageProvider.class.getName()); + assertThat(componentsRep.size(), equalTo(1)); + MultivaluedHashMap config = componentsRep.get(0).getConfig(); + assertNotNull(config); + assertThat(config.getFirst(LDAPConstants.USE_TRUSTSTORE_SPI), equalTo(LDAPConstants.USE_TRUSTSTORE_ALWAYS)); + } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/JsonFileImport1903MigrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/JsonFileImport1903MigrationTest.java index 5de6d14b47c..2daa2d14a34 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/JsonFileImport1903MigrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/JsonFileImport1903MigrationTest.java @@ -67,7 +67,7 @@ public class JsonFileImport1903MigrationTest extends AbstractJsonFileImportMigra testMigrationTo21_x(); testMigrationTo22_x(); testMigrationTo23_x(true); - testMigrationTo24_x(true); + testMigrationTo24_x(true, true); } @Test diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationTest.java index 1f5e736b2dc..22c5edbadbd 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/migration/MigrationTest.java @@ -69,6 +69,6 @@ public class MigrationTest extends AbstractMigrationTest { testMigrationTo21_x(); testMigrationTo22_x(); testMigrationTo23_x(true); - testMigrationTo24_x(true); + testMigrationTo24_x(true, true); } } diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-19.0.3.json b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-19.0.3.json index 9ec90da5143..5a85c082919 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-19.0.3.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/migration-test/migration-realm-19.0.3.json @@ -2912,6 +2912,42 @@ "identityProviders" : [ ], "identityProviderMappers" : [ ], "components" : { + "org.keycloak.storage.UserStorageProvider": [ + { + "id": "307bba27-f8da-4c4d-a1b9-3c1bf49f824e", + "name": "ldap", + "providerId": "ldap", + "subComponents": {}, + "config": { + "pagination": [ "false" ], + "fullSyncPeriod": [ "-1" ], + "startTls": [ "false" ], + "usersDn": [ "ou=People,dc=keycloak,dc=org" ], + "connectionPooling": [ "false" ], + "cachePolicy": [ "DEFAULT" ], + "useKerberosForPasswordAuthentication": [ "false" ], + "importEnabled": [ "true" ], + "enabled": [ "false" ], + "bindCredential": [ "anything" ], + "changedSyncPeriod": [ "-1" ], + "bindDn": [ "uid=admin,ou=system" ], + "usernameLDAPAttribute": [ "uid" ], + "vendor": [ "other" ], + "uuidLDAPAttribute": [ "entryUUID" ], + "connectionUrl": [ "ldap://localhost:10389" ], + "allowKerberosAuthentication": [ "false" ], + "syncRegistrations": [ "true" ], + "authType": [ "simple" ], + "useTruststoreSpi": [ "ldapsOnly" ], + "usePasswordModifyExtendedOp": [ "false" ], + "trustEmail": [ "false" ], + "userObjectClasses": [ "inetOrgPerson, organizationalPerson" ], + "rdnLDAPAttribute": [ "uid" ], + "editMode": [ "WRITABLE" ], + "validatePasswordPolicy": [ "false" ] + } + } + ], "org.keycloak.userprofile.UserProfileProvider": [ { "id": "88cef18c-bcd8-40d2-9e7d-d257298317f2",