Skip to content

Commit 40cbf7c

Browse files
fix: system address book card restriction
Signed-off-by: SebastianKrupinski <krupinskis05@gmail.com>
1 parent 0417403 commit 40cbf7c

2 files changed

Lines changed: 334 additions & 41 deletions

File tree

apps/dav/lib/CardDAV/SystemAddressbook.php

Lines changed: 72 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCP\IGroupManager;
1616
use OCP\IL10N;
1717
use OCP\IRequest;
18+
use OCP\IUser;
1819
use OCP\IUserSession;
1920
use Sabre\CardDAV\Backend\BackendInterface;
2021
use Sabre\CardDAV\Backend\SyncSupport;
@@ -53,12 +54,14 @@ public function __construct(
5354
* 'Allow username autocompletion in share dialog' + 'Allow username autocompletion to users within the same groups' -> show only users in intersecting groups
5455
* 'Allow username autocompletion in share dialog' + 'Allow username autocompletion to users based on phone number integration' -> show only the same user
5556
* 'Allow username autocompletion in share dialog' + 'Allow username autocompletion to users within the same groups' + 'Allow username autocompletion to users based on phone number integration' -> show only users in intersecting groups
57+
* 'Restrict users to only share with users in their groups' -> show only users in intersecting groups, unless already narrowed further above
5658
*/
5759
#[\Override]
5860
public function getChildren() {
5961
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
6062
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
6163
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
64+
$restrictToOwnGroups = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
6265
$user = $this->userSession->getUser();
6366
if (!$user) {
6467
// Should never happen because we don't allow anonymous access
@@ -77,18 +80,17 @@ public function getChildren() {
7780
// Group manager is not available, so we can't determine which data is safe
7881
return [];
7982
}
80-
$groups = $this->groupManager->getUserGroups($user);
81-
$names = [];
82-
foreach ($groups as $group) {
83-
$users = $group->getUsers();
84-
foreach ($users as $groupUser) {
85-
if ($groupUser->getBackendClassName() === 'Guests') {
86-
continue;
87-
}
88-
$names[] = SyncService::getCardUri($groupUser);
89-
}
83+
return parent::getMultipleChildren($this->getCardsForUsersGroups($user));
84+
}
85+
if ($restrictToOwnGroups) {
86+
if ($this->groupManager === null) {
87+
// Group manager is not available, so we can't determine which data is safe
88+
return [];
9089
}
91-
return parent::getMultipleChildren(array_unique($names));
90+
if (!$this->exemptFromOwnGroupsRestriction($user)) {
91+
return parent::getMultipleChildren($this->getCardsForUsersGroups($user));
92+
}
93+
// Member of an excluded group -> restriction does not apply, fall through to show everyone
9294
}
9395

9496
$children = parent::getChildren();
@@ -108,6 +110,7 @@ public function getMultipleChildren($paths): array {
108110
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
109111
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
110112
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
113+
$restrictToOwnGroups = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
111114
$user = $this->userSession->getUser();
112115
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
113116
// No user or cards with no access
@@ -126,18 +129,18 @@ public function getMultipleChildren($paths): array {
126129
// Group manager or user is not available, so we can't determine which data is safe
127130
return [];
128131
}
129-
$groups = $this->groupManager->getUserGroups($user);
130-
$allowedNames = [];
131-
foreach ($groups as $group) {
132-
$users = $group->getUsers();
133-
foreach ($users as $groupUser) {
134-
if ($groupUser->getBackendClassName() === 'Guests') {
135-
continue;
136-
}
137-
$allowedNames[] = SyncService::getCardUri($groupUser);
138-
}
132+
return parent::getMultipleChildren(array_intersect($paths, $this->getCardsForUsersGroups($user)));
133+
}
134+
if ($restrictToOwnGroups) {
135+
if ($this->groupManager === null || $user === null) {
136+
// Group manager or user is not available, so we can't determine which data is safe
137+
return [];
138+
}
139+
if (!$this->exemptFromOwnGroupsRestriction($user)) {
140+
$allowedNames = $this->getCardsForUsersGroups($user);
141+
return parent::getMultipleChildren(array_intersect($paths, $allowedNames));
139142
}
140-
return parent::getMultipleChildren(array_intersect($paths, $allowedNames));
143+
// Member of an excluded group -> restriction does not apply, fall through to show everyone
141144
}
142145
if (!$this->isFederation()) {
143146
return parent::getMultipleChildren($paths);
@@ -173,6 +176,7 @@ public function getChild($name): Card {
173176
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
174177
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
175178
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
179+
$restrictToOwnGroups = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'no') === 'yes';
176180
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
177181
$ownName = $user !== null ? SyncService::getCardUri($user) : null;
178182
if ($ownName === $name) {
@@ -185,20 +189,24 @@ public function getChild($name): Card {
185189
// Group manager is not available, so we can't determine which data is safe
186190
throw new Forbidden();
187191
}
188-
$groups = $this->groupManager->getUserGroups($user);
189-
foreach ($groups as $group) {
190-
foreach ($group->getUsers() as $groupUser) {
191-
if ($groupUser->getBackendClassName() === 'Guests') {
192-
continue;
193-
}
194-
$otherName = SyncService::getCardUri($groupUser);
195-
if ($otherName === $name) {
196-
return parent::getChild($name);
197-
}
198-
}
192+
if (in_array($name, $this->getCardsForUsersGroups($user), true)) {
193+
return parent::getChild($name);
199194
}
200195
throw new Forbidden();
201196
}
197+
if ($restrictToOwnGroups) {
198+
if ($user === null || $this->groupManager === null) {
199+
// Group manager is not available, so we can't determine which data is safe
200+
throw new Forbidden();
201+
}
202+
if (!$this->exemptFromOwnGroupsRestriction($user)) {
203+
if (in_array($name, $this->getCardsForUsersGroups($user), true)) {
204+
return parent::getChild($name);
205+
}
206+
throw new Forbidden();
207+
}
208+
// Member of an excluded group -> restriction does not apply, fall through to show everyone
209+
}
202210
if (!$this->isFederation()) {
203211
return parent::getChild($name);
204212
}
@@ -290,6 +298,37 @@ private function isFederation(): bool {
290298
return true;
291299
}
292300

301+
/**
302+
* A user who belongs to at least one excluded group is not subject to the
303+
* 'shareapi_only_share_with_group_members' restriction at all, i.e. they can see everyone.
304+
*/
305+
private function exemptFromOwnGroupsRestriction(IUser $user): bool {
306+
$excludedGroupIds = json_decode(
307+
$this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '[]'),
308+
true
309+
);
310+
if (!is_array($excludedGroupIds) || $excludedGroupIds === []) {
311+
return false;
312+
}
313+
return array_intersect($this->groupManager->getUserGroupIds($user), $excludedGroupIds) !== [];
314+
}
315+
316+
/**
317+
* @return string[] card URIs of non-guest users sharing at least one group with $user
318+
*/
319+
private function getCardsForUsersGroups(IUser $user): array {
320+
$names = [];
321+
foreach ($this->groupManager->getUserGroups($user) as $group) {
322+
foreach ($group->getUsers() as $groupUser) {
323+
if ($groupUser->getBackendClassName() === 'Guests') {
324+
continue;
325+
}
326+
$names[] = SyncService::getCardUri($groupUser);
327+
}
328+
}
329+
return array_values(array_unique($names));
330+
}
331+
293332
/**
294333
* If the validation doesn't work the card is "not found" so we
295334
* return empty carddata even if the carddata might exist in the local backend.

0 commit comments

Comments
 (0)