#1141 Check that user is logged in before changing TOTP key

- Use PlayerCache to check that user is logged in where appropriate
- Add log statements

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

@@ -2,7 +2,7 @@ package fr.xephi.authme.command.executable.totp;
 
 import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.data.auth.PlayerAuth;
-import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.security.totp.GenerateTotpService;
@@ -21,16 +21,16 @@ public class AddTotpCommand extends PlayerCommand {
     private GenerateTotpService generateTotpService;
 
     @Inject
-    private DataSource dataSource;
+    private PlayerCache playerCache;
 
     @Inject
     private Messages messages;
 
     @Override
     protected void runCommand(Player player, List<String> arguments) {
-        PlayerAuth auth = dataSource.getAuth(player.getName());
+        PlayerAuth auth = playerCache.getAuth(player.getName());
         if (auth == null) {
-            messages.send(player, MessageKey.REGISTER_MESSAGE);
+            messages.send(player, MessageKey.NOT_LOGGED_IN);
         } else if (auth.getTotpKey() == null) {
             TotpGenerationResult createdTotpInfo = generateTotpService.generateTotpKey(player);
             messages.send(player, MessageKey.TWO_FACTOR_CREATE,

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

@@ -1,7 +1,9 @@
 package fr.xephi.authme.command.executable.totp;
 
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
@@ -20,6 +22,9 @@ public class ConfirmTotpCommand extends PlayerCommand {
     @Inject
     private GenerateTotpService generateTotpService;
 
+    @Inject
+    private PlayerCache playerCache;
+
     @Inject
     private DataSource dataSource;
 
@@ -28,17 +33,17 @@ public class ConfirmTotpCommand extends PlayerCommand {
 
     @Override
     protected void runCommand(Player player, List<String> arguments) {
-        PlayerAuth auth = dataSource.getAuth(player.getName());
+        PlayerAuth auth = playerCache.getAuth(player.getName());
         if (auth == null) {
-            messages.send(player, MessageKey.REGISTER_MESSAGE);
+            messages.send(player, MessageKey.NOT_LOGGED_IN);
         } else if (auth.getTotpKey() != null) {
             messages.send(player, MessageKey.TWO_FACTOR_ALREADY_ENABLED);
         } else {
-            verifyTotpCodeConfirmation(player, arguments.get(0));
+            verifyTotpCodeConfirmation(player, auth, arguments.get(0));
         }
     }
 
-    private void verifyTotpCodeConfirmation(Player player, String inputTotpCode) {
+    private void verifyTotpCodeConfirmation(Player player, PlayerAuth auth, String inputTotpCode) {
         final TotpGenerationResult totpDetails = generateTotpService.getGeneratedTotpKey(player);
         if (totpDetails == null) {
             messages.send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_NO_CODE);
@@ -46,11 +51,21 @@ public class ConfirmTotpCommand extends PlayerCommand {
             boolean isCodeValid = generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, inputTotpCode);
             if (isCodeValid) {
                 generateTotpService.removeGenerateTotpKey(player);
-                dataSource.setTotpKey(player.getName(), totpDetails.getTotpKey());
+                insertTotpKeyIntoDatabase(player, auth, totpDetails);
-                messages.send(player, MessageKey.TWO_FACTOR_ENABLE_SUCCESS);
             } else {
                 messages.send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_WRONG_CODE);
             }
         }
     }
+
+    private void insertTotpKeyIntoDatabase(Player player, PlayerAuth auth, TotpGenerationResult totpDetails) {
+        if (dataSource.setTotpKey(player.getName(), totpDetails.getTotpKey())) {
+            messages.send(player, MessageKey.TWO_FACTOR_ENABLE_SUCCESS);
+            auth.setTotpKey(totpDetails.getTotpKey());
+            playerCache.updatePlayer(auth);
+            ConsoleLogger.info("Player '" + player.getName() + "' has successfully added a TOTP key to their account");
+        } else {
+            messages.send(player, MessageKey.ERROR);
+        }
+    }
 }

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

@@ -1,7 +1,9 @@
 package fr.xephi.authme.command.executable.totp;
 
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
@@ -19,6 +21,9 @@ public class RemoveTotpCommand extends PlayerCommand {
     @Inject
     private DataSource dataSource;
 
+    @Inject
+    private PlayerCache playerCache;
+
     @Inject
     private TotpAuthenticator totpAuthenticator;
 
@@ -27,16 +32,28 @@ public class RemoveTotpCommand extends PlayerCommand {
 
     @Override
     protected void runCommand(Player player, List<String> arguments) {
-        PlayerAuth auth = dataSource.getAuth(player.getName());
+        PlayerAuth auth = playerCache.getAuth(player.getName());
-        if (auth.getTotpKey() == null) {
+        if (auth == null) {
+            messages.send(player, MessageKey.NOT_LOGGED_IN);
+        } else if (auth.getTotpKey() == null) {
             messages.send(player, MessageKey.TWO_FACTOR_NOT_ENABLED_ERROR);
         } else {
             if (totpAuthenticator.checkCode(auth, arguments.get(0))) {
-                dataSource.removeTotpKey(auth.getNickname());
+                removeTotpKeyFromDatabase(player, auth);
-                messages.send(player, MessageKey.TWO_FACTOR_REMOVED_SUCCESS);
             } else {
                 messages.send(player, MessageKey.TWO_FACTOR_INVALID_CODE);
             }
         }
     }
+
+    private void removeTotpKeyFromDatabase(Player player, PlayerAuth auth) {
+        if (dataSource.removeTotpKey(auth.getNickname())) {
+            auth.setTotpKey(null);
+            playerCache.updatePlayer(auth);
+            messages.send(player, MessageKey.TWO_FACTOR_REMOVED_SUCCESS);
+            ConsoleLogger.info("Player '" + player.getName() + "' removed their TOTP key");
+        } else {
+            messages.send(player, MessageKey.ERROR);
+        }
+    }
 }

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

@@ -1,5 +1,6 @@
 package fr.xephi.authme.command.executable.totp;
 
+import fr.xephi.authme.ConsoleLogger;
 import fr.xephi.authme.command.PlayerCommand;
 import fr.xephi.authme.data.auth.PlayerAuth;
 import fr.xephi.authme.data.auth.PlayerCache;
@@ -56,6 +57,8 @@ public class TotpCodeCommand extends PlayerCommand {
         if (limbo != null && limbo.getState() == LimboPlayerState.TOTP_REQUIRED) {
             processCode(player, auth, arguments.get(0));
         } else {
+            ConsoleLogger.debug(() -> "Aborting TOTP check for player '" + player.getName()
+                + "'. Invalid limbo state: " + (limbo == null ? "no limbo" : limbo.getState()));
             messages.send(player, MessageKey.LOGIN_MESSAGE);
         }
     }
@@ -63,8 +66,10 @@ public class TotpCodeCommand extends PlayerCommand {
     private void processCode(Player player, PlayerAuth auth, String inputCode) {
         boolean isCodeValid = totpAuthenticator.checkCode(auth, inputCode);
         if (isCodeValid) {
+            ConsoleLogger.debug("Successfully checked TOTP code for `{0}`", player.getName());
             asynchronousLogin.performLogin(player, auth);
         } else {
+            ConsoleLogger.debug("Input TOTP code was invalid for player `{0}`", player.getName());
             messages.send(player, MessageKey.TWO_FACTOR_INVALID_CODE);
         }
     }

src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java

@@ -165,6 +165,10 @@ public class PlayerAuth {
         return totpKey;
     }
 
+    public void setTotpKey(String totpKey) {
+        this.totpKey = totpKey;
+    }
+
     @Override
     public boolean equals(Object obj) {
         if (!(obj instanceof PlayerAuth)) {

src/test/java/fr/xephi/authme/command/executable/totp/AddTotpCommandTest.java

@@ -1,7 +1,7 @@
 package fr.xephi.authme.command.executable.totp;
 
 import fr.xephi.authme.data.auth.PlayerAuth;
-import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.security.totp.GenerateTotpService;
@@ -32,21 +32,21 @@ public class AddTotpCommandTest {
     @Mock
     private GenerateTotpService generateTotpService;
     @Mock
-    private DataSource dataSource;
+    private PlayerCache playerCache;
     @Mock
     private Messages messages;
 
     @Test
-    public void shouldHandleNonExistentUser() {
+    public void shouldHandleNonLoggedInUser() {
         // given
         Player player = mockPlayerWithName("bob");
-        given(dataSource.getAuth("bob")).willReturn(null);
+        given(playerCache.getAuth("bob")).willReturn(null);
 
         // when
         addTotpCommand.runCommand(player, Collections.emptyList());
 
         // then
-        verify(messages).send(player, MessageKey.REGISTER_MESSAGE);
+        verify(messages).send(player, MessageKey.NOT_LOGGED_IN);
         verifyZeroInteractions(generateTotpService);
     }
 
@@ -56,7 +56,7 @@ public class AddTotpCommandTest {
         Player player = mockPlayerWithName("arend");
         PlayerAuth auth = PlayerAuth.builder().name("arend")
             .totpKey("TOTP2345").build();
-        given(dataSource.getAuth("arend")).willReturn(auth);
+        given(playerCache.getAuth("arend")).willReturn(auth);
 
         // when
         addTotpCommand.runCommand(player, Collections.emptyList());
@@ -71,7 +71,7 @@ public class AddTotpCommandTest {
         // given
         Player player = mockPlayerWithName("charles");
         PlayerAuth auth = PlayerAuth.builder().name("charles").build();
-        given(dataSource.getAuth("charles")).willReturn(auth);
+        given(playerCache.getAuth("charles")).willReturn(auth);
 
         TotpGenerationResult generationResult = new TotpGenerationResult(
             "777Key214", "http://example.org/qr-code/link");

src/test/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommandTest.java

@@ -1,12 +1,15 @@
 package fr.xephi.authme.command.executable.totp;
 
+import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.data.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.message.MessageKey;
 import fr.xephi.authme.message.Messages;
 import fr.xephi.authme.security.totp.GenerateTotpService;
 import fr.xephi.authme.security.totp.TotpAuthenticator.TotpGenerationResult;
 import org.bukkit.entity.Player;
+import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
@@ -15,7 +18,10 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.Collections;
 
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -37,8 +43,15 @@ public class ConfirmTotpCommandTest {
     @Mock
     private DataSource dataSource;
     @Mock
+    private PlayerCache playerCache;
+    @Mock
     private Messages messages;
 
+    @BeforeClass
+    public static void setUpLogger() {
+        TestHelper.setupLogger();
+    }
+
     @Test
     public void shouldAddTotpCodeToUserAfterSuccessfulConfirmation() {
         // given
@@ -46,10 +59,12 @@ public class ConfirmTotpCommandTest {
         String playerName = "George";
         given(player.getName()).willReturn(playerName);
         PlayerAuth auth = PlayerAuth.builder().name(playerName).build();
-        given(dataSource.getAuth(playerName)).willReturn(auth);
+        given(playerCache.getAuth(playerName)).willReturn(auth);
-        given(generateTotpService.getGeneratedTotpKey(player)).willReturn(new TotpGenerationResult("totp-key", "url-not-relevant"));
+        String generatedTotpKey = "totp-key";
+        given(generateTotpService.getGeneratedTotpKey(player)).willReturn(new TotpGenerationResult(generatedTotpKey, "url-not-relevant"));
         String totpCode = "954321";
         given(generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, totpCode)).willReturn(true);
+        given(dataSource.setTotpKey(anyString(), anyString())).willReturn(true);
 
         // when
         command.runCommand(player, Collections.singletonList(totpCode));
@@ -57,8 +72,10 @@ public class ConfirmTotpCommandTest {
         // then
         verify(generateTotpService).isTotpCodeCorrectForGeneratedTotpKey(player, totpCode);
         verify(generateTotpService).removeGenerateTotpKey(player);
-        verify(dataSource).setTotpKey(playerName, "totp-key");
+        verify(dataSource).setTotpKey(playerName, generatedTotpKey);
+        verify(playerCache).updatePlayer(auth);
         verify(messages).send(player, MessageKey.TWO_FACTOR_ENABLE_SUCCESS);
+        assertThat(auth.getTotpKey(), equalTo(generatedTotpKey));
     }
 
     @Test
@@ -68,7 +85,7 @@ public class ConfirmTotpCommandTest {
         String playerName = "George";
         given(player.getName()).willReturn(playerName);
         PlayerAuth auth = PlayerAuth.builder().name(playerName).build();
-        given(dataSource.getAuth(playerName)).willReturn(auth);
+        given(playerCache.getAuth(playerName)).willReturn(auth);
         given(generateTotpService.getGeneratedTotpKey(player)).willReturn(new TotpGenerationResult("totp-key", "url-not-relevant"));
         String totpCode = "754321";
         given(generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, totpCode)).willReturn(false);
@@ -79,8 +96,9 @@ public class ConfirmTotpCommandTest {
         // then
         verify(generateTotpService).isTotpCodeCorrectForGeneratedTotpKey(player, totpCode);
         verify(generateTotpService, never()).removeGenerateTotpKey(any(Player.class));
-        verify(dataSource, only()).getAuth(playerName);
+        verify(playerCache, only()).getAuth(playerName);
         verify(messages).send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_WRONG_CODE);
+        verifyZeroInteractions(dataSource);
     }
 
     @Test
@@ -90,7 +108,7 @@ public class ConfirmTotpCommandTest {
         String playerName = "George";
         given(player.getName()).willReturn(playerName);
         PlayerAuth auth = PlayerAuth.builder().name(playerName).build();
-        given(dataSource.getAuth(playerName)).willReturn(auth);
+        given(playerCache.getAuth(playerName)).willReturn(auth);
         given(generateTotpService.getGeneratedTotpKey(player)).willReturn(null);
 
         // when
@@ -98,8 +116,9 @@ public class ConfirmTotpCommandTest {
 
         // then
         verify(generateTotpService, only()).getGeneratedTotpKey(player);
-        verify(dataSource, only()).getAuth(playerName);
+        verify(playerCache, only()).getAuth(playerName);
         verify(messages).send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_NO_CODE);
+        verifyZeroInteractions(dataSource);
     }
 
     @Test
@@ -109,14 +128,14 @@ public class ConfirmTotpCommandTest {
         String playerName = "George";
         given(player.getName()).willReturn(playerName);
         PlayerAuth auth = PlayerAuth.builder().name(playerName).totpKey("A987234").build();
-        given(dataSource.getAuth(playerName)).willReturn(auth);
+        given(playerCache.getAuth(playerName)).willReturn(auth);
 
         // when
         command.runCommand(player, Collections.singletonList("871634"));
 
         // then
-        verify(dataSource, only()).getAuth(playerName);
+        verify(playerCache, only()).getAuth(playerName);
-        verifyZeroInteractions(generateTotpService);
+        verifyZeroInteractions(generateTotpService, dataSource);
         verify(messages).send(player, MessageKey.TWO_FACTOR_ALREADY_ENABLED);
     }
 
@@ -126,14 +145,14 @@ public class ConfirmTotpCommandTest {
         Player player = mock(Player.class);
         String playerName = "George";
         given(player.getName()).willReturn(playerName);
-        given(dataSource.getAuth(playerName)).willReturn(null);
+        given(playerCache.getAuth(playerName)).willReturn(null);
 
         // when
         command.runCommand(player, Collections.singletonList("984685"));
 
         // then
-        verify(dataSource, only()).getAuth(playerName);
+        verify(playerCache, only()).getAuth(playerName);
-        verifyZeroInteractions(generateTotpService);
+        verifyZeroInteractions(generateTotpService, dataSource);
-        verify(messages).send(player, MessageKey.REGISTER_MESSAGE);
+        verify(messages).send(player, MessageKey.NOT_LOGGED_IN);
     }
 }

src/test/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommandTest.java

@@ -0,0 +1,149 @@
+package fr.xephi.authme.command.executable.totp;
+
+import fr.xephi.authme.TestHelper;
+import fr.xephi.authme.data.auth.PlayerAuth;
+import fr.xephi.authme.data.auth.PlayerCache;
+import fr.xephi.authme.datasource.DataSource;
+import fr.xephi.authme.message.MessageKey;
+import fr.xephi.authme.message.Messages;
+import fr.xephi.authme.security.totp.TotpAuthenticator;
+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.junit.MockitoJUnitRunner;
+
+import static java.util.Collections.singletonList;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+import static org.mockito.BDDMockito.given;
+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 RemoveTotpCommand}.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class RemoveTotpCommandTest {
+
+    @InjectMocks
+    private RemoveTotpCommand command;
+
+    @Mock
+    private DataSource dataSource;
+    @Mock
+    private PlayerCache playerCache;
+    @Mock
+    private TotpAuthenticator totpAuthenticator;
+    @Mock
+    private Messages messages;
+
+    @BeforeClass
+    public static void setUpLogger() {
+        TestHelper.setupLogger();
+    }
+
+    @Test
+    public void shouldRemoveTotpKey() {
+        // given
+        String name = "aws";
+        PlayerAuth auth = PlayerAuth.builder().name(name).totpKey("some-totp-key").build();
+        given(playerCache.getAuth(name)).willReturn(auth);
+        String inputCode = "93847";
+        given(totpAuthenticator.checkCode(auth, inputCode)).willReturn(true);
+        given(dataSource.removeTotpKey(name)).willReturn(true);
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+
+        // when
+        command.runCommand(player, singletonList(inputCode));
+
+        // then
+        verify(dataSource).removeTotpKey(name);
+        verify(messages, only()).send(player, MessageKey.TWO_FACTOR_REMOVED_SUCCESS);
+        verify(playerCache).updatePlayer(auth);
+        assertThat(auth.getTotpKey(), nullValue());
+    }
+
+    @Test
+    public void shouldHandleDatabaseError() {
+        // given
+        String name = "aws";
+        PlayerAuth auth = PlayerAuth.builder().name(name).totpKey("some-totp-key").build();
+        given(playerCache.getAuth(name)).willReturn(auth);
+        String inputCode = "93847";
+        given(totpAuthenticator.checkCode(auth, inputCode)).willReturn(true);
+        given(dataSource.removeTotpKey(name)).willReturn(false);
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+
+        // when
+        command.runCommand(player, singletonList(inputCode));
+
+        // then
+        verify(dataSource).removeTotpKey(name);
+        verify(messages, only()).send(player, MessageKey.ERROR);
+        verify(playerCache, only()).getAuth(name);
+    }
+
+    @Test
+    public void shouldHandleInvalidCode() {
+        // given
+        String name = "cesar";
+        PlayerAuth auth = PlayerAuth.builder().name(name).totpKey("some-totp-key").build();
+        given(playerCache.getAuth(name)).willReturn(auth);
+        String inputCode = "93847";
+        given(totpAuthenticator.checkCode(auth, inputCode)).willReturn(false);
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+
+        // when
+        command.runCommand(player, singletonList(inputCode));
+
+        // then
+        verifyZeroInteractions(dataSource);
+        verify(messages, only()).send(player, MessageKey.TWO_FACTOR_INVALID_CODE);
+        verify(playerCache, only()).getAuth(name);
+    }
+
+    @Test
+    public void shouldHandleUserWithoutTotpKey() {
+        // given
+        String name = "cesar";
+        PlayerAuth auth = PlayerAuth.builder().name(name).build();
+        given(playerCache.getAuth(name)).willReturn(auth);
+        String inputCode = "654684";
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+
+        // when
+        command.runCommand(player, singletonList(inputCode));
+
+        // then
+        verifyZeroInteractions(dataSource, totpAuthenticator);
+        verify(messages, only()).send(player, MessageKey.TWO_FACTOR_NOT_ENABLED_ERROR);
+        verify(playerCache, only()).getAuth(name);
+    }
+
+    @Test
+    public void shouldHandleNonLoggedInUser() {
+        // given
+        String name = "cesar";
+        given(playerCache.getAuth(name)).willReturn(null);
+        String inputCode = "654684";
+        Player player = mock(Player.class);
+        given(player.getName()).willReturn(name);
+
+        // when
+        command.runCommand(player, singletonList(inputCode));
+
+        // then
+        verifyZeroInteractions(dataSource, totpAuthenticator);
+        verify(messages, only()).send(player, MessageKey.NOT_LOGGED_IN);
+        verify(playerCache, only()).getAuth(name);
+    }
+}