Skip to content

Commit

Permalink
Merge pull request #38173 from arawa/feature/37677/exclude-some-group…
Browse files Browse the repository at this point in the history
…s-from-sharing-with-users

New core setting : shareapi_only_share_with_group_members_exclude_gro…
  • Loading branch information
artonge authored Jan 31, 2024
2 parents 6594360 + 2f64411 commit 7dc5a91
Show file tree
Hide file tree
Showing 13 changed files with 96 additions and 4 deletions.
2 changes: 2 additions & 0 deletions apps/settings/lib/Settings/Admin/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public function getForm() {
$excludedGroups = $this->config->getAppValue('core', 'shareapi_exclude_groups_list', '');
$linksExcludedGroups = $this->config->getAppValue('core', 'shareapi_allow_links_exclude_groups', '');
$excludedPasswordGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
$onlyShareWithGroupMembersExcludeGroupList = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');

$parameters = [
// Built-In Sharing
Expand All @@ -83,6 +84,7 @@ public function getForm() {
'passwordExcludedGroups' => json_decode($excludedPasswordGroups) ?? [],
'passwordExcludedGroupsFeatureEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),
'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(),
'onlyShareWithGroupMembersExcludeGroupList' => json_decode($onlyShareWithGroupMembersExcludeGroupList) ?? [],
'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'),
Expand Down
8 changes: 8 additions & 0 deletions apps/settings/src/components/AdminSettingsSharingForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@
<NcCheckboxRadioSwitch :checked.sync="settings.onlyShareWithGroupMembers">
{{ t('settings', 'Restrict users to only share with users in their groups') }}
</NcCheckboxRadioSwitch>
<div v-show="settings.onlyShareWithGroupMembers" id="settings-sharing-api-excluded-groups" class="sharing__labeled-entry sharing__input">
<label for="settings-sharing-only-group-members-excluded-groups">{{ t('settings', 'Ignore the following groups when checking group membership') }}</label>
<NcSettingsSelectGroup id="settings-sharing-only-group-members-excluded-groups"
v-model="settings.onlyShareWithGroupMembersExcludeGroupList"
:label="t('settings', 'Ignore the following groups when checking group membership')"
style="width: 100%" />
</div>
</div>

<div v-show="settings.enabled" id="settings-sharing-api" class="sharing__section">
Expand Down Expand Up @@ -216,6 +223,7 @@ interface IShareSettings {
passwordExcludedGroups: string[]
passwordExcludedGroupsFeatureEnabled: boolean
onlyShareWithGroupMembers: boolean
onlyShareWithGroupMembersExcludeGroupList: string[]
defaultExpireDate: boolean
expireAfterNDays: string
enforceExpireDate: boolean
Expand Down
4 changes: 4 additions & 0 deletions apps/settings/tests/Settings/Admin/SharingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ public function testGetFormWithoutExcludedGroups(): void {
['core', 'shareapi_remote_expire_after_n_days', '7', '7'],
['core', 'shareapi_enforce_remote_expire_date', 'no', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);
$this->shareManager->method('shareWithGroupMembersOnly')
->willReturn(false);
Expand Down Expand Up @@ -163,6 +164,7 @@ public function testGetFormWithoutExcludedGroups(): void {
'allowLinksExcludeGroups' => [],
'passwordExcludedGroups' => [],
'passwordExcludedGroupsFeatureEnabled' => false,
'onlyShareWithGroupMembersExcludeGroupList' => [],
]
],
);
Expand Down Expand Up @@ -209,6 +211,7 @@ public function testGetFormWithExcludedGroups(): void {
['core', 'shareapi_remote_expire_after_n_days', '7', '7'],
['core', 'shareapi_enforce_remote_expire_date', 'no', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);
$this->shareManager->method('shareWithGroupMembersOnly')
->willReturn(false);
Expand Down Expand Up @@ -254,6 +257,7 @@ public function testGetFormWithExcludedGroups(): void {
'allowLinksExcludeGroups' => [],
'passwordExcludedGroups' => [],
'passwordExcludedGroupsFeatureEnabled' => false,
'onlyShareWithGroupMembersExcludeGroupList' => [],
]
],
);
Expand Down
4 changes: 2 additions & 2 deletions dist/settings-vue-settings-admin-sharing.js

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions lib/private/Collaboration/Collaborators/GroupPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,16 @@ public function __construct(
private IConfig $config,
private IGroupManager $groupManager,
private IUserSession $userSession,
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
) {
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$this->groupSharingDisabled = $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes') === 'no';

if ($this->shareWithGroupOnly) {
$this->shareWithGroupOnlyExcludeGroupsList = json_decode($this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', ''), true) ?? [];
}
}

public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
Expand Down Expand Up @@ -81,6 +86,9 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
return $group->getGID();
}, $userGroups);
$groupIds = array_intersect($groupIds, $userGroups);

