QPID-6132: [Java Broker] Fix defect introduced by r1629664 (validateChange was not validating the to be state).

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1629695 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 51 insertions, 56 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
index 8175deb48b..260fdfd1c6 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManager.java
@@ -30,13 +30,13 @@ import org.apache.qpid.server.model.TrustStore;
 public interface SimpleLDAPAuthenticationManager<X extends SimpleLDAPAuthenticationManager<X>> extends AuthenticationProvider<X>
 {
     String PROVIDER_TYPE = "SimpleLDAP";
-    String TRUST_STORE = "trustStore";
     String PROVIDER_URL = "providerUrl";
     String PROVIDER_AUTH_URL = "providerAuthUrl";
     String SEARCH_CONTEXT = "searchContext";
     String LDAP_CONTEXT_FACTORY = "ldapContextFactory";
     String SEARCH_USERNAME = "getSearchUsername";
     String SEARCH_PASSWORD = "getSearchPassword";
+    String TRUST_STORE = "trustStore";
 
 
     @ManagedAttribute( description = "LDAP server URL", mandatory = true)
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java
index 33dfa4a46b..6a0b83eb94 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerImpl.java
@@ -25,8 +25,6 @@ import static java.util.Collections.singletonList;
 
 import java.io.IOException;
 import java.security.GeneralSecurityException;
-import java.security.KeyManagementException;
-import java.security.NoSuchAlgorithmException;
 import java.security.Principal;
 import java.util.Arrays;
 import java.util.Hashtable;
@@ -66,7 +64,6 @@ import org.apache.qpid.server.security.auth.manager.ldap.AbstractLDAPSSLSocketFa
 import org.apache.qpid.server.security.auth.manager.ldap.LDAPSSLSocketFactoryGenerator;
 import org.apache.qpid.server.security.auth.sasl.plain.PlainPasswordCallback;
 import org.apache.qpid.server.security.auth.sasl.plain.PlainSaslServer;
-import org.apache.qpid.server.util.ServerScopedRuntimeException;
 import org.apache.qpid.server.util.StringUtil;
 import org.apache.qpid.ssl.SSLContextFactory;
 
@@ -80,7 +77,8 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
                                                                              SEARCH_CONTEXT,
                                                                              LDAP_CONTEXT_FACTORY,
                                                                              SEARCH_USERNAME,
-                                                                             SEARCH_PASSWORD));
+                                                                             SEARCH_PASSWORD,
+                                                                             TRUST_STORE));
 
     /**
      * Environment key to instruct {@link InitialDirContext} to override the socket factory.
@@ -129,17 +127,24 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
     protected void validateOnCreate()
     {
         super.validateOnCreate();
-        validateInitialDirContext();
+
+        Class<? extends SocketFactory> sslSocketFactoryOverrideClass = _trustStore == null ? null : createSslSocketFactoryOverrideClass(_trustStore);
+        validateInitialDirContext(sslSocketFactoryOverrideClass, _providerUrl, _searchUsername, _searchPassword);
     }
 
     @Override
-    protected void validateChange(ConfiguredObject<?> proxyForValidation, Set<String> changedAttributes)
+    protected void validateChange(final ConfiguredObject<?> proxyForValidation, final Set<String> changedAttributes)
     {
         super.validateChange(proxyForValidation, changedAttributes);
 
         if (!disjoint(changedAttributes, CONNECTIVITY_ATTRS))
         {
-            validateInitialDirContext();
+            SimpleLDAPAuthenticationManager changed = (SimpleLDAPAuthenticationManager)proxyForValidation;
+            TrustStore changedTruststore = changed.getTrustStore();
+            Class<? extends SocketFactory> sslSocketFactoryOverrideClass = changedTruststore == null ? null : createSslSocketFactoryOverrideClass(
+                    changedTruststore);
+            validateInitialDirContext(sslSocketFactoryOverrideClass, changed.getProviderUrl(), changed.getSearchUsername(),
+                                      changed.getSearchPassword());
         }
     }
 
@@ -148,7 +153,7 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
     {
         super.onOpen();
 
-        _sslSocketFactoryOverrideClass = createSslSocketFactoryOverrideClass();
+        _sslSocketFactoryOverrideClass = _trustStore == null ? null : createSslSocketFactoryOverrideClass(_trustStore);
     }
 
     @Override
@@ -288,7 +293,7 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         InitialDirContext ctx = null;
         try
         {
-            ctx = createInitialDirContext(env);
+            ctx = createInitialDirContext(env, _sslSocketFactoryOverrideClass);
 
             //Authentication succeeded
             return new AuthenticationResult(new UsernamePrincipal(name));
@@ -320,7 +325,8 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         return env;
     }
 
-    private InitialDirContext createInitialDirContext(Hashtable<String, Object> env) throws NamingException
+    private InitialDirContext createInitialDirContext(Hashtable<String, Object> env,
+                                                      Class<? extends SocketFactory> sslSocketFactoryOverrideClass) throws NamingException
     {
         ClassLoader existingContextClassLoader = null;
 
@@ -329,11 +335,11 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         boolean revertContentClassLoader = false;
         try
         {
-            if (isLdaps && _sslSocketFactoryOverrideClass != null)
+            if (isLdaps && sslSocketFactoryOverrideClass != null)
             {
                 existingContextClassLoader = Thread.currentThread().getContextClassLoader();
-                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, _sslSocketFactoryOverrideClass.getName());
-                Thread.currentThread().setContextClassLoader(_sslSocketFactoryOverrideClass.getClassLoader());
+                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, sslSocketFactoryOverrideClass.getName());
+                Thread.currentThread().setContextClassLoader(sslSocketFactoryOverrideClass.getClassLoader());
                 revertContentClassLoader = true;
             }
             return new InitialDirContext(env);
@@ -352,43 +358,29 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
      * associated with the {@link SSLContext} generated from that trust store.
      *
      * @return generated socket factory class
+     * @param trustStore
      */
