You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2021/07/12 15:28:17 UTC

[activemq-artemis] branch main updated (4c06d44 -> ec508d8)

This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git.


    from 4c06d44  ARTEMIS-2974 - fix up the regexp used in the test to match to the end
     new bf875c3  ARTEMIS-3374 fix tests and shared subscriber case
     new ec508d8  ARTEMIS-3381 AMQP bypasses session when deleting queues

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../artemis/jms/client/ActiveMQSession.java        |  8 ++---
 .../protocol/amqp/broker/AMQPSessionCallback.java  |  2 +-
 .../amqp/proton/ProtonServerSenderContext.java     |  4 +--
 .../server/SecureConfigurationTest.java            | 35 ++++++++--------------
 .../src/test/resources/multicast_topic.xml         |  6 ++--
 5 files changed, 20 insertions(+), 35 deletions(-)

[activemq-artemis] 02/02: ARTEMIS-3381 AMQP bypasses session when deleting queues

Posted by cl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git

commit ec508d8306b8e0db5dcdbf8c26a0f480608d97dc
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Thu Jul 8 20:01:44 2021 -0500

    ARTEMIS-3381 AMQP bypasses session when deleting queues
    
    The AMQP implementation bypasses the ServerSession when deleting queues
    which also bypasses security authorization.
---
 .../protocol/amqp/broker/AMQPSessionCallback.java     |  2 +-
 .../integration/server/SecureConfigurationTest.java   | 19 ++++++++++++++++++-
 .../src/test/resources/multicast_topic.xml            |  6 +++---
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java
index 9be3858..82dbec7 100644
--- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java
+++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java
@@ -642,7 +642,7 @@ public class AMQPSessionCallback implements SessionCallback {
    }
 
    public void deleteQueue(SimpleString queueName) throws Exception {
-      manager.getServer().destroyQueue(queueName);
+      serverSession.deleteQueue(queueName);
    }
 
    public void resetContext(OperationContext oldContext) {
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
index 87adf23..7d6e802 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
@@ -120,7 +120,7 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
    }
 
    @Test
