Resolve @handle owner/subscriber mentions in Slack web integration

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 @<email prefix> 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]:

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