Introduces resource permissions for detectors

.github/workflows/test_security.yml

@@ -1,5 +1,5 @@
 name: Security test workflow
-# This workflow is triggered on pull requests to main
+# This workflow is triggered on pull requests or pushes to any branch
 on:
   pull_request:
     branches:
@@ -16,34 +16,41 @@ jobs:
 
   security-test:
     needs: Get-CI-Image-Tag
-    # This job runs on Linux
     runs-on: ubuntu-latest
     container:
-      # using the same image which is used by opensearch-build team to build the OpenSearch Distribution
-      # this image tag is subject to change as more dependencies and updates will arrive over time
       image: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-version-linux }}
       options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }}
 
+    strategy:
+      matrix:
+        # empty string = disabled, flag = enabled
+        resource_sharing_flag: ["", "-Dresource_sharing.enabled=true"]
+
     steps:
       - name: Run start commands
         run: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-command }}
-      # This step uses the setup-java Github action: https://github.com/actions/setup-java
+
       - name: Set Up JDK
         uses: actions/setup-java@v4
         with:
-          distribution: temurin # Temurin is a distribution of adoptium
+          distribution: temurin
           java-version: 21
-      # index-management
+
       - name: Checkout Branch
         uses: actions/checkout@v4
+
       - name: Run integration tests
         run: |
           chown -R 1000:1000 `pwd`
-          su `id -un 1000` -c "./gradlew integTest -Dsecurity=true -Dhttps=true --tests '*IT'"
+          su `id -un 1000` -c "./gradlew integTest \
+            -Dsecurity=true \
+            -Dhttps=true \
+            ${{ matrix.resource_sharing_flag }} \
+            --tests '*IT'"
+
       - name: Upload failed logs
-        uses: actions/upload-artifact@v4
         if: failure()
+        uses: actions/upload-artifact@v4
         with:
