Merge from trunk

git-svn-id: https://svn.apache.org/repos/asf/qpid/branches/QPID-6262-JavaBrokerNIO@1666219 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java
index 205ff57..30fff15 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhost/berkeleydb/BDBHAReplicaVirtualHostImpl.java
@@ -47,6 +47,7 @@
 import org.apache.qpid.server.protocol.AMQConnectionModel;
 import org.apache.qpid.server.protocol.LinkRegistry;
 import org.apache.qpid.server.queue.AMQQueue;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.stats.StatisticsCounter;
 import org.apache.qpid.server.store.DurableConfigurationStore;
 import org.apache.qpid.server.store.MessageStore;
@@ -355,9 +356,9 @@
     }
 
     @Override
-    public org.apache.qpid.server.security.SecurityManager getSecurityManager()
+    public SecurityManager getSecurityManager()
     {
-        return null;
+        return super.getSecurityManager();
     }
 
     @Override
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
index c475824..3bd44a9 100755
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/security/SecurityManager.java
@@ -39,9 +39,7 @@
 
 import javax.security.auth.Subject;
 
-import org.apache.log4j.Logger;
 import org.apache.qpid.server.model.AccessControlProvider;
-import org.apache.qpid.server.model.AuthenticationProvider;
 import org.apache.qpid.server.model.Binding;
 import org.apache.qpid.server.model.Broker;
 import org.apache.qpid.server.model.ConfiguredObject;
@@ -51,17 +49,13 @@
 import org.apache.qpid.server.model.ExclusivityPolicy;
 import org.apache.qpid.server.model.Group;
 import org.apache.qpid.server.model.GroupMember;
-import org.apache.qpid.server.model.GroupProvider;
-import org.apache.qpid.server.model.KeyStore;
 import org.apache.qpid.server.model.LifetimePolicy;
 import org.apache.qpid.server.model.Model;
-import org.apache.qpid.server.model.Plugin;
-import org.apache.qpid.server.model.Port;
+import org.apache.qpid.server.model.PreferencesProvider;
 import org.apache.qpid.server.model.Queue;
 import org.apache.qpid.server.model.RemoteReplicationNode;
 import org.apache.qpid.server.model.Session;
 import org.apache.qpid.server.model.State;
-import org.apache.qpid.server.model.TrustStore;
 import org.apache.qpid.server.model.User;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.model.VirtualHostAlias;
@@ -78,7 +72,6 @@
 
 public class SecurityManager
 {
-    private static final Logger LOGGER = Logger.getLogger(SecurityManager.class);
 
     private static final Subject SYSTEM = new Subject(true,
                                                      Collections.singleton(new SystemPrincipal()),
@@ -274,38 +267,17 @@
             return;
         }
 
-        if (Operation.CREATE == operation && configuredObject instanceof RemoteReplicationNode)
+        if (isAllowedOperation(operation, configuredObject))
         {
             // creation of remote replication node is out of control for user of this broker
             return;
         }
 
-        if ((Operation.CREATE == operation) && configuredObject instanceof RemoteReplicationNode)
-        {
-            // creation of remote replication node is out of control for user of this broker
-            return;
-        }
-
-        if ((EnumSet.of(Operation.CREATE, Operation.UPDATE, Operation.DELETE).contains(operation)) && configuredObject instanceof Session)
-        {
-            return;
-        }
-
-        if ((EnumSet.of(Operation.UPDATE, Operation.DELETE).contains(operation)) && (configuredObject instanceof Consumer || configuredObject instanceof Connection))
-        {
-            return;
-        }
-
-
         Class<? extends ConfiguredObject> categoryClass = configuredObject.getCategoryClass();
-        LOGGER.debug("getCategoryClass " + categoryClass);
         ObjectType objectType = getACLObjectTypeManagingConfiguredObjectOfCategory(categoryClass);
-        LOGGER.debug("objectType " + objectType);
         if (objectType == null)
         {
-            LOGGER.warn("Cannot determine object type for " + configuredObject.getName() + " of category "
-                    + categoryClass + ". Skipping ACL check...");
-            return;
+            throw new IllegalArgumentException("Cannot identify object type for category " + categoryClass );
         }
 
         ObjectProperties properties = getACLObjectProperties(configuredObject, operation);
@@ -336,6 +308,28 @@
         }
     }
 
