Refactored workflowIndexing validation - removed coroutine and contex… (

#857) * Refactored workflowIndexing validation - removed coroutine and context client context lost Signed-off-by: Stevan Buzejic <[email protected]> * refactored getting the workflows Signed-off-by: Stevan Buzejic <[email protected]> * Changed the logic according to secure test findings Signed-off-by: Stevan Buzejic <[email protected]> * [Backport 2.x] Notification security fix (#861) (#863) * Notification security fix (#852) * added injecting whole user object in threadContext before calling notification APIs so that backend roles are available to notification plugin * compile fix * refactored user_info injection to use InjectSecurity * ktlint fix --------- (cherry picked from commit e0b7a5a) * remove unneeded import --------- Signed-off-by: Ashish Agrawal <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Petar Dzepina <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]> * Stashed user together with it's roles Signed-off-by: Stevan Buzejic <[email protected]> --------- Signed-off-by: Stevan Buzejic <[email protected]> Signed-off-by: Ashish Agrawal <[email protected]> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: Petar Dzepina <[email protected]> Co-authored-by: Ashish Agrawal <[email protected]>
opensearch-project · May 24, 2023 · 37987b7 · 37987b7
1 parent 8b6dae6
commit 37987b7
Show file tree

Hide file tree

Showing 3 changed files with 238 additions and 131 deletions.
diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteWorkflowAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteWorkflowAction.kt
@@ -22,8 +22,10 @@ import org.opensearch.action.search.SearchResponse
 import org.opensearch.action.support.ActionFilters
 import org.opensearch.action.support.HandledTransportAction
 import org.opensearch.action.support.WriteRequest.RefreshPolicy
+import org.opensearch.alerting.opensearchapi.InjectorContextElement
 import org.opensearch.alerting.opensearchapi.addFilter
 import org.opensearch.alerting.opensearchapi.suspendUntil
+import org.opensearch.alerting.opensearchapi.withClosableContext
 import org.opensearch.alerting.settings.AlertingSettings
 import org.opensearch.alerting.util.AlertingException
 import org.opensearch.client.Client
@@ -53,6 +55,7 @@ import org.opensearch.rest.RestStatus
 import org.opensearch.search.builder.SearchSourceBuilder
 import org.opensearch.tasks.Task
 import org.opensearch.transport.TransportService
+import java.util.UUID
 
 private val scope: CoroutineScope = CoroutineScope(Dispatchers.IO)
 /**
@@ -64,7 +67,7 @@ class TransportDeleteWorkflowAction @Inject constructor(
     val client: Client,
     actionFilters: ActionFilters,
     val clusterService: ClusterService,
-    settings: Settings,
+    val settings: Settings,
     val xContentRegistry: NamedXContentRegistry
 ) : HandledTransportAction<ActionRequest, DeleteWorkflowResponse>(
     AlertingActions.DELETE_WORKFLOW_ACTION_NAME, transportService, actionFilters, ::DeleteWorkflowRequest
@@ -149,7 +152,22 @@ class TransportDeleteWorkflowAction @Inject constructor(
 
                     val deleteResponse = deleteWorkflow(workflow)
                     if (deleteDelegateMonitors == true) {
-                        deleteMonitors(delegateMonitorIds, RefreshPolicy.IMMEDIATE)
+                        if (user == null) {
+                            deleteMonitors(delegateMonitorIds, RefreshPolicy.IMMEDIATE)
+                        } else {
+                            // Un-stash the context
+                            withClosableContext(
+                                InjectorContextElement(
+                                    user.name.plus(UUID.randomUUID().toString()),
+                                    settings,
+                                    client.threadPool().threadContext,
+                                    user.roles,
+                                    user
+                                )
+                            ) {
+                                deleteMonitors(delegateMonitorIds, RefreshPolicy.IMMEDIATE)
+                            }
+                        }
                     }
                     actionListener.onResponse(DeleteWorkflowResponse(deleteResponse.id, deleteResponse.version))
                 } else {

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetWorkflowAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetWorkflowAction.kt
@@ -85,7 +85,21 @@ class TransportGetWorkflowAction @Inject constructor(
                                 xContentRegistry, LoggingDeprecationHandler.INSTANCE,
                                 response.sourceAsBytesRef, XContentType.JSON
                             ).use { xcp ->
-                                workflow = ScheduledJob.parse(xcp, response.id, response.version) as Workflow
+                                val compositeMonitor = ScheduledJob.parse(xcp, response.id, response.version)
+                                if (compositeMonitor is Workflow) {
+                                    workflow = compositeMonitor
+                                } else {
+                                    log.error("Wrong monitor type returned")
+                                    actionListener.onFailure(
+                                        AlertingException.wrap(
+                                            OpenSearchStatusException(
+                                                "Workflow not found.",
+                                                RestStatus.NOT_FOUND
+                                            )
+                                        )
+                                    )
+                                    return
+                                }
 
                                 // security is enabled and filterby is enabled
                                 if (!checkUserPermissionsWithResource(