Support to specify backend roles for monitors (opensearch-project#635) (

opensearch-project#643)

* Support specify backend roles for monitors

Signed-off-by: Ashish Agrawal <[email protected]>
(cherry picked from commit f986238c477132611f15859c2e68da9cd0acfdf1)

Co-authored-by: Ashish Agrawal <[email protected]>

.github/workflows/security-test-workflow.yml

@@ -43,7 +43,12 @@ jobs:
           plugin_version=`echo $plugin|awk -F- '{print $3}'| cut -d. -f 1-4`
           qualifier=`echo $plugin|awk -F- '{print $4}'| cut -d. -f 1-1`
           candidate_version=`echo $plugin|awk -F- '{print $5}'| cut -d. -f 1-1`
-          docker_version=$version-$qualifier
+          if qualifier
+          then
+            docker_version=$version-$qualifier
+          else
+            docker_version=$version
+          fi
 
           [[ -z $candidate_version ]] && candidate_version=$qualifier && qualifier=""
 

alerting/src/main/kotlin/org/opensearch/alerting/resthandler/RestIndexMonitorAction.kt

@@ -84,6 +84,8 @@ class RestIndexMonitorAction : BaseRestHandler() {
         val xcp = request.contentParser()
         ensureExpectedToken(Token.START_OBJECT, xcp.nextToken(), xcp)
         val monitor = Monitor.parse(xcp, id).copy(lastUpdateTime = Instant.now())
+        val rbacRoles = request.contentParser().map()["rbac_roles"] as List<String>?
+
         validateDataSources(monitor)
         validateOwner(monitor.owner)
         val monitorType = monitor.monitorType
@@ -118,7 +120,7 @@ class RestIndexMonitorAction : BaseRestHandler() {
         } else {
             WriteRequest.RefreshPolicy.IMMEDIATE
         }
-        val indexMonitorRequest = IndexMonitorRequest(id, seqNo, primaryTerm, refreshPolicy, request.method(), monitor)
+        val indexMonitorRequest = IndexMonitorRequest(id, seqNo, primaryTerm, refreshPolicy, request.method(), monitor, rbacRoles)
 
         return RestChannelConsumer { channel ->
             client.execute(AlertingActions.INDEX_MONITOR_ACTION_TYPE, indexMonitorRequest, indexMonitorResponse(channel, request.method()))

alerting/src/main/kotlin/org/opensearch/alerting/transport/SecureTransportAction.kt

@@ -61,7 +61,7 @@ interface SecureTransportAction {
     /**
      *  'all_access' role users are treated as admins.
      */
-    private fun isAdmin(user: User?): Boolean {
+    fun isAdmin(user: User?): Boolean {
         return when {
             user == null -> {
                 false

alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportIndexMonitorAction.kt

@@ -123,6 +123,40 @@ class TransportIndexMonitorAction @Inject constructor(
             return
         }
 
+        if (
+            user != null &&
+            !isAdmin(user) &&
+            transformedRequest.rbacRoles != null
+        ) {
+            if (transformedRequest.rbacRoles?.stream()?.anyMatch { !user.backendRoles.contains(it) } == true) {
+                log.debug(
+                    "User specified backend roles, ${transformedRequest.rbacRoles}, " +
+                        "that they don' have access to. User backend roles: ${user.backendRoles}"
+                )
+                actionListener.onFailure(
+                    AlertingException.wrap(
+                        OpenSearchStatusException(
+                            "User specified backend roles that they don't have access to. Contact administrator", RestStatus.FORBIDDEN
+                        )
+                    )
+                )
+                return
+            } else if (transformedRequest.rbacRoles?.isEmpty() == true) {
+                log.debug(
+                    "Non-admin user are not allowed to specify an empty set of backend roles. " +
+                        "Please don't pass in the parameter or pass in at least one backend role."
+                )
+                actionListener.onFailure(
+                    AlertingException.wrap(
+                        OpenSearchStatusException(
+                            "Non-admin user are not allowed to specify an empty set of backend roles.", RestStatus.FORBIDDEN
+                        )
+                    )
+                )
+                return
+            }
+        }
+
         if (!isADMonitor(transformedRequest.monitor)) {
             checkIndicesAndExecute(client, actionListener, transformedRequest, user)
         } else {
@@ -405,6 +439,19 @@ class TransportIndexMonitorAction @Inject constructor(
         private suspend fun indexMonitor() {
             var metadata = createMetadata()
 
+            if (user != null) {
+                // Use the backend roles which is an intersection of the requested backend roles and the user's backend roles.
+                // Admins can pass in any backend role. Also if no backend role is passed in, all the user's backend roles are used.
+                val rbacRoles = if (request.rbacRoles == null) user.backendRoles.toSet()
+                else if (!isAdmin(user)) request.rbacRoles?.intersect(user.backendRoles)?.toSet()
+                else request.rbacRoles
+
+                request.monitor = request.monitor.copy(
+                    user = User(user.name, rbacRoles.orEmpty().toList(), user.roles, user.customAttNames)
+                )
+                log.debug("Created monitor's backend roles: $rbacRoles")
+            }
+
             val indexRequest = IndexRequest(SCHEDULED_JOBS_INDEX)
                 .setRefreshPolicy(request.refreshPolicy)
                 .source(request.monitor.toXContentWithUser(jsonBuilder(), ToXContent.MapParams(mapOf("with_type" to "true"))))
@@ -499,6 +546,42 @@ class TransportIndexMonitorAction @Inject constructor(
             if (request.monitor.enabled && currentMonitor.enabled)
                 request.monitor = request.monitor.copy(enabledTime = currentMonitor.enabledTime)
 
+            /**
+             * On update monitor check which backend roles to associate to the monitor.
+             * Below are 2 examples of how the logic works
+             *
+             * Example 1, say we have a Monitor with backend roles [a, b, c, d] associated with it.
+             * If I'm User A (non-admin user) and I have backend roles [a, b, c] associated with me and I make a request to update
+             * the Monitor's backend roles to [a, b]. This would mean that the roles to remove are [c] and the roles to add are [a, b].
+             * The Monitor's backend roles would then be [a, b, d].
+             *
+             * Example 2, say we have a Monitor with backend roles [a, b, c, d] associated with it.
+             * If I'm User A (admin user) and I have backend roles [a, b, c] associated with me and I make a request to update
+             * the Monitor's backend roles to [a, b]. This would mean that the roles to remove are [c, d] and the roles to add are [a, b].
+             * The Monitor's backend roles would then be [a, b].
+             */
+            if (user != null) {
+                if (request.rbacRoles != null) {
+                    if (isAdmin(user)) {
+                        request.monitor = request.monitor.copy(
+                            user = User(user.name, request.rbacRoles, user.roles, user.customAttNames)
+                        )
+                    } else {
+                        // rolesToRemove: these are the backend roles to remove from the monitor
+                        val rolesToRemove = user.backendRoles - request.rbacRoles.orEmpty()
+                        // remove the monitor's roles with rolesToRemove and add any roles passed into the request.rbacRoles
+                        val updatedRbac = currentMonitor.user?.backendRoles.orEmpty() - rolesToRemove + request.rbacRoles.orEmpty()
+                        request.monitor = request.monitor.copy(
+                            user = User(user.name, updatedRbac, user.roles, user.customAttNames)
+                        )
+                    }
+                } else {
+                    request.monitor = request.monitor
+                        .copy(user = User(user.name, currentMonitor.user!!.backendRoles, user.roles, user.customAttNames))
+                }
+                log.debug("Update monitor backend roles to: ${request.monitor.user?.backendRoles}")
+            }
+
             request.monitor = request.monitor.copy(schemaVersion = IndexUtils.scheduledJobIndexSchemaVersion)
             val indexRequest = IndexRequest(SCHEDULED_JOBS_INDEX)
                 .setRefreshPolicy(request.refreshPolicy)

alerting/src/test/kotlin/org/opensearch/alerting/AccessRoles.kt

@@ -6,6 +6,7 @@
 package org.opensearch.alerting
 
 val ALL_ACCESS_ROLE = "all_access"
+val READALL_AND_MONITOR_ROLE = "readall_and_monitor"
 val ALERTING_FULL_ACCESS_ROLE = "alerting_full_access"
 val ALERTING_READ_ONLY_ACCESS = "alerting_read_access"
 val ALERTING_NO_ACCESS_ROLE = "no_access"

alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt

@@ -106,10 +106,26 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
         return entityAsMap(this)
     }
 
-    protected fun createMonitorWithClient(client: RestClient, monitor: Monitor, refresh: Boolean = true): Monitor {
+    private fun createMonitorEntityWithBackendRoles(monitor: Monitor, rbacRoles: List<String>?): HttpEntity {
+        if (rbacRoles == null) {
+            return monitor.toHttpEntity()
+        }
+        val temp = monitor.toJsonString()
+        val toReplace = temp.lastIndexOf("}")
+        val rbacString = rbacRoles.joinToString { "\"$it\"" }
+        val jsonString = temp.substring(0, toReplace) + ", \"rbac_roles\": [$rbacString] }"
+        return StringEntity(jsonString, APPLICATION_JSON)
+    }
+
+    protected fun createMonitorWithClient(
+        client: RestClient,
+        monitor: Monitor,
+        rbacRoles: List<String>? = null,
+        refresh: Boolean = true
+    ): Monitor {
         val response = client.makeRequest(
             "POST", "$ALERTING_BASE_URI?refresh=$refresh", emptyMap(),
-            monitor.toHttpEntity()
+            createMonitorEntityWithBackendRoles(monitor, rbacRoles)
         )
         assertEquals("Unable to create a new monitor", RestStatus.CREATED, response.restStatus())
 
@@ -123,7 +139,7 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
     }
 
     protected fun createMonitor(monitor: Monitor, refresh: Boolean = true): Monitor {
-        return createMonitorWithClient(client(), monitor, refresh)
+        return createMonitorWithClient(client(), monitor, emptyList(), refresh)
     }
 
     protected fun deleteMonitor(monitor: Monitor, refresh: Boolean = true): Response {
@@ -499,6 +515,21 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
         return getMonitor(monitorId = monitor.id)
     }
 
+    protected fun updateMonitorWithClient(
+        client: RestClient,
+        monitor: Monitor,
+        rbacRoles: List<String> = emptyList(),
+        refresh: Boolean = true
+    ): Monitor {
+        val response = client.makeRequest(
+            "PUT", "${monitor.relativeUrl()}?refresh=$refresh",
+            emptyMap(), createMonitorEntityWithBackendRoles(monitor, rbacRoles)
+        )
+        assertEquals("Unable to update a monitor", RestStatus.OK, response.restStatus())
+        assertUserNull(response.asMap()["monitor"] as Map<String, Any>)
+        return getMonitor(monitorId = monitor.id)
+    }
+
     protected fun getMonitor(monitorId: String, header: BasicHeader = BasicHeader(HttpHeaders.CONTENT_TYPE, "application/json")): Monitor {
         val response = client().makeRequest("GET", "$ALERTING_BASE_URI/$monitorId", null, header)
         assertEquals("Unable to get monitor $monitorId", RestStatus.OK, response.restStatus())
@@ -1089,6 +1120,18 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
         client().performRequest(request)
     }
 
+    fun patchUserBackendRoles(name: String, backendRoles: Array<String>) {
+        val request = Request("PATCH", "/_plugins/_security/api/internalusers/$name")
+        val broles = backendRoles.joinToString { "\"$it\"" }
+        var entity = " [{\n" +
+            "\"op\": \"replace\",\n" +
+            "\"path\": \"/backend_roles\",\n" +
+            "\"value\": [$broles]\n" +
+            "}]"
+        request.setJsonEntity(entity)
+        client().performRequest(request)
+    }
+
     fun createIndexRole(name: String, index: String) {
         val request = Request("PUT", "/_plugins/_security/api/roles/$name")
         var entity = "{\n" +
@@ -1174,6 +1217,22 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
         client().performRequest(request)
     }
 
+    fun updateRoleMapping(role: String, users: List<String>, addUser: Boolean) {
+        val request = Request("PATCH", "/_plugins/_security/api/rolesmapping/$role")
+        val usersStr = users.joinToString { it -> "\"$it\"" }
+
+        val op = if (addUser) "add" else "remove"
+
+        val entity = "[{\n" +
+            "  \"op\" : \"$op\",\n" +
+            "  \"path\" : \"/users\",\n" +
+            "  \"value\" : [$usersStr]\n" +
+            "}]"
+
+        request.setJsonEntity(entity)
+        client().performRequest(request)
+    }
+
     fun deleteUser(name: String) {
         client().makeRequest("DELETE", "/_plugins/_security/api/internalusers/$name")
     }
@@ -1202,15 +1261,31 @@ abstract class AlertingRestTestCase : ODFERestTestCase() {
         user: String,
         index: String,
         role: String,
-        backendRole: String,
+        backendRoles: List<String>,
         clusterPermissions: String?
     ) {
-        createUser(user, user, arrayOf(backendRole))
+        createUser(user, user, backendRoles.toTypedArray())
         createTestIndex(index)
         createCustomIndexRole(role, index, clusterPermissions)
         createUserRolesMapping(role, arrayOf(user))
     }
 
+    fun createUserWithRoles(
+        user: String,
+        roles: List<String>,
+        backendRoles: List<String>,
+        isExistingRole: Boolean
+    ) {
+        createUser(user, user, backendRoles.toTypedArray())
+        for (role in roles) {
+            if (isExistingRole) {
+                updateRoleMapping(role, listOf(user), true)
+            } else {
+                createUserRolesMapping(role, arrayOf(user))
+            }
+        }
+    }
+
     fun createUserWithDocLevelSecurityTestData(
         user: String,
         index: String,

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureDestinationRestApiIT.kt

@@ -139,7 +139,7 @@ class SecureDestinationRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_GET_DESTINATION_ACCESS)
         )
 

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailAccountRestApiIT.kt

@@ -76,7 +76,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_GET_EMAIL_ACCOUNT_ACCESS)
         )
 
@@ -105,7 +105,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_SEARCH_EMAIL_ACCOUNT_ACCESS)
         )
 
@@ -132,7 +132,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE)
         )
 
@@ -162,7 +162,7 @@ class SecureEmailAccountRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_NO_ACCESS_ROLE)
         )
 

alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureEmailGroupsRestApiIT.kt

@@ -78,7 +78,7 @@ class SecureEmailGroupsRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_GET_EMAIL_GROUP_ACCESS)
         )
 
@@ -105,7 +105,7 @@ class SecureEmailGroupsRestApiIT : AlertingRestTestCase() {
             user,
             TEST_HR_INDEX,
             TEST_HR_ROLE,
-            TEST_HR_BACKEND_ROLE,
+            listOf(TEST_HR_BACKEND_ROLE),
             getClusterPermissionsFromCustomRole(ALERTING_SEARCH_EMAIL_GROUP_ACCESS)
         )