+    private boolean isAllowedOperation(Operation operation, ConfiguredObject<?> configuredObject)
+    {
+        if (configuredObject instanceof Session && (operation == Operation.CREATE || operation == Operation.UPDATE
+                || operation ==  Operation.DELETE))
+        {
+            return true;
+
+        }
+
+        if (configuredObject instanceof Consumer && (operation == Operation.UPDATE || operation ==  Operation.DELETE))
+        {
+            return true;
+        }
+
+        if (configuredObject instanceof Connection && (operation == Operation.UPDATE || operation ==  Operation.DELETE))
+        {
+            return true;
+        }
+
+        return false;
+    }
+
     private Model getModel()
     {
         return _aclProvidersParent.getModel();
@@ -371,7 +365,7 @@
                 // CREATE GROUP MEMBER is transformed into UPDATE GROUP rule
                 return Operation.UPDATE;
             }
-            else if (isBrokerOrBrokerChild(category))
+            else if (isBrokerOrBrokerChildOrPreferencesProvider(category))
             {
                 // CREATE/UPDATE broker child is transformed into CONFIGURE BROKER rule
                 return Operation.CONFIGURE;
@@ -384,10 +378,11 @@
                 // DELETE BINDING is transformed into UNBIND EXCHANGE rule
                 return Operation.UNBIND;
             }
-            else if (isBrokerOrBrokerChild(category))
+            else if (isBrokerOrBrokerChildOrPreferencesProvider(category))
             {
                 // DELETE broker child is transformed into CONFIGURE BROKER rule
                 return Operation.CONFIGURE;
+
             }
             else if (GroupMember.class.isAssignableFrom(category))
             {
@@ -398,16 +393,11 @@
         return operation;
     }
 
-    private boolean isBrokerOrBrokerChild(Class<? extends ConfiguredObject> category)
+    private boolean isBrokerOrBrokerChildOrPreferencesProvider(Class<? extends ConfiguredObject> category)
     {
-        return Broker.class.isAssignableFrom(category)
-                || Port.class.isAssignableFrom(category)
-                || AuthenticationProvider.class.isAssignableFrom(category)
-                || AccessControlProvider.class.isAssignableFrom(category)
-                || GroupProvider.class.isAssignableFrom(category)
-                || KeyStore.class.isAssignableFrom(category)
-                || TrustStore.class.isAssignableFrom(category)
-                || Plugin.class.isAssignableFrom(category);
+        return Broker.class.isAssignableFrom(category) ||
+               PreferencesProvider.class.isAssignableFrom(category) ||
+               ( !VirtualHostNode.class.isAssignableFrom(category) && getModel().getChildTypes(Broker.class).contains(category));
     }
 
     private ObjectProperties getACLObjectProperties(ConfiguredObject<?> configuredObject, Operation configuredObjectOperation)
@@ -448,7 +438,7 @@
             Queue<?> queue = (Queue<?>)configuredObject.getParent(Queue.class);
             setQueueProperties(queue, properties);
         }
