Skip to content

Commit

Permalink
Fix getAlerts RBAC problem (#991) (#1045)
Browse files Browse the repository at this point in the history 
Signed-off-by: Ashish Agrawal <[email protected]>
  • Loading branch information
@lezzago
lezzago authored Jul 18, 2023
1 parent 8cb40c8 commit ca8851f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 24 deletions.
52 changes: 28 additions & 24 deletions alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportGetAlertsAction.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ package org.opensearch.alerting.transport
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext
import org.apache.logging.log4j.LogManager
import org.opensearch.action.ActionListener
import org.opensearch.action.ActionRequest
import org.opensearch.action.get.GetRequest
import org.opensearch.action.get.GetResponse
import org.opensearch.action.search.SearchRequest
import org.opensearch.action.search.SearchResponse
import org.opensearch.action.support.ActionFilters
import org.opensearch.action.support.HandledTransportAction
import org.opensearch.alerting.action.GetMonitorAction
import org.opensearch.alerting.action.GetMonitorRequest
import org.opensearch.alerting.action.GetMonitorResponse
import org.opensearch.alerting.alerts.AlertIndices
import org.opensearch.alerting.opensearchapi.addFilter
import org.opensearch.alerting.opensearchapi.suspendUntil
Expand All @@ -38,13 +36,13 @@ import org.opensearch.commons.alerting.action.AlertingActions
import org.opensearch.commons.alerting.action.GetAlertsRequest
import org.opensearch.commons.alerting.action.GetAlertsResponse
import org.opensearch.commons.alerting.model.Alert
import org.opensearch.commons.alerting.model.Monitor
import org.opensearch.commons.alerting.model.ScheduledJob
import org.opensearch.commons.authuser.User
import org.opensearch.commons.utils.recreateObject
import org.opensearch.index.query.Operator
import org.opensearch.index.query.QueryBuilders
import org.opensearch.rest.RestRequest
import org.opensearch.search.builder.SearchSourceBuilder
import org.opensearch.search.fetch.subphase.FetchSourceContext
import org.opensearch.search.sort.SortBuilders
import org.opensearch.search.sort.SortOrder
import org.opensearch.tasks.Task
Expand All @@ -60,8 +58,7 @@ class TransportGetAlertsAction @Inject constructor(
clusterService: ClusterService,
actionFilters: ActionFilters,
val settings: Settings,
val xContentRegistry: NamedXContentRegistry,
val transportGetMonitorAction: TransportGetMonitorAction
val xContentRegistry: NamedXContentRegistry
) : HandledTransportAction<ActionRequest, GetAlertsResponse>(
AlertingActions.GET_ALERTS_ACTION_NAME, transportService, actionFilters, ::GetAlertsRequest
),
Expand Down Expand Up @@ -150,30 +147,37 @@ class TransportGetAlertsAction @Inject constructor(
*/
suspend fun resolveAlertsIndexName(getAlertsRequest: GetAlertsRequest): String {
var alertIndex = AlertIndices.ALL_ALERT_INDEX_PATTERN
if (getAlertsRequest.alertIndex.isNullOrEmpty() == false) {
if (!getAlertsRequest.alertIndex.isNullOrEmpty()) {
alertIndex = getAlertsRequest.alertIndex!!
} else if (getAlertsRequest.monitorId.isNullOrEmpty() == false)
withContext(Dispatchers.IO) {
val getMonitorRequest = GetMonitorRequest(
getAlertsRequest.monitorId!!,
-3L,
RestRequest.Method.GET,
FetchSourceContext.FETCH_SOURCE
)
val getMonitorResponse: GetMonitorResponse =
transportGetMonitorAction.client.suspendUntil {
execute(GetMonitorAction.INSTANCE, getMonitorRequest, it)
}
if (getMonitorResponse.monitor != null) {
alertIndex = getMonitorResponse.monitor!!.dataSources.alertsIndex
}
} else if (!getAlertsRequest.monitorId.isNullOrEmpty()) {
val retrievedMonitor = getMonitor(getAlertsRequest)
if (retrievedMonitor != null) {
alertIndex = retrievedMonitor.dataSources.alertsIndex
}
}
return if (alertIndex == AlertIndices.ALERT_INDEX)
AlertIndices.ALL_ALERT_INDEX_PATTERN
else
alertIndex
}

private suspend fun getMonitor(getAlertsRequest: GetAlertsRequest): Monitor? {
val getRequest = GetRequest(ScheduledJob.SCHEDULED_JOBS_INDEX, getAlertsRequest.monitorId!!)
try {
val getResponse: GetResponse = client.suspendUntil { client.get(getRequest, it) }
if (!getResponse.isExists) {
return null
}
val xcp = XContentHelper.createParser(
xContentRegistry, LoggingDeprecationHandler.INSTANCE,
getResponse.sourceAsBytesRef, XContentType.JSON
)
return ScheduledJob.parse(xcp, getResponse.id, getResponse.version) as Monitor
} catch (t: Exception) {
return null
}
}

fun getAlerts(
alertIndex: String,
searchSourceBuilder: SearchSourceBuilder,
Expand Down
44 changes: 44 additions & 0 deletions alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureMonitorRestApiIT.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,50 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
*/

fun `test query all alerts in all states with filter by1`() {
enableFilterBy()
putAlertMappings()
val adminUser = User(ADMIN, listOf(ADMIN), listOf(ALL_ACCESS_ROLE), listOf())
var monitor = createRandomMonitor(refresh = true).copy(user = adminUser)
createAlert(randomAlert(monitor).copy(state = Alert.State.ACKNOWLEDGED))
createAlert(randomAlert(monitor).copy(state = Alert.State.COMPLETED))
createAlert(randomAlert(monitor).copy(state = Alert.State.ERROR))
createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE))
randomAlert(monitor).copy(id = "foobar")

val inputMap = HashMap<String, Any>()
inputMap["missing"] = "_last"
inputMap["monitorId"] = monitor.id

// search as "admin" - must get 4 docs
val adminResponseMap = getAlerts(client(), inputMap).asMap()
assertEquals(4, adminResponseMap["totalAlerts"])

// search as userOne without alerting roles - must return 403 Forbidden
try {
getAlerts(userClient as RestClient, inputMap).asMap()
fail("Expected 403 FORBIDDEN response")
} catch (e: ResponseException) {
assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus())
}
// createUserWithTestDataAndCustomRole(
// user,
// TEST_HR_INDEX,
// TEST_HR_ROLE,
// listOf(ADMIN),
// getClusterPermissionsFromCustomRole(ALERTING_INDEX_MONITOR_ACCESS)
// )
createUserWithRoles(user, listOf(ALERTING_FULL_ACCESS_ROLE), listOf(ADMIN), false)
// add alerting roles and search as userOne - must return 0 docs
// createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, arrayOf(user))
try {
val responseMap = getAlerts(userClient as RestClient, inputMap).asMap()
assertEquals(4, responseMap["totalAlerts"])
} finally {
deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE)
}
}

fun `test get alerts with an user with get alerts role`() {

putAlertMappings()
Expand Down

0 comments on commit ca8851f

Please sign in to comment.