From ef3cd329167ce236b7f7a09293be439623e585c5 Mon Sep 17 00:00:00 2001 From: Arthur Schiwon Date: Tue, 31 Oct 2017 18:13:48 +0100 Subject: [PATCH] don't skip updating when ajax is set as background job mode Signed-off-by: Arthur Schiwon --- apps/user_ldap/lib/Access.php | 17 +++- apps/user_ldap/lib/Jobs/UpdateGroups.php | 2 +- apps/user_ldap/lib/Proxy.php | 2 +- apps/user_ldap/tests/AccessTest.php | 78 ++++++++++++++++++- .../Integration/AbstractIntegrationTest.php | 2 +- 5 files changed, 91 insertions(+), 10 deletions(-) diff --git a/apps/user_ldap/lib/Access.php b/apps/user_ldap/lib/Access.php index 989a941f58..012d9a4712 100644 --- a/apps/user_ldap/lib/Access.php +++ b/apps/user_ldap/lib/Access.php @@ -51,6 +51,7 @@ use OCA\User_LDAP\User\OfflineUser; use OCA\User_LDAP\Mapping\AbstractMapping; use OC\ServerNotAvailableException; +use OCP\IServerContainer; /** * Class Access @@ -89,14 +90,22 @@ class Access extends LDAPUtility implements IUserTools { * @var \OCA\User_LDAP\Helper */ private $helper; + /** @var IServerContainer */ + private $c; - public function __construct(Connection $connection, ILDAPWrapper $ldap, - Manager $userManager, Helper $helper) { + public function __construct( + Connection $connection, + ILDAPWrapper $ldap, + Manager $userManager, + Helper $helper, + IServerContainer $c + ) { parent::__construct($ldap); $this->connection = $connection; $this->userManager = $userManager; $this->userManager->setLdapAccess($this); $this->helper = $helper; + $this->c = $c; } /** @@ -821,6 +830,8 @@ class Access extends LDAPUtility implements IUserTools { */ public function batchApplyUserAttributes(array $ldapRecords){ $displayNameAttribute = strtolower($this->connection->ldapUserDisplayName); + $config = $this->c->getConfig(); + $isBackgroundJobModeAjax = $config->getAppValue('core', 'backgroundjobs_mode', 'ajax') === 'ajax'; foreach($ldapRecords as $userRecord) { if(!isset($userRecord[$displayNameAttribute])) { // displayName is obligatory @@ -828,7 +839,7 @@ class Access extends LDAPUtility implements IUserTools { } $newlyMapped = false; $ocName = $this->dn2ocname($userRecord['dn'][0], null, true, $newlyMapped); - if($ocName === false || $newlyMapped === false) { + if($ocName === false || ($newlyMapped === false && !$isBackgroundJobModeAjax)) { continue; } $this->cacheUserExists($ocName); diff --git a/apps/user_ldap/lib/Jobs/UpdateGroups.php b/apps/user_ldap/lib/Jobs/UpdateGroups.php index 49b79f2d61..42579ec078 100644 --- a/apps/user_ldap/lib/Jobs/UpdateGroups.php +++ b/apps/user_ldap/lib/Jobs/UpdateGroups.php @@ -192,7 +192,7 @@ class UpdateGroups extends \OC\BackgroundJob\TimedJob { \OC::$server->getUserManager(), \OC::$server->getNotificationManager()); $connector = new Connection($ldapWrapper, $configPrefixes[0]); - $ldapAccess = new Access($connector, $ldapWrapper, $userManager, $helper); + $ldapAccess = new Access($connector, $ldapWrapper, $userManager, $helper, \OC::$server); $groupMapper = new GroupMapping($dbc); $userMapper = new UserMapping($dbc); $ldapAccess->setGroupMapper($groupMapper); diff --git a/apps/user_ldap/lib/Proxy.php b/apps/user_ldap/lib/Proxy.php index ec075d761c..0317d33c2c 100644 --- a/apps/user_ldap/lib/Proxy.php +++ b/apps/user_ldap/lib/Proxy.php @@ -82,7 +82,7 @@ abstract class Proxy { new Manager($ocConfig, $fs, $log, $avatarM, new \OCP\Image(), $db, $coreUserManager, $coreNotificationManager); $connector = new Connection($this->ldap, $configPrefix); - $access = new Access($connector, $this->ldap, $userManager, new Helper(\OC::$server->getConfig())); + $access = new Access($connector, $this->ldap, $userManager, new Helper(\OC::$server->getConfig()), \OC::$server); $access->setUserMapper($userMap); $access->setGroupMapper($groupMap); self::$accesses[$configPrefix] = $access; diff --git a/apps/user_ldap/tests/AccessTest.php b/apps/user_ldap/tests/AccessTest.php index 9f4e8db35a..047fe7c3fb 100644 --- a/apps/user_ldap/tests/AccessTest.php +++ b/apps/user_ldap/tests/AccessTest.php @@ -48,6 +48,7 @@ use OCP\IAvatarManager; use OCP\IConfig; use OCP\IDBConnection; use OCP\Image; +use OCP\IServerContainer; use OCP\IUserManager; use OCP\Notification\IManager as INotificationManager; use Test\TestCase; @@ -68,6 +69,8 @@ class AccessTest extends TestCase { private $userManager; /** @var Helper|\PHPUnit_Framework_MockObject_MockObject */ private $helper; + /** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject */ + private $c; /** @var Access */ private $access; @@ -76,12 +79,14 @@ class AccessTest extends TestCase { $this->ldap = $this->createMock(LDAP::class); $this->userManager = $this->createMock(Manager::class); $this->helper = $this->createMock(Helper::class); + $this->c = $this->createMock(IServerContainer::class); $this->access = new Access( $this->connection, $this->ldap, $this->userManager, - $this->helper + $this->helper, + $this->c ); } @@ -216,7 +221,9 @@ class AccessTest extends TestCase { */ public function testStringResemblesDN($case) { list($lw, $con, $um, $helper) = $this->getConnectorAndLdapMock(); - $access = new Access($con, $lw, $um, $helper); + /** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject $c */ + $c = $this->createMock(IServerContainer::class); + $access = new Access($con, $lw, $um, $helper, $c); $lw->expects($this->exactly(1)) ->method('explodeDN') @@ -236,8 +243,10 @@ class AccessTest extends TestCase { */ public function testStringResemblesDNLDAPmod($case) { list(, $con, $um, $helper) = $this->getConnectorAndLdapMock(); + /** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject $c */ + $c = $this->createMock(IServerContainer::class); $lw = new LDAP(); - $access = new Access($con, $lw, $um, $helper); + $access = new Access($con, $lw, $um, $helper, $c); if(!function_exists('ldap_explode_dn')) { $this->markTestSkipped('LDAP Module not available'); @@ -303,6 +312,10 @@ class AccessTest extends TestCase { ->method('get') ->will($this->returnValue($userMock)); + $this->c->expects($this->any()) + ->method('getConfig') + ->willReturn($this->createMock(IConfig::class)); + $this->access->batchApplyUserAttributes($data); } @@ -343,6 +356,61 @@ class AccessTest extends TestCase { $this->userManager->expects($this->never()) ->method('get'); + $this->c->expects($this->any()) + ->method('getConfig') + ->willReturn($this->createMock(IConfig::class)); + + $this->access->batchApplyUserAttributes($data); + } + + public function testBatchApplyUserAttributesDontSkip() { + /** @var UserMapping|\PHPUnit_Framework_MockObject_MockObject $mapperMock */ + $mapperMock = $this->createMock(UserMapping::class); + $mapperMock->expects($this->any()) + ->method('getNameByDN') + ->will($this->returnValue('a_username')); + + $userMock = $this->createMock(User::class); + + $this->access->connection->expects($this->any()) + ->method('__get') + ->will($this->returnValue('displayName')); + + $this->access->setUserMapper($mapperMock); + + $displayNameAttribute = strtolower($this->access->connection->ldapUserDisplayName); + $data = [ + [ + 'dn' => ['foobar'], + $displayNameAttribute => 'barfoo' + ], + [ + 'dn' => ['foo'], + $displayNameAttribute => 'bar' + ], + [ + 'dn' => ['raboof'], + $displayNameAttribute => 'oofrab' + ] + ]; + + $userMock->expects($this->exactly(count($data))) + ->method('processAttributes'); + + $this->userManager->expects($this->exactly(count($data))) + ->method('get') + ->will($this->returnValue($userMock)); + + $configMock = $this->createMock(IConfig::class); + $configMock->expects($this->once()) + ->method('getAppValue') + ->with('core', 'backgroundjobs_mode', $this->anything()) + ->willReturn('ajax'); + + $this->c->expects($this->any()) + ->method('getConfig') + ->willReturn($configMock); + $this->access->batchApplyUserAttributes($data); } @@ -362,6 +430,8 @@ class AccessTest extends TestCase { */ public function testSanitizeDN($attribute) { list($lw, $con, $um, $helper) = $this->getConnectorAndLdapMock(); + /** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject $c */ + $c = $this->createMock(IServerContainer::class); $dnFromServer = 'cn=Mixed Cases,ou=Are Sufficient To,ou=Test,dc=example,dc=org'; @@ -374,7 +444,7 @@ class AccessTest extends TestCase { $attribute => array('count' => 1, $dnFromServer) ))); - $access = new Access($con, $lw, $um, $helper); + $access = new Access($con, $lw, $um, $helper, $c); $values = $access->readAttribute('uid=whoever,dc=example,dc=org', $attribute); $this->assertSame($values[0], strtolower($dnFromServer)); } diff --git a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php index 3bc3699fba..64bd2df388 100644 --- a/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php +++ b/apps/user_ldap/tests/Integration/AbstractIntegrationTest.php @@ -140,7 +140,7 @@ abstract class AbstractIntegrationTest { * initializes the Access test instance */ protected function initAccess() { - $this->access = new Access($this->connection, $this->ldap, $this->userManager, $this->helper); + $this->access = new Access($this->connection, $this->ldap, $this->userManager, $this->helper, \OC::$server); } /**