fix(channels): accept single slack allowed user (#2481)
Some checks are pending
Unit Tests / backend-unit-tests (push) Waiting to run
Frontend Unit Tests / frontend-unit-tests (push) Waiting to run
Lint Check / lint (push) Waiting to run
Lint Check / lint-frontend (push) Waiting to run

* fix(channels): accept single slack allowed user

* docs: address Slack allowed_users review notes

* ci: rerun backend unit tests

* docs: clarify Slack allowed_users config

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
ming1523 2026-04-25 19:40:52 +08:00 committed by GitHub
parent 1f59e945af
commit 410f0c48b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 95 additions and 8 deletions

View file

@ -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:

View file

@ -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

View file

@ -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