QPID-5634: [Java Broker] Remove support for AccessPlugins at the level of the virtualhost. Introduce supports for ACLs rules that include virtualhost predicate.

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

7 files changed, 123 insertions, 39 deletions

diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
index 74ea7639ff..72dadb736f 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/DefaultAccessControlTest.java
@@ -20,7 +20,9 @@
  */
 package org.apache.qpid.server.security.access.plugins;
 
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -182,7 +184,7 @@ public class DefaultAccessControlTest extends TestCase
         final RuleSet rs = new RuleSet(mock(EventLoggerProvider.class));
 
         // grant user4 access right on any method in any component
-        rs.grant(1, "user4", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, new ObjectProperties(ObjectProperties.STAR));
+        rs.grant(1, "user4", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, new ObjectProperties(ObjectProperties.WILD_CARD));
         configureAccessControl(rs);
         Subject.doAs(TestPrincipalUtils.createTestSubject("user4"), new PrivilegedAction<Object>()
         {
@@ -208,7 +210,7 @@ public class DefaultAccessControlTest extends TestCase
         final RuleSet rs = new RuleSet(mock(EventLoggerProvider.class));
 
         // grant user5 access right on any methods in "Test" component
-        ObjectProperties ruleProperties = new ObjectProperties(ObjectProperties.STAR);
+        ObjectProperties ruleProperties = new ObjectProperties(ObjectProperties.WILD_CARD);
         ruleProperties.put(ObjectProperties.Property.COMPONENT, "Test");
         rs.grant(1, "user5", Permission.ALLOW, Operation.ACCESS, ObjectType.METHOD, ruleProperties);
         configureAccessControl(rs);
@@ -235,6 +237,7 @@ public class DefaultAccessControlTest extends TestCase
     public void testAccess() throws Exception
     {
         final Subject subject = TestPrincipalUtils.createTestSubject("user1");
+        final String testVirtualHost = getName();
         final InetAddress inetAddress = InetAddress.getLocalHost();
         final InetSocketAddress inetSocketAddress = new InetSocketAddress(inetAddress, 1);
 
@@ -250,13 +253,12 @@ public class DefaultAccessControlTest extends TestCase
             {
                 RuleSet mockRuleSet = mock(RuleSet.class);
 
-
-
                 DefaultAccessControl accessControl = new DefaultAccessControl(mockRuleSet);
 
-                accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+                ObjectProperties properties = new ObjectProperties(testVirtualHost);
+                accessControl.authorise(Operation.ACCESS, ObjectType.VIRTUALHOST, properties);
 
-                verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY, inetAddress);
+                verify(mockRuleSet).check(subject, Operation.ACCESS, ObjectType.VIRTUALHOST, properties, inetAddress);
                 return null;
             }
         });
diff --git a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
index caf9b2fb61..32037807cd 100644
--- a/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
+++ b/qpid/java/broker-plugins/access-control/src/test/java/org/apache/qpid/server/security/access/plugins/RuleSetTest.java
@@ -21,24 +21,26 @@
 
 package org.apache.qpid.server.security.access.plugins;
 
-import java.security.Principal;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import javax.security.auth.Subject;
 
-import org.apache.qpid.server.logging.EventLogger;
+import org.apache.qpid.server.exchange.ExchangeImpl;
 import org.apache.qpid.server.logging.EventLoggerProvider;
+import org.apache.qpid.server.model.VirtualHost;
+import org.apache.qpid.server.queue.AMQQueue;
 import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.access.ObjectProperties;
 import org.apache.qpid.server.security.access.ObjectType;
 import org.apache.qpid.server.security.access.Operation;
 import org.apache.qpid.server.security.access.Permission;
+import org.apache.qpid.server.security.access.ObjectProperties.Property;
 import org.apache.qpid.server.security.access.config.Rule;
 import org.apache.qpid.server.security.access.config.RuleSet;
 import org.apache.qpid.server.security.auth.TestPrincipalUtils;
 import org.apache.qpid.test.utils.QpidTestCase;
 
