QPID-3997: [Java Broker] Allow assignment of AuthenticationManager on a per-port basis

Address review comments from Rob Godfrey.

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

7 files changed, 91 insertions, 14 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
index 0e538a13a6..b48d8c5fdb 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/ServerConfiguration.java
@@ -84,6 +84,9 @@ public class ServerConfiguration extends ConfigurationPlugin
     public static final String MGMT_CUSTOM_REGISTRY_SOCKET = "management.custom-registry-socket";
     public static final String MGMT_JMXPORT_REGISTRYSERVER = "management.jmxport.registryServer";
     public static final String MGMT_JMXPORT_CONNECTORSERVER = "management.jmxport.connectorServer";
+    public static final String SECURITY_DEFAULT_AUTH_MANAGER = "security.default-auth-manager";
+    public static final String SECURITY_PORT_MAPPINGS_PORT_MAPPING_AUTH_MANAGER = "security.port-mappings.port-mapping.auth-manager";
+    public static final String SECURITY_PORT_MAPPINGS_PORT_MAPPING_PORT = "security.port-mappings.port-mapping.port";
     public static final String STATUS_UPDATES = "status-updates";
     public static final String ADVANCED_LOCALE = "advanced.locale";
     public static final String CONNECTOR_AMQP10ENABLED = "connector.amqp10enabled";
@@ -252,6 +255,13 @@ public class ServerConfiguration extends ConfigurationPlugin
             throw new ConfigurationException(message);
         }
 
