Skip to content

Commit a7c1ea4

Browse files
nibixnibix
committed
fixes
Signed-off-by: Nils Bandener <nils.bandener@eliatra.com>
1 parent 55ad845 commit a7c1ea4

File tree

3 files changed

+130
-15
lines changed

3 files changed

+130
-15
lines changed

‎src/integrationTest/java/org/opensearch/security/privileges/IndexRequestModifierTest.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*
8+
* Modifications Copyright OpenSearch Contributors. See
9+
* GitHub history for details.
10+
*/
11+
12+
package org.opensearch.security.privileges;
13+
14+
import org.junit.Test;
15+
import org.junit.runner.RunWith;
16+
import org.junit.runners.Parameterized;
17+
import org.junit.runners.Suite;
18+
import org.opensearch.action.ActionRequest;
19+
import org.opensearch.action.IndicesRequest;
20+
import org.opensearch.action.OriginalIndices;
21+
import org.opensearch.action.index.IndexRequest;
22+
import org.opensearch.action.search.SearchRequest;
23+
import org.opensearch.action.support.IndicesOptions;
24+
import org.opensearch.cluster.ClusterState;
25+
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
26+
import org.opensearch.cluster.metadata.Metadata;
27+
import org.opensearch.cluster.metadata.ResolvedIndices;
28+
import org.opensearch.common.settings.Settings;
29+
import org.opensearch.common.util.concurrent.ThreadContext;
30+
import org.opensearch.security.util.MockIndexMetadataBuilder;
31+
32+
import java.util.Arrays;
33+
import java.util.Collection;
34+
import java.util.Map;
35+
36+
import static org.junit.Assert.assertEquals;
37+
import static org.junit.Assert.assertFalse;
38+
import static org.junit.Assert.assertTrue;
39+
40+
@RunWith(Suite.class)
41+
@Suite.SuiteClasses({
42+
IndexRequestModifierTest.SetLocalIndicesToEmpty.class })
43+
public class IndexRequestModifierTest {
44+
45+
static final IndexNameExpressionResolver indexNameExpressionResolver = new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY));
46+
static final Metadata metadata = MockIndexMetadataBuilder.indices("index").build();
47+
final static ClusterState clusterState = ClusterState.builder(ClusterState.EMPTY_STATE).metadata(metadata).build();
48+
49+
@RunWith(Parameterized.class)
50+
public static class SetLocalIndicesToEmpty {
51+
52+
53+
String description;
54+
IndicesRequest request;
55+
56+
@Test
57+
public void setLocalIndicesToEmpty() {
58+
59+
ResolvedIndices resolvedIndices = ResolvedIndices.of("index");
60+
61+
if (Arrays.asList(request.indices()).contains("remote:index")) {
62+
resolvedIndices = resolvedIndices.withRemoteIndices(Map.of("remote", new OriginalIndices(new String [] {"index"}, request.indicesOptions())));
63+
}
64+
65+
boolean success = new IndicesRequestModifier().setLocalIndicesToEmpty((ActionRequest) request, resolvedIndices);
66+
67+
if (!(request instanceof IndicesRequest.Replaceable)) {
68+
assertFalse(success);
69+
} else
70+
if (!request.indicesOptions().allowNoIndices()) {
71+
assertFalse(success);
72+
} else {
73+
assertTrue(success);
74+
75+
String [] finalResolvedIndices = indexNameExpressionResolver.concreteIndexNames(clusterState, request);
76+
77+
assertEquals("Resolved to empty indices: " + Arrays.asList(finalResolvedIndices), 0, finalResolvedIndices.length);
78+
}
79+
}
80+
81+
82+
@Parameterized.Parameters(name = "{0}")
83+
public static Collection<Object[]> params() {
84+
return Arrays.asList(
85+
new Object [] {
86+
"lenient expand open", new SearchRequest("index").indicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN)
87+
},
88+
new Object [] {
89+
"lenient expand open/closed", new SearchRequest("index").indicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED)
90+
},
91+
new Object [] {
92+
"lenient expand open/closed/hidden", new SearchRequest("index").indicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN_CLOSED_HIDDEN)
93+
},
94+
new Object [] {
95+
"allow no indices", new SearchRequest("index").indicesOptions(IndicesOptions.fromOptions(false, true, false, false))
96+
},
97+
new Object [] {
98+
"ignore unavailable", new SearchRequest("index").indicesOptions(IndicesOptions.fromOptions(true, false, false, false))
99+
},
100+
new Object [] {
101+
"strict single index", new SearchRequest("index").indicesOptions(IndicesOptions.STRICT_SINGLE_INDEX_NO_EXPAND_FORBID_CLOSED)
102+
},
103+
new Object [] {
104+
"with remote index", new SearchRequest("index", "remote:index").indicesOptions(IndicesOptions.LENIENT_EXPAND_OPEN)
105+
},
106+
new Object [] {
107+
"not implementing IndicesRequest.Replaceable", new IndexRequest("index")
108+
}
109+
);
110+
111+
}
112+
113+
public SetLocalIndicesToEmpty(String description, IndicesRequest request) {
114+
this.description = description;
115+
this.request = request;
116+
}
117+
}
118+
}

