Unit test verifications done on joining

- Write unit tests for checks done when a player joins
- Move join event handler methods back to PlayerListener; move join check logic to new separate class

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

@@ -22,7 +22,6 @@ import fr.xephi.authme.initialization.MetricsStarter;
 import fr.xephi.authme.listener.AuthMeBlockListener;
 import fr.xephi.authme.listener.AuthMeEntityListener;
 import fr.xephi.authme.listener.AuthMeInventoryPacketAdapter;
-import fr.xephi.authme.listener.AuthMePlayerJoinListener;
 import fr.xephi.authme.listener.AuthMePlayerListener;
 import fr.xephi.authme.listener.AuthMePlayerListener16;
 import fr.xephi.authme.listener.AuthMePlayerListener18;
@@ -360,7 +359,6 @@ public class AuthMe extends JavaPlugin {
         pluginManager.registerEvents(initializer.get(AuthMeBlockListener.class),  this);
         pluginManager.registerEvents(initializer.get(AuthMeEntityListener.class), this);
         pluginManager.registerEvents(initializer.get(AuthMeServerListener.class), this);
-        pluginManager.registerEvents(initializer.get(AuthMePlayerJoinListener.class), this);
 
         // Try to register 1.6 player listeners
         try {

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

@@ -1,6 +1,9 @@
 package fr.xephi.authme.listener;
 
+import com.google.common.io.ByteArrayDataOutput;
+import com.google.common.io.ByteStreams;
 import fr.xephi.authme.AntiBot;
+import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.hooks.PluginHooks;
@@ -24,6 +27,7 @@ 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;
@@ -33,6 +37,7 @@ import org.bukkit.event.player.PlayerInteractEvent;
 import org.bukkit.event.player.PlayerItemConsumeEvent;
 import org.bukkit.event.player.PlayerJoinEvent;
 import org.bukkit.event.player.PlayerKickEvent;
+import org.bukkit.event.player.PlayerLoginEvent;
 import org.bukkit.event.player.PlayerMoveEvent;
 import org.bukkit.event.player.PlayerPickupItemEvent;
 import org.bukkit.event.player.PlayerQuitEvent;
@@ -72,6 +77,10 @@ public class AuthMePlayerListener implements Listener {
     private SpawnLoader spawnLoader;
     @Inject
     private PluginHooks pluginHooks;
+    @Inject
+    private OnJoinVerifier onJoinVerifier;
+    @Inject
+    private AuthMe plugin;
 
     private void sendLoginOrRegisterMessage(final Player player) {
         bukkitService.runTaskAsynchronously(new Runnable() {
@@ -208,6 +217,77 @@ public class AuthMePlayerListener implements Listener {
         }
     }
 
+    @EventHandler(priority = EventPriority.LOW)
+    public void onPlayerJoin(PlayerJoinEvent event) {
+        final Player player = event.getPlayer();
+        if (player != null) {
+            // Schedule login task so works after the prelogin
+            // (Fix found by Koolaid5000)
+            bukkitService.runTask(new Runnable() {
+                @Override
+                public void run() {
+                    management.performJoin(player);
+                }
+            });
+        }
+    }
+
+    // Note ljacqu 20160528: 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
+    @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);
+        } catch (FailedVerificationException e) {
+            event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
+            event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER);
+        }
+    }
+
+    @EventHandler(priority = EventPriority.HIGHEST)
+    public void onPlayerLogin(PlayerLoginEvent event) {
+        final Player player = event.getPlayer();
+        if (player == null || Utils.isUnrestricted(player)) {
+            return;
+        } else if (onJoinVerifier.refusePlayerForFullServer(event)) {
+            return;
+        } else if (event.getResult() != PlayerLoginEvent.Result.ALLOWED) {
+            return;
+        }
+
+        final String name = player.getName().toLowerCase();
+        final PlayerAuth auth = dataSource.getAuth(player.getName());
+        final boolean isAuthAvailable = auth != null;
+
+        try {
+            onJoinVerifier.checkAntibot(name, isAuthAvailable);
+            onJoinVerifier.checkKickNonRegistered(isAuthAvailable);
+            onJoinVerifier.checkIsValidName(name);
+            onJoinVerifier.checkNameCasing(player, auth);
+            onJoinVerifier.checkSingleSession(player);
+            onJoinVerifier.checkPlayerCountry(isAuthAvailable, event);
+        } catch (FailedVerificationException e) {
+            event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
+            event.setResult(PlayerLoginEvent.Result.KICK_OTHER);
+            return;
+        }
+
+        antiBot.handlePlayerJoin(player);
+
+        if (settings.getProperty(HooksSettings.BUNGEECORD)) {
+            ByteArrayDataOutput out = ByteStreams.newDataOutput();
+            out.writeUTF("IP");
+            player.sendPluginMessage(plugin, "BungeeCord", out.toByteArray());
+        }
+    }
+
     @EventHandler(priority = EventPriority.HIGHEST)
     public void onPlayerQuit(PlayerQuitEvent event) {
         Player player = event.getPlayer();

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

@@ -0,0 +1,32 @@
+package fr.xephi.authme.listener;
+
+import fr.xephi.authme.output.MessageKey;
+import fr.xephi.authme.util.StringUtils;
+
+/**
+ * Exception thrown when a verification has failed.
+ */
+public class FailedVerificationException extends Exception {
+
+    private final MessageKey reason;
+    private final String[] args;
+
+    public FailedVerificationException(MessageKey reason, String... args) {
+        this.reason = reason;
+        this.args = args;
+    }
+
+    public MessageKey getReason() {
+        return reason;
+    }
+
+    public String[] getArgs() {
+        return args;
+    }
+
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + ": reason=" + (reason == null ? "null" : reason)
+            + ";args=" + (args == null ? "null" : StringUtils.join(", ", args));
+    }
+}

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

@@ -1,9 +1,6 @@
 package fr.xephi.authme.listener;
 
-import com.google.common.io.ByteArrayDataOutput;
-import com.google.common.io.ByteStreams;
 import fr.xephi.authme.AntiBot;
-import fr.xephi.authme.AuthMe;
 import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.cache.auth.PlayerAuth;
 import fr.xephi.authme.cache.auth.PlayerCache;
@@ -15,9 +12,7 @@ import fr.xephi.authme.output.MessageKey;
 import fr.xephi.authme.output.Messages;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.permission.PlayerStatePermission;
-import fr.xephi.authme.process.Management;
 import fr.xephi.authme.settings.NewSetting;
-import fr.xephi.authme.settings.properties.HooksSettings;
 import fr.xephi.authme.settings.properties.ProtectionSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
@@ -25,12 +20,8 @@ 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;
-import org.bukkit.event.EventHandler;
-import org.bukkit.event.EventPriority;
-import org.bukkit.event.Listener;
-import org.bukkit.event.player.AsyncPlayerPreLoginEvent;
-import org.bukkit.event.player.PlayerJoinEvent;
 import org.bukkit.event.player.PlayerLoginEvent;
 
 import javax.annotation.PostConstruct;
@@ -39,103 +30,33 @@ import java.util.Collection;
 import java.util.regex.Pattern;
 
 /**
- * Listener for player join events.
+ * Service for performing various verifications when a player joins.
  */
-public class AuthMePlayerJoinListener implements Listener, Reloadable {
+class OnJoinVerifier implements Reloadable {
 
-    @Inject
-    private BukkitService bukkitService;
-    @Inject
-    private DataSource dataSource;
-    @Inject
-    private AntiBot antiBot;
-    @Inject
-    private Management management;
     @Inject
     private NewSetting settings;
     @Inject
-    private Messages m;
+    private DataSource dataSource;
+    @Inject
+    private Messages messages;
     @Inject
     private PermissionsManager permissionsManager;
     @Inject
+    private AntiBot antiBot;
+    @Inject
     private ValidationService validationService;
     @Inject
-    private AuthMe plugin;
+    private BukkitService bukkitService;
     @Inject
     private LimboCache limboCache;
+    @Inject
+    private Server server;
 
     private Pattern nicknamePattern;
 
-    @EventHandler(priority = EventPriority.LOW)
+    OnJoinVerifier() { }
-    public void onPlayerJoin(PlayerJoinEvent event) {
-        final Player player = event.getPlayer();
-        if (player != null) {
-            // Schedule login task so works after the prelogin
-            // (Fix found by Koolaid5000)
-            bukkitService.runTask(new Runnable() {
-                @Override
-                public void run() {
-                    management.performJoin(player);
-                }
-            });
-        }
-    }
 
-    // Note ljacqu 20160528: 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
-    @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
-            checkAntibot(name, isAuthAvailable);
-            checkKickNonRegistered(isAuthAvailable);
-            checkIsValidName(name);
-        } catch (VerificationFailedException e) {
-            event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
-            event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER);
-        }
-    }
-
-    @EventHandler(priority = EventPriority.HIGHEST)
-    public void onPlayerLogin(PlayerLoginEvent event) {
-        final Player player = event.getPlayer();
-        if (player == null || Utils.isUnrestricted(player)) {
-            return;
-        } else if (refusePlayerForFullServer(event)) {
-            return;
-        } else if (event.getResult() != PlayerLoginEvent.Result.ALLOWED) {
-            return;
-        }
-
-        final String name = player.getName().toLowerCase();
-        final PlayerAuth auth = dataSource.getAuth(player.getName());
-        final boolean isAuthAvailable = auth != null;
-
-        try {
-            checkAntibot(name, isAuthAvailable);
-            checkKickNonRegistered(isAuthAvailable);
-            checkIsValidName(name);
-            checkNameCasing(player, auth);
-            checkSingleSession(player);
-            checkPlayerCountry(isAuthAvailable, event);
-        } catch (VerificationFailedException e) {
-            event.setKickMessage(m.retrieveSingle(e.getReason(), e.getArgs()));
-            event.setResult(PlayerLoginEvent.Result.KICK_OTHER);
-            return;
-        }
-
-        antiBot.handlePlayerJoin(player);
-
-        if (settings.getProperty(HooksSettings.BUNGEECORD)) {
-            ByteArrayDataOutput out = ByteStreams.newDataOutput();
-            out.writeUTF("IP");
-            player.sendPluginMessage(plugin, "BungeeCord", out.toByteArray());
-        }
-    }
 
     @PostConstruct
     @Override
