diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java index 1ec557fd..3750065c 100644 --- a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java +++ b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java @@ -162,7 +162,12 @@ private void resolveUserAndExecute( function.execute(); } } catch (Exception e) { - listener.onFailure(e); + String errorMessage = "Failed to create or update workflow"; + if (e instanceof FlowFrameworkException) { + listener.onFailure(e); + } else { + listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e))); + } } } diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java index 7dfa5ec8..f052a4b8 100644 --- a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java +++ b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java @@ -301,7 +301,7 @@ public static void resolveUserAndExecute( try { if (requestedUser == null || !filterByEnabled) { // requestedUser == null means security is disabled or user is superadmin. In this case we don't need to - // check if request user have access to the detector or not. + // check if request user have access to the workflow or not. // !filterByEnabled means security is enabled and filterByEnabled is disabled function.execute(); } else { diff --git a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java index e379f6b0..051845ef 100644 --- a/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java +++ b/src/test/java/org/opensearch/flowframework/rest/FlowFrameworkSecureRestApiIT.java @@ -50,8 +50,6 @@ public class FlowFrameworkSecureRestApiIT extends FlowFrameworkRestTestCase { RestClient elkClient; String fishUser = "fish"; RestClient fishClient; - String goatUser = "goat"; - RestClient goatClient; String lionUser = "lion"; RestClient lionClient; private String indexAllAccessRole = "index_all_access"; @@ -63,8 +61,6 @@ public class FlowFrameworkSecureRestApiIT extends FlowFrameworkRestTestCase { @Before public void setupSecureTests() throws IOException { if (!isHttps()) throw new IllegalArgumentException("Secure Tests are running but HTTPS is not set"); - createIndexRole(indexAllAccessRole, "*"); - createSearchRole(indexSearchAccessRole, "*"); String alicePassword = generatePassword(aliceUser); createUser(aliceUser, alicePassword, List.of("odfe")); aliceClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), aliceUser, alicePassword) @@ -101,12 +97,6 @@ public void setupSecureTests() throws IOException { .setSocketTimeout(60000) .build(); - String goatPassword = generatePassword(goatUser); - createUser(goatUser, goatPassword, List.of("opensearch")); - goatClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), goatUser, goatPassword) - .setSocketTimeout(60000) - .build(); - String lionPassword = generatePassword(lionUser); createUser(lionUser, lionPassword, List.of("opensearch")); lionClient = new SecureRestClientBuilder(getClusterHosts().toArray(new HttpHost[0]), isHttps(), lionUser, lionPassword) @@ -114,10 +104,8 @@ public void setupSecureTests() throws IOException { .build(); createRoleMapping(FLOW_FRAMEWORK_READ_ACCESS_ROLE, List.of(bobUser)); - createRoleMapping(ML_COMMONS_FULL_ACCESS_ROLE, List.of(aliceUser, catUser, dogUser, elkUser, fishUser, goatUser)); - createRoleMapping(FLOW_FRAMEWORK_FULL_ACCESS_ROLE, List.of(aliceUser, catUser, dogUser, elkUser, fishUser, goatUser)); - createRoleMapping(indexAllAccessRole, List.of(aliceUser, bobUser, catUser, dogUser, fishUser, lionUser)); - createRoleMapping(indexSearchAccessRole, List.of(goatUser)); + 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)); } @After @@ -128,7 +116,6 @@ public void tearDownSecureTests() throws IOException { dogClient.close(); elkClient.close(); fishClient.close(); - goatClient.close(); lionClient.close(); deleteUser(aliceUser); deleteUser(bobUser); @@ -136,7 +123,6 @@ public void tearDownSecureTests() throws IOException { deleteUser(dogUser); deleteUser(elkUser); deleteUser(fishUser); - deleteUser(goatUser); deleteUser(lionUser); } @@ -486,7 +472,7 @@ public void testUpdateWorkflowWithNoFFAccess() throws Exception { enableFilterBy(); - // User elk has FF full access, but has no read permission of index + // User lion has no FF access and should not be able to update the workflow created by alice ResponseException exception1 = expectThrows(ResponseException.class, () -> { updateWorkflow(lionClient, workflowId, template); }); assertEquals(RestStatus.FORBIDDEN.getStatus(), exception1.getResponse().getStatusLine().getStatusCode()); }