+        String[] ports = getConfig().getStringArray(SECURITY_PORT_MAPPINGS_PORT_MAPPING_PORT);
+        String[] authManagers = getConfig().getStringArray(SECURITY_PORT_MAPPINGS_PORT_MAPPING_AUTH_MANAGER);
+        if (ports.length != authManagers.length)
+        {
+            throw new ConfigurationException("Validation error: Each port-mapping must have exactly one port and exactly one auth-manager.");
+        }
+
         // QPID-3517: Inconsistency in capitalisation in the SSL configuration keys used within the connector and management configuration
         // sections. For the moment, continue to understand both but generate a deprecated warning if the less preferred keystore is used.
         for (String key : new String[] {"management.ssl.keystorePath",
@@ -586,14 +596,13 @@ public class ServerConfiguration extends ConfigurationPlugin
 
     public String getDefaultAuthenticationManager()
     {
-        return getStringValue("security.default-auth-manager");
+        return getStringValue(SECURITY_DEFAULT_AUTH_MANAGER);
     }
 
-
     public Map<Integer, String> getPortAuthenticationMappings()
     {
-        String[] ports = getConfig().getStringArray("security.port-mappings.port-mapping.port");
-        String[] authManagers = getConfig().getStringArray("security.port-mappings.port-mapping.auth-manager");
+        String[] ports = getConfig().getStringArray(SECURITY_PORT_MAPPINGS_PORT_MAPPING_PORT);
+        String[] authManagers = getConfig().getStringArray(SECURITY_PORT_MAPPINGS_PORT_MAPPING_AUTH_MANAGER);
 
         Map<Integer,String> portMappings = new HashMap<Integer, String>();
         for(int i = 0; i < ports.length; i++)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
index ec6f6d0410..80a91be262 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/registry/ApplicationRegistry.java
@@ -564,7 +564,7 @@ public abstract class ApplicationRegistry implements IApplicationRegistry
     @Override
     public AuthenticationManager getAuthenticationManager(SocketAddress address)
     {
-        return _authenticationManagerRegistry.getAuthenticationManagerFor(address);
+        return _authenticationManagerRegistry.getAuthenticationManager(address);
     }
 
     public PluginManager getPluginManager()
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistry.java
index 34f038b037..3a1ca4f19d 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistry.java
@@ -90,7 +90,7 @@ public class AuthenticationManagerRegistry implements Closeable, IAuthentication
     }
 
     @Override
-    public AuthenticationManager getAuthenticationManagerFor(SocketAddress address)
+    public AuthenticationManager getAuthenticationManager(SocketAddress address)
     {
         AuthenticationManager authManager =
                 address instanceof InetSocketAddress
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/IAuthenticationManagerRegistry.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/IAuthenticationManagerRegistry.java
index 5c20d77804..bfb49b8ed6 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/IAuthenticationManagerRegistry.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/auth/manager/IAuthenticationManagerRegistry.java
@@ -26,7 +26,7 @@ import org.apache.qpid.common.Closeable;
 /**
  * Registry for {@link AuthenticationManager} instances.
  *
- * <p>A lookup method {@link #getAuthenticationManagerFor(SocketAddress)} allows a caller to determine
+ * <p>A lookup method {@link #getAuthenticationManager(SocketAddress)} allows a caller to determine
  * the AuthenticationManager associated with a particular port number.</p>
  *
  * <p>It is important to {@link #close()} the registry after use and this allows the AuthenticationManagers
@@ -42,5 +42,5 @@ public interface IAuthenticationManagerRegistry extends Closeable
      * @param address
      * @return authentication manager.
      */
-    public AuthenticationManager getAuthenticationManagerFor(SocketAddress address);
+    public AuthenticationManager getAuthenticationManager(SocketAddress address);
 }
\ No newline at end of file
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
index 958cb23da0..c2d2eb37c1 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/ServerConfigurationTest.java
@@ -1682,6 +1682,75 @@ public class ServerConfigurationTest extends QpidTestCase
         assertEquals(AmqpProtocolVersion.v0_10, _serverConfig.getDefaultSupportedProtocolReply());
     }
 
+    public void testDefaultAuthenticationManager() throws Exception
+    {
+        // Check default
+        _serverConfig.initialise();
+        assertNull("unexpected default value", _serverConfig.getDefaultAuthenticationManager());
+
+        // Check values we set
+        String testAuthManager = "myauthmanager";
+        _config.addProperty("security.default-auth-manager", testAuthManager);
+        _serverConfig = new ServerConfiguration(_config);
+        _serverConfig.initialise();
+        assertEquals(testAuthManager, _serverConfig.getDefaultAuthenticationManager());
+    }
+
+    public void testPortAuthenticationMappingsDefault() throws Exception
+    {
+        _serverConfig.initialise();
+        assertEquals("unexpected default number of port/authmanager mappings", 0, _serverConfig.getPortAuthenticationMappings().size());
+    }
+
+    public void testPortAuthenticationMappingsWithSingleMapping() throws Exception
+    {
+        String testAuthManager = "myauthmanager";
+        _config.addProperty("security.port-mappings.port-mapping.port", 1234);
+        _config.addProperty("security.port-mappings.port-mapping.auth-manager", testAuthManager);
+
+        _serverConfig = new ServerConfiguration(_config);
+        _serverConfig.initialise();
+        assertEquals("unexpected number of port/authmanager mappings", 1, _serverConfig.getPortAuthenticationMappings().size());
+        assertEquals("unexpected mapping for port", testAuthManager, _serverConfig.getPortAuthenticationMappings().get(1234));
+    }
+
+    public void testPortAuthenticationMappingsWithManyMapping() throws Exception
+    {
+        String testAuthManager1 = "myauthmanager1";
+        String testAuthManager2 = "myauthmanager2";
+        _config.addProperty("security.port-mappings.port-mapping(-1).port", 1234);
+        _config.addProperty("security.port-mappings.port-mapping.auth-manager", testAuthManager1);
+
+        _config.addProperty("security.port-mappings.port-mapping(-1).port", 2345);
+        _config.addProperty("security.port-mappings.port-mapping.auth-manager", testAuthManager2);
+
+        _serverConfig = new ServerConfiguration(_config);
+        _serverConfig.initialise();
+
+        assertEquals("unexpected number of port/authmanager mappings", 2, _serverConfig.getPortAuthenticationMappings().size());
+        assertEquals("unexpected mapping for port", testAuthManager1, _serverConfig.getPortAuthenticationMappings().get(1234));
+        assertEquals("unexpected mapping for port", testAuthManager2, _serverConfig.getPortAuthenticationMappings().get(2345));
+    }
+
+    public void testPortAuthenticationMappingWithMissingAuthManager() throws Exception
+    {
+        _config.addProperty("security.port-mappings.port-mapping(-1).port", 1234);
+        // no auth manager defined for port
+        _serverConfig = new ServerConfiguration(_config);
+        try
+        {
+            _serverConfig.initialise();
+            fail("Exception not thrown");
+        }
+        catch(ConfigurationException ce)
+        {
+            // PASS
+            assertEquals("Incorrect error message",
+                    "Validation error: Each port-mapping must have exactly one port and exactly one auth-manager.",
+                    ce.getMessage());
+        }
+    }
+
     /**
      * Convenience method to output required security preamble for broker config
      */
@@ -1699,7 +1768,6 @@ public class ServerConfigurationTest extends QpidTestCase
         out.write("\t\t\t\t\t</attribute>\n");
         out.write("\t\t\t\t</attributes>\n");
         out.write("\t\t\t</principal-database>\n");
-        out.write("\t\t\t<jmx-access>/dev/null</jmx-access>\n");
         out.write("\t\t</pd-auth-manager>\n");
         out.write("\t</security>\n");
     }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistryTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistryTest.java
