diff --git a/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java b/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java index 537c3ebc..050b908a 100644 --- a/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java +++ b/src/main/java/fr/xephi/authme/permission/AuthGroupHandler.java @@ -5,14 +5,11 @@ import fr.xephi.authme.data.limbo.LimboCache; import fr.xephi.authme.data.limbo.LimboPlayer; import fr.xephi.authme.initialization.Reloadable; import fr.xephi.authme.settings.Settings; -import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.PluginSettings; -import fr.xephi.authme.settings.properties.SecuritySettings; import org.bukkit.entity.Player; import javax.annotation.PostConstruct; import javax.inject.Inject; -import java.util.Arrays; /** * Changes the permission group according to the auth status of the player and the configuration. @@ -28,7 +25,6 @@ public class AuthGroupHandler implements Reloadable { @Inject private LimboCache limboCache; - private String unloggedInGroup; private String unregisteredGroup; private String registeredGroup; @@ -36,15 +32,15 @@ public class AuthGroupHandler implements Reloadable { } /** - * Set the group of a player, by its AuthMe group type. + * Sets the group of a player by its authentication status. * - * @param player The player. - * @param group The group type. + * @param player the player + * @param groupType the group type * - * @return True if succeeded, false otherwise. False is also returned if groups aren't supported + * @return True upon success, false otherwise. False is also returned if groups aren't supported * with the current permissions system. */ - public boolean setGroup(Player player, AuthGroupType group) { + public boolean setGroup(Player player, AuthGroupType groupType) { // Check whether the permissions check is enabled if (!settings.getProperty(PluginSettings.ENABLE_PERMISSION_CHECK)) { return false; @@ -56,71 +52,45 @@ public class AuthGroupHandler implements Reloadable { return false; } - switch (group) { + switch (groupType) { case UNREGISTERED: - // Remove the other group type groups, set the current group - permissionsManager.removeGroups(player, Arrays.asList(registeredGroup, unloggedInGroup)); + // Remove the other group, set the current group + permissionsManager.removeGroups(player, registeredGroup); return permissionsManager.addGroup(player, unregisteredGroup); - case REGISTERED: - // Remove the other group type groups, set the current group - permissionsManager.removeGroups(player, Arrays.asList(unregisteredGroup, unloggedInGroup)); + case REGISTERED_UNAUTHENTICATED: + // Remove the other group, set the current group + permissionsManager.removeGroups(player, unregisteredGroup); return permissionsManager.addGroup(player, registeredGroup); - case NOT_LOGGED_IN: - // Remove the other group type groups, set the current group - permissionsManager.removeGroups(player, Arrays.asList(unregisteredGroup, registeredGroup)); - return permissionsManager.addGroup(player, unloggedInGroup); - case LOGGED_IN: - // Get the player data - LimboPlayer data = limboCache.getPlayerData(player.getName().toLowerCase()); - if (data == null) { - return false; - } + return restoreGroup(player); - // Get the players group - String realGroup = data.getGroup(); - - // Remove the other group types groups, set the real group - permissionsManager.removeGroups(player, - Arrays.asList(unregisteredGroup, registeredGroup, unloggedInGroup) - ); - return permissionsManager.addGroup(player, realGroup); default: - return false; + throw new IllegalStateException("Encountered unhandled auth group type '" + groupType + "'"); } } - /** - * TODO: This method requires better explanation. - *

- * Set the normal group of a player. - * - * @param player The player. - * @param group The normal group. - * - * @return True on success, false on failure. - */ - public boolean addNormal(Player player, String group) { - // Check whether the permissions check is enabled - if (!settings.getProperty(PluginSettings.ENABLE_PERMISSION_CHECK)) { + private boolean restoreGroup(Player player) { + // Get the player's LimboPlayer + LimboPlayer limbo = limboCache.getPlayerData(player.getName()); + if (limbo == null) { return false; } - // Remove old groups - permissionsManager.removeGroups(player, Arrays.asList(unregisteredGroup, registeredGroup, unloggedInGroup)); + // Get the players group + String realGroup = limbo.getGroup(); - // Add the normal group, return the result - return permissionsManager.addGroup(player, group); + // Remove the other group types groups, set the real group + permissionsManager.removeGroups(player, unregisteredGroup, registeredGroup); + return permissionsManager.addGroup(player, realGroup); } @Override @PostConstruct public void reload() { - unloggedInGroup = settings.getProperty(SecuritySettings.UNLOGGEDIN_GROUP); - unregisteredGroup = settings.getProperty(HooksSettings.UNREGISTERED_GROUP); - registeredGroup = settings.getProperty(HooksSettings.REGISTERED_GROUP); + unregisteredGroup = settings.getProperty(PluginSettings.UNREGISTERED_GROUP); + registeredGroup = settings.getProperty(PluginSettings.REGISTERED_GROUP); } } diff --git a/src/main/java/fr/xephi/authme/permission/AuthGroupType.java b/src/main/java/fr/xephi/authme/permission/AuthGroupType.java index 9ab2a370..dfedf8ee 100644 --- a/src/main/java/fr/xephi/authme/permission/AuthGroupType.java +++ b/src/main/java/fr/xephi/authme/permission/AuthGroupType.java @@ -8,11 +8,8 @@ public enum AuthGroupType { /** Player does not have an account. */ UNREGISTERED, - /** Registered? */ - REGISTERED, // TODO #761: Remove this or the NOT_LOGGED_IN one - - /** Player is registered and not logged in. */ - NOT_LOGGED_IN, + /** Player is registered but not logged in. */ + REGISTERED_UNAUTHENTICATED, /** Player is logged in. */ LOGGED_IN diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 7ce0b3e5..6af00070 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -289,7 +289,7 @@ public class PermissionsManager implements Reloadable { * @return True if the player is in the specified group, false otherwise. * False is also returned if groups aren't supported by the used permissions system. */ - public boolean inGroup(Player player, String groupName) { + public boolean isInGroup(Player player, String groupName) { // If no permissions system is used, return false if (!isEnabled()) return false; @@ -307,42 +307,12 @@ public class PermissionsManager implements Reloadable { * False is also returned if this feature isn't supported for the current permissions system. */ public boolean addGroup(Player player, String groupName) { - if (StringUtils.isEmpty(groupName)) { + if (!isEnabled() || StringUtils.isEmpty(groupName)) { return false; } - - // If no permissions system is used, return false - if (!isEnabled()) { - return false; - } - return handler.addToGroup(player, groupName); } - /** - * Add the permission groups of a player, if supported. - * - * @param player The player - * @param groupNames The name of the groups to add. - * - * @return True if succeed, false otherwise. - * False is also returned if this feature isn't supported for the current permissions system. - */ - public boolean addGroups(Player player, List groupNames) { - // If no permissions system is used, return false - if (!isEnabled()) - return false; - - // Add each group to the user - boolean result = true; - for (String groupName : groupNames) - if (!addGroup(player, groupName)) - result = false; - - // Return the result - return result; - } - /** * Remove the permission group of a player, if supported. * @@ -352,8 +322,7 @@ public class PermissionsManager implements Reloadable { * @return True if succeed, false otherwise. * False is also returned if this feature isn't supported for the current permissions system. */ - public boolean removeGroup(Player player, String groupName) { - // If no permissions system is used, return false + public boolean removeGroups(Player player, String groupName) { if (!isEnabled()) return false; @@ -369,16 +338,18 @@ public class PermissionsManager implements Reloadable { * @return True if succeed, false otherwise. * False is also returned if this feature isn't supported for the current permissions system. */ - public boolean removeGroups(Player player, List groupNames) { + public boolean removeGroups(Player player, String... groupNames) { // If no permissions system is used, return false if (!isEnabled()) return false; // Add each group to the user boolean result = true; - for (String groupName : groupNames) - if (!removeGroup(player, groupName)) + for (String groupName : groupNames) { + if (!handler.removeFromGroup(player, groupName)) { result = false; + } + } // Return the result return result; @@ -402,41 +373,6 @@ public class PermissionsManager implements Reloadable { return handler.setGroup(player, groupName); } - /** - * Set the permission groups of a player, if supported. - * This clears the current groups of the player. - * - * @param player The player - * @param groupNames The name of the groups to set. - * - * @return True if succeed, false otherwise. - * False is also returned if this feature isn't supported for the current permissions system. - */ - public boolean setGroups(Player player, List groupNames) { - // If no permissions system is used or if there's no group supplied, return false - if (!isEnabled() || groupNames.isEmpty()) - return false; - - // Set the main group - if (!setGroup(player, groupNames.get(0))) - return false; - - // Add the rest of the groups - boolean result = true; - for (int i = 1; i < groupNames.size(); i++) { - // Get the group name - String groupName = groupNames.get(i); - - // Add this group - if (!addGroup(player, groupName)) { - result = false; - } - } - - // Return the result - return result; - } - /** * Remove all groups of the specified player, if supported. * Systems like Essentials GroupManager don't allow all groups to be removed from a player, thus the user will stay @@ -456,6 +392,6 @@ public class PermissionsManager implements Reloadable { List groupNames = getGroups(player); // Remove each group - return removeGroups(player, groupNames); + return removeGroups(player, groupNames.toArray(new String[groupNames.size()])); } } 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 dab9cdc2..0f7e819f 100644 --- a/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java +++ b/src/main/java/fr/xephi/authme/process/join/AsynchronousJoin.java @@ -112,7 +112,7 @@ public class AsynchronousJoin implements AsynchronousProcess { if (isAuthAvailable) { limboCache.addPlayerData(player); - service.setGroup(player, AuthGroupType.NOT_LOGGED_IN); + service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); // Protect inventory if (service.getProperty(PROTECT_INVENTORY_BEFORE_LOGIN)) { 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 17710df4..18229146 100644 --- a/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java +++ b/src/main/java/fr/xephi/authme/process/logout/ProcessSynchronousPlayerLogout.java @@ -77,7 +77,7 @@ public class ProcessSynchronousPlayerLogout implements SynchronousProcess { } // Set player's data to unauthenticated - service.setGroup(player, AuthGroupType.NOT_LOGGED_IN); + service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); player.setOp(false); player.setAllowFlight(false); // Remove speed diff --git a/src/main/java/fr/xephi/authme/process/register/ProcessSyncEmailRegister.java b/src/main/java/fr/xephi/authme/process/register/ProcessSyncEmailRegister.java index aea1b4be..cebbcdcb 100644 --- a/src/main/java/fr/xephi/authme/process/register/ProcessSyncEmailRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/ProcessSyncEmailRegister.java @@ -3,9 +3,8 @@ package fr.xephi.authme.process.register; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.permission.AuthGroupType; -import fr.xephi.authme.service.CommonService; import fr.xephi.authme.process.SynchronousProcess; -import fr.xephi.authme.settings.properties.HooksSettings; +import fr.xephi.authme.service.CommonService; import fr.xephi.authme.task.LimboPlayerTaskManager; import fr.xephi.authme.util.PlayerUtils; import org.bukkit.entity.Player; @@ -25,12 +24,10 @@ public class ProcessSyncEmailRegister implements SynchronousProcess { } public void processEmailRegister(Player player) { - final String name = player.getName().toLowerCase(); - if (!service.getProperty(HooksSettings.REGISTERED_GROUP).isEmpty()) { - service.setGroup(player, AuthGroupType.REGISTERED); - } + service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); service.send(player, MessageKey.ACCOUNT_NOT_ACTIVATED); + final String name = player.getName().toLowerCase(); limboPlayerTaskManager.registerTimeoutTask(player); limboPlayerTaskManager.registerMessageTask(name, true); diff --git a/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java b/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java index fae428d0..0b06df61 100644 --- a/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java +++ b/src/main/java/fr/xephi/authme/process/register/ProcessSyncPasswordRegister.java @@ -4,12 +4,11 @@ import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.data.limbo.LimboCache; import fr.xephi.authme.message.MessageKey; import fr.xephi.authme.permission.AuthGroupType; -import fr.xephi.authme.service.CommonService; import fr.xephi.authme.process.SynchronousProcess; import fr.xephi.authme.service.BungeeService; +import fr.xephi.authme.service.CommonService; import fr.xephi.authme.settings.commandconfig.CommandManager; import fr.xephi.authme.settings.properties.EmailSettings; -import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.task.LimboPlayerTaskManager; import fr.xephi.authme.util.PlayerUtils; @@ -56,10 +55,7 @@ public class ProcessSyncPasswordRegister implements SynchronousProcess { } public void processPasswordRegister(Player player) { - if (!service.getProperty(HooksSettings.REGISTERED_GROUP).isEmpty()) { - service.setGroup(player, AuthGroupType.REGISTERED); - } - + service.setGroup(player, AuthGroupType.REGISTERED_UNAUTHENTICATED); service.send(player, MessageKey.REGISTER_SUCCESS); if (!service.getProperty(EmailSettings.MAIL_ACCOUNT).isEmpty()) { diff --git a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java index aac60dde..e4b175b7 100644 --- a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java +++ b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java @@ -71,6 +71,7 @@ public class SettingsMigrationService extends PlainMigrationService { | hasOldHelpHeaderProperty(resource) | hasSupportOldPasswordProperty(resource) | convertToRegistrationType(resource) + | mergeAndMovePermissionGroupSettings(resource) || hasDeprecatedProperties(resource); } @@ -251,6 +252,26 @@ public class SettingsMigrationService extends PlainMigrationService { return true; } + private static boolean mergeAndMovePermissionGroupSettings(PropertyResource resource) { + boolean performedChanges; + + // We have two old settings replaced by only one: move the first non-empty one + Property oldUnloggedInGroup = newProperty("settings.security.unLoggedinGroup", ""); + Property oldRegisteredGroup = newProperty("GroupOptions.RegisteredPlayerGroup", ""); + if (!oldUnloggedInGroup.getValue(resource).isEmpty()) { + performedChanges = moveProperty(oldUnloggedInGroup, PluginSettings.REGISTERED_GROUP, resource); + } else { + performedChanges = moveProperty(oldRegisteredGroup, PluginSettings.REGISTERED_GROUP, resource); + } + + // Move paths of other old options + performedChanges |= moveProperty(newProperty("GroupOptions.UnregisteredPlayerGroup", ""), + PluginSettings.UNREGISTERED_GROUP, resource); + performedChanges |= moveProperty(newProperty("permission.EnablePermissionCheck", false), + PluginSettings.ENABLE_PERMISSION_CHECK, resource); + return performedChanges; + } + /** * Checks for an old property path and moves it to a new path if present. * @@ -264,9 +285,10 @@ public class SettingsMigrationService extends PlainMigrationService { Property newProperty, PropertyResource resource) { if (resource.contains(oldProperty.getPath())) { - ConsoleLogger.info("Detected deprecated property " + oldProperty.getPath()); - if (!resource.contains(newProperty.getPath())) { - ConsoleLogger.info("Renamed " + oldProperty.getPath() + " to " + newProperty.getPath()); + if (resource.contains(newProperty.getPath())) { + ConsoleLogger.info("Detected deprecated property " + oldProperty.getPath()); + } else { + ConsoleLogger.info("Renaming " + oldProperty.getPath() + " to " + newProperty.getPath()); resource.setValue(newProperty.getPath(), oldProperty.getValue(resource)); } return true; diff --git a/src/main/java/fr/xephi/authme/settings/properties/HooksSettings.java b/src/main/java/fr/xephi/authme/settings/properties/HooksSettings.java index b1eaa222..f512d67b 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/HooksSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/HooksSettings.java @@ -58,13 +58,6 @@ public class HooksSettings implements SettingsHolder { public static final Property WORDPRESS_TABLE_PREFIX = newProperty("ExternalBoardOptions.wordpressTablePrefix", "wp_"); - @Comment("Unregistered permission group") - public static final Property UNREGISTERED_GROUP = - newProperty("GroupOptions.UnregisteredPlayerGroup", ""); - - @Comment("Registered permission group") - public static final Property REGISTERED_GROUP = - newProperty("GroupOptions.RegisteredPlayerGroup", ""); private HooksSettings() { } diff --git a/src/main/java/fr/xephi/authme/settings/properties/PluginSettings.java b/src/main/java/fr/xephi/authme/settings/properties/PluginSettings.java index cfd717eb..eb6ffd6d 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/PluginSettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/PluginSettings.java @@ -44,13 +44,33 @@ public class PluginSettings implements SettingsHolder { newProperty("settings.messagesLanguage", "en"); @Comment({ - "Take care with this option; if you want", - "to use group switching of AuthMe", - "for unloggedIn players, set this setting to true.", - "Default is false." + "Enables switching a player to defined permission groups before they log in.", + "See below for a detailed explanation." }) public static final Property ENABLE_PERMISSION_CHECK = - newProperty("permission.EnablePermissionCheck", false); + newProperty("GroupOptions.enablePermissionCheck", false); + + @Comment({ + "This is a very important option: if a registered player joins the server", + "AuthMe will switch him to unLoggedInGroup. This should prevent all major exploits.", + "You can set up your permission plugin with this special group to have no permissions,", + "or only permission to chat (or permission to send private messages etc.).", + "The better way is to set up this group with few permissions, so if a player", + "tries to exploit an account they can do only what you've defined for the group.", + "After login, the player will be moved to his correct permissions group!", + "Please note that the group name is case-sensitive, so 'admin' is different from 'Admin'", + "Otherwise your group will be wiped and the player will join in the default group []!", + "Example: registeredPlayerGroup: 'NotLogged'" + }) + public static final Property REGISTERED_GROUP = + newProperty("GroupOptions.registeredPlayerGroup", ""); + + @Comment({ + "Similar to above, unregistered players can be set to the following", + "permissions group" + }) + public static final Property UNREGISTERED_GROUP = + newProperty("GroupOptions.unregisteredPlayerGroup", ""); @Comment({ "Log level: INFO, FINE, DEBUG. Use INFO for general messages,", diff --git a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index 5d3d870c..054651ad 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -48,22 +48,6 @@ public class SecuritySettings implements SettingsHolder { public static final Property MAX_PASSWORD_LENGTH = newProperty("settings.security.passwordMaxLength", 30); - @Comment({ - "This is a very important option: every time a player joins the server,", - "if they are registered, AuthMe will switch him to unLoggedInGroup.", - "This should prevent all major exploits.", - "You can set up your permission plugin with this special group to have no permissions,", - "or only permission to chat (or permission to send private messages etc.).", - "The better way is to set up this group with few permissions, so if a player", - "tries to exploit an account they can do only what you've defined for the group.", - "After, a logged in player will be moved to his correct permissions group!", - "Please note that the group name is case-sensitive, so 'admin' is different from 'Admin'", - "Otherwise your group will be wiped and the player will join in the default group []!", - "Example unLoggedinGroup: NotLogged" - }) - public static final Property UNLOGGEDIN_GROUP = - newProperty("settings.security.unLoggedinGroup", "unLoggedinGroup"); - @Comment({ "Possible values: SHA256, BCRYPT, BCRYPT2Y, PBKDF2, SALTEDSHA512, WHIRLPOOL,", "MYBB, IPB3, PHPBB, PHPFUSION, SMF, XENFORO, XAUTH, JOOMLA, WBB3, WBB4, MD5VB,", diff --git a/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java b/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java index 67893666..da7c270b 100644 --- a/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java +++ b/src/test/java/fr/xephi/authme/settings/SettingsMigrationServiceTest.java @@ -18,7 +18,10 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import static fr.xephi.authme.TestHelper.getJarFile; +import static fr.xephi.authme.settings.properties.PluginSettings.ENABLE_PERMISSION_CHECK; import static fr.xephi.authme.settings.properties.PluginSettings.LOG_LEVEL; +import static fr.xephi.authme.settings.properties.PluginSettings.REGISTERED_GROUP; +import static fr.xephi.authme.settings.properties.PluginSettings.UNREGISTERED_GROUP; import static fr.xephi.authme.settings.properties.RegistrationSettings.DELAY_JOIN_MESSAGE; import static fr.xephi.authme.settings.properties.RegistrationSettings.REGISTER_SECOND_ARGUMENT; import static fr.xephi.authme.settings.properties.RegistrationSettings.REGISTRATION_TYPE; @@ -65,6 +68,9 @@ public class SettingsMigrationServiceTest { assertThat(settings.getProperty(LOG_LEVEL), equalTo(LogLevel.INFO)); assertThat(settings.getProperty(REGISTRATION_TYPE), equalTo(RegistrationType.EMAIL)); assertThat(settings.getProperty(REGISTER_SECOND_ARGUMENT), equalTo(RegisterSecondaryArgument.CONFIRMATION)); + assertThat(settings.getProperty(ENABLE_PERMISSION_CHECK), equalTo(true)); + assertThat(settings.getProperty(REGISTERED_GROUP), equalTo("unLoggedinGroup")); + assertThat(settings.getProperty(UNREGISTERED_GROUP), equalTo("")); // Check migration of old setting to email.html assertThat(Files.readLines(new File(dataFolder, "email.html"), StandardCharsets.UTF_8), diff --git a/src/test/resources/fr/xephi/authme/settings/config-old.yml b/src/test/resources/fr/xephi/authme/settings/config-old.yml index 5181b3f7..65e2614c 100644 --- a/src/test/resources/fr/xephi/authme/settings/config-old.yml +++ b/src/test/resources/fr/xephi/authme/settings/config-old.yml @@ -290,7 +290,7 @@ permission: # to use Vault and Group Switching of # AuthMe for unloggedIn players put true # below, default is false. - EnablePermissionCheck: false + EnablePermissionCheck: true BackupSystem: # Enable or Disable Automatic Backup ActivateBackup: false