#1035 Migrate other accounts config from config.yml to commands.yml

docs/config.md

@@ -1,5 +1,5 @@
 <!-- AUTO-GENERATED FILE! Do not edit this directly -->
-<!-- File auto-generated on Wed Dec 13 23:12:29 CET 2017. See docs/config/config.tpl.md -->
+<!-- File auto-generated on Sun Jan 21 18:49:44 CET 2018. See docs/config/config.tpl.md -->
 
 ## AuthMe Configuration
 The first time you run AuthMe it will create a config.yml file in the plugins/AuthMe folder, 
@@ -214,11 +214,6 @@ settings:
         # characters, which is what we recommend. See also http://asciitable.com
         # You can test your regex with https://regex101.com
         allowedPasswordCharacters: '[!-~]*'
-        # Threshold of the other accounts command, a value less than 2 means disabled.
-        otherAccountsCmdThreshold: 0
-        # Command to run when a user has more accounts than the configured threshold.
-        # Available variables: %playername%, %playerip%
-        otherAccountsCmd: 'say The player %playername% with ip %playerip% has multiple accounts!'
     GameMode:
         # Force survival gamemode when player joins?
         ForceSurvivalMode: false
@@ -444,6 +439,8 @@ Security:
         captchaLength: 5
         # Minutes after which login attempts count is reset for a player
         captchaCountReset: 60
+        # Require captcha before a player may register?
+        requireForRegistration: false
     tempban:
         # Tempban a user's IP address if they enter the wrong password too many times
         enableTempban: false
@@ -558,4 +555,4 @@ To change settings on a running server, save your changes to config.yml and use
 
 ---
 
-This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Wed Dec 13 23:12:29 CET 2017
+This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sun Jan 21 18:49:44 CET 2018

src/main/java/fr/xephi/authme/process/login/ProcessSyncPlayerLogin.java

@@ -15,8 +15,6 @@ import fr.xephi.authme.service.bungeecord.BungeeSender;
 import fr.xephi.authme.settings.WelcomeMessageConfiguration;
 import fr.xephi.authme.settings.commandconfig.CommandManager;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
-import fr.xephi.authme.settings.properties.RestrictionSettings;
-import fr.xephi.authme.util.PlayerUtils;
 import org.bukkit.entity.Player;
 import org.bukkit.potion.PotionEffectType;
 
@@ -99,9 +97,6 @@ public class ProcessSyncPlayerLogin implements SynchronousProcess {
         bukkitService.callEvent(new LoginEvent(player));
         player.saveData();
 
-        // Run command if player has other accounts
-        runCommandOtherAccounts(authsWithSameIp, player);
-
         // Login is done, display welcome message
         welcomeMessageConfiguration.sendWelcomeMessage(player);
 
@@ -114,16 +109,4 @@ public class ProcessSyncPlayerLogin implements SynchronousProcess {
         // Send Bungee stuff. The service will check if it is enabled or not.
         bungeeSender.connectPlayerOnLogin(player);
     }
-
-    private void runCommandOtherAccounts(List<String> auths, Player player) {
-        int threshold = commonService.getProperty(RestrictionSettings.OTHER_ACCOUNTS_CMD_THRESHOLD);
-        String command = commonService.getProperty(RestrictionSettings.OTHER_ACCOUNTS_CMD);
-
-        if (threshold >= 2 && !command.isEmpty() && auths.size() >= threshold) {
-            bukkitService.dispatchConsoleCommand(command
-                .replace("%playername%", player.getName())
-                .replace("%playerip%", PlayerUtils.getPlayerIp(player))
-            );
-        }
-    }
 }

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

@@ -13,6 +13,7 @@ import fr.xephi.authme.security.HashAlgorithm;
 import fr.xephi.authme.settings.properties.PluginSettings;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.SecuritySettings;
+import fr.xephi.authme.util.StringUtils;
 
 import javax.inject.Inject;
 import java.io.File;
