diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f7532c0b1..3a031169b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Register cluster settings listener for `plugins.security.cache.ttl_minutes` ([#5324](https://github.com/opensearch-project/security/pull/5324)) ### Changed +- Use extendedPlugins in integrationTest framework for sample resource plugin testing ([#5322](https://github.com/opensearch-project/security/pull/5322)) ### Dependencies - Bump `guava_version` from 33.4.6-jre to 33.4.8-jre ([#5284](https://github.com/opensearch-project/security/pull/5284)) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java index 014de1dbc2..e3135e60a7 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/SampleResourcePluginTests.java @@ -8,6 +8,7 @@ package org.opensearch.sample; +import java.util.List; import java.util.Map; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; @@ -19,8 +20,10 @@ import org.junit.Test; import org.junit.runner.RunWith; +import org.opensearch.Version; import org.opensearch.painless.PainlessModulePlugin; -import org.opensearch.sample.resource.client.ResourceSharingClientAccessor; +import org.opensearch.plugins.PluginInfo; +import org.opensearch.security.OpenSearchSecurityPlugin; import org.opensearch.security.resources.ResourcePluginInfo; import org.opensearch.security.spi.resources.ResourceAccessActionGroups; import org.opensearch.security.spi.resources.ResourceSharingExtension; @@ -39,7 +42,6 @@ import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_SHARE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SAMPLE_RESOURCE_UPDATE_ENDPOINT; import static org.opensearch.sample.SampleResourcePluginTestHelper.SHARED_WITH_USER; -import static org.opensearch.sample.SampleResourcePluginTestHelper.createResourceAccessControlClient; import static org.opensearch.sample.SampleResourcePluginTestHelper.revokeAccessPayload; import static org.opensearch.sample.SampleResourcePluginTestHelper.shareWithPayload; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -61,7 +63,20 @@ public class SampleResourcePluginTests { @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .plugin(SampleResourcePlugin.class, PainlessModulePlugin.class) + .plugin(PainlessModulePlugin.class) + .plugin( + new PluginInfo( + SampleResourcePlugin.class.getName(), + "classpath plugin", + "NA", + Version.CURRENT, + "1.8", + SampleResourcePlugin.class.getName(), + null, + List.of(OpenSearchSecurityPlugin.class.getName()), + false + ) + ) .anonymousAuth(true) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(USER_ADMIN, SHARED_WITH_USER) @@ -95,7 +110,6 @@ public void testPluginInstalledCorrectly() { @Test public void testCreateUpdateDeleteSampleResource() throws Exception { String resourceId; - String resourceSharingDocId; // create sample resource try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { String sampleResource = """ @@ -106,35 +120,13 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { response.assertStatusCode(HttpStatus.SC_OK); resourceId = response.getTextFromJsonBody("/message").split(":")[1].trim(); - } - - // Create an entry in resource-sharing index - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - String json = """ - { - "source_idx": ".sample_resource_sharing_plugin", - "resource_id": "%s", - "created_by": { - "user": "admin" - } - } - """.formatted(resourceId); - - TestRestClient.HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - resourceSharingDocId = response.bodyAsJsonNode().get("_id").asText(); - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); Awaitility.await() .alias("Wait until resource data is populated") .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(200)); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.getBody(), containsString("sample")); - // Wait until resource-sharing entry is successfully created + } + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { Awaitility.await() .alias("Wait until resource-sharing data is populated") .until( @@ -154,16 +146,7 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { sampleResourceUpdated ); updateResponse.assertStatusCode(HttpStatus.SC_OK); - } - // resource should be visible to super-admin - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - Awaitility.await() - .alias("Wait until resource-sharing data is populated") - .until( - () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), - equalTo(1) - ); TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); response.assertStatusCode(HttpStatus.SC_OK); assertThat(response.getBody(), containsString("sampleUpdated")); @@ -194,15 +177,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { ); } - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - Awaitility.await() - .alias("Wait until resource-sharing data is populated") - .until( - () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), - equalTo(1) - ); - } - // share resource with shared_with user try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { TestRestClient.HttpResponse response = client.postJson( @@ -227,13 +201,6 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { assertThat(response.bodyAsJsonNode().get("resources").size(), equalTo(1)); } - // resource is still visible to super-admin - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.getBody(), containsString("sampleUpdated")); - } - // revoke share_with_user's access try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { TestRestClient.HttpResponse response = client.postJson( @@ -266,30 +233,18 @@ public void testCreateUpdateDeleteSampleResource() throws Exception { response.assertStatusCode(HttpStatus.SC_OK); } - // corresponding entry should be removed from resource-sharing index - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to delete the resource sharing entry manually - TestRestClient.HttpResponse response = client.delete(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc/" + resourceSharingDocId); - response.assertStatusCode(HttpStatus.SC_OK); - - Awaitility.await() - .alias("Wait until resource-sharing data is updated") - .until( - () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), - equalTo(0) - ); - } - // get sample resource with SHARED_WITH_USER try (TestRestClient client = cluster.getRestClient(SHARED_WITH_USER)) { - TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_NOT_FOUND); + Awaitility.await() + .alias("Wait until resource-sharing data is deleted") + .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(HttpStatus.SC_NOT_FOUND)); } // get sample resource with admin try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) { - TestRestClient.HttpResponse response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); - response.assertStatusCode(HttpStatus.SC_NOT_FOUND); + Awaitility.await() + .alias("Wait until resource-sharing data is deleted") + .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(HttpStatus.SC_NOT_FOUND)); } } @@ -310,34 +265,13 @@ public void testDirectAccess() throws Exception { resourceId = response.getTextFromJsonBody("/message").split(":")[1].trim(); } - // Create an entry in resource-sharing index try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - // Since test framework doesn't yet allow loading ex tensions we need to create a resource sharing entry manually - String json = """ - { - "source_idx": "%s", - "resource_id": "%s", - "created_by": { - "user": "admin" - } - } - """.formatted(RESOURCE_INDEX_NAME, resourceId); - HttpResponse response = client.postJson(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_doc", json); - assertThat(response.getStatusReason(), containsString("Created")); - resourcePluginInfo.getResourceSharingExtensionsMutable().add(resourceSharingExtension); - - ResourceSharingClientAccessor.getInstance().setResourceSharingClient(createResourceAccessControlClient(cluster)); - Awaitility.await() .alias("Wait until resource-sharing data is populated") .until( () -> client.get(OPENSEARCH_RESOURCE_SHARING_INDEX + "/_search").bodyAsJsonNode().get("hits").get("hits").size(), equalTo(1) ); - response = client.get(SAMPLE_RESOURCE_GET_ENDPOINT); - response.assertStatusCode(HttpStatus.SC_OK); - assertThat(response.bodyAsJsonNode().get("resources").size(), equalTo(1)); - assertThat(response.getBody(), containsString("sample")); } // admin should not be able to access resource directly since system index protection is enabled, but can access via sample plugin diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java index 933fec7fb4..007087f9ef 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java @@ -36,6 +36,13 @@ */ public class ResourceSharing implements ToXContentFragment, NamedWriteable { + /** + * The unique identifier of the resource sharing entry + * + * TODO If this moves to a shadow index for each resource index, then use the resourceId as the key for both + */ + private String docId; + /** * The index where the resource is defined */ @@ -63,6 +70,14 @@ public ResourceSharing(String sourceIdx, String resourceId, CreatedBy createdBy, this.shareWith = shareWith; } + public String getDocId() { + return docId; + } + + public void setDocId(String docId) { + this.docId = docId; + } + public String getSourceIdx() { return sourceIdx; } @@ -95,6 +110,15 @@ public void setShareWith(ShareWith shareWith) { this.shareWith = shareWith; } + public void share(String accessLevel, SharedWithActionGroup target) { + if (shareWith == null) { + shareWith = new ShareWith(Set.of(target)); + } else { + SharedWithActionGroup sharedWith = shareWith.atAccessLevel(accessLevel); + sharedWith.share(target); + } + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java index 926007c0d5..3de22827a0 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ShareWith.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; import org.opensearch.core.common.io.stream.NamedWriteable; import org.opensearch.core.common.io.stream.StreamInput; @@ -57,6 +58,14 @@ public Set getSharedWithActionGroups() { return sharedWithActionGroups; } + public Set accessLevels() { + return sharedWithActionGroups.stream().map(SharedWithActionGroup::getActionGroup).collect(Collectors.toSet()); + } + + public SharedWithActionGroup atAccessLevel(String accessLevel) { + return sharedWithActionGroups.stream().filter(g -> accessLevel.equals(g.getActionGroup())).findFirst().orElse(null); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java index 1c3800782d..737f2ba3cf 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/SharedWithActionGroup.java @@ -57,6 +57,14 @@ public ActionGroupRecipients getSharedWithPerActionGroup() { return actionGroupRecipients; } + public void share(SharedWithActionGroup target) { + Map> targetRecipients = target.actionGroupRecipients.getRecipients(); + for (Recipient recipientType : targetRecipients.keySet()) { + Set recipients = actionGroupRecipients.getRecipientsByType(recipientType); + recipients.addAll(targetRecipients.get(recipientType)); + } + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.field(actionGroup); @@ -117,6 +125,10 @@ public Map> getRecipients() { return recipients; } + public Set getRecipientsByType(Recipient recipientType) { + return recipients.computeIfAbsent(recipientType, key -> new HashSet<>()); + } + @Override public String getWriteableName() { return "action_group_recipients"; diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index c41635e227..741b06098a 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -11,7 +11,6 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; @@ -400,6 +399,7 @@ public void onResponse(SearchResponse searchResponse) { ) { parser.nextToken(); ResourceSharing resourceSharing = ResourceSharing.fromXContent(parser); + resourceSharing.setDocId(hit.getId()); LOGGER.debug( "Successfully fetched document from {} matching resource_id: {} and source_idx: {}", @@ -497,6 +497,7 @@ public void onFailure(Exception e) { * @param listener Listener to be notified when the operation completes * @throws RuntimeException if there's an error during the update operation */ + @SuppressWarnings("unchecked") public void updateResourceSharingInfo( String resourceId, String sourceIdx, @@ -520,16 +521,14 @@ public void updateResourceSharingInfo( } StepListener fetchDocListener = new StepListener<>(); - StepListener updateScriptListener = new StepListener<>(); - StepListener updatedSharingListener = new StepListener<>(); // Fetch resource sharing doc fetchResourceSharingDocument(sourceIdx, resourceId, fetchDocListener); // build update script - fetchDocListener.whenComplete(currentSharingInfo -> { + fetchDocListener.whenComplete(sharingInfo -> { // Check if user can share. At present only the resource creator and admin is allowed to share the resource - if (!isAdmin && currentSharingInfo != null && !currentSharingInfo.getCreatedBy().getCreator().equals(requestUserName)) { + if (!isAdmin && sharingInfo != null && !sharingInfo.getCreatedBy().getCreator().equals(requestUserName)) { LOGGER.error("User {} is not authorized to share resource {}", requestUserName, resourceId); listener.onFailure( @@ -538,65 +537,30 @@ public void updateResourceSharingInfo( RestStatus.FORBIDDEN ) ); + return; } - Script updateScript = new Script(ScriptType.INLINE, "painless", """ - if (ctx._source.share_with == null) { - ctx._source.share_with = [:]; - } - - for (def entry : params.shareWith.entrySet()) { - def actionGroupName = entry.getKey(); - def newActionGroup = entry.getValue(); - - if (!ctx._source.share_with.containsKey(actionGroupName)) { - def newActionGroupEntry = [:]; - for (def field : newActionGroup.entrySet()) { - if (field.getValue() != null && !field.getValue().isEmpty()) { - newActionGroupEntry[field.getKey()] = new HashSet(field.getValue()); - } - } - ctx._source.share_with[actionGroupName] = newActionGroupEntry; - } else { - def existingActionGroup = ctx._source.share_with[actionGroupName]; - - for (def field : newActionGroup.entrySet()) { - def fieldName = field.getKey(); - def newValues = field.getValue(); - - if (newValues != null && !newValues.isEmpty()) { - if (!existingActionGroup.containsKey(fieldName)) { - existingActionGroup[fieldName] = new HashSet(); - } - - for (def value : newValues) { - if (!existingActionGroup[fieldName].contains(value)) { - existingActionGroup[fieldName].add(value); - } - } - } - } - } - } - """, Collections.singletonMap("shareWith", shareWithMap)); - - updateByQueryResourceSharing(sourceIdx, resourceId, updateScript, updateScriptListener); - - }, listener::onFailure); + for (String accessLevel : shareWith.accessLevels()) { + SharedWithActionGroup target = shareWith.atAccessLevel(accessLevel); + assert sharingInfo != null; + sharingInfo.share(accessLevel, target); + } - // Build & return the updated ResourceSharing - updateScriptListener.whenComplete(success -> { - if (!success) { - LOGGER.error("Failed to update resource sharing info for resource {}", resourceId); - listener.onResponse(null); - return; + try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { + IndexRequest ir = client.prepareIndex(resourceSharingIndex) + .setId(sharingInfo.getDocId()) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + .setSource(sharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .setOpType(DocWriteRequest.OpType.INDEX) + .request(); + + ActionListener irListener = ActionListener.wrap(idxResponse -> { + LOGGER.info("Successfully updated {} entry for resource {} in index {}.", resourceSharingIndex, resourceId, sourceIdx); + listener.onResponse(sharingInfo); + }, (failResponse) -> { LOGGER.error(failResponse.getMessage()); }); + client.index(ir, irListener); } - // TODO check if this should be replaced by Java in-memory computation (current intuition is that it will be more memory - // intensive to do it in java) - fetchResourceSharingDocument(sourceIdx, resourceId, updatedSharingListener); }, listener::onFailure); - - updatedSharingListener.whenComplete(listener::onResponse, listener::onFailure); } /**