8000 bug #41755 [PasswordHasher] UserPasswordHasher only calls getSalt whe… · symfony/symfony@19cd5b4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 19cd5b4

Browse files
derrabusderrabus
committed
bug #41755 [PasswordHasher] UserPasswordHasher only calls getSalt when method exists (dbrumann)
This PR was squashed before being merged into the 5.3 branch. Discussion ---------- [PasswordHasher] UserPasswordHasher only calls getSalt when method exists | Q | A | ------------- | --- | Branch? | 5.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #41753 | License | MIT Commits ------- 7f9c9d6 [PasswordHasher] UserPasswordHasher only calls getSalt when method exists
2 parents 07ba79a + 7f9c9d6 commit 19cd5b4

File tree

4 files changed

+147
-28
lines changed

4 files changed

+147
-28
lines changed

‎src/Symfony/Component/PasswordHasher/Hasher/UserPasswordHasher.php

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,16 @@ public function hashPassword($user, string $plainPassword): string
4343
trigger_deprecation('symfony/password-hasher', '5.3', 'The "%s()" method expects a "%s" instance as first argument. Not implementing it in class "%s" is deprecated.', __METHOD__, PasswordAuthenticatedUserInterface::class, get_debug_type($user));
4444
}
4545

46-
$salt = $user->getSalt();
47-
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
48-
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
46+
$salt = null;
47+
48+
if ($user instanceof LegacyPasswordAuthenticatedUserInterface) {
49+
$salt = $user->getSalt();
50+
} elseif ($user instanceof UserInterface) {
51+
$salt = $user->getSalt();
52+
53+
if (null !== $salt) {
54+
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
55+
}
4956
}
5057

5158
$hasher = $this->hasherFactory->getPasswordHasher($user);
@@ -65,9 +72,16 @@ public function isPasswordValid($user, string $plainPassword): bool
6572
trigger_deprecation('symfony/password-hasher', '5.3', 'The "%s()" method expects a "%s" instance as first argument. Not implementing it in class "%s" is deprecated.', __METHOD__, PasswordAuthenticatedUserInterface::class, get_debug_type($user));
6673
}
6774

68-
$salt = $user->getSalt();
69-
if ($salt && !$user instanceof LegacyPasswordAuthenticatedUserInterface) {
70-
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
75+
$salt = null;
76+
77+
if ($user instanceof LegacyPasswordAuthenticatedUserInterface) {
78+
$salt = $user->getSalt();
79+
} elseif ($user instanceof UserInterface) {
80+
$salt = $user->getSalt();
81+
82+
if (null !== $salt) {
83+
trigger_deprecation('symfony/password-hasher', '5.3', 'Returning a string from "getSalt()" without implementing the "%s" interface is deprecated, the "%s" class should implement it.', LegacyPasswordAuthenticatedUserInterface::class, get_debug_type($user));
84+
}
7185
}
7286

7387
if (null === $user->getPassword()) {

‎src/Symfony/Component/PasswordHasher/Tests/Fixtures/TestLegacyPasswordAuthenticatedUser.php

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<?php
2+
3+
namespace Symfony\Component\PasswordHasher\Tests\Fixtures;
4+
5+
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
6+
use Symfony\Component\Security\Core\User\UserInterface;
7+
8+
final class TestLegacyPasswordAuthenticatedUser implements LegacyPasswordAuthenticatedUserInterface, UserInterface
9+
{
10+
private $username;
11+
private $password;
12+
private $salt;
13+
private $roles;
14+
15+
public function __construct(string $username, ?string $password = null, ?string $salt = null, array $roles = [])
16+
{
17+
$this->roles = $roles;
18+
$this->salt = $salt;
19+
$this->password = $password;
20+
$this->username = $username;
21+
}
22+
23+
public function getSalt(): ?string
24+
{
25+
return $this->salt;
26+
}
27+
28+
public function getPassword(): ?string
29+
{
30+
return $this->password;
31+
}
32+
33+
public function getRoles()
34+
{
35+
return $this->roles;
36+
}
37+
38+
public function eraseCredentials()
39+
{
40+
// Do nothing
41+
return;
42+
}
43+
44+
public function getUsername()
45+
{
46+
return $this->username;
47+
}
48+
49+
public function getUserIdentifier()
50+
{
51+
return $this->username;
52+
}
53+
}

‎src/Symfony/Component/PasswordHasher/Tests/Fixtures/TestPasswordAuthenticatedUser.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php
2+
3+
namespace Symfony\Component\PasswordHasher\Tests\Fixtures;
4+
5+
use Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface;
6+
7+
final class TestPasswordAuthenticatedUser implements PasswordAuthenticatedUserInterface
8+
{
9+
private $password;
10+
11+
public function __construct(?string $password = null)
12+
{
13+
$this->password = $password;
14+
}
15+
16+
public function getPassword(): ?string
17+
{
18+
return $this->password;
19+
}
20+
}

‎src/Symfony/Component/PasswordHasher/Tests/Hasher/UserPasswordHasherTest.php

Lines changed: 54 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@
1717
use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactoryInterface;
1818
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher;
1919
use Symfony\Component\PasswordHasher\PasswordHasherInterface;
20+
use Symfony\Component\PasswordHasher\Tests\Fixtures\TestLegacyPasswordAuthenticatedUser;
21+
use Symfony\Component\PasswordHasher\Tests\Fixtures\TestPasswordAuthenticatedUser;
2022
use Symfony\Component\Security\Core\User\InMemoryUser;
21-
use Symfony\Component\Security\Core\User\LegacyPasswordAuthenticatedUserInterface;
2223
use Symfony\Component\Security\Core\User\User;
23-
use Symfony\Component\Security\Core\User\UserInterface;
2424

2525
class UserPasswordHasherTest extends TestCase
2626
{
@@ -56,12 +56,9 @@ public function testHashWithNonPasswordAuthenticatedUser()
5656
$this->assertEquals('hash', $encoded);
5757
}
5858

59-
public function testHash()
59+
public function testHashWithLegacyUser()
6060
{
61-
$userMock = $this->createMock(TestPasswordAuthenticatedUser::class);
62-
$userMock->expects($this->any())
63-
->method('getSalt')
64-
->willReturn('userSalt');
61+
$user = new TestLegacyPasswordAuthenticatedUser('name', null, 'userSalt');
6562

6663
$mockHasher = $this->createMock(PasswordHasherInterface::class);
6764
$mockHasher->expects($this->any())
@@ -72,25 +69,42 @@ public function testHash()
7269
$mockPasswordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class);
7370
$mockPasswordHasherFactory->expects($this->any())
7471
->method('getPasswordHasher')
75-
->with($this->equalTo($userMock))
72+
->with($user)
7673
->willReturn($mockHasher);
7774

7875
$passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory);
7976

80-
$encoded = $passwordHasher->hashPassword($userMock, 'plainPassword');
77+
$encoded = $passwordHasher->hashPassword($user, 'plainPassword');
8178
$this->assertEquals('hash', $encoded);
8279
}
8380

