Use BackendUserIterator with uid filter for syncing single user using…

… user:sync
owncloud · May 17, 2020 · 66f26e1 · 66f26e1
1 parent aeaef3b
commit 66f26e1
Show file tree

Hide file tree

Showing 7 changed files with 33 additions and 18 deletions.
diff --git a/changelog/unreleased/37398 b/changelog/unreleased/37398
@@ -0,0 +1,8 @@
+Adjust user:sync --uid to use user backend iterator
+
+It fixes the behavior for user:sync --uid that attempts to retrieve all
+user backend users without limit at offset, that is not supported by LDAP backend.
+Instead, proper iterator and search query has been used
+
+https://github.com/owncloud/core/pull/37398
+https://github.com/owncloud/enterprise/issues/3981
diff --git a/core/Command/User/SyncBackend.php b/core/Command/User/SyncBackend.php
@@ -24,7 +24,7 @@
 
 use OC\User\Account;
 use OC\User\AccountMapper;
-use OC\User\Sync\AllUsersIterator;
+use OC\User\Sync\BackendUsersIterator;
 use OC\User\Sync\SeenUsersIterator;
 use OC\User\SyncService;
 use OCP\IConfig;
@@ -214,7 +214,7 @@ private function syncMultipleUsers(
 			$iterator = new SeenUsersIterator($this->accountMapper, $backendClass);
 		} else {
 			$output->writeln("Inserting new and updating all known users from $backendClass ...");
-			$iterator = new AllUsersIterator($backend);
+			$iterator = new BackendUsersIterator($backend);
 		}
 
 		$p = new ProgressBar($output);
