From 4fa39250e714b3ee5aa16a5f9ce8c77daa44311b Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 21 Aug 2014 17:59:13 +0200 Subject: LDAP User Cleanup: Port from stable7 without further adjustements LDAP User Cleanup background job for user clean up adjust user backend for clean up register background job remove dead code dependency injection make Helper non-static for proper testing check whether it is OK to run clean up job. Do not forget to pass arguments. use correct method to get the config from server methods can be private, proper indirect testing is given no automatic user deletion make limit readable for test purposes make method less complex add first tests let preferences accept limit and offset for getUsersForValue DI via constructor does not work for background jobs after detecting, now we have retrieving deleted users and their details we need this method to be public for now finalize export method, add missing getter clean up namespaces and get rid of unnecessary files helper is not static anymore cleanup according to scrutinizer add cli tool to show deleted users uses are necessary after recent namespace change also remove user from mappings table on deletion add occ command to delete users fix use statement improve output big fixes / improvements PHP doc return true in userExists early for cleaning up deleted users bump version control state and interval with one config.php setting, now ldapUserCleanupInterval. 0 will disable it. enabled by default. improve doc rename cli method to be consistent with others introduce ldapUserCleanupInterval in sample config don't show last login as unix epoche start when no login happend less log output consistent namespace for OfflineUser rename GarbageCollector to DeletedUsersIndex and move it to user subdir fix unit tests add tests for deleteUser more test adjustements Conflicts: apps/user_ldap/ajax/clearMappings.php apps/user_ldap/appinfo/app.php apps/user_ldap/lib/access.php apps/user_ldap/lib/helper.php apps/user_ldap/tests/helper.php core/register_command.php lib/private/preferences.php lib/private/user.php add ldap:check-user to check user existance on the fly Conflicts: apps/user_ldap/lib/helper.php forgotten file PHPdoc fixes, no code change and don't forget to adjust tests --- apps/user_ldap/ajax/deleteConfiguration.php | 3 +- apps/user_ldap/ajax/getNewServerConfigPrefix.php | 3 +- apps/user_ldap/appinfo/app.php | 14 +- apps/user_ldap/appinfo/register_command.php | 13 ++ apps/user_ldap/appinfo/update.php | 3 +- apps/user_ldap/appinfo/version | 2 +- apps/user_ldap/command/checkuser.php | 129 +++++++++++++ apps/user_ldap/command/search.php | 3 +- apps/user_ldap/command/setconfig.php | 3 +- apps/user_ldap/command/showconfig.php | 3 +- apps/user_ldap/command/showremnants.php | 81 ++++++++ apps/user_ldap/command/testconfig.php | 3 +- apps/user_ldap/lib/access.php | 1 + apps/user_ldap/lib/connection.php | 3 +- apps/user_ldap/lib/helper.php | 24 ++- apps/user_ldap/lib/jobs.php | 3 +- apps/user_ldap/lib/jobs/cleanup.php | 227 +++++++++++++++++++++++ apps/user_ldap/lib/user/deletedusersindex.php | 125 +++++++++++++ apps/user_ldap/lib/user/iusertools.php | 3 + apps/user_ldap/lib/user/manager.php | 63 +++++-- apps/user_ldap/lib/user/offlineuser.php | 217 ++++++++++++++++++++++ apps/user_ldap/lib/user/user.php | 25 +++ apps/user_ldap/lib/wizard.php | 3 +- apps/user_ldap/settings.php | 5 +- apps/user_ldap/tests/jobs/cleanup.php | 155 ++++++++++++++++ apps/user_ldap/tests/user/manager.php | 2 +- apps/user_ldap/tests/user_ldap.php | 20 +- apps/user_ldap/user_ldap.php | 116 ++++++++++-- apps/user_ldap/user_proxy.php | 14 +- config/config.sample.php | 16 +- core/command/user/delete.php | 36 ++++ core/register_command.php | 1 + lib/private/preferences.php | 4 +- 33 files changed, 1257 insertions(+), 66 deletions(-) create mode 100644 apps/user_ldap/command/checkuser.php create mode 100644 apps/user_ldap/command/showremnants.php create mode 100644 apps/user_ldap/lib/jobs/cleanup.php create mode 100644 apps/user_ldap/lib/user/deletedusersindex.php create mode 100644 apps/user_ldap/lib/user/offlineuser.php create mode 100644 apps/user_ldap/tests/jobs/cleanup.php create mode 100644 core/command/user/delete.php diff --git a/apps/user_ldap/ajax/deleteConfiguration.php b/apps/user_ldap/ajax/deleteConfiguration.php index bca687c81ab..d409d891f61 100644 --- a/apps/user_ldap/ajax/deleteConfiguration.php +++ b/apps/user_ldap/ajax/deleteConfiguration.php @@ -27,7 +27,8 @@ OCP\JSON::checkAppEnabled('user_ldap'); OCP\JSON::callCheck(); $prefix = $_POST['ldap_serverconfig_chooser']; -if(\OCA\user_ldap\lib\Helper::deleteServerConfiguration($prefix)) { +$helper = new \OCA\user_ldap\lib\Helper(); +if($helper->deleteServerConfiguration($prefix)) { OCP\JSON::success(); } else { $l = \OC::$server->getL10N('user_ldap'); diff --git a/apps/user_ldap/ajax/getNewServerConfigPrefix.php b/apps/user_ldap/ajax/getNewServerConfigPrefix.php index 1c68b2e9a76..ce6c5ae76e8 100644 --- a/apps/user_ldap/ajax/getNewServerConfigPrefix.php +++ b/apps/user_ldap/ajax/getNewServerConfigPrefix.php @@ -26,7 +26,8 @@ OCP\JSON::checkAdminUser(); OCP\JSON::checkAppEnabled('user_ldap'); OCP\JSON::callCheck(); -$serverConnections = \OCA\user_ldap\lib\Helper::getServerConfigurationPrefixes(); +$helper = new \OCA\user_ldap\lib\Helper(); +$serverConnections = $helper->getServerConfigurationPrefixes(); sort($serverConnections); $lk = array_pop($serverConnections); $ln = intval(str_replace('s', '', $lk)); diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php index 98d5fb60183..302575c3682 100644 --- a/apps/user_ldap/appinfo/app.php +++ b/apps/user_ldap/appinfo/app.php @@ -5,6 +5,7 @@ * * @author Dominik Schmidt * @copyright 2011 Dominik Schmidt dev@dominik-schmidt.de +* @copyright 2014 Arthur Schiwon * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE @@ -23,7 +24,8 @@ OCP\App::registerAdmin('user_ldap', 'settings'); -$configPrefixes = OCA\user_ldap\lib\Helper::getServerConfigurationPrefixes(true); +$helper = new \OCA\user_ldap\lib\Helper(); +$configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new OCA\user_ldap\lib\LDAP(); if(count($configPrefixes) === 1) { $ocConfig = \OC::$server->getConfig(); @@ -50,16 +52,10 @@ if(count($configPrefixes) > 0) { OC_Group::useBackend($groupBackend); } -// add settings page to navigation -$entry = array( - 'id' => 'user_ldap_settings', - 'order'=>1, - 'href' => OCP\Util::linkTo( 'user_ldap', 'settings.php' ), - 'name' => 'LDAP' -); OCP\Util::addTranslations('user_ldap'); - OCP\Backgroundjob::registerJob('OCA\user_ldap\lib\Jobs'); +OCP\Backgroundjob::registerJob('\OCA\User_LDAP\Jobs\CleanUp'); + if(OCP\App::isEnabled('user_webdavauth')) { OCP\Util::writeLog('user_ldap', 'user_ldap and user_webdavauth are incompatible. You may experience unexpected behaviour', diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index 1a0227db95e..55a187d9654 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -6,9 +6,22 @@ * See the COPYING-README file. */ +use OCA\user_ldap\lib\Helper; +use OCA\user_ldap\lib\LDAP; +use OCA\user_ldap\User_Proxy; + $application->add(new OCA\user_ldap\Command\ShowConfig()); $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\ShowRemnants()); +$helper = new OCA\user_ldap\lib\Helper(); +$uBackend = new OCA\user_ldap\User_Proxy( + $helper->getServerConfigurationPrefixes(true), + new OCA\user_ldap\lib\LDAP() +); +$application->add(new OCA\user_ldap\Command\CheckUser( + $uBackend, $helper, \OC::$server->getConfig() +)); diff --git a/apps/user_ldap/appinfo/update.php b/apps/user_ldap/appinfo/update.php index 9bf0ca4ab53..b4121b19852 100644 --- a/apps/user_ldap/appinfo/update.php +++ b/apps/user_ldap/appinfo/update.php @@ -12,7 +12,8 @@ if($state === 'unset') { $installedVersion = $configInstance->getAppValue('user_ldap', 'installed_version'); $enableRawMode = version_compare($installedVersion, '0.4.1', '<'); -$configPrefixes = OCA\user_ldap\lib\Helper::getServerConfigurationPrefixes(true); +$helper = new \OCA\user_ldap\lib\Helper(); +$configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldap = new OCA\user_ldap\lib\LDAP(); foreach($configPrefixes as $config) { $connection = new OCA\user_ldap\lib\Connection($ldap, $config); diff --git a/apps/user_ldap/appinfo/version b/apps/user_ldap/appinfo/version index 6f2743d65dc..0bfccb08040 100644 --- a/apps/user_ldap/appinfo/version +++ b/apps/user_ldap/appinfo/version @@ -1 +1 @@ -0.4.4 +0.4.5 diff --git a/apps/user_ldap/command/checkuser.php b/apps/user_ldap/command/checkuser.php new file mode 100644 index 00000000000..96c6c832356 --- /dev/null +++ b/apps/user_ldap/command/checkuser.php @@ -0,0 +1,129 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\user_ldap\Command; + +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +use OCA\user_ldap\lib\user\User; +use OCA\User_LDAP\lib\user\Manager; +use OCA\user_ldap\lib\Helper; +use OCA\user_ldap\User_Proxy; + +class CheckUser extends Command { + /** @var \OCA\user_ldap\User_Proxy */ + protected $backend; + + /** @var \OCA\User_LDAP\lib\Helper */ + protected $helper; + + /** @var \OCP\IConfig */ + protected $config; + + /** + * @param OCA\user_ldap\User_Proxy $uBackend + * @param OCA\User_LDAP\lib\Helper $helper + * @param OCP\IConfig $config + */ + public function __construct(User_Proxy $uBackend, Helper $helper, \OCP\IConfig $config) { + $this->backend = $uBackend; + $this->helper = $helper; + $this->config = $config; + parent::__construct(); + } + + protected function configure() { + $this + ->setName('ldap:check-user') + ->setDescription('checks whether a user exists on LDAP.') + ->addArgument( + 'ocName', + InputArgument::REQUIRED, + 'the user name as used in ownCloud' + ) + ->addOption( + 'force', + null, + InputOption::VALUE_NONE, + 'ignores disabled LDAP configuration' + ) + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) { + try { + $uid = $input->getArgument('ocName'); + $this->isAllowed($input->getOption('force')); + $this->confirmUserIsMapped($uid); + $exists = $this->backend->userExistsOnLDAP($uid); + if($exists === true) { + $output->writeln('The user is still available on LDAP.'); + return; + } + + // TODO FIXME consolidate next line in DeletedUsersIndex + // (impractical now, because of class dependencies) + $this->config->setUserValue($uid, 'user_ldap', 'isDeleted', '1'); + + $output->writeln('The user does not exists on LDAP anymore.'); + $output->writeln('Clean up the user\'s remnants by: ./occ user:delete "' + . $uid . '"'); + } catch (\Exception $e) { + $output->writeln('' . $e->getMessage(). ''); + } + } + + /** + * checks whether a user is actually mapped + * @param string $ocName the username as used in ownCloud + * @throws \Exception + * @return bool + */ + protected function confirmUserIsMapped($ocName) { + //TODO FIXME this should go to Mappings in OC 8 + $db = \OC::$server->getDatabaseConnection(); + $query = $db->prepare(' + SELECT + `ldap_dn` AS `dn` + FROM `*PREFIX*ldap_user_mapping` + WHERE `owncloud_name` = ?' + ); + + $query->execute(array($ocName)); + $result = $query->fetchColumn(); + + if($result === false) { + throw new \Exception('The given user is not a recognized LDAP user.'); + } + + return true; + } + + /** + * checks whether the setup allows reliable checking of LDAP user existance + * @throws \Exception + * @return bool + */ + protected function isAllowed($force) { + if($this->helper->haveDisabledConfigurations() && !$force) { + throw new \Exception('Cannot check user existance, because ' + . 'disabled LDAP configurations are present.'); + } + + // we don't check ldapUserCleanupInterval from config.php because this + // action is triggered manually, while the setting only controls the + // background job. + + return true; + } + +} diff --git a/apps/user_ldap/command/search.php b/apps/user_ldap/command/search.php index e20255510d8..d826303c55d 100644 --- a/apps/user_ldap/command/search.php +++ b/apps/user_ldap/command/search.php @@ -74,7 +74,8 @@ class Search extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $configPrefixes = Helper::getServerConfigurationPrefixes(true); + $helper = new Helper(); + $configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new LDAP(); $offset = intval($input->getOption('offset')); diff --git a/apps/user_ldap/command/setconfig.php b/apps/user_ldap/command/setconfig.php index ab1c8d39ead..9128fcf04fc 100644 --- a/apps/user_ldap/command/setconfig.php +++ b/apps/user_ldap/command/setconfig.php @@ -41,7 +41,8 @@ class SetConfig extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $availableConfigs = Helper::getServerConfigurationPrefixes(); + $helper = new Helper(); + $availableConfigs = $helper->getServerConfigurationPrefixes(); $configID = $input->getArgument('configID'); if(!in_array($configID, $availableConfigs)) { $output->writeln("Invalid configID"); diff --git a/apps/user_ldap/command/showconfig.php b/apps/user_ldap/command/showconfig.php index f51d641beec..ddbc45243ff 100644 --- a/apps/user_ldap/command/showconfig.php +++ b/apps/user_ldap/command/showconfig.php @@ -31,7 +31,8 @@ class ShowConfig extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $availableConfigs = Helper::getServerConfigurationPrefixes(); + $helper = new Helper(); + $availableConfigs = $helper->getServerConfigurationPrefixes(); $configID = $input->getArgument('configID'); if(!is_null($configID)) { $configIDs[] = $configID; diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php new file mode 100644 index 00000000000..3d39f977421 --- /dev/null +++ b/apps/user_ldap/command/showremnants.php @@ -0,0 +1,81 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\user_ldap\Command; + +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; + +use OCA\user_ldap\lib\user\DeletedUsersIndex; +use OCA\User_LDAP\lib\Connection; +use OCA\User_LDAP\lib\Access; + +class ShowRemnants extends Command { + + protected function configure() { + $this + ->setName('ldap:show-remnants') + ->setDescription('shows which users are not available on LDAP anymore, but have remnants in ownCloud.') + ; + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $dui = new DeletedUsersIndex( + new \OC\Preferences(\OC_DB::getConnection()), + \OC::$server->getDatabaseConnection(), + $this->getAccess() + ); + + /** @var \Symfony\Component\Console\Helper\Table $table */ + $table = $this->getHelperSet()->get('table'); + $table->setHeaders(array( + 'ownCloud name', 'Display Name', 'LDAP UID', 'LDAP DN', 'Last Login', + 'Dir', 'Sharer')); + $rows = array(); + $offset = 0; + do { + $resultSet = $dui->getUsers($offset); + $offset += count($resultSet); + foreach($resultSet as $user) { + $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; + $lastLogin = ($user->getLastLogin() > 0) ? + \OCP\Util::formatDate($user->getLastLogin()) : '-'; + $rows[] = array( + $user->getOCName(), + $user->getDisplayName(), + $user->getUid(), + $user->getDN(), + $lastLogin, + $user->getHomePath(), + $hAS + ); + } + } while (count($resultSet) === 10); + + $table->setRows($rows); + $table->render($output); + } + + protected function getAccess() { + $ldap = new \OCA\user_ldap\lib\LDAP(); + $dummyConnection = new Connection($ldap, '', null); + $userManager = new \OCA\user_ldap\lib\user\Manager( + \OC::$server->getConfig(), + new \OCA\user_ldap\lib\FilesystemHelper(), + new \OCA\user_ldap\lib\LogWrapper(), + \OC::$server->getAvatarManager(), + new \OCP\Image() + ); + $access = new Access($dummyConnection, $ldap, $userManager); + return $access; + } + +} diff --git a/apps/user_ldap/command/testconfig.php b/apps/user_ldap/command/testconfig.php index 00b4acf2f66..a44e22415e9 100644 --- a/apps/user_ldap/command/testconfig.php +++ b/apps/user_ldap/command/testconfig.php @@ -31,7 +31,8 @@ class TestConfig extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $availableConfigs = Helper::getServerConfigurationPrefixes(); + $helper = new Helper(); + $availableConfigs = $helper->getServerConfigurationPrefixes(); $configID = $input->getArgument('configID'); if(!in_array($configID, $availableConfigs)) { $output->writeln("Invalid configID"); diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 5d0910320bf..692afb98f99 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -290,6 +290,7 @@ class Access extends LDAPUtility implements user\IUserTools { } /** + public function ocname2dn($name, $isUser) { * returns the internal ownCloud name for the given LDAP DN of the group, false on DN outside of search DN or failure * @param string $fdn the dn of the group object * @param string $ldapName optional, the display name of the object diff --git a/apps/user_ldap/lib/connection.php b/apps/user_ldap/lib/connection.php index 54aafb93410..5df5031d001 100644 --- a/apps/user_ldap/lib/connection.php +++ b/apps/user_ldap/lib/connection.php @@ -71,8 +71,9 @@ class Connection extends LDAPUtility { } $this->hasPagedResultSupport = $this->ldap->hasPagedResultSupport(); + $helper = new Helper(); $this->doNotValidate = !in_array($this->configPrefix, - Helper::getServerConfigurationPrefixes()); + $helper->getServerConfigurationPrefixes()); } public function __destruct() { diff --git a/apps/user_ldap/lib/helper.php b/apps/user_ldap/lib/helper.php index fa36e304171..7a96cfa36c4 100644 --- a/apps/user_ldap/lib/helper.php +++ b/apps/user_ldap/lib/helper.php @@ -45,7 +45,7 @@ class Helper { * except the default (first) server shall be connected to. * */ - static public function getServerConfigurationPrefixes($activeConfigurations = false) { + public function getServerConfigurationPrefixes($activeConfigurations = false) { $referenceConfigkey = 'ldap_configuration_active'; $sql = ' @@ -83,7 +83,7 @@ class Helper { * @return array an array with configprefix as keys * */ - static public function getServerConfigurationHosts() { + public function getServerConfigurationHosts() { $referenceConfigkey = 'ldap_host'; $query = ' @@ -110,7 +110,7 @@ class Helper { * @param string $prefix the configuration prefix of the config to delete * @return bool true on success, false otherwise */ - static public function deleteServerConfiguration($prefix) { + public function deleteServerConfiguration($prefix) { if(!in_array($prefix, self::getServerConfigurationPrefixes())) { return false; } @@ -141,12 +141,28 @@ class Helper { return true; } + /** + * checks whether there is one or more disabled LDAP configurations + * @throws \Exception + * @return bool + */ + public function haveDisabledConfigurations() { + $all = $this->getServerConfigurationPrefixes(false); + $active = $this->getServerConfigurationPrefixes(true); + + if(!is_array($all) || !is_array($active)) { + throw new \Exception('Unexpected Return Value'); + } + + return count($all) !== count($active) || count($all) === 0; + } + /** * extracts the domain from a given URL * @param string $url the URL * @return string|false domain as string on success, false otherwise */ - static public function getDomainFromURL($url) { + public function getDomainFromURL($url) { $uinfo = parse_url($url); if(!is_array($uinfo)) { return false; diff --git a/apps/user_ldap/lib/jobs.php b/apps/user_ldap/lib/jobs.php index 47e536f8f64..30f09cdc8f8 100644 --- a/apps/user_ldap/lib/jobs.php +++ b/apps/user_ldap/lib/jobs.php @@ -156,7 +156,8 @@ class Jobs extends \OC\BackgroundJob\TimedJob { if(!is_null(self::$groupBE)) { return self::$groupBE; } - $configPrefixes = Helper::getServerConfigurationPrefixes(true); + $helper = new Helper(); + $configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new LDAP(); if(count($configPrefixes) === 1) { //avoid the proxy when there is only one LDAP server configured diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php new file mode 100644 index 00000000000..56fb296609d --- /dev/null +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -0,0 +1,227 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\User_LDAP\Jobs; + +use \OCA\user_ldap\User_Proxy; +use \OCA\user_ldap\lib\Helper; +use \OCA\user_ldap\lib\LDAP; + +/** + * Class CleanUp + * + * a Background job to clean up deleted users + * + * @package OCA\user_ldap\lib; + */ +class CleanUp extends \OC\BackgroundJob\TimedJob { + /** + * @var int $limit amount of users that should be checked per run + */ + protected $limit = 50; + + /** + * @var \OCP\UserInterface $userBackend + */ + protected $userBackend; + + /** + * @var \OCP\IConfig $ocConfig + */ + protected $ocConfig; + + /** + * @var \OCP\IDBConnection $db + */ + protected $db; + + /** + * @var Helper $ldapHelper + */ + protected $ldapHelper; + + /** + * @var int $defaultIntervalMin default interval in minutes + */ + protected $defaultIntervalMin = 51; + + public function __construct() { + $minutes = \OC::$server->getConfig()->getSystemValue( + 'ldapUserCleanupInterval', strval($this->defaultIntervalMin)); + $this->setInterval(intval($minutes) * 60); + } + + /** + * assigns the instances passed to run() to the class properties + * @param array $arguments + */ + public function setArguments($arguments) { + //Dependency Injection is not possible, because the constructor will + //only get values that are serialized to JSON. I.e. whatever we would + //pass in app.php we do add here, except something else is passed e.g. + //in tests. + + if(isset($arguments['helper'])) { + $this->ldapHelper = $arguments['helper']; + } else { + $this->ldapHelper = new Helper(); + } + + if(isset($arguments['userBackend'])) { + $this->userBackend = $arguments['userBackend']; + } else { + $this->userBackend = new User_Proxy( + $this->ldapHelper->getServerConfigurationPrefixes(true), + new LDAP() + ); + } + + if(isset($arguments['ocConfig'])) { + $this->ocConfig = $arguments['ocConfig']; + } else { + $this->ocConfig = \OC::$server->getConfig(); + } + + if(isset($arguments['db'])) { + $this->db = $arguments['db']; + } else { + $this->db = \OC::$server->getDatabaseConnection(); + } + } + + /** + * makes the background job do its work + * @param array $argument + */ + public function run($argument) { + $this->setArguments($argument); + + if(!$this->isCleanUpAllowed()) { + return; + } + $users = $this->getMappedUsers($this->limit, $this->getOffset()); + if(!is_array($users)) { + //something wrong? Let's start from the beginning next time and + //abort + $this->setOffset(true); + return; + } + $resetOffset = $this->isOffsetResetNecessary(count($users)); + $this->checkUsers($users); + $this->setOffset($resetOffset); + } + + /** + * checks whether next run should start at 0 again + * @param int $resultCount + * @return bool + */ + public function isOffsetResetNecessary($resultCount) { + return ($resultCount < $this->limit) ? true : false; + } + + /** + * checks whether cleaning up LDAP users is allowed + * @return bool + */ + public function isCleanUpAllowed() { + try { + if($this->ldapHelper->haveDisabledConfigurations()) { + return false; + } + } catch (\Exception $e) { + return false; + } + + $enabled = $this->isCleanUpEnabled(); + + return $enabled; + } + + /** + * checks whether clean up is enabled by configuration + * @return bool + */ + private function isCleanUpEnabled() { + return (bool)$this->ocConfig->getSystemValue( + 'ldapUserCleanupInterval', strval($this->defaultIntervalMin)); + } + + /** + * checks users whether they are still existing + * @param array $users result from getMappedUsers() + */ + private function checkUsers($users) { + foreach($users as $user) { + $this->checkUser($user); + } + } + + /** + * checks whether a user is still existing in LDAP + * @param string[] $user + */ + private function checkUser($user) { + if($this->userBackend->userExistsOnLDAP($user['name'])) { + //still available, all good + return; + } + + // TODO FIXME consolidate next line in DeletedUsersIndex + // (impractical now, because of class dependencies) + $this->ocConfig->setUserValue($user['name'], 'user_ldap', 'isDeleted', '1'); + } + + /** + * returns a batch of users from the mappings table + * @param int $limit + * @param int $offset + * @return array + */ + public function getMappedUsers($limit, $offset) { + $query = $this->db->prepare(' + SELECT + `ldap_dn` AS `dn`, + `owncloud_name` AS `name`, + `directory_uuid` AS `uuid` + FROM `*PREFIX*ldap_user_mapping`', + $limit, + $offset + ); + + $query->execute(); + return $query->fetchAll(); + } + + /** + * gets the offset to fetch users from the mappings table + * @return int + */ + private function getOffset() { + return $this->ocConfig->getAppValue('user_ldap', 'cleanUpJobOffset', 0); + } + + /** + * sets the new offset for the next run + * @param bool $reset whether the offset should be set to 0 + */ + public function setOffset($reset = false) { + $newOffset = $reset ? 0 : + $this->getOffset() + $this->limit; + $this->ocConfig->setAppValue('user_ldap', 'cleanUpJobOffset', $newOffset); + } + + /** + * returns the chunk size (limit in DB speak) + * @return int + */ + public function getChunkSize() { + return $this->limit; + } + +} diff --git a/apps/user_ldap/lib/user/deletedusersindex.php b/apps/user_ldap/lib/user/deletedusersindex.php new file mode 100644 index 00000000000..0d8bacffe94 --- /dev/null +++ b/apps/user_ldap/lib/user/deletedusersindex.php @@ -0,0 +1,125 @@ + + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE + * License as published by the Free Software Foundation; either + * version 3 of the License, or any later version. + * + * This library 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 along with this library. If not, see . + * + */ + +namespace OCA\user_ldap\lib\user; + +use OCA\user_ldap\lib\user\OfflineUser; +use OCA\user_ldap\lib\Access; + +/** + * Class DeletedUsersIndex + * @package OCA\User_LDAP + */ +class DeletedUsersIndex { + /** + * @var \OC\Preferences $preferences + */ + protected $preferences; + + /** + * @var \OCP\IDBConnection $db + */ + protected $db; + + /** + * @var \OCA\user_ldap\lib\Access $access + */ + protected $access; + + /** + * @var int $limit + */ + protected $limit = 10; + + /** + * @var array $deletedUsers + */ + protected $deletedUsers = false; + + public function __construct(\OC\Preferences $preferences, \OCP\IDBConnection $db, Access $access) { + $this->preferences = $preferences; + $this->db = $db; + $this->access = $access; + } + + /** + * returns key to be used against $this->deletedUsers + * @param int $limit + * @param int $offset + * @return string + */ + private function getDeletedUsersCacheKey($limit, $offset) { + return strval($limit) . '.' . strval($offset); + } + + /** + * reads LDAP users marked as deleted from the database + * @param int $offset + * @return OCA\user_ldap\lib\user\OfflineUser[] + */ + private function fetchDeletedUsers($offset) { + $deletedUsers = $this->preferences->getUsersForValue( + 'user_ldap', 'isDeleted', '1', $this->limit, $offset); + $key = $this->getDeletedUsersCacheKey($this->limit, $offset); + + $userObjects = array(); + foreach($deletedUsers as $user) { + $userObjects[] = new OfflineUser($user, $this->preferences, $this->db, $this->access); + } + + $this->deletedUsers[$key] = $userObjects; + if(count($userObjects) > 0) { + $this->hasUsers(); + } + return $this->deletedUsers[$key]; + } + + /** + * returns all LDAP users that are marked as deleted + * @param int|null $offset + * @return OCA\user_ldap\lib\user\OfflineUser[] + */ + public function getUsers($offset = null) { + $key = $this->getDeletedUsersCacheKey($this->limit, $offset); + if(is_array($this->deletedUsers) && isset($this->deletedUsers[$key])) { + return $this->deletedUsers[$key]; + } + return $this->fetchDeletedUsers($offset); + } + + /** + * whether at least one user was detected as deleted + * @return bool + */ + public function hasUsers() { + if($this->deletedUsers === false) { + $this->fetchDeletedUsers(0); + } + foreach($this->deletedUsers as $batch) { + if(count($batch) > 0) { + return true; + } + } + return false; + } +} diff --git a/apps/user_ldap/lib/user/iusertools.php b/apps/user_ldap/lib/user/iusertools.php index bbc678153de..ffdef62410d 100644 --- a/apps/user_ldap/lib/user/iusertools.php +++ b/apps/user_ldap/lib/user/iusertools.php @@ -39,4 +39,7 @@ interface IUserTools { public function username2dn($name); + //temporary hack for LDAP user cleanup, will be removed in OC 8. + public function ocname2dn($name, $isUser); + } diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index 0ed3d09c48f..1bcc9b96d8a 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -27,6 +27,7 @@ use OCA\user_ldap\lib\user\IUserTools; use OCA\user_ldap\lib\user\User; use OCA\user_ldap\lib\LogWrapper; use OCA\user_ldap\lib\FilesystemHelper; +use OCA\user_ldap\lib\user\OfflineUser; /** * Manager @@ -60,7 +61,9 @@ class Manager { */ protected $avatarManager; /** - * @var string[][] + * array['byDN'] \OCA\user_ldap\lib\User[] + * ['byUid'] \OCA\user_ldap\lib\User[] + * @var array $users */ protected $users = array( 'byDN' => array(), @@ -130,10 +133,46 @@ class Manager { } } + /** + * Checks whether the specified user is marked as deleted + * @param string $id the ownCloud user name + * @return bool + */ + public function isDeletedUser($id) { + $isDeleted = $this->ocConfig->getUserValue( + $id, 'user_ldap', 'isDeleted', 0); + return intval($isDeleted) === 1; + } + + /** + * creates and returns an instance of OfflineUser for the specified user + * @param string $id + * @return \OCA\user_ldap\lib\user\OfflineUser + */ + public function getDeletedUser($id) { + return new OfflineUser( + $id, + new \OC\Preferences(\OC_DB::getConnection()), + \OC::$server->getDatabaseConnection(), + $this->access); + } + + protected function createInstancyByUserName($id) { + //most likely a uid. Check whether it is a deleted user + if($this->isDeletedUser($id)) { + return $this->getDeletedUser($id); + } + $dn = $this->access->username2dn($id); + if($dn !== false) { + return $this->createAndCache($dn, $id); + } + throw new \Exception('Could not create User instance'); + } + /** * @brief returns a User object by it's DN or ownCloud username * @param string the DN or username of the user - * @return \OCA\user_ldap\lib\User | null + * @return \OCA\user_ldap\lib\user\User|\OCA\user_ldap\lib\user\OfflineUser|null */ public function get($id) { $this->checkAccess(); @@ -143,25 +182,19 @@ class Manager { return $this->users['byUid'][$id]; } - if(!$this->access->stringResemblesDN($id) ) { - //most likely a uid - $dn = $this->access->username2dn($id); - if($dn !== false) { - return $this->createAndCache($dn, $id); - } - } else { - //so it's a DN + if($this->access->stringResemblesDN($id) ) { $uid = $this->access->dn2username($id); if($uid !== false) { return $this->createAndCache($id, $uid); } } - //either funny uid or invalid. Assume funny to be on the safe side. - $dn = $this->access->username2dn($id); - if($dn !== false) { - return $this->createAndCache($dn, $id); + + try { + $user = $this->createInstancyByUserName($id); + return $user; + } catch (\Exception $e) { + return null; } - return null; } } diff --git a/apps/user_ldap/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php new file mode 100644 index 00000000000..7750348a280 --- /dev/null +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -0,0 +1,217 @@ +. + * + */ + +namespace OCA\user_ldap\lib\user; + +use OCA\user_ldap\lib\Access; + +class OfflineUser { + /** + * @var string $ocName + */ + protected $ocName; + /** + * @var string $dn + */ + protected $dn; + /** + * @var string $uid the UID as provided by LDAP + */ + protected $uid; + /** + * @var string $displayName + */ + protected $displayName; + /** + * @var string $homePath + */ + protected $homePath; + /** + * @var string $lastLogin the timestamp of the last login + */ + protected $lastLogin; + /** + * @var string $email + */ + protected $email; + /** + * @var bool $hasActiveShares + */ + protected $hasActiveShares; + /** + * @var \OC\Preferences $preferences + */ + protected $preferences; + /** + * @var \OCP\IDBConnection $db + */ + protected $db; + /** + * @var \OCA\user_ldap\lib\Access + */ + protected $access; + + public function __construct($ocName, \OC\Preferences $preferences, \OCP\IDBConnection $db, Access $access) { + $this->ocName = $ocName; + $this->preferences = $preferences; + $this->db = $db; + $this->access = $access; + $this->fetchDetails(); + } + + /** + * exports the user details in an assoc array + * @return array + */ + public function export() { + $data = array(); + $data['ocName'] = $this->getOCName(); + $data['dn'] = $this->getDN(); + $data['uid'] = $this->getUID(); + $data['displayName'] = $this->getDisplayName(); + $data['homePath'] = $this->getHomePath(); + $data['lastLogin'] = $this->getLastLogin(); + $data['email'] = $this->getEmail(); + $data['hasActiveShares'] = $this->getHasActiveShares(); + + return $data; + } + + /** + * getter for ownCloud internal name + * @return string + */ + public function getOCName() { + return $this->ocName; + } + + /** + * getter for LDAP uid + * @return string + */ + public function getUID() { + return $this->uid; + } + + /** + * getter for LDAP DN + * @return string + */ + public function getDN() { + return $this->dn; + } + + /** + * getter for display name + * @return string + */ + public function getDisplayName() { + return $this->displayName; + } + + /** + * getter for email + * @return string + */ + public function getEmail() { + return $this->email; + } + + /** + * getter for home directory path + * @return string + */ + public function getHomePath() { + return $this->homePath; + } + + /** + * getter for the last login timestamp + * @return int + */ + public function getLastLogin() { + return intval($this->lastLogin); + } + + /** + * getter for having active shares + * @return bool + */ + public function getHasActiveShares() { + return $this->hasActiveShares; + } + + /** + * reads the user details + */ + protected function fetchDetails() { + $properties = array ( + 'displayName' => 'user_ldap', + 'uid' => 'user_ldap', + 'homePath' => 'user_ldap', + 'email' => 'settings', + 'lastLogin' => 'login' + ); + foreach($properties as $property => $app) { + $this->$property = $this->preferences->getValue($this->ocName, $app, $property, ''); + } + + $dn = $this->access->ocname2dn($this->ocName, true); + $this->dn = ($dn !== false) ? $dn : ''; + + $this->determineShares(); + } + + + /** + * finds out whether the user has active shares. The result is stored in + * $this->hasActiveShares + */ + protected function determineShares() { + $query = $this->db->prepare(' + SELECT COUNT(`uid_owner`) + FROM `*PREFIX*share` + WHERE `uid_owner` = ? + ', 1); + $query->execute(array($this->ocName)); + $sResult = $query->fetchColumn(0); + if(intval($sResult) === 1) { + $this->hasActiveShares = true; + return; + } + + $query = $this->db->prepare(' + SELECT COUNT(`owner`) + FROM `*PREFIX*share_external` + WHERE `owner` = ? + ', 1); + $query->execute(array($this->ocName)); + $sResult = $query->fetchColumn(0); + if(intval($sResult) === 1) { + $this->hasActiveShares = true; + return; + } + + $this->hasActiveShares = false; + } +} diff --git a/apps/user_ldap/lib/user/user.php b/apps/user_ldap/lib/user/user.php index d4d2294307d..c81fb25b541 100644 --- a/apps/user_ldap/lib/user/user.php +++ b/apps/user_ldap/lib/user/user.php @@ -212,6 +212,31 @@ class User { return true; } + /** + * Stores a key-value pair in relation to this user + * @param string $key + * @param string $value + */ + private function store($key, $value) { + $this->config->setUserValue($this->uid, 'user_ldap', $key, $value); + } + + /** + * Stores the display name in the databae + * @param string $displayName + */ + public function storeDisplayName($displayName) { + $this->store('displayName', $displayName); + } + + /** + * Stores the LDAP Username in the Database + * @param string $userName + */ + public function storeLDAPUserName($userName) { + $this->store('uid', $userName); + } + /** * @brief checks whether an update method specified by feature was run * already. If not, it will marked like this, because it is expected that diff --git a/apps/user_ldap/lib/wizard.php b/apps/user_ldap/lib/wizard.php index 578a920f00e..2e4507a2585 100644 --- a/apps/user_ldap/lib/wizard.php +++ b/apps/user_ldap/lib/wizard.php @@ -659,7 +659,8 @@ class Wizard extends LDAPUtility { //this did not help :( //Let's see whether we can parse the Host URL and convert the domain to //a base DN - $domain = Helper::getDomainFromURL($this->configuration->ldapHost); + $helper = new Helper(); + $domain = $helper->getDomainFromURL($this->configuration->ldapHost); if(!$domain) { return false; } diff --git a/apps/user_ldap/settings.php b/apps/user_ldap/settings.php index 5527cf2c6da..a19ec0bda6f 100644 --- a/apps/user_ldap/settings.php +++ b/apps/user_ldap/settings.php @@ -35,8 +35,9 @@ OCP\Util::addStyle('user_ldap', 'settings'); // fill template $tmpl = new OCP\Template('user_ldap', 'settings'); -$prefixes = \OCA\user_ldap\lib\Helper::getServerConfigurationPrefixes(); -$hosts = \OCA\user_ldap\lib\Helper::getServerConfigurationHosts(); +$helper = new \OCA\user_ldap\lib\Helper(); +$prefixes = $helper->getServerConfigurationPrefixes(); +$hosts = $helper->getServerConfigurationHosts(); $wizardHtml = ''; $toc = array(); diff --git a/apps/user_ldap/tests/jobs/cleanup.php b/apps/user_ldap/tests/jobs/cleanup.php new file mode 100644 index 00000000000..3aa9a4a43c5 --- /dev/null +++ b/apps/user_ldap/tests/jobs/cleanup.php @@ -0,0 +1,155 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OCA\user_ldap\tests; + +class Test_CleanUp extends \PHPUnit_Framework_TestCase { + public function getMocks() { + $mocks = array(); + $mocks['userBackend'] = + $this->getMockBuilder('\OCA\user_ldap\User_Proxy') + ->disableOriginalConstructor() + ->getMock(); + $mocks['ocConfig'] = $this->getMock('\OCP\IConfig'); + $mocks['db'] = $this->getMock('\OCP\IDBConnection'); + $mocks['helper'] = $this->getMock('\OCA\user_ldap\lib\Helper'); + + return $mocks; + } + + /** + * clean up job must not run when there are disabled configurations + */ + public function test_runNotAllowedByDisabledConfigurations() { + $args = $this->getMocks(); + $args['helper']->expects($this->once()) + ->method('haveDisabledConfigurations') + ->will($this->returnValue(true) ); + + $args['ocConfig']->expects($this->never()) + ->method('getSystemValue'); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isCleanUpAllowed(); + $this->assertSame(false, $result); + } + + /** + * clean up job must not run when LDAP Helper is broken i.e. + * returning unexpected results + */ + public function test_runNotAllowedByBrokenHelper() { + $args = $this->getMocks(); + $args['helper']->expects($this->once()) + ->method('haveDisabledConfigurations') + ->will($this->throwException(new \Exception())); + + $args['ocConfig']->expects($this->never()) + ->method('getSystemValue'); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isCleanUpAllowed(); + $this->assertSame(false, $result); + } + + /** + * clean up job must not run when it is not enabled + */ + public function test_runNotAllowedBySysConfig() { + $args = $this->getMocks(); + $args['helper']->expects($this->once()) + ->method('haveDisabledConfigurations') + ->will($this->returnValue(false)); + + $args['ocConfig']->expects($this->once()) + ->method('getSystemValue') + ->will($this->returnValue(false)); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isCleanUpAllowed(); + $this->assertSame(false, $result); + } + + /** + * clean up job is allowed to run + */ + public function test_runIsAllowed() { + $args = $this->getMocks(); + $args['helper']->expects($this->once()) + ->method('haveDisabledConfigurations') + ->will($this->returnValue(false)); + + $args['ocConfig']->expects($this->once()) + ->method('getSystemValue') + ->will($this->returnValue(true)); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isCleanUpAllowed(); + $this->assertSame(true, $result); + } + + /** + * test whether sql is OK + */ + public function test_getMappedUsers() { + $args = $this->getMocks(); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + if(version_compare(\PHPUnit_Runner_Version::id(), '3.8', '<')) { + //otherwise we run into + //https://github.com/sebastianbergmann/phpunit-mock-objects/issues/103 + $this->markTestIncomplete(); + } + + $stmt = $this->getMock('\Doctrine\DBAL\Driver\Statement'); + + $args['db']->expects($this->once()) + ->method('prepare') + ->will($this->returnValue($stmt)); + + $bgJob->getMappedUsers(0, $bgJob->getChunkSize()); + } + + /** + * check whether offset will be reset when it needs to + */ + public function test_OffsetResetIsNecessary() { + $args = $this->getMocks(); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize() - 1); + $this->assertSame(true, $result); + } + + /** + * make sure offset is not reset when it is not due + */ + public function test_OffsetResetIsNotNecessary() { + $args = $this->getMocks(); + + $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); + $bgJob->setArguments($args); + + $result = $bgJob->isOffsetResetNecessary($bgJob->getChunkSize()); + $this->assertSame(false, $result); + } + +} + diff --git a/apps/user_ldap/tests/user/manager.php b/apps/user_ldap/tests/user/manager.php index b3e52084dba..fb47f60539f 100644 --- a/apps/user_ldap/tests/user/manager.php +++ b/apps/user_ldap/tests/user/manager.php @@ -183,7 +183,7 @@ class Test_User_Manager extends \Test\TestCase { $access->expects($this->never()) ->method('dn2username'); - $access->expects($this->exactly(2)) + $access->expects($this->exactly(1)) ->method('username2dn') ->with($this->equalTo($uid)) ->will($this->returnValue(false)); diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 33cec0247b6..876b3d0903a 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -123,7 +123,7 @@ class Test_User_Ldap_Direct extends \Test\TestCase { ->method('fetchListOfUsers') ->will($this->returnCallback(function($filter) { if($filter === 'roland') { - return array('dnOfRoland,dc=test'); + return array(array('dn' => 'dnOfRoland,dc=test')); } return array(); })); @@ -230,6 +230,24 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $this->assertFalse($result); } + public function testDeleteUserCancel() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + $result = $backend->deleteUser('notme'); + $this->assertFalse($result); + } + + public function testDeleteUserSuccess() { + $access = $this->getAccessMock(); + $backend = new UserLDAP($access); + + $pref = \OC::$server->getConfig(); + $pref->setUserValue('jeremy', 'user_ldap', 'isDeleted', 1); + + $result = $backend->deleteUser('jeremy'); + $this->assertTrue($result); + } + /** * Prepares the Access mock for getUsers tests * @param \OCA\user_ldap\lib\Access $access mock diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 482715b3686..2274e4156cc 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -26,8 +26,15 @@ namespace OCA\user_ldap; use OCA\user_ldap\lib\BackendUtility; +use OCA\user_ldap\lib\user\OfflineUser; +use OCA\User_LDAP\lib\User\User; class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserInterface { + /** + * @var string[] $homesToKill + */ + protected $homesToKill = array(); + /** * checks whether the user is allowed to change his avatar in ownCloud * @param string $uid the ownCloud user name @@ -35,7 +42,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn */ public function canChangeAvatar($uid) { $user = $this->access->userManager->get($uid); - if(is_null($user)) { + if(!$user instanceof User) { return false; } if($user->getAvatarImage() === false) { @@ -57,15 +64,17 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $uid = $this->access->escapeFilterPart($uid); //find out dn of the user name + $attrs = array($this->access->connection->ldapUserDisplayName, 'dn', + 'uid', 'samaccountname'); $filter = \OCP\Util::mb_str_replace( '%uid', $uid, $this->access->connection->ldapLoginFilter, 'UTF-8'); - $ldap_users = $this->access->fetchListOfUsers($filter, 'dn'); - if(count($ldap_users) < 1) { + $users = $this->access->fetchListOfUsers($filter, $attrs); + if(count($users) < 1) { return false; } - $dn = $ldap_users[0]; + $dn = $users[0]['dn']; $user = $this->access->userManager->get($dn); - if(is_null($user)) { + if(!$user instanceof User) { \OCP\Util::writeLog('user_ldap', 'LDAP Login: Could not get user object for DN ' . $dn . '. Maybe the LDAP entry has no set display name attribute?', @@ -79,6 +88,15 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn } $user->markLogin(); + if(isset($users[0][$this->access->connection->ldapUserDisplayName])) { + $dpn = $users[0][$this->access->connection->ldapUserDisplayName]; + $user->storeDisplayName($dpn); + } + if(isset($users[0]['uid'])) { + $user->storeLDAPUserName($users[0]['uid']); + } else if(isset($users[0]['samaccountname'])) { + $user->storeLDAPUserName($users[0]['samaccountname']); + } return $user->getUsername(); } @@ -127,6 +145,33 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn return $ldap_users; } + /** + * checks whether a user is still available on LDAP + * @param string|\OCA\User_LDAP\lib\user\User $user either the ownCloud user + * name or an instance of that user + * @return bool + */ + public function userExistsOnLDAP($user) { + if(is_string($user)) { + $user = $this->access->userManager->get($user); + } + if(!$user instanceof User) { + return false; + } + + $dn = $user->getDN(); + //check if user really still exists by reading its entry + if(!is_array($this->access->readAttribute($dn, ''))) { + $lcr = $this->access->connection->getConnectionResource(); + if(is_null($lcr)) { + throw new \Exception('No LDAP Connection to server ' . $this->access->connection->ldapHost); + } + return false; + } + + return true; + } + /** * check if a user exists * @param string $uid the username @@ -143,36 +188,56 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn $this->access->connection->ldapHost, \OCP\Util::DEBUG); $this->access->connection->writeToCache('userExists'.$uid, false); return false; + } else if($user instanceof OfflineUser) { + //express check for users marked as deleted. Returning true is + //necessary for cleanup + return true; } - $dn = $user->getDN(); - //check if user really still exists by reading its entry - if(!is_array($this->access->readAttribute($dn, ''))) { - \OCP\Util::writeLog('user_ldap', 'LDAP says no user '.$dn.' on '. - $this->access->connection->ldapHost, \OCP\Util::DEBUG); - $this->access->connection->writeToCache('userExists'.$uid, false); + + try { + $result = $this->userExistsOnLDAP($user); + $this->access->connection->writeToCache('userExists'.$uid, $result); + if($result === true) { + $user->update(); + } + return $result; + } catch (\Exception $e) { + \OCP\Util::writeLog('user_ldap', $e->getMessage(), \OCP\Util::WARN); return false; } - - $this->access->connection->writeToCache('userExists'.$uid, true); - $user->update(); - return true; } /** - * delete a user + * returns whether a user was deleted in LDAP + * * @param string $uid The username of the user to delete * @return bool - * - * Deletes a user */ public function deleteUser($uid) { - return false; + $pref = \OC::$server->getConfig(); + $marked = $pref->getUserValue($uid, 'user_ldap', 'isDeleted', 0); + if(intval($marked) === 0) { + \OC::$server->getLogger()->notice( + 'User '.$uid . ' is not marked as deleted, not cleaning up.', + array('app' => 'user_ldap')); + return false; + } + \OC::$server->getLogger()->info('Cleaning up after user ' . $uid, + array('app' => 'user_ldap')); + + //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', ''); + $this->homesToKill[$uid] = $home; + $this->access->unmapUser($uid); + + return true; } /** * get the user's home directory * @param string $uid the username - * @return boolean + * @return string|bool */ public function getHome($uid) { // user Exists check required as it is not done in user proxy! @@ -180,10 +245,16 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn return false; } + if(isset($this->homesToKill[$uid]) && !empty($this->homesToKill[$uid])) { + //a deleted user who needs some clean up + return $this->homesToKill[$uid]; + } + $cacheKey = 'getHome'.$uid; 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( @@ -203,12 +274,17 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn \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); + //TODO: if home directory changes, the old one needs to be removed. return $homedir; } } //false will apply default behaviour as defined and done by OC_User $this->access->connection->writeToCache($cacheKey, false); + $pref->setUserValue($uid, 'user_ldap', 'homePath', ''); return false; } diff --git a/apps/user_ldap/user_proxy.php b/apps/user_ldap/user_proxy.php index 6414a048071..77caa84ecd9 100644 --- a/apps/user_ldap/user_proxy.php +++ b/apps/user_ldap/user_proxy.php @@ -24,6 +24,7 @@ namespace OCA\user_ldap; use OCA\user_ldap\lib\ILDAPWrapper; +use OCA\User_LDAP\lib\User\User; class User_Proxy extends lib\Proxy implements \OCP\IUserBackend, \OCP\UserInterface { private $backends = array(); @@ -152,6 +153,17 @@ class User_Proxy extends lib\Proxy implements \OCP\IUserBackend, \OCP\UserInterf return $this->handleRequest($uid, 'userExists', array($uid)); } + /** + * check if a user exists on LDAP + * @param string|OCA\User_LDAP\lib\User\User $user either the ownCloud user + * name or an instance of that user + * @return boolean + */ + public function userExistsOnLDAP($user) { + $id = ($user instanceof User) ? $user->getUsername() : $user; + return $this->handleRequest($id, 'userExistsOnLDAP', array($user)); + } + /** * Check if the password is correct * @param string $uid The username @@ -217,7 +229,7 @@ class User_Proxy extends lib\Proxy implements \OCP\IUserBackend, \OCP\UserInterf * Deletes a user */ public function deleteUser($uid) { - return false; + return $this->handleRequest($uid, 'deleteUser', array($uid)); } /** diff --git a/config/config.sample.php b/config/config.sample.php index 35e3f6ce5f1..e5b8344ad37 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -80,7 +80,7 @@ $CONFIG = array( /** * Where user files are stored; this defaults to ``data/`` in the ownCloud - * directory. The SQLite database is also stored here, when you use SQLite. (SQLite is + * directory. The SQLite database is also stored here, when you use SQLite. (SQLite is * available only in ownCloud Community Edition) */ 'datadirectory' => '/var/www/owncloud/data', @@ -665,6 +665,20 @@ $CONFIG = array( 'OC\Preview\MarkDown' ), +/** + * LDAP + * + * Global settings used by LDAP User and Group Backend + */ + +/** + * defines the interval in minutes for the background job that checks user + * existance and marks them as ready to be cleaned up. The number is always + * minutes. Setting it to 0 disables the feature. + * See command line (occ) methods ldap:show-remnants and user:delete + */ +'ldapUserCleanupInterval' => 51, + /** * Maintenance diff --git a/core/command/user/delete.php b/core/command/user/delete.php new file mode 100644 index 00000000000..f64b40e4921 --- /dev/null +++ b/core/command/user/delete.php @@ -0,0 +1,36 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\Core\Command\User; + +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Input\InputArgument; + +class Delete extends Command { + protected function configure() { + $this + ->setName('user:delete') + ->setDescription('deletes the specified user') + ->addArgument( + 'uid', + InputArgument::REQUIRED, + 'the username' + ); + } + + protected function execute(InputInterface $input, OutputInterface $output) { + $wasSuccessful = \OC_User::deleteUser($input->getArgument('uid')); + if($wasSuccessful === true) { + $output->writeln('The specified user was deleted'); + return; + } + $output->writeln('The specified could not be deleted. Please check the logs.'); + } +} diff --git a/core/register_command.php b/core/register_command.php index 8f79473ced8..690e9879c47 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -22,5 +22,6 @@ $application->add(new OC\Core\Command\Maintenance\Repair($repair, \OC::$server-> $application->add(new OC\Core\Command\User\Report()); $application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\LastSeen()); +$application->add(new OC\Core\Command\User\Delete()); $application->add(new OC\Core\Command\L10n\CreateJs()); diff --git a/lib/private/preferences.php b/lib/private/preferences.php index cd4a9fd1c19..1784d372261 100644 --- a/lib/private/preferences.php +++ b/lib/private/preferences.php @@ -137,10 +137,12 @@ class Preferences { * @param string $app * @param string $key * @param string $value + * @param int|null $limit + * @param int|null $offset * @return array * @deprecated use getUsersForUserValue of \OCP\IConfig instead */ - public function getUsersForValue($app, $key, $value) { + public function getUsersForValue($app, $key, $value, $limit = null, $offset = null) { return $this->config->getUsersForUserValue($app, $key, $value); } -- cgit v1.2.3 From 144d95de7dde29cd85e795cdcd7ac1576639d641 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 20 Dec 2014 16:09:04 +0100 Subject: basic adjustments for OC 8. I.e. no visible issues, LDAP tests pass. --- apps/user_ldap/appinfo/register_command.php | 10 +++++---- apps/user_ldap/command/showremnants.php | 31 ++++++++++++--------------- apps/user_ldap/lib/access.php | 28 ++++++++++++++++++++++-- apps/user_ldap/lib/jobs.php | 3 +++ apps/user_ldap/lib/user/deletedusersindex.php | 12 +++++------ apps/user_ldap/lib/user/iusertools.php | 4 ---- apps/user_ldap/lib/user/manager.php | 2 +- apps/user_ldap/lib/user/offlineuser.php | 12 +++++------ apps/user_ldap/tests/user_ldap.php | 10 +++++++++ apps/user_ldap/user_ldap.php | 2 +- 10 files changed, 73 insertions(+), 41 deletions(-) diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index 55a187d9654..314c73e6c4a 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -9,6 +9,7 @@ use OCA\user_ldap\lib\Helper; use OCA\user_ldap\lib\LDAP; use OCA\user_ldap\User_Proxy; +use OCA\User_LDAP\Mapping\UserMapping; $application->add(new OCA\user_ldap\Command\ShowConfig()); $application->add(new OCA\user_ldap\Command\SetConfig()); @@ -16,11 +17,12 @@ $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\ShowRemnants()); -$helper = new OCA\user_ldap\lib\Helper(); -$uBackend = new OCA\user_ldap\User_Proxy( +$userMapping = new UserMapping(\OC::$server->getDatabaseConnection()); +$application->add(new OCA\user_ldap\Command\ShowRemnants($userMapping)); +$helper = new Helper(); +$uBackend = new User_Proxy( $helper->getServerConfigurationPrefixes(true), - new OCA\user_ldap\lib\LDAP() + new LDAP() ); $application->add(new OCA\user_ldap\Command\CheckUser( $uBackend, $helper, \OC::$server->getConfig() diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php index 3d39f977421..fb9fc54ff6a 100644 --- a/apps/user_ldap/command/showremnants.php +++ b/apps/user_ldap/command/showremnants.php @@ -16,9 +16,21 @@ use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\DeletedUsersIndex; use OCA\User_LDAP\lib\Connection; -use OCA\User_LDAP\lib\Access; +use OCA\User_LDAP\Mapping\UserMapping; class ShowRemnants extends Command { + /** @var OCA\User_LDAP\Mapping\UserMapping */ + protected $mapping; + + /** + * @param OCA\user_ldap\User_Proxy $uBackend + * @param OCA\User_LDAP\lib\Helper $helper + * @param OCP\IConfig $config + */ + public function __construct(UserMapping $mapper) { + $this->mapper = $mapper; + parent::__construct(); + } protected function configure() { $this @@ -31,7 +43,7 @@ class ShowRemnants extends Command { $dui = new DeletedUsersIndex( new \OC\Preferences(\OC_DB::getConnection()), \OC::$server->getDatabaseConnection(), - $this->getAccess() + $this->mapper ); /** @var \Symfony\Component\Console\Helper\Table $table */ @@ -63,19 +75,4 @@ class ShowRemnants extends Command { $table->setRows($rows); $table->render($output); } - - protected function getAccess() { - $ldap = new \OCA\user_ldap\lib\LDAP(); - $dummyConnection = new Connection($ldap, '', null); - $userManager = new \OCA\user_ldap\lib\user\Manager( - \OC::$server->getConfig(), - new \OCA\user_ldap\lib\FilesystemHelper(), - new \OCA\user_ldap\lib\LogWrapper(), - \OC::$server->getAvatarManager(), - new \OCP\Image() - ); - $access = new Access($dummyConnection, $ldap, $userManager); - return $access; - } - } diff --git a/apps/user_ldap/lib/access.php b/apps/user_ldap/lib/access.php index 692afb98f99..3e9869b4d71 100644 --- a/apps/user_ldap/lib/access.php +++ b/apps/user_ldap/lib/access.php @@ -75,6 +75,18 @@ class Access extends LDAPUtility implements user\IUserTools { $this->userMapper = $mapper; } + /** + * returns the User Mapper + * @throws \Exception + * @return AbstractMapping + */ + public function getUserMapper() { + if(is_null($this->userMapper)) { + throw new \Exception('UserMapper was not assigned to this Access instance.'); + } + return $this->userMapper; + } + /** * sets the Group Mapper * @param AbstractMapping $mapper @@ -83,6 +95,18 @@ class Access extends LDAPUtility implements user\IUserTools { $this->groupMapper = $mapper; } + /** + * returns the Group Mapper + * @throws \Exception + * @return AbstractMapping + */ + public function getGroupMapper() { + if(is_null($this->groupMapper)) { + throw new \Exception('GroupMapper was not assigned to this Access instance.'); + } + return $this->groupMapper; + } + /** * @return bool */ @@ -333,10 +357,10 @@ class Access extends LDAPUtility implements user\IUserTools { */ public function dn2ocname($fdn, $ldapName = null, $isUser = true) { if($isUser) { - $mapper = $this->userMapper; + $mapper = $this->getUserMapper(); $nameAttribute = $this->connection->ldapUserDisplayName; } else { - $mapper = $this->groupMapper; + $mapper = $this->getGroupMapper(); $nameAttribute = $this->connection->ldapGroupDisplayName; } diff --git a/apps/user_ldap/lib/jobs.php b/apps/user_ldap/lib/jobs.php index 30f09cdc8f8..391a10d31f8 100644 --- a/apps/user_ldap/lib/jobs.php +++ b/apps/user_ldap/lib/jobs.php @@ -23,6 +23,8 @@ namespace OCA\user_ldap\lib; +use OCA\User_LDAP\Mapping\GroupMapping; + class Jobs extends \OC\BackgroundJob\TimedJob { static private $groupsFromDB; @@ -169,6 +171,7 @@ class Jobs extends \OC\BackgroundJob\TimedJob { new \OCP\Image()); $connector = new Connection($ldapWrapper, $configPrefixes[0]); $ldapAccess = new Access($connector, $ldapWrapper, $userManager); + $groupMapper = new GroupMapping(\OC::$server->getDatabaseConnection()); self::$groupBE = new \OCA\user_ldap\GROUP_LDAP($ldapAccess); } else { self::$groupBE = new \OCA\user_ldap\Group_Proxy($configPrefixes, $ldapWrapper); diff --git a/apps/user_ldap/lib/user/deletedusersindex.php b/apps/user_ldap/lib/user/deletedusersindex.php index 0d8bacffe94..e544d29bad5 100644 --- a/apps/user_ldap/lib/user/deletedusersindex.php +++ b/apps/user_ldap/lib/user/deletedusersindex.php @@ -24,7 +24,7 @@ namespace OCA\user_ldap\lib\user; use OCA\user_ldap\lib\user\OfflineUser; -use OCA\user_ldap\lib\Access; +use OCA\User_LDAP\Mapping\UserMapping; /** * Class DeletedUsersIndex @@ -42,9 +42,9 @@ class DeletedUsersIndex { protected $db; /** - * @var \OCA\user_ldap\lib\Access $access + * @var \OCA\User_LDAP\Mapping\UserMapping $mapping */ - protected $access; + protected $mapping; /** * @var int $limit @@ -56,10 +56,10 @@ class DeletedUsersIndex { */ protected $deletedUsers = false; - public function __construct(\OC\Preferences $preferences, \OCP\IDBConnection $db, Access $access) { + public function __construct(\OC\Preferences $preferences, \OCP\IDBConnection $db, UserMapping $mapping) { $this->preferences = $preferences; $this->db = $db; - $this->access = $access; + $this->mapping = $mapping; } /** @@ -84,7 +84,7 @@ class DeletedUsersIndex { $userObjects = array(); foreach($deletedUsers as $user) { - $userObjects[] = new OfflineUser($user, $this->preferences, $this->db, $this->access); + $userObjects[] = new OfflineUser($user, $this->preferences, $this->db, $this->mapping); } $this->deletedUsers[$key] = $userObjects; diff --git a/apps/user_ldap/lib/user/iusertools.php b/apps/user_ldap/lib/user/iusertools.php index ffdef62410d..fcb00d2f746 100644 --- a/apps/user_ldap/lib/user/iusertools.php +++ b/apps/user_ldap/lib/user/iusertools.php @@ -38,8 +38,4 @@ interface IUserTools { public function dn2username($dn, $ldapname = null); public function username2dn($name); - - //temporary hack for LDAP user cleanup, will be removed in OC 8. - public function ocname2dn($name, $isUser); - } diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index 1bcc9b96d8a..cd4f4441e1d 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -154,7 +154,7 @@ class Manager { $id, new \OC\Preferences(\OC_DB::getConnection()), \OC::$server->getDatabaseConnection(), - $this->access); + $this->access->getUserMapper()); } protected function createInstancyByUserName($id) { diff --git a/apps/user_ldap/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php index 7750348a280..7cf48bc05b1 100644 --- a/apps/user_ldap/lib/user/offlineuser.php +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -23,7 +23,7 @@ namespace OCA\user_ldap\lib\user; -use OCA\user_ldap\lib\Access; +use OCA\User_LDAP\Mapping\UserMapping; class OfflineUser { /** @@ -67,15 +67,15 @@ class OfflineUser { */ protected $db; /** - * @var \OCA\user_ldap\lib\Access + * @var OCA\User_LDAP\Mapping\UserMapping */ - protected $access; + protected $mapping; - public function __construct($ocName, \OC\Preferences $preferences, \OCP\IDBConnection $db, Access $access) { + public function __construct($ocName, \OC\Preferences $preferences, \OCP\IDBConnection $db, UserMapping $mapping) { $this->ocName = $ocName; $this->preferences = $preferences; $this->db = $db; - $this->access = $access; + $this->mapping = $mapping; $this->fetchDetails(); } @@ -176,7 +176,7 @@ class OfflineUser { $this->$property = $this->preferences->getValue($this->ocName, $app, $property, ''); } - $dn = $this->access->ocname2dn($this->ocName, true); + $dn = $this->mapping->getDNByName($this->ocName); $this->dn = ($dn !== false) ? $dn : ''; $this->determineShares(); diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index 876b3d0903a..fdda35b63c9 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -239,6 +239,16 @@ class Test_User_Ldap_Direct extends \Test\TestCase { public function testDeleteUserSuccess() { $access = $this->getAccessMock(); + $mapping = $this->getMockBuilder('\OCA\User_LDAP\Mapping\UserMapping') + ->disableOriginalConstructor() + ->getMock(); + $mapping->expects($this->once()) + ->method('unmap') + ->will($this->returnValue(true)); + $access->expects($this->once()) + ->method('getUserMapper') + ->will($this->returnValue($mapping)); + $backend = new UserLDAP($access); $pref = \OC::$server->getConfig(); diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index 2274e4156cc..c2d0f387b9c 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -229,7 +229,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn //necessary for removing directories as done by OC_User. $home = $pref->getUserValue($uid, 'user_ldap', 'homePath', ''); $this->homesToKill[$uid] = $home; - $this->access->unmapUser($uid); + $this->access->getUserMapper()->unmap($uid); return true; } -- cgit v1.2.3 From 3ca70d647a36144e64cbe4b90ffa97b3d9b64470 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 20 Dec 2014 16:33:37 +0100 Subject: move from \OC\Preferences to \OCP\IConfig --- apps/user_ldap/command/showremnants.php | 36 ++++++++--------- apps/user_ldap/lib/user/deletedusersindex.php | 57 ++++++++------------------- apps/user_ldap/lib/user/manager.php | 2 +- apps/user_ldap/lib/user/offlineuser.php | 10 ++--- 4 files changed, 38 insertions(+), 67 deletions(-) diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php index fb9fc54ff6a..8144a54cbee 100644 --- a/apps/user_ldap/command/showremnants.php +++ b/apps/user_ldap/command/showremnants.php @@ -41,7 +41,7 @@ class ShowRemnants extends Command { protected function execute(InputInterface $input, OutputInterface $output) { $dui = new DeletedUsersIndex( - new \OC\Preferences(\OC_DB::getConnection()), + \OC::$server->getConfig(), \OC::$server->getDatabaseConnection(), $this->mapper ); @@ -52,25 +52,21 @@ class ShowRemnants extends Command { 'ownCloud name', 'Display Name', 'LDAP UID', 'LDAP DN', 'Last Login', 'Dir', 'Sharer')); $rows = array(); - $offset = 0; - do { - $resultSet = $dui->getUsers($offset); - $offset += count($resultSet); - foreach($resultSet as $user) { - $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; - $lastLogin = ($user->getLastLogin() > 0) ? - \OCP\Util::formatDate($user->getLastLogin()) : '-'; - $rows[] = array( - $user->getOCName(), - $user->getDisplayName(), - $user->getUid(), - $user->getDN(), - $lastLogin, - $user->getHomePath(), - $hAS - ); - } - } while (count($resultSet) === 10); + $resultSet = $dui->getUsers(); + foreach($resultSet as $user) { + $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; + $lastLogin = ($user->getLastLogin() > 0) ? + \OCP\Util::formatDate($user->getLastLogin()) : '-'; + $rows[] = array( + $user->getOCName(), + $user->getDisplayName(), + $user->getUid(), + $user->getDN(), + $lastLogin, + $user->getHomePath(), + $hAS + ); + } $table->setRows($rows); $table->render($output); diff --git a/apps/user_ldap/lib/user/deletedusersindex.php b/apps/user_ldap/lib/user/deletedusersindex.php index e544d29bad5..62abe2e10de 100644 --- a/apps/user_ldap/lib/user/deletedusersindex.php +++ b/apps/user_ldap/lib/user/deletedusersindex.php @@ -32,9 +32,9 @@ use OCA\User_LDAP\Mapping\UserMapping; */ class DeletedUsersIndex { /** - * @var \OC\Preferences $preferences + * @var \OCP\IConfig $config */ - protected $preferences; + protected $config; /** * @var \OCP\IDBConnection $db @@ -46,65 +46,42 @@ class DeletedUsersIndex { */ protected $mapping; - /** - * @var int $limit - */ - protected $limit = 10; - /** * @var array $deletedUsers */ protected $deletedUsers = false; - public function __construct(\OC\Preferences $preferences, \OCP\IDBConnection $db, UserMapping $mapping) { - $this->preferences = $preferences; + public function __construct(\OCP\IConfig $config, \OCP\IDBConnection $db, UserMapping $mapping) { + $this->config = $config; $this->db = $db; $this->mapping = $mapping; } - /** - * returns key to be used against $this->deletedUsers - * @param int $limit - * @param int $offset - * @return string - */ - private function getDeletedUsersCacheKey($limit, $offset) { - return strval($limit) . '.' . strval($offset); - } - /** * reads LDAP users marked as deleted from the database - * @param int $offset * @return OCA\user_ldap\lib\user\OfflineUser[] */ - private function fetchDeletedUsers($offset) { - $deletedUsers = $this->preferences->getUsersForValue( - 'user_ldap', 'isDeleted', '1', $this->limit, $offset); - $key = $this->getDeletedUsersCacheKey($this->limit, $offset); + private function fetchDeletedUsers() { + $deletedUsers = $this->config->getUsersForUserValue( + 'user_ldap', 'isDeleted', '1'); $userObjects = array(); foreach($deletedUsers as $user) { - $userObjects[] = new OfflineUser($user, $this->preferences, $this->db, $this->mapping); + $userObjects[] = new OfflineUser($user, $this->config, $this->db, $this->mapping); } - $this->deletedUsers[$key] = $userObjects; - if(count($userObjects) > 0) { - $this->hasUsers(); - } - return $this->deletedUsers[$key]; + return $this->deletedUsers; } /** * returns all LDAP users that are marked as deleted - * @param int|null $offset * @return OCA\user_ldap\lib\user\OfflineUser[] */ - public function getUsers($offset = null) { - $key = $this->getDeletedUsersCacheKey($this->limit, $offset); - if(is_array($this->deletedUsers) && isset($this->deletedUsers[$key])) { - return $this->deletedUsers[$key]; + public function getUsers() { + if(is_array($this->deletedUsers)) { + return $this->deletedUsers; } - return $this->fetchDeletedUsers($offset); + return $this->fetchDeletedUsers(); } /** @@ -113,12 +90,10 @@ class DeletedUsersIndex { */ public function hasUsers() { if($this->deletedUsers === false) { - $this->fetchDeletedUsers(0); + $this->fetchDeletedUsers(); } - foreach($this->deletedUsers as $batch) { - if(count($batch) > 0) { - return true; - } + if(is_array($this->deletedUsers) && count($this->deletedUsers) > 0) { + return true; } return false; } diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index cd4f4441e1d..431609071e6 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -152,7 +152,7 @@ class Manager { public function getDeletedUser($id) { return new OfflineUser( $id, - new \OC\Preferences(\OC_DB::getConnection()), + $this->ocConfig, \OC::$server->getDatabaseConnection(), $this->access->getUserMapper()); } diff --git a/apps/user_ldap/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php index 7cf48bc05b1..9383320fae2 100644 --- a/apps/user_ldap/lib/user/offlineuser.php +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -59,9 +59,9 @@ class OfflineUser { */ protected $hasActiveShares; /** - * @var \OC\Preferences $preferences + * @var \OCP\IConfig $config */ - protected $preferences; + protected $config; /** * @var \OCP\IDBConnection $db */ @@ -71,9 +71,9 @@ class OfflineUser { */ protected $mapping; - public function __construct($ocName, \OC\Preferences $preferences, \OCP\IDBConnection $db, UserMapping $mapping) { + public function __construct($ocName, \OCP\IConfig $config, \OCP\IDBConnection $db, UserMapping $mapping) { $this->ocName = $ocName; - $this->preferences = $preferences; + $this->config = $config; $this->db = $db; $this->mapping = $mapping; $this->fetchDetails(); @@ -173,7 +173,7 @@ class OfflineUser { 'lastLogin' => 'login' ); foreach($properties as $property => $app) { - $this->$property = $this->preferences->getValue($this->ocName, $app, $property, ''); + $this->$property = $this->config->getUserValue($this->ocName, $app, $property, ''); } $dn = $this->mapping->getDNByName($this->ocName); -- cgit v1.2.3 From 61ed363f820a3b25b68289ed2c03ff5e5edfed91 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Sat, 20 Dec 2014 17:08:26 +0100 Subject: planned refactorings for OC 8 --- apps/user_ldap/appinfo/register_command.php | 20 ++++++----- apps/user_ldap/command/checkuser.php | 35 +++++++----------- apps/user_ldap/lib/jobs/cleanup.php | 49 +++++++++++++------------- apps/user_ldap/lib/mapping/abstractmapping.php | 21 +++++++++++ apps/user_ldap/lib/user/deletedusersindex.php | 9 +++++ apps/user_ldap/tests/jobs/cleanup.php | 4 +++ 6 files changed, 83 insertions(+), 55 deletions(-) diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index 314c73e6c4a..e0013c4eb7a 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -10,6 +10,17 @@ use OCA\user_ldap\lib\Helper; use OCA\user_ldap\lib\LDAP; use OCA\user_ldap\User_Proxy; use OCA\User_LDAP\Mapping\UserMapping; +use OCA\User_LDAP\lib\User\DeletedUsersIndex; + +$dbConnection = \OC::$server->getDatabaseConnection(); +$userMapping = new UserMapping($dbConnection); +$helper = new Helper(); +$uBackend = new User_Proxy( + $helper->getServerConfigurationPrefixes(true), + new LDAP() +); +$deletedUsersIndex = new DeletedUsersIndex( + \OC::$server->getConfig(), $dbConnection, $userMapping); $application->add(new OCA\user_ldap\Command\ShowConfig()); $application->add(new OCA\user_ldap\Command\SetConfig()); @@ -17,13 +28,6 @@ $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()); -$userMapping = new UserMapping(\OC::$server->getDatabaseConnection()); $application->add(new OCA\user_ldap\Command\ShowRemnants($userMapping)); -$helper = new Helper(); -$uBackend = new User_Proxy( - $helper->getServerConfigurationPrefixes(true), - new LDAP() -); $application->add(new OCA\user_ldap\Command\CheckUser( - $uBackend, $helper, \OC::$server->getConfig() -)); + $uBackend, $helper, $deletedUsersIndex, $userMapping)); diff --git a/apps/user_ldap/command/checkuser.php b/apps/user_ldap/command/checkuser.php index 96c6c832356..5b1dc36c1d0 100644 --- a/apps/user_ldap/command/checkuser.php +++ b/apps/user_ldap/command/checkuser.php @@ -16,6 +16,7 @@ use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\User; use OCA\User_LDAP\lib\user\Manager; +use OCA\User_LDAP\lib\User\DeletedUsersIndex; use OCA\user_ldap\lib\Helper; use OCA\user_ldap\User_Proxy; @@ -26,18 +27,22 @@ class CheckUser extends Command { /** @var \OCA\User_LDAP\lib\Helper */ protected $helper; - /** @var \OCP\IConfig */ - protected $config; + /** @var \OCA\User_LDAP\lib\User\DeletedUsersIndex */ + protected $dui; + + /** @var \OCA\User_LDAP\Mapping\UserMapping */ + protected $mapping; /** * @param OCA\user_ldap\User_Proxy $uBackend * @param OCA\User_LDAP\lib\Helper $helper * @param OCP\IConfig $config */ - public function __construct(User_Proxy $uBackend, Helper $helper, \OCP\IConfig $config) { + public function __construct(User_Proxy $uBackend, Helper $helper, DeletedUsersIndex $dui, UserMapping $mapping) { $this->backend = $uBackend; $this->helper = $helper; - $this->config = $config; + $this->dui = $dui; + $this->mapping = $mapping; parent::__construct(); } @@ -70,10 +75,7 @@ class CheckUser extends Command { return; } - // TODO FIXME consolidate next line in DeletedUsersIndex - // (impractical now, because of class dependencies) - $this->config->setUserValue($uid, 'user_ldap', 'isDeleted', '1'); - + $this->dui->markUser($uid); $output->writeln('The user does not exists on LDAP anymore.'); $output->writeln('Clean up the user\'s remnants by: ./occ user:delete "' . $uid . '"'); @@ -86,22 +88,11 @@ class CheckUser extends Command { * checks whether a user is actually mapped * @param string $ocName the username as used in ownCloud * @throws \Exception - * @return bool + * @return true */ protected function confirmUserIsMapped($ocName) { - //TODO FIXME this should go to Mappings in OC 8 - $db = \OC::$server->getDatabaseConnection(); - $query = $db->prepare(' - SELECT - `ldap_dn` AS `dn` - FROM `*PREFIX*ldap_user_mapping` - WHERE `owncloud_name` = ?' - ); - - $query->execute(array($ocName)); - $result = $query->fetchColumn(); - - if($result === false) { + $dn = $this->mapping->getDNByName($ocName); + if ($dn === false) { throw new \Exception('The given user is not a recognized LDAP user.'); } diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php index 56fb296609d..35252d6f6e5 100644 --- a/apps/user_ldap/lib/jobs/cleanup.php +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -11,6 +11,8 @@ namespace OCA\User_LDAP\Jobs; use \OCA\user_ldap\User_Proxy; use \OCA\user_ldap\lib\Helper; use \OCA\user_ldap\lib\LDAP; +use \OCA\User_LDAP\lib\User\DeletedUsersIndex; +use \OCA\User_LDAP\Mapping\UserMapping; /** * Class CleanUp @@ -45,6 +47,12 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { */ protected $ldapHelper; + /** @var \OCA\User_LDAP\Mapping\UserMapping */ + protected $mapping; + + /** @var \OCA\User_LDAP\lib\User\DeletedUsersIndex */ + protected $dui; + /** * @var int $defaultIntervalMin default interval in minutes */ @@ -92,6 +100,19 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { } else { $this->db = \OC::$server->getDatabaseConnection(); } + + if(isset($arguments['mapping'])) { + $this->mapping = $arguments['mapping']; + } else { + $this->mapping = new UserMapping($this->db); + } + + if(isset($arguments['deletedUsersIndex'])) { + $this->dui = $arguments['deletedUsersIndex']; + } else { + $this->dui = new DeletedUsersIndex( + $this->ocConfig, $this->db, $this->mapping); + } } /** @@ -104,7 +125,7 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { if(!$this->isCleanUpAllowed()) { return; } - $users = $this->getMappedUsers($this->limit, $this->getOffset()); + $users = $this->mapping->getList($this->limit, $this->getOffset()); if(!is_array($users)) { //something wrong? Let's start from the beginning next time and //abort @@ -169,33 +190,11 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { private function checkUser($user) { if($this->userBackend->userExistsOnLDAP($user['name'])) { //still available, all good + return; } - // TODO FIXME consolidate next line in DeletedUsersIndex - // (impractical now, because of class dependencies) - $this->ocConfig->setUserValue($user['name'], 'user_ldap', 'isDeleted', '1'); - } - - /** - * returns a batch of users from the mappings table - * @param int $limit - * @param int $offset - * @return array - */ - public function getMappedUsers($limit, $offset) { - $query = $this->db->prepare(' - SELECT - `ldap_dn` AS `dn`, - `owncloud_name` AS `name`, - `directory_uuid` AS `uuid` - FROM `*PREFIX*ldap_user_mapping`', - $limit, - $offset - ); - - $query->execute(); - return $query->fetchAll(); + $this->dui->markUser($user['name']); } /** diff --git a/apps/user_ldap/lib/mapping/abstractmapping.php b/apps/user_ldap/lib/mapping/abstractmapping.php index 2c45c6bb1c1..19f173577f5 100644 --- a/apps/user_ldap/lib/mapping/abstractmapping.php +++ b/apps/user_ldap/lib/mapping/abstractmapping.php @@ -152,6 +152,27 @@ abstract class AbstractMapping { return $this->getXbyY('owncloud_name', 'directory_uuid', $uuid); } + /** + * gets a piece of the mapping list + * @param int $offset + * @param int $limit + * @return array + */ + public function getList($offset = null, $limit = null) { + $query = $this->dbc->prepare(' + SELECT + `ldap_dn` AS `dn`, + `owncloud_name` AS `name`, + `directory_uuid` AS `uuid` + FROM `*PREFIX*ldap_user_mapping`', + $limit, + $offset + ); + + $query->execute(); + return $query->fetchAll(); + } + /** * attempts to map the given entry * @param string $fdn fully distinguished name (from LDAP) diff --git a/apps/user_ldap/lib/user/deletedusersindex.php b/apps/user_ldap/lib/user/deletedusersindex.php index 62abe2e10de..67585530279 100644 --- a/apps/user_ldap/lib/user/deletedusersindex.php +++ b/apps/user_ldap/lib/user/deletedusersindex.php @@ -69,6 +69,7 @@ class DeletedUsersIndex { foreach($deletedUsers as $user) { $userObjects[] = new OfflineUser($user, $this->config, $this->db, $this->mapping); } + $this->deletedUsers = $userObjects; return $this->deletedUsers; } @@ -97,4 +98,12 @@ class DeletedUsersIndex { } return false; } + + /** + * marks a user as deleted + * @param string ocName + */ + public function markUser($ocName) { + $this->config->setUserValue($ocName, 'user_ldap', 'isDeleted', '1'); + } } diff --git a/apps/user_ldap/tests/jobs/cleanup.php b/apps/user_ldap/tests/jobs/cleanup.php index 3aa9a4a43c5..642bad57134 100644 --- a/apps/user_ldap/tests/jobs/cleanup.php +++ b/apps/user_ldap/tests/jobs/cleanup.php @@ -15,6 +15,10 @@ class Test_CleanUp extends \PHPUnit_Framework_TestCase { $this->getMockBuilder('\OCA\user_ldap\User_Proxy') ->disableOriginalConstructor() ->getMock(); + $mocks['deletedUsersIndex'] = + $this->getMockBuilder('\OCA\user_ldap\lib\user\deletedUsersIndex') + ->disableOriginalConstructor() + ->getMock(); $mocks['ocConfig'] = $this->getMock('\OCP\IConfig'); $mocks['db'] = $this->getMock('\OCP\IDBConnection'); $mocks['helper'] = $this->getMock('\OCA\user_ldap\lib\Helper'); -- cgit v1.2.3 From e724b78694cb17c7a3ff4427ae103b01baa4688c Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 6 Jan 2015 17:50:06 +0100 Subject: smaller fixes: coding style, PHPdoc, typos and few for DI --- apps/user_ldap/appinfo/register_command.php | 8 ++++--- apps/user_ldap/command/checkuser.php | 16 ++++++++------ apps/user_ldap/command/showremnants.php | 25 +++++++++------------ apps/user_ldap/lib/jobs.php | 4 ++++ apps/user_ldap/lib/jobs/cleanup.php | 32 +++++++++------------------ apps/user_ldap/lib/user/deletedusersindex.php | 7 +++++- apps/user_ldap/lib/user/offlineuser.php | 6 +++++ apps/user_ldap/tests/jobs/cleanup.php | 24 -------------------- 8 files changed, 51 insertions(+), 71 deletions(-) diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index e0013c4eb7a..0e918b82335 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -20,7 +20,8 @@ $uBackend = new User_Proxy( new LDAP() ); $deletedUsersIndex = new DeletedUsersIndex( - \OC::$server->getConfig(), $dbConnection, $userMapping); + \OC::$server->getConfig(), $dbConnection, $userMapping +); $application->add(new OCA\user_ldap\Command\ShowConfig()); $application->add(new OCA\user_ldap\Command\SetConfig()); @@ -28,6 +29,7 @@ $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\ShowRemnants($userMapping)); +$application->add(new OCA\user_ldap\Command\ShowRemnants($deletedUsersIndex)); $application->add(new OCA\user_ldap\Command\CheckUser( - $uBackend, $helper, $deletedUsersIndex, $userMapping)); + $uBackend, $helper, $deletedUsersIndex, $userMapping) +); diff --git a/apps/user_ldap/command/checkuser.php b/apps/user_ldap/command/checkuser.php index 5b1dc36c1d0..f9065a7c8d6 100644 --- a/apps/user_ldap/command/checkuser.php +++ b/apps/user_ldap/command/checkuser.php @@ -17,7 +17,8 @@ use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\User; use OCA\User_LDAP\lib\user\Manager; use OCA\User_LDAP\lib\User\DeletedUsersIndex; -use OCA\user_ldap\lib\Helper; +use OCA\User_LDAP\Mapping\UserMapping; +use OCA\user_ldap\lib\Helper as LDAPHelper; use OCA\user_ldap\User_Proxy; class CheckUser extends Command { @@ -35,10 +36,11 @@ class CheckUser extends Command { /** * @param OCA\user_ldap\User_Proxy $uBackend - * @param OCA\User_LDAP\lib\Helper $helper - * @param OCP\IConfig $config + * @param OCA\user_ldap\lib\Helper $helper + * @param OCA\User_LDAP\lib\User\DeletedUsersIndex $dui + * @param OCA\User_LDAP\Mapping\UserMapping $mapping */ - public function __construct(User_Proxy $uBackend, Helper $helper, DeletedUsersIndex $dui, UserMapping $mapping) { + public function __construct(User_Proxy $uBackend, LDAPHelper $helper, DeletedUsersIndex $dui, UserMapping $mapping) { $this->backend = $uBackend; $this->helper = $helper; $this->dui = $dui; @@ -100,13 +102,13 @@ class CheckUser extends Command { } /** - * checks whether the setup allows reliable checking of LDAP user existance + * checks whether the setup allows reliable checking of LDAP user existence * @throws \Exception - * @return bool + * @return true */ protected function isAllowed($force) { if($this->helper->haveDisabledConfigurations() && !$force) { - throw new \Exception('Cannot check user existance, because ' + throw new \Exception('Cannot check user existence, because ' . 'disabled LDAP configurations are present.'); } diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php index 8144a54cbee..ab78cee96e7 100644 --- a/apps/user_ldap/command/showremnants.php +++ b/apps/user_ldap/command/showremnants.php @@ -19,16 +19,14 @@ use OCA\User_LDAP\lib\Connection; use OCA\User_LDAP\Mapping\UserMapping; class ShowRemnants extends Command { - /** @var OCA\User_LDAP\Mapping\UserMapping */ - protected $mapping; + /** @var use OCA\User_LDAP\lib\User\DeletedUsersIndex; */ + protected $dui; /** - * @param OCA\user_ldap\User_Proxy $uBackend - * @param OCA\User_LDAP\lib\Helper $helper - * @param OCP\IConfig $config + * @param OCA\user_ldap\lib\user\DeletedUsersIndex $dui */ - public function __construct(UserMapping $mapper) { - $this->mapper = $mapper; + public function __construct(DeletedUsersIndex $dui) { + $this->dui = $dui; parent::__construct(); } @@ -39,20 +37,19 @@ class ShowRemnants extends Command { ; } + /** + * executes the command, i.e. creeates and outputs a table of LDAP users marked as deleted + * + * {@inheritdoc} + */ protected function execute(InputInterface $input, OutputInterface $output) { - $dui = new DeletedUsersIndex( - \OC::$server->getConfig(), - \OC::$server->getDatabaseConnection(), - $this->mapper - ); - /** @var \Symfony\Component\Console\Helper\Table $table */ $table = $this->getHelperSet()->get('table'); $table->setHeaders(array( 'ownCloud name', 'Display Name', 'LDAP UID', 'LDAP DN', 'Last Login', 'Dir', 'Sharer')); $rows = array(); - $resultSet = $dui->getUsers(); + $resultSet = $this->dui->getUsers(); foreach($resultSet as $user) { $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; $lastLogin = ($user->getLastLogin() > 0) ? diff --git a/apps/user_ldap/lib/jobs.php b/apps/user_ldap/lib/jobs.php index 391a10d31f8..e8e6df0b9d0 100644 --- a/apps/user_ldap/lib/jobs.php +++ b/apps/user_ldap/lib/jobs.php @@ -24,6 +24,7 @@ namespace OCA\user_ldap\lib; use OCA\User_LDAP\Mapping\GroupMapping; +use OCA\User_LDAP\Mapping\UserMapping; class Jobs extends \OC\BackgroundJob\TimedJob { static private $groupsFromDB; @@ -172,6 +173,9 @@ class Jobs extends \OC\BackgroundJob\TimedJob { $connector = new Connection($ldapWrapper, $configPrefixes[0]); $ldapAccess = new Access($connector, $ldapWrapper, $userManager); $groupMapper = new GroupMapping(\OC::$server->getDatabaseConnection()); + $userMapper = new UserMapping(\OC::$server->getDatabaseConnection()); + $ldapAccess->setGroupMapper($groupMapper); + $ldapAccess->setUserMapper($userMapper); self::$groupBE = new \OCA\user_ldap\GROUP_LDAP($ldapAccess); } else { self::$groupBE = new \OCA\user_ldap\Group_Proxy($configPrefixes, $ldapWrapper); diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php index 35252d6f6e5..83cfeb048c8 100644 --- a/apps/user_ldap/lib/jobs/cleanup.php +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -22,29 +22,22 @@ use \OCA\User_LDAP\Mapping\UserMapping; * @package OCA\user_ldap\lib; */ class CleanUp extends \OC\BackgroundJob\TimedJob { - /** - * @var int $limit amount of users that should be checked per run - */ + /** @var int $limit amount of users that should be checked per run */ protected $limit = 50; - /** - * @var \OCP\UserInterface $userBackend - */ + /** @var int $defaultIntervalMin default interval in minutes */ + protected $defaultIntervalMin = 51; + + /** @var \OCP\UserInterface $userBackend */ protected $userBackend; - /** - * @var \OCP\IConfig $ocConfig - */ + /** @var \OCP\IConfig $ocConfig */ protected $ocConfig; - /** - * @var \OCP\IDBConnection $db - */ + /** @var \OCP\IDBConnection $db */ protected $db; - /** - * @var Helper $ldapHelper - */ + /** @var Helper $ldapHelper */ protected $ldapHelper; /** @var \OCA\User_LDAP\Mapping\UserMapping */ @@ -53,11 +46,6 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { /** @var \OCA\User_LDAP\lib\User\DeletedUsersIndex */ protected $dui; - /** - * @var int $defaultIntervalMin default interval in minutes - */ - protected $defaultIntervalMin = 51; - public function __construct() { $minutes = \OC::$server->getConfig()->getSystemValue( 'ldapUserCleanupInterval', strval($this->defaultIntervalMin)); @@ -177,7 +165,7 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { * checks users whether they are still existing * @param array $users result from getMappedUsers() */ - private function checkUsers($users) { + private function checkUsers(array $users) { foreach($users as $user) { $this->checkUser($user); } @@ -187,7 +175,7 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { * checks whether a user is still existing in LDAP * @param string[] $user */ - private function checkUser($user) { + private function checkUser(array $user) { if($this->userBackend->userExistsOnLDAP($user['name'])) { //still available, all good diff --git a/apps/user_ldap/lib/user/deletedusersindex.php b/apps/user_ldap/lib/user/deletedusersindex.php index 67585530279..e17ed3384da 100644 --- a/apps/user_ldap/lib/user/deletedusersindex.php +++ b/apps/user_ldap/lib/user/deletedusersindex.php @@ -49,8 +49,13 @@ class DeletedUsersIndex { /** * @var array $deletedUsers */ - protected $deletedUsers = false; + protected $deletedUsers; + /** + * @param OCP\IConfig $config + * @param OCP\IDBConnection $db + * @param OCA\User_LDAP\Mapping\UserMapping $mapping + */ public function __construct(\OCP\IConfig $config, \OCP\IDBConnection $db, UserMapping $mapping) { $this->config = $config; $this->db = $db; diff --git a/apps/user_ldap/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php index 9383320fae2..e5c87fd23fc 100644 --- a/apps/user_ldap/lib/user/offlineuser.php +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -71,6 +71,12 @@ class OfflineUser { */ protected $mapping; + /** + * @param string $ocName + * @param OCP\IConfig $config + * @param OCP\IDBConnection $db + * @param OCA\User_LDAP\Mapping\UserMapping $mapping + */ public function __construct($ocName, \OCP\IConfig $config, \OCP\IDBConnection $db, UserMapping $mapping) { $this->ocName = $ocName; $this->config = $config; diff --git a/apps/user_ldap/tests/jobs/cleanup.php b/apps/user_ldap/tests/jobs/cleanup.php index 642bad57134..78bda66c54f 100644 --- a/apps/user_ldap/tests/jobs/cleanup.php +++ b/apps/user_ldap/tests/jobs/cleanup.php @@ -105,30 +105,6 @@ class Test_CleanUp extends \PHPUnit_Framework_TestCase { $this->assertSame(true, $result); } - /** - * test whether sql is OK - */ - public function test_getMappedUsers() { - $args = $this->getMocks(); - - $bgJob = new \OCA\User_LDAP\Jobs\CleanUp(); - $bgJob->setArguments($args); - - if(version_compare(\PHPUnit_Runner_Version::id(), '3.8', '<')) { - //otherwise we run into - //https://github.com/sebastianbergmann/phpunit-mock-objects/issues/103 - $this->markTestIncomplete(); - } - - $stmt = $this->getMock('\Doctrine\DBAL\Driver\Statement'); - - $args['db']->expects($this->once()) - ->method('prepare') - ->will($this->returnValue($stmt)); - - $bgJob->getMappedUsers(0, $bgJob->getChunkSize()); - } - /** * check whether offset will be reset when it needs to */ -- cgit v1.2.3 From 8e488f726c6f6aaaf1b33c6bc53f7ecb417b0d28 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 6 Jan 2015 17:52:54 +0100 Subject: revert changes to deprecated preferences as it is a not needed leftover --- lib/private/preferences.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/private/preferences.php b/lib/private/preferences.php index 1784d372261..cd4a9fd1c19 100644 --- a/lib/private/preferences.php +++ b/lib/private/preferences.php @@ -137,12 +137,10 @@ class Preferences { * @param string $app * @param string $key * @param string $value - * @param int|null $limit - * @param int|null $offset * @return array * @deprecated use getUsersForUserValue of \OCP\IConfig instead */ - public function getUsersForValue($app, $key, $value, $limit = null, $offset = null) { + public function getUsersForValue($app, $key, $value) { return $this->config->getUsersForUserValue($app, $key, $value); } -- cgit v1.2.3 From 40ecd30fba6d29362c455b1f0eef875ff19cd544 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 6 Jan 2015 23:28:49 +0100 Subject: 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 | 30 +++++++++---- apps/user_ldap/user_proxy.php | 6 ++- 9 files changed, 89 insertions(+), 53 deletions(-) diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php index 302575c3682..980477bb273 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 0e918b82335..6abfd699c90 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 d826303c55d..ba87982d167 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 83cfeb048c8..b044f997aa9 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 431609071e6..955a2923a64 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 c81fb25b541..7f67ebca39b 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 fdda35b63c9..c4ba4f45795 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 c2d0f387b9c..f9a39ddca17 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -26,14 +26,26 @@ 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 */ + protected $homesToKill = array(); + + /** @var \OCP\IConfig */ + protected $ocConfig; + /** - * @var string[] $homesToKill + * @param \OCA\user_ldap\lib\Access $access + * @param \OCP\IConfig $ocConfig */ - protected $homesToKill = array(); + 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 @@ -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 77caa84ecd9..f5912fe1355 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]; } -- cgit v1.2.3 From 64f0b055e67974141cf2e156d2b062f55768ce7d Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 6 Jan 2015 23:40:00 +0100 Subject: inject IDateTimeFormatter to show-remnants command --- apps/user_ldap/appinfo/register_command.php | 4 +++- apps/user_ldap/command/showremnants.php | 11 ++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/apps/user_ldap/appinfo/register_command.php b/apps/user_ldap/appinfo/register_command.php index 6abfd699c90..651e6a564e1 100644 --- a/apps/user_ldap/appinfo/register_command.php +++ b/apps/user_ldap/appinfo/register_command.php @@ -31,7 +31,9 @@ $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($ocConfig)); -$application->add(new OCA\user_ldap\Command\ShowRemnants($deletedUsersIndex)); +$application->add(new OCA\user_ldap\Command\ShowRemnants( + $deletedUsersIndex, \OC::$server->getDateTimeFormatter()) +); $application->add(new OCA\user_ldap\Command\CheckUser( $uBackend, $helper, $deletedUsersIndex, $userMapping) ); diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php index ab78cee96e7..5b7322e7711 100644 --- a/apps/user_ldap/command/showremnants.php +++ b/apps/user_ldap/command/showremnants.php @@ -17,16 +17,21 @@ use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\DeletedUsersIndex; use OCA\User_LDAP\lib\Connection; use OCA\User_LDAP\Mapping\UserMapping; +use OCP\IDateTimeFormatter; class ShowRemnants extends Command { - /** @var use OCA\User_LDAP\lib\User\DeletedUsersIndex; */ + /** @var OCA\User_LDAP\lib\User\DeletedUsersIndex */ protected $dui; + /** @var OCP\IDateTimeFormatter */ + protected $dateFormatter; + /** * @param OCA\user_ldap\lib\user\DeletedUsersIndex $dui */ - public function __construct(DeletedUsersIndex $dui) { + public function __construct(DeletedUsersIndex $dui, IDateTimeFormatter $dateFormatter) { $this->dui = $dui; + $this->dateFormatter = $dateFormatter; parent::__construct(); } @@ -53,7 +58,7 @@ class ShowRemnants extends Command { foreach($resultSet as $user) { $hAS = $user->getHasActiveShares() ? 'Y' : 'N'; $lastLogin = ($user->getLastLogin() > 0) ? - \OCP\Util::formatDate($user->getLastLogin()) : '-'; + $this->dateFormatter->formatDate($user->getLastLogin()) : '-'; $rows[] = array( $user->getOCName(), $user->getDisplayName(), -- cgit v1.2.3 From b9235e2a24ada2bf69fc23cd83405661bde7f0da Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Jan 2015 00:52:18 +0100 Subject: inject DB Connection to user manager --- apps/user_ldap/ajax/wizard.php | 3 +- apps/user_ldap/appinfo/app.php | 7 ++-- apps/user_ldap/lib/jobs.php | 8 +++-- apps/user_ldap/lib/proxy.php | 8 +++-- apps/user_ldap/lib/user/manager.php | 56 ++++++++++++++----------------- apps/user_ldap/tests/access.php | 3 +- apps/user_ldap/tests/group_ldap.php | 3 +- apps/user_ldap/tests/user/manager.php | 27 +++++++-------- apps/user_ldap/tests/user/user.php | 63 ++++++++++++++++++----------------- apps/user_ldap/tests/user_ldap.php | 3 +- 10 files changed, 94 insertions(+), 87 deletions(-) diff --git a/apps/user_ldap/ajax/wizard.php b/apps/user_ldap/ajax/wizard.php index 6d7d70c8c5a..48bfb56311c 100644 --- a/apps/user_ldap/ajax/wizard.php +++ b/apps/user_ldap/ajax/wizard.php @@ -52,7 +52,8 @@ $userManager = new \OCA\user_ldap\lib\user\Manager( new \OCA\user_ldap\lib\FilesystemHelper(), new \OCA\user_ldap\lib\LogWrapper(), \OC::$server->getAvatarManager(), - new \OCP\Image()); + new \OCP\Image(), + \OC::$server->getDatabaseConnection()); $access = new \OCA\user_ldap\lib\Access($con, $ldapWrapper, $userManager); diff --git a/apps/user_ldap/appinfo/app.php b/apps/user_ldap/appinfo/app.php index 980477bb273..911688a5c20 100644 --- a/apps/user_ldap/appinfo/app.php +++ b/apps/user_ldap/appinfo/app.php @@ -29,14 +29,17 @@ $configPrefixes = $helper->getServerConfigurationPrefixes(true); $ldapWrapper = new OCA\user_ldap\lib\LDAP(); $ocConfig = \OC::$server->getConfig(); if(count($configPrefixes) === 1) { + $dbc = \OC::$server->getDatabaseConnection(); $userManager = new OCA\user_ldap\lib\user\Manager($ocConfig, new OCA\user_ldap\lib\FilesystemHelper(), new OCA\user_ldap\lib\LogWrapper(), \OC::$server->getAvatarManager(), - new \OCP\Image()); + new \OCP\Image(), + $dbc + ); $connector = new OCA\user_ldap\lib\Connection($ldapWrapper, $configPrefixes[0]); $ldapAccess = new OCA\user_ldap\lib\Access($connector, $ldapWrapper, $userManager); - $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, $ocConfig); diff --git a/apps/user_ldap/lib/jobs.php b/apps/user_ldap/lib/jobs.php index e8e6df0b9d0..a887b65251c 100644 --- a/apps/user_ldap/lib/jobs.php +++ b/apps/user_ldap/lib/jobs.php @@ -164,16 +164,18 @@ class Jobs extends \OC\BackgroundJob\TimedJob { $ldapWrapper = new LDAP(); if(count($configPrefixes) === 1) { //avoid the proxy when there is only one LDAP server configured + $dbc = \OC::$server->getDatabaseConnection(); $userManager = new user\Manager( \OC::$server->getConfig(), new FilesystemHelper(), new LogWrapper(), \OC::$server->getAvatarManager(), - new \OCP\Image()); + new \OCP\Image(), + $dbc); $connector = new Connection($ldapWrapper, $configPrefixes[0]); $ldapAccess = new Access($connector, $ldapWrapper, $userManager); - $groupMapper = new GroupMapping(\OC::$server->getDatabaseConnection()); - $userMapper = new UserMapping(\OC::$server->getDatabaseConnection()); + $groupMapper = new GroupMapping($dbc); + $userMapper = new UserMapping($dbc); $ldapAccess->setGroupMapper($groupMapper); $ldapAccess->setUserMapper($userMapper); self::$groupBE = new \OCA\user_ldap\GROUP_LDAP($ldapAccess); diff --git a/apps/user_ldap/lib/proxy.php b/apps/user_ldap/lib/proxy.php index 39d4b36c8bb..b4e6e33c1f4 100644 --- a/apps/user_ldap/lib/proxy.php +++ b/apps/user_ldap/lib/proxy.php @@ -49,16 +49,18 @@ abstract class Proxy { static $avatarM; static $userMap; static $groupMap; + static $db; if(is_null($fs)) { $ocConfig = \OC::$server->getConfig(); $fs = new FilesystemHelper(); $log = new LogWrapper(); $avatarM = \OC::$server->getAvatarManager(); - $userMap = new UserMapping(\OC::$server->getDatabaseConnection()); - $groupMap = new GroupMapping(\OC::$server->getDatabaseConnection()); + $db = \OC::$server->getDatabaseConnection(); + $userMap = new UserMapping($db); + $groupMap = new GroupMapping($db); } $userManager = - new user\Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image()); + new user\Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image(), $db); $connector = new Connection($this->ldap, $configPrefix); $access = new Access($connector, $this->ldap, $userManager); $access->setUserMapper($userMap); diff --git a/apps/user_ldap/lib/user/manager.php b/apps/user_ldap/lib/user/manager.php index 955a2923a64..ec50e031281 100644 --- a/apps/user_ldap/lib/user/manager.php +++ b/apps/user_ldap/lib/user/manager.php @@ -36,30 +36,27 @@ use OCA\user_ldap\lib\user\OfflineUser; * cache */ class Manager { - /** - * @var IUserTools - */ + /** @var IUserTools */ protected $access; - /** - * @var \OCP\IConfig - */ + + /** @var \OCP\IConfig */ protected $ocConfig; - /** - * @var FilesystemHelper - */ + + /** @var \OCP\IDBConnection */ + protected $db; + + /** @var FilesystemHelper */ protected $ocFilesystem; - /** - * @var LogWrapper - */ + + /** @var LogWrapper */ protected $ocLog; - /** - * @var \OCP\Image - */ + + /** @var \OCP\Image */ protected $image; - /** - * @param \OCP\IAvatarManager - */ + + /** @param \OCP\IAvatarManager */ protected $avatarManager; + /** * array['byDN'] \OCA\user_ldap\lib\User[] * ['byUid'] \OCA\user_ldap\lib\User[] @@ -71,28 +68,25 @@ class Manager { ); /** - * @brief Constructor - * @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 - * @param \OCP\IAvatarManager - * @param \OCP\Image an empty image instance + * @param \OCP\IConfig $ocConfig + * @param \OCA\user_ldap\lib\FilesystemHelper $ocFilesystem object that + * gives access to necessary functions from the OC filesystem + * @param \OCA\user_ldap\lib\LogWrapper $ocLog + * @param \OCP\IAvatarManager $avatarManager + * @param \OCP\Image $image an empty image instance + * @param \OCP\IDBConnection $db * @throws Exception when the methods mentioned above do not exist */ public function __construct(\OCP\IConfig $ocConfig, FilesystemHelper $ocFilesystem, LogWrapper $ocLog, - \OCP\IAvatarManager $avatarManager, \OCP\Image $image) { + \OCP\IAvatarManager $avatarManager, \OCP\Image $image, \OCP\IDBConnection $db) { - if(!method_exists($ocConfig, 'setUserValue') - || !method_exists($ocConfig, 'getUserValue')) { - throw new \Exception('Invalid ownCloud User Config object'); - } $this->ocConfig = $ocConfig; $this->ocFilesystem = $ocFilesystem; $this->ocLog = $ocLog; $this->avatarManager = $avatarManager; $this->image = $image; + $this->db = $db; } /** @@ -152,7 +146,7 @@ class Manager { return new OfflineUser( $id, $this->ocConfig, - \OC::$server->getDatabaseConnection(), + $this->db, $this->access->getUserMapper()); } diff --git a/apps/user_ldap/tests/access.php b/apps/user_ldap/tests/access.php index 85849229152..5c502f288eb 100644 --- a/apps/user_ldap/tests/access.php +++ b/apps/user_ldap/tests/access.php @@ -47,7 +47,8 @@ class Test_Access extends \Test\TestCase { $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), $this->getMock('\OCA\user_ldap\lib\LogWrapper'), $this->getMock('\OCP\IAvatarManager'), - $this->getMock('\OCP\Image'))); + $this->getMock('\OCP\Image'), + $this->getMock('\OCP\IDBConnection'))); return array($lw, $connector, $um); } diff --git a/apps/user_ldap/tests/group_ldap.php b/apps/user_ldap/tests/group_ldap.php index 0e01eb3ba6f..efd7f803f3b 100644 --- a/apps/user_ldap/tests/group_ldap.php +++ b/apps/user_ldap/tests/group_ldap.php @@ -45,7 +45,8 @@ class Test_Group_Ldap extends \Test\TestCase { $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), $this->getMock('\OCA\user_ldap\lib\LogWrapper'), $this->getMock('\OCP\IAvatarManager'), - $this->getMock('\OCP\Image') + $this->getMock('\OCP\Image'), + $this->getMock('\OCP\IDBConnection') ); $access = $this->getMock('\OCA\user_ldap\lib\Access', $accMethods, diff --git a/apps/user_ldap/tests/user/manager.php b/apps/user_ldap/tests/user/manager.php index fb47f60539f..4ce504365b8 100644 --- a/apps/user_ldap/tests/user/manager.php +++ b/apps/user_ldap/tests/user/manager.php @@ -33,12 +33,13 @@ class Test_User_Manager extends \Test\TestCase { $log = $this->getMock('\OCA\user_ldap\lib\LogWrapper'); $avaMgr = $this->getMock('\OCP\IAvatarManager'); $image = $this->getMock('\OCP\Image'); + $dbc = $this->getMock('\OCP\IDBConnection'); - return array($access, $config, $filesys, $image, $log, $avaMgr); + return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc); } public function testGetByDNExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $inputDN = 'cn=foo,dc=foobar,dc=bar'; @@ -57,7 +58,7 @@ class Test_User_Manager extends \Test\TestCase { $access->expects($this->never()) ->method('username2dn'); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($inputDN); @@ -65,7 +66,7 @@ class Test_User_Manager extends \Test\TestCase { } public function testGetByEDirectoryDN() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $inputDN = 'uid=foo,o=foobar,c=bar'; @@ -84,7 +85,7 @@ class Test_User_Manager extends \Test\TestCase { $access->expects($this->never()) ->method('username2dn'); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($inputDN); @@ -92,7 +93,7 @@ class Test_User_Manager extends \Test\TestCase { } public function testGetByExoticDN() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $inputDN = 'ab=cde,f=ghei,mno=pq'; @@ -111,7 +112,7 @@ class Test_User_Manager extends \Test\TestCase { $access->expects($this->never()) ->method('username2dn'); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($inputDN); @@ -119,7 +120,7 @@ class Test_User_Manager extends \Test\TestCase { } public function testGetByDNNotExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $inputDN = 'cn=gone,dc=foobar,dc=bar'; @@ -139,7 +140,7 @@ class Test_User_Manager extends \Test\TestCase { ->with($this->equalTo($inputDN)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($inputDN); @@ -147,7 +148,7 @@ class Test_User_Manager extends \Test\TestCase { } public function testGetByUidExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $dn = 'cn=foo,dc=foobar,dc=bar'; @@ -166,7 +167,7 @@ class Test_User_Manager extends \Test\TestCase { ->with($this->equalTo($uid)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($uid); @@ -174,7 +175,7 @@ class Test_User_Manager extends \Test\TestCase { } public function testGetByUidNotExisting() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); $dn = 'cn=foo,dc=foobar,dc=bar'; @@ -188,7 +189,7 @@ class Test_User_Manager extends \Test\TestCase { ->with($this->equalTo($uid)) ->will($this->returnValue(false)); - $manager = new Manager($config, $filesys, $log, $avaMgr, $image); + $manager = new Manager($config, $filesys, $log, $avaMgr, $image, $dbc); $manager->setLdapAccess($access); $user = $manager->get($uid); diff --git a/apps/user_ldap/tests/user/user.php b/apps/user_ldap/tests/user/user.php index e110921d2d3..5282a9f8b6e 100644 --- a/apps/user_ldap/tests/user/user.php +++ b/apps/user_ldap/tests/user/user.php @@ -33,11 +33,12 @@ class Test_User_User extends \Test\TestCase { $log = $this->getMock('\OCA\user_ldap\lib\LogWrapper'); $avaMgr = $this->getMock('\OCP\IAvatarManager'); $image = $this->getMock('\OCP\Image'); + $dbc = $this->getMock('\OCP\IDBConnection'); - return array($access, $config, $filesys, $image, $log, $avaMgr); + return array($access, $config, $filesys, $image, $log, $avaMgr, $dbc); } - private function getAdvancedMocks($cfMock, $fsMock, $logMock, $avaMgr) { + private function getAdvancedMocks($cfMock, $fsMock, $logMock, $avaMgr, $dbc) { static $conMethods; static $accMethods; static $umMethods; @@ -52,7 +53,7 @@ class Test_User_User extends \Test\TestCase { $lw = $this->getMock('\OCA\user_ldap\lib\ILDAPWrapper'); $im = $this->getMock('\OCP\Image'); $um = $this->getMock('\OCA\user_ldap\lib\user\Manager', - $umMethods, array($cfMock, $fsMock, $logMock, $avaMgr, $im)); + $umMethods, array($cfMock, $fsMock, $logMock, $avaMgr, $im, $dbc)); $connector = $this->getMock('\OCA\user_ldap\lib\Connection', $conMethods, array($lw, null, null)); $access = $this->getMock('\OCA\user_ldap\lib\Access', @@ -76,11 +77,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateEmailProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->once()) ->method('__get') @@ -110,11 +111,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateEmailNotProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->once()) ->method('__get') @@ -140,11 +141,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateEmailNotConfigured() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->once()) ->method('__get') @@ -167,11 +168,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateQuotaAllProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->at(0)) ->method('__get') @@ -210,11 +211,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateQuotaDefaultProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->at(0)) ->method('__get') @@ -253,11 +254,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateQuotaIndividualProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->at(0)) ->method('__get') @@ -296,11 +297,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateQuotaNoneProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->at(0)) ->method('__get') @@ -334,11 +335,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateQuotaNoneConfigured() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $connection->expects($this->at(0)) ->method('__get') @@ -370,11 +371,11 @@ class Test_User_User extends \Test\TestCase { //the testUpdateAvatar series also implicitely tests getAvatarImage public function testUpdateAvatarJpegPhotoProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $access->expects($this->once()) ->method('readAttribute') @@ -419,11 +420,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateAvatarThumbnailPhotoProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $access->expects($this->at(0)) ->method('readAttribute') @@ -477,11 +478,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateAvatarNotProvided() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $access->expects($this->at(0)) ->method('readAttribute') @@ -523,11 +524,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateBeforeFirstLogin() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $config->expects($this->at(0)) ->method('getUserValue') @@ -559,11 +560,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateAfterFirstLogin() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $config->expects($this->at(0)) ->method('getUserValue') @@ -599,11 +600,11 @@ class Test_User_User extends \Test\TestCase { } public function testUpdateNoRefresh() { - list($access, $config, $filesys, $image, $log, $avaMgr) = + list($access, $config, $filesys, $image, $log, $avaMgr, $dbc) = $this->getTestInstances(); list($access, $connection) = - $this->getAdvancedMocks($config, $filesys, $log, $avaMgr); + $this->getAdvancedMocks($config, $filesys, $log, $avaMgr, $dbc); $config->expects($this->at(0)) ->method('getUserValue') diff --git a/apps/user_ldap/tests/user_ldap.php b/apps/user_ldap/tests/user_ldap.php index c4ba4f45795..3fa4f2bf0a1 100644 --- a/apps/user_ldap/tests/user_ldap.php +++ b/apps/user_ldap/tests/user_ldap.php @@ -62,7 +62,8 @@ class Test_User_Ldap_Direct extends \Test\TestCase { $this->getMock('\OCA\user_ldap\lib\FilesystemHelper'), $this->getMock('\OCA\user_ldap\lib\LogWrapper'), $this->getMock('\OCP\IAvatarManager'), - $this->getMock('\OCP\Image') + $this->getMock('\OCP\Image'), + $this->getMock('\OCP\IDBConnection') ); $access = $this->getMock('\OCA\user_ldap\lib\Access', -- cgit v1.2.3 From ae9c9a46b8f11a0f548dfbbc23d3c215910130aa Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Jan 2015 12:21:28 +0100 Subject: inject and use user manager to delete command instead of using old static oc_user way --- core/command/user/delete.php | 13 ++++++++++++- core/register_command.php | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/core/command/user/delete.php b/core/command/user/delete.php index f64b40e4921..d5ec3ee0bde 100644 --- a/core/command/user/delete.php +++ b/core/command/user/delete.php @@ -14,6 +14,17 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Input\InputArgument; class Delete extends Command { + /** @var \OC\User\Manager */ + protected $userManager; + + /** + * @param \OC\User\Manager $userManager + */ + public function __construct(\OC\User\Manager $userManager) { + $this->userManager = $userManager; + parent::__construct(); + } + protected function configure() { $this ->setName('user:delete') @@ -26,7 +37,7 @@ class Delete extends Command { } protected function execute(InputInterface $input, OutputInterface $output) { - $wasSuccessful = \OC_User::deleteUser($input->getArgument('uid')); + $wasSuccessful = $this->userManager->get($input->getArgument('uid'))->delete(); if($wasSuccessful === true) { $output->writeln('The specified user was deleted'); return; diff --git a/core/register_command.php b/core/register_command.php index 690e9879c47..5aa55be3e2c 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -22,6 +22,6 @@ $application->add(new OC\Core\Command\Maintenance\Repair($repair, \OC::$server-> $application->add(new OC\Core\Command\User\Report()); $application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\LastSeen()); -$application->add(new OC\Core\Command\User\Delete()); +$application->add(new OC\Core\Command\User\Delete(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\L10n\CreateJs()); -- cgit v1.2.3 From 6c335ee6fc846275b2138d480286a1dbcf1f4afe Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Jan 2015 12:39:04 +0100 Subject: add test for mapping's getList method --- .../tests/mapping/abstractmappingtest.php | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/apps/user_ldap/tests/mapping/abstractmappingtest.php b/apps/user_ldap/tests/mapping/abstractmappingtest.php index a5cb62253af..cafa36a4edb 100644 --- a/apps/user_ldap/tests/mapping/abstractmappingtest.php +++ b/apps/user_ldap/tests/mapping/abstractmappingtest.php @@ -191,4 +191,28 @@ abstract class AbstractMappingTest extends \Test\TestCase { $this->assertFalse($name); } } + + /** + * tests getList() method + */ + public function testList() { + list($mapper, $data) = $this->initTest(); + + // get all entries without specifying offset or limit + $results = $mapper->getList(); + $this->assertSame(3, count($results)); + + // get all-1 entries by specifying offset, and an high limit + // specifying only offset without limit will not work by underlying lib + $results = $mapper->getList(1, 999); + $this->assertSame(count($data) - 1, count($results)); + + // get first 2 entries by limit, but not offset + $results = $mapper->getList(null, 2); + $this->assertSame(2, count($results)); + + // get 2nd entry by specifying both offset and limit + $results = $mapper->getList(1, 1); + $this->assertSame(1, count($results)); + } } -- cgit v1.2.3 From 9668405ec73e5fcf648b93e2ebfa217ac81518ea Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Wed, 7 Jan 2015 13:28:56 +0100 Subject: doc fixes and removal of unnecessary use statements --- apps/user_ldap/command/checkuser.php | 1 - apps/user_ldap/command/showremnants.php | 9 +++------ apps/user_ldap/lib/jobs/cleanup.php | 2 +- apps/user_ldap/lib/user/offlineuser.php | 2 +- apps/user_ldap/user_ldap.php | 2 +- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/apps/user_ldap/command/checkuser.php b/apps/user_ldap/command/checkuser.php index f9065a7c8d6..202855e4853 100644 --- a/apps/user_ldap/command/checkuser.php +++ b/apps/user_ldap/command/checkuser.php @@ -15,7 +15,6 @@ use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\User; -use OCA\User_LDAP\lib\user\Manager; use OCA\User_LDAP\lib\User\DeletedUsersIndex; use OCA\User_LDAP\Mapping\UserMapping; use OCA\user_ldap\lib\Helper as LDAPHelper; diff --git a/apps/user_ldap/command/showremnants.php b/apps/user_ldap/command/showremnants.php index 5b7322e7711..5cfab34ef95 100644 --- a/apps/user_ldap/command/showremnants.php +++ b/apps/user_ldap/command/showremnants.php @@ -9,25 +9,22 @@ namespace OCA\user_ldap\Command; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use OCA\user_ldap\lib\user\DeletedUsersIndex; -use OCA\User_LDAP\lib\Connection; -use OCA\User_LDAP\Mapping\UserMapping; use OCP\IDateTimeFormatter; class ShowRemnants extends Command { - /** @var OCA\User_LDAP\lib\User\DeletedUsersIndex */ + /** @var \OCA\User_LDAP\lib\User\DeletedUsersIndex */ protected $dui; - /** @var OCP\IDateTimeFormatter */ + /** @var \OCP\IDateTimeFormatter */ protected $dateFormatter; /** * @param OCA\user_ldap\lib\user\DeletedUsersIndex $dui + * @param OCP\IDateTimeFormatter $dateFormatter */ public function __construct(DeletedUsersIndex $dui, IDateTimeFormatter $dateFormatter) { $this->dui = $dui; diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php index b044f997aa9..7209126c82f 100644 --- a/apps/user_ldap/lib/jobs/cleanup.php +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -191,7 +191,7 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { * @return int */ private function getOffset() { - return $this->ocConfig->getAppValue('user_ldap', 'cleanUpJobOffset', 0); + return intval($this->ocConfig->getAppValue('user_ldap', 'cleanUpJobOffset', 0)); } /** diff --git a/apps/user_ldap/lib/user/offlineuser.php b/apps/user_ldap/lib/user/offlineuser.php index e5c87fd23fc..1833f4be968 100644 --- a/apps/user_ldap/lib/user/offlineuser.php +++ b/apps/user_ldap/lib/user/offlineuser.php @@ -67,7 +67,7 @@ class OfflineUser { */ protected $db; /** - * @var OCA\User_LDAP\Mapping\UserMapping + * @var \OCA\User_LDAP\Mapping\UserMapping */ protected $mapping; diff --git a/apps/user_ldap/user_ldap.php b/apps/user_ldap/user_ldap.php index f9a39ddca17..051e760105b 100644 --- a/apps/user_ldap/user_ldap.php +++ b/apps/user_ldap/user_ldap.php @@ -68,7 +68,7 @@ class USER_LDAP extends BackendUtility implements \OCP\IUserBackend, \OCP\UserIn * Check if the password is correct * @param string $uid The username * @param string $password The password - * @return boolean + * @return false|string * * Check if the password is correct without logging in the user */ -- cgit v1.2.3 From c7f273040e9e2e200bbe458d89114a98464c3355 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Jan 2015 14:21:40 +0100 Subject: fix table name for getList --- apps/user_ldap/lib/mapping/abstractmapping.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/user_ldap/lib/mapping/abstractmapping.php b/apps/user_ldap/lib/mapping/abstractmapping.php index 19f173577f5..cb9db83f683 100644 --- a/apps/user_ldap/lib/mapping/abstractmapping.php +++ b/apps/user_ldap/lib/mapping/abstractmapping.php @@ -32,7 +32,7 @@ abstract class AbstractMapping { } /** - * checks whether a provided string represents an exisiting table col + * checks whether a provided string represents an existing table col * @param string $col * @return bool */ @@ -164,7 +164,7 @@ abstract class AbstractMapping { `ldap_dn` AS `dn`, `owncloud_name` AS `name`, `directory_uuid` AS `uuid` - FROM `*PREFIX*ldap_user_mapping`', + FROM `' . $this->getTableName() . '`', $limit, $offset ); -- cgit v1.2.3 From c1a79d24c54d3c1d6ecb2c65540f9b52a3d6dc31 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Thu, 8 Jan 2015 17:45:07 +0100 Subject: fix order of initalizing instance properties, and paremeter order in a method call --- apps/user_ldap/lib/jobs/cleanup.php | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/apps/user_ldap/lib/jobs/cleanup.php b/apps/user_ldap/lib/jobs/cleanup.php index 7209126c82f..caf31f89820 100644 --- a/apps/user_ldap/lib/jobs/cleanup.php +++ b/apps/user_ldap/lib/jobs/cleanup.php @@ -8,10 +8,12 @@ namespace OCA\User_LDAP\Jobs; +use \OC\BackgroundJob\TimedJob; +use \OCA\user_ldap\User_LDAP; use \OCA\user_ldap\User_Proxy; use \OCA\user_ldap\lib\Helper; use \OCA\user_ldap\lib\LDAP; -use \OCA\User_LDAP\lib\User\DeletedUsersIndex; +use \OCA\user_ldap\lib\user\DeletedUsersIndex; use \OCA\User_LDAP\Mapping\UserMapping; /** @@ -21,14 +23,14 @@ use \OCA\User_LDAP\Mapping\UserMapping; * * @package OCA\user_ldap\lib; */ -class CleanUp extends \OC\BackgroundJob\TimedJob { +class CleanUp extends TimedJob { /** @var int $limit amount of users that should be checked per run */ protected $limit = 50; /** @var int $defaultIntervalMin default interval in minutes */ protected $defaultIntervalMin = 51; - /** @var \OCP\UserInterface $userBackend */ + /** @var User_LDAP|User_Proxy $userBackend */ protected $userBackend; /** @var \OCP\IConfig $ocConfig */ @@ -68,6 +70,12 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { $this->ldapHelper = new Helper(); } + if(isset($arguments['ocConfig'])) { + $this->ocConfig = $arguments['ocConfig']; + } else { + $this->ocConfig = \OC::$server->getConfig(); + } + if(isset($arguments['userBackend'])) { $this->userBackend = $arguments['userBackend']; } else { @@ -78,12 +86,6 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { ); } - if(isset($arguments['ocConfig'])) { - $this->ocConfig = $arguments['ocConfig']; - } else { - $this->ocConfig = \OC::$server->getConfig(); - } - if(isset($arguments['db'])) { $this->db = $arguments['db']; } else { @@ -114,7 +116,7 @@ class CleanUp extends \OC\BackgroundJob\TimedJob { if(!$this->isCleanUpAllowed()) { return; } - $users = $this->mapping->getList($this->limit, $this->getOffset()); + $users = $this->mapping->getList($this->getOffset(), $this->limit); if(!is_array($users)) { //something wrong? Let's start from the beginning next time and //abort -- cgit v1.2.3