diff options
| -rw-r--r-- | apps/user_ldap/composer/composer/autoload_classmap.php | 1 | ||||
| -rw-r--r-- | apps/user_ldap/composer/composer/autoload_static.php | 1 | ||||
| -rw-r--r-- | apps/user_ldap/lib/Access.php | 3 | ||||
| -rw-r--r-- | apps/user_ldap/lib/User/IUserTools.php | 42 | ||||
| -rw-r--r-- | apps/user_ldap/lib/User/Manager.php | 10 | ||||
| -rw-r--r-- | apps/user_ldap/lib/User/User.php | 8 | ||||
| -rw-r--r-- | apps/user_ldap/tests/User/ManagerTest.php | 224 |
7 files changed, 110 insertions, 179 deletions
diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 98a1bbfa1b7..e25b7ee3126 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -56,7 +56,6 @@ return array( 'OCA\\User_LDAP\\Settings\\Section' => $baseDir . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => $baseDir . '/../lib/UserPluginManager.php', 'OCA\\User_LDAP\\User\\DeletedUsersIndex' => $baseDir . '/../lib/User/DeletedUsersIndex.php', - 'OCA\\User_LDAP\\User\\IUserTools' => $baseDir . '/../lib/User/IUserTools.php', 'OCA\\User_LDAP\\User\\Manager' => $baseDir . '/../lib/User/Manager.php', 'OCA\\User_LDAP\\User\\OfflineUser' => $baseDir . '/../lib/User/OfflineUser.php', 'OCA\\User_LDAP\\User\\User' => $baseDir . '/../lib/User/User.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index 83e49daf0f3..23819055be4 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -71,7 +71,6 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Settings\\Section' => __DIR__ . '/..' . '/../lib/Settings/Section.php', 'OCA\\User_LDAP\\UserPluginManager' => __DIR__ . '/..' . '/../lib/UserPluginManager.php', 'OCA\\User_LDAP\\User\\DeletedUsersIndex' => __DIR__ . '/..' . '/../lib/User/DeletedUsersIndex.php', - 'OCA\\User_LDAP\\User\\IUserTools' => __DIR__ . '/..' . '/../lib/User/IUserTools.php', 'OCA\\User_LDAP\\User\\Manager' => __DIR__ . '/..' . '/../lib/User/Manager.php', 'OCA\\User_LDAP\\User\\OfflineUser' => __DIR__ . '/..' . '/../lib/User/OfflineUser.php', 'OCA\\User_LDAP\\User\\User' => __DIR__ . '/..' . '/../lib/User/User.php', diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index d0d51ae8c85..fb2582e8266 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -46,7 +46,6 @@ namespace OCA\User_LDAP; use OC\HintException; use OC\Hooks\PublicEmitter; use OCA\User_LDAP\Exceptions\ConstraintViolationException; -use OCA\User_LDAP\User\IUserTools; use OCA\User_LDAP\User\Manager; use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\Mapping\AbstractMapping; @@ -59,7 +58,7 @@ use OCP\IUserManager; * Class Access * @package OCA\User_LDAP */ -class Access extends LDAPUtility implements IUserTools { +class Access extends LDAPUtility { const UUID_ATTRIBUTES = ['entryuuid', 'nsuniqueid', 'objectguid', 'guid', 'ipauniqueid']; /** @var \OCA\User_LDAP\Connection */ diff --git a/apps/user_ldap/lib/User/IUserTools.php b/apps/user_ldap/lib/User/IUserTools.php deleted file mode 100644 index 4ba9cebb1a6..00000000000 --- a/apps/user_ldap/lib/User/IUserTools.php +++ /dev/null @@ -1,42 +0,0 @@ -<?php -/** - * @copyright Copyright (c) 2016, ownCloud, Inc. - * - * @author Arthur Schiwon <blizzz@arthur-schiwon.de> - * @author Joas Schilling <coding@schilljs.com> - * @author Morris Jobke <hey@morrisjobke.de> - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ - -namespace OCA\User_LDAP\User; - -/** - * IUserTools - * - * defines methods that are required by User class for LDAP interaction - */ -interface IUserTools { - public function getConnection(); - - 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 9f2f3649777..13555f9e31b 100644 --- a/apps/user_ldap/lib/User/Manager.php +++ b/apps/user_ldap/lib/User/Manager.php @@ -45,7 +45,7 @@ use OCP\Notification\IManager as INotificationManager; * cache */ class Manager { - /** @var IUserTools */ + /** @var Access */ protected $access; /** @var IConfig */ @@ -110,11 +110,11 @@ class Manager { } /** - * @brief binds manager to an instance of IUserTools (implemented by - * Access). It needs to be assigned first before the manager can be used. - * @param IUserTools + * Binds manager to an instance of Access. + * It needs to be assigned first before the manager can be used. + * @param Access */ - public function setLdapAccess(IUserTools $access) { + public function setLdapAccess(Access $access) { $this->access = $access; } diff --git a/apps/user_ldap/lib/User/User.php b/apps/user_ldap/lib/User/User.php index f4be19a7ad5..706424d3189 100644 --- a/apps/user_ldap/lib/User/User.php +++ b/apps/user_ldap/lib/User/User.php @@ -30,6 +30,7 @@ namespace OCA\User_LDAP\User; +use OCA\User_LDAP\Access; use OCA\User_LDAP\Connection; use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\LogWrapper; @@ -48,7 +49,7 @@ use OCP\Notification\IManager as INotificationManager; */ class User { /** - * @var IUserTools + * @var Access */ protected $access; /** @@ -110,8 +111,7 @@ class User { * @brief constructor, make sure the subclasses call this one! * @param string $username the internal username * @param string $dn the LDAP DN - * @param IUserTools $access an instance that implements IUserTools for - * LDAP interaction + * @param Access $access * @param IConfig $config * @param FilesystemHelper $fs * @param Image $image any empty instance @@ -120,7 +120,7 @@ class User { * @param IUserManager $userManager * @param INotificationManager $notificationManager */ - public function __construct($username, $dn, IUserTools $access, + public function __construct($username, $dn, Access $access, IConfig $config, FilesystemHelper $fs, Image $image, LogWrapper $log, IAvatarManager $avatarManager, IUserManager $userManager, INotificationManager $notificationManager) { diff --git a/apps/user_ldap/tests/User/ManagerTest.php b/apps/user_ldap/tests/User/ManagerTest.php index 104a70ff700..5c111abdc4e 100644 --- a/apps/user_ldap/tests/User/ManagerTest.php +++ b/apps/user_ldap/tests/User/ManagerTest.php @@ -28,11 +28,13 @@ namespace OCA\User_LDAP\Tests\User; +use OCA\User_LDAP\Access; +use OCA\User_LDAP\Connection; use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\ILDAPWrapper; use OCA\User_LDAP\LogWrapper; -use OCA\User_LDAP\User\IUserTools; use OCA\User_LDAP\User\Manager; +use OCA\User_LDAP\User\User; use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; @@ -48,200 +50,181 @@ use OCP\Notification\IManager as INotificationManager; * @package OCA\User_LDAP\Tests\User */ class ManagerTest extends \Test\TestCase { + /** @var Access|\PHPUnit_Framework_MockObject_MockObject */ + protected $access; - private function getTestInstances() { - $access = $this->createMock(IUserTools::class); - $config = $this->createMock(IConfig::class); - $filesys = $this->createMock(FilesystemHelper::class); - $log = $this->createMock(LogWrapper::class); - $avaMgr = $this->createMock(IAvatarManager::class); - $image = $this->createMock(Image::class); - $dbc = $this->createMock(IDBConnection::class); - $userMgr = $this->createMock(IUserManager::class); - $notiMgr = $this->createMock(INotificationManager::class); - - $connection = new \OCA\User_LDAP\Connection( - $lw = $this->createMock(ILDAPWrapper::class), - '', - null - ); + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ + protected $config; - $access->expects($this->any()) - ->method('getConnection') - ->will($this->returnValue($connection)); + /** @var FilesystemHelper|\PHPUnit_Framework_MockObject_MockObject */ + protected $fileSystemHelper; - return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr); - } + /** @var LogWrapper|\PHPUnit_Framework_MockObject_MockObject */ + protected $log; - public function testGetByDNExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); + /** @var IAvatarManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $avatarManager; - $inputDN = 'cn=foo,dc=foobar,dc=bar'; - $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; + /** @var Image|\PHPUnit_Framework_MockObject_MockObject */ + protected $image; - $access->expects($this->once()) - ->method('stringResemblesDN') - ->with($this->equalTo($inputDN)) - ->will($this->returnValue(true)); + /** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */ + protected $dbc; - $access->expects($this->once()) - ->method('dn2username') - ->with($this->equalTo($inputDN)) - ->will($this->returnValue($uid)); + /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $ncUserManager; - $access->expects($this->never()) - ->method('username2dn'); + /** @var INotificationManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $notificationManager; - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($inputDN); + /** @var ILDAPWrapper|\PHPUnit_Framework_MockObject_MockObject */ + protected $ldapWrapper; - // Now we fetch the user again. If this leads to a failing test, - // runtime caching the manager is broken. - $user = $manager->get($inputDN); - - $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user); - } + /** @var Connection */ + protected $connection; - public function testGetByEDirectoryDN() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); + /** @var Manager */ + protected $manager; - $inputDN = 'uid=foo,o=foobar,c=bar'; - $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; + public function setUp() { + parent::setUp(); - $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)); + $this->access = $this->createMock(Access::class); + $this->config = $this->createMock(IConfig::class); + $this->fileSystemHelper = $this->createMock(FilesystemHelper::class); + $this->log = $this->createMock(LogWrapper::class); + $this->avatarManager = $this->createMock(IAvatarManager::class); + $this->image = $this->createMock(Image::class); + $this->dbc = $this->createMock(IDBConnection::class); + $this->ncUserManager = $this->createMock(IUserManager::class); + $this->notificationManager = $this->createMock(INotificationManager::class); - $access->expects($this->never()) - ->method('username2dn'); + $this->ldapWrapper = $this->createMock(ILDAPWrapper::class); + $this->connection = new Connection($this->ldapWrapper, '', null); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($inputDN); + $this->access->expects($this->any()) + ->method('getConnection') + ->will($this->returnValue($this->connection)); + + /** @noinspection PhpUnhandledExceptionInspection */ + $this->manager = new Manager( + $this->config, + $this->fileSystemHelper, + $this->log, + $this->avatarManager, + $this->image, + $this->dbc, + $this->ncUserManager, + $this->notificationManager + ); - $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user); + $this->manager->setLdapAccess($this->access); } - public function testGetByExoticDN() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); + public function dnProvider() { + return [ + ['cn=foo,dc=foobar,dc=bar'], + ['uid=foo,o=foobar,c=bar'], + ['ab=cde,f=ghei,mno=pq'], + ]; + } - $inputDN = 'ab=cde,f=ghei,mno=pq'; + /** + * @dataProvider dnProvider + */ + public function testGetByDNExisting(string $inputDN) { $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('stringResemblesDN') ->with($this->equalTo($inputDN)) ->will($this->returnValue(true)); - - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('dn2username') ->with($this->equalTo($inputDN)) ->will($this->returnValue($uid)); - - $access->expects($this->never()) + $this->access->expects($this->never()) ->method('username2dn'); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($inputDN); + /** @noinspection PhpUnhandledExceptionInspection */ + $this->manager->get($inputDN); - $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user); + // Now we fetch the user again. If this leads to a failing test, + // runtime caching the manager is broken. + /** @noinspection PhpUnhandledExceptionInspection */ + $user = $this->manager->get($inputDN); + + $this->assertInstanceOf(User::class, $user); } public function testGetByDNNotExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); - $inputDN = 'cn=gone,dc=foobar,dc=bar'; - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('stringResemblesDN') ->with($this->equalTo($inputDN)) ->will($this->returnValue(true)); - - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('dn2username') ->with($this->equalTo($inputDN)) ->will($this->returnValue(false)); - - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('username2dn') ->with($this->equalTo($inputDN)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($inputDN); + /** @noinspection PhpUnhandledExceptionInspection */ + $user = $this->manager->get($inputDN); $this->assertNull($user); } public function testGetByUidExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); - $dn = 'cn=foo,dc=foobar,dc=bar'; $uid = '563418fc-423b-1033-8d1c-ad5f418ee02e'; - $access->expects($this->never()) + $this->access->expects($this->never()) ->method('dn2username'); - - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('username2dn') ->with($this->equalTo($uid)) ->will($this->returnValue($dn)); - - $access->expects($this->once()) + $this->access->expects($this->once()) ->method('stringResemblesDN') ->with($this->equalTo($uid)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($uid); + /** @noinspection PhpUnhandledExceptionInspection */ + $this->manager->get($uid); // Now we fetch the user again. If this leads to a failing test, // runtime caching the manager is broken. - $user = $manager->get($uid); + /** @noinspection PhpUnhandledExceptionInspection */ + $user = $this->manager->get($uid); - $this->assertInstanceOf('\OCA\User_LDAP\User\User', $user); + $this->assertInstanceOf(User::class, $user); } public function testGetByUidNotExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); - $uid = 'gone'; - $access->expects($this->never()) + $this->access->expects($this->never()) ->method('dn2username'); - - $access->expects($this->exactly(1)) + $this->access->expects($this->exactly(1)) ->method('username2dn') ->with($this->equalTo($uid)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - $user = $manager->get($uid); + /** @noinspection PhpUnhandledExceptionInspection */ + $user = $this->manager->get($uid); $this->assertNull($user); } public function attributeRequestProvider() { return [ - [ false ], - [ true ], + [false], + [true], ]; } @@ -249,23 +232,16 @@ class ManagerTest extends \Test\TestCase { * @dataProvider attributeRequestProvider */ public function testGetAttributes($minimal) { - list($access, $config, $filesys, $image, $log, $avaMgr, $dbc, $userMgr, $notiMgr) = - $this->getTestInstances(); - - $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc, $userMgr, $notiMgr); - $manager->setLdapAccess($access); - - $connection = $access->getConnection(); - $connection->setConfiguration([ + $this->connection->setConfiguration([ 'ldapEmailAttribute' => 'mail', 'ldapUserAvatarRule' => 'default', 'ldapQuotaAttribute' => '', ]); - $attributes = $manager->getAttributes($minimal); + $attributes = $this->manager->getAttributes($minimal); $this->assertTrue(in_array('dn', $attributes)); - $this->assertTrue(in_array($access->getConnection()->ldapEmailAttribute, $attributes)); + $this->assertTrue(in_array($this->access->getConnection()->ldapEmailAttribute, $attributes)); $this->assertFalse(in_array('', $attributes)); $this->assertSame(!$minimal, in_array('jpegphoto', $attributes)); $this->assertSame(!$minimal, in_array('thumbnailphoto', $attributes)); |