8000 feature #46683 [Ldap] Deprecate '{username}' parameter use in favour … · symfony/symfony@e137bd3 · GitHub
[go: up one dir, main page]
Skip to content

Commit e137bd3

Browse files
fabpotfabpot
committed
feature #46683 [Ldap] Deprecate '{username}' parameter use in favour of '{user_identifier}' in LDAP configuration (EXT - THERAGE Kevin)
This PR was merged into the 6.2 branch. Discussion ---------- [Ldap] Deprecate '{username}' parameter use in favour of '{user_identifier}' in LDAP configuration | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | | License | MIT | Doc PR | This PR aims to fix a missing forward compatibility as done in src/Symfony/Component/Ldap/Security/LdapUserProvider.php at line 80 when authenticating with LDAP and using the following configuration : ```yaml security: // ... firewalls:œ // ... main: form_login_ldap: // ... query_string: '(whatever={user_identifier})' dn_string: 'uid={user_identifier},dc=example,dc=com' ``` instead of : ```yaml security: // ... firewalls: // ... main: form_login_ldap: // ... query_string: '(whatever={username})' dn_string: 'uid={username},dc=example,dc=com' ``` maybe related to : #40403 Commits ------- 6dd1221 [Ldap] Deprecate '{username}' parameter use in favour of '{user_identifier}' in LDAP configuration
2 parents 6e95b8b + 6dd1221 commit e137bd3

10 files changed

+93
-33
lines changed

‎UPGRADE-6.2.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,15 @@ HttpFoundation
1414

1515
* Deprecate `Request::getContentType()`, use `Request::getContentTypeFormat()` instead
1616

17+
Ldap
18+
----
19+
20+
* Deprecate `{username}` parameter use in favour of `{user_identifier}`
21+
1722
Mailer
18-
--------
23+
------
1924

20-
* Deprecate the `OhMySMTP` transport, use `MailPace` instead
25+
* Deprecate the `OhMySMTP` transport, use `MailPace` instead
2126

2227
Security
2328
--------

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
6.2
5+
---
6+
7+
* Deprecate `{username}` parameter use in favour of `{user_identifier}`
8+
49
6.1
510
---
611

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,17 @@ public function onCheckPassport(CheckPassportEvent $event)
8383
} else {
8484
throw new LogicException('Using the "query_string" config without using a "search_dn" and a "search_password" is not supported.');
8585
}
86-
$username = $ldap->escape($user->getUserIdentifier(), '', LdapInterface::ESCAPE_FILTER);
87-
$query = str_replace('{username}', $username, $ldapBadge->getQueryString());
86+
$identifier = $ldap->escape($user->getUserIdentifier(), '', LdapInterface::ESCAPE_FILTER);
87+
$query = str_replace('{user_identifier}', $identifier, $ldapBadge->getQueryString());
8888
$result = $ldap->query($ldapBadge->getDnString(), $query)->execute();
8989
if (1 !== $result->count()) {
90-
throw new BadCredentialsException('The presented username is invalid.');
90+
throw new BadCredentialsException('The presented user identifier is invalid.');
9191
}
9292

9393
$dn = $result[0]->getDn();
9494
} else {
95-
$username = $ldap->escape($user->getUserIdentifier(), '', LdapInterface::ESCAPE_DN);
96-
$dn = str_replace('{username}', $username, $ldapBadge->getDnString());
95+
$identifier = $ldap->escape($user->getUserIdentifier(), '', LdapInterface::ESCAPE_DN);
96+
$dn = str_replace('{user_identifier}', $identifier, $ldapBadge->getDnString());
9797
}
9898

9999
$ldap->bind($dn, $presentedPassword);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class LdapAuthenticator implements AuthenticationEntryPointInterface, Interactiv
4242
private string $searchPassword;
4343
private string $queryString;
4444

