Merge pull request #3595 from nextcloud/dav-sharing-changes

Restrict DAV share handling to the owner only
pull/4418/head
Morris Jobke 9 years ago committed by GitHub
commit b99d2b78bd
  1. 71
      apps/dav/lib/CalDAV/Calendar.php
  2. 66
      apps/dav/lib/CardDAV/AddressBook.php
  3. 4
      apps/dav/tests/unit/CalDAV/CalDavBackendTest.php
  4. 4
      apps/dav/tests/unit/CardDAV/AddressBookTest.php
  5. 2
      apps/dav/tests/unit/CardDAV/ContactsManagerTest.php

@ -30,6 +30,12 @@ use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\PropPatch;
/**
* Class Calendar
*
* @package OCA\DAV\CalDAV
* @property BackendInterface|CalDavBackend $caldavBackend
*/
class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
public function __construct(BackendInterface $caldavBackend, $calendarInfo, IL10N $l10n) {
@ -61,11 +67,13 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
* @param array $add
* @param array $remove
* @return void
* @throws Forbidden
*/
function updateShares(array $add, array $remove) {
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
$calDavBackend->updateShares($this, $add, $remove);
public function updateShares(array $add, array $remove) {
if ($this->isShared()) {
throw new Forbidden();
}
$this->caldavBackend->updateShares($this, $add, $remove);
}
/**
@ -80,10 +88,11 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
*
* @return array
*/
function getShares() {
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
return $calDavBackend->getShares($this->getResourceId());
public function getShares() {
if ($this->isShared()) {
return [];
}
return $this->caldavBackend->getShares($this->getResourceId());
}
/**
@ -100,7 +109,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
return $this->calendarInfo['principaluri'];
}
function getACL() {
public function getACL() {
$acl = [
[
'privilege' => '{DAV:}read',
@ -136,27 +145,29 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
];
}
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
return $calDavBackend->applyShareAcl($this->getResourceId(), $acl);
if ($this->isShared()) {
return $acl;
}
return $this->caldavBackend->applyShareAcl($this->getResourceId(), $acl);
}
function getChildACL() {
public function getChildACL() {
return $this->getACL();
}
function getOwner() {
public function getOwner() {
if (isset($this->calendarInfo['{http://owncloud.org/ns}owner-principal'])) {
return $this->calendarInfo['{http://owncloud.org/ns}owner-principal'];
}
return parent::getOwner();
}
function delete() {
public function delete() {
if (isset($this->calendarInfo['{http://owncloud.org/ns}owner-principal']) &&
$this->calendarInfo['{http://owncloud.org/ns}owner-principal'] !== $this->calendarInfo['principaluri']) {
$principal = 'principal:' . parent::getOwner();
$shares = $this->getShares();
$shares = $this->caldavBackend->getShares($this->getResourceId());
$shares = array_filter($shares, function($share) use ($principal){
return $share['href'] === $principal;
});
@ -164,9 +175,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
throw new Forbidden();
}
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
$calDavBackend->updateShares($this, [], [
$this->caldavBackend->updateShares($this, [], [
'href' => $principal
]);
return;
@ -174,7 +183,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
parent::delete();
}
function propPatch(PropPatch $propPatch) {
public function propPatch(PropPatch $propPatch) {
// parent::propPatch will only update calendars table
// if calendar is shared, changes have to be made to the properties table
if (!$this->isShared()) {
@ -182,7 +191,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
}
function getChild($name) {
public function getChild($name) {
$obj = $this->caldavBackend->getCalendarObject($this->calendarInfo['id'], $name);
@ -190,7 +199,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
throw new NotFound('Calendar object not found');
}
if ($this->isShared() && $obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE) {
if ($obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE && $this->isShared()) {
throw new NotFound('Calendar object not found');
}
@ -200,12 +209,12 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
function getChildren() {
public function getChildren() {
$objs = $this->caldavBackend->getCalendarObjects($this->calendarInfo['id']);
$children = [];
foreach ($objs as $obj) {
if ($this->isShared() && $obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE) {
if ($obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE && $this->isShared()) {
continue;
}
$obj['acl'] = $this->getChildACL();
@ -215,12 +224,12 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
function getMultipleChildren(array $paths) {
public function getMultipleChildren(array $paths) {
$objs = $this->caldavBackend->getMultipleCalendarObjects($this->calendarInfo['id'], $paths);
$children = [];
foreach ($objs as $obj) {
if ($this->isShared() && $obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE) {
if ($obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE && $this->isShared()) {
continue;
}
$obj['acl'] = $this->getChildACL();
@ -230,19 +239,19 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
function childExists($name) {
public function childExists($name) {
$obj = $this->caldavBackend->getCalendarObject($this->calendarInfo['id'], $name);
if (!$obj) {
return false;
}
if ($this->isShared() && $obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE) {
if ($obj['classification'] === CalDavBackend::CLASSIFICATION_PRIVATE && $this->isShared()) {
return false;
}
return true;
}
function calendarQuery(array $filters) {
public function calendarQuery(array $filters) {
$uris = $this->caldavBackend->calendarQuery($this->calendarInfo['id'], $filters);
if ($this->isShared()) {
@ -258,7 +267,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
* @param boolean $value
* @return string|null
*/
function setPublishStatus($value) {
public function setPublishStatus($value) {
$publicUri = $this->caldavBackend->setPublishStatus($value, $this);
$this->calendarInfo['publicuri'] = $publicUri;
return $publicUri;
@ -267,7 +276,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
/**
* @return mixed $value
*/
function getPublishStatus() {
public function getPublishStatus() {
return $this->caldavBackend->getPublishStatus($this);
}

@ -29,6 +29,12 @@ use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\PropPatch;
/**
* Class AddressBook
*
* @package OCA\DAV\CardDAV
* @property BackendInterface|CardDavBackend $carddavBackend
*/
class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
/**
@ -41,8 +47,8 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
public function __construct(BackendInterface $carddavBackend, array $addressBookInfo, IL10N $l10n) {
parent::__construct($carddavBackend, $addressBookInfo);
if ($this->getName() === CardDavBackend::PERSONAL_ADDRESSBOOK_URI &&
$this->addressBookInfo['{DAV:}displayname'] === CardDavBackend::PERSONAL_ADDRESSBOOK_NAME) {
if ($this->addressBookInfo['{DAV:}displayname'] === CardDavBackend::PERSONAL_ADDRESSBOOK_NAME &&
$this->getName() === CardDavBackend::PERSONAL_ADDRESSBOOK_URI) {
$this->addressBookInfo['{DAV:}displayname'] = $l10n->t('Contacts');
}
}
@ -64,11 +70,13 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
* @param array $add
* @param array $remove
* @return void
* @throws Forbidden
*/
function updateShares(array $add, array $remove) {
/** @var CardDavBackend $carddavBackend */
$carddavBackend = $this->carddavBackend;
$carddavBackend->updateShares($this, $add, $remove);
public function updateShares(array $add, array $remove) {
if ($this->isShared()) {
throw new Forbidden();
}
$this->carddavBackend->updateShares($this, $add, $remove);
}
/**
@ -83,13 +91,14 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
*
* @return array
*/
function getShares() {
/** @var CardDavBackend $carddavBackend */
$carddavBackend = $this->carddavBackend;
return $carddavBackend->getShares($this->getResourceId());
public function getShares() {
if ($this->isShared()) {
return [];
}
return $this->carddavBackend->getShares($this->getResourceId());
}
function getACL() {
public function getACL() {
$acl = [
[
'privilege' => '{DAV:}read',
@ -123,16 +132,18 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
];
}
/** @var CardDavBackend $carddavBackend */
$carddavBackend = $this->carddavBackend;
return $carddavBackend->applyShareAcl($this->getResourceId(), $acl);
if ($this->isShared()) {
return $acl;
}
return $this->carddavBackend->applyShareAcl($this->getResourceId(), $acl);
}
function getChildACL() {
public function getChildACL() {
return $this->getACL();
}
function getChild($name) {
public function getChild($name) {
$obj = $this->carddavBackend->getCard($this->addressBookInfo['id'], $name);
if (!$obj) {
@ -150,17 +161,17 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
return $this->addressBookInfo['id'];
}
function getOwner() {
public function getOwner() {
if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
return $this->addressBookInfo['{http://owncloud.org/ns}owner-principal'];
}
return parent::getOwner();
}
function delete() {
public function delete() {
if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
$principal = 'principal:' . parent::getOwner();
$shares = $this->getShares();
$shares = $this->carddavBackend->getShares($this->getResourceId());
$shares = array_filter($shares, function($share) use ($principal){
return $share['href'] === $principal;
});
@ -168,9 +179,7 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
throw new Forbidden();
}
/** @var CardDavBackend $cardDavBackend */
$cardDavBackend = $this->carddavBackend;
$cardDavBackend->updateShares($this, [], [
$this->carddavBackend->updateShares($this, [], [
'href' => $principal
]);
return;
@ -178,7 +187,7 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
parent::delete();
}
function propPatch(PropPatch $propPatch) {
public function propPatch(PropPatch $propPatch) {
if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
throw new Forbidden();
}
@ -186,10 +195,15 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
}
public function getContactsGroups() {
/** @var CardDavBackend $cardDavBackend */
$cardDavBackend = $this->carddavBackend;
return $this->carddavBackend->collectCardProperties($this->getResourceId(), 'CATEGORIES');
}
private function isShared() {
if (!isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
return false;
}
return $cardDavBackend->collectCardProperties($this->getResourceId(), 'CATEGORIES');
return $this->addressBookInfo['{http://owncloud.org/ns}owner-principal'] !== $this->addressBookInfo['principaluri'];
}
private function canWrite() {

@ -143,8 +143,6 @@ class CalDavBackendTest extends AbstractCalDavBackendTest {
$this->assertAcl(self::UNIT_TEST_USER, '{DAV:}write', $acl);
$this->assertAccess($userCanRead, self::UNIT_TEST_USER1, '{DAV:}read', $acl);
$this->assertAccess($userCanWrite, self::UNIT_TEST_USER1, '{DAV:}write', $acl);
$this->assertAccess($groupCanRead, self::UNIT_TEST_GROUP, '{DAV:}read', $acl);
$this->assertAccess($groupCanWrite, self::UNIT_TEST_GROUP, '{DAV:}write', $acl);
$this->assertEquals(self::UNIT_TEST_USER, $calendar->getOwner());
// test acls on the child
@ -178,8 +176,6 @@ EOD;
$this->assertAcl(self::UNIT_TEST_USER, '{DAV:}write', $acl);
$this->assertAccess($userCanRead, self::UNIT_TEST_USER1, '{DAV:}read', $acl);
$this->assertAccess($userCanWrite, self::UNIT_TEST_USER1, '{DAV:}write', $acl);
$this->assertAccess($groupCanRead, self::UNIT_TEST_GROUP, '{DAV:}read', $acl);
$this->assertAccess($groupCanWrite, self::UNIT_TEST_GROUP, '{DAV:}write', $acl);
// delete the address book
$this->dispatcher->expects($this->at(0))

@ -40,6 +40,7 @@ class AddressBookTest extends TestCase {
]);
$calendarInfo = [
'{http://owncloud.org/ns}owner-principal' => 'user1',
'{DAV:}displayname' => 'Test address book',
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default',
@ -61,6 +62,7 @@ class AddressBookTest extends TestCase {
]);
$calendarInfo = [
'{http://owncloud.org/ns}owner-principal' => 'user1',
'{DAV:}displayname' => 'Test address book',
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default',
@ -78,6 +80,7 @@ class AddressBookTest extends TestCase {
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$calendarInfo = [
'{http://owncloud.org/ns}owner-principal' => 'user1',
'{DAV:}displayname' => 'Test address book',
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default',
@ -95,6 +98,7 @@ class AddressBookTest extends TestCase {
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$backend->expects($this->any())->method('applyShareAcl')->willReturnArgument(1);
$calendarInfo = [
'{DAV:}displayname' => 'Test address book',
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default'

@ -39,7 +39,7 @@ class ContactsManagerTest extends TestCase {
/** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backEnd */
$backEnd = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$backEnd->method('getAddressBooksForUser')->willReturn([
['uri' => 'default'],
['{DAV:}displayname' => 'Test address book', 'uri' => 'default'],
]);
$l = $this->createMock(IL10N::class);

Loading…
Cancel
Save