keycloak-memoizeit
Changes
federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java 12(+3 -9)
Details
diff --git a/docbook/auth-server-docs/reference/en/en-US/modules/user-federation.xml b/docbook/auth-server-docs/reference/en/en-US/modules/user-federation.xml
index 93ce15e..f44e6ab 100755
--- a/docbook/auth-server-docs/reference/en/en-US/modules/user-federation.xml
+++ b/docbook/auth-server-docs/reference/en/en-US/modules/user-federation.xml
@@ -135,6 +135,23 @@
                 </variablelist>
             </para>
         </section>
+        <section>
+            <title>Connect to LDAP over SSL</title>
+            <para>
+                When you configure secured connection URL to LDAP (for example <literal>ldaps://myhost.com:636</literal> ) the Keycloak will
+                use SSL for the communication with LDAP server. The important thing is to properly configure truststore on the Keycloak server side,
+                because SSL won't work if Keycloak can't trust the SSL connection with LDAP (Keycloak acts as the <literal>client</literal> here, when LDAP acts as server).
+            </para>
+            <para>
+                The global truststore for the Keycloak can be configured with Truststore SPI in the <literal>keycloak-server.json</literal> file and it's described in the details <link linkend="truststore">here</link>.
+                If you don't configure truststore SPI, the truststore will fallback to the default mechanism provided by Java (either the file provided by system property <literal>javax.net.ssl.trustStore</literal> or finally
+                the cacerts file from JDK if even the system property is not set).
+            </para>
+            <para>There is configuration property <literal>Use Truststore SPI</literal> in the LDAP federation provider configuration, where you can choose
+                whether Truststore SPI is used. By default, the value is <literal>ldaps only</literal>, which is fine for most of deployments, because attempt
+                to use Truststore SPI is done just if connection to LDAP starts with <literal>ldaps</literal> .
+            </para>
+        </section>
     </section>
     <section>
         <title>Sync of LDAP users to Keycloak</title>
                diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPDn.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPDn.java
index 239639c..a872b77 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPDn.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/model/LDAPDn.java
@@ -23,6 +23,8 @@ import java.util.LinkedList;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import javax.naming.ldap.Rdn;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
@@ -127,7 +129,7 @@ public class LDAPDn {
     }
 
     public void addFirst(String rdnName, String rdnValue) {
-        rdnValue = escape(rdnValue);
+        rdnValue = Rdn.escapeValue(rdnValue);
         entries.addFirst(new Entry(rdnName, rdnValue));
     }
 
@@ -135,26 +137,6 @@ public class LDAPDn {
         entries.addLast(new Entry(rdnName, rdnValue));
     }
 
