feat(files_sharing): allow to specify allowed groups to share instead of excluded groups

Relates to #3387

Signed-off-by: Corentin Damman <c.damman@intopix.com>

apps/settings/lib/Settings/Admin/Sharing.php

@@ -88,7 +88,7 @@ class Sharing implements IDelegatedSettings {
 			'defaultExpireDate' => $this->getHumanBooleanConfig('core', 'shareapi_default_expire_date'),
 			'expireAfterNDays' => $this->config->getAppValue('core', 'shareapi_expire_after_n_days', '7'),
 			'enforceExpireDate' => $this->getHumanBooleanConfig('core', 'shareapi_enforce_expire_date'),
-			'excludeGroups' => $this->getHumanBooleanConfig('core', 'shareapi_exclude_groups'),
+			'excludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no'),
 			'excludeGroupsList' => json_decode($excludedGroups, true) ?? [],
 			'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext', null),
 			'enableLinkPasswordByDefault' => $this->getHumanBooleanConfig('core', 'shareapi_enable_link_password_by_default'),

apps/settings/src/components/AdminSettingsSharingForm.vue

@@ -76,19 +76,31 @@
 				</label>
 			</fieldset>
 
-			<NcCheckboxRadioSwitch type="switch" :checked.sync="settings.excludeGroups">
+			<label>{{ t('settings', 'Limit sharing based on groups') }}</label>
-				{{ t('settings', 'Exclude groups from sharing') }}
+			<div class="sharing__sub-section">
-			</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch :checked.sync="settings.excludeGroups"
-			<div v-show="settings.excludeGroups" class="sharing__sub-section">
+															 name="excludeGroups" value="no"
-				<div class="sharing__labeled-entry sharing__input">
+															 type="radio" @update:checked="onUpdateExcludeGroups">
-					<label for="settings-sharing-excluded-groups">{{ t('settings', 'Groups excluded from sharing') }}</label>
+					{{ t('settings', 'Allow sharing for everyone (default)') }}
+				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch :checked.sync="settings.excludeGroups"
+															 name="excludeGroups" value="yes"
+															 type="radio" @update:checked="onUpdateExcludeGroups">
+					{{ t('settings', 'Exclude some groups from sharing') }}
+				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch :checked.sync="settings.excludeGroups"
+															 name="excludeGroups" value="allow"
+															 type="radio" @update:checked="onUpdateExcludeGroups">
+					{{ t('settings', 'Limit sharing to some groups') }}
+				</NcCheckboxRadioSwitch>
+				<div v-show="settings.excludeGroups !== 'no'" class="sharing__labeled-entry sharing__input">
 					<NcSettingsSelectGroup id="settings-sharing-excluded-groups"
 						v-model="settings.excludeGroupsList"
 						aria-describedby="settings-sharing-excluded-groups-desc"
-						:label="t('settings', 'Groups excluded from sharing')"
+						:label="settings.excludeGroups === 'allow' ? t('settings', 'Groups allowed to share') : t('settings', 'Groups excluded from sharing')"
-						:disabled="!settings.excludeGroups"
+						:disabled="settings.excludeGroups === 'no'"
 						style="width: 100%" />
-					<em id="settings-sharing-excluded-groups-desc">{{ t('settings', 'These groups will still be able to receive shares, but not to initiate them.') }}</em>
+					<em id="settings-sharing-excluded-groups-desc">{{ t('settings', 'Not allowed groups will still be able to receive shares, but not to initiate them.') }}</em>
 				</div>
 			</div>
 
@@ -227,7 +239,7 @@ interface IShareSettings {
 	defaultExpireDate: boolean
 	expireAfterNDays: string
 	enforceExpireDate: boolean
-	excludeGroups: boolean
+	excludeGroups: string
 	excludeGroupsList: string[]
 	publicShareDisclaimerText?: string
 	enableLinkPasswordByDefault: boolean
@@ -306,6 +318,10 @@ export default defineComponent({
 			}
 			this.settingsData.publicShareDisclaimerText = value
 		}, 500) as (v?: string) => void,
+		onUpdateExcludeGroups: debounce(function(value: string) {
+			window.OCP.AppConfig.setValue('core', 'excludeGroups', value)
+			this.settings.excludeGroups = value
+		}, 500) as (v?: string) => void
 	},
 })
 </script>

apps/settings/tests/Settings/Admin/SharingTest.php

