Skip to content

Commit

Permalink
Adds featureFlag for resource sharing
Browse files Browse the repository at this point in the history
Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura committed Jan 13, 2025
1 parent 6849242 commit 3e6531d
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
import static org.opensearch.security.Song.TITLE_POISON;
import static org.opensearch.security.Song.TITLE_SONG_1_PLUS_1;
import static org.opensearch.security.auditlog.impl.AuditCategory.INDEX_EVENT;
import static org.opensearch.security.support.ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
import static org.opensearch.test.framework.TestSecurityConfig.Role.ALL_ACCESS;
import static org.opensearch.test.framework.audit.AuditMessagePredicate.auditPredicate;
Expand Down Expand Up @@ -383,6 +384,7 @@ public class SearchOperationTest {
new AuditConfiguration(true).compliance(new AuditCompliance().enabled(true))
.filters(new AuditFilters().enabledRest(true).enabledTransport(true))
)
.nodeSettings(Map.of(OPENSEARCH_RESOURCE_SHARING_ENABLED, false))
.build();

@Rule
Expand Down
60 changes: 42 additions & 18 deletions src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -696,14 +696,19 @@ public List<RestHandler> getRestHandlers(
);

// Adds rest handlers for resource-access-control actions
handlers.addAll(
List.of(
new RestShareResourceAction(),
new RestRevokeResourceAccessAction(),
new RestListAccessibleResourcesAction(),
new RestVerifyResourceAccessAction()
)
);
if (settings.getAsBoolean(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT
)) {
handlers.addAll(
List.of(

Check warning on line 704 in src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L703-L704

Added lines #L703 - L704 were not covered by tests
new RestShareResourceAction(),
new RestRevokeResourceAccessAction(),
new RestListAccessibleResourcesAction(),
new RestVerifyResourceAccessAction()
)
);
}
log.debug("Added {} rest handler(s)", handlers.size());
}
}
Expand Down Expand Up @@ -733,14 +738,19 @@ public UnaryOperator<RestHandler> getRestHandlerWrapper(final ThreadContext thre
actions.add(new ActionHandler<>(WhoAmIAction.INSTANCE, TransportWhoAmIAction.class));

// Resource-access-control related actions
actions.addAll(
List.of(
new ActionHandler<>(ShareResourceAction.INSTANCE, TransportShareResourceAction.class),
new ActionHandler<>(RevokeResourceAccessAction.INSTANCE, TransportRevokeResourceAccessAction.class),
new ActionHandler<>(ListAccessibleResourcesAction.INSTANCE, TransportListAccessibleResourcesAction.class),
new ActionHandler<>(VerifyResourceAccessAction.INSTANCE, TransportVerifyResourceAccessAction.class)
)
);
if (settings.getAsBoolean(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT
)) {
actions.addAll(
List.of(

Check warning on line 746 in src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L745-L746

Added lines #L745 - L746 were not covered by tests
new ActionHandler<>(ShareResourceAction.INSTANCE, TransportShareResourceAction.class),
new ActionHandler<>(RevokeResourceAccessAction.INSTANCE, TransportRevokeResourceAccessAction.class),
new ActionHandler<>(ListAccessibleResourcesAction.INSTANCE, TransportListAccessibleResourcesAction.class),
new ActionHandler<>(VerifyResourceAccessAction.INSTANCE, TransportVerifyResourceAccessAction.class)
)
);
}
}
return actions;
}
Expand Down Expand Up @@ -1271,8 +1281,12 @@ public Collection<Object> createComponents(
);
resourceAccessHandler = new ResourceAccessHandler(threadPool, rsIndexHandler, adminDns);
resourceAccessHandler.initializeRecipientTypes();

rmr = ResourceSharingIndexManagementRepository.create(rsIndexHandler);
// Resource Sharing index is enabled by default
boolean isResourceSharingEnabled = settings.getAsBoolean(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT
);
rmr = ResourceSharingIndexManagementRepository.create(rsIndexHandler, isResourceSharingEnabled);

components.add(adminDns);
components.add(cr);
Expand Down Expand Up @@ -2139,6 +2153,16 @@ public List<Setting<?>> getSettings() {

// Privileges evaluation
settings.add(ActionPrivileges.PRECOMPUTED_PRIVILEGES_MAX_HEAP_SIZE);

// Resource Sharing
settings.add(
Setting.boolSetting(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT,
Property.NodeScope,
Property.Filtered
)
);
}

