Merge branch '2.10' into 2.10

opensearch-project · Aug 26, 2024 · a64f273 · a64f273
2 parents 72c81ac + 8203ad6
commit a64f273
Show file tree

Hide file tree

Showing 12 changed files with 105 additions and 23 deletions.
diff --git a/...notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt b/...notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt
@@ -56,7 +56,9 @@ object ConfigIndexingActions {
 
     @Suppress("UnusedPrivateMember")
     private fun validateSlackConfig(slack: Slack, user: User?) {
-        // TODO: URL validation with rules
+        require(slack.url.contains(Regex("https://hooks\\.(?:gov-)?slack\\.com/services"))) {
+            "Wrong Slack url. Should contain \"hooks.slack.com/services/\" or \"hooks.gov-slack.com/services/\""
+        }
     }
 
     @Suppress("UnusedPrivateMember")

diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/IntegTestHelpers.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/IntegTestHelpers.kt
@@ -116,7 +116,7 @@ fun getCreateNotificationRequestJsonString(
         .joinToString("")
     val configObjectString = when (configType) {
         ConfigType.SLACK -> """
-            "slack":{"url":"https://slack.domain.com/sample_slack_url#$randomString"}
+            "slack":{"url":"https://hooks.slack.com/services/sample_slack_url#$randomString"}
         """.trimIndent()
         ConfigType.CHIME -> """
             "chime":{"url":"https://chime.domain.com/sample_chime_url#$randomString"}

diff --git a/...ications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt b/...ications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt
@@ -53,7 +53,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_CREATE_CONFIG_ACCESS, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_CREATE_CONFIG_ACCESS])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -96,7 +96,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_NO_ACCESS_ROLE, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_NO_ACCESS_ROLE])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -132,7 +132,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_UPDATE_CONFIG_ACCESS, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_UPDATE_CONFIG_ACCESS])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -209,7 +209,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_NO_ACCESS_ROLE, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_NO_ACCESS_ROLE])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -245,7 +245,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_GET_CONFIG_ACCESS, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_GET_CONFIG_ACCESS])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -301,7 +301,7 @@ class SecurityNotificationIT : PluginRestTestCase() {
         createUserWithCustomRole(user, password, NOTIFICATION_DELETE_CONFIG_ACCESS, "", ROLE_TO_PERMISSION_MAPPING[NOTIFICATION_DELETE_CONFIG_ACCESS])
 
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",

diff --git a/...ons/src/test/kotlin/org/opensearch/integtest/bwc/NotificationsBackwardsCompatibilityIT.kt b/...ons/src/test/kotlin/org/opensearch/integtest/bwc/NotificationsBackwardsCompatibilityIT.kt
@@ -103,7 +103,7 @@ class NotificationsBackwardsCompatibilityIT : PluginRestTestCase() {
                 "description": "This is a sample config description $configId",
                 "config_type": "slack",
                 "is_enabled": true,
-                "slack": { "url": "https://slack.domain.com/sample_slack_url#$configId" }
+                "slack": { "url": "https://hooks.slack.com/services/sample_slack_url#$configId" }
             }
         }
         """.trimIndent()

diff --git a/...ications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt
@@ -142,7 +142,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${referenceObject.description}",
                 "config_type":"chime",
                 "is_enabled":${referenceObject.isEnabled},
-                "slack":{"url":"https://dummy.com"}
+                "slack":{"url":"https://hooks.slack.com/services/sample_slack_url"}
                 "chime":{"url":"${(referenceObject.configData as Chime).url}"}
             }
         }
@@ -190,7 +190,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"this is a updated config description",
                 "config_type":"slack",
                 "is_enabled":"true",
-                "slack":{"url":"https://updated.domain.com/updated_slack_url#0987654321"}
+                "slack":{"url":"https://hooks.slack.com/services/sample_slack_url"}
             }
         }
         """.trimIndent()

diff --git a/...tifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt b/...tifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt
@@ -27,7 +27,7 @@ class CreateNotificationConfigIT : PluginRestTestCase() {
 
     fun `test Create slack notification config`() {
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",

diff --git a/...ications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt
@@ -908,7 +908,7 @@ class EmailNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${smtpAccountConfig.description}",
                 "config_type":"smtp_account",
                 "is_enabled":${smtpAccountConfig.isEnabled},
-                "slack": {"url": "https://dummy.com"},
+                "slack": {"url": "https://hooks.slack.com/services/sample_slack_url"},
                 "smtp_account":{
                     "host":"${sampleSmtpAccount.host}",
                     "port":"${sampleSmtpAccount.port}",
@@ -949,7 +949,7 @@ class EmailNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${emailConfig.description}",
                 "config_type":"email",
                 "is_enabled":${emailConfig.isEnabled},
-                "slack":{"url": "https://dummy.com"},
+                "slack":{"url": "https://hooks.slack.com/services/sample_slack_url"},
                 "email":{
                     "email_account_id":"${sampleEmail.emailAccountID}",
                     "default_recipients":[

diff --git a/...otifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt b/...otifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt
@@ -627,7 +627,7 @@ class QueryNotificationConfigIT : PluginRestTestCase() {
         val urlIds = setOf(slackId, chimeId, microsoftTeamsId, webhookId)
         val recipientIds = setOf(emailGroupId)
         val fromIds = setOf(emailGroupId, smtpAccountId)
-        val domainIds = setOf(slackId, chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
         Thread.sleep(1000)
 
         // Get notification configs using query=slack
@@ -702,7 +702,7 @@ class QueryNotificationConfigIT : PluginRestTestCase() {
         val urlIds = setOf(slackId, chimeId, microsoftTeamsId, webhookId)
         val recipientIds = setOf(emailGroupId)
         val fromIds = setOf(emailGroupId, smtpAccountId)
-        val domainIds = setOf(slackId, chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
         Thread.sleep(1000)
 
         // Get notification configs using text_query=slack should not return any item

diff --git a/...ications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt
@@ -6,11 +6,16 @@
 package org.opensearch.integtest.config
 
 import org.junit.Assert
+import org.opensearch.client.Request
+import org.opensearch.client.RequestOptions
+import org.opensearch.client.ResponseException
 import org.opensearch.commons.notifications.model.ConfigType
 import org.opensearch.commons.notifications.model.NotificationConfig
 import org.opensearch.commons.notifications.model.Slack
 import org.opensearch.core.rest.RestStatus
 import org.opensearch.integtest.PluginRestTestCase
+import org.opensearch.integtest.getResponseBody
+import org.opensearch.integtest.jsonify
 import org.opensearch.notifications.NotificationPlugin.Companion.PLUGIN_BASE_URI
 import org.opensearch.notifications.verifySingleConfigEquals
 import org.opensearch.rest.RestRequest
@@ -19,7 +24,7 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test Create, Get, Update, Delete slack notification config using REST client`() {
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -67,7 +72,7 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
         Thread.sleep(100)
 
         // Updated notification config object
-        val updatedSlack = Slack("https://updated.domain.com/updated_slack_url#0987654321")
+        val updatedSlack = Slack("https://hooks.slack.com/services/updated_slack_url")
         val updatedObject = NotificationConfig(
             "this is a updated config name",
             "this is a updated config description",
@@ -126,7 +131,7 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test Bad Request for multiple config data for Slack using REST Client`() {
         // Create sample config request reference
-        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -155,4 +160,40 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
             RestStatus.BAD_REQUEST.status
         )
     }
+
+    fun `test create config with wrong Slack url and get error text`() {
+        val sampleSlack = Slack("https://webhook.slack.com/services/sample_slack_url")
+        val referenceObject = NotificationConfig(
+            "this is a sample config name",
+            "this is a sample config description",
+            ConfigType.SLACK,
+            isEnabled = true,
+            configData = sampleSlack
+        )
+        val createRequestJsonString = """
+        {
+            "config":{
+                "name":"${referenceObject.name}",
+                "description":"${referenceObject.description}",
+                "config_type":"slack",
+                "is_enabled":${referenceObject.isEnabled},
+                "slack":{"url":"${(referenceObject.configData as Slack).url}"}
+            }
+        }
+        """.trimIndent()
+        val response = try {
+            val request = Request(RestRequest.Method.POST.name, "$PLUGIN_BASE_URI/configs")
+            request.setJsonEntity(createRequestJsonString)
+            val restOptionsBuilder = RequestOptions.DEFAULT.toBuilder()
+            restOptionsBuilder.addHeader("Content-Type", "application/json")
+            request.setOptions(restOptionsBuilder)
+            client().performRequest(request)
+            fail("Expected wrong Slack URL.")
+        } catch (exception: ResponseException) {
+            Assert.assertEquals(
+                "Wrong Slack url. Should contain \"hooks.slack.com/services/\" or \"hooks.gov-slack.com/services/\"",
+                jsonify(getResponseBody(exception.response))["error"].asJsonObject["reason"].asString
+            )
+        }
+    }
 }
diff --git a/...ations/src/test/kotlin/org/opensearch/integtest/config/WebhookNotificationConfigCrudIT.kt b/...ations/src/test/kotlin/org/opensearch/integtest/config/WebhookNotificationConfigCrudIT.kt
@@ -159,7 +159,7 @@ class WebhookNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${referenceObject.description}",
                 "config_type":"webhook",
                 "is_enabled":${referenceObject.isEnabled},
-                "slack":{"url":"https://dummy.com"}
+                "slack":{"url":"https://hooks.slack.com/services/sample_slack_url"}
                 "webhook":{"url":"${(referenceObject.configData as Webhook).url}"}
             }
         }

diff --git a/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt b/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt
@@ -8,6 +8,7 @@ import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.Test
 import org.opensearch.commons.authuser.User
 import org.opensearch.commons.notifications.model.MicrosoftTeams
+import org.opensearch.commons.notifications.model.Slack
 import java.lang.reflect.Method
 import kotlin.test.assertFails
 
@@ -28,8 +29,42 @@ class ConfigIndexingActionsTests {
         assertFails { validateMicrosoftTeamsConfig.invoke(ConfigIndexingActions, microsoftTeams, user) }
     }
 
+    @Test
+    fun `test validate slack`() {
+        val user = User()
+        var slack = Slack("https://hooks.slack.com/services/123456789/123456789/123456789")
+        validateSlackConfig.invoke(ConfigIndexingActions, slack, user)
+        slack = Slack("https://hooks.gov-slack.com/services/123456789/123456789/123456789")
+        validateSlackConfig.invoke(ConfigIndexingActions, slack, user)
+        slack = Slack("https://hooks.slack.com/services/samplesamplesamplesamplesamplesamplesamplesamplesample")
+        validateSlackConfig.invoke(ConfigIndexingActions, slack, user)
+        slack = Slack("https://hooks.gov-slack.com/services/samplesamplesamplesamplesamplesamplesamplesamplesample")
+        validateSlackConfig.invoke(ConfigIndexingActions, slack, user)
+        slack = Slack("http://hooks.slack.com/services/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("http://hooks.gov-slack.com/services/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://slack.com/services/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://gov-slack.com/services/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hooks.slack.com/123456789/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hooks.gov-slack.com/123456789/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hook.slack.com/services/123456789/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hook.gov-slack.com/services/123456789/123456789/123456789/123456789/123456789")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hooks.slack.com/")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+        slack = Slack("https://hooks.gov-slack.com/")
+        assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
+    }
+
     companion object {
         private lateinit var validateMicrosoftTeamsConfig: Method
+        private lateinit var validateSlackConfig: Method
 
         @BeforeAll
         @JvmStatic
@@ -38,8 +73,12 @@ class ConfigIndexingActionsTests {
             validateMicrosoftTeamsConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
                 "validateMicrosoftTeamsConfig", MicrosoftTeams::class.java, User::class.java
             )
+            validateSlackConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
+                "validateSlackConfig", Slack::class.java, User::class.java
+            )
 
             validateMicrosoftTeamsConfig.isAccessible = true
+            validateSlackConfig.isAccessible = true
         }
     }
 }
diff --git a/...ications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt b/...ications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt
@@ -25,7 +25,7 @@ internal class NotificationConfigDocTests {
             createdTimeMs,
             listOf("br1", "br2", "br3")
         )
-        val sampleSlack = Slack("https://domain.com/sample_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val config = NotificationConfig(
             "name",
             "description",
@@ -47,7 +47,7 @@ internal class NotificationConfigDocTests {
             createdTimeMs,
             listOf("br1", "br2", "br3")
         )
-        val sampleSlack = Slack("https://domain.com/sample_url#1234567890")
+        val sampleSlack = Slack("https://hooks.slack.com/services/sample_slack_url")
         val config = NotificationConfig(
             "name",
             "description",
@@ -67,7 +67,7 @@ internal class NotificationConfigDocTests {
                 "description":"description",
                 "config_type":"slack",
                 "is_enabled":true,
-                "slack":{"url":"https://domain.com/sample_url#1234567890"}
+                "slack":{"url":"https://hooks.slack.com/services/sample_slack_url"}
             },
             "extra_field_1":["extra", "value"],
             "extra_field_2":{"extra":"value"},