45-
public function __construct(AuthenticatorInterface $authenticator, string $ldapServiceId, string $dnString = '{username}', string $searchDn = '', string $searchPassword = '', string $queryString = '')
45+
public function __construct(AuthenticatorInterface $authenticator, string $ldapServiceId, string $dnString = '{user_identifier}', string $searchDn = '', string $searchPassword = '', string $queryString = '')
4646
{
4747
$this->authenticator = $authenticator;
4848
$this->ldapServiceId = $ldapServiceId;

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,20 @@ class LdapBadge implements BadgeInterface
3131
private string $searchPassword;
3232
private ?string $queryString;
3333

34-
public function __construct(string $ldapServiceId, string $dnString = '{username}', string $searchDn = '', string $searchPassword = '', string $queryString = null)
34+
public function __construct(string $ldapServiceId, string $dnString = '{user_identifier}', string $searchDn = '', string $searchPassword = '', string $queryString = null)
3535
{
3636
$this->ldapServiceId = $ldapServiceId;
37+
$dnString = str_replace('{username}', '{user_identifier}', $dnString, $replaceCount);
38+
if ($replaceCount > 0) {
39+
trigger_deprecation('symfony/ldap', '6.2', 'Using "{username}" parameter in LDAP configuration is deprecated, consider using "{user_identifier}" instead.');
40+
}
3741
$this->dnString = $dnString;
3842
$this->searchDn = $searchDn;
3943
$this->searchPassword = $searchPassword;
44+
$queryString = str_replace('{username}', '{user_identifier}', $queryString ?? '', $replaceCount);
45+
if ($replaceCount > 0) {
46+
trigger_deprecation('symfony/ldap', '6.2', 'Using "{username}" parameter in LDAP configuration is deprecated, consider using "{user_identifier}" instead.');
47+
}
4048
$this->queryString = $queryString;
4149
}
4250

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,19 @@
2424
class LdapUser implements UserInterface, PasswordAuthenticatedUserInterface, EquatableInterface
2525
{
2626
private Entry $entry;
27-
private string $username;
27+
private string $identifier;
2828
private ?string $password;
2929
private array $roles;
3030
private array $extraFields;
3131

32-
public function __construct(Entry $entry, string $username, #[\SensitiveParameter] ?string $password, array $roles = [], array $extraFields = [])
32+
public function __construct(Entry $entry, string $identifier, #[\SensitiveParameter] ?string $password, array $roles = [], array $extraFields = [])
3333
{
34-
if (!$username) {
34+
if (!$identifier) {
3535
throw new \InvalidArgumentException('The username cannot be empty.');
3636
}
3737

3838
$this->entry = $entry;
39-
$this->username = $username;
39+
$this->identifier = $identifier;
4040
$this->password = $password;
4141
$this->roles = $roles;
4242
$this->extraFields = $extraFields;
@@ -81,7 +81,7 @@ public function getUsername(): string
8181

8282
public function getUserIdentifier(): string
8383
{
84-
return $this->username;
84+
return $this->identifier;
8585
}
8686

8787
/**

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,11 @@ public function loadUserByIdentifier(string $identifier): UserInterface
8181
}
8282

8383
$identifier = $this->ldap->escape($identifier, '', LdapInterface::ESCAPE_FILTER);
84-
$query = str_replace(['{username}', '{user_identifier}'], $identifier, $this->defaultSearch);
84+
$query = str_replace('{username}', '{user_identifier}', $this->defaultSearch, $replaceCount);
85+
if ($replaceCount > 0) {
86+
trigger_deprecation('symfony/ldap', '6.2', 'Using "{username}" parameter in LDAP configuration is deprecated, consider using "{user_identifier}" instead.');
87+
}
88+
$query = str_replace('{user_identifier}', $identifier, $query);
8589
$search = $this->ldap->query($this->baseDn, $query, ['filter' => 0 == \count($this->extraFields) ? '*' : $this->extraFields]);
8690

8791
$entries = $search->execute();

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

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,43 @@ public function testBindFailureShouldThrowAnException()
127127
$listener->onCheckPassport($this->createEvent());
128128
}
129129

130+
/**
131+
* @group legacy
132+
*
133+
* @dataProvider queryForDnProvider
134+
*/
135+
public function testLegacyQueryForDn(string $dnString, string $queryString)
136+
{
137+
$collection = new class([new Entry('')]) extends \ArrayObject implements CollectionInterface {
138+
public function toArray(): array
139+
{
140+
return $this->getArrayCopy();
141+
}
142+
};
143+
144+
$query = $this->createMock(QueryInterface::class);
145+
$query->expects($this->once())->method('execute')->willReturn($collection);
146+
147+
$this->ldap
148+
->method('bind')
149+
->withConsecutive(
150+
['elsa', 'test1234A$']
151+
);
152+
$this->ldap->expects($this->any())->method('escape')->with('Wouter', '', LdapInterface::ESCAPE_FILTER)->willReturn('wouter');
153+
$this->ldap->expects($this->once())->method('query')->with('{user_identifier}', 'wouter_test')->willReturn($query);
154+
155+
$listener = $this->createListener();
156+
$li 10000 stener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', $dnString, 'elsa', 'test1234A$', $queryString)));
157+
}
158+
159+
public function queryForDnProvider(): iterable
160+
{
161+
yield ['{username}', '{username}_test'];
162+
yield ['{user_identifier}', '{username}_test'];
163+
yield ['{username}', '{user_identifier}_test'];
164+
yield ['{user_identifier}', '{user_identifier}_test'];
165+
}
166+
130167
public function testQueryForDn()
131168
{
132169
$collection = new class([new Entry('')]) extends \ArrayObject implements CollectionInterface {
@@ -145,16 +182,16 @@ public function toArray(): array
145182
['elsa', 'test1234A$']
146183
);
147184
$this->ldap->expects($this->any())->method('escape')->with('Wouter', '', LdapInterface::ESCAPE_FILTER)->willReturn('wouter');
148-
$this->ldap->expects($this->once())->method('query')->with('{username}', 'wouter_test')->willReturn($query);
185+
$this->ldap->expects($this->once())->method('query')->with('{user_identifier}', 'wouter_test')->willReturn($query);
149186

150187
$listener = $this->createListener();
151-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test')));
188+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test')));
152189
}
153190

154191
public function testEmptyQueryResultShouldThrowAnException()
155192
{
156193
$this->expectException(BadCredentialsException::class);
157-
$this->expectExceptionMessage('The presented username is invalid.');
194+
$this->expectExceptionMessage('The presented user identifier is invalid.');
158195

159196
$collection = $this->createMock(CollectionInterface::class);
160197

@@ -170,7 +207,7 @@ public function testEmptyQueryResultShouldThrowAnException()
170207
$this->ldap->expects($this->once())->method('query')->willReturn($query);
171208

172209
$listener = $this->createListener();
173-
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{username}', 'elsa', 'test1234A$', '{username}_test')));
210+
$listener->onCheckPassport($this->createEvent('s3cr3t', new LdapBadge('app.ldap', '{user_identifier}', 'elsa', 'test1234A$', '{user_identifier}_test')));
174211
}
175212

176213
private function createEvent($password = 's3cr3t', $ldapBadge = null)

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
*/
2828
class LdapUserProviderTest extends TestCase
2929
{
30-
public function testLoadUserByUsernameFailsIfCantConnectToLdap()
30+
public function testLoadUserByIdentifierFailsIfCantConnectToLdap()
3131
{
3232
$this->expectException(ConnectionException::class);
3333

@@ -42,7 +42,7 @@ public function testLoadUserByUsernameFailsIfCantConnectToLdap()
4242
$provider->loadUserByIdentifier('foo');
4343
}
4444

45-
public function testLoadUserByUsernameFailsIfNoLdapEntries()
45+
public function testLoadUserByIdentifierFailsIfNoLdapEntries()
4646
{
4747
$this->expectException(UserNotFoundException::class);
4848

@@ -74,7 +74,7 @@ public function testLoadUserByUsernameFailsIfNoLdapEntries()
7474
$provider->loadUserByIdentifier('foo');
7575
}
7676

77-
public function testLoadUserByUsernameFailsIfMoreThanOneLdapEntry()
77+
public function testLoadUserByIdentifierFailsIfMoreThanOneLdapEntry()
7878
{
7979
$this->expectException(UserNotFoundException::class);
8080

@@ -106,7 +106,7 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapEntry()
106106
$provider->loadUserByIdentifier('foo');
107107
}
108108

109-
public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
109+
public function testLoadUserByIdentifierFailsIfMoreThanOneLdapPasswordsInEntry()
110110
{
111111
$this->expectException(InvalidArgumentException::class);
112112

@@ -143,11 +143,11 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
143143
->willReturn($query)
144144
;
145145

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

150-
public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()
150+
public function testLoadUserByIdentifierShouldNotFailIfEntryHasNoUidKeyAttribute()
151151
{
152152
$result = $this->createMock(CollectionInterface::class);
153153
$query = $this->createMock(QueryInterface::class);
@@ -179,11 +179,11 @@ public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()
179179
->willReturn($query)
180180
;
181181

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

186-
public function testLoadUserByUsernameFailsIfEntryHasNoPasswordAttribute()
186+
public function testLoadUserByIdentifierFailsIfEntryHasNoPasswordAttribute()
187187
{
188188
$this->expectException(InvalidArgumentException::class);
189189

@@ -217,11 +217,11 @@ public function testLoadUserByUsernameFailsIfEntryHasNoPasswordAttribute()
217217
->willReturn($query)
218218
;
219219

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

224-
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
224+
public function testLoadUserByIdentifierIsSuccessfulWithoutPasswordAttribute()
225225
{
226226
$result = $this->createMock(CollectionInterface::class);
227227
$query = $this->createMock(QueryInterface::class);
@@ -257,7 +257,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
257257
$this->assertInstanceOf(LdapUser::class, $provider->loadUserByIdentifier('foo'));
258258
}
259259

260-
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWrongCase()
260+
public function testLoadUserByIdentifierIsSuccessfulWithoutPasswordAttributeAndWrongCase()
261261
{
262262
$result = $this->createMock(CollectionInterface::class);
263263
$query = $this->createMock(QueryInterface::class);
@@ -293,7 +293,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWro
293293
$this->assertSame('foo', $provider->loadUserByIdentifier('Foo')->getUserIdentifier());
294294
}
295295

296-
public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
296+
public function testLoadUserByIdentifierIsSuccessfulWithPasswordAttribute()
297297
{
298298
$result = $this->createMock(CollectionInterface::class);
299299
$query = $this->createMock(QueryInterface::class);
@@ -329,14 +329,14 @@ public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
329329
->willReturn($query)
330330
;
331331

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

336336
public function testRefreshUserShouldReturnUserWithSameProperties()
337337
{
338338
$ldap = $this->createMock(LdapInterface::class);
339-
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={username})', 'userpassword', ['email']);
339+
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, [], 'sAMAccountName', '({uid_key}={user_identifier})', 'userpassword', ['email']);
340340

341341
$user = new LdapUser(new Entry('foo'), 'foo', 'bar', ['ROLE_DUMMY'], ['email' => 'foo@symfony.com']);
342342

‎src/Symfony/Component/Ldap/composer.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
"require": {
1919
"php": ">=8.1",
2020
"ext-ldap": "*",
21+
"symfony/deprecation-contracts": "^2.1|^3",
2122
"symfony/options-resolver": "^5.4|^6.0"
2223
},
2324
"require-dev": {

0 commit comments

Comments
 (0)
0