From 1163d542a6028faeb7941349ea9232f1c490a45d Mon Sep 17 00:00:00 2001 From: Aniruddh Date: Mon, 27 Nov 2023 22:11:29 -0500 Subject: [PATCH] Adds webhook format check for Chime (#816) * Add regex for Chime webhook URLs. Signed-off-by: Aniruddh Srivastava * Update tests for new Chime webhook regex. Signed-off-by: Aniruddh Srivastava * Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava * Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava * Linting Signed-off-by: Aniruddh Srivastava * Actually make the Chime URL wrong. Signed-off-by: Aniruddh Srivastava --------- Signed-off-by: Aniruddh Srivastava --- .../index/ConfigIndexingActions.kt | 4 +- .../opensearch/integtest/IntegTestHelpers.kt | 2 +- .../config/ChimeNotificationConfigCrudIT.kt | 53 ++++++++++++++++--- .../config/CreateNotificationConfigIT.kt | 2 +- .../MicrosoftTeamsNotificationConfigCrudIT.kt | 2 +- .../config/QueryNotificationConfigIT.kt | 6 +-- .../config/SlackNotificationConfigCrudIT.kt | 2 +- .../config/SnsNotificationConfigCrudIT.kt | 2 +- .../send/SendTestMessageRestHandlerIT.kt | 2 +- .../index/ConfigIndexingActionsTests.kt | 25 +++++++++ 10 files changed, 84 insertions(+), 16 deletions(-) diff --git a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt b/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt index 01e76dfa..c98f7367 100644 --- a/notifications/notifications/src/main/kotlin/org/opensearch/notifications/index/ConfigIndexingActions.kt +++ b/notifications/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 index da8e44ed..b942d401 100644 --- 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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt index 744429ef..ca682eba 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt index 163388c7..be7a8cfe 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/MicrosoftTeamsNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/MicrosoftTeamsNotificationConfigCrudIT.kt index f830ece4..90094c04 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/MicrosoftTeamsNotificationConfigCrudIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt index 6a8451ed..c16a9da1 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt index 5bbac40c..8a906204 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SnsNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SnsNotificationConfigCrudIT.kt index 10b09f3a..27b880ba 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SnsNotificationConfigCrudIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt index c36e2666..9754494a 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/send/SendTestMessageRestHandlerIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt index ddbe104e..2e510108 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt +++ b/notifications/notifications/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 } } }