diff --git a/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java b/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index 3c6264cc10..38000ac130 100644 --- a/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java +++ b/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -4,7 +4,6 @@ import fr.xephi.authme.process.Management; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.ValidationService; -import fr.xephi.authme.service.ValidationService.ValidationResult; import org.bukkit.command.CommandSender; import javax.inject.Inject; @@ -31,11 +30,12 @@ public void executeCommand(CommandSender sender, List arguments) { final String playerPass = arguments.get(1); // Validate the password - ValidationResult validationResult = validationService.validatePassword(playerPass, playerName); - if (validationResult.hasError()) { - commonService.send(sender, validationResult.getMessageKey(), validationResult.getArgs()); - } else { - management.performPasswordChangeAsAdmin(sender, playerName, playerPass); - } + validationService.validatePasswordAsync(playerPass, playerName).thenAccept(validationResult -> { + if (validationResult.hasError()) { + commonService.send(sender, validationResult.getMessageKey(), validationResult.getArgs()); + } else { + management.performPasswordChangeAsAdmin(sender, playerName, playerPass); + } + }); } } diff --git a/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java b/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java index ed108014d7..697df77877 100644 --- a/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java +++ b/authme-core/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java @@ -11,7 +11,6 @@ import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.ValidationService; -import fr.xephi.authme.service.ValidationService.ValidationResult; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; @@ -49,37 +48,39 @@ public void executeCommand(final CommandSender sender, List arguments) { final String playerNameLowerCase = playerName.toLowerCase(Locale.ROOT); // Command logic - ValidationResult passwordValidation = validationService.validatePassword(playerPass, playerName); - if (passwordValidation.hasError()) { - commonService.send(sender, passwordValidation.getMessageKey(), passwordValidation.getArgs()); - return; - } - - bukkitService.runTaskOptionallyAsync(() -> { - if (dataSource.isAuthAvailable(playerNameLowerCase)) { - commonService.send(sender, MessageKey.NAME_ALREADY_REGISTERED); + validationService.validatePasswordAsync(playerPass, playerName).thenAccept(passwordValidation -> { + if (passwordValidation.hasError()) { + commonService.send(sender, passwordValidation.getMessageKey(), passwordValidation.getArgs()); return; } - HashedPassword hashedPassword = passwordSecurity.computeHash(playerPass, playerNameLowerCase); - PlayerAuth auth = PlayerAuth.builder() - .name(playerNameLowerCase) - .realName(playerName) - .password(hashedPassword) - .registrationDate(System.currentTimeMillis()) - .build(); - if (!dataSource.saveAuth(auth)) { - commonService.send(sender, MessageKey.ERROR); - return; - } + bukkitService.runTaskOptionallyAsync(() -> { + if (dataSource.isAuthAvailable(playerNameLowerCase)) { + commonService.send(sender, MessageKey.NAME_ALREADY_REGISTERED); + return; + } + HashedPassword hashedPassword = passwordSecurity.computeHash(playerPass, playerNameLowerCase); + PlayerAuth auth = PlayerAuth.builder() + .name(playerNameLowerCase) + .realName(playerName) + .password(hashedPassword) + .registrationDate(System.currentTimeMillis()) + .build(); - commonService.send(sender, MessageKey.REGISTER_SUCCESS); - logger.info(sender.getName() + " registered " + playerName); - final Player player = bukkitService.getPlayerExact(playerName); - if (player != null) { - bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(player, - () -> player.kickPlayer(commonService.retrieveSingleMessage(player, MessageKey.KICK_FOR_ADMIN_REGISTER))); - } + if (!dataSource.saveAuth(auth)) { + commonService.send(sender, MessageKey.ERROR); + return; + } + + commonService.send(sender, MessageKey.REGISTER_SUCCESS); + logger.info(sender.getName() + " registered " + playerName); + final Player player = bukkitService.getPlayerExact(playerName); + if (player != null) { + bukkitService.scheduleSyncTaskFromOptionallyAsyncTask(player, + () -> player.kickPlayer( + commonService.retrieveSingleMessage(player, MessageKey.KICK_FOR_ADMIN_REGISTER))); + } + }); }); } } diff --git a/authme-core/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java b/authme-core/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java index 72ff55bc0b..f710a08f00 100644 --- a/authme-core/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java +++ b/authme-core/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java @@ -7,7 +7,6 @@ import fr.xephi.authme.process.Management; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.ValidationService; -import fr.xephi.authme.service.ValidationService.ValidationResult; import org.bukkit.entity.Player; import javax.inject.Inject; @@ -54,13 +53,13 @@ public void runCommand(Player player, List arguments) { String newPassword = arguments.get(1); // Make sure the password is allowed - ValidationResult passwordValidation = validationService.validatePassword(newPassword, name); - if (passwordValidation.hasError()) { - commonService.send(player, passwordValidation.getMessageKey(), passwordValidation.getArgs()); - return; - } - - management.performPasswordChange(player, oldPassword, newPassword); + validationService.validatePasswordAsync(newPassword, name).thenAccept(passwordValidation -> { + if (passwordValidation.hasError()) { + commonService.send(player, passwordValidation.getMessageKey(), passwordValidation.getArgs()); + } else { + management.performPasswordChange(player, oldPassword, newPassword); + } + }); } @Override diff --git a/authme-core/src/main/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommand.java b/authme-core/src/main/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommand.java index 734fc064bf..4be9ea2cab 100644 --- a/authme-core/src/main/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommand.java +++ b/authme-core/src/main/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommand.java @@ -10,7 +10,6 @@ import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.PasswordRecoveryService; import fr.xephi.authme.service.ValidationService; -import fr.xephi.authme.service.ValidationService.ValidationResult; import org.bukkit.entity.Player; import javax.inject.Inject; @@ -44,16 +43,17 @@ protected void runCommand(Player player, List arguments) { String name = player.getName(); String password = arguments.get(0); - ValidationResult result = validationService.validatePassword(password, name); - if (!result.hasError()) { - HashedPassword hashedPassword = passwordSecurity.computeHash(password, name); - dataSource.updatePassword(name, hashedPassword); - recoveryService.removeFromSuccessfulRecovery(player); - logger.info("Player '" + name + "' has changed their password from recovery"); - commonService.send(player, MessageKey.PASSWORD_CHANGED_SUCCESS); - } else { - commonService.send(player, result.getMessageKey(), result.getArgs()); - } + validationService.validatePasswordAsync(password, name).thenAccept(result -> { + if (!result.hasError()) { + HashedPassword hashedPassword = passwordSecurity.computeHash(password, name); + dataSource.updatePassword(name, hashedPassword); + recoveryService.removeFromSuccessfulRecovery(player); + logger.info("Player '" + name + "' has changed their password from recovery"); + commonService.send(player, MessageKey.PASSWORD_CHANGED_SUCCESS); + } else { + commonService.send(player, result.getMessageKey(), result.getArgs()); + } + }); } else { commonService.send(player, MessageKey.CHANGE_PASSWORD_EXPIRED); } diff --git a/authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java b/authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java index e585dc0d86..32d169735e 100644 --- a/authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java +++ b/authme-core/src/main/java/fr/xephi/authme/message/MessageKey.java @@ -80,6 +80,9 @@ public enum MessageKey { /** The chosen password isn't safe, please choose another one... */ PASSWORD_UNSAFE_ERROR("password.unsafe_password"), + /** Your chosen password is not secure. It has been seen %pwned_count times before! */ + PASSWORD_PWNED_ERROR("password.pwned_password", "%pwned_count"), + /** Your password contains illegal characters. Allowed chars: %valid_chars */ PASSWORD_CHARACTERS_ERROR("password.forbidden_characters", "%valid_chars"), diff --git a/authme-core/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/authme-core/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index f5fe3be654..f36d3bc1ee 100644 --- a/authme-core/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/authme-core/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -7,11 +7,13 @@ import fr.xephi.authme.events.AuthMeAsyncPreRegisterEvent; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.process.AsynchronousProcess; +import fr.xephi.authme.process.register.executors.AbstractPasswordRegisterParams; import fr.xephi.authme.process.register.executors.RegistrationExecutor; import fr.xephi.authme.process.register.executors.RegistrationMethod; import fr.xephi.authme.process.register.executors.RegistrationParameters; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; +import fr.xephi.authme.service.ValidationService; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.util.InternetProtocolUtils; @@ -21,6 +23,7 @@ import javax.inject.Inject; import java.util.List; import java.util.Locale; +import java.util.concurrent.CompletableFuture; import static fr.xephi.authme.permission.PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS; @@ -38,6 +41,8 @@ public class AsyncRegister implements AsynchronousProcess { @Inject private CommonService service; @Inject + private ValidationService validationService; + @Inject private SingletonStore registrationExecutorFactory; AsyncRegister() { @@ -54,11 +59,30 @@ public

void register(RegistrationMethod

va if (preRegisterCheck(variant, parameters.getPlayer())) { RegistrationExecutor

executor = registrationExecutorFactory.getSingleton(variant.getExecutorClass()); if (executor.isRegistrationAdmitted(parameters)) { - executeRegistration(parameters, executor); + validatePwnedPassword(parameters).thenAccept(passwordValidation -> { + if (passwordValidation.hasError()) { + service.send(parameters.getPlayer(), passwordValidation.getMessageKey(), + passwordValidation.getArgs()); + } else { + executeRegistration(parameters, executor); + } + }); } } } + private CompletableFuture validatePwnedPassword(RegistrationParameters parameters) { + if (!(parameters instanceof AbstractPasswordRegisterParams)) { + return CompletableFuture.completedFuture(new ValidationService.ValidationResult()); + } + + AbstractPasswordRegisterParams passwordParams = (AbstractPasswordRegisterParams) parameters; + if (passwordParams.getPassword() == null) { + return CompletableFuture.completedFuture(new ValidationService.ValidationResult()); + } + return validationService.validatePasswordAsync(passwordParams.getPassword(), passwordParams.getPlayerName()); + } + /** * Checks if the player is able to register, in that case the {@link AuthMeAsyncPreRegisterEvent} is invoked. * diff --git a/authme-core/src/main/java/fr/xephi/authme/service/PwnedPasswordService.java b/authme-core/src/main/java/fr/xephi/authme/service/PwnedPasswordService.java new file mode 100644 index 0000000000..f914c0bb88 --- /dev/null +++ b/authme-core/src/main/java/fr/xephi/authme/service/PwnedPasswordService.java @@ -0,0 +1,109 @@ +package fr.xephi.authme.service; + +import com.google.common.annotations.VisibleForTesting; +import fr.xephi.authme.ConsoleLogger; +import fr.xephi.authme.output.ConsoleLoggerFactory; +import fr.xephi.authme.security.HashUtils; +import fr.xephi.authme.security.MessageDigestAlgorithm; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStreamReader; +import java.math.BigInteger; +import java.net.HttpURLConnection; +import java.net.URI; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.Locale; +import java.util.OptionalLong; +import java.util.stream.Collectors; + +/** + * Queries the Have I Been Pwned Pwned Passwords range API. + */ +public class PwnedPasswordService { + + private static final String RANGE_API_URL = "https://api.pwnedpasswords.com/range/"; + private static final String USER_AGENT = "AuthMeReloaded"; + private static final int HASH_PREFIX_LENGTH = 5; + private static final int CONNECT_TIMEOUT_MILLIS = 5_000; + private static final int READ_TIMEOUT_MILLIS = 5_000; + + private final ConsoleLogger logger = ConsoleLoggerFactory.get(PwnedPasswordService.class); + + /** + * Returns how many times the password appears in the Pwned Passwords database. + * + * @param password the password to check + * @return the count, 0 when absent, or empty if the API could not be queried + */ + public OptionalLong getPwnedCount(String password) { + String hash = sha1Utf8(password).toUpperCase(Locale.ROOT); + String hashPrefix = hash.substring(0, HASH_PREFIX_LENGTH); + String hashSuffix = hash.substring(HASH_PREFIX_LENGTH); + + try { + return parsePwnedCount(hashSuffix, requestHashRange(hashPrefix)); + } catch (IOException e) { + logger.debug("Could not query the Pwned Passwords API: {0}", e.getMessage()); + return OptionalLong.empty(); + } + } + + @VisibleForTesting + protected String requestHashRange(String hashPrefix) throws IOException { + HttpURLConnection connection = (HttpURLConnection) URI.create(RANGE_API_URL + hashPrefix) + .toURL() + .openConnection(); + connection.setRequestMethod("GET"); + connection.setRequestProperty("User-Agent", USER_AGENT); + connection.setRequestProperty("Add-Padding", "true"); + connection.setConnectTimeout(CONNECT_TIMEOUT_MILLIS); + connection.setReadTimeout(READ_TIMEOUT_MILLIS); + + try { + int responseCode = connection.getResponseCode(); + if (responseCode != HttpURLConnection.HTTP_OK) { + throw new IOException("HTTP " + responseCode); + } + + try (BufferedReader reader = new BufferedReader( + new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8))) { + return reader.lines().collect(Collectors.joining("\n")); + } + } finally { + connection.disconnect(); + } + } + + private OptionalLong parsePwnedCount(String searchedHashSuffix, String response) { + String[] entries = response.split("\\R"); + for (String entry : entries) { + int delimiterIndex = entry.indexOf(':'); + if (delimiterIndex < 0) { + continue; + } + + String hashSuffix = entry.substring(0, delimiterIndex); + if (hashSuffix.equalsIgnoreCase(searchedHashSuffix)) { + return parseCount(entry.substring(delimiterIndex + 1)); + } + } + return OptionalLong.of(0); + } + + private OptionalLong parseCount(String count) { + try { + return OptionalLong.of(Long.parseLong(count.trim())); + } catch (NumberFormatException e) { + logger.debug("Could not parse Pwned Passwords count: {0}", count); + return OptionalLong.empty(); + } + } + + private static String sha1Utf8(String password) { + MessageDigest digest = HashUtils.getDigest(MessageDigestAlgorithm.SHA1); + byte[] hashedPassword = digest.digest(password.getBytes(StandardCharsets.UTF_8)); + return String.format("%0" + (hashedPassword.length << 1) + "x", new BigInteger(1, hashedPassword)); + } +} diff --git a/authme-core/src/main/java/fr/xephi/authme/service/ValidationService.java b/authme-core/src/main/java/fr/xephi/authme/service/ValidationService.java index ad99e066f4..a118bfec75 100644 --- a/authme-core/src/main/java/fr/xephi/authme/service/ValidationService.java +++ b/authme-core/src/main/java/fr/xephi/authme/service/ValidationService.java @@ -28,7 +28,9 @@ import java.util.Collection; import java.util.List; import java.util.Locale; +import java.util.OptionalLong; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.regex.Pattern; import static fr.xephi.authme.util.StringUtils.isInsideString; @@ -48,6 +50,10 @@ public class ValidationService implements Reloadable { private PermissionsManager permissionsManager; @Inject private GeoIpService geoIpService; + @Inject + private PwnedPasswordService pwnedPasswordService; + @Inject + private BukkitService bukkitService; private Pattern passwordRegex; private Multimap restrictedNames; @@ -86,6 +92,41 @@ public ValidationResult validatePassword(String password, String username) { return new ValidationResult(); } + /** + * Verifies whether a password is valid, including the optional Pwned Passwords API check. + * + * @param password the password to verify + * @param username the username the password is associated with + * @return the validation result future + */ + public CompletableFuture validatePasswordAsync(String password, String username) { + ValidationResult validationResult = validatePassword(password, username); + if (validationResult.hasError() || !settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)) { + return CompletableFuture.completedFuture(validationResult); + } + + CompletableFuture future = new CompletableFuture<>(); + bukkitService.runTaskAsynchronously(() -> { + try { + future.complete(validatePwnedPassword(password)); + } catch (RuntimeException e) { + future.completeExceptionally(e); + } catch (Error e) { + future.completeExceptionally(e); + } + }); + return future; + } + + private ValidationResult validatePwnedPassword(String password) { + OptionalLong pwnedCount = pwnedPasswordService.getPwnedCount(password); + int threshold = settings.getProperty(SecuritySettings.PWNED_PASSWORD_CHECK_THRESHOLD); + if (pwnedCount.isPresent() && pwnedCount.getAsLong() > threshold) { + return new ValidationResult(MessageKey.PASSWORD_PWNED_ERROR, Long.toString(pwnedCount.getAsLong())); + } + return new ValidationResult(); + } + /** * Verifies whether the email is valid and admitted for use according to the plugin settings. * diff --git a/authme-core/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/authme-core/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index b69f062530..e8bb6cc096 100644 --- a/authme-core/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/authme-core/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -89,6 +89,17 @@ public final class SecuritySettings implements SettingsHolder { newLowercaseStringSetProperty("settings.security.unsafePasswords", "123456", "password", "qwerty", "12345", "54321", "123456789", "help"); + @Comment({ + "Reject passwords found in the Have I Been Pwned Pwned Passwords database.", + "Only the first 5 SHA-1 hash characters are sent; the full hash and password stay local." + }) + public static final Property ENABLE_PWNED_PASSWORD_CHECK = + newProperty("settings.security.pwnedPasswords.enabled", false); + + @Comment("Reject passwords found more than this many times in the Pwned Passwords database") + public static final Property PWNED_PASSWORD_CHECK_THRESHOLD = + newProperty("settings.security.pwnedPasswords.threshold", 0); + @Comment("Tempban a user's IP address if they enter the wrong password too many times") public static final Property TEMPBAN_ON_MAX_LOGINS = newProperty("Security.tempban.enableTempban", false); diff --git a/authme-core/src/main/resources/messages/messages_en.yml b/authme-core/src/main/resources/messages/messages_en.yml index 43951f4928..907bf05684 100644 --- a/authme-core/src/main/resources/messages/messages_en.yml +++ b/authme-core/src/main/resources/messages/messages_en.yml @@ -18,6 +18,7 @@ password: match_error: '&cPasswords didn''t match, check them again!' name_in_password: '&cYou can''t use your name as password, please choose another one...' unsafe_password: '&cThe chosen password isn''t safe, please choose another one...' + pwned_password: '&cYour chosen password is not secure. It has been seen %pwned_count times before! Please use a stronger password...' forbidden_characters: '&4Your password contains illegal characters. Allowed chars: %valid_chars' wrong_length: '&cYour password is too short or too long! Please try with another one!' diff --git a/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java b/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java index 95f0a968bf..5e556e632f 100644 --- a/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommandTest.java @@ -42,14 +42,15 @@ void shouldForwardRequestToManagement() { // given String name = "theUser"; String pass = "newPassword"; - given(validationService.validatePassword(pass, name)).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync(pass, name)) + .willReturn(completedValidation(new ValidationResult())); CommandSender sender = mock(CommandSender.class); // when command.executeCommand(sender, Arrays.asList(name, pass)); // then - verify(validationService).validatePassword(pass, name); + verify(validationService).validatePasswordAsync(pass, name); verify(management).performPasswordChangeAsAdmin(sender, name, pass); } @@ -58,16 +59,21 @@ void shouldSendErrorToCommandSender() { // given String name = "theUser"; String pass = "newPassword"; - given(validationService.validatePassword(pass, name)).willReturn( - new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH, "7")); + given(validationService.validatePasswordAsync(pass, name)).willReturn( + completedValidation(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH, "7"))); CommandSender sender = mock(CommandSender.class); // when command.executeCommand(sender, Arrays.asList(name, pass)); // then - verify(validationService).validatePassword(pass, name); + verify(validationService).validatePasswordAsync(pass, name); verify(commonService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, "7"); verifyNoInteractions(management); } + + private static java.util.concurrent.CompletableFuture completedValidation( + ValidationResult validationResult) { + return java.util.concurrent.CompletableFuture.completedFuture(validationResult); + } } diff --git a/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java b/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java index 914ebd8d48..0fe0240981 100644 --- a/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommandTest.java @@ -67,15 +67,15 @@ void shouldRejectInvalidPassword() { // given String user = "tester"; String password = "myPassword"; - given(validationService.validatePassword(password, user)) - .willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH)); + given(validationService.validatePasswordAsync(password, user)) + .willReturn(completedValidation(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH))); CommandSender sender = mock(CommandSender.class); // when command.executeCommand(sender, Arrays.asList(user, password)); // then - verify(validationService).validatePassword(password, user); + verify(validationService).validatePasswordAsync(password, user); verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]); verify(bukkitService, never()).runTaskAsynchronously(any(Runnable.class)); } @@ -85,7 +85,8 @@ void shouldRejectAlreadyRegisteredAccount() { // given String user = "my_name55"; String password = "@some-pass@"; - given(validationService.validatePassword(password, user)).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync(password, user)) + .willReturn(completedValidation(new ValidationResult())); given(dataSource.isAuthAvailable(user)).willReturn(true); CommandSender sender = mock(CommandSender.class); setBukkitServiceToRunTaskOptionallyAsync(bukkitService); @@ -94,7 +95,7 @@ void shouldRejectAlreadyRegisteredAccount() { command.executeCommand(sender, Arrays.asList(user, password)); // then - verify(validationService).validatePassword(password, user); + verify(validationService).validatePasswordAsync(password, user); verify(commandService).send(sender, MessageKey.NAME_ALREADY_REGISTERED); verify(dataSource, never()).saveAuth(any(PlayerAuth.class)); } @@ -104,7 +105,8 @@ void shouldHandleSavingError() { // given String user = "test-test"; String password = "afdjhfkt"; - given(validationService.validatePassword(password, user)).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync(password, user)) + .willReturn(completedValidation(new ValidationResult())); given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(false); HashedPassword hashedPassword = new HashedPassword("235sdf4w5udsgf"); @@ -116,7 +118,7 @@ void shouldHandleSavingError() { command.executeCommand(sender, Arrays.asList(user, password)); // then - verify(validationService).validatePassword(password, user); + verify(validationService).validatePasswordAsync(password, user); verify(commandService).send(sender, MessageKey.ERROR); ArgumentCaptor captor = ArgumentCaptor.forClass(PlayerAuth.class); verify(dataSource).saveAuth(captor.capture()); @@ -128,7 +130,8 @@ void shouldRegisterOfflinePlayer() { // given String user = "someone"; String password = "Al1O3P49S5%"; - given(validationService.validatePassword(password, user)).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync(password, user)) + .willReturn(completedValidation(new ValidationResult())); given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true); HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048"); @@ -141,7 +144,7 @@ void shouldRegisterOfflinePlayer() { command.executeCommand(sender, Arrays.asList(user, password)); // then - verify(validationService).validatePassword(password, user); + verify(validationService).validatePasswordAsync(password, user); verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS); ArgumentCaptor captor = ArgumentCaptor.forClass(PlayerAuth.class); verify(dataSource).saveAuth(captor.capture()); @@ -153,7 +156,8 @@ void shouldRegisterOnlinePlayer() { // given String user = "someone"; String password = "Al1O3P49S5%"; - given(validationService.validatePassword(password, user)).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync(password, user)) + .willReturn(completedValidation(new ValidationResult())); given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true); HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048"); @@ -170,7 +174,7 @@ void shouldRegisterOnlinePlayer() { command.executeCommand(sender, Arrays.asList(user, password)); // then - verify(validationService).validatePassword(password, user); + verify(validationService).validatePasswordAsync(password, user); verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS); ArgumentCaptor captor = ArgumentCaptor.forClass(PlayerAuth.class); verify(dataSource).saveAuth(captor.capture()); @@ -183,4 +187,9 @@ private void assertAuthHasInfo(PlayerAuth auth, String name, HashedPassword hash assertThat(auth.getNickname(), equalTo(name.toLowerCase(Locale.ROOT))); assertThat(auth.getPassword(), equalTo(hashedPassword)); } + + private static java.util.concurrent.CompletableFuture completedValidation( + ValidationResult validationResult) { + return java.util.concurrent.CompletableFuture.completedFuture(validationResult); + } } diff --git a/authme-core/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/authme-core/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index a7bcbe44c9..689adba1ad 100644 --- a/authme-core/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -91,14 +91,15 @@ public void shouldRejectInvalidPassword() { // given Player sender = initPlayerWithName("abc12", true); String password = "newPW"; - given(validationService.validatePassword(password, "abc12")).willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH)); + given(validationService.validatePasswordAsync(password, "abc12")) + .willReturn(completedValidation(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH))); given(codeManager.isVerificationRequired(sender)).willReturn(false); // when command.executeCommand(sender, Arrays.asList("tester", password)); // then - verify(validationService).validatePassword(password, "abc12"); + verify(validationService).validatePasswordAsync(password, "abc12"); verify(commonService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]); verify(codeManager).isVerificationRequired(sender); } @@ -109,14 +110,15 @@ public void shouldForwardTheDataForValidPassword() { String oldPass = "oldpass"; String newPass = "abc123"; Player player = initPlayerWithName("parker", true); - given(validationService.validatePassword("abc123", "parker")).willReturn(new ValidationResult()); + given(validationService.validatePasswordAsync("abc123", "parker")) + .willReturn(completedValidation(new ValidationResult())); given(codeManager.isVerificationRequired(player)).willReturn(false); // when command.executeCommand(player, Arrays.asList(oldPass, newPass)); // then - verify(validationService).validatePassword(newPass, "parker"); + verify(validationService).validatePasswordAsync(newPass, "parker"); verify(commonService, never()).send(eq(player), any(MessageKey.class)); verify(management).performPasswordChange(player, oldPass, newPass); verify(codeManager).isVerificationRequired(player); @@ -134,6 +136,10 @@ private Player initPlayerWithName(String name, boolean loggedIn) { when(playerCache.isAuthenticated(name)).thenReturn(loggedIn); return player; } -} + private static java.util.concurrent.CompletableFuture completedValidation( + ValidationResult validationResult) { + return java.util.concurrent.CompletableFuture.completedFuture(validationResult); + } +} diff --git a/authme-core/src/test/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommandTest.java b/authme-core/src/test/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommandTest.java index 5b87821dc4..32ddaacb57 100644 --- a/authme-core/src/test/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommandTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/command/executable/email/EmailSetPasswordCommandTest.java @@ -64,14 +64,14 @@ public void shouldChangePassword() { given(recoveryService.canChangePassword(player)).willReturn(true); HashedPassword hashedPassword = passwordSecurity.computeHash("abc123", name); given(passwordSecurity.computeHash("abc123", name)).willReturn(hashedPassword); - given(validationService.validatePassword("abc123", name)) - .willReturn(new ValidationService.ValidationResult()); + given(validationService.validatePasswordAsync("abc123", name)) + .willReturn(completedValidation(new ValidationService.ValidationResult())); // when command.runCommand(player, Collections.singletonList("abc123")); // then - verify(validationService).validatePassword("abc123", name); + verify(validationService).validatePasswordAsync("abc123", name); verify(dataSource).updatePassword(name, hashedPassword); verify(recoveryService).removeFromSuccessfulRecovery(player); verify(commonService).send(player, MessageKey.PASSWORD_CHANGED_SUCCESS); @@ -84,15 +84,16 @@ public void shouldRejectInvalidPassword() { String name = "Morgan"; given(player.getName()).willReturn(name); String password = "newPW"; - given(validationService.validatePassword(password, name)) - .willReturn(new ValidationService.ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH)); + given(validationService.validatePasswordAsync(password, name)) + .willReturn(completedValidation( + new ValidationService.ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH))); given(recoveryService.canChangePassword(player)).willReturn(true); // when command.executeCommand(player, Collections.singletonList(password)); // then - verify(validationService).validatePassword(password, name); + verify(validationService).validatePasswordAsync(password, name); verify(commonService).send(player, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]); } @@ -108,6 +109,10 @@ public void shouldDoNothingCantChangePass() { verifyNoInteractions(validationService, dataSource); verify(commonService).send(player, MessageKey.CHANGE_PASSWORD_EXPIRED); } -} + private static java.util.concurrent.CompletableFuture completedValidation( + ValidationService.ValidationResult validationResult) { + return java.util.concurrent.CompletableFuture.completedFuture(validationResult); + } +} diff --git a/authme-core/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java b/authme-core/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java index 8d5bcc7fa1..cbfc46b2a5 100644 --- a/authme-core/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import ch.jalu.injector.factory.SingletonStore; import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.auth.PlayerAuth; import fr.xephi.authme.data.auth.PlayerCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.events.AuthMeAsyncPreRegisterEvent; @@ -16,6 +17,7 @@ import fr.xephi.authme.process.register.executors.TwoFactorRegisterParams; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; +import fr.xephi.authme.service.ValidationService; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import org.bukkit.entity.Player; @@ -23,10 +25,12 @@ import org.mockito.InjectMocks; import org.mockito.Mock; +import java.util.concurrent.CompletableFuture; import java.util.function.Function; import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; @@ -51,6 +55,8 @@ public class AsyncRegisterTest { @Mock private DataSource dataSource; @Mock + private ValidationService validationService; + @Mock private SingletonStore registrationExecutorStore; @Test @@ -150,6 +156,70 @@ public void shouldStopForFailedExecutorCheck() { verify(executor, only()).isRegistrationAdmitted(params); } + @Test + @SuppressWarnings("unchecked") + public void shouldExecuteRegistrationAfterPasswordValidation() { + // given + String name = "edbert"; + Player player = mockPlayerWithName(name); + TestHelper.mockIpAddressToPlayer(player, "33.44.55.66"); + given(playerCache.isAuthenticated(name)).willReturn(false); + 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(bukkitService.createAndCallEvent(any(Function.class))) + .willReturn(new AuthMeAsyncPreRegisterEvent(player, false)); + + String password = "abc"; + PasswordRegisterParams params = PasswordRegisterParams.of(player, password, null); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + given(executor.isRegistrationAdmitted(params)).willReturn(true); + singletonStoreWillReturn(registrationExecutorStore, executor); + given(validationService.validatePasswordAsync(password, name)) + .willReturn(completedValidation(new ValidationService.ValidationResult())); + + PlayerAuth auth = mock(PlayerAuth.class); + given(executor.buildPlayerAuth(params)).willReturn(auth); + given(dataSource.saveAuth(auth)).willReturn(true); + + // when + asyncRegister.register(RegistrationMethod.PASSWORD_REGISTRATION, params); + + // then + verify(executor).executePostPersistAction(params); + } + + @Test + @SuppressWarnings("unchecked") + public void shouldStopForRejectedPwnedPassword() { + // given + String name = "edbert"; + Player player = mockPlayerWithName(name); + TestHelper.mockIpAddressToPlayer(player, "33.44.55.66"); + given(playerCache.isAuthenticated(name)).willReturn(false); + 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(bukkitService.createAndCallEvent(any(Function.class))) + .willReturn(new AuthMeAsyncPreRegisterEvent(player, false)); + + String password = "abc"; + PasswordRegisterParams params = PasswordRegisterParams.of(player, password, null); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + given(executor.isRegistrationAdmitted(params)).willReturn(true); + singletonStoreWillReturn(registrationExecutorStore, executor); + given(validationService.validatePasswordAsync(password, name)) + .willReturn(completedValidation( + new ValidationService.ValidationResult(MessageKey.PASSWORD_PWNED_ERROR, "42"))); + + // when + asyncRegister.register(RegistrationMethod.PASSWORD_REGISTRATION, params); + + // then + verify(commonService).send(player, MessageKey.PASSWORD_PWNED_ERROR, "42"); + verify(executor, never()).buildPlayerAuth(params); + } + private static Player mockPlayerWithName(String name) { Player player = mock(Player.class); given(player.getName()).willReturn(name); @@ -161,6 +231,9 @@ private static void singletonStoreWillReturn(SingletonStore completedValidation( + ValidationService.ValidationResult validationResult) { + return CompletableFuture.completedFuture(validationResult); + } +} diff --git a/authme-core/src/test/java/fr/xephi/authme/service/PwnedPasswordServiceTest.java b/authme-core/src/test/java/fr/xephi/authme/service/PwnedPasswordServiceTest.java new file mode 100644 index 0000000000..50954654fe --- /dev/null +++ b/authme-core/src/test/java/fr/xephi/authme/service/PwnedPasswordServiceTest.java @@ -0,0 +1,99 @@ +package fr.xephi.authme.service; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import java.util.OptionalLong; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; + +/** + * Test for {@link PwnedPasswordService}. + */ +class PwnedPasswordServiceTest { + + @Test + void shouldReturnPwnedCountForMatchingPasswordHash() { + // given + TestPwnedPasswordService service = new TestPwnedPasswordService( + "003D68EB55068C33ACE09247EE4C639306B:1\n" + + "1E4C9B93F3F0682250B6CF8331B7EE68FD8:12345\n" + + "FFFFF0AC487871FEEC1891C490136E006E2:0"); + + // when + OptionalLong count = service.getPwnedCount("password"); + + // then + assertThat(count.isPresent(), equalTo(true)); + assertThat(count.getAsLong(), equalTo(12345L)); + assertThat(service.requestedHashPrefix, equalTo("5BAA6")); + } + + @Test + void shouldReturnZeroWhenPasswordHashIsAbsent() { + // given + TestPwnedPasswordService service = new TestPwnedPasswordService( + "003D68EB55068C33ACE09247EE4C639306B:1\n" + + "FFFFF0AC487871FEEC1891C490136E006E2:0"); + + // when + OptionalLong count = service.getPwnedCount("password"); + + // then + assertThat(count.isPresent(), equalTo(true)); + assertThat(count.getAsLong(), equalTo(0L)); + } + + @Test + void shouldHashPasswordWithUtf8Encoding() { + // given + TestPwnedPasswordService service = new TestPwnedPasswordService( + "494475F5F874980B7676D511E23D886DA64:123"); + + // when + OptionalLong count = service.getPwnedCount("pässword"); + + // then + assertThat(count.isPresent(), equalTo(true)); + assertThat(count.getAsLong(), equalTo(123L)); + assertThat(service.requestedHashPrefix, equalTo("23B74")); + } + + @Test + void shouldReturnEmptyWhenRangeRequestFails() { + // given + TestPwnedPasswordService service = new TestPwnedPasswordService(new IOException("boom")); + + // when + OptionalLong count = service.getPwnedCount("password"); + + // then + assertThat(count.isPresent(), equalTo(false)); + } + + private static final class TestPwnedPasswordService extends PwnedPasswordService { + private final String response; + private final IOException exception; + private String requestedHashPrefix; + + private TestPwnedPasswordService(String response) { + this.response = response; + this.exception = null; + } + + private TestPwnedPasswordService(IOException exception) { + this.response = null; + this.exception = exception; + } + + @Override + protected String requestHashRange(String hashPrefix) throws IOException { + requestedHashPrefix = hashPrefix; + if (exception != null) { + throw exception; + } + return response; + } + } +} diff --git a/authme-core/src/test/java/fr/xephi/authme/service/ValidationServiceTest.java b/authme-core/src/test/java/fr/xephi/authme/service/ValidationServiceTest.java index 51e5d80565..83356558f1 100644 --- a/authme-core/src/test/java/fr/xephi/authme/service/ValidationServiceTest.java +++ b/authme-core/src/test/java/fr/xephi/authme/service/ValidationServiceTest.java @@ -26,9 +26,11 @@ import java.net.InetSocketAddress; import java.util.Collections; +import java.util.OptionalLong; import java.util.logging.Logger; import static com.google.common.collect.Sets.newHashSet; +import static fr.xephi.authme.service.BukkitServiceTestHelper.setBukkitServiceToRunTaskAsynchronously; import static java.util.Arrays.asList; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; @@ -56,6 +58,10 @@ public class ValidationServiceTest { private PermissionsManager permissionsManager; @Mock private GeoIpService geoIpService; + @Mock + private PwnedPasswordService pwnedPasswordService; + @Mock + private BukkitService bukkitService; @Captor private ArgumentCaptor stringCaptor; @@ -65,9 +71,11 @@ public void createService() { 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(newHashSet("unsafe", "other-unsafe")); + given(settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)).willReturn(false); given(settings.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(newHashSet("name01", "npc")); given(settings.getProperty(RestrictionSettings.ENABLE_RESTRICTED_USERS)).willReturn(false); + setBukkitServiceToRunTaskAsynchronously(bukkitService); } @Test @@ -116,6 +124,48 @@ public void shouldRejectUnsafePassword() { assertErrorEquals(error, MessageKey.PASSWORD_UNSAFE_ERROR); } + @Test + public void shouldRejectPwnedPasswordOverThreshold() { + // given + given(settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)).willReturn(true); + given(settings.getProperty(SecuritySettings.PWNED_PASSWORD_CHECK_THRESHOLD)).willReturn(10); + given(pwnedPasswordService.getPwnedCount("safePass")).willReturn(OptionalLong.of(42)); + + // when + ValidationResult error = validationService.validatePasswordAsync("safePass", "some_user").join(); + + // then + assertErrorEquals(error, MessageKey.PASSWORD_PWNED_ERROR, "42"); + } + + @Test + public void shouldAcceptPwnedPasswordAtThreshold() { + // given + given(settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)).willReturn(true); + given(settings.getProperty(SecuritySettings.PWNED_PASSWORD_CHECK_THRESHOLD)).willReturn(10); + given(pwnedPasswordService.getPwnedCount("safePass")).willReturn(OptionalLong.of(10)); + + // when + ValidationResult error = validationService.validatePasswordAsync("safePass", "some_user").join(); + + // then + assertThat(error.hasError(), equalTo(false)); + } + + @Test + public void shouldAcceptPasswordWhenPwnedCheckIsUnavailable() { + // given + given(settings.getProperty(SecuritySettings.ENABLE_PWNED_PASSWORD_CHECK)).willReturn(true); + given(settings.getProperty(SecuritySettings.PWNED_PASSWORD_CHECK_THRESHOLD)).willReturn(10); + given(pwnedPasswordService.getPwnedCount("safePass")).willReturn(OptionalLong.empty()); + + // when + ValidationResult error = validationService.validatePasswordAsync("safePass", "some_user").join(); + + // then + assertThat(error.hasError(), equalTo(false)); + } + @Test public void shouldAcceptValidPassword() { // given/when @@ -423,5 +473,3 @@ private static void assertErrorEquals(ValidationResult validationResult, Message assertThat(validationResult.getArgs(), equalTo(args)); } } - -