index 213039a7fb..efb8df3a38 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistryTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/auth/manager/AuthenticationManagerRegistryTest.java
@@ -189,7 +189,7 @@ public class AuthenticationManagerRegistryTest extends TestCase
 
         AuthenticationManagerRegistry registry = new AuthenticationManagerRegistry(_serverConfiguration, _pluginManager);
 
-        AuthenticationManager authenticationManager = registry.getAuthenticationManagerFor(new InetSocketAddress(1234));
+        AuthenticationManager authenticationManager = registry.getAuthenticationManager(new InetSocketAddress(1234));
         assertEquals("TestAuthenticationManager1", authenticationManager.getMechanisms());
 
         registry.close();
@@ -204,7 +204,7 @@ public class AuthenticationManagerRegistryTest extends TestCase
 
         AuthenticationManagerRegistry registry = new AuthenticationManagerRegistry(_serverConfiguration, _pluginManager);
 
-        AuthenticationManager authenticationManager = registry.getAuthenticationManagerFor(mock(SocketAddress.class));
+        AuthenticationManager authenticationManager = registry.getAuthenticationManager(mock(SocketAddress.class));
         assertEquals("TestAuthenticationManager1", authenticationManager.getMechanisms());
 
         registry.close();
@@ -227,10 +227,10 @@ public class AuthenticationManagerRegistryTest extends TestCase
 
         AuthenticationManagerRegistry registry = new AuthenticationManagerRegistry(_serverConfiguration, _pluginManager);
 
-        AuthenticationManager authenticationManager1 = registry.getAuthenticationManagerFor(new InetSocketAddress(unmappedPortNumber));
+        AuthenticationManager authenticationManager1 = registry.getAuthenticationManager(new InetSocketAddress(unmappedPortNumber));
         assertEquals("TestAuthenticationManager1", authenticationManager1.getMechanisms());
 
-        AuthenticationManager authenticationManager2 = registry.getAuthenticationManagerFor(new InetSocketAddress(mappedPortNumber));
+        AuthenticationManager authenticationManager2 = registry.getAuthenticationManager(new InetSocketAddress(mappedPortNumber));
         assertEquals("TestAuthenticationManager2", authenticationManager2.getMechanisms());
 
         registry.close();
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
index 4bb468e823..9bd69e3889 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/util/TestApplicationRegistry.java
@@ -96,7 +96,7 @@ public class TestApplicationRegistry extends ApplicationRegistry
             }
 
             @Override
-            public AuthenticationManager getAuthenticationManagerFor(
+            public AuthenticationManager getAuthenticationManager(
                     SocketAddress address)
             {
                 return pdam;