return settings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public class DlsFlsValveImpl implements DlsFlsRequestValve {
private final FieldMasking.Config fieldMaskingConfig;
private final Settings settings;
private final ResourceAccessHandler resourceAccessHandler;
private final boolean isResourceSharingEnabled;

public DlsFlsValveImpl(
Settings settings,
Expand All @@ -123,6 +124,10 @@ public DlsFlsValveImpl(
this.dlsFlsBaseContext = dlsFlsBaseContext;
this.settings = settings;
this.resourceAccessHandler = resourceAccessHandler;
this.isResourceSharingEnabled = settings.getAsBoolean(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT
);

clusterService.addListener(event -> {
DlsFlsProcessedConfig config = dlsFlsProcessedConfig.get();
Expand Down Expand Up @@ -390,11 +395,8 @@ public void handleSearchContext(SearchContext searchContext, ThreadPool threadPo
DlsRestriction dlsRestriction;

Set<String> resourceIds;
if (OpenSearchSecurityPlugin.getResourceIndices().contains(index)) {
if (this.isResourceSharingEnabled && OpenSearchSecurityPlugin.getResourceIndices().contains(index)) {
resourceIds = this.resourceAccessHandler.getAccessibleResourceIdsForCurrentUser(index);

Check warning on line 399 in src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java#L399

Added line #L399 was not covered by tests
if (resourceIds.isEmpty()) {
return;
}
// Create a DLS restriction to filter search results with accessible resources only
dlsRestriction = this.resourceAccessHandler.createResourceDLSRestriction(resourceIds, namedXContentRegistry);

Check warning on line 401 in src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/configuration/DlsFlsValveImpl.java#L401

Added line #L401 was not covered by tests

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public class SecurityFlsDlsIndexSearcherWrapper extends SystemIndexSearcherWrapp
private final Supplier<DlsFlsProcessedConfig> dlsFlsProcessedConfigSupplier;
private final DlsFlsBaseContext dlsFlsBaseContext;
private final ResourceAccessHandler resourceAccessHandler;
private final boolean isResourceSharingEnabled;

public SecurityFlsDlsIndexSearcherWrapper(
final IndexService indexService,
Expand Down Expand Up @@ -109,6 +110,10 @@ public SecurityFlsDlsIndexSearcherWrapper(
this.dlsFlsProcessedConfigSupplier = dlsFlsProcessedConfigSupplier;
this.dlsFlsBaseContext = dlsFlsBaseContext;
this.resourceAccessHandler = resourceAccessHandler;
this.isResourceSharingEnabled = settings.getAsBoolean(
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED,
ConfigConstants.OPENSEARCH_RESOURCE_SHARING_ENABLED_DEFAULT
);
}

@SuppressWarnings("unchecked")
Expand All @@ -123,9 +128,14 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm
}

String indexName = shardId != null ? shardId.getIndexName() : null;
Set<String> resourceIds = null;
if (!Strings.isNullOrEmpty(indexName) && OpenSearchSecurityPlugin.getResourceIndices().contains(indexName)) {
Set<String> resourceIds;
if (this.isResourceSharingEnabled
&& !Strings.isNullOrEmpty(indexName)
&& OpenSearchSecurityPlugin.getResourceIndices().contains(indexName)) {
resourceIds = this.resourceAccessHandler.getAccessibleResourceIdsForCurrentUser(indexName);

Check warning on line 135 in src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java#L135

Added line #L135 was not covered by tests
// resourceIds.isEmpty() indicates that the index is a resource index but the user does not have access to any resource under
// the
// index
if (resourceIds.isEmpty()) {
return new EmptyFilterLeafReader.EmptyDirectoryReader(reader);

Check warning on line 140 in src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/configuration/SecurityFlsDlsIndexSearcherWrapper.java#L140

Added line #L140 was not covered by tests
}
Expand All @@ -148,10 +158,7 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm
);
}

// resourceIds == null indicates that the index is not a resource index
// resourceIds.isEmpty() indicates that the index is a resource index but the user does not have access to any resource under the
// index
if (isAdmin || privilegesEvaluationContext == null || resourceIds == null) {
if (isAdmin || privilegesEvaluationContext == null) {
return new DlsFlsFilterLeafReader.DlsFlsDirectoryReader(
reader,
FieldPrivileges.FlsRule.ALLOW_ALL,
Expand All @@ -167,7 +174,6 @@ protected DirectoryReader dlsFlsWrap(final DirectoryReader reader, boolean isAdm
}

try {

DlsFlsProcessedConfig config = this.dlsFlsProcessedConfigSupplier.get();
DlsRestriction dlsRestriction;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,13 @@ public Query createResourceDLSQuery(Set<String> resourceIds, QueryShardContext q
*/
public DlsRestriction createResourceDLSRestriction(Set<String> resourceIds, NamedXContentRegistry xContentRegistry)
throws JsonProcessingException, PrivilegesConfigurationValidationException {

// resourceIds.isEmpty() is true when user doesn't have access to any resources
if (resourceIds.isEmpty()) {
LOGGER.debug("No resources found for user");
return DlsRestriction.FULL;

Check warning on line 404 in src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java#L403-L404

Added lines #L403 - L404 were not covered by tests
}

String jsonQuery = String.format(

Check warning on line 407 in src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java#L407

Added line #L407 was not covered by tests
"{ \"bool\": { \"filter\": [ { \"terms\": { \"_id\": %s } } ] } }",
DefaultObjectMapper.writeValueAsString(resourceIds, true)

Check warning on line 409 in src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceAccessHandler.java#L409

Added line #L409 was not covered by tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,16 @@ public void createResourceSharingIndexIfAbsent(Callable<Boolean> callable) {
CreateIndexRequest cir = new CreateIndexRequest(resourceSharingIndex).settings(INDEX_SETTINGS).waitForActiveShards(1);
ActionListener<CreateIndexResponse> cirListener = ActionListener.wrap(response -> {
LOGGER.info("Resource sharing index {} created.", resourceSharingIndex);

Check warning on line 125 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java#L123-L125

Added lines #L123 - L125 were not covered by tests
callable.call();
if (callable != null) {
callable.call();

Check warning on line 127 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java#L127

Added line #L127 was not covered by tests
}
}, (failResponse) -> {

Check warning on line 129 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java#L129

Added line #L129 was not covered by tests
/* Index already exists, ignore and continue */
LOGGER.info("Index {} already exists.", resourceSharingIndex);

Check warning on line 131 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java#L131

Added line #L131 was not covered by tests
try {
callable.call();
if (callable != null) {
callable.call();

Check warning on line 134 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexHandler.java#L134

Added line #L134 was not covered by tests
}
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,29 @@

package org.opensearch.security.resources;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

public class ResourceSharingIndexManagementRepository {

private static final Logger log = LogManager.getLogger(ResourceSharingIndexManagementRepository.class);

private final ResourceSharingIndexHandler resourceSharingIndexHandler;
private final boolean resourceSharingEnabled;

protected ResourceSharingIndexManagementRepository(final ResourceSharingIndexHandler resourceSharingIndexHandler) {
protected ResourceSharingIndexManagementRepository(
final ResourceSharingIndexHandler resourceSharingIndexHandler,
boolean isResourceSharingEnabled
) {
this.resourceSharingIndexHandler = resourceSharingIndexHandler;
this.resourceSharingEnabled = isResourceSharingEnabled;
}

public static ResourceSharingIndexManagementRepository create(ResourceSharingIndexHandler resourceSharingIndexHandler) {

return new ResourceSharingIndexManagementRepository(resourceSharingIndexHandler);
public static ResourceSharingIndexManagementRepository create(
ResourceSharingIndexHandler resourceSharingIndexHandler,
boolean isResourceSharingEnabled
) {
return new ResourceSharingIndexManagementRepository(resourceSharingIndexHandler, isResourceSharingEnabled);
}

/**
Expand All @@ -32,7 +44,10 @@ public static ResourceSharingIndexManagementRepository create(ResourceSharingInd
*/
public void createResourceSharingIndexIfAbsent() {
// TODO check if this should be wrapped in an atomic completable future
if (resourceSharingEnabled) {
log.info("Attempting to create Resource Sharing index");
this.resourceSharingIndexHandler.createResourceSharingIndexIfAbsent(() -> null);

Check warning on line 49 in src/main/java/org/opensearch/security/resources/ResourceSharingIndexManagementRepository.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/resources/ResourceSharingIndexManagementRepository.java#L48-L49

Added lines #L48 - L49 were not covered by tests
}

this.resourceSharingIndexHandler.createResourceSharingIndexIfAbsent(() -> null);
}
}
Loading

0 comments on commit 3e6531d

Please sign in to comment.