Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EZP-31825: Fixed user with user:password unable to change password #117

Merged
merged 37 commits into from
Oct 22, 2020
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
5b3d545
EZP-31825: Fixed user with user/password unable to change password
Steveb-p Oct 12, 2020
79df0a1
Prefer to use existing method to get user account field
Steveb-p Oct 14, 2020
67726e4
Add missing PHPDoc params
Steveb-p Oct 14, 2020
d5707d6
Proof of Concept for new password update API
Steveb-p Oct 14, 2020
a0d7d5d
Remove unused private method
Steveb-p Oct 14, 2020
37886cb
Move error message to constant
Steveb-p Oct 14, 2020
0d367a0
Fix code style
Steveb-p Oct 14, 2020
8d1d838
Maintain compatibility with old version
Steveb-p Oct 15, 2020
1391111
Move private method to the end of the file
Steveb-p Oct 15, 2020
072145c
Fix tests
Steveb-p Oct 15, 2020
a813e49
Fix missing argument type
Steveb-p Oct 15, 2020
3a0c98b
Add new exception class for missing user field type
Steveb-p Oct 15, 2020
b62e0f4
Bring back previous implementation of UserUpdate
Steveb-p Oct 16, 2020
56b2c74
Finalize implementation of updateUserPassword before review
Steveb-p Oct 16, 2020
c030d83
Add missing newline
Steveb-p Oct 16, 2020
8056fd6
Fix tests
Steveb-p Oct 16, 2020
dbebf22
Remove unnecessary newline
Steveb-p Oct 16, 2020
463d857
Add authorization test
Steveb-p Oct 16, 2020
9c19f6a
Fix code style
Steveb-p Oct 16, 2020
6e4fc65
Fix tests
Steveb-p Oct 16, 2020
4dc9e47
Fix tests
Steveb-p Oct 16, 2020
b1e425d
Improve password changing test
Steveb-p Oct 16, 2020
29df0f0
Rename $password to $newPassword where applicable
Steveb-p Oct 16, 2020
4e4b99b
Update testUpdateUserPasswordThrowsUnauthorizedException
Steveb-p Oct 19, 2020
f1a8591
Move testUpdateUserPasswordWorksWithUserPasswordRole
Steveb-p Oct 19, 2020
18474be
Fix new API PhpDoc
Steveb-p Oct 19, 2020
02b7a78
Remove unnecessary login / email updates
Steveb-p Oct 19, 2020
5bca28b
Fix deprecation note
Steveb-p Oct 19, 2020
63587c3
Apply review
Steveb-p Oct 19, 2020
30b37cb
Add missing return type hint
Steveb-p Oct 19, 2020
b625a0f
Improve new MissingUserFieldTypeException
Steveb-p Oct 19, 2020
0ccda1e
Apply review
Steveb-p Oct 19, 2020
d2dec1a
Reduce dependencies needed for UserService::updateUserPassword
Steveb-p Oct 19, 2020
7cdb80e
Fix code style
Steveb-p Oct 19, 2020
f36240c
Add missing import
Steveb-p Oct 19, 2020
14a4a5b
Apply review, removed phpdoc
Steveb-p Oct 19, 2020
b887eb0
Revert out-of-scope change
Steveb-p Oct 20, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use eZ\Publish\Core\Repository\User\PasswordHashServiceInterface;
use eZ\Publish\Core\Repository\Helper\RelationProcessor;
use eZ\Publish\Core\Repository\Mapper;
use eZ\Publish\Core\Repository\User\PasswordValidatorInterface;
use eZ\Publish\Core\Search\Common\BackgroundIndexer;
use eZ\Publish\SPI\Persistence\Filter\Content\Handler as ContentFilteringHandler;
use eZ\Publish\SPI\Persistence\Filter\Location\Handler as LocationFilteringHandler;
Expand Down Expand Up @@ -88,7 +89,8 @@ public function buildRepository(
LimitationService $limitationService,
PermissionService $permissionService,
ContentFilteringHandler $contentFilteringHandler,
LocationFilteringHandler $locationFilteringHandler
LocationFilteringHandler $locationFilteringHandler,
PasswordValidatorInterface $passwordValidator
): Repository {
$config = $this->container->get('ezpublish.api.repository_configuration_provider')->getRepositoryConfig();

Expand All @@ -111,6 +113,7 @@ public function buildRepository(
$permissionService,
$contentFilteringHandler,
$locationFilteringHandler,
$passwordValidator,
[
'role' => [
'policyMap' => $this->policyMap,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace eZ\Publish\API\Repository\Events\User;

use eZ\Publish\API\Repository\Values\User\User;
use eZ\Publish\SPI\Repository\Event\BeforeEvent;
use UnexpectedValueException;

final class BeforeUpdateUserPasswordEvent extends BeforeEvent
{
/** @var \eZ\Publish\API\Repository\Values\User\User */
private $user;

/** @var string */
private $newPassword;

/** @var \eZ\Publish\API\Repository\Values\User\User|null */
private $updatedUser;

public function __construct(User $user, string $newPassword)
{
$this->user = $user;
$this->newPassword = $newPassword;
}

public function getUser(): User
{
return $this->user;
}

public function getNewPassword(): string
{
return $this->newPassword;
}

public function getUpdatedUser(): User
{
if (!$this->hasUpdatedUser()) {
throw new UnexpectedValueException(sprintf('Return value is not set or not of type %s. Check hasUpdatedUser() or set it using setUpdatedUser() before you call the getter.', User::class));
}

return $this->updatedUser;
}

public function setUpdatedUser(?User $updatedUser): void
{
$this->updatedUser = $updatedUser;
}

public function hasUpdatedUser(): bool
{
return $this->updatedUser instanceof User;
}
}
49 changes: 49 additions & 0 deletions eZ/Publish/API/Repository/Events/User/UpdateUserPasswordEvent.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<?php

/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace eZ\Publish\API\Repository\Events\User;

use eZ\Publish\API\Repository\Values\User\User;
use eZ\Publish\SPI\Repository\Event\AfterEvent;

final class UpdateUserPasswordEvent extends AfterEvent
{
/** @var \eZ\Publish\API\Repository\Values\User\User */
private $user;

/** @var string */
private $newPassword;

/** @var \eZ\Publish\API\Repository\Values\User\User */
private $updatedUser;

public function __construct(
User $updatedUser,
User $user,
string $newPassword
) {
$this->user = $user;
$this->newPassword = $newPassword;
$this->updatedUser = $updatedUser;
}

public function getUser(): User
{
return $this->user;
}

public function getNewPassword(): string
{
return $this->newPassword;
}

public function getUpdatedUser(): User
{
return $this->updatedUser;
}
}
25 changes: 25 additions & 0 deletions eZ/Publish/API/Repository/Tests/UserServiceAuthorizationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,31 @@ public function testUpdateUserThrowsUnauthorizedException()
/* END: Use Case */
}

/**
* @covers \eZ\Publish\API\Repository\UserService::updateUserPassword
*/
public function testUpdateUserPasswordThrowsUnauthorizedException(): void
{
$repository = $this->getRepository();
$userService = $repository->getUserService();
$permissionResolver = $repository->getPermissionResolver();

$this->createRoleWithPolicies('CannotChangePassword', []);

$user = $this->createCustomUserWithLogin(
'without_role_password',
'without_role_password@example.com',
'Anons',
'CannotChangePassword'
);

// Now set the currently created "Editor" as current user
$permissionResolver->setCurrentUserReference($user);

$this->expectException(UnauthorizedException::class);
$userService->updateUserPassword($user, 'new password');
}

/**
* Test for the assignUserToUserGroup() method.
*
Expand Down
29 changes: 29 additions & 0 deletions eZ/Publish/API/Repository/Tests/UserServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2016,6 +2016,35 @@ public function testUpdateUserWithStrongPassword()
$this->assertInstanceOf(User::class, $user);
}

/**
* @covers \eZ\Publish\API\Repository\UserService::updateUserPassword
*/
public function testUpdateUserPasswordWorksWithUserPasswordRole(): void
{
$repository = $this->getRepository();
$userService = $repository->getUserService();
$permissionResolver = $repository->getPermissionResolver();

$this->createRoleWithPolicies('CanChangePassword', [
['module' => 'user', 'function' => 'password'],
]);

$user = $this->createCustomUserWithLogin(
'with_role_password',
'with_role_password@example.com',
'Anons',
'CanChangePassword'
);
$previousHash = $user->passwordHash;

$permissionResolver->setCurrentUserReference($user);

$userService->updateUserPassword($user, 'new password');

$user = $userService->loadUserByLogin('with_role_password');
$this->assertNotEquals($previousHash, $user->passwordHash);
}

/**
* Test for the loadUserGroupsOfUser() method.
*
Expand Down
12 changes: 10 additions & 2 deletions eZ/Publish/API/Repository/UserService.php
Original file line number Diff line number Diff line change
Expand Up @@ -218,21 +218,29 @@ public function deleteUser(User $user): iterable;
/**
* Updates a user.
*
* 4.x: If the versionUpdateStruct is set in the user update structure, this method internally creates a content draft, updates ts with the provided data
* 4.x: If the versionUpdateStruct is set in the user update structure, this method internally creates a content draft, updates it with the provided data
Steveb-p marked this conversation as resolved.
Show resolved Hide resolved
* and publishes the draft. If a draft is explicitly required, the user group can be updated via the content service methods.
*
* @param \eZ\Publish\API\Repository\Values\User\User $user
* @param \eZ\Publish\API\Repository\Values\User\UserUpdateStruct $userUpdateStruct
*
* @return \eZ\Publish\API\Repository\Values\User\User
*
*@throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException if a field in the $userUpdateStruct is not valid
* @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException if a field in the $userUpdateStruct is not valid
* @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException if a required field is set empty
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if a field value is not accepted by the field type
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException if the authenticated user is not allowed to update the user
*/
public function updateUser(User $user, UserUpdateStruct $userUpdateStruct): User;

/**
* Updates user's password.
*
* @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException if new password does not pass validation
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException if the authenticated user is not allowed to update the user
*/
public function updateUserPassword(User $user, string $newPassword): User;

/**
* Update the user token information specified by the user token struct.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use eZ\Publish\Core\Repository\User\PasswordHashServiceInterface;
use eZ\Publish\Core\Repository\Helper\RelationProcessor;
use eZ\Publish\Core\Repository\Mapper;
use eZ\Publish\Core\Repository\User\PasswordValidatorInterface;
use eZ\Publish\Core\Search\Common\BackgroundIndexer;
use eZ\Publish\SPI\Persistence\Filter\Content\Handler as ContentFilteringHandler;
use eZ\Publish\SPI\Persistence\Filter\Location\Handler as LocationFilteringHandler;
Expand Down Expand Up @@ -79,6 +80,7 @@ public function buildRepository(
PermissionService $permissionService,
ContentFilteringHandler $contentFilteringHandler,
LocationFilteringHandler $locationFilteringHandler,
PasswordValidatorInterface $passwordValidator,
array $languages
): Repository {
return new $this->repositoryClass(
Expand All @@ -100,6 +102,7 @@ public function buildRepository(
$permissionService,
$contentFilteringHandler,
$locationFilteringHandler,
$passwordValidator,
[
'role' => [
'policyMap' => $this->policyMap,
Expand Down
26 changes: 26 additions & 0 deletions eZ/Publish/Core/Base/Exceptions/MissingUserFieldTypeException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/**
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
namespace eZ\Publish\Core\Base\Exceptions;

use eZ\Publish\API\Repository\Values\ContentType\ContentType;

/**
* @internal
*/
final class MissingUserFieldTypeException extends ContentValidationException
{
public function __construct(ContentType $contentType, string $fieldType)
{
parent::__construct(
'The provided Content Type "%contentType%" does not contain the %fieldType% Field Type',
[
'contentType' => $contentType->identifier,
'fieldType' => $fieldType,
]
);
}
}
54 changes: 54 additions & 0 deletions eZ/Publish/Core/Event/UserService.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

namespace eZ\Publish\Core\Event;

use eZ\Publish\API\Repository\Events\User\BeforeUpdateUserPasswordEvent;
use eZ\Publish\API\Repository\Events\User\UpdateUserPasswordEvent;
use eZ\Publish\API\Repository\UserService as UserServiceInterface;
use eZ\Publish\API\Repository\Values\User\User;
use eZ\Publish\API\Repository\Values\User\UserCreateStruct;
Expand Down Expand Up @@ -228,6 +230,58 @@ public function updateUser(
return $updatedUser;
}

public function updateUserPassword(
User $user,
string $newPassword
): User {
$eventData = [
$user,
new UserUpdateStruct([
'password' => $newPassword,
]),
];

/**
* @deprecated since eZ Platform by Ibexa v3.1. listening on BeforeUpdateUserEvent when updating password has been deprecated. Use BeforeUpdateUserPasswordEvent instead.
*/
$beforeEvent = new BeforeUpdateUserEvent(...$eventData);

$this->eventDispatcher->dispatch($beforeEvent);
if ($beforeEvent->isPropagationStopped()) {
return $beforeEvent->getUpdatedUser();
}

$beforePasswordEvent = new BeforeUpdateUserPasswordEvent($user, $newPassword);

$this->eventDispatcher->dispatch($beforePasswordEvent);
if ($beforePasswordEvent->isPropagationStopped()) {
return $beforePasswordEvent->getUpdatedUser();
}

if ($beforeEvent->hasUpdatedUser()) {
$updatedUser = $beforeEvent->getUpdatedUser();
} elseif ($beforePasswordEvent->hasUpdatedUser()) {
$updatedUser = $beforePasswordEvent->getUpdatedUser();
} else {
$updatedUser = $this->innerService->updateUserPassword($user, $newPassword);
}

/**
* @deprecated since eZ Platform by Ibexa v3.1. Listening on UpdateUserEvent when updating password has been deprecated. Use UpdateUserPasswordEvent instead.
*/
$afterEvent = new UpdateUserEvent($updatedUser, ...$eventData);
$this->eventDispatcher->dispatch(
$afterEvent
);

$afterPasswordEvent = new UpdateUserPasswordEvent($updatedUser, $user, $newPassword);
$this->eventDispatcher->dispatch(
$afterPasswordEvent
);

return $updatedUser;
}

public function updateUserToken(
User $user,
UserTokenUpdateStruct $userTokenUpdateStruct
Expand Down
15 changes: 15 additions & 0 deletions eZ/Publish/Core/Persistence/Cache/UserHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,21 @@ public function update(User $user)
return $user;
}

public function updatePassword(User $user): void
{
$this->logger->logCall(__METHOD__, ['user-login' => $user->login]);

$this->persistenceHandler->userHandler()->updatePassword($user);

// Clear corresponding content cache as update of the User changes it's external data
$this->cache->invalidateTags(['content-' . $user->id, 'user-' . $user->id]);
// Clear especially by email key as it might already be cached and this might represent change to email
$this->cache->deleteItems([
'ez-user-' . $this->escapeForCacheKey($user->email) . '-by-email',
'ez-users-' . $this->escapeForCacheKey($user->email) . '-by-email',
]);
}

/**
* {@inheritdoc}
*/
Expand Down
8 changes: 8 additions & 0 deletions eZ/Publish/Core/Persistence/Legacy/User/Gateway.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

namespace eZ\Publish\Core\Persistence\Legacy\User;

use eZ\Publish\SPI\Persistence\User;
use eZ\Publish\SPI\Persistence\User\UserTokenUpdateStruct;

/**
Expand Down Expand Up @@ -39,6 +40,13 @@ abstract public function loadByEmail(string $email): array;
*/
abstract public function loadUserByToken(string $hash): array;

/**
* Update the user password as specified by the user struct.
*
* @param \eZ\Publish\SPI\Persistence\User $user
*/
abstract public function updateUserPassword(User $user): void;

/**
* Update a User token specified by UserTokenUpdateStruct.
*
Expand Down
Loading