diff options
| -rw-r--r-- | bot/cogs/filtering.py | 85 | ||||
| -rw-r--r-- | bot/cogs/help_channels.py | 124 | ||||
| -rw-r--r-- | bot/cogs/moderation/infractions.py | 53 | ||||
| -rw-r--r-- | bot/cogs/moderation/scheduler.py | 7 | ||||
| -rw-r--r-- | bot/cogs/moderation/silence.py | 37 | ||||
| -rw-r--r-- | bot/cogs/token_remover.py | 119 | ||||
| -rw-r--r-- | bot/cogs/utils.py | 4 | ||||
| -rw-r--r-- | bot/constants.py | 1 | ||||
| -rw-r--r-- | bot/resources/tags/customcooldown.md | 20 | ||||
| -rw-r--r-- | bot/utils/__init__.py | 5 | ||||
| -rw-r--r-- | bot/utils/messages.py | 2 | ||||
| -rw-r--r-- | bot/utils/redis_cache.py | 23 | ||||
| -rw-r--r-- | config-default.yml | 5 | ||||
| -rw-r--r-- | tests/bot/cogs/moderation/test_silence.py | 18 | ||||
| -rw-r--r-- | tests/bot/cogs/test_token_remover.py | 367 | ||||
| -rw-r--r-- | tests/bot/utils/test_redis_cache.py | 4 | ||||
| -rw-r--r-- | tests/helpers.py | 20 | 
17 files changed, 643 insertions, 251 deletions
diff --git a/bot/cogs/filtering.py b/bot/cogs/filtering.py index 4ebc831e1..76ea68660 100644 --- a/bot/cogs/filtering.py +++ b/bot/cogs/filtering.py @@ -2,11 +2,12 @@ import asyncio  import logging  import re  from datetime import datetime, timedelta -from typing import List, Optional, Union +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 @@ -17,6 +18,8 @@ from bot.constants import (      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__) @@ -54,7 +57,10 @@ 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 @@ -62,6 +68,8 @@ class Filtering(Cog):      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!" @@ -75,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, @@ -86,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, @@ -96,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.""" @@ -242,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: @@ -359,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("\\", "") @@ -428,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 6ff285c37..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 @@ -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/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/scheduler.py b/bot/cogs/moderation/scheduler.py index b03d89537..d75a72ddb 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -127,18 +127,17 @@ class InfractionScheduler(Scheduler):                      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}.") 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/token_remover.py b/bot/cogs/token_remover.py index 6721f0e02..d55e079e9 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.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): @@ -65,64 +74,58 @@ 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()          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 +133,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/constants.py b/bot/constants.py index 24726c20d..b3ef1660f 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] 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..23519a514 100644 --- a/bot/utils/messages.py +++ b/bot/utils/messages.py @@ -97,7 +97,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) diff --git a/bot/utils/redis_cache.py b/bot/utils/redis_cache.py index 354e987b9..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__` @@ -108,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 @@ -122,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 6b827b63d..4c0196dc5 100644 --- a/config-default.yml +++ b/config-default.yml @@ -274,7 +274,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 @@ -304,6 +305,7 @@ filter:          - 185590609631903755  # Blender Hub          - 420324994703163402  # /r/FlutterDev          - 488751051629920277  # Python Atlanta +        - 143867839282020352  # C#      domain_blacklist:          - pornhub.com @@ -335,6 +337,7 @@ filter:          - ssteam.site          - steamwalletgift.com          - discord.gift +        - lmgtfy.com      word_watchlist:          - goo+ks* 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/test_token_remover.py b/tests/bot/cogs/test_token_remover.py index 33d1ec170..a10124d2d 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 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,206 @@ 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.""" +    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("bot.cogs.token_remover", "TOKEN_RE") +    def test_find_token_ignores_bot_messages(self, token_re): +        """The token finder should ignore messages authored by bots."""          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'), + +        return_value = TokenRemover.find_token_in_message(self.msg) + +        self.assertIsNone(return_value) +        token_re.finditer.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 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.""" +        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 +        ) + + +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/utils/test_redis_cache.py b/tests/bot/utils/test_redis_cache.py index e5d6e4078..a2f0fe55d 100644 --- a/tests/bot/utils/test_redis_cache.py +++ b/tests/bot/utils/test_redis_cache.py @@ -49,7 +49,9 @@ class RedisCacheTests(unittest.IsolatedAsyncioTestCase):          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.  |