diff --git a/zerver/webhooks/slack/fixtures/slack_conversations_info_api_response.json b/zerver/webhooks/slack/fixtures/slack_conversations_info_api_response.json new file mode 100644 index 0000000000..2fafb93654 --- /dev/null +++ b/zerver/webhooks/slack/fixtures/slack_conversations_info_api_response.json @@ -0,0 +1,61 @@ +{ + "ok": true, + "channel": { + "id": "C012AB3CD", + "name": "general", + "is_channel": true, + "is_group": false, + "is_im": false, + "is_mpim": false, + "is_private": false, + "created": 1654868334, + "is_archived": false, + "is_general": true, + "unlinked": 0, + "name_normalized": "general", + "is_shared": false, + "is_frozen": false, + "is_org_shared": false, + "is_pending_ext_shared": false, + "pending_shared": [], + "context_team_id": "T123ABC456", + "updated": 1723130875818, + "parent_conversation": null, + "creator": "U123ABC456", + "is_ext_shared": false, + "shared_team_ids": [ + "T123ABC456" + ], + "pending_connected_team_ids": [], + "topic": { + "value": "For public discussion of generalities", + "creator": "W012A3BCD", + "last_set": 1449709364 + }, + "purpose": { + "value": "This part of the workspace is for fun. Make fun here.", + "creator": "W012A3BCD", + "last_set": 1449709364 + }, + "properties": { + "tabs": [ + { + "id": "workflows", + "label": "", + "type": "workflows" + }, + { + "id": "files", + "label": "", + "type": "files" + }, + { + "id": "bookmarks", + "label": "", + "type": "bookmarks" + } + ] + }, + "previous_names": [] + } +} diff --git a/zerver/webhooks/slack/fixtures/slack_users_info_api_response.json b/zerver/webhooks/slack/fixtures/slack_users_info_api_response.json new file mode 100644 index 0000000000..0d871dff3d --- /dev/null +++ b/zerver/webhooks/slack/fixtures/slack_users_info_api_response.json @@ -0,0 +1,41 @@ +{ + "ok": true, + "user": { + "id": "W012A3CDE", + "team_id": "T012AB3C4", + "name": "supersecretemail", + "deleted": false, + "color": "9f69e7", + "real_name": "John Doe", + "tz": "America/Los_Angeles", + "tz_label": "Pacific Daylight Time", + "tz_offset": -25200, + "profile": { + "avatar_hash": "ge3b51ca72de", + "status_text": "Print is dead", + "status_emoji": ":books:", + "real_name": "John Doe", + "display_name": "john", + "real_name_normalized": "John Doe", + "display_name_normalized": "john", + "email": "supersecretemail@ghostbusters.example.com", + "image_original": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_24": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_32": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_48": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_72": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_192": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "image_512": "https://.../avatar/e3b51ca72dee4ef87916ae2b9240df50.jpg", + "team": "T012AB3C4" + }, + "is_admin": true, + "is_owner": false, + "is_primary_owner": false, + "is_restricted": false, + "is_ultra_restricted": false, + "is_bot": false, + "updated": 1502138686, + "is_app_user": false, + "has_2fa": false + } +} diff --git a/zerver/webhooks/slack/tests.py b/zerver/webhooks/slack/tests.py index b1e7325637..89568f34f7 100644 --- a/zerver/webhooks/slack/tests.py +++ b/zerver/webhooks/slack/tests.py @@ -1,7 +1,10 @@ -from typing import Any +from collections.abc import Callable +from functools import wraps +from typing import Concatenate from unittest.mock import patch -from typing_extensions import override +import responses +from typing_extensions import ParamSpec, override from zerver.lib.test_classes import WebhookTestCase from zerver.webhooks.slack.view import INVALID_SLACK_TOKEN_MESSAGE @@ -9,45 +12,43 @@ from zerver.webhooks.slack.view import INVALID_SLACK_TOKEN_MESSAGE EXPECTED_TOPIC = "Message from Slack" MESSAGE_WITH_NORMAL_TEXT = "Hello, this is a normal text message" -USER = "John Doe" +USER = "supersecretemail" CHANNEL = "general" EXPECTED_MESSAGE = "**{user}**: {message}" TOPIC_WITH_CHANNEL = "channel: {channel}" LEGACY_USER = "slack_user" +ParamT = ParamSpec("ParamT") + + +def mock_slack_api_calls( + test_func: Callable[Concatenate["SlackWebhookTests", ParamT], None], +) -> Callable[Concatenate["SlackWebhookTests", ParamT], None]: + @wraps(test_func) + @responses.activate + def _wrapped(self: "SlackWebhookTests", /, *args: ParamT.args, **kwargs: ParamT.kwargs) -> None: + responses.add( + responses.GET, + "https://slack.com/api/users.info", + self.webhook_fixture_data("slack", "slack_users_info_api_response"), + ) + responses.add( + responses.GET, + "https://slack.com/api/conversations.info", + self.webhook_fixture_data("slack", "slack_conversations_info_api_response"), + ) + test_func(self, *args, **kwargs) + + return _wrapped + class SlackWebhookTests(WebhookTestCase): CHANNEL_NAME = "slack" URL_TEMPLATE = "/api/v1/external/slack?stream={stream}&api_key={api_key}&slack_app_token=xoxp-XXXXXXXXXXXXXXXXXXXXX" WEBHOOK_DIR_NAME = "slack" - @override - def setUp(self) -> None: - super().setUp() - self.get_slack_api_data_patcher = patch("zerver.webhooks.slack.view.get_slack_api_data") - self.check_slack_token_patcher = patch("zerver.webhooks.slack.view.check_token_access") - - self.mock_check_slack_token = self.check_slack_token_patcher.start() - self.mock_get_slack_api_data = self.get_slack_api_data_patcher.start() - self.mock_get_slack_api_data.side_effect = self.mocked_get_slack_api_data - - @override - def tearDown(self) -> None: - self.get_slack_api_data_patcher.stop() - self.check_slack_token_patcher.stop() - super().tearDown() - - def mocked_get_slack_api_data( - self, url: str, get_param: str, token: str, **kwargs: dict[str, Any] - ) -> dict[str, Any]: - slack_api_endpoints: dict[str, Any] = { - "https://slack.com/api/users.info": {"name": USER}, - "https://slack.com/api/conversations.info": {"name": CHANNEL}, - } - self.assertIn(url, slack_api_endpoints) - return slack_api_endpoints[url] - + @mock_slack_api_calls def test_slack_only_stream_parameter(self) -> None: expected_message = EXPECTED_MESSAGE.format(user=USER, message=MESSAGE_WITH_NORMAL_TEXT) self.check_webhook( @@ -57,6 +58,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_slack_with_user_specified_topic(self) -> None: expected_topic_name = "test" self.url = self.build_webhook_url(topic=expected_topic_name) @@ -68,6 +70,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_slack_channels_map_to_topics_true(self) -> None: self.url = self.build_webhook_url(channels_map_to_topics="1") expected_message = EXPECTED_MESSAGE.format(user=USER, message=MESSAGE_WITH_NORMAL_TEXT) @@ -79,6 +82,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_slack_channels_map_to_topics_true_and_user_specified_topic(self) -> None: expected_topic_name = "test" self.url = self.build_webhook_url(topic=expected_topic_name, channels_map_to_topics="1") @@ -90,6 +94,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_slack_channels_map_to_topics_false(self) -> None: self.CHANNEL_NAME = CHANNEL self.url = self.build_webhook_url(channels_map_to_topics="0") @@ -101,6 +106,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_slack_channels_map_to_topics_false_and_user_specified_topic(self) -> None: self.CHANNEL_NAME = CHANNEL expected_topic_name = "test" @@ -113,24 +119,31 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_invalid_channels_map_to_topics(self) -> None: payload = self.get_body("message_with_normal_text") url = self.build_webhook_url(channels_map_to_topics="abc") result = self.client_post(url, payload, content_type="application/json") self.assert_json_error(result, "Error: channels_map_to_topics parameter other than 0 or 1") + @mock_slack_api_calls def test_challenge_handshake_payload(self) -> None: url = self.build_webhook_url(channels_map_to_topics="1") payload = self.get_body("challenge_handshake_payload") - result = self.client_post(url, payload, content_type="application/json") + with self.assertLogs(level="INFO") as info_logs: + result = self.client_post(url, payload, content_type="application/json") expected_challenge_response = { "msg": "", "result": "success", "challenge": "3eZbrw1aBm2rZgRNFdxV2595E9CY3gmdALWMmHkvFXO7tYXAYM8P", } - + self.assertTrue( + "INFO:root:This is a Slack user token, which grants all rights the user has!" + in info_logs.output[0] + ) self.assertJSONEqual(result.content.decode("utf-8"), expected_challenge_response) + @mock_slack_api_calls def test_block_message_from_slack_bridge_bot(self) -> None: self.check_webhook( "message_from_slack_bridge_bot", @@ -140,6 +153,7 @@ class SlackWebhookTests(WebhookTestCase): expect_noop=True, ) + @mock_slack_api_calls def test_message_with_bullet_points(self) -> None: message_body = "• list three\n• list two" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -150,8 +164,11 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_channel_and_user_mentions(self) -> None: - message_body = "@**John Doe** **#general** message with both channel and user mentions" + message_body = ( + "@**supersecretemail** **#general** message with both channel and user mentions" + ) expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) self.check_webhook( "message_with_channel_and_user_mentions", @@ -160,6 +177,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_channel_mentions(self) -> None: message_body = "**#zulip-mirror** **#general** message with channel mentions" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -170,6 +188,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_formatted_texts(self) -> None: message_body = "**Bold text** *italic text* ~~strikethrough~~" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -180,6 +199,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_image_files(self) -> None: message_body = """ *[5e44bcbc-e43c-4a2e-85de-4be126f392f4.jpg](https://ds-py62195.slack.com/files/U06NU4E26M9/F079E4173BL/5e44bcbc-e43c-4a2e-85de-4be126f392f4.jpg)* @@ -193,6 +213,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_inline_code(self) -> None: message_body = "`asdasda this is a code block`" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -203,6 +224,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_ordered_list(self) -> None: message_body = "1. point one\n2. point two\n3. mix both\n4. pour water\n5. etc" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -213,10 +235,9 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_user_mentions(self) -> None: - message_body = ( - "@**John Doe** @**John Doe** @**John Doe** hello, this is a message with mentions" - ) + message_body = "@**supersecretemail** @**supersecretemail** @**supersecretemail** hello, this is a message with mentions" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) self.check_webhook( "message_with_user_mentions", @@ -225,6 +246,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_variety_files(self) -> None: message_body = """Message with an assortment of file types *[postman-agent-0.4.25-linux-x64.tar.gz](https://ds-py62195.slack.com/files/U06NU4E26M9/F079E4CMY5Q/postman-agent-0.4.25-linux-x64.tar.gz)* @@ -241,6 +263,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_workspace_mentions(self) -> None: message_body = "@**all** @**all** Sorry for mentioning. This is for the test fixtures for the Slack integration update PR I'm working on and can't be done in a private channel. :bow:" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -251,6 +274,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_from_slack_integration_bot(self) -> None: self.check_webhook( "message_from_slack_integration_bot", @@ -260,6 +284,7 @@ class SlackWebhookTests(WebhookTestCase): expect_noop=True, ) + @mock_slack_api_calls def test_message_with_code_block(self) -> None: message_body = """```def is_bot_message(payload: WildValue) -> bool:\n app_api_id = payload.get(\"api_app_id\").tame(check_none_or(check_string))\n bot_app_id = (\n payload.get(\"event\", {})\n .get(\"bot_profile\", {})\n .get(\"app_id\")\n .tame(check_none_or(check_string))\n )\n return bot_app_id is not None and app_api_id == bot_app_id```""" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -270,6 +295,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_complex_formatted_texts(self) -> None: message_body = "this is text messages with overlapping formatting\n***bold with italic***\n~~**bold with strike through**~~\n~~*italic with strike through*~~\n~~***all three***~~" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -280,8 +306,9 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_complex_formatted_mentions(self) -> None: - message_body = "@**John Doe** **#general** ~~***@**all*****~~" + message_body = "@**supersecretemail** **#general** ~~***@**all*****~~" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) self.check_webhook( "message_with_complex_formatted_mentions", @@ -290,6 +317,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_message_with_quote_block(self) -> None: message_body = "> This is a quote" expected_message = EXPECTED_MESSAGE.format(user=USER, message=message_body) @@ -300,6 +328,7 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + @mock_slack_api_calls def test_block_slack_retries(self) -> None: payload = self.get_body("message_with_normal_text") with patch("zerver.webhooks.slack.view.check_send_webhook_message") as m: @@ -312,6 +341,7 @@ class SlackWebhookTests(WebhookTestCase): self.assertFalse(m.called) self.assert_json_success(result) + @mock_slack_api_calls def test_missing_api_token_scope(self) -> None: error_message = "Slack token is missing the following required scopes: ['users:read', 'users:read.email']" user_facing_error_message = INVALID_SLACK_TOKEN_MESSAGE.format(error_message=error_message) @@ -334,6 +364,7 @@ class SlackWebhookTests(WebhookTestCase): self.assertEqual(actual_error_message, user_facing_error_message) + @mock_slack_api_calls def test_missing_slack_api_token(self) -> None: error_message = "slack_app_token is missing." self.url = self.build_webhook_url(slack_app_token="")