From 370d203873948087f5288d25c8d4200534918c13 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 18 Dec 2016 13:11:56 +0100 Subject: [PATCH] #830 Write tests for registration process --- .../PasswordRegisterExecutorProvider.java | 4 +- .../executors/PlayerAuthBuilderHelper.java | 6 +- .../executors/RegistrationExecutor.java | 3 + .../process/register/AsyncRegisterTest.java | 121 ++++++++++++ .../EmailRegisterExecutorProviderTest.java | 173 ++++++++++++++++++ .../PasswordRegisterExecutorProviderTest.java | 152 +++++++++++++++ .../PlayerAuthBuilderHelperTest.java | 49 +++++ 7 files changed, 504 insertions(+), 4 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java create mode 100644 src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java create mode 100644 src/test/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProviderTest.java create mode 100644 src/test/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelperTest.java diff --git a/src/main/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProvider.java b/src/main/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProvider.java index c5a4761ba..a79c63c82 100644 --- a/src/main/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProvider.java +++ b/src/main/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProvider.java @@ -20,7 +20,9 @@ import javax.inject.Inject; import static fr.xephi.authme.process.register.executors.PlayerAuthBuilderHelper.createPlayerAuth; - +/** + * Provides registration executors for password-based registration variants. + */ class PasswordRegisterExecutorProvider { /** diff --git a/src/main/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelper.java b/src/main/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelper.java index dfad47cf9..1943b5d54 100644 --- a/src/main/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelper.java +++ b/src/main/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelper.java @@ -10,6 +10,9 @@ import org.bukkit.entity.Player; */ final class PlayerAuthBuilderHelper { + private PlayerAuthBuilderHelper() { + } + static PlayerAuth createPlayerAuth(Player player, HashedPassword hashedPassword, String email) { return PlayerAuth.builder() .name(player.getName().toLowerCase()) @@ -20,7 +23,4 @@ final class PlayerAuthBuilderHelper { .location(player.getLocation()) .build(); } - - - } diff --git a/src/main/java/fr/xephi/authme/process/register/executors/RegistrationExecutor.java b/src/main/java/fr/xephi/authme/process/register/executors/RegistrationExecutor.java index cfb02393d..cceb5c18e 100644 --- a/src/main/java/fr/xephi/authme/process/register/executors/RegistrationExecutor.java +++ b/src/main/java/fr/xephi/authme/process/register/executors/RegistrationExecutor.java @@ -10,6 +10,9 @@ public interface RegistrationExecutor { /** * Returns whether the registration may take place. Use this method to execute * checks specific to the registration method. + *

