QPID-4463: [Java Broker] SimpleLDAPAuthManager - address review comments from Robbie Gemmell

* Ensure that trust stores used by auth managers cannot be deleted.
* Stop unnecessary dependency on jakarta-regexp when built by Ant.
* Make check for ldaps:/ check trim / case insensitive

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

3 files changed, 47 insertions, 16 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
index 5e7bfff4de..2d703e2baf 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/TrustStoreAdapter.java
@@ -38,12 +38,15 @@ import javax.net.ssl.TrustManagerFactory;
 
 import javax.net.ssl.X509TrustManager;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.IntegrityViolationException;
 import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.security.access.Operation;
+import org.apache.qpid.server.security.auth.manager.SimpleLDAPAuthenticationManagerFactory;
 import org.apache.qpid.server.util.MapValueConverter;
 import org.apache.qpid.transport.network.security.ssl.QpidMultipleTrustManager;
 import org.apache.qpid.transport.network.security.ssl.QpidPeersOnlyTrustManager;
@@ -111,9 +114,20 @@ public class TrustStoreAdapter extends AbstractKeyStoreAdapter implements TrustS
                 }
             }
 
+            Collection<AuthenticationProvider> authenticationProviders = new ArrayList<AuthenticationProvider>(_broker.getAuthenticationProviders());
+            for (AuthenticationProvider authProvider : authenticationProviders)
+            {
+                Object attributeType = authProvider.getAttribute(AuthenticationProvider.TYPE);
+                Object attributeValue = authProvider.getAttribute(SimpleLDAPAuthenticationManagerFactory.ATTRIBUTE_TRUST_STORE);
+                if (SimpleLDAPAuthenticationManagerFactory.PROVIDER_TYPE.equals(attributeType)
+                    && storeName.equals(attributeValue))
+                {
+                    throw new IntegrityViolationException("Trust store '" + storeName + "' can't be deleted as it is in use by an authentication manager: " + authProvider.getName());
+                }
+            }
+
             return true;
         }
-
         return false;
     }
 
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 0fb8579eff..3ac185d78b 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
@@ -77,7 +77,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
     /**
      * Dynamically created SSL Socket Factory implementation used in the case where user has specified a trust store.
      */
-    private Class<? extends SocketFactory> _sslSocketFactoryOverride;
+    private Class<? extends SocketFactory> _sslSocketFactoryOverrideClass;
 
 
     SimpleLDAPAuthenticationManager(String authManagerName, String providerSearchUrl, String providerAuthUrl, String searchContext, String searchFilter, String ldapContextFactory, TrustStore trustStore)
@@ -94,7 +94,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
     @Override
     public void initialise()
     {
-        _sslSocketFactoryOverride = createSslSocketFactoryOverride();
+        _sslSocketFactoryOverrideClass = createSslSocketFactoryOverrideClass();
 
         validateInitialDirContext();
     }
@@ -129,7 +129,10 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
             if (server.isComplete())
             {
                 String authorizationID = server.getAuthorizationID();
-                _logger.debug("Authenticated as " + authorizationID);
+                if (_logger.isDebugEnabled())
+                {
+                    _logger.debug("Authenticated as " + authorizationID);
+                }
 
                 return new AuthenticationResult(new UsernamePrincipal(authorizationID));
             }
@@ -174,7 +177,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
             return new AuthenticationResult(AuthenticationStatus.CONTINUE);
         }
 
-        Hashtable<String, Object> env = createInitialDirContentEnvironment(_providerAuthURL);
+        Hashtable<String, Object> env = createInitialDirContextEnvironment(_providerAuthURL);
 
         env.put(Context.SECURITY_AUTHENTICATION, "simple");
         env.put(Context.SECURITY_PRINCIPAL, name);
@@ -212,7 +215,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
     {
     }
 
