#1141 Move TOTP code during login as separate step: /2fa code

Rough version.
- Introduces a limbo player state on the LimboPlayer, allowing us to add further mandatory actions between successful (password) authentication and the ability to play on the server

src/main/java/fr/xephi/authme/command/CommandInitializer.java

@@ -44,6 +44,7 @@ import fr.xephi.authme.command.executable.totp.AddTotpCommand;
 import fr.xephi.authme.command.executable.totp.ConfirmTotpCommand;
 import fr.xephi.authme.command.executable.totp.RemoveTotpCommand;
 import fr.xephi.authme.command.executable.totp.TotpBaseCommand;
+import fr.xephi.authme.command.executable.totp.TotpCodeCommand;
 import fr.xephi.authme.command.executable.unregister.UnregisterCommand;
 import fr.xephi.authme.command.executable.verification.VerificationCommand;
 import fr.xephi.authme.permission.AdminPermission;
@@ -92,7 +93,6 @@ public class CommandInitializer {
             .description("Login command")
             .detailedDescription("Command to log in using AuthMeReloaded.")
             .withArgument("password", "Login password", MANDATORY)
-            .withArgument("2facode", "TOTP code", OPTIONAL)
             .permission(PlayerPermission.LOGIN)
             .executableCommand(LoginCommand.class)
             .register();
@@ -561,6 +561,15 @@ public class CommandInitializer {
             .executableCommand(TotpBaseCommand.class)
             .register();
 
+        // Register the base totp code
+        CommandDescription.builder()
+            .parent(null)
+            .labels("code", "c")
+            .description("Command for logging in")
+            .detailedDescription("Processes the two-factor authentication code during login.")
+            .executableCommand(TotpCodeCommand.class)
+            .register();
+
         // Register totp add
         CommandDescription.builder()
             .parent(totpBase)

src/main/java/fr/xephi/authme/command/executable/captcha/CaptchaCommand.java

@@ -4,6 +4,7 @@ import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.data.captcha.LoginCaptchaManager;
 import fr.xephi.authme.data.captcha.RegistrationCaptchaManager;
+import fr.xephi.authme.data.limbo.LimboMessageType;
 import fr.xephi.authme.data.limbo.LimboService;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.message.MessageKey;
@@ -80,6 +81,6 @@ public class CaptchaCommand extends PlayerCommand {
             String newCode = registrationCaptchaManager.getCaptchaCodeOrGenerateNew(player.getName());
             commonService.send(player, MessageKey.CAPTCHA_WRONG_ERROR, newCode);
         }
-        limboService.resetMessageTask(player, false);
+        limboService.resetMessageTask(player, LimboMessageType.REGISTER);
     }
 }

src/main/java/fr/xephi/authme/command/executable/login/LoginCommand.java

