#1190 Show settings warnings on reload also (#1384)

- Extract setting checks into their own class, called on startup and reload

src/main/java/fr/xephi/authme/AuthMe.java

@@ -23,15 +23,12 @@ import fr.xephi.authme.listener.PlayerListener18;
 import fr.xephi.authme.listener.PlayerListener19;
 import fr.xephi.authme.listener.PlayerListener19Spigot;
 import fr.xephi.authme.listener.ServerListener;
-import fr.xephi.authme.security.HashAlgorithm;
-import fr.xephi.authme.security.crypts.Argon2;
 import fr.xephi.authme.security.crypts.Sha256;
 import fr.xephi.authme.service.BackupService;
 import fr.xephi.authme.service.BukkitService;
 import fr.xephi.authme.service.MigrationService;
 import fr.xephi.authme.settings.Settings;
-import fr.xephi.authme.settings.properties.EmailSettings;
+import fr.xephi.authme.settings.SettingsWarner;
-import fr.xephi.authme.settings.properties.RestrictionSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
 import fr.xephi.authme.task.CleanupTask;
 import fr.xephi.authme.task.purge.PurgeService;
@@ -143,7 +140,7 @@ public class AuthMe extends JavaPlugin {
         }
 
         // Show settings warnings
-        showSettingsWarnings();
+        injector.getSingleton(SettingsWarner.class).logWarningsForMisconfigurations();
 
         // Do a backup on start
         backupService.doBackup(BackupService.BackupCause.START);
@@ -255,29 +252,6 @@ public class AuthMe extends JavaPlugin {
         injector.getSingleton(NewAPI.class);
     }
 
-    /**
-     * Show the settings warnings, for various risky settings.
-     */
-    private void showSettingsWarnings() {
-        // Force single session disabled
-        if (!settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) {
-            ConsoleLogger.warning("WARNING!!! By disabling ForceSingleSession, your server protection is inadequate!");
-        }
-
-        // Use TLS property only affects port 25
-        if (!settings.getProperty(EmailSettings.PORT25_USE_TLS)
-            && settings.getProperty(EmailSettings.SMTP_PORT) != 25) {
-            ConsoleLogger.warning("Note: You have set Email.useTls to false but this only affects mail over port 25");
-        }
-        // Check if argon2 library is present and can be loaded
-        if (settings.getProperty(SecuritySettings.PASSWORD_HASH).equals(HashAlgorithm.ARGON2)
-            && !Argon2.isLibraryLoaded()) {
-            ConsoleLogger.warning("WARNING!!! You use Argon2 Hash Algorithm method but we can't find the Argon2 "
-                + "library on your system! See https://github.com/AuthMe/AuthMeReloaded/wiki/Argon2-as-Password-Hash");
-            stopOrUnload();
-        }
-    }
-
     /**
      * Registers all event listeners.
      *

src/main/java/fr/xephi/authme/command/executable/authme/ReloadCommand.java

@@ -10,6 +10,7 @@ import fr.xephi.authme.initialization.factory.SingletonStore;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.SettingsWarner;
 import fr.xephi.authme.settings.properties.DatabaseSettings;
 import fr.xephi.authme.util.Utils;
 import org.bukkit.command.CommandSender;
@@ -34,6 +35,9 @@ public class ReloadCommand implements ExecutableCommand {
     @Inject
     private CommonService commonService;
 
+    @Inject
+    private SettingsWarner settingsWarner;
+
     @Inject
     private SingletonStore<Reloadable> reloadableStore;
 
@@ -45,6 +49,8 @@ public class ReloadCommand implements ExecutableCommand {
         try {
             settings.reload();
             ConsoleLogger.setLoggingOptions(settings);
+            settingsWarner.logWarningsForMisconfigurations();
+
             // We do not change database type for consistency issues, but we'll output a note in the logs
             if (!settings.getProperty(DatabaseSettings.BACKEND).equals(dataSource.getType())) {
                 Utils.logAndSendMessage(sender, "Note: cannot change database type during /authme reload");

src/main/java/fr/xephi/authme/settings/SettingsWarner.java

@@ -0,0 +1,61 @@
+package fr.xephi.authme.settings;
+
+import fr.xephi.authme.AuthMe;
+import fr.xephi.authme.ConsoleLogger;
+import fr.xephi.authme.security.HashAlgorithm;
+import fr.xephi.authme.security.crypts.Argon2;
+import fr.xephi.authme.settings.properties.EmailSettings;
+import fr.xephi.authme.settings.properties.PluginSettings;
+import fr.xephi.authme.settings.properties.RestrictionSettings;
+import fr.xephi.authme.settings.properties.SecuritySettings;
+
+import javax.inject.Inject;
+
+/**
+ * Logs warning messages in cases where the configured values suggest a misconfiguration.
+ * <p>
+ * Note that this class does not modify any settings and it is called after the settings have been fully loaded.
+ * For actual migrations (= verifications which trigger changes and a resave of the settings),
+ * see {@link SettingsMigrationService}.
+ */
+public class SettingsWarner {
+
+    @Inject
+    private Settings settings;
+
+    @Inject
+    private AuthMe authMe;
+
+    SettingsWarner() {
+    }
+
+    /**
+     * Logs warning when necessary to notify the user about misconfigurations.
+     */
+    public void logWarningsForMisconfigurations() {
+        // Force single session disabled
+        if (!settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)) {
+            ConsoleLogger.warning("WARNING!!! By disabling ForceSingleSession, your server protection is inadequate!");
+        }
+
+        // Use TLS property only affects port 25
+        if (!settings.getProperty(EmailSettings.PORT25_USE_TLS)
+            && settings.getProperty(EmailSettings.SMTP_PORT) != 25) {
+            ConsoleLogger.warning("Note: You have set Email.useTls to false but this only affects mail over port 25");
+        }
+
+        // Output hint if sessions are enabled that the timeout must be positive
+        if (settings.getProperty(PluginSettings.SESSIONS_ENABLED)
+            && settings.getProperty(PluginSettings.SESSIONS_TIMEOUT) <= 0) {
+            ConsoleLogger.warning("Warning: Session timeout needs to be positive in order to work!");
+        }
+
+        // Check if argon2 library is present and can be loaded
+        if (settings.getProperty(SecuritySettings.PASSWORD_HASH).equals(HashAlgorithm.ARGON2)
+            && !Argon2.isLibraryLoaded()) {
+            ConsoleLogger.warning("WARNING!!! You use Argon2 Hash Algorithm method but we can't find the Argon2 "
+                + "library on your system! See https://github.com/AuthMe/AuthMeReloaded/wiki/Argon2-as-Password-Hash");
+            authMe.stopOrUnload();
+        }
+    }
+}

