#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

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(USE_EMAIL_REGISTRATION)) {
-        if (commandService.getProperty(RegistrationSettings.USE_EMAIL_REGISTRATION)
+            handleEmailRegistration(player, arguments, commandService);
-                && !commandService.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) {
+        } else {
-            boolean emailDoubleCheck = commandService.getProperty(RegistrationSettings.ENABLE_CONFIRM_EMAIL);
+            handlePasswordRegistration(player, arguments, commandService);
-            if (emailDoubleCheck && arguments.size() < 2 || !arguments.get(0).equals(arguments.get(1))) {
+        }
-                commandService.send(player, MessageKey.USAGE_REGISTER);
+    }
+
+    @Override
+    protected String getAlternativeCommand() {
+        return "/authme register <playername> <password>";
+    }
+
+    private void handlePasswordRegistration(Player player, List<String> 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<String> 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);
-                return;
+        } 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);
+        }
     }
 
-            final String thePass = RandomString.generate(commandService.getProperty(RECOVERY_PASSWORD_LENGTH));
+    /**
-            management.performRegister(player, thePass, email);
+     * Return whether the password or email has to be confirmed.
-            return;
+     *
-        }
+     * @param commandService The command service
-
+     * @return True if the confirmation is needed, false otherwise
-        if (arguments.size() > 1 && Settings.enablePasswordConfirmation && !arguments.get(0).equals(arguments.get(1))) {
+     */
-            commandService.send(player, MessageKey.PASSWORD_MATCH_ERROR);
+    private boolean isConfirmationRequired(CommandService commandService) {
-            return;
+        return commandService.getProperty(USE_EMAIL_REGISTRATION)
-        }
+            ? commandService.getProperty(ENABLE_CONFIRM_EMAIL)
-
+            : commandService.getProperty(ENABLE_PASSWORD_CONFIRMATION);
-        management.performRegister(player, arguments.get(0), "");
-    }
-
-    @Override
-    public String getAlternativeCommand() {
-        return "/authme register <playername> <password>";
     }
 }

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() {
+    public void linkMocksAndProvideSettingDefaults() {
-        WrapperMock.createInstance();
+        given(commandService.getManagement()).willReturn(management);
-        commandService = mock(CommandService.class);
+        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<String>(), 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);
+        given(commandService.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.TWO_FACTOR);
-        RegisterCommand command = new RegisterCommand();
+        ExecutableCommand command = new RegisterCommand();
 
         // when
-        command.executeCommand(sender, new ArrayList<String>(), commandService);
+        command.executeCommand(sender, Collections.<String>emptyList(), commandService);
+
+        // then
+        verify(management).performRegister(sender, "", "");
+    }
+
+    @Test
+    public void shouldReturnErrorForEmptyArguments() {
+        // given
+        ExecutableCommand command = new RegisterCommand();
+
+        // when
+        command.executeCommand(sender, Collections.<String>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);
+        given(commandService.getProperty(RestrictionSettings.ENABLE_PASSWORD_CONFIRMATION)).willReturn(true);
-        RegisterCommand command = new RegisterCommand();
+        ExecutableCommand 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);
 
         // 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<String> stringWithLength(final int length) {
+        return new TypeSafeMatcher<String>() {
+            @Override
+            protected boolean matchesSafely(String item) {
+                return item != null && item.length() == length;
+            }
+
+            @Override
+            public void describeTo(Description description) {
+                description.appendText("String with length " + length);
+            }
+        };
+    }
 }

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