Skip to content

Only expand authorized indices for requests with wildcards #72118

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.

Sign up for GitHub

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

Closed
wants to merge 17 commits into from
Closed

Only expand authorized indices for requests with wildcards #72118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
42922ed
Avoid passing authorizedIndices together with resolvedIndices
albertzaharovits Apr 22, 2021
7daff92
Merge branch 'master' into avoid_authz_expand_for_bulks
albertzaharovits Apr 22, 2021
dc59ac1
Continued
albertzaharovits Apr 22, 2021
538da36
Noop
albertzaharovits Apr 22, 2021
080071b
nitty nit
albertzaharovits Apr 23, 2021
0dd0e34
Fancy async supplier
albertzaharovits Apr 24, 2021
c2e7587
Move CachingAsyncSupplier
albertzaharovits Apr 24, 2021
fc8415e
Merge branch 'master' into avoid_authz_expand_for_bulks
albertzaharovits Apr 24, 2021
ae35032
Refactor resolveIndicesAndAliases to make it amenable for async
albertzaharovits Apr 24, 2021
d2b6d40
Noop getPutMappingIndexOrAlias refactor for async
albertzaharovits Apr 24, 2021
53069f8
Done
albertzaharovits Apr 24, 2021
019583b
checkstyle
albertzaharovits Apr 24, 2021
726c957
Polish
albertzaharovits Apr 24, 2021
c1f0edb
Remote still to do
albertzaharovits Apr 24, 2021
9c3bc7d
the trappy fixes
albertzaharovits Apr 25, 2021
c2466b8
Async indexAbstractionResolver.resolveIndexAbstractions
albertzaharovits Apr 25, 2021
0435d04
checkstyle
albertzaharovits Apr 25, 2021
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
1 change: 1 addition & 0 deletions ...thorization-engine/src/main/java/org/elasticsearch/example/CustomAuthorizationEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.common.util.AsyncSupplier;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRequest;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse.Indices;
Expand Down
170 changes: 115 additions & 55 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexAbstractionResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,26 @@

package org.elasticsearch.cluster.metadata;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.ActionRunnable;
import org.elasticsearch.action.StepListener;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.util.AsyncSupplier;
import org.elasticsearch.common.util.concurrent.FutureUtils;
import org.elasticsearch.index.IndexNotFoundException;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;