@@ -150,6 +71,141 @@ public class AuthMePlayerJoinListener implements Listener, Reloadable {
         }
     }
 
+    /**
+     * Checks if Antibot is enabled.
+     *
+     * @param playerName the name of the player (lowercase)
+     * @param isAuthAvailable whether or not the player is registered
+     */
+    public void checkAntibot(String playerName, boolean isAuthAvailable) throws FailedVerificationException {
+        if (antiBot.getAntiBotStatus() == AntiBot.AntiBotStatus.ACTIVE && !isAuthAvailable) {
+            antiBot.antibotKicked.addIfAbsent(playerName);
+            throw new FailedVerificationException(MessageKey.KICK_ANTIBOT);
+        }
+    }
+
+    /**
+     * Checks whether non-registered players should be kicked, and if so, whether the player should be kicked.
+     *
+     * @param isAuthAvailable whether or not the player is registered
+     */
+    public void checkKickNonRegistered(boolean isAuthAvailable) throws FailedVerificationException {
+        if (!isAuthAvailable && settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)) {
+            throw new FailedVerificationException(MessageKey.MUST_REGISTER_MESSAGE);
+        }
+    }
+
+    /**
+     * Checks that the name adheres to the configured username restrictions.
+     *
+     * @param name the name to verify
+     */
+    public void checkIsValidName(String name) throws FailedVerificationException {
+        if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)
+            || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) {
+            throw new FailedVerificationException(MessageKey.INVALID_NAME_LENGTH);
+        }
+        if (!nicknamePattern.matcher(name).matches()) {
+            throw new FailedVerificationException(MessageKey.INVALID_NAME_CHARACTERS, nicknamePattern.pattern());
+        }
+    }
+
+    /**
+     * Handles the case of a full server and verifies if the user's connection should really be refused
+     * by adjusting the event object accordingly. Attempts to kick a non-VIP player to make room if the
+     * joining player is a VIP.
+     *
+     * @param event the login event to verify
+     * @return true if the player's connection should be refused (i.e. the event does not need to be processed
+     * further), false if the player is not refused
+     */
+    public boolean refusePlayerForFullServer(PlayerLoginEvent event) {
+        final Player player = event.getPlayer();
+        if (event.getResult() != PlayerLoginEvent.Result.KICK_FULL) {
+            // Server is not full, no need to do anything
+            return false;
+        } else if (!permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) {
+            // Server is full and player is NOT VIP; set kick message and proceed with kick
+            event.setKickMessage(messages.retrieveSingle(MessageKey.KICK_FULL_SERVER));
+            return true;
+        }
+
+        // Server is full and player is VIP; attempt to kick a non-VIP player to make room
+        Collection<? extends Player> onlinePlayers = bukkitService.getOnlinePlayers();
+        if (onlinePlayers.size() < server.getMaxPlayers()) {
+            event.allow();
+            return false;
+        }
+        Player nonVipPlayer = generateKickPlayer(onlinePlayers);
+        if (nonVipPlayer != null) {
+            nonVipPlayer.kickPlayer(messages.retrieveSingle(MessageKey.KICK_FOR_VIP));
+            event.allow();
+            return false;
+        } else {
+            ConsoleLogger.info("VIP player " + player.getName() + " tried to join, but the server was full");
+            event.setKickMessage(messages.retrieveSingle(MessageKey.KICK_FULL_SERVER));
+            return true;
+        }
+    }
+
+    /**
+     * Checks that the casing in the username corresponds to the one in the database, if so configured.
+     *
+     * @param player the player to verify
+     * @param auth the auth object associated with the player
+     */
+    public void checkNameCasing(Player player, PlayerAuth auth) throws FailedVerificationException {
+        if (auth != null && settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)) {
+            String realName = auth.getRealName(); // might be null or "Player"
+            String connectingName = player.getName();
+
+            if (StringUtils.isEmpty(realName) || "Player".equals(realName)) {
+                dataSource.updateRealName(connectingName.toLowerCase(), connectingName);
+            } else if (!realName.equals(connectingName)) {
+                throw new FailedVerificationException(MessageKey.INVALID_NAME_CASE, realName, connectingName);
+            }
+        }
+    }
+
+    /**
+     * Checks that the player's country is admitted if he is not registered.
+     *
+     * @param isAuthAvailable whether or not the user is registered
+     * @param event the login event of the player
+     */
+    public void checkPlayerCountry(boolean isAuthAvailable,
+                                    PlayerLoginEvent event) throws FailedVerificationException {
+        if (!isAuthAvailable && settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)) {
+            String playerIp = event.getAddress().getHostAddress();
+            if (!validationService.isCountryAdmitted(playerIp)) {
+                throw new FailedVerificationException(MessageKey.COUNTRY_BANNED_ERROR);
+            }
+        }
+    }
+
+    /**
+     * Checks if a player with the same name (case-insensitive) is already playing and refuses the
+     * connection if so configured.
+     *
+     * @param player the player to verify
+     */
+    public void checkSingleSession(Player player) throws FailedVerificationException {
+        if (!settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) {
+            return;
+        }
+
+        Player onlinePlayer = bukkitService.getPlayerExact(player.getName());
+        if (onlinePlayer != null) {
+            String name = player.getName().toLowerCase();
+            LimboPlayer limbo = limboCache.getLimboPlayer(name);
+            if (limbo != null && PlayerCache.getInstance().isAuthenticated(name)) {
+                Utils.addNormal(player, limbo.getGroup());
+                limboCache.deleteLimboPlayer(name);
+            }
+            throw new FailedVerificationException(MessageKey.USERNAME_ALREADY_ONLINE_ERROR);
+        }
+    }
+
     /**
      * Selects a non-VIP player to kick when a VIP player joins the server when full.
      *
@@ -164,160 +220,4 @@ public class AuthMePlayerJoinListener implements Listener, Reloadable {
         }
         return null;
     }
-
-    /**
-     * Checks if Antibot is enabled.
-     *
-     * @param playerName the name of the player (lowercase)
-     * @param isAuthAvailable whether or not the player is registered
-     */
-    private void checkAntibot(String playerName, boolean isAuthAvailable) throws VerificationFailedException {
-        if (antiBot.getAntiBotStatus() == AntiBot.AntiBotStatus.ACTIVE && !isAuthAvailable) {
-            antiBot.antibotKicked.addIfAbsent(playerName);
-            throw new VerificationFailedException(MessageKey.KICK_ANTIBOT);
-        }
-    }
-
-    /**
-     * Checks whether non-registered players should be kicked, and if so, whether the player should be kicked.
-     *
-     * @param isAuthAvailable whether or not the player is registered
-     */
-    private void checkKickNonRegistered(boolean isAuthAvailable) throws VerificationFailedException {
-        if (!isAuthAvailable && settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)) {
-            throw new VerificationFailedException(MessageKey.MUST_REGISTER_MESSAGE);
-        }
-    }
-
-    /**
-     * Checks that the name adheres to the configured username restrictions.
-     *
-     * @param name the name to verify
-     */
-    private void checkIsValidName(String name) throws VerificationFailedException {
-        if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)
-            || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) {
-            throw new VerificationFailedException(MessageKey.INVALID_NAME_LENGTH);
-        }
-        if (!nicknamePattern.matcher(name).matches()) {
-            throw new VerificationFailedException(MessageKey.INVALID_NAME_CHARACTERS, nicknamePattern.pattern());
-        }
-    }
-
-    /**
-     * Handles the case of a full server and verifies if the user's connection should really be refused
-     * by adjusting the event object accordingly. Attempts to kick a non-VIP player to make room if the
-     * joining player is a VIP.
-     *
-     * @param event the login event to verify
-     * @return true if the player's connection should be refused (i.e. the event does not need to be processed
-     * further), false if the player is not refused
-     */
-    private boolean refusePlayerForFullServer(PlayerLoginEvent event) {
-        final Player player = event.getPlayer();
-        if (event.getResult() != PlayerLoginEvent.Result.KICK_FULL) {
-            // Server is not full, no need to do anything
-            return false;
-        } else if (!permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) {
-            // Server is full and player is NOT VIP; set kick message and proceed with kick
-            event.setKickMessage(m.retrieveSingle(MessageKey.KICK_FULL_SERVER));
-            return true;
-        }
-
-        // Server is full and player is VIP; attempt to kick a non-VIP player to make room
-        Collection<? extends Player> onlinePlayers = bukkitService.getOnlinePlayers();
-        if (onlinePlayers.size() < plugin.getServer().getMaxPlayers()) {
-            event.allow();
-            return false;
-        }
-        Player nonVipPlayer = generateKickPlayer(onlinePlayers);
-        if (nonVipPlayer != null) {
-            nonVipPlayer.kickPlayer(m.retrieveSingle(MessageKey.KICK_FOR_VIP));
-            event.allow();
-            return false;
-        } else {
-            ConsoleLogger.info("VIP player " + player.getName() + " tried to join, but the server was full");
-            event.setKickMessage(m.retrieveSingle(MessageKey.KICK_FULL_SERVER));
-            return true;
-        }
-    }
-
-    /**
-     * Checks that the casing in the username corresponds to the one in the database, if so configured.
-     *
-     * @param player the player to verify
-     * @param auth the auth object associated with the player
-     */
-    private void checkNameCasing(Player player, PlayerAuth auth) throws VerificationFailedException {
-        if (auth != null && settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)) {
-            String realName = auth.getRealName(); // might be null or "Player"
-            String connectingName = player.getName();
-
-            if (StringUtils.isEmpty(realName) || "Player".equals(realName)) {
-                dataSource.updateRealName(connectingName.toLowerCase(), connectingName);
-            } else if (!realName.equals(connectingName)) {
-                throw new VerificationFailedException(MessageKey.INVALID_NAME_CASE, realName, connectingName);
-            }
-        }
-    }
-
-    /**
-     * Checks that the player's country is admitted if he is not registered.
-     *
-     * @param isAuthAvailable whether or not the user is registered
-     * @param event the login event of the player
-     */
-    private void checkPlayerCountry(boolean isAuthAvailable,
-                                    PlayerLoginEvent event) throws VerificationFailedException {
-        if (!isAuthAvailable && settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)) {
-            String playerIp = event.getAddress().getHostAddress();
-            if (!validationService.isCountryAdmitted(playerIp)) {
-                throw new VerificationFailedException(MessageKey.COUNTRY_BANNED_ERROR);
-            }
-        }
-    }
-
-    /**
-     * Checks if a player with the same name (case-insensitive) is already playing and refuses the
-     * connection if so configured.
-     *
-     * @param player the player to verify
-     */
-    private void checkSingleSession(Player player) throws VerificationFailedException {
-        if (!settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) {
-            return;
-        }
-
-        Player onlinePlayer = bukkitService.getPlayerExact(player.getName());
-        if (onlinePlayer != null) {
-            String name = player.getName().toLowerCase();
-            LimboPlayer limbo = limboCache.getLimboPlayer(name);
-            if (limbo != null && PlayerCache.getInstance().isAuthenticated(name)) {
-                Utils.addNormal(player, limbo.getGroup());
-                limboCache.deleteLimboPlayer(name);
-            }
-            throw new VerificationFailedException(MessageKey.USERNAME_ALREADY_ONLINE_ERROR);
-        }
-    }
-
-    /**
-     * Exception thrown when a verification has failed and the player should be kicked.
-     */
-    private static final class VerificationFailedException extends Exception {
-        private final MessageKey reason;
-        private final String[] args;
-
-        public VerificationFailedException(MessageKey reason, String... args) {
-            this.reason = reason;
-            this.args = args;
-        }
-
-        public MessageKey getReason() {
-            return reason;
-        }
-
-        public String[] getArgs() {
-            return args;
-        }
-    }
 }

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

