Skip to content

Commit aa0eff4

Browse files
cwperkscwperks
authored
[Backport 2.x] Ensure that plugin can search on system index when utilizing pluginSubject.runAs (#5032) (#5054)
1 parent 254f697 commit aa0eff4

File tree

7 files changed

+223
-4
lines changed

7 files changed

+223
-4
lines changed

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.opensearch.security.systemindex;
1111

1212
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
13+
import com.fasterxml.jackson.databind.JsonNode;
1314
import org.junit.Before;
1415
import org.junit.Test;
1516
import org.junit.runner.RunWith;
@@ -81,6 +82,73 @@ public void testPluginShouldBeAbleToIndexDocumentIntoItsSystemIndex() {
8182
}
8283
}
8384

85+
@Test
86+
public void testPluginShouldBeAbleSearchOnItsSystemIndex() {
87+
JsonNode searchResponse1;
88+
JsonNode searchResponse2;
89+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
90+
HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
91+
92+
assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
93+
94+
HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1);
95+
96+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
97+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
98+
99+
searchResponse1 = searchResponse.bodyAsJsonNode();
100+
}
101+
102+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
103+
HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search");
104+
105+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
106+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
107+
108+
searchResponse2 = searchResponse.bodyAsJsonNode();
109+
}
110+
111+
JsonNode hits1 = searchResponse1.get("hits");
112+
JsonNode hits2 = searchResponse2.get("hits");
113+
assertThat(hits1.toPrettyString(), equalTo(hits2.toPrettyString()));
114+
}
115+
116+
@Test
117+
public void testPluginShouldBeAbleGetOnItsSystemIndex() {
118+
JsonNode getResponse1;
119+
JsonNode getResponse2;
120+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
121+
HttpResponse response = client.put("try-create-and-bulk-index/" + SYSTEM_INDEX_1);
122+
123+
assertThat(response.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
124+
125+
HttpResponse searchResponse = client.get("search-on-system-index/" + SYSTEM_INDEX_1);
126+
127+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
128+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
129+
130+
String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id");
131+
132+
HttpResponse getResponse = client.get("get-on-system-index/" + SYSTEM_INDEX_1 + "/" + docId);
133+
134+
getResponse1 = getResponse.bodyAsJsonNode();
135+
}
136+
137+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
138+
HttpResponse searchResponse = client.get(SYSTEM_INDEX_1 + "/_search");
139+
140+
assertThat(searchResponse.getStatusCode(), equalTo(RestStatus.OK.getStatus()));
141+
assertThat(searchResponse.getIntFromJsonBody("/hits/total/value"), equalTo(2));
142+
143+
String docId = searchResponse.getTextFromJsonBody("/hits/hits/0/_id");
144+
145+
HttpResponse getResponse = client.get(SYSTEM_INDEX_1 + "/_doc/" + docId);
146+
147+
getResponse2 = getResponse.bodyAsJsonNode();
148+
}
149+
assertThat(getResponse1.toPrettyString(), equalTo(getResponse2.toPrettyString()));
150+
}
151+
84152
@Test
85153
public void testPluginShouldNotBeAbleToIndexDocumentIntoSystemIndexRegisteredByOtherPlugin() {
86154
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
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+
111
package org.opensearch.security.systemindex;
212

313
import java.util.List;

‎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
@@ -162,7 +162,7 @@ protected final boolean isBlockedSystemIndexRequest() {
162162

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

179179
protected final boolean isAdminDnOrPluginRequest() {
180180
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
181-
if (user == null) {
182-
// allow request without user from plugin.
181+
if (HeaderHelper.isInternalOrPluginRequest(threadContext)) {
183182
return true;
184183
} else if (adminDns.isAdmin(user)) {
185184
return true;

‎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

@@ -50,6 +51,18 @@ public static boolean isExtensionRequest(final ThreadContext context) {
5051
return context.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_TRANSPORT_EXTENSION_REQUEST) == Boolean.TRUE;
5152
}
5253

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

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

0 commit comments

Comments
 (0)