#743 Add proper error message for "invalid chars in password"

- Change password validation to return a ValidationResult object for passing message arguments
- Remove wrapping methods in ProcessService and CommandService and use ValidationService directly
This commit is contained in:
ljacqu 2016-06-03 12:51:49 +02:00
parent 6549ebbf5e
commit 55f7e8097a
15 changed files with 163 additions and 111 deletions

View File

@ -104,18 +104,6 @@ public class CommandService {
return settings; return 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) {
return validationService.validatePassword(password, username);
}
public boolean validateEmail(String email) { public boolean validateEmail(String email) {
return validationService.validateEmail(email); return validationService.validateEmail(email);
} }

View File

@ -10,6 +10,8 @@ import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import javax.inject.Inject; import javax.inject.Inject;
@ -32,6 +34,9 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
@Inject @Inject
private BukkitService bukkitService; private BukkitService bukkitService;
@Inject
private ValidationService validationService;
@Override @Override
public void executeCommand(final CommandSender sender, List<String> arguments, public void executeCommand(final CommandSender sender, List<String> arguments,
final CommandService commandService) { final CommandService commandService) {
@ -40,9 +45,9 @@ public class ChangePasswordAdminCommand implements ExecutableCommand {
final String playerPass = arguments.get(1); final String playerPass = arguments.get(1);
// Validate the password // Validate the password
MessageKey passwordError = commandService.validatePassword(playerPass, playerName); ValidationResult validationResult = validationService.validatePassword(playerPass, playerName);
if (passwordError != null) { if (validationResult.hasError()) {
commandService.send(sender, passwordError); commandService.send(sender, validationResult.getMessageKey(), validationResult.getArgs());
return; return;
} }

View File

@ -9,6 +9,8 @@ import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
@ -29,6 +31,9 @@ public class RegisterAdminCommand implements ExecutableCommand {
@Inject @Inject
private BukkitService bukkitService; private BukkitService bukkitService;
@Inject
private ValidationService validationService;
@Override @Override
public void executeCommand(final CommandSender sender, List<String> arguments, public void executeCommand(final CommandSender sender, List<String> arguments,
final CommandService commandService) { final CommandService commandService) {
@ -38,9 +43,9 @@ public class RegisterAdminCommand implements ExecutableCommand {
final String playerNameLowerCase = playerName.toLowerCase(); final String playerNameLowerCase = playerName.toLowerCase();
// Command logic // Command logic
MessageKey passwordError = commandService.validatePassword(playerPass, playerName); ValidationResult passwordValidation = validationService.validatePassword(playerPass, playerName);
if (passwordError != null) { if (passwordValidation.hasError()) {
commandService.send(sender, passwordError); commandService.send(sender, passwordValidation.getMessageKey(), passwordValidation.getArgs());
return; return;
} }

View File

@ -8,6 +8,8 @@ import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.task.ChangePasswordTask; import fr.xephi.authme.task.ChangePasswordTask;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
import javax.inject.Inject; import javax.inject.Inject;
@ -24,6 +26,9 @@ public class ChangePasswordCommand extends PlayerCommand {
@Inject @Inject
private BukkitService bukkitService; private BukkitService bukkitService;
@Inject
private ValidationService validationService;
@Inject @Inject
// TODO ljacqu 20160531: Remove this once change password task runs as a process (via Management) // TODO ljacqu 20160531: Remove this once change password task runs as a process (via Management)
private PasswordSecurity passwordSecurity; private PasswordSecurity passwordSecurity;
@ -40,9 +45,9 @@ public class ChangePasswordCommand extends PlayerCommand {
} }
// Make sure the password is allowed // Make sure the password is allowed
MessageKey passwordError = commandService.validatePassword(newPassword, name); ValidationResult passwordValidation = validationService.validatePassword(newPassword, name);
if (passwordError != null) { if (passwordValidation.hasError()) {
commandService.send(player, passwordError); commandService.send(player, passwordValidation.getMessageKey(), passwordValidation.getArgs());
return; return;
} }

View File

@ -63,6 +63,8 @@ public enum MessageKey {
PASSWORD_UNSAFE_ERROR("password_error_unsafe"), PASSWORD_UNSAFE_ERROR("password_error_unsafe"),
PASSWORD_CHARACTERS_ERROR("password_error_chars", "REG_EX"),
SESSION_EXPIRED("invalid_session"), SESSION_EXPIRED("invalid_session"),
MUST_REGISTER_MESSAGE("reg_only"), MUST_REGISTER_MESSAGE("reg_only"),

View File

@ -5,6 +5,8 @@ import fr.xephi.authme.permission.PermissionsSystemType;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
import java.util.List; import java.util.List;
public interface PermissionHandler { public interface PermissionHandler {
/** /**

View File

@ -104,17 +104,6 @@ public class ProcessService {
pluginManager.callEvent(event); pluginManager.callEvent(event);
} }
/**
* 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);
}
public boolean validateEmail(String email) { public boolean validateEmail(String email) {
return validationService.validateEmail(email); return validationService.validateEmail(email);
} }

View File

@ -20,6 +20,8 @@ import fr.xephi.authme.settings.properties.RestrictionSettings;
import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.StringUtils;
import fr.xephi.authme.util.Utils; import fr.xephi.authme.util.Utils;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.Bukkit; import org.bukkit.Bukkit;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
@ -51,6 +53,9 @@ public class AsyncRegister implements AsynchronousProcess {
@Inject @Inject
private PermissionsManager permissionsManager; private PermissionsManager permissionsManager;
@Inject
private ValidationService validationService;
AsyncRegister() { } AsyncRegister() { }
private boolean preRegisterCheck(Player player, String password) { private boolean preRegisterCheck(Player player, String password) {
@ -65,9 +70,9 @@ public class AsyncRegister implements AsynchronousProcess {
//check the password safety only if it's not a automatically generated password //check the password safety only if it's not a automatically generated password
if (service.getProperty(SecuritySettings.PASSWORD_HASH) != HashAlgorithm.TWO_FACTOR) { if (service.getProperty(SecuritySettings.PASSWORD_HASH) != HashAlgorithm.TWO_FACTOR) {
MessageKey passwordError = service.validatePassword(password, player.getName()); ValidationResult passwordValidation = validationService.validatePassword(password, player.getName());
if (passwordError != null) { if (passwordValidation.hasError()) {
service.send(player, passwordError); service.send(player, passwordValidation.getMessageKey(), passwordValidation.getArgs());
return false; return false;
} }
} }

View File

@ -1,6 +1,7 @@
package fr.xephi.authme.util; package fr.xephi.authme.util;
import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.datasource.DataSource;
import fr.xephi.authme.initialization.Reloadable;
import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.permission.PermissionsManager;
import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.permission.PlayerStatePermission;
@ -15,21 +16,29 @@ import org.bukkit.command.CommandSender;
import javax.inject.Inject; import javax.inject.Inject;
import java.util.Collection; import java.util.Collection;
import java.util.List; import java.util.List;
import java.util.regex.Pattern;
/** /**
* Validation service. * Validation service.
*/ */
public class ValidationService { public class ValidationService implements Reloadable {
private final NewSetting settings; private final NewSetting settings;
private final DataSource dataSource; private final DataSource dataSource;
private final PermissionsManager permissionsManager; private final PermissionsManager permissionsManager;
private Pattern passwordRegex;
@Inject @Inject
public ValidationService(NewSetting settings, DataSource dataSource, PermissionsManager permissionsManager) { public ValidationService(NewSetting settings, DataSource dataSource, PermissionsManager permissionsManager) {
this.settings = settings; this.settings = settings;
this.dataSource = dataSource; this.dataSource = dataSource;
this.permissionsManager = permissionsManager; this.permissionsManager = permissionsManager;
reload();
}
@Override
public void reload() {
passwordRegex = Pattern.compile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX));
} }
/** /**
@ -37,21 +46,21 @@ public class ValidationService {
* *
* @param password the password to verify * @param password the password to verify
* @param username the username the password is associated with * @param username the username the password is associated with
* @return message key with the password error, or {@code null} if password is valid * @return the validation result
*/ */
public MessageKey validatePassword(String password, String username) { public ValidationResult validatePassword(String password, String username) {
String passLow = password.toLowerCase(); String passLow = password.toLowerCase();
if (!passLow.matches(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX))) { if (!passwordRegex.matcher(passLow).matches()) {
return MessageKey.PASSWORD_MATCH_ERROR; return new ValidationResult(MessageKey.PASSWORD_CHARACTERS_ERROR, passwordRegex.pattern());
} else if (passLow.equalsIgnoreCase(username)) { } else if (passLow.equalsIgnoreCase(username)) {
return MessageKey.PASSWORD_IS_USERNAME_ERROR; return new ValidationResult(MessageKey.PASSWORD_IS_USERNAME_ERROR);
} else if (password.length() < settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH) } else if (password.length() < settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)
|| password.length() > settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) { || password.length() > settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)) {
return MessageKey.INVALID_PASSWORD_LENGTH; return new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH);
} else if (settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(passLow)) { } else if (settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS).contains(passLow)) {
return MessageKey.PASSWORD_UNSAFE_ERROR; return new ValidationResult(MessageKey.PASSWORD_UNSAFE_ERROR);
} }
return null; return new ValidationResult();
} }
/** /**
@ -130,4 +139,44 @@ public class ValidationService {
} }
return false; return false;
} }
public static final class ValidationResult {
private final MessageKey messageKey;
private final String[] args;
/**
* Constructor for a successful validation.
*/
public ValidationResult() {
this.messageKey = null;
this.args = null;
}
/**
* Constructor for a failed validation.
*
* @param messageKey message key of the validation error
* @param args arguments for the message key
*/
public ValidationResult(MessageKey messageKey, String... args) {
this.messageKey = messageKey;
this.args = args;
}
/**
* Returns whether an error was found during the validation, i.e. whether the validation failed.
*
* @return true if there is an error, false if the validation was successful
*/
public boolean hasError() {
return messageKey != null;
}
public MessageKey getMessageKey() {
return messageKey;
}
public String[] getArgs() {
return args;
}
}
} }

View File

@ -141,21 +141,6 @@ public class CommandServiceTest {
assertThat(result, equalTo(settings)); assertThat(result, equalTo(settings));
} }
@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);
}
@Test @Test
public void shouldValidateEmail() { public void shouldValidateEmail() {
// given // given

View File

@ -9,6 +9,8 @@ import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
@ -51,6 +53,9 @@ public class ChangePasswordAdminCommandTest {
@Mock @Mock
private BukkitService bukkitService; private BukkitService bukkitService;
@Mock
private ValidationService validationService;
@BeforeClass @BeforeClass
public static void setUpLogger() { public static void setUpLogger() {
TestHelper.setupLogger(); TestHelper.setupLogger();
@ -60,14 +65,15 @@ public class ChangePasswordAdminCommandTest {
public void shouldRejectInvalidPassword() { public void shouldRejectInvalidPassword() {
// given // given
CommandSender sender = mock(CommandSender.class); CommandSender sender = mock(CommandSender.class);
given(service.validatePassword("Bobby", "bobby")).willReturn(MessageKey.PASSWORD_IS_USERNAME_ERROR); given(validationService.validatePassword("Bobby", "bobby")).willReturn(
new ValidationResult(MessageKey.PASSWORD_IS_USERNAME_ERROR));
// when // when
command.executeCommand(sender, Arrays.asList("bobby", "Bobby"), service); command.executeCommand(sender, Arrays.asList("bobby", "Bobby"), service);
// then // then
verify(service).validatePassword("Bobby", "bobby"); verify(validationService).validatePassword("Bobby", "bobby");
verify(service).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); verify(service).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR, new String[0]);
verifyZeroInteractions(dataSource); verifyZeroInteractions(dataSource);
} }
@ -76,11 +82,13 @@ public class ChangePasswordAdminCommandTest {
// given // given
CommandSender sender = mock(CommandSender.class); CommandSender sender = mock(CommandSender.class);
String player = "player"; String player = "player";
String password = "password";
given(playerCache.isAuthenticated(player)).willReturn(false); given(playerCache.isAuthenticated(player)).willReturn(false);
given(dataSource.getAuth(player)).willReturn(null); given(dataSource.getAuth(player)).willReturn(null);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
// when // when
command.executeCommand(sender, Arrays.asList(player, "password"), service); command.executeCommand(sender, Arrays.asList(player, password), service);
runInnerRunnable(bukkitService); runInnerRunnable(bukkitService);
// then // then
@ -102,13 +110,14 @@ public class ChangePasswordAdminCommandTest {
HashedPassword hashedPassword = mock(HashedPassword.class); HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
given(dataSource.updatePassword(auth)).willReturn(true); given(dataSource.updatePassword(auth)).willReturn(true);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
// when // when
command.executeCommand(sender, Arrays.asList(player, password), service); command.executeCommand(sender, Arrays.asList(player, password), service);
runInnerRunnable(bukkitService); runInnerRunnable(bukkitService);
// then // then
verify(service).validatePassword(password, player); verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
verify(passwordSecurity).computeHash(password, player); verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword); verify(auth).setPassword(hashedPassword);
@ -126,6 +135,7 @@ public class ChangePasswordAdminCommandTest {
given(dataSource.isAuthAvailable(player)).willReturn(true); given(dataSource.isAuthAvailable(player)).willReturn(true);
given(dataSource.getAuth(player)).willReturn(auth); given(dataSource.getAuth(player)).willReturn(auth);
given(dataSource.updatePassword(auth)).willReturn(true); given(dataSource.updatePassword(auth)).willReturn(true);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
HashedPassword hashedPassword = mock(HashedPassword.class); HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
@ -135,7 +145,7 @@ public class ChangePasswordAdminCommandTest {
runInnerRunnable(bukkitService); runInnerRunnable(bukkitService);
// then // then
verify(service).validatePassword(password, player); verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS); verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
verify(passwordSecurity).computeHash(password, player); verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword); verify(auth).setPassword(hashedPassword);
@ -151,6 +161,7 @@ public class ChangePasswordAdminCommandTest {
PlayerAuth auth = mock(PlayerAuth.class); PlayerAuth auth = mock(PlayerAuth.class);
given(playerCache.isAuthenticated(player)).willReturn(true); given(playerCache.isAuthenticated(player)).willReturn(true);
given(playerCache.getAuth(player)).willReturn(auth); given(playerCache.getAuth(player)).willReturn(auth);
given(validationService.validatePassword(password, player)).willReturn(new ValidationResult());
HashedPassword hashedPassword = mock(HashedPassword.class); HashedPassword hashedPassword = mock(HashedPassword.class);
given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, player)).willReturn(hashedPassword);
@ -161,7 +172,7 @@ public class ChangePasswordAdminCommandTest {
runInnerRunnable(bukkitService); runInnerRunnable(bukkitService);
// then // then
verify(service).validatePassword(password, player); verify(validationService).validatePassword(password, player);
verify(service).send(sender, MessageKey.ERROR); verify(service).send(sender, MessageKey.ERROR);
verify(passwordSecurity).computeHash(password, player); verify(passwordSecurity).computeHash(password, player);
verify(auth).setPassword(hashedPassword); verify(auth).setPassword(hashedPassword);

View File

@ -8,6 +8,8 @@ import fr.xephi.authme.output.MessageKey;
import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.PasswordSecurity;
import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.HashedPassword;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
import org.junit.BeforeClass; import org.junit.BeforeClass;
@ -49,10 +51,10 @@ public class RegisterAdminCommandTest {
private BukkitService bukkitService; private BukkitService bukkitService;
@Mock @Mock
private CommandSender sender; private CommandService commandService;
@Mock @Mock
private CommandService commandService; private ValidationService validationService;
@BeforeClass @BeforeClass
public static void setUpLogger() { public static void setUpLogger() {
@ -64,14 +66,16 @@ public class RegisterAdminCommandTest {
// given // given
String user = "tester"; String user = "tester";
String password = "myPassword"; String password = "myPassword";
given(commandService.validatePassword(password, user)).willReturn(MessageKey.INVALID_PASSWORD_LENGTH); given(validationService.validatePassword(password, user))
.willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH));
CommandSender sender = mock(CommandSender.class);
// when // when
command.executeCommand(sender, Arrays.asList(user, password), commandService); command.executeCommand(sender, Arrays.asList(user, password), commandService);
// then // then
verify(commandService).validatePassword(password, user); verify(validationService).validatePassword(password, user);
verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]);
verify(bukkitService, never()).runTaskAsynchronously(any(Runnable.class)); verify(bukkitService, never()).runTaskAsynchronously(any(Runnable.class));
} }
@ -80,15 +84,16 @@ public class RegisterAdminCommandTest {
// given // given
String user = "my_name55"; String user = "my_name55";
String password = "@some-pass@"; String password = "@some-pass@";
given(commandService.validatePassword(password, user)).willReturn(null); given(validationService.validatePassword(password, user)).willReturn(new ValidationResult());
given(dataSource.isAuthAvailable(user)).willReturn(true); given(dataSource.isAuthAvailable(user)).willReturn(true);
CommandSender sender = mock(CommandSender.class);
// when // when
command.executeCommand(sender, Arrays.asList(user, password), commandService); command.executeCommand(sender, Arrays.asList(user, password), commandService);
TestHelper.runInnerRunnable(bukkitService); TestHelper.runInnerRunnable(bukkitService);
// then // then
verify(commandService).validatePassword(password, user); verify(validationService).validatePassword(password, user);
verify(commandService).send(sender, MessageKey.NAME_ALREADY_REGISTERED); verify(commandService).send(sender, MessageKey.NAME_ALREADY_REGISTERED);
verify(dataSource, never()).saveAuth(any(PlayerAuth.class)); verify(dataSource, never()).saveAuth(any(PlayerAuth.class));
} }
@ -98,18 +103,19 @@ public class RegisterAdminCommandTest {
// given // given
String user = "test-test"; String user = "test-test";
String password = "afdjhfkt"; String password = "afdjhfkt";
given(commandService.validatePassword(password, user)).willReturn(null); given(validationService.validatePassword(password, user)).willReturn(new ValidationResult());
given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.isAuthAvailable(user)).willReturn(false);
given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(false); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(false);
HashedPassword hashedPassword = new HashedPassword("235sdf4w5udsgf"); HashedPassword hashedPassword = new HashedPassword("235sdf4w5udsgf");
given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword);
CommandSender sender = mock(CommandSender.class);
// when // when
command.executeCommand(sender, Arrays.asList(user, password), commandService); command.executeCommand(sender, Arrays.asList(user, password), commandService);
TestHelper.runInnerRunnable(bukkitService); TestHelper.runInnerRunnable(bukkitService);
// then // then
verify(commandService).validatePassword(password, user); verify(validationService).validatePassword(password, user);
verify(commandService).send(sender, MessageKey.ERROR); verify(commandService).send(sender, MessageKey.ERROR);
ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class); ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class);
verify(dataSource).saveAuth(captor.capture()); verify(dataSource).saveAuth(captor.capture());
@ -121,19 +127,20 @@ public class RegisterAdminCommandTest {
// given // given
String user = "someone"; String user = "someone";
String password = "Al1O3P49S5%"; String password = "Al1O3P49S5%";
given(commandService.validatePassword(password, user)).willReturn(null); given(validationService.validatePassword(password, user)).willReturn(new ValidationResult());
given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.isAuthAvailable(user)).willReturn(false);
given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true);
HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048"); HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048");
given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword);
given(bukkitService.getPlayerExact(user)).willReturn(null); given(bukkitService.getPlayerExact(user)).willReturn(null);
CommandSender sender = mock(CommandSender.class);
// when // when
command.executeCommand(sender, Arrays.asList(user, password), commandService); command.executeCommand(sender, Arrays.asList(user, password), commandService);
TestHelper.runInnerRunnable(bukkitService); TestHelper.runInnerRunnable(bukkitService);
// then // then
verify(commandService).validatePassword(password, user); verify(validationService).validatePassword(password, user);
verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS); verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS);
ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class); ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class);
verify(dataSource).saveAuth(captor.capture()); verify(dataSource).saveAuth(captor.capture());
@ -146,13 +153,14 @@ public class RegisterAdminCommandTest {
// given // given
String user = "someone"; String user = "someone";
String password = "Al1O3P49S5%"; String password = "Al1O3P49S5%";
given(commandService.validatePassword(password, user)).willReturn(null); given(validationService.validatePassword(password, user)).willReturn(new ValidationResult());
given(dataSource.isAuthAvailable(user)).willReturn(false); given(dataSource.isAuthAvailable(user)).willReturn(false);
given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true); given(dataSource.saveAuth(any(PlayerAuth.class))).willReturn(true);
HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048"); HashedPassword hashedPassword = new HashedPassword("$aea2345EW235dfsa@#R%987048");
given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword); given(passwordSecurity.computeHash(password, user)).willReturn(hashedPassword);
Player player = mock(Player.class); Player player = mock(Player.class);
given(bukkitService.getPlayerExact(user)).willReturn(player); given(bukkitService.getPlayerExact(user)).willReturn(player);
CommandSender sender = mock(CommandSender.class);
// when // when
command.executeCommand(sender, Arrays.asList(user, password), commandService); command.executeCommand(sender, Arrays.asList(user, password), commandService);
@ -160,7 +168,7 @@ public class RegisterAdminCommandTest {
runSyncDelayedTask(bukkitService); runSyncDelayedTask(bukkitService);
// then // then
verify(commandService).validatePassword(password, user); verify(validationService).validatePassword(password, user);
verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS); verify(commandService).send(sender, MessageKey.REGISTER_SUCCESS);
ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class); ArgumentCaptor<PlayerAuth> captor = ArgumentCaptor.forClass(PlayerAuth.class);
verify(dataSource).saveAuth(captor.capture()); verify(dataSource).saveAuth(captor.capture());

