From cf9f833bc0c1cec7397daffbb19966b5dd778a2a Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 18:31:40 -0700 Subject: [PATCH 01/18] Add blocklist blocklist.py contains the initial classes and code for the new blocklist system. The intention of this system is to unify blocking to one new mongodb collection and vastly simplify bot code required to manage it. Additionally, most simply methods should be side effect free. --- core/blocklist.py | 134 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 core/blocklist.py diff --git a/core/blocklist.py b/core/blocklist.py new file mode 100644 index 0000000000..379e60afff --- /dev/null +++ b/core/blocklist.py @@ -0,0 +1,134 @@ +import datetime +import enum +from dataclasses import dataclass +from typing import Optional, Self, Tuple + +import discord +import isodate +from bson import CodecOptions, ObjectId +from motor.core import AgnosticCollection + + +class BlockType(enum.IntEnum): + USER = 0 + ROLE = 1 + + +class BlockReason(enum.StrEnum): + GUILD_AGE = "guild_age" + ACCOUNT_AGE = "account_age" + BLOCKED_ROLE = "blocked_role" + BLOCKED_USER = "blocked_user" + + +@dataclass(frozen=True) +class BlockedUser: + # _id: ObjectId + # May be role or user id + id: int + expires_at: Optional[datetime.datetime] + reason: str + timestamp: datetime.datetime + blocking_user_id: int + # specifies if the id is a role or user id + type: BlockType + + +class Blocklist: + blocklist_collection: AgnosticCollection + + def __init__(self: Self, bot) -> None: + self.blocklist_collection = bot.api.db.blocklist.with_options( + codec_options=CodecOptions(tz_aware=True, tzinfo=datetime.timezone.utc)) + self.bot = bot + + async def setup(self): + index_info = await self.blocklist_collection.index_information() + print(index_info) + await self.blocklist_collection.create_index("id") + await self.blocklist_collection.create_index("expires_at", expireAfterSeconds=0) + + async def block_user(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, + blocked_by: int) -> None: + now = datetime.datetime.utcnow() + + await self.blocklist_collection.insert_one(BlockedUser( + id=user_id, + expires_at=expires_at, + reason=reason, + timestamp=now, + blocking_user_id=blocked_by, + type=BlockType.USER + )) + + # we will probably want to cache these + async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[BlockReason]]: + """ + Side effect free version of is_blocked + + Parameters + ---------- + member + + Returns + ------- + True if the user is blocked + """ + # + + if str(member.id) in self.bot.blocked_whitelisted_users: + return False + + blocked = await self.blocklist_collection.find_one({"id": member.id}) + if blocked is not None: + print(blocked) + return True, BlockReason.BLOCKED_USER + + roles = member.roles + + blocked = await self.blocklist_collection.find_one(filter={"id": {"$in": [r.id for r in roles]}}) + if blocked is not None: + return True, BlockReason.BLOCKED_ROLE + + if not self.is_valid_account_age(member): + print("account_age") + return True, BlockReason.ACCOUNT_AGE + if not self.is_valid_guild_age(member): + print("guild_age") + return True, BlockReason.GUILD_AGE + + return False, None + + def is_valid_account_age(self, author: discord.User) -> bool: + account_age = self.bot.config.get("account_age") + + if account_age is None or account_age == isodate.Duration(): + return True + + now = discord.utils.utcnow() + + min_account_age = author.created_at + account_age + + if min_account_age < now: + # User account has not reached the required time + return False + return True + + def is_valid_guild_age(self, author: discord.Member) -> bool: + guild_age = self.bot.config.get("guild_age") + + if guild_age is None or guild_age == isodate.Duration(): + return True + + now = discord.utils.utcnow() + + if not hasattr(author, "joined_at"): + self.bot.logger.warning("Not in guild, cannot verify guild_age, %s.", author.name) + return False + + min_guild_age = author.joined_at + guild_age + + if min_guild_age > now: + # User has not stayed in the guild for long enough + return False + return True From da788b0e6133edc8a6b6d6f518890688eafd6d64 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 18:33:02 -0700 Subject: [PATCH 02/18] Add migrations.py migrations.py contains functions to migrate legacy blocklists to the new blocklist. contains handling for both legacy modmail string and the newer dict storage --- core/migrations.py | 121 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 core/migrations.py diff --git a/core/migrations.py b/core/migrations.py new file mode 100644 index 0000000000..12cd878978 --- /dev/null +++ b/core/migrations.py @@ -0,0 +1,121 @@ +import datetime +import re + +from core import blocklist +from core.models import getLogger + +logger = getLogger(__name__) + +old_format_matcher = re.compile("by ([\w]*#[\d]{1,4})(?: until )?.") + + +def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlockedUser: + blocked_by = int(v["blocked_by"]) + if "until" in v: + # todo make sure this is the correct from format + blocked_until = datetime.datetime.fromisoformat(v["until"]) + else: + blocked_until = None + if "reason" in v: + reason = v["reason"] + else: + reason = None + blocked_ts = datetime.datetime.fromisoformat(v["blocked_at"]) + return blocklist.BlockedUser( + id=int(k), + expires_at=blocked_until, + reason=reason, + timestamp=blocked_ts, + blocking_user_id=blocked_by, + type=block_type + ) + + +def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlockedUser: + match = old_format_matcher.match(v) + # blocked_by = match.group(1) + blocked_until = match.group(2) + if blocked_until is not None: + blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) + + return blocklist.BlockedUser( + id=int(k), + expires_at=blocked_until, + reason=f"migrated from old format `{v}`", + timestamp=datetime.datetime.utcnow(), + # I'm not bothering to fetch the user object here, discords username migrations will have broken all of them + blocking_user_id=0, + type=block_type + ) + + +async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlockedUser], + block_type: blocklist.BlockType, bot) -> int: + skipped = 0 + + for k, v in foo.items(): + # handle new block format + if type(v) is dict: + block = _convert_legacy_v2_block_format(k, v, block_type=block_type) + if block.expires_at is not None and block.expires_at < datetime.datetime.now(datetime.timezone.utc): + logger.debug("skipping expired block entry") + skipped += 1 + continue + logger.debug(f"migrating new format {v}") + blocklist_batch.append(block) + + continue + + match = old_format_matcher.match(v) + blocked_by = match.group(1) + blocked_until = match.group(2) + if blocked_until is not None: + blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) + + # skip this item if blocked_until occurred in the past + if blocked_until is not None and blocked_until < datetime.datetime.now(datetime.timezone.utc): + logger.debug("skipping expired block entry") + skipped += 1 + continue + + logger.debug(f"migrating id:{k} ts:{blocked_until} blocker:{blocked_by}") + + blocklist_batch.append(blocklist.BlockedUser( + id=int(k), + expires_at=blocked_until, + reason=f"migrated from old format `{v}`", + timestamp=datetime.datetime.utcnow(), + # I'm not bothering to fetch the user object here, discords username migrations will have broken all of them + blocking_user_id=0, + type=blocklist.BlockType.USER + )) + if len(blocklist_batch) >= 100: + await bot.api.db.blocklist.insert_many([x.__dict__ for x in blocklist_batch]) + blocklist_batch.clear() + + return skipped + + +async def migrate_blocklist(bot): + start_time = datetime.datetime.utcnow() + + blocked_users = bot.blocked_users + logger.info(f"preparing to migrate blocklist") + skipped = 0 + + blocklist_batch: list[blocklist.BlockedUser] = [] + logger.info(f"preparing to process {len(blocked_users)} blocked users") + skipped += await _convert_legacy_block_list(foo=blocked_users, blocklist_batch=blocklist_batch, + block_type=blocklist.BlockType.USER, bot=bot) + logger.info(f"processed blocked users") + logger.info(f"preparing to process {len(bot.blocked_roles)} blocked roles") + skipped += await _convert_legacy_block_list(foo=bot.blocked_roles, blocklist_batch=blocklist_batch, + block_type=blocklist.BlockType.ROLE, bot=bot) + logger.info(f"processed blocked roles") + + await bot.api.db.blocklist.insert_many([x.__dict__ for x in blocklist_batch]) + blocklist_batch.clear() + + logger.info(f"Migration complete! skipped {skipped} entries") + logger.info(f"migrated in {datetime.datetime.utcnow() - start_time}") + return From b3b1c6ea7dae5640eab58e797c86ddde6efd6cad Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 18:34:16 -0700 Subject: [PATCH 03/18] Add blocklist bot.py integration --- bot.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/bot.py b/bot.py index 780e94c7ab..80ba8e2071 100644 --- a/bot.py +++ b/bot.py @@ -24,6 +24,8 @@ from emoji import UNICODE_EMOJI from pkg_resources import parse_version +from core.blocklist import Blocklist + try: # noinspection PyUnresolvedReferences from colorama import init @@ -87,6 +89,9 @@ def __init__(self): self._configure_logging() self.plugin_db = PluginDatabaseClient(self) # Deprecated + + self.blocklist = Blocklist(bot=self) + self.startup() def get_guild_icon( @@ -524,6 +529,7 @@ async def on_connect(self): logger.debug("Connected to gateway.") await self.config.refresh() await self.api.setup_indexes() + await self.blocklist.setup() await self.load_extensions() self._connected.set() @@ -758,6 +764,8 @@ async def _process_blocked(self, message): # This is to store blocked message cooldown in memory _block_msg_cooldown = dict() + + # This has a bunch of side effectr async def is_blocked( self, author: discord.User, @@ -1221,7 +1229,7 @@ async def on_typing(self, channel, user, _): await user.typing() async def handle_reaction_events(self, payload): - user = self.get_user(payload.user_id) + user = self.get_user(payload.id) if user is None or user.bot: return @@ -1307,8 +1315,8 @@ async def handle_react_to_contact(self, payload): if emoji_fmt != react_message_emoji: return channel = self.get_channel(payload.channel_id) - member = channel.guild.get_member(payload.user_id) or await channel.guild.fetch_member( - payload.user_id + member = channel.guild.get_member(payload.id) or await channel.guild.fetch_member( + payload.id ) if member.bot: return From 32708e78ad342ba8ac7d2ca3309cfa8d77a416f0 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 19:14:40 -0700 Subject: [PATCH 04/18] rename BlockedUser to BlocklistItem & add more blocklist methods Adds add_block, unblock_id, and is_id_blocked to blocklist class --- bot.py | 2 +- core/blocklist.py | 23 +++++++++++++++++++---- core/migrations.py | 14 +++++++------- 3 files changed, 27 insertions(+), 12 deletions(-) diff --git a/bot.py b/bot.py index 80ba8e2071..e209bccbcc 100644 --- a/bot.py +++ b/bot.py @@ -765,7 +765,7 @@ async def _process_blocked(self, message): _block_msg_cooldown = dict() - # This has a bunch of side effectr + # This has a bunch of side effects async def is_blocked( self, author: discord.User, diff --git a/core/blocklist.py b/core/blocklist.py index 379e60afff..2bbf372c6c 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -22,7 +22,7 @@ class BlockReason(enum.StrEnum): @dataclass(frozen=True) -class BlockedUser: +class BlocklistItem: # _id: ObjectId # May be role or user id id: int @@ -49,17 +49,32 @@ async def setup(self): await self.blocklist_collection.create_index("expires_at", expireAfterSeconds=0) async def block_user(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, - blocked_by: int) -> None: + blocked_by: int, block_type: BlockType) -> None: now = datetime.datetime.utcnow() - await self.blocklist_collection.insert_one(BlockedUser( + await self.blocklist_collection.insert_one(BlocklistItem( id=user_id, expires_at=expires_at, reason=reason, timestamp=now, blocking_user_id=blocked_by, type=BlockType.USER - )) + ).__dict__) + + async def add_block(self, block: BlocklistItem) -> None: + await self.blocklist_collection.insert_one(block.__dict__) + + async def unblock_id(self, id: int) -> bool: + result = await self.blocklist_collection.delete_one({"id": id}) + if result.deleted_count == 0: + return False + return True + + async def is_id_blocked(self, user_or_role_id: int) -> bool: + result = await self.blocklist_collection.find_one({"id": user_or_role_id}) + if result is None: + return False + return True # we will probably want to cache these async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[BlockReason]]: diff --git a/core/migrations.py b/core/migrations.py index 12cd878978..8d946b7b79 100644 --- a/core/migrations.py +++ b/core/migrations.py @@ -9,7 +9,7 @@ old_format_matcher = re.compile("by ([\w]*#[\d]{1,4})(?: until )?.") -def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlockedUser: +def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlocklistItem: blocked_by = int(v["blocked_by"]) if "until" in v: # todo make sure this is the correct from format @@ -21,7 +21,7 @@ def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) else: reason = None blocked_ts = datetime.datetime.fromisoformat(v["blocked_at"]) - return blocklist.BlockedUser( + return blocklist.BlocklistItem( id=int(k), expires_at=blocked_until, reason=reason, @@ -31,14 +31,14 @@ def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) ) -def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlockedUser: +def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlocklistItem: match = old_format_matcher.match(v) # blocked_by = match.group(1) blocked_until = match.group(2) if blocked_until is not None: blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) - return blocklist.BlockedUser( + return blocklist.BlocklistItem( id=int(k), expires_at=blocked_until, reason=f"migrated from old format `{v}`", @@ -49,7 +49,7 @@ def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> ) -async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlockedUser], +async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlocklistItem], block_type: blocklist.BlockType, bot) -> int: skipped = 0 @@ -80,7 +80,7 @@ async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist. logger.debug(f"migrating id:{k} ts:{blocked_until} blocker:{blocked_by}") - blocklist_batch.append(blocklist.BlockedUser( + blocklist_batch.append(blocklist.BlocklistItem( id=int(k), expires_at=blocked_until, reason=f"migrated from old format `{v}`", @@ -103,7 +103,7 @@ async def migrate_blocklist(bot): logger.info(f"preparing to migrate blocklist") skipped = 0 - blocklist_batch: list[blocklist.BlockedUser] = [] + blocklist_batch: list[blocklist.BlocklistItem] = [] logger.info(f"preparing to process {len(blocked_users)} blocked users") skipped += await _convert_legacy_block_list(foo=blocked_users, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.USER, bot=bot) From 31aa1d7d9b04b61e0d83eba12a8923c4841fe291 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 19:15:18 -0700 Subject: [PATCH 05/18] migrate block and unblock commands to new blocklist --- cogs/modmail.py | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/cogs/modmail.py b/cogs/modmail.py index e41ecbe321..dc87f37577 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -13,6 +13,7 @@ from discord.ext.commands.view import StringView from core import checks +from core.blocklist import BlockType from core.models import DMDisabled, PermissionLevel, SimilarCategoryConverter, getLogger from core.paginator import EmbedPaginatorSession from core.thread import Thread @@ -1843,8 +1844,6 @@ async def send_embed(title: str, message: str): ): return await send_embed("Error", f"Cannot block {mention}, user is whitelisted.") - now, blocked = discord.utils.utcnow(), dict() - desc = f"{mention} is now blocked." if duration: desc += f"\n- Expires: {discord.utils.format_dt(duration.dt, style='R')}" @@ -1852,25 +1851,23 @@ async def send_embed(title: str, message: str): if reason: desc += f"\n- Reason: {reason}" - blocked["blocked_at"] = str(now) - blocked["blocked_by"] = ctx.author.id - if duration: - blocked["until"] = str(duration.dt) - if reason: - blocked["reason"] = reason - if isinstance(user_or_role, discord.Role): - self.bot.blocked_roles[str(user_or_role.id)] = blocked + await self.bot.blocklist.block_user(user_id=user_or_role.id, + reason=reason, + expires_at=duration.dt if duration is not None else None, + blocked_by=ctx.author.id, + block_type=BlockType.ROLE) elif isinstance(user_or_role, discord.User): - blocked_users = self.bot.blocked_users - blocked_users[str(user_or_role.id)] = blocked + await self.bot.blocklist.block_user(user_id=user_or_role.id, + reason=reason, + expires_at=duration.dt if duration is not None else None, + blocked_by=ctx.author.id, + block_type=BlockType.USER) else: return logger.warning( f"{__name__}: cannot block user, user is neither an instance of Discord Role or User" ) - await self.bot.config.update() - return await send_embed("Success", desc) @commands.command() @@ -1901,20 +1898,13 @@ async def send_embed(title: str, message: str): title, desc = "Error", f"{mention} is not blocked." - if isinstance(user_or_role, discord.Role): - if str(user_or_role.id) not in self.bot.blocked_roles: - return await send_embed(title, desc) - self.bot.blocked_roles.pop(str(user_or_role.id)) - elif isinstance(user_or_role, discord.User): - if str(user_or_role.id) not in self.bot.blocked_users: - return await send_embed(title, desc) - self.bot.blocked_users.pop(str(user_or_role.id)) - else: + if not isinstance(user_or_role, (discord.Role, discord.User)): return logger.warning( f"{__name__}: cannot unblock, user is neither an instance of Discord Role or User" ) - await self.bot.config.update() + if not await self.bot.blocklist.unblock_id(user_or_role.id): + return await send_embed(title, desc) return await send_embed("Success", f"{mention} has been unblocked.") From 3507e21dccc1dd0634465ea9c70b0e31c0a1cd4b Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Sun, 16 Jul 2023 19:37:35 -0700 Subject: [PATCH 06/18] fix overzealous ide rename --- bot.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot.py b/bot.py index e209bccbcc..078cf3db47 100644 --- a/bot.py +++ b/bot.py @@ -1229,7 +1229,7 @@ async def on_typing(self, channel, user, _): await user.typing() async def handle_reaction_events(self, payload): - user = self.get_user(payload.id) + user = self.get_user(payload.user_id) if user is None or user.bot: return @@ -1315,8 +1315,8 @@ async def handle_react_to_contact(self, payload): if emoji_fmt != react_message_emoji: return channel = self.get_channel(payload.channel_id) - member = channel.guild.get_member(payload.id) or await channel.guild.fetch_member( - payload.id + member = channel.guild.get_member(payload.user_id) or await channel.guild.fetch_member( + payload.user_id ) if member.bot: return From e1de36959cb56e13e4116a69c3f62eb72e44fa51 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:29:10 -0700 Subject: [PATCH 07/18] add get_all_blocks method and documentation + cleanup to blocklist.py --- core/blocklist.py | 71 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 58 insertions(+), 13 deletions(-) diff --git a/core/blocklist.py b/core/blocklist.py index 2bbf372c6c..ac5d6a8318 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -5,7 +5,7 @@ import discord import isodate -from bson import CodecOptions, ObjectId +from bson import CodecOptions from motor.core import AgnosticCollection @@ -27,12 +27,23 @@ class BlocklistItem: # May be role or user id id: int expires_at: Optional[datetime.datetime] - reason: str + reason: Optional[str] timestamp: datetime.datetime blocking_user_id: int # specifies if the id is a role or user id type: BlockType + @staticmethod + def from_dict(data: dict): + return BlocklistItem( + id=data["id"], + expires_at=data["expires_at"], + reason=data["reason"], + timestamp=data["timestamp"], + blocking_user_id=data["blocking_user_id"], + type=data["type"] + ) + class Blocklist: blocklist_collection: AgnosticCollection @@ -48,35 +59,69 @@ async def setup(self): await self.blocklist_collection.create_index("id") await self.blocklist_collection.create_index("expires_at", expireAfterSeconds=0) - async def block_user(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, - blocked_by: int, block_type: BlockType) -> None: + async def add_block(self, block: BlocklistItem) -> None: + await self.blocklist_collection.insert_one(block.__dict__) + + async def block_id(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, + blocked_by: int, block_type: BlockType) -> None: now = datetime.datetime.utcnow() - await self.blocklist_collection.insert_one(BlocklistItem( + await self.add_block(block=BlocklistItem( id=user_id, expires_at=expires_at, reason=reason, timestamp=now, blocking_user_id=blocked_by, - type=BlockType.USER - ).__dict__) - - async def add_block(self, block: BlocklistItem) -> None: - await self.blocklist_collection.insert_one(block.__dict__) + type=block_type + )) - async def unblock_id(self, id: int) -> bool: - result = await self.blocklist_collection.delete_one({"id": id}) + async def unblock_id(self, user_or_role_id: int) -> bool: + result = await self.blocklist_collection.delete_one({"id": user_or_role_id}) if result.deleted_count == 0: return False return True async def is_id_blocked(self, user_or_role_id: int) -> bool: + """ + Checks if the given ID is blocked + + This method only checks to see if there is an active manual block for the given ID. + It does not do any checks for whitelisted users or roles, account age, or guild age. + + Parameters + ---------- + user_or_role_id + + Returns + ------- + + True if there is an active block for the given ID + + """ result = await self.blocklist_collection.find_one({"id": user_or_role_id}) if result is None: return False return True - # we will probably want to cache these + async def get_all_blocks(self) -> list[BlocklistItem]: + """ + Returns a list of all active blocks + + THIS RETRIEVES ALL ITEMS FROM THE DATABASE COLLECTION, USE WITH CAUTION + + Returns + ------- + + A list of BlockListItems + + """ + dict_list = await self.blocklist_collection.find().to_list(length=None) + dataclass_list: list[BlocklistItem] = [] + for i in dict_list: + dataclass_list.append(BlocklistItem.from_dict(i)) + return dataclass_list + + # TODO we will probably want to cache these async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[BlockReason]]: """ Side effect free version of is_blocked From 3a37a34e447aa2b74c9e9c9f86140ec5fa65a5fe Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:30:50 -0700 Subject: [PATCH 08/18] refactor blocked command to use new block system --- cogs/modmail.py | 70 ++++++++++++++++--------------------------------- 1 file changed, 23 insertions(+), 47 deletions(-) diff --git a/cogs/modmail.py b/cogs/modmail.py index dc87f37577..a72054c905 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1,5 +1,4 @@ import asyncio -import datetime import re from datetime import timezone from itertools import zip_longest @@ -12,7 +11,7 @@ from discord.ext.commands.cooldowns import BucketType from discord.ext.commands.view import StringView -from core import checks +from core import blocklist, checks from core.blocklist import BlockType from core.models import DMDisabled, PermissionLevel, SimilarCategoryConverter, getLogger from core.paginator import EmbedPaginatorSession @@ -1646,57 +1645,34 @@ async def contact( async def blocked(self, ctx): """Retrieve a list of blocked users.""" - roles, users, now = [], [], discord.utils.utcnow() + roles, users = [], [] - blocked_users = list(self.bot.blocked_users.items()) - for id_, data in blocked_users: - blocked_by_id = data["blocked_by"] - blocked_at = parser.parse(data["blocked_at"]) - human_blocked_at = discord.utils.format_dt(blocked_at, style="R") - if "until" in data: - blocked_until = parser.parse(data["until"]) - human_blocked_until = discord.utils.format_dt(blocked_until, style="R") - else: - blocked_until = human_blocked_until = "Permanent" - - if isinstance(blocked_until, datetime.datetime) and blocked_until < now: - self.bot.blocked_users.pop(str(id_)) - logger.debug("No longer blocked, user %s.", id_) - continue - - string = f"<@{id_}> ({human_blocked_until})" - string += f"\n- Issued {human_blocked_at} by <@{blocked_by_id}>" + blocked: list[blocklist.BlocklistItem] = await self.bot.blocklist.get_all_blocks() - reason = data.get("reason") - if reason: - string += f"\n- Blocked for {reason}" - - users.append(string + "\n") + for item in blocked: + human_blocked_at = discord.utils.format_dt(item.timestamp, style="R") + if item.expires_at is not None: + human_blocked_until = discord.utils.format_dt(item.expires_at, style="R") + else: + human_blocked_until = "Permanent" - blocked_roles = list(self.bot.blocked_roles.items()) - for id_, data in blocked_roles: - blocked_by_id = data["blocked_by"] - blocked_at = parser.parse(data["blocked_at"]) - human_blocked_at = discord.utils.format_dt(blocked_at, style="R") - if "until" in data: - blocked_until = parser.parse(data["until"]) - human_blocked_until = discord.utils.format_dt(blocked_until, style="R") + if item.type == blocklist.BlockType.USER: + string = f"<@{item.id}>" else: - blocked_until = human_blocked_until = "Permanent" + string = f"<@&{item.id}>" - if isinstance(blocked_until, datetime.datetime) and blocked_until < now: - self.bot.blocked_users.pop(str(id_)) - logger.debug("No longer blocked, user %s.", id_) - continue + string += f" ({human_blocked_until})" - string = f"<@&{id_}> ({human_blocked_until})" - string += f"\n- Issued {human_blocked_at} by <@{blocked_by_id}>" + string += f"\n- Issued {human_blocked_at} by <@{item.blocking_user_id}>" - reason = data.get("reason") - if reason: - string += f"\n- Blocked for {reason}" + if item.reason is not None: + string += f"\n- Blocked for {item.reason}" + string += "\n" - roles.append(string + "\n") + if item.type == blocklist.BlockType.USER: + users.append(string) + elif item.type == blocklist.BlockType.ROLE: + roles.append(string) user_embeds = [discord.Embed(title="Blocked Users", color=self.bot.main_color, description="")] @@ -1714,7 +1690,7 @@ async def blocked(self, ctx): else: embed.description += line else: - user_embeds[0].description = "Currently there are no blocked users." + user_embeds[0].description = "No users are currently blocked." if len(user_embeds) > 1: for n, em in enumerate(user_embeds): @@ -1737,7 +1713,7 @@ async def blocked(self, ctx): else: embed.description += line else: - role_embeds[-1].description = "Currently there are no blocked roles." + role_embeds[-1].description = "No roles are currently blocked." if len(role_embeds) > 1: for n, em in enumerate(role_embeds): From 1920188bc94f5ece4386fb02d0fa72ebb280071b Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 13:34:03 -0700 Subject: [PATCH 09/18] cleanup block command code now with more DRY --- cogs/modmail.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cogs/modmail.py b/cogs/modmail.py index a72054c905..f46644d4c4 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1827,23 +1827,23 @@ async def send_embed(title: str, message: str): if reason: desc += f"\n- Reason: {reason}" + blocktype: BlockType + if isinstance(user_or_role, discord.Role): - await self.bot.blocklist.block_user(user_id=user_or_role.id, - reason=reason, - expires_at=duration.dt if duration is not None else None, - blocked_by=ctx.author.id, - block_type=BlockType.ROLE) + blocktype = BlockType.ROLE elif isinstance(user_or_role, discord.User): - await self.bot.blocklist.block_user(user_id=user_or_role.id, - reason=reason, - expires_at=duration.dt if duration is not None else None, - blocked_by=ctx.author.id, - block_type=BlockType.USER) + blocktype = BlockType.USER else: return logger.warning( f"{__name__}: cannot block user, user is neither an instance of Discord Role or User" ) + await self.bot.blocklist.block_id(user_id=user_or_role.id, + reason=reason, + expires_at=duration.dt if duration is not None else None, + blocked_by=ctx.author.id, + block_type=blocktype) + return await send_embed("Success", desc) @commands.command() From 27dd0f4ad48aff3887f65ae56689c4af4ba3032e Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 14:25:55 -0700 Subject: [PATCH 10/18] migrate bot.py is_blocked method to new blocklist system --- bot.py | 58 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/bot.py b/bot.py index 078cf3db47..c58b706f47 100644 --- a/bot.py +++ b/bot.py @@ -24,7 +24,7 @@ from emoji import UNICODE_EMOJI from pkg_resources import parse_version -from core.blocklist import Blocklist +from core.blocklist import Blocklist, BlockReason try: # noinspection PyUnresolvedReferences @@ -764,7 +764,6 @@ async def _process_blocked(self, message): # This is to store blocked message cooldown in memory _block_msg_cooldown = dict() - # This has a bunch of side effects async def is_blocked( self, @@ -802,33 +801,36 @@ async def send_embed(title=None, desc=None): self._block_msg_cooldown[str(author)] = now + timedelta(minutes=5) return await channel.send(embed=emb) - if str(author.id) in self.blocked_whitelisted_users: - if str(author.id) in self.blocked_users: - self.blocked_users.pop(str(author.id)) - await self.config.update() - return False - - if not self.check_account_age(author): - blocked_reason = "Sorry, your account is too new to initiate a ticket." - await send_embed(title="Message not sent!", desc=blocked_reason) - return True - - if not self.check_manual_blocked(author): - blocked_reason = "You have been blocked from contacting Modmail." - await send_embed(title="Message not sent!", desc=blocked_reason) - return True - - if not self.check_guild_age(member): - blocked_reason = "Sorry, you joined the server too recently to initiate a ticket." - await send_embed(title="Message not sent!", desc=blocked_reason) - return True - - if not self.check_manual_blocked_roles(member): - blocked_reason = "Sorry, your role(s) has been blacklisted from contacting Modmail." - await send_embed(title="Message not sent!", desc=blocked_reason) - return True + if member is not None: + blocked, block_type = self.blocklist.is_user_blocked(member) + else: + blocked, block_type = self.blocklist.is_id_blocked(author.id) + if not self.blocklist.is_valid_account_age(author): + blocked = True + block_type = BlockReason.ACCOUNT_AGE - return False + if blocked: + if block_type == BlockReason.ACCOUNT_AGE: + blocked_reason = "Sorry, your account is too new to initiate a ticket." + await send_embed(title="Message not sent!", desc=blocked_reason) + return True + + if block_type == BlockReason.BLOCKED_USER: + blocked_reason = "You have been blocked from contacting Modmail." + await send_embed(title="Message not sent!", desc=blocked_reason) + return True + + if block_type == BlockReason.GUILD_AGE: + blocked_reason = "Sorry, you joined the server too recently to initiate a ticket." + await send_embed(title="Message not sent!", desc=blocked_reason) + return True + + if block_type == BlockReason.BLOCKED_ROLE: + blocked_reason = "Sorry, your role(s) has been blacklisted from contacting Modmail." + await send_embed(title="Message not sent!", desc=blocked_reason) + return True + + return blocked async def get_thread_cooldown(self, author: discord.Member): thread_cooldown = self.config.get("thread_cooldown") From bd20487977556cfc0d62ad5300c52e8263a8165d Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:51:37 -0700 Subject: [PATCH 11/18] migrate whitelist command and refactor is_id_blocked to return tuple is_id_blocked now returns a tuple[bool, Optional[BlocklistItem]] so the calling code can get information about the block easily --- cogs/modmail.py | 22 ++++++++++------------ core/blocklist.py | 8 ++++---- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/cogs/modmail.py b/cogs/modmail.py index f46644d4c4..08610b737a 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1740,7 +1740,6 @@ async def blocked_whitelist(self, ctx, *, user: User = None): return await ctx.send_help(ctx.command) mention = getattr(user, "mention", f"`{user.id}`") - msg = "" if str(user.id) in self.bot.blocked_whitelisted_users: embed = discord.Embed( @@ -1752,21 +1751,20 @@ async def blocked_whitelist(self, ctx, *, user: User = None): return await ctx.send(embed=embed) self.bot.blocked_whitelisted_users.append(str(user.id)) - - if str(user.id) in self.bot.blocked_users: - msg = self.bot.blocked_users.get(str(user.id)) or "" - self.bot.blocked_users.pop(str(user.id)) - await self.bot.config.update() - if msg.startswith("System Message: "): - # If the user is blocked internally (for example: below minimum account age) - # Show an extended message stating the original internal message - reason = msg[16:].strip().rstrip(".") + blocked: bool + foo: blocklist.BlocklistItem + + blocked, foo = await self.bot.blocklist.is_id_blocked(user.id) + if blocked: + await self.bot.blocklist.unblock_id(user.id) embed = discord.Embed( title="Success", - description=f"{mention} was previously blocked internally for " - f'"{reason}". {mention} is now whitelisted.', + description=f""" + {mention} has been whitelisted. + They were previously blocked by <@{foo.blocking_user_id}> {" for "+foo.reason if foo.reason is not None else ""}. + """, color=self.bot.main_color, ) else: diff --git a/core/blocklist.py b/core/blocklist.py index ac5d6a8318..fbf4b75116 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -81,7 +81,7 @@ async def unblock_id(self, user_or_role_id: int) -> bool: return False return True - async def is_id_blocked(self, user_or_role_id: int) -> bool: + async def is_id_blocked(self, user_or_role_id: int) -> Tuple[bool, Optional[BlocklistItem]]: """ Checks if the given ID is blocked @@ -100,8 +100,8 @@ async def is_id_blocked(self, user_or_role_id: int) -> bool: """ result = await self.blocklist_collection.find_one({"id": user_or_role_id}) if result is None: - return False - return True + return False, None + return True, BlocklistItem.from_dict(result) async def get_all_blocks(self) -> list[BlocklistItem]: """ @@ -137,7 +137,7 @@ async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[ # if str(member.id) in self.bot.blocked_whitelisted_users: - return False + return False, None blocked = await self.blocklist_collection.find_one({"id": member.id}) if blocked is not None: From 047dc2193e1dbea5761c808319407a0c5cd1207e Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:53:43 -0700 Subject: [PATCH 12/18] rename BlocklistItem to BlocklistEntry --- cogs/modmail.py | 8 ++++---- core/blocklist.py | 20 +++++++++----------- core/migrations.py | 14 +++++++------- 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/cogs/modmail.py b/cogs/modmail.py index 08610b737a..d4c6f2eb44 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1647,7 +1647,7 @@ async def blocked(self, ctx): roles, users = [], [] - blocked: list[blocklist.BlocklistItem] = await self.bot.blocklist.get_all_blocks() + blocked: list[blocklist.BlocklistEntry] = await self.bot.blocklist.get_all_blocks() for item in blocked: human_blocked_at = discord.utils.format_dt(item.timestamp, style="R") @@ -1754,16 +1754,16 @@ async def blocked_whitelist(self, ctx, *, user: User = None): await self.bot.config.update() blocked: bool - foo: blocklist.BlocklistItem + blocklist_entry: blocklist.BlocklistEntry - blocked, foo = await self.bot.blocklist.is_id_blocked(user.id) + blocked, blocklist_entry = await self.bot.blocklist.is_id_blocked(user.id) if blocked: await self.bot.blocklist.unblock_id(user.id) embed = discord.Embed( title="Success", description=f""" {mention} has been whitelisted. - They were previously blocked by <@{foo.blocking_user_id}> {" for "+foo.reason if foo.reason is not None else ""}. + They were previously blocked by <@{blocklist_entry.blocking_user_id}> {" for "+blocklist_entry.reason if blocklist_entry.reason is not None else ""}. """, color=self.bot.main_color, ) diff --git a/core/blocklist.py b/core/blocklist.py index fbf4b75116..73677e2bc0 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -22,7 +22,7 @@ class BlockReason(enum.StrEnum): @dataclass(frozen=True) -class BlocklistItem: +class BlocklistEntry: # _id: ObjectId # May be role or user id id: int @@ -35,7 +35,7 @@ class BlocklistItem: @staticmethod def from_dict(data: dict): - return BlocklistItem( + return BlocklistEntry( id=data["id"], expires_at=data["expires_at"], reason=data["reason"], @@ -54,19 +54,17 @@ def __init__(self: Self, bot) -> None: self.bot = bot async def setup(self): - index_info = await self.blocklist_collection.index_information() - print(index_info) await self.blocklist_collection.create_index("id") await self.blocklist_collection.create_index("expires_at", expireAfterSeconds=0) - async def add_block(self, block: BlocklistItem) -> None: + async def add_block(self, block: BlocklistEntry) -> None: await self.blocklist_collection.insert_one(block.__dict__) async def block_id(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, blocked_by: int, block_type: BlockType) -> None: now = datetime.datetime.utcnow() - await self.add_block(block=BlocklistItem( + await self.add_block(block=BlocklistEntry( id=user_id, expires_at=expires_at, reason=reason, @@ -81,7 +79,7 @@ async def unblock_id(self, user_or_role_id: int) -> bool: return False return True - async def is_id_blocked(self, user_or_role_id: int) -> Tuple[bool, Optional[BlocklistItem]]: + async def is_id_blocked(self, user_or_role_id: int) -> Tuple[bool, Optional[BlocklistEntry]]: """ Checks if the given ID is blocked @@ -101,9 +99,9 @@ async def is_id_blocked(self, user_or_role_id: int) -> Tuple[bool, Optional[Bloc result = await self.blocklist_collection.find_one({"id": user_or_role_id}) if result is None: return False, None - return True, BlocklistItem.from_dict(result) + return True, BlocklistEntry.from_dict(result) - async def get_all_blocks(self) -> list[BlocklistItem]: + async def get_all_blocks(self) -> list[BlocklistEntry]: """ Returns a list of all active blocks @@ -116,9 +114,9 @@ async def get_all_blocks(self) -> list[BlocklistItem]: """ dict_list = await self.blocklist_collection.find().to_list(length=None) - dataclass_list: list[BlocklistItem] = [] + dataclass_list: list[BlocklistEntry] = [] for i in dict_list: - dataclass_list.append(BlocklistItem.from_dict(i)) + dataclass_list.append(BlocklistEntry.from_dict(i)) return dataclass_list # TODO we will probably want to cache these diff --git a/core/migrations.py b/core/migrations.py index 8d946b7b79..bca9442129 100644 --- a/core/migrations.py +++ b/core/migrations.py @@ -9,7 +9,7 @@ old_format_matcher = re.compile("by ([\w]*#[\d]{1,4})(?: until )?.") -def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlocklistItem: +def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlocklistEntry: blocked_by = int(v["blocked_by"]) if "until" in v: # todo make sure this is the correct from format @@ -21,7 +21,7 @@ def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) else: reason = None blocked_ts = datetime.datetime.fromisoformat(v["blocked_at"]) - return blocklist.BlocklistItem( + return blocklist.BlocklistEntry( id=int(k), expires_at=blocked_until, reason=reason, @@ -31,14 +31,14 @@ def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) ) -def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlocklistItem: +def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlocklistEntry: match = old_format_matcher.match(v) # blocked_by = match.group(1) blocked_until = match.group(2) if blocked_until is not None: blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) - return blocklist.BlocklistItem( + return blocklist.BlocklistEntry( id=int(k), expires_at=blocked_until, reason=f"migrated from old format `{v}`", @@ -49,7 +49,7 @@ def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> ) -async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlocklistItem], +async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlocklistEntry], block_type: blocklist.BlockType, bot) -> int: skipped = 0 @@ -80,7 +80,7 @@ async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist. logger.debug(f"migrating id:{k} ts:{blocked_until} blocker:{blocked_by}") - blocklist_batch.append(blocklist.BlocklistItem( + blocklist_batch.append(blocklist.BlocklistEntry( id=int(k), expires_at=blocked_until, reason=f"migrated from old format `{v}`", @@ -103,7 +103,7 @@ async def migrate_blocklist(bot): logger.info(f"preparing to migrate blocklist") skipped = 0 - blocklist_batch: list[blocklist.BlocklistItem] = [] + blocklist_batch: list[blocklist.BlocklistEntry] = [] logger.info(f"preparing to process {len(blocked_users)} blocked users") skipped += await _convert_legacy_block_list(foo=blocked_users, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.USER, bot=bot) From 82cb5801b555384022037cdcdefe42242f42b3fc Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 15:58:00 -0700 Subject: [PATCH 13/18] Add deprecation warning logs to legacy block code and docstrings to bot.py block related code --- bot.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/bot.py b/bot.py index c58b706f47..d871297d75 100644 --- a/bot.py +++ b/bot.py @@ -173,6 +173,9 @@ def startup(self): logger.line() logger.info("discord.py: v%s", discord.__version__) logger.line() + if not self.config["blocked"] or not self.config["blocked_roles"]: + logger.warning("Un-migrated blocklists found. Please run the '[p]migrate blocklist' command after backing " + "up your config/database. Blocklist functionality will be disabled until this is done.") async def load_extensions(self): for cog in self.loaded_cogs: @@ -467,10 +470,14 @@ def main_category(self) -> typing.Optional[discord.CategoryChannel]: @property def blocked_users(self) -> typing.Dict[str, str]: + """DEPRECATED, used blocklist instead""" + logger.warning("blocked_users is deprecated and does not function, its usage is a bug") return self.config["blocked"] @property def blocked_roles(self) -> typing.Dict[str, str]: + """DEPRECATED, used blocklist instead""" + logger.warning("blocked_roles is deprecated and does not function, its usage is a bug") return self.config["blocked_roles"] @property @@ -724,6 +731,7 @@ def check_guild_age(self, author: discord.Member) -> bool: return True def check_manual_blocked_roles(self, author: discord.Member) -> bool: + logger.error("check_manual_blocked_roles is deprecated, usage is a bug.") for role in author.roles: if str(role.id) not in self.blocked_roles: continue @@ -740,6 +748,7 @@ def check_manual_blocked_roles(self, author: discord.Member) -> bool: return True def check_manual_blocked(self, author: discord.User) -> bool: + logger.error("check_manual_blocked is deprecated, usage is a bug.") if str(author.id) not in self.blocked_users: return True @@ -772,6 +781,24 @@ async def is_blocked( channel: discord.TextChannel = None, send_message: bool = False, ) -> bool: + """ + Check if a user is blocked for any reason and send a message if they are (if send_message is true). + + If you are using this method with send_message set to false or not set, + You should be using blocklist.is_user_blocked() or blocklist.is_id_blocked() + if you only care whether a user is manually blocked then use blocklist.is_id_blocked(). + + Parameters + ---------- + author + channel + send_message + + Returns + ------- + bool + Whether the user is blocked or not. + """ member = self.guild.get_member(author.id) or await MemberConverter.convert(author) if member is None: # try to find in other guilds From f7a4ef12fbd0deaf3ac4a4858777f3d5df74881d Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:08:27 -0700 Subject: [PATCH 14/18] clean up migrations.py code --- core/migrations.py | 83 +++++++++++++++++++++++++--------------------- 1 file changed, 46 insertions(+), 37 deletions(-) diff --git a/core/migrations.py b/core/migrations.py index bca9442129..591255deaa 100644 --- a/core/migrations.py +++ b/core/migrations.py @@ -1,26 +1,39 @@ import datetime import re +from typing import Optional from core import blocklist from core.models import getLogger logger = getLogger(__name__) -old_format_matcher = re.compile("by ([\w]*#[\d]{1,4})(?: until )?.") +old_format_matcher = re.compile("by (\w*#\d{1,4})(?: until )?.") -def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) -> blocklist.BlocklistEntry: +def _convert_legacy_dict_block_format(k, v: dict, block_type: blocklist.BlockType) -> Optional[blocklist.BlocklistEntry]: + """ + Converts a legacy dict based blocklist entry to the new dataclass format + + Returns None if the block has expired + """ + blocked_by = int(v["blocked_by"]) if "until" in v: # todo make sure this is the correct from format blocked_until = datetime.datetime.fromisoformat(v["until"]) + # skip if blocked_until occurred in the past + if blocked_until < datetime.datetime.now(datetime.timezone.utc): + return None else: blocked_until = None + if "reason" in v: reason = v["reason"] else: reason = None + blocked_ts = datetime.datetime.fromisoformat(v["blocked_at"]) + return blocklist.BlocklistEntry( id=int(k), expires_at=blocked_until, @@ -31,12 +44,20 @@ def _convert_legacy_v2_block_format(k, v: dict, block_type: blocklist.BlockType) ) -def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> blocklist.BlocklistEntry: +def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> Optional[blocklist.BlocklistEntry]: + """ + Converts a legacy string based blocklist entry to the new dataclass format + + Returns None if the block has expired + """ + match = old_format_matcher.match(v) - # blocked_by = match.group(1) blocked_until = match.group(2) if blocked_until is not None: blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) + # skip if blocked_until occurred in the past + if blocked_until < datetime.datetime.now(datetime.timezone.utc): + return None return blocklist.BlocklistEntry( id=int(k), @@ -56,39 +77,22 @@ async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist. for k, v in foo.items(): # handle new block format if type(v) is dict: - block = _convert_legacy_v2_block_format(k, v, block_type=block_type) - if block.expires_at is not None and block.expires_at < datetime.datetime.now(datetime.timezone.utc): + block = _convert_legacy_dict_block_format(k, v, block_type=block_type) + if block is None: logger.debug("skipping expired block entry") skipped += 1 continue - logger.debug(f"migrating new format {v}") - blocklist_batch.append(block) - - continue - - match = old_format_matcher.match(v) - blocked_by = match.group(1) - blocked_until = match.group(2) - if blocked_until is not None: - blocked_until = datetime.datetime.fromtimestamp(int(blocked_until), tz=datetime.timezone.utc) - - # skip this item if blocked_until occurred in the past - if blocked_until is not None and blocked_until < datetime.datetime.now(datetime.timezone.utc): - logger.debug("skipping expired block entry") - skipped += 1 - continue - - logger.debug(f"migrating id:{k} ts:{blocked_until} blocker:{blocked_by}") - - blocklist_batch.append(blocklist.BlocklistEntry( - id=int(k), - expires_at=blocked_until, - reason=f"migrated from old format `{v}`", - timestamp=datetime.datetime.utcnow(), - # I'm not bothering to fetch the user object here, discords username migrations will have broken all of them - blocking_user_id=0, - type=blocklist.BlockType.USER - )) + logger.debug(f"migrating new format {k}: {v}") + else: + block = _convert_legacy_block_format(k, v, block_type=block_type) + if block is None: + logger.debug("skipping expired block entry") + skipped += 1 + continue + logger.debug(f"migrating legacy format {k}: {v}") + + blocklist_batch.append(block) + if len(blocklist_batch) >= 100: await bot.api.db.blocklist.insert_many([x.__dict__ for x in blocklist_batch]) blocklist_batch.clear() @@ -100,22 +104,27 @@ async def migrate_blocklist(bot): start_time = datetime.datetime.utcnow() blocked_users = bot.blocked_users - logger.info(f"preparing to migrate blocklist") + logger.info("preparing to migrate blocklist") skipped = 0 blocklist_batch: list[blocklist.BlocklistEntry] = [] logger.info(f"preparing to process {len(blocked_users)} blocked users") skipped += await _convert_legacy_block_list(foo=blocked_users, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.USER, bot=bot) - logger.info(f"processed blocked users") + logger.info("processed blocked users") logger.info(f"preparing to process {len(bot.blocked_roles)} blocked roles") skipped += await _convert_legacy_block_list(foo=bot.blocked_roles, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.ROLE, bot=bot) - logger.info(f"processed blocked roles") + logger.info("processed blocked roles") await bot.api.db.blocklist.insert_many([x.__dict__ for x in blocklist_batch]) blocklist_batch.clear() + logger.info("clearing old blocklists") + bot.blocked_users.clear() + bot.blocked_roles.clear() + await bot.config.update() + logger.info(f"Migration complete! skipped {skipped} entries") logger.info(f"migrated in {datetime.datetime.utcnow() - start_time}") return From 692ae7c0b51d6a387b1b802c0b2e0fefb2b7e727 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:11:05 -0700 Subject: [PATCH 15/18] Add [p]migration command --- cogs/utility.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cogs/utility.py b/cogs/utility.py index f4c1a5f663..ddb1ab9a3c 100644 --- a/cogs/utility.py +++ b/cogs/utility.py @@ -22,7 +22,7 @@ from discord.ext.commands.view import StringView from pkg_resources import parse_version -from core import checks, utils +from core import checks, migrations, utils from core.changelog import Changelog from core.models import HostingMethod, InvalidConfigError, PermissionLevel, UnseenFormatter, getLogger from core.paginator import EmbedPaginatorSession, MessagePaginatorSession @@ -1952,6 +1952,19 @@ async def autotrigger_list(self, ctx): await EmbedPaginatorSession(ctx, *embeds).run() + @commands.command() + @checks.has_permissions(PermissionLevel.OWNER) + async def migrate(self, ctx, migration: str): + """Perform a given database migration""" + if migration == "blocklist": + try: + await migrations.migrate_blocklist(self.bot) + except Exception as e: + await ctx.send(embed=discord.Embed(title="Error", description=str(e), color=self.bot.error_color)) + raise e + + await ctx.send(embed=discord.Embed(title="Success", color=self.bot.main_color)) + @commands.command() @checks.has_permissions(PermissionLevel.OWNER) @checks.github_token_required() From e1adc4bce77957914cb12b8daa6eedfa2945a0f1 Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:15:48 -0700 Subject: [PATCH 16/18] add final deprecation docstrings and refactor contact command to use blocklist.is_user_blocked() instead of core is_blocked() --- bot.py | 2 ++ cogs/modmail.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/bot.py b/bot.py index d871297d75..9991f22008 100644 --- a/bot.py +++ b/bot.py @@ -731,6 +731,7 @@ def check_guild_age(self, author: discord.Member) -> bool: return True def check_manual_blocked_roles(self, author: discord.Member) -> bool: + """DEPRECATED""" logger.error("check_manual_blocked_roles is deprecated, usage is a bug.") for role in author.roles: if str(role.id) not in self.blocked_roles: @@ -748,6 +749,7 @@ def check_manual_blocked_roles(self, author: discord.Member) -> bool: return True def check_manual_blocked(self, author: discord.User) -> bool: + """DEPRECATED""" logger.error("check_manual_blocked is deprecated, usage is a bug.") if str(author.id) not in self.blocked_users: return True diff --git a/cogs/modmail.py b/cogs/modmail.py index d4c6f2eb44..52ea0cda0f 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1562,7 +1562,7 @@ async def contact( elif u.bot: errors.append(f"{u} is a bot, cannot add to thread.") users.remove(u) - elif await self.bot.is_blocked(u): + elif await self.bot.blocklist.is_user_blocked(u): ref = f"{u.mention} is" if ctx.author != u else "You are" errors.append(f"{ref} currently blocked from contacting {self.bot.user.name}.") users.remove(u) From 1b2af9735789f98395eb0ac123e2810891af6aaf Mon Sep 17 00:00:00 2001 From: Khakers <22665282+khakers@users.noreply.github.com> Date: Mon, 17 Jul 2023 16:17:18 -0700 Subject: [PATCH 17/18] remove blocklist print statements --- core/blocklist.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/blocklist.py b/core/blocklist.py index 73677e2bc0..50b5dfd677 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -139,7 +139,6 @@ async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[ blocked = await self.blocklist_collection.find_one({"id": member.id}) if blocked is not None: - print(blocked) return True, BlockReason.BLOCKED_USER roles = member.roles @@ -149,10 +148,8 @@ async def is_user_blocked(self, member: discord.Member) -> Tuple[bool, Optional[ return True, BlockReason.BLOCKED_ROLE if not self.is_valid_account_age(member): - print("account_age") return True, BlockReason.ACCOUNT_AGE if not self.is_valid_guild_age(member): - print("guild_age") return True, BlockReason.GUILD_AGE return False, None From 7b532614a6bf104d8cec70ce37faef347851b9e3 Mon Sep 17 00:00:00 2001 From: Raiden Sakura Date: Tue, 18 Jul 2023 15:08:22 +0800 Subject: [PATCH 18/18] Format with black --- bot.py | 6 ++++-- cogs/modmail.py | 12 +++++++----- cogs/utility.py | 4 +++- core/blocklist.py | 33 +++++++++++++++++++++------------ core/migrations.py | 27 +++++++++++++++++---------- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/bot.py b/bot.py index 9991f22008..48ad5982bb 100644 --- a/bot.py +++ b/bot.py @@ -174,8 +174,10 @@ def startup(self): logger.info("discord.py: v%s", discord.__version__) logger.line() if not self.config["blocked"] or not self.config["blocked_roles"]: - logger.warning("Un-migrated blocklists found. Please run the '[p]migrate blocklist' command after backing " - "up your config/database. Blocklist functionality will be disabled until this is done.") + logger.warning( + "Un-migrated blocklists found. Please run the '[p]migrate blocklist' command after backing " + "up your config/database. Blocklist functionality will be disabled until this is done." + ) async def load_extensions(self): for cog in self.loaded_cogs: diff --git a/cogs/modmail.py b/cogs/modmail.py index 52ea0cda0f..061640e264 100644 --- a/cogs/modmail.py +++ b/cogs/modmail.py @@ -1836,11 +1836,13 @@ async def send_embed(title: str, message: str): f"{__name__}: cannot block user, user is neither an instance of Discord Role or User" ) - await self.bot.blocklist.block_id(user_id=user_or_role.id, - reason=reason, - expires_at=duration.dt if duration is not None else None, - blocked_by=ctx.author.id, - block_type=blocktype) + await self.bot.blocklist.block_id( + user_id=user_or_role.id, + reason=reason, + expires_at=duration.dt if duration is not None else None, + blocked_by=ctx.author.id, + block_type=blocktype, + ) return await send_embed("Success", desc) diff --git a/cogs/utility.py b/cogs/utility.py index ddb1ab9a3c..135737dd8e 100644 --- a/cogs/utility.py +++ b/cogs/utility.py @@ -1960,7 +1960,9 @@ async def migrate(self, ctx, migration: str): try: await migrations.migrate_blocklist(self.bot) except Exception as e: - await ctx.send(embed=discord.Embed(title="Error", description=str(e), color=self.bot.error_color)) + await ctx.send( + embed=discord.Embed(title="Error", description=str(e), color=self.bot.error_color) + ) raise e await ctx.send(embed=discord.Embed(title="Success", color=self.bot.main_color)) diff --git a/core/blocklist.py b/core/blocklist.py index 50b5dfd677..47295a9756 100644 --- a/core/blocklist.py +++ b/core/blocklist.py @@ -41,7 +41,7 @@ def from_dict(data: dict): reason=data["reason"], timestamp=data["timestamp"], blocking_user_id=data["blocking_user_id"], - type=data["type"] + type=data["type"], ) @@ -50,7 +50,8 @@ class Blocklist: def __init__(self: Self, bot) -> None: self.blocklist_collection = bot.api.db.blocklist.with_options( - codec_options=CodecOptions(tz_aware=True, tzinfo=datetime.timezone.utc)) + codec_options=CodecOptions(tz_aware=True, tzinfo=datetime.timezone.utc) + ) self.bot = bot async def setup(self): @@ -60,18 +61,26 @@ async def setup(self): async def add_block(self, block: BlocklistEntry) -> None: await self.blocklist_collection.insert_one(block.__dict__) - async def block_id(self, user_id: int, expires_at: Optional[datetime.datetime], reason: str, - blocked_by: int, block_type: BlockType) -> None: + async def block_id( + self, + user_id: int, + expires_at: Optional[datetime.datetime], + reason: str, + blocked_by: int, + block_type: BlockType, + ) -> None: now = datetime.datetime.utcnow() - await self.add_block(block=BlocklistEntry( - id=user_id, - expires_at=expires_at, - reason=reason, - timestamp=now, - blocking_user_id=blocked_by, - type=block_type - )) + await self.add_block( + block=BlocklistEntry( + id=user_id, + expires_at=expires_at, + reason=reason, + timestamp=now, + blocking_user_id=blocked_by, + type=block_type, + ) + ) async def unblock_id(self, user_or_role_id: int) -> bool: result = await self.blocklist_collection.delete_one({"id": user_or_role_id}) diff --git a/core/migrations.py b/core/migrations.py index 591255deaa..87ddbbe766 100644 --- a/core/migrations.py +++ b/core/migrations.py @@ -10,7 +10,9 @@ old_format_matcher = re.compile("by (\w*#\d{1,4})(?: until )?.") -def _convert_legacy_dict_block_format(k, v: dict, block_type: blocklist.BlockType) -> Optional[blocklist.BlocklistEntry]: +def _convert_legacy_dict_block_format( + k, v: dict, block_type: blocklist.BlockType +) -> Optional[blocklist.BlocklistEntry]: """ Converts a legacy dict based blocklist entry to the new dataclass format @@ -40,11 +42,13 @@ def _convert_legacy_dict_block_format(k, v: dict, block_type: blocklist.BlockTyp reason=reason, timestamp=blocked_ts, blocking_user_id=blocked_by, - type=block_type + type=block_type, ) -def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> Optional[blocklist.BlocklistEntry]: +def _convert_legacy_block_format( + k, v: str, block_type: blocklist.BlockType +) -> Optional[blocklist.BlocklistEntry]: """ Converts a legacy string based blocklist entry to the new dataclass format @@ -66,12 +70,13 @@ def _convert_legacy_block_format(k, v: str, block_type: blocklist.BlockType) -> timestamp=datetime.datetime.utcnow(), # I'm not bothering to fetch the user object here, discords username migrations will have broken all of them blocking_user_id=0, - type=block_type + type=block_type, ) -async def _convert_legacy_block_list(foo: dict, blocklist_batch: list[blocklist.BlocklistEntry], - block_type: blocklist.BlockType, bot) -> int: +async def _convert_legacy_block_list( + foo: dict, blocklist_batch: list[blocklist.BlocklistEntry], block_type: blocklist.BlockType, bot +) -> int: skipped = 0 for k, v in foo.items(): @@ -109,12 +114,14 @@ async def migrate_blocklist(bot): blocklist_batch: list[blocklist.BlocklistEntry] = [] logger.info(f"preparing to process {len(blocked_users)} blocked users") - skipped += await _convert_legacy_block_list(foo=blocked_users, blocklist_batch=blocklist_batch, - block_type=blocklist.BlockType.USER, bot=bot) + skipped += await _convert_legacy_block_list( + foo=blocked_users, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.USER, bot=bot + ) logger.info("processed blocked users") logger.info(f"preparing to process {len(bot.blocked_roles)} blocked roles") - skipped += await _convert_legacy_block_list(foo=bot.blocked_roles, blocklist_batch=blocklist_batch, - block_type=blocklist.BlockType.ROLE, bot=bot) + skipped += await _convert_legacy_block_list( + foo=bot.blocked_roles, blocklist_batch=blocklist_batch, block_type=blocklist.BlockType.ROLE, bot=bot + ) logger.info("processed blocked roles") await bot.api.db.blocklist.insert_many([x.__dict__ for x in blocklist_batch])