diff --git a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/addpolicy/TransportAddPolicyAction.kt b/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/addpolicy/TransportAddPolicyAction.kt index 707845dc9..f16d8a92e 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/indexstatemanagement/transport/action/addpolicy/TransportAddPolicyAction.kt +++ b/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, 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() private val indicesToAdd = mutableMapOf() // uuid: name private val failedIndices: MutableList = 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) { - val request = ManagedIndexRequest().indices(indices[current]) - client.execute( - ManagedIndexAction.INSTANCE, - request, - object : ActionListener { - 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) { + val permittedIndices = mutableListOf() + 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) { - 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() { diff --git a/src/main/kotlin/org/opensearch/indexmanagement/opensearchapi/OpenSearchExtensions.kt b/src/main/kotlin/org/opensearch/indexmanagement/opensearchapi/OpenSearchExtensions.kt index 54c77a8bd..7d21aabea 100644 --- a/src/main/kotlin/org/opensearch/indexmanagement/opensearchapi/OpenSearchExtensions.kt +++ b/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 diff --git a/src/test/kotlin/org/opensearch/indexmanagement/IndexStateManagementSecurityBehaviorIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/IndexStateManagementSecurityBehaviorIT.kt index 18fbd72ba..7966222c6 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/IndexStateManagementSecurityBehaviorIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/IndexStateManagementSecurityBehaviorIT.kt @@ -43,7 +43,7 @@ import java.util.Locale class IndexStateManagementSecurityBehaviorIT : SecurityRestTestCase() { private val testIndexName = javaClass.simpleName.lowercase(Locale.ROOT) - private val password = "Test123!" + private val password = "Test123sdfsdfds435346FDGDFGDFG2342&^%#$@#35!" private val superIsmUser = "john" private var superUserClient: RestClient? = null diff --git a/src/test/kotlin/org/opensearch/indexmanagement/PolicySecurityBehaviorIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/PolicySecurityBehaviorIT.kt new file mode 100644 index 000000000..5af30a616 --- /dev/null +++ b/src/test/kotlin/org/opensearch/indexmanagement/PolicySecurityBehaviorIT.kt @@ -0,0 +1,121 @@ +/* + * 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.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 = "TestpgfhertergGd435AASA123!" + + 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() + val notPermittedindices = mutableListOf() + 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 explainResponseAsMap = managedIndexExplainAllAsMap(client()) + assertEquals(5, explainResponseAsMap["total_managed_indices"] as Int) + } catch (e: ResponseException) { + logger.error(e.message, e) + } finally { + deleteIndexByName("$permittedIndicesPrefix*") + deleteIndexByName("$notPermittedIndexPrefix*") + } + } +} diff --git a/src/test/kotlin/org/opensearch/indexmanagement/RollupSecurityBehaviorIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/RollupSecurityBehaviorIT.kt index b0c963e50..a34930c90 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/RollupSecurityBehaviorIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/RollupSecurityBehaviorIT.kt @@ -34,7 +34,7 @@ import java.time.temporal.ChronoUnit @TestLogging("level:DEBUG", reason = "Debug for tests.") class RollupSecurityBehaviorIT : SecurityRestTestCase() { - private val password = "Test123!" + private val password = "TestpgfhertergGd435AASA123!" private val superRollupUser = "john" private var superUserClient: RestClient? = null diff --git a/src/test/kotlin/org/opensearch/indexmanagement/SecurityBehaviorIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/SecurityBehaviorIT.kt index 90dfc398b..e7678745c 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/SecurityBehaviorIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/SecurityBehaviorIT.kt @@ -21,7 +21,7 @@ import org.opensearch.test.junit.annotations.TestLogging @TestLogging("level:DEBUG", reason = "Debug for tests.") class SecurityBehaviorIT : SecurityRestTestCase() { - private val password = "Test123!" + private val password = "TestpgfhertergGd435AASA123!" private val john = "john" private var johnClient: RestClient? = null diff --git a/src/test/kotlin/org/opensearch/indexmanagement/SecurityRestTestCase.kt b/src/test/kotlin/org/opensearch/indexmanagement/SecurityRestTestCase.kt index 642baf09e..05612f8d4 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/SecurityRestTestCase.kt +++ b/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 @@ -25,6 +25,7 @@ import org.opensearch.common.xcontent.XContentType import org.opensearch.indexmanagement.indexstatemanagement.IndexStateManagementRestTestCase import org.opensearch.indexmanagement.indexstatemanagement.model.ManagedIndexConfig import org.opensearch.indexmanagement.indexstatemanagement.model.Policy +import org.opensearch.indexmanagement.indexstatemanagement.resthandler.RestExplainAction import org.opensearch.indexmanagement.indexstatemanagement.settings.ManagedIndexSettings import org.opensearch.indexmanagement.indexstatemanagement.toJsonString import org.opensearch.indexmanagement.indexstatemanagement.util.INDEX_NUMBER_OF_REPLICAS @@ -213,6 +214,13 @@ abstract class SecurityRestTestCase : IndexManagementRestTestCase() { return IndexStateManagementRestTestCaseExt.createPolicyExt(policy, policyId, refresh, client) } + protected fun managedIndexExplainAllAsMap( + client: RestClient?, + ): Map<*, *> { + val request = Request("GET", "${RestExplainAction.EXPLAIN_BASE_URI}") + return entityAsMap(executeRequest(request, RestStatus.OK, client!!)) + } + protected fun getExistingManagedIndexConfig(index: String) = IndexStateManagementRestTestCaseExt.getExistingManagedIndexConfigExt(index) protected fun createPolicyJson( @@ -400,7 +408,7 @@ abstract class SecurityRestTestCase : IndexManagementRestTestCase() { val request = Request(RestRequest.Method.PUT.name, "_plugins/_security/api/internalusers/$name") request.setJsonEntity(json) - executeRequest(request, null, client()) + executeRequest(request, RestStatus.CREATED, client()) } protected fun createUserWithCustomRole( @@ -453,7 +461,7 @@ abstract class SecurityRestTestCase : IndexManagementRestTestCase() { """.trimIndent() request.setJsonEntity(entity) - client().performRequest(request) + executeRequest(request, RestStatus.CREATED, client()) } protected fun assignRoleToUsers(role: String, users: List) { @@ -538,7 +546,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" diff --git a/src/test/kotlin/org/opensearch/indexmanagement/TransformSecurityBehaviorIT.kt b/src/test/kotlin/org/opensearch/indexmanagement/TransformSecurityBehaviorIT.kt index 371866e29..071ece9c9 100644 --- a/src/test/kotlin/org/opensearch/indexmanagement/TransformSecurityBehaviorIT.kt +++ b/src/test/kotlin/org/opensearch/indexmanagement/TransformSecurityBehaviorIT.kt @@ -28,7 +28,7 @@ import java.time.temporal.ChronoUnit @TestLogging("level:DEBUG", reason = "Debug for tests.") class TransformSecurityBehaviorIT : SecurityRestTestCase() { - private val password = "Test123!" + private val password = "TestpgfhertergGd435AASA123!" private val superTransformUser = "john" private var superUserClient: RestClient? = null