chore(slack): remove old check signing secret code (#72627)

src/sentry/features/temporary.py

@@ -393,8 +393,6 @@ def register_temporary_features(manager: FeatureManager):
     # Feature flags for migrating to the Slack SDK WebClient
     # SlackNotifyServiceAction
     manager.add("organizations:slack-sdk-issue-alert-action", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
-    # Use new Slack SDK Client for verifying webhook signatures
-    manager.add("organizations:slack-sdk-signature", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
     # Use new Slack SDK Client for sending activity messages in threads
     manager.add("organizations:slack-sdk-activity-threads", OrganizationFeature, FeatureHandlerStrategy.OPTIONS)
     # Use new Slack SDK Client for sending metric alerts

src/sentry/integrations/slack/requests/base.py

@@ -8,17 +8,15 @@ from rest_framework import status as status_
 from rest_framework.request import Request
 from slack_sdk.signature import SignatureVerifier
 
-from sentry import features, options
+from sentry import options
 from sentry.services.hybrid_cloud.identity import RpcIdentity, identity_service
 from sentry.services.hybrid_cloud.identity.model import RpcIdentityProvider
 from sentry.services.hybrid_cloud.integration import RpcIntegration, integration_service
-from sentry.services.hybrid_cloud.organization import organization_service
-from sentry.services.hybrid_cloud.organization.model import RpcOrganizationSummary
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
 from sentry.utils.safe import get_path
 
-from ..utils import check_signing_secret, logger
+from ..utils import logger
 
 
 def _get_field_id_option(data: Mapping[str, Any], field_name: str) -> str | None:
@@ -67,7 +65,6 @@ class SlackRequest:
         self._provider: RpcIdentityProvider | None = None
         self._user: RpcUser | None = None
         self._data: MutableMapping[str, Any] = {}
-        self._organization: RpcOrganizationSummary | None = None  # temporary, used for FF check
 
     def validate(self) -> None:
         """
@@ -112,15 +109,6 @@ class SlackRequest:
         self._identity = context.identity
         self._user = context.user
 
-        if context.integration:
-            org_integrations = integration_service.get_organization_integrations(
-                integration_id=context.integration.id, status=0
-            )
-            if org_integrations:
-                self._organization = organization_service.get(
-                    id=org_integrations[0].organization_id
-                )
-
     @property
     def integration(self) -> RpcIntegration:
         if not self._integration:
@@ -225,22 +213,9 @@ class SlackRequest:
         if not (signature and timestamp):
             return False
 
-        log_extra = {
-            "organization_id": self._organization.id if self._organization else None,
-            "integration_id": self._integration.id if self._integration else None,
-        }
-
-        if self._organization and features.has(
-            "organizations:slack-sdk-signature", self._organization
-        ):
-            logger.info("slack.request.verify_signature", extra=log_extra)
-            return SignatureVerifier(signing_secret).is_valid(
-                body=self.request.body, timestamp=timestamp, signature=signature
-            )
-
-        logger.info("slack.request.check_signing_secret", extra=log_extra)
-
-        return check_signing_secret(signing_secret, self.request.body, timestamp, signature)
+        return SignatureVerifier(signing_secret).is_valid(
+            body=self.request.body, timestamp=timestamp, signature=signature
+        )
 
     def _check_verification_token(self, verification_token: str) -> bool:
         return self.data.get("token") == verification_token

src/sentry/integrations/slack/utils/__init__.py

@@ -1,5 +1,4 @@
 __all__ = (
-    "check_signing_secret",
     "get_channel_id",
     "get_channel_id_with_timeout",
     "get_slack_data_by_user",
@@ -20,7 +19,7 @@ import logging
 
 logger = logging.getLogger("sentry.integrations.slack")
 
-from .auth import check_signing_secret, is_valid_role, set_signing_secret
+from .auth import is_valid_role, set_signing_secret
 from .channel import (
     get_channel_id,
     get_channel_id_with_timeout,

src/sentry/integrations/slack/utils/auth.py

@@ -33,10 +33,3 @@ def set_signing_secret(secret: str, data: bytes) -> SigningSecretKwargs:
         "HTTP_X_SLACK_REQUEST_TIMESTAMP": timestamp,
         "HTTP_X_SLACK_SIGNATURE": signature,
     }
-
-
-def check_signing_secret(signing_secret: str, data: bytes, timestamp: str, signature: str) -> bool:
-    # Taken from: https://github.com/slackapi/python-slack-events-api/blob/master/slackeventsapi/server.py#L47
-    # Slack docs on this here: https://api.slack.com/authentication/verifying-requests-from-slack#about
-    request_hash = _encode_data(signing_secret, data, timestamp)
-    return hmac.compare_digest(request_hash.encode("utf-8"), signature.encode("utf-8"))

tests/sentry/integrations/slack/test_requests.py

@@ -13,7 +13,6 @@ from sentry.integrations.slack.requests.event import SlackEventRequest
 from sentry.integrations.slack.utils import set_signing_secret
 from sentry.testutils.cases import TestCase
 from sentry.testutils.helpers import override_options
-from sentry.testutils.helpers.features import with_feature
 from sentry.testutils.silo import control_silo_test
 
 
@@ -39,13 +38,6 @@ class SlackRequestTest(TestCase):
     def slack_request(self):
         return SlackRequest(self.request)
 
-    def test_validate(self):
-        self.create_integration(
-            organization=self.organization, external_id="T001", provider="slack"
-        )
-        self.slack_request.validate()
-
-    @with_feature("organizations:slack-sdk-signature")
     @patch("slack_sdk.signature.SignatureVerifier.is_valid")
     def test_validate_using_sdk(self, mock_verify):
         self.create_integration(

tests/sentry/middleware/integrations/parsers/test_slack.py

@@ -38,7 +38,11 @@ class SlackRequestParserTest(TestCase):
         return HttpResponse(status=200, content="passthrough")
 
     @responses.activate
-    def test_webhook(self):
+    @patch(
+        "slack_sdk.signature.SignatureVerifier.is_valid",
+        return_value=True,
+    )
+    def test_webhook(self, mock_verify):
         # Retrieve the correct integration
         data = urlencode({"team_id": self.integration.external_id}).encode("utf-8")
         signature = _encode_data(secret="slack-signing-secret", data=data, timestamp=self.timestamp)
@@ -142,8 +146,9 @@ class SlackRequestParserTest(TestCase):
                 {"team_id": self.integration.external_id, "response_url": response_url}
             )
         }
-        with assume_test_silo_mode_of(OrganizationIntegration), outbox_context(
-            transaction.atomic(using=router.db_for_write(OrganizationIntegration))
+        with (
+            assume_test_silo_mode_of(OrganizationIntegration),
+            outbox_context(transaction.atomic(using=router.db_for_write(OrganizationIntegration))),
         ):
             OrganizationIntegration.objects.filter(organization_id=self.organization.id).delete()
         request = self.factory.post(reverse("sentry-integration-slack-action"), data=data)