From 0cd03d358140193cc84434d385947e84e0b70607 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Mon, 15 Oct 2018 20:52:54 +0100 Subject: [PATCH] Use RoleRetrievalResult for better caching (#34197) Security caches the result of role lookups and negative lookups are cached indefinitely. In the case of transient failures this leads to a bad experience as the roles could truly exist. The CompositeRolesStore needs to know if a failure occurred in one of the roles stores in order to make the appropriate decision as it relates to caching. In order to provide this information to the CompositeRolesStore, the return type of methods to retrieve roles has changed to a new class, RoleRetrievalResult. This class provides the ability to pass back an exception to the roles store. This exception does not mean that a request should be failed but instead serves as a signal to the roles store that missing roles should not be cached and neither should the combined role if there are missing roles. As part of this, the negative lookup cache was also changed from an unbounded cache to a cache with a configurable limit. Relates #33205 --- .../action/TransportXPackUsageAction.java | 10 +- .../core/common/IteratingActionListener.java | 50 +++-- .../core/security/SecurityExtension.java | 13 +- .../authz/store/ReservedRolesStore.java | 20 +- .../authz/store/RoleRetrievalResult.java | 75 +++++++ .../common/IteratingActionListenerTests.java | 50 ++++- .../xpack/security/Security.java | 6 +- .../action/role/TransportGetRolesAction.java | 17 +- .../authz/store/CompositeRolesStore.java | 211 ++++++++++-------- .../security/authz/store/FileRolesStore.java | 18 +- .../authz/store/NativeRolesStore.java | 55 +++-- .../integration/ClearRolesCacheTests.java | 11 +- .../role/TransportGetRolesActionTests.java | 42 ++-- .../authz/store/CompositeRolesStoreTests.java | 181 ++++++++++++--- .../example/ExampleSecurityExtension.java | 4 +- .../role/CustomInMemoryRolesProvider.java | 7 +- 16 files changed, 554 insertions(+), 216 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleRetrievalResult.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/TransportXPackUsageAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/TransportXPackUsageAction.java index 6b7d5b96d2024..554e8e076660f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/TransportXPackUsageAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/action/TransportXPackUsageAction.java @@ -53,8 +53,7 @@ protected XPackUsageResponse newResponse() { } @Override - protected void masterOperation(XPackUsageRequest request, ClusterState state, ActionListener listener) - throws Exception { + protected void masterOperation(XPackUsageRequest request, ClusterState state, ActionListener listener) { final ActionListener> usageActionListener = new ActionListener>() { @Override public void onResponse(List usages) { @@ -73,7 +72,8 @@ public void onFailure(Exception e) { @Override public void onResponse(Usage usage) { featureSetUsages.set(position.getAndIncrement(), usage); - iteratingListener.onResponse(null); // just send null back and keep iterating + // the value sent back doesn't matter since our predicate keeps iterating + iteratingListener.onResponse(Collections.emptyList()); } @Override @@ -84,13 +84,13 @@ public void onFailure(Exception e) { }; IteratingActionListener, XPackFeatureSet> iteratingActionListener = new IteratingActionListener<>(usageActionListener, consumer, featureSets, - threadPool.getThreadContext(), () -> { + threadPool.getThreadContext(), (ignore) -> { final List usageList = new ArrayList<>(featureSetUsages.length()); for (int i = 0; i < featureSetUsages.length(); i++) { usageList.add(featureSetUsages.get(i)); } return usageList; - }); + }, (ignore) -> true); iteratingActionListener.run(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/IteratingActionListener.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/IteratingActionListener.java index 46ebd89b8ea76..7eb3a01b44129 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/IteratingActionListener.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/common/IteratingActionListener.java @@ -6,12 +6,14 @@ package org.elasticsearch.xpack.core.common; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.common.Nullable; import org.elasticsearch.common.util.concurrent.ThreadContext; import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.function.Predicate; import java.util.function.Supplier; /** @@ -32,7 +34,8 @@ public final class IteratingActionListener implements ActionListener, R private final ActionListener delegate; private final BiConsumer> consumer; private final ThreadContext threadContext; - private final Supplier consumablesFinishedResponse; + private final Function finalResultFunction; + private final Predicate iterationPredicate; private int position = 0; @@ -46,7 +49,7 @@ public final class IteratingActionListener implements ActionListener, R */ public IteratingActionListener(ActionListener delegate, BiConsumer> consumer, List consumables, ThreadContext threadContext) { - this(delegate, consumer, consumables, threadContext, null); + this(delegate, consumer, consumables, threadContext, Function.identity()); } /** @@ -56,18 +59,36 @@ public IteratingActionListener(ActionListener delegate, BiConsumer delegate, BiConsumer> consumer, List consumables, - ThreadContext threadContext, @Nullable Supplier consumablesFinishedResponse) { + ThreadContext threadContext, Function finalResultFunction) { + this(delegate, consumer, consumables, threadContext, finalResultFunction, Objects::isNull); + } + + /** + * Constructs an {@link IteratingActionListener}. + * + * @param delegate the delegate listener to call when all consumables have finished executing + * @param consumer the consumer that is executed for each consumable instance + * @param consumables the instances that can be consumed to produce a response which is ultimately sent on the delegate listener + * @param threadContext the thread context for the thread pool that created the listener + * @param finalResultFunction a function that maps the response which terminated iteration to a response that will be sent to the + * delegate listener. This is useful if the delegate listener should receive some other value (perhaps + * a concatenation of the results of all the called consumables). + * @param iterationPredicate a {@link Predicate} that checks if iteration should continue based on the returned result + */ + public IteratingActionListener(ActionListener delegate, BiConsumer> consumer, List consumables, + ThreadContext threadContext, Function finalResultFunction, + Predicate iterationPredicate) { this.delegate = delegate; this.consumer = consumer; this.consumables = Collections.unmodifiableList(consumables); this.threadContext = threadContext; - this.consumablesFinishedResponse = consumablesFinishedResponse; + this.finalResultFunction = finalResultFunction; + this.iterationPredicate = iterationPredicate; } @Override @@ -88,18 +109,15 @@ public void onResponse(T response) { // we need to store the context here as there is a chance that this method is called from a thread outside of the ThreadPool // like a LDAP connection reader thread and we can pollute the context in certain cases try (ThreadContext.StoredContext ignore = threadContext.newStoredContext(false)) { - if (response == null) { + final boolean continueIteration = iterationPredicate.test(response); + if (continueIteration) { if (position == consumables.size()) { - if (consumablesFinishedResponse != null) { - delegate.onResponse(consumablesFinishedResponse.get()); - } else { - delegate.onResponse(null); - } + delegate.onResponse(finalResultFunction.apply(response)); } else { consumer.accept(consumables.get(position++), this); } } else { - delegate.onResponse(response); + delegate.onResponse(finalResultFunction.apply(response)); } } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java index 190e9f7520b6c..f422d073cfeb5 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java @@ -14,6 +14,7 @@ import org.elasticsearch.xpack.core.security.authc.Realm; import org.elasticsearch.xpack.core.security.authc.RealmConfig; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import java.util.ArrayList; import java.util.Collections; @@ -72,16 +73,20 @@ default AuthenticationFailureHandler getAuthenticationFailureHandler() { * should be asynchronous if the computation is lengthy or any disk and/or network * I/O is involved. The implementation is responsible for resolving whatever roles * it can into a set of {@link RoleDescriptor} instances. If successful, the - * implementation must invoke {@link ActionListener#onResponse(Object)} to pass along - * the resolved set of role descriptors. If a failure was encountered, the - * implementation must invoke {@link ActionListener#onFailure(Exception)}. + * implementation must wrap the set of {@link RoleDescriptor} instances in a + * {@link RoleRetrievalResult} using {@link RoleRetrievalResult#success(Set)} and then invoke + * {@link ActionListener#onResponse(Object)}. If a failure was encountered, the + * implementation should wrap the failure in a {@link RoleRetrievalResult} using + * {@link RoleRetrievalResult#failure(Exception)} and then invoke + * {@link ActionListener#onResponse(Object)} unless the failure needs to terminate the request, + * in which case the implementation should invoke {@link ActionListener#onFailure(Exception)}. * * By default, an empty list is returned. * * @param settings The configured settings for the node * @param resourceWatcherService Use to watch configuration files for changes */ - default List, ActionListener>>> + default List, ActionListener>> getRolesProviders(Settings settings, ResourceWatcherService resourceWatcherService) { return Collections.emptyList(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java index 22cb1c357c661..3999e9ad3d094 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/ReservedRolesStore.java @@ -5,6 +5,7 @@ */ package org.elasticsearch.xpack.core.security.authz.store; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.xpack.core.monitoring.action.MonitoringBulkAction; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; @@ -21,9 +22,12 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.Objects; import java.util.Set; +import java.util.function.BiConsumer; +import java.util.stream.Collectors; -public class ReservedRolesStore { +public class ReservedRolesStore implements BiConsumer, ActionListener> { public static final RoleDescriptor SUPERUSER_ROLE_DESCRIPTOR = new RoleDescriptor("superuser", new String[] { "all" }, @@ -165,4 +169,18 @@ public Collection roleDescriptors() { public static Set names() { return RESERVED_ROLES.keySet(); } + + @Override + public void accept(Set roleNames, ActionListener listener) { + final Set descriptors = roleNames.stream() + .map(RESERVED_ROLES::get) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + listener.onResponse(RoleRetrievalResult.success(descriptors)); + } + + @Override + public String toString() { + return "reserved roles store"; + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleRetrievalResult.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleRetrievalResult.java new file mode 100644 index 0000000000000..e084da384d830 --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/store/RoleRetrievalResult.java @@ -0,0 +1,75 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +package org.elasticsearch.xpack.core.security.authz.store; + +import org.elasticsearch.common.Nullable; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; + +import java.util.Objects; +import java.util.Set; + +/** + * The result of attempting to retrieve roles from a roles provider. The result can either be + * successful or a failure. A successful result indicates that no errors occurred while retrieving + * roles, even if none of the requested roles could be found. A failure indicates an error + * occurred while retrieving the results but the error is not fatal and the request may be able + * to continue. + */ +public final class RoleRetrievalResult { + + private final Set descriptors; + + @Nullable + private final Exception failure; + + private RoleRetrievalResult(Set descriptors, @Nullable Exception failure) { + if (descriptors != null && failure != null) { + throw new IllegalArgumentException("either descriptors or failure must be null"); + } + this.descriptors = descriptors; + this.failure = failure; + } + + /** + * @return the resolved descriptors or {@code null} if there was a failure + */ + public Set getDescriptors() { + return descriptors; + } + + /** + * @return the failure or {@code null} if retrieval succeeded + */ + @Nullable + public Exception getFailure() { + return failure; + } + + /** + * @return true if the retrieval succeeded + */ + public boolean isSuccess() { + return descriptors != null; + } + + /** + * Creates a successful result with the provided {@link RoleDescriptor} set, + * which must be non-null + */ + public static RoleRetrievalResult success(Set descriptors) { + Objects.requireNonNull(descriptors, "descriptors must not be null if successful"); + return new RoleRetrievalResult(descriptors, null); + } + + /** + * Creates a failed result with the provided non-null exception + */ + public static RoleRetrievalResult failure(Exception e) { + Objects.requireNonNull(e, "Exception must be provided"); + return new RoleRetrievalResult(null, e); + } +} diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/IteratingActionListenerTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/IteratingActionListenerTests.java index 0648d774075d5..9b134a75b3f7c 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/IteratingActionListenerTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/common/IteratingActionListenerTests.java @@ -7,7 +7,6 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; -import org.elasticsearch.common.collect.HppcMaps.Object; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.test.ESTestCase; @@ -18,8 +17,12 @@ import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.function.Predicate; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; public class IteratingActionListenerTests extends ESTestCase { @@ -136,4 +139,49 @@ public void testFailure() { assertEquals(numberOfIterations, iterations.get()); assertTrue(onFailureCalled.get()); } + + public void testFunctionApplied() { + final int numberOfItems = scaledRandomIntBetween(2, 32); + final int numberOfIterations = scaledRandomIntBetween(1, numberOfItems); + List items = new ArrayList<>(numberOfItems); + for (int i = 0; i < numberOfItems; i++) { + items.add(new Object()); + } + + final AtomicInteger iterations = new AtomicInteger(0); + final Predicate iterationPredicate = object -> { + final int current = iterations.incrementAndGet(); + return current != numberOfIterations; + }; + final BiConsumer> consumer = (listValue, listener) -> { + listener.onResponse(items.get(iterations.get())); + }; + + final AtomicReference originalObject = new AtomicReference<>(); + final AtomicReference result = new AtomicReference<>(); + final Function responseFunction = object -> { + originalObject.set(object); + Object randomResult; + do { + randomResult = randomFrom(items); + } while (randomResult == object); + result.set(randomResult); + return randomResult; + }; + + IteratingActionListener iteratingListener = new IteratingActionListener<>(ActionListener.wrap((object) -> { + assertNotNull(object); + assertNotNull(originalObject.get()); + assertThat(object, sameInstance(result.get())); + assertThat(object, not(sameInstance(originalObject.get()))); + assertThat(originalObject.get(), sameInstance(items.get(iterations.get() - 1))); + }, (e) -> { + logger.error("unexpected exception", e); + fail("exception should not have been thrown"); + }), consumer, items, new ThreadContext(Settings.EMPTY), responseFunction, iterationPredicate); + iteratingListener.run(); + + // we never really went async, its all chained together so verify this for sanity + assertEquals(numberOfIterations, iterations.get()); + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 2a49a1299943a..eca4fdc0c2a57 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -116,7 +116,6 @@ import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; import org.elasticsearch.xpack.core.security.authz.accesscontrol.SecurityIndexSearcherWrapper; import org.elasticsearch.xpack.core.security.authz.permission.FieldPermissions; @@ -184,6 +183,7 @@ import org.elasticsearch.xpack.security.authz.store.FileRolesStore; import org.elasticsearch.xpack.security.authz.store.NativePrivilegeStore; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.security.ingest.SetSecurityUserProcessor; import org.elasticsearch.xpack.security.rest.SecurityRestFilter; import org.elasticsearch.xpack.security.rest.action.RestAuthenticateAction; @@ -458,7 +458,7 @@ Collection createComponents(Client client, ThreadPool threadPool, Cluste final FileRolesStore fileRolesStore = new FileRolesStore(settings, env, resourceWatcherService, getLicenseState()); final NativeRolesStore nativeRolesStore = new NativeRolesStore(settings, client, getLicenseState(), securityIndex.get()); final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(); - List, ActionListener>>> rolesProviders = new ArrayList<>(); + List, ActionListener>> rolesProviders = new ArrayList<>(); for (SecurityExtension extension : securityExtensions) { rolesProviders.addAll(extension.getRolesProviders(settings, resourceWatcherService)); } @@ -610,7 +610,7 @@ public static List> getSettings(boolean transportClientMode, List { @@ -41,7 +43,7 @@ public TransportGetRolesAction(Settings settings, ActionFilters actionFilters, protected void doExecute(Task task, final GetRolesRequest request, final ActionListener listener) { final String[] requestedRoles = request.names(); final boolean specificRolesRequested = requestedRoles != null && requestedRoles.length > 0; - final List rolesToSearchFor = new ArrayList<>(); + final Set rolesToSearchFor = new HashSet<>(); final List roles = new ArrayList<>(); if (specificRolesRequested) { @@ -66,11 +68,14 @@ protected void doExecute(Task task, final GetRolesRequest request, final ActionL // specific roles were requested but they were built in only, no need to hit the store listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); } else { - String[] roleNames = rolesToSearchFor.toArray(new String[rolesToSearchFor.size()]); - nativeRolesStore.getRoleDescriptors(roleNames, ActionListener.wrap((foundRoles) -> { - roles.addAll(foundRoles); - listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); - }, listener::onFailure)); + nativeRolesStore.getRoleDescriptors(rolesToSearchFor, ActionListener.wrap((retrievalResult) -> { + if (retrievalResult.isSuccess()) { + roles.addAll(retrievalResult.getDescriptors()); + listener.onResponse(new GetRolesResponse(roles.toArray(new RoleDescriptor[roles.size()]))); + } else { + listener.onFailure(retrievalResult.getFailure()); + } + }, listener::onFailure)); } } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java index 7e1cc49e2c0bc..a7a73397d50ab 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStore.java @@ -7,6 +7,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesReference; @@ -17,7 +18,6 @@ import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.util.concurrent.ConcurrentCollections; import org.elasticsearch.common.util.concurrent.ReleasableLock; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.util.set.Sets; @@ -34,6 +34,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.Privilege; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.util.ArrayList; @@ -51,10 +52,11 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.BiConsumer; +import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Collectors; import static org.elasticsearch.common.util.set.Sets.newHashSet; -import static org.elasticsearch.xpack.core.security.SecurityField.setting; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isMoveFromRedToNonRed; @@ -77,29 +79,30 @@ public class CompositeRolesStore extends AbstractComponent { writeLock = new ReleasableLock(iterationLock.writeLock()); } - public static final Setting CACHE_SIZE_SETTING = - Setting.intSetting(setting("authz.store.roles.cache.max_size"), 10000, Property.NodeScope); + private static final Setting CACHE_SIZE_SETTING = + Setting.intSetting("xpack.security.authz.store.roles.cache.max_size", 10000, Property.NodeScope); + private static final Setting NEGATIVE_LOOKUP_CACHE_SIZE_SETTING = + Setting.intSetting("xpack.security.authz.store.roles.negative_lookup_cache.max_size", 10000, Property.NodeScope); private final FileRolesStore fileRolesStore; private final NativeRolesStore nativeRolesStore; - private final ReservedRolesStore reservedRolesStore; private final NativePrivilegeStore privilegeStore; private final XPackLicenseState licenseState; private final Cache, Role> roleCache; - private final Set negativeLookupCache; + private final Cache negativeLookupCache; private final ThreadContext threadContext; private final AtomicLong numInvalidation = new AtomicLong(); - private final List, ActionListener>>> customRolesProviders; + private final List, ActionListener>> builtInRoleProviders; + private final List, ActionListener>> allRoleProviders; public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, NativeRolesStore nativeRolesStore, ReservedRolesStore reservedRolesStore, NativePrivilegeStore privilegeStore, - List, ActionListener>>> rolesProviders, + List, ActionListener>> rolesProviders, ThreadContext threadContext, XPackLicenseState licenseState) { super(settings); this.fileRolesStore = fileRolesStore; fileRolesStore.addListener(this::invalidate); this.nativeRolesStore = nativeRolesStore; - this.reservedRolesStore = reservedRolesStore; this.privilegeStore = privilegeStore; this.licenseState = licenseState; CacheBuilder, Role> builder = CacheBuilder.builder(); @@ -109,8 +112,22 @@ public CompositeRolesStore(Settings settings, FileRolesStore fileRolesStore, Nat } this.roleCache = builder.build(); this.threadContext = threadContext; - this.negativeLookupCache = ConcurrentCollections.newConcurrentSet(); - this.customRolesProviders = Collections.unmodifiableList(rolesProviders); + CacheBuilder nlcBuilder = CacheBuilder.builder(); + final int nlcCacheSize = NEGATIVE_LOOKUP_CACHE_SIZE_SETTING.get(settings); + if (nlcCacheSize >= 0) { + nlcBuilder.setMaximumWeight(nlcCacheSize); + } + this.negativeLookupCache = nlcBuilder.build(); + this.builtInRoleProviders = Collections.unmodifiableList(Arrays.asList(reservedRolesStore, fileRolesStore, nativeRolesStore)); + if (rolesProviders.isEmpty()) { + this.allRoleProviders = this.builtInRoleProviders; + } else { + List, ActionListener>> allList = + new ArrayList<>(builtInRoleProviders.size() + rolesProviders.size()); + allList.addAll(builtInRoleProviders); + allList.addAll(rolesProviders); + this.allRoleProviders = Collections.unmodifiableList(allList); + } } public void roles(Set roleNames, FieldPermissionsCache fieldPermissionsCache, ActionListener roleActionListener) { @@ -120,18 +137,23 @@ public void roles(Set roleNames, FieldPermissionsCache fieldPermissionsC } else { final long invalidationCounter = numInvalidation.get(); roleDescriptors(roleNames, ActionListener.wrap( - descriptors -> { + rolesRetrievalResult -> { + final boolean missingRoles = rolesRetrievalResult.getMissingRoles().isEmpty() == false; + if (missingRoles) { + logger.debug("Could not find roles with names {}", rolesRetrievalResult.getMissingRoles()); + } + final Set effectiveDescriptors; if (licenseState.isDocumentAndFieldLevelSecurityAllowed()) { - effectiveDescriptors = descriptors; + effectiveDescriptors = rolesRetrievalResult.getRoleDescriptors(); } else { - effectiveDescriptors = descriptors.stream() + effectiveDescriptors = rolesRetrievalResult.getRoleDescriptors().stream() .filter((rd) -> rd.isUsingDocumentOrFieldLevelSecurity() == false) .collect(Collectors.toSet()); } logger.trace("Building role from descriptors [{}] for names [{}]", effectiveDescriptors, roleNames); buildRoleFromDescriptors(effectiveDescriptors, fieldPermissionsCache, privilegeStore, ActionListener.wrap(role -> { - if (role != null) { + if (role != null && rolesRetrievalResult.isSuccess()) { try (ReleasableLock ignored = readLock.acquire()) { /* this is kinda spooky. We use a read/write lock to ensure we don't modify the cache if we hold * the write lock (fetching stats for instance - which is kinda overkill?) but since we fetching @@ -144,6 +166,10 @@ public void roles(Set roleNames, FieldPermissionsCache fieldPermissionsC roleCache.computeIfAbsent(roleNames, (s) -> role); } } + + for (String missingRole : rolesRetrievalResult.getMissingRoles()) { + negativeLookupCache.computeIfAbsent(missingRole, s -> Boolean.TRUE); + } } roleActionListener.onResponse(role); }, roleActionListener::onFailure)); @@ -152,97 +178,56 @@ public void roles(Set roleNames, FieldPermissionsCache fieldPermissionsC } } - private void roleDescriptors(Set roleNames, ActionListener> roleDescriptorActionListener) { + private void roleDescriptors(Set roleNames, ActionListener rolesResultListener) { final Set filteredRoleNames = roleNames.stream().filter((s) -> { - if (negativeLookupCache.contains(s)) { + if (negativeLookupCache.get(s) != null) { logger.debug("Requested role [{}] does not exist (cached)", s); return false; } else { return true; } }).collect(Collectors.toSet()); - final Set builtInRoleDescriptors = getBuiltInRoleDescriptors(filteredRoleNames); - Set remainingRoleNames = difference(filteredRoleNames, builtInRoleDescriptors); - if (remainingRoleNames.isEmpty()) { - roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors)); - } else { - nativeRolesStore.getRoleDescriptors(remainingRoleNames.toArray(Strings.EMPTY_ARRAY), ActionListener.wrap((descriptors) -> { - logger.debug(() -> new ParameterizedMessage("Roles [{}] were resolved from the native index store", names(descriptors))); - builtInRoleDescriptors.addAll(descriptors); - callCustomRoleProvidersIfEnabled(builtInRoleDescriptors, filteredRoleNames, roleDescriptorActionListener); - }, e -> { - logger.warn("role retrieval failed from the native roles store", e); - callCustomRoleProvidersIfEnabled(builtInRoleDescriptors, filteredRoleNames, roleDescriptorActionListener); - })); - } - } - private void callCustomRoleProvidersIfEnabled(Set builtInRoleDescriptors, Set filteredRoleNames, - ActionListener> roleDescriptorActionListener) { - if (builtInRoleDescriptors.size() != filteredRoleNames.size()) { - final Set 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) -> { - logger.debug(() -> - new ParameterizedMessage("Roles [{}] were resolved by [{}]", names(resolvedDescriptors), rolesProvider)); - 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 { - logger.debug(() -> - new ParameterizedMessage("Requested roles [{}] do not exist", Strings.collectionToCommaDelimitedString(missing))); - negativeLookupCache.addAll(missing); - roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors)); - } - } else { - roleDescriptorActionListener.onResponse(Collections.unmodifiableSet(builtInRoleDescriptors)); - } + loadRoleDescriptorsAsync(filteredRoleNames, rolesResultListener); } - private Set getBuiltInRoleDescriptors(Set roleNames) { - final Set descriptors = reservedRolesStore.roleDescriptors().stream() - .filter((rd) -> roleNames.contains(rd.getName())) - .collect(Collectors.toCollection(HashSet::new)); - if (descriptors.size() > 0) { - logger.debug(() -> new ParameterizedMessage("Roles [{}] are builtin roles", names(descriptors))); - } - final Set difference = difference(roleNames, descriptors); - if (difference.isEmpty() == false) { - final Set fileRoles = fileRolesStore.roleDescriptors(difference); - logger.debug(() -> - new ParameterizedMessage("Roles [{}] were resolved from [{}]", names(fileRoles), fileRolesStore.getFile())); - descriptors.addAll(fileRoles); - } - - return descriptors; + private void loadRoleDescriptorsAsync(Set roleNames, ActionListener listener) { + final RolesRetrievalResult rolesResult = new RolesRetrievalResult(); + final List, ActionListener>> asyncRoleProviders = + licenseState.isCustomRoleProvidersAllowed() ? allRoleProviders : builtInRoleProviders; + + final ActionListener descriptorsListener = + ContextPreservingActionListener.wrapPreservingContext(ActionListener.wrap(ignore -> { + rolesResult.setMissingRoles(roleNames); + listener.onResponse(rolesResult); + }, listener::onFailure), threadContext); + + final Predicate iterationPredicate = result -> roleNames.isEmpty() == false; + new IteratingActionListener<>(descriptorsListener, (rolesProvider, providerListener) -> { + // try to resolve descriptors with role provider + rolesProvider.accept(roleNames, ActionListener.wrap(result -> { + if (result.isSuccess()) { + logger.debug(() -> new ParameterizedMessage("Roles [{}] were resolved by [{}]", + names(result.getDescriptors()), rolesProvider)); + final Set resolvedDescriptors = result.getDescriptors(); + rolesResult.addDescriptors(resolvedDescriptors); + // remove resolved descriptors from the set of roles still needed to be resolved + for (RoleDescriptor descriptor : resolvedDescriptors) { + roleNames.remove(descriptor.getName()); + } + } else { + logger.warn(new ParameterizedMessage("role retrieval failed from [{}]", rolesProvider), result.getFailure()); + rolesResult.setFailure(); + } + providerListener.onResponse(result); + }, providerListener::onFailure)); + }, asyncRoleProviders, threadContext, Function.identity(), iterationPredicate).run(); } private String names(Collection descriptors) { return descriptors.stream().map(RoleDescriptor::getName).collect(Collectors.joining(",")); } - private Set difference(Set roleNames, Set descriptors) { - Set foundNames = descriptors.stream().map(RoleDescriptor::getName).collect(Collectors.toSet()); - return Sets.difference(roleNames, foundNames); - } - public static void buildRoleFromDescriptors(Collection roleDescriptors, FieldPermissionsCache fieldPermissionsCache, NativePrivilegeStore privilegeStore, ActionListener listener) { if (roleDescriptors.isEmpty()) { @@ -332,7 +317,7 @@ public static void buildRoleFromDescriptors(Collection roleDescr public void invalidateAll() { numInvalidation.incrementAndGet(); - negativeLookupCache.clear(); + negativeLookupCache.invalidateAll(); try (ReleasableLock ignored = readLock.acquire()) { roleCache.invalidateAll(); } @@ -351,7 +336,7 @@ public void invalidate(String role) { } } } - negativeLookupCache.remove(role); + negativeLookupCache.invalidate(role); } public void invalidate(Set roles) { @@ -368,7 +353,7 @@ public void invalidate(Set roles) { } } - negativeLookupCache.removeAll(roles); + roles.forEach(negativeLookupCache::invalidate); } public void usageStats(ActionListener> listener) { @@ -387,6 +372,11 @@ public void onSecurityIndexStateChange(SecurityIndexManager.State previousState, } } + // pkg - private for testing + boolean isValueInNegativeLookupCache(String key) { + return negativeLookupCache.get(key) != null; + } + /** * A mutable class that can be used to represent the combination of one or more {@link IndicesPrivileges} */ @@ -421,4 +411,39 @@ void merge(MergeableIndicesPrivilege other) { } } } + + private static final class RolesRetrievalResult { + + private final Set roleDescriptors = new HashSet<>(); + private Set missingRoles = Collections.emptySet(); + private boolean success = true; + + private void addDescriptors(Set descriptors) { + roleDescriptors.addAll(descriptors); + } + + private Set getRoleDescriptors() { + return roleDescriptors; + } + + private void setFailure() { + success = false; + } + + private boolean isSuccess() { + return success; + } + + private void setMissingRoles(Set missingRoles) { + this.missingRoles = missingRoles; + } + + private Set getMissingRoles() { + return missingRoles; + } + } + + public static List> getSettings() { + return Arrays.asList(CACHE_SIZE_SETTING, NEGATIVE_LOOKUP_CACHE_SIZE_SETTING); + } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java index 868a7076b8b1f..ccc4f1fe3ea13 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/FileRolesStore.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.action.ActionListener; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Settings; @@ -27,6 +28,7 @@ import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.core.security.support.NoOpLogger; import org.elasticsearch.xpack.core.security.support.Validation; @@ -42,6 +44,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -49,7 +52,7 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableMap; -public class FileRolesStore extends AbstractComponent { +public class FileRolesStore extends AbstractComponent implements BiConsumer, ActionListener> { private static final Pattern IN_SEGMENT_LINE = Pattern.compile("^\\s+.+"); private static final Pattern SKIP_LINE = Pattern.compile("(^#.*|^\\s*)"); @@ -79,7 +82,13 @@ public FileRolesStore(Settings settings, Environment env, ResourceWatcherService permissions = parseFile(file, logger, settings, licenseState); } - public Set roleDescriptors(Set roleNames) { + + @Override + public void accept(Set names, ActionListener listener) { + listener.onResponse(RoleRetrievalResult.success(roleDescriptors(names))); + } + + Set roleDescriptors(Set roleNames) { final Map localPermissions = permissions; Set descriptors = new HashSet<>(); roleNames.forEach((name) -> { @@ -129,6 +138,11 @@ Set getAllRoleNames() { return permissions.keySet(); } + @Override + public String toString() { + return "file roles store (" + file + ")"; + } + public static Path resolveFile(Environment env) { return XPackPlugin.resolveConfigFile(env, "roles.yml"); } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java index e032d524038a7..d604d166812c8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authz/store/NativeRolesStore.java @@ -19,7 +19,6 @@ import org.elasticsearch.action.search.MultiSearchResponse.Item; import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.support.ContextPreservingActionListener; -import org.elasticsearch.action.support.TransportActions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; @@ -43,6 +42,7 @@ import org.elasticsearch.xpack.core.security.action.role.PutRoleRequest; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.IndicesPrivileges; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.core.security.client.SecurityClient; import org.elasticsearch.xpack.security.support.SecurityIndexManager; @@ -52,9 +52,12 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.function.BiConsumer; import java.util.function.Supplier; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -75,7 +78,7 @@ * * No caching is done by this class, it is handled at a higher level */ -public class NativeRolesStore extends AbstractComponent { +public class NativeRolesStore extends AbstractComponent implements BiConsumer, ActionListener> { // these are no longer used, but leave them around for users upgrading private static final Setting CACHE_SIZE_SETTING = @@ -98,24 +101,27 @@ public NativeRolesStore(Settings settings, Client client, XPackLicenseState lice this.securityIndex = securityIndex; } + @Override + public void accept(Set names, ActionListener listener) { + getRoleDescriptors(names, listener); + } + /** * Retrieve a list of roles, if rolesToGet is null or empty, fetch all roles */ - public void getRoleDescriptors(String[] names, final ActionListener> listener) { + public void getRoleDescriptors(Set names, final ActionListener listener) { if (securityIndex.indexExists() == false) { // TODO remove this short circuiting and fix tests that fail without this! - listener.onResponse(Collections.emptyList()); - } else if (names != null && names.length == 1) { - getRoleDescriptor(Objects.requireNonNull(names[0]), ActionListener.wrap(roleDescriptor -> - listener.onResponse(roleDescriptor == null ? Collections.emptyList() : Collections.singletonList(roleDescriptor)), - listener::onFailure)); + listener.onResponse(RoleRetrievalResult.success(Collections.emptySet())); + } else if (names != null && names.size() == 1) { + getRoleDescriptor(Objects.requireNonNull(names.iterator().next()), listener); } else { securityIndex.prepareIndexIfNeededThenExecute(listener::onFailure, () -> { QueryBuilder query; - if (names == null || names.length == 0) { + if (names == null || names.isEmpty()) { query = QueryBuilders.termQuery(RoleDescriptor.Fields.TYPE.getPreferredName(), ROLE_TYPE); } else { - final String[] roleNames = Arrays.stream(names).map(s -> getIdForUser(s)).toArray(String[]::new); + final String[] roleNames = names.stream().map(NativeRolesStore::getIdForUser).toArray(String[]::new); query = QueryBuilders.boolQuery().filter(QueryBuilders.idsQuery(ROLE_DOC_TYPE).addIds(roleNames)); } final Supplier supplier = client.threadPool().getThreadContext().newRestorableContext(false); @@ -127,7 +133,10 @@ public void getRoleDescriptors(String[] names, final ActionListener(supplier, listener), + final ActionListener> descriptorsListener = ActionListener.wrap( + roleDescriptors -> listener.onResponse(RoleRetrievalResult.success(new HashSet<>(roleDescriptors))), + e -> listener.onResponse(RoleRetrievalResult.failure(e))); + ScrollHelper.fetchAllByEntity(client, request, new ContextPreservingActionListener<>(supplier, descriptorsListener), (hit) -> transformRole(hit.getId(), hit.getSourceRef(), logger, licenseState)); } }); @@ -261,30 +270,28 @@ public void onFailure(Exception e) { } } - private void getRoleDescriptor(final String roleId, ActionListener roleActionListener) { + @Override + public String toString() { + return "native roles store"; + } + + private void getRoleDescriptor(final String roleId, ActionListener resultListener) { if (securityIndex.indexExists() == false) { // TODO remove this short circuiting and fix tests that fail without this! - roleActionListener.onResponse(null); + resultListener.onResponse(RoleRetrievalResult.success(Collections.emptySet())); } else { - securityIndex.prepareIndexIfNeededThenExecute(roleActionListener::onFailure, () -> + securityIndex.prepareIndexIfNeededThenExecute(e -> resultListener.onResponse(RoleRetrievalResult.failure(e)), () -> executeGetRoleRequest(roleId, new ActionListener() { @Override public void onResponse(GetResponse response) { final RoleDescriptor descriptor = transformRole(response); - roleActionListener.onResponse(descriptor); + resultListener.onResponse(RoleRetrievalResult.success( + descriptor == null ? Collections.emptySet() : Collections.singleton(descriptor))); } @Override public void onFailure(Exception e) { - // if the index or the shard is not there / available we just claim the role is not there - if (TransportActions.isShardNotAvailableException(e)) { - logger.warn((org.apache.logging.log4j.util.Supplier) () -> - new ParameterizedMessage("failed to load role [{}] index not available", roleId), e); - roleActionListener.onResponse(null); - } else { - logger.error(new ParameterizedMessage("failed to load role [{}]", roleId), e); - roleActionListener.onFailure(e); - } + resultListener.onResponse(RoleRetrievalResult.failure(e)); } })); } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java index d269de25c612d..79d1d19c50e8a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/integration/ClearRolesCacheTests.java @@ -11,7 +11,7 @@ import org.elasticsearch.xpack.core.security.action.role.DeleteRoleResponse; import org.elasticsearch.xpack.core.security.action.role.GetRolesResponse; import org.elasticsearch.xpack.core.security.action.role.PutRoleResponse; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.core.security.client.SecurityClient; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import org.elasticsearch.xpack.security.support.SecurityIndexManager; @@ -19,8 +19,9 @@ import org.junit.BeforeClass; import java.util.Arrays; -import java.util.Collection; +import java.util.HashSet; import java.util.List; +import java.util.Set; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.NONE; @@ -57,11 +58,13 @@ public void setupForTests() { ensureGreen(SecurityIndexManager.SECURITY_INDEX_NAME); + final Set rolesSet = new HashSet<>(Arrays.asList(roles)); // warm up the caches on every node for (NativeRolesStore rolesStore : internalCluster().getInstances(NativeRolesStore.class)) { - PlainActionFuture> future = new PlainActionFuture<>(); - rolesStore.getRoleDescriptors(roles, future); + PlainActionFuture future = new PlainActionFuture<>(); + rolesStore.getRoleDescriptors(rolesSet, future); assertThat(future.actionGet(), notNullValue()); + assertTrue(future.actionGet().isSuccess()); } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java index eecf3f0202f32..9fffba3195515 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/role/TransportGetRolesActionTests.java @@ -18,12 +18,15 @@ import org.elasticsearch.xpack.core.security.action.role.GetRolesResponse; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.security.authz.store.NativeRolesStore; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -31,8 +34,8 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; -import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -56,10 +59,10 @@ public void testReservedRoles() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; - listener.onResponse(Collections.emptyList()); + ActionListener listener = (ActionListener) args[1]; + listener.onResponse(RoleRetrievalResult.success(Collections.emptySet())); return null; - }).when(rolesStore).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); + }).when(rolesStore).getRoleDescriptors(eq(new HashSet<>()), any(ActionListener.class)); GetRolesRequest request = new GetRolesRequest(); request.names(names.toArray(Strings.EMPTY_ARRAY)); @@ -100,10 +103,10 @@ public void testStoreRoles() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; - listener.onResponse(storeRoleDescriptors); + ActionListener listener = (ActionListener) args[1]; + listener.onResponse(RoleRetrievalResult.success(new HashSet<>(storeRoleDescriptors))); return null; - }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); + }).when(rolesStore).getRoleDescriptors(eq(new HashSet<>(Arrays.asList(request.names()))), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); @@ -160,18 +163,17 @@ public void testGetAllOrMix() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); assert args.length == 2; - String[] requestedNames1 = (String[]) args[0]; - ActionListener> listener = (ActionListener>) args[1]; - if (requestedNames1.length == 0) { - listener.onResponse(storeRoleDescriptors); + Set requestedNames1 = (Set) args[0]; + ActionListener listener = (ActionListener) args[1]; + if (requestedNames1.size() == 0) { + listener.onResponse(RoleRetrievalResult.success(new HashSet<>(storeRoleDescriptors))); } else { - List requestedNamesList = Arrays.asList(requestedNames1); - listener.onResponse(storeRoleDescriptors.stream() - .filter(r -> requestedNamesList.contains(r.getName())) - .collect(Collectors.toList())); + listener.onResponse(RoleRetrievalResult.success(storeRoleDescriptors.stream() + .filter(r -> requestedNames1.contains(r.getName())) + .collect(Collectors.toSet()))); } return null; - }).when(rolesStore).getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); + }).when(rolesStore).getRoleDescriptors(eq(new HashSet<>(specificStoreNames)), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); @@ -194,10 +196,10 @@ public void onFailure(Exception e) { assertThat(retrievedRoleNames, containsInAnyOrder(expectedNames.toArray(Strings.EMPTY_ARRAY))); if (all) { - verify(rolesStore, times(1)).getRoleDescriptors(aryEq(Strings.EMPTY_ARRAY), any(ActionListener.class)); + verify(rolesStore, times(1)).getRoleDescriptors(eq(new HashSet<>()), any(ActionListener.class)); } else { verify(rolesStore, times(1)) - .getRoleDescriptors(aryEq(specificStoreNames.toArray(Strings.EMPTY_ARRAY)), any(ActionListener.class)); + .getRoleDescriptors(eq(new HashSet<>(specificStoreNames)), any(ActionListener.class)); } } @@ -216,10 +218,10 @@ public void testException() { doAnswer(invocation -> { Object[] args = invocation.getArguments(); assert args.length == 2; - ActionListener> listener = (ActionListener>) args[1]; + ActionListener listener = (ActionListener) args[1]; listener.onFailure(e); return null; - }).when(rolesStore).getRoleDescriptors(aryEq(request.names()), any(ActionListener.class)); + }).when(rolesStore).getRoleDescriptors(eq(new HashSet<>(Arrays.asList(request.names()))), any(ActionListener.class)); final AtomicReference throwableRef = new AtomicReference<>(); final AtomicReference responseRef = new AtomicReference<>(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java index 9f1490856d67b..6801ffd6bdf84 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authz/store/CompositeRolesStoreTests.java @@ -41,6 +41,7 @@ import org.elasticsearch.xpack.core.security.authz.privilege.ConditionalClusterPrivilege; import org.elasticsearch.xpack.core.security.authz.privilege.IndexPrivilege; import org.elasticsearch.xpack.core.security.authz.store.ReservedRolesStore; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import org.elasticsearch.xpack.security.support.SecurityIndexManager; import java.io.IOException; @@ -66,6 +67,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doCallRealMethod; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -112,12 +114,18 @@ public void testRolesWhenDlsFlsUnlicensed() throws IOException { .build() }, null); FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + ReservedRolesStore reservedRolesStore = mock(ReservedRolesStore.class); + doCallRealMethod().when(reservedRolesStore).accept(any(Set.class), any(ActionListener.class)); + NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); + when(fileRolesStore.roleDescriptors(Collections.singleton("fls"))).thenReturn(Collections.singleton(flsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("dls"))).thenReturn(Collections.singleton(dlsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("fls_dls"))).thenReturn(Collections.singleton(flsDlsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("no_fls_dls"))).thenReturn(Collections.singleton(noFlsDlsRole)); - CompositeRolesStore compositeRolesStore = new CompositeRolesStore(Settings.EMPTY, fileRolesStore, mock(NativeRolesStore.class), - mock(ReservedRolesStore.class), mock(NativePrivilegeStore.class), Collections.emptyList(), + CompositeRolesStore compositeRolesStore = new CompositeRolesStore(Settings.EMPTY, fileRolesStore, nativeRolesStore, + reservedRolesStore, mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(Settings.EMPTY), licenseState); FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); @@ -173,12 +181,17 @@ public void testRolesWhenDlsFlsLicensed() throws IOException { .build() }, null); FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + ReservedRolesStore reservedRolesStore = mock(ReservedRolesStore.class); + doCallRealMethod().when(reservedRolesStore).accept(any(Set.class), any(ActionListener.class)); + NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); when(fileRolesStore.roleDescriptors(Collections.singleton("fls"))).thenReturn(Collections.singleton(flsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("dls"))).thenReturn(Collections.singleton(dlsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("fls_dls"))).thenReturn(Collections.singleton(flsDlsRole)); when(fileRolesStore.roleDescriptors(Collections.singleton("no_fls_dls"))).thenReturn(Collections.singleton(noFlsDlsRole)); - CompositeRolesStore compositeRolesStore = new CompositeRolesStore(Settings.EMPTY, fileRolesStore, mock(NativeRolesStore.class), - mock(ReservedRolesStore.class), mock(NativePrivilegeStore.class), Collections.emptyList(), + CompositeRolesStore compositeRolesStore = new CompositeRolesStore(Settings.EMPTY, fileRolesStore, nativeRolesStore, + reservedRolesStore, mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(Settings.EMPTY), licenseState); FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); @@ -201,13 +214,15 @@ public void testRolesWhenDlsFlsLicensed() throws IOException { public void testNegativeLookupsAreCached() { final FileRolesStore fileRolesStore = mock(FileRolesStore.class); - when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); + when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); doAnswer((invocationOnMock) -> { - ActionListener> callback = (ActionListener>) invocationOnMock.getArguments()[1]; - callback.onResponse(Collections.emptySet()); + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.success(Collections.emptySet())); return null; - }).when(nativeRolesStore).getRoleDescriptors(isA(String[].class), any(ActionListener.class)); + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore()); final CompositeRolesStore compositeRolesStore = @@ -222,9 +237,11 @@ public void testNegativeLookupsAreCached() { compositeRolesStore.roles(Collections.singleton(roleName), fieldPermissionsCache, future); final Role role = future.actionGet(); assertEquals(Role.EMPTY, role); - verify(reservedRolesStore).roleDescriptors(); + verify(reservedRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); verify(fileRolesStore).roleDescriptors(eq(Collections.singleton(roleName))); - verify(nativeRolesStore).getRoleDescriptors(isA(String[].class), any(ActionListener.class)); + verify(nativeRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); final int numberOfTimesToCall = scaledRandomIntBetween(0, 32); final boolean getSuperuserRole = randomBoolean() @@ -239,23 +256,107 @@ public void testNegativeLookupsAreCached() { if (getSuperuserRole && numberOfTimesToCall > 0) { // the superuser role was requested so we get the role descriptors again - verify(reservedRolesStore, times(2)).roleDescriptors(); + verify(reservedRolesStore, times(2)).accept(anySetOf(String.class), any(ActionListener.class)); } verifyNoMoreInteractions(fileRolesStore, reservedRolesStore, nativeRolesStore); + } + + public void testNegativeLookupsCacheDisabled() { + final FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); + when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); + doAnswer((invocationOnMock) -> { + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.success(Collections.emptySet())); + return null; + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); + final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore()); + + final Settings settings = Settings.builder().put(SECURITY_ENABLED_SETTINGS) + .put("xpack.security.authz.store.roles.negative_lookup_cache.max_size", 0) + .build(); + final CompositeRolesStore compositeRolesStore = new CompositeRolesStore(settings, fileRolesStore, nativeRolesStore, + reservedRolesStore, mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(settings), + new XPackLicenseState(settings)); + verify(fileRolesStore).addListener(any(Consumer.class)); // adds a listener in ctor + + final String roleName = randomAlphaOfLengthBetween(1, 10); + PlainActionFuture future = new PlainActionFuture<>(); + final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); + compositeRolesStore.roles(Collections.singleton(roleName), fieldPermissionsCache, future); + final Role role = future.actionGet(); + assertEquals(Role.EMPTY, role); + verify(reservedRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore).roleDescriptors(eq(Collections.singleton(roleName))); + verify(nativeRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); + + assertFalse(compositeRolesStore.isValueInNegativeLookupCache(roleName)); + verifyNoMoreInteractions(fileRolesStore, reservedRolesStore, nativeRolesStore); + } + + public void testNegativeLookupsAreNotCachedWithFailures() { + final FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); + when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); + doAnswer((invocationOnMock) -> { + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.failure(new RuntimeException("intentionally failed!"))); + return null; + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); + final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore()); - // force a cache clear + final CompositeRolesStore compositeRolesStore = + new CompositeRolesStore(SECURITY_ENABLED_SETTINGS, fileRolesStore, nativeRolesStore, reservedRolesStore, + mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(SECURITY_ENABLED_SETTINGS), + new XPackLicenseState(SECURITY_ENABLED_SETTINGS)); + verify(fileRolesStore).addListener(any(Consumer.class)); // adds a listener in ctor + final String roleName = randomAlphaOfLengthBetween(1, 10); + PlainActionFuture future = new PlainActionFuture<>(); + final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(Settings.EMPTY); + compositeRolesStore.roles(Collections.singleton(roleName), fieldPermissionsCache, future); + final Role role = future.actionGet(); + assertEquals(Role.EMPTY, role); + verify(reservedRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore).roleDescriptors(eq(Collections.singleton(roleName))); + verify(nativeRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); + verify(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); + + final int numberOfTimesToCall = scaledRandomIntBetween(0, 32); + final Set names = Collections.singleton(roleName); + for (int i = 0; i < numberOfTimesToCall; i++) { + future = new PlainActionFuture<>(); + compositeRolesStore.roles(names, fieldPermissionsCache, future); + future.actionGet(); + } + + assertFalse(compositeRolesStore.isValueInNegativeLookupCache(roleName)); + verify(reservedRolesStore, times(numberOfTimesToCall + 1)).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore, times(numberOfTimesToCall + 1)).accept(anySetOf(String.class), any(ActionListener.class)); + verify(fileRolesStore, times(numberOfTimesToCall + 1)).roleDescriptors(eq(Collections.singleton(roleName))); + verify(nativeRolesStore, times(numberOfTimesToCall + 1)).accept(anySetOf(String.class), any(ActionListener.class)); + verify(nativeRolesStore, times(numberOfTimesToCall + 1)).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); + verifyNoMoreInteractions(fileRolesStore, reservedRolesStore, nativeRolesStore); } public void testCustomRolesProviders() { final FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); doAnswer((invocationOnMock) -> { - ActionListener> callback = (ActionListener>) invocationOnMock.getArguments()[1]; - callback.onResponse(Collections.emptySet()); + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.success(Collections.emptySet())); return null; - }).when(nativeRolesStore).getRoleDescriptors(isA(String[].class), any(ActionListener.class)); + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); final ReservedRolesStore reservedRolesStore = spy(new ReservedRolesStore()); final InMemoryRolesProvider inMemoryProvider1 = spy(new InMemoryRolesProvider((roles) -> { @@ -266,7 +367,7 @@ public void testCustomRolesProviders() { IndicesPrivileges.builder().privileges("READ").indices("foo").grantedFields("*").build() }, null)); } - return descriptors; + return RoleRetrievalResult.success(descriptors); })); final InMemoryRolesProvider inMemoryProvider2 = spy(new InMemoryRolesProvider((roles) -> { @@ -285,7 +386,7 @@ public void testCustomRolesProviders() { IndicesPrivileges.builder().privileges("READ").indices("bar").grantedFields("*").build() }, null)); } - return descriptors; + return RoleRetrievalResult.success(descriptors); })); final CompositeRolesStore compositeRolesStore = @@ -468,13 +569,15 @@ public void testMergingBasicRoles() { public void testCustomRolesProviderFailures() throws Exception { final FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); doAnswer((invocationOnMock) -> { - ActionListener> callback = (ActionListener>) invocationOnMock.getArguments()[1]; - callback.onResponse(Collections.emptySet()); + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.success(Collections.emptySet())); return null; - }).when(nativeRolesStore).getRoleDescriptors(isA(String[].class), any(ActionListener.class)); + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(); final InMemoryRolesProvider inMemoryProvider1 = new InMemoryRolesProvider((roles) -> { @@ -485,10 +588,10 @@ public void testCustomRolesProviderFailures() throws Exception { IndicesPrivileges.builder().privileges("READ").indices("foo").grantedFields("*").build() }, null)); } - return descriptors; + return RoleRetrievalResult.success(descriptors); }); - final BiConsumer, ActionListener>> failingProvider = + final BiConsumer, ActionListener> failingProvider = (roles, listener) -> listener.onFailure(new Exception("fake failure")); final CompositeRolesStore compositeRolesStore = @@ -510,13 +613,15 @@ public void testCustomRolesProviderFailures() throws Exception { public void testCustomRolesProvidersLicensing() { final FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); when(fileRolesStore.roleDescriptors(anySetOf(String.class))).thenReturn(Collections.emptySet()); final NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(anySetOf(String.class), any(ActionListener.class)); doAnswer((invocationOnMock) -> { - ActionListener> callback = (ActionListener>) invocationOnMock.getArguments()[1]; - callback.onResponse(Collections.emptySet()); + ActionListener callback = (ActionListener) invocationOnMock.getArguments()[1]; + callback.onResponse(RoleRetrievalResult.success(Collections.emptySet())); return null; - }).when(nativeRolesStore).getRoleDescriptors(isA(String[].class), any(ActionListener.class)); + }).when(nativeRolesStore).getRoleDescriptors(isA(Set.class), any(ActionListener.class)); final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(); final InMemoryRolesProvider inMemoryProvider = new InMemoryRolesProvider((roles) -> { @@ -527,7 +632,7 @@ public void testCustomRolesProvidersLicensing() { IndicesPrivileges.builder().privileges("READ").indices("foo").grantedFields("*").build() }, null)); } - return descriptors; + return RoleRetrievalResult.success(descriptors); }); UpdatableLicenseState xPackLicenseState = new UpdatableLicenseState(SECURITY_ENABLED_SETTINGS); @@ -580,8 +685,14 @@ private SecurityIndexManager.State dummyState(ClusterHealthStatus indexStatus) { public void testCacheClearOnIndexHealthChange() { final AtomicInteger numInvalidation = new AtomicInteger(0); + FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + ReservedRolesStore reservedRolesStore = mock(ReservedRolesStore.class); + doCallRealMethod().when(reservedRolesStore).accept(any(Set.class), any(ActionListener.class)); + NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); CompositeRolesStore compositeRolesStore = new CompositeRolesStore( - Settings.EMPTY, mock(FileRolesStore.class), mock(NativeRolesStore.class), mock(ReservedRolesStore.class), + Settings.EMPTY, fileRolesStore, nativeRolesStore, reservedRolesStore, mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(Settings.EMPTY), new XPackLicenseState(SECURITY_ENABLED_SETTINGS)) { @Override @@ -626,8 +737,14 @@ public void invalidateAll() { public void testCacheClearOnIndexOutOfDateChange() { final AtomicInteger numInvalidation = new AtomicInteger(0); + FileRolesStore fileRolesStore = mock(FileRolesStore.class); + doCallRealMethod().when(fileRolesStore).accept(any(Set.class), any(ActionListener.class)); + ReservedRolesStore reservedRolesStore = mock(ReservedRolesStore.class); + doCallRealMethod().when(reservedRolesStore).accept(any(Set.class), any(ActionListener.class)); + NativeRolesStore nativeRolesStore = mock(NativeRolesStore.class); + doCallRealMethod().when(nativeRolesStore).accept(any(Set.class), any(ActionListener.class)); CompositeRolesStore compositeRolesStore = new CompositeRolesStore(SECURITY_ENABLED_SETTINGS, - mock(FileRolesStore.class), mock(NativeRolesStore.class), mock(ReservedRolesStore.class), + fileRolesStore, nativeRolesStore, reservedRolesStore, mock(NativePrivilegeStore.class), Collections.emptyList(), new ThreadContext(SECURITY_ENABLED_SETTINGS), new XPackLicenseState(SECURITY_ENABLED_SETTINGS)) { @Override @@ -647,15 +764,15 @@ public void invalidateAll() { assertEquals(2, numInvalidation.get()); } - private static class InMemoryRolesProvider implements BiConsumer, ActionListener>> { - private final Function, Set> roleDescriptorsFunc; + private static class InMemoryRolesProvider implements BiConsumer, ActionListener> { + private final Function, RoleRetrievalResult> roleDescriptorsFunc; - InMemoryRolesProvider(Function, Set> roleDescriptorsFunc) { + InMemoryRolesProvider(Function, RoleRetrievalResult> roleDescriptorsFunc) { this.roleDescriptorsFunc = roleDescriptorsFunc; } @Override - public void accept(Set roles, ActionListener> listener) { + public void accept(Set roles, ActionListener listener) { listener.onResponse(roleDescriptorsFunc.apply(roles)); } } diff --git a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java index e426265c8a467..90b5eefcb56d4 100644 --- a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java +++ b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java @@ -13,8 +13,8 @@ import org.elasticsearch.watcher.ResourceWatcherService; import org.elasticsearch.xpack.core.security.authc.AuthenticationFailureHandler; import org.elasticsearch.xpack.core.security.authc.Realm; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.SecurityExtension; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import java.security.AccessController; import java.security.PrivilegedAction; @@ -54,7 +54,7 @@ public AuthenticationFailureHandler getAuthenticationFailureHandler() { @Override - public List, ActionListener>>> + public List, ActionListener>> getRolesProviders(Settings settings, ResourceWatcherService resourceWatcherService) { CustomInMemoryRolesProvider rp1 = new CustomInMemoryRolesProvider(settings, Collections.singletonMap(ROLE_A, "read")); Map roles = new HashMap<>(); diff --git a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/role/CustomInMemoryRolesProvider.java b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/role/CustomInMemoryRolesProvider.java index df9d3b5a6b875..0d5a71e6244b4 100644 --- a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/role/CustomInMemoryRolesProvider.java +++ b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/role/CustomInMemoryRolesProvider.java @@ -9,6 +9,7 @@ import org.elasticsearch.common.component.AbstractComponent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.store.RoleRetrievalResult; import java.util.HashSet; import java.util.Map; @@ -21,7 +22,7 @@ */ public class CustomInMemoryRolesProvider extends AbstractComponent - implements BiConsumer, ActionListener>> { + implements BiConsumer, ActionListener> { public static final String INDEX = "foo"; public static final String ROLE_A = "roleA"; @@ -35,7 +36,7 @@ public CustomInMemoryRolesProvider(Settings settings, Map rolePe } @Override - public void accept(Set roles, ActionListener> listener) { + public void accept(Set roles, ActionListener listener) { Set roleDescriptors = new HashSet<>(); for (String role : roles) { if (rolePermissionSettings.containsKey(role)) { @@ -52,6 +53,6 @@ public void accept(Set roles, ActionListener> listen } } - listener.onResponse(roleDescriptors); + listener.onResponse(RoleRetrievalResult.success(roleDescriptors)); } }