@@ -37,6 +38,13 @@ public class SettingsMigrationService extends PlainMigrationService {
 
     private final File pluginFolder;
 
+    // Stores old "other accounts command" config if present.
+    // We need to store it in here for retrieval when we build the CommandConfig. Retrieving it from the config.yml is
+    // not possible since this migration service may trigger the config.yml to be resaved. As the old command settings
+    // don't exist in the code anymore, as soon as config.yml is resaved we lose this information.
+    private String oldOtherAccountsCommand;
+    private int oldOtherAccountsCommandThreshold;
+
     @Inject
     SettingsMigrationService(@DataFolder File pluginFolder) {
         this.pluginFolder = pluginFolder;
@@ -51,6 +59,8 @@ public class SettingsMigrationService extends PlainMigrationService {
             changes = true;
         }
 
+        setOldOtherAccountsCommandFieldsIfSet(resource);
+
         // Note ljacqu 20160211: Concatenating migration methods with | instead of the usual ||
         // ensures that all migrations will be performed
         return changes
@@ -75,7 +85,8 @@ public class SettingsMigrationService extends PlainMigrationService {
             "Hooks.customAttributes", "Security.stop.kickPlayersBeforeStopping",
             "settings.restrictions.keepCollisionsDisabled", "settings.forceCommands", "settings.forceCommandsAsConsole",
             "settings.forceRegisterCommands", "settings.forceRegisterCommandsAsConsole",
-            "settings.sessions.sessionExpireOnIpChange"};
+            "settings.sessions.sessionExpireOnIpChange", "settings.restrictions.otherAccountsCmd",
+            "settings.restrictions.otherAccountsCmdThreshold"};
         for (String deprecatedPath : deprecatedProperties) {
             if (resource.contains(deprecatedPath)) {
                 return true;
@@ -84,6 +95,20 @@ public class SettingsMigrationService extends PlainMigrationService {
         return false;
     }
 
+    // --------
+    // Old other accounts
+    // --------
+    public boolean hasOldOtherAccountsCommand() {
+        return !StringUtils.isEmpty(oldOtherAccountsCommand);
+    }
+
+    public String getOldOtherAccountsCommand() {
+        return oldOtherAccountsCommand;
+    }
+
+    public int getOldOtherAccountsCommandThreshold() {
+        return oldOtherAccountsCommandThreshold;
+    }
 
     // --------
     // Specific migrations
@@ -288,6 +313,22 @@ public class SettingsMigrationService extends PlainMigrationService {
         return false;
     }
 
+    /**
+     * Retrieves the old config to run a command when alt accounts are detected and sets them to this instance
+     * for further processing.
+     *
+     * @param resource The property resource
+     */
+    private void setOldOtherAccountsCommandFieldsIfSet(PropertyResource resource) {
+        Property<String> commandProperty = newProperty("settings.restrictions.otherAccountsCmd", "");
+        Property<Integer> commandThresholdProperty = newProperty("settings.restrictions.otherAccountsCmdThreshold", 0);
+
+        if (commandProperty.isPresent(resource) && commandThresholdProperty.getValue(resource) >= 2) {
+            oldOtherAccountsCommand = commandProperty.getValue(resource);
+            oldOtherAccountsCommandThreshold = commandThresholdProperty.getValue(resource);
+        }
+    }
+
     /**
      * Checks for an old property path and moves it to a new path if it is present and the new path is not yet set.
      *

src/main/java/fr/xephi/authme/settings/commandconfig/CommandMigrationService.java

@@ -5,8 +5,13 @@ import ch.jalu.configme.properties.Property;
 import ch.jalu.configme.resource.PropertyResource;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import fr.xephi.authme.settings.SettingsMigrationService;
+import fr.xephi.authme.util.RandomStringUtils;
 
+import javax.inject.Inject;
 import java.util.List;
+import java.util.Map;
+import java.util.Optional;
 
 /**
  * Migrates the commands from their old location, in config.yml, to the dedicated commands configuration file.
@@ -18,19 +23,42 @@ class CommandMigrationService implements MigrationService {
     static final List<String> COMMAND_CONFIG_PROPERTIES = ImmutableList.of(
         "onJoin", "onLogin", "onSessionLogin", "onFirstLogin", "onRegister", "onUnregister", "onLogout");
 
+    @Inject
+    private SettingsMigrationService settingsMigrationService;
+
     CommandMigrationService() {
     }
 
     @Override
     public boolean checkAndMigrate(PropertyResource resource, List<Property<?>> properties) {
         final CommandConfig commandConfig = CommandSettingsHolder.COMMANDS.getValue(resource);
-        if (isFileEmpty(resource)) {
+        if (moveOtherAccountsConfig(commandConfig) || isFileEmpty(resource)) {
             resource.setValue("", commandConfig);
             return true;
         }
         return false;
     }
 
+    private boolean moveOtherAccountsConfig(CommandConfig commandConfig) {
+        if (settingsMigrationService.hasOldOtherAccountsCommand()) {
+            OnLoginCommand command = new OnLoginCommand(
+                replaceOldPlaceholdersWithNew(settingsMigrationService.getOldOtherAccountsCommand()), Executor.CONSOLE);
+            command.setIfNumberOfAccountsAtLeast(
+                Optional.of(settingsMigrationService.getOldOtherAccountsCommandThreshold()));
+
+            Map<String, OnLoginCommand> onLoginCommands = commandConfig.getOnLogin();
+            onLoginCommands.put(RandomStringUtils.generate(10), command);
+            return true;
+        }
+        return false;
+    }
+
+    private static String replaceOldPlaceholdersWithNew(String oldOtherAccountsCommand) {
+        return oldOtherAccountsCommand
+            .replace("%playername%", "%p")
+            .replace("%playerip%", "%ip");
+    }
+
     private static boolean isFileEmpty(PropertyResource resource) {
         return COMMAND_CONFIG_PROPERTIES.stream().anyMatch(property -> resource.getObject(property) == null);
     }

src/main/java/fr/xephi/authme/settings/commandconfig/OnLoginCommand.java

@@ -7,8 +7,8 @@ import java.util.Optional;
  */
 public class OnLoginCommand extends Command {
 
-    private Optional<Integer> ifNumberOfAccountsAtLeast;
+    private Optional<Integer> ifNumberOfAccountsAtLeast = Optional.empty();
-    private Optional<Integer> ifNumberOfAccountsLessThan;
+    private Optional<Integer> ifNumberOfAccountsLessThan = Optional.empty();
 
     /**
      * Default constructor (for bean mapping).

src/main/java/fr/xephi/authme/settings/properties/RestrictionSettings.java

@@ -180,18 +180,6 @@ public final class RestrictionSettings implements SettingsHolder {
     public static final Property<List<String>> UNRESTRICTED_NAMES =
         newLowercaseListProperty("settings.unrestrictions.UnrestrictedName");
 
-    @Comment("Threshold of the other accounts command, a value less than 2 means disabled.")
-    public static final Property<Integer> OTHER_ACCOUNTS_CMD_THRESHOLD =
-        newProperty("settings.restrictions.otherAccountsCmdThreshold", 0);
-
-    @Comment({
-        "Command to run when a user has more accounts than the configured threshold.",
-        "Available variables: %playername%, %playerip%"
-    })
-    public static final Property<String> OTHER_ACCOUNTS_CMD =
-        newProperty("settings.restrictions.otherAccountsCmd",
-            "say The player %playername% with ip %playerip% has multiple accounts!");
-
     private RestrictionSettings() {
     }
 

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

@@ -97,6 +97,24 @@ public class SettingsMigrationServiceTest {
         assertThat(migrationService.returnedValues, contains(true, false));
     }
 
+    @Test
+    public void shouldKeepOldOtherAccountsSettings() throws IOException {
+        // given
+        File dataFolder = temporaryFolder.newFolder();
+        File configFile = new File(dataFolder, "config.yml");
+        Files.copy(getJarFile(OLD_CONFIG_FILE), configFile);
+        PropertyResource resource = new YamlFileResource(configFile);
+        SettingsMigrationService migrationService = new SettingsMigrationService(dataFolder);
+
+        // when
+        migrationService.performMigrations(resource, AuthMeSettingsRetriever.buildConfigurationData().getProperties());
+
+        // then
+        assertThat(migrationService.hasOldOtherAccountsCommand(), equalTo(true));
+        assertThat(migrationService.getOldOtherAccountsCommand(), equalTo("msg admin %playername% has a lot of accounts!"));
+        assertThat(migrationService.getOldOtherAccountsCommandThreshold(), equalTo(5));
+    }
+
     private void verifyHasUpToDateSettings(Settings settings, File dataFolder) throws IOException {
         assertThat(settings.getProperty(ALLOWED_NICKNAME_CHARACTERS), equalTo(ALLOWED_NICKNAME_CHARACTERS.getDefaultValue()));
         assertThat(settings.getProperty(DELAY_JOIN_MESSAGE), equalTo(true));

src/test/java/fr/xephi/authme/settings/commandconfig/CommandMigrationServiceTest.java

@@ -6,17 +6,26 @@ import ch.jalu.configme.configurationdata.ConfigurationDataBuilder;
 import ch.jalu.configme.resource.PropertyResource;
 import ch.jalu.configme.resource.YamlFileResource;
 import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.settings.SettingsMigrationService;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.io.File;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
+import static com.google.common.collect.Sets.newHashSet;
+import static org.hamcrest.Matchers.aMapWithSize;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
 
 /**
  * Test for {@link CommandMigrationService}.
@@ -27,6 +36,9 @@ public class CommandMigrationServiceTest {
     @InjectMocks
     private CommandMigrationService commandMigrationService;
 
+    @Mock
+    private SettingsMigrationService settingsMigrationService;
+
     @BeforeClass
     public static void setUpLogger() {
         TestHelper.setupLogger();
@@ -90,4 +102,43 @@ public class CommandMigrationServiceTest {
         // when / then
         assertThat(CommandMigrationService.COMMAND_CONFIG_PROPERTIES, containsInAnyOrder(properties));
     }
+
+    @Test
+    public void shouldMigrateOldOtherAccountsCommand() {
+        // given
+        given(settingsMigrationService.hasOldOtherAccountsCommand()).willReturn(true);
+        given(settingsMigrationService.getOldOtherAccountsCommand())
+            .willReturn("helpop %playername% (%playerip%) has other accounts!");
+        given(settingsMigrationService.getOldOtherAccountsCommandThreshold()).willReturn(3);
+        File commandFile = TestHelper.getJarFile(TestHelper.PROJECT_ROOT + "settings/commandconfig/commands.complete.yml");
+        PropertyResource resource = new YamlFileResource(commandFile);
+
+        // when
+        commandMigrationService.checkAndMigrate(
+            resource, ConfigurationDataBuilder.collectData(CommandSettingsHolder.class).getProperties());
+
+        // then
+        Map<String, OnLoginCommand> onLoginCommands = CommandSettingsHolder.COMMANDS.getValue(resource).getOnLogin();
+        assertThat(onLoginCommands, aMapWithSize(6)); // 5 in the file + the newly migrated on
+        OnLoginCommand newCommand = getUnknownOnLoginCommand(onLoginCommands);
+        assertThat(newCommand.getCommand(), equalTo("helpop %p (%ip) has other accounts!"));
+        assertThat(newCommand.getExecutor(), equalTo(Executor.CONSOLE));
+        assertThat(newCommand.getIfNumberOfAccountsAtLeast().get(), equalTo(3));
+        assertThat(newCommand.getIfNumberOfAccountsLessThan().isPresent(), equalTo(false));
+    }
+
+    /*
+     * Returns the command under onLogin from commands.complete.yml that isn't present in the beginning.
+     */
+    private static OnLoginCommand getUnknownOnLoginCommand(Map<String, OnLoginCommand> onLoginCommands) {
+        Set<String> knownKeys = newHashSet("welcome", "show_motd", "display_list", "warn_for_alts", "log_suspicious_user");
+        List<String> unknownKeys = onLoginCommands.keySet().stream()
+            .filter(key -> !knownKeys.contains(key))
+            .collect(Collectors.toList());
+        if (unknownKeys.size() == 1) {
+            return onLoginCommands.get(unknownKeys.get(0));
+        } else {
+            throw new IllegalStateException("Expected 1 unknown key but found " + unknownKeys.size() + ": " + unknownKeys);
+        }
+    }
 }

src/test/java/fr/xephi/authme/settings/commandconfig/CommandYmlConsistencyTest.java

@@ -4,17 +4,20 @@ import ch.jalu.configme.configurationdata.ConfigurationDataBuilder;
 import ch.jalu.configme.resource.PropertyResource;
 import ch.jalu.configme.resource.YamlFileResource;
 import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.settings.SettingsMigrationService;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
+import org.mockito.Mock;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import java.io.File;
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
+import static org.mockito.Mockito.verify;
 
 /**
  * Tests that commands.yml is well-formed.
@@ -25,6 +28,9 @@ public class CommandYmlConsistencyTest {
     @InjectMocks
     private CommandMigrationService commandMigrationService;
 
+    @Mock
+    private SettingsMigrationService settingsMigrationService;
+
     @Rule
     public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
@@ -40,5 +46,6 @@ public class CommandYmlConsistencyTest {
 
         // then
         assertThat(result, equalTo(false));
+        verify(settingsMigrationService).hasOldOtherAccountsCommand();
     }
 }

src/test/resources/fr/xephi/authme/settings/config-old.yml

@@ -154,6 +154,11 @@ settings:
         noTeleport: false
         # Regex sintax for allowed Chars in passwords.
         allowedPasswordCharacters: '[\x21-\x7E]*'
+        # Command to run when a user has more accounts than the configured threshold.
+        # Available variables: %playername%, %playerip%
+        otherAccountsCmd: 'msg admin %playername% has a lot of accounts!'
+        # Threshold of the other accounts command, a value less than 2 means disabled.
+        otherAccountsCmdThreshold: 5
     GameMode:
         # ForceSurvivalMode to player when join ?
         ForceSurvivalMode: false