From 2703388946789c5e5b802ae8b1f43b76cd873f0f Mon Sep 17 00:00:00 2001 From: mhajas Date: Tue, 10 Sep 2019 19:28:13 +0200 Subject: [PATCH] KEYCLOAK-11245 Adapt LDAPConnectionTestManager to use newly introduced LDAPContextManager --- .../managers/LDAPConnectionTestManager.java | 108 ++++++------------ .../resources/admin/RealmAdminResource.java | 2 +- .../admin-client-test_ldap__bindCredential | 1 + .../servers/auth-server/jboss/pom.xml | 1 + .../UserFederationLdapConnectionTest.java | 7 ++ .../admin-client-test_ldap__bindCredential | 1 + 6 files changed, 46 insertions(+), 74 deletions(-) create mode 100644 testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/admin-client-test_ldap__bindCredential create mode 100644 testsuite/integration-arquillian/tests/base/src/test/resources/vault/admin-client-test_ldap__bindCredential 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 ca7eb17a84..e378a52c91 100755 --- a/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java +++ b/services/src/main/java/org/keycloak/services/managers/LDAPConnectionTestManager.java @@ -17,17 +17,12 @@ package org.keycloak.services.managers; import org.jboss.logging.Logger; +import org.keycloak.common.util.MultivaluedHashMap; +import org.keycloak.models.KeycloakSession; import org.keycloak.models.LDAPConstants; import org.keycloak.services.ServicesLogger; - -import javax.naming.Context; -import javax.naming.NamingException; -import javax.naming.ldap.InitialLdapContext; -import javax.naming.ldap.StartTlsRequest; -import javax.naming.ldap.StartTlsResponse; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLSession; -import java.util.Hashtable; +import org.keycloak.storage.ldap.LDAPConfig; +import org.keycloak.storage.ldap.idm.store.ldap.LDAPContextManager; /** * @author Marek Posolda @@ -39,86 +34,53 @@ 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, String useTruststoreSpi, String connectionTimeout, String tls) { + public static boolean testLDAP(KeycloakSession session, String action, String connectionUrl, String bindDn, + String bindCredential, String useTruststoreSpi, String connectionTimeout, String tls) { if (!TEST_CONNECTION.equals(action) && !TEST_AUTHENTICATION.equals(action)) { ServicesLogger.LOGGER.unknownAction(action); return false; } - InitialLdapContext ldapContext = null; - try { - Hashtable env = new Hashtable(); - env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); - if (connectionUrl == null) { - logger.errorf("Unknown connection URL"); + // Prepare MultivaluedHashMap so that it is usable in LDAPContext class + MultivaluedHashMap ldapConfig = new MultivaluedHashMap<>(); + + if (connectionUrl == null) { + logger.errorf("Unknown connection URL"); + return false; + } + ldapConfig.putSingle(LDAPConstants.CONNECTION_URL, connectionUrl); + ldapConfig.putSingle(LDAPConstants.USE_TRUSTSTORE_SPI, useTruststoreSpi); + ldapConfig.putSingle(LDAPConstants.CONNECTION_TIMEOUT, connectionTimeout); + ldapConfig.putSingle(LDAPConstants.START_TLS, tls); + + if (TEST_AUTHENTICATION.equals(action)) { + // If AUTHENTICATION action is executed add also dn and credentials to configuration + // LDAPContextManager is responsible for correct order of addition of credentials to context in case + // tls is true + + if (bindDn == null) { + logger.error("Unknown bind DN"); return false; } - env.put(Context.PROVIDER_URL, connectionUrl); - LDAPConstants.setTruststoreSpiIfNeeded(useTruststoreSpi, connectionUrl, env); + ldapConfig.putSingle(LDAPConstants.AUTH_TYPE, LDAPConstants.AUTH_TYPE_SIMPLE); + ldapConfig.putSingle(LDAPConstants.BIND_DN, bindDn); + ldapConfig.putSingle(LDAPConstants.BIND_CREDENTIAL, bindCredential); + } else { + ldapConfig.putSingle(LDAPConstants.AUTH_TYPE, LDAPConstants.AUTH_TYPE_NONE); + } - if (connectionTimeout != null && !connectionTimeout.isEmpty()) { - env.put("com.sun.jndi.ldap.connect.timeout", connectionTimeout); - } - - if(tls != null && Boolean.parseBoolean(tls)) { - ldapContext = new InitialLdapContext(env, null); - try { - StartTlsResponse tlsResponse = (StartTlsResponse) ldapContext.extendedOperation(new StartTlsRequest()); - tlsResponse.negotiate(); - } catch (Exception e) { - logger.error("Could not negotiate TLS", e); - } - - if (TEST_AUTHENTICATION.equals(action)) { - ldapContext.addToEnvironment(Context.SECURITY_AUTHENTICATION, "simple"); - - if (bindDn == null) { - logger.error("Unknown bind DN"); - return false; - } - ldapContext.addToEnvironment(Context.SECURITY_PRINCIPAL, bindDn); - - char[] bindCredentialChar = null; - if (bindCredential != null) { - bindCredentialChar = bindCredential.toCharArray(); - } - ldapContext.addToEnvironment(Context.SECURITY_CREDENTIALS, bindCredentialChar); - ldapContext.lookup(""); - } - } else { - 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; - if (bindCredential != null) { - bindCredentialChar = bindCredential.toCharArray(); - } - env.put(Context.SECURITY_CREDENTIALS, bindCredentialChar); - } - ldapContext = new InitialLdapContext(env, null); - } + // Create ldapContextManager in try-with-resource so that ldapContext/tlsResponse/VaultSecret is closed/removed when it is not needed anymore + try (LDAPContextManager ldapContextManager = LDAPContextManager.create(session, new LDAPConfig(ldapConfig))) { + ldapContextManager.getLdapContext(); + // Connection was successful, no exception was raised returning true return true; } catch (Exception ne) { String errorMessage = (TEST_AUTHENTICATION.equals(action)) ? "Error when authenticating to LDAP: " : "Error when connecting to LDAP: "; ServicesLogger.LOGGER.errorAuthenticating(ne, errorMessage + ne.getMessage()); return false; - } finally { - if (ldapContext != null) { - try { - ldapContext.close(); - } catch (NamingException ne) { - ServicesLogger.LOGGER.errorClosingLDAP(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 f2d140deeb..e39573ac2d 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 @@ -937,7 +937,7 @@ public class RealmAdminResource { bindCredential = realm.getComponent(componentId).getConfig().getFirst(LDAPConstants.BIND_CREDENTIAL); } - boolean result = new LDAPConnectionTestManager().testLDAP(action, connectionUrl, bindDn, bindCredential, useTruststoreSpi, connectionTimeout, startTls); + boolean result = LDAPConnectionTestManager.testLDAP(session, action, connectionUrl, bindDn, bindCredential, useTruststoreSpi, connectionTimeout, startTls); return result ? Response.noContent().build() : ErrorResponse.error("LDAP test error", Response.Status.BAD_REQUEST); } diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/admin-client-test_ldap__bindCredential b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/admin-client-test_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/common/vault/admin-client-test_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml b/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml index d4eb073ccf..b42b550f4f 100644 --- a/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/pom.xml @@ -224,6 +224,7 @@ master_smtp__password master_ldap__bindCredential test_ldap__bindCredential + admin-client-test_ldap__bindCredential diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java index 03a1809d70..95cef290d2 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java @@ -55,6 +55,9 @@ public class UserFederationLdapConnectionTest extends AbstractAdminTest { response = realm.testLDAPConnection(LDAPConnectionTestManager.TEST_AUTHENTICATION, "ldap://localhost:10389", "uid=admin,ou=system", "secret", "false", null); assertStatus(response, 204); + // Authentication success with bindCredential from Vault + response = realm.testLDAPConnection(LDAPConnectionTestManager.TEST_AUTHENTICATION, "ldap://localhost:10389", "uid=admin,ou=system", "${vault.ldap_bindCredential}", "false", null); + assertStatus(response, 204); } @Test @@ -74,6 +77,10 @@ public class UserFederationLdapConnectionTest extends AbstractAdminTest { response = realm.testLDAPConnection(LDAPConnectionTestManager.TEST_AUTHENTICATION, "ldaps://localhost:10636", "uid=admin,ou=system", "secret", "true", "10000"); assertStatus(response, 204); + + // Authentication success with bindCredential from Vault + response = realm.testLDAPConnection(LDAPConnectionTestManager.TEST_AUTHENTICATION, "ldaps://localhost:10636", "uid=admin,ou=system", "${vault.ldap_bindCredential}", "true", null); + assertStatus(response, 204); } private void assertStatus(Response response, int status) { diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/vault/admin-client-test_ldap__bindCredential b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/admin-client-test_ldap__bindCredential new file mode 100644 index 0000000000..536aca34db --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/vault/admin-client-test_ldap__bindCredential @@ -0,0 +1 @@ +secret \ No newline at end of file