-    private Hashtable<String, Object> createInitialDirContentEnvironment(String providerUrl)
+    private Hashtable<String, Object> createInitialDirContextEnvironment(String providerUrl)
     {
         Hashtable<String,Object> env = new Hashtable<String,Object>();
         env.put(Context.INITIAL_CONTEXT_FACTORY, _ldapContextFactory);
@@ -224,16 +227,16 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
     {
         ClassLoader existingContextClassloader = null;
 
-        boolean isLdaps = ((String)env.get(Context.PROVIDER_URL)).startsWith("ldaps:");
+        boolean isLdaps = String.valueOf(env.get(Context.PROVIDER_URL)).trim().toLowerCase().startsWith("ldaps:");
 
         boolean revertContentClassLoader = false;
         try
         {
-            if (isLdaps && _sslSocketFactoryOverride != null)
+            if (isLdaps && _sslSocketFactoryOverrideClass != null)
             {
                 existingContextClassloader = Thread.currentThread().getContextClassLoader();
-                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, _sslSocketFactoryOverride.getName());
-                Thread.currentThread().setContextClassLoader(_sslSocketFactoryOverride.getClassLoader());
+                env.put(JAVA_NAMING_LDAP_FACTORY_SOCKET, _sslSocketFactoryOverrideClass.getName());
+                Thread.currentThread().setContextClassLoader(_sslSocketFactoryOverrideClass.getClassLoader());
                 revertContentClassLoader = true;
             }
             return new InitialDirContext(env);
@@ -253,7 +256,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
      *
      * @return generated socket factory class
      */
-    private Class<? extends SocketFactory> createSslSocketFactoryOverride()
+    private Class<? extends SocketFactory> createSslSocketFactoryOverrideClass()
     {
         if (_trustStore != null)
         {
@@ -267,10 +270,13 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
             catch (Exception e)
             {
                 _logger.error("Exception creating SSLContext", e);
-                throw new RuntimeException(e);
+                throw new RuntimeException("Error creating SSLContext for trust store : " + _trustStore.getName() , e);
             }
             Class<? extends AbstractLDAPSSLSocketFactory> clazz = LDAPSSLSocketFactoryGenerator.createSubClass(clazzName, sslContext.getSocketFactory());
-            _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
+            if (_logger.isDebugEnabled())
+            {
+                _logger.debug("Connection to Directory will use custom SSL socket factory : " +  clazz);
+            }
             return clazz;
         }
 
@@ -279,7 +285,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
 
     private void validateInitialDirContext()
     {
-        Hashtable<String,Object> env = createInitialDirContentEnvironment(_providerSearchURL);
+        Hashtable<String,Object> env = createInitialDirContextEnvironment(_providerSearchURL);
         env.put(Context.SECURITY_AUTHENTICATION, "none");
 
         InitialDirContext ctx = null;
@@ -350,7 +356,7 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
 
     private String getNameFromId(String id) throws NamingException
     {
-        Hashtable<String,Object> env = createInitialDirContentEnvironment(_providerSearchURL);
+        Hashtable<String,Object> env = createInitialDirContextEnvironment(_providerSearchURL);
 
         env.put(Context.SECURITY_AUTHENTICATION, "none");
         InitialDirContext ctx = createInitialDirContext(env);
@@ -383,7 +389,11 @@ public class SimpleLDAPAuthenticationManager implements AuthenticationManager
     {
         try
         {
-            ctx.close();
+            if (ctx != null)
+            {
+                ctx.close();
+                ctx = null;
+            }
         }
         catch (Exception e)
         {
diff --git a/qpid/java/lib/poms/bcel-5.2.xml b/qpid/java/lib/poms/bcel-5.2.xml
index b218981e8a..09d404adf2 100644
--- a/qpid/java/lib/poms/bcel-5.2.xml
+++ b/qpid/java/lib/poms/bcel-5.2.xml
@@ -19,4 +19,11 @@
   <groupId>org.apache.bcel</groupId>
   <artifactId>bcel</artifactId>
   <version>5.2</version>
+  <exclusions>
+    <exclusion>
+      <!--  Qpid doesn't require BCEL InstructionFinder, so does not need jakarta-regexp. -->
+      <groupId>jakarta-regexp</groupId>
+      <artifactId>jakarta-regexp</artifactId>
+    </exclusion>
+  </exclusions>
 </dep>