ARTEMIS-1872 Fixing address security checks

Ensure CREATE_ADDRESS is honored and behavior is consistent across protocols.

artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPSessionCallback.java

@@ -298,7 +298,7 @@ public class AMQPSessionCallback implements SessionCallback {
       }
 
       // if auto-create we will return whatever type was used before
-      if (!queueQueryResult.isAutoCreated() && queueQueryResult.getRoutingType() != routingType) {
+      if (queueQueryResult.isExists() && !queueQueryResult.isAutoCreated() && queueQueryResult.getRoutingType() != routingType) {
          throw new IllegalStateException("Incorrect Routing Type for queue, expecting: " + routingType);
       }
 

artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java

@@ -583,9 +583,15 @@ public class ServerSessionImpl implements ServerSession, FailureListener {
          securityCheck(addressInfo.getName(), name, CheckType.CREATE_NON_DURABLE_QUEUE, this);
       }
 
+      AddressSettings as = server.getAddressSettingsRepository().getMatch(art.getName().toString());
+
+      if (as.isAutoCreateAddresses() && server.getAddressInfo(art.getName()) == null) {
+         securityCheck(addressInfo.getName(), name, CheckType.CREATE_ADDRESS, this);
+      }
+
       server.checkQueueCreationLimit(getUsername());
 
-      Queue queue = server.createQueue(art, unPrefixedName, filterString, SimpleString.toSimpleString(getUsername()), durable, temporary, autoCreated, maxConsumers, purgeOnNoConsumers, exclusive, lastValue, server.getAddressSettingsRepository().getMatch(art.getName().toString()).isAutoCreateAddresses());
+      Queue queue = server.createQueue(art, unPrefixedName, filterString, SimpleString.toSimpleString(getUsername()), durable, temporary, autoCreated, maxConsumers, purgeOnNoConsumers, exclusive, lastValue, as.isAutoCreateAddresses());
 
       if (temporary) {
          // Temporary queue in core simply means the queue will be deleted if

tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/server/SecureConfigurationTest.java

@@ -16,7 +16,6 @@
  */
 package org.apache.activemq.artemis.tests.integration.server;
 
-import java.lang.IllegalStateException;
 import java.util.Arrays;
 import java.util.Collection;
 import javax.jms.Connection;
@@ -45,7 +44,6 @@ import org.junit.After;
 import org.junit.Assert;
 import org.junit.Assume;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -168,9 +166,14 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
 
    @Test
    public void testTemporaryQueue() throws Exception {
-      ConnectionFactory connectionFactory = getConnectionFactory("c", "c");
+      ConnectionFactory connectionFactory = getConnectionFactory("a", "a");
       String message = "blah";
 
+      //Expect to be able to create subscriber on pre-defined/existing queue.
+      String messageRecieved = sendAndReceiveText(connectionFactory, "clientId", message, s -> s.createTemporaryQueue(), (d, s) -> s.createConsumer(d));
+      Assert.assertEquals(message, messageRecieved);
+
+      connectionFactory = getConnectionFactory("c", "c");
       try {
          sendAndReceiveText(connectionFactory, "clientId", message, s -> s.createTemporaryQueue(), (d, s) -> s.createConsumer(d));
          Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to create a temporary queue");
@@ -183,9 +186,14 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
 
    @Test
    public void testTemporaryTopic() throws Exception {
-      ConnectionFactory connectionFactory = getConnectionFactory("c", "c");
+      ConnectionFactory connectionFactory = getConnectionFactory("a", "a");
       String message = "blah";
 
+      //Expect to be able to create subscriber on pre-defined/existing queue.
+      String messageRecieved = sendAndReceiveText(connectionFactory, "clientId", message, s -> s.createTemporaryTopic(), (d, s) -> s.createConsumer(d));
+      Assert.assertEquals(message, messageRecieved);
+
+      connectionFactory = getConnectionFactory("c", "c");
       try {
          sendAndReceiveText(connectionFactory, "clientId", message, s -> s.createTemporaryTopic(), (d, s) -> s.createConsumer(d));
          Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to create a temporary queue");
@@ -198,8 +206,6 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
 
    @Test
    public void testSecureQueue() throws Exception {
-      // Core & OpenWire are not creating the queue as the test expects.. just querying
-      Assume.assumeTrue(protocol.equals("AMQP"));
       ConnectionFactory connectionFactory = getConnectionFactory("b", "b");
       String message = "blah";
 
@@ -207,12 +213,22 @@ public class SecureConfigurationTest extends ActiveMQTestBase {
       String messageRecieved = sendAndReceiveTextUsingQueue(connectionFactory, "clientId", message, "secured_queue", (q, s) -> s.createConsumer(q));
       Assert.assertEquals(message, messageRecieved);
 
+      connectionFactory = getConnectionFactory("a", "a");
+      messageRecieved = sendAndReceiveTextUsingQueue(connectionFactory, "clientId", message, "new-queue-1", (q, s) -> s.createConsumer(q));
+      Assert.assertEquals(message, messageRecieved);
+
+      connectionFactory = getConnectionFactory("b", "b");
       try {
-         sendAndReceiveTextUsingQueue(connectionFactory, "clientId", message, "non-existent-queue", (q, s) -> s.createConsumer(q));
+         sendAndReceiveTextUsingQueue(connectionFactory, "clientId", message, "new-queue-2", (q, s) -> s.createConsumer(q));
-         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create queue");
+         Assert.fail("Security exception expected, but did not occur, excepetion expected as not permissioned to dynamically create address, or queue");
       } catch (JMSSecurityException j) {
          //Expected exception
       }
+
+      connectionFactory = getConnectionFactory("a", "a");
+      messageRecieved = sendAndReceiveTextUsingQueue(connectionFactory, "clientId", message, "new-queue-2", (q, s) -> s.createConsumer(q));
+      Assert.assertEquals(message, messageRecieved);
+
    }
 
 

tests/integration-tests/src/test/resources/multicast_topic.xml

@@ -91,6 +91,7 @@ under the License.
 
       <security-settings>
          <security-setting match="#">
+            <permission type="createAddress" roles="a" />
             <permission type="createNonDurableQueue" roles="a,b"/>
             <permission type="deleteNonDurableQueue" roles="a,b"/>
             <permission type="createDurableQueue" roles="a,b"/>
@@ -139,7 +140,6 @@ under the License.
       <address-settings>
          <!--default for catch all-->
          <address-setting match="#">
-            <auto-create-queues>false</auto-create-queues>
             <dead-letter-address>DLQ</dead-letter-address>
             <expiry-address>ExpiryQueue</expiry-address>
             <redelivery-delay>0</redelivery-delay>