From 0d609eed8788e45e16b5eeb6e80827ffc9c50bc6 Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Mon, 20 Nov 2023 00:05:55 -0500 Subject: [PATCH 1/6] Add regex for Chime webhook URLs. Signed-off-by: Aniruddh Srivastava --- .../opensearch/notifications/index/ConfigIndexingActions.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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?) { From 1a7bcfaa6eb47de9710287d42d62b865bfbf494d Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Mon, 20 Nov 2023 01:30:35 -0500 Subject: [PATCH 2/6] Update tests for new Chime webhook regex. Signed-off-by: Aniruddh Srivastava --- .../org/opensearch/integtest/IntegTestHelpers.kt | 2 +- .../config/ChimeNotificationConfigCrudIT.kt | 12 ++++++------ .../integtest/config/CreateNotificationConfigIT.kt | 2 +- .../config/MicrosoftTeamsNotificationConfigCrudIT.kt | 2 +- .../integtest/config/QueryNotificationConfigIT.kt | 6 +++--- .../config/SlackNotificationConfigCrudIT.kt | 2 +- .../integtest/config/SnsNotificationConfigCrudIT.kt | 2 +- .../integtest/send/SendTestMessageRestHandlerIT.kt | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-) 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..b12e364e 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 @@ -18,7 +18,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 +66,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 +125,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 +157,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 +245,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 +278,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() 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" } } } From fa7a5f71ca2e59a9cff31b9c1c2c10049f2f36ab Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Wed, 22 Nov 2023 21:57:25 -0500 Subject: [PATCH 3/6] Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava --- .../config/ChimeNotificationConfigCrudIT.kt | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) 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 b12e364e..5631f93c 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 @@ -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://hooks.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 + ) + } + } } From 20ef656a3a51afe3af79b8a72c906ef78c5f5e10 Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Wed, 22 Nov 2023 22:14:28 -0500 Subject: [PATCH 4/6] Add test for validating Chime url. Signed-off-by: Aniruddh Srivastava --- .../index/ConfigIndexingActionsTests.kt | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) 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..d941611a 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,30 @@ 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 +98,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 } } } From f879bc2e8b3d71ee4f89940a66e2150901725f65 Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Wed, 22 Nov 2023 22:19:14 -0500 Subject: [PATCH 5/6] Linting Signed-off-by: Aniruddh Srivastava --- .../opensearch/notifications/index/ConfigIndexingActionsTests.kt | 1 - 1 file changed, 1 deletion(-) 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 d941611a..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 @@ -80,7 +80,6 @@ class ConfigIndexingActionsTests { 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 { From 0ebbd73017d6ad8734ba07a1d88308484eb0f5b8 Mon Sep 17 00:00:00 2001 From: Aniruddh Srivastava Date: Thu, 23 Nov 2023 02:03:34 -0500 Subject: [PATCH 6/6] Actually make the Chime URL wrong. Signed-off-by: Aniruddh Srivastava --- .../integtest/config/ChimeNotificationConfigCrudIT.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5631f93c..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 @@ -314,7 +314,7 @@ class ChimeNotificationConfigCrudIT : PluginRestTestCase() { } fun `test create config with wrong Chime url and get error text`() { - val sampleChime = Chime("https://hooks.chime.aws/incomingwebhooks/sample_chime_url?token=123456") + 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",