diff --git a/src/main/java/fr/xephi/authme/command/executable/totp/AddTotpCommand.java b/src/main/java/fr/xephi/authme/command/executable/totp/AddTotpCommand.java index a52741b22..e8a78bd1d 100644 --- a/src/main/java/fr/xephi/authme/command/executable/totp/AddTotpCommand.java +++ b/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 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, diff --git a/src/main/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommand.java b/src/main/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommand.java index 9dcd99a65..1ab8192be 100644 --- a/src/main/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommand.java +++ b/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 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()); - messages.send(player, MessageKey.TWO_FACTOR_ENABLE_SUCCESS); + insertTotpKeyIntoDatabase(player, auth, totpDetails); } 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); + } + } } diff --git a/src/main/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommand.java b/src/main/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommand.java index 1ad42cb8a..ebcf554c2 100644 --- a/src/main/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommand.java +++ b/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 arguments) { - PlayerAuth auth = dataSource.getAuth(player.getName()); - if (auth.getTotpKey() == null) { + PlayerAuth auth = playerCache.getAuth(player.getName()); + 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()); - messages.send(player, MessageKey.TWO_FACTOR_REMOVED_SUCCESS); + removeTotpKeyFromDatabase(player, auth); } 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); + } + } } diff --git a/src/main/java/fr/xephi/authme/command/executable/totp/TotpCodeCommand.java b/src/main/java/fr/xephi/authme/command/executable/totp/TotpCodeCommand.java index 3ddbb9649..398759028 100644 --- a/src/main/java/fr/xephi/authme/command/executable/totp/TotpCodeCommand.java +++ b/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); } } diff --git a/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java b/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java index 534c0c019..53d74dfba 100644 --- a/src/main/java/fr/xephi/authme/data/auth/PlayerAuth.java +++ b/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)) { diff --git a/src/test/java/fr/xephi/authme/command/executable/totp/AddTotpCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/totp/AddTotpCommandTest.java index 8791226a5..ff8608b37 100644 --- a/src/test/java/fr/xephi/authme/command/executable/totp/AddTotpCommandTest.java +++ b/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"); diff --git a/src/test/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommandTest.java index 0d921d37a..17a011ee8 100644 --- a/src/test/java/fr/xephi/authme/command/executable/totp/ConfirmTotpCommandTest.java +++ b/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(generateTotpService.getGeneratedTotpKey(player)).willReturn(new TotpGenerationResult("totp-key", "url-not-relevant")); + given(playerCache.getAuth(playerName)).willReturn(auth); + 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); - verifyZeroInteractions(generateTotpService); + verify(playerCache, only()).getAuth(playerName); + 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); - verifyZeroInteractions(generateTotpService); - verify(messages).send(player, MessageKey.REGISTER_MESSAGE); + verify(playerCache, only()).getAuth(playerName); + verifyZeroInteractions(generateTotpService, dataSource); + verify(messages).send(player, MessageKey.NOT_LOGGED_IN); } } diff --git a/src/test/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommandTest.java b/src/test/java/fr/xephi/authme/command/executable/totp/RemoveTotpCommandTest.java new file mode 100644 index 000000000..18903972d --- /dev/null +++ b/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); + } +}