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 11169e2cca..f51e693ac9 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 @@ -289,19 +289,12 @@ public void testRawAccess_allAccessUser() { api.assertDirectPostSearch(searchByNamePayload("sample"), FULL_ACCESS_USER, HttpStatus.SC_OK, 1, "sample"); 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_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); - - // can update or delete admin resource, since system index protection is disabled and user has direct index access. - api.assertDirectUpdate(adminResId, FULL_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_OK); - api.assertDirectDelete(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK); } @Test 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 143872b7a0..321b32338f 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.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.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()) { @@ -73,7 +71,7 @@ private void updateResource(UpdateResourceRequest request, ActionListener { + pluginClient.index(ir, ActionListener.wrap(updateResponse -> { listener.onResponse( new CreateResourceResponse("Resource " + request.getResource().getName() + " updated successfully.") ); diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index 3758b3a5cf..8983a19833 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -18,6 +18,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; @@ -105,7 +106,23 @@ 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())) { log.debug("Request id is blank or null, request is of type {}", docRequest.getClass().getName()); return false;