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.
This commit is contained in:
Noi 2022-10-10 14:27:54 -07:00
parent adf9485b7a
commit e265cafd25
4 changed files with 17 additions and 20 deletions

View file

@ -3,7 +3,6 @@ using Discord.Interactions;
using System.Text; using System.Text;
namespace BirthdayBot.ApplicationCommands; namespace BirthdayBot.ApplicationCommands;
[RequireBotModerator] [RequireBotModerator]
[Group("config", HelpCmdConfig)] [Group("config", HelpCmdConfig)]
public class ConfigModule : BotModuleBase { public class ConfigModule : BotModuleBase {
@ -57,7 +56,7 @@ public class ConfigModule : BotModuleBase {
[SlashCommand("set-channel", HelpPfxModOnly + HelpSubCmdChannel + HelpPofxBlankUnset)] [SlashCommand("set-channel", HelpPfxModOnly + HelpSubCmdChannel + HelpPofxBlankUnset)]
public async Task CmdSetChannel([Summary(description: HelpOptChannel)] SocketTextChannel? channel = null) { 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 " + await RespondAsync(":white_check_mark: The announcement channel has been " +
(channel == null ? "unset." : $"set to **{channel.Name}**.")); (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); await RespondAsync(":x: This role cannot be used for this setting.", ephemeral: true);
return; 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); 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($"Server ID: `{guild.Id}` | Bot shard ID: `{Shard.ShardId:00}`");
result.AppendLine($"Number of registered birthdays: `{ guildconf.UserEntries.Count }`"); 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(); result.AppendLine();
bool hasMembers = Common.HasMostMembersDownloaded(guild); bool hasMembers = Common.HasMostMembersDownloaded(guild);
@ -212,7 +211,7 @@ public class ConfigModule : BotModuleBase {
result.Append(DoTestFor("Birthday processing", delegate { result.Append(DoTestFor("Birthday processing", delegate {
if (!hasMembers) return false; if (!hasMembers) return false;
if (guildconf.IsNew) 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; return true;
})); }));
if (!hasMembers) result.AppendLine(" - Previous step failed."); 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 { result.AppendLine(DoTestFor("Birthday role set with `/config role set-birthday-role`", delegate {
if (guildconf.IsNew) return false; if (guildconf.IsNew) return false;
SocketRole? role = guild.GetRole((ulong)(guildconf.RoleId ?? 0)); SocketRole? role = guild.GetRole((ulong)(guildconf.BirthdayRole ?? 0));
return role != null; return role != null;
})); }));
result.AppendLine(DoTestFor("Birthday role can be managed by bot", delegate { result.AppendLine(DoTestFor("Birthday role can be managed by bot", delegate {
if (guildconf.IsNew) return false; 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; if (role == null) return false;
return guild.CurrentUser.GuildPermissions.ManageRoles && role.Position < guild.CurrentUser.Hierarchy; return guild.CurrentUser.GuildPermissions.ManageRoles && role.Position < guild.CurrentUser.Hierarchy;
})); }));
@ -236,7 +235,7 @@ public class ConfigModule : BotModuleBase {
SocketTextChannel? announcech = null; SocketTextChannel? announcech = null;
result.AppendLine(DoTestFor("(Optional) Announcement channel set with `bb.config channel`", delegate { result.AppendLine(DoTestFor("(Optional) Announcement channel set with `bb.config channel`", delegate {
if (guildconf.IsNew) return false; if (guildconf.IsNew) return false;
announcech = guild.GetTextChannel((ulong)(guildconf.ChannelAnnounceId ?? 0)); announcech = guild.GetTextChannel((ulong)(guildconf.AnnouncementChannel ?? 0));
return announcech != null; return announcech != null;
})); }));
string disp = announcech == null ? "announcement channel" : $"<#{announcech.Id}>"; 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 "; const string Response = ":white_check_mark: The server's time zone has been ";
if (zone == null) { if (zone == null) {
await DoDatabaseUpdate(Context, s => s.TimeZone = null); await DoDatabaseUpdate(Context, s => s.GuildTimeZone = null);
await RespondAsync(Response + "unset.").ConfigureAwait(false); await RespondAsync(Response + "unset.").ConfigureAwait(false);
} else { } else {
string parsedZone; string parsedZone;
@ -287,7 +286,7 @@ public class ConfigModule : BotModuleBase {
return; return;
} }
await DoDatabaseUpdate(Context, s => s.TimeZone = parsedZone); await DoDatabaseUpdate(Context, s => s.GuildTimeZone = parsedZone);
await RespondAsync(Response + $"set to **{parsedZone}**.").ConfigureAwait(false); await RespondAsync(Response + $"set to **{parsedZone}**.").ConfigureAwait(false);
} }
} }

