From f25ac342398090f4b4cbb796fecb91596ab545e5 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 26 Aug 2025 15:54:45 -0400 Subject: [PATCH 01/27] WIP on tracking principals on the resource doc Signed-off-by: Craig Perkins --- .../resources/ResourceIndexListener.java | 11 ++++- .../ResourceSharingIndexHandler.java | 48 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index e861394a86..9922308cfe 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -9,6 +9,7 @@ package org.opensearch.security.resources; import java.io.IOException; +import java.util.List; import java.util.Objects; import org.apache.logging.log4j.LogManager; @@ -54,7 +55,7 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re String resourceId = index.id(); // Only proceed if this was a create operation and for primary shard - if (!result.isCreated() && index.origin().equals(Engine.Operation.Origin.PRIMARY)) { + if (!result.isCreated() || !index.origin().equals(Engine.Operation.Origin.PRIMARY)) { log.debug("Skipping resource sharing entry creation as this was an update operation for resource {}", resourceId); return; } @@ -72,6 +73,14 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re resourceId, resourceIndex ); + this.resourceSharingIndexHandler.updateResourceVisibility( + resourceId, + resourceIndex, + List.of("user:" + user.getName()), + ActionListener.wrap((updateResponse) -> { + log.debug("postUpdate: Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); + }, (e) -> { log.debug(e.getMessage()); }) + ); }, e -> { log.debug(e.getMessage()); }); this.resourceSharingIndexHandler.indexResourceSharing(resourceId, resourceIndex, new CreatedBy(user.getName()), null, listener); diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 96e89cae7b..c9c6720d69 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -35,6 +36,8 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.action.search.SearchScrollRequest; import org.opensearch.action.support.WriteRequest; +import org.opensearch.action.update.UpdateRequest; +import org.opensearch.action.update.UpdateResponse; import org.opensearch.common.inject.Inject; import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; @@ -135,6 +138,51 @@ public static String getSharingIndex(String resourceIndex) { return resourceIndex + "-sharing"; } + /** + * Updates the visibility of a resource document by replacing its {@code principals} field + * with the provided list of principals. The update is executed immediately with + * {@link WriteRequest.RefreshPolicy#IMMEDIATE} to ensure the change is visible in subsequent + * searches. + *

