From 48f38cbd37916ff811e53027bc560ba49e24c19c Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Tue, 10 Sep 2024 16:30:56 -0700 Subject: [PATCH 1/6] Adding Alerting Comments system indices Signed-off-by: Dennis Toepker --- .../src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt index f7383aaab..5256eff51 100644 --- a/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt +++ b/alerting/src/main/kotlin/org/opensearch/alerting/AlertingPlugin.kt @@ -17,6 +17,7 @@ import org.opensearch.alerting.action.SearchEmailGroupAction import org.opensearch.alerting.alerts.AlertIndices import org.opensearch.alerting.alerts.AlertIndices.Companion.ALL_ALERT_INDEX_PATTERN import org.opensearch.alerting.comments.CommentsIndices +import org.opensearch.alerting.comments.CommentsIndices.Companion.ALL_COMMENTS_INDEX_PATTERN import org.opensearch.alerting.core.JobSweeper import org.opensearch.alerting.core.ScheduledJobIndices import org.opensearch.alerting.core.action.node.ScheduledJobsStatsAction @@ -434,7 +435,8 @@ internal class AlertingPlugin : PainlessExtension, ActionPlugin, ScriptPlugin, R override fun getSystemIndexDescriptors(settings: Settings): Collection { return listOf( SystemIndexDescriptor(ALL_ALERT_INDEX_PATTERN, "Alerting Plugin system index pattern"), - SystemIndexDescriptor(SCHEDULED_JOBS_INDEX, "Alerting Plugin Configuration index") + SystemIndexDescriptor(SCHEDULED_JOBS_INDEX, "Alerting Plugin Configuration index"), + SystemIndexDescriptor(ALL_COMMENTS_INDEX_PATTERN, "Alerting Comments system index pattern") ) } From 14c84265a676122f7cc171c4fde54d7274fde1fd Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Wed, 25 Sep 2024 14:59:08 -0700 Subject: [PATCH 2/6] Add security ITs for Alerting Comments Signed-off-by: Dennis Toepker --- .../org/opensearch/alerting/AccessRoles.kt | 1 + .../alerting/AlertingRestTestCase.kt | 129 ++++-- .../resthandler/AlertingCommentsRestApiIT.kt | 63 +-- .../SecureAlertingCommentsRestApiIT.kt | 394 ++++++++++++++++++ .../SecureAlertingNotesRestApiIT.kt | 10 - 5 files changed, 498 insertions(+), 99 deletions(-) create mode 100644 alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt delete mode 100644 alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingNotesRestApiIT.kt diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/AccessRoles.kt b/alerting/src/test/kotlin/org/opensearch/alerting/AccessRoles.kt index 7f415a8ac..d14473884 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/AccessRoles.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/AccessRoles.kt @@ -11,6 +11,7 @@ import org.opensearch.commons.alerting.action.AlertingActions val ALL_ACCESS_ROLE = "all_access" val READALL_AND_MONITOR_ROLE = "readall_and_monitor" val ALERTING_FULL_ACCESS_ROLE = "alerting_full_access" +val ALERTING_ACK_ALERTS_ROLE = "alerting_ack_alerts" val ALERTING_READ_ONLY_ACCESS = "alerting_read_access" val ALERTING_NO_ACCESS_ROLE = "no_access" val ALERTING_GET_EMAIL_ACCOUNT_ACCESS = "alerting_get_email_account_access" diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt index d43a154fc..30ed82b5c 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/AlertingRestTestCase.kt @@ -47,6 +47,7 @@ import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.alerting.model.BucketLevelTrigger import org.opensearch.commons.alerting.model.ChainedAlertTrigger import org.opensearch.commons.alerting.model.Comment +import org.opensearch.commons.alerting.model.Comment.Companion.COMMENT_CONTENT_FIELD import org.opensearch.commons.alerting.model.DocLevelMonitorInput import org.opensearch.commons.alerting.model.DocLevelQuery import org.opensearch.commons.alerting.model.DocumentLevelTrigger @@ -66,6 +67,7 @@ import org.opensearch.core.xcontent.XContentBuilder import org.opensearch.core.xcontent.XContentParser import org.opensearch.core.xcontent.XContentParserUtils import org.opensearch.search.SearchModule +import org.opensearch.search.builder.SearchSourceBuilder import java.net.URLEncoder import java.nio.file.Files import java.time.Instant @@ -524,40 +526,6 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { return alert.copy(id = alertJson["_id"] as String, version = (alertJson["_version"] as Int).toLong()) } - protected fun createAlertComment(alertId: String, content: String): Comment { - val createRequestBody = jsonBuilder() - .startObject() - .field(Comment.COMMENT_CONTENT_FIELD, content) - .endObject() - .string() - - val createResponse = client().makeRequest( - "POST", - "$COMMENTS_BASE_URI/$alertId", - StringEntity(createRequestBody, APPLICATION_JSON) - ) - - assertEquals("Unable to create a new alert", RestStatus.CREATED, createResponse.restStatus()) - - val responseBody = createResponse.asMap() - val commentId = responseBody["_id"] as String - assertNotEquals("response is missing Id", Comment.NO_ID, commentId) - - val comment = responseBody["comment"] as Map<*, *> - - return Comment( - id = commentId, - entityId = comment["entity_id"] as String, - entityType = comment["entity_type"] as String, - content = comment["content"] as String, - createdTime = Instant.ofEpochMilli(comment["created_time"] as Long), - lastUpdatedTime = if (comment["last_updated_time"] != null) { - Instant.ofEpochMilli(comment["last_updated_time"] as Long) - } else null, - user = comment["user"]?.let { User(it as String, emptyList(), emptyList(), emptyList()) } - ) - } - protected fun createRandomMonitor(refresh: Boolean = false, withMetadata: Boolean = false): Monitor { val monitor = randomQueryLevelMonitor(withMetadata = withMetadata) val monitorId = createMonitor(monitor, refresh).id @@ -1889,4 +1857,97 @@ abstract class AlertingRestTestCase : ODFERestTestCase() { } protected fun Workflow.relativeUrl() = "$WORKFLOW_ALERTING_BASE_URI/$id" + + protected fun createAlertComment(alertId: String, content: String, client: RestClient): Comment { + val createRequestBody = jsonBuilder() + .startObject() + .field(COMMENT_CONTENT_FIELD, content) + .endObject() + .string() + + val createResponse = client.makeRequest( + "POST", + "$COMMENTS_BASE_URI/$alertId", + StringEntity(createRequestBody, APPLICATION_JSON) + ) + + assertEquals("Unable to create a new comment", RestStatus.CREATED, createResponse.restStatus()) + + val responseBody = createResponse.asMap() + val commentId = responseBody["_id"] as String + assertNotEquals("response is missing Id", Comment.NO_ID, commentId) + + val comment = responseBody["comment"] as Map<*, *> + + return Comment( + id = commentId, + entityId = comment["entity_id"] as String, + entityType = comment["entity_type"] as String, + content = comment["content"] as String, + createdTime = Instant.ofEpochMilli(comment["created_time"] as Long), + lastUpdatedTime = if (comment["last_updated_time"] != null) { + Instant.ofEpochMilli(comment["last_updated_time"] as Long) + } else null, + user = comment["user"]?.let { User(it as String, emptyList(), emptyList(), emptyList()) } + ) + } + + protected fun updateAlertComment(commentId: String, content: String, client: RestClient): Comment { + val updateRequestBody = jsonBuilder() + .startObject() + .field(COMMENT_CONTENT_FIELD, content) + .endObject() + .string() + + val updateResponse = client.makeRequest( + "PUT", + "$COMMENTS_BASE_URI/$commentId", + StringEntity(updateRequestBody, APPLICATION_JSON) + ) + + assertEquals("Update comment failed", RestStatus.OK, updateResponse.restStatus()) + + val updateResponseBody = updateResponse.asMap() + + val comment = updateResponseBody["comment"] as Map<*, *> + + return Comment( + id = commentId, + entityId = comment["entity_id"] as String, + entityType = comment["entity_type"] as String, + content = comment["content"] as String, + createdTime = Instant.ofEpochMilli(comment["created_time"] as Long), + lastUpdatedTime = if (comment["last_updated_time"] != null) { + Instant.ofEpochMilli(comment["last_updated_time"] as Long) + } else null, + user = comment["user"]?.let { User(it as String, emptyList(), emptyList(), emptyList()) } + ) + } + + protected fun searchAlertComments(query: SearchSourceBuilder, client: RestClient): XContentParser { + val searchResponse = client.makeRequest( + "GET", + "$COMMENTS_BASE_URI/_search", + StringEntity(query.toString(), APPLICATION_JSON) + ) + + val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) + + return xcp + } + + // returns the ID of the delete comment + protected fun deleteAlertComment(commentId: String, client: RestClient): String { + val deleteResponse = client.makeRequest( + "DELETE", + "$COMMENTS_BASE_URI/$commentId" + ) + + assertEquals("Delete comment failed", RestStatus.OK, deleteResponse.restStatus()) + + val deleteResponseBody = deleteResponse.asMap() + val deletedCommentId = deleteResponseBody["_id"] as String + + return deletedCommentId + } } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/AlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/AlertingCommentsRestApiIT.kt index 601a1860c..b59a7ce07 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/AlertingCommentsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/AlertingCommentsRestApiIT.kt @@ -5,24 +5,13 @@ package org.opensearch.alerting.resthandler -import org.apache.hc.core5.http.ContentType -import org.apache.hc.core5.http.io.entity.StringEntity -import org.opensearch.alerting.AlertingPlugin.Companion.COMMENTS_BASE_URI import org.opensearch.alerting.AlertingRestTestCase -import org.opensearch.alerting.makeRequest import org.opensearch.alerting.randomAlert import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_COMMENTS_ENABLED -import org.opensearch.common.xcontent.XContentFactory -import org.opensearch.common.xcontent.XContentType import org.opensearch.commons.alerting.model.Alert -import org.opensearch.commons.alerting.model.Comment.Companion.COMMENT_CONTENT_FIELD -import org.opensearch.commons.alerting.util.string -import org.opensearch.core.rest.RestStatus import org.opensearch.index.query.QueryBuilders import org.opensearch.search.builder.SearchSourceBuilder -import org.opensearch.test.OpenSearchTestCase import org.opensearch.test.junit.annotations.TestLogging -import java.util.concurrent.TimeUnit @TestLogging("level:DEBUG", reason = "Debug for tests.") @Suppress("UNCHECKED_CAST") @@ -36,7 +25,7 @@ class AlertingCommentsRestApiIT : AlertingRestTestCase() { val alertId = alert.id val commentContent = "test comment" - val comment = createAlertComment(alertId, commentContent) + val comment = createAlertComment(alertId, commentContent, client()) assertEquals("Comment does not have correct content", commentContent, comment.content) assertEquals("Comment does not have correct alert ID", alertId, comment.entityId) @@ -50,27 +39,11 @@ class AlertingCommentsRestApiIT : AlertingRestTestCase() { val alertId = alert.id val commentContent = "test comment" - val commentId = createAlertComment(alertId, commentContent).id + val commentId = createAlertComment(alertId, commentContent, client()).id val updateContent = "updated comment" - val updateRequestBody = XContentFactory.jsonBuilder() - .startObject() - .field(COMMENT_CONTENT_FIELD, updateContent) - .endObject() - .string() + val actualContent = updateAlertComment(commentId, updateContent, client()).content - val updateResponse = client().makeRequest( - "PUT", - "$COMMENTS_BASE_URI/$commentId", - StringEntity(updateRequestBody, ContentType.APPLICATION_JSON) - ) - - assertEquals("Update comment failed", RestStatus.OK, updateResponse.restStatus()) - - val updateResponseBody = updateResponse.asMap() - - val comment = updateResponseBody["comment"] as Map<*, *> - val actualContent = comment["content"] as String assertEquals("Comment does not have correct content after update", updateContent, actualContent) } @@ -82,20 +55,11 @@ class AlertingCommentsRestApiIT : AlertingRestTestCase() { val alertId = alert.id val commentContent = "test comment" - createAlertComment(alertId, commentContent) + createAlertComment(alertId, commentContent, client()) - OpenSearchTestCase.waitUntil({ - return@waitUntil false - }, 3, TimeUnit.SECONDS) + val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + val xcp = searchAlertComments(search, client()) - val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()).toString() - val searchResponse = client().makeRequest( - "GET", - "$COMMENTS_BASE_URI/_search", - StringEntity(search, ContentType.APPLICATION_JSON) - ) - - val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) val hits = xcp.map()["hits"]!! as Map> logger.info("hits: $hits") val numberDocsFound = hits["total"]?.get("value") @@ -116,21 +80,10 @@ class AlertingCommentsRestApiIT : AlertingRestTestCase() { val alertId = alert.id val commentContent = "test comment" - val commentId = createAlertComment(alertId, commentContent).id - OpenSearchTestCase.waitUntil({ - return@waitUntil false - }, 3, TimeUnit.SECONDS) - - val deleteResponse = client().makeRequest( - "DELETE", - "$COMMENTS_BASE_URI/$commentId" - ) - - assertEquals("Delete comment failed", RestStatus.OK, deleteResponse.restStatus()) + val commentId = createAlertComment(alertId, commentContent, client()).id - val deleteResponseBody = deleteResponse.asMap() + val deletedCommentId = deleteAlertComment(commentId, client()) - val deletedCommentId = deleteResponseBody["_id"] as String assertEquals("Deleted Comment ID does not match Comment ID in delete request", commentId, deletedCommentId) } diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt new file mode 100644 index 000000000..579be5751 --- /dev/null +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt @@ -0,0 +1,394 @@ +/* + * 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.ALERTING_ACK_ALERTS_ROLE +import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE +import org.opensearch.alerting.ALERTING_READ_ONLY_ACCESS +import org.opensearch.alerting.AlertingRestTestCase +import org.opensearch.alerting.makeRequest +import org.opensearch.alerting.randomAlert +import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_COMMENTS_ENABLED +import org.opensearch.client.ResponseException +import org.opensearch.client.RestClient +import org.opensearch.common.xcontent.XContentType +import org.opensearch.commons.alerting.model.Alert +import org.opensearch.commons.rest.SecureRestClientBuilder +import org.opensearch.core.rest.RestStatus +import org.opensearch.index.query.QueryBuilders +import org.opensearch.search.builder.SearchSourceBuilder + +class SecureAlertingCommentsRestApiIT : AlertingRestTestCase() { + + companion object { + @BeforeClass + @JvmStatic fun setup() { + // things to execute once and keep around for the class + org.junit.Assume.assumeTrue(System.getProperty("security", "false")!!.toBoolean()) + } + } + + val userA = "userA" + val userB = "userB" + var userAClient: RestClient? = null + var userBClient: RestClient? = null + + @Before + fun create() { + if (userAClient == null) { + createUser(userA, arrayOf()) + userAClient = SecureRestClientBuilder(clusterHosts.toTypedArray(), isHttps(), userA, password) + .setSocketTimeout(60000) + .build() + } + if (userBClient == null) { + createUser(userB, arrayOf()) + userBClient = SecureRestClientBuilder(clusterHosts.toTypedArray(), isHttps(), userB, password) + .setSocketTimeout(6000) + .build() + } + client().updateSettings(ALERTING_COMMENTS_ENABLED.key, "true") + } + + @After + fun cleanup() { + userAClient?.close() + userBClient?.close() + deleteUser(userA) + deleteUser(userB) + } + + fun `test user with alerting full access can create comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + createAlertComment(alertId, commentContent, userAClient!!).id + + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + + fun `test user with alerting full access can view comments`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val comment1Content = "test comment 1" + val comment2Content = "test comment 2" + + createAlertComment(alertId, comment1Content, userAClient!!).id + createAlertComment(alertId, comment2Content, userAClient!!).id + + val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + val xcp = searchAlertComments(search, userAClient!!) + + val hits = xcp.map()["hits"]!! as Map> + val numberDocsFound = hits["total"]?.get("value") + assertEquals("Incorrect number of comments found", 2, numberDocsFound) + + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + + fun `test user with alerting full access can edit comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + val updatedContent = "updated comment" + updateAlertComment(commentId, updatedContent, userAClient!!) + + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + + fun `test user with alerting full access can delete comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + deleteAlertComment(commentId, userAClient!!) + + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + + fun `test user with alerting ack alerts can create comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_ACK_ALERTS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + createAlertComment(alertId, commentContent, userAClient!!).id + + deleteRoleMapping(ALERTING_ACK_ALERTS_ROLE) + } + + fun `test user with alerting ack alerts can view comments`() { + createUserWithRoles( + userA, + listOf(ALERTING_ACK_ALERTS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val comment1Content = "test comment 1" + val comment2Content = "test comment 2" + + createAlertComment(alertId, comment1Content, userAClient!!).id + createAlertComment(alertId, comment2Content, userAClient!!).id + + val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + val xcp = searchAlertComments(search, userAClient!!) + + val hits = xcp.map()["hits"]!! as Map> + val numberDocsFound = hits["total"]?.get("value") + assertEquals("Incorrect number of comments found", 2, numberDocsFound) + + deleteRoleMapping(ALERTING_ACK_ALERTS_ROLE) + } + + fun `test user with alerting ack alerts can edit comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_ACK_ALERTS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + val updatedContent = "updated comment" + updateAlertComment(commentId, updatedContent, userAClient!!) + + deleteRoleMapping(ALERTING_ACK_ALERTS_ROLE) + } + + fun `test user with alerting ack alerts can delete comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_ACK_ALERTS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + deleteAlertComment(commentId, userAClient!!) + + deleteRoleMapping(ALERTING_ACK_ALERTS_ROLE) + } + + fun `test user with alerting read access cannot create comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_READ_ONLY_ACCESS), + listOf(), + false + ) + + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + try { + createAlertComment(alertId, commentContent, userAClient!!) + fail("User with read access was able to create comment") + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) + } finally { + deleteRoleMapping(ALERTING_READ_ONLY_ACCESS) + } + } + + fun `test user with alerting read access can view comments`() { + createUserWithRoles( + userA, + listOf(ALERTING_READ_ONLY_ACCESS), + listOf(), + false + ) + createUserWithRoles( + userB, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val comment1Content = "test comment 1" + val comment2Content = "test comment 2" + + createAlertComment(alertId, comment1Content, userBClient!!).id + createAlertComment(alertId, comment2Content, userBClient!!).id + + val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + val xcp = searchAlertComments(search, userAClient!!) + + val hits = xcp.map()["hits"]!! as Map> + val numberDocsFound = hits["total"]?.get("value") + assertEquals("Incorrect number of comments found", 2, numberDocsFound) + + deleteRoleMapping(ALERTING_READ_ONLY_ACCESS) + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + + fun `test user with no roles cannot create comment`() { + createUserWithRoles( + userA, + listOf(), + listOf(), + false + ) + + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + try { + createAlertComment(alertId, commentContent, userAClient!!) + fail("User with no access was able to create comment") + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) + } + } + + fun `test user with no roles cannot view comments`() { + createUserWithRoles( + userA, + listOf(), + listOf(), + false + ) + createUserWithRoles( + userB, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val comment1Content = "test comment 1" + val comment2Content = "test comment 2" + + createAlertComment(alertId, comment1Content, userBClient!!).id + createAlertComment(alertId, comment2Content, userBClient!!).id + + val search = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + + try { + searchAlertComments(search, userAClient!!) + fail("User with no roles was able to view comment") + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) + } finally { + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + } + + fun `test user cannot edit someone else's comment`() { + createUser(userA, listOf().toTypedArray()) + createUser(userB, listOf().toTypedArray()) + createUserRolesMapping(ALERTING_FULL_ACCESS_ROLE, listOf(userA, userB).toTypedArray()) + + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + try { + updateAlertComment(commentId, commentContent, userBClient!!) + fail("User was able to edit someone else's comment") + } catch (e: ResponseException) { + assertEquals("Unexpected status", RestStatus.FORBIDDEN, e.response.restStatus()) + } finally { + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + } + + // TODO: Uncomment and this should pass in CIs +// fun `test user cannot directly search comments system index`() { +// createUserWithRoles( +// userA, +// listOf(ALERTING_FULL_ACCESS_ROLE), +// listOf(), +// false +// ) +// +// val monitor = createRandomMonitor(refresh = true) +// val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) +// val alertId = alert.id +// val commentContent = "test comment" +// +// createAlertComment(alertId, commentContent, userAClient!!).id +// +// val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) +// val searchResponse = userAClient!!.makeRequest( +// "GET", +// ".opensearch-alerting-comments-history-*/_search", +// StringEntity(query.toString(), APPLICATION_JSON) +// ) +// +// val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) +// val hits = xcp.map()["hits"]!! as Map> +// val numberDocsFound = hits["total"]?.get("value") +// assertEquals("User was able to directly inspect alerting comments system index docs", 0, numberDocsFound) +// } +} diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingNotesRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingNotesRestApiIT.kt deleted file mode 100644 index a16f92d3b..000000000 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingNotesRestApiIT.kt +++ /dev/null @@ -1,10 +0,0 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.alerting.resthandler - -import org.opensearch.alerting.AlertingRestTestCase - -class SecureAlertingNotesRestApiIT : AlertingRestTestCase() From 0d4d474a35fd178d1b65f62d1f1ad22937c28a61 Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Thu, 26 Sep 2024 13:21:53 -0700 Subject: [PATCH 3/6] removed unused imports Signed-off-by: Dennis Toepker --- .../alerting/resthandler/SecureAlertingCommentsRestApiIT.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt index 579be5751..1826d8724 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt @@ -5,8 +5,6 @@ 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 @@ -14,12 +12,10 @@ import org.opensearch.alerting.ALERTING_ACK_ALERTS_ROLE import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE import org.opensearch.alerting.ALERTING_READ_ONLY_ACCESS import org.opensearch.alerting.AlertingRestTestCase -import org.opensearch.alerting.makeRequest import org.opensearch.alerting.randomAlert import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_COMMENTS_ENABLED import org.opensearch.client.ResponseException import org.opensearch.client.RestClient -import org.opensearch.common.xcontent.XContentType import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.core.rest.RestStatus From 6161cafac579213601035667ffc017fc1ef64b22 Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Thu, 26 Sep 2024 13:23:54 -0700 Subject: [PATCH 4/6] uncomment system index viewing IT Signed-off-by: Dennis Toepker --- .../SecureAlertingCommentsRestApiIT.kt | 59 ++++++++++--------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt index 1826d8724..1da468485 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt @@ -5,6 +5,8 @@ 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 @@ -12,10 +14,12 @@ import org.opensearch.alerting.ALERTING_ACK_ALERTS_ROLE import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE import org.opensearch.alerting.ALERTING_READ_ONLY_ACCESS import org.opensearch.alerting.AlertingRestTestCase +import org.opensearch.alerting.makeRequest import org.opensearch.alerting.randomAlert import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_COMMENTS_ENABLED import org.opensearch.client.ResponseException import org.opensearch.client.RestClient +import org.opensearch.common.xcontent.XContentType import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.core.rest.RestStatus @@ -359,32 +363,31 @@ class SecureAlertingCommentsRestApiIT : AlertingRestTestCase() { } } - // TODO: Uncomment and this should pass in CIs -// fun `test user cannot directly search comments system index`() { -// createUserWithRoles( -// userA, -// listOf(ALERTING_FULL_ACCESS_ROLE), -// listOf(), -// false -// ) -// -// val monitor = createRandomMonitor(refresh = true) -// val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) -// val alertId = alert.id -// val commentContent = "test comment" -// -// createAlertComment(alertId, commentContent, userAClient!!).id -// -// val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) -// val searchResponse = userAClient!!.makeRequest( -// "GET", -// ".opensearch-alerting-comments-history-*/_search", -// StringEntity(query.toString(), APPLICATION_JSON) -// ) -// -// val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) -// val hits = xcp.map()["hits"]!! as Map> -// val numberDocsFound = hits["total"]?.get("value") -// assertEquals("User was able to directly inspect alerting comments system index docs", 0, numberDocsFound) -// } + fun `test user cannot directly search comments system index`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + createAlertComment(alertId, commentContent, userAClient!!).id + + val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) + val searchResponse = userAClient!!.makeRequest( + "GET", + ".opensearch-alerting-comments-history-*/_search", + StringEntity(query.toString(), APPLICATION_JSON) + ) + + val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) + val hits = xcp.map()["hits"]!! as Map> + val numberDocsFound = hits["total"]?.get("value") + assertEquals("User was able to directly inspect alerting comments system index docs", 0, numberDocsFound) + } } From 3b6d97d77e19d40e558eb257296960c709dba52e Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Thu, 26 Sep 2024 15:20:53 -0700 Subject: [PATCH 5/6] uncommenting system index IT for now Signed-off-by: Dennis Toepker --- .../SecureAlertingCommentsRestApiIT.kt | 62 +++++++++---------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt index 1da468485..5b82a1935 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt @@ -5,8 +5,6 @@ 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 @@ -14,12 +12,10 @@ import org.opensearch.alerting.ALERTING_ACK_ALERTS_ROLE import org.opensearch.alerting.ALERTING_FULL_ACCESS_ROLE import org.opensearch.alerting.ALERTING_READ_ONLY_ACCESS import org.opensearch.alerting.AlertingRestTestCase -import org.opensearch.alerting.makeRequest import org.opensearch.alerting.randomAlert import org.opensearch.alerting.settings.AlertingSettings.Companion.ALERTING_COMMENTS_ENABLED import org.opensearch.client.ResponseException import org.opensearch.client.RestClient -import org.opensearch.common.xcontent.XContentType import org.opensearch.commons.alerting.model.Alert import org.opensearch.commons.rest.SecureRestClientBuilder import org.opensearch.core.rest.RestStatus @@ -363,31 +359,35 @@ class SecureAlertingCommentsRestApiIT : AlertingRestTestCase() { } } - fun `test user cannot directly search comments system index`() { - createUserWithRoles( - userA, - listOf(ALERTING_FULL_ACCESS_ROLE), - listOf(), - false - ) - - val monitor = createRandomMonitor(refresh = true) - val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) - val alertId = alert.id - val commentContent = "test comment" - - createAlertComment(alertId, commentContent, userAClient!!).id - - val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) - val searchResponse = userAClient!!.makeRequest( - "GET", - ".opensearch-alerting-comments-history-*/_search", - StringEntity(query.toString(), APPLICATION_JSON) - ) - - val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) - val hits = xcp.map()["hits"]!! as Map> - val numberDocsFound = hits["total"]?.get("value") - assertEquals("User was able to directly inspect alerting comments system index docs", 0, numberDocsFound) - } + // TODO: this will cause security ITs to fail because the getSystemIndexDescriptors() change + // introduced will not yet be consumed by Security plugin to allow this test to pass. + // Will uncomment this in a later PR once Security plugin has consumed the getSystemIndexDescriptors() + // change +// fun `test user cannot directly search comments system index`() { +// createUserWithRoles( +// userA, +// listOf(ALERTING_FULL_ACCESS_ROLE), +// listOf(), +// false +// ) +// +// val monitor = createRandomMonitor(refresh = true) +// val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) +// val alertId = alert.id +// val commentContent = "test comment" +// +// createAlertComment(alertId, commentContent, userAClient!!).id +// +// val query = SearchSourceBuilder().query(QueryBuilders.matchAllQuery()) +// val searchResponse = userAClient!!.makeRequest( +// "GET", +// ".opensearch-alerting-comments-history-*/_search", +// StringEntity(query.toString(), APPLICATION_JSON) +// ) +// +// val xcp = createParser(XContentType.JSON.xContent(), searchResponse.entity.content) +// val hits = xcp.map()["hits"]!! as Map> +// val numberDocsFound = hits["total"]?.get("value") +// assertEquals("User was able to directly inspect alerting comments system index docs", 0, numberDocsFound) +// } } From 1084528f84ab3ca8bb4d7a0d5e3f023de1692348 Mon Sep 17 00:00:00 2001 From: Dennis Toepker Date: Tue, 1 Oct 2024 10:15:40 -0700 Subject: [PATCH 6/6] adding IT for admin editting someone else's comment Signed-off-by: Dennis Toepker --- .../SecureAlertingCommentsRestApiIT.kt | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt index 5b82a1935..9a92ddec0 100644 --- a/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt +++ b/alerting/src/test/kotlin/org/opensearch/alerting/resthandler/SecureAlertingCommentsRestApiIT.kt @@ -359,6 +359,27 @@ class SecureAlertingCommentsRestApiIT : AlertingRestTestCase() { } } + fun `test admin can edit someone else's comment`() { + createUserWithRoles( + userA, + listOf(ALERTING_FULL_ACCESS_ROLE), + listOf(), + false + ) + + val monitor = createRandomMonitor(refresh = true) + val alert = createAlert(randomAlert(monitor).copy(state = Alert.State.ACTIVE)) + val alertId = alert.id + val commentContent = "test comment" + + val commentId = createAlertComment(alertId, commentContent, userAClient!!).id + + val updatedContent = "updated comment" + updateAlertComment(commentId, updatedContent, client()) + + deleteRoleMapping(ALERTING_FULL_ACCESS_ROLE) + } + // TODO: this will cause security ITs to fail because the getSystemIndexDescriptors() change // introduced will not yet be consumed by Security plugin to allow this test to pass. // Will uncomment this in a later PR once Security plugin has consumed the getSystemIndexDescriptors()