Skip to content

Commit

Permalink
Updates revoke handler to use painless script
Browse files Browse the repository at this point in the history 
Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
@DarshitChanpura
DarshitChanpura committed Dec 5, 2024
1 parent ac53c8f commit 9e6ae85
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 71 deletions.
9 changes: 7 additions & 2 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2223,8 +2223,13 @@ public ResourceSharing shareWith(String resourceId, String systemIndexName, Shar
}

@Override
public ResourceSharing revokeAccess(String resourceId, String systemIndexName, Map<EntityType, List<String>> entities) {
return this.resourceAccessHandler.revokeAccess(resourceId, systemIndexName, entities);
public ResourceSharing revokeAccess(
String resourceId,
String systemIndexName,
Map<EntityType, List<String>> entities,
List<String> scopes
) {
return this.resourceAccessHandler.revokeAccess(resourceId, systemIndexName, entities, scopes);
}

@Override
Expand Down
9 changes: 7 additions & 2 deletions src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,16 @@ public ResourceSharing shareWith(String resourceId, String systemIndexName, Shar
return this.resourceSharingIndexHandler.updateResourceSharingInfo(resourceId, systemIndexName, createdBy, shareWith);
}

public ResourceSharing revokeAccess(String resourceId, String systemIndexName, Map<EntityType, List<String>> revokeAccess) {
public ResourceSharing revokeAccess(
String resourceId,
String systemIndexName,
Map<EntityType, List<String>> revokeAccess,
List<String> scopes
) {
final User user = threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_USER);
LOGGER.info("Revoking access to resource {} created by {} for {}", resourceId, user.getName(), revokeAccess);

return this.resourceSharingIndexHandler.revokeAccess(resourceId, systemIndexName, revokeAccess);
return this.resourceSharingIndexHandler.revokeAccess(resourceId, systemIndexName, revokeAccess, scopes);
}

public boolean deleteResourceSharingRecord(String resourceId, String systemIndexName) {
Expand Down
130 changes: 63 additions & 67 deletions src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.lucene.search.join.ScoreMode;

import org.opensearch.accesscontrol.resources.CreatedBy;
import org.opensearch.accesscontrol.resources.EntityType;
import org.opensearch.accesscontrol.resources.ResourceSharing;
import org.opensearch.accesscontrol.resources.ShareWith;
import org.opensearch.accesscontrol.resources.SharedWithScope;
import org.opensearch.action.admin.indices.create.CreateIndexRequest;
import org.opensearch.action.admin.indices.create.CreateIndexResponse;
import org.opensearch.action.index.IndexRequest;
Expand All @@ -50,6 +48,7 @@
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.index.query.BoolQueryBuilder;
import org.opensearch.index.query.MultiMatchQueryBuilder;
import org.opensearch.index.query.QueryBuilders;
import org.opensearch.index.reindex.BulkByScrollResponse;
import org.opensearch.index.reindex.DeleteByQueryAction;
Expand Down Expand Up @@ -156,8 +155,6 @@ public ResourceSharing indexResourceSharing(String resourceId, String resourceIn
.setSource(entry.toXContent(jsonBuilder(), ToXContent.EMPTY_PARAMS))
.request();

LOGGER.info("Index Request: {}", ir.toString());

ActionListener<IndexResponse> irListener = ActionListener.wrap(idxResponse -> {
LOGGER.info("Successfully created {} entry.", resourceSharingIndex);
}, (failResponse) -> {
Expand Down Expand Up @@ -405,14 +402,19 @@ public List<String> fetchDocumentsForAGivenScope(String pluginIndex, Set<String>
BoolQueryBuilder boolQuery = QueryBuilders.boolQuery().must(QueryBuilders.termQuery("source_idx.keyword", pluginIndex));

BoolQueryBuilder shouldQuery = QueryBuilders.boolQuery();
for (String entity : entities) {
shouldQuery.should(
QueryBuilders.nestedQuery(
"share_with." + scope + "." + entityType,
QueryBuilders.termQuery("share_with." + scope + "." + entityType, entity),
ScoreMode.None
)
);
if ("*".equals(scope)) {
// Wildcard behavior: Match any scope dynamically
for (String entity : entities) {
shouldQuery.should(
QueryBuilders.multiMatchQuery(entity, "share_with.*." + entityType + ".keyword")
.type(MultiMatchQueryBuilder.Type.BEST_FIELDS)
);
}
} else {
// Match the specific scope
for (String entity : entities) {
shouldQuery.should(QueryBuilders.termQuery("share_with." + scope + "." + entityType + ".keyword", entity));
}
}
shouldQuery.minimumShouldMatch(1);

Expand Down Expand Up @@ -666,7 +668,7 @@ private void executeSearchRequest(List<String> resourceIds, Scroll scroll, Searc

/**
* Updates the sharing configuration for an existing resource in the resource sharing index.
* NOTE: This method only grants new access. To remove access use {@link #revokeAccess(String, String, Map)}
* NOTE: This method only grants new access. To remove access use {@link #revokeAccess(String, String, Map, List)}
* This method modifies the sharing permissions for a specific resource identified by its
* resource ID and source index.
*
Expand Down Expand Up @@ -861,11 +863,12 @@ private boolean updateByQueryResourceSharing(String sourceIdx, String resourceId
* </pre>
*
* @param resourceId The ID of the resource from which to revoke access
* @param systemIndexName The name of the system index where the resource exists
* @param sourceIdx The name of the system index where the resource exists
* @param revokeAccess A map containing entity types (USER, ROLE, BACKEND_ROLE) and their corresponding
* values to be removed from the sharing configuration
* @param scopes A list of scopes to revoke access from. If null or empty, access is revoked from all scopes
* @return The updated ResourceSharing object after revoking access, or null if the document doesn't exist
* @throws IllegalArgumentException if resourceId, systemIndexName is null/empty, or if revokeAccess is null/empty
* @throws IllegalArgumentException if resourceId, sourceIdx is null/empty, or if revokeAccess is null/empty
* @throws RuntimeException if the update operation fails or encounters an error
*
* @see EntityType
Expand All @@ -881,65 +884,58 @@ private boolean updateByQueryResourceSharing(String sourceIdx, String resourceId
* ResourceSharing updated = revokeAccess("resourceId", "systemIndex", revokeAccess);
* </pre>
*/
public ResourceSharing revokeAccess(String resourceId, String systemIndexName, Map<EntityType, List<String>> revokeAccess) {
// TODO; check if this needs to be done per scope rather than for all scopes

if (StringUtils.isBlank(resourceId) || StringUtils.isBlank(systemIndexName) || revokeAccess == null || revokeAccess.isEmpty()) {
throw new IllegalArgumentException("resourceId, systemIndexName, and revokeAccess must not be null or empty");
public ResourceSharing revokeAccess(
String resourceId,
String sourceIdx,
Map<EntityType, List<String>> revokeAccess,
List<String> scopes
) {
if (StringUtils.isBlank(resourceId) || StringUtils.isBlank(sourceIdx) || revokeAccess == null || revokeAccess.isEmpty()) {
throw new IllegalArgumentException("resourceId, sourceIdx, and revokeAccess must not be null or empty");
}

LOGGER.debug("Revoking access for resource {} in {} for entities: {}", resourceId, systemIndexName, revokeAccess);
LOGGER.debug("Revoking access for resource {} in {} for entities: {} and scopes: {}", resourceId, sourceIdx, revokeAccess, scopes);

try {
ResourceSharing existingResource = fetchDocumentById(systemIndexName, resourceId);
if (existingResource == null) {
LOGGER.warn("No document found for resourceId: {} in index: {}", resourceId, systemIndexName);
return null;
}

ShareWith shareWith = existingResource.getShareWith();
boolean modified = false;

if (shareWith != null) {
for (SharedWithScope sharedWithScope : shareWith.getSharedWithScopes()) {
SharedWithScope.SharedWithPerScope sharedWithPerScope = sharedWithScope.getSharedWithPerScope();

for (Map.Entry<EntityType, List<String>> entry : revokeAccess.entrySet()) {
EntityType entityType = entry.getKey();
List<String> entities = entry.getValue();

// Check if the entity type exists in the share_with configuration
switch (entityType) {
case USERS:
if (sharedWithPerScope.getUsers() != null) {
modified = sharedWithPerScope.getUsers().removeAll(entities) || modified;
}
break;
case ROLES:
if (sharedWithPerScope.getRoles() != null) {
modified = sharedWithPerScope.getRoles().removeAll(entities) || modified;
// Revoke resource access
Script revokeScript = new Script(
ScriptType.INLINE,
"painless",
"""
if (ctx._source.share_with != null) {
List scopesToProcess = params.scopes == null || params.scopes.isEmpty() ? new ArrayList(ctx._source.share_with.keySet()) : params.scopes;
for (def scopeName : scopesToProcess) {
if (ctx._source.share_with.containsKey(scopeName)) {
def existingScope = ctx._source.share_with.get(scopeName);
for (def entry : params.revokeAccess.entrySet()) {
def entityType = entry.getKey();
def entitiesToRemove = entry.getValue();
if (existingScope.containsKey(entityType) && existingScope[entityType] != null) {
existingScope[entityType].removeAll(entitiesToRemove);
if (existingScope[entityType].isEmpty()) {
existingScope.remove(entityType);
}
}
}
if (existingScope.isEmpty()) {
ctx._source.share_with.remove(scopeName);
}
}
break;
case BACKEND_ROLES:
if (sharedWithPerScope.getBackendRoles() != null) {
modified = sharedWithPerScope.getBackendRoles().removeAll(entities) || modified;
}
break;
}
}
}
}
}

if (!modified) {
LOGGER.debug("No modifications needed for resource: {}", resourceId);
return existingResource;
}
""",
Map.of("revokeAccess", revokeAccess, "scopes", scopes)
);

// Update resource sharing info
return updateResourceSharingInfo(resourceId, systemIndexName, existingResource.getCreatedBy(), shareWith);
boolean success = updateByQueryResourceSharing(sourceIdx, resourceId, revokeScript);
return success ? fetchDocumentById(sourceIdx, resourceId) : null;

} catch (Exception e) {
LOGGER.error("Failed to revoke access for resource: {} in index: {}", resourceId, systemIndexName, e);
LOGGER.error("Failed to revoke access for resource: {} in index: {}", resourceId, sourceIdx, e);
throw new RuntimeException("Failed to revoke access: " + e.getMessage(), e);
}
}
Expand Down Expand Up @@ -986,7 +982,7 @@ public ResourceSharing revokeAccess(String resourceId, String systemIndexName, M
* </pre>
*/
public boolean deleteResourceSharingRecord(String resourceId, String sourceIdx) {
LOGGER.info("Deleting documents from {} where source_idx = {} and resource_id = {}", resourceSharingIndex, sourceIdx, resourceId);
LOGGER.debug("Deleting documents from {} where source_idx = {} and resource_id = {}", resourceSharingIndex, sourceIdx, resourceId);

try {
DeleteByQueryRequest dbq = new DeleteByQueryRequest(resourceSharingIndex).setQuery(
Expand Down Expand Up @@ -1074,7 +1070,7 @@ public boolean deleteAllRecordsForUser(String name) {
throw new IllegalArgumentException("Username must not be null or empty");
}

LOGGER.info("Deleting all records for user {}", name);
LOGGER.debug("Deleting all records for user {}", name);

try {
DeleteByQueryRequest deleteRequest = new DeleteByQueryRequest(resourceSharingIndex).setQuery(
Expand Down

0 comments on commit 9e6ae85

Please sign in to comment.