QPID-4752: Make the GroupProvider a full broker-level configuration entry instead of a broker attribute.

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

16 files changed, 519 insertions, 167 deletions

diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java
index be75b4d2e9..9f6bda67df 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/BrokerRecoverer.java
@@ -39,10 +39,10 @@ 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.KeyStore;
-import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.model.adapter.AuthenticationProviderFactory;
 import org.apache.qpid.server.model.adapter.BrokerAdapter;
+import org.apache.qpid.server.model.adapter.GroupProviderFactory;
 import org.apache.qpid.server.model.adapter.PortFactory;
 import org.apache.qpid.server.stats.StatisticsGatherer;
 import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
@@ -57,11 +57,13 @@ public class BrokerRecoverer implements ConfiguredObjectRecoverer<Broker>
     private final PortFactory _portFactory;
     private final TaskExecutor _taskExecutor;
     private final BrokerOptions _brokerOptions;
+    private final GroupProviderFactory _groupProviderFactory;
 
-    public BrokerRecoverer(AuthenticationProviderFactory authenticationProviderFactory, PortFactory portFactory,
-            StatisticsGatherer statisticsGatherer, VirtualHostRegistry virtualHostRegistry, LogRecorder logRecorder,
-            RootMessageLogger rootMessageLogger, TaskExecutor taskExecutor, BrokerOptions brokerOptions)
+    public BrokerRecoverer(AuthenticationProviderFactory authenticationProviderFactory, GroupProviderFactory groupProviderFactory,
+            PortFactory portFactory, StatisticsGatherer statisticsGatherer, VirtualHostRegistry virtualHostRegistry,
+            LogRecorder logRecorder, RootMessageLogger rootMessageLogger, TaskExecutor taskExecutor, BrokerOptions brokerOptions)
     {
+        _groupProviderFactory = groupProviderFactory;
         _portFactory = portFactory;
         _authenticationProviderFactory = authenticationProviderFactory;
         _statisticsGatherer = statisticsGatherer;
@@ -77,7 +79,8 @@ public class BrokerRecoverer implements ConfiguredObjectRecoverer<Broker>
     {
         StoreConfigurationChangeListener storeChangeListener = new StoreConfigurationChangeListener(entry.getStore());
         BrokerAdapter broker = new BrokerAdapter(entry.getId(), entry.getAttributes(), _statisticsGatherer, _virtualHostRegistry,
-                _logRecorder, _rootMessageLogger, _authenticationProviderFactory, _portFactory, _taskExecutor, entry.getStore(), _brokerOptions);
+                _logRecorder, _rootMessageLogger, _authenticationProviderFactory, _groupProviderFactory, _portFactory,
+                _taskExecutor, entry.getStore(), _brokerOptions);
 
         broker.addChangeListener(storeChangeListener);
 
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/DefaultRecovererProvider.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/DefaultRecovererProvider.java
index 35209e5b41..89f949d8d8 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/DefaultRecovererProvider.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/DefaultRecovererProvider.java
@@ -34,6 +34,7 @@ import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.model.adapter.AuthenticationProviderFactory;
+import org.apache.qpid.server.model.adapter.GroupProviderFactory;
 import org.apache.qpid.server.model.adapter.PortFactory;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.plugin.AuthenticationManagerFactory;
@@ -52,7 +53,7 @@ public class DefaultRecovererProvider implements RecovererProvider
     private final RootMessageLogger _rootMessageLogger;
     private final AuthenticationProviderFactory _authenticationProviderFactory;
     private final PortFactory _portFactory;
-    private final QpidServiceLoader<GroupManagerFactory> _groupManagerServiceLoader;
+    private final GroupProviderFactory _groupProviderFactory;
     private final QpidServiceLoader<PluginFactory> _pluginFactoryServiceLoader;
     private final TaskExecutor _taskExecutor;
     private final BrokerOptions _brokerOptions;
@@ -61,12 +62,12 @@ public class DefaultRecovererProvider implements RecovererProvider
             LogRecorder logRecorder, RootMessageLogger rootMessageLogger, TaskExecutor taskExecutor, BrokerOptions brokerOptions)
     {
         _authenticationProviderFactory = new AuthenticationProviderFactory(new QpidServiceLoader<AuthenticationManagerFactory>());
+        _groupProviderFactory = new GroupProviderFactory(new QpidServiceLoader<GroupManagerFactory>());
         _portFactory = new PortFactory();
         _brokerStatisticsGatherer = brokerStatisticsGatherer;
         _virtualHostRegistry = virtualHostRegistry;
         _logRecorder = logRecorder;
         _rootMessageLogger = rootMessageLogger;
-        _groupManagerServiceLoader = new QpidServiceLoader<GroupManagerFactory>();
         _pluginFactoryServiceLoader = new QpidServiceLoader<PluginFactory>();
         _taskExecutor = taskExecutor;
         _brokerOptions = brokerOptions;
@@ -77,7 +78,7 @@ public class DefaultRecovererProvider implements RecovererProvider
     {
         if (Broker.class.getSimpleName().equals(type))
         {
-            return new BrokerRecoverer(_authenticationProviderFactory, _portFactory, _brokerStatisticsGatherer, _virtualHostRegistry,
+            return new BrokerRecoverer(_authenticationProviderFactory, _groupProviderFactory, _portFactory, _brokerStatisticsGatherer, _virtualHostRegistry,
                     _logRecorder, _rootMessageLogger, _taskExecutor, _brokerOptions);
         }
         else if(VirtualHost.class.getSimpleName().equals(type))
@@ -94,7 +95,7 @@ public class DefaultRecovererProvider implements RecovererProvider
         }
         else if(GroupProvider.class.getSimpleName().equals(type))
         {
-            return new GroupProviderRecoverer(_groupManagerServiceLoader);
+            return new GroupProviderRecoverer(_groupProviderFactory);
         }
         else if(KeyStore.class.getSimpleName().equals(type))
         {
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/GroupProviderRecoverer.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/GroupProviderRecoverer.java
index 275a0c736c..00f23b3c1c 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/GroupProviderRecoverer.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/configuration/startup/GroupProviderRecoverer.java
@@ -24,24 +24,20 @@ import java.util.Map;
 
 import org.apache.qpid.server.configuration.ConfigurationEntry;
 import org.apache.qpid.server.configuration.ConfiguredObjectRecoverer;
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.configuration.RecovererProvider;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.GroupProvider;
-import org.apache.qpid.server.model.adapter.GroupProviderAdapter;
-import org.apache.qpid.server.plugin.GroupManagerFactory;
-import org.apache.qpid.server.plugin.QpidServiceLoader;
-import org.apache.qpid.server.security.group.GroupManager;
+import org.apache.qpid.server.model.adapter.GroupProviderFactory;
 
 public class GroupProviderRecoverer implements ConfiguredObjectRecoverer<GroupProvider>
 {
-    private QpidServiceLoader<GroupManagerFactory> _groupManagerServiceLoader;
+    private GroupProviderFactory _groupProviderFactory;
 
-    public GroupProviderRecoverer(QpidServiceLoader<GroupManagerFactory> groupManagerServiceLoader)
+    public GroupProviderRecoverer(GroupProviderFactory groupProviderFactory)
     {
         super();
-        _groupManagerServiceLoader = groupManagerServiceLoader;
+        _groupProviderFactory = groupProviderFactory;
     }
 
     @Override
@@ -49,26 +45,9 @@ public class GroupProviderRecoverer implements ConfiguredObjectRecoverer<GroupPr
     {
         Broker broker = RecovererHelper.verifyOnlyBrokerIsParent(parents);
         Map<String, Object> attributes = configurationEntry.getAttributes();
-        GroupManager groupManager = createGroupManager(attributes);
-        if (groupManager == null)
-        {
-            throw new IllegalConfigurationException("Cannot create GroupManager from attributes : " + attributes);
-        }
-        GroupProviderAdapter groupProviderAdapter = new GroupProviderAdapter(configurationEntry.getId(), groupManager, broker);
-        return groupProviderAdapter;
-    }
 
-    private GroupManager createGroupManager(Map<String, Object> attributes)
-    {
-        for(GroupManagerFactory factory : _groupManagerServiceLoader.instancesOf(GroupManagerFactory.class))
-        {
-            GroupManager groupManager = factory.createInstance(attributes);
-            if (groupManager != null)
-            {
-               return groupManager;
-            }
-        }
-        return null;
-    }
+        GroupProvider groupProvider = _groupProviderFactory.recover(configurationEntry.getId(), broker, attributes);
 
+        return groupProvider;
+    }
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Broker.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Broker.java
index 712b53f627..44adf3414d 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Broker.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/Broker.java
@@ -86,12 +86,6 @@ public interface Broker extends ConfiguredObject
      */
     String ACL_FILE = "aclFile";
 
-    /*
-     * A temporary attributes to set the broker group file.
-     * TODO: Remove them after adding a full support to configure authorization providers via management layers.
-     */
-    String GROUP_FILE = "groupFile";
-
     // Attributes
     Collection<String> AVAILABLE_ATTRIBUTES =
             Collections.unmodifiableList(
@@ -134,7 +128,6 @@ public interface Broker extends ConfiguredObject
                               VIRTUALHOST_STORE_TRANSACTION_IDLE_TIMEOUT_WARN,
                               VIRTUALHOST_STORE_TRANSACTION_OPEN_TIMEOUT_CLOSE,
                               VIRTUALHOST_STORE_TRANSACTION_OPEN_TIMEOUT_WARN,
-                              GROUP_FILE,
                               ACL_FILE
                               ));
 
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 49af66126c..17a8f19cd3 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
@@ -63,8 +63,6 @@ import org.apache.qpid.server.security.SubjectCreator;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.auth.manager.AuthenticationManager;
 import org.apache.qpid.server.security.auth.manager.SimpleAuthenticationManager;
-import org.apache.qpid.server.security.group.FileGroupManager;
-import org.apache.qpid.server.security.group.GroupManager;
 import org.apache.qpid.server.stats.StatisticsGatherer;
 import org.apache.qpid.server.store.MessageStoreCreator;
 import org.apache.qpid.server.util.MapValueConverter;
@@ -97,7 +95,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         put(NAME, String.class);
         put(DEFAULT_VIRTUAL_HOST, String.class);
 
-        put(GROUP_FILE, String.class);
         put(VIRTUALHOST_STORE_TRANSACTION_IDLE_TIMEOUT_CLOSE, Long.class);
         put(VIRTUALHOST_STORE_TRANSACTION_IDLE_TIMEOUT_WARN, Long.class);
         put(VIRTUALHOST_STORE_TRANSACTION_OPEN_TIMEOUT_CLOSE, Long.class);
@@ -123,7 +120,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
     public static final long DEFAULT_STORE_TRANSACTION_IDLE_TIMEOUT_WARN = 0l;
     public static final long DEFAULT_STORE_TRANSACTION_OPEN_TIMEOUT_CLOSE = 0l;
     public static final long DEFAULT_STORE_TRANSACTION_OPEN_TIMEOUT_WARN = 0l;
-    private static final String DEFAULT_GROUP_PROVIDER_NAME = "defaultGroupProvider";
 
     @SuppressWarnings("serial")
     private static final Map<String, Object> DEFAULTS = Collections.unmodifiableMap(new HashMap<String, Object>(){{
@@ -170,6 +166,7 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
     private final Map<String, KeyStore> _keyStores = new HashMap<String, KeyStore>();
     private final Map<String, TrustStore> _trustStores = new HashMap<String, TrustStore>();
 
+    private final GroupProviderFactory _groupProviderFactory;
     private final AuthenticationProviderFactory _authenticationProviderFactory;
 
     private final PortFactory _portFactory;
@@ -183,7 +180,8 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
 
     public BrokerAdapter(UUID id, Map<String, Object> attributes, StatisticsGatherer statisticsGatherer, VirtualHostRegistry virtualHostRegistry,
             LogRecorder logRecorder, RootMessageLogger rootMessageLogger, AuthenticationProviderFactory authenticationProviderFactory,
-            PortFactory portFactory, TaskExecutor taskExecutor, ConfigurationEntryStore brokerStore, BrokerOptions brokerOptions)
+            GroupProviderFactory groupProviderFactory, PortFactory portFactory, TaskExecutor taskExecutor, ConfigurationEntryStore brokerStore,
+            BrokerOptions brokerOptions)
     {
         super(id, DEFAULTS,  MapValueConverter.convert(attributes, ATTRIBUTE_TYPES), taskExecutor);
         _statisticsGatherer = statisticsGatherer;
@@ -192,14 +190,13 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         _rootMessageLogger = rootMessageLogger;
         _statistics = new StatisticsAdapter(statisticsGatherer);
         _authenticationProviderFactory = authenticationProviderFactory;
+        _groupProviderFactory = groupProviderFactory;
         _portFactory = portFactory;
         _securityManager = new SecurityManager((String)getAttribute(ACL_FILE));
         addChangeListener(_securityManager);
-        createBrokerChildrenFromAttributes();
         _supportedStoreTypes = new MessageStoreCreator().getStoreTypes();
         _brokerStore = brokerStore;
         _brokerOptions = brokerOptions;
-        createBrokerChildrenFromAttributes();
         if (_brokerOptions.isManagementMode())
         {
             AuthenticationManager authManager = new SimpleAuthenticationManager(BrokerOptions.MANAGEMENT_MODE_USER_NAME, _brokerOptions.getManagementModePassword());
@@ -209,32 +206,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         }
     }
 
-    /*
-     * A temporary method to create broker children that can be only configured via broker attributes
-     */
-    private void createBrokerChildrenFromAttributes()
-    {
-        createGroupProvider();
-
-    }
-
-    private void createGroupProvider()
-    {
-        String groupFile = (String) getAttribute(GROUP_FILE);
-        if (groupFile != null)
-        {
-            GroupManager groupManager = new FileGroupManager(groupFile);
-            UUID groupProviderId = UUIDGenerator.generateBrokerChildUUID(GroupProvider.class.getSimpleName(),
-                    DEFAULT_GROUP_PROVIDER_NAME);
-            GroupProviderAdapter groupProviderAdapter = new GroupProviderAdapter(groupProviderId, groupManager, this);
-            _groupProviders.put(DEFAULT_GROUP_PROVIDER_NAME, groupProviderAdapter);
-        }
-        else
-        {
-            _groupProviders.remove(DEFAULT_GROUP_PROVIDER_NAME);
-        }
-    }
-
     public Collection<VirtualHost> getVirtualHosts()
     {
         synchronized(_vhostAdapters)
@@ -462,6 +433,10 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         {
             return (C) createTrustStore(attributes);
         }
+        else if(childClass == GroupProvider.class)
+        {
+            return (C) createGroupProvider(attributes);
+        }
         else
         {
             throw new IllegalArgumentException("Cannot create child of class " + childClass.getSimpleName());
@@ -535,6 +510,14 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         authenticationProvider.addChangeListener(this);
     }
 
+    private GroupProvider createGroupProvider(Map<String, Object> attributes)
+    {
+        GroupProvider groupProvider = _groupProviderFactory.create(UUID.randomUUID(), this, attributes);
+        groupProvider.setDesiredState(State.INITIALISING, State.ACTIVE);
+        addGroupProvider(groupProvider);
+        return groupProvider;
+    }
+
     private void addGroupProvider(GroupProvider groupProvider)
     {
         synchronized (_groupProviders)
@@ -549,9 +532,20 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         groupProvider.addChangeListener(this);
     }
 
-    private boolean deleteGroupProvider(GroupProvider object)
+    private boolean deleteGroupProvider(GroupProvider groupProvider)
     {
-        throw new UnsupportedOperationException("Not implemented yet!");
+        GroupProvider removedGroupProvider = null;
+        synchronized (_groupProviders)
+        {
+            removedGroupProvider = _groupProviders.remove(groupProvider.getName());
+        }
+
+        if(removedGroupProvider != null)
+        {
+            removedGroupProvider.removeChangeListener(this);
+        }
+
+        return removedGroupProvider != null;
     }
 
     private KeyStore createKeyStore(Map<String, Object> attributes)
@@ -724,6 +718,12 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         {
             removedPort = _portAdapters.remove(portAdapter.getPort());
         }
+
+        if (removedPort != null)
+        {
+            removedPort.removeChangeListener(this);
+        }
+
         return removedPort != null;
     }
 
@@ -734,6 +734,12 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
         {
             removedAuthenticationProvider = _authenticationProviders.remove(authenticationProvider.getId());
         }
+
+        if(removedAuthenticationProvider != null)
+        {
+            removedAuthenticationProvider.removeChangeListener(this);
+        }
+
         return removedAuthenticationProvider != null;
     }
 
@@ -1030,11 +1036,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
                 Object expected = getAttribute(name);
                 if (changeAttribute(name, expected, desired))
                 {
-                    if (GROUP_FILE.equals(name))
-                    {
-                        createGroupProvider();
-                    }
-
                     attributeSet(name, expected, desired);
                 }
             }
@@ -1049,12 +1050,6 @@ public class BrokerAdapter extends AbstractAdapter implements Broker, Configurat
             // create a security manager to validate the ACL specified in file
             new SecurityManager(aclFile);
         }
-        String groupFile = (String) convertedAttributes.get(GROUP_FILE);
-        if (groupFile != null)
-        {
-            // create a group manager to validate the groups specified in file
-            new FileGroupManager(groupFile);
-        }
 
         String defaultVirtualHost = (String) convertedAttributes.get(DEFAULT_VIRTUAL_HOST);
         if (defaultVirtualHost != null)
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
index 9ad58f9670..a0e5bdb0e8 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderAdapter.java
@@ -24,6 +24,7 @@ import java.security.Principal;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
@@ -48,9 +49,11 @@ public class GroupProviderAdapter extends AbstractAdapter implements
 {
     private final GroupManager _groupManager;
     private final Broker _broker;
-    public GroupProviderAdapter(UUID id, GroupManager groupManager, Broker broker)
+    private Collection<String> _supportedAttributes;
+
+    public GroupProviderAdapter(UUID id, Broker broker, GroupManager groupManager, Map<String, Object> attributes, Collection<String> attributeNames)
     {
-        super(id, broker.getTaskExecutor());
+        super(id, null, null, broker.getTaskExecutor());
 
         if (groupManager == null)
         {
@@ -58,13 +61,37 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         }
         _groupManager = groupManager;
         _broker = broker;
+        _supportedAttributes = createSupportedAttributes(attributeNames);
        addParent(Broker.class, broker);
+
+       // set attributes now after all attribute names are known
+       if (attributes != null)
+       {
+           for (String name : _supportedAttributes)
+           {
+               if (attributes.containsKey(name))
+               {
+                   changeAttribute(name, null, attributes.get(name));
+               }
+           }
+       }
+    }
+
+    protected Collection<String> createSupportedAttributes(Collection<String> factoryAttributes)
+    {
+        List<String> attributesNames = new ArrayList<String>(AVAILABLE_ATTRIBUTES);
+        if (factoryAttributes != null)
+        {
+            attributesNames.addAll(factoryAttributes);
+        }
+
+        return Collections.unmodifiableCollection(attributesNames);
     }
 
     @Override
     public String getName()
     {
-        return _groupManager.getClass().getSimpleName();
+        return (String)getAttribute(NAME);
     }
 
     @Override
@@ -129,17 +156,13 @@ public class GroupProviderAdapter extends AbstractAdapter implements
     @Override
     public Collection<String> getAttributeNames()
     {
-        return GroupProvider.AVAILABLE_ATTRIBUTES;
+        return _supportedAttributes;
     }
 
     @Override
     public Object getAttribute(String name)
     {
-        if (TYPE.equals(name))
-        {
-            return getName();
-        }
-        else if (CREATED.equals(name))
+        if (CREATED.equals(name))
         {
             // TODO
         }
@@ -155,10 +178,6 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         {
             return LifetimePolicy.PERMANENT;
         }
-        else if (NAME.equals(name))
-        {
-            return getName();
-        }
         else if (STATE.equals(name))
         {
             return State.ACTIVE; // TODO
@@ -220,11 +239,91 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         }
     }
 
+    public GroupManager getGroupManager()
+    {
+        return _groupManager;
+    }
+
     private SecurityManager getSecurityManager()
     {
         return _broker.getSecurityManager();
     }
 
+    @Override
+    protected boolean setState(State currentState, State desiredState)
+    {
+        if (desiredState == State.ACTIVE)
+        {
+            _groupManager.open();
+            return true;
+        }
+        else if (desiredState == State.STOPPED)
+        {
+            _groupManager.close();
+            return true;
+        }
+        else if (desiredState == State.DELETED)
+        {
+            _groupManager.close();
+            _groupManager.onDelete();
+            return true;
+        }
+        return false;
+    }
+
+    public Set<Principal> getGroupPrincipalsForUser(String username)
+    {
+        return _groupManager.getGroupPrincipalsForUser(username);
+    }
+
+    @Override
+    protected void childAdded(ConfiguredObject child)
+    {
+        // no-op, prevent storing groups in the broker store
+    }
+
+    @Override
+    protected void childRemoved(ConfiguredObject child)
+    {
+        // no-op, as per above, groups are not in the store
+    }
+
+    @Override
+    protected void authoriseSetDesiredState(State currentState, State desiredState) throws AccessControlException
+    {
+        if(desiredState == State.DELETED)
+        {
+            if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), GroupProvider.class, Operation.DELETE))
+            {
+                throw new AccessControlException("Deletion of groups provider is denied");
+            }
+        }
+    }
+
+    @Override
+    protected void authoriseSetAttribute(String name, Object expected, Object desired) throws AccessControlException
+    {
+        if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), GroupProvider.class, Operation.UPDATE))
+        {
+            throw new AccessControlException("Setting of group provider attributes is denied");
+        }
+    }
+
+    @Override
+    protected void authoriseSetAttributes(Map<String, Object> attributes) throws AccessControlException
+    {
+        if (!_broker.getSecurityManager().authoriseConfiguringBroker(getName(), GroupProvider.class, Operation.UPDATE))
+        {
+            throw new AccessControlException("Setting of group provider attributes is denied");
+        }
+    }
+
+    @Override
+    protected void changeAttributes(Map<String, Object> attributes)
+    {
+        throw new UnsupportedOperationException("Changing attributes on group providers is not supported.");
+    }
+
     private class GroupAdapter extends AbstractAdapter implements Group
     {
         private final String _group;
@@ -526,23 +625,5 @@ public class GroupProviderAdapter extends AbstractAdapter implements
         }
     }
 
