Skip to content

Commit

Permalink
check if ad filter is enabled
Browse files Browse the repository at this point in the history
Signed-off-by: Amit Galitzky <[email protected]>
  • Loading branch information
amitgalitz committed Jun 22, 2023
1 parent b3c3f23 commit 70878cf
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand Down
18 changes: 9 additions & 9 deletions alerting/src/main/kotlin/org/opensearch/alerting/InputService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down

0 comments on commit 70878cf

Please sign in to comment.