bug #21579 [Security] LdapUserProvider should not throw an exception if the UID key does not exist in an LDAP entry (csarrazi)

fabpot · fabpot · commit f376080b4b90 · 2017-02-11T09:51:37.000+01:00
This PR was submitted for the 3.1 branch but it was merged into the 3.2 branch instead (closes #21579). Discussion ---------- [Security] LdapUserProvider should not throw an exception if the UID key does not exist in an LDAP entry | Q | A | ------------- | --- | Branch? | 3.1+ | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21577 | License | MIT | Doc PR | This ticket should fix #21577, which was introduced by commit 6641b79 LdapUserProvider should not throw an exception if the uid key does not exist in the entry. Commits ------- ee4d9a7 [Security] LdapUserProvider should not throw an exception if the UID key does not exist in an LDAP entry
diff --git a/src/Symfony/Component/Security/Core/Tests/User/LdapUserProviderTest.php b/src/Symfony/Component/Security/Core/Tests/User/LdapUserProviderTest.php
@@ -151,10 +151,7 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()
         );
     }
 
-    /**
-     * @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException
-     */
-    public function testLoadUserByUsernameFailsIfEntryHasNoUidKeyAttribute()
+    public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()
     {
         $result = $this->getMockBuilder(CollectionInterface::class)->getMock();
         $query = $this->getMockBuilder(QueryInterface::class)->getMock();
diff --git a/src/Symfony/Component/Security/Core/User/LdapUserProvider.php b/src/Symfony/Component/Security/Core/User/LdapUserProvider.php
@@ -48,7 +48,7 @@ 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';
+            $uidKey = 'sAMAccountName';
         }
 
         $this->ldap = $ldap;
@@ -87,7 +87,13 @@ public function loadUserByUsername($username)
         }
 
         $entry = $entries[0];
-        $username = $this->getAttributeValue($entry, $this->uidKey);
+
+        try {
+            if (null !== $this->uidKey) {
+                $username = $this->getAttributeValue($entry, $this->uidKey);
+            }
+        } catch (InvalidArgumentException $e) {
+        }
 
         return $this->loadUser($username, $entry);
     }
@@ -123,6 +129,7 @@ public function supportsClass($class)
     protected function loadUser($username, Entry $entry)
     {
         $password = null;
+
         if (null !== $this->passwordAttribute) {
             $password = $this->getAttributeValue($entry, $this->passwordAttribute);
         }

Original file line number	Diff line number	Diff line change
`@@ -151,10 +151,7 @@ public function testLoadUserByUsernameFailsIfMoreThanOneLdapPasswordsInEntry()`
`151`	`151`	`);`
`152`	`152`	`}`
`153`	`153`
`154`		`- /**`
`155`		`- * @expectedException \Symfony\Component\Security\Core\Exception\InvalidArgumentException`
`156`		`- */`
`157`		`- public function testLoadUserByUsernameFailsIfEntryHasNoUidKeyAttribute()`
	`154`	`+ public function testLoadUserByUsernameShouldNotFailIfEntryHasNoUidKeyAttribute()`
`158`	`155`	`{`
`159`	`156`	`$result = $this->getMockBuilder(CollectionInterface::class)->getMock();`
`160`	`157`	`$query = $this->getMockBuilder(QueryInterface::class)->getMock();`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,7 @@ class LdapUserProvider implements UserProviderInterface`
`48`	`48`	`public function __construct(LdapInterface $ldap, $baseDn, $searchDn = null, $searchPassword = null, array $defaultRoles = array(), $uidKey = 'sAMAccountName', $filter = '({uid_key}={username})', $passwordAttribute = null)`
`49`	`49`	`{`
`50`	`50`	`if (null === $uidKey) {`
`51`		`- $uidKey = 'uid';`
	`51`	`+ $uidKey = 'sAMAccountName';`
`52`	`52`	`}`
`53`	`53`
`54`	`54`	`$this->ldap = $ldap;`
`@@ -87,7 +87,13 @@ public function loadUserByUsername($username)`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`$entry = $entries[0];`
`90`		`- $username = $this->getAttributeValue($entry, $this->uidKey);`
	`90`	`+`
	< 998F code>91	`+ try {`
	`92`	`+ if (null !== $this->uidKey) {`
	`93`	`+ $username = $this->getAttributeValue($entry, $this->uidKey);`
	`94`	`+ }`
	`95`	`+ } catch (InvalidArgumentException $e) {`
	`96`	`+ }`
`91`	`97`
`92`	`98`	`return $this->loadUser($username, $entry);`
`93`	`99`	`}`
`@@ -123,6 +129,7 @@ public function supportsClass($class)`
`123`	`129`	`protected function loadUser($username, Entry $entry)`
`124`	`130`	`{`
`125`	`131`	`$password = null;`
	`132`	`+`
`126`	`133`	`if (null !== $this->passwordAttribute) {`
`127`	`134`	`$password = $this->getAttributeValue($entry, $this->passwordAttribute);`
`128`	`135`	`}`