AddPolicyAction - Recursion removal

src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/addpolicy/TransportAddPolicyAction.kt

@@ -32,9 +32,9 @@ import org.opensearch.cluster.service.ClusterService
 import org.opensearch.common.inject.Inject
 import org.opensearch.common.settings.Settings
 import org.opensearch.common.unit.TimeValue
-import org.opensearch.core.xcontent.NamedXContentRegistry
 import org.opensearch.commons.ConfigConstants
 import org.opensearch.commons.authuser.User
+import org.opensearch.core.xcontent.NamedXContentRegistry
 import org.opensearch.index.Index
 import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.INDEX_MANAGEMENT_INDEX
 import org.opensearch.indexmanagement.indexstatemanagement.DefaultIndexMetadataService
@@ -50,7 +50,10 @@ import org.opensearch.indexmanagement.indexstatemanagement.util.DEFAULT_INDEX_TY
 import org.opensearch.indexmanagement.indexstatemanagement.util.FailedIndex
 import org.opensearch.indexmanagement.indexstatemanagement.util.managedIndexConfigIndexRequest
 import org.opensearch.indexmanagement.indexstatemanagement.util.removeClusterStateMetadatas
+import org.opensearch.indexmanagement.opensearchapi.IndexManagementSecurityContext
 import org.opensearch.indexmanagement.opensearchapi.parseFromGetResponse
+import org.opensearch.indexmanagement.opensearchapi.suspendUntil
+import org.opensearch.indexmanagement.opensearchapi.withClosableContext
 import org.opensearch.indexmanagement.settings.IndexManagementSettings
 import org.opensearch.indexmanagement.spi.indexstatemanagement.model.ISMIndexMetadata
 import org.opensearch.indexmanagement.util.IndexUtils
@@ -60,8 +63,6 @@ import org.opensearch.indexmanagement.util.SecurityUtils.Companion.validateUserC
 import org.opensearch.rest.RestStatus
 import org.opensearch.tasks.Task
 import org.opensearch.transport.TransportService
-import java.lang.Exception
-import java.lang.IllegalArgumentException
 import java.time.Duration
 import java.time.Instant
 
@@ -105,11 +106,10 @@ class TransportAddPolicyAction @Inject constructor(
         private val client: NodeClient,
         private val actionListener: ActionListener<ISMStatusResponse>,
         private val request: AddPolicyRequest,
-        private val user: User? = buildUser(client.threadPool().threadContext)
+        private val user: User? = buildUser(client.threadPool().threadContext),
     ) {
         private lateinit var startTime: Instant
         private lateinit var policy: Policy
-        private val permittedIndices = mutableListOf<String>()
         private val indicesToAdd = mutableMapOf<String, String>() // uuid: name
         private val failedIndices: MutableList<FailedIndex> = mutableListOf()
 
@@ -145,55 +145,35 @@ class TransportAddPolicyAction @Inject constructor(
                     return@launch
                 }
                 if (user != null) {
-                    validateIndexPermissions(0, indicesToAdd.values.toList())
-                } else {
-                    removeClosedIndices()
+                    withClosableContext(IndexManagementSecurityContext("AddPolicyHandler", settings, client.threadPool().threadContext, user)) {
+                        validateIndexPermissions(indicesToAdd.values.toList())
+                    }
                 }
+                removeClosedIndices()
             }
         }
 
         /**
          * We filter the requested indices to the indices user has permission to manage and apply policies only on top of those
          */
