QPID-6132: [Java Broker] Mark SimpleLDAP attributes providerUrl, searchFilter, searchContext as mandatory.

Also:

* 'ping' the Directory on object creation/change (rationale: discover config errors early, rather than awaiting the first auth)
* Reinstate the LDAP context factory class default.

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

4 files changed, 105 insertions, 39 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 4e285df384..8175deb48b 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
@@ -22,6 +22,7 @@ package org.apache.qpid.server.security.auth.manager;
 
 import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.ManagedAttribute;
+import org.apache.qpid.server.model.ManagedContextDefault;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.TrustStore;
 
@@ -30,23 +31,33 @@ public interface SimpleLDAPAuthenticationManager<X extends SimpleLDAPAuthenticat
 {
     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";
 
-    @ManagedAttribute( description = "LDAP server URL" )
+
+    @ManagedAttribute( description = "LDAP server URL", mandatory = true)
     String getProviderUrl();
 
     @ManagedAttribute( description = "LDAP authentication URL")
     String getProviderAuthUrl();
 
-    @ManagedAttribute( description = "Search context")
+    @ManagedAttribute( description = "Search context", mandatory = true)
     String getSearchContext();
 
-    @ManagedAttribute( description = "Search filter")
+    @ManagedAttribute( description = "Search filter", mandatory = true)
     String getSearchFilter();
 
     @ManagedAttribute( description = "Bind without search")
     boolean isBindWithoutSearch();
 
-    @ManagedAttribute( description = "LDAP context factory")
+    @ManagedContextDefault( name = "ldap.context.factory")
+    String DEFAULT_LDAP_CONTEXT_FACTORY = "com.sun.jndi.ldap.LdapCtxFactory";
+
+    @ManagedAttribute( description = "LDAP context factory", defaultValue = "${ldap.context.factory}")
     String getLdapContextFactory();
 
     @ManagedAttribute( description = "Trust store name")
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 a0ba4518c8..33dfa4a46b 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
@@ -19,15 +19,20 @@
 
 package org.apache.qpid.server.security.auth.manager;
 
+import static java.util.Collections.disjoint;
+import static java.util.Collections.unmodifiableList;
+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.Collections;
+import java.util.Arrays;
 import java.util.Hashtable;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import javax.naming.AuthenticationException;
 import javax.naming.Context;
@@ -48,7 +53,9 @@ import javax.security.sasl.SaslServer;
 
 import org.apache.log4j.Logger;
 
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
 import org.apache.qpid.server.model.TrustStore;
@@ -68,6 +75,13 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
 {
     private static final Logger _logger = Logger.getLogger(SimpleLDAPAuthenticationManagerImpl.class);
 
+    private static final List<String> CONNECTIVITY_ATTRS = unmodifiableList(Arrays.asList(PROVIDER_URL,
+                                                                             PROVIDER_AUTH_URL,
+                                                                             SEARCH_CONTEXT,
+                                                                             LDAP_CONTEXT_FACTORY,
+                                                                             SEARCH_USERNAME,
+                                                                             SEARCH_PASSWORD));
+
     /**
      * Environment key to instruct {@link InitialDirContext} to override the socket factory.
      */
@@ -111,6 +125,23 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         super(attributes, broker);
     }
 
+    @Override
+    protected void validateOnCreate()
+    {
+        super.validateOnCreate();
+        validateInitialDirContext();
+    }
+
+    @Override
+    protected void validateChange(ConfiguredObject<?> proxyForValidation, Set<String> changedAttributes)
+    {
+        super.validateChange(proxyForValidation, changedAttributes);
+
+        if (!disjoint(changedAttributes, CONNECTIVITY_ATTRS))
+        {
+            validateInitialDirContext();
+        }
+    }
 
     @Override
     protected void onOpen()
@@ -118,8 +149,6 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         super.onOpen();
 
         _sslSocketFactoryOverrideClass = createSslSocketFactoryOverrideClass();
-
-      //  validateInitialDirContext();
     }
 
     @Override
@@ -174,7 +203,7 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
     @Override
     public List<String> getMechanisms()
     {
-        return Collections.singletonList(PlainSaslServer.MECHANISM);
+        return singletonList(PlainSaslServer.MECHANISM);
     }
 
     @Override
@@ -362,6 +391,17 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         return null;
     }
 
