Added support for legacy passwords

src/Dao/EntityConverters/UserEntityConverter.php

@@ -12,6 +12,7 @@ class UserEntityConverter extends AbstractEntityConverter implements IEntityConv
 			'email' => $entity->getEmail(),
 			'emailUnconfirmed' => $entity->getEmailUnconfirmed(),
 			'passwordHash' => $entity->getPasswordHash(),
+			'passwordSalt' => $entity->getPasswordSalt(),
 			'accessRank' => $entity->getAccessRank(),
 			'registrationTime' => $this->entityTimeToDbTime($entity->getRegistrationTime()),
 			'lastLoginTime' => $this->entityTimeToDbTime($entity->getLastLoginTime()),
@@ -29,6 +30,7 @@ class UserEntityConverter extends AbstractEntityConverter implements IEntityConv
 		$entity->setEmail($array['email']);
 		$entity->setEmailUnconfirmed($array['emailUnconfirmed']);
 		$entity->setPasswordHash($array['passwordHash']);
+		$entity->setPasswordSalt($array['passwordSalt']);
 		$entity->setAccessRank(intval($array['accessRank']));
 		$entity->setRegistrationTime($this->dbTimeToEntityTime($array['registrationTime']));
 		$entity->setLastLoginTime($this->dbTimeToEntityTime($array['lastLoginTime']));

src/Entities/User.php

@@ -20,6 +20,7 @@ final class User extends Entity
 	protected $email;
 	protected $emailUnconfirmed;
 	protected $passwordHash;
+	protected $passwordSalt;
 	protected $accessRank;
 	protected $registrationTime;
 	protected $lastLoginTime;
@@ -88,6 +89,16 @@ final class User extends Entity
 		$this->passwordHash = $passwordHash;
 	}
 
+	public function getPasswordSalt()
+	{
+		return $this->passwordSalt;
+	}
+
+	public function setPasswordSalt($passwordSalt)
+	{
+		$this->passwordSalt = $passwordSalt;
+	}
+
 	public function getAccessRank()
 	{
 		return $this->accessRank;

src/Services/AuthService.php

@@ -48,8 +48,12 @@ class AuthService
 		$user = $this->userService->getByNameOrEmail($formData->userNameOrEmail);
 		$this->doFinalChecksOnUser($user);
 
-		$passwordHash = $this->passwordService->getHash($formData->password);
+		$hashValid = $this->passwordService->isHashValid(
-		if ($user->getPasswordHash() !== $passwordHash)
+			$formData->password,
+			$user->getPasswordSalt(),
+			$user->getPasswordHash());
+
+		if (!$hashValid)
 			throw new \InvalidArgumentException('Specified password is invalid.');
 
 		$this->loginToken = $this->createAndSaveLoginToken($user);

src/Services/PasswordService.php

@@ -19,9 +19,25 @@ class PasswordService
 		$this->pattern = str_split('cvcvnncvcv');
 	}
 
-	public function getHash($password)
+	public function getLegacyHash($password, $salt)
 	{
-		return hash('sha256', $this->config->security->secret . '/' . $password);
+		//hash used by old szurubooru version
+		return sha1('1A2/$_4xVa' . $salt . $password);
+	}
+
+	public function getHash($password, $salt)
+	{
+		return hash('sha256', $this->config->security->secret . $salt . $password);
+	}
+
+	public function isHashValid($password, $salt, $expectedPasswordHash)
+	{
+		$hashes =
+		[
+			$this->getLegacyHash($password, $salt),
+			$this->getHash($password, $salt),
+		];
+		return in_array($expectedPasswordHash, $hashes);
 	}
 
 	public function getRandomPassword()

src/Services/UserService.php

@@ -100,6 +100,7 @@ class UserService
 			$user->setAccessRank($this->userDao->hasAnyUsers()
 				? \Szurubooru\Entities\User::ACCESS_RANK_REGULAR_USER
 				: \Szurubooru\Entities\User::ACCESS_RANK_ADMINISTRATOR);
+			$user->setPasswordSalt($this->passwordService->getRandomPassword());
 
 			$this->updateUserName($user, $formData->userName);
 			$this->updateUserPassword($user, $formData->password);
@@ -173,7 +174,7 @@ class UserService
 
 			$user = $this->getByName($token->getAdditionalData());
 			$newPassword = $this->passwordService->getRandomPassword();
-			$user->setPasswordHash($this->passwordService->getHash($newPassword));
+			$this->updateUserPassword($user, $newPassword);
 			$this->userDao->save($user);
 			$this->tokenService->invalidateByName($token->getName());
 			return $newPassword;
@@ -231,7 +232,7 @@ class UserService
 
 	private function updateUserPassword(\Szurubooru\Entities\User $user, $newPassword)
 	{
-		$user->setPasswordHash($this->passwordService->getHash($newPassword));
+		$user->setPasswordHash($this->passwordService->getHash($newPassword, $user->getPasswordSalt()));
 	}
 
 	private function updateUserEmail(\Szurubooru\Entities\User $user, $newEmail)

src/Upgrades/Upgrade17.php

@@ -0,0 +1,13 @@
+<?php
+namespace Szurubooru\Upgrades;
+
+class Upgrade17 implements IUpgrade
+{
+	public function run(\Szurubooru\DatabaseConnection $databaseConnection)
+	{
+		$pdo = $databaseConnection->getPDO();
+
+		$pdo->exec('ALTER TABLE users ADD COLUMN passwordSalt VARCHAR(32)');
+		$pdo->exec('UPDATE users SET passwordSalt = "/"');
+	}
+}

src/di.php

