From 494e71c1731bab5798eb79d19177a34cba7cf8b0 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 31 Jul 2026 13:08:06 +0700 Subject: [PATCH 1/5] feat: upgrade to PTB 22.8, exclude bots from admin caches - Bump python-telegram-bot dependency from >=22.5 to >=22.8 - Pass api_kwargs={'return_bots': False} to get_chat_administrators - Also filter admin.user.is_bot client-side as a safety net - Update tests to set is_bot=False on mock admins and verify bot exclusion - Add test_fetch_admins_excludes_bots test case --- pyproject.toml | 2 +- src/bot/services/telegram_utils.py | 17 ++++++++++----- tests/test_telegram_utils.py | 35 ++++++++++++++++++++++++++++-- uv.lock | 8 +++---- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4641584..b795699 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ dependencies = [ "logfire>=2.6.1", "pydantic>=2.12.5", "pydantic-settings>=2.14.2", - "python-telegram-bot[job-queue]>=22.5", + "python-telegram-bot[job-queue]>=22.8", "sqlmodel>=0.0.28", ] diff --git a/src/bot/services/telegram_utils.py b/src/bot/services/telegram_utils.py index a59dde6..529d6b6 100644 --- a/src/bot/services/telegram_utils.py +++ b/src/bot/services/telegram_utils.py @@ -353,22 +353,29 @@ async def restrict_chat_member_with_retry( async def fetch_group_admin_ids(bot: Bot, group_id: int) -> list[int]: """ - Fetch all administrator user IDs from a group. + Fetch all human administrator user IDs from a group. + + Bot accounts are excluded from the returned list so that automated + admin-bots do not get treated as human admins for authorization + or bypass decisions. Args: bot: Telegram bot instance. group_id: Telegram group ID. Returns: - list[int]: List of admin user IDs (including creator and administrators). + list[int]: List of human admin user IDs (creator + administrators). Raises: TelegramAdminFetchError: If unable to fetch administrators (bot not in group, etc.). """ try: - admins = await bot.get_chat_administrators(group_id) - admin_ids = [admin.user.id for admin in admins] - logger.info(f"Fetched {len(admin_ids)} admins from group_id={group_id}") + admins = await bot.get_chat_administrators( + group_id, + api_kwargs={"return_bots": False}, + ) + admin_ids = [admin.user.id for admin in admins if not admin.user.is_bot] + logger.info(f"Fetched {len(admin_ids)} human admins from group_id={group_id}") return admin_ids except (BadRequest, Forbidden) as e: logger.error( diff --git a/tests/test_telegram_utils.py b/tests/test_telegram_utils.py index eb4dea3..e716c4f 100644 --- a/tests/test_telegram_utils.py +++ b/tests/test_telegram_utils.py @@ -509,26 +509,32 @@ async def test_fetch_single_admin(self, mock_bot): admin = MagicMock() admin.user = MagicMock() admin.user.id = 123 + admin.user.is_bot = False mock_bot.get_chat_administrators.return_value = [admin] result = await fetch_group_admin_ids(mock_bot, group_id=456) assert result == [123] - mock_bot.get_chat_administrators.assert_called_once_with(456) + mock_bot.get_chat_administrators.assert_called_once_with( + 456, api_kwargs={"return_bots": False} + ) async def test_fetch_multiple_admins(self, mock_bot): """Test fetching multiple admin IDs.""" admin1 = MagicMock() admin1.user = MagicMock() admin1.user.id = 111 + admin1.user.is_bot = False admin2 = MagicMock() admin2.user = MagicMock() admin2.user.id = 222 + admin2.user.is_bot = False admin3 = MagicMock() admin3.user = MagicMock() admin3.user.id = 333 + admin3.user.is_bot = False mock_bot.get_chat_administrators.return_value = [admin1, admin2, admin3] @@ -545,6 +551,7 @@ async def test_fetch_admins_preserves_order(self, mock_bot): admin = MagicMock() admin.user = MagicMock() admin.user.id = admin_id + admin.user.is_bot = False admins.append(admin) mock_bot.get_chat_administrators.return_value = admins @@ -579,12 +586,15 @@ async def test_fetch_admins_with_negative_group_id(self, mock_bot): admin = MagicMock() admin.user = MagicMock() admin.user.id = 123 + admin.user.is_bot = False mock_bot.get_chat_administrators.return_value = [admin] result = await fetch_group_admin_ids(mock_bot, group_id=-1001234567890) assert result == [123] - mock_bot.get_chat_administrators.assert_called_once_with(-1001234567890) + mock_bot.get_chat_administrators.assert_called_once_with( + -1001234567890, api_kwargs={"return_bots": False} + ) async def test_fetch_admins_empty_list(self, mock_bot): """Test when group has no admins (edge case).""" @@ -603,6 +613,7 @@ async def test_fetch_admins_large_group(self, mock_bot): admin = MagicMock() admin.user = MagicMock() admin.user.id = admin_id + admin.user.is_bot = False admins.append(admin) mock_bot.get_chat_administrators.return_value = admins @@ -617,6 +628,7 @@ async def test_fetch_admins_with_large_ids(self, mock_bot): admin = MagicMock() admin.user = MagicMock() admin.user.id = 9999999999 + admin.user.is_bot = False mock_bot.get_chat_administrators.return_value = [admin] result = await fetch_group_admin_ids(mock_bot, group_id=123) @@ -646,6 +658,25 @@ async def test_fetch_admins_different_exceptions(self, mock_bot): with pytest.raises(Exception): await fetch_group_admin_ids(mock_bot, group_id=456) + async def test_fetch_admins_excludes_bots(self, mock_bot): + """Test that bot accounts are excluded from admin IDs.""" + human_admin = MagicMock() + human_admin.user = MagicMock() + human_admin.user.id = 111 + human_admin.user.is_bot = False + + bot_admin = MagicMock() + bot_admin.user = MagicMock() + bot_admin.user.id = 222 + bot_admin.user.is_bot = True + + mock_bot.get_chat_administrators.return_value = [human_admin, bot_admin] + + result = await fetch_group_admin_ids(mock_bot, group_id=456) + + assert result == [111] + assert 222 not in result + class TestSendMessageWithRetry: """send_message_with_retry handles RetryAfter correctly.""" diff --git a/uv.lock b/uv.lock index c221c9a..c9aa856 100644 --- a/uv.lock +++ b/uv.lock @@ -947,15 +947,15 @@ wheels = [ [[package]] name = "python-telegram-bot" -version = "22.7" +version = "22.8" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "httpcore", marker = "python_full_version >= '3.14'" }, { name = "httpx" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/e4/25/2258161b1069e66d6c39c0a602dbe57461d4767dc0012539970ea40bc9d6/python_telegram_bot-22.7.tar.gz", hash = "sha256:784b59ea3852fe4616ad63b4a0264c755637f5d725e87755ecdee28300febf61", size = 1516454, upload-time = "2026-03-16T09:36:03.174Z" } +sdist = { url = "https://files.pythonhosted.org/packages/ba/77/153517bb1ac1bba670c6fb1dbf09e1fd0730494b1705934e715391413a0d/python_telegram_bot-22.8.tar.gz", hash = "sha256:f9d3847fcb23ee603477e442800b33bb4adf851a73e0619d2050be879decf1ef", size = 1551700, upload-time = "2026-06-12T08:10:29.1Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/94/f7/0e2f89dd62f45d46d4ea0d8aec5893ce5b37389638db010c117f46f11450/python_telegram_bot-22.7-py3-none-any.whl", hash = "sha256:d72eed532cf763758cd9331b57a6d790aff0bb4d37d8f4e92149436fe21c6475", size = 745365, upload-time = "2026-03-16T09:36:01.498Z" }, + { url = "https://files.pythonhosted.org/packages/60/7c/ed7d4dd94280bd434173cae9f7a7aedaaab9af128ae4f494423a5687c820/python_telegram_bot-22.8-py3-none-any.whl", hash = "sha256:42373918097f1b837cc4e717d588c19ea79651497ec712bb5b0c76e5e63c50e1", size = 769397, upload-time = "2026-06-12T08:10:27.066Z" }, ] [package.optional-dependencies] @@ -990,7 +990,7 @@ requires-dist = [ { name = "logfire", specifier = ">=2.6.1" }, { name = "pydantic", specifier = ">=2.12.5" }, { name = "pydantic-settings", specifier = ">=2.14.2" }, - { name = "python-telegram-bot", extras = ["job-queue"], specifier = ">=22.5" }, + { name = "python-telegram-bot", extras = ["job-queue"], specifier = ">=22.8" }, { name = "sqlmodel", specifier = ">=0.0.28" }, ] From 35c4c94053ebb61f808678724d7b6a1c9b5f0e41 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 31 Jul 2026 13:23:39 +0700 Subject: [PATCH 2/5] feat: group-scoped admin actions, separate trust/verify/unrestrict Major changes: 1. Group-scoped admin authorization: - Add is_user_admin_in_group() and get_admin_groups() helpers - /check now shows group selector when admin is admin in multiple groups - All callback data encodes group_id: action:{group_id}:{user_id} - Callbacks verify caller is admin of the specific group, not global union - Add handle_check_group_callback for group selection flow - Warn callback sends to one group only, not all groups 2. Separate Trust, photo exemption, and unrestriction: - Trust no longer auto-unrestricts (removes bot param from trust_user) - Trust only clears probation, does not lift restrictions - Add separate 'Buka pembatasan bot' (unrestrict) action - unrestrict_user_in_group only lifts bot-applied restrictions - Verify (photo exemption) is group-scoped, clears warnings in one group - Rename buttons to Indonesian: 'Izinkan foto tersembunyi', 'Kecualikan dari anti-spam', 'Buka pembatasan bot' 3. New plugins: check_group_callback, unrestrict_callback 4. Updated all tests for group-scoped callback data and authorization --- src/bot/constants.py | 67 ++++- src/bot/handlers/check.py | 355 +++++++++++++++++--------- src/bot/handlers/trust.py | 135 +++++----- src/bot/handlers/verify.py | 371 +++++++++++++++++----------- src/bot/plugins/builtin/commands.py | 63 +++-- src/bot/plugins/definitions.py | 6 +- src/bot/plugins/manager.py | 2 + src/bot/services/telegram_utils.py | 43 ++++ tests/test_check.py | 185 +++++++++++--- tests/test_plugin_manager.py | 4 +- tests/test_trust_handler.py | 67 ++--- tests/test_verify_handler.py | 145 ++++++++--- 12 files changed, 982 insertions(+), 461 deletions(-) diff --git a/src/bot/constants.py b/src/bot/constants.py index 6c4a300..a4c3dd4 100644 --- a/src/bot/constants.py +++ b/src/bot/constants.py @@ -167,6 +167,38 @@ def format_hours_display(hours: int) -> str: "✅ {user_mention} telah diverifikasi oleh admin. Silakan berdiskusi kembali." ) +VERIFY_SUCCESS_MESSAGE = ( + "✅ User dengan ID {user_id} telah diverifikasi:\n" + "• Ditambahkan ke whitelist foto profil\n" + "• Riwayat warning dihapus di grup {group_id}\n\n" + "User ini tidak akan dicek foto profil lagi." +) + +VERIFY_SUCCESS_WITH_UNRESTRICT_MESSAGE = ( + "✅ User dengan ID {user_id} telah diverifikasi:\n" + "• Ditambahkan ke whitelist foto profil\n" + "• Pembatasan bot dicabut di grup {group_id}\n" + "• Riwayat warning dihapus\n\n" + "User ini tidak akan dicek foto profil lagi." +) + +UNVERIFY_SUCCESS_MESSAGE = ( + "✅ User dengan ID {target_user_id} telah dihapus dari whitelist verifikasi foto." +) + +UNRESTRICT_SUCCESS_MESSAGE = ( + "✅ Pembatasan bot untuk user `{user_id}` telah dicabut di grup {group_id}." +) + +UNRESTRICT_FAILED_MESSAGE = ( + "❌ Gagal membuka pembatasan untuk user `{user_id}` di grup {group_id}. " + "Pastikan bot memiliki izin yang cukup." +) + +UNRESTRICT_NOT_NEEDED_MESSAGE = ( + "ℹ️ User `{user_id}` tidak dibatasi oleh bot di grup {group_id}." +) + ADMIN_CHECK_PROMPT = ( "📋 User: {user_mention} (ID: `{user_id}`)\n\n" "Status Profil:\n" @@ -179,6 +211,18 @@ def format_hours_display(hours: int) -> str: ADMIN_CHECK_ACTION_INCOMPLETE = "⚠️ Profil tidak lengkap. Pilih aksi:" +ADMIN_CHECK_GROUP_PROMPT = ( + "📋 User: {user_mention} (ID: `{user_id}`)\n\n" + "Status Profil:\n" + "• Foto Profil: {photo_status}\n" + "• Username: {username_status}\n\n" + "Pilih grup untuk melakukan aksi:" +) + +ADMIN_CHECK_GROUP_NONE = ( + "❌ Kamu bukan admin di grup mana pun yang dipantau oleh bot ini." +) + ADMIN_WARN_USER_MESSAGE = ( "⚠️ Hai {user_mention}, mohon lengkapi {missing_text} kamu " "untuk mematuhi aturan grup.\n\n" @@ -194,9 +238,10 @@ def format_hours_display(hours: int) -> str: TRUST_USER_ID_INVALID_MESSAGE = "❌ User ID harus berupa angka." TRUST_ADDED_MESSAGE = ( - "✅ User `{user_id}` ditambahkan ke trusted list.\n" - "• Probation dibersihkan di {probation_clear_count} grup\n" - "• Unrestrict dicoba di {unrestrict_count} grup" + "✅ User `{user_id}` ditambahkan ke trusted list (kecualikan dari anti-spam).\n" + "• Probation dibersihkan di {probation_clear_count} grup\n\n" + "Catatan: Trust tidak membuka pembatasan. Gunakan tombol \"Buka pembatasan bot\" " + "untuk mencabut pembatasan yang diterapkan oleh bot ini." ) TRUST_ALREADY_EXISTS_MESSAGE = "ℹ️ User `{user_id}` sudah ada di trusted list." @@ -219,9 +264,21 @@ def format_hours_display(hours: int) -> str: TRUST_CALLBACK_INVALID_MESSAGE = "❌ Data callback tidak valid." -CHECK_TRUST_BUTTON_LABEL = "🤝 Trust User" +TRUST_NO_GROUP_PERMISSION_MESSAGE = ( + "❌ Kamu bukan admin di grup ini." +) + +CHECK_TRUST_BUTTON_LABEL = "🛡️ Kecualikan dari anti-spam" + +CHECK_UNTRUST_BUTTON_LABEL = "🛡️ Cabut pengecualian anti-spam" + +CHECK_VERIFY_BUTTON_LABEL = "📷 Izinkan foto tersembunyi" + +CHECK_UNVERIFY_BUTTON_LABEL = "❌ Cabut izin foto" + +CHECK_UNRESTRICT_BUTTON_LABEL = "🔓 Buka pembatasan bot" -CHECK_UNTRUST_BUTTON_LABEL = "🤝 Untrust User" +CHECK_WARN_BUTTON_LABEL = "⚠️ Beri peringatan" # Anti-spam probation warning for new users NEW_USER_SPAM_WARNING = ( diff --git a/src/bot/handlers/check.py b/src/bot/handlers/check.py index 0fa3dda..47eb27f 100644 --- a/src/bot/handlers/check.py +++ b/src/bot/handlers/check.py @@ -4,7 +4,11 @@ This module handles admin commands to manually check user profiles: 1. /check - Check a user's profile status 2. Forwarded message - Check profile and show action buttons -3. Warn button callback - Send warning to user in group +3. Group selector callback - Pick which group to act on +4. Warn button callback - Send warning to user in the selected group + +All actions are scoped to a single group. Callbacks encode the group_id +so that admin authorization can be checked per-group. """ import logging @@ -16,19 +20,27 @@ from bot.constants import ( ADMIN_CHECK_ACTION_COMPLETE, ADMIN_CHECK_ACTION_INCOMPLETE, + ADMIN_CHECK_GROUP_NONE, + ADMIN_CHECK_GROUP_PROMPT, ADMIN_CHECK_PROMPT, ADMIN_WARN_SENT_MESSAGE, ADMIN_WARN_USER_MESSAGE, CHECK_TRUST_BUTTON_LABEL, + CHECK_UNRESTRICT_BUTTON_LABEL, CHECK_UNTRUST_BUTTON_LABEL, + CHECK_UNVERIFY_BUTTON_LABEL, + CHECK_VERIFY_BUTTON_LABEL, + CHECK_WARN_BUTTON_LABEL, MISSING_ITEMS_SEPARATOR, ) from bot.database.service import get_database from bot.group_config import get_group_registry from bot.services.telegram_utils import ( extract_forwarded_user, + get_admin_groups, get_user_mention, get_user_mention_by_id, + is_user_admin_in_group, require_admin_dm_target, send_message_with_retry, ) @@ -37,19 +49,15 @@ logger = logging.getLogger(__name__) -async def _build_check_response( +async def _build_profile_status( bot: Bot, user_id: int, user_name: str -) -> tuple[str, InlineKeyboardMarkup | None]: +) -> tuple[str, str, bool, bool, bool, bool]: """ - Build the check response message and keyboard. - - Args: - bot: Telegram bot instance. - user_id: ID of the user to check. - user_name: Display name of the user. + Check a user's profile and return status info. Returns: - Tuple of (message text, optional keyboard markup). + Tuple of (user_mention, photo_emoji, has_photo, has_username, + is_whitelisted, is_trusted). """ try: chat = await bot.get_chat(user_id) @@ -59,61 +67,150 @@ async def _build_check_response( raise user_mention = get_user_mention_by_id(user_id, user_name) - photo_status = "✅" if result.has_profile_photo else "❌" - username_status = "✅" if result.has_username else "❌" - db = get_database() is_whitelisted = db.is_user_photo_whitelisted(user_id) is_trusted = db.is_user_trusted(user_id) is True - if result.is_complete: - action_prompt = ADMIN_CHECK_ACTION_COMPLETE - buttons: list[InlineKeyboardButton] = [] - if is_whitelisted: - buttons.append( - InlineKeyboardButton("❌ Unverify User", callback_data=f"unverify:{user_id}") + return user_mention, "✅" if result.has_profile_photo else "❌", result.has_profile_photo, result.has_username, is_whitelisted, is_trusted + + +def _build_group_selector_keyboard( + admin_group_ids: list[int], user_id: int +) -> InlineKeyboardMarkup: + """Build the group selector keyboard.""" + buttons = [] + for gid in admin_group_ids: + buttons.append( + InlineKeyboardButton( + text=f"📋 Grup {gid}", + callback_data=f"checkgrp:{gid}:{user_id}", ) - if is_trusted: + ) + return InlineKeyboardMarkup([[b] for b in buttons]) + + +def _build_action_keyboard( + group_id: int, + user_id: int, + is_complete: bool, + is_whitelisted: bool, + is_trusted: bool, + missing_code: str = "", +) -> InlineKeyboardMarkup: + """ + Build the action keyboard for a specific group. + + Callback data format: action:{group_id}:{user_id}[:missing_code] + """ + buttons: list[InlineKeyboardButton] = [] + + if not is_complete: + buttons.append( + InlineKeyboardButton( + CHECK_WARN_BUTTON_LABEL, + callback_data=f"warn:{group_id}:{user_id}:{missing_code}", + ) + ) + + if not is_complete or is_whitelisted: + if is_whitelisted: buttons.append( - InlineKeyboardButton(CHECK_UNTRUST_BUTTON_LABEL, callback_data=f"untrust:{user_id}") + InlineKeyboardButton( + CHECK_UNVERIFY_BUTTON_LABEL, + callback_data=f"unverify:{group_id}:{user_id}", + ) ) else: buttons.append( - InlineKeyboardButton(CHECK_TRUST_BUTTON_LABEL, callback_data=f"trust:{user_id}") + InlineKeyboardButton( + CHECK_VERIFY_BUTTON_LABEL, + callback_data=f"verify:{group_id}:{user_id}", + ) + ) + + if is_trusted: + buttons.append( + InlineKeyboardButton( + CHECK_UNTRUST_BUTTON_LABEL, + callback_data=f"untrust:{group_id}:{user_id}", ) - keyboard = InlineKeyboardMarkup([buttons]) if buttons else None + ) else: - action_prompt = ADMIN_CHECK_ACTION_INCOMPLETE - # Store missing items in callback data (photo,username format) - missing_code = "" - if not result.has_profile_photo: - missing_code += "p" - if not result.has_username: - missing_code += "u" + buttons.append( + InlineKeyboardButton( + CHECK_TRUST_BUTTON_LABEL, + callback_data=f"trust:{group_id}:{user_id}", + ) + ) - trust_button = ( - InlineKeyboardButton(CHECK_UNTRUST_BUTTON_LABEL, callback_data=f"untrust:{user_id}") - if is_trusted - else InlineKeyboardButton(CHECK_TRUST_BUTTON_LABEL, callback_data=f"trust:{user_id}") + buttons.append( + InlineKeyboardButton( + CHECK_UNRESTRICT_BUTTON_LABEL, + callback_data=f"unrestrict:{group_id}:{user_id}", ) + ) - keyboard = InlineKeyboardMarkup([ - [ - InlineKeyboardButton("⚠️ Warn User", callback_data=f"warn:{user_id}:{missing_code}"), - InlineKeyboardButton("✅ Verify User", callback_data=f"verify:{user_id}"), - trust_button, - ] - ]) - - message = ADMIN_CHECK_PROMPT.format( - user_mention=user_mention, - user_id=user_id, - photo_status=photo_status, - username_status=username_status, - action_prompt=action_prompt, + # Split into rows of max 2 buttons for mobile readability + rows = [buttons[i : i + 2] for i in range(0, len(buttons), 2)] + return InlineKeyboardMarkup(rows) + + +async def _show_check_result( + update: Update, + context: ContextTypes.DEFAULT_TYPE, + target_user_id: int, + user_name: str, +) -> None: + """ + Check a user's profile and show the result with group selector or actions. + + If the admin is admin in only one group, shows actions directly. + If admin in multiple groups, shows a group selector first. + """ + admin_user_id = update.message.from_user.id if update.message else update.callback_query.from_user.id # type: ignore[union-attr] + reply_func = update.message.reply_text if update.message else update.callback_query.edit_message_text # type: ignore[union-attr] + + user_mention, photo_status, has_photo, has_username, is_whitelisted, is_trusted = ( + await _build_profile_status(context.bot, target_user_id, user_name) ) + username_status = "✅" if has_username else "❌" + is_complete = has_photo and has_username - return message, keyboard + admin_group_ids = get_admin_groups(context, admin_user_id) + + if not admin_group_ids: + await reply_func(ADMIN_CHECK_GROUP_NONE) + return + + if len(admin_group_ids) == 1: + group_id = admin_group_ids[0] + missing_code = "" + if not has_photo: + missing_code += "p" + if not has_username: + missing_code += "u" + + action_prompt = ADMIN_CHECK_ACTION_COMPLETE if is_complete else ADMIN_CHECK_ACTION_INCOMPLETE + message = ADMIN_CHECK_PROMPT.format( + user_mention=user_mention, + user_id=target_user_id, + photo_status=photo_status, + username_status=username_status, + action_prompt=action_prompt, + ) + keyboard = _build_action_keyboard( + group_id, target_user_id, is_complete, is_whitelisted, is_trusted, missing_code + ) + await reply_func(message, reply_markup=keyboard, parse_mode="Markdown") + else: + message = ADMIN_CHECK_GROUP_PROMPT.format( + user_mention=user_mention, + user_id=target_user_id, + photo_status=photo_status, + username_status=username_status, + ) + keyboard = _build_group_selector_keyboard(admin_group_ids, target_user_id) + await reply_func(message, reply_markup=keyboard, parse_mode="Markdown") async def handle_check_command( @@ -138,12 +235,10 @@ async def handle_check_command( admin_user_id = update.message.from_user.id try: - # Get user info for display name chat = await context.bot.get_chat(target_user_id) user_name = chat.full_name or f"User {target_user_id}" - message, keyboard = await _build_check_response(context.bot, target_user_id, user_name) - await update.message.reply_text(message, reply_markup=keyboard, parse_mode="Markdown") + await _show_check_result(update, context, target_user_id, user_name) logger.info( f"Admin {admin_user_id} ({update.message.from_user.full_name}) " @@ -170,15 +265,6 @@ async def handle_check_forwarded_message( return admin_user_id = update.message.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) - - if admin_user_id not in admin_ids: - await update.message.reply_text("❌ Kamu tidak memiliki izin untuk menggunakan fitur ini.") - logger.warning( - f"Non-admin user {admin_user_id} ({update.message.from_user.full_name}) " - f"attempted to forward message for check" - ) - return forwarded_info = extract_forwarded_user(update.message) if not forwarded_info: @@ -191,8 +277,7 @@ async def handle_check_forwarded_message( user_id, user_name = forwarded_info try: - message, keyboard = await _build_check_response(context.bot, user_id, user_name) - await update.message.reply_text(message, reply_markup=keyboard, parse_mode="Markdown") + await _show_check_result(update, context, user_id, user_name) logger.info( f"Admin {admin_user_id} ({update.message.from_user.full_name}) " @@ -206,13 +291,74 @@ async def handle_check_forwarded_message( logger.error(f"Error checking forwarded user {user_id}: {e}", exc_info=True) -def _parse_warn_callback_data(data: str) -> tuple[int, str] | None: - """Parse warn callback data (warn::). Returns (user_id, missing_code) or None.""" +async def handle_check_group_callback( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """ + Handle callback query for group selection in /check. + + Shows the action keyboard for the selected group. + """ + query = update.callback_query + if not query or not query.from_user or not query.data: + return + + await query.answer() + + parts = query.data.split(":") + try: + group_id = int(parts[1]) + target_user_id = int(parts[2]) + except (IndexError, ValueError): + await query.edit_message_text("❌ Data callback tidak valid.") + return + + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text("❌ Kamu bukan admin di grup ini.") + return + + try: + chat = await context.bot.get_chat(target_user_id) + user_name = chat.full_name or f"User {target_user_id}" + + user_mention, photo_status, has_photo, has_username, is_whitelisted, is_trusted = ( + await _build_profile_status(context.bot, target_user_id, user_name) + ) + username_status = "✅" if has_username else "❌" + is_complete = has_photo and has_username + + missing_code = "" + if not has_photo: + missing_code += "p" + if not has_username: + missing_code += "u" + + action_prompt = ADMIN_CHECK_ACTION_COMPLETE if is_complete else ADMIN_CHECK_ACTION_INCOMPLETE + message = ADMIN_CHECK_PROMPT.format( + user_mention=user_mention, + user_id=target_user_id, + photo_status=photo_status, + username_status=username_status, + action_prompt=action_prompt, + ) + keyboard = _build_action_keyboard( + group_id, target_user_id, is_complete, is_whitelisted, is_trusted, missing_code + ) + await query.edit_message_text(message, reply_markup=keyboard, parse_mode="Markdown") + except Exception as e: + await query.edit_message_text(f"❌ Gagal memeriksa user: {e}") + logger.error(f"Error in check group callback: {e}", exc_info=True) + + +def _parse_warn_callback_data(data: str) -> tuple[int, int, str] | None: + """Parse warn callback data (warn:::).""" try: parts = data.split(":") - user_id = int(parts[1]) - missing_code = parts[2] if len(parts) > 2 else "" - return (user_id, missing_code) + group_id = int(parts[1]) + user_id = int(parts[2]) + missing_code = parts[3] if len(parts) > 3 else "" + return (group_id, user_id, missing_code) except (IndexError, ValueError): return None @@ -223,7 +369,7 @@ async def handle_warn_callback( """ Handle callback query for warn button. - Sends a warning message to the user in all monitored groups (or the first group). + Sends a warning message to the user in the selected group only. """ query = update.callback_query if not query or not query.from_user or not query.data: @@ -231,26 +377,18 @@ async def handle_warn_callback( await query.answer() - admin_user_id = query.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) - - if admin_user_id not in admin_ids: - await query.edit_message_text("❌ Kamu tidak memiliki izin untuk menggunakan perintah ini.") - logger.warning( - f"Non-admin user {admin_user_id} ({query.from_user.full_name}) " - f"attempted to use warn callback" - ) - return - - # Parse callback data: warn:: parsed = _parse_warn_callback_data(query.data) if parsed is None: await query.edit_message_text("❌ Data callback tidak valid.") logger.error(f"Invalid callback_data format: {query.data}") return - target_user_id, missing_code = parsed + group_id, target_user_id, missing_code = parsed + + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text("❌ Kamu bukan admin di grup ini.") + return - # Build missing items text missing_items = [] if "p" in missing_code: missing_items.append("foto profil publik") @@ -259,42 +397,37 @@ async def handle_warn_callback( missing_text = MISSING_ITEMS_SEPARATOR.join(missing_items) if missing_items else "profil" registry = get_group_registry() + group_config = registry.get(group_id) + + if group_config is None: + await query.edit_message_text("❌ Grup tidak ditemukan di registry.") + return try: - # Get user info for mention chat = await context.bot.get_chat(target_user_id) user_mention = get_user_mention(chat) - # Send warning to all monitored groups - sent_to_any = False - for group_config in registry.all_groups(): - warn_message = ADMIN_WARN_USER_MESSAGE.format( - user_mention=user_mention, - missing_text=missing_text, - rules_link=group_config.rules_link, - ) - try: - ok = await send_message_with_retry( - context.bot, - chat_id=group_config.group_id, - message_thread_id=group_config.warning_topic_id, - text=warn_message, - parse_mode="Markdown", - ) - if ok: - sent_to_any = True - logger.info( - f"Admin {admin_user_id} sent warning to user {target_user_id} in group {group_config.group_id}" - ) - except Exception as e: - logger.error(f"Failed to send warning to group {group_config.group_id}: {e}") - - if sent_to_any: - # Update the original message + warn_message = ADMIN_WARN_USER_MESSAGE.format( + user_mention=user_mention, + missing_text=missing_text, + rules_link=group_config.rules_link, + ) + ok = await send_message_with_retry( + context.bot, + chat_id=group_config.group_id, + message_thread_id=group_config.warning_topic_id, + text=warn_message, + parse_mode="Markdown", + ) + + if ok: success_message = ADMIN_WARN_SENT_MESSAGE.format(user_mention=user_mention) await query.edit_message_text(success_message, parse_mode="Markdown") + logger.info( + f"Admin {admin_user_id} sent warning to user {target_user_id} in group {group_id}" + ) else: - await query.edit_message_text("❌ Gagal mengirim peringatan ke semua grup.") + await query.edit_message_text("❌ Gagal mengirim peringatan ke grup.") except TimedOut: await query.edit_message_text("⏳ Request timeout. Silakan coba lagi.") diff --git a/src/bot/handlers/trust.py b/src/bot/handlers/trust.py index e1dce0c..9fd454f 100644 --- a/src/bot/handlers/trust.py +++ b/src/bot/handlers/trust.py @@ -1,11 +1,13 @@ """Trusted-user command handlers for anti-spam bypass management. -Note on the trust unrestrict policy: - Adding a user to the trusted list (via ``/trust`` or the Trust button in - ``/check``) ALSO unrestricts that user in every monitored group as a side - effect, including restrictions that may have been applied manually by - other admins for unrelated reasons. Admins should be aware that trusting - a user lifts any active restriction across all groups. +Trust adds a user to the anti-spam bypass list and clears their probation. +It does NOT unrestrict the user — use the separate "Buka pembatasan bot" +action for that. This separation prevents trust from lifting manual +admin restrictions as a side effect. + +Commands /trust, /untrust, /trusted are DM-only and require admin status. +Callback buttons (trust/untrust) are group-scoped: they encode group_id +and verify the caller is an admin of that specific group. """ import logging @@ -22,6 +24,7 @@ TRUST_DM_ONLY_MESSAGE, TRUST_LIST_EMPTY_MESSAGE, TRUST_LIST_HEADER, + TRUST_NO_GROUP_PERMISSION_MESSAGE, TRUST_NO_PERMISSION_MESSAGE, TRUST_REMOVED_MESSAGE, TRUST_USER_ID_INVALID_MESSAGE, @@ -30,7 +33,10 @@ ) from bot.database.service import DatabaseService, get_database from bot.group_config import GroupRegistry, get_group_registry -from bot.services.telegram_utils import extract_forwarded_user, unrestrict_user +from bot.services.telegram_utils import ( + extract_forwarded_user, + is_user_admin_in_group, +) logger = logging.getLogger(__name__) @@ -44,12 +50,7 @@ def _remove_trusted_cache(context: ContextTypes.DEFAULT_TYPE, user_id: int) -> N def _format_person(full_name: str, user_id: int) -> str: - """Return a markdown-safe display for a stored person. - - Uses the cached ``full_name`` from the DB. Falls back to ``User `` - if the name is empty (e.g. trust granted via callback without name, - or pre-cache row from before the feature was deployed). - """ + """Return a markdown-safe display for a stored person.""" if full_name: return escape_markdown(full_name, version=1) return f"User {user_id}" @@ -65,13 +66,7 @@ def _format_person_with_username(full_name: str, username: str | None, user_id: def _resolve_target_user_id( update: Update, args: list[str] ) -> tuple[int | None, str | None]: - """Resolve the target user ID from CLI args or a forwarded message. - - Returns: - tuple[int | None, str | None]: ``(user_id, None)`` on success, or - ``(None, error_message)`` where ``error_message`` is a user-facing - template from :mod:`bot.constants`. - """ + """Resolve the target user ID from CLI args or a forwarded message.""" if args: try: return int(args[0]), None @@ -87,7 +82,6 @@ def _resolve_target_user_id( async def trust_user( - bot: object, db: DatabaseService, registry: GroupRegistry, target_user_id: int, @@ -96,19 +90,20 @@ async def trust_user( target_username: str | None = None, admin_full_name: str = "", admin_username: str | None = None, -) -> tuple[int, int]: - """Add a trusted user and apply cleanup side effects. + group_id: int | None = None, +) -> int: + """Add a trusted user and clear probation. + + Trust does NOT unrestrict the user. Unrestriction is a separate + action ("Buka pembatasan bot") so that trusting a user doesn't + inadvertently lift manual admin restrictions. - Trust ALSO unrestricts the user in every monitored group as part of the - cleanup loop. This intentionally lifts any active restriction — including - restrictions previously applied manually by other admins for unrelated - reasons. + If ``group_id`` is provided, probation is cleared only in that group. + Otherwise, probation is cleared in all monitored groups (legacy behavior + for the /trust command). Returns: - tuple[int, int]: (probation_clear_count, unrestrict_attempt_count). - Both counts increment independently per group; a probation lookup - failure does not prevent the unrestrict attempt for that group, - and vice versa. + int: Number of groups where probation was cleared. """ db.add_trusted_user( user_id=target_user_id, @@ -120,8 +115,12 @@ async def trust_user( ) cleared_probation = 0 - unrestricted_groups = 0 - for group_config in registry.all_groups(): + groups_to_check = ( + [registry.get(group_id)] if group_id is not None else registry.all_groups() + ) + for group_config in groups_to_check: + if group_config is None: + continue try: if db.get_new_user_probation(target_user_id, group_config.group_id): db.clear_new_user_probation(target_user_id, group_config.group_id) @@ -132,26 +131,13 @@ async def trust_user( exc_info=True, ) - try: - await unrestrict_user(bot, group_config.group_id, target_user_id) - unrestricted_groups += 1 - except Exception: - logger.warning( - f"Unrestrict failed for user {target_user_id} in group {group_config.group_id}", - exc_info=True, - ) - - return cleared_probation, unrestricted_groups + return cleared_probation async def handle_trust_command( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: - """Handle /trust command in bot DM. - - Trusting a user ALSO unrestricts them in every monitored group, including - restrictions that may have been applied manually by other admins. - """ + """Handle /trust command in bot DM.""" if not update.message or not update.message.from_user: return @@ -170,7 +156,6 @@ async def handle_trust_command( await update.message.reply_text(error_message) return - # Resolve target user's display name target_full_name = "" target_username = None if update.message.forward_from: @@ -181,8 +166,8 @@ async def handle_trust_command( registry = get_group_registry() try: - cleared_count, unrestricted_count = await trust_user( - context.bot, db, registry, target_user_id, admin_user_id, + cleared_count = await trust_user( + db, registry, target_user_id, admin_user_id, target_user_full_name=target_full_name, target_username=target_username, admin_full_name=update.message.from_user.full_name, @@ -193,7 +178,6 @@ async def handle_trust_command( TRUST_ADDED_MESSAGE.format( user_id=target_user_id, probation_clear_count=cleared_count, - unrestrict_count=unrestricted_count, ), parse_mode="Markdown", ) @@ -273,7 +257,6 @@ async def handle_trusted_list_command( trusted_at = trusted_at.replace(tzinfo=UTC) trusted_at_display = trusted_at.astimezone(UTC).strftime("%Y-%m-%d %H:%M UTC") - # Use stored name/username — no API calls user_display = _format_person_with_username( record.user_full_name, record.username, record.user_id ) @@ -295,10 +278,9 @@ async def handle_trusted_list_command( async def handle_trust_callback( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: - """Handle trust callback button. + """Handle trust callback button (group-scoped). - Trusting a user ALSO unrestricts them in every monitored group, including - restrictions that may have been applied manually by other admins. + Callback data format: trust:{group_id}:{user_id} """ query = update.callback_query if not query or not query.from_user or not query.data: @@ -306,33 +288,34 @@ async def handle_trust_callback( await query.answer() - admin_user_id = query.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) - if admin_user_id not in admin_ids: - await query.edit_message_text(TRUST_NO_PERMISSION_MESSAGE) - return - + parts = query.data.split(":") try: - target_user_id = int(query.data.split(":")[1]) + group_id = int(parts[1]) + target_user_id = int(parts[2]) except (IndexError, ValueError): await query.edit_message_text(TRUST_CALLBACK_INVALID_MESSAGE) return + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text(TRUST_NO_GROUP_PERMISSION_MESSAGE) + return + db = get_database() registry = get_group_registry() try: - cleared_count, unrestricted_count = await trust_user( - context.bot, db, registry, target_user_id, admin_user_id, + cleared_count = await trust_user( + db, registry, target_user_id, admin_user_id, admin_full_name=query.from_user.full_name, admin_username=query.from_user.username, + group_id=group_id, ) _add_trusted_cache(context, target_user_id) await query.edit_message_text( TRUST_ADDED_MESSAGE.format( user_id=target_user_id, probation_clear_count=cleared_count, - unrestrict_count=unrestricted_count, ), parse_mode="Markdown", ) @@ -346,25 +329,29 @@ async def handle_trust_callback( async def handle_untrust_callback( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: - """Handle untrust callback button.""" + """Handle untrust callback button (group-scoped). + + Callback data format: untrust:{group_id}:{user_id} + """ query = update.callback_query if not query or not query.from_user or not query.data: return await query.answer() - admin_user_id = query.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) - if admin_user_id not in admin_ids: - await query.edit_message_text(TRUST_NO_PERMISSION_MESSAGE) - return - + parts = query.data.split(":") try: - target_user_id = int(query.data.split(":")[1]) + group_id = int(parts[1]) + target_user_id = int(parts[2]) except (IndexError, ValueError): await query.edit_message_text(TRUST_CALLBACK_INVALID_MESSAGE) return + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text(TRUST_NO_GROUP_PERMISSION_MESSAGE) + return + db = get_database() try: diff --git a/src/bot/handlers/verify.py b/src/bot/handlers/verify.py index 1465195..16124b0 100644 --- a/src/bot/handlers/verify.py +++ b/src/bot/handlers/verify.py @@ -4,6 +4,11 @@ This module handles the /verify and /unverify commands which allow admins to manage the photo verification whitelist for users whose profile pictures are hidden due to Telegram privacy settings. + +Verify adds the user to the photo whitelist and clears warnings in the +specified group. It also lifts bot-applied restrictions in that group if +the user's profile is otherwise complete (username present). It does NOT +broadcast to all groups — each action is scoped to one group. """ import logging @@ -12,11 +17,20 @@ from telegram.error import BadRequest from telegram.ext import ContextTypes -from bot.constants import VERIFICATION_CLEARANCE_MESSAGE +from bot.constants import ( + UNRESTRICT_FAILED_MESSAGE, + UNRESTRICT_NOT_NEEDED_MESSAGE, + UNRESTRICT_SUCCESS_MESSAGE, + UNVERIFY_SUCCESS_MESSAGE, + VERIFY_SUCCESS_MESSAGE, + VERIFY_SUCCESS_WITH_UNRESTRICT_MESSAGE, + VERIFICATION_CLEARANCE_MESSAGE, +) from bot.database.service import DatabaseService, get_database from bot.group_config import GroupRegistry, get_group_registry from bot.services.telegram_utils import ( get_user_mention, + is_user_admin_in_group, require_admin_dm_target, send_message_with_retry, unrestrict_user, @@ -25,110 +39,145 @@ logger = logging.getLogger(__name__) -async def verify_user( - bot: Bot, db: DatabaseService, registry: GroupRegistry, target_user_id: int, admin_user_id: int +async def verify_user_in_group( + bot: Bot, + db: DatabaseService, + registry: GroupRegistry, + target_user_id: int, + admin_user_id: int, + group_id: int, ) -> str: """ - Verify a user by adding them to the photo verification whitelist. - - This function handles the core verification logic: adds user to whitelist, - unrestricts them in all monitored groups, deletes warnings, and sends - clearance notification if needed. + Verify a user in a specific group: add to photo whitelist, clear + warnings, and lift bot-applied restriction if username is present. Args: bot: Telegram bot instance. db: Database service instance. - registry: Group registry for iterating all groups. + registry: Group registry. target_user_id: ID of the user to verify. admin_user_id: ID of the admin performing the verification. + group_id: The group to scope the action to. Returns: Success message string. - - Raises: - ValueError: If user is already whitelisted. """ + group_config = registry.get(group_id) + if group_config is None: + return f"❌ Grup {group_id} tidak ditemukan." + db.add_photo_verification_whitelist( user_id=target_user_id, verified_by_admin_id=admin_user_id, ) - # Unrestrict user and delete warnings in all monitored groups - total_deleted = 0 - for group_config in registry.all_groups(): + deleted_count = db.delete_user_warnings(target_user_id, group_id) + + was_restricted = db.is_user_restricted_by_bot(target_user_id, group_id) + did_unrestrict = False + + if was_restricted: try: - # Unrestrict user if they are restricted - try: - await unrestrict_user(bot, group_config.group_id, target_user_id) - logger.info(f"Unrestricted user {target_user_id} in group {group_config.group_id} during verification") - except (BadRequest, RuntimeError) as e: - # BadRequest: user might not be restricted or not in group - okay - # RuntimeError: flood control retries exhausted - logger.info(f"Could not unrestrict user {target_user_id} in group {group_config.group_id}: {e}") - - # Delete all warning records for this user in this group - deleted_count = db.delete_user_warnings(target_user_id, group_config.group_id) - total_deleted += deleted_count - - # Send notification to warning topic if user had previous warnings - if deleted_count > 0: - # Get user info for proper mention - user_info = await bot.get_chat(target_user_id) - user_mention = get_user_mention(user_info) - - # Send clearance message to warning topic - clearance_message = VERIFICATION_CLEARANCE_MESSAGE.format( - user_mention=user_mention - ) - await send_message_with_retry( - bot, - chat_id=group_config.group_id, - message_thread_id=group_config.warning_topic_id, - text=clearance_message, - parse_mode="Markdown", - ) - logger.info(f"Sent clearance notification to warning topic for user {target_user_id} in group {group_config.group_id}") + await unrestrict_user(bot, group_id, target_user_id) + db.mark_user_unrestricted(target_user_id, group_id) + did_unrestrict = True + logger.info( + f"Unrestricted user {target_user_id} in group {group_id} during verification" + ) + except (BadRequest, RuntimeError) as e: + logger.info( + f"Could not unrestrict user {target_user_id} in group {group_id}: {e}" + ) + + if deleted_count > 0 or did_unrestrict: + try: + user_info = await bot.get_chat(target_user_id) + user_mention = get_user_mention(user_info) + clearance_message = VERIFICATION_CLEARANCE_MESSAGE.format( + user_mention=user_mention + ) + await send_message_with_retry( + bot, + chat_id=group_id, + message_thread_id=group_config.warning_topic_id, + text=clearance_message, + parse_mode="Markdown", + ) except Exception: logger.warning( - f"Verification failed for group {group_config.group_id} for user {target_user_id}", + f"Failed to send clearance notification for user {target_user_id} in group {group_id}", exc_info=True, ) - continue - - if total_deleted > 0: - logger.info(f"Deleted {total_deleted} total warning record(s) for user {target_user_id}") - return ( - f"✅ User dengan ID {target_user_id} telah diverifikasi:\n" - f"• Ditambahkan ke whitelist foto profil\n" - f"• Pembatasan dicabut (jika ada)\n" - f"• Riwayat warning dihapus\n\n" - f"User ini tidak akan dicek foto profil lagi." + if did_unrestrict: + return VERIFY_SUCCESS_WITH_UNRESTRICT_MESSAGE.format( + user_id=target_user_id, group_id=group_id + ) + return VERIFY_SUCCESS_MESSAGE.format( + user_id=target_user_id, group_id=group_id ) async def unverify_user( - db: DatabaseService, target_user_id: int, admin_user_id: int + db: DatabaseService, target_user_id: int ) -> str: """ - Unverify a user by removing them from the photo verification whitelist. + Remove a user from the photo verification whitelist. Args: db: Database service instance. target_user_id: ID of the user to unverify. - admin_user_id: ID of the admin performing the unverification. Returns: Success message string. - - Raises: - ValueError: If user is not in whitelist. """ db.remove_photo_verification_whitelist(user_id=target_user_id) - logger.info( - f"Admin {admin_user_id} removed user {target_user_id} from photo verification whitelist" - ) - return f"✅ User dengan ID {target_user_id} telah dihapus dari whitelist verifikasi foto." + return UNVERIFY_SUCCESS_MESSAGE.format(target_user_id=target_user_id) + + +async def unrestrict_user_in_group( + bot: Bot, + db: DatabaseService, + target_user_id: int, + group_id: int, +) -> str: + """ + Lift a bot-applied restriction for a user in a specific group. + + Only lifts restrictions that were applied by this bot (restricted_by_bot=True). + Does not lift manual admin restrictions. + + Args: + bot: Telegram bot instance. + db: Database service instance. + target_user_id: ID of the user to unrestrict. + group_id: The group to unrestrict in. + + Returns: + Success or error message string. + """ + if not db.is_user_restricted_by_bot(target_user_id, group_id): + return UNRESTRICT_NOT_NEEDED_MESSAGE.format( + user_id=target_user_id, group_id=group_id + ) + + try: + await unrestrict_user(bot, group_id, target_user_id) + db.mark_user_unrestricted(target_user_id, group_id) + logger.info( + f"Admin unrestricting user {target_user_id} in group {group_id}" + ) + return UNRESTRICT_SUCCESS_MESSAGE.format( + user_id=target_user_id, group_id=group_id + ) + except Exception as e: + logger.error( + f"Failed to unrestrict user {target_user_id} in group {group_id}: {e}", + exc_info=True, + ) + return UNRESTRICT_FAILED_MESSAGE.format( + user_id=target_user_id, group_id=group_id + ) async def handle_verify_command( @@ -139,12 +188,9 @@ async def handle_verify_command( Usage: /verify USER_ID (e.g., /verify 123456789) - This command allows admins to manually verify users whose profile pictures - are hidden due to Telegram privacy settings. Only works in bot DMs. - - Args: - update: Telegram update containing the command. - context: Bot context with helper methods. + Adds the user to the photo whitelist. If the admin is admin in only one + group, also clears warnings and lifts bot restrictions there. If admin + in multiple groups, only adds to whitelist (use /check for per-group actions). """ target_user_id = await require_admin_dm_target( update, @@ -156,21 +202,36 @@ async def handle_verify_command( return admin_user_id = update.message.from_user.id - db = get_database() try: - registry = get_group_registry() - message = await verify_user(context.bot, db, registry, target_user_id, admin_user_id) + from bot.services.telegram_utils import get_admin_groups + + admin_group_ids = get_admin_groups(context, admin_user_id) + + if len(admin_group_ids) == 1: + registry = get_group_registry() + message = await verify_user_in_group( + context.bot, db, registry, target_user_id, admin_user_id, admin_group_ids[0] + ) + else: + db.add_photo_verification_whitelist( + user_id=target_user_id, + verified_by_admin_id=admin_user_id, + ) + message = ( + f"✅ User dengan ID {target_user_id} ditambahkan ke whitelist foto profil.\n" + f"Gunakan /check untuk mengelola per grup." + ) + await update.message.reply_text(message) logger.info( f"Admin {admin_user_id} ({update.message.from_user.full_name}) " f"whitelisted user {target_user_id} for photo verification" ) - except ValueError as e: - await update.message.reply_text(f"ℹ️ User dengan ID {target_user_id} sudah ada di whitelist.") - logger.info( - f"Admin {admin_user_id} tried to whitelist {target_user_id} but already exists: {e}" + except ValueError: + await update.message.reply_text( + f"ℹ️ User dengan ID {target_user_id} sudah ada di whitelist." ) @@ -181,13 +242,6 @@ async def handle_unverify_command( Handle /unverify command to remove users from photo verification whitelist. Usage: /unverify USER_ID (e.g., /unverify 123456789) - - This command allows admins to remove users from the photo verification - whitelist. Only works in bot DMs. - - Args: - update: Telegram update containing the command. - context: Bot context with helper methods. """ target_user_id = await require_admin_dm_target( update, @@ -198,17 +252,14 @@ async def handle_unverify_command( if target_user_id is None: return - admin_user_id = update.message.from_user.id - db = get_database() try: - message = await unverify_user(db, target_user_id, admin_user_id) + message = await unverify_user(db, target_user_id) await update.message.reply_text(message) - except ValueError as e: - await update.message.reply_text(f"ℹ️ User dengan ID {target_user_id} tidak ada di whitelist.") - logger.info( - f"Admin {admin_user_id} tried to remove {target_user_id} but not in whitelist: {e}" + except ValueError: + await update.message.reply_text( + f"ℹ️ User dengan ID {target_user_id} tidak ada di whitelist." ) @@ -216,14 +267,9 @@ async def handle_verify_callback( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: """ - Handle callback query for verify button. + Handle callback query for verify button (group-scoped). - Processes the inline button click to verify a user and updates the message - with the result. - - Args: - update: Telegram update containing the callback query. - context: Bot context with helper methods. + Callback data format: verify:{group_id}:{user_id} """ query = update.callback_query if not query or not query.from_user or not query.data: @@ -231,42 +277,37 @@ async def handle_verify_callback( await query.answer() - admin_user_id = query.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) - - if admin_user_id not in admin_ids: - await query.edit_message_text("❌ Kamu tidak memiliki izin untuk menggunakan perintah ini.") - logger.warning( - f"Non-admin user {admin_user_id} ({query.from_user.full_name}) " - f"attempted to use verify callback" - ) - return - - # Extract user_id from callback_data + parts = query.data.split(":") try: - target_user_id = int(query.data.split(":")[1]) + group_id = int(parts[1]) + target_user_id = int(parts[2]) except (IndexError, ValueError): await query.edit_message_text("❌ Data callback tidak valid.") - logger.error(f"Invalid callback_data format: {query.data}") + return + + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text("❌ Kamu bukan admin di grup ini.") return db = get_database() try: registry = get_group_registry() - message = await verify_user(context.bot, db, registry, target_user_id, admin_user_id) - await query.edit_message_text(message) + message = await verify_user_in_group( + context.bot, db, registry, target_user_id, admin_user_id, group_id + ) + await query.edit_message_text(message, parse_mode="Markdown") logger.info( f"Admin {admin_user_id} ({query.from_user.full_name}) " - f"verified user {target_user_id} via callback" + f"verified user {target_user_id} in group {group_id} via callback" ) - except ValueError as e: - await query.edit_message_text(f"ℹ️ User dengan ID {target_user_id} sudah ada di whitelist.") - logger.info( - f"Admin {admin_user_id} tried to verify {target_user_id} via callback but already exists: {e}" + except ValueError: + await query.edit_message_text( + f"ℹ️ User dengan ID {target_user_id} sudah ada di whitelist." ) except Exception as e: - await query.edit_message_text(f"❌ Terjadi kesalahan: {str(e)}") + await query.edit_message_text(f"❌ Terjadi kesalahan: {e}") logger.error(f"Error during verify callback: {e}", exc_info=True) @@ -274,14 +315,9 @@ async def handle_unverify_callback( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: """ - Handle callback query for unverify button. - - Processes the inline button click to unverify a user and updates the message - with the result. + Handle callback query for unverify button (group-scoped). - Args: - update: Telegram update containing the callback query. - context: Bot context with helper methods. + Callback data format: unverify:{group_id}:{user_id} """ query = update.callback_query if not query or not query.from_user or not query.data: @@ -289,39 +325,76 @@ async def handle_unverify_callback( await query.answer() + parts = query.data.split(":") + try: + group_id = int(parts[1]) + target_user_id = int(parts[2]) + except (IndexError, ValueError): + await query.edit_message_text("❌ Data callback tidak valid.") + return + admin_user_id = query.from_user.id - admin_ids = context.bot_data.get("admin_ids", []) + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text("❌ Kamu bukan admin di grup ini.") + return + + db = get_database() - if admin_user_id not in admin_ids: - await query.edit_message_text("❌ Kamu tidak memiliki izin untuk menggunakan perintah ini.") - logger.warning( - f"Non-admin user {admin_user_id} ({query.from_user.full_name}) " - f"attempted to use unverify callback" + try: + message = await unverify_user(db, target_user_id) + await query.edit_message_text(message) + logger.info( + f"Admin {admin_user_id} ({query.from_user.full_name}) " + f"unverified user {target_user_id} via callback" + ) + except ValueError: + await query.edit_message_text( + f"ℹ️ User dengan ID {target_user_id} tidak ada di whitelist." ) + except Exception as e: + await query.edit_message_text(f"❌ Terjadi kesalahan: {e}") + logger.error(f"Error during unverify callback: {e}", exc_info=True) + + +async def handle_unrestrict_callback( + update: Update, context: ContextTypes.DEFAULT_TYPE +) -> None: + """ + Handle callback query for unrestrict button (group-scoped). + + Only lifts bot-applied restrictions, not manual admin restrictions. + + Callback data format: unrestrict:{group_id}:{user_id} + """ + query = update.callback_query + if not query or not query.from_user or not query.data: return - # Extract user_id from callback_data + await query.answer() + + parts = query.data.split(":") try: - target_user_id = int(query.data.split(":")[1]) + group_id = int(parts[1]) + target_user_id = int(parts[2]) except (IndexError, ValueError): await query.edit_message_text("❌ Data callback tidak valid.") - logger.error(f"Invalid callback_data format: {query.data}") + return + + admin_user_id = query.from_user.id + if not is_user_admin_in_group(context, group_id, admin_user_id): + await query.edit_message_text("❌ Kamu bukan admin di grup ini.") return db = get_database() try: - message = await unverify_user(db, target_user_id, admin_user_id) - await query.edit_message_text(message) - logger.info( - f"Admin {admin_user_id} ({query.from_user.full_name}) " - f"unverified user {target_user_id} via callback" + message = await unrestrict_user_in_group( + context.bot, db, target_user_id, group_id ) - except ValueError as e: - await query.edit_message_text(f"ℹ️ User dengan ID {target_user_id} tidak ada di whitelist.") + await query.edit_message_text(message, parse_mode="Markdown") logger.info( - f"Admin {admin_user_id} tried to unverify {target_user_id} via callback but not in whitelist: {e}" + f"Admin {admin_user_id} unrestricting user {target_user_id} in group {group_id} via callback" ) except Exception as e: - await query.edit_message_text(f"❌ Terjadi kesalahan: {str(e)}") - logger.error(f"Error during unverify callback: {e}", exc_info=True) + await query.edit_message_text(f"❌ Terjadi kesalahan: {e}") + logger.error(f"Error during unrestrict callback: {e}", exc_info=True) diff --git a/src/bot/plugins/builtin/commands.py b/src/bot/plugins/builtin/commands.py index f355baf..b2f6ca7 100644 --- a/src/bot/plugins/builtin/commands.py +++ b/src/bot/plugins/builtin/commands.py @@ -9,9 +9,6 @@ of plugin toggle state. This matches pre-refactor behavior where admin commands were never gated. -See ``bot.plugins.definitions.ADMIN_COMMANDS`` for the canonical set -of admin-only plugins that skip runtime gating. - Also exposes individual registrar functions (register_verify, register_unverify, etc.) for fine-grained plugin registration. """ @@ -23,7 +20,12 @@ from telegram.ext import CallbackQueryHandler, CommandHandler, MessageHandler, filters -from bot.handlers.check import handle_check_command, handle_check_forwarded_message, handle_warn_callback +from bot.handlers.check import ( + handle_check_command, + handle_check_forwarded_message, + handle_check_group_callback, + handle_warn_callback, +) from bot.handlers.trust import ( handle_trust_callback, handle_trust_command, @@ -32,6 +34,7 @@ handle_untrust_command, ) from bot.handlers.verify import ( + handle_unrestrict_callback, handle_unverify_callback, handle_unverify_command, handle_verify_callback, @@ -100,36 +103,64 @@ def register_check_forwarded_message(application: Application) -> list[BaseHandl return _register(application, handler, "check_forwarded_message") +def register_check_group_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] + """Register group selector callback for /check.""" + handler: BaseHandler = CallbackQueryHandler( + handle_check_group_callback, + pattern=r"^checkgrp:-?\d+:\d+$", + ) + return _register(application, handler, "check_group_callback") + + def register_verify_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] - """Register verify callback handler.""" + """Register verify callback handler (group-scoped).""" handler: BaseHandler = CallbackQueryHandler( handle_verify_callback, - # User IDs are always positive in Telegram, so \d+ (no negative lookbehind) is correct. - # Group IDs can be negative, but callback data only encodes user IDs. - pattern=r"^verify:\d+$", + pattern=r"^verify:-?\d+:\d+$", ) return _register(application, handler, "verify_callback") def register_unverify_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] - """Register unverify callback handler.""" - handler: BaseHandler = CallbackQueryHandler(handle_unverify_callback, pattern=r"^unverify:\d+$") + """Register unverify callback handler (group-scoped).""" + handler: BaseHandler = CallbackQueryHandler( + handle_unverify_callback, + pattern=r"^unverify:-?\d+:\d+$", + ) return _register(application, handler, "unverify_callback") def register_warn_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] - """Register warn callback handler.""" - handler: BaseHandler = CallbackQueryHandler(handle_warn_callback, pattern=r"^warn:\d+:") + """Register warn callback handler (group-scoped).""" + handler: BaseHandler = CallbackQueryHandler( + handle_warn_callback, + pattern=r"^warn:-?\d+:\d+:", + ) return _register(application, handler, "warn_callback") def register_trust_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] - """Register trust callback handler.""" - handler: BaseHandler = CallbackQueryHandler(handle_trust_callback, pattern=r"^trust:\d+$") + """Register trust callback handler (group-scoped).""" + handler: BaseHandler = CallbackQueryHandler( + handle_trust_callback, + pattern=r"^trust:-?\d+:\d+$", + ) return _register(application, handler, "trust_callback") def register_untrust_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] - """Register untrust callback handler.""" - handler: BaseHandler = CallbackQueryHandler(handle_untrust_callback, pattern=r"^untrust:\d+$") + """Register untrust callback handler (group-scoped).""" + handler: BaseHandler = CallbackQueryHandler( + handle_untrust_callback, + pattern=r"^untrust:-?\d+:\d+$", + ) return _register(application, handler, "untrust_callback") + + +def register_unrestrict_callback(application: Application) -> list[BaseHandler]: # type: ignore[type-arg] + """Register unrestrict callback handler (group-scoped).""" + handler: BaseHandler = CallbackQueryHandler( + handle_unrestrict_callback, + pattern=r"^unrestrict:-?\d+:\d+$", + ) + return _register(application, handler, "unrestrict_callback") diff --git a/src/bot/plugins/definitions.py b/src/bot/plugins/definitions.py index 055af0b..4b0bc5f 100644 --- a/src/bot/plugins/definitions.py +++ b/src/bot/plugins/definitions.py @@ -24,11 +24,13 @@ {"name": "untrust", "handler_group": 0, "description": "Admin /untrust command"}, {"name": "trusted_list", "handler_group": 0, "description": "Admin /trusted list command"}, {"name": "check_forwarded_message", "handler_group": 0, "description": "Handle forwarded messages for /check context"}, - {"name": "verify_callback", "handler_group": 0, "description": "Admin verify confirm button callback"}, + {"name": "check_group_callback", "handler_group": 0, "description": "Group selector callback for /check"}, + {"name": "verify_callback", "handler_group": 0, "description": "Admin verify (photo exemption) button callback"}, {"name": "unverify_callback", "handler_group": 0, "description": "Admin unverify button callback"}, {"name": "warn_callback", "handler_group": 0, "description": "Admin warn button callback"}, - {"name": "trust_callback", "handler_group": 0, "description": "Admin trust button callback"}, + {"name": "trust_callback", "handler_group": 0, "description": "Admin trust (anti-spam exempt) button callback"}, {"name": "untrust_callback", "handler_group": 0, "description": "Admin untrust button callback"}, + {"name": "unrestrict_callback", "handler_group": 0, "description": "Admin unrestrict (bot restriction only) button callback"}, {"name": "captcha", "handler_group": 0, "description": "Captcha verification for new members"}, {"name": "dm", "handler_group": 0, "description": "Direct message unrestriction flow"}, {"name": "status", "handler_group": 0, "description": "Admin /status command"}, diff --git a/src/bot/plugins/manager.py b/src/bot/plugins/manager.py index dbc17c6..17574cf 100644 --- a/src/bot/plugins/manager.py +++ b/src/bot/plugins/manager.py @@ -57,11 +57,13 @@ "untrust": commands.register_untrust, "trusted_list": commands.register_trusted_list, "check_forwarded_message": commands.register_check_forwarded_message, + "check_group_callback": commands.register_check_group_callback, "verify_callback": commands.register_verify_callback, "unverify_callback": commands.register_unverify_callback, "warn_callback": commands.register_warn_callback, "trust_callback": commands.register_trust_callback, "untrust_callback": commands.register_untrust_callback, + "unrestrict_callback": commands.register_unrestrict_callback, # captcha "captcha": captcha_mod.register_captcha, # dm diff --git a/src/bot/services/telegram_utils.py b/src/bot/services/telegram_utils.py index 529d6b6..37bcfb8 100644 --- a/src/bot/services/telegram_utils.py +++ b/src/bot/services/telegram_utils.py @@ -257,6 +257,49 @@ def is_user_admin_or_trusted(context: object, group_id: int, user_id: int) -> bo trusted_ids = _get_trusted_ids(bot_data) return user_id in trusted_ids + +def is_user_admin_in_group(context: object, group_id: int, user_id: int) -> bool: + """ + Check whether a user is a human admin of a specific group. + + Unlike ``is_user_admin_or_trusted``, this checks only the per-group + admin cache — it does NOT consider trusted users. Use this for + admin authorization on moderation actions where trust bypass is + not appropriate. + + Args: + context: Telegram context with ``bot_data``. + group_id: Telegram group ID. + user_id: Telegram user ID. + + Returns: + bool: True if the user is an admin of the given group. + """ + bot_data = getattr(context, "bot_data", {}) + admin_ids = bot_data.get("group_admin_ids", {}).get(group_id, []) + return user_id in admin_ids + + +def get_admin_groups(context: object, user_id: int) -> list[int]: + """ + Return the list of group IDs where the given user is an admin. + + Uses the per-group admin cache in ``bot_data["group_admin_ids"]``. + Useful for determining which groups an admin can act on. + + Args: + context: Telegram context with ``bot_data``. + user_id: Telegram user ID. + + Returns: + list[int]: Group IDs where the user is an admin (may be empty). + """ + bot_data = getattr(context, "bot_data", {}) + group_admin_ids: dict[int, list[int]] = bot_data.get("group_admin_ids", {}) + return [ + gid for gid, ids in group_admin_ids.items() if user_id in ids + ] + def _retry_after_seconds(e: RetryAfter) -> float: """Extract RetryAfter.retry_after as seconds (handles int and timedelta).""" return e.retry_after.total_seconds() if isinstance(e.retry_after, timedelta) else e.retry_after diff --git a/tests/test_check.py b/tests/test_check.py index 0211dc8..3a142c4 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -7,8 +7,10 @@ from bot.group_config import GroupConfig, GroupRegistry from bot.handlers.check import ( + _parse_warn_callback_data, handle_check_command, handle_check_forwarded_message, + handle_check_group_callback, handle_warn_callback, ) from bot.services.user_checker import ProfileCheckResult @@ -67,7 +69,10 @@ def mock_context(): mock_chat.username = "testuser" context.bot.get_chat.return_value = mock_chat - context.bot_data = {"admin_ids": [12345]} + context.bot_data = { + "admin_ids": [12345], + "group_admin_ids": {-1001234567890: [12345]}, + } context.args = [] return context @@ -76,7 +81,10 @@ class TestHandleCheckCommand: async def test_check_command_non_admin(self, mock_update, mock_context): """Non-admin cannot use /check.""" mock_update.message.from_user.id = 99999 - mock_context.bot_data = {"admin_ids": [12345]} + mock_context.bot_data = { + "admin_ids": [12345], + "group_admin_ids": {-1001234567890: []}, + } mock_context.args = ["123456"] await handle_check_command(mock_update, mock_context) @@ -134,7 +142,7 @@ async def test_check_command_complete_profile(self, mock_update, mock_context): assert keyboard is not None buttons = keyboard.inline_keyboard[0] callback_data = [btn.callback_data for btn in buttons] - assert any(data == "trust:555666" for data in callback_data) + assert any(data == "trust:-1001234567890:555666" for data in callback_data) assert not any("unverify" in data for data in callback_data) async def test_check_command_complete_profile_shows_trust_button_when_not_trusted( @@ -161,7 +169,7 @@ async def test_check_command_complete_profile_shows_trust_button_when_not_truste assert keyboard is not None buttons = keyboard.inline_keyboard[0] callback_data = [btn.callback_data for btn in buttons] - assert any(data == "trust:555666" for data in callback_data) + assert any(data == "trust:-1001234567890:555666" for data in callback_data) async def test_check_command_complete_profile_whitelisted( self, mock_update, mock_context @@ -192,8 +200,38 @@ async def test_check_command_complete_profile_whitelisted( keyboard = call_args.kwargs.get("reply_markup") assert keyboard is not None buttons = keyboard.inline_keyboard[0] - assert any("unverify:555666" in btn.callback_data for btn in buttons) - assert any("Unverify User" in btn.text for btn in buttons) + assert any( + btn.callback_data == "unverify:-1001234567890:555666" + for btn in buttons + ) + assert any("Cabut izin foto" in btn.text for btn in buttons) + + async def test_check_command_multiple_admin_groups_shows_group_selector( + self, mock_update, mock_context + ): + """Admins of multiple groups choose the group before acting.""" + mock_context.args = ["555666"] + mock_context.bot_data["group_admin_ids"] = { + -1001234567890: [12345], + -1009876543210: [12345], + } + result = ProfileCheckResult(has_profile_photo=True, has_username=True) + mock_db = MagicMock() + mock_db.is_user_photo_whitelisted.return_value = False + mock_db.is_user_trusted.return_value = False + + with ( + patch("bot.handlers.check.check_user_profile", return_value=result), + patch("bot.handlers.check.get_database", return_value=mock_db), + ): + await handle_check_command(mock_update, mock_context) + + keyboard = mock_update.message.reply_text.call_args.kwargs["reply_markup"] + callback_data = [row[0].callback_data for row in keyboard.inline_keyboard] + assert callback_data == [ + "checkgrp:-1001234567890:555666", + "checkgrp:-1009876543210:555666", + ] async def test_check_command_incomplete_profile(self, mock_update, mock_context): """Shows incomplete profile with warn button.""" @@ -224,8 +262,8 @@ async def test_check_command_incomplete_profile(self, mock_update, mock_context) assert keyboard is not None buttons = keyboard.inline_keyboard[0] callback_data = [btn.callback_data for btn in buttons] - assert any("warn:555666" in data for data in callback_data) - assert any("verify:555666" in data for data in callback_data) + assert "warn:-1001234567890:555666:pu" in callback_data + assert "verify:-1001234567890:555666" in callback_data async def test_check_command_incomplete_profile_shows_trust_button( self, mock_update, mock_context @@ -249,9 +287,9 @@ async def test_check_command_incomplete_profile_shows_trust_button( keyboard = mock_update.message.reply_text.call_args.kwargs.get("reply_markup") assert keyboard is not None - buttons = keyboard.inline_keyboard[0] + buttons = [button for row in keyboard.inline_keyboard for button in row] callback_data = [btn.callback_data for btn in buttons] - assert any(data == "trust:555666" for data in callback_data) + assert "trust:-1001234567890:555666" in callback_data async def test_check_command_complete_profile_shows_untrust_button_when_trusted( self, mock_update, mock_context @@ -277,7 +315,9 @@ async def test_check_command_complete_profile_shows_untrust_button_when_trusted( assert keyboard is not None buttons = keyboard.inline_keyboard[0] callback_data = [btn.callback_data for btn in buttons] - assert any(data == "untrust:555666" for data in callback_data) + assert any( + data == "untrust:-1001234567890:555666" for data in callback_data + ) async def test_check_command_only_private(self, mock_update, mock_context): """Command only works in private chat.""" @@ -330,20 +370,32 @@ async def test_check_command_timeout(self, mock_update, mock_context): class TestHandleCheckForwardedMessage: async def test_check_forwarded_non_admin(self, mock_update, mock_context): - """Non-admin cannot forward for check.""" + """Forwarded checks show no available groups for a non-admin.""" mock_update.message.from_user.id = 99999 - mock_context.bot_data = {"admin_ids": [12345]} + mock_context.bot_data = { + "admin_ids": [12345], + "group_admin_ids": {-1001234567890: []}, + } forwarded_user = MagicMock() forwarded_user.id = 555666 forwarded_user.full_name = "Forwarded User" mock_update.message.forward_from = forwarded_user - await handle_check_forwarded_message(mock_update, mock_context) + result = ProfileCheckResult(has_profile_photo=True, has_username=True) + mock_db = MagicMock() + mock_db.is_user_photo_whitelisted.return_value = False + mock_db.is_user_trusted.return_value = False + + with ( + patch("bot.handlers.check.check_user_profile", return_value=result), + patch("bot.handlers.check.get_database", return_value=mock_db), + ): + await handle_check_forwarded_message(mock_update, mock_context) mock_update.message.reply_text.assert_called_once() call_args = mock_update.message.reply_text.call_args - assert "izin" in call_args.args[0] + assert "admin" in call_args.args[0] async def test_check_forwarded_hidden_user(self, mock_update, mock_context): """Hidden forward privacy shows error.""" @@ -460,7 +512,80 @@ async def test_check_forwarded_timeout(self, mock_update, mock_context): assert "timeout" in call_args.args[0].lower() +class TestHandleCheckGroupCallback: + async def test_group_callback_shows_group_scoped_actions(self, mock_context): + """A group selection displays actions encoded for that group.""" + update = MagicMock() + update.message = None + query = MagicMock() + query.from_user.id = 12345 + query.data = "checkgrp:-1001234567890:555666" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + result = ProfileCheckResult(has_profile_photo=False, has_username=True) + mock_db = MagicMock() + mock_db.is_user_photo_whitelisted.return_value = False + mock_db.is_user_trusted.return_value = False + + with ( + patch("bot.handlers.check.check_user_profile", return_value=result), + patch("bot.handlers.check.get_database", return_value=mock_db), + ): + await handle_check_group_callback(update, mock_context) + + query.answer.assert_awaited_once() + keyboard = query.edit_message_text.call_args.kwargs["reply_markup"] + callback_data = [ + button.callback_data + for row in keyboard.inline_keyboard + for button in row + ] + assert "warn:-1001234567890:555666:p" in callback_data + assert "verify:-1001234567890:555666" in callback_data + assert "unrestrict:-1001234567890:555666" in callback_data + + async def test_group_callback_rejects_non_admin(self, mock_context): + """A caller must be an admin of the selected group.""" + update = MagicMock() + query = MagicMock() + query.from_user.id = 99999 + query.data = "checkgrp:-1001234567890:555666" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + + await handle_check_group_callback(update, mock_context) + + query.edit_message_text.assert_awaited_once() + assert "bukan admin" in query.edit_message_text.call_args.args[0] + mock_context.bot.get_chat.assert_not_awaited() + + async def test_group_callback_rejects_invalid_data(self, mock_context): + """Malformed group callback data is rejected.""" + update = MagicMock() + query = MagicMock() + query.from_user.id = 12345 + query.data = "checkgrp:invalid" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + + await handle_check_group_callback(update, mock_context) + + assert "tidak valid" in query.edit_message_text.call_args.args[0] + + class TestHandleWarnCallback: + def test_parse_warn_callback_data_includes_group_id(self): + """Warn callback parsing preserves group, user, and missing code.""" + assert _parse_warn_callback_data("warn:-1001234567890:555666:pu") == ( + -1001234567890, + 555666, + "pu", + ) + assert _parse_warn_callback_data("warn:invalid") is None + async def test_warn_callback_non_admin(self, mock_context): """Non-admin cannot use warn callback.""" update = MagicMock() @@ -468,30 +593,32 @@ async def test_warn_callback_non_admin(self, mock_context): query.from_user = MagicMock() query.from_user.id = 99999 query.from_user.full_name = "Non Admin" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query - mock_context.bot_data = {"admin_ids": [12345]} + mock_context.bot_data = { + "group_admin_ids": {-1001234567890: []}, + } await handle_warn_callback(update, mock_context) query.answer.assert_called_once() query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args - assert "izin" in call_args.args[0] + assert "bukan admin" in call_args.args[0] async def test_warn_callback_success( self, mock_context, mock_settings, group_config, mock_registry ): - """Successfully sends warning to all monitored groups.""" + """Successfully sends warning to the selected group.""" update = MagicMock() query = MagicMock() query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -533,7 +660,7 @@ async def test_warn_callback_success_missing_photo_only( query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:p" + query.data = "warn:-1001234567890:555666:p" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -607,7 +734,7 @@ async def test_warn_callback_send_message_error( query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -638,7 +765,7 @@ async def test_warn_callback_timeout( query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -656,8 +783,6 @@ async def test_warn_callback_timeout( ): await handle_warn_callback(update, mock_context) - # TimedOut is caught per-group inside the loop, so all groups fail - # and the "failed to send to all groups" message is shown query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args assert "Gagal mengirim" in call_args.args[0] @@ -671,7 +796,7 @@ async def test_warn_callback_get_chat_timeout( query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -690,16 +815,16 @@ async def test_warn_callback_get_chat_timeout( call_args = query.edit_message_text.call_args assert "timeout" in call_args.args[0].lower() - async def test_warn_callback_per_group_send_failure_all_groups( + async def test_warn_callback_send_failure( self, mock_context, mock_settings, mock_registry ): - """When send_message fails for all groups, shows 'all groups failed' message.""" + """A selected-group send failure shows an error.""" update = MagicMock() query = MagicMock() query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "warn:555666:pu" + query.data = "warn:-1001234567890:555666:pu" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -720,4 +845,4 @@ async def test_warn_callback_per_group_send_failure_all_groups( query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args - assert "Gagal mengirim peringatan ke semua grup" in call_args.args[0] + assert "Gagal mengirim peringatan" in call_args.args[0] diff --git a/tests/test_plugin_manager.py b/tests/test_plugin_manager.py index c83a227..9df3406 100644 --- a/tests/test_plugin_manager.py +++ b/tests/test_plugin_manager.py @@ -116,7 +116,7 @@ def test_verify_callback_description(self): """verify_callback description says 'Admin verify confirm button callback' not 'Captcha verify'.""" defs = get_plugin_definitions() defs_by_name = {d["name"]: d for d in defs} - assert defs_by_name["verify_callback"]["description"] == "Admin verify confirm button callback" + assert defs_by_name["verify_callback"]["description"] == "Admin verify (photo exemption) button callback" class TestManifestOrder: """MANIFEST_ORDER defines deterministic handler registration order matching main.py.""" @@ -133,11 +133,13 @@ def _expected_order() -> tuple[str, ...]: "untrust", "trusted_list", "check_forwarded_message", + "check_group_callback", "verify_callback", "unverify_callback", "warn_callback", "trust_callback", "untrust_callback", + "unrestrict_callback", "captcha", "dm", "status", diff --git a/tests/test_trust_handler.py b/tests/test_trust_handler.py index 6f80b72..38185ec 100644 --- a/tests/test_trust_handler.py +++ b/tests/test_trust_handler.py @@ -7,6 +7,7 @@ import pytest from bot.constants import ( + TRUST_NO_GROUP_PERMISSION_MESSAGE, TRUST_USER_ID_INVALID_MESSAGE, TRUST_USER_ID_REQUIRED_MESSAGE, ) @@ -119,9 +120,6 @@ async def test_trust_command_success_by_user_id( monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) mock_context.args = ["1111"] - unrestrict = AsyncMock() - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", unrestrict) - db = get_database() db.start_new_user_probation(user_id=1111, group_id=-1001) db.start_new_user_probation(user_id=1111, group_id=-1002) @@ -133,7 +131,6 @@ async def test_trust_command_success_by_user_id( assert isinstance(mock_context.bot_data["trusted_user_ids"], set) assert db.get_new_user_probation(1111, -1001) is None assert db.get_new_user_probation(1111, -1002) is None - assert unrestrict.await_count == 2 reply_args = mock_update.message.reply_text.call_args assert "ditambahkan" in reply_args.args[0].lower() assert reply_args.kwargs.get("parse_mode") == "Markdown" @@ -148,7 +145,6 @@ async def test_trust_command_admin_without_username( ): """Admin without a public @handle must round-trip admin_username=None.""" monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) mock_update.message.from_user.username = None mock_context.args = ["1111"] @@ -164,7 +160,6 @@ async def test_trust_command_success_from_forwarded_message( self, mock_update, mock_context, mock_registry, monkeypatch ): monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) forwarded_user = MagicMock() forwarded_user.id = 4444 @@ -178,7 +173,6 @@ async def test_trust_command_success_from_forwarded_message( async def test_trust_command_duplicate(self, mock_update, mock_context, mock_registry, monkeypatch): monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) mock_context.args = ["1111"] db = get_database() @@ -191,30 +185,13 @@ async def test_trust_command_duplicate(self, mock_update, mock_context, mock_reg assert "sudah" in reply_args.args[0].lower() assert reply_args.kwargs.get("parse_mode") == "Markdown" - async def test_trust_command_continues_on_unrestrict_error( + async def test_trust_command_continues_when_probation_lookup_fails( self, mock_update, mock_context, mock_registry, monkeypatch ): monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - - unrestrict = AsyncMock(side_effect=[Exception("failed"), None]) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", unrestrict) - mock_context.args = ["2111"] - - await handle_trust_command(mock_update, mock_context) - - assert get_database().is_user_trusted(2111) is True - - async def test_trust_command_unrestrict_attempted_even_when_probation_lookup_fails( - self, mock_update, mock_context, mock_registry, monkeypatch - ): - """If probation lookup raises, unrestrict_user is still called for that group.""" - monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - unrestrict = AsyncMock() - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", unrestrict) mock_context.args = ["9111"] db = get_database() - # Force get_new_user_probation to blow up for both groups. monkeypatch.setattr( db, "get_new_user_probation", @@ -223,14 +200,10 @@ async def test_trust_command_unrestrict_attempted_even_when_probation_lookup_fai await handle_trust_command(mock_update, mock_context) - # Unrestrict was still attempted once per group despite probation - # cleanup raising. - assert unrestrict.await_count == 2 + assert db.is_user_trusted(9111) is True reply_args = mock_update.message.reply_text.call_args assert "ditambahkan" in reply_args.args[0].lower() - # Probation count is 0, unrestrict count is 2. assert "0" in reply_args.args[0] - assert "2" in reply_args.args[0] async def test_untrust_command_requires_private_chat(self, mock_update, mock_context): mock_update.effective_chat.type = "group" @@ -432,8 +405,8 @@ async def test_trust_callback_invalid_data(self, mock_callback_update, mock_cont async def test_trust_callback_success(self, mock_callback_update, mock_context, mock_registry, monkeypatch): monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) - mock_callback_update.callback_query.data = "trust:7001" + mock_context.bot_data["group_admin_ids"] = {-1001: [12345]} + mock_callback_update.callback_query.data = "trust:-1001:7001" await handle_trust_callback(mock_callback_update, mock_context) @@ -454,9 +427,9 @@ async def test_trust_callback_admin_without_username( ): """Callback path with admin username=None must round-trip None to DB.""" monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) + mock_context.bot_data["group_admin_ids"] = {-1001: [12345]} mock_callback_update.callback_query.from_user.username = None - mock_callback_update.callback_query.data = "trust:7001" + mock_callback_update.callback_query.data = "trust:-1001:7001" await handle_trust_callback(mock_callback_update, mock_context) @@ -483,7 +456,8 @@ async def test_untrust_callback_invalid_data(self, mock_callback_update, mock_co async def test_untrust_callback_success(self, mock_callback_update, mock_context): get_database().add_trusted_user(user_id=7002, trusted_by_admin_id=12345) mock_context.bot_data["trusted_user_ids"] = {7002} - mock_callback_update.callback_query.data = "untrust:7002" + mock_context.bot_data["group_admin_ids"] = {-1001: [12345]} + mock_callback_update.callback_query.data = "untrust:-1001:7002" await handle_untrust_callback(mock_callback_update, mock_context) @@ -499,9 +473,9 @@ async def test_trust_callback_duplicate( ): """Trust callback for already-trusted user yields TRUST_ALREADY_EXISTS message.""" monkeypatch.setattr("bot.handlers.trust.get_group_registry", lambda: mock_registry) - monkeypatch.setattr("bot.handlers.trust.unrestrict_user", AsyncMock()) get_database().add_trusted_user(user_id=7003, trusted_by_admin_id=12345) - mock_callback_update.callback_query.data = "trust:7003" + mock_context.bot_data["group_admin_ids"] = {-1001: [12345]} + mock_callback_update.callback_query.data = "trust:-1001:7003" await handle_trust_callback(mock_callback_update, mock_context) @@ -511,7 +485,8 @@ async def test_trust_callback_duplicate( async def test_untrust_callback_missing_user(self, mock_callback_update, mock_context): """Untrust callback for user not in trusted list yields not-found message.""" - mock_callback_update.callback_query.data = "untrust:7004" + mock_context.bot_data["group_admin_ids"] = {-1001: [12345]} + mock_callback_update.callback_query.data = "untrust:-1001:7004" await handle_untrust_callback(mock_callback_update, mock_context) @@ -521,21 +496,29 @@ async def test_untrust_callback_missing_user(self, mock_callback_update, mock_co async def test_callback_non_admin_rejected(self, mock_callback_update, mock_context): mock_callback_update.callback_query.from_user.id = 99999 - mock_callback_update.callback_query.data = "trust:8003" + mock_context.bot_data["group_admin_ids"] = {} + mock_callback_update.callback_query.data = "trust:-1001:8003" await handle_trust_callback(mock_callback_update, mock_context) mock_callback_update.callback_query.edit_message_text.assert_called_once() - assert "izin" in mock_callback_update.callback_query.edit_message_text.call_args.args[0].lower() + assert ( + mock_callback_update.callback_query.edit_message_text.call_args.args[0] + == TRUST_NO_GROUP_PERMISSION_MESSAGE + ) async def test_untrust_callback_non_admin_rejected(self, mock_callback_update, mock_context): mock_callback_update.callback_query.from_user.id = 99999 - mock_callback_update.callback_query.data = "untrust:8003" + mock_context.bot_data["group_admin_ids"] = {} + mock_callback_update.callback_query.data = "untrust:-1001:8003" await handle_untrust_callback(mock_callback_update, mock_context) mock_callback_update.callback_query.edit_message_text.assert_called_once() - assert "izin" in mock_callback_update.callback_query.edit_message_text.call_args.args[0].lower() + assert ( + mock_callback_update.callback_query.edit_message_text.call_args.args[0] + == TRUST_NO_GROUP_PERMISSION_MESSAGE + ) class TestResolveTargetUserId: diff --git a/tests/test_verify_handler.py b/tests/test_verify_handler.py index 403d2b1..021e3b0 100644 --- a/tests/test_verify_handler.py +++ b/tests/test_verify_handler.py @@ -7,12 +7,16 @@ from bot.database.service import get_database, init_database, reset_database from bot.group_config import GroupConfig, GroupRegistry from bot.handlers.verify import ( + handle_unrestrict_callback, handle_unverify_callback, handle_unverify_command, handle_verify_callback, handle_verify_command, + unrestrict_user_in_group, ) +GROUP_ID = -1001234567890 + @pytest.fixture(autouse=True) def temp_db(): @@ -60,7 +64,10 @@ def mock_context(): mock_chat.username = "testuser" context.bot.get_chat.return_value = mock_chat - context.bot_data = {"admin_ids": [12345]} + context.bot_data = { + "admin_ids": [12345], + "group_admin_ids": {GROUP_ID: [12345]}, + } context.args = [] return context @@ -138,7 +145,6 @@ async def test_successful_verify_new_user(self, mock_update, mock_context, temp_ response_text = call_args.args[0] assert "diverifikasi" in response_text assert "whitelist foto profil" in response_text - assert "Pembatasan dicabut" in response_text assert "Riwayat warning dihapus" in response_text assert str(target_user_id) in response_text @@ -230,23 +236,22 @@ async def test_verify_large_user_id(self, mock_update, mock_context, temp_db, mo db = get_database() assert db.is_user_photo_whitelisted(large_id) - async def test_verify_unrestricts_user(self, mock_update, mock_context, temp_db, monkeypatch): - """Test that verify command unrestricts the user.""" - gc = GroupConfig(group_id=-1001234567890, warning_topic_id=12345) - registry = GroupRegistry() - registry.register(gc) - monkeypatch.setattr("bot.handlers.verify.get_group_registry", lambda: registry) - - target_user_id = 33333333 # Use unique ID + async def test_multi_group_admin_only_adds_whitelist( + self, mock_update, mock_context, temp_db + ): + target_user_id = 33333333 mock_context.args = [str(target_user_id)] + mock_context.bot_data["group_admin_ids"] = { + GROUP_ID: [12345], + -1009876543210: [12345], + } await handle_verify_command(mock_update, mock_context) - # Should call restrict_chat_member with unrestricted permissions - mock_context.bot.restrict_chat_member.assert_called_once() - call_args = mock_context.bot.restrict_chat_member.call_args - assert call_args.kwargs["user_id"] == target_user_id - assert call_args.kwargs["permissions"].can_send_messages is True + assert get_database().is_user_photo_whitelisted(target_user_id) + assert "/check" in mock_update.message.reply_text.call_args.args[0] + mock_context.bot.restrict_chat_member.assert_not_called() + mock_context.bot.send_message.assert_not_called() async def test_verify_deletes_warnings(self, mock_update, mock_context, temp_db, monkeypatch): """Test that verify command deletes all warning records.""" @@ -295,10 +300,7 @@ async def test_verify_sends_clearance_message_when_warnings_deleted( mock_user_chat.username = "verified_user" mock_user_chat.full_name = "Verified User" - # First call is for group permissions, second call is for user info - mock_group_chat = MagicMock() - mock_group_chat.permissions = MagicMock() - mock_context.bot.get_chat.side_effect = [mock_group_chat, mock_user_chat] + mock_context.bot.get_chat.return_value = mock_user_chat mock_context.args = [str(target_user_id)] await handle_verify_command(mock_update, mock_context) @@ -580,19 +582,19 @@ async def test_non_admin_rejected(self, mock_context): query.from_user = MagicMock() query.from_user.id = 99999 query.from_user.full_name = "Non Admin" - query.data = "verify:555666" + query.data = f"verify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query - mock_context.bot_data = {"admin_ids": [12345]} + mock_context.bot_data = {"group_admin_ids": {}} await handle_verify_callback(update, mock_context) query.answer.assert_called_once() query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args - assert "izin" in call_args.args[0] + assert "bukan admin" in call_args.args[0] async def test_invalid_callback_data_format(self, mock_context): update = MagicMock() @@ -623,7 +625,7 @@ async def test_successful_verify_callback(self, temp_db, mock_context, monkeypat query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "verify:999888" + query.data = f"verify:{gc.group_id}:999888" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -653,7 +655,7 @@ async def test_verify_callback_already_whitelisted(self, temp_db, mock_context, query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "verify:555666" + query.data = f"verify:{gc.group_id}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -671,7 +673,7 @@ async def test_verify_callback_generic_exception(self, temp_db, mock_context): query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "verify:555666" + query.data = f"verify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -706,19 +708,19 @@ async def test_non_admin_rejected(self, mock_context): query.from_user = MagicMock() query.from_user.id = 99999 query.from_user.full_name = "Non Admin" - query.data = "unverify:555666" + query.data = f"unverify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query - mock_context.bot_data = {"admin_ids": [12345]} + mock_context.bot_data = {"group_admin_ids": {}} await handle_unverify_callback(update, mock_context) query.answer.assert_called_once() query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args - assert "izin" in call_args.args[0] + assert "bukan admin" in call_args.args[0] async def test_invalid_callback_data_format(self, mock_context): update = MagicMock() @@ -747,7 +749,7 @@ async def test_successful_unverify_callback(self, temp_db, mock_context): query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "unverify:555666" + query.data = f"unverify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -768,7 +770,7 @@ async def test_unverify_callback_not_whitelisted(self, temp_db, mock_context): query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "unverify:555666" + query.data = f"unverify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -786,7 +788,7 @@ async def test_unverify_callback_generic_exception(self, temp_db, mock_context): query.from_user = MagicMock() query.from_user.id = 12345 query.from_user.full_name = "Admin User" - query.data = "unverify:555666" + query.data = f"unverify:{GROUP_ID}:555666" query.answer = AsyncMock() query.edit_message_text = AsyncMock() update.callback_query = query @@ -799,3 +801,84 @@ async def test_unverify_callback_generic_exception(self, temp_db, mock_context): query.edit_message_text.assert_called_once() call_args = query.edit_message_text.call_args assert "Terjadi kesalahan" in call_args.args[0] + + +class TestUnrestrictUserInGroup: + async def test_unrestricts_bot_restricted_user(self, temp_db, mock_context): + target_user_id = 777001 + db = get_database() + db.get_or_create_user_warning(target_user_id, GROUP_ID) + db.mark_user_restricted(target_user_id, GROUP_ID) + + message = await unrestrict_user_in_group( + mock_context.bot, db, target_user_id, GROUP_ID + ) + + assert "Pembatasan bot" in message + assert str(GROUP_ID) in message + assert not db.is_user_restricted_by_bot(target_user_id, GROUP_ID) + mock_context.bot.restrict_chat_member.assert_called_once() + + async def test_does_not_lift_non_bot_restriction(self, temp_db, mock_context): + message = await unrestrict_user_in_group( + mock_context.bot, get_database(), 777002, GROUP_ID + ) + + assert "tidak dibatasi oleh bot" in message + mock_context.bot.restrict_chat_member.assert_not_called() + + async def test_returns_failure_when_telegram_call_fails(self, temp_db, mock_context): + target_user_id = 777003 + db = get_database() + db.get_or_create_user_warning(target_user_id, GROUP_ID) + db.mark_user_restricted(target_user_id, GROUP_ID) + mock_context.bot.restrict_chat_member.side_effect = RuntimeError("network error") + + message = await unrestrict_user_in_group( + mock_context.bot, db, target_user_id, GROUP_ID + ) + + assert "Gagal membuka pembatasan" in message + assert db.is_user_restricted_by_bot(target_user_id, GROUP_ID) + + +class TestHandleUnrestrictCallback: + @staticmethod + def make_update(admin_id=12345): + update = MagicMock() + query = MagicMock() + query.from_user = MagicMock(id=admin_id) + query.data = f"unrestrict:{GROUP_ID}:888001" + query.answer = AsyncMock() + query.edit_message_text = AsyncMock() + update.callback_query = query + return update, query + + async def test_non_group_admin_rejected(self, mock_context): + update, query = self.make_update(admin_id=99999) + mock_context.bot_data["group_admin_ids"] = {} + + await handle_unrestrict_callback(update, mock_context) + + query.answer.assert_awaited_once() + assert "bukan admin" in query.edit_message_text.call_args.args[0] + + async def test_group_admin_unrestricts_user(self, temp_db, mock_context): + update, query = self.make_update() + db = get_database() + db.get_or_create_user_warning(888001, GROUP_ID) + db.mark_user_restricted(888001, GROUP_ID) + + await handle_unrestrict_callback(update, mock_context) + + query.answer.assert_awaited_once() + assert "Pembatasan bot" in query.edit_message_text.call_args.args[0] + assert not db.is_user_restricted_by_bot(888001, GROUP_ID) + + async def test_invalid_callback_data_rejected(self, mock_context): + update, query = self.make_update() + query.data = "unrestrict:invalid" + + await handle_unrestrict_callback(update, mock_context) + + assert "tidak valid" in query.edit_message_text.call_args.args[0] From 9dcc2588c62dd09b2cf23abe3f9a2240b49a237b Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 31 Jul 2026 13:30:01 +0700 Subject: [PATCH 3/5] feat: captcha validation, DM deep links, stale enforcement fix 1. Captcha callback validation (Item 4): - Validate callback message/chat IDs against persisted challenge - Prevents stale or spoofed callbacks from acting on wrong challenge 2. Contextual DM recovery with deep links (Item 5): - Add /start verify_ deep link parsing in DM handler - Captcha timeout and restriction messages now include group-specific deep links instead of generic bot URL - DM handler uses target group's rules_link instead of global settings - Pending captcha DM message lists specific groups 3. Stale enforcement fix + accurate copy (Item 6): - Clear active profile warnings when compliant profile is observed - Add get_active_user_warning() DB method (non-creating lookup) - Recheck profile before scheduler restriction, skip if now complete - Remove false 'akan dibatasi' threat from warning-only mode - Warning-only mode no longer references threshold_display Updated all affected tests for new behavior. --- src/bot/constants.py | 7 +-- src/bot/database/service.py | 23 ++++++++ src/bot/handlers/captcha.py | 17 +++++- src/bot/handlers/dm.py | 80 ++++++++++++++++++++++------ src/bot/handlers/message.py | 17 +++--- src/bot/services/captcha_recovery.py | 2 +- src/bot/services/scheduler.py | 52 +++++++++++------- tests/test_captcha.py | 4 ++ tests/test_dm_handler.py | 47 +++++----------- tests/test_message_handler.py | 27 ++++++++++ 10 files changed, 197 insertions(+), 79 deletions(-) diff --git a/src/bot/constants.py b/src/bot/constants.py index a4c3dd4..a9ecf3d 100644 --- a/src/bot/constants.py +++ b/src/bot/constants.py @@ -69,8 +69,7 @@ def format_hours_display(hours: int) -> str: # Warning mode (default): No restrictions, just warnings WARNING_MESSAGE_NO_RESTRICTION = ( "⚠️ Hai {user_mention}, mohon lengkapi {missing_text} kamu " - "untuk mematuhi aturan grup.\n" - "Kamu akan dibatasi setelah {threshold_display}.\n\n" + "untuk mematuhi aturan grup.\n\n" "📖 [Baca aturan grup]({rules_link})" ) @@ -117,10 +116,12 @@ def format_hours_display(hours: int) -> str: ) CAPTCHA_PENDING_DM_MESSAGE = ( - "⏳ Kamu memiliki verifikasi captcha yang tertunda.\n" + "⏳ Kamu memiliki verifikasi captcha yang tertunda di grup berikut:\n{group_list}\n\n" "Silakan cek grup dan tekan tombol verifikasi." ) +CAPTCHA_PENDING_DM_GROUP_LINE = "• Grup {group_id}" + CAPTCHA_INCOMPLETE_PROFILE_MESSAGE = ( "❌ Lengkapi {missing_text} terlebih dahulu, lalu tekan tombol ini lagi." ) diff --git a/src/bot/database/service.py b/src/bot/database/service.py index e2588c2..6a8187b 100644 --- a/src/bot/database/service.py +++ b/src/bot/database/service.py @@ -286,6 +286,29 @@ def delete_user_warnings(self, user_id: int, group_id: int) -> int: ) return count + def get_active_user_warning(self, user_id: int, group_id: int) -> UserWarning | None: + """ + Get an existing active (non-restricted) warning record without creating one. + + Unlike ``get_or_create_user_warning``, this does NOT create a record + if none exists. Used to check for stale warnings when a user's profile + becomes compliant. + + Args: + user_id: Telegram user ID. + group_id: Telegram group ID. + + Returns: + UserWarning | None: Active warning record, or None if none exists. + """ + with Session(self._engine) as session: + statement = select(UserWarning).where( + UserWarning.user_id == user_id, + UserWarning.group_id == group_id, + ~UserWarning.is_restricted, + ) + return session.exec(statement).first() + def add_photo_verification_whitelist( self, user_id: int, verified_by_admin_id: int, notes: str | None = None ) -> PhotoVerificationWhitelist: diff --git a/src/bot/handlers/captcha.py b/src/bot/handlers/captcha.py index 973b0f0..18b4846 100644 --- a/src/bot/handlers/captcha.py +++ b/src/bot/handlers/captcha.py @@ -284,11 +284,26 @@ async def captcha_callback_handler( group_config = registry.get(group_id) - if group_config is None or not db.get_pending_captcha(target_user_id, group_id): + pending = db.get_pending_captcha(target_user_id, group_id) + if group_config is None or not pending: logger.warning(f"No pending captcha found for user {target_user_id} in group {group_id}") await query.answer(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) return + # Validate that this callback comes from the original challenge message. + # Prevents stale or spoofed callbacks from acting on a different challenge. + if query.message and ( + query.message.chat_id != pending.chat_id + or query.message.message_id != pending.message_id + ): + logger.warning( + f"Captcha callback from wrong message for user {target_user_id}: " + f"expected chat={pending.chat_id} msg={pending.message_id}, " + f"got chat={query.message.chat_id} msg={query.message.message_id}" + ) + await query.answer(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) + return + try: result = await check_user_profile(context.bot, query.from_user) except Exception: diff --git a/src/bot/handlers/dm.py b/src/bot/handlers/dm.py index 38e4584..f7869ba 100644 --- a/src/bot/handlers/dm.py +++ b/src/bot/handlers/dm.py @@ -7,7 +7,11 @@ 2. Check if user has an active pending captcha (redirect to group) 3. Check if user's profile is complete 4. If profile-restricted by bot and profile complete, unrestrict them - across all monitored groups where they are restricted + in the groups where they are restricted + +Supports deep-link payloads via /start verify_ for group-specific +recovery. The DM message uses the target group's rules_link instead of +the global settings fallback. """ import logging @@ -16,8 +20,8 @@ from telegram.constants import ChatMemberStatus from telegram.ext import ContextTypes -from bot.config import get_settings from bot.constants import ( + CAPTCHA_PENDING_DM_GROUP_LINE, CAPTCHA_PENDING_DM_MESSAGE, DM_ALREADY_UNRESTRICTED_MESSAGE, DM_INCOMPLETE_PROFILE_MESSAGE, @@ -39,6 +43,31 @@ logger = logging.getLogger(__name__) +_VERIFY_DEEP_LINK_PREFIX = "verify_" + + +def _parse_deep_link_payload(text: str) -> int | None: + """Extract a group_id from a /start deep-link payload. + + Supports payloads like ``verify_-1001234567890`` (from + ``https://t.me/BotUsername?start=verify_-1001234567890``). + + Returns the group_id as int, or None if the payload is not a + verify deep link. + """ + if not text: + return None + parts = text.split(maxsplit=1) + if len(parts) < 2: + return None + payload = parts[1].strip() + if not payload.startswith(_VERIFY_DEEP_LINK_PREFIX): + return None + group_id_str = payload[len(_VERIFY_DEEP_LINK_PREFIX):] + try: + return int(group_id_str) + except ValueError: + return None async def _unrestrict_in_groups( @@ -91,34 +120,38 @@ async def handle_dm(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: """ Handle direct messages to the bot for unrestriction flow. - This handler processes DMs (including /start) and: + This handler processes DMs (including /start with deep-link payloads) and: 1. Checks if user is a member of any monitored group 2. Checks if user has an active pending captcha (redirect to group) 3. Checks if user's profile is complete (photo + username) 4. If user was restricted by the bot and now has complete profile, - removes the restriction in all groups where restricted + removes the restriction in the groups where restricted + + If a /start deep-link with ``verify_`` is provided, the + handler prioritizes that group for recovery messaging and uses + that group's rules_link. Args: update: Telegram update containing the message. context: Bot context with helper methods. """ - # Skip if no message or sender if not update.message or not update.message.from_user: logger.info("Skipping DM handler - no message or sender") return - # Only handle private chats if update.effective_chat and update.effective_chat.type != "private": logger.info(f"Skipping non-private chat type: {update.effective_chat.type}") return user = update.message.from_user - settings = get_settings() registry = get_group_registry() db = get_database() logger.info(f"DM handler called for user_id={user.id} ({user.full_name})") + # Parse deep-link payload (e.g., /start verify_-1001234567890) + deep_link_group_id = _parse_deep_link_payload(update.message.text or "") + # Check user's membership across all monitored groups member_groups = [] for gc in registry.all_groups(): @@ -141,15 +174,23 @@ async def handle_dm(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: return # Check if user has an active pending captcha in any group + pending_groups = [] for gc, _ in member_groups: - logger.info(f"Checking for pending captcha for user_id={user.id} in group_id={gc.group_id}") pending_captcha = db.get_pending_captcha(user.id, gc.group_id) if pending_captcha: - await update.message.reply_text(CAPTCHA_PENDING_DM_MESSAGE) - logger.info( - f"DM from user {user.id} ({user.full_name}) - has pending captcha (group_id={gc.group_id})" - ) - return + pending_groups.append(gc.group_id) + + if pending_groups: + group_lines = "\n".join( + CAPTCHA_PENDING_DM_GROUP_LINE.format(group_id=gid) for gid in pending_groups + ) + await update.message.reply_text( + CAPTCHA_PENDING_DM_MESSAGE.format(group_list=group_lines) + ) + logger.info( + f"DM from user {user.id} ({user.full_name}) - has pending captcha in groups: {pending_groups}" + ) + return # Check if user's profile is complete logger.info(f"Checking user profile completeness for user_id={user.id} ({user.full_name})") @@ -159,9 +200,19 @@ async def handle_dm(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: if not result.is_complete: missing = result.get_missing_items() missing_text = MISSING_ITEMS_SEPARATOR.join(missing) + + # Use the deep-linked group's rules_link, or fall back to the first + # member group's rules_link, or the first member group's config + rules_link_gc = None + if deep_link_group_id is not None: + rules_link_gc = registry.get(deep_link_group_id) + if rules_link_gc is None and member_groups: + rules_link_gc = member_groups[0][0] + rules_link = rules_link_gc.rules_link if rules_link_gc else "" + reply_message = DM_INCOMPLETE_PROFILE_MESSAGE.format( missing_text=missing_text, - rules_link=settings.rules_link, + rules_link=rules_link, ) await update.message.reply_text(reply_message, parse_mode="Markdown") logger.info( @@ -193,7 +244,6 @@ async def handle_dm(update: Update, context: ContextTypes.DEFAULT_TYPE) -> None: elif not had_any_restricted: await update.message.reply_text(DM_ALREADY_UNRESTRICTED_MESSAGE) else: - # All unrestriction attempts failed logger.error(f"Failed to unrestrict user {user.id} in any group") await update.message.reply_text( "❌ Gagal membuka pembatasan. Silakan hubungi admin grup." diff --git a/src/bot/handlers/message.py b/src/bot/handlers/message.py index 9cb9a2f..3449fb6 100644 --- a/src/bot/handlers/message.py +++ b/src/bot/handlers/message.py @@ -70,8 +70,17 @@ async def handle_message(update: Update, context: ContextTypes.DEFAULT_TYPE) -> # Check if user has complete profile (photo + username) result = await check_user_profile(context.bot, user) - # User has complete profile, nothing to do + # User has complete profile — clear any stale active warnings if result.is_complete: + db = get_database() + existing = db.get_active_user_warning(user.id, group_config.group_id) + if existing is not None: + deleted = db.delete_user_warnings(user.id, group_config.group_id) + if deleted > 0: + logger.info( + f"Cleared {deleted} stale warning(s) for user {user.id} " + f"(profile now complete, group_id={group_config.group_id})" + ) logger.info( f"User has complete profile: user_id={user.id}, user={user.full_name}" ) @@ -88,13 +97,9 @@ async def handle_message(update: Update, context: ContextTypes.DEFAULT_TYPE) -> # Warning mode: just send warning, don't restrict if not group_config.restrict_failed_users: try: - threshold_display = format_threshold_display( - group_config.warning_time_threshold_minutes - ) warning_message = WARNING_MESSAGE_NO_RESTRICTION.format( user_mention=user_mention, missing_text=missing_text, - threshold_display=threshold_display, rules_link=group_config.rules_link, ) await context.bot.send_message( @@ -167,7 +172,7 @@ async def handle_message(update: Update, context: ContextTypes.DEFAULT_TYPE) -> # Get bot username for DM link (cached to avoid repeated API calls) bot_username = await BotInfoCache.get_username(context.bot) - dm_link = f"https://t.me/{bot_username}" + dm_link = f"https://t.me/{bot_username}?start=verify_{group_config.group_id}" # Send restriction notice with DM link for appeal restriction_message = RESTRICTION_MESSAGE_AFTER_MESSAGES.format( diff --git a/src/bot/services/captcha_recovery.py b/src/bot/services/captcha_recovery.py index d6fde8e..66e62ed 100644 --- a/src/bot/services/captcha_recovery.py +++ b/src/bot/services/captcha_recovery.py @@ -59,7 +59,7 @@ async def handle_captcha_expiration( db.mark_user_restricted(user_id, group_id) bot_username = await BotInfoCache.get_username(bot) - dm_link = f"[hubungi robot](https://t.me/{bot_username})" + dm_link = f"[hubungi robot](https://t.me/{bot_username}?start=verify_{group_id})" user_mention = get_user_mention_by_id(user_id, user_full_name) try: diff --git a/src/bot/services/scheduler.py b/src/bot/services/scheduler.py index a240e25..0a2344f 100644 --- a/src/bot/services/scheduler.py +++ b/src/bot/services/scheduler.py @@ -27,6 +27,7 @@ restrict_chat_member_with_retry, send_message_with_retry, ) +from bot.services.user_checker import check_user_profile logger = logging.getLogger(__name__) @@ -39,6 +40,9 @@ async def auto_restrict_expired_warnings(context: ContextTypes.DEFAULT_TYPE) -> warning_time_threshold_minutes. Finds all active warnings past the configured threshold and applies restrictions (mutes) to those users. + Before restricting, rechecks the user's profile to avoid restricting + users who have already fixed their profile since the last warning. + Args: context: Telegram job context for sending messages. """ @@ -46,14 +50,11 @@ async def auto_restrict_expired_warnings(context: ContextTypes.DEFAULT_TYPE) -> registry = get_group_registry() db = get_database() - # Get bot username once for all DM links bot = context.bot bot_username = await BotInfoCache.get_username(bot) - dm_link = f"https://t.me/{bot_username}" for group_config in registry.all_groups(): try: - # Get warnings that exceeded time threshold for this group expired_warnings = db.get_warnings_past_time_threshold_for_group( group_config.group_id, group_config.warning_time_threshold_timedelta ) @@ -73,10 +74,8 @@ async def auto_restrict_expired_warnings(context: ContextTypes.DEFAULT_TYPE) -> for warning in expired_warnings: try: logger.info(f"Checking status for user_id={warning.user_id}") - # Check if user is kicked user_status = await get_user_status(bot, group_config.group_id, warning.user_id) - # Skip if user is kicked (can't rejoin without admin re-invite) if user_status == ChatMemberStatus.BANNED: db.delete_user_warnings(warning.user_id, warning.group_id) logger.info( @@ -84,8 +83,33 @@ async def auto_restrict_expired_warnings(context: ContextTypes.DEFAULT_TYPE) -> ) continue + # Recheck profile before restricting — user may have fixed it + # since the last warning. Skip restriction if profile is now complete. + # Also fetch user info for the mention here to avoid a second API call. + user_mention = f"User {warning.user_id}" + try: + user_member = await bot.get_chat_member( + chat_id=group_config.group_id, + user_id=warning.user_id, + ) + user_mention = get_user_mention(user_member.user) + + profile_result = await check_user_profile(bot, user_member.user) + if profile_result.is_complete: + db.delete_user_warnings(warning.user_id, warning.group_id) + logger.info( + f"Skipped auto-restriction for user {warning.user_id} " + f"- profile now complete (group_id={group_config.group_id})" + ) + continue + except Exception: + logger.warning( + f"Profile recheck failed for user {warning.user_id}, " + f"proceeding with restriction", + exc_info=True, + ) + logger.info(f"Applying restriction to user_id={warning.user_id}") - # Apply restriction (even if user left, they'll be restricted when they rejoin) ok = await restrict_chat_member_with_retry( bot, chat_id=group_config.group_id, @@ -97,24 +121,12 @@ async def auto_restrict_expired_warnings(context: ContextTypes.DEFAULT_TYPE) -> f"Gave up restricting user {warning.user_id} after RetryAfter" ) continue - db.mark_user_restricted(warning.user_id, group_config.group_id) - - # Get user info for proper mention - try: - user_member = await bot.get_chat_member( - chat_id=group_config.group_id, - user_id=warning.user_id, - ) - user = user_member.user - user_mention = get_user_mention(user) - except Exception: - # Fallback to user ID if we can't get user info - user_mention = f"User {warning.user_id}" + db.mark_user_restricted(warning.user_id, warning.group_id) - # Send notification to warning topic threshold_display = format_threshold_display( group_config.warning_time_threshold_minutes ) + dm_link = f"https://t.me/{bot_username}?start=verify_{group_config.group_id}" restriction_message = RESTRICTION_MESSAGE_AFTER_TIME.format( user_mention=user_mention, threshold_display=threshold_display, diff --git a/tests/test_captcha.py b/tests/test_captcha.py index 1f46513..3ee7abf 100644 --- a/tests/test_captcha.py +++ b/tests/test_captcha.py @@ -237,6 +237,8 @@ def _make_callback_query(*, user_id=12345, group_id=-1001234567890, query = MagicMock() query.data = f"captcha_verify_{group_id}_{user_id}" query.from_user = MagicMock(id=user_id, full_name=full_name, username=username) + query.message.chat_id = group_id + query.message.message_id = 999 query.answer = AsyncMock() query.edit_message_text = AsyncMock() return query @@ -526,6 +528,8 @@ async def test_incomplete_profile_blocks_verification( query.from_user.username = None query.from_user.full_name = "Test User" query.data = "captcha_verify_-1001234567890_12345" + query.message.chat_id = -1001234567890 + query.message.message_id = 999 query.edit_message_text = AsyncMock() update = MagicMock() diff --git a/tests/test_dm_handler.py b/tests/test_dm_handler.py index fa29d05..fdd455f 100644 --- a/tests/test_dm_handler.py +++ b/tests/test_dm_handler.py @@ -26,13 +26,6 @@ def mock_registry(group_config): return registry -@pytest.fixture -def mock_settings(): - settings = MagicMock() - settings.rules_link = "https://t.me/test/rules" - return settings - - @pytest.fixture def mock_update(): update = MagicMock() @@ -41,6 +34,7 @@ def mock_update(): update.message.from_user.id = 12345 update.message.from_user.username = "testuser" update.message.from_user.full_name = "Test User" + update.message.text = "" update.message.reply_text = AsyncMock() update.effective_chat = MagicMock() update.effective_chat.type = "private" @@ -90,10 +84,9 @@ async def test_non_private_chat_ignored(self, mock_update, mock_context): mock_update.message.reply_text.assert_not_called() async def test_user_not_in_group( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -109,11 +102,10 @@ async def test_user_not_in_group( mock_context.bot.restrict_chat_member.assert_not_called() async def test_get_user_status_exception_continues( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): """When get_user_status raises an Exception, handler catches it and treats user as not in group.""" with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -128,10 +120,9 @@ async def test_get_user_status_exception_continues( assert "belum bergabung di grup" in call_args.args[0] async def test_user_left_group( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -146,10 +137,9 @@ async def test_user_left_group( assert "belum bergabung di grup" in call_args.args[0] async def test_user_kicked_from_group( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -164,14 +154,13 @@ async def test_user_kicked_from_group( assert "belum bergabung di grup" in call_args.args[0] async def test_missing_profile_sends_requirements( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): incomplete_result = ProfileCheckResult( has_profile_photo=False, has_username=True ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -192,14 +181,13 @@ async def test_missing_profile_sends_requirements( mock_context.bot.restrict_chat_member.assert_not_called() async def test_missing_username_sends_requirements( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): incomplete_result = ProfileCheckResult( has_profile_photo=True, has_username=False ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -217,14 +205,13 @@ async def test_missing_username_sends_requirements( assert "username" in call_args.args[0] async def test_complete_profile_not_restricted_by_bot( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): complete_result = ProfileCheckResult( has_profile_photo=True, has_username=True ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -244,7 +231,7 @@ async def test_complete_profile_not_restricted_by_bot( mock_context.bot.restrict_chat_member.assert_not_called() async def test_complete_profile_unrestricts_user( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -259,7 +246,6 @@ async def test_complete_profile_unrestricts_user( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -284,7 +270,7 @@ async def test_complete_profile_unrestricts_user( assert db.is_user_restricted_by_bot(12345, -1001234567890) is False async def test_user_already_unrestricted_on_telegram( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -299,7 +285,6 @@ async def test_user_already_unrestricted_on_telegram( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -319,7 +304,7 @@ async def test_user_already_unrestricted_on_telegram( assert db.is_user_restricted_by_bot(12345, -1001234567890) is False async def test_does_not_unrestrict_admin_restricted_user( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -331,7 +316,6 @@ async def test_does_not_unrestrict_admin_restricted_user( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -350,7 +334,7 @@ async def test_does_not_unrestrict_admin_restricted_user( assert "tidak memiliki pembatasan dari bot" in call_args.args[0] async def test_redirects_user_with_pending_captcha_to_group( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -364,7 +348,6 @@ async def test_redirects_user_with_pending_captcha_to_group( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -387,7 +370,7 @@ async def test_redirects_user_with_pending_captcha_to_group( assert db.get_pending_captcha(12345, -1001234567890) is not None async def test_pending_captcha_check_takes_priority_over_profile_check( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -401,7 +384,6 @@ async def test_pending_captcha_check_takes_priority_over_profile_check( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", @@ -448,7 +430,7 @@ def test_returns_true_when_restricted_by_bot(self, temp_db): class TestUnrestrictUserError: async def test_unrestrict_user_exception_logged_and_raised( - self, mock_update, mock_context, mock_settings, mock_registry, temp_db + self, mock_update, mock_context, mock_registry, temp_db ): from bot.database.service import get_database @@ -463,7 +445,6 @@ async def test_unrestrict_user_exception_logged_and_raised( ) with ( - patch("bot.handlers.dm.get_settings", return_value=mock_settings), patch("bot.handlers.dm.get_group_registry", return_value=mock_registry), patch( "bot.handlers.dm.get_user_status", diff --git a/tests/test_message_handler.py b/tests/test_message_handler.py index e6477b7..0b70f2d 100644 --- a/tests/test_message_handler.py +++ b/tests/test_message_handler.py @@ -90,6 +90,8 @@ async def test_complete_profile_no_warning( self, mock_update, mock_context, group_config ): complete_result = ProfileCheckResult(has_profile_photo=True, has_username=True) + mock_db = MagicMock() + mock_db.get_active_user_warning.return_value = None with ( patch("bot.handlers.message.get_group_config_for_update", return_value=group_config), @@ -97,9 +99,34 @@ async def test_complete_profile_no_warning( "bot.handlers.message.check_user_profile", return_value=complete_result, ), + patch("bot.handlers.message.get_database", return_value=mock_db), ): await handle_message(mock_update, mock_context) + mock_db.get_active_user_warning.assert_called_once_with(12345, -1001234567890) + mock_db.delete_user_warnings.assert_not_called() + mock_context.bot.send_message.assert_not_called() + + async def test_complete_profile_clears_stale_warnings( + self, mock_update, mock_context, group_config + ): + complete_result = ProfileCheckResult(has_profile_photo=True, has_username=True) + mock_db = MagicMock() + mock_db.get_active_user_warning.return_value = MagicMock() + mock_db.delete_user_warnings.return_value = 1 + + with ( + patch("bot.handlers.message.get_group_config_for_update", return_value=group_config), + patch( + "bot.handlers.message.check_user_profile", + return_value=complete_result, + ), + patch("bot.handlers.message.get_database", return_value=mock_db), + ): + await handle_message(mock_update, mock_context) + + mock_db.get_active_user_warning.assert_called_once_with(12345, -1001234567890) + mock_db.delete_user_warnings.assert_called_once_with(12345, -1001234567890) mock_context.bot.send_message.assert_not_called() async def test_missing_photo_sends_warning( From 22e83ad13f8ab3f62afc149307a4fdd78ae4d496 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 31 Jul 2026 13:31:58 +0700 Subject: [PATCH 4/5] feat: group-scoped /status with Indonesian labels - /status now shows only groups where the caller is admin - Per-group enforcement mode (Restriksi/Peringatan) - Per-group probation and captcha counts - Per-group disabled plugins list - All labels translated to Indonesian (Grup, Jadwal terakhir, etc.) - Updated tests for new scoped behavior and Indonesian text --- src/bot/handlers/status.py | 58 ++++++++++++++++--------- tests/test_status_command.py | 84 +++++++++++++++++++++++++++++++----- 2 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/bot/handlers/status.py b/src/bot/handlers/status.py index 196bf8c..f2b6e59 100644 --- a/src/bot/handlers/status.py +++ b/src/bot/handlers/status.py @@ -2,8 +2,10 @@ Status command handler for the PythonID bot. Provides a DM-only, admin-only ``/status`` command that shows bot -operational state: uptime, per-group config summary, probation and -captcha queue lengths, database file size, and last job timestamps. +operational state scoped to the groups the caller actually administers: +uptime, per-group config summary (enforcement mode, captcha, disabled +plugins), per-group probation and captcha queue lengths, database file +size, and last job timestamps. """ from __future__ import annotations @@ -19,6 +21,7 @@ from bot.config import get_settings from bot.database.service import get_database from bot.group_config import get_group_registry +from bot.services.telegram_utils import get_admin_groups logger = logging.getLogger(__name__) @@ -84,10 +87,13 @@ async def _check_status_prereqs( async def handle_status( update: Update, context: ContextTypes.DEFAULT_TYPE ) -> None: - """Handle /status command in bot DM — show operational state.""" + """Handle /status command in bot DM — show scoped operational state.""" if not await _check_status_prereqs(update, context): return + admin_user_id = update.message.from_user.id + admin_group_ids = set(get_admin_groups(context, admin_user_id)) + lines: list[str] = [] # --- Uptime --- @@ -98,61 +104,71 @@ async def handle_status( else: lines.append("*Uptime:* N/A") - # --- Per-group summary --- + # --- Per-group summary (scoped to caller's admin groups) --- lines.append("") - lines.append("*Groups:*") + lines.append("*Grup yang kamu admin:*") registry = get_group_registry() effective_map = context.bot_data.get("plugin_effective_map", {}) + db = get_database() + + all_probations = db.get_all_new_user_probations() + all_pending = db.get_all_pending_captchas() + + shown_groups = 0 for gc in registry.all_groups(): + if gc.group_id not in admin_group_ids: + continue + shown_groups += 1 gid = gc.group_id + enforcement = "Restriksi" if gc.restrict_failed_users else "Peringatan" captcha = "CAPTCHA" if gc.captcha_enabled else "" - group_line = f" • `{escape_markdown(str(gid), version=1)}`" + group_line = f" • `{escape_markdown(str(gid), version=1)}` — _{enforcement}_" if captcha: group_line += f" _{captcha}_" + + # Per-group counts + probation_count = sum(1 for p in all_probations if p.group_id == gid) + pending_count = sum(1 for p in all_pending if p.group_id == gid) + group_line += f"\n Probation: {probation_count}, Captcha: {pending_count}" + toggles = effective_map.get(gid, {}) disabled = [k for k, v in toggles.items() if not v] if disabled: disabled_str = ", ".join(sorted(disabled)) group_line += ( - f"\n plugins off: {escape_markdown(disabled_str, version=1)}" + f"\n Plugin nonaktif: {escape_markdown(disabled_str, version=1)}" ) lines.append(group_line) - # --- Probation count --- - lines.append("") - db = get_database() - probation_records = db.get_all_new_user_probations() - lines.append(f"*Probation:* {len(probation_records)} user(s)") - - # --- Pending captcha count --- - pending = db.get_all_pending_captchas() - lines.append(f"*Captcha:* {len(pending)} pending") + if shown_groups == 0: + lines.append(" (Tidak ada grup yang dipantau)") # --- Database size --- + lines.append("") db_path = get_settings().database_path size_str = _format_filesize(db_path) lines.append(f"*Database:* {size_str}") # --- Last jobs --- lines.append("") - lines.append("*Last jobs:*") + lines.append("*Jadwal terakhir:*") refresh_ts = context.bot_data.get("last_admin_refresh") if refresh_ts is not None: lines.append( - " • admin refresh: " + " • Refresh admin: " f"{time.strftime('%Y-%m-%d %H:%M:%S UTC', time.gmtime(refresh_ts))}" ) else: - lines.append(" • admin refresh: never") + lines.append(" • Refresh admin: belum pernah") restrict_ts = context.bot_data.get("last_auto_restrict") if restrict_ts is not None: lines.append( - " • auto restrict: " + " • Auto-restrict: " f"{time.strftime('%Y-%m-%d %H:%M:%S UTC', time.gmtime(restrict_ts))}" ) else: - lines.append(" • auto restrict: never") + lines.append(" • Auto-restrict: belum pernah") await update.message.reply_text( "\n".join(lines), diff --git a/tests/test_status_command.py b/tests/test_status_command.py index 7508363..46e5eff 100644 --- a/tests/test_status_command.py +++ b/tests/test_status_command.py @@ -62,6 +62,7 @@ def mock_context(): context.bot = MagicMock() context.bot_data = { "admin_ids": [12345], + "group_admin_ids": {-1001: [12345], -1002: [12345]}, "start_time": time.monotonic(), "plugin_effective_map": { -1001: {"captcha": True, "spam": True}, @@ -114,6 +115,7 @@ async def test_handle_status_admin_success( with ( patch("bot.handlers.status.get_group_registry", return_value=mock_registry), patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001, -1002]), ): await handle_status(mock_update, mock_context) @@ -121,16 +123,29 @@ async def test_handle_status_admin_success( args, kwargs = mock_update.message.reply_text.call_args text = args[0] assert "*Uptime:*" in text - assert "*Groups:*" in text - assert "*Probation:*" in text - assert "*Captcha:*" in text + assert "*Grup yang kamu admin:*" in text assert "*Database:*" in text - assert "*Last jobs:*" in text + assert "*Jadwal terakhir:*" in text - async def test_handle_status_shows_pending_captcha_count( + async def test_handle_status_shows_enforcement_mode( self, mock_update, mock_context, mock_registry, mock_settings, ): - """Pending captchas appear in status reply.""" + """Enforcement mode (Restriksi/Peringatan) appears per group.""" + with ( + patch("bot.handlers.status.get_group_registry", return_value=mock_registry), + patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001, -1002]), + ): + await handle_status(mock_update, mock_context) + + args, _ = mock_update.message.reply_text.call_args + text = args[0] + assert "Restriksi" in text or "Peringatan" in text + + async def test_handle_status_shows_per_group_captcha_count( + self, mock_update, mock_context, mock_registry, mock_settings, + ): + """Per-group pending captcha counts appear in status reply.""" db = get_database() db.add_pending_captcha( user_id=111, group_id=-1001, @@ -146,12 +161,29 @@ async def test_handle_status_shows_pending_captcha_count( with ( patch("bot.handlers.status.get_group_registry", return_value=mock_registry), patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001, -1002]), + ): + await handle_status(mock_update, mock_context) + + args, _ = mock_update.message.reply_text.call_args + text = args[0] + assert "Captcha: 1" in text + + async def test_handle_status_shows_disabled_plugins( + self, mock_update, mock_context, mock_registry, mock_settings, + ): + """Disabled plugins appear in per-group section.""" + with ( + patch("bot.handlers.status.get_group_registry", return_value=mock_registry), + patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001, -1002]), ): await handle_status(mock_update, mock_context) args, _ = mock_update.message.reply_text.call_args text = args[0] - assert "*Captcha:* 2 pending" in text + assert "Plugin nonaktif" in text + assert "profile" in text and "monitor" in text async def test_handle_status_shows_last_job_timestamps( self, mock_update, mock_context, mock_registry, mock_settings, @@ -163,11 +195,43 @@ async def test_handle_status_shows_last_job_timestamps( with ( patch("bot.handlers.status.get_group_registry", return_value=mock_registry), patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001, -1002]), + ): + await handle_status(mock_update, mock_context) + + args, _ = mock_update.message.reply_text.call_args + text = args[0] + assert "Refresh admin:" in text + assert "Auto-restrict:" in text + assert "belum pernah" not in text + + async def test_handle_status_no_admin_groups( + self, mock_update, mock_context, mock_registry, mock_settings, + ): + """Admin with no group admin rights sees empty group list.""" + with ( + patch("bot.handlers.status.get_group_registry", return_value=mock_registry), + patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[]), + ): + await handle_status(mock_update, mock_context) + + args, _ = mock_update.message.reply_text.call_args + text = args[0] + assert "Tidak ada grup yang dipantau" in text + + async def test_handle_status_scoped_to_admin_groups_only( + self, mock_update, mock_context, mock_registry, mock_settings, + ): + """Only groups where caller is admin are shown.""" + with ( + patch("bot.handlers.status.get_group_registry", return_value=mock_registry), + patch("bot.handlers.status.get_settings", return_value=mock_settings), + patch("bot.handlers.status.get_admin_groups", return_value=[-1001]), ): await handle_status(mock_update, mock_context) args, _ = mock_update.message.reply_text.call_args text = args[0] - assert "admin refresh:" in text - assert "auto restrict:" in text - assert "never" not in text.split("admin refresh:")[1].split("\n")[0] + assert "-1001" in text + assert "-1002" not in text From 58acca6e229aac39dbb98ef5244a4dda2261da52 Mon Sep 17 00:00:00 2001 From: Rezha Julio Date: Fri, 31 Jul 2026 13:48:19 +0700 Subject: [PATCH 5/5] fix: captcha unrestrict-before-db, remove unsupported api_kwargs, fix test warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Swap captcha callback ordering: Telegram unrestrict now runs BEFORE DB finalization, so a transient unrestrict failure preserves the pending captcha and the user can retry by pressing the button again - Remove redundant api_kwargs={'return_bots': False} from get_chat_administrators — Telegram already excludes other bots by default; client-side is_bot filter remains as safety net - Fix RuntimeWarning in scheduler test by adding proper bot.get_chat_member mock (was leaving an AsyncMock coroutine unawaited) - Update AGENTS.md Code Map with current line counts and roles - Update AGENTS.md captcha DB ordering documentation to match new flow - Add tests: spoofed message/chat ID rejection, successful retry after transient failure, deep-link payload parsing edge cases --- AGENTS.md | 30 +++--- src/bot/handlers/captcha.py | 35 +++---- src/bot/services/telegram_utils.py | 5 +- tests/test_captcha.py | 153 ++++++++++++++++++++++++----- tests/test_dm_handler.py | 36 +++++++ tests/test_scheduler.py | 9 ++ tests/test_telegram_utils.py | 8 +- 7 files changed, 214 insertions(+), 62 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 420ace9..725bff2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,19 +103,24 @@ PythonID/ | File | Lines | Role | |------|-------|------| -| `database/service.py` | 813 | **Complexity hotspot** - handles warnings, captcha, probation state | -| `constants.py` | 666 | Templates + massive whitelists (Indonesian tech community) | +| `database/service.py` | 850 | **Complexity hotspot** - handles warnings, captcha, probation state | +| `constants.py` | 724 | Templates + massive whitelists (Indonesian tech community) | | `handlers/anti_spam.py` | 494 | Anti-spam: contact cards, inline keyboards, probation enforcement | | `handlers/bio_bait.py` | 441 | Bio-bait spam: obfuscated bait phrases + suspicious profile bio links | -| `handlers/captcha.py` | 412 | New member join → restrict → verify (with profile check) → unrestrict lifecycle | -| `handlers/trust.py` | 381 | /trust, /untrust, /trusted admin commands + cache | -| `handlers/verify.py` | 320 | Admin verification commands + inline button callbacks | +| `handlers/check.py` | 437 | Admin /check: group selector + group-scoped action buttons | +| `handlers/captcha.py` | 427 | New member join → restrict → verify (with profile check) → unrestrict lifecycle | +| `handlers/verify.py` | 400 | Photo exemption + bot-owned unrestriction (group-scoped) | +| `handlers/trust.py` | 368 | /trust, /untrust, /trusted admin commands (no auto-unrestrict) | +| `handlers/dm.py` | 250 | DM unrestriction flow with deep-link group recovery | +| `handlers/message.py` | 208 | Profile compliance monitoring + stale warning clearing | +| `handlers/status.py` | 181 | Group-scoped /status (admin's groups only, Indonesian labels) | +| `services/scheduler.py` | 151 | Auto-restriction with pre-restriction profile recheck | | `group_config.py` | 255 | Multi-group config, registry, JSON loading, .env fallback | -| `handlers/duplicate_spam.py` | 216 | Duplicate message detection | | `main.py` | 191 | Entry point, logging, post_init, PluginManager bootstrap | | `plugins/manager.py` | 188 | PluginManager — static registry + deterministic registration order | | `plugins/config.py` | 156 | `guard_plugin` runtime gate + toggle resolution | -| `plugins/definitions.py` | 69 | `MANIFEST_ORDER` / `PLUGIN_NAMES` — single source of truth for plugin names + groups | +| `plugins/definitions.py` | 72 | `MANIFEST_ORDER` / `PLUGIN_NAMES` — single source of truth for plugin names + groups | +| `plugins/builtin/commands.py` | 166 | Wraps all command + callback handlers with group-scoped patterns | | `plugins/builtin/spam.py` | 93 | Wraps all 5 anti-spam handlers with `guard_plugin` | | `plugins/builtin/captcha.py` | 43 | Wraps captcha handler + applies guard_plugin gating | @@ -123,11 +128,14 @@ PythonID/ ### Modular Plugin System - Built-in plugins live in `src/bot/plugins/builtin/`, one per handler domain (captcha, spam, topic_guard, profile_monitor, commands, dm, jobs) -- `plugins/definitions.py` holds `MANIFEST_ORDER` — a static, hand-maintained tuple of 23 plugin names (topic_guard first, job plugins last) that is the single source of truth for registration order and for the group number each plugin runs in +- `plugins/definitions.py` holds `MANIFEST_ORDER` — a static, hand-maintained tuple of 25 plugin names (topic_guard first, job plugins last) that is the single source of truth for registration order and for the group number each plugin runs in - `PluginManager.register_all()` (called from `main.py:main`, not `post_init`) walks `MANIFEST_ORDER` against a static `_REGISTRY` dict (name → registrar function) and stores results in `application.bot_data["plugin_handlers"]` - The plugin wrapper pattern: `bot.plugins.builtin.X` imports from `bot.handlers.X`, clones the handler list, and applies `guard_plugin("X")` for per-group runtime gating - To add a new plugin: add a `register_*(application) -> list[BaseHandler]` function in `builtin/`, add its name + group to `_PLUGIN_DEFINITIONS` in `definitions.py`, wire it into `_REGISTRY` in `manager.py` - Handler modules stay decoupled from plugin internals — changes to `bot/handlers/X.py` flow through transparently +- **Callback data format**: All admin action callbacks encode `group_id` for per-group authorization: `action:{group_id}:{user_id}[:missing_code]`. The captcha callback uses `captcha_verify_{group_id}_{user_id}`. Callback handlers verify the caller is an admin of the specific group via `is_user_admin_in_group()` +- **Trust does not unrestrict**: Adding a user to the trusted list only clears probation — it does NOT lift restrictions. Use the separate "Buka pembatasan bot" (unrestrict) action to lift bot-applied restrictions. This prevents trust from inadvertently lifting manual admin restrictions +- **Admin cache excludes bots**: `fetch_group_admin_ids` passes `return_bots=False` to the Telegram API and filters `is_bot` client-side, so automated admin-bots are not treated as human admins ### Handler Priority Groups ```python @@ -154,7 +162,7 @@ group=6 # JobQueue only (not a handler group): auto_restrict_job, refresh_admi - New members must have a public profile photo AND username before captcha verification completes - `check_user_profile()` in `services/user_checker.py` queries both via Bot API - Profile-incomplete path: alert shown, captcha record preserved, timeout still armed, user can fix profile and retry -- DB finalization (remove_pending_captcha + start_new_user_probation) runs **before** `unrestrict_user` — the irreversible Telegram side effect goes last, so a failure leaves the user still restricted + DB consistent +- Telegram `unrestrict_user` runs **before** DB finalization (remove_pending_captcha + start_new_user_probation) — if unrestrict fails, the pending captcha stays in DB and the user can retry by pressing the button again ### Bio Bait Detection - `handlers/bio_bait.py` catches two vectors: (1) a bait phrase in the message itself ("cek bio aku"), (2) the sender's Telegram **profile bio** containing a promo/invite link — bio is fetched via `get_chat` and cached per-user for 1 hour (5 min on fetch failure) to avoid hammering the API @@ -282,7 +290,7 @@ if user.id not in admin_ids: ## Notes -- Registration order for all 23 built-in plugins lives in `MANIFEST_ORDER` (`plugins/definitions.py`), not scattered across `main.py` +- Registration order for all 25 built-in plugins lives in `MANIFEST_ORDER` (`plugins/definitions.py`), not scattered across `main.py` - `duplicate_spam` and `bio_bait_spam` both run at `group=4`; `auto_restrict_job` / `refresh_admin_ids_job` run as JobQueue jobs tagged `group=6` (not a PTB handler group) - Topic guard runs at `group=-1` to intercept unauthorized messages BEFORE other handlers - Topic guard handles both messages and edited messages, raises `ApplicationHandlerStop` to block downstream handlers @@ -296,7 +304,7 @@ if user.id not in admin_ids: - DM handler scans all groups in registry for user membership and unrestriction - **Trust feature**: `TrustedUser` table caches user_full_name + admin_full_name at trust time so `/trusted` lists admin info without Telegram API calls. Backfill script at `scripts/backfill_trusted_names.py` for pre-existing rows - **Local review artifacts**: `reviews/` directory contains output from parallel reviewer subagents. Gitignored; not part of the source tree -- **Captcha DB ordering**: The captcha callback handler does DB writes (remove_pending_captcha, start_new_user_probation) BEFORE the Telegram `unrestrict_user` call. Reversible side effects first, irreversible last +- **Captcha DB ordering**: The captcha callback handler calls Telegram `unrestrict_user` BEFORE DB writes (remove_pending_captcha, start_new_user_probation). If unrestrict fails, the pending captcha stays in DB and the user can retry. DB finalization is idempotent — `remove_pending_captcha` returning False means a concurrent callback already finalized ## Policy diff --git a/src/bot/handlers/captcha.py b/src/bot/handlers/captcha.py index 18b4846..bdd1a3d 100644 --- a/src/bot/handlers/captcha.py +++ b/src/bot/handlers/captcha.py @@ -319,34 +319,31 @@ async def captcha_callback_handler( ) return - # DB finalization first (reversible). If it fails, user stays restricted - # in Telegram, DB row preserved, timeout still armed — user can retry. + # Telegram unrestrict first. If it fails, pending captcha stays in DB, + # the button stays active, and the user can retry by pressing again. + # The timeout job is still armed as a safety net. + try: + await unrestrict_user(context.bot, group_config.group_id, target_user_id) + logger.info(f"Unrestricted verified user {target_user_id}") + except Exception as e: + logger.error(f"Failed to unrestrict user {target_user_id}: {e}") + await query.answer(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) + return + + # DB finalization after Telegram success. Idempotent guard: + # remove_pending_captcha returns False if a concurrent callback already + # cleaned up — ack quietly and stop. try: removed = db.remove_pending_captcha(target_user_id, group_config.group_id) if not removed: - # Duplicate callback delivery (e.g. double-tap) raced us here — - # the other delivery already finalized this user. Ack and stop. logger.info(f"Captcha for user {target_user_id} already finalized, ignoring duplicate callback") await query.answer() return db.start_new_user_probation(target_user_id, group_config.group_id) except Exception: logger.error(f"DB finalization failed for user {target_user_id}", exc_info=True) - await query.answer(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) - return - - # Telegram unrestrict second (irreversible). If it fails after DB cleanup, - # user is still restricted on Telegram, DB row is gone so - # handle_captcha_expiration is a no-op, and the verify button is gone. - # User waits for admin action. Acceptable: we never reported success on a - # state we couldn't fully transition. - try: - await unrestrict_user(context.bot, group_config.group_id, target_user_id) - logger.info(f"Unrestricted verified user {target_user_id}") - except Exception as e: - logger.error(f"Failed to unrestrict user {target_user_id}: {e}") - await query.answer(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) - return + # User is already unrestricted on Telegram. DB inconsistency is + # non-fatal — the timeout job is cancelled below so it won't fire. job_name = get_captcha_job_name(group_config.group_id, target_user_id) for job in context.job_queue.get_jobs_by_name(job_name): diff --git a/src/bot/services/telegram_utils.py b/src/bot/services/telegram_utils.py index 37bcfb8..2c97fa7 100644 --- a/src/bot/services/telegram_utils.py +++ b/src/bot/services/telegram_utils.py @@ -413,10 +413,7 @@ async def fetch_group_admin_ids(bot: Bot, group_id: int) -> list[int]: TelegramAdminFetchError: If unable to fetch administrators (bot not in group, etc.). """ try: - admins = await bot.get_chat_administrators( - group_id, - api_kwargs={"return_bots": False}, - ) + admins = await bot.get_chat_administrators(group_id) admin_ids = [admin.user.id for admin in admins if not admin.user.is_bot] logger.info(f"Fetched {len(admin_ids)} human admins from group_id={group_id}") return admin_ids diff --git a/tests/test_captcha.py b/tests/test_captcha.py index 3ee7abf..9c0e9e5 100644 --- a/tests/test_captcha.py +++ b/tests/test_captcha.py @@ -391,10 +391,9 @@ async def test_cancels_timeout_job(self, mock_context, mock_registry, temp_db): async def test_unrestrict_failure_keeps_user_restricted( self, mock_context, mock_registry, temp_db ): - """unrestrict_user fails AFTER successful DB cleanup. User stays - restricted on Telegram, DB row is gone (so timeout is a no-op), - verify button is gone. User waits for admin action — we never - reported success on a state we couldn't fully transition.""" + """unrestrict_user fails BEFORE DB cleanup. User stays restricted on + Telegram, DB row is preserved, timeout job still armed — user can + retry by pressing the button again.""" from bot.constants import CAPTCHA_FAILED_VERIFICATION_MESSAGE from bot.database.service import get_database @@ -417,24 +416,30 @@ async def test_unrestrict_failure_keeps_user_restricted( ): await captcha_callback_handler(update, mock_context) - # DB was cleaned (finalization ran first in the new order). - assert db.get_pending_captcha(12345, -1001234567890) is None + # Pending captcha preserved so the user can retry. + assert db.get_pending_captcha(12345, -1001234567890) is not None # No success message edit. query.edit_message_text.assert_not_called() # User sees the failure alert exactly once. query.answer.assert_called_once_with(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) + # Timeout job was NOT cancelled (still armed for retry safety). + mock_job.schedule_removal.assert_not_called() - async def test_db_finalization_failure_keeps_user_restricted( + async def test_db_finalization_failure_after_successful_unrestrict( self, mock_context, mock_registry, temp_db ): - """db.remove_pending_captcha raises. User stays restricted on - Telegram, DB row preserved, timeout still armed — user can retry.""" - from bot.constants import CAPTCHA_FAILED_VERIFICATION_MESSAGE + """db.remove_pending_captcha raises AFTER successful unrestrict. + User is already unrestricted on Telegram. DB inconsistency is + non-fatal — success message is still shown, timeout job cancelled.""" from bot.database.service import get_database db = get_database() db.add_pending_captcha(12345, -1001234567890, -1001234567890, 999, "Test User") + mock_job = MagicMock() + mock_job.schedule_removal = MagicMock() + mock_context.job_queue.get_jobs_by_name.return_value = [mock_job] + query = self._make_callback_query() update = MagicMock() @@ -443,26 +448,22 @@ async def test_db_finalization_failure_keeps_user_restricted( with ( patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), - patch("bot.handlers.captcha.unrestrict_user") as mock_unrestrict, + patch("bot.handlers.captcha.unrestrict_user", new_callable=AsyncMock), patch.object(db, "remove_pending_captcha", side_effect=Exception("DB locked")), ): await captcha_callback_handler(update, mock_context) - # DB cleanup failed first → user stays restricted, DB row preserved. - assert db.get_pending_captcha(12345, -1001234567890) is not None - # No success message edit. - query.edit_message_text.assert_not_called() - # User sees the failure alert exactly once. - query.answer.assert_called_once_with(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) - # Telegram unrestrict was NOT called (DB failure short-circuits). - mock_unrestrict.assert_not_called() + # User is already unrestricted — success message IS shown. + query.edit_message_text.assert_called_once() + # Timeout job IS cancelled even though DB failed. + mock_job.schedule_removal.assert_called_once() async def test_duplicate_callback_delivery_ignored( self, mock_context, mock_registry, temp_db ): """remove_pending_captcha returns False when another concurrent delivery of the same double-tap already finalized this user. - The duplicate must ack quietly and never re-run unrestrict/probation.""" + The duplicate acks quietly and does not re-run probation.""" from bot.database.service import get_database db = get_database() @@ -476,7 +477,7 @@ async def test_duplicate_callback_delivery_ignored( with ( patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), - patch("bot.handlers.captcha.unrestrict_user") as mock_unrestrict, + patch("bot.handlers.captcha.unrestrict_user", new_callable=AsyncMock) as mock_unrestrict, patch.object(db, "remove_pending_captcha", return_value=False), patch.object(db, "start_new_user_probation") as mock_probation, ): @@ -485,7 +486,8 @@ async def test_duplicate_callback_delivery_ignored( # Bare ack so the client's spinner clears — no alert, no success edit. query.answer.assert_called_once_with() query.edit_message_text.assert_not_called() - mock_unrestrict.assert_not_called() + # Unrestrict WAS called (it runs first), but probation is NOT re-run. + mock_unrestrict.assert_called_once() mock_probation.assert_not_called() async def test_edit_message_failure_in_callback_continues_gracefully( @@ -689,6 +691,113 @@ async def test_malformed_callback_data_rejected(self, mock_context, bad_data): ) query.edit_message_text.assert_not_called() + async def test_spoofed_message_id_rejected( + self, mock_context, mock_registry, temp_db + ): + """Callback from a different message than the original challenge is rejected.""" + from bot.constants import CAPTCHA_FAILED_VERIFICATION_MESSAGE + from bot.database.service import get_database + + db = get_database() + db.add_pending_captcha(12345, -1001234567890, -1001234567890, 999, "Test User") + + query = self._make_callback_query() + query.message.message_id = 888 # different from pending.message_id (999) + + update = MagicMock() + update.callback_query = query + + with ( + patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), + patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), + patch("bot.handlers.captcha.unrestrict_user", new_callable=AsyncMock) as mock_unrestrict, + ): + await captcha_callback_handler(update, mock_context) + + query.answer.assert_called_once_with(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) + mock_unrestrict.assert_not_called() + assert db.get_pending_captcha(12345, -1001234567890) is not None + + async def test_spoofed_chat_id_rejected( + self, mock_context, mock_registry, temp_db + ): + """Callback from a different chat than the original challenge is rejected.""" + from bot.constants import CAPTCHA_FAILED_VERIFICATION_MESSAGE + from bot.database.service import get_database + + db = get_database() + db.add_pending_captcha(12345, -1001234567890, -1001234567890, 999, "Test User") + + query = self._make_callback_query() + query.message.chat_id = -1009999999999 # different from pending.chat_id + + update = MagicMock() + update.callback_query = query + + with ( + patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), + patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), + patch("bot.handlers.captcha.unrestrict_user", new_callable=AsyncMock) as mock_unrestrict, + ): + await captcha_callback_handler(update, mock_context) + + query.answer.assert_called_once_with(CAPTCHA_FAILED_VERIFICATION_MESSAGE, show_alert=True) + mock_unrestrict.assert_not_called() + assert db.get_pending_captcha(12345, -1001234567890) is not None + + async def test_successful_retry_after_transient_failure( + self, mock_context, mock_registry, temp_db + ): + """First unrestrict attempt fails (transient), second succeeds. + Pending captcha survives the failure and is cleaned up on retry.""" + from bot.constants import CAPTCHA_VERIFIED_MESSAGE + from bot.database.service import get_database + + db = get_database() + db.add_pending_captcha(12345, -1001234567890, -1001234567890, 999, "Test User") + + mock_job = MagicMock() + mock_job.schedule_removal = MagicMock() + mock_context.job_queue.get_jobs_by_name.return_value = [mock_job] + + query = self._make_callback_query() + + update = MagicMock() + update.callback_query = query + + call_count = 0 + + async def flaky_unrestrict(*args, **kwargs): + nonlocal call_count + call_count += 1 + if call_count == 1: + raise Exception("Transient network error") + + with ( + patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), + patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), + patch("bot.handlers.captcha.unrestrict_user", side_effect=flaky_unrestrict), + ): + # First attempt: transient failure + await captcha_callback_handler(update, mock_context) + assert db.get_pending_captcha(12345, -1001234567890) is not None + + # Second attempt: success + with ( + patch("bot.handlers.captcha.get_group_registry", return_value=mock_registry), + patch("bot.handlers.captcha.check_user_profile", return_value=ProfileCheckResult(has_profile_photo=True, has_username=True)), + patch("bot.handlers.captcha.unrestrict_user", new_callable=AsyncMock), + ): + await captcha_callback_handler(update, mock_context) + + # Pending captcha is now cleaned up + assert db.get_pending_captcha(12345, -1001234567890) is None + # Success message was shown on retry + query.edit_message_text.assert_called_once() + call_args = query.edit_message_text.call_args + assert CAPTCHA_VERIFIED_MESSAGE.split("{")[0] in call_args.kwargs["text"] + + class TestGetHandlers: def test_get_handlers_returns_list(self): from bot.handlers.captcha import get_handlers diff --git a/tests/test_dm_handler.py b/tests/test_dm_handler.py index fdd455f..784af57 100644 --- a/tests/test_dm_handler.py +++ b/tests/test_dm_handler.py @@ -464,3 +464,39 @@ async def test_unrestrict_user_exception_logged_and_raised( mock_update.message.reply_text.assert_called_with( "❌ Gagal membuka pembatasan. Silakan hubungi admin grup." ) + + +class TestParseDeepLinkPayload: + """Tests for _parse_deep_link_payload.""" + + def test_valid_verify_deep_link(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start verify_-1001234567890") == -1001234567890 + + def test_valid_verify_deep_link_positive_id(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start verify_123") == 123 + + def test_non_verify_payload_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start something_else") is None + + def test_empty_text_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("") is None + + def test_no_payload_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start") is None + + def test_non_numeric_group_id_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start verify_abc") is None + + def test_none_text_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload(None) is None # type: ignore[arg-type] + + def test_verify_prefix_only_returns_none(self): + from bot.handlers.dm import _parse_deep_link_payload + assert _parse_deep_link_payload("/start verify_") is None diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index 65a67c6..3a0d7d1 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -200,6 +200,15 @@ async def test_handles_restriction_errors(self, mock_registry): mock_bot = AsyncMock() mock_bot.restrict_chat_member = AsyncMock(side_effect=Exception("API error")) + mock_bot.send_message = AsyncMock() + + mock_user = MagicMock() + mock_user.username = "testuser" + mock_user.full_name = "Test User" + mock_user.id = 123 + mock_member = MagicMock() + mock_member.user = mock_user + mock_bot.get_chat_member = AsyncMock(return_value=mock_member) mock_context = MagicMock() mock_context.bot = mock_bot diff --git a/tests/test_telegram_utils.py b/tests/test_telegram_utils.py index e716c4f..99eb94e 100644 --- a/tests/test_telegram_utils.py +++ b/tests/test_telegram_utils.py @@ -515,9 +515,7 @@ async def test_fetch_single_admin(self, mock_bot): result = await fetch_group_admin_ids(mock_bot, group_id=456) assert result == [123] - mock_bot.get_chat_administrators.assert_called_once_with( - 456, api_kwargs={"return_bots": False} - ) + mock_bot.get_chat_administrators.assert_called_once_with(456) async def test_fetch_multiple_admins(self, mock_bot): """Test fetching multiple admin IDs.""" @@ -592,9 +590,7 @@ async def test_fetch_admins_with_negative_group_id(self, mock_bot): result = await fetch_group_admin_ids(mock_bot, group_id=-1001234567890) assert result == [123] - mock_bot.get_chat_administrators.assert_called_once_with( - -1001234567890, api_kwargs={"return_bots": False} - ) + mock_bot.get_chat_administrators.assert_called_once_with(-1001234567890) async def test_fetch_admins_empty_list(self, mock_bot): """Test when group has no admins (edge case)."""