From f11f242e5615039894b3081b7ba112dc9046e673 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Sep 2025 12:31:12 -0400 Subject: [PATCH 1/5] Also check for DocWriteRequest Signed-off-by: Craig Perkins --- .../opensearch/security/privileges/ResourceAccessEvaluator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index 1984028b77..b92cc91157 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -17,6 +17,7 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.DocRequest; +import org.opensearch.action.DocWriteRequest; import org.opensearch.action.get.GetRequest; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionListener; @@ -101,6 +102,7 @@ public boolean shouldEvaluate(ActionRequest request) { if (!isResourceSharingFeatureEnabled) return false; if (!(request instanceof DocRequest docRequest)) return false; if (request instanceof GetRequest) return false; + if (request instanceof DocWriteRequest) return false; if (Strings.isNullOrEmpty(docRequest.id())) { log.debug("Request id is blank or null, request is of type {}", docRequest.getClass().getName()); return false; From 5f16c1a3998a71ceff284d1e52c980974db4cd8e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Sep 2025 14:16:40 -0400 Subject: [PATCH 2/5] Fix tests Signed-off-by: Craig Perkins --- .../feature/enabled/DirectIndexAccessTests.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java index 6e209d4efe..b7c5107daf 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java @@ -291,17 +291,11 @@ public void testRawAccess_allAccessUser() { api.assertDirectPostSearch(searchByNamePayload("sampleUser"), FULL_ACCESS_USER, HttpStatus.SC_OK, 1, "sampleUser"); // cannot update or delete resource - api.assertDirectUpdate(adminResId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); - api.assertDirectDelete(adminResId, FULL_ACCESS_USER, HttpStatus.SC_FORBIDDEN); + api.assertDirectUpdate(adminResId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_OK); + api.assertDirectDelete(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); // can update and delete own resource api.assertDirectUpdate(userResId, FULL_ACCESS_USER, "sampleUpdateUser", HttpStatus.SC_OK); api.assertDirectDelete(userResId, FULL_ACCESS_USER, HttpStatus.SC_OK); - - // can view, share, revoke and delete resource sharing record(s) directly - api.assertDirectViewSharingRecord(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); - api.assertDirectShare(adminResId, FULL_ACCESS_USER, NO_ACCESS_USER, SAMPLE_FULL_ACCESS_RESOURCE_AG, HttpStatus.SC_OK); - api.assertDirectRevoke(adminResId, FULL_ACCESS_USER, NO_ACCESS_USER, SAMPLE_FULL_ACCESS_RESOURCE_AG, HttpStatus.SC_OK); - api.assertDirectDeleteResourceSharingRecord(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); } @Test From 004f07b566f044a32befb5abf1a7d89a5449e4ad Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Sep 2025 16:00:17 -0400 Subject: [PATCH 3/5] Add comment explaining rationale Signed-off-by: Craig Perkins --- .../privileges/ResourceAccessEvaluator.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index b92cc91157..cfda755b35 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -101,6 +101,21 @@ public boolean shouldEvaluate(ActionRequest request) { ); if (!isResourceSharingFeatureEnabled) return false; if (!(request instanceof DocRequest docRequest)) return false; + /** + * Authorization notes: + * + * - Treat {@link GetRequest} and all {@link DocWriteRequest} types as standard *index actions*. + * They should NOT be evaluated by {@code ResourceAccessEvaluator}. + * + * - {@code ResourceAccessEvaluator} is for higher-level transport actions that operate on a + * single shareable resource. Those actions may perform plugin/system-level index operations + * against the system (resource) index that stores resource metadata. Such accesses must be + * evaluated by {@code SystemIndexAccessEvaluator}. + * + * - {@link DocWriteRequest} is the abstract base for write requests + * ({@link IndexRequest}, {@link UpdateRequest}, {@link DeleteRequest}) and may appear as items + * in a {@code _bulk} request. + */ if (request instanceof GetRequest) return false; if (request instanceof DocWriteRequest) return false; if (Strings.isNullOrEmpty(docRequest.id())) { From 0211607b66cc730ab6ff2f1d5df2331032d78bac Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Sep 2025 16:11:52 -0400 Subject: [PATCH 4/5] Use pluginClient Signed-off-by: Craig Perkins --- .../CreateResourceTransportAction.java | 29 ++++++++----------- .../DeleteResourceTransportAction.java | 16 ++++------ .../UpdateResourceTransportAction.java | 14 ++++----- 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/CreateResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/CreateResourceTransportAction.java index ee6c79467f..7202a2d098 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/CreateResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/CreateResourceTransportAction.java @@ -32,9 +32,9 @@ import org.opensearch.sample.resource.actions.rest.create.CreateResourceAction; import org.opensearch.sample.resource.actions.rest.create.CreateResourceRequest; import org.opensearch.sample.resource.actions.rest.create.CreateResourceResponse; +import org.opensearch.sample.utils.PluginClient; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import org.opensearch.transport.client.Client; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -45,13 +45,13 @@ public class CreateResourceTransportAction extends HandledTransportAction listener) { @@ -88,9 +83,9 @@ private void createResource(CreateResourceRequest request, User user, ActionList } // 2. Ensure index exists with mapping, then index the doc - ensureIndexWithMapping(nodeClient, mappingJson, ActionListener.wrap(v -> { + ensureIndexWithMapping(pluginClient, mappingJson, ActionListener.wrap(v -> { try (XContentBuilder builder = org.opensearch.common.xcontent.XContentFactory.jsonBuilder()) { - IndexRequest ir = nodeClient.prepareIndex(RESOURCE_INDEX_NAME) + IndexRequest ir = pluginClient.prepareIndex(RESOURCE_INDEX_NAME) .setWaitForActiveShards(1) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .setSource(sample.toXContent(builder, ToXContent.EMPTY_PARAMS)) @@ -98,7 +93,7 @@ private void createResource(CreateResourceRequest request, User user, ActionList log.debug("Index Request: {}", ir); - nodeClient.index(ir, ActionListener.wrap(idxResponse -> { + pluginClient.index(ir, ActionListener.wrap(idxResponse -> { log.debug("Created resource: {}", idxResponse.getId()); listener.onResponse(new CreateResourceResponse("Created resource: " + idxResponse.getId())); }, listener::onFailure)); @@ -113,12 +108,12 @@ private void createResource(CreateResourceRequest request, User user, ActionList * - If the index does not exist: creates it with the mapping. * - If the index exists: updates (puts) the mapping. */ - private void ensureIndexWithMapping(Client client, String mappingJson, ActionListener listener) { + private void ensureIndexWithMapping(PluginClient pluginClient, String mappingJson, ActionListener listener) { String indexName = RESOURCE_INDEX_NAME; - client.admin().indices().prepareExists(indexName).execute(ActionListener.wrap(existsResp -> { + pluginClient.admin().indices().prepareExists(indexName).execute(ActionListener.wrap(existsResp -> { if (!existsResp.isExists()) { // Create index with mapping - client.admin().indices().prepareCreate(indexName).setMapping(mappingJson).execute(ActionListener.wrap(createResp -> { + pluginClient.admin().indices().prepareCreate(indexName).setMapping(mappingJson).execute(ActionListener.wrap(createResp -> { if (!createResp.isAcknowledged()) { listener.onFailure(new IllegalStateException("CreateIndex not acknowledged for " + indexName)); return; @@ -127,7 +122,7 @@ private void ensureIndexWithMapping(Client client, String mappingJson, ActionLis }, listener::onFailure)); } else { // Update mapping on existing index - client.admin() + pluginClient.admin() .indices() .preparePutMapping(indexName) .setSource(mappingJson, XContentType.JSON) diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java index f5abc3c46f..78028d775a 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/DeleteResourceTransportAction.java @@ -19,14 +19,13 @@ import org.opensearch.action.support.HandledTransportAction; import org.opensearch.action.support.WriteRequest; import org.opensearch.common.inject.Inject; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; import org.opensearch.sample.resource.actions.rest.delete.DeleteResourceAction; import org.opensearch.sample.resource.actions.rest.delete.DeleteResourceRequest; import org.opensearch.sample.resource.actions.rest.delete.DeleteResourceResponse; +import org.opensearch.sample.utils.PluginClient; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -37,13 +36,13 @@ public class DeleteResourceTransportAction extends HandledTransportAction listener) { @@ -75,7 +71,7 @@ private void deleteResource(String resourceId, ActionListener li WriteRequest.RefreshPolicy.IMMEDIATE ); - nodeClient.delete(deleteRequest, listener); + pluginClient.delete(deleteRequest, listener); } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java index a9b74b3c4e..38319fd928 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/UpdateResourceTransportAction.java @@ -16,7 +16,6 @@ import org.opensearch.action.support.WriteRequest; import org.opensearch.action.update.UpdateRequest; import org.opensearch.common.inject.Inject; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; @@ -24,9 +23,9 @@ import org.opensearch.sample.resource.actions.rest.create.CreateResourceResponse; import org.opensearch.sample.resource.actions.rest.create.UpdateResourceAction; import org.opensearch.sample.resource.actions.rest.create.UpdateResourceRequest; +import org.opensearch.sample.utils.PluginClient; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.common.xcontent.XContentFactory.jsonBuilder; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -38,13 +37,13 @@ public class UpdateResourceTransportAction extends HandledTransportAction listener) { - ThreadContext threadContext = this.transportService.getThreadPool().getThreadContext(); - try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { + try { String resourceId = request.getResourceId(); SampleResource sample = request.getResource(); try (XContentBuilder builder = jsonBuilder()) { @@ -68,7 +66,7 @@ private void updateResource(UpdateResourceRequest request, ActionListener { + pluginClient.update(ur, ActionListener.wrap(updateResponse -> { listener.onResponse( new CreateResourceResponse("Resource " + request.getResource().getName() + " updated successfully.") ); From 654c3f18a5dbb2808bd21ed90fbc72698ae1779e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 26 Sep 2025 16:16:30 -0400 Subject: [PATCH 5/5] Comment out for test Signed-off-by: Craig Perkins --- .../opensearch/security/privileges/ResourceAccessEvaluator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index cfda755b35..4d805438c1 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -117,7 +117,7 @@ public boolean shouldEvaluate(ActionRequest request) { * in a {@code _bulk} request. */ if (request instanceof GetRequest) return false; - if (request instanceof DocWriteRequest) return false; + // if (request instanceof DocWriteRequest) return false; if (Strings.isNullOrEmpty(docRequest.id())) { log.debug("Request id is blank or null, request is of type {}", docRequest.getClass().getName()); return false;