-    @Override
-    protected boolean setState(State currentState, State desiredState)
-    {
-        if (desiredState == State.ACTIVE)
-        {
-            return true;
-        }
-        else if (desiredState == State.STOPPED)
-        {
-            return true;
-        }
-        // TODO: DELETE state is ignored for now
-        return false;
-    }
 
-    public Set<Principal> getGroupPrincipalsForUser(String username)
-    {
-        return _groupManager.getGroupPrincipalsForUser(username);
-    }
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderFactory.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderFactory.java
new file mode 100644
index 0000000000..1d3ccd81b3
--- /dev/null
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/model/adapter/GroupProviderFactory.java
@@ -0,0 +1,118 @@
+/*
+ *
+ * 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.model.adapter;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.qpid.server.configuration.IllegalConfigurationException;
+import org.apache.qpid.server.model.Broker;
+import org.apache.qpid.server.model.GroupProvider;
+import org.apache.qpid.server.plugin.GroupManagerFactory;
+import org.apache.qpid.server.plugin.QpidServiceLoader;
+import org.apache.qpid.server.security.group.GroupManager;
+
+public class GroupProviderFactory
+{
+    private final Map<String, GroupManagerFactory> _factories;
+    private Collection<String> _supportedGroupProviders;
+
+    public GroupProviderFactory(QpidServiceLoader<GroupManagerFactory> groupManagerFactoryServiceLoader)
+    {
+        Iterable<GroupManagerFactory> factories = groupManagerFactoryServiceLoader.instancesOf(GroupManagerFactory.class);
+
+        Map<String, GroupManagerFactory> registeredGroupProviderFactories = new HashMap<String, GroupManagerFactory>();
+        for (GroupManagerFactory factory : factories)
+        {
+            GroupManagerFactory existingFactory = registeredGroupProviderFactories.put(factory.getType(), factory);
+            if (existingFactory != null)
+            {
+                throw new IllegalConfigurationException("Group provider factory of the same type '" + factory.getType()
+                        + "' is already registered using class '" + existingFactory.getClass().getName()
+                        + "', can not register class '" + factory.getClass().getName() + "'");
+            }
+        }
+        _factories = registeredGroupProviderFactories;
+        _supportedGroupProviders = Collections.unmodifiableCollection(registeredGroupProviderFactories.keySet());
+    }
+
+    /**
+     * Creates {@link GroupProvider} for given ID, {@link Broker} and attributes.
+     * <p>
+     * The configured {@link GroupManagerFactory}'s are used to try to create the {@link GroupProvider}. The first non-null
+     * instance is returned. The factories are used in non-deterministic order.
+     */
+    public GroupProvider create(UUID id, Broker broker, Map<String, Object> attributes)
+    {
+        GroupProviderAdapter authenticationProvider = createGroupProvider(id, broker, attributes);
+        authenticationProvider.getGroupManager().onCreate();
+        return authenticationProvider;
+    }
+
+    /**
+     * Recovers {@link GroupProvider} with given ID, {@link Broker} and attributes.
+     * <p>
+     * The configured {@link GroupManagerFactory}'s are used to try to create the {@link GroupProvider}. The first non-null
+     * instance is returned. The factories are used in non-deterministic order.
+     */
+    public GroupProvider recover(UUID id, Broker broker, Map<String, Object> attributes)
+    {
+        return createGroupProvider(id, broker, attributes);
+    }
+
+    public Collection<String> getSupportedGroupProviders()
+    {
+        return _supportedGroupProviders;
+    }
+
+    private GroupProviderAdapter createGroupProvider(UUID id, Broker broker, Map<String, Object> attributes)
+    {
+        for (GroupManagerFactory factory : _factories.values())
+        {
+            GroupManager manager = factory.createInstance(attributes);
+            if (manager != null)
+            {
+                verifyGroupManager(manager, broker);
+                return new GroupProviderAdapter(id, broker, manager, attributes,factory.getAttributeNames());
+            }
+        }
+        throw new IllegalConfigurationException("No group provider factory found for configuration attributes " + attributes);
+    }
+
+    private void verifyGroupManager(GroupManager manager, Broker broker)
+    {
+        Collection<GroupProvider> groupProviders = broker.getGroupProviders();
+        for (GroupProvider groupProvider : groupProviders)
+        {
+            if (groupProvider instanceof GroupProviderAdapter)
+            {
+                GroupManager providerManager = ((GroupProviderAdapter) groupProvider).getGroupManager();
+                if (manager.equals(providerManager))
+                {
+                    throw new IllegalConfigurationException("A group provider with the same settings already exists");
+                }
+            }
+        }
+    }
+}
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/plugin/GroupManagerFactory.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/plugin/GroupManagerFactory.java
index 5d80ca24fd..2ab9701e2e 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/plugin/GroupManagerFactory.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/plugin/GroupManagerFactory.java
@@ -18,11 +18,34 @@
  */
 package org.apache.qpid.server.plugin;
 
