diff --git a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php index 6bae439688d65..085b28769bdc6 100644 --- a/apps/provisioning_api/lib/Controller/AUserDataOCSController.php +++ b/apps/provisioning_api/lib/Controller/AUserDataOCSController.php @@ -25,6 +25,7 @@ use OCP\Group\ISubAdmin; use OCP\IConfig; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -251,6 +252,62 @@ protected function getUserSubAdminGroupsData(string $userId): array { return $groups; } + /** + * Collect users matching the filters from a paginated source, skipping missing ones. + * + * Whole source pages are consumed, as sources with multiple backends apply $limit and + * $offset per backend. So more than $limit users may be returned, and fewer if no full + * page was found within 10 source pages. + * + * @param \Closure(?int, int): array $search Returns the source page for a limit and offset + * @param bool $enabled Only collect users with this enabled state + * @return array{list, int, bool} The matching users, the source offset of the next page, + * and whether the source may have more users + */ + protected function searchFilteredUsers(\Closure $search, ?int $limit, int $offset, bool $enabled): array { + $users = []; + $rounds = 0; + while (true) { + $page = $search($limit, $offset); + foreach ($page as $user) { + if ($user !== null && $user->isEnabled() === $enabled) { + $users[] = $user; + } + } + if ($limit === null || $limit <= 0 || count($page) === 0) { + return [$users, $offset, false]; + } + $offset += $limit; + if (count($users) >= $limit || ++$rounds >= 10) { + return [$users, $offset, true]; + } + } + } + + /** + * Build the `Link: ; rel="next"` header if there is a next page. + * + * @param array|bool $items The page of results, or whether more results are known to exist + * @param array $params Query parameters for the next page + * @param int|null $forceOffset Offset of the next page when it is not `$offset + $limit`, + * e.g. because some results were skipped + * @return array{Link?: string} + */ + protected function buildOffsetNextPageLinkHeader(IURLGenerator $urlGenerator, array|bool $items, array $params, ?int $limit, int $offset, ?int $forceOffset = null): array { + $hasMore = is_bool($items) ? $items : ($limit !== null && $limit > 0 && count($items) >= $limit); + if (!$hasMore) { + return []; + } + + $params = array_merge($params, [ + 'limit' => ($limit ?? 100), + 'offset' => $forceOffset ?? $offset + ($limit ?? 100), + ]); + $path = (string)parse_url($this->request->getRequestUri(), PHP_URL_PATH); + $url = $urlGenerator->getAbsoluteURL($path) . '?' . http_build_query($params); + return ['Link' => '<' . $url . '>; rel="next"']; + } + /** * A full group has id, name, usercount, disabled, canAdd and canRemove. Only * the displayname is cached; usercount/disabled are not cached. So this only diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 5dd29350e371f..2810db18d6743 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -28,6 +28,7 @@ use OCP\IGroup; use OCP\IGroupManager; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -54,6 +55,7 @@ public function __construct( IRootFolder $rootFolder, private LoggerInterface $logger, GroupDisplayNameCache $groupDisplayNameCache, + private IURLGenerator $urlGenerator, ) { parent::__construct($appName, $request, @@ -189,14 +191,15 @@ public function getGroupUsers(string $groupId): DataResponse { * @param string $search Text to search for * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups + * @param bool|null $enabled Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset * - * @return DataResponse, groups: list}, array{}> + * @return DataResponse, groups: list}, array{Link?: string}> * @throws OCSException * * 200: Group users details returned */ #[NoAdminRequired] - public function getGroupUsersDetails(string $groupId, string $search = '', ?int $limit = null, int $offset = 0): DataResponse { + public function getGroupUsersDetails(string $groupId, string $search = '', ?int $limit = null, int $offset = 0, ?bool $enabled = null): DataResponse { $groupId = urldecode($groupId); $currentUser = $this->userSession->getUser(); @@ -212,7 +215,18 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int $isAdmin = $this->groupManager->isAdmin($currentUser->getUID()); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentUser->getUID()); if ($isAdmin || $isDelegatedAdmin || $isSubadminOfGroup) { - $users = $group->searchUsers($search, $limit, $offset); + $hasMore = null; + $nextOffset = null; + if ($enabled !== null) { + [$users, $nextOffset, $hasMore] = $this->searchFilteredUsers( + fn (?int $limit, int $offset): array => $group->searchUsers($search, $limit, $offset), + $limit, + $offset, + enabled: $enabled, + ); + } else { + $users = $group->searchUsers($search, $limit, $offset); + } // Extract required number $usersDetails = []; @@ -233,10 +247,15 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int // continue if a users ceased to exist. } } + $params = [ + 'search' => $search, + 'enabled' => $enabled + ]; + $headers = $this->buildOffsetNextPageLinkHeader($this->urlGenerator, $hasMore ?? $users, $params, $limit, $offset, $nextOffset); return new DataResponse([ 'users' => $usersDetails, 'groups' => $this->findGroupsWithDisplayname($usersDetails), - ]); + ], headers: $headers); } throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND); diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index d0a47c83df19e..c8d8ef0170063 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -150,14 +150,17 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset = * @param string $search Text to search for * @param int|null $limit Limit the amount of groups returned * @param int $offset Offset for searching for groups - * @return DataResponse, groups: list}, array{}> + * @param bool|null $enabled Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset + * @return DataResponse, groups: list}, array{Link?: string}> * * 200: Users details returned */ #[NoAdminRequired] - public function getUsersDetails(string $search = '', ?int $limit = null, int $offset = 0): DataResponse { + public function getUsersDetails(string $search = '', ?int $limit = null, int $offset = 0, ?bool $enabled = null): DataResponse { $currentUser = $this->userSession->getUser(); $users = []; + $hasMore = null; + $nextOffset = null; // Admin? Or SubAdmin? $uid = $currentUser->getUID(); @@ -165,19 +168,47 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of $isAdmin = $this->groupManager->isAdmin($uid); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid); if ($isAdmin || $isDelegatedAdmin) { - $users = $this->userManager->search($search, $limit, $offset); - $users = array_keys($users); - } elseif ($subAdminManager->isSubAdmin($currentUser)) { - $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser); - foreach ($subAdminOfGroups as $key => $group) { - $subAdminOfGroups[$key] = $group->getGID(); + $searchUsers = fn (?int $limit, int $offset): array => $this->userManager->search($search, $limit, $offset); + if ($enabled !== null) { + [$users, $nextOffset, $hasMore] = $this->searchFilteredUsers( + $searchUsers, + $limit, + $offset, + enabled: $enabled, + ); + $users = array_map(fn (IUser $user): string => $user->getUID(), $users); + } else { + $users = $searchUsers($limit, $offset); + $users = array_keys($users); } + } elseif ($subAdminManager->isSubAdmin($currentUser)) { + if ($enabled !== null) { + $users = []; + /* The groups are searched together, so offset and limit are applied to the combined list */ + $tempLimit = ($limit === null ? null : $limit + $offset); + foreach ($subAdminManager->getSubAdminsGroups($currentUser) as $group) { + foreach ($group->searchUsers($search) as $user) { + if ($user->isEnabled() === $enabled) { + $users[$user->getUID()] = $user->getUID(); + } + } + if (($tempLimit !== null) && (count($users) >= $tempLimit)) { + break; + } + } + $users = array_slice(array_values($users), $offset, $limit); + } else { + $subAdminOfGroups = $subAdminManager->getSubAdminsGroups($currentUser); + foreach ($subAdminOfGroups as $key => $group) { + $subAdminOfGroups[$key] = $group->getGID(); + } - $users = []; - foreach ($subAdminOfGroups as $group) { - $users[] = array_keys($this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); + $users = []; + foreach ($subAdminOfGroups as $group) { + $users[] = array_keys($this->groupManager->displayNamesInGroup($group, $search, $limit, $offset)); + } + $users = array_merge(...$users); } - $users = array_merge(...$users); } $usersDetails = []; @@ -201,10 +232,15 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of } } + $params = [ + 'search' => $search, + 'enabled' => $enabled + ]; + $headers = $this->buildOffsetNextPageLinkHeader($this->urlGenerator, $hasMore ?? $users, $params, $limit, $offset, $nextOffset); return new DataResponse([ 'users' => $usersDetails, 'groups' => $this->findGroupsWithDisplayname($usersDetails), - ]); + ], headers: $headers); } /** @@ -295,7 +331,8 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null, * @param string $search Text to search for * @param ?int $limit Limit the amount of users returned * @param int $offset Offset - * @return DataResponse}, array{}> + * @param bool|null $enabled Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset + * @return DataResponse}, array{Link?: string}> * * 200: Users details returned based on last logged in information */ @@ -303,6 +340,7 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null, public function getLastLoggedInUsers(string $search = '', ?int $limit = null, int $offset = 0, + ?bool $enabled = null, ): DataResponse { $currentUser = $this->userSession->getUser(); if ($currentUser === null) { @@ -316,9 +354,24 @@ public function getLastLoggedInUsers(string $search = '', } $users = []; + $hasMore = null; + $nextOffset = null; // For Admin alone user sorting based on lastLogin. For sub admin and groups this is not supported - $users = $this->userManager->getLastLoggedInUsers($limit, $offset, $search); + if ($enabled !== null) { + [$users, $nextOffset, $hasMore] = $this->searchFilteredUsers( + fn (?int $limit, int $offset): array => array_map( + fn (string $userId): ?IUser => $this->userManager->get($userId), + $this->userManager->getLastLoggedInUsers($limit, $offset, $search), + ), + $limit, + $offset, + enabled: $enabled, + ); + $users = array_map(fn (IUser $user): string => $user->getUID(), $users); + } else { + $users = $this->userManager->getLastLoggedInUsers($limit, $offset, $search); + } $usersDetails = []; foreach ($users as $userId) { @@ -340,9 +393,14 @@ public function getLastLoggedInUsers(string $search = '', } } + $params = [ + 'search' => $search, + 'enabled' => $enabled + ]; + $headers = $this->buildOffsetNextPageLinkHeader($this->urlGenerator, $hasMore ?? $users, $params, $limit, $offset, $nextOffset); return new DataResponse([ 'users' => $usersDetails - ]); + ], headers: $headers); } diff --git a/apps/provisioning_api/openapi-administration.json b/apps/provisioning_api/openapi-administration.json index 4d1cac9e25b04..ebd51e87887d8 100644 --- a/apps/provisioning_api/openapi-administration.json +++ b/apps/provisioning_api/openapi-administration.json @@ -1474,6 +1474,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -1488,6 +1498,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/openapi-full.json b/apps/provisioning_api/openapi-full.json index 2560bf0e99006..69ce1c4b6a6a6 100644 --- a/apps/provisioning_api/openapi-full.json +++ b/apps/provisioning_api/openapi-full.json @@ -1773,6 +1773,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -1787,6 +1797,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3506,6 +3523,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -3520,6 +3547,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3978,6 +4012,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -3992,6 +4036,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/openapi.json b/apps/provisioning_api/openapi.json index 0fa6365306120..3330030a7511c 100644 --- a/apps/provisioning_api/openapi.json +++ b/apps/provisioning_api/openapi.json @@ -910,6 +910,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -924,6 +934,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1493,6 +1510,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -1507,6 +1534,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php index 87476f5e7d9c2..ea97e36a92c0f 100644 --- a/apps/provisioning_api/tests/Controller/GroupsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/GroupsControllerTest.php @@ -19,6 +19,7 @@ use OCP\IConfig; use OCP\IGroup; use OCP\IRequest; +use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; @@ -39,6 +40,7 @@ class GroupsControllerTest extends \Test\TestCase { protected LoggerInterface&MockObject $logger; protected GroupsController&MockObject $api; private GroupDisplayNameCache&MockObject $groupDisplayNameCache; + private IURLGenerator&MockObject $urlGenerator; private IRootFolder $rootFolder; @@ -56,6 +58,7 @@ protected function setUp(): void { $this->logger = $this->createMock(LoggerInterface::class); $this->rootFolder = $this->createMock(IRootFolder::class); $this->groupDisplayNameCache = $this->createMock(GroupDisplayNameCache::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); $this->groupManager ->method('getSubAdmin') @@ -75,6 +78,7 @@ protected function setUp(): void { $this->rootFolder, $this->logger, $this->groupDisplayNameCache, + $this->urlGenerator, ]) ->onlyMethods(['fillStorageInfo']) ->getMock(); @@ -508,6 +512,88 @@ public function testGetGroupUsersDetails(): void { ], $data['groups']); } + public function testGetEnabledGroupUsersDetailsSkipsDisabledUntilPageIsFull(): void { + $gid = 'ncg1'; + + $this->asAdmin(); + + $users = []; + foreach (['user1' => false, 'alice' => true, 'user2' => false, 'bob' => true, 'joe' => true] as $uid => $enabled) { + $users[$uid] = $this->createUser($uid); + $users[$uid]->method('isEnabled')->willReturn($enabled); + } + $this->userManager + ->method('get') + ->willReturnCallback(fn (string $uid) => $users[$uid] ?? null); + $this->groupManager + ->method('getUserGroupIds') + ->willReturn([]); + + $group = $this->createGroup($gid); + $group->expects($this->exactly(2)) + ->method('searchUsers') + ->willReturnMap([ + ['', 2, 0, [$users['user1'], $users['alice']]], + ['', 2, 2, [$users['user2'], $users['bob']]], + ]); + $this->groupManager + ->method('get') + ->with($gid) + ->willReturn($group); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/cloud/groups/ncg1/users/details?limit=2&enabled=1'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/cloud/groups/ncg1/users/details') + ->willReturn('https://cloud.example.com/ocs/v2.php/cloud/groups/ncg1/users/details'); + + $response = $this->api->getGroupUsersDetails($gid, '', 2, 0, true); + + $this->assertSame(['alice', 'bob'], array_keys($response->getData()['users'])); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + + public function testGetDisabledGroupUsersDetailsViaEnabledFilter(): void { + $gid = 'ncg1'; + + $this->asAdmin(); + + $users = []; + foreach (['alice' => true, 'user1' => false, 'bob' => true] as $uid => $enabled) { + $users[$uid] = $this->createUser($uid); + $users[$uid]->method('isEnabled')->willReturn($enabled); + } + $this->userManager + ->method('get') + ->willReturnCallback(fn (string $uid) => $users[$uid] ?? null); + $this->groupManager + ->method('getUserGroupIds') + ->willReturn([]); + + $group = $this->createGroup($gid); + $group->expects($this->exactly(3)) + ->method('searchUsers') + ->willReturnMap([ + ['', 2, 0, [$users['alice'], $users['user1']]], + ['', 2, 2, [$users['bob']]], + ['', 2, 4, []], + ]); + $this->groupManager + ->method('get') + ->with($gid) + ->willReturn($group); + + $response = $this->api->getGroupUsersDetails($gid, '', 2, 0, false); + + $this->assertSame(['user1'], array_keys($response->getData()['users'])); + $this->assertArrayNotHasKey('Link', $response->getHeaders()); + } + public function testGetGroupUsersDetailsEncoded(): void { $gid = 'Department A/B C/D'; diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 4ee940d26784a..665af7d43763a 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -419,6 +419,310 @@ public function testGetDisabledUsersAsSubAdmin(): void { $this->assertEquals($expected, $this->api->getDisabledUsersDetails('MyCustomSearch', 3)->getData()); } + public function testGetEnabledUsersDetailsAsAdminSkipsDisabledUntilPageIsFull(): void { + $loggedInUser = $this->createUserMock('admin', true); + $this->userSession + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->method('getSubAdmin') + ->willReturn($this->subAdminManager); + $this->groupManager + ->method('isAdmin') + ->willReturn(true); + $this->userManager + ->expects($this->exactly(3)) + ->method('search') + ->willReturnMap([ + ['MyCustomSearch', 2, 0, ['user1' => $this->createUserMock('user1', false), 'user2' => $this->createUserMock('user2', false)]], + ['MyCustomSearch', 2, 2, ['alice' => $this->createUserMock('alice', true), 'user3' => $this->createUserMock('user3', false)]], + ['MyCustomSearch', 2, 4, ['bob' => $this->createUserMock('bob', true), 'joe' => $this->createUserMock('joe', true)]], + ]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/cloud/users/details?search=MyCustomSearch&limit=2&enabled=1'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/cloud/users/details') + ->willReturn('https://cloud.example.com/ocs/v2.php/cloud/users/details'); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 0, true); + $this->assertSame(['alice', 'bob', 'joe'], array_keys($response->getData()['users'])); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + + public function testGetDisabledUsersDetailsAsAdminViaEnabledFilter(): void { + $this->userSession + ->method('getUser') + ->willReturn($this->createUserMock('admin', true)); + $this->groupManager + ->method('getSubAdmin') + ->willReturn($this->subAdminManager); + $this->groupManager + ->method('isAdmin') + ->willReturn(true); + $this->userManager + ->expects($this->exactly(2)) + ->method('search') + ->willReturnMap([ + ['MyCustomSearch', 2, 0, ['alice' => $this->createUserMock('alice', true), 'user1' => $this->createUserMock('user1', false)]], + ['MyCustomSearch', 2, 2, ['user2' => $this->createUserMock('user2', false), 'bob' => $this->createUserMock('bob', true)]], + ]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/cloud/users/details?search=MyCustomSearch&limit=2&enabled=0'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/cloud/users/details') + ->willReturn('https://cloud.example.com/ocs/v2.php/cloud/users/details'); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 0, false); + $this->assertSame(['user1', 'user2'], array_keys($response->getData()['users'])); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + + public function testGetUsersDetailsWithoutEnabledFilterReturnsAllUsers(): void { + $this->userSession + ->method('getUser') + ->willReturn($this->createUserMock('admin', true)); + $this->groupManager + ->method('getSubAdmin') + ->willReturn($this->subAdminManager); + $this->groupManager + ->method('isAdmin') + ->willReturn(true); + $this->userManager + ->expects($this->once()) + ->method('search') + ->with('MyCustomSearch', 2, 0) + ->willReturn(['alice' => $this->createUserMock('alice', true), 'user1' => $this->createUserMock('user1', false)]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/cloud/users/details?search=MyCustomSearch&limit=2'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/cloud/users/details') + ->willReturn('https://cloud.example.com/ocs/v2.php/cloud/users/details'); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 0); + $this->assertSame(['alice', 'user1'], array_keys($response->getData()['users'])); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + + public function testGetEnabledUsersDetailsAsAdminStopsAtEndOfResults(): void { + $loggedInUser = $this->createUserMock('admin', true); + $this->userSession + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->method('getSubAdmin') + ->willReturn($this->subAdminManager); + $this->groupManager + ->method('isAdmin') + ->willReturn(true); + $this->userManager + ->expects($this->exactly(3)) + ->method('search') + ->willReturnMap([ + ['MyCustomSearch', 2, 0, ['alice' => $this->createUserMock('alice', true), 'user1' => $this->createUserMock('user1', false)]], + ['MyCustomSearch', 2, 2, ['user2' => $this->createUserMock('user2', false)]], + ['MyCustomSearch', 2, 4, []], + ]); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 0, true); + $this->assertSame(['alice'], array_keys($response->getData()['users'])); + $this->assertArrayNotHasKey('Link', $response->getHeaders()); + } + + public function testGetUsersDetailsAsSubAdminFiltersByEnabledState(): void { + $loggedInUser = $this->createUserMock('subadmin', true); + $this->userSession + ->method('getUser') + ->willReturn($loggedInUser); + $this->groupManager + ->method('isAdmin') + ->willReturn(false); + $firstGroup = $this->createMock(IGroup::class); + $secondGroup = $this->createMock(IGroup::class); + $subAdminManager = $this->createMock(SubAdmin::class); + $subAdminManager + ->method('isSubAdmin') + ->with($loggedInUser) + ->willReturn(true); + $subAdminManager + ->method('getSubAdminsGroups') + ->with($loggedInUser) + ->willReturn([$firstGroup, $secondGroup]); + $this->groupManager + ->method('getSubAdmin') + ->willReturn($subAdminManager); + $this->groupManager + ->expects($this->never()) + ->method('displayNamesInGroup'); + + $firstGroup + ->method('searchUsers') + ->with('MyCustomSearch') + ->willReturn([ + $this->createUserMock('user1', false), + $this->createUserMock('bob', true), + $this->createUserMock('alice', true), + ]); + $secondGroup + ->method('searchUsers') + ->with('MyCustomSearch') + ->willReturn([ + $this->createUserMock('alice', true), + $this->createUserMock('user2', false), + $this->createUserMock('joe', true), + $this->createUserMock('jim', true), + ]); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 1, true); + $this->assertSame(['alice', 'joe'], array_keys($response->getData()['users'])); + + $response = $this->api->getUsersDetails('MyCustomSearch', 2, 0, false); + $this->assertSame(['user1', 'user2'], array_keys($response->getData()['users'])); + } + + public function testGetEnabledLastLoggedInUsersSkipsDisabledAndMissingUntilPageIsFull(): void { + $this->userSession + ->method('getUser') + ->willReturn($this->createUserMock('admin', true)); + $users = [ + 'user1' => $this->createUserMock('user1', false), + 'alice' => $this->createUserMock('alice', true), + 'bob' => $this->createUserMock('bob', true), + ]; + $this->userManager + ->method('get') + ->willReturnCallback(fn (string $uid) => $users[$uid] ?? null); + $this->userManager + ->expects($this->exactly(2)) + ->method('getLastLoggedInUsers') + ->willReturnMap([ + [2, 0, 'MyCustomSearch', ['user1', 'alice']], + [2, 2, 'MyCustomSearch', ['removed', 'bob']], + ]); + + $this->request + ->method('getRequestUri') + ->willReturn('/ocs/v2.php/cloud/users/recent?search=MyCustomSearch&limit=2&enabled=1'); + $this->urlGenerator + ->method('getAbsoluteURL') + ->with('/ocs/v2.php/cloud/users/recent') + ->willReturn('https://cloud.example.com/ocs/v2.php/cloud/users/recent'); + + $api = $this->getMockBuilder(UsersController::class) + ->setConstructorArgs([ + 'provisioning_api', + $this->request, + $this->userManager, + $this->config, + $this->groupManager, + $this->userSession, + $this->accountManager, + $this->subAdminManager, + $this->l10nFactory, + $this->rootFolder, + $this->urlGenerator, + $this->logger, + $this->newUserMailHelper, + $this->secureRandom, + $this->remoteWipe, + $this->knownUserService, + $this->eventDispatcher, + $this->phoneNumberUtil, + $this->appManager, + $this->groupDisplayNameCache, + ]) + ->onlyMethods(['getUserData']) + ->getMock(); + $api->method('getUserData') + ->willReturnCallback(fn (string $uid) => ['id' => $uid]); + + $response = $api->getLastLoggedInUsers('MyCustomSearch', 2, 0, true); + $this->assertSame(['alice', 'bob'], array_keys($response->getData()['users'])); + $this->assertSame( + '; rel="next"', + $response->getHeaders()['Link'] + ); + } + + public function testSearchFilteredUsersCollectsDisabledUsers(): void { + $pages = [ + 0 => [$this->createUserMock('alice', true), null], + 2 => [$this->createUserMock('user1', false), $this->createUserMock('bob', true)], + 4 => [$this->createUserMock('user2', false)], + ]; + + [$users, $nextOffset, $hasMore] = self::invokePrivate($this->api, 'searchFilteredUsers', [ + fn (?int $limit, int $offset): array => $pages[$offset] ?? [], + 2, + 0, + false, + ]); + + $this->assertSame(['user1', 'user2'], array_map(fn (IUser $user): string => $user->getUID(), $users)); + $this->assertSame(6, $nextOffset); + $this->assertTrue($hasMore); + } + + public function testSearchFilteredUsersDoesNotSkipUsersOfMultipleBackends(): void { + $backends = []; + foreach (['a', 'b'] as $backend) { + for ($i = 0; $i < 30; $i++) { + $backends[$backend][] = $this->createUserMock(sprintf('%s%02d', $backend, $i), $i % 3 !== 0); + } + } + // Like IUserManager::search(), limit and offset are applied per backend + $search = function (?int $limit, int $offset) use ($backends): array { + $page = []; + foreach ($backends as $users) { + $page = array_merge($page, array_slice($users, $offset, $limit)); + } + return $page; + }; + + $found = []; + $offset = 0; + do { + [$users, $offset, $hasMore] = self::invokePrivate($this->api, 'searchFilteredUsers', [$search, 25, $offset, true]); + foreach ($users as $user) { + $found[] = $user->getUID(); + } + } while ($hasMore); + + $this->assertCount(40, $found); + $this->assertCount(40, array_unique($found)); + } + + public function testSearchFilteredUsersStopsAfterTenSourcePages(): void { + $calls = 0; + $search = function (?int $limit, int $offset) use (&$calls): array { + $calls++; + return [$this->createUserMock('user' . $offset, false), $this->createUserMock('user' . ($offset + 1), false)]; + }; + + [$users, $nextOffset, $hasMore] = self::invokePrivate($this->api, 'searchFilteredUsers', [$search, 2, 0, true]); + + $this->assertSame([], $users); + $this->assertSame(10, $calls); + $this->assertSame(20, $nextOffset); + $this->assertTrue($hasMore); + } public function testAddUserAlreadyExisting(): void { $this->expectException(OCSException::class); diff --git a/apps/settings/src/components/UserList.vue b/apps/settings/src/components/UserList.vue index 459548fad2674..63d55a54318c4 100644 --- a/apps/settings/src/components/UserList.vue +++ b/apps/settings/src/components/UserList.vue @@ -290,6 +290,7 @@ export default { offset: this.usersOffset, limit: this.usersLimit, search: this.searchQuery, + enabled: true, }) } else { await this.$store.dispatch('getUsers', { @@ -297,6 +298,7 @@ export default { limit: this.usersLimit, group: this.selectedGroup, search: this.searchQuery, + enabled: true, }) } logger.debug(`${this.users.length} total user(s) loaded`) diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js index 6e1354251cd76..63292634ef78e 100644 --- a/apps/settings/src/store/users.js +++ b/apps/settings/src/store/users.js @@ -67,6 +67,9 @@ const mutations = { state.usersOffset += state.usersLimit state.users = users }, + setUsersOffset(state, offset) { + state.usersOffset = offset + }, updateDisabledUsers(state, _usersObj) { state.disabledUsersOffset += state.disabledUsersLimit }, @@ -333,6 +336,41 @@ function commitGroupsFromUsersResponse(context, response) { }) } +/** + * Get the offset of the next page from the `Link: ; rel="next"` response header. + * + * @param {object} response axios response + * @return {number|null} The offset, or null if there is no next page + */ +function getNextPageOffset(response) { + const url = response.headers?.link?.match(/<([^>]+)>;\s*rel="next"/)?.[1] + const offset = url ? new URL(url).searchParams.get('offset') : null + return offset === null ? null : Number(offset) +} + +/** + * Build the `enabled` query parameter filtering users by their enabled state. + * + * @param {boolean|null} enabled The enabled state to filter by, or null for all users + * @return {string} + */ +function getEnabledParam(enabled) { + return enabled === null ? '' : `&enabled=${enabled ? 1 : 0}` +} + +/** + * Continue from the next page offset given by the server, as it may have skipped users. + * + * @param {object} context store context + * @param {object} response axios response + */ +function commitNextPageOffset(context, response) { + const nextOffset = getNextPageOffset(response) + if (nextOffset !== null) { + context.commit('setUsersOffset', nextOffset) + } +} + const actions = { /** @@ -379,9 +417,10 @@ const actions = { * @param {number} options.limit List number to return from offset * @param {string} options.search Search amongst users * @param {string} options.group Get users from group + * @param {boolean|null} options.enabled Only get users with this enabled state, or all users if null * @return {Promise} */ - getUsers(context, { offset, limit, search, group }) { + getUsers(context, { offset, limit, search, group, enabled = null }) { if (searchRequestCancelSource) { searchRequestCancelSource.cancel('Operation canceled by another search request.') } @@ -396,8 +435,9 @@ const actions = { search = search.replace(/in:[^\s]+/g, '').trim() group = typeof group === 'string' ? group : '' + const enabledParam = getEnabledParam(enabled) if (group !== '') { - return api.get(generateOcsUrl('cloud/groups/{group}/users/details?offset={offset}&limit={limit}&search={search}', { group: encodeURIComponent(group), offset, limit, search }), { + return api.get(generateOcsUrl('cloud/groups/{group}/users/details?offset={offset}&limit={limit}&search={search}', { group: encodeURIComponent(group), offset, limit, search }) + enabledParam, { cancelToken: searchRequestCancelSource.token, }) .then((response) => { @@ -405,6 +445,7 @@ const actions = { if (usersCount > 0) { context.commit('appendUsers', response.data.ocs.data.users) } + commitNextPageOffset(context, response) commitGroupsFromUsersResponse(context, response) return usersCount }) @@ -415,7 +456,7 @@ const actions = { }) } - return api.get(generateOcsUrl('cloud/users/details?offset={offset}&limit={limit}&search={search}', { offset, limit, search }), { + return api.get(generateOcsUrl('cloud/users/details?offset={offset}&limit={limit}&search={search}', { offset, limit, search }) + enabledParam, { cancelToken: searchRequestCancelSource.token, }) .then((response) => { @@ -423,6 +464,7 @@ const actions = { if (usersCount > 0) { context.commit('appendUsers', response.data.ocs.data.users) } + commitNextPageOffset(context, response) commitGroupsFromUsersResponse(context, response) return usersCount }) @@ -441,16 +483,19 @@ const actions = { * @param {number} options.offset List offset to request * @param {number} options.limit List number to return from offset * @param {string} options.search Search query + * @param {boolean|null} options.enabled Only get users with this enabled state, or all users if null * @return {Promise} */ - async getRecentUsers(context, { offset, limit, search }) { + async getRecentUsers(context, { offset, limit, search, enabled = null }) { const url = generateOcsUrl('cloud/users/recent?offset={offset}&limit={limit}&search={search}', { offset, limit, search }) + + getEnabledParam(enabled) try { const response = await api.get(url) const usersCount = Object.keys(response.data.ocs.data.users).length if (usersCount > 0) { context.commit('appendUsers', response.data.ocs.data.users) } + commitNextPageOffset(context, response) return usersCount } catch (error) { context.commit('API_FAILURE', error) diff --git a/apps/settings/src/store/users.spec.ts b/apps/settings/src/store/users.spec.ts index 128fa5c25445c..dd2929d0c7229 100644 --- a/apps/settings/src/store/users.spec.ts +++ b/apps/settings/src/store/users.spec.ts @@ -3,10 +3,13 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { describe, expect, it } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import api from './api.js' import usersStore from './users.js' -const { mutations } = usersStore +vi.mock('./api.js', () => ({ default: { get: vi.fn() } })) + +const { mutations, actions } = usersStore describe('store:users addGroup', () => { it('inserts a new group filled up with defaults', () => { @@ -60,3 +63,85 @@ describe('store:users addGroup', () => { ]) }) }) + +describe('store:users getUsers', () => { + const users = { alice: { id: 'alice' } } + + /** + * @param headers Response headers + */ + function respondWith(headers: Record) { + vi.mocked(api.get).mockResolvedValue({ headers, data: { ocs: { data: { users, groups: [] } } } }) + } + + beforeEach(() => { + vi.mocked(api.get).mockReset() + }) + + it('requests only enabled users and continues from the offset in the Link header', async () => { + respondWith({ link: '; rel="next"' }) + const commit = vi.fn() + + await actions.getUsers({ commit }, { offset: 0, limit: 25, search: '', enabled: true }) + + expect(vi.mocked(api.get).mock.calls[0][0]).toMatch(/cloud\/users\/details\?offset=0&limit=25&search=&enabled=1$/) + expect(commit).toHaveBeenCalledWith('appendUsers', users) + expect(commit).toHaveBeenCalledWith('setUsersOffset', 42) + }) + + it('requests only enabled users of a group', async () => { + respondWith({}) + const commit = vi.fn() + + await actions.getUsers({ commit }, { offset: 0, limit: 25, search: '', group: 'group1', enabled: true }) + + expect(vi.mocked(api.get).mock.calls[0][0]).toMatch(/cloud\/groups\/group1\/users\/details\?offset=0&limit=25&search=&enabled=1$/) + }) + + it('requests only disabled users', async () => { + respondWith({}) + const commit = vi.fn() + + await actions.getUsers({ commit }, { offset: 0, limit: 25, search: '', enabled: false }) + + expect(vi.mocked(api.get).mock.calls[0][0]).toMatch(/cloud\/users\/details\?offset=0&limit=25&search=&enabled=0$/) + }) + + it('keeps the offset when there is no next page', async () => { + respondWith({}) + const commit = vi.fn() + + await actions.getUsers({ commit }, { offset: 0, limit: 25, search: '' }) + + expect(vi.mocked(api.get).mock.calls[0][0]).not.toContain('enabled') + expect(commit).not.toHaveBeenCalledWith('setUsersOffset', expect.anything()) + }) +}) + +describe('store:users getRecentUsers', () => { + beforeEach(() => { + vi.mocked(api.get).mockReset() + }) + + it('requests only enabled users and continues from the offset in the Link header', async () => { + vi.mocked(api.get).mockResolvedValue({ + headers: { link: '; rel="next"' }, + data: { ocs: { data: { users: { alice: { id: 'alice' } } } } }, + }) + const commit = vi.fn() + + await actions.getRecentUsers({ commit }, { offset: 0, limit: 25, search: '', enabled: true }) + + expect(vi.mocked(api.get).mock.calls[0][0]).toMatch(/cloud\/users\/recent\?offset=0&limit=25&search=&enabled=1$/) + expect(commit).toHaveBeenCalledWith('setUsersOffset', 30) + }) +}) + +describe('store:users setUsersOffset', () => { + it('replaces the users offset', () => { + const state = { usersOffset: 25 } + mutations.setUsersOffset(state, 42) + + expect(state.usersOffset).toBe(42) + }) +}) diff --git a/openapi.json b/openapi.json index 7ba886c79d6f2..2c8ca3f559f2d 100644 --- a/openapi.json +++ b/openapi.json @@ -28097,6 +28097,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -28111,6 +28121,13 @@ "responses": { "200": { "description": "Users details returned based on last logged in information", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -29830,6 +29847,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -29844,6 +29871,13 @@ "responses": { "200": { "description": "Group users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -30302,6 +30336,16 @@ "default": 0 } }, + { + "name": "enabled", + "in": "query", + "description": "Only return users with this enabled state, or all users if null. Use the `Link` header for the next page offset", + "schema": { + "type": "boolean", + "nullable": true, + "default": null + } + }, { "name": "OCS-APIRequest", "in": "header", @@ -30316,6 +30360,13 @@ "responses": { "200": { "description": "Users details returned", + "headers": { + "Link": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": {