Skip to content

[5.19.x] Ensure composite temp dests are handled in AuthorizationBroker (#2219) #2221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cshannon merged 1 commit into from
Jul 15, 2026
Merged

[5.19.x] Ensure composite temp dests are handled in AuthorizationBroker (#2219) #2221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 74 additions & 47 deletions activemq-broker/src/main/java/org/apache/activemq/security/AuthorizationBroker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand All @@ -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());
}
Expand All @@ -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());
Expand All @@ -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
// -------------------------------------------------------------------------

Expand Down
9 changes: 8 additions & 1 deletion activemq-broker/src/main/java/org/apache/activemq/security/DefaultAuthorizationMap.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,16 @@ public int hashCode() {
return instance;
}

class WildcardAwareSet<T> extends HashSet<T> {
static class WildcardAwareSet<T> extends HashSet<T> {
boolean hasWildcard = false;

public WildcardAwareSet(Collection<? extends T> c) {
super(c);
}

public WildcardAwareSet() {
}

@Override
public boolean contains(Object e) {
if (hasWildcard) {
Expand Down
171 changes: 171 additions & 0 deletions activemq-broker/src/test/java/org/apache/activemq/security/AuthorizationBrokerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<ActiveMQDestination, Set<?>> 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<ActiveMQDestination, Set<?>> 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<ActiveMQDestination, Set<?>> 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);
}
}
Loading
Loading