fix: Validate email format before trying to do anything with it

Author: Xephi

authme-core/src/main/java/fr/xephi/authme/command/executable/email/ChangeEmailCommand.java

@@ -5,6 +5,7 @@
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.service.CommonService;
+import fr.xephi.authme.service.ValidationService;
 import org.bukkit.entity.Player;
 
 import javax.inject.Inject;
@@ -24,10 +25,12 @@ public class ChangeEmailCommand extends PlayerCommand {
     @Inject
     private VerificationCodeManager codeManager;
 
+    @Inject
+    private ValidationService validationService;
+
     @Override
     public void runCommand(Player player, List<String> arguments) {
         final String playerName = player.getName();
-        // Check if the user has been verified or not
         if (codeManager.isVerificationRequired(player)) {
             codeManager.codeExistOrGenerateNew(playerName);
             commonService.send(player, MessageKey.VERIFICATION_CODE_REQUIRED);
@@ -36,6 +39,14 @@ public void runCommand(Player player, List<String> arguments) {
 
         String playerMailOld = arguments.get(0);
         String playerMailNew = arguments.get(1);
+        if (!validationService.validateEmail(playerMailOld)) {
+            commonService.send(player, MessageKey.INVALID_OLD_EMAIL);
+            return;
+        }
+        if (!validationService.validateEmail(playerMailNew)) {
+            commonService.send(player, MessageKey.INVALID_NEW_EMAIL);
+            return;
+        }
         management.performChangeEmail(player, playerMailOld, playerMailNew);
     }
 

authme-core/src/main/java/fr/xephi/authme/command/executable/email/RecoverEmailCommand.java

@@ -12,6 +12,7 @@
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.service.PasswordRecoveryService;
 import fr.xephi.authme.service.RecoveryCodeService;
+import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.entity.Player;
 
@@ -46,6 +47,9 @@ public class RecoverEmailCommand extends PlayerCommand {
     @Inject
     private BukkitService bukkitService;
 
+    @Inject
+    private ValidationService validationService;
+
     @Override
     protected void runCommand(Player player, List<String> arguments) {
         final String playerMail = arguments.get(0);
@@ -60,6 +64,10 @@ protected void runCommand(Player player, List<String> arguments) {
             commonService.send(player, MessageKey.ALREADY_LOGGED_IN_ERROR);
             return;
         }
+        if (!validationService.validateEmail(playerMail)) {
+            commonService.send(player, MessageKey.INVALID_EMAIL);
+            return;
+        }
 
         DataSourceValue<String> emailResult = dataSource.getEmail(playerName);
         if (!emailResult.rowExists()) {

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

@@ -9,6 +9,7 @@
 import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.process.Management;
 import fr.xephi.authme.service.CommonService;
+import fr.xephi.authme.service.ValidationService;
 import org.bukkit.command.BlockCommandSender;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
@@ -46,6 +47,9 @@ public class ChangeEmailCommandTest {
     @Mock
     private VerificationCodeManager codeManager;
 
+    @Mock
+    private ValidationService validationService;
+
     @Mock
     private Messages messages;
 
@@ -83,13 +87,45 @@ public void shouldForwardData() {
         // given
         Player sender = initPlayerWithName("AmATest");
         given(codeManager.isVerificationRequired(sender)).willReturn(false);
+        given(validationService.validateEmail("old_mail@example.org")).willReturn(true);
+        given(validationService.validateEmail("new.mail@example.org")).willReturn(true);
+
+        // when
+        command.executeCommand(sender, Arrays.asList("old_mail@example.org", "new.mail@example.org"));
+
+        // then
+        verify(management).performChangeEmail(sender, "old_mail@example.org", "new.mail@example.org");
+    }
+
+    @Test
+    public void shouldFailForInvalidOldEmail() {
+        // given
+        Player sender = initPlayerWithName("AmATest");
+        given(codeManager.isVerificationRequired(sender)).willReturn(false);
+        given(validationService.validateEmail("notanemail")).willReturn(false);
 
         // when
-        command.executeCommand(sender, Arrays.asList("new.mail@example.org", "old_mail@example.org"));
+        command.executeCommand(sender, Arrays.asList("notanemail", "new@example.org"));
 
         // then
-        verify(management).performChangeEmail(sender, "new.mail@example.org", "old_mail@example.org");
-        verify(codeManager).isVerificationRequired(sender);
+        verifyNoInteractions(management);
+        verify(commonService).send(sender, MessageKey.INVALID_OLD_EMAIL);
+    }
+
+    @Test
+    public void shouldFailForInvalidNewEmail() {
+        // given
+        Player sender = initPlayerWithName("AmATest");
+        given(codeManager.isVerificationRequired(sender)).willReturn(false);
+        given(validationService.validateEmail("old@example.org")).willReturn(true);
+        given(validationService.validateEmail("notanemail")).willReturn(false);
+
+        // when
+        command.executeCommand(sender, Arrays.asList("old@example.org", "notanemail"));
+
+        // then
+        verifyNoInteractions(management);
+        verify(commonService).send(sender, MessageKey.INVALID_NEW_EMAIL);
     }
 
     @Test

authme-core/src/test/java/fr/xephi/authme/command/executable/email/RecoverEmailCommandTest.java

@@ -16,9 +16,11 @@
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.service.PasswordRecoveryService;
 import fr.xephi.authme.service.RecoveryCodeService;
+import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import org.bukkit.entity.Player;
 import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mock;
 
@@ -30,6 +32,7 @@
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
@@ -70,6 +73,9 @@ public class RecoverEmailCommandTest {
     @Mock
     private BukkitService bukkitService;
 
+    @Mock
+    private ValidationService validationService;
+
     @Mock
     private Messages messages;
 
@@ -83,6 +89,11 @@ public void initSettings() {
         given(commonService.getProperty(SecuritySettings.EMAIL_RECOVERY_COOLDOWN_SECONDS)).willReturn(40);
     }
 
+    @BeforeEach
+    public void allowValidEmailsByDefault() {
+        lenient().when(validationService.validateEmail(anyString())).thenReturn(true);
+    }
+
     @Test
     public void shouldHandleMissingMailProperties() {
         // given
@@ -115,6 +126,24 @@ public void shouldShowErrorForAuthenticatedUser() {
         verify(commonService).send(sender, MessageKey.ALREADY_LOGGED_IN_ERROR);
     }
 
+    @Test
+    public void shouldRejectInvalidEmailFormat() {
+        // given
+        String name = "SomePlayer";
+        Player sender = mock(Player.class);
+        given(sender.getName()).willReturn(name);
+        given(emailService.hasAllInformation()).willReturn(true);
+        given(playerCache.isAuthenticated(name)).willReturn(false);
+        given(validationService.validateEmail("notanemail")).willReturn(false);
+
+        // when
+        command.executeCommand(sender, Collections.singletonList("notanemail"));
+
+        // then
+        verifyNoInteractions(dataSource);
+        verify(commonService).send(sender, MessageKey.INVALID_EMAIL);
+    }
+
     @Test
     public void shouldShowRegisterMessageForUnregisteredPlayer() {
         // given