From 86a2b550b980361a99314632c3e6e0a112a0d43d Mon Sep 17 00:00:00 2001 From: sco1 Date: Mon, 7 Jan 2019 19:15:22 -0500 Subject: Add role hierarchy comparison check method --- bot/cogs/moderation.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index ac08d3dd4..a15a1b71c 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1207,6 +1207,19 @@ class Moderation(Scheduler): if User in error.converters: await ctx.send(str(error.errors[0])) + async def respect_role_hierarchy(self, guild: Guild, actor: Member, target: Member) -> bool: + """ + Check if the highest role of the invoking member is less than or equal to the target member + + Implement as a method rather than a check in order to avoid having to reimplement parameter + checks & conversions in a dedicated check decorater + """ + + # Build role hierarchy + role_hierarchy = {role: rank for rank, role in enumerate(reversed(guild.roles))} + + return role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + def setup(bot): bot.add_cog(Moderation(bot)) -- cgit v1.2.3 From 646a892348af4cf99fe2b08d6a1688e0aff56d3b Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:40:54 -0500 Subject: Add hierarchy check to desired infractions Per #134, only implement for all variations of the ban & kick infractions Update helper method inputs to facilitate a generic logging & context message --- bot/cogs/moderation.py | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index a15a1b71c..057b51bbf 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -125,6 +125,11 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ + if not self.respect_role_hierarchy(ctx, user, 'kick'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Kick", @@ -171,6 +176,11 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ + if not self.respect_role_hierarchy(ctx, user, 'ban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Ban", @@ -327,6 +337,11 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ + if not self.respect_role_hierarchy(ctx, user, 'tempban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + notified = await self.notify_infraction( user=user, infr_type="Ban", @@ -420,6 +435,11 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowkick'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="kick", reason=reason, hidden=True) if response_object is None: return @@ -456,6 +476,11 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="ban", reason=reason, hidden=True) if response_object is None: return @@ -581,6 +606,11 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ + if not self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + # Ensure ctx author has a higher top role than the target user + # Warning is sent to ctx by the helper method + return + response_object = await post_infraction(ctx, user, type="ban", reason=reason, duration=duration, hidden=True) if response_object is None: return @@ -1207,18 +1237,29 @@ class Moderation(Scheduler): if User in error.converters: await ctx.send(str(error.errors[0])) - async def respect_role_hierarchy(self, guild: Guild, actor: Member, target: Member) -> bool: + async def respect_role_hierarchy(self, ctx: Context, target: Member, infraction_type: str) -> bool: """ Check if the highest role of the invoking member is less than or equal to the target member + If this check fails, a warning is sent to the invoking ctx + Implement as a method rather than a check in order to avoid having to reimplement parameter checks & conversions in a dedicated check decorater """ # Build role hierarchy - role_hierarchy = {role: rank for rank, role in enumerate(reversed(guild.roles))} + actor = ctx.author + role_hierarchy = {role: rank for rank, role in enumerate(reversed(ctx.guild.roles))} + hierarchy_check = role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + + if not hierarchy_check: + log.info( + f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " + f"{target.display_name} ({target.id}), who has a higher top role" + ) + ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") - return role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] + return hierarchy_check def setup(bot): -- cgit v1.2.3 From 5dd0d53e3ee468c13e44880de8fbd2e733f54667 Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:53:49 -0500 Subject: Add missing awaits --- bot/cogs/moderation.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 057b51bbf..1db7df3dd 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -125,7 +125,7 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ - if not self.respect_role_hierarchy(ctx, user, 'kick'): + if not await self.respect_role_hierarchy(ctx, user, 'kick'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -176,7 +176,7 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not self.respect_role_hierarchy(ctx, user, 'ban'): + if not await self.respect_role_hierarchy(ctx, user, 'ban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -337,7 +337,7 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not self.respect_role_hierarchy(ctx, user, 'tempban'): + if not await self.respect_role_hierarchy(ctx, user, 'tempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -435,7 +435,7 @@ class Moderation(Scheduler): :param reason: The reason for the kick. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowkick'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowkick'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -476,7 +476,7 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowban'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -606,7 +606,7 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + if not await self.respect_role_hierarchy(ctx, user, 'shadowtempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -1257,7 +1257,7 @@ class Moderation(Scheduler): f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " f"{target.display_name} ({target.id}), who has a higher top role" ) - ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") + await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") return hierarchy_check -- cgit v1.2.3 From d25ac2c3fc1d83a0022f456ee2189507e249e35a Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 16:59:43 -0500 Subject: Get member object for ban infractions to pass to hierarchy check --- bot/cogs/moderation.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 1db7df3dd..46c10e084 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -176,7 +176,8 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'ban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'ban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -337,7 +338,8 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'tempban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'tempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -476,7 +478,8 @@ class Moderation(Scheduler): :param reason: The reason for the ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'shadowban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'shadowban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return @@ -606,7 +609,8 @@ class Moderation(Scheduler): :param reason: The reason for the temporary ban. """ - if not await self.respect_role_hierarchy(ctx, user, 'shadowtempban'): + member = ctx.guild.get_member(user.id) + if not await self.respect_role_hierarchy(ctx, member, 'shadowtempban'): # Ensure ctx author has a higher top role than the target user # Warning is sent to ctx by the helper method return -- cgit v1.2.3 From 3ed3e2e96ef9e9ab827e331d796f9549bc63455c Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 9 Jan 2019 17:09:57 -0500 Subject: Change logging statement to use author instead of username --- bot/cogs/moderation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index 46c10e084..d9c217b83 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1258,8 +1258,8 @@ class Moderation(Scheduler): if not hierarchy_check: log.info( - f"{actor.display_name} ({actor.id}) attempted to {infraction_type} " - f"{target.display_name} ({target.id}), who has a higher top role" + f"{actor} ({actor.id}) attempted to {infraction_type} " + f"{target} ({target.id}), who has a higher top role" ) await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") -- cgit v1.2.3 From 6d3fbf07efa0768ad32ef9319dac73a3b1e840cc Mon Sep 17 00:00:00 2001 From: sco1 Date: Wed, 16 Jan 2019 20:55:58 -0500 Subject: Simplify role check logic --- bot/cogs/moderation.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/bot/cogs/moderation.py b/bot/cogs/moderation.py index d9c217b83..64c76ae8a 100644 --- a/bot/cogs/moderation.py +++ b/bot/cogs/moderation.py @@ -1243,7 +1243,7 @@ class Moderation(Scheduler): async def respect_role_hierarchy(self, ctx: Context, target: Member, infraction_type: str) -> bool: """ - Check if the highest role of the invoking member is less than or equal to the target member + Check if the highest role of the invoking member is greater than that of the target member If this check fails, a warning is sent to the invoking ctx @@ -1251,19 +1251,18 @@ class Moderation(Scheduler): checks & conversions in a dedicated check decorater """ - # Build role hierarchy actor = ctx.author - role_hierarchy = {role: rank for rank, role in enumerate(reversed(ctx.guild.roles))} - hierarchy_check = role_hierarchy[actor.top_role] <= role_hierarchy[target.top_role] - - if not hierarchy_check: + target_is_lower = target.top_role < actor.top_role + if not target_is_lower: log.info( f"{actor} ({actor.id}) attempted to {infraction_type} " - f"{target} ({target.id}), who has a higher top role" + f"{target} ({target.id}), who has an equal or higher top role" + ) + await ctx.send( + f":x: {actor.mention}, you may not {infraction_type} someone with an equal or higher top role" ) - await ctx.send(f":x: {actor.mention}, you may not {infraction_type} someone with a higher top role") - return hierarchy_check + return target_is_lower def setup(bot): -- cgit v1.2.3