diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 018ebf31e..862c6dd7c 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -62,6 +62,7 @@ import fr.xephi.authme.util.GeoLiteAPI; import fr.xephi.authme.util.MigrationService; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.Utils; +import fr.xephi.authme.util.ValidationService; import org.apache.logging.log4j.LogManager; import org.bukkit.Bukkit; import org.bukkit.Location; @@ -260,8 +261,9 @@ public class AuthMe extends JavaPlugin { // Set up the permissions manager and command handler permsMan = initializePermissionsManager(); + ValidationService validationService = new ValidationService(newSettings); commandHandler = initializeCommandHandler(permsMan, messages, passwordSecurity, newSettings, ipAddressManager, - pluginHooks, spawnLoader, antiBot); + pluginHooks, spawnLoader, antiBot, validationService); // AntiBot delay BukkitService bukkitService = new BukkitService(this); @@ -299,7 +301,7 @@ public class AuthMe extends JavaPlugin { // Set up the management ProcessService processService = new ProcessService(newSettings, messages, this, database, ipAddressManager, - passwordSecurity, pluginHooks, spawnLoader); + passwordSecurity, pluginHooks, spawnLoader, validationService); management = new Management(this, processService, database, PlayerCache.getInstance()); // Set up the BungeeCord hook @@ -432,12 +434,13 @@ public class AuthMe extends JavaPlugin { private CommandHandler initializeCommandHandler(PermissionsManager permissionsManager, Messages messages, PasswordSecurity passwordSecurity, NewSetting settings, IpAddressManager ipAddressManager, PluginHooks pluginHooks, - SpawnLoader spawnLoader, AntiBot antiBot) { + SpawnLoader spawnLoader, AntiBot antiBot, + ValidationService validationService) { HelpProvider helpProvider = new HelpProvider(permissionsManager, settings.getProperty(HELP_HEADER)); Set baseCommands = CommandInitializer.buildCommands(); CommandMapper mapper = new CommandMapper(baseCommands, permissionsManager); CommandService commandService = new CommandService(this, mapper, helpProvider, messages, passwordSecurity, - permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot); + permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot, validationService); return new CommandHandler(commandService); } diff --git a/src/main/java/fr/xephi/authme/command/CommandService.java b/src/main/java/fr/xephi/authme/command/CommandService.java index a4791e688..9d8252a0c 100644 --- a/src/main/java/fr/xephi/authme/command/CommandService.java +++ b/src/main/java/fr/xephi/authme/command/CommandService.java @@ -15,6 +15,7 @@ import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.domain.Property; +import fr.xephi.authme.util.ValidationService; import org.bukkit.command.CommandSender; import java.util.List; @@ -36,6 +37,7 @@ public class CommandService { private final PluginHooks pluginHooks; private final SpawnLoader spawnLoader; private final AntiBot antiBot; + private final ValidationService validationService; /** * Constructor. @@ -54,7 +56,7 @@ public class CommandService { public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages, PasswordSecurity passwordSecurity, PermissionsManager permissionsManager, NewSetting settings, IpAddressManager ipAddressManager, PluginHooks pluginHooks, SpawnLoader spawnLoader, - AntiBot antiBot) { + AntiBot antiBot, ValidationService validationService) { this.authMe = authMe; this.messages = messages; this.helpProvider = helpProvider; @@ -66,6 +68,7 @@ public class CommandService { this.pluginHooks = pluginHooks; this.spawnLoader = spawnLoader; this.antiBot = antiBot; + this.validationService = validationService; } /** @@ -219,4 +222,15 @@ public class CommandService { return antiBot; } + /** + * Verifies whether a password is valid according to the plugin settings. + * + * @param password the password to verify + * @param username the username the password is associated with + * @return message key with the password error, or {@code null} if password is valid + */ + public MessageKey validatePassword(String password, String username) { + return validationService.validatePassword(password, username); + } + } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index 8ba9d740f..867443edb 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -7,8 +7,6 @@ import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.settings.properties.RestrictionSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.command.CommandSender; import java.util.List; @@ -22,30 +20,16 @@ public class ChangePasswordAdminCommand implements ExecutableCommand { public void executeCommand(final CommandSender sender, List arguments, final CommandService commandService) { // Get the player and password - String playerName = arguments.get(0); + final String playerName = arguments.get(0); final String playerPass = arguments.get(1); // Validate the password - String playerPassLowerCase = playerPass.toLowerCase(); - if (!playerPassLowerCase.matches(commandService.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX))) { - commandService.send(sender, MessageKey.PASSWORD_MATCH_ERROR); - return; - } - if (playerPassLowerCase.equalsIgnoreCase(playerName)) { - commandService.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); - return; - } - if (playerPassLowerCase.length() < commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH) - || playerPassLowerCase.length() > commandService.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) { - commandService.send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - return; - } - // TODO #602 20160312: The UNSAFE_PASSWORDS should be all lowercase - // -> introduce a lowercase String list property type - if (commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(playerPassLowerCase)) { - commandService.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); + MessageKey passwordError = commandService.validatePassword(playerPass, playerName); + if (passwordError != null) { + commandService.send(sender, passwordError); return; } + // Set the password final String playerNameLowerCase = playerName.toLowerCase(); commandService.runTaskAsynchronously(new Runnable() { diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java index a33f3eb39..e0010a5bc 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java @@ -6,8 +6,6 @@ import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.Bukkit; import org.bukkit.command.CommandSender; @@ -25,26 +23,14 @@ public class RegisterAdminCommand implements ExecutableCommand { final String playerName = arguments.get(0); final String playerPass = arguments.get(1); final String playerNameLowerCase = playerName.toLowerCase(); - final String playerPassLowerCase = playerPass.toLowerCase(); // Command logic - if (!playerPassLowerCase.matches(Settings.getPassRegex)) { - commandService.send(sender, MessageKey.PASSWORD_MATCH_ERROR); - return; - } - if (playerPassLowerCase.equalsIgnoreCase(playerName)) { - commandService.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); - return; - } - if (playerPassLowerCase.length() < commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH) - || playerPassLowerCase.length() > commandService.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) { - commandService.send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - return; - } - if (!Settings.unsafePasswords.isEmpty() && Settings.unsafePasswords.contains(playerPassLowerCase)) { - commandService.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); + MessageKey passwordError = commandService.validatePassword(playerPass, playerName); + if (passwordError != null) { + commandService.send(sender, passwordError); return; } + commandService.runTaskAsynchronously(new Runnable() { @Override diff --git a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java index 336df1529..7d378ca88 100644 --- a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java @@ -5,8 +5,6 @@ import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.output.MessageKey; -import fr.xephi.authme.settings.properties.RestrictionSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.task.ChangePasswordTask; import org.bukkit.entity.Player; @@ -30,22 +28,9 @@ public class ChangePasswordCommand extends PlayerCommand { } // Make sure the password is allowed - String playerPassLowerCase = newPassword.toLowerCase(); - if (!playerPassLowerCase.matches(commandService.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX))) { - commandService.send(player, MessageKey.PASSWORD_MATCH_ERROR); - return; - } - if (playerPassLowerCase.equalsIgnoreCase(name)) { - commandService.send(player, MessageKey.PASSWORD_IS_USERNAME_ERROR); - return; - } - if (playerPassLowerCase.length() < commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH) - || playerPassLowerCase.length() > commandService.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) { - commandService.send(player, MessageKey.INVALID_PASSWORD_LENGTH); - return; - } - if (commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(playerPassLowerCase)) { - commandService.send(player, MessageKey.PASSWORD_UNSAFE_ERROR); + MessageKey passwordError = commandService.validatePassword(newPassword, name); + if (passwordError != null) { + commandService.send(player, passwordError); return; } diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 4cea16fc4..4b279c9a1 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -41,7 +41,7 @@ public class Management { } public void performRegister(final Player player, final String password, final String email) { - runTask(new AsyncRegister(player, password, email, plugin, dataSource, processService)); + runTask(new AsyncRegister(player, password, email, plugin, dataSource, playerCache, processService)); } public void performUnregister(final Player player, final String password, final boolean force) { diff --git a/src/main/java/fr/xephi/authme/process/ProcessService.java b/src/main/java/fr/xephi/authme/process/ProcessService.java index 7f696fe1d..0489447a0 100644 --- a/src/main/java/fr/xephi/authme/process/ProcessService.java +++ b/src/main/java/fr/xephi/authme/process/ProcessService.java @@ -11,6 +11,7 @@ import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.domain.Property; +import fr.xephi.authme.util.ValidationService; import org.bukkit.command.CommandSender; import org.bukkit.event.Event; import org.bukkit.scheduler.BukkitTask; @@ -28,10 +29,11 @@ public class ProcessService { private final PasswordSecurity passwordSecurity; private final PluginHooks pluginHooks; private final SpawnLoader spawnLoader; + private final ValidationService validationService; public ProcessService(NewSetting settings, Messages messages, AuthMe authMe, DataSource dataSource, IpAddressManager ipAddressManager, PasswordSecurity passwordSecurity, PluginHooks pluginHooks, - SpawnLoader spawnLoader) { + SpawnLoader spawnLoader, ValidationService validationService) { this.settings = settings; this.messages = messages; this.authMe = authMe; @@ -40,6 +42,7 @@ public class ProcessService { this.passwordSecurity = passwordSecurity; this.pluginHooks = pluginHooks; this.spawnLoader = spawnLoader; + this.validationService = validationService; } /** @@ -199,4 +202,15 @@ public class ProcessService { return dataSource; } + /** + * Verifies whether a password is valid according to the plugin settings. + * + * @param password the password to verify + * @param username the username the password is associated with + * @return message key with the password error, or {@code null} if password is valid + */ + public MessageKey validatePassword(String password, String username) { + return validationService.validatePassword(password, username); + } + } diff --git a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index 47b715a38..e212fabc7 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -12,6 +12,8 @@ import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.TwoFactor; import fr.xephi.authme.settings.Settings; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.util.StringUtils; import org.bukkit.Bukkit; @@ -30,10 +32,11 @@ public class AsyncRegister implements Process { private final String email; private final AuthMe plugin; private final DataSource database; + private final PlayerCache playerCache; private final ProcessService service; public AsyncRegister(Player player, String password, String email, AuthMe plugin, DataSource data, - ProcessService service) { + PlayerCache playerCache, ProcessService service) { this.player = player; this.password = password; this.name = player.getName().toLowerCase(); @@ -41,32 +44,24 @@ public class AsyncRegister implements Process { this.plugin = plugin; this.database = data; this.ip = service.getIpAddressManager().getPlayerIp(player); + this.playerCache = playerCache; this.service = service; } private boolean preRegisterCheck() { - String passLow = password.toLowerCase(); - if (PlayerCache.getInstance().isAuthenticated(name)) { + if (playerCache.isAuthenticated(name)) { service.send(player, MessageKey.ALREADY_LOGGED_IN_ERROR); return false; - } else if (!Settings.isRegistrationEnabled) { + } else if (!service.getProperty(RegistrationSettings.IS_ENABLED)) { service.send(player, MessageKey.REGISTRATION_DISABLED); return false; } //check the password safety only if it's not a automatically generated password if (service.getProperty(SecuritySettings.PASSWORD_HASH) != HashAlgorithm.TWO_FACTOR) { - if (!passLow.matches(Settings.getPassRegex)) { - service.send(player, MessageKey.PASSWORD_MATCH_ERROR); - return false; - } else if (passLow.equalsIgnoreCase(player.getName())) { - service.send(player, MessageKey.PASSWORD_IS_USERNAME_ERROR); - return false; - } else if (password.length() < Settings.getPasswordMinLen || password.length() > Settings.passwordMaxLength) { - service.send(player, MessageKey.INVALID_PASSWORD_LENGTH); - return false; - } else if (!Settings.unsafePasswords.isEmpty() && Settings.unsafePasswords.contains(password.toLowerCase())) { - service.send(player, MessageKey.PASSWORD_UNSAFE_ERROR); + MessageKey passwordError = service.validatePassword(password, player.getName()); + if (passwordError != null) { + service.send(player, passwordError); return false; } } @@ -75,15 +70,17 @@ public class AsyncRegister implements Process { if (database.isAuthAvailable(name)) { service.send(player, MessageKey.NAME_ALREADY_REGISTERED); return false; - } else if(Settings.getmaxRegPerIp > 0 - && !ip.equalsIgnoreCase("127.0.0.1") - && !ip.equalsIgnoreCase("localhost") + } + + final int maxRegPerIp = service.getProperty(RestrictionSettings.MAX_REGISTRATION_PER_IP); + if (maxRegPerIp > 0 + && !"127.0.0.1".equalsIgnoreCase(ip) + && !"localhost".equalsIgnoreCase(ip) && !plugin.getPermissionsManager().hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)) { - int maxReg = Settings.getmaxRegPerIp; List otherAccounts = database.getAllAuthsByIp(ip); - if (otherAccounts.size() >= maxReg) { - service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxReg), - Integer.toString(otherAccounts.size()), StringUtils.join(", ", otherAccounts.toString())); + if (otherAccounts.size() >= maxRegPerIp) { + service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxRegPerIp), + Integer.toString(otherAccounts.size()), StringUtils.join(", ", otherAccounts)); return false; } } diff --git a/src/main/java/fr/xephi/authme/settings/Settings.java b/src/main/java/fr/xephi/authme/settings/Settings.java index a35c6df84..ef9a4b9d6 100644 --- a/src/main/java/fr/xephi/authme/settings/Settings.java +++ b/src/main/java/fr/xephi/authme/settings/Settings.java @@ -38,11 +38,10 @@ public final class Settings { public static List forceCommandsAsConsole; public static List forceRegisterCommands; public static List forceRegisterCommandsAsConsole; - public static List unsafePasswords; public static HashAlgorithm getPasswordHash; public static Pattern nickPattern; public static boolean useLogging = false; - public static boolean isChatAllowed, isPermissionCheckEnabled, isRegistrationEnabled, + public static boolean isChatAllowed, isPermissionCheckEnabled, isForcedRegistrationEnabled, isTeleportToSpawnEnabled, isSessionsEnabled, isAllowRestrictedIp, isMovementAllowed, isKickNonRegisteredEnabled, @@ -64,10 +63,10 @@ public final class Settings { unRegisteredGroup, backupWindowsPath, getRegisteredGroup, rakamakUsers, rakamakUsersIp, getmailAccount, defaultWorld, - spawnPriority, crazyloginFileName, getPassRegex, sendPlayerTo; + spawnPriority, crazyloginFileName, sendPlayerTo; public static int getWarnMessageInterval, getSessionTimeout, getRegistrationTimeout, getMaxNickLength, getMinNickLength, - getPasswordMinLen, getMovementRadius, getmaxRegPerIp, + getPasswordMinLen, getMovementRadius, getNonActivatedGroup, passwordMaxLength, getRecoveryPassLength, getMailPort, maxLoginTry, captchaLength, saltLength, getmaxRegPerEmail, bCryptLog2Rounds, getMaxLoginPerIp, getMaxJoinPerIp; @@ -86,7 +85,6 @@ public final class Settings { private static void loadVariables() { isPermissionCheckEnabled = load(PluginSettings.ENABLE_PERMISSION_CHECK); isForcedRegistrationEnabled = configFile.getBoolean("settings.registration.force", true); - isRegistrationEnabled = configFile.getBoolean("settings.registration.enabled", true); isTeleportToSpawnEnabled = load(RestrictionSettings.TELEPORT_UNAUTHED_TO_SPAWN); getWarnMessageInterval = load(RegistrationSettings.MESSAGE_INTERVAL); isSessionsEnabled = load(PluginSettings.SESSIONS_ENABLED); @@ -110,7 +108,6 @@ public final class Settings { isForceSpawnLocOnJoinEnabled = configFile.getBoolean("settings.restrictions.ForceSpawnLocOnJoinEnabled", false); isSaveQuitLocationEnabled = configFile.getBoolean("settings.restrictions.SaveQuitLocation", false); isForceSurvivalModeEnabled = configFile.getBoolean("settings.GameMode.ForceSurvivalMode", false); - getmaxRegPerIp = configFile.getInt("settings.restrictions.maxRegPerIp", 1); getPasswordHash = load(SecuritySettings.PASSWORD_HASH); getUnloggedinGroup = load(SecuritySettings.UNLOGGEDIN_GROUP); getNonActivatedGroup = configFile.getInt("ExternalBoardOptions.nonActivedUserGroup", -1); @@ -174,7 +171,6 @@ public final class Settings { forceCommandsAsConsole = configFile.getStringList("settings.forceCommandsAsConsole"); recallEmail = configFile.getBoolean("Email.recallPlayers", false); useWelcomeMessage = load(RegistrationSettings.USE_WELCOME_MESSAGE); - unsafePasswords = configFile.getStringList("settings.security.unsafePasswords"); countriesBlacklist = configFile.getStringList("Protection.countriesBlacklist"); broadcastWelcomeMessage = load(RegistrationSettings.BROADCAST_WELCOME_MESSAGE); forceRegKick = configFile.getBoolean("settings.registration.forceKickAfterRegister", false); @@ -188,7 +184,6 @@ public final class Settings { delayJoinMessage = load(RegistrationSettings.DELAY_JOIN_MESSAGE); noTeleport = load(RestrictionSettings.NO_TELEPORT); crazyloginFileName = configFile.getString("Converter.CrazyLogin.fileName", "accounts.db"); - getPassRegex = configFile.getString("settings.restrictions.allowedPasswordCharacters", "[\\x21-\\x7E]*"); forceRegisterCommands = configFile.getStringList("settings.forceRegisterCommands"); forceRegisterCommandsAsConsole = configFile.getStringList("settings.forceRegisterCommandsAsConsole"); customAttributes = configFile.getBoolean("Hooks.customAttributes"); @@ -198,21 +193,6 @@ public final class Settings { keepCollisionsDisabled = configFile.getBoolean("settings.keepCollisionsDisabled"); } - /** - * Method switchAntiBotMod. - * - * @param mode boolean - */ - public static void switchAntiBotMod(boolean mode) { - if (mode) { - isKickNonRegisteredEnabled = true; - antiBotInAction = true; - } else { - isKickNonRegisteredEnabled = configFile.getBoolean("settings.restrictions.kickNonRegistered", false); - antiBotInAction = false; - } - } - /** * Load the value via the new Property setup for temporary support within this old settings manager. * diff --git a/src/main/java/fr/xephi/authme/util/ValidationService.java b/src/main/java/fr/xephi/authme/util/ValidationService.java new file mode 100644 index 000000000..1adbda717 --- /dev/null +++ b/src/main/java/fr/xephi/authme/util/ValidationService.java @@ -0,0 +1,42 @@ +package fr.xephi.authme.util; + +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; + +/** + * Validation service. + */ +public class ValidationService { + + private final NewSetting settings; + + public ValidationService(NewSetting settings) { + this.settings = settings; + } + + /** + * Verifies whether a password is valid according to the plugin settings. + * + * @param password the password to verify + * @param username the username the password is associated with + * @return message key with the password error, or {@code null} if password is valid + */ + public MessageKey validatePassword(String password, String username) { + String passLow = password.toLowerCase(); + if (!passLow.matches(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX))) { + return MessageKey.PASSWORD_MATCH_ERROR; + } else if (passLow.equalsIgnoreCase(username)) { + return MessageKey.PASSWORD_IS_USERNAME_ERROR; + } else if (password.length() < settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH) + || password.length() > settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) { + return MessageKey.INVALID_PASSWORD_LENGTH; + } else if (settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(passLow)) { + // TODO #602 20160312: The UNSAFE_PASSWORDS should be all lowercase + // -> introduce a lowercase String list property type + return MessageKey.PASSWORD_UNSAFE_ERROR; + } + return null; + } +} diff --git a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java index d34ee6331..dafe69667 100644 --- a/src/test/java/fr/xephi/authme/command/CommandServiceTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandServiceTest.java @@ -15,6 +15,7 @@ import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.domain.Property; import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.ValidationService; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import org.junit.Before; @@ -63,11 +64,13 @@ public class CommandServiceTest { private SpawnLoader spawnLoader; @Mock private AntiBot antiBot; + @Mock + private ValidationService validationService; @Before public void setUpService() { commandService = new CommandService(authMe, commandMapper, helpProvider, messages, passwordSecurity, - permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot); + permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot, validationService); } @Test @@ -228,4 +231,19 @@ public class CommandServiceTest { // then assertThat(ipManager, equalTo(ipAddressManager)); } + + @Test + public void shouldValidatePassword() { + // given + String user = "asdf"; + String password = "mySecret55"; + given(validationService.validatePassword(password, user)).willReturn(MessageKey.INVALID_PASSWORD_LENGTH); + + // when + MessageKey result = commandService.validatePassword(password, user); + + // then + assertThat(result, equalTo(MessageKey.INVALID_PASSWORD_LENGTH)); + verify(validationService).validatePassword(password, user); + } } diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java index 28a6a3f87..93c6c6ff2 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java @@ -1,6 +1,5 @@ package fr.xephi.authme.command.executable.authme; -import com.google.common.base.Strings; import fr.xephi.authme.ConsoleLoggerTestInitializer; import fr.xephi.authme.cache.auth.PlayerAuth; import fr.xephi.authme.cache.auth.PlayerCache; @@ -10,12 +9,12 @@ import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; -import fr.xephi.authme.settings.properties.RestrictionSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.command.CommandSender; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; import java.util.Arrays; @@ -29,8 +28,10 @@ import static org.mockito.Mockito.verify; /** * Test for {@link ChangePasswordAdminCommand}. */ +@RunWith(MockitoJUnitRunner.class) public class ChangePasswordAdminCommandTest { + @Mock private CommandService service; @BeforeClass @@ -38,88 +39,22 @@ public class ChangePasswordAdminCommandTest { ConsoleLoggerTestInitializer.setupLogger(); } - @Before - public void setUpServiceMock() { - service = mock(CommandService.class); - given(service.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)).willReturn("[a-zA-Z]+"); - given(service.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3); - given(service.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(20); - given(service.getProperty(SecuritySettings.UNSAFE_PASSWORDS)) - .willReturn(Arrays.asList("unsafe", "otherUnsafe")); - } - @Test - public void shouldRejectPasswordSameAsUsername() { + public void shouldRejectInvalidPassword() { // given ExecutableCommand command = new ChangePasswordAdminCommand(); CommandSender sender = mock(CommandSender.class); + given(service.validatePassword("Bobby", "bobby")).willReturn(MessageKey.PASSWORD_IS_USERNAME_ERROR); // when command.executeCommand(sender, Arrays.asList("bobby", "Bobby"), service); // then + verify(service).validatePassword("Bobby", "bobby"); verify(service).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); verify(service, never()).getDataSource(); } - @Test - public void shouldRejectPasswordNotMatchingPattern() { - // given - ExecutableCommand command = new ChangePasswordAdminCommand(); - CommandSender sender = mock(CommandSender.class); - // service mock returns pattern a-zA-Z -> numbers should not be accepted - String invalidPassword = "invalid1234"; - - // when - command.executeCommand(sender, Arrays.asList("myPlayer123", invalidPassword), service); - - // then - verify(service).send(sender, MessageKey.PASSWORD_MATCH_ERROR); - verify(service, never()).getDataSource(); - } - - @Test - public void shouldRejectTooShortPassword() { - // given - ExecutableCommand command = new ChangePasswordAdminCommand(); - CommandSender sender = mock(CommandSender.class); - - // when - command.executeCommand(sender, Arrays.asList("player", "ab"), service); - - // then - verify(service).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - verify(service, never()).getDataSource(); - } - - @Test - public void shouldRejectTooLongPassword() { - // given - ExecutableCommand command = new ChangePasswordAdminCommand(); - CommandSender sender = mock(CommandSender.class); - - // when - command.executeCommand(sender, Arrays.asList("player", Strings.repeat("a", 30)), service); - - // then - verify(service).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - verify(service, never()).getDataSource(); - } - - @Test - public void shouldRejectUnsafePassword() { - // given - ExecutableCommand command = new ChangePasswordAdminCommand(); - CommandSender sender = mock(CommandSender.class); - - // when - command.executeCommand(sender, Arrays.asList("player", "unsafe"), service); - - // then - verify(service).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); - verify(service, never()).getDataSource(); - } - @Test public void shouldRejectCommandForUnknownUser() { // given @@ -173,6 +108,7 @@ public class ChangePasswordAdminCommandTest { runInnerRunnable(service); // then + verify(service).validatePassword(password, player); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(passwordSecurity).computeHash(password, player); verify(auth).setPassword(hashedPassword); @@ -209,6 +145,7 @@ public class ChangePasswordAdminCommandTest { runInnerRunnable(service); // then + verify(service).validatePassword(password, player); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(passwordSecurity).computeHash(password, player); verify(auth).setPassword(hashedPassword); @@ -244,6 +181,7 @@ public class ChangePasswordAdminCommandTest { runInnerRunnable(service); // then + verify(service).validatePassword(password, player); verify(service).send(sender, MessageKey.ERROR); verify(passwordSecurity).computeHash(password, player); verify(auth).setPassword(hashedPassword); diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index 213dba4c3..ef4572055 100644 --- a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -22,6 +22,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -57,9 +58,7 @@ public class ChangePasswordCommandTest { command.executeCommand(sender, new ArrayList(), commandService); // then - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - verify(sender).sendMessage(captor.capture()); - assertThat(captor.getValue(), containsString("only for players")); + verify(sender).sendMessage(argThat(containsString("only for players"))); } @Test @@ -76,75 +75,21 @@ public class ChangePasswordCommandTest { } @Test - public void shouldDenyInvalidPassword() { - // given - CommandSender sender = initPlayerWithName("name", true); - ChangePasswordCommand command = new ChangePasswordCommand(); - - // when - command.executeCommand(sender, Arrays.asList("old123", "!pass"), commandService); - - // then - verify(commandService).send(sender, MessageKey.PASSWORD_MATCH_ERROR); - } - - - @Test - public void shouldRejectPasswordEqualToNick() { - // given - CommandSender sender = initPlayerWithName("tester", true); - ChangePasswordCommand command = new ChangePasswordCommand(); - - // when - command.executeCommand(sender, Arrays.asList("old_", "Tester"), commandService); - - // then - verify(commandService).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); - } - - @Test - public void shouldRejectTooLongPassword() { + public void shouldRejectInvalidPassword() { // given CommandSender sender = initPlayerWithName("abc12", true); ChangePasswordCommand command = new ChangePasswordCommand(); - given(commandService.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(3); + String password = "newPW"; + given(commandService.validatePassword(password, "abc12")).willReturn(MessageKey.INVALID_PASSWORD_LENGTH); // when - command.executeCommand(sender, Arrays.asList("12", "test"), commandService); + command.executeCommand(sender, Arrays.asList("tester", password), commandService); // then + verify(commandService).validatePassword(password, "abc12"); verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); } - @Test - public void shouldRejectTooShortPassword() { - // given - CommandSender sender = initPlayerWithName("abc12", true); - ChangePasswordCommand command = new ChangePasswordCommand(); - given(commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(7); - - // when - command.executeCommand(sender, Arrays.asList("oldverylongpassword", "tester"), commandService); - - // then - verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); - } - - @Test - public void shouldRejectUnsafeCustomPassword() { - // given - CommandSender sender = initPlayerWithName("player", true); - ChangePasswordCommand command = new ChangePasswordCommand(); - given(commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS)) - .willReturn(Arrays.asList("test", "abc123")); - - // when - command.executeCommand(sender, Arrays.asList("oldpw", "abc123"), commandService); - - // then - verify(commandService).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); - } - @Test public void shouldForwardTheDataForValidPassword() { // given @@ -155,6 +100,7 @@ public class ChangePasswordCommandTest { command.executeCommand(sender, Arrays.asList("abc123", "abc123"), commandService); // then + verify(commandService).validatePassword("abc123", "parker"); verify(commandService, never()).send(eq(sender), any(MessageKey.class)); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class); verify(commandService).runTaskAsynchronously(taskCaptor.capture()); diff --git a/src/test/java/fr/xephi/authme/process/ProcessServiceTest.java b/src/test/java/fr/xephi/authme/process/ProcessServiceTest.java index da3c2f8d9..23a87beef 100644 --- a/src/test/java/fr/xephi/authme/process/ProcessServiceTest.java +++ b/src/test/java/fr/xephi/authme/process/ProcessServiceTest.java @@ -11,7 +11,9 @@ import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; import fr.xephi.authme.settings.properties.SecuritySettings; +import fr.xephi.authme.util.ValidationService; import org.bukkit.command.CommandSender; +import org.hamcrest.MatcherAssert; import org.junit.Before; import org.junit.Test; @@ -37,7 +39,7 @@ public class ProcessServiceTest { mocks = new HashMap<>(); processService = new ProcessService(newMock(NewSetting.class), newMock(Messages.class), newMock(AuthMe.class), newMock(DataSource.class), newMock(IpAddressManager.class), newMock(PasswordSecurity.class), - newMock(PluginHooks.class), newMock(SpawnLoader.class)); + newMock(PluginHooks.class), newMock(SpawnLoader.class), newMock(ValidationService.class)); } @Test @@ -186,6 +188,22 @@ public class ProcessServiceTest { verify(passwordSecurity).computeHash(password, username); } + @Test + public void shouldValidatePassword() { + // given + String user = "test-user"; + String password = "passw0rd"; + ValidationService validationService = getMock(ValidationService.class); + given(validationService.validatePassword(password, user)).willReturn(MessageKey.PASSWORD_MATCH_ERROR); + + // when + MessageKey result = processService.validatePassword(password, user); + + // then + MatcherAssert.assertThat(result, equalTo(MessageKey.PASSWORD_MATCH_ERROR)); + verify(validationService).validatePassword(password, user); + } + private T newMock(Class clazz) { T mock = mock(clazz); mocks.put(clazz, mock); diff --git a/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java new file mode 100644 index 000000000..0fd567f2b --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/ValidationServiceTest.java @@ -0,0 +1,92 @@ +package fr.xephi.authme.util; + +import com.google.common.base.Strings; +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; + +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link ValidationService}. + */ +public class ValidationServiceTest { + + private ValidationService validationService; + + @Before + public void createService() { + NewSetting settings = mock(NewSetting.class); + given(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)).willReturn("[a-zA-Z]+"); + given(settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3); + given(settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(20); + given(settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS)) + .willReturn(Arrays.asList("unsafe", "other-unsafe")); + validationService = new ValidationService(settings); + } + + @Test + public void shouldRejectPasswordSameAsUsername() { + // given/when + MessageKey error = validationService.validatePassword("bobby", "Bobby"); + + // then + assertThat(error, equalTo(MessageKey.PASSWORD_IS_USERNAME_ERROR)); + } + + @Test + public void shouldRejectPasswordNotMatchingPattern() { + // given/when + // service mock returns pattern a-zA-Z -> numbers should not be accepted + MessageKey error = validationService.validatePassword("invalid1234", "myPlayer"); + + // then + assertThat(error, equalTo(MessageKey.PASSWORD_MATCH_ERROR)); + } + + @Test + public void shouldRejectTooShortPassword() { + // given/when + MessageKey error = validationService.validatePassword("ab", "tester"); + + // then + assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH)); + } + + @Test + public void shouldRejectTooLongPassword() { + // given/when + MessageKey error = validationService.validatePassword(Strings.repeat("a", 30), "player"); + + // then + assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH)); + } + + @Test + public void shouldRejectUnsafePassword() { + // given/when + MessageKey error = validationService.validatePassword("unsafe", "playertest"); + + // then + assertThat(error, equalTo(MessageKey.PASSWORD_UNSAFE_ERROR)); + } + + @Test + public void shouldAcceptValidPassword() { + // given/when + MessageKey error = validationService.validatePassword("safePass", "some_user"); + + // then + assertThat(error, nullValue()); + } + +}