Provision API pagination for subadmins is broken

[rullzer]

Now that we allow subadmins to get the list of users they have access to via the provisioning API we run into issues.

core/apps/provisioning_api/lib/users.php

Lines 84 to 97 in e1e1f4f

	} else if (\OC_SubAdmin::isSubAdmin($user->getUID())) {
	$subAdminOfGroups = \OC_SubAdmin::getSubAdminsGroups($user->getUID());
	if($offset === null) {
	$offset = 0;
	}
	$users = [];
	foreach ($subAdminOfGroups as $group) {
	$users = array_merge($users, $this->groupManager->displayNamesInGroup($group, $search));
	}
	$users = array_slice($users, $offset, $limit);
	} else {

As you can see what we do there is for each group the subadmin is a subadmin of we get the list of users. We merge those lists and then slice the array accordingly.

Now this introduces a few issues:

1. Assume 10 groups with 1000 members each, your system won't be happy
2. Since we ask the backends to sort it will be hard to write proper pagination test

To solve problem 1 I think we might need to introduce new calls to the groupManager i.e. displayNamesInGroups and then the additional calls all the way down. This will allow us to let the backend handle deduplication (which is most likely can do much more efficiently).

Problem 2 is something different but can be solved with integration tests. Basically we do not quarantee any order on the results. So if we fill the database with random users to query all we need to check is if (when doing pagination) all users get enumerated exactly once.

Tagging for 9.0 since especially problem 1 could be an issue for large installs.

CC: @DeepDiver1975 @tomneedham

[DeepDiver1975]

Tagging for 9.0 since especially problem 1 could be an issue for large installs.

@rperezb please add tests to the QA plan - THX

[rullzer]

I already made an issue a while back for the displayNamesInGroups: #18418

[PVince81]

@rullzer is that still an issue ?

[rullzer]

yes.

[karlitschek]

Is this really a sev2?

[PVince81]

Moving to 9.2 as per PR

[PVince81]

@tomneedham assigning to you as you're familiar with that API. I'm aware that you might not have time, but let's see if you have some insights.

[PVince81]

moving to backlog as no one complained about this, maybe no one is actually using this pagination currently

[ownclouders]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

[ownclouders]

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

[stale]

This issue has been automatically closed.