fix(provisioning_api): tighten editUserMultiField permission checks

-e
Signed-off-by: Peter Ringelmann <peter.ringelmann@nextcloud.com>
This commit is contained in:
Peter Ringelmann 2026-04-27 18:44:42 +02:00
parent 25ce2a17e5
commit 2f927e0d94
2 changed files with 32 additions and 30 deletions

View file

@ -966,10 +966,9 @@ class UsersController extends AUserDataOCSController {
if ($displayName !== null) {
$backend = $targetUser->getBackend();
if ($canEditOther) {
if (!$isSelf) {
$canSetDisplayName = $backend instanceof ISetDisplayNameBackend
|| ($backend !== null && $backend->implementsActions(Backend::SET_DISPLAYNAME))
|| $targetUser->canChangeDisplayName();
|| ($backend !== null && $backend->implementsActions(Backend::SET_DISPLAYNAME));
} else {
$canSetDisplayName = $targetUser->canChangeDisplayName();
}
@ -1009,20 +1008,28 @@ class UsersController extends AUserDataOCSController {
}
}
if ($groups !== null && ($isAdmin || $isDelegatedAdmin)) {
foreach ($groups as $gid) {
if (!$this->groupManager->groupExists($gid)) {
$errors['groups'] = $this->l10n->t('Group %s does not exist', [$gid]);
break;
if ($groups !== null) {
if (!$isAdmin && !$isDelegatedAdmin) {
$errors['groups'] = $this->l10n->t('Insufficient permissions to change groups');
} else {
foreach ($groups as $gid) {
if (!$this->groupManager->groupExists($gid)) {
$errors['groups'] = $this->l10n->t('Group %s does not exist', [$gid]);
break;
}
}
}
}
if ($subadminGroups !== null && ($isAdmin || $isDelegatedAdmin)) {
foreach ($subadminGroups as $gid) {
if (!$this->groupManager->groupExists($gid)) {
$errors['subadminGroups'] = $this->l10n->t('Group %s does not exist', [$gid]);
break;
if ($subadminGroups !== null) {
if (!$isAdmin && !$isDelegatedAdmin) {
$errors['subadminGroups'] = $this->l10n->t('Insufficient permissions to change sub-admin groups');
} else {
foreach ($subadminGroups as $gid) {
if (!$this->groupManager->groupExists($gid)) {
$errors['subadminGroups'] = $this->l10n->t('Group %s does not exist', [$gid]);
break;
}
}
}
}
@ -1057,24 +1064,19 @@ class UsersController extends AUserDataOCSController {
$targetUser->setSystemEMailAddress(mb_strtolower(trim($email)));
}
// Quota and manager can only be set by admins, delegated admins,
// or subadmins with access — never by regular users for themselves.
if ($quota !== null && $canEditOther) {
if ($quota !== null) {
$targetUser->setQuota($quota);
}
if ($language !== null) {
$forceLanguage = $this->config->getSystemValue('force_language', false);
if ($forceLanguage === false || $isAdmin || $isDelegatedAdmin) {
$this->config->setUserValue($targetUser->getUID(), 'core', 'lang', $language);
}
$this->config->setUserValue($targetUser->getUID(), 'core', 'lang', $language);
}
if ($manager !== null && $canEditOther) {
if ($manager !== null) {
$targetUser->setManagerUids(array_filter([$manager]));
}
if ($groups !== null && ($isAdmin || $isDelegatedAdmin)) {
if ($groups !== null) {
$currentGroups = $this->groupManager->getUserGroups($targetUser);
$currentGroupIds = array_map(fn (IGroup $g) => $g->getGID(), $currentGroups);
foreach (array_diff($currentGroupIds, $groups) as $gid) {
@ -1085,15 +1087,11 @@ class UsersController extends AUserDataOCSController {
if (!$isAdmin && $gid === 'admin') {
continue;
}
$group = $this->groupManager->get($gid);
if ($group === null) {
continue;
}
$group->addUser($targetUser);
$this->groupManager->get($gid)?->addUser($targetUser);
}
}
if ($subadminGroups !== null && ($isAdmin || $isDelegatedAdmin)) {
if ($subadminGroups !== null) {
$currentSubAdminGroups = $subAdminManager->getSubAdminsGroups($targetUser);
$currentSubAdminGroupIds = array_map(fn (IGroup $g) => $g->getGID(), $currentSubAdminGroups);
foreach (array_diff($currentSubAdminGroupIds, $subadminGroups) as $gid) {

View file

@ -2564,7 +2564,9 @@ class UsersControllerTest extends TestCase {
$targetUser = $this->createMock(IUser::class);
$targetUser->method('getUID')->willReturn('targetuser');
$targetUser->method('canChangeDisplayName')->willReturn(true);
$targetUser->method('getBackend')->willReturn($this->createMock(UserInterface::class));
$backend = $this->createMock(UserInterface::class);
$backend->method('implementsActions')->willReturn(true);
$targetUser->method('getBackend')->willReturn($backend);
$this->userManager->method('get')->with('targetuser')->willReturn($targetUser);
$this->groupManager->method('isAdmin')->with('admin')->willReturn(true);
@ -2848,7 +2850,9 @@ class UsersControllerTest extends TestCase {
$targetUser = $this->createMock(IUser::class);
$targetUser->method('getUID')->willReturn('targetuser');
$targetUser->method('canChangeDisplayName')->willReturn(true);
$targetUser->method('getBackend')->willReturn($this->createMock(UserInterface::class));
$backend = $this->createMock(UserInterface::class);
$backend->method('implementsActions')->willReturn(true);
$targetUser->method('getBackend')->willReturn($backend);
$this->userManager->method('get')->with('targetuser')->willReturn($targetUser);
$this->groupManager->method('isAdmin')->with('admin')->willReturn(true);