minor symfony#59539 [Security] Don't invalidate the user when the password was not stored in the session (nicolas-grekas)

This PR was merged into the 7.3 branch.

Discussion
----------

[Security] Don't invalidate the user when the password was not stored in the session

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Related to symfony#59106: this PR does is considering that if `$originalUser` (the object coming from the session) has a null password, then we don't consider it changed from `$refreshedUser`. Aka we don't log out the user in such case.

The benefit is allowing to not put the hashed password in the session. I think that's desirable.

Commits
-------

3d618db [Security] Don't invalidate the user when the password was not stored in the session

Author: nicolas-grekas

src/Symfony/Component/Security/Core/Tests/Authentication/Token/Fixtures/CustomUser.php

@@ -1,20 +1,24 @@
 <?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\Tests\Authentication\Token\Fixtures;
 
 use Symfony\Component\Security\Core\User\UserInterface;
 
 final class CustomUser implements UserInterface
 {
-    /** @var string */
-    private $username;
-    /** @var array */
-    private $roles;
-
-    public function __construct(string $username, array $roles)
-    {
-        $this->username = $username;
-        $this->roles = $roles;
+    public function __construct(
+        private string $username,
+        private array $roles,
+    ) {
     }
 
     public function getUserIdentifier(): string
@@ -27,16 +31,6 @@ public function getRoles(): array
         return $this->roles;
     }
 
-    public function getPassword(): ?string
-    {
-        return null;
-    }
-
-    public function getSalt(): ?string
-    {
-        return null;
-    }
-
     public function eraseCredentials(): void
     {
     }

src/Symfony/Component/Security/Http/Firewall/ContextListener.php

@@ -191,7 +191,7 @@ public function onKernelResponse(ResponseEvent $event): void
      *
      * @throws \RuntimeException
      */
-    protected function refreshUser(TokenInterface $token): ?TokenInterface
+    private function refreshUser(TokenInterface $token): ?TokenInterface
     {
         $user = $token->getUser();
 
@@ -292,7 +292,10 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa
         }
 
         if ($originalUser instanceof PasswordAuthenticatedUserInterface || $refreshedUser instanceof PasswordAuthenticatedUserInterface) {
-            if (!$originalUser instanceof PasswordAuthenticatedUserInterface || !$refreshedUser instanceof PasswordAuthenticatedUserInterface || $originalUser->getPassword() !== $refreshedUser->getPassword()) {
+            if (!$originalUser instanceof PasswordAuthenticatedUserInterface
+                || !$refreshedUser instanceof PasswordAuthenticatedUserInterface
+                || $refreshedUser->getPassword() !== ($originalUser->getPassword() ?? $refreshedUser->getPassword())
+            ) {
                 return true;
             }
 

src/Symfony/Component/Security/Http/Tests/Firewall/AccessListenerTest.php

@@ -42,11 +42,7 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess()
             ->willReturn([['foo' => 'bar'], null])
         ;
 
-        $token = new class extends AbstractToken {
-            public function getCredentials(): mixed
-            {
-            }
-        };
+        $token = new class extends AbstractToken {};
 
         $tokenStorage = $this->createMock(TokenStorageInterface::class);
         $tokenStorage

src/Symfony/Component/Security/Http/Tests/Firewall/ContextListenerTest.php

@@ -36,6 +36,7 @@
 use Symfony\Component\Security\Core\User\UserInterface;
 use Symfony\Component\Security\Core\User\UserProviderInterface;
 use Symfony\Component\Security\Http\Firewall\ContextListener;
+use Symfony\Component\Security\Http\Tests\Fixtures\CustomUser;
 use Symfony\Component\Security\Http\Tests\Fixtures\NullUserToken;
 use Symfony\Contracts\Service\ServiceLocatorTrait;
 
@@ -376,6 +377,25 @@ public function testOnKernelResponseRemoveListener()
         $this->assertEmpty($dispatcher->getListeners());
     }
 
+    public function testRemovingPasswordFromSessionDoesntInvalidateTheToken()
+    {
+        $user = new CustomUser('user', ['ROLE_USER'], 'pass');
+
+        $userProvider = $this->createMock(UserProviderInterface::class);
+        $userProvider->expects($this->once())
+            ->method('supportsClass')
+            ->with(CustomUser::class)
+            ->willReturn(true);
+        $userProvider->expects($this->once())
+            ->method('refreshUser')
+            ->willReturn($user);
+
+        $tokenStorage = $this->handleEventWithPreviousSession([$userProvider], $user);
+
+        $this->assertInstanceOf(UsernamePasswordToken::class, $tokenStorage->getToken());
+        $this->assertSame($user, $tokenStorage->getToken()->getUser());
+    }
+
     protected function runSessionOnKernelResponse($newToken, $original = null)
     {
         $session = new Session(new MockArraySessionStorage());
@@ -568,10 +588,6 @@ public function getRoleNames(): array
         return $this->roles;
     }
 
-    public function getCredentials()
-    {
-    }
-
     public function getUser(): UserInterface
     {
         return $this->user;

src/Symfony/Component/Security/Http/Tests/Fixtures/CustomUser.php

@@ -0,0 +1,49 @@
+<?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\Http\Tests\Fixtures;
+
+use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
+use Symfony\Component\Security\Core\User\UserInterface;
+
+final class CustomUser implements UserInterface, PasswordAuthenticatedUserInterface
+{
+    public function __construct(
+        private string $username,
+        private array $roles,
+        private ?string $password = null,
+    ) {
+    }
+
+    public function getUserIdentifier(): string
+    {
+        return $this->username;
+    }
+
+    public function getRoles(): array
+    {
+        return $this->roles;
+    }
+
+    public function getPassword(): ?string
+    {
+        return $this->password ?? null;
+    }
+
+    public function eraseCredentials(): void
+    {
+    }
+
+    public function __serialize(): array
+    {
+        return [\sprintf("\0%s\0username", self::class) => $this->username];
+    }
+}