From 8d489efffdd6017951bb73ce99056c71195bda44 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Fri, 29 Apr 2016 21:58:32 +0200 Subject: [PATCH] #683 Plaintext to SHA256: Add warning message + skip SHA hashes - Add message not to stop the server before conversion finishes - Skip hashes starting with $SHA$ during conversion - Create unit tests --- .../xephi/authme/util/MigrationService.java | 14 +- .../authme/util/MigrationServiceTest.java | 129 ++++++++++++++++++ 2 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 src/test/java/fr/xephi/authme/util/MigrationServiceTest.java diff --git a/src/main/java/fr/xephi/authme/util/MigrationService.java b/src/main/java/fr/xephi/authme/util/MigrationService.java index 86f98e4c1..6d86adbe3 100644 --- a/src/main/java/fr/xephi/authme/util/MigrationService.java +++ b/src/main/java/fr/xephi/authme/util/MigrationService.java @@ -37,12 +37,18 @@ public final class MigrationService { if (HashAlgorithm.PLAINTEXT == settings.getProperty(SecuritySettings.PASSWORD_HASH)) { ConsoleLogger.showError("Your HashAlgorithm has been detected as plaintext and is now deprecated;" + " it will be changed and hashed now to the AuthMe default hashing method"); + ConsoleLogger.showError("Don't stop your server; wait for the conversion to have been completed!"); List allAuths = dataSource.getAllAuths(); for (PlayerAuth auth : allAuths) { - HashedPassword hashedPassword = authmeSha256.computeHash( - auth.getPassword().getHash(), auth.getNickname()); - auth.setPassword(hashedPassword); - dataSource.updatePassword(auth); + String hash = auth.getPassword().getHash(); + if (hash.startsWith("$SHA$")) { + ConsoleLogger.showError("Skipping conversion for " + auth.getNickname() + "; detected SHA hash"); + } else { + HashedPassword hashedPassword = authmeSha256.computeHash( + auth.getPassword().getHash(), auth.getNickname()); + auth.setPassword(hashedPassword); + dataSource.updatePassword(auth); + } } settings.setProperty(SecuritySettings.PASSWORD_HASH, HashAlgorithm.SHA256); settings.save(); diff --git a/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java b/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java new file mode 100644 index 000000000..d27f642ec --- /dev/null +++ b/src/test/java/fr/xephi/authme/util/MigrationServiceTest.java @@ -0,0 +1,129 @@ +package fr.xephi.authme.util; + +import fr.xephi.authme.TestHelper; +import fr.xephi.authme.cache.auth.PlayerAuth; +import fr.xephi.authme.datasource.DataSource; +import fr.xephi.authme.security.HashAlgorithm; +import fr.xephi.authme.security.crypts.HashedPassword; +import fr.xephi.authme.security.crypts.SHA256; +import fr.xephi.authme.settings.NewSetting; +import fr.xephi.authme.settings.properties.SecuritySettings; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.stubbing.Answer; + +import java.util.Arrays; + +import static fr.xephi.authme.AuthMeMatchers.equalToHash; +import static org.hamcrest.Matchers.equalToIgnoringCase; +import static org.junit.Assert.assertThat; +import static org.mockito.BDDMockito.given; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +/** + * Test for {@link MigrationService}. + */ +@RunWith(MockitoJUnitRunner.class) +public class MigrationServiceTest { + + @Mock + private NewSetting settings; + + @Mock + private DataSource dataSource; + + @Mock + private SHA256 sha256; + + @BeforeClass + public static void setUpLogger() { + TestHelper.setupLogger(); + } + + @Test + public void shouldMigratePlaintextHashes() { + // given + PlayerAuth auth1 = authWithNickAndHash("bobby", "test"); + PlayerAuth auth2 = authWithNickAndHash("user", "myPassword"); + PlayerAuth auth3 = authWithNickAndHash("Tester12", "$tester12_pw"); + given(dataSource.getAllAuths()).willReturn(Arrays.asList(auth1, auth2, auth3)); + setSha256MockToUppercase(sha256); + given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.PLAINTEXT); + + // when + MigrationService.changePlainTextToSha256(settings, dataSource, sha256); + + // then + verify(sha256, times(3)).computeHash(anyString(), anyString()); + verify(dataSource).getAllAuths(); // need to verify this because we use verifyNoMoreInteractions() after + verify(dataSource).updatePassword(auth1); + assertThat(auth1.getPassword(), equalToHash("TEST")); + verify(dataSource).updatePassword(auth2); + assertThat(auth2.getPassword(), equalToHash("MYPASSWORD")); + verify(dataSource).updatePassword(auth3); + assertThat(auth3.getPassword(), equalToHash("$TESTER12_PW")); + verifyNoMoreInteractions(dataSource); + verify(settings).setProperty(SecuritySettings.PASSWORD_HASH, HashAlgorithm.SHA256); + } + + @Test + public void shouldNotMigrateShaHashes() { + // given + PlayerAuth auth1 = authWithNickAndHash("testUser", "abc1234"); + PlayerAuth auth2 = authWithNickAndHash("minecraft", "$SHA$f28930ae09823eba4cd98a3"); + given(dataSource.getAllAuths()).willReturn(Arrays.asList(auth1, auth2)); + setSha256MockToUppercase(sha256); + given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.PLAINTEXT); + + // when + MigrationService.changePlainTextToSha256(settings, dataSource, sha256); + + // then + verify(sha256).computeHash(eq("abc1234"), argThat(equalToIgnoringCase("testUser"))); + verifyNoMoreInteractions(sha256); + verify(dataSource).getAllAuths(); // need to verify this because we use verifyNoMoreInteractions() after + verify(dataSource).updatePassword(auth1); + assertThat(auth1.getPassword(), equalToHash("ABC1234")); + verifyNoMoreInteractions(dataSource); + verify(settings).setProperty(SecuritySettings.PASSWORD_HASH, HashAlgorithm.SHA256); + } + + @Test + public void shouldNotMigrateForHashOtherThanPlaintext() { + // given + given(settings.getProperty(SecuritySettings.PASSWORD_HASH)).willReturn(HashAlgorithm.BCRYPT); + + // when + MigrationService.changePlainTextToSha256(settings, dataSource, sha256); + + // then + verify(settings).getProperty(SecuritySettings.PASSWORD_HASH); + verifyNoMoreInteractions(settings, dataSource, sha256); + } + + private static PlayerAuth authWithNickAndHash(String nick, String hash) { + return PlayerAuth.builder() + .name(nick) + .password(hash, null) + .build(); + } + + private static void setSha256MockToUppercase(SHA256 sha256) { + given(sha256.computeHash(anyString(), anyString())).willAnswer(new Answer() { + @Override + public HashedPassword answer(InvocationOnMock invocation) { + String plainPassword = (String) invocation.getArguments()[0]; + return new HashedPassword(plainPassword.toUpperCase(), null); + } + }); + } +}