From f9c08112da9e263ccb98a48c228deb31f3046b61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Fri, 8 Jan 2016 12:11:02 +0100 Subject: [PATCH 1/2] Adding group principals to new dav endpoint --- apps/dav/lib/connector/sabre/principal.php | 24 ++- apps/dav/lib/dav/groupprincipalbackend.php | 153 ++++++++++++++++++ apps/dav/lib/rootcollection.php | 24 +-- .../tests/unit/connector/sabre/principal.php | 6 +- .../dav/tests/unit/dav/groupprincipaltest.php | 146 +++++++++++++++++ 5 files changed, 326 insertions(+), 27 deletions(-) create mode 100644 apps/dav/lib/dav/groupprincipalbackend.php create mode 100644 apps/dav/tests/unit/dav/groupprincipaltest.php diff --git a/apps/dav/lib/connector/sabre/principal.php b/apps/dav/lib/connector/sabre/principal.php index cc9c1c40d5..c917b11225 100644 --- a/apps/dav/lib/connector/sabre/principal.php +++ b/apps/dav/lib/connector/sabre/principal.php @@ -33,22 +33,20 @@ namespace OCA\DAV\Connector\Sabre; use OCP\IUser; use OCP\IUserManager; use OCP\IConfig; +use Sabre\DAV\Exception; use \Sabre\DAV\PropPatch; +use Sabre\DAVACL\PrincipalBackend\BackendInterface; use Sabre\HTTP\URLUtil; -class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface { - /** @var IConfig */ - private $config; +class Principal implements BackendInterface { + /** @var IUserManager */ private $userManager; /** - * @param IConfig $config * @param IUserManager $userManager */ - public function __construct(IConfig $config, - IUserManager $userManager) { - $this->config = $config; + public function __construct(IUserManager $userManager) { $this->userManager = $userManager; } @@ -108,13 +106,13 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface { * * @param string $principal * @return string[] - * @throws \Sabre\DAV\Exception + * @throws Exception */ public function getGroupMemberSet($principal) { // TODO: for now the group principal has only one member, the user itself $principal = $this->getPrincipalByPath($principal); if (!$principal) { - throw new \Sabre\DAV\Exception('Principal not found'); + throw new Exception('Principal not found'); } return [$principal['uri']]; @@ -125,7 +123,7 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface { * * @param string $principal * @return array - * @throws \Sabre\DAV\Exception + * @throws Exception */ public function getGroupMembership($principal) { list($prefix, $name) = URLUtil::splitPath($principal); @@ -134,7 +132,7 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface { if ($prefix === 'principals/users') { $principal = $this->getPrincipalByPath($principal); if (!$principal) { - throw new \Sabre\DAV\Exception('Principal not found'); + throw new Exception('Principal not found'); } // TODO: for now the user principal has only its own groups @@ -157,10 +155,10 @@ class Principal implements \Sabre\DAVACL\PrincipalBackend\BackendInterface { * * @param string $principal * @param string[] $members - * @throws \Sabre\DAV\Exception + * @throws Exception */ public function setGroupMemberSet($principal, array $members) { - throw new \Sabre\DAV\Exception('Setting members of the group is not supported yet'); + throw new Exception('Setting members of the group is not supported yet'); } /** diff --git a/apps/dav/lib/dav/groupprincipalbackend.php b/apps/dav/lib/dav/groupprincipalbackend.php new file mode 100644 index 0000000000..156dc2c128 --- /dev/null +++ b/apps/dav/lib/dav/groupprincipalbackend.php @@ -0,0 +1,153 @@ +groupManager = $IGroupManager; + } + + /** + * Returns a list of principals based on a prefix. + * + * This prefix will often contain something like 'principals'. You are only + * expected to return principals that are in this base path. + * + * You are expected to return at least a 'uri' for every user, you can + * return any additional properties if you wish so. Common properties are: + * {DAV:}displayname + * + * @param string $prefixPath + * @return string[] + */ + public function getPrincipalsByPrefix($prefixPath) { + $principals = []; + + if ($prefixPath === self::PRINCIPAL_PREFIX) { + foreach($this->groupManager->search('') as $user) { + $principals[] = $this->groupToPrincipal($user); + } + } + + return $principals; + } + + /** + * Returns a specific principal, specified by it's path. + * The returned structure should be the exact same as from + * getPrincipalsByPrefix. + * + * @param string $path + * @return array + */ + public function getPrincipalByPath($path) { + $elements = explode('/', $path); + if ($elements[0] !== 'principals') { + return null; + } + if ($elements[1] !== 'groups') { + return null; + } + $name = $elements[2]; + $user = $this->groupManager->get($name); + + if (!is_null($user)) { + return $this->groupToPrincipal($user); + } + + return null; + } + + /** + * Returns the list of members for a group-principal + * + * @param string $principal + * @return string[] + * @throws Exception + */ + public function getGroupMemberSet($principal) { + // TODO: implement if we want that + return []; + } + + /** + * Returns the list of groups a principal is a member of + * + * @param string $principal + * @return array + * @throws Exception + */ + public function getGroupMembership($principal) { + return []; + } + + /** + * Updates the list of group members for a group principal. + * + * The principals should be passed as a list of uri's. + * + * @param string $principal + * @param string[] $members + * @throws Exception + */ + public function setGroupMemberSet($principal, array $members) { + throw new Exception('Setting members of the group is not supported yet'); + } + + /** + * @param string $path + * @param PropPatch $propPatch + * @return int + */ + function updatePrincipal($path, PropPatch $propPatch) { + return 0; + } + + /** + * @param string $prefixPath + * @param array $searchProperties + * @param string $test + * @return array + */ + function searchPrincipals($prefixPath, array $searchProperties, $test = 'allof') { + return []; + } + + /** + * @param string $uri + * @param string $principalPrefix + * @return string + */ + function findByUri($uri, $principalPrefix) { + return ''; + } + + /** + * @param IGroup $group + * @return array + */ + protected function groupToPrincipal($group) { + $groupId = $group->getGID(); + $principal = [ + 'uri' => "principals/groups/$groupId", + '{DAV:}displayname' => $groupId, + ]; + + return $principal; + } +} diff --git a/apps/dav/lib/rootcollection.php b/apps/dav/lib/rootcollection.php index 96cc2bbc46..2261712200 100644 --- a/apps/dav/lib/rootcollection.php +++ b/apps/dav/lib/rootcollection.php @@ -6,6 +6,7 @@ use OCA\DAV\CalDAV\CalDavBackend; use OCA\DAV\CardDAV\AddressBookRoot; use OCA\DAV\CardDAV\CardDavBackend; use OCA\DAV\Connector\Sabre\Principal; +use OCA\DAV\DAV\GroupPrincipalBackend; use OCA\DAV\DAV\SystemPrincipalBackend; use Sabre\CalDAV\CalendarRoot; use Sabre\CalDAV\Principal\Collection; @@ -16,22 +17,26 @@ class RootCollection extends SimpleCollection { public function __construct() { $config = \OC::$server->getConfig(); $db = \OC::$server->getDatabaseConnection(); - $principalBackend = new Principal( - $config, - \OC::$server->getUserManager() + $userPrincipalBackend = new Principal( + \OC::$server->getUserManager() + ); + $groupPrincipalBackend = new GroupPrincipalBackend( + \OC::$server->getGroupManager() ); // as soon as debug mode is enabled we allow listing of principals $disableListing = !$config->getSystemValue('debug', false); // setup the first level of the dav tree - $userPrincipals = new Collection($principalBackend, 'principals/users'); + $userPrincipals = new Collection($userPrincipalBackend, 'principals/users'); $userPrincipals->disableListing = $disableListing; + $groupPrincipals = new Collection($groupPrincipalBackend, 'principals/groups'); + $groupPrincipals->disableListing = $disableListing; $systemPrincipals = new Collection(new SystemPrincipalBackend(), 'principals/system'); $systemPrincipals->disableListing = $disableListing; - $filesCollection = new Files\RootCollection($principalBackend, 'principals/users'); + $filesCollection = new Files\RootCollection($userPrincipalBackend, 'principals/users'); $filesCollection->disableListing = $disableListing; $caldavBackend = new CalDavBackend($db); - $calendarRoot = new CalendarRoot($principalBackend, $caldavBackend, 'principals/users'); + $calendarRoot = new CalendarRoot($userPrincipalBackend, $caldavBackend, 'principals/users'); $calendarRoot->disableListing = $disableListing; $systemTagCollection = new SystemTag\SystemTagsByIdCollection( \OC::$server->getSystemTagManager() @@ -41,17 +46,18 @@ class RootCollection extends SimpleCollection { \OC::$server->getSystemTagObjectMapper() ); - $usersCardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getLogger()); - $usersAddressBookRoot = new AddressBookRoot($principalBackend, $usersCardDavBackend, 'principals/users'); + $usersCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getLogger()); + $usersAddressBookRoot = new AddressBookRoot($userPrincipalBackend, $usersCardDavBackend, 'principals/users'); $usersAddressBookRoot->disableListing = $disableListing; - $systemCardDavBackend = new CardDavBackend($db, $principalBackend, \OC::$server->getLogger()); + $systemCardDavBackend = new CardDavBackend($db, $userPrincipalBackend, \OC::$server->getLogger()); $systemAddressBookRoot = new AddressBookRoot(new SystemPrincipalBackend(), $systemCardDavBackend, 'principals/system'); $systemAddressBookRoot->disableListing = $disableListing; $children = [ new SimpleCollection('principals', [ $userPrincipals, + $groupPrincipals, $systemPrincipals]), $filesCollection, $calendarRoot, diff --git a/apps/dav/tests/unit/connector/sabre/principal.php b/apps/dav/tests/unit/connector/sabre/principal.php index 9a6ae545b5..160d574426 100644 --- a/apps/dav/tests/unit/connector/sabre/principal.php +++ b/apps/dav/tests/unit/connector/sabre/principal.php @@ -17,18 +17,14 @@ use OCP\IConfig; class Principal extends \Test\TestCase { /** @var IUserManager */ private $userManager; - /** @var IConfig */ - private $config; /** @var \OCA\DAV\Connector\Sabre\Principal */ private $connector; public function setUp() { $this->userManager = $this->getMockBuilder('\OCP\IUserManager') ->disableOriginalConstructor()->getMock(); - $this->config = $this->getMockBuilder('\OCP\IConfig') - ->disableOriginalConstructor()->getMock(); - $this->connector = new \OCA\DAV\Connector\Sabre\Principal($this->config, $this->userManager); + $this->connector = new \OCA\DAV\Connector\Sabre\Principal($this->userManager); parent::setUp(); } diff --git a/apps/dav/tests/unit/dav/groupprincipaltest.php b/apps/dav/tests/unit/dav/groupprincipaltest.php new file mode 100644 index 0000000000..bb1bd2526e --- /dev/null +++ b/apps/dav/tests/unit/dav/groupprincipaltest.php @@ -0,0 +1,146 @@ +groupManager = $this->getMockBuilder('\OCP\IGroupManager') + ->disableOriginalConstructor()->getMock(); + + $this->connector = new GroupPrincipalBackend($this->groupManager); + parent::setUp(); + } + + public function testGetPrincipalsByPrefixWithoutPrefix() { + $response = $this->connector->getPrincipalsByPrefix(''); + $this->assertSame([], $response); + } + + public function testGetPrincipalsByPrefixWithUsers() { + $group1 = $this->mockGroup('foo'); + $group2 = $this->mockGroup('bar'); + $this->groupManager + ->expects($this->once()) + ->method('search') + ->with('') + ->will($this->returnValue([$group1, $group2])); + + $expectedResponse = [ + 0 => [ + 'uri' => 'principals/groups/foo', + '{DAV:}displayname' => 'foo' + ], + 1 => [ + 'uri' => 'principals/groups/bar', + '{DAV:}displayname' => 'bar', + ] + ]; + $response = $this->connector->getPrincipalsByPrefix('principals/groups'); + $this->assertSame($expectedResponse, $response); + } + + public function testGetPrincipalsByPrefixEmpty() { + $this->groupManager + ->expects($this->once()) + ->method('search') + ->with('') + ->will($this->returnValue([])); + + $response = $this->connector->getPrincipalsByPrefix('principals/groups'); + $this->assertSame([], $response); + } + + public function testGetPrincipalsByPathWithoutMail() { + $group1 = $this->mockGroup('foo'); + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('foo') + ->will($this->returnValue($group1)); + + $expectedResponse = [ + 'uri' => 'principals/groups/foo', + '{DAV:}displayname' => 'foo' + ]; + $response = $this->connector->getPrincipalByPath('principals/groups/foo'); + $this->assertSame($expectedResponse, $response); + } + + public function testGetPrincipalsByPathWithMail() { + $fooUser = $this->mockGroup('foo'); + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('foo') + ->will($this->returnValue($fooUser)); + + $expectedResponse = [ + 'uri' => 'principals/groups/foo', + '{DAV:}displayname' => 'foo', + ]; + $response = $this->connector->getPrincipalByPath('principals/groups/foo'); + $this->assertSame($expectedResponse, $response); + } + + public function testGetPrincipalsByPathEmpty() { + $this->groupManager + ->expects($this->once()) + ->method('get') + ->with('foo') + ->will($this->returnValue(null)); + + $response = $this->connector->getPrincipalByPath('principals/groups/foo'); + $this->assertSame(null, $response); + } + + public function testGetGroupMemberSet() { + $response = $this->connector->getGroupMemberSet('principals/groups/foo'); + $this->assertSame([], $response); + } + + public function testGetGroupMembership() { + $response = $this->connector->getGroupMembership('principals/groups/foo'); + $this->assertSame([], $response); + } + + /** + * @expectedException \Sabre\DAV\Exception + * @expectedExceptionMessage Setting members of the group is not supported yet + */ + public function testSetGroupMembership() { + $this->connector->setGroupMemberSet('principals/groups/foo', ['foo']); + } + + public function testUpdatePrincipal() { + $this->assertSame(0, $this->connector->updatePrincipal('foo', new PropPatch(array()))); + } + + public function testSearchPrincipals() { + $this->assertSame([], $this->connector->searchPrincipals('principals/groups', [])); + } + + /** + * @return PHPUnit_Framework_MockObject_MockObject + */ + private function mockGroup($gid) { + $fooUser = $this->getMockBuilder('\OC\Group\Group') + ->disableOriginalConstructor()->getMock(); + $fooUser + ->expects($this->exactly(1)) + ->method('getGID') + ->will($this->returnValue($gid)); + return $fooUser; + } +} From bb01efdcbd0f290c7a6f26ca99fd89f09b29735d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= Date: Thu, 10 Dec 2015 16:57:46 +0100 Subject: [PATCH 2/2] Adding unit tests for SystemPrincipalBackend --- apps/dav/lib/dav/systemprincipalbackend.php | 6 +- .../unit/dav/systemprincipalbackendtest.php | 131 ++++++++++++++++++ apps/dav/tests/unit/phpunit.xml | 2 +- 3 files changed, 133 insertions(+), 6 deletions(-) create mode 100644 apps/dav/tests/unit/dav/systemprincipalbackendtest.php diff --git a/apps/dav/lib/dav/systemprincipalbackend.php b/apps/dav/lib/dav/systemprincipalbackend.php index 2c2049ace6..f65cd8bfc3 100644 --- a/apps/dav/lib/dav/systemprincipalbackend.php +++ b/apps/dav/lib/dav/systemprincipalbackend.php @@ -65,11 +65,7 @@ class SystemPrincipalBackend extends AbstractBackend { */ function getPrincipalByPath($path) { - $elements = explode('/', $path); - if ($elements[0] !== 'principals') { - return null; - } - if ($elements[1] === 'system') { + if ($path === 'principals/system/system') { $principal = [ 'uri' => 'principals/system/system', '{DAV:}displayname' => 'system', diff --git a/apps/dav/tests/unit/dav/systemprincipalbackendtest.php b/apps/dav/tests/unit/dav/systemprincipalbackendtest.php new file mode 100644 index 0000000000..96b1a8592c --- /dev/null +++ b/apps/dav/tests/unit/dav/systemprincipalbackendtest.php @@ -0,0 +1,131 @@ + + * + * @copyright Copyright (c) 2015, 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 + * + */ + +namespace OCA\DAV\Tests\DAV; + +use OCA\DAV\DAV\SystemPrincipalBackend; +use Test\TestCase; + +class SystemPrincipalBackendTest extends TestCase { + + /** + * @dataProvider providesPrefix + * @param $expected + * @param $prefix + */ + public function testGetPrincipalsByPrefix($expected, $prefix) { + $backend = new SystemPrincipalBackend(); + $result = $backend->getPrincipalsByPrefix($prefix); + $this->assertEquals($expected, $result); + } + + public function providesPrefix() { + return [ + [[], ''], + [[[ + 'uri' => 'principals/system/system', + '{DAV:}displayname' => 'system', + ]], 'principals/system'], + ]; + } + + /** + * @dataProvider providesPath + * @param $expected + * @param $path + */ + public function testGetPrincipalByPath($expected, $path) { + $backend = new SystemPrincipalBackend(); + $result = $backend->getPrincipalByPath($path); + $this->assertEquals($expected, $result); + } + + public function providesPath() { + return [ + [null, ''], + [null, 'principals'], + [null, 'principals/system'], + [[ + 'uri' => 'principals/system/system', + '{DAV:}displayname' => 'system', + ], 'principals/system/system'], + ]; + } + + /** + * @dataProvider providesPrincipalForGetGroupMemberSet + * @expectedException \Sabre\DAV\Exception + * @expectedExceptionMessage Principal not found + * + * @param string $principal + * @throws \Sabre\DAV\Exception + */ + public function testGetGroupMemberSetExceptional($principal) { + $backend = new SystemPrincipalBackend(); + $backend->getGroupMemberSet($principal); + } + + public function providesPrincipalForGetGroupMemberSet() { + return [ + [null], + ['principals/system'], + ]; + } + + /** + * @throws \Sabre\DAV\Exception + */ + public function testGetGroupMemberSet() { + $backend = new SystemPrincipalBackend(); + $result = $backend->getGroupMemberSet('principals/system/system'); + $this->assertEquals(['principals/system/system'], $result); + } + + /** + * @dataProvider providesPrincipalForGetGroupMembership + * @expectedException \Sabre\DAV\Exception + * @expectedExceptionMessage Principal not found + * + * @param string $principal + * @throws \Sabre\DAV\Exception + */ + public function testGetGroupMembershipExceptional($principal) { + $backend = new SystemPrincipalBackend(); + $backend->getGroupMembership($principal); + } + + public function providesPrincipalForGetGroupMembership() { + return [ + ['principals/system/a'], + ]; + } + + /** + * @throws \Sabre\DAV\Exception + */ + public function testGetGroupMembership() { + $backend = new SystemPrincipalBackend(); + $result = $backend->getGroupMembership('principals/system/system'); + $this->assertEquals([], $result); + } + + +} diff --git a/apps/dav/tests/unit/phpunit.xml b/apps/dav/tests/unit/phpunit.xml index 46c3cdfb34..314855d863 100644 --- a/apps/dav/tests/unit/phpunit.xml +++ b/apps/dav/tests/unit/phpunit.xml @@ -6,7 +6,7 @@ timeoutForLargeTests="900" > - . + .