Skip to content

Commit

Permalink
feat(files_sharing): allow to specify allowed groups to share instead…
Browse files Browse the repository at this point in the history 
… of excluded groups

Relates to #3387

Signed-off-by: Corentin Damman <c.damman@intopix.com>
  • Loading branch information
@cdammanintopix
cdammanintopix committed Mar 14, 2024
1 parent 10593d1 commit d5527e9
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 30 deletions.
2 changes: 1 addition & 1 deletion apps/settings/lib/Settings/Admin/Sharing.php
View file
Expand Up @@ -88,7 +88,7 @@ public function getForm() {
'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'),

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::getAppValue has been marked as deprecated
'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'),
Expand Down
36 changes: 26 additions & 10 deletions apps/settings/src/components/AdminSettingsSharingForm.vue
View file
Expand Up @@ -76,19 +76,31 @@
</label>
</fieldset>

<NcCheckboxRadioSwitch type="switch" :checked.sync="settings.excludeGroups">
{{ t('settings', 'Exclude groups from sharing') }}
</NcCheckboxRadioSwitch>
<div v-show="settings.excludeGroups" class="sharing__sub-section">
<div class="sharing__labeled-entry sharing__input">
<label for="settings-sharing-excluded-groups">{{ t('settings', 'Groups excluded from sharing') }}</label>
<label>{{ t('settings', 'Limit sharing based on groups') }}</label>
<div class="sharing__sub-section">
<NcCheckboxRadioSwitch :checked.sync="settings.excludeGroups"
name="excludeGroups" value="no"
type="radio" @update:checked="onUpdateExcludeGroups">
{{ 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')"
:disabled="!settings.excludeGroups"
:label="settings.excludeGroups === 'allow' ? t('settings', 'Groups allowed to share') : t('settings', 'Groups excluded from sharing')"
: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>

Expand Down Expand Up @@ -227,7 +239,7 @@ interface IShareSettings {
defaultExpireDate: boolean
expireAfterNDays: string
enforceExpireDate: boolean
excludeGroups: boolean
excludeGroups: string
excludeGroupsList: string[]
publicShareDisclaimerText?: string
enableLinkPasswordByDefault: boolean
Expand Down Expand Up @@ -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>
Expand Down
4 changes: 2 additions & 2 deletions apps/settings/tests/Settings/Admin/SharingTest.php
View file
Expand Up @@ -150,7 +150,7 @@ public function testGetFormWithoutExcludedGroups(): void {
'defaultExpireDate' => false,
'expireAfterNDays' => '7',
'enforceExpireDate' => false,
'excludeGroups' => false,
'excludeGroups' => 'no',
'excludeGroupsList' => [],
'publicShareDisclaimerText' => 'Lorem ipsum',
'enableLinkPasswordByDefault' => true,
Expand Down Expand Up @@ -243,7 +243,7 @@ public function testGetFormWithExcludedGroups(): void {
'defaultExpireDate' => false,
'expireAfterNDays' => '7',
'enforceExpireDate' => false,
'excludeGroups' => true,
'excludeGroups' => 'yes',
'excludeGroupsList' => ['NoSharers','OtherNoSharers'],
'publicShareDisclaimerText' => 'Lorem ipsum',
'enableLinkPasswordByDefault' => true,
Expand Down
4 changes: 2 additions & 2 deletions dist/settings-vue-settings-admin-sharing.js
View file

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-vue-settings-admin-sharing.js.map
View file

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions lib/private/Contacts/ContactsMenu/ContactsStore.php
View file
Expand Up @@ -193,7 +193,7 @@ private function filterContacts(
$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;
Expand All @@ -202,14 +202,22 @@ private function filterContacts(

$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) {
// a group of the current user is excluded -> filter all local users
if ($excludeGroups != 'allow') {
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;
}
}
}

Expand Down
32 changes: 24 additions & 8 deletions lib/private/Share20/ShareDisableChecker.php
View file
Expand Up @@ -35,7 +35,9 @@ public function sharingDisabledForUser(?string $userId) {
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)) {
Expand All @@ -48,14 +50,28 @@ public function sharingDisabledForUser(?string $userId) {
return false;
}
$usersGroups = $this->groupManager->getUserGroupIds($user);
if (!empty($usersGroups)) {
$remainingGroups = array_diff($usersGroups, $excludedGroups);
// if the user is only in groups which are disabled for sharing then
// sharing is also disabled for the user
if (empty($remainingGroups)) {
$this->sharingDisabledForUsersCache[$userId] = true;
return true;
if ($excludeGroups !== 'allow') {
if (!empty($usersGroups)) {
$remainingGroups = array_diff($usersGroups, $excludedGroups);
// if the user is only in groups which are disabled for sharing then
// sharing is also disabled for the user
if (empty($remainingGroups)) {
$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;
}
}

Expand Down
11 changes: 9 additions & 2 deletions tests/lib/Share20/ManagerTest.php
View file
Expand Up @@ -2099,26 +2099,33 @@ public function dataIsSharingDisabledForUser() {
// 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;
}
Expand Down

0 comments on commit d5527e9

Please sign in to comment.