-        else if (isBrokerOrBrokerChild(configuredObjectType))
+        else if (isBrokerOrBrokerChildOrPreferencesProvider(configuredObjectType))
         {
             String description = String.format("%s %s '%s'",
                     configuredObjectOperation == null? null : configuredObjectOperation.name().toLowerCase(),
@@ -494,7 +484,7 @@
         {
             return ObjectType.VIRTUALHOSTNODE;
         }
-        else if (isBrokerOrBrokerChild(category))
+        else if (isBrokerOrBrokerChildOrPreferencesProvider(category))
         {
             return ObjectType.BROKER;
         }
diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java
index cacc981..917c2fd 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/virtualhostnode/RedirectingVirtualHostImpl.java
@@ -48,6 +48,7 @@
 import org.apache.qpid.server.protocol.AMQConnectionModel;
 import org.apache.qpid.server.protocol.LinkRegistry;
 import org.apache.qpid.server.queue.AMQQueue;
+import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.stats.StatisticsCounter;
 import org.apache.qpid.server.store.DurableConfigurationStore;
 import org.apache.qpid.server.store.MessageStore;
@@ -355,9 +356,9 @@
     }
 
     @Override
-    public org.apache.qpid.server.security.SecurityManager getSecurityManager()
+    public SecurityManager getSecurityManager()
     {
-        return null;
+        return super.getSecurityManager();
     }
 
     @Override
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java
new file mode 100644
index 0000000..de2fb8f
--- /dev/null
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/TestSecurityManager.java
@@ -0,0 +1,40 @@
+/*
+ *
+ * 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.testmodels;
+
+
+import org.apache.qpid.server.model.ConfiguredObject;
+import org.apache.qpid.server.security.SecurityManager;
+import org.apache.qpid.server.security.access.Operation;
+
+public class TestSecurityManager extends SecurityManager
+{
+    public TestSecurityManager(ConfiguredObject<?> aclProvidersParent)
+    {
+        super(aclProvidersParent, false);
+    }
+
+    @Override
+    public void authorise(Operation operation, ConfiguredObject<?> configuredObject)
+    {
+        // noop
+    }
+}
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java
index 43dcecd..bc60e0d 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestKitCarImpl.java
@@ -25,6 +25,7 @@
 import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
+import org.apache.qpid.server.model.testmodels.TestSecurityManager;
 import org.apache.qpid.server.security.SecurityManager;
 
 @ManagedObject( category = false,
@@ -39,7 +40,7 @@
     public TestKitCarImpl(final Map<String, Object> attributes)
     {
         super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance());
-        _securityManager = new SecurityManager(this, false);
+        _securityManager = new TestSecurityManager(this);
     }
 
     @Override
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java
index 7582de2..719e631 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/hierarchy/TestStandardCarImpl.java
@@ -29,6 +29,7 @@
 import org.apache.qpid.server.model.AbstractConfiguredObject;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
+import org.apache.qpid.server.model.testmodels.TestSecurityManager;
 import org.apache.qpid.server.security.SecurityManager;
 
 @ManagedObject( category = false,
@@ -44,7 +45,7 @@
     public TestStandardCarImpl(final Map<String, Object> attributes)
     {
         super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance());
-        _securityManager = new SecurityManager(this, false);
+        _securityManager = new TestSecurityManager(this);
     }
 
     private static CurrentThreadTaskExecutor newTaskExecutor()
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java
index d4223a2..aa4e611 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/lifecycle/TestConfiguredObject.java
@@ -41,6 +41,7 @@
 import org.apache.qpid.server.model.Model;
 import org.apache.qpid.server.model.State;
 import org.apache.qpid.server.model.StateTransition;
+import org.apache.qpid.server.model.testmodels.TestSecurityManager;
 import org.apache.qpid.server.plugin.ConfiguredObjectRegistration;
 import org.apache.qpid.server.security.SecurityManager;
 
@@ -81,7 +82,7 @@
     {
         super(parents, attributes, taskExecutor, model);
         _opened = false;
-        _securityManager = new SecurityManager(this, false);
+        _securityManager = new TestSecurityManager(this);
     }
 
     @Override
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java
index 5de4004..794c2cf 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodels/singleton/TestSingletonImpl.java
@@ -24,9 +24,11 @@
 import org.apache.qpid.server.configuration.updater.CurrentThreadTaskExecutor;
 import org.apache.qpid.server.configuration.updater.TaskExecutor;
 import org.apache.qpid.server.model.AbstractConfiguredObject;
+import org.apache.qpid.server.model.ConfiguredObject;
 import org.apache.qpid.server.model.ManagedAttributeField;
 import org.apache.qpid.server.model.ManagedObject;
 import org.apache.qpid.server.model.ManagedObjectFactoryConstructor;
+import org.apache.qpid.server.model.testmodels.TestSecurityManager;
 import org.apache.qpid.server.security.SecurityManager;
 
 @ManagedObject( category = false, type = TestSingletonImpl.TEST_SINGLETON_TYPE)
@@ -73,7 +75,7 @@
     public TestSingletonImpl(final Map<String, Object> attributes)
     {
         super(parentsMap(), attributes, newTaskExecutor(), TestModel.getInstance());
-        _securityManager = new SecurityManager(this, false);
+        _securityManager = new TestSecurityManager(this);
     }
 
     private static CurrentThreadTaskExecutor newTaskExecutor()
@@ -87,7 +89,7 @@
                              final TaskExecutor taskExecutor)
     {
         super(parentsMap(), attributes, taskExecutor);
-        _securityManager = new SecurityManager(this, false);
+        _securityManager = new TestSecurityManager(this);
     }
 
 
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/AbstractVirtualHostTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/AbstractVirtualHostTest.java
index 8b53042..5a99538 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/AbstractVirtualHostTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/AbstractVirtualHostTest.java
@@ -62,6 +62,7 @@
         when(systemConfig.getEventLogger()).thenReturn(mock(EventLogger.class));
         Broker<?> broker = mock(Broker.class);
         when(broker.getParent(SystemConfig.class)).thenReturn(systemConfig);
+        when(broker.getModel()).thenReturn(BrokerModel.getInstance());
         when(broker.getSecurityManager()).thenReturn(new SecurityManager(broker, false));
 
         _taskExecutor = new TaskExecutorImpl();