-import static org.mockito.Mockito.mock;
-
 /**
  * This test checks that the {@link RuleSet} object which forms the core of the access control plugin performs correctly.
  *
@@ -51,6 +53,9 @@ import static org.mockito.Mockito.mock;
  */
 public class RuleSetTest extends QpidTestCase
 {
+    private static final String DENIED_VH = "deniedVH";
+    private static final String ALLOWED_VH = "allowedVH";
+
     private RuleSet _ruleSet; // Object under test
 
     private static final String TEST_USER = "user";
@@ -60,6 +65,8 @@ public class RuleSetTest extends QpidTestCase
     private String _exchangeName = "amq.direct";
     private String _exchangeType = "direct";
     private Subject _testSubject = TestPrincipalUtils.createTestSubject(TEST_USER);
+    private AMQQueue<?> _queue;
+    private VirtualHost<?> _virtualHost;
 
     @Override
     public void setUp() throws Exception
@@ -67,6 +74,11 @@ public class RuleSetTest extends QpidTestCase
         super.setUp();
 
         _ruleSet = new RuleSet(mock(EventLoggerProvider.class));
+
+        _virtualHost = mock(VirtualHost.class);
+        _queue = mock(AMQQueue.class);
+        when(_queue.getName()).thenReturn(_queueName);
+        when(_queue.getParent(VirtualHost.class)).thenReturn(_virtualHost);
     }
 
     @Override
@@ -83,10 +95,8 @@ public class RuleSetTest extends QpidTestCase
 
     public void assertDenyGrantAllow(Subject subject, Operation operation, ObjectType objectType, ObjectProperties properties)
     {
-        final Principal identity = subject.getPrincipals().iterator().next();
-
         assertEquals(Result.DENIED, _ruleSet.check(subject, operation, objectType, properties));
-        _ruleSet.grant(0, identity.getName(), Permission.ALLOW, operation, objectType, properties);
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, operation, objectType, properties);
         assertEquals(1, _ruleSet.getRuleCount());
         assertEquals(Result.ALLOWED, _ruleSet.check(subject, operation, objectType, properties));
     }
@@ -98,17 +108,77 @@ public class RuleSetTest extends QpidTestCase
         assertEquals(_ruleSet.getDefault(), _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY));
     }
 
-    public void testVirtualHostAccess() throws Exception
+    public void testVirtualHostAccessAllowPermissionWithVirtualHostName() throws Exception
     {
-        assertDenyGrantAllow(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST);
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.DEFER, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
     }
 
+    public void testVirtualHostAccessAllowPermissionWithNameSetToWildCard() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ObjectProperties.WILD_CARD));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+    public void testVirtualHostAccessAllowPermissionWithNoName() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+    public void testVirtualHostAccessDenyPermissionWithNoName() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, ObjectProperties.EMPTY);
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+    public void testVirtualHostAccessDenyPermissionWithNameSetToWildCard() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ObjectProperties.WILD_CARD));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+    public void testVirtualHostAccessAllowDenyPermissions() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.DENY, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH));
+        _ruleSet.grant(1, TEST_USER, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH));
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(ALLOWED_VH)));
+        assertEquals(Result.DENIED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+    public void testVirtualHostAccessAllowPermissionWithVirtualHostNameOtherPredicate() throws Exception
+    {
+        ObjectProperties properties = new ObjectProperties();
+        properties.put(Property.VIRTUALHOST_NAME, ALLOWED_VH);
+
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.ACCESS, ObjectType.VIRTUALHOST, properties);
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, properties));
+        assertEquals(Result.DEFER, _ruleSet.check(_testSubject, Operation.ACCESS, ObjectType.VIRTUALHOST, new ObjectProperties(DENIED_VH)));
+    }
+
+
     public void testQueueCreateNamed() throws Exception
     {
         assertDenyGrantAllow(_testSubject, Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(_queueName));
     }
 
