Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service Accounts - cache clear API #71605

Merged
merged 5 commits into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,17 @@ public void testGetServiceAccountTokens() throws IOException {
assertThat(responseAsMap(deleteTokenResponse2).get("found"), is(false));
}

public void testClearCache() throws IOException {
final Request clearCacheRequest = new Request("POST", "_security/service/elastic/fleet-server/credential/token/"
+ randomFrom("", "*", "api-token-1", "api-token-1,api-token2") + "/_clear_cache");
final Response clearCacheResponse = client().performRequest(clearCacheRequest);
assertOK(clearCacheResponse);
final Map<String, Object> clearCacheResponseMap = responseAsMap(clearCacheResponse);
@SuppressWarnings("unchecked")
final Map<String, Object> nodesMap = (Map<String, Object>) clearCacheResponseMap.get("_nodes");
assertThat(nodesMap.get("failed"), equalTo(0));
}

public void testManageOwnApiKey() throws IOException {
final String token;
if (randomBoolean()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@
package org.elasticsearch.xpack.security.authc.service;

import org.elasticsearch.Version;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.Client;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.cache.Cache;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.ListenableFuture;
import org.elasticsearch.node.Node;
import org.elasticsearch.test.SecuritySingleNodeTestCase;
import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheAction;
import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheRequest;
import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheResponse;
import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenAction;
import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenRequest;
import org.elasticsearch.xpack.core.security.action.service.CreateServiceAccountTokenResponse;
Expand Down Expand Up @@ -99,6 +104,40 @@ public void testApiServiceAccountToken() {
assertThat(cache.count(), equalTo(0));
}

public void testClearCache() {
final IndexServiceAccountsTokenStore indexStore = node().injector().getInstance(IndexServiceAccountsTokenStore.class);
final Cache<String, ListenableFuture<CachingServiceAccountsTokenStore.CachedResult>> cache = indexStore.getCache();
final SecureString secret1 = createApiServiceToken("api-token-1");
final SecureString secret2 = createApiServiceToken("api-token-2");
assertThat(cache.count(), equalTo(0));

authenticateWithApiToken("api-token-1", secret1);
assertThat(cache.count(), equalTo(1));
authenticateWithApiToken("api-token-2", secret2);
assertThat(cache.count(), equalTo(2));

final ClearSecurityCacheRequest clearSecurityCacheRequest1 = new ClearSecurityCacheRequest().cacheName("service");
if (randomBoolean()) {
clearSecurityCacheRequest1.keys("elastic/fleet-server/");
}
final PlainActionFuture<ClearSecurityCacheResponse> future1 = new PlainActionFuture<>();
client().execute(ClearSecurityCacheAction.INSTANCE, clearSecurityCacheRequest1, future1);
assertThat(future1.actionGet().failures().isEmpty(), is(true));
assertThat(cache.count(), equalTo(0));

authenticateWithApiToken("api-token-1", secret1);
assertThat(cache.count(), equalTo(1));
authenticateWithApiToken("api-token-2", secret2);
assertThat(cache.count(), equalTo(2));

final ClearSecurityCacheRequest clearSecurityCacheRequest2
= new ClearSecurityCacheRequest().cacheName("service").keys("elastic/fleet-server/api-token-" + randomFrom("1", "2"));
final PlainActionFuture<ClearSecurityCacheResponse> future2 = new PlainActionFuture<>();
client().execute(ClearSecurityCacheAction.INSTANCE, clearSecurityCacheRequest2, future2);
assertThat(future2.actionGet().failures().isEmpty(), is(true));
assertThat(cache.count(), equalTo(1));
}

private Client createServiceAccountClient() {
return createServiceAccountClient(BEARER_TOKEN);
}
Expand All @@ -116,4 +155,21 @@ private Authentication getExpectedAuthentication(String tokenName) {
null, Version.CURRENT, Authentication.AuthenticationType.TOKEN, Map.of("_token_name", tokenName)
);
}

private SecureString createApiServiceToken(String tokenName) {
final CreateServiceAccountTokenRequest createServiceAccountTokenRequest =
new CreateServiceAccountTokenRequest("elastic", "fleet-server", tokenName);
final CreateServiceAccountTokenResponse createServiceAccountTokenResponse =
client().execute(CreateServiceAccountTokenAction.INSTANCE, createServiceAccountTokenRequest).actionGet();
assertThat(createServiceAccountTokenResponse.getName(), equalTo(tokenName));
return createServiceAccountTokenResponse.getValue();
}

private void authenticateWithApiToken(String tokenName, SecureString secret) {
final AuthenticateRequest authenticateRequest = new AuthenticateRequest("elastic/fleet-server");
final AuthenticateResponse authenticateResponse =
createServiceAccountClient(secret.toString())
.execute(AuthenticateAction.INSTANCE, authenticateRequest).actionGet();
assertThat(authenticateResponse.authentication(), equalTo(getExpectedAuthentication(tokenName)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlLogoutAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlPrepareAuthenticationAction;
import org.elasticsearch.xpack.security.rest.action.saml.RestSamlSpMetadataAction;
import org.elasticsearch.xpack.security.rest.action.service.RestClearServiceAccountTokenStoreCacheAction;
import org.elasticsearch.xpack.security.rest.action.service.RestCreateServiceAccountTokenAction;
import org.elasticsearch.xpack.security.rest.action.service.RestDeleteServiceAccountTokenAction;
import org.elasticsearch.xpack.security.rest.action.service.RestGetServiceAccountAction;
Expand Down Expand Up @@ -490,6 +491,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
securityIndex.get().addIndexStateListener(privilegeStore::onSecurityIndexStateChange);

final CacheInvalidatorRegistry cacheInvalidatorRegistry = new CacheInvalidatorRegistry();
cacheInvalidatorRegistry.registerAlias("service", Set.of("file_service_account_token", "index_service_account_token"));
components.add(cacheInvalidatorRegistry);
securityIndex.get().addIndexStateListener(cacheInvalidatorRegistry::onSecurityIndexStageChange);

Expand All @@ -516,7 +518,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
components.add(indexServiceAccountsTokenStore);

final FileServiceAccountsTokenStore fileServiceAccountsTokenStore =
new FileServiceAccountsTokenStore(environment, resourceWatcherService, threadPool);
new FileServiceAccountsTokenStore(environment, resourceWatcherService, threadPool, cacheInvalidatorRegistry);

final ServiceAccountService serviceAccountService = new ServiceAccountService(new CompositeServiceAccountsTokenStore(
List.of(fileServiceAccountsTokenStore, indexServiceAccountsTokenStore), threadPool.getThreadContext()), httpTlsRuntimeCheck);
Expand Down Expand Up @@ -583,6 +585,8 @@ auditTrailService, failureHandler, threadPool, anonymousUser, getAuthorizationEn

components.add(new SecurityUsageServices(realms, allRolesStore, nativeRoleMappingStore, ipFilter.get()));

cacheInvalidatorRegistry.validate();

return components;
}

Expand Down Expand Up @@ -906,6 +910,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
new RestClearRolesCacheAction(settings, getLicenseState()),
new RestClearPrivilegesCacheAction(settings, getLicenseState()),
new RestClearApiKeyCacheAction(settings, getLicenseState()),
new RestClearServiceAccountTokenStoreCacheAction(settings, getLicenseState()),
new RestGetUsersAction(settings, getLicenseState()),
new RestPutUserAction(settings, getLicenseState()),
new RestDeleteUserAction(settings, getLicenseState()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.common.util.concurrent.ListenableFuture;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.support.CacheIteratorHelper;
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;

import java.util.Collection;
Expand All @@ -40,6 +41,7 @@ public abstract class CachingServiceAccountsTokenStore implements ServiceAccount
private final Settings settings;
private final ThreadPool threadPool;
private final Cache<String, ListenableFuture<CachedResult>> cache;
private CacheIteratorHelper<String, ListenableFuture<CachedResult>> cacheIteratorHelper;
private final Hasher hasher;

CachingServiceAccountsTokenStore(Settings settings, ThreadPool threadPool) {
Expand All @@ -51,8 +53,10 @@ public abstract class CachingServiceAccountsTokenStore implements ServiceAccount
.setExpireAfterWrite(ttl)
.setMaximumWeight(CACHE_MAX_TOKENS_SETTING.get(settings))
.build();
cacheIteratorHelper = new CacheIteratorHelper<>(cache);
} else {
cache = null;
cacheIteratorHelper = null;
}
hasher = Hasher.resolve(CACHE_HASH_ALGO_SETTING.get(settings));
}
Expand Down Expand Up @@ -92,7 +96,12 @@ private void authenticateWithCache(ServiceAccountToken token, ActionListener<Boo
}, listener::onFailure), threadPool.generic(), threadPool.getThreadContext());
} else {
doAuthenticate(token, ActionListener.wrap(success -> {
logger.trace("cache service token [{}] authentication result", token.getQualifiedName());
if (false == success) {
// Do not cache failed attempt
cache.invalidate(token.getQualifiedName(), listenableCacheEntry);
} else {
logger.trace("cache service token [{}] authentication result", token.getQualifiedName());
}
Comment on lines +99 to +104
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change so the logic is mostly the same as the one of CachingUsernamePasswordRealm. Basically the cache now does not cache negative results. I think this is a better choice since service account will be well known and it is easy to cause cache thrashing if negative results are cached. This is probably the similar argument why CachingUsernamePasswordRealm does not cache negative results as well. Note that ApiKeyService cache is different in that it does cache negative results. But it can afford to do that because the ApiKey ID, unlike username/service account, is not well known.

listenableCacheEntry.onResponse(new CachedResult(hasher, success, token));
listener.onResponse(success);
}, e -> {
Expand All @@ -112,7 +121,17 @@ public final void invalidate(Collection<String> qualifiedTokenNames) {
if (cache != null) {
logger.trace("invalidating cache for service token [{}]",
Strings.collectionToCommaDelimitedString(qualifiedTokenNames));
qualifiedTokenNames.forEach(cache::invalidate);
if (qualifiedTokenNames.size() == 1) {
final String qualifiedTokenName = qualifiedTokenNames.iterator().next();
if (qualifiedTokenName.endsWith("/")) {
// Wildcard case of invalidating all tokens for a service account, e.g. "elastic/fleet-server/"
cacheIteratorHelper.removeKeysIf(key -> key.startsWith(qualifiedTokenName));
} else {
cache.invalidate(qualifiedTokenName);
}
} else {
qualifiedTokenNames.forEach(cache::invalidate);
}
tvernum marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.xpack.core.security.authc.support.Hasher;
import org.elasticsearch.xpack.core.security.support.NoOpLogger;
import org.elasticsearch.xpack.security.authc.service.ServiceAccount.ServiceAccountId;
import org.elasticsearch.xpack.security.support.CacheInvalidatorRegistry;
import org.elasticsearch.xpack.security.support.FileLineParser;
import org.elasticsearch.xpack.security.support.FileReloadListener;
import org.elasticsearch.xpack.security.support.SecurityFiles;
Expand All @@ -47,7 +48,8 @@ public class FileServiceAccountsTokenStore extends CachingServiceAccountsTokenSt
private final CopyOnWriteArrayList<Runnable> refreshListeners;
private volatile Map<String, char[]> tokenHashes;

public FileServiceAccountsTokenStore(Environment env, ResourceWatcherService resourceWatcherService, ThreadPool threadPool) {
public FileServiceAccountsTokenStore(Environment env, ResourceWatcherService resourceWatcherService, ThreadPool threadPool,
CacheInvalidatorRegistry cacheInvalidatorRegistry) {
super(env.settings(), threadPool);
file = resolveFile(env);
FileWatcher watcher = new FileWatcher(file.getParent());
Expand All @@ -63,6 +65,7 @@ public FileServiceAccountsTokenStore(Environment env, ResourceWatcherService res
throw new IllegalStateException("Failed to load service_tokens file [" + file + "]", e);
}
refreshListeners = new CopyOnWriteArrayList<>(List.of(this::invalidateAll));
cacheInvalidatorRegistry.registerCacheInvalidator("file_service_account_token", this);
}

@Override
Expand All @@ -89,6 +92,11 @@ public void addListener(Runnable listener) {
refreshListeners.add(listener);
}

@Override
public boolean shouldClearOnSecurityIndexStateChange() {
return false;
}

private void notifyRefresh() {
refreshListeners.forEach(Runnable::run);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.security.rest.action.service;

import org.elasticsearch.client.node.NodeClient;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.license.XPackLicenseState;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestActions;
import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheAction;
import org.elasticsearch.xpack.core.security.action.ClearSecurityCacheRequest;
import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler;

import java.io.IOException;
import java.util.Arrays;
import java.util.List;

import static org.elasticsearch.rest.RestRequest.Method.POST;

public class RestClearServiceAccountTokenStoreCacheAction extends SecurityBaseRestHandler {

public RestClearServiceAccountTokenStoreCacheAction(Settings settings, XPackLicenseState licenseState) {
super(settings, licenseState);
}

@Override
public List<Route> routes() {
return List.of(new Route(POST, "/_security/service/{namespace}/{service}/credential/token/{name}/_clear_cache"));
}

@Override
public String getName() {
return "xpack_security_clear_service_account_token_store_cache";
}

@Override
protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClient client) throws IOException {
final String namespace = request.param("namespace");
final String service = request.param("service");
String[] tokenNames = request.paramAsStringArrayOrEmptyIfAll("name");

ClearSecurityCacheRequest req = new ClearSecurityCacheRequest().cacheName("service");
if (tokenNames.length == 0) {
// This is the wildcard case for tokenNames
req.keys(namespace + "/" + service + "/");
} else {
req.keys(Arrays.stream(tokenNames).map(name -> namespace + "/" + service + "/" + name).toArray(String[]::new));
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cache clearing URL is different from all the other ones in that the cache key is comprised of three parts, i.e. {namespace}/{service}/{name}. This makes it challenging to conform 100% to existing behaviours. For an exmple, it is not possible to use a single * for clearing everything, it would be more like /_security/service/*/*/credential/token/*/_clear_cache. This is doable but I think it is not really needed for now as it adds complexity for the key comparison logic, e.g. should namespace also support comma separated list?

So for now, I am keep this simple by taking the namespace and service fields literally and only allow * and comma separated list for the token name field. Since we only have a single service account now, this approach can still clear all caches with /_security/service/elastic/fleet-server/credential/token/*/_clear_cache. We should revisit this once we need to add another service account.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine.
I don't think it really makes sense to clear all tokens for all accounts (but not other credential types).

If we get to the point where we think something like this is needed, then maybe we actually want

POST /_security/service/_clear_cache

to clear all caches for all service accounts.

Alternatively, for App Privileges we only support clearing a whole application (not a specific privilege)
We could decide to do something that here if we wanted, and only support clearing by account, not token.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. I'll leave it as is then. Always clearing all tokens of a service account can provide some simplication, but not a lot. Since I already have things in place, I'd just keep them. Plus even with the clearing applicaiton privileges cache API, the URL format is something like .../{application}/_clear_cache which maps better to .../{token_name}/_clear_cache.

return channel -> client.execute(ClearSecurityCacheAction.INSTANCE, req, new RestActions.NodesResponseRestListener<>(channel));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,13 @@

package org.elasticsearch.xpack.security.support;

import org.elasticsearch.common.Strings;
import org.elasticsearch.common.util.set.Sets;

import java.util.Collection;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import static org.elasticsearch.xpack.security.support.SecurityIndexManager.isIndexDeleted;
Expand All @@ -21,6 +25,7 @@
public class CacheInvalidatorRegistry {

private final Map<String, CacheInvalidator> cacheInvalidators = new ConcurrentHashMap<>();
private final Map<String, Set<String>> cacheAliases = new ConcurrentHashMap<>();

public CacheInvalidatorRegistry() {
}
Expand All @@ -32,16 +37,57 @@ public void registerCacheInvalidator(String name, CacheInvalidator cacheInvalida
cacheInvalidators.put(name, cacheInvalidator);
}

public void registerAlias(String alias, Set<String> names) {
Objects.requireNonNull(alias, "cache alias cannot be null");
if (names.isEmpty()) {
throw new IllegalArgumentException("cache names cannot be empty for aliasing");
}
if (cacheAliases.containsKey(alias)) {
throw new IllegalArgumentException("cache alias already exists: [" + alias + "]");
}
cacheAliases.put(alias, names);
}

public void validate() {
for (String alias : cacheAliases.keySet()) {
if (cacheInvalidators.containsKey(alias)) {
throw new IllegalStateException("cache alias cannot clash with cache name: [" + alias + "]");
}
final Set<String> names = cacheAliases.get(alias);
if (false == cacheInvalidators.keySet().containsAll(names)) {
throw new IllegalStateException("cache names not found: ["
+ Strings.collectionToCommaDelimitedString(Sets.difference(names, cacheInvalidators.keySet())) + "]");
}
}
}

public void onSecurityIndexStageChange(SecurityIndexManager.State previousState, SecurityIndexManager.State currentState) {
if (isMoveFromRedToNonRed(previousState, currentState)
|| isIndexDeleted(previousState, currentState)
|| Objects.equals(previousState.indexUUID, currentState.indexUUID) == false
|| previousState.isIndexUpToDate != currentState.isIndexUpToDate) {
cacheInvalidators.values().forEach(CacheInvalidator::invalidateAll);
cacheInvalidators.values().stream()
.filter(CacheInvalidator::shouldClearOnSecurityIndexStateChange).forEach(CacheInvalidator::invalidateAll);
}
}

public void invalidateByKey(String cacheName, Collection<String> keys) {
if (cacheAliases.containsKey(cacheName)) {
cacheAliases.get(cacheName).forEach(name -> doInvalidateByKey(name, keys));
} else {
doInvalidateByKey(cacheName, keys);
}
}

public void invalidateCache(String cacheName) {
if (cacheAliases.containsKey(cacheName)) {
cacheAliases.get(cacheName).forEach(this::doInvalidateCache);
} else {
doInvalidateCache(cacheName);
}
}

private void doInvalidateByKey(String cacheName, Collection<String> keys) {
final CacheInvalidator cacheInvalidator = cacheInvalidators.get(cacheName);
if (cacheInvalidator != null) {
cacheInvalidator.invalidate(keys);
Expand All @@ -50,7 +96,7 @@ public void invalidateByKey(String cacheName, Collection<String> keys) {
}
}

public void invalidateCache(String cacheName) {
private void doInvalidateCache(String cacheName) {
final CacheInvalidator cacheInvalidator = cacheInvalidators.get(cacheName);
if (cacheInvalidator != null) {
cacheInvalidator.invalidateAll();
Expand All @@ -63,5 +109,9 @@ public interface CacheInvalidator {
void invalidate(Collection<String> keys);

void invalidateAll();

default boolean shouldClearOnSecurityIndexStateChange() {
return true;
}
}
}
Loading