From 3ad00a45f9a13251dd3a1b70e18f87fc041aca34 Mon Sep 17 00:00:00 2001 From: EbonJaguar Date: Mon, 30 May 2016 16:47:48 -0400 Subject: [PATCH 1/2] Move default permissions out of Commands and into PermissionNode - fixes #606 --- .../authme/command/CommandDescription.java | 33 ++++++----- .../authme/command/CommandInitializer.java | 56 +++++++++---------- .../authme/command/CommandPermissions.java | 52 ----------------- .../authme/command/help/HelpProvider.java | 19 +++---- .../authme/permission/AdminPermission.java | 52 ++++++++++------- .../authme/permission/PermissionNode.java | 6 ++ .../authme/permission/PermissionsManager.java | 7 +-- .../authme/permission/PlayerPermission.java | 35 ++++++++---- .../permission/PlayerStatePermission.java | 21 +++++-- .../command/CommandInitializerTest.java | 15 ++--- .../authme/command/TestCommandsUtil.java | 10 +--- .../authme/command/help/HelpProviderTest.java | 7 ++- .../tools/commands/CommandPageCreater.java | 14 ++--- 13 files changed, 150 insertions(+), 177 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/command/CommandPermissions.java diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 22a3f05cd..595f513df 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -52,9 +52,9 @@ public class CommandDescription { */ private List arguments; /** - * Command permissions required to execute this command. + * Permission node required to execute this command. */ - private CommandPermissions permissions; + private PermissionNode permission; /** * Private constructor. Use {@link CommandDescription#builder()} to create instances of this class. @@ -74,7 +74,7 @@ public class CommandDescription { * @param executableCommand The executable command, or null. * @param parent Parent command. * @param arguments Command arguments. - * @param permissions The permissions required to execute this command. + * @param permission The permission node required to execute this command. * * @return The created instance * @see CommandDescription#builder() @@ -82,7 +82,7 @@ public class CommandDescription { private static CommandDescription createInstance(List labels, String description, String detailedDescription, ExecutableCommand executableCommand, CommandDescription parent, List arguments, - CommandPermissions permissions) { + PermissionNode permission) { CommandDescription instance = new CommandDescription(); instance.labels = labels; instance.description = description; @@ -90,7 +90,7 @@ public class CommandDescription { instance.executableCommand = executableCommand; instance.parent = parent; instance.arguments = arguments; - instance.permissions = permissions; + instance.permission = permission; if (parent != null) { parent.addChild(instance); @@ -196,12 +196,12 @@ public class CommandDescription { } /** - * Return the permissions required to execute the command. + * Return the permission node required to execute the command. * - * @return The command permissions, or null if none are required to execute the command. + * @return The permission node, or null if none are required to execute the command. */ - public CommandPermissions getCommandPermissions() { - return permissions; + public PermissionNode getPermission() { + return permission; } /** @@ -223,7 +223,7 @@ public class CommandDescription { private ExecutableCommand executableCommand; private CommandDescription parent; private List arguments = new ArrayList<>(); - private CommandPermissions permissions; + private PermissionNode permission; /** * Build a CommandDescription from the builder or throw an exception if a mandatory @@ -239,7 +239,7 @@ public class CommandDescription { // parents and permissions may be null; arguments may be empty return createInstance(labels, description, detailedDescription, executableCommand, - parent, arguments, permissions); + parent, arguments, permission); } public CommandBuilder labels(List labels) { @@ -286,9 +286,14 @@ public class CommandDescription { return this; } - public CommandBuilder permissions(DefaultPermission defaultPermission, - PermissionNode... permissionNodes) { - this.permissions = new CommandPermissions(asList(permissionNodes), defaultPermission); + /** + * Add a permission node that the a user must have to execute the command. + * + * @param permission The PermissionNode to add + * @return The builder + */ + public CommandBuilder permission(PermissionNode permission) { + this.permission = permission; return this; } } diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index ca4cec3b7..162d02896 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -82,7 +82,7 @@ public class CommandInitializer { .detailedDescription("Register the specified player with the specified password.") .withArgument("player", "Player name", false) .withArgument("password", "Password", false) - .permissions(OP_ONLY, AdminPermission.REGISTER) + .permission(AdminPermission.REGISTER) .executableCommand(initializer.newInstance(RegisterAdminCommand.class)) .build(); @@ -93,7 +93,7 @@ public class CommandInitializer { .description("Unregister a player") .detailedDescription("Unregister the specified player.") .withArgument("player", "Player name", false) - .permissions(OP_ONLY, AdminPermission.UNREGISTER) + .permission(AdminPermission.UNREGISTER) .executableCommand(initializer.newInstance(UnregisterAdminCommand.class)) .build(); @@ -104,7 +104,7 @@ public class CommandInitializer { .description("Enforce login player") .detailedDescription("Enforce the specified player to login.") .withArgument("player", "Online player name", true) - .permissions(OP_ONLY, AdminPermission.FORCE_LOGIN) + .permission(AdminPermission.FORCE_LOGIN) .executableCommand(initializer.newInstance(ForceLoginCommand.class)) .build(); @@ -116,7 +116,7 @@ public class CommandInitializer { .detailedDescription("Change the password of a player.") .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) - .permissions(OP_ONLY, AdminPermission.CHANGE_PASSWORD) + .permission(AdminPermission.CHANGE_PASSWORD) .executableCommand(initializer.newInstance(ChangePasswordAdminCommand.class)) .build(); @@ -127,7 +127,7 @@ public class CommandInitializer { .description("Player's last login") .detailedDescription("View the date of the specified players last login.") .withArgument("player", "Player name", true) - .permissions(OP_ONLY, AdminPermission.LAST_LOGIN) + .permission(AdminPermission.LAST_LOGIN) .executableCommand(initializer.newInstance(LastLoginCommand.class)) .build(); @@ -138,7 +138,7 @@ public class CommandInitializer { .description("Display player accounts") .detailedDescription("Display all accounts of a player by his player name or IP.") .withArgument("player", "Player name or IP", true) - .permissions(OP_ONLY, AdminPermission.ACCOUNTS) + .permission(AdminPermission.ACCOUNTS) .executableCommand(initializer.newInstance(AccountsCommand.class)) .build(); @@ -149,7 +149,7 @@ public class CommandInitializer { .description("Display player's email") .detailedDescription("Display the email address of the specified player if set.") .withArgument("player", "Player name", true) - .permissions(OP_ONLY, AdminPermission.GET_EMAIL) + .permission(AdminPermission.GET_EMAIL) .executableCommand(initializer.newInstance(GetEmailCommand.class)) .build(); @@ -161,7 +161,7 @@ public class CommandInitializer { .detailedDescription("Change the email address of the specified player.") .withArgument("player", "Player name", false) .withArgument("email", "Player email", false) - .permissions(OP_ONLY, AdminPermission.CHANGE_EMAIL) + .permission(AdminPermission.CHANGE_EMAIL) .executableCommand(initializer.newInstance(SetEmailCommand.class)) .build(); @@ -172,7 +172,7 @@ public class CommandInitializer { .description("Get player's IP") .detailedDescription("Get the IP address of the specified online player.") .withArgument("player", "Player name", false) - .permissions(OP_ONLY, AdminPermission.GET_IP) + .permission(AdminPermission.GET_IP) .executableCommand(initializer.newInstance(GetIpCommand.class)) .build(); @@ -182,7 +182,7 @@ public class CommandInitializer { .labels("spawn", "home") .description("Teleport to spawn") .detailedDescription("Teleport to the spawn.") - .permissions(OP_ONLY, AdminPermission.SPAWN) + .permission(AdminPermission.SPAWN) .executableCommand(initializer.newInstance(SpawnCommand.class)) .build(); @@ -192,7 +192,7 @@ public class CommandInitializer { .labels("setspawn", "chgspawn") .description("Change the spawn") .detailedDescription("Change the player's spawn to your current position.") - .permissions(OP_ONLY, AdminPermission.SET_SPAWN) + .permission(AdminPermission.SET_SPAWN) .executableCommand(initializer.newInstance(SetSpawnCommand.class)) .build(); @@ -202,7 +202,7 @@ public class CommandInitializer { .labels("firstspawn", "firsthome") .description("Teleport to first spawn") .detailedDescription("Teleport to the first spawn.") - .permissions(OP_ONLY, AdminPermission.FIRST_SPAWN) + .permission(AdminPermission.FIRST_SPAWN) .executableCommand(initializer.newInstance(FirstSpawnCommand.class)) .build(); @@ -212,7 +212,7 @@ public class CommandInitializer { .labels("setfirstspawn", "chgfirstspawn") .description("Change the first spawn") .detailedDescription("Change the first player's spawn to your current position.") - .permissions(OP_ONLY, AdminPermission.SET_FIRST_SPAWN) + .permission(AdminPermission.SET_FIRST_SPAWN) .executableCommand(initializer.newInstance(SetFirstSpawnCommand.class)) .build(); @@ -223,7 +223,7 @@ public class CommandInitializer { .description("Purge old data") .detailedDescription("Purge old AuthMeReloaded data longer than the specified amount of days ago.") .withArgument("days", "Number of days", false) - .permissions(OP_ONLY, AdminPermission.PURGE) + .permission(AdminPermission.PURGE) .executableCommand(initializer.newInstance(PurgeCommand.class)) .build(); @@ -235,7 +235,7 @@ public class CommandInitializer { .description("Purge player's last position") .detailedDescription("Purge the last know position of the specified player or all of them.") .withArgument("player/*", "Player name or * for all players", false) - .permissions(OP_ONLY, AdminPermission.PURGE_LAST_POSITION) + .permission(AdminPermission.PURGE_LAST_POSITION) .executableCommand(initializer.newInstance(PurgeLastPositionCommand.class)) .build(); @@ -245,7 +245,7 @@ public class CommandInitializer { .labels("purgebannedplayers", "purgebannedplayer", "deletebannedplayers", "deletebannedplayer") .description("Purge banned players data") .detailedDescription("Purge all AuthMeReloaded data for banned players.") - .permissions(OP_ONLY, AdminPermission.PURGE_BANNED_PLAYERS) + .permission(AdminPermission.PURGE_BANNED_PLAYERS) .executableCommand(initializer.newInstance(PurgeBannedPlayersCommand.class)) .build(); @@ -256,7 +256,7 @@ public class CommandInitializer { .description("Switch AntiBot mode") .detailedDescription("Switch or toggle the AntiBot mode to the specified state.") .withArgument("mode", "ON / OFF", true) - .permissions(OP_ONLY, AdminPermission.SWITCH_ANTIBOT) + .permission(AdminPermission.SWITCH_ANTIBOT) .executableCommand(initializer.newInstance(SwitchAntiBotCommand.class)) .build(); @@ -266,7 +266,7 @@ public class CommandInitializer { .labels("reload", "rld") .description("Reload plugin") .detailedDescription("Reload the AuthMeReloaded plugin.") - .permissions(OP_ONLY, AdminPermission.RELOAD) + .permission(AdminPermission.RELOAD) .executableCommand(initializer.newInstance(ReloadCommand.class)) .build(); @@ -287,7 +287,7 @@ public class CommandInitializer { .detailedDescription("Converter command for AuthMeReloaded.") .withArgument("job", "Conversion job: xauth / crazylogin / rakamak / " + "royalauth / vauth / sqlitetosql", false) - .permissions(OP_ONLY, AdminPermission.CONVERTER) + .permission(AdminPermission.CONVERTER) .executableCommand(initializer.newInstance(ConverterCommand.class)) .build(); @@ -298,7 +298,7 @@ public class CommandInitializer { .description("Login command") .detailedDescription("Command to log in using AuthMeReloaded.") .withArgument("password", "Login password", false) - .permissions(ALLOWED, PlayerPermission.LOGIN) + .permission(PlayerPermission.LOGIN) .executableCommand(initializer.newInstance(LoginCommand.class)) .build(); @@ -308,7 +308,7 @@ public class CommandInitializer { .labels("logout") .description("Logout command") .detailedDescription("Command to logout using AuthMeReloaded.") - .permissions(ALLOWED, PlayerPermission.LOGOUT) + .permission(PlayerPermission.LOGOUT) .executableCommand(initializer.newInstance(LogoutCommand.class)) .build(); @@ -320,7 +320,7 @@ public class CommandInitializer { .detailedDescription("Command to register using AuthMeReloaded.") .withArgument("password", "Password", true) .withArgument("verifyPassword", "Verify password", true) - .permissions(ALLOWED, PlayerPermission.REGISTER) + .permission(PlayerPermission.REGISTER) .executableCommand(initializer.newInstance(RegisterCommand.class)) .build(); @@ -331,7 +331,7 @@ public class CommandInitializer { .description("Unregistration Command") .detailedDescription("Command to unregister using AuthMeReloaded.") .withArgument("password", "Password", false) - .permissions(ALLOWED, PlayerPermission.UNREGISTER) + .permission(PlayerPermission.UNREGISTER) .executableCommand(initializer.newInstance(UnregisterCommand.class)) .build(); @@ -343,7 +343,7 @@ public class CommandInitializer { .detailedDescription("Command to change your password using AuthMeReloaded.") .withArgument("oldPassword", "Old Password", false) .withArgument("newPassword", "New Password.", false) - .permissions(ALLOWED, PlayerPermission.CHANGE_PASSWORD) + .permission(PlayerPermission.CHANGE_PASSWORD) .executableCommand(initializer.newInstance(ChangePasswordCommand.class)) .build(); @@ -364,7 +364,7 @@ public class CommandInitializer { .detailedDescription("Add a new email address to your account.") .withArgument("email", "Email address", false) .withArgument("verifyEmail", "Email address verification", false) - .permissions(ALLOWED, PlayerPermission.ADD_EMAIL) + .permission(PlayerPermission.ADD_EMAIL) .executableCommand(initializer.newInstance(AddEmailCommand.class)) .build(); @@ -376,7 +376,7 @@ public class CommandInitializer { .detailedDescription("Change an email address of your account.") .withArgument("oldEmail", "Old email address", false) .withArgument("newEmail", "New email address", false) - .permissions(ALLOWED, PlayerPermission.CHANGE_EMAIL) + .permission(PlayerPermission.CHANGE_EMAIL) .executableCommand(initializer.newInstance(ChangeEmailCommand.class)) .build(); @@ -388,7 +388,7 @@ public class CommandInitializer { .detailedDescription("Recover your account using an Email address by sending a mail containing " + "a new password.") .withArgument("email", "Email address", false) - .permissions(ALLOWED, PlayerPermission.RECOVER_EMAIL) + .permission(PlayerPermission.RECOVER_EMAIL) .executableCommand(initializer.newInstance(RecoverEmailCommand.class)) .build(); @@ -399,7 +399,7 @@ public class CommandInitializer { .description("Captcha Command") .detailedDescription("Captcha command for AuthMeReloaded.") .withArgument("captcha", "The Captcha", false) - .permissions(ALLOWED, PlayerPermission.CAPTCHA) + .permission(PlayerPermission.CAPTCHA) .executableCommand(initializer.newInstance(CaptchaCommand.class)) .build(); diff --git a/src/main/java/fr/xephi/authme/command/CommandPermissions.java b/src/main/java/fr/xephi/authme/command/CommandPermissions.java deleted file mode 100644 index 0082b8583..000000000 --- a/src/main/java/fr/xephi/authme/command/CommandPermissions.java +++ /dev/null @@ -1,52 +0,0 @@ -package fr.xephi.authme.command; - -import fr.xephi.authme.permission.DefaultPermission; -import fr.xephi.authme.permission.PermissionNode; - -import java.util.List; - -/** - */ -public class CommandPermissions { - - /** - * Defines the permission nodes required to have permission to execute this command. - */ - private List permissionNodes; - /** - * Defines the default permission if the permission nodes couldn't be used. - */ - private DefaultPermission defaultPermission; - - /** - * Constructor. - * - * @param permissionNodes The permission nodes required to execute a command. - * @param defaultPermission The default permission if the permission nodes couldn't be used. - */ - public CommandPermissions(List permissionNodes, DefaultPermission defaultPermission) { - this.permissionNodes = permissionNodes; - this.defaultPermission = defaultPermission; - } - - /** - * Get the permission nodes required to execute this command. - * - * @return The permission nodes required to execute this command. - */ - public List getPermissionNodes() { - return this.permissionNodes; - } - - - /** - * Get the default permission if the permission nodes couldn't be used. - * - * @return The default permission. - */ - public DefaultPermission getDefaultPermission() { - return this.defaultPermission; - } - - -} diff --git a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java index 0acf62f88..1d8b6bdb7 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpProvider.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpProvider.java @@ -4,7 +4,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; -import fr.xephi.authme.command.CommandPermissions; import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.initialization.SettingsDependent; @@ -13,7 +12,6 @@ import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.permission.PermissionsManager; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PluginSettings; -import fr.xephi.authme.util.CollectionUtils; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; @@ -139,22 +137,19 @@ public class HelpProvider implements SettingsDependent { private static void printPermissions(CommandDescription command, CommandSender sender, PermissionsManager permissionsManager, List lines) { - CommandPermissions permissions = command.getCommandPermissions(); - // TODO ljacqu 20151224: Isn't it possible to have a default permission but no permission nodes? - if (permissions == null || CollectionUtils.isEmpty(permissions.getPermissionNodes())) { + PermissionNode permission = command.getPermission(); + if (permission == null) { return; } lines.add(ChatColor.GOLD + "Permissions:"); - for (PermissionNode node : permissions.getPermissionNodes()) { - boolean hasPermission = permissionsManager.hasPermission(sender, node); - final String nodePermsString = "" + ChatColor.GRAY + ChatColor.ITALIC - + (hasPermission ? " (You have permission)" : " (No permission)"); - lines.add(" " + ChatColor.YELLOW + ChatColor.ITALIC + node.getNode() + nodePermsString); - } + boolean hasPermission = permissionsManager.hasPermission(sender, permission); + final String nodePermsString = "" + ChatColor.GRAY + ChatColor.ITALIC + + (hasPermission ? " (You have permission)" : " (No permission)"); + lines.add(" " + ChatColor.YELLOW + ChatColor.ITALIC + permission.getNode() + nodePermsString); // Addendum to the line to specify whether the sender has permission or not when default is OP_ONLY - final DefaultPermission defaultPermission = permissions.getDefaultPermission(); + final DefaultPermission defaultPermission = permission.getDefaultPermission(); String addendum = ""; if (DefaultPermission.OP_ONLY.equals(defaultPermission)) { addendum = PermissionsManager.evaluateDefaultPermission(defaultPermission, sender) diff --git a/src/main/java/fr/xephi/authme/permission/AdminPermission.java b/src/main/java/fr/xephi/authme/permission/AdminPermission.java index 6dedf827e..043ba6dc2 100644 --- a/src/main/java/fr/xephi/authme/permission/AdminPermission.java +++ b/src/main/java/fr/xephi/authme/permission/AdminPermission.java @@ -8,115 +8,121 @@ public enum AdminPermission implements PermissionNode { /** * Administrator command to register a new user. */ - REGISTER("authme.admin.register"), + REGISTER("authme.admin.register", DefaultPermission.OP_ONLY), /** * Administrator command to unregister an existing user. */ - UNREGISTER("authme.admin.unregister"), + UNREGISTER("authme.admin.unregister", DefaultPermission.OP_ONLY), /** * Administrator command to force-login an existing user. */ - FORCE_LOGIN("authme.admin.forcelogin"), + FORCE_LOGIN("authme.admin.forcelogin", DefaultPermission.OP_ONLY), /** * Administrator command to change the password of a user. */ - CHANGE_PASSWORD("authme.admin.changepassword"), + CHANGE_PASSWORD("authme.admin.changepassword", DefaultPermission.OP_ONLY), /** * Administrator command to see the last login date and time of a user. */ - LAST_LOGIN("authme.admin.lastlogin"), + LAST_LOGIN("authme.admin.lastlogin", DefaultPermission.OP_ONLY), /** * Administrator command to see all accounts associated with a user. */ - ACCOUNTS("authme.admin.accounts"), + ACCOUNTS("authme.admin.accounts", DefaultPermission.OP_ONLY), /** * Administrator command to get the email address of a user, if set. */ - GET_EMAIL("authme.admin.getemail"), + GET_EMAIL("authme.admin.getemail", DefaultPermission.OP_ONLY), /** * Administrator command to set or change the email address of a user. */ - CHANGE_EMAIL("authme.admin.changemail"), + CHANGE_EMAIL("authme.admin.changemail", DefaultPermission.OP_ONLY), /** * Administrator command to get the last known IP of a user. */ - GET_IP("authme.admin.getip"), + GET_IP("authme.admin.getip", DefaultPermission.OP_ONLY), /** * Administrator command to teleport to the AuthMe spawn. */ - SPAWN("authme.admin.spawn"), + SPAWN("authme.admin.spawn", DefaultPermission.OP_ONLY), /** * Administrator command to set the AuthMe spawn. */ - SET_SPAWN("authme.admin.setspawn"), + SET_SPAWN("authme.admin.setspawn", DefaultPermission.OP_ONLY), /** * Administrator command to teleport to the first AuthMe spawn. */ - FIRST_SPAWN("authme.admin.firstspawn"), + FIRST_SPAWN("authme.admin.firstspawn", DefaultPermission.OP_ONLY), /** * Administrator command to set the first AuthMe spawn. */ - SET_FIRST_SPAWN("authme.admin.setfirstspawn"), + SET_FIRST_SPAWN("authme.admin.setfirstspawn", DefaultPermission.OP_ONLY), /** * Administrator command to purge old user data. */ - PURGE("authme.admin.purge"), + PURGE("authme.admin.purge", DefaultPermission.OP_ONLY), /** * Administrator command to purge the last position of a user. */ - PURGE_LAST_POSITION("authme.admin.purgelastpos"), + PURGE_LAST_POSITION("authme.admin.purgelastpos", DefaultPermission.OP_ONLY), /** * Administrator command to purge all data associated with banned players. */ - PURGE_BANNED_PLAYERS("authme.admin.purgebannedplayers"), + PURGE_BANNED_PLAYERS("authme.admin.purgebannedplayers", DefaultPermission.OP_ONLY), /** * Administrator command to toggle the AntiBot protection status. */ - SWITCH_ANTIBOT("authme.admin.switchantibot"), + SWITCH_ANTIBOT("authme.admin.switchantibot", DefaultPermission.OP_ONLY), /** * Administrator command to convert old or other data to AuthMe data. */ - CONVERTER("authme.admin.converter"), + CONVERTER("authme.admin.converter", DefaultPermission.OP_ONLY), /** * Administrator command to reload the plugin configuration. */ - RELOAD("authme.admin.reload"), + RELOAD("authme.admin.reload", DefaultPermission.OP_ONLY), /** * Permission to see the other accounts of the players that log in. */ - SEE_OTHER_ACCOUNTS("authme.admin.seeotheraccounts"); + SEE_OTHER_ACCOUNTS("authme.admin.seeotheraccounts", DefaultPermission.OP_ONLY); /** * The permission node. */ private String node; + /** + * The default permission level + */ + private DefaultPermission defaultPermission; + /** * Constructor. * * @param node Permission node. */ - AdminPermission(String node) { + AdminPermission(String node, DefaultPermission defaultPermission) { this.node = node; + this.defaultPermission = defaultPermission; } @Override @@ -124,4 +130,8 @@ public enum AdminPermission implements PermissionNode { return node; } + @Override + public DefaultPermission getDefaultPermission() { + return defaultPermission; + } } diff --git a/src/main/java/fr/xephi/authme/permission/PermissionNode.java b/src/main/java/fr/xephi/authme/permission/PermissionNode.java index 04f9c80cc..189f97b58 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionNode.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionNode.java @@ -12,4 +12,10 @@ public interface PermissionNode { */ String getNode(); + /** + * Return the default permission for this node, e.g. "OP_ONLY" + * + * @return The default level of permission + */ + DefaultPermission getDefaultPermission(); } diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 1a5fd9291..358bc5556 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -285,15 +285,14 @@ public class PermissionsManager implements PermissionsService { } public boolean hasPermission(CommandSender sender, CommandDescription command) { - if (command.getCommandPermissions() == null - || CollectionUtils.isEmpty(command.getCommandPermissions().getPermissionNodes())) { + if (command.getPermission() == null) { return true; } - DefaultPermission defaultPermission = command.getCommandPermissions().getDefaultPermission(); + DefaultPermission defaultPermission = command.getPermission().getDefaultPermission(); boolean def = evaluateDefaultPermission(defaultPermission, sender); return (sender instanceof Player) - ? hasPermission((Player) sender, command.getCommandPermissions().getPermissionNodes(), def) + ? hasPermission((Player) sender, command.getPermission().getNode(), def) : def; } diff --git a/src/main/java/fr/xephi/authme/permission/PlayerPermission.java b/src/main/java/fr/xephi/authme/permission/PlayerPermission.java index 210cbce93..e2932e932 100644 --- a/src/main/java/fr/xephi/authme/permission/PlayerPermission.java +++ b/src/main/java/fr/xephi/authme/permission/PlayerPermission.java @@ -8,70 +8,76 @@ public enum PlayerPermission implements PermissionNode { /** * Command permission to login. */ - LOGIN("authme.player.login"), + LOGIN("authme.player.login", DefaultPermission.ALLOWED), /** * Command permission to logout. */ - LOGOUT("authme.player.logout"), + LOGOUT("authme.player.logout", DefaultPermission.ALLOWED), /** * Command permission to register. */ - REGISTER("authme.player.register"), + REGISTER("authme.player.register", DefaultPermission.ALLOWED), /** * Command permission to unregister. */ - UNREGISTER("authme.player.unregister"), + UNREGISTER("authme.player.unregister", DefaultPermission.ALLOWED), /** * Command permission to change the password. */ - CHANGE_PASSWORD("authme.player.changepassword"), + CHANGE_PASSWORD("authme.player.changepassword", DefaultPermission.ALLOWED), /** * Command permission to add an email address. */ - ADD_EMAIL("authme.player.email.add"), + ADD_EMAIL("authme.player.email.add", DefaultPermission.ALLOWED), /** * Command permission to change the email address. */ - CHANGE_EMAIL("authme.player.email.change"), + CHANGE_EMAIL("authme.player.email.change", DefaultPermission.ALLOWED), /** * Command permission to recover an account using it's email address. */ - RECOVER_EMAIL("authme.player.email.recover"), + RECOVER_EMAIL("authme.player.email.recover", DefaultPermission.ALLOWED), /** * Command permission to use captcha. */ - CAPTCHA("authme.player.captcha"), + CAPTCHA("authme.player.captcha", DefaultPermission.ALLOWED), /** * Permission for users a login can be forced to. */ - CAN_LOGIN_BE_FORCED("authme.player.canbeforced"), + CAN_LOGIN_BE_FORCED("authme.player.canbeforced", DefaultPermission.ALLOWED), /** * Permission to use to see own other accounts. */ - SEE_OWN_ACCOUNTS("authme.player.seeownaccounts"); + SEE_OWN_ACCOUNTS("authme.player.seeownaccounts", DefaultPermission.ALLOWED); /** * The permission node. */ private String node; + /** + * The default permission level + */ + private DefaultPermission defaultPermission; + /** * Constructor. * * @param node Permission node. */ - PlayerPermission(String node) { + PlayerPermission(String node, DefaultPermission defaultPermission) { this.node = node; + this.defaultPermission = defaultPermission; } @Override @@ -79,4 +85,9 @@ public enum PlayerPermission implements PermissionNode { return node; } + @Override + public DefaultPermission getDefaultPermission() { + return defaultPermission; + } + } diff --git a/src/main/java/fr/xephi/authme/permission/PlayerStatePermission.java b/src/main/java/fr/xephi/authme/permission/PlayerStatePermission.java index b91a1d4b9..fd0239358 100644 --- a/src/main/java/fr/xephi/authme/permission/PlayerStatePermission.java +++ b/src/main/java/fr/xephi/authme/permission/PlayerStatePermission.java @@ -9,39 +9,50 @@ public enum PlayerStatePermission implements PermissionNode { /** * Permission node to bypass AntiBot protection. */ - BYPASS_ANTIBOT("authme.bypassantibot"), + BYPASS_ANTIBOT("authme.bypassantibot", DefaultPermission.OP_ONLY), /** * Permission for users to bypass force-survival mode. */ - BYPASS_FORCE_SURVIVAL("authme.bypassforcesurvival"), + BYPASS_FORCE_SURVIVAL("authme.bypassforcesurvival", DefaultPermission.OP_ONLY), /** * Permission node to identify VIP users. */ - IS_VIP("authme.vip"), + IS_VIP("authme.vip", DefaultPermission.OP_ONLY), /** * Permission to be able to register multiple accounts. */ - ALLOW_MULTIPLE_ACCOUNTS("authme.allowmultipleaccounts"); + ALLOW_MULTIPLE_ACCOUNTS("authme.allowmultipleaccounts", DefaultPermission.OP_ONLY); /** * The permission node. */ private String node; + /** + * The default permission level + */ + private DefaultPermission defaultPermission; + /** * Constructor. * * @param node Permission node. */ - PlayerStatePermission(String node) { + PlayerStatePermission(String node, DefaultPermission defaultPermission) { this.node = node; + this.defaultPermission = defaultPermission; } @Override public String getNode() { return node; } + + @Override + public DefaultPermission getDefaultPermission() { + return defaultPermission; + } } diff --git a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index 1af9687ba..81f4741a1 100644 --- a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -256,21 +256,16 @@ public class CommandInitializerTest { BiConsumer adminPermissionChecker = new BiConsumer() { @Override public void accept(CommandDescription command, int depth) { - CommandPermissions permissions = command.getCommandPermissions(); - if (permissions != null && OP_ONLY.equals(permissions.getDefaultPermission()) - && !hasAdminNode(permissions)) { + PermissionNode permission = command.getPermission(); + if (permission != null && OP_ONLY.equals(permission.getDefaultPermission()) + && !hasAdminNode(permission)) { fail("The command with labels " + command.getLabels() + " has OP_ONLY default " + "permission but no permission node on admin level"); } } - private boolean hasAdminNode(CommandPermissions permissions) { - for (PermissionNode node : permissions.getPermissionNodes()) { - if (node instanceof AdminPermission) { - return true; - } - } - return false; + private boolean hasAdminNode(PermissionNode permission) { + return permission instanceof AdminPermission; } }; diff --git a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java index 6f7857b0b..2fc4c2158 100644 --- a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java +++ b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java @@ -84,18 +84,10 @@ public final class TestCommandsUtil { /** Shortcut command to initialize a new test command. */ private static CommandDescription createCommand(PermissionNode permission, CommandDescription parent, List labels, CommandArgumentDescription... arguments) { - PermissionNode[] notNullPermission; - if (permission == null) { - notNullPermission = new PermissionNode[0]; - } else { - notNullPermission = new PermissionNode[1]; - notNullPermission[0] = permission; - } - CommandDescription.CommandBuilder command = CommandDescription.builder() .labels(labels) .parent(parent) - .permissions(DefaultPermission.OP_ONLY, notNullPermission) + .permission(permission) .description(labels.get(0) + " cmd") .detailedDescription("'" + labels.get(0) + "' test command") .executableCommand(mock(ExecutableCommand.class)); diff --git a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java index e7d85f146..7c6cf74d3 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java @@ -12,6 +12,7 @@ import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import java.util.Arrays; @@ -124,6 +125,8 @@ public class HelpProviderTest { } @Test + @Ignore + // TODO Gnat008 20160530: Update tests for new PermissionNode setup public void shouldShowAndEvaluatePermissions() { // given CommandDescription command = getCommandWithLabel(commands, "authme", "login"); @@ -145,6 +148,8 @@ public class HelpProviderTest { } @Test + @Ignore + // TODO Gnat008 20160530: Update tests for new PermissionNode setup public void shouldShowAndEvaluateForbiddenPermissions() { // given CommandDescription command = getCommandWithLabel(commands, "authme", "login"); @@ -182,7 +187,7 @@ public class HelpProviderTest { public void shouldNotShowAnythingForNullPermissionsOnCommand() { // given CommandDescription command = mock(CommandDescription.class); - given(command.getCommandPermissions()).willReturn(null); + given(command.getPermission()).willReturn(null); given(command.getLabels()).willReturn(Collections.singletonList("test")); FoundCommandResult result = newFoundResult(command, Collections.singletonList("test")); diff --git a/src/test/java/tools/commands/CommandPageCreater.java b/src/test/java/tools/commands/CommandPageCreater.java index 74bc74a9e..b84782052 100644 --- a/src/test/java/tools/commands/CommandPageCreater.java +++ b/src/test/java/tools/commands/CommandPageCreater.java @@ -3,7 +3,6 @@ package tools.commands; import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandInitializer; -import fr.xephi.authme.command.CommandPermissions; import fr.xephi.authme.command.CommandUtils; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.initialization.AuthMeServiceInitializer; @@ -58,7 +57,7 @@ public class CommandPageCreater implements AutoToolTask { .put("command", CommandUtils.constructCommandPath(command)) .put("description", command.getDetailedDescription()) .put("arguments", formatArguments(command.getArguments())) - .put("permissions", formatPermissions(command.getCommandPermissions())); + .put("permissions", formatPermissions(command.getPermission())); commandTags.add(tags); if (!command.getChildren().isEmpty()) { @@ -67,15 +66,12 @@ public class CommandPageCreater implements AutoToolTask { } } - private static String formatPermissions(CommandPermissions permissions) { - if (permissions == null) { + private static String formatPermissions(PermissionNode permission) { + if (permission == null) { return ""; + } else { + return permission.getNode(); } - String result = ""; - for (PermissionNode node : permissions.getPermissionNodes()) { - result += node.getNode() + " "; - } - return result.trim(); } private static String formatArguments(Iterable arguments) { From 30b72bec4c9ba14a016037c125dd5f2b87fa64b1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Mon, 30 May 2016 23:49:59 +0200 Subject: [PATCH 2/2] #604 Fix HelpProvider tests --- .../authme/command/CommandDescription.java | 3 +-- .../authme/command/TestCommandsUtil.java | 6 ++--- .../authme/command/help/HelpProviderTest.java | 23 ++++++++----------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandDescription.java b/src/main/java/fr/xephi/authme/command/CommandDescription.java index 595f513df..7ff0e7394 100644 --- a/src/main/java/fr/xephi/authme/command/CommandDescription.java +++ b/src/main/java/fr/xephi/authme/command/CommandDescription.java @@ -1,6 +1,5 @@ package fr.xephi.authme.command; -import fr.xephi.authme.permission.DefaultPermission; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.util.CollectionUtils; import fr.xephi.authme.util.StringUtils; @@ -287,7 +286,7 @@ public class CommandDescription { } /** - * Add a permission node that the a user must have to execute the command. + * Add a permission node that a user must have to execute the command. * * @param permission The PermissionNode to add * @return The builder diff --git a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java index 2fc4c2158..65fb74e25 100644 --- a/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java +++ b/src/test/java/fr/xephi/authme/command/TestCommandsUtil.java @@ -1,7 +1,7 @@ package fr.xephi.authme.command; import fr.xephi.authme.command.executable.HelpCommand; -import fr.xephi.authme.permission.DefaultPermission; +import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.permission.PlayerPermission; @@ -43,8 +43,8 @@ public final class TestCommandsUtil { .description("test").detailedDescription("Test.").withArgument("Query", "", false).build(); // Register /unregister , alias: /unreg - CommandDescription unregisterBase = createCommand(null, null, asList("unregister", "unreg"), - newArgument("player", false)); + CommandDescription unregisterBase = createCommand(AdminPermission.UNREGISTER, null, + asList("unregister", "unreg"), newArgument("player", false)); return newHashSet(authMeBase, emailBase, unregisterBase); } diff --git a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java index 7c6cf74d3..1c877b738 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpProviderTest.java @@ -4,15 +4,14 @@ import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.FoundCommandResult; import fr.xephi.authme.command.FoundResultStatus; import fr.xephi.authme.command.TestCommandsUtil; +import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PermissionsManager; -import fr.xephi.authme.permission.PlayerPermission; import fr.xephi.authme.settings.NewSetting; import fr.xephi.authme.settings.properties.PluginSettings; import org.bukkit.ChatColor; import org.bukkit.command.CommandSender; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; import java.util.Arrays; @@ -125,14 +124,12 @@ public class HelpProviderTest { } @Test - @Ignore - // TODO Gnat008 20160530: Update tests for new PermissionNode setup public void shouldShowAndEvaluatePermissions() { // given - CommandDescription command = getCommandWithLabel(commands, "authme", "login"); - FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); + CommandDescription command = getCommandWithLabel(commands, "unregister"); + FoundCommandResult result = newFoundResult(command, Collections.singletonList("unreg")); given(sender.isOp()).willReturn(true); - given(permissionsManager.hasPermission(sender, PlayerPermission.LOGIN)).willReturn(true); + given(permissionsManager.hasPermission(sender, AdminPermission.UNREGISTER)).willReturn(true); given(permissionsManager.hasPermission(sender, command)).willReturn(true); // when @@ -142,20 +139,18 @@ public class HelpProviderTest { assertThat(lines, hasSize(5)); assertThat(removeColors(lines.get(1)), containsString("Permissions:")); assertThat(removeColors(lines.get(2)), - containsString(PlayerPermission.LOGIN.getNode() + " (You have permission)")); + containsString(AdminPermission.UNREGISTER.getNode() + " (You have permission)")); assertThat(removeColors(lines.get(3)), containsString("Default: OP's only (You have permission)")); assertThat(removeColors(lines.get(4)), containsString("Result: You have permission")); } @Test - @Ignore - // TODO Gnat008 20160530: Update tests for new PermissionNode setup public void shouldShowAndEvaluateForbiddenPermissions() { // given - CommandDescription command = getCommandWithLabel(commands, "authme", "login"); - FoundCommandResult result = newFoundResult(command, Collections.singletonList("authme")); + CommandDescription command = getCommandWithLabel(commands, "unregister"); + FoundCommandResult result = newFoundResult(command, Collections.singletonList("unregister")); given(sender.isOp()).willReturn(false); - given(permissionsManager.hasPermission(sender, PlayerPermission.LOGIN)).willReturn(false); + given(permissionsManager.hasPermission(sender, AdminPermission.UNREGISTER)).willReturn(false); given(permissionsManager.hasPermission(sender, command)).willReturn(false); // when @@ -165,7 +160,7 @@ public class HelpProviderTest { assertThat(lines, hasSize(5)); assertThat(removeColors(lines.get(1)), containsString("Permissions:")); assertThat(removeColors(lines.get(2)), - containsString(PlayerPermission.LOGIN.getNode() + " (No permission)")); + containsString(AdminPermission.UNREGISTER.getNode() + " (No permission)")); assertThat(removeColors(lines.get(3)), containsString("Default: OP's only (No permission)")); assertThat(removeColors(lines.get(4)), containsString("Result: No permission")); }