Skip to content

More avoidance for loadAuthorizedIndices #81237

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

Merged
merged 19 commits into from
Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
@@ -0,0 +1,54 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.cluster.metadata;

import java.util.HashSet;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;

public class AvailableIndices {

private final Metadata metadata;
private final Predicate<IndexAbstraction> predicate;
private Set<String> availableNames;

public AvailableIndices(Metadata metadata, Predicate<IndexAbstraction> predicate) {
this.metadata = Objects.requireNonNull(metadata);
this.predicate = Objects.requireNonNull(predicate);
}

public AvailableIndices(Set<String> availableNames) {
this.availableNames = availableNames;
this.metadata = null;
this.predicate = null;
}

public Set<String> getAvailableNames() {
if (availableNames == null) {
availableNames = new HashSet<>();
assert predicate != null && metadata != null;
for (IndexAbstraction indexAbstraction : metadata.getIndicesLookup().values()) {
if (predicate.test(indexAbstraction)) {
availableNames.add(indexAbstraction.getName());
}
}
}
return availableNames;
}

public boolean isAvailableName(String name) {
if (availableNames == null) {
assert predicate != null && metadata != null;
return predicate.test(metadata.getIndicesLookup().get(name));
} else {
return availableNames.contains(name);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand Down Expand Up @@ -55,11 +54,30 @@ public List<String> resolveIndexAbstractions(
);
}

// TODO: remove
public List<String> resolveIndexAbstractions(
Iterable<String> indices,
IndicesOptions indicesOptions,
Metadata metadata,
Collection<String> availableIndexAbstractions,
Set<String> availableIndexAbstractions,
boolean replaceWildcards,
boolean includeDataStreams
) {
return resolveIndexAbstractions(
indices,
indicesOptions,
metadata,
new AvailableIndices(availableIndexAbstractions),
replaceWildcards,
includeDataStreams
);
}

public List<String> resolveIndexAbstractions(
Iterable<String> indices,
IndicesOptions indicesOptions,
Metadata metadata,
AvailableIndices availableIndices,
boolean replaceWildcards,
boolean includeDataStreams
) {
Expand All @@ -82,7 +100,7 @@ public List<String> resolveIndexAbstractions(
if (replaceWildcards && Regex.isSimpleMatchPattern(dateMathName)) {
// continue
indexAbstraction = dateMathName;
} else if (availableIndexAbstractions.contains(dateMathName)
} else if (availableIndices.isAvailableName(dateMathName)
&& isIndexVisible(
indexAbstraction,
dateMathName,
Expand All @@ -107,7 +125,7 @@ && isIndexVisible(
if (replaceWildcards && Regex.isSimpleMatchPattern(indexAbstraction)) {
wildcardSeen = true;
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : availableIndexAbstractions) {
for (String authorizedIndex : availableIndices.getAvailableNames()) {
if (Regex.simpleMatch(indexAbstraction, authorizedIndex)
&& isIndexVisible(
indexAbstraction,
Expand Down Expand Up @@ -135,7 +153,7 @@ && isIndexVisible(
} else if (dateMathName.equals(indexAbstraction)) {
if (minus) {
finalIndices.remove(indexAbstraction);
} else if (indicesOptions.ignoreUnavailable() == false || availableIndexAbstractions.contains(indexAbstraction)) {
} else if (indicesOptions.ignoreUnavailable() == false || availableIndices.isAvailableName(indexAbstraction)) {
finalIndices.add(indexAbstraction);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ public void testResolveHiddenProperlyWithDateMath() {
List.of(requestedIndex),
IndicesOptions.LENIENT_EXPAND_OPEN,
metadata,
List.of("logs-pgsql-prod-" + todaySuffix, "logs-pgsql-prod-" + tomorrowSuffix),
Set.of("logs-pgsql-prod-" + todaySuffix, "logs-pgsql-prod-" + tomorrowSuffix),
randomBoolean(),
randomBoolean()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.Nullable;
import org.elasticsearch.transport.TransportRequest;
Expand All @@ -27,6 +28,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;

/**
* <p>
Expand Down Expand Up @@ -152,13 +154,20 @@ void authorizeIndexAction(
* alias or index
* @param listener the listener to be notified of the authorization result
*/
// TODO: remove?
void loadAuthorizedIndices(
RequestInfo requestInfo,
AuthorizationInfo authorizationInfo,
Map<String, IndexAbstraction> indicesLookup,
ActionListener<Set<String>> listener
);

Predicate<IndexAbstraction> predicateForAuthorizedIndices(
RequestInfo requestInfo,
AuthorizationInfo authorizationInfo,
Metadata metadata
);

/**
* Asynchronously checks that the permissions a user would have for a given list of names do
* not exceed their permissions for a given name. This is used to ensure that a user cannot
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,15 @@ private Predicate<IndexAbstraction> buildIndexMatcherPredicateForAction(String a
}
}
final StringMatcher nameMatcher = indexMatcher(ordinaryIndices, restrictedIndices);
final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices);
return indexAbstraction -> nameMatcher.test(indexAbstraction.getName())
|| (indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM
&& (indexAbstraction.getParentDataStream() == null)
&& bwcSpecialCaseMatcher.test(indexAbstraction.getName()));
if (grantMappingUpdatesOnIndices.isEmpty() && grantMappingUpdatesOnRestrictedIndices.isEmpty()) {
return indexAbstraction -> nameMatcher.test(indexAbstraction.getName());
} else {
final StringMatcher bwcSpecialCaseMatcher = indexMatcher(grantMappingUpdatesOnIndices, grantMappingUpdatesOnRestrictedIndices);
return indexAbstraction -> nameMatcher.test(indexAbstraction.getName())
|| (indexAbstraction.getType() != IndexAbstraction.Type.DATA_STREAM
&& (indexAbstraction.getParentDataStream() == null)
&& bwcSpecialCaseMatcher.test(indexAbstraction.getName()));
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import org.elasticsearch.action.support.GroupedActionListener;
import org.elasticsearch.action.support.replication.TransportReplicationAction.ConcreteShardRequest;
import org.elasticsearch.action.update.UpdateAction;
import org.elasticsearch.cluster.metadata.AvailableIndices;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.service.ClusterService;
Expand Down Expand Up @@ -405,38 +406,28 @@ private void authorizeAction(
}, clusterAuthzListener::onFailure));
} else if (isIndexAction(action)) {
final Metadata metadata = clusterService.state().metadata();
final AsyncSupplier<Set<String>> authorizedIndicesSupplier = new CachingAsyncSupplier<>(authzIndicesListener -> {
LoadAuthorizedIndiciesTimeChecker timeChecker = LoadAuthorizedIndiciesTimeChecker.start(requestInfo, authzInfo);
authzEngine.loadAuthorizedIndices(
requestInfo,
authzInfo,
metadata.getIndicesLookup(),
authzIndicesListener.map(authzIndices -> {
timeChecker.done(authzIndices);
return authzIndices;
})
);
});
final AsyncSupplier<ResolvedIndices> resolvedIndicesAsyncSupplier = new CachingAsyncSupplier<>(resolvedIndicesListener -> {
final ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);
ResolvedIndices resolvedIndices = indicesAndAliasesResolver.tryResolveWithoutWildcards(action, request);
if (resolvedIndices != null) {
resolvedIndicesListener.onResponse(resolvedIndices);
} else {
authorizedIndicesSupplier.getAsync(
ActionListener.wrap(
authorizedIndices -> resolvedIndicesListener.onResponse(
indicesAndAliasesResolver.resolve(action, request, metadata, authorizedIndices)
),
e -> {
auditTrail.accessDenied(requestId, authentication, action, request, authzInfo);
if (e instanceof IndexNotFoundException) {
listener.onFailure(e);
} else {
listener.onFailure(denialException(authentication, action, request, e));
}
}
)
);
try {
resolvedIndicesListener.onResponse(
indicesAndAliasesResolver.resolve(
action,
request,
metadata,
new AvailableIndices(metadata, authzEngine.predicateForAuthorizedIndices(requestInfo, authzInfo, metadata))
)
);
} catch (Exception e) {
auditTrail.accessDenied(requestId, authentication, action, request, authzInfo);
if (e instanceof IndexNotFoundException) {
listener.onFailure(e);
} else {
listener.onFailure(denialException(authentication, action, request, e));
}
}
}
});
authzEngine.authorizeIndexAction(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.cluster.metadata.AliasMetadata;
import org.elasticsearch.cluster.metadata.AvailableIndices;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.cluster.metadata.IndexAbstractionResolver;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
Expand Down Expand Up @@ -92,12 +93,17 @@ class IndicesAndAliasesResolver {
* Otherwise, <em>N</em> will be added to the <em>local</em> index list.
*/

// TODO: remove
ResolvedIndices resolve(String action, TransportRequest request, Metadata metadata, Set<String> authorizedIndices) {
return resolve(action, request, metadata, new AvailableIndices(authorizedIndices));
}

ResolvedIndices resolve(String action, TransportRequest request, Metadata metadata, AvailableIndices availableIndices) {
if (request instanceof IndicesAliasesRequest) {
ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
IndicesAliasesRequest indicesAliasesRequest = (IndicesAliasesRequest) request;
for (IndicesRequest indicesRequest : indicesAliasesRequest.getAliasActions()) {
final ResolvedIndices resolved = resolveIndicesAndAliases(action, indicesRequest, metadata, authorizedIndices);
final ResolvedIndices resolved = resolveIndicesAndAliases(action, indicesRequest, metadata, availableIndices);
resolvedIndicesBuilder.addLocal(resolved.getLocal());
resolvedIndicesBuilder.addRemote(resolved.getRemote());
}
Expand All @@ -108,7 +114,7 @@ ResolvedIndices resolve(String action, TransportRequest request, Metadata metada
if (request instanceof IndicesRequest == false) {
throw new IllegalStateException("Request [" + request + "] is not an Indices request, but should be.");
}
return resolveIndicesAndAliases(action, (IndicesRequest) request, metadata, authorizedIndices);
return resolveIndicesAndAliases(action, (IndicesRequest) request, metadata, availableIndices);
}

/**
Expand Down Expand Up @@ -183,11 +189,21 @@ ResolvedIndices resolveIndicesAndAliasesWithoutWildcards(String action, IndicesR
return new ResolvedIndices(localIndices, List.of());
}

// TODO: remove
ResolvedIndices resolveIndicesAndAliases(
String action,
IndicesRequest indicesRequest,
Metadata metadata,
Set<String> authorizedIndices
) {
return resolveIndicesAndAliases(action, indicesRequest, metadata, new AvailableIndices(authorizedIndices));
}

ResolvedIndices resolveIndicesAndAliases(
String action,
IndicesRequest indicesRequest,
Metadata metadata,
AvailableIndices availableIndices
) {
final ResolvedIndices.Builder resolvedIndicesBuilder = new ResolvedIndices.Builder();
boolean indicesReplacedWithNoIndices = false;
Expand All @@ -199,7 +215,7 @@ ResolvedIndices resolveIndicesAndAliases(
*/
assert indicesRequest.indices() == null || indicesRequest.indices().length == 0
: "indices are: " + Arrays.toString(indicesRequest.indices()); // Arrays.toString() can handle null values - all good
resolvedIndicesBuilder.addLocal(getPutMappingIndexOrAlias((PutMappingRequest) indicesRequest, authorizedIndices, metadata));
resolvedIndicesBuilder.addLocal(getPutMappingIndexOrAlias((PutMappingRequest) indicesRequest, availableIndices, metadata));
} else if (indicesRequest instanceof IndicesRequest.Replaceable) {
final IndicesRequest.Replaceable replaceable = (IndicesRequest.Replaceable) indicesRequest;
final IndicesOptions indicesOptions = indicesRequest.indicesOptions();
Expand All @@ -208,7 +224,7 @@ ResolvedIndices resolveIndicesAndAliases(
// check for all and return list of authorized indices
if (IndexNameExpressionResolver.isAllIndices(indicesList(indicesRequest.indices()))) {
if (replaceWildcards) {
for (String authorizedIndex : authorizedIndices) {
for (String authorizedIndex : availableIndices.getAvailableNames()) {
if (IndexAbstractionResolver.isIndexVisible(
"*",
authorizedIndex,
Expand All @@ -234,7 +250,7 @@ ResolvedIndices resolveIndicesAndAliases(
split.getLocal(),
indicesOptions,
metadata,
authorizedIndices,
availableIndices,
replaceWildcards,
indicesRequest.includeDataStreams()
);
Expand Down Expand Up @@ -272,7 +288,7 @@ ResolvedIndices resolveIndicesAndAliases(
if (aliasesRequest.expandAliasesWildcards()) {
List<String> aliases = replaceWildcardsWithAuthorizedAliases(
aliasesRequest.aliases(),
loadAuthorizedAliases(authorizedIndices, metadata)
loadAuthorizedAliases(availableIndices, metadata)
);
aliasesRequest.replaceAliases(aliases.toArray(new String[aliases.size()]));
}
Expand Down Expand Up @@ -314,7 +330,12 @@ ResolvedIndices resolveIndicesAndAliases(
* request's concrete index is not in the list of authorized indices, then we need to look to
* see if this can be authorized against an alias
*/
static String getPutMappingIndexOrAlias(PutMappingRequest request, Set<String> authorizedIndicesList, Metadata metadata) {
// TODO: remove
static String getPutMappingIndexOrAlias(PutMappingRequest request, Set<String> authorizedIndices, Metadata metadata) {
return getPutMappingIndexOrAlias(request, new AvailableIndices(authorizedIndices), metadata);
}

static String getPutMappingIndexOrAlias(PutMappingRequest request, AvailableIndices availableIndices, Metadata metadata) {
final String concreteIndexName = request.getConcreteIndex().getName();

// validate that the concrete index exists, otherwise there is no remapping that we could do
Expand All @@ -330,7 +351,7 @@ static String getPutMappingIndexOrAlias(PutMappingRequest request, Set<String> a
+ indexAbstraction.getType().getDisplayName()
+ "], but a concrete index is expected"
);
} else if (authorizedIndicesList.contains(concreteIndexName)) {
} else if (availableIndices.isAvailableName(concreteIndexName)) {
// user is authorized to put mappings for this index
resolvedAliasOrIndex = concreteIndexName;
} else {
Expand All @@ -341,7 +362,7 @@ static String getPutMappingIndexOrAlias(PutMappingRequest request, Set<String> a
if (aliasMetadata != null) {
Optional<String> foundAlias = aliasMetadata.stream()
.map(AliasMetadata::alias)
.filter(authorizedIndicesList::contains)
.filter(availableIndices::isAvailableName)
.filter(aliasName -> {
IndexAbstraction alias = metadata.getIndicesLookup().get(aliasName);
List<Index> indices = alias.getIndices();
Expand All @@ -363,10 +384,10 @@ static String getPutMappingIndexOrAlias(PutMappingRequest request, Set<String> a
return resolvedAliasOrIndex;
}

private List<String> loadAuthorizedAliases(Set<String> authorizedIndices, Metadata metadata) {
private List<String> loadAuthorizedAliases(AvailableIndices availableIndices, Metadata metadata) {
List<String> authorizedAliases = new ArrayList<>();
SortedMap<String, IndexAbstraction> existingAliases = metadata.getIndicesLookup();
for (String authorizedIndex : authorizedIndices) {
for (String authorizedIndex : availableIndices.getAvailableNames()) {
IndexAbstraction indexAbstraction = existingAliases.get(authorizedIndex);
if (indexAbstraction != null && indexAbstraction.getType() == IndexAbstraction.Type.ALIAS) {
authorizedAliases.add(authorizedIndex);
Expand Down
Loading