diff options
| author | 2020-08-07 15:22:56 +0200 | |
|---|---|---|
| committer | 2020-08-07 15:22:56 +0200 | |
| commit | 6be9f0d24caa792b23d8f93ce9d87e48df3e92a5 (patch) | |
| tree | 8fe49b3924922f031ac3fc5c18eef4e634b45759 | |
| parent | Verification: 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.py | 29 |
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 |