From fc6793f2ae778448d1c797625149c17ed77bfcc0 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 11 Aug 2014 16:40:41 +0200 Subject: better check whether string resembles a DN, fixes #9887 --- apps/user_ldap/lib/access.php | 12 ++++++ apps/user_ldap/lib/user/iusertools.php | 2 + apps/user_ldap/lib/user/manager.php | 3 +- apps/user_ldap/tests/access.php | 76 ++++++++++++++++++++++++++++++++++ apps/user_ldap/tests/user/manager.php | 47 +++++++++++++++++++++ 5 files changed, 138 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 23ba4253ed3..570f445650d 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -140,6 +140,18 @@ class Access extends LDAPUtility implements user\IUserTools { return in_array($attr, $resemblingAttributes); } + /** + * checks whether the given string is probably a DN + * @param string $string + * @return boolean + */ + public function stringResemblesDN($string) { + $r = $this->ldap->explodeDN($string, 0); + // if exploding a DN succeeds and does not end up in + // an empty array except for $r[count] being 0. + return (is_array($r) && count($r) > 1); + } + /** * sanitizes a DN received from the LDAP server * @param array $dn the DN in question diff --git a/apps/user_ldap/lib/user/iusertools.php b/apps/user_ldap/lib/user/iusertools.php index e409f3afed3..bbc678153de 100644 --- a/apps/user_ldap/lib/user/iusertools.php +++ b/apps/user_ldap/lib/user/iusertools.php @@ -33,6 +33,8 @@ interface IUserTools { public function readAttribute($dn, $attr, $filter = 'objectClass=*'); + public function stringResemblesDN($string); + public function dn2username($dn, $ldapname = null); public function username2dn($name); diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index 0f17900b5f3..0ed3d09c48f 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -143,8 +143,7 @@ class Manager { return $this->users['byUid'][$id]; } - if(strpos(mb_strtolower($id, 'UTF-8'), 'dc=') === false - && strpos(mb_strtolower($id, 'UTF-8'), 'uid=') === false ) { + if(!$this->access->stringResemblesDN($id) ) { //most likely a uid $dn = $this->access->username2dn($id); if($dn !== false) { diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php index e77aad769d4..72b9780c018 100644 --- a/apps/user_ldap/tests/access.php +++ b/apps/user_ldap/tests/access.php @@ -156,4 +156,80 @@ class Test_Access extends \PHPUnit_Framework_TestCase { $this->assertSame($expected, $access->getDomainDNFromDN($inputDN)); } + + public function stringResemblesDNYes() { + list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + $access = new Access($con, $lw, $um); + + $input = 'foo=bar,bar=foo,dc=foobar'; + $interResult = array( + 'count' => 3, + 0 => 'foo=bar', + 1 => 'bar=foo', + 2 => 'dc=foobar' + ); + + $lw->expects($this->once()) + ->method('explodeDN') + ->will($this->returnValue($interResult)); + + $this->assertTrue($access->stringResemblesDN($input)); + } + + public function stringResemblesDNYesLDAPmod() { + list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + $lw = new \OCA\user_ldap\lib\LDAP(); + $access = new Access($con, $lw, $um); + + if(!function_exists('ldap_explode_dn')) { + $this->markTestSkipped('LDAP Module not available'); + } + + $input = 'foo=bar,bar=foo,dc=foobar'; + $interResult = array( + 'count' => 3, + 0 => 'foo=bar', + 1 => 'bar=foo', + 2 => 'dc=foobar' + ); + + $lw->expects($this->once()) + ->method('explodeDN') + ->will($this->returnValue($interResult)); + + $this->assertTrue($access->stringResemblesDN($input)); + } + + public function stringResemblesDNNo() { + list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + $access = new Access($con, $lw, $um); + + $input = 'foobarbarfoodcfoobar'; + $interResult = false; + + $lw->expects($this->once()) + ->method('explodeDN') + ->will($this->returnValue($interResult)); + + $this->assertFalse($access->stringResemblesDN($input)); + } + + public function stringResemblesDNNoLDAPMod() { + list($lw, $con, $um) = $this->getConnecterAndLdapMock(); + $lw = new \OCA\user_ldap\lib\LDAP(); + $access = new Access($con, $lw, $um); + + if(!function_exists('ldap_explode_dn')) { + $this->markTestSkipped('LDAP Module not available'); + } + + $input = 'foobarbarfoodcfoobar'; + $interResult = false; + + $lw->expects($this->once()) + ->method('explodeDN') + ->will($this->returnValue($interResult)); + + $this->assertFalse($access->stringResemblesDN($input)); + } } diff --git a/apps/user_ldap/tests/user/manager.php b/apps/user_ldap/tests/user/manager.php index 7599980ff9a..7d687867213 100644 --- a/apps/user_ldap/tests/user/manager.php +++ b/apps/user_ldap/tests/user/manager.php @@ -44,6 +44,11 @@ class Test_User_Manager extends \PHPUnit_Framework_TestCase { $inputDN = 'cn=foo,dc=foobar,dc=bar'; $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; + $access->expects($this->once()) + ->method('stringResemblesDN') + ->with($this->equalTo($inputDN)) + ->will($this->returnValue(true)); + $access->expects($this->once()) ->method('dn2username') ->with($this->equalTo($inputDN)) @@ -66,6 +71,38 @@ class Test_User_Manager extends \PHPUnit_Framework_TestCase { $inputDN = 'uid=foo,o=foobar,c=bar'; $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; + $access->expects($this->once()) + ->method('stringResemblesDN') + ->with($this->equalTo($inputDN)) + ->will($this->returnValue(true)); + + $access->expects($this->once()) + ->method('dn2username') + ->with($this->equalTo($inputDN)) + ->will($this->returnValue($uid)); + + $access->expects($this->never()) + ->method('username2dn'); + + $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager->setLdapAccess($access); + $user = $manager->get($inputDN); + + $this->assertInstanceOf('\OCA\user_ldap\lib\user\User', $user); + } + + public function testGetByExoticDN() { + list($access, $config, $filesys, $image, $log, $avaMgr) = + $this->getTestInstances(); + + $inputDN = 'ab=cde,f=ghei,mno=pq'; + $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; + + $access->expects($this->once()) + ->method('stringResemblesDN') + ->with($this->equalTo($inputDN)) + ->will($this->returnValue(true)); + $access->expects($this->once()) ->method('dn2username') ->with($this->equalTo($inputDN)) @@ -87,6 +124,11 @@ class Test_User_Manager extends \PHPUnit_Framework_TestCase { $inputDN = 'cn=gone,dc=foobar,dc=bar'; + $access->expects($this->once()) + ->method('stringResemblesDN') + ->with($this->equalTo($inputDN)) + ->will($this->returnValue(true)); + $access->expects($this->once()) ->method('dn2username') ->with($this->equalTo($inputDN)) @@ -119,6 +161,11 @@ class Test_User_Manager extends \PHPUnit_Framework_TestCase { ->with($this->equalTo($uid)) ->will($this->returnValue($dn)); + $access->expects($this->once()) + ->method('stringResemblesDN') + ->with($this->equalTo($uid)) + ->will($this->returnValue(false)); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image); $manager->setLdapAccess($access); $user = $manager->get($uid); -- cgit v1.2.3 From eb7d70fc393a0a3b4441872de4a7ceb8bf0c7cc2 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 12 Aug 2014 16:13:17 +0200 Subject: adjust login test to code changes --- apps/user_ldap/tests/user_ldap.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 8787e023655..e51f6cb5bb9 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -130,6 +130,11 @@ class Test_User_Ldap_Direct extends \PHPUnit_Framework_TestCase { ->with($this->equalTo('dnOfRoland,dc=test')) ->will($this->returnValue('gunslinger')); + $access->expects($this->any()) + ->method('stringResemblesDN') + ->with($this->equalTo('dnOfRoland,dc=test')) + ->will($this->returnValue(true)); + $access->expects($this->any()) ->method('areCredentialsValid') ->will($this->returnCallback(function($dn, $pwd) { -- cgit v1.2.3 From 97fd39e983645bf743f8abd5c05bfe619f859690 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Mon, 18 Aug 2014 16:55:08 +0200 Subject: unify tests --- apps/user_ldap/tests/access.php | 91 ++++++++++++++++------------------------- 1 file changed, 36 insertions(+), 55 deletions(-) diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php index 72b9780c018..f436784675d 100644 --- a/apps/user_ldap/tests/access.php +++ b/apps/user_ldap/tests/access.php @@ -157,64 +157,48 @@ class Test_Access extends \PHPUnit_Framework_TestCase { $this->assertSame($expected, $access->getDomainDNFromDN($inputDN)); } - public function stringResemblesDNYes() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); - $access = new Access($con, $lw, $um); - - $input = 'foo=bar,bar=foo,dc=foobar'; - $interResult = array( - 'count' => 3, - 0 => 'foo=bar', - 1 => 'bar=foo', - 2 => 'dc=foobar' + private function getResemblesDNInputData() { + return $cases = array( + array( + 'input' => 'foo=bar,bar=foo,dc=foobar', + 'interResult' => array( + 'count' => 3, + 0 => 'foo=bar', + 1 => 'bar=foo', + 2 => 'dc=foobar' + ), + 'expectedResult' => true + ), + array( + 'input' => 'foobarbarfoodcfoobar', + 'interResult' => false, + 'expectedResult' => false + ) ); - - $lw->expects($this->once()) - ->method('explodeDN') - ->will($this->returnValue($interResult)); - - $this->assertTrue($access->stringResemblesDN($input)); } - public function stringResemblesDNYesLDAPmod() { + public function testStringResemblesDN() { list($lw, $con, $um) = $this->getConnecterAndLdapMock(); - $lw = new \OCA\user_ldap\lib\LDAP(); $access = new Access($con, $lw, $um); - if(!function_exists('ldap_explode_dn')) { - $this->markTestSkipped('LDAP Module not available'); - } + $cases = $this->getResemblesDNInputData(); - $input = 'foo=bar,bar=foo,dc=foobar'; - $interResult = array( - 'count' => 3, - 0 => 'foo=bar', - 1 => 'bar=foo', - 2 => 'dc=foobar' - ); - - $lw->expects($this->once()) + $lw->expects($this->exactly(2)) ->method('explodeDN') - ->will($this->returnValue($interResult)); - - $this->assertTrue($access->stringResemblesDN($input)); - } - - public function stringResemblesDNNo() { - list($lw, $con, $um) = $this->getConnecterAndLdapMock(); - $access = new Access($con, $lw, $um); - - $input = 'foobarbarfoodcfoobar'; - $interResult = false; - - $lw->expects($this->once()) - ->method('explodeDN') - ->will($this->returnValue($interResult)); - - $this->assertFalse($access->stringResemblesDN($input)); + ->will($this->returnCallback(function ($dn) use ($cases) { + foreach($cases as $case) { + if($dn === $case['input']) { + return $case['interResult']; + } + } + })); + + foreach($cases as $case) { + $this->assertSame($case['expectedResult'], $access->stringResemblesDN($case['input'])); + } } - public function stringResemblesDNNoLDAPMod() { + public function testStringResemblesDNLDAPmod() { list($lw, $con, $um) = $this->getConnecterAndLdapMock(); $lw = new \OCA\user_ldap\lib\LDAP(); $access = new Access($con, $lw, $um); @@ -223,13 +207,10 @@ class Test_Access extends \PHPUnit_Framework_TestCase { $this->markTestSkipped('LDAP Module not available'); } - $input = 'foobarbarfoodcfoobar'; - $interResult = false; + $cases = $this->getResemblesDNInputData(); - $lw->expects($this->once()) - ->method('explodeDN') - ->will($this->returnValue($interResult)); - - $this->assertFalse($access->stringResemblesDN($input)); + foreach($cases as $case) { + $this->assertSame($case['expectedResult'], $access->stringResemblesDN($case['input'])); + } } } -- cgit v1.2.3