+import java.util.Collection;
 import java.util.Map;
 
+import org.apache.qpid.server.model.GroupProvider;
 import org.apache.qpid.server.security.group.GroupManager;
 
 public interface GroupManagerFactory
 {
+    public static final String ATTRIBUTE_TYPE = GroupProvider.TYPE;
+
     GroupManager createInstance(Map<String, Object> attributes);
+
+    /**
+     * Returns the authentication provider type
+     * @return authentication provider type
+     */
+    String getType();
+
+    /**
+     * Get the names of attributes the group manager which can be passed into {@link #createInstance(Map)} to create the
+     * group manager
+     *
+     * @return the collection of attribute names
+     */
+    Collection<String> getAttributeNames();
+
+    /**
+     * @return returns human readable descriptions for the attributes
+     */
+    Map<String, String> getAttributeDescriptions();
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManager.java
index 8295f28f9e..e11a4f83db 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManager.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManager.java
@@ -19,6 +19,7 @@
  */
 package org.apache.qpid.server.security.group;
 
+import java.io.File;
 import java.io.IOException;
 import java.security.Principal;
 import java.util.Collections;
@@ -34,32 +35,26 @@ import org.apache.qpid.server.security.auth.UsernamePrincipal;
  * This plugin is configured in the following manner:
  * </p>
  * <pre>
- * &lt;file-group-manager&gt;
- *    &lt;attributes&gt;
- *       &lt;attribute&gt;
- *            &lt;name>groupFile&lt;/name&gt;
- *            &lt;value>${conf}/groups&lt;/value&gt;
- *        &lt;/attribute&gt;
- *    &lt;/attributes&gt;
- * &lt;/file-group-manager&gt;
+ * "groupproviders":[
+ * ...
+ * {
+ *  "name" : "...",
+ *  "type" : "GroupFile",
+ *  "path" : "path/to/file/with/groups",
+ * }
+ * ...
+ * ]
  * </pre>
  */
 public class FileGroupManager implements GroupManager
 {
     private final FileGroupDatabase _groupDatabase;
-
+    private final String _groupFile;
 
     public FileGroupManager(String groupFile)
     {
+        _groupFile = groupFile;
         _groupDatabase = new FileGroupDatabase();
-        try
-        {
-            _groupDatabase.setGroupFile(groupFile);
-        }
-        catch (IOException e)
-        {
-            throw new IllegalConfigurationException("Unable to set group file " + groupFile, e);
-        }
     }
 
     @Override
@@ -144,4 +139,101 @@ public class FileGroupManager implements GroupManager
 
     }
 
+    @Override
+    public void onDelete()
+    {
+        File file = new File(_groupFile);
+        if (file.exists())
+        {
+            if (!file.delete())
+            {
+                throw new IllegalConfigurationException("Cannot delete group file");
+            }
+        }
+    }
+
+    @Override
+    public void onCreate()
+    {
+        File file = new File(_groupFile);
+        if (!file.exists())
+        {
+            File parent = file.getParentFile();
+            if (!parent.exists())
+            {
+                parent.mkdirs();
+            }
+            if (parent.exists())
+            {
+                try
+                {
+                    file.createNewFile();
+                }
+                catch (IOException e)
+                {
+                    throw new IllegalConfigurationException("Cannot create group file");
+                }
+            }
+            else
+            {
+                throw new IllegalConfigurationException("Cannot create group file");
+            }
+        }
+    }
+
+    @Override
+    public void open()
+    {
+        try
+        {
+            _groupDatabase.setGroupFile(_groupFile);
+        }
+        catch (IOException e)
+        {
+            throw new IllegalConfigurationException("Unable to set group file " + _groupFile, e);
+        }
+    }
+
+    @Override
+    public void close()
+    {
+        // no-op
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return ((_groupFile == null) ? 0 : _groupFile.hashCode());
+    }
+
+    @Override
+    public boolean equals(Object obj)
+    {
+        if (this == obj)
+        {
+            return true;
+        }
+        if (obj == null)
+        {
+            return false;
+        }
+        if (getClass() != obj.getClass())
+        {
+            return false;
+        }
+        FileGroupManager other = (FileGroupManager) obj;
+        if (_groupFile == null)
+        {
+            if (other._groupFile != null)
+            {
+                return false;
+            }
+            else
+            {
+                return true;
+            }
+        }
+        return _groupFile.equals(other._groupFile);
+    }
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManagerFactory.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManagerFactory.java
index 5c4730a9c8..50f08623cd 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManagerFactory.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/FileGroupManagerFactory.java
@@ -20,32 +20,60 @@ package org.apache.qpid.server.security.group;
 
 import static org.apache.qpid.server.util.MapValueConverter.getStringAttribute;
 
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.Map;
 
-import org.apache.commons.lang.StringUtils;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
-import org.apache.qpid.server.model.GroupProvider;
 import org.apache.qpid.server.plugin.GroupManagerFactory;
+import org.apache.qpid.server.util.ResourceBundleLoader;
 
 public class FileGroupManagerFactory implements GroupManagerFactory
 {
-    static final String FILE_GROUP_MANAGER_TYPE = "file-group-manager";
-    static final String FILE = "file";
+    public static final String RESOURCE_BUNDLE = "org.apache.qpid.server.security.group.FileGroupProviderAttributeDescriptions";
+
+    public static final String GROUP_FILE_PROVIDER_TYPE = "GroupFile";
+    public static final String PATH = "path";
+
+    public static final Collection<String> ATTRIBUTES = Collections.<String> unmodifiableList(Arrays.asList(
+            ATTRIBUTE_TYPE,
+            PATH
+            ));
 
     @Override
     public GroupManager createInstance(Map<String, Object> attributes)
     {
-        if(!FILE_GROUP_MANAGER_TYPE.equals(getStringAttribute(GroupProvider.TYPE, attributes, null)))
+        if(attributes == null || !GROUP_FILE_PROVIDER_TYPE.equals(attributes.get(ATTRIBUTE_TYPE)))
         {
             return null;
         }
 
-        String groupFile =  getStringAttribute(FILE, attributes, null);
-        if (StringUtils.isBlank(groupFile))
+        String groupFile =  getStringAttribute(PATH, attributes, null);
+        if (groupFile == null || "".equals(groupFile.trim()))
         {
             throw new IllegalConfigurationException("Path to file containing groups is not specified!");
         }
+
         return new FileGroupManager(groupFile);
     }
 
+    @Override
+    public String getType()
+    {
+        return GROUP_FILE_PROVIDER_TYPE;
+    }
+
+    @Override
+    public Collection<String> getAttributeNames()
+    {
+        return ATTRIBUTES;
+    }
+
+    @Override
+    public Map<String, String> getAttributeDescriptions()
+    {
+        return ResourceBundleLoader.getResources(RESOURCE_BUNDLE);
+    }
+
 }
diff --git a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupManager.java b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupManager.java
index 6d2df86919..3fd6dd2800 100644
--- a/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupManager.java
+++ b/qpid/java/broker/src/main/java/org/apache/qpid/server/security/group/GroupManager.java
@@ -37,4 +37,12 @@ public interface GroupManager
     void addUserToGroup(String user, String group);
 
     void removeUserFromGroup(String user, String group);
+
+    void open();
+
+    void close();
+
+    void onDelete();
+
+    void onCreate();
 }
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java
index 5a8580fd26..5c3f062cae 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/BrokerRecovererTest.java
@@ -49,6 +49,7 @@ import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.model.adapter.AuthenticationProviderFactory;
+import org.apache.qpid.server.model.adapter.GroupProviderFactory;
 import org.apache.qpid.server.model.adapter.PortFactory;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.stats.StatisticsGatherer;
