From e265cafd25d4ed6b4b8aefbd096c625186354295 Mon Sep 17 00:00:00 2001 From: Noi Date: Mon, 10 Oct 2022 14:27:54 -0700 Subject: [PATCH] Fix incorrect value checked for determining moderator The public instance will be updated with this fix immediately. This fixes cases in which those with moderator roles are unable to use mod-only commands as intended. It also fixes a dangerous bug in which users with the birthday role assigned to them have unrestricted access to moderator commands. --- ApplicationCommands/ConfigModule.cs | 19 +++++++++---------- .../Preconditions/RequireBotModerator.cs | 3 +-- BackgroundServices/BirthdayRoleUpdate.cs | 8 ++++---- Data/GuildConfig.cs | 7 +++---- 4 files changed, 17 insertions(+), 20 deletions(-) diff --git a/ApplicationCommands/ConfigModule.cs b/ApplicationCommands/ConfigModule.cs index d80a9b4..d9036fa 100644 --- a/ApplicationCommands/ConfigModule.cs +++ b/ApplicationCommands/ConfigModule.cs @@ -3,7 +3,6 @@ using Discord.Interactions; using System.Text; namespace BirthdayBot.ApplicationCommands; - [RequireBotModerator] [Group("config", HelpCmdConfig)] public class ConfigModule : BotModuleBase { @@ -57,7 +56,7 @@ public class ConfigModule : BotModuleBase { [SlashCommand("set-channel", HelpPfxModOnly + HelpSubCmdChannel + HelpPofxBlankUnset)] public async Task CmdSetChannel([Summary(description: HelpOptChannel)] SocketTextChannel? channel = null) { - await DoDatabaseUpdate(Context, s => s.ChannelAnnounceId = (long?)channel?.Id); + await DoDatabaseUpdate(Context, s => s.AnnouncementChannel = (long?)channel?.Id); await RespondAsync(":white_check_mark: The announcement channel has been " + (channel == null ? "unset." : $"set to **{channel.Name}**.")); } @@ -127,7 +126,7 @@ public class ConfigModule : BotModuleBase { await RespondAsync(":x: This role cannot be used for this setting.", ephemeral: true); return; } - await DoDatabaseUpdate(Context, s => s.RoleId = (long)role.Id); + await DoDatabaseUpdate(Context, s => s.BirthdayRole = (long)role.Id); await RespondAsync($":white_check_mark: The birthday role has been set to **{role.Name}**.").ConfigureAwait(false); } @@ -202,7 +201,7 @@ public class ConfigModule : BotModuleBase { result.AppendLine($"Server ID: `{guild.Id}` | Bot shard ID: `{Shard.ShardId:00}`"); result.AppendLine($"Number of registered birthdays: `{ guildconf.UserEntries.Count }`"); - result.AppendLine($"Server time zone: `{ (guildconf.TimeZone ?? "Not set - using UTC") }`"); + result.AppendLine($"Server time zone: `{ (guildconf.GuildTimeZone ?? "Not set - using UTC") }`"); result.AppendLine(); bool hasMembers = Common.HasMostMembersDownloaded(guild); @@ -212,7 +211,7 @@ public class ConfigModule : BotModuleBase { result.Append(DoTestFor("Birthday processing", delegate { if (!hasMembers) return false; if (guildconf.IsNew) return false; - bdayCount = BackgroundServices.BirthdayRoleUpdate.GetGuildCurrentBirthdays(guildconf.UserEntries, guildconf.TimeZone).Count; + bdayCount = BackgroundServices.BirthdayRoleUpdate.GetGuildCurrentBirthdays(guildconf.UserEntries, guildconf.GuildTimeZone).Count; return true; })); if (!hasMembers) result.AppendLine(" - Previous step failed."); @@ -222,12 +221,12 @@ public class ConfigModule : BotModuleBase { result.AppendLine(DoTestFor("Birthday role set with `/config role set-birthday-role`", delegate { if (guildconf.IsNew) return false; - SocketRole? role = guild.GetRole((ulong)(guildconf.RoleId ?? 0)); + SocketRole? role = guild.GetRole((ulong)(guildconf.BirthdayRole ?? 0)); return role != null; })); result.AppendLine(DoTestFor("Birthday role can be managed by bot", delegate { if (guildconf.IsNew) return false; - SocketRole? role = guild.GetRole((ulong)(guildconf.RoleId ?? 0)); + SocketRole? role = guild.GetRole((ulong)(guildconf.BirthdayRole ?? 0)); if (role == null) return false; return guild.CurrentUser.GuildPermissions.ManageRoles && role.Position < guild.CurrentUser.Hierarchy; })); @@ -236,7 +235,7 @@ public class ConfigModule : BotModuleBase { SocketTextChannel? announcech = null; result.AppendLine(DoTestFor("(Optional) Announcement channel set with `bb.config channel`", delegate { if (guildconf.IsNew) return false; - announcech = guild.GetTextChannel((ulong)(guildconf.ChannelAnnounceId ?? 0)); + announcech = guild.GetTextChannel((ulong)(guildconf.AnnouncementChannel ?? 0)); return announcech != null; })); string disp = announcech == null ? "announcement channel" : $"<#{announcech.Id}>"; @@ -276,7 +275,7 @@ public class ConfigModule : BotModuleBase { const string Response = ":white_check_mark: The server's time zone has been "; if (zone == null) { - await DoDatabaseUpdate(Context, s => s.TimeZone = null); + await DoDatabaseUpdate(Context, s => s.GuildTimeZone = null); await RespondAsync(Response + "unset.").ConfigureAwait(false); } else { string parsedZone; @@ -287,7 +286,7 @@ public class ConfigModule : BotModuleBase { return; } - await DoDatabaseUpdate(Context, s => s.TimeZone = parsedZone); + await DoDatabaseUpdate(Context, s => s.GuildTimeZone = parsedZone); await RespondAsync(Response + $"set to **{parsedZone}**.").ConfigureAwait(false); } } diff --git a/ApplicationCommands/Preconditions/RequireBotModerator.cs b/ApplicationCommands/Preconditions/RequireBotModerator.cs index e458ed9..f62aa07 100644 --- a/ApplicationCommands/Preconditions/RequireBotModerator.cs +++ b/ApplicationCommands/Preconditions/RequireBotModerator.cs @@ -2,7 +2,6 @@ using Discord.Interactions; namespace BirthdayBot.ApplicationCommands; - /// /// 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. @@ -24,7 +23,7 @@ class RequireBotModeratorAttribute : PreconditionAttribute { using var db = new BotDatabaseContext(); var checkRole = (ulong?)db.GuildConfigurations .Where(g => g.GuildId == (long)((SocketGuild)context.Guild).Id) - .Select(g => g.RoleId).FirstOrDefault(); + .Select(g => g.ModeratorRole).FirstOrDefault(); if (checkRole.HasValue && user.Roles.Any(r => r.Id == checkRole.Value)) return Task.FromResult(PreconditionResult.FromSuccess()); diff --git a/BackgroundServices/BirthdayRoleUpdate.cs b/BackgroundServices/BirthdayRoleUpdate.cs index e97906f..48d908b 100644 --- a/BackgroundServices/BirthdayRoleUpdate.cs +++ b/BackgroundServices/BirthdayRoleUpdate.cs @@ -46,13 +46,13 @@ class BirthdayRoleUpdate : BackgroundService { try { // Verify that role settings and permissions are usable - SocketRole? role = guild.GetRole((ulong)(settings.RoleId ?? 0)); + SocketRole? role = guild.GetRole((ulong)(settings.BirthdayRole ?? 0)); if (role == null || !guild.CurrentUser.GuildPermissions.ManageRoles || role.Position >= guild.CurrentUser.Hierarchy) continue; if (role.IsEveryone || role.IsManaged) { // Invalid role was configured. Clear the setting and quit. - settings.RoleId = null; + settings.BirthdayRole = null; db.Update(settings); await db.SaveChangesAsync(CancellationToken.None); continue; @@ -60,7 +60,7 @@ class BirthdayRoleUpdate : BackgroundService { // Load up user configs and begin processing birthdays await db.Entry(settings).Collection(t => t.UserEntries).LoadAsync(CancellationToken.None); - var birthdays = GetGuildCurrentBirthdays(settings.UserEntries, settings.TimeZone); + var birthdays = GetGuildCurrentBirthdays(settings.UserEntries, settings.GuildTimeZone); // Add or remove roles as appropriate var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays); @@ -144,7 +144,7 @@ class BirthdayRoleUpdate : BackgroundService { /// Attempts to send an announcement message. /// internal static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketGuild g, IEnumerable names) { - var c = g.GetTextChannel((ulong)(settings.ChannelAnnounceId ?? 0)); + var c = g.GetTextChannel((ulong)(settings.AnnouncementChannel ?? 0)); if (c == null) return; if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return; diff --git a/Data/GuildConfig.cs b/Data/GuildConfig.cs index e2fa2f6..df66398 100644 --- a/Data/GuildConfig.cs +++ b/Data/GuildConfig.cs @@ -2,7 +2,6 @@ using System.ComponentModel.DataAnnotations.Schema; namespace BirthdayBot.Data; - [Table("settings")] public class GuildConfig { public GuildConfig() { @@ -14,11 +13,11 @@ public class GuildConfig { [Column("guild_id")] public long GuildId { get; set; } [Column("role_id")] - public long? RoleId { get; set; } + public long? BirthdayRole { get; set; } [Column("channel_announce_id")] - public long? ChannelAnnounceId { get; set; } + public long? AnnouncementChannel { get; set; } [Column("time_zone")] - public string? TimeZone { get; set; } + public string? GuildTimeZone { get; set; } [Column("moderated")] public bool Moderated { get; set; } [Column("moderator_role")]