Skip to content

Commit

Permalink
Merge pull request #46418 from nextcloud/artonge/feat/user_admin_dele…
Browse files Browse the repository at this point in the history
…gation

feat(users): Add users and group management to admin delegation
  • Loading branch information
artonge authored Jul 24, 2024
2 parents f3a2806 + 7f0f671 commit 7266a9e
Show file tree
Hide file tree
Showing 26 changed files with 297 additions and 135 deletions.
4 changes: 3 additions & 1 deletion apps/provisioning_api/lib/Controller/AUserData.php
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
}

$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if ($isAdmin
|| $isDelegatedAdmin
|| $this->groupManager->getSubAdmin()->isUserAccessible($currentLoggedInUser, $targetUserObject)) {
$data['enabled'] = $this->config->getUserValue($targetUserObject->getUID(), 'core', 'enabled', 'true') === 'true';
} else {
Expand All @@ -117,7 +119,7 @@ protected function getUserData(string $userId, bool $includeScopes = false): ?ar
$gids[] = $group->getGID();
}

if ($isAdmin) {
if ($isAdmin || $isDelegatedAdmin) {
try {
# might be thrown by LDAP due to handling of users disappears
# from the external source (reasons unknown to us)
Expand Down
15 changes: 12 additions & 3 deletions apps/provisioning_api/lib/Controller/GroupsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
namespace OCA\Provisioning_API\Controller;

use OCA\Provisioning_API\ResponseDefinitions;
use OCA\Settings\Settings\Admin\Users;
use OCP\Accounts\IAccountManager;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
Expand Down Expand Up @@ -154,8 +156,9 @@ public function getGroupUsers(string $groupId): DataResponse {
}

// Check subadmin has access to this group
if ($this->groupManager->isAdmin($user->getUID())
|| $isSubadminOfGroup) {
$isAdmin = $this->groupManager->isAdmin($user->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($user->getUID());
if ($isAdmin || $isDelegatedAdmin || $isSubadminOfGroup) {
$users = $this->groupManager->get($groupId)->getUsers();
$users = array_map(function ($user) {
/** @var IUser $user */
Expand Down Expand Up @@ -197,7 +200,9 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
}

// Check subadmin has access to this group
if ($this->groupManager->isAdmin($currentUser->getUID()) || $isSubadminOfGroup) {
$isAdmin = $this->groupManager->isAdmin($currentUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentUser->getUID());
if ($isAdmin || $isDelegatedAdmin || $isSubadminOfGroup) {
$users = $group->searchUsers($search, $limit, $offset);

// Extract required number
Expand Down Expand Up @@ -237,6 +242,7 @@ public function getGroupUsersDetails(string $groupId, string $search = '', ?int
*
* 200: Group created successfully
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function addGroup(string $groupid, string $displayname = ''): DataResponse {
// Validate name
if (empty($groupid)) {
Expand Down Expand Up @@ -270,6 +276,7 @@ public function addGroup(string $groupid, string $displayname = ''): DataRespons
*
* 200: Group updated successfully
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function updateGroup(string $groupId, string $key, string $value): DataResponse {
$groupId = urldecode($groupId);

Expand Down Expand Up @@ -299,6 +306,7 @@ public function updateGroup(string $groupId, string $key, string $value): DataRe
*
* 200: Group deleted successfully
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function deleteGroup(string $groupId): DataResponse {
$groupId = urldecode($groupId);

Expand All @@ -322,6 +330,7 @@ public function deleteGroup(string $groupId): DataResponse {
*
* 200: Sub admins returned
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function getSubAdminsOfGroup(string $groupId): DataResponse {
// Check group exists
$targetGroup = $this->groupManager->get($groupId);
Expand Down
77 changes: 56 additions & 21 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
use OC\User\Backend;
use OCA\Provisioning_API\ResponseDefinitions;
use OCA\Settings\Mailer\NewUserMailHelper;
use OCA\Settings\Settings\Admin\Users;
use OCP\Accounts\IAccountManager;
use OCP\Accounts\IAccountProperty;
use OCP\Accounts\PropertyDoesNotExistException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\OCS\OCSException;
use OCP\AppFramework\OCS\OCSForbiddenException;
Expand Down Expand Up @@ -101,7 +103,9 @@ public function getUsers(string $search = '', ?int $limit = null, int $offset =
// Admin? Or SubAdmin?
$uid = $user->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();
if ($this->groupManager->isAdmin($uid)) {
$isAdmin = $this->groupManager->isAdmin($uid);
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid);
if ($isAdmin || $isDelegatedAdmin) {
$users = $this->userManager->search($search, $limit, $offset);
} elseif ($subAdminManager->isSubAdmin($user)) {
$subAdminOfGroups = $subAdminManager->getSubAdminsGroups($user);
Expand Down Expand Up @@ -142,7 +146,9 @@ public function getUsersDetails(string $search = '', ?int $limit = null, int $of
// Admin? Or SubAdmin?
$uid = $currentUser->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();
if ($this->groupManager->isAdmin($uid)) {
$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)) {
Expand Down Expand Up @@ -213,7 +219,9 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null,
// Admin? Or SubAdmin?
$uid = $currentUser->getUID();
$subAdminManager = $this->groupManager->getSubAdmin();
if ($this->groupManager->isAdmin($uid)) {
$isAdmin = $this->groupManager->isAdmin($uid);
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid);
if ($isAdmin || $isDelegatedAdmin) {
$users = $this->userManager->getDisabledUsers($limit, $offset, $search);
$users = array_map(fn (IUser $user): string => $user->getUID(), $users);
} elseif ($subAdminManager->isSubAdmin($currentUser)) {
Expand Down Expand Up @@ -275,6 +283,7 @@ public function getDisabledUsersDetails(string $search = '', ?int $limit = null,
*
* 200: Users details returned based on last logged in information
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function getLastLoggedInUsers(string $search = '',
?int $limit = null,
int $offset = 0,
Expand Down Expand Up @@ -447,6 +456,7 @@ public function addUser(
): DataResponse {
$user = $this->userSession->getUser();
$isAdmin = $this->groupManager->isAdmin($user->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($user->getUID());
$subAdminManager = $this->groupManager->getSubAdmin();

if (empty($userid) && $this->config->getAppValue('core', 'newUser.generateUserID', 'no') === 'yes') {
Expand All @@ -463,12 +473,12 @@ public function addUser(
if (!$this->groupManager->groupExists($group)) {
throw new OCSException($this->l10n->t('Group %1$s does not exist', [$group]), 104);
}
if (!$isAdmin && !$subAdminManager->isSubAdminOfGroup($user, $this->groupManager->get($group))) {
if (!$isAdmin && !($isDelegatedAdmin && $group !== 'admin') && !$subAdminManager->isSubAdminOfGroup($user, $this->groupManager->get($group))) {
throw new OCSException($this->l10n->t('Insufficient privileges for group %1$s', [$group]), 105);
}
}
} else {
if (!$isAdmin) {
if (!$isAdmin && !$isDelegatedAdmin) {
throw new OCSException($this->l10n->t('No group specified (required for sub-admins)'), 106);
}
}
Expand All @@ -486,7 +496,7 @@ public function addUser(
throw new OCSException($this->l10n->t('Cannot create sub-admins for admin group'), 103);
}
// Check if has permission to promote subadmins
if (!$subAdminManager->isSubAdminOfGroup($user, $group) && !$isAdmin) {
if (!$subAdminManager->isSubAdminOfGroup($user, $group) && !$isAdmin && !$isDelegatedAdmin) {
throw new OCSForbiddenException($this->l10n->t('No permissions to promote sub-admins'));
}
$subadminGroups[] = $group;
Expand Down Expand Up @@ -718,8 +728,10 @@ public function getEditableFieldsForUser(string $userId): DataResponse {
}

$subAdminManager = $this->groupManager->getSubAdmin();
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if (
!$this->groupManager->isAdmin($currentLoggedInUser->getUID())
!($isAdmin || $isDelegatedAdmin)
&& !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)
) {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
Expand Down Expand Up @@ -788,6 +800,7 @@ public function editUserMultiValue(
}

$subAdminManager = $this->groupManager->getSubAdmin();
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
$isAdminOrSubadmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID())
|| $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser);

Expand All @@ -798,7 +811,7 @@ public function editUserMultiValue(
$permittedFields[] = IAccountManager::COLLECTION_EMAIL . self::SCOPE_SUFFIX;
} else {
// Check if admin / subadmin
if ($isAdminOrSubadmin) {
if ($isAdminOrSubadmin || $isDelegatedAdmin && !$this->groupManager->isInGroup($targetUser->getUID(), 'admin')) {
// They have permissions over the user
$permittedFields[] = IAccountManager::COLLECTION_EMAIL;
} else {
Expand Down Expand Up @@ -903,14 +916,16 @@ public function editUser(string $userId, string $key, string $value): DataRespon
$permittedFields[] = self::USER_FIELD_NOTIFICATION_EMAIL;
if (
$this->config->getSystemValue('force_language', false) === false ||
$this->groupManager->isAdmin($currentLoggedInUser->getUID())
$this->groupManager->isAdmin($currentLoggedInUser->getUID()) ||
$this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID())
) {
$permittedFields[] = self::USER_FIELD_LANGUAGE;
}

if (
$this->config->getSystemValue('force_locale', false) === false ||
$this->groupManager->isAdmin($currentLoggedInUser->getUID())
$this->groupManager->isAdmin($currentLoggedInUser->getUID()) ||
$this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID())
) {
$permittedFields[] = self::USER_FIELD_LOCALE;
$permittedFields[] = self::USER_FIELD_FIRST_DAY_OF_WEEK;
Expand Down Expand Up @@ -942,15 +957,18 @@ public function editUser(string $userId, string $key, string $value): DataRespon
$permittedFields[] = IAccountManager::PROPERTY_AVATAR . self::SCOPE_SUFFIX;

// If admin they can edit their own quota and manager
if ($this->groupManager->isAdmin($currentLoggedInUser->getUID())) {
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if ($isAdmin || $isDelegatedAdmin) {
$permittedFields[] = self::USER_FIELD_QUOTA;
$permittedFields[] = self::USER_FIELD_MANAGER;
}
} else {
// Check if admin / subadmin
$subAdminManager = $this->groupManager->getSubAdmin();
if (
$this->groupManager->isAdmin($currentLoggedInUser->getUID())
$this->groupManager->isAdmin($currentLoggedInUser->getUID()) ||
$this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID()) && !$this->groupManager->isInGroup($targetUser->getUID(), 'admin')
|| $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)
) {
// They have permissions over the user
Expand Down Expand Up @@ -1217,7 +1235,9 @@ public function wipeUserDevices(string $userId): DataResponse {

// If not permitted
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if (!$isAdmin && !($isDelegatedAdmin && !$this->groupManager->isInGroup($targetUser->getUID(), 'admin')) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

Expand Down Expand Up @@ -1253,7 +1273,9 @@ public function deleteUser(string $userId): DataResponse {

// If not permitted
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if (!$isAdmin && !($isDelegatedAdmin && !$this->groupManager->isInGroup($targetUser->getUID(), 'admin')) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

Expand Down Expand Up @@ -1313,7 +1335,9 @@ private function setEnabled(string $userId, bool $value): DataResponse {

// If not permitted
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if (!$isAdmin && !($isDelegatedAdmin && !$this->groupManager->isInGroup($targetUser->getUID(), 'admin')) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

Expand Down Expand Up @@ -1342,7 +1366,9 @@ public function getUsersGroups(string $userId): DataResponse {
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
}

if ($targetUser->getUID() === $loggedInUser->getUID() || $this->groupManager->isAdmin($loggedInUser->getUID())) {
$isAdmin = $this->groupManager->isAdmin($loggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($loggedInUser->getUID());
if ($targetUser->getUID() === $loggedInUser->getUID() || $isAdmin || $isDelegatedAdmin) {
// Self lookup or admin lookup
return new DataResponse([
'groups' => $this->groupManager->getUserGroupIds($targetUser)
Expand Down Expand Up @@ -1401,7 +1427,9 @@ public function addToGroup(string $userId, string $groupid = ''): DataResponse {
// If they're not an admin, check they are a subadmin of the group in question
$loggedInUser = $this->userSession->getUser();
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
$isAdmin = $this->groupManager->isAdmin($loggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($loggedInUser->getUID());
if (!$isAdmin && !($isDelegatedAdmin && $groupid !== 'admin') && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
throw new OCSException('', 104);
}

Expand Down Expand Up @@ -1442,21 +1470,23 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse {

// If they're not an admin, check they are a subadmin of the group in question
$subAdminManager = $this->groupManager->getSubAdmin();
if (!$this->groupManager->isAdmin($loggedInUser->getUID()) && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
$isAdmin = $this->groupManager->isAdmin($loggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($loggedInUser->getUID());
if (!$isAdmin && !($isDelegatedAdmin && $groupid !== 'admin') && !$subAdminManager->isSubAdminOfGroup($loggedInUser, $group)) {
throw new OCSException('', 104);
}

// Check they aren't removing themselves from 'admin' or their 'subadmin; group
if ($targetUser->getUID() === $loggedInUser->getUID()) {
if ($this->groupManager->isAdmin($loggedInUser->getUID())) {
if ($isAdmin || $isDelegatedAdmin) {
if ($group->getGID() === 'admin') {
throw new OCSException($this->l10n->t('Cannot remove yourself from the admin group'), 105);
}
} else {
// Not an admin, so the user must be a subadmin of this group, but that is not allowed.
throw new OCSException($this->l10n->t('Cannot remove yourself from this group as you are a sub-admin'), 105);
}
} elseif (!$this->groupManager->isAdmin($loggedInUser->getUID())) {
} elseif (!($isAdmin || $isDelegatedAdmin)) {
/** @var IGroup[] $subAdminGroups */
$subAdminGroups = $subAdminManager->getSubAdminsGroups($loggedInUser);
$subAdminGroups = array_map(function (IGroup $subAdminGroup) {
Expand Down Expand Up @@ -1488,6 +1518,7 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse {
*
* 200: User added as group subadmin successfully
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function addSubAdmin(string $userId, string $groupid): DataResponse {
$group = $this->groupManager->get($groupid);
$user = $this->userManager->get($userId);
Expand Down Expand Up @@ -1528,6 +1559,7 @@ public function addSubAdmin(string $userId, string $groupid): DataResponse {
*
* 200: User removed as group subadmin successfully
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function removeSubAdmin(string $userId, string $groupid): DataResponse {
$group = $this->groupManager->get($groupid);
$user = $this->userManager->get($userId);
Expand Down Expand Up @@ -1560,6 +1592,7 @@ public function removeSubAdmin(string $userId, string $groupid): DataResponse {
*
* 200: User subadmin groups returned
*/
#[AuthorizedAdminSetting(settings:Users::class)]
public function getUserSubAdminGroups(string $userId): DataResponse {
$groups = $this->getUserSubAdminGroupsData($userId);
return new DataResponse($groups);
Expand Down Expand Up @@ -1587,9 +1620,11 @@ public function resendWelcomeMessage(string $userId): DataResponse {

// Check if admin / subadmin
$subAdminManager = $this->groupManager->getSubAdmin();
$isAdmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID());
$isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID());
if (
!$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)
&& !$this->groupManager->isAdmin($currentLoggedInUser->getUID())
&& !($isAdmin || $isDelegatedAdmin)
) {
// No rights
throw new OCSException('', OCSController::RESPOND_NOT_FOUND);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,17 @@ protected function setUp(): void {
);
}

protected function tearDown(): void {
$this->userSession->setUser(null);
}

public function testGetAppInfo() {
$result = $this->api->getAppInfo('provisioning_api');
$expected = $this->appManager->getAppInfo('provisioning_api');
$this->assertEquals($expected, $result->getData());
}


public function testGetAppInfoOnBadAppID() {
$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
$this->expectExceptionCode(998);
Expand Down
Loading

0 comments on commit 7266a9e

Please sign in to comment.