-    private Class<? extends SocketFactory> createSslSocketFactoryOverrideClass()
+    private Class<? extends SocketFactory> createSslSocketFactoryOverrideClass(final TrustStore trustStore)
     {
-        if (_trustStore != null)
+        String clazzName = new StringUtil().createUniqueJavaName(getName() + "_" + trustStore.getName());
+        SSLContext sslContext = null;
+        try
         {
-            String clazzName = new StringUtil().createUniqueJavaName(getName());
-            SSLContext sslContext = null;
-            try
-            {
-                sslContext = SSLContext.getInstance("TLS");
-                sslContext.init(null, _trustStore.getTrustManagers(), null);
-            }
-            catch (NoSuchAlgorithmException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
-            catch (KeyManagementException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
-            catch (GeneralSecurityException e)
-            {
-                _logger.error("Exception creating SSLContext", e);
-                throw new ServerScopedRuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
-            }
-
-            Class<? extends AbstractLDAPSSLSocketFactory> clazz = LDAPSSLSocketFactoryGenerator.createSubClass(clazzName, sslContext.getSocketFactory());
-            if (_logger.isDebugEnabled())
-            {
-                _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
-            }
-            return clazz;
+            sslContext = SSLContext.getInstance("TLS");
+            sslContext.init(null, trustStore.getTrustManagers(), null);
+        }
+        catch (GeneralSecurityException e)
+        {
+            _logger.error("Exception creating SSLContext", e);
+            throw new IllegalConfigurationException("Error creating SSLContext with trust store : " + trustStore.getName() , e);
         }
 
-        return null;
+        Class<? extends AbstractLDAPSSLSocketFactory> clazz = LDAPSSLSocketFactoryGenerator.createSubClass(clazzName, sslContext.getSocketFactory());
+        if (_logger.isDebugEnabled())
+        {
+            _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
+        }
+        return clazz;
     }
 
     @Override
@@ -402,20 +394,22 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
                ", searchUsername=" + _searchUsername + "]";
     }
 
-    private void validateInitialDirContext()
+    private void validateInitialDirContext(Class<? extends SocketFactory> sslSocketFactoryOverrideClass,
+                                           final String providerUrl,
+                                           final String searchUsername, final String searchPassword)
     {
-        Hashtable<String,Object> env = createInitialDirContextEnvironment(_providerUrl);
+        Hashtable<String,Object> env = createInitialDirContextEnvironment(providerUrl);
 
-        setupSearchContext(env);
+        setupSearchContext(env, searchUsername, searchPassword);
 
         InitialDirContext ctx = null;
         try
         {
-            ctx = createInitialDirContext(env);
+            ctx = createInitialDirContext(env, sslSocketFactoryOverrideClass);
         }
         catch (NamingException e)
         {
-            _logger.error("Failed to establish connectivity to the ldap server for " + this, e);
+            _logger.error("Failed to establish connectivity to the ldap server for " + providerUrl, e);
             throw new IllegalConfigurationException("Failed to establish connectivity to the ldap server." , e);
         }
         finally
@@ -424,13 +418,14 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         }
     }
 
-    private void setupSearchContext(final Hashtable<String, Object> env)
+    private void setupSearchContext(final Hashtable<String, Object> env,
+                                    final String searchUsername, final String searchPassword)
     {
         if(_searchUsername != null && _searchUsername.trim().length()>0)
         {
             env.put(Context.SECURITY_AUTHENTICATION, "simple");
-            env.put(Context.SECURITY_PRINCIPAL, _searchUsername);
-            env.put(Context.SECURITY_CREDENTIALS, _searchPassword);
+            env.put(Context.SECURITY_PRINCIPAL, searchUsername);
+            env.put(Context.SECURITY_CREDENTIALS, searchPassword);
         }
         else
         {
@@ -495,9 +490,9 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         {
             Hashtable<String, Object> env = createInitialDirContextEnvironment(_providerUrl);
 
-            setupSearchContext(env);
+            setupSearchContext(env, _searchUsername, _searchPassword);
 
-            InitialDirContext ctx = createInitialDirContext(env);
+            InitialDirContext ctx = createInitialDirContext(env, _sslSocketFactoryOverrideClass);
 
             try
             {