From e72d5d5e819f496c39e41af8b9080c64f376e1c1 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 9 Mar 2018 18:37:01 +0100 Subject: [PATCH] #1141 Require TOTP code to be passed with /login (temporary) - Temporarily require the TOTP code to be provided with /login - Future implementation should require it as a second step --- .../authme/command/CommandInitializer.java | 1 + .../authme/debug/PlayerAuthViewer.java | 1 + .../executable/login/LoginCommand.java | 5 +- .../fr/xephi/authme/process/Management.java | 4 +- .../process/login/AsynchronousLogin.java | 23 +++- .../authme/debug/PlayerAuthViewerTest.java | 106 ++++++++++++++++++ .../executable/login/LoginCommandTest.java | 16 ++- 7 files changed, 147 insertions(+), 9 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewerTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandInitializer.java b/src/main/java/fr/xephi/authme/command/CommandInitializer.java index 7f0d33e7..dd3cc37b 100644 --- a/src/main/java/fr/xephi/authme/command/CommandInitializer.java +++ b/src/main/java/fr/xephi/authme/command/CommandInitializer.java @@ -89,6 +89,7 @@ public class CommandInitializer { .description("Login command") .detailedDescription("Command to log in using AuthMeReloaded.") .withArgument("password", "Login password", false) + .withArgument("2facode", "TOTP code", true) .permission(PlayerPermission.LOGIN) .executableCommand(LoginCommand.class) .register(); diff --git a/src/main/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewer.java b/src/main/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewer.java index 07e160a4..ae314e09 100644 --- a/src/main/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewer.java +++ b/src/main/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewer.java @@ -77,6 +77,7 @@ class PlayerAuthViewer implements DebugSection { HashedPassword hashedPass = auth.getPassword(); sender.sendMessage("Hash / salt (partial): '" + safeSubstring(hashedPass.getHash(), 6) + "' / '" + safeSubstring(hashedPass.getSalt(), 4) + "'"); + sender.sendMessage("TOTP code (partial): '" + safeSubstring(auth.getTotpKey(), 3) + "'"); } /** diff --git a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java index 32a4d87d..e2878271 100644 --- a/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java +++ b/src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java @@ -18,8 +18,9 @@ public class LoginCommand extends PlayerCommand { @Override public void runCommand(Player player, List arguments) { - final String password = arguments.get(0); - management.performLogin(player, password); + String password = arguments.get(0); + String totpCode = arguments.size() > 1 ? arguments.get(1) : null; + management.performLogin(player, password, totpCode); } @Override diff --git a/src/main/java/fr/xephi/authme/process/Management.java b/src/main/java/fr/xephi/authme/process/Management.java index 5260fed1..a5a879e0 100644 --- a/src/main/java/fr/xephi/authme/process/Management.java +++ b/src/main/java/fr/xephi/authme/process/Management.java @@ -49,8 +49,8 @@ public class Management { } - public void performLogin(Player player, String password) { - runTask(() -> asynchronousLogin.login(player, password)); + public void performLogin(Player player, String password, String totpCode) { + runTask(() -> asynchronousLogin.login(player, password, totpCode)); } public void forceLogin(Player player) { diff --git a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java index c52202ef..6cadf820 100644 --- a/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java +++ b/src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java @@ -18,6 +18,7 @@ import fr.xephi.authme.permission.PlayerStatePermission; import fr.xephi.authme.process.AsynchronousProcess; import fr.xephi.authme.process.SyncProcessManager; import fr.xephi.authme.security.PasswordSecurity; +import fr.xephi.authme.security.TotpService; import fr.xephi.authme.service.BukkitService; import fr.xephi.authme.service.CommonService; import fr.xephi.authme.service.SessionService; @@ -78,6 +79,9 @@ public class AsynchronousLogin implements AsynchronousProcess { @Inject private BungeeSender bungeeSender; + @Inject + private TotpService totpService; + AsynchronousLogin() { } @@ -86,10 +90,11 @@ public class AsynchronousLogin implements AsynchronousProcess { * * @param player the player to log in * @param password the password to log in with + * @param totpCode the totp code (nullable) */ - public void login(Player player, String password) { + public void login(Player player, String password, String totpCode) { PlayerAuth auth = getPlayerAuth(player); - if (auth != null && checkPlayerInfo(player, auth, password)) { + if (auth != null && checkPlayerInfo(player, auth, password, totpCode)) { performLogin(player, auth); } } @@ -156,10 +161,11 @@ public class AsynchronousLogin implements AsynchronousProcess { * @param player the player requesting to log in * @param auth the PlayerAuth object of the player * @param password the password supplied by the player + * @param totpCode the input totp code (nullable) * @return true if the password matches and all other conditions are met (e.g. no captcha required), * false otherwise */ - private boolean checkPlayerInfo(Player player, PlayerAuth auth, String password) { + private boolean checkPlayerInfo(Player player, PlayerAuth auth, String password, String totpCode) { final String name = player.getName().toLowerCase(); // If captcha is required send a message to the player and deny to log in @@ -174,6 +180,17 @@ public class AsynchronousLogin implements AsynchronousProcess { loginCaptchaManager.increaseLoginFailureCount(name); tempbanManager.increaseCount(ip, name); + if (auth.getTotpKey() != null) { + if (totpCode == null) { + player.sendMessage( + "You have two-factor authentication enabled. Please provide it: /login <2faCode>"); + return false; + } else if (!totpService.verifyCode(auth, totpCode)) { + player.sendMessage("Invalid code for two-factor authentication. Please try again"); + return false; + } + } + if (passwordSecurity.comparePassword(password, auth.getPassword(), player.getName())) { return true; } else { diff --git a/src/test/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewerTest.java b/src/test/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewerTest.java new file mode 100644 index 00000000..8f34bcbf --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/executable/authme/debug/PlayerAuthViewerTest.java @@ -0,0 +1,106 @@ +package fr.xephi.authme.command.executable.authme.debug; + +import fr.xephi.authme.data.auth.PlayerAuth; +import fr.xephi.authme.datasource.DataSource; +import org.bukkit.command.CommandSender; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Collections; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.hamcrest.MockitoHamcrest.argThat; + +/** + * Test for {@link PlayerAuthViewer}. + */ +@RunWith(MockitoJUnitRunner.class) +public class PlayerAuthViewerTest { + + @InjectMocks + private PlayerAuthViewer authViewer; + + @Mock + private DataSource dataSource; + + @Test + public void shouldMakeExample() { + // given + CommandSender sender = mock(CommandSender.class); + + // when + authViewer.execute(sender, Collections.emptyList()); + + // then + verify(sender).sendMessage(argThat(containsString("Example: /authme debug db Bobby"))); + } + + @Test + public void shouldHandleMissingPlayer() { + // given + CommandSender sender = mock(CommandSender.class); + + // when + authViewer.execute(sender, Collections.singletonList("bogus")); + + // then + verify(dataSource).getAuth("bogus"); + verify(sender).sendMessage(argThat(containsString("No record exists for 'bogus'"))); + } + + @Test + public void shouldDisplayAuthInfo() { + // given + CommandSender sender = mock(CommandSender.class); + PlayerAuth auth = PlayerAuth.builder().name("george").realName("George") + .password("abcdefghijkl", "mnopqrst") + .lastIp("127.1.2.7").registrationDate(1111140000000L) + .totpKey("SECRET1321") + .build(); + given(dataSource.getAuth("George")).willReturn(auth); + + // when + authViewer.execute(sender, Collections.singletonList("George")); + + // then + ArgumentCaptor textCaptor = ArgumentCaptor.forClass(String.class); + verify(sender, atLeastOnce()).sendMessage(textCaptor.capture()); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Player george / George"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Registration: 2005-03-18T"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Hash / salt (partial): 'abcdef...' / 'mnop...'"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("TOTP code (partial): 'SEC...'"))); + } + + @Test + public void shouldHandleCornerCases() { + // given + CommandSender sender = mock(CommandSender.class); + PlayerAuth auth = PlayerAuth.builder().name("tar") + .password("abcd", null) + .lastIp("127.1.2.7").registrationDate(0L) + .build(); + given(dataSource.getAuth("Tar")).willReturn(auth); + + // when + authViewer.execute(sender, Collections.singletonList("Tar")); + + // then + ArgumentCaptor textCaptor = ArgumentCaptor.forClass(String.class); + verify(sender, atLeastOnce()).sendMessage(textCaptor.capture()); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Player tar / Player"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Registration: Not available (0)"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Last login: Not available (null)"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("Hash / salt (partial): 'ab...' / ''"))); + assertThat(textCaptor.getAllValues(), hasItem(containsString("TOTP code (partial): ''"))); + } +} diff --git a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java index 38307f93..1335ca6f 100644 --- a/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java +++ b/src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java @@ -11,12 +11,12 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; +import java.util.Arrays; import java.util.Collections; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyZeroInteractions; @@ -57,7 +57,19 @@ public class LoginCommandTest { command.executeCommand(sender, Collections.singletonList("password")); // then - verify(management).performLogin(eq(sender), eq("password")); + verify(management).performLogin(sender, "password", null); + } + + @Test + public void shouldCallManagementForPasswordAndTotpCode() { + // given + Player sender = mock(Player.class); + + // when + command.executeCommand(sender, Arrays.asList("pwd", "12345")); + + // then + verify(management).performLogin(sender, "pwd", "12345"); } @Test