-   public void testSecureDurableSubscriber() throws Exception {
+   public void testCreateSecureDurableSubscriber() throws Exception {
       ConnectionFactory connectionFactory = getConnectionFactory("b", "b");
       String message = "blah";
 
@@ -137,6 +137,23 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
    }
 
    @Test
+   public void testDeleteSecureDurableSubscriber() throws Exception {
+      ConnectionFactory connectionFactory = getConnectionFactory("c", "c");
+      String message = "blah";
+
+      //Expect to be able to create durable queue for subscription
+      String messageRecieved = sendAndReceiveTextUsingTopic(connectionFactory, "clientId", message, "secured_topic_durable", (t, s) -> s.createDurableSubscriber(t, "secured_topic_durable/non-existant-queue"));
+      Assert.assertEquals(message, messageRecieved);
+
+      try {
+         sendAndReceiveTextUsingTopic(connectionFactory, "clientId", message, "secured_topic_durable", (t, s) -> s.createDurableSubscriber(t, "secured_topic_durable/non-existant-queue", "age > 10", false));
+         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically delete queue");
+      } catch (JMSSecurityException j) {
+         //Expected exception
+      }
+   }
+
+   @Test
    public void testTemporaryQueue() throws Exception {
       ConnectionFactory connectionFactory = getConnectionFactory("a", "a");
       String message = "blah";
diff --git a/tests/integration-tests/src/test/resources/multicast_topic.xml b/tests/integration-tests/src/test/resources/multicast_topic.xml
index a4891d0..009b3e8 100644
--- a/tests/integration-tests/src/test/resources/multicast_topic.xml
+++ b/tests/integration-tests/src/test/resources/multicast_topic.xml
@@ -127,11 +127,11 @@ under the License.
          <security-setting match="secured_topic_durable">
             <permission type="createNonDurableQueue" roles="a"/>
             <permission type="deleteNonDurableQueue" roles="a"/>
-            <permission type="createDurableQueue" roles="a"/>
+            <permission type="createDurableQueue" roles="a,c"/>
             <permission type="deleteDurableQueue" roles="a"/>
             <permission type="browse" roles="a"/>
-            <permission type="send" roles="a,b"/>
-            <permission type="consume" roles="a,b" />
+            <permission type="send" roles="a,b,c"/>
+            <permission type="consume" roles="a,b,c" />
             <!-- we need this otherwise ./artemis data imp wouldn't work -->
             <permission type="manage" roles="a"/>
          </security-setting>

[activemq-artemis] 01/02: ARTEMIS-3374 fix tests and shared subscriber case

Posted by cl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git

commit bf875c3a376726083ea97313380c89ed1c02d526
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Thu Jul 8 11:55:55 2021 -0500

    ARTEMIS-3374 fix tests and shared subscriber case
    
    A couple of tests were explicitly written to delete the
    configuration-managed queue. Since this is no longer allowed these
    tests had to change.
---
 .../artemis/jms/client/ActiveMQSession.java        |  8 ++-----
 .../amqp/proton/ProtonServerSenderContext.java     |  4 ++--
 .../server/SecureConfigurationTest.java            | 28 ----------------------
 3 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java
index 2076478..6ec2225 100644
--- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java
+++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQSession.java
@@ -743,13 +743,9 @@ public class ActiveMQSession implements QueueSession, TopicSession {
 
          QueueQuery subResponse = session.queueQuery(queueName);
 
-         if (!(subResponse.isExists() && Objects.equals(subResponse.getAddress(), dest.getSimpleAddress()) && Objects.equals(subResponse.getFilterString(), coreFilterString))) {
+         if ((!subResponse.isExists() || !Objects.equals(subResponse.getAddress(), dest.getSimpleAddress()) || !Objects.equals(subResponse.getFilterString(), coreFilterString)) && !subResponse.isConfigurationManaged()) {
             try {
-               if (durability == ConsumerDurability.DURABLE) {
-                  createSharedQueue(dest, RoutingType.MULTICAST, queueName, coreFilterString, true, response);
-               } else {
-                  createSharedQueue(dest, RoutingType.MULTICAST, queueName, coreFilterString, false, response);
-               }
+               createSharedQueue(dest, RoutingType.MULTICAST, queueName, coreFilterString, durability == ConsumerDurability.DURABLE, response);
             } catch (ActiveMQQueueExistsException ignored) {
                // We ignore this because querying and then creating the queue wouldn't be idempotent
                // we could also add a parameter to ignore existence what would require a bigger work around to avoid
diff --git a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
index b383405..e2ffb3f 100644
--- a/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
+++ b/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
@@ -1110,7 +1110,7 @@ public class ProtonServerSenderContext extends ProtonInitializable implements Pr
                       * recreate the queue (JMS semantics). However, if the corresponding queue is managed via the
                       * configuration then we don't want to change it
                       */
-                     if (!result.isConfigurationManaged() && (!Objects.equals(result.getFilterString(), simpleStringSelector) || (sender.getSource() != null && !sender.getSource().getAddress().equals(result.getAddress().toString())))) {
+                     if (!result.isConfigurationManaged() && (!Objects.equals(result.getAddress(), addressToUse) || !Objects.equals(result.getFilterString(), simpleStringSelector))) {
 
                         if (result.getConsumerCount() == 0) {
                            sessionSPI.deleteQueue(queue);
@@ -1132,7 +1132,7 @@ public class ProtonServerSenderContext extends ProtonInitializable implements Pr
                   if (shared && sender.getName() != null) {
                      queue = createQueueName(connection.isUseCoreSubscriptionNaming(), getClientId(), sender.getName(), shared, global, isVolatile);
                      QueueQueryResult result = sessionSPI.queueQuery(queue, routingTypeToUse, false);
-                     if (!(result.isExists() && Objects.equals(result.getAddress(), addressToUse) && Objects.equals(result.getFilterString(), simpleStringSelector))) {
+                     if ((!result.isExists() || !Objects.equals(result.getAddress(), addressToUse) || !Objects.equals(result.getFilterString(), simpleStringSelector)) && !result.isConfigurationManaged()) {
                         sessionSPI.createSharedVolatileQueue(addressToUse, RoutingType.MULTICAST, queue, simpleStringSelector);
                      }
                   } else {
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
index b119b2d..87adf23 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java
@@ -98,13 +98,6 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
       } catch (JMSSecurityException j) {
          //Expected exception
       }
-
-      try {
-         sendAndReceiveTextUsingTopic(connectionFactory, null, message, "secured_topic_shared_durable", (t, s) -> s.createSharedDurableConsumer(t, "secured_topic_shared_durable/queue", "age < 10"));
-         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create queue");
-      } catch (JMSSecurityException j) {
-         //Expected exception
-      }
    }
 
    @Test
@@ -124,13 +117,6 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
       } catch (JMSSecurityException j) {
          //Expected exception
       }
-
-      try {
-         sendAndReceiveTextUsingTopic(connectionFactory, null, message, "secured_topic_shared", (t, s) -> s.createSharedConsumer(t, "secured_topic_shared/queue", "age < 10"));
-         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create queue");
-      } catch (JMSSecurityException j) {
-         //Expected exception
-      }
    }
 
    @Test
@@ -148,20 +134,6 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
       } catch (JMSSecurityException j) {
          //Expected exception
       }
-
-      try {
-         sendAndReceiveTextUsingTopic(connectionFactory, "clientId", message, "secured_topic_durable", (t, s) -> s.createDurableSubscriber(t, "secured_topic_durable/queue", "age < 10", false));
-         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create queue");
-      } catch (JMSSecurityException j) {
-         //Expected exception
-      }
-
-      try {
-         sendAndReceiveTextUsingTopic(connectionFactory, "clientId", message, "secured_topic_durable", (t, s) -> s.createDurableSubscriber(t, "secured_topic_durable/queue", "age < 10", true));
-         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create queue");
-      } catch (JMSSecurityException j) {
-         //Expected exception
-      }
    }
 
    @Test