Skip to content

Commit 472f7c7

Browse files
cwperkscwperks
authored
Ensure that plugin can search on system index when utilizing pluginSubject.runAs (#5032)
Signed-off-by: Craig Perkins <[email protected]>
1 parent a1ccbd5 commit 472f7c7

File tree

9 files changed

+303
-6
lines changed

9 files changed

+303
-6
lines changed

‎src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexDisabledTests.java‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* The OpenSearch Contributors require contributions made to
6+
* this file be licensed under the Apache-2.0 license or a
7+
* compatible open source license.
8+
*
9+
*/
10+
package org.opensearch.security.systemindex;
11+
12+
import java.util.List;
13+
import java.util.Map;
14+
15+
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
16+
import org.junit.Before;
17+
import org.junit.ClassRule;
18+
import org.junit.Test;
19+
import org.junit.runner.RunWith;
20+
21+
import org.opensearch.core.rest.RestStatus;
22+
import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1;
23+
import org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin2;
24+
import org.opensearch.test.framework.TestSecurityConfig.AuthcDomain;
25+
import org.opensearch.test.framework.cluster.ClusterManager;
26+
import org.opensearch.test.framework.cluster.LocalCluster;
27+
import org.opensearch.test.framework.cluster.TestRestClient;
28+
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
29+
30+
import static org.hamcrest.MatcherAssert.assertThat;
31+
import static org.hamcrest.Matchers.containsString;
32+
import static org.hamcrest.Matchers.not;
33+
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
34+
import static org.opensearch.security.support.ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_KEY;
35+
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
36+
import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN;
37+
38+
@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
39+
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
40+
public class SystemIndexDisabledTests {
41+
42+
public static final AuthcDomain AUTHC_DOMAIN = new AuthcDomain("basic", 0).httpAuthenticatorWithChallenge("basic").backend("internal");
43+
44+
@ClassRule
45+
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
46+
.anonymousAuth(false)
47+
.authc(AUTHC_DOMAIN)
48+
.users(USER_ADMIN)
49+
.plugin(SystemIndexPlugin1.class, SystemIndexPlugin2.class)
50+
.nodeSettings(
51+
Map.of(
52+
SECURITY_RESTAPI_ROLES_ENABLED,
53+
List.of("user_" + USER_ADMIN.getName() + "__" + ALL_ACCESS.getName()),
54+
SECURITY_SYSTEM_INDICES_ENABLED_KEY,
55+
false
56+
)
57+
)
58+
.build();
59+
60+
@Before
61+
public void setup() {
62+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
63+
client.delete(".system-index1");
64+
}
65+
}
66+
67+
@Test
68+
public void testPluginShouldBeAbleToIndexIntoAnySystemIndexWhenProtectionIsDisabled() {
69+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
70+
client.put(".system-index1");
71+
client.put(".system-index2");
72+
}
73+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
74+
HttpResponse response = client.put("try-create-and-bulk-mixed-index");
75+
76+
response.assertStatusCode(RestStatus.OK.getStatus());
77+
78+
assertThat(
79+
response.getBody(),
80+
not(
81+
containsString(
82+
"no permissions for [] and User [name=plugin:org.opensearch.security.systemindex.sampleplugin.SystemIndexPlugin1"
83+
)
84+
)
85+
);
86+
}
87+
}
88+
}

‎src/integrationTest/java/org/opensearch/security/systemindex/SystemIndexTests.java‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import java.util.Map;
1414

1515
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
16+
import com.fasterxml.jackson.databind.JsonNode;
1617
import org.junit.Before;
1718
import org.junit.ClassRule;
1819
import org.junit.Test;
@@ -170,6 +171,73 @@ public void testPluginShouldBeAbleToBulkIndexDocumentIntoItsSystemIndex() {
170171
}
171172
}
172173

