Merge pull request #9919 from nextcloud/immutable-groups

Don't pretend we can add/remove users to/from groups when we can't

apps/provisioning_api/lib/Controller/GroupsController.php

@@ -115,7 +115,9 @@ class GroupsController extends AUserData {
 				'id' => $group->getGID(),
 				'displayname' => $group->getDisplayName(),
 				'usercount' => $group->count(),
-				'disabled' => $group->countDisabled()
+				'disabled' => $group->countDisabled(),
+				'canAdd' => $group->canAddUser(),
+				'canRemove' => $group->canRemoveUser(),
 			];
 		}, $groups);
 

apps/provisioning_api/tests/Controller/GroupsControllerTest.php

@@ -107,6 +107,12 @@ class GroupsControllerTest extends \Test\TestCase {
 		$group
 			->method('countDisabled')
 			->willReturn(11);
+		$group
+			->method('canAddUser')
+			->willReturn(true);
+		$group
+			->method('canRemoveUser')
+			->willReturn(true);
 
 		return $group;
 	}
@@ -215,13 +221,18 @@ class GroupsControllerTest extends \Test\TestCase {
 				'id' => 'group1',
 				'displayname' => 'group1-name',
 				'usercount' => 123,
-				'disabled' => 11
+				'disabled' => 11,
+				'canAdd' => true,
+				'canRemove' => true
 			), 
 			Array(
 				'id' => 'group2',
 				'displayname' => 'group2-name',
 				'usercount' => 123,
-				'disabled' => 11
+				'disabled' => 11,
+				'canAdd' => true,
+				'canRemove' => true
+				
 				)
 		]], $result->getData());
 

core/css/inputs.scss