‎src/main/java/org/opensearch/security/privileges/IndicesRequestModifier.java‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@
1818
import org.opensearch.action.IndicesRequest;
1919
import org.opensearch.cluster.metadata.ResolvedIndices;
2020

21+
/**
22+
* Provides methods to modify the local indices of an IndicesRequest. All methods use the ResolvedIndices metadata object
23+
* to make sure that remote indices are properly retained.
24+
* <p>
25+
* We need the distinction between local indices and remote indices because authorization on remote indices is performed
26+
* on the remote cluster - thus, we can leave them here just as they are.
27+
*/
2128
public class IndicesRequestModifier {
2229

2330
public boolean reduceLocalIndices(ActionRequest targetRequest, ResolvedIndices resolvedIndices, Collection<String> newIndices) {
@@ -37,23 +44,13 @@ public boolean setLocalIndicesToEmpty(ActionRequest targetRequest, ResolvedIndic
3744
if (targetRequest instanceof IndicesRequest.Replaceable replaceable) {
3845
if (resolvedIndices.remote().isEmpty()) {
3946
if (replaceable.indicesOptions().expandWildcardsOpen()
40-
|| replaceable.indicesOptions().expandWildcardsClosed()
41-
|| replaceable.indicesOptions().expandWildcardsHidden()) {
47+
|| replaceable.indicesOptions().expandWildcardsClosed()) {
4248
// If the request expands wildcards, we use an index expression which resolves to no indices
43-
// This expression cannot resolve to anything because indices with a leading underscore are not allowed
44-
replaceable.indices("_empty*,-*");
49+
replaceable.indices(".none*,-*");
4550
return true;
4651
} else if (replaceable.indicesOptions().allowNoIndices()) {
47-
// If the request does not expand wildcards, we have to look for two different conditions due to
48-
// a slightly odd behavior of IndexNameExpressionResolver:
49-
// https://github.com/opensearch-project/OpenSearch/blob/afb08a071269b234936b778f62800bded0e5ea7a/server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java#L249
50-
// For allowNoIndices(), we just select a non-existing index. Again, index names with leading
51-
// underscores never exist.
52-
replaceable.indices("_empty");
53-
return true;
54-
} else if (replaceable.indicesOptions().ignoreUnavailable()) {
55-
// Second case for the special behavior of IndexNameExpressionResolver:
56-
replaceable.indices("_empty", "-_empty*");
52+
// If the request does not expand wildcards, we use a index name that cannot exist.
53+
replaceable.indices("-.none*");
5754
return true;
5855
} else {
5956
// In this case, we cannot perform replacement. But it also won't be necessary due to the

‎src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ private void evaluateSystemIndicesAccess(
269269
presponse.markComplete();
270270
return;
271271
} else {
272-
Set<String> matchingSystemIndices = SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.getAllIndices());
272+
Set<String> matchingSystemIndices = SystemIndexRegistry.matchesSystemIndexPattern(requestedResolved.local().names());
273273
matchingSystemIndices.removeAll(matchingPluginIndices);
274274
// See if request matches other system indices not belong to the plugin
275275
if (!matchingSystemIndices.isEmpty()) {

0 commit comments

Comments
 (0)