Email verification code (#1382)

* Base verification code implementation, must define command, manager, events
* VerificationManager, verification command and messages, handled some sensible commands, configuration values
* Improved manager and sensible commands trigger
* Updated messages
* Updated verification code manager, fixed tests
* Switched to a permission based command
* Verification manager and command improved and added tests
* Edited messages

src/test/java/fr/xephi/authme/command/CommandInitializerTest.java

@@ -44,7 +44,7 @@ public class CommandInitializerTest {
         // It obviously doesn't make sense to test much of the concrete data
         // that is being initialized; we just want to guarantee with this test
         // that data is indeed being initialized and we take a few "probes"
-        assertThat(commands, hasSize(8));
+        assertThat(commands, hasSize(9));
         assertThat(commandsIncludeLabel(commands, "authme"), equalTo(true));
         assertThat(commandsIncludeLabel(commands, "register"), equalTo(true));
         assertThat(commandsIncludeLabel(commands, "help"), equalTo(false));

src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command.executable.changepassword;
 
+import fr.xephi.authme.data.VerificationCodeManager;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.process.Management;
@@ -40,11 +41,14 @@ public class ChangePasswordCommandTest {
     private ChangePasswordCommand command;
 
     @Mock
-    private CommonService commandService;
+    private CommonService commonService;
 
     @Mock
     private PlayerCache playerCache;
 
+    @Mock
+    private VerificationCodeManager codeManager;
+
     @Mock
     private ValidationService validationService;
 
@@ -72,23 +76,24 @@ public class ChangePasswordCommandTest {
         command.executeCommand(sender, Arrays.asList("pass", "pass"));
 
         // then
-        verify(commandService).send(sender, MessageKey.NOT_LOGGED_IN);
+        verify(commonService).send(sender, MessageKey.NOT_LOGGED_IN);
     }
 
     @Test
     public void shouldRejectInvalidPassword() {
         // given
-        CommandSender sender = initPlayerWithName("abc12", true);
+        Player sender = initPlayerWithName("abc12", true);
         String password = "newPW";
-        given(validationService.validatePassword(password, "abc12"))
-            .willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH));
+        given(validationService.validatePassword(password, "abc12")).willReturn(new ValidationResult(MessageKey.INVALID_PASSWORD_LENGTH));
+        given(codeManager.isVerificationRequired(sender)).willReturn(false);
 
         // when
         command.executeCommand(sender, Arrays.asList("tester", password));
 
         // then
         verify(validationService).validatePassword(password, "abc12");
-        verify(commandService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]);
+        verify(commonService).send(sender, MessageKey.INVALID_PASSWORD_LENGTH, new String[0]);
+        verify(codeManager).isVerificationRequired(sender);
     }
 
     @Test
@@ -98,14 +103,16 @@ public class ChangePasswordCommandTest {
         String newPass = "abc123";
         Player player = initPlayerWithName("parker", true);
         given(validationService.validatePassword("abc123", "parker")).willReturn(new ValidationResult());
+        given(codeManager.isVerificationRequired(player)).willReturn(false);
 
         // when
         command.executeCommand(player, Arrays.asList(oldPass, newPass));
 
         // then
         verify(validationService).validatePassword(newPass, "parker");
-        verify(commandService, never()).send(eq(player), any(MessageKey.class));
+        verify(commonService, never()).send(eq(player), any(MessageKey.class));
         verify(management).performPasswordChange(player, oldPass, newPass);
+        verify(codeManager).isVerificationRequired(player);
     }
 
     @Test

src/test/java/fr/xephi/authme/command/executable/email/ChangeEmailCommandTest.java

@@ -1,7 +1,9 @@
 package fr.xephi.authme.command.executable.email;
 
+import fr.xephi.authme.data.VerificationCodeManager;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.process.Management;
+import fr.xephi.authme.service.CommonService;
 import org.bukkit.command.BlockCommandSender;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