View file

@ -2,7 +2,6 @@
using Discord.Interactions; using Discord.Interactions;
namespace BirthdayBot.ApplicationCommands; namespace BirthdayBot.ApplicationCommands;
/// <summary> /// <summary>
/// Precondition requiring the executing user be recognized as a bot moderator.<br/> /// Precondition requiring the executing user be recognized as a bot moderator.<br/>
/// A bot moderator has either the Manage Server permission or is a member of the designated bot moderator role. /// 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(); using var db = new BotDatabaseContext();
var checkRole = (ulong?)db.GuildConfigurations var checkRole = (ulong?)db.GuildConfigurations
.Where(g => g.GuildId == (long)((SocketGuild)context.Guild).Id) .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)) if (checkRole.HasValue && user.Roles.Any(r => r.Id == checkRole.Value))
return Task.FromResult(PreconditionResult.FromSuccess()); return Task.FromResult(PreconditionResult.FromSuccess());

View file

@ -46,13 +46,13 @@ class BirthdayRoleUpdate : BackgroundService {
try { try {
// Verify that role settings and permissions are usable // 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 if (role == null
|| !guild.CurrentUser.GuildPermissions.ManageRoles || !guild.CurrentUser.GuildPermissions.ManageRoles
|| role.Position >= guild.CurrentUser.Hierarchy) continue; || role.Position >= guild.CurrentUser.Hierarchy) continue;
if (role.IsEveryone || role.IsManaged) { if (role.IsEveryone || role.IsManaged) {
// Invalid role was configured. Clear the setting and quit. // Invalid role was configured. Clear the setting and quit.
settings.RoleId = null; settings.BirthdayRole = null;
db.Update(settings); db.Update(settings);
await db.SaveChangesAsync(CancellationToken.None); await db.SaveChangesAsync(CancellationToken.None);
continue; continue;
@ -60,7 +60,7 @@ class BirthdayRoleUpdate : BackgroundService {
// Load up user configs and begin processing birthdays // Load up user configs and begin processing birthdays
await db.Entry(settings).Collection(t => t.UserEntries).LoadAsync(CancellationToken.None); 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 // Add or remove roles as appropriate
var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays); var announcementList = await UpdateGuildBirthdayRoles(guild, role, birthdays);
@ -144,7 +144,7 @@ class BirthdayRoleUpdate : BackgroundService {
/// Attempts to send an announcement message. /// Attempts to send an announcement message.
/// </summary> /// </summary>
internal static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketGuild g, IEnumerable<SocketGuildUser> names) { internal static async Task AnnounceBirthdaysAsync(GuildConfig settings, SocketGuild g, IEnumerable<SocketGuildUser> names) {
var c = g.GetTextChannel((ulong)(settings.ChannelAnnounceId ?? 0)); var c = g.GetTextChannel((ulong)(settings.AnnouncementChannel ?? 0));
if (c == null) return; if (c == null) return;
if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return; if (!c.Guild.CurrentUser.GetPermissions(c).SendMessages) return;

View file

@ -2,7 +2,6 @@
using System.ComponentModel.DataAnnotations.Schema; using System.ComponentModel.DataAnnotations.Schema;
namespace BirthdayBot.Data; namespace BirthdayBot.Data;
[Table("settings")] [Table("settings")]
public class GuildConfig { public class GuildConfig {
public GuildConfig() { public GuildConfig() {
@ -14,11 +13,11 @@ public class GuildConfig {
[Column("guild_id")] [Column("guild_id")]
public long GuildId { get; set; } public long GuildId { get; set; }
[Column("role_id")] [Column("role_id")]
public long? RoleId { get; set; } public long? BirthdayRole { get; set; }
[Column("channel_announce_id")] [Column("channel_announce_id")]
public long? ChannelAnnounceId { get; set; } public long? AnnouncementChannel { get; set; }
[Column("time_zone")] [Column("time_zone")]
public string? TimeZone { get; set; } public string? GuildTimeZone { get; set; }
[Column("moderated")] [Column("moderated")]
public bool Moderated { get; set; } public bool Moderated { get; set; }
[Column("moderator_role")] [Column("moderator_role")]