@@ -32,6 +32,7 @@ return [
 			$container->get(\Szurubooru\Upgrades\Upgrade14::class),
 			$container->get(\Szurubooru\Upgrades\Upgrade15::class),
 			$container->get(\Szurubooru\Upgrades\Upgrade16::class),
+			$container->get(\Szurubooru\Upgrades\Upgrade17::class),
 		];
 	}),
 

tests/Services/AuthServiceTest.php

@@ -22,11 +22,12 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 	public function testInvalidPassword()
 	{
 		$this->configMock->set('security/needEmailActivationToRegister', false);
-		$this->passwordServiceMock->expects($this->once())->method('getHash')->willReturn('unmatchingHash');
+		$this->passwordServiceMock->expects($this->once())->method('isHashValid')->with('godzilla', 'salt', 'hash')->willReturn(false);
 
 		$testUser = new \Szurubooru\Entities\User();
 		$testUser->setName('dummy');
 		$testUser->setPasswordHash('hash');
+		$testUser->setPasswordSalt('salt');
 		$this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser);
 
 		$this->setExpectedException(\Exception::class, 'Specified password is invalid');
@@ -40,11 +41,12 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 	public function testValidCredentials()
 	{
 		$this->configMock->set('security/needEmailActivationToRegister', false);
-		$this->passwordServiceMock->expects($this->once())->method('getHash')->willReturn('hash');
+		$this->passwordServiceMock->expects($this->once())->method('isHashValid')->with('godzilla', 'salt', 'hash')->willReturn(true);
 
 		$testUser = new \Szurubooru\Entities\User('an unusual database identifier');
 		$testUser->setName('dummy');
 		$testUser->setPasswordHash('hash');
+		$testUser->setPasswordSalt('salt');
 		$this->userServiceMock->expects($this->once())->method('getByNameOrEmail')->willReturn($testUser);
 
 		$testToken = new \Szurubooru\Entities\Token();
@@ -68,7 +70,7 @@ class AuthServiceTest extends \Szurubooru\Tests\AbstractTestCase
 	public function testValidCredentialsUnconfirmedEmail()
 	{
 		$this->configMock->set('security/needEmailActivationToRegister', true);
-		$this->passwordServiceMock->expects($this->never())->method('getHash')->willReturn('hash');
+		$this->passwordServiceMock->expects($this->never())->method('isHashValid')->willReturn('hash');
 
 		$testUser = new \Szurubooru\Entities\User();
 		$testUser->setName('dummy');

tests/Services/PasswordServiceTest.php

@@ -3,10 +3,31 @@ namespace Szurubooru\Tests\Service;
 
 class PasswordServiceTest extends \Szurubooru\Tests\AbstractTestCase
 {
+	private $configMock;
+
+	public function setUp()
+	{
+		parent::setUp();
+		$this->configMock = $this->mockConfig();
+	}
+
+	public function testLegacyPasswordValidation()
+	{
+		$passwordService = $this->getPasswordService();
+		$this->configMock->set('security/secret', 'doesnt matter');
+		$this->assertTrue($passwordService->isHashValid('testt', 'ac63e0bcdf20b82db509d123166c4592', '2602572e077d48b35af39d1cff84bfcaa5363116'));
+	}
+
+	public function testPasswordValidation()
+	{
+		$passwordService = $this->getPasswordService();
+		$this->configMock->set('security/secret', 'change');
+		$this->assertTrue($passwordService->isHashValid('testt', '/', '4f4f8b836cd65f3f1d0b7751fc442f79595c23439cd8a928af15e10807bf08cc'));
+	}
+
 	public function testGeneratingPasswords()
 	{
-		$configMock = $this->mockConfig();
+		$passwordService = $this->getPasswordService();
-		$passwordService = new \Szurubooru\Services\PasswordService($configMock);
 
 		$sampleCount = 10000;
 		$distribution = [];
@@ -44,4 +65,9 @@ class PasswordServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$mean = array_sum($sample) / count($sample);
 		return 100 * $this->getStandardDeviation($sample) / $mean;
 	}
+
+	private function getPasswordService()
+	{
+		return new \Szurubooru\Services\PasswordService($this->configMock);
+	}
 }

tests/Services/UserServiceTest.php

@@ -73,7 +73,8 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$formData->email = 'human@people.gov';
 
 		$this->configMock->set('security/needEmailActivationToRegister', false);
-		$this->passwordServiceMock->expects($this->once())->method('getHash')->willReturn('hash');
+		$this->passwordServiceMock->expects($this->once())->method('getRandomPassword')->willReturn('salt');
+		$this->passwordServiceMock->expects($this->once())->method('getHash')->with('password', 'salt')->willReturn('hash');
 		$this->timeServiceMock->expects($this->once())->method('getCurrentTime')->willReturn('now');
 		$this->userDaoMock->expects($this->once())->method('hasAnyUsers')->willReturn(true);
 		$this->userDaoMock->expects($this->once())->method('save')->will($this->returnArgument(0));
@@ -99,7 +100,8 @@ final class UserServiceTest extends \Szurubooru\Tests\AbstractTestCase
 		$formData->email = 'human@people.gov';
 
 		$this->configMock->set('security/needEmailActivationToRegister', true);
-		$this->passwordServiceMock->expects($this->once())->method('getHash')->willReturn('hash');
+		$this->passwordServiceMock->expects($this->once())->method('getRandomPassword')->willReturn('salt');
+		$this->passwordServiceMock->expects($this->once())->method('getHash')->with('password', 'salt')->willReturn('hash');
 		$this->timeServiceMock->expects($this->once())->method('getCurrentTime')->willReturn('now');
 		$this->userDaoMock->expects($this->once())->method('hasAnyUsers')->willReturn(true);
 		$this->userDaoMock->expects($this->once())->method('save')->will($this->returnArgument(0));