From 1af827fdb39dd182743853e17cf5493a8b4637d2 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 11 Jul 2024 11:54:06 +0200 Subject: [PATCH 1/6] fix(users): Improve error handling of some fields update Signed-off-by: Louis Chemineau --- .../settings/src/components/Users/UserRow.vue | 56 +++++++++++-------- apps/settings/src/store/users.js | 27 +++++---- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/apps/settings/src/components/Users/UserRow.vue b/apps/settings/src/components/Users/UserRow.vue index 39ecebb968f..dbe17db8d1d 100644 --- a/apps/settings/src/components/Users/UserRow.vue +++ b/apps/settings/src/components/Users/UserRow.vue @@ -624,18 +624,21 @@ export default { * * @param {string} displayName The display name */ - updateDisplayName() { + async updateDisplayName() { this.loading.displayName = true - this.$store.dispatch('setUserData', { - userid: this.user.id, - key: 'displayname', - value: this.editedDisplayName, - }).then(() => { - this.loading.displayName = false + try { + await this.$store.dispatch('setUserData', { + userid: this.user.id, + key: 'displayname', + value: this.editedDisplayName, + }) + if (this.editedDisplayName === this.user.displayname) { showSuccess(t('setting', 'Display name was successfully changed')) } - }) + } finally { + this.loading.displayName = false + } }, /** @@ -643,21 +646,23 @@ export default { * * @param {string} password The email address */ - updatePassword() { + async updatePassword() { this.loading.password = true if (this.editedPassword.length === 0) { showError(t('setting', "Password can't be empty")) this.loading.password = false } else { - this.$store.dispatch('setUserData', { - userid: this.user.id, - key: 'password', - value: this.editedPassword, - }).then(() => { - this.loading.password = false + try { + await this.$store.dispatch('setUserData', { + userid: this.user.id, + key: 'password', + value: this.editedPassword, + }) this.editedPassword = '' showSuccess(t('setting', 'Password was successfully changed')) - }) + } finally { + this.loading.password = false + } } }, @@ -666,23 +671,26 @@ export default { * * @param {string} mailAddress The email address */ - updateEmail() { + async updateEmail() { this.loading.mailAddress = true if (this.editedMail === '') { showError(t('setting', "Email can't be empty")) this.loading.mailAddress = false this.editedMail = this.user.email } else { - this.$store.dispatch('setUserData', { - userid: this.user.id, - key: 'email', - value: this.editedMail, - }).then(() => { - this.loading.mailAddress = false + try { + await this.$store.dispatch('setUserData', { + userid: this.user.id, + key: 'email', + value: this.editedMail, + }) + if (this.editedMail === this.user.email) { showSuccess(t('setting', 'Email was successfully changed')) } - }) + } finally { + this.loading.mailAddress = false + } } }, diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js index 8b60b3ab328..15c40d77072 100644 --- a/apps/settings/src/store/users.js +++ b/apps/settings/src/store/users.js @@ -641,11 +641,14 @@ const actions = { * @param {string} userid User id * @return {Promise} */ - wipeUserDevices(context, userid) { - return api.requireAdmin().then((response) => { - return api.post(generateOcsUrl('cloud/users/{userid}/wipe', { userid })) - .catch((error) => { throw error }) - }).catch((error) => context.commit('API_FAILURE', { userid, error })) + async wipeUserDevices(context, userid) { + try { + await api.requireAdmin() + return await api.post(generateOcsUrl('cloud/users/{userid}/wipe', { userid })) + } catch (error) { + context.commit('API_FAILURE', { userid, error }) + return Promise.reject(new Error('Failed to wipe user devices')) + } }, /** @@ -735,7 +738,7 @@ const actions = { * @param {string} options.value Value of the change * @return {Promise} */ - setUserData(context, { userid, key, value }) { + async setUserData(context, { userid, key, value }) { const allowedEmpty = ['email', 'displayname', 'manager'] if (['email', 'language', 'quota', 'displayname', 'password', 'manager'].indexOf(key) !== -1) { // We allow empty email or displayname @@ -745,11 +748,13 @@ const actions = { || allowedEmpty.indexOf(key) !== -1 ) ) { - return api.requireAdmin().then((response) => { - return api.put(generateOcsUrl('cloud/users/{userid}', { userid }), { key, value }) - .then((response) => context.commit('setUserData', { userid, key, value })) - .catch((error) => { throw error }) - }).catch((error) => context.commit('API_FAILURE', { userid, error })) + try { + await api.requireAdmin() + await api.put(generateOcsUrl('cloud/users/{userid}', { userid }), { key, value }) + return context.commit('setUserData', { userid, key, value }) + } catch (error) { + context.commit('API_FAILURE', { userid, error }) + } } } return Promise.reject(new Error('Invalid request data')) From dff881544920f426b984f91b7bc8dece1f351342 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Thu, 11 Jul 2024 12:09:39 +0200 Subject: [PATCH 2/6] feat(users): Add support for admin delegation for users and groups management Signed-off-by: Louis Chemineau --- .../lib/Controller/AUserData.php | 4 +- .../lib/Controller/GroupsController.php | 15 +++- .../lib/Controller/UsersController.php | 77 ++++++++++++++----- apps/settings/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Controller/UsersController.php | 10 ++- apps/settings/lib/Settings/Admin/Users.php | 61 +++++++++++++++ lib/private/Group/Manager.php | 13 ++++ lib/private/Group/MetaData.php | 25 ++---- lib/private/SubAdmin.php | 5 ++ lib/public/IGroupManager.php | 8 ++ tests/lib/Group/MetaDataTest.php | 14 ++-- 13 files changed, 182 insertions(+), 53 deletions(-) create mode 100644 apps/settings/lib/Settings/Admin/Users.php diff --git a/apps/provisioning_api/lib/Controller/AUserData.php b/apps/provisioning_api/lib/Controller/AUserData.php index d7db48dc33f..1b8e759bcce 100644 --- a/apps/provisioning_api/lib/Controller/AUserData.php +++ b/apps/provisioning_api/lib/Controller/AUserData.php @@ -98,7 +98,9 @@ abstract class AUserData extends OCSController { } $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 { @@ -116,7 +118,7 @@ abstract class AUserData extends OCSController { $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) diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 9320fe13aaa..97480058fd1 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -9,8 +9,10 @@ declare(strict_types=1); 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; @@ -154,8 +156,9 @@ class GroupsController extends AUserData { } // 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 */ @@ -197,7 +200,9 @@ class GroupsController extends AUserData { } // 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 @@ -237,6 +242,7 @@ class GroupsController extends AUserData { * * 200: Group created successfully */ + #[AuthorizedAdminSetting(settings:Users::class)] public function addGroup(string $groupid, string $displayname = ''): DataResponse { // Validate name if (empty($groupid)) { @@ -270,6 +276,7 @@ class GroupsController extends AUserData { * * 200: Group updated successfully */ + #[AuthorizedAdminSetting(settings:Users::class)] public function updateGroup(string $groupId, string $key, string $value): DataResponse { $groupId = urldecode($groupId); @@ -299,6 +306,7 @@ class GroupsController extends AUserData { * * 200: Group deleted successfully */ + #[AuthorizedAdminSetting(settings:Users::class)] public function deleteGroup(string $groupId): DataResponse { $groupId = urldecode($groupId); @@ -322,6 +330,7 @@ class GroupsController extends AUserData { * * 200: Sub admins returned */ + #[AuthorizedAdminSetting(settings:Users::class)] public function getSubAdminsOfGroup(string $groupId): DataResponse { // Check group exists $targetGroup = $this->groupManager->get($groupId); diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 1cdc1392596..a6bb2330390 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -16,10 +16,12 @@ use OC\KnownUser\KnownUserService; 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; @@ -101,7 +103,9 @@ class UsersController extends AUserData { // 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); @@ -142,7 +146,9 @@ class UsersController extends AUserData { // 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)) { @@ -213,7 +219,9 @@ class UsersController extends AUserData { // 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)) { @@ -275,6 +283,7 @@ class UsersController extends AUserData { * * 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, @@ -447,6 +456,7 @@ class UsersController extends AUserData { ): 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') { @@ -463,12 +473,12 @@ class UsersController extends AUserData { 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); } } @@ -486,7 +496,7 @@ class UsersController extends AUserData { 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; @@ -718,8 +728,10 @@ class UsersController extends AUserData { } $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); @@ -788,6 +800,7 @@ class UsersController extends AUserData { } $subAdminManager = $this->groupManager->getSubAdmin(); + $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($currentLoggedInUser->getUID()); $isAdminOrSubadmin = $this->groupManager->isAdmin($currentLoggedInUser->getUID()) || $subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser); @@ -798,7 +811,7 @@ class UsersController extends AUserData { $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 { @@ -903,14 +916,16 @@ class UsersController extends AUserData { $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; } @@ -941,7 +956,9 @@ class UsersController extends AUserData { $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; } @@ -949,7 +966,8 @@ class UsersController extends AUserData { // 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 @@ -1204,7 +1222,9 @@ class UsersController extends AUserData { // 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); } @@ -1240,7 +1260,9 @@ class UsersController extends AUserData { // 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); } @@ -1300,7 +1322,9 @@ class UsersController extends AUserData { // 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); } @@ -1329,7 +1353,9 @@ class UsersController extends AUserData { 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) @@ -1388,7 +1414,9 @@ class UsersController extends AUserData { // 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); } @@ -1429,13 +1457,15 @@ class UsersController extends AUserData { // 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); } @@ -1443,7 +1473,7 @@ class UsersController extends AUserData { // 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) { @@ -1475,6 +1505,7 @@ class UsersController extends AUserData { * * 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); @@ -1515,6 +1546,7 @@ class UsersController extends AUserData { * * 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); @@ -1547,6 +1579,7 @@ class UsersController extends AUserData { * * 200: User subadmin groups returned */ + #[AuthorizedAdminSetting(settings:Users::class)] public function getUserSubAdminGroups(string $userId): DataResponse { $groups = $this->getUserSubAdminGroupsData($userId); return new DataResponse($groups); @@ -1574,9 +1607,11 @@ class UsersController extends AUserData { // 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); diff --git a/apps/settings/appinfo/info.xml b/apps/settings/appinfo/info.xml index a87e459aaf7..2674b97b57d 100644 --- a/apps/settings/appinfo/info.xml +++ b/apps/settings/appinfo/info.xml @@ -34,6 +34,7 @@ OCA\Settings\Settings\Admin\Sharing OCA\Settings\Settings\Admin\Security OCA\Settings\Settings\Admin\Delegation + OCA\Settings\Settings\Admin\Users OCA\Settings\Sections\Admin\Additional OCA\Settings\Sections\Admin\Delegation OCA\Settings\Sections\Admin\Groupware diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 27c1496008e..41f70c3a8e6 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -71,6 +71,7 @@ return array( 'OCA\\Settings\\Settings\\Admin\\Security' => $baseDir . '/../lib/Settings/Admin/Security.php', 'OCA\\Settings\\Settings\\Admin\\Server' => $baseDir . '/../lib/Settings/Admin/Server.php', 'OCA\\Settings\\Settings\\Admin\\Sharing' => $baseDir . '/../lib/Settings/Admin/Sharing.php', + 'OCA\\Settings\\Settings\\Admin\\Users' => $baseDir . '/../lib/Settings/Admin/Users.php', 'OCA\\Settings\\Settings\\Personal\\Additional' => $baseDir . '/../lib/Settings/Personal/Additional.php', 'OCA\\Settings\\Settings\\Personal\\PersonalInfo' => $baseDir . '/../lib/Settings/Personal/PersonalInfo.php', 'OCA\\Settings\\Settings\\Personal\\Security\\Authtokens' => $baseDir . '/../lib/Settings/Personal/Security/Authtokens.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 14e4c362887..4fa905b55bb 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -86,6 +86,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\Settings\\Admin\\Security' => __DIR__ . '/..' . '/../lib/Settings/Admin/Security.php', 'OCA\\Settings\\Settings\\Admin\\Server' => __DIR__ . '/..' . '/../lib/Settings/Admin/Server.php', 'OCA\\Settings\\Settings\\Admin\\Sharing' => __DIR__ . '/..' . '/../lib/Settings/Admin/Sharing.php', + 'OCA\\Settings\\Settings\\Admin\\Users' => __DIR__ . '/..' . '/../lib/Settings/Admin/Users.php', 'OCA\\Settings\\Settings\\Personal\\Additional' => __DIR__ . '/..' . '/../lib/Settings/Personal/Additional.php', 'OCA\\Settings\\Settings\\Personal\\PersonalInfo' => __DIR__ . '/..' . '/../lib/Settings/Personal/PersonalInfo.php', 'OCA\\Settings\\Settings\\Personal\\Security\\Authtokens' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/Authtokens.php', diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 999f883bad8..1e934cdf43c 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -19,12 +19,14 @@ use OC\Security\IdentityProof\Manager; use OC\User\Manager as UserManager; use OCA\Settings\BackgroundJobs\VerifyUserData; use OCA\Settings\Events\BeforeTemplateRenderedEvent; +use OCA\Settings\Settings\Admin\Users; use OCA\User_LDAP\User_Proxy; use OCP\Accounts\IAccount; use OCP\Accounts\IAccountManager; use OCP\Accounts\PropertyDoesNotExistException; use OCP\App\IAppManager; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Attribute\OpenAPI; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\Http\JSONResponse; @@ -93,6 +95,7 @@ class UsersController extends Controller { $user = $this->userSession->getUser(); $uid = $user->getUID(); $isAdmin = $this->groupManager->isAdmin($uid); + $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid); \OC::$server->getNavigationManager()->setActiveEntry('core_users'); @@ -118,6 +121,7 @@ class UsersController extends Controller { $groupsInfo = new \OC\Group\MetaData( $uid, $isAdmin, + $isDelegatedAdmin, $this->groupManager, $this->userSession ); @@ -135,7 +139,7 @@ class UsersController extends Controller { $userCount = 0; if (!$isLDAPUsed) { - if ($isAdmin) { + if ($isAdmin || $isDelegatedAdmin) { $disabledUsers = $this->userManager->countDisabledUsers(); $userCount = array_reduce($this->userManager->countUsers(), function ($v, $w) { return $v + (int)$w; @@ -200,7 +204,8 @@ class UsersController extends Controller { // groups $serverData['groups'] = array_merge_recursive($adminGroup, [$recentUsersGroup, $disabledUsersGroup], $groups); // Various data - $serverData['isAdmin'] = $isAdmin; + $serverData['isAdmin'] = $isAdmin || $isDelegatedAdmin; + $serverData['isDelegatedAdmin'] = $isDelegatedAdmin; $serverData['sortGroups'] = $forceSortGroupByName ? \OC\Group\MetaData::SORT_GROUPNAME : (int)$this->config->getAppValue('core', 'group.sortBy', (string)\OC\Group\MetaData::SORT_USERCOUNT); @@ -232,6 +237,7 @@ class UsersController extends Controller { * * @return JSONResponse */ + #[AuthorizedAdminSetting(settings:Users::class)] public function setPreference(string $key, string $value): JSONResponse { $allowed = ['newUser.sendEmail', 'group.sortBy']; if (!in_array($key, $allowed, true)) { diff --git a/apps/settings/lib/Settings/Admin/Users.php b/apps/settings/lib/Settings/Admin/Users.php new file mode 100644 index 00000000000..3af018e0cf1 --- /dev/null +++ b/apps/settings/lib/Settings/Admin/Users.php @@ -0,0 +1,61 @@ + */ class($this->appName, '') extends TemplateResponse { + public function render(): string { + return ''; + } + }; + } + + public function getSection(): ?string { + return 'admindelegation'; + } + + /** + * @return int whether the form should be rather on the top or bottom of + * the admin section. The forms are arranged in ascending order of the + * priority values. It is required to return a value between 0 and 100. + * + * E.g.: 70 + */ + public function getPriority(): int { + return 0; + } + + public function getName(): string { + return $this->l10n->t('Users'); + } + + public function getAuthorizedAppConfig(): array { + return []; + } +} diff --git a/lib/private/Group/Manager.php b/lib/private/Group/Manager.php index 092c56fe00a..cad8e76fbbd 100644 --- a/lib/private/Group/Manager.php +++ b/lib/private/Group/Manager.php @@ -8,6 +8,7 @@ namespace OC\Group; use OC\Hooks\PublicEmitter; +use OC\Settings\AuthorizedGroupMapper; use OCP\EventDispatcher\IEventDispatcher; use OCP\Group\Backend\IBatchMethodsBackend; use OCP\Group\Backend\ICreateNamedGroupBackend; @@ -333,6 +334,18 @@ class Manager extends PublicEmitter implements IGroupManager { return $this->isInGroup($userId, 'admin'); } + public function isDelegatedAdmin(string $userId): bool { + if (!$this->remoteAddress->allowsAdminActions()) { + return false; + } + + // Check if the user as admin delegation for users listing + $authorizedGroupMapper = \OCP\Server::get(AuthorizedGroupMapper::class); + $user = $this->userManager->get($userId); + $authorizedClasses = $authorizedGroupMapper->findAllClassesForUser($user); + return in_array(\OCA\Settings\Settings\Admin\Users::class, $authorizedClasses, true); + } + /** * Checks if a userId is in a group * diff --git a/lib/private/Group/MetaData.php b/lib/private/Group/MetaData.php index 638dc184812..da553c89a7b 100644 --- a/lib/private/Group/MetaData.php +++ b/lib/private/Group/MetaData.php @@ -17,33 +17,22 @@ class MetaData { public const SORT_USERCOUNT = 1; // May have performance issues on LDAP backends public const SORT_GROUPNAME = 2; - /** @var string */ - protected $user; - /** @var bool */ - protected $isAdmin; /** @var array */ protected $metaData = []; - /** @var GroupManager */ - protected $groupManager; /** @var int */ protected $sorting = self::SORT_NONE; - /** @var IUserSession */ - protected $userSession; /** * @param string $user the uid of the current user * @param bool $isAdmin whether the current users is an admin */ public function __construct( - string $user, - bool $isAdmin, - IGroupManager $groupManager, - IUserSession $userSession + private string $user, + private bool $isAdmin, + private bool $isDelegatedAdmin, + private IGroupManager $groupManager, + private IUserSession $userSession ) { - $this->user = $user; - $this->isAdmin = $isAdmin; - $this->groupManager = $groupManager; - $this->userSession = $userSession; } /** @@ -162,11 +151,11 @@ class MetaData { * @return IGroup[] */ public function getGroups(string $search = ''): array { - if ($this->isAdmin) { + if ($this->isAdmin || $this->isDelegatedAdmin) { return $this->groupManager->search($search); } else { $userObject = $this->userSession->getUser(); - if ($userObject !== null) { + if ($userObject !== null && $this->groupManager instanceof GroupManager) { $groups = $this->groupManager->getSubAdmin()->getSubAdminsGroups($userObject); } else { $groups = []; diff --git a/lib/private/SubAdmin.php b/lib/private/SubAdmin.php index 9ffa0ab03a3..c025ab7b012 100644 --- a/lib/private/SubAdmin.php +++ b/lib/private/SubAdmin.php @@ -233,6 +233,11 @@ class SubAdmin extends PublicEmitter implements ISubAdmin { return true; } + // Check if the user is already an admin + if ($this->groupManager->isDelegatedAdmin($user->getUID())) { + return true; + } + $qb = $this->dbConn->getQueryBuilder(); $result = $qb->select('gid') diff --git a/lib/public/IGroupManager.php b/lib/public/IGroupManager.php index d4ea3f70722..ee88990df79 100644 --- a/lib/public/IGroupManager.php +++ b/lib/public/IGroupManager.php @@ -114,6 +114,14 @@ interface IGroupManager { */ public function isAdmin($userId); + /** + * Checks if a userId is eligible to users administration delegation + * @param string $userId + * @return bool if delegated admin + * @since 30.0.0 + */ + public function isDelegatedAdmin(string $userId): bool; + /** * Checks if a userId is in a group * @param string $userId diff --git a/tests/lib/Group/MetaDataTest.php b/tests/lib/Group/MetaDataTest.php index 2ae33c63a25..0c9029aaf56 100644 --- a/tests/lib/Group/MetaDataTest.php +++ b/tests/lib/Group/MetaDataTest.php @@ -10,14 +10,11 @@ namespace Test\Group; use OCP\IUserSession; class MetaDataTest extends \Test\TestCase { - /** @var \OC\Group\Manager */ - private $groupManager; - /** @var \OCP\IUserSession */ - private $userSession; - /** @var \OC\Group\MetaData */ - private $groupMetadata; - /** @var bool */ - private $isAdmin = true; + private \OC\Group\Manager $groupManager; + private IUserSession $userSession; + private \OC\Group\MetaData $groupMetadata; + private bool $isAdmin = true; + private bool $isDelegatedAdmin = true; protected function setUp(): void { parent::setUp(); @@ -28,6 +25,7 @@ class MetaDataTest extends \Test\TestCase { $this->groupMetadata = new \OC\Group\MetaData( 'foo', $this->isAdmin, + $this->isDelegatedAdmin, $this->groupManager, $this->userSession ); From 26beedef36cfe4b94837c1e8e98275604f22e03b Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Tue, 16 Jul 2024 14:39:58 +0200 Subject: [PATCH 3/6] test(users): Adapt tests to new admin delegation checks Signed-off-by: Louis Chemineau --- .../tests/Controller/AppsControllerTest.php | 6 +- .../tests/Controller/UsersControllerTest.php | 74 ++++++++++++------- .../AdminSettingsControllerTest.php | 2 + 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/apps/provisioning_api/tests/Controller/AppsControllerTest.php b/apps/provisioning_api/tests/Controller/AppsControllerTest.php index 66c873e5327..b59433b365f 100644 --- a/apps/provisioning_api/tests/Controller/AppsControllerTest.php +++ b/apps/provisioning_api/tests/Controller/AppsControllerTest.php @@ -45,13 +45,17 @@ class AppsControllerTest extends \OCA\Provisioning_API\Tests\TestCase { ); } + 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); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 978530e55e1..d2b0a3a4c38 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -232,7 +232,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -263,7 +263,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -299,7 +299,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -344,7 +344,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -456,7 +456,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -502,7 +502,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -552,7 +552,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -595,7 +595,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -624,7 +624,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -706,7 +706,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('adminUser'); $this->userSession @@ -732,7 +732,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('regularUser'); $this->userSession @@ -765,7 +765,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('regularUser'); $this->userSession @@ -814,7 +814,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('subAdminUser'); $this->userSession @@ -931,7 +931,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('admin'); $targetUser = $this->getMockBuilder(IUser::class) @@ -1077,7 +1077,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class) @@ -1223,7 +1223,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->exactly(3)) + ->expects($this->exactly(4)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class) @@ -1263,7 +1263,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('UID'); $targetUser = $this->getMockBuilder(IUser::class) @@ -2662,7 +2662,7 @@ class UsersControllerTest extends TestCase { public function testGetUsersGroupsSelfTargetted() { $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('UserToLookup'); $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); @@ -2691,7 +2691,7 @@ class UsersControllerTest extends TestCase { public function testGetUsersGroupsForAdminUser() { $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('admin'); $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); @@ -2725,7 +2725,7 @@ class UsersControllerTest extends TestCase { public function testGetUsersGroupsForSubAdminUserAndUserIsAccessible() { $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); @@ -2789,7 +2789,7 @@ class UsersControllerTest extends TestCase { $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); @@ -2873,7 +2873,7 @@ class UsersControllerTest extends TestCase { $targetUser = $this->createMock(IUser::class); $loggedInUser = $this->createMock(IUser::class); - $loggedInUser->expects($this->once()) + $loggedInUser->expects($this->exactly(2)) ->method('getUID') ->willReturn('subadmin'); @@ -2917,7 +2917,7 @@ class UsersControllerTest extends TestCase { public function testAddToGroupSuccessAsSubadmin() { $targetUser = $this->createMock(IUser::class); $loggedInUser = $this->createMock(IUser::class); - $loggedInUser->expects($this->once()) + $loggedInUser->expects($this->exactly(2)) ->method('getUID') ->willReturn('subadmin'); @@ -2961,7 +2961,7 @@ class UsersControllerTest extends TestCase { public function testAddToGroupSuccessAsAdmin() { $targetUser = $this->createMock(IUser::class); $loggedInUser = $this->createMock(IUser::class); - $loggedInUser->expects($this->once()) + $loggedInUser->expects($this->exactly(2)) ->method('getUID') ->willReturn('admin'); @@ -3079,7 +3079,7 @@ class UsersControllerTest extends TestCase { $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->once()) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('unauthorizedUser'); $targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); @@ -3600,7 +3600,7 @@ class UsersControllerTest extends TestCase { ->willReturn($targetUser); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('admin'); $this->userSession @@ -3627,7 +3627,7 @@ class UsersControllerTest extends TestCase { ->willReturn($targetUser); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('getUID') ->willReturn('admin'); $this->userSession @@ -3814,7 +3814,7 @@ class UsersControllerTest extends TestCase { ->disableOriginalConstructor() ->getMock(); $loggedInUser - ->expects($this->exactly(1)) + ->expects($this->exactly(2)) ->method('getUID') ->willReturn('subadmin'); $targetUser = $this->getMockBuilder(IUser::class) @@ -3883,6 +3883,10 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getSubAdmin') ->willReturn($subAdminManager); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->willReturn('logged-user-id'); $targetUser ->expects($this->once()) ->method('getEmailAddress') @@ -3924,6 +3928,10 @@ class UsersControllerTest extends TestCase { ->expects($this->once()) ->method('getSubAdmin') ->willReturn($subAdminManager); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->willReturn('logged-user-id'); $targetUser ->expects($this->once()) ->method('getEmailAddress') @@ -3939,6 +3947,9 @@ class UsersControllerTest extends TestCase { $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $loggedInUser + ->method('getUID') + ->willReturn('logged-user-id'); $targetUser ->method('getUID') ->willReturn('user-id'); @@ -3987,6 +3998,9 @@ class UsersControllerTest extends TestCase { $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $loggedInUser + ->method('getUID') + ->willReturn('logged-user-id'); $targetUser ->method('getUID') ->willReturn('user-id'); @@ -4040,6 +4054,10 @@ class UsersControllerTest extends TestCase { $targetUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); + $loggedInUser + ->expects($this->exactly(2)) + ->method('getUID') + ->willReturn('logged-user-id'); $targetUser ->method('getUID') ->willReturn('user-id'); diff --git a/apps/settings/tests/Controller/AdminSettingsControllerTest.php b/apps/settings/tests/Controller/AdminSettingsControllerTest.php index 6f4a941011e..578348a3031 100644 --- a/apps/settings/tests/Controller/AdminSettingsControllerTest.php +++ b/apps/settings/tests/Controller/AdminSettingsControllerTest.php @@ -81,6 +81,8 @@ class AdminSettingsControllerTest extends TestCase { protected function tearDown(): void { \OC::$server->getUserManager()->get($this->adminUid)->delete(); + \OC_User::setUserId(null); + \OC::$server->getUserSession()->setUser(null); parent::tearDown(); } From 3574d7eec8673ce506b814707d8dc213f5db2336 Mon Sep 17 00:00:00 2001 From: Louis Chemineau Date: Mon, 22 Jul 2024 12:33:34 +0200 Subject: [PATCH 4/6] feat(users): Enable features for delegated user admins Signed-off-by: Louis Chemineau --- apps/settings/lib/Controller/UsersController.php | 2 +- apps/settings/src/components/GroupListItem.vue | 4 ++-- apps/settings/src/components/Users/NewUserDialog.vue | 4 ++-- apps/settings/src/components/Users/UserListHeader.vue | 2 +- apps/settings/src/components/Users/UserRow.vue | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 1e934cdf43c..823d3d4cb8b 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -204,7 +204,7 @@ class UsersController extends Controller { // groups $serverData['groups'] = array_merge_recursive($adminGroup, [$recentUsersGroup, $disabledUsersGroup], $groups); // Various data - $serverData['isAdmin'] = $isAdmin || $isDelegatedAdmin; + $serverData['isAdmin'] = $isAdmin; $serverData['isDelegatedAdmin'] = $isDelegatedAdmin; $serverData['sortGroups'] = $forceSortGroupByName ? \OC\Group\MetaData::SORT_GROUPNAME diff --git a/apps/settings/src/components/GroupListItem.vue b/apps/settings/src/components/GroupListItem.vue index 98a96c9b4ef..44b0605c9de 100644 --- a/apps/settings/src/components/GroupListItem.vue +++ b/apps/settings/src/components/GroupListItem.vue @@ -45,7 +45,7 @@ -