diff --git a/docs/config.md b/docs/config.md index 4bf54286b..efe739cd3 100644 --- a/docs/config.md +++ b/docs/config.md @@ -1,5 +1,5 @@ - + ## AuthMe Configuration The first time you run AuthMe it will create a config.yml file in the plugins/AuthMe folder, @@ -245,10 +245,12 @@ settings: passwordHash: 'SHA256' # Salt length for the SALTED2MD5 MD5(MD5(password)+salt) doubleMD5SaltLength: 8 - # If password checking return false, do we need to check with all - # other password algorithm to check an old password? - # AuthMe will update the password to the new password hash - supportOldPasswordHash: false + # If a password check fails, AuthMe will also try to check with the following hash methods. + # Use this setting when you change from one hash method to another. + # AuthMe will update the password to the new hash. Example: + # legacyHashes: + # - 'SHA1' + legacyHashes: [] # Prevent unsafe passwords from being used; put them in lowercase! # You should always set 'help' as unsafePassword due to possible conflicts. # unsafePasswords: @@ -461,4 +463,4 @@ To change settings on a running server, save your changes to config.yml and use --- -This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sun Oct 30 12:57:15 CET 2016 +This page was automatically generated on the [AuthMe/AuthMeReloaded repository](https://github.com/AuthMe/AuthMeReloaded/tree/master/docs/) on Sun Nov 13 10:33:55 CET 2016 diff --git a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java index 521decfae..8549b388b 100644 --- a/src/main/java/fr/xephi/authme/security/PasswordSecurity.java +++ b/src/main/java/fr/xephi/authme/security/PasswordSecurity.java @@ -12,6 +12,7 @@ import org.bukkit.plugin.PluginManager; import javax.annotation.PostConstruct; import javax.inject.Inject; +import java.util.Collection; /** * Manager class for password-related operations. @@ -31,7 +32,7 @@ public class PasswordSecurity implements Reloadable { private Injector injector; private HashAlgorithm algorithm; - private boolean supportOldAlgorithm; + private Collection legacyAlgorithms; /** * Load or reload the configuration. @@ -40,7 +41,7 @@ public class PasswordSecurity implements Reloadable { @Override public void reload() { this.algorithm = settings.getProperty(SecuritySettings.PASSWORD_HASH); - this.supportOldAlgorithm = settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH); + this.legacyAlgorithms = settings.getProperty(SecuritySettings.LEGACY_HASHES); } /** @@ -83,7 +84,7 @@ public class PasswordSecurity implements Reloadable { EncryptionMethod method = initializeEncryptionMethodWithEvent(algorithm, playerName); String playerLowerCase = playerName.toLowerCase(); return methodMatches(method, password, hashedPassword, playerLowerCase) - || supportOldAlgorithm && compareWithAllEncryptionMethods(password, hashedPassword, playerLowerCase); + || compareWithLegacyHashes(password, hashedPassword, playerLowerCase); } /** @@ -97,14 +98,12 @@ public class PasswordSecurity implements Reloadable { * * @return True if there was a password match with another encryption method, false otherwise */ - private boolean compareWithAllEncryptionMethods(String password, HashedPassword hashedPassword, String playerName) { - for (HashAlgorithm algorithm : HashAlgorithm.values()) { - if (!HashAlgorithm.CUSTOM.equals(algorithm)) { - EncryptionMethod method = initializeEncryptionMethod(algorithm); - if (methodMatches(method, password, hashedPassword, playerName)) { - hashPasswordForNewAlgorithm(password, playerName); - return true; - } + private boolean compareWithLegacyHashes(String password, HashedPassword hashedPassword, String playerName) { + for (HashAlgorithm algorithm : legacyAlgorithms) { + EncryptionMethod method = initializeEncryptionMethod(algorithm); + if (methodMatches(method, password, hashedPassword, playerName)) { + hashPasswordForNewAlgorithm(password, playerName); + return true; } } return false; diff --git a/src/main/java/fr/xephi/authme/settings/EnumSetProperty.java b/src/main/java/fr/xephi/authme/settings/EnumSetProperty.java new file mode 100644 index 000000000..4ca284d72 --- /dev/null +++ b/src/main/java/fr/xephi/authme/settings/EnumSetProperty.java @@ -0,0 +1,43 @@ +package fr.xephi.authme.settings; + +import com.github.authme.configme.properties.Property; +import com.github.authme.configme.resource.PropertyResource; + +import java.util.List; +import java.util.stream.Collectors; + +/** + * Property whose value is a set of entries of a given enum. + */ +// TODO https://github.com/AuthMe/ConfigMe/issues/27: Should be a Property of Set type +public class EnumSetProperty> extends Property> { + + private final Class enumClass; + + public EnumSetProperty(Class enumClass, String path, List defaultValue) { + super(path, defaultValue); + this.enumClass = enumClass; + } + + @Override + protected List getFromResource(PropertyResource resource) { + List elements = resource.getList(getPath()); + if (elements != null) { + return elements.stream() + .map(val -> toEnum(String.valueOf(val))) + .filter(e -> e != null) + .distinct() + .collect(Collectors.toList()); + } + return null; + } + + private E toEnum(String str) { + for (E e : enumClass.getEnumConstants()) { + if (str.equalsIgnoreCase(e.name())) { + return e; + } + } + return null; + } +} diff --git a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java index 30163bf0f..954116a4a 100644 --- a/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java +++ b/src/main/java/fr/xephi/authme/settings/SettingsMigrationService.java @@ -7,6 +7,7 @@ import com.google.common.base.Objects; import fr.xephi.authme.ConsoleLogger; import fr.xephi.authme.output.LogLevel; import fr.xephi.authme.settings.properties.PluginSettings; +import fr.xephi.authme.settings.properties.SecuritySettings; import java.io.File; import java.io.FileWriter; @@ -49,6 +50,7 @@ public class SettingsMigrationService extends PlainMigrationService { | migrateForceSpawnSettings(resource) | changeBooleanSettingToLogLevelProperty(resource) | hasOldHelpHeaderProperty(resource) + | hasSupportOldPasswordProperty(resource) || hasDeprecatedProperties(resource); } @@ -163,6 +165,16 @@ public class SettingsMigrationService extends PlainMigrationService { return false; } + private static boolean hasSupportOldPasswordProperty(PropertyResource resource) { + String path = "settings.security.supportOldPasswordHash"; + if (resource.contains(path)) { + ConsoleLogger.warning("Property '" + path + "' is no longer supported. " + + "Use '" + SecuritySettings.LEGACY_HASHES.getPath() + "' instead."); + return true; + } + return false; + } + /** * Checks for an old property path and moves it to a new path if present. * diff --git a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java index 92ce2e5ec..e8035491b 100644 --- a/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java +++ b/src/main/java/fr/xephi/authme/settings/properties/SecuritySettings.java @@ -4,7 +4,9 @@ import com.github.authme.configme.Comment; import com.github.authme.configme.SettingsHolder; import com.github.authme.configme.properties.Property; import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.settings.EnumSetProperty; +import java.util.Collections; import java.util.List; import static com.github.authme.configme.properties.PropertyInitializer.newLowercaseListProperty; @@ -74,11 +76,15 @@ public class SecuritySettings implements SettingsHolder { public static final Property DOUBLE_MD5_SALT_LENGTH = newProperty("settings.security.doubleMD5SaltLength", 8); - @Comment({"If password checking return false, do we need to check with all", - "other password algorithm to check an old password?", - "AuthMe will update the password to the new password hash"}) - public static final Property SUPPORT_OLD_PASSWORD_HASH = - newProperty("settings.security.supportOldPasswordHash", false); + @Comment({ + "If a password check fails, AuthMe will also try to check with the following hash methods.", + "Use this setting when you change from one hash method to another.", + "AuthMe will update the password to the new hash. Example:", + "legacyHashes:", + "- 'SHA1'" + }) + public static final Property> LEGACY_HASHES = + new EnumSetProperty<>(HashAlgorithm.class, "settings.security.legacyHashes", Collections.emptyList()); @Comment({"Prevent unsafe passwords from being used; put them in lowercase!", "You should always set 'help' as unsafePassword due to possible conflicts.", diff --git a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java index 1911bb39a..ec206c869 100644 --- a/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java +++ b/src/test/java/fr/xephi/authme/security/PasswordSecurityTest.java @@ -24,6 +24,10 @@ import org.mockito.invocation.InvocationOnMock; import org.mockito.runners.MockitoJUnitRunner; import org.mockito.stubbing.Answer; +import java.util.Collections; +import java.util.List; + +import static com.google.common.collect.Lists.newArrayList; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.equalToIgnoringCase; import static org.junit.Assert.assertThat; @@ -97,7 +101,7 @@ public class PasswordSecurityTest { given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(true); - initSettings(HashAlgorithm.BCRYPT, false); + initSettings(HashAlgorithm.BCRYPT); PasswordSecurity security = newPasswordSecurity(); // when @@ -120,7 +124,7 @@ public class PasswordSecurityTest { given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); - initSettings(HashAlgorithm.CUSTOM, false); + initSettings(HashAlgorithm.CUSTOM); PasswordSecurity security = newPasswordSecurity(); // when @@ -140,7 +144,7 @@ public class PasswordSecurityTest { String clearTextPass = "tables"; given(dataSource.getPassword(playerName)).willReturn(null); - initSettings(HashAlgorithm.MD5, false); + initSettings(HashAlgorithm.MD5); PasswordSecurity security = newPasswordSecurity(); // when @@ -168,7 +172,8 @@ public class PasswordSecurityTest { given(dataSource.getPassword(argThat(equalToIgnoringCase(playerName)))).willReturn(password); given(method.comparePassword(clearTextPass, password, playerLowerCase)).willReturn(false); given(method.computeHash(clearTextPass, playerLowerCase)).willReturn(newPassword); - initSettings(HashAlgorithm.MD5, true); + initSettings(HashAlgorithm.MD5); + given(settings.getProperty(SecuritySettings.LEGACY_HASHES)).willReturn(newArrayList(HashAlgorithm.BCRYPT)); PasswordSecurity security = newPasswordSecurity(); // when @@ -193,7 +198,7 @@ public class PasswordSecurityTest { String clearTextPass = "someInvalidPassword"; given(dataSource.getPassword(playerName)).willReturn(password); given(method.comparePassword(clearTextPass, password, playerName)).willReturn(false); - initSettings(HashAlgorithm.MD5, true); + initSettings(HashAlgorithm.MD5); PasswordSecurity security = newPasswordSecurity(); // when @@ -212,7 +217,7 @@ public class PasswordSecurityTest { String usernameLowerCase = username.toLowerCase(); HashedPassword hashedPassword = new HashedPassword("$T$est#Hash", "__someSalt__"); given(method.computeHash(password, usernameLowerCase)).willReturn(hashedPassword); - initSettings(HashAlgorithm.JOOMLA, true); + initSettings(HashAlgorithm.JOOMLA); PasswordSecurity security = newPasswordSecurity(); // when @@ -234,7 +239,7 @@ public class PasswordSecurityTest { String username = "someone12"; HashedPassword hashedPassword = new HashedPassword("~T!est#Hash"); given(method.hasSeparateSalt()).willReturn(true); - initSettings(HashAlgorithm.XAUTH, false); + initSettings(HashAlgorithm.XAUTH); PasswordSecurity security = newPasswordSecurity(); // when @@ -250,19 +255,20 @@ public class PasswordSecurityTest { @Test public void shouldReloadSettings() { // given - initSettings(HashAlgorithm.BCRYPT, false); + initSettings(HashAlgorithm.BCRYPT); PasswordSecurity passwordSecurity = newPasswordSecurity(); given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.MD5); - given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(true); + List legacyHashes = newArrayList(HashAlgorithm.CUSTOM, HashAlgorithm.BCRYPT); + given(settings.getProperty(SecuritySettings.LEGACY_HASHES)).willReturn(legacyHashes); // when passwordSecurity.reload(); // then assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "algorithm"), - equalTo((Object) HashAlgorithm.MD5)); - assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "supportOldAlgorithm"), - equalTo((Object) Boolean.TRUE)); + equalTo(HashAlgorithm.MD5)); + assertThat(ReflectionTestUtils.getFieldValue(PasswordSecurity.class, passwordSecurity, "legacyAlgorithms"), + equalTo(legacyHashes)); } private PasswordSecurity newPasswordSecurity() { @@ -275,11 +281,10 @@ public class PasswordSecurityTest { return passwordSecurity; } - private void initSettings(HashAlgorithm algorithm, boolean supportOldPassword) { + private void initSettings(HashAlgorithm algorithm) { given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(algorithm); - given(settings.getProperty(SecuritySettings.SUPPORT_OLD_PASSWORD_HASH)).willReturn(supportOldPassword); + given(settings.getProperty(SecuritySettings.LEGACY_HASHES)).willReturn(Collections.emptyList()); given(settings.getProperty(HooksSettings.BCRYPT_LOG2_ROUND)).willReturn(8); - given(settings.getProperty(SecuritySettings.DOUBLE_MD5_SALT_LENGTH)).willReturn(16); } }