Merge pull request #50171 from nextcloud/enh/limit-ldap-user-count

Limit ldap user count
nextcloud · Jan 16, 2025 · 626bc72 · 626bc72
2 parents 8b46ec2 + 892f815
commit 626bc72
Show file tree

Hide file tree

Showing 15 changed files with 106 additions and 204 deletions.
diff --git a/apps/dav/lib/Migration/Version1027Date20230504122946.php b/apps/dav/lib/Migration/Version1027Date20230504122946.php
@@ -33,7 +33,7 @@ public function __construct(
 	 * @param array $options
 	 */
 	public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void {
-		if ($this->userManager->countSeenUsers() > 100 || array_sum($this->userManager->countUsers()) > 100) {
+		if ($this->userManager->countSeenUsers() > 100 || $this->userManager->countUsersTotal(100) >= 100) {
 			$this->config->setAppValue('dav', 'needs_system_address_book_sync', 'yes');
 			$output->info('Could not sync system address books during update - too many user records have been found. Please call occ dav:sync-system-addressbook manually.');
 			return;

diff --git a/apps/updatenotification/lib/Settings/Admin.php b/apps/updatenotification/lib/Settings/Admin.php
@@ -8,7 +8,6 @@
  */
 namespace OCA\UpdateNotification\Settings;
 
-use OC\User\Backend;
 use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
@@ -20,7 +19,6 @@
 use OCP\L10N\IFactory;
 use OCP\Settings\ISettings;
 use OCP\Support\Subscription\IRegistry;
-use OCP\User\Backend\ICountUsersBackend;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
 
@@ -141,26 +139,6 @@ public function getPriority(): int {
 	}
 
 	private function isWebUpdaterRecommended(): bool {
-		return $this->getUserCount() < 100;
-	}
-
-	/**
-	 * @see https://github.com/nextcloud/server/blob/39494fbf794d982f6f6551c984e6ca4c4e947d01/lib/private/Support/Subscription/Registry.php#L188-L216 implementation reference
-	 */
-	private function getUserCount(): int {
-		$userCount = 0;
-		$backends = $this->userManager->getBackends();
-		foreach ($backends as $backend) {
-			// TODO: change below to 'if ($backend instanceof ICountUsersBackend) {'
-			if ($backend->implementsActions(Backend::COUNT_USERS)) {
-				/** @var ICountUsersBackend $backend */
-				$backendUsers = $backend->countUsers();
-				if ($backendUsers !== false) {
-					$userCount += $backendUsers;
-				}
-			}
-		}
-
-		return $userCount;
+		return (int)$this->userManager->countUsersTotal(100) < 100;
 	}
 }
diff --git a/apps/updatenotification/tests/Settings/AdminTest.php b/apps/updatenotification/tests/Settings/AdminTest.php
@@ -8,7 +8,6 @@
  */
 namespace OCA\UpdateNotification\Tests\Settings;
 
-use OC\User\Backend;
 use OCA\UpdateNotification\Settings\Admin;
 use OCA\UpdateNotification\UpdateChecker;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -22,8 +21,6 @@
 use OCP\L10N\IFactory;
 use OCP\L10N\ILanguageIterator;
 use OCP\Support\Subscription\IRegistry;
-use OCP\User\Backend\ICountUsersBackend;
-use OCP\UserInterface;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -81,42 +78,10 @@ protected function setUp(): void {
 	}
 
 	public function testGetFormWithUpdate(): void {
-		$backend1 = $this->createMock(CountUsersBackend::class);
-		$backend2 = $this->createMock(CountUsersBackend::class);
-		$backend3 = $this->createMock(CountUsersBackend::class);
-		$backend1
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(false);
-		$backend2
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend3
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend1
-			->expects($this->never())
-			->method('countUsers');
-		$backend2
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(false);
-		$backend3
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(5);
 		$this->userManager
 			->expects($this->once())
-			->method('getBackends')
-			->with()
-			->willReturn([$backend1, $backend2, $backend3]);
+			->method('countUsersTotal')
+			->willReturn(5);
 		$channels = [
 			'daily',
 			'beta',
@@ -207,42 +172,10 @@ public function testGetFormWithUpdate(): void {
 	}
 
 	public function testGetFormWithUpdateAndChangedUpdateServer(): void {
-		$backend1 = $this->createMock(CountUsersBackend::class);
-		$backend2 = $this->createMock(CountUsersBackend::class);
-		$backend3 = $this->createMock(CountUsersBackend::class);
-		$backend1
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(false);
-		$backend2
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend3
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend1
-			->expects($this->never())
-			->method('countUsers');
-		$backend2
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(false);
-		$backend3
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(5);
 		$this->userManager
 			->expects($this->once())
-			->method('getBackends')
-			->with()
-			->willReturn([$backend1, $backend2, $backend3]);
+			->method('countUsersTotal')
+			->willReturn(5);
 		$channels = [
 			'daily',
 			'beta',
@@ -334,42 +267,10 @@ public function testGetFormWithUpdateAndChangedUpdateServer(): void {
 	}
 
 	public function testGetFormWithUpdateAndCustomersUpdateServer(): void {
-		$backend1 = $this->createMock(CountUsersBackend::class);
-		$backend2 = $this->createMock(CountUsersBackend::class);
-		$backend3 = $this->createMock(CountUsersBackend::class);
-		$backend1
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(false);
-		$backend2
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend3
-			->expects($this->once())
-			->method('implementsActions')
-			->with(Backend::COUNT_USERS)
-			->willReturn(true);
-		$backend1
-			->expects($this->never())
-			->method('countUsers');
-		$backend2
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(false);
-		$backend3
-			->expects($this->once())
-			->method('countUsers')
-			->with()
-			->willReturn(5);
 		$this->userManager
 			->expects($this->once())
-			->method('getBackends')
-			->with()
-			->willReturn([$backend1, $backend2, $backend3]);
+			->method('countUsersTotal')
+			->willReturn(5);
 		$channels = [
 			'daily',
 			'beta',
@@ -543,7 +444,3 @@ public function testFilterChanges($changes, $userLang, $expectation): void {
 		$this->assertSame($expectation, $result);
 	}
 }
-
-abstract class CountUsersBackend implements UserInterface, ICountUsersBackend {
-
-}
diff --git a/apps/user_ldap/lib/User_LDAP.php b/apps/user_ldap/lib/User_LDAP.php
@@ -17,12 +17,12 @@
 use OCP\IUserBackend;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
 use OCP\UserInterface;
 use Psr\Log\LoggerInterface;
 
-class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ICountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
+class User_LDAP extends BackendUtility implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
 	public function __construct(
 		Access $access,
 		protected INotificationManager $notificationManager,
@@ -528,20 +528,18 @@ public function hasUserListings() {
 
 	/**
 	 * counts the users in LDAP
-	 *
-	 * @return int|false
 	 */
-	public function countUsers() {
+	public function countUsers(int $limit = 0): int|false {
 		if ($this->userPluginManager->implementsActions(Backend::COUNT_USERS)) {
 			return $this->userPluginManager->countUsers();
 		}
 
 		$filter = $this->access->getFilterForUserCount();
-		$cacheKey = 'countUsers-' . $filter;
+		$cacheKey = 'countUsers-' . $filter . '-' . $limit;
 		if (!is_null($entries = $this->access->connection->getFromCache($cacheKey))) {
 			return $entries;
 		}
-		$entries = $this->access->countUsers($filter);
+		$entries = $this->access->countUsers($filter, limit:$limit);
 		$this->access->connection->writeToCache($cacheKey, $entries);
 		return $entries;
 	}

diff --git a/apps/user_ldap/lib/User_Proxy.php b/apps/user_ldap/lib/User_Proxy.php
@@ -13,12 +13,12 @@
 use OCP\IUserBackend;
 use OCP\Notification\IManager as INotificationManager;
 use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IProvideEnabledStateBackend;
 use OCP\UserInterface;
 use Psr\Log\LoggerInterface;
 
-class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ICountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
+class User_Proxy extends Proxy implements IUserBackend, UserInterface, IUserLDAP, ILimitAwareCountUsersBackend, ICountMappedUsersBackend, IProvideEnabledStateBackend {
 	/** @var User_LDAP[] */
 	private array $backends = [];
 	private ?User_LDAP $refBackend = null;
@@ -350,17 +350,21 @@ public function hasUserListings() {
 
 	/**
 	 * Count the number of users
-	 *
-	 * @return int|false
 	 */
-	public function countUsers() {
+	public function countUsers(int $limit = 0): int|false {
 		$this->setup();
 
 		$users = false;
 		foreach ($this->backends as $backend) {
-			$backendUsers = $backend->countUsers();
+			$backendUsers = $backend->countUsers($limit);
 			if ($backendUsers !== false) {
 				$users = (int)$users + $backendUsers;
+				if ($limit > 0) {
+					if ($users >= $limit) {
+						break;
+					}
+					$limit -= $users;
+				}
 			}
 		}
 		return $users;

diff --git a/lib/base.php b/lib/base.php
@@ -279,8 +279,7 @@ private static function printUpgradePage(\OC\SystemConfig $systemConfig): void {
 			}
 			if (!$tooBig) {
 				// count users
-				$stats = Server::get(\OCP\IUserManager::class)->countUsers();
-				$totalUsers = array_sum($stats);
+				$totalUsers = Server::get(\OCP\IUserManager::class)->countUsersTotal(51);
 				$tooBig = ($totalUsers > 50);
 			}
 		}

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -892,6 +892,7 @@
     'OCP\\User\\Backend\\IGetDisplayNameBackend' => $baseDir . '/lib/public/User/Backend/IGetDisplayNameBackend.php',
     'OCP\\User\\Backend\\IGetHomeBackend' => $baseDir . '/lib/public/User/Backend/IGetHomeBackend.php',
     'OCP\\User\\Backend\\IGetRealUIDBackend' => $baseDir . '/lib/public/User/Backend/IGetRealUIDBackend.php',
+    'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => $baseDir . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php',
     'OCP\\User\\Backend\\IPasswordConfirmationBackend' => $baseDir . '/lib/public/User/Backend/IPasswordConfirmationBackend.php',
     'OCP\\User\\Backend\\IPasswordHashBackend' => $baseDir . '/lib/public/User/Backend/IPasswordHashBackend.php',
     'OCP\\User\\Backend\\IProvideAvatarBackend' => $baseDir . '/lib/public/User/Backend/IProvideAvatarBackend.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -933,6 +933,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\User\\Backend\\IGetDisplayNameBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetDisplayNameBackend.php',
         'OCP\\User\\Backend\\IGetHomeBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetHomeBackend.php',
         'OCP\\User\\Backend\\IGetRealUIDBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IGetRealUIDBackend.php',
+        'OCP\\User\\Backend\\ILimitAwareCountUsersBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/ILimitAwareCountUsersBackend.php',
         'OCP\\User\\Backend\\IPasswordConfirmationBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordConfirmationBackend.php',
         'OCP\\User\\Backend\\IPasswordHashBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IPasswordHashBackend.php',
         'OCP\\User\\Backend\\IProvideAvatarBackend' => __DIR__ . '/../../..' . '/lib/public/User/Backend/IProvideAvatarBackend.php',

diff --git a/lib/private/Support/Subscription/Registry.php b/lib/private/Support/Subscription/Registry.php
@@ -8,7 +8,6 @@
  */
 namespace OC\Support\Subscription;
 
-use OC\User\Backend;
 use OCP\AppFramework\QueryException;
 use OCP\IConfig;
 use OCP\IGroupManager;
@@ -19,8 +18,6 @@
 use OCP\Support\Subscription\IRegistry;
 use OCP\Support\Subscription\ISubscription;
 use OCP\Support\Subscription\ISupportedApps;
-use OCP\User\Backend\ICountMappedUsersBackend;
-use OCP\User\Backend\ICountUsersBackend;
 use Psr\Log\LoggerInterface;
 
 class Registry implements IRegistry {
@@ -167,22 +164,8 @@ public function delegateIsHardUserLimitReached(?IManager $notificationManager =
 	}
 
 	private function getUserCount(): int {
-		$userCount = 0;
-		$backends = $this->userManager->getBackends();
-		foreach ($backends as $backend) {
-			if ($backend instanceof ICountMappedUsersBackend) {
-				$userCount += $backend->countMappedUsers();
-			} elseif ($backend->implementsActions(Backend::COUNT_USERS)) {
-				/** @var ICountUsersBackend $backend */
-				$backendUsers = $backend->countUsers();
-				if ($backendUsers !== false) {
-					$userCount += $backendUsers;
-				} else {
-					// TODO what if the user count can't be determined?
-					$this->logger->warning('Can not determine user count for ' . get_class($backend), ['app' => 'lib']);
-				}
-			}
-		}
+		/* We cannot limit because we substract disabled users afterward. But we limit to mapped users so should be not too expensive. */
+		$userCount = (int)$this->userManager->countUsersTotal(0, true);
 
 		$disabledUsers = $this->config->getUsersForUserValue('core', 'enabled', 'false');
 		$disabledUsersCount = count($disabledUsers);

diff --git a/lib/private/User/Database.php b/lib/private/User/Database.php
@@ -17,11 +17,11 @@
 use OCP\Security\IHasher;
 use OCP\User\Backend\ABackend;
 use OCP\User\Backend\ICheckPasswordBackend;
-use OCP\User\Backend\ICountUsersBackend;
 use OCP\User\Backend\ICreateUserBackend;
 use OCP\User\Backend\IGetDisplayNameBackend;
 use OCP\User\Backend\IGetHomeBackend;
 use OCP\User\Backend\IGetRealUIDBackend;
+use OCP\User\Backend\ILimitAwareCountUsersBackend;
 use OCP\User\Backend\IPasswordHashBackend;
 use OCP\User\Backend\ISearchKnownUsersBackend;
 use OCP\User\Backend\ISetDisplayNameBackend;
@@ -37,7 +37,7 @@ class Database extends ABackend implements
 	IGetDisplayNameBackend,
 	ICheckPasswordBackend,
 	IGetHomeBackend,
-	ICountUsersBackend,
+	ILimitAwareCountUsersBackend,
 	ISearchKnownUsersBackend,
 	IGetRealUIDBackend,
 	IPasswordHashBackend {
@@ -463,10 +463,8 @@ public function hasUserListings() {
 
 	/**
 	 * counts the users in the database
-	 *
-	 * @return int|false
 	 */
-	public function countUsers() {
+	public function countUsers(int $limit = 0): int|false {
 		$this->fixDI();
 
 		$query = $this->dbConn->getQueryBuilder();