diff options
author | Ferdinand Thiessen <opensource@fthiessen.de> | 2025-04-24 13:10:37 +0200 |
---|---|---|
committer | Ferdinand Thiessen <opensource@fthiessen.de> | 2025-04-27 11:35:40 +0200 |
commit | 6d8c49f0d4bc6c705fe42274ebe045b837c6ca87 (patch) | |
tree | b88f6f2afad2ad230c0f880014e312f3e614ec43 | |
parent | 8cc0c266d5ce2dd1966c8285d7b347ec1b3a0e98 (diff) | |
download | nextcloud-server-6d8c49f0d4bc6c705fe42274ebe045b837c6ca87.tar.gz nextcloud-server-6d8c49f0d4bc6c705fe42274ebe045b837c6ca87.zip |
fix(settings): group admins only can add users to their groups
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
-rw-r--r-- | apps/settings/lib/Controller/UsersController.php | 21 | ||||
-rw-r--r-- | apps/settings/src/components/AppNavigationGroupList.vue | 18 | ||||
-rw-r--r-- | apps/settings/src/components/UserList.vue | 6 | ||||
-rw-r--r-- | apps/settings/src/components/Users/NewUserDialog.vue | 17 | ||||
-rw-r--r-- | apps/settings/src/store/users.js | 20 | ||||
-rw-r--r-- | cypress/e2e/settings/users-group-admin.cy.ts | 186 |
6 files changed, 239 insertions, 29 deletions
diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index f9277affef8..fd5831cb2be 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -40,6 +40,7 @@ use OCP\AppFramework\Services\IInitialState; use OCP\BackgroundJob\IJobList; use OCP\Encryption\IManager; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Group\ISubAdmin; use OCP\IConfig; use OCP\IGroup; use OCP\IGroupManager; @@ -50,7 +51,6 @@ use OCP\IUser; use OCP\IUserSession; use OCP\L10N\IFactory; use OCP\Mail\IMailer; -use OCP\Server; use OCP\Util; use function in_array; @@ -100,13 +100,13 @@ class UsersController extends Controller { */ #[NoAdminRequired] #[NoCSRFRequired] - public function usersList(): TemplateResponse { + public function usersList(INavigationManager $navigationManager, ISubAdmin $subAdmin): TemplateResponse { $user = $this->userSession->getUser(); $uid = $user->getUID(); $isAdmin = $this->groupManager->isAdmin($uid); $isDelegatedAdmin = $this->groupManager->isDelegatedAdmin($uid); - Server::get(INavigationManager::class)->setActiveEntry('core_users'); + $navigationManager->setActiveEntry('core_users'); /* SORT OPTION: SORT_USERCOUNT or SORT_GROUPNAME */ $sortGroupsBy = MetaData::SORT_USERCOUNT; @@ -182,6 +182,14 @@ class UsersController extends Controller { 'usercount' => $disabledUsers ]; + if (!$isAdmin && !$isDelegatedAdmin) { + $subAdminGroups = array_map( + fn (IGroup $group) => ['id' => $group->getGID(), 'name' => $group->getDisplayName()], + $subAdmin->getSubAdminsGroups($user), + ); + $subAdminGroups = array_values($subAdminGroups); + } + /* QUOTAS PRESETS */ $quotaPreset = $this->parseQuotaPreset($this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB')); $allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1'; @@ -205,12 +213,7 @@ class UsersController extends Controller { $serverData = []; // groups $serverData['systemGroups'] = [$adminGroupData, $recentUsersGroup, $disabledUsersGroup]; - $serverData['userGroups'] = array_values( - array_map( - fn (IGroup $group) => ['id' => $group->getGID(), 'name' => $group->getDisplayName()], - $this->groupManager->getUserGroups($user), - ), - ); + $serverData['subAdminGroups'] = $subAdminGroups ?? []; // Various data $serverData['isAdmin'] = $isAdmin; $serverData['isDelegatedAdmin'] = $isDelegatedAdmin; diff --git a/apps/settings/src/components/AppNavigationGroupList.vue b/apps/settings/src/components/AppNavigationGroupList.vue index b32a07bc9b8..5c648a17098 100644 --- a/apps/settings/src/components/AppNavigationGroupList.vue +++ b/apps/settings/src/components/AppNavigationGroupList.vue @@ -57,12 +57,16 @@ </template> <script setup lang="ts"> +import type CancelablePromise from 'cancelable-promise' +import type { IGroup } from '../views/user-types.d.ts' + +import { mdiAccountGroup, mdiPlus } from '@mdi/js' +import { showError } from '@nextcloud/dialogs' +import { t } from '@nextcloud/l10n' +import { useElementVisibility } from '@vueuse/core' import { computed, ref, watch, onBeforeMount } from 'vue' import { Fragment } from 'vue-frag' import { useRoute, useRouter } from 'vue-router/composables' -import { useElementVisibility } from '@vueuse/core' -import { showError } from '@nextcloud/dialogs' -import { mdiAccountGroup, mdiPlus } from '@mdi/js' import NcActionInput from '@nextcloud/vue/components/NcActionInput' import NcActionText from '@nextcloud/vue/components/NcActionText' @@ -137,12 +141,16 @@ watch(groupsSearchQuery, async () => { }) /** Cancelable promise for search groups request */ -const promise = ref(null) +const promise = ref<CancelablePromise<IGroup[]>>() /** * Load groups */ async function loadGroups() { + if (!isAdminOrDelegatedAdmin.value) { + return + } + if (promise.value) { promise.value.cancel() } @@ -163,7 +171,7 @@ async function loadGroups() { } catch (error) { logger.error(t('settings', 'Failed to load groups'), { error }) } - promise.value = null + promise.value = undefined loadingGroups.value = false } diff --git a/apps/settings/src/components/UserList.vue b/apps/settings/src/components/UserList.vue index 5d6bd5f04ee..84c204805cc 100644 --- a/apps/settings/src/components/UserList.vue +++ b/apps/settings/src/components/UserList.vue @@ -350,11 +350,13 @@ export default { setNewUserDefaultGroup(value) { // Is no value set, but user is a line manager we set their group as this is a requirement for line manager if (!value && !this.settings.isAdmin && !this.settings.isDelegatedAdmin) { + const groups = this.$store.getters.getSubAdminGroups // if there are multiple groups we do not know which to add, // so we cannot make the managers life easier by preselecting it. - if (this.groups.length === 1) { - value = this.groups[0].id + if (groups.length === 1) { + this.newUser.groups = [...groups] } + return } if (value) { diff --git a/apps/settings/src/components/Users/NewUserDialog.vue b/apps/settings/src/components/Users/NewUserDialog.vue index 3e50efc2072..19445bc187e 100644 --- a/apps/settings/src/components/Users/NewUserDialog.vue +++ b/apps/settings/src/components/Users/NewUserDialog.vue @@ -61,6 +61,7 @@ :required="newUser.password === '' || settings.newUserRequireEmail" /> <div class="dialog__item"> <NcSelect class="dialog__select" + data-test="groups" :input-label="!settings.isAdmin && !settings.isDelegatedAdmin ? t('settings', 'Member of the following groups (required)') : t('settings', 'Member of the following groups')" :placeholder="t('settings', 'Set account groups')" :disabled="loading.groups || loading.all" @@ -69,7 +70,7 @@ label="name" :close-on-select="false" :multiple="true" - :taggable="true" + :taggable="settings.isAdmin || settings.isDelegatedAdmin" :required="!settings.isAdmin && !settings.isDelegatedAdmin" :create-option="(value) => ({ id: value, name: value, isCreating: true })" @search="searchGroups" @@ -178,7 +179,7 @@ export default { data() { return { - availableGroups: this.$store.getters.getSortedGroups.filter(group => group.id !== '__nc_internal_recent' && group.id !== 'disabled'), + availableGroups: [], possibleManagers: [], // TRANSLATORS This string describes a manager in the context of an organization managerInputLabel: t('settings', 'Manager'), @@ -235,6 +236,13 @@ export default { }, mounted() { + // admins also can assign the system groups + if (this.isAdmin || this.isDelegatedAdmin) { + this.availableGroups = this.$store.getters.getSortedGroups.filter(group => group.id !== '__nc_internal_recent' && group.id !== 'disabled') + } else { + this.availableGroups = [...this.$store.getters.getSubAdminGroups] + } + this.$refs.username?.focus?.() }, @@ -273,6 +281,11 @@ export default { }, async searchGroups(query, toggleLoading) { + if (!this.isAdmin && !this.isDelegatedAdmin) { + // managers cannot search for groups + return + } + if (this.promise) { this.promise.cancel() } diff --git a/apps/settings/src/store/users.js b/apps/settings/src/store/users.js index 6b7eb749089..3734b7008df 100644 --- a/apps/settings/src/store/users.js +++ b/apps/settings/src/store/users.js @@ -37,7 +37,7 @@ const defaults = { const state = { users: [], groups: [ - ...(usersSettings.userGroups ?? []), + ...(usersSettings.getSubAdminGroups ?? []), ...(usersSettings.systemGroups ?? []), ], orderBy: usersSettings.sortGroups ?? GroupSorting.UserCount, @@ -235,12 +235,10 @@ const mutations = { * @param {object} state the store state */ resetGroups(state) { - const systemGroups = state.groups.filter(group => [ - 'admin', - '__nc_internal_recent', - 'disabled', - ].includes(group.id)) - state.groups = [...systemGroups] + state.groups = [ + ...(usersSettings.getSubAdminGroups ?? []), + ...(usersSettings.systemGroups ?? []), + ] }, setShowConfig(state, { key, value }) { @@ -267,16 +265,16 @@ const mutations = { } const getters = { - getUserGroups() { - return usersSettings.userGroups ?? [] - }, - getUsers(state) { return state.users }, getGroups(state) { return state.groups }, + getSubAdminGroups() { + return usersSettings.subAdminGroups ?? [] + }, + getSortedGroups(state) { const groups = [...state.groups] if (state.orderBy === GroupSorting.UserCount) { diff --git a/cypress/e2e/settings/users-group-admin.cy.ts b/cypress/e2e/settings/users-group-admin.cy.ts new file mode 100644 index 00000000000..5b5dcfd33a8 --- /dev/null +++ b/cypress/e2e/settings/users-group-admin.cy.ts @@ -0,0 +1,186 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +/// <reference types="cypress-if" /> +import { User } from '@nextcloud/cypress' +import { getUserListRow, handlePasswordConfirmation } from './usersUtils' +// eslint-disable-next-line n/no-extraneous-import +import randomString from 'crypto-random-string' + +const admin = new User('admin', 'admin') +const john = new User('john', '123456') + +/** + * Make a user subadmin of a group. + * + * @param user - The user to make subadmin + * @param group - The group the user should be subadmin of + */ +function makeSubAdmin(user: User, group: string): void { + cy.request({ + url: `${Cypress.config('baseUrl')!.replace('/index.php', '')}/ocs/v2.php/cloud/users/${user.userId}/subadmins`, + method: 'POST', + auth: { + user: admin.userId, + password: admin.userId, + }, + headers: { + 'OCS-ApiRequest': 'true', + }, + body: { + groupid: group, + }, + }) +} + +describe('Settings: Create accounts as a group admin', function() { + + let subadmin: User + let group: string + + beforeEach(() => { + group = randomString(7) + cy.deleteUser(john) + cy.createRandomUser().then((user) => { + subadmin = user + cy.runOccCommand(`group:add '${group}'`) + cy.runOccCommand(`group:adduser '${group}' '${subadmin.userId}'`) + makeSubAdmin(subadmin, group) + }) + }) + + it('Can create a user with prefilled single group', () => { + cy.login(subadmin) + // open the User settings + cy.visit('/settings/users') + + // open the New user modal + cy.get('button#new-user-button').click() + + cy.get('form[data-test="form"]').within(() => { + // see that the correct group is preselected + cy.contains('[data-test="groups"] .vs__selected', group).should('be.visible') + // see that the username is "" + cy.get('input[data-test="username"]').should('exist').and('have.value', '') + // set the username to john + cy.get('input[data-test="username"]').type(john.userId) + // see that the username is john + cy.get('input[data-test="username"]').should('have.value', john.userId) + // see that the password is "" + cy.get('input[type="password"]').should('exist').and('have.value', '') + // set the password to 123456 + cy.get('input[type="password"]').type(john.password) + // see that the password is 123456 + cy.get('input[type="password"]').should('have.value', john.password) + }) + + cy.get('form[data-test="form"]').parents('[role="dialog"]').within(() => { + // submit the new user form + cy.get('button[type="submit"]').click({ force: true }) + }) + + // Make sure no confirmation modal is shown + handlePasswordConfirmation(admin.password) + + // see that the created user is in the list + getUserListRow(john.userId) + // see that the list of users contains the user john + .contains(john.userId).should('exist') + }) + + it('Can create a new user when member of multiple groups', () => { + const group2 = randomString(7) + cy.runOccCommand(`group:add '${group2}'`) + cy.runOccCommand(`group:adduser '${group2}' '${subadmin.userId}'`) + makeSubAdmin(subadmin, group2) + + cy.login(subadmin) + // open the User settings + cy.visit('/settings/users') + + // open the New user modal + cy.get('button#new-user-button').click() + + cy.get('form[data-test="form"]').within(() => { + // see that no group is pre-selected + cy.get('[data-test="groups"] .vs__selected').should('not.exist') + // see both groups are available + cy.findByRole('combobox', { name: /member of the following groups/i }) + .should('be.visible') + .click() + // can select both groups + cy.document().its('body') + .findByRole('listbox', { name: 'Options' }) + .should('be.visible') + .as('options') + .findAllByRole('option') + .should('have.length', 2) + .get('@options') + .findByRole('option', { name: group }) + .should('be.visible') + .get('@options') + .findByRole('option', { name: group2 }) + .should('be.visible') + .click() + // see group is selected + cy.contains('[data-test="groups"] .vs__selected', group2).should('be.visible') + + // see that the username is "" + cy.get('input[data-test="username"]').should('exist').and('have.value', '') + // set the username to john + cy.get('input[data-test="username"]').type(john.userId) + // see that the username is john + cy.get('input[data-test="username"]').should('have.value', john.userId) + // see that the password is "" + cy.get('input[type="password"]').should('exist').and('have.value', '') + // set the password to 123456 + cy.get('input[type="password"]').type(john.password) + // see that the password is 123456 + cy.get('input[type="password"]').should('have.value', john.password) + }) + + cy.get('form[data-test="form"]').parents('[role="dialog"]').within(() => { + // submit the new user form + cy.get('button[type="submit"]').click({ force: true }) + }) + + // Make sure no confirmation modal is shown + handlePasswordConfirmation(admin.password) + + // see that the created user is in the list + getUserListRow(john.userId) + // see that the list of users contains the user john + .contains(john.userId).should('exist') + }) + + it('Only sees groups they are subadmin of', () => { + const group2 = randomString(7) + cy.runOccCommand(`group:add '${group2}'`) + cy.runOccCommand(`group:adduser '${group2}' '${subadmin.userId}'`) + // not a subadmin! + + cy.login(subadmin) + // open the User settings + cy.visit('/settings/users') + + // open the New user modal + cy.get('button#new-user-button').click() + + cy.get('form[data-test="form"]').within(() => { + // see that the subadmin group is pre-selected + cy.contains('[data-test="groups"] .vs__selected', group).should('be.visible') + // see only the subadmin group is available + cy.findByRole('combobox', { name: /member of the following groups/i }) + .should('be.visible') + .click() + // can select both groups + cy.document().its('body') + .findByRole('listbox', { name: 'Options' }) + .should('be.visible') + .as('options') + .findAllByRole('option') + .should('have.length', 1) + }) + }) +}) |