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 ec7b50ca8ab..abec6465866 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 @@ -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 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 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. diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java index 016aa944015..5dc808e07c1 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/security/impl/SecurityStoreImplTest.java +++ b/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) () -> { + try { + securityStore.check(SimpleString.of("test"), CheckType.VIEW, session); + return true; + } catch (Exception ignore) { + return false; + } + }); + + assertEquals(true, checkResult); + } }