Do not fail requests on exceptions from native roles store (elastic/x-pack-elasticsearch#2857)

This commit changes the handling of exceptions when retrieving roles from the native roles store.
Previously, exceptions would have caused the request to terminate and the exception would be
sent back to the user. This makes for a bad experience when a cluster hasn't been upgraded to the
latest index format and anonymous access is enabled with a native role as all requests without
preemptive basic authentication would result in an exception. The change here is to allow the
request to continue processing. Once the security index is up to date, the roles cache is cleared
so that the native roles can be picked up.

relates elastic/x-pack-elasticsearch#2686

Original commit: elastic/x-pack-elasticsearch@ef5149140f
This commit is contained in:
Jay Modi 2017-11-06 10:27:56 -07:00 committed by GitHub
parent 457c49c332
commit be773363c9
7 changed files with 127 additions and 36 deletions

View File

@ -404,6 +404,7 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin, Clus
final CompositeRolesStore allRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore,
reservedRolesStore, rolesProviders, threadPool.getThreadContext(), licenseState);
securityLifecycleService.addSecurityIndexHealthChangeListener(allRolesStore::onSecurityIndexHealthChange);
securityLifecycleService.addSecurityIndexOutOfDateListener(allRolesStore::onSecurityIndexOutOfDateChange);
// to keep things simple, just invalidate all cached entries on license change. this happens so rarely that the impact should be
// minimal
licenseState.addListener(allRolesStore::invalidateAll);

View File

@ -149,6 +149,15 @@ public class SecurityLifecycleService extends AbstractComponent implements Clust
securityIndex.addIndexHealthChangeListener(listener);
}
/**
* Adds a listener which will be notified when the security index out of date value changes. The previous and
* current value will be provided to the listener so that the listener can determine if any action
* needs to be taken.
*/
public void addSecurityIndexOutOfDateListener(BiConsumer<Boolean, Boolean> listener) {
securityIndex.addIndexOutOfDateListener(listener);
}
// this is called in a lifecycle listener beforeStop on the cluster service
private void close() {
if (indexAuditTrail != null) {

View File

@ -152,38 +152,46 @@ public class CompositeRolesStore extends AbstractComponent {
} else {
nativeRolesStore.getRoleDescriptors(remainingRoleNames.toArray(Strings.EMPTY_ARRAY), ActionListener.wrap((descriptors) -> {
builtInRoleDescriptors.addAll(descriptors);
if (builtInRoleDescriptors.size() != filteredRoleNames.size()) {
final Set<String> missing = difference(filteredRoleNames, builtInRoleDescriptors);
assert missing.isEmpty() == false : "the missing set should not be empty if the sizes didn't match";
if (licenseState.isCustomRoleProvidersAllowed() && !customRolesProviders.isEmpty()) {
new IteratingActionListener<>(roleDescriptorActionListener, (rolesProvider, listener) -> {
// resolve descriptors with role provider
rolesProvider.accept(missing, ActionListener.wrap((resolvedDescriptors) -> {
builtInRoleDescriptors.addAll(resolvedDescriptors);
// remove resolved descriptors from the set of roles still needed to be resolved
for (RoleDescriptor descriptor : resolvedDescriptors) {
missing.remove(descriptor.getName());
}
if (missing.isEmpty()) {
// no more roles to resolve, send the response
listener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
} else {
// still have roles to resolve, keep trying with the next roles provider
listener.onResponse(null);
}
}, listener::onFailure));
}, customRolesProviders, threadContext, () -> {
negativeLookupCache.addAll(missing);
return builtInRoleDescriptors;
}).run();
} else {
negativeLookupCache.addAll(missing);
roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
}
} else {
roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
}
}, roleDescriptorActionListener::onFailure));
callCustomRoleProvidersIfEnabled(builtInRoleDescriptors, filteredRoleNames, roleDescriptorActionListener);
}, e -> {
logger.warn("role retrieval failed from the native roles store", e);
callCustomRoleProvidersIfEnabled(builtInRoleDescriptors, filteredRoleNames, roleDescriptorActionListener);
}));
}
}
private void callCustomRoleProvidersIfEnabled(Set<RoleDescriptor> builtInRoleDescriptors, Set<String> filteredRoleNames,
ActionListener<Set<RoleDescriptor>> roleDescriptorActionListener) {
if (builtInRoleDescriptors.size() != filteredRoleNames.size()) {
final Set<String> missing = difference(filteredRoleNames, builtInRoleDescriptors);
assert missing.isEmpty() == false : "the missing set should not be empty if the sizes didn't match";
if (licenseState.isCustomRoleProvidersAllowed() && !customRolesProviders.isEmpty()) {
new IteratingActionListener<>(roleDescriptorActionListener, (rolesProvider, listener) -> {
// resolve descriptors with role provider
rolesProvider.accept(missing, ActionListener.wrap((resolvedDescriptors) -> {
builtInRoleDescriptors.addAll(resolvedDescriptors);
// remove resolved descriptors from the set of roles still needed to be resolved
for (RoleDescriptor descriptor : resolvedDescriptors) {
missing.remove(descriptor.getName());
}
if (missing.isEmpty()) {
// no more roles to resolve, send the response
listener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
} else {
// still have roles to resolve, keep trying with the next roles provider
listener.onResponse(null);
}
}, listener::onFailure));
}, customRolesProviders, threadContext, () -> {
negativeLookupCache.addAll(missing);
return builtInRoleDescriptors;
}).run();
} else {
negativeLookupCache.addAll(missing);
roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
}
} else {
roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors));
}
}
@ -300,6 +308,11 @@ public class CompositeRolesStore extends AbstractComponent {
}
}
public void onSecurityIndexOutOfDateChange(boolean prevOutOfDate, boolean outOfDate) {
assert prevOutOfDate != outOfDate : "this method should only be called if the two values are different";
invalidateAll();
}
/**
* A mutable class that can be used to represent the combination of one or more {@link IndicesPrivileges}
*/

