ARTEMIS-4399 fix disabled authn/z cache
This commit is contained in:
Justin Bertram
Robbie Gemmell
parent
61a8e19ecd
commit
29fafb5fed
|
|
@ -98,14 +98,22 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
this.managementClusterUser = managementClusterUser;
|
||||
this.managementClusterPassword = managementClusterPassword;
|
||||
this.notificationService = notificationService;
|
||||
authenticationCache = CacheBuilder.newBuilder()
|
||||
.maximumSize(authenticationCacheSize)
|
||||
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
|
||||
.build();
|
||||
authorizationCache = CacheBuilder.newBuilder()
|
||||
.maximumSize(authorizationCacheSize)
|
||||
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
|
||||
.build();
|
||||
if (authenticationCacheSize == 0) {
|
||||
authenticationCache = null;
|
||||
} else {
|
||||
authenticationCache = CacheBuilder.newBuilder()
|
||||
.maximumSize(authenticationCacheSize)
|
||||
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
|
||||
.build();
|
||||
}
|
||||
if (authorizationCacheSize == 0) {
|
||||
authorizationCache = null;
|
||||
} else {
|
||||
authorizationCache = CacheBuilder.newBuilder()
|
||||
.maximumSize(authorizationCacheSize)
|
||||
.expireAfterWrite(invalidationInterval, TimeUnit.MILLISECONDS)
|
||||
.build();
|
||||
}
|
||||
this.securityRepository.registerListener(this);
|
||||
}
|
||||
|
||||
|
|
@ -159,7 +167,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
boolean check = true;
|
||||
|
||||
Subject subject = null;
|
||||
Pair<Boolean, Subject> cacheEntry = authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password, connection));
|
||||
Pair<Boolean, Subject> cacheEntry = getAuthenticationCacheEntry(user, password, connection);
|
||||
if (cacheEntry != null) {
|
||||
if (!cacheEntry.getA()) {
|
||||
// cached authentication failed previously so don't check again
|
||||
|
|
@ -186,7 +194,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
if (securityManager instanceof ActiveMQSecurityManager5) {
|
||||
try {
|
||||
subject = ((ActiveMQSecurityManager5) securityManager).authenticate(user, password, connection, securityDomain);
|
||||
authenticationCache.put(createAuthenticationCacheKey(user, password, connection), new Pair<>(subject != null, subject));
|
||||
putAuthenticationCacheEntry(user, password, connection, subject);
|
||||
validatedUser = getUserFromSubject(subject);
|
||||
} catch (NoCacheLoginException e) {
|
||||
handleNoCacheLoginException(e);
|
||||
|
|
@ -313,12 +321,12 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
// if we get here we're granted, add to the cache
|
||||
ConcurrentHashSet<SimpleString> set;
|
||||
String key = createAuthorizationCacheKey(user, checkType);
|
||||
ConcurrentHashSet<SimpleString> act = authorizationCache.getIfPresent(key);
|
||||
ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(key);
|
||||
if (act != null) {
|
||||
set = act;
|
||||
} else {
|
||||
set = new ConcurrentHashSet<>();
|
||||
authorizationCache.put(key, set);
|
||||
putAuthorizationCacheEntry(set, key);
|
||||
}
|
||||
set.add(fqqn != null ? fqqn : bareAddress);
|
||||
}
|
||||
|
|
@ -394,7 +402,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
* @return the authenticated Subject with all associated role principals
|
||||
*/
|
||||
private Subject getSubjectForAuthorization(SecurityAuth auth, ActiveMQSecurityManager5 securityManager) {
|
||||
Pair<Boolean, Subject> cached = authenticationCache.getIfPresent(createAuthenticationCacheKey(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection()));
|
||||
Pair<Boolean, Subject> cached = getAuthenticationCacheEntry(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection());
|
||||
|
||||
if (cached == null && auth.getUsername() == null && auth.getPassword() == null && auth.getRemotingConnection() instanceof ManagementRemotingConnection) {
|
||||
AccessControlContext accessControlContext = AccessController.getContext();
|
||||
|
|
@ -410,7 +418,7 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
if (cached == null) {
|
||||
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));
|
||||
putAuthenticationCacheEntry(auth.getUsername(), auth.getPassword(), auth.getRemotingConnection(), subject);
|
||||
return subject;
|
||||
} catch (NoCacheLoginException e) {
|
||||
handleNoCacheLoginException(e);
|
||||
|
|
@ -424,26 +432,71 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
logger.debug("Skipping authentication cache due to exception: {}", e.getMessage());
|
||||
}
|
||||
|
||||
private void putAuthenticationCacheEntry(String user,
|
||||
String password,
|
||||
RemotingConnection connection,
|
||||
Subject subject) {
|
||||
if (authenticationCache != null) {
|
||||
authenticationCache.put(createAuthenticationCacheKey(user, password, connection), new Pair<>(subject != null, subject));
|
||||
}
|
||||
}
|
||||
|
||||
private Pair<Boolean, Subject> getAuthenticationCacheEntry(String user,
|
||||
String password,
|
||||
RemotingConnection connection) {
|
||||
if (authenticationCache == null) {
|
||||
return null;
|
||||
} else {
|
||||
return authenticationCache.getIfPresent(createAuthenticationCacheKey(user, password, connection));
|
||||
}
|
||||
}
|
||||
|
||||
private void putAuthorizationCacheEntry(ConcurrentHashSet<SimpleString> set, String key) {
|
||||
if (authorizationCache != null) {
|
||||
authorizationCache.put(key, set);
|
||||
}
|
||||
}
|
||||
|
||||
private ConcurrentHashSet<SimpleString> getAuthorizationCacheEntry(String key) {
|
||||
if (authorizationCache == null) {
|
||||
return null;
|
||||
} else {
|
||||
return authorizationCache.getIfPresent(key);
|
||||
}
|
||||
}
|
||||
|
||||
public void invalidateAuthorizationCache() {
|
||||
authorizationCache.invalidateAll();
|
||||
if (authorizationCache != null) {
|
||||
authorizationCache.invalidateAll();
|
||||
}
|
||||
}
|
||||
|
||||
public void invalidateAuthenticationCache() {
|
||||
authenticationCache.invalidateAll();
|
||||
if (authenticationCache != null) {
|
||||
authenticationCache.invalidateAll();
|
||||
}
|
||||
}
|
||||
|
||||
public long getAuthenticationCacheSize() {
|
||||
return authenticationCache.size();
|
||||
if (authenticationCache == null) {
|
||||
return 0;
|
||||
} else {
|
||||
return authenticationCache.size();
|
||||
}
|
||||
}
|
||||
|
||||
public long getAuthorizationCacheSize() {
|
||||
return authorizationCache.size();
|
||||
if (authorizationCache == null) {
|
||||
return 0;
|
||||
} else {
|
||||
return authorizationCache.size();
|
||||
}
|
||||
}
|
||||
|
||||
private boolean checkAuthorizationCache(final SimpleString dest, final String user, final CheckType checkType) {
|
||||
boolean granted = false;
|
||||
|
||||
ConcurrentHashSet<SimpleString> act = authorizationCache.getIfPresent(createAuthorizationCacheKey(user, checkType));
|
||||
ConcurrentHashSet<SimpleString> act = getAuthorizationCacheEntry(createAuthorizationCacheKey(user, checkType));
|
||||
if (act != null) {
|
||||
granted = act.contains(dest);
|
||||
}
|
||||
|
|
@ -458,4 +511,14 @@ public class SecurityStoreImpl implements SecurityStore, HierarchicalRepositoryC
|
|||
private String createAuthorizationCacheKey(String user, CheckType checkType) {
|
||||
return user + "." + checkType.name();
|
||||
}
|
||||
|
||||
// for testing
|
||||
protected Cache<String, Pair<Boolean, Subject>> getAuthenticationCache() {
|
||||
return authenticationCache;
|
||||
}
|
||||
|
||||
// for testing
|
||||
protected Cache<String, ConcurrentHashSet<SimpleString>> getAuthorizationCache() {
|
||||
return authorizationCache;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,94 @@
|
|||
/*
|
||||
* Licensed to the Apache Software Foundation (ASF) under one or more
|
||||
* contributor license agreements. See the NOTICE file distributed with
|
||||
* this work for additional information regarding copyright ownership.
|
||||
* The ASF licenses this file to You under the Apache License, Version 2.0
|
||||
* (the "License"); you may not use this file except in compliance with
|
||||
* the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
package org.apache.activemq.artemis.core.security.impl;
|
||||
|
||||
import javax.security.auth.Subject;
|
||||
import java.util.Set;
|
||||
|
||||
import org.apache.activemq.artemis.core.security.CheckType;
|
||||
import org.apache.activemq.artemis.core.security.Role;
|
||||
import org.apache.activemq.artemis.core.security.SecurityAuth;
|
||||
import org.apache.activemq.artemis.core.settings.impl.HierarchicalObjectRepository;
|
||||
import org.apache.activemq.artemis.spi.core.protocol.RemotingConnection;
|
||||
import org.apache.activemq.artemis.spi.core.security.ActiveMQSecurityManager5;
|
||||
import org.apache.activemq.artemis.spi.core.security.jaas.NoCacheLoginException;
|
||||
import org.apache.activemq.artemis.utils.RandomUtil;
|
||||
import org.junit.Test;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertNull;
|
||||
|
||||
public class SecurityStoreImplTest {
|
||||
|
||||
@Test
|
||||
public void zeroCacheSizeTest() throws Exception {
|
||||
ActiveMQSecurityManager5 securityManager = new ActiveMQSecurityManager5() {
|
||||
@Override
|
||||
public Subject authenticate(String user,
|
||||
String password,
|
||||
RemotingConnection remotingConnection,
|
||||
String securityDomain) throws NoCacheLoginException {
|
||||
return new Subject();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean authorize(Subject subject, Set<Role> roles, CheckType checkType, String address) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean validateUser(String user, String password) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean validateUserAndRole(String user, String password, Set<Role> roles, CheckType checkType) {
|
||||
return false;
|
||||
}
|
||||
};
|
||||
SecurityStoreImpl securityStore = new SecurityStoreImpl(new HierarchicalObjectRepository<>(), securityManager, 999, true, "", null, null, 0, 0);
|
||||
assertNull(securityStore.getAuthenticationCache());
|
||||
securityStore.authenticate(RandomUtil.randomString(), RandomUtil.randomString(), null);
|
||||
assertEquals(0, securityStore.getAuthenticationCacheSize());
|
||||
securityStore.invalidateAuthenticationCache(); // ensure this doesn't throw an NPE
|
||||
|
||||
assertNull(securityStore.getAuthorizationCache());
|
||||
securityStore.check(RandomUtil.randomSimpleString(), CheckType.SEND, new SecurityAuth() {
|
||||
@Override
|
||||
public String getUsername() {
|
||||
return RandomUtil.randomString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getPassword() {
|
||||
return RandomUtil.randomString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public RemotingConnection getRemotingConnection() {
|
||||
return null;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getSecurityDomain() {
|
||||
return null;
|
||||
}
|
||||
});
|
||||
assertEquals(0, securityStore.getAuthorizationCacheSize());
|
||||
securityStore.invalidateAuthorizationCache(); // ensure this doesn't throw an NPE
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue