Register the doc-level query and job lock indices as system indices

alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt

@@ -570,7 +570,9 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R
             SystemIndexDescriptor(ALL_ALERT_INDEX_PATTERN, "Alerting Plugin system index pattern"),
             SystemIndexDescriptor(ALL_FINDING_INDEX_PATTERN, "Alerting Findings system index pattern"),
             SystemIndexDescriptor(SCHEDULED_JOBS_INDEX, "Alerting Plugin Configuration index"),
-            SystemIndexDescriptor(ALL_COMMENTS_INDEX_PATTERN, "Alerting Comments system index pattern")
+            SystemIndexDescriptor(ALL_COMMENTS_INDEX_PATTERN, "Alerting Comments system index pattern"),
+            SystemIndexDescriptor("${ScheduledJob.DOC_LEVEL_QUERIES_INDEX}*", "Alerting doc-level query index pattern"),
+            SystemIndexDescriptor(LockService.LOCK_INDEX_NAME, "Alerting job lock index")
         )
     }
 

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

@@ -0,0 +1,52 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.alerting
+
+import org.apache.lucene.util.automaton.Operations
+import org.opensearch.alerting.core.lock.LockService
+import org.opensearch.common.regex.Regex
+import org.opensearch.common.settings.Settings
+import org.opensearch.commons.alerting.model.ScheduledJob
+import org.opensearch.test.OpenSearchTestCase
+
+class AlertingPluginSystemIndicesTests : OpenSearchTestCase() {
+
+    private val descriptors = AlertingPlugin().getSystemIndexDescriptors(Settings.EMPTY)
+
+    fun `test doc-level query index and its rollovers are system indices`() {
+        val alias = ScheduledJob.DOC_LEVEL_QUERIES_INDEX
+        assertEquals(".opensearch-alerting-queries", alias)
+        // the alias name was a concrete index before the query index moved behind an alias
+        listOf(alias, "$alias-000001", "$alias-000002").forEach { assertSingleDescriptorFor(it) }
+    }
+
+    fun `test job lock index is a system index`() {
+        assertEquals(".opensearch-alerting-config-lock", LockService.LOCK_INDEX_NAME)
+        assertSingleDescriptorFor(LockService.LOCK_INDEX_NAME)
+    }
+
+    fun `test query indices that a caller names in data sources are not covered`() {
+        assertTrue(descriptors.none { it.matchesIndexPattern(".opensearch-sap-windows-detectors-queries-000001") })
+    }
+
+    // Core refuses to start a node whose system index patterns overlap, so this mirrors its check.
+    fun `test descriptors do not overlap`() {
+        descriptors.forEach { a ->
+            descriptors.filter { it !== a }.forEach { b ->
+                val intersection = Operations.intersection(
+                    Regex.simpleMatchToAutomaton(a.indexPattern),
+                    Regex.simpleMatchToAutomaton(b.indexPattern)
+                )
+                assertTrue("[${a.indexPattern}] overlaps [${b.indexPattern}]", Operations.isEmpty(intersection))
+            }
+        }
+    }
+
+    private fun assertSingleDescriptorFor(index: String) {
+        val matching = descriptors.filter { it.matchesIndexPattern(index) }
+        assertEquals("descriptors matching [$index]: ${matching.map { it.indexPattern }}", 1, matching.size)
+    }
+}

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

@@ -641,18 +641,18 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() {
         indexDoc(testIndex, "1", testDoc)
         indexDoc(testIndex, "5", testDoc)
         OpenSearchTestCase.waitUntil({
-            val response = client().makeRequest("HEAD", LockService.LOCK_INDEX_NAME)
+            val response = adminClient().makeRequest("HEAD", LockService.LOCK_INDEX_NAME)
             return@waitUntil (response.restStatus().status == 200)
         }, 240, TimeUnit.SECONDS)
 
-        var response = client().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search")
+        var response = adminClient().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search")
         var responseMap = entityAsMap(response)
         var noOfLocks = ((responseMap["hits"] as Map<String, Any>)["hits"] as List<Any>).size
         assertEquals(1, noOfLocks)
 
         deleteMonitor(monitor)
         refreshIndex(LockService.LOCK_INDEX_NAME)
-        response = client().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search")
+        response = adminClient().makeRequest("GET", LockService.LOCK_INDEX_NAME + "/_search")
         responseMap = entityAsMap(response)
         noOfLocks = ((responseMap["hits"] as Map<String, Any>)["hits"] as List<Any>).size
         assertEquals(0, noOfLocks)
@@ -3070,7 +3070,7 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() {
 
         val currTimeStampMinusTenMinutes = System.currentTimeMillis() - 600000L
         val lock = "{\"scheduled_job_id\":\"${monitor.id}\",\"lock_time\":${currTimeStampMinusTenMinutes / 1000},\"released\":false}"
-        updateDoc(client(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock)
+        updateDoc(adminClient(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock)
 
         updateMonitor(monitor.copy(enabled = true), true)
 
@@ -3129,7 +3129,7 @@ class DocumentMonitorRunnerIT : AlertingRestTestCase() {
 
         val currTimeStamp = System.currentTimeMillis()
         val lock = "{\"scheduled_job_id\":\"${monitor.id}\",\"lock_time\":$currTimeStamp,\"released\":false}"
-        updateDoc(client(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock)
+        updateDoc(adminClient(), LockService.LOCK_INDEX_NAME, "${monitor.id}-lock", lock)
 
         indexDoc(testIndex, "4", testDoc)
         indexDoc(testIndex, "5", testDoc)

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

@@ -48,7 +48,6 @@ import org.opensearch.alerting.randomTemplateScript
 import org.opensearch.client.Response
 import org.opensearch.client.ResponseException
 import org.opensearch.client.RestClient
-import org.opensearch.common.settings.Settings
 import org.opensearch.common.xcontent.LoggingDeprecationHandler
 import org.opensearch.common.xcontent.XContentType
 import org.opensearch.common.xcontent.json.JsonXContent
@@ -2670,25 +2669,27 @@ class SecureMonitorRestApiIT : AlertingRestTestCase() {
             false
         )
         val docLevelQueryIndex = ".opensearch-alerting-queries-000001"
-        createIndex(
-            docLevelQueryIndex, Settings.EMPTY,
-            """
-                 "properties" : {
-                  "query": {
-                              "type": "percolator_ext"
-                            },
-                            "monitor_id": {
-                              "type": "text"
-                            },
-                            "index": {
-                              "type": "text"
-                            }
-                }
+        // the query index is a system index, so create and close it with the admin certificate; closing it as a user is refused
+        adminClient().makeRequest(
+            "PUT", docLevelQueryIndex,
+            StringEntity(
+                """
+                {
+                  "mappings": {
+                    "properties": {
+                      "query": { "type": "percolator_ext" },
+                      "monitor_id": { "type": "text" },
+                      "index": { "type": "text" }
+                    }
+                  },
+                  "aliases": { ".opensearch-alerting-queries": {} }
                 }
-            """.trimIndent(),
-            ".opensearch-alerting-queries"
+                """.trimIndent(),
+                ContentType.APPLICATION_JSON
+            )
         )
-        closeIndex(docLevelQueryIndex) // close index to simulate doc level query indexing failure
+        // close index to simulate doc level query indexing failure
+        adminClient().makeRequest("POST", "$docLevelQueryIndex/_close")
         try {
             val monitor = randomDocumentLevelMonitor(
                 withMetadata = false,

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

@@ -0,0 +1,128 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.alerting.resthandler
+
+import org.apache.hc.core5.http.ContentType.APPLICATION_JSON
+import org.apache.hc.core5.http.io.entity.StringEntity
+import org.junit.After
+import org.junit.Before
+import org.junit.BeforeClass
+import org.opensearch.alerting.AlertingRestTestCase
+import org.opensearch.alerting.core.lock.LockService.Companion.LOCK_INDEX_NAME
+import org.opensearch.alerting.makeRequest
+import org.opensearch.alerting.randomDocumentLevelMonitor
+import org.opensearch.client.ResponseException
+import org.opensearch.client.RestClient
+import org.opensearch.common.xcontent.XContentType
+import org.opensearch.commons.alerting.model.DocLevelMonitorInput
+import org.opensearch.commons.alerting.model.DocLevelQuery
+import org.opensearch.commons.alerting.model.ScheduledJob.Companion.DOC_LEVEL_QUERIES_INDEX
+import org.opensearch.commons.rest.SecureRestClientBuilder
+import org.opensearch.core.rest.RestStatus
+import org.opensearch.index.query.QueryBuilders
+import org.opensearch.search.builder.SearchSourceBuilder
+
+/**
+ * The doc-level query indices and the job lock index are system indices, so a user can't read or write them directly, even
+ * with index permissions on every index.
+ */
+class SecureQueryAndLockIndexRestApiIT : AlertingRestTestCase() {
+
+    companion object {
+        @BeforeClass
+        @JvmStatic fun setup() {
+            org.junit.Assume.assumeTrue(System.getProperty("security", "false")!!.toBoolean())
+        }
+    }
+
+    private val user = "queryLockUser"
+    private val role = "query_lock_all_index_role"
+    private var userClient: RestClient? = null
+
+    @Before
+    fun create() {
+        createUser(user, arrayOf())
+        // crud on every index plus the bulk permission a document write needs, so only the system index protection keeps the
+        // user out of these indices
+        createCustomIndexRole(role, "*", "indices:data/write/bulk")
+        createUserRolesMapping(role, arrayOf(user))
+        userClient = SecureRestClientBuilder(clusterHosts.toTypedArray(), isHttps(), user, password)
+            .setSocketTimeout(60000)
+            .build()
+    }
+
+    @After
+    fun cleanup() {
+        userClient?.close()
+        deleteRoleAndRoleMapping(role)
+        deleteUser(user)
+    }
+
+    fun `test user cannot directly search doc-level query system index`() {
+        createDocLevelMonitorWithQuery()
+        assertSearchHits(adminClient(), "$DOC_LEVEL_QUERIES_INDEX*", expectAny = true)
+
+        // The security plugin leaves the system index out of the user's search, so it succeeds with no hits rather than failing
+        assertSearchHits(userClient!!, "$DOC_LEVEL_QUERIES_INDEX*", expectAny = false)
+    }
+
+    fun `test user cannot directly write to doc-level query system index`() {
+        createDocLevelMonitorWithQuery()
+
+        assertWriteForbidden(DOC_LEVEL_QUERIES_INDEX)
+    }
+
+    fun `test user cannot directly search job lock system index`() {
+        createLock()
+        assertSearchHits(adminClient(), LOCK_INDEX_NAME, expectAny = true)
+
+        assertSearchHits(userClient!!, LOCK_INDEX_NAME, expectAny = false)
+    }
+
+    fun `test user cannot directly write to job lock system index`() {
+        createLock()
+
+        assertWriteForbidden(LOCK_INDEX_NAME)
+    }
+
+    private fun createDocLevelMonitorWithQuery() {
+        val testIndex = createTestIndex()
+        val docQuery = DocLevelQuery(query = "test_field:\"us-west-2\"", name = "3", fields = listOf())
+        createMonitor(
+            randomDocumentLevelMonitor(
+                inputs = listOf(DocLevelMonitorInput("description", listOf(testIndex), listOf(docQuery))),
+                triggers = listOf(),
+                enabled = false
+            )
+        )
+    }
+
+    private fun createLock() {
+        indexDocWithAdminClient(LOCK_INDEX_NAME, "test-lock", """{"scheduled_job_id": "test", "lock_time": 1, "released": true}""")
+    }
+
+    private fun assertSearchHits(client: RestClient, index: String, expectAny: Boolean) {
+        adminClient().makeRequest("POST", "/$index/_refresh?expand_wildcards=all")
+        val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()).toString()
+        val response = client.makeRequest("GET", "$index/_search", StringEntity(query, APPLICATION_JSON))
+        assertEquals("Unexpected status", RestStatus.OK, response.restStatus())
+        val hits = createParser(XContentType.JSON.xContent(), response.entity.content).map()["hits"] as Map<*, *>
+        val numberDocsFound = (hits["total"] as Map<*, *>)["value"] as Int
+        assertEquals("Docs found in [$index]: $numberDocsFound", expectAny, numberDocsFound > 0)
+    }
+
+    private fun assertWriteForbidden(index: String) {
+        // control: the same user can write the same document to an ordinary index, so the 403 below comes from the system index
+        val doc = """{"monitor_id": "fake"}"""
+        val control = userClient!!.makeRequest("POST", "${createTestIndex()}/_doc", StringEntity(doc, APPLICATION_JSON))
+        assertEquals("Unexpected status", RestStatus.CREATED, control.restStatus())
+
+        val e = expectThrows(ResponseException::class.java) {
+            userClient!!.makeRequest("POST", "$index/_doc", StringEntity(doc, APPLICATION_JSON))
+        }
+        assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus())
+    }
+}