Merge branch 'master' of https://github.com/AuthMe-Team/AuthMeReloaded

src/main/java/fr/xephi/authme/AntiBot.java

@@ -24,7 +24,7 @@ public class AntiBot {
     private final Messages messages;
     private final PermissionsManager permissionsManager;
     private final BukkitService bukkitService;
-    public final CopyOnWriteArrayList<String> antibotKicked = new CopyOnWriteArrayList<String>();
+    private final CopyOnWriteArrayList<String> antibotKicked = new CopyOnWriteArrayList<String>();
     private final CopyOnWriteArrayList<String> antibotPlayers = new CopyOnWriteArrayList<String>();
     private AntiBotStatus antiBotStatus = AntiBotStatus.DISABLED;
 
@@ -112,6 +112,27 @@ public class AntiBot {
         }, 15 * TICKS_PER_SECOND);
     }
 
+    /**
+     * Returns whether the player was kicked because of activated antibot. The list is reset
+     * when antibot is deactivated.
+     *
+     * @param name the name to check
+     * @return true if the given name has been kicked because of Antibot
+     */
+    public boolean wasPlayerKicked(String name) {
+        return antibotKicked.contains(name.toLowerCase());
+    }
+
+    /**
+     * Adds a name to the list of players kicked by antibot. Should only be used when a player
+     * is determined to be kicked because of failed antibot verification.
+     *
+     * @param name the name to add
+     */
+    public void addPlayerKick(String name) {
+        antibotKicked.addIfAbsent(name.toLowerCase());
+    }
+
     public enum AntiBotStatus {
         LISTENING,
         DISABLED,

src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java

@@ -23,7 +23,6 @@ import org.bukkit.event.entity.EntityDamageByEntityEvent;
 import org.bukkit.event.inventory.InventoryClickEvent;
 import org.bukkit.event.inventory.InventoryOpenEvent;
 import org.bukkit.event.player.AsyncPlayerChatEvent;
-import org.bukkit.event.player.AsyncPlayerPreLoginEvent;
 import org.bukkit.event.player.PlayerBedEnterEvent;
 import org.bukkit.event.player.PlayerCommandPreprocessEvent;
 import org.bukkit.event.player.PlayerDropItemEvent;
@@ -195,34 +194,10 @@ public class AuthMePlayerListener implements Listener {
         management.performJoin(player);
     }
 
-    // Note ljacqu 20160528: AsyncPlayerPreLoginEvent is not fired by all servers in offline mode
+    // Note: AsyncPlayerPreLoginEvent is not fired by all servers in offline mode
     // e.g. CraftBukkit does not. So we need to run crucial things in onPlayerLogin, too
-
-    // Note sgdc3 20160619: No performance improvements if we do the same thing on the Sync method
-    // let's try to remove this, about the single session issue,
-    // i tried to use the low priority to the sync handler
-
-    /*
-    @EventHandler(priority = EventPriority.HIGHEST)
-    public void onPreLogin(AsyncPlayerPreLoginEvent event) {
-        final String name = event.getName().toLowerCase();
-        //final boolean isAuthAvailable = dataSource.isAuthAvailable(event.getName());
-
-        try {
-            // Potential performance improvement: make checkAntiBot not require `isAuthAvailable` info and use
-            // "checkKickNonRegistered" as last -> no need to query the DB before checking antibot / name
-            // onJoinVerifier.checkAntibot(name, isAuthAvailable);
-            // onJoinVerifier.checkKickNonRegistered(isAuthAvailable);
-            // onJoinVerifier.checkIsValidName(name);
-            // Note #760: Single session must be checked here - checking with PlayerLoginEvent is too late and
-            // the first connection will have been kicked. This means this feature doesn't work on CraftBukkit.
-            onJoinVerifier.checkSingleSession(name);
-        } catch (FailedVerificationException e) {
-            event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
-            event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER);
-        }
-    }
-    */
+    // We have no performance improvements if we do the same thing on two different events
+    // Important: the single session feature works if we use the low priority to the sync handler
 
     @EventHandler(priority = EventPriority.LOW)
     public void onPlayerLogin(PlayerLoginEvent event) {
@@ -264,7 +239,7 @@ public class AuthMePlayerListener implements Listener {
             event.setQuitMessage(null);
         }
 
-        if (antiBot.antibotKicked.contains(player.getName())) {
+        if (antiBot.wasPlayerKicked(player.getName())) {
             return;
         }
 
@@ -275,7 +250,7 @@ public class AuthMePlayerListener implements Listener {
     public void onPlayerKick(PlayerKickEvent event) {
         Player player = event.getPlayer();
 
-        if (!antiBot.antibotKicked.contains(player.getName())) {
+        if (!antiBot.wasPlayerKicked(player.getName())) {
             management.performQuit(player, true);
         }
     }

src/main/java/fr/xephi/authme/listener/OnJoinVerifier.java

@@ -15,6 +15,7 @@ import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.util.BukkitService;
 import fr.xephi.authme.util.StringUtils;
+import fr.xephi.authme.util.Utils;
 import fr.xephi.authme.util.ValidationService;
 import org.bukkit.Server;
 import org.bukkit.entity.Player;
@@ -56,13 +57,7 @@ class OnJoinVerifier implements Reloadable {
     @Override
     public void reload() {
         String nickRegEx = settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS);
-        try {
-            nicknamePattern = Pattern.compile(nickRegEx);
-        } catch (Exception e) {
-            nicknamePattern = Pattern.compile(".*?");
-            ConsoleLogger.showError("Nickname pattern is not a valid regular expression! "
-                + "Fallback to allowing all nicknames");
-        }
+        nicknamePattern = Utils.safePatternCompile(nickRegEx);
     }
 
     /**
@@ -73,7 +68,7 @@ class OnJoinVerifier implements Reloadable {
      */
     public void checkAntibot(String playerName, boolean isAuthAvailable) throws FailedVerificationException {
         if (antiBot.getAntiBotStatus() == AntiBot.AntiBotStatus.ACTIVE && !isAuthAvailable) {
-            antiBot.antibotKicked.addIfAbsent(playerName);
+            antiBot.addPlayerKick(playerName);
             throw new FailedVerificationException(MessageKey.KICK_ANTIBOT);
         }
     }

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

@@ -13,7 +13,6 @@ import fr.xephi.authme.security.HashAlgorithm;
 import fr.xephi.authme.security.PasswordSecurity;
 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;
@@ -159,10 +158,7 @@ public class AsyncRegister implements AsynchronousProcess {
             return;
         }
 
-        if (!Settings.forceRegLogin && autoLogin) {
-            //PlayerCache.getInstance().addPlayer(auth);
-            //database.setLogged(name);
-            // TODO: check this...
+        if (!service.getProperty(RegistrationSettings.FORCE_LOGIN_AFTER_REGISTER) && autoLogin) {
             plugin.getManagement().performLogin(player, "dontneed", true);
         }
         syncProcessManager.processSyncPasswordRegister(player);

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

@@ -3,7 +3,6 @@ package fr.xephi.authme.settings;
 import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.settings.domain.Property;
 import fr.xephi.authme.settings.properties.PluginSettings;
-import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import org.bukkit.configuration.file.FileConfiguration;
@@ -25,7 +24,6 @@ public final class Settings {
     public static boolean protectInventoryBeforeLogInEnabled;
     public static boolean isStopEnabled;
     public static boolean reloadSupport;
-    public static boolean forceRegLogin;
     public static boolean noTeleport;
     public static boolean isRemoveSpeedEnabled;
     public static String getUnloggedinGroup;
@@ -64,7 +62,6 @@ public final class Settings {
         isStopEnabled = configFile.getBoolean("Security.SQLProblem.stopServer", true);
         reloadSupport = configFile.getBoolean("Security.ReloadCommand.useReloadCommandSupport", true);
         defaultWorld = configFile.getString("Purge.defaultWorld", "world");
-        forceRegLogin = load(RegistrationSettings.FORCE_LOGIN_AFTER_REGISTER);
         noTeleport = load(RestrictionSettings.NO_TELEPORT);
         crazyloginFileName = configFile.getString("Converter.CrazyLogin.fileName", "accounts.db");
     }

src/main/java/fr/xephi/authme/util/Utils.java

@@ -10,6 +10,7 @@ import org.bukkit.OfflinePlayer;
 import org.bukkit.entity.Player;
 
 import java.util.Arrays;
+import java.util.regex.Pattern;
 
 /**
  * Utility class for various operations used in the codebase.
@@ -79,6 +80,15 @@ public final class Utils {
         }
     }
 
+    public static Pattern safePatternCompile(String pattern) {
+        try {
+            return Pattern.compile(pattern);
+        } catch (Exception e) {
+            ConsoleLogger.showError("Failed to compile pattern '" + pattern + "' - defaulting to allowing everything");
+            return Pattern.compile(".*?");
+        }
+    }
+
     /**
      * Returns the IP of the given player.
      *

src/main/java/fr/xephi/authme/util/ValidationService.java

@@ -38,7 +38,7 @@ public class ValidationService implements Reloadable {
 
     @Override
     public void reload() {
-        passwordRegex = Pattern.compile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX));
+        passwordRegex = Utils.safePatternCompile(settings.getProperty(RestrictionSettings.ALLOWED_PASSWORD_REGEX));
     }
 
     /**

src/test/java/fr/xephi/authme/listener/OnJoinVerifierTest.java

@@ -36,6 +36,7 @@ import java.util.List;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -377,6 +378,51 @@ public class OnJoinVerifierTest {
         verifyZeroInteractions(bukkitService);
     }
 
+    @Test
+    public void shouldCheckAntiBot() throws FailedVerificationException {
+        // given
+        String name = "user123";
+        boolean hasAuth = false;
+        given(antiBot.getAntiBotStatus()).willReturn(AntiBot.AntiBotStatus.LISTENING);
+
+        // when
+        onJoinVerifier.checkAntibot(name, hasAuth);
+
+        // then
+        verify(antiBot).getAntiBotStatus();
+    }
+
+    @Test
+    public void shouldAllowUserWithAuth() throws FailedVerificationException {
+        // given
+        String name = "Bobby";
+        boolean hasAuth = true;
+        given(antiBot.getAntiBotStatus()).willReturn(AntiBot.AntiBotStatus.ACTIVE);
+
+        // when
+        onJoinVerifier.checkAntibot(name, hasAuth);
+
+        // then
+        verify(antiBot).getAntiBotStatus();
+    }
+
+    @Test
+    public void shouldThrowForActiveAntiBot() {
+        // given
+        String name = "Bobby";
+        boolean hasAuth = false;
+        given(antiBot.getAntiBotStatus()).willReturn(AntiBot.AntiBotStatus.ACTIVE);
+
+        // when / then
+        try {
+            onJoinVerifier.checkAntibot(name, hasAuth);
+            fail("Expected exception to be thrown");
+        } catch (FailedVerificationException e) {
+            assertThat(e, exceptionWithData(MessageKey.KICK_ANTIBOT));
+            verify(antiBot).addPlayerKick(name);
+        }
+    }
+
     private static Player newPlayerWithName(String name) {
         Player player = mock(Player.class);
         given(player.getName()).willReturn(name);

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

@@ -0,0 +1,93 @@
+package fr.xephi.authme.util;
+
+import fr.xephi.authme.TestHelper;
+import org.bukkit.entity.Player;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.UUID;
+import java.util.regex.Pattern;
+
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+
+/**
+ * Test for {@link Utils}.
+ */
+public class UtilsTest {
+
+    @BeforeClass
+    public static void setAuthmeInstance() {
+        TestHelper.setupLogger();
+    }
+
+    @Test
+    public void shouldCompilePattern() {
+        // given
+        String pattern = "gr(a|e)ys?";
+
+        // when
+        Pattern result = Utils.safePatternCompile(pattern);
+
+        // then
+        assertThat(result.toString(), equalTo(pattern));
+    }
+
+    @Test
+    public void shouldDefaultToAllAllowedPattern() {
+        // given
+        String invalidPattern = "gr(a|eys?"; // missing closing ')'
+
+        // when
+        Pattern result = Utils.safePatternCompile(invalidPattern);
+
+        // then
+        assertThat(result.toString(), equalTo(".*?"));
+    }
+
+    @Test
+    public void shouldGetPlayerIp() {
+        // given
+        Player player = mock(Player.class);
+        String ip = "124.86.248.62";
+        TestHelper.mockPlayerIp(player, ip);
+
+        // when
+        String result = Utils.getPlayerIp(player);
+
+        // then
+        assertThat(result, equalTo(ip));
+    }
+
+    @Test
+    public void shouldGetUuid() {
+        // given
+        UUID uuid = UUID.randomUUID();
+        Player player = mock(Player.class);
+        given(player.getUniqueId()).willReturn(uuid);
+
+        // when
+        String result = Utils.getUUIDorName(player);
+
+        // then
+        assertThat(result, equalTo(uuid.toString()));
+    }
+
+    @Test
+    public void shouldFallbackToName() {
+        // given
+        Player player = mock(Player.class);
+        doThrow(RuntimeException.class).when(player).getUniqueId();
+        String name = "Bobby12";
+        given(player.getName()).willReturn(name);
+
+        // when
+        String result = Utils.getUUIDorName(player);
+
+        // then
+        assertThat(result, equalTo(name));
+    }
+}