Fix #683 IgnoringSelfEvents middleware does not filter out Message se…

…nt via response_url (#684)
slackapi · Jul 21, 2022 · e8136c0 · e8136c0
1 parent 398c900
commit e8136c0
Show file tree

Hide file tree

Showing 4 changed files with 156 additions and 6 deletions.
diff --git a/slack_bolt/middleware/ignoring_self_events/async_ignoring_self_events.py b/slack_bolt/middleware/ignoring_self_events/async_ignoring_self_events.py
@@ -15,7 +15,9 @@ async def async_process(
         next: Callable[[], Awaitable[BoltResponse]],
     ) -> BoltResponse:
         auth_result = req.context.authorize_result
-        if self._is_self_event(auth_result, req.context.user_id, req.body):
+        # message events can have $.event.bot_id while it does not have its user_id
+        bot_id = req.body.get("event", {}).get("bot_id")
+        if self._is_self_event(auth_result, req.context.user_id, bot_id, req.body):
             self._debug_log(req.body)
             return await req.context.ack()
         else:

diff --git a/slack_bolt/middleware/ignoring_self_events/ignoring_self_events.py b/slack_bolt/middleware/ignoring_self_events/ignoring_self_events.py
@@ -21,25 +21,35 @@ def process(
         next: Callable[[], BoltResponse],
     ) -> BoltResponse:
         auth_result = req.context.authorize_result
-        if self._is_self_event(auth_result, req.context.user_id, req.body):
+        # message events can have $.event.bot_id while it does not have its user_id
+        bot_id = req.body.get("event", {}).get("bot_id")
+        if self._is_self_event(auth_result, req.context.user_id, bot_id, req.body):
             self._debug_log(req.body)
             return req.context.ack()
         else:
             return next()
 
     # -----------------------------------------
 
-    # Its an Events API event that isn't of type message,
+    # It's an Events API event that isn't of type message,
     # but the user ID might match our own app. Filter these out.
     # However, some events still must be fired, because they can make sense.
     events_that_should_be_kept = ["member_joined_channel", "member_left_channel"]
 
     @classmethod
-    def _is_self_event(cls, auth_result: AuthorizeResult, user_id: str, body: Dict[str, Any]):
+    def _is_self_event(
+        cls,
+        auth_result: AuthorizeResult,
+        user_id: Optional[str],
+        bot_id: Optional[str],
+        body: Dict[str, Any],
+    ):
         return (
             auth_result is not None
-            and user_id is not None
-            and user_id == auth_result.bot_user_id
+            and (
+                (user_id is not None and user_id == auth_result.bot_user_id)
+                or (bot_id is not None and bot_id == auth_result.bot_id)  # for bot_message events
+            )
             and body.get("event") is not None
             and body.get("event", {}).get("type") not in cls.events_that_should_be_kept
         )

diff --git a/tests/scenario_tests/test_events_ignore_self.py b/tests/scenario_tests/test_events_ignore_self.py
@@ -42,6 +42,35 @@ def handle_app_mention(say):
         # The listener should not be executed
         assert self.mock_received_requests.get("/chat.postMessage") is None
 
+    def test_self_events_response_url(self):
+        app = App(client=self.web_client)
+
+        @app.event("message")
+        def handle_app_mention(say):
+            say("What's up?")
+
+        request: BoltRequest = BoltRequest(body=response_url_message_event, mode="socket_mode")
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert_auth_test_count(self, 1)
+        sleep(1)  # wait a bit after auto ack()
+        # The listener should not be executed
+        assert self.mock_received_requests.get("/chat.postMessage") is None
+
+    def test_not_self_events_response_url(self):
+        app = App(client=self.web_client)
+
+        @app.event("message")
+        def handle_app_mention(say):
+            say("What's up?")
+
+        request: BoltRequest = BoltRequest(body=different_app_response_url_message_event, mode="socket_mode")
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert_auth_test_count(self, 1)
+        sleep(1)  # wait a bit after auto ack()
+        assert self.mock_received_requests.get("/chat.postMessage") == 1
+
     def test_self_events_disabled(self):
         app = App(
             client=self.web_client,
@@ -83,3 +112,45 @@ def handle_app_mention(say):
     "event_time": 1599616881,
     "authed_users": ["W111"],
 }
+
+response_url_message_event = {
+    "token": "verification_token",
+    "team_id": "T111",
+    "enterprise_id": "E111",
+    "api_app_id": "A111",
+    "event": {
+        "type": "message",
+        "subtype": "bot_message",
+        "text": "Hi there! This is a reply using response_url.",
+        "ts": "1658282075.825129",
+        "bot_id": "BZYBOTHED",
+        "channel": "C111",
+        "event_ts": "1658282075.825129",
+        "channel_type": "channel",
+    },
+    "type": "event_callback",
+    "event_id": "Ev111",
+    "event_time": 1599616881,
+    "authed_users": ["W111"],
+}
+
+different_app_response_url_message_event = {
+    "token": "verification_token",
+    "team_id": "T111",
+    "enterprise_id": "E111",
+    "api_app_id": "A111",
+    "event": {
+        "type": "message",
+        "subtype": "bot_message",
+        "text": "Hi there! This is a reply using response_url.",
+        "ts": "1658282075.825129",
+        "bot_id": "B_DIFFERENT_ONE",
+        "channel": "C111",
+        "event_ts": "1658282075.825129",
+        "channel_type": "channel",
+    },
+    "type": "event_callback",
+    "event_id": "Ev111",
+    "event_time": 1599616881,
+    "authed_users": ["W111"],
+}
diff --git a/tests/scenario_tests_async/test_events_ignore_self.py b/tests/scenario_tests_async/test_events_ignore_self.py
@@ -45,6 +45,29 @@ async def test_self_events(self):
         # The listener should not be executed
         assert self.mock_received_requests.get("/chat.postMessage") is None
 
+    @pytest.mark.asyncio
+    async def test_self_events_response_url(self):
+        app = AsyncApp(client=self.web_client)
+        app.event("message")(whats_up)
+        request = AsyncBoltRequest(body=response_url_message_event, mode="socket_mode")
+        response = await app.async_dispatch(request)
+        assert response.status == 200
+        await assert_auth_test_count_async(self, 1)
+        await asyncio.sleep(1)  # wait a bit after auto ack()
+        # The listener should not be executed
+        assert self.mock_received_requests.get("/chat.postMessage") is None
+
+    @pytest.mark.asyncio
+    async def test_not_self_events_response_url(self):
+        app = AsyncApp(client=self.web_client)
+        app.event("message")(whats_up)
+        request = AsyncBoltRequest(body=different_app_response_url_message_event, mode="socket_mode")
+        response = await app.async_dispatch(request)
+        assert response.status == 200
+        await assert_auth_test_count_async(self, 1)
+        await asyncio.sleep(1)  # wait a bit after auto ack()
+        assert self.mock_received_requests.get("/chat.postMessage") == 1
+
     @pytest.mark.asyncio
     async def test_self_events_disabled(self):
         app = AsyncApp(client=self.web_client, ignoring_self_events_enabled=False)
@@ -82,6 +105,50 @@ async def test_self_events_disabled(self):
 }
 
 
