diff --git a/apps/user_ldap/composer/composer/autoload_classmap.php b/apps/user_ldap/composer/composer/autoload_classmap.php index 7bade37d9f7e8..98a1bbfa1b76f 100644 --- a/apps/user_ldap/composer/composer/autoload_classmap.php +++ b/apps/user_ldap/composer/composer/autoload_classmap.php @@ -7,6 +7,7 @@ return array( 'OCA\\User_LDAP\\Access' => $baseDir . '/../lib/Access.php', + 'OCA\\User_LDAP\\AccessFactory' => $baseDir . '/../lib/AccessFactory.php', 'OCA\\User_LDAP\\AppInfo\\Application' => $baseDir . '/../lib/AppInfo/Application.php', 'OCA\\User_LDAP\\BackendUtility' => $baseDir . '/../lib/BackendUtility.php', 'OCA\\User_LDAP\\Command\\CheckUser' => $baseDir . '/../lib/Command/CheckUser.php', @@ -19,6 +20,7 @@ 'OCA\\User_LDAP\\Command\\TestConfig' => $baseDir . '/../lib/Command/TestConfig.php', 'OCA\\User_LDAP\\Configuration' => $baseDir . '/../lib/Configuration.php', 'OCA\\User_LDAP\\Connection' => $baseDir . '/../lib/Connection.php', + 'OCA\\User_LDAP\\ConnectionFactory' => $baseDir . '/../lib/ConnectionFactory.php', 'OCA\\User_LDAP\\Controller\\ConfigAPIController' => $baseDir . '/../lib/Controller/ConfigAPIController.php', 'OCA\\User_LDAP\\Controller\\RenewPasswordController' => $baseDir . '/../lib/Controller/RenewPasswordController.php', 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => $baseDir . '/../lib/Exceptions/ConstraintViolationException.php', diff --git a/apps/user_ldap/composer/composer/autoload_static.php b/apps/user_ldap/composer/composer/autoload_static.php index fbe63285273f0..6c97237d62d2a 100644 --- a/apps/user_ldap/composer/composer/autoload_static.php +++ b/apps/user_ldap/composer/composer/autoload_static.php @@ -19,6 +19,7 @@ class ComposerStaticInitUser_LDAP public static $classMap = array ( 'OCA\\User_LDAP\\Access' => __DIR__ . '/..' . '/../lib/Access.php', + 'OCA\\User_LDAP\\AccessFactory' => __DIR__ . '/..' . '/../lib/AccessFactory.php', 'OCA\\User_LDAP\\AppInfo\\Application' => __DIR__ . '/..' . '/../lib/AppInfo/Application.php', 'OCA\\User_LDAP\\BackendUtility' => __DIR__ . '/..' . '/../lib/BackendUtility.php', 'OCA\\User_LDAP\\Command\\CheckUser' => __DIR__ . '/..' . '/../lib/Command/CheckUser.php', @@ -31,6 +32,7 @@ class ComposerStaticInitUser_LDAP 'OCA\\User_LDAP\\Command\\TestConfig' => __DIR__ . '/..' . '/../lib/Command/TestConfig.php', 'OCA\\User_LDAP\\Configuration' => __DIR__ . '/..' . '/../lib/Configuration.php', 'OCA\\User_LDAP\\Connection' => __DIR__ . '/..' . '/../lib/Connection.php', + 'OCA\\User_LDAP\\ConnectionFactory' => __DIR__ . '/..' . '/../lib/ConnectionFactory.php', 'OCA\\User_LDAP\\Controller\\ConfigAPIController' => __DIR__ . '/..' . '/../lib/Controller/ConfigAPIController.php', 'OCA\\User_LDAP\\Controller\\RenewPasswordController' => __DIR__ . '/..' . '/../lib/Controller/RenewPasswordController.php', 'OCA\\User_LDAP\\Exceptions\\ConstraintViolationException' => __DIR__ . '/..' . '/../lib/Exceptions/ConstraintViolationException.php', diff --git a/apps/user_ldap/lib/AccessFactory.php b/apps/user_ldap/lib/AccessFactory.php new file mode 100644 index 0000000000000..45ff779bb01e5 --- /dev/null +++ b/apps/user_ldap/lib/AccessFactory.php @@ -0,0 +1,61 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\User_LDAP; + + +use OCA\User_LDAP\User\Manager; +use OCP\IConfig; + +class AccessFactory { + /** @var ILDAPWrapper */ + protected $ldap; + /** @var Manager */ + protected $userManager; + /** @var Helper */ + protected $helper; + /** @var IConfig */ + protected $config; + + public function __construct( + ILDAPWrapper $ldap, + Manager $userManager, + Helper $helper, + IConfig $config) + { + $this->ldap = $ldap; + $this->userManager = $userManager; + $this->helper = $helper; + $this->config = $config; + } + + public function get(Connection $connection) { + return new Access( + $connection, + $this->ldap, + $this->userManager, + $this->helper, + $this->config + ); + } +} diff --git a/apps/user_ldap/lib/ConnectionFactory.php b/apps/user_ldap/lib/ConnectionFactory.php new file mode 100644 index 0000000000000..0857afdcc8d11 --- /dev/null +++ b/apps/user_ldap/lib/ConnectionFactory.php @@ -0,0 +1,38 @@ + + * + * @author Arthur Schiwon + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\User_LDAP; + + +class ConnectionFactory { + /** @var ILDAPWrapper */ + private $ldap; + + public function __construct(ILDAPWrapper $ldap) { + $this->ldap = $ldap; + } + + public function get($prefix) { + return new Connection($this->ldap, $prefix, 'user_ldap'); + } +} diff --git a/apps/user_ldap/lib/Jobs/Sync.php b/apps/user_ldap/lib/Jobs/Sync.php index d8e0e85471842..b78a1947e27db 100644 --- a/apps/user_ldap/lib/Jobs/Sync.php +++ b/apps/user_ldap/lib/Jobs/Sync.php @@ -26,8 +26,10 @@ use OC\BackgroundJob\TimedJob; use OC\ServerNotAvailableException; use OCA\User_LDAP\Access; +use OCA\User_LDAP\AccessFactory; use OCA\User_LDAP\Configuration; use OCA\User_LDAP\Connection; +use OCA\User_LDAP\ConnectionFactory; use OCA\User_LDAP\FilesystemHelper; use OCA\User_LDAP\Helper; use OCA\User_LDAP\LDAP; @@ -62,6 +64,10 @@ class Sync extends TimedJob { protected $ncUserManager; /** @var IManager */ protected $notificationManager; + /** @var ConnectionFactory */ + protected $connectionFactory; + /** @var AccessFactory */ + protected $accessFactory; public function __construct() { $this->setInterval( @@ -112,7 +118,7 @@ protected function getMinPagingSize() { /** * @param array $argument */ - protected function run($argument) { + public function run($argument) { $this->setArgument($argument); $isBackgroundJobModeAjax = $this->config @@ -140,11 +146,11 @@ protected function run($argument) { if ($expectMoreResults) { $this->increaseOffset($cycleData); } else { - $this->determineNextCycle(); + $this->determineNextCycle($cycleData); } $this->updateInterval(); } catch (ServerNotAvailableException $e) { - $this->determineNextCycle(); + $this->determineNextCycle($cycleData); } } @@ -153,8 +159,8 @@ protected function run($argument) { * @return bool whether more results are expected from the same configuration */ public function runCycle($cycleData) { - $connection = new Connection($this->ldap, $cycleData['prefix']); - $access = new Access($connection, $this->ldap, $this->userManager, $this->ldapHelper, $this->config); + $connection = $this->connectionFactory->get($cycleData['prefix']); + $access = $this->accessFactory->get($connection); $access->setUserMapper($this->mapper); $filter = $access->combineFilterWithAnd(array( @@ -173,7 +179,7 @@ public function runCycle($cycleData) { if($connection->ldapPagingSize === 0) { return true; } - return count($results) !== $connection->ldapPagingSize; + return count($results) >= $connection->ldapPagingSize; } /** @@ -246,7 +252,7 @@ public function determineNextCycle(array $cycleData = null) { * @param $cycleData * @return bool */ - protected function qualifiesToRun($cycleData) { + public function qualifiesToRun($cycleData) { $lastChange = $this->config->getAppValue('user_ldap', $cycleData['prefix'] . '_lastChange', 0); if((time() - $lastChange) > 60 * 30) { return true; @@ -358,5 +364,22 @@ public function setArgument($argument) { } else { $this->mapper = new UserMapping($this->dbc); } + + if(isset($argument['connectionFactory'])) { + $this->connectionFactory = $argument['connectionFactory']; + } else { + $this->connectionFactory = new ConnectionFactory($this->ldap); + } + + if(isset($argument['accessFactory'])) { + $this->accessFactory = $argument['accessFactory']; + } else { + $this->accessFactory = new AccessFactory( + $this->ldap, + $this->userManager, + $this->ldapHelper, + $this->config + ); + } } } diff --git a/apps/user_ldap/tests/Jobs/SyncTest.php b/apps/user_ldap/tests/Jobs/SyncTest.php index f8a44de87e8dd..f8852a46664df 100644 --- a/apps/user_ldap/tests/Jobs/SyncTest.php +++ b/apps/user_ldap/tests/Jobs/SyncTest.php @@ -23,6 +23,10 @@ namespace OCA\User_LDAP\Tests\Jobs; +use OCA\User_LDAP\Access; +use OCA\User_LDAP\AccessFactory; +use OCA\User_LDAP\Connection; +use OCA\User_LDAP\ConnectionFactory; use OCA\User_LDAP\Helper; use OCA\User_LDAP\Jobs\Sync; use OCA\User_LDAP\LDAP; @@ -33,6 +37,7 @@ use OCP\IDBConnection; use OCP\IUserManager; use OCP\Notification\IManager; +use function Sodium\memcmp; use Test\TestCase; class SyncTest extends TestCase { @@ -59,6 +64,10 @@ class SyncTest extends TestCase { protected $ncUserManager; /** @var IManager|\PHPUnit_Framework_MockObject_MockObject */ protected $notificationManager; + /** @var ConnectionFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $connectionFactory; + /** @var AccessFactory|\PHPUnit_Framework_MockObject_MockObject */ + protected $accessFactory; public function setUp() { parent::setUp(); @@ -72,6 +81,8 @@ public function setUp() { $this->dbc = $this->createMock(IDBConnection::class); $this->ncUserManager = $this->createMock(IUserManager::class); $this->notificationManager = $this->createMock(IManager::class); + $this->connectionFactory = $this->createMock(ConnectionFactory::class); + $this->accessFactory = $this->createMock(AccessFactory::class); $this->arguments = [ 'helper' => $this->helper, @@ -83,6 +94,8 @@ public function setUp() { 'dbc' => $this->dbc, 'ncUserManager' => $this->ncUserManager, 'notificationManager' => $this->notificationManager, + 'connectionFactory' => $this->connectionFactory, + 'accessFactory' => $this->accessFactory, ]; $this->sync = new Sync(); @@ -141,4 +154,216 @@ public function testUpdateInterval($userCount, $pagingSize1, $pagingSize2) { $this->sync->updateInterval(); } + public function moreResultsProvider() { + return [ + [ 3, 3, true ], + [ 3, 5, true ], + [ 3, 2, false] + ]; + } + + /** + * @dataProvider moreResultsProvider + */ + public function testMoreResults($pagingSize, $results, $expected) { + $connection = $this->createMock(Connection::class); + $this->connectionFactory->expects($this->any()) + ->method('get') + ->willReturn($connection); + $connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($key) use ($pagingSize) { + if($key === 'ldapPagingSize') { + return $pagingSize; + } + return null; + }); + + /** @var Access|\PHPUnit_Framework_MockObject_MockObject $access */ + $access = $this->createMock(Access::class); + $this->accessFactory->expects($this->any()) + ->method('get') + ->with($connection) + ->willReturn($access); + + $access->expects($this->once()) + ->method('fetchListOfUsers') + ->willReturn(array_pad([], $results, 'someUser')); + $access->connection = $connection; + $access->userManager = $this->userManager; + + $this->sync->setArgument($this->arguments); + $hasMoreResults = $this->sync->runCycle(['prefix' => 's01', 'offset' => 100]); + $this->assertSame($expected, $hasMoreResults); + } + + public function cycleDataProvider() { + $lastCycle = ['prefix' => 's01', 'offset' => 1000]; + $lastCycle2 = ['prefix' => '', 'offset' => 1000]; + return [ + [ null, ['s01'], ['prefix' => 's01', 'offset' => 0] ], + [ null, [''], ['prefix' => '', 'offset' => 0] ], + [ $lastCycle, ['s01', 's02'], ['prefix' => 's02', 'offset' => 0] ], + [ $lastCycle, [''], ['prefix' => '', 'offset' => 0] ], + [ $lastCycle2, ['', 's01'], ['prefix' => 's01', 'offset' => 0] ], + [ $lastCycle, [], null ], + ]; + } + + /** + * @dataProvider cycleDataProvider + */ + public function testDetermineNextCycle($cycleData, $prefixes, $expectedCycle) { + $this->helper->expects($this->any()) + ->method('getServerConfigurationPrefixes') + ->with(true) + ->willReturn($prefixes); + + if(is_array($expectedCycle)) { + $this->config->expects($this->exactly(2)) + ->method('setAppValue') + ->withConsecutive( + ['user_ldap', 'background_sync_prefix', $expectedCycle['prefix']], + ['user_ldap', 'background_sync_offset', $expectedCycle['offset']] + ); + } else { + $this->config->expects($this->never()) + ->method('setAppValue'); + } + + $this->sync->setArgument($this->arguments); + $nextCycle = $this->sync->determineNextCycle($cycleData); + + if($expectedCycle === null) { + $this->assertNull($nextCycle); + } else { + $this->assertSame($expectedCycle['prefix'], $nextCycle['prefix']); + $this->assertSame($expectedCycle['offset'], $nextCycle['offset']); + } + } + + public function testQualifiesToRun() { + $cycleData = ['prefix' => 's01']; + + $this->config->expects($this->exactly(2)) + ->method('getAppValue') + ->willReturnOnConsecutiveCalls(time() - 60*40, time() - 60*20); + + $this->sync->setArgument($this->arguments); + $this->assertTrue($this->sync->qualifiesToRun($cycleData)); + $this->assertFalse($this->sync->qualifiesToRun($cycleData)); + } + + public function runDataProvider() { + return [ + #0 - one LDAP server, reset + [[ + 'prefixes' => [''], + 'scheduledCycle' => ['prefix' => '', 'offset' => '4500'], + 'pagingSize' => 500, + 'usersThisCycle' => 0, + 'expectedNextCycle' => ['prefix' => '', 'offset' => '0'], + 'mappedUsers' => 123, + ]], + #1 - 2 LDAP servers, next prefix + [[ + 'prefixes' => ['', 's01'], + 'scheduledCycle' => ['prefix' => '', 'offset' => '4500'], + 'pagingSize' => 500, + 'usersThisCycle' => 0, + 'expectedNextCycle' => ['prefix' => 's01', 'offset' => '0'], + 'mappedUsers' => 123, + ]], + #2 - 2 LDAP servers, rotate prefix + [[ + 'prefixes' => ['', 's01'], + 'scheduledCycle' => ['prefix' => 's01', 'offset' => '4500'], + 'pagingSize' => 500, + 'usersThisCycle' => 0, + 'expectedNextCycle' => ['prefix' => '', 'offset' => '0'], + 'mappedUsers' => 123, + ]], + ]; + } + + /** + * @dataProvider runDataProvider + */ + public function testRun($runData) { + $this->config->expects($this->any()) + ->method('getAppValue') + ->willReturnCallback(function($app, $key, $default) use ($runData) { + if($app === 'core' && $key === 'backgroundjobs_mode') { + return 'cron'; + } + if($app = 'user_ldap') { + // for getCycle() + if($key === 'background_sync_prefix') { + return $runData['scheduledCycle']['prefix']; + } + if($key === 'background_sync_offset') { + return $runData['scheduledCycle']['offset']; + } + // for qualifiesToRun() + if($key === $runData['scheduledCycle']['prefix'] . '_lastChange') { + return time() - 60*40; + } + // for getMinPagingSize + if($key === $runData['scheduledCycle']['prefix'] . 'ldap_paging_size') { + return $runData['pagingSize']; + } + } + + return $default; + }); + $this->config->expects($this->exactly(3)) + ->method('setAppValue') + ->withConsecutive( + ['user_ldap', 'background_sync_prefix', $runData['expectedNextCycle']['prefix']], + ['user_ldap', 'background_sync_offset', $runData['expectedNextCycle']['offset']], + ['user_ldap', 'background_sync_interval', $this->anything()] + ); + $this->config->expects($this->any()) + ->method('getAppKeys') + ->with('user_ldap') + ->willReturn([$runData['scheduledCycle']['prefix'] . 'ldap_paging_size']); + + $this->helper->expects($this->any()) + ->method('getServerConfigurationPrefixes') + ->with(true) + ->willReturn($runData['prefixes']); + + $connection = $this->createMock(Connection::class); + $this->connectionFactory->expects($this->any()) + ->method('get') + ->willReturn($connection); + $connection->expects($this->any()) + ->method('__get') + ->willReturnCallback(function ($key) use ($runData) { + if($key === 'ldapPagingSize') { + return $runData['pagingSize']; + } + return null; + }); + + /** @var Access|\PHPUnit_Framework_MockObject_MockObject $access */ + $access = $this->createMock(Access::class); + $this->accessFactory->expects($this->any()) + ->method('get') + ->with($connection) + ->willReturn($access); + + $access->expects($this->once()) + ->method('fetchListOfUsers') + ->willReturn(array_pad([], $runData['usersThisCycle'], 'someUser')); + $access->connection = $connection; + $access->userManager = $this->userManager; + + $this->mapper->expects($this->any()) + ->method('count') + ->willReturn($runData['mappedUsers']); + + $this->sync->run($this->arguments); + } + }