@@ -150,7 +150,7 @@ class SharingTest extends TestCase {
 						'defaultExpireDate' => false,
 						'expireAfterNDays' => '7',
 						'enforceExpireDate' => false,
-						'excludeGroups' => false,
+						'excludeGroups' => 'no',
 						'excludeGroupsList' => [],
 						'publicShareDisclaimerText' => 'Lorem ipsum',
 						'enableLinkPasswordByDefault' => true,
@@ -243,7 +243,7 @@ class SharingTest extends TestCase {
 						'defaultExpireDate' => false,
 						'expireAfterNDays' => '7',
 						'enforceExpireDate' => false,
-						'excludeGroups' => true,
+						'excludeGroups' => 'yes',
 						'excludeGroupsList' => ['NoSharers','OtherNoSharers'],
 						'publicShareDisclaimerText' => 'Lorem ipsum',
 						'enableLinkPasswordByDefault' => true,

lib/private/Contacts/ContactsMenu/ContactsStore.php

@@ -193,7 +193,7 @@ class ContactsStore implements IContactsStore {
 		$restrictEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
 		$restrictEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
 		$allowEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
-		$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes';
+		$excludeGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no');
 
 		// whether to filter out local users
 		$skipLocal = false;
@@ -202,14 +202,22 @@ class ContactsStore implements IContactsStore {
 
 		$selfGroups = $this->groupManager->getUserGroupIds($self);
 
-		if ($excludedGroups) {
+		if ($excludeGroups && $excludeGroups !== 'no') {
 			$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
 			$decodedExcludeGroups = json_decode($excludedGroups, true);
 			$excludeGroupsList = $decodedExcludeGroups ?? [];
 
-			if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) {
+			if ($excludeGroups != 'allow') {
-				// a group of the current user is excluded -> filter all local users
+				if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) {
+					// a group of the current user is excluded -> filter all local users
+					$skipLocal = true;
+				}
+			} else {
 				$skipLocal = true;
+				if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) {
+					// a group of the current user is allowed -> do not filter all local users
+					$skipLocal = false;
+				}
 			}
 		}
 

lib/private/Share20/ShareDisableChecker.php

@@ -35,7 +35,9 @@ class ShareDisableChecker {
 			return $this->sharingDisabledForUsersCache[$userId];
 		}
 
-		if ($this->config->getAppValue('core', 'shareapi_exclude_groups', 'no') === 'yes') {
+		$excludeGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no');
+
+		if ($excludeGroups && $excludeGroups !== 'no') {
 			$groupsList = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
 			$excludedGroups = json_decode($groupsList);
 			if (is_null($excludedGroups)) {
@@ -48,14 +50,28 @@ class ShareDisableChecker {
 				return false;
 			}
 			$usersGroups = $this->groupManager->getUserGroupIds($user);
-			if (!empty($usersGroups)) {
+			if ($excludeGroups !== 'allow') {
-				$remainingGroups = array_diff($usersGroups, $excludedGroups);
+				if (!empty($usersGroups)) {
-				// if the user is only in groups which are disabled for sharing then
+					$remainingGroups = array_diff($usersGroups, $excludedGroups);
-				// sharing is also disabled for the user
+					// if the user is only in groups which are disabled for sharing then
-				if (empty($remainingGroups)) {
+					// sharing is also disabled for the user
-					$this->sharingDisabledForUsersCache[$userId] = true;
+					if (empty($remainingGroups)) {
-					return true;
+						$this->sharingDisabledForUsersCache[$userId] = true;
+						return true;
+					}
+				}
+			} else {
+				if (!empty($usersGroups)) {
+					$remainingGroups = array_intersect($usersGroups, $excludedGroups);
+					// if the user is in any group which is allowed for sharing then
+					// sharing is also allowed for the user
+					if (!empty($remainingGroups)) {
+						$this->sharingDisabledForUsersCache[$userId] = false;
+						return false;
+					}
 				}
+				$this->sharingDisabledForUsersCache[$userId] = true;
+				return true;
 			}
 		}
 

tests/lib/Share20/ManagerTest.php

@@ -2099,26 +2099,33 @@ class ManagerTest extends \Test\TestCase {
 		// No exclude groups
 		$data[] = ['no', null, null, [], false];
 
-		// empty exclude list, user no groups
+		// empty exclude / allow list, user no groups
 		$data[] = ['yes', '', json_encode(['']), [], false];
+		$data[] = ['allow', '', json_encode(['']), [], true];
 
-		// empty exclude list, user groups
+		// empty exclude / allow list, user groups
 		$data[] = ['yes', '', json_encode(['']), ['group1', 'group2'], false];
+		$data[] = ['allow', '', json_encode(['']), ['group1', 'group2'], true];
 
 		// Convert old list to json
 		$data[] = ['yes', 'group1,group2', json_encode(['group1', 'group2']), [], false];
+		$data[] = ['allow', 'group1,group2', json_encode(['group1', 'group2']), [], true];
 
 		// Old list partly groups in common
 		$data[] = ['yes', 'group1,group2', json_encode(['group1', 'group2']), ['group1', 'group3'], false];
+		$data[] = ['allow', 'group1,group2', json_encode(['group1', 'group2']), ['group1', 'group3'], false];
 
 		// Old list only groups in common
 		$data[] = ['yes', 'group1,group2', json_encode(['group1', 'group2']), ['group1'], true];
+		$data[] = ['allow', 'group1,group2', json_encode(['group1', 'group2']), ['group1'], false];
 
 		// New list partly in common
 		$data[] = ['yes', json_encode(['group1', 'group2']), null, ['group1', 'group3'], false];
+		$data[] = ['allow', json_encode(['group1', 'group2']), null, ['group1', 'group3'], false];
 
 		// New list only groups in common
 		$data[] = ['yes', json_encode(['group1', 'group2']), null, ['group2'], true];
+		$data[] = ['allow', json_encode(['group1', 'group2']), null, ['group2'], false];
 
 		return $data;
 	}