+ * The supplied {@link ActionListener} will be invoked with the {@link UpdateResponse} + * on success, or with an exception on failure. + * + * @param resourceId the unique identifier of the resource document to update + * @param resourceIndex the name of the index containing the resource + * @param principals the list of principals (e.g. {@code user:alice}, {@code role:admin}) + * that should be assigned to the resource + * @param listener callback that will be notified with the update response or an error + */ + public void updateResourceVisibility( + String resourceId, + String resourceIndex, + List principals, + ActionListener listener + ) { + try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { + UpdateRequest ur = client.prepareUpdate(resourceIndex, resourceId) + .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) + .setDoc(Map.of("principals", principals)) + .setId(resourceId) + .request(); + + ActionListener urListener = ActionListener.wrap(response -> { + ctx.restore(); + LOGGER.info( + "Successfully updated visibility of resource {} in index {} to principals {}.", + resourceIndex, + resourceId, + principals + ); + listener.onResponse(response); + }, (e) -> { + LOGGER.error("Failed to update visibility in [{}] for resource [{}]", resourceIndex, resourceId, e); + listener.onFailure(e); + }); + client.update(ur, urListener); + } + } + /** * Creates or updates a resource sharing record in the dedicated resource sharing index. * This method handles the persistence of sharing metadata for resources, including From 746b239fe78509e848d510f619c1feb0429c8831 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 26 Aug 2025 21:16:31 -0400 Subject: [PATCH 02/27] Update resource visibility in indexResourceSharing Signed-off-by: Craig Perkins --- .../resources/ResourceIndexListener.java | 9 --------- .../resources/ResourceSharingIndexHandler.java | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index 9922308cfe..765320eec2 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.List; import java.util.Objects; import org.apache.logging.log4j.LogManager; @@ -73,14 +72,6 @@ public void postIndex(ShardId shardId, Engine.Index index, Engine.IndexResult re resourceId, resourceIndex ); - this.resourceSharingIndexHandler.updateResourceVisibility( - resourceId, - resourceIndex, - List.of("user:" + user.getName()), - ActionListener.wrap((updateResponse) -> { - log.debug("postUpdate: Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); - }, (e) -> { log.debug(e.getMessage()); }) - ); }, e -> { log.debug(e.getMessage()); }); this.resourceSharingIndexHandler.indexResourceSharing(resourceId, resourceIndex, new CreatedBy(user.getName()), null, listener); diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index c9c6720d69..fa889a650f 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -225,7 +225,22 @@ public void indexResourceSharing( ActionListener irListener = ActionListener.wrap(idxResponse -> { ctx.restore(); LOGGER.info("Successfully created {} entry for resource {} in index {}.", resourceSharingIndex, resourceId, resourceIndex); - listener.onResponse(entry); + updateResourceVisibility( + resourceId, + resourceIndex, + List.of("user:" + createdBy.getUsername()), + ActionListener.wrap((updateResponse) -> { + LOGGER.debug( + "postUpdate: Successfully updated visibility for resource {} within index {}", + resourceId, + resourceIndex + ); + listener.onResponse(entry); + }, (e) -> { + LOGGER.error("Failed to create principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); + listener.onResponse(entry); + }) + ); }, (e) -> { if (ExceptionsHelper.unwrapCause(e) instanceof VersionConflictEngineException) { // already exists → skipping From 3b1f0fd60044d5a71db114239f0a2508184d13f4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 26 Aug 2025 21:33:51 -0400 Subject: [PATCH 03/27] Update visibility when sharing Signed-off-by: Craig Perkins --- .../resources/sharing/ResourceSharing.java | 45 +++++++++++++++++++ .../ResourceSharingIndexHandler.java | 13 +++++- 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java index 30cfb146d4..a7c25241b1 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/sharing/ResourceSharing.java @@ -9,8 +9,10 @@ package org.opensearch.security.spi.resources.sharing; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -267,4 +269,47 @@ public Set fetchAccessLevels(Recipient recipientType, Set entiti } return matchingGroups; } + + /** + * Returns all principals (users, roles, backend_roles) that have access to this resource, + * including the creator and all shared recipients, formatted with appropriate prefixes. + * + * @return List of principals in format ["user:username", "role:rolename", "backend:backend_role"] + */ + public List getAllPrincipals() { + List principals = new ArrayList<>(); + + // Add creator + if (createdBy != null) { + principals.add("user:" + createdBy.getUsername()); + } + + // Add shared recipients + if (shareWith != null) { + // shared with at any access level + for (Recipients recipients : shareWith.getSharingInfo().values()) { + Map> recipientMap = recipients.getRecipients(); + + // Add users + Set users = recipientMap.getOrDefault(Recipient.USERS, Collections.emptySet()); + for (String user : users) { + principals.add("user:" + user); + } + + // Add roles + Set roles = recipientMap.getOrDefault(Recipient.ROLES, Collections.emptySet()); + for (String role : roles) { + principals.add("role:" + role); + } + + // Add backend roles + Set backendRoles = recipientMap.getOrDefault(Recipient.BACKEND_ROLES, Collections.emptySet()); + for (String backendRole : backendRoles) { + principals.add("backend:" + backendRole); + } + } + } + + return principals; + } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index fa889a650f..828182e13f 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -534,7 +534,18 @@ public void share(String resourceId, String resourceIndex, ShareWith shareWith, resourceId, resourceIndex ); - listener.onResponse(sharingInfo); + updateResourceVisibility( + resourceId, + resourceIndex, + sharingInfo.getAllPrincipals(), + ActionListener.wrap((updateResponse) -> { + LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); + listener.onResponse(sharingInfo); + }, (e) -> { + LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); + listener.onResponse(sharingInfo); + }) + ); }, (failResponse) -> { LOGGER.error(failResponse.getMessage()); listener.onFailure(failResponse); From 0bfc778df03ac0e5dcea82ad5253221d07dc76cc Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 26 Aug 2025 21:35:12 -0400 Subject: [PATCH 04/27] Update in revoke as well Signed-off-by: Craig Perkins --- .../resources/ResourceSharingIndexHandler.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 828182e13f..0bff18016c 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -634,7 +634,18 @@ public void revoke(String resourceId, String resourceIndex, ShareWith revokeAcce ActionListener irListener = ActionListener.wrap(idxResponse -> { ctx.restore(); LOGGER.info("Successfully revoked access of {} to resource {} in index {}.", revokeAccess, resourceId, resourceIndex); - listener.onResponse(sharingInfo); + updateResourceVisibility( + resourceId, + resourceIndex, + sharingInfo.getAllPrincipals(), + ActionListener.wrap((updateResponse) -> { + LOGGER.debug("Successfully updated visibility for resource {} within index {}", resourceId, resourceIndex); + listener.onResponse(sharingInfo); + }, (e) -> { + LOGGER.error("Failed to update principals field in [{}] for resource [{}]", resourceIndex, resourceId, e); + listener.onResponse(sharingInfo); + }) + ); }, (failResponse) -> { LOGGER.error(failResponse.getMessage()); listener.onFailure(failResponse); From 4aa049bf318a96b236919538a7a67ea4386978b6 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 27 Aug 2025 15:43:19 -0400 Subject: [PATCH 05/27] Keep track of list of principals for which sharable resource is visible for searching Signed-off-by: Craig Perkins --- .../sample/resource/MigrateApiTests.java | 2 - .../resource/SecurityDisabledTests.java | 2 - .../opensearch/sample/resource/TestUtils.java | 2 - .../sample/secure/SecurePluginTests.java | 2 - .../SearchResourceTransportAction.java | 1 + .../ResourceSharingIndexHandler.java | 149 ++++++++++-------- 6 files changed, 82 insertions(+), 76 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/MigrateApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/MigrateApiTests.java index f9b9780999..fb6f92276b 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/MigrateApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/MigrateApiTests.java @@ -26,7 +26,6 @@ import org.junit.runner.RunWith; import org.opensearch.Version; -import org.opensearch.painless.PainlessModulePlugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.sample.SampleResourcePlugin; import org.opensearch.security.OpenSearchSecurityPlugin; @@ -64,7 +63,6 @@ public class MigrateApiTests { @ClassRule public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.DEFAULT) - .plugin(PainlessModulePlugin.class) .plugin( new PluginInfo( SampleResourcePlugin.class.getName(), diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/SecurityDisabledTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/SecurityDisabledTests.java index 2b7aa6c4b4..3c9379d012 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/SecurityDisabledTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/SecurityDisabledTests.java @@ -19,7 +19,6 @@ import org.junit.runner.RunWith; import org.opensearch.Version; -import org.opensearch.painless.PainlessModulePlugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.sample.SampleResourcePlugin; import org.opensearch.security.OpenSearchSecurityPlugin; @@ -65,7 +64,6 @@ public class SecurityDisabledTests { false ) ) - .plugin(PainlessModulePlugin.class) .loadConfigurationIntoIndex(false) .nodeSettings(Map.of("plugins.security.disabled", true, "plugins.security.ssl.http.enabled", false)) .build(); 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 cd6977e711..36126aa463 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 @@ -22,7 +22,6 @@ import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.painless.PainlessModulePlugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.sample.SampleResourcePlugin; import org.opensearch.security.OpenSearchSecurityPlugin; @@ -112,7 +111,6 @@ public static LocalCluster newCluster(boolean featureEnabled, boolean systemInde false ) ) - .plugin(PainlessModulePlugin.class) .anonymousAuth(true) .authc(AUTHC_HTTPBASIC_INTERNAL) .users(USER_ADMIN, FULL_ACCESS_USER, LIMITED_ACCESS_USER, NO_ACCESS_USER) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java index ff88739d5c..ae33cada04 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/secure/SecurePluginTests.java @@ -19,7 +19,6 @@ import org.opensearch.Version; import org.opensearch.core.rest.RestStatus; -import org.opensearch.painless.PainlessModulePlugin; import org.opensearch.plugins.PluginInfo; import org.opensearch.sample.SampleResourcePlugin; import org.opensearch.security.OpenSearchSecurityPlugin; @@ -47,7 +46,6 @@ public class SecurePluginTests { .anonymousAuth(false) .authc(AUTHC_DOMAIN) .users(USER_ADMIN) - .plugin(PainlessModulePlugin.class) .plugin( new PluginInfo( SampleResourcePlugin.class.getName(), diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java index 0add62ba8d..f29ef3e221 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java @@ -70,6 +70,7 @@ protected void doExecute(Task task, SearchRequest request, ActionListener listener) { SearchSourceBuilder src = request.source() != null ? request.source() : new SearchSourceBuilder(); ActionListener> idsListener = ActionListener.wrap(resourceIds -> { + System.out.println("resourceIds to filter: " + resourceIds); mergeAccessibleFilter(src, resourceIds); request.source(src); nodeClient.search(request, listener); diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 0bff18016c..3ec62f0e8a 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -53,8 +53,6 @@ import org.opensearch.index.query.BoolQueryBuilder; import org.opensearch.index.query.MatchAllQueryBuilder; import org.opensearch.index.query.QueryBuilders; -import org.opensearch.script.Script; -import org.opensearch.script.ScriptType; import org.opensearch.search.Scroll; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; @@ -339,28 +337,100 @@ public void fetchAccessibleResourceIds( ActionListener> listener ) { final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); - String resourceSharingIndex = getSharingIndex(resourceIndex); + try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); + // Search the RESOURCE INDEX directly (not the *-sharing index) + SearchRequest searchRequest = new SearchRequest(resourceIndex); searchRequest.scroll(scroll); - BoolQueryBuilder boolQuery = QueryBuilders.boolQuery(); - - boolQuery.must(actionGroupQuery); + // 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("principals.keyword", entities)); - executeFlattenedSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { + executeIdCollectingSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { ctx.restore(); - LOGGER.debug("Found {} documents matching the criteria in {}", resourceIds.size(), resourceSharingIndex); + LOGGER.debug("Found {} accessible resources in {}", resourceIds.size(), resourceIndex); listener.onResponse(resourceIds); - }, exception -> { LOGGER.error("Search failed for resourceIndex={}, entities={}", resourceIndex, entities, exception); listener.onFailure(exception); - })); } } + /** + * Executes a search request against the resource index and collects _id values (resource IDs) using scroll. + * + * @param scroll Search scroll context + * @param searchRequest Initial search request + * @param query Query builder for the request + * @param listener Listener to receive the collected resource IDs + */ + private void executeIdCollectingSearchRequest( + Scroll scroll, + SearchRequest searchRequest, + AbstractQueryBuilder> query, + ActionListener> listener + ) { + SearchSourceBuilder ssb = new SearchSourceBuilder().query(query).size(1000).fetchSource(false); // we only need _id + + searchRequest.source(ssb); + + StepListener searchStep = new StepListener<>(); + client.search(searchRequest, searchStep); + + searchStep.whenComplete(initialResponse -> { + Set collectedResourceIds = new HashSet<>(); + String scrollId = initialResponse.getScrollId(); + processScrollIds(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); + }, listener::onFailure); + } + + /** + * Recursively processes scroll results and collects hit IDs. + * + * @param collectedResourceIds Internal accumulator for resource IDs + * @param scroll Scroll context + * @param scrollId Scroll ID + * @param hits Search hits + * @param listener Listener to receive final set of resource IDs + */ + private void processScrollIds( + Set collectedResourceIds, + Scroll scroll, + String scrollId, + SearchHit[] hits, + ActionListener> listener + ) { + if (hits == null || hits.length == 0) { + clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onResponse(collectedResourceIds), listener::onFailure)); + return; + } + + for (SearchHit hit : hits) { + // Resource ID is the document _id in the resource index + collectedResourceIds.add(hit.getId()); + } + + SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); + client.searchScroll( + scrollRequest, + ActionListener.wrap( + scrollResponse -> processScrollIds( + collectedResourceIds, + scroll, + scrollResponse.getScrollId(), + scrollResponse.getHits().getHits(), + listener + ), + e -> clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onFailure(e), ex -> { + e.addSuppressed(ex); + listener.onFailure(e); + })) + ) + ); + } + /** * Fetches a specific resource sharing document by its resource ID and system resourceIndex. * This method performs an exact match search and parses the result into a ResourceSharing object. @@ -831,63 +901,6 @@ private void executeSearchRequest( }, listener::onFailure); } - /** - * Executes a multi-clause query in a flattened fashion to boost performance by almost 20x for large queries. - * This is specifically to replace multi-match queries for wild-card expansions. - * @param scroll Search scroll context - * @param searchRequest Initial search request - * @param filterQuery Query builder for the request - * @param listener Listener to receive the collected resource IDs - */ - private void executeFlattenedSearchRequest( - Scroll scroll, - SearchRequest searchRequest, - BoolQueryBuilder filterQuery, - ActionListener> listener - ) { - // Painless script to emit all share_with principals - String scriptSource = """ - // handle shared - if (params._source.share_with instanceof Map) { - for (def grp : params._source.share_with.values()) { - if (grp.users instanceof List) { - for (u in grp.users) { - emit("user:" + u); - } - } - if (grp.roles instanceof List) { - for (r in grp.roles) { - emit("role:" + r); - } - } - if (grp.backend_roles instanceof List) { - for (b in grp.backend_roles) { - emit("backend:" + b); - } - } - } - } - """; - - Script script = new Script(ScriptType.INLINE, "painless", scriptSource, Map.of()); - - SearchSourceBuilder ssb = new SearchSourceBuilder().derivedField( - "all_shared_principals", // flattened runtime field - "keyword", // type - script - ).query(filterQuery).size(1000).fetchSource(new String[] { "resource_id" }, null); - - searchRequest.source(ssb); - - StepListener searchStep = new StepListener<>(); - client.search(searchRequest, searchStep); - searchStep.whenComplete(initialResponse -> { - Set collectedResourceIds = new HashSet<>(); - String scrollId = initialResponse.getScrollId(); - processScrollResults(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); - }, listener::onFailure); - } - /** * Recursively processes scroll results and collects resource IDs. * From 8bff6f38a6bf3e08e9f827967550c1e3c5b19d5f Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Wed, 27 Aug 2025 15:52:03 -0400 Subject: [PATCH 06/27] Add to CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57bb0d7db8..5b3c02cdbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Enhancements +- [Resource Sharing] Keep track of list of principals for which sharable resource is visible for searching ([#5596](https://github.com/opensearch-project/security/pull/5596)) + ### Bug Fixes -* Added new option skip_users to client cert authenticator (clientcert_auth_domain.http_authenticator.config.skip_users in config.yml)([#4378](https://github.com/opensearch-project/security/pull/5525)) -* [Resource Sharing] Fixes accessible resource ids search by marking created_by.user field as keyword search instead of text ([#5574](https://github.com/opensearch-project/security/pull/5574)) -* [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576)) +- Added new option skip_users to client cert authenticator (clientcert_auth_domain.http_authenticator.config.skip_users in config.yml)([#4378](https://github.com/opensearch-project/security/pull/5525)) +- [Resource Sharing] Fixes accessible resource ids search by marking created_by.user field as keyword search instead of text ([#5574](https://github.com/opensearch-project/security/pull/5574)) +- [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576)) ### Refactoring From fee622bdc90378d1420adc8804e330ad93710a27 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 14:08:09 -0400 Subject: [PATCH 07/27] Rename to all_shared_principals Signed-off-by: Craig Perkins --- .../security/resources/ResourceSharingIndexHandler.java | 5 +++-- 1 file changed, 3 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 3ec62f0e8a..a5ffbfa3e2 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -160,7 +160,7 @@ public void updateResourceVisibility( try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { UpdateRequest ur = client.prepareUpdate(resourceIndex, resourceId) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) - .setDoc(Map.of("principals", principals)) + .setDoc(Map.of("all_shared_principals", principals)) .setId(resourceId) .request(); @@ -345,7 +345,8 @@ public void fetchAccessibleResourceIds( // 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("principals.keyword", entities)); + BoolQueryBuilder boolQuery = QueryBuilders.boolQuery() + .filter(QueryBuilders.termsQuery("all_shared_principals.keyword", entities)); executeIdCollectingSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { ctx.restore(); From 373547e0520f43061ff418a3aa67bb6942e6e9c4 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 16:23:49 -0400 Subject: [PATCH 08/27] WIP on RP DLS solution Signed-off-by: Craig Perkins --- .../opensearch/sample/resource/TestUtils.java | 8 ++ .../transport/GetResourceTransportAction.java | 21 +---- .../SearchResourceTransportAction.java | 82 ++----------------- .../configuration/DlsFlsValveImpl.java | 25 +++++- 4 files changed, 40 insertions(+), 96 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 36126aa463..3436c5f10f 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 @@ -459,6 +459,8 @@ private void assertPostSearch( ) { try (TestRestClient client = cluster.getRestClient(user)) { TestRestClient.HttpResponse response = client.postJson(endpoint, searchPayload); + System.out.println("User: " + user); + System.out.println("Search response: " + response.getBody()); response.assertStatusCode(status); if (status == HttpStatus.SC_OK) { Map hits = (Map) response.bodyAsMap().get("hits"); @@ -466,6 +468,12 @@ private void assertPostSearch( assertThat(response.getBody(), containsString(expectedResourceName)); } } + + try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { + TestRestClient.HttpResponse response = client.postJson(endpoint, searchPayload); + System.out.println("SuperAdmin: " + user); + System.out.println("Search response: " + response.getBody()); + } } public void assertDirectGetAll(TestSecurityConfig.User user, int status, String expectedResourceName) { 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 86931c3b88..096e963fe9 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 @@ -10,7 +10,6 @@ import java.io.IOException; import java.util.Arrays; -import java.util.Collections; import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -20,7 +19,6 @@ import org.opensearch.action.search.SearchRequest; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.common.Nullable; import org.opensearch.common.inject.Inject; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; @@ -72,18 +70,8 @@ protected void doExecute(Task task, GetResourceRequest request, ActionListener listener, ResourceSharingClient client) { - if (client == null) { - fetchResourcesByIds(null, listener); - return; - } - - client.getAccessibleResourceIds(RESOURCE_INDEX_NAME, ActionListener.wrap(ids -> { - if (ids.isEmpty()) { - listener.onResponse(new GetResourceResponse(Collections.emptySet())); - } else { - fetchResourcesByIds(ids, listener); - } - }, listener::onFailure)); + System.out.println("fetchAllResources called"); + fetchResourcesByIds(listener); } private void fetchResourceById(String resourceId, ActionListener listener) { @@ -100,10 +88,9 @@ private void fetchResourceById(String resourceId, ActionListener ids, ActionListener listener) { + private void fetchResourcesByIds(ActionListener listener) { withThreadContext(stashed -> { - SearchSourceBuilder ssb = new SearchSourceBuilder().size(1000) - .query(ids == null ? QueryBuilders.matchAllQuery() : QueryBuilders.idsQuery().addIds(ids.toArray(String[]::new))); + SearchSourceBuilder ssb = new SearchSourceBuilder().size(1000).query(QueryBuilders.matchAllQuery()); SearchRequest req = new SearchRequest(RESOURCE_INDEX_NAME).source(ssb); nodeClient.search(req, ActionListener.wrap(searchResponse -> { diff --git a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java index f29ef3e221..eb4ceea97b 100644 --- a/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java +++ b/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/SearchResourceTransportAction.java @@ -8,8 +8,6 @@ package org.opensearch.sample.resource.actions.transport; -import java.util.Set; - import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -18,20 +16,11 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; import org.opensearch.common.inject.Inject; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; -import org.opensearch.index.query.BoolQueryBuilder; -import org.opensearch.index.query.QueryBuilder; -import org.opensearch.index.query.QueryBuilders; -import org.opensearch.sample.client.ResourceSharingClientAccessor; import org.opensearch.sample.resource.actions.rest.search.SearchResourceAction; -import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.spi.resources.client.ResourceSharingClient; +import org.opensearch.sample.utils.PluginClient; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import org.opensearch.transport.client.Client; - -import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; /** * Transport action for searching sample resources @@ -39,79 +28,18 @@ public class SearchResourceTransportAction extends HandledTransportAction { private static final Logger log = LogManager.getLogger(SearchResourceTransportAction.class); - private final TransportService transportService; - private final Client nodeClient; - private final ResourceSharingClient resourceSharingClient; + private final PluginClient pluginClient; @Inject - public SearchResourceTransportAction(TransportService transportService, ActionFilters actionFilters, Client nodeClient) { + public SearchResourceTransportAction(TransportService transportService, ActionFilters actionFilters, PluginClient pluginClient) { super(SearchResourceAction.NAME, transportService, actionFilters, SearchRequest::new); - this.transportService = transportService; - this.nodeClient = nodeClient; - this.resourceSharingClient = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); + this.pluginClient = pluginClient; } @Override protected void doExecute(Task task, SearchRequest request, ActionListener listener) { - ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { - if (resourceSharingClient == null) { - nodeClient.search(request, listener); - return; - } - // if the resource sharing feature is enabled, we only allow search from documents that requested user has access to - searchFilteredIds(request, listener); - } catch (Exception e) { - log.error("Failed to search resources", e); - listener.onFailure(e); - } - } - - private void searchFilteredIds(SearchRequest request, ActionListener listener) { - SearchSourceBuilder src = request.source() != null ? request.source() : new SearchSourceBuilder(); - ActionListener> idsListener = ActionListener.wrap(resourceIds -> { - System.out.println("resourceIds to filter: " + resourceIds); - mergeAccessibleFilter(src, resourceIds); - request.source(src); - nodeClient.search(request, listener); - }, e -> { - mergeAccessibleFilter(src, Set.of()); - request.source(src); - nodeClient.search(request, listener); - }); - - resourceSharingClient.getAccessibleResourceIds(RESOURCE_INDEX_NAME, idsListener); - } - - private void mergeAccessibleFilter(SearchSourceBuilder src, Set resourceIds) { - QueryBuilder accessQB; - - if (resourceIds == null || resourceIds.isEmpty()) { - // match nothing - accessQB = QueryBuilders.boolQuery().mustNot(QueryBuilders.matchAllQuery()); - } else { - // match only from a provided set of resources - accessQB = QueryBuilders.idsQuery().addIds(resourceIds.toArray(new String[0])); - } - - QueryBuilder existing = src.query(); - if (existing == null) { - // No existing query → just the filter - src.query(QueryBuilders.boolQuery().filter(accessQB)); - return; - } + pluginClient.search(request, listener); - if (existing instanceof BoolQueryBuilder) { - // Reuse existing bool: just add a filter clause - ((BoolQueryBuilder) existing).filter(accessQB); - src.query(existing); - } else { - // Preserve existing scoring by keeping it in MUST, add our filter - BoolQueryBuilder merged = QueryBuilders.boolQuery() - .must(existing) // keep original query semantics/scoring - .filter(accessQB); // filter results - src.query(merged); - } } } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 9762bfdc64..d318ad8a17 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -67,6 +67,7 @@ import org.opensearch.search.internal.SearchContext; import org.opensearch.search.query.QuerySearchResult; import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.privileges.DocumentAllowList; import org.opensearch.security.privileges.PrivilegesEvaluationContext; import org.opensearch.security.privileges.PrivilegesEvaluationException; @@ -139,8 +140,26 @@ public DlsFlsValveImpl( */ @Override public boolean invoke(PrivilegesEvaluationContext context, final ActionListener listener) { - if (HeaderHelper.isInternalOrPluginRequest(threadContext) - || (isClusterPerm(context.getAction()) && !MultiGetAction.NAME.equals(context.getAction()))) { + System.out.println("context.action: " + context.getAction()); + System.out.println("context.request: " + context.getRequest()); + UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER); + System.out.println("userSubject: " + userSubject.getUser()); + System.out.println( + "HeaderHelper.isInternalOrPluginRequest(threadContext): " + HeaderHelper.isInternalOrPluginRequest(threadContext) + ); + if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { + System.out.println("set unrestricted"); + return DlsFilterLevelActionHandler.handle( + context, + IndexToRuleMap.unrestricted(), + listener, + nodeClient, + clusterService, + OpenSearchSecurityPlugin.GuiceHolder.getIndicesService(), + resolver, + threadContext + ); + } else if (isClusterPerm(context.getAction()) && !MultiGetAction.NAME.equals(context.getAction())) { return true; } DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); @@ -152,6 +171,8 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< boolean hasFlsRestrictions = !config.getFieldPrivileges().isUnrestricted(context, resolved); boolean hasFieldMasking = !config.getFieldMasking().isUnrestricted(context, resolved); + System.out.println("hasDlsRestrictions: " + hasDlsRestrictions); + if (!hasDlsRestrictions && !hasFlsRestrictions && !hasFieldMasking) { return true; } From 9cae2465f9cbb96b24e3f5ccc0bec2d939b81963 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 17:21:13 -0400 Subject: [PATCH 09/27] WIP on resource restrictions Signed-off-by: Craig Perkins --- .../configuration/DlsFlsValveImpl.java | 12 +++++- .../privileges/dlsfls/DocumentPrivileges.java | 14 ++++++ .../privileges/dlsfls/IndexToRuleMap.java | 43 +++++++++++++++++++ 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index d318ad8a17..f68fd440a9 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -149,9 +149,19 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< ); if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { System.out.println("set unrestricted"); + DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); + IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); + + IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( + namedXContentRegistry, + userSubject.getUser() + ); + + System.out.println("sharedResourceMap: " + sharedResourceMap); + return DlsFilterLevelActionHandler.handle( context, - IndexToRuleMap.unrestricted(), + sharedResourceMap, listener, nodeClient, clusterService, diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java index 0252fb2772..bb4563517e 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DocumentPrivileges.java @@ -10,6 +10,7 @@ */ package org.opensearch.security.privileges.dlsfls; +import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -213,4 +214,17 @@ public String getRenderedSource() { } } + public static RenderedDlsQuery getRenderedDlsQuery(NamedXContentRegistry xContentRegistry, String query) throws IOException { + return new RenderedDlsQuery(parseQuery(xContentRegistry, query), query); + } + + static QueryBuilder parseQuery(NamedXContentRegistry xContentRegistry, String queryString) throws IOException { + XContentParser parser = JsonXContent.jsonXContent.createParser( + xContentRegistry, + DeprecationHandler.THROW_UNSUPPORTED_OPERATION, + queryString + ); + return AbstractQueryBuilder.parseInnerQueryBuilder(parser); + } + } diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index 2c359af032..4fa8600ba5 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -10,9 +10,18 @@ */ package org.opensearch.security.privileges.dlsfls; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.function.Predicate; +import java.util.stream.Collectors; import com.google.common.collect.ImmutableMap; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; + +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.security.user.User; /** * Maps index names to DLS/FLS/FM rules. @@ -24,6 +33,7 @@ * of the sub-classes of AbstractRuleBasedPrivileges. */ public class IndexToRuleMap { + private static final Logger LOGGER = LogManager.getLogger(IndexToRuleMap.class); private static final IndexToRuleMap UNRESTRICTED = new IndexToRuleMap(ImmutableMap.of()); private final ImmutableMap indexMap; @@ -58,4 +68,37 @@ public boolean containsAny(Predicate predicate) { public static IndexToRuleMap unrestricted() { return (IndexToRuleMap) UNRESTRICTED; } + + public static IndexToRuleMap resourceRestrictions(NamedXContentRegistry xContentRegistry, User user) { + + List principals = new ArrayList<>(); + principals.add("user:" + user.getName()); + + // Security roles (OpenSearch Security roles) + if (user.getRoles() != null) { + user.getRoles().forEach(r -> principals.add("role:" + r)); + } + + // Backend roles (LDAP/SAML/etc) – adjust getter name to your version! + if (user.getRoles() != null) { + user.getRoles().forEach(br -> principals.add("backend_role:" + br)); + } + + // Build a single `terms` query JSON + String dlsJson = "{\"terms\":{\"all_shared_principals.keyword\":[" + + principals.stream().map(p -> "\"" + p.replace("\"", "\\\"") + "\"").collect(Collectors.joining(",")) + + "]}}"; + + System.out.println("dlsJson: " + dlsJson); + + DlsRestriction restriction; + try { + restriction = new DlsRestriction(List.of(DocumentPrivileges.getRenderedDlsQuery(xContentRegistry, dlsJson))); + } catch (IOException e) { + LOGGER.warn("Received error while applying resource restrictions.", e); + restriction = DlsRestriction.FULL; + } + + return new IndexToRuleMap<>(ImmutableMap.of(".sample_resource", restriction)); + } } From 2cba16e7b5c5dd25a72ac190503934b63d336be3 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 21:23:15 -0400 Subject: [PATCH 10/27] Get all resource plugin tests working Signed-off-by: Craig Perkins --- .../opensearch/sample/resource/TestUtils.java | 1 + .../feature/enabled/ApiAccessTests.java | 1 + .../transport/GetResourceTransportAction.java | 40 +-- .../client/ResourceSharingClient.java | 9 - .../security/OpenSearchSecurityPlugin.java | 3 +- .../configuration/DlsFlsValveImpl.java | 32 +- .../privileges/dlsfls/IndexToRuleMap.java | 15 +- .../ResourceAccessControlClient.java | 13 - .../resources/ResourceAccessHandler.java | 68 ----- .../ResourceSharingIndexHandler.java | 289 ------------------ .../resources/ResourceAccessHandlerTest.java | 36 --- 11 files changed, 49 insertions(+), 458 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 3436c5f10f..503011ddc8 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 @@ -357,6 +357,7 @@ private void assertGet(String endpoint, TestSecurityConfig.User user, int status try (TestRestClient client = cluster.getRestClient(user)) { TestRestClient.HttpResponse response = client.get(endpoint); response.assertStatusCode(status); + System.out.println("get response json: " + response.getBody()); if (status == HttpStatus.SC_OK) assertThat(response.getBody(), containsString(expectedString)); } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java index 607d6bb4d6..6216a664b0 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java @@ -160,6 +160,7 @@ public void testApiAccess_limitedAccessUser() { api.assertApiShare(userResId, LIMITED_ACCESS_USER, USER_ADMIN, sampleReadOnlyAG.name(), HttpStatus.SC_OK); api.assertApiGet(userResId, USER_ADMIN, HttpStatus.SC_OK, "sampleUpdateUser"); api.assertApiRevoke(userResId, LIMITED_ACCESS_USER, USER_ADMIN, sampleReadOnlyAG.name(), HttpStatus.SC_OK); + System.out.println("Performing get request"); api.assertApiGet(userResId, USER_ADMIN, HttpStatus.SC_FORBIDDEN, ""); // should not be able to search for admin's resource, can only see self-resource 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 096e963fe9..d973cdd1c0 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 @@ -29,13 +29,11 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.query.QueryBuilders; import org.opensearch.sample.SampleResource; -import org.opensearch.sample.client.ResourceSharingClientAccessor; import org.opensearch.sample.resource.actions.rest.get.GetResourceAction; import org.opensearch.sample.resource.actions.rest.get.GetResourceRequest; import org.opensearch.sample.resource.actions.rest.get.GetResourceResponse; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; -import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; import org.opensearch.transport.client.node.NodeClient; @@ -59,36 +57,16 @@ public GetResourceTransportAction(TransportService transportService, ActionFilte @Override protected void doExecute(Task task, GetResourceRequest request, ActionListener listener) { - ResourceSharingClient client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); String resourceId = request.getResourceId(); if (Strings.isNullOrEmpty(resourceId)) { - fetchAllResources(listener, client); + fetchAllResources(listener); } else { fetchResourceById(resourceId, listener); } } - private void fetchAllResources(ActionListener listener, ResourceSharingClient client) { - System.out.println("fetchAllResources called"); - fetchResourcesByIds(listener); - } - - private void fetchResourceById(String resourceId, ActionListener listener) { - withThreadContext(stashed -> { - GetRequest req = new GetRequest(RESOURCE_INDEX_NAME, resourceId); - nodeClient.get(req, ActionListener.wrap(resp -> { - if (resp.isSourceEmpty()) { - listener.onFailure(new ResourceNotFoundException("Resource " + resourceId + " not found.")); - } else { - SampleResource resource = parseResource(resp.getSourceAsString()); - listener.onResponse(new GetResourceResponse(Set.of(resource))); - } - }, listener::onFailure)); - }); - } - - private void fetchResourcesByIds(ActionListener listener) { + private void fetchAllResources(ActionListener listener) { withThreadContext(stashed -> { SearchSourceBuilder ssb = new SearchSourceBuilder().size(1000).query(QueryBuilders.matchAllQuery()); @@ -111,6 +89,20 @@ private void fetchResourcesByIds(ActionListener listener) { }); } + private void fetchResourceById(String resourceId, ActionListener listener) { + withThreadContext(stashed -> { + GetRequest req = new GetRequest(RESOURCE_INDEX_NAME, resourceId); + nodeClient.get(req, ActionListener.wrap(resp -> { + if (resp.isSourceEmpty()) { + listener.onFailure(new ResourceNotFoundException("Resource " + resourceId + " not found.")); + } else { + SampleResource resource = parseResource(resp.getSourceAsString()); + listener.onResponse(new GetResourceResponse(Set.of(resource))); + } + }, listener::onFailure)); + }); + } + private SampleResource parseResource(String json) throws IOException { try ( XContentParser parser = XContentType.JSON.xContent() diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java index 9a92020254..c2811b6415 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java @@ -8,8 +8,6 @@ package org.opensearch.security.spi.resources.client; -import java.util.Set; - import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.spi.resources.sharing.ShareWith; @@ -47,11 +45,4 @@ public interface ResourceSharingClient { * @param listener The listener to be notified with the updated ResourceSharing document. */ void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener); - - /** - * Lists resourceIds of all shareable resources accessible by the current user. - * @param resourceIndex The index containing the resources. - * @param listener The listener to be notified with the set of accessible resources. - */ - void getAccessibleResourceIds(String resourceIndex, ActionListener> listener); } diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index cd58773fea..544e3fe7e2 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1195,7 +1195,8 @@ public Collection createComponents( resolver, xContentRegistry, threadPool, - dlsFlsBaseContext + dlsFlsBaseContext, + adminDns ); cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index f68fd440a9..882c117f0e 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -103,6 +103,8 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve { private final AtomicReference dlsFlsProcessedConfig = new AtomicReference<>(); private final FieldMasking.Config fieldMaskingConfig; private final Settings settings; + private final AdminDNs adminDNs; + private boolean isResourceSharingFeatureEnabled = false; public DlsFlsValveImpl( Settings settings, @@ -111,7 +113,8 @@ public DlsFlsValveImpl( IndexNameExpressionResolver resolver, NamedXContentRegistry namedXContentRegistry, ThreadPool threadPool, - DlsFlsBaseContext dlsFlsBaseContext + DlsFlsBaseContext dlsFlsBaseContext, + AdminDNs adminDNs ) { super(); this.nodeClient = nodeClient; @@ -123,6 +126,7 @@ public DlsFlsValveImpl( this.fieldMaskingConfig = FieldMasking.Config.fromSettings(settings); this.dlsFlsBaseContext = dlsFlsBaseContext; this.settings = settings; + this.adminDNs = adminDNs; clusterService.addListener(event -> { DlsFlsProcessedConfig config = dlsFlsProcessedConfig.get(); @@ -131,6 +135,11 @@ public DlsFlsValveImpl( config.updateClusterStateMetadataAsync(clusterService, threadPool); } }); + boolean isResourceSharingFeatureEnabled = settings.getAsBoolean( + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED, + ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT + ); + this.isResourceSharingFeatureEnabled = isResourceSharingFeatureEnabled; } /** @@ -140,25 +149,22 @@ public DlsFlsValveImpl( */ @Override public boolean invoke(PrivilegesEvaluationContext context, final ActionListener listener) { - System.out.println("context.action: " + context.getAction()); - System.out.println("context.request: " + context.getRequest()); UserSubjectImpl userSubject = (UserSubjectImpl) threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER); - System.out.println("userSubject: " + userSubject.getUser()); - System.out.println( - "HeaderHelper.isInternalOrPluginRequest(threadContext): " + HeaderHelper.isInternalOrPluginRequest(threadContext) - ); - if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { - System.out.println("set unrestricted"); - DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); + if (isClusterPerm(context.getAction()) && !MultiGetAction.NAME.equals(context.getAction())) { + return true; + } + if (userSubject != null && adminDNs.isAdmin(userSubject.getUser())) { + return true; + } + if (isResourceSharingFeatureEnabled && HeaderHelper.isInternalOrPluginRequest(threadContext)) { IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( namedXContentRegistry, + resolved, userSubject.getUser() ); - System.out.println("sharedResourceMap: " + sharedResourceMap); - return DlsFilterLevelActionHandler.handle( context, sharedResourceMap, @@ -169,8 +175,6 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< resolver, threadContext ); - } else if (isClusterPerm(context.getAction()) && !MultiGetAction.NAME.equals(context.getAction())) { - return true; } DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); ActionRequest request = context.getRequest(); diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index 4fa8600ba5..e4a2927706 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -21,6 +21,7 @@ import org.apache.logging.log4j.Logger; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.user.User; /** @@ -69,7 +70,11 @@ public static IndexToRuleMap) UNRESTRICTED; } - public static IndexToRuleMap resourceRestrictions(NamedXContentRegistry xContentRegistry, User user) { + public static IndexToRuleMap resourceRestrictions( + NamedXContentRegistry xContentRegistry, + IndexResolverReplacer.Resolved resolved, + User user + ) { List principals = new ArrayList<>(); principals.add("user:" + user.getName()); @@ -89,8 +94,6 @@ public static IndexToRuleMap resourceRestrictions(NamedXContentR + principals.stream().map(p -> "\"" + p.replace("\"", "\\\"") + "\"").collect(Collectors.joining(",")) + "]}}"; - System.out.println("dlsJson: " + dlsJson); - DlsRestriction restriction; try { restriction = new DlsRestriction(List.of(DocumentPrivileges.getRenderedDlsQuery(xContentRegistry, dlsJson))); @@ -99,6 +102,10 @@ public static IndexToRuleMap resourceRestrictions(NamedXContentR restriction = DlsRestriction.FULL; } - return new IndexToRuleMap<>(ImmutableMap.of(".sample_resource", restriction)); + ImmutableMap.Builder mapBuilder = ImmutableMap.builder(); + for (String index : resolved.getAllIndices()) { + mapBuilder.put(index, restriction); + } + return new IndexToRuleMap<>(mapBuilder.build()); } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java index 66afbe4e56..482abd3790 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java @@ -8,8 +8,6 @@ package org.opensearch.security.resources; -import java.util.Set; - import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.security.spi.resources.sharing.ResourceSharing; @@ -70,15 +68,4 @@ public void share(String resourceId, String resourceIndex, ShareWith target, Act public void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener) { resourceAccessHandler.revoke(resourceId, resourceIndex, target, listener); } - - /** - * Lists all resources the current user has access to within the given index. - * - * @param resourceIndex The index to search for accessible resources. - * @param listener Callback receiving a set of resource ids. - */ - @Override - public void getAccessibleResourceIds(String resourceIndex, ActionListener> listener) { - resourceAccessHandler.getOwnAndSharedResourceIdsForCurrentUser(resourceIndex, listener); - } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index 9cf2c870a1..d8f57cfc13 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -11,11 +11,8 @@ package org.opensearch.security.resources; -import java.util.Collections; import java.util.HashSet; import java.util.Set; -import java.util.stream.Collectors; -import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -26,8 +23,6 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; -import org.opensearch.index.query.BoolQueryBuilder; -import org.opensearch.index.query.QueryBuilders; import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.privileges.PrivilegesEvaluationContext; @@ -71,59 +66,6 @@ public ResourceAccessHandler( this.privilegesEvaluator = evaluator; } - /** - * Returns a set of accessible resource IDs for the current user within the specified resource index. - * - * @param resourceIndex The resource index to check for accessible resources. - * @param listener The listener to be notified with the set of accessible resource IDs. - */ - public void getOwnAndSharedResourceIdsForCurrentUser(@NonNull String resourceIndex, ActionListener> listener) { - UserSubjectImpl userSub = (UserSubjectImpl) threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER); - User user = userSub == null ? null : userSub.getUser(); - - if (user == null) { - LOGGER.warn("No authenticated user; returning empty set"); - listener.onResponse(Collections.emptySet()); - return; - } - - if (adminDNs.isAdmin(user)) { - loadAllResources(resourceIndex, ActionListener.wrap(listener::onResponse, listener::onFailure)); - return; - } - - // 1) collect all entities we’ll match against share_with arrays - // for users: - Set users = new HashSet<>(); - users.add(user.getName()); - users.add("*"); // for matching against publicly shared resource - - // for roles: - Set roles = new HashSet<>(user.getSecurityRoles()); - roles.add("*"); // for matching against publicly shared resource - - // for backend_roles: - Set backendRoles = new HashSet<>(user.getRoles()); - backendRoles.add("*"); // for matching against publicly shared resource - - // 2) build a flattened query (allows us to compute large number of entries in less than a second compared to multi-match query with - // BEST_FIELDS) - Set flatPrincipals = Stream.concat( - // users - users.stream().map(u -> "user:" + u), - // then roles and backend_roles - Stream.concat(roles.stream().map(r -> "role:" + r), backendRoles.stream().map(b -> "backend:" + b)) - ).collect(Collectors.toSet()); - - BoolQueryBuilder query = QueryBuilders.boolQuery() - .should(QueryBuilders.termQuery("created_by.user.keyword", user.getName())) - .should(QueryBuilders.termsQuery("all_shared_principals", flatPrincipals)) - .minimumShouldMatch(1); - - // 3) Fetch all accessible resource IDs - resourceSharingIndexHandler.fetchAccessibleResourceIds(resourceIndex, flatPrincipals, query, listener); - } - /** * Checks whether current user has permission to access given resource. * @@ -386,14 +328,4 @@ public void revoke( listener.onFailure(exception); })); } - - /** - * Loads all resources within the specified resource index. - * - * @param resourceIndex The resource index to load resources from. - * @param listener The listener to be notified with the set of resource IDs. - */ - private void loadAllResources(String resourceIndex, ActionListener> listener) { - this.resourceSharingIndexHandler.fetchAllResourceIds(resourceIndex, listener); - } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index a5ffbfa3e2..b09dfcf6fb 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -11,7 +11,6 @@ import java.io.IOException; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -31,15 +30,10 @@ import org.opensearch.action.get.GetRequest; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; -import org.opensearch.action.search.ClearScrollRequest; -import org.opensearch.action.search.SearchRequest; -import org.opensearch.action.search.SearchResponse; -import org.opensearch.action.search.SearchScrollRequest; import org.opensearch.action.support.WriteRequest; import org.opensearch.action.update.UpdateRequest; import org.opensearch.action.update.UpdateResponse; import org.opensearch.common.inject.Inject; -import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentType; @@ -49,13 +43,6 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.engine.VersionConflictEngineException; -import org.opensearch.index.query.AbstractQueryBuilder; -import org.opensearch.index.query.BoolQueryBuilder; -import org.opensearch.index.query.MatchAllQueryBuilder; -import org.opensearch.index.query.QueryBuilders; -import org.opensearch.search.Scroll; -import org.opensearch.search.SearchHit; -import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.security.spi.resources.sharing.CreatedBy; import org.opensearch.security.spi.resources.sharing.Recipient; import org.opensearch.security.spi.resources.sharing.Recipients; @@ -253,185 +240,6 @@ public void indexResourceSharing( } } - /** - * Fetches all resource sharing records that match the specified system index. This method retrieves - * a get of resource IDs associated with the given system index from the resource sharing index. - * - *

The method executes the following steps: - *

    - *
  1. Creates a search request with term query matching the system index
    2. - *
  2. Applies source filtering to only fetch resource_id field
    3. - *
  3. Executes the search with a limit of 10000 documents
    4. - *
  4. Processes the results to extract resource IDs
    5. - *
- * - *

Example query structure: - * 

-     * {
-     *   "query": {
-     *     "term": {
-     *       "source_idx": "resource_index_name"
-     *     }
-     *   },
-     *   "_source": ["resource_id"],
-     *   "size": 10000
-     * }
-     *
- * - * @param resourceIndex The source index to match against the source_idx field - * @param listener The listener to be notified when the operation completes. - * The listener receives a set of resource IDs as a result. - * @apiNote This method: - *
    - *
  • Uses source filtering for optimal performance
    • - *
  • Performs exact matching on the source_idx field
    • - *
  • Returns an empty get instead of throwing exceptions
    • - *
- */ - public void fetchAllResourceIds(String resourceIndex, ActionListener> listener) { - String resourceSharingIndex = getSharingIndex(resourceIndex); - LOGGER.debug("Fetching all documents asynchronously from {}", resourceSharingIndex); - Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); - - try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { - final SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); - searchRequest.scroll(scroll); - - MatchAllQueryBuilder query = QueryBuilders.matchAllQuery(); - - executeSearchRequest(scroll, searchRequest, query, ActionListener.wrap(resourceIds -> { - ctx.restore(); - LOGGER.debug("Found {} documents in {}", resourceIds.size(), resourceSharingIndex); - listener.onResponse(resourceIds); - }, exception -> { - LOGGER.error("Search failed while locating all records inside resourceIndex={} ", resourceIndex, exception); - listener.onFailure(exception); - })); - } - } - - /** - * Helper method to fetch own and shared documents based on action-group match. - * This method uses scroll API to handle large result sets efficiently. - * - * - * @param resourceIndex The source index to match against the source_idx field - * @param entities Set of values to match in the specified Recipient field. Used for logging. ActionGroupQuery is already updated with these values. - * @param actionGroupQuery The query to match against the action-group field - * @param listener The listener to be notified when the operation completes. - * The listener receives a set of resource IDs as a result. - * @throws RuntimeException if the search operation fails - * @apiNote This method: - *
    - *
  • Uses scroll API with 1-minute timeout
    • - *
  • Processes results in batches of 1000 documents
    • - *
  • Performs source filtering for optimization
    • - *
  • Uses nested queries for accessing array elements
    • - *
  • Properly cleans up scroll context after use
    • - *
- */ - public void fetchAccessibleResourceIds( - String resourceIndex, - Set entities, - BoolQueryBuilder actionGroupQuery, - ActionListener> listener - ) { - final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); - - try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { - // Search the RESOURCE INDEX directly (not the *-sharing index) - SearchRequest searchRequest = new SearchRequest(resourceIndex); - searchRequest.scroll(scroll); - - // 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.keyword", entities)); - - executeIdCollectingSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { - ctx.restore(); - LOGGER.debug("Found {} accessible resources in {}", resourceIds.size(), resourceIndex); - listener.onResponse(resourceIds); - }, exception -> { - LOGGER.error("Search failed for resourceIndex={}, entities={}", resourceIndex, entities, exception); - listener.onFailure(exception); - })); - } - } - - /** - * Executes a search request against the resource index and collects _id values (resource IDs) using scroll. - * - * @param scroll Search scroll context - * @param searchRequest Initial search request - * @param query Query builder for the request - * @param listener Listener to receive the collected resource IDs - */ - private void executeIdCollectingSearchRequest( - Scroll scroll, - SearchRequest searchRequest, - AbstractQueryBuilder> query, - ActionListener> listener - ) { - SearchSourceBuilder ssb = new SearchSourceBuilder().query(query).size(1000).fetchSource(false); // we only need _id - - searchRequest.source(ssb); - - StepListener searchStep = new StepListener<>(); - client.search(searchRequest, searchStep); - - searchStep.whenComplete(initialResponse -> { - Set collectedResourceIds = new HashSet<>(); - String scrollId = initialResponse.getScrollId(); - processScrollIds(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); - }, listener::onFailure); - } - - /** - * Recursively processes scroll results and collects hit IDs. - * - * @param collectedResourceIds Internal accumulator for resource IDs - * @param scroll Scroll context - * @param scrollId Scroll ID - * @param hits Search hits - * @param listener Listener to receive final set of resource IDs - */ - private void processScrollIds( - Set collectedResourceIds, - Scroll scroll, - String scrollId, - SearchHit[] hits, - ActionListener> listener - ) { - if (hits == null || hits.length == 0) { - clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onResponse(collectedResourceIds), listener::onFailure)); - return; - } - - for (SearchHit hit : hits) { - // Resource ID is the document _id in the resource index - collectedResourceIds.add(hit.getId()); - } - - SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); - client.searchScroll( - scrollRequest, - ActionListener.wrap( - scrollResponse -> processScrollIds( - collectedResourceIds, - scroll, - scrollResponse.getScrollId(), - scrollResponse.getHits().getHits(), - listener - ), - e -> clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onFailure(e), ex -> { - e.addSuppressed(ex); - listener.onFailure(e); - })) - ) - ); - } - /** * Fetches a specific resource sharing document by its resource ID and system resourceIndex. * This method performs an exact match search and parses the result into a ResourceSharing object. @@ -872,101 +680,4 @@ public void deleteResourceSharingRecord(String resourceId, String resourceIndex, } } - /** - * Executes a search request and returns a set of collected resource IDs using scroll. - * - * @param scroll Search scroll context - * @param searchRequest Initial search request - * @param query Query builder for the request - * @param listener Listener to receive the collected resource IDs - */ - private void executeSearchRequest( - Scroll scroll, - SearchRequest searchRequest, - AbstractQueryBuilder> query, - ActionListener> listener - ) { - SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(query) - .size(1000) - .fetchSource(new String[] { "resource_id" }, null); - - searchRequest.source(searchSourceBuilder); - - StepListener searchStep = new StepListener<>(); - client.search(searchRequest, searchStep); - - searchStep.whenComplete(initialResponse -> { - Set collectedResourceIds = new HashSet<>(); - String scrollId = initialResponse.getScrollId(); - processScrollResults(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); - }, listener::onFailure); - } - - /** - * Recursively processes scroll results and collects resource IDs. - * - * @param collectedResourceIds Internal accumulator for resource IDs - * @param scroll Scroll context - * @param scrollId Scroll ID - * @param hits Search hits - * @param listener Listener to receive final set of resource IDs - */ - private void processScrollResults( - Set collectedResourceIds, - Scroll scroll, - String scrollId, - SearchHit[] hits, - ActionListener> listener - ) { - if (hits == null || hits.length == 0) { - clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onResponse(collectedResourceIds), listener::onFailure)); - return; - } - - for (SearchHit hit : hits) { - Map source = hit.getSourceAsMap(); - if (source != null && source.containsKey("resource_id")) { - collectedResourceIds.add(source.get("resource_id").toString()); - } - } - - SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); - client.searchScroll( - scrollRequest, - ActionListener.wrap( - scrollResponse -> processScrollResults( - collectedResourceIds, - scroll, - scrollResponse.getScrollId(), - scrollResponse.getHits().getHits(), - listener - ), - e -> clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onFailure(e), ex -> { - e.addSuppressed(ex); - listener.onFailure(e); - })) - ) - ); - } - - /** - * Clears scroll context after scrolling is complete or on error. - * - * @param scrollId Scroll ID to clear - * @param listener Listener to notify when clearing is done - */ - private void clearScroll(String scrollId, ActionListener listener) { - if (scrollId == null) { - listener.onResponse(null); - return; - } - - ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); - clearScrollRequest.addScrollId(scrollId); - client.clearScroll(clearScrollRequest, ActionListener.wrap(r -> listener.onResponse(null), e -> { - LOGGER.warn("Failed to clear scroll context", e); - listener.onResponse(null); - })); - } - } diff --git a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java index 5803c5e6f0..1185822cdf 100644 --- a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java +++ b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java @@ -205,42 +205,6 @@ public void testHasPermission_pluginUserDenied() { verify(listener).onResponse(false); } - @Test - public void testGetOwnAndSharedResources_asAdmin() { - User admin = new User("admin", ImmutableSet.of(), ImmutableSet.of(), null, ImmutableMap.of(), false); - injectUser(admin); - when(adminDNs.isAdmin(admin)).thenReturn(true); - - ActionListener> listener = mock(ActionListener.class); - - doAnswer(inv -> { - ActionListener> l = inv.getArgument(1); - l.onResponse(Set.of("res1", "res2")); - return null; - }).when(sharingIndexHandler).fetchAllResourceIds(eq(INDEX), any()); - - handler.getOwnAndSharedResourceIdsForCurrentUser(INDEX, listener); - verify(listener).onResponse(Set.of("res1", "res2")); - } - - @Test - public void testGetOwnAndSharedResources_asNormalUser() { - User user = new User("alice", ImmutableSet.of("r1"), ImmutableSet.of("b1"), null, ImmutableMap.of(), false); - injectUser(user); - when(adminDNs.isAdmin(user)).thenReturn(false); - - ActionListener> listener = mock(ActionListener.class); - - doAnswer(inv -> { - ActionListener> l = inv.getArgument(3); - l.onResponse(Set.of("res1")); - return null; - }).when(sharingIndexHandler).fetchAccessibleResourceIds(any(), any(), any(), any()); - - handler.getOwnAndSharedResourceIdsForCurrentUser(INDEX, listener); - verify(listener).onResponse(Set.of("res1")); - } - @Test public void testShareSuccess() { User user = new User("user2", ImmutableSet.of(), ImmutableSet.of(), null, ImmutableMap.of(), false); From 21ae88827182edb126b5a9ef64fb8cc42030c132 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 22:10:18 -0400 Subject: [PATCH 11/27] Fix tests Signed-off-by: Craig Perkins --- .../configuration/DlsFlsValveImpl.java | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 882c117f0e..24839ad6bf 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -156,25 +156,29 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< if (userSubject != null && adminDNs.isAdmin(userSubject.getUser())) { return true; } - if (isResourceSharingFeatureEnabled && HeaderHelper.isInternalOrPluginRequest(threadContext)) { - IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); - - IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( - namedXContentRegistry, - resolved, - userSubject.getUser() - ); - - return DlsFilterLevelActionHandler.handle( - context, - sharedResourceMap, - listener, - nodeClient, - clusterService, - OpenSearchSecurityPlugin.GuiceHolder.getIndicesService(), - resolver, - threadContext - ); + if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { + if (isResourceSharingFeatureEnabled) { + IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); + + IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( + namedXContentRegistry, + resolved, + userSubject.getUser() + ); + + return DlsFilterLevelActionHandler.handle( + context, + sharedResourceMap, + listener, + nodeClient, + clusterService, + OpenSearchSecurityPlugin.GuiceHolder.getIndicesService(), + resolver, + threadContext + ); + } else { + return true; + } } DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); ActionRequest request = context.getRequest(); @@ -185,8 +189,6 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< boolean hasFlsRestrictions = !config.getFieldPrivileges().isUnrestricted(context, resolved); boolean hasFieldMasking = !config.getFieldMasking().isUnrestricted(context, resolved); - System.out.println("hasDlsRestrictions: " + hasDlsRestrictions); - if (!hasDlsRestrictions && !hasFlsRestrictions && !hasFieldMasking) { return true; } From 2613852d9575ed6c8050d8ed0d758fcb924bbc02 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 29 Aug 2025 22:55:38 -0400 Subject: [PATCH 12/27] Only on SearchRequest Signed-off-by: Craig Perkins --- .../java/org/opensearch/sample/resource/TestUtils.java | 1 - .../sample/resource/feature/enabled/ApiAccessTests.java | 1 - .../opensearch/security/configuration/DlsFlsValveImpl.java | 4 ++-- 3 files changed, 2 insertions(+), 4 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 503011ddc8..3436c5f10f 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 @@ -357,7 +357,6 @@ private void assertGet(String endpoint, TestSecurityConfig.User user, int status try (TestRestClient client = cluster.getRestClient(user)) { TestRestClient.HttpResponse response = client.get(endpoint); response.assertStatusCode(status); - System.out.println("get response json: " + response.getBody()); if (status == HttpStatus.SC_OK) assertThat(response.getBody(), containsString(expectedString)); } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java index 6216a664b0..607d6bb4d6 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java @@ -160,7 +160,6 @@ public void testApiAccess_limitedAccessUser() { api.assertApiShare(userResId, LIMITED_ACCESS_USER, USER_ADMIN, sampleReadOnlyAG.name(), HttpStatus.SC_OK); api.assertApiGet(userResId, USER_ADMIN, HttpStatus.SC_OK, "sampleUpdateUser"); api.assertApiRevoke(userResId, LIMITED_ACCESS_USER, USER_ADMIN, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - System.out.println("Performing get request"); api.assertApiGet(userResId, USER_ADMIN, HttpStatus.SC_FORBIDDEN, ""); // should not be able to search for admin's resource, can only see self-resource diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 24839ad6bf..cb33bed85b 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -156,8 +156,9 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< if (userSubject != null && adminDNs.isAdmin(userSubject.getUser())) { return true; } + ActionRequest request = context.getRequest(); if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { - if (isResourceSharingFeatureEnabled) { + if (isResourceSharingFeatureEnabled && request instanceof SearchRequest) { IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( @@ -181,7 +182,6 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< } } DlsFlsProcessedConfig config = this.dlsFlsProcessedConfig.get(); - ActionRequest request = context.getRequest(); IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); try { From 3c9ceeca0871d00c74e30947ee725163f901dc49 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 2 Sep 2025 10:41:50 -0400 Subject: [PATCH 13/27] Only apply DLS if all indices in the request are resource indices Signed-off-by: Craig Perkins --- .../security/support/WildcardMatcherTest.java | 22 ++++++++++ .../security/OpenSearchSecurityPlugin.java | 3 +- .../configuration/DlsFlsValveImpl.java | 13 ++++-- .../security/support/WildcardMatcher.java | 42 +++++++++++++++++++ 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/support/WildcardMatcherTest.java b/src/integrationTest/java/org/opensearch/security/support/WildcardMatcherTest.java index 7b524cb5f2..e9227ea1f0 100644 --- a/src/integrationTest/java/org/opensearch/security/support/WildcardMatcherTest.java +++ b/src/integrationTest/java/org/opensearch/security/support/WildcardMatcherTest.java @@ -54,6 +54,28 @@ public void none() { assertEquals("", subject.toString()); } + @Test + public void all() { + WildcardMatcher any = applyCase(WildcardMatcher.ANY); + assertTrue(any.matchAll("any_string", "other_string")); + assertTrue(any.matchAll(Arrays.asList("any_string", "other_string"))); + assertTrue(any.matchAll(Stream.of("any_string", "other_string"))); + + WildcardMatcher none = applyCase(WildcardMatcher.NONE); + assertFalse(none.matchAll("any_string")); + assertFalse(none.matchAll(Arrays.asList("any_string"))); + assertFalse(none.matchAll(Stream.of("any_string"))); + assertTrue(none.matchAll()); + assertTrue(none.matchAll(Collections.emptyList())); + assertTrue(none.matchAll(Stream.empty())); + + WildcardMatcher prefix = applyCase(WildcardMatcher.from("test*")); + assertTrue(prefix.matchAll("test1", "test2", "testing")); + assertFalse(prefix.matchAll("test1", "other", "testing")); + assertTrue(prefix.matchAll(Arrays.asList("test1", "test2"))); + assertFalse(prefix.matchAll(Arrays.asList("test1", "other"))); + } + @Test public void anyFromStar() { assertSame(WildcardMatcher.ANY, applyCase(WildcardMatcher.from("*"))); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 544e3fe7e2..3d36549854 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1196,7 +1196,8 @@ public Collection createComponents( xContentRegistry, threadPool, dlsFlsBaseContext, - adminDns + adminDns, + resourcePluginInfo != null ? resourcePluginInfo.getResourceIndices() : null ); cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); } diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index cb33bed85b..48267c4baf 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -17,6 +17,7 @@ import java.util.Comparator; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.stream.StreamSupport; @@ -83,6 +84,7 @@ import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.HeaderHelper; +import org.opensearch.security.support.WildcardMatcher; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.client.Client; @@ -105,6 +107,7 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve { private final Settings settings; private final AdminDNs adminDNs; private boolean isResourceSharingFeatureEnabled = false; + private final WildcardMatcher resourceIndicesMatcher; public DlsFlsValveImpl( Settings settings, @@ -114,7 +117,8 @@ public DlsFlsValveImpl( NamedXContentRegistry namedXContentRegistry, ThreadPool threadPool, DlsFlsBaseContext dlsFlsBaseContext, - AdminDNs adminDNs + AdminDNs adminDNs, + Set resourceIndices ) { super(); this.nodeClient = nodeClient; @@ -127,6 +131,7 @@ public DlsFlsValveImpl( this.dlsFlsBaseContext = dlsFlsBaseContext; this.settings = settings; this.adminDNs = adminDNs; + this.resourceIndicesMatcher = WildcardMatcher.from(resourceIndices); clusterService.addListener(event -> { DlsFlsProcessedConfig config = dlsFlsProcessedConfig.get(); @@ -158,8 +163,10 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< } ActionRequest request = context.getRequest(); if (HeaderHelper.isInternalOrPluginRequest(threadContext)) { - if (isResourceSharingFeatureEnabled && request instanceof SearchRequest) { - IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); + IndexResolverReplacer.Resolved resolved = context.getResolvedRequest(); + if (isResourceSharingFeatureEnabled + && request instanceof SearchRequest + && resourceIndicesMatcher.matchAll(resolved.getAllIndices())) { IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( namedXContentRegistry, diff --git a/src/main/java/org/opensearch/security/support/WildcardMatcher.java b/src/main/java/org/opensearch/security/support/WildcardMatcher.java index dcf9917ebe..6f73f5dc49 100644 --- a/src/main/java/org/opensearch/security/support/WildcardMatcher.java +++ b/src/main/java/org/opensearch/security/support/WildcardMatcher.java @@ -62,6 +62,21 @@ public boolean matchAny(String... candidates) { return true; } + @Override + public boolean matchAll(Stream candidates) { + return true; + } + + @Override + public boolean matchAll(Collection candidates) { + return true; + } + + @Override + public boolean matchAll(String... candidates) { + return true; + } + @Override public > T getMatchAny(Stream candidates, Collector collector) { return candidates.collect(collector); @@ -100,6 +115,21 @@ public boolean matchAny(String... candidates) { return false; } + @Override + public boolean matchAll(Stream candidates) { + return candidates.findAny().isEmpty(); + } + + @Override + public boolean matchAll(Collection candidates) { + return candidates.isEmpty(); + } + + @Override + public boolean matchAll(String... candidates) { + return candidates.length == 0; + } + @Override public > T getMatchAny(Stream candidates, Collector collector) { return Stream.empty().collect(collector); @@ -233,6 +263,18 @@ public boolean matchAny(String... candidates) { return matchAny(Arrays.stream(candidates)); } + public boolean matchAll(Stream candidates) { + return candidates.allMatch(this); + } + + public boolean matchAll(Collection candidates) { + return matchAll(candidates.stream()); + } + + public boolean matchAll(String... candidates) { + return matchAll(Arrays.stream(candidates)); + } + public > T getMatchAny(Stream candidates, Collector collector) { return candidates.filter(this).collect(collector); } From f532d8e88a38ae2d341859c1224b26e6572f58d1 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 2 Sep 2025 10:52:11 -0400 Subject: [PATCH 14/27] Handle publicly visible Signed-off-by: Craig Perkins --- .../opensearch/security/privileges/dlsfls/IndexToRuleMap.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index e4a2927706..df52cc8054 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -77,6 +77,7 @@ public static IndexToRuleMap resourceRestrictions( ) { List principals = new ArrayList<>(); + principals.add("user:*"); // Convention for publicly visible principals.add("user:" + user.getName()); // Security roles (OpenSearch Security roles) From f5392955360bcdbfb70968ccc4fb9ead67735d5a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Tue, 2 Sep 2025 11:41:16 -0400 Subject: [PATCH 15/27] Fix CHANGELOG Signed-off-by: Craig Perkins --- CHANGELOG.md | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 64eae83c67..227bf7e441 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,13 +9,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Enhancements -- [Resource Sharing] Keep track of list of principals for which sharable resource is visible for searching ([#5596](https://github.com/opensearch-project/security/pull/5596)) +- [Resource Sharing] Use DLS to automatically filter sharable resources for authenticated user based on `all_shared_principals` ([#5600](https://github.com/opensearch-project/security/pull/5600)) ### Bug Fixes -- Added new option skip_users to client cert authenticator (clientcert_auth_domain.http_authenticator.config.skip_users in config.yml)([#4378](https://github.com/opensearch-project/security/pull/5525)) -- [Resource Sharing] Fixes accessible resource ids search by marking created_by.user field as keyword search instead of text ([#5574](https://github.com/opensearch-project/security/pull/5574)) -- [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576)) * Added new option skip_users to client cert authenticator (clientcert_auth_domain.http_authenticator.config.skip_users in config.yml)([#4378](https://github.com/opensearch-project/security/pull/5525)) * [Resource Sharing] Fixes accessible resource ids search by marking created_by.user field as keyword search instead of text ([#5574](https://github.com/opensearch-project/security/pull/5574)) * [Resource Sharing] Reverts @Inject pattern usage for ResourceSharingExtension to client accessor pattern. ([#5576](https://github.com/opensearch-project/security/pull/5576)) From 8c58c37dea40fbdac804d67eed10da634ae3ae5a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 13:43:41 -0400 Subject: [PATCH 16/27] Remove sysouts Signed-off-by: Craig Perkins --- .../java/org/opensearch/sample/resource/TestUtils.java | 4 ---- 1 file changed, 4 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 037de5f0df..fbd58b8510 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 @@ -460,8 +460,6 @@ private void assertPostSearch( ) { try (TestRestClient client = cluster.getRestClient(user)) { TestRestClient.HttpResponse response = client.postJson(endpoint, searchPayload); - System.out.println("User: " + user); - System.out.println("Search response: " + response.getBody()); response.assertStatusCode(status); if (status == HttpStatus.SC_OK) { Map hits = (Map) response.bodyAsMap().get("hits"); @@ -472,8 +470,6 @@ private void assertPostSearch( try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { TestRestClient.HttpResponse response = client.postJson(endpoint, searchPayload); - System.out.println("SuperAdmin: " + user); - System.out.println("Search response: " + response.getBody()); } } From 7ceafa818b3c8f20c1d996faf12ff9bad65be0e9 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 14:12:20 -0400 Subject: [PATCH 17/27] Address review feedback Signed-off-by: Craig Perkins --- .../privileges/dlsfls/IndexToRuleMap.java | 20 +- .../ResourceAccessControlClient.java | 13 + .../resources/ResourceAccessHandler.java | 58 ++++ .../ResourceSharingIndexHandler.java | 289 ++++++++++++++++++ .../resources/ResourceAccessHandlerTest.java | 36 +++ 5 files changed, 407 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index df52cc8054..a024b64e59 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -14,13 +14,14 @@ import java.util.ArrayList; import java.util.List; import java.util.function.Predicate; -import java.util.stream.Collectors; import com.google.common.collect.ImmutableMap; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.security.resolver.IndexResolverReplacer; import org.opensearch.security.user.User; @@ -78,25 +79,26 @@ public static IndexToRuleMap resourceRestrictions( List principals = new ArrayList<>(); principals.add("user:*"); // Convention for publicly visible - principals.add("user:" + user.getName()); + principals.add("user:" + user.getName()); // owner // Security roles (OpenSearch Security roles) if (user.getRoles() != null) { user.getRoles().forEach(r -> principals.add("role:" + r)); } - // Backend roles (LDAP/SAML/etc) – adjust getter name to your version! + // Backend roles (LDAP/SAML/etc) if (user.getRoles() != null) { - user.getRoles().forEach(br -> principals.add("backend_role:" + br)); + user.getRoles().forEach(br -> principals.add("backend:" + br)); } - // Build a single `terms` query JSON - String dlsJson = "{\"terms\":{\"all_shared_principals.keyword\":[" - + principals.stream().map(p -> "\"" + p.replace("\"", "\\\"") + "\"").collect(Collectors.joining(",")) - + "]}}"; - + XContentBuilder builder = null; DlsRestriction restriction; try { + // Build a single `terms` query JSON + builder = XContentFactory.jsonBuilder(); + builder.startObject().startObject("terms").array("all_shared_principals.keyword", principals.toArray()).endObject().endObject(); + + String dlsJson = builder.toString(); restriction = new DlsRestriction(List.of(DocumentPrivileges.getRenderedDlsQuery(xContentRegistry, dlsJson))); } catch (IOException e) { LOGGER.warn("Received error while applying resource restrictions.", e); diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java index 482abd3790..66afbe4e56 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessControlClient.java @@ -8,6 +8,8 @@ package org.opensearch.security.resources; +import java.util.Set; + import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.client.ResourceSharingClient; import org.opensearch.security.spi.resources.sharing.ResourceSharing; @@ -68,4 +70,15 @@ public void share(String resourceId, String resourceIndex, ShareWith target, Act public void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener) { resourceAccessHandler.revoke(resourceId, resourceIndex, target, listener); } + + /** + * Lists all resources the current user has access to within the given index. + * + * @param resourceIndex The index to search for accessible resources. + * @param listener Callback receiving a set of resource ids. + */ + @Override + public void getAccessibleResourceIds(String resourceIndex, ActionListener> listener) { + resourceAccessHandler.getOwnAndSharedResourceIdsForCurrentUser(resourceIndex, listener); + } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index d8f57cfc13..4c0bc95fd2 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -11,8 +11,11 @@ package org.opensearch.security.resources; +import java.util.Collections; import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -23,6 +26,8 @@ import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.QueryBuilders; import org.opensearch.security.auth.UserSubjectImpl; import org.opensearch.security.configuration.AdminDNs; import org.opensearch.security.privileges.PrivilegesEvaluationContext; @@ -66,6 +71,59 @@ public ResourceAccessHandler( this.privilegesEvaluator = evaluator; } + /** + * Returns a set of accessible resource IDs for the current user within the specified resource index. + * + * @param resourceIndex The resource index to check for accessible resources. + * @param listener The listener to be notified with the set of accessible resource IDs. + */ + public void getOwnAndSharedResourceIdsForCurrentUser(@NonNull String resourceIndex, ActionListener> listener) { + UserSubjectImpl userSub = (UserSubjectImpl) threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER); + User user = userSub == null ? null : userSub.getUser(); + + if (user == null) { + LOGGER.warn("No authenticated user; returning empty set"); + listener.onResponse(Collections.emptySet()); + return; + } + + if (adminDNs.isAdmin(user)) { + loadAllResources(resourceIndex, ActionListener.wrap(listener::onResponse, listener::onFailure)); + return; + } + + // 1) collect all entities we’ll match against share_with arrays + // for users: + Set users = new HashSet<>(); + users.add(user.getName()); + users.add("*"); // for matching against publicly shared resource + + // for roles: + Set roles = new HashSet<>(user.getSecurityRoles()); + roles.add("*"); // for matching against publicly shared resource + + // for backend_roles: + Set backendRoles = new HashSet<>(user.getRoles()); + backendRoles.add("*"); // for matching against publicly shared resource + + // 2) build a flattened query (allows us to compute large number of entries in less than a second compared to multi-match query with + // BEST_FIELDS) + Set flatPrincipals = Stream.concat( + // users + users.stream().map(u -> "user:" + u), + // then roles and backend_roles + Stream.concat(roles.stream().map(r -> "role:" + r), backendRoles.stream().map(b -> "backend:" + b)) + ).collect(Collectors.toSet()); + + BoolQueryBuilder query = QueryBuilders.boolQuery() + .should(QueryBuilders.termQuery("created_by.user.keyword", user.getName())) + .should(QueryBuilders.termsQuery("all_shared_principals.keyword", flatPrincipals)) + .minimumShouldMatch(1); + + // 3) Fetch all accessible resource IDs + resourceSharingIndexHandler.fetchAccessibleResourceIds(resourceIndex, flatPrincipals, query, listener); + } + /** * Checks whether current user has permission to access given resource. * diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 87784cd6ef..3542ca6fef 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -30,10 +31,15 @@ import org.opensearch.action.get.GetRequest; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; +import org.opensearch.action.search.ClearScrollRequest; +import org.opensearch.action.search.SearchRequest; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.action.search.SearchScrollRequest; import org.opensearch.action.support.WriteRequest; import org.opensearch.action.update.UpdateRequest; import org.opensearch.action.update.UpdateResponse; import org.opensearch.common.inject.Inject; +import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentType; @@ -43,6 +49,13 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.index.engine.VersionConflictEngineException; +import org.opensearch.index.query.AbstractQueryBuilder; +import org.opensearch.index.query.BoolQueryBuilder; +import org.opensearch.index.query.MatchAllQueryBuilder; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.search.Scroll; +import org.opensearch.search.SearchHit; +import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.security.spi.resources.sharing.CreatedBy; import org.opensearch.security.spi.resources.sharing.Recipient; import org.opensearch.security.spi.resources.sharing.Recipients; @@ -233,6 +246,185 @@ public void indexResourceSharing( } } + /** + * Fetches all resource sharing records that match the specified system index. This method retrieves + * a get of resource IDs associated with the given system index from the resource sharing index. + * + *

The method executes the following steps: + *

    + *
  1. Creates a search request with term query matching the system index
    2. + *
  2. Applies source filtering to only fetch resource_id field
    3. + *
  3. Executes the search with a limit of 10000 documents
    4. + *
  4. Processes the results to extract resource IDs
    5. + *
+ * + *

Example query structure: + * 

+     * {
+     *   "query": {
+     *     "term": {
+     *       "source_idx": "resource_index_name"
+     *     }
+     *   },
+     *   "_source": ["resource_id"],
+     *   "size": 10000
+     * }
+     *
+ * + * @param resourceIndex The source index to match against the source_idx field + * @param listener The listener to be notified when the operation completes. + * The listener receives a set of resource IDs as a result. + * @apiNote This method: + *
    + *
  • Uses source filtering for optimal performance
    • + *
  • Performs exact matching on the source_idx field
    • + *
  • Returns an empty get instead of throwing exceptions
    • + *
+ */ + public void fetchAllResourceIds(String resourceIndex, ActionListener> listener) { + String resourceSharingIndex = getSharingIndex(resourceIndex); + LOGGER.debug("Fetching all documents asynchronously from {}", resourceSharingIndex); + Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); + + try (ThreadContext.StoredContext ctx = threadPool.getThreadContext().stashContext()) { + final SearchRequest searchRequest = new SearchRequest(resourceSharingIndex); + searchRequest.scroll(scroll); + + MatchAllQueryBuilder query = QueryBuilders.matchAllQuery(); + + executeSearchRequest(scroll, searchRequest, query, ActionListener.wrap(resourceIds -> { + ctx.restore(); + LOGGER.debug("Found {} documents in {}", resourceIds.size(), resourceSharingIndex); + listener.onResponse(resourceIds); + }, exception -> { + LOGGER.error("Search failed while locating all records inside resourceIndex={} ", resourceIndex, exception); + listener.onFailure(exception); + })); + } + } + + /** + * Helper method to fetch own and shared documents based on action-group match. + * This method uses scroll API to handle large result sets efficiently. + * + * + * @param resourceIndex The source index to match against the source_idx field + * @param entities Set of values to match in the specified Recipient field. Used for logging. ActionGroupQuery is already updated with these values. + * @param actionGroupQuery The query to match against the action-group field + * @param listener The listener to be notified when the operation completes. + * The listener receives a set of resource IDs as a result. + * @throws RuntimeException if the search operation fails + * @apiNote This method: + *
    + *
  • Uses scroll API with 1-minute timeout
    • + *
  • Processes results in batches of 1000 documents
    • + *
  • Performs source filtering for optimization
    • + *
  • Uses nested queries for accessing array elements
    • + *
  • Properly cleans up scroll context after use
    • + *
+ */ + public void fetchAccessibleResourceIds( + String resourceIndex, + Set entities, + BoolQueryBuilder actionGroupQuery, + ActionListener> listener + ) { + final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); + + try (ThreadContext.StoredContext ctx = this.threadPool.getThreadContext().stashContext()) { + // Search the RESOURCE INDEX directly (not the *-sharing index) + SearchRequest searchRequest = new SearchRequest(resourceIndex); + searchRequest.scroll(scroll); + + // 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.keyword", entities)); + + executeIdCollectingSearchRequest(scroll, searchRequest, boolQuery, ActionListener.wrap(resourceIds -> { + ctx.restore(); + LOGGER.debug("Found {} accessible resources in {}", resourceIds.size(), resourceIndex); + listener.onResponse(resourceIds); + }, exception -> { + LOGGER.error("Search failed for resourceIndex={}, entities={}", resourceIndex, entities, exception); + listener.onFailure(exception); + })); + } + } + + /** + * Executes a search request against the resource index and collects _id values (resource IDs) using scroll. + * + * @param scroll Search scroll context + * @param searchRequest Initial search request + * @param query Query builder for the request + * @param listener Listener to receive the collected resource IDs + */ + private void executeIdCollectingSearchRequest( + Scroll scroll, + SearchRequest searchRequest, + AbstractQueryBuilder> query, + ActionListener> listener + ) { + SearchSourceBuilder ssb = new SearchSourceBuilder().query(query).size(1000).fetchSource(false); // we only need _id + + searchRequest.source(ssb); + + StepListener searchStep = new StepListener<>(); + client.search(searchRequest, searchStep); + + searchStep.whenComplete(initialResponse -> { + Set collectedResourceIds = new HashSet<>(); + String scrollId = initialResponse.getScrollId(); + processScrollIds(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); + }, listener::onFailure); + } + + /** + * Recursively processes scroll results and collects hit IDs. + * + * @param collectedResourceIds Internal accumulator for resource IDs + * @param scroll Scroll context + * @param scrollId Scroll ID + * @param hits Search hits + * @param listener Listener to receive final set of resource IDs + */ + private void processScrollIds( + Set collectedResourceIds, + Scroll scroll, + String scrollId, + SearchHit[] hits, + ActionListener> listener + ) { + if (hits == null || hits.length == 0) { + clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onResponse(collectedResourceIds), listener::onFailure)); + return; + } + + for (SearchHit hit : hits) { + // Resource ID is the document _id in the resource index + collectedResourceIds.add(hit.getId()); + } + + SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); + client.searchScroll( + scrollRequest, + ActionListener.wrap( + scrollResponse -> processScrollIds( + collectedResourceIds, + scroll, + scrollResponse.getScrollId(), + scrollResponse.getHits().getHits(), + listener + ), + e -> clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onFailure(e), ex -> { + e.addSuppressed(ex); + listener.onFailure(e); + })) + ) + ); + } + /** * Fetches a specific resource sharing document by its resource ID and system resourceIndex. * This method performs an exact match search and parses the result into a ResourceSharing object. @@ -673,4 +865,101 @@ public void deleteResourceSharingRecord(String resourceId, String resourceIndex, } } + /** + * Executes a search request and returns a set of collected resource IDs using scroll. + * + * @param scroll Search scroll context + * @param searchRequest Initial search request + * @param query Query builder for the request + * @param listener Listener to receive the collected resource IDs + */ + private void executeSearchRequest( + Scroll scroll, + SearchRequest searchRequest, + AbstractQueryBuilder> query, + ActionListener> listener + ) { + SearchSourceBuilder searchSourceBuilder = new SearchSourceBuilder().query(query) + .size(1000) + .fetchSource(new String[] { "resource_id" }, null); + + searchRequest.source(searchSourceBuilder); + + StepListener searchStep = new StepListener<>(); + client.search(searchRequest, searchStep); + + searchStep.whenComplete(initialResponse -> { + Set collectedResourceIds = new HashSet<>(); + String scrollId = initialResponse.getScrollId(); + processScrollResults(collectedResourceIds, scroll, scrollId, initialResponse.getHits().getHits(), listener); + }, listener::onFailure); + } + + /** + * Recursively processes scroll results and collects resource IDs. + * + * @param collectedResourceIds Internal accumulator for resource IDs + * @param scroll Scroll context + * @param scrollId Scroll ID + * @param hits Search hits + * @param listener Listener to receive final set of resource IDs + */ + private void processScrollResults( + Set collectedResourceIds, + Scroll scroll, + String scrollId, + SearchHit[] hits, + ActionListener> listener + ) { + if (hits == null || hits.length == 0) { + clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onResponse(collectedResourceIds), listener::onFailure)); + return; + } + + for (SearchHit hit : hits) { + Map source = hit.getSourceAsMap(); + if (source != null && source.containsKey("resource_id")) { + collectedResourceIds.add(source.get("resource_id").toString()); + } + } + + SearchScrollRequest scrollRequest = new SearchScrollRequest(scrollId).scroll(scroll); + client.searchScroll( + scrollRequest, + ActionListener.wrap( + scrollResponse -> processScrollResults( + collectedResourceIds, + scroll, + scrollResponse.getScrollId(), + scrollResponse.getHits().getHits(), + listener + ), + e -> clearScroll(scrollId, ActionListener.wrap(ignored -> listener.onFailure(e), ex -> { + e.addSuppressed(ex); + listener.onFailure(e); + })) + ) + ); + } + + /** + * Clears scroll context after scrolling is complete or on error. + * + * @param scrollId Scroll ID to clear + * @param listener Listener to notify when clearing is done + */ + private void clearScroll(String scrollId, ActionListener listener) { + if (scrollId == null) { + listener.onResponse(null); + return; + } + + ClearScrollRequest clearScrollRequest = new ClearScrollRequest(); + clearScrollRequest.addScrollId(scrollId); + client.clearScroll(clearScrollRequest, ActionListener.wrap(r -> listener.onResponse(null), e -> { + LOGGER.warn("Failed to clear scroll context", e); + listener.onResponse(null); + })); + } + } diff --git a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java index 1185822cdf..5803c5e6f0 100644 --- a/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java +++ b/src/test/java/org/opensearch/security/resources/ResourceAccessHandlerTest.java @@ -205,6 +205,42 @@ public void testHasPermission_pluginUserDenied() { verify(listener).onResponse(false); } + @Test + public void testGetOwnAndSharedResources_asAdmin() { + User admin = new User("admin", ImmutableSet.of(), ImmutableSet.of(), null, ImmutableMap.of(), false); + injectUser(admin); + when(adminDNs.isAdmin(admin)).thenReturn(true); + + ActionListener> listener = mock(ActionListener.class); + + doAnswer(inv -> { + ActionListener> l = inv.getArgument(1); + l.onResponse(Set.of("res1", "res2")); + return null; + }).when(sharingIndexHandler).fetchAllResourceIds(eq(INDEX), any()); + + handler.getOwnAndSharedResourceIdsForCurrentUser(INDEX, listener); + verify(listener).onResponse(Set.of("res1", "res2")); + } + + @Test + public void testGetOwnAndSharedResources_asNormalUser() { + User user = new User("alice", ImmutableSet.of("r1"), ImmutableSet.of("b1"), null, ImmutableMap.of(), false); + injectUser(user); + when(adminDNs.isAdmin(user)).thenReturn(false); + + ActionListener> listener = mock(ActionListener.class); + + doAnswer(inv -> { + ActionListener> l = inv.getArgument(3); + l.onResponse(Set.of("res1")); + return null; + }).when(sharingIndexHandler).fetchAccessibleResourceIds(any(), any(), any(), any()); + + handler.getOwnAndSharedResourceIdsForCurrentUser(INDEX, listener); + verify(listener).onResponse(Set.of("res1")); + } + @Test public void testShareSuccess() { User user = new User("user2", ImmutableSet.of(), ImmutableSet.of(), null, ImmutableMap.of(), false); From f6519f028d40a2362120a3f5a4dd353bb06c6846 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 14:17:42 -0400 Subject: [PATCH 18/27] Re-add to client Signed-off-by: Craig Perkins --- .../java/org/opensearch/sample/resource/TestUtils.java | 4 ---- .../spi/resources/client/ResourceSharingClient.java | 9 +++++++++ .../security/resources/ResourceAccessHandler.java | 10 ++++++++++ 3 files changed, 19 insertions(+), 4 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 fbd58b8510..7723eb5e72 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 @@ -467,10 +467,6 @@ private void assertPostSearch( assertThat(response.getBody(), containsString(expectedResourceName)); } } - - try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - TestRestClient.HttpResponse response = client.postJson(endpoint, searchPayload); - } } public void assertDirectGetAll(TestSecurityConfig.User user, int status, String expectedResourceName) { diff --git a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java index c2811b6415..9a92020254 100644 --- a/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java +++ b/spi/src/main/java/org/opensearch/security/spi/resources/client/ResourceSharingClient.java @@ -8,6 +8,8 @@ package org.opensearch.security.spi.resources.client; +import java.util.Set; + import org.opensearch.core.action.ActionListener; import org.opensearch.security.spi.resources.sharing.ResourceSharing; import org.opensearch.security.spi.resources.sharing.ShareWith; @@ -45,4 +47,11 @@ public interface ResourceSharingClient { * @param listener The listener to be notified with the updated ResourceSharing document. */ void revoke(String resourceId, String resourceIndex, ShareWith target, ActionListener listener); + + /** + * Lists resourceIds of all shareable resources accessible by the current user. + * @param resourceIndex The index containing the resources. + * @param listener The listener to be notified with the set of accessible resources. + */ + void getAccessibleResourceIds(String resourceIndex, ActionListener> listener); } diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index 4c0bc95fd2..f69517e203 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -386,4 +386,14 @@ public void revoke( listener.onFailure(exception); })); } + + /** + * Loads all resources within the specified resource index. + * + * @param resourceIndex The resource index to load resources from. + * @param listener The listener to be notified with the set of resource IDs. + */ + private void loadAllResources(String resourceIndex, ActionListener> listener) { + this.resourceSharingIndexHandler.fetchAllResourceIds(resourceIndex, listener); + } } From ddb9a7c0ec0526d0c735a34876c1668758ac3c26 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 14:21:33 -0400 Subject: [PATCH 19/27] Use pluginClient in get Signed-off-by: Craig Perkins --- .../transport/GetResourceTransportAction.java | 75 ++++++++----------- 1 file changed, 30 insertions(+), 45 deletions(-) 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 d973cdd1c0..3ab5e2fe59 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 @@ -11,7 +11,6 @@ import java.io.IOException; import java.util.Arrays; import java.util.Set; -import java.util.function.Consumer; import java.util.stream.Collectors; import org.opensearch.ResourceNotFoundException; @@ -20,7 +19,6 @@ import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; import org.opensearch.common.inject.Inject; -import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.action.ActionListener; @@ -32,11 +30,11 @@ import org.opensearch.sample.resource.actions.rest.get.GetResourceAction; import org.opensearch.sample.resource.actions.rest.get.GetResourceRequest; import org.opensearch.sample.resource.actions.rest.get.GetResourceResponse; +import org.opensearch.sample.utils.PluginClient; import org.opensearch.search.SearchHit; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.tasks.Task; import org.opensearch.transport.TransportService; -import org.opensearch.transport.client.node.NodeClient; import static org.opensearch.sample.utils.Constants.RESOURCE_INDEX_NAME; @@ -45,14 +43,12 @@ */ public class GetResourceTransportAction extends HandledTransportAction { - private final TransportService transportService; - private final NodeClient nodeClient; + private final PluginClient pluginClient; @Inject - public GetResourceTransportAction(TransportService transportService, ActionFilters actionFilters, NodeClient nodeClient) { + public GetResourceTransportAction(TransportService transportService, ActionFilters actionFilters, PluginClient pluginClient) { super(GetResourceAction.NAME, transportService, actionFilters, GetResourceRequest::new); - this.transportService = transportService; - this.nodeClient = nodeClient; + this.pluginClient = pluginClient; } @Override @@ -67,40 +63,36 @@ protected void doExecute(Task task, GetResourceRequest request, ActionListener listener) { - withThreadContext(stashed -> { - SearchSourceBuilder ssb = new SearchSourceBuilder().size(1000).query(QueryBuilders.matchAllQuery()); + SearchSourceBuilder ssb = new SearchSourceBuilder().size(1000).query(QueryBuilders.matchAllQuery()); - SearchRequest req = new SearchRequest(RESOURCE_INDEX_NAME).source(ssb); - nodeClient.search(req, ActionListener.wrap(searchResponse -> { - SearchHit[] hits = searchResponse.getHits().getHits(); - if (hits.length == 0) { - listener.onFailure(new ResourceNotFoundException("No resources found in index: " + RESOURCE_INDEX_NAME)); - } else { - Set resources = Arrays.stream(hits).map(hit -> { - try { - return parseResource(hit.getSourceAsString()); - } catch (IOException e) { - throw new RuntimeException(e); - } - }).collect(Collectors.toSet()); - listener.onResponse(new GetResourceResponse(resources)); - } - }, listener::onFailure)); - }); + SearchRequest req = new SearchRequest(RESOURCE_INDEX_NAME).source(ssb); + pluginClient.search(req, ActionListener.wrap(searchResponse -> { + SearchHit[] hits = searchResponse.getHits().getHits(); + if (hits.length == 0) { + listener.onFailure(new ResourceNotFoundException("No resources found in index: " + RESOURCE_INDEX_NAME)); + } else { + Set resources = Arrays.stream(hits).map(hit -> { + try { + return parseResource(hit.getSourceAsString()); + } catch (IOException e) { + throw new RuntimeException(e); + } + }).collect(Collectors.toSet()); + listener.onResponse(new GetResourceResponse(resources)); + } + }, listener::onFailure)); } private void fetchResourceById(String resourceId, ActionListener listener) { - withThreadContext(stashed -> { - GetRequest req = new GetRequest(RESOURCE_INDEX_NAME, resourceId); - nodeClient.get(req, ActionListener.wrap(resp -> { - if (resp.isSourceEmpty()) { - listener.onFailure(new ResourceNotFoundException("Resource " + resourceId + " not found.")); - } else { - SampleResource resource = parseResource(resp.getSourceAsString()); - listener.onResponse(new GetResourceResponse(Set.of(resource))); - } - }, listener::onFailure)); - }); + GetRequest req = new GetRequest(RESOURCE_INDEX_NAME, resourceId); + pluginClient.get(req, ActionListener.wrap(resp -> { + if (resp.isSourceEmpty()) { + listener.onFailure(new ResourceNotFoundException("Resource " + resourceId + " not found.")); + } else { + SampleResource resource = parseResource(resp.getSourceAsString()); + listener.onResponse(new GetResourceResponse(Set.of(resource))); + } + }, listener::onFailure)); } private SampleResource parseResource(String json) throws IOException { @@ -112,11 +104,4 @@ private SampleResource parseResource(String json) throws IOException { } } - private void withThreadContext(Consumer action) { - ThreadContext tc = transportService.getThreadPool().getThreadContext(); - try (ThreadContext.StoredContext st = tc.stashContext()) { - action.accept(st); - } - } - } From 7e10713fa6c853cb7746a7a119158590f9af7207 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 14:32:21 -0400 Subject: [PATCH 20/27] Change to SecurityRoles Signed-off-by: Craig Perkins --- .../opensearch/security/privileges/dlsfls/IndexToRuleMap.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index a024b64e59..3ad5e7ebf2 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -82,8 +82,8 @@ public static IndexToRuleMap resourceRestrictions( principals.add("user:" + user.getName()); // owner // Security roles (OpenSearch Security roles) - if (user.getRoles() != null) { - user.getRoles().forEach(r -> principals.add("role:" + r)); + if (user.getSecurityRoles() != null) { + user.getSecurityRoles().forEach(r -> principals.add("role:" + r)); } // Backend roles (LDAP/SAML/etc) From a3b35112b61f6c1c6eafb1e8f8e02d4985951964 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 15:38:25 -0400 Subject: [PATCH 21/27] Add new test for sharing with roles Signed-off-by: Craig Perkins --- .../sample/resource/ShareApiTests.java | 2 +- .../opensearch/sample/resource/TestUtils.java | 53 ++++++++++++++----- .../feature/enabled/ApiAccessTests.java | 4 +- .../enabled/DirectIndexAccessTests.java | 2 +- .../feature/enabled/DryRunAccessTests.java | 2 +- .../ResourceSharingMultiTenancyTests.java | 2 +- .../AdminCertificateAccessTests.java | 2 +- .../enabled/multi_share/FullAccessTests.java | 4 +- .../enabled/multi_share/MixedAccessTests.java | 45 +++++++++++++--- .../multi_share/PubliclySharedDocTests.java | 4 +- .../multi_share/ReadOnlyAccessTests.java | 2 +- .../privileges/ResourceAccessEvaluator.java | 2 + 12 files changed, 91 insertions(+), 33 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/ShareApiTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/ShareApiTests.java index d1b7ad08ca..c395091fd7 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/ShareApiTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/ShareApiTests.java @@ -79,7 +79,7 @@ public static class RoutesTests extends BaseTests { @Before public void setup() { adminResId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); + api.awaitSharingEntry(adminResId); } @Test 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 7723eb5e72..e328dee0b8 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 @@ -75,13 +75,11 @@ public final class TestUtils { public static final TestSecurityConfig.ActionGroup sampleReadOnlyAG = new TestSecurityConfig.ActionGroup( "sample_plugin_index_read_access", TestSecurityConfig.ActionGroup.Type.INDEX, - "indices:data/read*", "cluster:admin/sample-resource-plugin/get" ); public static final TestSecurityConfig.ActionGroup sampleAllAG = new TestSecurityConfig.ActionGroup( "sample_plugin_index_all_access", TestSecurityConfig.ActionGroup.Type.INDEX, - "indices:*", "cluster:admin/sample-resource-plugin/*", "cluster:admin/security/resource/share" ); @@ -163,6 +161,18 @@ public static String revokeAccessPayload(String user, String accessLevel) { } + public static String shareWithRolePayload(String role, String accessLevel) { + return """ + { + "share_with": { + "%s" : { + "roles": ["%s"] + } + } + } + """.formatted(accessLevel, role); + } + static String migrationPayload_valid() { return """ { @@ -565,6 +575,22 @@ public void assertApiRevoke( assertRevoke(SAMPLE_RESOURCE_REVOKE_ENDPOINT + "/" + resourceId, user, target, accessLevel, status); } + public void assertApiShareByRole( + String resourceId, + TestSecurityConfig.User user, + String targetRole, + String accessLevel, + int status + ) { + try (TestRestClient client = cluster.getRestClient(user)) { + TestRestClient.HttpResponse response = client.postJson( + SAMPLE_RESOURCE_SHARE_ENDPOINT + "/" + resourceId, + shareWithRolePayload(targetRole, accessLevel) + ); + response.assertStatusCode(status); + } + } + private void assertRevoke( String endpoint, TestSecurityConfig.User user, @@ -597,20 +623,21 @@ private void assertDelete(String endpoint, TestSecurityConfig.User user, int sta } } - public void awaitSharingEntry() { - awaitSharingEntry("admin"); + public void awaitSharingEntry(String resourceId) { + awaitSharingEntry(resourceId, "admin"); } - public void awaitSharingEntry(String expectedString) { + public void awaitSharingEntry(String resourceId, String expectedString) { try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { - Awaitility.await("Wait for sharing entry").pollInterval(Duration.ofMillis(500)).untilAsserted(() -> { - TestRestClient.HttpResponse response = client.get(RESOURCE_SHARING_INDEX + "/_search"); - response.assertStatusCode(200); - String hitsJson = response.bodyAsMap().get("hits").toString(); - assertThat(hitsJson, containsString(expectedString)); - int size = response.bodyAsJsonNode().get("hits").get("hits").size(); - assertThat(size, greaterThanOrEqualTo(1)); - }); + Awaitility.await("Wait for sharing entry for resource " + resourceId) + .pollInterval(Duration.ofMillis(500)) + .untilAsserted(() -> { + TestRestClient.HttpResponse response = client.get(RESOURCE_SHARING_INDEX + "/_doc/" + resourceId); + response.assertStatusCode(200); + String body = response.getBody(); + assertThat(body, containsString(expectedString)); + assertThat(body, containsString(resourceId)); + }); } } } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java index 607d6bb4d6..24269b0fb4 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ApiAccessTests.java @@ -70,7 +70,7 @@ public static class SystemIndexEnabled { @Before public void setup() { adminResId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(adminResId); // wait until sharing entry is created } @After @@ -290,7 +290,7 @@ public static class SystemIndexDisabled { @Before public void setup() { adminResId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); + api.awaitSharingEntry(adminResId); } @Test 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 835820bad2..03996ab0fe 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 @@ -180,7 +180,7 @@ public static class SystemIndexDisabled { @Before public void setup() { adminResId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(adminResId); // wait until sharing entry is created } @After diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DryRunAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DryRunAccessTests.java index bb5000742c..fdcb7ec3bb 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DryRunAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DryRunAccessTests.java @@ -60,7 +60,7 @@ public class DryRunAccessTests { @Before public void setup() { adminResId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(adminResId); // wait until sharing entry is created } @After diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ResourceSharingMultiTenancyTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ResourceSharingMultiTenancyTests.java index 96706e4143..bb59352e36 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ResourceSharingMultiTenancyTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/ResourceSharingMultiTenancyTests.java @@ -44,7 +44,7 @@ public class ResourceSharingMultiTenancyTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN, new BasicHeader("securitytenant", "customtenant")); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created } @After diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/AdminCertificateAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/AdminCertificateAccessTests.java index 53c5a83da6..6192f31879 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/AdminCertificateAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/AdminCertificateAccessTests.java @@ -49,7 +49,7 @@ public class AdminCertificateAccessTests { public void adminCertificate_canCRUD() { TestUtils.ApiHelper api = new TestUtils.ApiHelper(cluster); String resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created try (TestRestClient client = cluster.getRestClient(cluster.getAdminCertificate())) { HttpResponse resp = client.get(SAMPLE_RESOURCE_GET_ENDPOINT + "/" + resourceId); resp.assertStatusCode(HttpStatus.SC_OK); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java index a05bc46505..c211998e04 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java @@ -45,7 +45,7 @@ public class FullAccessTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created } @After @@ -79,7 +79,7 @@ private void assertSharingAccess(TestSecurityConfig.User user, TestSecurityConfi api.assertApiShare(resourceId, target, new TestSecurityConfig.User("test"), sampleAllAG.name(), HttpStatus.SC_OK); api.assertApiRevoke(resourceId, user, target, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(); + api.awaitSharingEntry(resourceId); api.assertApiGet(resourceId, target, HttpStatus.SC_FORBIDDEN, ""); } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/MixedAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/MixedAccessTests.java index 0ea80a6539..e9ed20dd92 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/MixedAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/MixedAccessTests.java @@ -45,7 +45,7 @@ public class MixedAccessTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created } @After @@ -76,7 +76,7 @@ private void assertFullAccess(TestSecurityConfig.User user) { api.assertApiUpdate(resourceId, user, "sampleUpdateAdmin", HttpStatus.SC_OK); api.assertApiShare(resourceId, user, user, sampleAllAG.name(), HttpStatus.SC_OK); api.assertApiRevoke(resourceId, user, USER_ADMIN, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(); + api.awaitSharingEntry(resourceId); api.assertApiDelete(resourceId, user, HttpStatus.SC_OK); } @@ -87,15 +87,15 @@ public void multipleUsers_multipleLevels() { // 1. share at read-only for full-access user and at full-access for limited-perms user api.assertApiShare(resourceId, USER_ADMIN, FULL_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(FULL_ACCESS_USER.getName()); - api.awaitSharingEntry(LIMITED_ACCESS_USER.getName()); + api.awaitSharingEntry(resourceId, FULL_ACCESS_USER.getName()); + api.awaitSharingEntry(resourceId, LIMITED_ACCESS_USER.getName()); // 2. check read-only access for full-access user assertReadOnly(FULL_ACCESS_USER); // 3. limited access user shares with full-access user at sampleAllAG api.assertApiShare(resourceId, LIMITED_ACCESS_USER, FULL_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(FULL_ACCESS_USER.getName()); + api.awaitSharingEntry(resourceId, FULL_ACCESS_USER.getName()); // 4. full-access user now has full-access to admin's resource assertFullAccess(FULL_ACCESS_USER); @@ -109,7 +109,7 @@ public void multipleUsers_sameLevel() { // 1. share with both users at read-only level api.assertApiShare(resourceId, USER_ADMIN, FULL_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(sampleReadOnlyAG.name()); + api.awaitSharingEntry(resourceId, sampleReadOnlyAG.name()); // 2. assert both now have read-only access assertReadOnly(LIMITED_ACCESS_USER); @@ -121,17 +121,46 @@ public void sameUser_multipleLevels() { // 1. share with user at read-only level api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(LIMITED_ACCESS_USER.getName()); + api.awaitSharingEntry(resourceId, LIMITED_ACCESS_USER.getName()); // 2. assert user now has read-only access assertReadOnly(LIMITED_ACCESS_USER); // 3. share with user at full-access level api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(sampleAllAG.name()); + api.awaitSharingEntry(resourceId, sampleAllAG.name()); // 4. assert user now has full access assertFullAccess(LIMITED_ACCESS_USER); } + private String getActualRoleName(TestSecurityConfig.User user, String baseRoleName) { + return "user_" + user.getName() + "__" + baseRoleName; + } + + @Test + public void multipleRoles_multipleLevels() { + assertNoAccessBeforeSharing(FULL_ACCESS_USER); + assertNoAccessBeforeSharing(LIMITED_ACCESS_USER); + + String fullAccessUserRole = getActualRoleName(FULL_ACCESS_USER, "shared_role"); + String limitedAccessUserRole = getActualRoleName(LIMITED_ACCESS_USER, "shared_role_limited_perms"); + + // 1. share at read-only for shared_role and at full-access for shared_role_limited_perms + api.assertApiShareByRole(resourceId, USER_ADMIN, fullAccessUserRole, sampleReadOnlyAG.name(), HttpStatus.SC_OK); + api.assertApiShareByRole(resourceId, USER_ADMIN, limitedAccessUserRole, sampleAllAG.name(), HttpStatus.SC_OK); + api.awaitSharingEntry(resourceId, fullAccessUserRole); + api.awaitSharingEntry(resourceId, limitedAccessUserRole); + + // 2. check read-only access for FULL_ACCESS_USER (has shared_role) + assertReadOnly(FULL_ACCESS_USER); + + // 3. LIMITED_ACCESS_USER (has shared_role_limited_perms) shares with shared_role at sampleAllAG + api.assertApiShareByRole(resourceId, LIMITED_ACCESS_USER, fullAccessUserRole, sampleAllAG.name(), HttpStatus.SC_OK); + api.awaitSharingEntry(resourceId, fullAccessUserRole); + + // 4. FULL_ACCESS_USER now has full-access to admin's resource + assertFullAccess(FULL_ACCESS_USER); + } + } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java index 54f941e969..77cdda2461 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java @@ -45,7 +45,7 @@ public class PubliclySharedDocTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created } @After @@ -76,7 +76,7 @@ private void assertFullAccess() { api.assertApiUpdate(resourceId, LIMITED_ACCESS_USER, "sampleUpdateAdmin", HttpStatus.SC_OK); api.assertApiShare(resourceId, LIMITED_ACCESS_USER, TestUtils.LIMITED_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); api.assertApiRevoke(resourceId, LIMITED_ACCESS_USER, USER_ADMIN, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(); + api.awaitSharingEntry(resourceId); api.assertApiDelete(resourceId, LIMITED_ACCESS_USER, HttpStatus.SC_OK); } diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java index 1f0d842b31..dcbc79427e 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java @@ -48,7 +48,7 @@ public class ReadOnlyAccessTests { @Before public void setup() { resourceId = api.createSampleResourceAs(USER_ADMIN); - api.awaitSharingEntry(); // wait until sharing entry is created + api.awaitSharingEntry(resourceId); // wait until sharing entry is created } @After diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index 8bdde516d6..d4e69a2de6 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -17,6 +17,7 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.DocRequest; +import org.opensearch.action.get.GetRequest; import org.opensearch.common.settings.Settings; import org.opensearch.core.action.ActionListener; import org.opensearch.core.common.Strings; @@ -98,6 +99,7 @@ public boolean shouldEvaluate(ActionRequest request) { ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT ); if (!isResourceSharingFeatureEnabled) return false; + if (request instanceof GetRequest) return false; if (!(request instanceof DocRequest docRequest)) return false; if (Strings.isNullOrEmpty(docRequest.id())) { log.debug("Request id is blank or null, request is of type {}", docRequest.getClass().getName()); From b663a0e5da92e334cb53fe0482169a505d2a8eaa Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 15:44:28 -0400 Subject: [PATCH 22/27] Re-arrange Signed-off-by: Craig Perkins --- .../opensearch/security/privileges/ResourceAccessEvaluator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java index d4e69a2de6..1984028b77 100644 --- a/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/ResourceAccessEvaluator.java @@ -99,8 +99,8 @@ public boolean shouldEvaluate(ActionRequest request) { ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT ); if (!isResourceSharingFeatureEnabled) return false; - if (request instanceof GetRequest) return false; if (!(request instanceof DocRequest docRequest)) return false; + if (request instanceof GetRequest) return false; if (Strings.isNullOrEmpty(docRequest.id())) { log.debug("Request id is blank or null, request is of type {}", docRequest.getClass().getName()); return false; From a86c2a8af7868f51687d02989e67f6c3e2e3e160 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 5 Sep 2025 23:16:15 -0400 Subject: [PATCH 23/27] Fix tests Signed-off-by: Craig Perkins --- .../feature/enabled/DirectIndexAccessTests.java | 15 +++++++-------- .../enabled/multi_share/FullAccessTests.java | 8 ++++---- .../multi_share/PubliclySharedDocTests.java | 4 ++-- .../enabled/multi_share/ReadOnlyAccessTests.java | 6 +++--- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/DirectIndexAccessTests.java index 03996ab0fe..ffc1c29af9 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 @@ -66,7 +66,7 @@ private void assertResourceIndexAccess(String id, TestSecurityConfig.User user) HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } - api.assertDirectGet(id, user, HttpStatus.SC_FORBIDDEN, ""); + api.assertDirectGet(id, user, HttpStatus.SC_NOT_FOUND, ""); api.assertDirectUpdate(id, user, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); api.assertDirectDelete(id, user, HttpStatus.SC_FORBIDDEN); @@ -84,7 +84,7 @@ private void assertResourceSharingIndexAccess(String id, TestSecurityConfig.User @Before public void setUp() { id = api.createRawResourceAs(cluster.getAdminCertificate()); - api.awaitSharingEntry("kirk"); + api.awaitSharingEntry(id, "kirk"); } @After @@ -225,10 +225,10 @@ public void testRawAccess_limitedAccessUser() { resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } // cannot read admin's resource directly since system index protection is enabled - api.assertDirectGet(adminResId, LIMITED_ACCESS_USER, HttpStatus.SC_FORBIDDEN, ""); + api.assertDirectGet(adminResId, LIMITED_ACCESS_USER, HttpStatus.SC_OK, "sample"); // once admin share's record, user can then query it directly api.assertDirectShare(adminResId, USER_ADMIN, LIMITED_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(LIMITED_ACCESS_USER.getName()); + api.awaitSharingEntry(adminResId, LIMITED_ACCESS_USER.getName()); api.assertDirectGet(adminResId, LIMITED_ACCESS_USER, HttpStatus.SC_OK, "sample"); // should be able to access the record since user has direct index access @@ -259,15 +259,14 @@ public void testRawAccess_allAccessUser() { resp.assertStatusCode(HttpStatus.SC_CREATED); userResId = resp.getTextFromJsonBody("/_id"); } - // cannot read admin's resource directly since resource is not shared with them - api.assertDirectGet(adminResId, FULL_ACCESS_USER, HttpStatus.SC_FORBIDDEN, ""); + api.assertDirectGet(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK, "sample"); // once admin share's record, user can then query it directly api.assertDirectShare(adminResId, USER_ADMIN, FULL_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(FULL_ACCESS_USER.getName()); + api.awaitSharingEntry(adminResId, FULL_ACCESS_USER.getName()); api.assertDirectGet(adminResId, FULL_ACCESS_USER, HttpStatus.SC_OK, "sample"); // admin cannot read user's resource until after they share it with admin - api.assertDirectGet(userResId, USER_ADMIN, HttpStatus.SC_FORBIDDEN, ""); + api.assertDirectGet(userResId, USER_ADMIN, HttpStatus.SC_OK, "sample"); api.assertDirectShare(userResId, FULL_ACCESS_USER, USER_ADMIN, sampleReadOnlyAG.name(), HttpStatus.SC_OK); api.assertDirectGet(userResId, USER_ADMIN, HttpStatus.SC_OK, "sample"); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java index c211998e04..95d602a91c 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/FullAccessTests.java @@ -73,7 +73,7 @@ private void assertRUDAccess(TestSecurityConfig.User user) { private void assertSharingAccess(TestSecurityConfig.User user, TestSecurityConfig.User target) { api.assertApiGet(resourceId, target, HttpStatus.SC_FORBIDDEN, ""); api.assertApiShare(resourceId, user, target, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(target.getName()); + api.awaitSharingEntry(resourceId, target.getName()); api.assertApiGet(resourceId, target, HttpStatus.SC_OK, "sample"); api.assertApiShare(resourceId, target, new TestSecurityConfig.User("test"), sampleAllAG.name(), HttpStatus.SC_OK); @@ -88,7 +88,7 @@ public void fullAccessUser_canCRUD() { assertNoAccessBeforeSharing(FULL_ACCESS_USER); // share at sampleAllAG level api.assertApiShare(resourceId, USER_ADMIN, FULL_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(FULL_ACCESS_USER.getName()); // wait until sharing info is populated + api.awaitSharingEntry(resourceId, FULL_ACCESS_USER.getName()); // wait until sharing info is populated // can share admin's resource with others since full access was granted assertSharingAccess(FULL_ACCESS_USER, LIMITED_ACCESS_USER); @@ -101,7 +101,7 @@ public void limitedAccessUser_canCRUD() { assertNoAccessBeforeSharing(LIMITED_ACCESS_USER); // share at sampleAllAG level api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(LIMITED_ACCESS_USER.getName()); // wait until sharing info is populated + api.awaitSharingEntry(resourceId, LIMITED_ACCESS_USER.getName()); // wait until sharing info is populated assertSharingAccess(LIMITED_ACCESS_USER, FULL_ACCESS_USER); @@ -113,7 +113,7 @@ public void noAccessUser_canCRUD() { assertNoAccessBeforeSharing(NO_ACCESS_USER); // share at sampleAllAG level api.assertApiShare(resourceId, USER_ADMIN, NO_ACCESS_USER, sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(NO_ACCESS_USER.getName()); + api.awaitSharingEntry(resourceId, NO_ACCESS_USER.getName()); assertSharingAccess(NO_ACCESS_USER, LIMITED_ACCESS_USER); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java index 77cdda2461..bc19c30126 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/PubliclySharedDocTests.java @@ -85,7 +85,7 @@ public void readOnly() { assertNoAccessBeforeSharing(FULL_ACCESS_USER); // 1. share at read-only for full-access user and at full-access for limited perms user api.assertApiShare(resourceId, USER_ADMIN, new TestSecurityConfig.User("*"), sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry("*"); + api.awaitSharingEntry(resourceId, "*"); // 2. check read-only access for full-access user assertReadOnly(); @@ -96,7 +96,7 @@ public void fullAccess() { assertNoAccessBeforeSharing(LIMITED_ACCESS_USER); // 1. share at read-only for full-access user and at full-access for limited perms user api.assertApiShare(resourceId, USER_ADMIN, new TestSecurityConfig.User("*"), sampleAllAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry("*"); + api.awaitSharingEntry(resourceId, "*"); // 2. check read-only access for full-access user assertFullAccess(); diff --git a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java index dcbc79427e..e0fe8236b7 100644 --- a/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java +++ b/sample-resource-plugin/src/integrationTest/java/org/opensearch/sample/resource/feature/enabled/multi_share/ReadOnlyAccessTests.java @@ -82,7 +82,7 @@ public void fullAccessUser_canRead_cannotUpdateDeleteShareRevoke() { assertNoAccessBeforeSharing(FULL_ACCESS_USER); // share at sampleReadOnly level api.assertApiShare(resourceId, USER_ADMIN, FULL_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(FULL_ACCESS_USER.getName()); // wait until sharing info is populated + api.awaitSharingEntry(resourceId, FULL_ACCESS_USER.getName()); // wait until sharing info is populated assertReadOnly(FULL_ACCESS_USER); } @@ -91,7 +91,7 @@ public void limitedAccessUser_canRead_cannotUpdateDeleteShareRevoke() { assertNoAccessBeforeSharing(LIMITED_ACCESS_USER); // share at sampleReadOnly level api.assertApiShare(resourceId, USER_ADMIN, LIMITED_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(LIMITED_ACCESS_USER.getName()); // wait until sharing info is populated + api.awaitSharingEntry(resourceId, LIMITED_ACCESS_USER.getName()); // wait until sharing info is populated assertReadOnly(LIMITED_ACCESS_USER); } @@ -100,7 +100,7 @@ public void noAccessUser_canRead_cannotUpdateDeleteShareRevoke() { assertNoAccessBeforeSharing(NO_ACCESS_USER); // share at sampleReadOnly level api.assertApiShare(resourceId, USER_ADMIN, NO_ACCESS_USER, sampleReadOnlyAG.name(), HttpStatus.SC_OK); - api.awaitSharingEntry(NO_ACCESS_USER.getName()); // wait until sharing info is populated + api.awaitSharingEntry(resourceId, NO_ACCESS_USER.getName()); // wait until sharing info is populated assertReadOnly(NO_ACCESS_USER); } From 7a1207c9f387d5e38fb8f245d8ed66d5b52112ac Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Sat, 6 Sep 2025 20:52:58 -0400 Subject: [PATCH 24/27] Fix test Signed-off-by: Craig Perkins --- .../resource/feature/enabled/DirectIndexAccessTests.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) 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 ffc1c29af9..9077af1e8b 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 @@ -66,7 +66,11 @@ private void assertResourceIndexAccess(String id, TestSecurityConfig.User user) HttpResponse resp = client.postJson(RESOURCE_INDEX_NAME + "/_doc", sample); resp.assertStatusCode(HttpStatus.SC_FORBIDDEN); } - api.assertDirectGet(id, user, HttpStatus.SC_NOT_FOUND, ""); + if (NO_ACCESS_USER.getName().equals(user.getName())) { + api.assertDirectGet(id, user, HttpStatus.SC_FORBIDDEN, ""); + } else { + api.assertDirectGet(id, user, HttpStatus.SC_NOT_FOUND, ""); + } api.assertDirectUpdate(id, user, "sampleUpdateAdmin", HttpStatus.SC_FORBIDDEN); api.assertDirectDelete(id, user, HttpStatus.SC_FORBIDDEN); From 4a4f36b17f520ff6272410d461b1f5ddeb41266a Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Fri, 12 Sep 2025 16:08:08 -0400 Subject: [PATCH 25/27] Add plugin dev documentation Signed-off-by: Craig Perkins --- RESOURCE_SHARING_AND_ACCESS_CONTROL.md | 51 +++++++++++++++++++++++++- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md index f715ebc37b..8e334ae403 100644 --- a/RESOURCE_SHARING_AND_ACCESS_CONTROL.md +++ b/RESOURCE_SHARING_AND_ACCESS_CONTROL.md @@ -187,7 +187,56 @@ Each **action-group** entry contains the following access definitions: } ``` -## **4. Using the Client for Access Control** +## **4a. Filtering results based on authenticated user** + +When performing a search on a resource index, Security will automatically filter the results based on the logged in user without +a plugin having to be conscious of who the logged in user is. One of the goals of Resource Sharing and Authorization is to remove +reasons for why plugins must rely on [common-utils](https://github.com/opensearch-project/common-utils/) today. + +In order for this implicit filtering to work, plugins must declare themselves as an `IdentityAwarePlugin` and use their assigned +subject to run privileged operations against the resource index. See [Geospatial PR](https://github.com/opensearch-project/geospatial/pull/715) for an example +of how to make the switch for system index access. In future versions of OpenSearch, it will be required for plugins to replace usages of ThreadContext.stashContext to +access system indices. + +Behind-the-scenes, Security will filter the resultset based on who the authenticated user is. + +To accomplish this, Security will keep track of the list of principals that a particular resource is visible to as a new field on the resource +metadata itself in your plugin's resource index. + +For example: + +``` +{ + "name": "sharedDashboard", + "description": "A dashboard resource that is shared with multiple principals", + "type": "dashboard", + "created_at": "2025-09-02T14:30:00Z", + "attributes": { + "category": "analytics", + "sensitivity": "internal" + }, + "all_shared_principals": [ + "user:resource_sharing_test_user_alice", + "user:resource_sharing_test_user_bob", + "role:analytics_team", + "role:all_access", + "role:auditor" + ] +} +``` + +For some high-level pseudo code for a plugin writing an API to search or list resources: + +1. Plugin will expose an API to list resources. For example `/_plugins/_reports/definitions` is an API that reporting plugin exposes to list report definitons. +2. The plugin implementing search or list, will perform a plugin system-level request to search on the resource index. In the reporting plugin example, that would be a search on `.opendistro-reports-definitions` +3. Security will apply a DLS Filter behind-the-scenes to limit the result set based on the logged in user. + +For the example above, imagine the authenticated user has `username: resource_sharing_test_user_alice` and `role: analytics_team` + +Resource sharing will limit the resultset to documents that have either `user:resource_sharing_test_user_alice`, `role:analytics_team` or `user:*` +in the `all_shared_principals` section. Note that `user:*` is the convention used for publicly visible. + +## **4b. Using the Client for Access Control** [`opensearch-security-spi` README.md](./spi/README.md) is a great resource to learn more about the components of SPI and how to set up. From c19d4a1678f46b28821843327a26cc5d6c10ec4e Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 22 Sep 2025 09:53:17 -0400 Subject: [PATCH 26/27] Address PR feedback Signed-off-by: Craig Perkins --- .../configuration/DlsFlsValveImpl.java | 3 +- .../privileges/dlsfls/IndexToRuleMap.java | 52 +------------- .../resources/ResourceSharingDlsUtils.java | 67 +++++++++++++++++++ 3 files changed, 70 insertions(+), 52 deletions(-) create mode 100644 src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java diff --git a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java index 48267c4baf..f28129a980 100644 --- a/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java +++ b/src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java @@ -79,6 +79,7 @@ import org.opensearch.security.privileges.dlsfls.FieldMasking; import org.opensearch.security.privileges.dlsfls.IndexToRuleMap; import org.opensearch.security.resolver.IndexResolverReplacer; +import org.opensearch.security.resources.ResourceSharingDlsUtils; import org.opensearch.security.securityconf.DynamicConfigFactory; import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration; import org.opensearch.security.securityconf.impl.v7.RoleV7; @@ -168,7 +169,7 @@ public boolean invoke(PrivilegesEvaluationContext context, final ActionListener< && request instanceof SearchRequest && resourceIndicesMatcher.matchAll(resolved.getAllIndices())) { - IndexToRuleMap sharedResourceMap = IndexToRuleMap.resourceRestrictions( + IndexToRuleMap sharedResourceMap = ResourceSharingDlsUtils.resourceRestrictions( namedXContentRegistry, resolved, userSubject.getUser() diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index 3ad5e7ebf2..3d03d12bf3 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -10,21 +10,12 @@ */ package org.opensearch.security.privileges.dlsfls; -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import java.util.function.Predicate; import com.google.common.collect.ImmutableMap; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.opensearch.common.xcontent.XContentFactory; -import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.security.resolver.IndexResolverReplacer; -import org.opensearch.security.user.User; - /** * Maps index names to DLS/FLS/FM rules. *

@@ -40,7 +31,7 @@ public class IndexToRuleMap { private final ImmutableMap indexMap; - IndexToRuleMap(ImmutableMap indexMap) { + public IndexToRuleMap(ImmutableMap indexMap) { this.indexMap = indexMap; } @@ -70,45 +61,4 @@ public boolean containsAny(Predicate predicate) { public static IndexToRuleMap unrestricted() { return (IndexToRuleMap) UNRESTRICTED; } - - public static IndexToRuleMap resourceRestrictions( - NamedXContentRegistry xContentRegistry, - IndexResolverReplacer.Resolved resolved, - User user - ) { - - List principals = new ArrayList<>(); - principals.add("user:*"); // Convention for publicly visible - principals.add("user:" + user.getName()); // owner - - // Security roles (OpenSearch Security roles) - if (user.getSecurityRoles() != null) { - user.getSecurityRoles().forEach(r -> principals.add("role:" + r)); - } - - // Backend roles (LDAP/SAML/etc) - if (user.getRoles() != null) { - user.getRoles().forEach(br -> principals.add("backend:" + br)); - } - - XContentBuilder builder = null; - DlsRestriction restriction; - try { - // Build a single `terms` query JSON - builder = XContentFactory.jsonBuilder(); - builder.startObject().startObject("terms").array("all_shared_principals.keyword", principals.toArray()).endObject().endObject(); - - String dlsJson = builder.toString(); - restriction = new DlsRestriction(List.of(DocumentPrivileges.getRenderedDlsQuery(xContentRegistry, dlsJson))); - } catch (IOException e) { - LOGGER.warn("Received error while applying resource restrictions.", e); - restriction = DlsRestriction.FULL; - } - - ImmutableMap.Builder mapBuilder = ImmutableMap.builder(); - for (String index : resolved.getAllIndices()) { - mapBuilder.put(index, restriction); - } - return new IndexToRuleMap<>(mapBuilder.build()); - } } diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java new file mode 100644 index 0000000000..08290ec316 --- /dev/null +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java @@ -0,0 +1,67 @@ +/* + * 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.security.resources; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; + +import com.google.common.collect.ImmutableMap; + +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.security.privileges.dlsfls.DlsRestriction; +import org.opensearch.security.privileges.dlsfls.DocumentPrivileges; +import org.opensearch.security.privileges.dlsfls.IndexToRuleMap; +import org.opensearch.security.resolver.IndexResolverReplacer; +import org.opensearch.security.user.User; + +public class ResourceSharingDlsUtils { + public static IndexToRuleMap resourceRestrictions( + NamedXContentRegistry xContentRegistry, + IndexResolverReplacer.Resolved resolved, + User user + ) { + + List principals = new ArrayList<>(); + principals.add("user:*"); // Convention for publicly visible + principals.add("user:" + user.getName()); // owner + + // Security roles (OpenSearch Security roles) + if (user.getSecurityRoles() != null) { + user.getSecurityRoles().forEach(r -> principals.add("role:" + r)); + } + + // Backend roles (LDAP/SAML/etc) + if (user.getRoles() != null) { + user.getRoles().forEach(br -> principals.add("backend:" + br)); + } + + XContentBuilder builder = null; + DlsRestriction restriction; + try { + // Build a single `terms` query JSON + builder = XContentFactory.jsonBuilder(); + builder.startObject().startObject("terms").array("all_shared_principals.keyword", principals.toArray()).endObject().endObject(); + + String dlsJson = builder.toString(); + restriction = new DlsRestriction(List.of(DocumentPrivileges.getRenderedDlsQuery(xContentRegistry, dlsJson))); + } catch (IOException e) { + LOGGER.warn("Received error while applying resource restrictions.", e); + restriction = DlsRestriction.FULL; + } + + ImmutableMap.Builder mapBuilder = ImmutableMap.builder(); + for (String index : resolved.getAllIndices()) { + mapBuilder.put(index, restriction); + } + return new IndexToRuleMap<>(mapBuilder.build()); + } +} From 683f009eea3f48416bfd620321ec529e9db95698 Mon Sep 17 00:00:00 2001 From: Craig Perkins Date: Mon, 22 Sep 2025 10:04:35 -0400 Subject: [PATCH 27/27] Address more feedback Signed-off-by: Craig Perkins --- .../org/opensearch/security/OpenSearchSecurityPlugin.java | 2 +- .../opensearch/security/privileges/dlsfls/DlsRestriction.java | 2 +- .../opensearch/security/privileges/dlsfls/IndexToRuleMap.java | 3 --- .../security/resources/ResourceSharingDlsUtils.java | 4 ++++ 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 442259da3e..80e6441630 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -1204,7 +1204,7 @@ public Collection createComponents( threadPool, dlsFlsBaseContext, adminDns, - resourcePluginInfo != null ? resourcePluginInfo.getResourceIndices() : null + resourcePluginInfo.getResourceIndices() ); cr.subscribeOnChange(configMap -> { ((DlsFlsValveImpl) dlsFlsValve).updateConfiguration(cr.getConfiguration(CType.ROLES)); }); } diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/DlsRestriction.java b/src/main/java/org/opensearch/security/privileges/dlsfls/DlsRestriction.java index 242e0000a4..01fccb78e6 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/DlsRestriction.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/DlsRestriction.java @@ -53,7 +53,7 @@ public class DlsRestriction extends AbstractRuleBasedPrivileges.Rule { private final ImmutableList queries; - DlsRestriction(List queries) { + public DlsRestriction(List queries) { this.queries = ImmutableList.copyOf(queries); } diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java index 3d03d12bf3..191b3810cc 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/IndexToRuleMap.java @@ -13,8 +13,6 @@ import java.util.function.Predicate; import com.google.common.collect.ImmutableMap; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; /** * Maps index names to DLS/FLS/FM rules. @@ -26,7 +24,6 @@ * of the sub-classes of AbstractRuleBasedPrivileges. */ public class IndexToRuleMap { - private static final Logger LOGGER = LogManager.getLogger(IndexToRuleMap.class); private static final IndexToRuleMap UNRESTRICTED = new IndexToRuleMap(ImmutableMap.of()); private final ImmutableMap indexMap; diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java index 08290ec316..474bbe5dc8 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingDlsUtils.java @@ -13,6 +13,8 @@ import java.util.List; import com.google.common.collect.ImmutableMap; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.core.xcontent.NamedXContentRegistry; @@ -24,6 +26,8 @@ import org.opensearch.security.user.User; public class ResourceSharingDlsUtils { + private static final Logger LOGGER = LogManager.getLogger(ResourceSharingDlsUtils.class); + public static IndexToRuleMap resourceRestrictions( NamedXContentRegistry xContentRegistry, IndexResolverReplacer.Resolved resolved,