+response_url_message_event = {
+    "token": "verification_token",
+    "team_id": "T111",
+    "enterprise_id": "E111",
+    "api_app_id": "A111",
+    "event": {
+        "type": "message",
+        "subtype": "bot_message",
+        "text": "Hi there! This is a reply using response_url.",
+        "ts": "1658282075.825129",
+        "bot_id": "BZYBOTHED",
+        "channel": "C111",
+        "event_ts": "1658282075.825129",
+        "channel_type": "channel",
+    },
+    "type": "event_callback",
+    "event_id": "Ev111",
+    "event_time": 1599616881,
+    "authed_users": ["W111"],
+}
+
+
+different_app_response_url_message_event = {
+    "token": "verification_token",
+    "team_id": "T111",
+    "enterprise_id": "E111",
+    "api_app_id": "A111",
+    "event": {
+        "type": "message",
+        "subtype": "bot_message",
+        "text": "Hi there! This is a reply using response_url.",
+        "ts": "1658282075.825129",
+        "bot_id": "B_DIFFERENT_ONE",
+        "channel": "C111",
+        "event_ts": "1658282075.825129",
+        "channel_type": "channel",
+    },
+    "type": "event_callback",
+    "event_id": "Ev111",
+    "event_time": 1599616881,
+    "authed_users": ["W111"],
+}
+
+
 async def whats_up(body, say, payload, event):
     assert body["event"] == payload
     assert payload == event