Merge pull request #52417 from nextcloud/fix/group-admin-new-user

fix(settings): only provide groups the subadmin has access to

Author: susnux

apps/settings/lib/Controller/UsersController.php

@@ -40,7 +40,9 @@
 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;
@@ -49,7 +51,6 @@
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
 use OCP\Mail\IMailer;
-use OCP\Server;
 use OCP\Util;
 use function in_array;
 
@@ -88,8 +89,8 @@ public function __construct(
 	 */
 	#[NoAdminRequired]
 	#[NoCSRFRequired]
-	public function usersListByGroup(): TemplateResponse {
-		return $this->usersList();
+	public function usersListByGroup(INavigationManager $navigationManager, ISubAdmin $subAdmin): TemplateResponse {
+		return $this->usersList($navigationManager, $subAdmin);
 	}
 
 	/**
@@ -99,13 +100,13 @@ public function usersListByGroup(): TemplateResponse {
 	 */
 	#[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;
@@ -181,6 +182,14 @@ public function usersList(): TemplateResponse {
 			'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';
@@ -204,6 +213,7 @@ public function usersList(): TemplateResponse {
 		$serverData = [];
 		// groups
 		$serverData['systemGroups'] = [$adminGroupData, $recentUsersGroup, $disabledUsersGroup];
+		$serverData['subAdminGroups'] = $subAdminGroups ?? [];
 		// Various data
 		$serverData['isAdmin'] = $isAdmin;
 		$serverData['isDelegatedAdmin'] = $isDelegatedAdmin;

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
 }
 

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) {

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()
 			}

apps/settings/src/store/users.js

@@ -36,7 +36,10 @@ const defaults = {
 
 const state = {
 	users: [],
-	groups: [...(usersSettings.systemGroups ?? [])],
+	groups: [
+		...(usersSettings.getSubAdminGroups ?? []),
+		...(usersSettings.systemGroups ?? []),
+	],
 	orderBy: usersSettings.sortGroups ?? GroupSorting.UserCount,
 	minPasswordLength: 0,
 	usersOffset: 0,
@@ -232,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 }) {
@@ -270,6 +271,10 @@ const getters = {
 	getGroups(state) {
 		return state.groups
 	},
+	getSubAdminGroups() {
+		return usersSettings.subAdminGroups ?? []
+	},
+
 	getSortedGroups(state) {
 		const groups = [...state.groups]
 		if (state.orderBy === GroupSorting.UserCount) {