@@ -19,8 +19,7 @@ public class LoginCommand extends PlayerCommand {
     @Override
     public void runCommand(Player player, List<String> arguments) {
         String password = arguments.get(0);
-        String totpCode = arguments.size() > 1 ? arguments.get(1) : null;
+        management.performLogin(player, password);
-        management.performLogin(player, password, totpCode);
     }
 
     @Override

src/main/java/fr/xephi/authme/command/executable/totp/TotpCodeCommand.java

@@ -0,0 +1,72 @@
+package fr.xephi.authme.command.executable.totp;
+
+import fr.xephi.authme.command.PlayerCommand;
+import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.data.auth.PlayerCache;
+import fr.xephi.authme.data.limbo.LimboPlayer;
+import fr.xephi.authme.data.limbo.LimboPlayerState;
+import fr.xephi.authme.data.limbo.LimboService;
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.message.MessageKey;
+import fr.xephi.authme.message.Messages;
+import fr.xephi.authme.process.login.AsynchronousLogin;
+import fr.xephi.authme.security.totp.TotpService;
+import org.bukkit.entity.Player;
+
+import javax.inject.Inject;
+import java.util.List;
+
+/**
+ * TOTP code command for processing the 2FA code during the login process.
+ */
+public class TotpCodeCommand extends PlayerCommand {
+
+    @Inject
+    private LimboService limboService;
+
+    @Inject
+    private PlayerCache playerCache;
+
+    @Inject
+    private Messages messages;
+
+    @Inject
+    private TotpService totpService;
+
+    @Inject
+    private DataSource dataSource;
+
+    @Inject
+    private AsynchronousLogin asynchronousLogin;
+
+    @Override
+    protected void runCommand(Player player, List<String> arguments) {
+        if (playerCache.isAuthenticated(player.getName())) {
+            messages.send(player, MessageKey.ALREADY_LOGGED_IN_ERROR);
+            return;
+        }
+
+        PlayerAuth auth = dataSource.getAuth(player.getName());
+        if (auth == null) {
+            messages.send(player, MessageKey.REGISTER_MESSAGE);
+            return;
+        }
+
+        LimboPlayer limbo = limboService.getLimboPlayer(player.getName());
+        if (limbo.getState() == LimboPlayerState.TOTP_REQUIRED) {
+            processCode(player, limbo, auth, arguments.get(0));
+        } else {
+            messages.send(player, MessageKey.LOGIN_MESSAGE);
+        }
+    }
+
+    private void processCode(Player player, LimboPlayer limbo, PlayerAuth auth, String inputCode) {
+        boolean isCodeValid = totpService.verifyCode(auth, inputCode);
+        if (isCodeValid) {
+            limbo.setState(LimboPlayerState.FINISHED);
+            asynchronousLogin.performLogin(player, auth);
+        } else {
+            player.sendMessage("Invalid code!");
+        }
+    }
+}

src/main/java/fr/xephi/authme/data/limbo/LimboMessageType.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.data.limbo;
+
+public enum LimboMessageType {
+
+    REGISTER,
+
+    LOG_IN,
+
+    TOTP_CODE
+
+}

src/main/java/fr/xephi/authme/data/limbo/LimboPlayer.java

@@ -23,6 +23,7 @@ public class LimboPlayer {
     private final float flySpeed;
     private BukkitTask timeoutTask = null;
     private MessageTask messageTask = null;
+    private LimboPlayerState state = LimboPlayerState.PASSWORD_REQUIRED;
 
     public LimboPlayer(Location loc, boolean operator, Collection<String> groups, boolean fly, float walkSpeed,
                        float flySpeed) {
@@ -124,4 +125,12 @@ public class LimboPlayer {
         setMessageTask(null);
         setTimeoutTask(null);
     }
+
+    public LimboPlayerState getState() {
+        return state;
+    }
+
+    public void setState(LimboPlayerState state) {
+        this.state = state;
+    }
 }

src/main/java/fr/xephi/authme/data/limbo/LimboPlayerState.java

@@ -0,0 +1,11 @@
+package fr.xephi.authme.data.limbo;
+
+public enum LimboPlayerState {
+
+    PASSWORD_REQUIRED,
+
+    TOTP_REQUIRED,
+
+    FINISHED
+
+}

src/main/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManager.java

@@ -45,11 +45,11 @@ class LimboPlayerTaskManager {
      *
      * @param player the player
      * @param limbo the associated limbo player of the player
-     * @param isRegistered whether the player is registered or not (needed to determine the message in the task)
+     * @param messageType message type
      */
-    void registerMessageTask(Player player, LimboPlayer limbo, boolean isRegistered) {
+    void registerMessageTask(Player player, LimboPlayer limbo, LimboMessageType messageType) {
         int interval = settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL);
-        MessageResult result = getMessageKey(player.getName(), isRegistered);
+        MessageResult result = getMessageKey(player.getName(), messageType);
         if (interval > 0) {
             String[] joinMessage = messages.retrieveSingle(player, result.messageKey, result.args).split("\n");
             MessageTask messageTask = new MessageTask(player, joinMessage);
@@ -89,12 +89,14 @@ class LimboPlayerTaskManager {
      * Returns the appropriate message key according to the registration status and settings.
      *
      * @param name the player's name
-     * @param isRegistered whether or not the username is registered
+     * @param messageType the message to show
      * @return the message key to display to the user
      */
-    private MessageResult getMessageKey(String name, boolean isRegistered) {
+    private MessageResult getMessageKey(String name, LimboMessageType messageType) {
-        if (isRegistered) {
+        if (messageType == LimboMessageType.LOG_IN) {
             return new MessageResult(MessageKey.LOGIN_MESSAGE);
+        } else if (messageType == LimboMessageType.TOTP_CODE) {
+            return new MessageResult(MessageKey.TWO_FACTOR_CODE_REQUIRED);
         } else if (registrationCaptchaManager.isCaptchaRequired(name)) {
             final String captchaCode = registrationCaptchaManager.getCaptchaCodeOrGenerateNew(name);
             return new MessageResult(MessageKey.CAPTCHA_FOR_REGISTRATION_REQUIRED, captchaCode);

src/main/java/fr/xephi/authme/data/limbo/LimboService.java

@@ -69,7 +69,8 @@ public class LimboService {
         LimboPlayer limboPlayer = helper.merge(existingLimbo, limboFromDisk);
         limboPlayer = helper.merge(helper.createLimboPlayer(player, isRegistered, location), limboPlayer);
 
-        taskManager.registerMessageTask(player, limboPlayer, isRegistered);
+        taskManager.registerMessageTask(player, limboPlayer,
+            isRegistered ? LimboMessageType.LOG_IN : LimboMessageType.REGISTER);
         taskManager.registerTimeoutTask(player, limboPlayer);
         helper.revokeLimboStates(player);
         authGroupHandler.setGroup(player, limboPlayer,
@@ -134,7 +135,7 @@ public class LimboService {
         Optional<LimboPlayer> limboPlayer = getLimboOrLogError(player, "reset tasks");
         limboPlayer.ifPresent(limbo -> {
             taskManager.registerTimeoutTask(player, limbo);
-            taskManager.registerMessageTask(player, limbo, true);
+            taskManager.registerMessageTask(player, limbo, LimboMessageType.LOG_IN);
         });
         authGroupHandler.setGroup(player, limboPlayer.orElse(null), AuthGroupType.REGISTERED_UNAUTHENTICATED);
     }
@@ -143,11 +144,11 @@ public class LimboService {
      * Resets the message task associated with the player's LimboPlayer.
      *
      * @param player the player to set a new message task for
-     * @param isRegistered whether or not the player is registered
+     * @param messageType the message to show for the limbo player
      */
-    public void resetMessageTask(Player player, boolean isRegistered) {
+    public void resetMessageTask(Player player, LimboMessageType messageType) {
         getLimboOrLogError(player, "reset message task")
-            .ifPresent(limbo -> taskManager.registerMessageTask(player, limbo, isRegistered));
+            .ifPresent(limbo -> taskManager.registerMessageTask(player, limbo, messageType));
     }
 
     /**

src/main/java/fr/xephi/authme/message/MessageKey.java

@@ -197,6 +197,9 @@ public enum MessageKey {
     /** Your secret code is %code. You can scan it from here %url */
     TWO_FACTOR_CREATE("misc.two_factor_create", "%code", "%url"),
 
+    /** Please submit your two-factor authentication code with /2fa code &ltcode>. */
+    TWO_FACTOR_CODE_REQUIRED("two_factor.code_required"),
+
     /** You are not the owner of this account. Please choose another name! */
     NOT_OWNER_ERROR("on_join_validation.not_owner_error"),
 

src/main/java/fr/xephi/authme/message/updater/MessageUpdater.java

@@ -131,6 +131,7 @@ public class MessageUpdater {
             .put("captcha", new String[]{"Captcha"})
             .put("verification", new String[]{"Verification code"})
             .put("time", new String[]{"Time units"})
+            .put("two_factor", new String[]{"Two-factor authentication"})
             .build();
 
         Set<String> addedKeys = new HashSet<>();

src/main/java/fr/xephi/authme/process/Management.java

@@ -49,8 +49,8 @@ public class Management {
     }
 
 
-    public void performLogin(Player player, String password, String totpCode) {
+    public void performLogin(Player player, String password) {
-        runTask(() -> asynchronousLogin.login(player, password, totpCode));
+        runTask(() -> asynchronousLogin.login(player, password));
     }
 
     public void forceLogin(Player player) {

src/main/java/fr/xephi/authme/process/login/AsynchronousLogin.java

@@ -6,6 +6,8 @@ import fr.xephi.authme.data.TempbanManager;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.data.captcha.LoginCaptchaManager;
+import fr.xephi.authme.data.limbo.LimboMessageType;
+import fr.xephi.authme.data.limbo.LimboPlayerState;
 import fr.xephi.authme.data.limbo.LimboService;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.events.AuthMeAsyncPreLoginEvent;
@@ -18,7 +20,6 @@ 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.totp.TotpService;
 import fr.xephi.authme.service.BukkitService;
 import fr.xephi.authme.service.CommonService;
 import fr.xephi.authme.service.SessionService;
@@ -79,9 +80,6 @@ public class AsynchronousLogin implements AsynchronousProcess {
     @Inject
     private BungeeSender bungeeSender;
 
-    @Inject
-    private TotpService totpService;
-
     AsynchronousLogin() {
     }
 
@@ -90,12 +88,17 @@ 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, String totpCode) {
+    public void login(Player player, String password) {
         PlayerAuth auth = getPlayerAuth(player);
-        if (auth != null && checkPlayerInfo(player, auth, password, totpCode)) {
+        if (auth != null && checkPlayerInfo(player, auth, password)) {
-            performLogin(player, auth);
+            if (auth.getTotpKey() != null) {
+                limboService.resetMessageTask(player, LimboMessageType.TOTP_CODE);
+                limboService.getLimboPlayer(player.getName()).setState(LimboPlayerState.TOTP_REQUIRED);
+                // TODO #1141: Check if we should check limbo state before processing password
+            } else {
+                performLogin(player, auth);
+            }
         }
     }
 
@@ -130,7 +133,7 @@ public class AsynchronousLogin implements AsynchronousProcess {
         if (auth == null) {
             service.send(player, MessageKey.UNKNOWN_USER);
             // Recreate the message task to immediately send the message again as response
-            limboService.resetMessageTask(player, false);
+            limboService.resetMessageTask(player, LimboMessageType.REGISTER);
             return null;
         }
 
@@ -161,11 +164,10 @@ 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, String totpCode) {
+    private boolean checkPlayerInfo(Player player, PlayerAuth auth, String password) {
         final String name = player.getName().toLowerCase();
 
         // If captcha is required send a message to the player and deny to log in
@@ -180,17 +182,6 @@ 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 <password> <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 {
@@ -235,7 +226,7 @@ public class AsynchronousLogin implements AsynchronousProcess {
      * @param player the player to log in
      * @param auth the associated PlayerAuth object
      */
-    private void performLogin(Player player, PlayerAuth auth) {
+    public void performLogin(Player player, PlayerAuth auth) {
         if (player.isOnline()) {
             final boolean isFirstLogin = (auth.getLastLogin() == null);
 

src/main/java/fr/xephi/authme/settings/properties/RestrictionSettings.java

@@ -26,7 +26,7 @@ public final class RestrictionSettings implements SettingsHolder {
     @Comment("Allowed commands for unauthenticated players")
     public static final Property<List<String>> ALLOW_COMMANDS =
         newLowercaseListProperty("settings.restrictions.allowCommands",
-            "/login", "/register", "/l", "/reg", "/email", "/captcha");
+            "/login", "/register", "/l", "/reg", "/email", "/captcha", "/2fa", "/totp");
 
     @Comment({
         "Max number of allowed registrations per IP",

src/main/resources/messages/messages_en.yml

@@ -128,6 +128,9 @@ verification:
   code_expired: '&3Your code has expired! Execute another sensitive command to get a new code!'
   email_needed: '&3To verify your identity you need to link an email address with your account!!'
 
+two_factor:
+  code_required: 'Please submit your two-factor authentication code with /2fa code <code>'
+
 # Time units
 time:
   second: 'second'

src/test/java/fr/xephi/authme/command/executable/login/LoginCommandTest.java

@@ -11,7 +11,6 @@ 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;
@@ -57,19 +56,7 @@ public class LoginCommandTest {
         command.executeCommand(sender, Collections.singletonList("password"));
 
         // then
-        verify(management).performLogin(sender, "password", null);
+        verify(management).performLogin(sender, "password" );
-    }
-
-    @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

src/test/java/fr/xephi/authme/data/limbo/LimboPlayerTaskManagerTest.java

@@ -76,7 +76,7 @@ public class LimboPlayerTaskManagerTest {
         given(settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL)).willReturn(interval);
 
         // when
-        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, false);
+        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, LimboMessageType.REGISTER);
 
         // then
         verify(limboPlayer).setMessageTask(any(MessageTask.class));
@@ -94,7 +94,7 @@ public class LimboPlayerTaskManagerTest {
         given(settings.getProperty(RegistrationSettings.MESSAGE_INTERVAL)).willReturn(0);
 
         // when
-        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, true);
+        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, LimboMessageType.LOG_IN);
 
         // then
         verifyZeroInteractions(limboPlayer, bukkitService);
@@ -113,7 +113,7 @@ public class LimboPlayerTaskManagerTest {
         given(messages.retrieveSingle(player, MessageKey.REGISTER_MESSAGE)).willReturn("Please register!");
 
         // when
-        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, false);
+        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, LimboMessageType.REGISTER);
 
         // then
         assertThat(limboPlayer.getMessageTask(), not(nullValue()));
@@ -137,7 +137,7 @@ public class LimboPlayerTaskManagerTest {
         given(messages.retrieveSingle(player, MessageKey.CAPTCHA_FOR_REGISTRATION_REQUIRED, captcha)).willReturn("Need to use captcha");
 
         // when
-        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, false);
+        limboPlayerTaskManager.registerMessageTask(player, limboPlayer, LimboMessageType.REGISTER);
 
         // then
         assertThat(limboPlayer.getMessageTask(), not(nullValue()));

src/test/java/fr/xephi/authme/data/limbo/LimboServiceTest.java

@@ -90,7 +90,7 @@ public class LimboServiceTest {
         limboService.createLimboPlayer(player, true);
 
         // then
-        verify(taskManager).registerMessageTask(eq(player), any(LimboPlayer.class), eq(true));
+        verify(taskManager).registerMessageTask(eq(player), any(LimboPlayer.class), eq(LimboMessageType.LOG_IN));
         verify(taskManager).registerTimeoutTask(eq(player), any(LimboPlayer.class));
         verify(player).setAllowFlight(false);
         verify(player).setFlySpeed(0.0f);
@@ -121,7 +121,7 @@ public class LimboServiceTest {
         limboService.createLimboPlayer(player, false);
 
         // then
-        verify(taskManager).registerMessageTask(eq(player), any(LimboPlayer.class), eq(false));
+        verify(taskManager).registerMessageTask(eq(player), any(LimboPlayer.class), eq(LimboMessageType.REGISTER));
         verify(taskManager).registerTimeoutTask(eq(player), any(LimboPlayer.class));
         verify(permissionsManager, only()).hasGroupSupport();
         verify(player).setAllowFlight(false);
@@ -209,7 +209,7 @@ public class LimboServiceTest {
 
         // then
         verify(taskManager).registerTimeoutTask(player, limbo);
-        verify(taskManager).registerMessageTask(player, limbo, true);
+        verify(taskManager).registerMessageTask(player, limbo, LimboMessageType.LOG_IN);
         verify(authGroupHandler).setGroup(player, limbo, AuthGroupType.REGISTERED_UNAUTHENTICATED);
     }