diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php index dc21684a60..4b48bab2a2 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php @@ -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; + } } diff --git a/src/Symfony/Component/Security/Core/User/ComparableInterface.php b/src/Symfony/Component/Security/Core/User/ComparableInterface.php new file mode 100644 index 0000000000..2d1fe8f069 --- /dev/null +++ b/src/Symfony/Component/Security/Core/User/ComparableInterface.php @@ -0,0 +1,32 @@ + + * + * 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 + */ +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); +} diff --git a/src/Symfony/Component/Security/Core/User/User.php b/src/Symfony/Component/Security/Core/User/User.php index d586511b65..6076603bab 100644 --- a/src/Symfony/Component/Security/Core/User/User.php +++ b/src/Symfony/Component/Security/Core/User/User.php @@ -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; - } } diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index 85356b77a0..ce3b3a8bac 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -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); } diff --git a/tests/Symfony/Tests/Component/Security/Core/Authentication/Token/AbstractTokenTest.php b/tests/Symfony/Tests/Component/Security/Core/Authentication/Token/AbstractTokenTest.php index 0adce5398c..02a01b4e83 100644 --- a/tests/Symfony/Tests/Component/Security/Core/Authentication/Token/AbstractTokenTest.php +++ b/tests/Symfony/Tests/Component/Security/Core/Authentication/Token/AbstractTokenTest.php @@ -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' ), diff --git a/tests/Symfony/Tests/Component/Security/Core/User/UserTest.php b/tests/Symfony/Tests/Component/Security/Core/User/UserTest.php index f087268a15..477bd929b4 100644 --- a/tests/Symfony/Tests/Component/Security/Core/User/UserTest.php +++ b/tests/Symfony/Tests/Component/Security/Core/User/UserTest.php @@ -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)); - } }