From cabbcd60d557103c263f56412c79600a4773a423 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 12 Dec 2024 16:48:28 -0500 Subject: [PATCH] Updates methods to return actual resources instead of resource ids Signed-off-by: Darshit Chanpura --- .../security/OpenSearchSecurityPlugin.java | 4 +- .../resources/ResourceAccessHandler.java | 26 +++---- .../ResourceSharingIndexHandler.java | 74 ++++++++++++++----- 3 files changed, 72 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index e0293a7abf..118b5bc88b 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -2208,8 +2208,8 @@ private void tryAddSecurityProvider() { } @Override - public Set getAccessibleResourcesForCurrentUser(String systemIndexName) { - return this.resourceAccessHandler.getAccessibleResourcesForCurrentUser(systemIndexName); + public Set getAccessibleResourcesForCurrentUser(String systemIndexName, Class clazz) { + return this.resourceAccessHandler.getAccessibleResourcesForCurrentUser(systemIndexName, clazz); } @Override diff --git a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java index 5d5b39b697..6837f88cbf 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java @@ -58,7 +58,7 @@ public ResourceAccessHandler( * @param resourceIndex The resource index to check for accessible resources. * @return A set of accessible resource IDs. */ - public Set getAccessibleResourcesForCurrentUser(String resourceIndex) { + public Set getAccessibleResourcesForCurrentUser(String resourceIndex, Class clazz) { final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER); if (user == null) { LOGGER.info("Unable to fetch user details "); @@ -69,24 +69,24 @@ public Set getAccessibleResourcesForCurrentUser(String resourceIndex) { // check if user is admin, if yes all resources should be accessible if (adminDNs.isAdmin(user)) { - return loadAllResources(resourceIndex); + return loadAllResources(resourceIndex, clazz); } - Set result = new HashSet<>(); + Set result = new HashSet<>(); // 0. Own resources - result.addAll(loadOwnResources(resourceIndex, user.getName())); + result.addAll(loadOwnResources(resourceIndex, user.getName(), clazz)); // 1. By username - result.addAll(loadSharedWithResources(resourceIndex, Set.of(user.getName()), EntityType.USERS.toString())); + result.addAll(loadSharedWithResources(resourceIndex, Set.of(user.getName()), EntityType.USERS.toString(), clazz)); // 2. By roles Set roles = user.getSecurityRoles(); - result.addAll(loadSharedWithResources(resourceIndex, roles, EntityType.ROLES.toString())); + result.addAll(loadSharedWithResources(resourceIndex, roles, EntityType.ROLES.toString(), clazz)); // 3. By backend_roles Set backendRoles = user.getRoles(); - result.addAll(loadSharedWithResources(resourceIndex, backendRoles, EntityType.BACKEND_ROLES.toString())); + result.addAll(loadSharedWithResources(resourceIndex, backendRoles, EntityType.BACKEND_ROLES.toString(), clazz)); return result; } @@ -210,8 +210,8 @@ public boolean deleteAllResourceSharingRecordsForCurrentUser() { * @param resourceIndex The resource index to load resources from. * @return A set of resource IDs. */ - private Set loadAllResources(String resourceIndex) { - return this.resourceSharingIndexHandler.fetchAllDocuments(resourceIndex); + private Set loadAllResources(String resourceIndex, Class clazz) { + return this.resourceSharingIndexHandler.fetchAllDocuments(resourceIndex, clazz); } /** @@ -221,8 +221,8 @@ private Set loadAllResources(String resourceIndex) { * @param userName The username of the owner. * @return A set of resource IDs owned by the user. */ - private Set loadOwnResources(String resourceIndex, String userName) { - return this.resourceSharingIndexHandler.fetchDocumentsByField(resourceIndex, "created_by.user", userName); + private Set loadOwnResources(String resourceIndex, String userName, Class clazz) { + return this.resourceSharingIndexHandler.fetchDocumentsByField(resourceIndex, "created_by.user", userName, clazz); } /** @@ -233,8 +233,8 @@ private Set loadOwnResources(String resourceIndex, String userName) { * @param entityType The type of entity (e.g., users, roles, backend_roles). * @return A set of resource IDs shared with the specified entities. */ - private Set loadSharedWithResources(String resourceIndex, Set entities, String entityType) { - return this.resourceSharingIndexHandler.fetchDocumentsForAllScopes(resourceIndex, entities, entityType); + private Set loadSharedWithResources(String resourceIndex, Set entities, String entityType, Class clazz) { + return this.resourceSharingIndexHandler.fetchDocumentsForAllScopes(resourceIndex, entities, entityType, clazz); } /** diff --git a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java index ec75515985..e53c1f1a56 100644 --- a/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java +++ b/src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java @@ -30,6 +30,9 @@ import org.opensearch.accesscontrol.resources.ShareWith; import org.opensearch.action.admin.indices.create.CreateIndexRequest; import org.opensearch.action.admin.indices.create.CreateIndexResponse; +import org.opensearch.action.get.MultiGetItemResponse; +import org.opensearch.action.get.MultiGetRequest; +import org.opensearch.action.get.MultiGetResponse; import org.opensearch.action.index.IndexRequest; import org.opensearch.action.index.IndexResponse; import org.opensearch.action.search.ClearScrollRequest; @@ -214,7 +217,7 @@ public ResourceSharing indexResourceSharing(String resourceId, String resourceIn *
  • Returns an empty list instead of throwing exceptions
    • * */ - public Set fetchAllDocuments(String pluginIndex) { + public Set fetchAllDocuments(String pluginIndex, Class clazz) { LOGGER.debug("Fetching all documents from {} where source_idx = {}", resourceSharingIndex, pluginIndex); try { @@ -242,7 +245,7 @@ public Set fetchAllDocuments(String pluginIndex) { LOGGER.debug("Found {} documents in {} for source_idx: {}", resourceIds.size(), resourceSharingIndex, pluginIndex); - return resourceIds; + return getResourcesFromIds(resourceIds, pluginIndex, clazz); } catch (Exception e) { LOGGER.error("Failed to fetch documents from {} for source_idx: {}", resourceSharingIndex, pluginIndex, e); @@ -316,9 +319,9 @@ public Set fetchAllDocuments(String pluginIndex) { * */ - public Set fetchDocumentsForAllScopes(String pluginIndex, Set entities, String entityType) { + public Set fetchDocumentsForAllScopes(String pluginIndex, Set entities, String entityType, Class clazz) { // "*" must match all scopes - return fetchDocumentsForAGivenScope(pluginIndex, entities, entityType, "*"); + return fetchDocumentsForAGivenScope(pluginIndex, entities, entityType, "*", clazz); } /** @@ -387,7 +390,13 @@ public Set fetchDocumentsForAllScopes(String pluginIndex, Set en *
  • Properly cleans up scroll context after use
    • * */ - public Set fetchDocumentsForAGivenScope(String pluginIndex, Set entities, String entityType, String scope) { + public Set fetchDocumentsForAGivenScope( + String pluginIndex, + Set entities, + String entityType, + String scope, + Class clazz + ) { LOGGER.debug( "Fetching documents from index: {}, where share_with.{}.{} contains any of {}", pluginIndex, @@ -426,11 +435,11 @@ public Set fetchDocumentsForAGivenScope(String pluginIndex, Set LOGGER.debug("Found {} documents matching the criteria in {}", resourceIds.size(), resourceSharingIndex); - return resourceIds; + return getResourcesFromIds(resourceIds, pluginIndex, clazz); } catch (Exception e) { LOGGER.error( - "Failed to fetch documents from {} for criteria - systemIndex: {}, scope: {}, entityType: {}, entities: {}", + "Failed to fetch documents from {} for criteria - pluginIndex: {}, scope: {}, entityType: {}, entities: {}", resourceSharingIndex, pluginIndex, scope, @@ -472,7 +481,7 @@ public Set fetchDocumentsForAGivenScope(String pluginIndex, Set * } * * - * @param systemIndex The source index to match against the source_idx field + * @param pluginIndex The source index to match against the source_idx field * @param field The field name to search in. Must be a valid field in the index mapping * @param value The value to match for the specified field. Performs exact term matching * @return Set List of resource IDs that match the criteria. Returns an empty list @@ -495,12 +504,12 @@ public Set fetchDocumentsForAGivenScope(String pluginIndex, Set * Set resources = fetchDocumentsByField("myIndex", "status", "active"); * */ - public Set fetchDocumentsByField(String systemIndex, String field, String value) { - if (StringUtils.isBlank(systemIndex) || StringUtils.isBlank(field) || StringUtils.isBlank(value)) { - throw new IllegalArgumentException("systemIndex, field, and value must not be null or empty"); + public Set fetchDocumentsByField(String pluginIndex, String field, String value, Class clazz) { + if (StringUtils.isBlank(pluginIndex) || StringUtils.isBlank(field) || StringUtils.isBlank(value)) { + throw new IllegalArgumentException("pluginIndex, field, and value must not be null or empty"); } - LOGGER.debug("Fetching documents from index: {}, where {} = {}", systemIndex, field, value); + LOGGER.debug("Fetching documents from index: {}, where {} = {}", pluginIndex, field, value); Set resourceIds = new HashSet<>(); final Scroll scroll = new Scroll(TimeValue.timeValueMinutes(1L)); @@ -510,14 +519,14 @@ public Set fetchDocumentsByField(String systemIndex, String field, Strin searchRequest.scroll(scroll); BoolQueryBuilder boolQuery = QueryBuilders.boolQuery() - .must(QueryBuilders.termQuery("source_idx.keyword", systemIndex)) + .must(QueryBuilders.termQuery("source_idx.keyword", pluginIndex)) .must(QueryBuilders.termQuery(field + ".keyword", value)); executeSearchRequest(resourceIds, scroll, searchRequest, boolQuery); LOGGER.info("Found {} documents in {} where {} = {}", resourceIds.size(), resourceSharingIndex, field, value); - return resourceIds; + return getResourcesFromIds(resourceIds, pluginIndex, clazz); } catch (Exception e) { LOGGER.error("Failed to fetch documents from {} where {} = {}", resourceSharingIndex, field, value, e); throw new RuntimeException("Failed to fetch documents: " + e.getMessage(), e); @@ -557,7 +566,7 @@ public Set fetchDocumentsByField(String systemIndex, String field, Strin * @return ResourceSharing object if a matching document is found, null if no document * matches the criteria * - * @throws IllegalArgumentException if systemIndexName or resourceId is null or empty + * @throws IllegalArgumentException if pluginIndexName or resourceId is null or empty * @throws RuntimeException if the search operation fails or parsing errors occur, * wrapping the underlying exception * @@ -581,7 +590,7 @@ public Set fetchDocumentsByField(String systemIndex, String field, Strin public ResourceSharing fetchDocumentById(String pluginIndex, String resourceId) { if (StringUtils.isBlank(pluginIndex) || StringUtils.isBlank(resourceId)) { - throw new IllegalArgumentException("systemIndexName and resourceId must not be null or empty"); + throw new IllegalArgumentException("pluginIndexName and resourceId must not be null or empty"); } LOGGER.debug("Fetching document from index: {}, with resourceId: {}", pluginIndex, resourceId); @@ -901,7 +910,7 @@ private boolean updateByQueryResourceSharing(String sourceIdx, String resourceId * Map> revokeAccess = new HashMap<>(); * revokeAccess.put(EntityType.USER, Set.of("user1", "user2")); * revokeAccess.put(EntityType.ROLE, Set.of("role1")); - * ResourceSharing updated = revokeAccess("resourceId", "systemIndex", revokeAccess); + * ResourceSharing updated = revokeAccess("resourceId", "pluginIndex", revokeAccess); * */ public ResourceSharing revokeAccess( @@ -1131,4 +1140,35 @@ public boolean deleteAllRecordsForUser(String name) { } } + /** + * Fetches all documents from the specified resource index and deserializes them into the specified class. + * + * @param resourceIndex The resource index to fetch documents from. + * @param clazz The class to deserialize the documents into. + * @return A set of deserialized documents. + */ + private Set getResourcesFromIds(Set resourceIds, String resourceIndex, Class clazz) { + + Set result = new HashSet<>(); + try { + MultiGetRequest request = new MultiGetRequest(); + for (String id : resourceIds) { + request.add(new MultiGetRequest.Item(resourceIndex, id)); + } + + MultiGetResponse response = client.multiGet(request).actionGet(); + + for (MultiGetItemResponse itemResponse : response.getResponses()) { + if (!itemResponse.isFailed() && itemResponse.getResponse().isExists()) { + String sourceAsString = itemResponse.getResponse().getSourceAsString(); + T resource = DefaultObjectMapper.readValue(sourceAsString, clazz); + result.add(resource); + } + } + } catch (Exception e) { + LOGGER.error("Failed to fetch resources with ids {} from index {}", resourceIds, resourceIndex, e); + } + + return result; + } }