diff --git a/src/python_italy_bot/db/base.py b/src/python_italy_bot/db/base.py index 69622875c..8c7d749c8 100644 --- a/src/python_italy_bot/db/base.py +++ b/src/python_italy_bot/db/base.py @@ -184,6 +184,11 @@ def mark_welcomed(self, user_id: int, chat_id: int) -> None: """Mark user as having been welcomed in this chat.""" ... + @abstractmethod + def remove_welcomed(self, user_id: int, chat_id: int) -> None: + """Clear the welcomed flag for a user in this chat.""" + ... + # -- Welcome delay -- @abstractmethod @@ -394,6 +399,11 @@ async def mark_welcomed(self, user_id: int, chat_id: int) -> None: """Mark user as having been welcomed in this chat.""" ... + @abstractmethod + async def remove_welcomed(self, user_id: int, chat_id: int) -> None: + """Clear the welcomed flag for a user in this chat.""" + ... + # -- Welcome delay -- @abstractmethod diff --git a/src/python_italy_bot/db/in_memory.py b/src/python_italy_bot/db/in_memory.py index 556e1905f..b458bedb0 100644 --- a/src/python_italy_bot/db/in_memory.py +++ b/src/python_italy_bot/db/in_memory.py @@ -216,6 +216,9 @@ async def has_been_welcomed(self, user_id: int, chat_id: int) -> bool: async def mark_welcomed(self, user_id: int, chat_id: int) -> None: self._welcomed.add((user_id, chat_id)) + async def remove_welcomed(self, user_id: int, chat_id: int) -> None: + self._welcomed.discard((user_id, chat_id)) + # -- Welcome delay -- async def get_welcome_delay(self, chat_id: int) -> int | None: diff --git a/src/python_italy_bot/db/postgres.py b/src/python_italy_bot/db/postgres.py index 4c9cbdccd..064bfe92a 100644 --- a/src/python_italy_bot/db/postgres.py +++ b/src/python_italy_bot/db/postgres.py @@ -366,6 +366,13 @@ async def mark_welcomed(self, user_id: int, chat_id: int) -> None: (user_id, chat_id), ) + async def remove_welcomed(self, user_id: int, chat_id: int) -> None: + async with self._pool.connection() as conn: + await conn.execute( + "DELETE FROM welcomed_users WHERE user_id = %s AND chat_id = %s", + (user_id, chat_id), + ) + # -- Welcome delay -- async def get_welcome_delay(self, chat_id: int) -> int | None: diff --git a/src/python_italy_bot/handlers/moderation.py b/src/python_italy_bot/handlers/moderation.py index a859acaab..bc090e663 100644 --- a/src/python_italy_bot/handlers/moderation.py +++ b/src/python_italy_bot/handlers/moderation.py @@ -36,6 +36,7 @@ def create_moderation_handlers(moderation_service: ModerationService) -> list: CommandHandler("unban", _handle_unban), CommandHandler("mute", _handle_mute), CommandHandler("unmute", _handle_unmute), + CommandHandler("unlock", _handle_unlock), CommandHandler("report", _handle_report), CommandHandler("forcegroupregistration", _handle_force_group_registration), MessageHandler( @@ -376,6 +377,61 @@ async def _handle_unmute(update: Update, context: ContextTypes.DEFAULT_TYPE) -> await message.reply_text(strings.UNMUTE_FAILED) +async def _handle_unlock(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: + """Clear a captcha lock: verify the user globally and restore permissions. + + Distinct from /unmute (which reverses a moderation /mute). /unlock frees a + user stuck in the captcha 'pending verification' state. Usage: /unlock @username. + """ + moderation_service: ModerationService = context.bot_data["moderation_service"] + captcha_service: CaptchaService = context.bot_data["captcha_service"] + message = update.message + if message is None or message.from_user is None: + return + + chat = update.effective_chat + if chat is None or chat.type == "private": + return + + if not await _is_admin(context, chat.id, message.from_user.id): + await message.reply_text(strings.ONLY_ADMINS) + return + + args = message.text.split(maxsplit=1)[1:] if message.text else [] + user_id: int | None = None + if message.reply_to_message and message.reply_to_message.from_user: + user_id = message.reply_to_message.from_user.id + elif args: + user_id = await _resolve_user_id(context, chat.id, args[0], moderation_service) + + if user_id is None: + await message.reply_text(strings.UNLOCK_USAGE) + return + + # Capture pending chats before global verification clears them, so we know + # where to restore permissions. Always include the current chat as well. + pending_chats = await captcha_service.get_pending_chats(user_id) + await captcha_service.verify_user_globally(user_id) + + full_perms = captcha_service.get_full_permissions() + for chat_id in {chat.id, *pending_chats}: + try: + await context.bot.restrict_chat_member( + chat_id=chat_id, + user_id=user_id, + permissions=full_perms, + ) + except Exception as e: + logger.warning( + "Unlock: could not restore permissions for %s in chat %s: %s", + user_id, + chat_id, + e, + ) + + await message.reply_text(strings.UNLOCK_SUCCESS) + + async def _handle_report(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """Report a message or user. Usage: /report [reason] or reply to message with /report [reason].""" moderation_service: ModerationService = context.bot_data["moderation_service"] diff --git a/src/python_italy_bot/handlers/welcome.py b/src/python_italy_bot/handlers/welcome.py index 9e79fb77f..d4a2b24b1 100644 --- a/src/python_italy_bot/handlers/welcome.py +++ b/src/python_italy_bot/handlers/welcome.py @@ -82,6 +82,17 @@ async def _handle_new_member( new_status = result.new_chat_member.status old_status = result.old_chat_member.status if result.old_chat_member else None + # User left or was kicked/banned: clear per-chat welcome + pending state so a + # genuine rejoin re-triggers the captcha instead of being silently re-muted. + # Global verification is intentionally preserved (verified users stay verified). + if new_status in (ChatMemberStatus.LEFT, ChatMemberStatus.BANNED): + departing = result.new_chat_member.user + chat = update.effective_chat + if departing is not None and chat is not None: + await captcha_service.remove_welcomed(departing.id, chat.id) + await captcha_service.remove_pending(departing.id, chat.id) + return + if new_status not in (ChatMemberStatus.MEMBER, ChatMemberStatus.RESTRICTED): return if old_status in ( diff --git a/src/python_italy_bot/services/captcha.py b/src/python_italy_bot/services/captcha.py index d05dbb9c4..6accd39d3 100644 --- a/src/python_italy_bot/services/captcha.py +++ b/src/python_italy_bot/services/captcha.py @@ -164,6 +164,10 @@ async def add_pending(self, user_id: int, chat_id: int) -> None: """Record that user joined and needs verification.""" await self._repo.add_pending_verification(user_id, chat_id) + async def remove_pending(self, user_id: int, chat_id: int) -> None: + """Remove a single pending-verification entry for a user in a chat.""" + await self._repo.remove_pending(user_id, chat_id) + async def is_globally_verified(self, user_id: int) -> bool: """Check if user is globally verified.""" return await self._repo.is_globally_verified(user_id) @@ -178,6 +182,10 @@ async def mark_welcomed(self, user_id: int, chat_id: int) -> None: """Mark user as having been welcomed in this chat.""" await self._repo.mark_welcomed(user_id, chat_id) + async def remove_welcomed(self, user_id: int, chat_id: int) -> None: + """Clear the welcomed flag for a user in this chat (e.g. on departure).""" + await self._repo.remove_welcomed(user_id, chat_id) + # -- Welcome delay -- async def get_welcome_delay(self, chat_id: int) -> int | None: diff --git a/src/python_italy_bot/strings.py b/src/python_italy_bot/strings.py index a05ee32c7..13432362c 100644 --- a/src/python_italy_bot/strings.py +++ b/src/python_italy_bot/strings.py @@ -173,6 +173,10 @@ def mute_success(duration: int | None, reason: str | None) -> str: UNMUTE_SUCCESS = "Utente smutato." UNMUTE_FAILED = "Impossibile smutare l'utente." +# Unlock (clear captcha lock for a user stuck pending verification) +UNLOCK_USAGE = "Uso: /unlock @username, /unlock user_id, o rispondi al messaggio" +UNLOCK_SUCCESS = "Utente sbloccato. La verifica captcha non è più richiesta." + # Report REPORT_USAGE = "Rispondi al messaggio da segnalare con /report [motivo]" REPORT_SUCCESS = "Segnalazione inviata. Gli amministratori la esamineranno." diff --git a/tests/test_unlock.py b/tests/test_unlock.py new file mode 100644 index 000000000..14ae162d1 --- /dev/null +++ b/tests/test_unlock.py @@ -0,0 +1,102 @@ +"""Tests for the /unlock command. + +/unlock is the admin tool to free a user who is stuck in the captcha +('pending verification') state — distinct from /unmute, which reverses a +moderation /mute. It globally verifies the user, clears pending state, and +restores send permissions in their pending chats. +""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock + +from telegram.constants import ChatMemberStatus + +from python_italy_bot import strings +from python_italy_bot.db.in_memory import InMemoryRepository +from python_italy_bot.handlers.moderation import _handle_unlock +from python_italy_bot.services.captcha import CaptchaService +from python_italy_bot.services.moderation import ModerationService + + +def _make_setup(admin_status: ChatMemberStatus = ChatMemberStatus.ADMINISTRATOR): + repo = InMemoryRepository() + captcha = CaptchaService(repo, "python-italy", "assets/regolamento.md") + moderation = ModerationService(repo) + + bot = AsyncMock() + bot.get_chat_member = AsyncMock(return_value=SimpleNamespace(status=admin_status)) + bot.restrict_chat_member = AsyncMock() + + admin = SimpleNamespace(id=1) + target = SimpleNamespace(id=999) + reply = SimpleNamespace(from_user=target) + message = SimpleNamespace( + from_user=admin, + text="/unlock", + reply_to_message=reply, + reply_text=AsyncMock(), + ) + chat = SimpleNamespace(id=-100, type="supergroup", title="PythonMilano") + update = SimpleNamespace(message=message, effective_chat=chat) + context = SimpleNamespace( + bot=bot, + bot_data={"moderation_service": moderation, "captcha_service": captcha}, + ) + return repo, update, context, bot, message + + +def test_unlock_verifies_globally_and_clears_pending() -> None: + """/unlock marks the user globally verified and clears pending state.""" + repo, update, context, bot, message = _make_setup() + asyncio.run(repo.add_pending_verification(999, -100)) + + asyncio.run(_handle_unlock(update, context)) + + assert asyncio.run(repo.is_globally_verified(999)) is True + assert asyncio.run(repo.get_pending_chats(999)) == [] + message.reply_text.assert_awaited_with(strings.UNLOCK_SUCCESS) + + +def test_unlock_restores_permissions_in_chat() -> None: + """/unlock restores send permissions via restrict_chat_member.""" + repo, update, context, bot, message = _make_setup() + asyncio.run(repo.add_pending_verification(999, -100)) + + asyncio.run(_handle_unlock(update, context)) + + bot.restrict_chat_member.assert_awaited() + # full permissions must allow sending messages + kwargs = bot.restrict_chat_member.await_args.kwargs + perms = kwargs["permissions"] + assert perms.can_send_messages is True + # defensive: must NOT elevate pin / change-info above group default + assert perms.can_change_info is False + assert perms.can_pin_messages is False + + +def test_unlock_rejects_non_admin() -> None: + """A non-admin cannot use /unlock; no state changes occur.""" + repo, update, context, bot, message = _make_setup( + admin_status=ChatMemberStatus.MEMBER + ) + asyncio.run(repo.add_pending_verification(999, -100)) + + asyncio.run(_handle_unlock(update, context)) + + assert asyncio.run(repo.is_globally_verified(999)) is False + assert asyncio.run(repo.get_pending_chats(999)) == [-100] + bot.restrict_chat_member.assert_not_awaited() + message.reply_text.assert_awaited_with(strings.ONLY_ADMINS) + + +def test_unlock_ignored_in_private_chat() -> None: + """/unlock does nothing in a private chat.""" + repo, update, context, bot, message = _make_setup() + update.effective_chat = SimpleNamespace(id=1, type="private", title=None) + asyncio.run(repo.add_pending_verification(999, -100)) + + asyncio.run(_handle_unlock(update, context)) + + assert asyncio.run(repo.is_globally_verified(999)) is False + bot.restrict_chat_member.assert_not_awaited() diff --git a/tests/test_welcome_rejoin.py b/tests/test_welcome_rejoin.py new file mode 100644 index 000000000..98acfd4b4 --- /dev/null +++ b/tests/test_welcome_rejoin.py @@ -0,0 +1,106 @@ +"""Tests for the rejoin fix: leaving a chat clears welcome/pending state. + +A user who is restricted by the captcha flow and then leaves must have the +'welcomed' and 'pending' flags cleared, so that a genuine rejoin re-triggers +the captcha instead of being silently re-muted forever. +""" + +import asyncio +from types import SimpleNamespace +from unittest.mock import AsyncMock + +from telegram.constants import ChatMemberStatus + +from python_italy_bot.db.in_memory import InMemoryRepository +from python_italy_bot.handlers.welcome import _handle_new_member +from python_italy_bot.services.captcha import CaptchaService +from python_italy_bot.services.moderation import ModerationService + + +# -- Repository: remove_welcomed -- + + +def test_remove_welcomed_clears_flag() -> None: + """remove_welcomed clears a previously set welcomed flag.""" + repo = InMemoryRepository() + asyncio.run(repo.mark_welcomed(999, -100)) + assert asyncio.run(repo.has_been_welcomed(999, -100)) is True + asyncio.run(repo.remove_welcomed(999, -100)) + assert asyncio.run(repo.has_been_welcomed(999, -100)) is False + + +def test_remove_welcomed_missing_is_noop() -> None: + """remove_welcomed on a missing entry does not raise.""" + repo = InMemoryRepository() + asyncio.run(repo.remove_welcomed(999, -100)) + assert asyncio.run(repo.has_been_welcomed(999, -100)) is False + + +# -- Handler: departure clears welcome/pending state -- + + +def _make_departure_update(status: ChatMemberStatus): + user = SimpleNamespace(id=999, is_bot=False, username="u", full_name="U") + new_member = SimpleNamespace(status=status, user=user) + old_member = SimpleNamespace(status=ChatMemberStatus.RESTRICTED) + chat_member = SimpleNamespace( + new_chat_member=new_member, old_chat_member=old_member + ) + chat = SimpleNamespace(id=-100, title="PythonMilano", type="supergroup") + return SimpleNamespace(chat_member=chat_member, effective_chat=chat) + + +def _make_context(repo: InMemoryRepository): + captcha = CaptchaService(repo, "python-italy", "assets/regolamento.md") + moderation = ModerationService(repo) + return SimpleNamespace( + bot=AsyncMock(), + bot_data={ + "captcha_service": captcha, + "moderation_service": moderation, + "repository": repo, + }, + ) + + +def test_member_leaving_clears_welcome_and_pending() -> None: + """When a restricted member leaves, welcomed + pending state is cleared.""" + repo = InMemoryRepository() + asyncio.run(repo.mark_welcomed(999, -100)) + asyncio.run(repo.add_pending_verification(999, -100)) + + update = _make_departure_update(ChatMemberStatus.LEFT) + context = _make_context(repo) + + asyncio.run(_handle_new_member(update, context)) + + assert asyncio.run(repo.has_been_welcomed(999, -100)) is False + assert asyncio.run(repo.get_pending_chats(999)) == [] + + +def test_member_kicked_clears_welcome_and_pending() -> None: + """A kicked/banned member also has welcomed + pending state cleared.""" + repo = InMemoryRepository() + asyncio.run(repo.mark_welcomed(999, -100)) + asyncio.run(repo.add_pending_verification(999, -100)) + + update = _make_departure_update(ChatMemberStatus.BANNED) + context = _make_context(repo) + + asyncio.run(_handle_new_member(update, context)) + + assert asyncio.run(repo.has_been_welcomed(999, -100)) is False + assert asyncio.run(repo.get_pending_chats(999)) == [] + + +def test_leaving_does_not_clear_global_verification() -> None: + """Leaving must NOT clear global verification (verified users stay verified).""" + repo = InMemoryRepository() + asyncio.run(repo.mark_globally_verified(999)) + + update = _make_departure_update(ChatMemberStatus.LEFT) + context = _make_context(repo) + + asyncio.run(_handle_new_member(update, context)) + + assert asyncio.run(repo.is_globally_verified(999)) is True