revert chime/slack validations

Signed-off-by: Riya Saxena <[email protected]>
opensearch-project · Aug 26, 2024 · d4a6dce · d4a6dce
1 parent 6e0874d
commit d4a6dce
Show file tree

Hide file tree

Showing 14 changed files with 31 additions and 120 deletions.
diff --git a/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt b/...cations/core/src/main/kotlin/org/opensearch/notifications/core/utils/ValidationHelpers.kt
@@ -5,12 +5,10 @@
 
 package org.opensearch.notifications.core.utils
 
-import inet.ipaddr.HostName
 import inet.ipaddr.IPAddressString
 import org.apache.http.client.methods.HttpPatch
 import org.apache.http.client.methods.HttpPost
 import org.apache.http.client.methods.HttpPut
-import org.apache.logging.log4j.LogManager
 import org.opensearch.core.common.Strings
 import java.net.URL
 
@@ -39,12 +37,9 @@ fun isHostInDenylist(urlString: String, hostDenyList: List<String>): Boolean {
     val url = URL(urlString)
     if (url.host != null) {
         val ipStr = IPAddressString(url.host)
-        val hostStr = HostName(url.host)
         for (network in hostDenyList) {
-            val denyIpStr = IPAddressString(network)
-            val denyHostStr = HostName(network)
-            if (denyIpStr.contains(ipStr) || denyHostStr.equals(hostStr)) {
-                LogManager.getLogger().error("${url.host} is denied")
+            val netStr = IPAddressString(network)
+            if (netStr.contains(ipStr)) {
                 return true
             }
         }

diff --git a/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt b/...ns/core/src/test/kotlin/org/opensearch/notifications/core/utils/ValidationHelpersTests.kt
@@ -10,8 +10,7 @@ import org.junit.jupiter.api.Test
 
 internal class ValidationHelpersTests {
 
-    private val hostDenyList = listOf(
-        "www.amazon.com",
+    private val hostDentyList = listOf(
         "127.0.0.0/8",
         "10.0.0.0/8",
         "172.16.0.0/12",
@@ -21,9 +20,8 @@ internal class ValidationHelpersTests {
     )
 
     @Test
-    fun `test hosts in denylist`() {
+    fun `test ips in denylist`() {
         val ips = listOf(
-            "www.amazon.com",
             "127.0.0.1", // 127.0.0.0/8
             "10.0.0.1", // 10.0.0.0/8
             "10.11.12.13", // 10.0.0.0/8
@@ -33,15 +31,15 @@ internal class ValidationHelpersTests {
             "9.9.9.9"
         )
         for (ip in ips) {
-            assertEquals(true, isHostInDenylist("https://$ip", hostDenyList), "address $ip was supposed to be identified as in the deny list, but was not")
+            assertEquals(true, isHostInDenylist("https://$ip", hostDentyList))
         }
     }
 
     @Test
-    fun `test hosts not in denylist`() {
-        val urls = listOf("156.4.77.1", "www.something.com")
+    fun `test url in denylist`() {
+        val urls = listOf("https://www.amazon.com", "https://mytest.com", "https://mytest.com")
         for (url in urls) {
-            assertEquals(false, isHostInDenylist("https://$url", hostDenyList), "address $url was not supposed to be identified as in the deny list, but was")
+            assertEquals(false, isHostInDenylist(url, hostDentyList))
         }
     }
 }
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,9 +56,7 @@ object ConfigIndexingActions {
 
     @Suppress("UnusedPrivateMember")
     private fun validateSlackConfig(slack: Slack, user: User?) {
-        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/\""
-        }
+        // TODO: URL validation with rules
     }
 
     @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://hooks.slack.com/services/sample_slack_url#$randomString"}
+            "slack":{"url":"https://slack.domain.com/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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url#$configId" }
+                "slack": { "url": "https://slack.domain.com/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://hooks.slack.com/services/sample_slack_url"}
+                "slack":{"url":"https://dummy.com"}
                 "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://hooks.slack.com/services/sample_slack_url"}
+                "slack":{"url":"https://updated.domain.com/updated_slack_url#0987654321"}
             }
         }
         """.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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         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://hooks.slack.com/services/sample_slack_url"},
+                "slack": {"url": "https://dummy.com"},
                 "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://hooks.slack.com/services/sample_slack_url"},
+                "slack":{"url": "https://dummy.com"},
                 "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(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(slackId, 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(chimeId, microsoftTeamsId, webhookId, smtpAccountId)
+        val domainIds = setOf(slackId, 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,16 +6,11 @@
 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
@@ -24,7 +19,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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -72,7 +67,7 @@ class SlackNotificationConfigCrudIT : PluginRestTestCase() {
         Thread.sleep(100)
 
         // Updated notification config object
-        val updatedSlack = Slack("https://hooks.slack.com/services/updated_slack_url")
+        val updatedSlack = Slack("https://updated.domain.com/updated_slack_url#0987654321")
         val updatedObject = NotificationConfig(
             "this is a updated config name",
             "this is a updated config description",
@@ -131,7 +126,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://hooks.slack.com/services/sample_slack_url")
+        val sampleSlack = Slack("https://domain.com/sample_slack_url#1234567890")
         val referenceObject = NotificationConfig(
             "this is a sample config name",
             "this is a sample config description",
@@ -160,40 +155,4 @@ 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://hooks.slack.com/services/sample_slack_url"}
+                "slack":{"url":"https://dummy.com"}
                 "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,7 +8,6 @@ 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
 
@@ -29,42 +28,8 @@ 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
@@ -73,12 +38,8 @@ 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
         }
     }
 }