upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-03-04 06:21:07 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 14

Merge pull request #38455 from nextcloud/backport/38423/stable27

Browse source 
[stable27] fix(carddav): Check enumeration settings for all SAB methods
This commit is contained in:
Andy Scherzinger 2023-05-25 12:11:45 +02:00 committed by GitHub
commit f5dc8fdfba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 309 additions and 3 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

70
apps/dav/lib/CardDAV/SystemAddressbook.php
View file

@ -46,7 +46,9 @@ use Sabre\DAV\ICollection;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Reader;
use function array_filter;
use function array_intersect;
use function array_unique;
use function in_array;
class SystemAddressbook extends AddressBook {
public const URI_SHARED = 'z-server-generated--system';
@ -92,7 +94,7 @@ class SystemAddressbook extends AddressBook {
// Should never happen because we don't allow anonymous access
return [];
}
if (!$shareEnumeration || !$shareEnumerationGroup && $shareEnumerationPhone) {
if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$name = SyncService::getCardUri($user);
try {
return [parent::getChild($name)];
@ -132,6 +134,41 @@ class SystemAddressbook extends AddressBook {
* @throws NotFound
*/
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';
if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$user = $this->userSession->getUser();
// No user or cards with no access
if ($user === null || !in_array(SyncService::getCardUri($user), $paths, true)) {
return [];
}
// Only return the own card
try {
return [parent::getChild(SyncService::getCardUri($user))];
} catch (NotFound $e) {
return [];
}
}
if ($shareEnumerationGroup) {
$user = $this->userSession->getUser();
if ($this->groupManager === null || $user === null) {
// 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, $allowedNames));
}
if (!$this->isFederation()) {
return parent::getMultipleChildren($paths);
}
@ -161,6 +198,37 @@ class SystemAddressbook extends AddressBook {
* @throws Forbidden
*/
public function getChild($name): Card {
$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';
if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$currentUser = $this->userSession->getUser();
$ownName = $currentUser !== null ? SyncService::getCardUri($currentUser) : null;
if ($ownName === $name) {
return parent::getChild($name);
}
throw new Forbidden();
}
if ($shareEnumerationGroup) {
$user = $this->userSession->getUser();
if ($user === null || $this->groupManager === null) {
// 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);
}
}
}
throw new Forbidden();
}
if (!$this->isFederation()) {
return parent::getChild($name);
}

242
apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php
View file

@ -26,21 +26,26 @@ declare(strict_types=1);
namespace OCA\DAV\Tests\unit\CardDAV;
use OC\AppFramework\Http\Request;
use OCA\DAV\CardDAV\SyncService;
use OCA\DAV\CardDAV\SystemAddressbook;
use OCA\Federation\TrustedServers;
use OCP\Accounts\IAccountManager;
use OCP\IConfig;
use OCP\IGroup;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserSession;
use PHPUnit\Framework\MockObject\MockObject;
use Sabre\CardDAV\Backend\BackendInterface;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\VObject\Component\VCard;
use Sabre\VObject\Reader;
use Test\TestCase;
class SystemAddressBookTest extends TestCase {
private MockObject|BackendInterface $cardDavBackend;
private array $addressBookInfo;
private IL10N|MockObject $l10n;
@ -49,6 +54,7 @@ class SystemAddressBookTest extends TestCase {
private IRequest|MockObject $request;
private array $server;
private TrustedServers|MockObject $trustedServers;
private IGroupManager|MockObject $groupManager;
private SystemAddressbook $addressBook;
protected function setUp(): void {
@ -70,6 +76,7 @@ class SystemAddressBookTest extends TestCase {
];
$this->request->method('__get')->with('server')->willReturn($this->server);
$this->trustedServers = $this->createMock(TrustedServers::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->addressBook = new SystemAddressbook(
$this->cardDavBackend,
@ -79,11 +86,18 @@ class SystemAddressBookTest extends TestCase {
$this->userSession,
$this->request,
$this->trustedServers,
null,
$this->groupManager,
);
}
public function testGetFilteredChildForFederation(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$this->trustedServers->expects(self::once())
->method('getServers')
->willReturn([
@ -125,4 +139,228 @@ VCF;
}
}
public function testGetChildNotFound(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$this->trustedServers->expects(self::once())
->method('getServers')
->willReturn([
[
'shared_secret' => 'shared123',
],
]);
$this->expectException(NotFound::class);
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetChildWithoutEnumeration(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$this->expectException(Forbidden::class);
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetChildWithGroupEnumerationRestriction(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$user = $this->createMock(IUser::class);
$user->method('getBackendClassName')->willReturn('LDAP');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$otherUser = $this->createMock(IUser::class);
$user->method('getBackendClassName')->willReturn('LDAP');
$otherUser->method('getUID')->willReturn('other');
$group = $this->createMock(IGroup::class);
$group->expects(self::once())
->method('getUsers')
->willReturn([$otherUser]);
$this->groupManager->expects(self::once())
->method('getUserGroups')
->with($user)
->willReturn([$group]);
$cardData = <<<VCF
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:other
END:VCARD
VCF;
$this->cardDavBackend->expects(self::once())
->method('getCard')
->with($this->addressBookInfo['id'], "{$otherUser->getBackendClassName()}:{$otherUser->getUID()}.vcf")
->willReturn([
'id' => 123,
'carddata' => $cardData,
]);
$this->addressBook->getChild("{$otherUser->getBackendClassName()}:{$otherUser->getUID()}.vcf");
}
public function testGetChildWithPhoneNumberEnumerationRestriction(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
]);
$user = $this->createMock(IUser::class);
$user->method('getBackendClassName')->willReturn('LDAP');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$this->expectException(Forbidden::class);
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetOwnChildWithPhoneNumberEnumerationRestriction(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'yes'],
]);
$user = $this->createMock(IUser::class);
$user->method('getBackendClassName')->willReturn('LDAP');
$user->method('getUID')->willReturn('user');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$cardData = <<<VCF
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:user
END:VCARD
VCF;
$this->cardDavBackend->expects(self::once())
->method('getCard')
->with($this->addressBookInfo['id'], 'LDAP:user.vcf')
->willReturn([
'id' => 123,
'carddata' => $cardData,
]);
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetMultipleChildrenWithGroupEnumerationRestriction(): void {
$this->config
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$user->method('getBackendClassName')->willReturn('LDAP');
$other1 = $this->createMock(IUser::class);
$other1->method('getUID')->willReturn('other1');
$other1->method('getBackendClassName')->willReturn('LDAP');
$other2 = $this->createMock(IUser::class);
$other2->method('getUID')->willReturn('other2');
$other2->method('getBackendClassName')->willReturn('LDAP');
$other3 = $this->createMock(IUser::class);
$other3->method('getUID')->willReturn('other3');
$other3->method('getBackendClassName')->willReturn('LDAP');
$this->userSession
->method('getUser')
->willReturn($user);
$group1 = $this->createMock(IGroup::class);
$group1
->method('getUsers')
->willReturn([$user, $other1]);
$group2 = $this->createMock(IGroup::class);
$group2
->method('getUsers')
->willReturn([$other1, $other2, $user]);
$this->groupManager
->method('getUserGroups')
->with($user)
->willReturn([$group1]);
$this->cardDavBackend->expects(self::once())
->method('getMultipleCards')
->with($this->addressBookInfo['id'], [
SyncService::getCardUri($user),
SyncService::getCardUri($other1),
])
->willReturn([
[],
[],
]);
$cards = $this->addressBook->getMultipleChildren([
SyncService::getCardUri($user),
SyncService::getCardUri($other1),
// SyncService::getCardUri($other2), // Omitted to test that it's not returned as stray
SyncService::getCardUri($other3), // No overlapping group with this one
]);
self::assertCount(2, $cards);
}
public function testGetMultipleChildren(): void {
$this->config
->method('getAppValue')
->willReturnMap([
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
]);
$this->trustedServers
->method('getServers')
->willReturn([
[
'shared_secret' => 'shared123',
],
]);
$cardData = <<<VCF
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:user
END:VCARD
VCF;
$this->cardDavBackend->expects(self::once())
->method('getMultipleCards')
->with($this->addressBookInfo['id'], ['Database:user1.vcf', 'LDAP:user2.vcf'])
->willReturn([
[
'id' => 123,
'carddata' => $cardData,
],
[
'id' => 321,
'carddata' => $cardData,
],
]);
$cards = $this->addressBook->getMultipleChildren(['Database:user1.vcf', 'LDAP:user2.vcf']);
self::assertCount(2, $cards);
}
}