// ShareWithGroupOnly filtering
$groupIds = array_diff($groupIds, $this->shareWithGroupOnlyExcludeGroupsList);
}

$lowerSearch = strtolower($search);
Expand Down
9 changes: 9 additions & 0 deletions lib/private/Collaboration/Collaborators/MailPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,18 @@ public function __construct(
private KnownUserService $knownUserService,
private IUserSession $userSession,
private IMailer $mailer,
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
) {
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumerationInGroupOnly = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$this->shareeEnumerationPhone = $this->shareeEnumeration && $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
$this->shareeEnumerationFullMatch = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes') === 'yes';
$this->shareeEnumerationFullMatchEmail = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';

if ($this->shareWithGroupOnly) {
$this->shareWithGroupOnlyExcludeGroupsList = json_decode($this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', ''), true) ?? [];
}
}

/**
Expand Down Expand Up @@ -127,6 +132,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
* Check if the user may share with the user associated with the e-mail of the just found contact
*/
$userGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());

// ShareWithGroupOnly filtering
$userGroups = array_diff($userGroups, $this->shareWithGroupOnlyExcludeGroupsList);

$found = false;
foreach ($userGroups as $userGroup) {
if ($this->groupManager->isInGroup($contact['UID'], $userGroup)) {
Expand Down
9 changes: 9 additions & 0 deletions lib/private/Collaboration/Collaborators/UserPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public function __construct(
private IUserSession $userSession,
private KnownUserService $knownUserService,
private IUserStatusManager $userStatusManager,
private mixed $shareWithGroupOnlyExcludeGroupsList = [],
) {
$this->shareWithGroupOnly = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
$this->shareeEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
Expand All @@ -76,6 +77,10 @@ public function __construct(
$this->shareeEnumerationFullMatchUserId = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes';
$this->shareeEnumerationFullMatchEmail = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';
$this->shareeEnumerationFullMatchIgnoreSecondDisplayName = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn', 'no') === 'yes';

if ($this->shareWithGroupOnly) {
$this->shareWithGroupOnlyExcludeGroupsList = json_decode($this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', ''), true) ?? [];
}
}

public function search($search, $limit, $offset, ISearchResult $searchResult): bool {
Expand All @@ -85,6 +90,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b

$currentUserId = $this->userSession->getUser()->getUID();
$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());

// ShareWithGroupOnly filtering
$currentUserGroups = array_diff($currentUserGroups, $this->shareWithGroupOnlyExcludeGroupsList);

if ($this->shareWithGroupOnly || $this->shareeEnumerationInGroupOnly) {
// Search in all the groups this user is part of
foreach ($currentUserGroups as $userGroupId) {
Expand Down
10 changes: 10 additions & 0 deletions lib/private/Contacts/ContactsMenu/ContactsStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ public function getContacts(IUser $user, ?string $filter, ?int $limit = null, ?i
* 3. if the `shareapi_only_share_with_group_members` config option is
* enabled it will filter all users which doesn't have a common group
* with the current user.
* If enabled, the 'shareapi_only_share_with_group_members_exclude_group_list'
* config option may specify some groups excluded from the principle of
* belonging to the same group.
*
* @param Entry[] $entries
* @return Entry[] the filtered contacts
Expand Down Expand Up @@ -210,6 +213,13 @@ private function filterContacts(
}
}

// ownGroupsOnly : some groups may be excluded
if ($ownGroupsOnly) {
$excludeGroupsFromOwnGroups = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');
$excludeGroupsFromOwnGroupsList = json_decode($excludeGroupsFromOwnGroups, true) ?? [];
$selfGroups = array_diff($selfGroups, $excludeGroupsFromOwnGroupsList);
}

$selfUID = $self->getUID();

return array_values(array_filter($entries, function (IEntry $entry) use ($skipLocal, $ownGroupsOnly, $selfGroups, $selfUID, $disallowEnumeration, $restrictEnumerationGroup, $restrictEnumerationPhone, $allowEnumerationFullMatch, $filter) {
Expand Down
25 changes: 24 additions & 1 deletion lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,11 @@ protected function userCreateChecks(IShare $share) {
$this->groupManager->getUserGroupIds($sharedBy),
$this->groupManager->getUserGroupIds($sharedWith)
);

// optional excluded groups
$excludedGroups = $this->shareWithGroupMembersOnlyExcludeGroupsList();
$groups = array_diff($groups, $excludedGroups);

if (empty($groups)) {
$message_t = $this->l->t('Sharing is only allowed with group members');
throw new \Exception($message_t);
Expand Down Expand Up @@ -608,7 +613,10 @@ protected function groupCreateChecks(IShare $share) {
if ($this->shareWithGroupMembersOnly()) {
$sharedBy = $this->userManager->get($share->getSharedBy());
$sharedWith = $this->groupManager->get($share->getSharedWith());
if (is_null($sharedWith) || !$sharedWith->inGroup($sharedBy)) {

// optional excluded groups
$excludedGroups = $this->shareWithGroupMembersOnlyExcludeGroupsList();
if (is_null($sharedWith) || in_array($share->getSharedWith(), $excludedGroups) || !$sharedWith->inGroup($sharedBy)) {
throw new \Exception('Sharing is only allowed within your own groups');
}
}
Expand Down Expand Up @@ -1938,6 +1946,21 @@ public function shareWithGroupMembersOnly() {
return $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
}

/**
* If shareWithGroupMembersOnly is enabled, return an optional
* list of groups that must be excluded from the principle of
* belonging to the same group.
*
* @return array
*/
public function shareWithGroupMembersOnlyExcludeGroupsList() {
if (!$this->shareWithGroupMembersOnly()) {
return [];
}
$excludeGroups = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');
return json_decode($excludeGroups, true) ?? [];
}

/**
* Check if users can share with groups
*
Expand Down
9 changes: 9 additions & 0 deletions lib/public/Share/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,15 @@ public function shareApiLinkAllowPublicUpload();
*/
public function shareWithGroupMembersOnly();

/**
* If shareWithGroupMembersOnly is enabled, return an optional
* list of groups that must be excluded from the principle of
* belonging to the same group.
* @return array
* @since 27.0.0
*/
public function shareWithGroupMembersOnlyExcludeGroupsList();

/**
* Check if users can share with groups
* @return bool
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,7 @@ public function testGetContactsWhenUserIsInExcludeGroups() {
['core', 'shareapi_exclude_groups', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_exclude_groups_list', '', '["group1", "group5", "group6"]'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

/** @var IUser|MockObject $currentUser */
Expand Down Expand Up @@ -260,6 +261,7 @@ public function testGetContactsOnlyShareIfInTheSameGroup() {
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
['core', 'shareapi_exclude_groups', 'no', 'no'],
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

/** @var IUser|MockObject $currentUser */
Expand Down Expand Up @@ -334,6 +336,7 @@ public function testGetContactsOnlyEnumerateIfInTheSameGroup() {
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
['core', 'shareapi_exclude_groups', 'no', 'no'],
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

/** @var IUser|MockObject $currentUser */
Expand Down Expand Up @@ -466,6 +469,7 @@ public function testGetContactsOnlyEnumerateIfPhoneBookMatchWithOwnGroupsOnly()
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
['core', 'shareapi_exclude_groups', 'no', 'no'],
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

/** @var IUser|MockObject $currentUser */
Expand Down Expand Up @@ -619,6 +623,7 @@ public function testGetContactsOnlyEnumerateIfPhoneBookOrSameGroupInOwnGroupsOnl
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
['core', 'shareapi_exclude_groups', 'no', 'no'],
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

/** @var IUser|MockObject $currentUser */
Expand Down
5 changes: 5 additions & 0 deletions tests/lib/Share20/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,7 @@ public function testUserCreateChecksShareWithGroupMembersOnlyDifferentGroups() {
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

self::invokePrivate($this->manager, 'userCreateChecks', [$share]);
Expand Down Expand Up @@ -1602,6 +1603,7 @@ public function testUserCreateChecksShareWithGroupMembersOnlySharedGroup() {
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

$this->defaultProvider
Expand Down Expand Up @@ -1794,6 +1796,7 @@ public function testGroupCreateChecksShareWithGroupMembersOnlyNotInGroup() {
->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

self::invokePrivate($this->manager, 'groupCreateChecks', [$share]);
Expand All @@ -1817,6 +1820,7 @@ public function testGroupCreateChecksShareWithGroupMembersOnlyNullGroup() {
->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

$this->assertNull($this->invokePrivate($this->manager, 'groupCreateChecks', [$share]));
Expand Down Expand Up @@ -1846,6 +1850,7 @@ public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() {
->willReturnMap([
['core', 'shareapi_only_share_with_group_members', 'no', 'yes'],
['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
]);

self::invokePrivate($this->manager, 'groupCreateChecks', [$share]);
Expand Down

0 comments on commit 7dc5a91

Please sign in to comment.