From 40ecd30fba6d29362c455b1f0eef875ff19cd544 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 6 Jan 2015 23:28:49 +0100 Subject: [PATCH] inject oc config to User_LDAP --- apps/user_ldap/appinfo/app.php | 8 ++- apps/user_ldap/appinfo/register_command.php | 8 ++- apps/user_ldap/command/search.php | 14 ++++- apps/user_ldap/lib/jobs/cleanup.php | 3 +- apps/user_ldap/lib/user/manager.php | 3 +- apps/user_ldap/lib/user/user.php | 2 +- apps/user_ldap/tests/user_ldap.php | 68 +++++++++++---------- apps/user_ldap/user_ldap.php | 32 +++++++--- apps/user_ldap/user_proxy.php | 6 +- 9 files changed, 90 insertions(+), 54 deletions(-) diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php index 302575c368..980477bb27 100644 --- a/apps/user_ldap/appinfo/app.php +++ b/apps/user_ldap/appinfo/app.php @@ -27,8 +27,8 @@ OCP\App::registerAdmin('user_ldap', 'settings'); $helper = new \OCA\user_ldap\lib\Helper(); $configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new OCA\user_ldap\lib\LDAP(); +$ocConfig = \OC::$server->getConfig(); if(count($configPrefixes) === 1) { - $ocConfig = \OC::$server->getConfig(); $userManager = new OCA\user_ldap\lib\user\Manager($ocConfig, new OCA\user_ldap\lib\FilesystemHelper(), new OCA\user_ldap\lib\LogWrapper(), @@ -39,10 +39,12 @@ if(count($configPrefixes) === 1) { $dbc = \OC::$server->getDatabaseConnection(); $ldapAccess->setUserMapper(new OCA\User_LDAP\Mapping\UserMapping($dbc)); $ldapAccess->setGroupMapper(new OCA\User_LDAP\Mapping\GroupMapping($dbc)); - $userBackend = new OCA\user_ldap\USER_LDAP($ldapAccess); + $userBackend = new OCA\user_ldap\USER_LDAP($ldapAccess, $ocConfig); $groupBackend = new OCA\user_ldap\GROUP_LDAP($ldapAccess); } else if(count($configPrefixes) > 1) { - $userBackend = new OCA\user_ldap\User_Proxy($configPrefixes, $ldapWrapper); + $userBackend = new OCA\user_ldap\User_Proxy( + $configPrefixes, $ldapWrapper, $ocConfig + ); $groupBackend = new OCA\user_ldap\Group_Proxy($configPrefixes, $ldapWrapper); } diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index 0e918b8233..6abfd699c9 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -15,12 +15,14 @@ use OCA\User_LDAP\lib\User\DeletedUsersIndex; $dbConnection = \OC::$server->getDatabaseConnection(); $userMapping = new UserMapping($dbConnection); $helper = new Helper(); +$ocConfig = \OC::$server->getConfig(); $uBackend = new User_Proxy( $helper->getServerConfigurationPrefixes(true), - new LDAP() + new LDAP(), + $ocConfig ); $deletedUsersIndex = new DeletedUsersIndex( - \OC::$server->getConfig(), $dbConnection, $userMapping + $ocConfig, $dbConnection, $userMapping ); $application->add(new OCA\user_ldap\Command\ShowConfig()); @@ -28,7 +30,7 @@ $application->add(new OCA\user_ldap\Command\SetConfig()); $application->add(new OCA\user_ldap\Command\TestConfig()); $application->add(new OCA\user_ldap\Command\CreateEmptyConfig()); $application->add(new OCA\user_ldap\Command\DeleteConfig()); -$application->add(new OCA\user_ldap\Command\Search()); +$application->add(new OCA\user_ldap\Command\Search($ocConfig)); $application->add(new OCA\user_ldap\Command\ShowRemnants($deletedUsersIndex)); $application->add(new OCA\user_ldap\Command\CheckUser( $uBackend, $helper, $deletedUsersIndex, $userMapping) diff --git a/apps/user_ldap/command/search.php b/apps/user_ldap/command/search.php index d826303c55..ba87982d16 100644 --- a/apps/user_ldap/command/search.php +++ b/apps/user_ldap/command/search.php @@ -18,8 +18,20 @@ use OCA\user_ldap\User_Proxy; use OCA\user_ldap\Group_Proxy; use OCA\user_ldap\lib\Helper; use OCA\user_ldap\lib\LDAP; +use OCP\IConfig; class Search extends Command { + /** @var \OCP\IConfig */ + protected $ocConfig; + + /** + * @param \OCP\IConfig $ocConfig + */ + public function __construct(IConfig $ocConfig) { + $this->ocConfig = $ocConfig; + parent::__construct(); + } + protected function configure() { $this ->setName('ldap:search') @@ -87,7 +99,7 @@ class Search extends Command { $getMethod = 'getGroups'; $printID = false; } else { - $proxy = new User_Proxy($configPrefixes, $ldapWrapper); + $proxy = new User_Proxy($configPrefixes, $ldapWrapper, $this->ocConfig); $getMethod = 'getDisplayNames'; $printID = true; } diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php index 83cfeb048c..b044f997aa 100644 --- a/apps/user_ldap/lib/jobs/cleanup.php +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -73,7 +73,8 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { } else { $this->userBackend = new User_Proxy( $this->ldapHelper->getServerConfigurationPrefixes(true), - new LDAP() + new LDAP(), + $this->ocConfig ); } diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index 431609071e..955a2923a6 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -72,8 +72,7 @@ class Manager { /** * @brief Constructor - * @param \OCP\IConfig respectively an instance that provides the methods - * setUserValue and getUserValue as implemented in \OCP\Config + * @param \OCP\IConfig * @param \OCA\user_ldap\lib\FilesystemHelper object that gives access to * necessary functions from the OC filesystem * @param \OCA\user_ldap\lib\LogWrapper diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index c81fb25b54..7f67ebca39 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -92,7 +92,7 @@ class User { * @param string the LDAP DN * @param IUserTools $access an instance that implements IUserTools for * LDAP interaction - * @param \OCP\Config + * @param \OCP\IConfig * @param FilesystemHelper * @param \OCP\Image any empty instance * @param LogWrapper diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index fdda35b63c..c4ba4f4579 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -156,7 +156,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = $backend->checkPassword('roland', 'dt19'); @@ -167,7 +167,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = $backend->checkPassword('roland', 'wrong'); @@ -178,7 +178,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = $backend->checkPassword('mallory', 'evil'); @@ -193,7 +193,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { ->method('username2dn') ->will($this->returnValue(false)); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = $backend->checkPassword('roland', 'dt19'); @@ -203,7 +203,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testCheckPasswordPublicAPI() { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::checkPassword('roland', 'dt19'); @@ -213,7 +213,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testCheckPasswordPublicAPIWrongPassword() { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::checkPassword('roland', 'wrong'); @@ -223,7 +223,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testCheckPasswordPublicAPIWrongUser() { $access = $this->getAccessMock(); $this->prepareAccessForCheckPassword($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::checkPassword('mallory', 'evil'); @@ -232,7 +232,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testDeleteUserCancel() { $access = $this->getAccessMock(); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->deleteUser('notme'); $this->assertFalse($result); } @@ -249,10 +249,12 @@ class Test_User_Ldap_Direct extends \Test\TestCase { ->method('getUserMapper') ->will($this->returnValue($mapping)); - $backend = new UserLDAP($access); + $config = $this->getMock('\OCP\IConfig'); + $config->expects($this->exactly(2)) + ->method('getUserValue') + ->will($this->returnValue(1)); - $pref = \OC::$server->getConfig(); - $pref->setUserValue('jeremy', 'user_ldap', 'isDeleted', 1); + $backend = new UserLDAP($access, $config); $result = $backend->deleteUser('jeremy'); $this->assertTrue($result); @@ -310,7 +312,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersNoParam() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->getUsers(); $this->assertEquals(3, count($result)); @@ -319,7 +321,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersLimitOffset() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->getUsers('', 1, 2); $this->assertEquals(1, count($result)); @@ -328,7 +330,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersLimitOffset2() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->getUsers('', 2, 1); $this->assertEquals(2, count($result)); @@ -337,7 +339,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersSearchWithResult() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->getUsers('yo'); $this->assertEquals(2, count($result)); @@ -346,7 +348,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersSearchEmptyResult() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->getUsers('nix'); $this->assertEquals(0, count($result)); @@ -355,7 +357,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersViaAPINoParam() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::getUsers(); @@ -365,7 +367,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersViaAPILimitOffset() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::getUsers('', 1, 2); @@ -375,7 +377,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersViaAPILimitOffset2() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::getUsers('', 2, 1); @@ -385,7 +387,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersViaAPISearchWithResult() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::getUsers('yo'); @@ -395,7 +397,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetUsersViaAPISearchEmptyResult() { $access = $this->getAccessMock(); $this->prepareAccessForGetUsers($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); \OC_User::useBackend($backend); $result = \OCP\User::getUsers('nix'); @@ -404,7 +406,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testUserExists() { $access = $this->getAccessMock(); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $this->prepareMockForUserExists($access); $access->expects($this->any()) @@ -431,7 +433,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testUserExistsPublicAPI() { $access = $this->getAccessMock(); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $this->prepareMockForUserExists($access); \OC_User::useBackend($backend); @@ -459,7 +461,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testDeleteUser() { $access = $this->getAccessMock(); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); //we do not support deleting users at all $result = $backend->deleteUser('gunslinger'); @@ -468,7 +470,8 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetHome() { $access = $this->getAccessMock(); - $backend = new UserLDAP($access); + $config = $this->getMock('\OCP\IConfig'); + $backend = new UserLDAP($access, $config); $this->prepareMockForUserExists($access); $access->connection->expects($this->any()) @@ -501,14 +504,17 @@ class Test_User_Ldap_Direct extends \Test\TestCase { } })); + $datadir = '/my/data/dir'; + $config->expects($this->once()) + ->method('getSystemValue') + ->will($this->returnValue($datadir)); + //absolut path $result = $backend->getHome('gunslinger'); $this->assertEquals('/tmp/rolandshome/', $result); //datadir-relativ path $result = $backend->getHome('ladyofshadows'); - $datadir = \OCP\Config::getSystemValue('datadirectory', - \OC::$SERVERROOT.'/data'); $this->assertEquals($datadir.'/susannah/', $result); //no path at all – triggers OC default behaviour @@ -546,7 +552,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetDisplayName() { $access = $this->getAccessMock(); $this->prepareAccessForGetDisplayName($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $this->prepareMockForUserExists($access); //with displayName @@ -561,7 +567,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testGetDisplayNamePublicAPI() { $access = $this->getAccessMock(); $this->prepareAccessForGetDisplayName($access); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $this->prepareMockForUserExists($access); \OC_User::useBackend($backend); @@ -584,7 +590,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { ->method('countUsers') ->will($this->returnValue(5)); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->countUsers(); $this->assertEquals(5, $result); @@ -597,7 +603,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { ->method('countUsers') ->will($this->returnValue(false)); - $backend = new UserLDAP($access); + $backend = new UserLDAP($access, $this->getMock('\OCP\IConfig')); $result = $backend->countUsers(); $this->assertFalse($result); diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index c2d0f387b9..f9a39ddca1 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -26,15 +26,27 @@ namespace OCA\user_ldap; use OCA\user_ldap\lib\BackendUtility; +use OCA\user_ldap\lib\Access; use OCA\user_ldap\lib\user\OfflineUser; use OCA\User_LDAP\lib\User\User; +use OCP\IConfig; class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface { - /** - * @var string[] $homesToKill - */ + /** @var string[] $homesToKill */ protected $homesToKill = array(); + /** @var \OCP\IConfig */ + protected $ocConfig; + + /** + * @param \OCA\user_ldap\lib\Access $access + * @param \OCP\IConfig $ocConfig + */ + public function __construct(Access $access, IConfig $ocConfig) { + parent::__construct($access); + $this->ocConfig = $ocConfig; + } + /** * checks whether the user is allowed to change his avatar in ownCloud * @param string $uid the ownCloud user name @@ -214,8 +226,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn * @return bool */ public function deleteUser($uid) { - $pref = \OC::$server->getConfig(); - $marked = $pref->getUserValue($uid, 'user_ldap', 'isDeleted', 0); + $marked = $this->ocConfig->getUserValue($uid, 'user_ldap', 'isDeleted', 0); if(intval($marked) === 0) { \OC::$server->getLogger()->notice( 'User '.$uid . ' is not marked as deleted, not cleaning up.', @@ -227,7 +238,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn //Get Home Directory out of user preferences so we can return it later, //necessary for removing directories as done by OC_User. - $home = $pref->getUserValue($uid, 'user_ldap', 'homePath', ''); + $home = $this->ocConfig->getUserValue($uid, 'user_ldap', 'homePath', ''); $this->homesToKill[$uid] = $home; $this->access->getUserMapper()->unmap($uid); @@ -254,7 +265,6 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn if($this->access->connection->isCached($cacheKey)) { return $this->access->connection->getFromCache($cacheKey); } - $pref = \OC::$server->getConfig(); if(strpos($this->access->connection->homeFolderNamingRule, 'attr:') === 0) { $attr = substr($this->access->connection->homeFolderNamingRule, strlen('attr:')); $homedir = $this->access->readAttribute( @@ -270,13 +280,15 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn ) { $homedir = $path; } else { - $homedir = \OC::$server->getConfig()->getSystemValue('datadirectory', + $homedir = $this->ocConfig->getSystemValue('datadirectory', \OC::$SERVERROOT.'/data' ) . '/' . $homedir[0]; } $this->access->connection->writeToCache($cacheKey, $homedir); //we need it to store it in the DB as well in case a user gets //deleted so we can clean up afterwards - $pref->setUserValue($uid, 'user_ldap', 'homePath', $homedir); + $this->ocConfig->setUserValue( + $uid, 'user_ldap', 'homePath', $homedir + ); //TODO: if home directory changes, the old one needs to be removed. return $homedir; } @@ -284,7 +296,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn //false will apply default behaviour as defined and done by OC_User $this->access->connection->writeToCache($cacheKey, false); - $pref->setUserValue($uid, 'user_ldap', 'homePath', ''); + $this->ocConfig->setUserValue($uid, 'user_ldap', 'homePath', ''); return false; } diff --git a/apps/user_ldap/user_proxy.php b/apps/user_ldap/user_proxy.php index 77caa84ecd..f5912fe135 100644 --- a/apps/user_ldap/user_proxy.php +++ b/apps/user_ldap/user_proxy.php @@ -25,6 +25,8 @@ namespace OCA\user_ldap; use OCA\user_ldap\lib\ILDAPWrapper; use OCA\User_LDAP\lib\User\User; +use \OCA\user_ldap\User_LDAP; +use OCP\IConfig; class User_Proxy extends lib\Proxy implements \OCP\IUserBackend, \OCP\UserInterface { private $backends = array(); @@ -34,11 +36,11 @@ class User_Proxy extends lib\Proxy implements \OCP\IUserBackend, \OCP\UserInterf * Constructor * @param array $serverConfigPrefixes array containing the config Prefixes */ - public function __construct($serverConfigPrefixes, ILDAPWrapper $ldap) { + public function __construct(array $serverConfigPrefixes, ILDAPWrapper $ldap, IConfig $ocConfig) { parent::__construct($ldap); foreach($serverConfigPrefixes as $configPrefix) { $this->backends[$configPrefix] = - new \OCA\user_ldap\USER_LDAP($this->getAccess($configPrefix)); + new User_LDAP($this->getAccess($configPrefix), $ocConfig); if(is_null($this->refBackend)) { $this->refBackend = &$this->backends[$configPrefix]; }