Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New core setting : shareapi_only_share_with_group_members_exclude_gro… #38173

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
$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', '');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::getAppValue has been marked as deprecated

$parameters = [
// Built-In Sharing
Expand All @@ -83,6 +84,7 @@
'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) ?? [];
Fixed Show fixed Hide fixed
}
}

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);
Fixed Show fixed Hide fixed
}

$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) ?? [];
Fixed Show fixed Hide fixed
}
}

/**
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);
Fixed Show fixed Hide fixed

$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) ?? [];
Fixed Show fixed Hide fixed
}
}

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);
Fixed Show fixed Hide fixed

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() {
artonge marked this conversation as resolved.
Show resolved Hide resolved
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.
smarinier marked this conversation as resolved.
Show resolved Hide resolved
* @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
Loading