From 71515f188ae3111ce7bd02cb541a5d9b2e1c59e9 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 15 Apr 2016 14:37:47 +0200 Subject: [PATCH] #551 Email registration should fail if no server email is configured - Stop registration and issue an error if the email address setting is empty for email registration - Refactor register command into smaller portions - Create tests --- .../executable/register/RegisterCommand.java | 81 ++++--- .../register/RegisterCommandTest.java | 209 ++++++++++++++++-- .../output/MessagesIntegrationTest.java | 2 +- 3 files changed, 240 insertions(+), 52 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 e488eac7f..efc688b09 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 @@ -1,20 +1,20 @@ package fr.xephi.authme.command.executable.register; +import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.output.MessageKey; -import fr.xephi.authme.process.Management; import fr.xephi.authme.security.HashAlgorithm; import fr.xephi.authme.security.RandomString; -import fr.xephi.authme.settings.Settings; import fr.xephi.authme.settings.properties.EmailSettings; -import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.entity.Player; import java.util.List; import static fr.xephi.authme.settings.properties.EmailSettings.RECOVERY_PASSWORD_LENGTH; +import static fr.xephi.authme.settings.properties.RegistrationSettings.ENABLE_CONFIRM_EMAIL; +import static fr.xephi.authme.settings.properties.RegistrationSettings.USE_EMAIL_REGISTRATION; import static fr.xephi.authme.settings.properties.RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION; public class RegisterCommand extends PlayerCommand { @@ -27,41 +27,62 @@ public class RegisterCommand extends PlayerCommand { return; } - if (arguments.isEmpty() || commandService.getProperty(ENABLE_PASSWORD_CONFIRMATION) && arguments.size() < 2) { + // Ensure that there is 1 argument, or 2 if confirmation is required + final boolean useConfirmation = isConfirmationRequired(commandService); + if (arguments.isEmpty() || useConfirmation && arguments.size() < 2) { commandService.send(player, MessageKey.USAGE_REGISTER); return; } - final Management management = commandService.getManagement(); - if (commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION) - && !commandService.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) { - boolean emailDoubleCheck = commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL); - if (emailDoubleCheck && arguments.size() < 2 || !arguments.get(0).equals(arguments.get(1))) { - commandService.send(player, MessageKey.USAGE_REGISTER); - return; - } - - final String email = arguments.get(0); - if (!commandService.validateEmail(email)) { - commandService.send(player, MessageKey.INVALID_EMAIL); - return; - } - - final String thePass = RandomString.generate(commandService.getProperty(RECOVERY_PASSWORD_LENGTH)); - management.performRegister(player, thePass, email); - return; + if (commandService.getProperty(USE_EMAIL_REGISTRATION)) { + handleEmailRegistration(player, arguments, commandService); + } else { + handlePasswordRegistration(player, arguments, commandService); } - - if (arguments.size() > 1 && Settings.enablePasswordConfirmation && !arguments.get(0).equals(arguments.get(1))) { - commandService.send(player, MessageKey.PASSWORD_MATCH_ERROR); - return; - } - - management.performRegister(player, arguments.get(0), ""); } @Override - public String getAlternativeCommand() { + protected String getAlternativeCommand() { return "/authme register "; } + + private void handlePasswordRegistration(Player player, List arguments, CommandService commandService) { + if (commandService.getProperty(ENABLE_PASSWORD_CONFIRMATION) && !arguments.get(0).equals(arguments.get(1))) { + commandService.send(player, MessageKey.PASSWORD_MATCH_ERROR); + } else { + commandService.getManagement().performRegister(player, arguments.get(0), ""); + } + } + + private void handleEmailRegistration(Player player, List arguments, CommandService commandService) { + if (commandService.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) { + player.sendMessage("Cannot register: no email address is set for the server. " + + "Please contact an administrator"); + ConsoleLogger.showError("Cannot register player '" + player.getName() + "': no email is set" + + "to send emails from. Please add one in your config at " + EmailSettings.MAIL_ACCOUNT.getPath()); + return; + } + + final String email = arguments.get(0); + if (!commandService.validateEmail(email)) { + commandService.send(player, MessageKey.INVALID_EMAIL); + } else if (commandService.getProperty(ENABLE_CONFIRM_EMAIL) && !email.equals(arguments.get(1))) { + commandService.send(player, MessageKey.USAGE_REGISTER); + } else { + String thePass = RandomString.generate(commandService.getProperty(RECOVERY_PASSWORD_LENGTH)); + commandService.getManagement().performRegister(player, thePass, email); + } + } + + /** + * Return whether the password or email has to be confirmed. + * + * @param commandService The command service + * @return True if the confirmation is needed, false otherwise + */ + private boolean isConfirmationRequired(CommandService commandService) { + return commandService.getProperty(USE_EMAIL_REGISTRATION) + ? commandService.getProperty(ENABLE_CONFIRM_EMAIL) + : commandService.getProperty(ENABLE_PASSWORD_CONFIRMATION); + } } 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 72f5e0c05..6f86a5f65 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 @@ -1,38 +1,63 @@ package fr.xephi.authme.command.executable.register; +import fr.xephi.authme.TestHelper; import fr.xephi.authme.command.CommandService; +import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.process.Management; +import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.settings.properties.EmailSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; -import fr.xephi.authme.util.WrapperMock; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.command.BlockCommandSender; import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; 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.ArrayList; +import java.util.Arrays; import java.util.Collections; -import static fr.xephi.authme.settings.properties.RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION; import static org.hamcrest.Matchers.containsString; import static org.mockito.BDDMockito.given; import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; /** * Test for {@link RegisterCommand}. */ +@RunWith(MockitoJUnitRunner.class) public class RegisterCommandTest { + @Mock private CommandService commandService; + @Mock + private Management management; + @Mock + private Player sender; + + @BeforeClass + public static void setup() { + TestHelper.setupLogger(); + } @Before - public void initializeAuthMeMock() { - WrapperMock.createInstance(); - commandService = mock(CommandService.class); + public void linkMocksAndProvideSettingDefaults() { + given(commandService.getManagement()).willReturn(management); + given(commandService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT); + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(false); + given(commandService.getProperty(RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION)).willReturn(false); } @Test @@ -45,39 +70,181 @@ public class RegisterCommandTest { command.executeCommand(sender, new ArrayList(), commandService); // then - verify(commandService, never()).getManagement(); verify(sender).sendMessage(argThat(containsString("Player only!"))); + verifyZeroInteractions(management); } @Test - public void shouldFailForEmptyArguments() { + public void shouldForwardToManagementForTwoFactor() { // given - CommandSender sender = mock(Player.class); - RegisterCommand command = new RegisterCommand(); + given(commandService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.TWO_FACTOR); + ExecutableCommand command = new RegisterCommand(); // when - command.executeCommand(sender, new ArrayList(), commandService); + command.executeCommand(sender, Collections.emptyList(), commandService); + + // then + verify(management).performRegister(sender, "", ""); + } + + @Test + public void shouldReturnErrorForEmptyArguments() { + // given + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Collections.emptyList(), commandService); // then verify(commandService).send(sender, MessageKey.USAGE_REGISTER); - verify(commandService, never()).getManagement(); + verifyZeroInteractions(management); } @Test - public void shouldForwardRegister() { + public void shouldReturnErrorForMissingConfirmation() { // given - Player sender = mock(Player.class); - RegisterCommand command = new RegisterCommand(); - Management management = mock(Management.class); - given(commandService.getManagement()).willReturn(management); - given(commandService.getProperty(ENABLE_PASSWORD_CONFIRMATION)).willReturn(false); - given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(false); + given(commandService.getProperty(RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION)).willReturn(true); + ExecutableCommand command = new RegisterCommand(); // when - command.executeCommand(sender, Collections.singletonList("password"), commandService); + command.executeCommand(sender, Collections.singletonList("arrrr"), commandService); // then - verify(management).performRegister(sender, "password", ""); + verify(commandService).send(sender, MessageKey.USAGE_REGISTER); + verifyZeroInteractions(management); } + @Test + public void shouldReturnErrorForMissingEmailConfirmation() { + // given + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); + given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Collections.singletonList("test@example.org"), commandService); + + // then + verify(commandService).send(sender, MessageKey.USAGE_REGISTER); + verifyZeroInteractions(management); + } + + @Test + public void shouldThrowErrorForMissingEmailConfiguration() { + // given + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); + given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(false); + given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn(""); + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Collections.singletonList("myMail@example.tld"), commandService); + + // then + verify(sender).sendMessage(argThat(containsString("no email address"))); + verifyZeroInteractions(management); + } + + @Test + public void shouldRejectInvalidEmail() { + // given + String playerMail = "player@example.org"; + given(commandService.validateEmail(playerMail)).willReturn(false); + + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); + given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); + given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Arrays.asList(playerMail, playerMail), commandService); + + // then + verify(commandService).validateEmail(playerMail); + verify(commandService).send(sender, MessageKey.INVALID_EMAIL); + verifyZeroInteractions(management); + } + + @Test + public void shouldRejectInvalidEmailConfirmation() { + // given + String playerMail = "bobber@bobby.org"; + given(commandService.validateEmail(playerMail)).willReturn(true); + + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); + given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); + given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Arrays.asList(playerMail, "invalid"), commandService); + + // then + verify(commandService).send(sender, MessageKey.USAGE_REGISTER); + verifyZeroInteractions(management); + } + + @Test + public void shouldPerformEmailRegistration() { + // given + String playerMail = "asfd@lakjgre.lds"; + given(commandService.validateEmail(playerMail)).willReturn(true); + int passLength = 7; + given(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH)).willReturn(passLength); + + given(commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)).willReturn(true); + given(commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL)).willReturn(true); + given(commandService.getProperty(EmailSettings.MAIL_ACCOUNT)).willReturn("server@example.com"); + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Arrays.asList(playerMail, playerMail), commandService); + + // then + verify(commandService).validateEmail(playerMail); + verify(management).performRegister(eq(sender), argThat(stringWithLength(passLength)), eq(playerMail)); + } + + @Test + public void shouldRejectInvalidPasswordConfirmation() { + // given + given(commandService.getProperty(RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION)).willReturn(true); + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Arrays.asList("myPass", "mypass"), commandService); + + // then + verify(commandService).send(sender, MessageKey.PASSWORD_MATCH_ERROR); + verifyZeroInteractions(management); + } + + @Test + public void shouldPerformPasswordValidation() { + // given + ExecutableCommand command = new RegisterCommand(); + + // when + command.executeCommand(sender, Collections.singletonList("myPass"), commandService); + + // then + verify(management).performRegister(sender, "myPass", ""); + } + + + private static TypeSafeMatcher stringWithLength(final int length) { + return new TypeSafeMatcher() { + @Override + protected boolean matchesSafely(String item) { + return item != null && item.length() == length; + } + + @Override + public void describeTo(Description description) { + description.appendText("String with length " + length); + } + }; + } } diff --git a/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java b/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java index 696d53d69..cd858ad8a 100644 --- a/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java +++ b/src/test/java/fr/xephi/authme/output/MessagesIntegrationTest.java @@ -144,7 +144,7 @@ public class MessagesIntegrationTest { messages.send(sender, key, "1234"); // then - verify(sender, times(1)).sendMessage(argThat(equalTo("Use /captcha 1234 to solve the captcha"))); + verify(sender, times(1)).sendMessage("Use /captcha 1234 to solve the captcha"); } @Test