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 2ef5b93a..01e76dfa 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 @@ -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 index fefc8a61..da8e44ed 100644 --- 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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt index e2d89769..fd185451 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/SecurityNotificationIT.kt @@ -54,7 +54,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", @@ -97,7 +97,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", @@ -133,7 +133,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", @@ -210,7 +210,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", @@ -246,7 +246,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", @@ -302,7 +302,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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/bwc/NotificationsBackwardsCompatibilityIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/bwc/NotificationsBackwardsCompatibilityIT.kt index 6e7eee5f..5ae57fb4 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/bwc/NotificationsBackwardsCompatibilityIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/ChimeNotificationConfigCrudIT.kt index 3707bb83..744429ef 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 @@ -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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/CreateNotificationConfigIT.kt index 64805e9d..163388c7 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 @@ -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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt index aed06e9f..c8b24b38 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt +++ b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/EmailNotificationConfigCrudIT.kt @@ -907,7 +907,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}", @@ -948,7 +948,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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/QueryNotificationConfigIT.kt index 47efbe32..6a8451ed 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(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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/SlackNotificationConfigCrudIT.kt index 86c64cf2..5bbac40c 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 @@ -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/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/WebhookNotificationConfigCrudIT.kt b/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/WebhookNotificationConfigCrudIT.kt index 893a64fe..1977a040 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/integtest/config/WebhookNotificationConfigCrudIT.kt +++ b/notifications/notifications/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/notifications/notifications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/index/ConfigIndexingActionsTests.kt index d2c6b0ee..ddbe104e 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 @@ -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/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt b/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt index 0f36945e..24a8baa8 100644 --- a/notifications/notifications/src/test/kotlin/org/opensearch/notifications/model/NotificationConfigDocTests.kt +++ b/notifications/notifications/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"},