Skip to content

Commit

Permalink
bug #9988 #9969 Fix when trying to delete shop user having same ID th…
Browse files Browse the repository at this point in the history
…an logged … (laurent35240)

This PR was merged into the 1.2 branch.

Discussion
----------

…in admin user

| Q               | A
| --------------- | -----
| Branch?         | 1.2
| Bug fix?        | yes
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | fixes #9969 
| License         | MIT



Commits
-------

cc0bf1d #9969 Fix when trying to delete shop user having same ID than logged in admin user
38ed97c #9969 Add sylius/core in userBundle dependencies in order to be able to use AdminUserInterface
203c57d  #9969 Remove sylius/core in userBundle dependencies. Refacto of test for checking if we are not trying to delete currently logged admin user
29bff9a  #9969 Fixing case of API user trying to delete himself
  • Loading branch information
lchrusciel committed Jan 4, 2019
2 parents e3f001c + 29bff9a commit 1d0bff9
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 5 deletions.
23 changes: 20 additions & 3 deletions src/Sylius/Bundle/UserBundle/EventListener/UserDeleteListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,7 @@ public function deleteUser(ResourceControllerEvent $event): void

Assert::isInstanceOf($user, UserInterface::class);

$token = $this->tokenStorage->getToken();

if ((null !== $token) && ($loggedUser = $token->getUser()) && ($loggedUser->getId() === $user->getId())) {
if ($this->isTryingToDeleteLoggedInAdminUser($user)) {
$event->stopPropagation();
$event->setErrorCode(Response::HTTP_UNPROCESSABLE_ENTITY);
$event->setMessage('Cannot remove currently logged in user.');
Expand All @@ -56,4 +54,23 @@ public function deleteUser(ResourceControllerEvent $event): void
$flashBag->add('error', 'Cannot remove currently logged in user.');
}
}

private function isTryingToDeleteLoggedInAdminUser(UserInterface $user): bool
{
if (!$user->hasRole('ROLE_ADMINISTRATION_ACCESS') && !$user->hasRole('ROLE_API_ACCESS')){
return false;
}

$token = $this->tokenStorage->getToken();
if (!$token) {
return false;
}

$loggedUser = $token->getUser();
if (!$loggedUser) {
return false;
}

return $loggedUser->getId() === $user->getId();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ function it_deletes_user_if_it_is_different_than_currently_logged_one(
): void {
$event->getSubject()->willReturn($userToBeDeleted);
$userToBeDeleted->getId()->willReturn(11);
$userToBeDeleted->hasRole('ROLE_ADMINISTRATION_ACCESS')->willReturn(true);
$userToBeDeleted->hasRole('ROLE_API_ACCESS')->willReturn(false);

$tokenStorage->getToken()->willReturn($tokenInterface);
$currentlyLoggedUser->getId()->willReturn(1);
Expand All @@ -61,6 +63,8 @@ function it_deletes_user_if_no_user_is_logged_in(
): void {
$event->getSubject()->willReturn($userToBeDeleted);
$userToBeDeleted->getId()->willReturn(11);
$userToBeDeleted->hasRole('ROLE_ADMINISTRATION_ACCESS')->willReturn(true);
$userToBeDeleted->hasRole('ROLE_API_ACCESS')->willReturn(false);

$tokenStorage->getToken()->willReturn($tokenInterface);
$tokenInterface->getUser()->willReturn(null);
Expand All @@ -81,6 +85,8 @@ function it_deletes_user_if_there_is_no_token(
): void {
$event->getSubject()->willReturn($userToBeDeleted);
$userToBeDeleted->getId()->willReturn(11);
$userToBeDeleted->hasRole('ROLE_ADMINISTRATION_ACCESS')->willReturn(true);
$userToBeDeleted->hasRole('ROLE_API_ACCESS')->willReturn(false);

$tokenStorage->getToken()->willReturn(null);

Expand All @@ -92,10 +98,18 @@ function it_deletes_user_if_there_is_no_token(
$this->deleteUser($event);
}

function it_does_not_allow_to_delete_currently_logged_user(ResourceControllerEvent $event, UserInterface $userToBeDeleted, UserInterface $currentlyLoggedInUser, $tokenStorage, $flashBag, TokenInterface $token): void
{
function it_does_not_allow_to_delete_currently_logged_user(
ResourceControllerEvent $event,
UserInterface $userToBeDeleted,
UserInterface $currentlyLoggedInUser,
$tokenStorage, $flashBag,
TokenInterface $token
): void {
$event->getSubject()->willReturn($userToBeDeleted);
$userToBeDeleted->getId()->willReturn(1);
$userToBeDeleted->hasRole('ROLE_ADMINISTRATION_ACCESS')->willReturn(true);
$userToBeDeleted->hasRole('ROLE_API_ACCESS')->willReturn(false);

$tokenStorage->getToken()->willReturn($token);
$currentlyLoggedInUser->getId()->willReturn(1);
$token->getUser()->willReturn($currentlyLoggedInUser);
Expand All @@ -107,4 +121,28 @@ function it_does_not_allow_to_delete_currently_logged_user(ResourceControllerEve

$this->deleteUser($event);
}

function it_deletes_shop_user_even_if_admin_user_has_same_id(
ResourceControllerEvent $event,
UserInterface $userToBeDeleted,
UserInterface $currentlyLoggedInUser,
$tokenStorage, $flashBag,
TokenInterface $token
): void {
$event->getSubject()->willReturn($userToBeDeleted);
$userToBeDeleted->getId()->willReturn(1);
$userToBeDeleted->hasRole('ROLE_ADMINISTRATION_ACCESS')->willReturn(false);
$userToBeDeleted->hasRole('ROLE_API_ACCESS')->willReturn(false);

$tokenStorage->getToken()->willReturn($token);
$currentlyLoggedInUser->getId()->willReturn(1);
$token->getUser()->willReturn($currentlyLoggedInUser);

$event->stopPropagation()->shouldNotBeCalled();
$event->setErrorCode(Argument::any())->shouldNotBeCalled();
$event->setMessage(Argument::any())->shouldNotBeCalled();
$flashBag->add('error', Argument::any())->shouldNotBeCalled();

$this->deleteUser($event);
}
}

0 comments on commit 1d0bff9

Please sign in to comment.