-    // Need to escape "john,dot" to be "john\,dot"
-    private String escape(String rdnValue) {
-        if (rdnValue.contains(",")) {
-            StringBuilder result = new StringBuilder();
-            boolean first = true;
-            for (String split : rdnValue.split(",")) {
-                if (!first) {
-                    result.append("\\,");
-                } else {
-                    first = false;
-                }
-                result.append(split);
-            }
-            return result.toString();
-        } else {
-            return rdnValue;
-        }
-    }
-
-
     private static class Entry {
         private final String attrName;
         private final String attrValue;
                diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java
index cf2b7ca..59b76be 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/idm/store/ldap/LDAPOperationManager.java
@@ -480,15 +480,6 @@ public class LDAPOperationManager {
         env.put(Context.INITIAL_CONTEXT_FACTORY, this.config.getFactoryName());
         env.put(Context.SECURITY_AUTHENTICATION, authType);
 
-        String protocol = this.config.getSecurityProtocol();
-
-        if (protocol != null) {
-            env.put(Context.SECURITY_PROTOCOL, protocol);
-            if ("ssl".equals(protocol)) {
-                env.put("java.naming.ldap.factory.socket", "org.keycloak.connections.truststore.SSLSocketFactory");
-            }
-        }
-
         String bindDN = this.config.getBindDN();
 
         char[] bindCredential = null;
@@ -510,6 +501,9 @@ public class LDAPOperationManager {
             logger.warn("LDAP URL is null. LDAPOperationManager won't work correctly");
         }
 
+        String useTruststoreSpi = this.config.getUseTruststoreSpi();
+        LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, url, env);
+
         String connectionPooling = this.config.getConnectionPooling();
         if (connectionPooling != null) {
             env.put("com.sun.jndi.ldap.connect.pool", connectionPooling);
                diff --git a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
index 537f6c8..15af133 100644
--- a/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
+++ b/federation/ldap/src/main/java/org/keycloak/federation/ldap/LDAPConfig.java
@@ -58,9 +58,8 @@ public class LDAPConfig {
         }
     }
 
-    public String getSecurityProtocol() {
-        // hardcoded for now
-        return config.get(LDAPConstants.SECURITY_PROTOCOL);
+    public String getUseTruststoreSpi() {
+        return config.get(LDAPConstants.USE_TRUSTSTORE_SPI);
     }
 
     public String getUsersDn() {
                diff --git a/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPDnTest.java b/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPDnTest.java
index cd7938c..620a166 100644
--- a/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPDnTest.java
+++ b/federation/ldap/src/test/java/org/keycloak/federation/ldap/idm/model/LDAPDnTest.java
@@ -31,9 +31,9 @@ public class LDAPDnTest {
         dn.addFirst("ou", "People");
         Assert.assertEquals("ou=People,dc=keycloak,dc=org", dn.toString());
 
-        dn.addFirst("uid", "Johny,Depp");
-        Assert.assertEquals("uid=Johny\\,Depp,ou=People,dc=keycloak,dc=org", dn.toString());
-        Assert.assertEquals(LDAPDn.fromString("uid=Johny\\,Depp,ou=People,dc=keycloak,dc=org"), dn);
+        dn.addFirst("uid", "Johny,Depp+Pepp");
+        Assert.assertEquals("uid=Johny\\,Depp\\+Pepp,ou=People,dc=keycloak,dc=org", dn.toString());
+        Assert.assertEquals(LDAPDn.fromString("uid=Johny\\,Depp\\+Pepp,ou=People,dc=keycloak,dc=org"), dn);
 
         Assert.assertEquals("ou=People,dc=keycloak,dc=org", dn.getParentDn());
 
@@ -44,6 +44,6 @@ public class LDAPDnTest {
         Assert.assertFalse(dn.isDescendantOf(dn));
 
         Assert.assertEquals("uid", dn.getFirstRdnAttrName());
-        Assert.assertEquals("Johny\\,Depp", dn.getFirstRdnAttrValue());
+        Assert.assertEquals("Johny\\,Depp\\+Pepp", dn.getFirstRdnAttrValue());
     }
 }
                diff --git a/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java b/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java
index a8830b6..b560e18 100644
--- a/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java
+++ b/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java
@@ -17,6 +17,8 @@
 
 package org.keycloak.models;
 
+import java.util.Map;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
@@ -38,7 +40,6 @@ public class LDAPConstants {
     public static final String USER_OBJECT_CLASSES = "userObjectClasses";
 
     public static final String CONNECTION_URL = "connectionUrl";
-    public static final String SECURITY_PROTOCOL = "securityProtocol";
     public static final String BASE_DN = "baseDn"; // used for tests only
     public static final String USERS_DN = "usersDn";
     public static final String BIND_DN = "bindDn";
@@ -48,6 +49,11 @@ public class LDAPConstants {
     public static final String AUTH_TYPE_NONE = "none";
     public static final String AUTH_TYPE_SIMPLE = "simple";
 
+    public static final String USE_TRUSTSTORE_SPI = "useTruststoreSpi";
+    public static final String USE_TRUSTSTORE_ALWAYS = "always";
+    public static final String USE_TRUSTSTORE_NEVER = "never";
+    public static final String USE_TRUSTSTORE_LDAPS_ONLY = "ldapsOnly";
+
     public static final String SEARCH_SCOPE = "searchScope";
     public static final String CONNECTION_POOLING = "connectionPooling";
     public static final String PAGINATION = "pagination";
@@ -119,4 +125,21 @@ public class LDAPConstants {
 
         return ENTRY_UUID;
     }
+
+
+
+    public static void setTruststoreSpiIfNeeded(String useTruststoreSpi, String url, Map<String, Object> env) {
+        boolean shouldSetTruststore;
+        if (useTruststoreSpi != null && useTruststoreSpi.equals(LDAPConstants.USE_TRUSTSTORE_ALWAYS)) {
+            shouldSetTruststore = true;
+        } else if (useTruststoreSpi != null && useTruststoreSpi.equals(LDAPConstants.USE_TRUSTSTORE_NEVER)) {
+            shouldSetTruststore = false;
+        } else {
+            shouldSetTruststore = (url != null && url.startsWith("ldaps"));
+        }
+
+        if (shouldSetTruststore) {
+            env.put("java.naming.ldap.factory.socket", "org.keycloak.truststore.SSLSocketFactory");
+        }
+    }
 }
                diff --git a/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java b/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java
index 441dca2..e4c7ce6 100755
--- a/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java
+++ b/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java
@@ -16,13 +16,14 @@
  */
 package org.keycloak.services.managers;
 
-import org.keycloak.services.ServicesLogger;
-
 import javax.naming.Context;
 import javax.naming.NamingException;
 import javax.naming.ldap.InitialLdapContext;
 import java.util.Hashtable;
 
+import org.keycloak.models.LDAPConstants;
+import org.keycloak.services.ServicesLogger;
+
 /**
  * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
  */
@@ -33,7 +34,7 @@ public class LDAPConnectionTestManager {
     public static final String TEST_CONNECTION = "testConnection";
     public static final String TEST_AUTHENTICATION = "testAuthentication";
 
-    public boolean testLDAP(String action, String connectionUrl, String bindDn, String bindCredential) {
+    public boolean testLDAP(String action, String connectionUrl, String bindDn, String bindCredential, String useTruststoreSpi) {
         if (!TEST_CONNECTION.equals(action) && !TEST_AUTHENTICATION.equals(action)) {
             logger.unknownAction(action);
             return false;
@@ -43,10 +44,20 @@ public class LDAPConnectionTestManager {
         try {
             Hashtable<String, Object> env = new Hashtable<String, Object>();
             env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory");
+
+            if (connectionUrl == null) {
+                logger.errorf("Unknown connection URL");
+                return false;
+            }
             env.put(Context.PROVIDER_URL, connectionUrl);
 
             if (TEST_AUTHENTICATION.equals(action)) {
                 env.put(Context.SECURITY_AUTHENTICATION, "simple");
+
+                if (bindDn == null) {
+                    logger.error("Unknown bind DN");
+                    return false;
+                }
                 env.put(Context.SECURITY_PRINCIPAL, bindDn);
 
                 char[] bindCredentialChar = null;
@@ -56,6 +67,8 @@ public class LDAPConnectionTestManager {
                 env.put(Context.SECURITY_CREDENTIALS, bindCredentialChar);
             }
 
+            LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, connectionUrl, env);
+
             ldapContext = new InitialLdapContext(env, null);
             return true;
         } catch (Exception ne) {
                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 f0cbe8b..0c9f4ce 100644
--- a/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/RealmAdminResource.java
@@ -652,10 +652,11 @@ public class RealmAdminResource {
     @GET
     @NoCache
     public Response testLDAPConnection(@QueryParam("action") String action, @QueryParam("connectionUrl") String connectionUrl,
-                                       @QueryParam("bindDn") String bindDn, @QueryParam("bindCredential") String bindCredential) {
+                                       @QueryParam("bindDn") String bindDn, @QueryParam("bindCredential") String bindCredential,
+                                       @QueryParam("useTruststoreSpi") String useTruststoreSpi) {
         auth.init(RealmAuth.Resource.REALM).requireManage();
 
-        boolean result = new LDAPConnectionTestManager().testLDAP(action, connectionUrl, bindDn, bindCredential);
+        boolean result = new LDAPConnectionTestManager().testLDAP(action, connectionUrl, bindDn, bindCredential, useTruststoreSpi);
         return result ? Response.noContent().build() : ErrorResponse.error("LDAP test error", Response.Status.BAD_REQUEST);
     }
 
                diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
index f6c59b0..273a125 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/FederationProvidersIntegrationTest.java
@@ -405,6 +405,9 @@ public class FederationProvidersIntegrationTest {
             if (!skip) {
                 LDAPObject johnComma = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "john,comma", "John", "Comma", "johncomma@email.org", null, "12387");
                 FederationTestUtils.updateLDAPPassword(ldapFedProvider, johnComma, "Password1");
+
+                LDAPObject johnPlus = FederationTestUtils.addLDAPUser(ldapFedProvider, appRealm, "john+plus,comma", "John", "Plus", "johnplus@email.org", null, "12387");
+                FederationTestUtils.updateLDAPPassword(ldapFedProvider, johnPlus, "Password1");
             }
         } finally {
             keycloakRule.stopSession(session, false);
@@ -413,6 +416,7 @@ public class FederationProvidersIntegrationTest {
         if (!skip) {
             // Try to import the user with comma in username into Keycloak
             loginSuccessAndLogout("john,comma", "Password1");
+            loginSuccessAndLogout("john+plus,comma", "Password1");
         }
     }
 
                diff --git a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
index 6d547f7..57836f5 100644
--- a/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
+++ b/themes/src/main/resources/theme/base/admin/messages/admin-messages_en.properties
@@ -653,6 +653,8 @@ custom-user-ldap-filter=Custom User LDAP Filter
 ldap.custom-user-ldap-filter.tooltip=Additional LDAP Filter for filtering searched users. Leave this empty if you don't need additional filter. Make sure that it starts with '(' and ends with ')'
 search-scope=Search Scope
 ldap.search-scope.tooltip=For one level, we search for users just in DNs specified by User DNs. For subtree, we search in whole of their subtree. See LDAP documentation for more details
+use-truststore-spi=Use Truststore SPI
+ldap.use-truststore-spi.tooltip=Specifies whether LDAP connection will use the truststore SPI with the truststore configured in keycloak-server.json. 'Always' means that it will always use it. 'Never' means that it won't use it. 'Only for ldaps' means that it will use if your connection URL use ldaps. Note even if keycloak-server.json is not configured, the default Java cacerts or certificate specified by 'javax.net.ssl.trustStore' property will be used.
 connection-pooling=Connection Pooling
 ldap.connection-pooling.tooltip=Does Keycloak should use connection pooling for accessing LDAP server
 ldap.pagination.tooltip=Does the LDAP server support pagination.
                diff --git a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
index 405e4e9..0fc333a 100755
--- a/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
+++ b/themes/src/main/resources/theme/base/admin/resources/js/controllers/users.js
@@ -771,6 +771,12 @@ module.controller('LDAPCtrl', function($scope, $location, $route, Notifications,
         { "id": "2", "name": "Subtree" }
     ];
 
+    $scope.useTruststoreOptions = [
+        { "id": "always", "name": "Always" },
+        { "id": "ldapsOnly", "name": "Only for ldaps" },
+        { "id": "never", "name": "Never" }
+    ];
+
     var DEFAULT_BATCH_SIZE = "1000";
 
     $scope.create = !instance.providerName;
@@ -793,6 +799,7 @@ module.controller('LDAPCtrl', function($scope, $location, $route, Notifications,
             instance.config.authType = 'simple';
             instance.config.batchSizeForSync = DEFAULT_BATCH_SIZE;
             instance.config.searchScope = "1";
+            instance.config.useTruststoreSpi = "ldapsOnly";
 
             $scope.fullSyncEnabled = false;
             $scope.changedSyncEnabled = false;
@@ -815,6 +822,9 @@ module.controller('LDAPCtrl', function($scope, $location, $route, Notifications,
             if (!instance.config.searchScope) {
                 instance.config.searchScope = '1';
             }
+            if (!instance.config.useTruststoreSpi) {
+                instance.config.useTruststoreSpi = "ldapsOnly";
+            }
 
             $scope.fullSyncEnabled = (instance.fullSyncPeriod && instance.fullSyncPeriod > 0);
             $scope.changedSyncEnabled = (instance.changedSyncPeriod && instance.changedSyncPeriod > 0);
@@ -939,7 +949,8 @@ module.controller('LDAPCtrl', function($scope, $location, $route, Notifications,
             realm: $scope.realm.realm,
             connectionUrl: ldapConfig.connectionUrl,
             bindDn: ldapConfig.bindDn,
-            bindCredential: ldapConfig.bindCredential
+            bindCredential: ldapConfig.bindCredential,
+            useTruststoreSpi: ldapConfig.useTruststoreSpi
         };
     };
 
                diff --git a/themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html b/themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
index 4a2dc92..5c046b7 100755
--- a/themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
+++ b/themes/src/main/resources/theme/base/admin/resources/partials/federated-ldap.html
@@ -163,6 +163,19 @@
                 </div>
                 <kc-tooltip>{{:: 'ldap.search-scope.tooltip' | translate}}</kc-tooltip>
             </div>
+            <div class="form-group">
+                <label class="col-md-2 control-label" for="useTruststoreSpi">{{:: 'use-truststore-spi' | translate}}</label>
+                <div class="col-md-6">
+                    <div>
+                        <select class="form-control" id="useTruststoreSpi"
+                                ng-model="instance.config.useTruststoreSpi"
+                                ng-options="useTruststoreSpi.id as useTruststoreSpi.name for useTruststoreSpi in useTruststoreOptions"
+                                required>
+                        </select>
+                    </div>
+                </div>
+                <kc-tooltip>{{:: 'ldap.use-truststore-spi.tooltip' | translate}}</kc-tooltip>
+            </div>
             <div class="form-group clearfix">
                 <label class="col-md-2 control-label" for="connectionPooling">{{:: 'connection-pooling' | translate}}</label>
                 <div class="col-md-6">