diff --git a/pom.xml b/pom.xml index 8f81c869..c4cfe5f8 100644 --- a/pom.xml +++ b/pom.xml @@ -150,7 +150,7 @@ org.jacoco jacoco-maven-plugin - 0.8.0 + 0.8.1 pre-unit-test diff --git a/src/main/java/fr/xephi/authme/AuthMe.java b/src/main/java/fr/xephi/authme/AuthMe.java index 4aeb536c..6a2b3583 100644 --- a/src/main/java/fr/xephi/authme/AuthMe.java +++ b/src/main/java/fr/xephi/authme/AuthMe.java @@ -2,9 +2,7 @@ package fr.xephi.authme; import ch.jalu.injector.Injector; import ch.jalu.injector.InjectorBuilder; - import com.google.common.annotations.VisibleForTesting; - import fr.xephi.authme.api.NewAPI; import fr.xephi.authme.command.CommandHandler; import fr.xephi.authme.datasource.DataSource; @@ -35,9 +33,6 @@ import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.task.CleanupTask; import fr.xephi.authme.task.purge.PurgeService; import fr.xephi.authme.util.ExceptionUtils; - -import java.io.File; - import org.apache.commons.lang.SystemUtils; import org.bukkit.Server; import org.bukkit.command.Command; @@ -48,6 +43,8 @@ import org.bukkit.plugin.java.JavaPlugin; import org.bukkit.plugin.java.JavaPluginLoader; import org.bukkit.scheduler.BukkitScheduler; +import java.io.File; + import static fr.xephi.authme.service.BukkitService.TICKS_PER_MINUTE; import static fr.xephi.authme.util.Utils.isClassLoaded; @@ -160,7 +157,7 @@ public class AuthMe extends JavaPlugin { // Sponsor messages ConsoleLogger.info("Development builds are available on our jenkins, thanks to FastVM.io"); - ConsoleLogger.info("Do you want a good vps for your game server? Look at our sponsor FastVM.io leader " + ConsoleLogger.info("Do you want a good vps for your game server? Look at our sponsor FastVM.io leader " + "as virtual server provider!"); // Successful message diff --git a/src/main/java/fr/xephi/authme/ConsoleLogger.java b/src/main/java/fr/xephi/authme/ConsoleLogger.java index 0d933298..3e8d59a4 100644 --- a/src/main/java/fr/xephi/authme/ConsoleLogger.java +++ b/src/main/java/fr/xephi/authme/ConsoleLogger.java @@ -5,7 +5,7 @@ import fr.xephi.authme.output.LogLevel; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.util.StringUtils; +import fr.xephi.authme.util.ExceptionUtils; import java.io.File; import java.io.FileWriter; @@ -101,7 +101,7 @@ public final class ConsoleLogger { * @param th The Throwable to log */ public static void logException(String message, Throwable th) { - warning(message + " " + StringUtils.formatException(th)); + warning(message + " " + ExceptionUtils.formatException(th)); writeLog(Throwables.getStackTraceAsString(th)); } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommand.java index c737b98d..d790962a 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommand.java @@ -7,6 +7,7 @@ import fr.xephi.authme.service.HelpTranslationGenerator; import org.bukkit.command.CommandSender; import javax.inject.Inject; +import java.io.File; import java.io.IOException; import java.util.List; @@ -24,8 +25,8 @@ public class UpdateHelpMessagesCommand implements ExecutableCommand { @Override public void executeCommand(CommandSender sender, List arguments) { try { - helpTranslationGenerator.updateHelpFile(); - sender.sendMessage("Successfully updated the help file"); + File updatedFile = helpTranslationGenerator.updateHelpFile(); + sender.sendMessage("Successfully updated the help file '" + updatedFile.getName() + "'"); helpMessagesService.reloadMessagesFile(); } catch (IOException e) { sender.sendMessage("Could not update help file: " + e.getMessage()); diff --git a/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java b/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java index 52388521..753650b6 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java +++ b/src/main/java/fr/xephi/authme/data/limbo/AllowFlightRestoreType.java @@ -32,7 +32,7 @@ public enum AllowFlightRestoreType { } }, - /** Always set flight enabled to false. */ + /** The user's flight handling is not modified. */ NOTHING { @Override public void restoreAllowFlight(Player player, LimboPlayer limbo) { 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 78eaa567..fa5d0361 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -14,7 +14,6 @@ import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.bungeecord.BungeeSender; import fr.xephi.authme.service.bungeecord.MessageType; -import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.util.PlayerUtils; @@ -82,9 +81,8 @@ public class AsyncRegister implements AsynchronousProcess { return false; } - boolean isAsync = service.getProperty(PluginSettings.USE_ASYNC_TASKS); - AuthMeAsyncPreRegisterEvent event = new AuthMeAsyncPreRegisterEvent(player, isAsync); - bukkitService.callEvent(event); + AuthMeAsyncPreRegisterEvent event = bukkitService.createAndCallEvent( + isAsync -> new AuthMeAsyncPreRegisterEvent(player, isAsync)); if (!event.canRegister()) { return false; } diff --git a/src/main/java/fr/xephi/authme/security/HashUtils.java b/src/main/java/fr/xephi/authme/security/HashUtils.java index 3578c80f..642081c6 100644 --- a/src/main/java/fr/xephi/authme/security/HashUtils.java +++ b/src/main/java/fr/xephi/authme/security/HashUtils.java @@ -1,6 +1,7 @@ package fr.xephi.authme.security; import java.math.BigInteger; +import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; @@ -78,6 +79,20 @@ public final class HashUtils { return hash.length() > 3 && hash.substring(0, 2).equals("$2"); } + /** + * Checks whether the two strings are equal to each other in a time-constant manner. + * This helps to avoid timing side channel attacks, + * cf. issue #1561. + * + * @param string1 first string + * @param string2 second string + * @return true if the strings are equal to each other, false otherwise + */ + public static boolean isEqual(String string1, String string2) { + return MessageDigest.isEqual( + string1.getBytes(StandardCharsets.UTF_8), string2.getBytes(StandardCharsets.UTF_8)); + } + /** * Hash the message with the given algorithm and return the hash in its hexadecimal notation. * diff --git a/src/main/java/fr/xephi/authme/security/crypts/BCrypt.java b/src/main/java/fr/xephi/authme/security/crypts/BCrypt.java index 02e12d45..8b454c79 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/BCrypt.java +++ b/src/main/java/fr/xephi/authme/security/crypts/BCrypt.java @@ -8,7 +8,7 @@ import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.Usage; import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.HooksSettings; -import fr.xephi.authme.util.StringUtils; +import fr.xephi.authme.util.ExceptionUtils; import javax.inject.Inject; @@ -39,7 +39,7 @@ public class BCrypt implements EncryptionMethod { try { return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash()); } catch (IllegalArgumentException e) { - ConsoleLogger.warning("Bcrypt checkpw() returned " + StringUtils.formatException(e)); + ConsoleLogger.warning("Bcrypt checkpw() returned " + ExceptionUtils.formatException(e)); } return false; } diff --git a/src/main/java/fr/xephi/authme/security/crypts/BCrypt2y.java b/src/main/java/fr/xephi/authme/security/crypts/BCrypt2y.java index cf4807ab..a22a6890 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/BCrypt2y.java +++ b/src/main/java/fr/xephi/authme/security/crypts/BCrypt2y.java @@ -3,6 +3,8 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; + @Recommendation(Usage.RECOMMENDED) public class BCrypt2y extends HexSaltedMethod { @@ -23,7 +25,7 @@ public class BCrypt2y extends HexSaltedMethod { // The salt is the first 29 characters of the hash String salt = hash.substring(0, 29); - return hash.equals(computeHash(password, salt, null)); + return isEqual(hash, computeHash(password, salt, null)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Ipb4.java b/src/main/java/fr/xephi/authme/security/crypts/Ipb4.java index 76289795..c7bfcd65 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Ipb4.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Ipb4.java @@ -2,12 +2,12 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.security.HashUtils; -import fr.xephi.authme.util.RandomStringUtils; import fr.xephi.authme.security.crypts.description.HasSalt; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.Usage; -import fr.xephi.authme.util.StringUtils; +import fr.xephi.authme.util.ExceptionUtils; +import fr.xephi.authme.util.RandomStringUtils; /** @@ -37,7 +37,7 @@ public class Ipb4 implements EncryptionMethod { try { return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash()); } catch (IllegalArgumentException e) { - ConsoleLogger.warning("Bcrypt checkpw() returned " + StringUtils.formatException(e)); + ConsoleLogger.warning("Bcrypt checkpw() returned " + ExceptionUtils.formatException(e)); } return false; } diff --git a/src/main/java/fr/xephi/authme/security/crypts/Joomla.java b/src/main/java/fr/xephi/authme/security/crypts/Joomla.java index 462f5cb2..2ecc1d8d 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Joomla.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Joomla.java @@ -4,6 +4,8 @@ import fr.xephi.authme.security.HashUtils; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; + @Recommendation(Usage.ACCEPTABLE) public class Joomla extends HexSaltedMethod { @@ -16,7 +18,7 @@ public class Joomla extends HexSaltedMethod { public boolean comparePassword(String password, HashedPassword hashedPassword, String unusedName) { String hash = hashedPassword.getHash(); String[] hashParts = hash.split(":"); - return hashParts.length == 2 && hash.equals(computeHash(password, hashParts[1], null)); + return hashParts.length == 2 && isEqual(hash, computeHash(password, hashParts[1], null)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Md5vB.java b/src/main/java/fr/xephi/authme/security/crypts/Md5vB.java index c244ec49..00656964 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Md5vB.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Md5vB.java @@ -1,5 +1,6 @@ package fr.xephi.authme.security.crypts; +import static fr.xephi.authme.security.HashUtils.isEqual; import static fr.xephi.authme.security.HashUtils.md5; public class Md5vB extends HexSaltedMethod { @@ -13,7 +14,7 @@ public class Md5vB extends HexSaltedMethod { public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { String hash = hashedPassword.getHash(); String[] line = hash.split("\\$"); - return line.length == 4 && hash.equals(computeHash(password, line[2], name)); + return line.length == 4 && isEqual(hash, computeHash(password, line[2], name)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java index 70ac322d..2d641706 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java +++ b/src/main/java/fr/xephi/authme/security/crypts/PhpBB.java @@ -10,6 +10,8 @@ import fr.xephi.authme.security.crypts.description.Usage; import java.io.UnsupportedEncodingException; import java.security.MessageDigest; +import static fr.xephi.authme.security.HashUtils.isEqual; + /** * Encryption method compatible with phpBB3. *

@@ -43,7 +45,7 @@ public class PhpBB implements EncryptionMethod { } else if (hash.length() == 34) { return PhpassSaltedMd5.phpbb_check_hash(password, hash); } else { - return PhpassSaltedMd5.md5(password).equals(hash); + return isEqual(hash, PhpassSaltedMd5.md5(password)); } } @@ -153,7 +155,7 @@ public class PhpBB implements EncryptionMethod { } private static boolean phpbb_check_hash(String password, String hash) { - return _hash_crypt_private(password, hash).equals(hash); + return isEqual(hash, _hash_crypt_private(password, hash)); // #1561: fix timing issue } } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/SeparateSaltMethod.java b/src/main/java/fr/xephi/authme/security/crypts/SeparateSaltMethod.java index d0dacda4..c0ec13dd 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/SeparateSaltMethod.java +++ b/src/main/java/fr/xephi/authme/security/crypts/SeparateSaltMethod.java @@ -1,5 +1,7 @@ package fr.xephi.authme.security.crypts; +import static fr.xephi.authme.security.HashUtils.isEqual; + /** * Common supertype for encryption methods which store their salt separately from the hash. */ @@ -19,7 +21,7 @@ public abstract class SeparateSaltMethod implements EncryptionMethod { @Override public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { - return hashedPassword.getHash().equals(computeHash(password, hashedPassword.getSalt(), null)); + return isEqual(hashedPassword.getHash(), computeHash(password, hashedPassword.getSalt(), null)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Sha256.java b/src/main/java/fr/xephi/authme/security/crypts/Sha256.java index 1b77a2e4..ce6b2549 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Sha256.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Sha256.java @@ -3,6 +3,7 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; import static fr.xephi.authme.security.HashUtils.sha256; @Recommendation(Usage.RECOMMENDED) @@ -14,10 +15,10 @@ public class Sha256 extends HexSaltedMethod { } @Override - public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) { + public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { String hash = hashedPassword.getHash(); String[] line = hash.split("\\$"); - return line.length == 4 && hash.equals(computeHash(password, line[2], "")); + return line.length == 4 && isEqual(hash, computeHash(password, line[2], name)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Smf.java b/src/main/java/fr/xephi/authme/security/crypts/Smf.java index 24d28fe6..e24c1b83 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Smf.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Smf.java @@ -7,6 +7,8 @@ import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.Usage; import fr.xephi.authme.util.RandomStringUtils; +import static fr.xephi.authme.security.HashUtils.isEqual; + /** * Hashing algorithm for SMF forums. *

@@ -32,7 +34,7 @@ public class Smf implements EncryptionMethod { @Override public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { - return computeHash(password, null, name).equals(hashedPassword.getHash()); + return isEqual(hashedPassword.getHash(), computeHash(password, null, name)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/UnsaltedMethod.java b/src/main/java/fr/xephi/authme/security/crypts/UnsaltedMethod.java index a8f2040e..33815ec7 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/UnsaltedMethod.java +++ b/src/main/java/fr/xephi/authme/security/crypts/UnsaltedMethod.java @@ -5,6 +5,8 @@ import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; + /** * Common type for encryption methods which do not use any salt whatsoever. */ @@ -26,7 +28,7 @@ public abstract class UnsaltedMethod implements EncryptionMethod { @Override public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { - return hashedPassword.getHash().equals(computeHash(password)); + return isEqual(hashedPassword.getHash(), computeHash(password)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/UsernameSaltMethod.java b/src/main/java/fr/xephi/authme/security/crypts/UsernameSaltMethod.java index 23101e22..f5930fcf 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/UsernameSaltMethod.java +++ b/src/main/java/fr/xephi/authme/security/crypts/UsernameSaltMethod.java @@ -5,6 +5,8 @@ import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.SaltType; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; + /** * Common supertype of encryption methods that use a player's username * (or something based on it) as embedded salt. @@ -23,7 +25,7 @@ public abstract class UsernameSaltMethod implements EncryptionMethod { @Override public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { - return hashedPassword.getHash().equals(computeHash(password, name).getHash()); + return isEqual(hashedPassword.getHash(), computeHash(password, name).getHash()); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Wbb4.java b/src/main/java/fr/xephi/authme/security/crypts/Wbb4.java index d1d4953d..f396c5d8 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Wbb4.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Wbb4.java @@ -3,6 +3,7 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; import static fr.xephi.authme.security.crypts.BCryptService.hashpw; @Recommendation(Usage.RECOMMENDED) @@ -14,12 +15,12 @@ public class Wbb4 extends HexSaltedMethod { } @Override - public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) { + public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { if (hashedPassword.getHash().length() != 60) { return false; } String salt = hashedPassword.getHash().substring(0, 29); - return computeHash(password, salt, null).equals(hashedPassword.getHash()); + return isEqual(hashedPassword.getHash(), computeHash(password, salt, name)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/Wordpress.java b/src/main/java/fr/xephi/authme/security/crypts/Wordpress.java index 768b92c5..f70c0949 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/Wordpress.java +++ b/src/main/java/fr/xephi/authme/security/crypts/Wordpress.java @@ -12,6 +12,8 @@ import java.security.MessageDigest; import java.security.SecureRandom; import java.util.Arrays; +import static fr.xephi.authme.security.HashUtils.isEqual; + @Recommendation(Usage.ACCEPTABLE) @HasSalt(value = SaltType.TEXT, length = 9) // Note ljacqu 20151228: Wordpress is actually a salted algorithm but salt generation is handled internally @@ -115,7 +117,7 @@ public class Wordpress extends UnsaltedMethod { public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { String hash = hashedPassword.getHash(); String comparedHash = crypt(password, hash); - return comparedHash.equals(hash); + return isEqual(hash, comparedHash); } } diff --git a/src/main/java/fr/xephi/authme/security/crypts/XAuth.java b/src/main/java/fr/xephi/authme/security/crypts/XAuth.java index 9f921b6a..62f2e0d7 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/XAuth.java +++ b/src/main/java/fr/xephi/authme/security/crypts/XAuth.java @@ -3,6 +3,8 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.security.crypts.description.Recommendation; import fr.xephi.authme.security.crypts.description.Usage; +import static fr.xephi.authme.security.HashUtils.isEqual; + @Recommendation(Usage.RECOMMENDED) public class XAuth extends HexSaltedMethod { @@ -23,14 +25,14 @@ public class XAuth extends HexSaltedMethod { } @Override - public boolean comparePassword(String password, HashedPassword hashedPassword, String playerName) { + public boolean comparePassword(String password, HashedPassword hashedPassword, String name) { String hash = hashedPassword.getHash(); int saltPos = password.length() >= hash.length() ? hash.length() - 1 : password.length(); if (saltPos + 12 > hash.length()) { return false; } String salt = hash.substring(saltPos, saltPos + 12); - return hash.equals(computeHash(password, salt, null)); + return isEqual(hash, computeHash(password, salt, name)); } @Override diff --git a/src/main/java/fr/xephi/authme/security/crypts/XfBCrypt.java b/src/main/java/fr/xephi/authme/security/crypts/XfBCrypt.java index 3ef4e430..846807e6 100644 --- a/src/main/java/fr/xephi/authme/security/crypts/XfBCrypt.java +++ b/src/main/java/fr/xephi/authme/security/crypts/XfBCrypt.java @@ -2,7 +2,7 @@ package fr.xephi.authme.security.crypts; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.security.HashUtils; -import fr.xephi.authme.util.StringUtils; +import fr.xephi.authme.util.ExceptionUtils; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -32,7 +32,7 @@ public class XfBCrypt implements EncryptionMethod { try { return HashUtils.isValidBcryptHash(hash.getHash()) && BCryptService.checkpw(password, hash.getHash()); } catch (IllegalArgumentException e) { - ConsoleLogger.warning("XfBCrypt checkpw() returned " + StringUtils.formatException(e)); + ConsoleLogger.warning("XfBCrypt checkpw() returned " + ExceptionUtils.formatException(e)); } return false; } diff --git a/src/main/java/fr/xephi/authme/service/HelpTranslationGenerator.java b/src/main/java/fr/xephi/authme/service/HelpTranslationGenerator.java index 6ecd0549..21407b4f 100644 --- a/src/main/java/fr/xephi/authme/service/HelpTranslationGenerator.java +++ b/src/main/java/fr/xephi/authme/service/HelpTranslationGenerator.java @@ -44,15 +44,17 @@ public class HelpTranslationGenerator { /** * Updates the help file to contain entries for all commands. * + * @return the help file that has been updated * @throws IOException if the help file cannot be written to */ - public void updateHelpFile() throws IOException { + public File updateHelpFile() throws IOException { String languageCode = settings.getProperty(PluginSettings.MESSAGES_LANGUAGE); File helpFile = new File(dataFolder, "messages/help_" + languageCode + ".yml"); Map helpEntries = generateHelpMessageEntries(); String helpEntriesYaml = exportToYaml(helpEntries); Files.write(helpFile.toPath(), helpEntriesYaml.getBytes(), StandardOpenOption.TRUNCATE_EXISTING); + return helpFile; } private static String exportToYaml(Map helpEntries) { diff --git a/src/main/java/fr/xephi/authme/util/ExceptionUtils.java b/src/main/java/fr/xephi/authme/util/ExceptionUtils.java index 6a5adde6..fd5ae885 100644 --- a/src/main/java/fr/xephi/authme/util/ExceptionUtils.java +++ b/src/main/java/fr/xephi/authme/util/ExceptionUtils.java @@ -33,4 +33,14 @@ public final class ExceptionUtils { } return null; } + + /** + * Format the information from a Throwable as string, retaining the type and its message. + * + * @param th the throwable to process + * @return string with the type of the Throwable and its message, e.g. "[IOException]: Could not open stream" + */ + public static String formatException(Throwable th) { + return "[" + th.getClass().getSimpleName() + "]: " + th.getMessage(); + } } diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index 1f200c0f..5c861300 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -66,17 +66,6 @@ public final class StringUtils { return str == null || str.trim().isEmpty(); } - /** - * Format the information from a Throwable as string, retaining the type and its message. - * - * @param th The throwable to process - * - * @return String with the type of the Throwable and its message, e.g. "[IOException]: Could not open stream" - */ - public static String formatException(Throwable th) { - return "[" + th.getClass().getSimpleName() + "]: " + th.getMessage(); - } - /** * Check that the given needle is in the middle of the haystack, i.e. that the haystack * contains the needle and that it is not at the very start or end. diff --git a/src/test/java/fr/xephi/authme/CodeClimateConfigTest.java b/src/test/java/fr/xephi/authme/CodeClimateConfigTest.java index 7eff3a72..e645922e 100644 --- a/src/test/java/fr/xephi/authme/CodeClimateConfigTest.java +++ b/src/test/java/fr/xephi/authme/CodeClimateConfigTest.java @@ -30,21 +30,9 @@ public class CodeClimateConfigTest { assertThat(excludePaths, not(empty())); removeTestsExclusionOrThrow(excludePaths); for (String path : excludePaths) { - verifySourceFileExists(path); - } - } - - private static void verifySourceFileExists(String path) { - // Note ljacqu 20170323: In the future, we could have legitimate exclusions that don't fulfill these checks, - // in which case this test needs to be adapted accordingly. - if (!path.startsWith(TestHelper.SOURCES_FOLDER)) { - fail("Unexpected path '" + path + "': expected to start with sources folder"); - } else if (!path.endsWith(".java")) { - fail("Expected path '" + path + "' to end with '.java'"); - } - - if (!new File(path).exists()) { - fail("Path '" + path + "' does not exist!"); + if (!new File(path).exists()) { + fail("Path '" + path + "' does not exist!"); + } } } diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommandTest.java new file mode 100644 index 00000000..94e53ff2 --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/authme/UpdateHelpMessagesCommandTest.java @@ -0,0 +1,70 @@ +package fr.xephi.authme.command.executable.authme; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.command.help.HelpMessagesService; +import fr.xephi.authme.service.HelpTranslationGenerator; +import org.bukkit.command.CommandSender; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.io.File; +import java.io.IOException; +import java.util.Collections; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link UpdateHelpMessagesCommand}. + */ +@RunWith(MockitoJUnitRunner.class) +public class UpdateHelpMessagesCommandTest { + + @InjectMocks + private UpdateHelpMessagesCommand command; + + @Mock + private HelpTranslationGenerator helpTranslationGenerator; + @Mock + private HelpMessagesService helpMessagesService; + + @BeforeClass + public static void setUpLogger() { + TestHelper.setupLogger(); + } + + @Test + public void shouldUpdateHelpMessage() throws IOException { + // given + File updatedFile = new File("some/path/help_xx.yml"); + given(helpTranslationGenerator.updateHelpFile()).willReturn(updatedFile); + CommandSender sender = mock(CommandSender.class); + + // when + command.executeCommand(sender, Collections.emptyList()); + + // then + verify(helpMessagesService).reloadMessagesFile(); + verify(sender).sendMessage("Successfully updated the help file 'help_xx.yml'"); + } + + @Test + public void shouldCatchAndReportException() throws IOException { + // given + given(helpTranslationGenerator.updateHelpFile()).willThrow(new IOException("Couldn't do the thing")); + CommandSender sender = mock(CommandSender.class); + + // when + command.executeCommand(sender, Collections.emptyList()); + + // then + verify(sender).sendMessage("Could not update help file: Couldn't do the thing"); + verifyZeroInteractions(helpMessagesService); + } +} diff --git a/src/test/java/fr/xephi/authme/message/YamlTextFileCheckerTest.java b/src/test/java/fr/xephi/authme/message/YamlTextFileCheckerTest.java index 9fbd27bd..b04259b2 100644 --- a/src/test/java/fr/xephi/authme/message/YamlTextFileCheckerTest.java +++ b/src/test/java/fr/xephi/authme/message/YamlTextFileCheckerTest.java @@ -2,6 +2,7 @@ package fr.xephi.authme.message; import fr.xephi.authme.TestHelper; import fr.xephi.authme.command.help.HelpSection; +import fr.xephi.authme.util.ExceptionUtils; import fr.xephi.authme.util.StringUtils; import org.bukkit.configuration.file.YamlConfiguration; import org.junit.BeforeClass; @@ -85,7 +86,7 @@ public class YamlTextFileCheckerTest { errors.add("Message for '" + mandatoryKey + "' is empty"); } } catch (Exception e) { - errors.add("Could not load file: " + StringUtils.formatException(e)); + errors.add("Could not load file: " + ExceptionUtils.formatException(e)); } } } diff --git a/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java index 59c384bb..029ff90c 100644 --- a/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java +++ b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java @@ -12,7 +12,6 @@ import fr.xephi.authme.process.register.executors.RegistrationMethod; import fr.xephi.authme.process.register.executors.TwoFactorRegisterParams; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; -import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Player; @@ -21,11 +20,11 @@ import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; -import org.mockito.stubbing.Answer; + +import java.util.function.Function; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; @@ -108,7 +107,7 @@ public class AsyncRegisterTest { @Test @SuppressWarnings("unchecked") - public void shouldStopForFailedExecutorCheck() { + public void shouldStopForCanceledEvent() { // given String name = "edbert"; Player player = mockPlayerWithName(name); @@ -116,14 +115,13 @@ public class AsyncRegisterTest { given(playerCache.isAuthenticated(name)).willReturn(false); given(commonService.getProperty(RegistrationSettings.IS_ENABLED)).willReturn(true); given(dataSource.isAuthAvailable(name)).willReturn(false); - given(commonService.getProperty(PluginSettings.USE_ASYNC_TASKS)).willReturn(true); RegistrationExecutor executor = mock(RegistrationExecutor.class); TwoFactorRegisterParams params = TwoFactorRegisterParams.of(player); singletonStoreWillReturn(registrationExecutorStore, executor); - doAnswer((Answer) invocation -> { - ((AuthMeAsyncPreRegisterEvent) invocation.getArgument(0)).setCanRegister(false); - return null; - }).when(bukkitService).callEvent(any(AuthMeAsyncPreRegisterEvent.class)); + + AuthMeAsyncPreRegisterEvent canceledEvent = new AuthMeAsyncPreRegisterEvent(player, true); + canceledEvent.setCanRegister(false); + given(bukkitService.createAndCallEvent(any(Function.class))).willReturn(canceledEvent); // when asyncRegister.register(RegistrationMethod.TWO_FACTOR_REGISTRATION, params); @@ -134,7 +132,7 @@ public class AsyncRegisterTest { @Test @SuppressWarnings("unchecked") - public void shouldStopForCancelledEvent() { + public void shouldStopForFailedExecutorCheck() { // given String name = "edbert"; Player player = mockPlayerWithName(name); @@ -143,12 +141,14 @@ public class AsyncRegisterTest { given(commonService.getProperty(RegistrationSettings.IS_ENABLED)).willReturn(true); given(commonService.getProperty(RestrictionSettings.MAX_REGISTRATION_PER_IP)).willReturn(0); given(dataSource.isAuthAvailable(name)).willReturn(false); - given(commonService.getProperty(PluginSettings.USE_ASYNC_TASKS)).willReturn(true); RegistrationExecutor executor = mock(RegistrationExecutor.class); TwoFactorRegisterParams params = TwoFactorRegisterParams.of(player); given(executor.isRegistrationAdmitted(params)).willReturn(false); singletonStoreWillReturn(registrationExecutorStore, executor); + given(bukkitService.createAndCallEvent(any(Function.class))) + .willReturn(new AuthMeAsyncPreRegisterEvent(player, false)); + // when asyncRegister.register(RegistrationMethod.TWO_FACTOR_REGISTRATION, params); diff --git a/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java b/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java index fc9fd0d6..e59dae64 100644 --- a/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/security/HashAlgorithmIntegrationTest.java @@ -2,6 +2,7 @@ package fr.xephi.authme.security; import ch.jalu.injector.Injector; import ch.jalu.injector.InjectorBuilder; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.security.crypts.Argon2; import fr.xephi.authme.security.crypts.EncryptionMethod; import fr.xephi.authme.security.crypts.HashedPassword; @@ -40,6 +41,7 @@ public class HashAlgorithmIntegrationTest { given(settings.getProperty(SecuritySettings.PBKDF2_NUMBER_OF_ROUNDS)).willReturn(10_000); injector = new InjectorBuilder().addDefaultHandlers("fr.xephi.authme").create(); injector.register(Settings.class, settings); + TestHelper.setupLogger(); } @Test diff --git a/src/test/java/fr/xephi/authme/security/HashUtilsTest.java b/src/test/java/fr/xephi/authme/security/HashUtilsTest.java index 5c1fda22..440e748a 100644 --- a/src/test/java/fr/xephi/authme/security/HashUtilsTest.java +++ b/src/test/java/fr/xephi/authme/security/HashUtilsTest.java @@ -123,4 +123,13 @@ public class HashUtilsTest { assertThat(HashUtils.isValidBcryptHash("#2ae5fc78"), equalTo(false)); } + @Test + public void shouldCompareStrings() { + // given / when / then + assertThat(HashUtils.isEqual("test", "test"), equalTo(true)); + assertThat(HashUtils.isEqual("test", "Test"), equalTo(false)); + assertThat(HashUtils.isEqual("1234", "1234."), equalTo(false)); + assertThat(HashUtils.isEqual("ພາສາຫວຽດນາມ", "ພາສາຫວຽດນາມ"), equalTo(true)); + assertThat(HashUtils.isEqual("test", "tëst"), equalTo(false)); + } } diff --git a/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java b/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java index 8685d7f3..9f60c53a 100644 --- a/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/ExceptionUtilsTest.java @@ -4,8 +4,10 @@ import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.TestHelper; import org.junit.Test; +import java.net.MalformedURLException; import java.util.ConcurrentModificationException; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertThat; @@ -58,4 +60,16 @@ public class ExceptionUtilsTest { // given / when / then TestHelper.validateHasOnlyPrivateEmptyConstructor(ExceptionUtils.class); } + + @Test + public void shouldFormatException() { + // given + MalformedURLException ex = new MalformedURLException("Unrecognized URL format"); + + // when + String result = ExceptionUtils.formatException(ex); + + // then + assertThat(result, equalTo("[MalformedURLException]: Unrecognized URL format")); + } } diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java index 7111f81b..76e7ae75 100644 --- a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -3,8 +3,6 @@ package fr.xephi.authme.util; import fr.xephi.authme.TestHelper; import org.junit.Test; -import java.net.MalformedURLException; - import static java.util.Arrays.asList; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; @@ -63,18 +61,6 @@ public class StringUtilsTest { assertFalse(StringUtils.isEmpty(" test")); } - @Test - public void shouldFormatException() { - // given - MalformedURLException ex = new MalformedURLException("Unrecognized URL format"); - - // when - String result = StringUtils.formatException(ex); - - // then - assertThat(result, equalTo("[MalformedURLException]: Unrecognized URL format")); - } - @Test public void shouldGetDifferenceWithNullString() { // given/when/then