From 2123f85ea967de85520b07ce47ef807c23c8f4d0 Mon Sep 17 00:00:00 2001 From: Justin Bertram Date: Thu, 1 Mar 2018 11:25:56 -0600 Subject: [PATCH] ARTEMIS-1717 create/delete address permissions ignored in broker.xml --- .../activemq/artemis/core/security/Role.java | 17 ++++++------ .../impl/FileConfigurationParser.java | 2 +- .../impl/LegacyLDAPSecuritySettingPlugin.java | 14 ++++++++-- .../config/impl/FileConfigurationTest.java | 27 +++++++++---------- docs/user-manual/en/security.md | 15 ++++++++--- .../amqp/AmqpClientTestSupport.java | 8 +++--- .../integration/jms/SimpleJNDIClientTest.java | 2 +- .../mqtt/imported/MQTTTestSupport.java | 8 +++--- .../ra/OutgoingConnectionNoJTATest.java | 2 +- tests/jms-tests/src/test/resources/broker.xml | 2 ++ 10 files changed, 58 insertions(+), 39 deletions(-) diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/security/Role.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/security/Role.java index a3b4c21b19..7757b3ba24 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/security/Role.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/security/Role.java @@ -79,6 +79,7 @@ public class Role implements Serializable { this(name, send, consume, createDurableQueue, deleteDurableQueue, createNonDurableQueue, deleteNonDurableQueue, manage, consume); } + @Deprecated public Role(final String name, final boolean send, final boolean consume, @@ -156,6 +157,14 @@ public class Role implements Serializable { return deleteNonDurableQueue; } + public boolean isManage() { + return manage; + } + + public boolean isBrowse() { + return browse; + } + @Override public String toString() { StringBuffer stringReturn = new StringBuffer("Role {name=" + name + "; allows=["); @@ -260,12 +269,4 @@ public class Role implements Serializable { result = 31 * result + (browse ? 1 : 0); return result; } - - public boolean isManage() { - return manage; - } - - public boolean isBrowse() { - return browse; - } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java index 8500ecf1fb..633a7168c6 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/deployers/impl/FileConfigurationParser.java @@ -871,7 +871,7 @@ public final class FileConfigurationParser extends XMLConfigurationUtil { } for (String role : allRoles) { - securityRoles.add(new Role(role, send.contains(role), consume.contains(role), createDurableQueue.contains(role), deleteDurableQueue.contains(role), createNonDurableQueue.contains(role), deleteNonDurableQueue.contains(role), manageRoles.contains(role), browseRoles.contains(role))); + securityRoles.add(new Role(role, send.contains(role), consume.contains(role), createDurableQueue.contains(role), deleteDurableQueue.contains(role), createNonDurableQueue.contains(role), deleteNonDurableQueue.contains(role), manageRoles.contains(role), browseRoles.contains(role), createAddressRoles.contains(role), deleteAddressRoles.contains(role))); } return securityMatch; diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java index 6aa762ed31..e38c337e88 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/LegacyLDAPSecuritySettingPlugin.java @@ -366,8 +366,18 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin { Rdn rdn = ldapname.getRdn(ldapname.size() - 1); String roleName = rdn.getValue().toString(); logger.debug("\tRole name: " + roleName); - Role role = new Role(roleName, permissionType.equalsIgnoreCase(writePermissionValue), permissionType.equalsIgnoreCase(readPermissionValue), permissionType.equalsIgnoreCase(adminPermissionValue), permissionType.equalsIgnoreCase(adminPermissionValue), permissionType.equalsIgnoreCase(adminPermissionValue), permissionType.equalsIgnoreCase(adminPermissionValue), false, // there is no permission from ActiveMQ 5.x that corresponds to the "manage" permission in ActiveMQ Artemis - permissionType.equalsIgnoreCase(readPermissionValue)); // the "browse" permission matches "read" from ActiveMQ 5.x + Role role = new Role(roleName, + permissionType.equalsIgnoreCase(writePermissionValue), // send + permissionType.equalsIgnoreCase(readPermissionValue), // consume + permissionType.equalsIgnoreCase(adminPermissionValue), // createDurableQueue + permissionType.equalsIgnoreCase(adminPermissionValue), // deleteDurableQueue + permissionType.equalsIgnoreCase(adminPermissionValue), // createNonDurableQueue + permissionType.equalsIgnoreCase(adminPermissionValue), // deleteNonDurableQueue + false, // manage - there is no permission from ActiveMQ 5.x that corresponds to this + permissionType.equalsIgnoreCase(readPermissionValue), // browse + permissionType.equalsIgnoreCase(adminPermissionValue), // createAddress + permissionType.equalsIgnoreCase(adminPermissionValue) // deleteAddress + ); roles.add(role); } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java index af48c62c97..4cdd11c477 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/config/impl/FileConfigurationTest.java @@ -508,63 +508,62 @@ public class FileConfigurationTest extends ConfigurationImplTest { Map> securityRoles = fc.getSecurityRoles(); Set roles = securityRoles.get("#"); - //N.B. - FileConfigurationParser uses the constructor without createAddress and deleteAddress //cn=mygroup,dc=local,dc=com = amq1 Role testRole1 = new Role("cn=mygroup,dc=local,dc=com",false, false, false, false, true, false, false, - false); + false, false, false); //myrole1 = amq1 + amq2 Role testRole2 = new Role("myrole1",false, false, false, false, true, true, false, - false); + false, false, false); //myrole3 = amq3 + amq4 Role testRole3 = new Role("myrole3",false, false, true, true, false, false, false, - false); + false, false, false); //myrole4 = amq5 + amq!@#$%^&*() + amq6 Role testRole4 = new Role("myrole4",true, true, false, false, false, false, false, - true); + true, true, true); //myrole5 = amq4 = amq3 + amq4 Role testRole5 = new Role("myrole5",false, false, true, true, false, false, false, - false); + false, false, false); Role testRole6 = new Role("amq1",false, false, false, false, true, false, false, - false); + false, false, false); Role testRole7 = new Role("amq2",false, false, false, false, false, true, false, - false); + false, false, false); Role testRole8 = new Role("amq3",false, false, true, false, false, false, false, - false); + false, false, false); Role testRole9 = new Role("amq4",false, false, true, true, false, false, false, - false); + false, false, false); Role testRole10 = new Role("amq5",false, false, false, false, false, false, false, - false); + false, true, true); Role testRole11 = new Role("amq6",false, true, false, false, false, false, false, - true); + true, false, false); Role testRole12 = new Role("amq7",false, false, false, false, false, false, true, - false); + false, false, false); Role testRole13 = new Role("amq!@#$%^&*()",true, false, false, false, false, false, false, - false); + false, false, false); assertEquals(13, roles.size()); assertTrue(roles.contains(testRole1)); diff --git a/docs/user-manual/en/security.md b/docs/user-manual/en/security.md index d1639608dc..82b062d834 100644 --- a/docs/user-manual/en/security.md +++ b/docs/user-manual/en/security.md @@ -35,6 +35,12 @@ wildcard match can be used using the wildcard characters '`#`' and Eight different permissions can be given to the set of queues which match the address. Those permissions are: +- `createAddress`. This permission allows the user to create an + address fitting the `match`. + +- `deleteAddress`. This permission allows the user to delete an + address fitting the `match`. + - `createDurableQueue`. This permission allows the user to create a durable queue under matching addresses. @@ -225,13 +231,14 @@ The name of the queue or topic defined in LDAP will serve as the "match" for the will be mapped from the ActiveMQ 5.x type to the Artemis type, and the role will be mapped as-is. ActiveMQ 5.x only has 3 permission types - `read`, `write`, and `admin`. These permission types are described on their -[website](http://activemq.apache.org/security.html). However, as described previously, ActiveMQ Artemis has 7 permission -types - `createDurableQueue`, `deleteDurableQueue`, `createNonDurableQueue`, `deleteNonDurableQueue`, `send`, `consume`, -`browse`, and `manage`. Here's how the old types are mapped to the new types: +[website](http://activemq.apache.org/security.html). However, as described previously, ActiveMQ Artemis has 9 permission +types - `createAddress`, `deleteAddress`, `createDurableQueue`, `deleteDurableQueue`, `createNonDurableQueue`, +`deleteNonDurableQueue`, `send`, `consume`, `browse`, and `manage`. Here's how the old types are mapped to the new types: - `read` - `consume`, `browse` - `write` - `send` -- `admin` - `createDurableQueue`, `deleteDurableQueue`, `createNonDurableQueue`, `deleteNonDurableQueue` +- `admin` - `createAddress`, `deleteAddress`, `createDurableQueue`, `deleteDurableQueue`, `createNonDurableQueue`, + `deleteNonDurableQueue` As mentioned, there are a few places where a translation was performed to achieve some equivalence.: diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpClientTestSupport.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpClientTestSupport.java index a01c975de8..991f467523 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpClientTestSupport.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/AmqpClientTestSupport.java @@ -256,10 +256,10 @@ public class AmqpClientTestSupport extends AmqpTestSupport { // Configure roles HierarchicalRepository> securityRepository = server.getSecurityRepository(); HashSet value = new HashSet<>(); - value.add(new Role("nothing", false, false, false, false, false, false, false, false)); - value.add(new Role("browser", false, false, false, false, false, false, false, true)); - value.add(new Role("guest", false, true, false, false, false, false, false, true)); - value.add(new Role("full", true, true, true, true, true, true, true, true)); + value.add(new Role("nothing", false, false, false, false, false, false, false, false, false, false)); + value.add(new Role("browser", false, false, false, false, false, false, false, true, false, false)); + value.add(new Role("guest", false, true, false, false, false, false, false, true, false, false)); + value.add(new Role("full", true, true, true, true, true, true, true, true, true, true)); securityRepository.addMatch(getQueueName(), value); server.getConfiguration().setSecurityEnabled(true); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java index d776961758..54109b1e9f 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/SimpleJNDIClientTest.java @@ -486,7 +486,7 @@ public class SimpleJNDIClientTest extends ActiveMQTestBase { //setup user and role on broker ((ActiveMQJAASSecurityManager) liveService.getSecurityManager()).getConfiguration().addUser("myUser", "myPassword"); ((ActiveMQJAASSecurityManager) liveService.getSecurityManager()).getConfiguration().addRole("myUser", "consumeCreateRole"); - Role consumeCreateRole = new Role("consumeCreateRole", false, true, true, true, true, true, true, true); + Role consumeCreateRole = new Role("consumeCreateRole", false, true, true, true, true, true, true, true, true, true); Set consumerCreateRoles = new HashSet<>(); consumerCreateRoles.add(consumeCreateRole); liveService.getSecurityRepository().addMatch("test.queue", consumerCreateRoles); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTestSupport.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTestSupport.java index 6ad13f1c14..e49ec92ed6 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTestSupport.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/mqtt/imported/MQTTTestSupport.java @@ -181,10 +181,10 @@ public class MQTTTestSupport extends ActiveMQTestBase { // Configure roles HierarchicalRepository> securityRepository = server.getSecurityRepository(); HashSet value = new HashSet<>(); - value.add(new Role("nothing", false, false, false, false, false, false, false, false)); - value.add(new Role("browser", false, false, false, false, false, false, false, true)); - value.add(new Role("guest", false, true, false, false, false, false, false, true)); - value.add(new Role("full", true, true, true, true, true, true, true, true)); + value.add(new Role("nothing", false, false, false, false, false, false, false, false, false, false)); + value.add(new Role("browser", false, false, false, false, false, false, false, true, false, false)); + value.add(new Role("guest", false, true, false, false, false, false, false, true, false, false)); + value.add(new Role("full", true, true, true, true, true, true, true, true, true, true)); securityRepository.addMatch(getQueueName(), value); server.getConfiguration().setSecurityEnabled(true); diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/OutgoingConnectionNoJTATest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/OutgoingConnectionNoJTATest.java index 2cc129f1e1..430c0f3fda 100644 --- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/OutgoingConnectionNoJTATest.java +++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/ra/OutgoingConnectionNoJTATest.java @@ -71,7 +71,7 @@ public class OutgoingConnectionNoJTATest extends ActiveMQRATestBase { ((ActiveMQJAASSecurityManager) server.getSecurityManager()).getConfiguration().setDefaultUser("guest"); ((ActiveMQJAASSecurityManager) server.getSecurityManager()).getConfiguration().addRole("testuser", "arole"); ((ActiveMQJAASSecurityManager) server.getSecurityManager()).getConfiguration().addRole("guest", "arole"); - Role role = new Role("arole", true, true, true, true, true, true, true, true); + Role role = new Role("arole", true, true, true, true, true, true, true, true, true, true); Set roles = new HashSet<>(); roles.add(role); server.getSecurityRepository().addMatch(MDBQUEUEPREFIXED, roles); diff --git a/tests/jms-tests/src/test/resources/broker.xml b/tests/jms-tests/src/test/resources/broker.xml index 28550ae647..733e8c37bc 100644 --- a/tests/jms-tests/src/test/resources/broker.xml +++ b/tests/jms-tests/src/test/resources/broker.xml @@ -44,6 +44,8 @@ + +