feature #23508 Deprecated the AdvancedUserInterface (iltar)

This PR was merged into the 4.1-dev branch.

Discussion
----------

Deprecated the AdvancedUserInterface

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #23292
| License       | MIT
| Doc PR        | ~

This PR deprecates the usages of the `AdvancedUserInterface`.

Commits
-------

8456f3b Deprecated the AdvancedUserInterface

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
 --------------

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
 --------------

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
 -----

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;
         }
 

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(
+            array('foo', 'bar'),
-                'foo', 'bar',
+            array('foo', new TestUser('bar')),
-            ),
+            array('foo', $user),
-            array(
+            array('foo', $advancedUser),
-                'foo', new TestUser('bar'),
+            array($user, 'foo'),
-            ),
+            array($advancedUser, 'foo'),
-            array(
+            array($user, new TestUser('foo')),
-                'foo', $user,
+            array($advancedUser, new TestUser('foo')),
-            ),
+            array(new TestUser('foo'), new TestUser('bar')),
-            array(
+            array(new TestUser('foo'), 'bar'),
-                'foo', $advancedUser,
+            array(new TestUser('foo'), $user),
-            ),
+            array(new TestUser('foo'), $advancedUser),
-            array(
+            array($user, $advancedUser),
-                $user, 'foo',
+            array($advancedUser, $user),
-            ),
-            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,
-            ),
         );
     }
 

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()
+    /**
-    {
+     * @group legacy
-        $checker = new UserChecker();
+     * @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.
-
+     */
-        $this->assertNull($checker->checkPreAuth($this->getMockBuilder('Symfony\Component\Security\Core\User\UserInterface')->getMock()));
+    public function testCheckPreAuthPassAdvancedUser()
-    }
-
-    public function testCheckPreAuthPass()
     {
         $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();
 

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()));
+    }
 }

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 <fabien@symfony.com>
  */

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);

src/Symfony/Component/Security/Core/User/User.php

@@ -18,7 +18,7 @@ namespace Symfony\Component\Security\Core\User;
  *
  * @author Fabien Potencier <fabien@symfony.com>
  */
-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;
+    }
 }

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);

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 <fabien@symfony.com>
  */