[BC Break][Security] Moved user comparsion logic out of UserInterface
As discussed on IRC meetings and in PR #2669 I came up with implementation. This is option2, I think more elegant. BC break: yes Feature addition: no/feature move Symfony2 test pass: yes Symfony2 test written: yes Todo: feedback needed
This commit is contained in:
parent
009e6d739e
commit
9386583b19
@ -14,6 +14,8 @@ namespace Symfony\Component\Security\Core\Authentication\Token;
|
||||
use Symfony\Component\Security\Core\Role\RoleInterface;
|
||||
use Symfony\Component\Security\Core\Role\Role;
|
||||
use Symfony\Component\Security\Core\User\UserInterface;
|
||||
use Symfony\Component\Security\Core\User\AdvancedUserInterface;
|
||||
use Symfony\Component\Security\Core\User\ComparableInterface;
|
||||
|
||||
/**
|
||||
* Base class for Token instances.
|
||||
@ -87,7 +89,7 @@ abstract class AbstractToken implements TokenInterface
|
||||
if (!$user instanceof UserInterface) {
|
||||
$changed = true;
|
||||
} else {
|
||||
$changed = !$this->user->equals($user);
|
||||
$changed = !$this->compareUser($user);
|
||||
}
|
||||
} elseif ($user instanceof UserInterface) {
|
||||
$changed = true;
|
||||
@ -220,4 +222,49 @@ abstract class AbstractToken implements TokenInterface
|
||||
|
||||
return sprintf('%s(user="%s", authenticated=%s, roles="%s")', $class, $this->getUsername(), json_encode($this->authenticated), implode(', ', $roles));
|
||||
}
|
||||
|
||||
private function compareUser(UserInterface $user)
|
||||
{
|
||||
if (!($this->user instanceof UserInterface)) {
|
||||
throw new \BadMethodCallException('Method "compareUser" should be called when current user class is instance of "UserInterface".');
|
||||
}
|
||||
|
||||
if ($this->user instanceof ComparableInterface) {
|
||||
return $this->user->compareTo($user);
|
||||
}
|
||||
|
||||
if ($this->user->getPassword() !== $user->getPassword()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user->getSalt() !== $user->getSalt()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user->getUsername() !== $user->getUsername()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user instanceof AdvancedUserInterface && $user instanceof AdvancedUserInterface) {
|
||||
if ($this->user->isAccountNonExpired() !== $user->isAccountNonExpired()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user->isAccountNonLocked() !== $user->isAccountNonLocked()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user->isCredentialsNonExpired() !== $user->isCredentialsNonExpired()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->user->isEnabled() !== $user->isEnabled()) {
|
||||
return false;
|
||||
}
|
||||
} elseif ($this->user instanceof AdvancedUserInterface xor $user instanceof AdvancedUserInterface) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -0,0 +1,32 @@
|
||||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the Symfony package.
|
||||
*
|
||||
* (c) Fabien Potencier <fabien@symfony.com>
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Symfony\Component\Security\Core\User;
|
||||
|
||||
/**
|
||||
* ComparatorInterface used to test if two object are equal in security
|
||||
* and re-authentication context.
|
||||
*
|
||||
* @author Dariusz Górecki <darek.krk@gmail.com>
|
||||
*/
|
||||
interface ComparableInterface
|
||||
{
|
||||
/**
|
||||
* The equality comparison should neither be done by referential equality
|
||||
* nor by comparing identities (i.e. getId() === getId()).
|
||||
*
|
||||
* However, you do not need to compare every attribute, but only those that
|
||||
* are relevant for assessing whether re-authentication is required.
|
||||
*
|
||||
* @return boolean
|
||||
*/
|
||||
public function compareTo($object);
|
||||
}
|
@ -112,44 +112,4 @@ final class User implements AdvancedUserInterface
|
||||
public function eraseCredentials()
|
||||
{
|
||||
}
|
||||
|
||||
/**
|
||||
* {@inheritDoc}
|
||||
*/
|
||||
public function equals(UserInterface $user)
|
||||
{
|
||||
if (!$user instanceof User) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->password !== $user->getPassword()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->getSalt() !== $user->getSalt()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->username !== $user->getUsername()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->accountNonExpired !== $user->isAccountNonExpired()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->accountNonLocked !== $user->isAccountNonLocked()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->credentialsNonExpired !== $user->isCredentialsNonExpired()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($this->enabled !== $user->isEnabled()) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
@ -84,19 +84,4 @@ interface UserInterface
|
||||
* @return void
|
||||
*/
|
||||
function eraseCredentials();
|
||||
|
||||
/**
|
||||
* Returns whether or not the given user is equivalent to *this* user.
|
||||
*
|
||||
* The equality comparison should neither be done by referential equality
|
||||
* nor by comparing identities (i.e. getId() === getId()).
|
||||
*
|
||||
* However, you do not need to compare every attribute, but only those that
|
||||
* are relevant for assessing whether re-authentication is required.
|
||||
*
|
||||
* @param UserInterface $user
|
||||
*
|
||||
* @return Boolean
|
||||
*/
|
||||
function equals(UserInterface $user);
|
||||
}
|
||||
|
@ -144,11 +144,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
|
||||
public function getUsers()
|
||||
{
|
||||
$user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
|
||||
$user
|
||||
->expects($this->any())
|
||||
->method('equals')
|
||||
->will($this->returnValue(true))
|
||||
;
|
||||
|
||||
return array(
|
||||
array($user),
|
||||
@ -176,11 +171,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
|
||||
public function getUserChanges()
|
||||
{
|
||||
$user = $this->getMock('Symfony\Component\Security\Core\User\UserInterface');
|
||||
$user
|
||||
->expects($this->any())
|
||||
->method('equals')
|
||||
->will($this->returnValue(false))
|
||||
;
|
||||
|
||||
return array(
|
||||
array(
|
||||
@ -192,9 +182,6 @@ class AbstractTokenTest extends \PHPUnit_Framework_TestCase
|
||||
array(
|
||||
'foo', $user,
|
||||
),
|
||||
array(
|
||||
$user, $user,
|
||||
),
|
||||
array(
|
||||
$user, 'foo'
|
||||
),
|
||||
|
@ -123,22 +123,4 @@ class UserTest extends \PHPUnit_Framework_TestCase
|
||||
$user->eraseCredentials();
|
||||
$this->assertEquals('superpass', $user->getPassword());
|
||||
}
|
||||
|
||||
public function testUsersAreEqual()
|
||||
{
|
||||
$user1 = new User('fabien', 'superpass', array('ROLE_USER'));
|
||||
$user2 = clone $user1;
|
||||
|
||||
$this->assertTrue($user1->equals($user2));
|
||||
$this->assertTrue($user2->equals($user1));
|
||||
}
|
||||
|
||||
public function testUsersAreNotEqual()
|
||||
{
|
||||
$user1 = new User('fabien', 'superpass', array('ROLE_USER'));
|
||||
$user2 = new User('fabien', 'superpass', array('ROLE_USER'), false);
|
||||
|
||||
$this->assertFalse($user1->equals($user2));
|
||||
$this->assertFalse($user2->equals($user1));
|
||||
}
|
||||
}
|
||||
|
Reference in New Issue
Block a user