Addressed additional PR Comments

Signed-off-by: Owais <[email protected]>
opensearch-project · Aug 23, 2024 · 7f3dbdc · 7f3dbdc
1 parent e5a4c74
commit 7f3dbdc
Show file tree

Hide file tree

Showing 3 changed files with 5 additions and 6 deletions.
diff --git a/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java b/src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
@@ -70,7 +70,6 @@ public class CreateWorkflowTransportAction extends HandledTransportAction<Workfl
     private final FlowFrameworkSettings flowFrameworkSettings;
     private final PluginsService pluginsService;
     private volatile Boolean filterByEnabled;
-    private final Settings settings;
     private final ClusterService clusterService;
     private final NamedXContentRegistry xContentRegistry;
 
@@ -110,7 +109,6 @@ public CreateWorkflowTransportAction(
         this.clusterService = clusterService;
         clusterService.getClusterSettings().addSettingsUpdateConsumer(FILTER_BY_BACKEND_ROLES, it -> filterByEnabled = it);
         this.xContentRegistry = xContentRegistry;
-        this.settings = settings;
     }
 
     @Override
@@ -141,7 +139,7 @@ private void resolveUserAndExecute(
         try {
             // Check if user has backend roles
             // When filter by is enabled, block users creating/updating workflows who do not have backend roles.
-            if (filterByEnabled) {
+            if (filterByEnabled == Boolean.TRUE) {
                 try {
                     checkFilterByBackendRoles(requestedUser);
                 } catch (FlowFrameworkException e) {

diff --git a/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java b/src/main/java/org/opensearch/flowframework/transport/handler/SearchHandler.java
@@ -88,7 +88,7 @@ public void validateRole(
             // Security is enabled, filter is enabled and user isn't admin
             try {
                 ParseUtils.addUserBackendRolesFilter(user, request.source());
-                logger.debug("Filtering result by " + user.getBackendRoles());
+                logger.debug("Filtering result by {}", user.getBackendRoles());
                 client.search(request, ActionListener.runBefore(listener, context::restore));
             } catch (Exception e) {
                 listener.onFailure(e);

diff --git a/src/main/java/org/opensearch/flowframework/util/ParseUtils.java b/src/main/java/org/opensearch/flowframework/util/ParseUtils.java
@@ -27,6 +27,7 @@
 import org.opensearch.commons.ConfigConstants;
 import org.opensearch.commons.authuser.User;
 import org.opensearch.core.action.ActionListener;
+import org.opensearch.core.action.ActionResponse;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.rest.RestStatus;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
@@ -292,14 +293,14 @@ public static void resolveUserAndExecute(
         User requestedUser,
         String workflowId,
         Boolean filterByEnabled,
-        ActionListener listener,
+        ActionListener<? extends ActionResponse> listener,
         Runnable function,
         Client client,
         ClusterService clusterService,
         NamedXContentRegistry xContentRegistry
     ) {
         try {
-            if (requestedUser == null || !filterByEnabled) {
+            if (requestedUser == null || filterByEnabled == Boolean.FALSE) {
                 // 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 workflow or not.
                 // !filterByEnabled means security is enabled and filterByEnabled is disabled