+ * If this method returns {@code false}, it is expected that the executor inform + * the player about the error within this method call. * * @return true if registration may be performed, false otherwise */ diff --git a/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java new file mode 100644 index 000000000..b845b8d59 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/register/AsyncRegisterTest.java @@ -0,0 +1,121 @@ +package fr.xephi.authme.process.register; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.auth.PlayerCache; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.message.MessageKey; +import fr.xephi.authme.permission.PermissionsManager; +import fr.xephi.authme.process.register.executors.RegistrationExecutor; +import fr.xephi.authme.service.CommonService; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import org.bukkit.entity.Player; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link AsyncRegister}. + */ +@RunWith(MockitoJUnitRunner.class) +public class AsyncRegisterTest { + + @InjectMocks + private AsyncRegister asyncRegister; + + @Mock + private PlayerCache playerCache; + @Mock + private PermissionsManager permissionsManager; + @Mock + private CommonService commonService; + @Mock + private DataSource dataSource; + + @Test + public void shouldDetectAlreadyLoggedInPlayer() { + // given + String name = "robert"; + Player player = mockPlayerWithName(name); + given(playerCache.isAuthenticated(name)).willReturn(true); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + + // when + asyncRegister.register(player, executor); + + // then + verify(commonService).send(player, MessageKey.ALREADY_LOGGED_IN_ERROR); + verifyZeroInteractions(dataSource, executor); + } + + @Test + public void shouldStopForDisabledRegistration() { + // given + String name = "albert"; + Player player = mockPlayerWithName(name); + given(playerCache.isAuthenticated(name)).willReturn(false); + given(commonService.getProperty(RegistrationSettings.IS_ENABLED)).willReturn(false); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + + // when + asyncRegister.register(player, executor); + + // then + verify(commonService).send(player, MessageKey.REGISTRATION_DISABLED); + verifyZeroInteractions(dataSource, executor); + } + + @Test + public void shouldStopForAlreadyRegisteredName() { + // given + String name = "dilbert"; + Player player = mockPlayerWithName(name); + given(playerCache.isAuthenticated(name)).willReturn(false); + given(commonService.getProperty(RegistrationSettings.IS_ENABLED)).willReturn(true); + given(dataSource.isAuthAvailable(name)).willReturn(true); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + + // when + asyncRegister.register(player, executor); + + // then + verify(commonService).send(player, MessageKey.NAME_ALREADY_REGISTERED); + verify(dataSource, only()).isAuthAvailable(name); + verifyZeroInteractions(executor); + } + + @Test + public void shouldStopForFailedExecutorCheck() { + // given + String name = "edbert"; + Player player = mockPlayerWithName(name); + TestHelper.mockPlayerIp(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); + RegistrationExecutor executor = mock(RegistrationExecutor.class); + given(executor.isRegistrationAdmitted()).willReturn(false); + + // when + asyncRegister.register(player, executor); + + // then + verify(dataSource, only()).isAuthAvailable(name); + verify(executor, only()).isRegistrationAdmitted(); + } + + private static Player mockPlayerWithName(String name) { + Player player = mock(Player.class); + given(player.getName()).willReturn(name); + return player; + } +} diff --git a/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java b/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java new file mode 100644 index 000000000..aecc158a3 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/register/executors/EmailRegisterExecutorProviderTest.java @@ -0,0 +1,173 @@ +package fr.xephi.authme.process.register.executors; + +import fr.xephi.authme.ReflectionTestUtils; +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.mail.SendMailSSL; +import fr.xephi.authme.message.MessageKey; +import fr.xephi.authme.permission.PermissionsManager; +import fr.xephi.authme.permission.PlayerStatePermission; +import fr.xephi.authme.process.SyncProcessManager; +import fr.xephi.authme.security.PasswordSecurity; +import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.service.CommonService; +import fr.xephi.authme.settings.properties.EmailSettings; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static fr.xephi.authme.AuthMeMatchers.hasAuthBasicData; +import static fr.xephi.authme.AuthMeMatchers.hasAuthLocation; +import static fr.xephi.authme.AuthMeMatchers.stringWithLength; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link EmailRegisterExecutorProvider}. + */ +@RunWith(MockitoJUnitRunner.class) +public class EmailRegisterExecutorProviderTest { + + @InjectMocks + private EmailRegisterExecutorProvider emailRegisterExecutorProvider; + + @Mock + private PermissionsManager permissionsManager; + @Mock + private DataSource dataSource; + @Mock + private CommonService commonService; + @Mock + private SendMailSSL sendMailSsl; + @Mock + private SyncProcessManager syncProcessManager; + @Mock + private PasswordSecurity passwordSecurity; + + @Test + public void shouldNotPassEmailValidation() { + // given + given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); + String email = "test@example.com"; + given(dataSource.countAuthsByEmail(email)).willReturn(4); + Player player = mock(Player.class); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, email); + + // when + boolean result = executor.isRegistrationAdmitted(); + + // then + assertThat(result, equalTo(false)); + verify(dataSource).countAuthsByEmail(email); + verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + verify(commonService).send(player, MessageKey.MAX_REGISTER_EXCEEDED, "3", "4", "@"); + } + + @Test + public void shouldPassVerificationForPlayerWithPermission() { + // given + given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(3); + Player player = mock(Player.class); + given(permissionsManager.hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)).willReturn(true); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, "test@example.com"); + + // when + boolean result = executor.isRegistrationAdmitted(); + + // then + assertThat(result, equalTo(true)); + verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + } + + @Test + public void shouldPassVerificationForPreviouslyUnregisteredIp() { + // given + given(commonService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(1); + String email = "test@example.com"; + given(dataSource.countAuthsByEmail(email)).willReturn(0); + Player player = mock(Player.class); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, "test@example.com"); + + // when + boolean result = executor.isRegistrationAdmitted(); + + // then + assertThat(result, equalTo(true)); + verify(permissionsManager).hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS); + verify(dataSource).countAuthsByEmail(email); + } + + @Test + public void shouldCreatePlayerAuth() { + // given + given(commonService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(12); + given(passwordSecurity.computeHash(anyString(), anyString())).willAnswer( + invocation -> new HashedPassword(invocation.getArgument(0))); + Player player = mock(Player.class); + TestHelper.mockPlayerIp(player, "123.45.67.89"); + given(player.getName()).willReturn("Veronica"); + World world = mock(World.class); + given(world.getName()).willReturn("someWorld"); + given(player.getLocation()).willReturn(new Location(world, 48, 96, 144)); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, "test@example.com"); + + // when + PlayerAuth auth = executor.buildPlayerAuth(); + + // then + assertThat(auth, hasAuthBasicData("veronica", "Veronica", "test@example.com", "123.45.67.89")); + assertThat(auth, hasAuthLocation(48, 96, 144, "someWorld")); + assertThat(auth.getPassword().getHash(), stringWithLength(12)); + } + + @Test + @SuppressWarnings("unchecked") + public void shouldPerformActionAfterDataSourceSave() { + // given + given(sendMailSsl.sendPasswordMail(anyString(), anyString(), anyString())).willReturn(true); + Player player = mock(Player.class); + given(player.getName()).willReturn("Laleh"); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, "test@example.com"); + String password = "A892C#@"; + ReflectionTestUtils.setField((Class) executor.getClass(), executor, "password", password); + + // when + executor.executePostPersistAction(); + + // then + verify(sendMailSsl).sendPasswordMail("Laleh", "test@example.com", password); + verify(syncProcessManager).processSyncEmailRegister(player); + } + + @Test + @SuppressWarnings("unchecked") + public void shouldHandleEmailSendingFailure() { + // given + given(sendMailSsl.sendPasswordMail(anyString(), anyString(), anyString())).willReturn(false); + Player player = mock(Player.class); + given(player.getName()).willReturn("Laleh"); + RegistrationExecutor executor = emailRegisterExecutorProvider.new EmailRegisterExecutor(player, "test@example.com"); + String password = "A892C#@"; + ReflectionTestUtils.setField((Class) executor.getClass(), executor, "password", password); + + // when + executor.executePostPersistAction(); + + // then + verify(sendMailSsl).sendPasswordMail("Laleh", "test@example.com", password); + verify(commonService).send(player, MessageKey.EMAIL_SEND_FAILURE); + verifyZeroInteractions(syncProcessManager); + } + +} diff --git a/src/test/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProviderTest.java b/src/test/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProviderTest.java new file mode 100644 index 000000000..a033975f6 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/register/executors/PasswordRegisterExecutorProviderTest.java @@ -0,0 +1,152 @@ +package fr.xephi.authme.process.register.executors; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.message.MessageKey; +import fr.xephi.authme.process.SyncProcessManager; +import fr.xephi.authme.process.login.AsynchronousLogin; +import fr.xephi.authme.security.PasswordSecurity; +import fr.xephi.authme.security.crypts.HashedPassword; +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 fr.xephi.authme.settings.properties.PluginSettings; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static fr.xephi.authme.AuthMeMatchers.equalToHash; +import static fr.xephi.authme.AuthMeMatchers.hasAuthBasicData; +import static fr.xephi.authme.AuthMeMatchers.hasAuthLocation; +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link PasswordRegisterExecutorProvider}. + */ +@RunWith(MockitoJUnitRunner.class) +public class PasswordRegisterExecutorProviderTest { + + @InjectMocks + private PasswordRegisterExecutorProvider passwordRegisterExecutorProvider; + + @Mock + private ValidationService validationService; + @Mock + private CommonService commonService; + @Mock + private PasswordSecurity passwordSecurity; + @Mock + private BukkitService bukkitService; + @Mock + private SyncProcessManager syncProcessManager; + @Mock + private AsynchronousLogin asynchronousLogin; + + @Test + public void shouldCheckPasswordValidity() { + // given + String password = "myPass"; + String name = "player040"; + given(validationService.validatePassword(password, name)).willReturn(new ValidationResult()); + Player player = mockPlayerWithName(name); + RegistrationExecutor executor = passwordRegisterExecutorProvider.new PasswordRegisterExecutor(player, password, null); + + // when + boolean result = executor.isRegistrationAdmitted(); + + // then + assertThat(result, equalTo(true)); + verify(validationService).validatePassword(password, name); + } + + @Test + public void shouldDetectInvalidPasswordAndInformPlayer() { + // given + String password = "myPass"; + String name = "player040"; + given(validationService.validatePassword(password, name)).willReturn( + new ValidationResult(MessageKey.PASSWORD_CHARACTERS_ERROR, "[a-z]")); + Player player = mockPlayerWithName(name); + RegistrationExecutor executor = passwordRegisterExecutorProvider.new PasswordRegisterExecutor(player, password, null); + + // when + boolean result = executor.isRegistrationAdmitted(); + + // then + assertThat(result, equalTo(false)); + verify(validationService).validatePassword(password, name); + verify(commonService).send(player, MessageKey.PASSWORD_CHARACTERS_ERROR, "[a-z]"); + } + + @Test + public void shouldCreatePlayerAuth() { + // given + given(passwordSecurity.computeHash(anyString(), anyString())).willAnswer( + invocation -> new HashedPassword(invocation.getArgument(0))); + Player player = mockPlayerWithName("S1m0N"); + TestHelper.mockPlayerIp(player, "123.45.67.89"); + World world = mock(World.class); + given(world.getName()).willReturn("someWorld"); + given(player.getLocation()).willReturn(new Location(world, 48, 96, 144)); + RegistrationExecutor executor = passwordRegisterExecutorProvider.new PasswordRegisterExecutor(player, "pass", "mail@example.org"); + + // when + PlayerAuth auth = executor.buildPlayerAuth(); + + // then + assertThat(auth, hasAuthBasicData("s1m0n", "S1m0N", "mail@example.org", "123.45.67.89")); + assertThat(auth, hasAuthLocation(48, 96, 144, "someWorld")); + assertThat(auth.getPassword(), equalToHash("pass")); + } + + @Test + public void shouldLogPlayerIn() { + // given + given(commonService.getProperty(RegistrationSettings.FORCE_LOGIN_AFTER_REGISTER)).willReturn(false); + given(commonService.getProperty(PluginSettings.USE_ASYNC_TASKS)).willReturn(false); + Player player = mock(Player.class); + RegistrationExecutor executor = passwordRegisterExecutorProvider.new PasswordRegisterExecutor(player, "pass", "mail@example.org"); + + // when + executor.executePostPersistAction(); + + // then + TestHelper.runSyncDelayedTaskWithDelay(bukkitService); + verify(asynchronousLogin).forceLogin(player); + verify(syncProcessManager).processSyncPasswordRegister(player); + } + + @Test + public void shouldNotLogPlayerIn() { + // given + given(commonService.getProperty(RegistrationSettings.FORCE_LOGIN_AFTER_REGISTER)).willReturn(true); + Player player = mock(Player.class); + RegistrationExecutor executor = passwordRegisterExecutorProvider.new PasswordRegisterExecutor(player, "pass", "mail@example.org"); + + // when + executor.executePostPersistAction(); + + // then + verifyZeroInteractions(bukkitService, asynchronousLogin); + verify(syncProcessManager).processSyncPasswordRegister(player); + } + + private static Player mockPlayerWithName(String name) { + Player player = mock(Player.class); + given(player.getName()).willReturn(name); + return player; + } +} diff --git a/src/test/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelperTest.java b/src/test/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelperTest.java new file mode 100644 index 000000000..567cf5949 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/register/executors/PlayerAuthBuilderHelperTest.java @@ -0,0 +1,49 @@ +package fr.xephi.authme.process.register.executors; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.security.crypts.HashedPassword; +import org.bukkit.Location; +import org.bukkit.World; +import org.bukkit.entity.Player; +import org.junit.Test; + +import static fr.xephi.authme.AuthMeMatchers.hasAuthBasicData; +import static fr.xephi.authme.AuthMeMatchers.hasAuthLocation; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + +/** + * Test for {@link PlayerAuthBuilderHelper}. + */ +public class PlayerAuthBuilderHelperTest { + + @Test + public void shouldConstructPlayerAuth() { + // given + Player player = mock(Player.class); + given(player.getName()).willReturn("Noah"); + String ip = "192.168.34.47"; + TestHelper.mockPlayerIp(player, ip); + World world = mock(World.class); + given(world.getName()).willReturn("worldName"); + Location location = new Location(world, 123, 80, -99); + given(player.getLocation()).willReturn(location); + HashedPassword hashedPassword = new HashedPassword("myHash0001"); + String email = "test@example.org"; + + // when + PlayerAuth auth = PlayerAuthBuilderHelper.createPlayerAuth(player, hashedPassword, email); + + // then + assertThat(auth, hasAuthBasicData("noah", "Noah", email, ip)); + assertThat(auth, hasAuthLocation(123, 80, -99, "worldName")); + } + + @Test + public void shouldHaveHiddenConstructor() { + TestHelper.validateHasOnlyPrivateEmptyConstructor(PlayerAuthBuilderHelper.class); + } + +}