From 92eba7e4daf4bc9abf8c8e6da976228be7d9360e Mon Sep 17 00:00:00 2001 From: Yusuf Arfan Ismail Date: Thu, 4 Nov 2021 09:30:29 +0100 Subject: [PATCH 01/21] Added moderation commands (Ban, Kick, Unban) --- .../togetherjava/tjbot/commands/Commands.java | 3 + .../tjbot/commands/moderation/BanCommand.java | 143 ++++++++++++++++++ .../commands/moderation/KickCommand.java | 116 ++++++++++++++ .../commands/moderation/ModerationUtils.java | 42 +++++ .../commands/moderation/UnbanCommand.java | 100 ++++++++++++ .../commands/moderation/package-info.java | 5 + 6 files changed, 409 insertions(+) create mode 100644 application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java create mode 100644 application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java create mode 100644 application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java create mode 100644 application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java create mode 100644 application/src/main/java/org/togetherjava/tjbot/commands/moderation/package-info.java diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java index 92a777b65b..16fe3b702f 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java @@ -49,6 +49,9 @@ public enum Commands { commands.add(new TagManageCommand(tagSystem)); commands.add(new TagsCommand(tagSystem)); commands.add(new VcActivityCommand()); + commands.add(new KickCommand()); + commands.add(new BanCommand()); + commands.add(new UnbanCommand()); return commands; } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java new file mode 100644 index 0000000000..e3776ea3c6 --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -0,0 +1,143 @@ +package org.togetherjava.tjbot.commands.moderation; + +import net.dv8tion.jda.api.Permission; +import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.Member; +import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.interactions.commands.OptionMapping; +import net.dv8tion.jda.api.interactions.commands.OptionType; +import net.dv8tion.jda.api.interactions.commands.build.OptionData; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.togetherjava.tjbot.commands.SlashCommandAdapter; +import org.togetherjava.tjbot.commands.SlashCommandVisibility; + +import java.util.Objects; + +/** + * This command can ban users and optionally remove their messages from the past days. Banning can + * also be paired with a ban reason. The command will also try to DM the user to inform them about + * the action and the reason. + *

+ * The command fails if the user triggering it is lacking permissions to either ban other users or + * to ban the specific given user (for example a moderator attempting to ban an admin). + */ +public final class BanCommand extends SlashCommandAdapter { + private static final String USER_OPTION = "user"; + private static final String DELETE_HISTORY_OPTION = "delete-history"; + private static final String REASON_OPTION = "reason"; + private static final Logger logger = LoggerFactory.getLogger(BanCommand.class); + + /** + * Constructs an instance. + */ + public BanCommand() { + super("ban", "Bans the given user from the server", SlashCommandVisibility.GUILD); + + getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to ban", true) + .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be banned", true) + .addOptions(new OptionData(OptionType.INTEGER, DELETE_HISTORY_OPTION, + "the amount of days of the message history to delete, none means no messages are deleted.", + true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); + } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + OptionMapping userOption = + Objects.requireNonNull(event.getOption(USER_OPTION), "The target is null"); + Member target = userOption.getAsMember(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + Guild guild = Objects.requireNonNull(event.getGuild()); + Member bot = guild.getSelfMember(); + + // Member doesn't exist if attempting to ban a user who is not part of the guild. + if (target != null && !handleCanInteractWithTarget(target, bot, author, event)) { + return; + } + + if (!handleHasPermissions(author, bot, event, guild)) { + return; + } + + int deleteHistoryDays = Math + .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); + + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + banUser(userOption.getAsUser(), author, reason, deleteHistoryDays, guild, event); + } + + private static void banUser(@NotNull User target, @NotNull Member author, + @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, + @NotNull SlashCommandEvent event) { + event.getJDA() + .openPrivateChannelById(target.getIdLong()) + .flatMap(channel -> channel.sendMessage( + """ + Hey there, sorry to tell you but unfortunately you have been banned from the server %s. + If you think this was a mistake, please contact a moderator or admin of the server. + The reason for the ban is: %s + """ + .formatted(guild.getName(), reason))) + .mapToResult() + .flatMap(result -> guild.ban(target, deleteHistoryDays, reason)) + .flatMap(v -> event.reply(target.getAsTag() + " was banned by " + + author.getUser().getAsTag() + " for: " + reason)) + .queue(); + + logger.info( + " '{} ({})' banned the user '{} ({})' and deleted their message history of the last '{}' days. Reason being'{}'", + author.getUser().getAsTag(), author.getIdLong(), target.getAsTag(), + target.getIdLong(), deleteHistoryDays, reason); + } + + private static boolean handleCanInteractWithTarget(@NotNull Member target, @NotNull Member bot, + @NotNull Member author, @NotNull SlashCommandEvent event) { + String targetTag = target.getUser().getAsTag(); + if (!author.canInteract(target)) { + event.reply("The user " + targetTag + " is too powerful for you to ban.") + .setEphemeral(true) + .queue(); + return false; + } + + if (!bot.canInteract(target)) { + event.reply("The user " + targetTag + " is too powerful for me to ban.") + .setEphemeral(true) + .queue(); + return false; + } + return true; + } + + private static boolean handleHasPermissions(@NotNull Member author, @NotNull Member bot, + @NotNull SlashCommandEvent event, @NotNull Guild guild) { + if (!author.hasPermission(Permission.BAN_MEMBERS)) { + event.reply( + "You can not ban users in this guild since you do not have the BAN_MEMBERS permission.") + .setEphemeral(true) + .queue(); + return false; + } + + if (!bot.hasPermission(Permission.BAN_MEMBERS)) { + event.reply( + "I can not ban users in this guild since I do not have the BAN_MEMBERS permission.") + .setEphemeral(true) + .queue(); + + logger.error("The bot does not have BAN_MEMBERS permission on the server '{}' ", + guild.getName()); + return false; + } + return true; + } +} diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java new file mode 100644 index 0000000000..d887bb8472 --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -0,0 +1,116 @@ +package org.togetherjava.tjbot.commands.moderation; + +import net.dv8tion.jda.api.Permission; +import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.Member; +import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.interactions.commands.OptionMapping; +import net.dv8tion.jda.api.interactions.commands.OptionType; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.togetherjava.tjbot.commands.SlashCommandAdapter; +import org.togetherjava.tjbot.commands.SlashCommandVisibility; +import java.util.Objects; + + +/** + * This command can kicks users. Kicking can also be paired with a kick reason. The command will + * also try to DM the user to inform them about the action and the reason. + *

+ * The command fails if the user triggering it is lacking permissions to either kick other users or + * to kick the specific given user (for example a moderator attempting to kick an admin). + * + */ +public final class KickCommand extends SlashCommandAdapter { + private static final Logger logger = LoggerFactory.getLogger(KickCommand.class); + private static final String USER_OPTION = "user"; + private static final String REASON_OPTION = "reason"; + + /** + * Constructs an instance. + */ + public KickCommand() { + super("kick", "Kicks the given user from the server", SlashCommandVisibility.GUILD); + + getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to kick", true) + .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be kicked", true); + } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + OptionMapping userOption = + Objects.requireNonNull(event.getOption(USER_OPTION), "The target is null"); + Member target = userOption.getAsMember(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + + Guild guild = event.getGuild(); + Member bot = guild.getSelfMember(); + + if (!author.hasPermission(Permission.KICK_MEMBERS)) { + event.reply( + "You can not kick users in this guild since you do not have the KICK_MEMBERS permission.") + .setEphemeral(true) + .queue(); + return; + } + + String userTag = userOption.getAsUser().getAsTag(); + if (!author.canInteract(target)) { + event.reply("The user " + userTag + " is too powerful for you to kick.") + .setEphemeral(true) + .queue(); + return; + } + + if (!bot.hasPermission(Permission.KICK_MEMBERS)) { + event.reply( + "I can not kick users in this guild since I do not have the KICK_MEMBERS permission.") + .setEphemeral(true) + .queue(); + + logger.error("The bot does not have KICK_MEMBERS permission on the server '{}' ", + event.getGuild().getName()); + return; + } + + if (!bot.canInteract(target)) { + event.reply("The user " + userTag + " is too powerful for me to kick.") + .setEphemeral(true) + .queue(); + return; + } + + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + kickUser(target, author, reason, guild, event); + } + + private static void kickUser(@NotNull Member target, @NotNull Member author, + @NotNull String reason, @NotNull Guild guild, @NotNull SlashCommandEvent event) { + event.getJDA() + .openPrivateChannelById(target.getUser().getId()) + .flatMap(channel -> channel.sendMessage( + """ + Hey there, sorry to tell you but unfortunately you have been kicked from the server %s. + If you think this was a mistake, please contact a moderator or admin of the server. + The reason for the kick is: %s + """ + .formatted(guild.getName(), reason))) + .mapToResult() + .flatMap(result -> guild.kick(target, reason).reason(reason)) + .flatMap(v -> event.reply(target.getUser().getAsTag() + " was kicked by " + + author.getUser().getAsTag() + " for: " + reason)) + .queue(); + + logger.info(" '{} ({})' kicked the user '{} ({})' due to reason being '{}'", + author.getUser().getAsTag(), author.getIdLong(), target.getUser().getAsTag(), + target.getUser().getId(), reason); + } +} diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java new file mode 100644 index 0000000000..d35963b9b3 --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -0,0 +1,42 @@ +package org.togetherjava.tjbot.commands.moderation; + +import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import org.jetbrains.annotations.NotNull; + +/** + * Utility class offering helpers revolving around user moderation, such as banning or kicking. + */ +public enum ModerationUtils { + ; + + /** + * As stated in {@link Guild#ban(User, int, String)}
+ * The reason can be only 512 characters. + */ + private static final int REASON_MAX_LENGTH = 512; + + /** + * This boolean checks if the reason that the user has entered violates the max character length + * or not.
+ * If it does the bot will tell the user they have violated the mex character length and will + * terminate the command
+ * If it does bot the bot will be allowed to continue running the command. + * + * @param reason the reason of the action such as banning. + * @param event This is used to be able to use slash command actions such as event.reply(); + * @throws IllegalArgumentException if the reason is over 512 characters. + * + */ + public static boolean handleReason(@NotNull String reason, @NotNull SlashCommandEvent event) { + if (reason.length() <= REASON_MAX_LENGTH) { + return true; + } + + event.reply("The reason can not be over " + REASON_MAX_LENGTH + " characters") + .setEphemeral(true) + .queue(); + return false; + } +} diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java new file mode 100644 index 0000000000..c11e6474c0 --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -0,0 +1,100 @@ +package org.togetherjava.tjbot.commands.moderation; + +import net.dv8tion.jda.api.Permission; +import net.dv8tion.jda.api.entities.Member; +import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.exceptions.ErrorResponseException; +import net.dv8tion.jda.api.interactions.commands.OptionType; +import net.dv8tion.jda.api.requests.ErrorResponse; +import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.togetherjava.tjbot.commands.SlashCommandAdapter; +import org.togetherjava.tjbot.commands.SlashCommandVisibility; + +import java.util.Objects; + +/** + * This command allows you to unban a user. The unban command requires the user to input the user + * who is subject to get unbanned. Unbanning can also be paired with a reason for unbanning the + * user. The command fails if the user is currently not banned already. + */ +public final class UnbanCommand extends SlashCommandAdapter { + private static final Logger logger = LoggerFactory.getLogger(UnbanCommand.class); + private static final String USER_OPTION = "user"; + private static final String REASON_OPTION = "reason"; + + /** + * Constructs an instance. + */ + public UnbanCommand() { + super("unban", "Unbans a given user", SlashCommandVisibility.GUILD); + + getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to unban", true) + .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be unbanned", true); + } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + User target = Objects.requireNonNull(event.getOption(USER_OPTION), "The user is null") + .getAsUser(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + + if (!author.hasPermission(Permission.BAN_MEMBERS)) { + event.reply( + "You can not unban users in this guild since you do not have the BAN_MEMBERS permission.") + .setEphemeral(true) + .queue(); + return; + } + + Member bot = Objects.requireNonNull(event.getGuild(), "The guild is null").getSelfMember(); + if (!bot.hasPermission(Permission.BAN_MEMBERS)) { + event.reply( + "I can not unban users in this guild since I do not have the BAN_MEMBERS permission.") + .setEphemeral(true) + .queue(); + + logger.error("The bot does not have BAN_MEMBERS permission on the server '{}' ", + event.getGuild().getName()); + return; + } + + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + unban(target, reason, author, event); + } + + private static void unban(@NotNull User target, @NotNull String reason, @NotNull Member author, + @NotNull SlashCommandEvent event) { + event.getGuild().unban(target).reason(reason).queue(v -> { + event + .reply("The user " + author.getUser().getAsTag() + " unbanned the user " + + target.getAsTag() + " for: " + reason) + .queue(); + + logger.info(" {} ({}) unbanned the user '{}' for: '{}'", author.getUser().getAsTag(), + author.getIdLong(), target.getAsTag(), reason); + }, throwable -> { + if (throwable instanceof ErrorResponseException errorResponseException + && errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { + + event.reply("The specified user doesn't exist").setEphemeral(true).queue(); + + logger.debug("I could not unban the user '{}' because they do not exist", + target.getAsTag()); + } else { + event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); + + logger.warn("Something went wrong during the process of unbanning the user ", + throwable); + } + }); + } +} diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/package-info.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/package-info.java new file mode 100644 index 0000000000..399fc1fa1f --- /dev/null +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/package-info.java @@ -0,0 +1,5 @@ +/** + * This package offers all the moderation commands from the application such as banning and kicking + * users. + */ +package org.togetherjava.tjbot.commands.moderation; From 54fafc6f6eedc287ca9219a5cee7d6001e92df31 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Wed, 3 Nov 2021 10:15:16 +0100 Subject: [PATCH 02/21] Fixed code flaws --- .../tjbot/commands/moderation/BanCommand.java | 122 +++++++++--------- .../commands/moderation/KickCommand.java | 109 ++++++++++------ .../commands/moderation/ModerationUtils.java | 27 ++-- .../commands/moderation/UnbanCommand.java | 90 +++++++------ 4 files changed, 190 insertions(+), 158 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index e3776ea3c6..5e411736a8 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -2,9 +2,11 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionMapping; import net.dv8tion.jda.api.interactions.commands.OptionType; import net.dv8tion.jda.api.interactions.commands.build.OptionData; @@ -25,7 +27,7 @@ * to ban the specific given user (for example a moderator attempting to ban an admin). */ public final class BanCommand extends SlashCommandAdapter { - private static final String USER_OPTION = "user"; + private static final String TARGET_OPTION = "user"; private static final String DELETE_HISTORY_OPTION = "delete-history"; private static final String REASON_OPTION = "reason"; private static final Logger logger = LoggerFactory.getLogger(BanCommand.class); @@ -36,71 +38,15 @@ public final class BanCommand extends SlashCommandAdapter { public BanCommand() { super("ban", "Bans the given user from the server", SlashCommandVisibility.GUILD); - getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to ban", true) + getData().addOption(OptionType.USER, TARGET_OPTION, "The user who you want to ban", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be banned", true) .addOptions(new OptionData(OptionType.INTEGER, DELETE_HISTORY_OPTION, "the amount of days of the message history to delete, none means no messages are deleted.", true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); } - @Override - public void onSlashCommand(@NotNull SlashCommandEvent event) { - OptionMapping userOption = - Objects.requireNonNull(event.getOption(USER_OPTION), "The target is null"); - Member target = userOption.getAsMember(); - Member author = Objects.requireNonNull(event.getMember(), "The author is null"); - - String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") - .getAsString(); - - Guild guild = Objects.requireNonNull(event.getGuild()); - Member bot = guild.getSelfMember(); - - // Member doesn't exist if attempting to ban a user who is not part of the guild. - if (target != null && !handleCanInteractWithTarget(target, bot, author, event)) { - return; - } - - if (!handleHasPermissions(author, bot, event, guild)) { - return; - } - - int deleteHistoryDays = Math - .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); - - if (!ModerationUtils.handleReason(reason, event)) { - return; - } - - banUser(userOption.getAsUser(), author, reason, deleteHistoryDays, guild, event); - } - - private static void banUser(@NotNull User target, @NotNull Member author, - @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, - @NotNull SlashCommandEvent event) { - event.getJDA() - .openPrivateChannelById(target.getIdLong()) - .flatMap(channel -> channel.sendMessage( - """ - Hey there, sorry to tell you but unfortunately you have been banned from the server %s. - If you think this was a mistake, please contact a moderator or admin of the server. - The reason for the ban is: %s - """ - .formatted(guild.getName(), reason))) - .mapToResult() - .flatMap(result -> guild.ban(target, deleteHistoryDays, reason)) - .flatMap(v -> event.reply(target.getAsTag() + " was banned by " - + author.getUser().getAsTag() + " for: " + reason)) - .queue(); - - logger.info( - " '{} ({})' banned the user '{} ({})' and deleted their message history of the last '{}' days. Reason being'{}'", - author.getUser().getAsTag(), author.getIdLong(), target.getAsTag(), - target.getIdLong(), deleteHistoryDays, reason); - } - - private static boolean handleCanInteractWithTarget(@NotNull Member target, @NotNull Member bot, - @NotNull Member author, @NotNull SlashCommandEvent event) { + private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, + @NotNull Member target, @NotNull Interaction event) { String targetTag = target.getUser().getAsTag(); if (!author.canInteract(target)) { event.reply("The user " + targetTag + " is too powerful for you to ban.") @@ -118,8 +64,8 @@ private static boolean handleCanInteractWithTarget(@NotNull Member target, @NotN return true; } - private static boolean handleHasPermissions(@NotNull Member author, @NotNull Member bot, - @NotNull SlashCommandEvent event, @NotNull Guild guild) { + private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, + @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { if (!author.hasPermission(Permission.BAN_MEMBERS)) { event.reply( "You can not ban users in this guild since you do not have the BAN_MEMBERS permission.") @@ -140,4 +86,56 @@ private static boolean handleHasPermissions(@NotNull Member author, @NotNull Mem } return true; } + + private static void banUser(@NotNull User target, @NotNull Member author, + @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, + @NotNull SlashCommandEvent event) { + event.getJDA() + .openPrivateChannelById(target.getIdLong()) + .flatMap(channel -> channel.sendMessage( + """ + Hey there, sorry to tell you but unfortunately you have been banned from the server %s. + If you think this was a mistake, please contact a moderator or admin of the server. + The reason for the ban is: %s + """ + .formatted(guild.getName(), reason))) + .mapToResult() + .flatMap(result -> guild.ban(target, deleteHistoryDays, reason)) + .flatMap(result -> event.reply("'%s' was banned by '%s' for: %s" + .formatted(target.getAsTag(), author.getUser().getAsTag(), reason))) + .queue(); + + logger.info( + "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'", + author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), + guild.getName(), deleteHistoryDays, reason); + } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + OptionMapping targetOption = + Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null"); + Member target = targetOption.getAsMember(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + Guild guild = Objects.requireNonNull(event.getGuild()); + Member bot = guild.getSelfMember(); + + // Member doesn't exist if attempting to ban a user who is not part of the guild. + if (target != null && !handleCanInteractWithTarget(bot, author, target, event)) { + return; + } + if (!handleHasPermissions(bot, author, guild, event)) { + return; + } + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + int deleteHistoryDays = Math + .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); + banUser(targetOption.getAsUser(), author, reason, deleteHistoryDays, guild, event); + } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index d887bb8472..466195aaf4 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -2,9 +2,10 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; -import net.dv8tion.jda.api.interactions.commands.OptionMapping; +import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionType; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; @@ -24,7 +25,7 @@ */ public final class KickCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(KickCommand.class); - private static final String USER_OPTION = "user"; + private static final String TARGET_OPTION = "user"; private static final String REASON_OPTION = "reason"; /** @@ -33,63 +34,56 @@ public final class KickCommand extends SlashCommandAdapter { public KickCommand() { super("kick", "Kicks the given user from the server", SlashCommandVisibility.GUILD); - getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to kick", true) + getData().addOption(OptionType.USER, TARGET_OPTION, "The user who you want to kick", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be kicked", true); } - @Override - public void onSlashCommand(@NotNull SlashCommandEvent event) { - OptionMapping userOption = - Objects.requireNonNull(event.getOption(USER_OPTION), "The target is null"); - Member target = userOption.getAsMember(); - Member author = Objects.requireNonNull(event.getMember(), "The author is null"); - - String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") - .getAsString(); - - - Guild guild = event.getGuild(); - Member bot = guild.getSelfMember(); + private static void handleAbsentTarget(@NotNull Interaction event) { + event.reply("I can not kick the given user since they are not part of the guild anymore.") + .setEphemeral(true) + .queue(); + } - if (!author.hasPermission(Permission.KICK_MEMBERS)) { - event.reply( - "You can not kick users in this guild since you do not have the KICK_MEMBERS permission.") + private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, + @NotNull Member target, @NotNull Interaction event) { + String targetTag = target.getUser().getAsTag(); + if (!author.canInteract(target)) { + event.reply("The user " + targetTag + " is too powerful for you to kick.") .setEphemeral(true) .queue(); - return; + return false; } - String userTag = userOption.getAsUser().getAsTag(); - if (!author.canInteract(target)) { - event.reply("The user " + userTag + " is too powerful for you to kick.") + if (!bot.canInteract(target)) { + event.reply("The user " + targetTag + " is too powerful for me to kick.") .setEphemeral(true) .queue(); - return; + return false; } + return true; + } - if (!bot.hasPermission(Permission.KICK_MEMBERS)) { + private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, + @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { + if (!author.hasPermission(Permission.KICK_MEMBERS)) { event.reply( - "I can not kick users in this guild since I do not have the KICK_MEMBERS permission.") + "You can not kick users in this guild since you do not have the KICK_MEMBERS permission.") .setEphemeral(true) .queue(); - - logger.error("The bot does not have KICK_MEMBERS permission on the server '{}' ", - event.getGuild().getName()); - return; + return false; } - if (!bot.canInteract(target)) { - event.reply("The user " + userTag + " is too powerful for me to kick.") + if (!bot.hasPermission(Permission.KICK_MEMBERS)) { + event.reply( + "I can not kick users in this guild since I do not have the KICK_MEMBERS permission.") .setEphemeral(true) .queue(); - return; - } - if (!ModerationUtils.handleReason(reason, event)) { - return; + logger.error("The bot does not have KICK_MEMBERS permission on the server '{}' ", + guild.getName()); + return false; } - - kickUser(target, author, reason, guild, event); + return true; } private static void kickUser(@NotNull Member target, @NotNull Member author, @@ -105,12 +99,41 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, .formatted(guild.getName(), reason))) .mapToResult() .flatMap(result -> guild.kick(target, reason).reason(reason)) - .flatMap(v -> event.reply(target.getUser().getAsTag() + " was kicked by " - + author.getUser().getAsTag() + " for: " + reason)) + .flatMap(result -> event.reply("'%s' was kicked by '%s' for: %s" + .formatted(target.getUser().getAsTag(), author.getUser().getAsTag(), reason))) .queue(); - logger.info(" '{} ({})' kicked the user '{} ({})' due to reason being '{}'", - author.getUser().getAsTag(), author.getIdLong(), target.getUser().getAsTag(), - target.getUser().getId(), reason); + logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'", + author.getUser().getAsTag(), author.getId(), target.getUser().getAsTag(), + target.getUser().getId(), guild.getName(), reason); + } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + Member target = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null") + .getAsMember(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + Guild guild = Objects.requireNonNull(event.getGuild()); + Member bot = guild.getSelfMember(); + + // Member doesn't exist if attempting to kick a user who is not part of the guild anymore. + if (target == null) { + handleAbsentTarget(event); + return; + } + if (!handleCanInteractWithTarget(bot, author, target, event)) { + return; + } + if (!handleHasPermissions(bot, author, guild, event)) { + return; + } + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + kickUser(target, author, reason, guild, event); } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index d35963b9b3..cd9bfec2ae 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -2,7 +2,7 @@ import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.User; -import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.interactions.Interaction; import org.jetbrains.annotations.NotNull; /** @@ -12,29 +12,28 @@ public enum ModerationUtils { ; /** - * As stated in {@link Guild#ban(User, int, String)}
- * The reason can be only 512 characters. + * The maximal character limit for the reason of an auditable action, see for example + * {@link Guild#ban(User, int, String)}. */ private static final int REASON_MAX_LENGTH = 512; /** - * This boolean checks if the reason that the user has entered violates the max character length - * or not.
- * If it does the bot will tell the user they have violated the mex character length and will - * terminate the command
- * If it does bot the bot will be allowed to continue running the command. - * - * @param reason the reason of the action such as banning. - * @param event This is used to be able to use slash command actions such as event.reply(); - * @throws IllegalArgumentException if the reason is over 512 characters. + * Checks whether the given reason is valid. If not, it will handle the situation and respond to + * the user. * + * @param reason the reason to check + * @param event the event used to respond to the user + * + * @return whether the reason is valid */ - public static boolean handleReason(@NotNull String reason, @NotNull SlashCommandEvent event) { + public static boolean handleReason(@NotNull CharSequence reason, @NotNull Interaction event) { if (reason.length() <= REASON_MAX_LENGTH) { return true; } - event.reply("The reason can not be over " + REASON_MAX_LENGTH + " characters") + event + .reply("The reason can not be longer than %d characters (current length is %d)" + .formatted(REASON_MAX_LENGTH, reason.length())) .setEphemeral(true) .queue(); return false; diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index c11e6474c0..973c7aaef3 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -1,10 +1,13 @@ package org.togetherjava.tjbot.commands.moderation; import net.dv8tion.jda.api.Permission; +import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.exceptions.ErrorResponseException; +import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionType; import net.dv8tion.jda.api.requests.ErrorResponse; import org.jetbrains.annotations.NotNull; @@ -16,40 +19,36 @@ import java.util.Objects; /** - * This command allows you to unban a user. The unban command requires the user to input the user - * who is subject to get unbanned. Unbanning can also be paired with a reason for unbanning the - * user. The command fails if the user is currently not banned already. + * Unbans a given user. Unbanning can also be paired with a reason. The command fails if the user is + * currently not banned. */ public final class UnbanCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(UnbanCommand.class); - private static final String USER_OPTION = "user"; + private static final String TARGET_OPTION = "user"; private static final String REASON_OPTION = "reason"; /** * Constructs an instance. */ public UnbanCommand() { - super("unban", "Unbans a given user", SlashCommandVisibility.GUILD); + super("unban", "Unbans the given user from the server", SlashCommandVisibility.GUILD); - getData().addOption(OptionType.USER, USER_OPTION, "The user who you want to unban", true) + getData() + .addOption(OptionType.USER, TARGET_OPTION, "The banned user who you want to unban", + true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be unbanned", true); } - @Override - public void onSlashCommand(@NotNull SlashCommandEvent event) { - User target = Objects.requireNonNull(event.getOption(USER_OPTION), "The user is null") - .getAsUser(); - Member author = Objects.requireNonNull(event.getMember(), "The author is null"); - + private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, + @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { if (!author.hasPermission(Permission.BAN_MEMBERS)) { event.reply( "You can not unban users in this guild since you do not have the BAN_MEMBERS permission.") .setEphemeral(true) .queue(); - return; + return false; } - Member bot = Objects.requireNonNull(event.getGuild(), "The guild is null").getSelfMember(); if (!bot.hasPermission(Permission.BAN_MEMBERS)) { event.reply( "I can not unban users in this guild since I do not have the BAN_MEMBERS permission.") @@ -57,44 +56,57 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { .queue(); logger.error("The bot does not have BAN_MEMBERS permission on the server '{}' ", - event.getGuild().getName()); - return; + guild.getName()); + return false; } - - String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") - .getAsString(); - - if (!ModerationUtils.handleReason(reason, event)) { - return; - } - - unban(target, reason, author, event); + return true; } - private static void unban(@NotNull User target, @NotNull String reason, @NotNull Member author, - @NotNull SlashCommandEvent event) { - event.getGuild().unban(target).reason(reason).queue(v -> { + private static void unban(@NotNull User target, @NotNull Member author, @NotNull String reason, + @NotNull Guild guild, @NotNull Interaction event) { + String targetTag = target.getAsTag(); + + guild.unban(target).reason(reason).queue(result -> { event - .reply("The user " + author.getUser().getAsTag() + " unbanned the user " - + target.getAsTag() + " for: " + reason) + .reply("'%s' was unbanned by '%s' for: %s".formatted(author.getUser().getAsTag(), + targetTag, reason)) .queue(); - - logger.info(" {} ({}) unbanned the user '{}' for: '{}'", author.getUser().getAsTag(), - author.getIdLong(), target.getAsTag(), reason); + logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'", + author.getUser().getAsTag(), author.getId(), targetTag, target.getId(), + guild.getName(), reason); }, throwable -> { if (throwable instanceof ErrorResponseException errorResponseException && errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { + event.reply("The specified user does not exist").setEphemeral(true).queue(); - event.reply("The specified user doesn't exist").setEphemeral(true).queue(); - - logger.debug("I could not unban the user '{}' because they do not exist", - target.getAsTag()); + logger.debug("Unable to unban the user '{}' because they do not exist", targetTag); } else { event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); - logger.warn("Something went wrong during the process of unbanning the user ", - throwable); + logger.warn("Something unexpected went wrong while trying to unban the user '{}'", + targetTag, throwable); } }); } + + @Override + public void onSlashCommand(@NotNull SlashCommandEvent event) { + User target = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null") + .getAsUser(); + Member author = Objects.requireNonNull(event.getMember(), "The author is null"); + String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") + .getAsString(); + + Guild guild = Objects.requireNonNull(event.getGuild()); + Member bot = guild.getSelfMember(); + + if (!handleHasPermissions(bot, author, guild, event)) { + return; + } + if (!ModerationUtils.handleReason(reason, event)) { + return; + } + + unban(target, author, reason, guild, event); + } } From 9329ce54bbf9bf2cbc66f83d2ed8c9188326b0ba Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Wed, 3 Nov 2021 10:17:51 +0100 Subject: [PATCH 03/21] Linter issues --- .../org/togetherjava/tjbot/commands/moderation/BanCommand.java | 3 +++ .../togetherjava/tjbot/commands/moderation/KickCommand.java | 2 ++ .../tjbot/commands/moderation/ModerationUtils.java | 1 + .../togetherjava/tjbot/commands/moderation/UnbanCommand.java | 1 + 4 files changed, 7 insertions(+) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 5e411736a8..f7c5d7a69b 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -45,6 +45,7 @@ public BanCommand() { true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); } + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, @NotNull Member target, @NotNull Interaction event) { String targetTag = target.getUser().getAsTag(); @@ -64,6 +65,7 @@ private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull return true; } + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { if (!author.hasPermission(Permission.BAN_MEMBERS)) { @@ -87,6 +89,7 @@ private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, return true; } + @SuppressWarnings("MethodWithTooManyParameters") private static void banUser(@NotNull User target, @NotNull Member author, @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, @NotNull SlashCommandEvent event) { diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 466195aaf4..3a6abee014 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -44,6 +44,7 @@ private static void handleAbsentTarget(@NotNull Interaction event) { .queue(); } + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, @NotNull Member target, @NotNull Interaction event) { String targetTag = target.getUser().getAsTag(); @@ -63,6 +64,7 @@ private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull return true; } + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { if (!author.hasPermission(Permission.KICK_MEMBERS)) { diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index cd9bfec2ae..44e4de1d6b 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -26,6 +26,7 @@ public enum ModerationUtils { * * @return whether the reason is valid */ + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") public static boolean handleReason(@NotNull CharSequence reason, @NotNull Interaction event) { if (reason.length() <= REASON_MAX_LENGTH) { return true; diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 973c7aaef3..4d7b160518 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -39,6 +39,7 @@ public UnbanCommand() { .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be unbanned", true); } + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { if (!author.hasPermission(Permission.BAN_MEMBERS)) { From 0060f603758685cd937c5399741a4cdee27ba350 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Wed, 3 Nov 2021 10:56:38 +0100 Subject: [PATCH 04/21] Readability fix (CR yusuf) --- .../org/togetherjava/tjbot/commands/moderation/BanCommand.java | 1 + 1 file changed, 1 insertion(+) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index f7c5d7a69b..89ce2479f7 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -139,6 +139,7 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { int deleteHistoryDays = Math .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); + banUser(targetOption.getAsUser(), author, reason, deleteHistoryDays, guild, event); } } From 4db3cc625b6c80e3247934a00296b9db54965939 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Wed, 3 Nov 2021 11:00:19 +0100 Subject: [PATCH 05/21] Moved several handle methods into helper gets rid of code duplication (as flagged by sonar) --- .../tjbot/commands/moderation/BanCommand.java | 54 ++---------- .../commands/moderation/KickCommand.java | 50 +---------- .../commands/moderation/ModerationUtils.java | 87 ++++++++++++++++++- .../commands/moderation/UnbanCommand.java | 28 +----- 4 files changed, 94 insertions(+), 125 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 89ce2479f7..1b6ba73f9f 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -2,11 +2,9 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; -import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionMapping; import net.dv8tion.jda.api.interactions.commands.OptionType; import net.dv8tion.jda.api.interactions.commands.build.OptionData; @@ -27,10 +25,10 @@ * to ban the specific given user (for example a moderator attempting to ban an admin). */ public final class BanCommand extends SlashCommandAdapter { + private static final Logger logger = LoggerFactory.getLogger(BanCommand.class); private static final String TARGET_OPTION = "user"; private static final String DELETE_HISTORY_OPTION = "delete-history"; private static final String REASON_OPTION = "reason"; - private static final Logger logger = LoggerFactory.getLogger(BanCommand.class); /** * Constructs an instance. @@ -45,50 +43,6 @@ public BanCommand() { true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); } - @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, - @NotNull Member target, @NotNull Interaction event) { - String targetTag = target.getUser().getAsTag(); - if (!author.canInteract(target)) { - event.reply("The user " + targetTag + " is too powerful for you to ban.") - .setEphemeral(true) - .queue(); - return false; - } - - if (!bot.canInteract(target)) { - event.reply("The user " + targetTag + " is too powerful for me to ban.") - .setEphemeral(true) - .queue(); - return false; - } - return true; - } - - @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, - @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { - if (!author.hasPermission(Permission.BAN_MEMBERS)) { - event.reply( - "You can not ban users in this guild since you do not have the BAN_MEMBERS permission.") - .setEphemeral(true) - .queue(); - return false; - } - - if (!bot.hasPermission(Permission.BAN_MEMBERS)) { - event.reply( - "I can not ban users in this guild since I do not have the BAN_MEMBERS permission.") - .setEphemeral(true) - .queue(); - - logger.error("The bot does not have BAN_MEMBERS permission on the server '{}' ", - guild.getName()); - return false; - } - return true; - } - @SuppressWarnings("MethodWithTooManyParameters") private static void banUser(@NotNull User target, @NotNull Member author, @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, @@ -127,10 +81,12 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Member bot = guild.getSelfMember(); // Member doesn't exist if attempting to ban a user who is not part of the guild. - if (target != null && !handleCanInteractWithTarget(bot, author, target, event)) { + if (target != null && !ModerationUtils.handleCanInteractWithTarget("ban", bot, author, + target, event)) { return; } - if (!handleHasPermissions(bot, author, guild, event)) { + if (!ModerationUtils.handleHasPermissions("ban", Permission.BAN_MEMBERS, bot, author, guild, + event)) { return; } if (!ModerationUtils.handleReason(reason, event)) { diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 3a6abee014..54d6c8bb72 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -2,7 +2,6 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.interactions.Interaction; @@ -44,50 +43,6 @@ private static void handleAbsentTarget(@NotNull Interaction event) { .queue(); } - @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - private static boolean handleCanInteractWithTarget(@NotNull Member bot, @NotNull Member author, - @NotNull Member target, @NotNull Interaction event) { - String targetTag = target.getUser().getAsTag(); - if (!author.canInteract(target)) { - event.reply("The user " + targetTag + " is too powerful for you to kick.") - .setEphemeral(true) - .queue(); - return false; - } - - if (!bot.canInteract(target)) { - event.reply("The user " + targetTag + " is too powerful for me to kick.") - .setEphemeral(true) - .queue(); - return false; - } - return true; - } - - @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, - @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { - if (!author.hasPermission(Permission.KICK_MEMBERS)) { - event.reply( - "You can not kick users in this guild since you do not have the KICK_MEMBERS permission.") - .setEphemeral(true) - .queue(); - return false; - } - - if (!bot.hasPermission(Permission.KICK_MEMBERS)) { - event.reply( - "I can not kick users in this guild since I do not have the KICK_MEMBERS permission.") - .setEphemeral(true) - .queue(); - - logger.error("The bot does not have KICK_MEMBERS permission on the server '{}' ", - guild.getName()); - return false; - } - return true; - } - private static void kickUser(@NotNull Member target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull SlashCommandEvent event) { event.getJDA() @@ -126,10 +81,11 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { handleAbsentTarget(event); return; } - if (!handleCanInteractWithTarget(bot, author, target, event)) { + if (!ModerationUtils.handleCanInteractWithTarget("kick", bot, author, target, event)) { return; } - if (!handleHasPermissions(bot, author, guild, event)) { + if (!ModerationUtils.handleHasPermissions("kick", Permission.KICK_MEMBERS, bot, author, + guild, event)) { return; } if (!ModerationUtils.handleReason(reason, event)) { diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index 44e4de1d6b..ca345f26c7 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -1,16 +1,22 @@ package org.togetherjava.tjbot.commands.moderation; +import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; +import net.dv8tion.jda.api.entities.IPermissionHolder; +import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.interactions.Interaction; import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Utility class offering helpers revolving around user moderation, such as banning or kicking. */ -public enum ModerationUtils { +enum ModerationUtils { ; + private static final Logger logger = LoggerFactory.getLogger(ModerationUtils.class); /** * The maximal character limit for the reason of an auditable action, see for example * {@link Guild#ban(User, int, String)}. @@ -23,11 +29,10 @@ public enum ModerationUtils { * * @param reason the reason to check * @param event the event used to respond to the user - * * @return whether the reason is valid */ @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - public static boolean handleReason(@NotNull CharSequence reason, @NotNull Interaction event) { + static boolean handleReason(@NotNull CharSequence reason, @NotNull Interaction event) { if (reason.length() <= REASON_MAX_LENGTH) { return true; } @@ -39,4 +44,80 @@ public static boolean handleReason(@NotNull CharSequence reason, @NotNull Intera .queue(); return false; } + + /** + * Checks whether the given author and bot can interact with the target user. For example + * whether they have enough permissions to ban the user. + *

+ * If not, it will handle the situation and respond to the user. + * + * @param actionVerb the interaction as verb, for example {@code "ban"} or {@code "kick"} + * @param bot the bot attempting to interact with the user + * @param author the author triggering the command + * @param target the target user of the interaction + * @param event the event used to respond to the user + * @return Whether the author and bot can interact with the target user + */ + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") + static boolean handleCanInteractWithTarget(@NotNull String actionVerb, @NotNull Member bot, + @NotNull Member author, @NotNull Member target, @NotNull Interaction event) { + String targetTag = target.getUser().getAsTag(); + if (!author.canInteract(target)) { + event + .reply("The user %s is too powerful for you to %s.".formatted(targetTag, + actionVerb)) + .setEphemeral(true) + .queue(); + return false; + } + + if (!bot.canInteract(target)) { + event + .reply("The user %s is too powerful for me to %s.".formatted(targetTag, actionVerb)) + .setEphemeral(true) + .queue(); + return false; + } + return true; + } + + /** + * Checks whether the given author and bot have enough permission to execute the given action. + * For example whether they have enough permissions to ban users. + *

+ * If not, it will handle the situation and respond to the user. + * + * @param actionVerb the interaction as verb, for example {@code "ban"} or {@code "kick"} + * @param permission the required permission to check + * @param bot the bot attempting to interact with the user + * @param author the author triggering the command + * @param event the event used to respond to the user + * @return Whether the author and bot have the required permission + */ + @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) + static boolean handleHasPermissions(@NotNull String actionVerb, @NotNull Permission permission, + @NotNull IPermissionHolder bot, @NotNull IPermissionHolder author, @NotNull Guild guild, + @NotNull Interaction event) { + if (!author.hasPermission(permission)) { + event + .reply("You can not %s users in this guild since you do not have the %s permission." + .formatted(actionVerb, permission)) + .setEphemeral(true) + .queue(); + return false; + } + + if (!bot.hasPermission(permission)) { + event + .reply("I can not %s users in this guild since I do not have the %s permission." + .formatted(actionVerb, permission)) + .setEphemeral(true) + .queue(); + + logger.error("The bot does not have the '{}' permission on the guild '{}' ", permission, + guild.getName()); + return false; + } + return true; + } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 4d7b160518..24f4dfbea2 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -2,7 +2,6 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.IPermissionHolder; import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; @@ -39,30 +38,6 @@ public UnbanCommand() { .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be unbanned", true); } - @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") - private static boolean handleHasPermissions(@NotNull IPermissionHolder bot, - @NotNull IPermissionHolder author, @NotNull Guild guild, @NotNull Interaction event) { - if (!author.hasPermission(Permission.BAN_MEMBERS)) { - event.reply( - "You can not unban users in this guild since you do not have the BAN_MEMBERS permission.") - .setEphemeral(true) - .queue(); - return false; - } - - if (!bot.hasPermission(Permission.BAN_MEMBERS)) { - event.reply( - "I can not unban users in this guild since I do not have the BAN_MEMBERS permission.") - .setEphemeral(true) - .queue(); - - logger.error("The bot does not have BAN_MEMBERS permission on the server '{}' ", - guild.getName()); - return false; - } - return true; - } - private static void unban(@NotNull User target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull Interaction event) { String targetTag = target.getAsTag(); @@ -101,7 +76,8 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Guild guild = Objects.requireNonNull(event.getGuild()); Member bot = guild.getSelfMember(); - if (!handleHasPermissions(bot, author, guild, event)) { + if (!ModerationUtils.handleHasPermissions("unban", Permission.BAN_MEMBERS, bot, author, + guild, event)) { return; } if (!ModerationUtils.handleReason(reason, event)) { From 445d61777fffb9786c956274c748e841dbafa310 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 13:49:43 +0100 Subject: [PATCH 06/21] Fixed some typos --- .../tjbot/commands/moderation/ModerationUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index ca345f26c7..02375e02e4 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -38,7 +38,7 @@ static boolean handleReason(@NotNull CharSequence reason, @NotNull Interaction e } event - .reply("The reason can not be longer than %d characters (current length is %d)" + .reply("The reason can not be longer than %d characters (current length is %d)." .formatted(REASON_MAX_LENGTH, reason.length())) .setEphemeral(true) .queue(); @@ -114,7 +114,7 @@ static boolean handleHasPermissions(@NotNull String actionVerb, @NotNull Permiss .setEphemeral(true) .queue(); - logger.error("The bot does not have the '{}' permission on the guild '{}' ", permission, + logger.error("The bot does not have the '{}' permission on the guild '{}'.", permission, guild.getName()); return false; } From a1c12f760d112af32712aa7a771f9df4968b8511 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 13:49:54 +0100 Subject: [PATCH 07/21] Not banning already banned users --- .../tjbot/commands/moderation/BanCommand.java | 77 +++++++++++++++---- 1 file changed, 64 insertions(+), 13 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 1b6ba73f9f..61feb97ead 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -5,9 +5,14 @@ import net.dv8tion.jda.api.entities.Member; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; +import net.dv8tion.jda.api.exceptions.ErrorResponseException; +import net.dv8tion.jda.api.interactions.Interaction; +import net.dv8tion.jda.api.interactions.InteractionHook; import net.dv8tion.jda.api.interactions.commands.OptionMapping; import net.dv8tion.jda.api.interactions.commands.OptionType; import net.dv8tion.jda.api.interactions.commands.build.OptionData; +import net.dv8tion.jda.api.requests.ErrorResponse; +import net.dv8tion.jda.api.requests.RestAction; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -15,6 +20,7 @@ import org.togetherjava.tjbot.commands.SlashCommandVisibility; import java.util.Objects; +import java.util.Optional; /** * This command can ban users and optionally remove their messages from the past days. Banning can @@ -43,11 +49,22 @@ public BanCommand() { true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); } + private static RestAction handleAlreadyBanned(@NotNull Guild.Ban ban, + @NotNull Interaction event) { + String reason = ban.getReason(); + String reasonText = + reason == null || reason.isBlank() ? "" : " (reason: %s)".formatted(reason); + + String message = "The user '%s' is already banned%s.".formatted(ban.getUser().getAsTag(), + reasonText); + return event.reply(message).setEphemeral(true); + } + @SuppressWarnings("MethodWithTooManyParameters") - private static void banUser(@NotNull User target, @NotNull Member author, + private static RestAction banUser(@NotNull User target, @NotNull Member author, @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, @NotNull SlashCommandEvent event) { - event.getJDA() + return event.getJDA() .openPrivateChannelById(target.getIdLong()) .flatMap(channel -> channel.sendMessage( """ @@ -57,22 +74,47 @@ private static void banUser(@NotNull User target, @NotNull Member author, """ .formatted(guild.getName(), reason))) .mapToResult() - .flatMap(result -> guild.ban(target, deleteHistoryDays, reason)) + .flatMap(result -> { + logger.info( + "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'", + author.getUser().getAsTag(), author.getId(), target.getAsTag(), + target.getId(), guild.getName(), deleteHistoryDays, reason); + + return guild.ban(target, deleteHistoryDays, reason); + }) .flatMap(result -> event.reply("'%s' was banned by '%s' for: %s" - .formatted(target.getAsTag(), author.getUser().getAsTag(), reason))) - .queue(); + .formatted(target.getAsTag(), author.getUser().getAsTag(), reason))); + } + + private static Optional> handleNotAlreadyBannedResponse( + @NotNull Throwable alreadyBannedFailure, @NotNull Interaction event, + @NotNull Guild guild, @NotNull User target) { + if (alreadyBannedFailure instanceof ErrorResponseException errorResponseException) { + if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_BAN) { + return Optional.empty(); + } - logger.info( - "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'", - author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), - guild.getName(), deleteHistoryDays, reason); + if (errorResponseException.getErrorResponse() == ErrorResponse.MISSING_PERMISSIONS) { + logger.error("The bot does not have the '{}' permission on the guild '{}'.", + Permission.BAN_MEMBERS, guild.getName()); + return Optional.of(event.reply( + "I can not ban users in this guild since I do not have the %s permission." + .formatted(Permission.BAN_MEMBERS)) + .setEphemeral(true)); + } + } + logger.warn("Something unexpected went wrong while trying to ban the user '{}'", + target.getAsTag(), alreadyBannedFailure); + return Optional.of(event.reply("Failed to ban the user due to an unexpected problem.") + .setEphemeral(true)); } @Override public void onSlashCommand(@NotNull SlashCommandEvent event) { OptionMapping targetOption = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null"); - Member target = targetOption.getAsMember(); + Member targetMember = targetOption.getAsMember(); + User targetUser = targetOption.getAsUser(); Member author = Objects.requireNonNull(event.getMember(), "The author is null"); String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") .getAsString(); @@ -81,8 +123,8 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Member bot = guild.getSelfMember(); // Member doesn't exist if attempting to ban a user who is not part of the guild. - if (target != null && !ModerationUtils.handleCanInteractWithTarget("ban", bot, author, - target, event)) { + if (targetMember != null && !ModerationUtils.handleCanInteractWithTarget("ban", bot, author, + targetMember, event)) { return; } if (!ModerationUtils.handleHasPermissions("ban", Permission.BAN_MEMBERS, bot, author, guild, @@ -96,6 +138,15 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { int deleteHistoryDays = Math .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); - banUser(targetOption.getAsUser(), author, reason, deleteHistoryDays, guild, event); + // Ban the user, but only if not already banned + guild.retrieveBan(targetUser).mapToResult().flatMap(alreadyBanned -> { + if (alreadyBanned.isSuccess()) { + return handleAlreadyBanned(alreadyBanned.get(), event); + } + + return handleNotAlreadyBannedResponse(Objects + .requireNonNull(alreadyBanned.getFailure()), event, guild, targetUser).orElseGet( + () -> banUser(targetUser, author, reason, deleteHistoryDays, guild, event)); + }).queue(); } } From db3d0231eb9ce88bd85e0130ada1492b2fbee7e5 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 14:27:20 +0100 Subject: [PATCH 08/21] Added "unable to send DM" notice --- .../tjbot/commands/moderation/BanCommand.java | 14 +++++++--- .../commands/moderation/KickCommand.java | 27 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 61feb97ead..35460fcb5e 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -74,16 +74,22 @@ private static RestAction banUser(@NotNull User target, @NotNul """ .formatted(guild.getName(), reason))) .mapToResult() - .flatMap(result -> { + .flatMap(sendDmResult -> { logger.info( "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'", author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), guild.getName(), deleteHistoryDays, reason); - return guild.ban(target, deleteHistoryDays, reason); + return guild.ban(target, deleteHistoryDays, reason) + .map(banResult -> sendDmResult.isSuccess()); }) - .flatMap(result -> event.reply("'%s' was banned by '%s' for: %s" - .formatted(target.getAsTag(), author.getUser().getAsTag(), reason))); + .flatMap(hasSentDm -> { + String dmNotice = + Boolean.TRUE.equals(hasSentDm) ? "" : " (unable to send them a DM)"; + String message = "'%s' was banned by '%s'%s for: %s".formatted(target.getAsTag(), + author.getUser().getAsTag(), dmNotice, reason); + return event.reply(message); + }); } private static Optional> handleNotAlreadyBannedResponse( diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 54d6c8bb72..56887b8cb6 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -45,8 +45,10 @@ private static void handleAbsentTarget(@NotNull Interaction event) { private static void kickUser(@NotNull Member target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull SlashCommandEvent event) { + String targetTag = target.getUser().getAsTag(); + String targetId = target.getUser().getId(); event.getJDA() - .openPrivateChannelById(target.getUser().getId()) + .openPrivateChannelById(targetId) .flatMap(channel -> channel.sendMessage( """ Hey there, sorry to tell you but unfortunately you have been kicked from the server %s. @@ -55,14 +57,23 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, """ .formatted(guild.getName(), reason))) .mapToResult() - .flatMap(result -> guild.kick(target, reason).reason(reason)) - .flatMap(result -> event.reply("'%s' was kicked by '%s' for: %s" - .formatted(target.getUser().getAsTag(), author.getUser().getAsTag(), reason))) - .queue(); + .flatMap(sendDmResult -> { + logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'", + author.getUser().getAsTag(), author.getId(), targetTag, targetId, + guild.getName(), reason); - logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'", - author.getUser().getAsTag(), author.getId(), target.getUser().getAsTag(), - target.getUser().getId(), guild.getName(), reason); + return guild.kick(target, reason) + .reason(reason) + .map(kickResult -> sendDmResult.isSuccess()); + }) + .flatMap(hasSentDm -> { + String dmNotice = + Boolean.TRUE.equals(hasSentDm) ? "" : " (unable to send them a DM)"; + String message = "'%s' was kicked by '%s'%s for: %s".formatted(targetTag, + author.getUser().getAsTag(), dmNotice, reason); + return event.reply(message); + }) + .queue(); } @Override From 2f1aba000aaf563a4cd726a11b1259886712d0d8 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 14:39:45 +0100 Subject: [PATCH 09/21] Fixed bug with unbanning non-banned users --- .../commands/moderation/UnbanCommand.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 24f4dfbea2..bd9a82a7f7 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -50,18 +50,24 @@ private static void unban(@NotNull User target, @NotNull Member author, @NotNull logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'", author.getUser().getAsTag(), author.getId(), targetTag, target.getId(), guild.getName(), reason); - }, throwable -> { - if (throwable instanceof ErrorResponseException errorResponseException - && errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { - event.reply("The specified user does not exist").setEphemeral(true).queue(); - - logger.debug("Unable to unban the user '{}' because they do not exist", targetTag); - } else { - event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); - - logger.warn("Something unexpected went wrong while trying to unban the user '{}'", - targetTag, throwable); + }, unbanFailure -> { + if (unbanFailure instanceof ErrorResponseException errorResponseException) { + if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { + event.reply("The specified user does not exist.").setEphemeral(true).queue(); + logger.debug("Unable to unban the user '{}' because they do not exist.", + targetTag); + return; + } + if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_BAN) { + event.reply("The specified user is not banned.").setEphemeral(true).queue(); + logger.debug("Unable to unban the user '{}' because they are not banned.", + targetTag); + return; + } } + event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); + logger.warn("Something unexpected went wrong while trying to unban the user '{}'.", + targetTag, unbanFailure); }); } From 9c8016ff0a7d3585d75882a8f388e6bd4516d315 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 14:39:53 +0100 Subject: [PATCH 10/21] Fixed some typos --- .../togetherjava/tjbot/commands/moderation/BanCommand.java | 4 ++-- .../togetherjava/tjbot/commands/moderation/KickCommand.java | 2 +- .../togetherjava/tjbot/commands/moderation/UnbanCommand.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 35460fcb5e..fbc904e408 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -76,7 +76,7 @@ private static RestAction banUser(@NotNull User target, @NotNul .mapToResult() .flatMap(sendDmResult -> { logger.info( - "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'", + "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'.", author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), guild.getName(), deleteHistoryDays, reason); @@ -109,7 +109,7 @@ private static Optional> handleNotAlreadyBannedRespo .setEphemeral(true)); } } - logger.warn("Something unexpected went wrong while trying to ban the user '{}'", + logger.warn("Something unexpected went wrong while trying to ban the user '{}'.", target.getAsTag(), alreadyBannedFailure); return Optional.of(event.reply("Failed to ban the user due to an unexpected problem.") .setEphemeral(true)); diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 56887b8cb6..ee597e562f 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -58,7 +58,7 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, .formatted(guild.getName(), reason))) .mapToResult() .flatMap(sendDmResult -> { - logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'", + logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'.", author.getUser().getAsTag(), author.getId(), targetTag, targetId, guild.getName(), reason); diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index bd9a82a7f7..00aa66d52d 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -47,7 +47,7 @@ private static void unban(@NotNull User target, @NotNull Member author, @NotNull .reply("'%s' was unbanned by '%s' for: %s".formatted(author.getUser().getAsTag(), targetTag, reason)) .queue(); - logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'", + logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'.", author.getUser().getAsTag(), author.getId(), targetTag, target.getId(), guild.getName(), reason); }, unbanFailure -> { From b72023820ba09b8e823649f6106bdfef25e3114f Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 5 Nov 2021 15:18:55 +0100 Subject: [PATCH 11/21] Using embed instead of plain messages --- .../tjbot/commands/moderation/BanCommand.java | 12 +-- .../commands/moderation/KickCommand.java | 20 ++--- .../commands/moderation/ModerationUtils.java | 83 ++++++++++++++++++- .../commands/moderation/UnbanCommand.java | 9 +- 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index fbc904e408..391ae7e4a7 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -83,13 +83,13 @@ private static RestAction banUser(@NotNull User target, @NotNul return guild.ban(target, deleteHistoryDays, reason) .map(banResult -> sendDmResult.isSuccess()); }) - .flatMap(hasSentDm -> { + .map(hasSentDm -> { String dmNotice = - Boolean.TRUE.equals(hasSentDm) ? "" : " (unable to send them a DM)"; - String message = "'%s' was banned by '%s'%s for: %s".formatted(target.getAsTag(), - author.getUser().getAsTag(), dmNotice, reason); - return event.reply(message); - }); + Boolean.TRUE.equals(hasSentDm) ? "" : "(Unable to send them a DM.)"; + return ModerationUtils.createActionResponse(author.getUser(), + ModerationUtils.Action.BAN, target, dmNotice, reason); + }) + .flatMap(event::replyEmbeds); } private static Optional> handleNotAlreadyBannedResponse( diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index ee597e562f..3cd6a86b7c 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -3,6 +3,7 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.Member; +import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionType; @@ -45,10 +46,9 @@ private static void handleAbsentTarget(@NotNull Interaction event) { private static void kickUser(@NotNull Member target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull SlashCommandEvent event) { - String targetTag = target.getUser().getAsTag(); - String targetId = target.getUser().getId(); + User targetUser = target.getUser(); event.getJDA() - .openPrivateChannelById(targetId) + .openPrivateChannelById(targetUser.getId()) .flatMap(channel -> channel.sendMessage( """ Hey there, sorry to tell you but unfortunately you have been kicked from the server %s. @@ -59,20 +59,20 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, .mapToResult() .flatMap(sendDmResult -> { logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'.", - author.getUser().getAsTag(), author.getId(), targetTag, targetId, - guild.getName(), reason); + author.getUser().getAsTag(), author.getId(), targetUser.getAsTag(), + targetUser.getId(), guild.getName(), reason); return guild.kick(target, reason) .reason(reason) .map(kickResult -> sendDmResult.isSuccess()); }) - .flatMap(hasSentDm -> { + .map(hasSentDm -> { String dmNotice = - Boolean.TRUE.equals(hasSentDm) ? "" : " (unable to send them a DM)"; - String message = "'%s' was kicked by '%s'%s for: %s".formatted(targetTag, - author.getUser().getAsTag(), dmNotice, reason); - return event.reply(message); + Boolean.TRUE.equals(hasSentDm) ? "" : "(Unable to send them a DM.)"; + return ModerationUtils.createActionResponse(author.getUser(), + ModerationUtils.Action.KICK, targetUser, dmNotice, reason); }) + .flatMap(event::replyEmbeds) .queue(); } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index 02375e02e4..5142a4e939 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -1,15 +1,17 @@ package org.togetherjava.tjbot.commands.moderation; +import net.dv8tion.jda.api.EmbedBuilder; import net.dv8tion.jda.api.Permission; -import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.IPermissionHolder; -import net.dv8tion.jda.api.entities.Member; -import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.entities.*; import net.dv8tion.jda.api.interactions.Interaction; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.awt.*; +import java.time.Instant; + /** * Utility class offering helpers revolving around user moderation, such as banning or kicking. */ @@ -22,6 +24,7 @@ enum ModerationUtils { * {@link Guild#ban(User, int, String)}. */ private static final int REASON_MAX_LENGTH = 512; + private static final Color AMBIENT_COLOR = Color.decode("#895FE8"); /** * Checks whether the given reason is valid. If not, it will handle the situation and respond to @@ -120,4 +123,76 @@ static boolean handleHasPermissions(@NotNull String actionVerb, @NotNull Permiss } return true; } + + /** + * Creates a message to be displayed as response to a moderation action. + * + * Essentially, it informs others about the action, such as "John banned Bob for playing with + * the fire.". + * + * @param author the author executing the action + * @param action the action that is executed + * @param target the target of the action + * @param extraMessage an optional extra message to be displayed in the response, {@code null} + * if not desired + * @param reason an optional reason for why the action is executed, {@code null} if not desired + * @return the created response + */ + static @NotNull MessageEmbed createActionResponse(@NotNull User author, @NotNull Action action, + @NotNull User target, @Nullable String extraMessage, @Nullable String reason) { + String description = "%s **%s** (id: %s).".formatted(action.getVerb(), target.getAsTag(), + target.getId()); + if (extraMessage != null && !extraMessage.isBlank()) { + description += "\n" + extraMessage; + } + if (reason != null && !reason.isBlank()) { + description += "\n\nReason: " + reason; + } + return new EmbedBuilder().setAuthor(author.getAsTag(), null, author.getAvatarUrl()) + .setDescription(description) + .setTimestamp(Instant.now()) + .setColor(AMBIENT_COLOR) + .build(); + } + + /** + * All available moderation actions. + */ + enum Action { + /** + * When a user bans another user. + */ + BAN("banned"), + /** + * When a user unbans another user. + */ + UNBAN("unbanned"), + /** + * When a user kicks another user. + */ + KICK("kicked"); + + private final String verb; + + /** + * Creates an instance with the given verb + * + * @param verb the verb of the action, as it would be used in a sentence, such as "banned" + * or "kicked" + */ + Action(@NotNull String verb) { + this.verb = verb; + } + + /** + * Gets the verb of the action, as it would be used in a sentence. + *

+ * Such as "banned" or "kicked" + * + * @return the verb of this action + */ + String getVerb() { + return verb; + } + } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 00aa66d52d..5ed8d90754 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -3,6 +3,7 @@ import net.dv8tion.jda.api.Permission; import net.dv8tion.jda.api.entities.Guild; import net.dv8tion.jda.api.entities.Member; +import net.dv8tion.jda.api.entities.MessageEmbed; import net.dv8tion.jda.api.entities.User; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.exceptions.ErrorResponseException; @@ -43,10 +44,10 @@ private static void unban(@NotNull User target, @NotNull Member author, @NotNull String targetTag = target.getAsTag(); guild.unban(target).reason(reason).queue(result -> { - event - .reply("'%s' was unbanned by '%s' for: %s".formatted(author.getUser().getAsTag(), - targetTag, reason)) - .queue(); + MessageEmbed message = ModerationUtils.createActionResponse(author.getUser(), + ModerationUtils.Action.UNBAN, target, null, reason); + event.replyEmbeds(message).queue(); + logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'.", author.getUser().getAsTag(), author.getId(), targetTag, target.getId(), guild.getName(), reason); From 427c22460953de4ee09028e153b5601a74a0ea83 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Sat, 6 Nov 2021 16:37:54 +0100 Subject: [PATCH 12/21] using constants to separate magic strings --- .../tjbot/commands/moderation/BanCommand.java | 12 +++++++----- .../tjbot/commands/moderation/KickCommand.java | 8 +++++--- .../tjbot/commands/moderation/UnbanCommand.java | 6 ++++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 391ae7e4a7..aa7a74eda8 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -35,12 +35,14 @@ public final class BanCommand extends SlashCommandAdapter { private static final String TARGET_OPTION = "user"; private static final String DELETE_HISTORY_OPTION = "delete-history"; private static final String REASON_OPTION = "reason"; + private static final String COMMAND_NAME = "ban"; + private static final String ACTION_VERB = "ban"; /** * Constructs an instance. */ public BanCommand() { - super("ban", "Bans the given user from the server", SlashCommandVisibility.GUILD); + super(COMMAND_NAME, "Bans the given user from the server", SlashCommandVisibility.GUILD); getData().addOption(OptionType.USER, TARGET_OPTION, "The user who you want to ban", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be banned", true) @@ -129,12 +131,12 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Member bot = guild.getSelfMember(); // Member doesn't exist if attempting to ban a user who is not part of the guild. - if (targetMember != null && !ModerationUtils.handleCanInteractWithTarget("ban", bot, author, - targetMember, event)) { + if (targetMember != null && !ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, + author, targetMember, event)) { return; } - if (!ModerationUtils.handleHasPermissions("ban", Permission.BAN_MEMBERS, bot, author, guild, - event)) { + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, + guild, event)) { return; } if (!ModerationUtils.handleReason(reason, event)) { diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 3cd6a86b7c..c3897bbc2c 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -27,12 +27,14 @@ public final class KickCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(KickCommand.class); private static final String TARGET_OPTION = "user"; private static final String REASON_OPTION = "reason"; + private static final String COMMAND_NAME = "kick"; + private static final String ACTION_VERB = "kick"; /** * Constructs an instance. */ public KickCommand() { - super("kick", "Kicks the given user from the server", SlashCommandVisibility.GUILD); + super(COMMAND_NAME, "Kicks the given user from the server", SlashCommandVisibility.GUILD); getData().addOption(OptionType.USER, TARGET_OPTION, "The user who you want to kick", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be kicked", true); @@ -92,10 +94,10 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { handleAbsentTarget(event); return; } - if (!ModerationUtils.handleCanInteractWithTarget("kick", bot, author, target, event)) { + if (!ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, author, target, event)) { return; } - if (!ModerationUtils.handleHasPermissions("kick", Permission.KICK_MEMBERS, bot, author, + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.KICK_MEMBERS, bot, author, guild, event)) { return; } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 5ed8d90754..a705576358 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -26,12 +26,14 @@ public final class UnbanCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(UnbanCommand.class); private static final String TARGET_OPTION = "user"; private static final String REASON_OPTION = "reason"; + private static final String COMMAND_NAME = "unban"; + private static final String ACTION_VERB = "unban"; /** * Constructs an instance. */ public UnbanCommand() { - super("unban", "Unbans the given user from the server", SlashCommandVisibility.GUILD); + super(COMMAND_NAME, "Unbans the given user from the server", SlashCommandVisibility.GUILD); getData() .addOption(OptionType.USER, TARGET_OPTION, "The banned user who you want to unban", @@ -83,7 +85,7 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Guild guild = Objects.requireNonNull(event.getGuild()); Member bot = guild.getSelfMember(); - if (!ModerationUtils.handleHasPermissions("unban", Permission.BAN_MEMBERS, bot, author, + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, guild, event)) { return; } From f7d565fa7fa4ac9812bf116c30b660fd7a12e1b7 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Sat, 6 Nov 2021 16:51:15 +0100 Subject: [PATCH 13/21] Readability cleanup on the "checks" * introduced helper method `handleChecks` where all the preconditions are checked (reason, permissions, ...) --- .../tjbot/commands/moderation/BanCommand.java | 37 +++++++++++------- .../commands/moderation/KickCommand.java | 39 +++++++++++-------- .../commands/moderation/UnbanCommand.java | 23 ++++++----- 3 files changed, 58 insertions(+), 41 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index aa7a74eda8..045bc172e1 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -14,6 +14,7 @@ import net.dv8tion.jda.api.requests.ErrorResponse; import net.dv8tion.jda.api.requests.RestAction; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.commands.SlashCommandAdapter; @@ -117,12 +118,27 @@ private static Optional> handleNotAlreadyBannedRespo .setEphemeral(true)); } + @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) + private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, + @Nullable Member target, @NotNull CharSequence reason, @NotNull Guild guild, + @NotNull Interaction event) { + // Member doesn't exist if attempting to ban a user who is not part of the guild. + if (target != null && !ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, author, + target, event)) { + return false; + } + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, + guild, event)) { + return false; + } + return ModerationUtils.handleReason(reason, event); + } + @Override public void onSlashCommand(@NotNull SlashCommandEvent event) { OptionMapping targetOption = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null"); - Member targetMember = targetOption.getAsMember(); - User targetUser = targetOption.getAsUser(); + User target = targetOption.getAsUser(); Member author = Objects.requireNonNull(event.getMember(), "The author is null"); String reason = Objects.requireNonNull(event.getOption(REASON_OPTION), "The reason is null") .getAsString(); @@ -130,16 +146,7 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Guild guild = Objects.requireNonNull(event.getGuild()); Member bot = guild.getSelfMember(); - // Member doesn't exist if attempting to ban a user who is not part of the guild. - if (targetMember != null && !ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, - author, targetMember, event)) { - return; - } - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, - guild, event)) { - return; - } - if (!ModerationUtils.handleReason(reason, event)) { + if (!handleChecks(bot, author, targetOption.getAsMember(), reason, guild, event)) { return; } @@ -147,14 +154,14 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { .toIntExact(Objects.requireNonNull(event.getOption(DELETE_HISTORY_OPTION)).getAsLong()); // Ban the user, but only if not already banned - guild.retrieveBan(targetUser).mapToResult().flatMap(alreadyBanned -> { + guild.retrieveBan(target).mapToResult().flatMap(alreadyBanned -> { if (alreadyBanned.isSuccess()) { return handleAlreadyBanned(alreadyBanned.get(), event); } return handleNotAlreadyBannedResponse(Objects - .requireNonNull(alreadyBanned.getFailure()), event, guild, targetUser).orElseGet( - () -> banUser(targetUser, author, reason, deleteHistoryDays, guild, event)); + .requireNonNull(alreadyBanned.getFailure()), event, guild, target).orElseGet( + () -> banUser(target, author, reason, deleteHistoryDays, guild, event)); }).queue(); } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index c3897bbc2c..4c8d8e7ad4 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -8,10 +8,12 @@ import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionType; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.commands.SlashCommandAdapter; import org.togetherjava.tjbot.commands.SlashCommandVisibility; + import java.util.Objects; @@ -21,7 +23,6 @@ *

* The command fails if the user triggering it is lacking permissions to either kick other users or * to kick the specific given user (for example a moderator attempting to kick an admin). - * */ public final class KickCommand extends SlashCommandAdapter { private static final Logger logger = LoggerFactory.getLogger(KickCommand.class); @@ -78,6 +79,25 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, .queue(); } + @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) + private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, + @Nullable Member target, @NotNull CharSequence reason, @NotNull Guild guild, + @NotNull Interaction event) { + // Member doesn't exist if attempting to kick a user who is not part of the guild anymore. + if (target == null) { + handleAbsentTarget(event); + return false; + } + if (!ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, author, target, event)) { + return false; + } + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.KICK_MEMBERS, bot, author, + guild, event)) { + return false; + } + return ModerationUtils.handleReason(reason, event); + } + @Override public void onSlashCommand(@NotNull SlashCommandEvent event) { Member target = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null") @@ -89,22 +109,9 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Guild guild = Objects.requireNonNull(event.getGuild()); Member bot = guild.getSelfMember(); - // Member doesn't exist if attempting to kick a user who is not part of the guild anymore. - if (target == null) { - handleAbsentTarget(event); - return; - } - if (!ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, author, target, event)) { - return; - } - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.KICK_MEMBERS, bot, author, - guild, event)) { + if (!handleChecks(bot, author, target, reason, guild, event)) { return; } - if (!ModerationUtils.handleReason(reason, event)) { - return; - } - - kickUser(target, author, reason, guild, event); + kickUser(Objects.requireNonNull(target), author, reason, guild, event); } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index a705576358..821118e148 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -1,10 +1,7 @@ package org.togetherjava.tjbot.commands.moderation; import net.dv8tion.jda.api.Permission; -import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.Member; -import net.dv8tion.jda.api.entities.MessageEmbed; -import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.entities.*; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.exceptions.ErrorResponseException; import net.dv8tion.jda.api.interactions.Interaction; @@ -74,6 +71,17 @@ private static void unban(@NotNull User target, @NotNull Member author, @NotNull }); } + @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion"}) + private static boolean handleChecks(@NotNull IPermissionHolder bot, + @NotNull IPermissionHolder author, @NotNull CharSequence reason, @NotNull Guild guild, + @NotNull Interaction event) { + if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, + guild, event)) { + return false; + } + return ModerationUtils.handleReason(reason, event); + } + @Override public void onSlashCommand(@NotNull SlashCommandEvent event) { User target = Objects.requireNonNull(event.getOption(TARGET_OPTION), "The target is null") @@ -85,14 +93,9 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { Guild guild = Objects.requireNonNull(event.getGuild()); Member bot = guild.getSelfMember(); - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, - guild, event)) { + if (!handleChecks(bot, author, reason, guild, event)) { return; } - if (!ModerationUtils.handleReason(reason, event)) { - return; - } - unban(target, author, reason, guild, event); } } From c2dd08da16ead46ab521f5036a7b6e931ea78a61 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Sun, 7 Nov 2021 10:23:10 +0100 Subject: [PATCH 14/21] NotNull for enum (CR by Tais) --- .../togetherjava/tjbot/commands/moderation/ModerationUtils.java | 1 + 1 file changed, 1 insertion(+) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index 5142a4e939..6d4977c39a 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -191,6 +191,7 @@ enum Action { * * @return the verb of this action */ + @NotNull String getVerb() { return verb; } From 20a8fea19fa5d3a7ac0ba9352d0f5d7fee636a7d Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 8 Nov 2021 09:59:57 +0100 Subject: [PATCH 15/21] Making moderation commands role based instead of perms * this allows to give selected roles the ability to use moderation commands without giving them the ability to use them without the bot. * for example staff assistants could kick users with the bot, but not without the bot * that way, we can force them to go through the bot instead of using Discord GUI or other bots * which gives us more control and visibility --- .../tjbot/commands/moderation/BanCommand.java | 14 +++++- .../commands/moderation/KickCommand.java | 14 +++++- .../commands/moderation/ModerationUtils.java | 48 +++++++++++++------ .../commands/moderation/UnbanCommand.java | 17 +++++-- .../org/togetherjava/tjbot/config/Config.java | 29 ++++++++++- 5 files changed, 98 insertions(+), 24 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index 045bc172e1..ef4d472bae 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -19,9 +19,12 @@ import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.commands.SlashCommandAdapter; import org.togetherjava.tjbot.commands.SlashCommandVisibility; +import org.togetherjava.tjbot.config.Config; import java.util.Objects; import java.util.Optional; +import java.util.function.Predicate; +import java.util.regex.Pattern; /** * This command can ban users and optionally remove their messages from the past days. Banning can @@ -38,6 +41,7 @@ public final class BanCommand extends SlashCommandAdapter { private static final String REASON_OPTION = "reason"; private static final String COMMAND_NAME = "ban"; private static final String ACTION_VERB = "ban"; + private final Predicate hasRequiredRole; /** * Constructs an instance. @@ -50,6 +54,9 @@ public BanCommand() { .addOptions(new OptionData(OptionType.INTEGER, DELETE_HISTORY_OPTION, "the amount of days of the message history to delete, none means no messages are deleted.", true).addChoice("none", 0).addChoice("recent", 1).addChoice("all", 7)); + + hasRequiredRole = Pattern.compile(Config.getInstance().getHeavyModerationRolePattern()) + .asMatchPredicate(); } private static RestAction handleAlreadyBanned(@NotNull Guild.Ban ban, @@ -119,7 +126,7 @@ private static Optional> handleNotAlreadyBannedRespo } @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) - private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, + private boolean handleChecks(@NotNull Member bot, @NotNull Member author, @Nullable Member target, @NotNull CharSequence reason, @NotNull Guild guild, @NotNull Interaction event) { // Member doesn't exist if attempting to ban a user who is not part of the guild. @@ -127,7 +134,10 @@ private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, target, event)) { return false; } - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, + if (!ModerationUtils.handleHasAuthorRole(ACTION_VERB, hasRequiredRole, author, event)) { + return false; + } + if (!ModerationUtils.handleHasBotPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, guild, event)) { return false; } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 4c8d8e7ad4..3d23ba5e28 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -13,8 +13,11 @@ import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.commands.SlashCommandAdapter; import org.togetherjava.tjbot.commands.SlashCommandVisibility; +import org.togetherjava.tjbot.config.Config; import java.util.Objects; +import java.util.function.Predicate; +import java.util.regex.Pattern; /** @@ -30,6 +33,7 @@ public final class KickCommand extends SlashCommandAdapter { private static final String REASON_OPTION = "reason"; private static final String COMMAND_NAME = "kick"; private static final String ACTION_VERB = "kick"; + private final Predicate hasRequiredRole; /** * Constructs an instance. @@ -39,6 +43,9 @@ public KickCommand() { getData().addOption(OptionType.USER, TARGET_OPTION, "The user who you want to kick", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be kicked", true); + + hasRequiredRole = Pattern.compile(Config.getInstance().getSoftModerationRolePattern()) + .asMatchPredicate(); } private static void handleAbsentTarget(@NotNull Interaction event) { @@ -80,7 +87,7 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, } @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) - private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, + private boolean handleChecks(@NotNull Member bot, @NotNull Member author, @Nullable Member target, @NotNull CharSequence reason, @NotNull Guild guild, @NotNull Interaction event) { // Member doesn't exist if attempting to kick a user who is not part of the guild anymore. @@ -91,7 +98,10 @@ private static boolean handleChecks(@NotNull Member bot, @NotNull Member author, if (!ModerationUtils.handleCanInteractWithTarget(ACTION_VERB, bot, author, target, event)) { return false; } - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.KICK_MEMBERS, bot, author, + if (!ModerationUtils.handleHasAuthorRole(ACTION_VERB, hasRequiredRole, author, event)) { + return false; + } + if (!ModerationUtils.handleHasBotPermissions(ACTION_VERB, Permission.KICK_MEMBERS, bot, guild, event)) { return false; } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index 6d4977c39a..15cf97cd46 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -11,6 +11,7 @@ import java.awt.*; import java.time.Instant; +import java.util.function.Predicate; /** * Utility class offering helpers revolving around user moderation, such as banning or kicking. @@ -85,31 +86,21 @@ static boolean handleCanInteractWithTarget(@NotNull String actionVerb, @NotNull } /** - * Checks whether the given author and bot have enough permission to execute the given action. - * For example whether they have enough permissions to ban users. + * Checks whether the given bot has enough permission to execute the given action. For example + * whether it has enough permissions to ban users. *

* If not, it will handle the situation and respond to the user. * * @param actionVerb the interaction as verb, for example {@code "ban"} or {@code "kick"} * @param permission the required permission to check * @param bot the bot attempting to interact with the user - * @param author the author triggering the command * @param event the event used to respond to the user - * @return Whether the author and bot have the required permission + * @return Whether the bot has the required permission */ @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) - static boolean handleHasPermissions(@NotNull String actionVerb, @NotNull Permission permission, - @NotNull IPermissionHolder bot, @NotNull IPermissionHolder author, @NotNull Guild guild, + static boolean handleHasBotPermissions(@NotNull String actionVerb, + @NotNull Permission permission, @NotNull IPermissionHolder bot, @NotNull Guild guild, @NotNull Interaction event) { - if (!author.hasPermission(permission)) { - event - .reply("You can not %s users in this guild since you do not have the %s permission." - .formatted(actionVerb, permission)) - .setEphemeral(true) - .queue(); - return false; - } - if (!bot.hasPermission(permission)) { event .reply("I can not %s users in this guild since I do not have the %s permission." @@ -124,6 +115,33 @@ static boolean handleHasPermissions(@NotNull String actionVerb, @NotNull Permiss return true; } + /** + * Checks whether the given bot has enough permission to execute the given action. For example + * whether it has enough permissions to ban users. + *

+ * If not, it will handle the situation and respond to the user. + * + * @param actionVerb the interaction as verb, for example {@code "ban"} or {@code "kick"} + * @param hasRequiredRole a predicate used to identify required roles by their name + * @param author the author attempting to interact with the target + * @param event the event used to respond to the user + * @return Whether the bot has the required permission + */ + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") + static boolean handleHasAuthorRole(@NotNull String actionVerb, + @NotNull Predicate hasRequiredRole, @NotNull Member author, + @NotNull Interaction event) { + if (author.getRoles().stream().map(Role::getName).anyMatch(hasRequiredRole)) { + return true; + } + event + .reply("You can not %s users in this guild since you do not have the required role." + .formatted(actionVerb)) + .setEphemeral(true) + .queue(); + return false; + } + /** * Creates a message to be displayed as response to a moderation action. * diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 821118e148..ec88b43315 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -12,8 +12,11 @@ import org.slf4j.LoggerFactory; import org.togetherjava.tjbot.commands.SlashCommandAdapter; import org.togetherjava.tjbot.commands.SlashCommandVisibility; +import org.togetherjava.tjbot.config.Config; import java.util.Objects; +import java.util.function.Predicate; +import java.util.regex.Pattern; /** * Unbans a given user. Unbanning can also be paired with a reason. The command fails if the user is @@ -25,6 +28,7 @@ public final class UnbanCommand extends SlashCommandAdapter { private static final String REASON_OPTION = "reason"; private static final String COMMAND_NAME = "unban"; private static final String ACTION_VERB = "unban"; + private final Predicate hasRequiredRole; /** * Constructs an instance. @@ -36,6 +40,9 @@ public UnbanCommand() { .addOption(OptionType.USER, TARGET_OPTION, "The banned user who you want to unban", true) .addOption(OptionType.STRING, REASON_OPTION, "Why the user should be unbanned", true); + + hasRequiredRole = Pattern.compile(Config.getInstance().getHeavyModerationRolePattern()) + .asMatchPredicate(); } private static void unban(@NotNull User target, @NotNull Member author, @NotNull String reason, @@ -72,10 +79,12 @@ private static void unban(@NotNull User target, @NotNull Member author, @NotNull } @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion"}) - private static boolean handleChecks(@NotNull IPermissionHolder bot, - @NotNull IPermissionHolder author, @NotNull CharSequence reason, @NotNull Guild guild, - @NotNull Interaction event) { - if (!ModerationUtils.handleHasPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, author, + private boolean handleChecks(@NotNull IPermissionHolder bot, @NotNull Member author, + @NotNull CharSequence reason, @NotNull Guild guild, @NotNull Interaction event) { + if (!ModerationUtils.handleHasAuthorRole(ACTION_VERB, hasRequiredRole, author, event)) { + return false; + } + if (!ModerationUtils.handleHasBotPermissions(ACTION_VERB, Permission.BAN_MEMBERS, bot, guild, event)) { return false; } diff --git a/application/src/main/java/org/togetherjava/tjbot/config/Config.java b/application/src/main/java/org/togetherjava/tjbot/config/Config.java index 90ed73f9bf..3306915aef 100644 --- a/application/src/main/java/org/togetherjava/tjbot/config/Config.java +++ b/application/src/main/java/org/togetherjava/tjbot/config/Config.java @@ -25,20 +25,27 @@ public final class Config { private final String discordGuildInvite; private final String modAuditLogChannelPattern; private final String mutedRolePattern; + private final String heavyModerationRolePattern; + private final String softModerationRolePattern; + @SuppressWarnings("ConstructorWithTooManyParameters") @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) private Config(@JsonProperty("token") String token, @JsonProperty("databasePath") String databasePath, @JsonProperty("projectWebsite") String projectWebsite, @JsonProperty("discordGuildInvite") String discordGuildInvite, @JsonProperty("modAuditLogChannelPattern") String modAuditLogChannelPattern, - @JsonProperty("mutedRolePattern") String mutedRolePattern) { + @JsonProperty("mutedRolePattern") String mutedRolePattern, + @JsonProperty("heavyModerationRolePattern") String heavyModerationRolePattern, + @JsonProperty("softModerationRolePattern") String softModerationRolePattern) { this.token = token; this.databasePath = databasePath; this.projectWebsite = projectWebsite; this.discordGuildInvite = discordGuildInvite; this.modAuditLogChannelPattern = modAuditLogChannelPattern; this.mutedRolePattern = mutedRolePattern; + this.heavyModerationRolePattern = heavyModerationRolePattern; + this.softModerationRolePattern = softModerationRolePattern; } /** @@ -119,4 +126,24 @@ public String getProjectWebsite() { public String getDiscordGuildInvite() { return discordGuildInvite; } + + /** + * Gets the REGEX pattern used to identify roles that are allowed to use heavy moderation + * commands, such as banning, based on role names. + * + * @return the REGEX pattern + */ + public String getHeavyModerationRolePattern() { + return heavyModerationRolePattern; + } + + /** + * Gets the REGEX pattern used to identify roles that are allowed to use soft moderation + * commands, such as kicking, muting or message deletion, based on role names. + * + * @return the REGEX pattern + */ + public String getSoftModerationRolePattern() { + return softModerationRolePattern; + } } From 5fec89f3132589e7037ca635d4427a938bf91121 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Fri, 12 Nov 2021 09:37:07 +0100 Subject: [PATCH 16/21] Added back permission based system, additionally as secondary layer --- .../tjbot/commands/moderation/BanCommand.java | 4 +++ .../commands/moderation/KickCommand.java | 4 +++ .../commands/moderation/ModerationUtils.java | 29 ++++++++++++++++++- .../commands/moderation/UnbanCommand.java | 4 +++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index ef4d472bae..de5f519251 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -141,6 +141,10 @@ private boolean handleChecks(@NotNull Member bot, @NotNull Member author, guild, event)) { return false; } + if (!ModerationUtils.handleHasAuthorPermissions(ACTION_VERB, Permission.BAN_MEMBERS, author, + guild, event)) { + return false; + } return ModerationUtils.handleReason(reason, event); } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 3d23ba5e28..2d07867fba 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -105,6 +105,10 @@ private boolean handleChecks(@NotNull Member bot, @NotNull Member author, guild, event)) { return false; } + if (!ModerationUtils.handleHasAuthorPermissions(ACTION_VERB, Permission.KICK_MEMBERS, + author, guild, event)) { + return false; + } return ModerationUtils.handleReason(reason, event); } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java index 15cf97cd46..4070c3dfa5 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/ModerationUtils.java @@ -97,7 +97,7 @@ static boolean handleCanInteractWithTarget(@NotNull String actionVerb, @NotNull * @param event the event used to respond to the user * @return Whether the bot has the required permission */ - @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") static boolean handleHasBotPermissions(@NotNull String actionVerb, @NotNull Permission permission, @NotNull IPermissionHolder bot, @NotNull Guild guild, @NotNull Interaction event) { @@ -115,6 +115,33 @@ static boolean handleHasBotPermissions(@NotNull String actionVerb, return true; } + /** + * Checks whether the given author has enough permission to execute the given action. For + * example whether they have enough permissions to ban users. + *

+ * If not, it will handle the situation and respond to the user. + * + * @param actionVerb the interaction as verb, for example {@code "ban"} or {@code "kick"} + * @param permission the required permission to check + * @param author the author attempting to interact with the target user + * @param event the event used to respond to the user + * @return Whether the author has the required permission + */ + @SuppressWarnings("BooleanMethodNameMustStartWithQuestion") + static boolean handleHasAuthorPermissions(@NotNull String actionVerb, + @NotNull Permission permission, @NotNull IPermissionHolder author, @NotNull Guild guild, + @NotNull Interaction event) { + if (!author.hasPermission(permission)) { + event + .reply("You can not %s users in this guild since you do not have the %s permission." + .formatted(actionVerb, permission)) + .setEphemeral(true) + .queue(); + return false; + } + return true; + } + /** * Checks whether the given bot has enough permission to execute the given action. For example * whether it has enough permissions to ban users. diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index ec88b43315..347027e928 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -88,6 +88,10 @@ private boolean handleChecks(@NotNull IPermissionHolder bot, @NotNull Member aut guild, event)) { return false; } + if (!ModerationUtils.handleHasAuthorPermissions(ACTION_VERB, Permission.BAN_MEMBERS, author, + guild, event)) { + return false; + } return ModerationUtils.handleReason(reason, event); } From ecad42d3e411b085f6c5fdcecebef6ba169dcbde Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 15 Nov 2021 10:26:01 +0100 Subject: [PATCH 17/21] Spotless after rebase --- .../main/java/org/togetherjava/tjbot/commands/Commands.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java index 16fe3b702f..4ddddd9d94 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java @@ -49,9 +49,9 @@ public enum Commands { commands.add(new TagManageCommand(tagSystem)); commands.add(new TagsCommand(tagSystem)); commands.add(new VcActivityCommand()); - commands.add(new KickCommand()); - commands.add(new BanCommand()); - commands.add(new UnbanCommand()); + commands.add(new KickCommand()); + commands.add(new BanCommand()); + commands.add(new UnbanCommand()); return commands; } From 0460a19e8f3deeadf34f2ff065fe504f4a727e22 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 15 Nov 2021 10:44:49 +0100 Subject: [PATCH 18/21] empty line for readability (CR Tais) --- .../org/togetherjava/tjbot/commands/moderation/UnbanCommand.java | 1 + 1 file changed, 1 insertion(+) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index 347027e928..e4c2b40fdd 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -92,6 +92,7 @@ private boolean handleChecks(@NotNull IPermissionHolder bot, @NotNull Member aut guild, event)) { return false; } + return ModerationUtils.handleReason(reason, event); } From c71ff666f381cd4c49ec2a968486f4a4fdfc600f Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 15 Nov 2021 10:55:29 +0100 Subject: [PATCH 19/21] Simplified ban/kick flow using individual methods (CR Tais) --- .../tjbot/commands/moderation/BanCommand.java | 63 +++++++++++------- .../commands/moderation/KickCommand.java | 64 +++++++++++-------- 2 files changed, 78 insertions(+), 49 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java index de5f519251..427eb63ee4 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java @@ -1,9 +1,8 @@ package org.togetherjava.tjbot.commands.moderation; import net.dv8tion.jda.api.Permission; -import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.Member; -import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.entities.*; +import net.dv8tion.jda.api.events.GenericEvent; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.exceptions.ErrorResponseException; import net.dv8tion.jda.api.interactions.Interaction; @@ -13,6 +12,8 @@ import net.dv8tion.jda.api.interactions.commands.build.OptionData; import net.dv8tion.jda.api.requests.ErrorResponse; import net.dv8tion.jda.api.requests.RestAction; +import net.dv8tion.jda.api.requests.restaction.AuditableRestAction; +import net.dv8tion.jda.api.utils.Result; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -71,11 +72,20 @@ private static RestAction handleAlreadyBanned(@NotNull Guild.Ba } @SuppressWarnings("MethodWithTooManyParameters") - private static RestAction banUser(@NotNull User target, @NotNull Member author, - @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild, - @NotNull SlashCommandEvent event) { + private static RestAction banUserFlow(@NotNull User target, + @NotNull Member author, @NotNull String reason, int deleteHistoryDays, + @NotNull Guild guild, @NotNull SlashCommandEvent event) { + return sendDm(target, reason, guild, event) + .flatMap(hasSentDm -> banUser(target, author, reason, deleteHistoryDays, guild) + .map(banResult -> hasSentDm)) + .map(hasSentDm -> sendFeedback(hasSentDm, target, author, reason)) + .flatMap(event::replyEmbeds); + } + + private static RestAction sendDm(@NotNull ISnowflake target, @NotNull String reason, + @NotNull Guild guild, @NotNull GenericEvent event) { return event.getJDA() - .openPrivateChannelById(target.getIdLong()) + .openPrivateChannelById(target.getId()) .flatMap(channel -> channel.sendMessage( """ Hey there, sorry to tell you but unfortunately you have been banned from the server %s. @@ -84,22 +94,27 @@ private static RestAction banUser(@NotNull User target, @NotNul """ .formatted(guild.getName(), reason))) .mapToResult() - .flatMap(sendDmResult -> { - logger.info( - "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'.", - author.getUser().getAsTag(), author.getId(), target.getAsTag(), - target.getId(), guild.getName(), deleteHistoryDays, reason); - - return guild.ban(target, deleteHistoryDays, reason) - .map(banResult -> sendDmResult.isSuccess()); - }) - .map(hasSentDm -> { - String dmNotice = - Boolean.TRUE.equals(hasSentDm) ? "" : "(Unable to send them a DM.)"; - return ModerationUtils.createActionResponse(author.getUser(), - ModerationUtils.Action.BAN, target, dmNotice, reason); - }) - .flatMap(event::replyEmbeds); + .map(Result::isSuccess); + } + + private static AuditableRestAction banUser(@NotNull User target, @NotNull Member author, + @NotNull String reason, int deleteHistoryDays, @NotNull Guild guild) { + logger.info( + "'{}' ({}) banned the user '{}' ({}) from guild '{}' and deleted their message history of the last {} days, for reason '{}'.", + author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), + guild.getName(), deleteHistoryDays, reason); + + return guild.ban(target, deleteHistoryDays, reason); + } + + private static @NotNull MessageEmbed sendFeedback(boolean hasSentDm, @NotNull User target, + @NotNull Member author, @NotNull String reason) { + String dmNoticeText = ""; + if (!hasSentDm) { + dmNoticeText = "(Unable to send them a DM.)"; + } + return ModerationUtils.createActionResponse(author.getUser(), ModerationUtils.Action.BAN, + target, dmNoticeText, reason); } private static Optional> handleNotAlreadyBannedResponse( @@ -175,7 +190,7 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { return handleNotAlreadyBannedResponse(Objects .requireNonNull(alreadyBanned.getFailure()), event, guild, target).orElseGet( - () -> banUser(target, author, reason, deleteHistoryDays, guild, event)); + () -> banUserFlow(target, author, reason, deleteHistoryDays, guild, event)); }).queue(); } } diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java index 2d07867fba..27f81c3501 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java @@ -1,12 +1,14 @@ package org.togetherjava.tjbot.commands.moderation; import net.dv8tion.jda.api.Permission; -import net.dv8tion.jda.api.entities.Guild; -import net.dv8tion.jda.api.entities.Member; -import net.dv8tion.jda.api.entities.User; +import net.dv8tion.jda.api.entities.*; +import net.dv8tion.jda.api.events.GenericEvent; import net.dv8tion.jda.api.events.interaction.SlashCommandEvent; import net.dv8tion.jda.api.interactions.Interaction; import net.dv8tion.jda.api.interactions.commands.OptionType; +import net.dv8tion.jda.api.requests.RestAction; +import net.dv8tion.jda.api.requests.restaction.AuditableRestAction; +import net.dv8tion.jda.api.utils.Result; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -54,11 +56,20 @@ private static void handleAbsentTarget(@NotNull Interaction event) { .queue(); } - private static void kickUser(@NotNull Member target, @NotNull Member author, + private static void kickUserFlow(@NotNull Member target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull SlashCommandEvent event) { - User targetUser = target.getUser(); - event.getJDA() - .openPrivateChannelById(targetUser.getId()) + sendDm(target, reason, guild, event) + .flatMap(hasSentDm -> kickUser(target, author, reason, guild) + .map(kickResult -> hasSentDm)) + .map(hasSentDm -> sendFeedback(hasSentDm, target, author, reason)) + .flatMap(event::replyEmbeds) + .queue(); + } + + private static RestAction sendDm(@NotNull ISnowflake target, @NotNull String reason, + @NotNull Guild guild, @NotNull GenericEvent event) { + return event.getJDA() + .openPrivateChannelById(target.getId()) .flatMap(channel -> channel.sendMessage( """ Hey there, sorry to tell you but unfortunately you have been kicked from the server %s. @@ -67,23 +78,26 @@ private static void kickUser(@NotNull Member target, @NotNull Member author, """ .formatted(guild.getName(), reason))) .mapToResult() - .flatMap(sendDmResult -> { - logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'.", - author.getUser().getAsTag(), author.getId(), targetUser.getAsTag(), - targetUser.getId(), guild.getName(), reason); - - return guild.kick(target, reason) - .reason(reason) - .map(kickResult -> sendDmResult.isSuccess()); - }) - .map(hasSentDm -> { - String dmNotice = - Boolean.TRUE.equals(hasSentDm) ? "" : "(Unable to send them a DM.)"; - return ModerationUtils.createActionResponse(author.getUser(), - ModerationUtils.Action.KICK, targetUser, dmNotice, reason); - }) - .flatMap(event::replyEmbeds) - .queue(); + .map(Result::isSuccess); + } + + private static AuditableRestAction kickUser(@NotNull Member target, + @NotNull Member author, @NotNull String reason, @NotNull Guild guild) { + logger.info("'{}' ({}) kicked the user '{}' ({}) from guild '{}' for reason '{}'.", + author.getUser().getAsTag(), author.getId(), target.getUser().getAsTag(), + target.getId(), guild.getName(), reason); + + return guild.kick(target, reason).reason(reason); + } + + private static @NotNull MessageEmbed sendFeedback(boolean hasSentDm, @NotNull Member target, + @NotNull Member author, @NotNull String reason) { + String dmNoticeText = ""; + if (!hasSentDm) { + dmNoticeText = "(Unable to send them a DM.)"; + } + return ModerationUtils.createActionResponse(author.getUser(), ModerationUtils.Action.KICK, + target.getUser(), dmNoticeText, reason); } @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion", "MethodWithTooManyParameters"}) @@ -126,6 +140,6 @@ public void onSlashCommand(@NotNull SlashCommandEvent event) { if (!handleChecks(bot, author, target, reason, guild, event)) { return; } - kickUser(Objects.requireNonNull(target), author, reason, guild, event); + kickUserFlow(Objects.requireNonNull(target), author, reason, guild, event); } } From 8b48e47a8ad2f6bdfca4324ab54723fa42e06cf8 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 15 Nov 2021 10:55:46 +0100 Subject: [PATCH 20/21] Moved failure handling for unban into method (CR Tais) --- .../commands/moderation/UnbanCommand.java | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java index e4c2b40fdd..a8908e20fe 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java @@ -47,35 +47,38 @@ public UnbanCommand() { private static void unban(@NotNull User target, @NotNull Member author, @NotNull String reason, @NotNull Guild guild, @NotNull Interaction event) { - String targetTag = target.getAsTag(); - guild.unban(target).reason(reason).queue(result -> { MessageEmbed message = ModerationUtils.createActionResponse(author.getUser(), ModerationUtils.Action.UNBAN, target, null, reason); event.replyEmbeds(message).queue(); logger.info("'{}' ({}) unbanned the user '{}' ({}) from guild '{}' for reason '{}'.", - author.getUser().getAsTag(), author.getId(), targetTag, target.getId(), + author.getUser().getAsTag(), author.getId(), target.getAsTag(), target.getId(), guild.getName(), reason); - }, unbanFailure -> { - if (unbanFailure instanceof ErrorResponseException errorResponseException) { - if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { - event.reply("The specified user does not exist.").setEphemeral(true).queue(); - logger.debug("Unable to unban the user '{}' because they do not exist.", - targetTag); - return; - } - if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_BAN) { - event.reply("The specified user is not banned.").setEphemeral(true).queue(); - logger.debug("Unable to unban the user '{}' because they are not banned.", - targetTag); - return; - } + }, unbanFailure -> handleFailure(unbanFailure, target, event)); + } + + private static void handleFailure(@NotNull Throwable unbanFailure, @NotNull User target, + @NotNull Interaction event) { + String targetTag = target.getAsTag(); + if (unbanFailure instanceof ErrorResponseException errorResponseException) { + if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_USER) { + event.reply("The specified user does not exist.").setEphemeral(true).queue(); + logger.debug("Unable to unban the user '{}' because they do not exist.", targetTag); + return; } - event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); - logger.warn("Something unexpected went wrong while trying to unban the user '{}'.", - targetTag, unbanFailure); - }); + + if (errorResponseException.getErrorResponse() == ErrorResponse.UNKNOWN_BAN) { + event.reply("The specified user is not banned.").setEphemeral(true).queue(); + logger.debug("Unable to unban the user '{}' because they are not banned.", + targetTag); + return; + } + } + + event.reply("Sorry, but something went wrong.").setEphemeral(true).queue(); + logger.warn("Something unexpected went wrong while trying to unban the user '{}'.", + targetTag, unbanFailure); } @SuppressWarnings({"BooleanMethodNameMustStartWithQuestion"}) From 60190a505c7420a3b4ec74392e9ff5449ca1abd9 Mon Sep 17 00:00:00 2001 From: Zabuzard Date: Mon, 15 Nov 2021 10:58:57 +0100 Subject: [PATCH 21/21] Missing imports after rebase --- .../main/java/org/togetherjava/tjbot/commands/Commands.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java index 4ddddd9d94..de704a1f7a 100644 --- a/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java +++ b/application/src/main/java/org/togetherjava/tjbot/commands/Commands.java @@ -5,6 +5,9 @@ import org.togetherjava.tjbot.commands.basic.PingCommand; import org.togetherjava.tjbot.commands.basic.VcActivityCommand; import org.togetherjava.tjbot.commands.mathcommands.TeXCommand; +import org.togetherjava.tjbot.commands.moderation.BanCommand; +import org.togetherjava.tjbot.commands.moderation.KickCommand; +import org.togetherjava.tjbot.commands.moderation.UnbanCommand; import org.togetherjava.tjbot.commands.tags.TagCommand; import org.togetherjava.tjbot.commands.tags.TagManageCommand; import org.togetherjava.tjbot.commands.tags.TagSystem;