-    public void testQueueCreatenamedNullRoutingKey()
+    public void testQueueCreateNamedVirtualHost() throws Exception
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(Property.VIRTUALHOST_NAME, ALLOWED_VH));
+
+        when(_virtualHost.getName()).thenReturn(ALLOWED_VH);
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(_queue)));
+
+        when(_virtualHost.getName()).thenReturn(DENIED_VH);
+        assertEquals(Result.DEFER, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.QUEUE, new ObjectProperties(_queue)));
+    }
+
+    public void testQueueCreateNamedNullRoutingKey()
     {
         ObjectProperties properties = new ObjectProperties(_queueName);
         properties.put(ObjectProperties.Property.ROUTING_KEY, (String) null);
@@ -116,6 +186,21 @@ public class RuleSetTest extends QpidTestCase
         assertDenyGrantAllow(_testSubject, Operation.CREATE, ObjectType.QUEUE, properties);
     }
 
+    public void testExchangeCreateNamedVirtualHost()
+    {
+        _ruleSet.grant(0, TEST_USER, Permission.ALLOW, Operation.CREATE, ObjectType.EXCHANGE, new ObjectProperties(Property.VIRTUALHOST_NAME, ALLOWED_VH));
+
+        ExchangeImpl<?> exchange = mock(ExchangeImpl.class);
+        when(exchange.getParent(VirtualHost.class)).thenReturn(_virtualHost);
+        when(exchange.getTypeName()).thenReturn(_exchangeType);
+        when(_virtualHost.getName()).thenReturn(ALLOWED_VH);
+
+        assertEquals(Result.ALLOWED, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.EXCHANGE, new ObjectProperties(exchange)));
+
+        when(_virtualHost.getName()).thenReturn(DENIED_VH);
+        assertEquals(Result.DEFER, _ruleSet.check(_testSubject, Operation.CREATE, ObjectType.EXCHANGE, new ObjectProperties(exchange)));
+    }
+
     public void testExchangeCreate()
     {
         ObjectProperties properties = new ObjectProperties(_exchangeName);
diff --git a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
index 040be92ceb..999da2da6c 100644
--- a/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
+++ b/qpid/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerSessionDelegate.java
@@ -291,7 +291,7 @@ public class ServerSessionDelegate extends SessionDelegate
         final VirtualHost virtualHost = getVirtualHost(ssn);
         try
         {
-            virtualHost.getSecurityManager().authorisePublish(messageMetaData.isImmediate(), messageMetaData.getRoutingKey(), exchange.getName());
+            virtualHost.getSecurityManager().authorisePublish(messageMetaData.isImmediate(), messageMetaData.getRoutingKey(), exchange.getName(), virtualHost.getName());
         }
         catch (AccessControlException e)
         {
diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
index baf5eceef7..7bde83cc99 100644
--- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
+++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQChannel.java
@@ -303,9 +303,10 @@ public class AMQChannel<T extends AMQProtocolSession<T>>
     public void setPublishFrame(MessagePublishInfo info, final MessageDestination e)
     {
         String routingKey = info.getRoutingKey() == null ? null : info.getRoutingKey().asString();
-        SecurityManager securityManager = getVirtualHost().getSecurityManager();
+        VirtualHost virtualHost = getVirtualHost();
+        SecurityManager securityManager = virtualHost.getSecurityManager();
 
-        securityManager.authorisePublish(info.isImmediate(), routingKey, e.getName());
+        securityManager.authorisePublish(info.isImmediate(), routingKey, e.getName(), virtualHost.getName());
 
         _currentMessage = new IncomingMessage(info);
         _currentMessage.setMessageDestination(e);
diff --git a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
index a29d56605a..1a29806f62 100644
--- a/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
+++ b/qpid/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/handler/ConnectionOpenMethodHandler.java
@@ -80,6 +80,8 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
         }
         else
         {
+            session.setVirtualHost(virtualHost);
+
             // Check virtualhost access
             try
             {
@@ -95,7 +97,6 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
                 throw body.getConnectionException(AMQConstant.CONNECTION_FORCED, "Virtual host '" + virtualHost.getName() + "' is not active");
             }
 
-            session.setVirtualHost(virtualHost);
 
             // See Spec (0.8.2). Section  3.1.2 Virtual Hosts
             if (session.getContextKey() == null)
diff --git a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java
index 0947ae2a89..b23f0cb168 100644
--- a/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java
+++ b/qpid/java/broker-plugins/management-http/src/main/java/org/apache/qpid/server/management/plugin/servlet/rest/MessageServlet.java
@@ -485,10 +485,10 @@ public class MessageServlet extends AbstractServlet
 
     }
 
-    private void authorizeMethod(String methodName, VirtualHost host)
+    private void authorizeMethod(String methodName, VirtualHost<?> vhost)
     {
-        SecurityManager securityManager = host.getSecurityManager();
-        securityManager.authoriseMethod(Operation.UPDATE, "VirtualHost.Queue", methodName);
+        SecurityManager securityManager = getBroker().getSecurityManager();
+        securityManager.authoriseMethod(Operation.UPDATE, "VirtualHost.Queue", methodName, vhost.getName());
     }
 
 }
diff --git a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
index 7b0a48cac1..5a7674d4fd 100644
--- a/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
+++ b/qpid/java/broker-plugins/management-jmx/src/main/java/org/apache/qpid/server/jmx/MBeanInvocationHandlerImpl.java
@@ -60,15 +60,15 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
     private MBeanServer _mbs;
 
     private final boolean _managementRightsInferAllAccess;
-    private final Broker _broker;
+    private final Broker<?> _broker;
 
-    MBeanInvocationHandlerImpl(Broker broker)
+    MBeanInvocationHandlerImpl(Broker<?> broker)
     {
         _managementRightsInferAllAccess = Boolean.valueOf(System.getProperty(BrokerProperties.PROPERTY_MANAGEMENT_RIGHTS_INFER_ALL_ACCESS, "true"));
         _broker = broker;
     }
 
-    public static MBeanServerForwarder newProxyInstance(Broker broker)
+    public static MBeanServerForwarder newProxyInstance(Broker<?> broker)
     {
         final InvocationHandler handler = new MBeanInvocationHandlerImpl(broker);
         final Class<?>[] interfaces = new Class[] { MBeanServerForwarder.class };
@@ -195,28 +195,23 @@ public class MBeanInvocationHandlerImpl implements InvocationHandler
         String methodName;
         // Get the component, type and impact, which may be null
         String type = getType(method, args);
-        String vhost = getVirtualHost(method, args);
+        String virtualHostName = getVirtualHost(method, args);
         int impact = getImpact(method, args);
 
-        // Get the security manager for the virtual host (if set)
-        SecurityManager security;
-        if (vhost == null)
+        if (virtualHostName != null)
         {
-            security = _broker.getSecurityManager();
-        }
-        else
-        {
-            VirtualHost virtualHost = _broker.findVirtualHostByName(vhost);
+            VirtualHost<?> virtualHost = _broker.findVirtualHostByName(virtualHostName);
             if (virtualHost == null)
             {
-                throw new IllegalArgumentException("Virtual host with name '" + vhost + "' is not found.");
+                throw new IllegalArgumentException("Virtual host with name '" + virtualHostName + "' is not found.");
             }
-            security = virtualHost.getSecurityManager();
         }
 
         methodName = getMethodName(method, args);
         Operation operation = (isAccessMethod(methodName) || impact == MBeanOperationInfo.INFO) ? Operation.ACCESS : Operation.UPDATE;
-        security.authoriseMethod(operation, type, methodName);
+
+        SecurityManager security = _broker.getSecurityManager();
+        security.authoriseMethod(operation, type, methodName, virtualHostName);
 
         if (_managementRightsInferAllAccess)
         {