@@ -16,9 +18,11 @@ import java.util.Collections;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
 
 /**
  * Test for {@link ChangeEmailCommand}.
@@ -32,6 +36,12 @@ public class ChangeEmailCommandTest {
     @Mock
     private Management management;
 
+    @Mock
+    private CommonService commonService;
+
+    @Mock
+    private VerificationCodeManager codeManager;
+
 
     @Test
     public void shouldRejectNonPlayerSender() {
@@ -45,16 +55,34 @@ public class ChangeEmailCommandTest {
         verifyZeroInteractions(management);
     }
 
+    @Test
+    public void shouldStopIfVerificationIsRequired() {
+        // given
+        String name = "Testeroni";
+        Player player = initPlayerWithName(name);
+        given(codeManager.isVerificationRequired(player)).willReturn(true);
+
+        // when
+        command.executeCommand(player, Arrays.asList("mail@example.org", "otherMail@example.com"));
+
+        // then
+        verify(codeManager).codeExistOrGenerateNew(name);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_REQUIRED);
+        verifyZeroInteractions(management);
+    }
+
     @Test
     public void shouldForwardData() {
         // given
-        Player sender = mock(Player.class);
+        Player sender = initPlayerWithName("AmATest");
+        given(codeManager.isVerificationRequired(sender)).willReturn(false);
 
         // when
         command.executeCommand(sender, Arrays.asList("new.mail@example.org", "old_mail@example.org"));
 
         // then
         verify(management).performChangeEmail(sender, "new.mail@example.org", "old_mail@example.org");
+        verify(codeManager).isVerificationRequired(sender);
     }
 
     @Test
@@ -62,4 +90,10 @@ public class ChangeEmailCommandTest {
         // given / when / then
         assertThat(command.getArgumentsMismatchMessage(), equalTo(MessageKey.USAGE_CHANGE_EMAIL));
     }
+
+    private Player initPlayerWithName(String name) {
+        Player player = mock(Player.class);
+        when(player.getName()).thenReturn(name);
+        return player;
+    }
 }

src/test/java/fr/xephi/authme/command/executable/unregister/UnregisterCommandTest.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command.executable.unregister;
 
+import fr.xephi.authme.data.VerificationCodeManager;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.process.Management;
@@ -17,11 +18,11 @@ import java.util.Collections;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
-import static org.mockito.hamcrest.MockitoHamcrest.argThat;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.hamcrest.MockitoHamcrest.argThat;
 
 /**
  * Test for {@link UnregisterCommand}.
@@ -36,11 +37,14 @@ public class UnregisterCommandTest {
     private Management management;
 
     @Mock
-    private CommonService commandService;
+    private CommonService commonService;
 
     @Mock
     private PlayerCache playerCache;
 
+    @Mock
+    private VerificationCodeManager codeManager;
+
     @Test
     public void shouldCatchUnauthenticatedUser() {
         // given
@@ -55,7 +59,26 @@ public class UnregisterCommandTest {
 
         // then
         verify(playerCache).isAuthenticated(name);
-        verify(commandService).send(player, MessageKey.NOT_LOGGED_IN);
+        verify(commonService).send(player, MessageKey.NOT_LOGGED_IN);
+        verifyZeroInteractions(management);
+    }
+
+    @Test
+    public void shouldStopForMissingVerificationCode() {
+        // given
+        String name = "asldjf";
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+        given(playerCache.isAuthenticated(name)).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(true);
+
+        // when
+        command.executeCommand(player, Collections.singletonList("blergh"));
+
+        // then
+        verify(playerCache).isAuthenticated(name);
+        verify(codeManager).codeExistOrGenerateNew(name);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_REQUIRED);
         verifyZeroInteractions(management);
     }
 
@@ -67,6 +90,7 @@ public class UnregisterCommandTest {
         Player player = mock(Player.class);
         given(player.getName()).willReturn(name);
         given(playerCache.isAuthenticated(name)).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(false);
 
         // when
         command.executeCommand(player, Collections.singletonList(password));
@@ -74,6 +98,7 @@ public class UnregisterCommandTest {
         // then
         verify(playerCache).isAuthenticated(name);
         verify(management).performUnregister(player, password);
+        verify(codeManager).isVerificationRequired(player);
     }
 
     @Test

src/test/java/fr/xephi/authme/command/executable/verification/VerificationCommandTest.java

@@ -0,0 +1,161 @@
+package fr.xephi.authme.command.executable.verification;
+
+import fr.xephi.authme.data.VerificationCodeManager;
+import fr.xephi.authme.message.MessageKey;
+import fr.xephi.authme.service.CommonService;
+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 java.util.Collections;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+/**
+ * Test for {@link VerificationCommand}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class VerificationCommandTest {
+
+    @InjectMocks
+    private VerificationCommand command;
+
+    @Mock
+    private CommonService commonService;
+
+    @Mock
+    private VerificationCodeManager codeManager;
+
+    @Test
+    public void shouldDetectIfMailHasASetup() {
+        // given
+        String name = "Alligator";
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(false);
+
+        // when
+        command.executeCommand(player, Collections.singletonList("code"));
+
+        // then
+        verify(commonService).send(player, MessageKey.INCOMPLETE_EMAIL_SETTINGS);
+    }
+
+    @Test
+    public void shouldRequireAndAcceptCode() {
+        // given
+        String name = "Duck";
+        String code = "123932";
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(true);
+        given(codeManager.isCodeRequired(name)).willReturn(true);
+        given(codeManager.checkCode(name, code)).willReturn(true);
+
+        // when
+        command.executeCommand(player, Collections.singletonList(code));
+
+        // then
+        verify(codeManager).isVerificationRequired(player);
+        verify(codeManager).isCodeRequired(name);
+        verify(codeManager).checkCode(name, code);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_VERIFIED);
+    }
+
+    @Test
+    public void shouldRejectCode() {
+        // given
+        String name = "Spider";
+        String code = "98345222";   // more than 6 digits
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(true);
+        given(codeManager.isCodeRequired(name)).willReturn(true);
+        given(codeManager.checkCode(name, code)).willReturn(false);
+
+        // when
+        command.executeCommand(player, Collections.singletonList(code));
+
+        // then
+        verify(codeManager).isVerificationRequired(player);
+        verify(codeManager).isCodeRequired(name);
+        verify(codeManager).checkCode(name, code);
+        verify(commonService).send(player, MessageKey.INCORRECT_VERIFICATION_CODE);
+    }
+
+    @Test
+    public void shouldRejectVerificationDueToExpiration() {
+        // given
+        String name = "Dog";
+        String code = "131552";
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(true);
+        given(codeManager.isCodeRequired(name)).willReturn(false);
+
+        // when
+        command.executeCommand(player, Collections.singletonList(code));
+
+        // then
+        verify(codeManager).isVerificationRequired(player);
+        verify(codeManager).isCodeRequired(name);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_EXPIRED);
+    }
+
+    @Test
+    public void shouldRejectVerificationDueToVerifiedIdentity() {
+        // given
+        String name = "Cow";
+        String code = "973583";
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(false);
+        given(codeManager.hasEmail(name)).willReturn(true);
+
+        // when
+        command.executeCommand(player, Collections.singletonList(code));
+
+        // then
+        verify(codeManager).isVerificationRequired(player);
+        verify(codeManager).hasEmail(name);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_ALREADY_VERIFIED);
+    }
+
+    @Test
+    public void shouldRejectVerificationDueToUndefinedEmail() {
+        // given
+        String name = "Frog";
+        String code = "774543";
+        Player player = mockPlayerWithName(name);
+        given(codeManager.canSendMail()).willReturn(true);
+        given(codeManager.isVerificationRequired(player)).willReturn(false);
+        given(codeManager.hasEmail(name)).willReturn(false);
+
+        // when
+        command.executeCommand(player, Collections.singletonList(code));
+
+        // then
+        verify(codeManager).isVerificationRequired(player);
+        verify(codeManager).hasEmail(name);
+        verify(commonService).send(player, MessageKey.VERIFICATION_CODE_EMAIL_NEEDED);
+        verify(commonService).send(player, MessageKey.ADD_EMAIL_MESSAGE);
+    }
+
+    @Test
+    public void shouldDefineArgumentMismatchMessage() {
+        // given / when / then
+        assertThat(command.getArgumentsMismatchMessage(), equalTo(MessageKey.USAGE_VERIFICATION_CODE));
+    }
+
+    private static Player mockPlayerWithName(String name) {
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+        return player;
+    }
+}

src/test/java/fr/xephi/authme/data/VerificationCodeManagerTest.java

@@ -0,0 +1,171 @@
+package fr.xephi.authme.data;
+
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.datasource.DataSourceResult;
+import fr.xephi.authme.mail.EmailService;
+import fr.xephi.authme.permission.PermissionsManager;
+import fr.xephi.authme.permission.PlayerPermission;
+import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.properties.SecuritySettings;
+import org.bukkit.entity.Player;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+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 VerificationCodeManager}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class VerificationCodeManagerTest {
+
+    @Mock
+    private Settings settings;
+
+    @Mock
+    private DataSource dataSource;
+
+    @Mock
+    private EmailService emailService;
+
+    @Mock
+    private PermissionsManager permissionsManager;
+
+    @Before
+    public void setUpBasicBehavior() {
+        given(emailService.hasAllInformation()).willReturn(true);
+        given(settings.getProperty(SecuritySettings.VERIFICATION_CODE_EXPIRATION_MINUTES)).willReturn(1);
+    }
+
+    @Test
+    public void shouldRequireVerification() {
+        // given
+        String name1 = "ILoveTests";
+        Player player1 = mockPlayerWithName(name1);
+        given(dataSource.getEmail(name1)).willReturn(DataSourceResult.of("ilovetests@test.com"));
+        given(permissionsManager.hasPermission(player1, PlayerPermission.VERIFICATION_CODE)).willReturn(true);
+        String name2 = "StillLovingTests";
+        Player player2 = mockPlayerWithName(name2);
+
+        VerificationCodeManager codeManager = createCodeManager();
+        codeManager.verify(name2);
+
+        // when
+        boolean test1 = codeManager.isVerificationRequired(player1);
+        boolean test2 = codeManager.isVerificationRequired(player2);
+
+        // then
+        assertThat(test1, equalTo(true));
+        assertThat(test2, equalTo(false));
+        verify(dataSource, only()).getEmail(name1);
+        verify(permissionsManager, only()).hasPermission(player1, PlayerPermission.VERIFICATION_CODE);
+    }
+
+    @Test
+    public void shouldNotRequireVerificationIfEmailSettingsAreIncomplete() {
+        // given
+        given(emailService.hasAllInformation()).willReturn(false);
+        VerificationCodeManager codeManager = createCodeManager();
+        Player player = mock(Player.class);
+
+        // when
+        boolean result = codeManager.isVerificationRequired(player);
+
+        // then
+        assertThat(result, equalTo(false));
+        verifyZeroInteractions(permissionsManager, dataSource);
+    }
+
+    @Test
+    public void shouldNotRequireVerificationForMissingPermission() {
+        // given
+        Player player = mockPlayerWithName("ILoveTests");
+        given(permissionsManager.hasPermission(player, PlayerPermission.VERIFICATION_CODE)).willReturn(false);
+        VerificationCodeManager codeManager = createCodeManager();
+
+        // when
+        boolean result = codeManager.isVerificationRequired(player);
+
+        // then
+        assertThat(result, equalTo(false));
+        verify(permissionsManager).hasPermission(player, PlayerPermission.VERIFICATION_CODE);
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldGenerateCode() {
+        // given
+        String player = "ILoveTests";
+        String email = "ilovetests@test.com";
+        given(dataSource.getEmail(player)).willReturn(DataSourceResult.of(email));
+        VerificationCodeManager codeManager1 = createCodeManager();
+        VerificationCodeManager codeManager2 = createCodeManager();
+        codeManager2.codeExistOrGenerateNew(player);
+
+        // when
+        boolean test1 = codeManager1.hasCode(player);
+        boolean test2 = codeManager2.hasCode(player);
+
+        // then
+        assertThat(test1, equalTo(false));
+        assertThat(test2, equalTo(true));
+    }
+
+    @Test
+    public void shouldRequireCode() {
+        // given
+        String player = "ILoveTests";
+        String email = "ilovetests@test.com";
+        given(dataSource.getEmail(player)).willReturn(DataSourceResult.of(email));
+        VerificationCodeManager codeManager1 = createCodeManager();
+        VerificationCodeManager codeManager2 = createCodeManager();
+        codeManager2.codeExistOrGenerateNew(player);
+
+        // when
+        boolean test1 = codeManager1.isCodeRequired(player);
+        boolean test2 = codeManager2.isCodeRequired(player);
+
+        // then
+        assertThat(test1, equalTo(false));
+        assertThat(test2, equalTo(true));
+    }
+
+    @Test
+    public void shouldVerifyCode() {
+        // given
+        String player = "ILoveTests";
+        String code = "193458";
+        String email = "ilovetests@test.com";
+        given(dataSource.getEmail(player)).willReturn(DataSourceResult.of(email));
+        VerificationCodeManager codeManager1 = createCodeManager();
+        VerificationCodeManager codeManager2 = createCodeManager();
+        codeManager1.codeExistOrGenerateNew(player);
+
+        // when
+        boolean test1 = codeManager1.checkCode(player, code);
+        boolean test2 = codeManager2.checkCode(player, code);
+
+        // then
+        assertThat(test1, equalTo(false));
+        assertThat(test2, equalTo(false));
+    }
+
+    private VerificationCodeManager createCodeManager() {
+        return new VerificationCodeManager(settings, dataSource, emailService, permissionsManager);
+    }
+
+    private static Player mockPlayerWithName(String name) {
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+        return player;
+    }
+}

src/test/java/fr/xephi/authme/settings/SettingsTest.java

@@ -77,6 +77,24 @@ public class SettingsTest {
         assertThat(result, equalTo(emailMessage));
     }
 
+    @Test
+    public void shouldLoadVerificationMessage() throws IOException {
+        // given
+        String emailMessage = "Please verify your identity with <recoverycode />.";
+        File emailFile = new File(testPluginFolder, "verification_code_email.html");
+        createFile(emailFile);
+        Files.write(emailFile.toPath(), emailMessage.getBytes());
+
+        PropertyResource resource = mock(PropertyResource.class);
+        Settings settings = new Settings(testPluginFolder, resource, null, CONFIG_DATA);
+
+        // when
+        String result = settings.getVerificationEmailMessage();
+
+        // then
+        assertThat(result, equalTo(emailMessage));
+    }
+
     private static void createFile(File file) {
         try {
             file.getParentFile().mkdirs();

src/test/java/fr/xephi/authme/util/RandomStringUtilsTest.java

@@ -61,6 +61,22 @@ public class RandomStringUtilsTest {
         }
     }
 
+    @Test
+    public void shouldGenerateRandomNumberString() {
+        // given
+        int[] lengths = {0, 1, 18, 147, 1833};
+        Pattern badChars = Pattern.compile(".*[^0-9].*");
+
+        // when / then
+        for (int length : lengths) {
+            String result = RandomStringUtils.generateNum(length);
+            assertThat("Result '" + result + "' should have length " + length,
+                result.length(), equalTo(length));
+            assertThat("Result '" + result + "' should only have characters 0-9",
+                badChars.matcher(result).matches(), equalTo(false));
+        }
+    }
+
     @Test(expected = IllegalArgumentException.class)
     public void shouldThrowForInvalidLength() {
         // given/when