Skip to content

Commit

Permalink
Introduces resource permissions for detectors
Browse files Browse the repository at this point in the history
Signed-off-by: Darshit Chanpura <[email protected]>
  • Loading branch information
DarshitChanpura committed Dec 31, 2024
1 parent 926b2e1 commit 687e6d9
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 168 deletions.
18 changes: 18 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,18 @@
package org.opensearch.ad.constant;

import org.opensearch.accesscontrol.resources.ResourceAccessScope;

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

private final String scopeName;

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

public String getScopeName() {
return scopeName;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
import static org.opensearch.ad.constant.ADCommonMessages.FAIL_TO_CREATE_DETECTOR;
import static org.opensearch.ad.constant.ADCommonMessages.FAIL_TO_UPDATE_DETECTOR;
import static org.opensearch.ad.settings.AnomalyDetectorSettings.AD_FILTER_BY_BACKEND_ROLES;
import static org.opensearch.timeseries.util.ParseUtils.checkFilterByBackendRoles;
import static org.opensearch.timeseries.util.ParseUtils.getConfig;
import static org.opensearch.timeseries.util.ParseUtils.*;
import static org.opensearch.timeseries.util.RestHandlerUtils.wrapRestActionListener;

import java.util.List;
Expand Down Expand Up @@ -45,7 +44,6 @@
import org.opensearch.rest.RestRequest;
import org.opensearch.search.builder.SearchSourceBuilder;
import org.opensearch.tasks.Task;
import org.opensearch.timeseries.common.exception.TimeSeriesException;
import org.opensearch.timeseries.feature.SearchFeatureDao;
import org.opensearch.timeseries.function.ExecutorFunction;
import org.opensearch.timeseries.util.ParseUtils;
Expand Down Expand Up @@ -100,48 +98,25 @@ protected void doExecute(Task task, IndexAnomalyDetectorRequest request, ActionL
String errorMessage = method == RestRequest.Method.PUT ? FAIL_TO_UPDATE_DETECTOR : FAIL_TO_CREATE_DETECTOR;
ActionListener<IndexAnomalyDetectorResponse> listener = wrapRestActionListener(actionListener, errorMessage);
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
resolveUserAndExecute(user, detectorId, method, listener, (detector) -> adExecute(request, user, detector, context, listener));
resolveUserAndExecute(detectorId, method, listener, (detector) -> adExecute(request, user, detector, context, listener));
} catch (Exception e) {
LOG.error(e);
listener.onFailure(e);
}
}

private void resolveUserAndExecute(
User requestedUser,
String detectorId,
RestRequest.Method method,
ActionListener<IndexAnomalyDetectorResponse> listener,
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) {
String error = checkFilterByBackendRoles(requestedUser);
if (error != null) {
listener.onFailure(new TimeSeriesException(error));
return;
}
}