src/test/java/fr/xephi/authme/command/executable/authme/ReloadCommandTest.java

@@ -11,6 +11,7 @@ import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.output.LogLevel;
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.settings.Settings;
+import fr.xephi.authme.settings.SettingsWarner;
 import fr.xephi.authme.settings.properties.DatabaseSettings;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
@@ -55,6 +56,9 @@ public class ReloadCommandTest {
     @Mock
     private CommonService commandService;
 
+    @Mock
+    private SettingsWarner settingsWarner;
+
     @Mock
     private SingletonStore<Reloadable> reloadableStore;
 
@@ -93,6 +97,7 @@ public class ReloadCommandTest {
         verify(settings).reload();
         verifyReloadingCalls(reloadables, dependents);
         verify(commandService).send(sender, MessageKey.CONFIG_RELOAD_SUCCESS);
+        verify(settingsWarner).logWarningsForMisconfigurations();
     }
 
     @Test

src/test/java/fr/xephi/authme/settings/SettingsWarnerTest.java

@@ -0,0 +1,76 @@
+package fr.xephi.authme.settings;
+
+import fr.xephi.authme.AuthMe;
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.security.HashAlgorithm;
+import fr.xephi.authme.settings.properties.EmailSettings;
+import fr.xephi.authme.settings.properties.PluginSettings;
+import fr.xephi.authme.settings.properties.RestrictionSettings;
+import fr.xephi.authme.settings.properties.SecuritySettings;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.logging.Logger;
+
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.BDDMockito.given;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.internal.verification.VerificationModeFactory.times;
+
+/**
+ * Test for {@link SettingsWarner}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class SettingsWarnerTest {
+
+    @Mock
+    private Settings settings;
+
+    @Mock
+    private AuthMe authMe;
+
+    @Test
+    public void shouldLogWarnings() {
+        // given
+        Logger logger = TestHelper.setupLogger();
+        given(settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)).willReturn(false);
+        given(settings.getProperty(EmailSettings.SMTP_PORT)).willReturn(44);
+        given(settings.getProperty(EmailSettings.PORT25_USE_TLS)).willReturn(false);
+        given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(true);
+        given(settings.getProperty(PluginSettings.SESSIONS_TIMEOUT)).willReturn(-5);
+        given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT);
+
+        // when
+        createSettingsWarner().logWarningsForMisconfigurations();
+
+        // then
+        verify(logger, times(3)).warning(anyString());
+    }
+
+    @Test
+    public void shouldNotLogAnyWarning() {
+        Logger logger = TestHelper.setupLogger();
+        given(settings.getProperty(RestrictionSettings.FORCE_SINGLE_SESSION)).willReturn(true);
+        given(settings.getProperty(EmailSettings.SMTP_PORT)).willReturn(25);
+        given(settings.getProperty(EmailSettings.PORT25_USE_TLS)).willReturn(false);
+        given(settings.getProperty(PluginSettings.SESSIONS_ENABLED)).willReturn(false);
+        given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5);
+
+        // when
+        createSettingsWarner().logWarningsForMisconfigurations();
+
+        // then
+        verifyZeroInteractions(logger);
+    }
+
+    private SettingsWarner createSettingsWarner() {
+        SettingsWarner warner = new SettingsWarner();
+        ReflectionTestUtils.setField(warner, "settings", settings);
+        ReflectionTestUtils.setField(warner, "authMe", authMe);
+        return warner;
+    }
+}