diff --git a/CHANGELOG.md b/CHANGELOG.md index e916feeacd..9c75a5339c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Allow any plugin system request when `plugins.security.system_indices.enabled` is set to `false` ([#5579](https://github.com/opensearch-project/security/pull/5579)) - [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)) - 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)) +- Updates resource visibility when handling PATCH api to update sharing record ([#5654](https://github.com/opensearch-project/security/pull/5654)) ### Refactoring diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java index dc2974f9f3..b88983b30a 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareApiTests.java @@ -39,6 +39,7 @@ import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_INDEX; import static org.opensearch.sample.resource.TestUtils.SAMPLE_FULL_ACCESS_RESOURCE_AG; import static org.opensearch.sample.resource.TestUtils.SAMPLE_READ_ONLY_RESOURCE_AG; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_GET_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.SECURITY_SHARE_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.newCluster; import static org.opensearch.sample.resource.TestUtils.putSharingInfoPayload; @@ -251,13 +252,17 @@ public void testPatchSharingInfo() { response.assertStatusCode(HttpStatus.SC_OK); } - // limited access user will now be able to call patch endpoint, but full-access won't + // limited access user will now be able to call get and patch endpoint, but full-access won't try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) { - TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build()); + TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId); + response.assertStatusCode(HttpStatus.SC_OK); + response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build()); response.assertStatusCode(HttpStatus.SC_OK); } try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) { - TestRestClient.HttpResponse response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build()); + TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + adminResId); + response.assertStatusCode(HttpStatus.SC_FORBIDDEN); + response = client.patch(SECURITY_SHARE_ENDPOINT, patchSharingInfoPayloadBuilder.build()); response.assertStatusCode(HttpStatus.SC_FORBIDDEN); } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 260b35ae94..88b7a89967 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -786,8 +786,8 @@ public void patchSharingInfo( fetchSharingInfo(resourceIndex, resourceId, sharingInfoListener); // Apply patch and update the document - sharingInfoListener.whenComplete(resourceSharing -> { - ShareWith updatedShareWith = resourceSharing.getShareWith(); + sharingInfoListener.whenComplete(sharingInfo -> { + ShareWith updatedShareWith = sharingInfo.getShareWith(); if (updatedShareWith == null) { updatedShareWith = new ShareWith(new HashMap<>()); } @@ -806,7 +806,7 @@ public void patchSharingInfo( } } - ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, resourceSharing.getCreatedBy(), cleaned); + ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, sharingInfo.getCreatedBy(), cleaned); try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { // update the record @@ -826,7 +826,19 @@ public void patchSharingInfo( resourceIndex ); - listener.onResponse(updatedSharingInfo); + updateResourceVisibility( + resourceId, + resourceIndex, + updatedSharingInfo.getAllPrincipals(), + ActionListener.wrap((updateResponse) -> { + LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); + listener.onResponse(updatedSharingInfo); + }, (e) -> { + LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); + listener.onResponse(updatedSharingInfo); + }) + ); + }, (e) -> { LOGGER.error(e.getMessage()); listener.onFailure(e);