Skip to content

Commit

Permalink
bug #21291 [Ldap] Ldap username case fix (quentinus95)
Browse files Browse the repository at this point in the history
This PR was submitted for the master branch but it was merged into the 3.1 branch instead (closes #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 | #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
  • Loading branch information
fabpot committed Jan 17, 2017
2 parents d228d34 + c91689b commit 0dd6a90
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 13 deletions.
Expand Up @@ -151,6 +151,48 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
);
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException
*/
public function testLoadUserByUsernameFailsIfEntryHasNoUidKeyAttribute()
{
$result = $this->getMock(CollectionInterface::class);
$query = $this->getMock(QueryInterface::class);
$query
->expects($this->once())
->method('execute')
->will($this->returnValue($result))
;
$ldap = $this->getMock(LdapInterface::class);
$result
->expects($this->once())
->method('offsetGet')
->with(0)
->will($this->returnValue(new Entry('foo', array())))
;
$result
->expects($this->once())
->method('count')
->will($this->returnValue(1))
;
$ldap
->expects($this->once())
->method('escape')
->will($this->returnValue('foo'))
;
$ldap
->expects($this->once())
->method('query')
->will($this->returnValue($query))
;

$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com', null, null, array(), 'sAMAccountName', '({uid_key}={username})');
$this->assertInstanceOf(
'Symfony\Component\Security\Core\User\User',
$provider->loadUserByUsername('foo')
);
}

/**
* @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException
*/
Expand Down Expand Up @@ -238,7 +280,7 @@ public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttribute()
);
}

public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
public function testLoadUserByUsernameIsSuccessfulWithoutPasswordAttributeAndWrongCase()
{
$result = $this->getMockBuilder(CollectionInterface::class)->getMock();
$query = $this->getMockBuilder(QueryInterface::class)->getMock();
Expand All @@ -248,6 +290,45 @@ public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
->will($this->returnValue($result))
;
$ldap = $this->getMockBuilder(LdapInterface::class)->getMock();
$result
->expects($this->once())
->method('offsetGet')
->with(0)
->will($this->returnValue(new Entry('foo', array(
'sAMAccountName' => array('foo'),
)
)))
;
$result
->expects($this->once())
->method('count')
->will($this->returnValue(1))
;
$ldap
->expects($this->once())
->method('escape')
->will($this->returnValue('Foo'))
;
$ldap
->expects($this->once())
->method('query')
->will($this->returnValue($query))
;

$provider = new LdapUserProvider($ldap, 'ou=MyBusiness,dc=symfony,dc=com');
$this->assertSame('foo', $provider->loadUserByUsername('Foo')->getUsername());
}

public function testLoadUserByUsernameIsSuccessfulWithPasswordAttribute()
{
$result = $this->getMock(CollectionInterface::class);
$query = $this->getMock(QueryInterface::class);
$query
->expects($this->once())
->method('execute')
->will($this->returnValue($result))
;
$ldap = $this->getMock(LdapInterface::class);
$result
->expects($this->once())
->method('offsetGet')
Expand Down
33 changes: 21 additions & 12 deletions src/Symfony/Component/Security/Core/User/LdapUserProvider.php
Expand Up @@ -31,6 +31,7 @@ class LdapUserProvider implements UserProviderInterface
private $searchDn;
private $searchPassword;
private $defaultRoles;
private $uidKey;
private $defaultSearch;
private $passwordAttribute;

Expand All @@ -46,11 +47,16 @@ class LdapUserProvider implements UserProviderInterface
*/
public function __construct(LdapInterface $ldap, $baseDn, $searchDn = null, $searchPassword = null, array $defaultRoles = array(), $uidKey = 'sAMAccountName', $filter = '({uid_key}={username})', $passwordAttribute = null)
{
if (null === $uidKey) {
$uidKey = 'uid';
}

$this->ldap = $ldap;
$this->baseDn = $baseDn;
$this->searchDn = $searchDn;
$this->searchPassword = $searchPassword;
$this->defaultRoles = $defaultRoles;
$this->uidKey = $uidKey;
$this->defaultSearch = str_replace('{uid_key}', $uidKey, $filter);
$this->passwordAttribute = $passwordAttribute;
}
Expand Down Expand Up @@ -80,7 +86,10 @@ public function loadUserByUsername($username)
throw new UsernameNotFoundException('More than one user found');
}

return $this->loadUser($username, $entries[0]);
$entry = $entries[0];
$username = $this->getAttributeValue($entry, $this->uidKey);

return $this->loadUser($username, $entry);
}

/**
Expand Down Expand Up @@ -113,30 +122,30 @@ public function supportsClass($class)
*/
protected function loadUser($username, Entry $entry)
{
$password = $this->getPassword($entry);
$password = null;
if (null !== $this->passwordAttribute) {
$password = $this->getAttributeValue($entry, $this->passwordAttribute);
}

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

/**
* Fetches the password from an LDAP entry.
* Fetches a required unique attribute value from an LDAP entry.
*
* @param null|Entry $entry
* @param string $attribute
*/
private function getPassword(Entry $entry)
private function getAttributeValue(Entry $entry, $attribute)
{
if (null === $this->passwordAttribute) {
return;
}

if (!$entry->hasAttribute($this->passwordAttribute)) {
throw new InvalidArgumentException(sprintf('Missing attribute "%s" for user "%s".', $this->passwordAttribute, $entry->getDn()));
if (!$entry->hasAttribute($attribute)) {
throw new InvalidArgumentException(sprintf('Missing attribute "%s" for user "%s".', $attribute, $entry->getDn()));
}

$values = $entry->getAttribute($this->passwordAttribute);
$values = $entry->getAttribute($attribute);

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

return $values[0];
Expand Down

0 comments on commit 0dd6a90

Please sign in to comment.