diff --git a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authc/esnative/ESNativeUsersStore.java b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authc/esnative/ESNativeUsersStore.java index babbb5fd773..01b6f6b2765 100644 --- a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authc/esnative/ESNativeUsersStore.java +++ b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authc/esnative/ESNativeUsersStore.java @@ -679,12 +679,13 @@ public class ESNativeUsersStore extends AbstractComponent implements ClusterStat final ObjectLongMap map = new ObjectLongHashMap<>(); SearchResponse response = null; try { + client.admin().indices().prepareRefresh(ShieldTemplateService.SECURITY_INDEX_NAME).get(); SearchRequest request = client.prepareSearch(ShieldTemplateService.SECURITY_INDEX_NAME) .setScroll(scrollKeepAlive) .setQuery(QueryBuilders.typeQuery(USER_DOC_TYPE)) .setSize(scrollSize) .setVersion(true) - .setFetchSource(true) + .setFetchSource(false) // we only need id and version .request(); response = client.search(request).actionGet(); diff --git a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java index 98396187539..4a024198bc3 100644 --- a/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java +++ b/elasticsearch/x-pack/shield/src/main/java/org/elasticsearch/shield/authz/esnative/ESNativeRolesStore.java @@ -496,6 +496,7 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore, // create a copy of the keys in the cache since we will be modifying this list final Set existingRoles = new HashSet<>(roleCache.keySet()); try { + client.admin().indices().prepareRefresh(ShieldTemplateService.SECURITY_INDEX_NAME); SearchRequest request = client.prepareSearch(ShieldTemplateService.SECURITY_INDEX_NAME) .setScroll(scrollKeepAlive) .setQuery(QueryBuilders.typeQuery(ROLE_DOC_TYPE)) @@ -513,12 +514,23 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore, for (SearchHit hit : response.getHits().getHits()) { final String roleName = hit.getId(); final long version = hit.version(); - existingRoles.remove(roleName); + final boolean existed = existingRoles.remove(roleName); // we use the locking mechanisms provided by the map/cache to help protect against concurrent operations // that will leave the cache in a bad state - roleCache.computeIfPresent(roleName, new BiFunction() { + roleCache.compute(roleName, new BiFunction() { @Override public RoleAndVersion apply(String roleName, RoleAndVersion existing) { + if (existing == null) { + if (existed) { + // the cache doesn't have this role anymore, it got cleared by something else, do nothing. + return null; + } else { + // it is new, we can cache it + RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef()); + return new RoleAndVersion(rd, version); + } + } + if (version > existing.getVersion()) { RoleDescriptor rd = transformRole(hit.getId(), hit.getSourceRef()); if (rd != null) { @@ -538,7 +550,8 @@ public class ESNativeRolesStore extends AbstractComponent implements RolesStore, // check to see if we had roles that do not exist in the index if (existingRoles.isEmpty() == false) { for (String roleName : existingRoles) { - invalidate(roleName); + logger.trace("role [{}] does not exist anymore, removing from cache", roleName); + roleCache.remove(roleName); } } } catch (IndexNotFoundException e) { diff --git a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java index 8ba77b32d07..065326da13f 100644 --- a/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java +++ b/elasticsearch/x-pack/shield/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java @@ -23,7 +23,6 @@ import org.elasticsearch.shield.authz.esnative.ESNativeRolesStore; import org.elasticsearch.shield.client.SecurityClient; import org.elasticsearch.test.NativeRealmIntegTestCase; import org.elasticsearch.test.ShieldSettingsSource; -import org.elasticsearch.test.junit.annotations.TestLogging; import org.elasticsearch.test.rest.client.http.HttpResponse; import org.junit.Before; import org.junit.BeforeClass; @@ -39,7 +38,6 @@ import static org.hamcrest.Matchers.notNullValue; * Test for the Shield clear roles API that changes the polling aspect of shield to only run once an hour in order to * test the cache clearing APIs. */ -@TestLogging("shield.authc.esnative:TRACE,shield.authz.esnative:TRACE,integration:DEBUG") public class ClearRolesCacheTests extends NativeRealmIntegTestCase { private static String[] roles; @@ -76,9 +74,11 @@ public class ClearRolesCacheTests extends NativeRealmIntegTestCase { @Override public Settings nodeSettings(int nodeOrdinal) { + TimeValue pollerInterval = TimeValue.timeValueMillis((long) randomIntBetween(2, 2000)); + logger.debug("using poller interval [{}]", pollerInterval); return Settings.builder() .put(super.nodeSettings(nodeOrdinal)) - .put("shield.authc.native.reload.interval", TimeValue.timeValueSeconds(2L)) + .put("shield.authc.native.reload.interval", pollerInterval) .put(NetworkModule.HTTP_ENABLED.getKey(), true) .build(); } @@ -90,11 +90,13 @@ public class ClearRolesCacheTests extends NativeRealmIntegTestCase { int modifiedRolesCount = randomIntBetween(1, roles.length); List toModify = randomSubsetOf(modifiedRolesCount, roles); logger.debug("--> modifying roles {} to have run_as", toModify); + final boolean refresh = randomBoolean(); for (String role : toModify) { PutRoleResponse response = securityClient.preparePutRole(role) .cluster("none") .addIndices(new String[] { "*" }, new String[] { "ALL" }, null, null) .runAs(role) + .refresh(refresh) .get(); assertThat(response.isCreated(), is(false)); logger.debug("--> updated role [{}] with run_as", role); @@ -107,10 +109,12 @@ public class ClearRolesCacheTests extends NativeRealmIntegTestCase { int modifiedRolesCount = randomIntBetween(1, roles.length); List toModify = randomSubsetOf(modifiedRolesCount, roles); logger.debug("--> modifying roles {} to have run_as", toModify); + final boolean refresh = randomBoolean(); for (String role : toModify) { UpdateResponse response = internalClient().prepareUpdate().setId(role).setIndex(ShieldTemplateService.SECURITY_INDEX_NAME) .setType(ESNativeRolesStore.ROLE_DOC_TYPE) .setDoc("run_as", new String[] { role }) + .setRefresh(refresh) .get(); assertThat(response.isCreated(), is(false)); logger.debug("--> updated role [{}] with run_as", role); @@ -150,8 +154,11 @@ public class ClearRolesCacheTests extends NativeRealmIntegTestCase { RoleDescriptor[] foundRoles = securityClient.prepareGetRoles().names(role).get().roles(); assertThat(foundRoles.length, is(1)); logger.debug("--> deleting role [{}]", role); + final boolean refresh = randomBoolean(); DeleteResponse response = internalClient() - .prepareDelete(ShieldTemplateService.SECURITY_INDEX_NAME, ESNativeRolesStore.ROLE_DOC_TYPE, role).get(); + .prepareDelete(ShieldTemplateService.SECURITY_INDEX_NAME, ESNativeRolesStore.ROLE_DOC_TYPE, role) + .setRefresh(refresh) + .get(); assertThat(response.isFound(), is(true)); assertBusy(new Runnable() {