Browse Source

Merge pull request #38448 from nextcloud/fix/carddav/no-sab-guest-users

fix(carddav): Don't show system address book cards to guests
pull/38459/head
Arthur Schiwon 3 years ago
committed by GitHub
parent
commit
cd5bd11771
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 14
      apps/dav/lib/CardDAV/SystemAddressbook.php
  2. 78
      apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php

14
apps/dav/lib/CardDAV/SystemAddressbook.php

@ -92,7 +92,7 @@ class SystemAddressbook extends AddressBook {
// Should never happen because we don't allow anonymous access
return [];
}
if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
if ($user->getBackendClassName() === 'Guests' || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$name = SyncService::getCardUri($user);
try {
return [parent::getChild($name)];
@ -135,8 +135,8 @@ class SystemAddressbook extends AddressBook {
$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();
$user = $this->userSession->getUser();
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
// No user or cards with no access
if ($user === null || !in_array(SyncService::getCardUri($user), $paths, true)) {
return [];
@ -149,7 +149,6 @@ class SystemAddressbook extends AddressBook {
}
}
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 [];
@ -196,19 +195,18 @@ class SystemAddressbook extends AddressBook {
* @throws Forbidden
*/
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';
if (!$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$currentUser = $this->userSession->getUser();
$ownName = $currentUser !== null ? SyncService::getCardUri($currentUser) : null;
if (($user !== null && $user->getBackendClassName() === 'Guests') || !$shareEnumeration || (!$shareEnumerationGroup && $shareEnumerationPhone)) {
$ownName = $user !== null ? SyncService::getCardUri($user) : 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();

78
apps/dav/tests/unit/CardDAV/SystemAddressBookTest.php

@ -90,6 +90,46 @@ class SystemAddressBookTest extends TestCase {
);
}
public function testGetChildrenAsGuest(): 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'],
]);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$user->method('getBackendClassName')->willReturn('Guests');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$vcfWithScopes = <<<VCF
BEGIN:VCARD
VERSION:3.0
PRODID:-//Sabre//Sabre VObject 4.4.2//EN
UID:admin
FN;X-NC-SCOPE=v2-federated:admin
N;X-NC-SCOPE=v2-federated:admin;;;;
ADR;TYPE=OTHER;X-NC-SCOPE=v2-local:Testing test test test;;;;;;
EMAIL;TYPE=OTHER;X-NC-SCOPE=v2-federated:miau_lalala@gmx.net
TEL;TYPE=OTHER;X-NC-SCOPE=v2-local:+435454454544
CLOUD:admin@http://localhost
END:VCARD
VCF;
$originalCard = [
'carddata' => $vcfWithScopes,
];
$this->cardDavBackend->expects(self::once())
->method('getCard')
->with(123, 'Guests:user.vcf')
->willReturn($originalCard);
$children = $this->addressBook->getChildren();
self::assertCount(1, $children);
}
public function testGetFilteredChildForFederation(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
@ -172,6 +212,24 @@ VCF;
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetChildAsGuest(): 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'],
]);
$user = $this->createMock(IUser::class);
$user->method('getBackendClassName')->willReturn('Guests');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$this->expectException(Forbidden::class);
$this->addressBook->getChild("LDAP:user.vcf");
}
public function testGetChildWithGroupEnumerationRestriction(): void {
$this->config->expects(self::exactly(3))
->method('getAppValue')
@ -322,6 +380,26 @@ VCF;
self::assertCount(2, $cards);
}
public function testGetMultipleChildrenAsGuest(): 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'],
]);
$user = $this->createMock(IUser::class);
$user->method('getUID')->willReturn('user');
$user->method('getBackendClassName')->willReturn('Guests');
$this->userSession->expects(self::once())
->method('getUser')
->willReturn($user);
$cards = $this->addressBook->getMultipleChildren(['Database:user1.vcf', 'LDAP:user2.vcf']);
self::assertEmpty($cards);
}
public function testGetMultipleChildren(): void {
$this->config
->method('getAppValue')

Loading…
Cancel
Save