From 25df4be0834f0ac4db3927db66a3f0869ab07222 Mon Sep 17 00:00:00 2001 From: Amit Galitzky Date: Thu, 22 Jun 2023 12:08:59 -0400 Subject: [PATCH] check if ad filter is enabled Signed-off-by: Amit Galitzky --- .../org/opensearch/alerting/AlertingPlugin.kt | 4 +-- .../org/opensearch/alerting/InputService.kt | 18 ++++++------- .../transport/TransportIndexMonitorAction.kt | 5 +++- .../alerting/util/AnomalyDetectionUtils.kt | 25 +++++++++++++++++++ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt index 5836171b0..2ca9036ba 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt @@ -242,7 +242,7 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R .registerSettings(settings) .registerThreadPool(threadPool) .registerAlertIndices(alertIndices) - .registerInputService(InputService(client, scriptService, namedWriteableRegistry, xContentRegistry)) + .registerInputService(InputService(client, scriptService, namedWriteableRegistry, xContentRegistry, clusterService, settings)) .registerTriggerService(TriggerService(scriptService)) .registerAlertService(AlertService(client, xContentRegistry, alertIndices)) .registerDocLevelMonitorQueries(DocLevelMonitorQueries(client, clusterService)) @@ -257,7 +257,7 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R .registerSettings(settings) .registerThreadPool(threadPool) .registerAlertIndices(alertIndices) - .registerInputService(InputService(client, scriptService, namedWriteableRegistry, xContentRegistry)) + .registerInputService(InputService(client, scriptService, namedWriteableRegistry, xContentRegistry, clusterService, settings)) .registerTriggerService(TriggerService(scriptService)) .registerAlertService(AlertService(client, xContentRegistry, alertIndices)) .registerDocLevelMonitorQueries(DocLevelMonitorQueries(client, clusterService)) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt b/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt index 94dc82d28..8048157d8 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt @@ -15,13 +15,16 @@ import org.opensearch.alerting.opensearchapi.suspendUntil import org.opensearch.alerting.util.AggregationQueryRewriter import org.opensearch.alerting.util.addUserBackendRolesFilter import org.opensearch.alerting.util.executeTransportAction +import org.opensearch.alerting.util.getADBackendRoleFilterEnabled import org.opensearch.alerting.util.toMap import org.opensearch.alerting.util.use import org.opensearch.alerting.workflow.WorkflowRunContext import org.opensearch.client.Client +import org.opensearch.cluster.service.ClusterService import org.opensearch.common.io.stream.BytesStreamOutput import org.opensearch.common.io.stream.NamedWriteableAwareStreamInput import org.opensearch.common.io.stream.NamedWriteableRegistry +import org.opensearch.common.settings.Settings import org.opensearch.common.xcontent.LoggingDeprecationHandler import org.opensearch.common.xcontent.XContentType import org.opensearch.commons.alerting.model.ClusterMetricsInput @@ -45,7 +48,9 @@ class InputService( val client: Client, val scriptService: ScriptService, val namedWriteableRegistry: NamedWriteableRegistry, - val xContentRegistry: NamedXContentRegistry + val xContentRegistry: NamedXContentRegistry, + val clusterService: ClusterService, + val settings: Settings ) { private val logger = LogManager.getLogger(InputService::class.java) @@ -194,13 +199,6 @@ class InputService( // Add user role filter for AD result client.threadPool().threadContext.stashContext().use { - // Currently we have no way to verify if user has AD read permission or not. So we always add user - // role filter here no matter AD backend role filter enabled or not. If we don't add user role filter - // when AD backend filter disabled, user can run monitor on any detector and get anomaly data even - // they have no AD read permission. So if domain disabled AD backend role filter, monitor runner - // still can't get AD result with different user backend role, even the monitor user has permission - // to read AD result. This is a short term solution to trade off between user experience and security. - // // Possible long term solution: // 1.Use secure rest client to send request to AD search result API. If no permission exception, // that mean user has read access on AD result. Then don't need to add user role filter when query @@ -209,7 +207,9 @@ class InputService( // Monitor runner will send transport request to check permission first. If security plugin response // is yes, user has permission to query AD result. If AD role filter enabled, we will add user role // filter to protect data at user role level; otherwise, user can query any AD result. - addUserBackendRolesFilter(monitor.user, searchRequest.source()) + if (getADBackendRoleFilterEnabled(clusterService, settings)) { + addUserBackendRolesFilter(monitor.user, searchRequest.source()) + } val searchResponse: SearchResponse = client.suspendUntil { client.search(searchRequest, it) } results += searchResponse.convertToMap() } 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 065a1ba82..bbb762ee7 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt @@ -45,6 +45,7 @@ import org.opensearch.alerting.util.AlertingException import org.opensearch.alerting.util.DocLevelMonitorQueries import org.opensearch.alerting.util.IndexUtils import org.opensearch.alerting.util.addUserBackendRolesFilter +import org.opensearch.alerting.util.getADBackendRoleFilterEnabled import org.opensearch.alerting.util.isADMonitor import org.opensearch.alerting.util.use import org.opensearch.client.Client @@ -269,7 +270,9 @@ class TransportIndexMonitorAction @Inject constructor( request.monitor = request.monitor .copy(user = User(user.name, user.backendRoles, user.roles, user.customAttNames)) val searchSourceBuilder = SearchSourceBuilder().size(0) - addUserBackendRolesFilter(user, searchSourceBuilder) + if (getADBackendRoleFilterEnabled(clusterService, settings)) { + addUserBackendRolesFilter(user, searchSourceBuilder) + } val searchRequest = SearchRequest().indices(".opendistro-anomaly-detectors").source(searchSourceBuilder) client.search( searchRequest, diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt b/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt index e83f45a15..0e54f6c60 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/util/AnomalyDetectionUtils.kt @@ -6,6 +6,8 @@ package org.opensearch.alerting.util import org.apache.lucene.search.join.ScoreMode +import org.opensearch.cluster.service.ClusterService +import org.opensearch.common.settings.Settings import org.opensearch.commons.alerting.model.Monitor import org.opensearch.commons.alerting.model.SearchInput import org.opensearch.commons.authuser.User @@ -32,6 +34,29 @@ fun isADMonitor(monitor: Monitor): Boolean { return false } +fun getADBackendRoleFilterEnabled(clusterService: ClusterService, settings: Settings): Boolean { + var adBackendRoleFilterEnabled: Boolean + val metaData = clusterService.state().metadata() + val adFilterString = "plugins.anomaly_detection.filter_by_backend_roles" + + // get default value for setting + if (clusterService.clusterSettings.get(adFilterString) != null) { + adBackendRoleFilterEnabled = clusterService.clusterSettings.get(adFilterString).getDefault(settings) as Boolean + } else { + // default setting doesn't exist, so returning false as it means AD plugins isn't in cluster anyway + return false + } + + // Transient settings are prioritized so those are checked first. + return if (metaData.transientSettings().get(adFilterString) != null) { + metaData.transientSettings().getAsBoolean(adFilterString, adBackendRoleFilterEnabled) + } else if (metaData.persistentSettings().get(adFilterString) != null) { + metaData.persistentSettings().getAsBoolean(adFilterString, adBackendRoleFilterEnabled) + } else { + adBackendRoleFilterEnabled + } +} + fun addUserBackendRolesFilter(user: User?, searchSourceBuilder: SearchSourceBuilder): SearchSourceBuilder { var boolQueryBuilder = BoolQueryBuilder() val userFieldName = "user"