Skip to content

Commit 34303a9

Browse files
wouterjwouterj
committed
[Security] Rename UserProviderInterface::loadUserByUsername() to loadUserByIdentifier()
1 parent 486cc29 commit 34303a9

File tree

48 files changed

+456
-213
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+456
-213
lines changed

‎UPGRADE-5.3.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ Security
169169

170170
* Deprecate `UserInterface::getUsername()` in favor of `UserInterface::getUserIdentifier()`
171171
* Deprecate `TokenInterface::getUsername()` in favor of `TokenInterface::getUserIdentifier()`
172+
* Deprecate `UserProviderInterface::loadUserByUsername()` in favor of `UserProviderInterface::loadUserByIdentifier()`
172173
* Deprecate calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface`
173174
* Deprecate calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface` with a `UserInterface` instance that does not implement `PasswordAuthenticatedUserInterface`
174175
* Deprecate all classes in the `Core\Encoder\` sub-namespace, use the `PasswordHasher` component instead

‎UPGRADE-6.0.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ Security
257257

258258
* Remove `UserInterface::getUsername()` in favor of `UserInterface::getUserIdentifier()`
259259
* Remove `TokenInterface::getUsername()` in favor of `TokenInterface::getUserIdentifier()`
260+
* Deprecate `UserProviderInterface::loadUserByUsername()` in favor of `UserProviderInterface::loadUserByIdentifier()`
260261
* Calling `PasswordUpgraderInterface::upgradePassword()` with a `UserInterface` instance that
261262
does not implement `PasswordAuthenticatedUserInterface` now throws a `\TypeError`.
262263
* Calling methods `hashPassword()`, `isPasswordValid()` and `needsRehash()` on `UserPasswordHasherInterface`

‎src/Symfony/Bridge/Doctrine/Security/User/EntityUserProvider.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,21 +50,28 @@ public function __construct(ManagerRegistry $registry, string $classOrAlias, str
5050
* {@inheritdoc}
5151
*/
5252
public function loadUserByUsername(string $username)
53+
{
54+
trigger_deprecation('symfony/doctrine-bridge', '5.3', 'Method "%s()" is deprecated, use loadUserByIdentifier() instead.', __METHOD__);
55+
56+
return $this->loadUserByIdentifier($username);
57+
}
58+
59+
public function loadUserByIdentifier(string $identifier): UserInterface
5360
{
5461
$repository = $this->getRepository();
5562
if (null !== $this->property) {
56-
$user = $repository->findOneBy([$this->property => $username]);
63+
$user = $repository->findOneBy([$this->property => $identifier]);
5764
} else {
5865
if (!$repository instanceof UserLoaderInterface) {
5966
throw new \InvalidArgumentException(sprintf('You must either make the "%s" entity Doctrine Repository ("%s") implement "Symfony\Bridge\Doctrine\Security\User\UserLoaderInterface" or set the "property" option in the corresponding entity provider configuration.', $this->classOrAlias, get_debug_type($repository)));
6067
}
6168

62-
$user = $repository->loadUserByUsername($username);
69+
$user = $repository->loadUserByUsername($identifier);
6370
}
6471

6572
if (null === $user) {
66-
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
67-
$e->setUsername($username);
73+
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $identifier));
74+
$e->setUsername($identifier);
6875

6976
throw $e;
7077
}

‎src/Symfony/Bridge/Doctrine/Tests/Security/User/EntityUserProviderTest.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function testLoadUserByUsername()
6060

6161
$provider = new EntityUserProvider($this->getManager($em), 'Symfony\Bridge\Doctrine\Tests\Fixtures\User', 'name');
6262

63-
$this->assertSame($user, $provider->loadUserByUsername('user1'));
63+
$this->assertSame($user, $provider->loadUserByIdentifier('user1'));
6464
}
6565

6666
public function testLoadUserByUsernameWithUserLoaderRepositoryAndWithoutProperty()
@@ -82,7 +82,7 @@ public function testLoadUserByUsernameWithUserLoaderRepositoryAndWithoutProperty
8282
->willReturn($repository);
8383

8484
$provider = new EntityUserProvider($this->getManager($em), 'Symfony\Bridge\Doctrine\Tests\Fixtures\User');
85-
$this->assertSame($user, $provider->loadUserByUsername('user1'));
85+
$this->assertSame($user, $provider->loadUserByIdentifier('user1'));
8686
}
8787

8888
public function testLoadUserByUsernameWithNonUserLoaderRepositoryAndWithoutProperty()
@@ -98,7 +98,7 @@ public function testLoadUserByUsernameWithNonUserLoaderRepositoryAndWithoutPrope
9898
$em->flush();
9999

100100
$provider = new EntityUserProvider($this->getManager($em), 'Symfony\Bridge\Doctrine\Tests\Fixtures\User');
101-
$provider->loadUserByUsername('user1');
101+
$provider->loadUserByIdentifier('user1');
102102
}
103103

104104
public function testRefreshUserRequiresId()
@@ -164,7 +164,7 @@ public function testLoadUserByUserNameShouldLoadUserWhenProperInterfaceProvided(
164164
'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
165165
);
166166

167-
$provider->loadUserByUsername('name');
167+
$provider->loadUserByIdentifier('name');
168168
}
169169

170170
public function testLoadUserByUserNameShouldDeclineInvalidInterface()
@@ -177,7 +177,7 @@ public function testLoadUserByUserNameShouldDeclineInvalidInterface()
177177
'Symfony\Bridge\Doctrine\Tests\Fixtures\User'
178178
);
179179

180-
$provider->loadUserByUsername('name');
180+
$provider->loadUserByIdentifier('name');
181181
}
182182

183183
public function testPasswordUpgrades()

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/Bundle/SecuredPageBundle/Security/Core/User/ArrayUserProvider.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,16 @@ public function getUser($username)
3131

3232
public function loadUserByUsername($username)
3333
{
34-
$user = $this->getUser($username);
34+
return $this->loadUserByIdentifier($username);
35+
}
36+
37+
public function loadUserByIdentifier(string $identifier): UserInterface
38+
{
39+
$user = $this->getUser($identifier);
3540

3641
if (null === $user) {
37-
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
38-
$e->setUsername($username);
42+
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $identifier));
43+
$e->setUsername($identifier);
3944

4045
throw $e;
4146
}

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/ClearRememberMeTest.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,12 @@ public function __construct(InMemoryUserProvider $inner)
6666

6767
public function loadUserByUsername($username)
6868
{
69-
return $this->inner->loadUserByUsername($username);
69+
return $this->loadUserByIdentifier($username);
70+
}
71+
72+
public function loadUserByIdentifier(string $identifier): UserInterface
73+
{
74+
return $this->inner->loadUserByIdentifier($identifier);
7075
}
7176

7277
public function refreshUser(UserInterface $user)

‎src/Symfony/Bundle/SecurityBundle/Tests/Functional/LoginLinkAuthenticationTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function testLoginLinkSuccess()
2828
$this->markTestSkipped('Login link auth requires symfony/security-http:^5.2');
2929
}
3030

31-
$client = $this->createClient(['test_case' => 'LoginLink', 'root_config' => 'config.yml']);
31+
$client = $this->createClient(['test_case' => 'LoginLink', 'root_config' => 'config.yml', 'debug' => true]);
3232

3333
// we need an active request that is under the firewall to use the linker
3434
$request = Request::create('/get-login-link');

‎src/Symfony/Component/Ldap/Security/LdapUserProvider.php

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function __construct(LdapInterface $ldap, string $baseDn, string $searchD
4848
}
4949

5050
if (null === $filter) {
51-
$filter = '({uid_key}={username})';
51+
$filter = '({uid_key}={user_identifier})';
5252
}
5353

5454
$this->ldap = $ldap;
@@ -66,15 +66,22 @@ public function __construct(LdapInterface $ldap, string $baseDn, string $searchD
6666
* {@inheritdoc}
6767
*/
6868
public function loadUserByUsername(string $username)
69+
{
70+
trigger_deprecation('symfony/ldap', '5.3', 'Method "%s()" is deprecated, use loadUserByIdentifier() instead.', __METHOD__);
71+
72+
return $this->loadUserByIdentifier($username);
73+
}
74+
75+
public function loadUserByIdentifier(string $identifier): UserInterface
6976
{
7077
try {
7178
$this->ldap->bind($this->searchDn, $this->searchPassword);
72-
$username = $this->ldap->escape($username, '', LdapInterface::ESCAPE_FILTER);
73-
$query = str_replace('{username}', $username, $this->defaultSearch);
79+
$identifier = $this->ldap->escape($identifier, '', LdapInterface::ESCAPE_FILTER);
80+
$query = str_replace(['{username}', '{user_identifier}'], $identifier, $this->defaultSearch);
7481
$search = $this->ldap->query($this->baseDn, $query);
7582
} catch (ConnectionException $e) {
76-
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $username), 0, $e);
77-
$e->setUsername($username);
83+
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $identifier), 0, $e);
84+
$e->setUsername($identifier);
7885

7986
throw $e;
8087
}
@@ -83,15 +90,15 @@ public function loadUserByUsername(string $username)
8390
$count = \count($entries);
8491

8592
if (!$count) {
86-
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $username));
87-
$e->setUsername($username);
93+
$e = new UsernameNotFoundException(sprintf('User "%s" not found.', $identifier));
94+
$e->setUsername($identifier);
8895

8996
throw $e;
9097
}
9198

9299
if ($count > 1) {
93100
$e = new UsernameNotFoundException('More than one user found.');
94-
$e->setUsername($username);
101+
$e->setUsername($identifier);
95102

96103
throw $e;
97104
}
@@ -100,12 +107,12 @@ public function loadUserByUsername(string $username)
100107

101108
try {
102109
if (null !== $this->uidKey) {
103-
$username = $this->getAttributeValue($entry, $this->uidKey);
110+
$identifier = $this->getAttributeValue($entry, $this->uidKey);
104111
}
105112
} catch (InvalidArgumentException $e) {
106113
}
107114

108-
return $this->loadUser($username, $entry);
115+
return $this->loadUser($identifier, $entry);
109116
}
110117

111118
/**
@@ -157,7 +164,7 @@ public function supportsClass(string $class)
157164
*
158165
* @return UserInterface
159166
*/
160-
protected function loadUser(string $username, Entry $entry)
167+
protected function loadUser(string $identifier, Entry $entry)
161168
{
162169
$password = null;
163170
$extraFields = [];
@@ -170,7 +177,7 @@ protected function loadUser(string $username, Entry $entry)
170177
$extraFields[$field] = $this->getAttributeValue($entry, $field);
171178
}
172179

173-
return new LdapUser($entry, $username, $password, $this->defaultRoles, $extraFields);
180+
return new LdapUser($entry, $identifier, $password, $this->defaultRoles, $extraFields);
174181
}
175182

176183
private function getAttributeValue(Entry $entry, string $attribute)

‎src/Symfony/Component/Ldap/Tests/Security/LdapUserProviderTest.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public function testLoadUserByUsernameFailsIfCantConnectToLdap()
3939
;
4040

4141
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
42-
$provider->loadUserByUsername('foo');
42+
$provider->loadUserByIdentifier('foo');
4343
}
4444

4545
public function testLoadUserByUsernameFailsIfNoLdapEntries()
@@ -71,7 +71,7 @@ public function testLoadUserByUsernameFailsIfNoLdapEntries()
7171
;
7272

7373
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
74-
$provider->loadUserByUsername('foo');
74+
$provider->loadUserByIdentifier('foo');
7575
}
7676

7777
public function testLoadUserByUsernameFailsIfMoreThanOneLdapEntry()
@@ -103,7 +103,7 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapEntry()
103103
;
104104

105105
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
106-
$provider->loadUserByUsername('foo');
106+
$provider->loadUserByIdentifier('foo');
107107
}
108108

109109
public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
@@ -144,7 +144,7 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
144144
;
145145

146146
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={username})', 'userpassword');
147-
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByUsername('foo'));
147+
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
148148
}
149149

150150
public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()
@@ -180,7 +180,7 @@ public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()
180180
;
181181

182182
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={username})');
183-
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByUsername('foo'));
183+
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
184184
}
185185

186186
public function testLoadUserByUsernameFailsIfEntryHasNoPasswordAttribute()
@@ -218,7 +218,7 @@ public function testLoadUserByUsernameFailsIfEntryHasNoPasswordAttribute()
218218
;
219219

220220
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={username})', 'userpassword');
221-
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByUsername('foo'));
221+
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
222222
}
223223

224224
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
@@ -254,7 +254,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
254254
;
255255

256256
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
257-
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByUsername('foo'));
257+
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
258258
}
259259

260260
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWrongCase()
@@ -290,7 +290,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWro
290290
;
291291

292292
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
293-
$this->assertSame('foo', $provider->loadUserByUsername('Foo')->getUserIdentifier());
293+
$this->assertSame('foo', $provider->loadUserByIdentifier('Foo')->getUserIdentifier());
294294
}
295295

296296
public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
@@ -330,7 +330,7 @@ public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
330330
;
331331

332332
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={username})', 'userpassword', ['email']);
333-
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByUsername('foo'));
333+
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
334334
}
335335

336336
public function testRefreshUserShouldReturnUserWithSameProperties()

‎src/Symfony/Component/Security/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ CHANGELOG
44
5.3
55
---
66

7+
* Deprecate `UserProviderInterface::loadUserByUsername()` in favor of `UserProviderInterface::loadUserByIdentifier()`
78
* Deprecate `TokenInterface::getUsername()` in favor of `TokenInterface::getUserIdentifier()`
89
* Deprecate `UserInterface::getUsername()` in favor of `getUserIdentifier()`
910
* Add `PasswordAuthenticatedUserInterface` for user classes that use passwords

0 commit comments

Comments
 (0)