From 93d16d273f66442a8db0aa0f7d355e4a0852ad6b Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 7 Dec 2018 12:24:57 +0100 Subject: [PATCH 1/4] Start working on sensitive commands [WIP] Test doesn't compile, needs feedback. #1703 related --- src/main/java/fr/xephi/authme/AuthMe.java | 4 +- .../authme/command/CommandDescription.java | 26 +++++++++- .../authme/command/CommandInitializer.java | 9 ++++ .../authme/initialization/OnStartupTasks.java | 11 ++-- .../fr/xephi/authme/output/ConsoleFilter.java | 8 ++- .../fr/xephi/authme/output/Log4JFilter.java | 12 +++-- .../xephi/authme/output/LogFilterHelper.java | 50 ------------------- .../xephi/authme/output/LogFilterService.java | 41 +++++++++++++++ .../authme/output/LogFilterHelperTest.java | 6 +-- 9 files changed, 102 insertions(+), 65 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/output/LogFilterHelper.java create mode 100644 src/main/java/fr/xephi/authme/output/LogFilterService.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 2f3035d7b..0a8a55e89 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -211,7 +211,8 @@ private void initialize() { // Get settings and set up logger settings = injector.getSingleton(Settings.class); ConsoleLogger.setLoggingOptions(settings); - OnStartupTasks.setupConsoleFilter(settings, getLogger()); + OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class); + onStartupTasks.setupConsoleFilter(settings, getLogger()); // Set all service fields on the AuthMe class instantiateServices(injector); @@ -229,7 +230,6 @@ private void initialize() { registerEventListeners(injector); // Start Email recall task if needed - OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class); onStartupTasks.scheduleRecallEmailTask(); } diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index e195c4752..72d78de8e 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -55,6 +55,10 @@ public class CommandDescription { * Permission node required to execute this command. */ private PermissionNode permission; + /** + * Defines if the command contains sensitive data + */ + private boolean sensitive; /** * Private constructor. @@ -72,7 +76,8 @@ public class CommandDescription { */ private CommandDescription(List labels, String description, String detailedDescription, Class executableCommand, CommandDescription parent, - List arguments, PermissionNode permission) { + List arguments, PermissionNode permission, + boolean sensitive) { this.labels = labels; this.description = description; this.detailedDescription = detailedDescription; @@ -80,6 +85,7 @@ private CommandDescription(List labels, String description, String detai this.parent = parent; this.arguments = arguments; this.permission = permission; + this.sensitive = sensitive; } /** @@ -184,6 +190,10 @@ public PermissionNode getPermission() { return permission; } + public boolean isSensitive() { + return sensitive; + } + /** * Return a builder instance to create a new command description. * @@ -204,6 +214,7 @@ public static final class CommandBuilder { private CommandDescription parent; private List arguments = new ArrayList<>(); private PermissionNode permission; + private Boolean sensitive; /** * Build a CommandDescription and register it onto the parent if available. @@ -232,7 +243,7 @@ public CommandDescription build() { // parents and permissions may be null; arguments may be empty return new CommandDescription(labels, description, detailedDescription, executableCommand, - parent, arguments, permission); + parent, arguments, permission, sensitive == null ? false : sensitive); } public CommandBuilder labels(List labels) { @@ -289,6 +300,17 @@ public CommandBuilder permission(PermissionNode permission) { this.permission = permission; return this; } + + /** + * Defines if the command contains sensitive data + * + * @param sensitive The sensitive data flag + * @return The builder + */ + public CommandBuilder sensitive(boolean sensitive) { + this.sensitive = sensitive; + return this; + } } } diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index ba48e0112..5ebb10d73 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -94,6 +94,7 @@ private void buildCommands() { .detailedDescription("Command to log in using AuthMeReloaded.") .withArgument("password", "Login password", MANDATORY) .permission(PlayerPermission.LOGIN) + .sensitive(true) .executableCommand(LoginCommand.class) .register(); @@ -116,6 +117,7 @@ private void buildCommands() { .withArgument("password", "Password", OPTIONAL) .withArgument("verifyPassword", "Verify password", OPTIONAL) .permission(PlayerPermission.REGISTER) + .sensitive(true) .executableCommand(RegisterCommand.class) .register(); @@ -139,6 +141,7 @@ private void buildCommands() { .withArgument("oldPassword", "Old password", MANDATORY) .withArgument("newPassword", "New password", MANDATORY) .permission(PlayerPermission.CHANGE_PASSWORD) + .sensitive(true) .executableCommand(ChangePasswordCommand.class) .register(); @@ -197,6 +200,7 @@ private CommandDescription buildAuthMeBaseCommand() { .withArgument("player", "Player name", MANDATORY) .withArgument("password", "Password", MANDATORY) .permission(AdminPermission.REGISTER) + .sensitive(true) .executableCommand(RegisterAdminCommand.class) .register(); @@ -231,6 +235,7 @@ private CommandDescription buildAuthMeBaseCommand() { .withArgument("player", "Player name", MANDATORY) .withArgument("pwd", "New password", MANDATORY) .permission(AdminPermission.CHANGE_PASSWORD) + .sensitive(true) .executableCommand(ChangePasswordAdminCommand.class) .register(); @@ -540,6 +545,7 @@ private CommandDescription buildEmailBaseCommand() { .detailedDescription("Set a new password after successfully recovering your account.") .withArgument("password", "New password", MANDATORY) .permission(PlayerPermission.RECOVER_EMAIL) + .sensitive(true) .executableCommand(EmailSetPasswordCommand.class) .register(); @@ -568,6 +574,7 @@ private CommandDescription buildTotpBaseCommand() { .description("Command for logging in") .detailedDescription("Processes the two-factor authentication code during login.") .withArgument("code", "The TOTP code to use to log in", MANDATORY) + .sensitive(true) .executableCommand(TotpCodeCommand.class) .register(); @@ -589,6 +596,7 @@ private CommandDescription buildTotpBaseCommand() { .detailedDescription("Saves the generated TOTP secret after confirmation.") .withArgument("code", "Code from the given secret from /totp add", MANDATORY) .permission(PlayerPermission.ENABLE_TWO_FACTOR_AUTH) + .sensitive(true) .executableCommand(ConfirmTotpCommand.class) .register(); @@ -600,6 +608,7 @@ private CommandDescription buildTotpBaseCommand() { .detailedDescription("Disables two-factor authentication for your account.") .withArgument("code", "Current 2FA code", MANDATORY) .permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH) + .sensitive(true) .executableCommand(RemoveTotpCommand.class) .register(); diff --git a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java index 498e5c266..26d5dd342 100644 --- a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java +++ b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java @@ -7,6 +7,7 @@ import fr.xephi.authme.message.Messages; import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.Log4JFilter; +import fr.xephi.authme.output.LogFilterService; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; @@ -36,6 +37,8 @@ public class OnStartupTasks { private BukkitService bukkitService; @Inject private Messages messages; + @Inject + private LogFilterService logFilterService; OnStartupTasks() { } @@ -61,7 +64,7 @@ public static void sendMetrics(AuthMe plugin, Settings settings) { * @param settings the settings * @param logger the plugin logger */ - public static void setupConsoleFilter(Settings settings, Logger logger) { + public void setupConsoleFilter(Settings settings, Logger logger) { // Try to set the log4j filter try { Class.forName("org.apache.logging.log4j.core.filter.AbstractFilter"); @@ -69,7 +72,7 @@ public static void setupConsoleFilter(Settings settings, Logger logger) { } catch (ClassNotFoundException | NoClassDefFoundError e) { // log4j is not available ConsoleLogger.info("You're using Minecraft 1.6.x or older, Log4J support will be disabled"); - ConsoleFilter filter = new ConsoleFilter(); + ConsoleFilter filter = new ConsoleFilter(logFilterService); logger.setFilter(filter); Bukkit.getLogger().setFilter(filter); Logger.getLogger("Minecraft").setFilter(filter); @@ -77,10 +80,10 @@ public static void setupConsoleFilter(Settings settings, Logger logger) { } // Set the console filter to remove the passwords - private static void setLog4JFilter() { + private void setLog4JFilter() { org.apache.logging.log4j.core.Logger logger; logger = (org.apache.logging.log4j.core.Logger) LogManager.getRootLogger(); - logger.addFilter(new Log4JFilter()); + logger.addFilter(new Log4JFilter(logFilterService)); } /** diff --git a/src/main/java/fr/xephi/authme/output/ConsoleFilter.java b/src/main/java/fr/xephi/authme/output/ConsoleFilter.java index 975cc4cc3..8de1744e9 100644 --- a/src/main/java/fr/xephi/authme/output/ConsoleFilter.java +++ b/src/main/java/fr/xephi/authme/output/ConsoleFilter.java @@ -10,13 +10,19 @@ */ public class ConsoleFilter implements Filter { + private LogFilterService filterService; + + public ConsoleFilter(LogFilterService filterService) { + this.filterService = filterService; + } + @Override public boolean isLoggable(LogRecord record) { if (record == null || record.getMessage() == null) { return true; } - if (LogFilterHelper.isSensitiveAuthMeCommand(record.getMessage())) { + if (filterService.isSensitiveAuthMeCommand(record.getMessage())) { String playerName = record.getMessage().split(" ")[0]; record.setMessage(playerName + " issued an AuthMe command"); } diff --git a/src/main/java/fr/xephi/authme/output/Log4JFilter.java b/src/main/java/fr/xephi/authme/output/Log4JFilter.java index 1ebf5141f..09fff8191 100644 --- a/src/main/java/fr/xephi/authme/output/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/output/Log4JFilter.java @@ -16,6 +16,12 @@ public class Log4JFilter extends AbstractFilter { private static final long serialVersionUID = -5594073755007974254L; + private LogFilterService filterService; + + public Log4JFilter(LogFilterService filterService) { + this.filterService = filterService; + } + /** * Validates a Message instance and returns the {@link Result} value * depending on whether the message contains sensitive AuthMe data. @@ -24,7 +30,7 @@ public class Log4JFilter extends AbstractFilter { * * @return The Result value */ - private static Result validateMessage(Message message) { + private Result validateMessage(Message message) { if (message == null) { return Result.NEUTRAL; } @@ -39,8 +45,8 @@ private static Result validateMessage(Message message) { * * @return The Result value */ - private static Result validateMessage(String message) { - return LogFilterHelper.isSensitiveAuthMeCommand(message) + private Result validateMessage(String message) { + return filterService.isSensitiveAuthMeCommand(message) ? Result.DENY : Result.NEUTRAL; } diff --git a/src/main/java/fr/xephi/authme/output/LogFilterHelper.java b/src/main/java/fr/xephi/authme/output/LogFilterHelper.java deleted file mode 100644 index fa43a01c8..000000000 --- a/src/main/java/fr/xephi/authme/output/LogFilterHelper.java +++ /dev/null @@ -1,50 +0,0 @@ -package fr.xephi.authme.output; - -import com.google.common.annotations.VisibleForTesting; -import fr.xephi.authme.util.StringUtils; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -/** - * Service class for the log filters. - */ -final class LogFilterHelper { - - @VisibleForTesting - static final List COMMANDS_TO_SKIP = withAndWithoutAuthMePrefix( - "/login ", "/l ", "/log ", "/register ", "/reg ", "/unregister ", "/unreg ", - "/changepassword ", "/cp ", "/changepass ", "/authme register ", "/authme reg ", "/authme r ", - "/authme changepassword ", "/authme password ", "/authme changepass ", "/authme cp ", "/email setpassword "); - - private static final String ISSUED_COMMAND_TEXT = "issued server command:"; - - private LogFilterHelper() { - // Util class - } - - /** - * Validate a message and return whether the message contains a sensitive AuthMe command. - * - * @param message The message to verify - * - * @return True if it is a sensitive AuthMe command, false otherwise - */ - static boolean isSensitiveAuthMeCommand(String message) { - if (message == null) { - return false; - } - String lowerMessage = message.toLowerCase(); - return lowerMessage.contains(ISSUED_COMMAND_TEXT) && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP); - } - - private static List withAndWithoutAuthMePrefix(String... commands) { - List commandList = new ArrayList<>(commands.length * 2); - for (String command : commands) { - commandList.add(command); - commandList.add(command.substring(0, 1) + "authme:" + command.substring(1)); - } - return Collections.unmodifiableList(commandList); - } -} diff --git a/src/main/java/fr/xephi/authme/output/LogFilterService.java b/src/main/java/fr/xephi/authme/output/LogFilterService.java new file mode 100644 index 000000000..0acd87a9c --- /dev/null +++ b/src/main/java/fr/xephi/authme/output/LogFilterService.java @@ -0,0 +1,41 @@ +package fr.xephi.authme.output; + +import fr.xephi.authme.command.CommandMapper; +import fr.xephi.authme.command.FoundCommandResult; + +import javax.inject.Inject; +import java.util.Arrays; +import java.util.List; + +/** + * Service class for the log filters. + */ +public class LogFilterService { + + @Inject + private CommandMapper commandMapper; + + private static final String ISSUED_COMMAND_PREFIX_TEXT = "issued server command: /"; + + /** + * Validate a message and return whether the message contains a sensitive AuthMe command. + * + * @param message The message to verify + * + * @return True if it is a sensitive AuthMe command, false otherwise + */ + public boolean isSensitiveAuthMeCommand(String message) { + if (message == null || !message.contains(ISSUED_COMMAND_PREFIX_TEXT)) { + return false; + } + String rawCommand = message.substring(message.indexOf("/")); + List components = Arrays.asList(rawCommand.split(" ")); + FoundCommandResult command = commandMapper.mapPartsToCommand(null, components); + switch (command.getResultStatus()) { + case UNKNOWN_LABEL: + case MISSING_BASE_COMMAND: + return false; + } + return command.getCommandDescription().isSensitive(); + } +} diff --git a/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java b/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java index 302345fb9..8ef148ced 100644 --- a/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java +++ b/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java @@ -13,14 +13,14 @@ import static org.junit.Assert.assertThat; /** - * Test for {@link LogFilterHelper}. + * Test for {@link LogFilterService}. */ public class LogFilterHelperTest { private static final List ALL_COMMANDS = new CommandInitializer().getCommands(); /** - * Checks that {@link LogFilterHelper#COMMANDS_TO_SKIP} contains the entries we expect + * Checks that {@link LogFilterService#COMMANDS_TO_SKIP} contains the entries we expect * (commands with password argument). */ @Test @@ -39,7 +39,7 @@ public void shouldBlacklistAllSensitiveCommands() { .toArray(String[]::new); // when / then - assertThat(LogFilterHelper.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries)); + assertThat(LogFilterService.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries)); } From b919c3381962338bd20a0b38d9ab9b710691b8cf Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Fri, 7 Dec 2018 12:28:06 +0100 Subject: [PATCH 2/4] Fix codestyle --- src/main/java/fr/xephi/authme/output/LogFilterService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/fr/xephi/authme/output/LogFilterService.java b/src/main/java/fr/xephi/authme/output/LogFilterService.java index 0acd87a9c..3cf2ee268 100644 --- a/src/main/java/fr/xephi/authme/output/LogFilterService.java +++ b/src/main/java/fr/xephi/authme/output/LogFilterService.java @@ -12,11 +12,11 @@ */ public class LogFilterService { + private static final String ISSUED_COMMAND_PREFIX_TEXT = "issued server command: /"; + @Inject private CommandMapper commandMapper; - private static final String ISSUED_COMMAND_PREFIX_TEXT = "issued server command: /"; - /** * Validate a message and return whether the message contains a sensitive AuthMe command. * @@ -35,6 +35,8 @@ public boolean isSensitiveAuthMeCommand(String message) { case UNKNOWN_LABEL: case MISSING_BASE_COMMAND: return false; + default: + break; } return command.getCommandDescription().isSensitive(); } From fac3a70634ef2941b70fd1a91864f48fb996e149 Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Sun, 9 Dec 2018 19:03:51 +0100 Subject: [PATCH 3/4] Requested changes --- .../authme/command/CommandDescription.java | 7 +- .../authme/command/CommandInitializer.java | 18 ++-- .../authme/initialization/OnStartupTasks.java | 2 +- .../fr/xephi/authme/output/ConsoleFilter.java | 2 + .../fr/xephi/authme/output/Log4JFilter.java | 1 + .../{output => service}/LogFilterService.java | 5 +- .../authme/output/LogFilterHelperTest.java | 89 ------------------- 7 files changed, 18 insertions(+), 106 deletions(-) rename src/main/java/fr/xephi/authme/{output => service}/LogFilterService.java (91%) delete mode 100644 src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 72d78de8e..8177f33bb 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -302,13 +302,12 @@ public CommandBuilder permission(PermissionNode permission) { } /** - * Defines if the command contains sensitive data + * Defines that the command contains sensitive data * - * @param sensitive The sensitive data flag * @return The builder */ - public CommandBuilder sensitive(boolean sensitive) { - this.sensitive = sensitive; + public CommandBuilder sensitive() { + this.sensitive = true; return this; } } diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 5ebb10d73..ba9151fb7 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -94,7 +94,7 @@ private void buildCommands() { .detailedDescription("Command to log in using AuthMeReloaded.") .withArgument("password", "Login password", MANDATORY) .permission(PlayerPermission.LOGIN) - .sensitive(true) + .sensitive() .executableCommand(LoginCommand.class) .register(); @@ -117,7 +117,7 @@ private void buildCommands() { .withArgument("password", "Password", OPTIONAL) .withArgument("verifyPassword", "Verify password", OPTIONAL) .permission(PlayerPermission.REGISTER) - .sensitive(true) + .sensitive() .executableCommand(RegisterCommand.class) .register(); @@ -141,7 +141,7 @@ private void buildCommands() { .withArgument("oldPassword", "Old password", MANDATORY) .withArgument("newPassword", "New password", MANDATORY) .permission(PlayerPermission.CHANGE_PASSWORD) - .sensitive(true) + .sensitive() .executableCommand(ChangePasswordCommand.class) .register(); @@ -200,7 +200,7 @@ private CommandDescription buildAuthMeBaseCommand() { .withArgument("player", "Player name", MANDATORY) .withArgument("password", "Password", MANDATORY) .permission(AdminPermission.REGISTER) - .sensitive(true) + .sensitive() .executableCommand(RegisterAdminCommand.class) .register(); @@ -235,7 +235,7 @@ private CommandDescription buildAuthMeBaseCommand() { .withArgument("player", "Player name", MANDATORY) .withArgument("pwd", "New password", MANDATORY) .permission(AdminPermission.CHANGE_PASSWORD) - .sensitive(true) + .sensitive() .executableCommand(ChangePasswordAdminCommand.class) .register(); @@ -545,7 +545,7 @@ private CommandDescription buildEmailBaseCommand() { .detailedDescription("Set a new password after successfully recovering your account.") .withArgument("password", "New password", MANDATORY) .permission(PlayerPermission.RECOVER_EMAIL) - .sensitive(true) + .sensitive() .executableCommand(EmailSetPasswordCommand.class) .register(); @@ -574,7 +574,7 @@ private CommandDescription buildTotpBaseCommand() { .description("Command for logging in") .detailedDescription("Processes the two-factor authentication code during login.") .withArgument("code", "The TOTP code to use to log in", MANDATORY) - .sensitive(true) + .sensitive() .executableCommand(TotpCodeCommand.class) .register(); @@ -596,7 +596,7 @@ private CommandDescription buildTotpBaseCommand() { .detailedDescription("Saves the generated TOTP secret after confirmation.") .withArgument("code", "Code from the given secret from /totp add", MANDATORY) .permission(PlayerPermission.ENABLE_TWO_FACTOR_AUTH) - .sensitive(true) + .sensitive() .executableCommand(ConfirmTotpCommand.class) .register(); @@ -608,7 +608,7 @@ private CommandDescription buildTotpBaseCommand() { .detailedDescription("Disables two-factor authentication for your account.") .withArgument("code", "Current 2FA code", MANDATORY) .permission(PlayerPermission.DISABLE_TWO_FACTOR_AUTH) - .sensitive(true) + .sensitive() .executableCommand(RemoveTotpCommand.class) .register(); diff --git a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java index 26d5dd342..1e920c0f8 100644 --- a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java +++ b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java @@ -7,7 +7,7 @@ import fr.xephi.authme.message.Messages; import fr.xephi.authme.output.ConsoleFilter; import fr.xephi.authme.output.Log4JFilter; -import fr.xephi.authme.output.LogFilterService; +import fr.xephi.authme.service.LogFilterService; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.DatabaseSettings; diff --git a/src/main/java/fr/xephi/authme/output/ConsoleFilter.java b/src/main/java/fr/xephi/authme/output/ConsoleFilter.java index 8de1744e9..8d50e701e 100644 --- a/src/main/java/fr/xephi/authme/output/ConsoleFilter.java +++ b/src/main/java/fr/xephi/authme/output/ConsoleFilter.java @@ -1,5 +1,7 @@ package fr.xephi.authme.output; +import fr.xephi.authme.service.LogFilterService; + import java.util.logging.Filter; import java.util.logging.LogRecord; diff --git a/src/main/java/fr/xephi/authme/output/Log4JFilter.java b/src/main/java/fr/xephi/authme/output/Log4JFilter.java index 09fff8191..b7627510e 100644 --- a/src/main/java/fr/xephi/authme/output/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/output/Log4JFilter.java @@ -1,5 +1,6 @@ package fr.xephi.authme.output; +import fr.xephi.authme.service.LogFilterService; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; import org.apache.logging.log4j.core.LogEvent; diff --git a/src/main/java/fr/xephi/authme/output/LogFilterService.java b/src/main/java/fr/xephi/authme/service/LogFilterService.java similarity index 91% rename from src/main/java/fr/xephi/authme/output/LogFilterService.java rename to src/main/java/fr/xephi/authme/service/LogFilterService.java index 3cf2ee268..b44a9c5f1 100644 --- a/src/main/java/fr/xephi/authme/output/LogFilterService.java +++ b/src/main/java/fr/xephi/authme/service/LogFilterService.java @@ -1,4 +1,4 @@ -package fr.xephi.authme.output; +package fr.xephi.authme.service; import fr.xephi.authme.command.CommandMapper; import fr.xephi.authme.command.FoundCommandResult; @@ -36,8 +36,7 @@ public boolean isSensitiveAuthMeCommand(String message) { case MISSING_BASE_COMMAND: return false; default: - break; + return command.getCommandDescription().isSensitive(); } - return command.getCommandDescription().isSensitive(); } } diff --git a/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java b/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java deleted file mode 100644 index 8ef148ced..000000000 --- a/src/test/java/fr/xephi/authme/output/LogFilterHelperTest.java +++ /dev/null @@ -1,89 +0,0 @@ -package fr.xephi.authme.output; - -import com.google.common.collect.Lists; -import fr.xephi.authme.command.CommandDescription; -import fr.xephi.authme.command.CommandInitializer; -import org.junit.Test; - -import java.util.Arrays; -import java.util.List; -import java.util.stream.Collectors; - -import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.junit.Assert.assertThat; - -/** - * Test for {@link LogFilterService}. - */ -public class LogFilterHelperTest { - - private static final List ALL_COMMANDS = new CommandInitializer().getCommands(); - - /** - * Checks that {@link LogFilterService#COMMANDS_TO_SKIP} contains the entries we expect - * (commands with password argument). - */ - @Test - public void shouldBlacklistAllSensitiveCommands() { - // given - List sensitiveCommands = Arrays.asList( - getCommand("register"), getCommand("login"), getCommand("changepassword"), getCommand("unregister"), - getCommand("authme", "register"), getCommand("authme", "changepassword"), - getCommand("email", "setpassword") - ); - // Build array with entries like "/register ", "/authme cp ", "/authme changepass " - String[] expectedEntries = sensitiveCommands.stream() - .map(cmd -> buildCommandSyntaxes(cmd)) - .flatMap(List::stream) - .map(syntax -> syntax + " ") - .toArray(String[]::new); - - // when / then - assertThat(LogFilterService.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries)); - - } - - private static CommandDescription getCommand(String label) { - return findCommandWithLabel(label, ALL_COMMANDS); - } - - private static CommandDescription getCommand(String parentLabel, String childLabel) { - CommandDescription parent = getCommand(parentLabel); - return findCommandWithLabel(childLabel, parent.getChildren()); - } - - private static CommandDescription findCommandWithLabel(String label, List commands) { - return commands.stream() - .filter(cmd -> cmd.getLabels().contains(label)) - .findFirst().orElseThrow(() -> new IllegalArgumentException(label)); - } - - /** - * Returns all "command syntaxes" from which the given command can be reached. - * For example, the result might be a List containing "/authme changepassword", "/authme changepass", - * "/authme cp", "/authme:authme changepassword" etc. - * - * @param command the command to build syntaxes for - * @return command syntaxes - */ - private static List buildCommandSyntaxes(CommandDescription command) { - List prefixes = getCommandPrefixes(command); - - return command.getLabels() - .stream() - .map(label -> Lists.transform(prefixes, p -> p + label)) - .flatMap(List::stream) - .collect(Collectors.toList()); - } - - private static List getCommandPrefixes(CommandDescription command) { - if (command.getParent() == null) { - return Arrays.asList("/", "/authme:"); - } - return command.getParent().getLabels() - .stream() - .map(label -> new String[]{"/" + label + " ", "/authme:" + label + " "}) - .flatMap(Arrays::stream) - .collect(Collectors.toList()); - } -} From 96bea07ad260b45c4da8b91d8302d40c94ff537c Mon Sep 17 00:00:00 2001 From: Gabriele C Date: Thu, 8 Aug 2019 17:42:40 +0200 Subject: [PATCH 4/4] Fix merge errors --- src/main/java/fr/xephi/authme/AuthMe.java | 2 +- .../authme/initialization/OnStartupTasks.java | 2 +- .../authme/output/ConsoleFilterTest.java | 77 ------ .../xephi/authme/output/Log4JFilterTest.java | 230 ------------------ 4 files changed, 2 insertions(+), 309 deletions(-) delete mode 100644 src/test/java/fr/xephi/authme/output/ConsoleFilterTest.java delete mode 100644 src/test/java/fr/xephi/authme/output/Log4JFilterTest.java diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 0da11ec6a..76ce55140 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -219,7 +219,7 @@ private void initialize() { ConsoleLoggerFactory.reloadSettings(settings); logger = ConsoleLoggerFactory.get(AuthMe.class); OnStartupTasks onStartupTasks = injector.newInstance(OnStartupTasks.class); - onStartupTasks.setupConsoleFilter(settings, getLogger()); + onStartupTasks.setupConsoleFilter(getLogger()); // Set all service fields on the AuthMe class instantiateServices(injector); diff --git a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java index 114a25120..0ffdba58f 100644 --- a/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java +++ b/src/main/java/fr/xephi/authme/initialization/OnStartupTasks.java @@ -66,7 +66,7 @@ public static void sendMetrics(AuthMe plugin, Settings settings) { * * @param logger the plugin logger */ - public void setupConsoleFilter(Settings settings, Logger logger) { + public void setupConsoleFilter(Logger logger) { // Try to set the log4j filter try { Class.forName("org.apache.logging.log4j.core.filter.AbstractFilter"); diff --git a/src/test/java/fr/xephi/authme/output/ConsoleFilterTest.java b/src/test/java/fr/xephi/authme/output/ConsoleFilterTest.java deleted file mode 100644 index 8975068f6..000000000 --- a/src/test/java/fr/xephi/authme/output/ConsoleFilterTest.java +++ /dev/null @@ -1,77 +0,0 @@ -package fr.xephi.authme.output; - -import org.junit.Test; -import org.mockito.Mockito; - -import java.util.logging.LogRecord; - -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -/** - * Test for {@link ConsoleFilter}. - */ -public class ConsoleFilterTest { - - private final ConsoleFilter filter = new ConsoleFilter(); - - private static final String SENSITIVE_COMMAND = "User issued server command: /login test test"; - private static final String NORMAL_COMMAND = "User issued server command: /motd 2"; - - - @Test - public void shouldReplaceSensitiveRecord() { - // given - LogRecord record = createRecord(SENSITIVE_COMMAND); - - // when - boolean result = filter.isLoggable(record); - - // then - assertThat(result, equalTo(true)); - verify(record).setMessage("User issued an AuthMe command"); - } - - @Test - public void shouldNotFilterRegularCommand() { - // given - LogRecord record = createRecord(NORMAL_COMMAND); - - // when - boolean result = filter.isLoggable(record); - - // then - assertThat(result, equalTo(true)); - verify(record, never()).setMessage("User issued an AuthMe command"); - } - - @Test - public void shouldManageRecordWithNullMessage() { - // given - LogRecord record = createRecord(null); - - // when - boolean result = filter.isLoggable(record); - - // then - assertThat(result, equalTo(true)); - verify(record, never()).setMessage("User issued an AuthMe command"); - } - - - /** - * Creates a mock of {@link LogRecord} and sets it to return the given message. - * - * @param message The message to set. - * - * @return Mock of LogRecord - */ - private static LogRecord createRecord(String message) { - LogRecord record = Mockito.mock(LogRecord.class); - when(record.getMessage()).thenReturn(message); - return record; - } -} diff --git a/src/test/java/fr/xephi/authme/output/Log4JFilterTest.java b/src/test/java/fr/xephi/authme/output/Log4JFilterTest.java deleted file mode 100644 index b9977de8e..000000000 --- a/src/test/java/fr/xephi/authme/output/Log4JFilterTest.java +++ /dev/null @@ -1,230 +0,0 @@ -package fr.xephi.authme.output; - -import static org.hamcrest.Matchers.equalTo; -import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.when; - -import org.apache.logging.log4j.core.Filter.Result; -import org.apache.logging.log4j.core.LogEvent; -import org.apache.logging.log4j.message.Message; -import org.junit.Test; -import org.mockito.Mockito; - -/** - * Test for {@link Log4JFilter}. - */ -public class Log4JFilterTest { - - private final Log4JFilter log4JFilter = new Log4JFilter(); - - private static final String SENSITIVE_COMMAND = "User issued server command: /login pass pass"; - private static final String NORMAL_COMMAND = "User issued server command: /help"; - private static final String OTHER_COMMAND = "Starting the server... Write /l for logs"; - - // --------- - // Test the filter(LogEvent) method - // --------- - @Test - public void shouldFilterSensitiveLogEvent() { - // given - Message message = mockMessage(SENSITIVE_COMMAND); - LogEvent event = Mockito.mock(LogEvent.class); - when(event.getMessage()).thenReturn(message); - - // when - Result result = log4JFilter.filter(event); - - // then - assertThat(result, equalTo(Result.DENY)); - } - - @Test - public void shouldNotFilterIrrelevantLogEvent() { - // given - Message message = mockMessage(NORMAL_COMMAND); - LogEvent event = Mockito.mock(LogEvent.class); - when(event.getMessage()).thenReturn(message); - - // when - Result result = log4JFilter.filter(event); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterNonCommandLogEvent() { - // given - Message message = mockMessage(OTHER_COMMAND); - LogEvent event = Mockito.mock(LogEvent.class); - when(event.getMessage()).thenReturn(message); - - // when - Result result = log4JFilter.filter(event); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterLogEventWithNullMessage() { - // given - Message message = mockMessage(null); - LogEvent event = Mockito.mock(LogEvent.class); - when(event.getMessage()).thenReturn(message); - - // when - Result result = log4JFilter.filter(event); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterWhenLogEventIsNull() { - // given / when - Result result = log4JFilter.filter(null); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - // ---------- - // Test filter(Logger, Level, Marker, String, Object...) - // ---------- - @Test - public void shouldFilterSensitiveStringMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, SENSITIVE_COMMAND); - - // then - assertThat(result, equalTo(Result.DENY)); - } - - @Test - public void shouldNotFilterNormalStringMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, NORMAL_COMMAND); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterNonCommandStringMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, OTHER_COMMAND); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldReturnNeutralForNullMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, null); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - // -------- - // Test filter(Logger, Level, Marker, Object, Throwable) - // -------- - @Test - public void shouldFilterSensitiveObjectMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, SENSITIVE_COMMAND, new Exception()); - - // then - assertThat(result, equalTo(Result.DENY)); - } - - @Test - public void shouldNotFilterNullObjectParam() { - // given / when - Result result = log4JFilter.filter(null, null, null, (Object) null, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterIrrelevantMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, OTHER_COMMAND, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterNonSensitiveCommand() { - // given / when - Result result = log4JFilter.filter(null, null, null, NORMAL_COMMAND, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - // -------- - // Test filter(Logger, Level, Marker, Message, Throwable) - // -------- - @Test - public void shouldFilterSensitiveMessage() { - // given - Message message = mockMessage(SENSITIVE_COMMAND); - - // when - Result result = log4JFilter.filter(null, null, null, message, new Exception()); - - // then - assertThat(result, equalTo(Result.DENY)); - } - - @Test - public void shouldNotFilterNonSensitiveMessage() { - // given - Message message = mockMessage(NORMAL_COMMAND); - - // when - Result result = log4JFilter.filter(null, null, null, message, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterNonCommandMessage() { - // given - Message message = mockMessage(OTHER_COMMAND); - - // when - Result result = log4JFilter.filter(null, null, null, message, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - @Test - public void shouldNotFilterNullMessage() { - // given / when - Result result = log4JFilter.filter(null, null, null, null, new Exception()); - - // then - assertThat(result, equalTo(Result.NEUTRAL)); - } - - /** - * Mocks a {@link Message} object and makes it return the given formatted message. - * - * @param formattedMessage the formatted message the mock should return - * @return Message mock - */ - private static Message mockMessage(String formattedMessage) { - Message message = Mockito.mock(Message.class); - when(message.getFormattedMessage()).thenReturn(formattedMessage); - return message; - } - -}