+    @Override
+    public String toString()
+    {
+        return "SimpleLDAPAuthenticationManagerImpl [id=" + getId() + ", name=" + getName() +
+               ", providerUrl=" + _providerUrl + ", providerAuthUrl=" + _providerAuthUrl +
+               ", searchContext=" + _searchContext + ", state=" + getState() +
+               ", searchFilter=" + _searchFilter + ", ldapContextFactory=" + _ldapContextFactory +
+               ", bindWithoutSearch=" + _bindWithoutSearch  + ", trustStore=" + _trustStore  +
+               ", searchUsername=" + _searchUsername + "]";
+    }
+
     private void validateInitialDirContext()
     {
         Hashtable<String,Object> env = createInitialDirContextEnvironment(_providerUrl);
@@ -375,7 +415,8 @@ public class SimpleLDAPAuthenticationManagerImpl extends AbstractAuthenticationM
         }
         catch (NamingException e)
         {
-            throw new ServerScopedRuntimeException("Unable to establish connection to the ldap server at " + _providerUrl, e);
+            _logger.error("Failed to establish connectivity to the ldap server for " + this, e);
+            throw new IllegalConfigurationException("Failed to establish connectivity to the ldap server." , e);
         }
         finally
         {
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java
index 6001ed1750..9e580d3157 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/SimpleLDAPAuthenticationManagerFactoryTest.java
@@ -57,40 +57,15 @@ public class SimpleLDAPAuthenticationManagerFactoryTest extends TestCase
         _configuration.put(AuthenticationProvider.NAME, getName());
     }
 
-    public void testLdapInstanceCreated() throws Exception
+    public void testLdapCreated() throws Exception
     {
         _configuration.put(AuthenticationProvider.TYPE, SimpleLDAPAuthenticationManager.PROVIDER_TYPE);
-        _configuration.put("providerUrl", "ldap://example.com:389/");
-        _configuration.put("searchContext", "dc=example");
-
-        AuthenticationProvider manager = _factory.create(AuthenticationProvider.class, _configuration, _broker);
-        assertNotNull(manager);
-
-    }
-
-    public void testLdapsInstanceCreated() throws Exception
-    {
-        _configuration.put(AuthenticationProvider.TYPE, SimpleLDAPAuthenticationManager.PROVIDER_TYPE);
-        _configuration.put("providerUrl", "ldaps://example.com:636/");
-        _configuration.put("searchContext", "dc=example");
-
-        AuthenticationProvider manager = _factory.create(AuthenticationProvider.class, _configuration, _broker);
-        assertNotNull(manager);
-
-    }
-
-    public void testLdapsWithTrustStoreInstanceCreated() throws Exception
-    {
-        when(_broker.getChildren(eq(TrustStore.class))).thenReturn(Collections.singletonList(_trustStore));
-
-
-        _configuration.put(AuthenticationProvider.TYPE, SimpleLDAPAuthenticationManager.PROVIDER_TYPE);
         _configuration.put("providerUrl", "ldaps://example.com:636/");
         _configuration.put("searchContext", "dc=example");
-        _configuration.put("trustStore", "mytruststore");
+        _configuration.put("searchFilter", "(uid={0})");
+        _configuration.put("ldapContextFactory", TestLdapDirectoryContext.class.getName());
 
-        AuthenticationProvider manager = _factory.create(AuthenticationProvider.class, _configuration, _broker);
-        assertNotNull(manager);
+        _factory.create(AuthenticationProvider.class, _configuration, _broker);
     }
 
     public void testLdapsWhenTrustStoreNotFound() throws Exception
@@ -100,6 +75,7 @@ public class SimpleLDAPAuthenticationManagerFactoryTest extends TestCase
         _configuration.put(AuthenticationProvider.TYPE, SimpleLDAPAuthenticationManager.PROVIDER_TYPE);
         _configuration.put("providerUrl", "ldaps://example.com:636/");
         _configuration.put("searchContext", "dc=example");
+        _configuration.put("searchFilter", "(uid={0})");
         _configuration.put("trustStore", "notfound");
 
         try
@@ -110,7 +86,7 @@ public class SimpleLDAPAuthenticationManagerFactoryTest extends TestCase
         catch(IllegalArgumentException e)
         {
             // PASS
-            assertTrue("Message does not include underlying issue", e.getMessage().contains("name 'notfound'"));
+            assertTrue("Message does not include underlying issue ", e.getMessage().contains("name 'notfound'"));
             assertTrue("Message does not include the attribute name", e.getMessage().contains("trustStore"));
             assertTrue("Message does not include the expected type", e.getMessage().contains("TrustStore"));
         }
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/TestLdapDirectoryContext.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/TestLdapDirectoryContext.java
new file mode 100644
index 0000000000..87b73c8373
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/security/auth/manager/TestLdapDirectoryContext.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.qpid.server.security.auth.manager;
+
+import static org.mockito.Mockito.mock;
+
+import java.util.Hashtable;
+
+import javax.naming.Context;
+import javax.naming.NamingException;
+import javax.naming.directory.DirContext;
+
+public class TestLdapDirectoryContext implements javax.naming.spi.InitialContextFactory
+{
+    @Override
+    public Context getInitialContext(final Hashtable<?, ?> environment) throws NamingException
+    {
+        return (DirContext)mock(DirContext.class);
+    }
+
+}