From 39d8a88142af971a7eecdc7019d1c2895dd8eefe Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sun, 29 May 2016 11:45:12 +0200 Subject: [PATCH] Structure checks done in PlayerJoinListener as individual methods --- src/main/java/fr/xephi/authme/AntiBot.java | 11 +- .../listener/AuthMePlayerJoinListener.java | 312 +++++++++++------- .../authme/listener/AuthMePlayerListener.java | 17 +- .../java/fr/xephi/authme/AntiBotTest.java | 4 +- 4 files changed, 216 insertions(+), 128 deletions(-) diff --git a/src/main/java/fr/xephi/authme/AntiBot.java b/src/main/java/fr/xephi/authme/AntiBot.java index 071842bc4..d85cc7843 100644 --- a/src/main/java/fr/xephi/authme/AntiBot.java +++ b/src/main/java/fr/xephi/authme/AntiBot.java @@ -29,8 +29,8 @@ public class AntiBot { private AntiBotStatus antiBotStatus = AntiBotStatus.DISABLED; @Inject - public AntiBot(NewSetting settings, Messages messages, PermissionsManager permissionsManager, - BukkitService bukkitService) { + AntiBot(NewSetting settings, Messages messages, PermissionsManager permissionsManager, + BukkitService bukkitService) { this.settings = settings; this.messages = messages; this.permissionsManager = permissionsManager; @@ -86,7 +86,12 @@ public class AntiBot { }, duration * TICKS_PER_MINUTE); } - public void checkAntiBot(final Player player) { + /** + * Handles a player joining the server and checks if AntiBot needs to be activated. + * + * @param player the player who joined the server + */ + public void handlePlayerJoin(final Player player) { if (antiBotStatus == AntiBotStatus.ACTIVE || antiBotStatus == AntiBotStatus.DISABLED) { return; } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerJoinListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerJoinListener.java index c83d77a50..5343f2146 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerJoinListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerJoinListener.java @@ -22,6 +22,7 @@ import fr.xephi.authme.settings.properties.ProtectionSettings; 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.entity.Player; @@ -60,79 +61,42 @@ public class AuthMePlayerJoinListener implements Listener, Reloadable { private ValidationService validationService; @Inject private AuthMe plugin; + @Inject + private LimboCache limboCache; private Pattern nicknamePattern; @EventHandler(priority = EventPriority.LOW) public void onPlayerJoin(PlayerJoinEvent event) { final Player player = event.getPlayer(); - if (player == null) { - return; + 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); + } + }); } - - // 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) { - PlayerAuth auth = dataSource.getAuth(event.getName()); - if (auth == null && antiBot.getAntiBotStatus() == AntiBot.AntiBotStatus.ACTIVE) { - event.setKickMessage(m.retrieveSingle(MessageKey.KICK_ANTIBOT)); - event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER); - antiBot.antibotKicked.addIfAbsent(event.getName()); - return; - } - if (auth == null && settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED)) { - event.setKickMessage(m.retrieveSingle(MessageKey.MUST_REGISTER_MESSAGE)); - event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER); - return; - } final String name = event.getName().toLowerCase(); - if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH) || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) { - event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_LENGTH)); - event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER); - return; - } - if (settings.getProperty(RegistrationSettings.PREVENT_OTHER_CASE) && auth != null && auth.getRealName() != null) { - String realName = auth.getRealName(); - if (!realName.isEmpty() && !"Player".equals(realName) && !realName.equals(event.getName())) { - event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER); - event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CASE, realName, event.getName())); - return; - } - if (realName.isEmpty() || "Player".equals(realName)) { - dataSource.updateRealName(event.getName().toLowerCase(), event.getName()); - } - } + final boolean isAuthAvailable = dataSource.isAuthAvailable(event.getName()); - if (auth == null && settings.getProperty(ProtectionSettings.ENABLE_PROTECTION)) { - String playerIp = event.getAddress().getHostAddress(); - if (!validationService.isCountryAdmitted(playerIp)) { - event.setLoginResult(AsyncPlayerPreLoginEvent.Result.KICK_OTHER); - event.setKickMessage(m.retrieveSingle(MessageKey.COUNTRY_BANNED_ERROR)); - return; - } - } - - final Player player = bukkitService.getPlayerExact(name); - // Check if forceSingleSession is set to true, so kick player that has - // joined with same nick of online player - if (player != null && settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) { + 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); - event.setKickMessage(m.retrieveSingle(MessageKey.USERNAME_ALREADY_ONLINE_ERROR)); - LimboPlayer limbo = LimboCache.getInstance().getLimboPlayer(name); - if (limbo != null && PlayerCache.getInstance().isAuthenticated(name)) { - Utils.addNormal(player, limbo.getGroup()); - LimboCache.getInstance().deleteLimboPlayer(name); - } } } @@ -141,65 +105,30 @@ public class AuthMePlayerJoinListener implements Listener, Reloadable { final Player player = event.getPlayer(); if (player == null || Utils.isUnrestricted(player)) { return; - } - - if (event.getResult() == PlayerLoginEvent.Result.KICK_FULL) { - if (permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) { - int playersOnline = bukkitService.getOnlinePlayers().size(); - if (playersOnline > plugin.getServer().getMaxPlayers()) { - event.allow(); - } else { - Player pl = generateKickPlayer(bukkitService.getOnlinePlayers()); - if (pl != null) { - pl.kickPlayer(m.retrieveSingle(MessageKey.KICK_FOR_VIP)); - event.allow(); - } else { - ConsoleLogger.info("The player " + event.getPlayer().getName() + " tried to join, but the server was full"); - event.setKickMessage(m.retrieveSingle(MessageKey.KICK_FULL_SERVER)); - event.setResult(PlayerLoginEvent.Result.KICK_FULL); - } - } - } else { - event.setKickMessage(m.retrieveSingle(MessageKey.KICK_FULL_SERVER)); - event.setResult(PlayerLoginEvent.Result.KICK_FULL); - return; - } - } - - if (event.getResult() != PlayerLoginEvent.Result.ALLOWED) { + } else if (refusePlayerForFullServer(event)) { + return; + } else if (event.getResult() != PlayerLoginEvent.Result.ALLOWED) { return; } final String name = player.getName().toLowerCase(); - boolean isAuthAvailable = dataSource.isAuthAvailable(name); + final PlayerAuth auth = dataSource.getAuth(player.getName()); + final boolean isAuthAvailable = auth != null; - if (antiBot.getAntiBotStatus() == AntiBot.AntiBotStatus.ACTIVE && !isAuthAvailable) { - event.setKickMessage(m.retrieveSingle(MessageKey.KICK_ANTIBOT)); - event.setResult(PlayerLoginEvent.Result.KICK_OTHER); - antiBot.antibotKicked.addIfAbsent(player.getName()); - return; - } - - if (settings.getProperty(RestrictionSettings.KICK_NON_REGISTERED) && !isAuthAvailable) { - event.setKickMessage(m.retrieveSingle(MessageKey.MUST_REGISTER_MESSAGE)); + 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; } - if (name.length() > settings.getProperty(RestrictionSettings.MAX_NICKNAME_LENGTH) || name.length() < settings.getProperty(RestrictionSettings.MIN_NICKNAME_LENGTH)) { - event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_LENGTH)); - event.setResult(PlayerLoginEvent.Result.KICK_OTHER); - return; - } - - if (name.equalsIgnoreCase("Player") || !nicknamePattern.matcher(player.getName()).matches()) { - event.setKickMessage(m.retrieveSingle(MessageKey.INVALID_NAME_CHARACTERS) - .replace("REG_EX", nicknamePattern.pattern())); - event.setResult(PlayerLoginEvent.Result.KICK_OTHER); - return; - } - - antiBot.checkAntiBot(player); + antiBot.handlePlayerJoin(player); if (settings.getProperty(HooksSettings.BUNGEECORD)) { ByteArrayDataOutput out = ByteStreams.newDataOutput(); @@ -221,13 +150,174 @@ public class AuthMePlayerJoinListener implements Listener, Reloadable { } } - // Select the player to kick when a vip player joins the server when full - private Player generateKickPlayer(Collection collection) { - for (Player player : collection) { + /** + * Selects a non-VIP player to kick when a VIP player joins the server when full. + * + * @param onlinePlayers list of online players + * @return the player to kick, or null if none applicable + */ + private Player generateKickPlayer(Collection onlinePlayers) { + for (Player player : onlinePlayers) { if (!permissionsManager.hasPermission(player, PlayerStatePermission.IS_VIP)) { return player; } } 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 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; + } + } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index a928b9154..ae3b49c41 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -1,12 +1,11 @@ package fr.xephi.authme.listener; 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; import fr.xephi.authme.output.MessageKey; import fr.xephi.authme.output.Messages; -import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.process.Management; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.SpawnLoader; @@ -15,8 +14,6 @@ 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.Utils; -import fr.xephi.authme.util.ValidationService; -import org.bukkit.Bukkit; import org.bukkit.Location; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; @@ -59,8 +56,6 @@ public class AuthMePlayerListener implements Listener { public static final ConcurrentHashMap joinMessage = new ConcurrentHashMap<>(); - @Inject - private AuthMe plugin; @Inject private NewSetting settings; @Inject @@ -76,9 +71,7 @@ public class AuthMePlayerListener implements Listener { @Inject private SpawnLoader spawnLoader; @Inject - private ValidationService validationService; - @Inject - private PermissionsManager permissionsManager; + private PluginHooks pluginHooks; private void sendLoginOrRegisterMessage(final Player player) { bukkitService.runTaskAsynchronously(new Runnable() { @@ -249,7 +242,7 @@ public class AuthMePlayerListener implements Listener { } if (!antiBot.antibotKicked.contains(player.getName())) { - plugin.getManagement().performQuit(player, true); + management.performQuit(player, true); } } @@ -287,7 +280,7 @@ public class AuthMePlayerListener implements Listener { * @note little hack cause InventoryOpenEvent cannot be cancelled for * real, cause no packet is send to server by client for the main inv */ - Bukkit.getScheduler().scheduleSyncDelayedTask(plugin, new Runnable() { + bukkitService.scheduleSyncDelayedTask(new Runnable() { @Override public void run() { player.closeInventory(); @@ -307,7 +300,7 @@ public class AuthMePlayerListener implements Listener { if (Utils.checkAuth(player)) { return; } - if (plugin.getPluginHooks().isNpc(player)) { + if (pluginHooks.isNpc(player)) { return; } event.setCancelled(true); diff --git a/src/test/java/fr/xephi/authme/AntiBotTest.java b/src/test/java/fr/xephi/authme/AntiBotTest.java index 9950963ab..ddf418314 100644 --- a/src/test/java/fr/xephi/authme/AntiBotTest.java +++ b/src/test/java/fr/xephi/authme/AntiBotTest.java @@ -164,7 +164,7 @@ public class AntiBotTest { AntiBot antiBot = createListeningAntiBot(); // when - antiBot.checkAntiBot(player); + antiBot.handlePlayerJoin(player); // then @SuppressWarnings("unchecked") @@ -194,7 +194,7 @@ public class AntiBotTest { AntiBot antiBot = createListeningAntiBot(); // when - antiBot.checkAntiBot(player); + antiBot.handlePlayerJoin(player); // then @SuppressWarnings("rawtypes")