diff --git a/UPGRADE-4.1.md b/UPGRADE-4.1.md index 274da2e1fe..8155492899 100644 --- a/UPGRADE-4.1.md +++ b/UPGRADE-4.1.md @@ -29,6 +29,10 @@ Security -------- * The `ContextListener::setLogoutOnUserChange()` method is deprecated and will be removed in 5.0. + * Using the `AdvancedUserInterface` is now deprecated. To use the existing + functionality, create a custom user-checker based on the + `Symfony\Component\Security\Core\User\UserChecker`. This functionality will + be removed in Symfony 5.0. SecurityBundle -------------- diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index 6383fb8932..7356161a89 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -26,6 +26,7 @@ Security -------- * The `ContextListener::setLogoutOnUserChange()` method has been removed. + * The `Symfony\Component\Security\Core\User\AdvancedUserInterface` has been removed. SecurityBundle -------------- diff --git a/src/Symfony/Component/Security/CHANGELOG.md b/src/Symfony/Component/Security/CHANGELOG.md index 59cd5b680c..0294b62def 100644 --- a/src/Symfony/Component/Security/CHANGELOG.md +++ b/src/Symfony/Component/Security/CHANGELOG.md @@ -6,6 +6,10 @@ CHANGELOG * The `ContextListener::setLogoutOnUserChange()` method is deprecated and will be removed in 5.0. * added `UserValueResolver`. + * Using the AdvancedUserInterface is now deprecated. To use the existing + functionality, create a custom user-checker based on the + `Symfony\Component\Security\Core\User\UserChecker`. This functionality will + be removed in Symfony 5.0. 4.0.0 ----- diff --git a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php index 465cf9e628..7a0522fd10 100644 --- a/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php +++ b/src/Symfony/Component/Security/Core/Authentication/Token/AbstractToken.php @@ -261,6 +261,7 @@ abstract class AbstractToken implements TokenInterface } if ($this->user instanceof AdvancedUserInterface && $user instanceof AdvancedUserInterface) { + @trigger_error(sprintf('Checking for the AdvancedUserInterface in %s has been deprecated in 4.1 and will be removed in 5.0. Implement the %s to check if the user has been changed,', __METHOD__, EquatableInterface::class), E_USER_DEPRECATED); if ($this->user->isAccountNonExpired() !== $user->isAccountNonExpired()) { return true; } @@ -277,6 +278,8 @@ abstract class AbstractToken implements TokenInterface return true; } } elseif ($this->user instanceof AdvancedUserInterface xor $user instanceof AdvancedUserInterface) { + @trigger_error(sprintf('Checking for the AdvancedUserInterface in %s has been deprecated in 4.1 and will be removed in 5.0. Implement the %s to check if the user has been changed,', __METHOD__, EquatableInterface::class), E_USER_DEPRECATED); + return true; } diff --git a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php index 4cdf982676..0d165a4cbd 100644 --- a/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php +++ b/src/Symfony/Component/Security/Core/Tests/Authentication/Token/AbstractTokenTest.php @@ -59,7 +59,6 @@ class ConcreteToken extends AbstractToken } } -/** @noinspection PhpUndefinedClassInspection */ class AbstractTokenTest extends TestCase { public function testGetUsername() @@ -185,10 +184,8 @@ class AbstractTokenTest extends TestCase public function getUsers() { $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock(); - $advancedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\AdvancedUserInterface')->getMock(); return array( - array($advancedUser), array($user), array(new TestUser('foo')), array('foo'), @@ -212,53 +209,59 @@ class AbstractTokenTest extends TestCase } public function getUserChanges() + { + $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock(); + + return array( + array('foo', 'bar'), + array('foo', new TestUser('bar')), + array('foo', $user), + array($user, 'foo'), + array($user, new TestUser('foo')), + array(new TestUser('foo'), new TestUser('bar')), + array(new TestUser('foo'), 'bar'), + array(new TestUser('foo'), $user), + ); + } + + /** + * @group legacy + * + * @dataProvider getUserChangesAdvancedUser + */ + public function testSetUserSetsAuthenticatedToFalseWhenUserChangesdvancedUser($firstUser, $secondUser) + { + $token = $this->getToken(); + $token->setAuthenticated(true); + $this->assertTrue($token->isAuthenticated()); + + $token->setUser($firstUser); + $this->assertTrue($token->isAuthenticated()); + + $token->setUser($secondUser); + $this->assertFalse($token->isAuthenticated()); + } + + public function getUserChangesAdvancedUser() { $user = $this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock(); $advancedUser = $this->getMockBuilder('Symfony\Component\Security\Core\User\AdvancedUserInterface')->getMock(); return array( - array( - 'foo', 'bar', - ), - array( - 'foo', new TestUser('bar'), - ), - array( - 'foo', $user, - ), - array( - 'foo', $advancedUser, - ), - array( - $user, 'foo', - ), - array( - $advancedUser, 'foo', - ), - array( - $user, new TestUser('foo'), - ), - array( - $advancedUser, new TestUser('foo'), - ), - array( - new TestUser('foo'), new TestUser('bar'), - ), - array( - new TestUser('foo'), 'bar', - ), - array( - new TestUser('foo'), $user, - ), - array( - new TestUser('foo'), $advancedUser, - ), - array( - $user, $advancedUser, - ), - array( - $advancedUser, $user, - ), + array('foo', 'bar'), + array('foo', new TestUser('bar')), + array('foo', $user), + array('foo', $advancedUser), + array($user, 'foo'), + array($advancedUser, 'foo'), + array($user, new TestUser('foo')), + array($advancedUser, new TestUser('foo')), + array(new TestUser('foo'), new TestUser('bar')), + array(new TestUser('foo'), 'bar'), + array(new TestUser('foo'), $user), + array(new TestUser('foo'), $advancedUser), + array($user, $advancedUser), + array($advancedUser, $user), ); } diff --git a/src/Symfony/Component/Security/Core/Tests/User/UserCheckerTest.php b/src/Symfony/Component/Security/Core/Tests/User/UserCheckerTest.php index 8ec34843ea..6cf82d8358 100644 --- a/src/Symfony/Component/Security/Core/Tests/User/UserCheckerTest.php +++ b/src/Symfony/Component/Security/Core/Tests/User/UserCheckerTest.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Security\Core\Tests\User; use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\User\User; use Symfony\Component\Security\Core\User\UserChecker; class UserCheckerTest extends TestCase @@ -24,6 +25,16 @@ class UserCheckerTest extends TestCase } public function testCheckPostAuthPass() + { + $checker = new UserChecker(); + $this->assertNull($checker->checkPostAuth(new User('John', 'password'))); + } + + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPostAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + */ + public function testCheckPostAuthPassAdvancedUser() { $checker = new UserChecker(); @@ -37,6 +48,17 @@ class UserCheckerTest extends TestCase * @expectedException \Symfony\Component\Security\Core\Exception\CredentialsExpiredException */ public function testCheckPostAuthCredentialsExpired() + { + $checker = new UserChecker(); + $checker->checkPostAuth(new User('John', 'password', array(), true, true, false, true)); + } + + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPostAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + * @expectedException \Symfony\Component\Security\Core\Exception\CredentialsExpiredException + */ + public function testCheckPostAuthCredentialsExpiredAdvancedUser() { $checker = new UserChecker(); @@ -46,14 +68,11 @@ class UserCheckerTest extends TestCase $checker->checkPostAuth($account); } - public function testCheckPreAuthNotAdvancedUserInterface() - { - $checker = new UserChecker(); - - $this->assertNull($checker->checkPreAuth($this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock())); - } - - public function testCheckPreAuthPass() + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPreAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + */ + public function testCheckPreAuthPassAdvancedUser() { $checker = new UserChecker(); @@ -69,6 +88,17 @@ class UserCheckerTest extends TestCase * @expectedException \Symfony\Component\Security\Core\Exception\LockedException */ public function testCheckPreAuthAccountLocked() + { + $checker = new UserChecker(); + $checker->checkPreAuth(new User('John', 'password', array(), true, true, false, false)); + } + + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPreAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + * @expectedException \Symfony\Component\Security\Core\Exception\LockedException + */ + public function testCheckPreAuthAccountLockedAdvancedUser() { $checker = new UserChecker(); @@ -82,6 +112,17 @@ class UserCheckerTest extends TestCase * @expectedException \Symfony\Component\Security\Core\Exception\DisabledException */ public function testCheckPreAuthDisabled() + { + $checker = new UserChecker(); + $checker->checkPreAuth(new User('John', 'password', array(), false, true, false, true)); + } + + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPreAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + * @expectedException \Symfony\Component\Security\Core\Exception\DisabledException + */ + public function testCheckPreAuthDisabledAdvancedUser() { $checker = new UserChecker(); @@ -96,6 +137,17 @@ class UserCheckerTest extends TestCase * @expectedException \Symfony\Component\Security\Core\Exception\AccountExpiredException */ public function testCheckPreAuthAccountExpired() + { + $checker = new UserChecker(); + $checker->checkPreAuth(new User('John', 'password', array(), true, false, true, true)); + } + + /** + * @group legacy + * @expectedDeprecation Calling Symfony\Component\Security\Core\User\UserChecker::checkPreAuth with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality. + * @expectedException \Symfony\Component\Security\Core\Exception\AccountExpiredException + */ + public function testCheckPreAuthAccountExpiredAdvancedUser() { $checker = new UserChecker(); diff --git a/src/Symfony/Component/Security/Core/Tests/User/UserTest.php b/src/Symfony/Component/Security/Core/Tests/User/UserTest.php index fa9f33107d..f052d85a1e 100644 --- a/src/Symfony/Component/Security/Core/Tests/User/UserTest.php +++ b/src/Symfony/Component/Security/Core/Tests/User/UserTest.php @@ -12,7 +12,9 @@ namespace Symfony\Component\Security\Core\Tests\User; use PHPUnit\Framework\TestCase; +use Symfony\Component\Security\Core\User\EquatableInterface; use Symfony\Component\Security\Core\User\User; +use Symfony\Component\Security\Core\User\UserInterface; class UserTest extends TestCase { @@ -99,4 +101,37 @@ class UserTest extends TestCase $user = new User('fabien', 'superpass'); $this->assertEquals('fabien', (string) $user); } + + /** + * @dataProvider isEqualToData + * + * @param bool $expectation + * @param EquatableInterface|UserInterface $a + * @param EquatableInterface|UserInterface $b + */ + public function testIsEqualTo($expectation, $a, $b) + { + $this->assertSame($expectation, $a->isEqualTo($b)); + $this->assertSame($expectation, $b->isEqualTo($a)); + } + + public static function isEqualToData() + { + return array( + array(true, new User('username', 'password'), new User('username', 'password')), + array(true, new User('username', 'password', array('ROLE')), new User('username', 'password')), + array(true, new User('username', 'password', array('ROLE')), new User('username', 'password', array('NO ROLE'))), + array(false, new User('diff', 'diff'), new User('username', 'password')), + array(false, new User('diff', 'diff', array(), false), new User('username', 'password')), + array(false, new User('diff', 'diff', array(), false, false), new User('username', 'password')), + array(false, new User('diff', 'diff', array(), false, false, false), new User('username', 'password')), + array(false, new User('diff', 'diff', array(), false, false, false, false), new User('username', 'password')), + ); + } + + public function testIsEqualToWithDifferentUser() + { + $user = new User('username', 'password'); + $this->assertFalse($user->isEqualTo($this->getMockBuilder(UserInterface::class)->getMock())); + } } diff --git a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php index 087c3c6e26..083833bc1f 100644 --- a/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php +++ b/src/Symfony/Component/Security/Core/User/AdvancedUserInterface.php @@ -32,6 +32,7 @@ use Symfony\Component\Security\Core\Exception\DisabledException; * * @see UserInterface * @see AccountStatusException + * @deprecated since version 4.1, will be removed in 5.0. * * @author Fabien Potencier */ diff --git a/src/Symfony/Component/Security/Core/User/EquatableInterface.php b/src/Symfony/Component/Security/Core/User/EquatableInterface.php index 4878637454..704081014c 100644 --- a/src/Symfony/Component/Security/Core/User/EquatableInterface.php +++ b/src/Symfony/Component/Security/Core/User/EquatableInterface.php @@ -26,9 +26,6 @@ interface EquatableInterface * However, you do not need to compare every attribute, but only those that * are relevant for assessing whether re-authentication is required. * - * Also implementation should consider that $user instance may implement - * the extended user interface `AdvancedUserInterface`. - * * @return bool */ public function isEqualTo(UserInterface $user); diff --git a/src/Symfony/Component/Security/Core/User/User.php b/src/Symfony/Component/Security/Core/User/User.php index 1f13b6630b..4d612798f2 100644 --- a/src/Symfony/Component/Security/Core/User/User.php +++ b/src/Symfony/Component/Security/Core/User/User.php @@ -18,7 +18,7 @@ namespace Symfony\Component\Security\Core\User; * * @author Fabien Potencier */ -final class User implements AdvancedUserInterface +final class User implements UserInterface, EquatableInterface, AdvancedUserInterface { private $username; private $password; @@ -117,4 +117,44 @@ final class User implements AdvancedUserInterface public function eraseCredentials() { } + + /** + * {@inheritdoc} + */ + public function isEqualTo(UserInterface $user) + { + if (!$user instanceof self) { + return false; + } + + if ($this->getPassword() !== $user->getPassword()) { + return false; + } + + if ($this->getSalt() !== $user->getSalt()) { + return false; + } + + if ($this->getUsername() !== $user->getUsername()) { + return false; + } + + if ($this->isAccountNonExpired() !== $user->isAccountNonExpired()) { + return false; + } + + if ($this->isAccountNonLocked() !== $user->isAccountNonLocked()) { + return false; + } + + if ($this->isCredentialsNonExpired() !== $user->isCredentialsNonExpired()) { + return false; + } + + if ($this->isEnabled() !== $user->isEnabled()) { + return false; + } + + return true; + } } diff --git a/src/Symfony/Component/Security/Core/User/UserChecker.php b/src/Symfony/Component/Security/Core/User/UserChecker.php index ac577a37ba..308b6d96d5 100644 --- a/src/Symfony/Component/Security/Core/User/UserChecker.php +++ b/src/Symfony/Component/Security/Core/User/UserChecker.php @@ -28,10 +28,14 @@ class UserChecker implements UserCheckerInterface */ public function checkPreAuth(UserInterface $user) { - if (!$user instanceof AdvancedUserInterface) { + if (!$user instanceof AdvancedUserInterface && !$user instanceof User) { return; } + if ($user instanceof AdvancedUserInterface && !$user instanceof User) { + @trigger_error(sprintf('Calling %s with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality.', __METHOD__), E_USER_DEPRECATED); + } + if (!$user->isAccountNonLocked()) { $ex = new LockedException('User account is locked.'); $ex->setUser($user); @@ -56,10 +60,14 @@ class UserChecker implements UserCheckerInterface */ public function checkPostAuth(UserInterface $user) { - if (!$user instanceof AdvancedUserInterface) { + if (!$user instanceof AdvancedUserInterface && !$user instanceof User) { return; } + if ($user instanceof AdvancedUserInterface && !$user instanceof User) { + @trigger_error(sprintf('Calling %s with an AdvancedUserInterface is deprecated as of 4.1 and will be removed in 5.0. Create a custom user checker if you wish to keep this functionality.', __METHOD__), E_USER_DEPRECATED); + } + if (!$user->isCredentialsNonExpired()) { $ex = new CredentialsExpiredException('User credentials have expired.'); $ex->setUser($user); diff --git a/src/Symfony/Component/Security/Core/User/UserInterface.php b/src/Symfony/Component/Security/Core/User/UserInterface.php index 747884282d..0a359d079d 100644 --- a/src/Symfony/Component/Security/Core/User/UserInterface.php +++ b/src/Symfony/Component/Security/Core/User/UserInterface.php @@ -11,8 +11,6 @@ namespace Symfony\Component\Security\Core\User; -use Symfony\Component\Security\Core\Role\Role; - /** * Represents the interface that all user classes must implement. * @@ -27,7 +25,6 @@ use Symfony\Component\Security\Core\Role\Role; * loaded by different objects that implement UserProviderInterface * * @see UserProviderInterface - * @see AdvancedUserInterface * * @author Fabien Potencier */