diff --git a/elementary/messages/messaging_integrations/slack_web.py b/elementary/messages/messaging_integrations/slack_web.py index f83b2ae51..99c2bf723 100644 --- a/elementary/messages/messaging_integrations/slack_web.py +++ b/elementary/messages/messaging_integrations/slack_web.py @@ -1,7 +1,7 @@ import json import ssl import time -from typing import Any, Dict, Iterator, Optional +from typing import Any, Dict, Iterator, List, Optional from ratelimit import limits, sleep_and_retry from slack_sdk import WebClient @@ -51,6 +51,10 @@ def __init__( self.client = client self.tracking = tracking self._email_to_user_id_cache: Dict[str, str] = {} + self._handle_to_user_id_cache: Optional[Dict[str, str]] = None + self._list_users_page = sleep_and_retry( + limits(calls=20, period=ONE_MINUTE)(self._request_users_page) + ) self.reply_broadcast = reply_broadcast @classmethod @@ -77,7 +81,7 @@ def supports_actions(self) -> bool: def send_message( self, destination: Channel, body: MessageBody ) -> MessageSendResult[SlackWebMessageContext]: - formatted_message = format_block_kit(body, self.get_user_id_from_email) + formatted_message = format_block_kit(body, self.resolve_user_id) return self._send_message(destination, formatted_message) def reply_to_message( @@ -86,7 +90,7 @@ def reply_to_message( message_context: SlackWebMessageContext, body: MessageBody, ) -> MessageSendResult[SlackWebMessageContext]: - formatted_message = format_block_kit(body, self.get_user_id_from_email) + formatted_message = format_block_kit(body, self.resolve_user_id) return self._send_message( destination, formatted_message, @@ -187,6 +191,67 @@ def _join_channel(self, channel_id: str) -> None: self.tracking.record_internal_exception(e) raise MessagingIntegrationError(f"Failed to join channel {channel_id}") + def resolve_user_id(self, user: str) -> Optional[str]: + if user.startswith("@"): + return self.get_user_id_from_handle(user[1:]) + return self.get_user_id_from_email(user) + + def get_user_id_from_handle(self, handle: str) -> Optional[str]: + if self._handle_to_user_id_cache is None: + self._handle_to_user_id_cache = self._build_handle_to_user_id_map() + return self._handle_to_user_id_cache.get(handle.lower()) + + def _build_handle_to_user_id_map(self) -> Dict[str, str]: + members: List[dict] = [] + guests: List[dict] = [] + # Never block an alert on mention resolution. On failure, the users listed + # so far are still cached so a failing workspace isn't re-crawled per mention. + try: + for user in self._iter_users(): + if user.get("deleted") or user.get("is_bot"): + continue + if user.get("is_restricted") or user.get("is_ultra_restricted"): + guests.append(user) + else: + members.append(user) + except Exception as err: + if self.tracking: + self.tracking.record_internal_exception(err) + logger.error(f"Unable to list Slack users: {err}.") + + email_prefix_to_user_id: Dict[str, str] = {} + username_to_user_id: Dict[str, str] = {} + for user in members + guests: + email = (user.get("profile") or {}).get("email") + if email: + email_prefix = email.split("@")[0].lower() + email_prefix_to_user_id.setdefault(email_prefix, user["id"]) + if user.get("name"): + username_to_user_id.setdefault(user["name"].lower(), user["id"]) + if (members or guests) and not email_prefix_to_user_id: + logger.warning( + "No Slack user emails are visible, so @ mentions can't " + "be resolved. Make sure the Slack app has the users:read.email scope." + ) + return {**username_to_user_id, **email_prefix_to_user_id} + + def _iter_users(self) -> Iterator[dict]: + cursor: Optional[str] = None + while True: + response = self._list_users_page(cursor) + yield from response["members"] + next_cursor = (response.get("response_metadata") or {}).get("next_cursor") + if not next_cursor: + return + if not isinstance(next_cursor, str): + raise ValueError("Next cursor is not a string") + if next_cursor == cursor: + raise ValueError("Next cursor did not advance") + cursor = next_cursor + + def _request_users_page(self, cursor: Optional[str]) -> Any: + return self.client.users_list(cursor=cursor, limit=200) + @sleep_and_retry @limits(calls=50, period=ONE_MINUTE) def get_user_id_from_email(self, email: str) -> Optional[str]: diff --git a/tests/unit/messages/messaging_integrations/test_slack_web.py b/tests/unit/messages/messaging_integrations/test_slack_web.py new file mode 100644 index 000000000..62449ef2f --- /dev/null +++ b/tests/unit/messages/messaging_integrations/test_slack_web.py @@ -0,0 +1,160 @@ +from unittest.mock import MagicMock + +import pytest +from slack_sdk.errors import SlackApiError + +from elementary.messages.blocks import LineBlock, LinesBlock, MentionBlock +from elementary.messages.message_body import MessageBody +from elementary.messages.messaging_integrations.slack_web import ( + SlackWebMessageContext, + SlackWebMessagingIntegration, +) + +USERS_PAGE_1 = { + "members": [ + { + "id": "U_JESSICA", + "name": "jjones", + "profile": {"email": "jessica.jones@marvel.com"}, + }, + {"id": "U_BOT", "name": "bot", "is_bot": True, "profile": {}}, + ], + "response_metadata": {"next_cursor": "page2"}, +} +USERS_PAGE_2 = { + "members": [ + {"id": "U_LUKE", "name": "luke", "profile": {"email": "lcage@marvel.com"}}, + { + "id": "U_DELETED", + "name": "deleted", + "deleted": True, + "profile": {"email": "deleted@marvel.com"}, + }, + ], + "response_metadata": {"next_cursor": ""}, +} + + +def _build_integration() -> SlackWebMessagingIntegration: + client = MagicMock() + client.users_list.side_effect = lambda cursor=None, limit=None: ( + USERS_PAGE_2 if cursor == "page2" else USERS_PAGE_1 + ) + client.users_lookupByEmail.side_effect = lambda email: { + "user": {"id": f"U_EMAIL_{email}"} + } + return SlackWebMessagingIntegration(client) + + +def test_resolve_user_id_by_email_prefix_handle(): + integration = _build_integration() + assert integration.resolve_user_id("@jessica.jones") == "U_JESSICA" + assert integration.resolve_user_id("@Jessica.Jones") == "U_JESSICA" + + +def test_resolve_user_id_by_username_handle(): + integration = _build_integration() + assert integration.resolve_user_id("@jjones") == "U_JESSICA" + assert integration.resolve_user_id("@luke") == "U_LUKE" + + +def test_resolve_user_id_skips_bots_deleted_and_unknown_handles(): + integration = _build_integration() + assert integration.resolve_user_id("@bot") is None + assert integration.resolve_user_id("@deleted") is None + assert integration.resolve_user_id("@unknown") is None + + +def test_users_are_listed_once(): + integration = _build_integration() + integration.resolve_user_id("@jessica.jones") + integration.resolve_user_id("@luke") + assert integration.client.users_list.call_count == 2 + + +def test_resolve_user_id_by_email(): + integration = _build_integration() + assert ( + integration.resolve_user_id("jessica.jones@marvel.com") + == "U_EMAIL_jessica.jones@marvel.com" + ) + integration.client.users_list.assert_not_called() + + +def test_resolve_user_id_when_users_list_fails(): + integration = _build_integration() + integration.client.users_list.side_effect = SlackApiError( + "error", MagicMock(data={"error": "missing_scope"}) + ) + assert integration.resolve_user_id("@jessica.jones") is None + + +HANDLE_MENTION_BODY = MessageBody( + blocks=[ + LinesBlock(lines=[LineBlock(inlines=[MentionBlock(user="@jessica.jones")])]) + ] +) + + +def _send(integration: SlackWebMessagingIntegration, reply: bool) -> str: + integration.client.chat_postMessage.return_value = { + "ts": "123.456", + "channel": "C1", + } + if reply: + context = SlackWebMessageContext(id="111.222", channel="C1") + integration.reply_to_message("C1", context, HANDLE_MENTION_BODY) + else: + integration.send_message("C1", HANDLE_MENTION_BODY) + sent = integration.client.chat_postMessage.call_args.kwargs + return sent["blocks"] + sent["attachments"] + + +@pytest.mark.parametrize("reply", [False, True]) +def test_send_resolves_handle_mentions(reply): + integration = _build_integration() + assert "<@U_JESSICA>" in _send(integration, reply) + + +@pytest.mark.parametrize("reply", [False, True]) +def test_send_keeps_plain_handle_when_users_list_raises(reply): + integration = _build_integration() + integration.client.users_list.side_effect = ConnectionError("connection reset") + sent = _send(integration, reply) + assert "@jessica.jones" in sent + assert "<@" not in sent + + +def test_resolve_user_id_collisions(): + integration = _build_integration() + integration.client.users_list.side_effect = lambda cursor=None, limit=None: { + "members": [ + { + "id": "U_GUEST", + "name": "john", + "is_restricted": True, + "profile": {"email": "john@partner.com"}, + }, + {"id": "U_JOHN", "name": "jdoe", "profile": {"email": "john@company.com"}}, + {"id": "U_JOHN_2", "name": "jd", "profile": {"email": "john@other.com"}}, + { + "id": "U_JANE", + "name": "jane.doe", + "profile": {"email": "jd@company.com"}, + }, + ], + } + # Full members win over guests, then the first listed user wins. + assert integration.resolve_user_id("@john") == "U_JOHN" + # Email prefix matches win over username matches. + assert integration.resolve_user_id("@jd") == "U_JANE" + + +def test_resolve_user_id_when_users_list_cursor_does_not_advance(): + integration = _build_integration() + integration.client.users_list.side_effect = lambda cursor=None, limit=None: { + "members": USERS_PAGE_1["members"], + "response_metadata": {"next_cursor": "stuck"}, + } + assert integration.resolve_user_id("@jessica.jones") == "U_JESSICA" + assert integration.client.users_list.call_count == 2