aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGravatar kwzrd <[email protected]>2020-08-07 15:22:56 +0200
committerGravatar kwzrd <[email protected]>2020-08-07 15:22:56 +0200
commit6be9f0d24caa792b23d8f93ce9d87e48df3e92a5 (patch)
tree8fe49b3924922f031ac3fc5c18eef4e634b45759
parentVerification: rename cache & document new use (diff)
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.
-rw-r--r--bot/cogs/verification.py29
1 files 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