From a52fb956563178890e24f8458deea749262d466a Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 17 Dec 2016 15:09:31 +0100 Subject: [PATCH] #427 Implement /register [pass] [email] variant --- .../executable/register/RegisterCommand.java | 12 ++- .../process/register/AsyncRegister.java | 22 +++-- .../properties/RegistrationArgumentType.java | 5 +- .../register/RegisterCommandTest.java | 90 ++++++++++++++----- 4 files changed, 96 insertions(+), 33 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java index 6e24fb4fa..9164ab471 100644 --- a/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/register/RegisterCommand.java @@ -19,6 +19,8 @@ import javax.inject.Inject; import java.util.List; import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWORD_LENGTH; +import static fr.xephi.authme.settings.properties.RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION; +import static fr.xephi.authme.settings.properties.RegistrationArgumentType.PASSWORD_WITH_EMAIL; import static fr.xephi.authme.settings.properties.RegistrationSettings.REGISTRATION_TYPE; /** @@ -66,11 +68,15 @@ public class RegisterCommand extends PlayerCommand { } private void handlePasswordRegistration(Player player, List arguments) { - if (commonService.getProperty(REGISTRATION_TYPE) == RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION - && !arguments.get(0).equals(arguments.get(1))) { + RegistrationArgumentType registrationType = commonService.getProperty(REGISTRATION_TYPE); + if (registrationType == PASSWORD_WITH_CONFIRMATION && !arguments.get(0).equals(arguments.get(1))) { commonService.send(player, MessageKey.PASSWORD_MATCH_ERROR); + } else if (registrationType == PASSWORD_WITH_EMAIL && !validationService.validateEmail(arguments.get(1))) { + commonService.send(player, MessageKey.INVALID_EMAIL); } else { - management.performRegister(player, arguments.get(0), "", true); + // We might only have received one argument, need to check that it's safe to do arguments.get(1) + String email = registrationType == PASSWORD_WITH_EMAIL ? arguments.get(1) : null; + management.performRegister(player, arguments.get(0), email, true); } } diff --git a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java index 02ef0ce14..7b84a73ed 100644 --- a/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/AsyncRegister.java @@ -7,23 +7,23 @@ import fr.xephi.authme.mail.SendMailSSL; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.AsynchronousProcess; -import fr.xephi.authme.service.CommonService; import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.process.login.AsynchronousLogin; import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.PasswordSecurity; import fr.xephi.authme.security.crypts.HashedPassword; import fr.xephi.authme.security.crypts.TwoFactor; +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.EmailSettings; import fr.xephi.authme.settings.properties.PluginSettings; +import fr.xephi.authme.settings.properties.RegistrationArgumentType.Execution; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.settings.properties.SecuritySettings; -import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.util.PlayerUtils; -import fr.xephi.authme.util.StringUtils; -import fr.xephi.authme.service.ValidationService; -import fr.xephi.authme.service.ValidationService.ValidationResult; import org.bukkit.Bukkit; import org.bukkit.entity.Player; @@ -31,6 +31,7 @@ import javax.inject.Inject; import java.util.List; import static fr.xephi.authme.permission.PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS; +import static fr.xephi.authme.settings.properties.RegistrationArgumentType.PASSWORD_WITH_EMAIL; /** * Asynchronous processing of a request for registration. @@ -111,10 +112,10 @@ public class AsyncRegister implements AsynchronousProcess { public void register(Player player, String password, String email, boolean autoLogin) { if (preRegisterCheck(player, password)) { - if (!StringUtils.isEmpty(email)) { + if (Execution.EMAIL == service.getProperty(RegistrationSettings.REGISTRATION_TYPE).getExecution()) { emailRegister(player, password, email); } else { - passwordRegister(player, password, autoLogin); + passwordRegister(player, password, email, autoLogin); } } } @@ -156,7 +157,8 @@ public class AsyncRegister implements AsynchronousProcess { } } - private void passwordRegister(final Player player, String password, boolean autoLogin) { + // Email arg might be the password depending on the registration type. TODO #830: Fix with more specific methods + private void passwordRegister(Player player, String password, String email, boolean autoLogin) { final String name = player.getName().toLowerCase(); final String ip = PlayerUtils.getPlayerIp(player); final HashedPassword hashedPassword = passwordSecurity.computeHash(password, name); @@ -168,6 +170,10 @@ public class AsyncRegister implements AsynchronousProcess { .location(player.getLocation()) .build(); + if (service.getProperty(RegistrationSettings.REGISTRATION_TYPE) == PASSWORD_WITH_EMAIL) { + auth.setEmail(email); + } + if (!database.saveAuth(auth)) { service.send(player, MessageKey.ERROR); return; diff --git a/src/main/java/fr/xephi/authme/settings/properties/RegistrationArgumentType.java b/src/main/java/fr/xephi/authme/settings/properties/RegistrationArgumentType.java index 72e274753..b06905083 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/RegistrationArgumentType.java +++ b/src/main/java/fr/xephi/authme/settings/properties/RegistrationArgumentType.java @@ -15,9 +15,10 @@ public enum RegistrationArgumentType { EMAIL(Execution.EMAIL, 1), /** /register [email] [email] */ - EMAIL_WITH_CONFIRMATION(Execution.EMAIL, 2); + EMAIL_WITH_CONFIRMATION(Execution.EMAIL, 2), - // TODO #427: PASSWORD_WITH_EMAIL(PASSWORD, 2); + /** /register [password] [email] */ + PASSWORD_WITH_EMAIL(Execution.PASSWORD, 2); private final Execution execution; private final int requiredNumberOfArgs; diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java index 049e3577b..2c2947c3b 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java @@ -44,7 +44,7 @@ public class RegisterCommandTest { private RegisterCommand command; @Mock - private CommonService commandService; + private CommonService commonService; @Mock private Management management; @@ -62,8 +62,8 @@ public class RegisterCommandTest { @Before public void linkMocksAndProvideSettingDefaults() { - given(commandService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT); - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD); + given(commonService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD); } @Test @@ -82,7 +82,7 @@ public class RegisterCommandTest { @Test public void shouldForwardToManagementForTwoFactor() { // given - given(commandService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.TWO_FACTOR); + given(commonService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.TWO_FACTOR); Player player = mock(Player.class); // when @@ -102,42 +102,42 @@ public class RegisterCommandTest { command.executeCommand(player, Collections.emptyList()); // then - verify(commandService).send(player, MessageKey.USAGE_REGISTER); + verify(commonService).send(player, MessageKey.USAGE_REGISTER); verifyZeroInteractions(management, sendMailSsl); } @Test public void shouldReturnErrorForMissingConfirmation() { // given - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION); Player player = mock(Player.class); // when command.executeCommand(player, Collections.singletonList("arrrr")); // then - verify(commandService).send(player, MessageKey.USAGE_REGISTER); + verify(commonService).send(player, MessageKey.USAGE_REGISTER); verifyZeroInteractions(management, sendMailSsl); } @Test public void shouldReturnErrorForMissingEmailConfirmation() { // given - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); Player player = mock(Player.class); // when command.executeCommand(player, Collections.singletonList("test@example.org")); // then - verify(commandService).send(player, MessageKey.USAGE_REGISTER); + verify(commonService).send(player, MessageKey.USAGE_REGISTER); verifyZeroInteractions(management, sendMailSsl); } @Test public void shouldThrowErrorForMissingEmailConfiguration() { // given - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL); given(sendMailSsl.hasAllInformation()).willReturn(false); Player player = mock(Player.class); @@ -145,7 +145,7 @@ public class RegisterCommandTest { command.executeCommand(player, Collections.singletonList("myMail@example.tld")); // then - verify(commandService).send(player, MessageKey.INCOMPLETE_EMAIL_SETTINGS); + verify(commonService).send(player, MessageKey.INCOMPLETE_EMAIL_SETTINGS); verify(sendMailSsl).hasAllInformation(); verifyZeroInteractions(management); } @@ -155,7 +155,7 @@ public class RegisterCommandTest { // given String playerMail = "player@example.org"; given(validationService.validateEmail(playerMail)).willReturn(false); - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); @@ -164,7 +164,7 @@ public class RegisterCommandTest { // then verify(validationService).validateEmail(playerMail); - verify(commandService).send(player, MessageKey.INVALID_EMAIL); + verify(commonService).send(player, MessageKey.INVALID_EMAIL); verifyZeroInteractions(management); } @@ -173,7 +173,7 @@ public class RegisterCommandTest { // given String playerMail = "bobber@bobby.org"; given(validationService.validateEmail(playerMail)).willReturn(true); - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); @@ -181,7 +181,7 @@ public class RegisterCommandTest { command.executeCommand(player, Arrays.asList(playerMail, "invalid")); // then - verify(commandService).send(player, MessageKey.USAGE_REGISTER); + verify(commonService).send(player, MessageKey.USAGE_REGISTER); verify(sendMailSsl).hasAllInformation(); verifyZeroInteractions(management); } @@ -192,9 +192,9 @@ public class RegisterCommandTest { String playerMail = "asfd@lakjgre.lds"; given(validationService.validateEmail(playerMail)).willReturn(true); int passLength = 7; - given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(passLength); + given(commonService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(passLength); - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.EMAIL_WITH_CONFIRMATION); given(sendMailSsl.hasAllInformation()).willReturn(true); Player player = mock(Player.class); @@ -210,14 +210,14 @@ public class RegisterCommandTest { @Test public void shouldRejectInvalidPasswordConfirmation() { // given - given(commandService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION); + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)).willReturn(RegistrationArgumentType.PASSWORD_WITH_CONFIRMATION); Player player = mock(Player.class); // when command.executeCommand(player, Arrays.asList("myPass", "mypass")); // then - verify(commandService).send(player, MessageKey.PASSWORD_MATCH_ERROR); + verify(commonService).send(player, MessageKey.PASSWORD_MATCH_ERROR); verifyZeroInteractions(management, sendMailSsl); } @@ -230,6 +230,56 @@ public class RegisterCommandTest { command.executeCommand(player, Collections.singletonList("myPass")); // then - verify(management).performRegister(player, "myPass", "", true); + verify(management).performRegister(player, "myPass", null, true); + } + + @Test + public void shouldPerformMailValidationForPasswordWithEmail() { + // given + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)) + .willReturn(RegistrationArgumentType.PASSWORD_WITH_EMAIL); + String email = "email@example.org"; + given(validationService.validateEmail(email)).willReturn(true); + Player player = mock(Player.class); + + // when + command.executeCommand(player, Arrays.asList("myPass", email)); + + // then + verify(validationService).validateEmail(email); + verify(management).performRegister(player, "myPass", email, true); + } + + @Test + public void shouldStopForInvalidEmail() { + // given + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)) + .willReturn(RegistrationArgumentType.PASSWORD_WITH_EMAIL); + String email = "email@example.org"; + given(validationService.validateEmail(email)).willReturn(false); + Player player = mock(Player.class); + + // when + command.executeCommand(player, Arrays.asList("myPass", email)); + + // then + verify(validationService).validateEmail(email); + verify(commonService).send(player, MessageKey.INVALID_EMAIL); + verifyZeroInteractions(management); + } + + @Test + public void shouldFailForInsufficientArguments() { + // given + given(commonService.getProperty(RegistrationSettings.REGISTRATION_TYPE)) + .willReturn(RegistrationArgumentType.PASSWORD_WITH_EMAIL); + Player player = mock(Player.class); + + // when + command.executeCommand(player, Collections.singletonList("myPass")); + + // then + verify(commonService).send(player, MessageKey.USAGE_REGISTER); + verifyZeroInteractions(management); } }