Meishin/LoginSystem
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

#298 Change password shows wrong 'pw cant be username' error

Browse Source 
- Change MessageKey to the proper message
- Change permissions for admin changepassword to admin
- Rename player changepassword command arguments to reflect their actual meaning
This commit is contained in:
ljacqu 2015-12-03 22:07:18 +01:00
parent cafad5b088
commit 1fbe4e0c3b
4 changed files with 32 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/fr/xephi/authme/command/CommandManager.java
View File

@ -107,7 +107,7 @@ public class CommandManager {
.description("Change a player's password") .description("Change a player's password")
.detailedDescription("Change the password of a player.") .detailedDescription("Change the password of a player.")
.parent(authMeBaseCommand) .parent(authMeBaseCommand)
.permissions(OP_ONLY, PlayerPermission.CHANGE_PASSWORD) .permissions(OP_ONLY, AdminPermission.CHANGE_PASSWORD)
.withArgument("player", "Player name", false) .withArgument("player", "Player name", false)
.withArgument("pwd", "New password", false) .withArgument("pwd", "New password", false)
.build(); .build();

20
src/main/java/fr/xephi/authme/command/executable/authme/ChangePasswordCommand.java
View File

@ -16,6 +16,7 @@ import org.bukkit.command.CommandSender;
import java.security.NoSuchAlgorithmException; import java.security.NoSuchAlgorithmException;
/** /**
* Admin command for changing a player's password.
*/ */
public class ChangePasswordCommand extends ExecutableCommand { public class ChangePasswordCommand extends ExecutableCommand {
@ -31,23 +32,26 @@ public class ChangePasswordCommand extends ExecutableCommand {
// Validate the password // Validate the password
String playerPassLowerCase = playerPass.toLowerCase(); 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)) { if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where")
m.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); || 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; return true;
} }
if (playerPassLowerCase.equalsIgnoreCase(playerName)) { if (playerPassLowerCase.equalsIgnoreCase(playerName)) {
m.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); m.send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR);
return true; 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); m.send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
return true; return true;
} }
if (!Settings.unsafePasswords.isEmpty()) { if (!Settings.unsafePasswords.isEmpty() && Settings.unsafePasswords.contains(playerPassLowerCase)) {
if (Settings.unsafePasswords.contains(playerPassLowerCase)) { m.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
m.send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); return true;
return true;
}
} }
// Set the password // Set the password
final String playerNameLowerCase = playerName.toLowerCase(); final String playerNameLowerCase = playerName.toLowerCase();

14
src/main/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommand.java
View File

@ -13,6 +13,7 @@ import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player; import org.bukkit.entity.Player;
/** /**
* The command for a player to change his password with.
*/ */
public class ChangePasswordCommand extends ExecutableCommand { public class ChangePasswordCommand extends ExecutableCommand {
@ -27,10 +28,10 @@ public class ChangePasswordCommand extends ExecutableCommand {
final Messages m = wrapper.getMessages(); final Messages m = wrapper.getMessages();
// Get the passwords // Get the passwords
String playerPass = commandArguments.get(0); String oldPassword = commandArguments.get(0);
String playerPassVerify = commandArguments.get(1); 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; Player player = (Player) sender;
String name = player.getName().toLowerCase(); String name = player.getName().toLowerCase();
final PlayerCache playerCache = wrapper.getPlayerCache(); final PlayerCache playerCache = wrapper.getPlayerCache();
@ -40,8 +41,7 @@ public class ChangePasswordCommand extends ExecutableCommand {
} }
// Make sure the password is allowed // Make sure the password is allowed
// TODO ljacqu 20151121: The password confirmation appears to be never verified String playerPassLowerCase = newPassword.toLowerCase();
String playerPassLowerCase = playerPass.toLowerCase();
if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where") if (playerPassLowerCase.contains("delete") || playerPassLowerCase.contains("where")
|| playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify") || playerPassLowerCase.contains("insert") || playerPassLowerCase.contains("modify")
|| playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select") || playerPassLowerCase.contains("from") || playerPassLowerCase.contains("select")
@ -66,8 +66,8 @@ public class ChangePasswordCommand extends ExecutableCommand {
// Set the password // Set the password
final AuthMe plugin = wrapper.getAuthMe(); final AuthMe plugin = wrapper.getAuthMe();
wrapper.getServer().getScheduler().runTaskAsynchronously(plugin, wrapper.getScheduler().runTaskAsynchronously(plugin,
new ChangePasswordTask(plugin, player, playerPass, playerPassVerify)); new ChangePasswordTask(plugin, player, oldPassword, newPassword));
return true; return true;
} }
} }

