Fix acls for calendar objects and cards - fixes #23273

This commit is contained in:
Thomas Müller 2016-03-17 15:39:08 +01:00
parent cb300d164e
commit f28817aed5
9 changed files with 193 additions and 84 deletions

View File

@ -23,6 +23,7 @@
// Backends
use OCA\DAV\CalDAV\CalDavBackend;
use OCA\DAV\Connector\LegacyDAVACL;
use OCA\DAV\CalDAV\CalendarRoot;
use OCA\DAV\Connector\Sabre\Auth;
use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin;
@ -67,7 +68,7 @@ $server->addPlugin(new MaintenancePlugin());
$server->addPlugin(new \Sabre\DAV\Auth\Plugin($authBackend, 'ownCloud'));
$server->addPlugin(new \Sabre\CalDAV\Plugin());
$acl = new \OCA\DAV\Connector\LegacyDAVACL();
$acl = new LegacyDAVACL();
$server->addPlugin($acl);
$server->addPlugin(new \Sabre\CalDAV\ICSExportPlugin());

View File

@ -199,6 +199,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
$fields[] = 'a.components';
$fields[] = 'a.principaluri';
$fields[] = 'a.transparent';
$fields[] = 's.access';
$query = $this->db->getQueryBuilder();
$result = $query->select($fields)
->from('dav_shares', 's')
@ -226,6 +227,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription
'{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet($components),
'{' . Plugin::NS_CALDAV . '}schedule-calendar-transp' => new ScheduleCalendarTransp($row['transparent']?'transparent':'opaque'),
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal' => $row['principaluri'],
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only' => (int)$row['access'] === Backend::ACCESS_READ,
];
foreach($this->propertyMap as $xmlName=>$dbName) {

View File

@ -86,7 +86,31 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
function getACL() {
$acl = parent::getACL();
$acl = [
[
'privilege' => '{DAV:}read',
'principal' => $this->getOwner(),
'protected' => true,
]];
$acl[] = [
'privilege' => '{DAV:}write',
'principal' => $this->getOwner(),
'protected' => true,
];
if ($this->getOwner() !== parent::getOwner()) {
$acl[] = [
'privilege' => '{DAV:}read',
'principal' => parent::getOwner(),
'protected' => true,
];
if ($this->canWrite()) {
$acl[] = [
'privilege' => '{DAV:}write',
'principal' => parent::getOwner(),
'protected' => true,
];
}
}
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
@ -94,11 +118,7 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
function getChildACL() {
$acl = parent::getChildACL();
/** @var CalDavBackend $calDavBackend */
$calDavBackend = $this->caldavBackend;
return $calDavBackend->applyShareAcl($this->getResourceId(), $acl, parent::getOwner());
return $this->getACL();
}
function getOwner() {
@ -137,4 +157,12 @@ class Calendar extends \Sabre\CalDAV\Calendar implements IShareable {
}
parent::propPatch($propPatch);
}
private function canWrite() {
if (isset($this->calendarInfo['{http://owncloud.org/ns}read-only'])) {
return !$this->calendarInfo['{http://owncloud.org/ns}read-only'];
}
return true;
}
}

View File

@ -21,6 +21,7 @@
namespace OCA\DAV\CardDAV;
use OCA\DAV\DAV\Sharing\IShareable;
use Sabre\CardDAV\Card;
use Sabre\DAV\Exception\Forbidden;
use Sabre\DAV\Exception\NotFound;
use Sabre\DAV\PropPatch;
@ -70,39 +71,31 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
}
function getACL() {
$acl = parent::getACL();
if ($this->getOwner() === 'principals/system/system') {
$acl[] = [
'privilege' => '{DAV:}read',
'principal' => '{DAV:}authenticated',
'protected' => true,
$acl = [
[
'privilege' => '{DAV:}read',
'principal' => $this->getOwner(),
'protected' => true,
]];
$acl[] = [
'privilege' => '{DAV:}write',
'principal' => $this->getOwner(),
'protected' => true,
];
}
// add the current user
if (isset($this->addressBookInfo['{http://owncloud.org/ns}owner-principal'])) {
$owner = $this->addressBookInfo['{http://owncloud.org/ns}owner-principal'];
$acl[] = [
if ($this->getOwner() !== parent::getOwner()) {
$acl[] = [
'privilege' => '{DAV:}read',
'principal' => $owner,
'principal' => parent::getOwner(),
'protected' => true,
];
if ($this->addressBookInfo['{http://owncloud.org/ns}read-only']) {
if ($this->canWrite()) {
$acl[] = [
'privilege' => '{DAV:}write',
'principal' => $owner,
'principal' => parent::getOwner(),
'protected' => true,
];
}
}
/** @var CardDavBackend $carddavBackend */
$carddavBackend = $this->carddavBackend;
return $carddavBackend->applyShareAcl($this->getResourceId(), $acl);
}
function getChildACL() {
$acl = parent::getChildACL();
if ($this->getOwner() === 'principals/system/system') {
$acl[] = [
'privilege' => '{DAV:}read',
@ -116,12 +109,19 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
return $carddavBackend->applyShareAcl($this->getResourceId(), $acl, parent::getOwner());
}
function getChildACL() {
return $this->getACL();
}
function getChild($name) {
$obj = $this->carddavBackend->getCard($this->getResourceId(), $name);
$obj = $this->carddavBackend->getCard($this->addressBookInfo['id'], $name);
if (!$obj) {
throw new NotFound('Card not found');
}
$obj['acl'] = $this->getChildACL();
return new Card($this->carddavBackend, $this->addressBookInfo, $obj);
}
/**
@ -172,4 +172,11 @@ class AddressBook extends \Sabre\CardDAV\AddressBook implements IShareable {
return $cardDavBackend->collectCardProperties($this->getResourceId(), 'CATEGORIES');
}
private function canWrite() {
if (isset($this->addressBookInfo['{http://owncloud.org/ns}read-only'])) {
return !$this->addressBookInfo['{http://owncloud.org/ns}read-only'];
}
return true;
}
}

View File

@ -1,45 +0,0 @@
<?php
/**
* @author Thomas Müller <thomas.mueller@tmit.eu>
*
* @copyright Copyright (c) 2016, ownCloud, Inc.
* @license AGPL-3.0
*
* This code is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License, version 3,
* as published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License, version 3,
* along with this program. If not, see <http://www.gnu.org/licenses/>
*
*/
namespace OCA\DAV\CardDAV;
class Card extends \Sabre\CardDAV\Card {
function getACL() {
$acl = parent::getACL();
if ($this->getOwner() === 'principals/system/system') {
$acl[] = [
'privilege' => '{DAV:}read',
'principal' => '{DAV:}authenticated',
'protected' => true,
];
}
/** @var CardDavBackend $carddavBackend */
$carddavBackend = $this->carddavBackend;
return $carddavBackend->applyShareAcl($this->getBookId(), $acl);
}
private function getBookId() {
return $this->addressBookInfo['id'];
}
}

View File

@ -64,10 +64,6 @@ class CardDavBackend implements BackendInterface, SyncSupport {
'BDAY', 'UID', 'N', 'FN', 'TITLE', 'ROLE', 'NOTE', 'NICKNAME',
'ORG', 'CATEGORIES', 'EMAIL', 'TEL', 'IMPP', 'ADR', 'URL', 'GEO', 'CLOUD');
const ACCESS_OWNER = 1;
const ACCESS_READ_WRITE = 2;
const ACCESS_READ = 3;
/** @var EventDispatcherInterface */
private $dispatcher;
@ -157,7 +153,7 @@ class CardDavBackend implements BackendInterface, SyncSupport {
'{http://calendarserver.org/ns/}getctag' => $row['synctoken'],
'{http://sabredav.org/ns}sync-token' => $row['synctoken']?$row['synctoken']:'0',
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}owner-principal' => $row['principaluri'],
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only' => $row['access'] === self::ACCESS_READ,
'{' . \OCA\DAV\DAV\Sharing\Plugin::NS_OWNCLOUD . '}read-only' => (int)$row['access'] === Backend::ACCESS_READ,
];
}
}

View File

@ -132,6 +132,7 @@ class Principal implements BackendInterface {
* Returns the list of groups a principal is a member of
*
* @param string $principal
* @param bool $needGroups
* @return array
* @throws Exception
*/

View File

@ -103,4 +103,64 @@ class CalendarTest extends TestCase {
$this->assertTrue(true);
}
}
/**
* @dataProvider providesReadOnlyInfo
*/
public function testAcl($expectsWrite, $readOnlyValue, $hasOwnerSet) {
/** @var \PHPUnit_Framework_MockObject_MockObject | CalDavBackend $backend */
$backend = $this->getMockBuilder('OCA\DAV\CalDAV\CalDavBackend')->disableOriginalConstructor()->getMock();
$backend->expects($this->any())->method('applyShareAcl')->willReturnArgument(1);
$calendarInfo = [
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default'
];
if (!is_null($readOnlyValue)) {
$calendarInfo['{http://owncloud.org/ns}read-only'] = $readOnlyValue;
}
if ($hasOwnerSet) {
$calendarInfo['{http://owncloud.org/ns}owner-principal'] = 'user1';
}
$c = new Calendar($backend, $calendarInfo);
$acl = $c->getACL();
$childAcl = $c->getChildACL();
$expectedAcl = [[
'privilege' => '{DAV:}read',
'principal' => $hasOwnerSet ? 'user1' : 'user2',
'protected' => true
], [
'privilege' => '{DAV:}write',
'principal' => $hasOwnerSet ? 'user1' : 'user2',
'protected' => true
]];
if ($hasOwnerSet) {
$expectedAcl[] = [
'privilege' => '{DAV:}read',
'principal' => 'user2',
'protected' => true
];
if ($expectsWrite) {
$expectedAcl[] = [
'privilege' => '{DAV:}write',
'principal' => 'user2',
'protected' => true
];
}
}
$this->assertEquals($expectedAcl, $acl);
$this->assertEquals($expectedAcl, $childAcl);
}
public function providesReadOnlyInfo() {
return [
'read-only property not set' => [true, null, true],
'read-only property is false' => [true, false, true],
'read-only property is true' => [false, true, true],
'read-only property not set and no owner' => [true, null, false],
'read-only property is false and no owner' => [true, false, false],
'read-only property is true and no owner' => [false, true, false],
];
}
}

View File

@ -32,7 +32,7 @@ class AddressBookTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject | CardDavBackend $backend */
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$backend->expects($this->once())->method('updateShares');
$backend->method('getShares')->willReturn([
$backend->expects($this->any())->method('getShares')->willReturn([
['href' => 'principal:user2']
]);
$calendarInfo = [
@ -51,7 +51,7 @@ class AddressBookTest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject | CardDavBackend $backend */
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$backend->expects($this->never())->method('updateShares');
$backend->method('getShares')->willReturn([
$backend->expects($this->any())->method('getShares')->willReturn([
['href' => 'principal:group2']
]);
$calendarInfo = [
@ -77,4 +77,63 @@ class AddressBookTest extends TestCase {
$c = new AddressBook($backend, $calendarInfo);
$c->propPatch(new PropPatch([]));
}
}
/**
* @dataProvider providesReadOnlyInfo
*/
public function testAcl($expectsWrite, $readOnlyValue, $hasOwnerSet) {
/** @var \PHPUnit_Framework_MockObject_MockObject | CardDavBackend $backend */
$backend = $this->getMockBuilder('OCA\DAV\CardDAV\CardDavBackend')->disableOriginalConstructor()->getMock();
$backend->expects($this->any())->method('applyShareAcl')->willReturnArgument(1);
$calendarInfo = [
'principaluri' => 'user2',
'id' => 666,
'uri' => 'default'
];
if (!is_null($readOnlyValue)) {
$calendarInfo['{http://owncloud.org/ns}read-only'] = $readOnlyValue;
}
if ($hasOwnerSet) {
$calendarInfo['{http://owncloud.org/ns}owner-principal'] = 'user1';
}
$c = new AddressBook($backend, $calendarInfo);
$acl = $c->getACL();
$childAcl = $c->getChildACL();
$expectedAcl = [[
'privilege' => '{DAV:}read',
'principal' => $hasOwnerSet ? 'user1' : 'user2',
'protected' => true
], [
'privilege' => '{DAV:}write',
'principal' => $hasOwnerSet ? 'user1' : 'user2',
'protected' => true
]];
if ($hasOwnerSet) {
$expectedAcl[] = [
'privilege' => '{DAV:}read',
'principal' => 'user2',
'protected' => true
];
if ($expectsWrite) {
$expectedAcl[] = [
'privilege' => '{DAV:}write',
'principal' => 'user2',
'protected' => true
];
}
}
$this->assertEquals($expectedAcl, $acl);
$this->assertEquals($expectedAcl, $childAcl);
}
public function providesReadOnlyInfo() {
return [
'read-only property not set' => [true, null, true],
'read-only property is false' => [true, false, true],
'read-only property is true' => [false, true, true],
'read-only property not set and no owner' => [true, null, false],
'read-only property is false and no owner' => [true, false, false],
'read-only property is true and no owner' => [false, true, false],
];
}}