From a2218956c881de81b41129e707091a43f6477d24 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Aug 2020 22:19:22 +0200 Subject: Verification: add initial on join message This message will be sent via direct message to each user who joins the guild. --- bot/cogs/verification.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ae156cf70..2293cad28 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -12,6 +12,13 @@ from bot.utils.checks import InWhitelistCheckFailure, without_role_check log = logging.getLogger(__name__) +ON_JOIN_MESSAGE = f""" +Hello! Welcome to Python Discord! + +In order to send messages, you first have to accept our rules. To do so, please visit \ +<#{constants.Channels.verification}>. Thank you! +""" + WELCOME_MESSAGE = f""" Hello! Welcome to the server, and thanks for verifying yourself! -- cgit v1.2.3 From fad796101d5f641c0c4315244303c8727df0462f Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Aug 2020 22:25:18 +0200 Subject: Verification: adjust & rename welcome message Let's give it a better name so that it's clear when this message is sent. The initial words are adjusted to avoid repetition after the on join message. --- bot/cogs/verification.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 2293cad28..c10940817 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -19,8 +19,8 @@ In order to send messages, you first have to accept our rules. To do so, please <#{constants.Channels.verification}>. Thank you! """ -WELCOME_MESSAGE = f""" -Hello! Welcome to the server, and thanks for verifying yourself! +VERIFIED_MESSAGE = f""" +Thanks for verifying yourself! For your records, these are the documents you accepted: @@ -121,7 +121,7 @@ class Verification(Cog): log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") await ctx.author.add_roles(Object(constants.Roles.verified), reason="Accepted the rules") try: - await ctx.author.send(WELCOME_MESSAGE) + await ctx.author.send(VERIFIED_MESSAGE) except Forbidden: log.info(f"Sending welcome message failed for {ctx.author}.") finally: -- cgit v1.2.3 From 0521af684f82fec50b46f744aebf76ccee88f318 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sun, 2 Aug 2020 23:06:21 +0200 Subject: Verification: send initial message on member join --- bot/cogs/verification.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index c10940817..1c1919bdf 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,7 +1,7 @@ import logging from contextlib import suppress -from discord import Colour, Forbidden, Message, NotFound, Object +from discord import Colour, Forbidden, Member, Message, NotFound, Object from discord.ext.commands import Cog, Context, command from bot import constants @@ -53,6 +53,16 @@ class Verification(Cog): """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") + @Cog.listener() + async def on_member_join(self, member: Member) -> None: + """Attempt to send initial direct message to each new member.""" + if member.guild.id != constants.Guild.id: + return # Only listen for PyDis events + + log.trace(f"Sending on join message to new member: {member.id}") + with suppress(Forbidden): + await member.send(ON_JOIN_MESSAGE) + @Cog.listener() async def on_message(self, message: Message) -> None: """Check new message event for messages to the checkpoint channel & process.""" -- cgit v1.2.3 From 226af0e676e072696dba503e3873deeaf73202ac Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:01:47 +0200 Subject: Verification: add @Unverified role to config --- bot/constants.py | 1 + config-default.yml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index cf4f3f666..cce64a7c4 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -454,6 +454,7 @@ class Roles(metaclass=YAMLGetter): partners: int python_community: int team_leaders: int + unverified: int verified: int # This is the Developers role on PyDis, here named verified for readability reasons. diff --git a/config-default.yml b/config-default.yml index fc093cc32..21a3eca87 100644 --- a/config-default.yml +++ b/config-default.yml @@ -225,8 +225,8 @@ guild: partners: 323426753857191936 python_community: &PY_COMMUNITY_ROLE 458226413825294336 - # This is the Developers role on PyDis, here named verified for readability reasons - verified: 352427296948486144 + unverified: 739794855945044069 + verified: 352427296948486144 # @Developers on PyDis # Staff admins: &ADMINS_ROLE 267628507062992896 -- cgit v1.2.3 From 5006105f14e575366575c1091af7cfd2b2da7abd Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:08:32 +0200 Subject: Verification: refactor `discord` imports Let's access these via the qualified name. The amount of imported names was starting to get unwieldy. --- bot/cogs/verification.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 1c1919bdf..f86356f33 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,7 +1,7 @@ import logging from contextlib import suppress -from discord import Colour, Forbidden, Member, Message, NotFound, Object +import discord from discord.ext.commands import Cog, Context, command from bot import constants @@ -54,17 +54,17 @@ class Verification(Cog): return self.bot.get_cog("ModLog") @Cog.listener() - async def on_member_join(self, member: Member) -> None: + async def on_member_join(self, member: discord.Member) -> None: """Attempt to send initial direct message to each new member.""" if member.guild.id != constants.Guild.id: return # Only listen for PyDis events log.trace(f"Sending on join message to new member: {member.id}") - with suppress(Forbidden): + with suppress(discord.Forbidden): await member.send(ON_JOIN_MESSAGE) @Cog.listener() - async def on_message(self, message: Message) -> None: + async def on_message(self, message: discord.Message) -> None: """Check new message event for messages to the checkpoint channel & process.""" if message.channel.id != constants.Channels.verification: return # Only listen for #checkpoint messages @@ -91,7 +91,7 @@ class Verification(Cog): # Send pretty mod log embed to mod-alerts await self.mod_log.send_log_message( icon_url=constants.Icons.filtering, - colour=Colour(constants.Colours.soft_red), + colour=discord.Colour(constants.Colours.soft_red), title=f"User/Role mentioned in {message.channel.name}", text=embed_text, thumbnail=message.author.avatar_url_as(static_format="png"), @@ -120,7 +120,7 @@ class Verification(Cog): ) log.trace(f"Deleting the message posted by {ctx.author}") - with suppress(NotFound): + with suppress(discord.NotFound): await ctx.message.delete() @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @@ -129,14 +129,14 @@ class Verification(Cog): async def accept_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Accept our rules and gain access to the rest of the server.""" log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") - await ctx.author.add_roles(Object(constants.Roles.verified), reason="Accepted the rules") + await ctx.author.add_roles(discord.Object(constants.Roles.verified), reason="Accepted the rules") try: await ctx.author.send(VERIFIED_MESSAGE) - except Forbidden: + except discord.Forbidden: log.info(f"Sending welcome message failed for {ctx.author}.") finally: log.trace(f"Deleting accept message by {ctx.author}.") - with suppress(NotFound): + with suppress(discord.NotFound): self.mod_log.ignore(constants.Event.message_delete, ctx.message.id) await ctx.message.delete() @@ -156,7 +156,7 @@ class Verification(Cog): return log.debug(f"{ctx.author} called !subscribe. Assigning the 'Announcements' role.") - await ctx.author.add_roles(Object(constants.Roles.announcements), reason="Subscribed to announcements") + await ctx.author.add_roles(discord.Object(constants.Roles.announcements), reason="Subscribed to announcements") log.trace(f"Deleting the message posted by {ctx.author}.") @@ -180,7 +180,9 @@ class Verification(Cog): return log.debug(f"{ctx.author} called !unsubscribe. Removing the 'Announcements' role.") - await ctx.author.remove_roles(Object(constants.Roles.announcements), reason="Unsubscribed from announcements") + await ctx.author.remove_roles( + discord.Object(constants.Roles.announcements), reason="Unsubscribed from announcements" + ) log.trace(f"Deleting the message posted by {ctx.author}.") -- cgit v1.2.3 From 8415ac8da18054be3258ee7816a70c58a3a9322a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:09:06 +0200 Subject: Verification: define time constants --- bot/cogs/verification.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index f86356f33..95d92899b 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -39,6 +39,9 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ +UNVERIFIED_AFTER = 3 # Amount of days after which non-Developers receive the @Unverified role +KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from the guild + BOT_MESSAGE_DELETE_DELAY = 10 -- cgit v1.2.3 From f0e0dcc599f974bb563e298199fcf5a76b1cfbe7 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:13:37 +0200 Subject: Verification: implement `check_users` coroutine See docstring for details. The coroutine will be registered as a task at a later point. --- bot/cogs/verification.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 95d92899b..ea4874450 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,5 +1,8 @@ +import asyncio import logging +import typing as t from contextlib import suppress +from datetime import datetime, timedelta import discord from discord.ext.commands import Cog, Context, command @@ -51,6 +54,61 @@ class Verification(Cog): def __init__(self, bot: Bot): self.bot = bot + async def _kick_members(self, members: t.Set[discord.Member]) -> int: + """Kick `members` from the PyDis guild.""" + ... + + async def _give_role(self, members: t.Set[discord.Member], role: discord.Role) -> int: + """Give `role` to all `members`.""" + ... + + async def check_users(self) -> None: + """ + Periodically check in on the verification status of PyDis members. + + This coroutine performs two actions: + * Find members who have not verified for `UNVERIFIED_AFTER` and give them the @Unverified role + * Find members who have not verified for `KICKED_AFTER` and kick them from the guild + + Within the body of this coroutine, we only select the members for each action. The work is then + delegated to `_kick_members` and `_give_role`. After each run, a report is sent via modlog. + """ + await self.bot.wait_until_guild_available() # Ensure cache is ready + pydis = self.bot.get_guild(constants.Guild.id) + + unverified = pydis.get_role(constants.Roles.unverified) + current_dt = datetime.utcnow() # Discord timestamps are UTC + + # Users to be given the @Unverified role, and those to be kicked, these should be entirely disjoint + for_role, for_kick = set(), set() + + log.debug("Checking verification status of guild members") + for member in pydis.members: + + # Skip all bots and users for which we don't know their join date + # This should be extremely rare, but can happen according to `joined_at` docs + if member.bot or member.joined_at is None: + continue + + # Now we check roles to determine whether this user has already verified + unverified_roles = {unverified, pydis.default_role} # Verified users have at least one more role + if set(member.roles) - unverified_roles: + continue + + # At this point, we know that `member` is an unverified user, and we will decide what + # to do with them based on time passed since their join date + since_join = current_dt - member.joined_at + + if since_join > timedelta(days=KICKED_AFTER): + for_kick.add(member) # User should be removed from the guild + + elif since_join > timedelta(days=UNVERIFIED_AFTER) and unverified not in member.roles: + for_role.add(member) # User should be given the @Unverified role + + log.debug(f"{len(for_role)} users will be given the {unverified} role, {len(for_kick)} users will be kicked") + n_kicks = await self._kick_members(for_kick) + n_roles = await self._give_role(for_role, unverified) + @property def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" @@ -184,7 +242,7 @@ class Verification(Cog): log.debug(f"{ctx.author} called !unsubscribe. Removing the 'Announcements' role.") await ctx.author.remove_roles( - discord.Object(constants.Roles.announcements), reason="Unsubscribed from announcements" + discord.Object(constants.Roles.announcements), reason="Unsubscribed from announcements" ) log.trace(f"Deleting the message posted by {ctx.author}.") -- cgit v1.2.3 From 6840dbe5539cd5a094c65b2d09ddda227ad2ca30 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:35:17 +0200 Subject: Verification: implement `_give_role` helper --- bot/cogs/verification.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ea4874450..683e60ddb 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -59,8 +59,27 @@ class Verification(Cog): ... async def _give_role(self, members: t.Set[discord.Member], role: discord.Role) -> int: - """Give `role` to all `members`.""" - ... + """ + Give `role` to all `members`. + + Returns the amount of successful requests. Status codes of unsuccessful requests + are logged at info level. + """ + log.info(f"Assigning {role} role to {len(members)} members (not verified after {UNVERIFIED_AFTER} days)") + n_success, bad_statuses = 0, set() + + for member in members: + try: + await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") + except discord.HTTPException as http_exc: + bad_statuses.add(http_exc.status) + else: + n_success += 1 + + if bad_statuses: + log.info(f"Failed to assign {len(members) - n_success} roles due to following statuses: {bad_statuses}") + + return n_success async def check_users(self) -> None: """ -- cgit v1.2.3 From d807324eef804ec4ea002ef34063046e4fdaeca5 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:40:03 +0200 Subject: Verification: implement `_kick_members` helper --- bot/cogs/verification.py | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 683e60ddb..94c21a568 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,4 +1,3 @@ -import asyncio import logging import typing as t from contextlib import suppress @@ -55,8 +54,27 @@ class Verification(Cog): self.bot = bot async def _kick_members(self, members: t.Set[discord.Member]) -> int: - """Kick `members` from the PyDis guild.""" - ... + """ + Kick `members` from the PyDis guild. + + Note that this is a potentially destructive operation. Returns the amount of successful + requests. Failed requests are logged at info level. + """ + log.info(f"Kicking {len(members)} members from the guild (not verified after {KICKED_AFTER} days)") + n_kicked, bad_statuses = 0, set() + + for member in members: + try: + await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") + except discord.HTTPException as http_exc: + bad_statuses.add(http_exc.status) + else: + n_kicked += 1 + + if bad_statuses: + log.info(f"Failed to kick {len(members) - n_kicked} members due to following statuses: {bad_statuses}") + + return n_kicked async def _give_role(self, members: t.Set[discord.Member], role: discord.Role) -> int: """ -- cgit v1.2.3 From 59f8ec77fa25519d1bc81052af7c2cc6460cedad Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 18:42:45 +0200 Subject: Verification: implement `_verify_kick` helper This will be used to guard the call to `_kick_members`. --- bot/cogs/verification.py | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 94c21a568..85a0e3ec4 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -1,3 +1,4 @@ +import asyncio import logging import typing as t from contextlib import suppress @@ -44,6 +45,11 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! UNVERIFIED_AFTER = 3 # Amount of days after which non-Developers receive the @Unverified role KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from the guild +# Number in range [0, 1] determining the percentage of unverified users that are safe +# to be kicked from the guild in one batch, any larger amount will require staff confirmation, +# set this to 0 to require explicit approval for batches of any size +KICK_CONFIRMATION_THRESHOLD = 0 + BOT_MESSAGE_DELETE_DELAY = 10 @@ -53,6 +59,63 @@ class Verification(Cog): def __init__(self, bot: Bot): self.bot = bot + async def _verify_kick(self, n_members: int) -> bool: + """ + Determine whether `n_members` is a reasonable amount of members to kick. + + First, `n_members` is checked against the size of the PyDis guild. If `n_members` are + more than `KICK_CONFIRMATION_THRESHOLD` of the guild, the operation must be confirmed + by staff in #core-dev. Otherwise, the operation is seen as safe. + """ + log.debug(f"Checking whether {n_members} members are safe to kick") + + await self.bot.wait_until_guild_available() # Ensure cache is populated before we grab the guild + pydis = self.bot.get_guild(constants.Guild.id) + + percentage = n_members / len(pydis.members) + if percentage < KICK_CONFIRMATION_THRESHOLD: + log.debug(f"Kicking {percentage:.2%} of the guild's population is seen as safe") + return True + + # Since `n_members` is a suspiciously large number, we will ask for confirmation + log.debug("Amount of users is too large, requesting staff confirmation") + + core_devs = pydis.get_channel(constants.Channels.dev_core) + confirmation_msg = await core_devs.send( + f"Verification determined that `{n_members}` members should be kicked as they haven't verified in " + f"`{KICKED_AFTER}` days. This is `{percentage:.2%}` of the guild's population. Proceed?" + ) + + options = (constants.Emojis.incident_actioned, constants.Emojis.incident_unactioned) + for option in options: + await confirmation_msg.add_reaction(option) + + def check(reaction: discord.Reaction, user: discord.User) -> bool: + """Check whether `reaction` is a valid reaction to `confirmation_msg`.""" + return ( + reaction.message.id == confirmation_msg.id # Reacted to `confirmation_msg` + and str(reaction.emoji) in options # With one of `options` + and not user.bot # By a human + ) + + timeout = 60 * 5 # Seconds, i.e. 5 minutes + try: + choice, _ = await self.bot.wait_for("reaction_add", check=check, timeout=timeout) + except asyncio.TimeoutError: + log.debug("Staff prompt not answered, aborting operation") + return False + finally: + await confirmation_msg.clear_reactions() + + result = str(choice) == constants.Emojis.incident_actioned + log.debug(f"Received answer: {choice}, result: {result}") + + # Edit the prompt message to reflect the final choice + await confirmation_msg.edit( + content=f"Request to kick `{n_members}` members was {'authorized' if result else 'denied'}!" + ) + return result + async def _kick_members(self, members: t.Set[discord.Member]) -> int: """ Kick `members` from the PyDis guild. -- cgit v1.2.3 From 56f9e84b3bfa07eab4f4623e861cdefada92cdce Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 19:12:31 +0200 Subject: Verification: repurpose & rename `_check_users` Let's only use this function to check on the guild status. It can be exposed via a command in the future. Name adjusted to be more accurate w.r.t. Discord terminology. --- bot/cogs/verification.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 85a0e3ec4..4a9983ac8 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -162,16 +162,15 @@ class Verification(Cog): return n_success - async def check_users(self) -> None: + async def _check_members(self) -> t.Tuple[t.Set[discord.Member], t.Set[discord.Member]]: """ - Periodically check in on the verification status of PyDis members. + Check in on the verification status of PyDis members. - This coroutine performs two actions: - * Find members who have not verified for `UNVERIFIED_AFTER` and give them the @Unverified role - * Find members who have not verified for `KICKED_AFTER` and kick them from the guild + This coroutine finds two sets of users: + * Not verified after `UNVERIFIED_AFTER` days, should be given the @Unverified role + * Not verified after `KICKED_AFTER` days, should be kicked from the guild - Within the body of this coroutine, we only select the members for each action. The work is then - delegated to `_kick_members` and `_give_role`. After each run, a report is sent via modlog. + These sets are always disjoint, i.e. share no common members. """ await self.bot.wait_until_guild_available() # Ensure cache is ready pydis = self.bot.get_guild(constants.Guild.id) @@ -205,9 +204,8 @@ class Verification(Cog): elif since_join > timedelta(days=UNVERIFIED_AFTER) and unverified not in member.roles: for_role.add(member) # User should be given the @Unverified role - log.debug(f"{len(for_role)} users will be given the {unverified} role, {len(for_kick)} users will be kicked") - n_kicks = await self._kick_members(for_kick) - n_roles = await self._give_role(for_role, unverified) + log.debug(f"Found {len(for_role)} users for {unverified} role, {len(for_kick)} users to be kicked") + return for_role, for_kick @property def mod_log(self) -> ModLog: -- cgit v1.2.3 From 4b1100500681d1f4b670f91fe0566e4e85c8371b Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 19:33:42 +0200 Subject: Verification: create task to update unverified members --- bot/cogs/verification.py | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 4a9983ac8..cc8d8eb7d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -5,6 +5,7 @@ from contextlib import suppress from datetime import datetime, timedelta import discord +from discord.ext import tasks from discord.ext.commands import Cog, Context, command from bot import constants @@ -207,6 +208,42 @@ class Verification(Cog): log.debug(f"Found {len(for_role)} users for {unverified} role, {len(for_kick)} users to be kicked") return for_role, for_kick + @tasks.loop(minutes=30) + async def update_unverified_members(self) -> None: + """ + Periodically call `_check_members` and update unverified members accordingly. + + After each run, a summary will be sent to the modlog channel. If a suspiciously high + amount of members to be kicked is found, the operation is guarded by `_verify_kick`. + """ + log.info("Updating unverified guild members") + + await self.bot.wait_until_guild_available() + unverified = self.bot.get_guild(constants.Guild.id).get_role(constants.Roles.unverified) + + for_role, for_kick = await self._check_members() + + if not for_role: + role_report = f"Found no users to be assigned the {unverified.mention} role." + else: + n_roles = await self._give_role(for_role, unverified) + role_report = f"Assigned {unverified.mention} role to `{n_roles}`/`{len(for_role)}` members." + + if not for_kick: + kick_report = "Found no users to be kicked." + elif not await self._verify_kick(len(for_kick)): + kick_report = f"Not authorized to kick `{len(for_kick)}` members." + else: + n_kicks = await self._kick_members(for_kick) + kick_report = f"Kicked `{n_kicks}`/`{len(for_kick)}` members from the guild." + + await self.mod_log.send_log_message( + icon_url=self.bot.user.avatar_url, + colour=discord.Colour.blurple(), + title="Verification system", + text=f"{kick_report}\n{role_report}", + ) + @property def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" -- cgit v1.2.3 From 1305380f2552622cd51e1e22dded80ed2791af44 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 19:41:43 +0200 Subject: Verification: add region comments & move property to top Cog is getting large so let's allow collapsing related bits. --- bot/cogs/verification.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index cc8d8eb7d..951736761 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -60,6 +60,13 @@ class Verification(Cog): def __init__(self, bot: Bot): self.bot = bot + @property + def mod_log(self) -> ModLog: + """Get currently loaded ModLog cog instance.""" + return self.bot.get_cog("ModLog") + + # region: automatically update unverified users + async def _verify_kick(self, n_members: int) -> bool: """ Determine whether `n_members` is a reasonable amount of members to kick. @@ -244,10 +251,8 @@ class Verification(Cog): text=f"{kick_report}\n{role_report}", ) - @property - def mod_log(self) -> ModLog: - """Get currently loaded ModLog cog instance.""" - return self.bot.get_cog("ModLog") + # endregion + # region: listeners @Cog.listener() async def on_member_join(self, member: discord.Member) -> None: @@ -319,6 +324,9 @@ class Verification(Cog): with suppress(discord.NotFound): await ctx.message.delete() + # endregion + # region: accept and subscribe commands + @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(constants.Roles.verified) @in_whitelist(channels=(constants.Channels.verification,)) @@ -386,6 +394,9 @@ class Verification(Cog): f"{ctx.author.mention} Unsubscribed from <#{constants.Channels.announcements}> notifications." ) + # endregion + # region: miscellaneous + # This cannot be static (must have a __func__ attribute). async def cog_command_error(self, ctx: Context, error: Exception) -> None: """Check for & ignore any InWhitelistCheckFailure.""" @@ -400,6 +411,8 @@ class Verification(Cog): else: return True + # endregion + def setup(bot: Bot) -> None: """Load the Verification cog.""" -- cgit v1.2.3 From 18f2f1b8817f0209922112a0576b9b0377c2958d Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 19:44:17 +0200 Subject: Verification: schedule member update task Turns out that it's necessary to cancel the task manually. Otherwise, duplicate tasks can be running concurrently should the extension be reloaded. --- bot/cogs/verification.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 951736761..0534e8d1e 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -57,9 +57,20 @@ BOT_MESSAGE_DELETE_DELAY = 10 class Verification(Cog): """User verification and role self-management.""" - def __init__(self, bot: Bot): + def __init__(self, bot: Bot) -> None: + """Start `update_unverified_members` task.""" self.bot = bot + self.update_unverified_members.start() + + def cog_unload(self) -> None: + """ + Kill `update_unverified_members` task. + + This is necessary, the task is not automatically cancelled on cog unload. + """ + self.update_unverified_members.cancel() + @property def mod_log(self) -> ModLog: """Get currently loaded ModLog cog instance.""" -- cgit v1.2.3 From ef0e2049b64a0ba61878161d4ac7edb6015acbc2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 20:07:24 +0200 Subject: Verification: make authorization message ping core devs --- bot/cogs/verification.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 0534e8d1e..803cb055b 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -101,8 +101,9 @@ class Verification(Cog): core_devs = pydis.get_channel(constants.Channels.dev_core) confirmation_msg = await core_devs.send( - f"Verification determined that `{n_members}` members should be kicked as they haven't verified in " - f"`{KICKED_AFTER}` days. This is `{percentage:.2%}` of the guild's population. Proceed?" + f"<@&{constants.Roles.core_developers}> Verification determined that `{n_members}` members should " + f"be kicked as they haven't verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the " + f"guild's population. Proceed?" ) options = (constants.Emojis.incident_actioned, constants.Emojis.incident_unactioned) -- cgit v1.2.3 From f4721ba580c5d47d0cd5fb5beb60e6af54098244 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 20:11:01 +0200 Subject: Verification: move time constants above messages Allows referencing the constants within the message bodies. --- bot/cogs/verification.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 803cb055b..7244d041d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -16,6 +16,16 @@ from bot.utils.checks import InWhitelistCheckFailure, without_role_check log = logging.getLogger(__name__) +UNVERIFIED_AFTER = 3 # Amount of days after which non-Developers receive the @Unverified role +KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from the guild + +# Number in range [0, 1] determining the percentage of unverified users that are safe +# to be kicked from the guild in one batch, any larger amount will require staff confirmation, +# set this to 0 to require explicit approval for batches of any size +KICK_CONFIRMATION_THRESHOLD = 0 + +BOT_MESSAGE_DELETE_DELAY = 10 + ON_JOIN_MESSAGE = f""" Hello! Welcome to Python Discord! @@ -43,16 +53,6 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ -UNVERIFIED_AFTER = 3 # Amount of days after which non-Developers receive the @Unverified role -KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from the guild - -# Number in range [0, 1] determining the percentage of unverified users that are safe -# to be kicked from the guild in one batch, any larger amount will require staff confirmation, -# set this to 0 to require explicit approval for batches of any size -KICK_CONFIRMATION_THRESHOLD = 0 - -BOT_MESSAGE_DELETE_DELAY = 10 - class Verification(Cog): """User verification and role self-management.""" -- cgit v1.2.3 From f101f5608dd840ae79db353b562a7c2f800533b2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 20:12:48 +0200 Subject: Verification: add reminder ping message & frequency --- bot/cogs/verification.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 7244d041d..a01c25010 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -53,6 +53,17 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ +REMINDER_MESSAGE = f""" +<@&{constants.Roles.unverified}> + +Welcome to Python Discord! Please read the documents mentioned above and type `!accept` to gain permissions \ +to send messages in the community! + +You will be kicked if you don't verify within `{KICKED_AFTER}` days. +""" + +REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` + class Verification(Cog): """User verification and role self-management.""" -- cgit v1.2.3 From a8a583068e0ecacd9f2279a4e24fea0f5920fb51 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 20:13:59 +0200 Subject: Verification: comment message uses --- bot/cogs/verification.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index a01c25010..3502fe5b5 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -26,6 +26,7 @@ KICK_CONFIRMATION_THRESHOLD = 0 BOT_MESSAGE_DELETE_DELAY = 10 +# Sent via DMs once user joins the guild ON_JOIN_MESSAGE = f""" Hello! Welcome to Python Discord! @@ -33,6 +34,7 @@ In order to send messages, you first have to accept our rules. To do so, please <#{constants.Channels.verification}>. Thank you! """ +# Sent via DMs once user verifies VERIFIED_MESSAGE = f""" Thanks for verifying yourself! @@ -53,6 +55,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ +# Sent periodically in the verification channel REMINDER_MESSAGE = f""" <@&{constants.Roles.unverified}> -- cgit v1.2.3 From b1d761cecf5612d49de47c50994e12ab45b20e5e Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 20:19:00 +0200 Subject: Verification: add reminder cache --- bot/cogs/verification.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 3502fe5b5..e32224554 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -13,6 +13,7 @@ from bot.bot import Bot from bot.cogs.moderation import ModLog from bot.decorators import in_whitelist, without_role from bot.utils.checks import InWhitelistCheckFailure, without_role_check +from bot.utils.redis_cache import RedisCache log = logging.getLogger(__name__) @@ -71,6 +72,10 @@ REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` class Verification(Cog): """User verification and role self-management.""" + # Cache last sent `REMINDER_MESSAGE` id + # RedisCache[str, discord.Message.id] + reminder_cache = RedisCache() + def __init__(self, bot: Bot) -> None: """Start `update_unverified_members` task.""" self.bot = bot -- cgit v1.2.3 From 39d7b32b258def7f9fcf01bebb4f82013ae2de76 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 21:26:47 +0200 Subject: Verification: ignore verification reminder message event --- bot/cogs/verification.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index e32224554..ca7631db2 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -301,6 +301,9 @@ class Verification(Cog): if message.channel.id != constants.Channels.verification: return # Only listen for #checkpoint messages + if message.content == REMINDER_MESSAGE.strip(): + return # Ignore bots own verification reminder + if message.author.bot: # They're a bot, delete their message after the delay. await message.delete(delay=BOT_MESSAGE_DELETE_DELAY) -- cgit v1.2.3 From 8f548f158e17481245809801b1285b17af279fb4 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 22:12:12 +0200 Subject: Verification: implement unverified role ping task We're making good use of d.py's tasks framework. RedisCache is used to persist the reminder message ids, which can conveniently be converted into timestamps. It is therefore trivial to determine the time to sleep before the first ping. After that, the bot simply pings every n hours. --- bot/cogs/verification.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ca7631db2..42088896d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta import discord from discord.ext import tasks from discord.ext.commands import Cog, Context, command +from discord.utils import snowflake_time from bot import constants from bot.bot import Bot @@ -282,6 +283,57 @@ class Verification(Cog): text=f"{kick_report}\n{role_report}", ) + # endregion + # region: periodically ping @Unverified + + @tasks.loop(hours=REMINDER_FREQUENCY) + async def ping_unverified(self) -> None: + """ + Delete latest `REMINDER_MESSAGE` and send it again. + + This utilizes RedisCache to persist the latest reminder message id. + """ + await self.bot.wait_until_guild_available() + verification = self.bot.get_guild(constants.Guild.id).get_channel(constants.Channels.verification) + + last_reminder: t.Optional[int] = await self.reminder_cache.get("last_reminder") + + if last_reminder is not None: + log.trace(f"Found verification reminder message in cache, deleting: {last_reminder}") + + with suppress(discord.HTTPException): # If something goes wrong, just ignore it + await self.bot.http.delete_message(verification.id, last_reminder) + + log.trace("Sending verification reminder") + new_reminder = await verification.send(REMINDER_MESSAGE) + + await self.reminder_cache.set("last_reminder", new_reminder.id) + + @ping_unverified.before_loop + async def _before_first_ping(self) -> None: + """ + Sleep until `REMINDER_MESSAGE` should be sent again. + + If latest reminder is not cached, exit instantly. Otherwise, wait wait until the + configured `REMINDER_FREQUENCY` has passed. + """ + last_reminder: t.Optional[int] = await self.reminder_cache.get("last_reminder") + + if last_reminder is None: + log.trace("Latest verification reminder message not cached, task will not wait") + return + + # Convert cached message id into a timestamp + time_since = datetime.utcnow() - snowflake_time(last_reminder) + log.trace(f"Time since latest verification reminder: {time_since}") + + to_sleep = timedelta(hours=REMINDER_FREQUENCY) - time_since + log.trace(f"Time to sleep until next ping: {to_sleep}") + + # Delta can be negative if `REMINDER_FREQUENCY` has already passed + secs = max(to_sleep.total_seconds(), 0) + await asyncio.sleep(secs) + # endregion # region: listeners -- cgit v1.2.3 From 04db02199c72dc0855da8ac90cb514a750dd1f22 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Tue, 4 Aug 2020 22:13:15 +0200 Subject: Verification: schedule ping task --- bot/cogs/verification.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 42088896d..5586be040 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -78,18 +78,20 @@ class Verification(Cog): reminder_cache = RedisCache() def __init__(self, bot: Bot) -> None: - """Start `update_unverified_members` task.""" + """Start internal tasks.""" self.bot = bot self.update_unverified_members.start() + self.ping_unverified.start() def cog_unload(self) -> None: """ - Kill `update_unverified_members` task. + Cancel internal tasks. - This is necessary, the task is not automatically cancelled on cog unload. + This is necessary, as tasks are not automatically cancelled on cog unload. """ self.update_unverified_members.cancel() + self.ping_unverified.cancel() @property def mod_log(self) -> ModLog: -- cgit v1.2.3 From fb042c89cd519a41f39eba1559df58fc31a97832 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 10:53:57 +0200 Subject: Verification: remove unverified role on accept --- bot/cogs/verification.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 5586be040..5bc4f81c1 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -422,6 +422,11 @@ class Verification(Cog): """Accept our rules and gain access to the rest of the server.""" log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") await ctx.author.add_roles(discord.Object(constants.Roles.verified), reason="Accepted the rules") + + if constants.Roles.unverified in [role.id for role in ctx.author.roles]: + log.debug(f"Removing Unverified role from: {ctx.author}") + await ctx.author.remove_roles(discord.Object(constants.Roles.unverified)) + try: await ctx.author.send(VERIFIED_MESSAGE) except discord.Forbidden: -- cgit v1.2.3 From 78c19d2f57a41acce231d6950b45dde0fa8832c0 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 13:04:53 +0200 Subject: Verification: add stats collection --- bot/cogs/verification.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 5bc4f81c1..64ff4d8e6 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -176,6 +176,8 @@ class Verification(Cog): else: n_kicked += 1 + self.bot.stats.incr("verification.kicked", count=n_kicked) + if bad_statuses: log.info(f"Failed to kick {len(members) - n_kicked} members due to following statuses: {bad_statuses}") @@ -415,6 +417,30 @@ class Verification(Cog): # endregion # region: accept and subscribe commands + def _bump_verified_stats(self, verified_member: discord.Member) -> None: + """ + Increment verification stats for `verified_member`. + + Each member falls into one of the three categories: + * Verified within 24 hours after joining + * Does not have @Unverified role yet + * Does have @Unverified role + + Stats for member kicking are handled separately. + """ + if verified_member.joined_at is None: # Docs mention this can happen + return + + if (datetime.utcnow() - verified_member.joined_at) < timedelta(hours=24): + category = "accepted_on_day_one" + elif constants.Roles.unverified not in [role.id for role in verified_member.roles]: + category = "accepted_before_unverified" + else: + category = "accepted_after_unverified" + + log.trace(f"Bumping verification stats in category: {category}") + self.bot.stats.incr(f"verification.{category}") + @command(name='accept', aliases=('verify', 'verified', 'accepted'), hidden=True) @without_role(constants.Roles.verified) @in_whitelist(channels=(constants.Channels.verification,)) @@ -423,6 +449,8 @@ class Verification(Cog): log.debug(f"{ctx.author} called !accept. Assigning the 'Developer' role.") await ctx.author.add_roles(discord.Object(constants.Roles.verified), reason="Accepted the rules") + self._bump_verified_stats(ctx.author) # This checks for @Unverified so make sure it's not yet removed + if constants.Roles.unverified in [role.id for role in ctx.author.roles]: log.debug(f"Removing Unverified role from: {ctx.author}") await ctx.author.remove_roles(discord.Object(constants.Roles.unverified)) -- cgit v1.2.3 From b1c800c623f90f46c4ecaff8da2269efcd04ee05 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 13:12:06 +0200 Subject: Verification: disable burst shared filter in verification We will begin pinging users in the verification channel, prompting them to join. This can cause a surge of activity that may trigger the filter. A better solution would involve allowing per-filter channel config, but after internal discussion this is seen as unnecessary for now. --- bot/rules/burst_shared.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/bot/rules/burst_shared.py b/bot/rules/burst_shared.py index bbe9271b3..0e66df69c 100644 --- a/bot/rules/burst_shared.py +++ b/bot/rules/burst_shared.py @@ -2,11 +2,20 @@ from typing import Dict, Iterable, List, Optional, Tuple from discord import Member, Message +from bot.constants import Channels + async def apply( last_message: Message, recent_messages: List[Message], config: Dict[str, int] ) -> Optional[Tuple[str, Iterable[Member], Iterable[Message]]]: - """Detects repeated messages sent by multiple users.""" + """ + Detects repeated messages sent by multiple users. + + This filter never triggers in the verification channel. + """ + if last_message.channel.id == Channels.verification: + return + total_recent = len(recent_messages) if total_recent > config['max']: -- cgit v1.2.3 From 0a9769c4fa8d378fb7949212e8733531e9c1591a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 13:52:57 +0200 Subject: Verification: enable role pings --- bot/cogs/verification.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 64ff4d8e6..2872e704a 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -69,6 +69,13 @@ You will be kicked if you don't verify within `{KICKED_AFTER}` days. REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` +MENTION_CORE_DEVS = discord.AllowedMentions( + everyone=False, roles=[discord.Object(constants.Roles.core_developers)] +) +MENTION_UNVERIFIED = discord.AllowedMentions( + everyone=False, roles=[discord.Object(constants.Roles.unverified)] +) + class Verification(Cog): """User verification and role self-management.""" @@ -125,7 +132,8 @@ class Verification(Cog): confirmation_msg = await core_devs.send( f"<@&{constants.Roles.core_developers}> Verification determined that `{n_members}` members should " f"be kicked as they haven't verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the " - f"guild's population. Proceed?" + f"guild's population. Proceed?", + allowed_mentions=MENTION_CORE_DEVS, ) options = (constants.Emojis.incident_actioned, constants.Emojis.incident_unactioned) @@ -309,7 +317,7 @@ class Verification(Cog): await self.bot.http.delete_message(verification.id, last_reminder) log.trace("Sending verification reminder") - new_reminder = await verification.send(REMINDER_MESSAGE) + new_reminder = await verification.send(REMINDER_MESSAGE, allowed_mentions=MENTION_UNVERIFIED) await self.reminder_cache.set("last_reminder", new_reminder.id) -- cgit v1.2.3 From c9f2a2accea4a380eccec9f14fe389e230144242 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 14:02:27 +0200 Subject: Verification: send DM to kicked members --- bot/cogs/verification.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 2872e704a..ac488497a 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -57,6 +57,12 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! <#{constants.Channels.bot_commands}>. """ +# Sent via DMs to users kicked for failing to verify +KICKED_MESSAGE = f""" +Hi! You have been automatically kicked from Python Discord as you have failed to accept our rules \ +within `{KICKED_AFTER}` days. If this was an accident, please feel free to join again. +""" + # Sent periodically in the verification channel REMINDER_MESSAGE = f""" <@&{constants.Roles.unverified}> @@ -177,6 +183,8 @@ class Verification(Cog): n_kicked, bad_statuses = 0, set() for member in members: + with suppress(discord.Forbidden): + await member.send(KICKED_MESSAGE) # Send message while user is still in guild try: await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") except discord.HTTPException as http_exc: -- cgit v1.2.3 From c7eebaa1b70ad6bdd6a84bd0e980d5ea66f0002f Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 14:15:47 +0200 Subject: Verification: bump confirmation threshold to 1% --- bot/cogs/verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ac488497a..23b61a337 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -24,7 +24,7 @@ KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from t # Number in range [0, 1] determining the percentage of unverified users that are safe # to be kicked from the guild in one batch, any larger amount will require staff confirmation, # set this to 0 to require explicit approval for batches of any size -KICK_CONFIRMATION_THRESHOLD = 0 +KICK_CONFIRMATION_THRESHOLD = 0.01 # 1% BOT_MESSAGE_DELETE_DELAY = 10 -- cgit v1.2.3 From 92cbc03204a3fde78b919fec9d5f17144d24bd83 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 14:38:57 +0200 Subject: Verification: make on-join message more accurate It now explains that new users can only see a limited amount of public channels, and that there will be more once they verify. Co-authored-by: Sebastiaan Zeeff --- bot/cogs/verification.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 23b61a337..ff4b358c7 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -32,8 +32,10 @@ BOT_MESSAGE_DELETE_DELAY = 10 ON_JOIN_MESSAGE = f""" Hello! Welcome to Python Discord! -In order to send messages, you first have to accept our rules. To do so, please visit \ -<#{constants.Channels.verification}>. Thank you! +As a new user, you have read-only access to a few select channels to give you a taste of what our server is like. + +In order to see the rest of the channels and to send messages, you first have to accept our rules. To do so, \ +please visit <#{constants.Channels.verification}>. Thank you! """ # Sent via DMs once user verifies -- cgit v1.2.3 From 16eec3d2d69af5178b03fb574b0f277dbcf1dea8 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 18:17:38 +0200 Subject: Verification: extend cog docstring --- bot/cogs/verification.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ff4b358c7..963a2369e 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -86,7 +86,23 @@ MENTION_UNVERIFIED = discord.AllowedMentions( class Verification(Cog): - """User verification and role self-management.""" + """ + User verification and role management. + + There are two internal tasks in this cog: + + * `update_unverified_members` + * Unverified members are given the @Unverified role after `UNVERIFIED_AFTER` days + * Unverified members are kicked after `UNVERIFIED_AFTER` days + + * `ping_unverified` + * Periodically ping the @Unverified role in the verification channel + + Statistics are collected in the 'verification.' namespace. + + Additionally, this cog offers the !accept, !subscribe and !unsubscribe commands, + and keeps the verification channel clean by deleting messages. + """ # Cache last sent `REMINDER_MESSAGE` id # RedisCache[str, discord.Message.id] -- cgit v1.2.3 From fe50d6457081e0e6ef86d821bcfab81a8a164ca5 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 23:45:53 +0200 Subject: Verification: add command interface for task management Allow checking whether tasks are running, starting them, and stopping them. Currently, the tasks cannot be started or stopped separately. It is not believed that we would need such a level of granularity. Calling `cancel` on a task that isn't running is a no-op. --- bot/cogs/verification.py | 62 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 963a2369e..152118d92 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -6,13 +6,13 @@ from datetime import datetime, timedelta import discord from discord.ext import tasks -from discord.ext.commands import Cog, Context, command +from discord.ext.commands import Cog, Context, command, group from discord.utils import snowflake_time from bot import constants from bot.bot import Bot from bot.cogs.moderation import ModLog -from bot.decorators import in_whitelist, without_role +from bot.decorators import in_whitelist, with_role, without_role from bot.utils.checks import InWhitelistCheckFailure, without_role_check from bot.utils.redis_cache import RedisCache @@ -448,6 +448,64 @@ class Verification(Cog): with suppress(discord.NotFound): await ctx.message.delete() + # endregion + # region: task management commands + + @with_role(*constants.MODERATION_ROLES) + @group(name="verification") + async def verification_group(self, ctx: Context) -> None: + """Manage internal verification tasks.""" + if ctx.invoked_subcommand is None: + await ctx.send_help(ctx.command) + + @verification_group.command(name="status") + async def status_cmd(self, ctx: Context) -> None: + """Check whether verification tasks are running.""" + log.trace("Checking status of verification tasks") + + if self.update_unverified_members.is_running(): + update_status = f"{constants.Emojis.incident_actioned} Member update task is running." + else: + update_status = f"{constants.Emojis.incident_unactioned} Member update task is **not** running." + + mention = f"<@&{constants.Roles.unverified}>" + if self.ping_unverified.is_running(): + ping_status = f"{constants.Emojis.incident_actioned} Ping {mention} is running." + else: + ping_status = f"{constants.Emojis.incident_unactioned} Ping {mention} is **not** running." + + embed = discord.Embed( + title="Verification system", + description=f"{update_status}\n{ping_status}", + colour=discord.Colour.blurple(), + ) + await ctx.send(embed=embed) + + @verification_group.command(name="start") + async def start_cmd(self, ctx: Context) -> None: + """Start verification tasks if they are not already running.""" + log.info("Starting verification tasks") + + if not self.update_unverified_members.is_running(): + self.update_unverified_members.start() + + if not self.ping_unverified.is_running(): + self.ping_unverified.start() + + colour = discord.Colour.blurple() + await ctx.send(embed=discord.Embed(title="Verification system", description="Done. :ok_hand:", colour=colour)) + + @verification_group.command(name="stop", aliases=["kill"]) + async def stop_cmd(self, ctx: Context) -> None: + """Stop verification tasks.""" + log.info("Stopping verification tasks") + + self.update_unverified_members.cancel() + self.ping_unverified.cancel() + + colour = discord.Colour.blurple() + await ctx.send(embed=discord.Embed(title="Verification system", description="Tasks canceled.", colour=colour)) + # endregion # region: accept and subscribe commands -- cgit v1.2.3 From 2d24e4730e0f9678c0d7833c2332c7f0821eb7e2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 6 Aug 2020 23:48:06 +0200 Subject: Verification: persist task settings in Redis If tasks are stopped manually, they will not automatically restart on cog reload or bot restart. Using `maybe_start_tasks` is necessary because we cannot interface with Redis from a sync context. We're using 1 and 0 because RedisCache does not currently permit bool values due to a typestring conversion bug. --- bot/cogs/verification.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 152118d92..b4dc1f145 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -111,9 +111,7 @@ class Verification(Cog): def __init__(self, bot: Bot) -> None: """Start internal tasks.""" self.bot = bot - - self.update_unverified_members.start() - self.ping_unverified.start() + self.bot.loop.create_task(self.maybe_start_tasks()) def cog_unload(self) -> None: """ @@ -129,6 +127,20 @@ class Verification(Cog): """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") + async def maybe_start_tasks(self) -> None: + """ + Poll Redis to check whether internal tasks should start. + + Redis must be interfaced with from an async function. + """ + log.trace("Checking whether background tasks should begin") + setting: t.Optional[int] = await self.reminder_cache.get("tasks_running") # This can be None if never set + + if setting: + log.trace("Background tasks will be started") + self.update_unverified_members.start() + self.ping_unverified.start() + # region: automatically update unverified users async def _verify_kick(self, n_members: int) -> bool: @@ -492,6 +504,8 @@ class Verification(Cog): if not self.ping_unverified.is_running(): self.ping_unverified.start() + await self.reminder_cache.set("tasks_running", 1) + colour = discord.Colour.blurple() await ctx.send(embed=discord.Embed(title="Verification system", description="Done. :ok_hand:", colour=colour)) @@ -503,6 +517,8 @@ class Verification(Cog): self.update_unverified_members.cancel() self.ping_unverified.cancel() + await self.reminder_cache.set("tasks_running", 0) + colour = discord.Colour.blurple() await ctx.send(embed=discord.Embed(title="Verification system", description="Tasks canceled.", colour=colour)) -- cgit v1.2.3 From 80063705dc2264c1a320100f3620b5a384780699 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 7 Aug 2020 10:21:28 +0200 Subject: Verification: rename cache & document new use --- bot/cogs/verification.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index b4dc1f145..6b245d574 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -100,13 +100,19 @@ class Verification(Cog): Statistics are collected in the 'verification.' namespace. + Moderators+ can use the `verification` command group to start or stop both internal + tasks, if necessary. Settings are persisted in Redis across sessions. + Additionally, this cog offers the !accept, !subscribe and !unsubscribe commands, and keeps the verification channel clean by deleting messages. """ - # Cache last sent `REMINDER_MESSAGE` id - # RedisCache[str, discord.Message.id] - reminder_cache = RedisCache() + # Persist task settings & last sent `REMINDER_MESSAGE` id + # RedisCache[ + # "tasks_running": int (0 or 1), + # "last_reminder": int (discord.Message.id), + # ] + task_cache = RedisCache() def __init__(self, bot: Bot) -> None: """Start internal tasks.""" @@ -134,7 +140,7 @@ class Verification(Cog): Redis must be interfaced with from an async function. """ log.trace("Checking whether background tasks should begin") - setting: t.Optional[int] = await self.reminder_cache.get("tasks_running") # This can be None if never set + setting: t.Optional[int] = await self.task_cache.get("tasks_running") # This can be None if never set if setting: log.trace("Background tasks will be started") @@ -346,7 +352,7 @@ class Verification(Cog): await self.bot.wait_until_guild_available() verification = self.bot.get_guild(constants.Guild.id).get_channel(constants.Channels.verification) - last_reminder: t.Optional[int] = await self.reminder_cache.get("last_reminder") + last_reminder: t.Optional[int] = await self.task_cache.get("last_reminder") if last_reminder is not None: log.trace(f"Found verification reminder message in cache, deleting: {last_reminder}") @@ -357,7 +363,7 @@ class Verification(Cog): log.trace("Sending verification reminder") new_reminder = await verification.send(REMINDER_MESSAGE, allowed_mentions=MENTION_UNVERIFIED) - await self.reminder_cache.set("last_reminder", new_reminder.id) + await self.task_cache.set("last_reminder", new_reminder.id) @ping_unverified.before_loop async def _before_first_ping(self) -> None: @@ -367,7 +373,7 @@ class Verification(Cog): If latest reminder is not cached, exit instantly. Otherwise, wait wait until the configured `REMINDER_FREQUENCY` has passed. """ - last_reminder: t.Optional[int] = await self.reminder_cache.get("last_reminder") + last_reminder: t.Optional[int] = await self.task_cache.get("last_reminder") if last_reminder is None: log.trace("Latest verification reminder message not cached, task will not wait") @@ -504,7 +510,7 @@ class Verification(Cog): if not self.ping_unverified.is_running(): self.ping_unverified.start() - await self.reminder_cache.set("tasks_running", 1) + await self.task_cache.set("tasks_running", 1) colour = discord.Colour.blurple() await ctx.send(embed=discord.Embed(title="Verification system", description="Done. :ok_hand:", colour=colour)) @@ -517,7 +523,7 @@ class Verification(Cog): self.update_unverified_members.cancel() self.ping_unverified.cancel() - await self.reminder_cache.set("tasks_running", 0) + await self.task_cache.set("tasks_running", 0) colour = discord.Colour.blurple() await ctx.send(embed=discord.Embed(title="Verification system", description="Tasks canceled.", colour=colour)) -- cgit v1.2.3 From 6be9f0d24caa792b23d8f93ce9d87e48df3e92a5 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 7 Aug 2020 15:22:56 +0200 Subject: Verification: address member update race condition In an edge case, the `_kick_members` and `_give_role` could act on a member who has verified *after* being marked by `_check_members` as unverified. To address this, we perform one additional check just before sending the request. Testing seems to indicate that the `discord.Member` instance get updates as appropriate, so this should at least reduce the chances of such a race happening to very close to nil. --- bot/cogs/verification.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 6b245d574..ed03b0a14 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -85,6 +85,20 @@ MENTION_UNVERIFIED = discord.AllowedMentions( ) +def is_verified(member: discord.Member) -> bool: + """ + Check whether `member` is considered verified. + + Members are considered verified if they have at least 1 role other than + the default role (@everyone) and the @Unverified role. + """ + unverified_roles = { + member.guild.get_role(constants.Roles.unverified), + member.guild.default_role, + } + return bool(set(member.roles) - unverified_roles) + + class Verification(Cog): """ User verification and role management. @@ -219,6 +233,8 @@ class Verification(Cog): n_kicked, bad_statuses = 0, set() for member in members: + if is_verified(member): # Member could have verified in the meantime + continue with suppress(discord.Forbidden): await member.send(KICKED_MESSAGE) # Send message while user is still in guild try: @@ -246,6 +262,8 @@ class Verification(Cog): n_success, bad_statuses = 0, set() for member in members: + if is_verified(member): # Member could have verified in the meantime + continue try: await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") except discord.HTTPException as http_exc: @@ -280,14 +298,9 @@ class Verification(Cog): log.debug("Checking verification status of guild members") for member in pydis.members: - # Skip all bots and users for which we don't know their join date - # This should be extremely rare, but can happen according to `joined_at` docs - if member.bot or member.joined_at is None: - continue - - # Now we check roles to determine whether this user has already verified - unverified_roles = {unverified, pydis.default_role} # Verified users have at least one more role - if set(member.roles) - unverified_roles: + # Skip verified members, bots, and members for which we do not know their join date, + # this should be extremely rare but docs mention that it can happen + if is_verified(member) or member.bot or member.joined_at is None: continue # At this point, we know that `member` is an unverified user, and we will decide what -- cgit v1.2.3 From 553b6a9118dc21634d0fd78fdb58f98cb02c3c7f Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 8 Aug 2020 11:02:51 +0200 Subject: Verification: improve `is_verified` check This just reads better. Co-authored-by: MarkKoz --- bot/cogs/verification.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index ed03b0a14..d4064cff7 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -96,7 +96,7 @@ def is_verified(member: discord.Member) -> bool: member.guild.get_role(constants.Roles.unverified), member.guild.default_role, } - return bool(set(member.roles) - unverified_roles) + return len(set(member.roles) - unverified_roles) > 0 class Verification(Cog): -- cgit v1.2.3 From f3c16f77d812be50c2b7bed4c046cd67f3b9b761 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 8 Aug 2020 11:04:24 +0200 Subject: Verification: only take reactions from core devs Co-authored-by: MarkKoz --- bot/cogs/verification.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index d4064cff7..da2f81e2d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -196,12 +196,14 @@ class Verification(Cog): for option in options: await confirmation_msg.add_reaction(option) + core_dev_ids = [member.id for member in pydis.get_role(constants.Roles.core_developers).members] + def check(reaction: discord.Reaction, user: discord.User) -> bool: """Check whether `reaction` is a valid reaction to `confirmation_msg`.""" return ( reaction.message.id == confirmation_msg.id # Reacted to `confirmation_msg` and str(reaction.emoji) in options # With one of `options` - and not user.bot # By a human + and user.id in core_dev_ids # By a core developer ) timeout = 60 * 5 # Seconds, i.e. 5 minutes -- cgit v1.2.3 From 9b91847950a31f094c92a77974edc19d7766f514 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 8 Aug 2020 11:05:57 +0200 Subject: Verification: widen set type annotation Co-authored-by: MarkKoz --- bot/cogs/verification.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index da2f81e2d..9dc65da1c 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -224,7 +224,7 @@ class Verification(Cog): ) return result - async def _kick_members(self, members: t.Set[discord.Member]) -> int: + async def _kick_members(self, members: t.Collection[discord.Member]) -> int: """ Kick `members` from the PyDis guild. @@ -253,7 +253,7 @@ class Verification(Cog): return n_kicked - async def _give_role(self, members: t.Set[discord.Member], role: discord.Role) -> int: + async def _give_role(self, members: t.Collection[discord.Member], role: discord.Role) -> int: """ Give `role` to all `members`. -- cgit v1.2.3 From 174796a9bf8fcb117f38e8d6dc1a4b17c3849334 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 8 Aug 2020 11:50:50 +0200 Subject: Verification: strip reminder message once and for all Co-authored-by: MarkKoz --- bot/cogs/verification.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 9dc65da1c..a22b91e5d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -73,7 +73,7 @@ Welcome to Python Discord! Please read the documents mentioned above and type `! to send messages in the community! You will be kicked if you don't verify within `{KICKED_AFTER}` days. -""" +""".strip() REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` @@ -424,7 +424,7 @@ class Verification(Cog): if message.channel.id != constants.Channels.verification: return # Only listen for #checkpoint messages - if message.content == REMINDER_MESSAGE.strip(): + if message.content == REMINDER_MESSAGE: return # Ignore bots own verification reminder if message.author.bot: -- cgit v1.2.3 From 286cdccb21ed035d697128c2212d88368cb48e8d Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 8 Aug 2020 13:56:24 +0200 Subject: Verification: improve confirmation message handling Suppress errors coming from Discord when changing the confirmation message in case it gets deleted, or something else goes wrong. This commit also adds either the ok hand or the warning emoji to the edited message content, as with the guild syncer confirmation. Co-authored-by: MarkKoz --- bot/cogs/verification.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index a22b91e5d..cbf2c51c3 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -213,15 +213,21 @@ class Verification(Cog): log.debug("Staff prompt not answered, aborting operation") return False finally: - await confirmation_msg.clear_reactions() + with suppress(discord.HTTPException): + await confirmation_msg.clear_reactions() result = str(choice) == constants.Emojis.incident_actioned log.debug(f"Received answer: {choice}, result: {result}") # Edit the prompt message to reflect the final choice - await confirmation_msg.edit( - content=f"Request to kick `{n_members}` members was {'authorized' if result else 'denied'}!" - ) + if result is True: + result_msg = f":ok_hand: Request to kick `{n_members}` members was authorized!" + else: + result_msg = f":warning: Request to kick `{n_members}` members was denied!" + + with suppress(discord.HTTPException): + await confirmation_msg.edit(content=result_msg) + return result async def _kick_members(self, members: t.Collection[discord.Member]) -> int: -- cgit v1.2.3 From 1958978e71dc5bd9e4ae007091db72de147afc12 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 13 Aug 2020 18:49:47 +0200 Subject: Verification: add `_send_requests` helper Generic request dispatch method to avoid code duplication with error handling & bad status logging. --- bot/cogs/verification.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index cbf2c51c3..e89f491cf 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -84,6 +84,9 @@ MENTION_UNVERIFIED = discord.AllowedMentions( everyone=False, roles=[discord.Object(constants.Roles.unverified)] ) +# An async function taking a Member param +Request = t.Callable[[discord.Member], t.Awaitable] + def is_verified(member: discord.Member) -> bool: """ @@ -230,6 +233,33 @@ class Verification(Cog): return result + async def _send_requests(self, members: t.Collection[discord.Member], request: Request) -> int: + """ + Pass `members` one by one to `request` handling Discord exceptions. + + This coroutine serves as a generic `request` executor for kicking members and adding + roles, as it allows us to define the error handling logic in one place only. + + Returns the amount of successful requests. Failed requests are logged at info level. + """ + log.info(f"Sending {len(members)} requests") + n_success, bad_statuses = 0, set() + + for member in members: + if is_verified(member): # Member could have verified in the meantime + continue + try: + await request(member) + except discord.HTTPException as http_exc: + bad_statuses.add(http_exc.status) + else: + n_success += 1 + + if bad_statuses: + log.info(f"Failed to send {len(members) - n_success} requests due to following statuses: {bad_statuses}") + + return n_success + async def _kick_members(self, members: t.Collection[discord.Member]) -> int: """ Kick `members` from the PyDis guild. -- cgit v1.2.3 From dc70a018bfbdb5546c05af0a60e0c58dad5e4de1 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 13 Aug 2020 18:51:12 +0200 Subject: Verification: adjust coroutines to use generic dispatch --- bot/cogs/verification.py | 43 +++++++++++-------------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index e89f491cf..8f1a773a8 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -264,55 +264,34 @@ class Verification(Cog): """ Kick `members` from the PyDis guild. - Note that this is a potentially destructive operation. Returns the amount of successful - requests. Failed requests are logged at info level. + Note that this is a potentially destructive operation. Returns the amount of successful requests. """ log.info(f"Kicking {len(members)} members from the guild (not verified after {KICKED_AFTER} days)") - n_kicked, bad_statuses = 0, set() - for member in members: - if is_verified(member): # Member could have verified in the meantime - continue + async def kick_request(member: discord.Member) -> None: + """Send `KICKED_MESSAGE` to `member` and kick them from the guild.""" with suppress(discord.Forbidden): - await member.send(KICKED_MESSAGE) # Send message while user is still in guild - try: - await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") - except discord.HTTPException as http_exc: - bad_statuses.add(http_exc.status) - else: - n_kicked += 1 + await member.send(KICKED_MESSAGE) + await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") + n_kicked = await self._send_requests(members, kick_request) self.bot.stats.incr("verification.kicked", count=n_kicked) - if bad_statuses: - log.info(f"Failed to kick {len(members) - n_kicked} members due to following statuses: {bad_statuses}") - return n_kicked async def _give_role(self, members: t.Collection[discord.Member], role: discord.Role) -> int: """ Give `role` to all `members`. - Returns the amount of successful requests. Status codes of unsuccessful requests - are logged at info level. + Returns the amount of successful requests. """ log.info(f"Assigning {role} role to {len(members)} members (not verified after {UNVERIFIED_AFTER} days)") - n_success, bad_statuses = 0, set() - for member in members: - if is_verified(member): # Member could have verified in the meantime - continue - try: - await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") - except discord.HTTPException as http_exc: - bad_statuses.add(http_exc.status) - else: - n_success += 1 + async def role_request(member: discord.Member) -> None: + """Add `role` to `member`.""" + await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") - if bad_statuses: - log.info(f"Failed to assign {len(members) - n_success} roles due to following statuses: {bad_statuses}") - - return n_success + return await self._send_requests(members, role_request) async def _check_members(self) -> t.Tuple[t.Set[discord.Member], t.Set[discord.Member]]: """ -- cgit v1.2.3 From 689e5203f993604f1cb40be65e22b79ceb04d382 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Fri, 14 Aug 2020 19:39:07 +0200 Subject: Verification: pause request execution after each batch The Limit values are mostly assumptions, as this feature is very difficult to test at scale. Please see docstring amendmends for further information. --- bot/cogs/verification.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 8f1a773a8..14c0abfda 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -88,6 +88,13 @@ MENTION_UNVERIFIED = discord.AllowedMentions( Request = t.Callable[[discord.Member], t.Awaitable] +class Limit(t.NamedTuple): + """Composition over config for throttling requests.""" + + batch_size: int # Amount of requests after which to pause + sleep_secs: int # Sleep this many seconds after each batch + + def is_verified(member: discord.Member) -> bool: """ Check whether `member` is considered verified. @@ -233,19 +240,22 @@ class Verification(Cog): return result - async def _send_requests(self, members: t.Collection[discord.Member], request: Request) -> int: + async def _send_requests(self, members: t.Collection[discord.Member], request: Request, limit: Limit) -> int: """ Pass `members` one by one to `request` handling Discord exceptions. This coroutine serves as a generic `request` executor for kicking members and adding roles, as it allows us to define the error handling logic in one place only. + To avoid rate-limits, pass a `limit` configuring the batch size and the amount of seconds + to sleep between batches. + Returns the amount of successful requests. Failed requests are logged at info level. """ log.info(f"Sending {len(members)} requests") n_success, bad_statuses = 0, set() - for member in members: + for progress, member in enumerate(members, start=1): if is_verified(member): # Member could have verified in the meantime continue try: @@ -255,6 +265,10 @@ class Verification(Cog): else: n_success += 1 + if progress % limit.batch_size == 0: + log.trace(f"Processed {progress} requests, pausing for {limit.sleep_secs} seconds") + await asyncio.sleep(limit.sleep_secs) + if bad_statuses: log.info(f"Failed to send {len(members) - n_success} requests due to following statuses: {bad_statuses}") @@ -264,6 +278,9 @@ class Verification(Cog): """ Kick `members` from the PyDis guild. + Due to strict ratelimits on sending messages (120 requests / 60 secs), we sleep for a second + after each 2 requests to allow breathing room for other features. + Note that this is a potentially destructive operation. Returns the amount of successful requests. """ log.info(f"Kicking {len(members)} members from the guild (not verified after {KICKED_AFTER} days)") @@ -274,7 +291,7 @@ class Verification(Cog): await member.send(KICKED_MESSAGE) await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") - n_kicked = await self._send_requests(members, kick_request) + n_kicked = await self._send_requests(members, kick_request, Limit(batch_size=2, sleep_secs=1)) self.bot.stats.incr("verification.kicked", count=n_kicked) return n_kicked @@ -283,6 +300,8 @@ class Verification(Cog): """ Give `role` to all `members`. + We pause for a second after batches of 25 requests to ensure ratelimits aren't exceeded. + Returns the amount of successful requests. """ log.info(f"Assigning {role} role to {len(members)} members (not verified after {UNVERIFIED_AFTER} days)") @@ -291,7 +310,7 @@ class Verification(Cog): """Add `role` to `member`.""" await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") - return await self._send_requests(members, role_request) + return await self._send_requests(members, role_request, Limit(batch_size=25, sleep_secs=1)) async def _check_members(self) -> t.Tuple[t.Set[discord.Member], t.Set[discord.Member]]: """ -- cgit v1.2.3 From ff91b76348d95e308589ac131898aba9f7cca986 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 19 Aug 2020 20:06:23 +0200 Subject: Verification: add missing word to task status message --- bot/cogs/verification.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 14c0abfda..08d54d575 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -537,9 +537,9 @@ class Verification(Cog): mention = f"<@&{constants.Roles.unverified}>" if self.ping_unverified.is_running(): - ping_status = f"{constants.Emojis.incident_actioned} Ping {mention} is running." + ping_status = f"{constants.Emojis.incident_actioned} Ping {mention} task is running." else: - ping_status = f"{constants.Emojis.incident_unactioned} Ping {mention} is **not** running." + ping_status = f"{constants.Emojis.incident_unactioned} Ping {mention} task is **not** running." embed = discord.Embed( title="Verification system", -- cgit v1.2.3 From a565bb529cbfcb241a7cf63c114a17f695451721 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 18:31:42 +0200 Subject: Verification: add guild invite to config --- bot/constants.py | 1 + config-default.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/bot/constants.py b/bot/constants.py index 0902858ac..daef6c095 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -458,6 +458,7 @@ class Guild(metaclass=YAMLGetter): section = "guild" id: int + invite: str # Discord invite, gets embedded in chat moderation_channels: List[int] moderation_roles: List[int] modlog_blacklist: List[int] diff --git a/config-default.yml b/config-default.yml index 58bdbe20f..a98fd14ef 100644 --- a/config-default.yml +++ b/config-default.yml @@ -123,6 +123,7 @@ style: guild: id: 267624335836053506 + invite: "https://discord.gg/python" categories: help_available: 691405807388196926 -- cgit v1.2.3 From 53189e815a5c260bb2636914ecd79f4f4c1182a0 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 18:33:16 +0200 Subject: Verification: send guild invite with kick message Makes it easy for users to re-join. Co-authored-by: Joe Banks --- bot/cogs/verification.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 08d54d575..56b469a3e 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -62,7 +62,8 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! # Sent via DMs to users kicked for failing to verify KICKED_MESSAGE = f""" Hi! You have been automatically kicked from Python Discord as you have failed to accept our rules \ -within `{KICKED_AFTER}` days. If this was an accident, please feel free to join again. +within `{KICKED_AFTER}` days. If this was an accident, please feel free to join us again! +{constants.Guild.invite} """ # Sent periodically in the verification channel -- cgit v1.2.3 From 3ea04e3ddeaeef1d7931df8f5b84293d5eac6a04 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 18:50:57 +0200 Subject: Verification: separate guild invite by empty line Co-authored-by: Joe Banks --- bot/cogs/verification.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 56b469a3e..a35681988 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -63,6 +63,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! KICKED_MESSAGE = f""" Hi! You have been automatically kicked from Python Discord as you have failed to accept our rules \ within `{KICKED_AFTER}` days. If this was an accident, please feel free to join us again! + {constants.Guild.invite} """ -- cgit v1.2.3 From eff2f75321fce0e8a8d11a1c85c2dad48552ded8 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 19:34:14 +0200 Subject: Verification: remove explicit everyones from allowed mentions If the kwarg isn't passed, it uses the value that was given to the bot on init (False), despite the kwarg defaulting to True. Thanks to Mark and Senjan for helping me understand this. Co-authored-by: MarkKoz Co-authored-by: Senjan21 <53477086+senjan21@users.noreply.github.com> --- bot/cogs/verification.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index a35681988..a0a82be0c 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -79,12 +79,8 @@ You will be kicked if you don't verify within `{KICKED_AFTER}` days. REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` -MENTION_CORE_DEVS = discord.AllowedMentions( - everyone=False, roles=[discord.Object(constants.Roles.core_developers)] -) -MENTION_UNVERIFIED = discord.AllowedMentions( - everyone=False, roles=[discord.Object(constants.Roles.unverified)] -) +MENTION_CORE_DEVS = discord.AllowedMentions(roles=[discord.Object(constants.Roles.core_developers)]) +MENTION_UNVERIFIED = discord.AllowedMentions(roles=[discord.Object(constants.Roles.unverified)]) # An async function taking a Member param Request = t.Callable[[discord.Member], t.Awaitable] -- cgit v1.2.3 From 44aae4528ecec5eef8e2b56f7ac851219b35f080 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 19:42:31 +0200 Subject: Verification: retain ping in edited confirmation msg Prevent a ghost ping from occurring upon reaction. Co-authored-by: Senjan21 <53477086+senjan21@users.noreply.github.com> --- bot/cogs/verification.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index a0a82be0c..d21395a1c 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -192,11 +192,12 @@ class Verification(Cog): # Since `n_members` is a suspiciously large number, we will ask for confirmation log.debug("Amount of users is too large, requesting staff confirmation") - core_devs = pydis.get_channel(constants.Channels.dev_core) - confirmation_msg = await core_devs.send( - f"<@&{constants.Roles.core_developers}> Verification determined that `{n_members}` members should " - f"be kicked as they haven't verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the " - f"guild's population. Proceed?", + core_dev_channel = pydis.get_channel(constants.Channels.dev_core) + core_dev_ping = f"<@&{constants.Roles.core_developers}>" + + confirmation_msg = await core_dev_channel.send( + f"{core_dev_ping} Verification determined that `{n_members}` members should be kicked as they haven't " + f"verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the guild's population. Proceed?", allowed_mentions=MENTION_CORE_DEVS, ) @@ -229,9 +230,9 @@ class Verification(Cog): # Edit the prompt message to reflect the final choice if result is True: - result_msg = f":ok_hand: Request to kick `{n_members}` members was authorized!" + result_msg = f":ok_hand: {core_dev_ping} Request to kick `{n_members}` members was authorized!" else: - result_msg = f":warning: Request to kick `{n_members}` members was denied!" + result_msg = f":warning: {core_dev_ping} Request to kick `{n_members}` members was denied!" with suppress(discord.HTTPException): await confirmation_msg.edit(content=result_msg) -- cgit v1.2.3 From 3672da9d6ad16452205e00a86162314f457fbbd0 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 22:26:15 +0200 Subject: Verification: add helper for alerting admins --- bot/cogs/verification.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index d21395a1c..d5216c7c0 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -79,6 +79,7 @@ You will be kicked if you don't verify within `{KICKED_AFTER}` days. REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` +MENTION_ADMINS = discord.AllowedMentions(roles=[discord.Object(constants.Roles.admins)]) MENTION_CORE_DEVS = discord.AllowedMentions(roles=[discord.Object(constants.Roles.core_developers)]) MENTION_UNVERIFIED = discord.AllowedMentions(roles=[discord.Object(constants.Roles.unverified)]) @@ -239,6 +240,23 @@ class Verification(Cog): return result + async def _alert_admins(self, exception: discord.HTTPException) -> None: + """ + Ping @Admins with information about `exception`. + + This is used when a critical `exception` caused a verification task to abort. + """ + await self.bot.wait_until_guild_available() + log.info(f"Sending admin alert regarding exception: {exception}") + + admins_channel = self.bot.get_guild(constants.Guild.id).get_channel(constants.Channels.admins) + ping = f"<@&{constants.Roles.admins}>" + + await admins_channel.send( + f"{ping} Aborted updating unverified users due to the following exception:\n```{exception}```", + allowed_mentions=MENTION_ADMINS, + ) + async def _send_requests(self, members: t.Collection[discord.Member], request: Request, limit: Limit) -> int: """ Pass `members` one by one to `request` handling Discord exceptions. -- cgit v1.2.3 From 1d6d845d2f052c74d6d92a1a98b537430296cc85 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Wed, 26 Aug 2020 22:27:47 +0200 Subject: Verification: stop kicking members on suspicious 403 A Discord error code 50_0007 signifies that the DM dispatch failed because the target user does not accept DMs from the bot. Such errors are ignored as before. Any other 403s will however cause the bot to stop making requests. This is in case the bot gets caught by an anti-spam filter and should immediately stop. --- bot/cogs/verification.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index d5216c7c0..196808b0d 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -87,6 +87,14 @@ MENTION_UNVERIFIED = discord.AllowedMentions(roles=[discord.Object(constants.Rol Request = t.Callable[[discord.Member], t.Awaitable] +class StopExecution(Exception): + """Signals that a task should halt immediately & alert admins.""" + + def __init__(self, reason: discord.HTTPException) -> None: + super().__init__() + self.reason = reason + + class Limit(t.NamedTuple): """Composition over config for throttling requests.""" @@ -277,6 +285,9 @@ class Verification(Cog): continue try: await request(member) + except StopExecution as stop_execution: + await self._alert_admins(stop_execution.reason) + break except discord.HTTPException as http_exc: bad_statuses.add(http_exc.status) else: @@ -304,8 +315,12 @@ class Verification(Cog): async def kick_request(member: discord.Member) -> None: """Send `KICKED_MESSAGE` to `member` and kick them from the guild.""" - with suppress(discord.Forbidden): + try: await member.send(KICKED_MESSAGE) + except discord.Forbidden as exc_403: + log.trace(f"DM dispatch failed on 403 error with code: {exc_403.code}") + if exc_403.code != 50_007: # 403 raised for any other reason than disabled DMs + raise StopExecution(reason=exc_403) await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") n_kicked = await self._send_requests(members, kick_request, Limit(batch_size=2, sleep_secs=1)) -- cgit v1.2.3 From 0ba23a7c47813280ab6157396e43d61e8fc7b4d2 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 27 Aug 2020 10:07:23 +0200 Subject: Verification: document StopExecution handling --- bot/cogs/verification.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 196808b0d..984f3cc95 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -272,6 +272,9 @@ class Verification(Cog): This coroutine serves as a generic `request` executor for kicking members and adding roles, as it allows us to define the error handling logic in one place only. + Any `request` has the ability to completely abort the execution by raising `StopExecution`. + In such a case, the @Admins will be alerted of the reason attribute. + To avoid rate-limits, pass a `limit` configuring the batch size and the amount of seconds to sleep between batches. -- cgit v1.2.3 From 69314d4b812361f2b2a02018093f9a504ac4674f Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 27 Aug 2020 14:29:21 +0200 Subject: Verification: improve allowed mentions handling I really didn't like the constants, but the construction of allowed mentions instances is syntactically noisy, so I prefer to keep it out of the important logic. Abstracting it behind a function seems to be the best approach yet. --- bot/cogs/verification.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 984f3cc95..0ae3c5b4c 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -79,10 +79,6 @@ You will be kicked if you don't verify within `{KICKED_AFTER}` days. REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` -MENTION_ADMINS = discord.AllowedMentions(roles=[discord.Object(constants.Roles.admins)]) -MENTION_CORE_DEVS = discord.AllowedMentions(roles=[discord.Object(constants.Roles.core_developers)]) -MENTION_UNVERIFIED = discord.AllowedMentions(roles=[discord.Object(constants.Roles.unverified)]) - # An async function taking a Member param Request = t.Callable[[discord.Member], t.Awaitable] @@ -102,6 +98,11 @@ class Limit(t.NamedTuple): sleep_secs: int # Sleep this many seconds after each batch +def mention_role(role_id: int) -> discord.AllowedMentions: + """Construct an allowed mentions instance that allows pinging `role_id`.""" + return discord.AllowedMentions(roles=[discord.Object(role_id)]) + + def is_verified(member: discord.Member) -> bool: """ Check whether `member` is considered verified. @@ -207,7 +208,7 @@ class Verification(Cog): confirmation_msg = await core_dev_channel.send( f"{core_dev_ping} Verification determined that `{n_members}` members should be kicked as they haven't " f"verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the guild's population. Proceed?", - allowed_mentions=MENTION_CORE_DEVS, + allowed_mentions=mention_role(constants.Roles.core_developers), ) options = (constants.Emojis.incident_actioned, constants.Emojis.incident_unactioned) @@ -262,7 +263,7 @@ class Verification(Cog): await admins_channel.send( f"{ping} Aborted updating unverified users due to the following exception:\n```{exception}```", - allowed_mentions=MENTION_ADMINS, + allowed_mentions=mention_role(constants.Roles.admins), ) async def _send_requests(self, members: t.Collection[discord.Member], request: Request, limit: Limit) -> int: @@ -445,7 +446,9 @@ class Verification(Cog): await self.bot.http.delete_message(verification.id, last_reminder) log.trace("Sending verification reminder") - new_reminder = await verification.send(REMINDER_MESSAGE, allowed_mentions=MENTION_UNVERIFIED) + new_reminder = await verification.send( + REMINDER_MESSAGE, allowed_mentions=mention_role(constants.Roles.unverified), + ) await self.task_cache.set("last_reminder", new_reminder.id) -- cgit v1.2.3 From c58ae662069d098dae45a36a5203b6d5f0048924 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 29 Aug 2020 18:29:24 +0200 Subject: Verification: add helper for stopping tasks --- bot/cogs/verification.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 0ae3c5b4c..a013a1b12 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -157,8 +157,7 @@ class Verification(Cog): This is necessary, as tasks are not automatically cancelled on cog unload. """ - self.update_unverified_members.cancel() - self.ping_unverified.cancel() + self._stop_tasks(gracefully=False) @property def mod_log(self) -> ModLog: @@ -179,6 +178,21 @@ class Verification(Cog): self.update_unverified_members.start() self.ping_unverified.start() + def _stop_tasks(self, *, gracefully: bool) -> None: + """ + Stop the update users & ping @Unverified tasks. + + If `gracefully` is True, the tasks will be able to finish their current iteration. + Otherwise, they are cancelled immediately. + """ + log.info(f"Stopping internal tasks ({gracefully=})") + if gracefully: + self.update_unverified_members.stop() + self.ping_unverified.stop() + else: + self.update_unverified_members.cancel() + self.ping_unverified.cancel() + # region: automatically update unverified users async def _verify_kick(self, n_members: int) -> bool: @@ -607,9 +621,7 @@ class Verification(Cog): """Stop verification tasks.""" log.info("Stopping verification tasks") - self.update_unverified_members.cancel() - self.ping_unverified.cancel() - + self._stop_tasks(gracefully=False) await self.task_cache.set("tasks_running", 0) colour = discord.Colour.blurple() -- cgit v1.2.3 From cf7388ff8490be95f1d677f424e8bec86de0e46a Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 29 Aug 2020 18:32:12 +0200 Subject: Verification: stop tasks on suspicious 403 --- bot/cogs/verification.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index a013a1b12..107ae1178 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -276,7 +276,9 @@ class Verification(Cog): ping = f"<@&{constants.Roles.admins}>" await admins_channel.send( - f"{ping} Aborted updating unverified users due to the following exception:\n```{exception}```", + f"{ping} Aborted updating unverified users due to the following exception:\n" + f"```{exception}```\n" + f"Internal tasks will be stopped.", allowed_mentions=mention_role(constants.Roles.admins), ) @@ -305,6 +307,7 @@ class Verification(Cog): await request(member) except StopExecution as stop_execution: await self._alert_admins(stop_execution.reason) + self._stop_tasks(gracefully=True) # Gracefully finish current iteration, then stop break except discord.HTTPException as http_exc: bad_statuses.add(http_exc.status) -- cgit v1.2.3 From ad305f2eb3bb33f5ce7bb3abd7def9f436928c8e Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 29 Aug 2020 18:37:32 +0200 Subject: Verification: denote `_maybe_start_tasks` as private Consistency with the new `_stop_tasks` method. --- bot/cogs/verification.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 107ae1178..5c8962577 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -149,7 +149,7 @@ class Verification(Cog): def __init__(self, bot: Bot) -> None: """Start internal tasks.""" self.bot = bot - self.bot.loop.create_task(self.maybe_start_tasks()) + self.bot.loop.create_task(self._maybe_start_tasks()) def cog_unload(self) -> None: """ @@ -164,7 +164,7 @@ class Verification(Cog): """Get currently loaded ModLog cog instance.""" return self.bot.get_cog("ModLog") - async def maybe_start_tasks(self) -> None: + async def _maybe_start_tasks(self) -> None: """ Poll Redis to check whether internal tasks should start. -- cgit v1.2.3 From 6c98df046c535282c6d4b194b0766afcddbdf669 Mon Sep 17 00:00:00 2001 From: kwzrd Date: Sat, 29 Aug 2020 19:54:27 +0200 Subject: Verification: set 'tasks_running' to 0 on suspicious 403s Prevent the tasks from starting again if the bot restarts. --- bot/cogs/verification.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 5c8962577..08f7c282e 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -307,6 +307,7 @@ class Verification(Cog): await request(member) except StopExecution as stop_execution: await self._alert_admins(stop_execution.reason) + await self.task_cache.set("tasks_running", 0) self._stop_tasks(gracefully=True) # Gracefully finish current iteration, then stop break except discord.HTTPException as http_exc: -- cgit v1.2.3 From 1b3a5c5d90bb658895fea8a2bed91366d2f2f76e Mon Sep 17 00:00:00 2001 From: kwzrd Date: Thu, 10 Sep 2020 18:41:07 +0200 Subject: Verification: move constants to config --- bot/cogs/verification.py | 45 ++++++++++++++++++++------------------------- bot/constants.py | 10 ++++++++++ config-default.yml | 13 +++++++++++++ 3 files changed, 43 insertions(+), 25 deletions(-) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 08f7c282e..0092a0898 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -18,16 +18,6 @@ from bot.utils.redis_cache import RedisCache log = logging.getLogger(__name__) -UNVERIFIED_AFTER = 3 # Amount of days after which non-Developers receive the @Unverified role -KICKED_AFTER = 30 # Amount of days after which non-Developers get kicked from the guild - -# Number in range [0, 1] determining the percentage of unverified users that are safe -# to be kicked from the guild in one batch, any larger amount will require staff confirmation, -# set this to 0 to require explicit approval for batches of any size -KICK_CONFIRMATION_THRESHOLD = 0.01 # 1% - -BOT_MESSAGE_DELETE_DELAY = 10 - # Sent via DMs once user joins the guild ON_JOIN_MESSAGE = f""" Hello! Welcome to Python Discord! @@ -62,7 +52,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! # Sent via DMs to users kicked for failing to verify KICKED_MESSAGE = f""" Hi! You have been automatically kicked from Python Discord as you have failed to accept our rules \ -within `{KICKED_AFTER}` days. If this was an accident, please feel free to join us again! +within `{constants.Verification.kicked_after}` days. If this was an accident, please feel free to join us again! {constants.Guild.invite} """ @@ -74,11 +64,9 @@ REMINDER_MESSAGE = f""" Welcome to Python Discord! Please read the documents mentioned above and type `!accept` to gain permissions \ to send messages in the community! -You will be kicked if you don't verify within `{KICKED_AFTER}` days. +You will be kicked if you don't verify within `{constants.Verification.kicked_after}` days. """.strip() -REMINDER_FREQUENCY = 28 # Hours to wait between sending `REMINDER_MESSAGE` - # An async function taking a Member param Request = t.Callable[[discord.Member], t.Awaitable] @@ -209,7 +197,7 @@ class Verification(Cog): pydis = self.bot.get_guild(constants.Guild.id) percentage = n_members / len(pydis.members) - if percentage < KICK_CONFIRMATION_THRESHOLD: + if percentage < constants.Verification.kick_confirmation_threshold: log.debug(f"Kicking {percentage:.2%} of the guild's population is seen as safe") return True @@ -221,7 +209,8 @@ class Verification(Cog): confirmation_msg = await core_dev_channel.send( f"{core_dev_ping} Verification determined that `{n_members}` members should be kicked as they haven't " - f"verified in `{KICKED_AFTER}` days. This is `{percentage:.2%}` of the guild's population. Proceed?", + f"verified in `{constants.Verification.kicked_after}` days. This is `{percentage:.2%}` of the guild's " + f"population. Proceed?", allowed_mentions=mention_role(constants.Roles.core_developers), ) @@ -333,7 +322,7 @@ class Verification(Cog): Note that this is a potentially destructive operation. Returns the amount of successful requests. """ - log.info(f"Kicking {len(members)} members from the guild (not verified after {KICKED_AFTER} days)") + log.info(f"Kicking {len(members)} members (not verified after {constants.Verification.kicked_after} days)") async def kick_request(member: discord.Member) -> None: """Send `KICKED_MESSAGE` to `member` and kick them from the guild.""" @@ -343,7 +332,7 @@ class Verification(Cog): log.trace(f"DM dispatch failed on 403 error with code: {exc_403.code}") if exc_403.code != 50_007: # 403 raised for any other reason than disabled DMs raise StopExecution(reason=exc_403) - await member.kick(reason=f"User has not verified in {KICKED_AFTER} days") + await member.kick(reason=f"User has not verified in {constants.Verification.kicked_after} days") n_kicked = await self._send_requests(members, kick_request, Limit(batch_size=2, sleep_secs=1)) self.bot.stats.incr("verification.kicked", count=n_kicked) @@ -358,11 +347,14 @@ class Verification(Cog): Returns the amount of successful requests. """ - log.info(f"Assigning {role} role to {len(members)} members (not verified after {UNVERIFIED_AFTER} days)") + log.info( + f"Assigning {role} role to {len(members)} members (not verified " + f"after {constants.Verification.unverified_after} days)" + ) async def role_request(member: discord.Member) -> None: """Add `role` to `member`.""" - await member.add_roles(role, reason=f"User has not verified in {UNVERIFIED_AFTER} days") + await member.add_roles(role, reason=f"Not verified after {constants.Verification.unverified_after} days") return await self._send_requests(members, role_request, Limit(batch_size=25, sleep_secs=1)) @@ -397,10 +389,13 @@ class Verification(Cog): # to do with them based on time passed since their join date since_join = current_dt - member.joined_at - if since_join > timedelta(days=KICKED_AFTER): + if since_join > timedelta(days=constants.Verification.kicked_after): for_kick.add(member) # User should be removed from the guild - elif since_join > timedelta(days=UNVERIFIED_AFTER) and unverified not in member.roles: + elif ( + since_join > timedelta(days=constants.Verification.unverified_after) + and unverified not in member.roles + ): for_role.add(member) # User should be given the @Unverified role log.debug(f"Found {len(for_role)} users for {unverified} role, {len(for_kick)} users to be kicked") @@ -445,7 +440,7 @@ class Verification(Cog): # endregion # region: periodically ping @Unverified - @tasks.loop(hours=REMINDER_FREQUENCY) + @tasks.loop(hours=constants.Verification.reminder_frequency) async def ping_unverified(self) -> None: """ Delete latest `REMINDER_MESSAGE` and send it again. @@ -488,7 +483,7 @@ class Verification(Cog): time_since = datetime.utcnow() - snowflake_time(last_reminder) log.trace(f"Time since latest verification reminder: {time_since}") - to_sleep = timedelta(hours=REMINDER_FREQUENCY) - time_since + to_sleep = timedelta(hours=constants.Verification.reminder_frequency) - time_since log.trace(f"Time to sleep until next ping: {to_sleep}") # Delta can be negative if `REMINDER_FREQUENCY` has already passed @@ -519,7 +514,7 @@ class Verification(Cog): if message.author.bot: # They're a bot, delete their message after the delay. - await message.delete(delay=BOT_MESSAGE_DELETE_DELAY) + await message.delete(delay=constants.Verification.bot_message_delete_delay) return # if a user mentions a role or guild member diff --git a/bot/constants.py b/bot/constants.py index daef6c095..820828a19 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -589,6 +589,16 @@ class PythonNews(metaclass=YAMLGetter): webhook: int +class Verification(metaclass=YAMLGetter): + section = "verification" + + unverified_after: int + kicked_after: int + reminder_frequency: int + bot_message_delete_delay: int + kick_confirmation_threshold: float + + class Event(Enum): """ Event names. This does not include every event (for example, raw diff --git a/config-default.yml b/config-default.yml index a98fd14ef..c89695bd9 100644 --- a/config-default.yml +++ b/config-default.yml @@ -493,5 +493,18 @@ python_news: channel: *PYNEWS_CHANNEL webhook: *PYNEWS_WEBHOOK + +verification: + unverified_after: 3 # Days after which non-Developers receive the @Unverified role + kicked_after: 30 # Days after which non-Developers get kicked from the guild + reminder_frequency: 28 # Hours between @Unverified pings + bot_message_delete_delay: 10 # Seconds before deleting bots response in #verification + + # Number in range [0, 1] determining the percentage of unverified users that are safe + # to be kicked from the guild in one batch, any larger amount will require staff confirmation, + # set this to 0 to require explicit approval for batches of any size + kick_confirmation_threshold: 0.01 # 1% + + config: required_keys: ['bot.token'] -- cgit v1.2.3