ref(slack): Remove slack-v2 creds and usage (#23587)

* ref(slack): Remove slack-v2 creds and usage

* update slack requests tests

* dont need verification token

* add broken test

* mock signing secret

src/sentry/identity/slack/provider.py

@@ -26,10 +26,10 @@ def get_oauth_access_token_url(self):
         return "https://slack.com/api/oauth.v2.access"
 
     def get_oauth_client_id(self):
-        return options.get("slack-v2.client-id") or options.get("slack.client-id")
+        return options.get("slack.client-id")
 
     def get_oauth_client_secret(self):
-        return options.get("slack-v2.client-secret") or options.get("slack.client-secret")
+        return options.get("slack.client-secret")
 
     def get_user_scopes(self):
         return self.config.get("user_scopes", self.user_scopes)

src/sentry/integrations/slack/requests.py

@@ -93,19 +93,18 @@ def _validate_data(self):
             raise SlackRequestError(status=400)
 
     def _authorize(self):
-        # check v1 then v2
+        # XXX(meredith): Signing secrets are the prefered way
+        # but self-hosted could still have an older slack bot
+        # app that just has the verification token.
         signing_secret = options.get("slack.signing-secret")
         verification_token = options.get("slack.verification-token")
-        # for v1, only check the verification_token if we don't have a signing_secret
+
         if signing_secret:
             if self._check_signing_secret(signing_secret):
                 return
         elif verification_token and self._check_verification_token(verification_token):
             return
-        # for v2, only check signing secret
-        signing_secret = options.get("slack-v2.signing-secret")
-        if signing_secret and self._check_signing_secret(signing_secret):
-            return
+
         # unfortunately, we can't know which auth was supposed to succeed
         self._error("slack.action.auth")
         raise SlackRequestError(status=401)

src/sentry/options/defaults.py

@@ -119,14 +119,9 @@
 # Slack Integration
 register("slack.client-id", flags=FLAG_PRIORITIZE_DISK)
 register("slack.client-secret", flags=FLAG_PRIORITIZE_DISK)
+# signing-secret is preferred, but need to keep verification-token for apps that use it
 register("slack.verification-token", flags=FLAG_PRIORITIZE_DISK)
 register("slack.signing-secret", flags=FLAG_PRIORITIZE_DISK)
-register("slack.legacy-app", flags=FLAG_PRIORITIZE_DISK, type=Bool, default=True)
-
-# Slack V2 Integration
-register("slack-v2.client-id", flags=FLAG_PRIORITIZE_DISK)
-register("slack-v2.client-secret", flags=FLAG_PRIORITIZE_DISK)
-register("slack-v2.signing-secret", flags=FLAG_PRIORITIZE_DISK)
 
 # GitHub Integration
 register("github-app.id", default=0)

src/sentry/utils/pytest/sentry.py

@@ -116,7 +116,7 @@ def pytest_configure(config):
             "slack.client-id": "slack-client-id",
             "slack.client-secret": "slack-client-secret",
             "slack.verification-token": "slack-verification-token",
-            "slack.legacy-app": True,
+            "slack.signing-secret": "slack-signing-secret",
             "github-app.name": "sentry-test-app",
             "github-app.client-id": "github-client-id",
             "github-app.client-secret": "github-client-secret",

tests/sentry/integrations/slack/test_action_endpoint.py

@@ -4,7 +4,6 @@
 from sentry.utils.compat.mock import patch
 
 from sentry.api import client
-from sentry import options
 from sentry.models import (
     Integration,
     OrganizationIntegration,
@@ -55,19 +54,20 @@ def setUp(self):
             "https://hooks.slack.com/actions/T47563693/6204672533/x7ZLaiVMoECAW50Gw1ZYAXEM"
         )
 
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
     def post_webhook(
         self,
+        check_signing_secret_mock,
         action_data=None,
         type="event_callback",
         data=None,
-        token=None,
         team_id="TXXXXXXX1",
         callback_id=None,
         slack_user=None,
         original_message=None,
     ):
-        if token is None:
-            token = options.get("slack.verification-token")
 
         if slack_user is None:
             slack_user = {"id": self.identity.external_id, "domain": "example"}
@@ -79,7 +79,6 @@ def post_webhook(
             original_message = {}
 
         payload = {
-            "token": token,
             "team": {"id": team_id, "domain": "example.com"},
             "channel": {"id": "C065W1189", "domain": "forgotten-works"},
             "user": slack_user,
@@ -398,27 +397,32 @@ def test_handle_submission_fail(self, client_put):
             associate_url=associate_url, user_email=self.user.email, org_name=self.org.name
         )
 
-    def test_invalid_token(self):
-        resp = self.post_webhook(token="invalid")
-        assert resp.status_code == 401
-
-    def test_no_integration(self):
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
+    def test_no_integration(self, check_signing_secret_mock):
         self.integration.delete()
         resp = self.post_webhook()
         assert resp.status_code == 403
 
-    def test_slack_bad_payload(self):
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
+    def test_slack_bad_payload(self, check_signing_secret_mock):
         resp = self.client.post("/extensions/slack/action/", data={"nopayload": 0})
         assert resp.status_code == 400
 
-    def test_sentry_docs_link_clicked(self):
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
+    def test_sentry_docs_link_clicked(self, check_signing_secret_mock):
         payload = {
-            "token": options.get("slack.verification-token"),
             "team": {"id": "TXXXXXXX1", "domain": "example.com"},
             "user": {"id": self.identity.external_id, "domain": "example"},
             "type": "block_actions",
             "actions": [{"value": "sentry_docs_link_clicked"}],
         }
+
         payload = {"payload": json.dumps(payload)}
 
         resp = self.client.post("/extensions/slack/action/", data=payload)

tests/sentry/integrations/slack/test_event_endpoint.py

@@ -1,7 +1,7 @@
 import responses
 from urllib.parse import parse_qsl
+from sentry.utils.compat.mock import patch
 
-from sentry import options
 from sentry.utils import json
 from sentry.integrations.slack.utils import build_group_attachment, build_incident_attachment
 from sentry.models import Integration, OrganizationIntegration
@@ -79,13 +79,19 @@ def setUp(self):
         )
         OrganizationIntegration.objects.create(organization=self.org, integration=self.integration)
 
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
     def post_webhook(
-        self, event_data=None, type="event_callback", data=None, token=UNSET, team_id="TXXXXXXX1"
+        self,
+        check_signing_secret_mock,
+        event_data=None,
+        type="event_callback",
+        data=None,
+        token=UNSET,
+        team_id="TXXXXXXX1",
     ):
-        if token is UNSET:
-            token = options.get("slack.verification-token")
         payload = {
-            "token": token,
             "team_id": team_id,
             "api_app_id": "AXXXXXXXX1",
             "type": type,
@@ -97,31 +103,27 @@ def post_webhook(
             payload.update(data)
         if event_data:
             payload.setdefault("event", {}).update(event_data)
+
         return self.client.post("/extensions/slack/event/", payload)
 
 
 class UrlVerificationEventTest(BaseEventTest):
     challenge = "3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P"
 
-    def test_valid_token(self):
+    @patch(
+        "sentry.integrations.slack.requests.SlackRequest._check_signing_secret", return_value=True
+    )
+    def test_valid_event(self, check_signing_secret_mock):
         resp = self.client.post(
             "/extensions/slack/event/",
             {
                 "type": "url_verification",
                 "challenge": self.challenge,
-                "token": options.get("slack.verification-token"),
             },
         )
         assert resp.status_code == 200, resp.content
         assert resp.data["challenge"] == self.challenge
 
-    def test_invalid_token(self):
-        resp = self.client.post(
-            "/extensions/slack/event/",
-            {"type": "url_verification", "challenge": self.challenge, "token": "fizzbuzz"},
-        )
-        assert resp.status_code == 401, resp.content
-
 
 class LinkSharedEventTest(BaseEventTest):
     @responses.activate

tests/sentry/integrations/slack/test_integration.py

@@ -13,7 +13,6 @@
     OrganizationIntegration,
 )
 from sentry.testutils import IntegrationTestCase, TestCase
-from sentry.testutils.helpers import override_options
 
 
 class SlackIntegrationTest(IntegrationTestCase):
@@ -149,16 +148,6 @@ def test_reassign_user(self):
         identity = Identity.objects.get()
         assert identity.external_id == "UXXXXXXX2"
 
-    @responses.activate
-    def test_install_v2(self):
-        with override_options(
-            {"slack-v2.client-id": "other-id", "slack-v2.client-secret": "other-secret"}
-        ):
-            self.assert_setup_flow(
-                expected_client_id="other-id",
-                expected_client_secret="other-secret",
-            )
-
 
 class SlackIntegrationConfigTest(TestCase):
     def setUp(self):

tests/sentry/integrations/slack/test_requests.py

@@ -18,6 +18,17 @@
 )
 
 
+def set_signature(secret, data):
+    timestamp = str(int(time.mktime(datetime.utcnow().timetuple())))
+    req = b"v0:%s:%s" % (timestamp.encode("utf-8"), data)
+
+    signature = "v0=" + hmac.new(secret.encode("utf-8"), req, sha256).hexdigest()
+    return {
+        "HTTP_X_SLACK_REQUEST_TIMESTAMP": timestamp,
+        "HTTP_X_SLACK_SIGNATURE": signature,
+    }
+
+
 class SlackRequestTest(TestCase):
     def setUp(self):
         super().setUp()
@@ -30,6 +41,8 @@ def setUp(self):
             "user": {"id": "2"},
             "api_app_id": "S1",
         }
