8000 minor #59539 [Security] Don't invalidate the user when the password w… · symfony/symfony@0134078 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0134078

Browse files
nicolas-grekasnicolas-grekas
committed
minor #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 #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
2 parents 5876c48 + 3d618db commit 0134078

File tree

5 files changed

+88
-30
lines changed

5 files changed

+88
-30
lines changed

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

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
<?php
22

3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
312
namespace Symfony\Component\Security\Core\Tests\Authentication\Token\Fixtures;
413

514
use Symfony\Component\Security\Core\User\UserInterface;
615

716
final class CustomUser implements UserInterface
817
{
9-
/** @var string */
10-
private $username;
11-
/** @var array */
12-
private $roles;
13-
14-
public function __construct(string $username, array $roles)
15-
{
16-
$this->username = $username;
17-
$this->roles = $roles;
18+
public function __construct(
19+
private string $username,
20+
private array $roles,
21+
) {
1822
}
1923

2024
public function getUserIdentifier(): string
@@ -27,16 +31,6 @@ public function getRoles(): array
2731
return $this->roles;
2832
}
2933

30-
public function getPassword(): ?string
31-
{
32-
return null;
33-
}
34-
35-
public function getSalt(): ?string
36-
{
37-
return null;
38-
}
39-
4034
public function eraseCredentials(): void
4135
{
4236
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ public function onKernelResponse(ResponseEvent $event): void
191191
*
192192
* @throws \RuntimeException
193193
*/
194-
protected function refreshUser(TokenInterface $token): ?TokenInterface
194+
private function refreshUser(TokenInterface $token): ?TokenInterface
195195
{
196196
$user = $token->getUser();
197197

@@ -292,7 +292,10 @@ private static function hasUserChanged(UserInterface $originalUser, TokenInterfa
292292
}
293293

294294
if ($originalUser instanceof PasswordAuthenticatedUserInterface || $refreshedUser instanceof PasswordAuthenticatedUserInterface) {
295-
if (!$originalUser instanceof PasswordAuthenticatedUserInterface || !$refreshedUser instanceof PasswordAuthenticatedUserInterface || $originalUser->getPassword() !== $refreshedUser->getPassword()) {
295+
if (!$originalUser instanceof PasswordAuthenticatedUserInterface
296+
|| !$refreshedUser instanceof PasswordAuthenticatedUserInterface
297+
|| $refreshedUser->getPassword() !== ($originalUser->getPassword() ?? $refreshedUser->getPassword())
298+
) {
296299
return true;
297300
}
298301

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,7 @@ public function testHandleWhenTheAccessDecisionManagerDecidesToRefuseAccess()
4242
->willReturn([['foo' => 'bar'], null])
4343
;
4444

45-
$token = new class extends AbstractToken {
46-
public function getCredentials(): mixed
47-
{
48-
}
49-
};
45+
$token = new class extends AbstractToken {};
5046

5147
$tokenStorage = $this->createMock(TokenStorageInterface::class);
5248
$tokenStorage

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
use Symfony\Component\Security\Core\User\UserInterface;
3737
use Symfony\Component\Security\Core\User\UserProviderInterface;
3838
use Symfony\Component\Security\Http\Firewall\ContextListener;
39+
use Symfony\Component\Security\Http\Tests\Fixtures\CustomUser;
3940
use Symfony\Component\Security\Http\Tests\Fixtures\NullUserToken;
4041
use Symfony\Contracts\Service\ServiceLocatorTrait;
4142

@@ -376,6 +377,25 @@ public function testOnKernelResponseRemoveListener()
376377
$this->assertEmpty($dispatcher->getListeners());
377378
}
378379

380+
public function testRemovingPasswordFromSessionDoesntInvalidateTheToken()
381+
{
382+
$user = new CustomUser('user', ['ROLE_USER'], 'pass');
383+
384+
$userProvider = $this->createMock(UserProviderInterface::class);
385+
$userProvider->expects($this->once())
386+
->method('supportsClass')
387+
->with(CustomUser::class)
388+
->willReturn(true);
389+
$userProvider->expects($this->once())
390+
->method('refreshUser')
391+
->willReturn($user);
392+
393+
$tokenStorage = $this->handleEventWithPreviousSession([$userProvider], $user);
394+
395+
$this->assertInstanceOf(UsernamePasswordToken::class, $tokenStorage->getToken());
396+
$this->assertSame($user, $tokenStorage->getToken()->getUser());
397+
}
398+
379399
protected function runSessionOnKernelResponse($newToken, $original = null)
380400
{
381401
$session = new Session(new MockArraySessionStorage());
@@ -568,10 +588,6 @@ public function getRoleNames(): array
568588
return $this->roles;
569589
}
570590

571-
public function getCredentials()
572-
{
573-
}
574-
575591
public function getUser(): UserInterface
576592
{
577593
return $this->user;

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

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\Security\Http\Tests\Fixtures;
13+
14+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
15+
use Symfony\Component\Security\Core\User\UserInterface;
16+
17+
final class CustomUser implements UserInterface, PasswordAuthenticatedUserInterface
18+
{
19+
public function __construct(
20+
private string $username,
21+
private array $roles,
22+
private ?string $password = null,
23+
) {
24+
}
25+
26+
public function getUserIdentifier(): string
27+
{
28+
return $this->username;
29+
}
30+
31+
public function getRoles(): array
32+
{
33+
return $this->roles;
34+
}
35+
36+
public function getPassword(): ?string
37+
{
38+
return $this->password ?? null;
39+
}
40+
41+
public function eraseCredentials(): void
42+
{
43+
}
44+
45+
public function __serialize(): array
46+
{
47+
return [\sprintf("\0%s\0username", self::class) => $this->username];
48+
}
49+
}

0 commit comments

Comments
 (0)
0