diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php index bd9de775f4..b005ec2fcb 100644 --- a/apps/dav/appinfo/v1/caldav.php +++ b/apps/dav/appinfo/v1/caldav.php @@ -49,7 +49,7 @@ $db = \OC::$server->getDatabaseConnection(); $userManager = \OC::$server->getUserManager(); $random = \OC::$server->getSecureRandom(); $dispatcher = \OC::$server->getEventDispatcher(); -$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, $random, $dispatcher, true); +$calDavBackend = new CalDavBackend($db, $principalBackend, $userManager, \OC::$server->getGroupManager(), $random, $dispatcher, true); $debugging = \OC::$server->getConfig()->getSystemValue('debug', false); diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php index 8dea668474..74003f11c9 100644 --- a/apps/dav/appinfo/v1/carddav.php +++ b/apps/dav/appinfo/v1/carddav.php @@ -48,7 +48,7 @@ $principalBackend = new Principal( 'principals/' ); $db = \OC::$server->getDatabaseConnection(); -$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager(), \OC::$server->getEventDispatcher()); +$cardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getUserManager(), \OC::$server->getGroupManager(), \OC::$server->getEventDispatcher()); $debugging = \OC::$server->getConfig()->getSystemValue('debug', false); diff --git a/apps/dav/lib/CalDAV/CalDavBackend.php b/apps/dav/lib/CalDAV/CalDavBackend.php index 83ef06f29e..2c34f6d6d3 100644 --- a/apps/dav/lib/CalDAV/CalDavBackend.php +++ b/apps/dav/lib/CalDAV/CalDavBackend.php @@ -32,6 +32,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCA\DAV\Connector\Sabre\Principal; use OCA\DAV\DAV\Sharing\Backend; use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; use OCP\Security\ISecureRandom; @@ -158,6 +159,7 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription * @param IDBConnection $db * @param Principal $principalBackend * @param IUserManager $userManager + * @param IGroupManager $groupManager * @param ISecureRandom $random * @param EventDispatcherInterface $dispatcher * @param bool $legacyEndpoint @@ -165,13 +167,14 @@ class CalDavBackend extends AbstractBackend implements SyncSupport, Subscription public function __construct(IDBConnection $db, Principal $principalBackend, IUserManager $userManager, + IGroupManager $groupManager, ISecureRandom $random, EventDispatcherInterface $dispatcher, $legacyEndpoint = false) { $this->db = $db; $this->principalBackend = $principalBackend; $this->userManager = $userManager; - $this->sharingBackend = new Backend($this->db, $principalBackend, 'calendar'); + $this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'calendar'); $this->random = $random; $this->dispatcher = $dispatcher; $this->legacyEndpoint = $legacyEndpoint; diff --git a/apps/dav/lib/CardDAV/CardDavBackend.php b/apps/dav/lib/CardDAV/CardDavBackend.php index 2e4acad6df..7c73a2cb94 100644 --- a/apps/dav/lib/CardDAV/CardDavBackend.php +++ b/apps/dav/lib/CardDAV/CardDavBackend.php @@ -33,6 +33,7 @@ use OCP\DB\QueryBuilder\IQueryBuilder; use OCA\DAV\DAV\Sharing\Backend; use OCA\DAV\DAV\Sharing\IShareable; use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; use PDO; @@ -88,17 +89,19 @@ class CardDavBackend implements BackendInterface, SyncSupport { * @param IDBConnection $db * @param Principal $principalBackend * @param IUserManager $userManager + * @param IGroupManager $groupManager * @param EventDispatcherInterface $dispatcher */ public function __construct(IDBConnection $db, Principal $principalBackend, IUserManager $userManager, + IGroupManager $groupManager, EventDispatcherInterface $dispatcher) { $this->db = $db; $this->principalBackend = $principalBackend; $this->userManager = $userManager; $this->dispatcher = $dispatcher; - $this->sharingBackend = new Backend($this->db, $principalBackend, 'addressbook'); + $this->sharingBackend = new Backend($this->db, $this->userManager, $groupManager, $principalBackend, 'addressbook'); } /** diff --git a/apps/dav/lib/Command/CreateCalendar.php b/apps/dav/lib/Command/CreateCalendar.php index 24990352fa..adc86faa19 100644 --- a/apps/dav/lib/Command/CreateCalendar.php +++ b/apps/dav/lib/Command/CreateCalendar.php @@ -79,7 +79,7 @@ class CreateCalendar extends Command { $dispatcher = \OC::$server->getEventDispatcher(); $name = $input->getArgument('name'); - $caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $random, $dispatcher); + $caldav = new CalDavBackend($this->dbConnection, $principalBackend, $this->userManager, $this->groupManager, $random, $dispatcher); $caldav->createCalendar("principals/users/$user", $name, []); } } diff --git a/apps/dav/lib/DAV/Sharing/Backend.php b/apps/dav/lib/DAV/Sharing/Backend.php index 6cc5e3b6f5..aa4b137f2b 100644 --- a/apps/dav/lib/DAV/Sharing/Backend.php +++ b/apps/dav/lib/DAV/Sharing/Backend.php @@ -26,11 +26,17 @@ namespace OCA\DAV\DAV\Sharing; use OCA\DAV\Connector\Sabre\Principal; use OCP\IDBConnection; +use OCP\IGroupManager; +use OCP\IUserManager; class Backend { /** @var IDBConnection */ private $db; + /** @var IUserManager */ + private $userManager; + /** @var IGroupManager */ + private $groupManager; /** @var Principal */ private $principalBackend; /** @var string */ @@ -42,11 +48,15 @@ class Backend { /** * @param IDBConnection $db + * @param IUserManager $userManager + * @param IGroupManager $groupManager * @param Principal $principalBackend * @param string $resourceType */ - public function __construct(IDBConnection $db, Principal $principalBackend, $resourceType) { + public function __construct(IDBConnection $db, IUserManager $userManager, IGroupManager $groupManager, Principal $principalBackend, $resourceType) { $this->db = $db; + $this->userManager = $userManager; + $this->groupManager = $groupManager; $this->principalBackend = $principalBackend; $this->resourceType = $resourceType; } @@ -81,6 +91,18 @@ class Backend { return; } + $principal = explode('/', $parts[1], 3); + if (count($principal) !== 3 || $principal[0] !== 'principals' || !in_array($principal[1], ['users', 'groups'], true)) { + // Invalid principal + return; + } + + if (($principal[1] === 'users' && !$this->userManager->userExists($principal[2])) || + ($principal[1] === 'groups' && !$this->groupManager->groupExists($principal[2]))) { + // User or group does not exist + return; + } + // remove the share if it already exists $this->unshare($shareable, $element['href']); $access = self::ACCESS_READ; diff --git a/apps/dav/lib/RootCollection.php b/apps/dav/lib/RootCollection.php index a243ec6d00..e4ba1f2c02 100644 --- a/apps/dav/lib/RootCollection.php +++ b/apps/dav/lib/RootCollection.php @@ -41,15 +41,14 @@ class RootCollection extends SimpleCollection { $config = \OC::$server->getConfig(); $random = \OC::$server->getSecureRandom(); $userManager = \OC::$server->getUserManager(); + $groupManager = \OC::$server->getGroupManager(); $db = \OC::$server->getDatabaseConnection(); $dispatcher = \OC::$server->getEventDispatcher(); $userPrincipalBackend = new Principal( $userManager, - \OC::$server->getGroupManager() - ); - $groupPrincipalBackend = new GroupPrincipalBackend( - \OC::$server->getGroupManager() + $groupManager ); + $groupPrincipalBackend = new GroupPrincipalBackend($groupManager); // as soon as debug mode is enabled we allow listing of principals $disableListing = !$config->getSystemValue('debug', false); @@ -62,7 +61,7 @@ class RootCollection extends SimpleCollection { $systemPrincipals->disableListing = $disableListing; $filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users'); $filesCollection->disableListing = $disableListing; - $caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $random, $dispatcher); + $caldavBackend = new CalDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $random, $dispatcher); $calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users'); $calendarRoot->disableListing = $disableListing; $publicCalendarRoot = new PublicCalendarRoot($caldavBackend); @@ -71,28 +70,28 @@ class RootCollection extends SimpleCollection { $systemTagCollection = new SystemTag\SystemTagsByIdCollection( \OC::$server->getSystemTagManager(), \OC::$server->getUserSession(), - \OC::$server->getGroupManager() + $groupManager ); $systemTagRelationsCollection = new SystemTag\SystemTagsRelationsCollection( \OC::$server->getSystemTagManager(), \OC::$server->getSystemTagObjectMapper(), \OC::$server->getUserSession(), - \OC::$server->getGroupManager(), + $groupManager, \OC::$server->getEventDispatcher() ); $commentsCollection = new Comments\RootCollection( \OC::$server->getCommentsManager(), - \OC::$server->getUserManager(), + $userManager, \OC::$server->getUserSession(), \OC::$server->getEventDispatcher(), \OC::$server->getLogger() ); - $usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher); + $usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $dispatcher); $usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, 'principals/users'); $usersAddressBookRoot->disableListing = $disableListing; - $systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getUserManager(), $dispatcher); + $systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, $userManager, $groupManager, $dispatcher); $systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, 'principals/system'); $systemAddressBookRoot->disableListing = $disableListing; diff --git a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php similarity index 95% rename from apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php rename to apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php index a7bf4432c6..3f3b744e5a 100644 --- a/apps/dav/tests/unit/CalDAV/AbstractCalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/AbstractCalDavBackend.php @@ -24,6 +24,7 @@ namespace OCA\DAV\Tests\unit\CalDAV; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\Connector\Sabre\Principal; +use OCP\IGroupManager; use OCP\IUserManager; use OCP\Security\ISecureRandom; use Sabre\CalDAV\Xml\Property\SupportedCalendarComponentSet; @@ -37,7 +38,7 @@ use Test\TestCase; * * @package OCA\DAV\Tests\unit\CalDAV */ -abstract class AbstractCalDavBackendTest extends TestCase { +abstract class AbstractCalDavBackend extends TestCase { /** @var CalDavBackend */ protected $backend; @@ -46,6 +47,8 @@ abstract class AbstractCalDavBackendTest extends TestCase { protected $principal; /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ protected $userManager; + /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $groupManager; /** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $dispatcher; @@ -61,6 +64,7 @@ abstract class AbstractCalDavBackendTest extends TestCase { parent::setUp(); $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->dispatcher = $this->createMock(EventDispatcherInterface::class); $this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal') ->disableOriginalConstructor() @@ -77,7 +81,7 @@ abstract class AbstractCalDavBackendTest extends TestCase { $db = \OC::$server->getDatabaseConnection(); $this->random = \OC::$server->getSecureRandom(); - $this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->random, $this->dispatcher); + $this->backend = new CalDavBackend($db, $this->principal, $this->userManager, $this->groupManager, $this->random, $this->dispatcher); $this->cleanUpBackend(); } diff --git a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php index fa298282d7..95ea09148b 100644 --- a/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php +++ b/apps/dav/tests/unit/CalDAV/CalDavBackendTest.php @@ -42,7 +42,7 @@ use Sabre\DAVACL\IACL; * * @package OCA\DAV\Tests\unit\CalDAV */ -class CalDavBackendTest extends AbstractCalDavBackendTest { +class CalDavBackendTest extends AbstractCalDavBackend { public function testCalendarOperations() { @@ -130,6 +130,14 @@ class CalDavBackendTest extends AbstractCalDavBackendTest { return vsprintf($text, $parameters); })); + $this->userManager->expects($this->any()) + ->method('userExists') + ->willReturn(true); + + $this->userManager->expects($this->any()) + ->method('groupExists') + ->willReturn(true); + $calendarId = $this->createTestCalendar(); $calendars = $this->backend->getCalendarsForUser(self::UNIT_TEST_USER); $this->assertCount(1, $calendars); diff --git a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php index eb9f9cd10d..ec767f3dbd 100644 --- a/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php +++ b/apps/dav/tests/unit/CalDAV/PublicCalendarRootTest.php @@ -5,6 +5,7 @@ namespace OCA\DAV\Tests\unit\CalDAV; use OCA\DAV\CalDAV\Calendar; use OCA\DAV\CalDAV\PublicCalendar; use OCA\DAV\Connector\Sabre\Principal; +use OCP\IGroupManager; use OCP\IL10N; use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CalDAV\PublicCalendarRoot; @@ -33,6 +34,8 @@ class PublicCalendarRootTest extends TestCase { private $principal; /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ protected $userManager; + /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $groupManager; /** @var ISecureRandom */ private $random; @@ -43,6 +46,7 @@ class PublicCalendarRootTest extends TestCase { $db = \OC::$server->getDatabaseConnection(); $this->principal = $this->createMock('OCA\DAV\Connector\Sabre\Principal'); $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->random = \OC::$server->getSecureRandom(); $dispatcher = $this->createMock(EventDispatcherInterface::class); @@ -54,6 +58,7 @@ class PublicCalendarRootTest extends TestCase { $db, $this->principal, $this->userManager, + $this->groupManager, $this->random, $dispatcher ); diff --git a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php index 8ca7e8a33b..992445392d 100644 --- a/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php +++ b/apps/dav/tests/unit/CardDAV/CardDavBackendTest.php @@ -33,6 +33,7 @@ use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\Connector\Sabre\Principal; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; +use OCP\IGroupManager; use OCP\IL10N; use OCP\IUserManager; use Sabre\DAV\PropPatch; @@ -60,6 +61,9 @@ class CardDavBackendTest extends TestCase { /** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */ private $userManager; + /** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */ + private $groupManager; + /** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */ private $dispatcher; @@ -80,6 +84,7 @@ class CardDavBackendTest extends TestCase { parent::setUp(); $this->userManager = $this->createMock(IUserManager::class); + $this->groupManager = $this->createMock(IGroupManager::class); $this->principal = $this->getMockBuilder('OCA\DAV\Connector\Sabre\Principal') ->disableOriginalConstructor() ->setMethods(['getPrincipalByPath', 'getGroupMembership']) @@ -96,7 +101,7 @@ class CardDavBackendTest extends TestCase { $this->db = \OC::$server->getDatabaseConnection(); - $this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, $this->dispatcher); + $this->backend = new CardDavBackend($this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher); // start every test with a empty cards_properties and cards table $query = $this->db->getQueryBuilder(); $query->delete('cards_properties')->execute(); @@ -154,6 +159,14 @@ class CardDavBackendTest extends TestCase { public function testAddressBookSharing() { + $this->userManager->expects($this->any()) + ->method('userExists') + ->willReturn(true); + + $this->groupManager->expects($this->any()) + ->method('groupExists') + ->willReturn(true); + $this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []); $books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER); $this->assertEquals(1, count($books)); @@ -180,7 +193,7 @@ class CardDavBackendTest extends TestCase { /** @var CardDavBackend | \PHPUnit_Framework_MockObject_MockObject $backend */ $backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher]) ->setMethods(['updateProperties', 'purgeProperties'])->getMock(); // create a new address book @@ -253,7 +266,7 @@ class CardDavBackendTest extends TestCase { public function testMultiCard() { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher]) ->setMethods(['updateProperties'])->getMock(); // create a new address book @@ -299,7 +312,7 @@ class CardDavBackendTest extends TestCase { public function testDeleteWithoutCard() { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher]) ->setMethods([ 'getCardId', 'addChange', @@ -339,7 +352,7 @@ class CardDavBackendTest extends TestCase { public function testSyncSupport() { $this->backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher]) ->setMethods(['updateProperties'])->getMock(); // create a new address book @@ -362,32 +375,41 @@ class CardDavBackendTest extends TestCase { } public function testSharing() { + + $this->userManager->expects($this->any()) + ->method('userExists') + ->willReturn(true); + + $this->groupManager->expects($this->any()) + ->method('groupExists') + ->willReturn(true); + $this->backend->createAddressBook(self::UNIT_TEST_USER, 'Example', []); $books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER); $this->assertEquals(1, count($books)); $l = $this->createMock(IL10N::class); $exampleBook = new AddressBook($this->backend, $books[0], $l); - $this->backend->updateShares($exampleBook, [['href' => 'principal:principals/best-friend']], []); + $this->backend->updateShares($exampleBook, [['href' => 'principal:' . self::UNIT_TEST_USER1]], []); $shares = $this->backend->getShares($exampleBook->getResourceId()); $this->assertEquals(1, count($shares)); // adding the same sharee again has no effect - $this->backend->updateShares($exampleBook, [['href' => 'principal:principals/best-friend']], []); + $this->backend->updateShares($exampleBook, [['href' => 'principal:' . self::UNIT_TEST_USER1]], []); $shares = $this->backend->getShares($exampleBook->getResourceId()); $this->assertEquals(1, count($shares)); - $books = $this->backend->getAddressBooksForUser('principals/best-friend'); + $books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER1); $this->assertEquals(1, count($books)); - $this->backend->updateShares($exampleBook, [], ['principal:principals/best-friend']); + $this->backend->updateShares($exampleBook, [], ['principal:' . self::UNIT_TEST_USER1]); $shares = $this->backend->getShares($exampleBook->getResourceId()); $this->assertEquals(0, count($shares)); - $books = $this->backend->getAddressBooksForUser('principals/best-friend'); + $books = $this->backend->getAddressBooksForUser(self::UNIT_TEST_USER1); $this->assertEquals(0, count($books)); } @@ -398,7 +420,7 @@ class CardDavBackendTest extends TestCase { $cardId = 2; $backend = $this->getMockBuilder(CardDavBackend::class) - ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->dispatcher]) + ->setConstructorArgs([$this->db, $this->principal, $this->userManager, $this->groupManager, $this->dispatcher]) ->setMethods(['getCardId'])->getMock(); $backend->expects($this->any())->method('getCardId')->willReturn($cardId);