+        self.request.body = urlencode(self.request.data).encode("utf-8")
+        self.request.META = set_signature(options.get("slack.signing-secret"), self.request.body)
 
     @memoize
     def slack_request(self):
@@ -104,15 +117,8 @@ def setUp(self):
             "user": {"id": "2"},
             "api_app_id": "S1",
         }
-        self.request.META = {}
-
-    def set_signature(self, secret, data):
-        timestamp = str(int(time.mktime(datetime.utcnow().timetuple())))
-        req = b"v0:%s:%s" % (timestamp.encode("utf-8"), data)
-
-        signature = "v0=" + hmac.new(secret.encode("utf-8"), req, sha256).hexdigest()
-        self.request.META["HTTP_X_SLACK_REQUEST_TIMESTAMP"] = timestamp
-        self.request.META["HTTP_X_SLACK_SIGNATURE"] = signature
+        self.request.body = urlencode(self.request.data).encode("utf-8")
+        self.request.META = set_signature(options.get("slack.signing-secret"), self.request.body)
 
     @memoize
     def slack_request(self):
@@ -153,50 +159,29 @@ def test_validate_missing_event_type(self):
     def test_type(self):
         assert self.slack_request.type == "bar"
 
-    def test_signing_secret(self):
-        with override_options({"slack.signing-secret": "secret"}):
-            self.request.data = {"challenge": "abc123", "type": "url_verification"}
-
-            # we get a url encoded body with Slack
-            self.request.body = urlencode(self.request.data).encode("utf-8")
-
-            self.set_signature(options.get("slack.signing-secret"), self.request.body)
-            self.slack_request.validate()
-
-    def test_signing_secret_v2(self):
-        with override_options({"slack-v2.signing-secret": "secret-v2"}):
-            self.request.data = {"challenge": "abc123", "type": "url_verification"}
-
-            # we get a url encoded body with Slack
-            self.request.body = urlencode(self.request.data).encode("utf-8")
+    def test_signing_secret_bad(self):
+        self.request.data = {
+            "token": options.get("slack.verification-token"),
+            "challenge": "abc123",
+            "type": "url_verification",
+        }
+        self.request.body = urlencode(self.request.data).encode("utf-8")
 
