diff options
37 files changed, 1140 insertions, 399 deletions
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml new file mode 100644 index 000000000..8760b35ec --- /dev/null +++ b/.github/workflows/codeql-analysis.yml @@ -0,0 +1,32 @@ +name: "Code scanning - action" + +on: + push: + pull_request: + schedule: + - cron: '0 12 * * *' + +jobs: + CodeQL-Build: + + runs-on: ubuntu-latest + + steps: + - name: Checkout repository + uses: actions/checkout@v2 + with: + fetch-depth: 2 + + - run: git checkout HEAD^2 + if: ${{ github.event_name == 'pull_request' }} + + - name: Initialize CodeQL + uses: github/codeql-action/init@v1 + with: + languages: python + + - name: Autobuild + uses: github/codeql-action/autobuild@v1 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v1 @@ -50,4 +50,5 @@ precommit = "pre-commit install" build = "docker build -t pythondiscord/bot:latest -f Dockerfile ." push = "docker push pythondiscord/bot:latest" test = "coverage run -m unittest" +html = "coverage html" report = "coverage report" diff --git a/bot/__main__.py b/bot/__main__.py index aa1d1aee8..4e0d4a111 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -3,7 +3,9 @@ import logging import discord import sentry_sdk from discord.ext.commands import when_mentioned_or +from sentry_sdk.integrations.aiohttp import AioHttpIntegration from sentry_sdk.integrations.logging import LoggingIntegration +from sentry_sdk.integrations.redis import RedisIntegration from bot import constants, patches from bot.bot import Bot @@ -15,7 +17,11 @@ sentry_logging = LoggingIntegration( sentry_sdk.init( dsn=constants.Bot.sentry_dsn, - integrations=[sentry_logging] + integrations=[ + sentry_logging, + AioHttpIntegration(), + RedisIntegration(), + ] ) bot = Bot( diff --git a/bot/cogs/duck_pond.py b/bot/cogs/duck_pond.py index 37d1786a2..5b6a7fd62 100644 --- a/bot/cogs/duck_pond.py +++ b/bot/cogs/duck_pond.py @@ -7,7 +7,7 @@ from discord.ext.commands import Cog from bot import constants from bot.bot import Bot -from bot.utils.messages import send_attachments +from bot.utils.messages import send_attachments, sub_clyde log = logging.getLogger(__name__) @@ -58,7 +58,7 @@ class DuckPond(Cog): try: await self.webhook.send( content=content, - username=username, + username=sub_clyde(username), avatar_url=avatar_url, embed=embed ) diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 1d9fddb12..76ea68660 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -1,10 +1,13 @@ +import asyncio import logging import re -from typing import Optional, Union +from datetime import datetime, timedelta +from typing import List, Mapping, Optional, Union +import dateutil import discord.errors from dateutil.relativedelta import relativedelta -from discord import Colour, Member, Message, TextChannel +from discord import Colour, HTTPException, Member, Message, NotFound, TextChannel from discord.ext.commands import Cog from discord.utils import escape_markdown @@ -14,6 +17,9 @@ from bot.constants import ( Channels, Colours, Filter, Icons, URLs ) +from bot.utils.redis_cache import RedisCache +from bot.utils.scheduling import Scheduler +from bot.utils.time import wait_until log = logging.getLogger(__name__) @@ -40,6 +46,8 @@ TOKEN_WATCHLIST_PATTERNS = [ ] WATCHLIST_PATTERNS = WORD_WATCHLIST_PATTERNS + TOKEN_WATCHLIST_PATTERNS +DAYS_BETWEEN_ALERTS = 3 + def expand_spoilers(text: str) -> str: """Return a string containing all interpretations of a spoilered message.""" @@ -49,11 +57,20 @@ def expand_spoilers(text: str) -> str: ) -class Filtering(Cog): +OFFENSIVE_MSG_DELETE_TIME = timedelta(days=Filter.offensive_msg_delete_days) + + +class Filtering(Cog, Scheduler): """Filtering out invites, blacklisting domains, and warning us of certain regular expressions.""" + # Redis cache mapping a user ID to the last timestamp a bad nickname alert was sent + name_alerts = RedisCache() + def __init__(self, bot: Bot): self.bot = bot + super().__init__() + + self.name_lock = asyncio.Lock() staff_mistake_str = "If you believe this was a mistake, please let staff know!" self.filters = { @@ -66,7 +83,8 @@ class Filtering(Cog): "notification_msg": ( "Your post has been removed for abusing Unicode character rendering (aka Zalgo text). " f"{staff_mistake_str}" - ) + ), + "schedule_deletion": False }, "filter_invites": { "enabled": Filter.filter_invites, @@ -77,7 +95,8 @@ class Filtering(Cog): "notification_msg": ( f"Per Rule 6, your invite link has been removed. {staff_mistake_str}\n\n" r"Our server rules can be found here: <https://pythondiscord.com/pages/rules>" - ) + ), + "schedule_deletion": False }, "filter_domains": { "enabled": Filter.filter_domains, @@ -87,22 +106,27 @@ class Filtering(Cog): "user_notification": Filter.notify_user_domains, "notification_msg": ( f"Your URL has been removed because it matched a blacklisted domain. {staff_mistake_str}" - ) + ), + "schedule_deletion": False }, "watch_regex": { "enabled": Filter.watch_regex, "function": self._has_watch_regex_match, "type": "watchlist", "content_only": True, + "schedule_deletion": True }, "watch_rich_embeds": { "enabled": Filter.watch_rich_embeds, "function": self._has_rich_embed, "type": "watchlist", "content_only": False, - }, + "schedule_deletion": False + } } + self.bot.loop.create_task(self.reschedule_offensive_msg_deletion()) + @property def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" @@ -112,6 +136,7 @@ class Filtering(Cog): async def on_message(self, msg: Message) -> None: """Invoke message filter for new messages.""" await self._filter_message(msg) + await self.check_bad_words_in_name(msg.author) @Cog.listener() async def on_message_edit(self, before: Message, after: Message) -> None: @@ -126,6 +151,55 @@ class Filtering(Cog): delta = relativedelta(after.edited_at, before.edited_at).microseconds await self._filter_message(after, delta) + @staticmethod + def get_name_matches(name: str) -> List[re.Match]: + """Check bad words from passed string (name). Return list of matches.""" + matches = [] + for pattern in WATCHLIST_PATTERNS: + if match := pattern.search(name): + matches.append(match) + return matches + + async def check_send_alert(self, member: Member) -> bool: + """When there is less than 3 days after last alert, return `False`, otherwise `True`.""" + if last_alert := await self.name_alerts.get(member.id): + last_alert = datetime.utcfromtimestamp(last_alert) + if datetime.utcnow() - timedelta(days=DAYS_BETWEEN_ALERTS) < last_alert: + log.trace(f"Last alert was too recent for {member}'s nickname.") + return False + + return True + + async def check_bad_words_in_name(self, member: Member) -> None: + """Send a mod alert every 3 days if a username still matches a watchlist pattern.""" + # Use lock to avoid race conditions + async with self.name_lock: + # Check whether the users display name contains any words in our blacklist + matches = self.get_name_matches(member.display_name) + + if not matches or not await self.check_send_alert(member): + return + + log.info(f"Sending bad nickname alert for '{member.display_name}' ({member.id}).") + + log_string = ( + f"**User:** {member.mention} (`{member.id}`)\n" + f"**Display Name:** {member.display_name}\n" + f"**Bad Matches:** {', '.join(match.group() for match in matches)}" + ) + + await self.mod_log.send_log_message( + icon_url=Icons.token_removed, + colour=Colours.soft_red, + title="Username filtering alert", + text=log_string, + channel_id=Channels.mod_alerts, + thumbnail=member.avatar_url + ) + + # Update time when alert sent + await self.name_alerts.set(member.id, datetime.utcnow().timestamp()) + async def _filter_message(self, msg: Message, delta: Optional[int] = None) -> None: """Filter the input message to see if it violates any of our rules, and then respond accordingly.""" # Should we filter this message? @@ -183,6 +257,20 @@ class Filtering(Cog): if _filter["user_notification"]: await self.notify_member(msg.author, _filter["notification_msg"], msg.channel) + # If the message is classed as offensive, we store it in the site db and + # it will be deleted it after one week. + if _filter["schedule_deletion"] and not is_private: + delete_date = (msg.created_at + OFFENSIVE_MSG_DELETE_TIME).isoformat() + data = { + 'id': msg.id, + 'channel_id': msg.channel.id, + 'delete_date': delete_date + } + + await self.bot.api_client.post('bot/offensive-messages', json=data) + self.schedule_task(msg.id, data) + log.trace(f"Offensive message {msg.id} will be deleted on {delete_date}") + if is_private: channel_str = "via DM" else: @@ -300,7 +388,7 @@ class Filtering(Cog): Attempts to catch some of common ways to try to cheat the system. """ - # Remove backslashes to prevent escape character aroundfuckery like + # Remove backslashes to prevent escape character around fuckery like # discord\.gg/gdudes-pony-farm text = text.replace("\\", "") @@ -369,6 +457,46 @@ class Filtering(Cog): except discord.errors.Forbidden: await channel.send(f"{filtered_member.mention} {reason}") + async def _scheduled_task(self, msg: dict) -> None: + """Delete an offensive message once its deletion date is reached.""" + delete_at = dateutil.parser.isoparse(msg['delete_date']).replace(tzinfo=None) + + await wait_until(delete_at) + await self.delete_offensive_msg(msg) + + async def reschedule_offensive_msg_deletion(self) -> None: + """Get all the pending message deletion from the API and reschedule them.""" + await self.bot.wait_until_ready() + response = await self.bot.api_client.get('bot/offensive-messages',) + + now = datetime.utcnow() + + for msg in response: + delete_at = dateutil.parser.isoparse(msg['delete_date']).replace(tzinfo=None) + + if delete_at < now: + await self.delete_offensive_msg(msg) + else: + self.schedule_task(msg['id'], msg) + + async def delete_offensive_msg(self, msg: Mapping[str, str]) -> None: + """Delete an offensive message, and then delete it from the db.""" + try: + channel = self.bot.get_channel(msg['channel_id']) + if channel: + msg_obj = await channel.fetch_message(msg['id']) + await msg_obj.delete() + except NotFound: + log.info( + f"Tried to delete message {msg['id']}, but the message can't be found " + f"(it has been probably already deleted)." + ) + except HTTPException as e: + log.warning(f"Failed to delete message {msg['id']}: status {e.status}") + + await self.bot.api_client.delete(f'bot/offensive-messages/{msg["id"]}') + log.info(f"Deleted the offensive message with id {msg['id']}.") + def setup(bot: Bot) -> None: """Load the Filtering cog.""" diff --git a/bot/cogs/help_channels.py b/bot/cogs/help_channels.py index 70cef339a..187adfe51 100644 --- a/bot/cogs/help_channels.py +++ b/bot/cogs/help_channels.py @@ -5,8 +5,7 @@ import logging import random import typing as t from collections import deque -from contextlib import suppress -from datetime import datetime +from datetime import datetime, timedelta, timezone from pathlib import Path import discord @@ -15,6 +14,7 @@ from discord.ext import commands from bot import constants from bot.bot import Bot +from bot.utils import RedisCache from bot.utils.checks import with_role_check from bot.utils.scheduling import Scheduler @@ -22,7 +22,7 @@ log = logging.getLogger(__name__) ASKING_GUIDE_URL = "https://pythondiscord.com/pages/asking-good-questions/" MAX_CHANNELS_PER_CATEGORY = 50 -EXCLUDED_CHANNELS = (constants.Channels.how_to_get_help,) +EXCLUDED_CHANNELS = (constants.Channels.how_to_get_help, constants.Channels.cooldown) HELP_CHANNEL_TOPIC = """ This is a Python help channel. You can claim your own help channel in the Python Help: Available category. @@ -99,13 +99,24 @@ class HelpChannels(Scheduler, commands.Cog): Help channels are named after the chemical elements in `bot/resources/elements.json`. """ + # This cache tracks which channels are claimed by which members. + # RedisCache[discord.TextChannel.id, t.Union[discord.User.id, discord.Member.id]] + help_channel_claimants = RedisCache() + + # This cache maps a help channel to whether it has had any + # activity other than the original claimant. True being no other + # activity and False being other activity. + # RedisCache[discord.TextChannel.id, bool] + unanswered = RedisCache() + + # This dictionary maps a help channel to the time it was claimed + # RedisCache[discord.TextChannel.id, UtcPosixTimestamp] + claim_times = RedisCache() + def __init__(self, bot: Bot): super().__init__() self.bot = bot - self.help_channel_claimants: ( - t.Dict[discord.TextChannel, t.Union[discord.Member, discord.User]] - ) = {} # Categories self.available_category: discord.CategoryChannel = None @@ -125,16 +136,6 @@ class HelpChannels(Scheduler, commands.Cog): self.on_message_lock = asyncio.Lock() self.init_task = self.bot.loop.create_task(self.init_cog()) - # Stats - - # This dictionary maps a help channel to the time it was claimed - self.claim_times: t.Dict[int, datetime] = {} - - # This dictionary maps a help channel to whether it has had any - # activity other than the original claimant. True being no other - # activity and False being other activity. - self.unanswered: t.Dict[int, bool] = {} - def cog_unload(self) -> None: """Cancel the init task and scheduled tasks when the cog unloads.""" log.trace("Cog unload: cancelling the init_cog task") @@ -197,7 +198,7 @@ class HelpChannels(Scheduler, commands.Cog): async def dormant_check(self, ctx: commands.Context) -> bool: """Return True if the user is the help channel claimant or passes the role check.""" - if self.help_channel_claimants.get(ctx.channel) == ctx.author: + if await self.help_channel_claimants.get(ctx.channel.id) == ctx.author.id: log.trace(f"{ctx.author} is the help channel claimant, passing the check for dormant.") self.bot.stats.incr("help.dormant_invoke.claimant") return True @@ -222,10 +223,11 @@ class HelpChannels(Scheduler, commands.Cog): log.trace("close command invoked; checking if the channel is in-use.") if ctx.channel.category == self.in_use_category: if await self.dormant_check(ctx): - with suppress(KeyError): - del self.help_channel_claimants[ctx.channel] + # Remove the claimant and the cooldown role + await self.help_channel_claimants.delete(ctx.channel.id) await self.remove_cooldown_role(ctx.author) + # Ignore missing task when cooldown has passed but the channel still isn't dormant. self.cancel_task(ctx.author.id, ignore_missing=True) @@ -284,6 +286,15 @@ class HelpChannels(Scheduler, commands.Cog): if channel.category_id == category.id and not self.is_excluded_channel(channel): yield channel + async def get_in_use_time(self, channel_id: int) -> t.Optional[timedelta]: + """Return the duration `channel_id` has been in use. Return None if it's not in use.""" + log.trace(f"Calculating in use time for channel {channel_id}.") + + claimed_timestamp = await self.claim_times.get(channel_id) + if claimed_timestamp: + claimed = datetime.utcfromtimestamp(claimed_timestamp) + return datetime.utcnow() - claimed + @staticmethod def get_names() -> t.List[str]: """ @@ -386,7 +397,7 @@ class HelpChannels(Scheduler, commands.Cog): log.trace("Initialising the cog.") await self.init_categories() - await self.reset_send_permissions() + await self.check_cooldowns() self.channel_queue = self.create_channel_queue() self.name_queue = self.create_name_queue() @@ -546,19 +557,17 @@ class HelpChannels(Scheduler, commands.Cog): self.bot.stats.incr(f"help.dormant_calls.{caller}") - if channel.id in self.claim_times: - claimed = self.claim_times[channel.id] - in_use_time = datetime.now() - claimed + in_use_time = await self.get_in_use_time(channel.id) + if in_use_time: self.bot.stats.timing("help.in_use_time", in_use_time) - if channel.id in self.unanswered: - if self.unanswered[channel.id]: - self.bot.stats.incr("help.sessions.unanswered") - else: - self.bot.stats.incr("help.sessions.answered") + unanswered = await self.unanswered.get(channel.id) + if unanswered: + self.bot.stats.incr("help.sessions.unanswered") + elif unanswered is not None: + self.bot.stats.incr("help.sessions.answered") log.trace(f"Position of #{channel} ({channel.id}) is actually {channel.position}.") - log.trace(f"Sending dormant message for #{channel} ({channel.id}).") embed = discord.Embed(description=DORMANT_MSG) await channel.send(embed=embed) @@ -637,17 +646,17 @@ class HelpChannels(Scheduler, commands.Cog): if self.is_in_category(channel, constants.Categories.help_in_use): log.trace(f"Checking if #{channel} ({channel.id}) has been answered.") - # Check if there is an entry in unanswered (does not persist across restarts) - if channel.id in self.unanswered: - claimant = self.help_channel_claimants.get(channel) - if not claimant: - # The mapping for this channel was lost, we can't do anything. + # Check if there is an entry in unanswered + if await self.unanswered.contains(channel.id): + claimant_id = await self.help_channel_claimants.get(channel.id) + if not claimant_id: + # The mapping for this channel doesn't exist, we can't do anything. return # Check the message did not come from the claimant - if claimant.id != message.author.id: + if claimant_id != message.author.id: # Mark the channel as answered - self.unanswered[channel.id] = False + await self.unanswered.set(channel.id, False) @commands.Cog.listener() async def on_message(self, message: discord.Message) -> None: @@ -680,12 +689,15 @@ class HelpChannels(Scheduler, commands.Cog): await self.move_to_in_use(channel) await self.revoke_send_permissions(message.author) # Add user with channel for dormant check. - self.help_channel_claimants[channel] = message.author + await self.help_channel_claimants.set(channel.id, message.author.id) self.bot.stats.incr("help.claimed") - self.claim_times[channel.id] = datetime.now() - self.unanswered[channel.id] = True + # Must use a timezone-aware datetime to ensure a correct POSIX timestamp. + timestamp = datetime.now(timezone.utc).timestamp() + await self.claim_times.set(channel.id, timestamp) + + await self.unanswered.set(channel.id, True) log.trace(f"Releasing on_message lock for {message.id}.") @@ -720,15 +732,28 @@ class HelpChannels(Scheduler, commands.Cog): msg = await self.get_last_message(channel) return self.match_bot_embed(msg, AVAILABLE_MSG) - async def reset_send_permissions(self) -> None: - """Reset send permissions in the Available category for claimants.""" - log.trace("Resetting send permissions in the Available category.") + async def check_cooldowns(self) -> None: + """Remove expired cooldowns and re-schedule active ones.""" + log.trace("Checking all cooldowns to remove or re-schedule them.") guild = self.bot.get_guild(constants.Guild.id) + cooldown = constants.HelpChannels.claim_minutes * 60 + + for channel_id, member_id in await self.help_channel_claimants.items(): + member = guild.get_member(member_id) + if not member: + continue # Member probably left the guild. + + in_use_time = await self.get_in_use_time(channel_id) - # TODO: replace with a persistent cache cause checking every member is quite slow - for member in guild.members: - if self.is_claimant(member): + if not in_use_time or in_use_time.seconds > cooldown: + # Remove the role if no claim time could be retrieved or if the cooldown expired. + # Since the channel is in the claimants cache, it is definitely strange for a time + # to not exist. However, it isn't a reason to keep the user stuck with a cooldown. await self.remove_cooldown_role(member) + else: + # The member is still on a cooldown; re-schedule it for the remaining time. + remaining = cooldown - in_use_time.seconds + await self.schedule_cooldown_expiration(member, remaining) async def add_cooldown_role(self, member: discord.Member) -> None: """Add the help cooldown role to `member`.""" @@ -781,11 +806,14 @@ class HelpChannels(Scheduler, commands.Cog): # Would mean the user somehow bypassed the lack of permissions (e.g. user is guild owner). self.cancel_task(member.id, ignore_missing=True) - timeout = constants.HelpChannels.claim_minutes * 60 - callback = self.remove_cooldown_role(member) + await self.schedule_cooldown_expiration(member, constants.HelpChannels.claim_minutes * 60) - log.trace(f"Scheduling {member}'s ({member.id}) send message permissions to be reinstated.") - self.schedule_task(member.id, TaskData(timeout, callback)) + async def schedule_cooldown_expiration(self, member: discord.Member, seconds: int) -> None: + """Schedule the cooldown role for `member` to be removed after a duration of `seconds`.""" + log.trace(f"Scheduling removal of {member}'s ({member.id}) cooldown.") + + callback = self.remove_cooldown_role(member) + self.schedule_task(member.id, TaskData(seconds, callback)) async def send_available_message(self, channel: discord.TextChannel) -> None: """Send the available message by editing a dormant message or sending a new message.""" diff --git a/bot/cogs/information.py b/bot/cogs/information.py index f0eb3a1ea..f0bd1afdb 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -6,7 +6,8 @@ from collections import Counter, defaultdict from string import Template from typing import Any, Mapping, Optional, Union -from discord import Colour, Embed, Member, Message, Role, Status, utils +from discord import ChannelType, Colour, Embed, Guild, Member, Message, Role, Status, utils +from discord.abc import GuildChannel from discord.ext.commands import BucketType, Cog, Context, Paginator, command, group from discord.utils import escape_markdown @@ -26,6 +27,49 @@ class Information(Cog): def __init__(self, bot: Bot): self.bot = bot + @staticmethod + def role_can_read(channel: GuildChannel, role: Role) -> bool: + """Return True if `role` can read messages in `channel`.""" + overwrites = channel.overwrites_for(role) + return overwrites.read_messages is True + + def get_staff_channel_count(self, guild: Guild) -> int: + """ + Get the number of channels that are staff-only. + + We need to know two things about a channel: + - Does the @everyone role have explicit read deny permissions? + - Do staff roles have explicit read allow permissions? + + If the answer to both of these questions is yes, it's a staff channel. + """ + channel_ids = set() + for channel in guild.channels: + if channel.type is ChannelType.category: + continue + + everyone_can_read = self.role_can_read(channel, guild.default_role) + + for role in constants.STAFF_ROLES: + role_can_read = self.role_can_read(channel, guild.get_role(role)) + if role_can_read and not everyone_can_read: + channel_ids.add(channel.id) + break + + return len(channel_ids) + + @staticmethod + def get_channel_type_counts(guild: Guild) -> str: + """Return the total amounts of the various types of channels in `guild`.""" + channel_counter = Counter(c.type for c in guild.channels) + channel_type_list = [] + for channel, count in channel_counter.items(): + channel_type = str(channel).title() + channel_type_list.append(f"{channel_type} channels: {count}") + + channel_type_list = sorted(channel_type_list) + return "\n".join(channel_type_list) + @with_role(*constants.MODERATION_ROLES) @command(name="roles") async def roles_info(self, ctx: Context) -> None: @@ -102,15 +146,16 @@ class Information(Cog): roles = len(ctx.guild.roles) member_count = ctx.guild.member_count - - # How many of each type of channel? - channels = Counter(c.type for c in ctx.guild.channels) - channel_counts = "".join(sorted(f"{str(ch).title()} channels: {channels[ch]}\n" for ch in channels)).strip() + channel_counts = self.get_channel_type_counts(ctx.guild) # How many of each user status? statuses = Counter(member.status for member in ctx.guild.members) embed = Embed(colour=Colour.blurple()) + # How many staff members and staff channels do we have? + staff_member_count = len(ctx.guild.get_role(constants.Roles.helpers).members) + staff_channel_count = self.get_staff_channel_count(ctx.guild) + # Because channel_counts lacks leading whitespace, it breaks the dedent if it's inserted directly by the # f-string. While this is correctly formated by Discord, it makes unit testing difficult. To keep the formatting # without joining a tuple of strings we can use a Template string to insert the already-formatted channel_counts @@ -122,12 +167,16 @@ class Information(Cog): Voice region: {region} Features: {features} - **Counts** + **Channel counts** + $channel_counts + Staff channels: {staff_channel_count} + + **Member counts** Members: {member_count:,} + Staff members: {staff_member_count} Roles: {roles} - $channel_counts - **Members** + **Member statuses** {constants.Emojis.status_online} {statuses[Status.online]:,} {constants.Emojis.status_idle} {statuses[Status.idle]:,} {constants.Emojis.status_dnd} {statuses[Status.dnd]:,} diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index 5bfaad796..3b28526b2 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -53,7 +53,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Permanent infractions @command() - async def warn(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def warn(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Warn a user for the given reason.""" infraction = await utils.post_infraction(ctx, user, "warning", reason, active=False) if infraction is None: @@ -62,12 +62,12 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user) @command() - async def kick(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def kick(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Kick a user for the given reason.""" await self.apply_kick(ctx, user, reason, active=False) @command() - async def ban(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def ban(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Permanently ban a user for the given reason and stop watching them with Big Brother.""" await self.apply_ban(ctx, user, reason) @@ -75,7 +75,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Temporary infractions @command(aliases=["mute"]) - async def tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: str = None) -> None: + async def tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: t.Optional[str] = None) -> None: """ Temporarily mute a user for the given reason and duration. @@ -94,7 +94,14 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_mute(ctx, user, reason, expires_at=duration) @command() - async def tempban(self, ctx: Context, user: FetchedMember, duration: Expiry, *, reason: str = None) -> None: + async def tempban( + self, + ctx: Context, + user: FetchedMember, + duration: Expiry, + *, + reason: t.Optional[str] = None + ) -> None: """ Temporarily ban a user for the given reason and duration. @@ -116,7 +123,7 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Permanent shadow infractions @command(hidden=True) - async def note(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def note(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Create a private note for a user with the given reason without notifying the user.""" infraction = await utils.post_infraction(ctx, user, "note", reason, hidden=True, active=False) if infraction is None: @@ -125,12 +132,12 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user) @command(hidden=True, aliases=['shadowkick', 'skick']) - async def shadow_kick(self, ctx: Context, user: Member, *, reason: str = None) -> None: + async def shadow_kick(self, ctx: Context, user: Member, *, reason: t.Optional[str] = None) -> None: """Kick a user for the given reason without notifying the user.""" await self.apply_kick(ctx, user, reason, hidden=True, active=False) @command(hidden=True, aliases=['shadowban', 'sban']) - async def shadow_ban(self, ctx: Context, user: FetchedMember, *, reason: str = None) -> None: + async def shadow_ban(self, ctx: Context, user: FetchedMember, *, reason: t.Optional[str] = None) -> None: """Permanently ban a user for the given reason without notifying the user.""" await self.apply_ban(ctx, user, reason, hidden=True) @@ -138,7 +145,13 @@ class Infractions(InfractionScheduler, commands.Cog): # region: Temporary shadow infractions @command(hidden=True, aliases=["shadowtempmute, stempmute", "shadowmute", "smute"]) - async def shadow_tempmute(self, ctx: Context, user: Member, duration: Expiry, *, reason: str = None) -> None: + async def shadow_tempmute( + self, ctx: Context, + user: Member, + duration: Expiry, + *, + reason: t.Optional[str] = None + ) -> None: """ Temporarily mute a user for the given reason and duration without notifying the user. @@ -163,7 +176,7 @@ class Infractions(InfractionScheduler, commands.Cog): user: FetchedMember, duration: Expiry, *, - reason: str = None + reason: t.Optional[str] = None ) -> None: """ Temporarily ban a user for the given reason and duration without notifying the user. @@ -198,7 +211,7 @@ class Infractions(InfractionScheduler, commands.Cog): # endregion # region: Base apply functions - async def apply_mute(self, ctx: Context, user: Member, reason: str, **kwargs) -> None: + async def apply_mute(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None: """Apply a mute infraction with kwargs passed to `post_infraction`.""" if await utils.get_active_infraction(ctx, user, "mute"): return @@ -218,7 +231,7 @@ class Infractions(InfractionScheduler, commands.Cog): await self.apply_infraction(ctx, infraction, user, action()) @respect_role_hierarchy() - async def apply_kick(self, ctx: Context, user: Member, reason: str, **kwargs) -> None: + async def apply_kick(self, ctx: Context, user: Member, reason: t.Optional[str], **kwargs) -> None: """Apply a kick infraction with kwargs passed to `post_infraction`.""" infraction = await utils.post_infraction(ctx, user, "kick", reason, active=False, **kwargs) if infraction is None: @@ -226,11 +239,14 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - action = user.kick(reason=textwrap.shorten(reason, width=512, placeholder="...")) + if reason: + reason = textwrap.shorten(reason, width=512, placeholder="...") + + action = user.kick(reason=reason) await self.apply_infraction(ctx, infraction, user, action) @respect_role_hierarchy() - async def apply_ban(self, ctx: Context, user: UserSnowflake, reason: str, **kwargs) -> None: + async def apply_ban(self, ctx: Context, user: UserSnowflake, reason: t.Optional[str], **kwargs) -> None: """ Apply a ban infraction with kwargs passed to `post_infraction`. @@ -259,9 +275,10 @@ class Infractions(InfractionScheduler, commands.Cog): self.mod_log.ignore(Event.member_remove, user.id) - truncated_reason = textwrap.shorten(reason, width=512, placeholder="...") + if reason: + reason = textwrap.shorten(reason, width=512, placeholder="...") - action = ctx.guild.ban(user, reason=truncated_reason, delete_message_days=0) + action = ctx.guild.ban(user, reason=reason, delete_message_days=0) await self.apply_infraction(ctx, infraction, user, action) if infraction.get('expires_at') is not None: @@ -281,7 +298,7 @@ class Infractions(InfractionScheduler, commands.Cog): # endregion # region: Base pardon functions - async def pardon_mute(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + async def pardon_mute(self, user_id: int, guild: discord.Guild, reason: t.Optional[str]) -> t.Dict[str, str]: """Remove a user's muted role, DM them a notification, and return a log dict.""" user = guild.get_member(user_id) log_text = {} @@ -307,7 +324,7 @@ class Infractions(InfractionScheduler, commands.Cog): return log_text - async def pardon_ban(self, user_id: int, guild: discord.Guild, reason: str) -> t.Dict[str, str]: + async def pardon_ban(self, user_id: int, guild: discord.Guild, reason: t.Optional[str]) -> t.Dict[str, str]: """Remove a user's ban on the Discord guild and return a log dict.""" user = discord.Object(user_id) log_text = {} diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 9d28030d9..41472c64c 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -555,6 +555,10 @@ class ModLog(Cog, name="ModLog"): channel = message.channel author = message.author + # Ignore DMs. + if not message.guild: + return + if message.guild.id != GuildConstant.id or channel.id in GuildConstant.modlog_blacklist: return diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index f0a3ad1b1..d75a72ddb 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -106,18 +106,38 @@ class InfractionScheduler(Scheduler): log_content = None failed = False + # DM the user about the infraction if it's not a shadow/hidden infraction. + # This needs to happen before we apply the infraction, as the bot cannot + # send DMs to user that it doesn't share a guild with. If we were to + # apply kick/ban infractions first, this would mean that we'd make it + # impossible for us to deliver a DM. See python-discord/bot#982. + if not infraction["hidden"]: + dm_result = f"{constants.Emojis.failmail} " + dm_log_text = "\nDM: **Failed**" + + # Sometimes user is a discord.Object; make it a proper user. + try: + if not isinstance(user, (discord.Member, discord.User)): + user = await self.bot.fetch_user(user.id) + except discord.HTTPException as e: + log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})") + else: + # Accordingly display whether the user was successfully notified via DM. + if await utils.notify_infraction(user, infr_type, expiry, reason, icon): + dm_result = ":incoming_envelope: " + dm_log_text = "\nDM: Sent" + + end_msg = "" if infraction["actor"] == self.bot.user.id: log.trace( f"Infraction #{id_} actor is bot; including the reason in the confirmation message." ) - - end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" + if reason: + end_msg = f" (reason: {textwrap.shorten(reason, width=1500, placeholder='...')})" elif ctx.channel.id not in STAFF_CHANNELS: log.trace( f"Infraction #{id_} context is not in a staff channel; omitting infraction count." ) - - end_msg = "" else: log.trace(f"Fetching total infraction count for {user}.") @@ -150,27 +170,7 @@ class InfractionScheduler(Scheduler): log.exception(log_msg) failed = True - # DM the user about the infraction if it's not a shadow/hidden infraction. - # Don't send DM when applying failed. - if not infraction["hidden"] and not failed: - dm_result = f"{constants.Emojis.failmail} " - dm_log_text = "\nDM: **Failed**" - - # Sometimes user is a discord.Object; make it a proper user. - try: - if not isinstance(user, (discord.Member, discord.User)): - user = await self.bot.fetch_user(user.id) - except discord.HTTPException as e: - log.error(f"Failed to DM {user.id}: could not fetch user (status {e.status})") - else: - # Accordingly display whether the user was successfully notified via DM. - if await utils.notify_infraction(user, infr_type, expiry, reason, icon): - dm_result = ":incoming_envelope: " - dm_log_text = "\nDM: Sent" - if failed: - dm_log_text = "\nDM: **Canceled**" - dm_result = f"{constants.Emojis.failmail} " log.trace(f"Deleted infraction {infraction['id']} from database because applying infraction failed.") try: await self.bot.api_client.delete(f"bot/infractions/{id_}") diff --git a/bot/cogs/moderation/silence.py b/bot/cogs/moderation/silence.py index 25febfa51..c8ab6443b 100644 --- a/bot/cogs/moderation/silence.py +++ b/bot/cogs/moderation/silence.py @@ -1,7 +1,7 @@ import asyncio import logging from contextlib import suppress -from typing import Optional +from typing import NamedTuple, Optional from discord import TextChannel from discord.ext import commands, tasks @@ -11,10 +11,18 @@ from bot.bot import Bot from bot.constants import Channels, Emojis, Guild, MODERATION_ROLES, Roles from bot.converters import HushDurationConverter from bot.utils.checks import with_role_check +from bot.utils.scheduling import Scheduler log = logging.getLogger(__name__) +class TaskData(NamedTuple): + """Data for a scheduled task.""" + + delay: int + ctx: Context + + class SilenceNotifier(tasks.Loop): """Loop notifier for posting notices to `alert_channel` containing added channels.""" @@ -53,15 +61,25 @@ class SilenceNotifier(tasks.Loop): await self._alert_channel.send(f"<@&{Roles.moderators}> currently silenced channels: {channels_text}") -class Silence(commands.Cog): +class Silence(Scheduler, commands.Cog): """Commands for stopping channel messages for `verified` role in a channel.""" def __init__(self, bot: Bot): + super().__init__() self.bot = bot self.muted_channels = set() self._get_instance_vars_task = self.bot.loop.create_task(self._get_instance_vars()) self._get_instance_vars_event = asyncio.Event() + async def _scheduled_task(self, task: TaskData) -> None: + """Calls `self.unsilence` on expired silenced channel to unsilence it.""" + await asyncio.sleep(task.delay) + log.info("Unsilencing channel after set delay.") + + # Because `self.unsilence` explicitly cancels this scheduled task, it is shielded + # to avoid prematurely cancelling itself + await asyncio.shield(task.ctx.invoke(self.unsilence)) + async def _get_instance_vars(self) -> None: """Get instance variables after they're available to get from the guild.""" await self.bot.wait_until_guild_available() @@ -90,9 +108,13 @@ class Silence(commands.Cog): return await ctx.send(f"{Emojis.check_mark} silenced current channel for {duration} minute(s).") - await asyncio.sleep(duration*60) - log.info("Unsilencing channel after set delay.") - await ctx.invoke(self.unsilence) + + task_data = TaskData( + delay=duration*60, + ctx=ctx + ) + + self.schedule_task(ctx.channel.id, task_data) @commands.command(aliases=("unhush",)) async def unsilence(self, ctx: Context) -> None: @@ -103,7 +125,9 @@ class Silence(commands.Cog): """ await self._get_instance_vars_event.wait() log.debug(f"Unsilencing channel #{ctx.channel} from {ctx.author}'s command.") - if await self._unsilence(ctx.channel): + if not await self._unsilence(ctx.channel): + await ctx.send(f"{Emojis.cross_mark} current channel was not silenced.") + else: await ctx.send(f"{Emojis.check_mark} unsilenced current channel.") async def _silence(self, channel: TextChannel, persistent: bool, duration: Optional[int]) -> bool: @@ -140,6 +164,7 @@ class Silence(commands.Cog): if current_overwrite.send_messages is False: await channel.set_permissions(self._verified_role, **dict(current_overwrite, send_messages=None)) log.info(f"Unsilenced channel #{channel} ({channel.id}).") + self.cancel_task(channel.id) self.notifier.remove_channel(channel) self.muted_channels.discard(channel) return True diff --git a/bot/cogs/python_news.py b/bot/cogs/python_news.py index d15d0371e..adefd5c7c 100644 --- a/bot/cogs/python_news.py +++ b/bot/cogs/python_news.py @@ -10,6 +10,7 @@ from discord.ext.tasks import loop from bot import constants from bot.bot import Bot +from bot.utils.messages import sub_clyde PEPS_RSS_URL = "https://www.python.org/dev/peps/peps.rss/" @@ -208,7 +209,7 @@ class PythonNews(Cog): return await self.webhook.send( embed=embed, - username=webhook_profile_name, + username=sub_clyde(webhook_profile_name), avatar_url=AVATAR_URL, wait=True ) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 3b77538a0..d853ab2ea 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -16,6 +16,7 @@ from bot.constants import Channels, ERROR_REPLIES, Emojis, Reddit as RedditConfi from bot.converters import Subreddit from bot.decorators import with_role from bot.pagination import LinePaginator +from bot.utils.messages import sub_clyde log = logging.getLogger(__name__) @@ -218,7 +219,8 @@ class Reddit(Cog): for subreddit in RedditConfig.subreddits: top_posts = await self.get_top_posts(subreddit=subreddit, time="day") - message = await self.webhook.send(username=f"{subreddit} Top Daily Posts", embed=top_posts, wait=True) + username = sub_clyde(f"{subreddit} Top Daily Posts") + message = await self.webhook.send(username=username, embed=top_posts, wait=True) if message.channel.is_news(): await message.publish() @@ -228,8 +230,8 @@ class Reddit(Cog): for subreddit in RedditConfig.subreddits: # Send and pin the new weekly posts. top_posts = await self.get_top_posts(subreddit=subreddit, time="week") - - message = await self.webhook.send(wait=True, username=f"{subreddit} Top Weekly Posts", embed=top_posts) + username = sub_clyde(f"{subreddit} Top Weekly Posts") + message = await self.webhook.send(wait=True, username=username, embed=top_posts) if subreddit.lower() == "r/python": if not self.channel: diff --git a/bot/cogs/site.py b/bot/cogs/site.py index 7fc2a9c34..ac29daa1d 100644 --- a/bot/cogs/site.py +++ b/bot/cogs/site.py @@ -33,7 +33,7 @@ class Site(Cog): embed.colour = Colour.blurple() embed.description = ( f"[Our official website]({url}) is an open-source community project " - "created with Python and Flask. It contains information about the server " + "created with Python and Django. It contains information about the server " "itself, lets you sign up for upcoming events, has its own wiki, contains " "a list of valuable learning resources, and much more." ) @@ -133,6 +133,9 @@ class Site(Cog): await ctx.send(f":x: Invalid rule indices: {indices}") return + for rule in rules: + self.bot.stats.incr(f"rule_uses.{rule}") + final_rules = tuple(f"**{pick}.** {full_rules[pick - 1]}" for pick in rules) await LinePaginator.paginate(final_rules, ctx, rules_embed, max_lines=3) diff --git a/bot/cogs/stats.py b/bot/cogs/stats.py index 4ebb6423c..d42f55466 100644 --- a/bot/cogs/stats.py +++ b/bot/cogs/stats.py @@ -36,7 +36,8 @@ class Stats(Cog): if message.guild.id != Guild.id: return - if message.channel.category.id == Categories.modmail: + cat = getattr(message.channel, "category", None) + if cat is not None and cat.id == Categories.modmail: if message.channel.id != Channels.incidents: # Do not report modmail channels to stats, there are too many # of them for interesting statistics to be drawn out of this. diff --git a/bot/cogs/sync/cog.py b/bot/cogs/sync/cog.py index 7cc3726b2..5ace957e7 100644 --- a/bot/cogs/sync/cog.py +++ b/bot/cogs/sync/cog.py @@ -34,18 +34,22 @@ class Sync(Cog): for syncer in (self.role_syncer, self.user_syncer): await syncer.sync(guild) - async def patch_user(self, user_id: int, updated_information: Dict[str, Any]) -> None: + async def patch_user(self, user_id: int, json: Dict[str, Any], ignore_404: bool = False) -> None: """Send a PATCH request to partially update a user in the database.""" try: - await self.bot.api_client.patch(f"bot/users/{user_id}", json=updated_information) + await self.bot.api_client.patch(f"bot/users/{user_id}", json=json) except ResponseCodeError as e: if e.response.status != 404: raise - log.warning("Unable to update user, got 404. Assuming race condition from join event.") + if not ignore_404: + log.warning("Unable to update user, got 404. Assuming race condition from join event.") @Cog.listener() async def on_guild_role_create(self, role: Role) -> None: """Adds newly create role to the database table over the API.""" + if role.guild.id != constants.Guild.id: + return + await self.bot.api_client.post( 'bot/roles', json={ @@ -60,11 +64,17 @@ class Sync(Cog): @Cog.listener() async def on_guild_role_delete(self, role: Role) -> None: """Deletes role from the database when it's deleted from the guild.""" + if role.guild.id != constants.Guild.id: + return + await self.bot.api_client.delete(f'bot/roles/{role.id}') @Cog.listener() async def on_guild_role_update(self, before: Role, after: Role) -> None: """Syncs role with the database if any of the stored attributes were updated.""" + if after.guild.id != constants.Guild.id: + return + was_updated = ( before.name != after.name or before.colour != after.colour @@ -93,6 +103,9 @@ class Sync(Cog): previously left), it will update the user's information. If the user is not yet known by the database, the user is added. """ + if member.guild.id != constants.Guild.id: + return + packed = { 'discriminator': int(member.discriminator), 'id': member.id, @@ -122,14 +135,20 @@ class Sync(Cog): @Cog.listener() async def on_member_remove(self, member: Member) -> None: """Set the in_guild field to False when a member leaves the guild.""" - await self.patch_user(member.id, updated_information={"in_guild": False}) + if member.guild.id != constants.Guild.id: + return + + await self.patch_user(member.id, json={"in_guild": False}) @Cog.listener() async def on_member_update(self, before: Member, after: Member) -> None: """Update the roles of the member in the database if a change is detected.""" + if after.guild.id != constants.Guild.id: + return + if before.roles != after.roles: updated_information = {"roles": sorted(role.id for role in after.roles)} - await self.patch_user(after.id, updated_information=updated_information) + await self.patch_user(after.id, json=updated_information) @Cog.listener() async def on_user_update(self, before: User, after: User) -> None: @@ -140,7 +159,8 @@ class Sync(Cog): "name": after.name, "discriminator": int(after.discriminator), } - await self.patch_user(after.id, updated_information=updated_information) + # A 404 likely means the user is in another guild. + await self.patch_user(after.id, json=updated_information, ignore_404=True) @commands.group(name='sync') @commands.has_permissions(administrator=True) diff --git a/bot/cogs/token_remover.py b/bot/cogs/token_remover.py index 6721f0e02..ef979f222 100644 --- a/bot/cogs/token_remover.py +++ b/bot/cogs/token_remover.py @@ -2,20 +2,22 @@ import base64 import binascii import logging import re -import struct import typing as t -from datetime import datetime -from discord import Colour, Message +from discord import Colour, Message, NotFound from discord.ext.commands import Cog -from discord.utils import snowflake_time +from bot import utils from bot.bot import Bot from bot.cogs.moderation import ModLog from bot.constants import Channels, Colours, Event, Icons log = logging.getLogger(__name__) +LOG_MESSAGE = ( + "Censored a seemingly valid token sent by {author} (`{author_id}`) in {channel}, " + "token was `{user_id}.{timestamp}.{hmac}`" +) DELETION_MESSAGE_TEMPLATE = ( "Hey {mention}! I noticed you posted a seemingly valid Discord API " "token in your message and have removed your message. " @@ -25,15 +27,22 @@ DELETION_MESSAGE_TEMPLATE = ( "Feel free to re-post it with the token removed. " "If you believe this was a mistake, please let us know!" ) -DISCORD_EPOCH_TIMESTAMP = datetime(2017, 1, 1) +DISCORD_EPOCH = 1_420_070_400 TOKEN_EPOCH = 1_293_840_000 -TOKEN_RE = re.compile( - r"[^\s\.()\"']+" # Matches token part 1: The user ID string, encoded as base64 - r"\." # Matches a literal dot between the token parts - r"[^\s\.()\"']+" # Matches token part 2: The creation timestamp, as an integer - r"\." # Matches a literal dot between the token parts - r"[^\s\.()\"']+" # Matches token part 3: The HMAC, unused by us, but check that it isn't empty -) + +# Three parts delimited by dots: user ID, creation timestamp, HMAC. +# The HMAC isn't parsed further, but it's in the regex to ensure it at least exists in the string. +# Each part only matches base64 URL-safe characters. +# Padding has never been observed, but the padding character '=' is matched just in case. +TOKEN_RE = re.compile(r"([\w\-=]+)\.([\w\-=]+)\.([\w\-=]+)", re.ASCII) + + +class Token(t.NamedTuple): + """A Discord Bot token.""" + + user_id: str + timestamp: str + hmac: str class TokenRemover(Cog): @@ -54,6 +63,10 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ + # Ignore DMs; can't delete messages in there anyway. + if not msg.guild or msg.author.bot: + return + found_token = self.find_token_in_message(msg) if found_token: await self.take_action(msg, found_token) @@ -65,64 +78,61 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ - found_token = self.find_token_in_message(after) - if found_token: - await self.take_action(after, found_token) + await self.on_message(after) - async def take_action(self, msg: Message, found_token: str) -> None: - """Remove the `msg` containing a token an send a mod_log message.""" - user_id, creation_timestamp, hmac = found_token.split('.') + async def take_action(self, msg: Message, found_token: Token) -> None: + """Remove the `msg` containing the `found_token` and send a mod log message.""" self.mod_log.ignore(Event.message_delete, msg.id) - await msg.delete() + + try: + await msg.delete() + except NotFound: + log.debug(f"Failed to remove token in message {msg.id}: message already deleted.") + return + await msg.channel.send(DELETION_MESSAGE_TEMPLATE.format(mention=msg.author.mention)) - message = ( - "Censored a seemingly valid token sent by " - f"{msg.author} (`{msg.author.id}`) in {msg.channel.mention}, token was " - f"`{user_id}.{creation_timestamp}.{'x' * len(hmac)}`" - ) - log.debug(message) + log_message = self.format_log_message(msg, found_token) + log.debug(log_message) # Send pretty mod log embed to mod-alerts await self.mod_log.send_log_message( icon_url=Icons.token_removed, colour=Colour(Colours.soft_red), title="Token removed!", - text=message, + text=log_message, thumbnail=msg.author.avatar_url_as(static_format="png"), channel_id=Channels.mod_alerts, ) self.bot.stats.incr("tokens.removed_tokens") + @staticmethod + def format_log_message(msg: Message, token: Token) -> str: + """Return the log message to send for `token` being censored in `msg`.""" + return LOG_MESSAGE.format( + author=msg.author, + author_id=msg.author.id, + channel=msg.channel.mention, + user_id=token.user_id, + timestamp=token.timestamp, + hmac='x' * len(token.hmac), + ) + @classmethod - def find_token_in_message(cls, msg: Message) -> t.Optional[str]: + def find_token_in_message(cls, msg: Message) -> t.Optional[Token]: """Return a seemingly valid token found in `msg` or `None` if no token is found.""" - if msg.author.bot: - return - - # Use findall rather than search to guard against method calls prematurely returning the + # Use finditer rather than search to guard against method calls prematurely returning the # token check (e.g. `message.channel.send` also matches our token pattern) - maybe_matches = TOKEN_RE.findall(msg.content) - for substr in maybe_matches: - if cls.is_maybe_token(substr): + for match in TOKEN_RE.finditer(msg.content): + token = Token(*match.groups()) + if cls.is_valid_user_id(token.user_id) and cls.is_valid_timestamp(token.timestamp): # Short-circuit on first match - return substr + return token # No matching substring return - @classmethod - def is_maybe_token(cls, test_str: str) -> bool: - """Check the provided string to see if it is a seemingly valid token.""" - try: - user_id, creation_timestamp, hmac = test_str.split('.') - except ValueError: - return False - - if cls.is_valid_user_id(user_id) and cls.is_valid_timestamp(creation_timestamp): - return True - @staticmethod def is_valid_user_id(b64_content: str) -> bool: """ @@ -130,29 +140,41 @@ class TokenRemover(Cog): See: https://discordapp.com/developers/docs/reference#snowflakes """ - b64_content += '=' * (-len(b64_content) % 4) + b64_content = utils.pad_base64(b64_content) try: - content: bytes = base64.b64decode(b64_content) - return content.decode('utf-8').isnumeric() - except (binascii.Error, UnicodeDecodeError): + decoded_bytes = base64.urlsafe_b64decode(b64_content) + string = decoded_bytes.decode('utf-8') + + # isdigit on its own would match a lot of other Unicode characters, hence the isascii. + return string.isascii() and string.isdigit() + except (binascii.Error, ValueError): return False @staticmethod def is_valid_timestamp(b64_content: str) -> bool: """ - Check potential token to see if it contains a valid timestamp. + Return True if `b64_content` decodes to a valid timestamp. - See: https://discordapp.com/developers/docs/reference#snowflakes + If the timestamp is greater than the Discord epoch, it's probably valid. + See: https://i.imgur.com/7WdehGn.png """ - b64_content += '=' * (-len(b64_content) % 4) + b64_content = utils.pad_base64(b64_content) try: - content = base64.urlsafe_b64decode(b64_content) - snowflake = struct.unpack('i', content)[0] - except (binascii.Error, struct.error): + decoded_bytes = base64.urlsafe_b64decode(b64_content) + timestamp = int.from_bytes(decoded_bytes, byteorder="big") + except (binascii.Error, ValueError) as e: + log.debug(f"Failed to decode token timestamp '{b64_content}': {e}") + return False + + # Seems like newer tokens don't need the epoch added, but add anyway since an upper bound + # is not checked. + if timestamp + TOKEN_EPOCH >= DISCORD_EPOCH: + return True + else: + log.debug(f"Invalid token timestamp '{b64_content}': smaller than Discord epoch") return False - return snowflake_time(snowflake + TOKEN_EPOCH) < DISCORD_EPOCH_TIMESTAMP def setup(bot: Bot) -> None: diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index 73b4a1c0a..697bf60ce 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -6,7 +6,7 @@ from email.parser import HeaderParser from io import StringIO from typing import Tuple, Union -from discord import Colour, Embed +from discord import Colour, Embed, utils from discord.ext.commands import BadArgument, Cog, Context, command from bot.bot import Bot @@ -145,7 +145,7 @@ class Utils(Cog): u_code = f"\\U{digit:>08}" url = f"https://www.compart.com/en/unicode/U+{digit:>04}" name = f"[{unicodedata.name(char, '')}]({url})" - info = f"`{u_code.ljust(10)}`: {name} - {char}" + info = f"`{u_code.ljust(10)}`: {name} - {utils.escape_markdown(char)}" return info, u_code charlist, rawlist = zip(*(get_info(c) for c in characters)) diff --git a/bot/cogs/watchchannels/watchchannel.py b/bot/cogs/watchchannels/watchchannel.py index 436778c46..7c58a0fb5 100644 --- a/bot/cogs/watchchannels/watchchannel.py +++ b/bot/cogs/watchchannels/watchchannel.py @@ -204,6 +204,7 @@ class WatchChannel(metaclass=CogABCMeta): embed: Optional[Embed] = None, ) -> None: """Sends a message to the webhook with the specified kwargs.""" + username = messages.sub_clyde(username) try: await self.webhook.send(content=content, username=username, avatar_url=avatar_url, embed=embed) except discord.HTTPException as exc: diff --git a/bot/cogs/webhook_remover.py b/bot/cogs/webhook_remover.py index 1b5c3f821..543869215 100644 --- a/bot/cogs/webhook_remover.py +++ b/bot/cogs/webhook_remover.py @@ -1,7 +1,7 @@ import logging import re -from discord import Colour, Message +from discord import Colour, Message, NotFound from discord.ext.commands import Cog from bot.bot import Bot @@ -35,7 +35,13 @@ class WebhookRemover(Cog): """Delete `msg` and send a warning that it contained the Discord webhook `redacted_url`.""" # Don't log this, due internal delete, not by user. Will make different entry. self.mod_log.ignore(Event.message_delete, msg.id) - await msg.delete() + + try: + await msg.delete() + except NotFound: + log.debug(f"Failed to remove webhook in message {msg.id}: message already deleted.") + return + await msg.channel.send(ALERT_MESSAGE_TEMPLATE.format(user=msg.author.mention)) message = ( @@ -59,6 +65,10 @@ class WebhookRemover(Cog): @Cog.listener() async def on_message(self, msg: Message) -> None: """Check if a Discord webhook URL is in `message`.""" + # Ignore DMs; can't delete messages in there anyway. + if not msg.guild or msg.author.bot: + return + matches = WEBHOOK_URL_RE.search(msg.content) if matches: await self.delete_and_respond(msg, matches[1] + "xxx") diff --git a/bot/constants.py b/bot/constants.py index b31a9c99e..a1b392c82 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -226,6 +226,7 @@ class Filter(metaclass=YAMLGetter): notify_user_domains: bool ping_everyone: bool + offensive_msg_delete_days: int guild_invite_whitelist: List[int] domain_blacklist: List[str] word_watchlist: List[str] @@ -389,6 +390,7 @@ class Channels(metaclass=YAMLGetter): attachment_log: int big_brother_logs: int bot_commands: int + cooldown: int defcon: int dev_contrib: int dev_core: int diff --git a/bot/converters.py b/bot/converters.py index 72c46fdf0..4deb59f87 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -217,7 +217,10 @@ class Duration(Converter): delta = relativedelta(**duration_dict) now = datetime.utcnow() - return now + delta + try: + return now + delta + except ValueError: + raise BadArgument(f"`{duration}` results in a datetime outside the supported range.") class ISODateTime(Converter): diff --git a/bot/resources/tags/customcooldown.md b/bot/resources/tags/customcooldown.md new file mode 100644 index 000000000..ac7e70aee --- /dev/null +++ b/bot/resources/tags/customcooldown.md @@ -0,0 +1,20 @@ +**Cooldowns in discord.py** + +Cooldowns can be used in discord.py to rate-limit. In this example, we're using it in an on_message. + +```python +from discord.ext import commands + +message_cooldown = commands.CooldownMapping.from_cooldown(1.0, 60.0, commands.BucketType.user) + +async def on_message(message): + bucket = message_cooldown.get_bucket(message) + retry_after = bucket.update_rate_limit() + if retry_after: + await message.channel.send(f"Slow down! Try again in {retry_after} seconds.") + else: + await message.channel.send("Not ratelimited!") +``` + +`from_cooldown` takes the amount of `update_rate_limit()`s needed to trigger the cooldown, the time in which the cooldown is triggered, and a [`BucketType`](https://discordpy.readthedocs.io/en/latest/ext/commands/api.html#discord.discord.ext.commands.BucketType). diff --git a/bot/utils/__init__.py b/bot/utils/__init__.py index c5a12d5e3..5a6e1811b 100644 --- a/bot/utils/__init__.py +++ b/bot/utils/__init__.py @@ -11,3 +11,8 @@ class CogABCMeta(CogMeta, ABCMeta): """Metaclass for ABCs meant to be implemented as Cogs.""" pass + + +def pad_base64(data: str) -> str: + """Return base64 `data` with padding characters to ensure its length is a multiple of 4.""" + return data + "=" * (-len(data) % 4) diff --git a/bot/utils/messages.py b/bot/utils/messages.py index de8e186f3..a40a12e98 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -1,6 +1,7 @@ import asyncio import contextlib import logging +import re from io import BytesIO from typing import List, Optional, Sequence, Union @@ -86,7 +87,7 @@ async def send_attachments( else: await destination.send( file=attachment_file, - username=message.author.display_name, + username=sub_clyde(message.author.display_name), avatar_url=message.author.avatar_url ) elif link_large: @@ -97,7 +98,7 @@ async def send_attachments( if link_large and e.status == 413: large.append(attachment) else: - log.warning(f"{failure_msg} with status {e.status}.") + log.warning(f"{failure_msg} with status {e.status}.", exc_info=e) if link_large and large: desc = "\n".join(f"[{attachment.filename}]({attachment.url})" for attachment in large) @@ -109,8 +110,25 @@ async def send_attachments( else: await destination.send( embed=embed, - username=message.author.display_name, + username=sub_clyde(message.author.display_name), avatar_url=message.author.avatar_url ) return urls + + +def sub_clyde(username: Optional[str]) -> Optional[str]: + """ + Replace "e"/"E" in any "clyde" in `username` with a Cyrillic "е"/"E" and return the new string. + + Discord disallows "clyde" anywhere in the username for webhooks. It will return a 400. + Return None only if `username` is None. + """ + def replace_e(match: re.Match) -> str: + char = "е" if match[2] == "e" else "Е" + return match[1] + char + + if username: + return re.sub(r"(clyd)(e)", replace_e, username, flags=re.I) + else: + return username # Empty string or None diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index de80cee84..58cfe1df5 100644 --- a/bot/utils/redis_cache.py +++ b/bot/utils/redis_cache.py @@ -11,7 +11,7 @@ log = logging.getLogger(__name__) # Type aliases RedisKeyType = Union[str, int] -RedisValueType = Union[str, int, float] +RedisValueType = Union[str, int, float, bool] RedisKeyOrValue = Union[RedisKeyType, RedisValueType] # Prefix tuples @@ -20,6 +20,7 @@ _VALUE_PREFIXES = ( ("f|", float), ("i|", int), ("s|", str), + ("b|", bool), ) _KEY_PREFIXES = ( ("i|", int), @@ -47,8 +48,8 @@ class RedisCache: behaves, and should be familiar to Python users. The biggest difference is that all the public methods in this class are coroutines, and must be awaited. - Because of limitations in Redis, this cache will only accept strings, integers and - floats both for keys and values. + Because of limitations in Redis, this cache will only accept strings and integers for keys, + and strings, integers, floats and booleans for values. Please note that this class MUST be created as a class attribute, and that that class must also contain an attribute with an instance of our Bot. See `__get__` and `__set_name__` @@ -100,16 +101,7 @@ class RedisCache: def _set_namespace(self, namespace: str) -> None: """Try to set the namespace, but do not permit collisions.""" - # We need a unique namespace, to prevent collisions. This loop - # will try appending underscores to the end of the namespace until - # it finds one that is unique. - # - # For example, if `john` and `john_` are both taken, the namespace will - # be `john__` at the end of this loop. - while namespace in self._namespaces: - namespace += "_" - - log.trace(f"RedisCache setting namespace to {self._namespace}") + log.trace(f"RedisCache setting namespace to {namespace}") self._namespaces.append(namespace) self._namespace = namespace @@ -117,8 +109,15 @@ class RedisCache: def _to_typestring(key_or_value: RedisKeyOrValue, prefixes: _PrefixTuple) -> str: """Turn a valid Redis type into a typestring.""" for prefix, _type in prefixes: - if isinstance(key_or_value, _type): + # Convert bools into integers before storing them. + if type(key_or_value) is bool: + bool_int = int(key_or_value) + return f"{prefix}{bool_int}" + + # isinstance is a bad idea here, because isintance(False, int) == True. + if type(key_or_value) is _type: return f"{prefix}{key_or_value}" + raise TypeError(f"RedisCache._to_typestring only supports the following: {prefixes}.") @staticmethod @@ -131,6 +130,13 @@ class RedisCache: # Now we convert our unicode string back into the type it originally was. for prefix, _type in prefixes: if key_or_value.startswith(prefix): + + # For booleans, we need special handling because bool("False") is True. + if prefix == "b|": + value = key_or_value[len(prefix):] + return bool(int(value)) + + # Otherwise we can just convert normally. return _type(key_or_value[len(prefix):]) raise TypeError(f"RedisCache._from_typestring only supports the following: {prefixes}.") diff --git a/config-default.yml b/config-default.yml index 2c85f5ef3..64c4e715b 100644 --- a/config-default.yml +++ b/config-default.yml @@ -142,6 +142,7 @@ guild: # Python Help: Available how_to_get_help: 704250143020417084 + cooldown: 720603994149486673 # Logs attachment_log: &ATTACH_LOG 649243850006855680 @@ -268,7 +269,8 @@ filter: notify_user_domains: false # Filter configuration - ping_everyone: true # Ping @everyone when we send a mod-alert? + ping_everyone: true # Ping @everyone when we send a mod-alert? + offensive_msg_delete_days: 7 # How many days before deleting an offensive message? guild_invite_whitelist: - 280033776820813825 # Functional Programming @@ -297,6 +299,8 @@ filter: - 613425648685547541 # Discord Developers - 185590609631903755 # Blender Hub - 420324994703163402 # /r/FlutterDev + - 488751051629920277 # Python Atlanta + - 143867839282020352 # C# domain_blacklist: - pornhub.com @@ -328,6 +332,7 @@ filter: - ssteam.site - steamwalletgift.com - discord.gift + - lmgtfy.com word_watchlist: - goo+ks* diff --git a/docker-compose.yml b/docker-compose.yml index 9884e35f0..cff7d33d6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -17,6 +17,14 @@ services: ports: - "127.0.0.1:6379:6379" + snekbox: + image: pythondiscord/snekbox:latest + init: true + ipc: none + ports: + - "127.0.0.1:8060:8060" + privileged: true + web: image: pythondiscord/site:latest command: ["run", "--debug"] @@ -47,6 +55,7 @@ services: depends_on: - web - redis + - snekbox environment: BOT_TOKEN: ${BOT_TOKEN} BOT_API_KEY: badbot13m0n8f570f942013fc818f234916ca531 diff --git a/tests/bot/cogs/moderation/test_silence.py b/tests/bot/cogs/moderation/test_silence.py index 3fd149f04..ab3d0742a 100644 --- a/tests/bot/cogs/moderation/test_silence.py +++ b/tests/bot/cogs/moderation/test_silence.py @@ -127,10 +127,20 @@ class SilenceTests(unittest.IsolatedAsyncioTestCase): self.ctx.reset_mock() async def test_unsilence_sent_correct_discord_message(self): - """Proper reply after a successful unsilence.""" - with mock.patch.object(self.cog, "_unsilence", return_value=True): - await self.cog.unsilence.callback(self.cog, self.ctx) - self.ctx.send.assert_called_once_with(f"{Emojis.check_mark} unsilenced current channel.") + """Check if proper message was sent when unsilencing channel.""" + test_cases = ( + (True, f"{Emojis.check_mark} unsilenced current channel."), + (False, f"{Emojis.cross_mark} current channel was not silenced.") + ) + for _unsilence_patch_return, result_message in test_cases: + with self.subTest( + starting_silenced_state=_unsilence_patch_return, + result_message=result_message + ): + with mock.patch.object(self.cog, "_unsilence", return_value=_unsilence_patch_return): + await self.cog.unsilence.callback(self.cog, self.ctx) + self.ctx.send.assert_called_once_with(result_message) + self.ctx.reset_mock() async def test_silence_private_for_false(self): """Permissions are not set and `False` is returned in an already silenced channel.""" diff --git a/tests/bot/cogs/sync/test_cog.py b/tests/bot/cogs/sync/test_cog.py index 14fd909c4..120bc991d 100644 --- a/tests/bot/cogs/sync/test_cog.py +++ b/tests/bot/cogs/sync/test_cog.py @@ -131,6 +131,15 @@ class SyncCogListenerTests(SyncCogTestCase): super().setUp() self.cog.patch_user = mock.AsyncMock(spec_set=self.cog.patch_user) + self.guild_id_patcher = mock.patch("bot.cogs.sync.cog.constants.Guild.id", 5) + self.guild_id = self.guild_id_patcher.start() + + self.guild = helpers.MockGuild(id=self.guild_id) + self.other_guild = helpers.MockGuild(id=0) + + def tearDown(self): + self.guild_id_patcher.stop() + async def test_sync_cog_on_guild_role_create(self): """A POST request should be sent with the new role's data.""" self.assertTrue(self.cog.on_guild_role_create.__cog_listener__) @@ -142,20 +151,32 @@ class SyncCogListenerTests(SyncCogTestCase): "permissions": 8, "position": 23, } - role = helpers.MockRole(**role_data) + role = helpers.MockRole(**role_data, guild=self.guild) await self.cog.on_guild_role_create(role) self.bot.api_client.post.assert_called_once_with("bot/roles", json=role_data) + async def test_sync_cog_on_guild_role_create_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=self.other_guild) + await self.cog.on_guild_role_create(role) + self.bot.api_client.post.assert_not_awaited() + async def test_sync_cog_on_guild_role_delete(self): """A DELETE request should be sent.""" self.assertTrue(self.cog.on_guild_role_delete.__cog_listener__) - role = helpers.MockRole(id=99) + role = helpers.MockRole(id=99, guild=self.guild) await self.cog.on_guild_role_delete(role) self.bot.api_client.delete.assert_called_once_with("bot/roles/99") + async def test_sync_cog_on_guild_role_delete_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=self.other_guild) + await self.cog.on_guild_role_delete(role) + self.bot.api_client.delete.assert_not_awaited() + async def test_sync_cog_on_guild_role_update(self): """A PUT request should be sent if the colour, name, permissions, or position changes.""" self.assertTrue(self.cog.on_guild_role_update.__cog_listener__) @@ -180,8 +201,8 @@ class SyncCogListenerTests(SyncCogTestCase): after_role_data = role_data.copy() after_role_data[attribute] = 876 - before_role = helpers.MockRole(**role_data) - after_role = helpers.MockRole(**after_role_data) + before_role = helpers.MockRole(**role_data, guild=self.guild) + after_role = helpers.MockRole(**after_role_data, guild=self.guild) await self.cog.on_guild_role_update(before_role, after_role) @@ -193,31 +214,43 @@ class SyncCogListenerTests(SyncCogTestCase): else: self.bot.api_client.put.assert_not_called() + async def test_sync_cog_on_guild_role_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + role = helpers.MockRole(guild=self.other_guild) + await self.cog.on_guild_role_update(role, role) + self.bot.api_client.put.assert_not_awaited() + async def test_sync_cog_on_member_remove(self): - """Member should patched to set in_guild as False.""" + """Member should be patched to set in_guild as False.""" self.assertTrue(self.cog.on_member_remove.__cog_listener__) - member = helpers.MockMember() + member = helpers.MockMember(guild=self.guild) await self.cog.on_member_remove(member) self.cog.patch_user.assert_called_once_with( member.id, - updated_information={"in_guild": False} + json={"in_guild": False} ) + async def test_sync_cog_on_member_remove_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=self.other_guild) + await self.cog.on_member_remove(member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_member_update_roles(self): """Members should be patched if their roles have changed.""" self.assertTrue(self.cog.on_member_update.__cog_listener__) # Roles are intentionally unsorted. before_roles = [helpers.MockRole(id=12), helpers.MockRole(id=30), helpers.MockRole(id=20)] - before_member = helpers.MockMember(roles=before_roles) - after_member = helpers.MockMember(roles=before_roles[1:]) + before_member = helpers.MockMember(roles=before_roles, guild=self.guild) + after_member = helpers.MockMember(roles=before_roles[1:], guild=self.guild) await self.cog.on_member_update(before_member, after_member) data = {"roles": sorted(role.id for role in after_member.roles)} - self.cog.patch_user.assert_called_once_with(after_member.id, updated_information=data) + self.cog.patch_user.assert_called_once_with(after_member.id, json=data) async def test_sync_cog_on_member_update_other(self): """Members should not be patched if other attributes have changed.""" @@ -233,13 +266,19 @@ class SyncCogListenerTests(SyncCogTestCase): with self.subTest(attribute=attribute): self.cog.patch_user.reset_mock() - before_member = helpers.MockMember(**{attribute: old_value}) - after_member = helpers.MockMember(**{attribute: new_value}) + before_member = helpers.MockMember(**{attribute: old_value}, guild=self.guild) + after_member = helpers.MockMember(**{attribute: new_value}, guild=self.guild) await self.cog.on_member_update(before_member, after_member) self.cog.patch_user.assert_not_called() + async def test_sync_cog_on_member_update_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=self.other_guild) + await self.cog.on_member_update(member, member) + self.cog.patch_user.assert_not_awaited() + async def test_sync_cog_on_user_update(self): """A user should be patched only if the name, discriminator, or avatar changes.""" self.assertTrue(self.cog.on_user_update.__cog_listener__) @@ -272,12 +311,15 @@ class SyncCogListenerTests(SyncCogTestCase): # Don't care if *all* keys are present; only the changed one is required call_args = self.cog.patch_user.call_args - self.assertEqual(call_args[0][0], after_user.id) - self.assertIn("updated_information", call_args[1]) + self.assertEqual(call_args.args[0], after_user.id) + self.assertIn("json", call_args.kwargs) + + self.assertIn("ignore_404", call_args.kwargs) + self.assertTrue(call_args.kwargs["ignore_404"]) - updated_information = call_args[1]["updated_information"] - self.assertIn(api_field, updated_information) - self.assertEqual(updated_information[api_field], api_value) + json = call_args.kwargs["json"] + self.assertIn(api_field, json) + self.assertEqual(json[api_field], api_value) else: self.cog.patch_user.assert_not_called() @@ -290,6 +332,7 @@ class SyncCogListenerTests(SyncCogTestCase): member = helpers.MockMember( discriminator="1234", roles=[helpers.MockRole(id=22), helpers.MockRole(id=12)], + guild=self.guild, ) data = { @@ -334,6 +377,13 @@ class SyncCogListenerTests(SyncCogTestCase): self.bot.api_client.post.assert_not_called() + async def test_sync_cog_on_member_join_ignores_guilds(self): + """Events from other guilds should be ignored.""" + member = helpers.MockMember(guild=self.other_guild) + await self.cog.on_member_join(member) + self.bot.api_client.post.assert_not_awaited() + self.bot.api_client.put.assert_not_awaited() + class SyncCogCommandTests(SyncCogTestCase, CommandTestCase): """Tests for the commands in the Sync cog.""" diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index aca6b594f..79c0e0ad3 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -148,14 +148,18 @@ class InformationCogTests(unittest.TestCase): Voice region: {self.ctx.guild.region} Features: {', '.join(self.ctx.guild.features)} - **Counts** - Members: {self.ctx.guild.member_count:,} - Roles: {len(self.ctx.guild.roles)} + **Channel counts** Category channels: 1 Text channels: 1 Voice channels: 1 + Staff channels: 0 + + **Member counts** + Members: {self.ctx.guild.member_count:,} + Staff members: 0 + Roles: {len(self.ctx.guild.roles)} - **Members** + **Member statuses** {constants.Emojis.status_online} 2 {constants.Emojis.status_idle} 1 {constants.Emojis.status_dnd} 4 diff --git a/tests/bot/cogs/test_logging.py b/tests/bot/cogs/test_logging.py new file mode 100644 index 000000000..8a18fdcd6 --- /dev/null +++ b/tests/bot/cogs/test_logging.py @@ -0,0 +1,32 @@ +import unittest +from unittest.mock import patch + +from bot import constants +from bot.cogs.logging import Logging +from tests.helpers import MockBot, MockTextChannel + + +class LoggingTests(unittest.IsolatedAsyncioTestCase): + """Test cases for connected login.""" + + def setUp(self): + self.bot = MockBot() + self.cog = Logging(self.bot) + self.dev_log = MockTextChannel(id=1234, name="dev-log") + + @patch("bot.cogs.logging.DEBUG_MODE", False) + async def test_debug_mode_false(self): + """Should send connected message to dev-log.""" + self.bot.get_channel.return_value = self.dev_log + + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_called_once_with(constants.Channels.dev_log) + self.dev_log.send.assert_awaited_once() + + @patch("bot.cogs.logging.DEBUG_MODE", True) + async def test_debug_mode_true(self): + """Should not send anything to dev-log.""" + await self.cog.startup_greeting() + self.bot.wait_until_guild_available.assert_awaited_once_with() + self.bot.get_channel.assert_not_called() diff --git a/tests/bot/cogs/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 33d1ec170..3349caa73 100644 --- a/tests/bot/cogs/test_token_remover.py +++ b/tests/bot/cogs/test_token_remover.py @@ -1,56 +1,89 @@ -import asyncio -import logging import unittest -from unittest.mock import AsyncMock, MagicMock +from re import Match +from unittest import mock +from unittest.mock import MagicMock -from discord import Colour +from discord import Colour, NotFound -from bot.cogs.token_remover import ( - DELETION_MESSAGE_TEMPLATE, - TokenRemover, - setup as setup_cog, -) -from bot.constants import Channels, Colours, Event, Icons -from tests.helpers import MockBot, MockMessage +from bot import constants +from bot.cogs import token_remover +from bot.cogs.moderation import ModLog +from bot.cogs.token_remover import Token, TokenRemover +from tests.helpers import MockBot, MockMessage, autospec -class TokenRemoverTests(unittest.TestCase): +class TokenRemoverTests(unittest.IsolatedAsyncioTestCase): """Tests the `TokenRemover` cog.""" def setUp(self): """Adds the cog, a bot, and a message to the instance for usage in tests.""" self.bot = MockBot() - self.bot.get_cog.return_value = MagicMock() - self.bot.get_cog.return_value.send_log_message = AsyncMock() self.cog = TokenRemover(bot=self.bot) - self.msg = MockMessage(id=555, content='') - self.msg.author.__str__ = MagicMock() - self.msg.author.__str__.return_value = 'lemon' - self.msg.author.bot = False - self.msg.author.avatar_url_as.return_value = 'picture-lemon.png' - self.msg.author.id = 42 - self.msg.author.mention = '@lemon' + self.msg = MockMessage(id=555, content="hello world") self.msg.channel.mention = "#lemonade-stand" + self.msg.author.__str__ = MagicMock(return_value=self.msg.author.name) + self.msg.author.avatar_url_as.return_value = "picture-lemon.png" - def test_is_valid_user_id_is_true_for_numeric_content(self): - """A string decoding to numeric characters is a valid user ID.""" - # MTIz = base64(123) - self.assertTrue(TokenRemover.is_valid_user_id('MTIz')) + def test_is_valid_user_id_valid(self): + """Should consider user IDs valid if they decode entirely to ASCII digits.""" + ids = ( + "NDcyMjY1OTQzMDYyNDEzMzMy", + "NDc1MDczNjI5Mzk5NTQ3OTA0", + "NDY3MjIzMjMwNjUwNzc3NjQx", + ) + + for user_id in ids: + with self.subTest(user_id=user_id): + result = TokenRemover.is_valid_user_id(user_id) + self.assertTrue(result) - def test_is_valid_user_id_is_false_for_alphabetic_content(self): - """A string decoding to alphabetic characters is not a valid user ID.""" - # YWJj = base64(abc) - self.assertFalse(TokenRemover.is_valid_user_id('YWJj')) + def test_is_valid_user_id_invalid(self): + """Should consider non-digit and non-ASCII IDs invalid.""" + ids = ( + ("SGVsbG8gd29ybGQ", "non-digit ASCII"), + ("0J_RgNC40LLQtdGCINC80LjRgA", "cyrillic text"), + ("4pO14p6L4p6C4pG34p264pGl8J-EiOKSj-KCieKBsA", "Unicode digits"), + ("4oaA4oaB4oWh4oWi4Lyz4Lyq4Lyr4LG9", "Unicode numerals"), + ("8J2fjvCdn5nwnZ-k8J2fr_Cdn7rgravvvJngr6c", "Unicode decimals"), + ("{hello}[world]&(bye!)", "ASCII invalid Base64"), + ("Þíß-ï§-ňøẗ-våłìÐ", "Unicode invalid Base64"), + ) - def test_is_valid_timestamp_is_true_for_valid_timestamps(self): - """A string decoding to a valid timestamp should be recognized as such.""" - self.assertTrue(TokenRemover.is_valid_timestamp('DN9r_A')) + for user_id, msg in ids: + with self.subTest(msg=msg): + result = TokenRemover.is_valid_user_id(user_id) + self.assertFalse(result) - def test_is_valid_timestamp_is_false_for_invalid_values(self): - """A string not decoding to a valid timestamp should not be recognized as such.""" - # MTIz = base64(123) - self.assertFalse(TokenRemover.is_valid_timestamp('MTIz')) + def test_is_valid_timestamp_valid(self): + """Should consider timestamps valid if they're greater than the Discord epoch.""" + timestamps = ( + "XsyRkw", + "Xrim9Q", + "XsyR-w", + "XsySD_", + "Dn9r_A", + ) + + for timestamp in timestamps: + with self.subTest(timestamp=timestamp): + result = TokenRemover.is_valid_timestamp(timestamp) + self.assertTrue(result) + + def test_is_valid_timestamp_invalid(self): + """Should consider timestamps invalid if they're before Discord epoch or can't be parsed.""" + timestamps = ( + ("B4Yffw", "DISCORD_EPOCH - TOKEN_EPOCH - 1"), + ("ew", "123"), + ("AoIKgA", "42076800"), + ("{hello}[world]&(bye!)", "ASCII invalid Base64"), + ("Þíß-ï§-ňøẗ-våłìÐ", "Unicode invalid Base64"), + ) + + for timestamp, msg in timestamps: + with self.subTest(msg=msg): + result = TokenRemover.is_valid_timestamp(timestamp) + self.assertFalse(result) def test_mod_log_property(self): """The `mod_log` property should ask the bot to return the `ModLog` cog.""" @@ -58,74 +91,220 @@ class TokenRemoverTests(unittest.TestCase): self.assertEqual(self.cog.mod_log, self.bot.get_cog.return_value) self.bot.get_cog.assert_called_once_with('ModLog') - def test_ignores_bot_messages(self): - """When the message event handler is called with a bot message, nothing is done.""" - self.msg.author.bot = True - coroutine = self.cog.on_message(self.msg) - self.assertIsNone(asyncio.run(coroutine)) - - def test_ignores_messages_without_tokens(self): - """Messages without anything looking like a token are ignored.""" - for content in ('', 'lemon wins'): - with self.subTest(content=content): - self.msg.content = content - coroutine = self.cog.on_message(self.msg) - self.assertIsNone(asyncio.run(coroutine)) - - def test_ignores_messages_with_invalid_tokens(self): - """Messages with values that are invalid tokens are ignored.""" - for content in ('foo.bar.baz', 'x.y.'): - with self.subTest(content=content): - self.msg.content = content - coroutine = self.cog.on_message(self.msg) - self.assertIsNone(asyncio.run(coroutine)) - - def test_censors_valid_tokens(self): - """Valid tokens are censored.""" - cases = ( - # (content, censored_token) - ('MTIz.DN9R_A.xyz', 'MTIz.DN9R_A.xxx'), + async def test_on_message_edit_uses_on_message(self): + """The edit listener should delegate handling of the message to the normal listener.""" + self.cog.on_message = mock.create_autospec(self.cog.on_message, spec_set=True) + + await self.cog.on_message_edit(MockMessage(), self.msg) + self.cog.on_message.assert_awaited_once_with(self.msg) + + @autospec(TokenRemover, "find_token_in_message", "take_action") + async def test_on_message_takes_action(self, find_token_in_message, take_action): + """Should take action if a valid token is found when a message is sent.""" + cog = TokenRemover(self.bot) + found_token = "foobar" + find_token_in_message.return_value = found_token + + await cog.on_message(self.msg) + + find_token_in_message.assert_called_once_with(self.msg) + take_action.assert_awaited_once_with(cog, self.msg, found_token) + + @autospec(TokenRemover, "find_token_in_message", "take_action") + async def test_on_message_skips_missing_token(self, find_token_in_message, take_action): + """Shouldn't take action if a valid token isn't found when a message is sent.""" + cog = TokenRemover(self.bot) + find_token_in_message.return_value = False + + await cog.on_message(self.msg) + + find_token_in_message.assert_called_once_with(self.msg) + take_action.assert_not_awaited() + + @autospec(TokenRemover, "find_token_in_message") + async def test_on_message_ignores_dms_bots(self, find_token_in_message): + """Shouldn't parse a message if it is a DM or authored by a bot.""" + cog = TokenRemover(self.bot) + dm_msg = MockMessage(guild=None) + bot_msg = MockMessage(author=MagicMock(bot=True)) + + for msg in (dm_msg, bot_msg): + await cog.on_message(msg) + find_token_in_message.assert_not_called() + + @autospec("bot.cogs.token_remover", "TOKEN_RE") + def test_find_token_no_matches(self, token_re): + """None should be returned if the regex matches no tokens in a message.""" + token_re.finditer.return_value = () + + return_value = TokenRemover.find_token_in_message(self.msg) + + self.assertIsNone(return_value) + token_re.finditer.assert_called_once_with(self.msg.content) + + @autospec(TokenRemover, "is_valid_user_id", "is_valid_timestamp") + @autospec("bot.cogs.token_remover", "Token") + @autospec("bot.cogs.token_remover", "TOKEN_RE") + def test_find_token_valid_match(self, token_re, token_cls, is_valid_id, is_valid_timestamp): + """The first match with a valid user ID and timestamp should be returned as a `Token`.""" + matches = [ + mock.create_autospec(Match, spec_set=True, instance=True), + mock.create_autospec(Match, spec_set=True, instance=True), + ] + tokens = [ + mock.create_autospec(Token, spec_set=True, instance=True), + mock.create_autospec(Token, spec_set=True, instance=True), + ] + + token_re.finditer.return_value = matches + token_cls.side_effect = tokens + is_valid_id.side_effect = (False, True) # The 1st match will be invalid, 2nd one valid. + is_valid_timestamp.return_value = True + + return_value = TokenRemover.find_token_in_message(self.msg) + + self.assertEqual(tokens[1], return_value) + token_re.finditer.assert_called_once_with(self.msg.content) + + @autospec(TokenRemover, "is_valid_user_id", "is_valid_timestamp") + @autospec("bot.cogs.token_remover", "Token") + @autospec("bot.cogs.token_remover", "TOKEN_RE") + def test_find_token_invalid_matches(self, token_re, token_cls, is_valid_id, is_valid_timestamp): + """None should be returned if no matches have valid user IDs or timestamps.""" + token_re.finditer.return_value = [mock.create_autospec(Match, spec_set=True, instance=True)] + token_cls.return_value = mock.create_autospec(Token, spec_set=True, instance=True) + is_valid_id.return_value = False + is_valid_timestamp.return_value = False + + return_value = TokenRemover.find_token_in_message(self.msg) + + self.assertIsNone(return_value) + token_re.finditer.assert_called_once_with(self.msg.content) + + def test_regex_invalid_tokens(self): + """Messages without anything looking like a token are not matched.""" + tokens = ( + "", + "lemon wins", + "..", + "x.y", + "x.y.", + ".y.z", + ".y.", + "..z", + "x..z", + " . . ", + "\n.\n.\n", + "hellö.world.bye", + "base64.nötbåse64.morebase64", + "19jd3J.dfkm3d.€víł§tüff", + ) + + for token in tokens: + with self.subTest(token=token): + results = token_remover.TOKEN_RE.findall(token) + self.assertEqual(len(results), 0) + + def test_regex_valid_tokens(self): + """Messages that look like tokens should be matched.""" + # Don't worry, these tokens have been invalidated. + tokens = ( + "NDcyMjY1OTQzMDYy_DEzMz-y.XsyRkw.VXmErH7j511turNpfURmb0rVNm8", + "NDcyMjY1OTQzMDYyNDEzMzMy.Xrim9Q.Ysnu2wacjaKs7qnoo46S8Dm2us8", + "NDc1MDczNjI5Mzk5NTQ3OTA0.XsyR-w.sJf6omBPORBPju3WJEIAcwW9Zds", + "NDY3MjIzMjMwNjUwNzc3NjQx.XsySD_.s45jqDV_Iisn-symw0yDRrk_jf4", + ) + + for token in tokens: + with self.subTest(token=token): + results = token_remover.TOKEN_RE.fullmatch(token) + self.assertIsNotNone(results, f"{token} was not matched by the regex") + + def test_regex_matches_multiple_valid(self): + """Should support multiple matches in the middle of a string.""" + token_1 = "NDY3MjIzMjMwNjUwNzc3NjQx.XsyWGg.uFNEQPCc4ePwGh7egG8UicQssz8" + token_2 = "NDcyMjY1OTQzMDYyNDEzMzMy.XsyWMw.l8XPnDqb0lp-EiQ2g_0xVFT1pyc" + message = f"garbage {token_1} hello {token_2} world" + + results = token_remover.TOKEN_RE.finditer(message) + results = [match[0] for match in results] + self.assertCountEqual((token_1, token_2), results) + + @autospec("bot.cogs.token_remover", "LOG_MESSAGE") + def test_format_log_message(self, log_message): + """Should correctly format the log message with info from the message and token.""" + token = Token("NDY3MjIzMjMwNjUwNzc3NjQx", "XsySD_", "s45jqDV_Iisn-symw0yDRrk_jf4") + log_message.format.return_value = "Howdy" + + return_value = TokenRemover.format_log_message(self.msg, token) + + self.assertEqual(return_value, log_message.format.return_value) + log_message.format.assert_called_once_with( + author=self.msg.author, + author_id=self.msg.author.id, + channel=self.msg.channel.mention, + user_id=token.user_id, + timestamp=token.timestamp, + hmac="x" * len(token.hmac), + ) + + @mock.patch.object(TokenRemover, "mod_log", new_callable=mock.PropertyMock) + @autospec("bot.cogs.token_remover", "log") + @autospec(TokenRemover, "format_log_message") + async def test_take_action(self, format_log_message, logger, mod_log_property): + """Should delete the message and send a mod log.""" + cog = TokenRemover(self.bot) + mod_log = mock.create_autospec(ModLog, spec_set=True, instance=True) + token = mock.create_autospec(Token, spec_set=True, instance=True) + log_msg = "testing123" + + mod_log_property.return_value = mod_log + format_log_message.return_value = log_msg + + await cog.take_action(self.msg, token) + + self.msg.delete.assert_called_once_with() + self.msg.channel.send.assert_called_once_with( + token_remover.DELETION_MESSAGE_TEMPLATE.format(mention=self.msg.author.mention) + ) + + format_log_message.assert_called_once_with(self.msg, token) + logger.debug.assert_called_with(log_msg) + self.bot.stats.incr.assert_called_once_with("tokens.removed_tokens") + + mod_log.ignore.assert_called_once_with(constants.Event.message_delete, self.msg.id) + mod_log.send_log_message.assert_called_once_with( + icon_url=constants.Icons.token_removed, + colour=Colour(constants.Colours.soft_red), + title="Token removed!", + text=log_msg, + thumbnail=self.msg.author.avatar_url_as.return_value, + channel_id=constants.Channels.mod_alerts ) - for content, censored_token in cases: - with self.subTest(content=content, censored_token=censored_token): - self.msg.content = content - coroutine = self.cog.on_message(self.msg) - with self.assertLogs(logger='bot.cogs.token_remover', level=logging.DEBUG) as cm: - self.assertIsNone(asyncio.run(coroutine)) # no return value - - [line] = cm.output - log_message = ( - "Censored a seemingly valid token sent by " - "lemon (`42`) in #lemonade-stand, " - f"token was `{censored_token}`" - ) - self.assertIn(log_message, line) - - self.msg.delete.assert_called_once_with() - self.msg.channel.send.assert_called_once_with( - DELETION_MESSAGE_TEMPLATE.format(mention='@lemon') - ) - self.bot.get_cog.assert_called_with('ModLog') - self.msg.author.avatar_url_as.assert_called_once_with(static_format='png') - - mod_log = self.bot.get_cog.return_value - mod_log.ignore.assert_called_once_with(Event.message_delete, self.msg.id) - mod_log.send_log_message.assert_called_once_with( - icon_url=Icons.token_removed, - colour=Colour(Colours.soft_red), - title="Token removed!", - text=log_message, - thumbnail='picture-lemon.png', - channel_id=Channels.mod_alerts - ) - - -class TokenRemoverSetupTests(unittest.TestCase): - """Tests setup of the `TokenRemover` cog.""" - - def test_setup(self): - """Setup of the extension should call add_cog.""" + @mock.patch.object(TokenRemover, "mod_log", new_callable=mock.PropertyMock) + async def test_take_action_delete_failure(self, mod_log_property): + """Shouldn't send any messages if the token message can't be deleted.""" + cog = TokenRemover(self.bot) + mod_log_property.return_value = mock.create_autospec(ModLog, spec_set=True, instance=True) + self.msg.delete.side_effect = NotFound(MagicMock(), MagicMock()) + + token = mock.create_autospec(Token, spec_set=True, instance=True) + await cog.take_action(self.msg, token) + + self.msg.delete.assert_called_once_with() + self.msg.channel.send.assert_not_awaited() + + +class TokenRemoverExtensionTests(unittest.TestCase): + """Tests for the token_remover extension.""" + + @autospec("bot.cogs.token_remover", "TokenRemover") + def test_extension_setup(self, cog): + """The TokenRemover cog should be added.""" bot = MockBot() - setup_cog(bot) + token_remover.setup(bot) + + cog.assert_called_once_with(bot) bot.add_cog.assert_called_once() + self.assertTrue(isinstance(bot.add_cog.call_args.args[0], TokenRemover)) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index ca8cb6825..c42111f3f 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -1,5 +1,5 @@ -import asyncio import datetime +import re import unittest from unittest.mock import MagicMock, patch @@ -16,7 +16,7 @@ from bot.converters import ( ) -class ConverterTests(unittest.TestCase): +class ConverterTests(unittest.IsolatedAsyncioTestCase): """Tests our custom argument converters.""" @classmethod @@ -26,7 +26,7 @@ class ConverterTests(unittest.TestCase): cls.fixed_utc_now = datetime.datetime.fromisoformat('2019-01-01T00:00:00') - def test_tag_content_converter_for_valid(self): + async def test_tag_content_converter_for_valid(self): """TagContentConverter should return correct values for valid input.""" test_values = ( ('hello', 'hello'), @@ -35,10 +35,10 @@ class ConverterTests(unittest.TestCase): for content, expected_conversion in test_values: with self.subTest(content=content, expected_conversion=expected_conversion): - conversion = asyncio.run(TagContentConverter.convert(self.context, content)) + conversion = await TagContentConverter.convert(self.context, content) self.assertEqual(conversion, expected_conversion) - def test_tag_content_converter_for_invalid(self): + async def test_tag_content_converter_for_invalid(self): """TagContentConverter should raise the proper exception for invalid input.""" test_values = ( ('', "Tag contents should not be empty, or filled with whitespace."), @@ -47,10 +47,10 @@ class ConverterTests(unittest.TestCase): for value, exception_message in test_values: with self.subTest(tag_content=value, exception_message=exception_message): - with self.assertRaises(BadArgument, msg=exception_message): - asyncio.run(TagContentConverter.convert(self.context, value)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await TagContentConverter.convert(self.context, value) - def test_tag_name_converter_for_valid(self): + async def test_tag_name_converter_for_valid(self): """TagNameConverter should return the correct values for valid tag names.""" test_values = ( ('tracebacks', 'tracebacks'), @@ -60,10 +60,10 @@ class ConverterTests(unittest.TestCase): for name, expected_conversion in test_values: with self.subTest(name=name, expected_conversion=expected_conversion): - conversion = asyncio.run(TagNameConverter.convert(self.context, name)) + conversion = await TagNameConverter.convert(self.context, name) self.assertEqual(conversion, expected_conversion) - def test_tag_name_converter_for_invalid(self): + async def test_tag_name_converter_for_invalid(self): """TagNameConverter should raise the correct exception for invalid tag names.""" test_values = ( ('👋', "Don't be ridiculous, you can't use that character!"), @@ -75,29 +75,29 @@ class ConverterTests(unittest.TestCase): for invalid_name, exception_message in test_values: with self.subTest(invalid_name=invalid_name, exception_message=exception_message): - with self.assertRaises(BadArgument, msg=exception_message): - asyncio.run(TagNameConverter.convert(self.context, invalid_name)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await TagNameConverter.convert(self.context, invalid_name) - def test_valid_python_identifier_for_valid(self): + async def test_valid_python_identifier_for_valid(self): """ValidPythonIdentifier returns valid identifiers unchanged.""" test_values = ('foo', 'lemon') for name in test_values: with self.subTest(identifier=name): - conversion = asyncio.run(ValidPythonIdentifier.convert(self.context, name)) + conversion = await ValidPythonIdentifier.convert(self.context, name) self.assertEqual(name, conversion) - def test_valid_python_identifier_for_invalid(self): + async def test_valid_python_identifier_for_invalid(self): """ValidPythonIdentifier raises the proper exception for invalid identifiers.""" test_values = ('nested.stuff', '#####') for name in test_values: with self.subTest(identifier=name): exception_message = f'`{name}` is not a valid Python identifier' - with self.assertRaises(BadArgument, msg=exception_message): - asyncio.run(ValidPythonIdentifier.convert(self.context, name)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await ValidPythonIdentifier.convert(self.context, name) - def test_duration_converter_for_valid(self): + async def test_duration_converter_for_valid(self): """Duration returns the correct `datetime` for valid duration strings.""" test_values = ( # Simple duration strings @@ -159,35 +159,35 @@ class ConverterTests(unittest.TestCase): mock_datetime.utcnow.return_value = self.fixed_utc_now with self.subTest(duration=duration, duration_dict=duration_dict): - converted_datetime = asyncio.run(converter.convert(self.context, duration)) + converted_datetime = await converter.convert(self.context, duration) self.assertEqual(converted_datetime, expected_datetime) - def test_duration_converter_for_invalid(self): + async def test_duration_converter_for_invalid(self): """Duration raises the right exception for invalid duration strings.""" test_values = ( # Units in wrong order - ('1d1w'), - ('1s1y'), + '1d1w', + '1s1y', # Duplicated units - ('1 year 2 years'), - ('1 M 10 minutes'), + '1 year 2 years', + '1 M 10 minutes', # Unknown substrings - ('1MVes'), - ('1y3breads'), + '1MVes', + '1y3breads', # Missing amount - ('ym'), + 'ym', # Incorrect whitespace - (" 1y"), - ("1S "), - ("1y 1m"), + " 1y", + "1S ", + "1y 1m", # Garbage - ('Guido van Rossum'), - ('lemon lemon lemon lemon lemon lemon lemon'), + 'Guido van Rossum', + 'lemon lemon lemon lemon lemon lemon lemon', ) converter = Duration() @@ -195,10 +195,21 @@ class ConverterTests(unittest.TestCase): for invalid_duration in test_values: with self.subTest(invalid_duration=invalid_duration): exception_message = f'`{invalid_duration}` is not a valid duration string.' - with self.assertRaises(BadArgument, msg=exception_message): - asyncio.run(converter.convert(self.context, invalid_duration)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, invalid_duration) - def test_isodatetime_converter_for_valid(self): + @patch("bot.converters.datetime") + async def test_duration_converter_out_of_range(self, mock_datetime): + """Duration converter should raise BadArgument if datetime raises a ValueError.""" + mock_datetime.__add__.side_effect = ValueError + mock_datetime.utcnow.return_value = mock_datetime + + duration = f"{datetime.MAXYEAR}y" + exception_message = f"`{duration}` results in a datetime outside the supported range." + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await Duration().convert(self.context, duration) + + async def test_isodatetime_converter_for_valid(self): """ISODateTime converter returns correct datetime for valid datetime string.""" test_values = ( # `YYYY-mm-ddTHH:MM:SSZ` | `YYYY-mm-dd HH:MM:SSZ` @@ -243,37 +254,37 @@ class ConverterTests(unittest.TestCase): for datetime_string, expected_dt in test_values: with self.subTest(datetime_string=datetime_string, expected_dt=expected_dt): - converted_dt = asyncio.run(converter.convert(self.context, datetime_string)) + converted_dt = await converter.convert(self.context, datetime_string) self.assertIsNone(converted_dt.tzinfo) self.assertEqual(converted_dt, expected_dt) - def test_isodatetime_converter_for_invalid(self): + async def test_isodatetime_converter_for_invalid(self): """ISODateTime converter raises the correct exception for invalid datetime strings.""" test_values = ( # Make sure it doesn't interfere with the Duration converter - ('1Y'), - ('1d'), - ('1H'), + '1Y', + '1d', + '1H', # Check if it fails when only providing the optional time part - ('10:10:10'), - ('10:00'), + '10:10:10', + '10:00', # Invalid date format - ('19-01-01'), + '19-01-01', # Other non-valid strings - ('fisk the tag master'), + 'fisk the tag master', ) converter = ISODateTime() for datetime_string in test_values: with self.subTest(datetime_string=datetime_string): exception_message = f"`{datetime_string}` is not a valid ISO-8601 datetime string" - with self.assertRaises(BadArgument, msg=exception_message): - asyncio.run(converter.convert(self.context, datetime_string)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, datetime_string) - def test_hush_duration_converter_for_valid(self): + async def test_hush_duration_converter_for_valid(self): """HushDurationConverter returns correct value for minutes duration or `"forever"` strings.""" test_values = ( ("0", 0), @@ -286,10 +297,10 @@ class ConverterTests(unittest.TestCase): converter = HushDurationConverter() for minutes_string, expected_minutes in test_values: with self.subTest(minutes_string=minutes_string, expected_minutes=expected_minutes): - converted = asyncio.run(converter.convert(self.context, minutes_string)) + converted = await converter.convert(self.context, minutes_string) self.assertEqual(expected_minutes, converted) - def test_hush_duration_converter_for_invalid(self): + async def test_hush_duration_converter_for_invalid(self): """HushDurationConverter raises correct exception for invalid minutes duration strings.""" test_values = ( ("16", "Duration must be at most 15 minutes."), @@ -299,5 +310,5 @@ class ConverterTests(unittest.TestCase): converter = HushDurationConverter() for invalid_minutes_string, exception_message in test_values: with self.subTest(invalid_minutes_string=invalid_minutes_string, exception_message=exception_message): - with self.assertRaisesRegex(BadArgument, exception_message): - asyncio.run(converter.convert(self.context, invalid_minutes_string)) + with self.assertRaisesRegex(BadArgument, re.escape(exception_message)): + await converter.convert(self.context, invalid_minutes_string) diff --git a/tests/bot/utils/test_messages.py b/tests/bot/utils/test_messages.py new file mode 100644 index 000000000..9c22c9751 --- /dev/null +++ b/tests/bot/utils/test_messages.py @@ -0,0 +1,27 @@ +import unittest + +from bot.utils import messages + + +class TestMessages(unittest.TestCase): + """Tests for functions in the `bot.utils.messages` module.""" + + def test_sub_clyde(self): + """Uppercase E's and lowercase e's are substituted with their cyrillic counterparts.""" + sub_e = "\u0435" + sub_E = "\u0415" # noqa: N806: Uppercase E in variable name + + test_cases = ( + (None, None), + ("", ""), + ("clyde", f"clyd{sub_e}"), + ("CLYDE", f"CLYD{sub_E}"), + ("cLyDe", f"cLyD{sub_e}"), + ("BIGclyde", f"BIGclyd{sub_e}"), + ("small clydeus the unholy", f"small clyd{sub_e}us the unholy"), + ("BIGCLYDE, babyclyde", f"BIGCLYD{sub_E}, babyclyd{sub_e}"), + ) + + for username_in, username_out in test_cases: + with self.subTest(input=username_in, expected_output=username_out): + self.assertEqual(messages.sub_clyde(username_in), username_out) diff --git a/tests/bot/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py index 8c1a40640..a2f0fe55d 100644 --- a/tests/bot/utils/test_redis_cache.py +++ b/tests/bot/utils/test_redis_cache.py @@ -44,22 +44,14 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase): with self.assertRaises(RuntimeError): await bad_cache.set("test", "me_up_deadman") - def test_namespace_collision(self): - """Test that we prevent colliding namespaces.""" - bob_cache_1 = RedisCache() - bob_cache_1._set_namespace("BobRoss") - self.assertEqual(bob_cache_1._namespace, "BobRoss") - - bob_cache_2 = RedisCache() - bob_cache_2._set_namespace("BobRoss") - self.assertEqual(bob_cache_2._namespace, "BobRoss_") - async def test_set_get_item(self): """Test that users can set and get items from the RedisDict.""" test_cases = ( ('favorite_fruit', 'melon'), ('favorite_number', 86), - ('favorite_fraction', 86.54) + ('favorite_fraction', 86.54), + ('favorite_boolean', False), + ('other_boolean', True), ) # Test that we can get and set different types. diff --git a/tests/helpers.py b/tests/helpers.py index faa839370..facc4e1af 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -5,7 +5,7 @@ import itertools import logging import unittest.mock from asyncio import AbstractEventLoop -from typing import Iterable, Optional +from typing import Callable, Iterable, Optional import discord from aiohttp import ClientSession @@ -26,6 +26,24 @@ for logger in logging.Logger.manager.loggerDict.values(): logger.setLevel(logging.CRITICAL) +def autospec(target, *attributes: str, **kwargs) -> Callable: + """Patch multiple `attributes` of a `target` with autospecced mocks and `spec_set` as True.""" + # Caller's kwargs should take priority and overwrite the defaults. + kwargs = {'spec_set': True, 'autospec': True, **kwargs} + + # Import the target if it's a string. + # This is to support both object and string targets like patch.multiple. + if type(target) is str: + target = unittest.mock._importer(target) + + def decorator(func): + for attribute in attributes: + patcher = unittest.mock.patch.object(target, attribute, **kwargs) + func = patcher(func) + return func + return decorator + + class HashableMixin(discord.mixins.EqualityComparable): """ Mixin that provides similar hashing and equality functionality as discord.py's `Hashable` mixin. |