View File

@ -109,14 +109,13 @@ public class NativeRolesStore extends AbstractComponent {
listener.onFailure(new IllegalStateException(
"Security index is not on the current version - the native realm will not be operational until " +
"the upgrade API is run on the security index"));
return;
} else {
try {
QueryBuilder query;
if (names == null || names.length == 0) {
query = QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE);
} else {
final String[] roleNames = Arrays.asList(names).stream().map(s -> getIdForUser(s)).toArray(String[]::new);
final String[] roleNames = Arrays.stream(names).map(s -> getIdForUser(s)).toArray(String[]::new);
query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(roleNames));
}
SearchRequest request = client.prepareSearch(SecurityLifecycleService.SECURITY_INDEX_NAME)
@ -129,7 +128,7 @@ public class NativeRolesStore extends AbstractComponent {
InternalClient.fetchAllByEntity(client, request, listener,
(hit) -> transformRole(hit.getId(), hit.getSourceRef(), logger, licenseState));
} catch (Exception e) {
logger.error((Supplier<?>) () -> new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e);
logger.error(new ParameterizedMessage("unable to retrieve roles {}", Arrays.toString(names)), e);
listener.onFailure(e);
}
}

View File

@ -62,6 +62,7 @@ public class IndexLifecycleManager extends AbstractComponent {
private final InternalSecurityClient client;
private final List<BiConsumer<ClusterIndexHealth, ClusterIndexHealth>> indexHealthChangeListeners = new CopyOnWriteArrayList<>();
private final List<BiConsumer<Boolean, Boolean>> indexOutOfDateListeners = new CopyOnWriteArrayList<>();
private volatile boolean templateIsUpToDate;
private volatile boolean indexExists;
@ -107,9 +108,22 @@ public class IndexLifecycleManager extends AbstractComponent {
indexHealthChangeListeners.add(listener);
}
/**
* Adds a listener which will be notified when the security index out of date value changes. The previous and
* current value will be provided to the listener so that the listener can determine if any action
* needs to be taken.
*/
public void addIndexOutOfDateListener(BiConsumer<Boolean, Boolean> listener) {
indexOutOfDateListeners.add(listener);
}
public void clusterChanged(ClusterChangedEvent event) {
final boolean previousUpToDate = this.isIndexUpToDate;
processClusterState(event.state());
checkIndexHealthChange(event);
if (previousUpToDate != this.isIndexUpToDate) {
notifyIndexOutOfDateListeners(previousUpToDate, this.isIndexUpToDate);
}
}
private void processClusterState(ClusterState state) {
@ -157,6 +171,16 @@ public class IndexLifecycleManager extends AbstractComponent {
}
}
private void notifyIndexOutOfDateListeners(boolean previous, boolean current) {
for (BiConsumer<Boolean, Boolean> consumer : indexOutOfDateListeners) {
try {
consumer.accept(previous, current);
} catch (Exception e) {
logger.warn(new ParameterizedMessage("failed to notify listener [{}] of index out of date change", consumer), e);
}
}
}
private boolean checkIndexAvailable(ClusterState state) {
final IndexRoutingTable routingTable = getIndexRoutingTable(state);
if (routingTable != null && routingTable.allPrimaryShardsActive()) {

View File

@ -488,6 +488,25 @@ public class CompositeRolesStoreTests extends ESTestCase {
assertEquals(expectedInvalidation, numInvalidation.get());
}
public void testCacheClearOnIndexOutOfDateChange() {
final AtomicInteger numInvalidation = new AtomicInteger(0);
CompositeRolesStore compositeRolesStore = new CompositeRolesStore(
Settings.EMPTY, mock(FileRolesStore.class), mock(NativeRolesStore.class), mock(ReservedRolesStore.class),
Collections.emptyList(), new ThreadContext(Settings.EMPTY), new XPackLicenseState()) {
@Override
public void invalidateAll() {
numInvalidation.incrementAndGet();
}
};
compositeRolesStore.onSecurityIndexOutOfDateChange(false, true);
assertEquals(1, numInvalidation.get());
compositeRolesStore.onSecurityIndexOutOfDateChange(true, false);
assertEquals(2, numInvalidation.get());
}
private static class InMemoryRolesProvider implements BiConsumer<Set<String>, ActionListener<Set<RoleDescriptor>>> {
private final Function<Set<String>, Set<RoleDescriptor>> roleDescriptorsFunc;

View File

@ -44,7 +44,6 @@ import org.elasticsearch.index.Index;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.security.InternalClient;
import org.elasticsearch.xpack.security.InternalSecurityClient;
import org.elasticsearch.xpack.security.test.SecurityTestUtils;
import org.elasticsearch.xpack.template.TemplateUtils;
@ -204,6 +203,32 @@ public class IndexLifecycleManagerTests extends ESTestCase {
assertEquals(ClusterHealthStatus.GREEN, currentHealth.get().getStatus());
}
public void testIndexOutOfDateListeners() throws Exception {
final AtomicBoolean listenerCalled = new AtomicBoolean(false);
manager.addIndexOutOfDateListener((prev, current) -> {
listenerCalled.set(true);
assertNotEquals(prev, current);
});
assertFalse(manager.isIndexUpToDate());
manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME)));
assertFalse(listenerCalled.get());
assertFalse(manager.isIndexUpToDate());
// index doesn't exist and now exists
final ClusterState.Builder clusterStateBuilder = createClusterState(INDEX_NAME, TEMPLATE_NAME);
markShardsAvailable(clusterStateBuilder);
manager.clusterChanged(event(clusterStateBuilder));
assertTrue(listenerCalled.get());
assertTrue(manager.isIndexUpToDate());
listenerCalled.set(false);
assertFalse(listenerCalled.get());
manager.clusterChanged(event(new ClusterState.Builder(CLUSTER_NAME)));
assertTrue(listenerCalled.get());
assertFalse(manager.isIndexUpToDate());
}
private void assertInitialState() {
assertThat(manager.indexExists(), Matchers.equalTo(false));
assertThat(manager.isAvailable(), Matchers.equalTo(false));
@ -250,6 +275,7 @@ public class IndexLifecycleManagerTests extends ESTestCase {
.put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1)
.put(IndexMetaData.INDEX_FORMAT_SETTING.getKey(), IndexLifecycleManager.INTERNAL_INDEX_FORMAT)
.build());
final Map<String, String> mappings = getTemplateMappings(templateName);