Skip to content

Commit d29095f

Browse files
authored
[Resource Sharing] Keep track of tenant for sharable resources by persisting user requested tenant with sharing info (#5588)
Signed-off-by: Craig Perkins <[email protected]>
1 parent 16993b4 commit d29095f

File tree

6 files changed

+140
-16
lines changed

6 files changed

+140
-16
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
99

1010
### Enhancements
1111

12+
- [Resource Sharing] Keep track of tenant for sharable resources by persisting user requested tenant with sharing info ([#5588](https://github.com/opensearch-project/security/pull/5588))
13+
1214
### Bug Fixes
1315

1416
* Added new option skip_users to client cert authenticator (clientcert_auth_domain.http_authenticator.config.skip_users in config.yml)([#4378](https://github.com/opensearch-project/security/pull/5525))

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.List;
1616
import java.util.Map;
1717

18+
import org.apache.hc.core5.http.Header;
1819
import org.apache.http.HttpStatus;
1920
import org.awaitility.Awaitility;
2021

@@ -325,10 +326,10 @@ public void wipeOutResourceEntries() {
325326
}
326327

327328
// Helper to create a sample resource and return its ID
328-
public String createSampleResourceAs(TestSecurityConfig.User user) {
329+
public String createSampleResourceAs(TestSecurityConfig.User user, Header... headers) {
329330
try (TestRestClient client = cluster.getRestClient(user)) {
330331
String sample = "{\"name\":\"sample\"}";
331-
TestRestClient.HttpResponse resp = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sample);
332+
TestRestClient.HttpResponse resp = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sample, headers);
332333
resp.assertStatusCode(HttpStatus.SC_OK);
333334
return resp.getTextFromJsonBody("/message").split(":")[1].trim();
334335
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
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.feature.enabled;
10+
11+
import com.carrotsearch.randomizedtesting.RandomizedRunner;
12+
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
13+
import org.apache.hc.core5.http.message.BasicHeader;
14+
import org.junit.After;
15+
import org.junit.Before;
16+
import org.junit.ClassRule;
17+
import org.junit.Test;
18+
import org.junit.runner.RunWith;
19+
20+
import org.opensearch.sample.resource.TestUtils;
21+
import org.opensearch.test.framework.cluster.LocalCluster;
22+
import org.opensearch.test.framework.cluster.TestRestClient;
23+
24+
import static org.hamcrest.MatcherAssert.assertThat;
25+
import static org.hamcrest.Matchers.containsString;
26+
import static org.opensearch.sample.resource.TestUtils.newCluster;
27+
import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME;
28+
import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN;
29+
30+
/**
31+
* Test resource access when shared with full-access (sampleAllAG action-group).
32+
* All tests are against USER_ADMIN's resource created during setup.
33+
*/
34+
@RunWith(RandomizedRunner.class)
35+
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
36+
public class ResourceSharingMultiTenancyTests {
37+
38+
@ClassRule
39+
public static LocalCluster cluster = newCluster(true, true);
40+
41+
private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster);
42+
private String resourceId;
43+
44+
@Before
45+
public void setup() {
46+
resourceId = api.createSampleResourceAs(USER_ADMIN, new BasicHeader("securitytenant", "customtenant"));
47+
api.awaitSharingEntry(); // wait until sharing entry is created
48+
}
49+
50+
@After
51+
public void cleanup() {
52+
api.wipeOutResourceEntries();
53+
}
54+
55+
@Test
56+
public void testCreateResourceWithMultiTenancyEnabled() {
57+
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
58+
TestRestClient.HttpResponse sharingInfoResponse = client.get(
59+
"_plugins/_security/api/resource/share?resource_id=" + resourceId + "&resource_type=" + RESOURCE_INDEX_NAME
60+
);
61+
assertThat(sharingInfoResponse.getBody(), containsString("\"created_by\":{\"user\":\"admin\",\"tenant\":\"customtenant\"}"));
62+
}
63+
}
64+
}

spi/src/main/java/org/opensearch/security/spi/resources/sharing/CreatedBy.java

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
import org.opensearch.core.xcontent.XContentBuilder;
1818
import org.opensearch.core.xcontent.XContentParser;
1919

20+
import static org.opensearch.core.xcontent.XContentParser.Token.VALUE_NULL;
21+
import static org.opensearch.core.xcontent.XContentParser.Token.VALUE_STRING;
22+
2023
/**
2124
* This class is used to store information about the creator of a resource.
2225
*
@@ -25,22 +28,41 @@
2528
public class CreatedBy implements ToXContentFragment, NamedWriteable {
2629

2730
private final String username;
31+
private final String tenant; // capture tenant if multi-tenancy is enabled
2832

2933
public CreatedBy(String username) {
3034
this.username = username;
35+
this.tenant = null;
36+
}
37+
38+
public CreatedBy(String username, String tenant) {
39+
this.username = username;
40+
this.tenant = tenant;
3141
}
3242

3343
public CreatedBy(StreamInput in) throws IOException {
3444
this.username = in.readString();
45+
this.tenant = in.readOptionalString();
3546
}
3647

3748
public String getUsername() {
3849
return username;
3950
}
4051

52+
public String getTenant() {
53+
return tenant;
54+
}
55+
4156
@Override
4257
public String toString() {
43-
return "CreatedBy {user='" + this.username + '\'' + '}';
58+
if (tenant != null) {
59+
return """
60+
CreatedBy {user='%s', tenant='%s'}
61+
""".formatted(username, tenant).trim();
62+
}
63+
return """
64+
CreatedBy {user='%s'}
65+
""".formatted(username).trim();
4466
}
4567

4668
@Override
@@ -51,31 +73,49 @@ public String getWriteableName() {
5173
@Override
5274
public void writeTo(StreamOutput out) throws IOException {
5375
out.writeString(username);
76+
out.writeOptionalString(tenant);
5477
}
5578

5679
@Override
5780
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
81+
if (tenant != null) {
82+
return builder.startObject().field("user", username).field("tenant", tenant).endObject();
83+
}
5884
return builder.startObject().field("user", username).endObject();
5985
}
6086

6187
public static CreatedBy fromXContent(XContentParser parser) throws IOException {
6288
String username = null;
63-
XContentParser.Token token;
64-
65-
while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) {
66-
if (token == XContentParser.Token.FIELD_NAME) {
67-
if (!"user".equals(parser.currentName())) {
68-
throw new IllegalArgumentException("created_by must only contain a single field with user");
89+
String tenant = null;
90+
91+
while (parser.nextToken() != XContentParser.Token.END_OBJECT) {
92+
if (parser.currentToken() == XContentParser.Token.FIELD_NAME) {
93+
String fieldName = parser.currentName();
94+
parser.nextToken();
95+
96+
switch (fieldName) {
97+
case "user":
98+
if (VALUE_STRING == parser.currentToken()) {
99+
username = parser.text();
100+
} else {
101+
throw new IllegalArgumentException("created_by cannot be empty");
102+
}
103+
break;
104+
105+
case "tenant":
106+
tenant = (parser.currentToken() == VALUE_NULL) ? null : parser.text();
107+
break;
108+
109+
default:
110+
throw new IllegalArgumentException("created_by contains unknown field: " + fieldName);
69111
}
70-
} else if (token == XContentParser.Token.VALUE_STRING) {
71-
username = parser.text();
72112
}
73113
}
74114

75115
if (username == null) {
76116
throw new IllegalArgumentException("created_by cannot be empty");
77117
}
78118

79-
return new CreatedBy(username);
119+
return new CreatedBy(username, tenant);
80120
}
81121
}

spi/src/test/java/org/opensearch/security/spi/resources/CreatedByTests.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,27 @@ public void testCreatedByConstructorWithValidUser() {
4646
}
4747

4848
@Test
49-
public void testCreatedByFromStreamInput() throws IOException {
49+
public void testCreatedByConstructorWithValidUserAndTenant() {
5050
String expectedUser = "testUser";
51+
String expectedTenant = "customTenant";
52+
CreatedBy createdBy = new CreatedBy(expectedUser, expectedTenant);
53+
54+
MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getUsername())));
55+
MatcherAssert.assertThat(expectedTenant, is(equalTo(createdBy.getTenant())));
56+
}
57+
58+
@Test
59+
public void testCreatedByFromStreamInput() throws IOException {
60+
CreatedBy expectedUser = new CreatedBy("testUser");
5161

5262
try (BytesStreamOutput out = new BytesStreamOutput()) {
53-
out.writeString(expectedUser);
63+
expectedUser.writeTo(out);
5464

5565
StreamInput in = out.bytes().streamInput();
5666

5767
CreatedBy createdBy = new CreatedBy(in);
5868

59-
MatcherAssert.assertThat(expectedUser, is(equalTo(createdBy.getUsername())));
69+
MatcherAssert.assertThat(expectedUser.getUsername(), is(equalTo(createdBy.getUsername())));
6070
}
6171
}
6272

src/main/java/org/opensearch/security/resources/ResourceIndexListener.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,14 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re
7373
resourceIndex
7474
);
7575
}, e -> { log.debug(e.getMessage()); });
76-
this.resourceSharingIndexHandler.indexResourceSharing(resourceId, resourceIndex, new CreatedBy(user.getName()), null, listener);
76+
// User.getRequestedTenant() is null if multi-tenancy is disabled
77+
this.resourceSharingIndexHandler.indexResourceSharing(
78+
resourceId,
79+
resourceIndex,
80+
new CreatedBy(user.getName(), user.getRequestedTenant()),
81+
null,
82+
listener
83+
);
7784

7885
} catch (IOException e) {
7986
log.debug("Failed to create a resource sharing entry for resource: {}", resourceId, e);

0 commit comments

Comments
 (0)