fix(gateway): allow chat-scoped telegram auth without sender user_id
This commit is contained in:
@@ -5851,6 +5851,33 @@ class GatewayRunner:
|
|||||||
return True
|
return True
|
||||||
|
|
||||||
user_id = source.user_id
|
user_id = source.user_id
|
||||||
|
|
||||||
|
# Telegram (and similar) authorize entire group/forum chats by
|
||||||
|
# chat ID via TELEGRAM_GROUP_ALLOWED_CHATS / QQ_GROUP_ALLOWED_USERS.
|
||||||
|
# That allowlist is chat-scoped, so it must work even when
|
||||||
|
# source.user_id is None — Telegram emits anonymous-admin posts
|
||||||
|
# and sender_chat traffic in groups with no `from_user`, and an
|
||||||
|
# operator who explicitly listed the chat expects those to be
|
||||||
|
# honored. Run this check before the no-user-id guard below so
|
||||||
|
# documented behavior matches reality
|
||||||
|
# (website/docs/reference/environment-variables.md,
|
||||||
|
# website/docs/user-guide/messaging/telegram.md).
|
||||||
|
if source.chat_type in {"group", "forum"} and source.chat_id:
|
||||||
|
chat_allowlist_env = {
|
||||||
|
Platform.TELEGRAM: "TELEGRAM_GROUP_ALLOWED_CHATS",
|
||||||
|
Platform.QQBOT: "QQ_GROUP_ALLOWED_USERS",
|
||||||
|
}.get(source.platform, "")
|
||||||
|
if chat_allowlist_env:
|
||||||
|
raw_chat_allowlist = os.getenv(chat_allowlist_env, "").strip()
|
||||||
|
if raw_chat_allowlist:
|
||||||
|
allowed_group_ids = {
|
||||||
|
cid.strip()
|
||||||
|
for cid in raw_chat_allowlist.split(",")
|
||||||
|
if cid.strip()
|
||||||
|
}
|
||||||
|
if "*" in allowed_group_ids or source.chat_id in allowed_group_ids:
|
||||||
|
return True
|
||||||
|
|
||||||
if not user_id:
|
if not user_id:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
@@ -6197,11 +6224,14 @@ class GatewayRunner:
|
|||||||
pass
|
pass
|
||||||
elif source.user_id is None:
|
elif source.user_id is None:
|
||||||
# Messages with no user identity (Telegram service messages,
|
# Messages with no user identity (Telegram service messages,
|
||||||
# channel forwards, anonymous admin actions) cannot be
|
# channel forwards, anonymous admin posts, sender_chat) can't
|
||||||
# authorized — drop silently instead of triggering the pairing
|
# be paired, but they can still be authorized via a
|
||||||
# flow with a None user_id.
|
# chat-scoped allowlist (e.g. TELEGRAM_GROUP_ALLOWED_CHATS
|
||||||
logger.debug("Ignoring message with no user_id from %s", source.platform.value)
|
# authorizes every member of the listed chat regardless of
|
||||||
return None
|
# sender). Defer to _is_user_authorized so that path runs.
|
||||||
|
if not self._is_user_authorized(source):
|
||||||
|
logger.debug("Ignoring message with no user_id from %s", source.platform.value)
|
||||||
|
return None
|
||||||
elif not self._is_user_authorized(source):
|
elif not self._is_user_authorized(source):
|
||||||
logger.warning("Unauthorized user: %s (%s) on %s", source.user_id, source.user_name, source.platform.value)
|
logger.warning("Unauthorized user: %s (%s) on %s", source.user_id, source.user_name, source.platform.value)
|
||||||
# In DMs: offer pairing code. In groups: silently ignore.
|
# In DMs: offer pairing code. In groups: silently ignore.
|
||||||
|
|||||||
@@ -276,6 +276,133 @@ def test_telegram_group_chat_allowlist_authorizes_group_chat_without_user_allowl
|
|||||||
assert runner._is_user_authorized(source) is True
|
assert runner._is_user_authorized(source) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_telegram_group_chat_allowlist_authorizes_anonymous_sender(monkeypatch):
|
||||||
|
"""TELEGRAM_GROUP_ALLOWED_CHATS must authorize chat traffic with no
|
||||||
|
sender user_id (Telegram anonymous-admin posts, sender_chat). The
|
||||||
|
docs state the chat allowlist authorizes "every member of that chat,
|
||||||
|
regardless of sender" — anonymous senders had been silently dropped
|
||||||
|
despite an explicit chat opt-in.
|
||||||
|
"""
|
||||||
|
_clear_auth_env(monkeypatch)
|
||||||
|
monkeypatch.setenv("TELEGRAM_GROUP_ALLOWED_CHATS", "-1001878443972")
|
||||||
|
|
||||||
|
runner, _adapter = _make_runner(
|
||||||
|
Platform.TELEGRAM,
|
||||||
|
GatewayConfig(platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="t")}),
|
||||||
|
)
|
||||||
|
|
||||||
|
source = SessionSource(
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
user_id=None,
|
||||||
|
chat_id="-1001878443972",
|
||||||
|
user_name=None,
|
||||||
|
chat_type="group",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert runner._is_user_authorized(source) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_telegram_group_chat_allowlist_rejects_anonymous_sender_in_other_chat(monkeypatch):
|
||||||
|
"""Anonymous senders in a chat *not* on the allowlist must still be
|
||||||
|
rejected — the early no-user-id path must not become an open gate.
|
||||||
|
"""
|
||||||
|
_clear_auth_env(monkeypatch)
|
||||||
|
monkeypatch.setenv("TELEGRAM_GROUP_ALLOWED_CHATS", "-1001878443972")
|
||||||
|
|
||||||
|
runner, _adapter = _make_runner(
|
||||||
|
Platform.TELEGRAM,
|
||||||
|
GatewayConfig(platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="t")}),
|
||||||
|
)
|
||||||
|
|
||||||
|
source = SessionSource(
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
user_id=None,
|
||||||
|
chat_id="-1009999999999",
|
||||||
|
user_name=None,
|
||||||
|
chat_type="group",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert runner._is_user_authorized(source) is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_handle_message_does_not_drop_anonymous_sender_in_allowlisted_chat(monkeypatch):
|
||||||
|
"""End-to-end: a group message with from_user=None in an allowlisted
|
||||||
|
chat must reach the dispatch path — not get silently dropped by the
|
||||||
|
no-user-id guard, and not trigger pairing (anonymous senders can't
|
||||||
|
be paired anyway).
|
||||||
|
"""
|
||||||
|
_clear_auth_env(monkeypatch)
|
||||||
|
monkeypatch.setenv("TELEGRAM_GROUP_ALLOWED_CHATS", "-1001878443972")
|
||||||
|
|
||||||
|
config = GatewayConfig(
|
||||||
|
platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="t")},
|
||||||
|
)
|
||||||
|
runner, adapter = _make_runner(Platform.TELEGRAM, config)
|
||||||
|
|
||||||
|
# Force _handle_message to bail with a sentinel right after the
|
||||||
|
# auth gate, so a successful "auth passed" call can be distinguished
|
||||||
|
# from the buggy "silently dropped" case (which would return None
|
||||||
|
# before this hook ever runs).
|
||||||
|
reached_dispatch = MagicMock(side_effect=RuntimeError("reached dispatch"))
|
||||||
|
runner._session_key_for_source = reached_dispatch
|
||||||
|
|
||||||
|
event = MessageEvent(
|
||||||
|
text="hi",
|
||||||
|
message_id="m1",
|
||||||
|
source=SessionSource(
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
user_id=None,
|
||||||
|
chat_id="-1001878443972",
|
||||||
|
user_name=None,
|
||||||
|
chat_type="group",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(RuntimeError, match="reached dispatch"):
|
||||||
|
await runner._handle_message(event)
|
||||||
|
|
||||||
|
reached_dispatch.assert_called_once()
|
||||||
|
runner.pairing_store.generate_code.assert_not_called()
|
||||||
|
adapter.send.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_handle_message_drops_anonymous_sender_outside_allowlist(monkeypatch):
|
||||||
|
"""Anonymous senders in a chat *not* on the allowlist remain silently
|
||||||
|
dropped — the fix must not become a backdoor for unauthorized chats.
|
||||||
|
"""
|
||||||
|
_clear_auth_env(monkeypatch)
|
||||||
|
monkeypatch.setenv("TELEGRAM_GROUP_ALLOWED_CHATS", "-1001878443972")
|
||||||
|
|
||||||
|
config = GatewayConfig(
|
||||||
|
platforms={Platform.TELEGRAM: PlatformConfig(enabled=True, token="t")},
|
||||||
|
)
|
||||||
|
runner, adapter = _make_runner(Platform.TELEGRAM, config)
|
||||||
|
|
||||||
|
must_not_run = MagicMock(side_effect=AssertionError("auth gate did not drop"))
|
||||||
|
runner._session_key_for_source = must_not_run
|
||||||
|
|
||||||
|
event = MessageEvent(
|
||||||
|
text="hi",
|
||||||
|
message_id="m1",
|
||||||
|
source=SessionSource(
|
||||||
|
platform=Platform.TELEGRAM,
|
||||||
|
user_id=None,
|
||||||
|
chat_id="-1009999999999",
|
||||||
|
user_name=None,
|
||||||
|
chat_type="group",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = await runner._handle_message(event)
|
||||||
|
|
||||||
|
assert result is None
|
||||||
|
must_not_run.assert_not_called()
|
||||||
|
runner.pairing_store.generate_code.assert_not_called()
|
||||||
|
adapter.send.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
def test_telegram_group_users_legacy_chat_ids_still_authorize(monkeypatch):
|
def test_telegram_group_users_legacy_chat_ids_still_authorize(monkeypatch):
|
||||||
"""Backward-compat: PR #15027 shipped TELEGRAM_GROUP_ALLOWED_USERS as a
|
"""Backward-compat: PR #15027 shipped TELEGRAM_GROUP_ALLOWED_USERS as a
|
||||||
chat-ID allowlist. PR #17686 renamed it to sender IDs and added
|
chat-ID allowlist. PR #17686 renamed it to sender IDs and added
|
||||||
|
|||||||
Reference in New Issue
Block a user