Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

outgoing webhooks: Fix inconsistencies with Slack's API. 

Apparently, our slack compatible outgoing webhook format didn't
exactly match Slack, especially in the types used for values.  Fix
this by using a much more consistent format, where we preserve their
pattern of prefixing IDs with letters.

This fixes a bug where Zulip's team_id could be the empty string,
which tripped up using GitLab's slash commands with Zulip.

Fixes #19588.
This commit is contained in:
Tim Abbott 2021-09-17 12:22:23 -07:00 committed by Tim Abbott
parent c233ee9935
commit 417c32629d
3 changed files with 63 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
templates/zerver/api/outgoing-webhooks.md
View File

@ -116,27 +116,31 @@ Here's how we fill in the fields that a Slack-format webhook expects:
</tr>
<tr>
<td><code>team_id</code></td>
<td>String ID of the Zulip organization</td>
<td>ID of the Zulip organization prefixed by "T".</td>
</tr>
<tr>
<td><code>team_domain</code></td>
<td>Domain of the Zulip organization</td>
<td>Hostname of the Zulip organization</td>
</tr>
<tr>
<td><code>channel_id</code></td>
<td>Stream ID</td>
<td>Stream ID prefixed by "C"</td>
</tr>
<tr>
<td><code>channel_name</code></td>
<td>Stream name</td>
</tr>
<tr>
<td><code>thread_ts</code></td>
<td>Timestamp for when message was sent</td>
</tr>
<tr>
<td><code>timestamp</code></td>
<td>Timestamp for when message was sent</td>
</tr>
<tr>
<td><code>user_id</code></td>
<td>ID of the user who sent the message</td>
<td>ID of the user who sent the message prefixed by "U"</td>
</tr>
<tr>
<td><code>user_name</code></td>
@ -161,13 +165,14 @@ The above data is posted as list of tuples (not JSON), here's an example:
```
[('token', 'v9fpCdldZIej2bco3uoUvGp06PowKFOf'),
('team_id', 'zulip'),
('team_domain', 'zulip.com'),
('channel_id', '123'),
('team_id', 'T1512'),
('team_domain', 'zulip.example.com'),
('channel_id', 'C123'),
('channel_name', 'integrations'),
('thread_ts', 1532078950),
('timestamp', 1532078950),
('user_id', 21),
('user_name', 'Sample User'),
('user_id', 'U21'),
('user_name', 'Full Name'),
('text', '@**test**'),
('trigger_word', 'mention'),
('service_id', 27)]

41
zerver/lib/outgoing_webhook.py
View File

@ -20,9 +20,9 @@ from zerver.lib.url_encoding import near_message_url
from zerver.models import (
GENERIC_INTERFACE,
SLACK_INTERFACE,
Realm,
Service,
UserProfile,
email_to_domain,
get_client,
get_user_profile_by_id,
)
@ -40,7 +40,9 @@ class OutgoingWebhookServiceInterface(metaclass=abc.ABCMeta):
)
@abc.abstractmethod
def make_request(self, base_url: str, event: Dict[str, Any]) -> Optional[Response]:
def make_request(
self, base_url: str, event: Dict[str, Any], realm: Realm
) -> Optional[Response]:
raise NotImplementedError
@abc.abstractmethod
@ -49,7 +51,9 @@ class OutgoingWebhookServiceInterface(metaclass=abc.ABCMeta):
class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface):
def make_request(self, base_url: str, event: Dict[str, Any]) -> Optional[Response]:
def make_request(
self, base_url: str, event: Dict[str, Any], realm: Realm
) -> Optional[Response]:
"""
We send a simple version of the message to outgoing
webhooks, since most of them really only need
@ -98,20 +102,38 @@ class GenericOutgoingWebhookService(OutgoingWebhookServiceInterface):
class SlackOutgoingWebhookService(OutgoingWebhookServiceInterface):
def make_request(self, base_url: str, event: Dict[str, Any]) -> Optional[Response]:
def make_request(
self, base_url: str, event: Dict[str, Any], realm: Realm
) -> Optional[Response]:
if event["message"]["type"] == "private":
failure_message = "Slack outgoing webhooks don't support private messages."
fail_with_message(event, failure_message)
return None
# https://api.slack.com/legacy/custom-integrations/outgoing-webhooks#legacy-info__post-data
# documents the Slack outgoing webhook format:
#
# token=XXXXXXXXXXXXXXXXXX
# team_id=T0001
# team_domain=example
# channel_id=C2147483705
# channel_name=test
# thread_ts=1504640714.003543
# timestamp=1504640775.000005
# user_id=U2147483697
# user_name=Steve
# text=googlebot: What is the air-speed velocity of an unladen swallow?
# trigger_word=googlebot:
request_data = [
("token", self.token),
("team_id", event["message"]["sender_realm_str"]),
("team_domain", email_to_domain(event["message"]["sender_email"])),
("channel_id", event["message"]["stream_id"]),
("team_id", f"T{realm.id}"),
("team_domain", realm.host),
("channel_id", f"C{event['message']['stream_id']}"),
("channel_name", event["message"]["display_recipient"]),
("thread_ts", event["message"]["timestamp"]),
("timestamp", event["message"]["timestamp"]),
("user_id", event["message"]["sender_id"]),
("user_id", f"U{event['message']['sender_id']}"),
("user_name", event["message"]["sender_full_name"]),
("text", event["command"]),
("trigger_word", event["trigger"]),
@ -326,11 +348,12 @@ def do_rest_call(
"""Returns response of call if no exception occurs."""
try:
start_time = perf_counter()
bot_profile = service_handler.user_profile
response = service_handler.make_request(
base_url,
event,
bot_profile.realm,
)
bot_profile = service_handler.user_profile
logging.info(
"Outgoing webhook request from %s@%s took %f seconds",
bot_profile.id,

27
zerver/tests/test_outgoing_webhook_interfaces.py
View File

@ -107,6 +107,7 @@ class TestGenericOutgoingWebhookService(ZulipTestCase):
self.handler.make_request(
test_url,
event,
othello.realm,
)
session.post.assert_called_once()
self.assertEqual(session.post.call_args[0], (test_url,))
@ -150,6 +151,7 @@ class TestGenericOutgoingWebhookService(ZulipTestCase):
class TestSlackOutgoingWebhookService(ZulipTestCase):
def setUp(self) -> None:
super().setUp()
self.bot_user = get_user("outgoing-webhook@zulip.com", get_realm("zulip"))
self.stream_message_event = {
"command": "@**test**",
"user_profile_id": 12,
@ -187,7 +189,9 @@ class TestSlackOutgoingWebhookService(ZulipTestCase):
}
service_class = get_service_interface_class(SLACK_INTERFACE)
self.handler = service_class(token="abcdef", user_profile=None, service_name="test-service")
self.handler = service_class(
token="abcdef", user_profile=self.bot_user, service_name="test-service"
)
def test_make_request_stream_message(self) -> None:
test_url = "https://example.com/example"
@ -195,22 +199,24 @@ class TestSlackOutgoingWebhookService(ZulipTestCase):
self.handler.make_request(
test_url,
self.stream_message_event,
self.bot_user.realm,
)
session.post.assert_called_once()
self.assertEqual(session.post.call_args[0], (test_url,))
request_data = session.post.call_args[1]["data"]
self.assertEqual(request_data[0][1], "abcdef") # token
self.assertEqual(request_data[1][1], "zulip") # team_id
self.assertEqual(request_data[2][1], "zulip.com") # team_domain
self.assertEqual(request_data[3][1], "123") # channel_id
self.assertEqual(request_data[1][1], "T2") # team_id
self.assertEqual(request_data[2][1], "zulip.testserver") # team_domain
self.assertEqual(request_data[3][1], "C123") # channel_id
self.assertEqual(request_data[4][1], "integrations") # channel_name
self.assertEqual(request_data[5][1], 123456) # timestamp
self.assertEqual(request_data[6][1], 21) # user_id
self.assertEqual(request_data[7][1], "Sample User") # user_name
self.assertEqual(request_data[8][1], "@**test**") # text
self.assertEqual(request_data[9][1], "mention") # trigger_word
self.assertEqual(request_data[10][1], 12) # user_profile_id
self.assertEqual(request_data[5][1], 123456) # thread_id
self.assertEqual(request_data[6][1], 123456) # timestamp
self.assertEqual(request_data[7][1], "U21") # user_id
self.assertEqual(request_data[8][1], "Sample User") # user_name
self.assertEqual(request_data[9][1], "@**test**") # text
self.assertEqual(request_data[10][1], "mention") # trigger_word
self.assertEqual(request_data[11][1], 12) # user_profile_id
@mock.patch("zerver.lib.outgoing_webhook.fail_with_message")
def test_make_request_private_message(self, mock_fail_with_message: mock.Mock) -> None:
@ -219,6 +225,7 @@ class TestSlackOutgoingWebhookService(ZulipTestCase):
response = self.handler.make_request(
test_url,
self.private_message_event,
self.bot_user.realm,
)
session.post.assert_not_called()
self.assertIsNone(response)