From 441d9f16618abf2d51c888b476a7fada3a976558 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <opensource@fthiessen.de>
Date: Thu, 24 Apr 2025 13:10:37 +0200
Subject: [PATCH] fix(settings): group admins only can add users to their
 groups

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
---
 .../lib/Controller/UsersController.php        |  25 ++-
 .../src/components/AppNavigationGroupList.vue |  18 +-
 apps/settings/src/components/UserList.vue     |   6 +-
 .../src/components/Users/NewUserDialog.vue    |  17 +-
 apps/settings/src/store/users.js              |  20 +-
 cypress/e2e/settings/users-group-admin.cy.ts  | 186 ++++++++++++++++++
 6 files changed, 242 insertions(+), 30 deletions(-)
 create mode 100644 cypress/e2e/settings/users-group-admin.cy.ts

diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php
index 3072f2072f3..a720425ab3c 100644
--- a/apps/settings/lib/Controller/UsersController.php
+++ b/apps/settings/lib/Controller/UsersController.php
@@ -40,10 +40,12 @@ 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;
 use OCP\IL10N;
+use OCP\INavigationManager;
 use OCP\IRequest;
 use OCP\IUser;
 use OCP\IUserSession;
@@ -87,8 +89,8 @@ class UsersController extends Controller {
 	 */
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
-	public function usersListByGroup(): TemplateResponse {
-		return $this->usersList();
+	public function usersListByGroup(INavigationManager $navigationManager, ISubAdmin $subAdmin): TemplateResponse {
+		return $this->usersList($navigationManager, $subAdmin);
 	}
 
 	/**
@@ -98,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);
 
-		\OC::$server->getNavigationManager()->setActiveEntry('core_users');
+		$navigationManager->setActiveEntry('core_users');
 
 		/* SORT OPTION: SORT_USERCOUNT or SORT_GROUPNAME */
 		$sortGroupsBy = MetaData::SORT_USERCOUNT;
@@ -180,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';
@@ -203,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 b27d2ef8832..86effa96a27 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 efb584979dc..f861d8b80cf 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)
+		})
+	})
+})