From 859a83b43d7f95d61f15971c563e135194dca1e3 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 6 Aug 2026 20:44:23 +0000 Subject: [PATCH 1/4] Create parent-linked sharing entries for child resources written without a user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ResourceIndexListener.postIndex previously required an authenticated user in the thread context to create a resource-sharing entry, and skipped silently (debug log, or NPE on a null subject) when one was absent. Writes performed under a plugin or system subject — e.g. reporting's on-demand report instances indexed via PluginClient, or scheduled jobs running under job-scheduler — therefore never received sharing entries, leaving those resources permanently invisible to the resource-sharing APIs, including to their creators. With this change: - The user subject is extracted null-safely. - When no user is present and the resource's provider declares a parent (parentType/parentIdField), the sharing entry is created by inheriting tenant and created_by from the parent's sharing record, linked via parentType/parentId so access evaluation delegates to the parent. - When no user is present and no parent is declared, the skip is now logged at WARN instead of silently at debug, making this failure mode visible to operators. - Failures to index sharing entries are also logged at WARN instead of debug. Companion change: opensearch-project/reporting declares report-instance as a child of report-definition to use this path. Signed-off-by: Darshit Chanpura --- .../resources/ResourceIndexListener.java | 109 ++++++++++++++---- 1 file changed, 84 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index 91b47b23cc..78d165d65d 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -9,7 +9,6 @@ package org.opensearch.security.resources; import java.io.IOException; -import java.util.Objects; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -107,36 +106,96 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re final UserSubjectImpl userSubject = (UserSubjectImpl) threadPool.getThreadContext() .getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER); - final User user = userSubject.getUser(); + final User user = (userSubject == null) ? null : userSubject.getUser(); - try { - Objects.requireNonNull(user); - ActionListener listener = ActionListener.wrap(entry -> { - log.debug( - "postIndex: Successfully created a resource sharing entry {} for resource {} within index {}", - entry, + final String parentType = provider.parentType(); + final String parentId = (parentType != null) ? ResourcePluginInfo.extractFieldFromIndexOp(provider.parentIdField(), index) : null; + + ActionListener listener = ActionListener.wrap( + entry -> log.debug( + "postIndex: Successfully created a resource sharing entry {} for resource {} within index {}", + entry, + resourceId, + resourceIndex + ), + e -> log.warn("postIndex: Failed to create a resource sharing entry for resource {}: {}", resourceId, e.getMessage()) + ); + + if (user != null) { + try { + // User.getRequestedTenant() is null if multi-tenancy is disabled + ResourceSharing.Builder builder = ResourceSharing.builder() + .resourceId(resourceId) + .resourceType(resourceType) + .tenant(user.getRequestedTenant()) + .createdBy(new CreatedBy(user.getName())); + if (parentType != null) { + builder.parentType(parentType).parentId(parentId); + } + this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, builder.build(), listener); + } catch (IOException e) { + log.warn("Failed to create a resource sharing entry for resource: {}", resourceId, e); + } + return; + } + + // No authenticated user in the thread context. This happens when the + // resource is written under a plugin/system subject (e.g. reporting's + // on-demand report instances via PluginClient, or scheduled jobs running + // under job-scheduler). For child resources we can still create the + // sharing entry by inheriting ownership from the parent's sharing record; + // for standalone resources there is nothing to attribute the entry to. + if (parentType == null || parentId == null) { + log.warn( + "Skipping resource-sharing entry creation for resource {} in index {}: no authenticated user found in the thread " + + "context and the resource does not declare a parent to inherit ownership from. The resource will not be " + + "visible through resource-sharing APIs.", + resourceId, + resourceIndex + ); + return; + } + + final String parentResourceIndex = resourcePluginInfo.indexByType(parentType); + if (parentResourceIndex == null) { + log.warn( + "Skipping resource-sharing entry creation for resource {} in index {}: parent type {} has no registered resource index.", + resourceId, + resourceIndex, + parentType + ); + return; + } + + this.resourceSharingIndexHandler.fetchSharingInfo(parentResourceIndex, parentId, ActionListener.wrap(parentSharing -> { + if (parentSharing == null) { + log.warn( + "Skipping resource-sharing entry creation for resource {} in index {}: no sharing record found for parent {} ({}).", resourceId, - resourceIndex + resourceIndex, + parentId, + parentType ); - }, e -> { log.debug(e.getMessage()); }); - // User.getRequestedTenant() is null if multi-tenancy is disabled - ResourceSharing.Builder builder = ResourceSharing.builder() + return; + } + ResourceSharing sharingInfo = ResourceSharing.builder() .resourceId(resourceId) .resourceType(resourceType) - .tenant(user.getRequestedTenant()) - .createdBy(new CreatedBy(user.getName())); - if (provider.parentType() != null) { - builder.parentType(provider.parentType()) - .parentId(ResourcePluginInfo.extractFieldFromIndexOp(provider.parentIdField(), index)); - } - ResourceSharing sharingInfo = builder.build(); - // User.getRequestedTenant() is null if multi-tenancy is disabled - + .tenant(parentSharing.getTenant()) + .createdBy(parentSharing.getCreatedBy()) + .parentType(parentType) + .parentId(parentId) + .build(); this.resourceSharingIndexHandler.indexResourceSharing(resourceIndex, sharingInfo, listener); - - } catch (IOException e) { - log.debug("Failed to create a resource sharing entry for resource: {}", resourceId, e); - } + }, + e -> log.warn( + "Failed to create a resource sharing entry for child resource {} in index {}: could not fetch parent {} sharing record: {}", + resourceId, + resourceIndex, + parentId, + e.getMessage() + ) + )); } /** From a34fb5046c6622c94660c509ddc0b1fbf0bdae99 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 6 Aug 2026 20:58:28 +0000 Subject: [PATCH 2/4] Add integration tests for user-less child-resource sharing entries Verifies via the sample plugin hierarchy that resources indexed without an authenticated user in the thread context (internal node client, mirroring plugin-subject writes): - child resources receive a parent-linked sharing entry inheriting the parent owner, and parent-level shares grant access to them - parent-less resources are skipped (no entry created) - children referencing a missing parent record are skipped Signed-off-by: Darshit Chanpura --- .../SystemContextChildResourceTests.java | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java new file mode 100644 index 0000000000..b1f7dce513 --- /dev/null +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java @@ -0,0 +1,151 @@ +/* + * 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 java.time.Duration; + +import com.carrotsearch.randomizedtesting.RandomizedRunner; +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.awaitility.Awaitility; +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.runner.RunWith; + +import org.opensearch.action.index.IndexRequest; +import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.support.WriteRequest; +import org.opensearch.common.xcontent.XContentType; +import org.opensearch.sample.resource.TestUtils; +import org.opensearch.test.framework.cluster.LocalCluster; +import org.opensearch.test.framework.cluster.TestRestClient; +import org.opensearch.transport.client.Client; + +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.RESOURCE_SHARING_INDEX; +import static org.opensearch.sample.resource.TestUtils.SAMPLE_GROUP_READ_ONLY; +import static org.opensearch.sample.resource.TestUtils.newCluster; +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.api.AbstractApiIntegrationTest.forbidden; +import static org.opensearch.security.api.AbstractApiIntegrationTest.ok; +import static org.opensearch.test.framework.TestSecurityConfig.User.USER_ADMIN; + +/** + * Tests sharing-entry creation for resources written WITHOUT an authenticated + * user in the thread context — the situation plugins are in when they index + * resource documents under a plugin/system subject (e.g. reporting's + * on-demand report instances via PluginClient, or scheduled jobs running + * under job-scheduler). + * + * Child resources (provider declares parentType/parentIdField) must still + * receive a sharing entry, inheriting ownership from the parent's record. + * Parent-less resources cannot be attributed and must be skipped. + */ +@RunWith(RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class SystemContextChildResourceTests { + + @ClassRule + public static LocalCluster cluster = newCluster(true, true); + + private final TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster); + private String resourceGroupId; + + @Before + public void setup() { + resourceGroupId = api.createSampleResourceGroupAs(USER_ADMIN); + api.awaitSharingEntry(resourceGroupId); // parent sharing entry exists + } + + @After + public void cleanup() { + api.wipeOutResourceEntries(); + } + + /** Indexes a resource document via the internal node client: no authenticated user in context. */ + private String indexResourceWithoutUser(String docJson) { + Client client = cluster.getInternalNodeClient(); + IndexRequest request = new IndexRequest(RESOURCE_INDEX_NAME).source(docJson, XContentType.JSON) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE); + IndexResponse response = client.index(request).actionGet(); + return response.getId(); + } + + @Test + public void testChildResourceWrittenWithoutUserInheritsParentSharing() throws Exception { + String childId = indexResourceWithoutUser( + "{\"group_id\":\"" + resourceGroupId + "\", \"name\":\"system-created\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}" + ); + + // A sharing entry must be created despite the absent user, attributed + // to the parent's owner and linked to the parent. + api.awaitSharingEntry(childId, USER_ADMIN.getName()); + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + TestRestClient.HttpResponse entry = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + childId); + entry.assertStatusCode(200); + assertThat(entry.getBody(), containsString("\"parent_id\":\"" + resourceGroupId + "\"")); + assertThat(entry.getBody(), containsString("\"parent_type\":\"" + RESOURCE_GROUP_TYPE + "\"")); + assertThat(entry.getBody(), containsString(USER_ADMIN.getName())); + } + + // Owner of the parent has access to the child through the inherited entry + ok(() -> api.getResource(childId, USER_ADMIN)); + + // A user without any share sees neither parent nor child + forbidden(() -> api.getResource(childId, FULL_ACCESS_USER)); + + // Sharing the parent group grants access to the system-created child + ok(() -> api.shareResourceGroup(resourceGroupId, USER_ADMIN, FULL_ACCESS_USER, SAMPLE_GROUP_READ_ONLY)); + ok(() -> api.getResource(childId, FULL_ACCESS_USER)); + } + + @Test + public void testParentlessResourceWrittenWithoutUserIsSkipped() throws Exception { + // A resource-group has no parent declared: with no user in context there + // is nothing to attribute the sharing entry to, so none must be created. + String orphanId = indexResourceWithoutUser( + "{\"name\":\"system-created-group\",\"resource_type\":\"" + RESOURCE_GROUP_TYPE + "\"}" + ); + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + Awaitility.await("sharing entry must not appear for parent-less user-less resource " + orphanId) + .pollDelay(Duration.ofSeconds(2)) + .pollInterval(Duration.ofMillis(500)) + .atMost(Duration.ofSeconds(4)) + .untilAsserted(() -> { + TestRestClient.HttpResponse response = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + orphanId); + response.assertStatusCode(404); + }); + } + } + + @Test + public void testChildResourceWithMissingParentRecordIsSkipped() throws Exception { + // Child pointing at a non-existent parent: no record to inherit from. + String childId = indexResourceWithoutUser( + "{\"group_id\":\"no-such-group\", \"name\":\"system-created\",\"resource_type\":\"" + RESOURCE_TYPE + "\"}" + ); + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + Awaitility.await("sharing entry must not appear for child with missing parent " + childId) + .pollDelay(Duration.ofSeconds(2)) + .pollInterval(Duration.ofMillis(500)) + .atMost(Duration.ofSeconds(4)) + .untilAsserted(() -> { + TestRestClient.HttpResponse response = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + childId); + response.assertStatusCode(404); + }); + } + } +} From 79f987980b037457565373648c409c0fbe960de6 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 20 Aug 2026 13:39:53 -0400 Subject: [PATCH 3/4] fix: Spotless Signed-off-by: Darshit Chanpura --- .../sample/resourcegroup/SystemContextChildResourceTests.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java index b1f7dce513..192f28c16d 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java @@ -114,9 +114,7 @@ public void testChildResourceWrittenWithoutUserInheritsParentSharing() throws Ex public void testParentlessResourceWrittenWithoutUserIsSkipped() throws Exception { // A resource-group has no parent declared: with no user in context there // is nothing to attribute the sharing entry to, so none must be created. - String orphanId = indexResourceWithoutUser( - "{\"name\":\"system-created-group\",\"resource_type\":\"" + RESOURCE_GROUP_TYPE + "\"}" - ); + String orphanId = indexResourceWithoutUser("{\"name\":\"system-created-group\",\"resource_type\":\"" + RESOURCE_GROUP_TYPE + "\"}"); try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { Awaitility.await("sharing entry must not appear for parent-less user-less resource " + orphanId) From 9d27c4fbca36257e26fefed07e17ec24c00abd6a Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 20 Aug 2026 23:04:13 -0400 Subject: [PATCH 4/4] Correct class doc: on-demand report instances carry the user, not user-less The SystemContextChildResourceTests class comment cited reporting's on-demand report instances as an example of a user-less write. They actually stash-then- restore the caller's context (PluginBaseAction), so the authenticated user is present when the instance is indexed and postIndex attributes it normally -- matching this PR's investigation-note correction. Update the doc to cite genuinely user-less writes (scheduled jobs under job-scheduler, system/ provisioning-context) and note the on-demand distinction. Signed-off-by: Darshit Chanpura --- .../resourcegroup/SystemContextChildResourceTests.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java index 192f28c16d..904c4d3f31 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resourcegroup/SystemContextChildResourceTests.java @@ -44,9 +44,13 @@ /** * Tests sharing-entry creation for resources written WITHOUT an authenticated * user in the thread context — the situation plugins are in when they index - * resource documents under a plugin/system subject (e.g. reporting's - * on-demand report instances via PluginClient, or scheduled jobs running - * under job-scheduler). + * resource documents under a genuinely user-less subject, e.g. scheduled jobs + * running under job-scheduler or system/provisioning-context writes. + * + * (Note: request-driven writes that stash-then-restore the caller's context — + * such as reporting's on-demand report instances — do carry the authenticated + * user, so postIndex attributes them normally; they are not the user-less case + * exercised here.) * * Child resources (provider declares parentType/parentIdField) must still * receive a sharing entry, inheriting ownership from the parent's record.