Skip to content

Commit

Permalink
Adds featureFlag control
Browse files Browse the repository at this point in the history
Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura committed Jan 20, 2025
1 parent d2a30d8 commit aedf1f3
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 17 deletions.
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ dependencies {
implementation "org.opensearch:opensearch:${opensearch_version}"
compileOnly "org.opensearch.plugin:opensearch-scripting-painless-spi:${opensearch_version}"
compileOnly "org.opensearch:opensearch-job-scheduler-spi:${job_scheduler_version}"
compileOnly "org.opensearch:opensearch-resource-sharing-spi:${opensearch_build}"
implementation "org.opensearch:common-utils:${common_utils_version}"
implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
compileOnly group: 'com.google.guava', name: 'guava', version:'32.1.3-jre'
Expand Down Expand Up @@ -205,7 +206,7 @@ opensearchplugin {
name 'opensearch-anomaly-detection'
description 'OpenSearch anomaly detector plugin'
classname 'org.opensearch.timeseries.TimeSeriesAnalyticsPlugin'
extendedPlugins = ['lang-painless', 'opensearch-job-scheduler']
extendedPlugins = ['lang-painless', 'opensearch-job-scheduler', 'opensearch-security;optional=true']
}

// Handle case where older versions of esplugin doesn't expose the joda time version it uses
Expand Down
19 changes: 19 additions & 0 deletions src/main/java/org/opensearch/ad/constant/ADResourceScope.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.opensearch.ad.constant;

import org.opensearch.security.spi.resources.ResourceAccessScope;

public enum ADResourceScope implements ResourceAccessScope<ADResourceScope> {
AD_READ_ACCESS("ad_read_access"),
AD_FULL_ACCESS("ad_full_access");

private final String scopeName;

ADResourceScope(String scopeName) {
this.scopeName = scopeName;
}

@Override
public String value() {
return scopeName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ private void resolveUserAndExecute(
Consumer<AnomalyDetector> function
) {
try {
// Check if user has backend roles
// When filter by is enabled, block users creating/updating detectors who do not have backend roles.
if (filterByEnabled) {
// If resource sharing flag is enabled then access evaluation will be performed at DLS level
if (!featureEnabled && filterByEnabled) {
// Check if user has backend roles
// When filter by is enabled, block users creating/updating detectors who do not have backend roles.
String error = checkFilterByBackendRoles(requestedUser);
if (error != null) {
listener.onFailure(new TimeSeriesException(error));
Expand Down Expand Up @@ -175,6 +176,8 @@ protected void adExecute(
checkIndicesAndExecute(detector.getIndices(), () -> {
// Don't replace detector's user when update detector
// Github issue: https://github.com/opensearch-project/anomaly-detection/issues/124
// TODO this and similar code should be updated to remove reference to a user

User detectorUser = currentDetector == null ? user : currentDetector.getUser();
IndexAnomalyDetectorActionHandler indexAnomalyDetectorActionHandler = new IndexAnomalyDetectorActionHandler(
clusterService,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ protected void doExecute(Task task, IndexForecasterRequest request, ActionListen
ActionListener<IndexForecasterResponse> listener = wrapRestActionListener(actionListener, errorMessage);
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
resolveUserAndExecute(
user,
forecasterId,
method,
listener,
Expand All @@ -125,9 +124,10 @@ private void resolveUserAndExecute(
// this case, so we can keep current forecaster's user data.
boolean filterByBackendRole = requestedUser == null ? false : filterByEnabled;

// Check if user has backend roles
// When filter by is enabled, block users creating/updating detectors who do not have backend roles.
if (filterByEnabled) {
// If resource sharing flag is enabled then access evaluation will be performed at DLS level
if (!featureEnabled && filterByEnabled) {
// Check if user has backend roles
// When filter by is enabled, block users creating/updating detectors who do not have backend roles.
String error = checkFilterByBackendRoles(requestedUser);
if (error != null) {
listener.onFailure(new IllegalArgumentException(error));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
import org.opensearch.script.ScriptService;
import org.opensearch.security.spi.resources.ResourceSharingExtension;
import org.opensearch.threadpool.ExecutorBuilder;
import org.opensearch.threadpool.ScalingExecutorBuilder;
import org.opensearch.threadpool.ThreadPool;
Expand Down Expand Up @@ -327,7 +328,13 @@
/**
* Entry point of time series analytics plugin.
*/
public class TimeSeriesAnalyticsPlugin extends Plugin implements ActionPlugin, ScriptPlugin, SystemIndexPlugin, JobSchedulerExtension {
public class TimeSeriesAnalyticsPlugin extends Plugin
implements
ActionPlugin,
ScriptPlugin,
SystemIndexPlugin,
JobSchedulerExtension,
ResourceSharingExtension {

private static final Logger LOG = LogManager.getLogger(TimeSeriesAnalyticsPlugin.class);

Expand Down Expand Up @@ -1758,4 +1765,14 @@ public void close() {
}
}
}

@Override
public String getResourceType() {
return "detectors";
}

@Override
public String getResourceIndex() {
return CommonName.CONFIG_INDEX;
}
}
22 changes: 14 additions & 8 deletions src/main/java/org/opensearch/timeseries/util/ParseUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -619,17 +619,23 @@ public static <ConfigType extends Config, GetConfigResponseType extends ActionRe
ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.nextToken(), parser);
@SuppressWarnings("unchecked")
ConfigType config = (ConfigType) Config.parseConfig(configTypeClass, parser);
User resourceUser = config.getUser();

if (!filterByBackendRole || checkUserPermissions(requestUser, resourceUser, configId) || isAdmin(requestUser)) {
if(featureFlagEnabled) {
// Permission evaluation will be done at DLS level in security plugin
function.accept(config);
} else {
logger.debug("User: " + requestUser.getName() + " does not have permissions to access config: " + configId);
listener
.onFailure(
new OpenSearchStatusException(CommonMessages.NO_PERMISSION_TO_ACCESS_CONFIG + configId, RestStatus.FORBIDDEN)
);
User resourceUser = config.getUser();

if (!filterByBackendRole || checkUserPermissions(requestUser, resourceUser, configId) || isAdmin(requestUser)) {
function.accept(config);
} else {
logger.debug("User: " + requestUser.getName() + " does not have permissions to access config: " + configId);
listener
.onFailure(
new OpenSearchStatusException(CommonMessages.NO_PERMISSION_TO_ACCESS_CONFIG + configId, RestStatus.FORBIDDEN)
);
}
}

} catch (Exception e) {
logger.error("Fail to parse user out of config", e);
listener.onFailure(new OpenSearchStatusException(CommonMessages.FAIL_TO_GET_USER_INFO + configId, RestStatus.BAD_REQUEST));
Expand Down

0 comments on commit aedf1f3

Please sign in to comment.