diff --git a/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java b/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java index c65f9a38517..50e00d9552e 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java +++ b/activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java @@ -35,6 +35,8 @@ import org.apache.activemq.command.DestinationInfo; import org.apache.activemq.command.Message; import org.apache.activemq.command.ProducerInfo; +import org.apache.activemq.filter.DestinationMap; +import org.apache.activemq.security.DefaultAuthorizationMap.WildcardAwareSet; /** * Verifies if a authenticated user can do an operation against the broker using @@ -91,12 +93,7 @@ protected boolean checkDestinationAdminRemove(SecurityContext securityContext, A protected boolean checkDestinationAdmin(SecurityContext securityContext, ActiveMQDestination destination) { if (!securityContext.isBrokerContext()) { - Set allowedACLs = null; - if (!destination.isTemporary()) { - allowedACLs = authorizationMap.getAdminACLs(destination); - } else { - allowedACLs = authorizationMap.getTempDestinationAdminACLs(); - } + final Set allowedACLs = getAdminACLs(destination); if (allowedACLs != null && !securityContext.isInOneOf(allowedACLs)) { return false; @@ -157,38 +154,12 @@ public void removeDestinationInfo(ConnectionContext context, DestinationInfo inf public Subscription addConsumer(ConnectionContext context, ConsumerInfo info) throws Exception { final SecurityContext securityContext = checkSecurityContext(context); - Set allowedACLs = null; - if (!info.getDestination().isTemporary()) { - allowedACLs = authorizationMap.getReadACLs(info.getDestination()); - } else { - allowedACLs = authorizationMap.getTempDestinationReadACLs(); - } + final Set allowedACLs = getReadACLs(info.getDestination()); if (!securityContext.isBrokerContext() && allowedACLs != null && !securityContext.isInOneOf(allowedACLs) ) { throw new SecurityException("User " + securityContext.getUserName() + " is not authorized to read from: " + info.getDestination()); } - /* - * Need to think about this a little more. We could do per message - * security checking to implement finer grained security checking. For - * example a user can only see messages with price>1000 . Perhaps this - * should just be another additional broker filter that installs this - * type of feature. If we did want to do that, then we would install a - * predicate. We should be careful since there may be an existing - * predicate already assigned and the consumer info may be sent to a - * remote broker, so it also needs to support being marshaled. - * info.setAdditionalPredicate(new BooleanExpression() { public boolean - * matches(MessageEvaluationContext message) throws JMSException { if( - * !subject.getAuthorizedReadDests().contains(message.getDestination()) ) { - * Set allowedACLs = - * authorizationMap.getReadACLs(message.getDestination()); - * if(allowedACLs!=null && !subject.isInOneOf(allowedACLs)) return - * false; subject.getAuthorizedReadDests().put(message.getDestination(), - * message.getDestination()); } return true; } public Object - * evaluate(MessageEvaluationContext message) throws JMSException { - * return matches(message) ? Boolean.TRUE : Boolean.FALSE; } }); - */ - return super.addConsumer(context, info); } @@ -197,13 +168,8 @@ public void addProducer(ConnectionContext context, ProducerInfo info) throws Exc final SecurityContext securityContext = checkSecurityContext(context); if (!securityContext.isBrokerContext() && info.getDestination() != null) { + final Set allowedACLs = getWriteACLs(info.getDestination()); - Set allowedACLs = null; - if (!info.getDestination().isTemporary()) { - allowedACLs = authorizationMap.getWriteACLs(info.getDestination()); - } else { - allowedACLs = authorizationMap.getTempDestinationWriteACLs(); - } if (allowedACLs != null && !securityContext.isInOneOf(allowedACLs)) { throw new SecurityException("User " + securityContext.getUserName() + " is not authorized to write to: " + info.getDestination()); } @@ -217,14 +183,8 @@ public void addProducer(ConnectionContext context, ProducerInfo info) throws Exc public void send(ProducerBrokerExchange producerExchange, Message messageSend) throws Exception { final SecurityContext securityContext = checkSecurityContext(producerExchange.getConnectionContext()); - if (!securityContext.isBrokerContext() && !securityContext.getAuthorizedWriteDests().containsValue(messageSend.getDestination())) { - - Set allowedACLs = null; - if (!messageSend.getDestination().isTemporary()) { - allowedACLs = authorizationMap.getWriteACLs(messageSend.getDestination()); - } else { - allowedACLs = authorizationMap.getTempDestinationWriteACLs(); - } + if (!securityContext.isBrokerContext() && !securityContext.getAuthorizedWriteDests().containsKey(messageSend.getDestination())) { + final Set allowedACLs = getWriteACLs(messageSend.getDestination()); if (allowedACLs != null && !securityContext.isInOneOf(allowedACLs)) { throw new SecurityException("User " + securityContext.getUserName() + " is not authorized to write to: " + messageSend.getDestination()); @@ -235,6 +195,73 @@ public void send(ProducerBrokerExchange producerExchange, Message messageSend) t super.send(producerExchange, messageSend); } + protected Set getReadACLs(ActiveMQDestination destination) { + Set allowedACLs; + + if (!destination.isTemporary()) { + // DefaultAuthorizationMap already handles composite destinations by + // returning the union of all the dests + allowedACLs = wcaSet(authorizationMap.getReadACLs(destination)); + } else { + allowedACLs = wcaSet(authorizationMap.getTempDestinationReadACLs()); + + // For temporary destinations we need to compute the union if composite + if (destination.isComposite()) { + for (ActiveMQDestination cd : destination.getCompositeDestinations()) { + allowedACLs = DestinationMap.union(allowedACLs, getReadACLs(cd)); + } + } + } + + return allowedACLs; + } + + protected Set getWriteACLs(ActiveMQDestination destination) { + Set allowedACLs; + if (!destination.isTemporary()) { + // DefaultAuthorizationMap already handles composite destinations by + // returning the union of all the dests + allowedACLs = wcaSet(authorizationMap.getWriteACLs(destination)); + } else { + allowedACLs = wcaSet(authorizationMap.getTempDestinationWriteACLs()); + // For temporary destinations we need to compute the union if composite + if (destination.isComposite()) { + for (ActiveMQDestination cd : destination.getCompositeDestinations()) { + allowedACLs = DestinationMap.union(allowedACLs, getWriteACLs(cd)); + } + } + } + + return allowedACLs; + } + + protected Set getAdminACLs(ActiveMQDestination destination) { + Set allowedACLs; + + if (!destination.isTemporary()) { + // DefaultAuthorizationMap already handles composite destinations by + // returning the union of all the dests + allowedACLs = wcaSet(authorizationMap.getAdminACLs(destination)); + } else { + allowedACLs = wcaSet(authorizationMap.getTempDestinationAdminACLs()); + // For temporary destinations we need to compute the union if composite + if (destination.isComposite()) { + for (ActiveMQDestination cd : destination.getCompositeDestinations()) { + allowedACLs = DestinationMap.union(allowedACLs, getAdminACLs(cd)); + } + } + } + + return allowedACLs; + } + + private static WildcardAwareSet wcaSet(Set acls) { + if (acls != null && !(acls instanceof WildcardAwareSet)) { + return new WildcardAwareSet<>(acls); + } + return (WildcardAwareSet) acls; + } + // SecurityAdminMBean interface // ------------------------------------------------------------------------- diff --git a/activemq-broker/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java b/activemq-broker/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java index 290f6bc07b5..b1807055fc9 100644 --- a/activemq-broker/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java +++ b/activemq-broker/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java @@ -266,9 +266,16 @@ public int hashCode() { return instance; } - class WildcardAwareSet extends HashSet { + static class WildcardAwareSet extends HashSet { boolean hasWildcard = false; + public WildcardAwareSet(Collection c) { + super(c); + } + + public WildcardAwareSet() { + } + @Override public boolean contains(Object e) { if (hasWildcard) { diff --git a/activemq-broker/src/test/java/org/apache/activemq/security/AuthorizationBrokerTest.java b/activemq-broker/src/test/java/org/apache/activemq/security/AuthorizationBrokerTest.java new file mode 100644 index 00000000000..97f67577cd4 --- /dev/null +++ b/activemq-broker/src/test/java/org/apache/activemq/security/AuthorizationBrokerTest.java @@ -0,0 +1,171 @@ +/* + * 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.activemq.security; + +import static org.junit.Assert.assertEquals; + +import java.util.Arrays; +import java.util.Set; +import java.util.function.Function; +import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.EmptyBroker; +import org.apache.activemq.broker.region.RegionBroker; +import org.apache.activemq.command.ActiveMQDestination; +import org.apache.activemq.command.ActiveMQQueue; +import org.apache.activemq.command.ActiveMQTempQueue; +import org.apache.activemq.command.ActiveMQTempTopic; +import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.filter.DestinationMap; +import org.apache.activemq.jaas.GroupPrincipal; +import org.apache.activemq.jaas.UserPrincipal; +import org.junit.Before; +import org.junit.Test; + +public class AuthorizationBrokerTest { + + private static final UserPrincipal USER1 = new UserPrincipal("user1"); + private static final UserPrincipal USER2 = new UserPrincipal("user2"); + private static final GroupPrincipal ADMINS = new GroupPrincipal("admins"); + + private AuthorizationBroker broker; + + @Before + public void setUp() throws Exception { + BrokerService brokerService = new BrokerService(); + brokerService.setPersistent(false); + brokerService.setUseJmx(false); + brokerService.setAdvisorySupport(false); + broker = new AuthorizationBroker(brokerService.getBroker(), createAuthorizationMap()); + } + + @Test + public void testReadACLs() throws Exception { + testAcls(broker::getReadACLs); + testTempAcls(broker::getReadACLs); + testAclsMixed(broker::getReadACLs); + } + + @Test + public void testWriteACLs() throws Exception { + testAcls(broker::getWriteACLs); + testTempAcls(broker::getWriteACLs); + testAclsMixed(broker::getWriteACLs); + } + + @Test + public void testAdminACLs() throws Exception { + testAcls(broker::getAdminACLs); + testTempAcls(broker::getWriteACLs); + testAclsMixed(broker::getWriteACLs); + } + + private void testAcls(Function> acls) throws Exception { + // Each user has access to their own dest + assertEquals(Set.of(ADMINS, USER1), acls.apply(q("USER1.dest.1"))); + assertEquals(Set.of(ADMINS, USER1), acls.apply(t("USER1.dest.1"))); + assertEquals(Set.of(ADMINS, USER2), acls.apply(q("USER2.dest.1"))); + assertEquals(Set.of(ADMINS, USER2), acls.apply(t("USER2.dest.1"))); + + // test composite, only admins should be authorized because each user only has permission + // for one of the destinations + assertEquals(Set.of(ADMINS), acls.apply(q("USER1.dest.1,USER2.dest.2"))); + assertEquals(Set.of(ADMINS), acls.apply(t("USER1.dest.1,USER2.dest.2"))); + // Composite should be authorized if all dests authorized for the user + assertEquals(Set.of(ADMINS, USER2), acls.apply(q("USER2.dest.1,USER2.dest.2"))); + assertEquals(Set.of(ADMINS, USER2), acls.apply(t("USER2.dest.1,USER2.dest.2"))); + } + + private void testAclsMixed(Function> acls) throws Exception { + // test composite, only admins should be authorized because each user only has permission + // for one of the destinations, mix queue/topic + assertEquals(Set.of(ADMINS), acls.apply(q("queue://USER1.dest.1,topic://USER2.dest.2"))); + assertEquals(Set.of(ADMINS), acls.apply(t("queue://USER1.dest.1,topic://USER2.dest.2"))); + + // Composite should be authorized if all dests authorized for the user. Mix queue/topic + assertEquals(Set.of(ADMINS, USER2), acls.apply(q("queue://USER2.dest.1,topic://USER2.dest.2"))); + assertEquals(Set.of(ADMINS, USER2), acls.apply(t("queue://USER2.dest.1,topic://USER2.dest.2"))); + + // Composite temp - mix non-temp with top level temp - user can only access their own plus temp + assertEquals(Set.of(ADMINS, USER1), acls.apply(tempQ("queue://USER1.dest.1,temp-topic://t.dest.2"))); + assertEquals(Set.of(ADMINS, USER1), acls.apply(tempT("topic://USER1.dest.1,temp-topic://t.dest.2"))); + + // Composite temp - each dest is checked and users only have access to one of 2 so only admins are allowed + assertEquals(Set.of(ADMINS), acls.apply(tempQ("queue://USER1.dest.1,topic://USER2.dest.1"))); + assertEquals(Set.of(ADMINS), acls.apply(tempT("topic://USER1.dest.1,topic://USER2.dest.1"))); + } + + private void testTempAcls(Function> acls) throws Exception { + // These are temp destinations so users to have access to all temp by any name + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempQ("temp.dest.1"))); + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempT("temp.dest.1"))); + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempQ("temp.dest.2"))); + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempT("temp.dest.2"))); + + // test composite mixed, but these are temp so name doesn't matter + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempQ("temp.dest.1,temp.dest.2"))); + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempT("temp.dest.1,temp.dest.2"))); + // Composite should be authorized if all temp topics + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempQ("t.dest.1,t.dest.2"))); + assertEquals(Set.of(ADMINS, USER1, USER2), acls.apply(tempT("t.dest.1,t.dest.2"))); + + // Test multi-level - top level dests are all temp and allowed but child should only be + // allowed for one + ActiveMQDestination dest = tempQ("t.dest.1,t.dest.2"); + Arrays.stream(dest.getCompositeDestinations()).findFirst().orElseThrow().setCompositeDestinations( + new ActiveMQDestination[]{new ActiveMQQueue("USER1.queue.1")}); + assertEquals(Set.of(ADMINS, USER1), acls.apply(dest)); + assertEquals(Set.of(ADMINS, USER1), acls.apply(dest)); + } + + private static AuthorizationMap createAuthorizationMap() throws Exception { + // Use the same ACLs for read, write, admin to test all 3 behave the same + var authorizationMap = new SimpleAuthorizationMap(createAclMap(), createAclMap(), createAclMap()); + var tempDestinationAuthorizationEntry = new TempDestinationAuthorizationEntry(); + tempDestinationAuthorizationEntry.setAdminACLs(Set.of(ADMINS, USER1, USER2)); + tempDestinationAuthorizationEntry.setReadACLs(Set.of(ADMINS, USER1, USER2)); + tempDestinationAuthorizationEntry.setWriteACLs(Set.of(ADMINS, USER1, USER2)); + authorizationMap.setTempDestinationAuthorizationEntry(tempDestinationAuthorizationEntry); + return authorizationMap; + } + + private static DestinationMap createAclMap() { + DestinationMap access = new DefaultAuthorizationMap(); + access.put(new ActiveMQQueue(">"), ADMINS); + access.put(new ActiveMQQueue("USER1.>"), USER1); + access.put(new ActiveMQQueue("USER2.>"), USER2); + access.put(new ActiveMQTopic(">"), ADMINS); + access.put(new ActiveMQTopic("USER1.>"), USER1); + access.put(new ActiveMQTopic("USER2.>"), USER2); + return access; + } + + private static ActiveMQQueue q(String queueName) { + return new ActiveMQQueue(queueName); + } + + private static ActiveMQTopic t(String topicName) { + return new ActiveMQTopic(topicName); + } + + private static ActiveMQTempQueue tempQ(String queueName) { + return new ActiveMQTempQueue(queueName); + } + + private static ActiveMQTempTopic tempT(String topicName) { + return new ActiveMQTempTopic(topicName); + } +} diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/security/SimpleSecurityBrokerSystemTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/security/SimpleSecurityBrokerSystemTest.java index a2e863cff43..c265a614418 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/security/SimpleSecurityBrokerSystemTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/security/SimpleSecurityBrokerSystemTest.java @@ -16,7 +16,12 @@ */ package org.apache.activemq.security; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.lang.management.ManagementFactory; +import java.lang.reflect.Method; import java.net.URL; import java.security.Principal; import java.util.Arrays; @@ -26,12 +31,22 @@ import java.util.Set; import junit.framework.Test; +import org.apache.activemq.ActiveMQConnection; +import org.apache.activemq.ActiveMQMessageTransformation; +import org.apache.activemq.ActiveMQSession; import org.apache.activemq.CombinationTestSupport; import org.apache.activemq.broker.Broker; import org.apache.activemq.broker.BrokerPlugin; import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.command.ActiveMQDestination; import org.apache.activemq.command.ActiveMQQueue; +import org.apache.activemq.command.ActiveMQTempQueue; +import org.apache.activemq.command.ActiveMQTempTopic; import org.apache.activemq.command.ActiveMQTopic; +import org.apache.activemq.command.ConsumerId; +import org.apache.activemq.command.ConsumerInfo; +import org.apache.activemq.command.ExceptionResponse; +import org.apache.activemq.command.Response; import org.apache.activemq.filter.DestinationMap; import org.apache.activemq.jaas.GroupPrincipal; import org.slf4j.Logger; @@ -111,6 +126,175 @@ public void testPopulateJMSXUserID() throws Exception { assertEquals("system", m.getStringProperty("JMSXUserID")); } + public void testComposite() throws Exception { + // user is authorized for both + destination = new ActiveMQQueue("USERS.queue.1,USERS.queue.2"); + Connection connection = factory.createConnection("user", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + sendMessages(session, destination, 1); + MessageConsumer consumer = session.createConsumer(destination); + // there should be 2 messages because a copy gets sent to each dest in the composite list + assertNotNull(consumer.receive(100)); + assertNotNull(consumer.receive(100)); + } + + public void testCompositeFail() throws Exception { + destination = new ActiveMQQueue("USERS.queue.1,GUEST.queue.2"); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + } + + public void testCompositeFailMulti() throws Exception { + destination = new ActiveMQQueue("GUEST.queue.1,GUEST.queue.2"); + // Add child composite that is not authorized + Arrays.stream(destination.getCompositeDestinations()).findFirst().orElseThrow().setCompositeDestinations( + new ActiveMQDestination[]{new ActiveMQQueue("USERS.queue.1")}); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + } + + public void testCompositeTempWriteDestsExist() throws Exception { + // pre-create dests + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("USERS.queue.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.1"), false); + + // Test that authorization works when dests already exist + testCompositeTempWrite(); + } + + public void testCompositeTempWrite() throws Exception { + // Test that authorization works on composite temp + destination = new ActiveMQTempQueue("queue://USERS.queue.1,queue://GUEST.queue.1"); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + JMSSecurityException e = assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + assertTrue(e.getMessage().contains("User guest is not authorized to write to:")); + } + + public void testCompositeTempMultiLevel() throws Exception { + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.2"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("USERS.queue.1"), false); + + // Test that authorization works on composite temp + // The top level composite list would be authorized, verify it fails because one of the + // composite dests itself contains a dest that isn't authorized + destination = new ActiveMQTempQueue("queue://GUEST.queue.1,queue://GUEST.queue.2"); + Arrays.stream(destination.getCompositeDestinations()).findFirst().orElseThrow().setCompositeDestinations( + new ActiveMQDestination[]{new ActiveMQQueue("USERS.queue.1")}); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + JMSSecurityException e = assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + assertTrue(e.getMessage().contains("User guest is not authorized to write to:")); + } + + public void testCompositeTempRead() throws Exception { + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("USERS.queue.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.1"), false); + + // Test that authorization works on composite temp + testCompositeTempRead(new ActiveMQTempQueue("queue://USERS.queue.1,queue://GUEST.queue.1")); + } + + public void testCompositeTempReadMultiLevel() throws Exception { + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.2"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("USERS.queue.1"), false); + + // test unauthorized dest that is a child of another composite fails + ActiveMQDestination dest = new ActiveMQTempQueue("queue://GUEST.queue.1,queue://GUEST.queue.2"); + Arrays.stream(dest.getCompositeDestinations()).findFirst().orElseThrow().setCompositeDestinations( + new ActiveMQDestination[]{new ActiveMQQueue("USERS.queue.1")}); + + // Test that authorization works on composite temp + testCompositeTempRead(dest); + } + + private void testCompositeTempRead(ActiveMQDestination dest) throws Exception { + destination = dest; + + // Test that authorization works on composite temp + ActiveMQConnection connection = (ActiveMQConnection) factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + // Consumers should be blocked + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + Method nextId = ActiveMQSession.class.getDeclaredMethod("getNextConsumerId"); + nextId.setAccessible(true); + + // the client normally blocks this so bypass the check to test the broker auth + ConsumerInfo info = new ConsumerInfo((ConsumerId) nextId.invoke(session)); + info.setDestination(destination); + Object result = connection.getTransport().request(info, 500); + assertTrue(result instanceof ExceptionResponse); + assertTrue(((Response) result).isException()); + assertTrue(((ExceptionResponse) result).getException() instanceof SecurityException); + assertTrue(((ExceptionResponse) result).getException().getMessage() + .contains("User guest is not authorized to read from")); + } + + public void testCompositeTempQueueWithTopics() throws Exception { + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQTopic("USERS.topic.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQTopic("GUEST.topic.1"), false); + + // Test that authorization works with contained topic instead of queues + destination = new ActiveMQTempQueue("topic://USERS.topic.1,topic://GUEST.topic.1"); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + JMSSecurityException e = assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + assertTrue(e.getMessage().contains("User guest is not authorized to write to:")); + } + + public void testCompositeTempTopicWithQueues() throws Exception { + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("USERS.queue.1"), false); + broker.getBroker().addDestination(broker.getAdminConnectionContext(), + new ActiveMQQueue("GUEST.queue.1"), false); + + // Test that authorization works with contained queues when temp is of type topic + destination = new ActiveMQTempTopic("queue://USERS.queue.1,queue://GUEST.queue.1"); + Connection connection = factory.createConnection("guest", "password"); + connections.add(connection); + connection.start(); + + Session session = connection.createSession(false, Session.AUTO_ACKNOWLEDGE); + JMSSecurityException e = assertThrows(JMSSecurityException.class, () -> sendMessages(session, destination, 1)); + assertTrue(e.getMessage().contains("User guest is not authorized to write to:")); + } + public static AuthorizationMap createAuthorizationMap() { DestinationMap readAccess = new DefaultAuthorizationMap(); readAccess.put(new ActiveMQQueue(">"), ADMINS); @@ -141,7 +325,19 @@ public static AuthorizationMap createAuthorizationMap() { adminAccess.put(new ActiveMQQueue(">"), USERS); adminAccess.put(new ActiveMQQueue(">"), GUESTS); - return new SimpleAuthorizationMap(writeAccess, readAccess, adminAccess); + var authorizationMap = new SimpleAuthorizationMap(writeAccess, readAccess, adminAccess); + var tempDestinationAuthorizationEntry = new TempDestinationAuthorizationEntry(); + try { + tempDestinationAuthorizationEntry.setAdmin("admins, users, guests"); + tempDestinationAuthorizationEntry.setRead("admins, users, guests"); + tempDestinationAuthorizationEntry.setWrite("admins, users, guests"); + } catch (Exception e) { + fail(e.getMessage()); + } + authorizationMap.setTempDestinationAuthorizationEntry(tempDestinationAuthorizationEntry); + + return authorizationMap; + } public static class SimpleAuthenticationFactory implements BrokerPlugin {