84-
public function testVerify()
81+
public function testHashWithPasswordAuthenticatedUser()
8582
{
86-
$userMock = $this->createMock(TestPasswordAuthenticatedUser::class);
87-
$userMock->expects($this->any())
88-
->method('getSalt')
89-
->willReturn('userSalt');
90-
$userMock->expects($this->any())
91-
->method('getPassword')
83+
$user = new TestPasswordAuthenticatedUser();
84+
85+
$mockHasher = $this->createMock(PasswordHasherInterface::class);
86+
$mockHasher->expects($this->any())
87+
->method('hash')
88+
->with($this->equalTo('plainPassword'), $this->equalTo(null))
9289
->willReturn('hash');
9390

91+
$mockPasswordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class);
92+
$mockPasswordHasherFactory->expects($this->any())
93+
->method('getPasswordHasher')
94+
->with($user)
95+
->willReturn($mockHasher);
96+
97+
$passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory);
98+
99+
$hashedPassword = $passwordHasher->hashPassword($user, 'plainPassword');
100+
101+
$this->assertSame('hash', $hashedPassword);
102+
}
103+
104+
public function testVerifyWithLegacyUser()
105+
{
106+
$user = new TestLegacyPasswordAuthenticatedUser('user', 'hash', 'userSalt');
107+
94108
$mockHasher = $this->createMock(PasswordHasherInterface::class);
95109
$mockHasher->expects($this->any())
96110
->method('verify')
@@ -100,12 +114,34 @@ public function testVerify()
100114
$mockPasswordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class);
101115
$mockPasswordHasherFactory->expects($this->any())
102116
->method('getPasswordHasher')
103-
->with($this->equalTo($userMock))
117+
->with($user)
118+
->willReturn($mockHasher);
119+
120+
$passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory);
121+
122+
$isValid = $passwordHasher->isPasswordValid($user, 'plainPassword');
123+
$this->assertTrue($isValid);
124+
}
125+
126+
public function testVerify()
127+
{
128+
$user = new TestPasswordAuthenticatedUser('hash');
129+
130+
$mockHasher = $this->createMock(PasswordHasherInterface::class);
131+
$mockHasher->expects($this->any())
132+
->method('verify')
133+
->with($this->equalTo('hash'), $this->equalTo('plainPassword'), $this->equalTo(null))
134+
->willReturn(true);
135+
136+
$mockPasswordHasherFactory = $this->createMock(PasswordHasherFactoryInterface::class);
137+
$mockPasswordHasherFactory->expects($this->any())
138+
->method('getPasswordHasher')
139+
->with($user)
104140
->willReturn($mockHasher);
105141

106142
$passwordHasher = new UserPasswordHasher($mockPasswordHasherFactory);
107143

108-
$isValid = $passwordHasher->isPasswordValid($userMock, 'plainPassword');
144+
$isValid = $passwordHasher->isPasswordValid($user, 'plainPassword');
109145
$this->assertTrue($isValid);
110146
}
111147

@@ -128,7 +164,3 @@ public function testNeedsRehash()
128164
$this->assertFalse($passwordHasher->needsRehash($user));
129165
}
130166
}
131-
132-
abstract class TestPasswordAuthenticatedUser implements LegacyPasswordAuthenticatedUserInterface, UserInterface
133-
{
134-
}

0 commit comments

Comments
 (0)
0