From daca6d706284f99f2b39403b499ec5fe2adae334 Mon Sep 17 00:00:00 2001 From: mposolda Date: Fri, 19 Feb 2016 18:02:39 +0100 Subject: [PATCH] KEYCLOAK-2505 Keystore configuration is not honored for LDAP over SSL connections --- .../keycloak/federation/ldap/LDAPConfig.java | 5 ++-- .../idm/store/ldap/LDAPOperationManager.java | 12 +++------ .../org/keycloak/models/LDAPConstants.java | 25 ++++++++++++++++++- .../managers/LDAPConnectionTestManager.java | 19 +++++++++++--- .../resources/admin/RealmAdminResource.java | 5 ++-- .../messages/admin-messages_en.properties | 2 ++ .../admin/resources/js/controllers/users.js | 13 +++++++++- .../resources/partials/federated-ldap.html | 13 ++++++++++ 8 files changed, 75 insertions(+), 19 deletions(-) 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 537f6c8ac5..15af1332a4 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/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 cf2b7caf83..59b76beece 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/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java b/server-spi/src/main/java/org/keycloak/models/LDAPConstants.java index a8830b683d..b560e1844e 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 Marek Posolda */ @@ -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 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 441dca2e75..e4c7ce6b95 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 Marek Posolda */ @@ -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 env = new Hashtable(); 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 f0cbe8b086..0c9f4ce341 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/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 6d547f7627..57836f55a2 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 405e4e9076..0fc333aaf8 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 4a2dc920cc..5c046b7f06 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 @@ {{:: 'ldap.search-scope.tooltip' | translate}} +
+ +
+
+ +
+
+ {{:: 'ldap.use-truststore-spi.tooltip' | translate}} +