From 49f8846861bf31553ca9a45168be26c5e41ce36a Mon Sep 17 00:00:00 2001
From: Justin Bertram <jbertram@apache.org>
Date: Thu, 26 Jan 2023 22:36:55 -0600
Subject: [PATCH] ARTEMIS-4146 reauthenticated subjects are not cached

---
 .../core/security/impl/SecurityStoreImpl.java |  8 ++++-
 .../integration/security/SecurityTest.java    | 36 +++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
index e364723121..ca671dfaac 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java
@@ -408,7 +408,13 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
        * successfully authenticate before requesting authorization for anything.
        */
       if (cached == null) {
-         return securityManager.authenticate(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection(), auth.getSecurityDomain());
+         try {
+            Subject subject = securityManager.authenticate(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection(), auth.getSecurityDomain());
+            authenticationCache.put(createAuthenticationCacheKey(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection()), new Pair<>(subject != null, subject));
+            return subject;
+         } catch (NoCacheLoginException e) {
+            return null;
+         }
       }
       return cached.getB();
    }
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
index 7923c6e37e..30a9287ea9 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/security/SecurityTest.java
@@ -2569,6 +2569,42 @@ public class SecurityTest extends ActiveMQTestBase {
       }
    }
 
+   @Test
+   public void testReauthenticationIsCached() throws Exception {
+      ActiveMQServer server = createServer();
+      server.start();
+
+      HierarchicalRepository<Set<Role>> securityRepository = server.getSecurityRepository();
+      ActiveMQJAASSecurityManager securityManager = (ActiveMQJAASSecurityManager) server.getSecurityManager();
+      securityManager.getConfiguration().addUser("auser", "pass");
+      Role role = new Role("arole", true, false, false, false, false, false, false, false, true, false);
+      Set<Role> roles = new HashSet<>();
+      roles.add(role);
+      securityRepository.addMatch(SecurityTest.addressA, roles);
+      securityManager.getConfiguration().addRole("auser", "arole");
+      server.createQueue(new QueueConfiguration(SecurityTest.queueA).setAddress(SecurityTest.addressA));
+
+      ((SecurityStoreImpl)server.getSecurityStore()).invalidateAuthenticationCache();
+      ((SecurityStoreImpl)server.getSecurityStore()).invalidateAuthorizationCache();
+
+      locator.setBlockOnNonDurableSend(true);
+      ClientSessionFactory cf = createSessionFactory(locator);
+      ClientSession session = cf.createSession("auser", "pass", false, true, true, false, -1);
+      ClientProducer cp = session.createProducer(SecurityTest.addressA);
+      cp.send(session.createMessage(false));
+
+      assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize());
+      assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthorizationCacheSize());
+
+      ((SecurityStoreImpl)server.getSecurityStore()).invalidateAuthenticationCache();
+      ((SecurityStoreImpl)server.getSecurityStore()).invalidateAuthorizationCache();
+
+      cp.send(session.createMessage(false));
+
+      assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthenticationCacheSize());
+      assertEquals(1, ((SecurityStoreImpl)server.getSecurityStore()).getAuthorizationCacheSize());
+   }
+
    // Check the user connection has both send and receive permissions on the queue
    private void checkUserSendAndReceive(final String genericQueueName,
                                         final ClientSession connection) throws Exception {