Skip to content

fix: system address book card restriction #63392

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.

Sign up for GitHub

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

Open
SebastianKrupinski wants to merge 1 commit into
base: master
Choose a base branch
from
+421 −120
Open

fix: system address book card restriction #63392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
127 changes: 83 additions & 44 deletions apps/dav/lib/CardDAV/SystemAddressbook.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@

use OCA\Federation\TrustedServers;
use OCP\Accounts\IAccountManager;
use OCP\IConfig;
use OCP\IAppConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserSession;
use Sabre\CardDAV\Backend\BackendInterface;
use Sabre\CardDAV\Backend\SyncSupport;
Expand All @@ -35,7 +36,7 @@ public function __construct(
BackendInterface $carddavBackend,
array $addressBookInfo,
IL10N $l10n,
private IConfig $config,
private IAppConfig $appConfig,
private IUserSession $userSession,
private ?IRequest $request = null,
private ?TrustedServers $trustedServers = null,
Expand All @@ -53,12 +54,14 @@ public function __construct(
* 'Allow username autocompletion in share dialog' + 'Allow username autocompletion to users within the same groups' -> show only users in intersecting groups
* 'Allow username autocompletion in share dialog' + 'Allow username autocompletion to users based on phone number integration' -> show only the same user
* '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
* 'Restrict users to only share with users in their groups' -> show only users in intersecting groups, unless already narrowed further above
*/
#[\Override]
public function getChildren() {
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
$shareEnumeration = $this->appConfig->getValueBool('core', 'shareapi_allow_share_dialog_user_enumeration', true);
$shareEnumerationGroup = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_group');
$shareEnumerationPhone = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_phone');
$restrictToOwnGroups = $this->appConfig->getValueBool('core', 'shareapi_only_share_with_group_members');
$user = $this->userSession->getUser();
if (!$user) {
// Should never happen because we don't allow anonymous access
Expand All @@ -77,18 +80,17 @@ public function getChildren() {
// Group manager is not available, so we can't determine which data is safe
return [];
}
$groups = $this->groupManager->getUserGroups($user);
$names = [];
foreach ($groups as $group) {
$users = $group->getUsers();
foreach ($users as $groupUser) {
if ($groupUser->getBackendClassName() === 'Guests') {
continue;
}
$names[] = SyncService::getCardUri($groupUser);
}
return parent::getMultipleChildren($this->getCardsForUsersGroups($user));
}
if ($restrictToOwnGroups) {
if ($this->groupManager === null) {
// Group manager is not available, so we can't determine which data is safe
return [];
}
return parent::getMultipleChildren(array_unique($names));
if (!$this->exemptFromOwnGroupsRestriction($user)) {
return parent::getMultipleChildren($this->getCardsForUsersGroups($user));
}
// Member of an excluded group -> restriction does not apply, fall through to show everyone
}

$children = parent::getChildren();
Expand All @@ -105,9 +107,10 @@ public function getChildren() {
*/
#[\Override]
public function getMultipleChildren($paths): array {
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
$shareEnumeration = $this->appConfig->getValueBool('core', 'shareapi_allow_share_dialog_user_enumeration', true);
$shareEnumerationGroup = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_group');
$shareEnumerationPhone = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_phone');
$restrictToOwnGroups = $this->appConfig->getValueBool('core', 'shareapi_only_share_with_group_members');
$user = $this->userSession->getUser();
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
// No user or cards with no access
Expand All @@ -126,18 +129,18 @@ public function getMultipleChildren($paths): array {
// Group manager or user is not available, so we can't determine which data is safe
return [];
}
$groups = $this->groupManager->getUserGroups($user);
$allowedNames = [];
foreach ($groups as $group) {
$users = $group->getUsers();
foreach ($users as $groupUser) {
if ($groupUser->getBackendClassName() === 'Guests') {
continue;
}
$allowedNames[] = SyncService::getCardUri($groupUser);
}
return parent::getMultipleChildren(array_intersect($paths, $this->getCardsForUsersGroups($user)));
}
if ($restrictToOwnGroups) {
if ($this->groupManager === null || $user === null) {
// Group manager or user is not available, so we can't determine which data is safe
return [];
}
if (!$this->exemptFromOwnGroupsRestriction($user)) {
$allowedNames = $this->getCardsForUsersGroups($user);
return parent::getMultipleChildren(array_intersect($paths, $allowedNames));
}
return parent::getMultipleChildren(array_intersect($paths, $allowedNames));
// Member of an excluded group -> restriction does not apply, fall through to show everyone
}
if (!$this->isFederation()) {
return parent::getMultipleChildren($paths);
Expand Down Expand Up @@ -170,9 +173,10 @@ public function getMultipleChildren($paths): array {
#[\Override]
public function getChild($name): Card {
$user = $this->userSession->getUser();
$shareEnumeration = $this->config->getAppValue('core', 'shareapi_allow_share_dialog_user_enumeration', 'yes') === 'yes';
$shareEnumerationGroup = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no') === 'yes';
$shareEnumerationPhone = $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no') === 'yes';
$shareEnumeration = $this->appConfig->getValueBool('core', 'shareapi_allow_share_dialog_user_enumeration', true);
$shareEnumerationGroup = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_group');
$shareEnumerationPhone = $this->appConfig->getValueBool('core', 'shareapi_restrict_user_enumeration_to_phone');
$restrictToOwnGroups = $this->appConfig->getValueBool('core', 'shareapi_only_share_with_group_members');
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$ownName = $user !== null ? SyncService::getCardUri($user) : null;
if ($ownName === $name) {
Expand All @@ -185,20 +189,24 @@ public function getChild($name): Card {
// Group manager is not available, so we can't determine which data is safe
throw new Forbidden();
}
$groups = $this->groupManager->getUserGroups($user);
foreach ($groups as $group) {
foreach ($group->getUsers() as $groupUser) {
if ($groupUser->getBackendClassName() === 'Guests') {
continue;
}
$otherName = SyncService::getCardUri($groupUser);
if ($otherName === $name) {
return parent::getChild($name);
}
}
if (in_array($name, $this->getCardsForUsersGroups($user), true)) {
return parent::getChild($name);
}
throw new Forbidden();
}
if ($restrictToOwnGroups) {
if ($user === null || $this->groupManager === null) {
// Group manager is not available, so we can't determine which data is safe
throw new Forbidden();
}
if (!$this->exemptFromOwnGroupsRestriction($user)) {
if (in_array($name, $this->getCardsForUsersGroups($user), true)) {
return parent::getChild($name);
}
throw new Forbidden();
}
// Member of an excluded group -> restriction does not apply, fall through to show everyone
}
if (!$this->isFederation()) {
return parent::getChild($name);
}
Expand Down Expand Up @@ -290,6 +298,37 @@ private function isFederation(): bool {
return true;
}

/**
* A user who belongs to at least one excluded group is not subject to the
* 'shareapi_only_share_with_group_members' restriction at all, i.e. they can see everyone.
*/
private function exemptFromOwnGroupsRestriction(IUser $user): bool {
$excludedGroupIds = json_decode(
$this->appConfig->getValueString('core', 'shareapi_only_share_with_group_members_exclude_group_list', '[]'),
true
);
if (!is_array($excludedGroupIds) || $excludedGroupIds === []) {
return false;
}
return array_intersect($this->groupManager->getUserGroupIds($user), $excludedGroupIds) !== [];
}

/**
* @return string[] card URIs of non-guest users sharing at least one group with $user
*/
private function getCardsForUsersGroups(IUser $user): array {
$names = [];
foreach ($this->groupManager->getUserGroups($user) as $group) {
foreach ($group->getUsers() as $groupUser) {
if ($groupUser->getBackendClassName() === 'Guests') {
continue;
}
$names[] = SyncService::getCardUri($groupUser);
}
}
return array_values(array_unique($names));
}

/**
* If the validation doesn't work the card is "not found" so we
* return empty carddata even if the carddata might exist in the local backend.
Expand Down
5 changes: 1 addition & 4 deletions apps/dav/lib/CardDAV/UserAddressBooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
use OCA\DAV\ConfigLexicon;
use OCA\Federation\TrustedServers;
use OCP\IAppConfig;
use OCP\IConfig;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
Expand All @@ -35,7 +34,6 @@

class UserAddressBooks extends \Sabre\CardDAV\AddressBookHome {
protected IL10N $l10n;
protected IConfig $config;
protected IAppConfig $appConfig;

public function __construct(
Expand All @@ -48,7 +46,6 @@ public function __construct(
parent::__construct($carddavBackend, $principalUri);

$this->l10n = Util::getL10N('dav');
$this->config = Server::get(IConfig::class);
$this->appConfig = Server::get(IAppConfig::class);
}

Expand Down Expand Up @@ -92,7 +89,7 @@ public function getChildren() {
$this->carddavBackend,
$addressBook,
$this->l10n,
$this->config,
$this->appConfig,
Server::get(IUserSession::class),
$request,
$trustedServers,
Expand Down
Loading
Loading