174+
@Test
175+
public void testPluginShouldBeAbleSearchOnItsSystemIndex() {
176+
JsonNode searchResponse1;
177+
JsonNode searchResponse2;
178+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
179+
HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
180+
181+
assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
182+
183+
HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1);
184+
185+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
186+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
187+
188+
searchResponse1 = searchResponse.bodyAsJsonNode();
189+
}
190+
191+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
192+
HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search");
193+
194+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
195+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
196+
197+
searchResponse2 = searchResponse.bodyAsJsonNode();
198+
}
199+
200+
JsonNode hits1 = searchResponse1.get("hits");
201+
JsonNode hits2 = searchResponse2.get("hits");
202+
assertThat(hits1.toPrettyString(), equalTo(hits2.toPrettyString()));
203+
}
204+
205+
@Test
206+
public void testPluginShouldBeAbleGetOnItsSystemIndex() {
207+
JsonNode getResponse1;
208+
JsonNode getResponse2;
209+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
210+
HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
211+
212+
assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
213+
214+
HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1);
215+
216+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
217+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
218+
219+
String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id");
220+
221+
HttpResponse getResponse = client.get("get-on-system-index/" + SYSTEM_INDEX_1 + "/" + docId);
222+
223+
getResponse1 = getResponse.bodyAsJsonNode();
224+
}
225+
226+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
227+
HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search");
228+
229+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
230+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
231+
232+
String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id");
233+
234+
HttpResponse getResponse = client.get(SYSTEM_INDEX_1 + "/_doc/" + docId);
235+
236+
getResponse2 = getResponse.bodyAsJsonNode();
237+
}
238+
assertThat(getResponse1.toPrettyString(), equalTo(getResponse2.toPrettyString()));
239+
}
240+
173241
@Test
174242
public void testPluginShouldNotBeAbleToBulkIndexDocumentIntoMixOfSystemIndexWhereAtLeastOneDoesNotBelongToPlugin() {
175243
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {

‎src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestGetOnSystemIndexAction.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* The OpenSearch Contributors require contributions made to
6+
* this file be licensed under the Apache-2.0 license or a
7+
* compatible open source license.
8+
*
9+
*/
10+
11+
package org.opensearch.security.systemindex.sampleplugin;
12+
13+
import java.util.List;
14+
15+
import org.opensearch.action.get.GetRequest;
16+
import org.opensearch.client.node.NodeClient;
17+
import org.opensearch.core.action.ActionListener;
18+
import org.opensearch.core.rest.RestStatus;
19+
import org.opensearch.core.xcontent.ToXContent;
20+
import org.opensearch.rest.BaseRestHandler;
21+
import org.opensearch.rest.BytesRestResponse;
22+
import org.opensearch.rest.RestChannel;
23+
import org.opensearch.rest.RestRequest;
24+
25+
import static java.util.Collections.singletonList;
26+
import static org.opensearch.rest.RestRequest.Method.GET;
27+
28+
public class RestGetOnSystemIndexAction extends BaseRestHandler {
29+
30+
private final RunAsSubjectClient pluginClient;
31+
32+
public RestGetOnSystemIndexAction(RunAsSubjectClient pluginClient) {
33+
this.pluginClient = pluginClient;
34+
}
35+
36+
@Override
37+
public List<Route> routes() {
38+
return singletonList(new Route(GET, "/get-on-system-index/{index}/{docId}"));
39+
}
40+
41+
@Override
42+
public String getName() {
43+
return "test_get_on_system_index_action";
44+
}
45+
46+
@Override
47+
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
48+
String indexName = request.param("index");
49+
String docId = request.param("docId");
50+
return new RestChannelConsumer() {
51+
52+
@Override
53+
public void accept(RestChannel channel) throws Exception {
54+
GetRequest getRequest = new GetRequest(indexName);
55+
getRequest.id(docId);
56+
pluginClient.get(getRequest, ActionListener.wrap(r -> {
57+
channel.sendResponse(new BytesRestResponse(RestStatus.OK, r.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)));
58+
}, fr -> { channel.sendResponse(new BytesRestResponse(RestStatus.FORBIDDEN, String.valueOf(fr))); }));
59+
}
60+
};
61+
}
62+
}

‎src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/RestSearchOnSystemIndexAction.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*
5+
* The OpenSearch Contributors require contributions made to
6+
* this file be licensed under the Apache-2.0 license or a
7+
* compatible open source license.
8+
*
9+
*/
10+
11+
package org.opensearch.security.systemindex.sampleplugin;
12+
13+
import java.util.List;
14+
15+
import org.opensearch.action.search.SearchRequest;
16+
import org.opensearch.client.node.NodeClient;
17+
import org.opensearch.core.action.ActionListener;
18+
import org.opensearch.core.rest.RestStatus;
19+
import org.opensearch.core.xcontent.ToXContent;
20+
import org.opensearch.index.query.QueryBuilders;
21+
import org.opensearch.rest.BaseRestHandler;
22+
import org.opensearch.rest.BytesRestResponse;
23+
import org.opensearch.rest.RestChannel;
24+
import org.opensearch.rest.RestRequest;
25+
import org.opensearch.search.builder.SearchSourceBuilder;
26+
27+
import static java.util.Collections.singletonList;
28+
import static org.opensearch.rest.RestRequest.Method.GET;
29+
30+
public class RestSearchOnSystemIndexAction extends BaseRestHandler {
31+
32+
private final RunAsSubjectClient pluginClient;
33+
34+
public RestSearchOnSystemIndexAction(RunAsSubjectClient pluginClient) {
35+
this.pluginClient = pluginClient;
36+
}
37+
38+
@Override
39+
public List<Route> routes() {
40+
return singletonList(new Route(GET, "/search-on-system-index/{index}"));
41+
}
42+
43+
@Override
44+
public String getName() {
45+
return "test_search_on_system_index_action";
46+
}
47+
48+
@Override
49+
public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) {
50+
String indexName = request.param("index");
51+
return new RestChannelConsumer() {
52+
53+
@Override
54+
public void accept(RestChannel channel) throws Exception {
55+
SearchRequest searchRequest = new SearchRequest(indexName);
56+
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder();
57+
sourceBuilder.query(QueryBuilders.matchAllQuery());
58+
searchRequest.source(sourceBuilder);
59+
pluginClient.search(searchRequest, ActionListener.wrap(r -> {
60+
channel.sendResponse(new BytesRestResponse(RestStatus.OK, r.toXContent(channel.newBuilder(), ToXContent.EMPTY_PARAMS)));
61+
}, fr -> { channel.sendResponse(new BytesRestResponse(RestStatus.FORBIDDEN, String.valueOf(fr))); }));
62+
}
63+
};
64+
}
65+
}