if (method == RestRequest.Method.PUT) {
// requestedUser == null means security is disabled or user is superadmin. In this case we don't need to
// check if request user have access to the detector or not. But we still need to get current detector for
// this case, so we can keep current detector's user data.
boolean filterByBackendRole = requestedUser == null ? false : filterByEnabled;
// Update detector request, check if user has permissions to update the detector
// Get detector and verify backend roles
getConfig(
requestedUser,
detectorId,
listener,
function,
client,
clusterService,
xContentRegistry,
filterByBackendRole,
AnomalyDetector.class
);
getConfig(detectorId, listener, function, client, clusterService, xContentRegistry, AnomalyDetector.class);
} else {
// Create Detector. No need to get current detector.
function.accept(null);
Expand Down Expand Up @@ -175,6 +150,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 @@ -43,7 +43,6 @@
import org.opensearch.common.inject.Inject;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.commons.authuser.User;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.NamedXContentRegistry;
Expand All @@ -55,7 +54,6 @@
import org.opensearch.timeseries.common.exception.TimeSeriesException;
import org.opensearch.timeseries.constant.CommonMessages;
import org.opensearch.timeseries.constant.CommonName;
import org.opensearch.timeseries.util.ParseUtils;
import org.opensearch.timeseries.util.RestHandlerUtils;
import org.opensearch.transport.TransportService;

Expand Down Expand Up @@ -103,13 +101,10 @@ protected void doExecute(
ActionListener<PreviewAnomalyDetectorResponse> actionListener
) {
String detectorId = request.getId();
User user = ParseUtils.getUserContext(client);
ActionListener<PreviewAnomalyDetectorResponse> listener = wrapRestActionListener(actionListener, FAIL_TO_PREVIEW_DETECTOR);
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
resolveUserAndExecute(
user,
detectorId,
filterByEnabled,
listener,
(anomalyDetector) -> previewExecute(request, context, listener),
client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.commons.authuser.User;
import org.opensearch.core.action.ActionListener;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.forecast.constant.ForecastCommonMessages;
Expand Down Expand Up @@ -71,7 +70,6 @@
import org.opensearch.timeseries.stats.StatNames;
import org.opensearch.timeseries.task.TaskCacheManager;
import org.opensearch.timeseries.transport.ResultProcessor;
import org.opensearch.timeseries.util.ParseUtils;
import org.opensearch.timeseries.util.SecurityClientUtil;
import org.opensearch.transport.TransportService;

Expand Down Expand Up @@ -154,13 +152,10 @@ public ForecastRunOnceTransportAction(
@Override
protected void doExecute(Task task, ForecastResultRequest request, ActionListener<ForecastResultResponse> listener) {
String forecastID = request.getConfigId();
User user = ParseUtils.getUserContext(client);
try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {

resolveUserAndExecute(
user,
forecastID,
filterByEnabled,
listener,
(forecaster) -> executeRunOnce(forecastID, request, listener),
client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@
import static org.opensearch.forecast.constant.ForecastCommonMessages.FAIL_TO_CREATE_FORECASTER;
import static org.opensearch.forecast.constant.ForecastCommonMessages.FAIL_TO_UPDATE_FORECASTER;
import static org.opensearch.forecast.settings.ForecastSettings.FORECAST_FILTER_BY_BACKEND_ROLES;
import static org.opensearch.timeseries.util.ParseUtils.checkFilterByBackendRoles;
import static org.opensearch.timeseries.util.ParseUtils.getConfig;
import static org.opensearch.timeseries.util.ParseUtils.getUserContext;
import static org.opensearch.timeseries.util.ParseUtils.*;
import static org.opensearch.timeseries.util.RestHandlerUtils.wrapRestActionListener;

import java.util.List;
Expand Down Expand Up @@ -100,7 +98,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 @@ -113,41 +110,16 @@ protected void doExecute(Task task, IndexForecasterRequest request, ActionListen
}

private void resolveUserAndExecute(
User requestedUser,
String forecasterId,
RestRequest.Method method,
ActionListener<IndexForecasterResponse> listener,
Consumer<Forecaster> function
) {
try {
// requestedUser == null means security is disabled or user is superadmin. In this case we don't need to
// check if request user have access to the forecaster or not. But we still need to get current forecaster for
// 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) {
String error = checkFilterByBackendRoles(requestedUser);
if (error != null) {
listener.onFailure(new IllegalArgumentException(error));
return;
}
}
if (method == RestRequest.Method.PUT) {
// Update forecaster request, check if user has permissions to update the forecaster
// Get forecaster and verify backend roles
getConfig(
requestedUser,
forecasterId,
listener,
function,
client,
clusterService,
xContentRegistry,
filterByBackendRole,
Forecaster.class
);
getConfig(forecasterId, listener, function, client, clusterService, xContentRegistry, Forecaster.class);
} else {
// Create Detector. No need to get current detector.
function.accept(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.SpecialPermission;
import org.opensearch.accesscontrol.resources.ResourceService;
import org.opensearch.action.ActionRequest;
import org.opensearch.ad.ADJobProcessor;
import org.opensearch.ad.ADTaskProfileRunner;
Expand Down Expand Up @@ -161,6 +162,10 @@
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.node.DiscoveryNodes;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.inject.Inject;
import org.opensearch.common.lifecycle.Lifecycle;
import org.opensearch.common.lifecycle.LifecycleComponent;
import org.opensearch.common.lifecycle.LifecycleListener;
import org.opensearch.common.settings.ClusterSettings;
import org.opensearch.common.settings.IndexScopedSettings;
import org.opensearch.common.settings.Setting;
Expand Down Expand Up @@ -267,10 +272,7 @@
import org.opensearch.jobscheduler.spi.ScheduledJobRunner;
import org.opensearch.monitor.jvm.JvmInfo;
import org.opensearch.monitor.jvm.JvmService;
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.ScriptPlugin;
import org.opensearch.plugins.SystemIndexPlugin;
import org.opensearch.plugins.*;
import org.opensearch.repositories.RepositoriesService;
import org.opensearch.rest.RestController;
import org.opensearch.rest.RestHandler;
Expand Down Expand Up @@ -327,7 +329,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,
ResourcePlugin {

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

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

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

@Override
public String getResourceIndex() {
return CommonName.CONFIG_INDEX;
}

@Override
public Collection<Class<? extends LifecycleComponent>> getGuiceServiceClasses() {
final List<Class<? extends LifecycleComponent>> services = new ArrayList<>(1);
services.add(GuiceHolder.class);
return services;
}

public static class GuiceHolder implements LifecycleComponent {

private static ResourceService resourceService;

@Inject
public GuiceHolder(final ResourceService resourceService) {
GuiceHolder.resourceService = resourceService;
}

public static ResourceService getResourceService() {
return resourceService;
}

@Override
public void close() {}

@Override
public Lifecycle.State lifecycleState() {
return null;
}

@Override
public void addLifecycleListener(LifecycleListener listener) {}

@Override
public void removeLifecycleListener(LifecycleListener listener) {}

@Override
public void start() {}

@Override
public void stop() {}

}
}
Loading

0 comments on commit 687e6d9

Please sign in to comment.