move disableFreeBusy check from User principal backend to Scheduling Outbox collection. This allows to keep local delivery of scheduling messages while prohibiting FreeBusy requests

Signed-off-by: Georg Ehrke <developer@georgehrke.com>
2018-10-14 12:48:42 +02:00 · 2018-10-14 12:48:42 +02:00 · aa94064cf5
parent e3a2b9e3e6
commit aa94064cf5
8 changed files with 302 additions and 105 deletions
--- a/apps/dav/composer/composer/ClassLoader.php
+++ b/apps/dav/composer/composer/ClassLoader.php
@ -377,7 +377,7 @@ class ClassLoader
            $subPath = $class;
            while (false !== $lastPos = strrpos($subPath, '\\')) {
                $subPath = substr($subPath, 0, $lastPos);
-                $search = $subPath.'\\';
+                $search = $subPath . '\\';
                if (isset($this->prefixDirsPsr4[$search])) {
                    $pathEnd = DIRECTORY_SEPARATOR . substr($logicalPathPsr4, $lastPos + 1);
                    foreach ($this->prefixDirsPsr4[$search] as $dir) {
--- a/apps/dav/composer/composer/autoload_classmap.php
+++ b/apps/dav/composer/composer/autoload_classmap.php
@ -31,6 +31,7 @@ return array(
    'OCA\\DAV\\CalDAV\\CalendarManager' => $baseDir . '/../lib/CalDAV/CalendarManager.php',
    'OCA\\DAV\\CalDAV\\CalendarObject' => $baseDir . '/../lib/CalDAV/CalendarObject.php',
    'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php',
+    'OCA\\DAV\\CalDAV\\Outbox' => $baseDir . '/../lib/CalDAV/Outbox.php',
    'OCA\\DAV\\CalDAV\\Plugin' => $baseDir . '/../lib/CalDAV/Plugin.php',
    'OCA\\DAV\\CalDAV\\Principal\\Collection' => $baseDir . '/../lib/CalDAV/Principal/Collection.php',
    'OCA\\DAV\\CalDAV\\Principal\\User' => $baseDir . '/../lib/CalDAV/Principal/User.php',
--- a/apps/dav/composer/composer/autoload_static.php
+++ b/apps/dav/composer/composer/autoload_static.php
@ -46,6 +46,7 @@ class ComposerStaticInitDAV
        'OCA\\DAV\\CalDAV\\CalendarManager' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarManager.php',
        'OCA\\DAV\\CalDAV\\CalendarObject' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObject.php',
        'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php',
+        'OCA\\DAV\\CalDAV\\Outbox' => __DIR__ . '/..' . '/../lib/CalDAV/Outbox.php',
        'OCA\\DAV\\CalDAV\\Plugin' => __DIR__ . '/..' . '/../lib/CalDAV/Plugin.php',
        'OCA\\DAV\\CalDAV\\Principal\\Collection' => __DIR__ . '/..' . '/../lib/CalDAV/Principal/Collection.php',
        'OCA\\DAV\\CalDAV\\Principal\\User' => __DIR__ . '/..' . '/../lib/CalDAV/Principal/User.php',
--- a/apps/dav/lib/CalDAV/CalendarHome.php
+++ b/apps/dav/lib/CalDAV/CalendarHome.php
@ -29,7 +29,6 @@ use Sabre\CalDAV\Backend\NotificationSupport;
 use Sabre\CalDAV\Backend\SchedulingSupport;
 use Sabre\CalDAV\Backend\SubscriptionSupport;
 use Sabre\CalDAV\Schedule\Inbox;
-use Sabre\CalDAV\Schedule\Outbox;
 use Sabre\CalDAV\Subscriptions\Subscription;
 use Sabre\DAV\Exception\NotFound;
 use Sabre\DAV\Exception\MethodNotAllowed;
@ -81,7 +80,7 @@ class CalendarHome extends \Sabre\CalDAV\CalendarHome {

 		if ($this->caldavBackend instanceof SchedulingSupport) {
 			$objects[] = new Inbox($this->caldavBackend, $this->principalInfo['uri']);
-			$objects[] = new Outbox($this->principalInfo['uri']);
+			$objects[] = new Outbox($this->config, $this->principalInfo['uri']);
 		}

 		// We're adding a notifications node, if it's supported by the backend.
@ -108,7 +107,7 @@ class CalendarHome extends \Sabre\CalDAV\CalendarHome {
 			return new Inbox($this->caldavBackend, $this->principalInfo['uri']);
 		}
 		if ($name === 'outbox' && $this->caldavBackend instanceof SchedulingSupport) {
-			return new Outbox($this->principalInfo['uri']);
+			return new Outbox($this->config, $this->principalInfo['uri']);
 		}
 		if ($name === 'notifications' && $this->caldavBackend instanceof NotificationSupport) {
 			return new \Sabre\CalDAv\Notifications\Collection($this->caldavBackend, $this->principalInfo['uri']);
--- a/apps/dav/lib/CalDAV/Outbox.php
+++ b/apps/dav/lib/CalDAV/Outbox.php
@ -0,0 +1,132 @@
+<?php
+/**
+ * @copyright Copyright (c) 2018, Georg Ehrke
+ *
+ * @author Georg Ehrke <oc.list@georgehrke.com>
+ *
+ * @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\CalDAV;
+
+use OCP\IConfig;
+use Sabre\CalDAV\Plugin as CalDAVPlugin;
+
+/**
+ * Class Outbox
+ *
+ * @package OCA\DAV\CalDAV
+ */
+class Outbox extends \Sabre\CalDAV\Schedule\Outbox {
+
+	/** @var IConfig */
+	private $config;
+
+	/** @var null|bool */
+	private $disableFreeBusy = null;
+
+	/**
+	 * Outbox constructor.
+	 *
+	 * @param IConfig $config
+	 * @param string $principalUri
+	 */
+	public function __construct(IConfig $config, $principalUri) {
+		parent::__construct($principalUri);
+		$this->config = $config;
+	}
+
+	/**
+	 * Returns a list of ACE's for this node.
+	 *
+	 * Each ACE has the following properties:
+	 *   * 'privilege', a string such as {DAV:}read or {DAV:}write. These are
+	 *     currently the only supported privileges
+	 *   * 'principal', a url to the principal who owns the node
+	 *   * 'protected' (optional), indicating that this ACE is not allowed to
+	 *      be updated.
+	 *
+	 * @return array
+	 */
+	function getACL() {
+		// getACL is called so frequently that we cache the config result
+		if ($this->disableFreeBusy === null) {
+			$this->disableFreeBusy = ($this->config->getAppValue('dav', 'disableFreeBusy', 'no') === 'yes');
+		}
+
+		$commonAcl = [
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => $this->getOwner(),
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => $this->getOwner() . '/calendar-proxy-read',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => $this->getOwner() . '/calendar-proxy-write',
+				'protected' => true,
+			],
+		];
+
+		// schedule-send is an aggregate privilege for:
+		// - schedule-send-invite
+		// - schedule-send-reply
+		// - schedule-send-freebusy
+		//
+		// If FreeBusy is disabled, we have to remove the latter privilege
+
+		if ($this->disableFreeBusy) {
+			return array_merge($commonAcl, [
+				[
+					'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send-invite',
+					'principal' => $this->getOwner(),
+					'protected' => true,
+				],
+				[
+					'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send-invite',
+					'principal' => $this->getOwner() . '/calendar-proxy-write',
+					'protected' => true,
+				],
+				[
+					'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send-reply',
+					'principal' => $this->getOwner(),
+					'protected' => true,
+				],
+				[
+					'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send-reply',
+					'principal' => $this->getOwner() . '/calendar-proxy-write',
+					'protected' => true,
+				],
+			]);
+		}
+
+		return array_merge($commonAcl, [
+			[
+				'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send',
+				'principal' => $this->getOwner(),
+				'protected' => true,
+			],
+			[
+				'privilege' => '{' . CalDAVPlugin::NS_CALDAV . '}schedule-send',
+				'principal' => $this->getOwner() . '/calendar-proxy-write',
+				'protected' => true,
+			],
+		]);
+	}
+}
--- a/apps/dav/lib/Connector/Sabre/Principal.php
+++ b/apps/dav/lib/Connector/Sabre/Principal.php
@ -213,10 +213,9 @@ class Principal implements BackendInterface {
 	protected function searchUserPrincipals(array $searchProperties, $test = 'allof') {
 		$results = [];

-		// If sharing is disabled (or FreeBusy was disabled on purpose), return the empty array
+		// If sharing is disabled, return the empty array
 		$shareAPIEnabled = $this->shareManager->shareApiEnabled();
-		$disableFreeBusy = $this->config->getAppValue('dav', 'disableFreeBusy', $shareAPIEnabled ? 'no' : 'yes');
-		if ($disableFreeBusy === 'yes') {
+		if (!$shareAPIEnabled) {
 			return [];
 		}

@ -299,10 +298,9 @@ class Principal implements BackendInterface {
 	 * @return string
 	 */
 	function findByUri($uri, $principalPrefix) {
-		// If sharing is disabled (or FreeBusy was disabled on purpose), return the empty array
+		// If sharing is disabled, return the empty array
 		$shareAPIEnabled = $this->shareManager->shareApiEnabled();
-		$disableFreeBusy = $this->config->getAppValue('dav', 'disableFreeBusy', $shareAPIEnabled ? 'no' : 'yes');
-		if ($disableFreeBusy === 'yes') {
+		if (!$shareAPIEnabled) {
 			return null;
 		}

--- a/apps/dav/tests/unit/CalDAV/OutboxTest.php
+++ b/apps/dav/tests/unit/CalDAV/OutboxTest.php
@ -0,0 +1,123 @@
+<?php
+/**
+ * @copyright Copyright (c) 2018, Georg Ehrke
+ *
+ * @author Georg Ehrke <oc.list@georgehrke.com>
+ *
+ * @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\Tests\unit\CalDAV;
+
+use OCA\DAV\CalDAV\Outbox;
+use OCP\IConfig;
+use Test\TestCase;
+
+class OutboxTest extends TestCase {
+
+	/** @var IConfig */
+	private $config;
+
+	/** @var Outbox */
+	private $outbox;
+
+	protected function setUp() {
+		parent::setUp();
+
+		$this->config = $this->createMock(IConfig::class);
+		$this->outbox = new Outbox($this->config, 'user-principal-123');
+	}
+
+	public function testGetACLFreeBusyEnabled() {
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('dav', 'disableFreeBusy', 'no')
+			->will($this->returnValue('no'));
+
+		$this->assertEquals([
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123/calendar-proxy-read',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123/calendar-proxy-write',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send',
+				'principal' => 'user-principal-123',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send',
+				'principal' => 'user-principal-123/calendar-proxy-write',
+				'protected' => true,
+			],
+		], $this->outbox->getACL());
+	}
+
+	public function testGetACLFreeBusyDisabled() {
+		$this->config->expects($this->once())
+			->method('getAppValue')
+			->with('dav', 'disableFreeBusy', 'no')
+			->will($this->returnValue('yes'));
+
+		$this->assertEquals([
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123/calendar-proxy-read',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{DAV:}read',
+				'principal' => 'user-principal-123/calendar-proxy-write',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send-invite',
+				'principal' => 'user-principal-123',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send-invite',
+				'principal' => 'user-principal-123/calendar-proxy-write',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send-reply',
+				'principal' => 'user-principal-123',
+				'protected' => true,
+			],
+			[
+				'privilege' => '{urn:ietf:params:xml:ns:caldav}schedule-send-reply',
+				'principal' => 'user-principal-123/calendar-proxy-write',
+				'protected' => true,
+			],
+		], $this->outbox->getACL());
+	}
+}
--- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php
@ -283,37 +283,26 @@ class PrincipalTest extends TestCase {
 	/**
 	 * @dataProvider searchPrincipalsDataProvider
 	 */
-	public function testSearchPrincipals($disableFreeBusy, $sharingEnabled, $disableFBSharingCombination, $groupsOnly, $result) {
+	public function testSearchPrincipals($sharingEnabled, $groupsOnly, $result) {
 		$this->shareManager->expects($this->once())
 			->method('shareAPIEnabled')
 			->will($this->returnValue($sharingEnabled));
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('dav', 'disableFreeBusy', $sharingEnabled ? 'no' : 'yes')
-			->will($this->returnValue($disableFBSharingCombination));

-		if ($disableFreeBusy === 'no') {
-			if ($sharingEnabled) {
-				$this->shareManager->expects($this->once())
-					->method('shareWithGroupMembersOnly')
-					->will($this->returnValue($groupsOnly));
+		if ($sharingEnabled) {
+			$this->shareManager->expects($this->once())
+				->method('shareWithGroupMembersOnly')
+				->will($this->returnValue($groupsOnly));

-				if ($groupsOnly) {
-					$user = $this->createMock(IUser::class);
-					$this->userSession->expects($this->once())
-						->method('getUser')
-						->will($this->returnValue($user));
+			if ($groupsOnly) {
+				$user = $this->createMock(IUser::class);
+				$this->userSession->expects($this->once())
+					->method('getUser')
+					->will($this->returnValue($user));

-					$this->groupManager->expects($this->at(0))
-						->method('getUserGroupIds')
-						->with($user)
-						->will($this->returnValue(['group1', 'group2']));
-				}
-			} else {
-				$this->shareManager->expects($this->never())
-					->method('shareWithGroupMembersOnly');
-				$this->groupManager->expects($this->never())
-					->method($this->anything());
+				$this->groupManager->expects($this->at(0))
+					->method('getUserGroupIds')
+					->with($user)
+					->will($this->returnValue(['group1', 'group2']));
 			}
 		} else {
 			$this->shareManager->expects($this->never())
@ -322,57 +311,42 @@ class PrincipalTest extends TestCase {
 				->method($this->anything());
 		}

-
 		$user2 = $this->createMock(IUser::class);
 		$user2->method('getUID')->will($this->returnValue('user2'));
 		$user3 = $this->createMock(IUser::class);
 		$user3->method('getUID')->will($this->returnValue('user3'));

-		if ($disableFreeBusy === 'no') {
-			if ($sharingEnabled) {
-				$this->userManager->expects($this->at(0))
-					->method('getByEmail')
-					->with('user')
-					->will($this->returnValue([$user2, $user3]));
-			} else {
-				$this->userManager->expects($this->never())
-					->method('getByEmail');
-			}
+		if ($sharingEnabled) {
+			$this->userManager->expects($this->at(0))
+				->method('getByEmail')
+				->with('user')
+				->will($this->returnValue([$user2, $user3]));
 		} else {
 			$this->userManager->expects($this->never())
 				->method('getByEmail');
 		}

-		if ($disableFreeBusy === 'no') {
-			if ($sharingEnabled && $groupsOnly) {
-				$this->groupManager->expects($this->at(1))
-					->method('getUserGroupIds')
-					->with($user2)
-					->will($this->returnValue(['group1', 'group3']));
-				$this->groupManager->expects($this->at(2))
-					->method('getUserGroupIds')
-					->with($user3)
-					->will($this->returnValue(['group3', 'group4']));
-			}
-		} else {
-			$this->groupManager->expects($this->never())
-				->method('getUserGroupIds');
-			$this->groupManager->expects($this->never())
-				->method('getUserGroupIds');
+		if ($sharingEnabled && $groupsOnly) {
+			$this->groupManager->expects($this->at(1))
+				->method('getUserGroupIds')
+				->with($user2)
+				->will($this->returnValue(['group1', 'group3']));
+			$this->groupManager->expects($this->at(2))
+				->method('getUserGroupIds')
+				->with($user3)
+				->will($this->returnValue(['group3', 'group4']));
 		}

+
 		$this->assertEquals($result, $this->connector->searchPrincipals('principals/users',
 			['{http://sabredav.org/ns}email-address' => 'user']));
 	}

 	public function searchPrincipalsDataProvider() {
 		return [
-			['yes', true, 'yes', false, []],
-			['no', true, 'no', false, ['principals/users/user2', 'principals/users/user3']],
-			['yes', true, 'yes', true, []],
-			['no', true, 'no', true, ['principals/users/user2']],
-			['yes', false, 'yes', false, []],
-			['no', false, 'yes', false, []],
+			[true, false, ['principals/users/user2', 'principals/users/user3']],
+			[true, true, ['principals/users/user2']],
+			[false, false, []],
 		];
 	}

@ -380,10 +354,6 @@ class PrincipalTest extends TestCase {
 		$this->shareManager->expects($this->once())
 			->method('shareApiEnabled')
 			->will($this->returnValue(false));
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('dav', 'disableFreeBusy', 'yes')
-			->will($this->returnValue('yes'));

 		$this->assertEquals(null, $this->connector->findByUri('mailto:user@foo.com', 'principals/users'));
 	}
@ -391,21 +361,11 @@ class PrincipalTest extends TestCase {
 	/**
 	 * @dataProvider findByUriWithGroupRestrictionDataProvider
 	 */
-	public function testFindByUriWithGroupRestriction($disableFreeBusy, $uri, $email, $expects) {
+	public function testFindByUriWithGroupRestriction($uri, $email, $expects) {
 		$this->shareManager->expects($this->once())
 			->method('shareApiEnabled')
 			->will($this->returnValue(true));
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('dav', 'disableFreeBusy', 'no')
-			->will($this->returnValue($disableFreeBusy));

-		if ($disableFreeBusy === 'yes') {
-			$this->shareManager->expects($this->never())
-				->method('shareWithGroupMembersOnly');
-			$this->userSession->expects($this->never())
-				->method('getUser');
-		} else {
 			$this->shareManager->expects($this->once())
 				->method('shareWithGroupMembersOnly')
 				->will($this->returnValue(true));
@ -441,39 +401,25 @@ class PrincipalTest extends TestCase {
 					->with($user3)
 					->will($this->returnValue(['group3', 'group3']));
 			}
-		}

 		$this->assertEquals($expects, $this->connector->findByUri($uri, 'principals/users'));
 	}

 	public function findByUriWithGroupRestrictionDataProvider() {
 		return [
-			['yes', 'mailto:user2@foo.bar', 'user2@foo.bar', null],
-			['no', 'mailto:user2@foo.bar', 'user2@foo.bar', 'principals/users/user2'],
-			['yes', 'mailto:user3@foo.bar', 'user3@foo.bar', null],
-			['no', 'mailto:user3@foo.bar', 'user3@foo.bar', null],
+			['mailto:user2@foo.bar', 'user2@foo.bar', 'principals/users/user2'],
+			['mailto:user3@foo.bar', 'user3@foo.bar', null],
 		];
 	}

 	/**
 	 * @dataProvider findByUriWithoutGroupRestrictionDataProvider
 	 */
-	public function testFindByUriWithoutGroupRestriction($disableFreeBusy, $uri, $email, $expects) {
+	public function testFindByUriWithoutGroupRestriction($uri, $email, $expects) {
 		$this->shareManager->expects($this->once())
 			->method('shareApiEnabled')
 			->will($this->returnValue(true));
-		$this->config->expects($this->once())
-			->method('getAppValue')
-			->with('dav', 'disableFreeBusy', 'no')
-			->will($this->returnValue($disableFreeBusy));

-		if ($disableFreeBusy === 'yes') {
-			$this->shareManager->expects($this->never())
-				->method('shareWithGroupMembersOnly');
-
-			$this->userManager->expects($this->never())
-				->method('getByEmail');
-		} else {
 			$this->shareManager->expects($this->once())
 				->method('shareWithGroupMembersOnly')
 				->will($this->returnValue(false));
@ -487,17 +433,14 @@ class PrincipalTest extends TestCase {
 				->method('getByEmail')
 				->with($email)
 				->will($this->returnValue([$email === 'user2@foo.bar' ? $user2 : $user3]));
-		}

 		$this->assertEquals($expects, $this->connector->findByUri($uri, 'principals/users'));
 	}

 	public function findByUriWithoutGroupRestrictionDataProvider() {
 		return [
-			['yes', 'mailto:user2@foo.bar', 'user2@foo.bar', null],
-			['yes', 'mailto:user3@foo.bar', 'user3@foo.bar', null],
-			['no', 'mailto:user2@foo.bar', 'user2@foo.bar', 'principals/users/user2'],
-			['no', 'mailto:user3@foo.bar', 'user3@foo.bar', 'principals/users/user3'],
+			['mailto:user2@foo.bar', 'user2@foo.bar', 'principals/users/user2'],
+			['mailto:user3@foo.bar', 'user3@foo.bar', 'principals/users/user3'],
 		];
 	}
 }