#762 Use timestamps for session + #816 Logout should invalidate a player's session

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<String, BukkitTask> sessions = new ConcurrentHashMap<>();
+    private static final int MINUTE_IN_MILLIS = 60_000;
+    // Player -> expiration of session in milliseconds
+    private final Map<String, Long> 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) {
+    public void addSession(String name) {
-        if (!enabled || sessionTimeout == 0) {
+        if (enabled) {
-            return;
+            long timeout = System.currentTimeMillis() + timeoutInMinutes * MINUTE_IN_MILLIS;
-        }
+            sessions.put(name.toLowerCase(), timeout);
-
-        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();
         }
     }
 
@@ -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);
+        timeoutInMinutes = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT);
-        this.sessionTimeout = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT);
+        enabled = timeoutInMinutes > 0 && settings.getProperty(PluginSettings.SESSIONS_ENABLED);
     }
 }

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);
         }
     }
 

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);
             }
         });
     }

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))) {
+            if (sessionManager.hasSession(name) || database.isLogged(name)) {
-                sessionManager.cancelSession(name);
-
                 PlayerAuth auth = database.getAuth(name);
                 database.setUnlogged(name);
                 playerCache.removePlayer(name);

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);
         }

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);
-    }
 }

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<String, Long> sessions = (Map<String, Long>) 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);