‎src/integrationTest/java/org/opensearch/security/systemindex/sampleplugin/SystemIndexPlugin1.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ public List<RestHandler> getRestHandlers(
8888
new RestIndexDocumentIntoSystemIndexAction(client),
8989
new RestRunClusterHealthAction(client),
9090
new RestBulkIndexDocumentIntoSystemIndexAction(client, pluginClient),
91-
new RestBulkIndexDocumentIntoMixOfSystemIndexAction(client, pluginClient)
91+
new RestBulkIndexDocumentIntoMixOfSystemIndexAction(client, pluginClient),
92+
new RestSearchOnSystemIndexAction(pluginClient),
93+
new RestGetOnSystemIndexAction(pluginClient)
9294
);
9395
}
9496

‎src/main/java/org/opensearch/security/configuration/SystemIndexSearcherWrapper.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ protected final boolean isBlockedSystemIndexRequest() {
163163

164164
if (systemIndexPermissionEnabled) {
165165
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
166-
if (user == null) {
166+
if (HeaderHelper.isInternalOrPluginRequest(threadContext)) {
167167
// allow request without user from plugin.
168168
return systemIndexMatcher.test(index.getName()) || matchesSystemIndexRegisteredWithCore;
169169
}
@@ -180,8 +180,7 @@ protected final boolean isBlockedSystemIndexRequest() {
180180

181181
protected final boolean isAdminDnOrPluginRequest() {
182182
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
183-
if (user == null) {
184-
// allow request without user from plugin.
183+
if (HeaderHelper.isInternalOrPluginRequest(threadContext)) {
185184
return true;
186185
} else if (adminDns.isAdmin(user)) {
187186
return true;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ private void evaluateSystemIndicesAccess(
304304

305305
// cluster actions will return true for requestedResolved.isLocalAll()
306306
// the following section should only be run for index actions
307-
if (user.isPluginUser() && !requestedResolved.isLocalAll()) {
307+
if (this.isSystemIndexEnabled && user.isPluginUser() && !requestedResolved.isLocalAll()) {
308308
Set<String> matchingSystemIndices = SystemIndexRegistry.matchesPluginSystemIndexPattern(
309309
user.getName().replace("plugin:", ""),
310310
requestedResolved.getAllIndices()

‎src/main/java/org/opensearch/security/privileges/dlsfls/DlsFlsBaseContext.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public DlsFlsBaseContext(PrivilegesEvaluator privilegesEvaluator, ThreadContext
3939
public PrivilegesEvaluationContext getPrivilegesEvaluationContext() {
4040
User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
4141

42-
if (user == null || adminDNs.isAdmin(user)) {
42+
if (HeaderHelper.isInternalOrPluginRequest(threadContext) || adminDNs.isAdmin(user)) {
4343
return null;
4444
}
4545

‎src/main/java/org/opensearch/security/support/HeaderHelper.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import com.google.common.base.Strings;
3434

3535
import org.opensearch.common.util.concurrent.ThreadContext;
36+
import org.opensearch.security.user.User;
3637

3738
public class HeaderHelper {
3839

@@ -52,6 +53,18 @@ public static boolean isExtensionRequest(final ThreadContext context) {
5253
}
5354
// CS-ENFORCE-SINGLE
5455

56+
public static boolean isInternalOrPluginRequest(final ThreadContext threadContext) {
57+
// If user is empty, this indicates a system-level request which should be permitted
58+
// If the requests originates from a plugin this will also return true
59+
final User user = (User) threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
60+
61+
if (user == null || user.isPluginUser()) {
62+
return true;
63+
}
64+
65+
return false;
66+
}
67+
5568
public static String getSafeFromHeader(final ThreadContext context, final String headerName) {
5669

5770
if (context == null || headerName == null || headerName.isEmpty()) {

0 commit comments

Comments
 (0)