@@ -251,10 +251,11 @@ private function syncSingleUser(
 		$uid,
 		$missingAccountsAction
 	) {
-		$output->writeln("Syncing $uid ...");
-		$userUids = $backend->getUsers('', null);
+		$output->writeln("Searching for $uid ...");
+
+		$iterator = new BackendUsersIterator($backend, $uid);
 		$userToSync = null;
-		foreach ($userUids as $userUid) {
+		foreach ($iterator as $userUid) {
 			if ($userUid === $uid) {
 				if ($userToSync === null) {
 					$userToSync = $userUid;
@@ -268,9 +269,11 @@ private function syncSingleUser(
 
 		if ($userToSync !== null) {
 			// Run the sync using the internal username if mapped
+			$output->writeln("Syncing $uid ...");
 			$syncService->run($backend, new \ArrayIterator([$userToSync]));
 		} else {
 			// Not found
+			$output->writeln("Exact match for user $uid not found in the backend.");
 			$this->handleRemovedUsers([$uid => $dummy], $input, $output, $missingAccountsAction);
 		}
 

diff --git a/core/Migrations/Version20170221114437.php b/core/Migrations/Version20170221114437.php
@@ -4,7 +4,7 @@
 use OC\User\AccountMapper;
 use OC\User\AccountTermMapper;
 use OC\User\Database;
-use OC\User\Sync\AllUsersIterator;
+use OC\User\Sync\BackendUsersIterator;
 use OC\User\SyncService;
 use OCP\Migration\ISimpleMigration;
 use OCP\Migration\IOutput;
@@ -25,7 +25,7 @@ public function run(IOutput $out) {
 		// insert/update known users
 		$out->info("Insert new users ...");
 		$out->startProgress($backend->countUsers());
-		$syncService->run($backend, new AllUsersIterator($backend), function () use ($out) {
+		$syncService->run($backend, new BackendUsersIterator($backend), function () use ($out) {
 			$out->advance();
 		});
 		$out->finishProgress();

diff --git a/lib/private/User/Sync/AllUsersIterator.php → ...rivate/User/Sync/BackendUsersIterator.php b/lib/private/User/Sync/AllUsersIterator.php → ...rivate/User/Sync/BackendUsersIterator.php
@@ -22,7 +22,7 @@
 
 use OCP\UserInterface;
 
-class AllUsersIterator extends UsersIterator {
+class BackendUsersIterator extends UsersIterator {
 	/**
 	 * @var UserInterface
 	 */
@@ -41,13 +41,17 @@ class AllUsersIterator extends UsersIterator {
 	/** @var bool false if the backend returned less than LIMIT results */
 	private $hasMoreData = false;
 
-	public function __construct(UserInterface $backend) {
+	/** @var string search for the uid string in backend */
+	private $search;
+
+	public function __construct(UserInterface $backend, $filterUID = '') {
 		$this->backend = $backend;
+		$this->search = $filterUID;
 	}
 
 	public function rewind() {
 		parent::rewind();
-		$this->data = $this->backend->getUsers('', self::LIMIT, 0);
+		$this->data = $this->backend->getUsers($this->search, self::LIMIT, 0);
 		$this->dataPos = 0;
 		$this->endPos = \count($this->data);
 		$this->hasMoreData = $this->endPos >= self::LIMIT;
@@ -59,7 +63,7 @@ public function next() {
 		if ($this->hasMoreData && $this->dataPos >= $this->endPos) {
 			$this->page++;
 			$offset = $this->page * self::LIMIT;
-			$this->data = $this->backend->getUsers('', self::LIMIT, $offset);
+			$this->data = $this->backend->getUsers($this->search, self::LIMIT, $offset);
 			$this->dataPos = 0;
 			$this->endPos = \count($this->data);
 			$this->hasMoreData = $this->endPos >= self::LIMIT;

diff --git a/tests/lib/Command/User/SyncBackendTest.php b/tests/lib/Command/User/SyncBackendTest.php
@@ -213,7 +213,7 @@ public function testInvalidAccountActionShowsError() {
 
 	public function executeProvider() {
 		return [
-			['foo', 'Syncing foo ...'],
+			['foo', 'Searching for foo ...'],
 			[null, 'Analysing known accounts ...'],
 		];
 	}

diff --git a/tests/lib/User/Sync/AllUsersIteratorTest.php → ...ib/User/Sync/BackendUsersIteratorTest.php b/tests/lib/User/Sync/AllUsersIteratorTest.php → ...ib/User/Sync/BackendUsersIteratorTest.php
@@ -25,13 +25,13 @@
 use Test\TestCase;
 
 /**
- * Class AllUsersIteratorTest
+ * Class BackendUsersIteratorTest
  *
  * @package OC\User\Sync
  *
  * @see http://php.net/manual/en/class.iterator.php for the order of calls on an iterator
  */
-class AllUsersIteratorTest extends TestCase {
+class BackendUsersIteratorTest extends TestCase {
 
 	/**
 	 * @var UserInterface|\PHPUnit\Framework\MockObject\MockObject
@@ -47,7 +47,7 @@ public function setUp(): void {
 
 		$this->backend = $this->createMock(UserInterface::class);
 
-		$this->iterator = new AllUsersIterator($this->backend);
+		$this->iterator = new BackendUsersIterator($this->backend);
 	}
 
 	/**

diff --git a/tests/lib/User/SyncServiceTest.php b/tests/lib/User/SyncServiceTest.php
@@ -26,7 +26,7 @@
 use OC\User\Account;
 use OC\User\AccountMapper;
 use OC\User\Database;
-use OC\User\Sync\AllUsersIterator;
+use OC\User\Sync\BackendUsersIterator;
 use OC\User\SyncService;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Db\MultipleObjectsReturnedException;
@@ -119,7 +119,7 @@ public function testSetupNewAccount() {
 		// Ignore state flag
 
 		$s = new SyncService($this->config, $this->logger, $this->mapper);
-		$s->run($backend, new AllUsersIterator($backend));
+		$s->run($backend, new BackendUsersIterator($backend));
 
 		static::invokePrivate($s, 'syncHome', [$account, $backend]);
 	}
@@ -142,7 +142,7 @@ public function testSetupNewAccountLogsErrorOnException() {
 		$this->logger->expects($this->at(1))->method('logException');
 
 		$s = new SyncService($this->config, $this->logger, $this->mapper);
-		$s->run($backend, new AllUsersIterator($backend));
+		$s->run($backend, new BackendUsersIterator($backend));
 	}
 
 	public function testSyncHomeLogsWhenBackendDiffersFromExisting() {