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 @@ public class AuthMe extends JavaPlugin { // 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 @@ public class AuthMe extends JavaPlugin { 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 @@ public class CommandDescription { this.parent = parent; this.arguments = arguments; this.permission = permission; + this.sensitive = sensitive; } /** @@ -184,6 +190,10 @@ public class CommandDescription { return permission; } + public boolean isSensitive() { + return sensitive; + } + /** * Return a builder instance to create a new command description. * @@ -204,6 +214,7 @@ public class CommandDescription { 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 class CommandDescription { // 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 class CommandDescription { 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 @@ public class CommandInitializer { .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 @@ public class CommandInitializer { .withArgument("password", "Password", OPTIONAL) .withArgument("verifyPassword", "Verify password", OPTIONAL) .permission(PlayerPermission.REGISTER) + .sensitive(true) .executableCommand(RegisterCommand.class) .register(); @@ -139,6 +141,7 @@ public class CommandInitializer { .withArgument("oldPassword", "Old password", MANDATORY) .withArgument("newPassword", "New password", MANDATORY) .permission(PlayerPermission.CHANGE_PASSWORD) + .sensitive(true) .executableCommand(ChangePasswordCommand.class) .register(); @@ -197,6 +200,7 @@ public class CommandInitializer { .withArgument("player", "Player name", MANDATORY) .withArgument("password", "Password", MANDATORY) .permission(AdminPermission.REGISTER) + .sensitive(true) .executableCommand(RegisterAdminCommand.class) .register(); @@ -231,6 +235,7 @@ public class CommandInitializer { .withArgument("player", "Player name", MANDATORY) .withArgument("pwd", "New password", MANDATORY) .permission(AdminPermission.CHANGE_PASSWORD) + .sensitive(true) .executableCommand(ChangePasswordAdminCommand.class) .register(); @@ -540,6 +545,7 @@ public class CommandInitializer { .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 @@ public class CommandInitializer { .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 @@ public class CommandInitializer { .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 @@ public class CommandInitializer { .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.MessageKey; 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 class OnStartupTasks { * @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 class OnStartupTasks { } 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 class OnStartupTasks { } // 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 @@ import java.util.logging.LogRecord; */ 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 @@ public class Log4JFilter extends AbstractFilter { * * @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.hamcrest.Matchers.containsInAnyOrder; 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 class LogFilterHelperTest { .toArray(String[]::new); // when / then - assertThat(LogFilterHelper.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries)); + assertThat(LogFilterService.COMMANDS_TO_SKIP, containsInAnyOrder(expectedEntries)); }