From ba5e7a1bf5431632cef56aaee507bb8360e0fba2 Mon Sep 17 00:00:00 2001 From: Gunnstein Lye Date: Tue, 14 Nov 2017 17:41:22 +0100 Subject: [PATCH] EZP-28158: Test for EZP-28147 Updated user cannot log in (#2135) * EZP-28158: Test for EZP-28147 Updated user cannot log in * Move password hash constants to API User * Move password hash constants to API User: Update usage Not strictly necessary, but... --- .../API/Repository/Tests/UserServiceTest.php | 77 ++++++++++++++++++- .../API/Repository/Values/User/User.php | 30 ++++++++ eZ/Publish/Core/Repository/UserService.php | 18 ++--- .../Core/Repository/Values/User/User.php | 30 -------- 4 files changed, 115 insertions(+), 40 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/UserServiceTest.php b/eZ/Publish/API/Repository/Tests/UserServiceTest.php index e144e5adece..5977b5e1d6f 100644 --- a/eZ/Publish/API/Repository/Tests/UserServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/UserServiceTest.php @@ -18,6 +18,7 @@ use eZ\Publish\Core\Repository\Values\Content\VersionInfo; use eZ\Publish\Core\Repository\Values\User\UserGroup; use Exception; +use ReflectionClass; /** * Test case for operations in the UserService using in memory storage. @@ -1573,6 +1574,80 @@ public function testUpdateUser() return $userVersion2; } + /** + * Test for the updateUser() method. + * + * @return \eZ\Publish\API\Repository\Values\User\User + * + * @see \eZ\Publish\API\Repository\UserService::updateUser() + * @depends eZ\Publish\API\Repository\Tests\UserServiceTest::testCreateUser + * @depends eZ\Publish\API\Repository\Tests\UserServiceTest::testNewUserUpdateStruct + * @depends eZ\Publish\API\Repository\Tests\ContentServiceTest::testUpdateContent + * @depends eZ\Publish\API\Repository\Tests\ContentServiceTest::testUpdateContentMetadata + */ + public function testUpdateUserNoPassword() + { + $repository = $this->getRepository(); + $signalSlotUserService = $repository->getUserService(); + + $signalSlotUserServiceReflection = new ReflectionClass($signalSlotUserService); + $userServiceProperty = $signalSlotUserServiceReflection->getProperty('service'); + $userServiceProperty->setAccessible(true); + $userService = $userServiceProperty->getValue($signalSlotUserService); + + $userServiceReflection = new ReflectionClass($userService); + $settingsProperty = $userServiceReflection->getProperty('settings'); + $settingsProperty->setAccessible(true); + $settingsProperty->setValue( + $userService, + [ + 'hashType' => User::PASSWORD_HASH_MD5_USER, + ] + $settingsProperty->getValue($userService) + ); + + /* BEGIN: Use Case */ + $user = $this->createUserVersion1(); + + $settingsProperty->setValue( + $userService, + [ + 'hashType' => User::PASSWORD_HASH_PHP_DEFAULT, + ] + $settingsProperty->getValue($userService) + ); + + // Create a new update struct instance + $userUpdate = $userService->newUserUpdateStruct(); + + // Set new values for maxLogin, don't change password + $userUpdate->maxLogin = 43; + $userUpdate->enabled = false; + + // Updated the user record. + $userVersion2 = $userService->updateUser($user, $userUpdate); + /* END: Use Case */ + + $this->assertInstanceOf(User::class, $user); + + $this->assertEquals( + [ + 'login' => $user->login, + 'email' => $user->email, + 'passwordHash' => $user->passwordHash, + 'hashAlgorithm' => $user->hashAlgorithm, + 'maxLogin' => 43, + 'enabled' => false, + ], + [ + 'login' => $userVersion2->login, + 'email' => $userVersion2->email, + 'passwordHash' => $userVersion2->passwordHash, + 'hashAlgorithm' => $userVersion2->hashAlgorithm, + 'maxLogin' => $userVersion2->maxLogin, + 'enabled' => $userVersion2->enabled, + ] + ); + } + /** * Test for the updateUser() method. * @@ -1607,7 +1682,7 @@ public function testUpdateUserUpdatesExpectedProperties(User $user) * @see \eZ\Publish\API\Repository\UserService::updateUser() * @depends eZ\Publish\API\Repository\Tests\UserServiceTest::testUpdateUser */ - public function testUpdateUserReturnsPublishedVersion($user) + public function testUpdateUserReturnsPublishedVersion(User $user) { $this->assertEquals( APIVersionInfo::STATUS_PUBLISHED, diff --git a/eZ/Publish/API/Repository/Values/User/User.php b/eZ/Publish/API/Repository/Values/User/User.php index c3d99554190..5f90008d687 100644 --- a/eZ/Publish/API/Repository/Values/User/User.php +++ b/eZ/Publish/API/Repository/Values/User/User.php @@ -22,6 +22,36 @@ */ abstract class User extends Content implements UserReference { + /** + * @var int MD5 of password, not recommended + */ + const PASSWORD_HASH_MD5_PASSWORD = 1; + + /** + * @var int MD5 of user and password + */ + const PASSWORD_HASH_MD5_USER = 2; + + /** + * @var int MD5 of site, user and password + */ + const PASSWORD_HASH_MD5_SITE = 3; + + /** + * @var int Passwords in plaintext, should not be used for real sites + */ + const PASSWORD_HASH_PLAINTEXT = 5; + + /** + * @var int Passwords in bcrypt + */ + const PASSWORD_HASH_BCRYPT = 6; + + /** + * @var int Passwords hashed by PHPs default algorithm, which may change over time + */ + const PASSWORD_HASH_PHP_DEFAULT = 7; + /** * User login. * diff --git a/eZ/Publish/Core/Repository/UserService.php b/eZ/Publish/Core/Repository/UserService.php index 335bd092919..2bb4fe21ef9 100644 --- a/eZ/Publish/Core/Repository/UserService.php +++ b/eZ/Publish/Core/Repository/UserService.php @@ -77,7 +77,7 @@ public function __construct(RepositoryInterface $repository, Handler $userHandle 'defaultUserPlacement' => 12, 'userClassID' => 4, // @todo Rename this settings to swap out "Class" for "Type" 'userGroupClassID' => 3, - 'hashType' => User::PASSWORD_HASH_PHP_DEFAULT, + 'hashType' => APIUser::PASSWORD_HASH_PHP_DEFAULT, 'siteName' => 'ez.no', ); } @@ -1125,8 +1125,8 @@ protected function buildDomainUserObject( protected function verifyPassword($login, $password, $spiUser) { // In case of bcrypt let php's password functionality do it's magic - if ($spiUser->hashAlgorithm === User::PASSWORD_HASH_BCRYPT || - $spiUser->hashAlgorithm === User::PASSWORD_HASH_PHP_DEFAULT) { + if ($spiUser->hashAlgorithm === APIUser::PASSWORD_HASH_BCRYPT || + $spiUser->hashAlgorithm === APIUser::PASSWORD_HASH_PHP_DEFAULT) { return password_verify($password, $spiUser->passwordHash); } @@ -1156,22 +1156,22 @@ protected function verifyPassword($login, $password, $spiUser) protected function createPasswordHash($login, $password, $site, $type) { switch ($type) { - case User::PASSWORD_HASH_MD5_PASSWORD: + case APIUser::PASSWORD_HASH_MD5_PASSWORD: return md5($password); - case User::PASSWORD_HASH_MD5_USER: + case APIUser::PASSWORD_HASH_MD5_USER: return md5("$login\n$password"); - case User::PASSWORD_HASH_MD5_SITE: + case APIUser::PASSWORD_HASH_MD5_SITE: return md5("$login\n$password\n$site"); - case User::PASSWORD_HASH_PLAINTEXT: + case APIUser::PASSWORD_HASH_PLAINTEXT: return $password; - case User::PASSWORD_HASH_BCRYPT: + case APIUser::PASSWORD_HASH_BCRYPT: return password_hash($password, PASSWORD_BCRYPT); - case User::PASSWORD_HASH_PHP_DEFAULT: + case APIUser::PASSWORD_HASH_PHP_DEFAULT: return password_hash($password, PASSWORD_DEFAULT); default: diff --git a/eZ/Publish/Core/Repository/Values/User/User.php b/eZ/Publish/Core/Repository/Values/User/User.php index ebab3d3d419..ada6f4d1593 100644 --- a/eZ/Publish/Core/Repository/Values/User/User.php +++ b/eZ/Publish/Core/Repository/Values/User/User.php @@ -17,36 +17,6 @@ */ class User extends APIUser { - /** - * @var int MD5 of password, not recommended - */ - const PASSWORD_HASH_MD5_PASSWORD = 1; - - /** - * @var int MD5 of user and password - */ - const PASSWORD_HASH_MD5_USER = 2; - - /** - * @var int MD5 of site, user and password - */ - const PASSWORD_HASH_MD5_SITE = 3; - - /** - * @var int Passwords in plaintext, should not be used for real sites - */ - const PASSWORD_HASH_PLAINTEXT = 5; - - /** - * @var int Passwords in bcrypt - */ - const PASSWORD_HASH_BCRYPT = 6; - - /** - * @var int Passwords hashed by PHPs default algorithm, which may change over time - */ - const PASSWORD_HASH_PHP_DEFAULT = 7; - /** * Internal content representation. *