Check valid password via service
- Create validation service; fixes same code being duplicated in four places - Goal is to remove Utils class, by moving methods to validation service or other services - Remove unused properties in legacy settings
This commit is contained in:
@@ -15,6 +15,7 @@ import fr.xephi.authme.settings.NewSetting;
|
||||
import fr.xephi.authme.settings.SpawnLoader;
|
||||
import fr.xephi.authme.settings.domain.Property;
|
||||
import fr.xephi.authme.settings.properties.SecuritySettings;
|
||||
import fr.xephi.authme.util.ValidationService;
|
||||
import org.bukkit.command.CommandSender;
|
||||
import org.bukkit.entity.Player;
|
||||
import org.junit.Before;
|
||||
@@ -63,11 +64,13 @@ public class CommandServiceTest {
|
||||
private SpawnLoader spawnLoader;
|
||||
@Mock
|
||||
private AntiBot antiBot;
|
||||
@Mock
|
||||
private ValidationService validationService;
|
||||
|
||||
@Before
|
||||
public void setUpService() {
|
||||
commandService = new CommandService(authMe, commandMapper, helpProvider, messages, passwordSecurity,
|
||||
permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot);
|
||||
permissionsManager, settings, ipAddressManager, pluginHooks, spawnLoader, antiBot, validationService);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -228,4 +231,19 @@ public class CommandServiceTest {
|
||||
// then
|
||||
assertThat(ipManager, equalTo(ipAddressManager));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldValidatePassword() {
|
||||
// given
|
||||
String user = "asdf";
|
||||
String password = "mySecret55";
|
||||
given(validationService.validatePassword(password, user)).willReturn(MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
|
||||
// when
|
||||
MessageKey result = commandService.validatePassword(password, user);
|
||||
|
||||
// then
|
||||
assertThat(result, equalTo(MessageKey.INVALID_PASSWORD_LENGTH));
|
||||
verify(validationService).validatePassword(password, user);
|
||||
}
|
||||
}
|
||||
|
||||
+11
-73
@@ -1,6 +1,5 @@
|
||||
package fr.xephi.authme.command.executable.authme;
|
||||
|
||||
import com.google.common.base.Strings;
|
||||
import fr.xephi.authme.ConsoleLoggerTestInitializer;
|
||||
import fr.xephi.authme.cache.auth.PlayerAuth;
|
||||
import fr.xephi.authme.cache.auth.PlayerCache;
|
||||
@@ -10,12 +9,12 @@ import fr.xephi.authme.datasource.DataSource;
|
||||
import fr.xephi.authme.output.MessageKey;
|
||||
import fr.xephi.authme.security.PasswordSecurity;
|
||||
import fr.xephi.authme.security.crypts.HashedPassword;
|
||||
import fr.xephi.authme.settings.properties.RestrictionSettings;
|
||||
import fr.xephi.authme.settings.properties.SecuritySettings;
|
||||
import org.bukkit.command.CommandSender;
|
||||
import org.junit.Before;
|
||||
import org.junit.BeforeClass;
|
||||
import org.junit.Test;
|
||||
import org.junit.runner.RunWith;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.runners.MockitoJUnitRunner;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
||||
@@ -29,8 +28,10 @@ import static org.mockito.Mockito.verify;
|
||||
/**
|
||||
* Test for {@link ChangePasswordAdminCommand}.
|
||||
*/
|
||||
@RunWith(MockitoJUnitRunner.class)
|
||||
public class ChangePasswordAdminCommandTest {
|
||||
|
||||
@Mock
|
||||
private CommandService service;
|
||||
|
||||
@BeforeClass
|
||||
@@ -38,88 +39,22 @@ public class ChangePasswordAdminCommandTest {
|
||||
ConsoleLoggerTestInitializer.setupLogger();
|
||||
}
|
||||
|
||||
@Before
|
||||
public void setUpServiceMock() {
|
||||
service = mock(CommandService.class);
|
||||
given(service.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)).willReturn("[a-zA-Z]+");
|
||||
given(service.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3);
|
||||
given(service.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(20);
|
||||
given(service.getProperty(SecuritySettings.UNSAFE_PASSWORDS))
|
||||
.willReturn(Arrays.asList("unsafe", "otherUnsafe"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectPasswordSameAsUsername() {
|
||||
public void shouldRejectInvalidPassword() {
|
||||
// given
|
||||
ExecutableCommand command = new ChangePasswordAdminCommand();
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
given(service.validatePassword("Bobby", "bobby")).willReturn(MessageKey.PASSWORD_IS_USERNAME_ERROR);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("bobby", "Bobby"), service);
|
||||
|
||||
// then
|
||||
verify(service).validatePassword("Bobby", "bobby");
|
||||
verify(service).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR);
|
||||
verify(service, never()).getDataSource();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectPasswordNotMatchingPattern() {
|
||||
// given
|
||||
ExecutableCommand command = new ChangePasswordAdminCommand();
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
// service mock returns pattern a-zA-Z -> numbers should not be accepted
|
||||
String invalidPassword = "invalid1234";
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("myPlayer123", invalidPassword), service);
|
||||
|
||||
// then
|
||||
verify(service).send(sender, MessageKey.PASSWORD_MATCH_ERROR);
|
||||
verify(service, never()).getDataSource();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooShortPassword() {
|
||||
// given
|
||||
ExecutableCommand command = new ChangePasswordAdminCommand();
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("player", "ab"), service);
|
||||
|
||||
// then
|
||||
verify(service).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
verify(service, never()).getDataSource();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooLongPassword() {
|
||||
// given
|
||||
ExecutableCommand command = new ChangePasswordAdminCommand();
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("player", Strings.repeat("a", 30)), service);
|
||||
|
||||
// then
|
||||
verify(service).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
verify(service, never()).getDataSource();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectUnsafePassword() {
|
||||
// given
|
||||
ExecutableCommand command = new ChangePasswordAdminCommand();
|
||||
CommandSender sender = mock(CommandSender.class);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("player", "unsafe"), service);
|
||||
|
||||
// then
|
||||
verify(service).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
|
||||
verify(service, never()).getDataSource();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectCommandForUnknownUser() {
|
||||
// given
|
||||
@@ -173,6 +108,7 @@ public class ChangePasswordAdminCommandTest {
|
||||
runInnerRunnable(service);
|
||||
|
||||
// then
|
||||
verify(service).validatePassword(password, player);
|
||||
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
|
||||
verify(passwordSecurity).computeHash(password, player);
|
||||
verify(auth).setPassword(hashedPassword);
|
||||
@@ -209,6 +145,7 @@ public class ChangePasswordAdminCommandTest {
|
||||
runInnerRunnable(service);
|
||||
|
||||
// then
|
||||
verify(service).validatePassword(password, player);
|
||||
verify(service).send(sender, MessageKey.PASSWORD_CHANGED_SUCCESS);
|
||||
verify(passwordSecurity).computeHash(password, player);
|
||||
verify(auth).setPassword(hashedPassword);
|
||||
@@ -244,6 +181,7 @@ public class ChangePasswordAdminCommandTest {
|
||||
runInnerRunnable(service);
|
||||
|
||||
// then
|
||||
verify(service).validatePassword(password, player);
|
||||
verify(service).send(sender, MessageKey.ERROR);
|
||||
verify(passwordSecurity).computeHash(password, player);
|
||||
verify(auth).setPassword(hashedPassword);
|
||||
|
||||
+8
-62
@@ -22,6 +22,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.Matchers.argThat;
|
||||
import static org.mockito.Mockito.any;
|
||||
import static org.mockito.Mockito.eq;
|
||||
import static org.mockito.Mockito.mock;
|
||||
@@ -57,9 +58,7 @@ public class ChangePasswordCommandTest {
|
||||
command.executeCommand(sender, new ArrayList<String>(), commandService);
|
||||
|
||||
// then
|
||||
ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
|
||||
verify(sender).sendMessage(captor.capture());
|
||||
assertThat(captor.getValue(), containsString("only for players"));
|
||||
verify(sender).sendMessage(argThat(containsString("only for players")));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -76,75 +75,21 @@ public class ChangePasswordCommandTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldDenyInvalidPassword() {
|
||||
// given
|
||||
CommandSender sender = initPlayerWithName("name", true);
|
||||
ChangePasswordCommand command = new ChangePasswordCommand();
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("old123", "!pass"), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).send(sender, MessageKey.PASSWORD_MATCH_ERROR);
|
||||
}
|
||||
|
||||
|
||||
@Test
|
||||
public void shouldRejectPasswordEqualToNick() {
|
||||
// given
|
||||
CommandSender sender = initPlayerWithName("tester", true);
|
||||
ChangePasswordCommand command = new ChangePasswordCommand();
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("old_", "Tester"), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooLongPassword() {
|
||||
public void shouldRejectInvalidPassword() {
|
||||
// given
|
||||
CommandSender sender = initPlayerWithName("abc12", true);
|
||||
ChangePasswordCommand command = new ChangePasswordCommand();
|
||||
given(commandService.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(3);
|
||||
String password = "newPW";
|
||||
given(commandService.validatePassword(password, "abc12")).willReturn(MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("12", "test"), commandService);
|
||||
command.executeCommand(sender, Arrays.asList("tester", password), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).validatePassword(password, "abc12");
|
||||
verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooShortPassword() {
|
||||
// given
|
||||
CommandSender sender = initPlayerWithName("abc12", true);
|
||||
ChangePasswordCommand command = new ChangePasswordCommand();
|
||||
given(commandService.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(7);
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("oldverylongpassword", "tester"), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectUnsafeCustomPassword() {
|
||||
// given
|
||||
CommandSender sender = initPlayerWithName("player", true);
|
||||
ChangePasswordCommand command = new ChangePasswordCommand();
|
||||
given(commandService.getProperty(SecuritySettings.UNSAFE_PASSWORDS))
|
||||
.willReturn(Arrays.asList("test", "abc123"));
|
||||
|
||||
// when
|
||||
command.executeCommand(sender, Arrays.asList("oldpw", "abc123"), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldForwardTheDataForValidPassword() {
|
||||
// given
|
||||
@@ -155,6 +100,7 @@ public class ChangePasswordCommandTest {
|
||||
command.executeCommand(sender, Arrays.asList("abc123", "abc123"), commandService);
|
||||
|
||||
// then
|
||||
verify(commandService).validatePassword("abc123", "parker");
|
||||
verify(commandService, never()).send(eq(sender), any(MessageKey.class));
|
||||
ArgumentCaptor<ChangePasswordTask> taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class);
|
||||
verify(commandService).runTaskAsynchronously(taskCaptor.capture());
|
||||
|
||||
@@ -11,7 +11,9 @@ import fr.xephi.authme.security.crypts.HashedPassword;
|
||||
import fr.xephi.authme.settings.NewSetting;
|
||||
import fr.xephi.authme.settings.SpawnLoader;
|
||||
import fr.xephi.authme.settings.properties.SecuritySettings;
|
||||
import fr.xephi.authme.util.ValidationService;
|
||||
import org.bukkit.command.CommandSender;
|
||||
import org.hamcrest.MatcherAssert;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
@@ -37,7 +39,7 @@ public class ProcessServiceTest {
|
||||
mocks = new HashMap<>();
|
||||
processService = new ProcessService(newMock(NewSetting.class), newMock(Messages.class), newMock(AuthMe.class),
|
||||
newMock(DataSource.class), newMock(IpAddressManager.class), newMock(PasswordSecurity.class),
|
||||
newMock(PluginHooks.class), newMock(SpawnLoader.class));
|
||||
newMock(PluginHooks.class), newMock(SpawnLoader.class), newMock(ValidationService.class));
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -186,6 +188,22 @@ public class ProcessServiceTest {
|
||||
verify(passwordSecurity).computeHash(password, username);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldValidatePassword() {
|
||||
// given
|
||||
String user = "test-user";
|
||||
String password = "passw0rd";
|
||||
ValidationService validationService = getMock(ValidationService.class);
|
||||
given(validationService.validatePassword(password, user)).willReturn(MessageKey.PASSWORD_MATCH_ERROR);
|
||||
|
||||
// when
|
||||
MessageKey result = processService.validatePassword(password, user);
|
||||
|
||||
// then
|
||||
MatcherAssert.assertThat(result, equalTo(MessageKey.PASSWORD_MATCH_ERROR));
|
||||
verify(validationService).validatePassword(password, user);
|
||||
}
|
||||
|
||||
private <T> T newMock(Class<T> clazz) {
|
||||
T mock = mock(clazz);
|
||||
mocks.put(clazz, mock);
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
package fr.xephi.authme.util;
|
||||
|
||||
import com.google.common.base.Strings;
|
||||
import fr.xephi.authme.output.MessageKey;
|
||||
import fr.xephi.authme.settings.NewSetting;
|
||||
import fr.xephi.authme.settings.properties.RestrictionSettings;
|
||||
import fr.xephi.authme.settings.properties.SecuritySettings;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.util.Arrays;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.nullValue;
|
||||
import static org.junit.Assert.assertThat;
|
||||
import static org.mockito.BDDMockito.given;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
/**
|
||||
* Test for {@link ValidationService}.
|
||||
*/
|
||||
public class ValidationServiceTest {
|
||||
|
||||
private ValidationService validationService;
|
||||
|
||||
@Before
|
||||
public void createService() {
|
||||
NewSetting settings = mock(NewSetting.class);
|
||||
given(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX)).willReturn("[a-zA-Z]+");
|
||||
given(settings.getProperty(SecuritySettings.MIN_PASSWORD_LENGTH)).willReturn(3);
|
||||
given(settings.getProperty(SecuritySettings.MAX_PASSWORD_LENGTH)).willReturn(20);
|
||||
given(settings.getProperty(SecuritySettings.UNSAFE_PASSWORDS))
|
||||
.willReturn(Arrays.asList("unsafe", "other-unsafe"));
|
||||
validationService = new ValidationService(settings);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectPasswordSameAsUsername() {
|
||||
// given/when
|
||||
MessageKey error = validationService.validatePassword("bobby", "Bobby");
|
||||
|
||||
// then
|
||||
assertThat(error, equalTo(MessageKey.PASSWORD_IS_USERNAME_ERROR));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectPasswordNotMatchingPattern() {
|
||||
// given/when
|
||||
// service mock returns pattern a-zA-Z -> numbers should not be accepted
|
||||
MessageKey error = validationService.validatePassword("invalid1234", "myPlayer");
|
||||
|
||||
// then
|
||||
assertThat(error, equalTo(MessageKey.PASSWORD_MATCH_ERROR));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooShortPassword() {
|
||||
// given/when
|
||||
MessageKey error = validationService.validatePassword("ab", "tester");
|
||||
|
||||
// then
|
||||
assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectTooLongPassword() {
|
||||
// given/when
|
||||
MessageKey error = validationService.validatePassword(Strings.repeat("a", 30), "player");
|
||||
|
||||
// then
|
||||
assertThat(error, equalTo(MessageKey.INVALID_PASSWORD_LENGTH));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldRejectUnsafePassword() {
|
||||
// given/when
|
||||
MessageKey error = validationService.validatePassword("unsafe", "playertest");
|
||||
|
||||
// then
|
||||
assertThat(error, equalTo(MessageKey.PASSWORD_UNSAFE_ERROR));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void shouldAcceptValidPassword() {
|
||||
// given/when
|
||||
MessageKey error = validationService.validatePassword("safePass", "some_user");
|
||||
|
||||
// then
|
||||
assertThat(error, nullValue());
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user