Skip to content

Commit

Permalink
ISPN-14260 Use local in-memory reads for the ClusterRoleMapper to avo…
Browse files Browse the repository at this point in the history 
…id deadlocks
  • Loading branch information
@tristantarrant
tristantarrant committed Nov 30, 2022
1 parent 90b6ed2 commit 82142a6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 14 deletions.
14 changes: 8 additions & 6 deletions core/src/main/java/org/infinispan/security/mappers/ClusterPermissionMapper.java
View file
Expand Up @@ -3,6 +3,7 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.CompletionStage;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -34,11 +35,12 @@ public class ClusterPermissionMapper implements MutableRolePermissionMapper {
private static final String CLUSTER_PERMISSION_MAPPER_CACHE = "org.infinispan.PERMISSIONS";
private EmbeddedCacheManager cacheManager;
private Cache<String, Role> clusterPermissionMap;
private Cache<String, Role> clusterPermissionReadMap;

@Start
void start() {
clusterPermissionMap = cacheManager.getCache(CLUSTER_PERMISSION_MAPPER_CACHE);
clusterPermissionMap = clusterPermissionMap.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD);
clusterPermissionReadMap = clusterPermissionMap.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD, Flag.CACHE_MODE_LOCAL);
}

@Override
Expand All @@ -48,7 +50,7 @@ public void setContext(AuthorizationMapperContext context) {
CacheMode cacheMode = globalConfiguration.isClustered() ? CacheMode.REPL_SYNC : CacheMode.LOCAL;
ConfigurationBuilder cfg = new ConfigurationBuilder();
cfg.clustering().cacheMode(cacheMode)
.stateTransfer().fetchInMemoryState(true).awaitInitialTransfer(false)
.stateTransfer().fetchInMemoryState(true).awaitInitialTransfer(globalConfiguration.isClustered())
.security().authorization().disable();
GlobalComponentRegistry gcr = SecurityActions.getGlobalComponentRegistry(cacheManager);
InternalCacheRegistry internalCacheRegistry = gcr.getComponent(InternalCacheRegistry.class);
Expand All @@ -63,22 +65,22 @@ public CompletionStage<Void> addRole(Role role) {

@Override
public CompletionStage<Boolean> removeRole(String name) {
return clusterPermissionMap.removeAsync(name).thenApply(old -> old != null);
return clusterPermissionMap.removeAsync(name).thenApply(Objects::nonNull);
}

@Override
public Map<String, Role> getAllRoles() {
return isActive() ? clusterPermissionMap.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)) : Collections.emptyMap();
return isActive() ? clusterPermissionReadMap.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)) : Collections.emptyMap();
}

@Override
public Role getRole(String name) {
return isActive() ? clusterPermissionMap.get(name) : null;
return isActive() ? clusterPermissionReadMap.get(name) : null;
}

@Override
public boolean hasRole(String name) {
return isActive() && clusterPermissionMap.containsKey(name);
return isActive() && clusterPermissionReadMap.containsKey(name);
}

