Skip to content

Commit 0dd6a90

Browse files
fabpotfabpot
committed
bug symfony#21291 [Ldap] Ldap username case fix (quentinus95)
This PR was submitted for the master branch but it was merged into the 3.1 branch instead (closes symfony#21291). Discussion ---------- [Ldap] Ldap username case fix | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony#20486 | License | MIT | Doc PR | Commits ------- c91689b [Ldap] Using Ldap stored username instead of form submitted one 6641b79 [Ldap] load users with the good username case
2 parents d228d34 + c91689b commit 0dd6a90

File tree

2 files changed

+103
-13
lines changed

2 files changed

+103
-13
lines changed

‎src/Symfony/Component/Security/Core/Tests/User/LdapUserProviderTest.php

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,48 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
151151
);
152152
}
153153

154+
/**
155+
* @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException
156+
*/
157+
public function testLoadUserByUsernameFailsIfEntryHasNoUidKeyAttribute()
158+
{
159+
$result = $this->getMock(CollectionInterface::class);
160+
$query = $this->getMock(QueryInterface::class);
161+
$query
162+
->expects($this->once())
163+
->method('execute')
164+
->will($this->returnValue($result))
165+
;
166+
$ldap = $this->getMock(LdapInterface::class);
167+
$result
168+
->expects($this->once())
169+
->method('offsetGet')
170+
->with(0)
171+
->will($this->returnValue(new Entry('foo', array())))
172+
;
173+
$result
174+
->expects($this->once())
175+
->method('count')
176+
->will($this->returnValue(1))
177+
;
178+
$ldap
179+
->expects($this->once())
180+
->method('escape')
181+
->will($this->returnValue('foo'))
182+
;
183+
$ldap
184+
->expects($this->once())
185+
->method('query')
186+
->will($this->returnValue($query))
187+
;
188+
189+
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, array(), 'sAMAccountName', '({uid_key}={username})');
190+
$this->assertInstanceOf(
191+
'Symfony\Component\Security\Core\User\User',
192+
$provider->loadUserByUsername('foo')
193+
);
194+
}
195+
154196
/**
155197
* @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException
156198
*/
@@ -238,7 +280,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
238280
);
239281
}
240282

241-
public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
283+
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWrongCase()
242284
{
243285
$result = $this->getMockBuilder(CollectionInterface::class)->getMock();
244286
$query = $this->getMockBuilder(QueryInterface::class)->getMock();
@@ -248,6 +290,45 @@ public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
248290
->will($this->returnValue($result))
249291
;
250292
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
293+
$result
294+
->expects($this->once())
295+
->method('offsetGet')
296+
->with(0)
297+
->will($this->returnValue(new Entry('foo', array(
298+
'sAMAccountName' => array('foo'),
299+
)
300+
)))
301+
;
302+
$result
303+
->expects($this->once())
304+
->method('count')
305+
->will($this->returnValue(1))
306+
;
307+
$ldap
308+
->expects($this->once())
309+
->method('escape')
310+
->will($this->returnValue('Foo'))
311+
;
312+
$ldap
313+
->expects($this->once())
314+
->method('query')
315+
->will($this->returnValue($query))
316+
;
317+
318+
$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
319+
$this->assertSame('foo', $provider->loadUserByUsername('Foo')->getUsername());
320+
}
321+
322+
public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
323+
{
324+
$result = $this->getMock(CollectionInterface::class);
325+
$query = $this->getMock(QueryInterface::class);
326+
$query
327+
->expects($this->once())
328+
->method('execute')
329+
->will($this->returnValue($result))
330+
;
331+
$ldap = $this->getMock(LdapInterface::class);
251332
$result
252333
->expects($this->once())
253334
->method('offsetGet')

‎src/Symfony/Component/Security/Core/User/LdapUserProvider.php

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class LdapUserProvider implements UserProviderInterface
3131
private $searchDn;
3232
private $searchPassword;
3333
private $defaultRoles;
34+
private $uidKey;
3435
private $defaultSearch;
3536
private $passwordAttribute;
3637

@@ -46,11 +47,16 @@ class LdapUserProvider implements UserProviderInterface
4647
*/
4748
public function __construct(LdapInterface $ldap, $baseDn, $searchDn = null, $searchPassword = null, array $defaultRoles = array(), $uidKey = 'sAMAccountName', $filter = '({uid_key}={username})', $passwordAttribute = null)
4849
{
50+
if (null === $uidKey) {
51+
$uidKey = 'uid';
52+
}
53+
4954
$this->ldap = $ldap;
5055
$this->baseDn = $baseDn;
5156
$this->searchDn = $searchDn;
5257
$this->searchPassword = $searchPassword;
5358
$this->defaultRoles = $defaultRoles;
59+
$this->uidKey = $uidKey;
5460
$this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter);
5561
$this->passwordAttribute = $passwordAttribute;
5662
}
@@ -80,7 +86,10 @@ public function loadUserByUsername($username)
8086
throw new UsernameNotFoundException('More than one user found');
8187
}
8288

83-
return $this->loadUser($username, $entries[0]);
89+
$entry = $entries[0];
90+
$username = $this->getAttributeValue($entry, $this->uidKey);
91+
92+
return $this->loadUser($username, $entry);
8493
}
8594

8695
/**
@@ -113,30 +122,30 @@ public function supportsClass($class)
113122
*/
114123
protected function loadUser($username, Entry $entry)
115124
{
116-
$password = $this->getPassword($entry);
125+
$password = null;
126+
if (null !== $this->passwordAttribute) {
127+
$password = $this->getAttributeValue($entry, $this->passwordAttribute);
128+
}
117129

118130
return new User($username, $password, $this->defaultRoles);
119131
}
120132

121133
/**
122-
* Fetches the password from an LDAP entry.
134+
* Fetches a required unique attribute value from an LDAP entry.
123135
*
124136
* @param null|Entry $entry
137+
* @param string $attribute
125138
*/
126-
private function getPassword(Entry $entry)
139+
private function getAttributeValue(Entry $entry, $attribute)
127140
{
128-
if (null === $this->passwordAttribute) {
129-
return;
130-
}
131-
132-
if (!$entry->hasAttribute($this->passwordAttribute)) {
133-
throw new InvalidArgumentException(sprintf('Missing attribute "%s" for user "%s".', $this->passwordAttribute, $entry->getDn()));
141+
if (!$entry->hasAttribute($attribute)) {
142+
throw new InvalidArgumentException(sprintf('Missing attribute "%s" for user "%s".', $attribute, $entry->getDn()));
134143
}
135144

136-
$values = $entry->getAttribute($this->passwordAttribute);
145+
$values = $entry->getAttribute($attribute);
137146

138147
if (1 !== count($values)) {
139-
throw new InvalidArgumentException(sprintf('Attribute "%s" has multiple values.', $this->passwordAttribute));
148+
throw new InvalidArgumentException(sprintf('Attribute "%s" has multiple values.', $attribute));
140149
}
141150

142151
return $values[0];

0 commit comments

Comments
 (0)