Skip to content

Commit

Permalink
AddPolicyAction - Recursion removal (opensearch-project#779)
Browse files Browse the repository at this point in the history
* recursion removal from AddPolicy Action

Signed-off-by: Petar Dzepina <[email protected]>

* added test

Signed-off-by: Petar Dzepina <[email protected]>

* debug logging

Signed-off-by: Petar Dzepina <[email protected]>

* removed single node testcase

Signed-off-by: Petar Dzepina <[email protected]>

* added security test; fixed index permission check

Signed-off-by: Petar Dzepina <[email protected]>

* test fix

Signed-off-by: Petar Dzepina <[email protected]>

* addressing comments

Signed-off-by: Petar Dzepina <[email protected]>

* test cleanup

Signed-off-by: Petar Dzepina <[email protected]>

* reverted security inject changes

Signed-off-by: Petar Dzepina <[email protected]>

* fixed weak password error when creating test user

Signed-off-by: Petar Dzepina <[email protected]>

* test tweak

Signed-off-by: Petar Dzepina <[email protected]>

---------

Signed-off-by: Petar Dzepina <[email protected]>
  • Loading branch information
petardz authored May 18, 2023
1 parent 7ed6299 commit 45b1476
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<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 explainResponseAsMap = managedIndexExplainAllAsMap(client())
assertEquals(5, explainResponseAsMap["total_managed_indices"] as Int)
} catch (e: ResponseException) {
logger.error(e.message, e)
} finally {
deleteIndexByName("$permittedIndicesPrefix*")
deleteIndexByName("$notPermittedIndexPrefix*")
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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<String>) {
Expand Down Expand Up @@ -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"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 45b1476

Please sign in to comment.