From dc06d08fb0f8fe7f6d4175950ef8cc460d3f8f33 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 21 Oct 2025 21:31:36 -0400 Subject: [PATCH 01/17] WIP on Add Resource to Group Signed-off-by: Craig Perkins --- .../sample/SampleResourcePlugin.java | 3 + .../rest/add/AddResourceToGroupAction.java | 29 +++++++ .../rest/add/AddResourceToGroupRequest.java | 87 +++++++++++++++++++ .../rest/add/AddResourceToGroupResponse.java | 55 ++++++++++++ .../add/AddResourceToGroupRestAction.java | 63 ++++++++++++++ .../AddResourceToGroupTransportAction.java | 79 +++++++++++++++++ 6 files changed, 316 insertions(+) create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupAction.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java create mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java index 3cddcaeefd..d96076a93f 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java @@ -50,6 +50,7 @@ import org.opensearch.sample.resource.actions.transport.GetResourceTransportAction; import org.opensearch.sample.resource.actions.transport.SearchResourceTransportAction; import org.opensearch.sample.resource.actions.transport.UpdateResourceTransportAction; +import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.create.UpdateResourceGroupAction; @@ -59,6 +60,7 @@ import org.opensearch.sample.resourcegroup.actions.rest.get.GetResourceGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.search.SearchResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.search.SearchResourceGroupRestAction; +import org.opensearch.sample.resourcegroup.actions.transport.AddResourceToGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.CreateResourceGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.DeleteResourceGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.GetResourceGroupTransportAction; @@ -141,6 +143,7 @@ public List getRestHandlers( actions.add(new ActionHandler<>(UpdateResourceGroupAction.INSTANCE, UpdateResourceGroupTransportAction.class)); actions.add(new ActionHandler<>(DeleteResourceGroupAction.INSTANCE, DeleteResourceGroupTransportAction.class)); actions.add(new ActionHandler<>(SearchResourceGroupAction.INSTANCE, SearchResourceGroupTransportAction.class)); + actions.add(new ActionHandler<>(AddResourceToGroupAction.INSTANCE, AddResourceToGroupTransportAction.class)); actions.add(new ActionHandler<>(SecurePluginAction.INSTANCE, SecurePluginTransportAction.class)); return actions; } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupAction.java new file mode 100644 index 0000000000..298ad0a752 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupAction.java @@ -0,0 +1,29 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup.actions.rest.add; + +import org.opensearch.action.ActionType; + +/** + * Action to add a sample resource to a resource group + */ +public class AddResourceToGroupAction extends ActionType { + /** + * Add sample resource to group action instance + */ + public static final AddResourceToGroupAction INSTANCE = new AddResourceToGroupAction(); + /** + * Add sample resource to group action name + */ + public static final String NAME = "cluster:admin/sample-resource-plugin/group/add"; + + private AddResourceToGroupAction() { + super(NAME, AddResourceToGroupResponse::new); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java new file mode 100644 index 0000000000..c09f28d8d8 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java @@ -0,0 +1,87 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup.actions.rest.add; + +import java.io.IOException; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.DocRequest; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; + +import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; +import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; + +/** + * Request object for AddResourceToGroupRequest transport action + */ +public class AddResourceToGroupRequest extends ActionRequest implements DocRequest.ParentReferencing { + + private final String groupId; + private final String resourceId; + + /** + * Default constructor + */ + public AddResourceToGroupRequest(String groupId, String resourceId) { + this.groupId = groupId; + this.resourceId = resourceId; + } + + public AddResourceToGroupRequest(StreamInput in) throws IOException { + this.groupId = in.readString(); + this.resourceId = in.readString(); + } + + @Override + public void writeTo(final StreamOutput out) throws IOException { + out.writeString(groupId); + out.writeString(resourceId); + } + + @Override + public ActionRequestValidationException validate() { + return null; + } + + public String getResourceId() { + return this.resourceId; + } + + public String getGroupId() { + return this.groupId; + } + + @Override + public String type() { + return RESOURCE_TYPE; + } + + @Override + public String index() { + return RESOURCE_INDEX_NAME; + } + + @Override + public String id() { + return resourceId; + } + + @Override + public String parentType() { + return RESOURCE_GROUP_TYPE; + } + + @Override + public String parentId() { + return groupId; + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java new file mode 100644 index 0000000000..36833b3af3 --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java @@ -0,0 +1,55 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup.actions.rest.add; + +import java.io.IOException; + +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.core.xcontent.ToXContentObject; +import org.opensearch.core.xcontent.XContentBuilder; + +/** + * Response to a AddSampleResourceToGroupRequest + */ +public class AddResourceToGroupResponse extends ActionResponse implements ToXContentObject { + private final String message; + + /** + * Default constructor + * + * @param message The message + */ + public AddResourceToGroupResponse(String message) { + this.message = message; + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeString(message); + } + + /** + * Constructor with StreamInput + * + * @param in the stream input + */ + public AddResourceToGroupResponse(final StreamInput in) throws IOException { + message = in.readString(); + } + + @Override + public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { + builder.startObject(); + builder.field("message", message); + builder.endObject(); + return builder; + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java new file mode 100644 index 0000000000..d8194296ac --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java @@ -0,0 +1,63 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup.actions.rest.add; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.rest.BaseRestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.rest.action.RestToXContentListener; +import org.opensearch.transport.client.node.NodeClient; + +import static org.opensearch.rest.RestRequest.Method.POST; +import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; + +/** + * Rest Action to add a Sample Resource to a Group. + */ +public class AddResourceToGroupRestAction extends BaseRestHandler { + + public AddResourceToGroupRestAction() {} + + @Override + public List routes() { + return List.of(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/group/add/{group_id}")); + } + + @Override + public String getName() { + return "add_sample_resource_to_group"; + } + + @Override + protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { + Map source; + try (XContentParser parser = request.contentParser()) { + source = parser.map(); + } + + return switch (request.method()) { + case POST -> addResourceToGroup(source, request.param("group_id"), client); + default -> throw new IllegalArgumentException("Illegal method: " + request.method()); + }; + } + + private RestChannelConsumer addResourceToGroup(Map source, String groupId, NodeClient client) throws IOException { + String resourceId = (String) source.get("resource_id"); + final AddResourceToGroupRequest addResourceToGroupRequest = new AddResourceToGroupRequest(groupId, resourceId); + return channel -> client.executeLocally( + AddResourceToGroupAction.INSTANCE, + addResourceToGroupRequest, + new RestToXContentListener<>(channel) + ); + } +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java new file mode 100644 index 0000000000..c984b5860a --- /dev/null +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java @@ -0,0 +1,79 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup.actions.transport; + +import java.util.Map; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.action.support.ActionFilters; +import org.opensearch.action.support.HandledTransportAction; +import org.opensearch.action.support.WriteRequest; +import org.opensearch.action.update.UpdateRequest; +import org.opensearch.common.inject.Inject; +import org.opensearch.core.action.ActionListener; +import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupAction; +import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupRequest; +import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupResponse; +import org.opensearch.sample.utils.PluginClient; +import org.opensearch.tasks.Task; +import org.opensearch.transport.TransportService; + +import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; + +/** + * Transport action for updating a resource. + */ +public class AddResourceToGroupTransportAction extends HandledTransportAction { + private static final Logger log = LogManager.getLogger(AddResourceToGroupTransportAction.class); + + private final TransportService transportService; + private final PluginClient pluginClient; + + @Inject + public AddResourceToGroupTransportAction(TransportService transportService, ActionFilters actionFilters, PluginClient pluginClient) { + super(AddResourceToGroupAction.NAME, transportService, actionFilters, AddResourceToGroupRequest::new); + this.transportService = transportService; + this.pluginClient = pluginClient; + } + + @Override + protected void doExecute(Task task, AddResourceToGroupRequest request, ActionListener listener) { + if (request.getResourceId() == null || request.getResourceId().isEmpty()) { + listener.onFailure(new IllegalArgumentException("Resource Group ID cannot be null or empty")); + return; + } + // Check permission to resource + addResourceToGroup(request, listener); + } + + private void addResourceToGroup(AddResourceToGroupRequest request, ActionListener listener) { + try { + String resourceId = request.getResourceId(); + String groupId = request.getGroupId(); + // because some plugins seem to treat update API calls as index request + UpdateRequest ur = new UpdateRequest().index(RESOURCE_INDEX_NAME) + .id(resourceId) + .setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL) // WAIT_UNTIL because we don't want tests to fail, as they + // execute search right after update + .upsert(Map.of("parent_id", groupId)); + + log.debug("Update Request: {}", ur.toString()); + + pluginClient.update(ur, ActionListener.wrap(updateResponse -> { + listener.onResponse(new AddResourceToGroupResponse("Resource ID " + resourceId + " added to group ID " + groupId + ".")); + }, listener::onFailure)); + } catch (Exception e) { + log.error("Failed to update resource: {}", request.getResourceId(), e); + listener.onFailure(e); + } + + } +} From 0d99679d6a03481def27c41482fd74bbc926959e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Thu, 23 Oct 2025 22:05:05 -0400 Subject: [PATCH 02/17] WIP on Hierarchy Signed-off-by: Craig Perkins --- .../opensearch/sample/resource/TestUtils.java | 11 ++ .../resourcegroup/ResourceHierarchyTests.java | 87 +++++++++++ .../org/opensearch/sample/SampleResource.java | 1 + .../sample/SampleResourceExtension.java | 28 +++- .../sample/SampleResourceGroup.java | 6 +- .../sample/SampleResourceGroupExtension.java | 17 ++- .../sample/SampleResourcePlugin.java | 2 + .../transport/GetResourceTransportAction.java | 1 + .../rest/add/AddResourceToGroupRequest.java | 15 +- .../AddResourceToGroupTransportAction.java | 3 +- .../src/main/resources/mappings.json | 3 + .../spi/resources/ResourceProvider.java | 33 ++++- .../security/OpenSearchSecurityPlugin.java | 10 +- .../resources/ResourceActionGroupsHelper.java | 6 +- .../resources/ResourceIndexListener.java | 4 + .../resources/ResourcePluginInfo.java | 139 ++++++++++-------- 16 files changed, 279 insertions(+), 87 deletions(-) create mode 100644 sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index f61440b288..e3a8837073 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -96,6 +96,8 @@ public final class TestUtils { public static final String SAMPLE_RESOURCE_GROUP_DELETE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/delete"; public static final String SAMPLE_RESOURCE_GROUP_SEARCH_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/search"; + public static final String SAMPLE_ADD_RESOURCE_TO_GROUP_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/add"; + public static final String RESOURCE_SHARING_MIGRATION_ENDPOINT = "_plugins/_security/api/resources/migrate"; public static final String SECURITY_SHARE_ENDPOINT = "_plugins/_security/api/resource/share"; public static final String SECURITY_TYPES_ENDPOINT = "_plugins/_security/api/resource/types"; @@ -351,6 +353,15 @@ public String createSampleResourceAs(TestSecurityConfig.User user, Header... hea } } + public String addResourceToGroup(TestSecurityConfig.User user, String resourceId, String groupId, Header... headers) { + try (TestRestClient client = cluster.getRestClient(user)) { + String payload = "{\"resource_id\":\"" + resourceId + "\"}"; + TestRestClient.HttpResponse resp = client.postJson(SAMPLE_ADD_RESOURCE_TO_GROUP_ENDPOINT + "/" + groupId, payload, headers); + resp.assertStatusCode(HttpStatus.SC_OK); + return resp.getTextFromJsonBody("/message").trim(); + } + } + public String createSampleResourceGroupAs(TestSecurityConfig.User user, Header... headers) { try (TestRestClient client = cluster.getRestClient(user)) { String sample = "{\"name\":\"samplegroup\"}"; diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java new file mode 100644 index 0000000000..aaee0c6bc6 --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java @@ -0,0 +1,87 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resourcegroup; + +import com.carrotsearch.randomizedtesting.RandomizedRunner; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.sample.resource.TestUtils; +import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.opensearch.sample.resource.TestUtils.FULL_ACCESS_USER; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_FULL_ACCESS; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_READ_ONLY; +import static org.opensearch.sample.resource.TestUtils.newCluster; +import static org.opensearch.security.api.AbstractApiIntegrationTest.forbidden; +import static org.opensearch.security.api.AbstractApiIntegrationTest.ok; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +/** + * Test resource access to a resource shared with mixed access-levels. Some users are shared at read_only, others at full_access. + * All tests are against USER_ADMIN's resource created during setup. + */ +@RunWith(RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class ResourceHierarchyTests { + + @ClassRule + public static LocalCluster cluster = newCluster(true, true); + + private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster); + private String resourceGroupId; + private String resourceId; + + @Before + public void setup() { + resourceGroupId = api.createSampleResourceGroupAs(USER_ADMIN); + api.awaitSharingEntry(resourceGroupId); // wait until sharing entry is created + resourceId = api.createSampleResourceAs(USER_ADMIN); + api.awaitSharingEntry(resourceId); // wait until sharing entry is created + api.addResourceToGroup(USER_ADMIN, resourceId, resourceGroupId); + } + + @After + public void cleanup() { + api.wipeOutResourceEntries(); + } + + private void assertNoAccessBeforeSharing(TestSecurityConfig.User user) throws Exception { + forbidden(() -> api.getResourceGroup(resourceGroupId, user)); + forbidden(() -> api.updateResourceGroup(resourceGroupId, user, "sampleUpdateAdmin")); + forbidden(() -> api.deleteResourceGroup(resourceGroupId, user)); + + forbidden(() -> api.shareResourceGroup(resourceGroupId, user, user, SAMPLE_GROUP_FULL_ACCESS)); + forbidden(() -> api.revokeResourceGroup(resourceGroupId, user, user, SAMPLE_GROUP_FULL_ACCESS)); + } + + @Test + public void testShouldHaveAccessToResourceWithGroupLevelAccess() throws Exception { + TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, USER_ADMIN)); + assertThat(response.getBody(), containsString("sample")); + + forbidden(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); + forbidden(() -> api.getResource(resourceGroupId, FULL_ACCESS_USER)); + + // 1. share at read-only for full-access user and at full-access for limited-perms user + ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); + + ok(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); + ok(() -> api.getResource(resourceGroupId, FULL_ACCESS_USER)); + } + +} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java index 63f1a20474..a11b760f43 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java @@ -82,6 +82,7 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par .field("description", description) .field("attributes", attributes) .field("user", user) + .field("resource_type", RESOURCE_TYPE) .endObject(); } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceExtension.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceExtension.java index 18fddfdff4..7678589f90 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceExtension.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceExtension.java @@ -18,6 +18,7 @@ import org.opensearch.security.spi.resources.ResourceSharingExtension; import org.opensearch.security.spi.resources.client.ResourceSharingClient; +import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; @@ -28,7 +29,32 @@ public class SampleResourceExtension implements ResourceSharingExtension { @Override public Set getResourceProviders() { - return Set.of(new ResourceProvider(RESOURCE_TYPE, RESOURCE_INDEX_NAME)); + return Set.of(new ResourceProvider() { + @Override + public String resourceType() { + return RESOURCE_TYPE; + } + + @Override + public String resourceIndexName() { + return RESOURCE_INDEX_NAME; + } + + @Override + public String typeField() { + return "resource_type"; + } + + @Override + public String parentType() { + return RESOURCE_GROUP_TYPE; + } + + @Override + public String parentIdField() { + return "group_id"; + } + }); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroup.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroup.java index fd566af87e..05f7e16156 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroup.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroup.java @@ -70,7 +70,11 @@ public static SampleResourceGroup fromXContent(XContentParser parser) throws IOE } public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject().field("name", name).field("description", description).endObject(); + return builder.startObject() + .field("name", name) + .field("description", description) + .field("resource_type", RESOURCE_GROUP_TYPE) + .endObject(); } public void writeTo(StreamOutput out) throws IOException { diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroupExtension.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroupExtension.java index 822ae612e7..77fb9753f0 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroupExtension.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourceGroupExtension.java @@ -17,7 +17,22 @@ public class SampleResourceGroupExtension implements ResourceSharingExtension { @Override public Set getResourceProviders() { - return Set.of(new ResourceProvider(RESOURCE_GROUP_TYPE, RESOURCE_INDEX_NAME)); + return Set.of(new ResourceProvider() { + @Override + public String resourceType() { + return RESOURCE_GROUP_TYPE; + } + + @Override + public String resourceIndexName() { + return RESOURCE_INDEX_NAME; + } + + @Override + public String typeField() { + return "resource_type"; + } + }); } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java index d96076a93f..e84b5f561a 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java @@ -51,6 +51,7 @@ import org.opensearch.sample.resource.actions.transport.SearchResourceTransportAction; import org.opensearch.sample.resource.actions.transport.UpdateResourceTransportAction; import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupAction; +import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.create.UpdateResourceGroupAction; @@ -125,6 +126,7 @@ public List getRestHandlers( handlers.add(new GetResourceGroupRestAction()); handlers.add(new DeleteResourceGroupRestAction()); handlers.add(new SearchResourceGroupRestAction()); + handlers.add(new AddResourceToGroupRestAction()); handlers.add(new SecurePluginRestAction()); return handlers; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java index 126503faf8..1b0c9c7528 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java @@ -87,6 +87,7 @@ private void fetchResourceById(String resourceId, ActionListener resourceSharingExtensions = new HashSet<>(); - // CS-ENFORCE-SINGLE public static boolean isActionTraceEnabled() { @@ -369,10 +366,9 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) // dynamic settings transportPassiveAuthSetting = new TransportPassiveAuthSetting(settings); - resourceSharingEnabledSetting = new ResourceSharingFeatureFlagSetting(settings, resourcePluginInfo); // not filtered - resourceSharingProtectedResourceTypesSetting = new ResourceSharingProtectedResourcesSetting(settings, resourcePluginInfo); // not - // filtered - resourcePluginInfo.setResourceSharingProtectedTypesSetting(resourceSharingProtectedResourceTypesSetting); + resourceSharingEnabledSetting = new ResourceSharingFeatureFlagSetting(settings, resourcePluginInfo); + resourceSharingProtectedResourceTypesSetting = new ResourceSharingProtectedResourcesSetting(settings, resourcePluginInfo); + resourcePluginInfo.setProtectedTypesSetting(resourceSharingProtectedResourceTypesSetting); if (disabled) { this.sslCertReloadEnabled = false; diff --git a/src/main/java/org/opensearch/security/resources/ResourceActionGroupsHelper.java b/src/main/java/org/opensearch/security/resources/ResourceActionGroupsHelper.java index d3b92d85c4..dc3c60c2ba 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceActionGroupsHelper.java +++ b/src/main/java/org/opensearch/security/resources/ResourceActionGroupsHelper.java @@ -15,7 +15,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.security.securityconf.FlattenedActionGroups; import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; @@ -89,11 +88,8 @@ public static void loadActionGroupsConfig(ResourcePluginInfo resourcePluginInfo) || e.getValue().getAllowed_actions().isEmpty() ); - FlattenedActionGroups flattened = new FlattenedActionGroups(cfg); - // Publish to ResourcePluginInfo → used by UI and authZ - resourcePluginInfo.registerActionGroupNames(resType, cfg.getCEntries().keySet()); - resourcePluginInfo.registerFlattened(resType, flattened); + resourcePluginInfo.registerAccessLevels(resType, cfg); log.info("Registered {} action-groups for {}", cfg.getCEntries().size(), resType); } diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index b0c67de13d..4a870be35d 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -71,6 +71,10 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re return; } + String resourceType = resourcePluginInfo.getResourceTypeForIndexOp(resourceIndex, index); + + // TODO Complete this + log.debug("postIndex called on {}", resourceIndex); String resourceId = index.id(); diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index a44287bbfe..470fc41b3f 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -10,25 +10,28 @@ // CS-SUPPRESS-SINGLE: RegexpSingleline get Resource Sharing Extensions import java.io.IOException; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.stream.Collectors; import com.google.common.collect.ImmutableSet; +import org.apache.lucene.index.IndexableField; import org.opensearch.OpenSearchSecurityException; import org.opensearch.core.xcontent.ToXContentObject; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.index.engine.Engine; import org.opensearch.security.securityconf.FlattenedActionGroups; +import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; +import org.opensearch.security.securityconf.impl.v7.ActionGroupsV7; import org.opensearch.security.setting.OpensearchDynamicSetting; +import org.opensearch.security.spi.resources.ResourceProvider; import org.opensearch.security.spi.resources.ResourceSharingExtension; import org.opensearch.security.spi.resources.client.ResourceSharingClient; @@ -42,33 +45,30 @@ public class ResourcePluginInfo { private ResourceSharingClient resourceAccessControlClient; - private OpensearchDynamicSetting> resourceSharingProtectedTypesSetting; + private OpensearchDynamicSetting> protectedTypesSetting; private final Set resourceSharingExtensions = new HashSet<>(); - // type <-> index - private final Map typeToIndex = new HashMap<>(); - // UI: action-group *names* per type - private final Map> typeToGroupNames = new HashMap<>(); + private final Map> typeToAccessLevels = new HashMap<>(); // AuthZ: resolved (flattened) groups per type private final Map typeToFlattened = new HashMap<>(); + private final Map typeToProvider = new HashMap<>(); + // cache current protected types and their indices private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); // make the updates/reads thread-safe - private Set currentProtectedTypes = Collections.emptySet(); // snapshot of last set - private Set cachedProtectedTypeIndices = Collections.emptySet(); // precomputed indices - public void setResourceSharingProtectedTypesSetting(OpensearchDynamicSetting> resourceSharingProtectedTypesSetting) { - this.resourceSharingProtectedTypesSetting = resourceSharingProtectedTypesSetting; + public void setProtectedTypesSetting(OpensearchDynamicSetting> protectedTypesSetting) { + this.protectedTypesSetting = protectedTypesSetting; } public void setResourceSharingExtensions(Set extensions) { lock.writeLock().lock(); try { resourceSharingExtensions.clear(); - typeToIndex.clear(); + typeToProvider.clear(); // Enforce resource-type unique-ness Set resourceTypes = new HashSet<>(); @@ -78,7 +78,7 @@ public void setResourceSharingExtensions(Set extension // add name seen so far to the resource-types set resourceTypes.add(rp.resourceType()); // also cache type->index and index->type mapping - typeToIndex.put(rp.resourceType(), rp.resourceIndexName()); + typeToProvider.put(rp.resourceType(), rp); } else { throw new OpenSearchSecurityException( String.format( @@ -91,10 +91,6 @@ public void setResourceSharingExtensions(Set extension } } resourceSharingExtensions.addAll(extensions); - - // Whenever providers change, invalidate protected caches so next update refreshes them - currentProtectedTypes = Collections.emptySet(); - cachedProtectedTypeIndices = Collections.emptySet(); } finally { lock.writeLock().unlock(); } @@ -104,35 +100,56 @@ public void updateProtectedTypes(List protectedTypes) { lock.writeLock().lock(); try { // Rebuild mappings based on the current allowlist - typeToIndex.clear(); + typeToProvider.clear(); if (protectedTypes == null || protectedTypes.isEmpty()) { // No protected types -> leave maps empty - currentProtectedTypes = Collections.emptySet(); - cachedProtectedTypeIndices = Collections.emptySet(); return; } - // Cache current protected set as an unmodifiable snapshot - currentProtectedTypes = Collections.unmodifiableSet(new LinkedHashSet<>(protectedTypes)); - for (ResourceSharingExtension extension : resourceSharingExtensions) { for (var rp : extension.getResourceProviders()) { final String type = rp.resourceType(); - if (!currentProtectedTypes.contains(type)) continue; + if (!protectedTypes.contains(type)) continue; - final String index = rp.resourceIndexName(); - typeToIndex.put(type, index); + typeToProvider.put(type, rp); } } - - // pre-compute indices for current protected set - cachedProtectedTypeIndices = Collections.unmodifiableSet(new LinkedHashSet<>(typeToIndex.values())); } finally { lock.writeLock().unlock(); } } + public String getResourceTypeForIndexOp(String resourceIndex, Engine.Index indexOp) { + lock.readLock().lock(); + try { + // Eagerly use type field from provider of same index as the indexOp + // If typeField is present, assume single resource type per index + for (var entry : typeToProvider.entrySet()) { + if (entry.getValue().resourceIndexName().equals(resourceIndex)) { + if (entry.getValue().typeField() != null) { + String resourceType = null; + for (IndexableField f : indexOp.parsedDoc().rootDoc().getFields(entry.getValue().typeField())) { + if (f.stringValue() != null) { + resourceType = f.stringValue(); + break; + } + if (f.binaryValue() != null) { // e.g., BytesRef-backed + resourceType = f.binaryValue().utf8ToString(); + break; + } + } + return resourceType; + } + return entry.getValue().typeField(); + } + } + return null; + } finally { + lock.readLock().unlock(); + } + } + public Set getResourceSharingExtensions() { return ImmutableSet.copyOf(resourceSharingExtensions); } @@ -146,64 +163,67 @@ public ResourceSharingClient getResourceAccessControlClient() { } /** Register/merge action-group names for a given resource type. */ - public record ResourceDashboardInfo(String resourceType, Set actionGroups // names only (for UI) + public record ResourceDashboardInfo(String resourceType, Set accessLevels // names only (for UI) ) implements ToXContentObject { @Override public XContentBuilder toXContent(XContentBuilder b, Params p) throws IOException { b.startObject(); b.field("type", resourceType); - b.field("action_groups", actionGroups == null ? Collections.emptyList() : actionGroups); + b.field("access_levels", accessLevels == null ? Collections.emptyList() : accessLevels); return b.endObject(); } } - public void registerActionGroupNames(String resourceType, Collection names) { - if (resourceType == null || names == null) return; + public void registerAccessLevels(String resourceType, SecurityDynamicConfiguration accessLevels) { + if (resourceType == null || accessLevels == null) return; lock.writeLock().lock(); try { - typeToGroupNames.computeIfAbsent(resourceType, k -> new LinkedHashSet<>()) - .addAll(names.stream().filter(Objects::nonNull).map(String::trim).filter(s -> !s.isEmpty()).toList()); + FlattenedActionGroups flattened = new FlattenedActionGroups(accessLevels); + typeToFlattened.put(resourceType, flattened); + typeToAccessLevels.computeIfAbsent(resourceType, k -> new LinkedHashSet<>()) + .addAll(accessLevels.getCEntries().keySet().stream().toList()); } finally { lock.writeLock().unlock(); } } - public void registerFlattened(String resourceType, FlattenedActionGroups flattened) { - if (resourceType == null || flattened == null) return; - lock.writeLock().lock(); + public FlattenedActionGroups flattenedForType(String resourceType) { + lock.readLock().lock(); try { - typeToFlattened.put(resourceType, flattened); + return typeToFlattened.getOrDefault(resourceType, FlattenedActionGroups.EMPTY); } finally { - lock.writeLock().unlock(); + lock.readLock().unlock(); } } - public FlattenedActionGroups flattenedForType(String resourceType) { + public String indexByType(String type) { lock.readLock().lock(); try { - return typeToFlattened.getOrDefault(resourceType, FlattenedActionGroups.EMPTY); + return typeToProvider.get(type).resourceIndexName(); } finally { lock.readLock().unlock(); } } - public String indexByType(String type) { + public String getParentIdField(String resourceType) { lock.readLock().lock(); try { - return typeToIndex.get(type); + if (!typeToProvider.containsKey(resourceType)) { + return null; + } + return typeToProvider.get(resourceType).parentIdField(); } finally { lock.readLock().unlock(); } } - public Set typesByIndex(String index) { + public String getParentType(String resourceType) { lock.readLock().lock(); try { - return typeToIndex.entrySet() - .stream() - .filter(entry -> Objects.equals(entry.getValue(), index)) - .map(Map.Entry::getKey) - .collect(Collectors.toSet()); + if (!typeToProvider.containsKey(resourceType)) { + return null; + } + return typeToProvider.get(resourceType).parentType(); } finally { lock.readLock().unlock(); } @@ -212,11 +232,9 @@ public Set typesByIndex(String index) { public Set getResourceTypes() { lock.readLock().lock(); try { - return typeToIndex.keySet() + return typeToProvider.keySet() .stream() - .map( - s -> new ResourceDashboardInfo(s, Collections.unmodifiableSet(typeToGroupNames.getOrDefault(s, new LinkedHashSet<>()))) - ) + .map(s -> new ResourceDashboardInfo(s, typeToAccessLevels.get(s))) .collect(Collectors.toCollection(LinkedHashSet::new)); } finally { lock.readLock().unlock(); @@ -226,29 +244,24 @@ public Set getResourceTypes() { public Set getResourceIndices() { lock.readLock().lock(); try { - return new HashSet<>(typeToIndex.values()); + return typeToProvider.values().stream().map(ResourceProvider::resourceIndexName).collect(Collectors.toSet()); } finally { lock.readLock().unlock(); } } public Set getResourceIndicesForProtectedTypes() { - List resourceTypes = this.resourceSharingProtectedTypesSetting.getDynamicSettingValue(); + List resourceTypes = this.protectedTypesSetting.getDynamicSettingValue(); if (resourceTypes == null || resourceTypes.isEmpty()) { return Collections.emptySet(); } lock.readLock().lock(); try { - // If caller is asking for the current protected set, return the cache - if (new LinkedHashSet<>(resourceTypes).equals(currentProtectedTypes)) { - return cachedProtectedTypeIndices; - } - - return typeToIndex.entrySet() + return typeToProvider.entrySet() .stream() .filter(e -> resourceTypes.contains(e.getKey())) - .map(Map.Entry::getValue) + .map(e -> e.getValue().resourceIndexName()) .collect(Collectors.toSet()); } finally { lock.readLock().unlock(); From 080fbe7f7a749ef839206976ec4519ea1e58d4de Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 24 Oct 2025 16:52:24 -0400 Subject: [PATCH 03/17] Store referential information in ResourceSharing document Signed-off-by: Craig Perkins --- .../opensearch/sample/resource/TestUtils.java | 8 +- .../resourcegroup/ResourceHierarchyTests.java | 3 +- .../org/opensearch/sample/SampleResource.java | 22 ++- .../sample/SampleResourcePlugin.java | 6 - .../rest/create/CreateResourceRestAction.java | 4 + .../transport/GetResourceTransportAction.java | 1 - .../rest/add/AddResourceToGroupAction.java | 29 ---- .../rest/add/AddResourceToGroupRequest.java | 88 ---------- .../rest/add/AddResourceToGroupResponse.java | 55 ------- .../add/AddResourceToGroupRestAction.java | 63 -------- .../create/CreateResourceGroupRequest.java | 16 +- .../create/CreateResourceGroupRestAction.java | 28 ++-- .../create/UpdateResourceGroupRequest.java | 16 +- .../AddResourceToGroupTransportAction.java | 80 ---------- .../CreateResourceGroupTransportAction.java | 8 +- .../UpdateResourceGroupTransportAction.java | 6 +- .../resources/ResourceIndexListener.java | 22 +-- .../resources/ResourcePluginInfo.java | 60 ++++--- .../ResourceSharingIndexHandler.java | 35 ++-- .../MigrateResourceSharingInfoApiAction.java | 7 +- .../resources/sharing/ResourceSharing.java | 151 +++++++++++++++--- 21 files changed, 263 insertions(+), 445 deletions(-) delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupAction.java delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java delete mode 100644 sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index e3a8837073..d4646119e3 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -353,12 +353,12 @@ public String createSampleResourceAs(TestSecurityConfig.User user, Header... hea } } - public String addResourceToGroup(TestSecurityConfig.User user, String resourceId, String groupId, Header... headers) { + public String createSampleResourceWithGroupAs(TestSecurityConfig.User user, String groupId, Header... headers) { try (TestRestClient client = cluster.getRestClient(user)) { - String payload = "{\"resource_id\":\"" + resourceId + "\"}"; - TestRestClient.HttpResponse resp = client.postJson(SAMPLE_ADD_RESOURCE_TO_GROUP_ENDPOINT + "/" + groupId, payload, headers); + String sample = "{\"group_id\":\"" + groupId + "\", \"name\":\"sample\"}"; + TestRestClient.HttpResponse resp = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sample, headers); resp.assertStatusCode(HttpStatus.SC_OK); - return resp.getTextFromJsonBody("/message").trim(); + return resp.getTextFromJsonBody("/message").split(":")[1].trim(); } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java index aaee0c6bc6..8729d120d8 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java @@ -50,9 +50,8 @@ public class ResourceHierarchyTests { public void setup() { resourceGroupId = api.createSampleResourceGroupAs(USER_ADMIN); api.awaitSharingEntry(resourceGroupId); // wait until sharing entry is created - resourceId = api.createSampleResourceAs(USER_ADMIN); + resourceId = api.createSampleResourceWithGroupAs(USER_ADMIN, resourceGroupId); api.awaitSharingEntry(resourceId); // wait until sharing entry is created - api.addResourceToGroup(USER_ADMIN, resourceId, resourceGroupId); } @After diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java index a11b760f43..6f316e9617 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResource.java @@ -36,6 +36,7 @@ public class SampleResource implements NamedWriteable, ToXContentObject { private String name; private String description; + private String groupId; private Map attributes; // NOTE: following field is added to specifically test migrate API, for newer resources this field must not be defined private User user; @@ -46,7 +47,8 @@ public SampleResource() throws IOException { public SampleResource(StreamInput in) throws IOException { this.name = in.readString(); - this.description = in.readString(); + this.description = in.readOptionalString(); + this.groupId = in.readOptionalString(); this.attributes = in.readMap(StreamInput::readString, StreamInput::readString); this.user = new User(in); } @@ -60,14 +62,18 @@ public SampleResource(StreamInput in) throws IOException { } s.setName((String) a[0]); s.setDescription((String) a[1]); - s.setAttributes((Map) a[2]); - s.setUser((User) a[3]); + s.setGroupId((String) a[2]); + // ignore a[3] as we know the type + s.setAttributes((Map) a[4]); + s.setUser((User) a[5]); return s; }); static { PARSER.declareString(constructorArg(), new ParseField("name")); PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("description")); + PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("group_id")); + PARSER.declareStringOrNull(optionalConstructorArg(), new ParseField("resource_type")); PARSER.declareObjectOrNull(optionalConstructorArg(), (p, c) -> p.mapStrings(), null, new ParseField("attributes")); PARSER.declareObjectOrNull(optionalConstructorArg(), (p, c) -> User.parse(p), null, new ParseField("user")); } @@ -80,15 +86,17 @@ public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params par return builder.startObject() .field("name", name) .field("description", description) + .field("group_id", groupId) + .field("resource_type", RESOURCE_TYPE) .field("attributes", attributes) .field("user", user) - .field("resource_type", RESOURCE_TYPE) .endObject(); } public void writeTo(StreamOutput out) throws IOException { out.writeString(name); - out.writeString(description); + out.writeOptionalString(description); + out.writeOptionalString(groupId); out.writeMap(attributes, StreamOutput::writeString, StreamOutput::writeString); user.writeTo(out); } @@ -101,6 +109,10 @@ public void setDescription(String description) { this.description = description; } + public void setGroupId(String groupId) { + this.groupId = groupId; + } + public void setAttributes(Map attributes) { this.attributes = attributes; } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java index e84b5f561a..5062bb0948 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java @@ -50,8 +50,6 @@ import org.opensearch.sample.resource.actions.transport.GetResourceTransportAction; import org.opensearch.sample.resource.actions.transport.SearchResourceTransportAction; import org.opensearch.sample.resource.actions.transport.UpdateResourceTransportAction; -import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupAction; -import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.create.UpdateResourceGroupAction; @@ -61,7 +59,6 @@ import org.opensearch.sample.resourcegroup.actions.rest.get.GetResourceGroupRestAction; import org.opensearch.sample.resourcegroup.actions.rest.search.SearchResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.search.SearchResourceGroupRestAction; -import org.opensearch.sample.resourcegroup.actions.transport.AddResourceToGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.CreateResourceGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.DeleteResourceGroupTransportAction; import org.opensearch.sample.resourcegroup.actions.transport.GetResourceGroupTransportAction; @@ -126,8 +123,6 @@ public List getRestHandlers( handlers.add(new GetResourceGroupRestAction()); handlers.add(new DeleteResourceGroupRestAction()); handlers.add(new SearchResourceGroupRestAction()); - handlers.add(new AddResourceToGroupRestAction()); - handlers.add(new SecurePluginRestAction()); return handlers; } @@ -145,7 +140,6 @@ public List getRestHandlers( actions.add(new ActionHandler<>(UpdateResourceGroupAction.INSTANCE, UpdateResourceGroupTransportAction.class)); actions.add(new ActionHandler<>(DeleteResourceGroupAction.INSTANCE, DeleteResourceGroupTransportAction.class)); actions.add(new ActionHandler<>(SearchResourceGroupAction.INSTANCE, SearchResourceGroupTransportAction.class)); - actions.add(new ActionHandler<>(AddResourceToGroupAction.INSTANCE, AddResourceToGroupTransportAction.class)); actions.add(new ActionHandler<>(SecurePluginAction.INSTANCE, SecurePluginTransportAction.class)); return actions; } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java index bab6815c4e..884fb263eb 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/rest/create/CreateResourceRestAction.java @@ -60,10 +60,12 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli private RestChannelConsumer updateResource(Map source, String resourceId, NodeClient client) throws IOException { String name = (String) source.get("name"); String description = source.containsKey("description") ? (String) source.get("description") : null; + String groupId = source.containsKey("group_id") ? (String) source.get("group_id") : null; Map attributes = getAttributes(source); SampleResource resource = new SampleResource(); resource.setName(name); resource.setDescription(description); + resource.setGroupId(groupId); resource.setAttributes(attributes); final UpdateResourceRequest updateResourceRequest = new UpdateResourceRequest(resourceId, resource); return channel -> client.executeLocally( @@ -75,10 +77,12 @@ private RestChannelConsumer updateResource(Map source, String re private RestChannelConsumer createResource(Map source, NodeClient client) throws IOException { String name = (String) source.get("name"); + String groupId = source.containsKey("group_id") ? (String) source.get("group_id") : null; String description = source.containsKey("description") ? (String) source.get("description") : null; Map attributes = getAttributes(source); boolean shouldStoreUser = source.containsKey("store_user") && (boolean) source.get("store_user"); SampleResource resource = new SampleResource(); + resource.setGroupId(groupId); resource.setName(name); resource.setDescription(description); resource.setAttributes(attributes); diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java index 1b0c9c7528..126503faf8 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/GetResourceTransportAction.java @@ -87,7 +87,6 @@ private void fetchResourceById(String resourceId, ActionListener { - /** - * Add sample resource to group action instance - */ - public static final AddResourceToGroupAction INSTANCE = new AddResourceToGroupAction(); - /** - * Add sample resource to group action name - */ - public static final String NAME = "cluster:admin/sample-resource-plugin/group/add"; - - private AddResourceToGroupAction() { - super(NAME, AddResourceToGroupResponse::new); - } -} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java deleted file mode 100644 index d7d6474444..0000000000 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRequest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.sample.resourcegroup.actions.rest.add; - -import java.io.IOException; - -import org.opensearch.action.ActionRequest; -import org.opensearch.action.ActionRequestValidationException; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; - -import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; -import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; -import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; - -/** - * Request object for AddResourceToGroupRequest transport action - */ -// TODO Make this implement DocRequest.ParentReferencing and show authorization of add to group/remove from group actions -// This is currently authorized as a cluster permission -public class AddResourceToGroupRequest extends ActionRequest { - - private final String groupId; - private final String resourceId; - - /** - * Default constructor - */ - public AddResourceToGroupRequest(String groupId, String resourceId) { - this.groupId = groupId; - this.resourceId = resourceId; - } - - public AddResourceToGroupRequest(StreamInput in) throws IOException { - this.groupId = in.readString(); - this.resourceId = in.readString(); - } - - @Override - public void writeTo(final StreamOutput out) throws IOException { - out.writeString(groupId); - out.writeString(resourceId); - } - - @Override - public ActionRequestValidationException validate() { - return null; - } - - public String getResourceId() { - return this.resourceId; - } - - public String getGroupId() { - return this.groupId; - } - - // @Override - public String type() { - return RESOURCE_TYPE; - } - - // @Override - public String index() { - return RESOURCE_INDEX_NAME; - } - - // @Override - public String id() { - return resourceId; - } - - // @Override - public String parentType() { - return RESOURCE_GROUP_TYPE; - } - - // @Override - public String parentId() { - return groupId; - } -} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java deleted file mode 100644 index 36833b3af3..0000000000 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupResponse.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.sample.resourcegroup.actions.rest.add; - -import java.io.IOException; - -import org.opensearch.core.action.ActionResponse; -import org.opensearch.core.common.io.stream.StreamInput; -import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.core.xcontent.ToXContentObject; -import org.opensearch.core.xcontent.XContentBuilder; - -/** - * Response to a AddSampleResourceToGroupRequest - */ -public class AddResourceToGroupResponse extends ActionResponse implements ToXContentObject { - private final String message; - - /** - * Default constructor - * - * @param message The message - */ - public AddResourceToGroupResponse(String message) { - this.message = message; - } - - @Override - public void writeTo(StreamOutput out) throws IOException { - out.writeString(message); - } - - /** - * Constructor with StreamInput - * - * @param in the stream input - */ - public AddResourceToGroupResponse(final StreamInput in) throws IOException { - message = in.readString(); - } - - @Override - public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - builder.startObject(); - builder.field("message", message); - builder.endObject(); - return builder; - } -} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java deleted file mode 100644 index d8194296ac..0000000000 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/add/AddResourceToGroupRestAction.java +++ /dev/null @@ -1,63 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.sample.resourcegroup.actions.rest.add; - -import java.io.IOException; -import java.util.List; -import java.util.Map; - -import org.opensearch.core.xcontent.XContentParser; -import org.opensearch.rest.BaseRestHandler; -import org.opensearch.rest.RestRequest; -import org.opensearch.rest.action.RestToXContentListener; -import org.opensearch.transport.client.node.NodeClient; - -import static org.opensearch.rest.RestRequest.Method.POST; -import static org.opensearch.sample.utils.Constants.SAMPLE_RESOURCE_PLUGIN_API_PREFIX; - -/** - * Rest Action to add a Sample Resource to a Group. - */ -public class AddResourceToGroupRestAction extends BaseRestHandler { - - public AddResourceToGroupRestAction() {} - - @Override - public List routes() { - return List.of(new Route(POST, SAMPLE_RESOURCE_PLUGIN_API_PREFIX + "/group/add/{group_id}")); - } - - @Override - public String getName() { - return "add_sample_resource_to_group"; - } - - @Override - protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient client) throws IOException { - Map source; - try (XContentParser parser = request.contentParser()) { - source = parser.map(); - } - - return switch (request.method()) { - case POST -> addResourceToGroup(source, request.param("group_id"), client); - default -> throw new IllegalArgumentException("Illegal method: " + request.method()); - }; - } - - private RestChannelConsumer addResourceToGroup(Map source, String groupId, NodeClient client) throws IOException { - String resourceId = (String) source.get("resource_id"); - final AddResourceToGroupRequest addResourceToGroupRequest = new AddResourceToGroupRequest(groupId, resourceId); - return channel -> client.executeLocally( - AddResourceToGroupAction.INSTANCE, - addResourceToGroupRequest, - new RestToXContentListener<>(channel) - ); - } -} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRequest.java index 0e7db2dc75..3b529852af 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRequest.java @@ -15,7 +15,7 @@ import org.opensearch.action.DocRequest; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.sample.SampleResource; +import org.opensearch.sample.SampleResourceGroup; import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -25,22 +25,22 @@ */ public class CreateResourceGroupRequest extends ActionRequest implements DocRequest { - private final SampleResource resource; + private final SampleResourceGroup resourceGroup; /** * Default constructor */ - public CreateResourceGroupRequest(SampleResource resource) { - this.resource = resource; + public CreateResourceGroupRequest(SampleResourceGroup resourceGroup) { + this.resourceGroup = resourceGroup; } public CreateResourceGroupRequest(StreamInput in) throws IOException { - this.resource = in.readNamedWriteable(SampleResource.class); + this.resourceGroup = in.readNamedWriteable(SampleResourceGroup.class); } @Override public void writeTo(final StreamOutput out) throws IOException { - resource.writeTo(out); + resourceGroup.writeTo(out); } @Override @@ -48,8 +48,8 @@ public ActionRequestValidationException validate() { return null; } - public SampleResource getResource() { - return this.resource; + public SampleResourceGroup getResourceGroup() { + return this.resourceGroup; } @Override diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRestAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRestAction.java index b3e5317966..fb555be28c 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRestAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/CreateResourceGroupRestAction.java @@ -16,7 +16,7 @@ import org.opensearch.rest.BaseRestHandler; import org.opensearch.rest.RestRequest; import org.opensearch.rest.action.RestToXContentListener; -import org.opensearch.sample.SampleResource; +import org.opensearch.sample.SampleResourceGroup; import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.rest.RestRequest.Method.POST; @@ -60,12 +60,10 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli private RestChannelConsumer updateResource(Map source, String resourceId, NodeClient client) throws IOException { String name = (String) source.get("name"); String description = source.containsKey("description") ? (String) source.get("description") : null; - Map attributes = getAttributes(source); - SampleResource resource = new SampleResource(); - resource.setName(name); - resource.setDescription(description); - resource.setAttributes(attributes); - final UpdateResourceGroupRequest updateResourceRequest = new UpdateResourceGroupRequest(resourceId, resource); + SampleResourceGroup resourceGroup = new SampleResourceGroup(); + resourceGroup.setName(name); + resourceGroup.setDescription(description); + final UpdateResourceGroupRequest updateResourceRequest = new UpdateResourceGroupRequest(resourceId, resourceGroup); return channel -> client.executeLocally( UpdateResourceGroupAction.INSTANCE, updateResourceRequest, @@ -76,22 +74,14 @@ private RestChannelConsumer updateResource(Map source, String re private RestChannelConsumer createResource(Map source, NodeClient client) throws IOException { String name = (String) source.get("name"); String description = source.containsKey("description") ? (String) source.get("description") : null; - Map attributes = getAttributes(source); - SampleResource resource = new SampleResource(); - resource.setName(name); - resource.setDescription(description); - resource.setAttributes(attributes); - final CreateResourceGroupRequest createSampleResourceRequest = new CreateResourceGroupRequest(resource); + SampleResourceGroup resourceGroup = new SampleResourceGroup(); + resourceGroup.setName(name); + resourceGroup.setDescription(description); + final CreateResourceGroupRequest createSampleResourceRequest = new CreateResourceGroupRequest(resourceGroup); return channel -> client.executeLocally( CreateResourceGroupAction.INSTANCE, createSampleResourceRequest, new RestToXContentListener<>(channel) ); } - - // NOTE: Do NOT use @SuppressWarnings("unchecked") on untrusted data in production code. This is used here only to keep the code simple - @SuppressWarnings("unchecked") - private Map getAttributes(Map source) { - return source.containsKey("attributes") ? (Map) source.get("attributes") : null; - } } diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/UpdateResourceGroupRequest.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/UpdateResourceGroupRequest.java index ad1d6eac9e..b1ba443fda 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/UpdateResourceGroupRequest.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/rest/create/UpdateResourceGroupRequest.java @@ -15,7 +15,7 @@ import org.opensearch.action.DocRequest; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; -import org.opensearch.sample.SampleResource; +import org.opensearch.sample.SampleResourceGroup; import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -26,25 +26,25 @@ public class UpdateResourceGroupRequest extends ActionRequest implements DocRequest { private final String resourceId; - private final SampleResource resource; + private final SampleResourceGroup resourceGroup; /** * Default constructor */ - public UpdateResourceGroupRequest(String resourceId, SampleResource resource) { + public UpdateResourceGroupRequest(String resourceId, SampleResourceGroup resourceGroup) { this.resourceId = resourceId; - this.resource = resource; + this.resourceGroup = resourceGroup; } public UpdateResourceGroupRequest(StreamInput in) throws IOException { this.resourceId = in.readString(); - this.resource = in.readNamedWriteable(SampleResource.class); + this.resourceGroup = in.readNamedWriteable(SampleResourceGroup.class); } @Override public void writeTo(final StreamOutput out) throws IOException { out.writeString(resourceId); - resource.writeTo(out); + resourceGroup.writeTo(out); } @Override @@ -52,8 +52,8 @@ public ActionRequestValidationException validate() { return null; } - public SampleResource getResource() { - return this.resource; + public SampleResourceGroup getResourceGroup() { + return this.resourceGroup; } public String getResourceId() { diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java deleted file mode 100644 index bbfd571e79..0000000000 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/AddResourceToGroupTransportAction.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.sample.resourcegroup.actions.transport; - -import java.util.Map; - -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; - -import org.opensearch.action.support.ActionFilters; -import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.action.support.WriteRequest; -import org.opensearch.action.update.UpdateRequest; -import org.opensearch.common.inject.Inject; -import org.opensearch.core.action.ActionListener; -import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupAction; -import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupRequest; -import org.opensearch.sample.resourcegroup.actions.rest.add.AddResourceToGroupResponse; -import org.opensearch.sample.utils.PluginClient; -import org.opensearch.tasks.Task; -import org.opensearch.transport.TransportService; - -import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; -import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; - -/** - * Transport action for updating a resource. - */ -public class AddResourceToGroupTransportAction extends HandledTransportAction { - private static final Logger log = LogManager.getLogger(AddResourceToGroupTransportAction.class); - - private final TransportService transportService; - private final PluginClient pluginClient; - - @Inject - public AddResourceToGroupTransportAction(TransportService transportService, ActionFilters actionFilters, PluginClient pluginClient) { - super(AddResourceToGroupAction.NAME, transportService, actionFilters, AddResourceToGroupRequest::new); - this.transportService = transportService; - this.pluginClient = pluginClient; - } - - @Override - protected void doExecute(Task task, AddResourceToGroupRequest request, ActionListener listener) { - if (request.getResourceId() == null || request.getResourceId().isEmpty()) { - listener.onFailure(new IllegalArgumentException("Resource Group ID cannot be null or empty")); - return; - } - // Check permission to resource - addResourceToGroup(request, listener); - } - - private void addResourceToGroup(AddResourceToGroupRequest request, ActionListener listener) { - try { - String resourceId = request.getResourceId(); - String groupId = request.getGroupId(); - // because some plugins seem to treat update API calls as index request - UpdateRequest ur = new UpdateRequest().index(RESOURCE_INDEX_NAME) - .id(resourceId) - .setRefreshPolicy(WriteRequest.RefreshPolicy.WAIT_UNTIL) // WAIT_UNTIL because we don't want tests to fail, as they - // execute search right after update - .doc(Map.of("parent_id", groupId, "parent_type", RESOURCE_GROUP_TYPE)); - - log.debug("Update Request: {}", ur.toString()); - - pluginClient.update(ur, ActionListener.wrap(updateResponse -> { - listener.onResponse(new AddResourceToGroupResponse("Resource ID " + resourceId + " added to group ID " + groupId + ".")); - }, listener::onFailure)); - } catch (Exception e) { - log.error("Failed to update resource: {}", request.getResourceId(), e); - listener.onFailure(e); - } - - } -} diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/CreateResourceGroupTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/CreateResourceGroupTransportAction.java index 6856417839..85d3640017 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/CreateResourceGroupTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/CreateResourceGroupTransportAction.java @@ -25,7 +25,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.sample.SampleResource; +import org.opensearch.sample.SampleResourceGroup; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupRequest; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupResponse; @@ -53,11 +53,11 @@ public CreateResourceGroupTransportAction(TransportService transportService, Act @Override protected void doExecute(Task task, CreateResourceGroupRequest request, ActionListener listener) { - createResource(request, listener); + createResourceGroup(request, listener); } - private void createResource(CreateResourceGroupRequest request, ActionListener listener) { - SampleResource sample = request.getResource(); + private void createResourceGroup(CreateResourceGroupRequest request, ActionListener listener) { + SampleResourceGroup sample = request.getResourceGroup(); // 1. Read mapping JSON from the config file final String mappingJson; diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/UpdateResourceGroupTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/UpdateResourceGroupTransportAction.java index b6fd7a67c0..0e34bc8491 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/UpdateResourceGroupTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resourcegroup/actions/transport/UpdateResourceGroupTransportAction.java @@ -19,7 +19,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.sample.SampleResource; +import org.opensearch.sample.SampleResourceGroup; import org.opensearch.sample.resourcegroup.actions.rest.create.CreateResourceGroupResponse; import org.opensearch.sample.resourcegroup.actions.rest.create.UpdateResourceGroupAction; import org.opensearch.sample.resourcegroup.actions.rest.create.UpdateResourceGroupRequest; @@ -59,7 +59,7 @@ protected void doExecute(Task task, UpdateResourceGroupRequest request, ActionLi private void updateResource(UpdateResourceGroupRequest request, ActionListener listener) { try { String resourceId = request.getResourceId(); - SampleResource sample = request.getResource(); + SampleResourceGroup sample = request.getResourceGroup(); try (XContentBuilder builder = jsonBuilder()) { sample.toXContent(builder, ToXContent.EMPTY_PARAMS); @@ -73,7 +73,7 @@ private void updateResource(UpdateResourceGroupRequest request, ActionListener { listener.onResponse( - new CreateResourceGroupResponse("Resource " + request.getResource().getName() + " updated successfully.") + new CreateResourceGroupResponse("Resource " + request.getResourceGroup().getName() + " updated successfully.") ); }, listener::onFailure)); } diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index 4a870be35d..56e41809e4 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -22,6 +22,7 @@ import org.opensearch.security.resources.sharing.CreatedBy; import org.opensearch.security.resources.sharing.ResourceSharing; import org.opensearch.security.setting.OpensearchDynamicSetting; +import org.opensearch.security.spi.resources.ResourceProvider; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.user.User; import org.opensearch.threadpool.ThreadPool; @@ -72,8 +73,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re } String resourceType = resourcePluginInfo.getResourceTypeForIndexOp(resourceIndex, index); - - // TODO Complete this + ResourceProvider provider = resourcePluginInfo.getResourceProvider(resourceType); log.debug("postIndex called on {}", resourceIndex); @@ -112,13 +112,17 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re ); }, e -> { log.debug(e.getMessage()); }); // User.getRequestedTenant() is null if multi-tenancy is disabled - this.resourceSharingIndexHandler.indexResourceSharing( - resourceId, - resourceIndex, - new CreatedBy(user.getName(), user.getRequestedTenant()), - null, - listener - ); + ResourceSharing.Builder builder = ResourceSharing.builder() + .resourceId(resourceId) + .resourceType(resourceType) + .createdBy(new CreatedBy(user.getName(), user.getRequestedTenant())); + if (provider.parentType() != null) { + builder.parentType(provider.parentType()) + .parentId(ResourcePluginInfo.extractFieldFromIndexOp(provider.parentIdField(), index)); + } + ResourceSharing sharingInfo = builder.build(); + + this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, sharingInfo, listener); } catch (IOException e) { log.debug("Failed to create a resource sharing entry for resource: {}", resourceId, e); diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index 470fc41b3f..3c40fdb34f 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -120,31 +120,40 @@ public void updateProtectedTypes(List protectedTypes) { } } + public static String extractFieldFromIndexOp(String fieldName, Engine.Index indexOp) { + String fieldValue = null; + for (IndexableField f : indexOp.parsedDoc().rootDoc().getFields(fieldName)) { + if (f.stringValue() != null) { + fieldValue = f.stringValue(); + break; + } + if (f.binaryValue() != null) { // e.g., BytesRef-backed + fieldValue = f.binaryValue().utf8ToString(); + break; + } + } + return fieldValue; + } + public String getResourceTypeForIndexOp(String resourceIndex, Engine.Index indexOp) { lock.readLock().lock(); try { - // Eagerly use type field from provider of same index as the indexOp - // If typeField is present, assume single resource type per index - for (var entry : typeToProvider.entrySet()) { - if (entry.getValue().resourceIndexName().equals(resourceIndex)) { - if (entry.getValue().typeField() != null) { - String resourceType = null; - for (IndexableField f : indexOp.parsedDoc().rootDoc().getFields(entry.getValue().typeField())) { - if (f.stringValue() != null) { - resourceType = f.stringValue(); - break; - } - if (f.binaryValue() != null) { // e.g., BytesRef-backed - resourceType = f.binaryValue().utf8ToString(); - break; - } - } - return resourceType; - } - return entry.getValue().typeField(); - } + // Eagerly use type field from first matching provider of same index as the indexOp + // If typeField is not present, assume single resource type per index and return type from provider + var provider = typeToProvider.values() + .stream() + .filter(p -> p.resourceIndexName().equals(resourceIndex)) + .findFirst() + .orElse(null); + if (provider == null) { + // should not happen + return null; } - return null; + if (provider.typeField() != null) { + return extractFieldFromIndexOp(provider.typeField(), indexOp); + } + // If `typeField` is not defined, assume single type to index and return type from provider + return provider.resourceType(); } finally { lock.readLock().unlock(); } @@ -196,6 +205,15 @@ public FlattenedActionGroups flattenedForType(String resourceType) { } } + public ResourceProvider getResourceProvider(String type) { + lock.readLock().lock(); + try { + return typeToProvider.get(type); + } finally { + lock.readLock().unlock(); + } + } + public String indexByType(String type) { lock.readLock().lock(); try { diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 179494a84e..4af8f0905b 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -233,36 +233,31 @@ public void fetchAndUpdateResourceVisibility(String resourceId, String resourceI * This method handles the persistence of sharing metadata for resources, including * the creator information and sharing permissions. * - * @param resourceId The unique identifier of the resource being shared * @param resourceIndex The source index where the original resource is stored - * @param createdBy Object containing information about the user creating/updating the sharing - * @param shareWith Object containing the sharing permissions' configuration. Can be null for initial creation. + * @param sharingInfo Object containing information about the user creating the resource and referential information + * about the location of the resource and related docs * When provided, it should contain the access control settings for different groups: * { - * "action-group": { - * "users": ["user1", "user2"], - * "roles": ["role1", "role2"], - * "backend_roles": ["backend_role1"] - * } + * "action-group": { + * "users": ["user1", "user2"], + * "roles": ["role1", "role2"], + * "backend_roles": ["backend_role1"] + * } * } * @param listener Returns resourceSharing object if the operation was successful, exception otherwise * @throws IOException if there are issues with index operations or JSON processing */ - public void indexResourceSharing( - String resourceId, - String resourceIndex, - CreatedBy createdBy, - ShareWith shareWith, - ActionListener listener - ) throws IOException { + public void indexResourceSharing(String resourceIndex, ResourceSharing sharingInfo, ActionListener listener) + throws IOException { + String resourceId = sharingInfo.getResourceId(); + CreatedBy createdBy = sharingInfo.getCreatedBy(); // TODO: Once stashContext is replaced with switchContext this call will have to be modified String resourceSharingIndex = getSharingIndex(resourceIndex); try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - ResourceSharing entry = new ResourceSharing(resourceId, createdBy, shareWith); IndexRequest ir = client.prepareIndex(resourceSharingIndex) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .setSource(entry.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .setSource(sharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) .setOpType(DocWriteRequest.OpType.CREATE) // only create if an entry doesn't exist .setId(resourceId) .request(); @@ -280,17 +275,17 @@ public void indexResourceSharing( resourceId, resourceIndex ); - listener.onResponse(entry); + listener.onResponse(sharingInfo); }, (e) -> { LOGGER.error("Failed to create principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); - listener.onResponse(entry); + listener.onResponse(sharingInfo); }) ); }, (e) -> { if (ExceptionsHelper.unwrapCause(e) instanceof VersionConflictEngineException) { // already exists → skipping LOGGER.debug("Entry for [{}] already exists in [{}], skipping", resourceId, resourceSharingIndex); - listener.onResponse(entry); + listener.onResponse(sharingInfo); } else { LOGGER.error("Failed to create entry in [{}] for resource [{}]", resourceSharingIndex, resourceId, e); listener.onFailure(e); diff --git a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java index 70bdf61de3..a3ec9973fb 100644 --- a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java +++ b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java @@ -60,6 +60,7 @@ import org.opensearch.security.resources.sharing.ResourceSharing; import org.opensearch.security.resources.sharing.ShareWith; import org.opensearch.security.securityconf.impl.CType; +import org.opensearch.security.spi.resources.ResourceProvider; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; @@ -292,6 +293,8 @@ private ValidationResult createNewSharingRecords(Triple createNewSharingRecords(Triple void validateRequiredField(String field, T value) { @@ -315,4 +379,53 @@ public List getAllPrincipals() { return principals; } + + public static final class Builder { + private String resourceId; + private String resourceType; + private String parentType; + private String parentId; + private CreatedBy createdBy; + private ShareWith shareWith; + + public Builder resourceId(String resourceId) { + this.resourceId = resourceId; + return this; + } + + public Builder resourceType(String resourceType) { + this.resourceType = resourceType; + return this; + } + + public Builder parentType(String parentType) { + this.parentType = parentType; + return this; + } + + public Builder parentId(String parentId) { + this.parentId = parentId; + return this; + } + + public Builder createdBy(CreatedBy createdBy) { + this.createdBy = createdBy; + return this; + } + + public Builder shareWith(ShareWith shareWith) { + this.shareWith = shareWith; + return this; + } + + /** + * Build the immutable/constructed instance, validating required fields. + */ + public ResourceSharing build() { + validateRequiredField("resource_id", resourceId); + validateRequiredField("created_by", createdBy); + + return new ResourceSharing(this); + } + } } From 2bdfacd121c0f33d27e84c12ea941239cc8d73e6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 24 Oct 2025 20:35:13 -0400 Subject: [PATCH 04/17] Show hierarchy working Signed-off-by: Craig Perkins --- .../resourcegroup/ResourceHierarchyTests.java | 2 +- .../main/resources/resource-action-groups.yml | 5 +++ .../resources/ResourceAccessHandler.java | 35 +++++++++++-------- .../resources/sharing/ResourceSharing.java | 28 +++++++++++++++ 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java index 8729d120d8..a79dc27edb 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java @@ -80,7 +80,7 @@ public void testShouldHaveAccessToResourceWithGroupLevelAccess() throws Exceptio ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); ok(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); - ok(() -> api.getResource(resourceGroupId, FULL_ACCESS_USER)); + ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); } } diff --git a/sample-resource-plugin/src/main/resources/resource-action-groups.yml b/sample-resource-plugin/src/main/resources/resource-action-groups.yml index c373634be9..3864586997 100644 --- a/sample-resource-plugin/src/main/resources/resource-action-groups.yml +++ b/sample-resource-plugin/src/main/resources/resource-action-groups.yml @@ -16,12 +16,17 @@ resource_types: sample_group_read_only: allowed_actions: - "cluster:admin/sample-resource-plugin/group/get" + # TODO: can sample-resource access levels be referenced here? i.e. sample_read_only + - "cluster:admin/sample-resource-plugin/get" sample_group_read_write: allowed_actions: - "cluster:admin/sample-resource-plugin/group/*" + - "cluster:admin/sample-resource-plugin/*" sample_group_full_access: allowed_actions: - "cluster:admin/sample-resource-plugin/group/*" + - "cluster:admin/sample-resource-plugin/*" + - "cluster:admin/security/resource/group/share" - "cluster:admin/security/resource/share" diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index e547732a9f..d60524d103 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -29,7 +29,6 @@ import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.privileges.PrivilegesEvaluator; -import org.opensearch.security.resources.sharing.Recipient; import org.opensearch.security.resources.sharing.ResourceSharing; import org.opensearch.security.resources.sharing.ShareWith; import org.opensearch.security.securityconf.FlattenedActionGroups; @@ -160,8 +159,6 @@ public void hasPermission( listener.onResponse(true); return; } - Set userRoles = new HashSet<>(user.getSecurityRoles()); - Set userBackendRoles = new HashSet<>(user.getRoles()); String resourceIndex = resourcePluginInfo.indexByType(resourceType); if (resourceIndex == null) { @@ -170,30 +167,29 @@ public void hasPermission( return; } - resourceSharingIndexHandler.fetchSharingInfo(resourceIndex, resourceId, ActionListener.wrap(document -> { + resourceSharingIndexHandler.fetchSharingInfo(resourceIndex, resourceId, ActionListener.wrap(sharingInfo -> { // Document may be null when cluster has enabled resource-sharing protection for that index, but have not migrated any records. // This also means that for non-existing documents, the evaluator will return 403 instead - if (document == null) { + if (sharingInfo == null) { LOGGER.warn("No sharing info found for '{}'. Action {} is not allowed.", resourceId, action); listener.onResponse(false); return; } - userRoles.add("*"); - userBackendRoles.add("*"); - - if (document.isCreatedBy(user.getName())) { + if (sharingInfo.isCreatedBy(user.getName())) { listener.onResponse(true); return; } - Set accessLevels = new HashSet<>(); - accessLevels.addAll(document.fetchAccessLevels(Recipient.USERS, Set.of(user.getName(), "*"))); - accessLevels.addAll(document.fetchAccessLevels(Recipient.ROLES, userRoles)); - accessLevels.addAll(document.fetchAccessLevels(Recipient.BACKEND_ROLES, userBackendRoles)); + Set accessLevels = sharingInfo.getAccessLevelsForUser(user); + // no matching access level, either recurse up or fail fast if (accessLevels.isEmpty()) { - listener.onResponse(false); + if (sharingInfo.getParentId() != null) { + hasPermission(sharingInfo.getParentId(), sharingInfo.getParentType(), action, listener); + } else { + listener.onResponse(false); + } return; } @@ -202,7 +198,16 @@ public void hasPermission( final Set allowedActions = agForType.resolve(accessLevels); final WildcardMatcher matcher = WildcardMatcher.from(allowedActions); - listener.onResponse(matcher.test(action)); + if (matcher.test(action)) { + listener.onResponse(true); + return; + } + + if (sharingInfo.getParentId() != null) { + hasPermission(sharingInfo.getParentId(), sharingInfo.getParentType(), action, listener); + } else { + listener.onResponse(false); + } }, e -> { LOGGER.error("Error while checking permission for user {} on resource {}: {}", user.getName(), resourceId, e.getMessage()); listener.onFailure(e); diff --git a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java index 2d9cebcfa8..32bf79161f 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java @@ -26,6 +26,7 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.security.user.User; /** * Represents a resource sharing configuration that manages access control for OpenSearch resources. @@ -114,6 +115,14 @@ public ShareWith getShareWith() { return shareWith; } + public String getParentType() { + return parentType; + } + + public String getParentId() { + return parentId; + } + public void share(String accessLevel, Recipients target) { if (shareWith == null) { Map recs = new HashMap<>(); @@ -308,6 +317,25 @@ public boolean isSharedWithEntity(Recipient recipientType, Set targets, return shareWith.atAccessLevel(accessLevel).isSharedWithAny(recipientType, targets); } + /** + * + * @param user + * @return + */ + public Set getAccessLevelsForUser(User user) { + Set userRoles = new HashSet<>(user.getSecurityRoles()); + Set userBackendRoles = new HashSet<>(user.getRoles()); + + userRoles.add("*"); + userBackendRoles.add("*"); + + Set accessLevels = new HashSet<>(); + accessLevels.addAll(fetchAccessLevels(Recipient.USERS, Set.of(user.getName(), "*"))); + accessLevels.addAll(fetchAccessLevels(Recipient.ROLES, userRoles)); + accessLevels.addAll(fetchAccessLevels(Recipient.BACKEND_ROLES, userBackendRoles)); + return accessLevels; + } + /** * Fetches all access-levels where at-least 1 recipient matches the given set of targets * @param recipientType the type of recipient to be matched against From 11d3ea38d6e738297ebea28a6643420894cf2243 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 24 Oct 2025 20:55:52 -0400 Subject: [PATCH 05/17] Use non-deprecated interface Signed-off-by: Craig Perkins --- .../java/org/opensearch/security/filter/SecurityFilter.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index 4e99f06a54..a23db341fd 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -61,6 +61,7 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.support.ActionFilter; import org.opensearch.action.support.ActionFilterChain; +import org.opensearch.action.support.ActionRequestMetadata; import org.opensearch.action.update.UpdateRequest; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; @@ -167,6 +168,7 @@ public void app Task task, final String action, Request request, + ActionRequestMetadata actionRequestMetadata, ActionListener listener, ActionFilterChain chain ) { From 33d09c86cfaf17bea9b6cee660a33f1bdc498412 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 24 Oct 2025 21:00:51 -0400 Subject: [PATCH 06/17] Fix code hygiene issue Signed-off-by: Craig Perkins --- .../org/opensearch/security/filter/SecurityFilterTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java index c127b280c7..5a311bec8e 100644 --- a/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java +++ b/src/test/java/org/opensearch/security/filter/SecurityFilterTests.java @@ -119,7 +119,7 @@ public void testUnexepectedCausesAreNotSendToCallers() { ); // Act - filter.apply(null, null, null, listener, null); + filter.apply(null, null, null, null, listener, null); // Verify verify(auditLog).getComplianceConfig(); // Make sure the exception was thrown From ad45784faab27653e29a6cea57f226de2ce81672 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 27 Oct 2025 16:41:42 -0400 Subject: [PATCH 07/17] Fix few tests Signed-off-by: Craig Perkins --- .../ResourceSharingIndexHandler.java | 27 +++++-------------- .../resources/sharing/ResourceSharing.java | 22 ++++++++++++--- .../security/resources/sharing/ShareWith.java | 14 +++++----- .../resources/ResourceAccessHandlerTest.java | 6 +---- 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 4af8f0905b..6ad6e481ef 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -13,7 +13,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -720,33 +719,19 @@ public void patchSharingInfo( // Apply patch and update the document sharingInfoListener.whenComplete(sharingInfo -> { - ShareWith updatedShareWith = sharingInfo.getShareWith(); - if (updatedShareWith == null) { - updatedShareWith = new ShareWith(new HashMap<>()); - } if (add != null) { - updatedShareWith = updatedShareWith.add(add); + sharingInfo.getShareWith().add(add); } if (revoke != null) { - updatedShareWith = updatedShareWith.revoke(revoke); + sharingInfo.getShareWith().revoke(revoke); } - ShareWith cleaned = null; - if (updatedShareWith != null) { - ShareWith pruned = updatedShareWith.prune(); - if (!pruned.isPrivate()) { - cleaned = pruned; // store only if something non-empty remains - } - } - - ResourceSharing updatedSharingInfo = new ResourceSharing(resourceId, sharingInfo.getCreatedBy(), cleaned); - try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { // update the record IndexRequest ir = client.prepareIndex(resourceSharingIndex) .setId(resourceId) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .setSource(updatedSharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) + .setSource(sharingInfo.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS)) .setOpType(DocWriteRequest.OpType.INDEX) .request(); @@ -762,13 +747,13 @@ public void patchSharingInfo( updateResourceVisibility( resourceId, resourceIndex, - updatedSharingInfo.getAllPrincipals(), + sharingInfo.getAllPrincipals(), ActionListener.wrap((updateResponse) -> { LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); - listener.onResponse(updatedSharingInfo); + listener.onResponse(sharingInfo); }, (e) -> { LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); - listener.onResponse(updatedSharingInfo); + listener.onResponse(sharingInfo); }) ); diff --git a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java index 32bf79161f..77d940c535 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java @@ -112,6 +112,10 @@ public CreatedBy getCreatedBy() { } public ShareWith getShareWith() { + if (shareWith == null) { + // never been shared before, private access + return new ShareWith(new HashMap<>()); + } return shareWith; } @@ -251,13 +255,25 @@ public static ResourceSharing fromXContent(XContentParser parser) throws IOExcep b.resourceId(parser.text()); break; case "resource_type": - b.resourceType(parser.text()); + if (token == XContentParser.Token.VALUE_NULL) { + b.resourceType(null); + } else { + b.resourceType(parser.text()); + } break; case "parent_type": - b.parentType(parser.text()); + if (token == XContentParser.Token.VALUE_NULL) { + b.parentType(null); + } else { + b.parentType(parser.text()); + } break; case "parent_id": - b.parentId(parser.text()); + if (token == XContentParser.Token.VALUE_NULL) { + b.parentId(null); + } else { + b.parentId(parser.text()); + } break; case "created_by": b.createdBy(CreatedBy.fromXContent(parser)); diff --git a/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java b/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java index b827e6cef9..84a3ccbfc8 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ShareWith.java @@ -141,16 +141,15 @@ public ShareWith add(ShareWith other) { if (other == null || other.isPrivate()) { return this; } - Map updated = new HashMap<>(this.sharingInfo); for (var entry : other.sharingInfo.entrySet()) { String level = entry.getKey(); Recipients patchRecipients = entry.getValue(); - updated.merge(level, patchRecipients, (orig, patchRec) -> { + sharingInfo.merge(level, patchRecipients, (orig, patchRec) -> { orig.share(patchRec); return orig; }); } - return new ShareWith(updated); + return this; } /** @@ -160,16 +159,15 @@ public ShareWith revoke(ShareWith other) { if (this.sharingInfo.isEmpty() || other == null || other.isPrivate()) { return this; } - Map updated = new HashMap<>(this.sharingInfo); for (var entry : other.sharingInfo.entrySet()) { String level = entry.getKey(); - Recipients revokeRecipients = entry.getValue(); - updated.computeIfPresent(level, (lvl, orig) -> { - orig.revoke(revokeRecipients); + Recipients toRevoke = entry.getValue(); + sharingInfo.computeIfPresent(level, (lvl, orig) -> { + orig.revoke(toRevoke); return orig; }); } - return new ShareWith(updated); + return this; } /** Return a normalized ShareWith with no empty buckets and no empty action-groups. */ diff --git a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java index 1a972f842d..8fb3f1ff23 100644 --- a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java +++ b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java @@ -24,7 +24,6 @@ import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.privileges.PrivilegesEvaluator; -import org.opensearch.security.resources.sharing.Recipient; import org.opensearch.security.resources.sharing.ResourceSharing; import org.opensearch.security.resources.sharing.ShareWith; import org.opensearch.security.securityconf.FlattenedActionGroups; @@ -124,10 +123,7 @@ public void testHasPermission_sharedWithUserAllowed() { // Document setup: shared with the user at access-level "read" ResourceSharing doc = mock(ResourceSharing.class); - when(doc.isCreatedBy("bob")).thenReturn(false); - when(doc.fetchAccessLevels(eq(Recipient.USERS), any())).thenReturn(Set.of("read")); - when(doc.fetchAccessLevels(eq(Recipient.ROLES), any())).thenReturn(Set.of()); - when(doc.fetchAccessLevels(eq(Recipient.BACKEND_ROLES), any())).thenReturn(Set.of()); + when(doc.getAccessLevelsForUser(user)).thenReturn(Set.of("read")); FlattenedActionGroups ag = mock(FlattenedActionGroups.class); when(resourcePluginInfo.flattenedForType(TYPE)).thenReturn(ag); From 2a1f18a85b5f482a25eb06933c173f3a53f99e6d Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 27 Oct 2025 17:17:05 -0400 Subject: [PATCH 08/17] Fix more tests Signed-off-by: Craig Perkins --- .../org/opensearch/security/resources/ResourcePluginInfo.java | 3 +++ .../security/resources/ResourceAccessHandlerTest.java | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index 3c40fdb34f..fd2de75535 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -217,6 +217,9 @@ public ResourceProvider getResourceProvider(String type) { public String indexByType(String type) { lock.readLock().lock(); try { + if (!typeToProvider.containsKey(type)) { + return null; + } return typeToProvider.get(type).resourceIndexName(); } finally { lock.readLock().unlock(); diff --git a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java index 8fb3f1ff23..e7a90a105e 100644 --- a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java +++ b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java @@ -150,8 +150,7 @@ public void testHasPermission_noAccessLevelsDenied() { when(adminDNs.isAdmin(user)).thenReturn(false); ResourceSharing doc = mock(ResourceSharing.class); - when(doc.isCreatedBy("charlie")).thenReturn(false); - when(doc.fetchAccessLevels(any(), any())).thenReturn(Collections.emptySet()); + when(doc.getAccessLevelsForUser(user)).thenReturn(Collections.emptySet()); doAnswer(inv -> { ActionListener l = inv.getArgument(2); From 87a93c0b79daa74b34da36dadfcdfcabb2e25ea0 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 28 Oct 2025 17:38:00 -0400 Subject: [PATCH 09/17] Fix tests Signed-off-by: Craig Perkins --- .../org/opensearch/sample/resource/TestUtils.java | 2 +- .../feature/disabled/DirectIndexAccessTests.java | 13 +++++++------ .../feature/enabled/DirectIndexAccessTests.java | 9 +++++---- .../resource/securityapis/MigrateApiTests.java | 1 + .../ShareableResourceTypesInfoApiTests.java | 4 ++-- .../MigrateResourceSharingInfoApiAction.java | 11 +++++++++-- .../security/resources/sharing/ResourceSharing.java | 2 +- 7 files changed, 26 insertions(+), 16 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index d4646119e3..9ff86dc893 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -373,7 +373,7 @@ public String createSampleResourceGroupAs(TestSecurityConfig.User user, Header.. public String createRawResourceAs(CertificateData adminCert) { try (TestRestClient client = cluster.getRestClient(adminCert)) { - String sample = "{\"name\":\"sample\"}"; + String sample = "{\"name\":\"sample\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; TestRestClient.HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_CREATED); return resp.getTextFromJsonBody("/_id"); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/disabled/DirectIndexAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/disabled/DirectIndexAccessTests.java index 279aed7513..ac921a24e9 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/disabled/DirectIndexAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/disabled/DirectIndexAccessTests.java @@ -31,6 +31,7 @@ import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_SEARCH_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.newCluster; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; import static org.opensearch.security.api.AbstractApiIntegrationTest.forbidden; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; @@ -66,7 +67,7 @@ public void testRawAccess_noAccessUser() throws Exception { // cannot access any raw request try (TestRestClient client = cluster.getRestClient(NO_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -89,7 +90,7 @@ public void testRawAccess_limitedAccessUser() { // cannot create a resource since user doesn't have indices:data/write/index permission try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -114,7 +115,7 @@ public void testRawAccess_allAccessUser() { // cannot create a resource directly since system index protection (SIP) is enabled try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -179,7 +180,7 @@ public void testRawAccess_noAccessUser() { // cannot access any raw request try (TestRestClient client = cluster.getRestClient(NO_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; TestRestClient.HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -202,7 +203,7 @@ public void testRawAccess_limitedAccessUser() { // cannot create a resource since user doesn't have indices:data/write/index permission try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; TestRestClient.HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -228,7 +229,7 @@ public void testRawAccess_allAccessUser() { // can create a resource String userResId; try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; TestRestClient.HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_CREATED); userResId = resp.getTextFromJsonBody("/_id"); 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 84e26512b5..0032acc0d6 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 @@ -36,6 +36,7 @@ import static org.opensearch.sample.resource.TestUtils.directSharePayload; import static org.opensearch.sample.resource.TestUtils.newCluster; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; /** @@ -62,7 +63,7 @@ public static class SystemIndexEnabled { private void assertResourceIndexAccess(String id, TestSecurityConfig.User user) { // cannot interact with resource index try (TestRestClient client = cluster.getRestClient(user)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -196,7 +197,7 @@ public void testRawAccess_noAccessUser() { // cannot access any raw request try (TestRestClient client = cluster.getRestClient(NO_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -221,7 +222,7 @@ public void testRawAccess_limitedAccessUser() { // cannot create a resource since user doesn't have indices:data/write/index permission try (TestRestClient client = cluster.getRestClient(LIMITED_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } @@ -259,7 +260,7 @@ public void testRawAccess_allAccessUser() { // can create a resource String userResId; try (TestRestClient client = cluster.getRestClient(FULL_ACCESS_USER)) { - String sample = "{\"name\":\"sampleUser\"}"; + String sample = "{\"name\":\"sampleUser\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc?refresh=true", sample); resp.assertStatusCode(HttpStatus.SC_CREATED); userResId = resp.getTextFromJsonBody("/_id"); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java index 372f27a2a8..e09a0bce7b 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java @@ -301,6 +301,7 @@ private ArrayNode expectedHits(String resourceId, String accessLevel) { // 3) Build the _source sub-object ObjectNode source = hit.putObject("_source"); source.put("resource_id", resourceId); + source.put("resource_type", RESOURCE_TYPE); ObjectNode createdBy = source.putObject("created_by"); createdBy.put("user", MIGRATION_USER.getName()); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareableResourceTypesInfoApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareableResourceTypesInfoApiTests.java index 770b588eb9..87376949df 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareableResourceTypesInfoApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ShareableResourceTypesInfoApiTests.java @@ -59,13 +59,13 @@ public void testTypesApi_mustListSampleResourceAsAType() { Map firstType = (Map) types.get(0); assertThat(firstType.get("type"), equalTo("sample-resource")); assertThat( - (List) firstType.get("action_groups"), + (List) firstType.get("access_levels"), containsInAnyOrder("sample_read_only", "sample_read_write", "sample_full_access") ); Map secondType = (Map) types.get(1); assertThat(secondType.get("type"), equalTo("sample-resource-group")); assertThat( - (List) secondType.get("action_groups"), + (List) secondType.get("access_levels"), containsInAnyOrder("sample_group_read_only", "sample_group_read_write", "sample_group_full_access") ); } diff --git a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java index a3ec9973fb..eaee3e5be6 100644 --- a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java +++ b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java @@ -322,8 +322,15 @@ private ValidationResult createNewSharingRecords(Triple()); + shareWith = new ShareWith(new HashMap<>()); } return shareWith; } From 7067c3d86d511516726fc2e79e9133e34672f30c Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 19 Nov 2025 13:37:39 -0500 Subject: [PATCH 10/17] Remove redundancy post merge conflict resolved Signed-off-by: Craig Perkins --- .../opensearch/security/resources/ResourceIndexListener.java | 2 -- .../org/opensearch/security/resources/ResourcePluginInfo.java | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index 92d71f578e..56e41809e4 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -77,8 +77,6 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re log.debug("postIndex called on {}", resourceIndex); - String resourceType = resourcePluginInfo.getResourceTypeForIndexOp(resourceIndex, index); - String resourceId = index.id(); // Only proceed if this was a create operation and for primary shard diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index 6fe3db7170..a8a5dc7a2e 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -57,8 +57,6 @@ public class ResourcePluginInfo { // AuthZ: resolved (flattened) groups per type private final Map typeToFlattened = new HashMap<>(); - private final Map typeToProvider = new HashMap<>(); - // cache current protected types and their indices private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); // make the updates/reads thread-safe From 2c8568bf96641af92e95a53b943d7f383e0e96a6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 19 Nov 2025 21:17:07 -0500 Subject: [PATCH 11/17] Fix conflicts Signed-off-by: Craig Perkins --- .../java/org/opensearch/sample/resource/TestUtils.java | 4 +--- .../org/opensearch/security/OpenSearchSecurityPlugin.java | 5 +++-- .../security/resources/ResourceIndexListener.java | 6 +++--- .../opensearch/security/resources/ResourcePluginInfo.java | 1 - 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index b5ed308045..04d4e0427a 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -96,8 +96,6 @@ public final class TestUtils { public static final String SAMPLE_RESOURCE_GROUP_DELETE_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/delete"; public static final String SAMPLE_RESOURCE_GROUP_SEARCH_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/search"; - public static final String SAMPLE_ADD_RESOURCE_TO_GROUP_ENDPOINT = SAMPLE_RESOURCE_PLUGIN_PREFIX + "/group/add"; - public static final String RESOURCE_SHARING_MIGRATION_ENDPOINT = "_plugins/_security/api/resources/migrate"; public static final String SECURITY_SHARE_ENDPOINT = "_plugins/_security/api/resource/share"; public static final String SECURITY_TYPES_ENDPOINT = "_plugins/_security/api/resource/types"; @@ -355,7 +353,7 @@ public String createSampleResourceAs(TestSecurityConfig.User user, Header... hea public String createSampleResourceWithGroupAs(TestSecurityConfig.User user, String groupId, Header... headers) { try (TestRestClient client = cluster.getRestClient(user)) { - String sample = "{\"group_id\":\"" + groupId + "\", \"name\":\"sample\"}"; + String sample = "{\"group_id\":\"" + groupId + "\", \"name\":\"sample\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}"; TestRestClient.HttpResponse resp = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sample, headers); resp.assertStatusCode(HttpStatus.SC_OK); return resp.getTextFromJsonBody("/message").split(":")[1].trim(); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 5d95dc9f50..9802e95680 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -360,8 +360,9 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) // dynamic settings transportPassiveAuthSetting = new TransportPassiveAuthSetting(settings); - resourceSharingEnabledSetting = new ResourceSharingFeatureFlagSetting(settings, resourcePluginInfo); - resourceSharingProtectedResourceTypesSetting = new ResourceSharingProtectedResourcesSetting(settings, resourcePluginInfo); + resourceSharingEnabledSetting = new ResourceSharingFeatureFlagSetting(settings, resourcePluginInfo); // not filtered + resourceSharingProtectedResourceTypesSetting = new ResourceSharingProtectedResourcesSetting(settings, resourcePluginInfo); // not + // filtered resourcePluginInfo.setProtectedTypesSetting(resourceSharingProtectedResourceTypesSetting); if (disabled) { diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index cf6f37ddfb..248e95fde2 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -72,11 +72,10 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re return; } - String resourceType = resourcePluginInfo.getResourceTypeForIndexOp(resourceIndex, index); - ResourceProvider provider = resourcePluginInfo.getResourceProvider(resourceType); - log.debug("postIndex called on {}", resourceIndex); + String resourceType = resourcePluginInfo.getResourceTypeForIndexOp(resourceIndex, index); + String resourceId = index.id(); ResourceProvider provider = resourcePluginInfo.getResourceProvider(resourceType); if (provider == null) { @@ -130,6 +129,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re .parentId(ResourcePluginInfo.extractFieldFromIndexOp(provider.parentIdField(), index)); } ResourceSharing sharingInfo = builder.build(); + // User.getRequestedTenant() is null if multi-tenancy is disabled this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, sharingInfo, listener); diff --git a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java index a8a5dc7a2e..c2793008ca 100644 --- a/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java +++ b/src/main/java/org/opensearch/security/resources/ResourcePluginInfo.java @@ -103,7 +103,6 @@ public void updateProtectedTypes(List protectedTypes) { typeToProvider.clear(); if (protectedTypes == null || protectedTypes.isEmpty()) { - // No protected types -> leave maps empty return; } From a86433b641d581c170e71573157b9cb78948ba35 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 19 Nov 2025 21:18:36 -0500 Subject: [PATCH 12/17] Fix conflicts Signed-off-by: Craig Perkins --- .../main/java/org/opensearch/sample/SampleResourcePlugin.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java index 5062bb0948..3cddcaeefd 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/SampleResourcePlugin.java @@ -123,6 +123,7 @@ public List getRestHandlers( handlers.add(new GetResourceGroupRestAction()); handlers.add(new DeleteResourceGroupRestAction()); handlers.add(new SearchResourceGroupRestAction()); + handlers.add(new SecurePluginRestAction()); return handlers; } From 01e5da1989b3f9d241732796e332a101bc1ab6c1 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 17 Mar 2026 13:33:35 -0400 Subject: [PATCH 13/17] Add more tests Signed-off-by: Craig Perkins --- .../resourcegroup/ResourceHierarchyTests.java | 53 ++++++++++++++----- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java index a79dc27edb..76cd740538 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/ResourceHierarchyTests.java @@ -17,14 +17,12 @@ import org.junit.runner.RunWith; import org.opensearch.sample.resource.TestUtils; -import org.opensearch.test.framework.TestSecurityConfig; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.opensearch.sample.resource.TestUtils.FULL_ACCESS_USER; -import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_FULL_ACCESS; import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_READ_ONLY; import static org.opensearch.sample.resource.TestUtils.newCluster; import static org.opensearch.security.api.AbstractApiIntegrationTest.forbidden; @@ -59,15 +57,6 @@ public void cleanup() { api.wipeOutResourceEntries(); } - private void assertNoAccessBeforeSharing(TestSecurityConfig.User user) throws Exception { - forbidden(() -> api.getResourceGroup(resourceGroupId, user)); - forbidden(() -> api.updateResourceGroup(resourceGroupId, user, "sampleUpdateAdmin")); - forbidden(() -> api.deleteResourceGroup(resourceGroupId, user)); - - forbidden(() -> api.shareResourceGroup(resourceGroupId, user, user, SAMPLE_GROUP_FULL_ACCESS)); - forbidden(() -> api.revokeResourceGroup(resourceGroupId, user, user, SAMPLE_GROUP_FULL_ACCESS)); - } - @Test public void testShouldHaveAccessToResourceWithGroupLevelAccess() throws Exception { TestRestClient.HttpResponse response = ok(() -> api.getResource(resourceId, USER_ADMIN)); @@ -76,11 +65,51 @@ public void testShouldHaveAccessToResourceWithGroupLevelAccess() throws Exceptio forbidden(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); forbidden(() -> api.getResource(resourceGroupId, FULL_ACCESS_USER)); - // 1. share at read-only for full-access user and at full-access for limited-perms user ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); ok(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); } + @Test + public void testGroupReadOnlyShouldNotGrantWriteOnChild() throws Exception { + ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); + + // read is allowed via parent + ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + + // write/delete on child should still be forbidden — read_only only maps to get actions + forbidden(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "shouldFail")); + forbidden(() -> api.deleteResource(resourceId, FULL_ACCESS_USER)); + } + + @Test + public void testRevokingGroupAccessRemovesChildAccess() throws Exception { + ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); + + ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + + ok(() -> api.revokeResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); + + forbidden(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); + forbidden(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + } + + @Test + public void testDirectChildShareGrantsAccessWithoutGroupShare() throws Exception { + // group is not shared with FULL_ACCESS_USER at all + forbidden(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); + forbidden(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + + // share the child directly at full_access + ok(() -> api.shareResource(resourceId, USER_ADMIN, FULL_ACCESS_USER, TestUtils.SAMPLE_FULL_ACCESS)); + + // child is now accessible + ok(() -> api.getResource(resourceId, FULL_ACCESS_USER)); + ok(() -> api.updateResource(resourceId, FULL_ACCESS_USER, "directShareUpdate")); + + // group itself remains inaccessible + forbidden(() -> api.getResourceGroup(resourceGroupId, FULL_ACCESS_USER)); + } + } From b9a58427a8053cd2212a0fafcf8e94ab5f63e3a3 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sat, 21 Mar 2026 10:38:17 -0400 Subject: [PATCH 14/17] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f296c9610b..a9d515c52f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Performance optimizations for building internal authorization data structures upon config updates ([#5988](https://github.com/opensearch-project/security/pull/5988)) - Make encryption_key optional for obo token authenticator ([#6017](https://github.com/opensearch-project/security/pull/6017) - Enable basic authentication for gRPC transport ([#6005](https://github.com/opensearch-project/security/pull/6005)) +- Allow specifying parentType and parentIdField in ResourceProvider ([#5735](https://github.com/opensearch-project/security/pull/5735)) ### Bug Fixes - Fix audit log writing errors for rollover-enabled alias indices ([#5878](https://github.com/opensearch-project/security/pull/5878) From 112fcd70cf62bcb8b537bd3424ba2d415f7d8f7d Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sat, 21 Mar 2026 10:54:15 -0400 Subject: [PATCH 15/17] Add support for parent_id in resource sharing migrate API Signed-off-by: Craig Perkins --- .../opensearch/sample/resource/TestUtils.java | 14 ++++ .../securityapis/MigrateApiTests.java | 80 +++++++++++++++++++ .../MigrateResourceSharingInfoApiAction.java | 27 +++++-- 3 files changed, 115 insertions(+), 6 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java index 618ce55d57..9c470a841b 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/TestUtils.java @@ -243,6 +243,20 @@ public static String migrationPayload_missingDefaultOwner() { """.formatted(RESOURCE_INDEX_NAME, "user/name", "user/backend_roles", "sample_read_only"); } + public static String migrationPayload_valid_withParent(String groupId) { + return """ + { + "source_index": "%s", + "username_path": "%s", + "backend_roles_path": "%s", + "default_owner": "%s", + "default_access_level": { + "sample-resource": "%s" + } + } + """.formatted(RESOURCE_INDEX_NAME, "user/name", "user/backend_roles", "some_user", "sample_read_only"); + } + public static String putSharingInfoPayload( String resourceId, String resourceType, diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java index 063e97e6ef..657a538d9f 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/MigrateApiTests.java @@ -42,13 +42,17 @@ import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_MIGRATION_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_CREATE_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_GET_ENDPOINT; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_GROUP_CREATE_ENDPOINT; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_RESOURCE_GROUP_GET_ENDPOINT; import static org.opensearch.sample.resource.TestUtils.migrationPayload_missingBackendRoles; import static org.opensearch.sample.resource.TestUtils.migrationPayload_missingDefaultAccessLevel; import static org.opensearch.sample.resource.TestUtils.migrationPayload_missingDefaultOwner; import static org.opensearch.sample.resource.TestUtils.migrationPayload_missingSourceIndex; import static org.opensearch.sample.resource.TestUtils.migrationPayload_missingUserName; import static org.opensearch.sample.resource.TestUtils.migrationPayload_valid; +import static org.opensearch.sample.resource.TestUtils.migrationPayload_valid_withParent; import static org.opensearch.sample.resource.TestUtils.migrationPayload_valid_withSpecifiedAccessLevel; +import static org.opensearch.sample.utils.Constants.RESOURCE_GROUP_TYPE; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; import static org.opensearch.security.resources.ResourceSharingIndexHandler.getSharingIndex; @@ -589,6 +593,82 @@ public void testMigrateAPI_inputValidation_invalidValues() { } } + @Test + public void testMigrateAPI_withParentHierarchy() { + // Create a resource group first, then a resource that belongs to it + String groupId = createSampleResourceGroup(); + String resourceId = createSampleResourceWithGroup(groupId); + clearResourceSharingEntries(); + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + TestRestClient.HttpResponse migrateResponse = client.postJson( + RESOURCE_SHARING_MIGRATION_ENDPOINT, + migrationPayload_valid_withParent(groupId) + ); + migrateResponse.assertStatusCode(HttpStatus.SC_OK); + assertThat( + migrateResponse.bodyAsMap().get("summary"), + equalTo("Migration complete. migrated 2; skippedNoType 0; skippedExisting 0; failed 0") + ); + + // Verify the sharing record for the resource has parent_type and parent_id set + TestRestClient.HttpResponse sharingResponse = client.get(RESOURCE_SHARING_INDEX + "/_search"); + sharingResponse.assertStatusCode(HttpStatus.SC_OK); + ArrayNode hitsNode = (ArrayNode) sharingResponse.bodyAsJsonNode().get("hits").get("hits"); + assertThat(hitsNode.size(), equalTo(2)); + + // Find the resource hit (not the group) and verify parent fields + com.fasterxml.jackson.databind.JsonNode resourceSource = null; + for (com.fasterxml.jackson.databind.JsonNode hit : hitsNode) { + com.fasterxml.jackson.databind.JsonNode src = hit.get("_source"); + if (RESOURCE_TYPE.equals(src.get("resource_type").asText())) { + resourceSource = src; + break; + } + } + assertThat("Expected a sharing record for resource type " + RESOURCE_TYPE, resourceSource != null); + assertThat(resourceSource.get("resource_id").asText(), equalTo(resourceId)); + assertThat(resourceSource.get("parent_type").asText(), equalTo(RESOURCE_GROUP_TYPE)); + assertThat(resourceSource.get("parent_id").asText(), equalTo(groupId)); + } + } + + private String createSampleResourceGroup() { + try (TestRestClient client = cluster.getRestClient(MIGRATION_USER)) { + String sampleGroup = """ + { + "name":"sample_group" + } + """; + TestRestClient.HttpResponse response = client.putJson(SAMPLE_RESOURCE_GROUP_CREATE_ENDPOINT, sampleGroup); + response.assertStatusCode(HttpStatus.SC_OK); + String groupId = response.getTextFromJsonBody("/message").split(":")[1].trim(); + Awaitility.await() + .alias("Wait until group is populated") + .until(() -> client.get(SAMPLE_RESOURCE_GROUP_GET_ENDPOINT + "/" + groupId).getStatusCode(), equalTo(200)); + return groupId; + } + } + + private String createSampleResourceWithGroup(String groupId) { + try (TestRestClient client = cluster.getRestClient(MIGRATION_USER)) { + String sampleResource = """ + { + "name":"sample_with_group", + "group_id":"%s", + "store_user": true + } + """.formatted(groupId); + TestRestClient.HttpResponse response = client.putJson(SAMPLE_RESOURCE_CREATE_ENDPOINT, sampleResource); + response.assertStatusCode(HttpStatus.SC_OK); + String resourceId = response.getTextFromJsonBody("/message").split(":")[1].trim(); + Awaitility.await() + .alias("Wait until resource is populated") + .until(() -> client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId).getStatusCode(), equalTo(200)); + return resourceId; + } + } + private String createSampleResource() { try (TestRestClient client = cluster.getRestClient(MIGRATION_USER)) { String sampleResource = """ diff --git a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java index 1f3482c14e..2c7d007ace 100644 --- a/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java +++ b/src/main/java/org/opensearch/security/resources/api/migrate/MigrateResourceSharingInfoApiAction.java @@ -255,7 +255,19 @@ private ValidationResult loadCurrentSharingInfo(RestRequest type = typeToDefaultAccessLevel.keySet().iterator().next(); } - results.add(new SourceDoc(id, username, backendRoles, type)); + // Extract parent ID if the provider declares a parentIdField + String parentId = null; + if (type != null) { + ResourceProvider hitProvider = resourcePluginInfo.getResourceProvider(type); + if (hitProvider != null && hitProvider.parentIdField() != null) { + parentId = rec.at("/" + hitProvider.parentIdField().replace(".", "/")).asText(null); + if (parentId != null && parentId.isEmpty()) { + parentId = null; + } + } + } + + results.add(new SourceDoc(id, username, backendRoles, type, parentId)); } // 4) fetch next batch SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); @@ -356,13 +368,16 @@ private ValidationResult createNewSharingRecords(ValidationResul failureCount.getAndIncrement(); migrationStatsLatch.countDown(); }); - // TODO account for hierarchy in migration as well (i.e. parent id) - ResourceSharing sharingInfo = ResourceSharing.builder() + // Build the ResourceSharing record, including parent hierarchy if the provider declares one + ResourceSharing.Builder sharingBuilder = ResourceSharing.builder() .resourceId(resourceId) .createdBy(createdBy) .shareWith(shareWith) - .resourceType(provider.resourceType()) - .build(); + .resourceType(provider.resourceType()); + if (doc.parentId != null && provider.parentType() != null) { + sharingBuilder.parentId(doc.parentId).parentType(provider.parentType()); + } + ResourceSharing sharingInfo = sharingBuilder.build(); sharingIndexHandler.indexResourceSharing(sourceInfo.sourceIndex, sharingInfo, listener); } catch (Exception e) { @@ -507,7 +522,7 @@ public Map allowedKeysWithCo }; } - record SourceDoc(String resourceId, String username, List backendRoles, String type) { + record SourceDoc(String resourceId, String username, List backendRoles, String type, String parentId) { } record ValidationResultArg(String sourceIndex, String defaultOwnerName, Map typeToDefaultAccessLevel, List< From 714128fd670a6912aa5d3bf978d1a89103c1ea74 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sat, 21 Mar 2026 11:35:00 -0400 Subject: [PATCH 16/17] Make child resources searchable if shared by a parent Signed-off-by: Craig Perkins --- .../ParentHierarchyAccessTests.java | 168 ++++++++++++++++++ .../src/main/resources/mappings.json | 3 + .../resources/ResourceAccessHandler.java | 52 +++++- .../ResourceSharingIndexHandler.java | 151 +++++++++++++++- .../resources/sharing/ResourceSharing.java | 4 + 5 files changed, 373 insertions(+), 5 deletions(-) create mode 100644 sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ParentHierarchyAccessTests.java diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ParentHierarchyAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ParentHierarchyAccessTests.java new file mode 100644 index 0000000000..a4593bf94f --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/securityapis/ParentHierarchyAccessTests.java @@ -0,0 +1,168 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.sample.resource.securityapis; + +import java.util.List; +import java.util.Map; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.apache.http.HttpStatus; +import org.junit.After; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.sample.resource.TestUtils; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.opensearch.sample.resource.TestUtils.RESOURCE_SHARING_INDEX; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_FULL_ACCESS; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_FULL_ACCESS; +import static org.opensearch.sample.resource.TestUtils.SECURITY_LIST_ENDPOINT; +import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; +import static org.opensearch.sample.utils.Constants.RESOURCE_TYPE; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +/** + * Tests that a user with access to a parent resource group can list/search child resources + * they were not directly shared with — i.e. parent-inherited access via the sharing index. + */ +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class ParentHierarchyAccessTests { + + @ClassRule + public static final LocalCluster cluster = TestUtils.newCluster(true, true); + + private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster); + + @After + public void clearIndices() { + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + client.delete(RESOURCE_INDEX_NAME); + client.delete(RESOURCE_SHARING_INDEX); + } + } + + /** + * Scenario: + * 1. Admin creates a resource group and a child resource that references it (group_id = groupId) + * 2. Admin shares the resource group with NO_ACCESS_USER at full access + * 3. NO_ACCESS_USER was never directly shared the child resource + * 4. Calling list API for "sample-resource" should still return the child resource + * because the user has access to its parent group + */ + @Test + @SuppressWarnings("unchecked") + public void testListChildResources_viaParentGroupAccess() { + // 1. Create a resource group and a child resource + String groupId = api.createSampleResourceGroupAs(USER_ADMIN); + api.awaitSharingEntry(groupId); + + String childId = api.createSampleResourceWithGroupAs(USER_ADMIN, groupId); + api.awaitSharingEntry(childId); + + // 2. Share the group with NO_ACCESS_USER — child is NOT directly shared + TestRestClient.HttpResponse shareResp = api.shareResourceGroup( + groupId, + USER_ADMIN, + TestUtils.NO_ACCESS_USER, + SAMPLE_GROUP_FULL_ACCESS + ); + shareResp.assertStatusCode(HttpStatus.SC_OK); + + // 3. NO_ACCESS_USER lists child resources — should see the child via parent inheritance + try (TestRestClient client = cluster.getRestClient(TestUtils.NO_ACCESS_USER)) { + TestRestClient.HttpResponse response = client.get(SECURITY_LIST_ENDPOINT + "?resource_type=" + RESOURCE_TYPE); + response.assertStatusCode(HttpStatus.SC_OK); + + List resources = (List) response.bodyAsMap().get("resources"); + assertThat("Expected 1 child resource via parent inheritance", resources.size(), equalTo(1)); + + Map resource = (Map) resources.getFirst(); + assertThat(resource.get("resource_id"), equalTo(childId)); + } + } + + /** + * Scenario: + * 1. Admin creates a resource group and two child resources referencing it + * 2. Admin directly shares one child with NO_ACCESS_USER + * 3. Admin shares the group with NO_ACCESS_USER + * 4. List API should return both children (union of direct + inherited) + */ + @Test + @SuppressWarnings("unchecked") + public void testListChildResources_unionOfDirectAndInherited() { + // 1. Create group and two children + String groupId = api.createSampleResourceGroupAs(USER_ADMIN); + api.awaitSharingEntry(groupId); + + String child1Id = api.createSampleResourceWithGroupAs(USER_ADMIN, groupId); + api.awaitSharingEntry(child1Id); + + String child2Id = api.createSampleResourceWithGroupAs(USER_ADMIN, groupId); + api.awaitSharingEntry(child2Id); + + // 2. Directly share child1 with NO_ACCESS_USER + TestRestClient.HttpResponse directShare = api.shareResource(child1Id, USER_ADMIN, TestUtils.NO_ACCESS_USER, SAMPLE_FULL_ACCESS); + directShare.assertStatusCode(HttpStatus.SC_OK); + + // 3. Share the group with NO_ACCESS_USER (gives inherited access to both children) + TestRestClient.HttpResponse groupShare = api.shareResourceGroup( + groupId, + USER_ADMIN, + TestUtils.NO_ACCESS_USER, + SAMPLE_GROUP_FULL_ACCESS + ); + groupShare.assertStatusCode(HttpStatus.SC_OK); + + // 4. List should return both children + try (TestRestClient client = cluster.getRestClient(TestUtils.NO_ACCESS_USER)) { + TestRestClient.HttpResponse response = client.get(SECURITY_LIST_ENDPOINT + "?resource_type=" + RESOURCE_TYPE); + response.assertStatusCode(HttpStatus.SC_OK); + + List resources = (List) response.bodyAsMap().get("resources"); + assertThat("Expected 2 child resources (direct + inherited)", resources.size(), equalTo(2)); + + List ids = resources.stream().map(r -> (String) ((Map) r).get("resource_id")).toList(); + assertThat(ids, hasItem(child1Id)); + assertThat(ids, hasItem(child2Id)); + } + } + + /** + * Scenario: + * 1. Admin creates a resource group and a child resource + * 2. NO_ACCESS_USER has NO access to the group + * 3. List API for child resources should return empty + */ + @Test + @SuppressWarnings("unchecked") + public void testListChildResources_noAccessToParent_returnsEmpty() { + String groupId = api.createSampleResourceGroupAs(USER_ADMIN); + api.awaitSharingEntry(groupId); + + String childId = api.createSampleResourceWithGroupAs(USER_ADMIN, groupId); + api.awaitSharingEntry(childId); + + // NO_ACCESS_USER has no access to group or child + try (TestRestClient client = cluster.getRestClient(TestUtils.NO_ACCESS_USER)) { + TestRestClient.HttpResponse response = client.get(SECURITY_LIST_ENDPOINT + "?resource_type=" + RESOURCE_TYPE); + response.assertStatusCode(HttpStatus.SC_OK); + + List resources = (List) response.bodyAsMap().get("resources"); + assertThat("Expected 0 resources when user has no parent access", resources.size(), equalTo(0)); + } + } +} diff --git a/sample-resource-plugin/src/main/resources/mappings.json b/sample-resource-plugin/src/main/resources/mappings.json index b163ee8c11..aa6bfde7d1 100644 --- a/sample-resource-plugin/src/main/resources/mappings.json +++ b/sample-resource-plugin/src/main/resources/mappings.json @@ -7,6 +7,9 @@ "resource_type": { "type": "keyword" }, + "group_id": { + "type": "keyword" + }, "all_shared_principals": { "type": "keyword" } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index adbcbd1eb9..e0f4e283e7 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -90,8 +90,31 @@ public void getOwnAndSharedResourceIdsForCurrentUser(@NonNull String resourceTyp } Set flatPrincipals = getFlatPrincipals(user); - // 3) Fetch all accessible resource IDs - resourceSharingIndexHandler.fetchAccessibleResourceIds(resourceIndex, flatPrincipals, listener); + // Phase 1: directly accessible resource IDs (via all_shared_principals on the resource index) + resourceSharingIndexHandler.fetchAccessibleResourceIds(resourceIndex, flatPrincipals, ActionListener.wrap(directIds -> { + String parentType = resourcePluginInfo.getParentType(resourceType); + if (parentType == null) { + // No parent hierarchy — return direct results as-is + listener.onResponse(directIds); + return; + } + + // Phase 2: resolve parent-inherited access + // 2a) get accessible parent resource IDs + String parentIndex = resourcePluginInfo.indexByType(parentType); + resourceSharingIndexHandler.fetchAccessibleResourceIds(parentIndex, flatPrincipals, ActionListener.wrap(parentIds -> { + if (parentIds.isEmpty()) { + listener.onResponse(directIds); + return; + } + // 2b) find child sharing records whose parent_id is in the accessible parent set + resourceSharingIndexHandler.fetchResourceIdsByParentIds(resourceIndex, parentIds, ActionListener.wrap(inheritedIds -> { + Set union = new HashSet<>(directIds); + union.addAll(inheritedIds); + listener.onResponse(union); + }, listener::onFailure)); + }, listener::onFailure)); + }, listener::onFailure)); } /** @@ -119,8 +142,29 @@ public void getResourceSharingInfoForCurrentUser(@NonNull String resourceType, A String resourceIndex = resourcePluginInfo.indexByType(resourceType); - // 3) Fetch all accessible resource sharing records - resourceSharingIndexHandler.fetchAccessibleResourceSharingRecords(resourceIndex, resourceType, user, flatPrincipals, listener); + // Phase 1: directly accessible resource IDs + resourceSharingIndexHandler.fetchAccessibleResourceIds(resourceIndex, flatPrincipals, ActionListener.wrap(directIds -> { + String parentType = resourcePluginInfo.getParentType(resourceType); + if (parentType == null) { + // No parent hierarchy — fetch sharing records for direct IDs only + resourceSharingIndexHandler.fetchResourceSharingRecordsByIds(resourceIndex, resourceType, user, directIds, listener); + return; + } + + // Phase 2: resolve parent-inherited access + String parentIndex = resourcePluginInfo.indexByType(parentType); + resourceSharingIndexHandler.fetchAccessibleResourceIds(parentIndex, flatPrincipals, ActionListener.wrap(parentIds -> { + if (parentIds.isEmpty()) { + resourceSharingIndexHandler.fetchResourceSharingRecordsByIds(resourceIndex, resourceType, user, directIds, listener); + return; + } + resourceSharingIndexHandler.fetchResourceIdsByParentIds(resourceIndex, parentIds, ActionListener.wrap(inheritedIds -> { + Set union = new HashSet<>(directIds); + union.addAll(inheritedIds); + resourceSharingIndexHandler.fetchResourceSharingRecordsByIds(resourceIndex, resourceType, user, union, listener); + }, listener::onFailure)); + }, listener::onFailure)); + }, listener::onFailure)); } /** diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 77ea71eafc..9d49ce658b 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -397,6 +397,26 @@ public void fetchAllResourceSharingRecords(String resourceIndex, String resource * */ public void fetchAccessibleResourceIds(String resourceIndex, Set entities, ActionListener> listener) { + fetchAccessibleResourceIds(resourceIndex, null, entities, listener); + } + + /** + * Fetches accessible resource IDs from the resource index, optionally filtered by resource type. + * Queries the {@code all_shared_principals} denormalized field and, when {@code resourceType} is + * provided, also filters on the {@code resource_type} field so that only IDs belonging to that + * type are returned (important when multiple types share the same index). + * + * @param resourceIndex the resource index to search + * @param resourceType optional resource type filter; pass {@code null} to return all types + * @param entities flat principals to match against {@code all_shared_principals} + * @param listener listener to receive the set of matching resource IDs + */ + public void fetchAccessibleResourceIds( + String resourceIndex, + String resourceType, + Set entities, + ActionListener> listener + ) { final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { @@ -407,6 +427,9 @@ public void fetchAccessibleResourceIds(String resourceIndex, Set entitie // We match any doc whose "principals" contains at least one of the entities // e.g., "user:alice", "role:admin", "backend:ops" BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().filter(QueryBuilders.termsQuery("all_shared_principals", entities)); + if (resourceType != null) { + boolQuery.filter(QueryBuilders.termQuery("resource_type", resourceType)); + } executeIdCollectingSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { ctx.restore(); @@ -424,6 +447,45 @@ public void fetchAccessibleResourceIds(String resourceIndex, Set entitie } } + /** + * Fetches child resource IDs from the sharing index whose {@code parent_id} field matches any of the given parent IDs. + * Used to resolve parent-inherited access: if a user has access to a parent resource, they implicitly have access + * to all child resources that reference that parent. + * + * @param resourceIndex the child resource index (sharing index is derived from this) + * @param parentIds the set of accessible parent resource IDs + * @param listener listener to receive the set of child resource IDs + */ + public void fetchResourceIdsByParentIds(String resourceIndex, Set parentIds, ActionListener> listener) { + if (parentIds == null || parentIds.isEmpty()) { + listener.onResponse(Collections.emptySet()); + return; + } + final String resourceSharingIndex = getSharingIndex(resourceIndex); + final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); + + try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { + SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); + searchRequest.scroll(scroll); + + BoolQueryBuilder query = QueryBuilders.boolQuery().filter(QueryBuilders.termsQuery("parent_id.keyword", parentIds)); + + executeSearchRequest(scroll, searchRequest, query, ActionListener.wrap(resourceIds -> { + ctx.restore(); + LOGGER.debug("Found {} child resources in {} with parent_ids {}", resourceIds.size(), resourceSharingIndex, parentIds); + listener.onResponse(resourceIds); + }, exception -> { + if (exception instanceof IndexNotFoundException) { + LOGGER.debug("Sharing index {} not found, returning empty set", resourceSharingIndex); + listener.onResponse(Collections.emptySet()); + return; + } + LOGGER.error("Search failed for child resources in {}, parentIds={}", resourceSharingIndex, parentIds, exception); + listener.onFailure(exception); + })); + } + } + /** * Executes a search request against the resource index and collects _id values (resource IDs) using scroll. * @@ -922,6 +984,91 @@ private void executeAllSearchRequest( }, listener::onFailure); } + /** + * Fetches resource-sharing records for a given set of resource IDs (pre-resolved). + * Used when the caller has already resolved the full set of accessible IDs (e.g. after parent-hierarchy union). + * + * @param resourceIndex the resource index + * @param resourceType the resource type + * @param user the requesting user (used for can_share check) + * @param resourceIds the pre-resolved set of accessible resource IDs + * @param listener listener to collect and return the sharing records + */ + public void fetchResourceSharingRecordsByIds( + String resourceIndex, + String resourceType, + User user, + Set resourceIds, + ActionListener> listener + ) { + if (resourceIds == null || resourceIds.isEmpty()) { + listener.onResponse(Collections.emptySet()); + return; + } + + final String resourceSharingIndex = getSharingIndex(resourceIndex); + final ThreadContext.StoredContext stored = this.threadPool.getThreadContext().stashContext(); + + final List idList = new ArrayList<>(resourceIds); + final int BATCH = 1000; + final Set out = ConcurrentHashMap.newKeySet(); + final AtomicInteger cursor = new AtomicInteger(0); + final String[] includes = { "resource_id", "resource_type", "created_by", "share_with" }; + + final AtomicReference submitNextRef = new AtomicReference<>(); + submitNextRef.set(() -> { + int start = cursor.getAndAdd(BATCH); + if (start >= idList.size()) { + stored.restore(); + listener.onResponse(out); + return; + } + int end = Math.min(start + BATCH, idList.size()); + + final MultiGetRequest mget = new MultiGetRequest(); + final FetchSourceContext fsc = new FetchSourceContext(true, includes, Strings.EMPTY_ARRAY); + for (int i = start; i < end; i++) { + mget.add(new MultiGetRequest.Item(resourceSharingIndex, idList.get(i)).fetchSourceContext(fsc)); + } + + client.multiGet(mget, ActionListener.wrap(mres -> { + for (MultiGetItemResponse item : mres.getResponses()) { + if (item == null || item.isFailed()) continue; + final GetResponse gr = item.getResponse(); + if (gr == null || !gr.isExists()) continue; + + try ( + XContentParser p = XContentHelper.createParser( + NamedXContentRegistry.EMPTY, + THROW_UNSUPPORTED_OPERATION, + gr.getSourceAsBytesRef(), + XContentType.JSON + ) + ) { + p.nextToken(); + ResourceSharing rs = ResourceSharing.fromXContent(p); + // skip records belonging to a different resource type (can happen when multiple + // types share the same resource index) + if (!resourceType.equals(rs.getResourceType())) continue; + boolean canShare = canUserShare(user, /* isAdmin */ false, rs, resourceType); + out.add(new SharingRecord(rs, canShare)); + } catch (Exception ex) { + LOGGER.warn("Failed to parse resource-sharing doc id={}", gr.getId(), ex); + } + } + submitNextRef.get().run(); + }, e -> { + try { + listener.onFailure(e); + } finally { + stored.restore(); + } + })); + }); + + submitNextRef.get().run(); + } + /** * Fetches resource-sharing records for this user for a given resource-index. * Executes in 2 steps: @@ -958,7 +1105,7 @@ public void fetchAccessibleResourceSharingRecords( final int BATCH = 1000; // tune if docs are large final Set out = ConcurrentHashMap.newKeySet(); final AtomicInteger cursor = new AtomicInteger(0); - final String[] includes = { "resource_id", "created_by", "share_with" }; + final String[] includes = { "resource_id", "resource_type", "created_by", "share_with" }; // self-referencing lambda for batch run final AtomicReference submitNextRef = new AtomicReference<>(); @@ -996,6 +1143,8 @@ public void fetchAccessibleResourceSharingRecords( ) { p.nextToken(); ResourceSharing rs = ResourceSharing.fromXContent(p); + // skip records belonging to a different resource type + if (!resourceType.equals(rs.getResourceType())) continue; boolean canShare = canUserShare(user, /* isAdmin */ false, rs, resourceType); out.add(new SharingRecord(rs, canShare)); } catch (Exception ex) { diff --git a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java index 77bf9e88d5..6a93156301 100644 --- a/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java +++ b/src/main/java/org/opensearch/security/resources/sharing/ResourceSharing.java @@ -113,6 +113,10 @@ public ShareWith getShareWith() { return shareWith; } + public String getResourceType() { + return resourceType; + } + public String getParentType() { return parentType; } From 24cc2d0966115d9ebaba155f4d1ec8ef76bf98ac Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sat, 21 Mar 2026 21:58:48 -0400 Subject: [PATCH 17/17] Handle missing sharing record Signed-off-by: Craig Perkins --- .../ResourceSharingIndexHandler.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 9d49ce658b..d9a069b53c 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -705,8 +705,15 @@ public void share(String resourceId, String resourceIndex, ShareWith shareWith, // build update script sharingInfoListener.whenComplete(sharingInfo -> { if (sharingInfo == null) { - LOGGER.debug("No sharing record found for resource {}", resourceId); - listener.onResponse(null); + LOGGER.warn("No sharing record found for resource {} in index {}", resourceId, resourceIndex); + listener.onFailure( + new OpenSearchStatusException( + "No sharing record found for resource [" + + resourceId + + "]. The resource may not exist or sharing has not been initialized yet.", + RestStatus.NOT_FOUND + ) + ); return; } for (String accessLevel : shareWith.accessLevels()) { @@ -780,6 +787,18 @@ public void patchSharingInfo( // Apply patch and update the document sharingInfoListener.whenComplete(sharingInfo -> { + if (sharingInfo == null) { + LOGGER.warn("No sharing record found for resource {} in index {}", resourceId, resourceIndex); + listener.onFailure( + new OpenSearchStatusException( + "No sharing record found for resource [" + + resourceId + + "]. The resource may not exist or sharing has not been initialized yet.", + RestStatus.NOT_FOUND + ) + ); + return; + } if (add != null) { sharingInfo.getShareWith().add(add); }