From 4a1ff63d3e1150deb46ddd37574e4a664fc5b802 Mon Sep 17 00:00:00 2001 From: were491 <34976611+were491@users.noreply.github.com> Date: Sun, 6 Feb 2022 13:24:19 -0600 Subject: [PATCH] Fix all the issues Read the last release post --- .../xyz/nikitacartes/easyauth/EasyAuth.java | 10 +++++++ .../easyauth/commands/LoginCommand.java | 29 ++++--------------- .../easyauth/event/AuthEventHandler.java | 12 ++++---- .../easyauth/storage/PlayerCache.java | 17 ++++++++++- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/src/main/java/xyz/nikitacartes/easyauth/EasyAuth.java b/src/main/java/xyz/nikitacartes/easyauth/EasyAuth.java index f5cbd1b8..b356737a 100644 --- a/src/main/java/xyz/nikitacartes/easyauth/EasyAuth.java +++ b/src/main/java/xyz/nikitacartes/easyauth/EasyAuth.java @@ -21,9 +21,11 @@ import java.util.Properties; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import static xyz.nikitacartes.easyauth.utils.EasyLogger.logInfo; +import static xyz.nikitacartes.easyauth.EasyAuth.RESET_LOGIN_THREAD; import static xyz.nikitacartes.easyauth.utils.EasyLogger.logError; public class EasyAuth implements ModInitializer { @@ -34,6 +36,11 @@ public class EasyAuth implements ModInitializer { public static final ExecutorService THREADPOOL = Executors.newCachedThreadPool(); + /** + * Used to reset login attempts with a delay and (in the future) other small tasks. + */ + public static final ScheduledThreadPoolExecutor RESET_LOGIN_THREAD = new ScheduledThreadPoolExecutor(1); + /** * HashMap of players that have joined the server. * It's cleared on server stop in order to save some interactions with database during runtime. @@ -77,6 +84,8 @@ public static void init(Path gameDir) { throw new RuntimeException("[EasyAuth] Error creating directory!"); // Loading config config = AuthConfig.load(new File(gameDirectory + "/mods/EasyAuth/config.json")); + // Allow the thread to shutdown properly when server is closed by canceling every scheduled task. + RESET_LOGIN_THREAD.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); // Connecting to db DB.openConnection(); } @@ -98,6 +107,7 @@ public static void stop() { logError(e.getMessage()); THREADPOOL.shutdownNow(); } + RESET_LOGIN_THREAD.shutdown(); // Closing DB connection DB.close(); diff --git a/src/main/java/xyz/nikitacartes/easyauth/commands/LoginCommand.java b/src/main/java/xyz/nikitacartes/easyauth/commands/LoginCommand.java index 04a38dfb..8ee42d80 100644 --- a/src/main/java/xyz/nikitacartes/easyauth/commands/LoginCommand.java +++ b/src/main/java/xyz/nikitacartes/easyauth/commands/LoginCommand.java @@ -17,15 +17,10 @@ import static net.minecraft.server.command.CommandManager.literal; import static xyz.nikitacartes.easyauth.EasyAuth.*; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; public class LoginCommand { - // To reset the login attempts... - public static final ScheduledExecutorService RESET_LOGIN_THREAD = Executors.newScheduledThreadPool(1); - public static void registerCommand(CommandDispatcher dispatcher) { LiteralCommandNode node = registerLogin(dispatcher); // Registering the "/login" command if (config.experimental.enableAliases) { @@ -57,10 +52,6 @@ private static int login(ServerCommandSource source, String pass) throws Command return 0; } - // ++ the login tries. Maybe it's more threadsafe here than in the thread pool? - // But if not, you could save the calls to set this back to 0. - playerCacheMap.get(uuid).loginTries++; - // Putting rest of the command in different thread to avoid lag spikes THREADPOOL.submit(() -> { int maxLoginTries = config.main.maxLoginTries; @@ -69,38 +60,30 @@ private static int login(ServerCommandSource source, String pass) throws Command if (passwordResult == AuthHelper.PasswordOptions.CORRECT) { player.sendMessage(TranslationHelper.getSuccessfullyAuthenticated(), false); ((PlayerAuth) player).setAuthenticated(true); - - // Reset their login tries - playerCacheMap.get(uuid).loginTries = 0; - + playerCacheMap.get(uuid).resetLoginTries(); // player.getServer().getPlayerManager().sendToAll(new PlayerListS2CPacket(PlayerListS2CPacket.Action.ADD_PLAYER, player)); return; } else if (passwordResult == AuthHelper.PasswordOptions.NOT_REGISTERED) { player.sendMessage(TranslationHelper.getRegisterRequired(), false); - - // Reset their login tries - playerCacheMap.get(uuid).loginTries = 0; - return; } // Kicking the player out - else if (maxLoginTries == 1) { - // Reset their login tries - playerCacheMap.get(uuid).loginTries = 0; - + else if (maxLoginTries == 1) { player.networkHandler.disconnect(TranslationHelper.getWrongPassword()); return; - } else if(playerCacheMap.get(uuid).loginTries == maxLoginTries) { + } else if (playerCacheMap.get(uuid).getLoginTries() >= maxLoginTries - 1) { player.networkHandler.disconnect(TranslationHelper.getLoginTriesExceeded()); // Reset their login try counter after the amount of seconds specified in the config. RESET_LOGIN_THREAD.schedule(() -> { - playerCacheMap.get(uuid).loginTries = 0; + playerCacheMap.get(uuid).resetLoginTries(); }, config.experimental.resetLoginAttemptsTime, TimeUnit.SECONDS); return; } // Sending wrong pass message player.sendMessage(TranslationHelper.getWrongPassword(), false); + // Increment (failed) login tries. Hopefully this is more thread-safe. + playerCacheMap.get(uuid).incrementLoginTries(); }); return 0; } diff --git a/src/main/java/xyz/nikitacartes/easyauth/event/AuthEventHandler.java b/src/main/java/xyz/nikitacartes/easyauth/event/AuthEventHandler.java index 0d0975b7..6e579652 100644 --- a/src/main/java/xyz/nikitacartes/easyauth/event/AuthEventHandler.java +++ b/src/main/java/xyz/nikitacartes/easyauth/event/AuthEventHandler.java @@ -48,7 +48,7 @@ public static LiteralText checkCanPlayerJoinServer(GameProfile profile, PlayerMa Pattern pattern = Pattern.compile(regex); Matcher matcher = pattern.matcher(incomingPlayerUsername); - + if ((onlinePlayer != null && !((PlayerAuth) onlinePlayer).canSkipAuth()) && config.experimental.preventAnotherLocationKick) { // Player needs to be kicked, since there's already a player with that name // playing on the server @@ -97,16 +97,14 @@ public static void onPlayerJoin(ServerPlayerEntity player) { player.setInvisible(false); return; } - - // If the player has too many login attempts, kick the player immediately. - if (playerCache.loginTries > config.main.maxLoginTries && config.main.maxLoginTries != -1) { + // If the player has too many login attempts, kick them immediately. + // TODO: Move this to checkCanPlayerJoinServer? + if (playerCache.getLoginTries() >= config.main.maxLoginTries - 1 && config.main.maxLoginTries != -1) { player.networkHandler.disconnect(TranslationHelper.getLoginTriesExceeded()); return; } - ((PlayerAuth) player).setAuthenticated(false); - - + // Tries to rescue player from nether portal if (config.main.tryPortalRescue && player.getBlockStateAtPos().getBlock().equals(Blocks.NETHER_PORTAL)) { BlockPos pos = player.getBlockPos(); diff --git a/src/main/java/xyz/nikitacartes/easyauth/storage/PlayerCache.java b/src/main/java/xyz/nikitacartes/easyauth/storage/PlayerCache.java index c9efec2a..777d4af5 100644 --- a/src/main/java/xyz/nikitacartes/easyauth/storage/PlayerCache.java +++ b/src/main/java/xyz/nikitacartes/easyauth/storage/PlayerCache.java @@ -35,8 +35,9 @@ public class PlayerCache { public String password = ""; /** * Stores how many times player has tried to log in. + * This should only be a max of maxLoginTries - 1. */ - public int loginTries = 0; + private int loginTries = 0; /** * Last recorded IP of player. * Used for {@link AuthEventHandler#onPlayerJoin(ServerPlayerEntity) sessions}. @@ -100,6 +101,9 @@ public static PlayerCache fromJson(ServerPlayerEntity player, String fakeUuid) { // playerCache.wasInPortal = player.getBlockStateAtPos().getBlock().equals(Blocks.NETHER_PORTAL); playerCache.wasInPortal = false; + + // This only happens on the first login after server reset. Reset login attempts just to be safe. + playerCache.resetLoginTries(); } return playerCache; @@ -113,4 +117,15 @@ public static boolean isAuthenticated(String uuid) { PlayerCache playerCache = playerCacheMap.get(uuid); return (playerCache != null && playerCache.isAuthenticated); } + + // Hide the actual login tries modifications behind synchronized functions for thread safety. + public synchronized void incrementLoginTries() { + loginTries++; + } + public synchronized void resetLoginTries() { + loginTries = 0; + } + public synchronized int getLoginTries() { + return loginTries; + } }