@@ -21,11 +21,11 @@ import static org.junit.Assert.fail;
 public final class ListenerConsistencyTest {
 
     private static final Class<?>[] LISTENERS = { AuthMeBlockListener.class, AuthMeEntityListener.class,
-        AuthMePlayerJoinListener.class, AuthMePlayerListener.class, AuthMePlayerListener16.class,
+        AuthMePlayerListener.class, AuthMePlayerListener16.class, AuthMePlayerListener18.class,
-        AuthMePlayerListener18.class, AuthMeServerListener.class };
+        AuthMeServerListener.class };
 
-    private static final Set<String> CANCELED_EXCEPTIONS = Sets.newHashSet("AuthMePlayerJoinListener#onPlayerJoin",
+    private static final Set<String> CANCELED_EXCEPTIONS = Sets.newHashSet("AuthMePlayerListener#onPlayerJoin",
-        "AuthMePlayerJoinListener#onPreLogin", "AuthMePlayerJoinListener#onPlayerLogin",
+        "AuthMePlayerListener#onPreLogin", "AuthMePlayerListener#onPlayerLogin",
         "AuthMePlayerListener#onPlayerQuit", "AuthMeServerListener#onPluginDisable",
         "AuthMeServerListener#onServerPing", "AuthMeServerListener#onPluginEnable",
         "AuthMePlayerListener#onJoinMessage");

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

@@ -0,0 +1,419 @@
+package fr.xephi.authme.listener;
+
+import fr.xephi.authme.AntiBot;
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.cache.auth.PlayerAuth;
+import fr.xephi.authme.cache.limbo.LimboCache;
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.output.MessageKey;
+import fr.xephi.authme.output.Messages;
+import fr.xephi.authme.permission.PermissionsManager;
+import fr.xephi.authme.permission.PlayerStatePermission;
+import fr.xephi.authme.settings.NewSetting;
+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.ValidationService;
+import org.bukkit.Server;
+import org.bukkit.entity.Player;
+import org.bukkit.event.player.PlayerLoginEvent;
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.TypeSafeMatcher;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+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.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
+
+/**
+ * Test for {@link OnJoinVerifier}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class OnJoinVerifierTest {
+
+    @InjectMocks
+    private OnJoinVerifier onJoinVerifier;
+
+    @Mock
+    private NewSetting settings;
+    @Mock
+    private DataSource dataSource;
+    @Mock
+    private Messages messages;
+    @Mock
+    private PermissionsManager permissionsManager;
+    @Mock
+    private AntiBot antiBot;
+    @Mock
+    private ValidationService validationService;
+    @Mock
+    private BukkitService bukkitService;
+    @Mock
+    private LimboCache limboCache;
+    @Mock
+    private Server server;
+
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
+    @BeforeClass
+    public static void setUpLogger() {
+        TestHelper.setupLogger();
+    }
+
+    @Test
+    public void shouldNotDoAnythingForNormalEvent() {
+        // given
+        PlayerLoginEvent event = mock(PlayerLoginEvent.class);
+        given(event.getResult()).willReturn(PlayerLoginEvent.Result.ALLOWED);
+
+        // when
+        boolean result = onJoinVerifier.refusePlayerForFullServer(event);
+
+        // then
+        assertThat(result, equalTo(false));
+        verify(event).getResult();
+        verifyNoMoreInteractions(event);
+        verifyZeroInteractions(bukkitService);
+        verifyZeroInteractions(dataSource);
+        verifyZeroInteractions(permissionsManager);
+    }
+
+    @Test
+    public void shouldRefuseNonVipPlayerForFullServer() {
+        // given
+        Player player = mock(Player.class);
+        PlayerLoginEvent event = new PlayerLoginEvent(player, "hostname", null);
+        event.setResult(PlayerLoginEvent.Result.KICK_FULL);
+        given(permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)).willReturn(false);
+        String serverFullMessage = "server is full";
+        given(messages.retrieveSingle(MessageKey.KICK_FULL_SERVER)).willReturn(serverFullMessage);
+
+        // when
+        boolean result = onJoinVerifier.refusePlayerForFullServer(event);
+
+        // then
+        assertThat(result, equalTo(true));
+        assertThat(event.getResult(), equalTo(PlayerLoginEvent.Result.KICK_FULL));
+        assertThat(event.getKickMessage(), equalTo(serverFullMessage));
+        verifyZeroInteractions(bukkitService);
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldKickNonVipForJoiningVipPlayer() {
+        // given
+        Player player = mock(Player.class);
+        PlayerLoginEvent event = new PlayerLoginEvent(player, "hostname", null);
+        event.setResult(PlayerLoginEvent.Result.KICK_FULL);
+        given(permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)).willReturn(true);
+        List<Player> onlinePlayers = Arrays.asList(mock(Player.class), mock(Player.class));
+        given(permissionsManager.hasPermission(onlinePlayers.get(0), PlayerStatePermission.IS_VIP)).willReturn(true);
+        given(permissionsManager.hasPermission(onlinePlayers.get(1), PlayerStatePermission.IS_VIP)).willReturn(false);
+        returnOnlineListFromBukkitServer(onlinePlayers);
+        given(server.getMaxPlayers()).willReturn(onlinePlayers.size());
+        given(messages.retrieveSingle(MessageKey.KICK_FOR_VIP)).willReturn("kick for vip");
+
+        // when
+        boolean result = onJoinVerifier.refusePlayerForFullServer(event);
+
+        // then
+        assertThat(result, equalTo(false));
+        assertThat(event.getResult(), equalTo(PlayerLoginEvent.Result.ALLOWED));
+        // First player is VIP, so expect no interactions there and second player to have been kicked
+        verifyZeroInteractions(onlinePlayers.get(0));
+        verify(onlinePlayers.get(1)).kickPlayer("kick for vip");
+    }
+
+    @Test
+    public void shouldKickVipPlayerIfNoPlayerCanBeKicked() {
+        // given
+        Player player = mock(Player.class);
+        PlayerLoginEvent event = new PlayerLoginEvent(player, "hostname", null);
+        event.setResult(PlayerLoginEvent.Result.KICK_FULL);
+        given(permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)).willReturn(true);
+        List<Player> onlinePlayers = Collections.singletonList(mock(Player.class));
+        given(permissionsManager.hasPermission(onlinePlayers.get(0), PlayerStatePermission.IS_VIP)).willReturn(true);
+        returnOnlineListFromBukkitServer(onlinePlayers);
+        given(server.getMaxPlayers()).willReturn(onlinePlayers.size());
+        given(messages.retrieveSingle(MessageKey.KICK_FULL_SERVER)).willReturn("kick full server");
+
+        // when
+        boolean result = onJoinVerifier.refusePlayerForFullServer(event);
+
+        // then
+        assertThat(result, equalTo(true));
+        assertThat(event.getResult(), equalTo(PlayerLoginEvent.Result.KICK_FULL));
+        assertThat(event.getKickMessage(), equalTo("kick full server"));
+        verifyZeroInteractions(onlinePlayers.get(0));
+    }
+
+    @Test
+    public void shouldKickNonRegistered() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)).willReturn(true);
+
+        // expect
+        expectValidationExceptionWith(MessageKey.MUST_REGISTER_MESSAGE);
+
+        // when
+        onJoinVerifier.checkKickNonRegistered(false);
+    }
+
+    @Test
+    public void shouldNotKickRegisteredPlayer() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)).willReturn(true);
+
+        // when
+        onJoinVerifier.checkKickNonRegistered(true);
+    }
+
+    @Test
+    public void shouldNotKickUnregisteredPlayer() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)).willReturn(false);
+
+        // when
+        onJoinVerifier.checkKickNonRegistered(false);
+    }
+
+    @Test
+    public void shouldAllowValidName() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)).willReturn(4);
+        given(settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)).willReturn(8);
+        given(settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS)).willReturn("[a-zA-Z0-9]+");
+        onJoinVerifier.reload(); // @PostConstruct method
+
+        // when
+        onJoinVerifier.checkIsValidName("Bobby5");
+    }
+
+    @Test
+    public void shouldRejectTooLongName() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)).willReturn(4);
+        given(settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)).willReturn(8);
+        given(settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS)).willReturn("[a-zA-Z0-9]+");
+        onJoinVerifier.reload(); // @PostConstruct method
+
+        // expect
+        expectValidationExceptionWith(MessageKey.INVALID_NAME_LENGTH);
+
+        // when
+        onJoinVerifier.checkIsValidName("longerthaneight");
+    }
+
+    @Test
+    public void shouldRejectTooShortName() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)).willReturn(4);
+        given(settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)).willReturn(8);
+        given(settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS)).willReturn("[a-zA-Z0-9]+");
+        onJoinVerifier.reload(); // @PostConstruct method
+
+        // expect
+        expectValidationExceptionWith(MessageKey.INVALID_NAME_LENGTH);
+
+        // when
+        onJoinVerifier.checkIsValidName("abc");
+    }
+
+    @Test
+    public void shouldRejectNameWithInvalidCharacters() throws FailedVerificationException {
+        // given
+        given(settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)).willReturn(4);
+        given(settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH)).willReturn(8);
+        given(settings.getProperty(RestrictionSettings.ALLOWED_NICKNAME_CHARACTERS)).willReturn("[a-zA-Z0-9]+");
+        onJoinVerifier.reload(); // @PostConstruct method
+
+        // expect
+        expectValidationExceptionWith(MessageKey.INVALID_NAME_CHARACTERS, "[a-zA-Z0-9]+");
+
+        // when
+        onJoinVerifier.checkIsValidName("Tester!");
+    }
+
+    @Test
+    public void shouldAllowProperlyCasedName() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Bobby");
+        PlayerAuth auth = PlayerAuth.builder().name("bobby").realName("Bobby").build();
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(true);
+
+        // when
+        onJoinVerifier.checkNameCasing(player, auth);
+
+        // then
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldRejectNameWithWrongCasing() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Tester");
+        PlayerAuth auth = PlayerAuth.builder().name("tester").realName("testeR").build();
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(true);
+
+        // expect
+        expectValidationExceptionWith(MessageKey.INVALID_NAME_CASE, "testeR", "Tester");
+
+        // when / then
+        onJoinVerifier.checkNameCasing(player, auth);
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldUpdateMissingRealName() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Authme");
+        PlayerAuth auth = PlayerAuth.builder().name("authme").realName("").build();
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(true);
+
+        // when
+        onJoinVerifier.checkNameCasing(player, auth);
+
+        // then
+        verify(dataSource).updateRealName("authme", "Authme");
+    }
+
+    @Test
+    public void shouldUpdateDefaultRealName() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("SOMEONE");
+        PlayerAuth auth = PlayerAuth.builder().name("someone").realName("Player").build();
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(true);
+
+        // when
+        onJoinVerifier.checkNameCasing(player, auth);
+
+        // then
+        verify(dataSource).updateRealName("someone", "SOMEONE");
+    }
+
+    @Test
+    public void shouldAcceptCasingMismatchForDisabledSetting() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Test");
+        PlayerAuth auth = PlayerAuth.builder().name("test").realName("TEST").build();
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(false);
+
+        // when
+        onJoinVerifier.checkNameCasing(player, auth);
+
+        // then
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldAcceptNameForUnregisteredAccount() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("MyPlayer");
+        PlayerAuth auth = null;
+        given(settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE)).willReturn(true);
+
+        // when
+        onJoinVerifier.checkNameCasing(player, auth);
+
+        // then
+        verifyZeroInteractions(dataSource);
+    }
+
+    @Test
+    public void shouldAcceptNameThatIsNotOnline() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("bobby");
+        given(settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)).willReturn(true);
+        given(bukkitService.getPlayerExact("bobby")).willReturn(null);
+
+        // when
+        onJoinVerifier.checkSingleSession(player);
+
+        // then
+        verifyZeroInteractions(limboCache);
+    }
+
+    @Test
+    public void shouldRejectNameAlreadyOnline() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Charlie");
+        Player onlinePlayer = newPlayerWithName("charlie");
+        given(bukkitService.getPlayerExact("Charlie")).willReturn(onlinePlayer);
+        given(settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)).willReturn(true);
+
+        // expect
+        expectValidationExceptionWith(MessageKey.USERNAME_ALREADY_ONLINE_ERROR);
+
+        // when / then
+        onJoinVerifier.checkSingleSession(player);
+        verify(limboCache).getLimboPlayer("charlie");
+    }
+
+    @Test
+    public void shouldAcceptAlreadyOnlineNameForDisabledSetting() throws FailedVerificationException {
+        // given
+        Player player = newPlayerWithName("Felipe");
+        given(settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)).willReturn(false);
+
+        // when
+        onJoinVerifier.checkSingleSession(player);
+
+        // then
+        verifyZeroInteractions(bukkitService);
+        verifyZeroInteractions(limboCache);
+    }
+
+    private static Player newPlayerWithName(String name) {
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+        return player;
+    }
+
+    @SuppressWarnings("unchecked")
+    private void returnOnlineListFromBukkitServer(Collection<Player> onlineList) {
+        // Note ljacqu 20160529: The compiler gets lost in generics because Collection<? extends Player> is returned
+        // from getOnlinePlayers(). We need to uncheck onlineList to a simple Collection or it will refuse to compile.
+        given(bukkitService.getOnlinePlayers()).willReturn((Collection) onlineList);
+    }
+
+    private void expectValidationExceptionWith(MessageKey messageKey, String... args) {
+        //expectedException.expect(FailedVerificationException.class);
+        expectedException.expect(exceptionWithData(messageKey, args));
+    }
+
+    private static Matcher<FailedVerificationException> exceptionWithData(final MessageKey messageKey,
+                                                                          final String... args) {
+        return new TypeSafeMatcher<FailedVerificationException>() {
+            @Override
+            protected boolean matchesSafely(FailedVerificationException item) {
+                return messageKey.equals(item.getReason()) && Arrays.equals(args, item.getArgs());
+            }
+
+            @Override
+            public void describeTo(Description description) {
+                description.appendValue("VerificationFailedException: reason=" + messageKey + ";args="
+                    + (args == null ? "null" : StringUtils.join(", ", args)));
+            }
+        };
+    }
+
+}