From b419c72271a985e9b52f00bf3facbfe64f0d15cb Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 5 Dec 2015 10:48:42 +0100 Subject: [PATCH] Fix #316 Ordinary player can use /authme unregister - Change wrong player permissions to admin permissions - Rename command class names that were the same for the ordinary vs. admin task - Create test to ensure that commands with OP_ONLY default require an admin permission node --- .../authme/command/CommandInitializer.java | 39 ++++++++++--------- ...d.java => ChangePasswordAdminCommand.java} | 2 +- ...Command.java => RegisterAdminCommand.java} | 3 +- ...mmand.java => UnregisterAdminCommand.java} | 3 +- .../command/CommandInitializerTest.java | 39 +++++++++++++++++++ ...est.java => RegisterAdminCommandTest.java} | 2 +- .../command/help/HelpSyntaxHelperTest.java | 4 +- 7 files changed, 68 insertions(+), 24 deletions(-) rename src/main/java/fr/xephi/authme/command/executable/authme/{ChangePasswordCommand.java => ChangePasswordAdminCommand.java} (98%) rename src/main/java/fr/xephi/authme/command/executable/authme/{RegisterCommand.java => RegisterAdminCommand.java} (97%) rename src/main/java/fr/xephi/authme/command/executable/authme/{UnregisterCommand.java => UnregisterAdminCommand.java} (97%) rename src/test/java/fr/xephi/authme/command/executable/register/{RegisterCommandTest.java => RegisterAdminCommandTest.java} (98%) diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 7fcd0f02..a8dceff0 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -3,7 +3,7 @@ package fr.xephi.authme.command; import fr.xephi.authme.command.executable.HelpCommand; import fr.xephi.authme.command.executable.authme.AccountsCommand; import fr.xephi.authme.command.executable.authme.AuthMeCommand; -import fr.xephi.authme.command.executable.authme.ChangePasswordCommand; +import fr.xephi.authme.command.executable.authme.ChangePasswordAdminCommand; import fr.xephi.authme.command.executable.authme.FirstSpawnCommand; import fr.xephi.authme.command.executable.authme.ForceLoginCommand; import fr.xephi.authme.command.executable.authme.GetEmailCommand; @@ -12,22 +12,25 @@ import fr.xephi.authme.command.executable.authme.LastLoginCommand; import fr.xephi.authme.command.executable.authme.PurgeBannedPlayersCommand; import fr.xephi.authme.command.executable.authme.PurgeCommand; import fr.xephi.authme.command.executable.authme.PurgeLastPositionCommand; -import fr.xephi.authme.command.executable.authme.RegisterCommand; +import fr.xephi.authme.command.executable.authme.RegisterAdminCommand; import fr.xephi.authme.command.executable.authme.ReloadCommand; import fr.xephi.authme.command.executable.authme.SetEmailCommand; import fr.xephi.authme.command.executable.authme.SetFirstSpawnCommand; import fr.xephi.authme.command.executable.authme.SetSpawnCommand; import fr.xephi.authme.command.executable.authme.SpawnCommand; import fr.xephi.authme.command.executable.authme.SwitchAntiBotCommand; -import fr.xephi.authme.command.executable.authme.UnregisterCommand; +import fr.xephi.authme.command.executable.authme.UnregisterAdminCommand; import fr.xephi.authme.command.executable.authme.VersionCommand; import fr.xephi.authme.command.executable.captcha.CaptchaCommand; +import fr.xephi.authme.command.executable.changepassword.ChangePasswordCommand; import fr.xephi.authme.command.executable.converter.ConverterCommand; import fr.xephi.authme.command.executable.email.AddEmailCommand; import fr.xephi.authme.command.executable.email.ChangeEmailCommand; import fr.xephi.authme.command.executable.email.RecoverEmailCommand; import fr.xephi.authme.command.executable.login.LoginCommand; import fr.xephi.authme.command.executable.logout.LogoutCommand; +import fr.xephi.authme.command.executable.register.RegisterCommand; +import fr.xephi.authme.command.executable.unregister.UnregisterCommand; import fr.xephi.authme.permission.AdminPermission; import fr.xephi.authme.permission.PlayerPermission; import fr.xephi.authme.util.Wrapper; @@ -87,8 +90,8 @@ public final class CommandInitializer { .detailedDescription("Register the specified player with the specified password.") .withArgument("player", "Player name", false) .withArgument("password", "Password", false) - .permissions(OP_ONLY, PlayerPermission.REGISTER) - .executableCommand(new RegisterCommand()) + .permissions(OP_ONLY, AdminPermission.REGISTER) + .executableCommand(new RegisterAdminCommand()) .build(); // Register the unregister command @@ -98,8 +101,8 @@ public final class CommandInitializer { .description("Unregister a player") .detailedDescription("Unregister the specified player.") .withArgument("player", "Player name", false) - .permissions(OP_ONLY, PlayerPermission.UNREGISTER) - .executableCommand(new UnregisterCommand()) + .permissions(OP_ONLY, AdminPermission.UNREGISTER) + .executableCommand(new UnregisterAdminCommand()) .build(); // Register the forcelogin command @@ -122,7 +125,7 @@ public final class CommandInitializer { .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) .permissions(OP_ONLY, AdminPermission.CHANGE_PASSWORD) - .executableCommand(new ChangePasswordCommand()) + .executableCommand(new ChangePasswordAdminCommand()) .build(); // Register the last login command @@ -316,7 +319,7 @@ public final class CommandInitializer { add("logout"); } }, "Logout command", "Command to logout using AuthMeReloaded.", null); - LOGOUT_BASE.setCommandPermissions(PlayerPermission.LOGOUT, CommandPermissions.DefaultPermission.ALLOWED); + LOGOUT_BASE.setCommandPermissions(PlayerPermission.LOGOUT, ALLOWED); // Register the help command CommandDescription logoutHelpCommand = new CommandDescription(helpCommandExecutable, helpCommandLabels, @@ -332,7 +335,7 @@ public final class CommandInitializer { .withArgument("password", "Password", false) .withArgument("verifyPassword", "Verify password", false) .permissions(ALLOWED, PlayerPermission.REGISTER) - .executableCommand(new fr.xephi.authme.command.executable.register.RegisterCommand()) + .executableCommand(new RegisterCommand()) .build(); // Register the help command @@ -341,13 +344,13 @@ public final class CommandInitializer { registerHelpCommand.addArgument(new CommandArgumentDescription("query", "The command or query to view help for.", true)); // Register the base unregister command - CommandDescription UNREGISTER_BASE = new CommandDescription(new fr.xephi.authme.command.executable.unregister.UnregisterCommand(), new ArrayList() { + CommandDescription UNREGISTER_BASE = new CommandDescription(new UnregisterCommand(), new ArrayList() { { add("unregister"); add("unreg"); } }, "Unregistration command", "Command to unregister using AuthMeReloaded.", null); - UNREGISTER_BASE.setCommandPermissions(PlayerPermission.UNREGISTER, CommandPermissions.DefaultPermission.ALLOWED); + UNREGISTER_BASE.setCommandPermissions(PlayerPermission.UNREGISTER, ALLOWED); UNREGISTER_BASE.addArgument(new CommandArgumentDescription("password", "Password", false)); // Register the help command @@ -356,13 +359,13 @@ public final class CommandInitializer { // Register the base changepassword command final CommandDescription CHANGE_PASSWORD_BASE = new CommandDescription( - new fr.xephi.authme.command.executable.changepassword.ChangePasswordCommand(), new ArrayList() { + new ChangePasswordCommand(), new ArrayList() { { add("changepassword"); add("changepass"); } }, "Change password command", "Command to change your password using AuthMeReloaded.", null); - CHANGE_PASSWORD_BASE.setCommandPermissions(PlayerPermission.CHANGE_PASSWORD, CommandPermissions.DefaultPermission.ALLOWED); + CHANGE_PASSWORD_BASE.setCommandPermissions(PlayerPermission.CHANGE_PASSWORD, ALLOWED); CHANGE_PASSWORD_BASE.addArgument(new CommandArgumentDescription("password", "Password", false)); CHANGE_PASSWORD_BASE.addArgument(new CommandArgumentDescription("verifyPassword", "Verify password", false)); @@ -392,7 +395,7 @@ public final class CommandInitializer { add("addmail"); } }, "Add E-mail", "Add an new E-Mail address to your account.", EMAIL_BASE); - addEmailCommand.setCommandPermissions(PlayerPermission.ADD_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); + addEmailCommand.setCommandPermissions(PlayerPermission.ADD_EMAIL, ALLOWED); addEmailCommand.addArgument(new CommandArgumentDescription("email", "Email address", false)); addEmailCommand.addArgument(new CommandArgumentDescription("verifyEmail", "Email address verification", false)); @@ -404,7 +407,7 @@ public final class CommandInitializer { add("changemail"); } }, "Change E-mail", "Change an E-Mail address of your account.", EMAIL_BASE); - changeEmailCommand.setCommandPermissions(PlayerPermission.CHANGE_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); + changeEmailCommand.setCommandPermissions(PlayerPermission.CHANGE_EMAIL, ALLOWED); changeEmailCommand.addArgument(new CommandArgumentDescription("oldEmail", "Old email address", false)); changeEmailCommand.addArgument(new CommandArgumentDescription("newEmail", "New email address", false)); @@ -417,7 +420,7 @@ public final class CommandInitializer { add("recovermail"); } }, "Recover using E-mail", "Recover your account using an E-mail address.", EMAIL_BASE); - recoverEmailCommand.setCommandPermissions(PlayerPermission.RECOVER_EMAIL, CommandPermissions.DefaultPermission.ALLOWED); + recoverEmailCommand.setCommandPermissions(PlayerPermission.RECOVER_EMAIL, ALLOWED); recoverEmailCommand.addArgument(new CommandArgumentDescription("email", "Email address", false)); // Register the base captcha command @@ -427,7 +430,7 @@ public final class CommandInitializer { add("capt"); } }, "Captcha command", "Captcha command for AuthMeReloaded.", null); - CAPTCHA_BASE.setCommandPermissions(PlayerPermission.CAPTCHA, CommandPermissions.DefaultPermission.ALLOWED); + CAPTCHA_BASE.setCommandPermissions(PlayerPermission.CAPTCHA, ALLOWED); CAPTCHA_BASE.addArgument(new CommandArgumentDescription("captcha", "The captcha", false)); // Register the help command diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java similarity index 98% rename from src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java rename to src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java index 06255128..d9889915 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordAdminCommand.java @@ -18,7 +18,7 @@ import java.security.NoSuchAlgorithmException; /** * Admin command for changing a player's password. */ -public class ChangePasswordCommand extends ExecutableCommand { +public class ChangePasswordAdminCommand extends ExecutableCommand { @Override public boolean executeCommand(final CommandSender sender, CommandParts commandReference, CommandParts commandArguments) { diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java similarity index 97% rename from src/main/java/fr/xephi/authme/command/executable/authme/RegisterCommand.java rename to src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java index 38e73d01..df204200 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/RegisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/RegisterAdminCommand.java @@ -15,8 +15,9 @@ import org.bukkit.command.CommandSender; import java.security.NoSuchAlgorithmException; /** + * Admin command to register a user. */ -public class RegisterCommand extends ExecutableCommand { +public class RegisterAdminCommand extends ExecutableCommand { /** * Execute the command. diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java similarity index 97% rename from src/main/java/fr/xephi/authme/command/executable/authme/UnregisterCommand.java rename to src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java index e5a8396f..8d2b1e11 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java @@ -21,8 +21,9 @@ import org.bukkit.scheduler.BukkitScheduler; import org.bukkit.scheduler.BukkitTask; /** + * Admin command to unregister a player. */ -public class UnregisterCommand extends ExecutableCommand { +public class UnregisterAdminCommand extends ExecutableCommand { /** * Execute the command. diff --git a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java index 51a8e3cd..ed063016 100644 --- a/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java +++ b/src/test/java/fr/xephi/authme/command/CommandInitializerTest.java @@ -1,5 +1,7 @@ package fr.xephi.authme.command; +import fr.xephi.authme.permission.AdminPermission; +import fr.xephi.authme.permission.PermissionNode; import fr.xephi.authme.util.StringUtils; import fr.xephi.authme.util.WrapperMock; import org.junit.BeforeClass; @@ -8,6 +10,7 @@ import org.junit.Test; import java.util.*; import java.util.regex.Pattern; +import static fr.xephi.authme.command.CommandPermissions.DefaultPermission.OP_ONLY; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -223,6 +226,42 @@ public class CommandInitializerTest { walkThroughCommands(commands, noArgumentForParentChecker); } + /** + * Test that commands defined with the OP_ONLY default permission have at least one admin permission node. + */ + @Test + public void shouldNotHavePlayerPermissionIfDefaultsToOpOnly() { + // given + BiConsumer adminPermissionChecker = new BiConsumer() { + // The only exception to this check is the force login command, which should default to OP_ONLY + // but semantically it is a player permission + final List forceLoginLabels = Arrays.asList("forcelogin", "login"); + + @Override + public void accept(CommandDescription command, int depth) { + CommandPermissions permissions = command.getCommandPermissions(); + if (permissions != null && OP_ONLY.equals(permissions.getDefaultPermission())) { + if (!hasAdminNode(permissions) && !command.getLabels().equals(forceLoginLabels)) { + 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; + } + }; + + // when/then + walkThroughCommands(commands, adminPermissionChecker); + } + // ------------ // Helper methods diff --git a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/register/RegisterAdminCommandTest.java similarity index 98% rename from src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java rename to src/test/java/fr/xephi/authme/command/executable/register/RegisterAdminCommandTest.java index 6dd19eb2..48ddec1a 100644 --- a/src/test/java/fr/xephi/authme/command/executable/register/RegisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/register/RegisterAdminCommandTest.java @@ -27,7 +27,7 @@ import static org.mockito.Mockito.verify; /** * Test for {@link RegisterCommand}. */ -public class RegisterCommandTest { +public class RegisterAdminCommandTest { private static Management managementMock; private static Messages messagesMock; diff --git a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java index acfe9028..3ca665f4 100644 --- a/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java +++ b/src/test/java/fr/xephi/authme/command/help/HelpSyntaxHelperTest.java @@ -3,7 +3,7 @@ package fr.xephi.authme.command.help; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; import fr.xephi.authme.command.ExecutableCommand; -import fr.xephi.authme.command.executable.authme.RegisterCommand; +import fr.xephi.authme.command.executable.authme.RegisterAdminCommand; import org.junit.Test; import org.mockito.Mockito; @@ -122,7 +122,7 @@ public class HelpSyntaxHelperTest { .build(); return CommandDescription.builder() - .executableCommand(Mockito.mock(RegisterCommand.class)) + .executableCommand(Mockito.mock(RegisterAdminCommand.class)) .labels("register", "r") .description("Register a player") .detailedDescription("Register the specified player with the specified password.")