diff --git a/README.md b/README.md index 9cd2b2c4a..91e5fa1c6 100644 --- a/README.md +++ b/README.md @@ -58,4 +58,4 @@ This project is licensed under the [Apache v2.0 License](LICENSE.txt). ## Copyright - Copyright 2020-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. \ No newline at end of file +Copyright OpenSearch Contributors. See [NOTICE](NOTICE.txt) for details. \ No newline at end of file diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/SecureTransportAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/SecureTransportAction.kt new file mode 100644 index 000000000..4ad7be890 --- /dev/null +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/SecureTransportAction.kt @@ -0,0 +1,139 @@ +/* + * 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.alerting.transport + +import org.apache.logging.log4j.LogManager +import org.opensearch.OpenSearchStatusException +import org.opensearch.action.ActionListener +import org.opensearch.alerting.settings.AlertingSettings +import org.opensearch.alerting.util.AlertingException +import org.opensearch.client.Client +import org.opensearch.cluster.service.ClusterService +import org.opensearch.commons.ConfigConstants +import org.opensearch.commons.authuser.User +import org.opensearch.rest.RestStatus + +private val log = LogManager.getLogger(SecureTransportAction::class.java) + +/** + * TransportActon classes extend this interface to add filter-by-backend-roles functionality. + * + * 1. If filterBy is enabled + * a) Don't allow to create monitor/ destination (throw error) if the logged-on user has no backend roles configured. + * + * 2. If filterBy is enabled & monitors are created when filterBy is disabled: + * a) If backend_roles are saved with config, results will get filtered and data is shown + * b) If backend_roles are not saved with monitor config, results will get filtered and no monitors + * will be displayed. + * c) Users can edit and save the monitors to associate their backend_roles. + * + * 3. If filterBy is enabled & monitors are created by older version: + * a) No User details are present on monitor. + * b) No monitors will be displayed. + * c) Users can edit and save the monitors to associate their backend_roles. + */ +interface SecureTransportAction { + + var filterByEnabled: Boolean + + fun listenFilterBySettingChange(clusterService: ClusterService) { + clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + } + + fun readUserFromThreadContext(client: Client): User? { + val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) + log.debug("User and roles string from thread context: $userStr") + return User.parse(userStr) + } + + fun doFilterForUser(user: User?): Boolean { + log.debug("Is filterByEnabled: $filterByEnabled ; Is admin user: ${isAdmin(user)}") + return if (isAdmin(user)) { + false + } else { + filterByEnabled + } + } + + /** + * 'all_access' role users are treated as admins. + */ + private fun isAdmin(user: User?): Boolean { + return when { + user == null -> { + false + } + user.roles?.isNullOrEmpty() == true -> { + false + } + else -> { + user.roles?.contains("all_access") == true + } + } + } + + fun validateUserBackendRoles(user: User?, actionListener: ActionListener): Boolean { + if (filterByEnabled) { + if (user == null) { + actionListener.onFailure( + AlertingException.wrap( + OpenSearchStatusException( + "Filter by user backend roles is enabled with security disabled.", RestStatus.FORBIDDEN + ) + ) + ) + return false + } else if (user.backendRoles.isNullOrEmpty()) { + actionListener.onFailure( + AlertingException.wrap( + OpenSearchStatusException("User doesn't have backend roles configured. Contact administrator", RestStatus.FORBIDDEN) + ) + ) + return false + } + } + return true + } + + /** + * If FilterBy is enabled, this function verifies that the requester user has FilterBy permissions to access + * the resource. If FilterBy is disabled, we will assume the user has permissions and return true. + * + * This check will later to moved to the security plugin. + */ + fun checkUserPermissionsWithResource( + requesterUser: User?, + resourceUser: User?, + actionListener: ActionListener, + resourceType: String, + resourceId: String + ): Boolean { + + if (!filterByEnabled) return true + + val resourceBackendRoles = resourceUser?.backendRoles + val requesterBackendRoles = requesterUser?.backendRoles + + if (resourceBackendRoles == null || requesterBackendRoles == null || resourceBackendRoles.intersect(requesterBackendRoles).isEmpty()) { + actionListener.onFailure( + AlertingException.wrap( + OpenSearchStatusException( + "Do not have permissions to resource, $resourceType, with id, $resourceId", + RestStatus.FORBIDDEN + ) + ) + ) + return false + } + return true + } +} diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteDestinationAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteDestinationAction.kt index 2537e0ffa..2657363c0 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteDestinationAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteDestinationAction.kt @@ -41,8 +41,6 @@ import org.opensearch.alerting.core.model.ScheduledJob import org.opensearch.alerting.model.destination.Destination import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.util.AlertingException -import org.opensearch.alerting.util.checkFilterByUserBackendRoles -import org.opensearch.alerting.util.checkUserFilterByPermissions import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject @@ -53,7 +51,6 @@ import org.opensearch.common.xcontent.XContentFactory import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.rest.RestStatus import org.opensearch.tasks.Task @@ -71,22 +68,21 @@ class TransportDeleteDestinationAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction( DeleteDestinationAction.NAME, transportService, actionFilters, ::DeleteDestinationRequest -) { +), + SecureTransportAction { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, request: DeleteDestinationRequest, actionListener: ActionListener) { - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) + val user = readUserFromThreadContext(client) val deleteRequest = DeleteRequest(ScheduledJob.SCHEDULED_JOBS_INDEX, request.destinationId) .setRefreshPolicy(request.refreshPolicy) - if (!checkFilterByUserBackendRoles(filterByEnabled, user, actionListener)) { + if (!validateUserBackendRoles(user, actionListener)) { return } client.threadPool().threadContext.stashContext().use { @@ -106,7 +102,7 @@ class TransportDeleteDestinationAction @Inject constructor( if (user == null) { // Security is disabled, so we can delete the destination without issues deleteDestination() - } else if (!filterByEnabled) { + } else if (!doFilterForUser(user)) { // security is enabled and filterby is disabled. deleteDestination() } else { @@ -153,7 +149,7 @@ class TransportDeleteDestinationAction @Inject constructor( } private fun onGetResponse(destination: Destination) { - if (!checkUserFilterByPermissions(filterByEnabled, user, destination.user, actionListener, "destination", destinationId)) { + if (!checkUserPermissionsWithResource(user, destination.user, actionListener, "destination", destinationId)) { return } else { deleteDestination() diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteMonitorAction.kt index fff81b2c6..9acbbd266 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportDeleteMonitorAction.kt @@ -41,8 +41,6 @@ import org.opensearch.alerting.core.model.ScheduledJob import org.opensearch.alerting.model.Monitor import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.util.AlertingException -import org.opensearch.alerting.util.checkFilterByUserBackendRoles -import org.opensearch.alerting.util.checkUserFilterByPermissions import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject @@ -51,7 +49,6 @@ import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.rest.RestStatus import org.opensearch.tasks.Task @@ -69,22 +66,21 @@ class TransportDeleteMonitorAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction( DeleteMonitorAction.NAME, transportService, actionFilters, ::DeleteMonitorRequest -) { +), + SecureTransportAction { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, request: DeleteMonitorRequest, actionListener: ActionListener) { - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) + val user = readUserFromThreadContext(client) val deleteRequest = DeleteRequest(ScheduledJob.SCHEDULED_JOBS_INDEX, request.monitorId) .setRefreshPolicy(request.refreshPolicy) - if (!checkFilterByUserBackendRoles(filterByEnabled, user, actionListener)) { + if (!validateUserBackendRoles(user, actionListener)) { return } client.threadPool().threadContext.stashContext().use { @@ -104,7 +100,7 @@ class TransportDeleteMonitorAction @Inject constructor( if (user == null) { // Security is disabled, so we can delete the destination without issues deleteMonitor() - } else if (!filterByEnabled) { + } else if (!doFilterForUser(user)) { // security is enabled and filterby is disabled. deleteMonitor() } else { @@ -145,7 +141,7 @@ class TransportDeleteMonitorAction @Inject constructor( } private fun onGetResponse(monitor: Monitor) { - if (!checkUserFilterByPermissions(filterByEnabled, user, monitor.user, actionListener, "monitor", monitorId)) { + if (!checkUserPermissionsWithResource(user, monitor.user, actionListener, "monitor", monitorId)) { return } else { deleteMonitor() diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt index f5da20102..6a10bd528 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt @@ -50,7 +50,6 @@ import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.index.query.Operator import org.opensearch.index.query.QueryBuilders @@ -72,12 +71,13 @@ class TransportGetAlertsAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction( GetAlertsAction.NAME, transportService, actionFilters, ::GetAlertsRequest -) { +), + SecureTransportAction { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute( @@ -85,11 +85,7 @@ class TransportGetAlertsAction @Inject constructor( getAlertsRequest: GetAlertsRequest, actionListener: ActionListener ) { - val userStr = client.threadPool().threadContext.getTransient( - ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT - ) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) + val user = readUserFromThreadContext(client) val tableProp = getAlertsRequest.table val sortBuilder = SortBuilders @@ -143,7 +139,7 @@ class TransportGetAlertsAction @Inject constructor( if (user == null) { // user is null when: 1/ security is disabled. 2/when user is super-admin. search(searchSourceBuilder, actionListener) - } else if (!filterByEnabled) { + } else if (!doFilterForUser(user)) { // security is enabled and filterby is disabled. search(searchSourceBuilder, actionListener) } else { diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetDestinationsAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetDestinationsAction.kt index 27683b928..b74df5508 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetDestinationsAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetDestinationsAction.kt @@ -51,7 +51,6 @@ import org.opensearch.common.xcontent.XContentFactory import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.index.query.Operator import org.opensearch.index.query.QueryBuilders @@ -75,12 +74,13 @@ class TransportGetDestinationsAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction ( GetDestinationsAction.NAME, transportService, actionFilters, ::GetDestinationsRequest -) { +), + SecureTransportAction { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute( @@ -88,12 +88,7 @@ class TransportGetDestinationsAction @Inject constructor( getDestinationsRequest: GetDestinationsRequest, actionListener: ActionListener ) { - val userStr = client.threadPool().threadContext.getTransient( - ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT - ) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) - + val user = readUserFromThreadContext(client) val tableProp = getDestinationsRequest.table val sortBuilder = SortBuilders @@ -144,7 +139,7 @@ class TransportGetDestinationsAction @Inject constructor( if (user == null) { // user is null when: 1/ security is disabled. 2/when user is super-admin. search(searchSourceBuilder, actionListener) - } else if (!filterByEnabled) { + } else if (!doFilterForUser(user)) { // security is enabled and filterby is disabled. search(searchSourceBuilder, actionListener) } else { diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetMonitorAction.kt index 8de0c5a08..9eb250c0f 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetMonitorAction.kt @@ -40,8 +40,6 @@ import org.opensearch.alerting.core.model.ScheduledJob import org.opensearch.alerting.model.Monitor import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.util.AlertingException -import org.opensearch.alerting.util.checkFilterByUserBackendRoles -import org.opensearch.alerting.util.checkUserFilterByPermissions import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject @@ -50,8 +48,6 @@ import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants -import org.opensearch.commons.authuser.User import org.opensearch.rest.RestStatus import org.opensearch.tasks.Task import org.opensearch.transport.TransportService @@ -67,24 +63,23 @@ class TransportGetMonitorAction @Inject constructor( settings: Settings ) : HandledTransportAction ( GetMonitorAction.NAME, transportService, actionFilters, ::GetMonitorRequest -) { +), + SecureTransportAction { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, getMonitorRequest: GetMonitorRequest, actionListener: ActionListener) { - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) + val user = readUserFromThreadContext(client) val getRequest = GetRequest(ScheduledJob.SCHEDULED_JOBS_INDEX, getMonitorRequest.monitorId) .version(getMonitorRequest.version) .fetchSourceContext(getMonitorRequest.srcContext) - if (!checkFilterByUserBackendRoles(filterByEnabled, user, actionListener)) { + if (!validateUserBackendRoles(user, actionListener)) { return } @@ -115,8 +110,7 @@ class TransportGetMonitorAction @Inject constructor( monitor = ScheduledJob.parse(xcp, response.id, response.version) as Monitor // security is enabled and filterby is enabled - if (!checkUserFilterByPermissions( - filterByEnabled, + if (!checkUserPermissionsWithResource( user, monitor?.user, actionListener, diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt index fd7f72fec..4a80acd4d 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexDestinationAction.kt @@ -32,8 +32,6 @@ import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.settings.DestinationSettings import org.opensearch.alerting.util.AlertingException import org.opensearch.alerting.util.IndexUtils -import org.opensearch.alerting.util.checkFilterByUserBackendRoles -import org.opensearch.alerting.util.checkUserFilterByPermissions import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject @@ -46,7 +44,6 @@ import org.opensearch.common.xcontent.XContentFactory.jsonBuilder import org.opensearch.common.xcontent.XContentParser import org.opensearch.common.xcontent.XContentParserUtils import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.rest.RestRequest import org.opensearch.rest.RestStatus @@ -66,24 +63,23 @@ class TransportIndexDestinationAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction( IndexDestinationAction.NAME, transportService, actionFilters, ::IndexDestinationRequest -) { +), + SecureTransportAction { @Volatile private var indexTimeout = AlertingSettings.INDEX_TIMEOUT.get(settings) @Volatile private var allowList = DestinationSettings.ALLOW_LIST.get(settings) - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.INDEX_TIMEOUT) { indexTimeout = it } clusterService.clusterSettings.addSettingsUpdateConsumer(DestinationSettings.ALLOW_LIST) { allowList = it } - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, request: IndexDestinationRequest, actionListener: ActionListener) { - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) + val user = readUserFromThreadContext(client) - if (!checkFilterByUserBackendRoles(filterByEnabled, user, actionListener)) { + if (!validateUserBackendRoles(user, actionListener)) { return } client.threadPool().threadContext.stashContext().use { @@ -267,8 +263,7 @@ class TransportIndexDestinationAction @Inject constructor( } private fun onGetResponse(destination: Destination) { - if (!checkUserFilterByPermissions( - filterByEnabled, + if (!checkUserPermissionsWithResource( user, destination.user, actionListener, diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt index 3602534f7..c1f9ccdcc 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt @@ -51,7 +51,6 @@ import org.opensearch.alerting.core.model.SearchInput import org.opensearch.alerting.model.Monitor import org.opensearch.alerting.settings.AlertingSettings import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_MAX_MONITORS -import org.opensearch.alerting.settings.AlertingSettings.Companion.FILTER_BY_BACKEND_ROLES import org.opensearch.alerting.settings.AlertingSettings.Companion.INDEX_TIMEOUT import org.opensearch.alerting.settings.AlertingSettings.Companion.MAX_ACTION_THROTTLE_VALUE import org.opensearch.alerting.settings.AlertingSettings.Companion.REQUEST_TIMEOUT @@ -59,8 +58,6 @@ import org.opensearch.alerting.settings.DestinationSettings.Companion.ALLOW_LIST import org.opensearch.alerting.util.AlertingException import org.opensearch.alerting.util.IndexUtils import org.opensearch.alerting.util.addUserBackendRolesFilter -import org.opensearch.alerting.util.checkFilterByUserBackendRoles -import org.opensearch.alerting.util.checkUserFilterByPermissions import org.opensearch.alerting.util.isADMonitor import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService @@ -73,7 +70,6 @@ import org.opensearch.common.xcontent.ToXContent import org.opensearch.common.xcontent.XContentFactory.jsonBuilder import org.opensearch.common.xcontent.XContentHelper import org.opensearch.common.xcontent.XContentType -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.index.query.QueryBuilders import org.opensearch.rest.RestRequest @@ -96,14 +92,15 @@ class TransportIndexMonitorAction @Inject constructor( val xContentRegistry: NamedXContentRegistry ) : HandledTransportAction( IndexMonitorAction.NAME, transportService, actionFilters, ::IndexMonitorRequest -) { +), + SecureTransportAction { @Volatile private var maxMonitors = ALERTING_MAX_MONITORS.get(settings) @Volatile private var requestTimeout = REQUEST_TIMEOUT.get(settings) @Volatile private var indexTimeout = INDEX_TIMEOUT.get(settings) @Volatile private var maxActionThrottle = MAX_ACTION_THROTTLE_VALUE.get(settings) @Volatile private var allowList = ALLOW_LIST.get(settings) - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) + @Volatile override var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { clusterService.clusterSettings.addSettingsUpdateConsumer(ALERTING_MAX_MONITORS) { maxMonitors = it } @@ -111,16 +108,13 @@ class TransportIndexMonitorAction @Inject constructor( clusterService.clusterSettings.addSettingsUpdateConsumer(INDEX_TIMEOUT) { indexTimeout = it } clusterService.clusterSettings.addSettingsUpdateConsumer(MAX_ACTION_THROTTLE_VALUE) { maxActionThrottle = it } clusterService.clusterSettings.addSettingsUpdateConsumer(ALLOW_LIST) { allowList = it } - clusterService.clusterSettings.addSettingsUpdateConsumer(FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, request: IndexMonitorRequest, actionListener: ActionListener) { + val user = readUserFromThreadContext(client) - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) - - if (!checkFilterByUserBackendRoles(filterByEnabled, user, actionListener)) { + if (!validateUserBackendRoles(user, actionListener)) { return } @@ -458,7 +452,7 @@ class TransportIndexMonitorAction @Inject constructor( } private fun onGetResponse(currentMonitor: Monitor) { - if (!checkUserFilterByPermissions(filterByEnabled, user, currentMonitor.user, actionListener, "monitor", request.monitorId)) { + if (!checkUserPermissionsWithResource(user, currentMonitor.user, actionListener, "monitor", request.monitorId)) { return } diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt index 198203e0f..686ce4442 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportSearchMonitorAction.kt @@ -41,7 +41,6 @@ import org.opensearch.client.Client import org.opensearch.cluster.service.ClusterService import org.opensearch.common.inject.Inject import org.opensearch.common.settings.Settings -import org.opensearch.commons.ConfigConstants import org.opensearch.commons.authuser.User import org.opensearch.tasks.Task import org.opensearch.transport.TransportService @@ -56,18 +55,16 @@ class TransportSearchMonitorAction @Inject constructor( actionFilters: ActionFilters ) : HandledTransportAction( SearchMonitorAction.NAME, transportService, actionFilters, ::SearchMonitorRequest -) { - @Volatile private var filterByEnabled = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) - +), + SecureTransportAction { + @Volatile + override var filterByEnabled: Boolean = AlertingSettings.FILTER_BY_BACKEND_ROLES.get(settings) init { - clusterService.clusterSettings.addSettingsUpdateConsumer(AlertingSettings.FILTER_BY_BACKEND_ROLES) { filterByEnabled = it } + listenFilterBySettingChange(clusterService) } override fun doExecute(task: Task, searchMonitorRequest: SearchMonitorRequest, actionListener: ActionListener) { - val userStr = client.threadPool().threadContext.getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT) - log.debug("User and roles string from thread context: $userStr") - val user: User? = User.parse(userStr) - + val user = readUserFromThreadContext(client) client.threadPool().threadContext.stashContext().use { resolve(searchMonitorRequest, actionListener, user) } @@ -77,7 +74,7 @@ class TransportSearchMonitorAction @Inject constructor( if (user == null) { // user header is null when: 1/ security is disabled. 2/when user is super-admin. search(searchMonitorRequest.searchRequest, actionListener) - } else if (!filterByEnabled) { + } else if (!doFilterForUser(user)) { // security is enabled and filterby is disabled. search(searchMonitorRequest.searchRequest, actionListener) } else { diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt b/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt index 61f4dd9e0..e5558ecaa 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/util/AlertingUtils.kt @@ -27,8 +27,6 @@ package org.opensearch.alerting.util import inet.ipaddr.IPAddressString -import org.opensearch.OpenSearchStatusException -import org.opensearch.action.ActionListener import org.opensearch.alerting.destination.message.BaseMessage import org.opensearch.alerting.model.AggregationResultBucket import org.opensearch.alerting.model.BucketLevelTriggerRunResult @@ -37,8 +35,6 @@ import org.opensearch.alerting.model.action.Action import org.opensearch.alerting.model.action.ActionExecutionPolicy import org.opensearch.alerting.model.destination.Destination import org.opensearch.alerting.settings.DestinationSettings -import org.opensearch.commons.authuser.User -import org.opensearch.rest.RestStatus /** * RFC 5322 compliant pattern matching: https://www.ietf.org/rfc/rfc5322.txt @@ -75,76 +71,6 @@ fun BaseMessage.isHostInDenylist(networks: List): Boolean { return false } -/** - 1. If filterBy is enabled - a) Don't allow to create monitor/ destination (throw error) if the logged-on user has no backend roles configured. - 2. If filterBy is enabled & monitors are created when filterBy is disabled: - a) If backend_roles are saved with config, results will get filtered and data is shown - b) If backend_roles are not saved with monitor config, results will get filtered and no monitors - will be displayed. - c) Users can edit and save the monitors to associate their backend_roles. - 3. If filterBy is enabled & monitors are created by older version: - a) No User details are present on monitor. - b) No monitors will be displayed. - c) Users can edit and save the monitors to associate their backend_roles. - */ -fun checkFilterByUserBackendRoles(filterByEnabled: Boolean, user: User?, actionListener: ActionListener): Boolean { - if (filterByEnabled) { - if (user == null) { - actionListener.onFailure( - AlertingException.wrap( - OpenSearchStatusException( - "Filter by user backend roles is not enabled with security disabled.", RestStatus.FORBIDDEN - ) - ) - ) - return false - } else if (user.backendRoles.isNullOrEmpty()) { - actionListener.onFailure( - AlertingException.wrap( - OpenSearchStatusException("User doesn't have backend roles configured. Contact administrator.", RestStatus.FORBIDDEN) - ) - ) - return false - } - } - return true -} - -/** - * If FilterBy is enabled, this function verifies that the requester user has FilterBy permissions to access - * the resource. If FilterBy is disabled, we will assume the user has permissions and return true. - * - * This check will later to moved to the security plugin. - */ -fun checkUserFilterByPermissions( - filterByEnabled: Boolean, - requesterUser: User?, - resourceUser: User?, - actionListener: ActionListener, - resourceType: String, - resourceId: String -): Boolean { - - if (!filterByEnabled) return true - - val resourceBackendRoles = resourceUser?.backendRoles - val requesterBackendRoles = requesterUser?.backendRoles - - if (resourceBackendRoles == null || requesterBackendRoles == null || resourceBackendRoles.intersect(requesterBackendRoles).isEmpty()) { - actionListener.onFailure( - AlertingException.wrap( - OpenSearchStatusException( - "Do not have permissions to resource, $resourceType, with id, $resourceId", - RestStatus.FORBIDDEN - ) - ) - ) - return false - } - return true -} - fun Monitor.isBucketLevelMonitor(): Boolean = this.monitorType == Monitor.MonitorType.BUCKET_LEVEL_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 d613d9719..e167e76c7 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt @@ -31,7 +31,10 @@ import org.opensearch.alerting.randomTemplateScript import org.opensearch.client.Response import org.opensearch.client.ResponseException import org.opensearch.client.RestClient +import org.opensearch.common.xcontent.LoggingDeprecationHandler +import org.opensearch.common.xcontent.NamedXContentRegistry import org.opensearch.common.xcontent.XContentType +import org.opensearch.common.xcontent.json.JsonXContent import org.opensearch.commons.authuser.User import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.index.query.QueryBuilders @@ -476,4 +479,46 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() { deleteRoleMapping("alerting_full_access") } } + + fun `test admin all access with enable filter by`() { + if (!securityEnabled()) + return + + enableFilterBy() + createUserWithTestData(user, "hr_data", "hr_role", "HR") + createUserRolesMapping("alerting_full_access", arrayOf(user)) + try { + // randomMonitor has a dummy user, api ignores the User passed as part of monitor, it picks user info from the logged-in user. + val monitor = randomQueryLevelMonitor().copy( + inputs = listOf( + SearchInput( + indices = listOf("hr_data"), query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + ) + ) + ) + + val createResponse = userClient?.makeRequest("POST", ALERTING_BASE_URI, emptyMap(), monitor.toHttpEntity()) + assertEquals("Create monitor failed", RestStatus.CREATED, createResponse?.restStatus()) + val monitorJson = JsonXContent.jsonXContent.createParser( + NamedXContentRegistry.EMPTY, LoggingDeprecationHandler.INSTANCE, + createResponse?.entity?.content + ).map() + + val search = SearchSourceBuilder().query(QueryBuilders.termQuery("_id", monitorJson["_id"])).toString() + + // search as "admin" - must get 1 docs + val adminSearchResponse = client().makeRequest( + "POST", + "$ALERTING_BASE_URI/_search", + emptyMap(), + NStringEntity(search, ContentType.APPLICATION_JSON) + ) + assertEquals("Search monitor failed", RestStatus.OK, adminSearchResponse.restStatus()) + assertEquals("Monitor not found during search", 1, getDocs(adminSearchResponse)) + } finally { + deleteRoleMapping("hr_role") + deleteRole("hr_role") + deleteRoleMapping("alerting_full_access") + } + } }