diff --git a/src/main/java/fr/xephi/authme/api/NewAPI.java b/src/main/java/fr/xephi/authme/api/NewAPI.java index dea31c90..6b87cb53 100644 --- a/src/main/java/fr/xephi/authme/api/NewAPI.java +++ b/src/main/java/fr/xephi/authme/api/NewAPI.java @@ -216,6 +216,6 @@ public class NewAPI { * @param player The player to unregister */ public void forceUnregister(Player player) { - management.performUnregister(player, "", true); + management.performUnregisterByAdmin(null, player.getName(), player); } } diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java index 5560c694..eaf14b6b 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommand.java @@ -1,13 +1,9 @@ package fr.xephi.authme.command.executable.authme; -import fr.xephi.authme.ConsoleLogger; -import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.command.ExecutableCommand; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; -import fr.xephi.authme.permission.AuthGroupHandler; -import fr.xephi.authme.permission.AuthGroupType; import fr.xephi.authme.process.Management; import fr.xephi.authme.util.BukkitService; import org.bukkit.command.CommandSender; @@ -27,47 +23,27 @@ public class UnregisterAdminCommand implements ExecutableCommand { @Inject private CommandService commandService; - @Inject - private PlayerCache playerCache; - @Inject private BukkitService bukkitService; - @Inject - private AuthGroupHandler authGroupHandler; - @Inject private Management management; + UnregisterAdminCommand() { + } @Override public void executeCommand(final CommandSender sender, List arguments) { - // Get the player name String playerName = arguments.get(0); - String playerNameLowerCase = playerName.toLowerCase(); - // Make sure the user is valid - if (!dataSource.isAuthAvailable(playerNameLowerCase)) { + // Make sure the user exists + if (!dataSource.isAuthAvailable(playerName)) { commandService.send(sender, MessageKey.UNKNOWN_USER); return; } - // Remove the player - if (!dataSource.removeAuth(playerNameLowerCase)) { - commandService.send(sender, MessageKey.ERROR); - return; - } - - // Unregister the player - Player target = bukkitService.getPlayerExact(playerNameLowerCase); - playerCache.removePlayer(playerNameLowerCase); - authGroupHandler.setGroup(target, AuthGroupType.UNREGISTERED); - if (target != null && target.isOnline()) { - management.performUnregister(target, "dontneed", true); - } - - // Show a status message - commandService.send(sender, MessageKey.UNREGISTERED_SUCCESS); - ConsoleLogger.info(sender.getName() + " unregistered " + playerName); + // Get the player from the server and perform unregister + Player target = bukkitService.getPlayerExact(playerName); + management.performUnregisterByAdmin(sender, playerName, target); } } diff --git a/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java b/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java index 720b136e..8526e4a1 100644 --- a/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/unregister/UnregisterCommand.java @@ -10,6 +10,9 @@ import org.bukkit.entity.Player; import javax.inject.Inject; import java.util.List; +/** + * Command for a player to unregister himself. + */ public class UnregisterCommand extends PlayerCommand { @Inject @@ -24,15 +27,15 @@ public class UnregisterCommand extends PlayerCommand { @Override public void runCommand(Player player, List arguments) { String playerPass = arguments.get(0); - final String playerNameLowerCase = player.getName().toLowerCase(); + final String playerName = player.getName(); // Make sure the player is authenticated - if (!playerCache.isAuthenticated(playerNameLowerCase)) { + if (!playerCache.isAuthenticated(playerName)) { commandService.send(player, MessageKey.NOT_LOGGED_IN); return; } // Unregister the player - management.performUnregister(player, playerPass, false); + management.performUnregister(player, playerPass); } } diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index d229edfb..63d96b97 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -10,6 +10,7 @@ import fr.xephi.authme.process.quit.AsynchronousQuit; import fr.xephi.authme.process.register.AsyncRegister; import fr.xephi.authme.process.unregister.AsynchronousUnregister; import fr.xephi.authme.util.BukkitService; +import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import javax.inject.Inject; @@ -70,11 +71,20 @@ public class Management { }); } - public void performUnregister(final Player player, final String password, final boolean isForce) { + public void performUnregister(final Player player, final String password) { runTask(new Runnable() { @Override public void run() { - asynchronousUnregister.unregister(player, password, isForce); + asynchronousUnregister.unregister(player, password); + } + }); + } + + public void performUnregisterByAdmin(final CommandSender initiator, final String name, final Player player) { + runTask(new Runnable() { + @Override + public void run() { + asynchronousUnregister.adminUnregister(initiator, name, player); } }); } diff --git a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java index 7515a0d3..82fea4db 100644 --- a/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java +++ b/src/main/java/fr/xephi/authme/process/unregister/AsynchronousUnregister.java @@ -6,16 +6,17 @@ import fr.xephi.authme.cache.auth.PlayerCache; import fr.xephi.authme.cache.limbo.LimboCache; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.permission.AuthGroupHandler; import fr.xephi.authme.permission.AuthGroupType; import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.ProcessService; import fr.xephi.authme.security.PasswordSecurity; -import fr.xephi.authme.settings.properties.HooksSettings; import fr.xephi.authme.settings.properties.RegistrationSettings; import fr.xephi.authme.settings.properties.RestrictionSettings; import fr.xephi.authme.task.PlayerDataTaskManager; import fr.xephi.authme.util.BukkitService; import fr.xephi.authme.util.TeleportationService; +import org.bukkit.command.CommandSender; import org.bukkit.entity.Player; import org.bukkit.potion.PotionEffect; import org.bukkit.potion.PotionEffectType; @@ -39,7 +40,7 @@ public class AsynchronousUnregister implements AsynchronousProcess { private PlayerCache playerCache; @Inject - private BukkitService bukktiService; + private BukkitService bukkitService; @Inject private LimboCache limboCache; @@ -50,51 +51,79 @@ public class AsynchronousUnregister implements AsynchronousProcess { @Inject private TeleportationService teleportationService; + @Inject + private AuthGroupHandler authGroupHandler; + AsynchronousUnregister() { } - - public void unregister(Player player, String password, boolean force) { - final String name = player.getName().toLowerCase(); - PlayerAuth cachedAuth = playerCache.getAuth(name); - if (force || passwordSecurity.comparePassword(password, cachedAuth.getPassword(), player.getName())) { - if (!dataSource.removeAuth(name)) { + /** + * Processes a player's request to unregister himself. Unregisters the player after + * successful password check. + * + * @param player the player + * @param password the input password to check before unregister + */ + public void unregister(Player player, String password) { + final String name = player.getName(); + final PlayerAuth cachedAuth = playerCache.getAuth(name); + if (passwordSecurity.comparePassword(password, cachedAuth.getPassword(), name)) { + if (dataSource.removeAuth(name)) { + performUnregister(name, player); + ConsoleLogger.info(name + " unregistered himself"); + } else { service.send(player, MessageKey.ERROR); - return; } - - if (service.getProperty(RegistrationSettings.FORCE)) { - teleportationService.teleportOnJoin(player); - player.saveData(); - playerCache.removePlayer(player.getName().toLowerCase()); - if (!service.getProperty(HooksSettings.REGISTERED_GROUP).isEmpty()) { - service.setGroup(player, AuthGroupType.UNREGISTERED); - } - limboCache.deletePlayerData(player); - limboCache.addPlayerData(player); - playerDataTaskManager.registerTimeoutTask(player); - playerDataTaskManager.registerMessageTask(name, false); - - service.send(player, MessageKey.UNREGISTERED_SUCCESS); - applyBlindEffect(player); - ConsoleLogger.info(player.getName() + " unregistered himself"); - return; // TODO ljacqu 20160612: Why return here? No blind effect? Player not removed from PlayerCache? - } - if (!service.getProperty(HooksSettings.UNREGISTERED_GROUP).isEmpty()) { - service.setGroup(player, AuthGroupType.UNREGISTERED); - } - playerCache.removePlayer(name); - - service.send(player, MessageKey.UNREGISTERED_SUCCESS); - ConsoleLogger.info(player.getName() + " unregistered himself"); } else { service.send(player, MessageKey.WRONG_PASSWORD); } } + /** + * Unregisters a player. + * + * @param initiator the initiator of this process (nullable) + * @param name the name of the player + * @param player the according Player object (nullable) + */ + // We need to have the name and the player separate because Player might be null in this case: + // we might have some player in the database that has never been online on the server + public void adminUnregister(CommandSender initiator, String name, Player player) { + if (dataSource.removeAuth(name)) { + performUnregister(name, player); + if (initiator == null) { + ConsoleLogger.info(name + " was unregistered"); + } else { + ConsoleLogger.info(name + " was unregistered by " + initiator.getName()); + service.send(initiator, MessageKey.UNREGISTERED_SUCCESS); + } + } + } + + private void performUnregister(String name, Player player) { + playerCache.removePlayer(name); + if (player == null || !player.isOnline()) { + return; + } + + if (service.getProperty(RegistrationSettings.FORCE)) { + teleportationService.teleportOnJoin(player); + player.saveData(); + + limboCache.deletePlayerData(player); + limboCache.addPlayerData(player); + + playerDataTaskManager.registerTimeoutTask(player); + playerDataTaskManager.registerMessageTask(name, false); + applyBlindEffect(player); + } + authGroupHandler.setGroup(player, AuthGroupType.UNREGISTERED); + service.send(player, MessageKey.UNREGISTERED_SUCCESS); + } + private void applyBlindEffect(final Player player) { if (service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)) { final int timeout = service.getProperty(RestrictionSettings.TIMEOUT) * TICKS_PER_SECOND; - bukktiService.runTask(new Runnable() { + bukkitService.runTask(new Runnable() { @Override public void run() { player.addPotionEffect(new PotionEffect(PotionEffectType.BLINDNESS, timeout, 2)); diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommandTest.java index f41eeb20..aea73ed7 100644 --- a/src/test/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/authme/UnregisterAdminCommandTest.java @@ -3,7 +3,10 @@ package fr.xephi.authme.command.executable.authme; import fr.xephi.authme.command.CommandService; import fr.xephi.authme.datasource.DataSource; import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.process.Management; +import fr.xephi.authme.util.BukkitService; import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -12,12 +15,10 @@ import org.mockito.runners.MockitoJUnitRunner; import java.util.Collections; -import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.mockito.BDDMockito.given; -import static org.mockito.Matchers.argThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; /** * Test for {@link UnregisterAdminCommand}. @@ -34,6 +35,12 @@ public class UnregisterAdminCommandTest { @Mock private CommandService commandService; + @Mock + private BukkitService bukkitService; + + @Mock + private Management management; + @Test public void shouldHandleUnknownPlayer() { // given @@ -45,27 +52,45 @@ public class UnregisterAdminCommandTest { command.executeCommand(sender, Collections.singletonList(user)); // then - verify(dataSource).isAuthAvailable(user); - verifyNoMoreInteractions(dataSource); + verify(dataSource, only()).isAuthAvailable(user); verify(commandService).send(sender, MessageKey.UNKNOWN_USER); } @Test - public void shouldHandleDatabaseError() { + public void shouldInvokeUnregisterProcess() { // given String user = "personaNonGrata"; - given(dataSource.isAuthAvailable(argThat(equalToIgnoringCase(user)))).willReturn(true); - given(dataSource.removeAuth(argThat(equalToIgnoringCase(user)))).willReturn(false); + given(dataSource.isAuthAvailable(user)).willReturn(true); + given(dataSource.removeAuth(user)).willReturn(false); + Player player = mock(Player.class); + given(bukkitService.getPlayerExact(user)).willReturn(player); CommandSender sender = mock(CommandSender.class); // when command.executeCommand(sender, Collections.singletonList(user)); // then - verify(dataSource).isAuthAvailable(argThat(equalToIgnoringCase(user))); - verify(dataSource).removeAuth(argThat(equalToIgnoringCase(user))); - verifyNoMoreInteractions(dataSource); - verify(commandService).send(sender, MessageKey.ERROR); + verify(dataSource, only()).isAuthAvailable(user); + verify(bukkitService).getPlayerExact(user); + verify(management).performUnregisterByAdmin(sender, user, player); + } + + @Test + public void shouldInvokeUnregisterProcessWithNullPlayer() { + // given + String user = "personaNonGrata"; + given(dataSource.isAuthAvailable(user)).willReturn(true); + given(dataSource.removeAuth(user)).willReturn(false); + given(bukkitService.getPlayerExact(user)).willReturn(null); + CommandSender sender = mock(CommandSender.class); + + // when + command.executeCommand(sender, Collections.singletonList(user)); + + // then + verify(dataSource, only()).isAuthAvailable(user); + verify(bukkitService).getPlayerExact(user); + verify(management).performUnregisterByAdmin(sender, user, null); } } diff --git a/src/test/java/fr/xephi/authme/command/executable/unregister/UnregisterCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/unregister/UnregisterCommandTest.java index 976649ae..33bbf4c6 100644 --- a/src/test/java/fr/xephi/authme/command/executable/unregister/UnregisterCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/unregister/UnregisterCommandTest.java @@ -68,7 +68,7 @@ public class UnregisterCommandTest { // then verify(playerCache).isAuthenticated(name); - verify(management).performUnregister(player, password, false); + verify(management).performUnregister(player, password); } } diff --git a/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java b/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java new file mode 100644 index 00000000..02035e32 --- /dev/null +++ b/src/test/java/fr/xephi/authme/process/unregister/AsynchronousUnregisterTest.java @@ -0,0 +1,121 @@ +package fr.xephi.authme.process.unregister; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.cache.auth.PlayerAuth; +import fr.xephi.authme.cache.auth.PlayerCache; +import fr.xephi.authme.cache.limbo.LimboCache; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.output.MessageKey; +import fr.xephi.authme.permission.AuthGroupHandler; +import fr.xephi.authme.permission.AuthGroupType; +import fr.xephi.authme.process.ProcessService; +import fr.xephi.authme.security.PasswordSecurity; +import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.settings.properties.RegistrationSettings; +import fr.xephi.authme.settings.properties.RestrictionSettings; +import fr.xephi.authme.task.PlayerDataTaskManager; +import fr.xephi.authme.util.BukkitService; +import fr.xephi.authme.util.TeleportationService; +import org.bukkit.entity.Player; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.only; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; + +/** + * Test for {@link AsynchronousUnregister}. + */ +@RunWith(MockitoJUnitRunner.class) +public class AsynchronousUnregisterTest { + + @InjectMocks + private AsynchronousUnregister asynchronousUnregister; + + @Mock + private DataSource dataSource; + @Mock + private ProcessService service; + @Mock + private PasswordSecurity passwordSecurity; + @Mock + private PlayerCache playerCache; + @Mock + private BukkitService bukkitService; + @Mock + private LimboCache limboCache; + @Mock + private PlayerDataTaskManager playerDataTaskManager; + @Mock + private TeleportationService teleportationService; + @Mock + private AuthGroupHandler authGroupHandler; + + @BeforeClass + public static void initLogger() { + TestHelper.setupLogger(); + } + + @Test + public void shouldRejectWrongPassword() { + // given + Player player = mock(Player.class); + String name = "Bobby"; + given(player.getName()).willReturn(name); + PlayerAuth auth = mock(PlayerAuth.class); + given(playerCache.getAuth(name)).willReturn(auth); + HashedPassword password = new HashedPassword("password", "in_auth_obj"); + given(auth.getPassword()).willReturn(password); + String userPassword = "pass"; + given(passwordSecurity.comparePassword(userPassword, password, name)).willReturn(false); + + // when + asynchronousUnregister.unregister(player, userPassword); + + // then + verify(service).send(player, MessageKey.WRONG_PASSWORD); + verify(passwordSecurity).comparePassword(userPassword, password, name); + verifyZeroInteractions(dataSource, playerDataTaskManager, limboCache, authGroupHandler, teleportationService); + verify(player, only()).getName(); + } + + @Test + public void shouldPerformUnregister() { + // given + Player player = mock(Player.class); + String name = "Frank21"; + given(player.getName()).willReturn(name); + given(player.isOnline()).willReturn(true); + PlayerAuth auth = mock(PlayerAuth.class); + given(playerCache.getAuth(name)).willReturn(auth); + HashedPassword password = new HashedPassword("password", "in_auth_obj"); + given(auth.getPassword()).willReturn(password); + String userPassword = "pass"; + given(passwordSecurity.comparePassword(userPassword, password, name)).willReturn(true); + given(dataSource.removeAuth(name)).willReturn(true); + given(service.getProperty(RegistrationSettings.FORCE)).willReturn(true); + given(service.getProperty(RegistrationSettings.APPLY_BLIND_EFFECT)).willReturn(true); + given(service.getProperty(RestrictionSettings.TIMEOUT)).willReturn(12); + + // when + asynchronousUnregister.unregister(player, userPassword); + + // then + verify(service).send(player, MessageKey.UNREGISTERED_SUCCESS); + verify(passwordSecurity).comparePassword(userPassword, password, name); + verify(dataSource).removeAuth(name); + verify(playerCache).removePlayer(name); + verify(teleportationService).teleportOnJoin(player); + verify(authGroupHandler).setGroup(player, AuthGroupType.UNREGISTERED); + verify(bukkitService).runTask(any(Runnable.class)); + } + +}