-            self.set_signature(options.get("slack-v2.signing-secret"), self.request.body)
+        self.request.META = set_signature("bad_key", self.request.body)
+        with self.assertRaises(SlackRequestError) as e:
             self.slack_request.validate()
+            assert e.status == 401
 
-    def test_signing_secret_bad(self):
-        with override_options({"slack.signing-secret": "secret"}):
-            # even though we provide the token, should still fail
+    def test_use_verification_token(self):
+        with override_options({"slack.signing-secret": None}):
             self.request.data = {
                 "token": options.get("slack.verification-token"),
                 "challenge": "abc123",
                 "type": "url_verification",
             }
-            self.request.body = urlencode(self.request.data).encode("utf-8")
-
-            self.set_signature("bad_key", self.request.body)
-            with self.assertRaises(SlackRequestError) as e:
-                self.slack_request.validate()
-                assert e.status == 401
-
-    def test_signing_secret_use_verification_token(self):
-        self.request.data = {
-            "token": options.get("slack.verification-token"),
-            "challenge": "abc123",
-            "type": "url_verification",
-        }
-        self.request.body = json.dumps(self.request.data).encode("utf-8")
+            self.request.body = json.dumps(self.request.data).encode("utf-8")
 
-        self.slack_request.validate()
+            self.slack_request.validate()
 
 
 class SlackActionRequestTest(TestCase):
@@ -216,6 +201,8 @@ def setUp(self):
                 }
             )
         }
+        self.request.body = urlencode(self.request.data).encode("utf-8")
+        self.request.META = set_signature(options.get("slack.signing-secret"), self.request.body)
 
     @memoize
     def slack_request(self):