Skip to content

fix(provisioning_api): filter users by enabled state on the server #65468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
SebastianKrupinski wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(provisioning_api): filter users by enabled state on the server #65468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions apps/provisioning_api/lib/Controller/AUserDataOCSController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,35 @@ protected function getUserSubAdminGroupsData(string $userId): array {
return $groups;
}

/**
* Collect up to $limit users matching the filters from a paginated source, skipping missing ones.
*
* @param \Closure(?int, int): iterable<?IUser> $search Returns the source page for a limit and offset
* @param bool $enabled Only collect users with this enabled state
* @return array{list<IUser>, 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 = [];
while (true) {
$count = 0;
foreach ($search($limit, $offset) as $user) {
$count++;
$offset++;

@pringelmann pringelmann Oct 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this might skip users when there's more than one user backend (e.g. LDAP + Database), since search() pages each backend separately but the offset here counts the merged result. Could you double check?

if ($user === null || $user->isEnabled() !== $enabled) {
continue;
}
$users[] = $user;
if ($limit !== null && $limit > 0 && count($users) >= $limit) {
return [$users, $offset, true];
}
}
if ($limit === null || $limit <= 0 || $count < $limit) {
return [$users, $offset, false];
}
}
}

/**
* A full group has id, name, usercount, disabled, canAdd and canRemove. Only
* the displayname is cached; usercount/disabled are not cached. So this only
Expand Down
22 changes: 18 additions & 4 deletions apps/provisioning_api/lib/Controller/GroupsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,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<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, 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();

Expand All @@ -223,7 +224,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 = [];
Expand All @@ -244,9 +256,11 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
// continue if a users ceased to exist.
}
}
$headers = $this->buildOffsetNextPageLinkHeader($users, [
$params = [
'search' => $search,
], $limit, $offset);
'enabled' => $enabled
];
$headers = $this->buildOffsetNextPageLinkHeader($hasMore ?? $users, $params, $limit, $offset, $nextOffset);
return new DataResponse([
'users' => $usersDetails,
'groups' => $this->findGroupsWithDisplayname($usersDetails),
Expand Down
84 changes: 68 additions & 16 deletions apps/provisioning_api/lib/Controller/UsersController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,34 +158,65 @@ 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
* @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<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>, groups: list<Provisioning_APIUserDetailsGroupDisplayname>}, 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();
$subAdminManager = $this->groupManager->getSubAdmin();
$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 = [];
Expand All @@ -209,9 +240,11 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
}
}

$headers = $this->buildOffsetNextPageLinkHeader($users, [
$params = [
'search' => $search,
], $limit, $offset);
'enabled' => $enabled
];
$headers = $this->buildOffsetNextPageLinkHeader($hasMore ?? $users, $params, $limit, $offset, $nextOffset);
return new DataResponse([
'users' => $usersDetails,
'groups' => $this->findGroupsWithDisplayname($usersDetails),
Expand Down Expand Up @@ -309,6 +342,7 @@ 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
* @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<Http::STATUS_OK, array{users: array<string, Provisioning_APIUserDetails|array{id: string}>}, array{Link?: string}>
*
* 200: Users details returned based on last logged in information
Expand All @@ -318,6 +352,7 @@ public function getLastLoggedInUsers(
string $search = '',
?int $limit = null,
int $offset = 0,
?bool $enabled = null,
): DataResponse {
$currentUser = $this->userSession->getUser();
if ($currentUser === null) {
Expand All @@ -331,9 +366,24 @@ public function getLastLoggedInUsers(
}

$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) {
Expand All @@ -355,9 +405,11 @@ public function getLastLoggedInUsers(
}
}

$headers = $this->buildOffsetNextPageLinkHeader($users, [
$params = [
'search' => $search,
], $limit, $offset);
'enabled' => $enabled
];
$headers = $this->buildOffsetNextPageLinkHeader($hasMore ?? $users, $params, $limit, $offset, $nextOffset);
return new DataResponse([
'users' => $usersDetails
], headers: $headers);
Expand Down
10 changes: 10 additions & 0 deletions apps/provisioning_api/openapi-administration.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1478,6 +1478,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",
Expand Down
30 changes: 30 additions & 0 deletions apps/provisioning_api/openapi-full.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1784,6 +1784,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",
Expand Down Expand Up @@ -3531,6 +3541,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",
Expand Down Expand Up @@ -4017,6 +4037,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",
Expand Down
20 changes: 20 additions & 0 deletions apps/provisioning_api/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,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",
Expand Down Expand Up @@ -1521,6 +1531,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",
Expand Down
Loading
Loading