From 3b06524796e14009750e70170b061386695d0be8 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 15 Jul 2016 19:35:35 +0200 Subject: [PATCH] #762 Use timestamps for session + #816 Logout should invalidate a player's session --- .../fr/xephi/authme/cache/SessionManager.java | 51 +++++++-------- .../authme/listener/AuthMePlayerListener.java | 4 +- .../fr/xephi/authme/process/Management.java | 4 +- .../authme/process/join/AsynchronousJoin.java | 4 +- .../ProcessSynchronousPlayerLogout.java | 5 +- .../authme/process/quit/AsynchronousQuit.java | 30 ++------- .../authme/cache/SessionManagerTest.java | 63 ++++++++++++------- 7 files changed, 77 insertions(+), 84 deletions(-) diff --git a/src/main/java/fr/xephi/authme/cache/SessionManager.java b/src/main/java/fr/xephi/authme/cache/SessionManager.java index 73434efb5..698c51bb7 100644 --- a/src/main/java/fr/xephi/authme/cache/SessionManager.java +++ b/src/main/java/fr/xephi/authme/cache/SessionManager.java @@ -3,17 +3,23 @@ package fr.xephi.authme.cache; import fr.xephi.authme.initialization.SettingsDependent; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PluginSettings; -import org.bukkit.scheduler.BukkitTask; import javax.inject.Inject; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +/** + * Manages sessions, allowing players to be automatically logged in if they join again + * within a configurable amount of time. + */ public class SessionManager implements SettingsDependent { - private final ConcurrentHashMap sessions = new ConcurrentHashMap<>(); + private static final int MINUTE_IN_MILLIS = 60_000; + // Player -> expiration of session in milliseconds + private final Map sessions = new ConcurrentHashMap<>(); private boolean enabled; - private int sessionTimeout; + private int timeoutInMinutes; @Inject SessionManager(NewSetting settings) { @@ -21,39 +27,30 @@ public class SessionManager implements SettingsDependent { } /** - * Check if a session for a player is currently being cached. + * Check if a session is available for the given player. * * @param name The name to check. * @return True if a session is found. */ public boolean hasSession(String name) { - return enabled && sessions.containsKey(name); + if (enabled) { + Long timeout = sessions.get(name.toLowerCase()); + if (timeout != null) { + return System.currentTimeMillis() <= timeout; + } + } + return false; } /** * Add a player session to the cache. * * @param name The name of the player. - * @param task The task to run. */ - public void addSession(String name, BukkitTask task) { - if (!enabled || sessionTimeout == 0) { - return; - } - - this.sessions.put(name, task); - } - - /** - * Cancels a player's session. After the task is cancelled, it will be removed from - * the cache. - * - * @param name The name of the player who's session to cancel. - */ - public void cancelSession(String name) { - BukkitTask task = sessions.remove(name); - if (task != null) { - task.cancel(); + public void addSession(String name) { + if (enabled) { + long timeout = System.currentTimeMillis() + timeoutInMinutes * MINUTE_IN_MILLIS; + sessions.put(name.toLowerCase(), timeout); } } @@ -63,12 +60,12 @@ public class SessionManager implements SettingsDependent { * @param name The name of the player. */ public void removeSession(String name) { - this.sessions.remove(name); + this.sessions.remove(name.toLowerCase()); } @Override public void reload(NewSetting settings) { - this.enabled = settings.getProperty(PluginSettings.SESSIONS_ENABLED); - this.sessionTimeout = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT); + timeoutInMinutes = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT); + enabled = timeoutInMinutes > 0 && settings.getProperty(PluginSettings.SESSIONS_ENABLED); } } diff --git a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java index 8eb947a7f..281ee5b9d 100644 --- a/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java +++ b/src/main/java/fr/xephi/authme/listener/AuthMePlayerListener.java @@ -252,7 +252,7 @@ public class AuthMePlayerListener implements Listener { return; } - management.performQuit(player, false); + management.performQuit(player); } @EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST) @@ -260,7 +260,7 @@ public class AuthMePlayerListener implements Listener { Player player = event.getPlayer(); if (!antiBot.wasPlayerKicked(player.getName())) { - management.performQuit(player, true); + management.performQuit(player); } } diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 5e72fad7c..d229edfb0 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -88,11 +88,11 @@ public class Management { }); } - public void performQuit(final Player player, final boolean isKick) { + public void performQuit(final Player player) { runTask(new Runnable() { @Override public void run() { - asynchronousQuit.processQuit(player, isKick); + asynchronousQuit.processQuit(player); } }); } diff --git a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java index 4ed006801..82c71e109 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -135,9 +135,7 @@ public class AsynchronousJoin implements AsynchronousProcess { } // Session logic - if (service.getProperty(PluginSettings.SESSIONS_ENABLED) && (sessionManager.hasSession(name) || database.isLogged(name))) { - sessionManager.cancelSession(name); - + if (sessionManager.hasSession(name) || database.isLogged(name)) { PlayerAuth auth = database.getAuth(name); database.setUnlogged(name); playerCache.removePlayer(name); diff --git a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java index ae7afe466..bfff2e3ac 100644 --- a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java @@ -51,9 +51,8 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { public void processSyncLogout(Player player) { final String name = player.getName().toLowerCase(); - if (sessionManager.hasSession(name)) { - sessionManager.cancelSession(name); - } + + sessionManager.removeSession(name); if (service.getProperty(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN)) { protocolLibService.sendBlankInventoryPacket(player); } diff --git a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java index a674fe3ed..e9373c649 100644 --- a/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java +++ b/src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java @@ -10,18 +10,14 @@ import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.ProcessService; import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.settings.SpawnLoader; -import fr.xephi.authme.settings.properties.PluginSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.Utils; import org.bukkit.Location; import org.bukkit.entity.Player; -import org.bukkit.scheduler.BukkitTask; import javax.inject.Inject; -import static fr.xephi.authme.util.BukkitService.TICKS_PER_MINUTE; - public class AsynchronousQuit implements AsynchronousProcess { @Inject @@ -52,13 +48,12 @@ public class AsynchronousQuit implements AsynchronousProcess { } - public void processQuit(Player player, boolean isKick) { + public void processQuit(Player player) { if (player == null || Utils.isUnrestricted(player)) { return; } final String name = player.getName().toLowerCase(); - String ip = Utils.getPlayerIp(player); if (playerCache.isAuthenticated(name)) { if (service.getProperty(RestrictionSettings.SAVE_QUIT_LOCATION)) { Location loc = spawnLoader.getPlayerLocationOrSpawn(player); @@ -67,6 +62,8 @@ public class AsynchronousQuit implements AsynchronousProcess { .realName(player.getName()).build(); database.updateQuitLoc(auth); } + + final String ip = Utils.getPlayerIp(player); PlayerAuth auth = PlayerAuth.builder() .name(name) .realName(player.getName()) @@ -74,27 +71,13 @@ public class AsynchronousQuit implements AsynchronousProcess { .lastLogin(System.currentTimeMillis()) .build(); database.updateSession(auth); + + sessionManager.addSession(name); } //always unauthenticate the player - use session only for auto logins on the same ip playerCache.removePlayer(name); - if (plugin.isEnabled() && service.getProperty(PluginSettings.SESSIONS_ENABLED)) { - BukkitTask task = bukkitService.runTaskLaterAsynchronously(new Runnable() { - - @Override - public void run() { - postLogout(name); - } - - }, service.getProperty(PluginSettings.SESSIONS_TIMEOUT) * TICKS_PER_MINUTE); - - sessionManager.addSession(name, task); - } else { - //plugin is disabled; we cannot schedule more tasks so run it directly here - postLogout(name); - } - //always update the database when the player quit the game database.setUnlogged(name); @@ -107,7 +90,4 @@ public class AsynchronousQuit implements AsynchronousProcess { } } - private void postLogout(String name) { - sessionManager.removeSession(name); - } } diff --git a/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java index 6e03b3128..5a9f1d137 100644 --- a/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java +++ b/src/test/java/fr/xephi/authme/cache/SessionManagerTest.java @@ -1,12 +1,14 @@ package fr.xephi.authme.cache; +import fr.xephi.authme.ReflectionTestUtils; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PluginSettings; -import org.bukkit.scheduler.BukkitTask; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.runners.MockitoJUnitRunner; +import java.util.Map; + import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; import static org.mockito.BDDMockito.given; @@ -24,10 +26,9 @@ public class SessionManagerTest { NewSetting settings = mockSettings(true, 10); SessionManager manager = new SessionManager(settings); String player = "playah"; - BukkitTask task = mock(BukkitTask.class); // when - manager.addSession(player, task); + manager.addSession(player); // then assertThat(manager.hasSession(player), equalTo(true)); @@ -44,31 +45,15 @@ public class SessionManagerTest { assertThat(manager.hasSession(player), equalTo(false)); } - @Test - public void shouldAddSession() { - // given - NewSetting settings = mockSettings(true, 10); - SessionManager manager = new SessionManager(settings); - String player = "playah"; - BukkitTask task = mock(BukkitTask.class); - - // when - manager.addSession(player, task); - - // then - assertThat(manager.hasSession(player), equalTo(true)); - } - @Test public void shouldNotAddSessionBecauseDisabled() { // given NewSetting settings = mockSettings(false, 10); SessionManager manager = new SessionManager(settings); String player = "playah"; - BukkitTask task = mock(BukkitTask.class); // when - manager.addSession(player, task); + manager.addSession(player); // then assertThat(manager.hasSession(player), equalTo(false)); @@ -80,15 +65,49 @@ public class SessionManagerTest { NewSetting settings = mockSettings(true, 0); SessionManager manager = new SessionManager(settings); String player = "playah"; - BukkitTask task = mock(BukkitTask.class); // when - manager.addSession(player, task); + manager.addSession(player); // then assertThat(manager.hasSession(player), equalTo(false)); } + @Test + public void shouldRemoveSession() { + // given + NewSetting settings = mockSettings(true, 10); + String player = "user"; + SessionManager manager = new SessionManager(settings); + manager.addSession(player); + + // when + manager.removeSession(player); + + // then + assertThat(manager.hasSession(player), equalTo(false)); + } + + @Test + @SuppressWarnings("unchecked") + public void shouldDenySessionIfTimeoutHasExpired() { + // given + int timeout = 20; + NewSetting settings = mockSettings(true, timeout); + String player = "patrick"; + SessionManager manager = new SessionManager(settings); + Map sessions = (Map) ReflectionTestUtils + .getFieldValue(SessionManager.class, manager, "sessions"); + // Add session entry for player that just has expired + sessions.put(player, System.currentTimeMillis() - 1000); + + // when + boolean result = manager.hasSession(player); + + // then + assertThat(result, equalTo(false)); + } + private static NewSetting mockSettings(boolean isEnabled, int sessionTimeout) { NewSetting settings = mock(NewSetting.class); given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(isEnabled);