From 84739466464a7710ba493a1e8a7cb57c29ec7078 Mon Sep 17 00:00:00 2001
From: Justin Bertram <jbertram@apache.org>
Date: Tue, 4 Aug 2020 08:20:06 -0500
Subject: [PATCH] ARTEMIS-2865 LegacyLDAPSecuritySettingPlugin can change
 default security match

---
 .../impl/LegacyLDAPSecuritySettingPlugin.java |  11 +-
 .../core/settings/HierarchicalRepository.java |   6 ++
 .../impl/HierarchicalObjectRepository.java    |   9 ++
 ...LDAPSecuritySettingPluginListenerTest.java | 101 +++++++++++++++++-
 4 files changed, 122 insertions(+), 5 deletions(-)

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 183d794e97..d9d61add68 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
@@ -463,8 +463,15 @@ public class LegacyLDAPSecuritySettingPlugin implements SecuritySettingPlugin {
          processSearchResult(newRoles, (SearchResult) namingEvent.getNewBinding());
          for (Map.Entry<String, Set<Role>> entry : newRoles.entrySet()) {
             Set<Role> existingRoles = securityRepository.getMatch(entry.getKey());
-            for (Role role : entry.getValue()) {
-               existingRoles.add(role);
+            // see if this the *actual* default object, not just "equals"; we don't want to change the default security match
+            if (existingRoles != securityRepository.getDefault()) {
+               for (Role role : entry.getValue()) {
+                  logger.debug("adding role " + role + " to existing roles " + existingRoles + " at " + entry.getKey());
+                  existingRoles.add(role);
+               }
+            } else {
+               logger.debug("adding new roles " + entry.getValue() + " at " + entry.getKey());
+               securityRepository.addMatch(entry.getKey(), entry.getValue());
             }
          }
       } catch (NamingException e) {
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java
index cb2054a922..71379ea88e 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/HierarchicalRepository.java
@@ -62,6 +62,12 @@ public interface HierarchicalRepository<T> {
     */
    void setDefault(T defaultValue);
 
+   /**
+    *
+    * @return the default match for this repo
+    */
+   T getDefault();
+
    /**
     * remove a match from the repository
     *
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java
index 3ed69d037c..d6b751df3a 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/settings/impl/HierarchicalObjectRepository.java
@@ -348,6 +348,15 @@ public class HierarchicalObjectRepository<T> implements HierarchicalRepository<T
       defaultmatch = defaultValue;
    }
 
+   /**
+    *
+    * @return the default match for this repo
+    */
+   @Override
+   public T getDefault() {
+      return defaultmatch;
+   }
+
    @Override
    public void clear() {
       lock.writeLock().lock();
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
index d7b227b283..3ad737b2a6 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/LegacyLDAPSecuritySettingPluginListenerTest.java
@@ -35,6 +35,7 @@ import java.util.Map;
 
 import org.apache.activemq.artemis.api.core.ActiveMQException;
 import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
 import org.apache.activemq.artemis.api.core.SimpleString;
 import org.apache.activemq.artemis.api.core.TransportConfiguration;
 import org.apache.activemq.artemis.api.core.client.ActiveMQClient;
@@ -49,7 +50,6 @@ import org.apache.activemq.artemis.core.remoting.impl.invm.InVMAcceptorFactory;
 import org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnectorFactory;
 import org.apache.activemq.artemis.core.server.ActiveMQServer;
 import org.apache.activemq.artemis.core.server.ActiveMQServers;
-import org.apache.activemq.artemis.api.core.RoutingType;
 import org.apache.activemq.artemis.core.server.impl.LegacyLDAPSecuritySettingPlugin;
 import org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager;
 import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
@@ -361,7 +361,7 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
       // authentication should succeed now, but authorization for sending should still fail
       try {
          ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
-         ClientProducer producer = session.createProducer(SimpleString.toSimpleString(queue));
+         ClientProducer producer = session.createProducer(queue);
          producer.send(session.createMessage(true));
          Assert.fail("Producing here should fail due to the original security data.");
       } catch (ActiveMQException e) {
@@ -377,9 +377,104 @@ public class LegacyLDAPSecuritySettingPluginListenerTest extends AbstractLdapTes
       }
 
       ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
-      ClientProducer producer = session.createProducer(SimpleString.toSimpleString(queue));
+      ClientProducer producer = session.createProducer(queue);
       producer.send(session.createMessage(true));
 
       cf.close();
    }
+
+   @Test
+   public void testNewUserAndRoleWithNewDestination() throws Exception {
+      server.getConfiguration().setSecurityInvalidationInterval(0);
+      server.start();
+      ClientSessionFactory cf = locator.createSessionFactory();
+
+      // these queue names actually matter based on what's in the ldif
+      String goodQueue = "queue3";
+      String badQueue = "queue4";
+
+      // authentication should fail
+      try {
+         cf.createSession("third", "secret", false, true, true, false, 0);
+         Assert.fail("Creating a session here should fail due to the original security data.");
+      } catch (ActiveMQException e) {
+         Assert.assertTrue(e.getMessage().contains("229031")); // authentication exception
+      }
+
+      { // add new user
+         DirContext ctx = getContext();
+         BasicAttributes basicAttributes = new BasicAttributes();
+         basicAttributes.put("userPassword", "secret");
+         Attribute objclass = new BasicAttribute("objectclass");
+         objclass.add("top");
+         objclass.add("simpleSecurityObject");
+         objclass.add("account");
+         basicAttributes.put(objclass);
+         ctx.bind("uid=third,ou=system", null, basicAttributes);
+      }
+
+      { // add new role
+         DirContext ctx = getContext();
+         BasicAttributes basicAttributes = new BasicAttributes();
+         basicAttributes.put("member", "uid=third,ou=system");
+         Attribute objclass = new BasicAttribute("objectclass");
+         objclass.add("top");
+         objclass.add("groupOfNames");
+         basicAttributes.put(objclass);
+         ctx.bind("cn=role3,ou=system", null, basicAttributes);
+      }
+
+      // authentication should succeed now, but authorization for sending should still fail
+      try {
+         ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
+         ClientProducer producer = session.createProducer(goodQueue);
+         producer.send(session.createMessage(true));
+         Assert.fail("Producing here should fail due to the original security data.");
+      } catch (ActiveMQException e) {
+         Assert.assertTrue(e.getMessage().contains("229032")); // authorization exception
+      }
+
+      { // add new destination
+         DirContext ctx = getContext();
+         BasicAttributes basicAttributes = new BasicAttributes();
+         Attribute objclass = new BasicAttribute("objectclass");
+         objclass.add("top");
+         objclass.add("applicationProcess");
+         basicAttributes.put(objclass);
+         ctx.bind("cn=" + goodQueue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
+      }
+
+      { // add permissions for new destination
+         DirContext ctx = getContext();
+         BasicAttributes basicAttributes = new BasicAttributes();
+         basicAttributes.put("uniquemember", "cn=role3");
+         Attribute objclass = new BasicAttribute("objectclass");
+         objclass.add("top");
+         objclass.add("groupOfUniqueNames");
+         basicAttributes.put(objclass);
+         ctx.bind("cn=write,cn=" + goodQueue + ",ou=queues,ou=destinations,o=ActiveMQ,ou=system", null, basicAttributes);
+      }
+
+      server.createQueue(new QueueConfiguration(goodQueue).setRoutingType(RoutingType.ANYCAST).setDurable(false));
+
+      ClientSession session = cf.createSession("third", "secret", false, true, true, false, 0);
+      ClientProducer producer = session.createProducer(goodQueue);
+      producer.send(session.createMessage(true));
+      session.close();
+      producer.close();
+
+      server.createQueue(new QueueConfiguration(badQueue).setRoutingType(RoutingType.ANYCAST).setDurable(false));
+
+      // authorization for sending should fail for the new queue
+      try {
+         session = cf.createSession("third", "secret", false, true, true, false, 0);
+         producer = session.createProducer(badQueue);
+         producer.send(session.createMessage(true));
+         Assert.fail("Producing here should fail.");
+      } catch (ActiveMQException e) {
+         Assert.assertTrue(e.getMessage().contains("229032")); // authorization exception
+      }
+
+      cf.close();
+   }
 }