Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Commit

Permalink
[Security] Fixed AbstractToken::hasUserChanged()
Browse files Browse the repository at this point in the history
  • Loading branch information
wouterj authored and nicolas-grekas committed May 30, 2020
1 parent 21d84ee commit be48df1
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 3 deletions.
7 changes: 5 additions & 2 deletions Core/Authentication/Token/AbstractToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,10 +317,13 @@ private function hasUserChanged(UserInterface $user): bool
return true;
}

$currentUserRoles = array_map('strval', (array) $this->user->getRoles());
$userRoles = array_map('strval', (array) $user->getRoles());

if (\count($userRoles) !== \count($currentUserRoles) || \count($userRoles) !== \count(array_intersect($userRoles, $currentUserRoles))) {
if ($this instanceof SwitchUserToken) {
$userRoles[] = 'ROLE_PREVIOUS_ADMIN';
}

if (\count($userRoles) !== \count($this->getRoleNames()) || \count($userRoles) !== \count(array_intersect($userRoles, $this->getRoleNames()))) {
return true;
}

Expand Down
67 changes: 66 additions & 1 deletion Core/Tests/Authentication/Token/AbstractTokenTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -238,13 +238,28 @@ public function getUserChangesAdvancedUser()
*/
public function testSetUserDoesNotSetAuthenticatedToFalseWhenUserDoesNotChange($user)
{
$token = new ConcreteToken(['ROLE_FOO']);
$token = new ConcreteToken();
$token->setAuthenticated(true);
$this->assertTrue($token->isAuthenticated());

$token->setUser($user);
$this->assertTrue($token->isAuthenticated());

$token->setUser($user);
$this->assertTrue($token->isAuthenticated());
}

public function testIsUserChangedWhenSerializing()
{
$token = new ConcreteToken(['ROLE_ADMIN']);
$token->setAuthenticated(true);
$this->assertTrue($token->isAuthenticated());

$user = new SerializableUser('wouter', ['ROLE_ADMIN']);
$token->setUser($user);
$this->assertTrue($token->isAuthenticated());

$token = unserialize(serialize($token));
$token->setUser($user);
$this->assertTrue($token->isAuthenticated());
}
Expand All @@ -265,6 +280,56 @@ public function __toString(): string
}
}

class SerializableUser implements UserInterface, \Serializable
{
private $roles;
private $name;

public function __construct($name, array $roles = [])
{
$this->name = $name;
$this->roles = $roles;
}

public function getUsername()
{
return $this->name;
}

public function getPassword()
{
return '***';
}

public function getRoles()
{
if (empty($this->roles)) {
return ['ROLE_USER'];
}

return $this->roles;
}

public function eraseCredentials()
{
}

public function getSalt()
{
return null;
}

public function serialize()
{
return serialize($this->name);
}

public function unserialize($serialized)
{
$this->name = unserialize($serialized);
}
}

class ConcreteToken extends AbstractToken
{
private $credentials = 'credentials_value';
Expand Down

0 comments on commit be48df1

Please sign in to comment.