aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFerdinand Thiessen <opensource@fthiessen.de>2025-04-24 13:10:37 +0200
committerFerdinand Thiessen <opensource@fthiessen.de>2025-04-27 11:35:40 +0200
commit6d8c49f0d4bc6c705fe42274ebe045b837c6ca87 (patch)
treeb88f6f2afad2ad230c0f880014e312f3e614ec43
parent8cc0c266d5ce2dd1966c8285d7b347ec1b3a0e98 (diff)
downloadnextcloud-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.php21
-rw-r--r--apps/settings/src/components/AppNavigationGroupList.vue18
-rw-r--r--apps/settings/src/components/UserList.vue6
-rw-r--r--apps/settings/src/components/Users/NewUserDialog.vue17
-rw-r--r--apps/settings/src/store/users.js20
-rw-r--r--cypress/e2e/settings/users-group-admin.cy.ts186
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)
+ })
+ })
+})