Onboards flow-framework plugin to resource-sharing and access control framework (#1251)

* Onboards flow-framework plugin to resource-sharing and access control framework

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds javadoc and a changelog entry

Signed-off-by: Darshit Chanpura <[email protected]>

* Adds tests for resource sharing flow and adds a CI job to run resource-sharing tests

Signed-off-by: Darshit Chanpura <[email protected]>

# Conflicts:
#	.github/workflows/test_security.yml

* Explicitly set security spi version to 3.4

Signed-off-by: Darshit Chanpura <[email protected]>

* Refactors test to use in house Recipient class

Signed-off-by: Darshit Chanpura <[email protected]>

* Address PR comments around renaming and version

Signed-off-by: Darshit Chanpura <[email protected]>

* Constant for awaitility version

Signed-off-by: Darshit Chanpura <[email protected]>

* Fix typo in awaitilityVersion

Co-authored-by: Owais Kazi <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>

* Update ResourceProvider to use anonymous class

Signed-off-by: Daniel Widdis <[email protected]>

* Fix javadocs

Signed-off-by: Daniel Widdis <[email protected]>

* Wrap checked exception from PluginClient

Signed-off-by: Daniel Widdis <[email protected]>

* Fix tests with param index and eq rather than raw strings

Signed-off-by: Daniel Widdis <[email protected]>

* Properly skip ResourceSharingApiIT

Signed-off-by: Daniel Widdis <[email protected]>

* Add unit test for ResourceSharingExtension

Signed-off-by: Daniel Widdis <[email protected]>

* Updates template to include `all_shared_principals` and marks relevant action-requests as DocRequests

Signed-off-by: Darshit Chanpura <[email protected]>

* Fix checkstyle errors

Signed-off-by: Darshit Chanpura <[email protected]>

* Fix early .onResponse return and add all_shared_principals to workflow state reset call

Signed-off-by: Darshit Chanpura <[email protected]>

* Add implementation of type method

Signed-off-by: Darshit Chanpura <[email protected]>

* Completes FlowFrameworkSecureRestApiIT

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes javadoc CI and addresses flakyness in search test when resource sharing is enab;ed

Signed-off-by: Darshit Chanpura <[email protected]>

* Fixes unit tests

Signed-off-by: Darshit Chanpura <[email protected]>

* Updates coverage and adds missing builder method call in Template class

Signed-off-by: Darshit Chanpura <[email protected]>

* Fix test to wait for async state deletion attempt

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Darshit Chanpura <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Co-authored-by: Daniel Widdis <[email protected]>
Co-authored-by: Owais Kazi <[email protected]>

Author: 3 people

.github/workflows/test_security.yml

@@ -20,6 +20,7 @@ jobs:
     strategy:
       matrix:
         java: [21, 25]
+        resource_sharing_flag: [ false, true ]
 
     name: Run Security Integration Tests on Linux
     runs-on: ubuntu-latest
@@ -45,4 +46,8 @@ jobs:
         # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip.
         run: |
           chown -R 1000:1000 `pwd`
-          su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true"
+          su `id -un 1000` -c "whoami && java -version && ./gradlew integTest \
+            -Dsecurity.enabled=true \
+            -Dhttps=true \
+            -Dresource_sharing.enabled=${{ matrix.resource_sharing_flag }} \
+            --tests '*IT'"

CHANGELOG.md

@@ -5,6 +5,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 
 ## [Unreleased 3.3](https://github.com/opensearch-project/flow-framework/compare/3.2...HEAD)
 ### Features
+- Onboards flow-framework plugin to resource-sharing and access control framework ([#1251](https://github.com/opensearch-project/flow-framework/pull/1251))
+
 ### Enhancements
 ### Bug Fixes
 - Pre-create ML Commons indices for Tenant Aware tests ([#1217](https://github.com/opensearch-project/flow-framework/pull/1217))

build.gradle

@@ -48,6 +48,7 @@ buildscript {
         parssonVersion = "1.1.7"
         swaggerVersion = "2.1.35"
         swaggerCoreVersion = "2.2.40"
+        awaitilityVersion= "4.3.0"
     }
 
     repositories {
@@ -95,6 +96,7 @@ opensearchplugin {
     classname = "${projectPath}.${pathToPlugin}.${pluginClassName}"
     licenseFile = rootProject.file('LICENSE')
     noticeFile = rootProject.file('NOTICE')
+    extendedPlugins = ['opensearch-security;optional=true']
 }
 
 dependencyLicenses.enabled = false
@@ -193,6 +195,9 @@ configurations {
 }
 
 dependencies {
+    // For resource access control
+    compileOnly("org.opensearch:opensearch-security-spi:${opensearch_build}")
+
     implementation("org.opensearch:opensearch:${opensearch_version}")
     api("org.opensearch:opensearch-ml-client:${opensearch_build}")
     // json, jsonpath, and commons-text are required by MLClient but must be provided by calling plugins
@@ -252,6 +257,8 @@ dependencies {
     testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}")
     testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}")
 
+    testImplementation("org.awaitility:awaitility:${awaitilityVersion}")
+
     // ZipArchive dependencies used for integration tests
     zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}")
     zipArchive("org.opensearch.plugin:opensearch-ml-plugin:${opensearch_build}")
@@ -354,6 +361,8 @@ integTest {
     systemProperty('user', user)
     systemProperty('password', password)
 
+    systemProperty "resource_sharing.enabled", System.getProperty("resource_sharing.enabled")
+
     // Only tenant aware test if set
     if (System.getProperty("tests.rest.tenantaware") == "true") {
         filter {
@@ -382,13 +391,15 @@ integTest {
     if (System.getProperty("https") == null || System.getProperty("https") == "false") {
         filter {
             excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT"
+            excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkResourceSharingRestApiIT"
         }
     }
 
     // Include only secure integration tests in security enabled clusters
     if (System.getProperty("https") != null && System.getProperty("https") == "true") {
         filter {
             includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT"
+            includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkResourceSharingRestApiIT"
             excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkRestApiIT"
             excludeTestsMatching "org.opensearch.flowframework.rest.*TenantAwareIT"
         }
@@ -513,6 +524,10 @@ testClusters.integTest {
                 '".plugins-flow-framework-state"' +
                 ']'
         )
+        if (System.getProperty("resource_sharing.enabled") == "true") {
+            setting("plugins.security.experimental.resource_sharing.enabled", "true")
+            setting("plugins.security.experimental.resource_sharing.protected_types", "[\"workflow\", \"workflow_state\"]")
+        }
         setSecure(true)
     }
 
@@ -545,7 +560,13 @@ testClusters.integTest {
     if (System.getProperty("opensearch.debug") != null) {
         def debugPort = 5005
         nodes.forEach { node ->
-            node.jvmArgs("-agentlib:jdwp=transport=dt_socket,server=n,suspend=y,address=*:${debugPort}")
+            // server=n,suspend=y -> node tries to connect to a debugger and hence test runs fails with
+            //            Exec output and error:
+            //            | Output for ./bin/opensearch-plugin:ERROR: transport error 202: connect failed: Connection refused
+            //            | ERROR: JDWP Transport dt_socket failed to initialize, TRANSPORT_INIT(510)
+            //                    | JDWP exit error AGENT_ERROR_TRANSPORT_INIT(197): No transports initialized [src/jdk.jdwp.agent/share/native/libjdwp/debugInit.c:700].
+            // So instead, we listen to a debugger by saying server=y and suspend=n
+            node.jvmArgs("-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:${debugPort}")
             debugPort += 1
         }
     }
@@ -573,6 +594,8 @@ task integTestRemote(type: RestIntegTestTask) {
     systemProperty 'cluster.number_of_nodes', "${_numNodes}"
     systemProperty 'tests.security.manager', 'false'
 
+    systemProperty "resource_sharing.enabled", System.getProperty("resource_sharing.enabled")
+
     // Run tests with remote cluster only if rest case is defined
     if (System.getProperty("tests.rest.cluster") != null) {
         filter {
@@ -584,6 +607,7 @@ task integTestRemote(type: RestIntegTestTask) {
     if (System.getProperty("https") == null || System.getProperty("https") == "false") {
         filter {
             excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT"
+            excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkResourceSharingRestApiIT"
         }
     }
 
@@ -592,6 +616,7 @@ task integTestRemote(type: RestIntegTestTask) {
         filter {
             includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT"
             excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkRestApiIT"
+            includeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkResourceSharingRestApiIT"
         }
     }
 }

src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java

@@ -57,11 +57,14 @@
 import org.opensearch.flowframework.transport.SearchWorkflowTransportAction;
 import org.opensearch.flowframework.transport.handler.SearchHandler;
 import org.opensearch.flowframework.util.EncryptorUtils;
+import org.opensearch.flowframework.util.PluginClient;
 import org.opensearch.flowframework.workflow.WorkflowProcessSorter;
 import org.opensearch.flowframework.workflow.WorkflowStepFactory;
+import org.opensearch.identity.PluginSubject;
 import org.opensearch.indices.SystemIndexDescriptor;
 import org.opensearch.ml.client.MachineLearningNodeClient;
 import org.opensearch.plugins.ActionPlugin;
+import org.opensearch.plugins.IdentityAwarePlugin;
 import org.opensearch.plugins.Plugin;
 import org.opensearch.plugins.SystemIndexPlugin;
 import org.opensearch.remote.metadata.client.SdkClient;
@@ -116,10 +119,12 @@
 /**
  * An OpenSearch plugin that enables builders to innovate AI apps on OpenSearch.
  */
-public class FlowFrameworkPlugin extends Plugin implements ActionPlugin, SystemIndexPlugin {
+public class FlowFrameworkPlugin extends Plugin implements ActionPlugin, SystemIndexPlugin, IdentityAwarePlugin {
 
     private FlowFrameworkSettings flowFrameworkSettings;
 
+    private PluginClient pluginClient;
+
     /**
      * Instantiate this plugin.
      */
@@ -143,8 +148,11 @@ public Collection<Object> createComponents(
         flowFrameworkSettings = new FlowFrameworkSettings(clusterService, settings);
         MachineLearningNodeClient mlClient = new MachineLearningNodeClient(client);
         boolean multiTenancyEnabled = FLOW_FRAMEWORK_MULTI_TENANCY_ENABLED.get(settings);
+
+        this.pluginClient = new PluginClient(client);
+
         SdkClient sdkClient = SdkClientFactory.createSdkClient(
-            client,
+            pluginClient,
             xContentRegistry,
             // Here we assume remote metadata client is only used with tenant awareness.
             // This may change in the future allowing more options for this map
@@ -296,4 +304,11 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
         );
     }
 
+    @Override
+    public void assignSubject(PluginSubject pluginSubject) {
+        if (this.pluginClient != null) {
+            this.pluginClient.setSubject(pluginSubject);
+        }
+    }
+
 }

src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java

@@ -0,0 +1,55 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+package org.opensearch.flowframework;
+
+import org.opensearch.flowframework.common.CommonValue;
+import org.opensearch.flowframework.util.ResourceSharingClientAccessor;
+import org.opensearch.security.spi.resources.ResourceProvider;
+import org.opensearch.security.spi.resources.ResourceSharingExtension;
+import org.opensearch.security.spi.resources.client.ResourceSharingClient;
+
+import java.util.Set;
+
+import static org.opensearch.flowframework.common.CommonValue.GLOBAL_CONTEXT_INDEX;
+import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_STATE_INDEX;
+
+/**
+ * Implementation for sharing resources that require access control.
+ */
+public class FlowFrameworkResourceSharingExtension implements ResourceSharingExtension {
+    @Override
+    public Set<ResourceProvider> getResourceProviders() {
+        return Set.of(new ResourceProvider() {
+            @Override
+            public String resourceType() {
+                return CommonValue.WORKFLOW_RESOURCE_TYPE;
+            }
+
+            @Override
+            public String resourceIndexName() {
+                return GLOBAL_CONTEXT_INDEX;
+            }
+        }, new ResourceProvider() {
+            @Override
+            public String resourceType() {
+                return CommonValue.WORKFLOW_STATE_RESOURCE_TYPE;
+            }
+
+            @Override
+            public String resourceIndexName() {
+                return WORKFLOW_STATE_INDEX;
+            }
+        });
+    }
+
+    @Override
+    public void assignResourceSharingClient(ResourceSharingClient resourceSharingClient) {
+        ResourceSharingClientAccessor.getInstance().setResourceSharingClient(resourceSharingClient);
+    }
+}

src/main/java/org/opensearch/flowframework/common/CommonValue.java

@@ -51,6 +51,8 @@ private CommonValue() {}
     public static final String CREATE_TIME = "create_time";
     /** The template field name for the user who created the workflow **/
     public static final String USER_FIELD = "user";
+    /** The template field name for the entities whom this workflow is shared with **/
+    public static final String ALL_SHARED_PRINCIPALS_FIELD = "all_shared_principals";
     /** The created time field */
     public static final String CREATED_TIME = "created_time";
     /** The last updated time field */
@@ -264,4 +266,10 @@ private CommonValue() {}
      */
     /** Version 2.19.0 */
     public static final Version VERSION_2_19_0 = Version.fromString("2.19.0");
+
+    /*
+     * Constants associated with resource-sharing
+     */
+    public static final String WORKFLOW_STATE_RESOURCE_TYPE = "workflow_state";
+    public static final String WORKFLOW_RESOURCE_TYPE = "workflow";
 }

src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java

@@ -410,9 +410,16 @@ public void initializeConfigIndex(String tenantId, ActionListener<Boolean> liste
      * @param workflowId the workflowId, corresponds to document ID
      * @param tenantId the tenant id
      * @param user passes the user that created the workflow
+     * @param allSharedPrincipals the entities this workflow is shared with
      * @param listener action listener
      */
-    public void putInitialStateToWorkflowState(String workflowId, String tenantId, User user, ActionListener<IndexResponse> listener) {
+    public void putInitialStateToWorkflowState(
+        String workflowId,
+        String tenantId,
+        User user,
+        List<String> allSharedPrincipals,
+        ActionListener<IndexResponse> listener
+    ) {
         WorkflowState state = WorkflowState.builder()
             .workflowId(workflowId)
             .state(State.NOT_STARTED.name())
@@ -421,6 +428,7 @@ public void putInitialStateToWorkflowState(String workflowId, String tenantId, U
             .resourcesCreated(Collections.emptyList())
             .userOutputs(Collections.emptyMap())
             .tenantId(tenantId)
+            .allSharedPrincipals(allSharedPrincipals)
             .build();
         initWorkflowStateIndexIfAbsent(ActionListener.wrap(indexCreated -> {
             if (!indexCreated) {