View File

@ -8,6 +8,8 @@ import fr.xephi.authme.settings.properties.RestrictionSettings;
import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.task.ChangePasswordTask; import fr.xephi.authme.task.ChangePasswordTask;
import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.BukkitService;
import fr.xephi.authme.util.ValidationService;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.BlockCommandSender; import org.bukkit.command.BlockCommandSender;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
@ -53,6 +55,9 @@ public class ChangePasswordCommandTest {
@Mock @Mock
private BukkitService bukkitService; private BukkitService bukkitService;
@Mock
private ValidationService validationService;
@Before @Before
public void setSettings() { public void setSettings() {
when(commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).thenReturn(2); when(commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).thenReturn(2);
@ -91,26 +96,28 @@ public class ChangePasswordCommandTest {
// given // given
CommandSender sender = initPlayerWithName("abc12", true); CommandSender sender = initPlayerWithName("abc12", true);
String password = "newPW"; String password = "newPW";
given(commandService.validatePassword(password, "abc12")).willReturn(MessageKey.INVALID_PASSWORD_LENGTH); given(validationService.validatePassword(password, "abc12"))
.willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH));
// when // when
command.executeCommand(sender, Arrays.asList("tester", password), commandService); command.executeCommand(sender, Arrays.asList("tester", password), commandService);
// then // then
verify(commandService).validatePassword(password, "abc12"); verify(validationService).validatePassword(password, "abc12");
verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]);
} }
@Test @Test
public void shouldForwardTheDataForValidPassword() { public void shouldForwardTheDataForValidPassword() {
// given // given
CommandSender sender = initPlayerWithName("parker", true); CommandSender sender = initPlayerWithName("parker", true);
given(validationService.validatePassword("abc123", "parker")).willReturn(new ValidationResult());
// when // when
command.executeCommand(sender, Arrays.asList("abc123", "abc123"), commandService); command.executeCommand(sender, Arrays.asList("abc123", "abc123"), commandService);
// then // then
verify(commandService).validatePassword("abc123", "parker"); verify(validationService).validatePassword("abc123", "parker");
verify(commandService, never()).send(eq(sender), any(MessageKey.class)); verify(commandService, never()).send(eq(sender), any(MessageKey.class));
ArgumentCaptor<ChangePasswordTask> taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class); ArgumentCaptor<ChangePasswordTask> taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class);
verify(bukkitService).runTaskAsynchronously(taskCaptor.capture()); verify(bukkitService).runTaskAsynchronously(taskCaptor.capture());

