From 1fbe4e0c3b312624197869533c11a7f28dafa9fa Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 3 Dec 2015 22:07:18 +0100 Subject: [PATCH] #298 Change password shows wrong 'pw cant be username' error - Change MessageKey to the proper message - Change permissions for admin changepassword to admin - Rename player changepassword command arguments to reflect their actual meaning --- .../xephi/authme/command/CommandManager.java | 2 +- .../authme/ChangePasswordCommand.java | 20 +++++++++++------- .../changepassword/ChangePasswordCommand.java | 14 ++++++------- .../ChangePasswordCommandTest.java | 21 +++++++++++-------- 4 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/main/java/fr/xephi/authme/command/CommandManager.java b/src/main/java/fr/xephi/authme/command/CommandManager.java index 1faa7f91..6c2f1c84 100644 --- a/src/main/java/fr/xephi/authme/command/CommandManager.java +++ b/src/main/java/fr/xephi/authme/command/CommandManager.java @@ -107,7 +107,7 @@ public class CommandManager { .description("Change a player's password") .detailedDescription("Change the password of a player.") .parent(authMeBaseCommand) - .permissions(OP_ONLY, PlayerPermission.CHANGE_PASSWORD) + .permissions(OP_ONLY, AdminPermission.CHANGE_PASSWORD) .withArgument("player", "Player name", false) .withArgument("pwd", "New password", false) .build(); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java index 1753a830..06255128 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java @@ -16,6 +16,7 @@ import org.bukkit.command.CommandSender; import java.security.NoSuchAlgorithmException; /** + * Admin command for changing a player's password. */ public class ChangePasswordCommand extends ExecutableCommand { @@ -31,23 +32,26 @@ public class ChangePasswordCommand extends ExecutableCommand { // Validate the password String playerPassLowerCase = playerPass.toLowerCase(); - if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where") || playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify") || playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select") || playerPassLowerCase.contains(";") || playerPassLowerCase.contains("null") || !playerPassLowerCase.matches(Settings.getPassRegex)) { - m.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); + if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where") + || playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify") + || playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select") + || playerPassLowerCase.contains(";") || playerPassLowerCase.contains("null") + || !playerPassLowerCase.matches(Settings.getPassRegex)) { + m.send(sender, MessageKey.PASSWORD_MATCH_ERROR); return true; } if (playerPassLowerCase.equalsIgnoreCase(playerName)) { m.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); return true; } - if (playerPassLowerCase.length() < Settings.getPasswordMinLen || playerPassLowerCase.length() > Settings.passwordMaxLength) { + if (playerPassLowerCase.length() < Settings.getPasswordMinLen + || playerPassLowerCase.length() > Settings.passwordMaxLength) { m.send(sender, MessageKey.INVALID_PASSWORD_LENGTH); return true; } - if (!Settings.unsafePasswords.isEmpty()) { - if (Settings.unsafePasswords.contains(playerPassLowerCase)) { - m.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); - return true; - } + if (!Settings.unsafePasswords.isEmpty() && Settings.unsafePasswords.contains(playerPassLowerCase)) { + m.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); + return true; } // Set the password final String playerNameLowerCase = playerName.toLowerCase(); diff --git a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java index c88ceaa8..f9219aba 100644 --- a/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java @@ -13,6 +13,7 @@ import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; /** + * The command for a player to change his password with. */ public class ChangePasswordCommand extends ExecutableCommand { @@ -27,10 +28,10 @@ public class ChangePasswordCommand extends ExecutableCommand { final Messages m = wrapper.getMessages(); // Get the passwords - String playerPass = commandArguments.get(0); - String playerPassVerify = commandArguments.get(1); + String oldPassword = commandArguments.get(0); + String newPassword = commandArguments.get(1); - // Get the player instance and make sure it's authenticated + // Get the player instance and make sure he's authenticated Player player = (Player) sender; String name = player.getName().toLowerCase(); final PlayerCache playerCache = wrapper.getPlayerCache(); @@ -40,8 +41,7 @@ public class ChangePasswordCommand extends ExecutableCommand { } // Make sure the password is allowed - // TODO ljacqu 20151121: The password confirmation appears to be never verified - String playerPassLowerCase = playerPass.toLowerCase(); + String playerPassLowerCase = newPassword.toLowerCase(); if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where") || playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify") || playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select") @@ -66,8 +66,8 @@ public class ChangePasswordCommand extends ExecutableCommand { // Set the password final AuthMe plugin = wrapper.getAuthMe(); - wrapper.getServer().getScheduler().runTaskAsynchronously(plugin, - new ChangePasswordTask(plugin, player, playerPass, playerPassVerify)); + wrapper.getScheduler().runTaskAsynchronously(plugin, + new ChangePasswordTask(plugin, player, oldPassword, newPassword)); return true; } } diff --git a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java index 87f113cc..a81611a5 100644 --- a/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java @@ -18,6 +18,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import java.util.Arrays; import java.util.Collections; import static java.util.Arrays.asList; @@ -85,7 +86,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("!pass")); + command.executeCommand(sender, new CommandParts(), newParts("old123", "!pass")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR); @@ -100,7 +101,7 @@ public class ChangePasswordCommandTest { ChangePasswordCommand command = new ChangePasswordCommand(); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("Tester")); + command.executeCommand(sender, new CommandParts(), newParts("old_", "Tester")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); @@ -115,7 +116,7 @@ public class ChangePasswordCommandTest { Settings.passwordMaxLength = 3; // when - command.executeCommand(sender, new CommandParts(), new CommandParts("test")); + command.executeCommand(sender, new CommandParts(), newParts("12", "test")); // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); @@ -130,7 +131,7 @@ public class ChangePasswordCommandTest { Settings.getPasswordMinLen = 7; // when - command.executeCommand(sender, new CommandParts(), new CommandParts("tester")); + command.executeCommand(sender, new CommandParts(), newParts("oldverylongpassword", "tester")); // then verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); @@ -145,7 +146,7 @@ public class ChangePasswordCommandTest { Settings.unsafePasswords = asList("test", "abc123"); // when - command.executeCommand(sender, new CommandParts(), new CommandParts("abc123")); + command.executeCommand(sender, new CommandParts(), newParts("oldpw", "abc123")); // then verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); @@ -157,16 +158,14 @@ public class ChangePasswordCommandTest { // given CommandSender sender = initPlayerWithName("parker", true); ChangePasswordCommand command = new ChangePasswordCommand(); - BukkitScheduler schedulerMock = mock(BukkitScheduler.class); - given(wrapperMock.getServer().getScheduler()).willReturn(schedulerMock); // when - command.executeCommand(sender, new CommandParts(), new CommandParts(asList("abc123", "abc123"))); + command.executeCommand(sender, new CommandParts(), newParts("abc123", "abc123")); // then verify(messagesMock, never()).send(eq(sender), any(MessageKey.class)); ArgumentCaptor taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class); - verify(schedulerMock).runTaskAsynchronously(any(AuthMe.class), taskCaptor.capture()); + verify(wrapperMock.getScheduler()).runTaskAsynchronously(any(AuthMe.class), taskCaptor.capture()); ChangePasswordTask task = taskCaptor.getValue(); assertThat((String) ReflectionTestUtils.getFieldValue(ChangePasswordTask.class, task, "newPassword"), equalTo("abc123")); @@ -179,4 +178,8 @@ public class ChangePasswordCommandTest { return player; } + private static CommandParts newParts(String... parts) { + return new CommandParts(Arrays.asList(parts)); + } + }