From 26532e0f4cd68bc248ad557df9064203278fb3e7 Mon Sep 17 00:00:00 2001 From: Georg Ehrke Date: Sun, 14 Oct 2018 12:48:42 +0200 Subject: [PATCH] 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 --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/CalDAV/CalendarHome.php | 5 +- apps/dav/lib/CalDAV/Outbox.php | 132 +++++++++++++++++ apps/dav/lib/Connector/Sabre/Principal.php | 10 +- apps/dav/tests/unit/CalDAV/OutboxTest.php | 123 ++++++++++++++++ .../unit/Connector/Sabre/PrincipalTest.php | 139 ++++++------------ 7 files changed, 304 insertions(+), 107 deletions(-) create mode 100644 apps/dav/lib/CalDAV/Outbox.php create mode 100644 apps/dav/tests/unit/CalDAV/OutboxTest.php diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 8b266c156f337..b03e866ab543e 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -35,6 +35,7 @@ 'OCA\\DAV\\CalDAV\\CalendarObject' => $baseDir . '/../lib/CalDAV/CalendarObject.php', 'OCA\\DAV\\CalDAV\\CalendarRoot' => $baseDir . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\InvitationResponse\\InvitationResponseServer' => $baseDir . '/../lib/CalDAV/InvitationResponse/InvitationResponseServer.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', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 09eb4d257ccc4..0d4c880138265 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -50,6 +50,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\CalDAV\\CalendarObject' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarObject.php', 'OCA\\DAV\\CalDAV\\CalendarRoot' => __DIR__ . '/..' . '/../lib/CalDAV/CalendarRoot.php', 'OCA\\DAV\\CalDAV\\InvitationResponse\\InvitationResponseServer' => __DIR__ . '/..' . '/../lib/CalDAV/InvitationResponse/InvitationResponseServer.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', diff --git a/apps/dav/lib/CalDAV/CalendarHome.php b/apps/dav/lib/CalDAV/CalendarHome.php index 3e645db459fae..6700b1b249638 100644 --- a/apps/dav/lib/CalDAV/CalendarHome.php +++ b/apps/dav/lib/CalDAV/CalendarHome.php @@ -29,7 +29,6 @@ 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 @@ function getChildren() { 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 @@ function getChild($name) { 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']); diff --git a/apps/dav/lib/CalDAV/Outbox.php b/apps/dav/lib/CalDAV/Outbox.php new file mode 100644 index 0000000000000..a334c083c4522 --- /dev/null +++ b/apps/dav/lib/CalDAV/Outbox.php @@ -0,0 +1,132 @@ + + * + * @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 + * + */ +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, string $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, + ], + ]); + } +} \ No newline at end of file diff --git a/apps/dav/lib/Connector/Sabre/Principal.php b/apps/dav/lib/Connector/Sabre/Principal.php index feba4d04624da..cbfa1bc83e05e 100644 --- a/apps/dav/lib/Connector/Sabre/Principal.php +++ b/apps/dav/lib/Connector/Sabre/Principal.php @@ -212,10 +212,9 @@ function updatePrincipal($path, PropPatch $propPatch) { 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 []; } @@ -298,10 +297,9 @@ function searchPrincipals($prefixPath, array $searchProperties, $test = 'allof') * @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; } diff --git a/apps/dav/tests/unit/CalDAV/OutboxTest.php b/apps/dav/tests/unit/CalDAV/OutboxTest.php new file mode 100644 index 0000000000000..429ecd705c6b9 --- /dev/null +++ b/apps/dav/tests/unit/CalDAV/OutboxTest.php @@ -0,0 +1,123 @@ + + * + * @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 + * + */ + +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()); + } +} \ No newline at end of file diff --git a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php index 7e82c4467607c..01907c858365d 100644 --- a/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/PrincipalTest.php @@ -283,37 +283,26 @@ public function testSearchPrincipalsWithWrongPrefixPath() { /** * @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 ($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()); + + 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)); + + $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 @@ public function testSearchPrincipals($disableFreeBusy, $sharingEnabled, $disable ->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 @@ public function testFindByUriSharingApiDisabled() { $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 @@ public function testFindByUriSharingApiDisabled() { /** * @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 @@ public function testFindByUriWithGroupRestriction($disableFreeBusy, $uri, $email ->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 @@ public function testFindByUriWithoutGroupRestriction($disableFreeBusy, $uri, $em ->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'], ]; } }