From e9ab82db6bea3f4c59095227208f4e426592bc0f Mon Sep 17 00:00:00 2001 From: ljacqu Date: Wed, 21 Mar 2018 23:56:13 +0100 Subject: [PATCH] #1141 Make 2fa messages translatable, various cleanups (null safety, ...) --- .../executable/totp/AddTotpCommand.java | 13 ++++++----- .../executable/totp/ConfirmTotpCommand.java | 19 +++++++++++---- .../executable/totp/RemoveTotpCommand.java | 11 ++++++--- .../executable/totp/TotpCodeCommand.java | 9 ++++---- .../authme/data/limbo/LimboPlayerState.java | 4 +--- .../fr/xephi/authme/message/MessageKey.java | 23 ++++++++++++++++++- .../security/totp/TotpAuthenticator.java | 7 ++++++ src/main/resources/messages/messages_en.yml | 9 +++++++- .../security/totp/TotpAuthenticatorTest.java | 8 +++++-- .../fr/xephi/authme/message/help_test.yml | 3 --- 10 files changed, 78 insertions(+), 28 deletions(-) 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 8fc9aaf10..9238c3b07 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 @@ -4,10 +4,9 @@ import fr.xephi.authme.command.PlayerCommand; import fr.xephi.authme.data.auth.PlayerAuth; 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 fr.xephi.authme.service.CommonService; -import org.bukkit.ChatColor; import org.bukkit.entity.Player; import javax.inject.Inject; @@ -25,17 +24,19 @@ public class AddTotpCommand extends PlayerCommand { private DataSource dataSource; @Inject - private CommonService commonService; + private Messages messages; @Override protected void runCommand(Player player, List arguments) { PlayerAuth auth = dataSource.getAuth(player.getName()); - if (auth.getTotpKey() == null) { + if (auth == null) { + messages.send(player, MessageKey.REGISTER_MESSAGE); + } else if (auth.getTotpKey() == null) { TotpGenerationResult createdTotpInfo = generateTotpService.generateTotpKey(player); - commonService.send(player, MessageKey.TWO_FACTOR_CREATE, + messages.send(player, MessageKey.TWO_FACTOR_CREATE, createdTotpInfo.getTotpKey(), createdTotpInfo.getAuthenticatorQrCodeUrl()); } else { - player.sendMessage(ChatColor.RED + "Two-factor authentication is already enabled for your account!"); + messages.send(player, MessageKey.TWO_FACTOR_ALREADY_ENABLED); } } } 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 34bdf56d2..52d1a9802 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,10 @@ 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.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; @@ -20,21 +23,29 @@ public class ConfirmTotpCommand extends PlayerCommand { @Inject private DataSource dataSource; + @Inject + private Messages messages; + @Override protected void runCommand(Player player, List arguments) { - // TODO #1141: Check if player already has TOTP + PlayerAuth auth = dataSource.getAuth(player.getName()); + if (auth == null) { + messages.send(player, MessageKey.REGISTER_MESSAGE); + } else if (auth.getTotpKey() != null) { + messages.send(player, MessageKey.TWO_FACTOR_ALREADY_ENABLED); + } final TotpGenerationResult totpDetails = generateTotpService.getGeneratedTotpKey(player); if (totpDetails == null) { - player.sendMessage("No TOTP key has been generated for you or it has expired. Please run /totp add"); + messages.send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_NO_CODE); } else { boolean isCodeValid = generateTotpService.isTotpCodeCorrectForGeneratedTotpKey(player, arguments.get(0)); if (isCodeValid) { generateTotpService.removeGenerateTotpKey(player); dataSource.setTotpKey(player.getName(), totpDetails.getTotpKey()); - player.sendMessage("Successfully enabled two-factor authentication for your account"); + messages.send(player, MessageKey.TWO_FACTOR_ENABLE_SUCCESS); } else { - player.sendMessage("Wrong code or code has expired. Please use /totp add again"); + messages.send(player, MessageKey.TWO_FACTOR_ENABLE_ERROR_WRONG_CODE); } } } 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 0a2a371d6..20ef4a1bf 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 @@ -3,6 +3,8 @@ 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.message.MessageKey; +import fr.xephi.authme.message.Messages; import fr.xephi.authme.security.totp.TotpService; import org.bukkit.entity.Player; @@ -20,17 +22,20 @@ public class RemoveTotpCommand extends PlayerCommand { @Inject private TotpService totpService; + @Inject + private Messages messages; + @Override protected void runCommand(Player player, List arguments) { PlayerAuth auth = dataSource.getAuth(player.getName()); if (auth.getTotpKey() == null) { - player.sendMessage("Two-factor authentication is not enabled for your account!"); + messages.send(player, MessageKey.TWO_FACTOR_NOT_ENABLED_ERROR); } else { if (totpService.verifyCode(auth, arguments.get(0))) { dataSource.removeTotpKey(auth.getNickname()); - player.sendMessage("Successfully removed two-factor authentication from your account"); + messages.send(player, MessageKey.TWO_FACTOR_REMOVED_SUCCESS); } else { - player.sendMessage("Invalid code!"); + messages.send(player, MessageKey.TWO_FACTOR_INVALID_CODE); } } } 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 74d85746f..d8b7a28f9 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 @@ -53,20 +53,19 @@ public class TotpCodeCommand extends PlayerCommand { } LimboPlayer limbo = limboService.getLimboPlayer(player.getName()); - if (limbo.getState() == LimboPlayerState.TOTP_REQUIRED) { - processCode(player, limbo, auth, arguments.get(0)); + if (limbo != null && limbo.getState() == LimboPlayerState.TOTP_REQUIRED) { + processCode(player, auth, arguments.get(0)); } else { messages.send(player, MessageKey.LOGIN_MESSAGE); } } - private void processCode(Player player, LimboPlayer limbo, PlayerAuth auth, String inputCode) { + private void processCode(Player player, 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!"); + messages.send(player, MessageKey.TWO_FACTOR_INVALID_CODE); } } } diff --git a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerState.java b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerState.java index 24d6cb0f8..5940ab206 100644 --- a/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerState.java +++ b/src/main/java/fr/xephi/authme/data/limbo/LimboPlayerState.java @@ -4,8 +4,6 @@ public enum LimboPlayerState { PASSWORD_REQUIRED, - TOTP_REQUIRED, - - FINISHED + TOTP_REQUIRED } diff --git a/src/main/java/fr/xephi/authme/message/MessageKey.java b/src/main/java/fr/xephi/authme/message/MessageKey.java index 90206ce7c..5d340964b 100644 --- a/src/main/java/fr/xephi/authme/message/MessageKey.java +++ b/src/main/java/fr/xephi/authme/message/MessageKey.java @@ -195,11 +195,32 @@ public enum MessageKey { EMAIL_ALREADY_USED_ERROR("email.already_used"), /** Your secret code is %code. You can scan it from here %url */ - TWO_FACTOR_CREATE("misc.two_factor_create", "%code", "%url"), + TWO_FACTOR_CREATE("two_factor.code_created", "%code", "%url"), /** Please submit your two-factor authentication code with /2fa code <code>. */ TWO_FACTOR_CODE_REQUIRED("two_factor.code_required"), + /** Two-factor authentication is already enabled for your account! */ + TWO_FACTOR_ALREADY_ENABLED("two_factor.already_enabled"), + + /** No 2fa key has been generated for you or it has expired. Please run /2fa add */ + TWO_FACTOR_ENABLE_ERROR_NO_CODE("two_factor.enable_error_no_code"), + + /** Successfully enabled two-factor authentication for your account */ + TWO_FACTOR_ENABLE_SUCCESS("two_factor.enable_success"), + + /** Wrong code or code has expired. Please run /2fa add */ + TWO_FACTOR_ENABLE_ERROR_WRONG_CODE("two_factor.enable_error_wrong_code"), + + /** Two-factor authentication is not enabled for your account. Run /2fa add */ + TWO_FACTOR_NOT_ENABLED_ERROR("two_factor.not_enabled_error"), + + /** Successfully removed two-factor auth from your account */ + TWO_FACTOR_REMOVED_SUCCESS("two_factor.removed_success"), + + /** Invalid code! */ + TWO_FACTOR_INVALID_CODE("two_factor.invalid_code"), + /** You are not the owner of this account. Please choose another name! */ NOT_OWNER_ERROR("on_join_validation.not_owner_error"), diff --git a/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java b/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java index ed31da3a7..d546df071 100644 --- a/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java +++ b/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java @@ -30,6 +30,13 @@ public class TotpAuthenticator { this.bukkitService = bukkitService; } + /** + * Returns whether the given input code matches for the provided TOTP key. + * + * @param totpKey the key to check with + * @param inputCode the input code to verify + * @return true if code is valid, false otherwise + */ public boolean checkCode(String totpKey, String inputCode) { try { Integer totpCode = Integer.valueOf(inputCode); diff --git a/src/main/resources/messages/messages_en.yml b/src/main/resources/messages/messages_en.yml index 1b5e09280..aca51fa75 100644 --- a/src/main/resources/messages/messages_en.yml +++ b/src/main/resources/messages/messages_en.yml @@ -56,7 +56,6 @@ unregister: misc: accounts_owned_self: 'You own %count accounts:' accounts_owned_other: 'The player %name has %count accounts:' - two_factor_create: '&2Your secret code is %code. You can scan it from here %url' account_not_activated: '&cYour account isn''t activated yet, please check your emails!' password_changed: '&2Password changed successfully!' logout: '&2Logged out successfully!' @@ -130,7 +129,15 @@ verification: email_needed: '&3To verify your identity you need to link an email address with your account!!' two_factor: + code_created: '&2Your secret code is %code. You can scan it from here %url' code_required: 'Please submit your two-factor authentication code with /2fa code ' + already_enabled: 'Two-factor authentication is already enabled for your account!' + enable_error_no_code: 'No 2fa key has been generated for you or it has expired. Please run /2fa add' + enable_success: 'Successfully enabled two-factor authentication for your account' + enable_error_wrong_code: 'Wrong code or code has expired. Please run /2fa add' + not_enabled_error: 'Two-factor authentication is not enabled for your account. Run /2fa add' + removed_success: 'Successfully removed two-factor auth from your account' + invalid_code: 'Invalid code!' # Time units time: diff --git a/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java b/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java index eb2746fa0..b675ef8a0 100644 --- a/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java +++ b/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java @@ -4,9 +4,9 @@ import com.warrenstrange.googleauth.IGoogleAuthenticator; import fr.xephi.authme.security.totp.TotpAuthenticator.TotpGenerationResult; import fr.xephi.authme.service.BukkitService; import org.bukkit.entity.Player; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -26,7 +26,6 @@ import static org.mockito.Mockito.verifyZeroInteractions; @RunWith(MockitoJUnitRunner.class) public class TotpAuthenticatorTest { - @InjectMocks private TotpAuthenticator totpAuthenticator; @Mock @@ -35,6 +34,11 @@ public class TotpAuthenticatorTest { @Mock private IGoogleAuthenticator googleAuthenticator; + @Before + public void initializeTotpAuthenticator() { + totpAuthenticator = new TotpAuthenticator(googleAuthenticator, bukkitService); + } + @Test public void shouldGenerateTotpKey() { // given diff --git a/src/test/resources/fr/xephi/authme/message/help_test.yml b/src/test/resources/fr/xephi/authme/message/help_test.yml index 9647ecd7a..d6970daa7 100644 --- a/src/test/resources/fr/xephi/authme/message/help_test.yml +++ b/src/test/resources/fr/xephi/authme/message/help_test.yml @@ -65,9 +65,6 @@ commands: arg1: label: loginArg nonExistent: does not exist - arg2: - label: 2faArg - rhetorical: false someProp: label: '''someProp'' does not exist' description: idk