diff --git a/backend/app/channels/slack.py b/backend/app/channels/slack.py index c9ad6a6e..65cb36cf 100644 --- a/backend/app/channels/slack.py +++ b/backend/app/channels/slack.py @@ -16,13 +16,31 @@ logger = logging.getLogger(__name__) _slack_md_converter = SlackMarkdownConverter() +def _normalize_allowed_users(allowed_users: Any) -> set[str]: + if allowed_users is None: + return set() + if isinstance(allowed_users, str): + values = [allowed_users] + elif isinstance(allowed_users, list | tuple | set): + values = allowed_users + else: + logger.warning( + "Slack allowed_users should be a list of Slack user IDs or a single Slack user ID string; treating %s as one string value", + type(allowed_users).__name__, + ) + values = [allowed_users] + return {str(user_id) for user_id in values if str(user_id)} + + class SlackChannel(Channel): """Slack IM channel using Socket Mode (WebSocket, no public IP). Configuration keys (in ``config.yaml`` under ``channels.slack``): - ``bot_token``: Slack Bot User OAuth Token (xoxb-...). - ``app_token``: Slack App-Level Token (xapp-...) for Socket Mode. - - ``allowed_users``: (optional) List of allowed Slack user IDs. Empty = allow all. + - ``allowed_users``: (optional) List of allowed Slack user IDs, or a + single Slack user ID string as shorthand. Empty = allow all. Other + scalar values are treated as a single string with a warning. """ def __init__(self, bus: MessageBus, config: dict[str, Any]) -> None: @@ -30,7 +48,7 @@ class SlackChannel(Channel): self._socket_client = None self._web_client = None self._loop: asyncio.AbstractEventLoop | None = None - self._allowed_users: set[str] = {str(user_id) for user_id in config.get("allowed_users", [])} + self._allowed_users = _normalize_allowed_users(config.get("allowed_users", [])) async def start(self) -> None: if self._running: diff --git a/backend/tests/test_channels.py b/backend/tests/test_channels.py index 7fc41265..e6fb0213 100644 --- a/backend/tests/test_channels.py +++ b/backend/tests/test_channels.py @@ -2046,6 +2046,11 @@ class TestSlackSendRetry: class TestSlackAllowedUsers: + @staticmethod + def _submit_coro(coro, loop): + coro.close() + return MagicMock() + def test_numeric_allowed_users_match_string_event_user_id(self): from app.channels.slack import SlackChannel @@ -2067,13 +2072,9 @@ class TestSlackAllowedUsers: "ts": "1710000000.000100", } - def submit_coro(coro, loop): - coro.close() - return MagicMock() - with patch( "app.channels.slack.asyncio.run_coroutine_threadsafe", - side_effect=submit_coro, + side_effect=self._submit_coro, ) as submit: channel._handle_message_event(event) @@ -2085,6 +2086,74 @@ class TestSlackAllowedUsers: assert inbound.chat_id == "C123" assert inbound.text == "hello from slack" + def test_string_allowed_users_match_event_user_id(self): + from app.channels.slack import SlackChannel + + bus = MessageBus() + bus.publish_inbound = AsyncMock() + channel = SlackChannel( + bus=bus, + config={"allowed_users": "U123456"}, + ) + channel._loop = MagicMock() + channel._loop.is_running.return_value = True + channel._add_reaction = MagicMock() + channel._send_running_reply = MagicMock() + + event = { + "user": "U123456", + "text": "hello from slack", + "channel": "C123", + "ts": "1710000000.000100", + } + + with patch( + "app.channels.slack.asyncio.run_coroutine_threadsafe", + side_effect=self._submit_coro, + ) as submit: + channel._handle_message_event(event) + + channel._add_reaction.assert_called_once_with("C123", "1710000000.000100", "eyes") + channel._send_running_reply.assert_called_once_with("C123", "1710000000.000100") + submit.assert_called_once() + inbound = bus.publish_inbound.call_args.args[0] + assert inbound.user_id == "U123456" + assert inbound.chat_id == "C123" + assert inbound.text == "hello from slack" + + def test_scalar_allowed_users_warns_and_matches_stringified_event_user_id(self, caplog): + from app.channels.slack import SlackChannel + + bus = MessageBus() + bus.publish_inbound = AsyncMock() + with caplog.at_level("WARNING"): + channel = SlackChannel( + bus=bus, + config={"allowed_users": 123456}, + ) + channel._loop = MagicMock() + channel._loop.is_running.return_value = True + channel._add_reaction = MagicMock() + channel._send_running_reply = MagicMock() + + event = { + "user": "123456", + "text": "hello from slack", + "channel": "C123", + "ts": "1710000000.000100", + } + + with patch( + "app.channels.slack.asyncio.run_coroutine_threadsafe", + side_effect=self._submit_coro, + ) as submit: + channel._handle_message_event(event) + + assert "Slack allowed_users should be a list" in caplog.text + submit.assert_called_once() + inbound = bus.publish_inbound.call_args.args[0] + assert inbound.user_id == "123456" + def test_raises_after_all_retries_exhausted(self): from app.channels.slack import SlackChannel diff --git a/config.example.yaml b/config.example.yaml index 32a94105..b9f7a963 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -867,7 +867,7 @@ checkpointer: # enabled: false # bot_token: $SLACK_BOT_TOKEN # xoxb-... # app_token: $SLACK_APP_TOKEN # xapp-... (Socket Mode) -# allowed_users: [] # empty = allow all +# allowed_users: [] # empty = allow all; can also be a single Slack user ID string, e.g. U123456, but list form is recommended # # telegram: # enabled: false