ARTEMIS-5187 Fix management authorization checks after authentication…

… failures

When the ArtemisRbacMBeanServerBuilder class is used for the RBAC management,
a clash of authentication cache keys between clients failing authentication and
web console authenticated users can cause web console authenticated users to
receive authorization errors, blank screens and similar issues after successful
login to the console.

artemis-server/src/main/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImpl.java

@@ -442,16 +442,16 @@ private void authenticationFailed(String user, RemotingConnection connection) th
     * @return the authenticated Subject with all associated role principals
     */
    private Subject getSubjectForAuthorization(SecurityAuth auth, ActiveMQSecurityManager5 securityManager) {
-      String authnCacheKey = createAuthenticationCacheKey(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection());
-      Pair<Boolean, Subject> cached = getAuthenticationCacheEntry(authnCacheKey);
-
-      if (cached == null && auth.getUsername() == null && auth.getPassword() == null && auth.getRemotingConnection() instanceof ManagementRemotingConnection) {
+      if (auth.getUsername() == null && auth.getPassword() == null && auth.getRemotingConnection() instanceof ManagementRemotingConnection) {
          AccessControlContext accessControlContext = AccessController.getContext();
          if (accessControlContext != null) {
-            cached = new Pair<>(true, Subject.getSubject(accessControlContext));
+            return Subject.getSubject(accessControlContext);
          }
       }
 
+      String authnCacheKey = createAuthenticationCacheKey(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection());
+      Pair<Boolean, Subject> cached = getAuthenticationCacheEntry(authnCacheKey);
+
       /*
        * We don't need to worry about the cached boolean being false as users always have to
        * successfully authenticate before requesting authorization for anything.

artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java

@@ -18,10 +18,15 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import javax.security.auth.Subject;
+import java.security.PrivilegedExceptionAction;
 import java.util.Set;
 
+import org.apache.activemq.artemis.api.core.ActiveMQSecurityException;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.management.impl.ManagementRemotingConnection;
 import org.apache.activemq.artemis.core.security.CheckType;
 import org.apache.activemq.artemis.core.security.Role;
 import org.apache.activemq.artemis.core.security.SecurityAuth;
@@ -32,6 +37,8 @@
 import org.apache.activemq.artemis.spi.core.security.jaas.UserPrincipal;
 import org.apache.activemq.artemis.utils.RandomUtil;
 import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
 
 public class SecurityStoreImplTest {
 
@@ -113,4 +120,49 @@ public void getCaller() throws Exception {
       assertEquals("joe", securityStore.getCaller("joe", subject));
    }
 
+   @Test
+   public void testManagementAuthorizationAfterNullAuthenticationFailure() throws Exception {
+      ActiveMQSecurityManager5 securityManager = Mockito.mock(ActiveMQSecurityManager5.class);
+      Mockito.when(securityManager.authorize(ArgumentMatchers.any(Subject.class),
+          ArgumentMatchers.isNull(),
+          ArgumentMatchers.any(CheckType.class),
+          ArgumentMatchers.anyString())).
+          thenReturn(true);
+
+      SecurityStoreImpl securityStore = new SecurityStoreImpl(
+          new HierarchicalObjectRepository<>(),
+          securityManager,
+          10000,
+          true,
+          "",
+          null,
+          null,
+          1000,
+          1000);
+
+      try {
+         securityStore.authenticate(null, null, Mockito.mock(RemotingConnection.class), null);
+         fail("Authentication must fail");
+      } catch (Throwable t) {
+         assertEquals(ActiveMQSecurityException.class, t.getClass());
+      }
+
+      SecurityAuth session = Mockito.mock(SecurityAuth.class);
+      Mockito.when(session.getRemotingConnection()).thenReturn(new ManagementRemotingConnection());
+
+      Subject viewSubject = new Subject();
+      viewSubject.getPrincipals().add(new UserPrincipal("v"));
+      viewSubject.getPrincipals().add(new RolePrincipal("viewers"));
+
+      Object checkResult = Subject.doAs(viewSubject, (PrivilegedExceptionAction<Object>) () -> {
+         try {
+            securityStore.check(SimpleString.of("test"), CheckType.VIEW, session);
+            return true;
+         } catch (Exception ignore) {
+            return false;
+         }
+      });
+
+      assertEquals(true, checkResult);
+   }
 }