View File

@ -127,21 +127,6 @@ public class ProcessServiceTest {
verify(messages).retrieveSingle(key); verify(messages).retrieveSingle(key);
} }
@Test
public void shouldValidatePassword() {
// given
String user = "test-user";
String password = "passw0rd";
given(validationService.validatePassword(password, user)).willReturn(MessageKey.PASSWORD_MATCH_ERROR);
// when
MessageKey result = processService.validatePassword(password, user);
// then
assertThat(result, equalTo(MessageKey.PASSWORD_MATCH_ERROR));
verify(validationService).validatePassword(password, user);
}
@Test @Test
public void shouldValidateEmail() { public void shouldValidateEmail() {
// given // given

View File

@ -9,6 +9,7 @@ import fr.xephi.authme.settings.NewSetting;
import fr.xephi.authme.settings.properties.EmailSettings; import fr.xephi.authme.settings.properties.EmailSettings;
import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.RestrictionSettings;
import fr.xephi.authme.settings.properties.SecuritySettings; import fr.xephi.authme.settings.properties.SecuritySettings;
import fr.xephi.authme.util.ValidationService.ValidationResult;
import org.bukkit.command.CommandSender; import org.bukkit.command.CommandSender;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@ -20,7 +21,6 @@ import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
@ -53,56 +53,56 @@ public class ValidationServiceTest {
@Test @Test
public void shouldRejectPasswordSameAsUsername() { public void shouldRejectPasswordSameAsUsername() {
// given/when // given/when
MessageKey error = validationService.validatePassword("bobby", "Bobby"); ValidationResult error = validationService.validatePassword("bobby", "Bobby");
// then // then
assertThat(error, equalTo(MessageKey.PASSWORD_IS_USERNAME_ERROR)); assertErrorEquals(error, MessageKey.PASSWORD_IS_USERNAME_ERROR);
} }
@Test @Test
public void shouldRejectPasswordNotMatchingPattern() { public void shouldRejectPasswordNotMatchingPattern() {
// given/when // given/when
// service mock returns pattern a-zA-Z -> numbers should not be accepted // service mock returns pattern a-zA-Z -> numbers should not be accepted
MessageKey error = validationService.validatePassword("invalid1234", "myPlayer"); ValidationResult error = validationService.validatePassword("invalid1234", "myPlayer");
// then // then
assertThat(error, equalTo(MessageKey.PASSWORD_MATCH_ERROR)); assertErrorEquals(error, MessageKey.PASSWORD_CHARACTERS_ERROR, "[a-zA-Z]+");
} }
@Test @Test
public void shouldRejectTooShortPassword() { public void shouldRejectTooShortPassword() {
// given/when // given/when
MessageKey error = validationService.validatePassword("ab", "tester"); ValidationResult error = validationService.validatePassword("ab", "tester");
// then // then
assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH)); assertErrorEquals(error, MessageKey.INVALID_PASSWORD_LENGTH);
} }
@Test @Test
public void shouldRejectTooLongPassword() { public void shouldRejectTooLongPassword() {
// given/when // given/when
MessageKey error = validationService.validatePassword(Strings.repeat("a", 30), "player"); ValidationResult error = validationService.validatePassword(Strings.repeat("a", 30), "player");
// then // then
assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH)); assertErrorEquals(error, MessageKey.INVALID_PASSWORD_LENGTH);
} }
@Test @Test
public void shouldRejectUnsafePassword() { public void shouldRejectUnsafePassword() {
// given/when // given/when
MessageKey error = validationService.validatePassword("unsafe", "playertest"); ValidationResult error = validationService.validatePassword("unsafe", "playertest");
// then // then
assertThat(error, equalTo(MessageKey.PASSWORD_UNSAFE_ERROR)); assertErrorEquals(error, MessageKey.PASSWORD_UNSAFE_ERROR);
} }
@Test @Test
public void shouldAcceptValidPassword() { public void shouldAcceptValidPassword() {
// given/when // given/when
MessageKey error = validationService.validatePassword("safePass", "some_user"); ValidationResult error = validationService.validatePassword("safePass", "some_user");
// then // then
assertThat(error, nullValue()); assertThat(error.hasError(), equalTo(false));
} }
@Test @Test
@ -230,4 +230,10 @@ public class ValidationServiceTest {
// then // then
assertThat(result, equalTo(true)); assertThat(result, equalTo(true));
} }
private static void assertErrorEquals(ValidationResult validationResult, MessageKey messageKey, String... args) {
assertThat(validationResult.hasError(), equalTo(true));
assertThat(validationResult.getMessageKey(), equalTo(messageKey));
assertThat(validationResult.getArgs(), equalTo(args));
}
} }