21
src/test/java/fr/xephi/authme/command/executable/changepassword/ChangePasswordCommandTest.java
View File

@ -18,6 +18,7 @@ import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.mockito.ArgumentCaptor; import org.mockito.ArgumentCaptor;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import static java.util.Arrays.asList; import static java.util.Arrays.asList;
@ -85,7 +86,7 @@ public class ChangePasswordCommandTest {
ChangePasswordCommand command = new ChangePasswordCommand(); ChangePasswordCommand command = new ChangePasswordCommand();
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts("!pass")); command.executeCommand(sender, new CommandParts(), newParts("old123", "!pass"));
// then // then
verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR); verify(messagesMock).send(sender, MessageKey.PASSWORD_MATCH_ERROR);
@ -100,7 +101,7 @@ public class ChangePasswordCommandTest {
ChangePasswordCommand command = new ChangePasswordCommand(); ChangePasswordCommand command = new ChangePasswordCommand();
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts("Tester")); command.executeCommand(sender, new CommandParts(), newParts("old_", "Tester"));
// then // then
verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR); verify(messagesMock).send(sender, MessageKey.PASSWORD_IS_USERNAME_ERROR);
@ -115,7 +116,7 @@ public class ChangePasswordCommandTest {
Settings.passwordMaxLength = 3; Settings.passwordMaxLength = 3;
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts("test")); command.executeCommand(sender, new CommandParts(), newParts("12", "test"));
// then // then
verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
@ -130,7 +131,7 @@ public class ChangePasswordCommandTest {
Settings.getPasswordMinLen = 7; Settings.getPasswordMinLen = 7;
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts("tester")); command.executeCommand(sender, new CommandParts(), newParts("oldverylongpassword", "tester"));
// then // then
verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH); verify(messagesMock).send(sender, MessageKey.INVALID_PASSWORD_LENGTH);
@ -145,7 +146,7 @@ public class ChangePasswordCommandTest {
Settings.unsafePasswords = asList("test", "abc123"); Settings.unsafePasswords = asList("test", "abc123");
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts("abc123")); command.executeCommand(sender, new CommandParts(), newParts("oldpw", "abc123"));
// then // then
verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR); verify(messagesMock).send(sender, MessageKey.PASSWORD_UNSAFE_ERROR);
@ -157,16 +158,14 @@ public class ChangePasswordCommandTest {
// given // given
CommandSender sender = initPlayerWithName("parker", true); CommandSender sender = initPlayerWithName("parker", true);
ChangePasswordCommand command = new ChangePasswordCommand(); ChangePasswordCommand command = new ChangePasswordCommand();
BukkitScheduler schedulerMock = mock(BukkitScheduler.class);
given(wrapperMock.getServer().getScheduler()).willReturn(schedulerMock);
// when // when
command.executeCommand(sender, new CommandParts(), new CommandParts(asList("abc123", "abc123"))); command.executeCommand(sender, new CommandParts(), newParts("abc123", "abc123"));
// then // then
verify(messagesMock, never()).send(eq(sender), any(MessageKey.class)); verify(messagesMock, never()).send(eq(sender), any(MessageKey.class));
ArgumentCaptor<ChangePasswordTask> taskCaptor = ArgumentCaptor.forClass(ChangePasswordTask.class); ArgumentCaptor<ChangePasswordTask> 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(); ChangePasswordTask task = taskCaptor.getValue();
assertThat((String) ReflectionTestUtils.getFieldValue(ChangePasswordTask.class, task, "newPassword"), assertThat((String) ReflectionTestUtils.getFieldValue(ChangePasswordTask.class, task, "newPassword"),
equalTo("abc123")); equalTo("abc123"));
@ -179,4 +178,8 @@ public class ChangePasswordCommandTest {
return player; return player;
} }
private static CommandParts newParts(String... parts) {
return new CommandParts(Arrays.asList(parts));
}
} }