Skip to content

Commit 9ef236e

Browse files
authored
Merge branch 'main' into null-field-name
Signed-off-by: Craig Perkins <cwperx@amazon.com>
2 parents 76107c2 + 8c69314 commit 9ef236e

File tree

22 files changed

+1182
-44
lines changed

22 files changed

+1182
-44
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2525
* Optimized performance for construction of internal action privileges data structure ([#5470](https://github.com/opensearch-project/security/pull/5470))
2626
* Restricting query optimization via star tree index for users with queries on indices with DLS/FLS/FieldMasked restrictions ([#5492](https://github.com/opensearch-project/security/pull/5492))
2727
* Handle subject in nested claim for JWT auth backends ([#5467](https://github.com/opensearch-project/security/pull/5467))
28+
* [Resource Sharing] Adds a Share API to fetch and update sharing information ([#5459](https://github.com/opensearch-project/security/pull/5459))
2829
* Integration with stream transport ([#5530](https://github.com/opensearch-project/security/pull/5530))
2930

3031
### Bug Fixes
@@ -36,6 +37,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
3637
* Fix usage of jwt_clock_skew_tolerance_seconds in HTTPJwtAuthenticator ([#5506](https://github.com/opensearch-project/security/pull/5506))
3738
* Always install demo certs if configured with demo certs ([#5517](https://github.com/opensearch-project/security/pull/5517))
3839
* [Resource Sharing] Restores client accessor pattern to fix compilation issues when security plugin is not installed ([#5541](https://github.com/opensearch-project/security/pull/5541))
40+
* Add serialized user custom attributes to the the thread context ([#5491](https://github.com/opensearch-project/security/pull/5491))
3941
* Fix NullPointerExceptions for "missing values" term aggregations and sorting on geo points ([#5537](https://github.com/opensearch-project/security/pull/5537))
4042

4143
### Refactoring

sample-resource-plugin/build.gradle

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ ext {
3838
licenseFile = rootProject.file('LICENSE.txt')
3939
noticeFile = rootProject.file('NOTICE.txt')
4040

41-
common_utils_version = System.getProperty("common_utils.version", "3.1.0.0")
41+
common_utils_version = System.getProperty("common_utils.version", "3.2.0.0-SNAPSHOT")
4242
}
4343

4444
repositories {
@@ -80,6 +80,7 @@ dependencies {
8080
integrationTestImplementation rootProject.sourceSets.integrationTest.output
8181
integrationTestImplementation rootProject.sourceSets.main.output
8282
integrationTestImplementation "org.opensearch.client:opensearch-rest-high-level-client:${opensearch_version}"
83+
integrationTestImplementation 'org.ldaptive:ldaptive:1.2.3'
8384

8485
// To be removed once integration test framework supports extended plugins
8586
integrationTestImplementation project(path: ":${rootProject.name}-spi", configuration: 'shadow')
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
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+
9+
package org.opensearch.sample.resource;
10+
11+
import java.util.HashMap;
12+
import java.util.HashSet;
13+
import java.util.Map;
14+
import java.util.Set;
15+
16+
import com.carrotsearch.randomizedtesting.RandomizedRunner;
17+
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
18+
import org.apache.http.HttpStatus;
19+
import org.junit.After;
20+
import org.junit.Before;
21+
import org.junit.ClassRule;
22+
import org.junit.Test;
23+
import org.junit.runner.RunWith;
24+
import org.junit.runners.Suite;
25+
26+
import org.opensearch.security.spi.resources.sharing.Recipient;
27+
import org.opensearch.security.spi.resources.sharing.Recipients;
28+
import org.opensearch.test.framework.cluster.LocalCluster;
29+
import org.opensearch.test.framework.cluster.TestRestClient;
30+
31+
import static org.hamcrest.MatcherAssert.assertThat;
32+
import static org.hamcrest.Matchers.containsString;
33+
import static org.hamcrest.Matchers.equalTo;
34+
import static org.hamcrest.Matchers.not;
35+
import static org.opensearch.sample.resource.TestUtils.FULL_ACCESS_USER;
36+
import static org.opensearch.sample.resource.TestUtils.LIMITED_ACCESS_USER;
37+
import static org.opensearch.sample.resource.TestUtils.NO_ACCESS_USER;
38+
import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_INDEX;
39+
import static org.opensearch.sample.resource.TestUtils.SECURITY_SHARE_ENDPOINT;
40+
import static org.opensearch.sample.resource.TestUtils.newCluster;
41+
import static org.opensearch.sample.resource.TestUtils.putSharingInfoPayload;
42+
import static org.opensearch.sample.resource.TestUtils.sampleAllAG;
43+
import static org.opensearch.sample.resource.TestUtils.sampleReadOnlyAG;
44+
import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME;
45+
import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN;
46+
47+
/**
48+
* This test file tests the share API defined by the security plugin.
49+
* Resource access control feature and system index protection are assumed to be enabled
50+
*/
51+
@RunWith(Suite.class)
52+
@Suite.SuiteClasses({ ShareApiTests.RoutesTests.class })
53+
public class ShareApiTests {
54+
/**
55+
* Base test class providing shared cluster setup and teardown
56+
*/
57+
public static abstract class BaseTests {
58+
@ClassRule
59+
public static LocalCluster cluster = newCluster(true, true);
60+
61+
@After
62+
public void clearIndices() {
63+
try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) {
64+
client.delete(RESOURCE_INDEX_NAME);
65+
client.delete(RESOURCE_SHARING_INDEX);
66+
}
67+
}
68+
}
69+
70+
/**
71+
* Tests exercising the share API endpoints, GET, PUT & PATCH
72+
*/
73+
@RunWith(RandomizedRunner.class)
74+
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
75+
public static class RoutesTests extends BaseTests {
76+
private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster);
77+
private String adminResId;
78+
79+
@Before
80+
public void setup() {
81+
adminResId = api.createSampleResourceAs(USER_ADMIN);
82+
api.awaitSharingEntry();
83+
}
84+
85+
@Test
86+
public void testPutSharingInfo() {
87+
// non-permission user cannot share resource
88+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
89+
TestRestClient.HttpResponse response = client.putJson(
90+
SECURITY_SHARE_ENDPOINT,
91+
putSharingInfoPayload(adminResId, RESOURCE_INDEX_NAME, sampleReadOnlyAG.name(), NO_ACCESS_USER.getName())
92+
);
93+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
94+
}
95+
96+
// a sharing entry should be created successfully since admin has access to share API
97+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
98+
TestRestClient.HttpResponse response = client.putJson(
99+
SECURITY_SHARE_ENDPOINT,
100+
putSharingInfoPayload(adminResId, RESOURCE_INDEX_NAME, sampleAllAG.name(), LIMITED_ACCESS_USER.getName())
101+
);
102+
response.assertStatusCode(HttpStatus.SC_OK);
103+
assertThat(response.getBody(), containsString(LIMITED_ACCESS_USER.getName()));
104+
assertThat(response.getBody(), not(containsString(NO_ACCESS_USER.getName())));
105+
}
106+
107+
// non-permission user will now have access to directly call share API
108+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
109+
TestRestClient.HttpResponse response = client.putJson(
110+
SECURITY_SHARE_ENDPOINT,
111+
putSharingInfoPayload(adminResId, RESOURCE_INDEX_NAME, sampleReadOnlyAG.name(), NO_ACCESS_USER.getName())
112+
);
113+
response.assertStatusCode(HttpStatus.SC_OK);
114+
assertThat(response.getBody(), containsString(NO_ACCESS_USER.getName()));
115+
}
116+
}
117+
118+
@Test
119+
public void testGetSharingInfo() {
120+
// non-permission user cannot list shared resources,
121+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
122+
TestRestClient.HttpResponse response = client.get(
123+
SECURITY_SHARE_ENDPOINT + "?resource_id=" + adminResId + "&resource_type=" + RESOURCE_INDEX_NAME
124+
);
125+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
126+
}
127+
128+
// a sharing entry should be created successfully since admin has access to share API
129+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
130+
TestRestClient.HttpResponse response = client.putJson(
131+
SECURITY_SHARE_ENDPOINT,
132+
putSharingInfoPayload(adminResId, RESOURCE_INDEX_NAME, sampleAllAG.name(), FULL_ACCESS_USER.getName())
133+
);
134+
response.assertStatusCode(HttpStatus.SC_OK);
135+
assertThat(response.getBody(), containsString(FULL_ACCESS_USER.getName()));
136+
}
137+
138+
// non-permission user can now list shared_with resources by calling share API
139+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
140+
TestRestClient.HttpResponse response = client.get(
141+
SECURITY_SHARE_ENDPOINT + "?resource_id=" + adminResId + "&resource_type=" + RESOURCE_INDEX_NAME
142+
);
143+
response.assertStatusCode(HttpStatus.SC_OK);
144+
assertThat(response.bodyAsJsonNode().get("sharing_info").get("resource_id").asText(), equalTo(adminResId));
145+
}
146+
}
147+
148+
@Test
149+
public void testPatchSharingInfo() {
150+
Map<Recipient, Set<String>> recs = new HashMap<>();
151+
Set<String> users = new HashSet<>();
152+
users.add(FULL_ACCESS_USER.getName());
153+
recs.put(Recipient.USERS, users);
154+
Recipients recipients = new Recipients(recs);
155+
156+
TestUtils.PatchSharingInfoPayloadBuilder patchSharingInfoPayloadBuilder = new TestUtils.PatchSharingInfoPayloadBuilder();
157+
patchSharingInfoPayloadBuilder.resourceId(adminResId).resourceIndex(RESOURCE_INDEX_NAME).share(recipients, sampleAllAG.name());
158+
159+
// full-access user cannot share with itself since user doesn't have permission to share
160+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
161+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
162+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
163+
}
164+
165+
// a sharing entry should be created successfully since admin has access to share API
166+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
167+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
168+
response.assertStatusCode(HttpStatus.SC_OK);
169+
assertThat(response.getBody(), containsString(FULL_ACCESS_USER.getName()));
170+
}
171+
172+
// limited access user will not be able to call patch endpoint
173+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
174+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
175+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
176+
}
177+
178+
// full-access user will now be able to patch and grant access to limited access user
179+
// they can also shoot themselves in the foot and remove own access
180+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
181+
// add limited user
182+
users.add(LIMITED_ACCESS_USER.getName());
183+
patchSharingInfoPayloadBuilder.share(recipients, sampleAllAG.name());
184+
// remove self
185+
Set<String> revokedUsers = new HashSet<>();
186+
revokedUsers.add(FULL_ACCESS_USER.getName());
187+
recs.put(Recipient.USERS, revokedUsers);
188+
recipients = new Recipients(recs);
189+
patchSharingInfoPayloadBuilder.revoke(recipients, sampleAllAG.name());
190+
191+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
192+
response.assertStatusCode(HttpStatus.SC_OK);
193+
}
194+
195+
// limited access user will now be able to call patch endpoint, but full-access won't
196+
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
197+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
198+
response.assertStatusCode(HttpStatus.SC_OK);
199+
}
200+
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
201+
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
202+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
203+
}
204+
}
205+
}
206+
207+
}

sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,25 @@
88

99
package org.opensearch.sample.resource;
1010

11+
import java.io.IOException;
1112
import java.time.Duration;
13+
import java.util.ArrayList;
14+
import java.util.HashMap;
1215
import java.util.List;
1316
import java.util.Map;
1417

1518
import org.apache.http.HttpStatus;
1619
import org.awaitility.Awaitility;
1720

1821
import org.opensearch.Version;
22+
import org.opensearch.common.xcontent.XContentFactory;
23+
import org.opensearch.core.xcontent.ToXContent;
24+
import org.opensearch.core.xcontent.XContentBuilder;
1925
import org.opensearch.painless.PainlessModulePlugin;
2026
import org.opensearch.plugins.PluginInfo;
2127
import org.opensearch.sample.SampleResourcePlugin;
2228
import org.opensearch.security.OpenSearchSecurityPlugin;
29+
import org.opensearch.security.spi.resources.sharing.Recipients;
2330
import org.opensearch.test.framework.TestSecurityConfig;
2431
import org.opensearch.test.framework.certificate.CertificateData;
2532
import org.opensearch.test.framework.cluster.ClusterManager;
@@ -72,7 +79,8 @@ public final class TestUtils {
7279
"sample_plugin_index_all_access",
7380
TestSecurityConfig.ActionGroup.Type.INDEX,
7481
"indices:*",
75-
"cluster:admin/sample-resource-plugin/*"
82+
"cluster:admin/sample-resource-plugin/*",
83+
"cluster:admin/security/resource/share"
7684
);
7785

7886
public static final String SAMPLE_RESOURCE_CREATE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/create";
@@ -83,6 +91,7 @@ public final class TestUtils {
8391
public static final String SAMPLE_RESOURCE_REVOKE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/revoke";
8492

8593
static final String RESOURCE_SHARING_MIGRATION_ENDPOINT = "_plugins/_security/api/resources/migrate";
94+
static final String SECURITY_SHARE_ENDPOINT = "_plugins/_security/api/resource/share";
8695

8796
public static LocalCluster newCluster(boolean featureEnabled, boolean systemIndexEnabled) {
8897
return new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS_COORDINATOR)
@@ -199,6 +208,90 @@ static String migrationPayload_missingBackendRoles() {
199208
""".formatted(RESOURCE_INDEX_NAME, "user/name");
200209
}
201210

211+
static String putSharingInfoPayload(String resourceId, String resourceIndex, String accessLevel, String user) {
212+
return """
213+
{
214+
"resource_id": "%s",
215+
"resource_type": "%s",
216+
"share_with": {
217+
"%s" : {
218+
"users": ["%s"]
219+
}
220+
}
221+
}
222+
""".formatted(resourceId, resourceIndex, accessLevel, user);
223+
}
224+
225+
public static class PatchSharingInfoPayloadBuilder {
226+
private String resourceId;
227+
private String resourceIndex;
228+
private final Map<String, Recipients> share = new HashMap<>();
229+
private final Map<String, Recipients> revoke = new HashMap<>();
230+
231+
public PatchSharingInfoPayloadBuilder resourceId(String resourceId) {
232+
this.resourceId = resourceId;
233+
return this;
234+
}
235+
236+
public PatchSharingInfoPayloadBuilder resourceIndex(String resourceIndex) {
237+
this.resourceIndex = resourceIndex;
238+
return this;
239+
}
240+
241+
public void share(Recipients recipients, String accessLevel) {
242+
Recipients existing = share.getOrDefault(accessLevel, new Recipients(new HashMap<>()));
243+
existing.share(recipients);
244+
share.put(accessLevel, existing);
245+
}
246+
247+
public void revoke(Recipients recipients, String accessLevel) {
248+
Recipients existing = revoke.getOrDefault(accessLevel, new Recipients(new HashMap<>()));
249+
// intentionally share() is called here since we are building a shareWith object, this final object will be used to remove
250+
// access
251+
// think of it as currentShareWith.removeAll(revokeShareWith)
252+
existing.share(recipients);
253+
revoke.put(accessLevel, existing);
254+
}
255+
256+
private String buildJsonString(Map<String, Recipients> input) {
257+
258+
List<String> output = new ArrayList<>();
259+
for (Map.Entry<String, Recipients> entry : input.entrySet()) {
260+
try {
261+
XContentBuilder builder = XContentFactory.jsonBuilder();
262+
entry.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS);
263+
String recipJson = builder.toString();
264+
output.add("""
265+
"%s" : %s
266+
""".formatted(entry.getKey(), recipJson));
267+
} catch (IOException e) {
268+
throw new RuntimeException(e);
269+
}
270+
271+
}
272+
273+
return String.join(",", output);
274+
275+
}
276+
277+
public String build() {
278+
String allShares = buildJsonString(share);
279+
String allRevokes = buildJsonString(revoke);
280+
return """
281+
{
282+
"resource_id": "%s",
283+
"resource_type": "%s",
284+
"add": {
285+
%s
286+
},
287+
"revoke": {
288+
%s
289+
}
290+
}
291+
""".formatted(resourceId, resourceIndex, allShares, allRevokes);
292+
}
293+
}
294+
202295
public static class ApiHelper {
203296
private final LocalCluster cluster;
204297

0 commit comments

Comments
 (0)