Skip to content

Commit e8a82af

Browse files
Updates resource visibility when handling PATCH api to update sharing record (#5654)
Signed-off-by: Darshit Chanpura <[email protected]>
1 parent f944fb4 commit e8a82af

File tree

3 files changed

+25
-7
lines changed

3 files changed

+25
-7
lines changed

CHANGELOG.md

Lines changed: 1 addition & 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
- Allow any plugin system request when `plugins.security.system_indices.enabled` is set to `false` ([#5579](https://github.com/opensearch-project/security/pull/5579))
2626
- [Resource Sharing] Always treat GET _doc request as indices request even when performed on sharable resource index ([#5631](https://github.com/opensearch-project/security/pull/5631))
2727
- Fix JWT log spam when JWT authenticator is configured with an empty list for roles_key ([#5640](https://github.com/opensearch-project/security/pull/5640))
28+
- Updates resource visibility when handling PATCH api to update sharing record ([#5654](https://github.com/opensearch-project/security/pull/5654))
2829

2930
### Refactoring
3031

sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_INDEX;
4040
import static org.opensearch.sample.resource.TestUtils.SAMPLE_FULL_ACCESS_RESOURCE_AG;
4141
import static org.opensearch.sample.resource.TestUtils.SAMPLE_READ_ONLY_RESOURCE_AG;
42+
import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_GET_ENDPOINT;
4243
import static org.opensearch.sample.resource.TestUtils.SECURITY_SHARE_ENDPOINT;
4344
import static org.opensearch.sample.resource.TestUtils.newCluster;
4445
import static org.opensearch.sample.resource.TestUtils.putSharingInfoPayload;
@@ -251,13 +252,17 @@ public void testPatchSharingInfo() {
251252
response.assertStatusCode(HttpStatus.SC_OK);
252253
}
253254

254-
// limited access user will now be able to call patch endpoint, but full-access won't
255+
// limited access user will now be able to call get and patch endpoint, but full-access won't
255256
try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) {
256-
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
257+
TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId);
258+
response.assertStatusCode(HttpStatus.SC_OK);
259+
response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
257260
response.assertStatusCode(HttpStatus.SC_OK);
258261
}
259262
try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) {
260-
TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
263+
TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId);
264+
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
265+
response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build());
261266
response.assertStatusCode(HttpStatus.SC_FORBIDDEN);
262267
}
263268
}

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -786,8 +786,8 @@ public void patchSharingInfo(
786786
fetchSharingInfo(resourceIndex, resourceId, sharingInfoListener);
787787

788788
// Apply patch and update the document
789-
sharingInfoListener.whenComplete(resourceSharing -> {
790-
ShareWith updatedShareWith = resourceSharing.getShareWith();
789+
sharingInfoListener.whenComplete(sharingInfo -> {
790+
ShareWith updatedShareWith = sharingInfo.getShareWith();
791791
if (updatedShareWith == null) {
792792
updatedShareWith = new ShareWith(new HashMap<>());
793793
}
@@ -806,7 +806,7 @@ public void patchSharingInfo(
806806
}
807807
}
808808

809-
ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, resourceSharing.getCreatedBy(), cleaned);
809+
ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, sharingInfo.getCreatedBy(), cleaned);
810810

811811
try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) {
812812
// update the record
@@ -826,7 +826,19 @@ public void patchSharingInfo(
826826
resourceIndex
827827
);
828828

829-
listener.onResponse(updatedSharingInfo);
829+
updateResourceVisibility(
830+
resourceId,
831+
resourceIndex,
832+
updatedSharingInfo.getAllPrincipals(),
833+
ActionListener.wrap((updateResponse) -> {
834+
LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex);
835+
listener.onResponse(updatedSharingInfo);
836+
}, (e) -> {
837+
LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e);
838+
listener.onResponse(updatedSharingInfo);
839+
})
840+
);
841+
830842
}, (e) -> {
831843
LOGGER.error(e.getMessage());
832844
listener.onFailure(e);

0 commit comments

Comments
 (0)