Adds webhook format check for Chime (opensearch-project#816)

* Add regex for Chime webhook URLs. Signed-off-by: Aniruddh Srivastava <[email protected]> * Update tests for new Chime webhook regex. Signed-off-by: Aniruddh Srivastava <[email protected]> * Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava <[email protected]> * Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava <[email protected]> * Linting Signed-off-by: Aniruddh Srivastava <[email protected]> * Actually make the Chime URL wrong. Signed-off-by: Aniruddh Srivastava <[email protected]> --------- Signed-off-by: Aniruddh Srivastava <[email protected]>
Noir01 · Nov 28, 2023 · 1163d54 · 1163d54
1 parent 87109a0
commit 1163d54
Show file tree

Hide file tree

Showing 10 changed files with 84 additions and 16 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
@@ -63,7 +63,9 @@ object ConfigIndexingActions {
 
     @Suppress("UnusedPrivateMember")
     private fun validateChimeConfig(chime: Chime, user: User?) {
-        // TODO: URL validation with rules
+        require(chime.url.contains(Regex("https://hooks\\.chime\\.aws/incomingwebhooks/.*\\?token="))) {
+            "Wrong Chime url. Should contain \"hooks.chime.aws/incomingwebhooks/\" and \"?token=\""
+        }
     }
 
     private fun validateMicrosoftTeamsConfig(microsoftTeams: MicrosoftTeams, user: User?) {

diff --git a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/IntegTestHelpers.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/IntegTestHelpers.kt
@@ -119,7 +119,7 @@ fun getCreateNotificationRequestJsonString(
             "slack":{"url":"https://hooks.slack.com/services/sample_slack_url#$randomString"}
         """.trimIndent()
         ConfigType.CHIME -> """
-            "chime":{"url":"https://chime.domain.com/sample_chime_url#$randomString"}
+            "chime":{"url":"https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=$randomString"}
         """.trimIndent()
         ConfigType.MICROSOFT_TEAMS -> """
             "microsoft_teams":{"url":"https://microsoftTeams.domain.webhook.office.com/sample_microsoft_teams_url#$randomString"}

diff --git a/...ications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt
@@ -5,11 +5,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.Chime
 import org.opensearch.commons.notifications.model.ConfigType
 import org.opensearch.commons.notifications.model.NotificationConfig
 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
@@ -18,7 +23,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test Create, Get, Update, Delete chime notification config using REST client`() {
         // Create sample config request reference
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -66,7 +71,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
         Thread.sleep(100)
 
         // Updated notification config object
-        val updatedChime = Chime("https://updated.domain.com/updated_chime_url#0987654321")
+        val updatedChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=654321")
         val updatedObject = NotificationConfig(
             "this is a updated config name",
             "this is a updated config description",
@@ -125,7 +130,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test BAD Request for multiple config data for Chime using REST Client`() {
         // Create sample config request reference
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -157,7 +162,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test update existing config to different config type`() {
         // Create sample config request reference
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -245,7 +250,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
 
     fun `test update Chime webhook URL`() {
         // Create sample config request reference
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -278,7 +283,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"this is a updated config description",
                 "config_type":"chime",
                 "is_enabled":"true",
-                "chime":{"url":"https://updated.domain.com/updated_chime_url#0987654321"}
+                "chime":{"url":"https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=654321"}
             }
         }
         """.trimIndent()
@@ -307,4 +312,40 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() {
             RestStatus.INTERNAL_SERVER_ERROR.status
         )
     }
+
+    fun `test create config with wrong Chime url and get error text`() {
+        val sampleChime = Chime("https://hook.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
+        val referenceObject = NotificationConfig(
+            "this is a sample config name",
+            "this is a sample config description",
+            ConfigType.CHIME,
+            isEnabled = true,
+            configData = sampleChime
+        )
+        val createRequestJsonString = """
+        {
+            "config":{
+                "name":"${referenceObject.name}",
+                "description":"${referenceObject.description}",
+                "config_type":"chime",
+                "is_enabled":${referenceObject.isEnabled},
+                "chime":{"url":"${(referenceObject.configData as Chime).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 Chime URL.")
+        } catch (exception: ResponseException) {
+            Assert.assertEquals(
+                "Wrong Chime url. Should contain \"hooks.chime.aws/incomingwebhooks/\" and \"?token=\"",
+                jsonify(getResponseBody(exception.response))["error"].asJsonObject["reason"].asString
+            )
+        }
+    }
 }
diff --git a/...tifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt b/...tifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt
@@ -66,7 +66,7 @@ class CreateNotificationConfigIT : PluginRestTestCase() {
     fun `test Create chime notification config with ID`() {
         // Create sample config request reference
         val configId = "sample_config_id"
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",

diff --git a/...src/test/kotlin/org/opensearch/integtest/config/MicrosoftTeamsNotificationConfigCrudIT.kt b/...src/test/kotlin/org/opensearch/integtest/config/MicrosoftTeamsNotificationConfigCrudIT.kt
@@ -184,7 +184,7 @@ class MicrosoftTeamsNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${referenceObject.description}",
                 "config_type":"microsoft_teams",
                 "is_enabled":${referenceObject.isEnabled},
-                "chime":{"url":"https://dummy.com"},
+                "chime":{"url":"https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456"},
                 "microsoft_teams":{"url":"${(referenceObject.configData as MicrosoftTeams).url}"}
             }
         }

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(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(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(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(microsoftTeamsId, webhookId, smtpAccountId)
         Thread.sleep(1000)
 
         // Get notification configs using text_query=slack should not return any item
@@ -780,7 +780,7 @@ class QueryNotificationConfigIT : PluginRestTestCase() {
         Thread.sleep(1000)
 
         // Create sample config request reference
-        val sampleChime = Chime("https://domain.com/sample_chime_url#1234567890")
+        val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
         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/SlackNotificationConfigCrudIT.kt b/...ications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt
@@ -148,7 +148,7 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${referenceObject.description}",
                 "config_type":"slack",
                 "is_enabled":${referenceObject.isEnabled},
-                "chime":{"url":"https://dummy.com"}
+                "chime":{"url":"https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456"}
                 "slack":{"url":"${(referenceObject.configData as Slack).url}
             }
         }

diff --git a/...ifications/src/test/kotlin/org/opensearch/integtest/config/SnsNotificationConfigCrudIT.kt b/...ifications/src/test/kotlin/org/opensearch/integtest/config/SnsNotificationConfigCrudIT.kt
@@ -143,7 +143,7 @@ class SnsNotificationConfigCrudIT : PluginRestTestCase() {
                 "description":"${referenceObject.description}",
                 "config_type":"sns",
                 "is_enabled":${referenceObject.isEnabled},
-                "chime":{"url":"https://dummy.com"}
+                "chime":{"url":"https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456"}
                 "sns":{"topic_arn":"${(referenceObject.configData as Sns).topicArn}","role_arn":"${(referenceObject.configData as Sns).roleArn}"}
             }
         }

diff --git a/...tifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt b/...tifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt
@@ -25,7 +25,7 @@ internal class SendTestMessageRestHandlerIT : PluginRestTestCase() {
                 "config_type":"chime",
                 "is_enabled":true,
                 "chime":{
-                    "url":"https://hooks.chime.aws/incomingwebhooks/xxxx"
+                    "url":"https://hooks.chime.aws/incomingwebhooks/xxxx?token=xxxx"
                 }
             }
         }

diff --git a/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt b/...ications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt
@@ -7,6 +7,7 @@ package org.opensearch.notifications.index
 import org.junit.jupiter.api.BeforeAll
 import org.junit.jupiter.api.Test
 import org.opensearch.commons.authuser.User
+import org.opensearch.commons.notifications.model.Chime
 import org.opensearch.commons.notifications.model.MicrosoftTeams
 import org.opensearch.commons.notifications.model.Slack
 import java.lang.reflect.Method
@@ -62,9 +63,29 @@ class ConfigIndexingActionsTests {
         assertFails { validateSlackConfig.invoke(ConfigIndexingActions, slack, user) }
     }
 
+    @Test
+    fun `test validate chime`() {
+        val user = User()
+        var chime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
+        validateChimeConfig.invoke(ConfigIndexingActions, chime, user)
+        chime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456&test=123")
+        validateChimeConfig.invoke(ConfigIndexingActions, chime, user)
+        chime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url")
+        assertFails { validateChimeConfig.invoke(ConfigIndexingActions, chime, user) }
+        chime = Chime("https://hooks.chime.aws/incomingwebhooks?token=123456")
+        assertFails { validateChimeConfig.invoke(ConfigIndexingActions, chime, user) }
+        chime = Chime("http://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
+        assertFails { validateChimeConfig.invoke(ConfigIndexingActions, chime, user) }
+        chime = Chime("https://sample.chime.aws/incomingwebhooks/sample_chime_url?token=123456")
+        assertFails { validateChimeConfig.invoke(ConfigIndexingActions, chime, user) }
+        chime = Chime("https://hooks.chime.aws/sample_chime_url?token=123456")
+        assertFails { validateChimeConfig.invoke(ConfigIndexingActions, chime, user) }
+    }
+
     companion object {
         private lateinit var validateMicrosoftTeamsConfig: Method
         private lateinit var validateSlackConfig: Method
+        private lateinit var validateChimeConfig: Method
 
         @BeforeAll
         @JvmStatic
@@ -76,9 +97,13 @@ class ConfigIndexingActionsTests {
             validateSlackConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
                 "validateSlackConfig", Slack::class.java, User::class.java
             )
+            validateChimeConfig = ConfigIndexingActions::class.java.getDeclaredMethod(
+                "validateChimeConfig", Chime::class.java, User::class.java
+            )
 
             validateMicrosoftTeamsConfig.isAccessible = true
             validateSlackConfig.isAccessible = true
+            validateChimeConfig.isAccessible = true
         }
     }
 }