From b0718e4565ab3ecf533797aff0679cee131a2adb Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Mon, 13 Oct 2025 13:41:24 -0400 Subject: [PATCH 01/23] Onboards flow-framework plugin to resource-sharing and access control framework Signed-off-by: Darshit Chanpura --- build.gradle | 4 ++ .../flowframework/FlowFrameworkPlugin.java | 8 ++- ...FlowFrameworkResourceSharingExtension.java | 35 +++++++++++ .../flowframework/common/CommonValue.java | 6 ++ .../CreateWorkflowTransportAction.java | 20 ++++--- .../DeleteWorkflowTransportAction.java | 31 ++++++---- .../DeprovisionWorkflowTransportAction.java | 31 ++++++---- .../GetWorkflowStateTransportAction.java | 31 ++++++---- .../transport/GetWorkflowTransportAction.java | 30 ++++++---- .../flowframework/transport/PluginClient.java | 60 +++++++++++++++++++ .../ProvisionWorkflowTransportAction.java | 30 ++++++---- .../ReprovisionWorkflowTransportAction.java | 30 ++++++---- .../SearchWorkflowStateTransportAction.java | 3 +- .../SearchWorkflowTransportAction.java | 3 +- .../transport/handler/SearchHandler.java | 18 ++++-- .../flowframework/util/ParseUtils.java | 26 +++++++- .../util/ResourceSharingClientAccessor.java | 44 ++++++++++++++ ...ity.spi.resources.ResourceSharingExtension | 1 + src/main/resources/resource-action-groups.yml | 35 +++++++++++ ...archWorkflowStateTransportActionTests.java | 13 +++- .../SearchWorkflowTransportActionTests.java | 13 +++- .../transport/handler/SearchHandlerTests.java | 7 ++- 22 files changed, 379 insertions(+), 100 deletions(-) create mode 100644 src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java create mode 100644 src/main/java/org/opensearch/flowframework/transport/PluginClient.java create mode 100644 src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java create mode 100644 src/main/resources/META-INF/services/org.opensearch.security.spi.resources.ResourceSharingExtension create mode 100644 src/main/resources/resource-action-groups.yml diff --git a/build.gradle b/build.gradle index e5374b66c..77c1f6daa 100644 --- a/build.gradle +++ b/build.gradle @@ -95,6 +95,7 @@ opensearchplugin { classname = "${projectPath}.${pathToPlugin}.${pluginClassName}" licenseFile = rootProject.file('LICENSE') noticeFile = rootProject.file('NOTICE') + extendedPlugins = ['opensearch-security;optional=true'] } dependencyLicenses.enabled = false @@ -193,6 +194,9 @@ configurations { } dependencies { + // For resource access control + compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${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 diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java index 7b8ff8236..2b1b60273 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java @@ -47,6 +47,7 @@ import org.opensearch.flowframework.transport.GetWorkflowStepAction; import org.opensearch.flowframework.transport.GetWorkflowStepTransportAction; import org.opensearch.flowframework.transport.GetWorkflowTransportAction; +import org.opensearch.flowframework.transport.PluginClient; import org.opensearch.flowframework.transport.ProvisionWorkflowAction; import org.opensearch.flowframework.transport.ProvisionWorkflowTransportAction; import org.opensearch.flowframework.transport.ReprovisionWorkflowAction; @@ -120,6 +121,8 @@ public class FlowFrameworkPlugin extends Plugin implements ActionPlugin, SystemI private FlowFrameworkSettings flowFrameworkSettings; + private PluginClient pluginClient; + /** * Instantiate this plugin. */ @@ -143,8 +146,11 @@ public Collection 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 diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java new file mode 100644 index 000000000..7083e7b70 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java @@ -0,0 +1,35 @@ +/* + * 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; + +public class FlowFrameworkResourceSharingExtension implements ResourceSharingExtension { + @Override + public Set getResourceProviders() { + return Set.of( + new ResourceProvider(CommonValue.WORKFLOW_RESOURCE_TYPE, GLOBAL_CONTEXT_INDEX), + new ResourceProvider(CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, WORKFLOW_STATE_INDEX) + ); + } + + @Override + public void assignResourceSharingClient(ResourceSharingClient resourceSharingClient) { + ResourceSharingClientAccessor.getInstance().setResourceSharingClient(resourceSharingClient); + } +} diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java index e7d3929c6..9968163ba 100644 --- a/src/main/java/org/opensearch/flowframework/common/CommonValue.java +++ b/src/main/java/org/opensearch/flowframework/common/CommonValue.java @@ -264,4 +264,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"; } diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java index e2ae03f22..21762d168 100644 --- a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java @@ -25,6 +25,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -63,6 +64,7 @@ import static org.opensearch.flowframework.util.ParseUtils.checkFilterByBackendRoles; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.getWorkflow; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport Action to index or update a use case template within the Global Context @@ -131,13 +133,17 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener createExecute(request, user, tenantId, listener) + verifyResourceAccessAndProcessRequest( + CommonValue.WORKFLOW_RESOURCE_TYPE, + () -> createExecute(request, user, tenantId, listener), + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> createExecute(request, user, tenantId, listener) + ) ); } catch (Exception e) { logger.error("Failed to create workflow", e); diff --git a/src/main/java/org/opensearch/flowframework/transport/DeleteWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/DeleteWorkflowTransportAction.java index 59f3a4aa5..a4b0db57b 100644 --- a/src/main/java/org/opensearch/flowframework/transport/DeleteWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/DeleteWorkflowTransportAction.java @@ -23,6 +23,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -39,6 +40,7 @@ import static org.opensearch.flowframework.common.FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport action to retrieve a use case template within the Global Context @@ -104,21 +106,24 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener executeDeleteRequest(request, tenantId, listener, context), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + clearStatus, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeDeleteRequest(request, tenantId, listener, context), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); - } else { String errorMessage = "There are no templates in the global context"; logger.error(errorMessage); diff --git a/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java index 97525a2a4..7b6e97643 100644 --- a/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java @@ -25,6 +25,7 @@ import org.opensearch.core.common.Strings; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -64,6 +65,7 @@ import static org.opensearch.flowframework.common.WorkflowResources.getResourceByWorkflowStep; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport Action to deprovision a workflow from a stored use case template @@ -142,21 +144,24 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener executeDeprovisionRequest(request, tenantId, listener, context, user), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + true, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeDeprovisionRequest(request, tenantId, listener, context, user), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); - } catch (Exception e) { String errorMessage = "Failed to retrieve template from global context."; logger.error(errorMessage, e); diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java index 36c843f19..59c8257df 100644 --- a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java @@ -21,6 +21,7 @@ import org.opensearch.commons.authuser.User; import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -33,6 +34,7 @@ import static org.opensearch.flowframework.common.FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; //TODO: Currently we only get the workflow status but we should change to be able to get the // full template as well @@ -98,21 +100,24 @@ protected void doExecute(Task task, GetWorkflowStateRequest request, ActionListe try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { - resolveUserAndExecute( - user, - workflowId, - tenantId, - filterByEnabled, - true, - flowFrameworkSettings.isMultiTenancyEnabled(), - listener, + verifyResourceAccessAndProcessRequest( + CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, () -> executeGetWorkflowStateRequest(request, tenantId, listener, context), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + true, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeGetWorkflowStateRequest(request, tenantId, listener, context), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); - } catch (Exception e) { String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to get workflow: {}", workflowId) .getFormattedMessage(); diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java index 8b86499d3..d3bd4608b 100644 --- a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java @@ -22,6 +22,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -38,6 +39,7 @@ import static org.opensearch.flowframework.common.FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport action to retrieve a use case template within the Global Context @@ -106,19 +108,23 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener executeGetRequest(request, tenantId, listener, context), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + false, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeGetRequest(request, tenantId, listener, context), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); } catch (Exception e) { String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage( diff --git a/src/main/java/org/opensearch/flowframework/transport/PluginClient.java b/src/main/java/org/opensearch/flowframework/transport/PluginClient.java new file mode 100644 index 000000000..288ae8009 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/transport/PluginClient.java @@ -0,0 +1,60 @@ +/* + * 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.transport; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionType; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.identity.Subject; +import org.opensearch.transport.client.Client; +import org.opensearch.transport.client.FilterClient; + +/** + * A special client for executing transport actions as this plugin's system subject. + */ +public class PluginClient extends FilterClient { + + private static final Logger logger = LogManager.getLogger(PluginClient.class); + + private Subject subject; + + public PluginClient(Client delegate) { + super(delegate); + } + + public PluginClient(Client delegate, Subject subject) { + super(delegate); + this.subject = subject; + } + + public void setSubject(Subject subject) { + this.subject = subject; + } + + @Override + protected void doExecute( + ActionType action, + Request request, + ActionListener listener + ) { + if (subject == null) { + throw new IllegalStateException("PluginClient is not initialized."); + } + try (ThreadContext.StoredContext ctx = threadPool().getThreadContext().newStoredContext(false)) { + subject.runAs(() -> { + logger.info("Running transport action with subject: {}", subject.getPrincipal().getName()); + super.doExecute(action, request, ActionListener.runBefore(listener, ctx::restore)); + }); + } + } +} diff --git a/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java index ae8431917..df69e6561 100644 --- a/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java @@ -24,6 +24,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -63,6 +64,7 @@ import static org.opensearch.flowframework.common.FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport Action to provision a workflow from a stored use case template @@ -143,19 +145,23 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener executeProvisionRequest(request, tenantId, listener, context), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + false, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeProvisionRequest(request, tenantId, listener, context), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); } catch (Exception e) { String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage( diff --git a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowTransportAction.java index e37100838..ebcef373f 100644 --- a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowTransportAction.java @@ -24,6 +24,7 @@ import org.opensearch.core.action.ActionListener; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; @@ -67,6 +68,7 @@ import static org.opensearch.flowframework.common.FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES; import static org.opensearch.flowframework.util.ParseUtils.getUserContext; import static org.opensearch.flowframework.util.ParseUtils.resolveUserAndExecute; +import static org.opensearch.flowframework.util.ParseUtils.verifyResourceAccessAndProcessRequest; /** * Transport Action to reprovision a provisioned template @@ -150,19 +152,23 @@ protected void doExecute(Task task, ReprovisionWorkflowRequest request, ActionLi User user = getUserContext(client); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { - resolveUserAndExecute( - user, - workflowId, - tenantId, - filterByEnabled, - false, - flowFrameworkSettings.isMultiTenancyEnabled(), - listener, + verifyResourceAccessAndProcessRequest( + CommonValue.WORKFLOW_RESOURCE_TYPE, () -> executeReprovisionRequest(request, tenantId, listener, context), - client, - sdkClient, - clusterService, - xContentRegistry + () -> resolveUserAndExecute( + user, + workflowId, + tenantId, + filterByEnabled, + false, + flowFrameworkSettings.isMultiTenancyEnabled(), + listener, + () -> executeReprovisionRequest(request, tenantId, listener, context), + client, + sdkClient, + clusterService, + xContentRegistry + ) ); } catch (Exception e) { String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage( diff --git a/src/main/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportAction.java index 138c55ea2..1272ddf24 100644 --- a/src/main/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportAction.java @@ -17,6 +17,7 @@ import org.opensearch.action.support.HandledTransportAction; import org.opensearch.common.inject.Inject; import org.opensearch.core.action.ActionListener; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.transport.handler.SearchHandler; import org.opensearch.tasks.Task; @@ -52,7 +53,7 @@ protected void doExecute(Task task, SearchRequest request, ActionListener actionListener) { + public void search(SearchRequest request, String tenantId, String resourceType, ActionListener actionListener) { // AccessController should take care of letting the user with right permission to view the workflow User user = ParseUtils.getUserContext(client); try (ThreadContext.StoredContext context = client.threadPool().getThreadContext().stashContext()) { logger.info("Searching workflows in global context"); SearchSourceBuilder searchSourceBuilder = request.source(); searchSourceBuilder.fetchSource(getSourceContext(user, searchSourceBuilder)); - validateRole(request, tenantId, user, actionListener, context); + validateRole(request, tenantId, user, resourceType, actionListener, context); } catch (Exception e) { logger.error("Failed to search workflows in global context", e); actionListener.onFailure(e); @@ -94,10 +97,15 @@ public void validateRole( SearchRequest request, String tenantId, User user, + String resourceType, ActionListener listener, ThreadContext.StoredContext context ) { - if (user == null || !filterByBackendRole || isAdmin(user)) { + if (shouldUseResourceAuthz(resourceType)) { + // When resource-sharing feature is enabled, + // Search is handled DLS style in security plugin + doSearch(request, tenantId, ActionListener.runBefore(listener, context::restore)); + } else if (user == null || !filterByBackendRole || isAdmin(user)) { // Case 1: user == null when 1. Security is disabled. 2. When user is super-admin // Case 2: If Security is enabled and filter is disabled, proceed with search as // user is already authenticated to hit this API. diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java index d85eb5447..e1cf54ed6 100644 --- a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java +++ b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java @@ -32,6 +32,7 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.exception.FlowFrameworkException; import org.opensearch.flowframework.indices.FlowFrameworkIndicesHandler; import org.opensearch.flowframework.model.Template; @@ -251,6 +252,26 @@ public static User getUserContext(Client client) { return User.parse(userStr); } + public static boolean shouldUseResourceAuthz(String resourceType) { + var client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); + return client != null && client.isFeatureEnabledForType(resourceType); + } + + /** + * Verifies whether the user has permission to access the resource. + * @param resourceType the type of resource to be authorized + * @param onSuccess consumer function to execute if resource sharing feature is enabled + * @param fallbackOn501 consumer function to execute if resource sharing feature is disabled. + */ + public static void verifyResourceAccessAndProcessRequest(String resourceType, Runnable onSuccess, Runnable fallbackOn501) { + // Resource access will be auto-evaluated + if (shouldUseResourceAuthz(resourceType)) { + onSuccess.run(); + } else { + fallbackOn501.run(); + } + } + /** * Add user backend roles filter to search source builder= * @param user the user @@ -505,7 +526,10 @@ public static void onGetWorkflowResponse( return; } } - if (!filterByEnabled || checkUserPermissions(requestUser, resourceUser, workflowId) || isAdmin(requestUser)) { + if (shouldUseResourceAuthz(CommonValue.WORKFLOW_RESOURCE_TYPE) + || !filterByEnabled + || checkUserPermissions(requestUser, resourceUser, workflowId) + || isAdmin(requestUser)) { function.run(); } else { logger.debug("User: " + requestUser.getName() + " does not have permissions to access workflow: " + workflowId); diff --git a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java new file mode 100644 index 000000000..62856ed18 --- /dev/null +++ b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java @@ -0,0 +1,44 @@ +/* + * 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.util; + +import org.opensearch.security.spi.resources.client.ResourceSharingClient; + +/** + * Accessor for resource sharing client + */ +public class ResourceSharingClientAccessor { + private ResourceSharingClient CLIENT; + + private static ResourceSharingClientAccessor resourceSharingClientAccessor; + + private ResourceSharingClientAccessor() {} + + public static ResourceSharingClientAccessor getInstance() { + if (resourceSharingClientAccessor == null) { + resourceSharingClientAccessor = new ResourceSharingClientAccessor(); + } + + return resourceSharingClientAccessor; + } + + /** + * Set the resource sharing client + */ + public void setResourceSharingClient(ResourceSharingClient client) { + resourceSharingClientAccessor.CLIENT = client; + } + + /** + * Get the resource sharing client + */ + public ResourceSharingClient getResourceSharingClient() { + return resourceSharingClientAccessor.CLIENT; + } +} diff --git a/src/main/resources/META-INF/services/org.opensearch.security.spi.resources.ResourceSharingExtension b/src/main/resources/META-INF/services/org.opensearch.security.spi.resources.ResourceSharingExtension new file mode 100644 index 000000000..eadaac9f2 --- /dev/null +++ b/src/main/resources/META-INF/services/org.opensearch.security.spi.resources.ResourceSharingExtension @@ -0,0 +1 @@ +org.opensearch.flowframework.FlowFrameworkResourceSharingExtension diff --git a/src/main/resources/resource-action-groups.yml b/src/main/resources/resource-action-groups.yml new file mode 100644 index 000000000..bf1bbd302 --- /dev/null +++ b/src/main/resources/resource-action-groups.yml @@ -0,0 +1,35 @@ +# For resource-access-management +resource_types: + workflow: + workflow_read_only: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow/get" + - "cluster:admin/opensearch/flow_framework/workflow/search" + + workflow_read_write: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow/*" + - "cluster:monitor/*" + + workflow_full_access: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow/*" + - "cluster:monitor/*" + - "cluster:admin/security/resource/share" + + workflow_state: + workflow_state_read_only: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow_state/get" + - "cluster:admin/opensearch/flow_framework/workflow_state/search" + + workflow_state_read_write: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow_state/*" + - "cluster:monitor/*" + + workflow_state_full_access: + allowed_actions: + - "cluster:admin/opensearch/flow_framework/workflow_state/*" + - "cluster:monitor/*" + - "cluster:admin/security/resource/share" diff --git a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java index fdb585245..f8f686212 100644 --- a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java @@ -14,6 +14,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.transport.handler.SearchHandler; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.tasks.Task; @@ -67,10 +68,11 @@ public void testSearchWorkflow() { SearchRequest request = invocation.getArgument(0); ActionListener responseListener = invocation.getArgument(1); ThreadContext.StoredContext storedContext = mock(ThreadContext.StoredContext.class); - searchHandler.validateRole(request, null, null, responseListener, storedContext); + searchHandler.validateRole(request, null, null, null, responseListener, storedContext); responseListener.onResponse(mock(SearchResponse.class)); return null; - }).when(searchHandler).search(any(SearchRequest.class), nullable(String.class), any(ActionListener.class)); + }).when(searchHandler) + .search(any(SearchRequest.class), nullable(String.class), CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, any(ActionListener.class)); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -79,7 +81,12 @@ public void testSearchWorkflow() { }).when(client).search(any(SearchRequest.class), any(ActionListener.class)); searchWorkflowStateTransportAction.doExecute(mock(Task.class), searchRequest, listener); - verify(searchHandler).search(any(SearchRequest.class), nullable(String.class), any(ActionListener.class)); + verify(searchHandler).search( + any(SearchRequest.class), + nullable(String.class), + CommonValue.WORKFLOW_RESOURCE_TYPE, + any(ActionListener.class) + ); } } diff --git a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java index cc4ea9691..71c4083c5 100644 --- a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java @@ -14,6 +14,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.core.action.ActionListener; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.transport.handler.SearchHandler; import org.opensearch.search.builder.SearchSourceBuilder; import org.opensearch.tasks.Task; @@ -71,10 +72,11 @@ public void testSearchWorkflow() { SearchRequest request = invocation.getArgument(0); ActionListener responseListener = invocation.getArgument(1); ThreadContext.StoredContext storedContext = mock(ThreadContext.StoredContext.class); - searchHandler.validateRole(request, null, null, responseListener, storedContext); + searchHandler.validateRole(request, null, null, null, responseListener, storedContext); responseListener.onResponse(mock(SearchResponse.class)); return null; - }).when(searchHandler).search(any(SearchRequest.class), nullable(String.class), any(ActionListener.class)); + }).when(searchHandler) + .search(any(SearchRequest.class), nullable(String.class), CommonValue.WORKFLOW_RESOURCE_TYPE, any(ActionListener.class)); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -83,7 +85,12 @@ public void testSearchWorkflow() { }).when(client).search(any(SearchRequest.class), any(ActionListener.class)); searchWorkflowTransportAction.doExecute(mock(Task.class), searchRequest, listener); - verify(searchHandler).search(any(SearchRequest.class), nullable(String.class), any(ActionListener.class)); + verify(searchHandler).search( + any(SearchRequest.class), + nullable(String.class), + CommonValue.WORKFLOW_RESOURCE_TYPE, + any(ActionListener.class) + ); } } diff --git a/src/test/java/org/opensearch/flowframework/transport/handler/SearchHandlerTests.java b/src/test/java/org/opensearch/flowframework/transport/handler/SearchHandlerTests.java index c1bc58206..83f207aac 100644 --- a/src/test/java/org/opensearch/flowframework/transport/handler/SearchHandlerTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/handler/SearchHandlerTests.java @@ -17,6 +17,7 @@ import org.opensearch.commons.ConfigConstants; import org.opensearch.core.action.ActionListener; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.common.FlowFrameworkSettings; import org.opensearch.remote.metadata.client.SdkClient; import org.opensearch.remote.metadata.client.impl.SdkClientFactory; @@ -75,7 +76,7 @@ public void setUp() throws Exception { public void testSearchException() { doThrow(new RuntimeException("test")).when(client).search(any(), any()); - searchHandler.search(request, null, listener); + searchHandler.search(request, null, CommonValue.WORKFLOW_RESOURCE_TYPE, listener); verify(listener, times(1)).onFailure(any()); } @@ -86,7 +87,7 @@ public void testFilterEnabledWithWrongSearch() { searchHandler = new SearchHandler(settings, clusterService, client, sdkClient, FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES); - searchHandler.search(request, null, listener); + searchHandler.search(request, null, CommonValue.WORKFLOW_RESOURCE_TYPE, listener); verify(listener, times(1)).onFailure(any()); } @@ -97,7 +98,7 @@ public void testFilterEnabled() { searchHandler = new SearchHandler(settings, clusterService, client, sdkClient, FlowFrameworkSettings.FILTER_BY_BACKEND_ROLES); - searchHandler.search(matchAllRequest(), null, listener); + searchHandler.search(matchAllRequest(), null, CommonValue.WORKFLOW_RESOURCE_TYPE, listener); verify(client, times(1)).search(any(), any()); } From cc508bb75e29c07c3d87e665ee99f468a5c6ec10 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Mon, 13 Oct 2025 13:58:02 -0400 Subject: [PATCH 02/23] Adds javadoc and a changelog entry Signed-off-by: Darshit Chanpura --- CHANGELOG.md | 2 ++ .../java/org/opensearch/flowframework/util/ParseUtils.java | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fdc4ce815..645ecb7a1 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java index e1cf54ed6..2a4318f3c 100644 --- a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java +++ b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java @@ -252,6 +252,11 @@ public static User getUserContext(Client client) { return User.parse(userStr); } + /** + * Checks whether resource authz framework should be used. + * @param resourceType the type to check for resource authz + * @return true if resource-authz should be used, false otherwise + */ public static boolean shouldUseResourceAuthz(String resourceType) { var client = ResourceSharingClientAccessor.getInstance().getResourceSharingClient(); return client != null && client.isFeatureEnabledForType(resourceType); From 7bc42598e4fa65103493f105f89b2a6119abd8c0 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 14 Oct 2025 11:53:23 -0400 Subject: [PATCH 03/23] Adds tests for resource sharing flow and adds a CI job to run resource-sharing tests Signed-off-by: Darshit Chanpura # Conflicts: # .github/workflows/test_security.yml --- .github/workflows/test_security.yml | 7 +- build.gradle | 16 +- .../FlowFrameworkRestTestCase.java | 205 ++++++++- ...FlowFrameworkResourceSharingRestApiIT.java | 413 ++++++++++++++++++ .../rest/FlowFrameworkSecureRestApiIT.java | 40 -- 5 files changed, 635 insertions(+), 46 deletions(-) create mode 100644 src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index c358b11f9..48c46784b 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -20,6 +20,7 @@ jobs: strategy: matrix: java: [21, 25] + resource_sharing_flag: [ "", "-Dresource_sharing.enabled=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 \ + ${{ matrix.resource_sharing_flag }} \ + --tests '*IT'" diff --git a/build.gradle b/build.gradle index 77c1f6daa..bd845b90b 100644 --- a/build.gradle +++ b/build.gradle @@ -256,6 +256,8 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}") testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}") + testImplementation 'org.awaitility:awaitility:4.3.0' + // ZipArchive dependencies used for integration tests zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}") zipArchive("org.opensearch.plugin:opensearch-ml-plugin:${opensearch_build}") @@ -358,6 +360,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 { @@ -517,6 +521,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) } @@ -549,7 +557,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 } } diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 4dbb88efe..7696562aa 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -9,7 +9,6 @@ package org.opensearch.flowframework; import com.google.gson.JsonArray; -import org.apache.commons.lang3.RandomStringUtils; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.UsernamePasswordCredentials; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; @@ -26,6 +25,7 @@ import org.apache.hc.core5.reactor.ssl.TlsDetails; import org.apache.hc.core5.ssl.SSLContextBuilder; import org.apache.hc.core5.util.Timeout; +import org.opensearch.OpenSearchStatusException; import org.opensearch.action.ingest.GetPipelineResponse; import org.opensearch.action.search.SearchResponse; import org.opensearch.client.Request; @@ -37,11 +37,14 @@ import org.opensearch.common.unit.TimeValue; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.LoggingDeprecationHandler; +import org.opensearch.common.xcontent.XContentFactory; import org.opensearch.common.xcontent.json.JsonXContent; import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.NamedXContentRegistry; +import org.opensearch.core.xcontent.ToXContent; +import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.model.ProvisioningProgress; @@ -51,13 +54,17 @@ import org.opensearch.flowframework.model.WorkflowState; import org.opensearch.flowframework.util.ParseUtils; import org.opensearch.ml.repackage.com.google.common.collect.ImmutableList; +import org.opensearch.security.spi.resources.sharing.Recipients; import org.opensearch.test.rest.OpenSearchRestTestCase; import org.junit.After; import org.junit.Before; import java.io.IOException; +import java.security.SecureRandom; +import java.time.Duration; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; import java.util.Map; @@ -65,6 +72,8 @@ import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +import org.awaitility.Awaitility; + import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_URI; @@ -73,6 +82,8 @@ */ public abstract class FlowFrameworkRestTestCase extends OpenSearchRestTestCase { + public static final String SHARE_WORKFLOW_URI = "/_plugins/_security/api/resource/share"; + @Before protected void setUpSettings() throws Exception { @@ -296,11 +307,42 @@ protected boolean preserveClusterSettings() { } /** - * Create an unique password. Simple password are weak due to https://tinyurl.com/383em9zk + * Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk * @return a random password. */ public static String generatePassword(String username) { - return RandomStringUtils.random(15, true, true); + String upperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; + String lowerCase = "abcdefghijklmnopqrstuvwxyz"; + String digits = "0123456789"; + String special = "_"; + String characters = upperCase + lowerCase + digits + special; + + SecureRandom rng = new SecureRandom(); + + // Ensure password includes at least one character from each set + char[] password = new char[15]; + password[0] = upperCase.charAt(rng.nextInt(upperCase.length())); + password[1] = lowerCase.charAt(rng.nextInt(lowerCase.length())); + password[2] = digits.charAt(rng.nextInt(digits.length())); + password[3] = special.charAt(rng.nextInt(special.length())); + + for (int i = 4; i < 15; i++) { + char nextChar; + do { + nextChar = characters.charAt(rng.nextInt(characters.length())); + } while (username.indexOf(nextChar) > -1); + password[i] = nextChar; + } + + // Shuffle the array to ensure the first 4 characters are not always in the same position + for (int i = password.length - 1; i > 0; i--) { + int index = rng.nextInt(i + 1); + char temp = password[index]; + password[index] = password[i]; + password[i] = temp; + } + + return new String(password); } /** @@ -485,7 +527,7 @@ protected Response createWorkflowValidation(RestClient client, Template template * Helper method to invoke the Reprovision Workflow API * @param client the rest client * @param workflowId the document id - * @param templateFields the template to reprovision + * @param template the template to reprovision * @throws Exception if the request fails * @return a rest response */ @@ -985,4 +1027,159 @@ protected List catPlugins() throws IOException { ).list(); return pluginsList.stream().map(o -> ((Map) o).get("component").toString()).collect(Collectors.toList()); } + + protected boolean isResourceSharingFeatureEnabled() { + return Optional.ofNullable(System.getProperty("resource_sharing.enabled")).map("true"::equalsIgnoreCase).orElse(false); + } + + public static Response shareConfig(RestClient client, Map params, String payload) throws IOException { + return TestHelpers.makeRequest(client, "PUT", SHARE_WORKFLOW_URI, params, payload, null); + } + + public static Response patchSharingInfo(RestClient client, Map params, String payload) throws IOException { + return TestHelpers.makeRequest(client, "PATCH", SHARE_WORKFLOW_URI, params, payload, null); + } + + public static String shareWithUserPayload(String resourceId, String resourceIndex, String accessLevel, String user) { + return String.format(Locale.ROOT, """ + { + "resource_id": "%s", + "resource_type": "%s", + "share_with": { + "%s" : { + "users": ["%s"] + } + } + } + """, resourceId, resourceIndex, accessLevel, user); + } + + public static class PatchSharingInfoPayloadBuilder { + private String configId; + private String configType; + private final Map share = new HashMap<>(); + private final Map revoke = new HashMap<>(); + + public PatchSharingInfoPayloadBuilder configId(String resourceId) { + this.configId = resourceId; + return this; + } + + public PatchSharingInfoPayloadBuilder configType(String resourceType) { + this.configType = resourceType; + return this; + } + + public void share(Recipients recipients, String accessLevel) { + Recipients existing = share.getOrDefault(accessLevel, new Recipients(new HashMap<>())); + existing.share(recipients); + share.put(accessLevel, existing); + } + + public void revoke(Recipients recipients, String accessLevel) { + Recipients existing = revoke.getOrDefault(accessLevel, new Recipients(new HashMap<>())); + // intentionally share() is called here since we are building a shareWith object, this final object will be used to remove + // access + // think of it as currentShareWith.removeAll(revokeShareWith) + existing.share(recipients); + revoke.put(accessLevel, existing); + } + + private String buildJsonString(Map input) { + + List output = new ArrayList<>(); + for (Map.Entry entry : input.entrySet()) { + try { + XContentBuilder builder = XContentFactory.jsonBuilder(); + entry.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS); + String recipJson = builder.toString(); + output.add(String.format(Locale.ROOT, "\"%s\" : %s", entry.getKey(), recipJson)); + } catch (IOException e) { + throw new RuntimeException(e); + } + + } + + return String.join(",", output); + + } + + public String build() { + String allShares = buildJsonString(share); + String allRevokes = buildJsonString(revoke); + return String.format(Locale.ROOT, """ + { + "resource_id": "%s", + "resource_type": "%s", + "add": { + %s + }, + "revoke": { + %s + } + } + """, configId, configType, allShares, allRevokes); + } + } + + public static boolean isForbidden(Exception e) { + if (e instanceof OpenSearchStatusException) { + return ((OpenSearchStatusException) e).status() == RestStatus.FORBIDDEN; + } + if (e instanceof ResponseException) { + return ((ResponseException) e).getResponse().getStatusLine().getStatusCode() == 403; + } + return false; + } + + private static final Duration RS_WAIT_TIMEOUT = Duration.ofSeconds(30); + private static final Duration RS_POLL_INTERVAL = Duration.ofMillis(200); + + // Core waiter: visible when the callable returns 200 OK; 403 means "not yet", anything else fails fast. + private void waitUntilVisible(java.util.concurrent.Callable op) { + Awaitility.await().atMost(RS_WAIT_TIMEOUT).pollInterval(RS_POLL_INTERVAL).until(() -> { + try { + Response r = op.call(); + return TestHelpers.restStatus(r) == RestStatus.OK; + } catch (Exception e) { + if (isForbidden(e)) { + // eventual consistency: not visible yet + return false; + } + // unexpected error: fail fast + throw e; + } + }); + } + + // Core waiter: non-visible when the callable throws 403; 200 means "still visible", anything else fails fast. + private void waitUntilForbidden(java.util.concurrent.Callable op) { + Awaitility.await().atMost(RS_WAIT_TIMEOUT).pollInterval(RS_POLL_INTERVAL).until(() -> { + try { + op.call(); // 200 => still visible + return false; + } catch (Exception e) { + if (isForbidden(e)) { + return true; // forbidden now + } + throw e; // unexpected error: fail fast + } + }); + } + + protected void waitForWorkflowSharingVisibility(String workflowId, RestClient client) { + waitUntilVisible(() -> getWorkflow(client, workflowId)); + } + + protected void waitForWorkflowRevokeNonVisibility(String workflowId, RestClient client) { + waitUntilForbidden(() -> getWorkflow(client, workflowId)); + } + + protected void waitForWorkflowStateSharingVisibility(String workflowId, RestClient client) { + waitUntilVisible(() -> getWorkflowStatus(client, workflowId, false)); + } + + protected void waitForWorkflowStateRevokeNonVisibility(String workflowId, RestClient client) { + waitUntilForbidden(() -> getWorkflowStatus(client, workflowId, false)); + } } diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java new file mode 100644 index 000000000..3c58384a8 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java @@ -0,0 +1,413 @@ +/* + * 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.rest; + +import org.apache.hc.core5.http.HttpHost; +import org.opensearch.action.search.SearchResponse; +import org.opensearch.client.Response; +import org.opensearch.client.ResponseException; +import org.opensearch.client.RestClient; +import org.opensearch.commons.rest.SecureRestClientBuilder; +import org.opensearch.core.rest.RestStatus; +import org.opensearch.flowframework.FlowFrameworkRestTestCase; +import org.opensearch.flowframework.TestHelpers; +import org.opensearch.flowframework.model.Template; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.util.List; +import java.util.Map; + +import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; +import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_RESOURCE_TYPE; +import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_STATE_RESOURCE_TYPE; + +public class FlowFrameworkResourceSharingRestApiIT extends FlowFrameworkRestTestCase { + + String aliceUser = "alice"; + RestClient aliceClient; + String bobUser = "bob"; + RestClient bobClient; + String catUser = "cat"; + RestClient catClient; + String dogUser = "dog"; + RestClient dogClient; + String elkUser = "elk"; + RestClient elkClient; + String fishUser = "fish"; + RestClient fishClient; + String lionUser = "lion"; + RestClient lionClient; + private String indexAllAccessRole = "index_all_access"; + private static String FLOW_FRAMEWORK_FULL_ACCESS_ROLE = "flow_framework_full_access"; + private static String ML_COMMONS_FULL_ACCESS_ROLE = "ml_full_access"; + private static String FLOW_FRAMEWORK_READ_ACCESS_ROLE = "flow_framework_read_access"; + + // Sharing action-group names for Flow Framework (mirror AD style) + private static final String WORKFLOW_READ_ONLY_AG = "workflow_read_only"; + private static final String WORKFLOW_FULL_ACCESS_AG = "workflow_full_access"; + private static final String WORKFLOW_STATE_READ_ONLY_AG = "workflow_state_read_only"; + private static final String WORKFLOW_STATE_FULL_ACCESS_AG = "workflow_state_full_access"; + + @Before + public void setupSecureTests() throws IOException { + if (!isHttps()) throw new IllegalArgumentException("Secure Tests are running but HTTPS is not set"); + + // If the suite is launched without the flag, just skip these tests cleanly. + assumeTrue("RS tests only run when resource_sharing.enabled=true", isResourceSharingFeatureEnabled()); + + createIndexRole(indexAllAccessRole, "*"); + String alicePassword = generatePassword(aliceUser); + createUser(aliceUser, alicePassword, List.of("odfe")); + aliceClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), aliceUser, alicePassword) + .setSocketTimeout(60000) + .build(); + + String bobPassword = generatePassword(bobUser); + createUser(bobUser, bobPassword, List.of("odfe")); + bobClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), bobUser, bobPassword) + .setSocketTimeout(60000) + .build(); + + String catPassword = generatePassword(catUser); + createUser(catUser, catPassword, List.of("aes")); + catClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), catUser, catPassword) + .setSocketTimeout(60000) + .build(); + + String dogPassword = generatePassword(dogUser); + createUser(dogUser, dogPassword, List.of()); + dogClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), dogUser, dogPassword) + .setSocketTimeout(60000) + .build(); + + String elkPassword = generatePassword(elkUser); + createUser(elkUser, elkPassword, List.of("odfe")); + elkClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), elkUser, elkPassword) + .setSocketTimeout(60000) + .build(); + + String fishPassword = generatePassword(fishUser); + createUser(fishUser, fishPassword, List.of("odfe", "aes")); + fishClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), fishUser, fishPassword) + .setSocketTimeout(60000) + .build(); + + String lionPassword = generatePassword(lionUser); + createUser(lionUser, lionPassword, List.of("opensearch")); + lionClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), lionUser, lionPassword) + .setSocketTimeout(60000) + .build(); + + createRoleMapping(FLOW_FRAMEWORK_READ_ACCESS_ROLE, List.of(bobUser)); + createRoleMapping(ML_COMMONS_FULL_ACCESS_ROLE, List.of(aliceUser, catUser, dogUser, elkUser, fishUser)); + createRoleMapping(FLOW_FRAMEWORK_FULL_ACCESS_ROLE, List.of(aliceUser, catUser, dogUser, elkUser, fishUser)); + createRoleMapping(indexAllAccessRole, List.of(aliceUser)); + } + + @After + public void tearDownSecureTests() throws IOException { + aliceClient.close(); + bobClient.close(); + catClient.close(); + dogClient.close(); + elkClient.close(); + fishClient.close(); + lionClient.close(); + deleteUser(aliceUser); + deleteUser(bobUser); + deleteUser(catUser); + deleteUser(dogUser); + deleteUser(elkUser); + deleteUser(fishUser); + deleteUser(lionUser); + } + + public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + Response created = createWorkflow(aliceClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + + // Unshared → cat/admin cannot GET + ResponseException ex = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + ex = expectThrows(ResponseException.class, () -> getWorkflow(client(), workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // Owner search sees ≥1; others see 0 + String matchAll = "{\"query\":{\"bool\":{\"must\":[{\"match_all\":{}}]}}}"; + SearchResponse ownerSr = searchWorkflows(aliceClient, matchAll); + assertEquals(RestStatus.OK, ownerSr.status()); + assertTrue(ownerSr.getHits().getTotalHits().value() >= 1); + + SearchResponse catSr = searchWorkflows(catClient, matchAll); + assertEquals(0, catSr.getHits().getTotalHits().value()); + SearchResponse adminSr = searchWorkflows(client(), matchAll); + assertEquals(0, adminSr.getHits().getTotalHits().value()); + + // Share READ_ONLY with cat → cat can GET & search=1 + Response shareRO = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_READ_ONLY_AG, catUser) + ); + assertEquals(200, shareRO.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, catClient); + + Response catGet = getWorkflow(catClient, workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(catGet)); + catSr = searchWorkflows(catClient, matchAll); + assertEquals(1, catSr.getHits().getTotalHits().value()); + + // Admin still unshared → 0 + adminSr = searchWorkflows(client(), matchAll); + assertEquals(0, adminSr.getHits().getTotalHits().value()); + + // Grant FULL_ACCESS to elk → elk can share to backend role "aes" as READ_ONLY + Response grantFullToElk = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_FULL_ACCESS_AG, elkUser) + ); + assertEquals(200, grantFullToElk.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, elkClient); + + var recsBR = new java.util.HashMap>(); + recsBR.put(org.opensearch.security.spi.resources.sharing.Recipient.BACKEND_ROLES, java.util.Set.of("aes")); + var recipientsBR = new org.opensearch.security.spi.resources.sharing.Recipients(recsBR); + + PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); + patch.share(recipientsBR, WORKFLOW_READ_ONLY_AG); + + Response elkAddsAes = patchSharingInfo(elkClient, Map.of(), patch.build()); + assertEquals(200, elkAddsAes.getStatusLine().getStatusCode()); + + // Revoke user-level cat; cat still sees via backend role "aes" + var recsUser = new java.util.HashMap>(); + recsUser.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(catUser)); + var recipientsUser = new org.opensearch.security.spi.resources.sharing.Recipients(recsUser); + + patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); + patch.revoke(recipientsUser, WORKFLOW_READ_ONLY_AG); + + Response elkRevokesUserCat = patchSharingInfo(elkClient, Map.of(), patch.build()); + assertEquals(200, elkRevokesUserCat.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, catClient); // still visible via backend role + + // Now revoke backend role "aes" → cat loses access + var recsRevokeBR = new java.util.HashMap>(); + recsRevokeBR.put(org.opensearch.security.spi.resources.sharing.Recipient.BACKEND_ROLES, java.util.Set.of("aes")); + var recipientsRevokeBR = new org.opensearch.security.spi.resources.sharing.Recipients(recsRevokeBR); + + patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); + patch.revoke(recipientsRevokeBR, WORKFLOW_READ_ONLY_AG); + + Response elkRevokesAes = patchSharingInfo(elkClient, Map.of(), patch.build()); + assertEquals(200, elkRevokesAes.getStatusLine().getStatusCode()); + waitForWorkflowRevokeNonVisibility(workflowId, catClient); + } + + public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + Response created = createWorkflow(aliceClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + + // Unshared → admin/fish/cat cannot update + ResponseException ex = expectThrows(ResponseException.class, () -> updateWorkflow(client(), workflowId, template)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + ex = expectThrows(ResponseException.class, () -> updateWorkflow(fishClient, workflowId, template)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // READ_ONLY to cat → still cannot update + Response shareRO = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_READ_ONLY_AG, catUser) + ); + assertEquals(200, shareRO.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, catClient); + + ex = expectThrows(ResponseException.class, () -> updateWorkflow(catClient, workflowId, template)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // FULL_ACCESS to elk → elk can update + Response shareFull = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_FULL_ACCESS_AG, elkUser) + ); + assertEquals(200, shareFull.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, elkClient); + + Response elkUpdate = updateWorkflow(elkClient, workflowId, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(elkUpdate)); + + // Revoke elk FULL_ACCESS → elk loses update ability + var revokeUsers = new java.util.HashMap>(); + revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(elkUser)); + var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); + + PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); + patch.revoke(recipientsUsers, WORKFLOW_FULL_ACCESS_AG); + + Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); + assertEquals(200, revoked.getStatusLine().getStatusCode()); + waitForWorkflowRevokeNonVisibility(workflowId, elkClient); + + ResponseException elkDenied = expectThrows(ResponseException.class, () -> updateWorkflow(elkClient, workflowId, template)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), elkDenied.getResponse().getStatusLine().getStatusCode()); + } + + public void testProvisionDeprovision_withResourceSharingEnabled() throws Exception { + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + Response created = createWorkflow(aliceClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + + // Unshared → cat cannot provision/deprovision + ResponseException ex = expectThrows(ResponseException.class, () -> provisionWorkflow(catClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + ex = expectThrows(ResponseException.class, () -> deprovisionWorkflow(catClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // READ_ONLY to cat → still cannot provision/deprovision + Response shareRO = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_READ_ONLY_AG, catUser) + ); + assertEquals(200, shareRO.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, catClient); + + ex = expectThrows(ResponseException.class, () -> provisionWorkflow(catClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // FULL_ACCESS to elk → elk can provision & deprovision + Response shareFull = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_FULL_ACCESS_AG, elkUser) + ); + assertEquals(200, shareFull.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, elkClient); + + Response prov = provisionWorkflow(elkClient, workflowId); + assertEquals(RestStatus.ACCEPTED, TestHelpers.restStatus(prov)); + + Response deprov = deprovisionWorkflow(elkClient, workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(deprov)); + + // Revoke elk FULL_ACCESS → elk loses ability + var revokeUsers = new java.util.HashMap>(); + revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(elkUser)); + var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); + + PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); + patch.revoke(recipientsUsers, WORKFLOW_FULL_ACCESS_AG); + + Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); + assertEquals(200, revoked.getStatusLine().getStatusCode()); + waitForWorkflowRevokeNonVisibility(workflowId, elkClient); + + ResponseException elkDenied = expectThrows(ResponseException.class, () -> provisionWorkflow(elkClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), elkDenied.getResponse().getStatusLine().getStatusCode()); + } + + public void testDeleteWorkflow_withResourceSharingEnabled() throws Exception { + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + Response created = createWorkflow(aliceClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + + // Unshared → cat/admin cannot delete + ResponseException ex = expectThrows(ResponseException.class, () -> deleteWorkflow(catClient, workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + ex = expectThrows(ResponseException.class, () -> deleteWorkflow(client(), workflowId)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + // FULL_ACCESS → can delete + Response grantFull = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_RESOURCE_TYPE, WORKFLOW_FULL_ACCESS_AG, catUser) + ); + assertEquals(200, grantFull.getStatusLine().getStatusCode()); + waitForWorkflowSharingVisibility(workflowId, catClient); + + Response del = deleteWorkflow(catClient, workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(del)); + } + + public void testWorkflowStateVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + // Create a workflow (state doc gets created/managed by FF) + Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); + Response created = createWorkflow(aliceClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + + // Unshared state → cat/admin cannot read status or find it + ResponseException ex = expectThrows(ResponseException.class, () -> getWorkflowStatus(catClient, workflowId, false)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + ex = expectThrows(ResponseException.class, () -> getWorkflowStatus(client(), workflowId, false)); + assertEquals(RestStatus.FORBIDDEN.getStatus(), ex.getResponse().getStatusLine().getStatusCode()); + + String matchAllState = "{\"query\":{\"bool\":{\"must\":[{\"match_all\":{}}]}}}"; + SearchResponse ownerSr = searchWorkflowState(aliceClient, matchAllState); + assertEquals(RestStatus.OK, ownerSr.status()); + // others see 0 + SearchResponse catSr = searchWorkflowState(catClient, matchAllState); + assertEquals(0, catSr.getHits().getTotalHits().value()); + SearchResponse adminSr = searchWorkflowState(client(), matchAllState); + assertEquals(0, adminSr.getHits().getTotalHits().value()); + + // Share workflow_state READ_ONLY to cat → cat can read status & search sees 1 + Response shareStateRO = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_STATE_RESOURCE_TYPE, WORKFLOW_STATE_READ_ONLY_AG, catUser) + ); + assertEquals(200, shareStateRO.getStatusLine().getStatusCode()); + // status endpoint visibility check (reuse the same wait helper by hitting GET workflow status in the lambda) + waitForWorkflowStateSharingVisibility(workflowId, catClient); + + Response catStatus = getWorkflowStatus(catClient, workflowId, false); + assertEquals(RestStatus.OK, TestHelpers.restStatus(catStatus)); + catSr = searchWorkflowState(catClient, matchAllState); + assertTrue(catSr.getHits().getTotalHits().value() >= 1); + + // Grant workflow_state FULL_ACCESS to elk → elk can also read/modify state if any write-state APIs exist + Response shareStateFull = shareConfig( + aliceClient, + Map.of(), + shareWithUserPayload(workflowId, WORKFLOW_STATE_RESOURCE_TYPE, WORKFLOW_STATE_FULL_ACCESS_AG, elkUser) + ); + assertEquals(200, shareStateFull.getStatusLine().getStatusCode()); + // sanity check visibility + Response elkStatus = getWorkflowStatus(elkClient, workflowId, false); + assertEquals(RestStatus.OK, TestHelpers.restStatus(elkStatus)); + + // Revoke cat (READ_ONLY on state) → cat loses status visibility + var revokeUsers = new java.util.HashMap>(); + revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(catUser)); + var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); + + PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId) + .configType(WORKFLOW_STATE_RESOURCE_TYPE); + patch.revoke(recipientsUsers, WORKFLOW_STATE_READ_ONLY_AG); + + Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); + assertEquals(200, revoked.getStatusLine().getStatusCode()); + + // wait until cat can no longer read status + waitForWorkflowStateRevokeNonVisibility(workflowId, catClient); + } +} diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index a6cae1653..f084f60b7 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -28,7 +28,6 @@ import org.junit.Before; import java.io.IOException; -import java.security.SecureRandom; import java.util.Collections; import java.util.List; import java.util.Map; @@ -129,45 +128,6 @@ public void tearDownSecureTests() throws IOException { deleteUser(lionUser); } - /** - * Create an unguessable password. Simple password are weak due to https://tinyurl.com/383em9zk - * @return a random password. - */ - public static String generatePassword(String username) { - String upperCase = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"; - String lowerCase = "abcdefghijklmnopqrstuvwxyz"; - String digits = "0123456789"; - String special = "_"; - String characters = upperCase + lowerCase + digits + special; - - SecureRandom rng = new SecureRandom(); - - // Ensure password includes at least one character from each set - char[] password = new char[15]; - password[0] = upperCase.charAt(rng.nextInt(upperCase.length())); - password[1] = lowerCase.charAt(rng.nextInt(lowerCase.length())); - password[2] = digits.charAt(rng.nextInt(digits.length())); - password[3] = special.charAt(rng.nextInt(special.length())); - - for (int i = 4; i < 15; i++) { - char nextChar; - do { - nextChar = characters.charAt(rng.nextInt(characters.length())); - } while (username.indexOf(nextChar) > -1); - password[i] = nextChar; - } - - // Shuffle the array to ensure the first 4 characters are not always in the same position - for (int i = password.length - 1; i > 0; i--) { - int index = rng.nextInt(i + 1); - char temp = password[index]; - password[index] = password[i]; - password[i] = temp; - } - - return new String(password); - } - public void testCreateWorkflowWithReadAccess() throws Exception { Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); ResponseException exception = expectThrows(ResponseException.class, () -> createWorkflow(bobClient, template)); From 78ca46bbaf1ff3d45d9490041550b14532d3f836 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 14 Oct 2025 18:58:13 -0400 Subject: [PATCH 04/23] Explicitly set security spi version to 3.4 Signed-off-by: Darshit Chanpura --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index bd845b90b..debd49b44 100644 --- a/build.gradle +++ b/build.gradle @@ -195,7 +195,7 @@ configurations { dependencies { // For resource access control - compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${opensearch_build}" + compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"3.4.0-SNAPSHOT" implementation("org.opensearch:opensearch:${opensearch_version}") api("org.opensearch:opensearch-ml-client:${opensearch_build}") From a438d125689b446f0ea98bb263c6c8a11c038f4b Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Thu, 16 Oct 2025 14:40:24 -0400 Subject: [PATCH 05/23] Refactors test to use in house Recipient class Signed-off-by: Darshit Chanpura --- .../FlowFrameworkRestTestCase.java | 108 +++++++++++++----- ...FlowFrameworkResourceSharingRestApiIT.java | 50 ++++---- 2 files changed, 100 insertions(+), 58 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java index 7696562aa..554b95945 100644 --- a/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkRestTestCase.java @@ -43,7 +43,6 @@ import org.opensearch.core.xcontent.DeprecationHandler; import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.NamedXContentRegistry; -import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; import org.opensearch.flowframework.common.CommonValue; @@ -54,7 +53,6 @@ import org.opensearch.flowframework.model.WorkflowState; import org.opensearch.flowframework.util.ParseUtils; import org.opensearch.ml.repackage.com.google.common.collect.ImmutableList; -import org.opensearch.security.spi.resources.sharing.Recipients; import org.opensearch.test.rest.OpenSearchRestTestCase; import org.junit.After; import org.junit.Before; @@ -65,10 +63,12 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -1054,11 +1054,29 @@ public static String shareWithUserPayload(String resourceId, String resourceInde """, resourceId, resourceIndex, accessLevel, user); } + public enum Recipient { + USERS("users"), + ROLES("roles"), + BACKEND_ROLES("backend_roles"); + + private final String name; + + Recipient(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } + public static class PatchSharingInfoPayloadBuilder { private String configId; private String configType; - private final Map share = new HashMap<>(); - private final Map revoke = new HashMap<>(); + + // accessLevel -> recipientType -> principals + private final Map>> share = new HashMap<>(); + private final Map>> revoke = new HashMap<>(); public PatchSharingInfoPayloadBuilder configId(String resourceId) { this.configId = resourceId; @@ -1070,38 +1088,27 @@ public PatchSharingInfoPayloadBuilder configType(String resourceType) { return this; } - public void share(Recipients recipients, String accessLevel) { - Recipients existing = share.getOrDefault(accessLevel, new Recipients(new HashMap<>())); - existing.share(recipients); - share.put(accessLevel, existing); + public void share(Map> recipients, String accessLevel) { + mergeInto(share, accessLevel, recipients); } - public void revoke(Recipients recipients, String accessLevel) { - Recipients existing = revoke.getOrDefault(accessLevel, new Recipients(new HashMap<>())); - // intentionally share() is called here since we are building a shareWith object, this final object will be used to remove - // access - // think of it as currentShareWith.removeAll(revokeShareWith) - existing.share(recipients); - revoke.put(accessLevel, existing); + public void revoke(Map> recipients, String accessLevel) { + mergeInto(revoke, accessLevel, recipients); } - private String buildJsonString(Map input) { + /* -------------------------------- Build -------------------------------- */ - List output = new ArrayList<>(); - for (Map.Entry entry : input.entrySet()) { + private String buildJsonString(Map>> input) { + List pieces = new ArrayList<>(); + for (Map.Entry>> e : input.entrySet()) { try { - XContentBuilder builder = XContentFactory.jsonBuilder(); - entry.getValue().toXContent(builder, ToXContent.EMPTY_PARAMS); - String recipJson = builder.toString(); - output.add(String.format(Locale.ROOT, "\"%s\" : %s", entry.getKey(), recipJson)); - } catch (IOException e) { - throw new RuntimeException(e); + String recipientsJson = toRecipientsJson(e.getValue()); + pieces.add(String.format(Locale.ROOT, "\"%s\" : %s", e.getKey(), recipientsJson)); + } catch (IOException ex) { + throw new RuntimeException(ex); } - } - - return String.join(",", output); - + return String.join(",", pieces); } public String build() { @@ -1120,6 +1127,51 @@ public String build() { } """, configId, configType, allShares, allRevokes); } + + /* ------------------------------ Internals ------------------------------ */ + + private static void mergeInto( + Map>> target, + String accessLevel, + Map> incoming + ) { + if (incoming == null || incoming.isEmpty()) return; + Map> existing = target.computeIfAbsent(accessLevel, k -> new HashMap<>()); + for (Map.Entry> e : incoming.entrySet()) { + if (e.getKey() == null) continue; + if (e.getValue() == null || e.getValue().isEmpty()) continue; + existing.computeIfAbsent(e.getKey().getName(), k -> new HashSet<>()).addAll(e.getValue()); + } + } + + private static String toRecipientsJson(Map> recipients) throws IOException { + if (recipients == null) { + recipients = Collections.emptyMap(); + } + + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + + for (Recipient recipient : Recipient.values()) { + String key = recipient.getName(); + if (recipients.containsKey(key)) { + writeArray(builder, key, recipients.get(key)); + } + } + + builder.endObject(); + return builder.toString(); + } + + private static void writeArray(XContentBuilder builder, String field, Set values) throws IOException { + builder.startArray(field); + if (values != null) { + for (String v : values) { + builder.value(v); + } + } + builder.endArray(); + } } public static boolean isForbidden(Exception e) { diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java index 3c58384a8..3ffb96e3c 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java @@ -22,8 +22,10 @@ import org.junit.Before; import java.io.IOException; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_ID; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_RESOURCE_TYPE; @@ -180,35 +182,29 @@ public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws assertEquals(200, grantFullToElk.getStatusLine().getStatusCode()); waitForWorkflowSharingVisibility(workflowId, elkClient); - var recsBR = new java.util.HashMap>(); - recsBR.put(org.opensearch.security.spi.resources.sharing.Recipient.BACKEND_ROLES, java.util.Set.of("aes")); - var recipientsBR = new org.opensearch.security.spi.resources.sharing.Recipients(recsBR); - + var recs = new HashMap>(); + recs.put(Recipient.BACKEND_ROLES, Set.of("aes")); PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); - patch.share(recipientsBR, WORKFLOW_READ_ONLY_AG); + patch.share(recs, WORKFLOW_READ_ONLY_AG); Response elkAddsAes = patchSharingInfo(elkClient, Map.of(), patch.build()); assertEquals(200, elkAddsAes.getStatusLine().getStatusCode()); // Revoke user-level cat; cat still sees via backend role "aes" - var recsUser = new java.util.HashMap>(); - recsUser.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(catUser)); - var recipientsUser = new org.opensearch.security.spi.resources.sharing.Recipients(recsUser); - + recs = new HashMap<>(); + recs.put(Recipient.USERS, Set.of(catUser)); patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); - patch.revoke(recipientsUser, WORKFLOW_READ_ONLY_AG); + patch.revoke(recs, WORKFLOW_READ_ONLY_AG); Response elkRevokesUserCat = patchSharingInfo(elkClient, Map.of(), patch.build()); assertEquals(200, elkRevokesUserCat.getStatusLine().getStatusCode()); waitForWorkflowSharingVisibility(workflowId, catClient); // still visible via backend role // Now revoke backend role "aes" → cat loses access - var recsRevokeBR = new java.util.HashMap>(); - recsRevokeBR.put(org.opensearch.security.spi.resources.sharing.Recipient.BACKEND_ROLES, java.util.Set.of("aes")); - var recipientsRevokeBR = new org.opensearch.security.spi.resources.sharing.Recipients(recsRevokeBR); - + recs = new HashMap<>(); + recs.put(Recipient.BACKEND_ROLES, Set.of("aes")); patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); - patch.revoke(recipientsRevokeBR, WORKFLOW_READ_ONLY_AG); + patch.revoke(recs, WORKFLOW_READ_ONLY_AG); Response elkRevokesAes = patchSharingInfo(elkClient, Map.of(), patch.build()); assertEquals(200, elkRevokesAes.getStatusLine().getStatusCode()); @@ -252,12 +248,10 @@ public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { assertEquals(RestStatus.CREATED, TestHelpers.restStatus(elkUpdate)); // Revoke elk FULL_ACCESS → elk loses update ability - var revokeUsers = new java.util.HashMap>(); - revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(elkUser)); - var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); - + var recs = new HashMap>(); + recs.put(Recipient.USERS, Set.of(elkUser)); PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); - patch.revoke(recipientsUsers, WORKFLOW_FULL_ACCESS_AG); + patch.revoke(recs, WORKFLOW_FULL_ACCESS_AG); Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); assertEquals(200, revoked.getStatusLine().getStatusCode()); @@ -307,12 +301,10 @@ public void testProvisionDeprovision_withResourceSharingEnabled() throws Excepti assertEquals(RestStatus.OK, TestHelpers.restStatus(deprov)); // Revoke elk FULL_ACCESS → elk loses ability - var revokeUsers = new java.util.HashMap>(); - revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(elkUser)); - var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); - + var recs = new HashMap>(); + recs.put(Recipient.USERS, Set.of(elkUser)); PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId).configType(WORKFLOW_RESOURCE_TYPE); - patch.revoke(recipientsUsers, WORKFLOW_FULL_ACCESS_AG); + patch.revoke(recs, WORKFLOW_FULL_ACCESS_AG); Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); assertEquals(200, revoked.getStatusLine().getStatusCode()); @@ -396,13 +388,11 @@ public void testWorkflowStateVisibilityAndSearch_withResourceSharingEnabled() th assertEquals(RestStatus.OK, TestHelpers.restStatus(elkStatus)); // Revoke cat (READ_ONLY on state) → cat loses status visibility - var revokeUsers = new java.util.HashMap>(); - revokeUsers.put(org.opensearch.security.spi.resources.sharing.Recipient.USERS, java.util.Set.of(catUser)); - var recipientsUsers = new org.opensearch.security.spi.resources.sharing.Recipients(revokeUsers); - + var recs = new HashMap>(); + recs.put(Recipient.USERS, Set.of(catUser)); PatchSharingInfoPayloadBuilder patch = new PatchSharingInfoPayloadBuilder().configId(workflowId) .configType(WORKFLOW_STATE_RESOURCE_TYPE); - patch.revoke(recipientsUsers, WORKFLOW_STATE_READ_ONLY_AG); + patch.revoke(recs, WORKFLOW_STATE_READ_ONLY_AG); Response revoked = patchSharingInfo(aliceClient, Map.of(), patch.build()); assertEquals(200, revoked.getStatusLine().getStatusCode()); From 33feaa2663c097bb6122882c2be2c069cb1d0702 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Fri, 24 Oct 2025 16:19:52 -0400 Subject: [PATCH 06/23] Address PR comments around renaming and version Signed-off-by: Darshit Chanpura --- build.gradle | 2 +- .../flowframework/FlowFrameworkPlugin.java | 13 +- .../flowframework/util/ParseUtils.java | 6 +- .../{transport => util}/PluginClient.java | 11 +- .../util/ResourceSharingClientAccessor.java | 8 +- .../flowframework/util/PluginClientTests.java | 186 ++++++++++++++++++ 6 files changed, 209 insertions(+), 17 deletions(-) rename src/main/java/org/opensearch/flowframework/{transport => util}/PluginClient.java (88%) create mode 100644 src/test/java/org/opensearch/flowframework/util/PluginClientTests.java diff --git a/build.gradle b/build.gradle index debd49b44..bd845b90b 100644 --- a/build.gradle +++ b/build.gradle @@ -195,7 +195,7 @@ configurations { dependencies { // For resource access control - compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"3.4.0-SNAPSHOT" + compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${opensearch_build}" implementation("org.opensearch:opensearch:${opensearch_version}") api("org.opensearch:opensearch-ml-client:${opensearch_build}") diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java index 2b1b60273..4746b557a 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkPlugin.java @@ -47,7 +47,6 @@ import org.opensearch.flowframework.transport.GetWorkflowStepAction; import org.opensearch.flowframework.transport.GetWorkflowStepTransportAction; import org.opensearch.flowframework.transport.GetWorkflowTransportAction; -import org.opensearch.flowframework.transport.PluginClient; import org.opensearch.flowframework.transport.ProvisionWorkflowAction; import org.opensearch.flowframework.transport.ProvisionWorkflowTransportAction; import org.opensearch.flowframework.transport.ReprovisionWorkflowAction; @@ -58,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; @@ -117,7 +119,7 @@ /** * 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; @@ -302,4 +304,11 @@ public Collection getSystemIndexDescriptors(Settings sett ); } + @Override + public void assignSubject(PluginSubject pluginSubject) { + if (this.pluginClient != null) { + this.pluginClient.setSubject(pluginSubject); + } + } + } diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java index 2a4318f3c..df1c4b887 100644 --- a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java +++ b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java @@ -266,14 +266,14 @@ public static boolean shouldUseResourceAuthz(String resourceType) { * Verifies whether the user has permission to access the resource. * @param resourceType the type of resource to be authorized * @param onSuccess consumer function to execute if resource sharing feature is enabled - * @param fallbackOn501 consumer function to execute if resource sharing feature is disabled. + * @param fallBackIfDisabled consumer function to execute if resource sharing feature is disabled. */ - public static void verifyResourceAccessAndProcessRequest(String resourceType, Runnable onSuccess, Runnable fallbackOn501) { + public static void verifyResourceAccessAndProcessRequest(String resourceType, Runnable onSuccess, Runnable fallBackIfDisabled) { // Resource access will be auto-evaluated if (shouldUseResourceAuthz(resourceType)) { onSuccess.run(); } else { - fallbackOn501.run(); + fallBackIfDisabled.run(); } } diff --git a/src/main/java/org/opensearch/flowframework/transport/PluginClient.java b/src/main/java/org/opensearch/flowframework/util/PluginClient.java similarity index 88% rename from src/main/java/org/opensearch/flowframework/transport/PluginClient.java rename to src/main/java/org/opensearch/flowframework/util/PluginClient.java index 288ae8009..c397afe44 100644 --- a/src/main/java/org/opensearch/flowframework/transport/PluginClient.java +++ b/src/main/java/org/opensearch/flowframework/util/PluginClient.java @@ -6,7 +6,7 @@ * this file be licensed under the Apache-2.0 license or a * compatible open source license. */ -package org.opensearch.flowframework.transport; +package org.opensearch.flowframework.util; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -28,13 +28,8 @@ public class PluginClient extends FilterClient { private Subject subject; - public PluginClient(Client delegate) { - super(delegate); - } - - public PluginClient(Client delegate, Subject subject) { - super(delegate); - this.subject = subject; + public PluginClient(Client client) { + super(client); } public void setSubject(Subject subject) { diff --git a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java index 62856ed18..411a9363d 100644 --- a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java +++ b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java @@ -10,11 +10,13 @@ import org.opensearch.security.spi.resources.client.ResourceSharingClient; +import java.util.concurrent.atomic.AtomicReference; + /** * Accessor for resource sharing client */ public class ResourceSharingClientAccessor { - private ResourceSharingClient CLIENT; + private final AtomicReference client = new AtomicReference<>(); private static ResourceSharingClientAccessor resourceSharingClientAccessor; @@ -32,13 +34,13 @@ public static ResourceSharingClientAccessor getInstance() { * Set the resource sharing client */ public void setResourceSharingClient(ResourceSharingClient client) { - resourceSharingClientAccessor.CLIENT = client; + resourceSharingClientAccessor.client.set(client); } /** * Get the resource sharing client */ public ResourceSharingClient getResourceSharingClient() { - return resourceSharingClientAccessor.CLIENT; + return resourceSharingClientAccessor.client.get(); } } diff --git a/src/test/java/org/opensearch/flowframework/util/PluginClientTests.java b/src/test/java/org/opensearch/flowframework/util/PluginClientTests.java new file mode 100644 index 000000000..f3ce6c807 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/util/PluginClientTests.java @@ -0,0 +1,186 @@ +/* + * 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.util; + +import org.opensearch.action.ActionRequest; +import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.ActionType; +import org.opensearch.common.CheckedRunnable; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.core.action.ActionListener; +import org.opensearch.core.action.ActionResponse; +import org.opensearch.core.common.io.stream.StreamInput; +import org.opensearch.core.common.io.stream.StreamOutput; +import org.opensearch.identity.Subject; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.client.Client; +import org.junit.After; +import org.junit.Before; + +import java.io.IOException; +import java.security.Principal; + +import org.mockito.ArgumentCaptor; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +public class PluginClientTests extends OpenSearchTestCase { + + private Client delegate; + private ThreadContext threadContext; + private Subject subject; + private PluginClient pluginClient; + + private static class TestRequest extends ActionRequest { + @Override + public ActionRequestValidationException validate() { + return null; + } + } + + private static class TestResponse extends ActionResponse { + public TestResponse() {} + + public TestResponse(StreamInput in) throws IOException { + super(in); + } + + @Override + public void writeTo(StreamOutput out) { /* no-op */ } + } + + private static final ActionType TEST_ACTION = new ActionType<>("test:action", TestResponse::new); + + @Before + @Override + public void setUp() throws Exception { + super.setUp(); + threadContext = new ThreadContext(Settings.EMPTY); + + ThreadPool threadPool = mock(ThreadPool.class); + when(threadPool.getThreadContext()).thenReturn(threadContext); + + delegate = mock(Client.class, withSettings().defaultAnswer(CALLS_REAL_METHODS)); + when(delegate.threadPool()).thenReturn(threadPool); + + subject = mock(Subject.class); + + Principal principal = mock(Principal.class); + when(principal.getName()).thenReturn("test-subject"); + when(subject.getPrincipal()).thenReturn(principal); + + doAnswer(inv -> { + CheckedRunnable r = inv.getArgument(0); + r.run(); // may throw Exception; Mockito will surface it + return null; + }).when(subject).runAs(any()); + + pluginClient = new PluginClient(delegate); + pluginClient.setSubject(subject); + } + + @After + @Override + public void tearDown() throws Exception { + super.tearDown(); + } + + public void testThrowsIfSubjectIsNull() { + pluginClient = new PluginClient(delegate); // subject not set + IllegalStateException ex = expectThrows( + IllegalStateException.class, + () -> pluginClient.execute(TEST_ACTION, new TestRequest(), ActionListener.wrap(r -> {}, e -> {})) + ); + assertTrue(ex.getMessage().contains("PluginClient is not initialized.")); + } + + public void testDelegatesExecuteAndRunsAsSubject_AndCapturesResponse() { + TestRequest req = new TestRequest(); + TestResponse resp = new TestResponse(); + + // Delegate responds with resp + doAnswer(inv -> { + ActionListener l = inv.getArgument(2); + l.onResponse(resp); + return null; + }).when(delegate).execute(eq(TEST_ACTION), eq(req), any()); + + @SuppressWarnings("unchecked") + ActionListener userListener = mock(ActionListener.class); + + pluginClient.execute(TEST_ACTION, req, userListener); + + verify(delegate, times(1)).execute(eq(TEST_ACTION), eq(req), any()); + + // Verify runAs was used with a CheckedRunnable + verify(subject, times(1)).runAs(any()); + + // Capture and assert the response delivered to user's listener + ArgumentCaptor respCaptor = ArgumentCaptor.forClass(TestResponse.class); + verify(userListener, times(1)).onResponse(respCaptor.capture()); + assertSame(resp, respCaptor.getValue()); + + verify(userListener, never()).onFailure(any()); + } + + public void testThreadContextIsRestoredBeforeUserListenerRuns() { + threadContext.putHeader("foo", "orig"); + + // Inside delegate: add a *new* header and prove it disappears after restore + doAnswer(inv -> { + ActionListener l = inv.getArgument(2); + + threadContext.putHeader("bar", "changed"); + + l.onResponse(new TestResponse()); + return null; + }).when(delegate).execute(eq(TEST_ACTION), any(TestRequest.class), any()); + + ActionListener assertingListener = ActionListener.wrap(r -> { + // ctx::restore ran BEFORE this listener, so: + assertEquals("orig", threadContext.getHeader("foo")); // preserved + assertNull(threadContext.getHeader("bar")); // reverted (wasn't in original) + }, e -> fail("Should not fail")); + + pluginClient.execute(TEST_ACTION, new TestRequest(), assertingListener); + } + + public void testCheckedExceptionFromRunAsIsWrapped() { + // Make runAs throw a checked Exception + doAnswer(inv -> { throw new Exception("boom"); }).when(subject).runAs(any()); + + RuntimeException ex = expectThrows( + RuntimeException.class, + () -> pluginClient.execute(TEST_ACTION, new TestRequest(), ActionListener.wrap(r -> {}, e -> {})) + ); + assertTrue(ex.getMessage().contains("boom")); + } + + public void testRuntimeExceptionFromDelegateBubblesUp() { + doAnswer(inv -> { throw new IllegalStateException("delegate-broke"); }).when(delegate) + .execute(eq(TEST_ACTION), any(TestRequest.class), any()); + + IllegalStateException ex = expectThrows( + IllegalStateException.class, + () -> pluginClient.execute(TEST_ACTION, new TestRequest(), ActionListener.wrap(r -> {}, e -> {})) + ); + assertTrue(ex.getMessage().contains("delegate-broke")); + } +} From bcc871eafb2a26852ba6781288842c3277370f42 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Fri, 24 Oct 2025 16:22:38 -0400 Subject: [PATCH 07/23] Constant for awaitility version Signed-off-by: Darshit Chanpura --- build.gradle | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index bd845b90b..4bf3f4c82 100644 --- a/build.gradle +++ b/build.gradle @@ -48,6 +48,7 @@ buildscript { parssonVersion = "1.1.7" swaggerVersion = "2.1.35" swaggerCoreVersion = "2.2.40" + awaitilityVersion= "4.3.0" } repositories { @@ -256,7 +257,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}") testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}") - testImplementation 'org.awaitility:awaitility:4.3.0' + testImplementation "org.awaitility:awaitility:${awaitlityVersion}" // ZipArchive dependencies used for integration tests zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}") From cfbf830f2107885256e790ee2a5aff88b790909b Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Wed, 5 Nov 2025 14:50:04 -0800 Subject: [PATCH 08/23] Fix typo in awaitilityVersion Co-authored-by: Owais Kazi Signed-off-by: Daniel Widdis --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 4bf3f4c82..e9b4bfcdf 100644 --- a/build.gradle +++ b/build.gradle @@ -257,7 +257,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}") testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}") - testImplementation "org.awaitility:awaitility:${awaitlityVersion}" + testImplementation "org.awaitility:awaitility:${awaitilityVersion}" // ZipArchive dependencies used for integration tests zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}") From 68d581cb2afc54a6c03426404741fbeaf7aa7a75 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Nov 2025 11:45:40 -0800 Subject: [PATCH 09/23] Update ResourceProvider to use anonymous class Signed-off-by: Daniel Widdis --- ...FlowFrameworkResourceSharingExtension.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java index 7083e7b70..07d0a6a88 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java @@ -22,10 +22,27 @@ public class FlowFrameworkResourceSharingExtension implements ResourceSharingExtension { @Override public Set getResourceProviders() { - return Set.of( - new ResourceProvider(CommonValue.WORKFLOW_RESOURCE_TYPE, GLOBAL_CONTEXT_INDEX), - new ResourceProvider(CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, WORKFLOW_STATE_INDEX) - ); + 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 From 64c9f9f54e7f625e4c5803093b36ddc42a3ddaf4 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Nov 2025 12:03:50 -0800 Subject: [PATCH 10/23] Fix javadocs Signed-off-by: Daniel Widdis --- .../FlowFrameworkResourceSharingExtension.java | 3 +++ .../flowframework/transport/handler/SearchHandler.java | 1 + .../org/opensearch/flowframework/util/PluginClient.java | 4 ++++ .../flowframework/util/ResourceSharingClientAccessor.java | 6 ++++++ 4 files changed, 14 insertions(+) diff --git a/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java index 07d0a6a88..89c228caf 100644 --- a/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java +++ b/src/main/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtension.java @@ -19,6 +19,9 @@ 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 getResourceProviders() { diff --git a/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java b/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java index 03ae2ddaf..e7c73a73f 100644 --- a/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java +++ b/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java @@ -90,6 +90,7 @@ public void search(SearchRequest request, String tenantId, String resourceType, * @param request SearchRequest * @param tenantId the tenant id * @param user User + * @param resourceType the resource type * @param listener ActionListener * @param context ThreadContext */ diff --git a/src/main/java/org/opensearch/flowframework/util/PluginClient.java b/src/main/java/org/opensearch/flowframework/util/PluginClient.java index c397afe44..2c0631fed 100644 --- a/src/main/java/org/opensearch/flowframework/util/PluginClient.java +++ b/src/main/java/org/opensearch/flowframework/util/PluginClient.java @@ -28,6 +28,10 @@ public class PluginClient extends FilterClient { private Subject subject; + /** + * Constructor for PluginClient + * @param client the underlying client to wrap + */ public PluginClient(Client client) { super(client); } diff --git a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java index 411a9363d..9848319a1 100644 --- a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java +++ b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java @@ -22,6 +22,10 @@ public class ResourceSharingClientAccessor { private ResourceSharingClientAccessor() {} + /** + * Get the singleton instance of ResourceSharingClientAccessor + * @return the singleton instance + */ public static ResourceSharingClientAccessor getInstance() { if (resourceSharingClientAccessor == null) { resourceSharingClientAccessor = new ResourceSharingClientAccessor(); @@ -32,6 +36,7 @@ public static ResourceSharingClientAccessor getInstance() { /** * Set the resource sharing client + * @param client the resource sharing client to set */ public void setResourceSharingClient(ResourceSharingClient client) { resourceSharingClientAccessor.client.set(client); @@ -39,6 +44,7 @@ public void setResourceSharingClient(ResourceSharingClient client) { /** * Get the resource sharing client + * @return the resource sharing client */ public ResourceSharingClient getResourceSharingClient() { return resourceSharingClientAccessor.client.get(); From 933d0ddedcbd5a6ff0a7312ce78426b12fe84777 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Nov 2025 12:11:19 -0800 Subject: [PATCH 11/23] Wrap checked exception from PluginClient Signed-off-by: Daniel Widdis --- .../java/org/opensearch/flowframework/util/PluginClient.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/org/opensearch/flowframework/util/PluginClient.java b/src/main/java/org/opensearch/flowframework/util/PluginClient.java index 2c0631fed..3c977fe53 100644 --- a/src/main/java/org/opensearch/flowframework/util/PluginClient.java +++ b/src/main/java/org/opensearch/flowframework/util/PluginClient.java @@ -10,6 +10,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.ExceptionsHelper; import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionType; import org.opensearch.common.util.concurrent.ThreadContext; @@ -54,6 +55,8 @@ protected void logger.info("Running transport action with subject: {}", subject.getPrincipal().getName()); super.doExecute(action, request, ActionListener.runBefore(listener, ctx::restore)); }); + } catch (Exception e) { + throw ExceptionsHelper.convertToRuntime(e); } } } From db0c55c16047d326c1038c0bbc3ab060788b0fb3 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Fri, 7 Nov 2025 12:12:20 -0800 Subject: [PATCH 12/23] Fix tests with param index and eq rather than raw strings Signed-off-by: Daniel Widdis --- .../SearchWorkflowStateTransportActionTests.java | 12 +++++++++--- .../SearchWorkflowTransportActionTests.java | 7 ++++--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java index f8f686212..eee005338 100644 --- a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowStateTransportActionTests.java @@ -24,6 +24,7 @@ import org.opensearch.transport.client.Client; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -66,13 +67,18 @@ public void testSearchWorkflow() { doAnswer(invocation -> { SearchRequest request = invocation.getArgument(0); - ActionListener responseListener = invocation.getArgument(1); + ActionListener responseListener = invocation.getArgument(3); ThreadContext.StoredContext storedContext = mock(ThreadContext.StoredContext.class); searchHandler.validateRole(request, null, null, null, responseListener, storedContext); responseListener.onResponse(mock(SearchResponse.class)); return null; }).when(searchHandler) - .search(any(SearchRequest.class), nullable(String.class), CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, any(ActionListener.class)); + .search( + any(SearchRequest.class), + nullable(String.class), + eq(CommonValue.WORKFLOW_STATE_RESOURCE_TYPE), + any(ActionListener.class) + ); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -84,7 +90,7 @@ public void testSearchWorkflow() { verify(searchHandler).search( any(SearchRequest.class), nullable(String.class), - CommonValue.WORKFLOW_RESOURCE_TYPE, + eq(CommonValue.WORKFLOW_STATE_RESOURCE_TYPE), any(ActionListener.class) ); } diff --git a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java index 71c4083c5..277860ca0 100644 --- a/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/SearchWorkflowTransportActionTests.java @@ -24,6 +24,7 @@ import org.opensearch.transport.client.Client; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -70,13 +71,13 @@ public void testSearchWorkflow() { doAnswer(invocation -> { SearchRequest request = invocation.getArgument(0); - ActionListener responseListener = invocation.getArgument(1); + ActionListener responseListener = invocation.getArgument(3); ThreadContext.StoredContext storedContext = mock(ThreadContext.StoredContext.class); searchHandler.validateRole(request, null, null, null, responseListener, storedContext); responseListener.onResponse(mock(SearchResponse.class)); return null; }).when(searchHandler) - .search(any(SearchRequest.class), nullable(String.class), CommonValue.WORKFLOW_RESOURCE_TYPE, any(ActionListener.class)); + .search(any(SearchRequest.class), nullable(String.class), eq(CommonValue.WORKFLOW_RESOURCE_TYPE), any(ActionListener.class)); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -88,7 +89,7 @@ public void testSearchWorkflow() { verify(searchHandler).search( any(SearchRequest.class), nullable(String.class), - CommonValue.WORKFLOW_RESOURCE_TYPE, + eq(CommonValue.WORKFLOW_RESOURCE_TYPE), any(ActionListener.class) ); } From 60477548c90f824caf070193ffef55ddfe48f0ad Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 10 Nov 2025 13:33:56 -0800 Subject: [PATCH 13/23] Properly skip ResourceSharingApiIT Signed-off-by: Daniel Widdis --- build.gradle | 4 +-- ...FlowFrameworkResourceSharingRestApiIT.java | 32 +++++++++++++++++-- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/build.gradle b/build.gradle index e9b4bfcdf..cdc4a5cbe 100644 --- a/build.gradle +++ b/build.gradle @@ -196,7 +196,7 @@ configurations { dependencies { // For resource access control - compileOnly group: 'org.opensearch', name:'opensearch-security-spi', version:"${opensearch_build}" + compileOnly("org.opensearch:opensearch-security-spi:${opensearch_build}") implementation("org.opensearch:opensearch:${opensearch_version}") api("org.opensearch:opensearch-ml-client:${opensearch_build}") @@ -257,7 +257,7 @@ dependencies { testImplementation("org.junit.jupiter:junit-jupiter:${junitJupiterVersion}") testImplementation("com.fasterxml.jackson.datatype:jackson-datatype-jsr310:${versions.jackson_databind}") - testImplementation "org.awaitility:awaitility:${awaitilityVersion}" + testImplementation("org.awaitility:awaitility:${awaitilityVersion}") // ZipArchive dependencies used for integration tests zipArchive("org.opensearch.plugin:opensearch-job-scheduler:${opensearch_build}") diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java index 3ffb96e3c..b23be9176 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java @@ -58,12 +58,19 @@ public class FlowFrameworkResourceSharingRestApiIT extends FlowFrameworkRestTest private static final String WORKFLOW_STATE_READ_ONLY_AG = "workflow_state_read_only"; private static final String WORKFLOW_STATE_FULL_ACCESS_AG = "workflow_state_full_access"; + // If the suite is launched without the flag, just skip these tests cleanly. + private boolean skipTests = !isResourceSharingFeatureEnabled(); + @Before public void setupSecureTests() throws IOException { - if (!isHttps()) throw new IllegalArgumentException("Secure Tests are running but HTTPS is not set"); + if (skipTests) { + logger.info("Skipping FlowFrameworkResourceSharingRestApiIT tests - resource sharing not enabled"); + return; + } - // If the suite is launched without the flag, just skip these tests cleanly. - assumeTrue("RS tests only run when resource_sharing.enabled=true", isResourceSharingFeatureEnabled()); + if (!isHttps()) { + fail("Secure Tests are running but HTTPS is not set"); + } createIndexRole(indexAllAccessRole, "*"); String alicePassword = generatePassword(aliceUser); @@ -116,6 +123,9 @@ public void setupSecureTests() throws IOException { @After public void tearDownSecureTests() throws IOException { + if (skipTests) { + return; + } aliceClient.close(); bobClient.close(); catClient.close(); @@ -133,6 +143,10 @@ public void tearDownSecureTests() throws IOException { } public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + if (skipTests) { + logger.info("Skipping test - resource sharing not enabled"); + return; + } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); Response created = createWorkflow(aliceClient, template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); @@ -212,6 +226,9 @@ public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws } public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { + if (skipTests) { + return; + } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); Response created = createWorkflow(aliceClient, template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); @@ -262,6 +279,9 @@ public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { } public void testProvisionDeprovision_withResourceSharingEnabled() throws Exception { + if (skipTests) { + return; + } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); Response created = createWorkflow(aliceClient, template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); @@ -315,6 +335,9 @@ public void testProvisionDeprovision_withResourceSharingEnabled() throws Excepti } public void testDeleteWorkflow_withResourceSharingEnabled() throws Exception { + if (skipTests) { + return; + } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); Response created = createWorkflow(aliceClient, template); assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); @@ -340,6 +363,9 @@ public void testDeleteWorkflow_withResourceSharingEnabled() throws Exception { } public void testWorkflowStateVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + if (skipTests) { + return; + } // Create a workflow (state doc gets created/managed by FF) Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); Response created = createWorkflow(aliceClient, template); From b20a348ca09e578ab1b0234b19b9a28ba030d8d5 Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Mon, 10 Nov 2025 13:48:05 -0800 Subject: [PATCH 14/23] Add unit test for ResourceSharingExtension Signed-off-by: Daniel Widdis --- ...rameworkResourceSharingExtensionTests.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/test/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtensionTests.java diff --git a/src/test/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtensionTests.java b/src/test/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtensionTests.java new file mode 100644 index 000000000..8e7a2dee4 --- /dev/null +++ b/src/test/java/org/opensearch/flowframework/FlowFrameworkResourceSharingExtensionTests.java @@ -0,0 +1,47 @@ +/* + * 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.client.ResourceSharingClient; +import org.opensearch.test.OpenSearchTestCase; + +import java.util.Set; + +import static org.mockito.Mockito.mock; + +public class FlowFrameworkResourceSharingExtensionTests extends OpenSearchTestCase { + + public void testGetResourceProviders() { + FlowFrameworkResourceSharingExtension extension = new FlowFrameworkResourceSharingExtension(); + Set providers = extension.getResourceProviders(); + + assertEquals(2, providers.size()); + + for (ResourceProvider provider : providers) { + if (CommonValue.WORKFLOW_RESOURCE_TYPE.equals(provider.resourceType())) { + assertEquals(CommonValue.GLOBAL_CONTEXT_INDEX, provider.resourceIndexName()); + } else { + assertEquals(CommonValue.WORKFLOW_STATE_RESOURCE_TYPE, provider.resourceType()); + assertEquals(CommonValue.WORKFLOW_STATE_INDEX, provider.resourceIndexName()); + } + } + } + + public void testAssignResourceSharingClient() { + FlowFrameworkResourceSharingExtension extension = new FlowFrameworkResourceSharingExtension(); + ResourceSharingClient mockClient = mock(ResourceSharingClient.class); + + extension.assignResourceSharingClient(mockClient); + + assertEquals(mockClient, ResourceSharingClientAccessor.getInstance().getResourceSharingClient()); + } +} From 1a956e4c073842e2f793c582f2230f845b4af009 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 10:12:54 -0800 Subject: [PATCH 15/23] Updates template to include `all_shared_principals` and marks relevant action-requests as DocRequests Signed-off-by: Darshit Chanpura --- build.gradle | 6 +++ .../flowframework/common/CommonValue.java | 2 + .../flowframework/model/Template.java | 44 ++++++++++++++++++- .../flowframework/model/WorkflowState.java | 39 +++++++++++++++- .../CreateWorkflowTransportAction.java | 3 +- .../transport/GetWorkflowStateRequest.java | 13 +++++- .../transport/ReprovisionWorkflowRequest.java | 13 +++++- .../transport/WorkflowRequest.java | 13 +++++- .../resources/mappings/global-context.json | 3 ++ .../resources/mappings/workflow-state.json | 3 ++ .../FlowFrameworkIndicesHandlerTests.java | 12 +++-- .../flowframework/model/TemplateTests.java | 6 ++- ...FlowFrameworkResourceSharingRestApiIT.java | 2 +- .../rest/RestCreateWorkflowActionTests.java | 3 +- .../CreateWorkflowTransportActionTests.java | 18 +++++--- .../GetWorkflowStateTransportActionTests.java | 3 +- .../GetWorkflowTransportActionTests.java | 3 +- ...ProvisionWorkflowTransportActionTests.java | 3 +- .../ReprovisionWorkflowRequestTests.java | 6 ++- .../WorkflowRequestResponseTests.java | 6 ++- .../util/EncryptorUtilsTests.java | 3 +- .../util/WorkflowTimeoutUtilityTests.java | 3 +- .../workflow/WorkflowProcessSorterTests.java | 12 +++-- 23 files changed, 185 insertions(+), 34 deletions(-) diff --git a/build.gradle b/build.gradle index cdc4a5cbe..edd63178d 100644 --- a/build.gradle +++ b/build.gradle @@ -391,6 +391,7 @@ integTest { if (System.getProperty("https") == null || System.getProperty("https") == "false") { filter { excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkSecureRestApiIT" + excludeTestsMatching "org.opensearch.flowframework.rest.FlowFrameworkResourceSharingRestApiIT" } } @@ -398,6 +399,7 @@ integTest { 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" } @@ -592,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 { @@ -603,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" } } @@ -611,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" } } } diff --git a/src/main/java/org/opensearch/flowframework/common/CommonValue.java b/src/main/java/org/opensearch/flowframework/common/CommonValue.java index 9968163ba..e0502102f 100644 --- a/src/main/java/org/opensearch/flowframework/common/CommonValue.java +++ b/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 */ diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java index a0d544f54..f6ffe7394 100644 --- a/src/main/java/org/opensearch/flowframework/model/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -33,6 +33,7 @@ import java.util.Set; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.opensearch.flowframework.common.CommonValue.ALL_SHARED_PRINCIPALS_FIELD; import static org.opensearch.flowframework.common.CommonValue.CREATED_TIME; import static org.opensearch.flowframework.common.CommonValue.DESCRIPTION_FIELD; import static org.opensearch.flowframework.common.CommonValue.LAST_PROVISIONED_TIME_FIELD; @@ -77,6 +78,7 @@ public class Template implements ToXContentObject { private final Instant lastUpdatedTime; private final Instant lastProvisionedTime; private String tenantId; + private final List allSharedPrincipals; // stores sharing info /** * Instantiate the object representing a use case template @@ -106,7 +108,8 @@ public Template( Instant createdTime, Instant lastUpdatedTime, Instant lastProvisionedTime, - String tenantId + String tenantId, + List allSharedPrincipals ) { this.name = name; this.description = description; @@ -120,6 +123,7 @@ public Template( this.lastUpdatedTime = lastUpdatedTime; this.lastProvisionedTime = lastProvisionedTime; this.tenantId = tenantId; + this.allSharedPrincipals = allSharedPrincipals; } /** @@ -138,6 +142,7 @@ public static class Builder { private Instant lastUpdatedTime = null; private Instant lastProvisionedTime = null; private String tenantId = null; + private List allSharedPrincipals = Collections.emptyList(); /** * Empty Constructor for the Builder object @@ -167,6 +172,7 @@ private Builder(Template t) { this.lastUpdatedTime = t.lastUpdatedTime(); this.lastProvisionedTime = t.lastProvisionedTime(); this.tenantId = t.getTenantId(); + this.allSharedPrincipals = t.allSharedPrincipals(); } /** @@ -289,6 +295,16 @@ public Builder tenantId(String tenantId) { return this; } + /** + * Builder method for adding allSharedPrincipals + * @param allSharedPrincipals entities this workflow is shared with + * @return the Builder object + */ + public Builder allSharedPrincipals(List allSharedPrincipals) { + this.allSharedPrincipals = allSharedPrincipals; + return this; + } + /** * Allows building a template * @return Template Object containing all needed fields @@ -306,7 +322,8 @@ public Template build() { this.createdTime, this.lastUpdatedTime, this.lastProvisionedTime, - this.tenantId + this.tenantId, + this.allSharedPrincipals ); } } @@ -380,6 +397,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws xContentBuilder.field(TENANT_ID_FIELD, tenantId); } + if (allSharedPrincipals != null && !allSharedPrincipals.isEmpty()) { + xContentBuilder.startArray(ALL_SHARED_PRINCIPALS_FIELD); + for (String p : this.allSharedPrincipals) { + xContentBuilder.value(p); + } + xContentBuilder.endArray(); + } + return xContentBuilder.endObject(); } @@ -409,6 +434,10 @@ public static Template updateExistingTemplate(Template existingTemplate, Templat if (templateWithNewFields.getUiMetadata() != null) { builder.uiMetadata(templateWithNewFields.getUiMetadata()); } + if (templateWithNewFields.allSharedPrincipals() != null && !templateWithNewFields.allSharedPrincipals().isEmpty()) { + existingTemplate.allSharedPrincipals().addAll(templateWithNewFields.allSharedPrincipals()); + builder.allSharedPrincipals(existingTemplate.allSharedPrincipals()); + } return builder.build(); } @@ -444,6 +473,7 @@ public static Template parse(XContentParser parser, boolean fieldUpdate) throws Instant lastUpdatedTime = null; Instant lastProvisionedTime = null; String tenantId = null; + List allSharedPrincipals = new ArrayList<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -514,6 +544,12 @@ public static Template parse(XContentParser parser, boolean fieldUpdate) throws case TENANT_ID_FIELD: tenantId = parser.text(); break; + case ALL_SHARED_PRINCIPALS_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + allSharedPrincipals.add(parser.text()); + } + break; default: throw new FlowFrameworkException( "Unable to parse field [" + fieldName + "] in a template object.", @@ -715,6 +751,10 @@ public void setTenantId(String tenantId) { this.tenantId = tenantId; } + public List allSharedPrincipals() { + return allSharedPrincipals; + } + @Override public String toString() { return "Template [name=" diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java index 2c5a23080..b311ebd5d 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java @@ -33,6 +33,7 @@ import java.util.Map; import static org.opensearch.core.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.opensearch.flowframework.common.CommonValue.ALL_SHARED_PRINCIPALS_FIELD; import static org.opensearch.flowframework.common.CommonValue.ERROR_FIELD; import static org.opensearch.flowframework.common.CommonValue.PROVISIONING_PROGRESS_FIELD; import static org.opensearch.flowframework.common.CommonValue.PROVISION_END_TIME_FIELD; @@ -61,6 +62,7 @@ public class WorkflowState implements ToXContentObject, Writeable { private Map userOutputs; private List resourcesCreated; private String tenantId; + private List allSharedPrincipals; /** * Instantiate the object representing the workflow state @@ -75,6 +77,7 @@ public class WorkflowState implements ToXContentObject, Writeable { * @param userOutputs A map of essential API responses for backend to use and lookup. * @param resourcesCreated A list of all the resources created. * @param tenantId The tenant id + * @param allSharedPrincipals the entities this workflow is shared with */ public WorkflowState( String workflowId, @@ -86,7 +89,8 @@ public WorkflowState( User user, Map userOutputs, List resourcesCreated, - String tenantId + String tenantId, + List allSharedPrincipals ) { this.workflowId = workflowId; this.error = error; @@ -98,6 +102,7 @@ public WorkflowState( this.userOutputs = Map.copyOf(userOutputs); this.resourcesCreated = List.copyOf(resourcesCreated); this.tenantId = tenantId; + this.allSharedPrincipals = allSharedPrincipals; } private WorkflowState() {} @@ -125,6 +130,7 @@ public WorkflowState(StreamInput input) throws IOException { if (input.getVersion().onOrAfter(CommonValue.VERSION_2_19_0)) { this.tenantId = input.readOptionalString(); } + } /** @@ -158,6 +164,7 @@ public static class Builder { private Map userOutputs = null; private List resourcesCreated = null; private String tenantId = null; + private List allSharedPrincipals = null; /** * Empty Constructor for the Builder object @@ -179,6 +186,7 @@ private Builder(WorkflowState existingState) { this.userOutputs = existingState.userOutputs(); this.resourcesCreated = existingState.resourcesCreated(); this.tenantId = existingState.getTenantId(); + this.allSharedPrincipals = existingState.getAllSharedPrincipals(); } /** @@ -281,6 +289,11 @@ public Builder tenantId(String tenantId) { return this; } + public Builder allSharedPrincipals(List allSharedPrincipals) { + this.allSharedPrincipals = allSharedPrincipals; + return this; + } + /** * Allows building a workflowState * @return WorkflowState workflowState Object containing all needed fields @@ -297,6 +310,7 @@ public WorkflowState build() { workflowState.userOutputs = this.userOutputs; workflowState.resourcesCreated = this.resourcesCreated; workflowState.tenantId = this.tenantId; + workflowState.allSharedPrincipals = this.allSharedPrincipals; return workflowState; } } @@ -339,6 +353,10 @@ public static WorkflowState updateExistingWorkflowState(WorkflowState existingSt if (stateWithNewFields.getTenantId() != null) { builder.tenantId(stateWithNewFields.getTenantId()); } + if (stateWithNewFields.getAllSharedPrincipals() != null && !stateWithNewFields.getAllSharedPrincipals().isEmpty()) { + existingState.getAllSharedPrincipals().addAll(stateWithNewFields.getAllSharedPrincipals()); + builder.allSharedPrincipals(existingState.getAllSharedPrincipals()); + } return builder.build(); } @@ -375,6 +393,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws if (tenantId != null) { xContentBuilder.field(TENANT_ID_FIELD, tenantId); } + + if (allSharedPrincipals != null && !allSharedPrincipals.isEmpty()) { + xContentBuilder.startArray(ALL_SHARED_PRINCIPALS_FIELD); + for (String allSharedPrincipal : allSharedPrincipals) { + xContentBuilder.value(allSharedPrincipal); + } + xContentBuilder.endArray(); + } return xContentBuilder.endObject(); } @@ -428,6 +454,7 @@ public static WorkflowState parse(XContentParser parser) throws IOException { Map userOutputs = new HashMap<>(); List resourcesCreated = new ArrayList<>(); String tenantId = null; + List allSharedPrincipals = new ArrayList<>(); ensureExpectedToken(XContentParser.Token.START_OBJECT, parser.currentToken(), parser); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { @@ -490,6 +517,12 @@ public static WorkflowState parse(XContentParser parser) throws IOException { case TENANT_ID_FIELD: tenantId = parser.text(); break; + case ALL_SHARED_PRINCIPALS_FIELD: + ensureExpectedToken(XContentParser.Token.START_ARRAY, parser.currentToken(), parser); + while (parser.nextToken() != XContentParser.Token.END_ARRAY) { + allSharedPrincipals.add(parser.text()); + } + break; default: throw new FlowFrameworkException( "Unable to parse field [" + fieldName + "] in a workflowState object.", @@ -606,6 +639,10 @@ public String getTenantId() { return tenantId; } + public List getAllSharedPrincipals() { + return allSharedPrincipals; + } + @Override public String toString() { return "WorkflowState [workflowId=" diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java index 21762d168..8c5d78138 100644 --- a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java @@ -239,7 +239,8 @@ private void createExecute(WorkflowRequest request, User user, String tenantId, creationTime, creationTime, null, - tenantId + tenantId, + request.getTemplate().allSharedPrincipals() ); String[] validateAll = { "all" }; diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java index 79f1c1f2c..bf262ac2b 100644 --- a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java @@ -10,6 +10,7 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.DocRequest; import org.opensearch.common.Nullable; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -20,7 +21,7 @@ /** * Transport Request to get a workflow status */ -public class GetWorkflowStateRequest extends ActionRequest { +public class GetWorkflowStateRequest extends ActionRequest implements DocRequest { /** * The documentId of the workflow entry within the Global Context index @@ -100,4 +101,14 @@ public void writeTo(StreamOutput out) throws IOException { public ActionRequestValidationException validate() { return null; } + + @Override + public String index() { + return CommonValue.WORKFLOW_STATE_INDEX; + } + + @Override + public String id() { + return workflowId; + } } diff --git a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java index b9647e17e..de191e004 100644 --- a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java @@ -10,6 +10,7 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.DocRequest; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.StreamInput; import org.opensearch.core.common.io.stream.StreamOutput; @@ -21,7 +22,7 @@ /** * Transport request to reprovision a workflow */ -public class ReprovisionWorkflowRequest extends ActionRequest { +public class ReprovisionWorkflowRequest extends ActionRequest implements DocRequest { /** * The workflow Id @@ -123,4 +124,14 @@ public Template getUpdatedTemplate() { public TimeValue getWaitForCompletionTimeout() { return this.waitForCompletionTimeout; } + + @Override + public String index() { + return CommonValue.GLOBAL_CONTEXT_INDEX; + } + + @Override + public String id() { + return workflowId; + } } diff --git a/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java b/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java index 6fc34d7ef..f20f94456 100644 --- a/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java @@ -10,6 +10,7 @@ import org.opensearch.action.ActionRequest; import org.opensearch.action.ActionRequestValidationException; +import org.opensearch.action.DocRequest; import org.opensearch.common.Nullable; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.common.io.stream.StreamInput; @@ -28,7 +29,7 @@ /** * Transport Request to create, provision, and deprovision a workflow */ -public class WorkflowRequest extends ActionRequest { +public class WorkflowRequest extends ActionRequest implements DocRequest { /** * The documentId of the workflow entry within the Global Context index @@ -283,4 +284,14 @@ public void writeTo(StreamOutput out) throws IOException { public ActionRequestValidationException validate() { return null; } + + @Override + public String index() { + return CommonValue.GLOBAL_CONTEXT_INDEX; + } + + @Override + public String id() { + return workflowId; + } } diff --git a/src/main/resources/mappings/global-context.json b/src/main/resources/mappings/global-context.json index b79ba07df..e089d8867 100644 --- a/src/main/resources/mappings/global-context.json +++ b/src/main/resources/mappings/global-context.json @@ -92,6 +92,9 @@ "ui_metadata": { "type": "object", "enabled": false + }, + "all_shared_principals": { + "type": "keyword" } } } diff --git a/src/main/resources/mappings/workflow-state.json b/src/main/resources/mappings/workflow-state.json index c9627d670..f90885e87 100644 --- a/src/main/resources/mappings/workflow-state.json +++ b/src/main/resources/mappings/workflow-state.json @@ -87,6 +87,9 @@ }, "tenant_id": { "type": "keyword" + }, + "all_shared_principals": { + "type": "keyword" } } } diff --git a/src/test/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandlerTests.java b/src/test/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandlerTests.java index bcfd6c6e4..d38a412e6 100644 --- a/src/test/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandlerTests.java +++ b/src/test/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandlerTests.java @@ -152,7 +152,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); } @@ -330,7 +331,8 @@ public void testCanDeleteWorkflowStateDoc() { TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -360,7 +362,8 @@ public void testCanNotDeleteWorkflowStateDocInProgress() { TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); @@ -390,7 +393,8 @@ public void testDeleteWorkflowStateDocResourcesExist() { TestHelpers.randomUser(), Collections.emptyMap(), List.of(new ResourceCreated("w", "x", "y", "z")), - null + null, + Collections.emptyList() ); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(1); diff --git a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java index ec419aeeb..b2c3751c5 100644 --- a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java @@ -61,7 +61,8 @@ public void testTemplate() throws IOException { now, now, null, - null + null, + Collections.emptyList() ); assertEquals("test", template.name()); @@ -123,7 +124,8 @@ public void testUpdateExistingTemplate() { now, now, null, - null + null, + Collections.emptyList() ); Template updated = Template.builder().name("name two").description("description two").useCase("use case two").build(); Template merged = Template.updateExistingTemplate(original, updated); diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java index b23be9176..fe0ca5e73 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java @@ -59,7 +59,7 @@ public class FlowFrameworkResourceSharingRestApiIT extends FlowFrameworkRestTest private static final String WORKFLOW_STATE_FULL_ACCESS_AG = "workflow_state_full_access"; // If the suite is launched without the flag, just skip these tests cleanly. - private boolean skipTests = !isResourceSharingFeatureEnabled(); + private final boolean skipTests = !isResourceSharingFeatureEnabled(); @Before public void setupSecureTests() throws IOException { diff --git a/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java b/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java index 3d0e299ee..40bdcaf1d 100644 --- a/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java +++ b/src/test/java/org/opensearch/flowframework/rest/RestCreateWorkflowActionTests.java @@ -84,7 +84,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); this.validTemplate = template.toJson(); diff --git a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java index 1b849ab9d..ebeb38213 100644 --- a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java @@ -174,7 +174,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); } @@ -240,7 +241,8 @@ public void testValidation_Failed() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); @SuppressWarnings("unchecked") @@ -664,7 +666,8 @@ public void testHandleReprovisionWithSpecifiedTimeout() throws IOException { TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ); // Mock the reprovision response with both workflow ID and state @@ -1003,7 +1006,8 @@ public void testCreateWorkflow_withValidation_withWaitForCompletion_withProvisio TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ) ); responseListener.onResponse(response); @@ -1079,7 +1083,8 @@ public void testCreateWorkflow_withValidation_withWaitForCompletionTimeSetZero_w TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ) ); responseListener.onResponse(response); @@ -1203,7 +1208,8 @@ private Template generateValidTemplate() { null, null, null, - null + null, + Collections.emptyList() ); return validTemplate; diff --git a/src/test/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportActionTests.java index 76311265f..2979a3672 100644 --- a/src/test/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportActionTests.java @@ -156,7 +156,8 @@ public void testGetWorkflowStateResponse() throws IOException { TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ); GetWorkflowStateResponse response = new GetWorkflowStateResponse(workFlowState, false); diff --git a/src/test/java/org/opensearch/flowframework/transport/GetWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/GetWorkflowTransportActionTests.java index 61a7249b1..7a2cbd721 100644 --- a/src/test/java/org/opensearch/flowframework/transport/GetWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/GetWorkflowTransportActionTests.java @@ -120,7 +120,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); ThreadPool clientThreadPool = mock(ThreadPool.class); diff --git a/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java index e0fce9eec..07a2d567c 100644 --- a/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportActionTests.java @@ -142,7 +142,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); ThreadPool clientThreadPool = mock(ThreadPool.class); diff --git a/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java b/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java index 397d4d9e7..a36042232 100644 --- a/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java @@ -55,7 +55,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); this.updatedTemplate = new Template( @@ -70,7 +71,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); } diff --git a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java index 32c77cbf5..5eb18ecd5 100644 --- a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java @@ -60,7 +60,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); } @@ -223,7 +224,8 @@ public void testWorkflowResponseWithWaitForCompletionTimeOut() throws IOExceptio TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ); WorkflowResponse response = new WorkflowResponse("123", workFlowState); diff --git a/src/test/java/org/opensearch/flowframework/util/EncryptorUtilsTests.java b/src/test/java/org/opensearch/flowframework/util/EncryptorUtilsTests.java index 33c36b74b..5d54783cd 100644 --- a/src/test/java/org/opensearch/flowframework/util/EncryptorUtilsTests.java +++ b/src/test/java/org/opensearch/flowframework/util/EncryptorUtilsTests.java @@ -106,7 +106,8 @@ public void setUp() throws Exception { null, null, null, - null + null, + Collections.emptyList() ); ClusterState clusterState = mock(ClusterState.class); diff --git a/src/test/java/org/opensearch/flowframework/util/WorkflowTimeoutUtilityTests.java b/src/test/java/org/opensearch/flowframework/util/WorkflowTimeoutUtilityTests.java index c7b9f3fb4..2d44e01a7 100644 --- a/src/test/java/org/opensearch/flowframework/util/WorkflowTimeoutUtilityTests.java +++ b/src/test/java/org/opensearch/flowframework/util/WorkflowTimeoutUtilityTests.java @@ -88,7 +88,8 @@ public void testWrapWithTimeoutCancellationListenerOnResponse() { TestHelpers.randomUser(), Collections.emptyMap(), Collections.emptyList(), - null + null, + Collections.emptyList() ) ); Scheduler.ScheduledCancellable scheduledCancellable = mock(Scheduler.ScheduledCancellable.class); diff --git a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java index 4d7375d6b..aa4fb9917 100644 --- a/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java +++ b/src/test/java/org/opensearch/flowframework/workflow/WorkflowProcessSorterTests.java @@ -177,7 +177,8 @@ public static void setup() throws IOException { now, now, null, - null + null, + Collections.emptyList() ); pipelineResource = new ResourceCreated(CreateSearchPipelineStep.NAME, "workflow_step_1", PIPELINE_ID, pipelineId); @@ -667,7 +668,8 @@ public void testCreateReprovisionSequenceWithDeletion() { now, now, null, - null + null, + Collections.emptyList() ); FlowFrameworkException ex = expectThrows( @@ -739,7 +741,8 @@ public void testCreateReprovisionSequenceWithAdditiveModification() throws Excep now, now, null, - null + null, + Collections.emptyList() ); List reprovisionSequence = workflowProcessSorter.createReprovisionSequence( @@ -805,7 +808,8 @@ public void testCreateReprovisionSequenceWithUpdates() throws Exception { now, now, null, - null + null, + Collections.emptyList() ); List reprovisionSequence = workflowProcessSorter.createReprovisionSequence( From 0a2f4fb4792f5e04da7cf93f80f00bcd059b6a10 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 13:55:18 -0800 Subject: [PATCH 16/23] Fix checkstyle errors Signed-off-by: Darshit Chanpura --- .../org/opensearch/flowframework/model/Template.java | 5 +++++ .../opensearch/flowframework/model/WorkflowState.java | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java index f6ffe7394..7a9f62f7e 100644 --- a/src/main/java/org/opensearch/flowframework/model/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -95,6 +95,7 @@ public class Template implements ToXContentObject { * @param lastUpdatedTime Last Updated time as an Instant * @param lastProvisionedTime Last Provisioned time as an Instant * @param tenantId The tenant id + * @param allSharedPrincipals The entities this template this shared with */ public Template( String name, @@ -751,6 +752,10 @@ public void setTenantId(String tenantId) { this.tenantId = tenantId; } + /** + * Sets the entities this resource is shared with + * @return the set shared principals + */ public List allSharedPrincipals() { return allSharedPrincipals; } diff --git a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java index b311ebd5d..477406296 100644 --- a/src/main/java/org/opensearch/flowframework/model/WorkflowState.java +++ b/src/main/java/org/opensearch/flowframework/model/WorkflowState.java @@ -289,6 +289,11 @@ public Builder tenantId(String tenantId) { return this; } + /** + * Builder method for adding all shared principals + * @param allSharedPrincipals the entities this workflow state is shared with + * @return the Builder object + */ public Builder allSharedPrincipals(List allSharedPrincipals) { this.allSharedPrincipals = allSharedPrincipals; return this; @@ -540,6 +545,7 @@ public static WorkflowState parse(XContentParser parser) throws IOException { .userOutputs(userOutputs) .resourcesCreated(resourcesCreated) .tenantId(tenantId) + .allSharedPrincipals(allSharedPrincipals) .build(); } @@ -639,6 +645,10 @@ public String getTenantId() { return tenantId; } + /** + * The entities this state is shared with + * @return the entities + */ public List getAllSharedPrincipals() { return allSharedPrincipals; } From 7be08f97ac8516d8d4cbb0dd87e0142050dd6b65 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 15:09:27 -0800 Subject: [PATCH 17/23] Fix early .onResponse return and add all_shared_principals to workflow state reset call Signed-off-by: Darshit Chanpura --- .../indices/FlowFrameworkIndicesHandler.java | 9 ++++++- .../CreateWorkflowTransportAction.java | 1 + .../DeprovisionWorkflowTransportAction.java | 26 +++++++++++++++---- .../CreateWorkflowTransportActionTests.java | 12 ++++----- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java index 8219dc3be..9c97be0d2 100644 --- a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java +++ b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java @@ -412,7 +412,13 @@ public void initializeConfigIndex(String tenantId, ActionListener liste * @param user passes the user that created the workflow * @param listener action listener */ - public void putInitialStateToWorkflowState(String workflowId, String tenantId, User user, ActionListener listener) { + public void putInitialStateToWorkflowState( + String workflowId, + String tenantId, + User user, + List allSharedPrincipals, + ActionListener listener + ) { WorkflowState state = WorkflowState.builder() .workflowId(workflowId) .state(State.NOT_STARTED.name()) @@ -421,6 +427,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) { diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java index 8c5d78138..b28b6754f 100644 --- a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java @@ -303,6 +303,7 @@ private void createExecute(WorkflowRequest request, User user, String tenantId, globalContextResponse.getId(), tenantId, user, + templateWithUser.allSharedPrincipals(), ActionListener.wrap(stateResponse -> { logger.info("Creating state workflow doc: {}", globalContextResponse.getId()); if (request.isProvision()) { diff --git a/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java index 7b6e97643..90f366b98 100644 --- a/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java @@ -191,6 +191,7 @@ private void executeDeprovisionRequest( workflowId, tenantId, response.getWorkflowState().resourcesCreated(), + response.getWorkflowState().getAllSharedPrincipals(), deleteAllowedResources, listener, user @@ -210,6 +211,7 @@ private void executeDeprovisionSequence( String workflowId, String tenantId, List resourcesCreated, + List allSharedPrincipals, Set deleteAllowedResources, ActionListener listener, User user @@ -328,13 +330,14 @@ private void executeDeprovisionSequence( logger.info("Resources requiring allow_delete: {}.", deleteNotAllowed); } // This is a redundant best-effort backup to the incremental deletion done earlier - updateWorkflowState(workflowId, tenantId, remainingResources, deleteNotAllowed, listener, user); + updateWorkflowState(workflowId, tenantId, remainingResources, allSharedPrincipals, deleteNotAllowed, listener, user); } private void updateWorkflowState( String workflowId, String tenantId, List remainingResources, + List allSharedPrincipals, List deleteNotAllowed, ActionListener listener, User user @@ -347,9 +350,17 @@ private void updateWorkflowState( workflowId, tenantId, user, + allSharedPrincipals, ActionListener.wrap(indexResponse -> { logger.info("Reset workflow {} state to NOT_STARTED", workflowId); - }, exception -> { logger.error("Failed to reset to initial workflow state for {}", workflowId, exception); }) + // return workflow ID + listener.onResponse(new WorkflowResponse(workflowId)); + }, exception -> { + logger.error("Failed to reset to initial workflow state for {}", workflowId, exception); + listener.onFailure( + new FlowFrameworkException("Failed to reset workflow state", ExceptionsHelper.status(exception)) + ); + }) ); } else { flowFrameworkIndicesHandler.deleteFlowFrameworkSystemIndexDoc( @@ -357,11 +368,16 @@ private void updateWorkflowState( tenantId, ActionListener.wrap(deleteResponse -> { logger.info("Deleted workflow {} state", workflowId); - }, exception -> { logger.error("Failed to delete workflow state for {}", workflowId, exception); }) + // return workflow ID + listener.onResponse(new WorkflowResponse(workflowId)); + }, exception -> { + logger.error("Failed to delete workflow state for {}", workflowId, exception); + listener.onFailure( + new FlowFrameworkException("Failed to reset workflow state", ExceptionsHelper.status(exception)) + ); + }) ); } - // return workflow ID - listener.onResponse(new WorkflowResponse(workflowId)); }, listener); } else { // Remaining resources only includes ones we tried to delete diff --git a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java index ebeb38213..0e2f6799c 100644 --- a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java @@ -379,7 +379,7 @@ public void testCreateNewWorkflow() { ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); ArgumentCaptor workflowResponseCaptor = ArgumentCaptor.forClass(WorkflowResponse.class); @@ -453,7 +453,7 @@ public void testCreateWithUserAndFilterOn() { ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); ArgumentCaptor workflowResponseCaptor = ArgumentCaptor.forClass(WorkflowResponse.class); @@ -928,7 +928,7 @@ public void testCreateWorkflow_withValidation_withProvision_Success() throws Exc ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(2); @@ -989,7 +989,7 @@ public void testCreateWorkflow_withValidation_withWaitForCompletion_withProvisio ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(2); @@ -1066,7 +1066,7 @@ public void testCreateWorkflow_withValidation_withWaitForCompletionTimeSetZero_w ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(2); @@ -1143,7 +1143,7 @@ public void testCreateWorkflow_withValidation_withProvision_FailedProvisioning() ActionListener responseListener = invocation.getArgument(3); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; - }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any()); + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); doAnswer(invocation -> { ActionListener responseListener = invocation.getArgument(2); From 821e137026f3853b1dc94c8cde015630f131f305 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 18:06:36 -0800 Subject: [PATCH 18/23] Add implementation of type method Signed-off-by: Darshit Chanpura --- .../flowframework/transport/GetWorkflowStateRequest.java | 5 +++++ .../flowframework/transport/ReprovisionWorkflowRequest.java | 5 +++++ .../opensearch/flowframework/transport/WorkflowRequest.java | 5 +++++ 3 files changed, 15 insertions(+) diff --git a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java index bf262ac2b..f798bdb5c 100644 --- a/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateRequest.java @@ -111,4 +111,9 @@ public String index() { public String id() { return workflowId; } + + @Override + public String type() { + return CommonValue.WORKFLOW_STATE_RESOURCE_TYPE; + } } diff --git a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java index de191e004..b961bfe62 100644 --- a/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequest.java @@ -134,4 +134,9 @@ public String index() { public String id() { return workflowId; } + + @Override + public String type() { + return CommonValue.WORKFLOW_RESOURCE_TYPE; + } } diff --git a/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java b/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java index f20f94456..3736f761e 100644 --- a/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java +++ b/src/main/java/org/opensearch/flowframework/transport/WorkflowRequest.java @@ -294,4 +294,9 @@ public String index() { public String id() { return workflowId; } + + @Override + public String type() { + return CommonValue.WORKFLOW_RESOURCE_TYPE; + } } From 12756cd607b50e442691c8ec37112ceaea2f80ba Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 18:06:58 -0800 Subject: [PATCH 19/23] Completes FlowFrameworkSecureRestApiIT Signed-off-by: Darshit Chanpura --- .../rest/FlowFrameworkSecureRestApiIT.java | 82 +++++++++++++++---- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index f084f60b7..0bc0f925e 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -179,7 +179,12 @@ public void testGetWorkflowStepsWithReadAccess() throws Exception { public void testGetWorkflowWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(bobClient, "test")); - assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + if (isResourceSharingFeatureEnabled()) { + // 403 because resource is not shared + assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + } else { + assertEquals(RestStatus.NOT_FOUND, TestHelpers.restStatus(exception.getResponse())); + } } public void testFilterByDisabled() throws Exception { @@ -188,8 +193,15 @@ public void testFilterByDisabled() throws Exception { Response aliceWorkflow = createWorkflow(aliceClient, template); Map responseMap = entityAsMap(aliceWorkflow); String workflowId = (String) responseMap.get(WORKFLOW_ID); - Response response = getWorkflow(catClient, workflowId); - assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + + // when resources sharing is enabled, we throw 403 + if (isResourceSharingFeatureEnabled()) { + ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId)); + assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus()); + } else { + Response response = getWorkflow(catClient, workflowId); + assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); + } } public void testSearchWorkflowWithReadAccess() throws Exception { @@ -212,9 +224,15 @@ public void testGetWorkflowStateWithReadAccess() throws Exception { Map responseMap = entityAsMap(response); String workflowId = (String) responseMap.get(WORKFLOW_ID); - // No permissions to create or provision, so we assert only that the response status isnt forbidden - Response searchResponse = getWorkflowStatus(bobClient, workflowId, false); - assertEquals(RestStatus.OK, TestHelpers.restStatus(searchResponse)); + if (isResourceSharingFeatureEnabled()) { + ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(bobClient, workflowId, false)); + assertTrue(exception.getMessage().contains("Failed to get workflow status")); + assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus()); + } else { + // No permissions to create or provision, so we assert only that the response status isnt forbidden + Response searchResponse = getWorkflowStatus(bobClient, workflowId, false); + assertEquals(RestStatus.OK, TestHelpers.restStatus(searchResponse)); + } } public void testSearchWorkflowStateWithReadAccess() throws Exception { @@ -226,18 +244,26 @@ public void testSearchWorkflowStateWithReadAccess() throws Exception { // No permissions to create, so we assert only that the response status isnt forbidden String termIdQuery = "{\"query\":{\"bool\":{\"filter\":[{\"term\":{\"ids\":\"test\"}}]}}}"; SearchResponse searchResponse = searchWorkflowState(bobClient, termIdQuery); + // when resource sharing is enabled this would return an empty response assertEquals(RestStatus.OK, searchResponse.status()); } - public void testCreateWorkflowWithNoBackendRole() throws IOException { + public void testCreateWorkflowWithNoBackendRole() throws Exception { enableFilterBy(); // User Dog has FF full access, but has no backend role // When filter by is enabled, we block creating workflows Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Exception exception = expectThrows(IOException.class, () -> { createWorkflow(dogClient, template); }); - assertTrue( - exception.getMessage().contains("Filter by backend roles is enabled, but User dog does not have any backend roles configured") - ); + if (isResourceSharingFeatureEnabled()) { + // If resource sharing is enabled, we allow creation of workflows regardless of the user's backend roles + Response dogWorkflow = createWorkflow(dogClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(dogWorkflow)); + } else { + Exception exception = expectThrows(IOException.class, () -> { createWorkflow(dogClient, template); }); + assertTrue( + exception.getMessage() + .contains("Filter by backend roles is enabled, but User dog does not have any backend roles configured") + ); + } } public void testDeprovisionWorkflowWithWriteAccess() throws Exception { @@ -259,7 +285,12 @@ public void testGetWorkflowWithFilterEnabled() throws Exception { String workflowId = (String) responseMap.get(WORKFLOW_ID); // User Cat has FF full access, but is part of different backend role so Cat should not be able to access alice workflow ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId)); - assertTrue(exception.getMessage().contains("User does not have permissions to access workflow: " + workflowId)); + if (isResourceSharingFeatureEnabled()) { + assertTrue(exception.getMessage().contains("Failed to get workflow.")); + assertEquals(exception.getResponse().getStatusLine().getStatusCode(), RestStatus.FORBIDDEN.getStatus()); + } else { + assertTrue(exception.getMessage().contains("User does not have permissions to access workflow: " + workflowId)); + } } public void testGetWorkflowFilterbyEnabledForAdmin() throws Exception { @@ -402,7 +433,15 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except // Invoke status API with failure ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(aliceClient, workflowId, false)); - assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + if (isResourceSharingFeatureEnabled()) { + // sample log message: No sharing info found for 'pmaAdZoBL3PmSyJoha0C'. Action + // cluster:admin/opensearch/flow_framework/workflow_state/get is not allowed. + // since record is deleted corresponding sharing record will also be deleted and thus we show 403 for non-existent sharing + // records, + assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + } else { + assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + } } public void testUpdateWorkflowEnabledForAdmin() throws Exception { @@ -470,11 +509,18 @@ public void testUpdateWorkflowWithFilterEnabled() throws Exception { String workflowId = (String) responseMap.get(WORKFLOW_ID); enableFilterBy(); - // User Fish has FF full access, and has "odfe" backend role which is one of Alice's backend role, so - // Fish should be able to update workflows created by Alice. But the workflow's backend role should - // not be replaced as Fish's backend roles. - Response updateResponse = updateWorkflow(fishClient, workflowId, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(updateResponse)); + if (isResourceSharingFeatureEnabled()) { + ResponseException exception = expectThrows(ResponseException.class, () -> updateWorkflow(fishClient, workflowId, template)); + // we show 403 because fishClient doesn't have resource access + assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + // it should be 200 after sharing but we test that in a resource-sharing test class + } else { + // User Fish has FF full access, and has "odfe" backend role which is one of Alice's backend role, so + // Fish should be able to update workflows created by Alice. But the workflow's backend role should + // not be replaced as Fish's backend roles. + Response updateResponse = updateWorkflow(fishClient, workflowId, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(updateResponse)); + } } public void testUpdateWorkflowWithNoFFAccess() throws Exception { From e9a62f45e773bd43c4e670f665a1830636a32982 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 18:33:01 -0800 Subject: [PATCH 20/23] Fixes javadoc CI and addresses flakyness in search test when resource sharing is enab;ed Signed-off-by: Darshit Chanpura --- .../indices/FlowFrameworkIndicesHandler.java | 1 + ...FlowFrameworkResourceSharingRestApiIT.java | 62 +++++++++++++------ 2 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java index 9c97be0d2..0f95783ab 100644 --- a/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java +++ b/src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java @@ -410,6 +410,7 @@ public void initializeConfigIndex(String tenantId, ActionListener 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( diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java index fe0ca5e73..5417cf11c 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkResourceSharingRestApiIT.java @@ -23,6 +23,7 @@ import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -31,6 +32,9 @@ import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_RESOURCE_TYPE; import static org.opensearch.flowframework.common.CommonValue.WORKFLOW_STATE_RESOURCE_TYPE; +/** + * Tests flow-framework resources: workflow and workflow_state with resource sharing enabled + */ public class FlowFrameworkResourceSharingRestApiIT extends FlowFrameworkRestTestCase { String aliceUser = "alice"; @@ -61,6 +65,8 @@ public class FlowFrameworkResourceSharingRestApiIT extends FlowFrameworkRestTest // If the suite is launched without the flag, just skip these tests cleanly. private final boolean skipTests = !isResourceSharingFeatureEnabled(); + private final Set workflowIdsToCleanup = new HashSet<>(); + @Before public void setupSecureTests() throws IOException { if (skipTests) { @@ -126,6 +132,24 @@ public void tearDownSecureTests() throws IOException { if (skipTests) { return; } + + // Clean up any workflows created by this test class + for (String workflowId : workflowIdsToCleanup) { + try { + // Owner (alice) can always delete the workflow + deleteWorkflow(aliceClient, workflowId); + } catch (ResponseException e) { + // Ignore 404 / already-deleted cases + if (e.getResponse().getStatusLine().getStatusCode() != RestStatus.NOT_FOUND.getStatus()) { + logger.warn("Non-404 error while cleaning up workflow {}", workflowId, e); + } + } catch (Exception e) { + logger.warn("Failed to clean up workflow {}", workflowId, e); + } + } + workflowIdsToCleanup.clear(); + + // Now close clients and delete users/roles aliceClient.close(); bobClient.close(); catClient.close(); @@ -142,15 +166,21 @@ public void tearDownSecureTests() throws IOException { deleteUser(lionUser); } - public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + private String createWorkflowAndTrack(RestClient restClient, Template template) throws Exception { + Response created = createWorkflow(restClient, template); + assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); + String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + workflowIdsToCleanup.add(workflowId); + return workflowId; + } + + public void testWorkflowVisibilityAndSearch() throws Exception { if (skipTests) { logger.info("Skipping test - resource sharing not enabled"); return; } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Response created = createWorkflow(aliceClient, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); - String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + String workflowId = createWorkflowAndTrack(aliceClient, template); // Unshared → cat/admin cannot GET ResponseException ex = expectThrows(ResponseException.class, () -> getWorkflow(catClient, workflowId)); @@ -225,14 +255,12 @@ public void testWorkflowVisibilityAndSearch_withResourceSharingEnabled() throws waitForWorkflowRevokeNonVisibility(workflowId, catClient); } - public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { + public void testWorkflowUpdate() throws Exception { if (skipTests) { return; } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Response created = createWorkflow(aliceClient, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); - String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + String workflowId = createWorkflowAndTrack(aliceClient, template); // Unshared → admin/fish/cat cannot update ResponseException ex = expectThrows(ResponseException.class, () -> updateWorkflow(client(), workflowId, template)); @@ -278,14 +306,12 @@ public void testWorkflowUpdate_withResourceSharingEnabled() throws Exception { assertEquals(RestStatus.FORBIDDEN.getStatus(), elkDenied.getResponse().getStatusLine().getStatusCode()); } - public void testProvisionDeprovision_withResourceSharingEnabled() throws Exception { + public void testProvisionDeprovision() throws Exception { if (skipTests) { return; } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Response created = createWorkflow(aliceClient, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); - String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + String workflowId = createWorkflowAndTrack(aliceClient, template); // Unshared → cat cannot provision/deprovision ResponseException ex = expectThrows(ResponseException.class, () -> provisionWorkflow(catClient, workflowId)); @@ -334,14 +360,12 @@ public void testProvisionDeprovision_withResourceSharingEnabled() throws Excepti assertEquals(RestStatus.FORBIDDEN.getStatus(), elkDenied.getResponse().getStatusLine().getStatusCode()); } - public void testDeleteWorkflow_withResourceSharingEnabled() throws Exception { + public void testDeleteWorkflow() throws Exception { if (skipTests) { return; } Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Response created = createWorkflow(aliceClient, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); - String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + String workflowId = createWorkflowAndTrack(aliceClient, template); // Unshared → cat/admin cannot delete ResponseException ex = expectThrows(ResponseException.class, () -> deleteWorkflow(catClient, workflowId)); @@ -362,15 +386,13 @@ public void testDeleteWorkflow_withResourceSharingEnabled() throws Exception { assertEquals(RestStatus.OK, TestHelpers.restStatus(del)); } - public void testWorkflowStateVisibilityAndSearch_withResourceSharingEnabled() throws Exception { + public void testWorkflowStateVisibilityAndSearch() throws Exception { if (skipTests) { return; } // Create a workflow (state doc gets created/managed by FF) Template template = TestHelpers.createTemplateFromFile("register-deploylocalsparseencodingmodel.json"); - Response created = createWorkflow(aliceClient, template); - assertEquals(RestStatus.CREATED, TestHelpers.restStatus(created)); - String workflowId = (String) entityAsMap(created).get(WORKFLOW_ID); + String workflowId = createWorkflowAndTrack(aliceClient, template); // Unshared state → cat/admin cannot read status or find it ResponseException ex = expectThrows(ResponseException.class, () -> getWorkflowStatus(catClient, workflowId, false)); From 582fca51bd4df150eda51c31e93d67abc580b2dd Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Tue, 11 Nov 2025 21:07:12 -0800 Subject: [PATCH 21/23] Fixes unit tests Signed-off-by: Darshit Chanpura --- .../CreateWorkflowTransportActionTests.java | 12 +++++----- ...provisionWorkflowTransportActionTests.java | 22 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java index 0e2f6799c..a9f2c3060 100644 --- a/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/CreateWorkflowTransportActionTests.java @@ -376,7 +376,7 @@ public void testCreateNewWorkflow() { // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); @@ -450,7 +450,7 @@ public void testCreateWithUserAndFilterOn() { // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); @@ -925,7 +925,7 @@ public void testCreateWorkflow_withValidation_withProvision_Success() throws Exc // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); @@ -986,7 +986,7 @@ public void testCreateWorkflow_withValidation_withWaitForCompletion_withProvisio // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); @@ -1063,7 +1063,7 @@ public void testCreateWorkflow_withValidation_withWaitForCompletionTimeSetZero_w // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); @@ -1140,7 +1140,7 @@ public void testCreateWorkflow_withValidation_withProvision_FailedProvisioning() // Bypass putInitialStateToWorkflowState and force on response doAnswer(invocation -> { - ActionListener responseListener = invocation.getArgument(3); + ActionListener responseListener = invocation.getArgument(4); responseListener.onResponse(new IndexResponse(new ShardId(WORKFLOW_STATE_INDEX, "", 1), "1", 1L, 1L, 1L, true)); return null; }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(any(), any(), any(), any(), any()); diff --git a/src/test/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportActionTests.java b/src/test/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportActionTests.java index 99f039686..492b9fa43 100644 --- a/src/test/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportActionTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportActionTests.java @@ -170,6 +170,13 @@ public void testDeprovisionWorkflow() throws Exception { return null; }).when(flowFrameworkIndicesHandler).doesTemplateExist(anyString(), any(), any(), any()); + // putInitialStateToWorkflowState must invoke its ActionListener.onResponse + doAnswer(inv -> { + ActionListener l = inv.getArgument(4); + l.onResponse(null); + return null; + }).when(flowFrameworkIndicesHandler).putInitialStateToWorkflowState(anyString(), any(), any(), any(), any()); + PlainActionFuture future = PlainActionFuture.newFuture(); future.onResponse(WorkflowData.EMPTY); when(this.deleteConnectorStep.execute(anyString(), any(WorkflowData.class), anyMap(), anyMap(), anyMap(), nullable(String.class))) @@ -291,6 +298,21 @@ public void testAllowDeleteRequired() throws Exception { "These resources require the allow_delete parameter to deprovision: [index_name test-index].", exceptionCaptor.getValue().getMessage() ); + + // doesTemplateExist -> false for this test + doAnswer(inv -> { + java.util.function.Consumer cb = inv.getArgument(2); + cb.accept(Boolean.FALSE); + return null; + }).when(flowFrameworkIndicesHandler).doesTemplateExist(anyString(), any(), any(), any()); + + // deleteFlowFrameworkSystemIndexDoc must call onResponse + doAnswer(inv -> { + ActionListener l = inv.getArgument(2); + l.onResponse(null); + return null; + }).when(flowFrameworkIndicesHandler).deleteFlowFrameworkSystemIndexDoc(anyString(), any(), any()); + verify(flowFrameworkIndicesHandler, times(0)).deleteResourceFromStateIndex( anyString(), nullable(String.class), From f34d4bacff5e1a625497dfa96c7dea2fb3dd17f7 Mon Sep 17 00:00:00 2001 From: Darshit Chanpura Date: Wed, 12 Nov 2025 16:22:44 -0800 Subject: [PATCH 22/23] Updates coverage and adds missing builder method call in Template class Signed-off-by: Darshit Chanpura --- .github/workflows/test_security.yml | 4 +- .../flowframework/model/Template.java | 1 + .../flowframework/util/PluginClient.java | 2 +- .../util/ResourceSharingClientAccessor.java | 12 ++---- .../flowframework/model/TemplateTests.java | 35 +++++++++++++++ .../model/WorkflowStateTests.java | 24 +++++++++++ .../ReprovisionWorkflowRequestTests.java | 10 +++++ .../WorkflowRequestResponseTests.java | 43 +++++++++++++++++++ 8 files changed, 120 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test_security.yml b/.github/workflows/test_security.yml index 48c46784b..b985b743a 100644 --- a/.github/workflows/test_security.yml +++ b/.github/workflows/test_security.yml @@ -20,7 +20,7 @@ jobs: strategy: matrix: java: [21, 25] - resource_sharing_flag: [ "", "-Dresource_sharing.enabled=true" ] + resource_sharing_flag: [ false, true ] name: Run Security Integration Tests on Linux runs-on: ubuntu-latest @@ -49,5 +49,5 @@ jobs: su `id -un 1000` -c "whoami && java -version && ./gradlew integTest \ -Dsecurity.enabled=true \ -Dhttps=true \ - ${{ matrix.resource_sharing_flag }} \ + -Dresource_sharing.enabled=${{ matrix.resource_sharing_flag }} \ --tests '*IT'" diff --git a/src/main/java/org/opensearch/flowframework/model/Template.java b/src/main/java/org/opensearch/flowframework/model/Template.java index 7a9f62f7e..1086cda32 100644 --- a/src/main/java/org/opensearch/flowframework/model/Template.java +++ b/src/main/java/org/opensearch/flowframework/model/Template.java @@ -582,6 +582,7 @@ public static Template parse(XContentParser parser, boolean fieldUpdate) throws .lastUpdatedTime(lastUpdatedTime) .lastProvisionedTime(lastProvisionedTime) .tenantId(tenantId) + .allSharedPrincipals(allSharedPrincipals) .build(); return template; } diff --git a/src/main/java/org/opensearch/flowframework/util/PluginClient.java b/src/main/java/org/opensearch/flowframework/util/PluginClient.java index 3c977fe53..641c612a6 100644 --- a/src/main/java/org/opensearch/flowframework/util/PluginClient.java +++ b/src/main/java/org/opensearch/flowframework/util/PluginClient.java @@ -50,7 +50,7 @@ protected void if (subject == null) { throw new IllegalStateException("PluginClient is not initialized."); } - try (ThreadContext.StoredContext ctx = threadPool().getThreadContext().newStoredContext(false)) { + try (ThreadContext.StoredContext ctx = threadPool().getThreadContext().stashContext()) { subject.runAs(() -> { logger.info("Running transport action with subject: {}", subject.getPrincipal().getName()); super.doExecute(action, request, ActionListener.runBefore(listener, ctx::restore)); diff --git a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java index 9848319a1..3344b716d 100644 --- a/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java +++ b/src/main/java/org/opensearch/flowframework/util/ResourceSharingClientAccessor.java @@ -18,7 +18,7 @@ public class ResourceSharingClientAccessor { private final AtomicReference client = new AtomicReference<>(); - private static ResourceSharingClientAccessor resourceSharingClientAccessor; + private static final ResourceSharingClientAccessor INSTANCE = new ResourceSharingClientAccessor(); private ResourceSharingClientAccessor() {} @@ -27,11 +27,7 @@ private ResourceSharingClientAccessor() {} * @return the singleton instance */ public static ResourceSharingClientAccessor getInstance() { - if (resourceSharingClientAccessor == null) { - resourceSharingClientAccessor = new ResourceSharingClientAccessor(); - } - - return resourceSharingClientAccessor; + return INSTANCE; } /** @@ -39,7 +35,7 @@ public static ResourceSharingClientAccessor getInstance() { * @param client the resource sharing client to set */ public void setResourceSharingClient(ResourceSharingClient client) { - resourceSharingClientAccessor.client.set(client); + this.client.set(client); } /** @@ -47,6 +43,6 @@ public void setResourceSharingClient(ResourceSharingClient client) { * @return the resource sharing client */ public ResourceSharingClient getResourceSharingClient() { - return resourceSharingClientAccessor.client.get(); + return this.client.get(); } } diff --git a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java index b2c3751c5..176332564 100644 --- a/src/test/java/org/opensearch/flowframework/model/TemplateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/TemplateTests.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -205,4 +206,38 @@ public void testCreateEmptyTemplateWithTenantId_NullTenantId() { Template t = Template.createEmptyTemplateWithTenantId(null); assertNull(t); } + + public void testUpdateExistingTemplateAllSharedPrincipals() { + // Time travel to guarantee update increments + Instant now = Instant.now().minusMillis(100); + + Template original = new Template( + "name one", + "description one", + "use case one", + Version.fromString("1.1.1"), + List.of(Version.fromString("1.1.1"), Version.fromString("1.1.1")), + Collections.emptyMap(), + Map.of("uiMetadata", "one"), + null, + now, + now, + null, + null, + new ArrayList<>(List.of("user:one")) + ); + + Template updated = Template.builder().allSharedPrincipals(List.of("user:two")).build(); + + Template merged = Template.updateExistingTemplate(original, updated); + + // Ensure both principals are present in the merged template + List mergedPrincipals = merged.allSharedPrincipals(); + assertEquals(2, mergedPrincipals.size()); + assertTrue(mergedPrincipals.contains("user:one")); + assertTrue(mergedPrincipals.contains("user:two")); + + // Ensure lastUpdatedTime was bumped + assertTrue(merged.lastUpdatedTime().isAfter(now)); + } } diff --git a/src/test/java/org/opensearch/flowframework/model/WorkflowStateTests.java b/src/test/java/org/opensearch/flowframework/model/WorkflowStateTests.java index ca7873036..43928f2b7 100644 --- a/src/test/java/org/opensearch/flowframework/model/WorkflowStateTests.java +++ b/src/test/java/org/opensearch/flowframework/model/WorkflowStateTests.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.time.Instant; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; @@ -163,4 +164,27 @@ public void testWorkflowStateUpdate() { rc = wfs.resourcesCreated().get(1); assertEquals("id two", rc.resourceId()); } + + public void testUpdateExistingWorkflowStateAllSharedPrincipals() { + // original state with one principal, use a mutable list + WorkflowState original = WorkflowState.builder() + .workflowId("wf-1") + .allSharedPrincipals(new ArrayList<>(List.of("user:one"))) + .build(); + + assertNotNull(original.getAllSharedPrincipals()); + assertEquals(1, original.getAllSharedPrincipals().size()); + assertTrue(original.getAllSharedPrincipals().contains("user:one")); + + // update state with another principal + WorkflowState update = WorkflowState.builder().allSharedPrincipals(List.of("user:two")).build(); + + WorkflowState merged = WorkflowState.updateExistingWorkflowState(original, update); + + // verify merged principals contain both + assertNotNull(merged.getAllSharedPrincipals()); + assertEquals(2, merged.getAllSharedPrincipals().size()); + assertTrue(merged.getAllSharedPrincipals().contains("user:one")); + assertTrue(merged.getAllSharedPrincipals().contains("user:two")); + } } diff --git a/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java b/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java index a36042232..052e1ea07 100644 --- a/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/ReprovisionWorkflowRequestTests.java @@ -14,6 +14,7 @@ import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.BytesStreamInput; import org.opensearch.flowframework.TestHelpers; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.model.Template; import org.opensearch.flowframework.model.Workflow; import org.opensearch.flowframework.model.WorkflowEdge; @@ -89,4 +90,13 @@ public void testReprovisionWorkflowRequest() throws IOException { assertEquals(request.getUpdatedTemplate().toJson(), requestFromStreamInput.getUpdatedTemplate().toJson()); } + public void testIndexIdAndTypeAndTimeout() { + TimeValue timeout = TimeValue.timeValueSeconds(10); + ReprovisionWorkflowRequest request = new ReprovisionWorkflowRequest("123", originalTemplate, updatedTemplate, timeout); + + assertEquals(CommonValue.GLOBAL_CONTEXT_INDEX, request.index()); + assertEquals("123", request.id()); + assertEquals(CommonValue.WORKFLOW_RESOURCE_TYPE, request.type()); + assertEquals(timeout, request.getWaitForCompletionTimeout()); + } } diff --git a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java index 5eb18ecd5..80f9ad874 100644 --- a/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java +++ b/src/test/java/org/opensearch/flowframework/transport/WorkflowRequestResponseTests.java @@ -10,6 +10,7 @@ import org.opensearch.Version; import org.opensearch.common.io.stream.BytesStreamOutput; +import org.opensearch.common.unit.TimeValue; import org.opensearch.common.xcontent.XContentType; import org.opensearch.core.common.bytes.BytesReference; import org.opensearch.core.common.io.stream.BytesStreamInput; @@ -17,6 +18,7 @@ import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.flowframework.TestHelpers; +import org.opensearch.flowframework.common.CommonValue; import org.opensearch.flowframework.model.Template; import org.opensearch.flowframework.model.Workflow; import org.opensearch.flowframework.model.WorkflowEdge; @@ -248,4 +250,45 @@ public void testWorkflowResponseWithWaitForCompletionTimeOut() throws IOExceptio assertTrue(builder.toString().contains("\"state\":\"PROVISIONING\"")); } + public void testWorkflowRequestIndexIdTypeAndTimeout() throws IOException { + TimeValue timeout = TimeValue.timeValueSeconds(15); + + WorkflowRequest request = new WorkflowRequest("123", template, Map.of("foo", "bar"), timeout); + + // Basic assertions + assertEquals("123", request.getWorkflowId()); + assertEquals(template, request.getTemplate()); + assertNull(request.validate()); + assertTrue(request.isProvision()); + assertFalse(request.isUpdateFields()); + assertFalse(request.isReprovision()); + assertEquals("bar", request.getParams().get("foo")); + assertEquals(timeout, request.getWaitForCompletionTimeout()); + + // index/id/type metadata + assertEquals(CommonValue.GLOBAL_CONTEXT_INDEX, request.index()); + assertEquals("123", request.id()); + assertEquals(CommonValue.WORKFLOW_RESOURCE_TYPE, request.type()); + + // Round-trip through StreamInput/StreamOutput + BytesStreamOutput out = new BytesStreamOutput(); + request.writeTo(out); + BytesStreamInput in = new BytesStreamInput(BytesReference.toBytes(out.bytes())); + + WorkflowRequest requestFromStream = new WorkflowRequest(in); + + // Ensure metadata & timeout survive serialization + assertEquals(request.getWorkflowId(), requestFromStream.getWorkflowId()); + assertEquals(request.getTemplate().toString(), requestFromStream.getTemplate().toString()); + assertTrue(requestFromStream.isProvision()); + assertFalse(requestFromStream.isUpdateFields()); + assertFalse(requestFromStream.isReprovision()); + assertEquals("bar", requestFromStream.getParams().get("foo")); + assertEquals(timeout, requestFromStream.getWaitForCompletionTimeout()); + assertEquals(CommonValue.GLOBAL_CONTEXT_INDEX, requestFromStream.index()); + assertEquals("123", requestFromStream.id()); + assertEquals(CommonValue.WORKFLOW_RESOURCE_TYPE, requestFromStream.type()); + + } + } From f65076dccea66a1a6b3b568a00b04ca0b01bec3a Mon Sep 17 00:00:00 2001 From: Daniel Widdis Date: Thu, 13 Nov 2025 11:06:37 -0800 Subject: [PATCH 23/23] Fix test to wait for async state deletion attempt Signed-off-by: Daniel Widdis --- .../rest/FlowFrameworkSecureRestApiIT.java | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index 0bc0f925e..a2cbbf68a 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -431,17 +431,20 @@ public void testCreateProvisionDeprovisionWorkflowWithFullAccess() throws Except response = deleteWorkflow(aliceClient, workflowId); assertEquals(RestStatus.OK, TestHelpers.restStatus(response)); - // Invoke status API with failure - ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(aliceClient, workflowId, false)); - if (isResourceSharingFeatureEnabled()) { - // sample log message: No sharing info found for 'pmaAdZoBL3PmSyJoha0C'. Action - // cluster:admin/opensearch/flow_framework/workflow_state/get is not allowed. - // since record is deleted corresponding sharing record will also be deleted and thus we show 403 for non-existent sharing - // records, - assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); - } else { - assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); - } + // Wait for state doc to be deleted asynchronously + assertBusy(() -> { + // Invoke status API with failure + ResponseException exception = expectThrows(ResponseException.class, () -> getWorkflowStatus(aliceClient, workflowId, false)); + if (isResourceSharingFeatureEnabled()) { + // sample log message: No sharing info found for 'pmaAdZoBL3PmSyJoha0C'. Action + // cluster:admin/opensearch/flow_framework/workflow_state/get is not allowed. + // since record is deleted corresponding sharing record will also be deleted and thus we show 403 for non-existent sharing + // records, + assertEquals(RestStatus.FORBIDDEN.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + } else { + assertEquals(RestStatus.NOT_FOUND.getStatus(), exception.getResponse().getStatusLine().getStatusCode()); + } + }, 30, TimeUnit.SECONDS); } public void testUpdateWorkflowEnabledForAdmin() throws Exception {