public class IndexAbstractionResolver {

Expand All @@ -43,69 +53,119 @@ public List<String> resolveIndexAbstractions(Iterable<String> indices, IndicesOp
public List<String> resolveIndexAbstractions(Iterable<String> indices, IndicesOptions indicesOptions, Metadata metadata,
Collection<String> availableIndexAbstractions, boolean replaceWildcards,
boolean includeDataStreams) {
List<String> finalIndices = new ArrayList<>();
boolean wildcardSeen = false;
for (String index : indices) {
String indexAbstraction;
boolean minus = false;
if (index.charAt(0) == '-' && wildcardSeen) {
indexAbstraction = index.substring(1);
minus = true;
} else {
indexAbstraction = index;
}
PlainActionFuture<List<String>> future = PlainActionFuture.newFuture();
// if the supplier is not async, the method is not async, hence get is non-blocking
resolveIndexAbstractions(indices, indicesOptions, metadata, listener -> listener.onResponse(availableIndexAbstractions),
replaceWildcards,
includeDataStreams,
future);
return FutureUtils.get(future, 0, TimeUnit.MILLISECONDS);
}

// we always need to check for date math expressions
final String dateMathName = indexNameExpressionResolver.resolveDateMathExpression(indexAbstraction);
if (dateMathName != indexAbstraction) {
assert dateMathName.equals(indexAbstraction) == false;
if (replaceWildcards && Regex.isSimpleMatchPattern(dateMathName)) {
// continue
indexAbstraction = dateMathName;
} else if (availableIndexAbstractions.contains(dateMathName) &&
isIndexVisible(indexAbstraction, dateMathName, indicesOptions, metadata, includeDataStreams, true)) {
if (minus) {
finalIndices.remove(dateMathName);
} else {
finalIndices.add(dateMathName);
}
public void resolveIndexAbstractions(Iterable<String> indices, IndicesOptions indicesOptions, Metadata metadata,
AsyncSupplier<Collection<String>> availableIndexAbstractionsSupplier, boolean replaceWildcards,
boolean includeDataStreams, ActionListener<List<String>> listener) {
final Iterator<String> indicesIterator = indices.iterator();
final Runnable evalItems = new ActionRunnable(listener) {

final List<String> finalIndices = new ArrayList<>();
final AtomicBoolean wildcardSeen = new AtomicBoolean(false);

@Override
public void doRun() {
if (false == indicesIterator.hasNext()) {
listener.onResponse(finalIndices);
} else {
if (indicesOptions.ignoreUnavailable() == false) {
throw new IndexNotFoundException(dateMathName);
final String index = indicesIterator.next();
final AtomicReference<String> indexAbstraction;
final boolean minus;
if (index.charAt(0) == '-' && wildcardSeen.get()) {
indexAbstraction = new AtomicReference<>(index.substring(1));
minus = true;
} else {
indexAbstraction = new AtomicReference<>(index);
minus = false;
}
}
}

if (replaceWildcards && Regex.isSimpleMatchPattern(indexAbstraction)) {
wildcardSeen = true;
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : availableIndexAbstractions) {
if (Regex.simpleMatch(indexAbstraction, authorizedIndex) &&
isIndexVisible(indexAbstraction, authorizedIndex, indicesOptions, metadata, includeDataStreams)) {
resolvedIndices.add(authorizedIndex);
}
}
if (resolvedIndices.isEmpty()) {
//es core honours allow_no_indices for each wildcard expression, we do the same here by throwing index not found.
if (indicesOptions.allowNoIndices() == false) {
throw new IndexNotFoundException(indexAbstraction);
}
} else {
if (minus) {
finalIndices.removeAll(resolvedIndices);
// we always need to check for date math expressions
final String dateMathName = indexNameExpressionResolver.resolveDateMathExpression(indexAbstraction.get());
final StepListener<Void> evaluateDateMathStep = new StepListener<>();
if (dateMathName != indexAbstraction.get()) {
assert dateMathName.equals(indexAbstraction.get()) == false;
if (replaceWildcards && Regex.isSimpleMatchPattern(dateMathName)) {
// this is a date math and a wildcard
indexAbstraction.set(dateMathName);
// continue
evaluateDateMathStep.onResponse(null);
} else {
availableIndexAbstractionsSupplier.getAsync(ActionListener.wrap(availableIndexAbstractions -> {
if (availableIndexAbstractions.contains(dateMathName) &&
isIndexVisible(indexAbstraction.get(), dateMathName, indicesOptions, metadata, includeDataStreams
, true)) {
if (minus) {
finalIndices.remove(dateMathName);
} else {
finalIndices.add(dateMathName);
}
} else {
if (indicesOptions.ignoreUnavailable() == false) {
listener.onFailure(new IndexNotFoundException(dateMathName));
return;
}
}
evaluateDateMathStep.onResponse(null);
}, listener::onFailure));
}
} else {
finalIndices.addAll(resolvedIndices);
evaluateDateMathStep.onResponse(null);
}
}
} else if (dateMathName.equals(indexAbstraction)) {
if (minus) {
finalIndices.remove(indexAbstraction);
} else {
finalIndices.add(indexAbstraction);

evaluateDateMathStep.whenComplete(anotherVoid -> {
if (replaceWildcards && Regex.isSimpleMatchPattern(indexAbstraction.get())) {
wildcardSeen.set(true);
availableIndexAbstractionsSupplier.getAsync(ActionListener.wrap(availableIndexAbstractions -> {
Set<String> resolvedIndices = new HashSet<>();
for (String authorizedIndex : availableIndexAbstractions) {
if (Regex.simpleMatch(indexAbstraction.get(), authorizedIndex) &&
isIndexVisible(indexAbstraction.get(), authorizedIndex, indicesOptions, metadata,
includeDataStreams)) {
resolvedIndices.add(authorizedIndex);
}
}
if (resolvedIndices.isEmpty()) {
//es core honours allow_no_indices for each wildcard expression,
// we do the same here by throwing index not found.
if (indicesOptions.allowNoIndices() == false) {
listener.onFailure(new IndexNotFoundException(indexAbstraction.get()));
return;
}
} else {
if (minus) {
finalIndices.removeAll(resolvedIndices);
} else {
finalIndices.addAll(resolvedIndices);
}
}
// next expression item
this.doRun();
}, listener::onFailure));
} else if (dateMathName.equals(indexAbstraction.get())) {
if (minus) {
finalIndices.remove(indexAbstraction.get());
} else {
finalIndices.add(indexAbstraction.get());
}
// next expression item
this.doRun();
} else {
// next expression item
this.doRun();
}
}, listener::onFailure);
}
}
}
return finalIndices;
};
evalItems.run();
}

public static boolean isIndexVisible(String expression, String index, IndicesOptions indicesOptions, Metadata metadata,
Expand Down
46 changes: 46 additions & 0 deletions server/src/main/java/org/elasticsearch/common/util/AsyncSupplier.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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.common.util;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.common.util.concurrent.ListenableFuture;

import java.util.concurrent.ConcurrentHashMap;

@FunctionalInterface
public interface AsyncSupplier<V> {

/**
* Asynchronously retrieves the value that is being supplied and notifies the listener upon
* completion.
*/
void getAsync(ActionListener<V> listener);

class CachingAsyncSupplier<V> implements AsyncSupplier<V> {

private final AsyncSupplier<V> asyncSupplierDelegate;
private final ConcurrentHashMap<AsyncSupplier<V>, ListenableFuture<V>> map;

public CachingAsyncSupplier(AsyncSupplier<V> supplierDelegate) {
this.asyncSupplierDelegate = supplierDelegate;
this.map = new ConcurrentHashMap<>(1);
}

@Override
public void getAsync(ActionListener<V> listener) {
map.computeIfAbsent(asyncSupplierDelegate, ignore -> {
ListenableFuture<V> cachedListener = new ListenableFuture();
// trigger async computation
asyncSupplierDelegate.getAsync(cachedListener);
return cachedListener;
}).addListener(listener, EsExecutors.newDirectExecutorService());
}
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a nested class inside the AuthorizationEngine, but because it is now passed around to more classes, I had to pull it up somewhere.
I also improved the threading of it, as it was harder to reason about the corner cases than to fix it.

10 changes: 1 addition & 9 deletions ...n/core/src/main/java/org/elasticsearch/xpack/core/security/authz/AuthorizationEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.cluster.metadata.IndexAbstraction;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.util.AsyncSupplier;
import org.elasticsearch.transport.TransportRequest;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesRequest;
import org.elasticsearch.xpack.core.security.action.user.GetUserPrivilegesResponse;
Expand Down Expand Up @@ -353,13 +354,4 @@ public IndicesAccessControl getIndicesAccessControl() {
}
}

@FunctionalInterface
interface AsyncSupplier<V> {

/**
* Asynchronously retrieves the value that is being supplied and notifies the listener upon
* completion.
*/
void getAsync(ActionListener<V> listener);
}
}
5 changes: 5 additions & 0 deletions ...lugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/ResolvedIndices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ public void addRemote(List<String> indices) {
remote.addAll(indices);
}

public void add(ResolvedIndices resolvedIndices) {
addLocal(resolvedIndices.getLocal());
addRemote(resolvedIndices.getRemote());
}

/** @return <code>true</code> if both the local and remote index lists are empty. */
public boolean isEmpty() {
return local.isEmpty() && remote.isEmpty();
Expand Down
Loading