From d700cd8ce98742a241cc6625683509b61cf0a64f Mon Sep 17 00:00:00 2001 From: Noi Date: Fri, 11 Mar 2022 21:52:46 -0800 Subject: [PATCH] Implement block check within a precondition With more preconditions in use, command logging has been modified to also be better able to respond to users in the event of an error. As a result, the bot is now able to respond to users and notify them properly if they fail any preconditions. --- ApplicationCommands/BirthdayModule.cs | 2 +- ApplicationCommands/BirthdayOverrideModule.cs | 1 - ApplicationCommands/BotModuleBase.cs | 1 + ApplicationCommands/ConfigModule.cs | 1 - .../Preconditions/EnforceBlocking.cs | 38 +++++++++++++++++++ .../RequireBotModerator.cs} | 20 +++++----- .../Preconditions/RequireGuildContext.cs | 16 ++++++++ BirthdayBot.csproj | 2 +- Data/GuildConfiguration.cs | 13 ++++++- ShardInstance.cs | 36 ++++++++---------- 10 files changed, 93 insertions(+), 37 deletions(-) create mode 100644 ApplicationCommands/Preconditions/EnforceBlocking.cs rename ApplicationCommands/{Preconditions.cs => Preconditions/RequireBotModerator.cs} (52%) create mode 100644 ApplicationCommands/Preconditions/RequireGuildContext.cs diff --git a/ApplicationCommands/BirthdayModule.cs b/ApplicationCommands/BirthdayModule.cs index 8fc08ef..03f0bad 100644 --- a/ApplicationCommands/BirthdayModule.cs +++ b/ApplicationCommands/BirthdayModule.cs @@ -4,7 +4,7 @@ namespace BirthdayBot.ApplicationCommands; -[RequireContext(ContextType.Guild)] +[RequireGuildContext] [Group("birthday", HelpCmdBirthday)] public class BirthdayModule : BotModuleBase { public const string HelpCmdBirthday = "Commands relating to birthdays."; diff --git a/ApplicationCommands/BirthdayOverrideModule.cs b/ApplicationCommands/BirthdayOverrideModule.cs index f42512a..0a04a63 100644 --- a/ApplicationCommands/BirthdayOverrideModule.cs +++ b/ApplicationCommands/BirthdayOverrideModule.cs @@ -3,7 +3,6 @@ namespace BirthdayBot.ApplicationCommands; -[RequireContext(ContextType.Guild)] [RequireBotModerator] [Group("override", HelpCmdOverride)] public class BirthdayOverrideModule : BotModuleBase { diff --git a/ApplicationCommands/BotModuleBase.cs b/ApplicationCommands/BotModuleBase.cs index a5d7eee..758b741 100644 --- a/ApplicationCommands/BotModuleBase.cs +++ b/ApplicationCommands/BotModuleBase.cs @@ -9,6 +9,7 @@ namespace BirthdayBot.ApplicationCommands; /// /// Base class for our interaction module classes. Contains common data for use in implementing classes. /// +[EnforceBlocking] public abstract class BotModuleBase : InteractionModuleBase { protected const string HelpPfxModOnly = "Bot moderators only: "; protected const string ErrGuildOnly = ":x: This command can only be run within a server."; diff --git a/ApplicationCommands/ConfigModule.cs b/ApplicationCommands/ConfigModule.cs index 9a0746f..e3b14cc 100644 --- a/ApplicationCommands/ConfigModule.cs +++ b/ApplicationCommands/ConfigModule.cs @@ -4,7 +4,6 @@ namespace BirthdayBot.ApplicationCommands; -[RequireContext(ContextType.Guild)] [RequireBotModerator] [Group("config", HelpCmdConfig)] public class ConfigModule : BotModuleBase { diff --git a/ApplicationCommands/Preconditions/EnforceBlocking.cs b/ApplicationCommands/Preconditions/EnforceBlocking.cs new file mode 100644 index 0000000..bda3335 --- /dev/null +++ b/ApplicationCommands/Preconditions/EnforceBlocking.cs @@ -0,0 +1,38 @@ +using BirthdayBot.Data; +using Discord.Interactions; + +namespace BirthdayBot.ApplicationCommands; + +/// +/// Only users not on the blocklist or affected by moderator mode may use the command.
+/// This is used in the base class. Manually using it anywhere else is unnecessary. +/// +class EnforceBlockingAttribute : PreconditionAttribute { + public const string FailModerated = "Guild has moderator mode enabled."; + public const string FailBlocked = "User is in the guild's block list."; + public const string ReplyModerated = ":x: This bot is in moderated mode, preventing you from using any bot commands in this server."; + public const string ReplyBlocked = ":x: You have been blocked from using bot commands in this server."; + + public override async Task CheckRequirementsAsync( + IInteractionContext context, ICommandInfo commandInfo, IServiceProvider services) { + // Not in guild context, unaffected by blocking + if (context.Guild is not SocketGuild guild) return PreconditionResult.FromSuccess(); + + // Manage Guild permission overrides any blocks + var user = (SocketGuildUser)context.User; + if (user.GuildPermissions.ManageGuild) return PreconditionResult.FromSuccess(); + + var gconf = await guild.GetConfigAsync().ConfigureAwait(false); + + // Bot moderators override any blocks + if (gconf.ModeratorRole.HasValue && user.Roles.Any(r => r.Id == gconf.ModeratorRole.Value)) return PreconditionResult.FromSuccess(); + + // Moderated mode check + if (gconf.IsModerated) return PreconditionResult.FromError(FailModerated); + + // Block list check + if (await gconf.IsUserInBlocklistAsync(user.Id)) return PreconditionResult.FromError(FailBlocked); + + return PreconditionResult.FromSuccess(); + } +} \ No newline at end of file diff --git a/ApplicationCommands/Preconditions.cs b/ApplicationCommands/Preconditions/RequireBotModerator.cs similarity index 52% rename from ApplicationCommands/Preconditions.cs rename to ApplicationCommands/Preconditions/RequireBotModerator.cs index 186dff6..a32e6cb 100644 --- a/ApplicationCommands/Preconditions.cs +++ b/ApplicationCommands/Preconditions/RequireBotModerator.cs @@ -3,29 +3,27 @@ namespace BirthdayBot.ApplicationCommands; -// Contains preconditions used by our interaction modules. - /// -/// Precondition requiring the executing user be considered a bot moderator. -/// That is, they must either have the Manage Server permission or be a member of the designated bot moderator role. +/// Precondition requiring the executing user be recognized as a bot moderator.
+/// A bot moderator has either the Manage Server permission or is a member of the designated bot moderator role. /// class RequireBotModeratorAttribute : PreconditionAttribute { - public const string FailMsg = "User did not pass the mod check."; + public const string Error = "User did not pass the mod check."; public const string Reply = ":x: You must be a moderator to use this command."; - public override string ErrorMessage => FailMsg; + public override string ErrorMessage => Error; public override async Task CheckRequirementsAsync( IInteractionContext context, ICommandInfo commandInfo, IServiceProvider services) { - if (context.User is not SocketGuildUser user) { - return PreconditionResult.FromError("Failed due to non-guild context."); - } + // A bot moderator can only exist in a guild context, so we must do this check. + // This check causes this precondition to become a functional equivalent to RequireGuildContextAttribute... + if (context.User is not SocketGuildUser user) return PreconditionResult.FromError(RequireGuildContextAttribute.Error); if (user.GuildPermissions.ManageGuild) return PreconditionResult.FromSuccess(); var gconf = await ((SocketGuild)context.Guild).GetConfigAsync().ConfigureAwait(false); if (gconf.ModeratorRole.HasValue && user.Roles.Any(r => r.Id == gconf.ModeratorRole.Value)) return PreconditionResult.FromSuccess(); - return PreconditionResult.FromError(FailMsg); + return PreconditionResult.FromError(Error); } -} +} \ No newline at end of file diff --git a/ApplicationCommands/Preconditions/RequireGuildContext.cs b/ApplicationCommands/Preconditions/RequireGuildContext.cs new file mode 100644 index 0000000..dccb7da --- /dev/null +++ b/ApplicationCommands/Preconditions/RequireGuildContext.cs @@ -0,0 +1,16 @@ +using Discord.Interactions; + +namespace BirthdayBot.ApplicationCommands; + +/// +/// Implements the included precondition from Discord.Net, requiring a guild context while using our custom error message.

+/// Combining this with is redundant. If possible, only use the latter instead. +/// +class RequireGuildContextAttribute : RequireContextAttribute { + public const string Error = "Command not received within a guild context."; + public const string Reply = ":x: This command is only available within a server."; + + public override string ErrorMessage => Error; + + public RequireGuildContextAttribute() : base(ContextType.Guild) { } +} \ No newline at end of file diff --git a/BirthdayBot.csproj b/BirthdayBot.csproj index e875647..ee4ceeb 100644 --- a/BirthdayBot.csproj +++ b/BirthdayBot.csproj @@ -5,7 +5,7 @@ net6.0 enable enable - 3.3.0 + 3.3.1 NoiTheCat diff --git a/Data/GuildConfiguration.cs b/Data/GuildConfiguration.cs index 65f0c99..b29cce4 100644 --- a/Data/GuildConfiguration.cs +++ b/Data/GuildConfiguration.cs @@ -71,12 +71,19 @@ class GuildConfiguration { } /// - /// Checks if the given user exists in the block list. - /// If the server is in moderated mode, this always returns true. + /// Checks if the specified user is blocked by current guild policy (block list or moderated mode). /// + [Obsolete("Block lists should be reimplemented in a more resource-efficient manner later.", false)] public async Task IsUserBlockedAsync(ulong userId) { if (IsModerated) return true; + return await IsUserInBlocklistAsync(userId).ConfigureAwait(false); + } + /// + /// Checks if the given user exists in the block list. + /// + [Obsolete("Block lists should be reimplemented in a more resource-efficient manner later.", false)] + public async Task IsUserInBlocklistAsync(ulong userId) { using var db = await Database.OpenConnectionAsync().ConfigureAwait(false); using var c = db.CreateCommand(); c.CommandText = $"select * from {BackingTableBans} " @@ -92,6 +99,7 @@ class GuildConfiguration { /// /// Adds the specified user to the block list corresponding to this guild. /// + [Obsolete("Block lists will be reimplemented in a more practical manner later.", false)] public async Task BlockUserAsync(ulong userId) { using var db = await Database.OpenConnectionAsync().ConfigureAwait(false); using var c = db.CreateCommand(); @@ -109,6 +117,7 @@ class GuildConfiguration { /// Removes the specified user from the block list corresponding to this guild. /// /// True if a user has been removed, false if the requested user was not in this list. + [Obsolete("Block lists will be reimplemented in a more practical manner later.", false)] public async Task UnblockUserAsync(ulong userId) { using var db = await Database.OpenConnectionAsync().ConfigureAwait(false); using var c = db.CreateCommand(); diff --git a/ShardInstance.cs b/ShardInstance.cs index de3388a..7dac9d2 100644 --- a/ShardInstance.cs +++ b/ShardInstance.cs @@ -170,26 +170,15 @@ public sealed class ShardInstance : IDisposable { private async Task DiscordClient_InteractionCreated(SocketInteraction arg) { var context = new SocketInteractionContext(DiscordClient, arg); - // Blocklist/moderated check - // TODO convert to precondition - var gconf = await GuildConfiguration.LoadAsync(context.Guild.Id, false); - if (context.Channel is SocketGuildChannel) { // Check only if in a guild context - if (!gconf!.IsBotModerator((SocketGuildUser)arg.User)) { // Moderators exempted from this check - if (await gconf.IsUserBlockedAsync(arg.User.Id)) { - Log("Interaction", $"Interaction blocked per guild policy for {context.Guild}!{context.User}"); - await arg.RespondAsync(BotModuleBase.AccessDeniedError, ephemeral: true).ConfigureAwait(false); - return; - } - } - } - try { await _interactionService.ExecuteCommandAsync(context, _services).ConfigureAwait(false); } catch (Exception e) { Log(nameof(DiscordClient_InteractionCreated), $"Unhandled exception. {e}"); - // TODO when implementing proper error logging, see here - if (arg.Type == InteractionType.ApplicationCommand) - if (arg.HasResponded) await arg.ModifyOriginalResponseAsync(prop => prop.Content = ":warning: An unknown error occured."); + // TODO when implementing proper application error logging, see here + if (arg.Type == InteractionType.ApplicationCommand) { + if (arg.HasResponded) await arg.ModifyOriginalResponseAsync(prop => prop.Content = InternalError); + else await arg.RespondAsync(InternalError); + } } } @@ -205,17 +194,24 @@ public sealed class ShardInstance : IDisposable { if (result.Error != null) { // Additional log information with error detail - logresult += Enum.GetName(typeof(InteractionCommandError), result.Error) + ": " + result.ErrorReason; + logresult += " " + Enum.GetName(typeof(InteractionCommandError), result.Error) + ": " + result.ErrorReason; // Specific responses to errors, if necessary - if (result.Error == InteractionCommandError.UnmetPrecondition && result.ErrorReason == RequireBotModeratorAttribute.FailMsg) { - await context.Interaction.RespondAsync(RequireBotModeratorAttribute.Reply, ephemeral: true).ConfigureAwait(false); + if (result.Error == InteractionCommandError.UnmetPrecondition) { + string errReply = result.ErrorReason switch { + RequireBotModeratorAttribute.Error => RequireBotModeratorAttribute.Reply, + EnforceBlockingAttribute.FailBlocked => EnforceBlockingAttribute.ReplyBlocked, + EnforceBlockingAttribute.FailModerated => EnforceBlockingAttribute.ReplyModerated, + RequireGuildContextAttribute.Error => RequireGuildContextAttribute.Reply, + _ => result.ErrorReason + }; + await context.Interaction.RespondAsync(errReply, ephemeral: true).ConfigureAwait(false); } else { // Generic error response + // TODO when implementing proper application error logging, see here var ia = context.Interaction; if (ia.HasResponded) await ia.ModifyOriginalResponseAsync(p => p.Content = InternalError).ConfigureAwait(false); else await ia.RespondAsync(InternalError).ConfigureAwait(false); - // TODO when implementing proper error logging, see here } }