-        private fun validateIndexPermissions(current: Int, indices: List<String>) {
-            val request = ManagedIndexRequest().indices(indices[current])
-            client.execute(
-                ManagedIndexAction.INSTANCE,
-                request,
-                object : ActionListener<AcknowledgedResponse> {
-                    override fun onResponse(response: AcknowledgedResponse) {
-                        permittedIndices.add(indices[current])
-                        proceed(current, indices)
-                    }
-
-                    override fun onFailure(e: Exception) {
-                        when (e is OpenSearchSecurityException) {
-                            true -> {
-                                proceed(current, indices)
-                            }
-                            false -> {
-                                // failing the request for any other exception
-                                actionListener.onFailure(e)
-                            }
-                        }
-                    }
+        private suspend fun validateIndexPermissions(indices: List<String>) {
+            val permittedIndices = mutableListOf<String>()
+            indices.forEach { index ->
+                try {
+                    client.suspendUntil { execute(ManagedIndexAction.INSTANCE, ManagedIndexRequest().indices(index), it) }
+                    permittedIndices.add(index)
+                } catch (e: OpenSearchSecurityException) {
+                    log.debug("No permissions for index [$index]")
                 }
-            )
-        }
+            }
 
-        private fun proceed(current: Int, indices: List<String>) {
-            if (current < indices.count() - 1) {
-                validateIndexPermissions(current + 1, indices)
-            } else {
-                // sanity check that there are indices - if none then return
-                if (permittedIndices.isEmpty()) {
-                    actionListener.onResponse(ISMStatusResponse(0, failedIndices))
-                    return
-                }
-                // Filter out the indices that the user does not have permissions for
-                indicesToAdd.values.removeIf { !permittedIndices.contains(it) }
-                removeClosedIndices()
+            // sanity check that there are indices - if none then return
+            if (permittedIndices.isEmpty()) {
+                actionListener.onResponse(ISMStatusResponse(0, failedIndices))
+                return
             }
+            // Filter out the indices that the user does not have permissions for
+            indicesToAdd.values.removeIf { !permittedIndices.contains(it) }
         }
 
         private fun removeClosedIndices() {

src/main/kotlin/org/opensearch/indexmanagement/opensearchapi/OpenSearchExtensions.kt

@@ -30,20 +30,20 @@ import org.opensearch.common.settings.Settings
 import org.opensearch.common.unit.TimeValue
 import org.opensearch.common.util.concurrent.ThreadContext
 import org.opensearch.common.xcontent.LoggingDeprecationHandler
-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.common.xcontent.XContentFactory
 import org.opensearch.common.xcontent.XContentHelper
-import org.opensearch.core.xcontent.XContentParser
-import org.opensearch.core.xcontent.XContentParser.Token
 import org.opensearch.common.xcontent.XContentParserUtils
 import org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken
 import org.opensearch.common.xcontent.XContentType
 import org.opensearch.commons.InjectSecurity
 import org.opensearch.commons.authuser.User
 import org.opensearch.commons.notifications.NotificationsPluginInterface
+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.core.xcontent.XContentParser.Token
 import org.opensearch.index.seqno.SequenceNumbers
 import org.opensearch.indexmanagement.indexstatemanagement.action.ShrinkAction
 import org.opensearch.indexmanagement.indexstatemanagement.model.ISMTemplate

src/test/kotlin/org/opensearch/indexmanagement/PolicySecurityBehaviorIT.kt

@@ -0,0 +1,124 @@
+/*
+ * 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.
+ *
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.indexmanagement
+
+import org.junit.After
+import org.junit.Before
+import org.opensearch.action.admin.indices.alias.IndicesAliasesRequest
+import org.opensearch.client.ResponseException
+import org.opensearch.client.RestClient
+import org.opensearch.commons.rest.SecureRestClientBuilder
+import org.opensearch.indexmanagement.IndexManagementPlugin.Companion.INDEX_MANAGEMENT_INDEX
+import org.opensearch.indexmanagement.indexstatemanagement.action.AliasAction
+import org.opensearch.indexmanagement.indexstatemanagement.model.Policy
+import org.opensearch.indexmanagement.indexstatemanagement.model.State
+import org.opensearch.indexmanagement.indexstatemanagement.randomErrorNotification
+import org.opensearch.indexmanagement.indexstatemanagement.transport.action.addpolicy.AddPolicyAction
+import org.opensearch.rest.RestRequest
+import org.opensearch.rest.RestStatus
+import org.opensearch.test.OpenSearchTestCase
+import org.opensearch.test.junit.annotations.TestLogging
+import java.time.Instant
+import java.time.temporal.ChronoUnit
+import java.util.Locale
+
+@TestLogging("level:DEBUG", reason = "Debug for tests.")
+class PolicySecurityBehaviorIT : SecurityRestTestCase() {
+    private val password = "Test123!"
+
+    private val ismUser = "john"
+    private var ismUserClient: RestClient? = null
+
+    private val permittedIndicesPrefix = "permitted-index"
+    private val permittedIndicesPattern = "permitted-index*"
+    @Before
+    fun setupUsersAndRoles() {
+//        updateClusterSetting(ManagedIndexSettings.JITTER.key, "0.0", false)
+
+        val custerPermissions = listOf(
+            AddPolicyAction.NAME
+        )
+
+        val indexPermissions = listOf(
+            MANAGED_INDEX,
+            CREATE_INDEX,
+            WRITE_INDEX,
+            BULK_WRITE_INDEX,
+            GET_INDEX_MAPPING,
+            SEARCH_INDEX,
+            PUT_INDEX_MAPPING
+        )
+        createUser(ismUser, password, listOf(HELPDESK))
+        createRole(HELPDESK_ROLE, custerPermissions, indexPermissions, listOf(permittedIndicesPattern))
+        assignRoleToUsers(HELPDESK_ROLE, listOf(ismUser))
+
+        ismUserClient =
+            SecureRestClientBuilder(clusterHosts.toTypedArray(), isHttps(), ismUser, password).setSocketTimeout(60000)
+                .build()
+    }
+
+    @After
+    fun cleanup() {
+        // Remove user
+        ismUserClient?.close()
+        deleteUser(ismUser)
+        deleteRole(HELPDESK_ROLE)
+
+        deleteIndexByName("$INDEX_MANAGEMENT_INDEX")
+    }
+
+    fun `test add policy`() {
+
+        val notPermittedIndexPrefix = OpenSearchTestCase.randomAlphaOfLength(10).lowercase(Locale.getDefault())
+        val policyId = OpenSearchTestCase.randomAlphaOfLength(10)
+
+        val permittedindices = mutableListOf<String>()
+        val notPermittedindices = mutableListOf<String>()
+        for (i in 1..5) {
+            createIndex("$notPermittedIndexPrefix-$i", """ "properties": { "field_a": { "type": "long" } }""", client())
+            createIndex("$permittedIndicesPrefix-$i", """ "properties": { "field_a": { "type": "long" } }""", client())
+            notPermittedindices += "$notPermittedIndexPrefix-$i"
+            permittedindices += "$permittedIndicesPrefix-$i"
+        }
+
+        val allIndicesJoined = (notPermittedindices + permittedindices).joinToString(separator = ",")
+        try {
+            val actions = listOf(IndicesAliasesRequest.AliasActions.add().alias("aaa"))
+            val actionConfig = AliasAction(actions = actions, index = 0)
+            val states = listOf(State("alias", listOf(actionConfig), listOf()))
+            val policy = Policy(
+                id = policyId,
+                description = "description",
+                schemaVersion = 1L,
+                lastUpdatedTime = Instant.now().truncatedTo(ChronoUnit.MILLIS),
+                errorNotification = randomErrorNotification(),
+                defaultState = "alias",
+                states = states
+            )
+            createPolicy(policy, policy.id, true, client())
+            // Call AddPolicyAction as user
+            addPolicyToIndex(index = allIndicesJoined, policyId = policy.id, expectedStatus = RestStatus.OK, client = ismUserClient!!)
+
+            refreshAllIndices()
+
+            val searchResponse = responseAsMap(client().makeRequest(RestRequest.Method.GET.toString(), "$INDEX_MANAGEMENT_INDEX/_search?size=1000"))
+            val numOfHits = ((searchResponse["hits"] as Map<*, *>)["total"] as Map<*, *>)["value"] as Int
+            // 1 Policy document + 5 ManagedIndex docs
+            assertEquals(1 + 5, numOfHits)
+        } catch (e: ResponseException) {
+            logger.error(e.message, e)
+        } finally {
+            deleteIndexByName("$permittedIndicesPrefix*")
+            deleteIndexByName("$notPermittedIndexPrefix*")
+        }
+    }
+}

src/test/kotlin/org/opensearch/indexmanagement/SecurityRestTestCase.kt

@@ -11,8 +11,8 @@
 
 package org.opensearch.indexmanagement
 
-import org.apache.hc.core5.http.HttpHeaders
 import org.apache.hc.core5.http.ContentType
+import org.apache.hc.core5.http.HttpHeaders
 import org.apache.hc.core5.http.io.entity.StringEntity
 import org.apache.hc.core5.http.message.BasicHeader
 import org.opensearch.client.Request
@@ -538,7 +538,7 @@ abstract class SecurityRestTestCase : IndexManagementRestTestCase() {
         const val AVAILABILITY_INDEX = "availability-1"
 
         const val PHONE_OPERATOR = "phone_operator"
-        const val HELPDESK = "helpdesk_stuff"
+        const val HELPDESK = "helpdesk_staff"
         const val HELPDESK_ROLE = "helpdesk_role"
         const val PHONE_OPERATOR_ROLE = "phone_operator_role"