fix(provisioning_api): cap group listings and throttle group creation

apps/provisioning_api/lib/Controller/GroupsController.php

@@ -19,6 +19,7 @@
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\NoSubAdminRequired;
 use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
+use OCP\AppFramework\Http\Attribute\UserRateLimit;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCS\OCSException;
 use OCP\AppFramework\OCS\OCSForbiddenException;
@@ -42,6 +43,8 @@
  * @psalm-import-type Provisioning_APIUserDetailsGroupDisplayname from ResponseDefinitions
  */
 class GroupsController extends AUserDataOCSController {
+	public const int MAX_SEARCH_RESULTS = 500;
+
 
 	public function __construct(
 		string $appName,
@@ -75,15 +78,15 @@ public function __construct(
 	 * Get a list of groups
 	 *
 	 * @param string $search Text to search for
-	 * @param ?int $limit Limit the amount of groups returned
+	 * @param ?int<1, 500> $limit Limit the amount of groups returned, defaults to 500
 	 * @param int $offset Offset for searching for groups
 	 * @return DataResponse<Http::STATUS_OK, array{groups: list<string>}, array{}>
 	 *
 	 * 200: Groups returned
 	 */
 	#[NoAdminRequired]
 	public function getGroups(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
-		$groups = $this->groupManager->search($search, $limit, $offset);
+		$groups = $this->searchGroups($search, $limit, $offset);
 		$groups = array_map(function ($group) {
 			/** @var IGroup $group */
 			return $group->getGID();
@@ -96,7 +99,7 @@ public function getGroups(string $search = '', ?int $limit = null, int $offset =
 	 * Get a list of groups details
 	 *
 	 * @param string $search Text to search for
-	 * @param ?int $limit Limit the amount of groups returned
+	 * @param ?int<1, 500> $limit Limit the amount of groups returned, defaults to 500
 	 * @param int $offset Offset for searching for groups
 	 * @return DataResponse<Http::STATUS_OK, array{groups: list<Provisioning_APIGroupDetails>}, array{}>
 	 *
@@ -106,7 +109,7 @@ public function getGroups(string $search = '', ?int $limit = null, int $offset =
 	#[AuthorizedAdminSetting(settings: Sharing::class)]
 	#[AuthorizedAdminSetting(settings: Users::class)]
 	public function getGroupsDetails(string $search = '', ?int $limit = null, int $offset = 0): DataResponse {
-		$groups = $this->groupManager->search($search, $limit, $offset);
+		$groups = $this->searchGroups($search, $limit, $offset);
 		$groups = array_map(function ($group) {
 			/** @var IGroup $group */
 			return [
@@ -122,6 +125,17 @@ public function getGroupsDetails(string $search = '', ?int $limit = null, int $o
 		return new DataResponse(['groups' => $groups]);
 	}
 
+	/**
+	 * @return list<IGroup>
+	 */
+	private function searchGroups(string $search, ?int $limit, int $offset): array {
+		if ($limit === null || $limit <= 0 || $limit > self::MAX_SEARCH_RESULTS) {
+			$limit = self::MAX_SEARCH_RESULTS;
+		}
+
+		return $this->groupManager->search($search, $limit, $offset);
+	}
+
 	/**
 	 * Get a list of users in the specified group
 	 *
@@ -254,6 +268,7 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
 	 * 200: Group created successfully
 	 */
 	#[AuthorizedAdminSetting(settings:Users::class)]
+	#[UserRateLimit(limit: 50, period: 600)]
 	#[PasswordConfirmationRequired]
 	public function addGroup(string $groupid, string $displayname = ''): DataResponse {
 		// Validate name

apps/provisioning_api/openapi-full.json

@@ -1245,12 +1245,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {
@@ -3186,12 +3188,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {

apps/provisioning_api/openapi.json

@@ -456,12 +456,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {
@@ -586,12 +588,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {

apps/provisioning_api/tests/Controller/GroupsControllerTest.php

@@ -157,46 +157,43 @@ private function asSubAdminOfGroup($group) {
 
 	public static function dataGetGroups(): array {
 		return [
-			[null, 0, 0],
-			['foo', 0, 0],
-			[null, 1, 0],
-			[null, 0, 2],
-			['foo', 1, 2],
+			// search, requested limit, offset, expected forwarded limit
+			[null, null, 0, GroupsController::MAX_SEARCH_RESULTS],
+			['foo', null, 0, GroupsController::MAX_SEARCH_RESULTS],
+			[null, 0, 0, GroupsController::MAX_SEARCH_RESULTS],
+			[null, 1, 0, 1],
+			[null, 10, 2, 10],
+			['foo', 1, 2, 1],
+			[null, GroupsController::MAX_SEARCH_RESULTS + 1, 0, GroupsController::MAX_SEARCH_RESULTS],
 		];
 	}
 
 	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataGetGroups')]
-	public function testGetGroups(?string $search, int $limit, int $offset): void {
+	public function testGetGroups(?string $search, ?int $limit, int $offset, int $expectedLimit): void {
 		$groups = [$this->createGroup('group1'), $this->createGroup('group2')];
 
 		$search = $search === null ? '' : $search;
 
 		$this->groupManager
 			->expects($this->once())
 			->method('search')
-			->with($search, $limit, $offset)
+			->with($search, $expectedLimit, $offset)
 			->willReturn($groups);
 
 		$result = $this->api->getGroups($search, $limit, $offset);
 		$this->assertEquals(['groups' => ['group1', 'group2']], $result->getData());
 	}
 
-	/**
-	 *
-	 * @param string|null $search
-	 * @param int|null $limit
-	 * @param int|null $offset
-	 */
 	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataGetGroups')]
-	public function testGetGroupsDetails($search, $limit, $offset): void {
+	public function testGetGroupsDetails(?string $search, ?int $limit, int $offset, int $expectedLimit): void {
 		$groups = [$this->createGroup('group1'), $this->createGroup('group2')];
 
 		$search = $search === null ? '' : $search;
 
 		$this->groupManager
 			->expects($this->once())
 			->method('search')
-			->with($search, $limit, $offset)
+			->with($search, $expectedLimit, $offset)
 			->willReturn($groups);
 
 		$result = $this->api->getGroupsDetails($search, $limit, $offset);
@@ -220,6 +217,29 @@ public function testGetGroupsDetails($search, $limit, $offset): void {
 		]], $result->getData());
 	}
 
+	public static function dataCapsBackendOverflow(): array {
+		return [['getGroups'], ['getGroupsDetails']];
+	}
+
+	/**
+	 * Each backend is searched with the full limit, so the merge can exceed it.
+	 * Truncating here would drop the last backend's groups entirely, so the
+	 * overflow is returned as-is.
+	 */
+	#[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataCapsBackendOverflow')]
+	public function testBackendOverflowIsNotTruncated(string $method): void {
+		$groups = array_fill(0, GroupsController::MAX_SEARCH_RESULTS + 5, $this->createGroup('group'));
+
+		$this->groupManager
+			->expects($this->once())
+			->method('search')
+			->with('', GroupsController::MAX_SEARCH_RESULTS, 0)
+			->willReturn($groups);
+
+		$result = $this->api->$method('', null, 0);
+		$this->assertCount(GroupsController::MAX_SEARCH_RESULTS + 5, $result->getData()['groups']);
+	}
+
 	public function testGetGroupAsSubadmin(): void {
 		$group = $this->createGroup('group');
 		$this->asSubAdminOfGroup($group);

apps/settings/lib/Controller/UsersController.php

@@ -141,14 +141,6 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
 		$canChangePassword = $this->canAdminChangeUserPasswords();
 
 		/* GROUPS */
-		$groupsInfo = new MetaData(
-			$uid,
-			$isAdmin,
-			$isDelegatedAdmin,
-			$this->groupManager,
-			$this->userSession
-		);
-
 		$adminGroup = $this->groupManager->get('admin');
 		$adminGroupData = [
 			'id' => $adminGroup->getGID(),
@@ -167,6 +159,7 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
 
 		$disabledUsers = -1;
 		$userCount = 0;
+		$ownSubAdminGroups = ($isAdmin || $isDelegatedAdmin) ? [] : $subAdmin->getSubAdminsGroups($user);
 
 		if (!$isLDAPUsed) {
 			if ($isAdmin || $isDelegatedAdmin) {
@@ -176,7 +169,7 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
 				}, 0);
 			} else {
 				// User is subadmin !
-				[$userCount,$disabledUsers] = $this->userManager->countUsersAndDisabledUsersOfGroups($groupsInfo->getGroups(), self::COUNT_LIMIT_FOR_SUBADMINS);
+				[$userCount,$disabledUsers] = $this->userManager->countUsersAndDisabledUsersOfGroups($ownSubAdminGroups, self::COUNT_LIMIT_FOR_SUBADMINS);
 			}
 
 			if ($disabledUsers > 0) {
@@ -199,7 +192,7 @@ public function usersList(INavigationManager $navigationManager, ISubAdmin $subA
 		if (!$isAdmin && !$isDelegatedAdmin) {
 			$subAdminGroups = array_map(
 				fn (IGroup $group) => ['id' => $group->getGID(), 'name' => $group->getDisplayName()],
-				$subAdmin->getSubAdminsGroups($user),
+				$ownSubAdminGroups,
 			);
 			$subAdminGroups = array_values($subAdminGroups);
 		}

lib/private/Group/MetaData.php

@@ -18,6 +18,8 @@ class MetaData {
 	public const SORT_USERCOUNT = 1; // May have performance issues on LDAP backends
 	public const SORT_GROUPNAME = 2;
 
+	public const DEFAULT_GROUP_LIMIT = 500;
+
 	/** @var array */
 	protected $metaData = [];
 	/** @var int */
@@ -148,12 +150,13 @@ private function sort(array &$entries, array $sortKeys): void {
 	}
 
 	/**
-	 * returns the available groups
+	 * returns the available groups, capped at {@see self::DEFAULT_GROUP_LIMIT}
+	 *
 	 * @return IGroup[]
 	 */
 	public function getGroups(string $search = ''): array {
 		if ($this->isAdmin || $this->isDelegatedAdmin) {
-			return $this->groupManager->search($search);
+			return $this->groupManager->search($search, self::DEFAULT_GROUP_LIMIT);
 		} else {
 			$userObject = $this->userSession->getUser();
 			if ($userObject !== null && $this->groupManager instanceof GroupManager) {

openapi.json

@@ -31801,12 +31801,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {
@@ -33742,12 +33744,14 @@
                     {
                         "name": "limit",
                         "in": "query",
-                        "description": "Limit the amount of groups returned",
+                        "description": "Limit the amount of groups returned, defaults to 500",
                         "schema": {
                             "type": "integer",
                             "format": "int64",
                             "nullable": true,
-                            "default": null
+                            "default": null,
+                            "minimum": 1,
+                            "maximum": 500
                         }
                     },
                     {

tests/lib/Group/MetaDataTest.php

@@ -112,10 +112,12 @@ public function testGetGroupsAsAdmin(): void {
 		$this->groupManager
 			->expects($this->once())
 			->method('search')
-			->with('Foo')
+			->with('Foo', MetaData::DEFAULT_GROUP_LIMIT)
 			->willReturn(['DummyValue']);
 
 		$expected = ['DummyValue'];
 		$this->assertSame($expected, $this->invokePrivate($this->groupMetadata, 'getGroups', ['Foo']));
 	}
+
+
 }