From b54de460c65cbb20b020ef662677a98de83c779d Mon Sep 17 00:00:00 2001
From: jbertram <jbertram@apache.org>
Date: Fri, 1 Jul 2016 21:18:06 -0500
Subject: [PATCH] ARTEMIS-592 finer-grained security for queues

---
 .../core/server/impl/ServerSessionImpl.java   | 14 ++++-
 .../integration/security/SecurityTest.java    | 59 +++++++++++++++++++
 .../src/test/resources/roles.properties       |  2 +
 .../src/test/resources/users.properties       |  2 +
 4 files changed, 75 insertions(+), 2 deletions(-)

diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java
index aeee1a8be1..c3d399a4bd 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java
@@ -420,10 +420,20 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
       }
 
       if (browseOnly) {
-         securityCheck(binding.getAddress(), CheckType.BROWSE, this);
+         try {
+            securityCheck(binding.getAddress(), CheckType.BROWSE, this);
+         }
+         catch (Exception e) {
+            securityCheck(binding.getAddress().concat(".").concat(queueName), CheckType.BROWSE, this);
+         }
       }
       else {
-         securityCheck(binding.getAddress(), CheckType.CONSUME, this);
+         try {
+            securityCheck(binding.getAddress(), CheckType.CONSUME, this);
+         }
+         catch (Exception e) {
+            securityCheck(binding.getAddress().concat(".").concat(queueName), CheckType.CONSUME, this);
+         }
       }
 
       Filter filter = FilterImpl.createFilter(filterString);
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
index 17b1126482..5059fab6b0 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
@@ -313,6 +313,65 @@ public class SecurityTest extends ActiveMQTestBase {
       }
    }
 
+   @Test
+   public void testJAASSecurityManagerAuthorizationSameAddressDifferentQueues() throws Exception {
+      final SimpleString ADDRESS = new SimpleString("address");
+      final SimpleString QUEUE_A = new SimpleString("a");
+      final SimpleString QUEUE_B = new SimpleString("b");
+
+      ActiveMQJAASSecurityManager securityManager = new ActiveMQJAASSecurityManager("PropertiesLogin");
+      ActiveMQServer server = addServer(ActiveMQServers.newActiveMQServer(createDefaultInVMConfig().setSecurityEnabled(true), ManagementFactory.getPlatformMBeanServer(), securityManager, false));
+      Set<Role> aRoles = new HashSet<>();
+      aRoles.add(new Role(QUEUE_A.toString(), false, true, false, false, false, false, false, false));
+      server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_A).toString(), aRoles);
+      Set<Role> bRoles = new HashSet<>();
+      bRoles.add(new Role(QUEUE_B.toString(), false, true, false, false, false, false, false, false));
+      server.getConfiguration().putSecurityRoles(ADDRESS.concat(".").concat(QUEUE_B).toString(), bRoles);
+      server.start();
+      server.createQueue(ADDRESS, QUEUE_A, null, true, false);
+      server.createQueue(ADDRESS, QUEUE_B, null, true, false);
+
+      ClientSessionFactory cf = createSessionFactory(locator);
+      ClientSession aSession = addClientSession(cf.createSession("a", "a", false, true, true, false, 0));
+      ClientSession bSession = addClientSession(cf.createSession("b", "b", false, true, true, false, 0));
+
+      // client A CONSUME from queue A
+      try {
+         ClientConsumer consumer = aSession.createConsumer(QUEUE_A);
+      }
+      catch (ActiveMQException e) {
+         e.printStackTrace();
+         Assert.fail("should not throw exception here");
+      }
+
+      // client B CONSUME from queue A
+      try {
+         ClientConsumer consumer = bSession.createConsumer(QUEUE_A);
+         Assert.fail("should throw exception here");
+      }
+      catch (ActiveMQException e) {
+         assertTrue(e instanceof ActiveMQSecurityException);
+      }
+
+      // client B CONSUME from queue B
+      try {
+         ClientConsumer consumer = bSession.createConsumer(QUEUE_B);
+      }
+      catch (ActiveMQException e) {
+         e.printStackTrace();
+         Assert.fail("should not throw exception here");
+      }
+
+      // client A CONSUME from queue B
+      try {
+         ClientConsumer consumer = aSession.createConsumer(QUEUE_B);
+         Assert.fail("should throw exception here");
+      }
+      catch (ActiveMQException e) {
+         assertTrue(e instanceof ActiveMQSecurityException);
+      }
+   }
+
    @Test
    public void testJAASSecurityManagerAuthorizationNegativeWithCerts() throws Exception {
       final SimpleString ADDRESS = new SimpleString("address");
diff --git a/tests/integration-tests/src/test/resources/roles.properties b/tests/integration-tests/src/test/resources/roles.properties
index de332d395d..12649f05d5 100644
--- a/tests/integration-tests/src/test/resources/roles.properties
+++ b/tests/integration-tests/src/test/resources/roles.properties
@@ -18,3 +18,5 @@
 programmers=first
 accounting=second
 employees=first,second
+a=a
+b=b
diff --git a/tests/integration-tests/src/test/resources/users.properties b/tests/integration-tests/src/test/resources/users.properties
index 1087b0b3f1..de63386c21 100644
--- a/tests/integration-tests/src/test/resources/users.properties
+++ b/tests/integration-tests/src/test/resources/users.properties
@@ -17,3 +17,5 @@
 
 first=secret
 second=password
+a=a
+b=b