private boolean isActive() {
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/org/infinispan/security/mappers/ClusterRoleMapper.java
View file
Expand Up @@ -11,6 +11,7 @@
import org.infinispan.configuration.cache.CacheMode;
import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.configuration.global.GlobalConfiguration;
import org.infinispan.context.Flag;
import org.infinispan.factories.GlobalComponentRegistry;
import org.infinispan.factories.annotations.Start;
import org.infinispan.factories.scopes.Scope;
Expand All @@ -36,18 +37,20 @@ public class ClusterRoleMapper implements MutablePrincipalRoleMapper {
private EmbeddedCacheManager cacheManager;
private static final String CLUSTER_ROLE_MAPPER_CACHE = "org.infinispan.ROLES";
private Cache<String, RoleSet> clusterRoleMap;
private Cache<String, RoleSet> clusterRoleReadMap;

@Start
void start() {
clusterRoleMap = cacheManager.getCache(CLUSTER_ROLE_MAPPER_CACHE);
clusterRoleReadMap = clusterRoleMap.getAdvancedCache().withFlags(Flag.SKIP_CACHE_LOAD, Flag.CACHE_MODE_LOCAL);
}

@Override
public Set<String> principalToRoles(Principal principal) {
if (clusterRoleMap == null) {
if (clusterRoleReadMap == null) {
return Collections.singleton(principal.getName());
}
RoleSet roleSet = clusterRoleMap.get(principal.getName());
RoleSet roleSet = clusterRoleReadMap.get(principal.getName());
if (roleSet != null && !roleSet.roles.isEmpty()) {
return roleSet.roles;
} else {
Expand All @@ -62,7 +65,7 @@ public void setContext(PrincipalRoleMapperContext context) {
CacheMode cacheMode = globalConfiguration.isClustered() ? CacheMode.REPL_SYNC : CacheMode.LOCAL;
ConfigurationBuilder cfg = new ConfigurationBuilder();
cfg.clustering().cacheMode(cacheMode)
.stateTransfer().fetchInMemoryState(true).awaitInitialTransfer(false)
.stateTransfer().fetchInMemoryState(true).awaitInitialTransfer(globalConfiguration.isClustered())
.security().authorization().disable();

GlobalComponentRegistry registry = SecurityActions.getGlobalComponentRegistry(cacheManager);
Expand Down
47 changes: 42 additions & 5 deletions core/src/test/java/org/infinispan/security/DynamicRBACTest.java
View file
@@ -1,10 +1,15 @@
package org.infinispan.security;

import static org.infinispan.functional.FunctionalTestUtils.await;
import static org.infinispan.test.TestingUtil.extractField;
import static org.testng.AssertJUnit.assertEquals;
import static org.testng.AssertJUnit.assertFalse;
import static org.testng.AssertJUnit.assertNotNull;
import static org.testng.AssertJUnit.assertNull;
import static org.testng.AssertJUnit.assertTrue;

import java.security.PrivilegedAction;
import java.security.PrivilegedActionException;
import java.security.PrivilegedExceptionAction;
import java.util.Map;

Expand All @@ -16,15 +21,18 @@
import org.infinispan.configuration.cache.ConfigurationBuilder;
import org.infinispan.configuration.global.GlobalAuthorizationConfigurationBuilder;
import org.infinispan.configuration.global.GlobalConfigurationBuilder;
import org.infinispan.manager.EmbeddedCacheManager;
import org.infinispan.security.mappers.ClusterPermissionMapper;
import org.infinispan.security.mappers.ClusterRoleMapper;
import org.infinispan.test.MultipleCacheManagersTest;
import org.infinispan.test.TestingUtil;
import org.infinispan.test.fwk.TestCacheManagerFactory;
import org.infinispan.test.fwk.TransportFlags;
import org.infinispan.topology.ClusterTopologyManager;
import org.testng.annotations.AfterClass;
import org.testng.annotations.Test;

@Test(groups="functional", testName="security.DynamicRBACTest")
@Test(groups = "functional", testName = "security.DynamicRBACTest")
public class DynamicRBACTest extends MultipleCacheManagersTest {
static final Subject ADMIN = TestingUtil.makeSubject("admin");
static final Subject SUBJECT_A = TestingUtil.makeSubject("A");
Expand All @@ -42,6 +50,8 @@ protected void createCacheManagers() throws Throwable {
crm = (ClusterRoleMapper) cacheManagers.get(0).getCacheManagerConfiguration().security().authorization().principalRoleMapper();
crm.grant("admin", "admin");
cpm = (ClusterPermissionMapper) cacheManagers.get(0).getCacheManagerConfiguration().security().authorization().rolePermissionMapper();
await(cpm.addRole(Role.newRole("wizard", true, AuthorizationPermission.ALL_WRITE)));
await(cpm.addRole(Role.newRole("cleric", true, AuthorizationPermission.ALL_READ)));
return null;
});
}
Expand Down Expand Up @@ -82,10 +92,6 @@ public void testClusterPrincipalMapper() {

public void testClusterPermissionMapper() {
Map<String, Role> roles = cpm.getAllRoles();
assertEquals(0, roles.size());
await(cpm.addRole(Role.newRole("wizard", true, AuthorizationPermission.ALL_WRITE)));
await(cpm.addRole(Role.newRole("cleric", true, AuthorizationPermission.ALL_READ)));
roles = cpm.getAllRoles();
assertEquals(2, roles.size());
assertTrue(roles.containsKey("wizard"));
assertTrue(roles.containsKey("cleric"));
Expand All @@ -107,6 +113,37 @@ public void testClusterPermissionMapper() {
assertEquals(1, roles.size());
}

public void testJoiner() throws PrivilegedActionException {
crm.grant("wizard", "gandalf");
Cache<?, ?> crmCache = extractField(crm, "clusterRoleMap");
ClusterTopologyManager crmTM = TestingUtil.extractComponent(crmCache, ClusterTopologyManager.class);
crmTM.setRebalancingEnabled(false);
await(cpm.addRole(Role.newRole("rogue", true, AuthorizationPermission.LISTEN)));
Cache<?, ?> cpmCache = extractField(cpm, "clusterPermissionMap");
ClusterTopologyManager cpmTM = TestingUtil.extractComponent(cpmCache, ClusterTopologyManager.class);
cpmTM.setRebalancingEnabled(false);
try {
EmbeddedCacheManager joiner = Security.doAs(ADMIN, (PrivilegedExceptionAction<EmbeddedCacheManager>) () ->
addClusterEnabledCacheManager(getGlobalConfigurationBuilder(), getConfigurationBuilder(), new TransportFlags().withFD(true))
);
ClusterRoleMapper joinerCrm = Security.doAs(ADMIN, (PrivilegedExceptionAction<ClusterRoleMapper>) () -> (ClusterRoleMapper) joiner.getCacheManagerConfiguration().security().authorization().principalRoleMapper());
TestingUtil.TestPrincipal gandalf = new TestingUtil.TestPrincipal("gandalf");
assertTrue(crm.principalToRoles(gandalf).contains("wizard"));
assertTrue(crm.list("gandalf").contains("wizard"));
assertFalse(joinerCrm.principalToRoles(gandalf).contains("wizard"));
assertFalse(joinerCrm.list("gandalf").contains("wizard"));

ClusterPermissionMapper joinerCpm = Security.doAs(ADMIN, (PrivilegedExceptionAction<ClusterPermissionMapper>) () -> (ClusterPermissionMapper) joiner.getCacheManagerConfiguration().security().authorization().rolePermissionMapper());
Role rogue0 = cpm.getRole("rogue");
assertNotNull(rogue0);
Role rogue2 = joinerCpm.getRole("rogue");
assertNull(rogue2);
} finally {
crmTM.setRebalancingEnabled(true);
cpmTM.setRebalancingEnabled(true);
}
}

@AfterClass(alwaysRun = true)
@Override
protected void destroy() {
Expand Down

0 comments on commit 82142a6

Please sign in to comment.