#567 Add/change email should be aware of account threshold

src/main/java/fr/xephi/authme/command/CommandService.java

@@ -39,19 +39,8 @@ public class CommandService {
     private final AntiBot antiBot;
     private final ValidationService validationService;
 
-    /**
+    /*
      * Constructor.
-     *
-     * @param authMe The plugin instance
-     * @param commandMapper Command mapper
-     * @param helpProvider Help provider
-     * @param messages Messages instance
-     * @param passwordSecurity The Password Security instance
-     * @param permissionsManager The permissions manager
-     * @param settings The settings manager
-     * @param ipAddressManager The IP address manager
-     * @param pluginHooks The plugin hooks instance
-     * @param spawnLoader The spawn loader
      */
     public CommandService(AuthMe authMe, CommandMapper commandMapper, HelpProvider helpProvider, Messages messages,
                           PasswordSecurity passwordSecurity, PermissionsManager permissionsManager, NewSetting settings,

src/main/java/fr/xephi/authme/command/executable/authme/SetEmailCommand.java

@@ -4,7 +4,9 @@ import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.command.CommandService;
 import fr.xephi.authme.command.ExecutableCommand;
+import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.command.CommandSender;
 
@@ -29,18 +31,20 @@ public class SetEmailCommand implements ExecutableCommand {
             @Override
             public void run() {
                 // Validate the user
-                PlayerAuth auth = commandService.getDataSource().getAuth(playerName);
+                DataSource dataSource = commandService.getDataSource();
+                PlayerAuth auth = dataSource.getAuth(playerName);
                 if (auth == null) {
                     commandService.send(sender, MessageKey.UNKNOWN_USER);
                     return;
-                } else if (commandService.getDataSource().isEmailStored(playerEmail)) {
+                } else if (dataSource.countAuthsByEmail(playerEmail)
+                        >= commandService.getProperty(EmailSettings.MAX_REG_PER_EMAIL)) {
                     commandService.send(sender, MessageKey.EMAIL_ALREADY_USED_ERROR);
                     return;
                 }
 
                 // Set the email address
                 auth.setEmail(playerEmail);
-                if (!commandService.getDataSource().updateEmail(auth)) {
+                if (!dataSource.updateEmail(auth)) {
                     commandService.send(sender, MessageKey.ERROR);
                     return;
                 }

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

@@ -1,6 +1,7 @@
 package fr.xephi.authme.command.executable.email;
 
 import fr.xephi.authme.AuthMe;
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.command.CommandService;
@@ -9,7 +10,7 @@ import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.security.RandomString;
 import fr.xephi.authme.security.crypts.HashedPassword;
-import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.util.StringUtils;
 import org.bukkit.entity.Player;
 
@@ -19,13 +20,12 @@ public class RecoverEmailCommand extends PlayerCommand {
 
     @Override
     public void runCommand(Player player, List<String> arguments, CommandService commandService) {
-        String playerMail = arguments.get(0);
+        final String playerMail = arguments.get(0);
         final String playerName = player.getName();
-
+        final AuthMe plugin = commandService.getAuthMe();
-        // Command logic
-        final AuthMe plugin = AuthMe.getInstance();
 
         if (plugin.mail == null) {
+            ConsoleLogger.showError("Mail API is not set");
             commandService.send(player, MessageKey.ERROR);
             return;
         }
@@ -36,7 +36,7 @@ public class RecoverEmailCommand extends PlayerCommand {
                 return;
             }
 
-            String thePass = RandomString.generate(Settings.getRecoveryPassLength);
+            String thePass = RandomString.generate(commandService.getProperty(EmailSettings.RECOVERY_PASSWORD_LENGTH));
             HashedPassword hashNew = commandService.getPasswordSecurity().computeHash(thePass, playerName);
             PlayerAuth auth;
             if (PlayerCache.getInstance().isAuthenticated(playerName)) {
@@ -47,13 +47,14 @@ public class RecoverEmailCommand extends PlayerCommand {
                 commandService.send(player, MessageKey.UNKNOWN_USER);
                 return;
             }
-            if (StringUtils.isEmpty(Settings.getmailAccount)) {
+            if (StringUtils.isEmpty(commandService.getProperty(EmailSettings.MAIL_ACCOUNT))) {
+                ConsoleLogger.showError("No mail account set in settings");
                 commandService.send(player, MessageKey.ERROR);
                 return;
             }
 
-            if (!playerMail.equalsIgnoreCase(auth.getEmail()) || playerMail.equalsIgnoreCase("your@email.com")
+            if (!playerMail.equalsIgnoreCase(auth.getEmail()) || "your@email.com".equalsIgnoreCase(playerMail)
-                || auth.getEmail().equalsIgnoreCase("your@email.com")) {
+                || "your@email.com".equalsIgnoreCase(auth.getEmail())) {
                 commandService.send(player, MessageKey.INVALID_EMAIL);
                 return;
             }

src/main/java/fr/xephi/authme/datasource/CacheDataSource.java

@@ -250,9 +250,4 @@ public class CacheDataSource implements DataSource {
     public List<PlayerAuth> getLoggedPlayers() {
         return new ArrayList<>(PlayerCache.getInstance().getCache().values());
     }
-
-    @Override
-    public boolean isEmailStored(String email) {
-        return source.isEmailStored(email);
-    }
 }

src/main/java/fr/xephi/authme/datasource/DataSource.java

@@ -185,8 +185,6 @@ public interface DataSource {
      */
     List<PlayerAuth> getLoggedPlayers();
 
-    boolean isEmailStored(String email);
-
     /**
      * Reload the data source.
      */

src/main/java/fr/xephi/authme/datasource/FlatFile.java

@@ -521,11 +521,6 @@ public class FlatFile implements DataSource {
         throw new UnsupportedOperationException("Flat file no longer supported");
     }
 
-    @Override
-    public boolean isEmailStored(String email) {
-        throw new UnsupportedOperationException("Flat file no longer supported");
-    }
-
     private static PlayerAuth buildAuthFromArray(String[] args) {
         // Format allows 2, 3, 4, 7, 8, 9 fields. Anything else is unknown
         if (args.length >= 2 && args.length <= 9 && args.length != 5 && args.length != 6) {

src/main/java/fr/xephi/authme/datasource/MySQL.java

@@ -908,20 +908,6 @@ public class MySQL implements DataSource {
         return auths;
     }
 
-    @Override
-    public synchronized boolean isEmailStored(String email) {
-        String sql = "SELECT 1 FROM " + tableName + " WHERE UPPER(" + col.EMAIL + ") = UPPER(?)";
-        try (Connection con = ds.getConnection(); PreparedStatement pst = con.prepareStatement(sql)) {
-            pst.setString(1, email);
-            try (ResultSet rs = pst.executeQuery()) {
-                return rs.next();
-            }
-        } catch (SQLException e) {
-            logSqlException(e);
-        }
-        return false;
-    }
-
     private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException {
         String salt = col.SALT.isEmpty() ? null : row.getString(col.SALT);
         int group = col.GROUP.isEmpty() ? -1 : row.getInt(col.GROUP);

src/main/java/fr/xephi/authme/datasource/SQLite.java

@@ -580,22 +580,6 @@ public class SQLite implements DataSource {
         return auths;
     }
 
-    @Override
-    public synchronized boolean isEmailStored(String email) {
-        String sql = "SELECT 1 FROM " + tableName + " WHERE " + col.EMAIL + " = ? COLLATE NOCASE;";
-        ResultSet rs = null;
-        try (PreparedStatement ps = con.prepareStatement(sql)) {
-            ps.setString(1, email);
-            rs = ps.executeQuery();
-            return rs.next();
-        } catch (SQLException e) {
-            logSqlException(e);
-        } finally {
-            close(rs);
-        }
-        return false;
-    }
-
     private PlayerAuth buildAuthFromResultSet(ResultSet row) throws SQLException {
         String salt = !col.SALT.isEmpty() ? row.getString(col.SALT) : null;
 

src/main/java/fr/xephi/authme/process/email/AsyncAddEmail.java

@@ -7,6 +7,7 @@ import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.Process;
 import fr.xephi.authme.process.ProcessService;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.entity.Player;
@@ -43,7 +44,7 @@ public class AsyncAddEmail implements Process {
                 service.send(player, MessageKey.USAGE_CHANGE_EMAIL);
             } else if (!Utils.isEmailCorrect(email, service.getSettings())) {
                 service.send(player, MessageKey.INVALID_EMAIL);
-            } else if (dataSource.isEmailStored(email)) {
+            } else if (dataSource.countAuthsByEmail(email) >= service.getProperty(EmailSettings.MAX_REG_PER_EMAIL)) {
                 service.send(player, MessageKey.EMAIL_ALREADY_USED_ERROR);
             } else {
                 auth.setEmail(email);

src/main/java/fr/xephi/authme/process/email/AsyncChangeEmail.java

@@ -6,6 +6,7 @@ import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.Process;
 import fr.xephi.authme.process.ProcessService;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.entity.Player;
@@ -45,7 +46,7 @@ public class AsyncChangeEmail implements Process {
                 service.send(player, MessageKey.INVALID_NEW_EMAIL);
             } else if (!oldEmail.equals(currentEmail)) {
                 service.send(player, MessageKey.INVALID_OLD_EMAIL);
-            } else if (dataSource.isEmailStored(newEmail)) {
+            } else if (dataSource.countAuthsByEmail(newEmail) >= service.getProperty(EmailSettings.MAX_REG_PER_EMAIL)) {
                 service.send(player, MessageKey.EMAIL_ALREADY_USED_ERROR);
             } else {
                 saveNewEmail(auth);
@@ -62,7 +63,6 @@ public class AsyncChangeEmail implements Process {
             service.send(player, MessageKey.EMAIL_CHANGED_SUCCESS);
         } else {
             service.send(player, MessageKey.ERROR);
-            auth.setEmail(newEmail);
         }
     }
 

src/main/java/fr/xephi/authme/process/register/AsyncRegister.java

@@ -12,6 +12,7 @@ import fr.xephi.authme.security.HashAlgorithm;
 import fr.xephi.authme.security.crypts.HashedPassword;
 import fr.xephi.authme.security.crypts.TwoFactor;
 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.RestrictionSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
@@ -99,12 +100,12 @@ public class AsyncRegister implements Process {
     }
 
     private void emailRegister() {
-        if (Settings.getmaxRegPerEmail > 0
+        final int maxRegPerEmail = service.getProperty(EmailSettings.MAX_REG_PER_EMAIL);
+        if (maxRegPerEmail > 0
             && !plugin.getPermissionsManager().hasPermission(player, PlayerStatePermission.ALLOW_MULTIPLE_ACCOUNTS)) {
-            int maxReg = Settings.getmaxRegPerEmail;
             int otherAccounts = database.countAuthsByEmail(email);
-            if (otherAccounts >= maxReg) {
+            if (otherAccounts >= maxRegPerEmail) {
-                service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxReg),
+                service.send(player, MessageKey.MAX_REGISTER_EXCEEDED, Integer.toString(maxRegPerEmail),
                     Integer.toString(otherAccounts), "@");
                 return;
             }

src/main/java/fr/xephi/authme/settings/Settings.java

@@ -3,6 +3,7 @@ package fr.xephi.authme.settings;
 import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.security.HashAlgorithm;
 import fr.xephi.authme.settings.domain.Property;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.settings.properties.HooksSettings;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
@@ -67,7 +68,7 @@ public final class Settings {
         getPasswordMinLen, getMovementRadius,
         getNonActivatedGroup, passwordMaxLength, getRecoveryPassLength,
         getMailPort, maxLoginTry, captchaLength, saltLength,
-        getmaxRegPerEmail, bCryptLog2Rounds, getMaxLoginPerIp, getMaxJoinPerIp;
+        bCryptLog2Rounds, getMaxLoginPerIp, getMaxJoinPerIp;
     protected static FileConfiguration configFile;
 
     /**
@@ -143,7 +144,7 @@ public final class Settings {
         rakamakUseIp = configFile.getBoolean("Converter.Rakamak.useIp", false);
         noConsoleSpam = load(SecuritySettings.REMOVE_SPAM_FROM_CONSOLE);
         removePassword = configFile.getBoolean("Security.console.removePassword", true);
-        getmailAccount = configFile.getString("Email.mailAccount", "");
+        getmailAccount = load(EmailSettings.MAIL_ACCOUNT);
         getMailPort = configFile.getInt("Email.mailPort", 465);
         getRecoveryPassLength = configFile.getInt("Email.RecoveryPasswordLength", 8);
         displayOtherAccounts = configFile.getBoolean("settings.restrictions.displayOtherAccounts", true);
@@ -151,7 +152,6 @@ public final class Settings {
         captchaLength = configFile.getInt("Security.captcha.captchaLength", 5);
         emailRegistration = load(RegistrationSettings.USE_EMAIL_REGISTRATION);
         saltLength = configFile.getInt("settings.security.doubleMD5SaltLength", 8);
-        getmaxRegPerEmail = configFile.getInt("Email.maxRegPerEmail", 1);
         multiverse = load(HooksSettings.MULTIVERSE);
         bungee = configFile.getBoolean("Hooks.bungeecord", false);
         getForcedWorlds = configFile.getStringList("settings.restrictions.ForceSpawnOnTheseWorlds");

src/test/java/fr/xephi/authme/datasource/AbstractDataSourceIntegrationTest.java

@@ -80,22 +80,6 @@ public abstract class AbstractDataSourceIntegrationTest {
         assertThat(userAuth.getPassword(), equalToHash("b28c32f624a4eb161d6adc9acb5bfc5b", "f750ba32"));
     }
 
-    @Test
-    public void shouldFindIfEmailExists() {
-        // given
-        DataSource dataSource = getDataSource();
-
-        // when
-        boolean isUserMailPresent = dataSource.isEmailStored("user@example.org");
-        boolean isUserMailPresentCaseInsensitive = dataSource.isEmailStored("user@example.ORG");
-        boolean isInvalidMailPresent = dataSource.isEmailStored("not-in-database@example.com");
-
-        // then
-        assertThat(isUserMailPresent, equalTo(true));
-        assertThat(isUserMailPresentCaseInsensitive, equalTo(true));
-        assertThat(isInvalidMailPresent, equalTo(false));
-    }
-
     @Test
     public void shouldCountAuthsByEmail() {
         // given

src/test/java/fr/xephi/authme/process/email/AsyncAddEmailTest.java

@@ -7,12 +7,14 @@ import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.ProcessService;
 import fr.xephi.authme.settings.NewSetting;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
-import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.entity.Player;
-import org.junit.After;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.any;
@@ -24,28 +26,23 @@ import static org.mockito.Mockito.when;
 /**
  * Test for {@link AsyncAddEmail}.
  */
+@RunWith(MockitoJUnitRunner.class)
 public class AsyncAddEmailTest {
 
+    @Mock
     private Player player;
+    @Mock
     private DataSource dataSource;
+    @Mock
     private PlayerCache playerCache;
+    @Mock
     private ProcessService service;
 
     @BeforeClass
     public static void setUp() {
-        WrapperMock.createInstance();
         ConsoleLoggerTestInitializer.setupLogger();
     }
 
-    // Clean up the fields to ensure that no test uses elements of another test
-    @After
-    public void removeFieldValues() {
-        player = null;
-        dataSource = null;
-        playerCache = null;
-        service = null;
-    }
-
     @Test
     public void shouldAddEmail() {
         // given
@@ -55,7 +52,7 @@ public class AsyncAddEmailTest {
         PlayerAuth auth = mock(PlayerAuth.class);
         given(auth.getEmail()).willReturn(null);
         given(playerCache.getAuth("tester")).willReturn(auth);
-        given(dataSource.isEmailStored("my.mail@example.org")).willReturn(false);
+        given(dataSource.countAuthsByEmail("my.mail@example.org")).willReturn(1);
         given(dataSource.updateEmail(any(PlayerAuth.class))).willReturn(true);
 
         // when
@@ -77,7 +74,7 @@ public class AsyncAddEmailTest {
         PlayerAuth auth = mock(PlayerAuth.class);
         given(auth.getEmail()).willReturn(null);
         given(playerCache.getAuth("tester")).willReturn(auth);
-        given(dataSource.isEmailStored("my.mail@example.org")).willReturn(false);
+        given(dataSource.countAuthsByEmail("my.mail@example.org")).willReturn(0);
         given(dataSource.updateEmail(any(PlayerAuth.class))).willReturn(false);
 
         // when
@@ -97,7 +94,7 @@ public class AsyncAddEmailTest {
         PlayerAuth auth = mock(PlayerAuth.class);
         given(auth.getEmail()).willReturn("another@mail.tld");
         given(playerCache.getAuth("my_player")).willReturn(auth);
-        given(dataSource.isEmailStored("some.mail@example.org")).willReturn(false);
+        given(dataSource.countAuthsByEmail("some.mail@example.org")).willReturn(0);
 
         // when
         process.run();
@@ -116,7 +113,7 @@ public class AsyncAddEmailTest {
         PlayerAuth auth = mock(PlayerAuth.class);
         given(auth.getEmail()).willReturn(null);
         given(playerCache.getAuth("my_player")).willReturn(auth);
-        given(dataSource.isEmailStored("invalid_mail")).willReturn(false);
+        given(dataSource.countAuthsByEmail("invalid_mail")).willReturn(0);
 
         // when
         process.run();
@@ -135,7 +132,7 @@ public class AsyncAddEmailTest {
         PlayerAuth auth = mock(PlayerAuth.class);
         given(auth.getEmail()).willReturn(null);
         given(playerCache.getAuth("testname")).willReturn(auth);
-        given(dataSource.isEmailStored("player@mail.tld")).willReturn(true);
+        given(dataSource.countAuthsByEmail("player@mail.tld")).willReturn(2);
 
         // when
         process.run();
@@ -196,17 +193,15 @@ public class AsyncAddEmailTest {
     }
 
     /**
-     * Create an instance of {@link AsyncAddEmail} and save the mcoks to this class' fields.
+     * Create an instance of {@link AsyncAddEmail} and save the mocks to this class' fields.
      *
      * @param email The email to use
      * @return The created process
      */
     private AsyncAddEmail createProcess(String email) {
-        player = mock(Player.class);
+        NewSetting settings = mock(NewSetting.class);
-        dataSource = mock(DataSource.class);
+        when(service.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).thenReturn(2);
-        playerCache = mock(PlayerCache.class);
+        when(service.getSettings()).thenReturn(settings);
-        service = mock(ProcessService.class);
-        when(service.getSettings()).thenReturn(mock(NewSetting.class));
         return new AsyncAddEmail(player, email, dataSource, playerCache, service);
     }
 

src/test/java/fr/xephi/authme/process/email/AsyncChangeEmailTest.java

@@ -6,12 +6,13 @@ import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.process.ProcessService;
 import fr.xephi.authme.settings.NewSetting;
+import fr.xephi.authme.settings.properties.EmailSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
-import fr.xephi.authme.util.WrapperMock;
 import org.bukkit.entity.Player;
-import org.junit.After;
-import org.junit.BeforeClass;
 import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
 
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Matchers.any;
@@ -23,29 +24,20 @@ import static org.mockito.Mockito.when;
 /**
  * Test for {@link AsyncChangeEmail}.
  */
+@RunWith(MockitoJUnitRunner.class)
 public class AsyncChangeEmailTest {
 
+    @Mock
     private Player player;
+    @Mock
     private PlayerCache playerCache;
+    @Mock
     private DataSource dataSource;
+    @Mock
     private ProcessService service;
+    @Mock
     private NewSetting settings;
 
-    @BeforeClass
-    public static void setUp() {
-        WrapperMock.createInstance();
-    }
-
-    // Prevent the accidental re-use of a field in another test
-    @After
-    public void cleanFields() {
-        player = null;
-        playerCache = null;
-        dataSource = null;
-        service = null;
-        settings = null;
-    }
-
     @Test
     public void shouldAddEmail() {
         // given
@@ -146,7 +138,7 @@ public class AsyncChangeEmailTest {
         given(playerCache.isAuthenticated("username")).willReturn(true);
         PlayerAuth auth = authWithMail("old@example.com");
         given(playerCache.getAuth("username")).willReturn(auth);
-        given(dataSource.isEmailStored("new@example.com")).willReturn(true);
+        given(dataSource.countAuthsByEmail("new@example.com")).willReturn(5);
 
         // when
         process.run();
@@ -217,11 +209,7 @@ public class AsyncChangeEmailTest {
     }
 
     private AsyncChangeEmail createProcess(String oldEmail, String newEmail) {
-        player = mock(Player.class);
+        given(service.getProperty(EmailSettings.MAX_REG_PER_EMAIL)).willReturn(5);
-        playerCache = mock(PlayerCache.class);
-        dataSource = mock(DataSource.class);
-        service = mock(ProcessService.class);
-        settings = mock(NewSetting.class);
         given(service.getSettings()).willReturn(settings);
         return new AsyncChangeEmail(player, oldEmail, newEmail, dataSource, playerCache, service);
     }