@@ -782,7 +782,7 @@ input {
                 color: nc-lighten($color-main-text, 33%);
                 width: 100%;
                 /* selected checkmark icon */
-                &:not(.multiselect__option--disabled)::before {
+                &::before {
                     content: ' ';
                     background-image: url('../img/actions/checkmark.svg?v=1');
                     background-repeat: no-repeat;
@@ -790,12 +790,13 @@ input {
                     min-width: 16px;
                     min-height: 16px;
                     display: block;
-                    opacity: 0.5;
+                    opacity: .5;
                     margin-right: 5px;
                     visibility: hidden;
                 }
                 &.multiselect__option--disabled {
-                    background-color: nc-darken($color-main-background, 8%);
+					background-color: nc-darken($color-main-background, 8%);
+					opacity: .5;
                 }
                 /* add the prop tag-placeholder="create" to add the +
 				 * icon on top of an unknown-and-ready-to-be-created entry
@@ -809,11 +810,15 @@ input {
                 &.multiselect__option--highlight {
                     color: $color-main-text;
                 }
-                &.multiselect__option--selected {
-                    &::before {
-                        visibility: visible;
-                    }
-                }
+				&:not(.multiselect__option--disabled):hover::before {
+					opacity: .3;
+				}
+				&.multiselect__option--selected,
+				&:not(.multiselect__option--disabled):hover {
+					&::before {
+						visibility: visible;
+					}
+				}
             }
         }
     }

lib/private/Group/Group.php

@@ -30,6 +30,7 @@
 
 namespace OC\Group;
 
+use OCP\GroupInterface;
 use OCP\IGroup;
 use OCP\IUser;
 use OCP\Group\Backend\ICountDisabledInGroup;
@@ -323,4 +324,30 @@ class Group implements IGroup {
 		}
 		return $users;
 	}
+
+	/**
+	 * @return bool
+	 * @since 14.0.0
+	 */
+	public function canRemoveUser() {
+		foreach ($this->backends as $backend) {
+			if ($backend->implementsActions(GroupInterface::REMOVE_FROM_GOUP)) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	/**
+	 * @return bool
+	 * @since 14.0.0
+	 */
+	public function canAddUser() {
+		foreach ($this->backends as $backend) {
+			if ($backend->implementsActions(GroupInterface::ADD_TO_GROUP)) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

lib/private/Group/MetaData.php

@@ -165,7 +165,9 @@ class MetaData {
 				'id' => $group->getGID(),
 				'name' => $group->getDisplayName(),
 				'usercount' => $this->sorting === self::SORT_USERCOUNT ? $group->count($userSearch) : 0,
-				'disabled' => $group->countDisabled()
+				'disabled' => $group->countDisabled(),
+				'canAdd' => $group->canAddUser(),
+				'canRemove' => $group->canRemoveUser(),
 			);
 	}
 

lib/public/IGroup.php

@@ -126,4 +126,16 @@ interface IGroup {
 	 * @since 8.0.0
 	 */
 	public function delete();
+
+	/**
+	 * @return bool
+	 * @since 14.0.0
+	 */
+	public function canRemoveUser();
+
+	/**
+	 * @return bool
+	 * @since 14.0.0
+	 */
+	public function canAddUser();
 }

settings/src/components/userList.vue

@@ -75,7 +75,7 @@
 				<!-- hidden input trick for vanilla html5 form validation -->
 				<input type="text" :value="newUser.groups" v-if="!settings.isAdmin"
 					   tabindex="-1" id="newgroups" :required="!settings.isAdmin" />
-				<multiselect :options="groups" v-model="newUser.groups"
+				<multiselect :options="canAddGroups" v-model="newUser.groups"
 							 :placeholder="t('settings', 'Add user in group')"
 							 label="name" track-by="id" class="multiselect-vue"
 							 :multiple="true" :close-on-select="false"
@@ -202,7 +202,7 @@ export default {
 				return disabledUsers;
 			}
 			if (!this.settings.isAdmin) {
-				// We don't want subadmins to edit themselves
+				// we don't want subadmins to edit themselves
 				return this.users.filter(user => user.enabled !== false && user.id !== oc_current_user);
 			}
 			return this.users.filter(user => user.enabled !== false);
@@ -213,6 +213,16 @@ export default {
 				.filter(group => group.id !== 'disabled')
 				.sort((a, b) => a.name.localeCompare(b.name));
 		},
+		canAddGroups() {
+			// disabled if no permission to add new users to group
+			return this.groups.map((group) => {
+				// clone object because we don't want
+				// to edit the original groups
+				group = Object.assign({}, group);
+				group.$isDisabled = group.canAdd !== true;
+				return group;
+			});
+		},
 		subAdminsGroups() {
 			// data provided php side
 			return this.$store.getters.getSubadminGroups;

settings/src/components/userList/userRow.vue

@@ -64,7 +64,7 @@
 			<input type="submit" class="icon-confirm" value="" />
 		</form>
 		<div class="groups" :class="{'icon-loading-small': loading.groups}">
-			<multiselect :value="userGroups" :options="groups" :disabled="loading.groups||loading.all"
+			<multiselect :value="userGroups" :options="availableGroups" :disabled="loading.groups||loading.all"
 						 tag-placeholder="create" :placeholder="t('settings', 'Add user in group')"
 						 label="name" track-by="id" class="multiselect-vue" :limit="2"
 						 :multiple="true" :taggable="settings.isAdmin" :closeOnSelect="false"
@@ -182,6 +182,23 @@ export default {
 			let userSubAdminsGroups = this.subAdminsGroups.filter(group => this.user.subadmin.includes(group.id));
 			return userSubAdminsGroups;
 		},
+		availableGroups() {
+			return this.groups.map((group) => {
+				// clone object because we don't want
+				// to edit the original groups
+				let groupClone = Object.assign({}, group);
+
+				// two settings here:
+				// 1. user NOT in group but no permission to add
+				// 2. user is in group but no permission to remove
+				groupClone.$isDisabled =
+					(group.canAdd !== true &&
+						!this.user.groups.includes(group.id)) ||
+					(group.canRemove !== true &&
+						this.user.groups.includes(group.id));
+				return groupClone;
+			});
+		},
 
 		/* QUOTA MANAGEMENT */
 		usedQuota() {
@@ -374,6 +391,9 @@ export default {
 		 * @returns {Promise}
 		 */
 		addUserGroup(group) {
+			if (!group.canAdd) {
+				return false;
+			}
 			this.loading.groups = true;
 			let userid = this.user.id;
 			let gid = group.id;
@@ -388,6 +408,9 @@ export default {
 		 * @returns {Promise}
 		 */
 		removeUserGroup(group) {
+			if (!group.canRemove) {
+				return false;
+			}
 			this.loading.groups = true;
 			let userid = this.user.id;
 			let gid = group.id;