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.
This commit is contained in:
parent
16e97125d1
commit
dbd2e9342f
|
@ -442,16 +442,16 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
* @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.
|
||||
|
|
|
@ -18,10 +18,15 @@ package org.apache.activemq.artemis.core.security.impl;
|
|||
|
||||
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.RolePrincipal;
|
|||
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 class SecurityStoreImplTest {
|
|||
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);
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue