Fix session system (#1337)

* Old session system

* Actually check if sessions are enabled

* Cleanup

* Don't use cached isLogged value

src/main/java/fr/xephi/authme/data/SessionManager.java

@@ -1,79 +0,0 @@
-package fr.xephi.authme.data;
-
-import fr.xephi.authme.ConsoleLogger;
-import fr.xephi.authme.initialization.HasCleanup;
-import fr.xephi.authme.initialization.SettingsDependent;
-import fr.xephi.authme.settings.Settings;
-import fr.xephi.authme.settings.properties.PluginSettings;
-import fr.xephi.authme.util.expiring.ExpiringSet;
-
-import javax.inject.Inject;
-import java.util.concurrent.TimeUnit;
-
-/**
- * Manages sessions, allowing players to be automatically logged in if they join again
- * within a configurable amount of time.
- */
-public class SessionManager implements SettingsDependent, HasCleanup {
-
-    private final ExpiringSet<String> sessions;
-    private boolean enabled;
-
-    @Inject
-    SessionManager(Settings settings) {
-        long timeout = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT);
-        sessions = new ExpiringSet<>(timeout, TimeUnit.MINUTES);
-        enabled = timeout > 0 && settings.getProperty(PluginSettings.SESSIONS_ENABLED);
-    }
-
-    /**
-     * 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.contains(name.toLowerCase());
-    }
-
-    /**
-     * Add a player session to the cache.
-     *
-     * @param name The name of the player.
-     */
-    public void addSession(String name) {
-        if (enabled) {
-            sessions.add(name.toLowerCase());
-        }
-    }
-
-    /**
-     * Remove a player's session from the cache.
-     *
-     * @param name The name of the player.
-     */
-    public void removeSession(String name) {
-        sessions.remove(name.toLowerCase());
-    }
-
-    @Override
-    public void reload(Settings settings) {
-        long timeoutInMinutes = settings.getProperty(PluginSettings.SESSIONS_TIMEOUT);
-        sessions.setExpiration(timeoutInMinutes, TimeUnit.MINUTES);
-        boolean oldEnabled = enabled;
-        enabled = timeoutInMinutes > 0 && settings.getProperty(PluginSettings.SESSIONS_ENABLED);
-
-        // With this reload, the sessions feature has just been disabled, so clear all stored sessions
-        if (oldEnabled && !enabled) {
-            sessions.clear();
-            ConsoleLogger.fine("Sessions disabled: cleared all sessions");
-        }
-    }
-
-    @Override
-    public void performCleanup() {
-        if (enabled) {
-            sessions.removeExpiredEntries();
-        }
-    }
-}

src/main/java/fr/xephi/authme/datasource/CacheDataSource.java

