From 99d432943ba3b09111ff12046272d24dda3c6b08 Mon Sep 17 00:00:00 2001
From: Subhobrata Dey <sbcd90@gmail.com>
Date: Wed, 20 Dec 2023 21:49:40 +0000
Subject: [PATCH] acknowledge api hangs when
 plugins.alerting.filter_by_backend_roles is enabled

Signed-off-by: Subhobrata Dey <sbcd90@gmail.com>
---
 .../TransportAcknowledgeAlertAction.kt        | 22 ++++++++++++++-----
 .../resthandler/SecureMonitorRestApiIT.kt     | 18 +++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt
index aa65c6826..b816702d1 100644
--- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt
+++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportAcknowledgeAlertAction.kt
@@ -20,7 +20,9 @@ import org.opensearch.action.search.SearchResponse
 import org.opensearch.action.support.ActionFilters
 import org.opensearch.action.support.HandledTransportAction
 import org.opensearch.action.update.UpdateRequest
+import org.opensearch.alerting.opensearchapi.InjectorContextElement
 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.alerting.util.use
@@ -66,12 +68,19 @@ class TransportAcknowledgeAlertAction @Inject constructor(
     val xContentRegistry: NamedXContentRegistry,
     val transportGetMonitorAction: TransportGetMonitorAction
 ) : HandledTransportAction<ActionRequest, AcknowledgeAlertResponse>(
-    AlertingActions.ACKNOWLEDGE_ALERTS_ACTION_NAME, transportService, actionFilters, ::AcknowledgeAlertRequest
-) {
+    AlertingActions.ACKNOWLEDGE_ALERTS_ACTION_NAME,
+    transportService,
+    actionFilters,
+    ::AcknowledgeAlertRequest
+),
+    SecureTransportAction {
 
     @Volatile
     private var isAlertHistoryEnabled = AlertingSettings.ALERT_HISTORY_ENABLED.get(settings)
 
+    @Volatile
+    override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings)
+
     init {
         clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.ALERT_HISTORY_ENABLED) { isAlertHistoryEnabled = it }
     }
@@ -83,8 +92,9 @@ class TransportAcknowledgeAlertAction @Inject constructor(
     ) {
         val request = acknowledgeAlertRequest as? AcknowledgeAlertRequest
             ?: recreateObject(acknowledgeAlertRequest) { AcknowledgeAlertRequest(it) }
-        client.threadPool().threadContext.stashContext().use {
-            scope.launch {
+        val user = readUserFromThreadContext(client)
+        scope.launch {
+            withClosableContext(InjectorContextElement(request.monitorId, settings, client.threadPool().threadContext, user?.roles)) {
                 val getMonitorResponse: GetMonitorResponse =
                     transportGetMonitorAction.client.suspendUntil {
                         val getMonitorRequest = GetMonitorRequest(
@@ -108,7 +118,9 @@ class TransportAcknowledgeAlertAction @Inject constructor(
                         )
                     )
                 } else {
-                    AcknowledgeHandler(client, actionListener, request).start(getMonitorResponse.monitor!!)
+                    client.threadPool().threadContext.stashContext().use {
+                        AcknowledgeHandler(client, actionListener, request).start(getMonitorResponse.monitor!!)
+                    }
                 }
             }
         }
diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt
index 79959c45a..6557ae68a 100644
--- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt
+++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt
@@ -1274,6 +1274,24 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
         }
     }
 
+    fun `test ack alert with enabled filter by`() {
+        enableFilterBy()
+        putAlertMappings()
+        val adminUser = User(ADMIN, listOf(ADMIN), listOf(ALL_ACCESS_ROLE), listOf())
+        val monitor = createRandomMonitor(refresh = true).copy(user = adminUser)
+        val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE))
+
+        val inputMap = HashMap<String, Any>()
+        inputMap["missing"] = "_last"
+        inputMap["monitorId"] = monitor.id
+
+        // search as "admin" - must get 4 docs
+        val adminResponseMap = getAlerts(client(), inputMap).asMap()
+        assertEquals(1, adminResponseMap["totalAlerts"])
+
+        acknowledgeAlerts(monitor, alert)
+    }
+
     fun `test get alerts with an user with get alerts role`() {
 
         putAlertMappings()