QPID-4596: Restrict the creation of PasswordCredentialManagingAuthenticationProvider to one instance

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

3 files changed, 62 insertions, 32 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactory.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactory.java
index fbdc2e42b5..fb4b41861d 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactory.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactory.java
@@ -27,8 +27,10 @@ import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
+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.PasswordCredentialManagingAuthenticationProvider;
 import org.apache.qpid.server.plugin.AuthenticationManagerFactory;
 import org.apache.qpid.server.plugin.QpidServiceLoader;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
@@ -68,6 +70,21 @@ public class AuthenticationProviderFactory
                 AuthenticationProviderAdapter<?> authenticationProvider;
                 if (manager instanceof PrincipalDatabaseAuthenticationManager)
                 {
+                    // a temporary restriction to prevent creation of several instances
+                    // of PlainPasswordFileAuthenticationProvider/Base64MD5PasswordFileAuthenticationProvider
+                    // due to current limitation of JMX management which cannot cope
+                    // with several user management MBeans as MBean type is used as a name.
+
+                    // TODO: Remove this check after fixing of JMX management
+                    for (AuthenticationProvider provider : broker.getAuthenticationProviders())
+                    {
+                        if (provider instanceof PasswordCredentialManagingAuthenticationProvider)
+                        {
+                            throw new IllegalConfigurationException("An authentication provider which can manage users alredy exists ["
+                                    + provider.getName() + "]. Only one instance is allowed.");
+                        }
+                    }
+
                     authenticationProvider = new PrincipalDatabaseAuthenticationManagerAdapter(id, broker,
                             (PrincipalDatabaseAuthenticationManager) manager, attributes, factory.getAttributeNames());
                 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index 7e4282f4ee..73706904e5 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -531,35 +531,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         AuthenticationProvider authenticationProvider = null;
         synchronized (_authenticationProviders)
         {
-            String type = (String)attributes.get(AuthenticationProvider.TYPE);
-            if (type == null)
-            {
-                throw new IllegalConfigurationException("Authentication provider type is not specified");
-            }
-
-            // a temporary restriction to prevent creation of several instances
-            // of PlainPasswordFileAuthenticationProvider/Base64MD5PasswordFileAuthenticationProvider
-            // due to current limitation of JMX management which cannot cope
-            // with several user management MBeans as MBean type is used as a name.
-
-            // TODO: Remove this check after fixing of JMX management
-            if (type.equals(PlainPasswordFileAuthenticationManagerFactory.PROVIDER_TYPE)
-                    || type.equals(Base64MD5PasswordFileAuthenticationManagerFactory.PROVIDER_TYPE))
-            {
-
-                for (AuthenticationProvider provider : _authenticationProviders.values())
-                {
-                    String providerType = (String) provider.getAttribute(AuthenticationProvider.TYPE);
-                    if (providerType.equals(PlainPasswordFileAuthenticationManagerFactory.PROVIDER_TYPE)
-                            || providerType.equals(Base64MD5PasswordFileAuthenticationManagerFactory.PROVIDER_TYPE))
-                    {
-                        throw new IllegalConfigurationException("An authentication provider which can manage users alredy exists ["
-                                + provider.getName() + "]. Only one instance is allowed.");
-                    }
-                }
-
-            }
-
             authenticationProvider = _authenticationProviderFactory.create(UUID.randomUUID(), this, attributes);
             addAuthenticationProvider(authenticationProvider);
         }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java
index 9bf80bb87e..eb721d93a0 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/model/adapter/AuthenticationProviderFactoryTest.java
@@ -22,6 +22,7 @@ package org.apache.qpid.server.model.adapter;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.any;
 
 import java.util.Collections;
 import java.util.HashMap;
@@ -30,7 +31,7 @@ import java.util.UUID;
 
 import junit.framework.TestCase;
 
-import org.apache.qpid.server.configuration.ConfigurationEntry;
+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.PasswordCredentialManagingAuthenticationProvider;
@@ -64,9 +65,7 @@ public class AuthenticationProviderFactoryTest extends TestCase
 
         QpidServiceLoader<AuthenticationManagerFactory> authManagerFactoryServiceLoader = mock(QpidServiceLoader.class);
         AuthenticationManagerFactory authenticationManagerFactory = mock(AuthenticationManagerFactory.class);
-        ConfigurationEntry configurationEntry = mock(ConfigurationEntry.class);
 
-        when(configurationEntry.getId()).thenReturn(id);
         Broker broker = mock(Broker.class);
 
         when(authManagerFactoryServiceLoader.atLeastOneInstanceOf(AuthenticationManagerFactory.class)).thenReturn(
@@ -82,4 +81,47 @@ public class AuthenticationProviderFactoryTest extends TestCase
         return provider;
     }
 
+    @SuppressWarnings("unchecked")
+    public void testCreatePasswordCredentialManagingAuthenticationProviderFailsWhenAnotherOneAlreadyExist()
+    {
+        Broker broker = mock(Broker.class);
+        PasswordCredentialManagingAuthenticationProvider anotherProvider = mock(PasswordCredentialManagingAuthenticationProvider.class);
+        when(broker.getAuthenticationProviders()).thenReturn(Collections.<AuthenticationProvider>singleton(anotherProvider));
+
+        QpidServiceLoader<AuthenticationManagerFactory> loader = mock(QpidServiceLoader.class);
+        AuthenticationManagerFactory managerFactory = mock(AuthenticationManagerFactory.class);
+        when(managerFactory.createInstance(any(Map.class))).thenReturn(mock(PrincipalDatabaseAuthenticationManager.class));
+        when(loader.atLeastOneInstanceOf(AuthenticationManagerFactory.class)).thenReturn(Collections.singleton(managerFactory));
+
+        AuthenticationProviderFactory providerFactory = new AuthenticationProviderFactory(loader);
+        try
+        {
+            providerFactory.create(UUID.randomUUID(), broker, new HashMap<String, Object>());
+            fail("Creation of anaother PasswordCredentialManagingAuthenticationProvider should fail");
+        }
+        catch (IllegalConfigurationException e)
+        {
+            // pass
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    public void testCreateNonPasswordCredentialManagingAuthenticationProviderWhenAnotherOneAlreadyExist()
+    {
+        Broker broker = mock(Broker.class);
+        AuthenticationProvider anotherProvider = mock(AuthenticationProvider.class);
+        when(broker.getAuthenticationProviders()).thenReturn(Collections.singleton(anotherProvider));
+
+        QpidServiceLoader<AuthenticationManagerFactory> loader = mock(QpidServiceLoader.class);
+        AuthenticationManagerFactory managerFactory = mock(AuthenticationManagerFactory.class);
+        when(managerFactory.createInstance(any(Map.class))).thenReturn(mock(AuthenticationManager.class));
+        when(loader.atLeastOneInstanceOf(AuthenticationManagerFactory.class)).thenReturn(Collections.singleton(managerFactory));
+
+        AuthenticationProviderFactory providerFactory = new AuthenticationProviderFactory(loader);
+        UUID id = UUID.randomUUID();
+        AuthenticationProvider provider = providerFactory.create(id, broker, new HashMap<String, Object>());
+
+        assertNotNull("Provider is not created", provider);
+        assertEquals("Unexpected ID", id, provider.getId());
+    }
 }