@@ -195,7 +195,7 @@ public class CacheDataSource implements DataSource {
 
     @Override
     public boolean isLogged(String user) {
-        return playerCache.isAuthenticated(user);
+        return source.isLogged(user);
     }
 
     @Override

src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java

@@ -1,7 +1,6 @@
 package fr.xephi.authme.process.join;
 
 import fr.xephi.authme.ConsoleLogger;
-import fr.xephi.authme.data.SessionManager;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.data.limbo.LimboService;
@@ -18,6 +17,7 @@ import fr.xephi.authme.service.PluginHookService;
 import fr.xephi.authme.service.ValidationService;
 import fr.xephi.authme.settings.commandconfig.CommandManager;
 import fr.xephi.authme.settings.properties.HooksSettings;
+import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.util.PlayerUtils;
@@ -53,9 +53,6 @@ public class AsynchronousJoin implements AsynchronousProcess {
     @Inject
     private LimboService limboService;
 
-    @Inject
-    private SessionManager sessionManager;
-
     @Inject
     private PluginHookService pluginHookService;
 
@@ -174,17 +171,22 @@ public class AsynchronousJoin implements AsynchronousProcess {
 
     private boolean canResumeSession(Player player) {
         final String name = player.getName();
-        if (sessionManager.hasSession(name) || database.isLogged(name)) {
-            PlayerAuth auth = database.getAuth(name);
+        if (database.isLogged(name)) {
             database.setUnlogged(name);
             playerCache.removePlayer(name);
-            if (auth != null) {
-                if (auth.getIp().equals(PlayerUtils.getPlayerIp(player))) {
-                    RestoreSessionEvent event = bukkitService.createAndCallEvent(
-                        isAsync -> new RestoreSessionEvent(player, isAsync));
-                    return !event.isCancelled();
-                } else {
-                    service.send(player, MessageKey.SESSION_EXPIRED);
+            if(service.getProperty(PluginSettings.SESSIONS_ENABLED)) {
+                PlayerAuth auth = database.getAuth(name);
+                if (auth != null) {
+                    long timeSinceLastLogin = System.currentTimeMillis() - auth.getLastLogin();
+                    if(timeSinceLastLogin < 0
+                        || timeSinceLastLogin > (service.getProperty(PluginSettings.SESSIONS_TIMEOUT) * 60 * 60 * 1000)
+                        || !auth.getIp().equals(PlayerUtils.getPlayerIp(player))) {
+                        service.send(player, MessageKey.SESSION_EXPIRED);
+                    } else {
+                        RestoreSessionEvent event = bukkitService.createAndCallEvent(
+                            isAsync -> new RestoreSessionEvent(player, isAsync));
+                        return !event.isCancelled();
+                    }
                 }
             }
         }

src/main/java/fr/xephi/authme/process/logout/ProcessSyncPlayerLogout.java

@@ -1,7 +1,7 @@
 package fr.xephi.authme.process.logout;
 
 import fr.xephi.authme.ConsoleLogger;
-import fr.xephi.authme.data.SessionManager;
+//import fr.xephi.authme.data.SessionManager;
 import fr.xephi.authme.data.limbo.LimboService;
 import fr.xephi.authme.events.LogoutEvent;
 import fr.xephi.authme.listener.protocollib.ProtocolLibService;
@@ -36,9 +36,6 @@ public class ProcessSyncPlayerLogout implements SynchronousProcess {
     @Inject
     private LimboService limboService;
 
-    @Inject
-    private SessionManager sessionManager;
-
     @Inject
     private TeleportationService teleportationService;
 
@@ -54,9 +51,6 @@ public class ProcessSyncPlayerLogout implements SynchronousProcess {
      * @param player the player logging out
      */
     public void processSyncLogout(Player player) {
-        final String name = player.getName().toLowerCase();
-
-        sessionManager.removeSession(name);
         if (service.getProperty(RestrictionSettings.PROTECT_INVENTORY_BEFORE_LOGIN)) {
             protocolLibService.sendBlankInventoryPacket(player);
         }

src/main/java/fr/xephi/authme/process/quit/AsynchronousQuit.java

@@ -1,7 +1,6 @@
 package fr.xephi.authme.process.quit;
 
 import fr.xephi.authme.AuthMe;
-import fr.xephi.authme.data.SessionManager;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.CacheDataSource;
@@ -10,6 +9,7 @@ import fr.xephi.authme.process.AsynchronousProcess;
 import fr.xephi.authme.service.CommonService;
 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.PlayerUtils;
 import fr.xephi.authme.service.ValidationService;
@@ -38,9 +38,6 @@ public class AsynchronousQuit implements AsynchronousProcess {
     @Inject
     private SyncProcessManager syncProcessManager;
 
-    @Inject
-    private SessionManager sessionManager;
-
     @Inject
     private SpawnLoader spawnLoader;
 
@@ -79,19 +76,20 @@ 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);
 
-        //always update the database when the player quit the game
-        database.setUnlogged(name);
+        //always update the database when the player quit the game (if sessions are disabled)
+        if(!wasLoggedIn || !service.getProperty(PluginSettings.SESSIONS_ENABLED)) {
+            database.setUnlogged(name);
+        }
 
         if (plugin.isEnabled()) {
             syncProcessManager.processSyncPlayerQuit(player, wasLoggedIn);
         }
+
         // remove player from cache
         if (database instanceof CacheDataSource) {
             ((CacheDataSource) database).getCachedAuths().invalidate(name);

src/test/java/fr/xephi/authme/data/SessionManagerTest.java

@@ -1,156 +0,0 @@
-package fr.xephi.authme.data;
-
-import fr.xephi.authme.ReflectionTestUtils;
-import fr.xephi.authme.settings.Settings;
-import fr.xephi.authme.settings.properties.PluginSettings;
-import fr.xephi.authme.util.expiring.ExpiringSet;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.junit.MockitoJUnitRunner;
-
-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.never;
-import static org.mockito.Mockito.verify;
-
-/**
- * Test for {@link SessionManager}.
- */
-@RunWith(MockitoJUnitRunner.class)
-public class SessionManagerTest {
-
-    @Test
-    public void shouldHaveSession() {
-        // given
-        Settings settings = mockSettings(true, 10);
-        SessionManager manager = new SessionManager(settings);
-        String player = "playah";
-
-        // when
-        manager.addSession(player);
-
-        // then
-        assertThat(manager.hasSession(player), equalTo(true));
-    }
-
-    @Test
-    public void shouldNotHaveSession() {
-        // given
-        Settings settings = mockSettings(true, 10);
-        SessionManager manager = new SessionManager(settings);
-        String player = "playah";
-
-        // when/then
-        assertThat(manager.hasSession(player), equalTo(false));
-    }
-
-    @Test
-    public void shouldNotAddSessionBecauseDisabled() {
-        // given
-        Settings settings = mockSettings(false, 10);
-        SessionManager manager = new SessionManager(settings);
-        String player = "playah";
-
-        // when
-        manager.addSession(player);
-
-        // then
-        assertThat(manager.hasSession(player), equalTo(false));
-    }
-
-    @Test
-    public void shouldNotAddSessionBecauseTimeoutIsZero() {
-        // given
-        Settings settings = mockSettings(true, 0);
-        SessionManager manager = new SessionManager(settings);
-        String player = "playah";
-
-        // when
-        manager.addSession(player);
-
-        // then
-        assertThat(manager.hasSession(player), equalTo(false));
-    }
-
-    @Test
-    public void shouldRemoveSession() {
-        // given
-        Settings 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
-    public void shouldClearAllSessionsAfterDisable() {
-        // given
-        Settings settings = mockSettings(true, 10);
-        SessionManager manager = new SessionManager(settings);
-        manager.addSession("player01");
-        manager.addSession("player02");
-
-        // when
-        manager.reload(mockSettings(false, 20));
-
-        // then
-        assertThat(getSessionsMap(manager).isEmpty(), equalTo(true));
-    }
-
-    @Test
-    public void shouldPerformCleanup() {
-        // given
-        Settings settings = mockSettings(true, 1);
-        SessionManager manager = new SessionManager(settings);
-        ExpiringSet<String> expiringSet = mockExpiringSet();
-        setSessionsMap(manager, expiringSet);
-
-        // when
-        manager.performCleanup();
-
-        // then
-        verify(expiringSet).removeExpiredEntries();
-    }
-
-    @Test
-    public void shouldNotPerformCleanup() {
-        // given
-        Settings settings = mockSettings(false, 1);
-        SessionManager manager = new SessionManager(settings);
-        ExpiringSet<String> expiringSet = mockExpiringSet();
-        setSessionsMap(manager, expiringSet);
-
-        // when
-        manager.performCleanup();
-
-        // then
-        verify(expiringSet, never()).removeExpiredEntries();
-    }
-
-    private static ExpiringSet<String> getSessionsMap(SessionManager manager) {
-        return ReflectionTestUtils.getFieldValue(SessionManager.class, manager, "sessions");
-    }
-
-    private static void setSessionsMap(SessionManager manager, ExpiringSet<String> sessionsMap) {
-        ReflectionTestUtils.setField(SessionManager.class, manager, "sessions", sessionsMap);
-    }
-
-    @SuppressWarnings("unchecked")
-    private static <T> ExpiringSet<T> mockExpiringSet() {
-        return mock(ExpiringSet.class);
-    }
-
-    private static Settings mockSettings(boolean isEnabled, int sessionTimeout) {
-        Settings settings = mock(Settings.class);
-        given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(isEnabled);
-        given(settings.getProperty(PluginSettings.SESSIONS_TIMEOUT)).willReturn(sessionTimeout);
-        return settings;
-    }
-}