From 9326094d9c1620845b5aa837a683583b0df092c7 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Tue, 3 Apr 2018 00:12:25 +0200 Subject: [PATCH] #1141 Fix review remarks by @games647 - Use SHA512 to generate keys instead of default SHA1 - Declare google authenticator dependency as optional and add relocation rule --- pom.xml | 17 +++++++--------- .../fr/xephi/authme/message/MessageKey.java | 2 +- .../security/totp/TotpAuthenticator.java | 20 ++++++++++++------- .../security/totp/TotpAuthenticatorTest.java | 14 ++++++++++++- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 0cd2db1d..703b9fa6 100644 --- a/pom.xml +++ b/pom.xml @@ -251,16 +251,8 @@ fr.xephi.authme.libs.com.google - ch.jalu.injector - fr.xephi.authme.libs.jalu.injector - - - ch.jalu.configme - fr.xephi.authme.libs.ch.jalu.configme - - - ch.jalu.datasourcecolumns - fr.xephi.authme.libs.ch.jalu.datasourcecolumns + ch.jalu + fr.xephi.authme.libs.ch.jalu com.zaxxer.hikari @@ -290,6 +282,10 @@ de.mkammerer fr.xephi.authme.libs.de.mkammerer + + com.warrenstrange + fr.xephi.authme.libs.com.warrenstrange + javax.inject fr.xephi.authme.libs.javax.inject @@ -482,6 +478,7 @@ com.warrenstrange googleauth 1.1.2 + true diff --git a/src/main/java/fr/xephi/authme/message/MessageKey.java b/src/main/java/fr/xephi/authme/message/MessageKey.java index 5d340964..538b5259 100644 --- a/src/main/java/fr/xephi/authme/message/MessageKey.java +++ b/src/main/java/fr/xephi/authme/message/MessageKey.java @@ -197,7 +197,7 @@ public enum MessageKey { /** Your secret code is %code. You can scan it from here %url */ TWO_FACTOR_CREATE("two_factor.code_created", "%code", "%url"), - /** Please submit your two-factor authentication code with /2fa code <code>. */ + /** 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! */ 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 d546df07..cffc09cd 100644 --- a/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java +++ b/src/main/java/fr/xephi/authme/security/totp/TotpAuthenticator.java @@ -1,9 +1,11 @@ package fr.xephi.authme.security.totp; -import com.google.common.annotations.VisibleForTesting; import com.warrenstrange.googleauth.GoogleAuthenticator; +import com.warrenstrange.googleauth.GoogleAuthenticatorConfig; +import com.warrenstrange.googleauth.GoogleAuthenticatorConfig.GoogleAuthenticatorConfigBuilder; import com.warrenstrange.googleauth.GoogleAuthenticatorKey; import com.warrenstrange.googleauth.GoogleAuthenticatorQRGenerator; +import com.warrenstrange.googleauth.HmacHashFunction; import com.warrenstrange.googleauth.IGoogleAuthenticator; import fr.xephi.authme.service.BukkitService; import org.bukkit.entity.Player; @@ -18,16 +20,20 @@ public class TotpAuthenticator { private final IGoogleAuthenticator authenticator; private final BukkitService bukkitService; - @Inject TotpAuthenticator(BukkitService bukkitService) { - this(new GoogleAuthenticator(), bukkitService); + this.authenticator = createGoogleAuthenticator(); + this.bukkitService = bukkitService; } - @VisibleForTesting - TotpAuthenticator(IGoogleAuthenticator authenticator, BukkitService bukkitService) { - this.authenticator = authenticator; - this.bukkitService = bukkitService; + /** + * @return new Google Authenticator instance + */ + protected IGoogleAuthenticator createGoogleAuthenticator() { + GoogleAuthenticatorConfig config = new GoogleAuthenticatorConfigBuilder() + .setHmacHashFunction(HmacHashFunction.HmacSHA512) + .build(); + return new GoogleAuthenticator(config); } /** 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 b675ef8a..27434cce 100644 --- a/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java +++ b/src/test/java/fr/xephi/authme/security/totp/TotpAuthenticatorTest.java @@ -36,7 +36,7 @@ public class TotpAuthenticatorTest { @Before public void initializeTotpAuthenticator() { - totpAuthenticator = new TotpAuthenticator(googleAuthenticator, bukkitService); + totpAuthenticator = new TotpAuthenticatorTestImpl(bukkitService); } @Test @@ -85,4 +85,16 @@ public class TotpAuthenticatorTest { assertThat(result, equalTo(false)); verifyZeroInteractions(googleAuthenticator); } + + private final class TotpAuthenticatorTestImpl extends TotpAuthenticator { + + TotpAuthenticatorTestImpl(BukkitService bukkitService) { + super(bukkitService); + } + + @Override + protected IGoogleAuthenticator createGoogleAuthenticator() { + return googleAuthenticator; + } + } }