@@ -70,7 +71,7 @@ public class BrokerRecovererTest extends TestCase
     {
         super.setUp();
 
-        _brokerRecoverer = new BrokerRecoverer(mock(AuthenticationProviderFactory.class), mock(PortFactory.class), mock(StatisticsGatherer.class),
+        _brokerRecoverer = new BrokerRecoverer(mock(AuthenticationProviderFactory.class), mock(GroupProviderFactory.class), mock(PortFactory.class), mock(StatisticsGatherer.class),
                 mock(VirtualHostRegistry.class), mock(LogRecorder.class), mock(RootMessageLogger.class), mock(TaskExecutor.class), mock(BrokerOptions.class));
         when(_brokerEntry.getId()).thenReturn(_brokerId);
         when(_brokerEntry.getChildren()).thenReturn(_brokerEntryChildren);
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/GroupProviderRecovererTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/GroupProviderRecovererTest.java
index 6713574e4b..d6f03a9758 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/GroupProviderRecovererTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/configuration/startup/GroupProviderRecovererTest.java
@@ -30,6 +30,7 @@ import org.apache.qpid.server.configuration.ConfigurationEntry;
 import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.GroupProvider;
+import org.apache.qpid.server.model.adapter.GroupProviderFactory;
 import org.apache.qpid.server.plugin.GroupManagerFactory;
 import org.apache.qpid.server.plugin.QpidServiceLoader;
 import org.apache.qpid.server.security.group.GroupManager;
@@ -46,6 +47,7 @@ public class GroupProviderRecovererTest extends TestCase
     private QpidServiceLoader<GroupManagerFactory> _groupManagerServiceLoader;
     private Broker _broker;
     private ConfigurationEntry _configurationEntry;
+    private GroupProviderFactory _groupProviderFactory;
 
     @SuppressWarnings("unchecked")
     protected void setUp() throws Exception
@@ -58,6 +60,7 @@ public class GroupProviderRecovererTest extends TestCase
 
         _groupManagerServiceLoader = mock(QpidServiceLoader.class);
         when(_groupManagerServiceLoader.instancesOf(GroupManagerFactory.class)).thenReturn(Collections.singletonList(_factory ));
+        _groupProviderFactory = new GroupProviderFactory(_groupManagerServiceLoader);
 
         _broker = mock(Broker.class);
 
@@ -70,8 +73,9 @@ public class GroupProviderRecovererTest extends TestCase
     {
         GroupManager groupManager = mock(GroupManager.class);
         String name = groupManager.getClass().getSimpleName();
+        _attributes.put(GroupProvider.NAME, name);
         when(_factory.createInstance(_attributes)).thenReturn(groupManager);
-        GroupProviderRecoverer groupProviderRecoverer = new GroupProviderRecoverer(_groupManagerServiceLoader);
+        GroupProviderRecoverer groupProviderRecoverer = new GroupProviderRecoverer(_groupProviderFactory);
         GroupProvider groupProvider = groupProviderRecoverer.create(null, _configurationEntry, _broker);
         assertNotNull("Null group provider", groupProvider);
         assertEquals("Unexpected name", name, groupProvider.getName());
@@ -82,7 +86,7 @@ public class GroupProviderRecovererTest extends TestCase
     {
         when(_factory.createInstance(_attributes)).thenReturn(null);
 
-        GroupProviderRecoverer groupProviderRecoverer = new GroupProviderRecoverer(_groupManagerServiceLoader);
+        GroupProviderRecoverer groupProviderRecoverer = new GroupProviderRecoverer(_groupProviderFactory);
         try
         {
             groupProviderRecoverer.create(null, _configurationEntry, _broker);
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerFactoryTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerFactoryTest.java
index 934c0082ea..90308d316b 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerFactoryTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerFactoryTest.java
@@ -35,8 +35,8 @@ public class FileGroupManagerFactoryTest extends TestCase
 
     public void testInstanceCreated() throws Exception
     {
-        _configuration.put(GroupProvider.TYPE, FileGroupManagerFactory.FILE_GROUP_MANAGER_TYPE);
-        _configuration.put(FileGroupManagerFactory.FILE, _emptyButValidGroupFile);
+        _configuration.put(GroupProvider.TYPE, FileGroupManagerFactory.GROUP_FILE_PROVIDER_TYPE);
+        _configuration.put(FileGroupManagerFactory.PATH, _emptyButValidGroupFile);
 
         GroupManager manager = _factory.createInstance(_configuration);
         assertNotNull(manager);
@@ -60,8 +60,8 @@ public class FileGroupManagerFactoryTest extends TestCase
 
     public void testRejectsConfigThatIsMissingAttributeValue() throws Exception
     {
-        _configuration.put(GroupProvider.TYPE, FileGroupManagerFactory.FILE_GROUP_MANAGER_TYPE);
-        _configuration.put(FileGroupManagerFactory.FILE, null);
+        _configuration.put(GroupProvider.TYPE, FileGroupManagerFactory.GROUP_FILE_PROVIDER_TYPE);
+        _configuration.put(FileGroupManagerFactory.PATH, null);
 
         try
         {
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerTest.java b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerTest.java
index b83d25b206..152703d548 100644
--- a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerTest.java
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupManagerTest.java
@@ -20,13 +20,11 @@
 package org.apache.qpid.server.security.group;
 
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.security.Principal;
 import java.util.Properties;
 import java.util.Set;
 
-import org.apache.qpid.server.configuration.IllegalConfigurationException;
 import org.apache.qpid.server.security.auth.UsernamePrincipal;
 import org.apache.qpid.test.utils.QpidTestCase;
 
@@ -62,17 +60,26 @@ public class FileGroupManagerTest extends QpidTestCase
 
     public void testNonExistentGroupFile() throws Exception
     {
-        final String filePath = "/does.not.exist/";
-
+        final String filePath = TMP_FOLDER + File.separator + "non.existing";
+        File file = new File(filePath);
+        if (file.exists())
+        {
+            file.delete();
+        }
+        assertFalse("File should not exist", file.exists());
         try
         {
             _manager = new FileGroupManager(filePath);
-            fail("expected exception was not thrown");
+            assertFalse("File should be created", file.exists());
+            _manager.onCreate();
+            assertTrue("File should be created", file.exists());
+            _manager.open();
+            Set<Principal> groups = _manager.getGroupPrincipals();
+            assertTrue("No group should exist", groups.isEmpty());
         }
-        catch(IllegalConfigurationException ce)
+        finally
         {
-            assertNotNull(ce.getCause());
-            assertTrue(ce.getCause() instanceof FileNotFoundException);
+            file.delete();
         }
     }
 
@@ -80,7 +87,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile();
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getGroupPrincipalsForUser("user1");
         assertEquals(1, principals.size());
         assertTrue(principals.contains(new GroupPrincipal("myGroup")));
@@ -90,7 +97,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile();
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getUserPrincipalsForGroup("myGroup");
         assertEquals(1, principals.size());
         assertTrue(principals.contains(new UsernamePrincipal("user1")));
@@ -100,7 +107,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile(MY_GROUP, MYGROUP_USERS, MY_GROUP2, MYGROUP_USERS);
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getGroupPrincipals();
         assertEquals(2, principals.size());
         assertTrue(principals.contains(new GroupPrincipal("myGroup")));
@@ -111,7 +118,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile();
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getGroupPrincipals();
         assertEquals(1, principals.size());
 
@@ -126,7 +133,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile(MY_GROUP, MYGROUP_USERS);
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getGroupPrincipals();
         assertEquals(1, principals.size());
 
@@ -140,7 +147,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile(MY_GROUP, MYGROUP_USERS);
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getUserPrincipalsForGroup("myGroup");
         assertEquals(1, principals.size());
         assertFalse(principals.contains(new UsernamePrincipal("user2")));
@@ -156,7 +163,7 @@ public class FileGroupManagerTest extends QpidTestCase
     {
         final String groupFileName = writeGroupFile(MY_GROUP, MYGROUP_USERS);
         _manager = new FileGroupManager(groupFileName);
-
+        _manager.open();
         Set<Principal> principals = _manager.getUserPrincipalsForGroup("myGroup");
         assertEquals(1, principals.size());
         assertTrue(principals.contains(new UsernamePrincipal("user1")));
diff --git a/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupProviderAttributeDescriptions.properties b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupProviderAttributeDescriptions.properties
new file mode 100644
index 0000000000..2c2d2ab9e3
--- /dev/null
+++ b/qpid/java/broker/src/test/java/org/apache/qpid/server/security/group/FileGroupProviderAttributeDescriptions.properties
@@ -0,0 +1,19 @@
+#
+#  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.
+
+path= File location*