-          name: logs
-          overwrite: 'true'
-          path: build/testclusters/integTest-*/logs/*
+          name: logs-${{ matrix.resource_sharing_flag != '' && 'resource-sharing' || 'no-resource-sharing' }}
+          path: build/testclusters/integTest-*/logs/*

CHANGELOG.md

@@ -6,6 +6,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 ## [Unreleased 3.x](https://github.com/opensearch-project/anomaly-detection/compare/3.0...HEAD)
 ### Features
 ### Enhancements
+- Use Centralized Resource Access Control framework provided by security plugin ([#1400](https://github.com/opensearch-project/anomaly-detection/pull/1400))
 ### Bug Fixes
 ### Infrastructure
 ### Documentation

build.gradle

@@ -132,13 +132,14 @@ configurations {
 }
 
 dependencies {
+    compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${opensearch_build}"
     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}"
     implementation "org.opensearch:common-utils:${common_utils_version}"
     implementation "org.opensearch.client:opensearch-rest-client:${opensearch_version}"
-    implementation group: 'com.google.guava', name: 'guava', version:'32.1.3-jre'
-    implementation group: 'com.google.guava', name: 'failureaccess', version:'1.0.2'
+    implementation group: 'com.google.guava', name: 'guava', version:'33.4.5-jre'
+    implementation group: 'com.google.guava', name: 'failureaccess', version:'1.0.3'
     implementation group: 'org.apache.commons', name: 'commons-math3', version: '3.6.1'
     implementation group: 'com.google.code.gson', name: 'gson', version: '2.11.0'
     implementation group: 'com.yahoo.datasketches', name: 'sketches-core', version: '0.13.4'
@@ -241,7 +242,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
@@ -261,9 +262,10 @@ configurations.all {
         force "com.google.code.gson:gson:2.11.0"
         force "junit:junit:4.13.2"
 
-        force "com.google.guava:guava:32.1.3-jre" // CVE for 31.1
+        force "com.google.guava:guava:33.4.5-jre" // CVE for 31.1
         force("com.fasterxml.jackson.core:jackson-core:${jacksonVersion}")
         force "org.eclipse.platform:org.eclipse.core.runtime:3.29.0" // CVE for < 3.29.0
+        force "org.ow2.asm:asm:9.7.1"
     }
 }
 
@@ -547,7 +549,9 @@ def configureClusterPlugins(cluster, jobSchedulerFile, securityPluginFile, secur
                 node.setting("plugins.security.check_snapshot_restore_write_privileges", "true")
                 node.setting("plugins.security.restapi.roles_enabled", "[\"all_access\", \"security_rest_api_access\"]")
                 node.setting("plugins.security.system_indices.enabled", "true")
-                // node.setting("plugins.security.system_indices.indices", "[\".opendistro-ism-config\"]")
+                if (System.getProperty("resource_sharing.enabled") == "true") {
+                    node.setting("plugins.security.experimental.resource_sharing.enabled", "true")
+                }
             }
         }
     }

src/main/java/org/opensearch/ad/transport/AnomalyDetectorJobTransportAction.java

@@ -60,7 +60,8 @@ public AnomalyDetectorJobTransportAction(
             FAIL_TO_START_DETECTOR,
             FAIL_TO_STOP_DETECTOR,
             AnomalyDetector.class,
-            adIndexJobActionHandler
+            adIndexJobActionHandler,
+            ADIndex.CONFIG.getIndexName()
         );
     }
 }

src/main/java/org/opensearch/ad/transport/DeleteAnomalyDetectorTransportAction.java

@@ -59,7 +59,8 @@ public DeleteAnomalyDetectorTransportAction(
             AnalysisType.AD,
             ADCommonName.DETECTION_STATE_INDEX,
             AnomalyDetector.class,
-            ADTaskType.HISTORICAL_DETECTOR_TASK_TYPES
+            ADTaskType.HISTORICAL_DETECTOR_TASK_TYPES,
+            ADIndex.CONFIG.getIndexName()
         );
     }
 }

src/main/java/org/opensearch/ad/transport/GetAnomalyDetectorTransportAction.java

@@ -81,7 +81,8 @@ public GetAnomalyDetectorTransportAction(
             ADTaskType.HISTORICAL_HC_DETECTOR.name(),
             ADTaskType.HISTORICAL_SINGLE_ENTITY.name(),
             AnomalyDetectorSettings.AD_FILTER_BY_BACKEND_ROLES,
-            adTaskProfileRunner
+            adTaskProfileRunner,
+            ADIndex.CONFIG.getIndexName()
         );
     }
 

src/main/java/org/opensearch/ad/transport/IndexAnomalyDetectorTransportAction.java

@@ -16,6 +16,8 @@
 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.shouldUseResourceAuthz;
+import static org.opensearch.timeseries.util.ParseUtils.verifyResourceAccessAndProcessRequest;
 import static org.opensearch.timeseries.util.RestHandlerUtils.wrapRestActionListener;
 
 import java.util.List;
@@ -27,6 +29,7 @@
 import org.opensearch.action.support.ActionFilters;
 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.action.support.WriteRequest;
+import org.opensearch.ad.indices.ADIndex;
 import org.opensearch.ad.indices.ADIndexManagement;
 import org.opensearch.ad.model.AnomalyDetector;
 import org.opensearch.ad.rest.handler.IndexAnomalyDetectorActionHandler;
@@ -99,8 +102,35 @@ protected void doExecute(Task task, IndexAnomalyDetectorRequest request, ActionL
         RestRequest.Method method = request.getMethod();
         String errorMessage = method == RestRequest.Method.PUT ? FAIL_TO_UPDATE_DETECTOR : FAIL_TO_CREATE_DETECTOR;
         ActionListener<IndexAnomalyDetectorResponse> listener = wrapRestActionListener(actionListener, errorMessage);
+
+        // TODO: Remove following and any other conditional check, post GA for Resource Authz.
+        boolean shouldEvaluateWithNewAuthz = shouldUseResourceAuthz(settings);
+
         try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
-            resolveUserAndExecute(user, detectorId, method, listener, (detector) -> adExecute(request, user, detector, context, listener));
+            verifyResourceAccessAndProcessRequest(
+                user,
+                ADIndex.CONFIG.getIndexName(),
+                detectorId,
+                shouldEvaluateWithNewAuthz,
+                listener,
+                args -> indexDetector(
+                    user,
+                    detectorId,
+                    method,
+                    listener,
+                    detector -> adExecute(request, user, detector, context, listener)
+                ),
+                new Object[] {},
+                (fallbackArgs) -> resolveUserAndExecute(
+                    user,
+                    detectorId,
+                    method,
+                    listener,
+                    (detector) -> adExecute(request, user, detector, context, listener)
+                ),
+                new Object[] {}
+            );
+
         } catch (Exception e) {
             LOG.error(e);
             listener.onFailure(e);
@@ -124,33 +154,44 @@ private void resolveUserAndExecute(
                     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
-                );
-            } else {
-                // Create Detector. No need to get current detector.
-                function.accept(null);
-            }
+
+            indexDetector(requestedUser, detectorId, method, listener, function);
         } catch (Exception e) {
             listener.onFailure(e);
         }
     }
 
+    private void indexDetector(
+        User requestedUser,
+        String detectorId,
+        RestRequest.Method method,
+        ActionListener<IndexAnomalyDetectorResponse> listener,
+        Consumer<AnomalyDetector> function
+    ) {
+        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
+            );
+        } else {
+            // Create Detector. No need to get current detector.
+            function.accept(null);
+        }
+    }
+
     protected void adExecute(
         IndexAnomalyDetectorRequest request,
         User user,
@@ -175,6 +216,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,

src/main/java/org/opensearch/ad/transport/PreviewAnomalyDetectorTransportAction.java

@@ -17,6 +17,8 @@
 import static org.opensearch.ad.settings.AnomalyDetectorSettings.MAX_CONCURRENT_PREVIEW;
 import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken;
 import static org.opensearch.timeseries.util.ParseUtils.resolveUserAndExecute;
+import static org.opensearch.timeseries.util.ParseUtils.shouldUseResourceAuthz;
+import static org.opensearch.timeseries.util.ParseUtils.verifyResourceAccessAndProcessRequest;
 import static org.opensearch.timeseries.util.RestHandlerUtils.wrapRestActionListener;
 
 import java.io.IOException;
@@ -34,6 +36,7 @@
 import org.opensearch.action.support.HandledTransportAction;
 import org.opensearch.ad.AnomalyDetectorRunner;
 import org.opensearch.ad.constant.ADCommonMessages;
+import org.opensearch.ad.indices.ADIndex;
 import org.opensearch.ad.model.AnomalyDetector;
 import org.opensearch.ad.model.AnomalyResult;
 import org.opensearch.ad.settings.AnomalyDetectorSettings;
@@ -65,6 +68,7 @@ public class PreviewAnomalyDetectorTransportAction extends
     private final AnomalyDetectorRunner anomalyDetectorRunner;
     private final ClusterService clusterService;
     private final Client client;
+    private final Settings settings;
     private final NamedXContentRegistry xContentRegistry;
     private volatile Integer maxAnomalyFeatures;
     private volatile Boolean filterByEnabled;
@@ -85,6 +89,7 @@ public PreviewAnomalyDetectorTransportAction(
         super(PreviewAnomalyDetectorAction.NAME, transportService, actionFilters, PreviewAnomalyDetectorRequest::new);
         this.clusterService = clusterService;
         this.client = client;
+        this.settings = settings;
         this.anomalyDetectorRunner = anomalyDetectorRunner;
         this.xContentRegistry = xContentRegistry;
         maxAnomalyFeatures = MAX_ANOMALY_FEATURES.get(settings);
@@ -105,18 +110,34 @@ protected void doExecute(
         String detectorId = request.getId();
         User user = ParseUtils.getUserContext(client);
         ActionListener<PreviewAnomalyDetectorResponse> listener = wrapRestActionListener(actionListener, FAIL_TO_PREVIEW_DETECTOR);
+
+        // TODO: Remove following and any other conditional check, post GA for Resource Authz.
+        boolean shouldEvaluateWithNewAuthz = shouldUseResourceAuthz(settings);
+
         try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) {
-            resolveUserAndExecute(
+            // Call the verifyResourceAccessAndProcessRequest method
+            verifyResourceAccessAndProcessRequest(
                 user,
+                ADIndex.CONFIG.getIndexName(),
                 detectorId,
-                filterByEnabled,
+                shouldEvaluateWithNewAuthz,
                 listener,
-                (anomalyDetector) -> previewExecute(request, context, listener),
-                client,
-                clusterService,
-                xContentRegistry,
-                AnomalyDetector.class
+                args -> previewExecute(request, context, listener),
+                new Object[] {},
+                (fallbackArgs) -> resolveUserAndExecute(
+                    user,
+                    detectorId,
+                    filterByEnabled,
+                    listener,
+                    ad -> previewExecute(request, context, listener),
+                    client,
+                    clusterService,
+                    xContentRegistry,
+                    AnomalyDetector.class
+                ),
+                new Object[] {}
             );
+
         } catch (Exception e) {
             logger.error(e);
             listener.onFailure(e);

src/main/java/org/opensearch/forecast/transport/DeleteForecasterTransportAction.java

@@ -52,7 +52,8 @@ public DeleteForecasterTransportAction(
             AnalysisType.FORECAST,
             ForecastIndex.STATE.getIndexName(),
             Forecaster.class,
-            ForecastTaskType.RUN_ONCE_TASK_TYPES
+            ForecastTaskType.RUN_ONCE_TASK_TYPES,
+            ForecastIndex.CONFIG.getIndexName()
         );
     }
 }