diff --git a/CHANGELOG.md b/CHANGELOG.md index 6eb0f8e66a..149d32253d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ 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] Keep track of tenant for sharable resources by persisting user requested tenant with sharing info ([#5588](https://github.com/opensearch-project/security/pull/5588)) ### Bug Fixes 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 5a35736f4e..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 @@ -23,7 +23,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; @@ -113,7 +112,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/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/ResourceIndexListener.java b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java index 7cb09accf4..2c8ed38c34 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java +++ b/src/main/java/org/opensearch/security/resources/ResourceIndexListener.java @@ -54,7 +54,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; } diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index 96e89cae7b..a5ffbfa3e2 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; @@ -50,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; @@ -135,6 +136,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("all_shared_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 @@ -177,7 +223,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 @@ -276,28 +337,101 @@ 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("all_shared_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. @@ -471,7 +605,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); @@ -560,7 +705,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); @@ -746,63 +902,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. *